Skip to content

Commit

Permalink
fix(Query) Fix Star_All delete query when used with ACL enabled (#6331)
Browse files Browse the repository at this point in the history
(cherry picked from commit 501f338)
  • Loading branch information
all-seeing-code committed Aug 31, 2020
1 parent 99ed46b commit 66bb89d
Show file tree
Hide file tree
Showing 5 changed files with 167 additions and 9 deletions.
9 changes: 6 additions & 3 deletions edgraph/access_ee.go
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,10 @@ func parsePredsFromMutation(nquads []*api.NQuad) []string {
// use a map to dedup predicates
predsMap := make(map[string]struct{})
for _, nquad := range nquads {
predsMap[nquad.Predicate] = struct{}{}
// _STAR_ALL is not a predicate in itself.
if nquad.Predicate != "_STAR_ALL" {
predsMap[nquad.Predicate] = struct{}{}
}
}

preds := make([]string, 0, len(predsMap))
Expand Down Expand Up @@ -658,7 +661,7 @@ func authorizeMutation(ctx context.Context, gmu *gql.Mutation) error {
return nil
}

blockedPreds, _ := authorizePreds(userId, groupIds, preds, acl.Write)
blockedPreds, allowedPreds := authorizePreds(userId, groupIds, preds, acl.Write)
if len(blockedPreds) > 0 {
var msg strings.Builder
for key := range blockedPreds {
Expand All @@ -668,7 +671,7 @@ func authorizeMutation(ctx context.Context, gmu *gql.Mutation) error {
return status.Errorf(codes.PermissionDenied,
"unauthorized to mutate following predicates: %s\n", msg.String())
}

gmu.AllowedPreds = allowedPreds
return nil
}

Expand Down
122 changes: 122 additions & 0 deletions ee/acl/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1255,6 +1255,128 @@ func TestExpandQueryWithACLPermissions(t *testing.T) {
testutil.CompareJSON(t, `{"me":[{"name":"RandomGuy","age":23, "nickname":"RG"},{"name":"RandomGuy2","age":25, "nickname":"RG2"}]}`,
string(resp.GetJson()))

}
func TestDeleteQueryWithACLPermissions(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Second)
defer cancel()
dg, err := testutil.DgraphClientWithGroot(testutil.SockAddr)
require.NoError(t, err)

testutil.DropAll(t, dg)

op := api.Operation{Schema: `
name : string @index(exact) .
nickname : string @index(exact) .
age : int .
type Person {
name: string
nickname: string
age: int
}
`}
require.NoError(t, dg.Alter(ctx, &op))

resetUser(t)

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

createGroup(t, accessJwt, devGroup)

addToGroup(t, accessJwt, userid, devGroup)

txn := dg.NewTxn()
mutation := &api.Mutation{
SetNquads: []byte(`
_:a <name> "RandomGuy" .
_:a <age> "23" .
_:a <nickname> "RG" .
_:a <dgraph.type> "Person" .
_:b <name> "RandomGuy2" .
_:b <age> "25" .
_:b <nickname> "RG2" .
_:b <dgraph.type> "Person" .
`),
CommitNow: true,
}
resp, err := txn.Mutate(ctx, mutation)
require.NoError(t, err)

nodeUID := resp.Uids["a"]
query := `{q1(func: type(Person)){
expand(_all_)
}}`

// Test that groot has access to all the predicates
resp, err = dg.NewReadOnlyTxn().Query(ctx, query)
require.NoError(t, err, "Error while querying data")
testutil.CompareJSON(t, `{"q1":[{"name":"RandomGuy","age":23, "nickname": "RG"},{"name":"RandomGuy2","age":25, "nickname": "RG2"}]}`,
string(resp.GetJson()))

// Give Write Access to alice for name and age predicate
addRulesToGroup(t, accessJwt, devGroup, []rule{{"name", Write.Code}, {"age", Write.Code}})

userClient, err := testutil.DgraphClient(testutil.SockAddr)
require.NoError(t, err)
time.Sleep(6 * time.Second)

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

// delete S * * (user now has permission to name and age)
txn = userClient.NewTxn()
mutString := fmt.Sprintf("<%s> * * .", nodeUID)
mutation = &api.Mutation{
DelNquads: []byte(mutString),
CommitNow: true,
}
_, err = txn.Mutate(ctx, mutation)
require.NoError(t, err)

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

resp, err = dg.NewReadOnlyTxn().Query(ctx, query)
require.NoError(t, err, "Error while querying data")
// Only name and age predicates got deleted via user - alice
testutil.CompareJSON(t, `{"q1":[{"nickname": "RG"},{"name":"RandomGuy2","age":25, "nickname": "RG2"}]}`,
string(resp.GetJson()))

// Give write access of <name> <dgraph.type> to dev
addRulesToGroup(t, accessJwt, devGroup, []rule{{"name", Write.Code}, {"age", Write.Code}, {"dgraph.type", Write.Code}})
time.Sleep(6 * time.Second)

// delete S * * (user now has permission to name, age and dgraph.type)
txn = userClient.NewTxn()
mutString = fmt.Sprintf("<%s> * * .", nodeUID)
mutation = &api.Mutation{
DelNquads: []byte(mutString),
CommitNow: true,
}
_, err = txn.Mutate(ctx, mutation)
require.NoError(t, err)

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

resp, err = dg.NewReadOnlyTxn().Query(ctx, query)
require.NoError(t, err, "Error while querying data")
// Because alise had permission to dgraph.type the node reference has been deleted
testutil.CompareJSON(t, `{"q1":[{"name":"RandomGuy2","age":25, "nickname": "RG2"}]}`,
string(resp.GetJson()))

}

func TestValQueryWithACLPermissions(t *testing.T) {
Expand Down
7 changes: 4 additions & 3 deletions gql/mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ var (

// Mutation stores the strings corresponding to set and delete operations.
type Mutation struct {
Cond string
Set []*api.NQuad
Del []*api.NQuad
Cond string
Set []*api.NQuad
Del []*api.NQuad
AllowedPreds []string

Metadata *pb.Metadata
}
Expand Down
8 changes: 5 additions & 3 deletions protos/pb/pb.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 30 additions & 0 deletions query/mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,22 @@ func expandEdges(ctx context.Context, m *pb.Mutations) ([]*pb.DirectedEdge, erro
}
preds = append(preds, getPredicatesFromTypes(types)...)
preds = append(preds, x.StarAllPredicates()...)
// AllowedPreds are used only with ACL. Do not delete all predicates but
// delete predicates to which the mutation has access
if edge.AllowedPreds != nil {
// Take intersection of preds and AllowedPreds
intersectPreds := make([]string, 0)
hashMap := make(map[string]bool)
for _, allowedPred := range edge.AllowedPreds {
hashMap[allowedPred] = true
}
for _, pred := range preds {
if _, found := hashMap[pred]; found {
intersectPreds = append(intersectPreds, pred)
}
}
preds = intersectPreds
}
}

for _, pred := range preds {
Expand Down Expand Up @@ -192,6 +208,20 @@ func ToDirectedEdges(gmuList []*gql.Mutation, newUids map[string]uint64) (
}

for _, gmu := range gmuList {
// We delete first and then we set. Order of the mutation is important.
for _, nq := range gmu.Del {
if nq.Subject == x.Star && nq.ObjectValue.GetDefaultVal() == x.Star {
return edges, errors.New("Predicate deletion should be called via alter")
}
if err := parse(nq, pb.DirectedEdge_DEL); err != nil {
return edges, err
}
if gmu.AllowedPreds != nil {
for _, e := range edges {
e.AllowedPreds = gmu.AllowedPreds
}
}
}
for _, nq := range gmu.Set {
if err := facets.SortAndValidate(nq.Facets); err != nil {
return edges, err
Expand Down

0 comments on commit 66bb89d

Please sign in to comment.