Repository: authts/oidc-client-ts (ts)

Issue:
filterProtocolClaims deletes properties required by the IdTokenClaims type
If filterProtocolClaims is set to true (the default), the properties `iss`, `aud`, `exp` and `iat` that are **_required_** by the IdTokenClaims type are deleted and undefined, thus breaking the defined types.

```typescript
const userManager = new UserManager({
  // ... other options omitted
  filterProtocolClaims: true, // defaults to true
  // ...
});
const user = await userManager.getUser();
const exp = user.profile.exp; // TypeScript says this is a number
console.log(typeof exp) // Should be "number" but is "undefined"
```

**Relevant parts of the code**

These properties are deleted from the response:
https://github.com/authts/oidc-client-ts/blob/bcfe363c685e8f243b43bc588ecba7495f88ffa9/src/ResponseValidator.ts#L26-L41
https://github.com/authts/oidc-client-ts/blob/bcfe363c685e8f243b43bc588ecba7495f88ffa9/src/ResponseValidator.ts#L225-L235
Some of which are mandatory here:
https://github.com/authts/oidc-client-ts/blob/bcfe363c685e8f243b43bc588ecba7495f88ffa9/src/Claims.ts#L109

Pull request:
fix(#780): filterProtocolClaims deletes properties required by the IdTokenClaims type
<!-- Please link relevant issue numbers or provide context for this change -->
Closes/fixes #780

### Checklist

- [x] This PR makes changes to the public API <!-- was the API report (docs/oidc-client-ts.api.md) updated by this PR? -->
- [x] I have included links for closing relevant issue numbers


Patch:
diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md
index 501f22a7..01a10a26 100644
--- a/docs/oidc-client-ts.api.md
+++ b/docs/oidc-client-ts.api.md
@@ -130,7 +130,7 @@ export type ExtraSignoutRequestArgs = Pick<CreateSignoutRequestArgs, "extraQuery
 // Warning: (ae-forgotten-export) The symbol "Mandatory" needs to be exported by the entry point index.d.ts
 //
 // @public
-export interface IdTokenClaims extends Mandatory<OidcStandardClaims, "sub">, Mandatory<JwtClaims, "iss" | "sub" | "aud" | "exp" | "iat"> {
+export interface IdTokenClaims extends Mandatory<OidcStandardClaims, "sub">, Mandatory<JwtClaims, "iss" | "sub" | "aud" | "exp" | "iat">, Pick<JwtClaims, "nbf" | "jti"> {
     // (undocumented)
     [claim: string]: unknown;
     acr?: string;
@@ -298,7 +298,7 @@ export class OidcClient {
     // (undocumented)
     readonly metadataService: MetadataService;
     // (undocumented)
-    processResourceOwnerPasswordCredentials({ username, password, skipUserInfo, extraTokenParams, }: ProcessResourceOwnerPasswordCredentialsArgs): Promise<SigninResponse>;
+    processResourceOwnerPasswordCredentials({ username, password, skipUserInfo, }: ProcessResourceOwnerPasswordCredentialsArgs): Promise<SigninResponse>;
     // (undocumented)
     processSigninResponse(url: string): Promise<SigninResponse>;
     // (undocumented)
@@ -344,7 +344,7 @@ export interface OidcClientSettings {
     // (undocumented)
     extraTokenParams?: Record<string, unknown>;
     fetchRequestCredentials?: RequestCredentials;
-    filterProtocolClaims?: boolean;
+    filterProtocolClaims?: boolean | string[];
     loadUserInfo?: boolean;
     max_age?: number;
     mergeClaims?: boolean;
@@ -394,7 +394,7 @@ export class OidcClientSettingsStore {
     // (undocumented)
     readonly fetchRequestCredentials: RequestCredentials;
     // (undocumented)
-    readonly filterProtocolClaims: boolean;
+    readonly filterProtocolClaims: boolean | string[];
     // (undocumented)
     readonly loadUserInfo: boolean;
     // (undocumented)
@@ -562,7 +562,6 @@ export type ProcessResourceOwnerPasswordCredentialsArgs = {
     username: string;
     password: string;
     skipUserInfo?: boolean;
-    extraTokenParams?: Record<string, unknown>;
 };
 
 // @public (undocumented)
diff --git a/src/Claims.ts b/src/Claims.ts
index 3e34b654..bfb3af4d 100644
--- a/src/Claims.ts
+++ b/src/Claims.ts
@@ -106,7 +106,7 @@ export interface JwtClaims {
  * @public
  * @see https://openid.net/specs/openid-connect-core-1_0.html#IDToken
  */
-export interface IdTokenClaims extends Mandatory<OidcStandardClaims, "sub">, Mandatory<JwtClaims, "iss" | "sub" | "aud" | "exp" | "iat"> {
+export interface IdTokenClaims extends Mandatory<OidcStandardClaims, "sub">, Mandatory<JwtClaims, "iss" | "sub" | "aud" | "exp" | "iat">, Pick<JwtClaims, "nbf" | "jti"> {
     [claim: string]: unknown;
 
     /** Time when the End-User authentication occurred. Its value is a JSON number representing the number of seconds from 1970-01-01T0:0:0Z as measured in UTC until the date/time. When a max_age request is made or when auth_time is requested as an Essential Claim, then this Claim is REQUIRED; otherwise, its inclusion is OPTIONAL. (The auth_time Claim semantically corresponds to the OpenID 2.0 PAPE [OpenID.PAPE] auth_time response parameter.) */
diff --git a/src/OidcClient.ts b/src/OidcClient.ts
index a3698030..031df5ae 100644
--- a/src/OidcClient.ts
+++ b/src/OidcClient.ts
@@ -65,7 +65,6 @@ export type ProcessResourceOwnerPasswordCredentialsArgs = {
     username: string;
     password: string;
     skipUserInfo?: boolean;
-    extraTokenParams?: Record<string, unknown>;
 };
 
 /**
@@ -179,9 +178,8 @@ export class OidcClient {
         username,
         password,
         skipUserInfo = false,
-        extraTokenParams = {},
     }: ProcessResourceOwnerPasswordCredentialsArgs): Promise<SigninResponse> {
-        const tokenResponse: Record<string, unknown> = await this._tokenClient.exchangeCredentials({ username, password, ...extraTokenParams });
+        const tokenResponse: Record<string, unknown> = await this._tokenClient.exchangeCredentials({ username, password });
         const signinResponse: SigninResponse = new SigninResponse(new URLSearchParams());
         Object.assign(signinResponse, tokenResponse);
         await this._validator.validateCredentialsResponse(signinResponse, skipUserInfo);
diff --git a/src/OidcClientSettings.ts b/src/OidcClientSettings.ts
index f85393c9..2c569b8c 100644
--- a/src/OidcClientSettings.ts
+++ b/src/OidcClientSettings.ts
@@ -71,8 +71,12 @@ export interface OidcClientSettings {
     /** optional protocol param (default: "query") */
     response_mode?: "query" | "fragment";
 
-    /** Should OIDC protocol claims be removed from profile (default: true) */
-    filterProtocolClaims?: boolean;
+    /**
+     * Should optional OIDC protocol claims be removed from profile or specify the ones to be removed (default: true)
+     * When true, the following claims are removed by default: ["nbf", "jti", "auth_time", "nonce", "acr", "amr", "azp", "at_hash"]
+     * When specifying claims, the following claims are not allowed: ["sub", "iss", "aud", "exp", "iat"]
+    */
+    filterProtocolClaims?: boolean | string[];
     /** Flag to control if additional identity data is loaded from the user info endpoint in order to populate the user's profile (default: false) */
     loadUserInfo?: boolean;
     /** Number (in seconds) indicating the age of state entries in storage for authorize requests that are considered abandoned and thus can be cleaned up (default: 900) */
@@ -153,7 +157,7 @@ export class OidcClientSettingsStore {
     public readonly response_mode: "query" | "fragment";
 
     // behavior flags
-    public readonly filterProtocolClaims: boolean;
+    public readonly filterProtocolClaims: boolean | string[];
     public readonly loadUserInfo: boolean;
     public readonly staleStateAgeInSeconds: number;
     public readonly clockSkewInSeconds: number;
@@ -229,7 +233,7 @@ export class OidcClientSettingsStore {
         this.resource = resource;
         this.response_mode = response_mode;
 
-        this.filterProtocolClaims = !!filterProtocolClaims;
+        this.filterProtocolClaims = filterProtocolClaims ?? true;
         this.loadUserInfo = !!loadUserInfo;
         this.staleStateAgeInSeconds = staleStateAgeInSeconds;
         this.clockSkewInSeconds = clockSkewInSeconds;
diff --git a/src/ResponseValidator.ts b/src/ResponseValidator.ts
index e811472e..637a3a15 100644
--- a/src/ResponseValidator.ts
+++ b/src/ResponseValidator.ts
@@ -16,6 +16,7 @@ import type { RefreshState } from "./RefreshState";
 import type { JwtClaims, IdTokenClaims } from "./Claims";
 
 /**
+ * Protocol claims that could be removed by default from profile.
  * Derived from the following sets of claims:
  * - {@link https://datatracker.ietf.org/doc/html/rfc7519.html#section-4.1}
  * - {@link https://openid.net/specs/openid-connect-core-1_0.html#IDToken}
@@ -23,23 +24,25 @@ import type { JwtClaims, IdTokenClaims } from "./Claims";
  *
  * @internal
  */
-const ProtocolClaims = [
-    "iss",
-    // "sub" should never be excluded, we need access to it internally
-    "aud",
-    "exp",
+const DefaultProtocolClaims = [
     "nbf",
-    "iat",
     "jti",
     "auth_time",
     "nonce",
     "acr",
     "amr",
     "azp",
-    // https://openid.net/specs/openid-connect-core-1_0.html#CodeIDToken
-    "at_hash",
+    "at_hash", // https://openid.net/specs/openid-connect-core-1_0.html#CodeIDToken
 ] as const;
 
+/**
+ * Protocol claims that should never be removed from profile.
+ * "sub" is needed internally and others should remain required as per the OIDC specs.
+ *
+ * @internal
+ */
+const InternalRequiredProtocolClaims = ["sub", "iss", "aud", "exp", "iat"];
+
 /**
  * @internal
  */
@@ -226,8 +229,17 @@ export class ResponseValidator {
         const result = { ...claims };
 
         if (this._settings.filterProtocolClaims) {
-            for (const type of ProtocolClaims) {
-                delete result[type];
+            let protocolClaims;
+            if (Array.isArray(this._settings.filterProtocolClaims)) {
+                protocolClaims = this._settings.filterProtocolClaims;
+            } else {
+                protocolClaims = DefaultProtocolClaims;
+            }
+
+            for (const claim of protocolClaims) {
+                if (!InternalRequiredProtocolClaims.includes(claim)) {
+                    delete result[claim];
+                }
             }
         }
 
diff --git a/src/TokenClient.ts b/src/TokenClient.ts
index c2a436f7..3678fcfc 100644
--- a/src/TokenClient.ts
+++ b/src/TokenClient.ts
@@ -127,7 +127,8 @@ export class TokenClient {
         client_id = this._settings.client_id,
         client_secret = this._settings.client_secret,
         scope = this._settings.scope,
-        ...args
+        username,
+        password,
     }: ExchangeCredentialsArgs): Promise<Record<string, unknown>> {
         const logger = this._logger.create("exchangeCredentials");
 
@@ -135,12 +136,7 @@ export class TokenClient {
             logger.throw(new Error("A client_id is required"));
         }
 
-        const params = new URLSearchParams({ grant_type, scope });
-        for (const [key, value] of Object.entries(args)) {
-            if (value != null) {
-                params.set(key, value);
-            }
-        }
+        const params = new URLSearchParams({ grant_type, username, password, scope });
 
         let basicAuth: string | undefined;
         switch (this._settings.client_authentication) {
diff --git a/src/UserManager.ts b/src/UserManager.ts
index 45d49b4c..d70ccb94 100644
--- a/src/UserManager.ts
+++ b/src/UserManager.ts
@@ -194,7 +194,7 @@ export class UserManager {
     }: SigninResourceOwnerCredentialsArgs ) {
         const logger = this._logger.create("signinResourceOwnerCredential");
 
-        const signinResponse = await this._client.processResourceOwnerPasswordCredentials({ username, password, skipUserInfo, extraTokenParams: this.settings.extraTokenParams });
+        const signinResponse = await this._client.processResourceOwnerPasswordCredentials({ username, password, skipUserInfo });
         logger.debug("got signin response");
 
         const user = await this._buildUser(signinResponse);


Test patch:
diff --git a/src/OidcClientSettings.test.ts b/src/OidcClientSettings.test.ts
index 07dd57f6..4a05c13d 100644
--- a/src/OidcClientSettings.test.ts
+++ b/src/OidcClientSettings.test.ts
@@ -320,6 +320,17 @@ describe("OidcClientSettings", () => {
 
             // assert
             expect(subject.filterProtocolClaims).toEqual(false);
+
+            // act
+            subject = new OidcClientSettingsStore({
+                authority: "authority",
+                client_id: "client",
+                redirect_uri: "redirect",
+                filterProtocolClaims: ["a", "b", "c"],
+            });
+
+            // assert
+            expect(subject.filterProtocolClaims).toEqual(["a", "b", "c"]);
         });
     });
 
diff --git a/src/ResponseValidator.test.ts b/src/ResponseValidator.test.ts
index 54133cc0..7f86a322 100644
--- a/src/ResponseValidator.test.ts
+++ b/src/ResponseValidator.test.ts
@@ -235,6 +235,7 @@ describe("ResponseValidator", () => {
             jest.spyOn(JwtUtils, "decode").mockReturnValue({
                 sub: "sub",
                 iss: "iss",
+                acr: "acr",
                 a: "apple",
                 b: "banana",
             });
@@ -244,7 +245,7 @@ describe("ResponseValidator", () => {
             await subject.validateSigninResponse(stubResponse, stubState);
 
             // assert
-            expect(stubResponse.profile).not.toHaveProperty("iss");
+            expect(stubResponse.profile).not.toHaveProperty("acr");
         });
 
         it("should not filter protocol claims if not OIDC", async () => {
@@ -718,8 +719,8 @@ describe("ResponseValidator", () => {
                 aud: "some_aud", iss: "issuer",
                 sub: "123", email: "foo@gmail.com",
                 role: ["admin", "dev"],
-                at_hash: "athash",
-                iat: 5, nbf: 10, exp: 20,
+                iat: 5, exp: 20,
+                nbf: 10, at_hash: "athash",
             };
 
             // act
@@ -728,8 +729,10 @@ describe("ResponseValidator", () => {
             // assert
             expect(result).toEqual({
                 foo: 1, bar: "test",
+                aud: "some_aud", iss: "issuer",
                 sub: "123", email: "foo@gmail.com",
                 role: ["admin", "dev"],
+                iat: 5, exp: 20,
             });
         });
 
@@ -758,5 +761,95 @@ describe("ResponseValidator", () => {
                 iat: 5, nbf: 10, exp: 20,
             });
         });
+
+        it("should filter protocol claims if specified in settings", () => {
+            // arrange
+            Object.assign(settings, { filterProtocolClaims: ["foo", "bar", "role", "nbf", "email"] });
+            const claims = {
+                foo: 1, bar: "test",
+                aud: "some_aud", iss: "issuer",
+                sub: "123", email: "foo@gmail.com",
+                role: ["admin", "dev"],
+                iat: 5, exp: 20,
+                nbf: 10, at_hash: "athash",
+            };
+
+            // act
+            const result = subject["_filterProtocolClaims"](claims);
+
+            // assert
+            expect(result).toEqual({
+                aud: "some_aud", iss: "issuer",
+                sub: "123",
+                iat: 5, exp: 20,
+                at_hash: "athash",
+            });
+        });
+
+        it("should filter only protocol claims defined by default by the library", () => {
+            // arrange
+            Object.assign(settings, { filterProtocolClaims: true });
+            const defaultProtocolClaims = {
+                nbf: 3, jti: "jti",
+                auth_time: 123,
+                nonce: "nonce",
+                acr: "acr",
+                amr: "amr",
+                azp: "azp",
+                at_hash: "athash",
+            };
+            const claims = {
+                foo: 1, bar: "test",
+                aud: "some_aud", iss: "issuer",
+                sub: "123", email: "foo@gmail.com",
+                role: ["admin", "dev"],
+                iat: 5, exp: 20,
+            };
+
+            // act
+            const result = subject["_filterProtocolClaims"]({ ...defaultProtocolClaims, ...claims });
+
+            // assert
+            expect(result).toEqual(claims);
+        });
+
+        it("should not filter protocol claims that are required by the library", () => {
+            // arrange
+            Object.assign(settings, { filterProtocolClaims: true });
+            const internalRequiredProtocolClaims = {
+                sub: "sub",
+                iss: "issuer",
+                aud: "some_aud",
+                exp: 20,
+                iat: 5,
+            };
+            const claims = {
+                foo: 1, bar: "test",
+                email: "foo@gmail.com",
+                role: ["admin", "dev"],
+                nbf: 10,
+            };
+
+            // act
+            let items = { ...internalRequiredProtocolClaims, ...claims };
+            let result = subject["_filterProtocolClaims"](items);
+
+            // assert
+            // nbf is part of the claims that should be filtered by the library by default, so we need to remove it
+            delete (items as Partial<typeof items>).nbf;
+            expect(result).toEqual(items);
+
+            // ... even if specified in settings
+
+            // arrange
+            Object.assign(settings, { filterProtocolClaims: ["sub", "iss", "aud", "exp", "iat"] });
+
+            // act
+            items = { ...internalRequiredProtocolClaims, ...claims };
+            result = subject["_filterProtocolClaims"](items);
+
+            // assert
+            expect(result).toEqual(items);
+        });
     });
 });