Skip to content

Commit

Permalink
feat: #1232 improve merge claims behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
@pamapa
pamapa committed Nov 2, 2023
1 parent 2a308c6 commit e5b1e7b
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 70 deletions.
2 changes: 2 additions & 0 deletions docs/migration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ The API is largely backwards-compatible.
- `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
Expand Down
10 changes: 7 additions & 3 deletions docs/oidc-client-ts.api.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
107 changes: 63 additions & 44 deletions src/ClaimsService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down Expand Up @@ -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" },
Expand All @@ -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({
Expand All @@ -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" });
});
});
});
34 changes: 17 additions & 17 deletions src/ClaimsService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
Expand Down
14 changes: 8 additions & 6 deletions src/OidcClientSettings.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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;

Expand All @@ -194,7 +196,7 @@ export class OidcClientSettingsStore {
filterProtocolClaims = true,
loadUserInfo = false,
staleStateAgeInSeconds = DefaultStaleStateAgeInSeconds,
mergeClaims = false,
mergeClaimsStrategy = { array: "replace" },
disablePKCE = false,
// other behavior
stateStore,
Expand Down Expand Up @@ -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;

Expand Down

0 comments on commit e5b1e7b

Please sign in to comment.