diff --git a/apps/web/src/Lifecycle.test.ts b/apps/web/src/Lifecycle.test.ts index c5c03592a3c..8ec403d9861 100644 --- a/apps/web/src/Lifecycle.test.ts +++ b/apps/web/src/Lifecycle.test.ts @@ -11,9 +11,8 @@ Please see LICENSE files in the repository root for full details. import { vi, describe, it, expect, beforeEach, afterEach, type MockedObject } from "vitest"; import { logger } from "matrix-js-sdk/src/logger"; import * as MatrixJs from "matrix-js-sdk/src/matrix"; -import { decodeBase64, encodeUnpaddedBase64, MatrixClient, OAuth2 } from "matrix-js-sdk/src/matrix"; +import { decodeBase64, encodeUnpaddedBase64 } from "matrix-js-sdk/src/matrix"; import * as encryptAESSecretStorageItemModule from "matrix-js-sdk/src/utils/encryptAESSecretStorageItem"; -import fetchMock from "@fetch-mock/vitest"; import { flushPromises, getMockClientWithEventEmitter, @@ -21,7 +20,6 @@ import { mockClientMethodsServer, mockPlatformPeg, } from "test-utils"; -import { makeDelegatedAuthMetadata } from "test-utils/auth"; import StorageEvictedDialog from "./components/views/dialogs/StorageEvictedDialog"; import * as Lifecycle from "./Lifecycle"; @@ -82,7 +80,6 @@ describe("Lifecycle", () => { localStorage.clear(); sessionStorage.clear(); - vi.spyOn(MatrixClient.prototype, "getAuthMetadata").mockResolvedValue(makeDelegatedAuthMetadata()); }); afterEach(() => { @@ -236,7 +233,6 @@ describe("Lifecycle", () => { userId, guest: true, }), - undefined, ); expect(localStorage.getItem("mx_is_guest")).toEqual("true"); }); @@ -275,19 +271,16 @@ describe("Lifecycle", () => { it("should create and start new matrix client with credentials", async () => { expect(await restoreSessionFromStorage()).toEqual(true); - expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith( - { - userId, - accessToken, - homeserverUrl, - identityServerUrl, - deviceId, - freshLogin: false, - guest: false, - pickleKey: undefined, - }, - undefined, - ); + expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith({ + userId, + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: false, + guest: false, + pickleKey: undefined, + }); expect(MatrixClientPeg.start).toHaveBeenCalledWith({}); }); @@ -325,21 +318,18 @@ describe("Lifecycle", () => { it("should create new matrix client with credentials", async () => { expect(await restoreSessionFromStorage()).toEqual(true); - expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith( - { - userId, - accessToken, - // refreshToken included in credentials - refreshToken, - homeserverUrl, - identityServerUrl, - deviceId, - freshLogin: false, - guest: false, - pickleKey: undefined, - }, - expect.any(OAuth2), - ); + expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith({ + userId, + accessToken, + // refreshToken included in credentials + refreshToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: false, + guest: false, + pickleKey: undefined, + }); }); }); }); @@ -408,20 +398,17 @@ describe("Lifecycle", () => { expect(await restoreSessionFromStorage()).toEqual(true); // Ensure that the expected calls were made - expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith( - { - userId, - // decrypted accessToken - accessToken, - homeserverUrl, - identityServerUrl, - deviceId, - freshLogin: false, - guest: false, - pickleKey, - }, - expect.any(OAuth2), - ); + expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith({ + userId, + // decrypted accessToken + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: false, + guest: false, + pickleKey, + }); expect(MatrixClientPeg.start).toHaveBeenCalledWith({ rustCryptoStoreKey: expect.any(Uint8Array) }); }); @@ -446,21 +433,18 @@ describe("Lifecycle", () => { it("should create new matrix client with credentials", async () => { expect(await restoreSessionFromStorage()).toEqual(true); - expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith( - { - userId, - accessToken, - // refreshToken included in credentials - refreshToken, - homeserverUrl, - identityServerUrl, - deviceId, - freshLogin: false, - guest: false, - pickleKey: pickleKey, - }, - expect.any(OAuth2), - ); + expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith({ + userId, + accessToken, + // refreshToken included in credentials + refreshToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: false, + guest: false, + pickleKey: pickleKey, + }); }); }); }); @@ -499,20 +483,17 @@ describe("Lifecycle", () => { expect(await restoreSessionFromStorage()).toEqual(true); // Ensure that the expected calls were made - expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith( - { - userId, - // decrypted accessToken - accessToken, - homeserverUrl, - identityServerUrl, - deviceId, - freshLogin: false, - guest: false, - pickleKey, - }, - undefined, - ); + expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith({ + userId, + // decrypted accessToken + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: false, + guest: false, + pickleKey, + }); expect(MatrixClientPeg.start).toHaveBeenCalledWith({ rustCryptoStorePassword: pickleKey }); }); @@ -665,19 +646,16 @@ describe("Lifecycle", () => { it("should create new matrix client with credentials", async () => { expect(await setLoggedIn(credentials)).toEqual(mockClient); - expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith( - { - userId, - accessToken, - homeserverUrl, - identityServerUrl, - deviceId, - freshLogin: true, - guest: false, - pickleKey: undefined, - }, - undefined, - ); + expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith({ + userId, + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: true, + guest: false, + pickleKey: undefined, + }); }); }); @@ -760,141 +738,28 @@ describe("Lifecycle", () => { vi.spyOn(createMatrixClientModule, "createClientWithCreds").mockReturnValue(mockClient); expect(await setLoggedIn(credentials)).toEqual(mockClient); - expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith( - { - userId, - accessToken, - homeserverUrl, - identityServerUrl, - deviceId, - freshLogin: true, - guest: false, - pickleKey: expect.any(String), - }, - undefined, - ); - }); - }); - - // XXX: these tests are broken, Lifecycle.setLoggedIn does not work with OIDC and its token refreshers due to clearing storage - describe.skip("when authenticated via OIDC native flow", () => { - const clientId = "test-client-id"; - const issuer = "https://auth.com/"; - - const delegatedAuthConfig = makeDelegatedAuthMetadata(issuer); - - beforeEach(() => { - // set values in local storage as they would be after a successful oidc authentication - localStorage.setItem("mx_oidc_client_id", clientId); - }); - - it("should not try to create a token refresher without a refresh token", async () => { - const cli = await setLoggedIn(credentials); - - // didn't try to initialise token refresher - expect(cli.http.opts.tokenRefreshFunction).toBeUndefined(); - }); - - it("should not try to create a token refresher without a deviceId", async () => { - await expect( - setLoggedIn({ - ...credentials, - refreshToken, - deviceId: undefined, - }), - ).rejects.toThrow("Expected deviceId in user credentials."); - - // didn't try to initialise token refresher - expect(fetchMock).toHaveFetchedTimes( - 0, - `${delegatedAuthConfig.issuer}.well-known/openid-configuration`, - ); - }); - - it("should not try to create a token refresher without an issuer in session storage", async () => { - await expect( - setLoggedIn({ - ...credentials, - refreshToken, - }), - ).rejects.toThrow("Cannot create an OIDC token refresher as no stored OIDC token issuer was found."); - - // didn't try to initialise token refresher - expect(fetchMock).toHaveFetchedTimes( - 0, - `${delegatedAuthConfig.issuer}.well-known/openid-configuration`, - ); - }); - - it("should create a client with a tokenRefreshFunction", async () => { - expect( - await setLoggedIn({ - ...credentials, - refreshToken, - }), - ).toEqual(mockClient); - - expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith( - expect.objectContaining({ - accessToken, - refreshToken, - }), - expect.any(Function), - ); - }); - - it("should create a client when creating token refresher fails", async () => { - // create invalid value in local storage for a malformed oidc authentication - localStorage.removeItem("mx_oidc_client_id"); - - // succeeded - expect( - await setLoggedIn({ - ...credentials, - refreshToken, - }), - ).toEqual(mockClient); - - expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith( - expect.objectContaining({ - accessToken, - refreshToken, - }), - // no token refresh function - undefined, - ); + expect(createMatrixClientModule.createClientWithCreds).toHaveBeenCalledWith({ + userId, + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: true, + guest: false, + pickleKey: expect.any(String), + }); }); }); }); describe("logout()", () => { - const accessToken = "test-access-token"; - const refreshToken = "test-refresh-token"; - - beforeEach(() => { - mockClient.getAccessToken.mockReturnValue(accessToken); - mockClient.getRefreshToken.mockReturnValue(refreshToken); - vi.spyOn(OAuth2.prototype, "revokeToken").mockResolvedValue(undefined); - }); - - it("should call logout on the client when oauth is not used", async () => { + it("should call logout on the client", async () => { logout(); await flushPromises(); expect(mockClient.logout).toHaveBeenCalledWith(true); }); - - it("should revoke tokens when user is authenticated with oauth2", async () => { - localStorage.setItem("mx_oidc_client_id", "test-client-id"); - logout(); - - await flushPromises(); - - expect(mockClient.logout).not.toHaveBeenCalled(); - expect(OAuth2.prototype.revokeToken).toHaveBeenCalledWith(accessToken, "access_token"); - expect(OAuth2.prototype.revokeToken).toHaveBeenCalledWith(refreshToken, "refresh_token"); - }); }); describe("overwritelogin", () => { @@ -928,7 +793,6 @@ describe("Lifecycle", () => { expect.objectContaining({ userId, }), - undefined, ); const otherCredentials = { @@ -962,7 +826,6 @@ describe("Lifecycle", () => { expect.objectContaining({ userId: otherCredentials.userId, }), - undefined, ); expect(MatrixClientPeg.unset).not.toHaveBeenCalled(); diff --git a/apps/web/src/Lifecycle.ts b/apps/web/src/Lifecycle.ts index 49cef456897..4e0c1030718 100644 --- a/apps/web/src/Lifecycle.ts +++ b/apps/web/src/Lifecycle.ts @@ -10,7 +10,7 @@ Please see LICENSE files in the repository root for full details. */ import { type ReactNode } from "react"; -import { MatrixClient, OAuth2, createClient, SSOAction, decodeBase64 } from "matrix-js-sdk/src/matrix"; +import { type MatrixClient, createClient, SSOAction, decodeBase64 } from "matrix-js-sdk/src/matrix"; import { type AESEncryptedSecretStoragePayload } from "matrix-js-sdk/src/types"; import { logger } from "matrix-js-sdk/src/logger"; @@ -52,7 +52,7 @@ import { SDKContextClass } from "./contexts/SDKContextClass"; import { messageForLoginError } from "./utils/ErrorUtils"; import { completeOAuthLogin, type CompleteOAuthLoginResponse } from "./utils/oauth/authorize"; import { getOAuthErrorMessage } from "./utils/oauth/error"; -import { getOAuthParams, getStoredOAuthClientId, persistOAuthClientId } from "./utils/oauth/persistOAuthSettings"; +import { persistOAuthClientId } from "./utils/oauth/persistOAuthSettings"; import { ACCESS_TOKEN_IV, ACCESS_TOKEN_STORAGE_KEY, @@ -66,7 +66,6 @@ import { import { checkBrowserSupport } from "./SupportedBrowser"; import { type URLParams } from "./vector/url_utils.ts"; import { type OnLoggedInPayload } from "./dispatcher/payloads/OnLoggedInPayload.ts"; -import { filterBoolean } from "./utils/arrays.ts"; import { clearUploadedMediaCache } from "./utils/UploadedMediaCache"; import { CallStatusListener } from "./CallStatusListener.ts"; import { CallStore } from "./stores/CallStore.ts"; @@ -811,14 +810,9 @@ async function doSetLoggedIn( await abortLogin(); } - let auth: OAuth2 | undefined; - try { - auth = await hydrateAuth(credentials); - } catch {} - // check the session lock just before creating the new client checkSessionLock(); - MatrixClientPeg.set(createClientWithCreds(credentials, auth)); + MatrixClientPeg.set(createClientWithCreds(credentials)); const client = MatrixClientPeg.safeGet(); setSentryUser(credentials.userId); @@ -921,32 +915,6 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise { - if (oauth) { - const accessToken = client.getAccessToken(); - const refreshToken = client.getRefreshToken(); - - await Promise.all( - filterBoolean([ - accessToken ? oauth.revokeToken(accessToken, "access_token") : null, - refreshToken ? oauth.revokeToken(refreshToken, "refresh_token") : null, - ]), - ); - - client.stopClient(); - client.http.abort(); - } else { - await client.logout(true); - } -} - /** * Logs the current session out and transitions to the logged-out state */ @@ -954,16 +922,6 @@ export async function logout(): Promise { const client = MatrixClientPeg.get(); if (!client) return; - let oauth: OAuth2 | undefined; - try { - oauth = await hydrateAuth({ - homeserverUrl: client.getHomeserverUrl(), - deviceId: client.getDeviceId()!, - }); - } catch { - // This is fine - } - PosthogAnalytics.instance.logout(); if (client.isGuest()) { @@ -977,17 +935,12 @@ export async function logout(): Promise { _isLoggingOut = true; void PlatformPeg.get()?.destroyPickleKey(client.getSafeUserId(), client.getDeviceId() ?? ""); - doLogout(client, oauth ?? null).then(onLoggedOut, (err) => { - // Just throwing an error here is going to be very unhelpful - // if you're trying to log out because your server's down and - // you want to log into a different server, so just forget the - // access token. It's annoying that this will leave the access - // token still valid, but we should fix this by having access - // tokens expire (and if you really think you've been compromised, - // change your password). + try { + await client.logout(true); + } catch (err) { logger.warn("Failed to call logout API: token will not be invalidated", err); - return onLoggedOut(); - }); + } + await onLoggedOut(); } export function softLogout(): void { @@ -1101,7 +1054,7 @@ async function startMatrixClient( /* * Stops a running client and all related services, and clears persistent - * storage. Used after a session has been logged out. + * storage. Used after a session has been logged out (or at least attempted to be logged out). */ export async function onLoggedOut(): Promise { // Ensure that we dispatch a view change **before** stopping the client, @@ -1228,18 +1181,3 @@ window.mxLoginWithAccessToken = async (hsUrl: string, accessToken: string): Prom false, ); }; - -/** - * Instantiate an OAuth2 instance from storage - * Returned promise will reject if the session or the server are not OAuth2-native. - */ -export async function hydrateAuth( - credentials: Pick, -): Promise { - const storedClientId = getStoredOAuthClientId(); - - const tempClient = new MatrixClient({ baseUrl: credentials.homeserverUrl }); - const authMetadata = await tempClient.getAuthMetadata(); - - return new OAuth2(authMetadata, { ...getOAuthParams(storedClientId), deviceId: credentials.deviceId }); -} diff --git a/apps/web/src/components/structures/MatrixChat.test.tsx b/apps/web/src/components/structures/MatrixChat.test.tsx index f2f917fa83c..7d14566cab0 100644 --- a/apps/web/src/components/structures/MatrixChat.test.tsx +++ b/apps/web/src/components/structures/MatrixChat.test.tsx @@ -554,6 +554,8 @@ describe("", () => { vi.spyOn(logger, "error").mockClear(); vi.spyOn(logger, "log").mockClear(); + mockPlatformPeg(); + loginClient.whoami.mockResolvedValue({ user_id: userId, device_id: deviceId, @@ -568,7 +570,6 @@ describe("", () => { clientId, codeVerifier: "123456", deviceId, - redirectUri: "https://cb", }, }); }); @@ -595,21 +596,21 @@ describe("", () => { it("should make correct request to complete authorization", async () => { getComponent({ urlParams }); - await flushPromises(); - - expect(OAuth2.prototype.completeAuthorizationCodeGrant).toHaveBeenCalledWith(code); + await waitFor(() => { + expect(OAuth2.prototype.completeAuthorizationCodeGrant).toHaveBeenCalledWith(code, expect.anything()); + }); }); it("should look up userId using access token", async () => { getComponent({ urlParams }); - await flushPromises(); - - // check we used a client with the correct accesstoken - expect(MatrixJs.createClient).toHaveBeenCalledWith({ - baseUrl: homeserverUrl, - accessToken, - idBaseUrl: identityServerUrl, + await waitFor(() => { + // check we used a client with the correct accesstoken + expect(MatrixJs.createClient).toHaveBeenCalledWith({ + baseUrl: homeserverUrl, + accessToken, + idBaseUrl: identityServerUrl, + }); }); expect(loginClient.whoami).toHaveBeenCalled(); }); @@ -618,12 +619,12 @@ describe("", () => { loginClient.whoami.mockRejectedValue(new Error("oups")); getComponent({ urlParams }); - await flushPromises(); - - expect(logger.error).toHaveBeenCalledWith( - "Failed to login via OAuth", - new Error("Failed to retrieve userId using accessToken"), - ); + await waitFor(() => { + expect(logger.error).toHaveBeenCalledWith( + "Failed to login via OAuth", + new Error("Failed to retrieve userId using accessToken"), + ); + }); await expectOAuthError(); }); diff --git a/apps/web/src/utils/createMatrixClient.test.ts b/apps/web/src/utils/createMatrixClient.test.ts index a261255df74..408df6c7da5 100644 --- a/apps/web/src/utils/createMatrixClient.test.ts +++ b/apps/web/src/utils/createMatrixClient.test.ts @@ -5,10 +5,14 @@ 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. */ +// @vitest-environment happy-dom + import { vi, describe, beforeEach, it, expect } from "vitest"; import { type MatrixClient, RoomNameType } from "matrix-js-sdk/src/matrix"; +import { mockPlatformPeg } from "test-utils"; import { createClientWithCreds } from "./createMatrixClient"; +import PlatformPeg from "../PlatformPeg"; describe("createMatrixClient", () => { let client: MatrixClient; @@ -150,4 +154,64 @@ describe("createMatrixClient", () => { }); }); }); + + describe("oauth2ClientConfig", () => { + const stubLocalStorage = (clientId: string | null): void => { + vi.stubGlobal("localStorage", { + getItem: vi.fn().mockImplementation((key: string) => (key === "mx_oidc_client_id" ? clientId : null)), + setItem: vi.fn(), + removeItem: vi.fn(), + }); + }; + + beforeEach(() => { + mockPlatformPeg(); + Object.defineProperty(PlatformPeg.get(), "getOAuthCallbackUrl", { + value: () => new URL("https://test.dummy/oauth/callback"), + }); + }); + + it("should not be set when there is no refresh token", () => { + stubLocalStorage("test-client-id"); + + client = createClientWithCreds({ + homeserverUrl: "https://test.dummy", + userId: "@user:test.dummy", + accessToken: "access_token", + }); + + expect(client.http.opts.oauth2ClientConfig).toBeUndefined(); + }); + + it("should not be set when there is a refresh token but no stored OAuth2 client ID", () => { + stubLocalStorage(null); + + client = createClientWithCreds({ + homeserverUrl: "https://test.dummy", + userId: "@user:test.dummy", + accessToken: "access_token", + refreshToken: "refresh_token", + }); + + expect(client.http.opts.oauth2ClientConfig).toBeUndefined(); + }); + + it("should be set from the stored OAuth2 client ID when there is a refresh token", () => { + stubLocalStorage("test-client-id"); + + client = createClientWithCreds({ + homeserverUrl: "https://test.dummy", + userId: "@user:test.dummy", + accessToken: "access_token", + refreshToken: "refresh_token", + }); + + expect(client.http.opts.oauth2ClientConfig).toEqual( + expect.objectContaining({ + clientId: "test-client-id", + getAuthMetadata: expect.any(Function), + }), + ); + }); + }); }); diff --git a/apps/web/src/utils/createMatrixClient.ts b/apps/web/src/utils/createMatrixClient.ts index 37097a36f06..8c1536f92c0 100644 --- a/apps/web/src/utils/createMatrixClient.ts +++ b/apps/web/src/utils/createMatrixClient.ts @@ -19,8 +19,6 @@ import { type RoomNameState, EventTimelineSet, EventTimeline, - type OAuth2, - TokenRefresher, } from "matrix-js-sdk/src/matrix"; import { VerificationMethod } from "matrix-js-sdk/src/types"; import { logger } from "matrix-js-sdk/src/logger"; @@ -32,6 +30,7 @@ import IdentityAuthClient from "../IdentityAuthClient"; import { _t } from "../languageHandler"; import { formatList } from "./FormattingUtils"; import { persistTokens } from "./tokens/tokens.ts"; +import { getStoredOAuthClientId } from "./oauth/persistOAuthSettings"; const localStorage = window.localStorage; @@ -119,17 +118,17 @@ function roomNameGenerator(_: string, state: RoomNameState): string | null { * Create a new matrix client from credentials with all the options needed. * * @param creds The credentials to create the client with - * @param oauth The OAuth2 instance for OAuth2-native sessions * * @returns {MatrixClient} the newly-created MatrixClient */ -export function createClientWithCreds(creds: IMatrixClientCreds, oauth?: OAuth2): MatrixClient { - let tokenRefreshFunction: ICreateClientOpts["tokenRefreshFunction"]; - if (creds.refreshToken && oauth) { - const tokenRefresher = new TokenRefresher(oauth, persistTokens.bind(null, creds.pickleKey)); - tokenRefreshFunction = tokenRefresher?.tokenRefreshFunction; - } else { - logger.debug("No refresh token was supplied: access token will not be refreshed"); +export function createClientWithCreds(creds: IMatrixClientCreds): MatrixClient { + let oauthClientId: string | undefined; + if (creds.refreshToken) { + try { + oauthClientId = getStoredOAuthClientId(); + } catch (e) { + logger.warn("Have a refresh token but no stored OAuth2 client ID: tokens will not be refreshed", e); + } } const opts: ICreateClientOpts = { @@ -137,7 +136,8 @@ export function createClientWithCreds(creds: IMatrixClientCreds, oauth?: OAuth2) idBaseUrl: creds.identityServerUrl, accessToken: creds.accessToken, refreshToken: creds.refreshToken, - tokenRefreshFunction, + onTokenRefresh: persistTokens.bind(null, creds.pickleKey), + oauthClientId, userId: creds.userId, deviceId: creds.deviceId, pickleKey: creds.pickleKey, diff --git a/apps/web/src/utils/oauth/authorize.test.ts b/apps/web/src/utils/oauth/authorize.test.ts index 87c49267a92..ccc44c71438 100644 --- a/apps/web/src/utils/oauth/authorize.test.ts +++ b/apps/web/src/utils/oauth/authorize.test.ts @@ -40,6 +40,7 @@ describe("OAuth2 authorization", () => { window.location = { href: baseUrl, origin: baseUrl, + pathname: "", }; mockPlatformPeg(); @@ -121,7 +122,6 @@ describe("OAuth2 authorization", () => { codeVerifier: "123456", clientId, deviceId: "DEADB33F", - redirectUri: "https://test.com/callback", }, }); }); @@ -135,7 +135,10 @@ describe("OAuth2 authorization", () => { it("should make request complete authorization code grant", async () => { await completeOAuthLogin(params); - expect(OAuth2.prototype.completeAuthorizationCodeGrant).toHaveBeenCalledWith(code); + expect(OAuth2.prototype.completeAuthorizationCodeGrant).toHaveBeenCalledWith( + code, + "https://test.com/?no_universal_links=true", + ); }); it("should return accessToken, configured homeserver and identityServer", async () => { diff --git a/apps/web/src/utils/oauth/authorize.ts b/apps/web/src/utils/oauth/authorize.ts index 36644d58cb7..be248d0ebaf 100644 --- a/apps/web/src/utils/oauth/authorize.ts +++ b/apps/web/src/utils/oauth/authorize.ts @@ -12,7 +12,7 @@ import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import { OAuthClientError } from "./error"; import PlatformPeg from "../../PlatformPeg"; import { type URLParams } from "../../vector/url_utils.ts"; -import { getOAuthParams, loadAuthContext, storeAuthContext } from "./persistOAuthSettings.ts"; +import { getRedirectUrl, loadAuthContext, storeAuthContext } from "./persistOAuthSettings.ts"; const RESPONSE_MODE = "fragment"; @@ -37,7 +37,7 @@ export const startOAuthLogin = async ( const platform = PlatformPeg.get()!; const state = secureRandomString(32) + platform.getOAuthClientState(); - const auth = new OAuth2(authMetadata, getOAuthParams(clientId)); + const auth = new OAuth2(authMetadata, { clientId }); storeAuthContext({ authContext: auth.context, metadata: authMetadata, @@ -48,6 +48,7 @@ export const startOAuthLogin = async ( const authorizationUrl = await auth.generateAuthorizationCodeGrantUrl( state, + getRedirectUrl(), RESPONSE_MODE, isRegistration ? "create" : undefined, ); @@ -115,7 +116,10 @@ export const completeOAuthLogin = async ( throw new Error(OAuth2Error.MissingOrInvalidStoredState); } - const bearerToken = await new OAuth2(context.metadata, context.authContext).completeAuthorizationCodeGrant(code); + const bearerToken = await new OAuth2(context.metadata, context.authContext).completeAuthorizationCodeGrant( + code, + getRedirectUrl(), + ); return { homeserverUrl: context.homeserverUrl, diff --git a/apps/web/src/utils/oauth/persistOAuthSettings.ts b/apps/web/src/utils/oauth/persistOAuthSettings.ts index c38ad4f8fe5..2815830f438 100644 --- a/apps/web/src/utils/oauth/persistOAuthSettings.ts +++ b/apps/web/src/utils/oauth/persistOAuthSettings.ts @@ -39,13 +39,12 @@ export const getStoredOAuthClientId = (): string => { type OAuth2Context = ConstructorParameters[1]; /** - * Utility function to get the OAuth parameters needed to construct an OAuth2 instance - * @param clientId - the registered OAuth client ID + * Utility function to get the redirect URL for the OAuth2 instance */ -export function getOAuthParams(clientId: string): OAuth2Context { +export function getRedirectUrl(): string { const platform = PlatformPeg.get()!; const redirectUri = platform.getOAuthCallbackUrl().href; - return { clientId, redirectUri }; + return redirectUri; } /**