PR feedback and more cleanup

Author: avolkovi

packages-exp/auth-exp/src/core/credentials/auth_credential.ts

@@ -0,0 +1,58 @@
+/**
+ * @license
+ * Copyright 2020 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { PhoneOrOauthTokenResponse } from '../../api/authentication/mfa';
+import { AuthCore } from '../../model/auth';
+import { IdTokenResponse } from '../../model/id_token';
+import { EmailAuthCredential, OAuthCredential, PhoneAuthCredential } from './';
+import { debugFail } from '../util/assert';
+
+export class AuthCredential {
+  protected constructor(
+    readonly providerId: string,
+    readonly signInMethod: string
+  ) {}
+
+  static fromJSON(json: object | string): AuthCredential | null {
+    for (const fn of [
+      /* SAMLAuthCredential.fromJSON, */
+      OAuthCredential.fromJSON,
+      EmailAuthCredential.fromJSON,
+      PhoneAuthCredential.fromJSON
+    ]) {
+      const credential = fn(json);
+      if (credential) {
+        return credential;
+      }
+    }
+    return null;
+  }
+
+  toJSON(): object {
+    return debugFail('not implemented');
+  }
+
+  _getIdTokenResponse(_auth: AuthCore): Promise<PhoneOrOauthTokenResponse> {
+    return debugFail('not implemented');
+  }
+  _linkToIdToken(_auth: AuthCore, _idToken: string): Promise<IdTokenResponse> {
+    return debugFail('not implemented');
+  }
+  _getReauthenticationResolver(_auth: AuthCore): Promise<IdTokenResponse> {
+    return debugFail('not implemented');
+  }
+}

packages-exp/auth-exp/src/core/credentials/index.ts

@@ -15,46 +15,7 @@
  * limitations under the License.
  */
 
-import { PhoneOrOauthTokenResponse } from '../../api/authentication/mfa';
-import { AuthCore } from '../../model/auth';
-import { IdTokenResponse } from '../../model/id_token';
-import { EmailAuthCredential } from './email';
-import { PhoneAuthCredential } from './phone';
-import { OAuthCredential } from './oauth';
-import { debugFail } from '../util/assert';
-
-export abstract class AuthCredential {
-  protected constructor(
-    readonly providerId: string,
-    readonly signInMethod: string
-  ) {}
-
-  static fromJSON(json: object | string): AuthCredential | null {
-    for (const fn of [
-      /* SAMLAuthCredential.fromJSON, */
-      OAuthCredential.fromJSON,
-      EmailAuthCredential.fromJSON,
-      PhoneAuthCredential.fromJSON
-    ]) {
-      const credential = fn(json);
-      if (credential) {
-        return credential;
-      }
-    }
-    return null;
-  }
-
-  toJSON(): object {
-    return debugFail('not implemented');
-  }
-
-  _getIdTokenResponse(_auth: AuthCore): Promise<PhoneOrOauthTokenResponse> {
-    return debugFail('not implemented');
-  }
-  _linkToIdToken(_auth: AuthCore, _idToken: string): Promise<IdTokenResponse> {
-    return debugFail('not implemented');
-  }
-  _getReauthenticationResolver(_auth: AuthCore): Promise<IdTokenResponse> {
-    return debugFail('not implemented');
-  }
-}
+export { AuthCredential } from './auth_credential';
+export { EmailAuthCredential } from './email';
+export { OAuthCredential } from './oauth';
+export { PhoneAuthCredential } from './phone';

packages-exp/auth-exp/src/core/persistence/persistence_user_manager.test.ts

