From 90e65e849009adde0c39fd523832ee98509917c6 Mon Sep 17 00:00:00 2001 From: Kerry Date: Wed, 5 Jul 2023 11:10:03 +1200 Subject: [PATCH] use more future proof config for static clients (#11175) --- src/IConfigOptions.ts | 7 ++++++- src/Login.ts | 6 +++--- src/utils/oidc/registerClient.ts | 10 +++++++--- test/components/structures/auth/Login-test.tsx | 6 ++++-- test/utils/oidc/registerClient-test.ts | 8 ++++---- 5 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/IConfigOptions.ts b/src/IConfigOptions.ts index 286942476b..2df5e7fca1 100644 --- a/src/IConfigOptions.ts +++ b/src/IConfigOptions.ts @@ -201,7 +201,12 @@ export interface IConfigOptions { * The issuer URL must have a trailing `/`. * OPTIONAL */ - oidc_static_client_ids?: Record; + oidc_static_clients?: Record< + string, + { + client_id: string; + } + >; } export interface ISsoRedirectOptions { diff --git a/src/Login.ts b/src/Login.ts index 42a5062210..644648b7cc 100644 --- a/src/Login.ts +++ b/src/Login.ts @@ -102,7 +102,7 @@ export default class Login { const oidcFlow = await tryInitOidcNativeFlow( this.delegatedAuthentication, SdkConfig.get().brand, - SdkConfig.get().oidc_static_client_ids, + SdkConfig.get().oidc_static_clients, ); return [oidcFlow]; } catch (error) { @@ -211,9 +211,9 @@ export interface OidcNativeFlow extends ILoginFlow { const tryInitOidcNativeFlow = async ( delegatedAuthConfig: ValidatedDelegatedAuthConfig, brand: string, - oidcStaticClientIds?: IConfigOptions["oidc_static_client_ids"], + oidcStaticClients?: IConfigOptions["oidc_static_clients"], ): Promise => { - const clientId = await getOidcClientId(delegatedAuthConfig, brand, window.location.origin, oidcStaticClientIds); + const clientId = await getOidcClientId(delegatedAuthConfig, brand, window.location.origin, oidcStaticClients); const flow = { type: "oidcNativeFlow", diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index 4e2df7832c..309709e18f 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -17,6 +17,7 @@ limitations under the License. import { logger } from "matrix-js-sdk/src/logger"; import { registerOidcClient } from "matrix-js-sdk/src/oidc/register"; +import { IConfigOptions } from "../../IConfigOptions"; import { ValidatedDelegatedAuthConfig } from "../ValidatedServerConfig"; /** @@ -25,10 +26,13 @@ import { ValidatedDelegatedAuthConfig } from "../ValidatedServerConfig"; * @param staticOidcClients static client config from config.json * @returns clientId if found, otherwise undefined */ -const getStaticOidcClientId = (issuer: string, staticOidcClients?: Record): string | undefined => { +const getStaticOidcClientId = ( + issuer: string, + staticOidcClients?: IConfigOptions["oidc_static_clients"], +): string | undefined => { // static_oidc_clients are configured with a trailing slash const issuerWithTrailingSlash = issuer.endsWith("/") ? issuer : issuer + "/"; - return staticOidcClients?.[issuerWithTrailingSlash]; + return staticOidcClients?.[issuerWithTrailingSlash]?.client_id; }; /** @@ -46,7 +50,7 @@ export const getOidcClientId = async ( delegatedAuthConfig: ValidatedDelegatedAuthConfig, clientName: string, baseUrl: string, - staticOidcClients?: Record, + staticOidcClients?: IConfigOptions["oidc_static_clients"], ): Promise => { const staticClientId = getStaticOidcClientId(delegatedAuthConfig.issuer, staticOidcClients); if (staticClientId) { diff --git a/test/components/structures/auth/Login-test.tsx b/test/components/structures/auth/Login-test.tsx index 62e5408430..6973f49468 100644 --- a/test/components/structures/auth/Login-test.tsx +++ b/test/components/structures/auth/Login-test.tsx @@ -37,7 +37,9 @@ jest.mock("matrix-js-sdk/src/matrix"); jest.useRealTimers(); const oidcStaticClientsConfig = { - "https://staticallyregisteredissuer.org/": "static-clientId-123", + "https://staticallyregisteredissuer.org/": { + client_id: "static-clientId-123", + }, }; describe("Login", function () { @@ -52,7 +54,7 @@ describe("Login", function () { SdkConfig.put({ brand: "test-brand", disable_custom_urls: true, - oidc_static_client_ids: oidcStaticClientsConfig, + oidc_static_clients: oidcStaticClientsConfig, }); mockClient.login.mockClear().mockResolvedValue({ access_token: "TOKEN", diff --git a/test/utils/oidc/registerClient-test.ts b/test/utils/oidc/registerClient-test.ts index 1e08c0085e..539ec33212 100644 --- a/test/utils/oidc/registerClient-test.ts +++ b/test/utils/oidc/registerClient-test.ts @@ -27,7 +27,9 @@ describe("getOidcClientId()", () => { const baseUrl = "https://just.testing"; const dynamicClientId = "xyz789"; const staticOidcClients = { - [issuer]: "abc123", + [issuer]: { + client_id: "abc123", + }, }; const delegatedAuthConfig = { issuer, @@ -42,9 +44,7 @@ describe("getOidcClientId()", () => { }); it("should return static clientId when configured", async () => { - expect(await getOidcClientId(delegatedAuthConfig, clientName, baseUrl, staticOidcClients)).toEqual( - staticOidcClients[issuer], - ); + expect(await getOidcClientId(delegatedAuthConfig, clientName, baseUrl, staticOidcClients)).toEqual("abc123"); // didn't try to register expect(fetchMockJest).toHaveFetchedTimes(0); });