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

Fix Native OIDC for Element Desktop #12253

Merged
merged 18 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 13 additions & 1 deletion src/BasePlatform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,11 @@ export default abstract class BasePlatform {
return null;
}

protected getSSOCallbackUrl(fragmentAfterLogin = ""): URL {
/**
* The URL to return to after a successful SSO/OIDC authentication
* @param fragmentAfterLogin optional fragment for specific view to return to
*/
public getSSOCallbackUrl(fragmentAfterLogin = ""): URL {
const url = new URL(window.location.href);
url.hash = fragmentAfterLogin;
return url;
Expand Down Expand Up @@ -478,4 +482,12 @@ export default abstract class BasePlatform {
policyUri: config.privacy_policy_url,
};
}

/**
* Suffix to append to the `state` parameter of OIDC /auth calls. Will be round-tripped to the callback URI.
* Currently only required for ElectronPlatform for passing element-desktop-ssoid.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there an associated PR which will actually populate this for ElectronPlatform ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*/
public getOidcClientState(): string {
return "";
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
}
}
2 changes: 1 addition & 1 deletion src/Lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ async function createOidcTokenRefresher(credentials: IMatrixClientCreds): Promis
try {
const clientId = getStoredOidcClientId();
const idTokenClaims = getStoredOidcIdTokenClaims();
const redirectUri = window.location.origin;
const redirectUri = PlatformPeg.get()!.getSSOCallbackUrl().href;
const deviceId = credentials.deviceId;
if (!deviceId) {
throw new Error("Expected deviceId in user credentials.");
Expand Down
3 changes: 2 additions & 1 deletion src/stores/oidc/OidcClientStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { OidcClient } from "oidc-client-ts";

import { getStoredOidcTokenIssuer, getStoredOidcClientId } from "../../utils/oidc/persistOidcSettings";
import { getDelegatedAuthAccountUrl } from "../../utils/oidc/getDelegatedAuthAccountUrl";
import PlatformPeg from "../../PlatformPeg";

/**
* @experimental
Expand Down Expand Up @@ -139,7 +140,7 @@ export class OidcClientStore {
...metadata,
authority: metadata.issuer,
signingKeys,
redirect_uri: window.location.origin,
redirect_uri: PlatformPeg.get()!.getSSOCallbackUrl().href,
client_id: clientId,
});
} catch (error) {
Expand Down
4 changes: 3 additions & 1 deletion src/utils/oidc/authorize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { randomString } from "matrix-js-sdk/src/randomstring";
import { IdTokenClaims } from "oidc-client-ts";

import { OidcClientError } from "./error";
import PlatformPeg from "../../PlatformPeg";

/**
* Start OIDC authorization code flow
Expand All @@ -39,7 +40,7 @@ export const startOidcLogin = async (
identityServerUrl?: string,
isRegistration?: boolean,
): Promise<void> => {
const redirectUri = window.location.origin;
const redirectUri = PlatformPeg.get()!.getSSOCallbackUrl().href;

const nonce = randomString(10);

Expand All @@ -53,6 +54,7 @@ export const startOidcLogin = async (
identityServerUrl,
nonce,
prompt,
urlState: PlatformPeg.get()?.getOidcClientState(),
});

window.location.href = authorizationUrl;
Expand Down
3 changes: 2 additions & 1 deletion test/stores/oidc/OidcClientStore-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { discoverAndValidateAuthenticationConfig } from "matrix-js-sdk/src/oidc/
import { OidcError } from "matrix-js-sdk/src/oidc/error";

import { OidcClientStore } from "../../../src/stores/oidc/OidcClientStore";
import { flushPromises, getMockClientWithEventEmitter } from "../../test-utils";
import { flushPromises, getMockClientWithEventEmitter, mockPlatformPeg } from "../../test-utils";
import { mockOpenIdConfiguration } from "../../test-utils/oidc";

jest.mock("matrix-js-sdk/src/oidc/discovery", () => ({
Expand Down Expand Up @@ -58,6 +58,7 @@ describe("OidcClientStore", () => {
jest.spyOn(logger, "error").mockClear();

fetchMock.get(`${metadata.issuer}.well-known/openid-configuration`, metadata);
mockPlatformPeg();
});

describe("isUserAuthenticatedWithOidc()", () => {
Expand Down
2 changes: 2 additions & 0 deletions test/utils/oidc/authorize-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { mocked } from "jest-mock";
import { completeOidcLogin, startOidcLogin } from "../../../src/utils/oidc/authorize";
import { makeDelegatedAuthConfig } from "../../test-utils/oidc";
import { OidcClientError } from "../../../src/utils/oidc/error";
import { mockPlatformPeg } from "../../test-utils";

jest.unmock("matrix-js-sdk/src/randomstring");

Expand Down Expand Up @@ -53,6 +54,7 @@ describe("OIDC authorization", () => {
};

jest.spyOn(randomStringUtils, "randomString").mockRestore();
mockPlatformPeg();
});

beforeAll(() => {
Expand Down
Loading