Movie client-side popup origin validation to _after_ popup is opened (#4514)

* Make origin validation happen asynchronously from the popup

* Formatting

Author: sam-gc

packages-exp/auth-exp/src/core/auth/initialize.test.ts

@@ -116,6 +116,7 @@ describe('core/auth/initialize', () => {
     ): void {
       cb(true);
     }
+    async _originValidation(): Promise<void> {}
     async _completeRedirectFn(
       _auth: Auth,
       _resolver: PopupRedirectResolver,

packages-exp/auth-exp/src/model/popup_redirect.ts

@@ -98,6 +98,7 @@ export interface PopupRedirectResolverInternal extends PopupRedirectResolver {
     cb: (support: boolean) => unknown
   ): void;
   _redirectPersistence: Persistence;
+  _originValidation(auth: Auth): Promise<void>;
 
   // This is needed so that auth does not have a hard dependency on redirect
   _completeRedirectFn: (

packages-exp/auth-exp/src/platform_browser/popup_redirect.test.ts

@@ -117,13 +117,6 @@ describe('platform_browser/popup_redirect', () => {
       );
     });
 
-    it('validates the origin', async () => {
-      await resolver._initialize(auth);
-
-      await resolver._openPopup(auth, provider, event);
-      expect(validateOrigin._validateOrigin).to.have.been.calledWith(auth);
-    });
-
     it('throws an error if apiKey is unspecified', async () => {
       delete (auth.config as Partial<Config>).apiKey;
       await resolver._initialize(auth);
@@ -132,17 +125,6 @@ describe('platform_browser/popup_redirect', () => {
         resolver._openPopup(auth, provider, event)
       ).to.be.rejectedWith(FirebaseError, 'auth/invalid-api-key');
     });
-
-    it('rejects immediately if origin validation fails', async () => {
-      await resolver._initialize(auth);
-      (validateOrigin._validateOrigin as sinon.SinonStub).returns(
-        Promise.reject(new Error('invalid-origin'))
-      );
-
-      await expect(
-        resolver._openPopup(auth, provider, event)
-      ).to.be.rejectedWith(Error, 'invalid-origin');
-    });
   });
 
   context('#_openRedirect', () => {
@@ -213,6 +195,27 @@ describe('platform_browser/popup_redirect', () => {
     });
   });
 
+  context('#_originValidation', () => {
+    it('validates the origin', async () => {
+      await resolver._initialize(auth);
+
+      await resolver._originValidation(auth);
+      expect(validateOrigin._validateOrigin).to.have.been.calledWith(auth);
+    });
+
+    it('rejects if origin validation fails', async () => {
+      await resolver._initialize(auth);
+      (validateOrigin._validateOrigin as sinon.SinonStub).returns(
+        Promise.reject(new Error('invalid-origin'))
+      );
+
+      await expect(resolver._originValidation(auth)).to.be.rejectedWith(
+        Error,
+        'invalid-origin'
+      );
+    });
+  });
+
   context('#_initialize', () => {
     it('returns different manager for a different auth', async () => {
       const manager = await resolver._initialize(auth);

packages-exp/auth-exp/src/platform_browser/popup_redirect.ts

@@ -73,7 +73,6 @@ class BrowserPopupRedirectResolver implements PopupRedirectResolverInternal {
       '_initialize() not called before _openPopup()'
     );
 
-    await this.originValidation(auth);
     const url = _getRedirectUrl(
       auth,
       provider,
@@ -90,7 +89,7 @@ class BrowserPopupRedirectResolver implements PopupRedirectResolverInternal {
     authType: AuthEventType,
     eventId?: string
   ): Promise<never> {
-    await this.originValidation(auth);
+    await this._originValidation(auth);
     _setWindowLocation(
       _getRedirectUrl(auth, provider, authType, _getCurrentUrl(), eventId)
     );
@@ -154,7 +153,7 @@ class BrowserPopupRedirectResolver implements PopupRedirectResolverInternal {
     );
   }
 
-  private originValidation(auth: AuthInternal): Promise<void> {
+  _originValidation(auth: AuthInternal): Promise<void> {
     const key = auth._key();
     if (!this.originValidationPromises[key]) {
       this.originValidationPromises[key] = _validateOrigin(auth);

packages-exp/auth-exp/src/platform_browser/strategies/popup.ts

@@ -241,13 +241,17 @@ class PopupOperation extends AbstractPopupRedirectOperation {
     );
     this.authWindow.associatedEvent = eventId;
 
-    // Check for web storage support _after_ the popup is loaded. Checking for
-    // web storage is slow (on the order of a second or so). Rather than
-    // waiting on that before opening the window, optimistically open the popup
+    // Check for web storage support and origin validation _after_ the popup is
+    // loaded. These operations are slow (~1 second or so) Rather than
+    // waiting on them before opening the window, optimistically open the popup
     // and check for storage support at the same time. If storage support is
     // not available, this will cause the whole thing to reject properly. It
     // will also close the popup, but since the promise has already rejected,
     // the popup closed by user poll will reject into the void.
+    this.resolver._originValidation(this.auth).catch(e => {
+      this.reject(e);
+    });
+
     this.resolver._isIframeWebStorageSupported(this.auth, isSupported => {
       if (!isSupported) {
         this.reject(

packages-exp/auth-exp/src/platform_cordova/popup_redirect/popup_redirect.ts

@@ -101,6 +101,10 @@ class CordovaPopupRedirectResolver implements PopupRedirectResolverInternal {
     throw new Error('Method not implemented.');
   }
 
+  _originValidation(): Promise<void> {
+    return Promise.resolve();
+  }
+
   private attachCallbackListeners(
     auth: AuthInternal,
     manager: AuthEventManager

packages-exp/auth-exp/test/helpers/mock_popup_redirect_resolver.ts

@@ -57,5 +57,7 @@ export function makeMockPopupRedirectResolver(
     _redirectPersistence?: Persistence;
 
     async _completeRedirectFn(): Promise<void> {}
+
+    async _originValidation(): Promise<void> {}
   };
 }