Skip to content

Commit

Permalink
feat(GraphQL): Dgraph.Authorization should with irrespective of numbe…
Browse files Browse the repository at this point in the history
…r of spaces after # (#7410)

This PR ignores the spacing between `#` and `Dgraph.Authorization` in the GraphQL schema. Now the user can give any number of spaces or no space between it. For example, the header `#Dgraph.Authorization.....` will also be valid which was earlier not the case.
  • Loading branch information
minhaj-shakeel authored Feb 12, 2021
1 parent a26993a commit 6383300
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 39 deletions.
5 changes: 4 additions & 1 deletion graphql/authorization/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type authVariablekey string
const (
AuthJwtCtxKey = ctxKey("authorizationJwt")
AuthVariables = authVariablekey("authVariable")
AuthMetaHeader = "# Dgraph.Authorization "
AuthMetaHeader = "Dgraph.Authorization"
)

var (
Expand Down Expand Up @@ -152,6 +152,9 @@ func Parse(schema string) (*AuthMeta, error) {
return nil, gqlerror.Errorf("JWT parsing failed: %v", err)
}

// authInfo with be like `Dgraph.Authorization ...`, we append prefix `# ` to authinfo
// to make it work with the regex matching algorithm.
authInfo = "# " + authInfo
idx := authMetaRegex.FindAllStringSubmatchIndex(authInfo, -1)
if len(idx) != 1 || len(idx[0]) != 12 ||
!strings.HasPrefix(authInfo, authInfo[idx[0][0]:idx[0][1]]) {
Expand Down
58 changes: 30 additions & 28 deletions graphql/schema/schemagen.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,39 +175,41 @@ func parseSecrets(sch string) (map[string]string, *authorization.AuthMeta, error
for scanner.Scan() {
text := strings.TrimSpace(scanner.Text())

if strings.HasPrefix(text, "# Dgraph.Authorization") {
if authSecret != "" {
return nil, nil, errors.Errorf("Dgraph.Authorization should be only be specified once in "+
"a schema, found second mention: %v", text)
if strings.HasPrefix(text, "#") {
header := strings.TrimSpace(text[1:])
if strings.HasPrefix(header, "Dgraph.Authorization") {
if authSecret != "" {
return nil, nil, errors.Errorf("Dgraph.Authorization should be only be specified once in "+
"a schema, found second mention: %v", text)
}
authSecret = text
continue
}
authSecret = text
continue
}
if !strings.HasPrefix(text, "# Dgraph.Secret") {
continue
}
parts := strings.Fields(text)
const doubleQuotesCode = 34
if !strings.HasPrefix(header, "Dgraph.Secret") {
continue
}
parts := strings.Fields(text)
const doubleQuotesCode = 34

if len(parts) < 4 {
return nil, nil, errors.Errorf("incorrect format for specifying Dgraph secret found for "+
"comment: `%s`, it should be `# Dgraph.Secret key value`", text)
}
val := strings.Join(parts[3:], " ")
if strings.Count(val, `"`) != 2 || val[0] != doubleQuotesCode || val[len(val)-1] != doubleQuotesCode {
return nil, nil, errors.Errorf("incorrect format for specifying Dgraph secret found for "+
"comment: `%s`, it should be `# Dgraph.Secret key value`", text)
}
if len(parts) < 4 {
return nil, nil, errors.Errorf("incorrect format for specifying Dgraph secret found for "+
"comment: `%s`, it should be `# Dgraph.Secret key value`", text)
}
val := strings.Join(parts[3:], " ")
if strings.Count(val, `"`) != 2 || val[0] != doubleQuotesCode || val[len(val)-1] != doubleQuotesCode {
return nil, nil, errors.Errorf("incorrect format for specifying Dgraph secret found for "+
"comment: `%s`, it should be `# Dgraph.Secret key value`", text)
}

val = strings.Trim(val, `"`)
key := strings.Trim(parts[2], `"`)
m[key] = val
}
val = strings.Trim(val, `"`)
key := strings.Trim(parts[2], `"`)
m[key] = val
}

if err := scanner.Err(); err != nil {
return nil, nil, errors.Wrapf(err, "while trying to parse secrets from schema file")
if err := scanner.Err(); err != nil {
return nil, nil, errors.Wrapf(err, "while trying to parse secrets from schema file")
}
}

if authSecret == "" {
return m, nil, nil
}
Expand Down
30 changes: 22 additions & 8 deletions graphql/schema/wrappers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,7 @@ func TestParseSecrets(t *testing.T) {
# Dgraph.Secret GITHUB_API_TOKEN "some-super-secret-token"
# Dgraph.Secret STRIPE_API_KEY "stripe-api-key-value"
`,
`,
map[string]string{"GITHUB_API_TOKEN": "some-super-secret-token",
"STRIPE_API_KEY": "stripe-api-key-value"},
"",
Expand All @@ -1137,7 +1137,7 @@ func TestParseSecrets(t *testing.T) {
# Dgraph.Secret STRIPE_API_KEY "stripe-api-key-value"
# random comment
`,
`,
map[string]string{"GITHUB_API_TOKEN": "some-super-secret-token",
"STRIPE_API_KEY": "stripe-api-key-value"},
"",
Expand All @@ -1146,13 +1146,13 @@ func TestParseSecrets(t *testing.T) {
{
"should throw an error if the secret is not in the correct format",
`
type User {
id: ID!
name: String!
}
type User {
id: ID!
name: String!
}
# Dgraph.Secret RANDOM_TOKEN
`,
# Dgraph.Secret RANDOM_TOKEN
`,
nil,
"",
errors.New("incorrect format for specifying Dgraph secret found for " +
Expand Down Expand Up @@ -1222,6 +1222,20 @@ func TestParseSecrets(t *testing.T) {
"",
errors.New("required field missing in Dgraph.Authorization: `Verification key`/`JWKUrl` `Algo` `Header` `Namespace`"),
},
{
"Should be able to parse Dgraph.Authorization irrespective of spacing between # and Dgraph.Authorization",
`
type User {
id: ID!
name: String!
}
#Dgraph.Authorization {"VerificationKey":"secretkey","Header":"X-Test-Auth","Namespace":"https://xyz.io/jwt/claims","Algo":"HS256","Audience":["aud1","63do0q16n6ebjgkumu05kkeian","aud5"]}
`,
map[string]string{},
"X-Test-Auth",
nil,
},
{
"Valid Dgraph.Authorization with audience field",
`
Expand Down
4 changes: 2 additions & 2 deletions testutil/graphql.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func AppendAuthInfo(schema []byte, algo, publicKeyFile string, closedByDefault b
}

func AppendAuthInfoWithJWKUrl(schema []byte) ([]byte, error) {
authInfo := `# Dgraph.Authorization {"VerificationKey":"","Header":"X-Test-Auth","jwkurl":"https://www.googleapis.com/service_accounts/v1/jwk/securetoken@system.gserviceaccount.com", "Namespace":"https://xyz.io/jwt/claims","Algo":"","Audience":["fir-project1-259e7"]}`
authInfo := `# Dgraph.Authorization {"VerificationKey":"","Header":"X-Test-Auth","jwkurl":"https://www.googleapis.com/service_accounts/v1/jwk/securetoken@system.gserviceaccount.com", "Namespace":"https://xyz.io/jwt/claims","Algo":"","Audience":["fir-project1-259e7"]}`
return append(schema, []byte(authInfo)...), nil
}

Expand All @@ -263,7 +263,7 @@ func AppendAuthInfoWithJWKUrlAndWithoutAudience(schema []byte) ([]byte, error) {
// Add JWKUrl and (VerificationKey, Algo) in the same Authorization JSON
// Adding Dummy values as this should result in validation error
func AppendJWKAndVerificationKey(schema []byte) ([]byte, error) {
authInfo := `# Dgraph.Authorization {"VerificationKey":"some-key","Header":"X-Test-Auth","jwkurl":"some-url", "Namespace":"https://xyz.io/jwt/claims","Algo":"algo","Audience":["fir-project1-259e7"]}`
authInfo := `#Dgraph.Authorization{"VerificationKey":"some-key","Header":"X-Test-Auth","jwkurl":"some-url", "Namespace":"https://xyz.io/jwt/claims","Algo":"algo","Audience":["fir-project1-259e7"]}`
return append(schema, []byte(authInfo)...), nil
}

Expand Down

0 comments on commit 6383300

Please sign in to comment.