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

OIDC: add friendly errors #11184

Merged
merged 89 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from 85 commits
Commits
Show all changes
89 commits
Select commit Hold shift + click to select a range
c2a78d1
add delegatedauthentication to validated server config
Jun 7, 2023
f946b85
dynamic client registration functions
Jun 9, 2023
223d3ff
Merge branch 'develop' into kerry/25466/oidc-validate-wk-mauthenticat…
Jun 11, 2023
f83499e
Merge branch 'kerry/25466/oidc-validate-wk-mauthentication-2' into ke…
Jun 11, 2023
4201725
test OP registration functions
Jun 12, 2023
314105d
add stubbed nativeOidc flow setup in Login
Jun 12, 2023
6fe2d35
cover more error cases in Login
Jun 12, 2023
c473f11
Merge branch 'kerry/25468/test-login' into kerry/25468/oidc-client-re…
Jun 12, 2023
19dc425
tidy
Jun 12, 2023
94d5c36
test dynamic client registration in Login
Jun 12, 2023
67f6c2e
comment oidc_static_clients
Jun 12, 2023
8df0929
register oidc inside Login.getFlows
Jun 12, 2023
0d4de13
Merge branch 'develop' into kerry/25466/oidc-validate-wk-mauthenticat…
Jun 12, 2023
1a53ad0
strict fixes
Jun 12, 2023
f9aaa28
remove unused code
Jun 12, 2023
9faa9c8
and imports
Jun 12, 2023
0f76ffa
Merge branch 'kerry/25466/oidc-validate-wk-mauthentication-2' into ke…
Jun 13, 2023
4dfa18d
Merge branch 'develop' into kerry/25468/oidc-client-registration
Jun 13, 2023
e920229
comments
Jun 13, 2023
5389d5c
comments 2
Jun 13, 2023
af63a90
util functions to get static client id
Jun 13, 2023
9c89f41
check static client ids in login flow
Jun 13, 2023
056a713
Merge branch 'kerry/25468/oidc-client-static-reg' into kerry/25468/oi…
Jun 13, 2023
422823e
remove dead code
Jun 14, 2023
8483b2f
Merge branch 'kerry/25468/oidc-client-static-reg' into kerry/25468/oi…
Jun 14, 2023
a76cde9
OidcRegistrationClientMetadata type
Jun 14, 2023
bf45be2
navigate to oidc authorize url
Jun 14, 2023
e1e22f3
Merge branch 'develop' into kerry/25468/oidc-client-static-reg
Jun 14, 2023
f4a2ff2
Merge branch 'kerry/25468/oidc-client-static-reg' into kerry/25468/oi…
Jun 14, 2023
3b8938c
exchange code for token
Jun 15, 2023
52d03c2
navigate to oidc authorize url
Jun 14, 2023
3c499da
navigate to oidc authorize url
Jun 15, 2023
551670f
test
Jun 15, 2023
423af53
Merge branch 'develop' into kerry/25574/oidc-authorization-endpoint
Jun 22, 2023
b100d6d
adjust for js-sdk code
Jun 23, 2023
6c1321f
Merge branch 'kerry/25574/oidc-authorization-endpoint' into kerry/255…
Jun 23, 2023
28a36ec
login with oidc native flow: messy version
Jun 23, 2023
a1b975d
tidy
Jun 23, 2023
72ff46a
update test for response_mode query
Jun 23, 2023
f566a60
Merge branch 'kerry/25574/oidc-authorization-endpoint' into kerry/255…
Jun 23, 2023
dd9944d
tidy up some TODOs
Jun 23, 2023
857df8d
use new types
Jun 26, 2023
d0ce2a2
Merge branch 'kerry/25574/oidc-authorization-endpoint' into kerry/255…
Jun 26, 2023
9510727
add identityServerUrl to stored params
Jun 26, 2023
044beb7
unit test completeOidcLogin
Jun 26, 2023
f4ff6f5
test tokenlogin
Jun 27, 2023
dbd54c5
Merge branch 'develop' into kerry/25574/oidc-authorization-endpoint
Jun 27, 2023
599baa2
strict
Jun 27, 2023
637eecc
Merge branch 'kerry/25574/oidc-authorization-endpoint' into kerry/255…
Jun 27, 2023
905bac0
whitespace
Jun 27, 2023
7c91acd
tidy
Jun 27, 2023
3dff562
Merge branch 'develop' into kerry/25574/test-token-login
Jun 27, 2023
bb2c50b
Merge branch 'kerry/25574/test-token-login' into kerry/25574/oidc-aut…
Jun 27, 2023
5ba262c
unit test oidc login flow in MatrixChat
Jun 27, 2023
94167e5
strict
Jun 27, 2023
0463d25
Merge branch 'kerry/25574/test-token-login' into kerry/25574/oidc-aut…
Jun 27, 2023
ffbc140
Merge branch 'develop' into kerry/25574/oidc-authorization-endpoint
Jun 28, 2023
88e0cf2
tidy
Jun 28, 2023
480da01
Merge branch 'kerry/25574/oidc-authorization-endpoint' into kerry/255…
Jun 28, 2023
26a745f
extract success/failure handlers from token login function
Jun 28, 2023
38ecd44
typo
Jun 28, 2023
046a4a4
Merge branch 'kerry/25574/token-login-refactor' into kerry/25574/oidc…
Jun 28, 2023
4890e66
use for no homeserver error dialog too
Jun 28, 2023
1a59a7b
Merge branch 'kerry/25574/token-login-refactor' into kerry/25574/oidc…
Jun 28, 2023
225e4f5
reuse post-token login functions, test
Jun 28, 2023
c7199e5
shuffle testing utils around
Jun 28, 2023
56e93ae
shuffle testing utils around
Jun 28, 2023
b9792bc
i18n
Jun 28, 2023
a9ed791
tidy
Jun 28, 2023
71f1633
Update src/Lifecycle.ts
Jun 28, 2023
c920e45
Merge branch 'develop' into kerry/25574/token-login-refactor
Jun 28, 2023
fdfaa01
Merge branch 'develop' into kerry/25574/token-login-refactor
Jun 29, 2023
3602b65
Merge branch 'develop' into kerry/25574/oidc-authorization
Jun 29, 2023
5fddb23
Merge branch 'kerry/25574/token-login-refactor' into kerry/25574/oidc…
Jun 29, 2023
11a6ccc
tidy
Jun 29, 2023
5a2aaad
Merge branch 'kerry/25574/token-login-refactor' into kerry/25574/oidc…
Jun 29, 2023
a7d4e44
comment
Jun 29, 2023
e389fde
update tests for id token validation
Jun 29, 2023
cb8832c
move try again responsibility
Jun 29, 2023
9dccd7a
Merge branch 'kerry/25574/token-login-refactor' into kerry/25574/oidc…
Jun 30, 2023
6ae1965
Merge branch 'develop' into kerry/25574/oidc-authorization
Jul 3, 2023
ccadb29
prettier
Jul 3, 2023
3bd71de
add friendly error messages for oidc authorization failures
Jul 4, 2023
0cb4198
Merge branch 'develop' into kerry/25665/oidc-friendly-errors
Jul 11, 2023
a1b23bd
i18n
Jul 11, 2023
2e7546d
Merge branch 'develop' into kerry/25665/oidc-friendly-errors
Oct 17, 2023
2866ac2
update for new translations, tidy
Oct 17, 2023
3b7759e
Merge branch 'develop' into kerry/25665/oidc-friendly-errors
Oct 18, 2023
153d056
Merge branch 'develop' into kerry/25665/oidc-friendly-errors
Oct 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import { OverwriteLoginPayload } from "./dispatcher/payloads/OverwriteLoginPaylo
import { SdkContextClass } from "./contexts/SDKContext";
import { messageForLoginError } from "./utils/ErrorUtils";
import { completeOidcLogin } from "./utils/oidc/authorize";
import { getErrorMessage } from "./utils/oidc/error";
richvdh marked this conversation as resolved.
Show resolved Hide resolved

