sorb/ThreadNet-Web
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 4 Wiki Activity

Switch OIDC to response_mode=fragment (#33100)

Browse Source 
* Refactor: kill off `parseQs` in favour of URLSearchParams

* Consolidate app-load url parameter handling

* Switch to responseMode=fragment
This commit is contained in:
Michael Telatynski
2026-04-15 09:35:02 +00:00
committed by GitHub
parent 5475edbbc5
commit de4a1e6d35
16 changed files with 309 additions and 206 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/web/test/unit-tests/Lifecycle-test.ts
+1 -1
View File
@@ -169,7 +169,7 @@ describe("Lifecycle", () => {
const prom = Lifecycle.loadSession({
enableGuest: true,
guestHsUrl: "https://guest.server",
fragmentQueryParams: { guest_user_id: "a", guest_access_token: "b" },
urlParams: { guest: { guest_user_id: "a", guest_access_token: "b" } },
abortSignal: abortController.signal,
});
abortController.abort();
apps/web/test/unit-tests/components/structures/MatrixChat-test.tsx
+47 -37
View File
@@ -223,7 +223,7 @@ describe("<MatrixChat />", () => {
},
onNewScreen: jest.fn(),
onTokenLoginCompleted: jest.fn(),
realQueryParams: {},
urlParams: {},
};
mockClient = getMockClientWithEventEmitter(getMockClientMethods());
@@ -320,9 +320,11 @@ describe("<MatrixChat />", () => {
const code = "test-oidc-auth-code";
const state = "test-oidc-state";
const realQueryParams = {
code,
state: state,
const urlParams = {
oidc: {
code,
state: state,
},
};
const deviceId = "test-device-id";
@@ -383,11 +385,13 @@ describe("<MatrixChat />", () => {
});
it("should fail when query params do not include valid code and state", async () => {
const queryParams = {
code: 123,
state: "abc",
const urlParams = {
oidc: {
code: "",
state: "abc",
},
};
getComponent({ realQueryParams: queryParams });
getComponent({ urlParams });
await flushPromises();
@@ -400,15 +404,15 @@ describe("<MatrixChat />", () => {
});
it("should make correct request to complete authorization", async () => {
getComponent({ realQueryParams });
getComponent({ urlParams });
await flushPromises();
expect(completeAuthorizationCodeGrant).toHaveBeenCalledWith(code, state);
expect(completeAuthorizationCodeGrant).toHaveBeenCalledWith(code, state, "fragment");
});
it("should look up userId using access token", async () => {
getComponent({ realQueryParams });
getComponent({ urlParams });
await flushPromises();
@@ -423,7 +427,7 @@ describe("<MatrixChat />", () => {
it("should log error and return to welcome page when userId lookup fails", async () => {
loginClient.whoami.mockRejectedValue(new Error("oups"));
getComponent({ realQueryParams });
getComponent({ urlParams });
await flushPromises();
@@ -436,7 +440,7 @@ describe("<MatrixChat />", () => {
it("should call onTokenLoginCompleted", async () => {
const onTokenLoginCompleted = jest.fn();
getComponent({ realQueryParams, onTokenLoginCompleted });
getComponent({ urlParams, onTokenLoginCompleted });
await waitFor(() => expect(onTokenLoginCompleted).toHaveBeenCalled());
});
@@ -450,7 +454,7 @@ describe("<MatrixChat />", () => {
mocked(completeAuthorizationCodeGrant).mockRejectedValue(
new Error(OidcError.MissingOrInvalidStoredState),
);
getComponent({ realQueryParams });
getComponent({ urlParams });
await flushPromises();
@@ -465,7 +469,7 @@ describe("<MatrixChat />", () => {
});
it("should log and return to welcome page", async () => {
getComponent({ realQueryParams });
getComponent({ urlParams });
await flushPromises();
@@ -479,7 +483,7 @@ describe("<MatrixChat />", () => {
});
it("should not clear storage", async () => {
getComponent({ realQueryParams });
getComponent({ urlParams });
await flushPromises();
@@ -488,7 +492,7 @@ describe("<MatrixChat />", () => {
it("should not store clientId or issuer", async () => {
const sessionStorageSetSpy = jest.spyOn(sessionStorage.__proto__, "setItem");
getComponent({ realQueryParams });
getComponent({ urlParams });
await flushPromises();
@@ -509,7 +513,7 @@ describe("<MatrixChat />", () => {
});
it("should persist login credentials", async () => {
getComponent({ realQueryParams });
getComponent({ urlParams });
await waitFor(() => expect(localStorage.getItem("mx_device_id")).toEqual(deviceId));
expect(localStorage.getItem("mx_hs_url")).toEqual(homeserverUrl);
@@ -518,14 +522,14 @@ describe("<MatrixChat />", () => {
});
it("should store clientId and issuer in session storage", async () => {
getComponent({ realQueryParams });
getComponent({ urlParams });
await waitFor(() => expect(localStorage.getItem("mx_oidc_client_id")).toEqual(clientId));
await waitFor(() => expect(localStorage.getItem("mx_oidc_token_issuer")).toEqual(issuer));
});
it("should set logged in and start MatrixClient", async () => {
getComponent({ realQueryParams });
getComponent({ urlParams });
defaultDispatcher.dispatch({
action: Action.WillStartClient,
@@ -545,7 +549,7 @@ describe("<MatrixChat />", () => {
jest.spyOn(Lifecycle, "attemptDelegatedAuthLogin");
getComponent({ realQueryParams });
getComponent({ urlParams });
await flushPromises();
expect(Lifecycle.attemptDelegatedAuthLogin).toHaveBeenCalled();
@@ -559,7 +563,7 @@ describe("<MatrixChat />", () => {
jest.spyOn(Lifecycle, "attemptDelegatedAuthLogin");
getComponent({ realQueryParams });
getComponent({ urlParams });
await flushPromises();
expect(Lifecycle.attemptDelegatedAuthLogin).toHaveBeenCalled();
@@ -1120,8 +1124,10 @@ describe("<MatrixChat />", () => {
describe("when query params have a loginToken", () => {
const loginToken = "test-login-token";
const realQueryParams = {
loginToken,
const urlParams = {
legacy_sso: {
loginToken,
},
};
let loginClient!: ReturnType<typeof getMockClientWithEventEmitter>;
@@ -1150,7 +1156,7 @@ describe("<MatrixChat />", () => {
mocked(loginClient.getCrypto()!.userHasCrossSigningKeys).mockResolvedValue(true);
// When we load the page
getComponent({ realQueryParams });
getComponent({ urlParams });
defaultDispatcher.dispatch({
action: Action.WillStartClient,
@@ -1400,8 +1406,10 @@ describe("<MatrixChat />", () => {
describe("when query params have a loginToken", () => {
const loginToken = "test-login-token";
const realQueryParams = {
loginToken,
const urlParams = {
legacy_sso: {
loginToken,
},
};
let loginClient!: ReturnType<typeof getMockClientWithEventEmitter>;
@@ -1426,7 +1434,7 @@ describe("<MatrixChat />", () => {
it("should show an error dialog when no homeserver is found in local storage", async () => {
localStorage.removeItem("mx_sso_hs_url");
const localStorageGetSpy = jest.spyOn(localStorage.__proto__, "getItem");
getComponent({ realQueryParams });
getComponent({ urlParams });
await flushPromises();
expect(localStorageGetSpy).toHaveBeenCalledWith("mx_sso_hs_url");
@@ -1444,7 +1452,7 @@ describe("<MatrixChat />", () => {
});
it("should attempt token login", async () => {
getComponent({ realQueryParams });
getComponent({ urlParams });
await flushPromises();
expect(loginClient.login).toHaveBeenCalledWith("m.login.token", {
@@ -1455,7 +1463,7 @@ describe("<MatrixChat />", () => {
it("should call onTokenLoginCompleted", async () => {
const onTokenLoginCompleted = jest.fn();
getComponent({ realQueryParams, onTokenLoginCompleted });
getComponent({ urlParams, onTokenLoginCompleted });
await waitFor(() => expect(onTokenLoginCompleted).toHaveBeenCalled());
});
@@ -1465,7 +1473,7 @@ describe("<MatrixChat />", () => {
loginClient.login.mockRejectedValue(new Error("oups"));
});
it("should show a dialog", async () => {
getComponent({ realQueryParams });
getComponent({ urlParams });
await flushPromises();
@@ -1480,7 +1488,7 @@ describe("<MatrixChat />", () => {
});
it("should not clear storage", async () => {
getComponent({ realQueryParams });
getComponent({ urlParams });
await flushPromises();
@@ -1502,7 +1510,7 @@ describe("<MatrixChat />", () => {
it("should clear storage", async () => {
const localStorageClearSpy = jest.spyOn(localStorage.__proto__, "clear");
getComponent({ realQueryParams });
getComponent({ urlParams });
// just check we called the clearStorage function
await waitFor(() => expect(loginClient.clearStores).toHaveBeenCalled());
@@ -1511,7 +1519,7 @@ describe("<MatrixChat />", () => {
});
it("should persist login credentials", async () => {
getComponent({ realQueryParams });
getComponent({ urlParams });
await waitFor(() => expect(localStorage.getItem("mx_hs_url")).toEqual(serverConfig.hsUrl));
expect(localStorage.getItem("mx_user_id")).toEqual(userId);
@@ -1521,7 +1529,7 @@ describe("<MatrixChat />", () => {
it("should set fresh login flag in session storage", async () => {
const sessionStorageSetSpy = jest.spyOn(sessionStorage.__proto__, "setItem");
getComponent({ realQueryParams });
getComponent({ urlParams });
await waitFor(() => expect(sessionStorageSetSpy).toHaveBeenCalledWith("mx_fresh_login", "true"));
});
@@ -1537,13 +1545,13 @@ describe("<MatrixChat />", () => {
},
};
loginClient.login.mockResolvedValue(loginResponseWithWellKnown);
getComponent({ realQueryParams });
getComponent({ urlParams });
await waitFor(() => expect(localStorage.getItem("mx_hs_url")).toEqual(hsUrlFromWk));
});
it("should continue to post login setup when no session is found in local storage", async () => {
getComponent({ realQueryParams });
getComponent({ urlParams });
defaultDispatcher.dispatch({
action: Action.WillStartClient,
});
@@ -1589,6 +1597,7 @@ describe("<MatrixChat />", () => {
getComponent({
initialScreenAfterLogin: {
screen: "start_sso",
params: {},
},
});
@@ -1604,6 +1613,7 @@ describe("<MatrixChat />", () => {
getComponent({
initialScreenAfterLogin: {
screen: "start_cas",
params: {},
},
});
apps/web/test/unit-tests/utils/oidc/authorize-test.ts
+5 -5
View File
@@ -75,7 +75,7 @@ describe("OIDC authorization", () => {
const authUrl = new URL(window.location.href);
expect(authUrl.searchParams.get("response_mode")).toEqual("query");
expect(authUrl.searchParams.get("response_mode")).toEqual("fragment");
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");
@@ -95,7 +95,7 @@ describe("OIDC authorization", () => {
describe("completeOidcLogin()", () => {
const state = "test-state-444";
const code = "test-code-777";
const queryDict = {
const params = {
code,
state: state,
};
@@ -137,13 +137,13 @@ describe("OIDC authorization", () => {
});
it("should make request complete authorization code grant", async () => {
await completeOidcLogin(queryDict);
await completeOidcLogin(params);
expect(completeAuthorizationCodeGrant).toHaveBeenCalledWith(code, state);
expect(completeAuthorizationCodeGrant).toHaveBeenCalledWith(code, state, "fragment");
});
it("should return accessToken, configured homeserver and identityServer", async () => {
const result = await completeOidcLogin(queryDict);
const result = await completeOidcLogin(params);
expect(result).toEqual({
accessToken: tokenResponse.access_token,
apps/web/test/unit-tests/vector/app-test.ts
+1 -1
View File
@@ -85,7 +85,7 @@ describe("sso_redirect_options", () => {
await loadApp({}, jest.fn());
expect(startOidcLoginSpy).toHaveBeenCalledWith(
"https://auth.org/auth?client_id=12345&redirect_uri=https%3A%2F%2Fapp.element.io%2F%3Fno_universal_links%3Dtrue&response_type=code&scope=openid+urn%3Amatrix%3Aorg.matrix.msc2967.client%3Aapi%3A*+urn%3Amatrix%3Aorg.matrix.msc2967.client%3Adevice%3AwKpa6hpi3Y&nonce=38QgU2Pomx&state=10000000100040008000100000000000&code_challenge=awE81eIsGff70JahvrTqWRbGKLI10ooyo_Xm1sxuZvU&code_challenge_method=S256&response_mode=query",
"https://auth.org/auth?client_id=12345&redirect_uri=https%3A%2F%2Fapp.element.io%2F%3Fno_universal_links%3Dtrue&response_type=code&scope=openid+urn%3Amatrix%3Aorg.matrix.msc2967.client%3Aapi%3A*+urn%3Amatrix%3Aorg.matrix.msc2967.client%3Adevice%3AwKpa6hpi3Y&nonce=38QgU2Pomx&state=10000000100040008000100000000000&code_challenge=awE81eIsGff70JahvrTqWRbGKLI10ooyo_Xm1sxuZvU&code_challenge_method=S256&response_mode=fragment",
);
});
});
apps/web/test/unit-tests/vector/init-test.ts
+5 -8
View File
@@ -1,6 +1,6 @@
/**
* @jest-environment jest-fixed-jsdom
* @jest-environment-options {"url": "https://app.element.io/?loginToken=123&state=abc&code=xyz&no_universal_links&something_else=value"}
* @jest-environment-options {"url": "https://app.element.io/?loginToken=123&no_universal_links&something_else=value#/home?state=abc&code=xyz"}
*/
/*
@@ -16,6 +16,7 @@ import { waitFor, screen } from "jest-matrix-react";
import { loadApp, showError, showIncompatibleBrowser } from "../../../src/vector/init.tsx";
import SdkConfig from "../../../src/SdkConfig.ts";
import MatrixChat from "../../../src/components/structures/MatrixChat.tsx";
import { parseAppUrl } from "../../../src/vector/url_utils.ts";
function setUpMatrixChatDiv() {
document.getElementById("matrixchat")?.remove();
@@ -57,17 +58,13 @@ describe("loadApp", () => {
await waitFor(() => expect(window.matrixChat).toBeInstanceOf(MatrixChat));
});
it("should pass onTokenLoginCompleted which strips searchParams to MatrixChat", async () => {
it("should pass onTokenLoginCompleted which strips searchParams & fragment to MatrixChat", async () => {
const spy = jest.spyOn(window.history, "replaceState");
await loadApp({});
await waitFor(() => expect(window.matrixChat).toBeInstanceOf(MatrixChat));
window.matrixChat!.props.onTokenLoginCompleted();
window.matrixChat!.props.onTokenLoginCompleted(parseAppUrl(window.location).params, "/home");
expect(spy).toHaveBeenCalledWith(
null,
"",
expect.stringContaining("https://app.element.io/?something_else=value"),
);
expect(spy).toHaveBeenCalledWith(null, "", "https://app.element.io/?something_else=value#/home");
});
});
apps/web/test/unit-tests/vector/url_utils-test.ts
+44 -27
View File
@@ -5,37 +5,54 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
Please see LICENSE files in the repository root for full details.
*/
import { parseQsFromFragment, parseQs } from "../../../src/vector/url_utils";
import { parseAppUrl, parseQsFromFragment, searchParamsToQueryDict } from "../../../src/vector/url_utils";
describe("url_utils.ts", function () {
// @ts-ignore
const location: Location = {
hash: "",
search: "",
};
// @ts-ignore
const location: Location = {
hash: "",
search: "",
};
it("parseQsFromFragment", function () {
location.hash = "/home?foo=bar";
describe("parseQsFromFragment", () => {
it("should parse correctly", () => {
location.hash = "#/home?foo=bar";
expect(parseQsFromFragment(location)).toEqual({
location: "home",
params: {
location: "/home",
params: new URLSearchParams({
foo: "bar",
},
});
});
it("parseQs", function () {
location.search = "?foo=bar";
expect(parseQs(location)).toEqual({
foo: "bar",
});
});
it("parseQs with arrays", function () {
location.search = "?via=s1&via=s2&via=s2&foo=bar";
expect(parseQs(location)).toEqual({
via: ["s1", "s2", "s2"],
foo: "bar",
}),
});
});
});
describe("searchParamsToQueryDict", () => {
it("should handle arrays correctly", () => {
const u = new URLSearchParams("a=b&b=c&c=d&a=e&a=f");
expect(searchParamsToQueryDict(u)).toEqual({
a: ["b", "e", "f"],
b: "c",
c: "d",
});
});
});
describe("parseUrlParameters", () => {
it("should parse legacy sso parameters from query", () => {
const u = new URL("https://app.element.io?loginToken=foobar");
const parsed = parseAppUrl(u);
expect(parsed.params.legacy_sso?.loginToken).toEqual("foobar");
});
it("should parse oidc parameters from oauth-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");
});
it("should parse guest parameters", () => {
const u = new URL("https://app.element.io?foo=bar#/room/!roomId:server?guest_access_token=foobar");
const parsed = parseAppUrl(u);
expect(parsed.params.guest?.guest_access_token).toEqual("foobar");
});
});