Skip to content

Commit

Permalink
fix(auth): check if the user is disabled on checkRevoked=true for ver…
Browse files Browse the repository at this point in the history
…ifyIdToken and verifySessionCookie (#585)

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

* fix: review comments
  • Loading branch information
lsirac authored Aug 9, 2021
1 parent b071b03 commit 2d50628
Show file tree
Hide file tree
Showing 5 changed files with 184 additions and 12 deletions.
11 changes: 8 additions & 3 deletions src/main/java/com/google/firebase/auth/AbstractFirebaseAuth.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/com/google/firebase/auth/AuthErrorCode.java
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
86 changes: 86 additions & 0 deletions src/test/java/com/google/firebase/auth/FirebaseAuthIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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(
Expand Down
70 changes: 70 additions & 0 deletions src/test/java/com/google/firebase/auth/FirebaseAuthTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 2d50628

Please sign in to comment.