From 64d3802efee3778c88ee8b1c0f1f0d59b2b590fc Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 16 Apr 2026 12:07:39 +0100 Subject: [PATCH] Fall back to OIDC response_mode query if fragment unsupported (#33169) * Fall back to OIDC response_mode query if fragment unsupported * Tidy comments * Fix test --- apps/web/src/Lifecycle.ts | 19 +++++++++----- .../src/components/structures/MatrixChat.tsx | 8 ++++-- apps/web/src/utils/oidc/authorize.ts | 26 ++++++++++++------- apps/web/src/utils/oidc/error.ts | 2 ++ apps/web/src/vector/app.tsx | 4 +-- apps/web/src/vector/url_utils.ts | 7 ++++- .../components/structures/MatrixChat-test.tsx | 4 +-- .../unit-tests/utils/oidc/authorize-test.ts | 20 +++++++++++--- apps/web/test/unit-tests/vector/app-test.ts | 2 +- .../test/unit-tests/vector/url_utils-test.ts | 13 +++++++--- 10 files changed, 75 insertions(+), 30 deletions(-) diff --git a/apps/web/src/Lifecycle.ts b/apps/web/src/Lifecycle.ts index d7d139249a..32d113adc8 100644 --- a/apps/web/src/Lifecycle.ts +++ b/apps/web/src/Lifecycle.ts @@ -277,9 +277,10 @@ export async function attemptDelegatedAuthLogin( defaultDeviceDisplayName?: string, fragmentAfterLogin?: string, ): Promise { - if (urlParams.oidc) { - console.log("We have OIDC params - attempting OIDC login"); - return attemptOidcNativeLogin(urlParams["oidc"]); + if (urlParams.oidc_fragment) { + return attemptOidcNativeLogin(urlParams.oidc_fragment, "fragment"); + } else if (urlParams.oidc_query) { + return attemptOidcNativeLogin(urlParams.oidc_query, "query"); } return attemptTokenLogin(urlParams["legacy_sso"], defaultDeviceDisplayName, fragmentAfterLogin); @@ -288,12 +289,18 @@ export async function attemptDelegatedAuthLogin( /** * Attempt to login by completing OIDC authorization code flow * @param urlParams subset of app-load url parameters relating to oidc auth + * @param responseMode - the response_mode used in the auth request * @returns Promise that resolves to true when login succeeded, else false */ -async function attemptOidcNativeLogin(urlParams: NonNullable): Promise { +async function attemptOidcNativeLogin( + urlParams: NonNullable, + responseMode: "fragment" | "query", +): Promise { + console.log("We have OIDC params - attempting OIDC login"); + try { const { accessToken, refreshToken, homeserverUrl, identityServerUrl, idToken, clientId, issuer } = - await completeOidcLogin(urlParams); + await completeOidcLogin(urlParams, responseMode); const { user_id: userId, @@ -1036,7 +1043,7 @@ export function isLoggingOut(): boolean { * By the time this method is called, we have successfully logged in if necessary, and the client has been set up with * the access token. * - * Emits {@link Acction.WillStartClient} before starting the client, and {@link Action.ClientStarted} when the client has + * Emits {@link Action.WillStartClient} before starting the client, and {@link Action.ClientStarted} when the client has * been started. * * @param client the matrix client to start diff --git a/apps/web/src/components/structures/MatrixChat.tsx b/apps/web/src/components/structures/MatrixChat.tsx index e93799fb1a..14e726e532 100644 --- a/apps/web/src/components/structures/MatrixChat.tsx +++ b/apps/web/src/components/structures/MatrixChat.tsx @@ -350,7 +350,11 @@ export default class MatrixChat extends React.PureComponent { ); // remove the loginToken or auth code from the URL regardless - if (!!this.props.urlParams.legacy_sso || !!this.props.urlParams.oidc) { + if ( + !!this.props.urlParams.legacy_sso || + !!this.props.urlParams.oidc_fragment || + !!this.props.urlParams.oidc_query + ) { this.props.onTokenLoginCompleted(this.props.urlParams, this.getFragmentAfterLogin()); } @@ -408,7 +412,7 @@ export default class MatrixChat extends React.PureComponent { * {@link onWillStartClient} and {@link onClientStarted} will already have been called (but not necessarily * completed). * - * This method either calls {@link onLiggedIn} directly, or switches to {@link Views.E2E_SETUP} or + * This method either calls {@link onLoggedIn} directly, or switches to {@link Views.E2E_SETUP} or * {@link Views.COMPLETE_SECURITY}, which will later call {@link onCompleteSecurityE2eSetupFinished}. */ private async postLoginSetup(): Promise { diff --git a/apps/web/src/utils/oidc/authorize.ts b/apps/web/src/utils/oidc/authorize.ts index b0020446ac..2e1420e32e 100644 --- a/apps/web/src/utils/oidc/authorize.ts +++ b/apps/web/src/utils/oidc/authorize.ts @@ -23,6 +23,7 @@ import { type URLParams } from "../../vector/url_utils.ts"; * @param clientId this client's id as registered with configured issuer * @param homeserverUrl target homeserver * @param identityServerUrl OPTIONAL target identity server + * @param isRegistration if true will set the prompt to "create" * @returns Promise that resolves after we have navigated to auth endpoint */ export const startOidcLogin = async ( @@ -47,7 +48,7 @@ export const startOidcLogin = async ( nonce, prompt, urlState: PlatformPeg.get()?.getOidcClientState(), - responseMode: "fragment", + responseMode: delegatedAuthConfig.response_modes_supported?.includes("fragment") ? "fragment" : "query", }); window.location.href = authorizationUrl; @@ -57,15 +58,20 @@ export const startOidcLogin = async ( * Gets `code` and `state` response params * * @param urlParams - the parameters to read + * @param responseMode - the response_mode used in the auth request * @returns code and state * @throws when code and state are not valid strings */ -const getCodeAndStateFromParams = ({ - code, - state, -}: NonNullable): { code: string; state: string } => { +const getCodeAndStateFromParams = ( + { code, state }: NonNullable, + responseMode: "fragment" | "query", +): { code: string; state: string } => { if (!code || typeof code !== "string" || !state || typeof state !== "string") { - throw new Error(OidcClientError.InvalidQueryParameters); + if (responseMode === "fragment") { + throw new Error(OidcClientError.InvalidFragmentParameters); + } else { + throw new Error(OidcClientError.InvalidQueryParameters); + } } return { code, state }; }; @@ -91,15 +97,17 @@ type CompleteOidcLoginResponse = { /** * Attempt to complete authorization code flow to get an access token * @param urlParams the parameters extracted from the app-load URI. + * @param responseMode - the response_mode used in the auth request * @returns Promise that resolves with a CompleteOidcLoginResponse when login was successful * @throws When we failed to get a valid access token */ export const completeOidcLogin = async ( - urlParams: NonNullable, + urlParams: NonNullable, + responseMode: "fragment" | "query", ): Promise => { - const { code, state } = getCodeAndStateFromParams(urlParams); + const { code, state } = getCodeAndStateFromParams(urlParams, responseMode); const { homeserverUrl, tokenResponse, idTokenClaims, identityServerUrl, oidcClientSettings } = - await completeAuthorizationCodeGrant(code, state, "fragment"); + await completeAuthorizationCodeGrant(code, state, responseMode); return { homeserverUrl, diff --git a/apps/web/src/utils/oidc/error.ts b/apps/web/src/utils/oidc/error.ts index f9334a739c..3cc5c14ec5 100644 --- a/apps/web/src/utils/oidc/error.ts +++ b/apps/web/src/utils/oidc/error.ts @@ -17,6 +17,7 @@ import { _t } from "../../languageHandler"; */ export enum OidcClientError { InvalidQueryParameters = "Invalid query parameters for OIDC native login. `code` and `state` are required.", + InvalidFragmentParameters = "Invalid fragment parameters for OIDC native login. `code` and `state` are required.", } /** @@ -30,6 +31,7 @@ export const getOidcErrorMessage = (error: Error): string | ReactNode => { case OidcError.MissingOrInvalidStoredState: return _t("auth|oidc|missing_or_invalid_stored_state"); case OidcClientError.InvalidQueryParameters: + case OidcClientError.InvalidFragmentParameters: case OidcError.CodeExchangeFailed: case OidcError.InvalidBearerTokenResponse: case OidcError.InvalidIdToken: diff --git a/apps/web/src/vector/app.tsx b/apps/web/src/vector/app.tsx index fa31bdd732..db5d0e4e99 100644 --- a/apps/web/src/vector/app.tsx +++ b/apps/web/src/vector/app.tsx @@ -44,7 +44,7 @@ function onTokenLoginCompleted(urlParams: URLParams, fragmentAfterLogin: string) const url = new URL(window.location.href); // if we did a token login, we're now left with the login token as query param in the url; clear it out - for (const param in { ...urlParams.legacy_sso }) { + for (const param in { ...urlParams.legacy_sso, ...urlParams.oidc_query }) { url.searchParams.delete(param); } @@ -112,7 +112,7 @@ export async function loadApp(urlParams: URLParams, matrixChatRef: React.Ref", () => { const code = "test-oidc-auth-code"; const state = "test-oidc-state"; const urlParams = { - oidc: { + oidc_fragment: { code, state: state, }, @@ -386,7 +386,7 @@ describe("", () => { it("should fail when query params do not include valid code and state", async () => { const urlParams = { - oidc: { + oidc_query: { code: "", state: "abc", }, diff --git a/apps/web/test/unit-tests/utils/oidc/authorize-test.ts b/apps/web/test/unit-tests/utils/oidc/authorize-test.ts index c28e6325d7..b7bd2bfd9f 100644 --- a/apps/web/test/unit-tests/utils/oidc/authorize-test.ts +++ b/apps/web/test/unit-tests/utils/oidc/authorize-test.ts @@ -75,7 +75,7 @@ describe("OIDC authorization", () => { const authUrl = new URL(window.location.href); - expect(authUrl.searchParams.get("response_mode")).toEqual("fragment"); + expect(authUrl.searchParams.get("response_mode")).toEqual("query"); expect(authUrl.searchParams.get("response_type")).toEqual("code"); expect(authUrl.searchParams.get("client_id")).toEqual(clientId); expect(authUrl.searchParams.get("code_challenge_method")).toEqual("S256"); @@ -90,6 +90,18 @@ describe("OIDC authorization", () => { expect(authUrl.searchParams.has("nonce")).toBeTruthy(); expect(authUrl.searchParams.has("code_challenge")).toBeTruthy(); }); + + it("should prefer response_mode fragment if supported", async () => { + await startOidcLogin( + { ...delegatedAuthConfig, response_modes_supported: ["query", "fragment"] }, + clientId, + homeserverUrl, + ); + + const authUrl = new URL(window.location.href); + + expect(authUrl.searchParams.get("response_mode")).toEqual("fragment"); + }); }); describe("completeOidcLogin()", () => { @@ -131,19 +143,19 @@ describe("OIDC authorization", () => { }); it("should throw when query params do not include state and code", async () => { - await expect(async () => await completeOidcLogin({})).rejects.toThrow( + await expect(async () => await completeOidcLogin({}, "query")).rejects.toThrow( OidcClientError.InvalidQueryParameters, ); }); it("should make request complete authorization code grant", async () => { - await completeOidcLogin(params); + await completeOidcLogin(params, "fragment"); expect(completeAuthorizationCodeGrant).toHaveBeenCalledWith(code, state, "fragment"); }); it("should return accessToken, configured homeserver and identityServer", async () => { - const result = await completeOidcLogin(params); + const result = await completeOidcLogin(params, "query"); expect(result).toEqual({ accessToken: tokenResponse.access_token, diff --git a/apps/web/test/unit-tests/vector/app-test.ts b/apps/web/test/unit-tests/vector/app-test.ts index 381f4a2c53..fa95a0053b 100644 --- a/apps/web/test/unit-tests/vector/app-test.ts +++ b/apps/web/test/unit-tests/vector/app-test.ts @@ -76,7 +76,7 @@ describe("sso_redirect_options", () => { }); it("should redirect for native OIDC", async () => { - const authConfig = makeDelegatedAuthConfig(issuer); + const authConfig = { ...makeDelegatedAuthConfig(issuer), response_modes_supported: ["query", "fragment"] }; fetchMock.get("https://synapse/_matrix/client/v1/auth_metadata", authConfig); fetchMock.get(`${authConfig.issuer}.well-known/openid-configuration`, authConfig); fetchMock.get(authConfig.jwks_uri!, { keys: [] }); diff --git a/apps/web/test/unit-tests/vector/url_utils-test.ts b/apps/web/test/unit-tests/vector/url_utils-test.ts index f6f42f573c..ad2c0e0390 100644 --- a/apps/web/test/unit-tests/vector/url_utils-test.ts +++ b/apps/web/test/unit-tests/vector/url_utils-test.ts @@ -43,11 +43,18 @@ describe("parseUrlParameters", () => { expect(parsed.params.legacy_sso?.loginToken).toEqual("foobar"); }); - it("should parse oidc parameters from oauth-fragment", () => { + it("should parse oidc parameters from fragment", () => { const u = new URL("https://app.element.io/#code=foobar&state=barfoo"); const parsed = parseAppUrl(u); - expect(parsed.params.oidc?.code).toEqual("foobar"); - expect(parsed.params.oidc?.state).toEqual("barfoo"); + expect(parsed.params.oidc_fragment?.code).toEqual("foobar"); + expect(parsed.params.oidc_fragment?.state).toEqual("barfoo"); + }); + + it("should parse oidc parameters from query", () => { + const u = new URL("https://app.element.io/?code=foobar&state=barfoo"); + const parsed = parseAppUrl(u); + expect(parsed.params.oidc_query?.code).toEqual("foobar"); + expect(parsed.params.oidc_query?.state).toEqual("barfoo"); }); it("should parse guest parameters", () => {