diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 9f2fcaa197..dc343e2b06 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -65,6 +65,7 @@ import { OverwriteLoginPayload } from "./dispatcher/payloads/OverwriteLoginPaylo import { SdkContextClass } from "./contexts/SDKContext"; import { messageForLoginError } from "./utils/ErrorUtils"; import { completeOidcLogin } from "./utils/oidc/authorize"; +import { getOidcErrorMessage } from "./utils/oidc/error"; import { OidcClientStore } from "./stores/oidc/OidcClientStore"; import { getStoredOidcClientId, @@ -306,8 +307,7 @@ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise } catch (error) { logger.error("Failed to login via OIDC", error); - // TODO(kerrya) nice error messages https://github.com/vector-im/element-web/issues/25665 - await onFailedDelegatedAuthLogin(_t("auth|oidc|error_generic")); + await onFailedDelegatedAuthLogin(getOidcErrorMessage(error as Error)); return false; } } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index e80adab5cb..12982b9117 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -228,9 +228,10 @@ "msisdn_field_required_invalid": "Enter phone number", "no_hs_url_provided": "No homeserver URL provided", "oidc": { - "error_generic": "Something went wrong.", "error_title": "We couldn't log you in", - "logout_redirect_warning": "You will be redirected to your server's authentication provider to complete sign out." + "generic_auth_error": "Something went wrong during authentication. Go to the sign in page and try again.", + "logout_redirect_warning": "You will be redirected to your server's authentication provider to complete sign out.", + "missing_or_invalid_stored_state": "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." }, "password_field_keep_going_prompt": "Keep going…", "password_field_label": "Enter password", diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 6b9b46f5fc..154b07d9ed 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -20,6 +20,8 @@ import { OidcClientConfig } from "matrix-js-sdk/src/matrix"; import { randomString } from "matrix-js-sdk/src/randomstring"; import { IdTokenClaims } from "oidc-client-ts"; +import { OidcClientError } from "./error"; + /** * Start OIDC authorization code flow * Generates auth params, stores them in session storage and @@ -68,7 +70,7 @@ const getCodeAndStateFromQueryParams = (queryParams: QueryDict): { code: string; const state = queryParams["state"]; if (!code || typeof code !== "string" || !state || typeof state !== "string") { - throw new Error("Invalid query parameters for OIDC native login. `code` and `state` are required."); + throw new Error(OidcClientError.InvalidQueryParameters); } return { code, state }; }; diff --git a/src/utils/oidc/error.ts b/src/utils/oidc/error.ts new file mode 100644 index 0000000000..7b44c5c823 --- /dev/null +++ b/src/utils/oidc/error.ts @@ -0,0 +1,47 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { ReactNode } from "react"; +import { OidcError } from "matrix-js-sdk/src/oidc/error"; + +import { _t } from "../../languageHandler"; + +/** + * Errors thrown by EW during OIDC native flow authentication. + * Intended to be logged, not read by users. + */ +export enum OidcClientError { + InvalidQueryParameters = "Invalid query parameters for OIDC native login. `code` and `state` are required.", +} + +/** + * Get a friendly translated error message for user consumption + * based on error encountered during authentication + * @param error + * @returns a friendly translated error message for user consumption + */ +export const getOidcErrorMessage = (error: Error): string | ReactNode => { + switch (error.message) { + case OidcError.MissingOrInvalidStoredState: + return _t("auth|oidc|missing_or_invalid_stored_state"); + case OidcClientError.InvalidQueryParameters: + case OidcError.CodeExchangeFailed: + case OidcError.InvalidBearerTokenResponse: + case OidcError.InvalidIdToken: + default: + return _t("auth|oidc|generic_auth_error"); + } +}; diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 3b5ed9390f..f64d0a2d79 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -44,6 +44,7 @@ import { unmockClientPeg, } from "../../test-utils"; import * as leaveRoomUtils from "../../../src/utils/leave-behaviour"; +import { OidcClientError } from "../../../src/utils/oidc/error"; import * as voiceBroadcastUtils from "../../../src/voice-broadcast/utils/cleanUpBroadcasts"; import LegacyCallHandler from "../../../src/LegacyCallHandler"; import { CallStore } from "../../../src/stores/CallStore"; @@ -915,10 +916,13 @@ describe("", () => { let loginClient!: ReturnType; - // for now when OIDC fails for any reason we just bump back to welcome - // error handling screens in https://github.com/vector-im/element-web/issues/25665 - const expectOIDCError = async (): Promise => { + 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(); }; @@ -972,7 +976,7 @@ describe("", () => { expect(logger.error).toHaveBeenCalledWith( "Failed to login via OIDC", - new Error("Invalid query parameters for OIDC native login. `code` and `state` are required."), + new Error(OidcClientError.InvalidQueryParameters), ); await expectOIDCError(); @@ -1027,6 +1031,24 @@ describe("", () => { 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 }); diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index daf65937c8..2fdfae8e12 100644 --- a/test/utils/oidc/authorize-test.ts +++ b/test/utils/oidc/authorize-test.ts @@ -22,6 +22,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"; jest.unmock("matrix-js-sdk/src/randomstring"); @@ -125,8 +126,8 @@ describe("OIDC authorization", () => { }); it("should throw when query params do not include state and code", async () => { - await expect(completeOidcLogin({})).rejects.toThrow( - "Invalid query parameters for OIDC native login. `code` and `state` are required.", + await expect(async () => await completeOidcLogin({})).rejects.toThrow( + OidcClientError.InvalidQueryParameters, ); });