Clears getRedirectResult() in AuthEventManager for a specified Auth instance when it is deleted. (#973)

app.auth().getRedirectResult() => resolves with redirect result provided previous page called a redirect operation.
app.delete(); // deletes auth instance corresponding to app and clears redirect result.
// re-initialize app with same parameters.
app = firebase.initializeApp(config);
app.auth().getRedirectResult() should resolve with null.

This is needed for firebaseui-web react wrapper which on component unmount will delete the firebaseui instance (which deletes the internal auth instance). When the component is mounted again, the previously temp auth instance was still caching the redirect result.
firebase/firebaseui-web#440

Cleans up private property access in AuthEventManager tests.

Author: bojeil-google

packages/auth/src/auth.js

@@ -1830,6 +1830,7 @@ fireauth.Auth.prototype.delete = function() {
   // Unsubscribe from Auth event handling.
   if (this.authEventManager_) {
     this.authEventManager_.unsubscribe(this);
+    this.authEventManager_.clearRedirectResult();
   }
   return firebase.Promise.resolve();
 };

packages/auth/src/autheventmanager.js

@@ -63,7 +63,7 @@ fireauth.AuthEventManager = function(authDomain, apiKey, appName) {
    * @private {boolean} Whether the Auth event manager instance is initialized.
    */
   this.initialized_ = false;
-  /** @private {!function(?fireauth.AuthEvent)} The Auth event handler. */
+  /** @private {function(?fireauth.AuthEvent)} The Auth event handler. */
   this.authEventHandler_ = goog.bind(this.handleAuthEvent_, this);
   /** @private {!fireauth.RedirectAuthEventProcessor} The redirect event
    *      processor. */
@@ -110,6 +110,20 @@ fireauth.AuthEventManager = function(authDomain, apiKey, appName) {
 };
 
 
+/**
+ * @return {!fireauth.RedirectAuthEventProcessor} The redirect event processor.
+ */
+fireauth.AuthEventManager.prototype.getRedirectAuthEventProcessor = function() {
+  return this.redirectAuthEventProcessor_;
+};
+
+
+/** @return {!fireauth.PopupAuthEventProcessor} The popup event processor. */
+fireauth.AuthEventManager.prototype.getPopupAuthEventProcessor = function() {
+  return this.popupAuthEventProcessor_;
+};
+
+
 /**
  * Instantiates an OAuth sign-in handler depending on the current environment
  * and returns it.
@@ -155,6 +169,15 @@ fireauth.AuthEventManager.prototype.reset = function() {
 };
 
 
+/**
+ * Clears the cached redirect result as long as there is no pending redirect
+ * result being processed. Unrecoverable errors will not be cleared.
+ */
+fireauth.AuthEventManager.prototype.clearRedirectResult = function() {
+  this.redirectAuthEventProcessor_.clearRedirectResult();
+};
+
+
 /**
  * @typedef {{
  *   user: (?fireauth.AuthUser|undefined),
@@ -539,7 +562,7 @@ fireauth.AuthEventManager.prototype.startPopupTimeout =
 
 
 /**
- * @private {!Object.<!string, !fireauth.AuthEventManager>} Map containing
+ * @private {!Object.<string, !fireauth.AuthEventManager>} Map containing
  *     Firebase event manager instances keyed by Auth event manager ID.
  */
 fireauth.AuthEventManager.manager_ = {};
@@ -634,9 +657,16 @@ fireauth.RedirectAuthEventProcessor = function(manager) {
   this.redirectReject_ = [];
   /** @private {?goog.Promise} Pending timeout promise for redirect. */
   this.redirectTimeoutPromise_ = null;
-  /** @private {boolean} Whether redirect result is resolved. This is true
-          when a valid Auth event has been triggered. */
+  /**
+   * @private {boolean} Whether redirect result is resolved. This is true
+   *     when a valid Auth event has been triggered.
+   */
   this.redirectResultResolved_ = false;
+  /**
+   * @private {boolean} Whether an unrecoverable error was detected. This
+   *     includes web storage unsupported or operation not allowed errors.
+   */
+  this.unrecoverableErrorDetected_ = false;
 };
 
 
@@ -684,6 +714,8 @@ fireauth.RedirectAuthEventProcessor.prototype.processAuthEvent =
       authEvent.getError() &&
       authEvent.getError()['code'] == 'auth/operation-not-supported-in-this-' +
           'environment';
+  this.unrecoverableErrorDetected_ =
+      !!(isWebStorageNotSupported || isOperationNotSupported);
   // UNKNOWN mode is always triggered on load by iframe when no popup/redirect
   // data is available. If web storage unsupported error is thrown, process as
   // error and not as unknown event. If the operation is not supported in this
@@ -722,6 +754,19 @@ fireauth.RedirectAuthEventProcessor.prototype.defaultToEmptyResponse =
 };
 
 
+/**
+ * Clears the cached redirect result as long as there is no pending redirect
+ * result being processed. Unrecoverable errors will not be cleared.
+ */
+fireauth.RedirectAuthEventProcessor.prototype.clearRedirectResult = function() {
+  // Clear the result if it is already resolved and no unrecoverable errors are
+  // detected.
+  if (this.redirectResultResolved_ && !this.unrecoverableErrorDetected_) {
+    this.setRedirectResult_(false, null, null);
+  }
+};
+
+
 /**
  * Processes the unknown event.
  * @return {!goog.Promise<undefined>}
@@ -921,6 +966,8 @@ fireauth.RedirectAuthEventProcessor.prototype.startRedirectTimeout_ =
       .then(function() {
         // If not resolved yet, reject with timeout error.
         if (!self.redirectedUserPromise_) {
+          // Consider redirect result resolved.
+          self.redirectResultResolved_ = true;
           self.setRedirectResult_(true, null, error);
         }
       });

packages/auth/test/auth_test.js

@@ -1988,7 +1988,8 @@ function testAuth_authEventManager() {
   initializeMockStorage();
   var expectedManager = {
     'subscribe': goog.testing.recordFunction(),
-    'unsubscribe': goog.testing.recordFunction()
+    'unsubscribe': goog.testing.recordFunction(),
+    'clearRedirectResult': goog.testing.recordFunction()
   };
   // Return stub manager.
   stubs.replace(
@@ -2012,10 +2013,14 @@ function testAuth_authEventManager() {
     assertEquals(1, expectedManager.subscribe.getCallCount());
     assertEquals(
         auth1, expectedManager.subscribe.getLastCall().getArgument(0));
+    assertEquals(0, expectedManager.clearRedirectResult.getCallCount());
+    // Delete should trigger unsubscribe and redirect result clearing.
     auth1.delete();
     // After destroy, Auth should be unsubscribed.
     assertEquals(1, expectedManager.subscribe.getCallCount());
     assertEquals(1, expectedManager.unsubscribe.getCallCount());
+    // Redirect result should also be cleared.
+    assertEquals(1, expectedManager.clearRedirectResult.getCallCount());
     assertEquals(
         auth1, expectedManager.unsubscribe.getLastCall().getArgument(0));
     asyncTestCase.signal();

packages/auth/test/autheventmanager_test.js

@@ -1611,7 +1611,12 @@ function testAuthEventManager_nonCordovaIosOrAndroidFileEnvironment() {
   // All popup/redirect methods should fail with operation not supported errors.
   manager.getRedirectResult().thenCatch(function(error) {
     assertErrorEquals(expectedError, error);
-    asyncTestCase.signal();
+    // Clear redirect result will not clear an operation not supported error.
+    manager.clearRedirectResult();
+    manager.getRedirectResult().thenCatch(function(error) {
+      assertErrorEquals(expectedError, error);
+      asyncTestCase.signal();
+    });
   });
   manager.processRedirect('linkViaRedirect', provider, '1234')
       .thenCatch(function(error) {
@@ -1781,7 +1786,12 @@ function testProcessRedirect_error_cordovahandler() {
         assertFalse(status);
         manager.getRedirectResult().thenCatch(function(error) {
           assertErrorEquals(expectedError, error);
-          asyncTestCase.signal();
+          // Clear redirect result should clear recoverable errors.
+          manager.clearRedirectResult();
+          manager.getRedirectResult().then(function(result) {
+            assertNull(result.user);
+            asyncTestCase.signal();
+          });
         });
       });
 }
@@ -1902,7 +1912,12 @@ function testGetRedirectResult_success_cordovahandler() {
     // Initial expected result should resolve.
     manager.getRedirectResult().then(function(result) {
       assertEquals(expectedResult, result);
-      asyncTestCase.signal();
+      // Clear redirect result should clear successful results.
+      manager.clearRedirectResult();
+      manager.getRedirectResult().then(function(result) {
+        assertNull(result.user);
+        asyncTestCase.signal();
+      });
     });
   });
 }
@@ -1946,7 +1961,12 @@ function testGetRedirectResult_error_cordovahandler() {
     // Initial expected result should resolve.
     manager.getRedirectResult().thenCatch(function(error) {
       assertErrorEquals(expectedError, error);
-      asyncTestCase.signal();
+      // Clear redirect result should clear recoverable errors.
+      manager.clearRedirectResult();
+      manager.getRedirectResult().then(function(result) {
+        assertNull(result.user);
+        asyncTestCase.signal();
+      });
     });
   });
 }
@@ -2177,7 +2197,7 @@ function testProcessAuthEvent_invalidAuthEvent() {
   var expectedAuthEvent = null;
   var manager = fireauth.AuthEventManager.getManager(
       authDomain1, apiKey1, appName1);
-  manager.redirectAuthEventProcessor_.processAuthEvent(
+  manager.getRedirectAuthEventProcessor().processAuthEvent(
       expectedAuthEvent, handler).thenCatch(function(error) {
         assertErrorEquals(
             new fireauth.AuthError(fireauth.authenum.Error.INVALID_AUTH_EVENT),
@@ -2204,7 +2224,7 @@ function testProcessAuthEvent_unknownAuthEvent() {
     assertObjectEquals(expectedResult, result);
     asyncTestCase.signal();
   });
-  manager.redirectAuthEventProcessor_.processAuthEvent(
+  manager.getRedirectAuthEventProcessor().processAuthEvent(
       expectedAuthEvent, handler).then(function() {
     asyncTestCase.signal();
   });
@@ -2261,7 +2281,13 @@ function testProcessAuthEvent_unknownAuthEvent_webStorageNotSupported() {
     manager.subscribe(handler);
     manager.getRedirectResult().thenCatch(function(error) {
       assertErrorEquals(expectedError, error);
-      asyncTestCase.signal();
+      // Unrecoverable errors like unsupported web storage should not be
+      // cleared.
+      manager.clearRedirectResult();
+      manager.getRedirectResult().thenCatch(function(error) {
+        assertErrorEquals(expectedError, error);
+        asyncTestCase.signal();
+      });
     });
   });
 }
@@ -2279,7 +2305,7 @@ function testProcessAuthEvent_popupErrorAuthEvent() {
       new fireauth.AuthError(fireauth.authenum.Error.INTERNAL_ERROR));
   var manager = fireauth.AuthEventManager.getManager(
       authDomain1, apiKey1, appName1);
-  manager.popupAuthEventProcessor_.processAuthEvent(
+  manager.getPopupAuthEventProcessor().processAuthEvent(
       expectedAuthEvent, handler).then(function() {
     // Resolve popup with error.
     assertEquals(1, handler.resolvePendingPopupEvent.getCallCount());
@@ -2316,7 +2342,7 @@ function testProcessAuthEvent_redirectErrorAuthEvent() {
     assertErrorEquals(expectedError, error);
     asyncTestCase.signal();
   });
-  manager.redirectAuthEventProcessor_.processAuthEvent(
+  manager.getRedirectAuthEventProcessor().processAuthEvent(
       expectedAuthEvent, handler).then(function() {
     // Should not be called as event is not a popup type.
     assertEquals(0, handler.resolvePendingPopupEvent.getCallCount());
@@ -2348,7 +2374,7 @@ function testProcessAuthEvent_finisher_successfulPopupAuthEvent() {
       'SESSION_ID');
   var manager = fireauth.AuthEventManager.getManager(
       authDomain1, apiKey1, appName1);
-  manager.popupAuthEventProcessor_.processAuthEvent(
+  manager.getPopupAuthEventProcessor().processAuthEvent(
       expectedAuthEvent, handler).then(function() {
     // Resolve popup with success.
     assertEquals(1, handler.resolvePendingPopupEvent.getCallCount());
@@ -2389,7 +2415,7 @@ function testProcessAuthEvent_finisher_errorPopupAuthEvent() {
       'SESSION_ID');
   var manager = fireauth.AuthEventManager.getManager(
       authDomain1, apiKey1, appName1);
-  manager.popupAuthEventProcessor_.processAuthEvent(
+  manager.getPopupAuthEventProcessor().processAuthEvent(
       expectedAuthEvent, handler).then(function() {
     // Resolve popup with error.
     assertEquals(1, handler.resolvePendingPopupEvent.getCallCount());
@@ -2439,7 +2465,7 @@ function testProcessAuthEvent_finisher_successfulRedirectAuthEvent() {
     assertObjectEquals(expectedRedirectResult, result);
     asyncTestCase.signal();
   });
-  manager.redirectAuthEventProcessor_.processAuthEvent(
+  manager.getRedirectAuthEventProcessor().processAuthEvent(
       expectedAuthEvent, handler).then(function() {
     // Popup resolve should not be called as this is not a popup event.
     assertEquals(0, handler.resolvePendingPopupEvent.getCallCount());
@@ -2472,7 +2498,7 @@ function testProcessAuthEvent_finisher_errorRedirectAuthEvent() {
     assertErrorEquals(expectedError, error);
     asyncTestCase.signal();
   });
-  manager.redirectAuthEventProcessor_.processAuthEvent(
+  manager.getRedirectAuthEventProcessor().processAuthEvent(
       expectedAuthEvent, handler).then(function() {
     // Popup resolve should not be called as this is not a popup event.
     assertEquals(0, handler.resolvePendingPopupEvent.getCallCount());
@@ -2494,7 +2520,7 @@ function testProcessAuthEvent_noHandler() {
       'SESSION_ID');
   var manager = fireauth.AuthEventManager.getManager(
       authDomain1, apiKey1, appName1);
-  manager.redirectAuthEventProcessor_.processAuthEvent(
+  manager.getRedirectAuthEventProcessor().processAuthEvent(
       expectedAuthEvent, handler).thenCatch(function(error) {
         assertErrorEquals(expectedError, error);
         asyncTestCase.signal();
@@ -2556,7 +2582,12 @@ function testRedirectResult_timeout() {
       authDomain1, apiKey1, appName1);
   manager.getRedirectResult().thenCatch(function(error) {
     assertErrorEquals(expectedError, error);
-    asyncTestCase.signal();
+    // Redirect results with recoverable errors should be cleared.
+    manager.clearRedirectResult();
+    manager.getRedirectResult().then(function(result) {
+      assertNull(result.user);
+      asyncTestCase.signal();
+    });
   });
   // Speed up timeout.
   clock.tick(timeoutDelay);
@@ -2567,7 +2598,9 @@ function testRedirectResult_overwritePreviousRedirectResult() {
   // Once redirect result is determined, it can still be overwritten, though
   // in some cases like ifchandler which gets redirect result from
   // sessionStorage, this should not happen.
-  asyncTestCase.waitForSignals(4);
+  // Test also that clearRedirectResult will clear the cached result as long as
+  // the operation is not pending.
+  asyncTestCase.waitForSignals(2);
   handler.getAuthEventHandlerFinisher = function(mode, eventId) {
     return function(requestUri, sessionId) {
       // Iterate through results array each call.
@@ -2595,21 +2628,32 @@ function testRedirectResult_overwritePreviousRedirectResult() {
     assertObjectEquals(expectedRedirectResult, result);
     asyncTestCase.signal();
   });
-  manager.redirectAuthEventProcessor_.processAuthEvent(
+  // Clear redirect result on a pending operation should have no effect.
+  // Above getRedirectResult() should still resolve with expected first result.
+  manager.clearRedirectResult();
+  manager.getRedirectAuthEventProcessor().processAuthEvent(
       expectedAuthEvent, handler).then(function() {
     // Popup resolve should not be called as this is not a popup event.
     assertEquals(0, handler.resolvePendingPopupEvent.getCallCount());
-    asyncTestCase.signal();
+    // Clear redirect result after resolved operation should reset result.
+    manager.clearRedirectResult();
+    return manager.getRedirectResult();
+  }).then(function(result) {
+    assertNull(result.user);
     // Call Second time.
-    manager.redirectAuthEventProcessor_.processAuthEvent(
-        expectedAuthEvent, handler).then(function() {
-      assertEquals(0, handler.resolvePendingPopupEvent.getCallCount());
-      asyncTestCase.signal();
-      // getRedirectResult should resolve with the second expected result.
-      manager.getRedirectResult().then(function(result) {
-        assertObjectEquals(expectedRedirectResult2, result);
-        asyncTestCase.signal();
-      });
-    });
+    return manager.getRedirectAuthEventProcessor().processAuthEvent(
+        expectedAuthEvent, handler);
+  }).then(function() {
+    assertEquals(0, handler.resolvePendingPopupEvent.getCallCount());
+    // getRedirectResult should resolve with the second expected result.
+    return manager.getRedirectResult();
+  }).then(function(result) {
+    assertObjectEquals(expectedRedirectResult2, result);
+    // Clear redirect result after resolved operation should reset result.
+    manager.clearRedirectResult();
+    return manager.getRedirectResult();
+  }).then(function(result) {
+    assertNull(result.user);
+    asyncTestCase.signal();
   });
 }