Skip to content

Commit 68c1574

Browse files
pamapapamapa
committed
feat: authts#1232 improve merge claims behavior
1 parent 2a308c6 commit 68c1574

5 files changed

Lines changed: 99 additions & 72 deletions

File tree

‎docs/migration.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
1-
## oidc-client v2.4.0 → oidc-client-ts v3.0.0
1+
## oidc-client-ts v2.4.0 → oidc-client-ts v3.0.0
22

3-
The API is largely backwards-compatible.
3+
The API is largely backwards-compatible. The merge claims behavior has been improved.
44

55
### [OidcClientSettings](https://authts.github.io/oidc-client-ts/interfaces/OidcClientSettings.html)
66

77
- the following deprecated properties were **removed**:
88
- `clockSkewInSeconds` unused since 2.0.0
99
- `userInfoJwtIssuer` unused since 2.0.0
1010
- `refreshTokenCredentials` use `fetchRequestCredentials` since 2.1.0
11+
- the `mergeClaims` has been replaced by `mergeClaimsStrategy`
12+
- if the previous behavior is needed `mergeClaimsStrategy: { array: "merge" }` can be used
1113

1214

1315
## oidc-client v1.11.5 → oidc-client-ts v2.0.0

‎docs/oidc-client-ts.api.md‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,9 @@ export interface OidcClientSettings {
358358
filterProtocolClaims?: boolean | string[];
359359
loadUserInfo?: boolean;
360360
max_age?: number;
361-
mergeClaims?: boolean;
361+
mergeClaimsStrategy?: {
362+
array: "replace" | "merge";
363+
};
362364
metadata?: Partial<OidcMetadata>;
363365
metadataSeed?: Partial<OidcMetadata>;
364366
// (undocumented)
@@ -380,7 +382,7 @@ export interface OidcClientSettings {
380382

381383
// @public
382384
export class OidcClientSettingsStore {
383-
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);
385+
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);
384386
// (undocumented)
385387
readonly acr_values: string | undefined;
386388
// (undocumented)
@@ -410,7 +412,9 @@ export class OidcClientSettingsStore {
410412
// (undocumented)
411413
readonly max_age: number | undefined;
412414
// (undocumented)
413-
readonly mergeClaims: boolean;
415+
readonly mergeClaimsStrategy: {
416+
array: "replace" | "merge";
417+
};
414418
// (undocumented)
415419
readonly metadata: Partial<OidcMetadata> | undefined;
416420
// (undocumented)

‎src/ClaimsService.test.ts‎

Lines changed: 63 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,19 @@
22
// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
33

44
import { ClaimsService } from "./ClaimsService";
5-
import type { OidcClientSettingsStore } from "./OidcClientSettings";
5+
import { OidcClientSettingsStore } from "./OidcClientSettings";
66
import type { UserProfile } from "./User";
77

88
describe("ClaimsService", () => {
99
let settings: OidcClientSettingsStore;
1010
let subject: ClaimsService;
1111

1212
beforeEach(() => {
13-
settings = {
14-
authority: "op",
15-
client_id: "client",
16-
loadUserInfo: true,
17-
} as OidcClientSettingsStore;
13+
settings = new OidcClientSettingsStore({
14+
authority: "authority",
15+
client_id: "client_id",
16+
redirect_uri: "redirect_uri",
17+
});
1818

1919
subject = new ClaimsService(settings);
2020
});
@@ -218,7 +218,7 @@ describe("ClaimsService", () => {
218218
expect(result).toEqual({ a: "apple", c: "carrot", b: "banana" });
219219
});
220220

221-
it("should not merge claims when claim types are objects", () => {
221+
it("should merge claims when claim types are objects", () => {
222222
// arrange
223223
const c1 = {
224224
custom: { apple: "foo", pear: "bar" },
@@ -233,87 +233,86 @@ describe("ClaimsService", () => {
233233

234234
// assert
235235
expect(result).toEqual({
236-
custom: [
237-
{ apple: "foo", pear: "bar" },
238-
{ apple: "foo", orange: "peel" },
239-
],
236+
custom: { apple: "foo", pear: "bar", orange: "peel" },
240237
b: "banana",
241238
});
242239
});
243240

244-
it("should merge claims when claim types are objects when mergeClaims settings is true", () => {
241+
it("should replace same claim types", () => {
245242
// arrange
246-
Object.assign(settings, { mergeClaims: true });
247-
248-
const c1 = {
249-
custom: { apple: "foo", pear: "bar" },
250-
} as unknown as UserProfile;
251-
const c2 = {
252-
custom: { apple: "foo", orange: "peel" },
253-
b: "banana",
254-
};
243+
const c1 = { a: "apple", b: "banana" } as unknown as UserProfile;
244+
const c2 = { a: "carrot" };
255245

256246
// act
257247
const result = subject.mergeClaims(c1, c2);
258248

259249
// assert
260-
expect(result).toEqual({
261-
custom: { apple: "foo", pear: "bar", orange: "peel" },
262-
b: "banana",
263-
});
250+
expect(result).toEqual({ a: "carrot", b: "banana" });
264251
});
265252

266-
it("should merge same claim types into array", () => {
253+
it("should remove duplicates when producing arrays", () => {
267254
// arrange
268255
const c1 = { a: "apple", b: "banana" } as unknown as UserProfile;
269-
const c2 = { a: "carrot" };
256+
const c2 = { a: ["apple", "durian"] };
270257

271258
// act
272259
const result = subject.mergeClaims(c1, c2);
273260

274261
// assert
275-
expect(result).toEqual({ a: ["apple", "carrot"], b: "banana" });
262+
expect(result).toEqual({ a: ["apple", "durian"], b: "banana" });
276263
});
277264

278-
it("should merge arrays of same claim types into array", () => {
265+
it("should merge arrays of same claim types into array (string vs. array) when mergeClaimsStrategy is 'merge'", () => {
279266
// arrange
280-
const c1 = { a: "apple", b: "banana" } as unknown as UserProfile;
281-
const c2 = { a: ["carrot", "durian"] };
267+
Object.assign(settings, { mergeClaimsStrategy: "merge" });
268+
const c1 = {
269+
a: "apple",
270+
b: "banana",
271+
} as unknown as UserProfile;
272+
const c2 = {
273+
a: ["carrot", "durian"],
274+
};
282275

283276
// act
284-
let result = subject.mergeClaims(c1, c2);
277+
const result = subject.mergeClaims(c1, c2);
285278

286279
// assert
287280
expect(result).toEqual({
288281
a: ["apple", "carrot", "durian"],
289282
b: "banana",
290283
});
284+
});
291285

286+
it("should merge arrays of same claim types into array (array vs. array) when mergeClaimsStrategy is 'merge'", () => {
292287
// arrange
288+
Object.assign(settings, { mergeClaimsStrategy: "merge" });
293289
const d1 = {
294290
a: ["apple", "carrot"],
295291
b: "banana",
296292
} as unknown as UserProfile;
297293
const d2 = { a: ["durian"] };
298294

299295
// act
300-
result = subject.mergeClaims(d1, d2);
296+
const result = subject.mergeClaims(d1, d2);
301297

302298
// assert
303299
expect(result).toEqual({
304300
a: ["apple", "carrot", "durian"],
305301
b: "banana",
306302
});
303+
});
307304

305+
it("should merge arrays of same claim types into array (array vs. string) when mergeClaimsStrategy is 'merge'", () => {
308306
// arrange
307+
Object.assign(settings, { mergeClaimsStrategy: "merge" });
309308
const e1 = {
310309
a: ["apple", "carrot"],
311310
b: "banana",
312311
} as unknown as UserProfile;
313312
const e2 = { a: "durian" };
314313

315314
// act
316-
result = subject.mergeClaims(e1, e2);
315+
const result = subject.mergeClaims(e1, e2);
317316

318317
// assert
319318
expect(result).toEqual({
@@ -322,31 +321,51 @@ describe("ClaimsService", () => {
322321
});
323322
});
324323

325-
it("should remove duplicates when producing arrays", () => {
326-
// arrange
327-
const c1 = { a: "apple", b: "banana" } as unknown as UserProfile;
328-
const c2 = { a: ["apple", "durian"] };
324+
it("should replace if type is different (array vs. string)", () => {
325+
// arrange
326+
const c1 = {
327+
a: ["apple", "durian"],
328+
b: "banana",
329+
} as unknown as UserProfile;
330+
const c2 = { a: "apple" };
329331

330332
// act
331333
const result = subject.mergeClaims(c1, c2);
332334

333335
// assert
334-
expect(result).toEqual({ a: ["apple", "durian"], b: "banana" });
336+
expect(result).toEqual({ a: "apple", b: "banana" });
335337
});
336338

337-
it("should not add if already present in array", () => {
338-
// arrange
339+
it("should replace if type is different (object vs. string)", () => {
340+
// arrange
339341
const c1 = {
340-
a: ["apple", "durian"],
342+
custom: { a: "apple" },
341343
b: "banana",
342344
} as unknown as UserProfile;
343-
const c2 = { a: "apple" };
345+
const c2 = { custom: "apple" };
344346

345347
// act
346348
const result = subject.mergeClaims(c1, c2);
347349

348350
// assert
349-
expect(result).toEqual({ a: ["apple", "durian"], b: "banana" });
351+
expect(result).toEqual({ custom: "apple", b: "banana" });
352+
});
353+
354+
it("should replace if type is different (array vs. object)", () => {
355+
// arrange
356+
const c1 = {
357+
custom: ["apple", "durian"],
358+
b: "banana",
359+
} as unknown as UserProfile;
360+
const c2 = {
361+
custom: { a: "apple" },
362+
} as unknown as UserProfile;
363+
364+
// act
365+
const result = subject.mergeClaims(c1, c2);
366+
367+
// assert
368+
expect(result).toEqual({ custom: { a: "apple" }, b: "banana" });
350369
});
351370
});
352371
});

‎src/ClaimsService.ts‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -64,27 +64,27 @@ export class ClaimsService {
6464
return result;
6565
}
6666

67+
public mergeClaims(claims1: JwtClaims, claims2: JwtClaims): UserProfile;
6768
public mergeClaims(claims1: UserProfile, claims2: JwtClaims): UserProfile {
6869
const result = { ...claims1 };
69-
7070
for (const [claim, values] of Object.entries(claims2)) {
71-
for (const value of Array.isArray(values) ? values : [values]) {
72-
const previousValue = result[claim];
73-
if (previousValue === undefined) {
74-
result[claim] = value;
75-
}
76-
else if (Array.isArray(previousValue)) {
77-
if (!previousValue.includes(value)) {
78-
previousValue.push(value);
79-
}
80-
}
81-
else if (result[claim] !== value) {
82-
if (typeof value === "object" && this._settings.mergeClaims) {
83-
result[claim] = this.mergeClaims(previousValue as UserProfile, value);
84-
}
85-
else {
86-
result[claim] = [previousValue, value];
71+
if (result[claim] !== values) {
72+
if (Array.isArray(result[claim]) || Array.isArray(values)) {
73+
if (this._settings.mergeClaimsStrategy.array == "replace") {
74+
result[claim] = values;
75+
} else {
76+
const mergedValues = Array.isArray(result[claim]) ? result[claim] as unknown[] : [result[claim]];
77+
for (const value of Array.isArray(values) ? values : [values]) {
78+
if (!mergedValues.includes(value)) {
79+
mergedValues.push(value);
80+
}
81+
}
82+
result[claim] = mergedValues;
8783
}
84+
} else if (typeof result[claim] === "object" && typeof values === "object") {
85+
result[claim] = this.mergeClaims(result[claim] as JwtClaims, values as JwtClaims);
86+
} else {
87+
result[claim] = values;
8888
}
8989
}
9090
}

‎src/OidcClientSettings.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,12 @@ export interface OidcClientSettings {
8787
staleStateAgeInSeconds?: number;
8888

8989
/**
90-
* 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.
91-
* Otherwise, they are added to an array as distinct objects for the claim type. (default: false)
90+
* Indicates how objects returned from the user info endpoint as claims (e.g. `address`) are merged into the claims from the
91+
* id token as a single object. (default: `{ array: "replace" }`)
92+
* - array: "replace": natives (string, int, float) and arrays are replaced, objects are merged as distinct objects
93+
* - array: "merge": natives (string, int, float) are replaced, arrays and objects are merged as distinct objects
9294
*/
93-
mergeClaims?: boolean;
95+
mergeClaimsStrategy?: { array: "replace" | "merge" };
9496

9597
/**
9698
* Storage object used to persist interaction state (default: window.localStorage, InMemoryWebStorage iff no window).
@@ -167,7 +169,7 @@ export class OidcClientSettingsStore {
167169
public readonly filterProtocolClaims: boolean | string[];
168170
public readonly loadUserInfo: boolean;
169171
public readonly staleStateAgeInSeconds: number;
170-
public readonly mergeClaims: boolean;
172+
public readonly mergeClaimsStrategy: { array: "replace" | "merge" };
171173

172174
public readonly stateStore: StateStore;
173175

@@ -194,7 +196,7 @@ export class OidcClientSettingsStore {
194196
filterProtocolClaims = true,
195197
loadUserInfo = false,
196198
staleStateAgeInSeconds = DefaultStaleStateAgeInSeconds,
197-
mergeClaims = false,
199+
mergeClaimsStrategy = { array: "replace" },
198200
disablePKCE = false,
199201
// other behavior
200202
stateStore,
@@ -244,7 +246,7 @@ export class OidcClientSettingsStore {
244246
this.filterProtocolClaims = filterProtocolClaims ?? true;
245247
this.loadUserInfo = !!loadUserInfo;
246248
this.staleStateAgeInSeconds = staleStateAgeInSeconds;
247-
this.mergeClaims = !!mergeClaims;
249+
this.mergeClaimsStrategy = mergeClaimsStrategy;
248250
this.disablePKCE = !!disablePKCE;
249251
this.revokeTokenAdditionalContentTypes = revokeTokenAdditionalContentTypes;
250252

0 commit comments

Comments
 (0)