Merge pull request #1784 from spotify/rugvip/safari

core-api: ensure instantPopup is synchronous in RefreshingAuthSessionManager

packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts

@@ -137,7 +137,7 @@ describe('RefreshingAuthSessionManager', () => {
     expect(refreshSession).toBeCalledTimes(1);
   });
 
-  it('should forward option to instantly show auth popup', async () => {
+  it('should forward option to instantly show auth popup and not attempt refresh', async () => {
     const createSession = jest.fn();
     const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE'));
     const manager = new RefreshingAuthSessionManager({
@@ -151,7 +151,7 @@ describe('RefreshingAuthSessionManager', () => {
       scopes: new Set(),
       instantPopup: true,
     });
-    expect(refreshSession).toBeCalledTimes(1);
+    expect(refreshSession).toBeCalledTimes(0);
   });
 
   it('should remove session straight away', async () => {

packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts

@@ -90,7 +90,12 @@ export class RefreshingAuthSessionManager<T> implements SessionManager<T> {
 
     // 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.
-    if (!this.currentSession) {
+    //
+    // 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 that the user will sometimes be requested to log in even if they
+    // already had an existing session.
+    if (!this.currentSession && !options.instantPopup) {
       try {
         const newSession = await this.collapsedSessionRefresh();
         this.currentSession = newSession;