From c2a78d146b6726bc23ae34276e73a8b9fd272370 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 7 Jun 2023 17:23:58 +1200 Subject: [PATCH 01/26] add delegatedauthentication to validated server config --- src/utils/AutoDiscoveryUtils.tsx | 17 ++++++++++ src/utils/ValidatedServerConfig.ts | 5 +++ test/utils/AutoDiscoveryUtils-test.tsx | 46 ++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) diff --git a/src/utils/AutoDiscoveryUtils.tsx b/src/utils/AutoDiscoveryUtils.tsx index aaa602abb40..5f1a0448477 100644 --- a/src/utils/AutoDiscoveryUtils.tsx +++ b/src/utils/AutoDiscoveryUtils.tsx @@ -16,8 +16,10 @@ limitations under the License. import React, { ReactNode } from "react"; import { AutoDiscovery, ClientConfig } from "matrix-js-sdk/src/autodiscovery"; +import { IDelegatedAuthConfig, M_AUTHENTICATION } from "matrix-js-sdk/src/client"; import { logger } from "matrix-js-sdk/src/logger"; import { IClientWellKnown } from "matrix-js-sdk/src/matrix"; +import { ValidatedIssuerConfig } from "matrix-js-sdk/src/oidc/validate"; import { _t, UserFriendlyError } from "../languageHandler"; import SdkConfig from "../SdkConfig"; @@ -260,6 +262,20 @@ export default class AutoDiscoveryUtils { throw new UserFriendlyError("Unexpected error resolving homeserver configuration"); } + let delegatedAuthentication = undefined; + if (discoveryResult[M_AUTHENTICATION.stable!]?.state === AutoDiscovery.SUCCESS) { + const { authorizationEndpoint, registrationEndpoint, tokenEndpoint, account, issuer } = discoveryResult[ + M_AUTHENTICATION.stable! + ] as IDelegatedAuthConfig & ValidatedIssuerConfig; + delegatedAuthentication = { + authorizationEndpoint, + registrationEndpoint, + tokenEndpoint, + account, + issuer, + }; + } + return { hsUrl: preferredHomeserverUrl, hsName: preferredHomeserverName, @@ -268,6 +284,7 @@ export default class AutoDiscoveryUtils { isDefault: false, warning: hsResult.error, isNameResolvable: !isSynthetic, + delegatedAuthentication, } as ValidatedServerConfig; } } diff --git a/src/utils/ValidatedServerConfig.ts b/src/utils/ValidatedServerConfig.ts index bac271eef6a..4b58b1ef909 100644 --- a/src/utils/ValidatedServerConfig.ts +++ b/src/utils/ValidatedServerConfig.ts @@ -14,6 +14,9 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { IDelegatedAuthConfig } from "matrix-js-sdk/src/client"; +import { ValidatedIssuerConfig } from "matrix-js-sdk/src/oidc/validate"; + export interface ValidatedServerConfig { hsUrl: string; hsName: string; @@ -26,4 +29,6 @@ export interface ValidatedServerConfig { isNameResolvable: boolean; warning: string | Error; + + delegatedAuthentication?: IDelegatedAuthConfig & ValidatedIssuerConfig; } diff --git a/test/utils/AutoDiscoveryUtils-test.tsx b/test/utils/AutoDiscoveryUtils-test.tsx index 7282f9a8241..a47532179c3 100644 --- a/test/utils/AutoDiscoveryUtils-test.tsx +++ b/test/utils/AutoDiscoveryUtils-test.tsx @@ -16,6 +16,7 @@ limitations under the License. import { AutoDiscovery, AutoDiscoveryAction, ClientConfig } from "matrix-js-sdk/src/autodiscovery"; import { logger } from "matrix-js-sdk/src/logger"; +import { M_AUTHENTICATION } from "matrix-js-sdk/src/client"; import AutoDiscoveryUtils from "../../src/utils/AutoDiscoveryUtils"; @@ -186,5 +187,50 @@ describe("AutoDiscoveryUtils", () => { warning: "Homeserver URL does not appear to be a valid Matrix homeserver", }); }); + + it("ignores delegated auth config when discovery was not successful", () => { + const discoveryResult = { + ...validIsConfig, + ...validHsConfig, + [M_AUTHENTICATION.stable!]: { + state: AutoDiscoveryAction.FAIL_ERROR, + error: "", + }, + }; + const syntaxOnly = true; + expect( + AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult, syntaxOnly), + ).toEqual({ + ...expectedValidatedConfig, + delegatedAuthentication: undefined, + warning: undefined, + }); + }); + + it("sets delegated auth config when discovery was successful", () => { + const authConfig = { + issuer: "https://test.com/", + authorizationEndpoint: "https://test.com/auth", + registrationEndpoint: "https://test.com/registration", + tokenEndpoint: "https://test.com/token", + }; + const discoveryResult = { + ...validIsConfig, + ...validHsConfig, + [M_AUTHENTICATION.stable!]: { + state: AutoDiscoveryAction.SUCCESS, + error: null, + ...authConfig, + }, + }; + const syntaxOnly = true; + expect( + AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult, syntaxOnly), + ).toEqual({ + ...expectedValidatedConfig, + delegatedAuthentication: authConfig, + warning: undefined, + }); + }); }); }); From f946b8588baee5125d317f9d6b10b3d71a524000 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 9 Jun 2023 18:03:17 +1200 Subject: [PATCH 02/26] dynamic client registration functions --- src/utils/oidc/registerClient.ts | 97 ++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 src/utils/oidc/registerClient.ts diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts new file mode 100644 index 00000000000..b88ff4ba846 --- /dev/null +++ b/src/utils/oidc/registerClient.ts @@ -0,0 +1,97 @@ +/* +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 { IDelegatedAuthConfig } from "matrix-js-sdk/src/client"; +import { Method } from "matrix-js-sdk/src/http-api"; +import { ValidatedIssuerConfig } from "matrix-js-sdk/src/oidc/validate"; + +// "staticOidcClients": { +// "https://dev-6525741.okta.com/": { +// "client_id": "0oa5x44w64wpNsxi45d7" +// }, +// "https://keycloak-oidc.lab.element.dev/realms/master/": { +// "client_id": "hydrogen-oidc-playground" +// }, +// "https://id.thirdroom.io/realms/thirdroom/": { +// "client_id": "hydrogen-oidc-playground" +// } +// } + +export type OidcRegistrationClientMetadata = { + clientName: string; + clientUri: string; + redirectUris: string[]; +}; + +/** + * + * @param issuer issue URL + * @param clientMetadata registration metadata + * @returns {Promise} resolves to the registered client id when registration is successful + * @throws when registration fails, or issue does not support dynamic registration + */ +const doRegistration = async ( + registrationEndpoint: string, + clientMetadata: OidcRegistrationClientMetadata, +): Promise => { + // https://openid.net/specs/openid-connect-registration-1_0.html + const metadata = { + client_name: clientMetadata.clientName, + client_uri: clientMetadata.clientUri, + response_types: ["code"], + grant_types: ["authorization_code", "refresh_token"], + redirect_uris: clientMetadata.redirectUris, + id_token_signed_response_alg: "RS256", + token_endpoint_auth_method: "none", + application_type: "web", + }; + const headers = { + "Accept": "application/json", + "Content-Type": "application/json", + }; + const response = await fetch(registrationEndpoint, { + method: Method.Post, + headers, + body: JSON.stringify(metadata), + }); + + if (response.status >= 400) { + throw new Error("Failed to register client"); + } + + const body = await response.json(); + const clientId = body["client_id"]; + if (!clientId) { + throw new Error("Invalid clientId"); + } + + return clientId; +}; + +export const registerOidcClient = async ( + delegatedAuthConfig: IDelegatedAuthConfig & ValidatedIssuerConfig, + clientName: string, + baseUrl: string, +): Promise => { + const clientMetadata = { + clientName, + clientUri: baseUrl, + redirectUris: [baseUrl], + }; + const clientId = await doRegistration(delegatedAuthConfig.registrationEndpoint, clientMetadata); + + return clientId; +}; From 42017250d9446fae5ad58cd0427d7b331e935966 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 12 Jun 2023 13:40:09 +1200 Subject: [PATCH 03/26] test OP registration functions --- src/utils/oidc/error.ts | 25 +++++ src/utils/oidc/registerClient.ts | 57 +++++++++++- test/utils/oidc/registerClient-test.ts | 121 +++++++++++++++++++++++++ 3 files changed, 199 insertions(+), 4 deletions(-) create mode 100644 src/utils/oidc/error.ts create mode 100644 test/utils/oidc/registerClient-test.ts diff --git a/src/utils/oidc/error.ts b/src/utils/oidc/error.ts new file mode 100644 index 00000000000..ea5fa46c1d3 --- /dev/null +++ b/src/utils/oidc/error.ts @@ -0,0 +1,25 @@ +/* +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 { OidcDiscoveryError } from "matrix-js-sdk/src/oidc/validate"; + +export enum OidcClientError { + DynamicRegistrationNotSupported = "Dynamic registration not supported", + DynamicRegistrationFailed = "Dynamic registration failed", + DynamicRegistrationInvalid = "Dynamic registration invalid response", +} + +export type OidcError = OidcClientError | OidcDiscoveryError; diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index b88ff4ba846..85973070bba 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -16,8 +16,11 @@ limitations under the License. import { IDelegatedAuthConfig } from "matrix-js-sdk/src/client"; import { Method } from "matrix-js-sdk/src/http-api"; +import { logger } from "matrix-js-sdk/src/logger"; import { ValidatedIssuerConfig } from "matrix-js-sdk/src/oidc/validate"; +import { OidcClientError } from "./error"; + // "staticOidcClients": { // "https://dev-6525741.okta.com/": { // "client_id": "0oa5x44w64wpNsxi45d7" @@ -37,7 +40,7 @@ export type OidcRegistrationClientMetadata = { }; /** - * + * Make the registration request * @param issuer issue URL * @param clientMetadata registration metadata * @returns {Promise} resolves to the registered client id when registration is successful @@ -69,19 +72,27 @@ const doRegistration = async ( }); if (response.status >= 400) { - throw new Error("Failed to register client"); + throw new Error(OidcClientError.DynamicRegistrationFailed); } const body = await response.json(); const clientId = body["client_id"]; if (!clientId) { - throw new Error("Invalid clientId"); + throw new Error(OidcClientError.DynamicRegistrationInvalid); } return clientId; }; -export const registerOidcClient = async ( +/** + * Attempts dynamic registration against the configured registration endpoint + * @param delegatedAuthConfig Auth config from ValidatedServerConfig + * @param clientName Client name to register with the OP, eg 'Element' + * @param baseUrl URL of the home page of the Client, eg 'https://app.element.io/' + * @returns Promise resolved with registered clientId + * @throws on failed request or invalid response + */ +const registerOidcClient = async ( delegatedAuthConfig: IDelegatedAuthConfig & ValidatedIssuerConfig, clientName: string, baseUrl: string, @@ -95,3 +106,41 @@ export const registerOidcClient = async ( return clientId; }; + +/** + * Get the configured clientId for the issuer + * @param issuer delegated auth OIDC issuer + * @param staticOidcClients static client config from config.json + * @returns clientId if found, otherwise undefined + */ +const getStaticOidcClientId = (issuer: string, staticOidcClients?: Record): string | undefined => { + return staticOidcClients?.[issuer]; +}; + +/** + * Get the clientId for configured oidc OP + * Checks statically configured clientIds first + * Then attempts dynamic registration with the OP + * @param delegatedAuthConfig Auth config from ValidatedServerConfig + * @param clientName Client name to register with the OP, eg 'Element' + * @param baseUrl URL of the home page of the Client, eg 'https://app.element.io/' + * @param staticOidcClients static client config from config.json + * @returns Promise resolves with clientId + * @throws if no clientId is found or registration failed + */ +export const getOidcClientId = async ( + delegatedAuthConfig: IDelegatedAuthConfig & ValidatedIssuerConfig, + clientName: string, + baseUrl: string, + staticOidcClients?: Record, +): Promise => { + const staticClientId = getStaticOidcClientId(delegatedAuthConfig.issuer, staticOidcClients); + if (staticClientId) { + logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.issuer}`); + return staticClientId; + } + if (!delegatedAuthConfig.registrationEndpoint) { + throw new Error(OidcClientError.DynamicRegistrationNotSupported); + } + return await registerOidcClient(delegatedAuthConfig, clientName, baseUrl); +}; diff --git a/test/utils/oidc/registerClient-test.ts b/test/utils/oidc/registerClient-test.ts new file mode 100644 index 00000000000..b5cb3d11bdb --- /dev/null +++ b/test/utils/oidc/registerClient-test.ts @@ -0,0 +1,121 @@ +/* +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 fetchMockJest from "fetch-mock-jest"; + +import { OidcClientError } from "../../../src/utils/oidc/error"; +import { getOidcClientId } from "../../../src/utils/oidc/registerClient"; + +describe("getOidcClientId()", () => { + const issuer = "https://auth.com/"; + const registrationEndpoint = "https://auth.com/register"; + const clientName = "Element"; + const baseUrl = "https://just.testing"; + const dynamicClientId = "xyz789"; + const staticOidcClients = { + [issuer]: "abc123", + }; + const delegatedAuthConfig = { + issuer, + registrationEndpoint, + authorizationEndpoint: issuer + "auth", + tokenEndpoint: issuer + "token", + }; + + beforeEach(() => { + fetchMockJest.mockClear(); + fetchMockJest.resetBehavior(); + }); + + it("should return static clientId when configured", async () => { + expect(await getOidcClientId(delegatedAuthConfig, clientName, baseUrl, staticOidcClients)).toEqual( + staticOidcClients[issuer], + ); + // didn't try to register + expect(fetchMockJest).toHaveFetchedTimes(0); + }); + + it("should throw when no static clientId is configured and no registration endpoint", async () => { + const authConfigWithoutRegistration = { + ...delegatedAuthConfig, + issuer: "https://issuerWithoutStaticClientId.org/", + registrationEndpoint: undefined, + }; + expect( + async () => await getOidcClientId(authConfigWithoutRegistration, clientName, baseUrl, staticOidcClients), + ).rejects.toThrow(OidcClientError.DynamicRegistrationNotSupported); + // didn't try to register + expect(fetchMockJest).toHaveFetchedTimes(0); + }); + + it("should handle when staticOidcClients object is falsy", async () => { + const authConfigWithoutRegistration = { + ...delegatedAuthConfig, + registrationEndpoint: undefined, + }; + expect(async () => await getOidcClientId(authConfigWithoutRegistration, clientName, baseUrl)).rejects.toThrow( + OidcClientError.DynamicRegistrationNotSupported, + ); + // didn't try to register + expect(fetchMockJest).toHaveFetchedTimes(0); + }); + + it("should make correct request to register client", async () => { + fetchMockJest.post(registrationEndpoint, { + status: 200, + body: JSON.stringify({ client_id: dynamicClientId }), + }); + expect(await getOidcClientId(delegatedAuthConfig, clientName, baseUrl)).toEqual(dynamicClientId); + // didn't try to register + expect(fetchMockJest).toHaveBeenCalledWith(registrationEndpoint, { + headers: { + "Accept": "application/json", + "Content-Type": "application/json", + }, + method: "POST", + body: JSON.stringify({ + client_name: clientName, + client_uri: baseUrl, + response_types: ["code"], + grant_types: ["authorization_code", "refresh_token"], + redirect_uris: [baseUrl], + id_token_signed_response_alg: "RS256", + token_endpoint_auth_method: "none", + application_type: "web", + }), + }); + }); + + it("should throw when registration request fails", async () => { + fetchMockJest.post(registrationEndpoint, { + status: 500, + }); + expect(() => getOidcClientId(delegatedAuthConfig, clientName, baseUrl)).rejects.toThrow( + OidcClientError.DynamicRegistrationFailed, + ); + }); + + it("should throw when registration response is invalid", async () => { + fetchMockJest.post(registrationEndpoint, { + status: 200, + // no clientId in response + body: "{}", + }); + expect(() => getOidcClientId(delegatedAuthConfig, clientName, baseUrl)).rejects.toThrow( + OidcClientError.DynamicRegistrationInvalid, + ); + }); +}); From 314105d1d05f060644cc19f2e10b692293b15224 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 12 Jun 2023 15:05:04 +1200 Subject: [PATCH 04/26] add stubbed nativeOidc flow setup in Login --- src/IConfigOptions.ts | 2 + src/components/structures/auth/Login.tsx | 104 ++++++++++++++---- .../components/structures/auth/Login-test.tsx | 1 + 3 files changed, 84 insertions(+), 23 deletions(-) diff --git a/src/IConfigOptions.ts b/src/IConfigOptions.ts index 1fa873ec87a..5753c6e85da 100644 --- a/src/IConfigOptions.ts +++ b/src/IConfigOptions.ts @@ -192,6 +192,8 @@ export interface IConfigOptions { existing_issues_url: string; new_issue_url: string; }; + + oidc_static_clients?: Record; } export interface ISsoRedirectOptions { diff --git a/src/components/structures/auth/Login.tsx b/src/components/structures/auth/Login.tsx index 3299cae3fc3..2061facbd73 100644 --- a/src/components/structures/auth/Login.tsx +++ b/src/components/structures/auth/Login.tsx @@ -38,6 +38,11 @@ import AuthHeader from "../../views/auth/AuthHeader"; import AccessibleButton, { ButtonEvent } from "../../views/elements/AccessibleButton"; import { ValidatedServerConfig } from "../../../utils/ValidatedServerConfig"; import { filterBoolean } from "../../../utils/arrays"; +import { Features } from "../../../settings/Settings"; +import { getOidcClientId } from "../../../utils/oidc/registerClient"; +import SdkConfig from "../../../SdkConfig"; +import { IConfigOptions } from "../../../IConfigOptions"; +import { OidcClientError } from "../../../utils/oidc/error"; // These are used in several places, and come from the js-sdk's autodiscovery // stuff. We define them here so that they'll be picked up by i18n. @@ -98,6 +103,9 @@ interface IState { serverIsAlive: boolean; serverErrorIsFatal: boolean; serverDeadError?: ReactNode; + + // @TODO(kerrya) does this need to be in state? + oidcClientId?: string; } type OnPasswordLogin = { @@ -110,6 +118,7 @@ type OnPasswordLogin = { */ export default class LoginComponent extends React.PureComponent { private unmounted = false; + private oidcNativeFlowEnabled = false; private loginLogic!: Login; private readonly stepRendererMap: Record ReactNode>; @@ -117,6 +126,9 @@ export default class LoginComponent extends React.PureComponent public constructor(props: IProps) { super(props); + // only set on a config level, so we don't need to watch + this.oidcNativeFlowEnabled = SettingsStore.getValue(Features.OidcNativeFlow); + this.state = { busy: false, errorText: null, @@ -156,7 +168,8 @@ export default class LoginComponent extends React.PureComponent public componentDidUpdate(prevProps: IProps): void { if ( prevProps.serverConfig.hsUrl !== this.props.serverConfig.hsUrl || - prevProps.serverConfig.isUrl !== this.props.serverConfig.isUrl + prevProps.serverConfig.isUrl !== this.props.serverConfig.isUrl || + prevProps.serverConfig.delegatedAuthentication !== this.props.serverConfig.delegatedAuthentication ) { // Ensure that we end up actually logging in to the right place this.initLoginLogic(this.props.serverConfig); @@ -322,28 +335,7 @@ export default class LoginComponent extends React.PureComponent } }; - private async initLoginLogic({ hsUrl, isUrl }: ValidatedServerConfig): Promise { - let isDefaultServer = false; - if ( - this.props.serverConfig.isDefault && - hsUrl === this.props.serverConfig.hsUrl && - isUrl === this.props.serverConfig.isUrl - ) { - isDefaultServer = true; - } - - const fallbackHsUrl = isDefaultServer ? this.props.fallbackHsUrl! : null; - - const loginLogic = new Login(hsUrl, isUrl, fallbackHsUrl, { - defaultDeviceDisplayName: this.props.defaultDeviceDisplayName, - }); - this.loginLogic = loginLogic; - - this.setState({ - busy: true, - loginIncorrect: false, - }); - + private async checkServerLiveliness({ hsUrl, isUrl }: Pick): Promise { // Do a quick liveliness check on the URLs try { const { warning } = await AutoDiscoveryUtils.validateServerConfigWithStaticUrls(hsUrl, isUrl); @@ -364,6 +356,70 @@ export default class LoginComponent extends React.PureComponent ...AutoDiscoveryUtils.authComponentStateForError(e), }); } + } + + private async tryInitOidcNativeFlow(): Promise { + if (!this.props.serverConfig.delegatedAuthentication || !this.oidcNativeFlowEnabled) { + return false; + } + try { + const clientId = await getOidcClientId( + this.props.serverConfig.delegatedAuthentication, + SdkConfig.get().brand, + window.location.origin, + SdkConfig.get().oidc_static_clients, + ); + + if (!this.isSupportedFlow('oidcNativeFlow')) { + throw new Error('OIDC native login flow configured but UI not yet supported.'); + } + + this.setState({ + busy: false, + oidcClientId: clientId, + flows: ['oidcNativeFlow'] + }); + return true; + } catch (error) { + console.error(error); + return false; + } + } + + private async initLoginLogic({ hsUrl, isUrl }: ValidatedServerConfig): Promise { + let isDefaultServer = false; + if ( + this.props.serverConfig.isDefault && + hsUrl === this.props.serverConfig.hsUrl && + isUrl === this.props.serverConfig.isUrl + ) { + isDefaultServer = true; + } + + const fallbackHsUrl = isDefaultServer ? this.props.fallbackHsUrl! : null; + + this.setState({ + busy: true, + loginIncorrect: false, + oidcClientId: undefined, + }); + + await this.checkServerLiveliness({ hsUrl, isUrl }); + + const useOidcNativeFlow = await this.tryInitOidcNativeFlow(); + + console.log('hhh initLoginLogic', { useOidcNativeFlow }); + + // don't continue with setup + // as we are using oidc native flow + if (useOidcNativeFlow) { + return; + } + + const loginLogic = new Login(hsUrl, isUrl, fallbackHsUrl, { + defaultDeviceDisplayName: this.props.defaultDeviceDisplayName, + }); + this.loginLogic = loginLogic; loginLogic .getFlows() @@ -473,6 +529,8 @@ export default class LoginComponent extends React.PureComponent const errorText = this.state.errorText; + console.log('hhh render()', this.state); + let errorTextSection; if (errorText) { errorTextSection =
{errorText}
; diff --git a/test/components/structures/auth/Login-test.tsx b/test/components/structures/auth/Login-test.tsx index a84e88b17c1..3accb5772bd 100644 --- a/test/components/structures/auth/Login-test.tsx +++ b/test/components/structures/auth/Login-test.tsx @@ -189,6 +189,7 @@ describe("Login", function () { versions: [], }); rerender(getRawComponent("https://server2")); + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); fireEvent.click(container.querySelector(".mx_SSOButton")!); expect(platform.startSingleSignOn.mock.calls[1][0].baseUrl).toBe("https://server2"); From 6fe2d354cb0b6546223c628313b5993327752059 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 12 Jun 2023 15:39:26 +1200 Subject: [PATCH 05/26] cover more error cases in Login --- .../components/structures/auth/Login-test.tsx | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/test/components/structures/auth/Login-test.tsx b/test/components/structures/auth/Login-test.tsx index a84e88b17c1..8eb25d49097 100644 --- a/test/components/structures/auth/Login-test.tsx +++ b/test/components/structures/auth/Login-test.tsx @@ -1,5 +1,5 @@ /* -Copyright 2019 New Vector Ltd +Copyright 2019, 2023 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -50,6 +50,7 @@ describe("Login", function () { mockClient.baseUrl = opts.baseUrl; return mockClient; }); + fetchMock.resetBehavior(); fetchMock.get("https://matrix.org/_matrix/client/versions", { unstable_features: {}, versions: [], @@ -253,4 +254,69 @@ describe("Login", function () { const ssoButtons = container.querySelectorAll(".mx_SSOButton"); expect(ssoButtons.length).toBe(idpsWithIcons.length + 1); }); + + it("should display an error when homeserver doesn't offer any supported login flows", async () => { + mockClient.loginFlows.mockResolvedValue({ + flows: [ + { + type: "just something weird", + }, + ], + }); + + getComponent(); + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + expect( + screen.getByText("This homeserver doesn't offer any login flows which are supported by this client."), + ).toBeInTheDocument(); + }); + + it("should display a connection error when getting login flows fails", async () => { + mockClient.loginFlows.mockRejectedValue("oups"); + + getComponent(); + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + expect( + screen.getByText("There was a problem communicating with the homeserver, please try again later."), + ).toBeInTheDocument(); + }); + + it("should display an error when homeserver fails liveliness check", async () => { + fetchMock.resetBehavior(); + fetchMock.get("https://matrix.org/_matrix/client/versions", { + status: 400, + }); + getComponent(); + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + // error displayed + expect(screen.getByText("Your test-brand is misconfigured")).toBeInTheDocument(); + }); + + it("should reset liveliness error when server config changes", async () => { + fetchMock.resetBehavior(); + // matrix.org is not alive + fetchMock.get("https://matrix.org/_matrix/client/versions", { + status: 400, + }); + // but server2 is + fetchMock.get("https://server2/_matrix/client/versions", { + unstable_features: {}, + versions: [], + }); + const { rerender } = render(getRawComponent()); + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + // error displayed + expect(screen.getByText("Your test-brand is misconfigured")).toBeInTheDocument(); + + rerender(getRawComponent("https://server2")); + + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + // error cleared + expect(screen.queryByText("Your test-brand is misconfigured")).not.toBeInTheDocument(); + }); }); From 19dc425de2b3d06d711eeea2a42f68cb901cc9ba Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 12 Jun 2023 15:49:19 +1200 Subject: [PATCH 06/26] tidy --- src/components/structures/auth/Login.tsx | 17 +++++++++-------- src/utils/oidc/registerClient.ts | 12 ------------ 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/src/components/structures/auth/Login.tsx b/src/components/structures/auth/Login.tsx index 2061facbd73..08716f73c88 100644 --- a/src/components/structures/auth/Login.tsx +++ b/src/components/structures/auth/Login.tsx @@ -41,8 +41,6 @@ import { filterBoolean } from "../../../utils/arrays"; import { Features } from "../../../settings/Settings"; import { getOidcClientId } from "../../../utils/oidc/registerClient"; import SdkConfig from "../../../SdkConfig"; -import { IConfigOptions } from "../../../IConfigOptions"; -import { OidcClientError } from "../../../utils/oidc/error"; // These are used in several places, and come from the js-sdk's autodiscovery // stuff. We define them here so that they'll be picked up by i18n. @@ -335,7 +333,10 @@ export default class LoginComponent extends React.PureComponent } }; - private async checkServerLiveliness({ hsUrl, isUrl }: Pick): Promise { + private async checkServerLiveliness({ + hsUrl, + isUrl, + }: Pick): Promise { // Do a quick liveliness check on the URLs try { const { warning } = await AutoDiscoveryUtils.validateServerConfigWithStaticUrls(hsUrl, isUrl); @@ -370,14 +371,14 @@ export default class LoginComponent extends React.PureComponent SdkConfig.get().oidc_static_clients, ); - if (!this.isSupportedFlow('oidcNativeFlow')) { - throw new Error('OIDC native login flow configured but UI not yet supported.'); + if (!this.isSupportedFlow("oidcNativeFlow")) { + throw new Error("OIDC native login flow configured but UI not yet supported."); } this.setState({ busy: false, oidcClientId: clientId, - flows: ['oidcNativeFlow'] + flows: ["oidcNativeFlow"], }); return true; } catch (error) { @@ -408,7 +409,7 @@ export default class LoginComponent extends React.PureComponent const useOidcNativeFlow = await this.tryInitOidcNativeFlow(); - console.log('hhh initLoginLogic', { useOidcNativeFlow }); + console.log("hhh initLoginLogic", { useOidcNativeFlow }); // don't continue with setup // as we are using oidc native flow @@ -529,7 +530,7 @@ export default class LoginComponent extends React.PureComponent const errorText = this.state.errorText; - console.log('hhh render()', this.state); + console.log("hhh render()", this.state); let errorTextSection; if (errorText) { diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index 85973070bba..3f5dc5265f5 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -21,18 +21,6 @@ import { ValidatedIssuerConfig } from "matrix-js-sdk/src/oidc/validate"; import { OidcClientError } from "./error"; -// "staticOidcClients": { -// "https://dev-6525741.okta.com/": { -// "client_id": "0oa5x44w64wpNsxi45d7" -// }, -// "https://keycloak-oidc.lab.element.dev/realms/master/": { -// "client_id": "hydrogen-oidc-playground" -// }, -// "https://id.thirdroom.io/realms/thirdroom/": { -// "client_id": "hydrogen-oidc-playground" -// } -// } - export type OidcRegistrationClientMetadata = { clientName: string; clientUri: string; From 94d5c369baf9f1ad86f98704d7cdcece3d71b530 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 12 Jun 2023 18:13:14 +1200 Subject: [PATCH 07/26] test dynamic client registration in Login --- src/components/structures/auth/Login.tsx | 31 ++--- src/utils/oidc/registerClient.ts | 38 +++--- .../components/structures/auth/Login-test.tsx | 114 +++++++++++++++++- test/test-utils/test-utils.ts | 7 +- 4 files changed, 155 insertions(+), 35 deletions(-) diff --git a/src/components/structures/auth/Login.tsx b/src/components/structures/auth/Login.tsx index 08716f73c88..23486187d2d 100644 --- a/src/components/structures/auth/Login.tsx +++ b/src/components/structures/auth/Login.tsx @@ -17,7 +17,7 @@ limitations under the License. import React, { ReactNode } from "react"; import classNames from "classnames"; import { logger } from "matrix-js-sdk/src/logger"; -import { ISSOFlow, LoginFlow, SSOAction } from "matrix-js-sdk/src/@types/auth"; +import { ILoginFlow, ISSOFlow, LoginFlow, SSOAction } from "matrix-js-sdk/src/@types/auth"; import { _t, _td, UserFriendlyError } from "../../../languageHandler"; import Login from "../../../Login"; @@ -52,6 +52,11 @@ _td("Invalid identity server discovery response"); _td("Invalid base_url for m.identity_server"); _td("Identity server URL does not appear to be a valid identity server"); _td("General failure"); +export interface OidcNativeFlow extends ILoginFlow { + type: "oidcNativeFlow"; + clientId: string; +} +type ClientLoginFlow = LoginFlow | OidcNativeFlow; interface IProps { serverConfig: ValidatedServerConfig; @@ -87,7 +92,7 @@ interface IState { // can we attempt to log in or are there validation errors? canTryLogin: boolean; - flows?: LoginFlow[]; + flows?: ClientLoginFlow[]; // used for preserving form values when changing homeserver username: string; @@ -101,9 +106,6 @@ interface IState { serverIsAlive: boolean; serverErrorIsFatal: boolean; serverDeadError?: ReactNode; - - // @TODO(kerrya) does this need to be in state? - oidcClientId?: string; } type OnPasswordLogin = { @@ -371,18 +373,22 @@ export default class LoginComponent extends React.PureComponent SdkConfig.get().oidc_static_clients, ); - if (!this.isSupportedFlow("oidcNativeFlow")) { + const flow = { + type: "oidcNativeFlow", + clientId, + }; + + if (!this.isSupportedFlow(flow)) { throw new Error("OIDC native login flow configured but UI not yet supported."); } this.setState({ busy: false, - oidcClientId: clientId, - flows: ["oidcNativeFlow"], + flows: [flow], }); return true; } catch (error) { - console.error(error); + logger.error(error); return false; } } @@ -402,15 +408,12 @@ export default class LoginComponent extends React.PureComponent this.setState({ busy: true, loginIncorrect: false, - oidcClientId: undefined, }); await this.checkServerLiveliness({ hsUrl, isUrl }); const useOidcNativeFlow = await this.tryInitOidcNativeFlow(); - console.log("hhh initLoginLogic", { useOidcNativeFlow }); - // don't continue with setup // as we are using oidc native flow if (useOidcNativeFlow) { @@ -458,7 +461,7 @@ export default class LoginComponent extends React.PureComponent }); } - private isSupportedFlow = (flow: LoginFlow): boolean => { + private isSupportedFlow = (flow: ClientLoginFlow): boolean => { // technically the flow can have multiple steps, but no one does this // for login and loginLogic doesn't support it so we can ignore it. if (!this.stepRendererMap[flow.type]) { @@ -530,8 +533,6 @@ export default class LoginComponent extends React.PureComponent const errorText = this.state.errorText; - console.log("hhh render()", this.state); - let errorTextSection; if (errorText) { errorTextSection =
{errorText}
; diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index 3f5dc5265f5..9279d043053 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -53,23 +53,33 @@ const doRegistration = async ( "Accept": "application/json", "Content-Type": "application/json", }; - const response = await fetch(registrationEndpoint, { - method: Method.Post, - headers, - body: JSON.stringify(metadata), - }); - if (response.status >= 400) { - throw new Error(OidcClientError.DynamicRegistrationFailed); - } + try { + const response = await fetch(registrationEndpoint, { + method: Method.Post, + headers, + body: JSON.stringify(metadata), + }); - const body = await response.json(); - const clientId = body["client_id"]; - if (!clientId) { - throw new Error(OidcClientError.DynamicRegistrationInvalid); - } + if (response.status >= 400) { + throw new Error(OidcClientError.DynamicRegistrationFailed); + } - return clientId; + const body = await response.json(); + const clientId = body["client_id"]; + if (!clientId) { + throw new Error(OidcClientError.DynamicRegistrationInvalid); + } + + return clientId; + } catch (error) { + if (Object.values(OidcClientError).includes(error.message)) { + throw error; + } else { + logger.error("Dynamic registration request failed", error); + throw new Error(OidcClientError.DynamicRegistrationFailed); + } + } }; /** diff --git a/test/components/structures/auth/Login-test.tsx b/test/components/structures/auth/Login-test.tsx index f5305434fbe..81294fb1d4d 100644 --- a/test/components/structures/auth/Login-test.tsx +++ b/test/components/structures/auth/Login-test.tsx @@ -17,19 +17,29 @@ limitations under the License. import React from "react"; import { fireEvent, render, screen, waitForElementToBeRemoved } from "@testing-library/react"; import { mocked, MockedObject } from "jest-mock"; -import { createClient, MatrixClient } from "matrix-js-sdk/src/matrix"; import fetchMock from "fetch-mock-jest"; import { DELEGATED_OIDC_COMPATIBILITY, IdentityProviderBrand } from "matrix-js-sdk/src/@types/auth"; +import { logger } from "matrix-js-sdk/src/logger"; +import { createClient, MatrixClient } from "matrix-js-sdk/src/matrix"; import SdkConfig from "../../../../src/SdkConfig"; import { mkServerConfig, mockPlatformPeg, unmockPlatformPeg } from "../../../test-utils"; import Login from "../../../../src/components/structures/auth/Login"; import BasePlatform from "../../../../src/BasePlatform"; +import SettingsStore from "../../../../src/settings/SettingsStore"; +import { Features } from "../../../../src/settings/Settings"; +import { ValidatedServerConfig } from "../../../../src/utils/ValidatedServerConfig"; +import * as registerClientUtils from "../../../../src/utils/oidc/registerClient"; +import { OidcClientError } from "../../../../src/utils/oidc/error"; jest.mock("matrix-js-sdk/src/matrix"); jest.useRealTimers(); +const oidcStaticClientsConfig = { + "https://staticallyregisteredissuer.org/": "static-clientId-123", +}; + describe("Login", function () { let platform: MockedObject; @@ -42,6 +52,7 @@ describe("Login", function () { SdkConfig.put({ brand: "test-brand", disable_custom_urls: true, + oidc_static_clients: oidcStaticClientsConfig, }); mockClient.login.mockClear().mockResolvedValue({}); mockClient.loginFlows.mockClear().mockResolvedValue({ flows: [{ type: "m.login.password" }] }); @@ -51,6 +62,7 @@ describe("Login", function () { return mockClient; }); fetchMock.resetBehavior(); + fetchMock.resetHistory(); fetchMock.get("https://matrix.org/_matrix/client/versions", { unstable_features: {}, versions: [], @@ -66,10 +78,14 @@ describe("Login", function () { unmockPlatformPeg(); }); - function getRawComponent(hsUrl = "https://matrix.org", isUrl = "https://vector.im") { + function getRawComponent( + hsUrl = "https://matrix.org", + isUrl = "https://vector.im", + delegatedAuthentication?: ValidatedServerConfig["delegatedAuthentication"], + ) { return ( {}} onRegisterClick={() => {}} onServerConfigChange={() => {}} @@ -77,8 +93,12 @@ describe("Login", function () { ); } - function getComponent(hsUrl?: string, isUrl?: string) { - return render(getRawComponent(hsUrl, isUrl)); + function getComponent( + hsUrl?: string, + isUrl?: string, + delegatedAuthentication?: ValidatedServerConfig["delegatedAuthentication"], + ) { + return render(getRawComponent(hsUrl, isUrl, delegatedAuthentication)); } it("should show form with change server link", async () => { @@ -320,4 +340,88 @@ describe("Login", function () { // error cleared expect(screen.queryByText("Your test-brand is misconfigured")).not.toBeInTheDocument(); }); + + describe("OIDC native flow", () => { + const hsUrl = "https://matrix.org"; + const isUrl = "https://vector.im"; + const issuer = "https://test.com/"; + const delegatedAuth = { + issuer, + registrationEndpoint: issuer + "register", + tokenEndpoint: issuer + "token", + authorizationEndpoint: issuer + "authorization", + }; + beforeEach(() => { + jest.spyOn(logger, "error").mockClear(); + jest.spyOn(SettingsStore, "getValue").mockImplementation( + (settingName) => settingName === Features.OidcNativeFlow, + ); + }); + + it("should not attempt registration when oidc native flow setting is disabled", async () => { + jest.spyOn(SettingsStore, "getValue").mockReturnValue(false); + + getComponent(hsUrl, isUrl, delegatedAuth); + + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + // didn't try to register + expect(fetchMock).not.toHaveBeenCalledWith(delegatedAuth.registrationEndpoint); + // continued with normal setup + expect(mockClient.loginFlows).toHaveBeenCalled(); + // normal password login rendered + expect(screen.getByLabelText("Username")).toBeInTheDocument(); + }); + + it("should attempt to register oidc client", async () => { + // dont mock, spy so we can check config values were correctly passed + jest.spyOn(registerClientUtils, "getOidcClientId"); + fetchMock.post(delegatedAuth.registrationEndpoint, { status: 500 }); + getComponent(hsUrl, isUrl, delegatedAuth); + + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + // tried to register + expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registrationEndpoint, expect.any(Object)); + // called with values from config + expect(registerClientUtils.getOidcClientId).toHaveBeenCalledWith( + delegatedAuth, + "test-brand", + "http://localhost", + oidcStaticClientsConfig, + ); + }); + + it("should fallback to normal login when client registration fails", async () => { + fetchMock.post(delegatedAuth.registrationEndpoint, { status: 500 }); + getComponent(hsUrl, isUrl, delegatedAuth); + + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + // tried to register + expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registrationEndpoint, expect.any(Object)); + expect(logger.error).toHaveBeenCalledWith(new Error(OidcClientError.DynamicRegistrationFailed)); + + // continued with normal setup + expect(mockClient.loginFlows).toHaveBeenCalled(); + // normal password login rendered + expect(screen.getByLabelText("Username")).toBeInTheDocument(); + }); + + it("should fallback to normal login while oidc native login is not supported in UI", async () => { + fetchMock.post(delegatedAuth.registrationEndpoint, { client_id: "abc123" }); + getComponent(hsUrl, isUrl, delegatedAuth); + + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + expect(logger.error).toHaveBeenCalledWith( + new Error("OIDC native login flow configured but UI not yet supported."), + ); + + // continued with normal setup + expect(mockClient.loginFlows).toHaveBeenCalled(); + // normal password login rendered + expect(screen.getByLabelText("Username")).toBeInTheDocument(); + }); + }); }); diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 6eea4114d23..a2ca4313407 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -617,12 +617,17 @@ export function mkStubRoom( } as unknown as Room; } -export function mkServerConfig(hsUrl: string, isUrl: string): ValidatedServerConfig { +export function mkServerConfig( + hsUrl: string, + isUrl: string, + delegatedAuthentication?: ValidatedServerConfig["delegatedAuthentication"], +): ValidatedServerConfig { return { hsUrl, hsName: "TEST_ENVIRONMENT", hsNameIsDifferent: false, // yes, we lie isUrl, + delegatedAuthentication, } as ValidatedServerConfig; } From 67f6c2ede6fe243a38dd033361cf5e0486c94153 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 12 Jun 2023 18:13:31 +1200 Subject: [PATCH 08/26] comment oidc_static_clients --- src/IConfigOptions.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/IConfigOptions.ts b/src/IConfigOptions.ts index 5753c6e85da..bf8ca1ee866 100644 --- a/src/IConfigOptions.ts +++ b/src/IConfigOptions.ts @@ -193,6 +193,12 @@ export interface IConfigOptions { new_issue_url: string; }; + /** + * Configuration for OIDC issuers where a static client_id has been issued for the app. + * Otherwise dynamic client registration is attempted. + * The issuer URL must have a trailing `/`. + * OPTIONAL + */ oidc_static_clients?: Record; } From 8df092968e95416ebbe058fcd34b4322af04d0cb Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 12 Jun 2023 18:49:37 +1200 Subject: [PATCH 09/26] register oidc inside Login.getFlows --- src/Login.ts | 64 ++++++++++++++++++- src/components/structures/auth/Login.tsx | 55 ++-------------- .../components/structures/auth/Login-test.tsx | 44 +++++++++++-- 3 files changed, 104 insertions(+), 59 deletions(-) diff --git a/src/Login.ts b/src/Login.ts index 569363eeb05..6426550eaa6 100644 --- a/src/Login.ts +++ b/src/Login.ts @@ -19,18 +19,32 @@ limitations under the License. import { createClient } from "matrix-js-sdk/src/matrix"; import { MatrixClient } from "matrix-js-sdk/src/client"; import { logger } from "matrix-js-sdk/src/logger"; -import { DELEGATED_OIDC_COMPATIBILITY, ILoginParams, LoginFlow } from "matrix-js-sdk/src/@types/auth"; +import { OidcDiscoveryError } from "matrix-js-sdk/src/oidc/validate"; +import { DELEGATED_OIDC_COMPATIBILITY, ILoginFlow, ILoginParams, LoginFlow } from "matrix-js-sdk/src/@types/auth"; import { IMatrixClientCreds } from "./MatrixClientPeg"; import SecurityCustomisations from "./customisations/Security"; +import { ValidatedServerConfig } from "./utils/ValidatedServerConfig"; +import { getOidcClientId } from "./utils/oidc/registerClient"; +import { IConfigOptions } from "./IConfigOptions"; +import SdkConfig from "./SdkConfig"; + +export type ClientLoginFlow = LoginFlow | OidcNativeFlow; interface ILoginOptions { defaultDeviceDisplayName?: string; + /** + * Delegated auth config from server config + * When prop is passed we will attempt to use delegated auth + * Labs flag should be checked in parent + */ + delegatedAuthentication?: ValidatedServerConfig["delegatedAuthentication"]; } export default class Login { - private flows: Array = []; + private flows: Array = []; private readonly defaultDeviceDisplayName?: string; + private readonly delegatedAuthentication?: ValidatedServerConfig["delegatedAuthentication"]; private tempClient: MatrixClient | null = null; // memoize public constructor( @@ -40,6 +54,7 @@ export default class Login { opts: ILoginOptions, ) { this.defaultDeviceDisplayName = opts.defaultDeviceDisplayName; + this.delegatedAuthentication = opts.delegatedAuthentication; } public getHomeserverUrl(): string { @@ -75,7 +90,20 @@ export default class Login { return this.tempClient; } - public async getFlows(): Promise> { + public async getFlows(): Promise> { + // try to use oidc native flow + try { + const oidcFlow = await tryInitOidcNativeFlow( + this.delegatedAuthentication, + SdkConfig.get().brand, + SdkConfig.get().oidc_static_clients, + ); + return [oidcFlow]; + } catch (error) { + logger.error(error); + } + + // oidc native flow not supported, continue with matrix login const client = this.createTemporaryClient(); const { flows }: { flows: LoginFlow[] } = await client.loginFlows(); // If an m.login.sso flow is present which is also flagged as being for MSC3824 OIDC compatibility then we only @@ -151,6 +179,36 @@ export default class Login { } } +export interface OidcNativeFlow extends ILoginFlow { + type: "oidcNativeFlow"; + clientId: string; +} +/** + * Finds static clientId for configured issuer, or attempts dynamic registration with the OP + * @param delegatedAuthConfig Auth config from ValidatedServerConfig + * @param clientName Client name to register with the OP, eg 'Element', used during client registration with OP + * @param staticOidcClients static client config from config.json, used during client registration with OP + * @returns Promise when oidc native authentication flow is supported and correctly configured + * @throws when oidc native flow is not configured, client can't register with OP, or any unexpected error + */ +const tryInitOidcNativeFlow = async ( + delegatedAuthConfig: ValidatedServerConfig["delegatedAuthentication"] | undefined, + brand: string, + oidcStaticClients?: IConfigOptions["oidc_static_clients"], +): Promise => { + if (!delegatedAuthConfig) { + throw new Error(OidcDiscoveryError.NotSupported); + } + const clientId = await getOidcClientId(delegatedAuthConfig, brand, window.location.origin, oidcStaticClients); + + const flow = { + type: "oidcNativeFlow", + clientId, + } as OidcNativeFlow; + + return flow; +}; + /** * Send a login request to the given server, and format the response * as a MatrixClientCreds diff --git a/src/components/structures/auth/Login.tsx b/src/components/structures/auth/Login.tsx index 23486187d2d..c6095294e55 100644 --- a/src/components/structures/auth/Login.tsx +++ b/src/components/structures/auth/Login.tsx @@ -17,10 +17,10 @@ limitations under the License. import React, { ReactNode } from "react"; import classNames from "classnames"; import { logger } from "matrix-js-sdk/src/logger"; -import { ILoginFlow, ISSOFlow, LoginFlow, SSOAction } from "matrix-js-sdk/src/@types/auth"; +import { ISSOFlow, SSOAction } from "matrix-js-sdk/src/@types/auth"; import { _t, _td, UserFriendlyError } from "../../../languageHandler"; -import Login from "../../../Login"; +import Login, { ClientLoginFlow } from "../../../Login"; import { messageForConnectionError, messageForLoginError } from "../../../utils/ErrorUtils"; import AutoDiscoveryUtils from "../../../utils/AutoDiscoveryUtils"; import AuthPage from "../../views/auth/AuthPage"; @@ -39,8 +39,6 @@ import AccessibleButton, { ButtonEvent } from "../../views/elements/AccessibleBu import { ValidatedServerConfig } from "../../../utils/ValidatedServerConfig"; import { filterBoolean } from "../../../utils/arrays"; import { Features } from "../../../settings/Settings"; -import { getOidcClientId } from "../../../utils/oidc/registerClient"; -import SdkConfig from "../../../SdkConfig"; // These are used in several places, and come from the js-sdk's autodiscovery // stuff. We define them here so that they'll be picked up by i18n. @@ -52,12 +50,6 @@ _td("Invalid identity server discovery response"); _td("Invalid base_url for m.identity_server"); _td("Identity server URL does not appear to be a valid identity server"); _td("General failure"); -export interface OidcNativeFlow extends ILoginFlow { - type: "oidcNativeFlow"; - clientId: string; -} -type ClientLoginFlow = LoginFlow | OidcNativeFlow; - interface IProps { serverConfig: ValidatedServerConfig; // If true, the component will consider itself busy. @@ -361,38 +353,6 @@ export default class LoginComponent extends React.PureComponent } } - private async tryInitOidcNativeFlow(): Promise { - if (!this.props.serverConfig.delegatedAuthentication || !this.oidcNativeFlowEnabled) { - return false; - } - try { - const clientId = await getOidcClientId( - this.props.serverConfig.delegatedAuthentication, - SdkConfig.get().brand, - window.location.origin, - SdkConfig.get().oidc_static_clients, - ); - - const flow = { - type: "oidcNativeFlow", - clientId, - }; - - if (!this.isSupportedFlow(flow)) { - throw new Error("OIDC native login flow configured but UI not yet supported."); - } - - this.setState({ - busy: false, - flows: [flow], - }); - return true; - } catch (error) { - logger.error(error); - return false; - } - } - private async initLoginLogic({ hsUrl, isUrl }: ValidatedServerConfig): Promise { let isDefaultServer = false; if ( @@ -412,16 +372,11 @@ export default class LoginComponent extends React.PureComponent await this.checkServerLiveliness({ hsUrl, isUrl }); - const useOidcNativeFlow = await this.tryInitOidcNativeFlow(); - - // don't continue with setup - // as we are using oidc native flow - if (useOidcNativeFlow) { - return; - } - const loginLogic = new Login(hsUrl, isUrl, fallbackHsUrl, { defaultDeviceDisplayName: this.props.defaultDeviceDisplayName, + delegatedAuthentication: this.oidcNativeFlowEnabled + ? this.props.serverConfig.delegatedAuthentication + : undefined, }); this.loginLogic = loginLogic; diff --git a/test/components/structures/auth/Login-test.tsx b/test/components/structures/auth/Login-test.tsx index 81294fb1d4d..9ae8d09db1e 100644 --- a/test/components/structures/auth/Login-test.tsx +++ b/test/components/structures/auth/Login-test.tsx @@ -408,20 +408,52 @@ describe("Login", function () { expect(screen.getByLabelText("Username")).toBeInTheDocument(); }); - it("should fallback to normal login while oidc native login is not supported in UI", async () => { + // short term during active development, UI will be added in next PRs + it("should show error when oidc native flow is correctly configured but not supported by UI", async () => { fetchMock.post(delegatedAuth.registrationEndpoint, { client_id: "abc123" }); getComponent(hsUrl, isUrl, delegatedAuth); await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); - expect(logger.error).toHaveBeenCalledWith( - new Error("OIDC native login flow configured but UI not yet supported."), - ); + // did not continue with matrix login + expect(mockClient.loginFlows).not.toHaveBeenCalled(); + // no oidc native UI yet + expect( + screen.getByText("This homeserver doesn't offer any login flows which are supported by this client."), + ).toBeInTheDocument(); + }); + + /** + * Oidc-aware flows still work while the oidc-native feature flag is disabled + */ + it("should show oidc-aware flow for oidc-enabled homeserver when oidc native flow setting is disabled", async () => { + jest.spyOn(SettingsStore, "getValue").mockReturnValue(false); + mockClient.loginFlows.mockResolvedValue({ + flows: [ + { + type: "m.login.sso", + [DELEGATED_OIDC_COMPATIBILITY.name]: true, + }, + { + type: "m.login.password", + }, + ], + }); + + const { container } = getComponent(hsUrl, isUrl, delegatedAuth); + + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + // didn't try to register + expect(fetchMock).not.toHaveBeenCalledWith(delegatedAuth.registrationEndpoint); // continued with normal setup expect(mockClient.loginFlows).toHaveBeenCalled(); - // normal password login rendered - expect(screen.getByLabelText("Username")).toBeInTheDocument(); + // oidc-aware 'continue' button displayed + const ssoButtons = container.querySelectorAll(".mx_SSOButton"); + expect(ssoButtons.length).toBe(1); + expect(ssoButtons[0].textContent).toBe("Continue"); + // no password form visible + expect(container.querySelector("form")).toBeFalsy(); }); }); }); From 1a53ad084cdb3d8c20c54c1816281e4fd8979f13 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 13 Jun 2023 11:25:28 +1200 Subject: [PATCH 10/26] strict fixes --- src/components/structures/auth/Login.tsx | 2 +- src/utils/oidc/registerClient.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/structures/auth/Login.tsx b/src/components/structures/auth/Login.tsx index c6095294e55..14801a93936 100644 --- a/src/components/structures/auth/Login.tsx +++ b/src/components/structures/auth/Login.tsx @@ -348,7 +348,7 @@ export default class LoginComponent extends React.PureComponent } catch (e) { this.setState({ busy: false, - ...AutoDiscoveryUtils.authComponentStateForError(e), + ...AutoDiscoveryUtils.authComponentStateForError(e as Error), }); } } diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index 9279d043053..ac7136fb53a 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -67,13 +67,13 @@ const doRegistration = async ( const body = await response.json(); const clientId = body["client_id"]; - if (!clientId) { + if (!clientId || typeof clientId !== "string") { throw new Error(OidcClientError.DynamicRegistrationInvalid); } return clientId; } catch (error) { - if (Object.values(OidcClientError).includes(error.message)) { + if (Object.values(OidcClientError).includes((error as Error).message as OidcClientError)) { throw error; } else { logger.error("Dynamic registration request failed", error); @@ -100,6 +100,9 @@ const registerOidcClient = async ( clientUri: baseUrl, redirectUris: [baseUrl], }; + if (!delegatedAuthConfig.registrationEndpoint) { + throw new Error(OidcClientError.DynamicRegistrationNotSupported); + } const clientId = await doRegistration(delegatedAuthConfig.registrationEndpoint, clientMetadata); return clientId; @@ -137,8 +140,5 @@ export const getOidcClientId = async ( logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.issuer}`); return staticClientId; } - if (!delegatedAuthConfig.registrationEndpoint) { - throw new Error(OidcClientError.DynamicRegistrationNotSupported); - } return await registerOidcClient(delegatedAuthConfig, clientName, baseUrl); }; From f9aaa28b88ff667f901d51ec4787c55dd0a63b7c Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 13 Jun 2023 11:29:40 +1200 Subject: [PATCH 11/26] remove unused code --- src/utils/oidc/error.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/utils/oidc/error.ts b/src/utils/oidc/error.ts index ea5fa46c1d3..35e7ba23c27 100644 --- a/src/utils/oidc/error.ts +++ b/src/utils/oidc/error.ts @@ -21,5 +21,3 @@ export enum OidcClientError { DynamicRegistrationFailed = "Dynamic registration failed", DynamicRegistrationInvalid = "Dynamic registration invalid response", } - -export type OidcError = OidcClientError | OidcDiscoveryError; From 9faa9c874c8a2e9d817cf4b37161b1b1105d8494 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 13 Jun 2023 11:29:54 +1200 Subject: [PATCH 12/26] and imports --- src/utils/oidc/error.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/utils/oidc/error.ts b/src/utils/oidc/error.ts index 35e7ba23c27..3604ee77c24 100644 --- a/src/utils/oidc/error.ts +++ b/src/utils/oidc/error.ts @@ -14,8 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { OidcDiscoveryError } from "matrix-js-sdk/src/oidc/validate"; - export enum OidcClientError { DynamicRegistrationNotSupported = "Dynamic registration not supported", DynamicRegistrationFailed = "Dynamic registration failed", From e92022990d09c8660e6d924a3f510c75e37b3ba4 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 14 Jun 2023 10:36:09 +1200 Subject: [PATCH 13/26] comments --- src/utils/oidc/registerClient.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index ac7136fb53a..8daa2c22002 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -29,10 +29,10 @@ export type OidcRegistrationClientMetadata = { /** * Make the registration request - * @param issuer issue URL + * @param registrationEndpoint URL as configured on ValidatedServerConfig * @param clientMetadata registration metadata * @returns {Promise} resolves to the registered client id when registration is successful - * @throws when registration fails, or issue does not support dynamic registration + * @throws when registration request fails, or response is invalid */ const doRegistration = async ( registrationEndpoint: string, From 5389d5c74c51c8bd14c2be17332e0e763c99a6c3 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 14 Jun 2023 10:37:03 +1200 Subject: [PATCH 14/26] comments 2 --- src/utils/oidc/registerClient.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index 8daa2c22002..7dc3998dc3d 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -28,7 +28,7 @@ export type OidcRegistrationClientMetadata = { }; /** - * Make the registration request + * Make the client registration request * @param registrationEndpoint URL as configured on ValidatedServerConfig * @param clientMetadata registration metadata * @returns {Promise} resolves to the registered client id when registration is successful @@ -88,7 +88,7 @@ const doRegistration = async ( * @param clientName Client name to register with the OP, eg 'Element' * @param baseUrl URL of the home page of the Client, eg 'https://app.element.io/' * @returns Promise resolved with registered clientId - * @throws on failed request or invalid response + * @throws when registration is not supported, on failed request or invalid response */ const registerOidcClient = async ( delegatedAuthConfig: IDelegatedAuthConfig & ValidatedIssuerConfig, @@ -109,7 +109,7 @@ const registerOidcClient = async ( }; /** - * Get the configured clientId for the issuer + * Get the statically configured clientId for the issuer * @param issuer delegated auth OIDC issuer * @param staticOidcClients static client config from config.json * @returns clientId if found, otherwise undefined @@ -119,7 +119,7 @@ const getStaticOidcClientId = (issuer: string, staticOidcClients?: Record Date: Wed, 14 Jun 2023 10:47:37 +1200 Subject: [PATCH 15/26] util functions to get static client id --- src/IConfigOptions.ts | 8 +++ src/utils/oidc/error.ts | 21 +++++++ src/utils/oidc/registerClient.ts | 66 ++++++++++++++++++++ test/utils/oidc/registerClient-test.ts | 86 ++++++++++++++++++++++++++ 4 files changed, 181 insertions(+) create mode 100644 src/utils/oidc/error.ts create mode 100644 src/utils/oidc/registerClient.ts create mode 100644 test/utils/oidc/registerClient-test.ts diff --git a/src/IConfigOptions.ts b/src/IConfigOptions.ts index 1fa873ec87a..bf8ca1ee866 100644 --- a/src/IConfigOptions.ts +++ b/src/IConfigOptions.ts @@ -192,6 +192,14 @@ export interface IConfigOptions { existing_issues_url: string; new_issue_url: string; }; + + /** + * Configuration for OIDC issuers where a static client_id has been issued for the app. + * Otherwise dynamic client registration is attempted. + * The issuer URL must have a trailing `/`. + * OPTIONAL + */ + oidc_static_clients?: Record; } export interface ISsoRedirectOptions { diff --git a/src/utils/oidc/error.ts b/src/utils/oidc/error.ts new file mode 100644 index 00000000000..3604ee77c24 --- /dev/null +++ b/src/utils/oidc/error.ts @@ -0,0 +1,21 @@ +/* +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. +*/ + +export enum OidcClientError { + DynamicRegistrationNotSupported = "Dynamic registration not supported", + DynamicRegistrationFailed = "Dynamic registration failed", + DynamicRegistrationInvalid = "Dynamic registration invalid response", +} diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts new file mode 100644 index 00000000000..425a62bcc23 --- /dev/null +++ b/src/utils/oidc/registerClient.ts @@ -0,0 +1,66 @@ +/* +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 { IDelegatedAuthConfig } from "matrix-js-sdk/src/client"; +import { logger } from "matrix-js-sdk/src/logger"; +import { ValidatedIssuerConfig } from "matrix-js-sdk/src/oidc/validate"; + +import { OidcClientError } from "./error"; + +export type OidcRegistrationClientMetadata = { + clientName: string; + clientUri: string; + redirectUris: string[]; +}; + +/** + * Get the statically configured clientId for the issuer + * @param issuer delegated auth OIDC issuer + * @param staticOidcClients static client config from config.json + * @returns clientId if found, otherwise undefined + */ +const getStaticOidcClientId = (issuer: string, staticOidcClients?: Record): string | undefined => { + return staticOidcClients?.[issuer]; +}; + +/** + * Get the clientId for an OIDC OP + * Checks statically configured clientIds first + * Then attempts dynamic registration with the OP + * @param delegatedAuthConfig Auth config from ValidatedServerConfig + * @param clientName Client name to register with the OP, eg 'Element' + * @param baseUrl URL of the home page of the Client, eg 'https://app.element.io/' + * @param staticOidcClients static client config from config.json + * @returns Promise resolves with clientId + * @throws if no clientId is found or registration failed + */ +export const getOidcClientId = async ( + delegatedAuthConfig: IDelegatedAuthConfig & ValidatedIssuerConfig, + // these are used in the following PR + _clientName: string, + _baseUrl: string, + staticOidcClients?: Record, +): Promise => { + const staticClientId = getStaticOidcClientId(delegatedAuthConfig.issuer, staticOidcClients); + if (staticClientId) { + logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.issuer}`); + return staticClientId; + } + + // TODO attempt dynamic registration + logger.error("Dynamic registration not yet implemented."); + throw new Error(OidcClientError.DynamicRegistrationNotSupported); +}; diff --git a/test/utils/oidc/registerClient-test.ts b/test/utils/oidc/registerClient-test.ts new file mode 100644 index 00000000000..06f67671d06 --- /dev/null +++ b/test/utils/oidc/registerClient-test.ts @@ -0,0 +1,86 @@ +/* +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 fetchMockJest from "fetch-mock-jest"; + +import { OidcClientError } from "../../../src/utils/oidc/error"; +import { getOidcClientId } from "../../../src/utils/oidc/registerClient"; + +describe("getOidcClientId()", () => { + const issuer = "https://auth.com/"; + const registrationEndpoint = "https://auth.com/register"; + const clientName = "Element"; + const baseUrl = "https://just.testing"; + const dynamicClientId = "xyz789"; + const staticOidcClients = { + [issuer]: "abc123", + }; + const delegatedAuthConfig = { + issuer, + registrationEndpoint, + authorizationEndpoint: issuer + "auth", + tokenEndpoint: issuer + "token", + }; + + beforeEach(() => { + fetchMockJest.mockClear(); + fetchMockJest.resetBehavior(); + }); + + it("should return static clientId when configured", async () => { + expect(await getOidcClientId(delegatedAuthConfig, clientName, baseUrl, staticOidcClients)).toEqual( + staticOidcClients[issuer], + ); + // didn't try to register + expect(fetchMockJest).toHaveFetchedTimes(0); + }); + + it("should throw when no static clientId is configured and no registration endpoint", async () => { + const authConfigWithoutRegistration = { + ...delegatedAuthConfig, + issuer: "https://issuerWithoutStaticClientId.org/", + registrationEndpoint: undefined, + }; + expect( + async () => await getOidcClientId(authConfigWithoutRegistration, clientName, baseUrl, staticOidcClients), + ).rejects.toThrow(OidcClientError.DynamicRegistrationNotSupported); + // didn't try to register + expect(fetchMockJest).toHaveFetchedTimes(0); + }); + + it("should handle when staticOidcClients object is falsy", async () => { + const authConfigWithoutRegistration = { + ...delegatedAuthConfig, + registrationEndpoint: undefined, + }; + expect(async () => await getOidcClientId(authConfigWithoutRegistration, clientName, baseUrl)).rejects.toThrow( + OidcClientError.DynamicRegistrationNotSupported, + ); + // didn't try to register + expect(fetchMockJest).toHaveFetchedTimes(0); + }); + + it("should throw while dynamic registration is not implemented", async () => { + fetchMockJest.post(registrationEndpoint, { + status: 200, + body: JSON.stringify({ client_id: dynamicClientId }), + }); + + expect(async () => await getOidcClientId(delegatedAuthConfig, clientName, baseUrl)).rejects.toThrow( + OidcClientError.DynamicRegistrationNotSupported, + ); + }); +}); From 9c89f414675168ebe56596fc0a0246053fb4ed5c Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 14 Jun 2023 10:55:10 +1200 Subject: [PATCH 16/26] check static client ids in login flow --- src/Login.ts | 64 +++++++- src/components/structures/auth/Login.tsx | 73 +++++---- src/utils/oidc/registerClient.ts | 3 +- .../components/structures/auth/Login-test.tsx | 140 +++++++++++++++++- test/test-utils/test-utils.ts | 7 +- 5 files changed, 247 insertions(+), 40 deletions(-) diff --git a/src/Login.ts b/src/Login.ts index 569363eeb05..6426550eaa6 100644 --- a/src/Login.ts +++ b/src/Login.ts @@ -19,18 +19,32 @@ limitations under the License. import { createClient } from "matrix-js-sdk/src/matrix"; import { MatrixClient } from "matrix-js-sdk/src/client"; import { logger } from "matrix-js-sdk/src/logger"; -import { DELEGATED_OIDC_COMPATIBILITY, ILoginParams, LoginFlow } from "matrix-js-sdk/src/@types/auth"; +import { OidcDiscoveryError } from "matrix-js-sdk/src/oidc/validate"; +import { DELEGATED_OIDC_COMPATIBILITY, ILoginFlow, ILoginParams, LoginFlow } from "matrix-js-sdk/src/@types/auth"; import { IMatrixClientCreds } from "./MatrixClientPeg"; import SecurityCustomisations from "./customisations/Security"; +import { ValidatedServerConfig } from "./utils/ValidatedServerConfig"; +import { getOidcClientId } from "./utils/oidc/registerClient"; +import { IConfigOptions } from "./IConfigOptions"; +import SdkConfig from "./SdkConfig"; + +export type ClientLoginFlow = LoginFlow | OidcNativeFlow; interface ILoginOptions { defaultDeviceDisplayName?: string; + /** + * Delegated auth config from server config + * When prop is passed we will attempt to use delegated auth + * Labs flag should be checked in parent + */ + delegatedAuthentication?: ValidatedServerConfig["delegatedAuthentication"]; } export default class Login { - private flows: Array = []; + private flows: Array = []; private readonly defaultDeviceDisplayName?: string; + private readonly delegatedAuthentication?: ValidatedServerConfig["delegatedAuthentication"]; private tempClient: MatrixClient | null = null; // memoize public constructor( @@ -40,6 +54,7 @@ export default class Login { opts: ILoginOptions, ) { this.defaultDeviceDisplayName = opts.defaultDeviceDisplayName; + this.delegatedAuthentication = opts.delegatedAuthentication; } public getHomeserverUrl(): string { @@ -75,7 +90,20 @@ export default class Login { return this.tempClient; } - public async getFlows(): Promise> { + public async getFlows(): Promise> { + // try to use oidc native flow + try { + const oidcFlow = await tryInitOidcNativeFlow( + this.delegatedAuthentication, + SdkConfig.get().brand, + SdkConfig.get().oidc_static_clients, + ); + return [oidcFlow]; + } catch (error) { + logger.error(error); + } + + // oidc native flow not supported, continue with matrix login const client = this.createTemporaryClient(); const { flows }: { flows: LoginFlow[] } = await client.loginFlows(); // If an m.login.sso flow is present which is also flagged as being for MSC3824 OIDC compatibility then we only @@ -151,6 +179,36 @@ export default class Login { } } +export interface OidcNativeFlow extends ILoginFlow { + type: "oidcNativeFlow"; + clientId: string; +} +/** + * Finds static clientId for configured issuer, or attempts dynamic registration with the OP + * @param delegatedAuthConfig Auth config from ValidatedServerConfig + * @param clientName Client name to register with the OP, eg 'Element', used during client registration with OP + * @param staticOidcClients static client config from config.json, used during client registration with OP + * @returns Promise when oidc native authentication flow is supported and correctly configured + * @throws when oidc native flow is not configured, client can't register with OP, or any unexpected error + */ +const tryInitOidcNativeFlow = async ( + delegatedAuthConfig: ValidatedServerConfig["delegatedAuthentication"] | undefined, + brand: string, + oidcStaticClients?: IConfigOptions["oidc_static_clients"], +): Promise => { + if (!delegatedAuthConfig) { + throw new Error(OidcDiscoveryError.NotSupported); + } + const clientId = await getOidcClientId(delegatedAuthConfig, brand, window.location.origin, oidcStaticClients); + + const flow = { + type: "oidcNativeFlow", + clientId, + } as OidcNativeFlow; + + return flow; +}; + /** * Send a login request to the given server, and format the response * as a MatrixClientCreds diff --git a/src/components/structures/auth/Login.tsx b/src/components/structures/auth/Login.tsx index 3299cae3fc3..14801a93936 100644 --- a/src/components/structures/auth/Login.tsx +++ b/src/components/structures/auth/Login.tsx @@ -17,10 +17,10 @@ limitations under the License. import React, { ReactNode } from "react"; import classNames from "classnames"; import { logger } from "matrix-js-sdk/src/logger"; -import { ISSOFlow, LoginFlow, SSOAction } from "matrix-js-sdk/src/@types/auth"; +import { ISSOFlow, SSOAction } from "matrix-js-sdk/src/@types/auth"; import { _t, _td, UserFriendlyError } from "../../../languageHandler"; -import Login from "../../../Login"; +import Login, { ClientLoginFlow } from "../../../Login"; import { messageForConnectionError, messageForLoginError } from "../../../utils/ErrorUtils"; import AutoDiscoveryUtils from "../../../utils/AutoDiscoveryUtils"; import AuthPage from "../../views/auth/AuthPage"; @@ -38,6 +38,7 @@ import AuthHeader from "../../views/auth/AuthHeader"; import AccessibleButton, { ButtonEvent } from "../../views/elements/AccessibleButton"; import { ValidatedServerConfig } from "../../../utils/ValidatedServerConfig"; import { filterBoolean } from "../../../utils/arrays"; +import { Features } from "../../../settings/Settings"; // These are used in several places, and come from the js-sdk's autodiscovery // stuff. We define them here so that they'll be picked up by i18n. @@ -49,7 +50,6 @@ _td("Invalid identity server discovery response"); _td("Invalid base_url for m.identity_server"); _td("Identity server URL does not appear to be a valid identity server"); _td("General failure"); - interface IProps { serverConfig: ValidatedServerConfig; // If true, the component will consider itself busy. @@ -84,7 +84,7 @@ interface IState { // can we attempt to log in or are there validation errors? canTryLogin: boolean; - flows?: LoginFlow[]; + flows?: ClientLoginFlow[]; // used for preserving form values when changing homeserver username: string; @@ -110,6 +110,7 @@ type OnPasswordLogin = { */ export default class LoginComponent extends React.PureComponent { private unmounted = false; + private oidcNativeFlowEnabled = false; private loginLogic!: Login; private readonly stepRendererMap: Record ReactNode>; @@ -117,6 +118,9 @@ export default class LoginComponent extends React.PureComponent public constructor(props: IProps) { super(props); + // only set on a config level, so we don't need to watch + this.oidcNativeFlowEnabled = SettingsStore.getValue(Features.OidcNativeFlow); + this.state = { busy: false, errorText: null, @@ -156,7 +160,8 @@ export default class LoginComponent extends React.PureComponent public componentDidUpdate(prevProps: IProps): void { if ( prevProps.serverConfig.hsUrl !== this.props.serverConfig.hsUrl || - prevProps.serverConfig.isUrl !== this.props.serverConfig.isUrl + prevProps.serverConfig.isUrl !== this.props.serverConfig.isUrl || + prevProps.serverConfig.delegatedAuthentication !== this.props.serverConfig.delegatedAuthentication ) { // Ensure that we end up actually logging in to the right place this.initLoginLogic(this.props.serverConfig); @@ -322,28 +327,10 @@ export default class LoginComponent extends React.PureComponent } }; - private async initLoginLogic({ hsUrl, isUrl }: ValidatedServerConfig): Promise { - let isDefaultServer = false; - if ( - this.props.serverConfig.isDefault && - hsUrl === this.props.serverConfig.hsUrl && - isUrl === this.props.serverConfig.isUrl - ) { - isDefaultServer = true; - } - - const fallbackHsUrl = isDefaultServer ? this.props.fallbackHsUrl! : null; - - const loginLogic = new Login(hsUrl, isUrl, fallbackHsUrl, { - defaultDeviceDisplayName: this.props.defaultDeviceDisplayName, - }); - this.loginLogic = loginLogic; - - this.setState({ - busy: true, - loginIncorrect: false, - }); - + private async checkServerLiveliness({ + hsUrl, + isUrl, + }: Pick): Promise { // Do a quick liveliness check on the URLs try { const { warning } = await AutoDiscoveryUtils.validateServerConfigWithStaticUrls(hsUrl, isUrl); @@ -361,9 +348,37 @@ export default class LoginComponent extends React.PureComponent } catch (e) { this.setState({ busy: false, - ...AutoDiscoveryUtils.authComponentStateForError(e), + ...AutoDiscoveryUtils.authComponentStateForError(e as Error), }); } + } + + private async initLoginLogic({ hsUrl, isUrl }: ValidatedServerConfig): Promise { + let isDefaultServer = false; + if ( + this.props.serverConfig.isDefault && + hsUrl === this.props.serverConfig.hsUrl && + isUrl === this.props.serverConfig.isUrl + ) { + isDefaultServer = true; + } + + const fallbackHsUrl = isDefaultServer ? this.props.fallbackHsUrl! : null; + + this.setState({ + busy: true, + loginIncorrect: false, + }); + + await this.checkServerLiveliness({ hsUrl, isUrl }); + + const loginLogic = new Login(hsUrl, isUrl, fallbackHsUrl, { + defaultDeviceDisplayName: this.props.defaultDeviceDisplayName, + delegatedAuthentication: this.oidcNativeFlowEnabled + ? this.props.serverConfig.delegatedAuthentication + : undefined, + }); + this.loginLogic = loginLogic; loginLogic .getFlows() @@ -401,7 +416,7 @@ export default class LoginComponent extends React.PureComponent }); } - private isSupportedFlow = (flow: LoginFlow): boolean => { + private isSupportedFlow = (flow: ClientLoginFlow): boolean => { // technically the flow can have multiple steps, but no one does this // for login and loginLogic doesn't support it so we can ignore it. if (!this.stepRendererMap[flow.type]) { diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index 425a62bcc23..dfb07dfff43 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -39,13 +39,12 @@ const getStaticOidcClientId = (issuer: string, staticOidcClients?: Record resolves with clientId - * @throws if no clientId is found or registration failed + * @throws if no clientId is found */ export const getOidcClientId = async ( delegatedAuthConfig: IDelegatedAuthConfig & ValidatedIssuerConfig, diff --git a/test/components/structures/auth/Login-test.tsx b/test/components/structures/auth/Login-test.tsx index 8eb25d49097..bb59a24121e 100644 --- a/test/components/structures/auth/Login-test.tsx +++ b/test/components/structures/auth/Login-test.tsx @@ -17,19 +17,29 @@ limitations under the License. import React from "react"; import { fireEvent, render, screen, waitForElementToBeRemoved } from "@testing-library/react"; import { mocked, MockedObject } from "jest-mock"; -import { createClient, MatrixClient } from "matrix-js-sdk/src/matrix"; import fetchMock from "fetch-mock-jest"; import { DELEGATED_OIDC_COMPATIBILITY, IdentityProviderBrand } from "matrix-js-sdk/src/@types/auth"; +import { logger } from "matrix-js-sdk/src/logger"; +import { createClient, MatrixClient } from "matrix-js-sdk/src/matrix"; import SdkConfig from "../../../../src/SdkConfig"; import { mkServerConfig, mockPlatformPeg, unmockPlatformPeg } from "../../../test-utils"; import Login from "../../../../src/components/structures/auth/Login"; import BasePlatform from "../../../../src/BasePlatform"; +import SettingsStore from "../../../../src/settings/SettingsStore"; +import { Features } from "../../../../src/settings/Settings"; +import { ValidatedServerConfig } from "../../../../src/utils/ValidatedServerConfig"; +import * as registerClientUtils from "../../../../src/utils/oidc/registerClient"; +import { OidcClientError } from "../../../../src/utils/oidc/error"; jest.mock("matrix-js-sdk/src/matrix"); jest.useRealTimers(); +const oidcStaticClientsConfig = { + "https://staticallyregisteredissuer.org/": "static-clientId-123", +}; + describe("Login", function () { let platform: MockedObject; @@ -42,6 +52,7 @@ describe("Login", function () { SdkConfig.put({ brand: "test-brand", disable_custom_urls: true, + oidc_static_clients: oidcStaticClientsConfig, }); mockClient.login.mockClear().mockResolvedValue({}); mockClient.loginFlows.mockClear().mockResolvedValue({ flows: [{ type: "m.login.password" }] }); @@ -51,6 +62,7 @@ describe("Login", function () { return mockClient; }); fetchMock.resetBehavior(); + fetchMock.resetHistory(); fetchMock.get("https://matrix.org/_matrix/client/versions", { unstable_features: {}, versions: [], @@ -66,10 +78,14 @@ describe("Login", function () { unmockPlatformPeg(); }); - function getRawComponent(hsUrl = "https://matrix.org", isUrl = "https://vector.im") { + function getRawComponent( + hsUrl = "https://matrix.org", + isUrl = "https://vector.im", + delegatedAuthentication?: ValidatedServerConfig["delegatedAuthentication"], + ) { return ( {}} onRegisterClick={() => {}} onServerConfigChange={() => {}} @@ -77,8 +93,12 @@ describe("Login", function () { ); } - function getComponent(hsUrl?: string, isUrl?: string) { - return render(getRawComponent(hsUrl, isUrl)); + function getComponent( + hsUrl?: string, + isUrl?: string, + delegatedAuthentication?: ValidatedServerConfig["delegatedAuthentication"], + ) { + return render(getRawComponent(hsUrl, isUrl, delegatedAuthentication)); } it("should show form with change server link", async () => { @@ -190,6 +210,7 @@ describe("Login", function () { versions: [], }); rerender(getRawComponent("https://server2")); + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); fireEvent.click(container.querySelector(".mx_SSOButton")!); expect(platform.startSingleSignOn.mock.calls[1][0].baseUrl).toBe("https://server2"); @@ -319,4 +340,113 @@ describe("Login", function () { // error cleared expect(screen.queryByText("Your test-brand is misconfigured")).not.toBeInTheDocument(); }); + + describe("OIDC native flow", () => { + const hsUrl = "https://matrix.org"; + const isUrl = "https://vector.im"; + const issuer = "https://test.com/"; + const delegatedAuth = { + issuer, + registrationEndpoint: issuer + "register", + tokenEndpoint: issuer + "token", + authorizationEndpoint: issuer + "authorization", + }; + beforeEach(() => { + jest.spyOn(logger, "error").mockClear(); + jest.spyOn(SettingsStore, "getValue").mockImplementation( + (settingName) => settingName === Features.OidcNativeFlow, + ); + }); + + it("should not attempt registration when oidc native flow setting is disabled", async () => { + jest.spyOn(SettingsStore, "getValue").mockReturnValue(false); + + getComponent(hsUrl, isUrl, delegatedAuth); + + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + // continued with normal setup + expect(mockClient.loginFlows).toHaveBeenCalled(); + // normal password login rendered + expect(screen.getByLabelText("Username")).toBeInTheDocument(); + }); + + it("should attempt to register oidc client", async () => { + // dont mock, spy so we can check config values were correctly passed + jest.spyOn(registerClientUtils, "getOidcClientId"); + getComponent(hsUrl, isUrl, delegatedAuth); + + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + // called with values from config + expect(registerClientUtils.getOidcClientId).toHaveBeenCalledWith( + delegatedAuth, + "test-brand", + "http://localhost", + oidcStaticClientsConfig, + ); + }); + + it("should fallback to normal login when client does not have static clientId", async () => { + getComponent(hsUrl, isUrl, delegatedAuth); + + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + expect(logger.error).toHaveBeenCalledWith(new Error(OidcClientError.DynamicRegistrationNotSupported)); + + // continued with normal setup + expect(mockClient.loginFlows).toHaveBeenCalled(); + // normal password login rendered + expect(screen.getByLabelText("Username")).toBeInTheDocument(); + }); + + // short term during active development, UI will be added in next PRs + it("should show error when oidc native flow is correctly configured but not supported by UI", async () => { + const delegatedAuthWithStaticClientId = { + ...delegatedAuth, + issuer: "https://staticallyregisteredissuer.org/", + }; + getComponent(hsUrl, isUrl, delegatedAuthWithStaticClientId); + + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + // did not continue with matrix login + expect(mockClient.loginFlows).not.toHaveBeenCalled(); + // no oidc native UI yet + expect( + screen.getByText("This homeserver doesn't offer any login flows which are supported by this client."), + ).toBeInTheDocument(); + }); + + /** + * Oidc-aware flows still work while the oidc-native feature flag is disabled + */ + it("should show oidc-aware flow for oidc-enabled homeserver when oidc native flow setting is disabled", async () => { + jest.spyOn(SettingsStore, "getValue").mockReturnValue(false); + mockClient.loginFlows.mockResolvedValue({ + flows: [ + { + type: "m.login.sso", + [DELEGATED_OIDC_COMPATIBILITY.name]: true, + }, + { + type: "m.login.password", + }, + ], + }); + + const { container } = getComponent(hsUrl, isUrl, delegatedAuth); + + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + + // continued with normal setup + expect(mockClient.loginFlows).toHaveBeenCalled(); + // oidc-aware 'continue' button displayed + const ssoButtons = container.querySelectorAll(".mx_SSOButton"); + expect(ssoButtons.length).toBe(1); + expect(ssoButtons[0].textContent).toBe("Continue"); + // no password form visible + expect(container.querySelector("form")).toBeFalsy(); + }); + }); }); diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 6eea4114d23..a2ca4313407 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -617,12 +617,17 @@ export function mkStubRoom( } as unknown as Room; } -export function mkServerConfig(hsUrl: string, isUrl: string): ValidatedServerConfig { +export function mkServerConfig( + hsUrl: string, + isUrl: string, + delegatedAuthentication?: ValidatedServerConfig["delegatedAuthentication"], +): ValidatedServerConfig { return { hsUrl, hsName: "TEST_ENVIRONMENT", hsNameIsDifferent: false, // yes, we lie isUrl, + delegatedAuthentication, } as ValidatedServerConfig; } From 422823e782ba388c370d5046c0a63c9fcd359dc0 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 14 Jun 2023 14:29:01 +1200 Subject: [PATCH 17/26] remove dead code --- src/utils/oidc/registerClient.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index dfb07dfff43..6cef00488da 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -20,12 +20,6 @@ import { ValidatedIssuerConfig } from "matrix-js-sdk/src/oidc/validate"; import { OidcClientError } from "./error"; -export type OidcRegistrationClientMetadata = { - clientName: string; - clientUri: string; - redirectUris: string[]; -}; - /** * Get the statically configured clientId for the issuer * @param issuer delegated auth OIDC issuer From a76cde9d33c4636304d69c1b58398864648d812d Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 14 Jun 2023 14:30:35 +1200 Subject: [PATCH 18/26] OidcRegistrationClientMetadata type --- src/utils/oidc/registerClient.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index 0625c6cbedc..7dc3998dc3d 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -21,6 +21,12 @@ import { ValidatedIssuerConfig } from "matrix-js-sdk/src/oidc/validate"; import { OidcClientError } from "./error"; +export type OidcRegistrationClientMetadata = { + clientName: string; + clientUri: string; + redirectUris: string[]; +}; + /** * Make the client registration request * @param registrationEndpoint URL as configured on ValidatedServerConfig From 52d03c22546a4eb945878ba3e0aa2b9fae04935e Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 14 Jun 2023 17:04:32 +1200 Subject: [PATCH 19/26] navigate to oidc authorize url --- res/css/structures/auth/_Login.pcss | 5 + src/Lifecycle.ts | 1 + src/components/structures/auth/Login.tsx | 21 +++- src/utils/oidc/authorize.ts | 121 +++++++++++++++++++++++ src/utils/oidc/registerClient.ts | 5 +- 5 files changed, 150 insertions(+), 3 deletions(-) create mode 100644 src/utils/oidc/authorize.ts diff --git a/res/css/structures/auth/_Login.pcss b/res/css/structures/auth/_Login.pcss index 2eba8cf3d14..eeca1e8e494 100644 --- a/res/css/structures/auth/_Login.pcss +++ b/res/css/structures/auth/_Login.pcss @@ -99,3 +99,8 @@ div.mx_AccessibleButton_kind_link.mx_Login_forgot { align-content: center; padding: 14px; } + +.mx_Login_fullWidthButton { + width: 100%; + margin-bottom: 16px; +} diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 6f49a0a0b61..bf9293e4df6 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -196,6 +196,7 @@ export function attemptTokenLogin( defaultDeviceDisplayName?: string, fragmentAfterLogin?: string, ): Promise { + debugger; if (!queryParams.loginToken) { return Promise.resolve(false); } diff --git a/src/components/structures/auth/Login.tsx b/src/components/structures/auth/Login.tsx index 14801a93936..a39b79988bb 100644 --- a/src/components/structures/auth/Login.tsx +++ b/src/components/structures/auth/Login.tsx @@ -20,7 +20,7 @@ import { logger } from "matrix-js-sdk/src/logger"; import { ISSOFlow, SSOAction } from "matrix-js-sdk/src/@types/auth"; import { _t, _td, UserFriendlyError } from "../../../languageHandler"; -import Login, { ClientLoginFlow } from "../../../Login"; +import Login, { ClientLoginFlow, OidcNativeFlow } from "../../../Login"; import { messageForConnectionError, messageForLoginError } from "../../../utils/ErrorUtils"; import AutoDiscoveryUtils from "../../../utils/AutoDiscoveryUtils"; import AuthPage from "../../views/auth/AuthPage"; @@ -39,6 +39,7 @@ import AccessibleButton, { ButtonEvent } from "../../views/elements/AccessibleBu import { ValidatedServerConfig } from "../../../utils/ValidatedServerConfig"; import { filterBoolean } from "../../../utils/arrays"; import { Features } from "../../../settings/Settings"; +import { login, startOidcLogin } from "../../../utils/oidc/authorize"; // These are used in several places, and come from the js-sdk's autodiscovery // stuff. We define them here so that they'll be picked up by i18n. @@ -146,6 +147,7 @@ export default class LoginComponent extends React.PureComponent "m.login.cas": () => this.renderSsoStep("cas"), // eslint-disable-next-line @typescript-eslint/naming-convention "m.login.sso": () => this.renderSsoStep("sso"), + "oidcNativeFlow": () => this.renderOidcNativeStep(), }; } @@ -430,7 +432,7 @@ export default class LoginComponent extends React.PureComponent if (!this.state.flows) return null; // this is the ideal order we want to show the flows in - const order = ["m.login.password", "m.login.sso"]; + const order = ["oidcNativeFlow", "m.login.password", "m.login.sso"]; const flows = filterBoolean(order.map((type) => this.state.flows?.find((flow) => flow.type === type))); return ( @@ -463,6 +465,21 @@ export default class LoginComponent extends React.PureComponent ); }; + private renderOidcNativeStep = (): React.ReactNode => { + const flow = this.state.flows!.find((flow) => flow.type === "oidcNativeFlow")! as OidcNativeFlow; + return ( + { + await startOidcLogin(this.props.serverConfig.delegatedAuthentication!, flow.clientId, this.props.serverConfig.hsUrl); + }} + > + {_t("Continue")} + + ); + }; + private renderSsoStep = (loginType: "cas" | "sso"): JSX.Element => { const flow = this.state.flows?.find((flow) => flow.type === "m.login." + loginType) as ISSOFlow; diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts new file mode 100644 index 00000000000..aa0a6722c7a --- /dev/null +++ b/src/utils/oidc/authorize.ts @@ -0,0 +1,121 @@ +/* +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 { logger } from "matrix-js-sdk/src/logger"; +import { randomString } from "matrix-js-sdk/src/randomstring"; +import { ValidatedServerConfig } from "../ValidatedServerConfig"; + +type AuthorizationParams = { + state: string; + scope: string; + redirectUri: string; + nonce?: string; + codeVerifier?: string; +}; + +const generateScope = (existingDeviceId?: string): string => { + const deviceId = existingDeviceId || randomString(10); + return `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${deviceId}`; +}; + +// https://www.rfc-editor.org/rfc/rfc7636 +const generateCodeChallenge = async (codeVerifier: string): Promise => { + if (!window.crypto || !window.crypto.subtle) { + // @TODO(kerrya) should this be allowed? configurable? + logger.warn("A secure context is required to generate code challenge. Using plain text code challenge"); + return codeVerifier; + } + const utf8 = new TextEncoder().encode(codeVerifier); + + const digest = await crypto.subtle.digest("SHA-256", utf8); + + return btoa(String.fromCharCode(...new Uint8Array(digest))) + .replace(/=/g, "") + .replace(/\+/g, "-") + .replace(/\//g, "_"); +}; + +const storeAuthorizationParams = async ( + { redirectUri, state, nonce, codeVerifier }: AuthorizationParams, + { issuer }: ValidatedServerConfig["delegatedAuthentication"], + clientId: string, + homeserver: string, +): Promise => { + window.sessionStorage.setItem(`oidc_${state}_nonce`, nonce); + window.sessionStorage.setItem(`oidc_${state}_redirectUri`, redirectUri); + window.sessionStorage.setItem(`oidc_${state}_codeVerifier`, codeVerifier); + window.sessionStorage.setItem(`oidc_${state}_clientId`, clientId); + window.sessionStorage.setItem(`oidc_${state}_issuer`, issuer); + window.sessionStorage.setItem(`oidc_${state}_homeserver`, homeserver); +}; + +const generateAuthorizationParams = ({ + deviceId, + redirectUri, +}: { + deviceId?: string; + redirectUri: string; +}): AuthorizationParams => ({ + scope: generateScope(deviceId), + redirectUri, + state: randomString(8), + nonce: randomString(8), + codeVerifier: randomString(64), // https://tools.ietf.org/html/rfc7636#section-4.1 length needs to be 43-128 characters +}); + +const generateAuthorizationUrl = async ( + authorizationUrl: string, + clientId: string, + { scope, redirectUri, state, nonce, codeVerifier }: AuthorizationParams, +): Promise => { + const url = new URL(authorizationUrl); + url.searchParams.append("response_mode", "fragment"); + url.searchParams.append("response_type", "code"); + url.searchParams.append("redirect_uri", redirectUri); + url.searchParams.append("client_id", clientId); + url.searchParams.append("state", state); + url.searchParams.append("scope", scope); + if (nonce) { + url.searchParams.append("nonce", nonce); + } + + if (codeVerifier) { + url.searchParams.append("code_challenge_method", "S256"); + url.searchParams.append("code_challenge", await generateCodeChallenge(codeVerifier)); + } + + return url.toString(); +}; + +export const startOidcLogin = async ( + delegatedAuthConfig: ValidatedServerConfig["delegatedAuthentication"], + clientId: string, + homeserver: string, +): Promise => { + // TODO(kerrya) afterloginfragment + const redirectUri = window.location.origin; + const authParams = generateAuthorizationParams({ redirectUri }); + + storeAuthorizationParams(authParams, delegatedAuthConfig, clientId, homeserver); + + const authorizationUrl = await generateAuthorizationUrl( + delegatedAuthConfig.authorizationEndpoint, + clientId, + authParams, + ); + + window.location.href = authorizationUrl; +}; diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index 7dc3998dc3d..ab98c756211 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -115,7 +115,10 @@ const registerOidcClient = async ( * @returns clientId if found, otherwise undefined */ const getStaticOidcClientId = (issuer: string, staticOidcClients?: Record): string | undefined => { - return staticOidcClients?.[issuer]; + console.log('hhh getStaticClient', issuer, staticOidcClients); + // static_oidc_clients are configured with a trailing slash + const issuerWithTrailingSlash = issuer.endsWith('/') ? issuer : issuer + '/'; + return staticOidcClients?.[issuerWithTrailingSlash]; }; /** From 3c499da47d035540fa2bf123da077a5b566b794b Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 15 Jun 2023 18:21:18 +1200 Subject: [PATCH 20/26] navigate to oidc authorize url --- src/Lifecycle.ts | 1 - src/components/structures/auth/Login.tsx | 8 ++++-- src/utils/oidc/authorize.ts | 25 +++++++++++++------ src/utils/oidc/registerClient.ts | 3 +-- .../components/structures/auth/Login-test.tsx | 7 ++---- 5 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index bf9293e4df6..6f49a0a0b61 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -196,7 +196,6 @@ export function attemptTokenLogin( defaultDeviceDisplayName?: string, fragmentAfterLogin?: string, ): Promise { - debugger; if (!queryParams.loginToken) { return Promise.resolve(false); } diff --git a/src/components/structures/auth/Login.tsx b/src/components/structures/auth/Login.tsx index a39b79988bb..1f97d734ea1 100644 --- a/src/components/structures/auth/Login.tsx +++ b/src/components/structures/auth/Login.tsx @@ -39,7 +39,7 @@ import AccessibleButton, { ButtonEvent } from "../../views/elements/AccessibleBu import { ValidatedServerConfig } from "../../../utils/ValidatedServerConfig"; import { filterBoolean } from "../../../utils/arrays"; import { Features } from "../../../settings/Settings"; -import { login, startOidcLogin } from "../../../utils/oidc/authorize"; +import { startOidcLogin } from "../../../utils/oidc/authorize"; // These are used in several places, and come from the js-sdk's autodiscovery // stuff. We define them here so that they'll be picked up by i18n. @@ -472,7 +472,11 @@ export default class LoginComponent extends React.PureComponent className="mx_Login_fullWidthButton" kind="primary" onClick={async () => { - await startOidcLogin(this.props.serverConfig.delegatedAuthentication!, flow.clientId, this.props.serverConfig.hsUrl); + await startOidcLogin( + this.props.serverConfig.delegatedAuthentication!, + flow.clientId, + this.props.serverConfig.hsUrl, + ); }} > {_t("Continue")} diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index aa0a6722c7a..80427f05ffe 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -16,6 +16,7 @@ limitations under the License. import { logger } from "matrix-js-sdk/src/logger"; import { randomString } from "matrix-js-sdk/src/randomstring"; + import { ValidatedServerConfig } from "../ValidatedServerConfig"; type AuthorizationParams = { @@ -48,6 +49,13 @@ const generateCodeChallenge = async (codeVerifier: string): Promise => { .replace(/\//g, "_"); }; +/** + * Store authorization params for retrieval when returning from OIDC OP + * @param { AuthorizationParams } authorizationParams + * @param { ValidatedServerConfig["delegatedAuthentication"] } delegatedAuthConfig used for future interactions with OP + * @param clientId this client's id as registered with configured issuer + * @param homeserver target homeserver + */ const storeAuthorizationParams = async ( { redirectUri, state, nonce, codeVerifier }: AuthorizationParams, { issuer }: ValidatedServerConfig["delegatedAuthentication"], @@ -88,18 +96,21 @@ const generateAuthorizationUrl = async ( url.searchParams.append("client_id", clientId); url.searchParams.append("state", state); url.searchParams.append("scope", scope); - if (nonce) { - url.searchParams.append("nonce", nonce); - } + url.searchParams.append("nonce", nonce); - if (codeVerifier) { - url.searchParams.append("code_challenge_method", "S256"); - url.searchParams.append("code_challenge", await generateCodeChallenge(codeVerifier)); - } + url.searchParams.append("code_challenge_method", "S256"); + url.searchParams.append("code_challenge", await generateCodeChallenge(codeVerifier)); return url.toString(); }; +/** + * Start OIDC authorization code flow + * Navigates to configured authorization endpoint + * @param delegatedAuthConfig from discovery + * @param clientId this client's id as registered with configured issuer + * @param homeserver target homeserver + */ export const startOidcLogin = async ( delegatedAuthConfig: ValidatedServerConfig["delegatedAuthentication"], clientId: string, diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index ab98c756211..5264e0fa757 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -115,9 +115,8 @@ const registerOidcClient = async ( * @returns clientId if found, otherwise undefined */ const getStaticOidcClientId = (issuer: string, staticOidcClients?: Record): string | undefined => { - console.log('hhh getStaticClient', issuer, staticOidcClients); // static_oidc_clients are configured with a trailing slash - const issuerWithTrailingSlash = issuer.endsWith('/') ? issuer : issuer + '/'; + const issuerWithTrailingSlash = issuer.endsWith("/") ? issuer : issuer + "/"; return staticOidcClients?.[issuerWithTrailingSlash]; }; diff --git a/test/components/structures/auth/Login-test.tsx b/test/components/structures/auth/Login-test.tsx index 9ae8d09db1e..e6744b2ac8f 100644 --- a/test/components/structures/auth/Login-test.tsx +++ b/test/components/structures/auth/Login-test.tsx @@ -409,7 +409,7 @@ describe("Login", function () { }); // short term during active development, UI will be added in next PRs - it("should show error when oidc native flow is correctly configured but not supported by UI", async () => { + it("should show continue button when oidc native flow is correctly configured", async () => { fetchMock.post(delegatedAuth.registrationEndpoint, { client_id: "abc123" }); getComponent(hsUrl, isUrl, delegatedAuth); @@ -417,10 +417,7 @@ describe("Login", function () { // did not continue with matrix login expect(mockClient.loginFlows).not.toHaveBeenCalled(); - // no oidc native UI yet - expect( - screen.getByText("This homeserver doesn't offer any login flows which are supported by this client."), - ).toBeInTheDocument(); + expect(screen.getByText("Continue")).toBeInTheDocument(); }); /** From 551670f6580c9bda52a57a3325df336ad1fc56d9 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 15 Jun 2023 19:38:00 +1200 Subject: [PATCH 21/26] test --- src/utils/oidc/authorize.ts | 4 +- test/utils/oidc/authorize-test.ts | 152 ++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 test/utils/oidc/authorize-test.ts diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 80427f05ffe..68b45cad2e3 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -56,12 +56,12 @@ const generateCodeChallenge = async (codeVerifier: string): Promise => { * @param clientId this client's id as registered with configured issuer * @param homeserver target homeserver */ -const storeAuthorizationParams = async ( +const storeAuthorizationParams = ( { redirectUri, state, nonce, codeVerifier }: AuthorizationParams, { issuer }: ValidatedServerConfig["delegatedAuthentication"], clientId: string, homeserver: string, -): Promise => { +): void => { window.sessionStorage.setItem(`oidc_${state}_nonce`, nonce); window.sessionStorage.setItem(`oidc_${state}_redirectUri`, redirectUri); window.sessionStorage.setItem(`oidc_${state}_codeVerifier`, codeVerifier); diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts new file mode 100644 index 00000000000..5b1898e9c4b --- /dev/null +++ b/test/utils/oidc/authorize-test.ts @@ -0,0 +1,152 @@ +/* +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 fetchMockJest from "fetch-mock-jest"; +import * as randomStringUtils from "matrix-js-sdk/src/randomstring"; +import { logger } from "matrix-js-sdk/src/logger"; + +import { startOidcLogin } from "../../../src/utils/oidc/authorize"; + +describe("startOidcLogin()", () => { + const issuer = "https://auth.com/"; + const authorizationEndpoint = "https://auth.com/authorization"; + const homeserver = "https://matrix.org"; + const clientId = "xyz789"; + const baseUrl = "https://test.com"; + + const delegatedAuthConfig = { + issuer, + registrationEndpoint: issuer + "registration", + authorizationEndpoint, + tokenEndpoint: issuer + "token", + }; + + const sessionStorageGetSpy = jest.spyOn(sessionStorage.__proto__, "setItem").mockReturnValue(undefined); + const randomStringMockImpl = (length) => new Array(length).fill("x").join(""); + + // to restore later + const realWindowLocation = window.location; + + beforeEach(() => { + fetchMockJest.mockClear(); + fetchMockJest.resetBehavior(); + + sessionStorageGetSpy.mockClear(); + + delete window.location; + // @ts-ignore ugly mocking + window.location = { + href: baseUrl, + origin: baseUrl, + }; + // @ts-ignore ugly mocking + window.crypto = undefined; + + jest.spyOn(randomStringUtils, "randomString").mockRestore(); + }); + + afterAll(() => { + window.location = realWindowLocation; + }); + + it("should store authorization params in session storage", async () => { + jest.spyOn(randomStringUtils, "randomString").mockReset().mockImplementation(randomStringMockImpl); + await startOidcLogin(delegatedAuthConfig, clientId, homeserver); + + const state = randomStringUtils.randomString(8); + + expect(sessionStorageGetSpy).toHaveBeenCalledWith(`oidc_${state}_nonce`, randomStringUtils.randomString(8)); + expect(sessionStorageGetSpy).toHaveBeenCalledWith(`oidc_${state}_redirectUri`, baseUrl); + expect(sessionStorageGetSpy).toHaveBeenCalledWith( + `oidc_${state}_codeVerifier`, + randomStringUtils.randomString(64), + ); + expect(sessionStorageGetSpy).toHaveBeenCalledWith(`oidc_${state}_clientId`, clientId); + expect(sessionStorageGetSpy).toHaveBeenCalledWith(`oidc_${state}_issuer`, issuer); + expect(sessionStorageGetSpy).toHaveBeenCalledWith(`oidc_${state}_homeserver`, homeserver); + }); + + it("navigates to authorization endpoint with correct parameters", async () => { + await startOidcLogin(delegatedAuthConfig, clientId, homeserver); + + const expectedScopeWithoutDeviceId = `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:`; + + const authUrl = new URL(window.location.href); + + expect(authUrl.searchParams.get("response_mode")).toEqual("fragment"); + expect(authUrl.searchParams.get("response_type")).toEqual("code"); + expect(authUrl.searchParams.get("client_id")).toEqual(clientId); + expect(authUrl.searchParams.get("code_challenge_method")).toEqual("S256"); + + // scope ends with a 10char randomstring deviceId + const scope = authUrl.searchParams.get("scope"); + expect(scope.substring(0, scope.length - 10)).toEqual(expectedScopeWithoutDeviceId); + expect(scope.substring(scope.length - 10)).toBeTruthy(); + + // random string, just check they are set + expect(authUrl.searchParams.has("state")).toBeTruthy(); + expect(authUrl.searchParams.has("nonce")).toBeTruthy(); + expect(authUrl.searchParams.has("code_challenge")).toBeTruthy(); + }); + + it("uses a plain text code challenge when crypto is not available", async () => { + jest.spyOn(logger, "warn"); + await startOidcLogin(delegatedAuthConfig, clientId, homeserver); + + expect(logger.warn).toHaveBeenCalledWith( + "A secure context is required to generate code challenge. Using plain text code challenge", + ); + + const authUrl = new URL(window.location.href); + + // compare the code challenge in url to the code verifier we stored in session storage + const codeChallenge = authUrl.searchParams.get("code_challenge"); + const codeVerifier = sessionStorageGetSpy.mock.calls.find(([key]) => + (key as string).includes("codeVerifier"), + )[1]; + expect(codeVerifier).toEqual(codeChallenge); + }); + + it("uses a s256 code challenge when crypto is available", async () => { + delete window.crypto; + window.crypto = { + // @ts-ignore mock + subtle: { + digest: jest.fn().mockReturnValue(new ArrayBuffer(32)), + }, + }; + + await startOidcLogin(delegatedAuthConfig, clientId, homeserver); + + expect(logger.warn).toHaveBeenCalledWith( + "A secure context is required to generate code challenge. Using plain text code challenge", + ); + + const authUrl = new URL(window.location.href); + + // compare the code challenge in url to the code verifier we stored in session storage + const codeChallenge = authUrl.searchParams.get("code_challenge"); + const codeVerifier = sessionStorageGetSpy.mock.calls.find(([key]) => + (key as string).includes("codeVerifier"), + )[1]; + + expect(window.crypto.subtle.digest).toHaveBeenCalledWith("SHA-256", expect.any(Object)); + + expect(codeVerifier).not.toEqual(codeChallenge); + // result from new ArrayBuffer(32) + expect(codeChallenge).toEqual("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); + }); +}); From b100d6d848f5638905a22ca23c0140a04a00779d Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 23 Jun 2023 12:34:27 +1200 Subject: [PATCH 22/26] adjust for js-sdk code --- src/utils/oidc/authorize.ts | 75 +++---------------------------- test/utils/oidc/authorize-test.ts | 51 --------------------- 2 files changed, 5 insertions(+), 121 deletions(-) diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 68b45cad2e3..4137b69188c 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -14,40 +14,9 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { logger } from "matrix-js-sdk/src/logger"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { generateAuthorizationParams, generateAuthorizationUrl } from "matrix-js-sdk/src/oidc/authorize"; -import { ValidatedServerConfig } from "../ValidatedServerConfig"; - -type AuthorizationParams = { - state: string; - scope: string; - redirectUri: string; - nonce?: string; - codeVerifier?: string; -}; - -const generateScope = (existingDeviceId?: string): string => { - const deviceId = existingDeviceId || randomString(10); - return `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${deviceId}`; -}; - -// https://www.rfc-editor.org/rfc/rfc7636 -const generateCodeChallenge = async (codeVerifier: string): Promise => { - if (!window.crypto || !window.crypto.subtle) { - // @TODO(kerrya) should this be allowed? configurable? - logger.warn("A secure context is required to generate code challenge. Using plain text code challenge"); - return codeVerifier; - } - const utf8 = new TextEncoder().encode(codeVerifier); - - const digest = await crypto.subtle.digest("SHA-256", utf8); - - return btoa(String.fromCharCode(...new Uint8Array(digest))) - .replace(/=/g, "") - .replace(/\+/g, "-") - .replace(/\//g, "_"); -}; +import { ValidatedDelegatedAuthConfig } from "../ValidatedServerConfig"; /** * Store authorization params for retrieval when returning from OIDC OP @@ -57,8 +26,8 @@ const generateCodeChallenge = async (codeVerifier: string): Promise => { * @param homeserver target homeserver */ const storeAuthorizationParams = ( - { redirectUri, state, nonce, codeVerifier }: AuthorizationParams, - { issuer }: ValidatedServerConfig["delegatedAuthentication"], + { redirectUri, state, nonce, codeVerifier }: ReturnType, + { issuer }: ValidatedDelegatedAuthConfig, clientId: string, homeserver: string, ): void => { @@ -70,40 +39,6 @@ const storeAuthorizationParams = ( window.sessionStorage.setItem(`oidc_${state}_homeserver`, homeserver); }; -const generateAuthorizationParams = ({ - deviceId, - redirectUri, -}: { - deviceId?: string; - redirectUri: string; -}): AuthorizationParams => ({ - scope: generateScope(deviceId), - redirectUri, - state: randomString(8), - nonce: randomString(8), - codeVerifier: randomString(64), // https://tools.ietf.org/html/rfc7636#section-4.1 length needs to be 43-128 characters -}); - -const generateAuthorizationUrl = async ( - authorizationUrl: string, - clientId: string, - { scope, redirectUri, state, nonce, codeVerifier }: AuthorizationParams, -): Promise => { - const url = new URL(authorizationUrl); - url.searchParams.append("response_mode", "fragment"); - url.searchParams.append("response_type", "code"); - url.searchParams.append("redirect_uri", redirectUri); - url.searchParams.append("client_id", clientId); - url.searchParams.append("state", state); - url.searchParams.append("scope", scope); - url.searchParams.append("nonce", nonce); - - url.searchParams.append("code_challenge_method", "S256"); - url.searchParams.append("code_challenge", await generateCodeChallenge(codeVerifier)); - - return url.toString(); -}; - /** * Start OIDC authorization code flow * Navigates to configured authorization endpoint @@ -112,7 +47,7 @@ const generateAuthorizationUrl = async ( * @param homeserver target homeserver */ export const startOidcLogin = async ( - delegatedAuthConfig: ValidatedServerConfig["delegatedAuthentication"], + delegatedAuthConfig: ValidatedDelegatedAuthConfig, clientId: string, homeserver: string, ): Promise => { diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index 5b1898e9c4b..74c27f71676 100644 --- a/test/utils/oidc/authorize-test.ts +++ b/test/utils/oidc/authorize-test.ts @@ -16,7 +16,6 @@ limitations under the License. import fetchMockJest from "fetch-mock-jest"; import * as randomStringUtils from "matrix-js-sdk/src/randomstring"; -import { logger } from "matrix-js-sdk/src/logger"; import { startOidcLogin } from "../../../src/utils/oidc/authorize"; @@ -52,8 +51,6 @@ describe("startOidcLogin()", () => { href: baseUrl, origin: baseUrl, }; - // @ts-ignore ugly mocking - window.crypto = undefined; jest.spyOn(randomStringUtils, "randomString").mockRestore(); }); @@ -101,52 +98,4 @@ describe("startOidcLogin()", () => { expect(authUrl.searchParams.has("nonce")).toBeTruthy(); expect(authUrl.searchParams.has("code_challenge")).toBeTruthy(); }); - - it("uses a plain text code challenge when crypto is not available", async () => { - jest.spyOn(logger, "warn"); - await startOidcLogin(delegatedAuthConfig, clientId, homeserver); - - expect(logger.warn).toHaveBeenCalledWith( - "A secure context is required to generate code challenge. Using plain text code challenge", - ); - - const authUrl = new URL(window.location.href); - - // compare the code challenge in url to the code verifier we stored in session storage - const codeChallenge = authUrl.searchParams.get("code_challenge"); - const codeVerifier = sessionStorageGetSpy.mock.calls.find(([key]) => - (key as string).includes("codeVerifier"), - )[1]; - expect(codeVerifier).toEqual(codeChallenge); - }); - - it("uses a s256 code challenge when crypto is available", async () => { - delete window.crypto; - window.crypto = { - // @ts-ignore mock - subtle: { - digest: jest.fn().mockReturnValue(new ArrayBuffer(32)), - }, - }; - - await startOidcLogin(delegatedAuthConfig, clientId, homeserver); - - expect(logger.warn).toHaveBeenCalledWith( - "A secure context is required to generate code challenge. Using plain text code challenge", - ); - - const authUrl = new URL(window.location.href); - - // compare the code challenge in url to the code verifier we stored in session storage - const codeChallenge = authUrl.searchParams.get("code_challenge"); - const codeVerifier = sessionStorageGetSpy.mock.calls.find(([key]) => - (key as string).includes("codeVerifier"), - )[1]; - - expect(window.crypto.subtle.digest).toHaveBeenCalledWith("SHA-256", expect.any(Object)); - - expect(codeVerifier).not.toEqual(codeChallenge); - // result from new ArrayBuffer(32) - expect(codeChallenge).toEqual("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); - }); }); From 72ff46a30722c7f2b6a0fd31434c962d9e8a1afa Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 23 Jun 2023 16:36:40 +1200 Subject: [PATCH 23/26] update test for response_mode query --- test/utils/oidc/authorize-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index 74c27f71676..569b226ab90 100644 --- a/test/utils/oidc/authorize-test.ts +++ b/test/utils/oidc/authorize-test.ts @@ -83,7 +83,7 @@ describe("startOidcLogin()", () => { const authUrl = new URL(window.location.href); - expect(authUrl.searchParams.get("response_mode")).toEqual("fragment"); + expect(authUrl.searchParams.get("response_mode")).toEqual("query"); expect(authUrl.searchParams.get("response_type")).toEqual("code"); expect(authUrl.searchParams.get("client_id")).toEqual(clientId); expect(authUrl.searchParams.get("code_challenge_method")).toEqual("S256"); From 857df8d14be7869b967bb245bd347bf9ee139726 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 26 Jun 2023 14:15:28 +1200 Subject: [PATCH 24/26] use new types --- src/utils/oidc/authorize.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 4137b69188c..08be4fca175 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -14,7 +14,11 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { generateAuthorizationParams, generateAuthorizationUrl } from "matrix-js-sdk/src/oidc/authorize"; +import { + AuthorizationParams, + generateAuthorizationParams, + generateAuthorizationUrl, +} from "matrix-js-sdk/src/oidc/authorize"; import { ValidatedDelegatedAuthConfig } from "../ValidatedServerConfig"; @@ -26,7 +30,7 @@ import { ValidatedDelegatedAuthConfig } from "../ValidatedServerConfig"; * @param homeserver target homeserver */ const storeAuthorizationParams = ( - { redirectUri, state, nonce, codeVerifier }: ReturnType, + { redirectUri, state, nonce, codeVerifier }: AuthorizationParams, { issuer }: ValidatedDelegatedAuthConfig, clientId: string, homeserver: string, From 599baa2b98300aef00948b98fabbbf390b58c71c Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 27 Jun 2023 15:59:28 +1200 Subject: [PATCH 25/26] strict --- test/utils/oidc/authorize-test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index 569b226ab90..5abdb19862a 100644 --- a/test/utils/oidc/authorize-test.ts +++ b/test/utils/oidc/authorize-test.ts @@ -34,7 +34,7 @@ describe("startOidcLogin()", () => { }; const sessionStorageGetSpy = jest.spyOn(sessionStorage.__proto__, "setItem").mockReturnValue(undefined); - const randomStringMockImpl = (length) => new Array(length).fill("x").join(""); + const randomStringMockImpl = (length: number) => new Array(length).fill("x").join(""); // to restore later const realWindowLocation = window.location; @@ -45,6 +45,7 @@ describe("startOidcLogin()", () => { sessionStorageGetSpy.mockClear(); + // @ts-ignore allow delete of non-optional prop delete window.location; // @ts-ignore ugly mocking window.location = { @@ -89,7 +90,7 @@ describe("startOidcLogin()", () => { expect(authUrl.searchParams.get("code_challenge_method")).toEqual("S256"); // scope ends with a 10char randomstring deviceId - const scope = authUrl.searchParams.get("scope"); + const scope = authUrl.searchParams.get("scope")!; expect(scope.substring(0, scope.length - 10)).toEqual(expectedScopeWithoutDeviceId); expect(scope.substring(scope.length - 10)).toBeTruthy(); From 88e0cf2cfa7021603f65d954f8499badeeea55ff Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 28 Jun 2023 15:05:30 +1200 Subject: [PATCH 26/26] tidy --- src/utils/oidc/authorize.ts | 8 +++++--- src/utils/oidc/error.ts | 21 --------------------- 2 files changed, 5 insertions(+), 24 deletions(-) delete mode 100644 src/utils/oidc/error.ts diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 08be4fca175..22e7a11bce1 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -24,8 +24,8 @@ import { ValidatedDelegatedAuthConfig } from "../ValidatedServerConfig"; /** * Store authorization params for retrieval when returning from OIDC OP - * @param { AuthorizationParams } authorizationParams - * @param { ValidatedServerConfig["delegatedAuthentication"] } delegatedAuthConfig used for future interactions with OP + * @param authorizationParams from `generateAuthorizationParams` + * @param delegatedAuthConfig used for future interactions with OP * @param clientId this client's id as registered with configured issuer * @param homeserver target homeserver */ @@ -45,17 +45,19 @@ const storeAuthorizationParams = ( /** * Start OIDC authorization code flow + * Generates auth params, stores them in session storage and * Navigates to configured authorization endpoint * @param delegatedAuthConfig from discovery * @param clientId this client's id as registered with configured issuer * @param homeserver target homeserver + * @returns Promise that resolves after we have navigated to auth endpoint */ export const startOidcLogin = async ( delegatedAuthConfig: ValidatedDelegatedAuthConfig, clientId: string, homeserver: string, ): Promise => { - // TODO(kerrya) afterloginfragment + // TODO(kerrya) afterloginfragment https://github.com/vector-im/element-web/issues/25656 const redirectUri = window.location.origin; const authParams = generateAuthorizationParams({ redirectUri }); diff --git a/src/utils/oidc/error.ts b/src/utils/oidc/error.ts deleted file mode 100644 index 3604ee77c24..00000000000 --- a/src/utils/oidc/error.ts +++ /dev/null @@ -1,21 +0,0 @@ -/* -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. -*/ - -export enum OidcClientError { - DynamicRegistrationNotSupported = "Dynamic registration not supported", - DynamicRegistrationFailed = "Dynamic registration failed", - DynamicRegistrationInvalid = "Dynamic registration invalid response", -}