OIDC: use oidc-client-ts (#3544)

* use oidc-client-ts during oidc discovery

* export new type for auth config

* deprecate generateAuthorizationUrl in favour of generateOidcAuthorizationUrl

* testing util for oidc configurations

* test generateOidcAuthorizationUrl

* lint

* test discovery

* dont pass whole client wellknown to oidc validation funcs

* add nonce

* use client userState for homeserver

spec/unit/autodiscovery.spec.ts

@@ -15,10 +15,17 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+import fetchMock from "fetch-mock-jest";
 import MockHttpBackend from "matrix-mock-request";
 
+import { M_AUTHENTICATION } from "../../src";
 import { AutoDiscovery } from "../../src/autodiscovery";
 import { OidcError } from "../../src/oidc/error";
+import { makeDelegatedAuthConfig } from "../test-utils/oidc";
+
+// keep to reset the fetch function after using MockHttpBackend
+// @ts-ignore private property
+const realAutoDiscoveryFetch: typeof global.fetch = AutoDiscovery.fetchFn;
 
 describe("AutoDiscovery", function () {
     const getHttpBackend = (): MockHttpBackend => {
@@ -27,6 +34,10 @@ describe("AutoDiscovery", function () {
         return httpBackend;
     };
 
+    afterAll(() => {
+        AutoDiscovery.setFetchFn(realAutoDiscoveryFetch);
+    });
+
     it("should throw an error when no domain is specified", function () {
         getHttpBackend();
         return Promise.all([
@@ -855,4 +866,75 @@ describe("AutoDiscovery", function () {
             }),
         ]);
     });
+
+    describe("m.authentication", () => {
+        const homeserverName = "example.org";
+        const homeserverUrl = "https://chat.example.org/";
+        const issuer = "https://auth.org/";
+
+        beforeAll(() => {
+            // make these tests independent from fetch mocking above
+            AutoDiscovery.setFetchFn(realAutoDiscoveryFetch);
+        });
+
+        beforeEach(() => {
+            fetchMock.resetBehavior();
+            fetchMock.get(`${homeserverUrl}_matrix/client/versions`, { versions: ["r0.0.1"] });
+
+            fetchMock.get("https://example.org/.well-known/matrix/client", {
+                "m.homeserver": {
+                    // Note: we also expect this test to trim the trailing slash
+                    base_url: "https://chat.example.org/",
+                },
+                "m.authentication": {
+                    issuer,
+                },
+            });
+        });
+
+        it("should return valid authentication configuration", async () => {
+            const config = makeDelegatedAuthConfig(issuer);
+
+            fetchMock.get(`${config.metadata.issuer}.well-known/openid-configuration`, config.metadata);
+            fetchMock.get(`${config.metadata.issuer}jwks`, {
+                status: 200,
+                headers: {
+                    "Content-Type": "application/json",
+                },
+                keys: [],
+            });
+
+            const result = await AutoDiscovery.findClientConfig(homeserverName);
+
+            expect(result[M_AUTHENTICATION.stable!]).toEqual({
+                state: AutoDiscovery.SUCCESS,
+                ...config,
+                signingKeys: [],
+                account: undefined,
+                error: null,
+            });
+        });
+
+        it("should set state to error for invalid authentication configuration", async () => {
+            const config = makeDelegatedAuthConfig(issuer);
+            // authorization_code is required
+            config.metadata.grant_types_supported = ["openid"];
+
+            fetchMock.get(`${config.metadata.issuer}.well-known/openid-configuration`, config.metadata);
+            fetchMock.get(`${config.metadata.issuer}jwks`, {
+                status: 200,
+                headers: {
+                    "Content-Type": "application/json",
+                },
+                keys: [],
+            });
+
+            const result = await AutoDiscovery.findClientConfig(homeserverName);
+
+            expect(result[M_AUTHENTICATION.stable!]).toEqual({
+                state: AutoDiscovery.FAIL_ERROR,
+                error: OidcError.OpSupport,
+            });
+        });
+    });
 });

spec/unit/oidc/authorize.spec.ts

@@ -1,3 +1,7 @@
+/**
+ * @jest-environment jsdom
+ */
+
 /*
 Copyright 2023 The Matrix.org Foundation C.I.C.
 
@@ -25,8 +29,10 @@ import {
     completeAuthorizationCodeGrant,
     generateAuthorizationParams,
     generateAuthorizationUrl,
+    generateOidcAuthorizationUrl,
 } from "../../../src/oidc/authorize";
 import { OidcError } from "../../../src/oidc/error";
+import { makeDelegatedAuthConfig, mockOpenIdConfiguration } from "../../test-utils/oidc";
 
 jest.mock("jwt-decode");
 
@@ -34,20 +40,16 @@ jest.mock("jwt-decode");
 const realSubtleCrypto = crypto.subtleCrypto;
 
 describe("oidc authorization", () => {
-    const issuer = "https://auth.com/";
-    const authorizationEndpoint = "https://auth.com/authorization";
-    const tokenEndpoint = "https://auth.com/token";
-    const delegatedAuthConfig = {
-        issuer,
-        registrationEndpoint: issuer + "registration",
-        authorizationEndpoint: issuer + "auth",
-        tokenEndpoint,
-    };
+    const delegatedAuthConfig = makeDelegatedAuthConfig();
+    const authorizationEndpoint = delegatedAuthConfig.metadata.authorization_endpoint;
+    const tokenEndpoint = delegatedAuthConfig.metadata.token_endpoint;
     const clientId = "xyz789";
     const baseUrl = "https://test.com";
 
     beforeAll(() => {
         jest.spyOn(logger, "warn");
+
+        fetchMock.get(delegatedAuthConfig.issuer + ".well-known/openid-configuration", mockOpenIdConfiguration());
     });
 
     afterEach(() => {
@@ -97,20 +99,36 @@ describe("oidc authorization", () => {
                 "A secure context is required to generate code challenge. Using plain text code challenge",
             );
         });
+    });
+
+    describe("generateOidcAuthorizationUrl()", () => {
+        it("should generate url with correct parameters", async () => {
+            const nonce = "abc123";
+
+            const metadata = delegatedAuthConfig.metadata;
 
-        it("uses a s256 code challenge when crypto is available", async () => {
-            jest.spyOn(crypto.subtleCrypto, "digest");
-            const authorizationParams = generateAuthorizationParams({ redirectUri: baseUrl });
             const authUrl = new URL(
-                await generateAuthorizationUrl(authorizationEndpoint, clientId, authorizationParams),
+                await generateOidcAuthorizationUrl({
+                    metadata,
+                    homeserverUrl: baseUrl,
+                    clientId,
+                    redirectUri: baseUrl,
+                    nonce,
+                }),
             );
 
-            const codeChallenge = authUrl.searchParams.get("code_challenge");
-            expect(crypto.subtleCrypto.digest).toHaveBeenCalledWith("SHA-256", expect.any(Object));
+            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");
+            // scope minus the 10char random device id at the end
+            expect(authUrl.searchParams.get("scope")!.slice(0, -10)).toEqual(
+                "openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:",
+            );
+            expect(authUrl.searchParams.get("state")).toBeTruthy();
+            expect(authUrl.searchParams.get("nonce")).toEqual(nonce);
 
-            // didn't use plain text code challenge
-            expect(authorizationParams.codeVerifier).not.toEqual(codeChallenge);
-            expect(codeChallenge).toBeTruthy();
+            expect(authUrl.searchParams.get("code_challenge")).toBeTruthy();
         });
     });
 

spec/unit/oidc/validate.spec.ts

@@ -35,7 +35,7 @@ describe("validateWellKnownAuthentication()", () => {
         },
     };
     it("should throw not supported error when wellKnown has no m.authentication section", () => {
-        expect(() => validateWellKnownAuthentication(baseWk)).toThrow(OidcError.NotSupported);
+        expect(() => validateWellKnownAuthentication(undefined)).toThrow(OidcError.NotSupported);
     });
 
     it("should throw misconfigured error when authentication issuer is not a string", () => {
@@ -45,7 +45,9 @@ describe("validateWellKnownAuthentication()", () => {
                 issuer: { url: "test.com" },
             },
         };
-        expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcError.Misconfigured);
+        expect(() => validateWellKnownAuthentication(wk[M_AUTHENTICATION.stable!] as any)).toThrow(
+            OidcError.Misconfigured,
+        );
     });
 
     it("should throw misconfigured error when authentication account is not a string", () => {
@@ -56,7 +58,9 @@ describe("validateWellKnownAuthentication()", () => {
                 account: { url: "test" },
             },
         };
-        expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcError.Misconfigured);
+        expect(() => validateWellKnownAuthentication(wk[M_AUTHENTICATION.stable!] as any)).toThrow(
+            OidcError.Misconfigured,
+        );
     });
 
     it("should throw misconfigured error when authentication account is false", () => {
@@ -67,7 +71,9 @@ describe("validateWellKnownAuthentication()", () => {
                 account: false,
             },
         };
-        expect(() => validateWellKnownAuthentication(wk)).toThrow(OidcError.Misconfigured);
+        expect(() => validateWellKnownAuthentication(wk[M_AUTHENTICATION.stable!] as any)).toThrow(
+            OidcError.Misconfigured,
+        );
     });
 
     it("should return valid config when wk uses stable m.authentication", () => {
@@ -78,7 +84,7 @@ describe("validateWellKnownAuthentication()", () => {
                 account: "account.com",
             },
         };
-        expect(validateWellKnownAuthentication(wk)).toEqual({
+        expect(validateWellKnownAuthentication(wk[M_AUTHENTICATION.stable!])).toEqual({
             issuer: "test.com",
             account: "account.com",
         });
@@ -91,7 +97,7 @@ describe("validateWellKnownAuthentication()", () => {
                 issuer: "test.com",
             },
         };
-        expect(validateWellKnownAuthentication(wk)).toEqual({
+        expect(validateWellKnownAuthentication(wk[M_AUTHENTICATION.stable!])).toEqual({
             issuer: "test.com",
         });
     });
@@ -104,24 +110,10 @@ describe("validateWellKnownAuthentication()", () => {
                 somethingElse: "test",
             },
         };
-        expect(validateWellKnownAuthentication(wk)).toEqual({
+        expect(validateWellKnownAuthentication(wk[M_AUTHENTICATION.stable!])).toEqual({
             issuer: "test.com",
         });
     });
-
-    it("should return valid config when wk uses unstable prefix for m.authentication", () => {
-        const wk = {
-            ...baseWk,
-            [M_AUTHENTICATION.unstable!]: {
-                issuer: "test.com",
-                account: "account.com",
-            },
-        };
-        expect(validateWellKnownAuthentication(wk)).toEqual({
-            issuer: "test.com",
-            account: "account.com",
-        });
-    });
 });
 
 describe("validateOIDCIssuerWellKnown", () => {
@@ -129,6 +121,7 @@ describe("validateOIDCIssuerWellKnown", () => {
         authorization_endpoint: "https://test.org/authorize",
         token_endpoint: "https://authorize.org/token",
         registration_endpoint: "https://authorize.org/regsiter",
+        revocation_endpoint: "https://authorize.org/regsiter",
         response_types_supported: ["code"],
         grant_types_supported: ["authorization_code"],
         code_challenge_methods_supported: ["S256"],