OIDC: add friendly errors (#11184)

* add delegatedauthentication to validated server config

* dynamic client registration functions

* test OP registration functions

* add stubbed nativeOidc flow setup in Login

* cover more error cases in Login

* tidy

* test dynamic client registration in Login

* comment oidc_static_clients

* register oidc inside Login.getFlows

* strict fixes

* remove unused code

* and imports

* comments

* comments 2

* util functions to get static client id

* check static client ids in login flow

* remove dead code

* OidcRegistrationClientMetadata type

* navigate to oidc authorize url

* exchange code for token

* navigate to oidc authorize url

* navigate to oidc authorize url

* test

* adjust for js-sdk code

* login with oidc native flow: messy version

* tidy

* update test for response_mode query

* tidy up some TODOs

* use new types

* add identityServerUrl to stored params

* unit test completeOidcLogin

* test tokenlogin

* strict

* whitespace

* tidy

* unit test oidc login flow in MatrixChat

* strict

* tidy

* extract success/failure handlers from token login function

* typo

* use for no homeserver error dialog too

* reuse post-token login functions, test

* shuffle testing utils around

* shuffle testing utils around

* i18n

* tidy

* Update src/Lifecycle.ts

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

* tidy

* comment

* update tests for id token validation

* move try again responsibility

* prettier

* add friendly error messages for oidc authorization failures

* i18n

* update for new translations, tidy

---------

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

Author: Kerry

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<boolean>
     } 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;
     }
 }

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",

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 };
 };

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");
+    }
+};

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("<MatrixChat />", () => {
 
         let loginClient!: ReturnType<typeof getMockClientWithEventEmitter>;
 
-        // 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<void> => {
+        const expectOIDCError = async (
+            errorMessage = "Something went wrong during authentication. Go to the sign in page and try again.",
+        ): Promise<void> => {
             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("<MatrixChat />", () => {
 
             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("<MatrixChat />", () => {
                 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 });
 

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,
             );
         });