From 3463eb9f5088b4a9e916293703dc05e0397f6552 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 16 Jun 2023 14:49:43 +1200 Subject: [PATCH 1/5] rename OidcDiscoveryError to OidcError --- spec/unit/autodiscovery.spec.ts | 12 ++++++------ spec/unit/oidc/validate.spec.ts | 21 +++++++++------------ src/autodiscovery.ts | 14 +++++--------- src/oidc/error.ts | 22 ++++++++++++++++++++++ src/oidc/validate.ts | 16 +++++----------- 5 files changed, 47 insertions(+), 38 deletions(-) create mode 100644 src/oidc/error.ts diff --git a/spec/unit/autodiscovery.spec.ts b/spec/unit/autodiscovery.spec.ts index 2a8f080ed20..e2e2c30824f 100644 --- a/spec/unit/autodiscovery.spec.ts +++ b/spec/unit/autodiscovery.spec.ts @@ -18,7 +18,7 @@ limitations under the License. import MockHttpBackend from "matrix-mock-request"; import { AutoDiscovery } from "../../src/autodiscovery"; -import { OidcDiscoveryError } from "../../src/oidc/validate"; +import { OidcError } from "../../src/oidc/error"; describe("AutoDiscovery", function () { const getHttpBackend = (): MockHttpBackend => { @@ -400,7 +400,7 @@ describe("AutoDiscovery", function () { }, "m.authentication": { state: "IGNORE", - error: OidcDiscoveryError.NotSupported, + error: OidcError.NotSupported, }, }; @@ -441,7 +441,7 @@ describe("AutoDiscovery", function () { }, "m.authentication": { state: "IGNORE", - error: OidcDiscoveryError.NotSupported, + error: OidcError.NotSupported, }, }; @@ -485,7 +485,7 @@ describe("AutoDiscovery", function () { }, "m.authentication": { state: "FAIL_ERROR", - error: OidcDiscoveryError.Misconfigured, + error: OidcError.Misconfigured, }, }; @@ -719,7 +719,7 @@ describe("AutoDiscovery", function () { }, "m.authentication": { state: "IGNORE", - error: OidcDiscoveryError.NotSupported, + error: OidcError.NotSupported, }, }; @@ -775,7 +775,7 @@ describe("AutoDiscovery", function () { }, "m.authentication": { state: "IGNORE", - error: OidcDiscoveryError.NotSupported, + error: OidcError.NotSupported, }, }; diff --git a/spec/unit/oidc/validate.spec.ts b/spec/unit/oidc/validate.spec.ts index 2ad62afc327..d5091ed89f9 100644 --- a/spec/unit/oidc/validate.spec.ts +++ b/spec/unit/oidc/validate.spec.ts @@ -16,11 +16,8 @@ limitations under the License. import { M_AUTHENTICATION } from "../../../src"; import { logger } from "../../../src/logger"; -import { - OidcDiscoveryError, - validateOIDCIssuerWellKnown, - validateWellKnownAuthentication, -} from "../../../src/oidc/validate"; +import { validateOIDCIssuerWellKnown, validateWellKnownAuthentication } from "../../../src/oidc/validate"; +import { OidcError } from "../../../src/oidc/error"; describe("validateWellKnownAuthentication()", () => { const baseWk = { @@ -29,7 +26,7 @@ describe("validateWellKnownAuthentication()", () => { }, }; it("should throw not supported error when wellKnown has no m.authentication section", () => { - expect(() => validateWellKnownAuthentication(baseWk)).toThrow(OidcDiscoveryError.NotSupported); + expect(() => validateWellKnownAuthentication(baseWk)).toThrow(OidcError.NotSupported); }); it("should throw misconfigured error when authentication issuer is not a string", () => { @@ -39,7 +36,7 @@ describe("validateWellKnownAuthentication()", () => { issuer: { url: "test.com" }, }, }; - expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcDiscoveryError.Misconfigured); + expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcError.Misconfigured); }); it("should throw misconfigured error when authentication account is not a string", () => { @@ -50,7 +47,7 @@ describe("validateWellKnownAuthentication()", () => { account: { url: "test" }, }, }; - expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcDiscoveryError.Misconfigured); + expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcError.Misconfigured); }); it("should throw misconfigured error when authentication account is false", () => { @@ -61,7 +58,7 @@ describe("validateWellKnownAuthentication()", () => { account: false, }, }; - expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcDiscoveryError.Misconfigured); + expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcError.Misconfigured); }); it("should return valid config when wk uses stable m.authentication", () => { @@ -137,7 +134,7 @@ describe("validateOIDCIssuerWellKnown", () => { it("should throw OP support error when wellKnown is not an object", () => { expect(() => { validateOIDCIssuerWellKnown([]); - }).toThrow(OidcDiscoveryError.OpSupport); + }).toThrow(OidcError.OpSupport); expect(logger.error).toHaveBeenCalledWith("Issuer configuration not found or malformed"); }); @@ -148,7 +145,7 @@ describe("validateOIDCIssuerWellKnown", () => { authorization_endpoint: undefined, response_types_supported: [], }); - }).toThrow(OidcDiscoveryError.OpSupport); + }).toThrow(OidcError.OpSupport); expect(logger.error).toHaveBeenCalledWith("OIDC issuer configuration: authorization_endpoint is invalid"); expect(logger.error).toHaveBeenCalledWith( "OIDC issuer configuration: response_types_supported is invalid. code is required.", @@ -194,6 +191,6 @@ describe("validateOIDCIssuerWellKnown", () => { ...validWk, [key]: value, }; - expect(() => validateOIDCIssuerWellKnown(wk)).toThrow(OidcDiscoveryError.OpSupport); + expect(() => validateOIDCIssuerWellKnown(wk)).toThrow(OidcError.OpSupport); }); }); diff --git a/src/autodiscovery.ts b/src/autodiscovery.ts index a43410bd630..4fad90c32f5 100644 --- a/src/autodiscovery.ts +++ b/src/autodiscovery.ts @@ -19,11 +19,11 @@ import { IClientWellKnown, IWellKnownConfig, IDelegatedAuthConfig, IServerVersio import { logger } from "./logger"; import { MatrixError, Method, timeoutSignal } from "./http-api"; import { - OidcDiscoveryError, ValidatedIssuerConfig, validateOIDCIssuerWellKnown, validateWellKnownAuthentication, } from "./oidc/validate"; +import { OidcError } from "./oidc/error"; // Dev note: Auto discovery is part of the spec. // See: https://matrix.org/docs/spec/client_server/r0.4.0.html#server-discovery @@ -297,7 +297,7 @@ export class AutoDiscovery { if (issuerWellKnown.action !== AutoDiscoveryAction.SUCCESS) { logger.error("Failed to fetch issuer openid configuration"); - throw new Error(OidcDiscoveryError.General); + throw new Error(OidcError.General); } const validatedIssuerConfig = validateOIDCIssuerWellKnown(issuerWellKnown.raw); @@ -310,15 +310,11 @@ export class AutoDiscovery { }; return delegatedAuthConfig; } catch (error) { - const errorMessage = (error as Error).message as unknown as OidcDiscoveryError; - const errorType = Object.values(OidcDiscoveryError).includes(errorMessage) - ? errorMessage - : OidcDiscoveryError.General; + const errorMessage = (error as Error).message as unknown as OidcError; + const errorType = Object.values(OidcError).includes(errorMessage) ? errorMessage : OidcError.General; const state = - errorType === OidcDiscoveryError.NotSupported - ? AutoDiscoveryAction.IGNORE - : AutoDiscoveryAction.FAIL_ERROR; + errorType === OidcError.NotSupported ? AutoDiscoveryAction.IGNORE : AutoDiscoveryAction.FAIL_ERROR; return { state, diff --git a/src/oidc/error.ts b/src/oidc/error.ts new file mode 100644 index 00000000000..ddec5ba6332 --- /dev/null +++ b/src/oidc/error.ts @@ -0,0 +1,22 @@ +/* +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 OidcError { + NotSupported = "OIDC authentication not supported", + Misconfigured = "OIDC is misconfigured", + General = "Something went wrong with OIDC discovery", + OpSupport = "Configured OIDC OP does not support required functions", +} diff --git a/src/oidc/validate.ts b/src/oidc/validate.ts index 1a5f672b4a7..1d3de910d4c 100644 --- a/src/oidc/validate.ts +++ b/src/oidc/validate.ts @@ -16,13 +16,7 @@ limitations under the License. import { IClientWellKnown, IDelegatedAuthConfig, M_AUTHENTICATION } from "../client"; import { logger } from "../logger"; - -export enum OidcDiscoveryError { - NotSupported = "OIDC authentication not supported", - Misconfigured = "OIDC is misconfigured", - General = "Something went wrong with OIDC discovery", - OpSupport = "Configured OIDC OP does not support required functions", -} +import { OidcError } from "./error"; export type ValidatedIssuerConfig = { authorizationEndpoint: string; @@ -41,7 +35,7 @@ export const validateWellKnownAuthentication = (wellKnown: IClientWellKnown): ID const authentication = M_AUTHENTICATION.findIn(wellKnown); if (!authentication) { - throw new Error(OidcDiscoveryError.NotSupported); + throw new Error(OidcError.NotSupported); } if ( @@ -54,7 +48,7 @@ export const validateWellKnownAuthentication = (wellKnown: IClientWellKnown): ID }; } - throw new Error(OidcDiscoveryError.Misconfigured); + throw new Error(OidcError.Misconfigured); }; const isRecord = (value: unknown): value is Record => @@ -93,7 +87,7 @@ const requiredArrayValue = (wellKnown: Record, key: string, val export const validateOIDCIssuerWellKnown = (wellKnown: unknown): ValidatedIssuerConfig => { if (!isRecord(wellKnown)) { logger.error("Issuer configuration not found or malformed"); - throw new Error(OidcDiscoveryError.OpSupport); + throw new Error(OidcError.OpSupport); } const isInvalid = [ @@ -114,5 +108,5 @@ export const validateOIDCIssuerWellKnown = (wellKnown: unknown): ValidatedIssuer } logger.error("Issuer configuration not valid"); - throw new Error(OidcDiscoveryError.OpSupport); + throw new Error(OidcError.OpSupport); }; From 4b0e2e614cf8ec56f31a275fb03d40912897ce69 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 16 Jun 2023 14:55:16 +1200 Subject: [PATCH 2/5] oidc client registration functions --- src/autodiscovery.ts | 6 +-- src/oidc/error.ts | 3 ++ src/oidc/register.ts | 111 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 src/oidc/register.ts diff --git a/src/autodiscovery.ts b/src/autodiscovery.ts index 4fad90c32f5..f9cf0398c2b 100644 --- a/src/autodiscovery.ts +++ b/src/autodiscovery.ts @@ -18,11 +18,7 @@ limitations under the License. import { IClientWellKnown, IWellKnownConfig, IDelegatedAuthConfig, IServerVersions, M_AUTHENTICATION } from "./client"; import { logger } from "./logger"; import { MatrixError, Method, timeoutSignal } from "./http-api"; -import { - ValidatedIssuerConfig, - validateOIDCIssuerWellKnown, - validateWellKnownAuthentication, -} from "./oidc/validate"; +import { ValidatedIssuerConfig, validateOIDCIssuerWellKnown, validateWellKnownAuthentication } from "./oidc/validate"; import { OidcError } from "./oidc/error"; // Dev note: Auto discovery is part of the spec. diff --git a/src/oidc/error.ts b/src/oidc/error.ts index ddec5ba6332..b77fbbf75f5 100644 --- a/src/oidc/error.ts +++ b/src/oidc/error.ts @@ -19,4 +19,7 @@ export enum OidcError { Misconfigured = "OIDC is misconfigured", General = "Something went wrong with OIDC discovery", OpSupport = "Configured OIDC OP does not support required functions", + DynamicRegistrationNotSupported = "Dynamic registration not supported", + DynamicRegistrationFailed = "Dynamic registration failed", + DynamicRegistrationInvalid = "Dynamic registration invalid response", } diff --git a/src/oidc/register.ts b/src/oidc/register.ts new file mode 100644 index 00000000000..c09517ba09d --- /dev/null +++ b/src/oidc/register.ts @@ -0,0 +1,111 @@ +/* +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 "../client"; +import { OidcError } from "./error"; +import { Method } from "../http-api"; +import { logger } from "../logger"; +import { ValidatedIssuerConfig } from "./validate"; + +/** + * Client metadata passed to registration endpoint + */ +export type OidcRegistrationClientMetadata = { + clientName: string; + clientUri: string; + redirectUris: string[]; +}; + +/** + * Make the client registration request + * @param registrationEndpoint - URL as returned from issuer ./well-known/openid-configuration + * @param clientMetadata - registration metadata + * @returns resolves to the registered client id when registration is successful + * @throws when registration request fails, or response is invalid + */ +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", + }; + + try { + const response = await fetch(registrationEndpoint, { + method: Method.Post, + headers, + body: JSON.stringify(metadata), + }); + + if (response.status >= 400) { + throw new Error(OidcError.DynamicRegistrationFailed); + } + + const body = await response.json(); + const clientId = body["client_id"]; + if (!clientId || typeof clientId !== "string") { + throw new Error(OidcError.DynamicRegistrationInvalid); + } + + return clientId; + } catch (error) { + if (Object.values(OidcError).includes((error as Error).message as OidcError)) { + throw error; + } else { + logger.error("Dynamic registration request failed", error); + throw new Error(OidcError.DynamicRegistrationFailed); + } + } +}; + +/** + * 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 when registration is not supported, on failed request or invalid response + */ +export const registerOidcClient = async ( + delegatedAuthConfig: IDelegatedAuthConfig & ValidatedIssuerConfig, + clientName: string, + baseUrl: string, +): Promise => { + const clientMetadata = { + clientName, + clientUri: baseUrl, + redirectUris: [baseUrl], + }; + if (!delegatedAuthConfig.registrationEndpoint) { + throw new Error(OidcError.DynamicRegistrationNotSupported); + } + const clientId = await doRegistration(delegatedAuthConfig.registrationEndpoint, clientMetadata); + + return clientId; +}; From 4f532f8997b6dd14788d12b3b7dd4eb72222e8f6 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 16 Jun 2023 15:00:23 +1200 Subject: [PATCH 3/5] test registerOidcClient --- spec/unit/oidc/register.spec.ts | 87 +++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 spec/unit/oidc/register.spec.ts diff --git a/spec/unit/oidc/register.spec.ts b/spec/unit/oidc/register.spec.ts new file mode 100644 index 00000000000..be551514a1a --- /dev/null +++ b/spec/unit/oidc/register.spec.ts @@ -0,0 +1,87 @@ +/* +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 { OidcError } from "../../../src/oidc/error"; +import { registerOidcClient } from "../../../src/oidc/register"; + +fetchMockJest; + +describe("registerOidcClient()", () => { + const issuer = "https://auth.com/"; + const registrationEndpoint = "https://auth.com/register"; + const clientName = "Element"; + const baseUrl = "https://just.testing"; + const dynamicClientId = "xyz789"; + + const delegatedAuthConfig = { + issuer, + registrationEndpoint, + authorizationEndpoint: issuer + "auth", + tokenEndpoint: issuer + "token", + }; + beforeEach(() => { + fetchMockJest.mockClear(); + fetchMockJest.resetBehavior(); + }); + + it("should make correct request to register client", async () => { + fetchMockJest.post(registrationEndpoint, { + status: 200, + body: JSON.stringify({ client_id: dynamicClientId }), + }); + expect(await registerOidcClient(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(() => registerOidcClient(delegatedAuthConfig, clientName, baseUrl)).rejects.toThrow( + OidcError.DynamicRegistrationFailed, + ); + }); + + it("should throw when registration response is invalid", async () => { + fetchMockJest.post(registrationEndpoint, { + status: 200, + // no clientId in response + body: "{}", + }); + expect(() => registerOidcClient(delegatedAuthConfig, clientName, baseUrl)).rejects.toThrow( + OidcError.DynamicRegistrationInvalid, + ); + }); +}); From 71e644696ae7e185d509df2fd33d50c4e4bba678 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 16 Jun 2023 15:02:33 +1200 Subject: [PATCH 4/5] tidy test file --- spec/unit/oidc/register.spec.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/spec/unit/oidc/register.spec.ts b/spec/unit/oidc/register.spec.ts index be551514a1a..e9ad60463a7 100644 --- a/spec/unit/oidc/register.spec.ts +++ b/spec/unit/oidc/register.spec.ts @@ -19,8 +19,6 @@ import fetchMockJest from "fetch-mock-jest"; import { OidcError } from "../../../src/oidc/error"; import { registerOidcClient } from "../../../src/oidc/register"; -fetchMockJest; - describe("registerOidcClient()", () => { const issuer = "https://auth.com/"; const registrationEndpoint = "https://auth.com/register"; @@ -45,7 +43,6 @@ describe("registerOidcClient()", () => { body: JSON.stringify({ client_id: dynamicClientId }), }); expect(await registerOidcClient(delegatedAuthConfig, clientName, baseUrl)).toEqual(dynamicClientId); - // didn't try to register expect(fetchMockJest).toHaveBeenCalledWith(registrationEndpoint, { headers: { "Accept": "application/json", From 0d0a3156e3673e37a890cb743ed3d50e3557e54a Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 21 Jun 2023 08:20:54 +1200 Subject: [PATCH 5/5] reexport OidcDiscoveryError for backwards compatibility --- src/oidc/validate.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/oidc/validate.ts b/src/oidc/validate.ts index 1d3de910d4c..09ecf5e609d 100644 --- a/src/oidc/validate.ts +++ b/src/oidc/validate.ts @@ -18,6 +18,12 @@ import { IClientWellKnown, IDelegatedAuthConfig, M_AUTHENTICATION } from "../cli import { logger } from "../logger"; import { OidcError } from "./error"; +/** + * re-export for backwards compatibility + * @deprecated use OidcError + */ +export { OidcError as OidcDiscoveryError }; + export type ValidatedIssuerConfig = { authorizationEndpoint: string; tokenEndpoint: string;