const HOMESERVER_URL_KEY = "mx_hs_url";
const ID_SERVER_URL_KEY = "mx_is_url";
Expand Down Expand Up @@ -238,8 +239,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("Something went wrong."));
await onFailedDelegatedAuthLogin(getErrorMessage(error as Error));
return false;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@
"Failed to transfer call": "Failed to transfer call",
"Permission Required": "Permission Required",
"You do not have permission to start a conference call in this room": "You do not have permission to start a conference call in this room",
"Something went wrong.": "Something went wrong.",
"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.": "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.",
"We couldn't log you in": "We couldn't log you in",
"Try again": "Try again",
Expand Down Expand Up @@ -792,6 +791,7 @@
"Invite to %(spaceName)s": "Invite to %(spaceName)s",
"Share your public space": "Share your public space",
"Unknown App": "Unknown App",
"Something went wrong during authentication. Go to the sign in page and try again.": "Something went wrong during authentication. Go to the sign in page and try again.",
"No media permissions": "No media permissions",
"You may need to manually permit %(brand)s to access your microphone/webcam": "You may need to manually permit %(brand)s to access your microphone/webcam",
"This homeserver is not configured to display maps.": "This homeserver is not configured to display maps.",
Expand Down
4 changes: 3 additions & 1 deletion src/utils/oidc/authorize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { OidcClientConfig } from "matrix-js-sdk/src/autodiscovery";
import { generateOidcAuthorizationUrl } from "matrix-js-sdk/src/oidc/authorize";
import { randomString } from "matrix-js-sdk/src/randomstring";

