diff --git a/edgraph/access.go b/edgraph/access.go index 57ac1285597..c9630d858ef 100644 --- a/edgraph/access.go +++ b/edgraph/access.go @@ -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 } diff --git a/edgraph/access_ee.go b/edgraph/access_ee.go index 6d4e27aa951..70d6712abf2 100644 --- a/edgraph/access_ee.go +++ b/edgraph/access_ee.go @@ -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 @@ -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) } @@ -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 { diff --git a/edgraph/server.go b/edgraph/server.go index f8cb5f887c5..477845dfc3c 100644 --- a/edgraph/server.go +++ b/edgraph/server.go @@ -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 } diff --git a/ee/acl/acl_test.go b/ee/acl/acl_test.go index c0315d1998b..939e374a00c 100644 --- a/ee/acl/acl_test.go +++ b/ee/acl/acl_test.go @@ -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)) +} diff --git a/gql/parser.go b/gql/parser.go index b0ea967c514..c9b9f50596d 100644 --- a/gql/parser.go +++ b/gql/parser.go @@ -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 } @@ -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") @@ -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: @@ -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 diff --git a/x/keys.go b/x/keys.go index 3a58329df0c..614f00416ab 100644 --- a/x/keys.go +++ b/x/keys.go @@ -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)]