fix(auth): check if the user is disabled on checkRevoked=true for verifyIdToken and verifySessionCookie (#585)

* fix(auth): check if the user is disabled on checkRevoked=true for verifyIdToken and verifySessionCookie

* fix: review comments

Author: lsirac

src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java

@@ -256,11 +256,13 @@ public FirebaseToken verifyIdToken(@NonNull String idToken) throws FirebaseAuthE
    * API call.
    *
    * @param idToken A Firebase ID token string to parse and verify.
-   * @param checkRevoked A boolean denoting whether to check if the tokens were revoked.
+   * @param checkRevoked A boolean denoting whether to check if the tokens were revoked or if
+   *                     the user is disabled.
    * @return A {@link FirebaseToken} representing the verified and decoded token.
    * @throws IllegalArgumentException If the token is null, empty, or if the {@link FirebaseApp}
    *     instance does not have a project ID associated with it.
-   * @throws FirebaseAuthException If an error occurs while parsing or validating the token.
+   * @throws FirebaseAuthException If an error occurs while parsing or validating the token, or if
+   *     the user is disabled.
    */
   public FirebaseToken verifyIdToken(@NonNull String idToken, boolean checkRevoked)
       throws FirebaseAuthException {
@@ -343,8 +345,11 @@ public FirebaseToken verifySessionCookie(String cookie) throws FirebaseAuthExcep
    * checkRevoked} is true, throws a {@link FirebaseAuthException}.
    *
    * @param cookie A Firebase session cookie string to verify and parse.
-   * @param checkRevoked A boolean indicating whether to check if the cookie was explicitly revoked.
+   * @param checkRevoked A boolean indicating whether to check if the cookie was explicitly revoked
+   *                    or if the user is disabled.
    * @return A {@link FirebaseToken} representing the verified and decoded cookie.
+   * @throws FirebaseAuthException If an error occurs while parsing or validating the token, or if
+   *     the user is disabled.
    */
   public FirebaseToken verifySessionCookie(String cookie, boolean checkRevoked)
       throws FirebaseAuthException {

src/main/java/com/google/firebase/auth/AuthErrorCode.java

@@ -108,4 +108,9 @@ public enum AuthErrorCode {
    * No user record found for the given identifier.
    */
   USER_NOT_FOUND,
+
+  /**
+   * The user record is disabled.
+   */
+  USER_DISABLED,
 }

src/main/java/com/google/firebase/auth/RevocationCheckDecorator.java

@@ -51,22 +51,28 @@ private RevocationCheckDecorator(
   @Override
   public FirebaseToken verifyToken(String token) throws FirebaseAuthException {
     FirebaseToken firebaseToken = tokenVerifier.verifyToken(token);
-    if (isRevoked(firebaseToken)) {
+    validateDisabledOrRevoked(firebaseToken);
+    return firebaseToken;
+  }
+
+  private void validateDisabledOrRevoked(FirebaseToken firebaseToken) throws FirebaseAuthException {
+    UserRecord user = userManager.getUserById(firebaseToken.getUid());
+    if (user.isDisabled()) {
+      throw new FirebaseAuthException(ErrorCode.INVALID_ARGUMENT,
+          "The user record is disabled.",
+          /* cause= */ null,
+          /* response= */ null,
+          AuthErrorCode.USER_DISABLED);
+    }
+    long issuedAtInSeconds = (long) firebaseToken.getClaims().get("iat");
+    if (user.getTokensValidAfterTimestamp() > issuedAtInSeconds * 1000) {
       throw new FirebaseAuthException(
           ErrorCode.INVALID_ARGUMENT,
           "Firebase " + shortName + " is revoked.",
           null,
           null,
           errorCode);
     }
-
-    return firebaseToken;
-  }
-
-  private boolean isRevoked(FirebaseToken firebaseToken) throws FirebaseAuthException {
-    UserRecord user = userManager.getUserById(firebaseToken.getUid());
-    long issuedAtInSeconds = (long) firebaseToken.getClaims().get("iat");
-    return user.getTokensValidAfterTimestamp() > issuedAtInSeconds * 1000;
   }
 
   static RevocationCheckDecorator decorateIdTokenVerifier(

src/test/java/com/google/firebase/auth/FirebaseAuthIT.java

@@ -625,6 +625,46 @@ public void testVerifyIdToken() throws Exception {
     auth.deleteUserAsync("user2");
   }
 
+  @Test
+  public void testVerifyIdTokenUserDisabled() throws Exception {
+    RandomUser user = UserTestUtils.generateRandomUserInfo();
+    String customToken = auth.createCustomToken(user.getUid());
+    String idToken = signInWithCustomToken(customToken);
+
+    temporaryUser.registerUid(user.getUid());
+
+    // User is not disabled, this should not throw an exception.
+    FirebaseToken decoded = auth.verifyIdToken(idToken, /* checkRevoked= */true);
+    assertEquals(user.getUid(), decoded.getUid());
+
+    // Disable the user record.
+    auth.updateUser(new UserRecord.UpdateRequest(user.getUid()).setDisabled(true));
+
+    // Verify the ID token without checking revocation. This should not throw an exception.
+    decoded = auth.verifyIdToken(idToken);
+    assertEquals(user.getUid(), decoded.getUid());
+
+    // Verify the ID token while checking revocation. This should throw an exception.
+    try {
+      auth.verifyIdToken(idToken, /* checkRevoked= */true);
+      fail("Should throw a FirebaseAuthException since the user is disabled.");
+    } catch (FirebaseAuthException e) {
+      assertEquals(ErrorCode.INVALID_ARGUMENT, e.getErrorCode());
+      assertEquals(AuthErrorCode.USER_DISABLED, e.getAuthErrorCode());
+      assertEquals("The user record is disabled.", e.getMessage());
+    }
+
+    // Revoke the tokens for the user. The USER_DISABLED should take precedence over
+    // the revocation error.
+    auth.revokeRefreshTokens(user.getUid());
+    try {
+      auth.verifyIdToken(idToken, /* checkRevoked= */ true);
+      fail("Should throw an exception as the ID tokens are revoked.");
+    } catch (FirebaseAuthException e) {
+      assertEquals(AuthErrorCode.USER_DISABLED, e.getAuthErrorCode());
+    }
+  }
+
   @Test
   public void testVerifySessionCookie() throws Exception {
     String customToken = auth.createCustomTokenAsync("user3").get();
@@ -661,6 +701,52 @@ public void testVerifySessionCookie() throws Exception {
     auth.deleteUserAsync("user3");
   }
 
+  @Test
+  public void testVerifySessionCookieUserDisabled() throws Exception {
+    RandomUser user = UserTestUtils.generateRandomUserInfo();
+    String customToken = auth.createCustomToken(user.getUid());
+    String idToken = signInWithCustomToken(customToken);
+
+    temporaryUser.registerUid(user.getUid());
+
+    SessionCookieOptions options = SessionCookieOptions.builder()
+        .setExpiresIn(TimeUnit.HOURS.toMillis(1))
+        .build();
+    String sessionCookie = auth.createSessionCookieAsync(idToken, options).get();
+    assertFalse(Strings.isNullOrEmpty(sessionCookie));
+
+    // User is not disabled, this should not throw an exception.
+    FirebaseToken decoded = auth.verifySessionCookie(sessionCookie, /* checkRevoked= */true);
+    assertEquals(user.getUid(), decoded.getUid());
+
+    // Disable the user record.
+    auth.updateUser(new UserRecord.UpdateRequest(user.getUid()).setDisabled(true));
+
+    // Verify the session cookie without checking revocation. This should not throw an exception.
+    decoded = auth.verifySessionCookie(sessionCookie);
+    assertEquals(user.getUid(), decoded.getUid());
+
+    // Verify the session cookie while checking revocation. This should throw an exception.
+    try {
+      auth.verifySessionCookie(sessionCookie, /* checkRevoked= */true);
+      fail("Should throw a FirebaseAuthException since the user is disabled.");
+    } catch (FirebaseAuthException e) {
+      assertEquals(ErrorCode.INVALID_ARGUMENT, e.getErrorCode());
+      assertEquals(AuthErrorCode.USER_DISABLED, e.getAuthErrorCode());
+      assertEquals("The user record is disabled.", e.getMessage());
+    }
+
+    // Revoke the tokens for the user. The USER_DISABLED should take precedence over
+    // the revocation error.
+    auth.revokeRefreshTokens(user.getUid());
+    try {
+      auth.verifySessionCookie(sessionCookie, /* checkRevoked= */ true);
+      fail("Should throw an exception as the tokens are revoked.");
+    } catch (FirebaseAuthException e) {
+      assertEquals(AuthErrorCode.USER_DISABLED, e.getAuthErrorCode());
+    }
+  }
+
   @Test
   public void testCustomTokenWithClaims() throws Exception {
     Map<String, Object> devClaims = ImmutableMap.<String, Object>of(

src/test/java/com/google/firebase/auth/FirebaseAuthTest.java

@@ -25,6 +25,8 @@
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import com.google.api.client.json.GenericJson;
+import com.google.api.client.json.JsonParser;
 import com.google.api.client.testing.http.MockHttpTransport;
 import com.google.api.client.testing.http.MockLowLevelHttpResponse;
 import com.google.api.core.ApiFuture;
@@ -36,10 +38,15 @@
 import com.google.firebase.FirebaseApp;
 import com.google.firebase.FirebaseOptions;
 import com.google.firebase.TestOnlyImplFirebaseTrampolines;
+import com.google.firebase.internal.ApiClientUtils;
 import com.google.firebase.internal.FirebaseProcessEnvironment;
 import com.google.firebase.testing.ServiceAccount;
 import com.google.firebase.testing.TestResponseInterceptor;
 import com.google.firebase.testing.TestUtils;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Map;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -246,6 +253,22 @@ public void testVerifyIdTokenWithRevocationCheck() throws Exception {
     assertEquals("idtoken", tokenVerifier.getLastTokenString());
   }
 
+  @Test
+  public void testVerifyIdTokenWithRevocationCheckAndUserDisabled() throws Exception {
+    MockTokenVerifier tokenVerifier = MockTokenVerifier.fromResult(
+        getFirebaseToken(VALID_SINCE + 1000));
+    FirebaseAuth auth =
+        getAuthForIdTokenVerificationWithRevocationCheckWithDisabledUser(tokenVerifier);
+    try {
+      auth.verifyIdToken("idtoken", true);
+      fail("No exception thrown for disabled user.");
+    } catch (FirebaseAuthException e) {
+      assertEquals(ErrorCode.INVALID_ARGUMENT, e.getErrorCode());
+      assertEquals(AuthErrorCode.USER_DISABLED, e.getAuthErrorCode());
+      assertEquals("The user record is disabled.", e.getMessage());
+    }
+  }
+
   @Test
   public void testVerifyIdTokenWithRevocationCheckFailure() {
     MockTokenVerifier tokenVerifier = MockTokenVerifier.fromResult(
@@ -444,6 +467,22 @@ public void testVerifySessionCookieWithRevocationCheck() throws Exception {
     assertEquals("cookie", tokenVerifier.getLastTokenString());
   }
 
+  @Test
+  public void testVerifySessionCookieWithRevocationCheckAndUserDisabled() throws Exception {
+    MockTokenVerifier tokenVerifier = MockTokenVerifier.fromResult(
+        getFirebaseToken(VALID_SINCE + 1000));
+    FirebaseAuth auth =
+        getAuthForSessionCookieVerificationWithRevocationCheckAndUserDisabled(tokenVerifier);
+    try {
+      auth.verifySessionCookie("cookie", true);
+      fail("No exception thrown for disabled user.");
+    } catch (FirebaseAuthException e) {
+      assertEquals(ErrorCode.INVALID_ARGUMENT, e.getErrorCode());
+      assertEquals(AuthErrorCode.USER_DISABLED, e.getAuthErrorCode());
+      assertEquals("The user record is disabled.", e.getMessage());
+    }
+  }
+
   @Test
   public void testVerifySessionCookieWithRevocationCheckFailure() {
     MockTokenVerifier tokenVerifier = MockTokenVerifier.fromResult(
@@ -513,6 +552,12 @@ FirebaseAuth getAuthForIdTokenVerificationWithRevocationCheck(
     return getAuthForIdTokenVerification(app, Suppliers.ofInstance(tokenVerifier));
   }
 
+  FirebaseAuth getAuthForIdTokenVerificationWithRevocationCheckWithDisabledUser(
+      FirebaseTokenVerifier tokenVerifier) throws IOException {
+    FirebaseApp app = getFirebaseAppForDisabledUserRetrieval();
+    return getAuthForIdTokenVerification(app, Suppliers.ofInstance(tokenVerifier));
+  }
+
   private FirebaseAuth getAuthForIdTokenVerification(FirebaseTokenVerifier tokenVerifier) {
     return getAuthForIdTokenVerification(Suppliers.ofInstance(tokenVerifier));
   }
@@ -540,6 +585,12 @@ FirebaseAuth getAuthForSessionCookieVerificationWithRevocationCheck(
     return getAuthForSessionCookieVerification(app, Suppliers.ofInstance(tokenVerifier));
   }
 
+  FirebaseAuth getAuthForSessionCookieVerificationWithRevocationCheckAndUserDisabled(
+      FirebaseTokenVerifier tokenVerifier) throws IOException {
+    FirebaseApp app = getFirebaseAppForDisabledUserRetrieval();
+    return getAuthForSessionCookieVerification(app, Suppliers.ofInstance(tokenVerifier));
+  }
+
   private FirebaseAuth getAuthForSessionCookieVerification(FirebaseTokenVerifier tokenVerifier) {
     return getAuthForSessionCookieVerification(Suppliers.ofInstance(tokenVerifier));
   }
@@ -573,6 +624,25 @@ private FirebaseApp getFirebaseAppForUserRetrieval() {
         .build());
   }
 
+  private FirebaseApp getFirebaseAppForDisabledUserRetrieval() throws IOException {
+    String getUserResponse = TestUtils.loadResource("getUser.json");
+    JsonParser parser = ApiClientUtils.getDefaultJsonFactory().createJsonParser(getUserResponse);
+    GenericJson json =
+        parser.parseAndClose(GenericJson.class);
+    Map<String, Object> users =
+        ((ArrayList<Map<String, Object>>) json.get("users")).get(0);
+    users.put("disabled", true);
+
+    MockHttpTransport transport = new MockHttpTransport.Builder()
+        .setLowLevelHttpResponse(new MockLowLevelHttpResponse().setContent(json.toString()))
+        .build();
+    return FirebaseApp.initializeApp(FirebaseOptions.builder()
+        .setCredentials(new MockGoogleCredentials("test-token"))
+        .setHttpTransport(transport)
+        .setProjectId("test-project-id")
+        .build());
+  }
+
   public static TestResponseInterceptor setUserManager(
       AbstractFirebaseAuth.Builder<?> builder, FirebaseApp app, String tenantId) {
     TestResponseInterceptor interceptor = new TestResponseInterceptor();