From feb7e9899bc64133bc2f6b0832382a4caf7eb27e Mon Sep 17 00:00:00 2001 From: Kerry Date: Wed, 4 Oct 2023 17:06:04 +1300 Subject: [PATCH] OIDC: persist id token claims (#11691) * persist idTokenClaims * tests * remove unused cde --- src/Lifecycle.ts | 4 +-- src/utils/oidc/authorize.ts | 6 +++- src/utils/oidc/persistOidcSettings.ts | 10 ++++++- .../components/structures/MatrixChat-test.tsx | 27 ++++++++++++------ test/utils/oidc/authorize-test.ts | 28 +++++++++++++------ test/utils/oidc/persistOidcSettings-test.ts | 19 +++++++++++-- 6 files changed, 70 insertions(+), 24 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index bfef686ea8..e11b9b8a01 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -272,7 +272,7 @@ export async function attemptDelegatedAuthLogin( */ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise { try { - const { accessToken, refreshToken, homeserverUrl, identityServerUrl, clientId, issuer } = + const { accessToken, refreshToken, homeserverUrl, identityServerUrl, idTokenClaims, clientId, issuer } = await completeOidcLogin(queryParams); const { @@ -294,7 +294,7 @@ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise logger.debug("Logged in via OIDC native flow"); await onSuccessfulDelegatedAuthLogin(credentials); // this needs to happen after success handler which clears storages - persistOidcAuthenticatedSettings(clientId, issuer); + persistOidcAuthenticatedSettings(clientId, issuer, idTokenClaims); return true; } catch (error) { logger.error("Failed to login via OIDC", error); diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index e475c1795c..3e6de7de14 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -18,6 +18,7 @@ import { completeAuthorizationCodeGrant, generateOidcAuthorizationUrl } from "ma import { QueryDict } from "matrix-js-sdk/src/utils"; import { OidcClientConfig } from "matrix-js-sdk/src/matrix"; import { randomString } from "matrix-js-sdk/src/randomstring"; +import { IdTokenClaims } from "oidc-client-ts"; /** * Start OIDC authorization code flow @@ -81,6 +82,8 @@ type CompleteOidcLoginResponse = { clientId: string; // issuer used during authentication issuer: string; + // claims of the given access token; used during token refresh to validate new tokens + idTokenClaims: IdTokenClaims; }; /** * Attempt to complete authorization code flow to get an access token @@ -90,7 +93,7 @@ type CompleteOidcLoginResponse = { */ export const completeOidcLogin = async (queryParams: QueryDict): Promise => { const { code, state } = getCodeAndStateFromQueryParams(queryParams); - const { homeserverUrl, tokenResponse, identityServerUrl, oidcClientSettings } = + const { homeserverUrl, tokenResponse, idTokenClaims, identityServerUrl, oidcClientSettings } = await completeAuthorizationCodeGrant(code, state); return { @@ -100,5 +103,6 @@ export const completeOidcLogin = async (queryParams: QueryDict): Promise { +export const persistOidcAuthenticatedSettings = ( + clientId: string, + issuer: string, + idTokenClaims: IdTokenClaims, +): void => { sessionStorage.setItem(clientIdStorageKey, clientId); sessionStorage.setItem(tokenIssuerStorageKey, issuer); + sessionStorage.setItem(idTokenClaimsStorageKey, JSON.stringify(idTokenClaims)); }; /** diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index ac8ad257f7..3b5ed9390f 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -924,15 +924,24 @@ describe("", () => { }; beforeEach(() => { - mocked(completeAuthorizationCodeGrant).mockClear().mockResolvedValue({ - oidcClientSettings: { - clientId, - issuer, - }, - tokenResponse, - homeserverUrl, - identityServerUrl, - }); + mocked(completeAuthorizationCodeGrant) + .mockClear() + .mockResolvedValue({ + oidcClientSettings: { + clientId, + issuer, + }, + tokenResponse, + homeserverUrl, + identityServerUrl, + idTokenClaims: { + aud: "123", + iss: issuer, + sub: "123", + exp: 123, + iat: 456, + }, + }); jest.spyOn(logger, "error").mockClear(); }); diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index 2f5f42db23..daf65937c8 100644 --- a/test/utils/oidc/authorize-test.ts +++ b/test/utils/oidc/authorize-test.ts @@ -104,15 +104,24 @@ describe("OIDC authorization", () => { }; beforeEach(() => { - mocked(completeAuthorizationCodeGrant).mockClear().mockResolvedValue({ - oidcClientSettings: { - clientId, - issuer, - }, - tokenResponse, - homeserverUrl, - identityServerUrl, - }); + mocked(completeAuthorizationCodeGrant) + .mockClear() + .mockResolvedValue({ + oidcClientSettings: { + clientId, + issuer, + }, + tokenResponse, + homeserverUrl, + identityServerUrl, + idTokenClaims: { + aud: "123", + iss: issuer, + sub: "123", + exp: 123, + iat: 456, + }, + }); }); it("should throw when query params do not include state and code", async () => { @@ -137,6 +146,7 @@ describe("OIDC authorization", () => { identityServerUrl, issuer, clientId, + idTokenClaims: result.idTokenClaims, }); }); }); diff --git a/test/utils/oidc/persistOidcSettings-test.ts b/test/utils/oidc/persistOidcSettings-test.ts index 4db5c4e75c..f71a7f3ed6 100644 --- a/test/utils/oidc/persistOidcSettings-test.ts +++ b/test/utils/oidc/persistOidcSettings-test.ts @@ -14,6 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { IdTokenClaims } from "oidc-client-ts"; + import { getStoredOidcClientId, getStoredOidcTokenIssuer, @@ -29,12 +31,25 @@ describe("persist OIDC settings", () => { const clientId = "test-client-id"; const issuer = "https://auth.org/"; + const idTokenClaims: IdTokenClaims = { + // audience is this client + aud: "123", + // issuer matches + iss: issuer, + sub: "123", + exp: 123, + iat: 456, + }; describe("persistOidcAuthenticatedSettings", () => { it("should set clientId and issuer in session storage", () => { - persistOidcAuthenticatedSettings(clientId, issuer); + persistOidcAuthenticatedSettings(clientId, issuer, idTokenClaims); expect(sessionStorage.setItem).toHaveBeenCalledWith("mx_oidc_client_id", clientId); expect(sessionStorage.setItem).toHaveBeenCalledWith("mx_oidc_token_issuer", issuer); + expect(sessionStorage.setItem).toHaveBeenCalledWith( + "mx_oidc_id_token_claims", + JSON.stringify(idTokenClaims), + ); }); }); @@ -50,7 +65,7 @@ describe("persist OIDC settings", () => { }); }); - describe("Name of the group", () => { + describe("getStoredOidcClientId()", () => { it("should return clientId from session storage", () => { jest.spyOn(sessionStorage.__proto__, "getItem").mockReturnValue(clientId); expect(getStoredOidcClientId()).toEqual(clientId);