Improving ACL

edgraph/access_ee.go

@@ -369,18 +369,7 @@ func ResetAcl() {
 		}
 
 		// Insert Groot.
-		createUserNQuads := []*api.NQuad{
-			{
-				Subject:     "_:newuser",
-				Predicate:   "dgraph.xid",
-				ObjectValue: &api.Value{Val: &api.Value_StrVal{StrVal: x.GrootId}},
-			},
-			{
-				Subject:     "_:newuser",
-				Predicate:   "dgraph.password",
-				ObjectValue: &api.Value{Val: &api.Value_StrVal{StrVal: "password"}},
-			}}
-
+		createUserNQuads := acl.CreateUserNQuads(x.GrootId, "password")
 		mu := &api.Mutation{
 			StartTs:   startTs,
 			CommitNow: true,

ee/acl/acl.go

@@ -41,9 +41,16 @@ func checkForbiddenOpts(conf *viper.Viper, forbiddenOpts []string) error {
 		var isSet bool
 		switch conf.Get(opt).(type) {
 		case string:
-			isSet = len(conf.GetString(opt)) > 0
+			if opt == "group_list" {
+				// handle group_list specially since the default value is not an empty string
+				isSet = conf.GetString(opt) != defaultGroupList
+			} else {
+				isSet = len(conf.GetString(opt)) > 0
+			}
 		case int:
 			isSet = conf.GetInt(opt) > 0
+		case bool:
+			isSet = conf.GetBool(opt)
 		default:
 			return fmt.Errorf("unexpected option type for %s", opt)
 		}
@@ -103,17 +110,7 @@ func userAdd(conf *viper.Viper, userid string, password string) error {
 		return fmt.Errorf("unable to create user because of conflict: %v", userid)
 	}
 
-	createUserNQuads := []*api.NQuad{
-		{
-			Subject:     "_:newuser",
-			Predicate:   "dgraph.xid",
-			ObjectValue: &api.Value{Val: &api.Value_StrVal{StrVal: userid}},
-		},
-		{
-			Subject:     "_:newuser",
-			Predicate:   "dgraph.password",
-			ObjectValue: &api.Value{Val: &api.Value_StrVal{StrVal: password}},
-		}}
+	createUserNQuads := CreateUserNQuads(userid, password)
 
 	mu := &api.Mutation{
 		CommitNow: true,
@@ -157,6 +154,11 @@ func groupAdd(conf *viper.Viper, groupId string) error {
 			Predicate:   "dgraph.xid",
 			ObjectValue: &api.Value{Val: &api.Value_StrVal{StrVal: groupId}},
 		},
+		{
+			Subject:     "_:newgroup",
+			Predicate:   "type",
+			ObjectValue: &api.Value{Val: &api.Value_StrVal{StrVal: "Group"}},
+		},
 	}
 
 	mu := &api.Mutation{
@@ -244,17 +246,23 @@ func mod(conf *viper.Viper) error {
 	if err != nil {
 		return err
 	}
-	groupList := conf.GetString("group_list")
+
 	if len(userId) != 0 {
 		// when modifying the user, some group options are forbidden
 		if err := checkForbiddenOpts(conf, []string{"pred", "pred_regex", "perm"}); err != nil {
 			return err
 		}
 
-		if len(groupList) != 0 {
-			return userMod(conf, userId, groupList)
+		if (conf.GetBool("new_password") && conf.GetString("group_list") != defaultGroupList) ||
+			(!conf.GetBool("new_password") && conf.GetString("group_list") == defaultGroupList) {
+			return fmt.Errorf("one of --new_password or --group_list must be provided, but not both")
 		}
-		return changePassword(conf, userId)
+
+		if conf.GetBool("new_password") {
+			return changePassword(conf, userId)
+		}
+
+		return userMod(conf, userId, conf.GetString("group_list"))
 	}
 
 	// when modifying the group, some user options are forbidden
@@ -274,13 +282,9 @@ func changePassword(conf *viper.Viper, userId string) error {
 	defer cancel()
 
 	// 2. get the new password
-	newPassword := conf.GetString("new_password")
-	if len(newPassword) == 0 {
-		var err error
-		newPassword, err = askUserPassword(userId, "New", 2)
-		if err != nil {
-			return err
-		}
+	newPassword, err := askUserPassword(userId, "New", 2)
+	if err != nil {
+		return err
 	}
 
 	ctx := context.Background()
@@ -385,8 +389,9 @@ func userMod(conf *viper.Viper, userId string, groups string) error {
 	if _, err := txn.Mutate(ctx, mu); err != nil {
 		return fmt.Errorf("error while mutating the group: %+v", err)
 	}
-	fmt.Printf("Successfully modified groups for user %v\n", userId)
-	return nil
+	fmt.Printf("Successfully modified groups for user %v.\n", userId)
+	fmt.Println("The latest info is:")
+	return queryAndPrintUser(ctx, dc.NewReadOnlyTxn(), userId)
 }
 
 func chMod(conf *viper.Viper) error {
@@ -401,6 +406,9 @@ func chMod(conf *viper.Viper) error {
 		return fmt.Errorf("one of --pred or --pred_regex must be specified, but not both")
 	case len(predicate) == 0 && len(predRegex) == 0:
 		return fmt.Errorf("one of --pred or --pred_regex must be specified, but not both")
+	case perm > 7:
+		return fmt.Errorf("the perm value must be less than or equal to 7, "+
+			"the provided value is %d", perm)
 	case len(predRegex) > 0:
 		// make sure the predRegex can be compiled as a regex
 		if _, err := regexp.Compile(predRegex); err != nil {
@@ -479,13 +487,14 @@ func chMod(conf *viper.Viper) error {
 	}
 	fmt.Printf("Successfully changed permission for group %v on predicate %v to %v\n",
 		groupId, predicate, perm)
-	return nil
+	fmt.Println("The latest info is:")
+	return queryAndPrintGroup(ctx, dc.NewReadOnlyTxn(), groupId)
 }
 
 func queryUser(ctx context.Context, txn *dgo.Txn, userid string) (user *User, err error) {
 	query := `
     query search($userid: string){
-      user(func: eq(dgraph.xid, $userid)) {
+      user(func: eq(dgraph.xid, $userid)) @filter(type(User)) {
 	    uid
         dgraph.xid
         dgraph.user.group {
@@ -533,7 +542,7 @@ func queryGroup(ctx context.Context, txn *dgo.Txn, groupid string,
 
 	// write query header
 	query := fmt.Sprintf(`query search($groupid: string){
-        group(func: eq(dgraph.xid, $groupid)) {
+        group(func: eq(dgraph.xid, $groupid)) @filter(type(Group)) {
 			uid
 		    %s }}`, strings.Join(fields, ", "))
 

ee/acl/acl_test.go

@@ -285,7 +285,7 @@ func createGroupAndAcls(t *testing.T, group string, addUserToGroup bool) {
 	addReadPermCmd1 := exec.Command(os.ExpandEnv("$GOPATH/bin/dgraph"),
 		"acl", "mod",
 		"-d", dgraphEndpoint,
-		"-g", group, "-p", predicateToRead, "-P", strconv.Itoa(int(Read.Code)), "-x",
+		"-g", group, "-p", predicateToRead, "-m", strconv.Itoa(int(Read.Code)), "-x",
 		"password")
 	if errOutput, err := addReadPermCmd1.CombinedOutput(); err != nil {
 		t.Fatalf("Unable to add READ permission on %s to group %s: %v",
@@ -296,7 +296,7 @@ func createGroupAndAcls(t *testing.T, group string, addUserToGroup bool) {
 	addReadPermCmd2 := exec.Command(os.ExpandEnv("$GOPATH/bin/dgraph"),
 		"acl", "mod",
 		"-d", dgraphEndpoint,
-		"-g", group, "-p", queryAttr, "-P", strconv.Itoa(int(Read.Code)), "-x",
+		"-g", group, "-p", queryAttr, "-m", strconv.Itoa(int(Read.Code)), "-x",
 		"password")
 	if errOutput, err := addReadPermCmd2.CombinedOutput(); err != nil {
 		t.Fatalf("Unable to add READ permission on %s to group %s: %v", queryAttr, group,
@@ -307,7 +307,7 @@ func createGroupAndAcls(t *testing.T, group string, addUserToGroup bool) {
 	addWritePermCmd := exec.Command(os.ExpandEnv("$GOPATH/bin/dgraph"),
 		"acl", "mod",
 		"-d", dgraphEndpoint,
-		"-g", group, "-p", predicateToWrite, "-P", strconv.Itoa(int(Write.Code)), "-x",
+		"-g", group, "-p", predicateToWrite, "-m", strconv.Itoa(int(Write.Code)), "-x",
 		"password")
 	if errOutput, err := addWritePermCmd.CombinedOutput(); err != nil {
 		t.Fatalf("Unable to add permission on %s to group %s: %v", predicateToWrite, group,
@@ -318,39 +318,14 @@ func createGroupAndAcls(t *testing.T, group string, addUserToGroup bool) {
 	addModifyPermCmd := exec.Command(os.ExpandEnv("$GOPATH/bin/dgraph"),
 		"acl", "mod",
 		"-d", dgraphEndpoint,
-		"-g", group, "-p", predicateToAlter, "-P", strconv.Itoa(int(Modify.Code)), "-x",
+		"-g", group, "-p", predicateToAlter, "-m", strconv.Itoa(int(Modify.Code)), "-x",
 		"password")
 	if errOutput, err := addModifyPermCmd.CombinedOutput(); err != nil {
 		t.Fatalf("Unable to add permission on %s to group %s: %v", predicateToAlter, group,
 			string(errOutput))
 	}
 }
 
-func TestPasswordReset(t *testing.T) {
-	glog.Infof("testing with port 9180")
-	dg := z.DgraphClientWithGroot(":9180")
-	createAccountAndData(t, dg)
-	// test login using the current password
-	ctx := context.Background()
-	err := dg.Login(ctx, userid, userpassword)
-	require.NoError(t, err, "Logging in with the current password should have succeeded")
-
-	// reset password for the user alice
-	newPassword := userpassword + "123"
-	chPdCmd := exec.Command("dgraph", "acl", "mod", "-d", dgraphEndpoint, "-u",
-		userid, "--new_password", newPassword, "-x", "password")
-	checkOutput(t, chPdCmd, false)
-	glog.Infof("Successfully changed password for %v", userid)
-
-	// test that logging in using the old password should now fail
-	err = dg.Login(ctx, userid, userpassword)
-	require.Error(t, err, "Logging in with old password should no longer work")
-
-	// test logging in using the new password
-	err = dg.Login(ctx, userid, newPassword)
-	require.NoError(t, err, "Logging in with new password should work now")
-}
-
 func TestPredicateRegex(t *testing.T) {
 	glog.Infof("testing with port 9180")
 	dg := z.DgraphClientWithGroot(":9180")
@@ -395,7 +370,7 @@ func TestPredicateRegex(t *testing.T) {
 	addReadToNameCmd := exec.Command(os.ExpandEnv("$GOPATH/bin/dgraph"),
 		"acl", "mod",
 		"-d", dgraphEndpoint,
-		"-g", devGroup, "--pred", "name", "-P", strconv.Itoa(int(Read.Code)|int(Write.Code)),
+		"-g", devGroup, "--pred", "name", "-m", strconv.Itoa(int(Read.Code)|int(Write.Code)),
 		"-x",
 		"password")
 	if errOutput, err := addReadToNameCmd.CombinedOutput(); err != nil {
@@ -408,7 +383,7 @@ func TestPredicateRegex(t *testing.T) {
 	addReadWriteToRegexPermCmd := exec.Command(os.ExpandEnv("$GOPATH/bin/dgraph"),
 		"acl", "mod",
 		"-d", dgraphEndpoint,
-		"-g", devGroup, "--pred_regex", predRegex, "-P",
+		"-g", devGroup, "-P", predRegex, "-m",
 		strconv.Itoa(int(Read.Code)|int(Write.Code)), "-x", "password")
 	if errOutput, err := addReadWriteToRegexPermCmd.CombinedOutput(); err != nil {
 		t.Fatalf("Unable to add READ+WRITE permission on %s to group %s:%v",

ee/acl/run_ee.go

@@ -38,6 +38,7 @@ var (
 )
 
 const gPassword = "gpassword"
+const defaultGroupList = "dgraph-unused-group"
 
 func init() {
 	CmdAcl.Cmd = &cobra.Command{
@@ -116,13 +117,14 @@ func initSubcommands() []*x.SubCommand {
 
 	modFlags := cmdMod.Cmd.Flags()
 	modFlags.StringP("user", "u", "", "The user id to be changed")
-	modFlags.StringP("new_password", "", "", "The new password for the user")
-	modFlags.StringP("group_list", "l", "", "The list of groups to be set for the user")
+	modFlags.BoolP("new_password", "", false, "Whether to reset password for the user")
+	modFlags.StringP("group_list", "l", defaultGroupList,
+		"The list of groups to be set for the user")
 	modFlags.StringP("group", "g", "", "The group whose permission is to be changed")
 	modFlags.StringP("pred", "p", "", "The predicates whose acls are to be changed")
-	modFlags.StringP("pred_regex", "", "", "The regular expression specifying predicates"+
+	modFlags.StringP("pred_regex", "P", "", "The regular expression specifying predicates"+
 		" whose acls are to be changed")
-	modFlags.IntP("perm", "P", 0, "The acl represented using "+
+	modFlags.IntP("perm", "m", 0, "The acl represented using "+
 		"an integer: 4 for read, 2 for write, and 1 for modify. Use a negative value to remove a "+
 		"predicate from the group")
 
@@ -169,13 +171,62 @@ func getDgraphClient(conf *viper.Viper) (*dgo.Dgraph, CloseFunc) {
 	}
 }
 
+func queryAndPrintUser(ctx context.Context, txn *dgo.Txn, userId string) error {
+	user, err := queryUser(ctx, txn, userId)
+	if err != nil {
+		return err
+	}
+	if user == nil {
+		return fmt.Errorf("The user %q does not exist.\n", userId)
+	}
+
+	fmt.Printf("User  : %s\n", userId)
+	fmt.Printf("UID   : %s\n", user.Uid)
+	for _, group := range user.Groups {
+		fmt.Printf("Group : %-5s\n", group.GroupID)
+	}
+	return nil
+}
+
+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")
+	if err != nil {
+		return err
+	}
+	if group == nil {
+		return fmt.Errorf("The group %q does not exist.\n", groupId)
+	}
+	fmt.Printf("Group: %s\n", groupId)
+	fmt.Printf("UID  : %s\n", group.Uid)
+	fmt.Printf("ID   : %s\n", group.GroupID)
+
+	var userNames []string
+	for _, user := range group.Users {
+		userNames = append(userNames, user.UserID)
+	}
+	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 fmt.Errorf("unable to unmarshal the acls associated with the group %v: %v",
+				groupId, err)
+		}
+
+		for _, acl := range acls {
+			fmt.Printf("ACL  : %v\n", acl)
+		}
+	}
+	return nil
+}
+
 func info(conf *viper.Viper) error {
-	userId := conf.GetString("user")
-	groupId := conf.GetString("group")
-	if (len(userId) == 0 && len(groupId) == 0) ||
-		(len(userId) != 0 && len(groupId) != 0) {
-		return fmt.Errorf("either the user or group should be specified, not both")
+	userId, groupId, err := getUserAndGroup(conf)
+	if err != nil {
+		return err
 	}
+
 	dc, cancel, err := getClientWithAdminCtx(conf)
 	defer cancel()
 	if err != nil {
@@ -191,53 +242,8 @@ func info(conf *viper.Viper) error {
 	}()
 
 	if len(userId) != 0 {
-		user, err := queryUser(ctx, txn, userId)
-		if err != nil {
-			return err
-		}
-		if user == nil {
-			return fmt.Errorf("The user %q does not exist.\n", userId)
-		}
-
-		fmt.Println()
-		fmt.Printf("User  : %s\n", userId)
-		fmt.Printf("UID   : %s\n", user.Uid)
-		for _, group := range user.Groups {
-			fmt.Printf("Group : %-5s\n", group.GroupID)
-		}
+		return queryAndPrintUser(ctx, txn, userId)
 	}
 
-	if len(groupId) != 0 {
-		group, err := queryGroup(ctx, txn, groupId, "dgraph.xid", "~dgraph.user.group{dgraph.xid}",
-			"dgraph.group.acl")
-		if err != nil {
-			return err
-		}
-		if group == nil {
-			return fmt.Errorf("The group %q does not exist.\n", groupId)
-		}
-		fmt.Printf("Group: %s\n", groupId)
-		fmt.Printf("UID  : %s\n", group.Uid)
-		fmt.Printf("ID   : %s\n", group.GroupID)
-
-		var userNames []string
-		for _, user := range group.Users {
-			userNames = append(userNames, user.UserID)
-		}
-		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 fmt.Errorf("unable to unmarshal the acls associated with the group %v: %v",
-					groupId, err)
-			}
-
-			for _, acl := range acls {
-				fmt.Printf("ACL  : %v\n", acl)
-			}
-		}
-	}
-
-	return nil
+	return queryAndPrintGroup(ctx, txn, groupId)
 }