resolve 2 calls on getUser(), improve tests and lints

src/auth/auth.ts

@@ -124,21 +124,7 @@ export class BaseAuth<T extends AbstractAuthRequestHandler> implements BaseAuthI
       .then((decodedIdToken: DecodedIdToken) => {
         // Whether to check if the token was revoked.
         if (checkRevoked || isEmulator) {
-          // Whether user has been disabled.
-          if (checkRevoked) {
-            const uid = decodedIdToken.uid;
-            return this.getUser(uid)
-              .then((userInfo: UserRecord) => {
-                if (userInfo.disabled) {
-                  throw new FirebaseAuthError(
-                    AuthClientErrorCode.USER_DISABLED, 'The user account has been disabled by an administrator');
-                } 
-                return this.verifyDecodedJWTNotRevoked(
-                  decodedIdToken,
-                  AuthClientErrorCode.ID_TOKEN_REVOKED);
-              });
-          }
-          return this.verifyDecodedJWTNotRevoked(
+          return this.verifyDecodedJWTNotRevokedOrDisabled(
             decodedIdToken,
             AuthClientErrorCode.ID_TOKEN_REVOKED);
         }
@@ -524,11 +510,11 @@ export class BaseAuth<T extends AbstractAuthRequestHandler> implements BaseAuthI
 
   /**
    * Verifies a Firebase session cookie. Returns a Promise with the tokens claims. Rejects
-   * the promise if the token could not be verified. 
+   * the promise if the cookie could not be verified. 
    * If checkRevoked is set to true, first verifies whether the corresponding userinfo.disabled
    * is true:
    * If yes, an auth/user-disabled error is thrown.
-   * If no, verifies if the session corresponding to the ID token was revoked.
+   * If no, verifies if the session corresponding to the session cookie was revoked.
    * If the corresponding user's session was invalidated, an auth/session-cookie-revoked error
    * is thrown.
    * If not specified the check is not performed.
@@ -543,23 +529,9 @@ export class BaseAuth<T extends AbstractAuthRequestHandler> implements BaseAuthI
     const isEmulator = useEmulator();
     return this.sessionCookieVerifier.verifyJWT(sessionCookie, isEmulator)
       .then((decodedIdToken: DecodedIdToken) => {
-        // Whether to check if the token was revoked.
-        if (checkRevoked || isEmulator) {
-          // Whether user has been disabled.
-          if (checkRevoked) {
-            const uid = decodedIdToken.uid;
-            return this.getUser(uid)
-              .then((userInfo: UserRecord) => {
-                if (userInfo.disabled) {
-                  throw new FirebaseAuthError(
-                    AuthClientErrorCode.USER_DISABLED, 'The user account has been disabled by an administrator');
-                } 
-                return this.verifyDecodedJWTNotRevoked(
-                  decodedIdToken,
-                  AuthClientErrorCode.SESSION_COOKIE_REVOKED);
-              });
-          }          
-          return this.verifyDecodedJWTNotRevoked(
+        // Whether to check if the cookie was revoked.
+        if (checkRevoked || isEmulator) {         
+          return this.verifyDecodedJWTNotRevokedOrDisabled(
             decodedIdToken,
             AuthClientErrorCode.SESSION_COOKIE_REVOKED);
         }
@@ -758,20 +730,27 @@ export class BaseAuth<T extends AbstractAuthRequestHandler> implements BaseAuthI
   }
 
   /**
-   * Verifies the decoded Firebase issued JWT is not revoked. Returns a promise that resolves
-   * with the decoded claims on success. Rejects the promise with revocation error if revoked.
+   * Verifies the decoded Firebase issued JWT is not revoked or disabled. Returns a promise that
+   * resolves with the decoded claims on success. Rejects the promise with revocation error if revoked
+   * or user disabled.
    *
    * @param {DecodedIdToken} decodedIdToken The JWT's decoded claims.
    * @param {ErrorInfo} revocationErrorInfo The revocation error info to throw on revocation
    *     detection.
    * @return {Promise<DecodedIdToken>} A Promise that will be fulfilled after a successful
    *     verification.
    */
-  private verifyDecodedJWTNotRevoked(
+  private verifyDecodedJWTNotRevokedOrDisabled(
     decodedIdToken: DecodedIdToken, revocationErrorInfo: ErrorInfo): Promise<DecodedIdToken> {
     // Get tokens valid after time for the corresponding user.
     return this.getUser(decodedIdToken.sub)
       .then((user: UserRecord) => {
+        // If user disabled, throw an auth/user-disabled error.
+        if (user.disabled) {
+          throw new FirebaseAuthError(
+            AuthClientErrorCode.USER_DISABLED,
+            'The user account has been disabled by an administrator');
+        }
         // If no tokens valid after time available, token is not revoked.
         if (user.tokensValidAfterTime) {
           // Get the ID token authentication time and convert to milliseconds UTC.

src/utils/error.ts

@@ -723,7 +723,7 @@ export class AuthClientErrorCode {
   };
   public static USER_DISABLED = {
     code: 'user-disabled',
-    message: 'The user account has been disabled by an administrator.',
+    message: 'The user record is disabled.',
   }
   public static USER_NOT_DISABLED = {
     code: 'user-not-disabled',

test/integration/auth.spec.ts

@@ -760,6 +760,20 @@ describe('admin.auth', () => {
         safeDelete(userRecord.uid);
       }
     });
+
+    it('A user with user record disabled is unable to sign in', async () => {
+      const password = 'password';
+      const email = '[email protected]';
+      return admin.auth().updateUser(updateUser.uid, { disabled : true , password, email })
+        .then(() => {
+          return clientAuth().signInWithEmailAndPassword(email, password);
+        })
+        .then(() => {
+          throw new Error('Unexpected success');
+        }, (error) => {
+          expect(error).to.have.property('code', 'auth/user-disabled');
+        });
+    });
   });
 
   it('getUser() fails when called with a non-existing UID', () => {
@@ -865,37 +879,37 @@ describe('admin.auth', () => {
 
     it('verifyIdToken() fails with checkRevoked set to true and corresponding user disabled', () => {
       let currentIdToken: string;
+      let currentUser: User;
       return clientAuth().signInWithEmailAndPassword(email, password)
-        .then((result) => {
-          expect(result.user).to.exist;
-          expect(result.user!.email).to.equal(email);
-          return result.user!.getIdToken()
+        .then(({ user }) => {
+          expect(user).to.exist;
+          expect(user!.email).to.equal(email);
+          currentUser = user!;
+          return user!.getIdToken();
         })
         .then((idToken) => {
           currentIdToken = idToken;
-          return admin.auth().verifyIdToken(currentIdToken, true)
+          return admin.auth().verifyIdToken(currentIdToken, true);
         })
         .then((decodedIdToken) => {
           expect(decodedIdToken.uid).to.equal(uid);
           expect(decodedIdToken.email).to.equal(email);
-          return admin.auth().updateUser(uid, { disabled: true })
+          return admin.auth().updateUser(uid, { disabled: true });
         })
         .then((userRecord) => {
           // Ensure disabled field has been updated.
           expect(userRecord.uid).to.equal(uid);
           expect(userRecord.email).to.equal(email);
           expect(userRecord.disabled).to.equal(true);
-          return clientAuth().signInWithEmailAndPassword(email, password)
+          return admin.auth().verifyIdToken(currentIdToken, false);
         })
-        .then((result) => {
-          expect(result.user).to.exist;
-          expect(result.user!.email).to.equal(email);
-          return result.user!.getIdToken()
+        .then(() => {
+          // Verification should be successful.
+          // Need to reload user to ensure currentUser been updated.
+          return currentUser.reload();
         })
-        .then((idToken) => {
-          // Ensure idToken is the same before verifying.
-          expect(currentIdToken).to.equal(idToken);
-          return admin.auth().verifyIdToken(idToken, true)
+        .then(() => {
+          return admin.auth().verifyIdToken(currentIdToken, true);
         })
         .then(() => {
           throw new Error('Unexpected success');
@@ -2053,15 +2067,15 @@ describe('admin.auth', () => {
     it('fails with checkRevoked set to true and corresponding user disabled', () => {
       const expiresIn = 24 * 60 * 60 * 1000;
       let currentIdToken: string;
-      let currentCustomToken: string;
       let currentSessioncookie: string;
+      let currentUser: User;
       return admin.auth().createCustomToken(uid, { admin: true, groupId: '1234' })
         .then((customToken) => {
-          currentCustomToken = customToken;
-          return clientAuth().signInWithCustomToken(customToken)
+          return clientAuth().signInWithCustomToken(customToken);
         })
         .then(({ user }) => {
           expect(user).to.exist;
+          currentUser = user!;
           return user!.getIdToken();
         })
         .then((idToken) => {
@@ -2074,7 +2088,7 @@ describe('admin.auth', () => {
         })
         .then((sessionCookie) => {
           currentSessioncookie = sessionCookie;
-          admin.auth().verifySessionCookie(sessionCookie, true)
+          return admin.auth().verifySessionCookie(sessionCookie, true);
         })
         .then(() => {
           return admin.auth().updateUser(uid, { disabled : true });
@@ -2083,9 +2097,13 @@ describe('admin.auth', () => {
           // Ensure disabled field has been updated.
           expect(userRecord.uid).to.equal(uid);
           expect(userRecord.disabled).to.equal(true);
-          return clientAuth().signInWithCustomToken(currentCustomToken)
+          return admin.auth().verifySessionCookie(currentSessioncookie, false);
+        })
+        .then(() => {
+          // Need to reload user to ensure currentUser been updated.
+          return currentUser.reload();
         }).then(() => {
-          admin.auth().verifySessionCookie(currentSessioncookie, true)
+          admin.auth().verifySessionCookie(currentSessioncookie, true);
         })
         .then(() => {
           throw new Error('Unexpected success');

test/unit/auth/auth.spec.ts

@@ -547,7 +547,7 @@ AUTH_CONFIGS.forEach((testConfig) => {
         return auth.verifyIdToken(mockIdToken, true)
           .then((result) => {
             // Confirm underlying API called with expected parameters.
-            expect(getUserStub).to.have.been.calledTwice.and.calledWith(uid);
+            expect(getUserStub).to.have.been.calledOnce.and.calledWith(uid);
             expect(result).to.deep.equal(decodedIdToken);
           });
       });
@@ -570,7 +570,7 @@ AUTH_CONFIGS.forEach((testConfig) => {
             throw new Error('Unexpected success');
           }, (error) => {
             // Confirm underlying API called with expected parameters.
-            expect(getUserStub).to.have.been.calledTwice.and.calledWith(uid);
+            expect(getUserStub).to.have.been.calledOnce.and.calledWith(uid);
             // Confirm expected error returned.
             expect(error).to.have.property('code', 'auth/id-token-revoked');
           });
@@ -629,7 +629,7 @@ AUTH_CONFIGS.forEach((testConfig) => {
         return auth.verifyIdToken(mockIdToken, true)
           .then((result) => {
             // Confirm underlying API called with expected parameters.
-            expect(getUserStub).to.have.been.calledTwice.and.calledWith(uid);
+            expect(getUserStub).to.have.been.calledOnce.and.calledWith(uid);
             expect(result).to.deep.equal(decodedIdToken);
           });
       });
@@ -791,7 +791,7 @@ AUTH_CONFIGS.forEach((testConfig) => {
         return auth.verifySessionCookie(mockSessionCookie, true)
           .then((result) => {
             // Confirm underlying API called with expected parameters.
-            expect(getUserStub).to.have.been.calledTwice.and.calledWith(uid);
+            expect(getUserStub).to.have.been.calledOnce.and.calledWith(uid);
             expect(result).to.deep.equal(decodedSessionCookie);
           });
       });
@@ -814,7 +814,7 @@ AUTH_CONFIGS.forEach((testConfig) => {
             throw new Error('Unexpected success');
           }, (error) => {
             // Confirm underlying API called with expected parameters.
-            expect(getUserStub).to.have.been.calledTwice.and.calledWith(uid);
+            expect(getUserStub).to.have.been.calledOnce.and.calledWith(uid);
             // Confirm expected error returned.
             expect(error).to.have.property('code', 'auth/session-cookie-revoked');
           });
@@ -892,7 +892,7 @@ AUTH_CONFIGS.forEach((testConfig) => {
         return auth.verifySessionCookie(mockSessionCookie, true)
           .then((result) => {
             // Confirm underlying API called with expected parameters.
-            expect(getUserStub).to.have.been.calledTwice.and.calledWith(uid);
+            expect(getUserStub).to.have.been.calledOnce.and.calledWith(uid);
             expect(result).to.deep.equal(decodedSessionCookie);
           });
       });