Lint

CHANGELOG.md

@@ -36,6 +36,11 @@ The following changes have been implemented but not released yet:
 
 - Fix the `Session` error listener typing by adding `Error` to the `errorDescription` type so that it reflects the actual behavior.
   Thanks to @NoelDeMartin for fixing this issue.
+- Previously, an application could end up in a bad state when using a dynamically registered
+  client identity beyond its expiration date. A user would be redirected to the OpenID Provider,
+  and end up on an error page unrelated to the application they were trying to log into. Now,
+  expired dynamic clients go through registration again: the user will need to authorize the client
+  after expiration, but will not experience further inconveniences.
 
 ## [2.3.0](https://github.com/inrupt/solid-client-authn-js/releases/tag/v2.3.0) - 2024-11-14
 

packages/node/src/login/oidc/ClientRegistrar.spec.ts

@@ -22,25 +22,25 @@
 import { randomUUID } from "crypto";
 import { jest, it, describe, expect } from "@jest/globals";
 import { mockStorageUtility } from "@inrupt/solid-client-authn-core";
+import type * as OpenIdClient from "openid-client";
+import type { IOpenIdDynamicClient } from "core";
 import ClientRegistrar from "./ClientRegistrar";
 import {
   IssuerConfigFetcherFetchConfigResponse,
   mockIssuerMetadata,
 } from "./__mocks__/IssuerConfigFetcher";
