Skip to content

Commit

Permalink
ACL: Allow users to query data for their groups (#4774)
Browse files Browse the repository at this point in the history
  • Loading branch information
animesh2049 authored Feb 14, 2020
1 parent 6e30c19 commit 0e022ca
Show file tree
Hide file tree
Showing 6 changed files with 271 additions and 8 deletions.
2 changes: 1 addition & 1 deletion edgraph/access.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func authorizeMutation(ctx context.Context, gmu *gql.Mutation) error {
return nil
}

func authorizeQuery(ctx context.Context, parsedReq *gql.Result) error {
func authorizeQuery(ctx context.Context, parsedReq *gql.Result, graphql bool) error {
// always allow access
return nil
}
Expand Down
141 changes: 140 additions & 1 deletion edgraph/access_ee.go
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@ func logAccess(log *accessEntry) {

//authorizeQuery authorizes the query using the aclCachePtr. It will silently drop all
// unauthorized predicates from query.
func authorizeQuery(ctx context.Context, parsedReq *gql.Result) error {
func authorizeQuery(ctx context.Context, parsedReq *gql.Result, graphql bool) error {
if len(worker.Config.HmacSecret) == 0 {
// the user has not turned on the acl feature
return nil
Expand Down Expand Up @@ -783,6 +783,20 @@ func authorizeQuery(ctx context.Context, parsedReq *gql.Result) error {
}

if len(blockedPreds) != 0 {
// 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.
if graphql {
for _, gq := range parsedReq.Query {
addUserFilterToQuery(gq, userId, groupIds)
}
// blockedPreds might have acl predicates which we want to allow access through
// graphql, so deleting those from here.
for _, pred := range x.AllACLPredicates() {
delete(blockedPreds, pred)
}
// In query context ~predicate and predicate are considered different.
delete(blockedPreds, "~dgraph.user.group")
}
parsedReq.Query = removePredsFromQuery(parsedReq.Query, blockedPreds)
}

Expand Down Expand Up @@ -819,6 +833,131 @@ func authorizeGroot(ctx context.Context) error {
return doAuthorizeGroot()
}

/*
addUserFilterToQuery applies makes sure that a user can access only its own
acl info by applying filter of userid and groupid to acl predicates. A query like
Conversion pattern:
* me(func: type(Group)) -> me(func: type(Group)) @filter(eq("dgraph.xid", groupIds...))
* me(func: type(User)) -> me(func: type(User)) @filter(eq("dgraph.xid", userId))
*/
func addUserFilterToQuery(gq *gql.GraphQuery, userId string, groupIds []string) {
if gq.Func != nil && gq.Func.Name == "type" {
// type function only supports one argument
if len(gq.Func.Args) != 1 {
return
}
arg := gq.Func.Args[0]
// The case where value of some varialble v (say) is "Group" and a
// query comes like `eq(dgraph.type, val(v))`, will be ingored here.
if arg.Value == "User" {
newFilter := userFilter(userId)
gq.Filter = parentFilter(newFilter, gq.Filter)
} else if arg.Value == "Group" {
newFilter := groupFilter(groupIds)
gq.Filter = parentFilter(newFilter, gq.Filter)
}
}

gq.Filter = addUserFilterToFilter(gq.Filter, userId, groupIds)

switch gq.Attr {
case "dgraph.user.group":
newFilter := groupFilter(groupIds)
gq.Filter = parentFilter(newFilter, gq.Filter)
case "~dgraph.user.group":
newFilter := userFilter(userId)
gq.Filter = parentFilter(newFilter, gq.Filter)
}

for _, ch := range gq.Children {
addUserFilterToQuery(ch, userId, groupIds)
}
}

func parentFilter(newFilter, filter *gql.FilterTree) *gql.FilterTree {
if filter == nil {
return newFilter
}
parentFilter := &gql.FilterTree{
Op: "AND",
Child: []*gql.FilterTree{filter, newFilter},
}
return parentFilter
}

func userFilter(userId string) *gql.FilterTree {
return &gql.FilterTree{
Func: &gql.Function{
Attr: "dgraph.xid",
Name: "eq",
Args: []gql.Arg{{Value: userId}},
},
}
}

func groupFilter(groupIds []string) *gql.FilterTree {
filter := &gql.FilterTree{
Func: &gql.Function{
Attr: "dgraph.xid",
Name: "eq",
},
}
for _, gid := range groupIds {
filter.Func.Args = append(filter.Func.Args,
gql.Arg{Value: gid})
}

return filter
}

/*
addUserFilterToFilter makes sure that user can't misue filters to access other user's info.
If the *filter* have type(Group) or type(User) functions, it generate a *newFilter* with function
like eq(dgraph.xid, userId) or eq(dgraph.xid, groupId...) and return a filter of the form
&gql.FilterTree{
Op: "AND",
Child: []gql.FilterTree{
{filter, newFilter}
}
}
*/
func addUserFilterToFilter(filter *gql.FilterTree, userId string,
groupIds []string) *gql.FilterTree {

if filter == nil {
return nil
}

if filter.Func != nil && filter.Func.Name == "type" {

// type function supports only one argument
if len(filter.Func.Args) != 1 {
return nil
}
arg := filter.Func.Args[0]
var newFilter *gql.FilterTree
switch arg.Value {
case "User":
newFilter = userFilter(userId)
case "Group":
newFilter = groupFilter(groupIds)
}

// If filter have function, it can't have children.
return parentFilter(newFilter, filter)
}

for idx, child := range filter.Child {
filter.Child[idx] = addUserFilterToFilter(child, userId, groupIds)
}

return filter
}

// removePredsFromQuery removes all the predicates in blockedPreds
// from all the queries in gqs.
func removePredsFromQuery(gqs []*gql.GraphQuery,
blockedPreds map[string]struct{}) []*gql.GraphQuery {

Expand Down
2 changes: 1 addition & 1 deletion edgraph/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,7 @@ func parseRequest(qc *queryContext) error {
}

func authorizeRequest(ctx context.Context, qc *queryContext) error {
if err := authorizeQuery(ctx, &qc.gqlRes); err != nil {
if err := authorizeQuery(ctx, &qc.gqlRes, qc.graphql); err != nil {
return err
}

Expand Down
116 changes: 116 additions & 0 deletions ee/acl/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1129,3 +1129,119 @@ func TestNonExistentGroup(t *testing.T) {
require.NoError(t, err, "login failed")
addRulesToGroup(t, accessJwt, devGroup, []rule{{"name", Read.Code}})
}

func TestQueryUserInfo(t *testing.T) {
ctx, _ := context.WithTimeout(context.Background(), 100*time.Second)

dg, err := testutil.DgraphClientWithGroot(testutil.SockAddr)
require.NoError(t, err)
addDataAndRules(ctx, t, dg)

accessJwt, _, err := testutil.HttpLogin(&testutil.LoginParams{
Endpoint: adminEndpoint,
UserID: userid,
Passwd: userpassword,
})
require.NoError(t, err, "login failed")

gqlQuery := `
query {
queryUser {
name
groups {
name
rules {
predicate
permission
}
users {
name
}
}
}
}
`

params := testutil.GraphQLParams{
Query: gqlQuery,
}
b := makeRequest(t, accessJwt, params)

testutil.CompareJSON(t, `
{
"data": {
"queryUser": [
{
"name": "alice",
"groups": [
{
"name": "dev",
"rules": [
{
"predicate": "name",
"permission": 4
},
{
"predicate": "nickname",
"permission": 2
}
],
"users": [
{
"name": "alice"
}
]
}
]
}
]
}
}`, string(b))

query := `
{
me(func: type(User)) {
dgraph.xid
dgraph.user.group {
dgraph.xid
dgraph.acl.rule {
dgraph.rule.predicate
dgraph.rule.permission
}
}
}
}
`

userClient, err := testutil.DgraphClient(testutil.SockAddr)
require.NoError(t, err)

err = userClient.Login(ctx, userid, userpassword)
require.NoError(t, err)

resp, err := userClient.NewReadOnlyTxn().Query(ctx, query)
require.NoError(t, err, "Error while querying ACL")

testutil.CompareJSON(t, `{"me":[]}`, string(resp.GetJson()))

gqlQuery = `
query {
getGroup(name: "guardians") {
name
rules {
predicate
permission
}
users {
name
}
}
}
`

params = testutil.GraphQLParams{
Query: gqlQuery,
}
b = makeRequest(t, accessJwt, params)
testutil.CompareJSON(t, `{"data": {"getGroup": null}}`, string(b))
}
8 changes: 4 additions & 4 deletions gql/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -1613,7 +1613,7 @@ func getValueArg(val string) (string, error) {
}

func validFuncName(name string) bool {
if isGeoFunc(name) || isInequalityFn(name) {
if isGeoFunc(name) || IsInequalityFn(name) {
return true
}

Expand Down Expand Up @@ -1714,7 +1714,7 @@ L:
return nil,
itemInFunc.Errorf("Multiple variables not allowed in len function")
}
if !isInequalityFn(function.Name) {
if !IsInequalityFn(function.Name) {
return nil,
itemInFunc.Errorf("len function only allowed inside inequality" +
" function")
Expand Down Expand Up @@ -1763,7 +1763,7 @@ L:
case isGeoFunc(function.Name):
err = parseGeoArgs(it, function)

case isInequalityFn(function.Name):
case IsInequalityFn(function.Name):
err = parseIneqArgs(it, function)

default:
Expand Down Expand Up @@ -3225,7 +3225,7 @@ func isGeoFunc(name string) bool {
return name == "near" || name == "contains" || name == "within" || name == "intersects"
}

func isInequalityFn(name string) bool {
func IsInequalityFn(name string) bool {
switch name {
case "eq", "le", "ge", "gt", "lt":
return true
Expand Down
10 changes: 9 additions & 1 deletion x/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -595,13 +595,21 @@ func IsAclPredicate(pred string) bool {
// ReservedPredicates returns the complete list of reserved predicates that needs to
// be expanded when * is given as a predicate.
func ReservedPredicates() []string {
var preds []string
preds := make([]string, 0, len(reservedPredicateMap))
for pred := range reservedPredicateMap {
preds = append(preds, pred)
}
return preds
}

func AllACLPredicates() []string {
preds := make([]string, 0, len(aclPredicateMap))
for pred := range aclPredicateMap {
preds = append(preds, pred)
}
return preds
}

// IsInternalPredicate returns true if the predicate is in the internal predicate list.
func IsInternalPredicate(pred string) bool {
_, ok := internalPredicateMap[strings.ToLower(pred)]
Expand Down

0 comments on commit 0e022ca

Please sign in to comment.