feat: #1232 improve merge claims behavior

Author: pamapa

docs/migration.md

@@ -1,13 +1,15 @@
 ## oidc-client v2.4.0 → oidc-client-ts v3.0.0
 
-The API is largely backwards-compatible.
+The API is largely backwards-compatible. The merge claims behavior has been improved.
 
 ### [OidcClientSettings](https://authts.github.io/oidc-client-ts/interfaces/OidcClientSettings.html)
 
 - the following deprecated properties were **removed**:
   - `clockSkewInSeconds` unused since 2.0.0
   - `userInfoJwtIssuer` unused since 2.0.0
   - `refreshTokenCredentials` use `fetchRequestCredentials` since 2.1.0
+- the `mergeClaims` has been replaced by `mergeClaimsStrategy`
+  - if the previous behavior is needed `mergeClaimsStrategy: { array: "merge" }` can be used
 
 
 ## oidc-client v1.11.5 → oidc-client-ts v2.0.0

docs/oidc-client-ts.api.md

@@ -358,7 +358,9 @@ export interface OidcClientSettings {
     filterProtocolClaims?: boolean | string[];
     loadUserInfo?: boolean;
     max_age?: number;
-    mergeClaims?: boolean;
+    mergeClaimsStrategy?: {
+        array: "replace" | "merge";
+    };
     metadata?: Partial<OidcMetadata>;
     metadataSeed?: Partial<OidcMetadata>;
     // (undocumented)
@@ -380,7 +382,7 @@ export interface OidcClientSettings {
 
 // @public
 export class OidcClientSettingsStore {
-    constructor({ authority, metadataUrl, metadata, signingKeys, metadataSeed, client_id, client_secret, response_type, scope, redirect_uri, post_logout_redirect_uri, client_authentication, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, filterProtocolClaims, loadUserInfo, staleStateAgeInSeconds, mergeClaims, disablePKCE, stateStore, revokeTokenAdditionalContentTypes, fetchRequestCredentials, refreshTokenAllowedScope, extraQueryParams, extraTokenParams, extraHeaders, }: OidcClientSettings);
+    constructor({ authority, metadataUrl, metadata, signingKeys, metadataSeed, client_id, client_secret, response_type, scope, redirect_uri, post_logout_redirect_uri, client_authentication, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, filterProtocolClaims, loadUserInfo, staleStateAgeInSeconds, mergeClaimsStrategy, disablePKCE, stateStore, revokeTokenAdditionalContentTypes, fetchRequestCredentials, refreshTokenAllowedScope, extraQueryParams, extraTokenParams, extraHeaders, }: OidcClientSettings);
     // (undocumented)
     readonly acr_values: string | undefined;
     // (undocumented)
@@ -410,7 +412,9 @@ export class OidcClientSettingsStore {
     // (undocumented)
     readonly max_age: number | undefined;
     // (undocumented)
-    readonly mergeClaims: boolean;
+    readonly mergeClaimsStrategy: {
+        array: "replace" | "merge";
+    };
     // (undocumented)
     readonly metadata: Partial<OidcMetadata> | undefined;
     // (undocumented)

src/ClaimsService.test.ts

@@ -2,19 +2,19 @@
 // Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
 
 import { ClaimsService } from "./ClaimsService";
-import type { OidcClientSettingsStore } from "./OidcClientSettings";
+import { OidcClientSettingsStore } from "./OidcClientSettings";
 import type { UserProfile } from "./User";
 
 describe("ClaimsService", () => {
     let settings: OidcClientSettingsStore;
     let subject: ClaimsService;
 
     beforeEach(() => {
-        settings = {
-            authority: "op",
-            client_id: "client",
-            loadUserInfo: true,
-        } as OidcClientSettingsStore;
+        settings = new OidcClientSettingsStore({
+            authority: "authority",
+            client_id: "client_id",
+            redirect_uri: "redirect_uri",
+        });
 
         subject = new ClaimsService(settings);
     });
@@ -218,7 +218,7 @@ describe("ClaimsService", () => {
             expect(result).toEqual({ a: "apple", c: "carrot", b: "banana" });
         });
 
-        it("should not merge claims when claim types are objects", () => {
+        it("should merge claims when claim types are objects", () => {
             // arrange
             const c1 = {
                 custom: { apple: "foo", pear: "bar" },
@@ -233,87 +233,86 @@ describe("ClaimsService", () => {
 
             // assert
             expect(result).toEqual({
-                custom: [
-                    { apple: "foo", pear: "bar" },
-                    { apple: "foo", orange: "peel" },
-                ],
+                custom: { apple: "foo", pear: "bar", orange: "peel" },
                 b: "banana",
             });
         });
 
-        it("should merge claims when claim types are objects when mergeClaims settings is true", () => {
+        it("should replace same claim types", () => {
             // arrange
-            Object.assign(settings, { mergeClaims: true });
-
-            const c1 = {
-                custom: { apple: "foo", pear: "bar" },
-            } as unknown as UserProfile;
-            const c2 = {
-                custom: { apple: "foo", orange: "peel" },
-                b: "banana",
-            };
+            const c1 = { a: "apple", b: "banana" } as unknown as UserProfile;
+            const c2 = { a: "carrot" };
 
             // act
             const result = subject.mergeClaims(c1, c2);
 
             // assert
-            expect(result).toEqual({
-                custom: { apple: "foo", pear: "bar", orange: "peel" },
-                b: "banana",
-            });
+            expect(result).toEqual({ a: "carrot", b: "banana" });
         });
 
-        it("should merge same claim types into array", () => {
+        it("should remove duplicates when producing arrays", () => {
             // arrange
             const c1 = { a: "apple", b: "banana" } as unknown as UserProfile;
-            const c2 = { a: "carrot" };
+            const c2 = { a: ["apple", "durian"] };
 
             // act
             const result = subject.mergeClaims(c1, c2);
 
             // assert
-            expect(result).toEqual({ a: ["apple", "carrot"], b: "banana" });
+            expect(result).toEqual({ a: ["apple", "durian"], b: "banana" });
         });
 
-        it("should merge arrays of same claim types into array", () => {
+        it("should merge arrays of same claim types into array (string vs. array) when mergeClaimsStrategy is 'merge'", () => {
             // arrange
-            const c1 = { a: "apple", b: "banana" } as unknown as UserProfile;
-            const c2 = { a: ["carrot", "durian"] };
+            Object.assign(settings, { mergeClaimsStrategy: "merge" });
+            const c1 = {
+                a: "apple",
+                b: "banana",
+            } as unknown as UserProfile;
+            const c2 = {
+                a: ["carrot", "durian"],
+            };
 
             // act
-            let result = subject.mergeClaims(c1, c2);
+            const result = subject.mergeClaims(c1, c2);
 
             // assert
             expect(result).toEqual({
                 a: ["apple", "carrot", "durian"],
                 b: "banana",
             });
+        });
 
+        it("should merge arrays of same claim types into array (array vs. array) when mergeClaimsStrategy is 'merge'", () => {
             // arrange
+            Object.assign(settings, { mergeClaimsStrategy: "merge" });
             const d1 = {
                 a: ["apple", "carrot"],
                 b: "banana",
             } as unknown as UserProfile;
             const d2 = { a: ["durian"] };
 
             // act
-            result = subject.mergeClaims(d1, d2);
+            const result = subject.mergeClaims(d1, d2);
 
             // assert
             expect(result).toEqual({
                 a: ["apple", "carrot", "durian"],
                 b: "banana",
             });
+        });
 
+        it("should merge arrays of same claim types into array (array vs. string) when mergeClaimsStrategy is 'merge'", () => {
             // arrange
+            Object.assign(settings, { mergeClaimsStrategy: "merge" });
             const e1 = {
                 a: ["apple", "carrot"],
                 b: "banana",
             } as unknown as UserProfile;
             const e2 = { a: "durian" };
 
             // act
-            result = subject.mergeClaims(e1, e2);
+            const result = subject.mergeClaims(e1, e2);
 
             // assert
             expect(result).toEqual({
@@ -322,31 +321,51 @@ describe("ClaimsService", () => {
             });
         });
 
-        it("should remove duplicates when producing arrays", () => {
-            // arrange
-            const c1 = { a: "apple", b: "banana" } as unknown as UserProfile;
-            const c2 = { a: ["apple", "durian"] };
+        it("should replace if type is different (array vs. string)", () => {
+        // arrange
+            const c1 = {
+                a: ["apple", "durian"],
+                b: "banana",
+            } as unknown as UserProfile;
+            const c2 = { a: "apple" };
 
             // act
             const result = subject.mergeClaims(c1, c2);
 
             // assert
-            expect(result).toEqual({ a: ["apple", "durian"], b: "banana" });
+            expect(result).toEqual({ a: "apple", b: "banana" });
         });
 
-        it("should not add if already present in array", () => {
-            // arrange
+        it("should replace if type is different (object vs. string)", () => {
+        // arrange
             const c1 = {
-                a: ["apple", "durian"],
+                custom: { a: "apple" },
                 b: "banana",
             } as unknown as UserProfile;
-            const c2 = { a: "apple" };
+            const c2 = { custom: "apple" };
 
             // act
             const result = subject.mergeClaims(c1, c2);
 
             // assert
-            expect(result).toEqual({ a: ["apple", "durian"], b: "banana" });
+            expect(result).toEqual({ custom: "apple", b: "banana" });
+        });
+
+        it("should replace if type is different (array vs. object)", () => {
+        // arrange
+            const c1 = {
+                custom: ["apple", "durian"],
+                b: "banana",
+            } as unknown as UserProfile;
+            const c2 = {
+                custom: { a: "apple" },
+            } as unknown as UserProfile;
+
+            // act
+            const result = subject.mergeClaims(c1, c2);
+
+            // assert
+            expect(result).toEqual({ custom: { a: "apple" }, b: "banana" });
         });
     });
 });

src/ClaimsService.ts

@@ -64,27 +64,27 @@ export class ClaimsService {
         return result;
     }
 
+    public mergeClaims(claims1: JwtClaims, claims2: JwtClaims): UserProfile;
     public mergeClaims(claims1: UserProfile, claims2: JwtClaims): UserProfile {
         const result = { ...claims1 };
-
         for (const [claim, values] of Object.entries(claims2)) {
-            for (const value of Array.isArray(values) ? values : [values]) {
-                const previousValue = result[claim];
-                if (previousValue === undefined) {
-                    result[claim] = value;
-                }
-                else if (Array.isArray(previousValue)) {
-                    if (!previousValue.includes(value)) {
-                        previousValue.push(value);
-                    }
-                }
-                else if (result[claim] !== value) {
-                    if (typeof value === "object" && this._settings.mergeClaims) {
-                        result[claim] = this.mergeClaims(previousValue as UserProfile, value);
-                    }
-                    else {
-                        result[claim] = [previousValue, value];
+            if (result[claim] !== values) {
+                if (Array.isArray(result[claim]) || Array.isArray(values)) {
+                    if (this._settings.mergeClaimsStrategy.array == "replace") {
+                        result[claim] = values;
+                    } else {
+                        const mergedValues = Array.isArray(result[claim]) ? result[claim] as unknown[] : [result[claim]];
+                        for (const value of Array.isArray(values) ? values : [values]) {
+                            if (!mergedValues.includes(value)) {
+                                mergedValues.push(value);
+                            }
+                        }
+                        result[claim] = mergedValues;
                     }
+                } else if (typeof result[claim] === "object" && typeof values === "object") {
+                    result[claim] = this.mergeClaims(result[claim] as JwtClaims, values as JwtClaims);
+                } else {
+                    result[claim] = values;
                 }
             }
         }

src/OidcClientSettings.ts

@@ -87,10 +87,12 @@ export interface OidcClientSettings {
     staleStateAgeInSeconds?: number;
 
     /**
-     * Indicates if objects returned from the user info endpoint as claims (e.g. `address`) are merged into the claims from the id token as a single object.
-     * Otherwise, they are added to an array as distinct objects for the claim type. (default: false)
+     * Indicates how objects returned from the user info endpoint as claims (e.g. `address`) are merged into the claims from the
+     * id token as a single object.  (default: `{ array: "replace" }`)
+     * - array: "replace": natives (string, int, float) and arrays are replaced, objects are merged as distinct objects
+     * - array: "merge": natives (string, int, float) are replaced, arrays and objects are merged as distinct objects
      */
-    mergeClaims?: boolean;
+    mergeClaimsStrategy?: { array: "replace" | "merge" };
 
     /**
      * Storage object used to persist interaction state (default: window.localStorage, InMemoryWebStorage iff no window).
@@ -167,7 +169,7 @@ export class OidcClientSettingsStore {
     public readonly filterProtocolClaims: boolean | string[];
     public readonly loadUserInfo: boolean;
     public readonly staleStateAgeInSeconds: number;
-    public readonly mergeClaims: boolean;
+    public readonly mergeClaimsStrategy: { array: "replace" | "merge" };
 
     public readonly stateStore: StateStore;
 
@@ -194,7 +196,7 @@ export class OidcClientSettingsStore {
         filterProtocolClaims = true,
         loadUserInfo = false,
         staleStateAgeInSeconds = DefaultStaleStateAgeInSeconds,
-        mergeClaims = false,
+        mergeClaimsStrategy = { array: "replace" },
         disablePKCE = false,
         // other behavior
         stateStore,
@@ -244,7 +246,7 @@ export class OidcClientSettingsStore {
         this.filterProtocolClaims = filterProtocolClaims ?? true;
         this.loadUserInfo = !!loadUserInfo;
         this.staleStateAgeInSeconds = staleStateAgeInSeconds;
-        this.mergeClaims = !!mergeClaims;
+        this.mergeClaimsStrategy = mergeClaimsStrategy;
         this.disablePKCE = !!disablePKCE;
         this.revokeTokenAdditionalContentTypes = revokeTokenAdditionalContentTypes;