From 3baeac796a2eddc1f665cb9ccc45c014ccac8ea7 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 6 Jul 2023 14:01:00 +1200 Subject: [PATCH 1/5] test util for oidcclientconfigs --- test/test-utils/oidc.ts | 52 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 test/test-utils/oidc.ts diff --git a/test/test-utils/oidc.ts b/test/test-utils/oidc.ts new file mode 100644 index 00000000000..526fb5fd319 --- /dev/null +++ b/test/test-utils/oidc.ts @@ -0,0 +1,52 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { OidcClientConfig } from "matrix-js-sdk/src/autodiscovery"; +import { ValidatedIssuerMetadata } from "matrix-js-sdk/src/oidc/validate"; + +/** + * Makes a valid OidcClientConfig with minimum valid values + * @param issuer used as the base for all other urls + * @returns OidcClientConfig + */ +export const makeDelegatedAuthConfig = (issuer = "https://auth.org/"): OidcClientConfig => { + const metadata = mockOpenIdConfiguration(issuer); + + return { + issuer, + account: issuer + "account", + registrationEndpoint: metadata.registration_endpoint, + authorizationEndpoint: metadata.authorization_endpoint, + tokenEndpoint: metadata.token_endpoint, + metadata, + }; +}; + +/** + * Useful for mocking /.well-known/openid-configuration + * @param issuer used as the base for all other urls + * @returns ValidatedIssuerMetadata + */ +export const mockOpenIdConfiguration = (issuer = "https://auth.org/"): ValidatedIssuerMetadata => ({ + issuer, + revocation_endpoint: issuer + "revoke", + token_endpoint: issuer + "token", + authorization_endpoint: issuer + "auth", + registration_endpoint: issuer + "registration", + response_types_supported: ["query"], + grant_types_supported: ["code", "refreshToken"], + code_challenge_methods_supported: ["S256"], +}); From fcfcfd9c30a2da70898f674d1c561f4f8f474352 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 6 Jul 2023 14:01:17 +1200 Subject: [PATCH 2/5] rename type and lint --- src/utils/AutoDiscoveryUtils.tsx | 28 +++++++------- src/utils/ValidatedServerConfig.ts | 3 +- src/utils/oidc/authorize.ts | 59 ++++++++++++++---------------- src/utils/oidc/registerClient.ts | 1 + test/utils/oidc/authorize-test.ts | 28 +++++--------- 5 files changed, 53 insertions(+), 66 deletions(-) diff --git a/src/utils/AutoDiscoveryUtils.tsx b/src/utils/AutoDiscoveryUtils.tsx index 213bc5ea986..2c8a4daf29a 100644 --- a/src/utils/AutoDiscoveryUtils.tsx +++ b/src/utils/AutoDiscoveryUtils.tsx @@ -15,14 +15,14 @@ limitations under the License. */ import React, { ReactNode } from "react"; -import { AutoDiscovery, ClientConfig } from "matrix-js-sdk/src/autodiscovery"; +import { AutoDiscovery, ClientConfig, OidcClientConfig } from "matrix-js-sdk/src/autodiscovery"; import { M_AUTHENTICATION } from "matrix-js-sdk/src/client"; import { logger } from "matrix-js-sdk/src/logger"; import { IClientWellKnown } from "matrix-js-sdk/src/matrix"; import { _t, UserFriendlyError } from "../languageHandler"; import SdkConfig from "../SdkConfig"; -import { ValidatedDelegatedAuthConfig, ValidatedServerConfig } from "./ValidatedServerConfig"; +import { ValidatedServerConfig } from "./ValidatedServerConfig"; const LIVELINESS_DISCOVERY_ERRORS: string[] = [ AutoDiscovery.ERROR_INVALID_HOMESERVER, @@ -261,25 +261,25 @@ export default class AutoDiscoveryUtils { throw new UserFriendlyError("Unexpected error resolving homeserver configuration"); } - let delegatedAuthentication: - | { - authorizationEndpoint: string; - registrationEndpoint?: string; - tokenEndpoint: string; - account?: string; - issuer: string; - } - | undefined; + let delegatedAuthentication: OidcClientConfig | undefined; if (discoveryResult[M_AUTHENTICATION.stable!]?.state === AutoDiscovery.SUCCESS) { - const { authorizationEndpoint, registrationEndpoint, tokenEndpoint, account, issuer } = discoveryResult[ - M_AUTHENTICATION.stable! - ] as ValidatedDelegatedAuthConfig; + const { + authorizationEndpoint, + registrationEndpoint, + tokenEndpoint, + account, + issuer, + metadata, + signingKeys, + } = discoveryResult[M_AUTHENTICATION.stable!] as OidcClientConfig; delegatedAuthentication = Object.freeze({ authorizationEndpoint, registrationEndpoint, tokenEndpoint, account, issuer, + metadata, + signingKeys, }); } diff --git a/src/utils/ValidatedServerConfig.ts b/src/utils/ValidatedServerConfig.ts index cb3edf3a9c8..b41ac6a642c 100644 --- a/src/utils/ValidatedServerConfig.ts +++ b/src/utils/ValidatedServerConfig.ts @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { OidcClientConfig } from "matrix-js-sdk/src/autodiscovery"; import { IDelegatedAuthConfig } from "matrix-js-sdk/src/client"; import { ValidatedIssuerConfig } from "matrix-js-sdk/src/oidc/validate"; @@ -38,5 +39,5 @@ export interface ValidatedServerConfig { * From homeserver .well-known m.authentication, and issuer's .well-known/openid-configuration * Used for OIDC native flow authentication */ - delegatedAuthentication?: ValidatedDelegatedAuthConfig; + delegatedAuthentication?: OidcClientConfig; } diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 22e7a11bce1..3ae7de612d2 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -14,33 +14,24 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { - AuthorizationParams, - generateAuthorizationParams, - generateAuthorizationUrl, -} from "matrix-js-sdk/src/oidc/authorize"; +import { OidcClientConfig } from "matrix-js-sdk/src/autodiscovery"; +import { generateOidcAuthorizationUrl } from "matrix-js-sdk/src/oidc/authorize"; +import { randomString } from "matrix-js-sdk/src/randomstring"; -import { ValidatedDelegatedAuthConfig } from "../ValidatedServerConfig"; +import { SSO_HOMESERVER_URL_KEY, SSO_ID_SERVER_URL_KEY } from "../../BasePlatform"; /** - * Store authorization params for retrieval when returning from OIDC OP - * @param authorizationParams from `generateAuthorizationParams` - * @param delegatedAuthConfig used for future interactions with OP - * @param clientId this client's id as registered with configured issuer + * Store homeserver for retrieval when returning from OIDC OP * @param homeserver target homeserver + * @param identityServerUrl OPTIONAL target identity server */ -const storeAuthorizationParams = ( - { redirectUri, state, nonce, codeVerifier }: AuthorizationParams, - { issuer }: ValidatedDelegatedAuthConfig, - clientId: string, - homeserver: string, -): void => { - window.sessionStorage.setItem(`oidc_${state}_nonce`, nonce); - window.sessionStorage.setItem(`oidc_${state}_redirectUri`, redirectUri); - window.sessionStorage.setItem(`oidc_${state}_codeVerifier`, codeVerifier); - window.sessionStorage.setItem(`oidc_${state}_clientId`, clientId); - window.sessionStorage.setItem(`oidc_${state}_issuer`, issuer); - window.sessionStorage.setItem(`oidc_${state}_homeserver`, homeserver); +const persistAuthorizationParams = (nonce, homeserverUrl: string, identityServerUrl?: string): void => { + // persist hs url and is url for when the user is returned to the app + sessionStorage.setItem(`oidc_nonce`, nonce); + sessionStorage.setItem(SSO_HOMESERVER_URL_KEY, homeserverUrl); + if (identityServerUrl) { + sessionStorage.setItem(SSO_ID_SERVER_URL_KEY, identityServerUrl); + } }; /** @@ -49,25 +40,29 @@ const storeAuthorizationParams = ( * Navigates to configured authorization endpoint * @param delegatedAuthConfig from discovery * @param clientId this client's id as registered with configured issuer - * @param homeserver target homeserver + * @param homeserverUrl target homeserver + * @param identityServerUrl OPTIONAL target identity server * @returns Promise that resolves after we have navigated to auth endpoint */ export const startOidcLogin = async ( - delegatedAuthConfig: ValidatedDelegatedAuthConfig, + delegatedAuthConfig: OidcClientConfig, clientId: string, - homeserver: string, + homeserverUrl: string, + identityServerUrl?: string, ): Promise => { - // TODO(kerrya) afterloginfragment https://github.com/vector-im/element-web/issues/25656 const redirectUri = window.location.origin; - const authParams = generateAuthorizationParams({ redirectUri }); - storeAuthorizationParams(authParams, delegatedAuthConfig, clientId, homeserver); + const nonce = randomString(10); + + persistAuthorizationParams(nonce, homeserverUrl, identityServerUrl); - const authorizationUrl = await generateAuthorizationUrl( - delegatedAuthConfig.authorizationEndpoint, + const authorizationUrl = await generateOidcAuthorizationUrl({ + metadata: delegatedAuthConfig.metadata, + redirectUri, clientId, - authParams, - ); + homeserverUrl, + nonce, + }); window.location.href = authorizationUrl; }; diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index 309709e18f1..355cc1c6303 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -52,6 +52,7 @@ export const getOidcClientId = async ( baseUrl: string, staticOidcClients?: IConfigOptions["oidc_static_clients"], ): Promise => { + console.log("hhh dele", delegatedAuthConfig); const staticClientId = getStaticOidcClientId(delegatedAuthConfig.issuer, staticOidcClients); if (staticClientId) { logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.issuer}`); diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index 5abdb19862a..d799da776f6 100644 --- a/test/utils/oidc/authorize-test.ts +++ b/test/utils/oidc/authorize-test.ts @@ -18,20 +18,15 @@ import fetchMockJest from "fetch-mock-jest"; import * as randomStringUtils from "matrix-js-sdk/src/randomstring"; import { startOidcLogin } from "../../../src/utils/oidc/authorize"; +import { makeDelegatedAuthConfig, mockOpenIdConfiguration } from "../../test-utils/oidc"; describe("startOidcLogin()", () => { const issuer = "https://auth.com/"; - const authorizationEndpoint = "https://auth.com/authorization"; const homeserver = "https://matrix.org"; const clientId = "xyz789"; const baseUrl = "https://test.com"; - const delegatedAuthConfig = { - issuer, - registrationEndpoint: issuer + "registration", - authorizationEndpoint, - tokenEndpoint: issuer + "token", - }; + const delegatedAuthConfig = makeDelegatedAuthConfig(issuer); const sessionStorageGetSpy = jest.spyOn(sessionStorage.__proto__, "setItem").mockReturnValue(undefined); const randomStringMockImpl = (length: number) => new Array(length).fill("x").join(""); @@ -53,6 +48,10 @@ describe("startOidcLogin()", () => { origin: baseUrl, }; + fetchMockJest.get( + delegatedAuthConfig.metadata.issuer + ".well-known/openid-configuration", + mockOpenIdConfiguration(), + ); jest.spyOn(randomStringUtils, "randomString").mockRestore(); }); @@ -60,21 +59,12 @@ describe("startOidcLogin()", () => { window.location = realWindowLocation; }); - it("should store authorization params in session storage", async () => { + it("should store auth params in session storage", async () => { jest.spyOn(randomStringUtils, "randomString").mockReset().mockImplementation(randomStringMockImpl); await startOidcLogin(delegatedAuthConfig, clientId, homeserver); - const state = randomStringUtils.randomString(8); - - expect(sessionStorageGetSpy).toHaveBeenCalledWith(`oidc_${state}_nonce`, randomStringUtils.randomString(8)); - expect(sessionStorageGetSpy).toHaveBeenCalledWith(`oidc_${state}_redirectUri`, baseUrl); - expect(sessionStorageGetSpy).toHaveBeenCalledWith( - `oidc_${state}_codeVerifier`, - randomStringUtils.randomString(64), - ); - expect(sessionStorageGetSpy).toHaveBeenCalledWith(`oidc_${state}_clientId`, clientId); - expect(sessionStorageGetSpy).toHaveBeenCalledWith(`oidc_${state}_issuer`, issuer); - expect(sessionStorageGetSpy).toHaveBeenCalledWith(`oidc_${state}_homeserver`, homeserver); + expect(sessionStorageGetSpy).toHaveBeenCalledWith(`mx_sso_hs_url`, homeserver); + expect(sessionStorageGetSpy).toHaveBeenCalledWith(`oidc_nonce`, expect.anything()); }); it("navigates to authorization endpoint with correct parameters", async () => { From 8824cb8f29a4e779628b6c6e04be89e065a72ee6 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 6 Jul 2023 16:07:29 +1200 Subject: [PATCH 3/5] correct oidc test util --- test/test-utils/oidc.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/test-utils/oidc.ts b/test/test-utils/oidc.ts index 526fb5fd319..569a09cff27 100644 --- a/test/test-utils/oidc.ts +++ b/test/test-utils/oidc.ts @@ -22,7 +22,7 @@ import { ValidatedIssuerMetadata } from "matrix-js-sdk/src/oidc/validate"; * @param issuer used as the base for all other urls * @returns OidcClientConfig */ -export const makeDelegatedAuthConfig = (issuer = "https://auth.org/"): OidcClientConfig => { + export const makeDelegatedAuthConfig = (issuer = "https://auth.org/"): OidcClientConfig => { const metadata = mockOpenIdConfiguration(issuer); return { @@ -46,7 +46,8 @@ export const mockOpenIdConfiguration = (issuer = "https://auth.org/"): Validated token_endpoint: issuer + "token", authorization_endpoint: issuer + "auth", registration_endpoint: issuer + "registration", - response_types_supported: ["query"], - grant_types_supported: ["code", "refreshToken"], + jwks_uri: issuer + "jwks", + response_types_supported: ["code"], + grant_types_supported: ["authorization_code", "refresh_token"], code_challenge_methods_supported: ["S256"], }); From 59c5d5e6f270fc22cae1d54c96e1d118747dc840 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 6 Jul 2023 17:08:03 +1200 Subject: [PATCH 4/5] store issuer and clientId pre auth navigation --- src/utils/oidc/authorize.ts | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 3ae7de612d2..614fb623ad6 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -15,22 +15,35 @@ limitations under the License. */ import { OidcClientConfig } from "matrix-js-sdk/src/autodiscovery"; +import { IDelegatedAuthConfig } from "matrix-js-sdk/src/client"; import { generateOidcAuthorizationUrl } from "matrix-js-sdk/src/oidc/authorize"; import { randomString } from "matrix-js-sdk/src/randomstring"; import { SSO_HOMESERVER_URL_KEY, SSO_ID_SERVER_URL_KEY } from "../../BasePlatform"; +type StoredAuthorizationParams = IDelegatedAuthConfig & { + nonce: string; + homeserverUrl: string; + identityServerUrl?: string; + clientId: string; +} + /** * Store homeserver for retrieval when returning from OIDC OP * @param homeserver target homeserver * @param identityServerUrl OPTIONAL target identity server */ -const persistAuthorizationParams = (nonce, homeserverUrl: string, identityServerUrl?: string): void => { +const persistAuthorizationParams = (state, { + nonce, homeserverUrl, identityServerUrl, issuer, account, clientId +}: StoredAuthorizationParams): void => { // persist hs url and is url for when the user is returned to the app - sessionStorage.setItem(`oidc_nonce`, nonce); - sessionStorage.setItem(SSO_HOMESERVER_URL_KEY, homeserverUrl); + sessionStorage.setItem(`mx_oidc_${state}_nonce`, nonce); + sessionStorage.setItem(`mx_oidc_${state}_issuer`, issuer); + sessionStorage.setItem(`mx_oidc_${state}_account`, account); + sessionStorage.setItem(`mx_oidc_${state}_clientId`, clientId); + sessionStorage.setItem(`mx_oidc_${state}_homeserverUrl`, homeserverUrl); if (identityServerUrl) { - sessionStorage.setItem(SSO_ID_SERVER_URL_KEY, identityServerUrl); + sessionStorage.setItem(`mx_oidc_${state}_identityServerUrl`, identityServerUrl); } }; @@ -53,8 +66,15 @@ export const startOidcLogin = async ( const redirectUri = window.location.origin; const nonce = randomString(10); - - persistAuthorizationParams(nonce, homeserverUrl, identityServerUrl); + const state = randomString(10); + persistAuthorizationParams(state, { + nonce, + issuer: delegatedAuthConfig.issuer, + account: delegatedAuthConfig.account, + homeserverUrl, + identityServerUrl, + clientId + }); const authorizationUrl = await generateOidcAuthorizationUrl({ metadata: delegatedAuthConfig.metadata, @@ -62,6 +82,7 @@ export const startOidcLogin = async ( clientId, homeserverUrl, nonce, + state, }); window.location.href = authorizationUrl; From d2f7f55ce61acdf4b3850c7c2f27abc4188e3a9d Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 7 Jul 2023 15:42:58 +1200 Subject: [PATCH 5/5] update for js-sdk userstate, tidy --- src/utils/oidc/authorize.ts | 40 +------------------------------ src/utils/oidc/registerClient.ts | 1 - test/test-utils/oidc.ts | 2 +- test/utils/oidc/authorize-test.ts | 9 ------- 4 files changed, 2 insertions(+), 50 deletions(-) diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 614fb623ad6..823e3cfab4a 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -15,38 +15,9 @@ limitations under the License. */ import { OidcClientConfig } from "matrix-js-sdk/src/autodiscovery"; -import { IDelegatedAuthConfig } from "matrix-js-sdk/src/client"; import { generateOidcAuthorizationUrl } from "matrix-js-sdk/src/oidc/authorize"; import { randomString } from "matrix-js-sdk/src/randomstring"; -import { SSO_HOMESERVER_URL_KEY, SSO_ID_SERVER_URL_KEY } from "../../BasePlatform"; - -type StoredAuthorizationParams = IDelegatedAuthConfig & { - nonce: string; - homeserverUrl: string; - identityServerUrl?: string; - clientId: string; -} - -/** - * Store homeserver for retrieval when returning from OIDC OP - * @param homeserver target homeserver - * @param identityServerUrl OPTIONAL target identity server - */ -const persistAuthorizationParams = (state, { - nonce, homeserverUrl, identityServerUrl, issuer, account, clientId -}: StoredAuthorizationParams): void => { - // persist hs url and is url for when the user is returned to the app - sessionStorage.setItem(`mx_oidc_${state}_nonce`, nonce); - sessionStorage.setItem(`mx_oidc_${state}_issuer`, issuer); - sessionStorage.setItem(`mx_oidc_${state}_account`, account); - sessionStorage.setItem(`mx_oidc_${state}_clientId`, clientId); - sessionStorage.setItem(`mx_oidc_${state}_homeserverUrl`, homeserverUrl); - if (identityServerUrl) { - sessionStorage.setItem(`mx_oidc_${state}_identityServerUrl`, identityServerUrl); - } -}; - /** * Start OIDC authorization code flow * Generates auth params, stores them in session storage and @@ -66,23 +37,14 @@ export const startOidcLogin = async ( const redirectUri = window.location.origin; const nonce = randomString(10); - const state = randomString(10); - persistAuthorizationParams(state, { - nonce, - issuer: delegatedAuthConfig.issuer, - account: delegatedAuthConfig.account, - homeserverUrl, - identityServerUrl, - clientId - }); const authorizationUrl = await generateOidcAuthorizationUrl({ metadata: delegatedAuthConfig.metadata, redirectUri, clientId, homeserverUrl, + identityServerUrl, nonce, - state, }); window.location.href = authorizationUrl; diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index 355cc1c6303..309709e18f1 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -52,7 +52,6 @@ export const getOidcClientId = async ( baseUrl: string, staticOidcClients?: IConfigOptions["oidc_static_clients"], ): Promise => { - console.log("hhh dele", delegatedAuthConfig); const staticClientId = getStaticOidcClientId(delegatedAuthConfig.issuer, staticOidcClients); if (staticClientId) { logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.issuer}`); diff --git a/test/test-utils/oidc.ts b/test/test-utils/oidc.ts index 569a09cff27..aa042eba5d4 100644 --- a/test/test-utils/oidc.ts +++ b/test/test-utils/oidc.ts @@ -22,7 +22,7 @@ import { ValidatedIssuerMetadata } from "matrix-js-sdk/src/oidc/validate"; * @param issuer used as the base for all other urls * @returns OidcClientConfig */ - export const makeDelegatedAuthConfig = (issuer = "https://auth.org/"): OidcClientConfig => { +export const makeDelegatedAuthConfig = (issuer = "https://auth.org/"): OidcClientConfig => { const metadata = mockOpenIdConfiguration(issuer); return { diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index d799da776f6..3dda4da2e9f 100644 --- a/test/utils/oidc/authorize-test.ts +++ b/test/utils/oidc/authorize-test.ts @@ -29,7 +29,6 @@ describe("startOidcLogin()", () => { const delegatedAuthConfig = makeDelegatedAuthConfig(issuer); const sessionStorageGetSpy = jest.spyOn(sessionStorage.__proto__, "setItem").mockReturnValue(undefined); - const randomStringMockImpl = (length: number) => new Array(length).fill("x").join(""); // to restore later const realWindowLocation = window.location; @@ -59,14 +58,6 @@ describe("startOidcLogin()", () => { window.location = realWindowLocation; }); - it("should store auth params in session storage", async () => { - jest.spyOn(randomStringUtils, "randomString").mockReset().mockImplementation(randomStringMockImpl); - await startOidcLogin(delegatedAuthConfig, clientId, homeserver); - - expect(sessionStorageGetSpy).toHaveBeenCalledWith(`mx_sso_hs_url`, homeserver); - expect(sessionStorageGetSpy).toHaveBeenCalledWith(`oidc_nonce`, expect.anything()); - }); - it("navigates to authorization endpoint with correct parameters", async () => { await startOidcLogin(delegatedAuthConfig, clientId, homeserver);