Skip to content

Commit

Permalink
fix(GraphQL): fix @auth rules evaluation in case of null variables in…
Browse files Browse the repository at this point in the history
… custom claims. (#7380)

Fixes GRAPHQL-988.

(cherry picked from commit f437724)
  • Loading branch information
minhaj-shakeel committed Feb 9, 2021
1 parent 161bae5 commit f1080f7
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 1 deletion.
44 changes: 44 additions & 0 deletions graphql/e2e/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,50 @@ func TestAuthOnInterfaces(t *testing.T) {
}
}

func TestAuthRulesWithNullValuesInJWT(t *testing.T) {
testCases := []TestCase{
{
name: "Query with null value in jwt",
query: `
query {
queryProject {
name
}
}
`,
result: `{"queryProject":[]}`,
},
{
name: "Query with null value in jwt: deep level",
query: `
query {
queryUser(order: {desc: username}, first: 1) {
username
issues {
msg
}
}
}
`,
role: "ADMIN",
result: `{"queryUser":[{"username":"user8","issues":[]}]}`,
},
}

for _, tcase := range testCases {
queryParams := &common.GraphQLParams{
Headers: common.GetJWTWithNullUser(t, tcase.role, metaInfo),
Query: tcase.query,
}
gqlResponse := queryParams.ExecuteAsPost(t, common.GraphqlURL)
common.RequireNoGQLErrors(t, gqlResponse)

if diff := cmp.Diff(tcase.result, string(gqlResponse.Data)); diff != "" {
t.Errorf("Test: %s result mismatch (-want +got):\n%s", tcase.name, diff)
}
}
}

func TestAuthOnInterfaceWithRBACPositive(t *testing.T) {
getVehicleParams := &common.GraphQLParams{
Query: `
Expand Down
12 changes: 12 additions & 0 deletions graphql/e2e/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,18 @@ func GetJWT(t *testing.T, user, role string, metaInfo *testutil.AuthMeta) http.H
return h
}

func GetJWTWithNullUser(t *testing.T, role interface{}, metaInfo *testutil.AuthMeta) http.Header {
metaInfo.AuthVars = map[string]interface{}{}
metaInfo.AuthVars["USER"] = nil
metaInfo.AuthVars["ROLE"] = role
require.NotNil(t, metaInfo.PrivateKeyPath)
jwtToken, err := metaInfo.GetSignedToken(metaInfo.PrivateKeyPath, 300*time.Second)
require.NoError(t, err)
h := make(http.Header)
h.Add(metaInfo.Header, jwtToken)
return h
}

func GetJWTForInterfaceAuth(t *testing.T, user, role string, ans bool, metaInfo *testutil.AuthMeta) http.Header {
metaInfo.AuthVars = map[string]interface{}{}
if user != "" {
Expand Down
37 changes: 37 additions & 0 deletions graphql/resolve/auth_query_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1021,6 +1021,20 @@
queryUserSecret()
}
- name: "Query with null variable - top level"
gqlquery: |
query {
queryUserSecret {
id
}
}
jwtVar:
USER: null
dgquery: |-
query {
queryUserSecret()
}
- name: "Get with top level RBAC false"
gqlquery: |
query {
Expand Down Expand Up @@ -1056,6 +1070,29 @@
User3 as var(func: type(User))
}
- name: "Query with null variable - deep query"
gqlquery: |
query {
queryUser {
username
tickets {
id
title
}
}
}
jwtvar:
USER: null
dgquery: |-
query {
queryUser(func: uid(UserRoot)) {
User.username : User.username
dgraph.uid : uid
}
UserRoot as var(func: uid(User3))
User3 as var(func: type(User))
}
- name: "Query with missing variable - partial jwt token"
gqlquery: |
query {
Expand Down
7 changes: 7 additions & 0 deletions graphql/resolve/query_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -1614,6 +1614,13 @@ func buildFilter(typ schema.Type, filter map[string]interface{}) *gql.FilterTree

fn, val := first(dgFunc)
if val == nil {
// If it is `eq` filter for eg: {filter: { title: {eq: null }}} then
// it will be interpreted as {filter: {not: {has: title}}}, rest of
// the filters with null values will be ignored in query rewriting.
if fn == "eq" {
hasFilterMap := map[string]interface{}{"not": map[string]interface{}{"has": []interface{}{field}}}
ands = append(ands, buildFilter(typ, hasFilterMap))
}
continue
}
args := []gql.Arg{{Value: typ.DgraphPredicate(field)}}
Expand Down
32 changes: 32 additions & 0 deletions graphql/resolve/query_test.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,35 @@
- name: "eq filter with null value get translated into NOT(has) filter"
gqlquery: |
query {
queryState(filter: {code: {eq: null}}) {
code
name
}
}
dgquery: |-
query {
queryState(func: type(State)) @filter(NOT (has(State.code))) {
State.code : State.code
State.name : State.name
dgraph.uid : uid
}
}
- name: "le filter with null value doesn't get translated"
gqlquery: |
query {
queryCountry(filter: {name: {le: null}}) {
name
}
}
dgquery: |-
query {
queryCountry(func: type(Country)) {
Country.name : Country.name
dgraph.uid : uid
}
}
-
name: "in filter on string type"
gqlquery: |
Expand Down
2 changes: 1 addition & 1 deletion graphql/schema/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (rq *RBACQuery) EvaluateRBACRule(av map[string]interface{}) RuleResult {

func (node *RuleNode) staticEvaluation(av map[string]interface{}) RuleResult {
for _, v := range node.Variables {
if _, ok := av[v.Variable]; !ok {
if val, ok := av[v.Variable]; !ok || val == nil {
return Negative
}
}
Expand Down

0 comments on commit f1080f7

Please sign in to comment.