Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions docs/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ The behavior of merging claims has been improved.
- the `mergeClaims` has been replaced by `mergeClaimsStrategy`
- if the previous behavior is required `mergeClaimsStrategy: { array: "merge" }` comes close to it
- default of `response_mode` changed from `query` → `undefined`
- when using `signoutRedirect` a working callback is required to remove the user and raise an event. As usual
either call `signoutCallback` or `signoutRedirectCallback` in this situation.
- when using `signoutRedirect` the user unload event is raised after the signout request (within callback)
- if not already done, implement that callback by using `signoutCallback` or `signoutRedirectCallback`
- if the previous behavior is required `raiserUserUnloadEventBeforeSignoutRequest: true` can be used


## oidc-client v1.11.5 → oidc-client-ts v2.0.0
Expand Down
7 changes: 6 additions & 1 deletion docs/oidc-client-ts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,8 @@ export class UserManager {
protected readonly _redirectNavigator: INavigator;
removeUser(): Promise<void>;
// (undocumented)
protected _removeUser(raiseEvent: boolean): Promise<void>;
// (undocumented)
protected _revokeInternal(user: User | null, types?: ("access_token" | "refresh_token")[]): Promise<void>;
// (undocumented)
revokeTokens(types?: RevokeTokensTypes): Promise<void>;
Expand Down Expand Up @@ -1040,7 +1042,7 @@ export class UserManagerEvents extends AccessTokenEvents {
removeUserSignedOut(cb: UserSignedOutCallback): void;
removeUserUnloaded(cb: UserUnloadedCallback): void;
// (undocumented)
unload(): Promise<void>;
unload(raiseEvent?: boolean): Promise<void>;
}

// @public
Expand All @@ -1062,6 +1064,7 @@ export interface UserManagerSettings extends OidcClientSettings {
popupWindowTarget?: string;
// (undocumented)
query_status_response_type?: string;
raiserUserUnloadEventBeforeSignoutRequest?: boolean;
redirectMethod?: "replace" | "assign";
redirectTarget?: "top" | "self";
revokeTokensOnSignout?: boolean;
Expand Down Expand Up @@ -1106,6 +1109,8 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore {
// (undocumented)
readonly query_status_response_type: string;
// (undocumented)
readonly raiserUserUnloadEventBeforeSignoutRequest: boolean;
// (undocumented)
readonly redirectMethod: "replace" | "assign";
// (undocumented)
readonly redirectTarget: "top" | "self";
Expand Down
75 changes: 68 additions & 7 deletions src/UserManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -846,15 +846,20 @@ describe("UserManager", () => {
});

describe("signoutRedirect", () => {
it("should not unload user to avoid race condition between actual signout and signout event handlers", async () => {
it("should remove user and send unload event (raiserUserUnloadEventBeforeSignoutRequest=true)", async () => {
// arrange
subject = new UserManager({
...subject.settings,
post_logout_redirect_uri: "post_logout_redirect_uri",
raiserUserUnloadEventBeforeSignoutRequest: true });
const navigateMock = jest.fn().mockReturnValue(Promise.resolve({
url: "http://localhost:8080",
} as NavigateResponse));
jest.spyOn(subject["_redirectNavigator"], "prepare").mockReturnValue(Promise.resolve({
navigate: navigateMock,
close: () => {},
}));
jest.spyOn(subject["_events"], "unload").mockImplementation(() => Promise.resolve());
const user = new User({
access_token: "access_token",
token_type: "token_type",
Expand All @@ -868,27 +873,83 @@ describe("UserManager", () => {
// assert
expect(navigateMock).toHaveBeenCalledTimes(1);
const storageString = await subject.settings.userStore.get(subject["_userStoreKey"]);
expect(storageString).not.toBeNull();
expect(storageString).toBeNull();
expect(subject["_events"].unload).toHaveBeenCalledWith(true);
});
});

describe("signoutRedirectCallback", () => {
it("should unload user", async () => {
it("should remove user and send defer unload event (raiserUserUnloadEventBeforeSignoutRequest=false)", async () => {
// arrange
subject = new UserManager({
...subject.settings,
post_logout_redirect_uri: "post_logout_redirect_uri",
raiserUserUnloadEventBeforeSignoutRequest: false });
const navigateMock = jest.fn().mockReturnValue(Promise.resolve({
url: "http://localhost:8080",
} as NavigateResponse));
jest.spyOn(subject["_redirectNavigator"], "prepare").mockReturnValue(Promise.resolve({
navigate: navigateMock,
close: () => {},
}));
jest.spyOn(subject["_events"], "unload").mockImplementation(() => Promise.resolve());
const user = new User({
access_token: "access_token",
token_type: "token_type",
profile: {} as UserProfile,
});
await subject.storeUser(user);

expect(await subject.settings.userStore.get(subject["_userStoreKey"])).not.toBeNull();
// act
await subject.signoutRedirect();

// assert
expect(navigateMock).toHaveBeenCalledTimes(1);
const storageString = await subject.settings.userStore.get(subject["_userStoreKey"]);
expect(storageString).toBeNull();
expect(subject["_events"].unload).toHaveBeenCalledWith(false);
});

it("should throw an error for invalid settings", async () => {
// arrange
subject = new UserManager({
...subject.settings,
raiserUserUnloadEventBeforeSignoutRequest: false });

// act
await expect(
subject.signoutRedirect(),
)
// assert
.rejects.toThrow();
});
});

describe("signoutRedirectCallback", () => {
it("should not raise unload event (raiserUserUnloadEventBeforeSignoutRequest=true)", async () => {
// arrange
subject = new UserManager({
...subject.settings,
raiserUserUnloadEventBeforeSignoutRequest: true });
jest.spyOn(subject["_events"], "unload").mockImplementation(() => Promise.resolve());

// act
await subject.signoutRedirectCallback();

// assert
expect(subject["_events"].unload).not.toHaveBeenCalled();
});

it("should not raise unload event (raiserUserUnloadEventBeforeSignoutRequest=false)", async () => {
// arrange
subject = new UserManager({
...subject.settings,
raiserUserUnloadEventBeforeSignoutRequest: false });
jest.spyOn(subject["_events"], "unload").mockImplementation(() => Promise.resolve());

// act
await subject.signoutRedirectCallback();

// assert
expect(await subject.settings.userStore.get(subject["_userStoreKey"])).toBeNull();
expect(subject["_events"].unload).toHaveBeenCalledTimes(1);
});
});

Expand Down
22 changes: 18 additions & 4 deletions src/UserManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,14 @@ export class UserManager {
* @returns A promise
*/
public async removeUser(): Promise<void> {
const logger = this._logger.create("removeUser");
await this._removeUser(true);
}

protected async _removeUser(raiseEvent: boolean): Promise<void> {
const logger = this._logger.create("_removeUser");
await this.storeUser(null);
logger.info("user removed from storage");
await this._events.unload();
await this._events.unload(raiseEvent);
}

/**
Expand Down Expand Up @@ -528,6 +532,11 @@ export class UserManager {
*/
public async signoutRedirect(args: SignoutRedirectArgs = {}): Promise<void> {
const logger = this._logger.create("signoutRedirect");

if (!this.settings.raiserUserUnloadEventBeforeSignoutRequest && !this.settings.post_logout_redirect_uri) {
throw new Error("post_logout_redirect_uri"); // to raise unload event
}

const {
redirectMethod,
...requestArgs
Expand Down Expand Up @@ -620,6 +629,10 @@ export class UserManager {
args.id_token_hint = id_token;
}

await this._removeUser(this.settings.raiserUserUnloadEventBeforeSignoutRequest);
logger.debug("user removed, creating signout request");

logger.debug("creating signout request");
const signoutRequest = await this._client.createSignoutRequest(args);
logger.debug("got signout request");

Expand All @@ -641,8 +654,9 @@ export class UserManager {
const signoutResponse = await this._client.processSignoutResponse(url);
logger.debug("got signout response");

await this.removeUser();
logger.debug("user removed");
if (!this.settings.raiserUserUnloadEventBeforeSignoutRequest) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should just call await this._removeUser(true) here, it should not hurt and is simpler (less code)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be ok as a signed in user would be unexpected in any case, right?

await this._events.unload();
}

return signoutResponse;
}
Expand Down
6 changes: 4 additions & 2 deletions src/UserManagerEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,11 @@ export class UserManagerEvents extends AccessTokenEvents {
await this._userLoaded.raise(user);
}
}
public async unload(): Promise<void> {
public async unload(raiseEvent=true): Promise<void> {
super.unload();
await this._userUnloaded.raise();
if (raiseEvent) {
await this._userUnloaded.raise();
}
}

/**
Expand Down
26 changes: 26 additions & 0 deletions src/UserManagerSettings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,4 +376,30 @@ describe("UserManagerSettings", () => {
expect(subject.stopCheckSessionOnError).toEqual(true);
});
});

describe("raiserUserUnloadEventBeforeSignoutRequest", () => {
it("should return value from initial settings", () => {
// act
const subject = new UserManagerSettingsStore({
authority: "authority",
client_id: "client",
redirect_uri: "redirect",
raiserUserUnloadEventBeforeSignoutRequest : true,
});

// assert
expect(subject.stopCheckSessionOnError).toEqual(true);
});
it("should use default value", () => {
// act
const subject = new UserManagerSettingsStore({
authority: "authority",
client_id: "client",
redirect_uri: "redirect",
});

// assert
expect(subject.raiserUserUnloadEventBeforeSignoutRequest).toEqual(false);
});
});
});
13 changes: 13 additions & 0 deletions src/UserManagerSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export interface UserManagerSettings extends OidcClientSettings {
/** The URL for the page containing the call to signinPopupCallback to handle the callback from the OIDC/OAuth2 */
popup_redirect_uri?: string;
popup_post_logout_redirect_uri?: string;

/**
* The features parameter to window.open for the popup signin window. By default, the popup is
* placed centered in front of the window opener.
Expand Down Expand Up @@ -78,6 +79,9 @@ export interface UserManagerSettings extends OidcClientSettings {
/** The number of seconds before an access token is to expire to raise the accessTokenExpiring event (default: 60) */
accessTokenExpiringNotificationTimeInSeconds?: number;

/** Raise user unload event before the sending the signout request, otherwise its raised within the logout callback (default: false) */
raiserUserUnloadEventBeforeSignoutRequest?: boolean;

/**
* Storage object used to persist User for currently authenticated user (default: window.sessionStorage, InMemoryWebStorage iff no window).
* E.g. `userStore: new WebStorageStateStore({ store: window.localStorage })`
Expand All @@ -94,6 +98,7 @@ export interface UserManagerSettings extends OidcClientSettings {
export class UserManagerSettingsStore extends OidcClientSettingsStore {
public readonly popup_redirect_uri: string;
public readonly popup_post_logout_redirect_uri: string | undefined;

public readonly popupWindowFeatures: PopupWindowFeatures;
public readonly popupWindowTarget: string;
public readonly redirectMethod: "replace" | "assign";
Expand All @@ -120,12 +125,15 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore {

public readonly accessTokenExpiringNotificationTimeInSeconds: number;

public readonly raiserUserUnloadEventBeforeSignoutRequest: boolean;

public readonly userStore: WebStorageStateStore;

public constructor(args: UserManagerSettings) {
const {
popup_redirect_uri = args.redirect_uri,
popup_post_logout_redirect_uri = args.post_logout_redirect_uri,

popupWindowFeatures = DefaultPopupWindowFeatures,
popupWindowTarget = DefaultPopupTarget,
redirectMethod = "assign",
Expand All @@ -152,13 +160,16 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore {

accessTokenExpiringNotificationTimeInSeconds = DefaultAccessTokenExpiringNotificationTimeInSeconds,

raiserUserUnloadEventBeforeSignoutRequest = false,

userStore,
} = args;

super(args);

this.popup_redirect_uri = popup_redirect_uri;
this.popup_post_logout_redirect_uri = popup_post_logout_redirect_uri;

this.popupWindowFeatures = popupWindowFeatures;
this.popupWindowTarget = popupWindowTarget;
this.redirectMethod = redirectMethod;
Expand All @@ -185,6 +196,8 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore {

this.accessTokenExpiringNotificationTimeInSeconds = accessTokenExpiringNotificationTimeInSeconds;

this.raiserUserUnloadEventBeforeSignoutRequest = raiserUserUnloadEventBeforeSignoutRequest;

if (userStore) {
this.userStore = userStore;
}
Expand Down