Skip to content

Commit 8c518d4

Browse files
author
Arijit Das
authored
fix(GraphQL): Fix cascade with auth query when RBAC is false (#6444)
* Fix cascade with auth query. * Added additional E2E test.
1 parent 5aca255 commit 8c518d4

File tree

3 files changed

+151
-5
lines changed

3 files changed

+151
-5
lines changed

graphql/e2e/auth/auth_test.go

+61
Original file line numberDiff line numberDiff line change
@@ -1240,6 +1240,67 @@ func TestDeleteDeepAuthRule(t *testing.T) {
12401240
}
12411241
}
12421242

1243+
func TestDeepRBACValueCascade(t *testing.T) {
1244+
testCases := []TestCase{
1245+
{
1246+
user: "user1",
1247+
role: "USER",
1248+
query: `
1249+
query {
1250+
queryUser (filter:{username:{eq:"user1"}}) @cascade {
1251+
username
1252+
issues {
1253+
msg
1254+
}
1255+
}
1256+
}`,
1257+
result: `{"queryUser": []}`,
1258+
},
1259+
{
1260+
user: "user1",
1261+
role: "USER",
1262+
query: `
1263+
query {
1264+
queryUser (filter:{username:{eq:"user1"}}) {
1265+
username
1266+
issues @cascade {
1267+
msg
1268+
}
1269+
}
1270+
}`,
1271+
result: `{"queryUser": [{"username": "user1", "issues":[]}]}`,
1272+
},
1273+
{
1274+
user: "user1",
1275+
role: "ADMIN",
1276+
query: `
1277+
query {
1278+
queryUser (filter:{username:{eq:"user1"}}) @cascade {
1279+
username
1280+
issues {
1281+
msg
1282+
}
1283+
}
1284+
}`,
1285+
result: `{"queryUser":[{"username":"user1","issues":[{"msg":"Issue1"}]}]}`,
1286+
},
1287+
}
1288+
1289+
for _, tcase := range testCases {
1290+
t.Run(tcase.role+tcase.user, func(t *testing.T) {
1291+
getUserParams := &common.GraphQLParams{
1292+
Headers: common.GetJWT(t, tcase.user, tcase.role, metaInfo),
1293+
Query: tcase.query,
1294+
}
1295+
1296+
gqlResponse := getUserParams.ExecuteAsPost(t, graphqlURL)
1297+
require.Nil(t, gqlResponse.Errors)
1298+
1299+
require.JSONEq(t, string(gqlResponse.Data), tcase.result)
1300+
})
1301+
}
1302+
}
1303+
12431304
func TestMain(m *testing.M) {
12441305
schemaFile := "schema.graphql"
12451306
schema, err := ioutil.ReadFile(schemaFile)

graphql/resolve/auth_query_test.yaml

+49
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,55 @@
101101
Contact5 as var(func: type(Contact))
102102
}
103103
104+
- name: "Deep RBAC rule with cascade - Level 1 false"
105+
gqlquery: |
106+
query {
107+
queryContact @cascade {
108+
id
109+
nickName
110+
adminTasks {
111+
id
112+
name
113+
occurrences {
114+
due
115+
comp
116+
}
117+
}
118+
}
119+
}
120+
jwtvar:
121+
ContactRole: ADMINISTRATOR
122+
TaskRole: User
123+
TaskOccuranceRole: ADMINISTRATOR
124+
dgquery: |-
125+
query {
126+
queryContact(func: uid(ContactRoot)) @cascade {
127+
id : uid
128+
nickName : Contact.nickName
129+
adminTasks : Contact.adminTasks @filter(uid(AdminTask6)) {
130+
id : uid
131+
name : AdminTask.name
132+
occurrences : AdminTask.occurrences @filter(uid(TaskOccurrence4)) {
133+
due : TaskOccurrence.due
134+
comp : TaskOccurrence.comp
135+
dgraph.uid : uid
136+
}
137+
}
138+
}
139+
ContactRoot as var(func: uid(Contact7))
140+
Contact7 as var(func: type(Contact))
141+
var(func: uid(ContactRoot)) {
142+
AdminTask1 as Contact.adminTasks
143+
}
144+
AdminTask6 as var(func: uid(AdminTask1)) @filter(uid(AdminTask5))
145+
var(func: uid(AdminTask1)) {
146+
TaskOccurrence2 as AdminTask.occurrences
147+
}
148+
TaskOccurrence4 as var(func: uid(TaskOccurrence2)) @filter(uid(TaskOccurrenceAuth3))
149+
TaskOccurrenceAuth3 as var(func: uid(TaskOccurrence2)) @filter(eq(TaskOccurrence.role, "ADMINISTRATOR")) @cascade
150+
AdminTask5 as var(func: uid())
151+
}
152+
104153
- name: "Deep RBAC rule - Level 2 false"
105154
gqlquery: |
106155
query {

graphql/resolve/query_rewriter.go

+41-5
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ type authRewriter struct {
4646
parentVarName string
4747
// `hasAuthRules` indicates if any of fields in the complete query hierarchy has auth rules.
4848
hasAuthRules bool
49+
// `hasCascade` indicates if any of fields in the complete query hierarchy has cascade directive.
50+
hasCascade bool
4951
}
5052

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

72+
func hasCascadeDirective(field schema.Field) bool {
73+
if c := field.Cascade(); c != nil {
74+
return true
75+
}
76+
77+
for _, childField := range field.SelectionSet() {
78+
if res := hasCascadeDirective(childField); res {
79+
return true
80+
}
81+
}
82+
return false
83+
}
84+
7085
// Rewrite rewrites a GraphQL query into a Dgraph GraphQuery.
7186
func (qr *queryRewriter) Rewrite(
7287
ctx context.Context,
@@ -93,6 +108,7 @@ func (qr *queryRewriter) Rewrite(
93108
parentVarName: gqlQuery.Type().Name() + "Root",
94109
}
95110
authRw.hasAuthRules = hasAuthRules(gqlQuery, authRw)
111+
authRw.hasCascade = hasCascadeDirective(gqlQuery)
96112

97113
switch gqlQuery.QueryType() {
98114
case schema.GetQuery:
@@ -748,14 +764,34 @@ func addSelectionSetFrom(
748764
q.Children = append(q.Children, child)
749765
}
750766

751-
// If RBAC rules are evaluated to Negative, we don't write queries for deeper levels.
752-
// Hence we don't need to do any further processing for this field.
753-
if rbac == schema.Negative {
767+
var fieldAuth []*gql.GraphQuery
768+
var authFilter *gql.FilterTree
769+
if rbac == schema.Negative && auth.hasAuthRules && auth.hasCascade && !auth.isWritingAuth {
770+
// If RBAC rules are evaluated to Negative but we have cascade directive we continue
771+
// to write the query and add a dummy filter that doesn't return anything.
772+
// Example: AdminTask5 as var(func: uid())
773+
q.Children = append(q.Children, child)
774+
varName := auth.varGen.Next(f.Type(), "", "", auth.isWritingAuth)
775+
fieldAuth = append(fieldAuth, &gql.GraphQuery{
776+
Var: varName,
777+
Attr: "var",
778+
Func: &gql.Function{
779+
Name: "uid",
780+
},
781+
})
782+
authFilter = &gql.FilterTree{
783+
Func: &gql.Function{
784+
Name: "uid",
785+
Args: []gql.Arg{{Value: varName}},
786+
},
787+
}
788+
rbac = schema.Positive
789+
} else if rbac == schema.Negative {
790+
// If RBAC rules are evaluated to Negative, we don't write queries for deeper levels.
791+
// Hence we don't need to do any further processing for this field.
754792
continue
755793
}
756794

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

0 commit comments

Comments
 (0)