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 cascade with auth query when RBAC is false (#6444) #6535

Merged
merged 2 commits into from
Sep 21, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
61 changes: 61 additions & 0 deletions graphql/e2e/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 {
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