Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Aug 9, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
lsirac marked this conversation as resolved.
Show resolved Hide resolved
* 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
lsirac marked this conversation as resolved.
Show resolved Hide resolved
* 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)) {
validateRevokedOrDisabled(firebaseToken);
return firebaseToken;
}

private void validateRevokedOrDisabled(FirebaseToken firebaseToken) throws FirebaseAuthException {
UserRecord user = userManager.getUserById(firebaseToken.getUid());
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;
if (user.isDisabled()) {
lsirac marked this conversation as resolved.
Show resolved Hide resolved
throw new FirebaseAuthException(ErrorCode.INVALID_ARGUMENT,
"The user record is disabled.",
/* cause= */ null,
/* response= */ null,
AuthErrorCode.USER_DISABLED);
}
}

static RevocationCheckDecorator decorateIdTokenVerifier(
Expand Down
84 changes: 84 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,45 @@ public void testVerifyIdToken() throws Exception {
auth.deleteUserAsync("user2");
}

@Test
public void testVerifyIdTokenUserDisabled() throws Exception {
String customToken = auth.createCustomToken("disabledUser");
lsirac marked this conversation as resolved.
Show resolved Hide resolved
String idToken = signInWithCustomToken(customToken);

// User is not disabled, this should not throw an exception.
FirebaseToken decoded = auth.verifyIdToken(idToken, /* checkRevoked= */true);
assertEquals("disabledUser", decoded.getUid());

// Disable the user record.
auth.updateUser(new UserRecord.UpdateRequest("disabledUser").setDisabled(true));

// Verify the ID token without checking revocation. This should not throw an exception.
decoded = auth.verifyIdToken(idToken);
assertEquals("disabledUser", 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 revocation error should take precedence over
// USER_DISABLED.
auth.revokeRefreshTokens("disabledUser");
try {
auth.verifyIdToken(idToken, /* checkRevoked= */ true);
fail("Should throw an exception as the ID tokens are revoked.");
} catch (FirebaseAuthException e) {
assertEquals(AuthErrorCode.REVOKED_ID_TOKEN, e.getAuthErrorCode());
}

auth.deleteUser("disabledUser");
lsirac marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
public void testVerifySessionCookie() throws Exception {
String customToken = auth.createCustomTokenAsync("user3").get();
Expand Down Expand Up @@ -661,6 +700,51 @@ public void testVerifySessionCookie() throws Exception {
auth.deleteUserAsync("user3");
}

@Test
public void testVerifySessionCookieUserDisabled() throws Exception {
String customToken = auth.createCustomToken("disabledUser2");
String idToken = signInWithCustomToken(customToken);

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("disabledUser2", decoded.getUid());

// Disable the user record.
auth.updateUser(new UserRecord.UpdateRequest("disabledUser2").setDisabled(true));

// Verify the session cookie without checking revocation. This should not throw an exception.
decoded = auth.verifySessionCookie(sessionCookie);
assertEquals("disabledUser2", 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 revocation error should take precedence over
// USER_DISABLED.
auth.revokeRefreshTokens("disabledUser2");
try {
auth.verifySessionCookie(sessionCookie, /* checkRevoked= */ true);
fail("Should throw an exception as the tokens are revoked.");
} catch (FirebaseAuthException e) {
assertEquals(AuthErrorCode.REVOKED_SESSION_COOKIE, e.getAuthErrorCode());
}

auth.deleteUser("disabledUser2");
lsirac marked this conversation as resolved.
Show resolved Hide resolved
}

@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,8 +25,11 @@
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.client.util.ArrayMap;
import com.google.api.core.ApiFuture;
import com.google.common.base.Supplier;
import com.google.common.base.Suppliers;
Expand All @@ -36,10 +39,14 @@
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.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);
ArrayMap<String, Object> users =
lsirac marked this conversation as resolved.
Show resolved Hide resolved
((ArrayList<ArrayMap<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