import { OidcClientError } from "./error";

/**
* Start OIDC authorization code flow
* Generates auth params, stores them in session storage and
Expand Down Expand Up @@ -64,7 +66,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 };
};
Expand Down
41 changes: 41 additions & 0 deletions src/utils/oidc/error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
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 { MatrixError } from "matrix-js-sdk/src/http-api";
import { OidcError } from "matrix-js-sdk/src/oidc/error";

import { _t } from "../../languageHandler";

export enum OidcClientError {
InvalidQueryParameters = "Invalid query parameters for OIDC native login. `code` and `state` are required.",
}

export const getErrorMessage = (error: Error | MatrixError): string | ReactNode => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MatrixError is an Error, right? so this is equivalent to:

Suggested change
export const getErrorMessage = (error: Error | MatrixError): string | ReactNode => {
export const getErrorMessage = (error: Error): string | ReactNode => {

switch (error.message) {
case OidcError.MissingOrInvalidStoredState:
return _t(
"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.",
);
case OidcClientError.InvalidQueryParameters:
case OidcError.CodeExchangeFailed:
case OidcError.InvalidBearerTokenResponse:
case OidcError.InvalidIdToken:
richvdh marked this conversation as resolved.
Show resolved Hide resolved
default:
return _t("Something went wrong during authentication. Go to the sign in page and try again.");
}
};
30 changes: 26 additions & 4 deletions test/components/structures/MatrixChat-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
mockClientMethodsUser,
} from "../../test-utils";
import * as leaveRoomUtils from "../../../src/utils/leave-behaviour";
import { OidcClientError } from "../../../src/utils/oidc/error";

jest.mock("matrix-js-sdk/src/oidc/authorize", () => ({
completeAuthorizationCodeGrant: jest.fn(),
Expand Down Expand Up @@ -762,10 +763,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();
};
Expand Down Expand Up @@ -811,7 +815,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();
Expand Down Expand Up @@ -866,6 +870,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 });

Expand Down
5 changes: 2 additions & 3 deletions test/utils/oidc/authorize-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.mock("matrix-js-sdk/src/oidc/authorize", () => ({
...jest.requireActual("matrix-js-sdk/src/oidc/authorize"),
Expand Down Expand Up @@ -114,9 +115,7 @@ describe("OIDC authorization", () => {
});

it("should throw when query params do not include state and code", async () => {
expect(async () => await completeOidcLogin({})).rejects.toThrow(
"Invalid query parameters for OIDC native login. `code` and `state` are required.",
);
expect(async () => await completeOidcLogin({})).rejects.toThrow(OidcClientError.InvalidQueryParameters);
});

it("should make request complete authorization code grant", async () => {
Expand Down