From 7bf209736d86d3f8c99ab6d571cd3c62d7dbfe14 Mon Sep 17 00:00:00 2001
From: Gasan Guseinov <gasan.guseinov@ing.com>
Date: Mon, 27 Jan 2025 23:51:03 +0100
Subject: [PATCH] 1. create session if not instant popup if refresh fails 2.
 add optimization to not call refresh session twice 3. add unit test

Signed-off-by: Gasan Guseinov <gasan.guseinov@ing.com>
---
 .../RefreshingAuthSessionManager.test.ts      | 22 +++++++++++++
 .../RefreshingAuthSessionManager.ts           | 33 +++++++++++++++++--
 2 files changed, 53 insertions(+), 2 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 9afc710bdd..302eb82de0 100644
--- a/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts
+++ b/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts
@@ -275,4 +275,26 @@ describe('RefreshingAuthSessionManager', () => {
     expect(refreshSession).toHaveBeenCalledTimes(1);
     expect(createSession).toHaveBeenCalledTimes(1);
   });
+
+  it('should create a new session if refresh fails with existing expired session', async () => {
+    const createSession = jest.fn();
+    const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE'));
+    const manager = new RefreshingAuthSessionManager({
+      connector: { createSession, refreshSession },
+      ...defaultOptions,
+    } as any);
+
+    createSession.mockResolvedValue({
+      scopes: new Set(['a']),
+      expired: true,
+    });
+    await manager.getSession({ scopes: new Set(['a']) });
+    expect(refreshSession).toHaveBeenCalledTimes(1);
+    expect(createSession).toHaveBeenCalledTimes(1);
+
+    await manager.getSession({ scopes: new Set(['a']) });
+    // call refresh session only once
+    expect(refreshSession).toHaveBeenCalledTimes(2);
+    expect(createSession).toHaveBeenCalledTimes(2);
+  });
 });
diff --git a/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts b/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts
index 1978e14d2d..64a1fe2170 100644
--- a/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts
+++ b/packages/core-app-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts
@@ -64,6 +64,7 @@ export class RefreshingAuthSessionManager<T> implements SessionManager<T> {
   }
 
   async getSession(options: GetSessionOptions): Promise<T | undefined> {
+    let alreadyTriedToRefreshSession = false;
     if (
       this.helper.sessionExistsAndHasScope(this.currentSession, options.scopes)
     ) {
@@ -72,6 +73,8 @@ export class RefreshingAuthSessionManager<T> implements SessionManager<T> {
         return this.currentSession!;
       }
 
+      alreadyTriedToRefreshSession = true;
+
       try {
         const refreshedSession = await this.collapsedSessionRefresh(
           options.scopes,
@@ -83,7 +86,20 @@ export class RefreshingAuthSessionManager<T> implements SessionManager<T> {
         }
         return refreshedSession;
       } catch (error) {
-        // If the refresh attempt fails we assume we don't have a session, so continue to create one.
+        this.removeLocalSession();
+
+        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
       }
     }
 
@@ -94,13 +110,14 @@ export class RefreshingAuthSessionManager<T> implements SessionManager<T> {
     // 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) {
+    if (!options.instantPopup && !alreadyTriedToRefreshSession) {
       try {
         const newSession = await this.collapsedSessionRefresh(options.scopes);
         this.currentSession = newSession;
         // The session might not have the scopes requested so go back and check again
         return this.getSession(options);
       } catch {
+        this.removeLocalSession();
         // If the refresh attempt fails we assume we don't have a session, so continue to create one.
       }
     }
@@ -119,6 +136,18 @@ export class RefreshingAuthSessionManager<T> implements SessionManager<T> {
     return this.currentSession;
   }
 
+  /**
+   * Sets `undefined` to this.{@link currentSession} and tells this.{@link stateTracker}, session state tracker,
+   * that a user has signed out.
+   *
+   * Does not propagate session removal to the connector like {@link removeSession}().
+   *
+   */
+  removeLocalSession() {
+    this.currentSession = undefined;
+    this.stateTracker.setIsSignedIn(false);
+  }
+
   async removeSession() {
     this.currentSession = undefined;
     await this.connector.removeSession();