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/4] 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/4] 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 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 3/4] 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 e636d93f8d657211171b4c080a9e763621ab735b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 13 May 2024 13:06:30 +0100 Subject: [PATCH 4/4] Update src/BasePlatform.ts Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/BasePlatform.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/BasePlatform.ts b/src/BasePlatform.ts index b29c9dc9b08..5950233641e 100644 --- a/src/BasePlatform.ts +++ b/src/BasePlatform.ts @@ -463,6 +463,8 @@ export default abstract class BasePlatform { */ public getOidcCallbackUrl(): URL { const url = new URL(window.location.href); + // The redirect URL has to exactly match that registered at the OIDC server, so + // ensure that the fragment part of the URL is empty. url.hash = ""; return url; }