@@ -134,7 +134,7 @@ describe('core/persistence/persistence_user_manager', () => {
 
     it('#getCurrentUser calls with instantiator', async () => {
       const rawObject = {};
-      const userImplStub = sinon.stub(UserImpl, 'fromPlainObject');
+      const userImplStub = sinon.stub(UserImpl, '_fromJSON');
       persistenceStub.get.returns(Promise.resolve(rawObject));
 
       await manager.getCurrentUser();

packages-exp/auth-exp/src/core/persistence/persistence_user_manager.ts

@@ -58,7 +58,7 @@ export class PersistenceUserManager {
 
   async getCurrentUser(): Promise<User | null> {
     const blob = await this.persistence.get<PersistedBlob>(this.fullUserKey);
-    return blob ? UserImpl.fromPlainObject(this.auth, blob) : null;
+    return blob ? UserImpl._fromJSON(this.auth, blob) : null;
   }
 
   removeCurrentUser(): Promise<void> {

packages-exp/auth-exp/src/core/user/id_token_result.ts

@@ -21,6 +21,7 @@ import { base64Decode } from '@firebase/util';
 import { User } from '../../model/user';
 import { assert } from '../util/assert';
 import { _logError } from '../util/log';
+import { utcTimestampToDateString } from '../util/time';
 
 export function getIdToken(
   user: externs.User,
@@ -67,19 +68,6 @@ function secondsStringToMilliseconds(seconds: string): number {
   return Number(seconds) * 1000;
 }
 
-function utcTimestampToDateString(timestamp: string | number): string {
-  try {
-    const date = new Date(Number(timestamp));
-    if (!isNaN(date.getTime())) {
-      return date.toUTCString();
-    }
-  } catch {
-    // Do nothing, return null
-  }
-
-  return ''; // TODO(avolkovi): is this the right fallback?
-}
-
 export function _parseToken(token: string): externs.ParsedToken | null {
   const [algorithm, payload, signature] = token.split('.');
   if (

packages-exp/auth-exp/src/core/user/reload.test.ts

@@ -31,6 +31,7 @@ import {
   ProviderUserInfo
 } from '../../api/account_management/account';
 import { _reloadWithoutSaving, reload } from './reload';
+import { UserMetadata } from './user_impl';
 
 use(chaiAsPromised);
 use(sinonChai);
@@ -89,9 +90,9 @@ describe('core/user/reload', () => {
     expect(user.emailVerified).to.be.true;
     expect(user.phoneNumber).to.eq('phone-number');
     expect(user.tenantId).to.eq('tenant-id');
-    expect(user.metadata).to.eql({
-      creationTime: '123',
-      lastSignInTime: '456'
+    expect((user.metadata as UserMetadata).toJSON()).to.eql({
+      createdAt: '123',
+      lastLoginAt: '456'
     });
   });
 

packages-exp/auth-exp/src/core/user/reload.ts

@@ -22,6 +22,7 @@ import {
   ProviderUserInfo
 } from '../../api/account_management/account';
 import { User } from '../../model/user';
+import { UserMetadata } from './user_impl';
 import { assert } from '../util/assert';
 
 export async function _reloadWithoutSaving(user: User): Promise<void> {
@@ -47,10 +48,10 @@ export async function _reloadWithoutSaving(user: User): Promise<void> {
     phoneNumber: coreAccount.phoneNumber || null,
     tenantId: coreAccount.tenantId || null,
     providerData: mergeProviderData(user.providerData, newProviderData),
-    metadata: {
-      creationTime: coreAccount.createdAt?.toString(),
-      lastSignInTime: coreAccount.lastLoginAt?.toString()
-    }
+    metadata: new UserMetadata(
+      coreAccount.createdAt?.toString(),
+      coreAccount.lastLoginAt?.toString()
+    )
   };
 
   Object.assign(user, updates);

packages-exp/auth-exp/src/core/user/token_manager.test.ts

@@ -170,7 +170,7 @@ describe('core/user/token_manager', () => {
     });
   });
 
-  describe('.fromPlainObject', () => {
+  describe('.fromJSON', () => {
     const errorString =
       'Firebase: An internal AuthError has occurred. (auth/internal-error).';
 

packages-exp/auth-exp/src/core/user/user_impl.test.ts

@@ -150,20 +150,20 @@ describe('core/user/user_impl', () => {
     });
   });
 
-  describe('.fromPlainObject', () => {
+  describe('.fromJSON', () => {
     const errorString =
       'Firebase: An internal AuthError has occurred. (auth/internal-error).';
 
     it('throws an error if uid is not present', () => {
-      expect(() => UserImpl.fromPlainObject(auth, { name: 'foo' })).to.throw(
+      expect(() => UserImpl._fromJSON(auth, { name: 'foo' })).to.throw(
         FirebaseError,
         errorString
       );
     });
 
     it('throws if a key is not undefined or string', () => {
       expect(() =>
-        UserImpl.fromPlainObject(auth, { uid: 'foo', displayName: 3 })
+        UserImpl._fromJSON(auth, { uid: 'foo', displayName: 3 })
       ).to.throw(FirebaseError, errorString);
     });
 
@@ -178,10 +178,12 @@ describe('core/user/user_impl', () => {
         displayName: 'name',
         email: 'email',
         phoneNumber: 'number',
-        photoURL: 'photo'
+        photoURL: 'photo',
+        emailVerified: false,
+        isAnonymous: false
       };
 
-      const user = UserImpl.fromPlainObject(auth, params);
+      const user = UserImpl._fromJSON(auth, params);
       expect(user.uid).to.eq(params.uid);
       expect(user.displayName).to.eq(params.displayName);
       expect(user.email).to.eq(params.email);