Switch OIDC primarily to new /auth_metadata API (#4626)

Author: t3chguy

Diff for: spec/integ/rendezvous/MSC4108SignInWithQR.spec.ts

@@ -36,7 +36,7 @@ import {
     MatrixError,
     MatrixHttpApi,
 } from "../../../src";
-import { mockOpenIdConfiguration } from "../../test-utils/oidc";
+import { makeDelegatedAuthConfig } from "../../test-utils/oidc";
 
 function makeMockClient(opts: { userId: string; deviceId: string; msc4108Enabled: boolean }): MatrixClient {
     const baseUrl = "https://example.com";
@@ -57,7 +57,7 @@ function makeMockClient(opts: { userId: string; deviceId: string; msc4108Enabled
         getDomain: () => "example.com",
         getDevice: jest.fn(),
         getCrypto: jest.fn(() => crypto),
-        getAuthIssuer: jest.fn().mockResolvedValue({ issuer: "https://issuer/" }),
+        getAuthMetadata: jest.fn().mockResolvedValue(makeDelegatedAuthConfig("https://issuer/", [DEVICE_CODE_SCOPE])),
     } as unknown as MatrixClient;
     client.http = new MatrixHttpApi<IHttpOpts & { onlyData: true }>(client, {
         baseUrl: client.baseUrl,
@@ -69,10 +69,6 @@ function makeMockClient(opts: { userId: string; deviceId: string; msc4108Enabled
 
 describe("MSC4108SignInWithQR", () => {
     beforeEach(() => {
-        fetchMock.get(
-            "https://issuer/.well-known/openid-configuration",
-            mockOpenIdConfiguration("https://issuer/", [DEVICE_CODE_SCOPE]),
-        );
         fetchMock.get("https://issuer/jwks", {
             status: 200,
             headers: {

Diff for: spec/test-utils/oidc.ts

@@ -14,42 +14,4 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { OidcClientConfig, ValidatedIssuerMetadata } from "../../src";
-
-/**
- * Makes a valid OidcClientConfig with minimum valid values
- * @param issuer used as the base for all other urls
- * @returns OidcClientConfig
- */
-export const makeDelegatedAuthConfig = (issuer = "https://auth.org/"): OidcClientConfig => {
-    const metadata = mockOpenIdConfiguration(issuer);
-
-    return {
-        accountManagementEndpoint: issuer + "account",
-        registrationEndpoint: metadata.registration_endpoint,
-        authorizationEndpoint: metadata.authorization_endpoint,
-        tokenEndpoint: metadata.token_endpoint,
-        metadata,
-    };
-};
-
-/**
- * Useful for mocking <issuer>/.well-known/openid-configuration
- * @param issuer used as the base for all other urls
- * @returns ValidatedIssuerMetadata
- */
-export const mockOpenIdConfiguration = (
-    issuer = "https://auth.org/",
-    additionalGrantTypes: string[] = [],
-): ValidatedIssuerMetadata => ({
-    issuer,
-    revocation_endpoint: issuer + "revoke",
-    token_endpoint: issuer + "token",
-    authorization_endpoint: issuer + "auth",
-    registration_endpoint: issuer + "registration",
-    device_authorization_endpoint: issuer + "device",
-    jwks_uri: issuer + "jwks",
-    response_types_supported: ["code"],
-    grant_types_supported: ["authorization_code", "refresh_token", ...additionalGrantTypes],
-    code_challenge_methods_supported: ["S256"],
-});
+export { makeDelegatedAuthConfig, mockOpenIdConfiguration } from "../../src/testing.ts";

Diff for: spec/unit/matrix-client.spec.ts

@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 import { Mocked, mocked } from "jest-mock";
+import fetchMock from "fetch-mock-jest";
 
 import { logger } from "../../src/logger";
 import { ClientEvent, IMatrixClientCreateOpts, ITurnServerResponse, MatrixClient, Store } from "../../src/client";
@@ -76,6 +77,7 @@ import { SecretStorageKeyDescriptionAesV1, ServerSideSecretStorageImpl } from ".
 import { CryptoBackend } from "../../src/common-crypto/CryptoBackend";
 import { KnownMembership } from "../../src/@types/membership";
 import { RoomMessageEventContent } from "../../src/@types/events";
+import { mockOpenIdConfiguration } from "../test-utils/oidc.ts";
 
 jest.useFakeTimers();
 
@@ -265,13 +267,17 @@ describe("MatrixClient", function () {
 
             if (next.error) {
                 // eslint-disable-next-line
-                return Promise.reject({
-                    errcode: (<MatrixError>next.error).errcode,
-                    httpStatus: (<MatrixError>next.error).httpStatus,
-                    name: (<MatrixError>next.error).errcode,
-                    message: "Expected testing error",
-                    data: next.error,
-                });
+                return Promise.reject(
+                    new MatrixError(
+                        {
+                            errcode: (<MatrixError>next.error).errcode,
+                            name: (<MatrixError>next.error).errcode,
+                            message: "Expected testing error",
+                            data: next.error,
+                        },
+                        (<MatrixError>next.error).httpStatus,
+                    ),
+                );
             }
             return Promise.resolve(next.data);
         }
@@ -3489,6 +3495,63 @@ describe("MatrixClient", function () {
         });
     });
 
+    describe("getAuthMetadata", () => {
+        beforeEach(() => {
+            fetchMock.mockReset();
+            // This request is made by oidc-client-ts so is not intercepted by httpLookups
+            fetchMock.get("https://auth.org/jwks", {
+                status: 200,
+                headers: {
+                    "Content-Type": "application/json",
+                },
+                keys: [],
+            });
+        });
+
+        it("should use unstable prefix", async () => {
+            const metadata = mockOpenIdConfiguration();
+            httpLookups = [
+                {
+                    method: "GET",
+                    path: `/auth_metadata`,
+                    data: metadata,
+                    prefix: "/_matrix/client/unstable/org.matrix.msc2965",
+                },
+            ];
+
+            await expect(client.getAuthMetadata()).resolves.toEqual({
+                ...metadata,
+                signingKeys: [],
+            });
+            expect(httpLookups.length).toEqual(0);
+        });
+
+        it("should fall back to auth_issuer + openid-configuration", async () => {
+            const metadata = mockOpenIdConfiguration();
+            httpLookups = [
+                {
+                    method: "GET",
+                    path: `/auth_metadata`,
+                    error: new MatrixError({ errcode: "M_UNRECOGNIZED" }, 404),
+                    prefix: "/_matrix/client/unstable/org.matrix.msc2965",
+                },
+                {
+                    method: "GET",
+                    path: `/auth_issuer`,
+                    data: { issuer: metadata.issuer },
+                    prefix: "/_matrix/client/unstable/org.matrix.msc2965",
+                },
+            ];
+            fetchMock.get("https://auth.org/.well-known/openid-configuration", metadata);
+
+            await expect(client.getAuthMetadata()).resolves.toEqual({
+                ...metadata,
+                signingKeys: [],
+            });
+            expect(httpLookups.length).toEqual(0);
+        });
+    });
+
     describe("identityHashedLookup", () => {
         it("should return hashed lookup results", async () => {
             const ID_ACCESS_TOKEN = "hello_id_server_please_let_me_make_a_request";

Diff for: spec/unit/oidc/authorize.spec.ts

@@ -40,8 +40,8 @@ jest.mock("jwt-decode");
 
 describe("oidc authorization", () => {
     const delegatedAuthConfig = makeDelegatedAuthConfig();
-    const authorizationEndpoint = delegatedAuthConfig.authorizationEndpoint;
-    const tokenEndpoint = delegatedAuthConfig.tokenEndpoint;
+    const authorizationEndpoint = delegatedAuthConfig.authorization_endpoint;
+    const tokenEndpoint = delegatedAuthConfig.token_endpoint;
     const clientId = "xyz789";
     const baseUrl = "https://test.com";
 
@@ -52,10 +52,7 @@ describe("oidc authorization", () => {
         jest.spyOn(logger, "warn");
         jest.setSystemTime(now);
 
-        fetchMock.get(
-            delegatedAuthConfig.metadata.issuer + ".well-known/openid-configuration",
-            mockOpenIdConfiguration(),
-        );
+        fetchMock.get(delegatedAuthConfig.issuer + ".well-known/openid-configuration", mockOpenIdConfiguration());
         globalThis.TextEncoder = TextEncoder;
     });
 
@@ -127,11 +124,9 @@ describe("oidc authorization", () => {
         it("should generate url with correct parameters", async () => {
             const nonce = "abc123";
 
-            const metadata = delegatedAuthConfig.metadata;
-
             const authUrl = new URL(
                 await generateOidcAuthorizationUrl({
-                    metadata,
+                    metadata: delegatedAuthConfig,
                     homeserverUrl: baseUrl,
                     clientId,
                     redirectUri: baseUrl,
@@ -156,11 +151,9 @@ describe("oidc authorization", () => {
         it("should generate url with create prompt", async () => {
             const nonce = "abc123";
 
-            const metadata = delegatedAuthConfig.metadata;
-
             const authUrl = new URL(
                 await generateOidcAuthorizationUrl({
-                    metadata,
+                    metadata: delegatedAuthConfig,
                     homeserverUrl: baseUrl,
                     clientId,
                     redirectUri: baseUrl,

Diff for: spec/unit/oidc/register.spec.ts

@@ -42,13 +42,13 @@ describe("registerOidcClient()", () => {
     });
 
     it("should make correct request to register client", async () => {
-        fetchMockJest.post(delegatedAuthConfig.registrationEndpoint!, {
+        fetchMockJest.post(delegatedAuthConfig.registration_endpoint!, {
             status: 200,
             body: JSON.stringify({ client_id: dynamicClientId }),
         });
         expect(await registerOidcClient(delegatedAuthConfig, metadata)).toEqual(dynamicClientId);
         expect(fetchMockJest).toHaveBeenCalledWith(
-            delegatedAuthConfig.registrationEndpoint!,
+            delegatedAuthConfig.registration_endpoint,
             expect.objectContaining({
                 headers: {
                     "Accept": "application/json",
@@ -72,7 +72,7 @@ describe("registerOidcClient()", () => {
     });
 
     it("should throw when registration request fails", async () => {
-        fetchMockJest.post(delegatedAuthConfig.registrationEndpoint!, {
+        fetchMockJest.post(delegatedAuthConfig.registration_endpoint!, {
             status: 500,
         });
         await expect(() => registerOidcClient(delegatedAuthConfig, metadata)).rejects.toThrow(
@@ -81,7 +81,7 @@ describe("registerOidcClient()", () => {
     });
 
     it("should throw when registration response is invalid", async () => {
-        fetchMockJest.post(delegatedAuthConfig.registrationEndpoint!, {
+        fetchMockJest.post(delegatedAuthConfig.registration_endpoint!, {
             status: 200,
             // no clientId in response
             body: "{}",
@@ -96,7 +96,7 @@ describe("registerOidcClient()", () => {
             registerOidcClient(
                 {
                     ...delegatedAuthConfig,
-                    registrationEndpoint: undefined,
+                    registration_endpoint: undefined,
                 },
                 metadata,
             ),
@@ -108,10 +108,7 @@ describe("registerOidcClient()", () => {
             registerOidcClient(
                 {
                     ...delegatedAuthConfig,
-                    metadata: {
-                        ...delegatedAuthConfig.metadata,
-                        grant_types_supported: [delegatedAuthConfig.metadata.grant_types_supported[0]],
-                    },
+                    grant_types_supported: [delegatedAuthConfig.grant_types_supported[0]],
                 },
                 metadata,
             ),

Diff for: spec/unit/oidc/tokenRefresher.spec.ts

@@ -55,16 +55,16 @@ describe("OidcTokenRefresher", () => {
     });
 
     beforeEach(() => {
-        fetchMock.get(`${config.metadata.issuer}.well-known/openid-configuration`, config.metadata);
-        fetchMock.get(`${config.metadata.issuer}jwks`, {
+        fetchMock.get(`${config.issuer}.well-known/openid-configuration`, config);
+        fetchMock.get(`${config.issuer}jwks`, {
             status: 200,
             headers: {
                 "Content-Type": "application/json",
             },
             keys: [],
         });
 
-        fetchMock.post(config.tokenEndpoint, {
+        fetchMock.post(config.token_endpoint, {
             status: 200,
             headers: {
                 "Content-Type": "application/json",
@@ -81,7 +81,7 @@ describe("OidcTokenRefresher", () => {
     it("throws when oidc client cannot be initialised", async () => {
         jest.spyOn(logger, "error");
         fetchMock.get(
-            `${config.metadata.issuer}.well-known/openid-configuration`,
+            `${config.issuer}.well-known/openid-configuration`,
             {
                 ok: false,
                 status: 404,
@@ -126,7 +126,7 @@ describe("OidcTokenRefresher", () => {
 
             const result = await refresher.doRefreshAccessToken("refresh-token");
 
-            expect(fetchMock).toHaveFetched(config.tokenEndpoint, {
+            expect(fetchMock).toHaveFetched(config.token_endpoint, {
                 method: "POST",
             });
 
@@ -153,7 +153,7 @@ describe("OidcTokenRefresher", () => {
         it("should only have one inflight refresh request at once", async () => {
             fetchMock
                 .postOnce(
-                    config.tokenEndpoint,
+                    config.token_endpoint,
                     {
                         status: 200,
                         headers: {
@@ -164,7 +164,7 @@ describe("OidcTokenRefresher", () => {
                     { overwriteRoutes: true },
                 )
                 .postOnce(
-                    config.tokenEndpoint,
+                    config.token_endpoint,
                     {
                         status: 200,
                         headers: {
@@ -188,7 +188,7 @@ describe("OidcTokenRefresher", () => {
             const result2 = await first;
 
             // only one call to token endpoint
-            expect(fetchMock).toHaveFetchedTimes(1, config.tokenEndpoint);
+            expect(fetchMock).toHaveFetchedTimes(1, config.token_endpoint);
             expect(result1).toEqual({
                 accessToken: "first-new-access-token",
                 refreshToken: "first-new-refresh-token",
@@ -208,7 +208,7 @@ describe("OidcTokenRefresher", () => {
 
         it("should log and rethrow when token refresh fails", async () => {
             fetchMock.post(
-                config.tokenEndpoint,
+                config.token_endpoint,
                 {
                     status: 503,
                     headers: {
@@ -228,7 +228,7 @@ describe("OidcTokenRefresher", () => {
             // make sure inflight request is cleared after a failure
             fetchMock
                 .postOnce(
-                    config.tokenEndpoint,
+                    config.token_endpoint,
                     {
                         status: 503,
                         headers: {
@@ -238,7 +238,7 @@ describe("OidcTokenRefresher", () => {
                     { overwriteRoutes: true },
                 )
                 .postOnce(
-                    config.tokenEndpoint,
+                    config.token_endpoint,
                     {
                         status: 200,
                         headers: {