From 76b7aa2d33dfb8b00e3253db4d89376dc4ef6d65 Mon Sep 17 00:00:00 2001 From: Milton Moura Date: Fri, 24 Nov 2023 14:51:28 -0100 Subject: [PATCH] Keep device language when it has been previosuly set, after a successful delegated authentication flow that clears localStorage (#11902) * Do not remove the language when clearing local storage Signed-off-by: Milton Moura * Revised comment on getting the defined language from settings Signed-off-by: Milton Moura * Explicitly checking for language set at the device level Signed-off-by: Milton Moura * Add test that checks if device language setting is kept after a successfull delegated authentication flow Signed-off-by: Milton Moura * Adds test for unhappy path and adjusts to use the SettingsStore instead of going directly to localStorage Signed-off-by: Milton Moura * Removing unnecessary variable after test refactor Signed-off-by: Milton Moura --------- Signed-off-by: Milton Moura --- src/Lifecycle.ts | 10 +- .../components/structures/MatrixChat-test.tsx | 534 +++++++++--------- 2 files changed, 291 insertions(+), 253 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index dc343e2b06..0f2435ffe8 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -40,6 +40,7 @@ import PlatformPeg from "./PlatformPeg"; import { sendLoginRequest } from "./Login"; import * as StorageManager from "./utils/StorageManager"; import SettingsStore from "./settings/SettingsStore"; +import { SettingLevel } from "./settings/SettingLevel"; import ToastStore from "./stores/ToastStore"; import { IntegrationManagers } from "./integrations/IntegrationManagers"; import { Mjolnir } from "./mjolnir/Mjolnir"; @@ -1105,6 +1106,9 @@ export async function onLoggedOut(): Promise { */ async function clearStorage(opts?: { deleteEverything?: boolean }): Promise { if (window.localStorage) { + // get the currently defined device language, if set, so we can restore it later + const language = SettingsStore.getValueAt(SettingLevel.DEVICE, "language", null, true, true); + // try to save any 3pid invites from being obliterated and registration time const pendingInvites = ThreepidInviteStore.instance.getWireInvites(); const registrationTime = window.localStorage.getItem("mx_registration_time"); @@ -1118,8 +1122,12 @@ async function clearStorage(opts?: { deleteEverything?: boolean }): Promise { ThreepidInviteStore.instance.storeInvite(roomId, invite); }); diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 7e4960fc23..bd1219c359 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -55,6 +55,8 @@ import PlatformPeg from "../../../src/PlatformPeg"; import EventIndexPeg from "../../../src/indexing/EventIndexPeg"; import * as Lifecycle from "../../../src/Lifecycle"; import { SSO_HOMESERVER_URL_KEY, SSO_ID_SERVER_URL_KEY } from "../../../src/BasePlatform"; +import SettingsStore from "../../../src/settings/SettingsStore"; +import { SettingLevel } from "../../../src/settings/SettingLevel"; jest.mock("matrix-js-sdk/src/oidc/authorize", () => ({ completeAuthorizationCodeGrant: jest.fn(), @@ -225,6 +227,286 @@ describe("", () => { expect(container).toMatchSnapshot(); }); + describe("when query params have a OIDC params", () => { + const issuer = "https://auth.com/"; + const homeserverUrl = "https://matrix.org"; + const identityServerUrl = "https://is.org"; + const clientId = "xyz789"; + + const code = "test-oidc-auth-code"; + const state = "test-oidc-state"; + const realQueryParams = { + code, + state: state, + }; + + const userId = "@alice:server.org"; + const deviceId = "test-device-id"; + const accessToken = "test-access-token-from-oidc"; + + const tokenResponse: BearerTokenResponse = { + access_token: accessToken, + refresh_token: "def456", + scope: "test", + token_type: "Bearer", + expires_at: 12345, + }; + + let loginClient!: ReturnType; + + const expectOIDCError = async ( + errorMessage = "Something went wrong during authentication. Go to the sign in page and try again.", + ): Promise => { + await flushPromises(); + const dialog = await screen.findByRole("dialog"); + + expect(within(dialog).getByText(errorMessage)).toBeInTheDocument(); + // just check we're back on welcome page + expect(document.querySelector(".mx_Welcome")!).toBeInTheDocument(); + }; + + beforeEach(() => { + mocked(completeAuthorizationCodeGrant) + .mockClear() + .mockResolvedValue({ + oidcClientSettings: { + clientId, + issuer, + }, + tokenResponse, + homeserverUrl, + identityServerUrl, + idTokenClaims: { + aud: "123", + iss: issuer, + sub: "123", + exp: 123, + iat: 456, + }, + }); + + jest.spyOn(logger, "error").mockClear(); + }); + + beforeEach(() => { + loginClient = getMockClientWithEventEmitter(getMockClientMethods()); + // this is used to create a temporary client during login + jest.spyOn(MatrixJs, "createClient").mockReturnValue(loginClient); + + jest.spyOn(logger, "error").mockClear(); + jest.spyOn(logger, "log").mockClear(); + + loginClient.whoami.mockResolvedValue({ + user_id: userId, + device_id: deviceId, + is_guest: false, + }); + }); + + it("should fail when query params do not include valid code and state", async () => { + const queryParams = { + code: 123, + state: "abc", + }; + getComponent({ realQueryParams: queryParams }); + + await flushPromises(); + + expect(logger.error).toHaveBeenCalledWith( + "Failed to login via OIDC", + new Error(OidcClientError.InvalidQueryParameters), + ); + + await expectOIDCError(); + }); + + it("should make correct request to complete authorization", async () => { + getComponent({ realQueryParams }); + + await flushPromises(); + + expect(completeAuthorizationCodeGrant).toHaveBeenCalledWith(code, state); + }); + + it("should look up userId using access token", async () => { + getComponent({ realQueryParams }); + + await flushPromises(); + + // check we used a client with the correct accesstoken + expect(MatrixJs.createClient).toHaveBeenCalledWith({ + baseUrl: homeserverUrl, + accessToken, + idBaseUrl: identityServerUrl, + }); + expect(loginClient.whoami).toHaveBeenCalled(); + }); + + it("should log error and return to welcome page when userId lookup fails", async () => { + loginClient.whoami.mockRejectedValue(new Error("oups")); + getComponent({ realQueryParams }); + + await flushPromises(); + + expect(logger.error).toHaveBeenCalledWith( + "Failed to login via OIDC", + new Error("Failed to retrieve userId using accessToken"), + ); + await expectOIDCError(); + }); + + it("should call onTokenLoginCompleted", async () => { + const onTokenLoginCompleted = jest.fn(); + getComponent({ realQueryParams, onTokenLoginCompleted }); + + await flushPromises(); + + expect(onTokenLoginCompleted).toHaveBeenCalled(); + }); + + describe("when login fails", () => { + beforeEach(() => { + mocked(completeAuthorizationCodeGrant).mockRejectedValue(new Error(OidcError.CodeExchangeFailed)); + }); + + it("should log and return to welcome page with correct error when login state is not found", async () => { + mocked(completeAuthorizationCodeGrant).mockRejectedValue( + new Error(OidcError.MissingOrInvalidStoredState), + ); + getComponent({ realQueryParams }); + + await flushPromises(); + + expect(logger.error).toHaveBeenCalledWith( + "Failed to login via OIDC", + new Error(OidcError.MissingOrInvalidStoredState), + ); + + await expectOIDCError( + "We asked the browser to remember which homeserver you use to let you sign in, but unfortunately your browser has forgotten it. Go to the sign in page and try again.", + ); + }); + + it("should log and return to welcome page", async () => { + getComponent({ realQueryParams }); + + await flushPromises(); + + expect(logger.error).toHaveBeenCalledWith( + "Failed to login via OIDC", + new Error(OidcError.CodeExchangeFailed), + ); + + // warning dialog + await expectOIDCError(); + }); + + it("should not clear storage", async () => { + getComponent({ realQueryParams }); + + await flushPromises(); + + expect(loginClient.clearStores).not.toHaveBeenCalled(); + }); + + it("should not store clientId or issuer", async () => { + const sessionStorageSetSpy = jest.spyOn(sessionStorage.__proto__, "setItem"); + getComponent({ realQueryParams }); + + await flushPromises(); + + expect(sessionStorageSetSpy).not.toHaveBeenCalledWith("mx_oidc_client_id", clientId); + expect(sessionStorageSetSpy).not.toHaveBeenCalledWith("mx_oidc_token_issuer", issuer); + }); + }); + + describe("when login succeeds", () => { + beforeEach(() => { + jest.spyOn(StorageManager, "idbLoad").mockImplementation( + async (_table: string, key: string | string[]) => (key === "mx_access_token" ? accessToken : null), + ); + loginClient.getProfileInfo.mockResolvedValue({ + displayname: "Ernie", + }); + }); + + it("should persist login credentials", async () => { + getComponent({ realQueryParams }); + + await flushPromises(); + + expect(localStorage.getItem("mx_hs_url")).toEqual(homeserverUrl); + expect(localStorage.getItem("mx_user_id")).toEqual(userId); + expect(localStorage.getItem("mx_has_access_token")).toEqual("true"); + expect(localStorage.getItem("mx_device_id")).toEqual(deviceId); + }); + + it("should store clientId and issuer in session storage", async () => { + getComponent({ realQueryParams }); + + await flushPromises(); + + expect(sessionStorage.getItem("mx_oidc_client_id")).toEqual(clientId); + expect(sessionStorage.getItem("mx_oidc_token_issuer")).toEqual(issuer); + }); + + it("should set logged in and start MatrixClient", async () => { + getComponent({ realQueryParams }); + + await flushPromises(); + await flushPromises(); + + expect(logger.log).toHaveBeenCalledWith( + "setLoggedIn: mxid: " + + userId + + " deviceId: " + + deviceId + + " guest: " + + false + + " hs: " + + homeserverUrl + + " softLogout: " + + false, + " freshLogin: " + true, + ); + + // client successfully started + expect(defaultDispatcher.dispatch).toHaveBeenCalledWith({ action: "client_started" }); + + // check we get to logged in view + await waitForSyncAndLoad(loginClient, true); + }); + + it("should persist device language when available", async () => { + await SettingsStore.setValue("language", null, SettingLevel.DEVICE, "en"); + const languageBefore = SettingsStore.getValueAt(SettingLevel.DEVICE, "language", null, true, true); + + jest.spyOn(Lifecycle, "attemptDelegatedAuthLogin"); + + getComponent({ realQueryParams }); + await flushPromises(); + + expect(Lifecycle.attemptDelegatedAuthLogin).toHaveBeenCalled(); + const languageAfter = SettingsStore.getValueAt(SettingLevel.DEVICE, "language", null, true, true); + expect(languageBefore).toEqual(languageAfter); + }); + + it("should not persist device language when not available", async () => { + await SettingsStore.setValue("language", null, SettingLevel.DEVICE, undefined); + const languageBefore = SettingsStore.getValueAt(SettingLevel.DEVICE, "language", null, true, true); + + jest.spyOn(Lifecycle, "attemptDelegatedAuthLogin"); + + getComponent({ realQueryParams }); + await flushPromises(); + + expect(Lifecycle.attemptDelegatedAuthLogin).toHaveBeenCalled(); + const languageAfter = SettingsStore.getValueAt(SettingLevel.DEVICE, "language", null, true, true); + expect(languageBefore).toEqual(languageAfter); + }); + }); + }); + describe("with an existing session", () => { const mockidb: Record> = { acccount: { @@ -891,258 +1173,6 @@ describe("", () => { }); }); - describe("when query params have a OIDC params", () => { - const issuer = "https://auth.com/"; - const homeserverUrl = "https://matrix.org"; - const identityServerUrl = "https://is.org"; - const clientId = "xyz789"; - - const code = "test-oidc-auth-code"; - const state = "test-oidc-state"; - const realQueryParams = { - code, - state: state, - }; - - const userId = "@alice:server.org"; - const deviceId = "test-device-id"; - const accessToken = "test-access-token-from-oidc"; - - const tokenResponse: BearerTokenResponse = { - access_token: accessToken, - refresh_token: "def456", - scope: "test", - token_type: "Bearer", - expires_at: 12345, - }; - - let loginClient!: ReturnType; - - const expectOIDCError = async ( - errorMessage = "Something went wrong during authentication. Go to the sign in page and try again.", - ): Promise => { - await flushPromises(); - const dialog = await screen.findByRole("dialog"); - - expect(within(dialog).getByText(errorMessage)).toBeInTheDocument(); - // just check we're back on welcome page - expect(document.querySelector(".mx_Welcome")!).toBeInTheDocument(); - }; - - beforeEach(() => { - mocked(completeAuthorizationCodeGrant) - .mockClear() - .mockResolvedValue({ - oidcClientSettings: { - clientId, - issuer, - }, - tokenResponse, - homeserverUrl, - identityServerUrl, - idTokenClaims: { - aud: "123", - iss: issuer, - sub: "123", - exp: 123, - iat: 456, - }, - }); - - jest.spyOn(logger, "error").mockClear(); - }); - - beforeEach(() => { - loginClient = getMockClientWithEventEmitter(getMockClientMethods()); - // this is used to create a temporary client during login - jest.spyOn(MatrixJs, "createClient").mockReturnValue(loginClient); - - jest.spyOn(logger, "error").mockClear(); - jest.spyOn(logger, "log").mockClear(); - - loginClient.whoami.mockResolvedValue({ - user_id: userId, - device_id: deviceId, - is_guest: false, - }); - }); - - it("should fail when query params do not include valid code and state", async () => { - const queryParams = { - code: 123, - state: "abc", - }; - getComponent({ realQueryParams: queryParams }); - - await flushPromises(); - - expect(logger.error).toHaveBeenCalledWith( - "Failed to login via OIDC", - new Error(OidcClientError.InvalidQueryParameters), - ); - - await expectOIDCError(); - }); - - it("should make correct request to complete authorization", async () => { - getComponent({ realQueryParams }); - - await flushPromises(); - - expect(completeAuthorizationCodeGrant).toHaveBeenCalledWith(code, state); - }); - - it("should look up userId using access token", async () => { - getComponent({ realQueryParams }); - - await flushPromises(); - - // check we used a client with the correct accesstoken - expect(MatrixJs.createClient).toHaveBeenCalledWith({ - baseUrl: homeserverUrl, - accessToken, - idBaseUrl: identityServerUrl, - }); - expect(loginClient.whoami).toHaveBeenCalled(); - }); - - it("should log error and return to welcome page when userId lookup fails", async () => { - loginClient.whoami.mockRejectedValue(new Error("oups")); - getComponent({ realQueryParams }); - - await flushPromises(); - - expect(logger.error).toHaveBeenCalledWith( - "Failed to login via OIDC", - new Error("Failed to retrieve userId using accessToken"), - ); - await expectOIDCError(); - }); - - it("should call onTokenLoginCompleted", async () => { - const onTokenLoginCompleted = jest.fn(); - getComponent({ realQueryParams, onTokenLoginCompleted }); - - await flushPromises(); - - expect(onTokenLoginCompleted).toHaveBeenCalled(); - }); - - describe("when login fails", () => { - beforeEach(() => { - mocked(completeAuthorizationCodeGrant).mockRejectedValue(new Error(OidcError.CodeExchangeFailed)); - }); - - it("should log and return to welcome page with correct error when login state is not found", async () => { - mocked(completeAuthorizationCodeGrant).mockRejectedValue( - new Error(OidcError.MissingOrInvalidStoredState), - ); - getComponent({ realQueryParams }); - - await flushPromises(); - - expect(logger.error).toHaveBeenCalledWith( - "Failed to login via OIDC", - new Error(OidcError.MissingOrInvalidStoredState), - ); - - await expectOIDCError( - "We asked the browser to remember which homeserver you use to let you sign in, but unfortunately your browser has forgotten it. Go to the sign in page and try again.", - ); - }); - - it("should log and return to welcome page", async () => { - getComponent({ realQueryParams }); - - await flushPromises(); - - expect(logger.error).toHaveBeenCalledWith( - "Failed to login via OIDC", - new Error(OidcError.CodeExchangeFailed), - ); - - // warning dialog - await expectOIDCError(); - }); - - it("should not clear storage", async () => { - getComponent({ realQueryParams }); - - await flushPromises(); - - expect(loginClient.clearStores).not.toHaveBeenCalled(); - }); - - it("should not store clientId or issuer", async () => { - const sessionStorageSetSpy = jest.spyOn(sessionStorage.__proto__, "setItem"); - getComponent({ realQueryParams }); - - await flushPromises(); - - expect(sessionStorageSetSpy).not.toHaveBeenCalledWith("mx_oidc_client_id", clientId); - expect(sessionStorageSetSpy).not.toHaveBeenCalledWith("mx_oidc_token_issuer", issuer); - }); - }); - - describe("when login succeeds", () => { - beforeEach(() => { - jest.spyOn(StorageManager, "idbLoad").mockImplementation( - async (_table: string, key: string | string[]) => (key === "mx_access_token" ? accessToken : null), - ); - loginClient.getProfileInfo.mockResolvedValue({ - displayname: "Ernie", - }); - }); - - it("should persist login credentials", async () => { - getComponent({ realQueryParams }); - - await flushPromises(); - - expect(localStorage.getItem("mx_hs_url")).toEqual(homeserverUrl); - expect(localStorage.getItem("mx_user_id")).toEqual(userId); - expect(localStorage.getItem("mx_has_access_token")).toEqual("true"); - expect(localStorage.getItem("mx_device_id")).toEqual(deviceId); - }); - - it("should store clientId and issuer in session storage", async () => { - getComponent({ realQueryParams }); - - await flushPromises(); - - expect(sessionStorage.getItem("mx_oidc_client_id")).toEqual(clientId); - expect(sessionStorage.getItem("mx_oidc_token_issuer")).toEqual(issuer); - }); - - it("should set logged in and start MatrixClient", async () => { - getComponent({ realQueryParams }); - - await flushPromises(); - await flushPromises(); - - expect(logger.log).toHaveBeenCalledWith( - "setLoggedIn: mxid: " + - userId + - " deviceId: " + - deviceId + - " guest: " + - false + - " hs: " + - homeserverUrl + - " softLogout: " + - false, - " freshLogin: " + true, - ); - - // client successfully started - expect(defaultDispatcher.dispatch).toHaveBeenCalledWith({ action: "client_started" }); - - // check we get to logged in view - await waitForSyncAndLoad(loginClient, true); - }); - }); - }); - describe("automatic SSO selection", () => { let ssoClient: ReturnType; let hrefSetter: jest.Mock;