feat(GraphQL): Validate audience in authorization JWT and change `Dgr…

…aph.Authorization` format. (#5927) (#5980)

1. Upgrades jwt-go to v4 to support multiple aud claims in JWT
2. Makes JWT token expiry field option in GraphQL.
3. Changes Dgraph.Authorization format.

Co-authored-by: David Peek <[email protected]>
(cherry picked from commit 5d789f0)

edgraph/access_ee.go

@@ -172,6 +172,7 @@ func validateToken(jwtStr string) ([]string, error) {
 		return nil, errors.Errorf("unable to parse jwt token:%v", err)
 	}
 
+	// TODO(arijit): Upgrade the jwt library to v4.0
 	claims, ok := token.Claims.(jwt.MapClaims)
 	if !ok || !token.Valid {
 		return nil, errors.Errorf("claims in jwt token is not map claims")

go.mod

@@ -20,6 +20,7 @@ require (
 	github.com/dgraph-io/dgo/v200 v200.0.0-20200401175452-e463f9234453
 	github.com/dgraph-io/ristretto v0.0.3-0.20200630154024-f66de99634de
 	github.com/dgrijalva/jwt-go v3.2.0+incompatible
+	github.com/dgrijalva/jwt-go/v4 v4.0.0-preview1
 	github.com/dgryski/go-farm v0.0.0-20200201041132-a6ae2369ad13
 	github.com/dgryski/go-groupvarint v0.0.0-20190318181831-5ce5df8ca4e1
 	github.com/dustin/go-humanize v1.0.0

go.sum

@@ -86,6 +86,8 @@ github.com/dgraph-io/ristretto v0.0.3-0.20200630154024-f66de99634de h1:t0UHb5vdo
 github.com/dgraph-io/ristretto v0.0.3-0.20200630154024-f66de99634de/go.mod h1:KPxhHT9ZxKefz+PCeOGsrHpl1qZ7i70dGTu2u+Ahh6E=
 github.com/dgrijalva/jwt-go v3.2.0+incompatible h1:7qlOGliEKZXTDg6OTjfoBKDXWrumCAMpl/TFQ4/5kLM=
 github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
+github.com/dgrijalva/jwt-go/v4 v4.0.0-preview1 h1:CaO/zOnF8VvUfEbhRatPcwKVWamvbYd8tQGRWacE9kU=
+github.com/dgrijalva/jwt-go/v4 v4.0.0-preview1/go.mod h1:+hnT3ywWDTAFrW5aE+u2Sa/wT555ZqwoCS+pk3p6ry4=
 github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2/go.mod h1:SqUrOPUnsFjfmXRMNPybcSiG0BgUW2AuFH8PAnS2iTw=
 github.com/dgryski/go-farm v0.0.0-20200201041132-a6ae2369ad13 h1:fAjc9m62+UWV/WAFKLNi6ZS0675eEUC9y3AlwSbQu1Y=
 github.com/dgryski/go-farm v0.0.0-20200201041132-a6ae2369ad13/go.mod h1:SqUrOPUnsFjfmXRMNPybcSiG0BgUW2AuFH8PAnS2iTw=

graphql/authorization/auth.go

@@ -20,78 +20,79 @@ import (
 	"bytes"
 	"context"
 	"crypto/rsa"
+	"crypto/subtle"
 	"encoding/json"
 	"fmt"
 	"net/http"
-	"regexp"
 	"strings"
-	"time"
 
-	"github.com/dgrijalva/jwt-go"
+	"github.com/dgrijalva/jwt-go/v4"
 	"github.com/pkg/errors"
 	"google.golang.org/grpc/metadata"
 )
 
 type ctxKey string
 
 const (
-	AuthJwtCtxKey = ctxKey("authorizationJwt")
-	RSA256        = "RS256"
-	HMAC256       = "HS256"
+	AuthJwtCtxKey  = ctxKey("authorizationJwt")
+	RSA256         = "RS256"
+	HMAC256        = "HS256"
+	AuthMetaHeader = "# Dgraph.Authorization "
 )
 
 var (
 	metainfo AuthMeta
 )
 
 type AuthMeta struct {
-	PublicKey    string
-	RSAPublicKey *rsa.PublicKey
-	Header       string
-	Namespace    string
-	Algo         string
+	VerificationKey string
+	RSAPublicKey    *rsa.PublicKey `json:"-"` // Ignoring this field
+	Header          string
+	Namespace       string
+	Algo            string
+	Audience        []string
 }
 
-func Parse(schema string) (AuthMeta, error) {
-	var meta AuthMeta
-	authInfoIdx := strings.LastIndex(schema, "# Dgraph.Authorization")
-	if authInfoIdx == -1 {
-		return meta, nil
+// Validate required fields.
+func (a *AuthMeta) validate() error {
+	var fields string
+	if a.VerificationKey == "" {
+		fields = " `Verification key`"
 	}
-	authInfo := schema[authInfoIdx:]
 
-	// This regex matches authorization information present in the last line of the schema.
-	// Format: # Dgraph.Authorization <HTTP header> <Claim namespace> <Algorithm> "<verification key>"
-	// Example: # Dgraph.Authorization X-Test-Auth https://xyz.io/jwt/claims HS256 "secretkey"
-	// On successful regex match the index for the following strings will be returned.
-	// [0][0]:[0][1] : # Dgraph.Authorization X-Test-Auth https://xyz.io/jwt/claims HS256 "secretkey"
-	// [0][2]:[0][3] : Authorization, [0][4]:[0][5] : X-Test-Auth,
-	// [0][6]:[0][7] : https://xyz.io/jwt/claims,
-	// [0][8]:[0][9] : HS256, [0][10]:[0][11] : secretkey
-	authMetaRegex, err :=
-		regexp.Compile(`^#[\s]([^\s]+)[\s]+([^\s]+)[\s]+([^\s]+)[\s]+([^\s]+)[\s]+"([^\"]+)"`)
-	if err != nil {
-		return meta, errors.Errorf("error while parsing jwt authorization info: %v", err)
+	if a.Header == "" {
+		fields += " `Header`"
+	}
+
+	if a.Namespace == "" {
+		fields += " `Namespace`"
 	}
-	idx := authMetaRegex.FindAllStringSubmatchIndex(authInfo, -1)
-	if len(idx) != 1 || len(idx[0]) != 12 ||
-		!strings.HasPrefix(authInfo, authInfo[idx[0][0]:idx[0][1]]) {
-		return meta, errors.Errorf("error while parsing jwt authorization info")
+
+	if a.Algo == "" {
+		fields += " `Algo`"
+	}
+
+	if len(fields) > 0 {
+		return fmt.Errorf("required field missing in Dgraph.Authorization:%s", fields)
 	}
+	return nil
+}
 
-	meta.Header = authInfo[idx[0][4]:idx[0][5]]
-	meta.Namespace = authInfo[idx[0][6]:idx[0][7]]
-	meta.Algo = authInfo[idx[0][8]:idx[0][9]]
-	meta.PublicKey = authInfo[idx[0][10]:idx[0][11]]
-	if meta.Algo == HMAC256 {
+func Parse(schema string) (AuthMeta, error) {
+	var meta AuthMeta
+	authInfoIdx := strings.LastIndex(schema, AuthMetaHeader)
+	if authInfoIdx == -1 {
 		return meta, nil
 	}
+	authInfo := schema[authInfoIdx:]
 
-	if meta.Algo != RSA256 {
-		return meta, errors.Errorf(
-			"invalid jwt algorithm: found %s, but supported options are HS256 or RS256", meta.Algo)
+	err := json.Unmarshal([]byte(authInfo[len(AuthMetaHeader):]), &meta)
+	if err != nil {
+		return meta, fmt.Errorf("Unable to parse Dgraph.Authorization. " +
+			"It may be that you are using the pre-release syntax. " +
+			"Please check the correct syntax at https://graphql.dgraph.io/authorization/")
 	}
-	return meta, nil
+	return meta, meta.validate()
 }
 
 func ParseAuthMeta(schema string) error {
@@ -108,7 +109,7 @@ func ParseAuthMeta(schema string) error {
 	// The jwt library internally uses `bytes.IndexByte(data, '\n')` to fetch new line and fails
 	// if we have newline "\n" as ASCII value {92,110} instead of the actual ASCII value of 10.
 	// To fix this we replace "\n" with new line's ASCII value.
-	bytekey := bytes.ReplaceAll([]byte(metainfo.PublicKey), []byte{92, 110}, []byte{10})
+	bytekey := bytes.ReplaceAll([]byte(metainfo.VerificationKey), []byte{92, 110}, []byte{10})
 
 	metainfo.RSAPublicKey, err = jwt.ParseRSAPublicKeyFromPEM(bytekey)
 	return err
@@ -118,6 +119,10 @@ func GetHeader() string {
 	return metainfo.Header
 }
 
+func GetAuthMeta() AuthMeta {
+	return metainfo
+}
+
 // AttachAuthorizationJwt adds any incoming JWT authorization data into the grpc context metadata.
 func AttachAuthorizationJwt(ctx context.Context, r *http.Request) context.Context {
 	authorizationJwt := r.Header.Get(metainfo.Header)
@@ -180,12 +185,41 @@ func ExtractAuthVariables(ctx context.Context) (map[string]interface{}, error) {
 	return validateToken(jwtToken[0])
 }
 
+func (c *CustomClaims) validateAudience() error {
+	// If there's no audience claim, ignore
+	if c.Audience == nil || len(c.Audience) == 0 {
+		return nil
+	}
+
+	// If there is an audience claim, but no value provided, fail
+	if metainfo.Audience == nil {
+		return fmt.Errorf("audience value was expected but not provided")
+	}
+
+	var match = false
+	for _, audStr := range c.Audience {
+		for _, expectedAudStr := range metainfo.Audience {
+			if subtle.ConstantTimeCompare([]byte(audStr), []byte(expectedAudStr)) == 1 {
+				match = true
+				break
+			}
+		}
+	}
+	if !match {
+		return fmt.Errorf("JWT `aud` value doesn't match with the audience")
+	}
+	return nil
+}
+
 func validateToken(jwtStr string) (map[string]interface{}, error) {
 	if metainfo.Algo == "" {
 		return nil, fmt.Errorf(
 			"jwt token cannot be validated because verification algorithm is not set")
 	}
 
+	// The JWT library supports comparison of `aud` in JWT against a single string. Hence, we
+	// disable the `aud` claim verification at the library end using `WithoutClaimsValidation` and
+	// use our custom validation function `validateAudience`.
 	token, err :=
 		jwt.ParseWithClaims(jwtStr, &CustomClaims{}, func(token *jwt.Token) (interface{}, error) {
 			algo, _ := token.Header["alg"].(string)
@@ -195,15 +229,15 @@ func validateToken(jwtStr string) (map[string]interface{}, error) {
 			}
 			if algo == HMAC256 {
 				if _, ok := token.Method.(*jwt.SigningMethodHMAC); ok {
-					return []byte(metainfo.PublicKey), nil
+					return []byte(metainfo.VerificationKey), nil
 				}
 			} else if algo == RSA256 {
 				if _, ok := token.Method.(*jwt.SigningMethodRSA); ok {
 					return metainfo.RSAPublicKey, nil
 				}
 			}
 			return nil, errors.Errorf("couldn't parse signing method from token header: %s", algo)
-		})
+		}, jwt.WithoutClaimsValidation())
 
 	if err != nil {
 		return nil, errors.Errorf("unable to parse jwt token:%v", err)
@@ -214,11 +248,8 @@ func validateToken(jwtStr string) (map[string]interface{}, error) {
 		return nil, errors.Errorf("claims in jwt token is not map claims")
 	}
 
-	// by default, the MapClaims.Valid will return true if the exp field is not set
-	// here we enforce the checking to make sure that the refresh token has not expired
-	now := time.Now().Unix()
-	if !claims.VerifyExpiresAt(now, true) {
-		return nil, errors.Errorf("Token is expired") // the same error msg that's used inside jwt-go
+	if err := claims.validateAudience(); err != nil {
+		return nil, err
 	}
 
 	return claims.AuthVariables, nil

graphql/e2e/auth/auth_test.go

@@ -1098,7 +1098,7 @@ func TestMain(m *testing.M) {
 		}
 
 		metaInfo = &testutil.AuthMeta{
-			PublicKey: authMeta.PublicKey,
+			PublicKey: authMeta.VerificationKey,
 			Namespace: authMeta.Namespace,
 			Algo:      authMeta.Algo,
 			Header:    authMeta.Header,

graphql/resolve/auth_test.go

@@ -19,6 +19,7 @@ package resolve
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"strconv"
 	"strings"
@@ -178,6 +179,68 @@ func TestStringCustomClaim(t *testing.T) {
 	require.Equal(t, authVar, result)
 }
 
+func TestAudienceClaim(t *testing.T) {
+	sch, err := ioutil.ReadFile("../e2e/auth/schema.graphql")
+	require.NoError(t, err, "Unable to read schema file")
+
+	authSchema, err := testutil.AppendAuthInfo(sch, authorization.HMAC256, "")
+	require.NoError(t, err)
+
+	test.LoadSchemaFromString(t, string(authSchema))
+
+	// Verify that authorization information is set correctly.
+	metainfo := authorization.GetAuthMeta()
+	require.Equal(t, metainfo.Algo, authorization.HMAC256)
+	require.Equal(t, metainfo.Header, "X-Test-Auth")
+	require.Equal(t, metainfo.Namespace, "https://xyz.io/jwt/claims")
+	require.Equal(t, metainfo.VerificationKey, "secretkey")
+	require.Equal(t, metainfo.Audience, []string{"aud1", "63do0q16n6ebjgkumu05kkeian", "aud5"})
+
+	testCases := []struct {
+		name  string
+		token string
+		err   error
+	}{
+		{
+			name:  `Token with valid audience: { "aud": "63do0q16n6ebjgkumu05kkeian" }`,
+			token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImF1ZCI6IjYzZG8wcTE2bjZlYmpna3VtdTA1a2tlaWFuIiwiZXZlbnRfaWQiOiIzMWM5ZDY4NC0xZDQ1LTQ2ZjctOGMyYi1jYzI3YjFmNmYwMWIiLCJ0b2tlbl91c2UiOiJpZCIsImF1dGhfdGltZSI6MTU5MDMzMzM1NiwibmFtZSI6IkRhdmlkIFBlZWsiLCJleHAiOjk1OTAzNzYwMzIsImlhdCI6MTU5MDM3MjQzMiwiZW1haWwiOiJkYXZpZEB0eXBlam9pbi5jb20ifQ.whgQ9QVMOa0jFYBKhCytlm25-dJiIxcfUFligjav0K0",
+		},
+		{
+			name:  `Token with invalid audience: { "aud": "invalidAudience" }`,
+			token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImF1ZCI6ImludmFsaWRBdWRpZW5jZSIsImV2ZW50X2lkIjoiMzFjOWQ2ODQtMWQ0NS00NmY3LThjMmItY2MyN2IxZjZmMDFiIiwidG9rZW5fdXNlIjoiaWQiLCJhdXRoX3RpbWUiOjE1OTAzMzMzNTYsIm5hbWUiOiJEYXZpZCBQZWVrIiwiZXhwIjo5NTkwMzc2MDMyLCJpYXQiOjE1OTAzNzI0MzIsImVtYWlsIjoiZGF2aWRAdHlwZWpvaW4uY29tIn0.b-K-NtNAswh3_YwYmvEMd0IkghbDQbw2w9kFkCJ3tuM",
+			err:   fmt.Errorf("JWT `aud` value doesn't match with the audience"),
+		},
+		{
+			name:  "Token without audience field",
+			token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImV2ZW50X2lkIjoiMzFjOWQ2ODQtMWQ0NS00NmY3LThjMmItY2MyN2IxZjZmMDFiIiwidG9rZW5fdXNlIjoiaWQiLCJhdXRoX3RpbWUiOjE1OTAzMzMzNTYsIm5hbWUiOiJEYXZpZCBQZWVrIiwiZXhwIjo5NTkwMzc2MDMyLCJpYXQiOjE1OTAzNzI0MzIsImVtYWlsIjoiZGF2aWRAdHlwZWpvaW4uY29tIn0.rWLwxa_IyQcQITVu7dVDYNygipfpQt9t2IimrmQ_BJo",
+		},
+		{
+			name:  `Token with multiple audience: {"aud": ["aud1", "aud2", "aud3"]}`,
+			token: "eyJraWQiOiIyRWplN2tIRklLZS92MFRVT3JRYlVJWWJxSWNNUHZ2TFBjM3RSQ25EclBBPSIsImFsZyI6IkhTMjU2In0.eyJzdWIiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJjb2duaXRvOmdyb3VwcyI6WyJBRE1JTiJdLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5hcC1zb3V0aGVhc3QtMi5hbWF6b25hd3MuY29tL2FwLXNvdXRoZWFzdC0yX0dmbWVIZEZ6NCIsImNvZ25pdG86dXNlcm5hbWUiOiI1MDk1MGI0MC0yNjJmLTRiMjYtODhhNy1jYmJiNzgwYjIxNzYiLCJodHRwczovL3h5ei5pby9qd3QvY2xhaW1zIjoie1wiVVNFUlwiOiBcIjUwOTUwYjQwLTI2MmYtNGIyNi04OGE3LWNiYmI3ODBiMjE3NlwiLCBcIlJPTEVcIjogXCJBRE1JTlwifSIsImF1ZCI6WyJhdWQxIiwiYXVkMiIsImF1ZDMiXSwiZXZlbnRfaWQiOiIzMWM5ZDY4NC0xZDQ1LTQ2ZjctOGMyYi1jYzI3YjFmNmYwMWIiLCJ0b2tlbl91c2UiOiJpZCIsImF1dGhfdGltZSI6MTU5MDMzMzM1NiwibmFtZSI6IkRhdmlkIFBlZWsiLCJleHAiOjk1OTAzNzYwMzIsImlhdCI6MTU5MDM3MjQzMiwiZW1haWwiOiJkYXZpZEB0eXBlam9pbi5jb20ifQ.gDI-4e9v15643CjxoSCjE_UJ-ext8eiP-OUMsaZmOdw",
+		},
+	}
+
+	for _, tcase := range testCases {
+		t.Run(tcase.name, func(t *testing.T) {
+			md := metadata.New(map[string]string{"authorizationJwt": tcase.token})
+			ctx := metadata.NewIncomingContext(context.Background(), md)
+
+			authVar, err := authorization.ExtractAuthVariables(ctx)
+			require.Equal(t, tcase.err, err)
+
+			if err != nil {
+				return
+			}
+
+			result := map[string]interface{}{
+				"ROLE": "ADMIN",
+				"USER": "50950b40-262f-4b26-88a7-cbbb780b2176",
+			}
+			require.Equal(t, authVar, result)
+		})
+	}
+}
+
 // Tests showing that the query rewriter produces the expected Dgraph queries
 // when it also needs to write in auth.
 func queryRewriting(t *testing.T, sch string, authMeta *testutil.AuthMeta) {
@@ -586,7 +649,7 @@ func TestAuthQueryRewriting(t *testing.T) {
 
 		authMeta, err := authorization.Parse(strSchema)
 		metaInfo := &testutil.AuthMeta{
-			PublicKey: authMeta.PublicKey,
+			PublicKey: authMeta.VerificationKey,
 			Namespace: authMeta.Namespace,
 			Algo:      authMeta.Algo,
 		}