From 575897989617083530b35838eefece7d969c3373 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Mon, 16 Apr 2018 17:18:56 -0700 Subject: [PATCH 1/3] Cleaning up the auth package --- auth/auth.go | 83 +++++++++++++++++++++++------------------------ auth/auth_test.go | 6 ++-- auth/crypto.go | 21 ++++++++++++ auth/jwt.go | 42 +++++++----------------- auth/jwt_test.go | 6 ++-- 5 files changed, 78 insertions(+), 80 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index 8149e08a..554b85a0 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -17,9 +17,7 @@ package auth import ( "crypto/rsa" - "crypto/x509" "encoding/json" - "encoding/pem" "errors" "fmt" "strings" @@ -31,10 +29,12 @@ import ( "google.golang.org/api/transport" ) -const firebaseAudience = "https://identitytoolkit.googleapis.com/google.identity.identitytoolkit.v1.IdentityToolkit" -const googleCertURL = "https://www.googleapis.com/robot/v1/metadata/x509/securetoken@system.gserviceaccount.com" -const issuerPrefix = "https://securetoken.google.com/" -const tokenExpSeconds = 3600 +const ( + firebaseAudience = "https://identitytoolkit.googleapis.com/google.identity.identitytoolkit.v1.IdentityToolkit" + idTokenCertURL = "https://www.googleapis.com/robot/v1/metadata/x509/securetoken@system.gserviceaccount.com" + issuerPrefix = "https://securetoken.google.com/" + tokenExpSeconds = 3600 +) var reservedClaims = []string{ "acr", "amr", "at_hash", "aud", "auth_time", "azp", "cnf", "c_hash", @@ -58,6 +58,22 @@ type Token struct { Claims map[string]interface{} `json:"-"` } +func (t *Token) decodeFrom(s string) error { + claims := make(map[string]interface{}) + if err := decode(s, &claims); err != nil { + return err + } + if err := decode(s, t); err != nil { + return err + } + + for _, r := range []string{"iss", "aud", "exp", "iat", "sub", "uid"} { + delete(claims, r) + } + t.Claims = claims + return nil +} + // Client is the interface for the Firebase auth service. // // Client facilitates generating custom JWT tokens for Firebase clients, and verifying ID tokens issued @@ -94,7 +110,7 @@ func NewClient(ctx context.Context, c *internal.AuthConfig) (*Client, error) { return nil, err } if svcAcct.PrivateKey != "" { - pk, err = parseKey(svcAcct.PrivateKey) + pk, err = parsePrivateKey(svcAcct.PrivateKey) if err != nil { return nil, err } @@ -124,7 +140,7 @@ func NewClient(ctx context.Context, c *internal.AuthConfig) (*Client, error) { return &Client{ is: is, - ks: newHTTPKeySource(googleCertURL, hc), + ks: newHTTPKeySource(idTokenCertURL, hc), projectID: c.ProjectID, snr: snr, version: "Go/Admin/" + c.Version, @@ -164,6 +180,7 @@ func (c *Client) CustomTokenWithClaims(ctx context.Context, uid string, devClaim } now := clk.Now().Unix() + header := jwtHeader{Algorithm: "RS256", Type: "JWT"} payload := &customToken{ Iss: iss, Sub: iss, @@ -173,7 +190,7 @@ func (c *Client) CustomTokenWithClaims(ctx context.Context, uid string, devClaim Exp: now + tokenExpSeconds, Claims: devClaims, } - return encodeToken(ctx, c.snr, defaultHeader(), payload) + return encodeToken(ctx, c.snr, header, payload) } // RevokeRefreshTokens revokes all refresh tokens issued to a user. @@ -201,7 +218,7 @@ func (c *Client) VerifyIDToken(ctx context.Context, idToken string) (*Token, err return nil, errors.New("project id not available") } if idToken == "" { - return nil, fmt.Errorf("ID token must be a non-empty string") + return nil, fmt.Errorf("id token must be a non-empty string") } h := &jwtHeader{} @@ -210,36 +227,36 @@ func (c *Client) VerifyIDToken(ctx context.Context, idToken string) (*Token, err return nil, err } - projectIDMsg := "Make sure the ID token comes from the same Firebase project as the credential used to" + + projectIDMsg := "make sure the id token comes from the same Firebase project as the credential used to" + " authenticate this SDK." - verifyTokenMsg := "See https://firebase.google.com/docs/auth/admin/verify-id-tokens for details on how to " + - "retrieve a valid ID token." + verifyTokenMsg := "see https://firebase.google.com/docs/auth/admin/verify-id-tokens for details on how to " + + "retrieve a valid id token." issuer := issuerPrefix + c.projectID var err error if h.KeyID == "" { if p.Audience == firebaseAudience { - err = fmt.Errorf("VerifyIDToken() expects an ID token, but was given a custom token") + err = fmt.Errorf("expected an id token but got a custom token") } else { - err = fmt.Errorf("ID token has no 'kid' header") + err = fmt.Errorf("id token has no 'kid' header") } } else if h.Algorithm != "RS256" { - err = fmt.Errorf("ID token has invalid incorrect algorithm. Expected 'RS256' but got %q. %s", + err = fmt.Errorf("id token has invalid algorithm; expected 'RS256' but got %q; %s", h.Algorithm, verifyTokenMsg) } else if p.Audience != c.projectID { - err = fmt.Errorf("ID token has invalid 'aud' (audience) claim. Expected %q but got %q. %s %s", + err = fmt.Errorf("id token has invalid 'aud' (audience) claim; expected %q but got %q; %s; %s", c.projectID, p.Audience, projectIDMsg, verifyTokenMsg) } else if p.Issuer != issuer { - err = fmt.Errorf("ID token has invalid 'iss' (issuer) claim. Expected %q but got %q. %s %s", + err = fmt.Errorf("id token has invalid 'iss' (issuer) claim; expected %q but got %q; %s; %s", issuer, p.Issuer, projectIDMsg, verifyTokenMsg) } else if p.IssuedAt > clk.Now().Unix() { - err = fmt.Errorf("ID token issued at future timestamp: %d", p.IssuedAt) + err = fmt.Errorf("id token issued at future timestamp: %d", p.IssuedAt) } else if p.Expires < clk.Now().Unix() { - err = fmt.Errorf("ID token has expired. Expired at: %d", p.Expires) + err = fmt.Errorf("id token has expired at: %d", p.Expires) } else if p.Subject == "" { - err = fmt.Errorf("ID token has empty 'sub' (subject) claim. %s", verifyTokenMsg) + err = fmt.Errorf("id token has empty 'sub' (subject) claim; %s", verifyTokenMsg) } else if len(p.Subject) > 128 { - err = fmt.Errorf("ID token has a 'sub' (subject) claim longer than 128 characters. %s", verifyTokenMsg) + err = fmt.Errorf("id token has a 'sub' (subject) claim longer than 128 characters; %s", verifyTokenMsg) } if err != nil { @@ -265,27 +282,7 @@ func (c *Client) VerifyIDTokenAndCheckRevoked(ctx context.Context, idToken strin } if p.IssuedAt*1000 < user.TokensValidAfterMillis { - return nil, internal.Error(idTokenRevoked, "ID token has been revoked") + return nil, internal.Error(idTokenRevoked, "id token has been revoked") } return p, nil } - -func parseKey(key string) (*rsa.PrivateKey, error) { - block, _ := pem.Decode([]byte(key)) - if block == nil { - return nil, fmt.Errorf("no private key data found in: %v", key) - } - k := block.Bytes - parsedKey, err := x509.ParsePKCS8PrivateKey(k) - if err != nil { - parsedKey, err = x509.ParsePKCS1PrivateKey(k) - if err != nil { - return nil, fmt.Errorf("private key should be a PEM or plain PKSC1 or PKCS8; parse error: %v", err) - } - } - parsed, ok := parsedKey.(*rsa.PrivateKey) - if !ok { - return nil, errors.New("private key is not an RSA key") - } - return parsed, nil -} diff --git a/auth/auth_test.go b/auth/auth_test.go index 25659605..133454f3 100644 --- a/auth/auth_test.go +++ b/auth/auth_test.go @@ -232,7 +232,7 @@ func TestVerifyIDTokenAndCheckRevokedInvalidated(t *testing.T) { tok := getIDToken(mockIDTokenPayload{"uid": "uid", "iat": 1970}) // old token p, err := s.Client.VerifyIDTokenAndCheckRevoked(ctx, tok) - we := "ID token has been revoked" + we := "id token has been revoked" if p != nil || err == nil || err.Error() != we || !IsIDTokenRevoked(err) { t.Errorf("VerifyIDTokenAndCheckRevoked(ctx, token) =(%v, %v); want = (%v, %v)", p, err, nil, we) @@ -371,7 +371,7 @@ func getIDTokenWithKid(kid string, p mockIDTokenPayload) string { for k, v := range p { pCopy[k] = v } - h := defaultHeader() + h := jwtHeader{Algorithm: "RS256", Type: "JWT"} h.KeyID = kid token, err := encodeToken(ctx, client.snr, h, pCopy) if err != nil { @@ -382,7 +382,7 @@ func getIDTokenWithKid(kid string, p mockIDTokenPayload) string { type mockIDTokenPayload map[string]interface{} -func (p mockIDTokenPayload) decode(s string) error { +func (p mockIDTokenPayload) decodeFrom(s string) error { return decode(s, &p) } diff --git a/auth/crypto.go b/auth/crypto.go index da2ab748..315659d3 100644 --- a/auth/crypto.go +++ b/auth/crypto.go @@ -24,6 +24,7 @@ import ( "encoding/json" "encoding/pem" "errors" + "fmt" "io/ioutil" "net/http" "strconv" @@ -185,6 +186,26 @@ func parsePublicKey(kid string, key []byte) (*publicKey, error) { return &publicKey{kid, pk}, nil } +func parsePrivateKey(key string) (*rsa.PrivateKey, error) { + block, _ := pem.Decode([]byte(key)) + if block == nil { + return nil, fmt.Errorf("no private key data found in: %v", key) + } + k := block.Bytes + parsedKey, err := x509.ParsePKCS8PrivateKey(k) + if err != nil { + parsedKey, err = x509.ParsePKCS1PrivateKey(k) + if err != nil { + return nil, fmt.Errorf("private key should be a PEM or plain PKSC1 or PKCS8; parse error: %v", err) + } + } + parsed, ok := parsedKey.(*rsa.PrivateKey) + if !ok { + return nil, errors.New("private key is not an RSA key") + } + return parsed, nil +} + func verifySignature(parts []string, k *publicKey) error { content := parts[0] + "." + parts[1] signature, err := base64.RawURLEncoding.DecodeString(parts[2]) diff --git a/auth/jwt.go b/auth/jwt.go index 376f404c..c62da6c5 100644 --- a/auth/jwt.go +++ b/auth/jwt.go @@ -32,7 +32,7 @@ type jwtHeader struct { } type jwtPayload interface { - decode(s string) error + decodeFrom(s string) error } type customToken struct { @@ -45,38 +45,11 @@ type customToken struct { Claims map[string]interface{} `json:"claims,omitempty"` } -func (p *customToken) decode(s string) error { +func (p *customToken) decodeFrom(s string) error { return decode(s, p) } -func (t *Token) decode(s string) error { - claims := make(map[string]interface{}) - if err := decode(s, &claims); err != nil { - return err - } - if err := decode(s, t); err != nil { - return err - } - - for _, r := range []string{"iss", "aud", "exp", "iat", "sub", "uid"} { - delete(claims, r) - } - t.Claims = claims - return nil -} - -func defaultHeader() jwtHeader { - return jwtHeader{Algorithm: "RS256", Type: "JWT"} -} - -func encode(i interface{}) (string, error) { - b, err := json.Marshal(i) - if err != nil { - return "", err - } - return base64.RawURLEncoding.EncodeToString(b), nil -} - +// decode accepts a JWT segment, and decodes it into the given interface. func decode(s string, i interface{}) error { decoded, err := base64.RawURLEncoding.DecodeString(s) if err != nil { @@ -86,6 +59,13 @@ func decode(s string, i interface{}) error { } func encodeToken(ctx context.Context, s signer, h jwtHeader, p jwtPayload) (string, error) { + encode := func(i interface{}) (string, error) { + b, err := json.Marshal(i) + if err != nil { + return "", err + } + return base64.RawURLEncoding.EncodeToString(b), nil + } header, err := encode(h) if err != nil { return "", err @@ -112,7 +92,7 @@ func decodeToken(ctx context.Context, token string, ks keySource, h *jwtHeader, if err := decode(s[0], h); err != nil { return err } - if err := p.decode(s[1]); err != nil { + if err := p.decodeFrom(s[1]); err != nil { return err } diff --git a/auth/jwt_test.go b/auth/jwt_test.go index b8865eda..76da8388 100644 --- a/auth/jwt_test.go +++ b/auth/jwt_test.go @@ -24,7 +24,7 @@ import ( ) func TestEncodeToken(t *testing.T) { - h := defaultHeader() + h := jwtHeader{Algorithm: "RS256", Type: "JWT"} p := mockIDTokenPayload{"key": "value"} s, err := encodeToken(ctx, &mockSigner{}, h, p) if err != nil { @@ -57,7 +57,7 @@ func TestEncodeToken(t *testing.T) { } func TestEncodeSignError(t *testing.T) { - h := defaultHeader() + h := jwtHeader{Algorithm: "RS256", Type: "JWT"} p := mockIDTokenPayload{"key": "value"} signer := &mockSigner{ err: errors.New("sign error"), @@ -68,7 +68,7 @@ func TestEncodeSignError(t *testing.T) { } func TestEncodeInvalidPayload(t *testing.T) { - h := defaultHeader() + h := jwtHeader{Algorithm: "RS256", Type: "JWT"} p := mockIDTokenPayload{"key": func() {}} if s, err := encodeToken(ctx, &mockSigner{}, h, p); s != "" || err == nil { t.Errorf("encodeToken() = (%v, %v); want = ('', error)", s, err) From a84e944e72ecee7d46046e41b7ceee1df206c0a4 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Mon, 16 Apr 2018 17:36:08 -0700 Subject: [PATCH 2/3] Variable grouping --- auth/auth_test.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/auth/auth_test.go b/auth/auth_test.go index 133454f3..1754a24a 100644 --- a/auth/auth_test.go +++ b/auth/auth_test.go @@ -37,11 +37,14 @@ import ( "firebase.google.com/go/internal" ) -var client *Client -var ctx context.Context -var testIDToken string -var testGetUserResponse []byte -var testListUsersResponse []byte +var ( + client *Client + ctx context.Context + testIDToken string + testGetUserResponse []byte + testListUsersResponse []byte +) + var defaultTestOpts = []option.ClientOption{ option.WithCredentialsFile("../testdata/service_account.json"), } From c37bdddcb10eb8891e311e9f8192c9da2f3f820d Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 19 Apr 2018 13:43:22 -0700 Subject: [PATCH 3/3] Removing some punctuation; Changed id to ID in error messages --- auth/auth.go | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index 554b85a0..cc1ef9b6 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -59,10 +59,12 @@ type Token struct { } func (t *Token) decodeFrom(s string) error { + // Decode into a regular map to access custom claims. claims := make(map[string]interface{}) if err := decode(s, &claims); err != nil { return err } + // Now decode into Token to access the standard claims. if err := decode(s, t); err != nil { return err } @@ -227,36 +229,36 @@ func (c *Client) VerifyIDToken(ctx context.Context, idToken string) (*Token, err return nil, err } - projectIDMsg := "make sure the id token comes from the same Firebase project as the credential used to" + - " authenticate this SDK." + projectIDMsg := "make sure the ID token comes from the same Firebase project as the credential used to" + + " authenticate this SDK" verifyTokenMsg := "see https://firebase.google.com/docs/auth/admin/verify-id-tokens for details on how to " + - "retrieve a valid id token." + "retrieve a valid ID token" issuer := issuerPrefix + c.projectID var err error if h.KeyID == "" { if p.Audience == firebaseAudience { - err = fmt.Errorf("expected an id token but got a custom token") + err = fmt.Errorf("expected an ID token but got a custom token") } else { - err = fmt.Errorf("id token has no 'kid' header") + err = fmt.Errorf("ID token has no 'kid' header") } } else if h.Algorithm != "RS256" { - err = fmt.Errorf("id token has invalid algorithm; expected 'RS256' but got %q; %s", + err = fmt.Errorf("ID token has invalid algorithm; expected 'RS256' but got %q; %s", h.Algorithm, verifyTokenMsg) } else if p.Audience != c.projectID { - err = fmt.Errorf("id token has invalid 'aud' (audience) claim; expected %q but got %q; %s; %s", + err = fmt.Errorf("ID token has invalid 'aud' (audience) claim; expected %q but got %q; %s; %s", c.projectID, p.Audience, projectIDMsg, verifyTokenMsg) } else if p.Issuer != issuer { - err = fmt.Errorf("id token has invalid 'iss' (issuer) claim; expected %q but got %q; %s; %s", + err = fmt.Errorf("ID token has invalid 'iss' (issuer) claim; expected %q but got %q; %s; %s", issuer, p.Issuer, projectIDMsg, verifyTokenMsg) } else if p.IssuedAt > clk.Now().Unix() { - err = fmt.Errorf("id token issued at future timestamp: %d", p.IssuedAt) + err = fmt.Errorf("ID token issued at future timestamp: %d", p.IssuedAt) } else if p.Expires < clk.Now().Unix() { - err = fmt.Errorf("id token has expired at: %d", p.Expires) + err = fmt.Errorf("ID token has expired at: %d", p.Expires) } else if p.Subject == "" { - err = fmt.Errorf("id token has empty 'sub' (subject) claim; %s", verifyTokenMsg) + err = fmt.Errorf("ID token has empty 'sub' (subject) claim; %s", verifyTokenMsg) } else if len(p.Subject) > 128 { - err = fmt.Errorf("id token has a 'sub' (subject) claim longer than 128 characters; %s", verifyTokenMsg) + err = fmt.Errorf("ID token has a 'sub' (subject) claim longer than 128 characters; %s", verifyTokenMsg) } if err != nil {