Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(GraphQL): fix @auth rules evaluation in case of null variables in custom claims. #7380

Merged
merged 20 commits into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions graphql/e2e/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,31 @@ func TestAuthOnInterfaces(t *testing.T) {
})
}
}

func TestAuthRulesWithNullValuesInJWT(t *testing.T) {
tcase := TestCase{
name: "Query with null value in jwt",
query: `
query {
queryProject {
name
}
}
`,
result: `{"queryProject":[]}`,
}

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 TestAuthRulesWithMissingJWT(t *testing.T) {
testCases := []TestCase{
{name: "Query non auth field without JWT Token",
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 @@ -1196,6 +1196,18 @@ func GetJWT(t *testing.T, user, role interface{}, metaInfo *testutil.AuthMeta) 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)) {
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 @@ -1747,6 +1747,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": 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))) {
code : State.code
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)) {
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 @@ -131,7 +131,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