PR feedback

Author: sam-gc

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

@@ -20,10 +20,9 @@ import * as sinon from 'sinon';
 import * as sinonChai from 'sinon-chai';
 
 import {
-  AuthEvent,
-  AuthEventConsumer,
-  AuthEventType
+    AuthEvent, AuthEventConsumer, AuthEventError, AuthEventType
 } from '../../model/popup_redirect';
+import { AuthErrorCode } from '../errors';
 import { AuthEventManager } from './auth_event_manager';
 
 use(sinonChai);
@@ -37,7 +36,8 @@ describe('src/core/auth/auth_event_manager', () => {
     const stub = sinon.stub({
       filter,
       isMatchingEvent: () => true,
-      onAuthEvent: () => {}
+      onAuthEvent: () => {},
+      onError: () => {}
     });
 
     // Make isMatchingEvent call through by default
@@ -54,7 +54,7 @@ describe('src/core/auth/auth_event_manager', () => {
   }
 
   beforeEach(() => {
-    manager = new AuthEventManager();
+    manager = new AuthEventManager('app-name');
   });
 
   it('multiple consumers may be registered for one event type', () => {
@@ -107,4 +107,33 @@ describe('src/core/auth/auth_event_manager', () => {
     expect(consumer.onAuthEvent).not.to.have.been.called;
     expect(consumer.isMatchingEvent).to.have.been.called;
   });
+
+  it('converts errors into FirebaseError if the type matches', () => {
+    const consumer = makeConsumer(AuthEventType.REAUTH_VIA_POPUP);
+    manager.registerConsumer(consumer);
+    const event = makeEvent(AuthEventType.REAUTH_VIA_POPUP);
+    event.error = {
+      code: `auth/${AuthErrorCode.INVALID_APP_CREDENTIAL}`,
+      message: 'foo',
+      name: 'name',
+    };
+
+    manager.onEvent(event);
+    const error = consumer.onError.getCall(0).args[0];
+    expect(error.code).to.eq(`auth/${AuthErrorCode.INVALID_APP_CREDENTIAL}`);
+  });
+
+  it('converts random errors into FirebaseError with internal error', () => {
+    const consumer = makeConsumer(AuthEventType.REAUTH_VIA_POPUP);
+    manager.registerConsumer(consumer);
+    const event = makeEvent(AuthEventType.REAUTH_VIA_POPUP);
+    event.error = {
+      message: 'foo',
+      name: 'name',
+    } as AuthEventError;
+
+    manager.onEvent(event);
+    const error = consumer.onError.getCall(0).args[0];
+    expect(error.code).to.eq(`auth/${AuthErrorCode.INTERNAL_ERROR}`);
+  });
 });

packages-exp/auth-exp/src/core/auth/auth_event_manager.ts

@@ -15,15 +15,14 @@
  * limitations under the License.
  */
 
-import {
-  AuthEvent,
-  AuthEventConsumer,
-  EventManager
-} from '../../model/popup_redirect';
+import { AuthEvent, AuthEventConsumer, EventManager } from '../../model/popup_redirect';
+import { AUTH_ERROR_FACTORY, AuthErrorCode } from '../errors';
 
 export class AuthEventManager implements EventManager {
   private readonly consumers: Set<AuthEventConsumer> = new Set();
 
+  constructor(private readonly appName: string) {}
+
   registerConsumer(authEventConsumer: AuthEventConsumer): void {
     this.consumers.add(authEventConsumer);
   }
@@ -38,7 +37,15 @@ export class AuthEventManager implements EventManager {
         consumer.filter === event.type &&
         consumer.isMatchingEvent(event.eventId)
       ) {
-        consumer.onAuthEvent(event);
+        if (event.error) {
+          console.error('ERROR');
+          const code = event.error.code?.split('auth/')[1] as AuthErrorCode || AuthErrorCode.INTERNAL_ERROR;
+          consumer.onError(AUTH_ERROR_FACTORY.create(code, {
+            appName: this.appName
+          }));
+        } else {
+          consumer.onAuthEvent(event);
+        }
       }
     });
   }

packages-exp/auth-exp/src/core/providers/oauth.ts

@@ -32,7 +32,7 @@ export class OAuthProvider implements externs.AuthProvider {
   defaultLanguageCode: string | null = null;
   private scopes: string[] = [];
   private customParameters: CustomParameters = {};
-  private constructor(readonly providerId: externs.ProviderId) {}
+  constructor(readonly providerId: externs.ProviderId) {}
   static credentialFromResult(
     _userCredential: externs.UserCredential
   ): externs.OAuthCredential | null {

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

@@ -16,6 +16,7 @@
  */
 
 import * as externs from '@firebase/auth-types-exp';
+import { FirebaseError } from '@firebase/util';
 
 import { AuthPopup } from '../core/util/popup';
 import { Auth } from './auth';
@@ -45,7 +46,6 @@ export const enum AuthEventType {
   VERIFY_APP = 'verifyApp'
 }
 
-// TODO: convert from these to FirebaseError
 export interface AuthEventError extends Error {
   code: string; // in the form of auth/${AuthErrorCode}
   message: string;
@@ -65,6 +65,7 @@ export interface AuthEventConsumer {
   readonly filter: AuthEventType;
   isMatchingEvent(eventId: string | null): boolean;
   onAuthEvent(event: AuthEvent): unknown;
+  onError(error: FirebaseError): unknown;
 }
 
 export interface EventManager {
@@ -73,8 +74,8 @@ export interface EventManager {
 }
 
 export interface PopupRedirectResolver extends externs.PopupRedirectResolver {
-  initialize(auth: Auth): Promise<EventManager>;
-  openPopup(
+  _initialize(auth: Auth): Promise<EventManager>;
+  _openPopup(
     auth: Auth,
     provider: externs.AuthProvider,
     authType: AuthEventType,

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

@@ -28,11 +28,7 @@ import { TEST_AUTH_DOMAIN, TEST_KEY, testAuth } from '../../test/mock_auth';
 import { AuthEventManager } from '../core/auth/auth_event_manager';
 import { OAuthProvider } from '../core/providers/oauth';
 import { Auth } from '../model/auth';
-import {
-  AuthEvent,
-  AuthEventType,
-  GapiAuthEvent
-} from '../model/popup_redirect';
+import { AuthEvent, AuthEventType, GapiAuthEvent } from '../model/popup_redirect';
 import * as gapiLoader from './iframe/gapi';
 import { BrowserPopupRedirectResolver } from './popup_redirect';
 
@@ -70,7 +66,7 @@ describe('src/platform_browser/popup_redirect', () => {
       provider.addScope('some-scope-b');
       provider.setCustomParameters({ foo: 'bar' });
 
-      await resolver.openPopup(auth, provider, event);
+      await resolver._openPopup(auth, provider, event);
       expect(popupUrl).to.include(
         `https://${TEST_AUTH_DOMAIN}/__/auth/handler`
       );
@@ -88,15 +84,15 @@ describe('src/platform_browser/popup_redirect', () => {
       delete auth.config.authDomain;
 
       await expect(
-        resolver.openPopup(auth, provider, event)
+        resolver._openPopup(auth, provider, event)
       ).to.be.rejectedWith(FirebaseError, 'auth/auth-domain-config-required');
     });
 
     it('throws an error if apiKey is unspecified', async () => {
       delete auth.config.apiKey;
 
       await expect(
-        resolver.openPopup(auth, provider, event)
+        resolver._openPopup(auth, provider, event)
       ).to.be.rejectedWith(FirebaseError, 'auth/invalid-api-key');
     });
   });
@@ -118,12 +114,12 @@ describe('src/platform_browser/popup_redirect', () => {
     });
 
     it('only registers once, returns same event manager', async () => {
-      const manager = await resolver.initialize(auth);
-      expect(await resolver.initialize(auth)).to.eq(manager);
+      const manager = await resolver._initialize(auth);
+      expect(await resolver._initialize(auth)).to.eq(manager);
     });
 
     it('iframe event goes through to the manager', async () => {
-      const manager = (await resolver.initialize(auth)) as AuthEventManager;
+      const manager = (await resolver._initialize(auth)) as AuthEventManager;
       sinon.spy(manager, 'onEvent');
       const response = await onIframeMessage({
         type: 'authEvent',

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

@@ -22,17 +22,13 @@ import { isEmpty, querystring } from '@firebase/util';
 import { AuthEventManager } from '../core/auth/auth_event_manager';
 import { AuthErrorCode } from '../core/errors';
 import { OAuthProvider } from '../core/providers/oauth';
-import { assert } from '../core/util/assert';
+import { assert, debugAssert } from '../core/util/assert';
 import { _generateEventId } from '../core/util/event_id';
 import { _getCurrentUrl } from '../core/util/location';
 import { _open, AuthPopup } from '../core/util/popup';
 import { ApiKey, AppName, Auth } from '../model/auth';
 import {
-  AuthEventType,
-  EventManager,
-  GapiAuthEvent,
-  GapiOutcome,
-  PopupRedirectResolver
+    AuthEventType, EventManager, GapiAuthEvent, GapiOutcome, PopupRedirectResolver
 } from '../model/popup_redirect';
 import { _openIframe } from './iframe/iframe';
 
@@ -43,26 +39,36 @@ const WIDGET_URL = '__/auth/handler';
 
 export class BrowserPopupRedirectResolver implements PopupRedirectResolver {
   private eventManager: EventManager | null = null;
+  private initializationPromise: Promise<EventManager>|null = null;
 
   // Wrapping in async even though we don't await anywhere in order
   // to make sure errors are raised as promise rejections
-  async openPopup(
+  async _openPopup(
     auth: Auth,
     provider: externs.AuthProvider,
     authType: AuthEventType,
     eventId?: string
   ): Promise<AuthPopup> {
+    debugAssert(this.eventManager, '_initialize() not called before _openPopup()');
     const url = getRedirectUrl(auth, provider, authType, eventId);
     return _open(auth.name, url, _generateEventId());
   }
 
-  async initialize(auth: Auth): Promise<EventManager> {
+  _initialize(auth: Auth): Promise<EventManager> {
     if (this.eventManager) {
-      return this.eventManager;
+      return Promise.resolve(this.eventManager);
     }
 
+    if (!this.initializationPromise) {
+      this.initializationPromise = this.initAndGetManager(auth);
+    }
+
+    return this.initializationPromise;
+  }
+
+  private async initAndGetManager(auth: Auth): Promise<EventManager> {
     const iframe = await _openIframe(auth);
-    const eventManager = new AuthEventManager();
+    const eventManager = new AuthEventManager(auth.name);
     iframe.register<GapiAuthEvent>(
       'authEvent',
       async (message: GapiAuthEvent) => {
@@ -90,6 +96,7 @@ type WidgetParams = {
   scopes?: string;
   customParameters?: string;
   eventId?: string;
+  tid?: string;
 };
 
 function getRedirectUrl(
@@ -107,7 +114,8 @@ function getRedirectUrl(
     authType,
     redirectUrl: _getCurrentUrl(),
     v: SDK_VERSION,
-    eventId
+    eventId,
+    tid: auth.tenantId || undefined,
   };
 
   if (provider instanceof OAuthProvider) {