-import {
-  mockDefaultClientConfig,
-} from "./__mocks__/ClientRegistrar";
-import type * as OpenIdClient from "openid-client";
-import { IOpenIdDynamicClient } from "core";
+import { mockDefaultClientConfig } from "./__mocks__/ClientRegistrar";
 
 jest.mock("openid-client");
 
 const mockClientRegistration = (
   issuerMetadata: Record<string, string | undefined>,
-  clientMetadata: OpenIdClient.ClientMetadata
+  clientMetadata: OpenIdClient.ClientMetadata,
 ) => {
   // Sets up the mock-up for DCR
-  const { Issuer } = jest.requireMock("openid-client") as jest.Mocked<typeof OpenIdClient>;
+  const { Issuer } = jest.requireMock("openid-client") as jest.Mocked<
+    typeof OpenIdClient
+  >;
   const mockedIssuerConfig = mockIssuerMetadata(issuerMetadata);
   const mockedIssuer: OpenIdClient.Issuer<OpenIdClient.BaseClient> = {
     metadata: mockedIssuerConfig,
@@ -52,8 +52,12 @@ const mockClientRegistration = (
     } as any,
   } as any;
 
-  Issuer.mockReturnValue(mockedIssuer as jest.MockedObject<OpenIdClient.Issuer<OpenIdClient.BaseClient>>);
-}
+  Issuer.mockReturnValue(
+    mockedIssuer as jest.MockedObject<
+      OpenIdClient.Issuer<OpenIdClient.BaseClient>
+    >,
+  );
+};
 
 /**
  * Test for ClientRegistrar
@@ -70,9 +74,12 @@ describe("ClientRegistrar", () => {
 
   describe("getClient", () => {
     it("fails if there is not registration endpoint", async () => {
-      mockClientRegistration({
-        registration_endpoint: undefined,
-      }, mockDefaultClientConfig());
+      mockClientRegistration(
+        {
+          registration_endpoint: undefined,
+        },
+        mockDefaultClientConfig(),
+      );
 
       // Run the test
       const clientRegistrar = getClientRegistrar({
@@ -189,7 +196,9 @@ describe("ClientRegistrar", () => {
       ).resolves.toEqual(mockDefaultClientConfig().client_secret);
       await expect(
         mockStorage.getForUser("mySession", "expiresAt"),
-      ).resolves.toEqual(String(mockDefaultClientConfig().client_secret_expires_at));
+      ).resolves.toEqual(
+        String(mockDefaultClientConfig().client_secret_expires_at),
+      );
       await expect(
         mockStorage.getForUser("mySession", "idTokenSignedResponseAlg"),
       ).resolves.toEqual(
@@ -229,7 +238,9 @@ describe("ClientRegistrar", () => {
       // The expired client should have been re-registered.
       expect(client.clientId).toBe(mockDefaultClientConfig().client_id);
       expect(client.clientSecret).toBe(mockDefaultClientConfig().client_secret);
-      expect((client as IOpenIdDynamicClient).expiresAt).toBe(mockDefaultClientConfig().client_secret_expires_at);
+      expect((client as IOpenIdDynamicClient).expiresAt).toBe(
+        mockDefaultClientConfig().client_secret_expires_at,
+      );
     });
 
     it("throws if the issuer doesn't avertise for supported signing algorithms", async () => {

packages/node/src/login/oidc/ClientRegistrar.ts

@@ -105,11 +105,17 @@ export default class ClientRegistrar implements IClientRegistrar {
     // It will be treated as an expired client for dynamic clients (legacy case),
     // and it will not impact static clients.
     const expirationDate =
-    storedExpiresAt !== undefined ? Number.parseInt(storedExpiresAt, 10) : -1;
+      storedExpiresAt !== undefined ? Number.parseInt(storedExpiresAt, 10) : -1;
     // Expiration is only applicable to confidential dynamic clients.
     const expired =
-      storedClientSecret !== undefined && storedClientType === "dynamic" && Date.now() > expirationDate;
-    if (storedClientId !== undefined && isKnownClientType(storedClientType) && !expired) {
+      storedClientSecret !== undefined &&
+      storedClientType === "dynamic" &&
+      Date.now() > expirationDate;
+    if (
+      storedClientId !== undefined &&
+      isKnownClientType(storedClientType) &&
+      !expired
+    ) {
       if (storedClientType === "static" && storedClientSecret === undefined) {
         throw new Error("Missing static client secret in storage.");
       }
@@ -159,26 +165,23 @@ export default class ClientRegistrar implements IClientRegistrar {
         registeredClient.metadata.id_token_signed_response_alg ?? signingAlg,
       clientType: "dynamic",
     };
-    await this.storageUtility.setForUser(
-      options.sessionId, {
-        clientId: persistedClientMetadata.clientId,
-        idTokenSignedResponseAlg: persistedClientMetadata.idTokenSignedResponseAlg!,
-        clientType: "dynamic",
-      }
-    );
+    await this.storageUtility.setForUser(options.sessionId, {
+      clientId: persistedClientMetadata.clientId,
+      idTokenSignedResponseAlg:
+        persistedClientMetadata.idTokenSignedResponseAlg!,
+      clientType: "dynamic",
+    });
     if (registeredClient.metadata.client_secret !== undefined) {
       persistedClientMetadata = {
         ...persistedClientMetadata,
         clientSecret: registeredClient.metadata.client_secret,
         // If a client secret is present, it has an expiration date.
-        expiresAt: registeredClient.metadata.client_secret_expires_at as number
-      }
-      await this.storageUtility.setForUser(
-        options.sessionId, {
-          clientSecret: persistedClientMetadata.clientSecret,
-          expiresAt: String(persistedClientMetadata.expiresAt),
-        }
-      );
+        expiresAt: registeredClient.metadata.client_secret_expires_at as number,
+      };
+      await this.storageUtility.setForUser(options.sessionId, {
+        clientSecret: persistedClientMetadata.clientSecret,
+        expiresAt: String(persistedClientMetadata.expiresAt),
+      });
     }
     return persistedClientMetadata;
   }

packages/node/src/login/oidc/oidcHandlers/RefreshTokenOidcHandler.spec.ts

@@ -38,9 +38,7 @@ import {
   mockDefaultOidcOptions,
   mockOidcOptions,
 } from "../__mocks__/IOidcOptions";
-import {
-  mockDefaultClient
-} from "../__mocks__/ClientRegistrar";
+import { mockDefaultClient } from "../__mocks__/ClientRegistrar";
 import RefreshTokenOidcHandler from "./RefreshTokenOidcHandler";
 import {
   mockDefaultTokenRefresher,