diff --git a/docs/migration.md b/docs/migration.md index 7757e4631..d977622d7 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -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 diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index fc28ae363..bc7ba88a1 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -965,6 +965,8 @@ export class UserManager { protected readonly _redirectNavigator: INavigator; removeUser(): Promise; // (undocumented) + protected _removeUser(raiseEvent: boolean): Promise; + // (undocumented) protected _revokeInternal(user: User | null, types?: ("access_token" | "refresh_token")[]): Promise; // (undocumented) revokeTokens(types?: RevokeTokensTypes): Promise; @@ -1040,7 +1042,7 @@ export class UserManagerEvents extends AccessTokenEvents { removeUserSignedOut(cb: UserSignedOutCallback): void; removeUserUnloaded(cb: UserUnloadedCallback): void; // (undocumented) - unload(): Promise; + unload(raiseEvent?: boolean): Promise; } // @public @@ -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; @@ -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"; diff --git a/src/UserManager.test.ts b/src/UserManager.test.ts index 73d49c0de..06ad3c661 100644 --- a/src/UserManager.test.ts +++ b/src/UserManager.test.ts @@ -846,8 +846,12 @@ 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)); @@ -855,6 +859,7 @@ describe("UserManager", () => { navigate: navigateMock, close: () => {}, })); + jest.spyOn(subject["_events"], "unload").mockImplementation(() => Promise.resolve()); const user = new User({ access_token: "access_token", token_type: "token_type", @@ -868,13 +873,24 @@ 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", @@ -882,13 +898,58 @@ describe("UserManager", () => { }); 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); }); }); diff --git a/src/UserManager.ts b/src/UserManager.ts index 204b73c3c..bb9167e2b 100644 --- a/src/UserManager.ts +++ b/src/UserManager.ts @@ -151,10 +151,14 @@ export class UserManager { * @returns A promise */ public async removeUser(): Promise { - const logger = this._logger.create("removeUser"); + await this._removeUser(true); + } + + protected async _removeUser(raiseEvent: boolean): Promise { + 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); } /** @@ -528,6 +532,11 @@ export class UserManager { */ public async signoutRedirect(args: SignoutRedirectArgs = {}): Promise { 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 @@ -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"); @@ -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) { + await this._events.unload(); + } return signoutResponse; } diff --git a/src/UserManagerEvents.ts b/src/UserManagerEvents.ts index a2b063e90..3536bf9f0 100644 --- a/src/UserManagerEvents.ts +++ b/src/UserManagerEvents.ts @@ -54,9 +54,11 @@ export class UserManagerEvents extends AccessTokenEvents { await this._userLoaded.raise(user); } } - public async unload(): Promise { + public async unload(raiseEvent=true): Promise { super.unload(); - await this._userUnloaded.raise(); + if (raiseEvent) { + await this._userUnloaded.raise(); + } } /** diff --git a/src/UserManagerSettings.test.ts b/src/UserManagerSettings.test.ts index e1e3cb397..abd496959 100644 --- a/src/UserManagerSettings.test.ts +++ b/src/UserManagerSettings.test.ts @@ -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); + }); + }); }); diff --git a/src/UserManagerSettings.ts b/src/UserManagerSettings.ts index cf6ae3662..4f87f7711 100644 --- a/src/UserManagerSettings.ts +++ b/src/UserManagerSettings.ts @@ -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. @@ -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 })` @@ -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"; @@ -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", @@ -152,6 +160,8 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { accessTokenExpiringNotificationTimeInSeconds = DefaultAccessTokenExpiringNotificationTimeInSeconds, + raiserUserUnloadEventBeforeSignoutRequest = false, + userStore, } = args; @@ -159,6 +169,7 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { 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; @@ -185,6 +196,8 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { this.accessTokenExpiringNotificationTimeInSeconds = accessTokenExpiringNotificationTimeInSeconds; + this.raiserUserUnloadEventBeforeSignoutRequest = raiserUserUnloadEventBeforeSignoutRequest; + if (userStore) { this.userStore = userStore; }