Skip to content

Commit ea909b1

Browse files
Galadrosgoogle-oss-botgIthurielhiranya911
authored
fix(auth): check disabled status on verifyIDToken (#455)
* check disabled status on verifyIDToken * go fmt * add test cases and some other tweks * tweak some declarations, remove a double call to checkRevokedOrDisabled * Added integration test for disabled status check * make tweaks * added unit test for VerifySessionCookie * Undid public API rename Co-authored-by: Google Open Source Bot <26440463+google-oss-bot@users.noreply.github.com> Co-authored-by: Patrick Jones <ithuriel@google.com> Co-authored-by: Hiranya Jayathilaka <hiranya911@gmail.com>
1 parent 4d3cc69 commit ea909b1

File tree

8 files changed

+145
-46
lines changed

8 files changed

+145
-46
lines changed

‎auth/auth.go‎

Lines changed: 41 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const (
3737

3838
// SDK-generated error codes
3939
idTokenRevoked = "ID_TOKEN_REVOKED"
40+
userDisabled = "USER_DISABLED"
4041
sessionCookieRevoked = "SESSION_COOKIE_REVOKED"
4142
tenantIDMismatch = "TENANT_ID_MISMATCH"
4243
)
@@ -288,14 +289,14 @@ func (c *baseClient) withTenantID(tenantID string) *baseClient {
288289
// These keys get cached up to 24 hours, and therefore the RPC overhead gets amortized
289290
// over many invocations of this function.
290291
//
291-
// This does not check whether or not the token has been revoked. Use `VerifyIDTokenAndCheckRevoked()`
292+
// This does not check whether or not the token has been revoked or disabled. Use `VerifyIDTokenAndCheckRevoked()`
292293
// when a revocation check is needed.
293294
func (c *baseClient) VerifyIDToken(ctx context.Context, idToken string) (*Token, error) {
294295
return c.verifyIDToken(ctx, idToken, false)
295296
}
296297

297298
// VerifyIDTokenAndCheckRevoked verifies the provided ID token, and additionally checks that the
298-
// token has not been revoked.
299+
// token has not been revoked or disabled.
299300
//
300301
// Unlike `VerifyIDToken()`, this function must make an RPC call to perform the revocation check.
301302
// 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
304305
return c.verifyIDToken(ctx, idToken, true)
305306
}
306307

307-
func (c *baseClient) verifyIDToken(ctx context.Context, idToken string, checkRevoked bool) (*Token, error) {
308+
func (c *baseClient) verifyIDToken(ctx context.Context, idToken string, checkRevokedOrDisabled bool) (*Token, error) {
308309
decoded, err := c.idTokenVerifier.VerifyToken(ctx, idToken, c.isEmulator)
309310
if err != nil {
310311
return nil, err
@@ -320,21 +321,11 @@ func (c *baseClient) verifyIDToken(ctx context.Context, idToken string, checkRev
320321
}
321322
}
322323

323-
if c.isEmulator || checkRevoked {
324-
revoked, err := c.checkRevoked(ctx, decoded)
324+
if c.isEmulator || checkRevokedOrDisabled {
325+
err = c.checkRevokedOrDisabled(ctx, decoded, idTokenRevoked, "ID token has been revoked")
325326
if err != nil {
326327
return nil, err
327328
}
328-
329-
if revoked {
330-
return nil, &internal.FirebaseError{
331-
ErrorCode: internal.InvalidArgument,
332-
String: "ID token has been revoked",
333-
Ext: map[string]interface{}{
334-
authErrorCode: idTokenRevoked,
335-
},
336-
}
337-
}
338329
}
339330

340331
return decoded, nil
@@ -347,11 +338,18 @@ func IsTenantIDMismatch(err error) bool {
347338

348339
// IsIDTokenRevoked checks if the given error was due to a revoked ID token.
349340
//
350-
// When IsIDTokenRevoked returns true, IsIDTokenInvalid is guranteed to return true.
341+
// When IsIDTokenRevoked returns true, IsIDTokenInvalid is guaranteed to return true.
351342
func IsIDTokenRevoked(err error) bool {
352343
return hasAuthErrorCode(err, idTokenRevoked)
353344
}
354345

346+
// IsUserDisabled checks if the given error was due to a disabled ID token
347+
//
348+
// When IsUserDisabled returns true, IsIDTokenInvalid is guaranteed to return true.
349+
func IsUserDisabled(err error) bool {
350+
return hasAuthErrorCode(err, userDisabled)
351+
}
352+
355353
// VerifySessionCookie verifies the signature and payload of the provided Firebase session cookie.
356354
//
357355
// 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)
371369
}
372370

373371
// VerifySessionCookieAndCheckRevoked verifies the provided session cookie, and additionally checks that the
374-
// cookie has not been revoked.
372+
// cookie has not been revoked and the user has not been disabled.
375373
//
376374
// Unlike `VerifySessionCookie()`, this function must make an RPC call to perform the revocation check.
377375
// Developers are advised to take this additional overhead into consideration when including this
@@ -380,46 +378,55 @@ func (c *Client) VerifySessionCookieAndCheckRevoked(ctx context.Context, session
380378
return c.verifySessionCookie(ctx, sessionCookie, true)
381379
}
382380

383-
func (c *Client) verifySessionCookie(ctx context.Context, sessionCookie string, checkRevoked bool) (*Token, error) {
381+
func (c *Client) verifySessionCookie(ctx context.Context, sessionCookie string, checkRevokedOrDisabled bool) (*Token, error) {
384382
decoded, err := c.cookieVerifier.VerifyToken(ctx, sessionCookie, c.isEmulator)
385383
if err != nil {
386384
return nil, err
387385
}
388386

389-
if c.isEmulator || checkRevoked {
390-
revoked, err := c.checkRevoked(ctx, decoded)
387+
if c.isEmulator || checkRevokedOrDisabled {
388+
err := c.checkRevokedOrDisabled(ctx, decoded, sessionCookieRevoked, "session cookie has been revoked")
391389
if err != nil {
392390
return nil, err
393391
}
394-
395-
if revoked {
396-
return nil, &internal.FirebaseError{
397-
ErrorCode: internal.InvalidArgument,
398-
String: "session cookie has been revoked",
399-
Ext: map[string]interface{}{
400-
authErrorCode: sessionCookieRevoked,
401-
},
402-
}
403-
}
404392
}
405393

406394
return decoded, nil
407395
}
408396

409397
// IsSessionCookieRevoked checks if the given error was due to a revoked session cookie.
410398
//
411-
// When IsSessionCookieRevoked returns true, IsSessionCookieInvalid is guranteed to return true.
399+
// When IsSessionCookieRevoked returns true, IsSessionCookieInvalid is guaranteed to return true.
412400
func IsSessionCookieRevoked(err error) bool {
413401
return hasAuthErrorCode(err, sessionCookieRevoked)
414402
}
415403

416-
func (c *baseClient) checkRevoked(ctx context.Context, token *Token) (bool, error) {
404+
// checkRevokedOrDisabled checks whether the input token has been revoked or disabled.
405+
func (c *baseClient) checkRevokedOrDisabled(ctx context.Context, token *Token, errCode string, errMessage string) error {
417406
user, err := c.GetUser(ctx, token.UID)
418407
if err != nil {
419-
return false, err
408+
return err
420409
}
410+
if user.Disabled {
411+
return &internal.FirebaseError{
412+
ErrorCode: internal.InvalidArgument,
413+
String: "user has been disabled",
414+
Ext: map[string]interface{}{
415+
authErrorCode: userDisabled,
416+
},
417+
}
421418

422-
return token.IssuedAt*1000 < user.TokensValidAfterMillis, nil
419+
}
420+
if token.IssuedAt*1000 < user.TokensValidAfterMillis {
421+
return &internal.FirebaseError{
422+
ErrorCode: internal.InvalidArgument,
423+
String: errMessage,
424+
Ext: map[string]interface{}{
425+
authErrorCode: errCode,
426+
},
427+
}
428+
}
429+
return nil
423430
}
424431

425432
func hasAuthErrorCode(err error, code string) bool {

‎auth/auth_test.go‎

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,13 @@ const (
4444
)
4545

4646
var (
47-
testGetUserResponse []byte
48-
testIDToken string
49-
testSessionCookie string
50-
testSigner cryptoSigner
51-
testIDTokenVerifier *tokenVerifier
52-
testCookieVerifier *tokenVerifier
47+
testGetUserResponse []byte
48+
testGetDisabledUserResponse []byte
49+
testIDToken string
50+
testSessionCookie string
51+
testSigner cryptoSigner
52+
testIDTokenVerifier *tokenVerifier
53+
testCookieVerifier *tokenVerifier
5354

5455
optsWithServiceAcct = []option.ClientOption{
5556
option.WithCredentialsFile("../testdata/service_account.json"),
@@ -76,6 +77,9 @@ func TestMain(m *testing.M) {
7677
testGetUserResponse, err = ioutil.ReadFile("../testdata/get_user.json")
7778
logFatal(err)
7879

80+
testGetDisabledUserResponse, err = ioutil.ReadFile("../testdata/get_disabled_user.json")
81+
logFatal(err)
82+
7983
testIDToken = getIDToken(nil)
8084
testSessionCookie = getSessionCookie(nil)
8185
os.Exit(m.Run())
@@ -852,13 +856,13 @@ func TestVerifyIDTokenDoesNotCheckRevoked(t *testing.T) {
852856
}
853857
}
854858

855-
func TestInvalidTokenDoesNotCheckRevoked(t *testing.T) {
859+
func TestInvalidTokenDoesNotCheckRevokedOrDisabled(t *testing.T) {
856860
s := echoServer(testGetUserResponse, t)
857861
defer s.Close()
858862
s.Client.idTokenVerifier = testIDTokenVerifier
859863

860864
ft, err := s.Client.VerifyIDTokenAndCheckRevoked(context.Background(), "")
861-
if ft != nil || !IsIDTokenInvalid(err) || IsIDTokenRevoked(err) {
865+
if ft != nil || !IsIDTokenInvalid(err) || IsIDTokenRevoked(err) || IsUserDisabled(err) {
862866
t.Errorf("VerifyIDTokenAndCheckRevoked() = (%v, %v); want = (nil, IDTokenInvalid)", ft, err)
863867
}
864868
if len(s.Req) != 0 {
@@ -880,6 +884,20 @@ func TestVerifyIDTokenAndCheckRevokedError(t *testing.T) {
880884
}
881885
}
882886

887+
func TestVerifyIDTokenAndCheckDisabledError(t *testing.T) {
888+
s := echoServer(testGetDisabledUserResponse, t)
889+
defer s.Close()
890+
revokedToken := getIDToken(mockIDTokenPayload{"uid": "uid", "iat": 1970})
891+
s.Client.idTokenVerifier = testIDTokenVerifier
892+
893+
p, err := s.Client.VerifyIDTokenAndCheckRevoked(context.Background(), revokedToken)
894+
we := "user has been disabled"
895+
if p != nil || !IsUserDisabled(err) || !IsIDTokenInvalid(err) || err.Error() != we {
896+
t.Errorf("VerifyIDTokenAndCheckRevoked(ctx, token) =(%v, %v); want = (%v, %v)",
897+
p, err, nil, we)
898+
}
899+
}
900+
883901
func TestIDTokenRevocationCheckUserMgtError(t *testing.T) {
884902
resp := `{
885903
"kind" : "identitytoolkit#GetAccountInfoResponse",
@@ -1116,6 +1134,20 @@ func TestVerifySessionCookieAndCheckRevokedError(t *testing.T) {
11161134
}
11171135
}
11181136

1137+
func TestVerifySessionCookieAndCheckDisabledError(t *testing.T) {
1138+
s := echoServer(testGetDisabledUserResponse, t)
1139+
defer s.Close()
1140+
revokedCookie := getSessionCookie(mockIDTokenPayload{"uid": "uid", "iat": 1970})
1141+
s.Client.cookieVerifier = testCookieVerifier
1142+
1143+
p, err := s.Client.VerifySessionCookieAndCheckRevoked(context.Background(), revokedCookie)
1144+
we := "user has been disabled"
1145+
if p != nil || !IsUserDisabled(err) || !IsSessionCookieInvalid(err) || err.Error() != we {
1146+
t.Errorf("VerifySessionCookieAndCheckRevoked(ctx, token) =(%v, %v); want = (%v, %v)",
1147+
p, err, nil, we)
1148+
}
1149+
}
1150+
11191151
func TestCookieRevocationCheckUserMgtError(t *testing.T) {
11201152
resp := `{
11211153
"kind" : "identitytoolkit#GetAccountInfoResponse",

‎auth/token_verifier.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func IsIDTokenExpired(err error) bool {
6969
// An ID token is considered invalid when it is malformed (i.e. contains incorrect data), expired
7070
// or revoked.
7171
func IsIDTokenInvalid(err error) bool {
72-
return hasAuthErrorCode(err, idTokenInvalid) || IsIDTokenExpired(err) || IsIDTokenRevoked(err)
72+
return hasAuthErrorCode(err, idTokenInvalid) || IsIDTokenExpired(err) || IsIDTokenRevoked(err) || IsUserDisabled(err)
7373
}
7474

7575
// IsSessionCookieExpired checks if the given error was due to an expired session cookie.
@@ -85,7 +85,7 @@ func IsSessionCookieExpired(err error) bool {
8585
// expired or revoked.
8686
func IsSessionCookieInvalid(err error) bool {
8787
return hasAuthErrorCode(err, sessionCookieInvalid) || IsSessionCookieExpired(err) ||
88-
IsSessionCookieRevoked(err)
88+
IsSessionCookieRevoked(err) || IsUserDisabled(err)
8989
}
9090

9191
// tokenVerifier verifies different types of Firebase token strings, including ID tokens and

‎integration/auth/auth_test.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,42 @@ func TestRevokeRefreshTokens(t *testing.T) {
186186
}
187187
}
188188

189+
func TestIDTokenForDisabledUser(t *testing.T) {
190+
uid := "user_disabled"
191+
ct, err := client.CustomToken(context.Background(), uid)
192+
if err != nil {
193+
t.Fatal(err)
194+
}
195+
idt, err := signInWithCustomToken(ct)
196+
if err != nil {
197+
t.Fatal(err)
198+
}
199+
defer deleteUser(uid)
200+
201+
vt, err := client.VerifyIDTokenAndCheckRevoked(context.Background(), idt)
202+
if err != nil {
203+
t.Fatal(err)
204+
}
205+
if vt.UID != uid {
206+
t.Errorf("UID = %q; want UID = %q", vt.UID, uid)
207+
}
208+
209+
// Disable the user
210+
updates := auth.UserToUpdate{}
211+
updates.Disabled(true)
212+
_, err = client.UpdateUser(context.Background(), uid, &updates)
213+
if err != nil {
214+
t.Fatalf("failed to disable user with UpdateUser: %v", err)
215+
}
216+
217+
vt, err = client.VerifyIDTokenAndCheckRevoked(context.Background(), idt)
218+
we := "user has been disabled"
219+
if vt != nil || err == nil || !auth.IsUserDisabled(err) || err.Error() != we {
220+
t.Errorf("tok, err := VerifyIDTokenAndCheckRevoked(); got (%v, %s) ; want (%v, %v)",
221+
vt, err, nil, we)
222+
}
223+
}
224+
189225
// verifyCustomToken verifies the given custom token by signing into a Firebase project with it.
190226
//
191227
// A successful sign in creates the user account in the Firebase back-end. This method ensures that

‎integration/storage/storage_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
"testing"
2525

2626
gcs "cloud.google.com/go/storage"
27-
"firebase.google.com/go/v4"
27+
firebase "firebase.google.com/go/v4"
2828
"firebase.google.com/go/v4/integration/internal"
2929
"firebase.google.com/go/v4/storage"
3030
)

‎snippets/auth.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1425,6 +1425,8 @@ func verifyIDTokenAndCheckRevokedTenant(ctx context.Context, tenantClient *auth.
14251425
if err != nil {
14261426
if auth.IsIDTokenRevoked(err) {
14271427
// Token is revoked. Inform the user to reauthenticate or signOut() the user.
1428+
} else if auth.IsUserDisabled(err) {
1429+
// Token is disabled.
14281430
} else {
14291431
// Token is invalid
14301432
}

‎snippets/db.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
"fmt"
2121
"log"
2222

23-
"firebase.google.com/go/v4"
23+
firebase "firebase.google.com/go/v4"
2424
"firebase.google.com/go/v4/db"
2525
"google.golang.org/api/option"
2626
)

‎testdata/get_disabled_user.json‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
{
2+
"kind": "identitytoolkit#GetAccountInfoResponse",
3+
"users": [
4+
{
5+
"localId": "testuser",
6+
"email": "testuser@example.com",
7+
"phoneNumber": "+1234567890",
8+
"emailVerified": true,
9+
"displayName": "Test User",
10+
"photoUrl": "http://www.example.com/testuser/photo.png",
11+
"passwordHash": "passwordhash",
12+
"salt": "salt===",
13+
"passwordUpdatedAt": 1.494364393E+12,
14+
"validSince": "1494364393",
15+
"disabled": true,
16+
"createdAt": "1234567890000",
17+
"lastLoginAt": "1233211232000",
18+
"customAttributes": "{\"admin\": true, \"package\": \"gold\"}",
19+
"tenantId": "testTenant"
20+
}
21+
]
22+
}

0 commit comments

Comments
 (0)