OIDC: attempt dynamic client registration (#11074)

* add delegatedauthentication to validated server config * dynamic client registration functions * test OP registration functions * add stubbed nativeOidc flow setup in Login * cover more error cases in Login * tidy * test dynamic client registration in Login * comment oidc_static_clients * register oidc inside Login.getFlows * strict fixes * remove unused code * and imports * comments * comments 2 * util functions to get static client id * check static client ids in login flow * remove dead code * OidcRegistrationClientMetadata type * use registerClient from js-sdk * use OidcError from js-sdk
matrix-org · Jun 22, 2023 · 358c37a · 358c37a
1 parent 0eda8c1
commit 358c37a
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 46 deletions.
diff --git a/src/components/structures/auth/Login.tsx b/src/components/structures/auth/Login.tsx
@@ -50,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.

diff --git a/src/utils/oidc/error.ts b/src/utils/oidc/error.ts
diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts
@@ -15,9 +15,9 @@ limitations under the License.
 */
 
 import { logger } from "matrix-js-sdk/src/logger";
+import { registerOidcClient } from "matrix-js-sdk/src/oidc/register";
 
 import { ValidatedDelegatedAuthConfig } from "../ValidatedServerConfig";
-import { OidcClientError } from "./error";
 
 /**
  * Get the statically configured clientId for the issuer
@@ -34,6 +34,7 @@ const getStaticOidcClientId = (issuer: string, staticOidcClients?: Record<string
 /**
  * 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/'
@@ -44,17 +45,14 @@ const getStaticOidcClientId = (issuer: string, staticOidcClients?: Record<string
 export const getOidcClientId = async (
     delegatedAuthConfig: ValidatedDelegatedAuthConfig,
     // these are used in the following PR
-    _clientName: string,
-    _baseUrl: string,
+    clientName: string,
+    baseUrl: string,
     staticOidcClients?: Record<string, string>,
 ): Promise<string> => {
     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);
+    return await registerOidcClient(delegatedAuthConfig, clientName, baseUrl);
 };
diff --git a/test/components/structures/auth/Login-test.tsx b/test/components/structures/auth/Login-test.tsx
@@ -21,6 +21,7 @@ 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 { OidcError } from "matrix-js-sdk/src/oidc/error";
 
 import SdkConfig from "../../../../src/SdkConfig";
 import { mkServerConfig, mockPlatformPeg, unmockPlatformPeg } from "../../../test-utils";
@@ -30,7 +31,6 @@ import SettingsStore from "../../../../src/settings/SettingsStore";
 import { Features } from "../../../../src/settings/Settings";
 import { ValidatedDelegatedAuthConfig } 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");
 
@@ -365,6 +365,8 @@ describe("Login", function () {
 
             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
@@ -374,10 +376,13 @@ describe("Login", function () {
         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,
@@ -387,12 +392,15 @@ describe("Login", function () {
             );
         });
 
-        it("should fallback to normal login when client does not have static clientId", async () => {
+        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…"));
 
-            expect(logger.error).toHaveBeenCalledWith(new Error(OidcClientError.DynamicRegistrationNotSupported));
+            // tried to register
+            expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registrationEndpoint, expect.any(Object));
+            expect(logger.error).toHaveBeenCalledWith(new Error(OidcError.DynamicRegistrationFailed));
 
             // continued with normal setup
             expect(mockClient.loginFlows).toHaveBeenCalled();
@@ -402,11 +410,8 @@ 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 () => {
-            const delegatedAuthWithStaticClientId = {
-                ...delegatedAuth,
-                issuer: "https://staticallyregisteredissuer.org/",
-            };
-            getComponent(hsUrl, isUrl, delegatedAuthWithStaticClientId);
+            fetchMock.post(delegatedAuth.registrationEndpoint, { client_id: "abc123" });
+            getComponent(hsUrl, isUrl, delegatedAuth);
 
             await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…"));
 
@@ -439,6 +444,8 @@ describe("Login", function () {
 
             await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…"));
 
+            // didn't try to register
+            expect(fetchMock).not.toHaveBeenCalledWith(delegatedAuth.registrationEndpoint);
             // continued with normal setup
             expect(mockClient.loginFlows).toHaveBeenCalled();
             // oidc-aware 'continue' button displayed

diff --git a/test/utils/oidc/registerClient-test.ts b/test/utils/oidc/registerClient-test.ts
@@ -15,8 +15,8 @@ limitations under the License.
 */
 
 import fetchMockJest from "fetch-mock-jest";
+import { OidcError } from "matrix-js-sdk/src/oidc/error";
 
-import { OidcClientError } from "../../../src/utils/oidc/error";
 import { getOidcClientId } from "../../../src/utils/oidc/registerClient";
 
 describe("getOidcClientId()", () => {
@@ -56,7 +56,7 @@ describe("getOidcClientId()", () => {
         };
         expect(
             async () => await getOidcClientId(authConfigWithoutRegistration, clientName, baseUrl, staticOidcClients),
-        ).rejects.toThrow(OidcClientError.DynamicRegistrationNotSupported);
+        ).rejects.toThrow(OidcError.DynamicRegistrationNotSupported);
         // didn't try to register
         expect(fetchMockJest).toHaveFetchedTimes(0);
     });
@@ -67,20 +67,55 @@ describe("getOidcClientId()", () => {
             registrationEndpoint: undefined,
         };
         expect(async () => await getOidcClientId(authConfigWithoutRegistration, clientName, baseUrl)).rejects.toThrow(
-            OidcClientError.DynamicRegistrationNotSupported,
+            OidcError.DynamicRegistrationNotSupported,
         );
         // didn't try to register
         expect(fetchMockJest).toHaveFetchedTimes(0);
     });
 
-    it("should throw while dynamic registration is not implemented", async () => {
+    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",
+            }),
+        });
+    });
 
-        expect(async () => await getOidcClientId(delegatedAuthConfig, clientName, baseUrl)).rejects.toThrow(
-            OidcClientError.DynamicRegistrationNotSupported,
+    it("should throw when registration request fails", async () => {
+        fetchMockJest.post(registrationEndpoint, {
+            status: 500,
+        });
+        expect(() => getOidcClientId(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(() => getOidcClientId(delegatedAuthConfig, clientName, baseUrl)).rejects.toThrow(
+            OidcError.DynamicRegistrationInvalid,
         );
     });
 });