[BREAKING] ACL schema change

dgraph/cmd/bulk/systest/test-bulk-schema.sh

@@ -191,9 +191,11 @@ EOF
   dgraph debug -p out/0/p 2>|/dev/null | grep '{s}' | cut -d' ' -f4  > all_dbs.out
   dgraph debug -p out/1/p 2>|/dev/null | grep '{s}' | cut -d' ' -f4 >> all_dbs.out
   diff <(LC_ALL=C sort all_dbs.out | uniq -c) - <<EOF
+      1 dgraph.acl.rule
       1 dgraph.graphql.schema
-      1 dgraph.group.acl
       1 dgraph.password
+      1 dgraph.rule.permission
+      1 dgraph.rule.predicate
       1 dgraph.type
       1 dgraph.user.group
       1 dgraph.xid

edgraph/access_ee.go

@@ -345,9 +345,12 @@ func RefreshAcls(closer *y.Closer) {
 
 const queryAcls = `
 {
-  allAcls(func: has(dgraph.group.acl)) {
+  allAcls(func: type(Group)) {
     dgraph.xid
-    dgraph.group.acl
+	dgraph.acl.rule {
+		dgraph.rule.predicate
+		dgraph.rule.permission
+	}
   }
 }
 `

edgraph/acl_cache.go

@@ -13,12 +13,10 @@
 package edgraph
 
 import (
-	"encoding/json"
 	"sync"
 
 	"github.com/dgraph-io/dgraph/ee/acl"
 	"github.com/dgraph-io/dgraph/x"
-	"github.com/golang/glog"
 	"github.com/pkg/errors"
 )
 
@@ -51,12 +49,7 @@ func (cache *aclCache) update(groups []acl.Group) {
 	// predicate to a submap, and the submap maps a group to a permission
 	predPerms := make(map[string]map[string]int32)
 	for _, group := range groups {
-		aclBytes := []byte(group.Acls)
-		var acls []acl.Acl
-		if err := json.Unmarshal(aclBytes, &acls); err != nil {
-			glog.Errorf("Unable to unmarshal the aclBytes: %v", err)
-			continue
-		}
+		acls := group.Rules
 
 		for _, acl := range acls {
 			if len(acl.Predicate) > 0 {

edgraph/acl_cache_test.go

@@ -13,7 +13,6 @@
 package edgraph
 
 import (
-	"encoding/json"
 	"testing"
 
 	"github.com/dgraph-io/dgraph/ee/acl"
@@ -37,11 +36,10 @@ func TestAclCache(t *testing.T) {
 			Perm:      4,
 		},
 	}
-	aclBytes, _ := json.Marshal(acls)
 	groups := []acl.Group{
 		{
 			GroupID: group,
-			Acls:    string(aclBytes),
+			Rules:   acls,
 		},
 	}
 	aclCachePtr.update(groups)

ee/acl/acl.go

@@ -14,7 +14,6 @@ package acl
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"strings"
 	"time"
@@ -406,7 +405,7 @@ func chMod(conf *viper.Viper) error {
 		return errors.Errorf("the groupid must not be empty")
 	case len(predicate) == 0:
 		return errors.Errorf("no predicates specified")
-	case perm > 7:
+	case perm > 7: // TODO(Animesh): also check if perm < 0
 		return errors.Errorf("the perm value must be less than or equal to 7, "+
 			"the provided value is %d", perm)
 	}
@@ -426,59 +425,69 @@ func chMod(conf *viper.Viper) error {
 		}
 	}()
 
-	group, err := queryGroup(ctx, txn, groupId, "dgraph.group.acl")
-	if err != nil {
-		return errors.Wrapf(err, "while querying group")
-	}
-	if group == nil || len(group.Uid) == 0 {
-		return errors.Errorf("unable to change permission for group because it does not exist: %v",
-			groupId)
-	}
-
-	var currentAcls []Acl
-	if len(group.Acls) != 0 {
-		if err := json.Unmarshal([]byte(group.Acls), &currentAcls); err != nil {
-			return errors.Wrapf(err, "unable to unmarshal the acls associated with the group %v",
-				groupId)
+	ruleQuery := fmt.Sprintf(`
+	{
+		var(func: eq(dgraph.xid, "%s")) @filter(type(Group)) {
+			gUID as uid
+			rUID as dgraph.acl.rule @filter(eq(dgraph.rule.predicate, "%s"))
 		}
-	}
-
-	var newAcl Acl
-	if len(predicate) > 0 {
-		newAcl = Acl{
-			Predicate: predicate,
-			Perm:      int32(perm),
-		}
-	}
-	newAcls, updated := updateAcl(currentAcls, newAcl)
-	if !updated {
-		fmt.Printf("Nothing needs to be changed for the permission of group: %v\n", groupId)
-		return nil
-	}
-
-	newAclBytes, err := json.Marshal(newAcls)
-	if err != nil {
-		return errors.Wrapf(err, "unable to marshal the updated acls")
-	}
-
-	chModNQuads := &api.NQuad{
-		Subject:     group.Uid,
-		Predicate:   "dgraph.group.acl",
-		ObjectValue: &api.Value{Val: &api.Value_BytesVal{BytesVal: newAclBytes}},
-	}
-	mu := &api.Mutation{
+		groupUIDCount(func: uid(gUID)) {count(uid)}
+	}`, groupId, predicate)
+
+	updateRule := &api.Mutation{
+		Set: []*api.NQuad{
+			{
+				Subject:     "uid(rUID)",
+				Predicate:   "dgraph.rule.permission",
+				ObjectValue: &api.Value{Val: &api.Value_IntVal{IntVal: int64(perm)}},
+			},
+		},
+		Cond: "@if(eq(len(rUID), 1) AND eq(len(gUID), 1))",
+	}
+
+	createRule := &api.Mutation{
+		Set: []*api.NQuad{
+			{
+				Subject:     "_:newrule",
+				Predicate:   "dgraph.rule.permission",
+				ObjectValue: &api.Value{Val: &api.Value_IntVal{IntVal: int64(perm)}},
+			},
+			{
+				Subject:     "_:newrule",
+				Predicate:   "dgraph.rule.predicate",
+				ObjectValue: &api.Value{Val: &api.Value_StrVal{StrVal: predicate}},
+			},
+			{
+				Subject:   "uid(gUID)",
+				Predicate: "dgraph.acl.rule",
+				ObjectId:  "_:newrule",
+			},
+		},
+		Cond: "@if(eq(len(rUID), 0) AND eq(len(gUID), 1))",
+	}
+
+	deleteRule := &api.Mutation{
+		Del: []*api.NQuad{
+			{
+				Subject:   "uid(gUID)",
+				Predicate: "dgraph.acl.rule",
+				ObjectId:  "uid(rUID)",
+			},
+		},
+		Cond: "@if(eq(len(rUID), 1) AND eq(len(gUID), 1))",
+	}
+
+	upsertRequest := &api.Request{
+		Query:     ruleQuery,
+		Mutations: []*api.Mutation{createRule, updateRule},
 		CommitNow: true,
-		Set:       []*api.NQuad{chModNQuads},
 	}
-
-	if _, err = txn.Mutate(ctx, mu); err != nil {
-		return errors.Wrapf(err, "unable to change mutations for the group %v on predicate %v",
-			groupId, predicate)
+	if perm < 0 {
+		upsertRequest.Mutations = []*api.Mutation{deleteRule}
 	}
-	fmt.Printf("Successfully changed permission for group %v on predicate %v to %v\n",
-		groupId, predicate, perm)
-	fmt.Println("The latest info is:")
-	return queryAndPrintGroup(ctx, dc.NewReadOnlyTxn(), groupId)
+	resp, err := txn.Do(ctx, upsertRequest)
+	fmt.Println(string(resp.GetJson()))
+	return err
 }
 
 func queryUser(ctx context.Context, txn *dgo.Txn, userid string) (user *User, err error) {
@@ -531,10 +540,13 @@ func queryGroup(ctx context.Context, txn *dgo.Txn, groupid string,
 	fields ...string) (group *Group, err error) {
 
 	// write query header
-	query := fmt.Sprintf(`query search($groupid: string){
-        group(func: eq(dgraph.xid, $groupid)) @filter(type(Group)) {
-			uid
-		    %s }}`, strings.Join(fields, ", "))
+	query := fmt.Sprintf(`
+		query search($groupid: string){
+			group(func: eq(dgraph.xid, $groupid)) @filter(type(Group)) {
+				uid
+				%s
+			}
+		}`, strings.Join(fields, ", "))
 
 	queryVars := map[string]string{
 		"$groupid": groupid,
@@ -552,33 +564,6 @@ func queryGroup(ctx context.Context, txn *dgo.Txn, groupid string,
 	return group, nil
 }
 
-func isSameAcl(acl1 *Acl, acl2 *Acl) bool {
-	return (len(acl1.Predicate) > 0 && len(acl2.Predicate) > 0 &&
-		acl1.Predicate == acl2.Predicate)
-}
-
-// returns whether the existing acls slice is changed
-func updateAcl(acls []Acl, newAcl Acl) ([]Acl, bool) {
-	for idx, aclEntry := range acls {
-		if isSameAcl(&aclEntry, &newAcl) {
-			if aclEntry.Perm == newAcl.Perm {
-				// new permission is the same as the current one, no update
-				return acls, false
-			}
-			if newAcl.Perm < 0 {
-				// remove the current aclEntry from the array
-				copy(acls[idx:], acls[idx+1:])
-				return acls[:len(acls)-1], true
-			}
-			acls[idx].Perm = newAcl.Perm
-			return acls, true
-		}
-	}
-
-	// we do not find any existing aclEntry matching the newAcl predicate
-	return append(acls, newAcl), true
-}
-
 func queryAndPrintUser(ctx context.Context, txn *dgo.Txn, userId string) error {
 	user, err := queryUser(ctx, txn, userId)
 	if err != nil {
@@ -598,13 +583,14 @@ func queryAndPrintUser(ctx context.Context, txn *dgo.Txn, userId string) error {
 
 func queryAndPrintGroup(ctx context.Context, txn *dgo.Txn, groupId string) error {
 	group, err := queryGroup(ctx, txn, groupId, "dgraph.xid", "~dgraph.user.group{dgraph.xid}",
-		"dgraph.group.acl")
+		"dgraph.acl.rule{dgraph.rule.predicate, dgraph.rule.permission}")
 	if err != nil {
 		return err
 	}
 	if group == nil {
-		return errors.Errorf("The group %q does not exist.\n", groupId)
+		return errors.Errorf("The group %s doesn't exist", groupId)
 	}
+
 	fmt.Printf("Group: %s\n", groupId)
 	fmt.Printf("UID  : %s\n", group.Uid)
 	fmt.Printf("ID   : %s\n", group.GroupID)
@@ -615,17 +601,10 @@ func queryAndPrintGroup(ctx context.Context, txn *dgo.Txn, groupId string) error
 	}
 	fmt.Printf("Users: %s\n", strings.Join(userNames, " "))
 
-	var acls []Acl
-	if len(group.Acls) != 0 {
-		if err := json.Unmarshal([]byte(group.Acls), &acls); err != nil {
-			return errors.Wrapf(err, "unable to unmarshal the acls associated with the group %v",
-				groupId)
-		}
-
-		for _, acl := range acls {
-			fmt.Printf("ACL  : %v\n", acl)
-		}
+	for _, acl := range group.Rules {
+		fmt.Printf("ACL: %v\n", acl)
 	}
+
 	return nil
 }