From 4e68b915159a53d9049c0aa29edf97565dd75184 Mon Sep 17 00:00:00 2001
From: Michael Telatynski <7t3chguy@gmail.com>
Date: Tue, 23 Jan 2024 13:34:10 +0000
Subject: [PATCH] Fix OIDC bugs due to amnesiac stores forgetting OIDC issuer &
other data (#12166)
* Fix OIDC bugs due to amnesiac stores forgetting OIDC issuer & other data
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
* Fix tests
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
---------
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
---
src/utils/oidc/persistOidcSettings.ts | 22 +++++++-------
.../components/structures/MatrixChat-test.tsx | 4 +--
test/stores/oidc/OidcClientStore-test.ts | 30 +++++--------------
test/utils/oidc/persistOidcSettings-test.ts | 28 ++++++++---------
4 files changed, 33 insertions(+), 51 deletions(-)
diff --git a/src/utils/oidc/persistOidcSettings.ts b/src/utils/oidc/persistOidcSettings.ts
index da4510bbac..8a557952f1 100644
--- a/src/utils/oidc/persistOidcSettings.ts
+++ b/src/utils/oidc/persistOidcSettings.ts
@@ -21,7 +21,7 @@ const tokenIssuerStorageKey = "mx_oidc_token_issuer";
const idTokenClaimsStorageKey = "mx_oidc_id_token_claims";
/**
- * Persists oidc clientId and issuer in session storage
+ * Persists oidc clientId and issuer in local storage
* Only set after successful authentication
* @param clientId
* @param issuer
@@ -31,27 +31,27 @@ export const persistOidcAuthenticatedSettings = (
issuer: string,
idTokenClaims: IdTokenClaims,
): void => {
- sessionStorage.setItem(clientIdStorageKey, clientId);
- sessionStorage.setItem(tokenIssuerStorageKey, issuer);
- sessionStorage.setItem(idTokenClaimsStorageKey, JSON.stringify(idTokenClaims));
+ localStorage.setItem(clientIdStorageKey, clientId);
+ localStorage.setItem(tokenIssuerStorageKey, issuer);
+ localStorage.setItem(idTokenClaimsStorageKey, JSON.stringify(idTokenClaims));
};
/**
- * Retrieve stored oidc issuer from session storage
+ * Retrieve stored oidc issuer from local storage
* When user has token from OIDC issuer, this will be set
* @returns issuer or undefined
*/
export const getStoredOidcTokenIssuer = (): string | undefined => {
- return sessionStorage.getItem(tokenIssuerStorageKey) ?? undefined;
+ return localStorage.getItem(tokenIssuerStorageKey) ?? undefined;
};
/**
- * Retrieves stored oidc client id from session storage
+ * Retrieves stored oidc client id from local storage
* @returns clientId
- * @throws when clientId is not found in session storage
+ * @throws when clientId is not found in local storage
*/
export const getStoredOidcClientId = (): string => {
- const clientId = sessionStorage.getItem(clientIdStorageKey);
+ const clientId = localStorage.getItem(clientIdStorageKey);
if (!clientId) {
throw new Error("Oidc client id not found in storage");
}
@@ -59,11 +59,11 @@ export const getStoredOidcClientId = (): string => {
};
/**
- * Retrieve stored id token claims from session storage
+ * Retrieve stored id token claims from local storage
* @returns idtokenclaims or undefined
*/
export const getStoredOidcIdTokenClaims = (): IdTokenClaims | undefined => {
- const idTokenClaims = sessionStorage.getItem(idTokenClaimsStorageKey);
+ const idTokenClaims = localStorage.getItem(idTokenClaimsStorageKey);
if (!idTokenClaims) {
return;
}
diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx
index 9004a461fa..2d6cfa6bf0 100644
--- a/test/components/structures/MatrixChat-test.tsx
+++ b/test/components/structures/MatrixChat-test.tsx
@@ -449,8 +449,8 @@ describe("", () => {
await flushPromises();
- expect(sessionStorage.getItem("mx_oidc_client_id")).toEqual(clientId);
- expect(sessionStorage.getItem("mx_oidc_token_issuer")).toEqual(issuer);
+ expect(localStorage.getItem("mx_oidc_client_id")).toEqual(clientId);
+ expect(localStorage.getItem("mx_oidc_token_issuer")).toEqual(issuer);
});
it("should set logged in and start MatrixClient", async () => {
diff --git a/test/stores/oidc/OidcClientStore-test.ts b/test/stores/oidc/OidcClientStore-test.ts
index d25b0fd541..baa8e2af1a 100644
--- a/test/stores/oidc/OidcClientStore-test.ts
+++ b/test/stores/oidc/OidcClientStore-test.ts
@@ -34,19 +34,16 @@ describe("OidcClientStore", () => {
const clientId = "test-client-id";
const metadata = mockOpenIdConfiguration();
const account = metadata.issuer + "account";
- const mockSessionStorage: Record = {
- mx_oidc_client_id: clientId,
- mx_oidc_token_issuer: metadata.issuer,
- };
const mockClient = getMockClientWithEventEmitter({
waitForClientWellKnown: jest.fn().mockResolvedValue({}),
});
beforeEach(() => {
- jest.spyOn(sessionStorage.__proto__, "getItem")
- .mockClear()
- .mockImplementation((key) => mockSessionStorage[key as string] ?? null);
+ localStorage.clear();
+ localStorage.setItem("mx_oidc_client_id", clientId);
+ localStorage.setItem("mx_oidc_token_issuer", metadata.issuer);
+
mocked(discoverAndValidateAuthenticationConfig).mockClear().mockResolvedValue({
metadata,
account,
@@ -71,7 +68,7 @@ describe("OidcClientStore", () => {
});
it("should return false when no issuer is in session storage", () => {
- jest.spyOn(sessionStorage.__proto__, "getItem").mockReturnValue(null);
+ localStorage.clear();
const store = new OidcClientStore(mockClient);
expect(store.isUserAuthenticatedWithOidc).toEqual(false);
@@ -98,14 +95,7 @@ describe("OidcClientStore", () => {
});
it("should log and return when no clientId is found in storage", async () => {
- const sessionStorageWithoutClientId: Record = {
- ...mockSessionStorage,
- mx_oidc_client_id: null,
- };
- jest.spyOn(sessionStorage.__proto__, "getItem").mockImplementation(
- (key) => sessionStorageWithoutClientId[key as string] ?? null,
- );
-
+ localStorage.removeItem("mx_oidc_client_id");
const store = new OidcClientStore(mockClient);
// no oidc client
@@ -209,13 +199,7 @@ describe("OidcClientStore", () => {
it("should throw when oidcClient could not be initialised", async () => {
// make oidcClient initialisation fail
mockClient.waitForClientWellKnown.mockResolvedValue(undefined as any);
- const sessionStorageWithoutIssuer: Record = {
- ...mockSessionStorage,
- mx_oidc_token_issuer: null,
- };
- jest.spyOn(sessionStorage.__proto__, "getItem").mockImplementation(
- (key) => sessionStorageWithoutIssuer[key as string] ?? null,
- );
+ localStorage.removeItem("mx_oidc_token_issuer");
const store = new OidcClientStore(mockClient);
diff --git a/test/utils/oidc/persistOidcSettings-test.ts b/test/utils/oidc/persistOidcSettings-test.ts
index 03ac61d199..3585c1576e 100644
--- a/test/utils/oidc/persistOidcSettings-test.ts
+++ b/test/utils/oidc/persistOidcSettings-test.ts
@@ -24,10 +24,11 @@ import {
} from "../../../src/utils/oidc/persistOidcSettings";
describe("persist OIDC settings", () => {
- beforeEach(() => {
- jest.spyOn(sessionStorage.__proto__, "getItem").mockClear().mockReturnValue(null);
+ jest.spyOn(Storage.prototype, "getItem");
+ jest.spyOn(Storage.prototype, "setItem");
- jest.spyOn(sessionStorage.__proto__, "setItem").mockClear();
+ beforeEach(() => {
+ localStorage.clear();
});
const clientId = "test-client-id";
@@ -45,20 +46,17 @@ describe("persist OIDC settings", () => {
describe("persistOidcAuthenticatedSettings", () => {
it("should set clientId and issuer in session storage", () => {
persistOidcAuthenticatedSettings(clientId, issuer, idTokenClaims);
- expect(sessionStorage.setItem).toHaveBeenCalledWith("mx_oidc_client_id", clientId);
- expect(sessionStorage.setItem).toHaveBeenCalledWith("mx_oidc_token_issuer", issuer);
- expect(sessionStorage.setItem).toHaveBeenCalledWith(
- "mx_oidc_id_token_claims",
- JSON.stringify(idTokenClaims),
- );
+ expect(localStorage.setItem).toHaveBeenCalledWith("mx_oidc_client_id", clientId);
+ expect(localStorage.setItem).toHaveBeenCalledWith("mx_oidc_token_issuer", issuer);
+ expect(localStorage.setItem).toHaveBeenCalledWith("mx_oidc_id_token_claims", JSON.stringify(idTokenClaims));
});
});
describe("getStoredOidcTokenIssuer()", () => {
it("should return issuer from session storage", () => {
- jest.spyOn(sessionStorage.__proto__, "getItem").mockReturnValue(issuer);
+ localStorage.setItem("mx_oidc_token_issuer", issuer);
expect(getStoredOidcTokenIssuer()).toEqual(issuer);
- expect(sessionStorage.getItem).toHaveBeenCalledWith("mx_oidc_token_issuer");
+ expect(localStorage.getItem).toHaveBeenCalledWith("mx_oidc_token_issuer");
});
it("should return undefined when no issuer in session storage", () => {
@@ -68,9 +66,9 @@ describe("persist OIDC settings", () => {
describe("getStoredOidcClientId()", () => {
it("should return clientId from session storage", () => {
- jest.spyOn(sessionStorage.__proto__, "getItem").mockReturnValue(clientId);
+ localStorage.setItem("mx_oidc_client_id", clientId);
expect(getStoredOidcClientId()).toEqual(clientId);
- expect(sessionStorage.getItem).toHaveBeenCalledWith("mx_oidc_client_id");
+ expect(localStorage.getItem).toHaveBeenCalledWith("mx_oidc_client_id");
});
it("should throw when no clientId in session storage", () => {
expect(() => getStoredOidcClientId()).toThrow("Oidc client id not found in storage");
@@ -79,9 +77,9 @@ describe("persist OIDC settings", () => {
describe("getStoredOidcIdTokenClaims()", () => {
it("should return issuer from session storage", () => {
- jest.spyOn(sessionStorage.__proto__, "getItem").mockReturnValue(JSON.stringify(idTokenClaims));
+ localStorage.setItem("mx_oidc_id_token_claims", JSON.stringify(idTokenClaims));
expect(getStoredOidcIdTokenClaims()).toEqual(idTokenClaims);
- expect(sessionStorage.getItem).toHaveBeenCalledWith("mx_oidc_id_token_claims");
+ expect(localStorage.getItem).toHaveBeenCalledWith("mx_oidc_id_token_claims");
});
it("should return undefined when no issuer in session storage", () => {