From ad23eb9ff4cbba84d3a8599de84242182cbce99b Mon Sep 17 00:00:00 2001 From: Gasan Guseinov Date: Fri, 31 Jan 2025 15:28:55 +0100 Subject: [PATCH] allow refresh session when client requests instant popup Signed-off-by: Gasan Guseinov --- .../RefreshingAuthSessionManager.test.ts | 4 ++-- .../RefreshingAuthSessionManager.ts | 19 ++++++------------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts b/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts index 302eb82de0..d5e72a2365 100644 --- a/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts +++ b/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts @@ -138,7 +138,7 @@ describe('RefreshingAuthSessionManager', () => { expect(refreshSession).toHaveBeenCalledWith({ scopes: new Set() }); }); - it('should forward option to instantly show auth popup and not attempt refresh', async () => { + it('should forward option to instantly show auth popup after attempting refresh', async () => { const createSession = jest.fn(); const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); const manager = new RefreshingAuthSessionManager({ @@ -152,7 +152,7 @@ describe('RefreshingAuthSessionManager', () => { scopes: new Set(), instantPopup: true, }); - expect(refreshSession).toHaveBeenCalledTimes(0); + expect(refreshSession).toHaveBeenCalledTimes(1); }); it('should remove session straight away', async () => { diff --git a/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts b/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts index 64a1fe2170..fbb57b4a5e 100644 --- a/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts +++ b/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts @@ -91,14 +91,6 @@ export class RefreshingAuthSessionManager implements SessionManager { if (options.optional) { return undefined; } - - if (options.instantPopup) { - // if `instantPopup`, then can't continue because - // we are in asynchronous control flow. Therefore, the application must not try to - // open the popup later in `connector#createSession(...)` because the browser may block the popup. - // Immediately return error. - throw error; - } // If the refresh attempt fails we assume we don't have a session, so continue to create one } } @@ -106,11 +98,12 @@ export class RefreshingAuthSessionManager implements SessionManager { // The user may still have a valid refresh token in their cookies. Attempt to // initiate a fresh session through the backend using that refresh token. // - // We skip this check if an instant login popup is requested, as we need to - // stay in a synchronous call stack from the user interaction. The downside - // is that the user will sometimes be requested to log in even if they - // already had an existing session. - if (!options.instantPopup && !alreadyTriedToRefreshSession) { + // We can still try to refresh even if client requested instant popup. + // With instant popup option, the client is responsible for providing the user login prompt modal window. + // If control flow executes this code and client requested instant popup, it means that + // must have clicked sign in on the login prompt. The browser allows asynchronous code to open a popup + // if it is caused by a user interaction, clicking on a sign-in button, for example. + if (!alreadyTriedToRefreshSession) { try { const newSession = await this.collapsedSessionRefresh(options.scopes); this.currentSession = newSession;