Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix OIDC bugs due to amnesiac stores forgetting OIDC issuer & other data #12166

Merged
merged 2 commits into from
Jan 23, 2024
Merged

Fix OIDC bugs due to amnesiac stores forgetting OIDC issuer & other data #12166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a743e99
Fix OIDC bugs due to amnesiac stores forgetting OIDC issuer & other data
t3chguy Jan 22, 2024
e557483
Fix tests
t3chguy Jan 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions src/utils/oidc/persistOidcSettings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -31,39 +31,39 @@ 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");
}
return clientId;
};

/**
* 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;
}
Expand Down
4 changes: 2 additions & 2 deletions test/components/structures/MatrixChat-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,8 @@ describe("<MatrixChat />", () => {

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 () => {
Expand Down
30 changes: 7 additions & 23 deletions test/stores/oidc/OidcClientStore-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,16 @@ describe("OidcClientStore", () => {
const clientId = "test-client-id";
const metadata = mockOpenIdConfiguration();
const account = metadata.issuer + "account";
const mockSessionStorage: Record<string, string> = {
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,
Expand All @@ -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);
Expand All @@ -98,14 +95,7 @@ describe("OidcClientStore", () => {
});

it("should log and return when no clientId is found in storage", async () => {
const sessionStorageWithoutClientId: Record<string, string | null> = {
...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
Expand Down Expand Up @@ -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<string, string | null> = {
...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);

Expand Down
28 changes: 13 additions & 15 deletions test/utils/oidc/persistOidcSettings-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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", () => {
Expand All @@ -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");
Expand All @@ -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", () => {
Expand Down
Loading