From a1cc83cc95023f36e4f637e7a64e1227e6b5b5aa Mon Sep 17 00:00:00 2001 From: Benedikt Buchner Date: Fri, 8 Jul 2022 22:56:31 +0200 Subject: [PATCH 1/2] - Add support for OIDC session management error status --- docs/oidc-client-ts.api.md | 5 +++++ src/UserManagerEvents.test.ts | 14 ++++++++++++++ src/UserManagerEvents.ts | 25 ++++++++++++++++++++++++- 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index c92188dc7..bda2235bb 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -941,6 +941,8 @@ export class UserManagerEvents extends AccessTokenEvents { addSilentRenewError(cb: SilentRenewErrorCallback): () => void; addUserLoaded(cb: UserLoadedCallback): () => void; addUserSessionChanged(cb: UserSessionChangedCallback): () => void; + // Warning: (ae-forgotten-export) The symbol "UserSessionErrorCallback" needs to be exported by the entry point index.d.ts + addUserSessionError(cb: UserSessionErrorCallback): () => void; addUserSignedIn(cb: UserSignedInCallback): () => void; addUserSignedOut(cb: UserSignedOutCallback): () => void; addUserUnloaded(cb: UserUnloadedCallback): () => void; @@ -953,12 +955,15 @@ export class UserManagerEvents extends AccessTokenEvents { // @internal (undocumented) _raiseUserSessionChanged(): void; // @internal (undocumented) + _raiseUserSessionError(): void; + // @internal (undocumented) _raiseUserSignedIn(): void; // @internal (undocumented) _raiseUserSignedOut(): void; removeSilentRenewError(cb: SilentRenewErrorCallback): void; removeUserLoaded(cb: UserLoadedCallback): void; removeUserSessionChanged(cb: UserSessionChangedCallback): void; + removeUserSessionError(cb: UserSessionErrorCallback): void; removeUserSignedIn(cb: UserSignedInCallback): void; removeUserSignedOut(cb: UserSignedOutCallback): void; removeUserUnloaded(cb: UserUnloadedCallback): void; diff --git a/src/UserManagerEvents.test.ts b/src/UserManagerEvents.test.ts index cdab47cb5..dbb44aed1 100644 --- a/src/UserManagerEvents.test.ts +++ b/src/UserManagerEvents.test.ts @@ -59,5 +59,19 @@ describe("UserManagerEvents", () => { // assert expect(e).toEqual(expected); }); + + it("should pass error to callback", () => { + // arrange + const e: Error | null = null; + const cb = jest.fn(); + const expected = new Error("boom"); + + // act + subject.addUserSessionError(cb); + subject._raiseUserSessionError(); + + // assert + expect(e).toEqual(expected); + }); }); }); diff --git a/src/UserManagerEvents.ts b/src/UserManagerEvents.ts index 746a14b8a..2dd4f34b7 100644 --- a/src/UserManagerEvents.ts +++ b/src/UserManagerEvents.ts @@ -30,7 +30,10 @@ export type UserSignedOutCallback = () => Promise | void; * @public */ export type UserSessionChangedCallback = () => Promise | void; - +/** + * @public + */ +export type UserSessionErrorCallback = () => Promise | void; /** * @public */ @@ -43,6 +46,7 @@ export class UserManagerEvents extends AccessTokenEvents { private readonly _userSignedIn = new Event<[]>("User signed in"); private readonly _userSignedOut = new Event<[]>("User signed out"); private readonly _userSessionChanged = new Event<[]>("User session changed"); + private readonly _userSessionError = new Event<[]>("User session error"); public constructor(settings: UserManagerSettingsStore) { super({ expiringNotificationTimeInSeconds: settings.accessTokenExpiringNotificationTimeInSeconds }); @@ -163,4 +167,23 @@ export class UserManagerEvents extends AccessTokenEvents { public _raiseUserSessionChanged(): void { this._userSessionChanged.raise(); } + /** + * Add callback: Raised when the user session changed (when `monitorSession` is set). + * @see {@link UserManagerSettings.monitorSession} + */ + public addUserSessionError(cb: UserSessionErrorCallback): () => void { + return this._userSessionError.addHandler(cb); + } + /** + * Remove callback: Raised when the user session changed (when `monitorSession` is set). + */ + public removeUserSessionError(cb: UserSessionErrorCallback): void { + this._userSessionError.removeHandler(cb); + } + /** + * @internal + */ + public _raiseUserSessionError(): void { + this._userSessionError.raise(); + } } From 40fb44970eb8994e61d391105ab068be964404bf Mon Sep 17 00:00:00 2001 From: Benedikt Buchner Date: Fri, 8 Jul 2022 23:22:42 +0200 Subject: [PATCH 2/2] - Add support for OIDC session management error status --- .gitignore | 3 ++ docs/oidc-client-ts.api.md | 6 +++- src/CheckSessionIFrame.ts | 6 ++++ src/SessionMonitor.ts | 40 +++++++++++------------ src/UserManagerEvents.test.ts | 60 ++++++++++++++++++++++++++++------- src/UserManagerSettings.ts | 4 +++ 6 files changed, 85 insertions(+), 34 deletions(-) diff --git a/.gitignore b/.gitignore index efbf017a1..7ff289577 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,6 @@ node_modules/ .vscode/ temp/ .history/ + +.idea +*.iml diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index bda2235bb..ada653b82 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -26,7 +26,7 @@ export class AccessTokenEvents { // @internal (undocumented) export class CheckSessionIFrame { - constructor(_callback: () => Promise, _client_id: string, url: string, _intervalInSeconds: number, _stopOnError: boolean); + constructor(_callback: () => Promise, _client_id: string, url: string, _intervalInSeconds: number, _stopOnError: boolean, propagateUserSessionError: boolean, userSessionErrorCallback: () => void); // (undocumented) load(): Promise; // (undocumented) @@ -988,6 +988,8 @@ export interface UserManagerSettings extends OidcClientSettings { popupWindowFeatures?: PopupWindowFeatures; popupWindowTarget?: string; // (undocumented) + propagateUserSessionError?: boolean; + // (undocumented) query_status_response_type?: string; redirectMethod?: "replace" | "assign"; redirectTarget?: "top" | "self"; @@ -1029,6 +1031,8 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { // (undocumented) readonly popupWindowTarget: string; // (undocumented) + readonly propagateUserSessionError: boolean; + // (undocumented) readonly query_status_response_type: string; // (undocumented) readonly redirectMethod: "replace" | "assign"; diff --git a/src/CheckSessionIFrame.ts b/src/CheckSessionIFrame.ts index 3e5430afc..255027a60 100644 --- a/src/CheckSessionIFrame.ts +++ b/src/CheckSessionIFrame.ts @@ -19,6 +19,8 @@ export class CheckSessionIFrame { url: string, private _intervalInSeconds: number, private _stopOnError: boolean, + private propagateUserSessionError: boolean, + private userSessionErrorCallback: () => void, ) { const parsedUrl = new URL(url); this._frame_origin = parsedUrl.origin; @@ -55,6 +57,10 @@ export class CheckSessionIFrame { if (this._stopOnError) { this.stop(); } + + if (this.propagateUserSessionError) { + this.userSessionErrorCallback(); + } } else if (e.data === "changed") { this._logger.debug("changed message from check session op iframe"); diff --git a/src/SessionMonitor.ts b/src/SessionMonitor.ts index 53f288a8a..43bac93da 100644 --- a/src/SessionMonitor.ts +++ b/src/SessionMonitor.ts @@ -37,8 +37,7 @@ export class SessionMonitor { // doesn't trigger load event. if (user) { void this._start(user); - } - else if (this._userManager.settings.monitorAnonymousSession) { + } else if (this._userManager.settings.monitorAnonymousSession) { const session = await this._userManager.querySessionStatus(); if (session) { const tmpUser = { @@ -69,8 +68,7 @@ export class SessionMonitor { this._sub = user.profile.sub; this._sid = user.profile.sid; logger.debug("session_state", session_state, ", sub", this._sub); - } - else { + } else { this._sub = undefined; this._sid = undefined; logger.debug("session_state", session_state, ", anonymous user"); @@ -90,21 +88,27 @@ export class SessionMonitor { const intervalInSeconds = this._userManager.settings.checkSessionIntervalInSeconds; const stopOnError = this._userManager.settings.stopCheckSessionOnError; - const checkSessionIFrame = new CheckSessionIFrame(this._callback, client_id, url, intervalInSeconds, stopOnError); + const checkSessionIFrame = new CheckSessionIFrame(this._callback, client_id, url, intervalInSeconds, stopOnError, this.doUserSessionErrorPropagation(), this.raiseUserSessionError.bind(this)); await checkSessionIFrame.load(); this._checkSessionIFrame = checkSessionIFrame; checkSessionIFrame.start(session_state); - } - else { + } else { logger.warn("no check session iframe found in the metadata"); } - } - catch (err) { + } catch (err) { // catch to suppress errors since we're in non-promise callback logger.error("Error from getCheckSessionIframe:", err instanceof Error ? err.message : err); } }; + private doUserSessionErrorPropagation(): boolean { + return this._userManager.settings.propagateUserSessionError; + } + + private raiseUserSessionError(): void { + this._userManager.events._raiseUserSessionError.bind(this); + } + protected _stop = (): void => { const logger = this._logger.create("_stop"); this._sub = undefined; @@ -133,8 +137,7 @@ export class SessionMonitor { }; void this._start(tmpUser); } - } - catch (err) { + } catch (err) { // catch to suppress errors since we're in a callback logger.error("error from querySessionStatus", err instanceof Error ? err.message : err); } @@ -155,32 +158,27 @@ export class SessionMonitor { if (session.sid === this._sid) { logger.debug("same sub still logged in at OP, restarting check session iframe; session_state", session.session_state); - } - else { + } else { logger.debug("same sub still logged in at OP, session state has changed, restarting check session iframe; session_state", session.session_state); this._userManager.events._raiseUserSessionChanged(); } - } - else { + } else { logger.debug("different subject signed into OP", session.sub); } - } - else { + } else { logger.debug("subject no longer signed into OP"); } if (raiseEvent) { if (this._sub) { this._userManager.events._raiseUserSignedOut(); - } - else { + } else { this._userManager.events._raiseUserSignedIn(); } } else { logger.debug("no change in session detected, no event to raise"); } - } - catch (err) { + } catch (err) { if (this._sub) { logger.debug("Error calling queryCurrentSigninSession; raising signed out event", err); this._userManager.events._raiseUserSignedOut(); diff --git a/src/UserManagerEvents.test.ts b/src/UserManagerEvents.test.ts index dbb44aed1..feff6710b 100644 --- a/src/UserManagerEvents.test.ts +++ b/src/UserManagerEvents.test.ts @@ -44,32 +44,68 @@ describe("UserManagerEvents", () => { expect(cb).toBeCalledTimes(0); }); - it("should pass error to callback", () => { + it("should allow callback", () => { // arrange - let e: Error | null = null; - const cb = function (arg_e: Error) { - e = arg_e; - }; - const expected = new Error("boom"); + const cb = jest.fn(); // act - subject.addSilentRenewError(cb); - subject._raiseSilentRenewError(expected); + subject.addUserSessionError(cb); + subject._raiseUserSessionError(); // assert - expect(e).toEqual(expected); + expect(cb).toBeCalled(); }); - it("should pass error to callback", () => { + it("should allow unregistering callback", () => { + // arrange + const cb = jest.fn(); + + // act + subject.addUserSessionError(cb); + subject.removeUserSessionError(cb); + subject._raiseUserSessionError(); + + // assert + expect(cb).toBeCalledTimes(0); + }); + + it("should allow callback", () => { // arrange - const e: Error | null = null; const cb = jest.fn(); - const expected = new Error("boom"); // act subject.addUserSessionError(cb); subject._raiseUserSessionError(); + // assert + expect(cb).toBeCalled(); + }); + + it("should allow unregistering callback", () => { + // arrange + const cb = jest.fn(); + + // act + subject.addUserSessionError(cb); + subject.removeUserSessionError(cb); + subject._raiseUserSessionError(); + + // assert + expect(cb).toBeCalledTimes(0); + }); + + it("should pass error to callback", () => { + // arrange + let e: Error | null = null; + const cb = function (arg_e: Error) { + e = arg_e; + }; + const expected = new Error("boom"); + + // act + subject.addSilentRenewError(cb); + subject._raiseSilentRenewError(expected); + // assert expect(e).toEqual(expected); }); diff --git a/src/UserManagerSettings.ts b/src/UserManagerSettings.ts index e289474d5..42040918c 100644 --- a/src/UserManagerSettings.ts +++ b/src/UserManagerSettings.ts @@ -62,6 +62,7 @@ export interface UserManagerSettings extends OidcClientSettings { checkSessionIntervalInSeconds?: number; query_status_response_type?: string; stopCheckSessionOnError?: boolean; + propagateUserSessionError?: boolean; /** * The `token_type_hint`s to pass to the authority server by default (default: ["access_token", "refresh_token"]) @@ -109,6 +110,7 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { public readonly checkSessionIntervalInSeconds: number; public readonly query_status_response_type: string; public readonly stopCheckSessionOnError: boolean; + public readonly propagateUserSessionError: boolean; public readonly revokeTokenTypes: ("access_token" | "refresh_token")[]; public readonly revokeTokensOnSignout: boolean; @@ -139,6 +141,7 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { checkSessionIntervalInSeconds = DefaultCheckSessionIntervalInSeconds, query_status_response_type = "code", stopCheckSessionOnError = true, + propagateUserSessionError = false, revokeTokenTypes = ["access_token", "refresh_token"], revokeTokensOnSignout = false, @@ -170,6 +173,7 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { this.checkSessionIntervalInSeconds = checkSessionIntervalInSeconds; this.stopCheckSessionOnError = stopCheckSessionOnError; this.query_status_response_type = query_status_response_type; + this.propagateUserSessionError = propagateUserSessionError; this.revokeTokenTypes = revokeTokenTypes; this.revokeTokensOnSignout = revokeTokensOnSignout;