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

Commit

Permalink
Fix element-desktop-ssoid being included in OIDC Authorization call (
Browse files Browse the repository at this point in the history
…#12495)

* Fix `element-desktop-ssoid being` included in OIDC Authorization call

Signed-off-by: Michael Telatynski <[email protected]>

* Split out oidc callback url into its own method

Signed-off-by: Michael Telatynski <[email protected]>

* Fix unexpected hash on oidc callback url

Signed-off-by: Michael Telatynski <[email protected]>

* Update src/BasePlatform.ts

Co-authored-by: Richard van der Hoff <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
  • Loading branch information
t3chguy and richvdh authored May 13, 2024
1 parent ed7a21a commit cc69589
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 6 deletions.
15 changes: 13 additions & 2 deletions src/BasePlatform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ export default abstract class BasePlatform {
}

/**
* The URL to return to after a successful SSO/OIDC authentication
* The URL to return to after a successful SSO authentication
* @param fragmentAfterLogin optional fragment for specific view to return to
*/
public getSSOCallbackUrl(fragmentAfterLogin = ""): URL {
Expand Down Expand Up @@ -438,7 +438,7 @@ export default abstract class BasePlatform {
return {
clientName: config.brand,
clientUri: this.baseUrl,
redirectUris: [this.getSSOCallbackUrl().href],
redirectUris: [this.getOidcCallbackUrl().href],
logoUri: new URL("vector-icons/1024.png", this.baseUrl).href,
applicationType: "web",
// XXX: We break the spec by not consistently supplying these required fields
Expand All @@ -457,4 +457,15 @@ export default abstract class BasePlatform {
public getOidcClientState(): string {
return "";
}

/**
* The URL to return to after a successful OIDC authentication
*/
public getOidcCallbackUrl(): URL {
const url = new URL(window.location.href);
// The redirect URL has to exactly match that registered at the OIDC server, so
// ensure that the fragment part of the URL is empty.
url.hash = "";
return url;
}
}
2 changes: 1 addition & 1 deletion src/Lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ async function createOidcTokenRefresher(credentials: IMatrixClientCreds): Promis
try {
const clientId = getStoredOidcClientId();
const idTokenClaims = getStoredOidcIdTokenClaims();
const redirectUri = PlatformPeg.get()!.getSSOCallbackUrl().href;
const redirectUri = PlatformPeg.get()!.getOidcCallbackUrl().href;
const deviceId = credentials.deviceId;
if (!deviceId) {
throw new Error("Expected deviceId in user credentials.");
Expand Down
2 changes: 1 addition & 1 deletion src/stores/oidc/OidcClientStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export class OidcClientStore {
...metadata,
authority: metadata.issuer,
signingKeys,
redirect_uri: PlatformPeg.get()!.getSSOCallbackUrl().href,
redirect_uri: PlatformPeg.get()!.getOidcCallbackUrl().href,
client_id: clientId,
});
} catch (error) {
Expand Down
2 changes: 1 addition & 1 deletion src/utils/oidc/authorize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export const startOidcLogin = async (
identityServerUrl?: string,
isRegistration?: boolean,
): Promise<void> => {
const redirectUri = PlatformPeg.get()!.getSSOCallbackUrl().href;
const redirectUri = PlatformPeg.get()!.getOidcCallbackUrl().href;

const nonce = randomString(10);

Expand Down
2 changes: 1 addition & 1 deletion test/utils/oidc/registerClient-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe("getOidcClientId()", () => {
return baseUrl;
},
});
Object.defineProperty(PlatformPeg.get(), "getSSOCallbackUrl", {
Object.defineProperty(PlatformPeg.get(), "getOidcCallbackUrl", {
value: () => ({
href: baseUrl,
}),
Expand Down

0 comments on commit cc69589

Please sign in to comment.