Skip to content

Commit

Permalink
fix(GraphQL): Fix cascade with auth query when RBAC is false (#6444)
Browse files Browse the repository at this point in the history
* Fix cascade with auth query.

* Added additional E2E test.
  • Loading branch information
Arijit Das committed Sep 16, 2020
1 parent 5aca255 commit 8c518d4
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 5 deletions.
61 changes: 61 additions & 0 deletions graphql/e2e/auth/auth_test.go
Expand Up @@ -1240,6 +1240,67 @@ func TestDeleteDeepAuthRule(t *testing.T) {
}
}

func TestDeepRBACValueCascade(t *testing.T) {
testCases := []TestCase{
{
user: "user1",
role: "USER",
query: `
query {
queryUser (filter:{username:{eq:"user1"}}) @cascade {
username
issues {
msg
}
}
}`,
result: `{"queryUser": []}`,
},
{
user: "user1",
role: "USER",
query: `
query {
queryUser (filter:{username:{eq:"user1"}}) {
username
issues @cascade {
msg
}
}
}`,
result: `{"queryUser": [{"username": "user1", "issues":[]}]}`,
},
{
user: "user1",
role: "ADMIN",
query: `
query {
queryUser (filter:{username:{eq:"user1"}}) @cascade {
username
issues {
msg
}
}
}`,
result: `{"queryUser":[{"username":"user1","issues":[{"msg":"Issue1"}]}]}`,
},
}

for _, tcase := range testCases {
t.Run(tcase.role+tcase.user, func(t *testing.T) {
getUserParams := &common.GraphQLParams{
Headers: common.GetJWT(t, tcase.user, tcase.role, metaInfo),
Query: tcase.query,
}

gqlResponse := getUserParams.ExecuteAsPost(t, graphqlURL)
require.Nil(t, gqlResponse.Errors)

require.JSONEq(t, string(gqlResponse.Data), tcase.result)
})
}
}

func TestMain(m *testing.M) {
schemaFile := "schema.graphql"
schema, err := ioutil.ReadFile(schemaFile)
Expand Down
49 changes: 49 additions & 0 deletions graphql/resolve/auth_query_test.yaml
Expand Up @@ -101,6 +101,55 @@
Contact5 as var(func: type(Contact))
}
- name: "Deep RBAC rule with cascade - Level 1 false"
gqlquery: |
query {
queryContact @cascade {
id
nickName
adminTasks {
id
name
occurrences {
due
comp
}
}
}
}
jwtvar:
ContactRole: ADMINISTRATOR
TaskRole: User
TaskOccuranceRole: ADMINISTRATOR
dgquery: |-
query {
queryContact(func: uid(ContactRoot)) @cascade {
id : uid
nickName : Contact.nickName
adminTasks : Contact.adminTasks @filter(uid(AdminTask6)) {
id : uid
name : AdminTask.name
occurrences : AdminTask.occurrences @filter(uid(TaskOccurrence4)) {
due : TaskOccurrence.due
comp : TaskOccurrence.comp
dgraph.uid : uid
}
}
}
ContactRoot as var(func: uid(Contact7))
Contact7 as var(func: type(Contact))
var(func: uid(ContactRoot)) {
AdminTask1 as Contact.adminTasks
}
AdminTask6 as var(func: uid(AdminTask1)) @filter(uid(AdminTask5))
var(func: uid(AdminTask1)) {
TaskOccurrence2 as AdminTask.occurrences
}
TaskOccurrence4 as var(func: uid(TaskOccurrence2)) @filter(uid(TaskOccurrenceAuth3))
TaskOccurrenceAuth3 as var(func: uid(TaskOccurrence2)) @filter(eq(TaskOccurrence.role, "ADMINISTRATOR")) @cascade
AdminTask5 as var(func: uid())
}
- name: "Deep RBAC rule - Level 2 false"
gqlquery: |
query {
Expand Down
46 changes: 41 additions & 5 deletions graphql/resolve/query_rewriter.go
Expand Up @@ -46,6 +46,8 @@ type authRewriter struct {
parentVarName string
// `hasAuthRules` indicates if any of fields in the complete query hierarchy has auth rules.
hasAuthRules bool
// `hasCascade` indicates if any of fields in the complete query hierarchy has cascade directive.
hasCascade bool
}

// NewQueryRewriter returns a new QueryRewriter.
Expand All @@ -67,6 +69,19 @@ func hasAuthRules(field schema.Field, authRw *authRewriter) bool {
return false
}

func hasCascadeDirective(field schema.Field) bool {
if c := field.Cascade(); c != nil {
return true
}

for _, childField := range field.SelectionSet() {
if res := hasCascadeDirective(childField); res {
return true
}
}
return false
}

// Rewrite rewrites a GraphQL query into a Dgraph GraphQuery.
func (qr *queryRewriter) Rewrite(
ctx context.Context,
Expand All @@ -93,6 +108,7 @@ func (qr *queryRewriter) Rewrite(
parentVarName: gqlQuery.Type().Name() + "Root",
}
authRw.hasAuthRules = hasAuthRules(gqlQuery, authRw)
authRw.hasCascade = hasCascadeDirective(gqlQuery)

switch gqlQuery.QueryType() {
case schema.GetQuery:
Expand Down Expand Up @@ -748,14 +764,34 @@ func addSelectionSetFrom(
q.Children = append(q.Children, child)
}

// If RBAC rules are evaluated to Negative, we don't write queries for deeper levels.
// Hence we don't need to do any further processing for this field.
if rbac == schema.Negative {
var fieldAuth []*gql.GraphQuery
var authFilter *gql.FilterTree
if rbac == schema.Negative && auth.hasAuthRules && auth.hasCascade && !auth.isWritingAuth {
// If RBAC rules are evaluated to Negative but we have cascade directive we continue
// to write the query and add a dummy filter that doesn't return anything.
// Example: AdminTask5 as var(func: uid())
q.Children = append(q.Children, child)
varName := auth.varGen.Next(f.Type(), "", "", auth.isWritingAuth)
fieldAuth = append(fieldAuth, &gql.GraphQuery{
Var: varName,
Attr: "var",
Func: &gql.Function{
Name: "uid",
},
})
authFilter = &gql.FilterTree{
Func: &gql.Function{
Name: "uid",
Args: []gql.Arg{{Value: varName}},
},
}
rbac = schema.Positive
} else if rbac == schema.Negative {
// If RBAC rules are evaluated to Negative, we don't write queries for deeper levels.
// Hence we don't need to do any further processing for this field.
continue
}

var fieldAuth []*gql.GraphQuery
var authFilter *gql.FilterTree
// If RBAC rules are evaluated to `Uncertain` then we add the Auth rules.
if rbac == schema.Uncertain {
fieldAuth, authFilter = auth.rewriteAuthQueries(f.Type())
Expand Down

0 comments on commit 8c518d4

Please sign in to comment.