Repository: firebase/firebase-admin-go
Pull request #455 (2021-08-12 00:45:11)

fix(auth): check disabled status on verifyIDToken
The `verifyIDToken()` function checks to see whether the ID token is revoked, but never actually checks to see whether or not the user is disabled.  This change makes it so that `verifyIDToken()` does check the `disabled` field by modifying `checkRevoked()`.  The function is renamed to `checkRevokedOrDisabled` and now additionally includes a check to make sure that the user is not disabled.  A new `authErrorCode` corresponding to this case is added.  `verifySessionCookie()` is slightly modified to account for the fact that `checkRevokedOrDisabled` now returns 3 values instead of 2.  `verifyIDToken()` has been modified to raise an `InvalidArgument` error with `authErrorCode` of "ID_TOKEN_DISABLED" if the user is disabled.

google-cla[bot]: We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors.  If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA ([login here](https://cla.developers.google.com/) to double check)?  If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment `@googlebot I fixed it.`. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Ffirebase%2Ffirebase-admin-go%2Fpull%2F455) for more info**.

<!-- need_author_cla -->
google-cla[bot]: We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors.  If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA ([login here](https://cla.developers.google.com/) to double check)?  If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment `@googlebot I fixed it.`. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Ffirebase%2Ffirebase-admin-go%2Fpull%2F455) for more info**.

<!-- need_author_cla -->
Galadros: I'm not sure how to fix the google-cla bot error.  It's complaining that "google-oss-bot" doesn't have a CLA signed.
hiranya911: You might want to rebase your commits at dev and do a force push, which should clean up the commit history.
google-cla[bot]: We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors.  If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA ([login here](https://cla.developers.google.com/) to double check)?  If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment `@googlebot I fixed it.`. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Ffirebase%2Ffirebase-admin-go%2Fpull%2F455) for more info**.

<!-- need_author_cla -->
google-cla[bot]: We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors.  If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA ([login here](https://cla.developers.google.com/) to double check)?  If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment `@googlebot I fixed it.`. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Ffirebase%2Ffirebase-admin-go%2Fpull%2F455) for more info**.

<!-- need_author_cla -->
google-cla[bot]: We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors.  If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA ([login here](https://cla.developers.google.com/) to double check)?  If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment `@googlebot I fixed it.`. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Ffirebase%2Ffirebase-admin-go%2Fpull%2F455) for more info**.

<!-- need_author_cla -->
google-cla[bot]: We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors.  If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA ([login here](https://cla.developers.google.com/) to double check)?  If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment `@googlebot I fixed it.`. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Ffirebase%2Ffirebase-admin-go%2Fpull%2F455) for more info**.

<!-- need_author_cla -->
google-cla[bot]: We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors.  If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA ([login here](https://cla.developers.google.com/) to double check)?  If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment `@googlebot I fixed it.`. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Ffirebase%2Ffirebase-admin-go%2Fpull%2F455) for more info**.

<!-- need_author_cla -->
google-cla[bot]: We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors.  If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA ([login here](https://cla.developers.google.com/) to double check)?  If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment `@googlebot I fixed it.`. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Ffirebase%2Ffirebase-admin-go%2Fpull%2F455) for more info**.

<!-- need_author_cla -->

Patch:
diff --git a/auth/auth.go b/auth/auth.go
index 17c5279..f035726 100644
--- a/auth/auth.go
+++ b/auth/auth.go
@@ -37,6 +37,7 @@ const (
 
 	// SDK-generated error codes
 	idTokenRevoked       = "ID_TOKEN_REVOKED"
+	userDisabled         = "USER_DISABLED"
 	sessionCookieRevoked = "SESSION_COOKIE_REVOKED"
 	tenantIDMismatch     = "TENANT_ID_MISMATCH"
 )
@@ -288,14 +289,14 @@ func (c *baseClient) withTenantID(tenantID string) *baseClient {
 // These keys get cached up to 24 hours, and therefore the RPC overhead gets amortized
 // over many invocations of this function.
 //
-// This does not check whether or not the token has been revoked. Use `VerifyIDTokenAndCheckRevoked()`
+// This does not check whether or not the token has been revoked or disabled. Use `VerifyIDTokenAndCheckRevoked()`
 // when a revocation check is needed.
 func (c *baseClient) VerifyIDToken(ctx context.Context, idToken string) (*Token, error) {
 	return c.verifyIDToken(ctx, idToken, false)
 }
 
 // VerifyIDTokenAndCheckRevoked verifies the provided ID token, and additionally checks that the
-// token has not been revoked.
+// token has not been revoked or disabled.
 //
 // Unlike `VerifyIDToken()`, this function must make an RPC call to perform the revocation check.
 // Developers are advised to take this additional overhead into consideration when including this
@@ -304,7 +305,7 @@ func (c *baseClient) VerifyIDTokenAndCheckRevoked(ctx context.Context, idToken s
 	return c.verifyIDToken(ctx, idToken, true)
 }
 
-func (c *baseClient) verifyIDToken(ctx context.Context, idToken string, checkRevoked bool) (*Token, error) {
+func (c *baseClient) verifyIDToken(ctx context.Context, idToken string, checkRevokedOrDisabled bool) (*Token, error) {
 	decoded, err := c.idTokenVerifier.VerifyToken(ctx, idToken, c.isEmulator)
 	if err != nil {
 		return nil, err
@@ -320,21 +321,11 @@ func (c *baseClient) verifyIDToken(ctx context.Context, idToken string, checkRev
 		}
 	}
 
-	if c.isEmulator || checkRevoked {
-		revoked, err := c.checkRevoked(ctx, decoded)
+	if c.isEmulator || checkRevokedOrDisabled {
+		err = c.checkRevokedOrDisabled(ctx, decoded, idTokenRevoked, "ID token has been revoked")
 		if err != nil {
 			return nil, err
 		}
-
-		if revoked {
-			return nil, &internal.FirebaseError{
-				ErrorCode: internal.InvalidArgument,
-				String:    "ID token has been revoked",
-				Ext: map[string]interface{}{
-					authErrorCode: idTokenRevoked,
-				},
-			}
-		}
 	}
 
 	return decoded, nil
@@ -347,11 +338,18 @@ func IsTenantIDMismatch(err error) bool {
 
 // IsIDTokenRevoked checks if the given error was due to a revoked ID token.
 //
-// When IsIDTokenRevoked returns true, IsIDTokenInvalid is guranteed to return true.
+// When IsIDTokenRevoked returns true, IsIDTokenInvalid is guaranteed to return true.
 func IsIDTokenRevoked(err error) bool {
 	return hasAuthErrorCode(err, idTokenRevoked)
 }
 
+// IsUserDisabled checks if the given error was due to a disabled ID token
+//
+// When IsUserDisabled returns true, IsIDTokenInvalid is guaranteed to return true.
+func IsUserDisabled(err error) bool {
+	return hasAuthErrorCode(err, userDisabled)
+}
+
 // VerifySessionCookie verifies the signature and payload of the provided Firebase session cookie.
 //
 // VerifySessionCookie accepts a signed JWT token string, and verifies that it is current, issued for the
@@ -371,7 +369,7 @@ func (c *Client) VerifySessionCookie(ctx context.Context, sessionCookie string)
 }
 
 // VerifySessionCookieAndCheckRevoked verifies the provided session cookie, and additionally checks that the
-// cookie has not been revoked.
+// cookie has not been revoked and the user has not been disabled.
 //
 // Unlike `VerifySessionCookie()`, this function must make an RPC call to perform the revocation check.
 // Developers are advised to take this additional overhead into consideration when including this
@@ -380,27 +378,17 @@ func (c *Client) VerifySessionCookieAndCheckRevoked(ctx context.Context, session
 	return c.verifySessionCookie(ctx, sessionCookie, true)
 }
 
-func (c *Client) verifySessionCookie(ctx context.Context, sessionCookie string, checkRevoked bool) (*Token, error) {
+func (c *Client) verifySessionCookie(ctx context.Context, sessionCookie string, checkRevokedOrDisabled bool) (*Token, error) {
 	decoded, err := c.cookieVerifier.VerifyToken(ctx, sessionCookie, c.isEmulator)
 	if err != nil {
 		return nil, err
 	}
 
-	if c.isEmulator || checkRevoked {
-		revoked, err := c.checkRevoked(ctx, decoded)
+	if c.isEmulator || checkRevokedOrDisabled {
+		err := c.checkRevokedOrDisabled(ctx, decoded, sessionCookieRevoked, "session cookie has been revoked")
 		if err != nil {
 			return nil, err
 		}
-
-		if revoked {
-			return nil, &internal.FirebaseError{
-				ErrorCode: internal.InvalidArgument,
-				String:    "session cookie has been revoked",
-				Ext: map[string]interface{}{
-					authErrorCode: sessionCookieRevoked,
-				},
-			}
-		}
 	}
 
 	return decoded, nil
@@ -408,18 +396,37 @@ func (c *Client) verifySessionCookie(ctx context.Context, sessionCookie string,
 
 // IsSessionCookieRevoked checks if the given error was due to a revoked session cookie.
 //
-// When IsSessionCookieRevoked returns true, IsSessionCookieInvalid is guranteed to return true.
+// When IsSessionCookieRevoked returns true, IsSessionCookieInvalid is guaranteed to return true.
 func IsSessionCookieRevoked(err error) bool {
 	return hasAuthErrorCode(err, sessionCookieRevoked)
 }
 
-func (c *baseClient) checkRevoked(ctx context.Context, token *Token) (bool, error) {
+// checkRevokedOrDisabled checks whether the input token has been revoked or disabled.
+func (c *baseClient) checkRevokedOrDisabled(ctx context.Context, token *Token, errCode string, errMessage string) error {
 	user, err := c.GetUser(ctx, token.UID)
 	if err != nil {
-		return false, err
+		return err
 	}
+	if user.Disabled {
+		return &internal.FirebaseError{
+			ErrorCode: internal.InvalidArgument,
+			String:    "user has been disabled",
+			Ext: map[string]interface{}{
+				authErrorCode: userDisabled,
+			},
+		}
 
-	return token.IssuedAt*1000 < user.TokensValidAfterMillis, nil
+	}
+	if token.IssuedAt*1000 < user.TokensValidAfterMillis {
+		return &internal.FirebaseError{
+			ErrorCode: internal.InvalidArgument,
+			String:    errMessage,
+			Ext: map[string]interface{}{
+				authErrorCode: errCode,
+			},
+		}
+	}
+	return nil
 }
 
 func hasAuthErrorCode(err error, code string) bool {
diff --git a/auth/token_verifier.go b/auth/token_verifier.go
index b323941..0b1c54b 100644
--- a/auth/token_verifier.go
+++ b/auth/token_verifier.go
@@ -69,7 +69,7 @@ func IsIDTokenExpired(err error) bool {
 // An ID token is considered invalid when it is malformed (i.e. contains incorrect data), expired
 // or revoked.
 func IsIDTokenInvalid(err error) bool {
-	return hasAuthErrorCode(err, idTokenInvalid) || IsIDTokenExpired(err) || IsIDTokenRevoked(err)
+	return hasAuthErrorCode(err, idTokenInvalid) || IsIDTokenExpired(err) || IsIDTokenRevoked(err) || IsUserDisabled(err)
 }
 
 // IsSessionCookieExpired checks if the given error was due to an expired session cookie.
@@ -85,7 +85,7 @@ func IsSessionCookieExpired(err error) bool {
 // expired or revoked.
 func IsSessionCookieInvalid(err error) bool {
 	return hasAuthErrorCode(err, sessionCookieInvalid) || IsSessionCookieExpired(err) ||
-		IsSessionCookieRevoked(err)
+		IsSessionCookieRevoked(err) || IsUserDisabled(err)
 }
 
 // tokenVerifier verifies different types of Firebase token strings, including ID tokens and
diff --git a/snippets/auth.go b/snippets/auth.go
index 229e0f9..e0864b2 100644
--- a/snippets/auth.go
+++ b/snippets/auth.go
@@ -1425,6 +1425,8 @@ func verifyIDTokenAndCheckRevokedTenant(ctx context.Context, tenantClient *auth.
 	if err != nil {
 		if auth.IsIDTokenRevoked(err) {
 			// Token is revoked. Inform the user to reauthenticate or signOut() the user.
+		} else if auth.IsUserDisabled(err) {
+			// Token is disabled.
 		} else {
 			// Token is invalid
 		}
diff --git a/snippets/db.go b/snippets/db.go
index 51af750..9277ff0 100644
--- a/snippets/db.go
+++ b/snippets/db.go
@@ -20,7 +20,7 @@ import (
 	"fmt"
 	"log"
 
-	"firebase.google.com/go/v4"
+	firebase "firebase.google.com/go/v4"
 	"firebase.google.com/go/v4/db"
 	"google.golang.org/api/option"
 )


Test patch:
diff --git a/auth/auth_test.go b/auth/auth_test.go
index 854f3f6..330c174 100644
--- a/auth/auth_test.go
+++ b/auth/auth_test.go
@@ -44,12 +44,13 @@ const (
 )
 
 var (
-	testGetUserResponse []byte
-	testIDToken         string
-	testSessionCookie   string
-	testSigner          cryptoSigner
-	testIDTokenVerifier *tokenVerifier
-	testCookieVerifier  *tokenVerifier
+	testGetUserResponse         []byte
+	testGetDisabledUserResponse []byte
+	testIDToken                 string
+	testSessionCookie           string
+	testSigner                  cryptoSigner
+	testIDTokenVerifier         *tokenVerifier
+	testCookieVerifier          *tokenVerifier
 
 	optsWithServiceAcct = []option.ClientOption{
 		option.WithCredentialsFile("../testdata/service_account.json"),
@@ -76,6 +77,9 @@ func TestMain(m *testing.M) {
 	testGetUserResponse, err = ioutil.ReadFile("../testdata/get_user.json")
 	logFatal(err)
 
+	testGetDisabledUserResponse, err = ioutil.ReadFile("../testdata/get_disabled_user.json")
+	logFatal(err)
+
 	testIDToken = getIDToken(nil)
 	testSessionCookie = getSessionCookie(nil)
 	os.Exit(m.Run())
@@ -852,13 +856,13 @@ func TestVerifyIDTokenDoesNotCheckRevoked(t *testing.T) {
 	}
 }
 
-func TestInvalidTokenDoesNotCheckRevoked(t *testing.T) {
+func TestInvalidTokenDoesNotCheckRevokedOrDisabled(t *testing.T) {
 	s := echoServer(testGetUserResponse, t)
 	defer s.Close()
 	s.Client.idTokenVerifier = testIDTokenVerifier
 
 	ft, err := s.Client.VerifyIDTokenAndCheckRevoked(context.Background(), "")
-	if ft != nil || !IsIDTokenInvalid(err) || IsIDTokenRevoked(err) {
+	if ft != nil || !IsIDTokenInvalid(err) || IsIDTokenRevoked(err) || IsUserDisabled(err) {
 		t.Errorf("VerifyIDTokenAndCheckRevoked() = (%v, %v); want = (nil, IDTokenInvalid)", ft, err)
 	}
 	if len(s.Req) != 0 {
@@ -880,6 +884,20 @@ func TestVerifyIDTokenAndCheckRevokedError(t *testing.T) {
 	}
 }
 
+func TestVerifyIDTokenAndCheckDisabledError(t *testing.T) {
+	s := echoServer(testGetDisabledUserResponse, t)
+	defer s.Close()
+	revokedToken := getIDToken(mockIDTokenPayload{"uid": "uid", "iat": 1970})
+	s.Client.idTokenVerifier = testIDTokenVerifier
+
+	p, err := s.Client.VerifyIDTokenAndCheckRevoked(context.Background(), revokedToken)
+	we := "user has been disabled"
+	if p != nil || !IsUserDisabled(err) || !IsIDTokenInvalid(err) || err.Error() != we {
+		t.Errorf("VerifyIDTokenAndCheckRevoked(ctx, token) =(%v, %v); want = (%v, %v)",
+			p, err, nil, we)
+	}
+}
+
 func TestIDTokenRevocationCheckUserMgtError(t *testing.T) {
 	resp := `{
 		"kind" : "identitytoolkit#GetAccountInfoResponse",
@@ -1116,6 +1134,20 @@ func TestVerifySessionCookieAndCheckRevokedError(t *testing.T) {
 	}
 }
 
+func TestVerifySessionCookieAndCheckDisabledError(t *testing.T) {
+	s := echoServer(testGetDisabledUserResponse, t)
+	defer s.Close()
+	revokedCookie := getSessionCookie(mockIDTokenPayload{"uid": "uid", "iat": 1970})
+	s.Client.cookieVerifier = testCookieVerifier
+
+	p, err := s.Client.VerifySessionCookieAndCheckRevoked(context.Background(), revokedCookie)
+	we := "user has been disabled"
+	if p != nil || !IsUserDisabled(err) || !IsSessionCookieInvalid(err) || err.Error() != we {
+		t.Errorf("VerifySessionCookieAndCheckRevoked(ctx, token) =(%v, %v); want = (%v, %v)",
+			p, err, nil, we)
+	}
+}
+
 func TestCookieRevocationCheckUserMgtError(t *testing.T) {
 	resp := `{
 		"kind" : "identitytoolkit#GetAccountInfoResponse",
diff --git a/integration/auth/auth_test.go b/integration/auth/auth_test.go
index 37e016a..6680997 100644
--- a/integration/auth/auth_test.go
+++ b/integration/auth/auth_test.go
@@ -186,6 +186,42 @@ func TestRevokeRefreshTokens(t *testing.T) {
 	}
 }
 
+func TestIDTokenForDisabledUser(t *testing.T) {
+	uid := "user_disabled"
+	ct, err := client.CustomToken(context.Background(), uid)
+	if err != nil {
+		t.Fatal(err)
+	}
+	idt, err := signInWithCustomToken(ct)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer deleteUser(uid)
+
+	vt, err := client.VerifyIDTokenAndCheckRevoked(context.Background(), idt)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if vt.UID != uid {
+		t.Errorf("UID = %q; want UID = %q", vt.UID, uid)
+	}
+
+	// Disable the user
+	updates := auth.UserToUpdate{}
+	updates.Disabled(true)
+	_, err = client.UpdateUser(context.Background(), uid, &updates)
+	if err != nil {
+		t.Fatalf("failed to disable user with UpdateUser: %v", err)
+	}
+
+	vt, err = client.VerifyIDTokenAndCheckRevoked(context.Background(), idt)
+	we := "user has been disabled"
+	if vt != nil || err == nil || !auth.IsUserDisabled(err) || err.Error() != we {
+		t.Errorf("tok, err := VerifyIDTokenAndCheckRevoked(); got (%v, %s) ; want (%v, %v)",
+			vt, err, nil, we)
+	}
+}
+
 // verifyCustomToken verifies the given custom token by signing into a Firebase project with it.
 //
 // A successful sign in creates the user account in the Firebase back-end. This method ensures that
diff --git a/integration/storage/storage_test.go b/integration/storage/storage_test.go
index 2c2706b..9912c66 100644
--- a/integration/storage/storage_test.go
+++ b/integration/storage/storage_test.go
@@ -24,7 +24,7 @@ import (
 	"testing"
 
 	gcs "cloud.google.com/go/storage"
-	"firebase.google.com/go/v4"
+	firebase "firebase.google.com/go/v4"
 	"firebase.google.com/go/v4/integration/internal"
 	"firebase.google.com/go/v4/storage"
 )
diff --git a/testdata/get_disabled_user.json b/testdata/get_disabled_user.json
new file mode 100644
index 0000000..cbcff7e
--- /dev/null
+++ b/testdata/get_disabled_user.json
@@ -0,0 +1,22 @@
+{
+  "kind": "identitytoolkit#GetAccountInfoResponse",
+  "users": [
+    {
+      "localId": "testuser",
+      "email": "testuser@example.com",
+      "phoneNumber": "+1234567890",
+      "emailVerified": true,
+      "displayName": "Test User",
+      "photoUrl": "http://www.example.com/testuser/photo.png",
+      "passwordHash": "passwordhash",
+      "salt": "salt===",
+      "passwordUpdatedAt": 1.494364393E+12,
+      "validSince": "1494364393",
+      "disabled": true,
+      "createdAt": "1234567890000",
+      "lastLoginAt": "1233211232000",
+      "customAttributes": "{\"admin\": true, \"package\": \"gold\"}",
+      "tenantId": "testTenant"
+    }
+  ]
+}