ACL: Allow users to query data for their groups, username, and permissions#4774
ACL: Allow users to query data for their groups, username, and permissions#4774animesh2049 merged 11 commits intomasterfrom
Conversation
addUserFilter should only work for graphql queries. Also non graphql query in test to make sure it returns empty result for acl queryies.
pawanrawal
left a comment
There was a problem hiding this comment.
after the current changes are made and the tests pass
Reviewed 1 of 3 files at r1, 1 of 3 files at r2, 4 of 4 files at r3.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @animesh2049 and @manishrjain)
edgraph/access_ee.go, line 789 at r1 (raw file):
addUserFilterToQuery(gq, userId, groupIds) } for _, pred := range x.AllACLPredicates() {
Add a comment here that the query could have ACL predicates which could be part of the blocked preds and we want to allow access to them, hence we delete them from the blocked preds map.
edgraph/access_ee.go, line 786 at r3 (raw file):
if len(blockedPreds) != 0 { if graphql {
Add a comment saying
For GraphQL requests, we allow filtered access to the ACL predicates. Filter for user_id and group_id is applied for the currently logged in user.
edgraph/access_ee.go, line 833 at r3 (raw file):
// addUserFilterToQuery applies makes sure that a user can access only its own // acl info by applying filter of userid and groupid to acl predicates
me(func: type(Group))
me(func: type(Group)) @filter(eq(dgraph.xid, ["group1", "group2",..]))
similarly for User.
edgraph/access_ee.go, line 857 at r3 (raw file):
case "dgraph.user.group": newFilter := groupFilter(groupIds) gq.Filter = parentFilter(newFilter, gq.Filter)
No need to return the pointer here, the function can just update gq.Filter.
edgraph/access_ee.go, line 860 at r3 (raw file):
case "~dgraph.user.group": newFilter := userFilter(userId) gq.Filter = parentFilter(newFilter, gq.Filter)
same here
edgraph/access_ee.go, line 916 at r3 (raw file):
} */ func addUserFilterToFilter(filter *gql.FilterTree, userId string,
We can update filter in place
| Op: "AND", | ||
| Child: []*gql.FilterTree{filter, newFilter}, | ||
| } | ||
| filter = parentFilter |
There was a problem hiding this comment.
ineffectual assignment to filter (from ineffassign)
Reverting this commit because updating pointers inside function doesn't look a neat way. Method used earlier seems more appropriate. This reverts commit 5b16b15.
animesh2049
left a comment
There was a problem hiding this comment.
Reviewable status: 5 of 6 files reviewed, 9 unresolved discussions (waiting on @filter, @golangcibot, @manishrjain, and @pawanrawal)
edgraph/access_ee.go, line 789 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Add a comment here that the query could have ACL predicates which could be part of the blocked preds and we want to allow access to them, hence we delete them from the blocked preds map.
Done.
edgraph/access_ee.go, line 852 at r1 (raw file):
Previously, golangcibot (Bot from GolangCI) wrote…
File is not
gofmt-ed with-s(fromgofmt){Value: userId},
Done.
edgraph/access_ee.go, line 849 at r2 (raw file):
Previously, golangcibot (Bot from GolangCI) wrote…
File is not
gofmt-ed with-s(fromgofmt)Args: []gql.Arg{{Value: userId}},
Done.
edgraph/access_ee.go, line 786 at r3 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Add a comment saying
For GraphQL requests, we allow filtered access to the ACL predicates. Filter for user_id and group_id is applied for the currently logged in user.
Done.
edgraph/access_ee.go, line 833 at r3 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
me(func: type(Group))
me(func: type(Group)) @filter(eq(dgraph.xid, ["group1", "group2",..]))
similarly for User.
Done.
edgraph/access_ee.go, line 857 at r3 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
No need to return the pointer here, the function can just update
gq.Filter.
for that I will have to do something like this
updateFilter(newFilter, &gq.Filter)
func updateFilter(newFilter *gql.Filter, filter **gql.Filter) {
if **filter == nil {
*filter = newFilter
...
The current version seems more appropriate to me, so leaving reverted my commit.
edgraph/access_ee.go, line 860 at r3 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
same here
Same as above.
edgraph/access_ee.go, line 916 at r3 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
We can update filter in place
Same as above.
edgraph/access_ee.go, line 886 at r4 (raw file):
Previously, golangcibot (Bot from GolangCI) wrote…
ineffectual assignment to
filter(fromineffassign)
Done.
This PR enables users to query their groups, username, permissions. Earlier only
guardianscould do any of that.This change is
Docs Preview: