From ad4e77d8e1ec94dc5870f687bd3d8ba6c6fad9b3 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 3 May 2024 12:47:37 +0100 Subject: [PATCH 1/6] Fix `element-desktop-ssoid being` included in OIDC Authorization call Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/BasePlatform.ts | 5 +++-- src/utils/oidc/authorize.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/BasePlatform.ts b/src/BasePlatform.ts index 7150336e450..19f9ab7b839 100644 --- a/src/BasePlatform.ts +++ b/src/BasePlatform.ts @@ -315,9 +315,10 @@ export default abstract class BasePlatform { /** * The URL to return to after a successful SSO/OIDC authentication + * @param forOidc whether the callback URL is for OIDC or legacy SSO * @param fragmentAfterLogin optional fragment for specific view to return to */ - public getSSOCallbackUrl(fragmentAfterLogin = ""): URL { + public getSSOCallbackUrl(forOidc = false, fragmentAfterLogin = ""): URL { const url = new URL(window.location.href); url.hash = fragmentAfterLogin; return url; @@ -346,7 +347,7 @@ export default abstract class BasePlatform { if (idpId) { localStorage.setItem(SSO_IDP_ID_KEY, idpId); } - const callbackUrl = this.getSSOCallbackUrl(fragmentAfterLogin); + const callbackUrl = this.getSSOCallbackUrl(false, fragmentAfterLogin); window.location.href = mxClient.getSsoLoginUrl(callbackUrl.toString(), loginType, idpId, action); // redirect to SSO } diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 8bbdd9894ae..5af9d99e85f 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -40,7 +40,7 @@ export const startOidcLogin = async ( identityServerUrl?: string, isRegistration?: boolean, ): Promise => { - const redirectUri = PlatformPeg.get()!.getSSOCallbackUrl().href; + const redirectUri = PlatformPeg.get()!.getSSOCallbackUrl(true).href; const nonce = randomString(10); From 49bb10df5d0eb0c0ee8145bffde90aca57e2cf7b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 10 May 2024 11:58:54 +0100 Subject: [PATCH 2/6] Split out oidc callback url into its own method Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/BasePlatform.ts | 16 +++++++++++----- src/Lifecycle.ts | 2 +- src/stores/oidc/OidcClientStore.ts | 2 +- src/utils/oidc/authorize.ts | 2 +- test/utils/oidc/registerClient-test.ts | 2 +- 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/BasePlatform.ts b/src/BasePlatform.ts index 19f9ab7b839..3673a2898f2 100644 --- a/src/BasePlatform.ts +++ b/src/BasePlatform.ts @@ -314,11 +314,10 @@ export default abstract class BasePlatform { } /** - * The URL to return to after a successful SSO/OIDC authentication - * @param forOidc whether the callback URL is for OIDC or legacy SSO + * The URL to return to after a successful SSO authentication * @param fragmentAfterLogin optional fragment for specific view to return to */ - public getSSOCallbackUrl(forOidc = false, fragmentAfterLogin = ""): URL { + public getSSOCallbackUrl(fragmentAfterLogin = ""): URL { const url = new URL(window.location.href); url.hash = fragmentAfterLogin; return url; @@ -347,7 +346,7 @@ export default abstract class BasePlatform { if (idpId) { localStorage.setItem(SSO_IDP_ID_KEY, idpId); } - const callbackUrl = this.getSSOCallbackUrl(false, fragmentAfterLogin); + const callbackUrl = this.getSSOCallbackUrl(fragmentAfterLogin); window.location.href = mxClient.getSsoLoginUrl(callbackUrl.toString(), loginType, idpId, action); // redirect to SSO } @@ -472,7 +471,7 @@ export default abstract class BasePlatform { return { clientName: config.brand, clientUri: this.baseUrl, - redirectUris: [this.getSSOCallbackUrl().href], + redirectUris: [this.getOidcCallbackUrl().href], logoUri: new URL("vector-icons/1024.png", this.baseUrl).href, applicationType: "web", // XXX: We break the spec by not consistently supplying these required fields @@ -491,4 +490,11 @@ export default abstract class BasePlatform { public getOidcClientState(): string { return ""; } + + /** + * The URL to return to after a successful OIDC authentication + */ + public getOidcCallbackUrl(): URL { + return new URL(window.location.href); + } } diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 61097c13c21..0c41929a681 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -719,7 +719,7 @@ async function createOidcTokenRefresher(credentials: IMatrixClientCreds): Promis try { const clientId = getStoredOidcClientId(); const idTokenClaims = getStoredOidcIdTokenClaims(); - const redirectUri = PlatformPeg.get()!.getSSOCallbackUrl().href; + const redirectUri = PlatformPeg.get()!.getOidcCallbackUrl().href; const deviceId = credentials.deviceId; if (!deviceId) { throw new Error("Expected deviceId in user credentials."); diff --git a/src/stores/oidc/OidcClientStore.ts b/src/stores/oidc/OidcClientStore.ts index 57fe1adcd1f..dd62bff91ae 100644 --- a/src/stores/oidc/OidcClientStore.ts +++ b/src/stores/oidc/OidcClientStore.ts @@ -156,7 +156,7 @@ export class OidcClientStore { ...metadata, authority: metadata.issuer, signingKeys, - redirect_uri: PlatformPeg.get()!.getSSOCallbackUrl().href, + redirect_uri: PlatformPeg.get()!.getOidcCallbackUrl().href, client_id: clientId, }); } catch (error) { diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 5af9d99e85f..e24ed7fd066 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -40,7 +40,7 @@ export const startOidcLogin = async ( identityServerUrl?: string, isRegistration?: boolean, ): Promise => { - const redirectUri = PlatformPeg.get()!.getSSOCallbackUrl(true).href; + const redirectUri = PlatformPeg.get()!.getOidcCallbackUrl().href; const nonce = randomString(10); diff --git a/test/utils/oidc/registerClient-test.ts b/test/utils/oidc/registerClient-test.ts index bf8d1793295..9d8ba0ac160 100644 --- a/test/utils/oidc/registerClient-test.ts +++ b/test/utils/oidc/registerClient-test.ts @@ -44,7 +44,7 @@ describe("getOidcClientId()", () => { return baseUrl; }, }); - Object.defineProperty(PlatformPeg.get(), "getSSOCallbackUrl", { + Object.defineProperty(PlatformPeg.get(), "getOidcCallbackUrl", { value: () => ({ href: baseUrl, }), From e2efdb19f6e879e5a27d1f99d6e3cbc8bb20c873 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 10 May 2024 13:13:56 +0100 Subject: [PATCH 3/6] Allow explicit configuration of OIDC dynamic registration metadata Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/BasePlatform.ts | 18 +++++++++++++----- src/IConfigOptions.ts | 20 ++++++++++++++------ 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/BasePlatform.ts b/src/BasePlatform.ts index 1a9d1deecb9..c894e5f974b 100644 --- a/src/BasePlatform.ts +++ b/src/BasePlatform.ts @@ -430,6 +430,13 @@ export default abstract class BasePlatform { return window.location.origin + window.location.pathname; } + /** + * Fallback Client URI to use for OIDC client registration for if one is not specified in config.json + */ + public get defaultOidcClientUri(): string { + return window.location.origin; + } + /** * Metadata to use for dynamic OIDC client registrations */ @@ -437,16 +444,17 @@ export default abstract class BasePlatform { const config = SdkConfig.get(); return { clientName: config.brand, - clientUri: this.baseUrl, + clientUri: config.oidc_metadata?.client_uri ?? this.defaultOidcClientUri, redirectUris: [this.getOidcCallbackUrl().href], - logoUri: new URL("vector-icons/1024.png", this.baseUrl).href, + logoUri: config.oidc_metadata?.logo_uri ?? new URL("vector-icons/1024.png", this.baseUrl).href, applicationType: "web", // XXX: We break the spec by not consistently supplying these required fields - // contacts: [], // @ts-ignore - tosUri: config.terms_and_conditions_links?.[0]?.url, + contacts: config.oidc_metadata?.contacts ?? [], + // @ts-ignore + tosUri: config.oidc_metadata?.tos_uri ?? config.terms_and_conditions_links?.[0]?.url, // @ts-ignore - policyUri: config.privacy_policy_url, + policyUri: config.oidc_metadata?.policy_uri ?? config.privacy_policy_url, }; } diff --git a/src/IConfigOptions.ts b/src/IConfigOptions.ts index 4dc537aab07..de36bd4370c 100644 --- a/src/IConfigOptions.ts +++ b/src/IConfigOptions.ts @@ -200,12 +200,20 @@ export interface IConfigOptions { * The issuer URL must have a trailing `/`. * OPTIONAL */ - oidc_static_clients?: Record< - string, - { - client_id: string; - } - >; + oidc_static_clients?: { + [issuer: string]: { client_id: string }; + }; + + /** + * Configuration for OIDC dynamic registration where a static OIDC client is not configured. + */ + oidc_metadata?: { + client_uri?: string; + logo_uri?: string; + tos_uri?: string; + policy_uri?: string; + contacts?: string[]; + }; } export interface ISsoRedirectOptions { From a4b48c2414d1f99aa027af0543c2f3e4266874af Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 10 May 2024 13:29:58 +0100 Subject: [PATCH 4/6] Fix test Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- test/utils/oidc/registerClient-test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/utils/oidc/registerClient-test.ts b/test/utils/oidc/registerClient-test.ts index 9d8ba0ac160..6feb4c7e91a 100644 --- a/test/utils/oidc/registerClient-test.ts +++ b/test/utils/oidc/registerClient-test.ts @@ -44,6 +44,11 @@ describe("getOidcClientId()", () => { return baseUrl; }, }); + Object.defineProperty(PlatformPeg.get(), "defaultOidcClientUri", { + get(): string { + return baseUrl; + }, + }); Object.defineProperty(PlatformPeg.get(), "getOidcCallbackUrl", { value: () => ({ href: baseUrl, From ab4b899bda89a156dae451c60172f1441e391578 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 10 May 2024 14:35:15 +0100 Subject: [PATCH 5/6] Fix unexpected hash on oidc callback url Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/BasePlatform.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/BasePlatform.ts b/src/BasePlatform.ts index 1a9d1deecb9..b29c9dc9b08 100644 --- a/src/BasePlatform.ts +++ b/src/BasePlatform.ts @@ -462,6 +462,8 @@ export default abstract class BasePlatform { * The URL to return to after a successful OIDC authentication */ public getOidcCallbackUrl(): URL { - return new URL(window.location.href); + const url = new URL(window.location.href); + url.hash = ""; + return url; } } From be6509f4a0e77bd6f588b6a24d44ad57617132b2 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 13 May 2024 22:52:39 +0100 Subject: [PATCH 6/6] undefined > [] Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/BasePlatform.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BasePlatform.ts b/src/BasePlatform.ts index 284b701224f..e7e4ff7e3cd 100644 --- a/src/BasePlatform.ts +++ b/src/BasePlatform.ts @@ -450,7 +450,7 @@ export default abstract class BasePlatform { applicationType: "web", // XXX: We break the spec by not consistently supplying these required fields // @ts-ignore - contacts: config.oidc_metadata?.contacts ?? [], + contacts: config.oidc_metadata?.contacts, // @ts-ignore tosUri: config.oidc_metadata?.tos_uri ?? config.terms_and_conditions_links?.[0]?.url, // @ts-ignore