From 0818557487f2da440343c6d81adaad6d901d430a Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Fri, 8 Mar 2019 18:49:09 -0800 Subject: [PATCH 01/10] Print latest info about a user or group after modifications --- ee/acl/acl.go | 16 ++++--- ee/acl/run_ee.go | 108 ++++++++++++++++++++++++----------------------- 2 files changed, 66 insertions(+), 58 deletions(-) diff --git a/ee/acl/acl.go b/ee/acl/acl.go index 2cfefe7a0f0..2d3752bf724 100644 --- a/ee/acl/acl.go +++ b/ee/acl/acl.go @@ -244,15 +244,15 @@ 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.Get("group_list") != nil { + return userMod(conf, userId, conf.GetString("group_list")) } return changePassword(conf, userId) } @@ -385,8 +385,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 +402,8 @@ 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 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,7 +482,8 @@ 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) { diff --git a/ee/acl/run_ee.go b/ee/acl/run_ee.go index 23da40ea1bc..86a2f9d4390 100644 --- a/ee/acl/run_ee.go +++ b/ee/acl/run_ee.go @@ -169,13 +169,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 +240,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) } From 29b1fd3df52af4567bf8a721881e4275b9a4c31e Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Mon, 11 Mar 2019 12:05:39 -0700 Subject: [PATCH 02/10] Using types to distinguish user and group --- edgraph/access_ee.go | 14 ++------------ ee/acl/acl.go | 22 +++++++++------------- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/edgraph/access_ee.go b/edgraph/access_ee.go index 60b5e903287..dafed576521 100644 --- a/edgraph/access_ee.go +++ b/edgraph/access_ee.go @@ -25,6 +25,7 @@ import ( "github.com/dgraph-io/dgo/protos/api" "github.com/dgraph-io/dgraph/ee/acl" + "github.com/dgraph-io/dgraph/ee/acl/lib" "github.com/dgraph-io/dgraph/gql" "github.com/dgraph-io/dgraph/schema" "github.com/dgraph-io/dgraph/x" @@ -369,18 +370,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 := lib.GetCreateUserNQuads(x.GrootId, "password") mu := &api.Mutation{ StartTs: startTs, CommitNow: true, diff --git a/ee/acl/acl.go b/ee/acl/acl.go index 2d3752bf724..2ebcf9b7e31 100644 --- a/ee/acl/acl.go +++ b/ee/acl/acl.go @@ -21,6 +21,7 @@ import ( "github.com/dgraph-io/dgo" "github.com/dgraph-io/dgo/protos/api" + "github.com/dgraph-io/dgraph/ee/acl/lib" "github.com/dgraph-io/dgraph/x" "github.com/golang/glog" "github.com/spf13/viper" @@ -103,17 +104,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 := lib.GetCreateUserNQuads(userid, password) mu := &api.Mutation{ CommitNow: true, @@ -157,6 +148,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{ @@ -489,7 +485,7 @@ func chMod(conf *viper.Viper) error { 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 { @@ -537,7 +533,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, ", ")) From 78d233a517d4d7929b738e6892dc4c35f9608277 Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Mon, 11 Mar 2019 12:06:08 -0700 Subject: [PATCH 03/10] Using type to distinguish user and group --- ee/acl/lib/acl_lib.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 ee/acl/lib/acl_lib.go diff --git a/ee/acl/lib/acl_lib.go b/ee/acl/lib/acl_lib.go new file mode 100644 index 00000000000..7c1668f7a96 --- /dev/null +++ b/ee/acl/lib/acl_lib.go @@ -0,0 +1,35 @@ +// +build !oss + +/* + * Copyright 2018 Dgraph Labs, Inc. and Contributors + * + * Licensed under the Dgraph Community License (the "License"); you + * may not use this file except in compliance with the License. You + * may obtain a copy of the License at + * + * https://github.com/dgraph-io/dgraph/blob/master/licenses/DCL.txt + */ + +package lib + +import "github.com/dgraph-io/dgo/protos/api" + +func GetCreateUserNQuads(userId string, password string) []*api.NQuad { + return []*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}}, + }, + { + Subject: "_:newuser", + Predicate: "type", + ObjectValue: &api.Value{Val: &api.Value_StrVal{StrVal: "User"}}, + }, + } +} From efd8d727b19afca8df1f7a650bacd8a93c480641 Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Mon, 11 Mar 2019 15:15:41 -0700 Subject: [PATCH 04/10] Checking if the option --group_list is provided to invoke userMod --- ee/acl/acl.go | 15 +++++++++++++-- ee/acl/run_ee.go | 2 +- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/ee/acl/acl.go b/ee/acl/acl.go index 2ebcf9b7e31..48252340841 100644 --- a/ee/acl/acl.go +++ b/ee/acl/acl.go @@ -24,6 +24,7 @@ import ( "github.com/dgraph-io/dgraph/ee/acl/lib" "github.com/dgraph-io/dgraph/x" "github.com/golang/glog" + "github.com/spf13/pflag" "github.com/spf13/viper" ) @@ -235,7 +236,17 @@ func userOrGroupDel(conf *viper.Viper, userOrGroupId string, return nil } -func mod(conf *viper.Viper) error { +func isFlagPassed(f *pflag.FlagSet, name string) bool { + found := false + f.Visit(func(f *pflag.Flag) { + if f.Name == name { + found = true + } + }) + return found +} + +func mod(flags *pflag.FlagSet, conf *viper.Viper) error { userId, _, err := getUserAndGroup(conf) if err != nil { return err @@ -247,7 +258,7 @@ func mod(conf *viper.Viper) error { return err } - if conf.Get("group_list") != nil { + if isFlagPassed(flags, "group_list") { return userMod(conf, userId, conf.GetString("group_list")) } return changePassword(conf, userId) diff --git a/ee/acl/run_ee.go b/ee/acl/run_ee.go index 86a2f9d4390..93ee2bfd86c 100644 --- a/ee/acl/run_ee.go +++ b/ee/acl/run_ee.go @@ -107,7 +107,7 @@ func initSubcommands() []*x.SubCommand { Short: "Run Dgraph acl tool to modify a user's password, a user's group list, or a" + "group's predicate permissions", Run: func(cmd *cobra.Command, args []string) { - if err := mod(cmdMod.Conf); err != nil { + if err := mod(cmdMod.Cmd.Flags(), cmdMod.Conf); err != nil { fmt.Printf("Unable to modify: %v\n", err) os.Exit(1) } From 01426eea92bd10c25156ca4654650a044a895c65 Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Mon, 11 Mar 2019 15:28:35 -0700 Subject: [PATCH 05/10] Updating error message to include the perm value 7 --- ee/acl/acl.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/acl/acl.go b/ee/acl/acl.go index 48252340841..2c390e46b2e 100644 --- a/ee/acl/acl.go +++ b/ee/acl/acl.go @@ -410,7 +410,7 @@ func chMod(conf *viper.Viper) error { 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 7, the provided value is %d", perm) + 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 { From 7499c775ae14642a92307c4eebdfabd46f88fd73 Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Mon, 11 Mar 2019 15:42:19 -0700 Subject: [PATCH 06/10] Reduce line width to below 100 chars --- ee/acl/acl.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ee/acl/acl.go b/ee/acl/acl.go index 2c390e46b2e..503941527da 100644 --- a/ee/acl/acl.go +++ b/ee/acl/acl.go @@ -410,7 +410,8 @@ func chMod(conf *viper.Viper) error { 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) + 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 { From c35c6af0a1f41d44b8d6bd194bab0412eefb3860 Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Mon, 11 Mar 2019 17:04:18 -0700 Subject: [PATCH 07/10] Addressed Manish's comments --- edgraph/access_ee.go | 3 +-- ee/acl/acl.go | 35 +++++++++++++---------------------- ee/acl/lib/acl_lib.go | 35 ----------------------------------- ee/acl/run_ee.go | 6 ++++-- ee/acl/utils.go | 20 ++++++++++++++++++++ 5 files changed, 38 insertions(+), 61 deletions(-) delete mode 100644 ee/acl/lib/acl_lib.go diff --git a/edgraph/access_ee.go b/edgraph/access_ee.go index dafed576521..bebb73a4053 100644 --- a/edgraph/access_ee.go +++ b/edgraph/access_ee.go @@ -25,7 +25,6 @@ import ( "github.com/dgraph-io/dgo/protos/api" "github.com/dgraph-io/dgraph/ee/acl" - "github.com/dgraph-io/dgraph/ee/acl/lib" "github.com/dgraph-io/dgraph/gql" "github.com/dgraph-io/dgraph/schema" "github.com/dgraph-io/dgraph/x" @@ -370,7 +369,7 @@ func ResetAcl() { } // Insert Groot. - createUserNQuads := lib.GetCreateUserNQuads(x.GrootId, "password") + createUserNQuads := acl.CreateUserNQuads(x.GrootId, "password") mu := &api.Mutation{ StartTs: startTs, CommitNow: true, diff --git a/ee/acl/acl.go b/ee/acl/acl.go index 503941527da..b7d48188557 100644 --- a/ee/acl/acl.go +++ b/ee/acl/acl.go @@ -21,7 +21,6 @@ import ( "github.com/dgraph-io/dgo" "github.com/dgraph-io/dgo/protos/api" - "github.com/dgraph-io/dgraph/ee/acl/lib" "github.com/dgraph-io/dgraph/x" "github.com/golang/glog" "github.com/spf13/pflag" @@ -105,7 +104,7 @@ func userAdd(conf *viper.Viper, userid string, password string) error { return fmt.Errorf("unable to create user because of conflict: %v", userid) } - createUserNQuads := lib.GetCreateUserNQuads(userid, password) + createUserNQuads := CreateUserNQuads(userid, password) mu := &api.Mutation{ CommitNow: true, @@ -236,16 +235,6 @@ func userOrGroupDel(conf *viper.Viper, userOrGroupId string, return nil } -func isFlagPassed(f *pflag.FlagSet, name string) bool { - found := false - f.Visit(func(f *pflag.Flag) { - if f.Name == name { - found = true - } - }) - return found -} - func mod(flags *pflag.FlagSet, conf *viper.Viper) error { userId, _, err := getUserAndGroup(conf) if err != nil { @@ -258,10 +247,16 @@ func mod(flags *pflag.FlagSet, conf *viper.Viper) error { return err } - if isFlagPassed(flags, "group_list") { - return userMod(conf, userId, conf.GetString("group_list")) + 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 @@ -281,13 +276,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() diff --git a/ee/acl/lib/acl_lib.go b/ee/acl/lib/acl_lib.go deleted file mode 100644 index 7c1668f7a96..00000000000 --- a/ee/acl/lib/acl_lib.go +++ /dev/null @@ -1,35 +0,0 @@ -// +build !oss - -/* - * Copyright 2018 Dgraph Labs, Inc. and Contributors - * - * Licensed under the Dgraph Community License (the "License"); you - * may not use this file except in compliance with the License. You - * may obtain a copy of the License at - * - * https://github.com/dgraph-io/dgraph/blob/master/licenses/DCL.txt - */ - -package lib - -import "github.com/dgraph-io/dgo/protos/api" - -func GetCreateUserNQuads(userId string, password string) []*api.NQuad { - return []*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}}, - }, - { - Subject: "_:newuser", - Predicate: "type", - ObjectValue: &api.Value{Val: &api.Value_StrVal{StrVal: "User"}}, - }, - } -} diff --git a/ee/acl/run_ee.go b/ee/acl/run_ee.go index 93ee2bfd86c..fecef9bb97e 100644 --- a/ee/acl/run_ee.go +++ b/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,8 +117,9 @@ 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"+ diff --git a/ee/acl/utils.go b/ee/acl/utils.go index 89ddec5f901..9dc3c9e3e5b 100644 --- a/ee/acl/utils.go +++ b/ee/acl/utils.go @@ -206,3 +206,23 @@ func getClientWithUserCtx(userid string, passwordOpt string, conf *viper.Viper) func getClientWithAdminCtx(conf *viper.Viper) (*dgo.Dgraph, CloseFunc, error) { return getClientWithUserCtx(x.GrootId, gPassword, conf) } + +func CreateUserNQuads(userId string, password string) []*api.NQuad { + return []*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}}, + }, + { + Subject: "_:newuser", + Predicate: "type", + ObjectValue: &api.Value{Val: &api.Value_StrVal{StrVal: "User"}}, + }, + } +} From 12f29e637e84d376be6ba99e72ed8cb6af6c8557 Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Tue, 12 Mar 2019 09:30:43 -0700 Subject: [PATCH 08/10] Fixed tests, and correctly checking if the group list is set --- ee/acl/acl.go | 9 ++++++++- ee/acl/acl_test.go | 37 ++++++------------------------------- ee/acl/run_ee.go | 4 ++-- 3 files changed, 16 insertions(+), 34 deletions(-) diff --git a/ee/acl/acl.go b/ee/acl/acl.go index b7d48188557..bf6e84d1b7c 100644 --- a/ee/acl/acl.go +++ b/ee/acl/acl.go @@ -42,9 +42,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) } diff --git a/ee/acl/acl_test.go b/ee/acl/acl_test.go index dc8ea8a1909..67f2a77a8fa 100644 --- a/ee/acl/acl_test.go +++ b/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,7 +318,7 @@ 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, @@ -326,31 +326,6 @@ func createGroupAndAcls(t *testing.T, group string, addUserToGroup bool) { } } -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", diff --git a/ee/acl/run_ee.go b/ee/acl/run_ee.go index fecef9bb97e..babedbedb5d 100644 --- a/ee/acl/run_ee.go +++ b/ee/acl/run_ee.go @@ -122,9 +122,9 @@ func initSubcommands() []*x.SubCommand { "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") From 2c1c07243ae45845bc8fecaf02be8d3aee9b1f0a Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Tue, 12 Mar 2019 11:39:48 -0700 Subject: [PATCH 09/10] Removed unused parameter --- ee/acl/acl.go | 3 +-- ee/acl/run_ee.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/ee/acl/acl.go b/ee/acl/acl.go index bf6e84d1b7c..b611938967d 100644 --- a/ee/acl/acl.go +++ b/ee/acl/acl.go @@ -23,7 +23,6 @@ import ( "github.com/dgraph-io/dgo/protos/api" "github.com/dgraph-io/dgraph/x" "github.com/golang/glog" - "github.com/spf13/pflag" "github.com/spf13/viper" ) @@ -242,7 +241,7 @@ func userOrGroupDel(conf *viper.Viper, userOrGroupId string, return nil } -func mod(flags *pflag.FlagSet, conf *viper.Viper) error { +func mod(conf *viper.Viper) error { userId, _, err := getUserAndGroup(conf) if err != nil { return err diff --git a/ee/acl/run_ee.go b/ee/acl/run_ee.go index babedbedb5d..ff6b263fd27 100644 --- a/ee/acl/run_ee.go +++ b/ee/acl/run_ee.go @@ -108,7 +108,7 @@ func initSubcommands() []*x.SubCommand { Short: "Run Dgraph acl tool to modify a user's password, a user's group list, or a" + "group's predicate permissions", Run: func(cmd *cobra.Command, args []string) { - if err := mod(cmdMod.Cmd.Flags(), cmdMod.Conf); err != nil { + if err := mod(cmdMod.Conf); err != nil { fmt.Printf("Unable to modify: %v\n", err) os.Exit(1) } From 9d1dbaed05a052b1a5adc0ecd0653c7b58acb814 Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Wed, 13 Mar 2019 11:15:37 -0700 Subject: [PATCH 10/10] Adding timeout on context for all commands --- ee/acl/acl.go | 107 ++++++++++++++++++++++++++++++++++++++++----- ee/acl/run_ee.go | 110 +---------------------------------------------- ee/acl/utils.go | 28 +++++++++++- 3 files changed, 125 insertions(+), 120 deletions(-) diff --git a/ee/acl/acl.go b/ee/acl/acl.go index b611938967d..3e0b3ab2b86 100644 --- a/ee/acl/acl.go +++ b/ee/acl/acl.go @@ -18,6 +18,7 @@ import ( "fmt" "regexp" "strings" + "time" "github.com/dgraph-io/dgo" "github.com/dgraph-io/dgo/protos/api" @@ -94,7 +95,8 @@ func userAdd(conf *viper.Viper, userid string, password string) error { } } - ctx := context.Background() + ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer ctxCancel() txn := dc.NewTxn() defer func() { if err := txn.Discard(ctx); err != nil { @@ -132,7 +134,8 @@ func groupAdd(conf *viper.Viper, groupId string) error { } defer cancel() - ctx := context.Background() + ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer ctxCancel() txn := dc.NewTxn() defer func() { if err := txn.Discard(ctx); err != nil { @@ -204,7 +207,8 @@ func userOrGroupDel(conf *viper.Viper, userOrGroupId string, } defer cancel() - ctx := context.Background() + ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer ctxCancel() txn := dc.NewTxn() defer func() { if err := txn.Discard(ctx); err != nil { @@ -253,16 +257,18 @@ func mod(conf *viper.Viper) error { return err } - if (conf.GetBool("new_password") && conf.GetString("group_list") != defaultGroupList) || - (!conf.GetBool("new_password") && conf.GetString("group_list") == defaultGroupList) { + newPassword := conf.GetBool("new_password") + groupList := conf.GetString("group_list") + if (newPassword && groupList != defaultGroupList) || + (!newPassword && groupList == defaultGroupList) { return fmt.Errorf("one of --new_password or --group_list must be provided, but not both") } - if conf.GetBool("new_password") { + if newPassword { return changePassword(conf, userId) } - return userMod(conf, userId, conf.GetString("group_list")) + return userMod(conf, userId, groupList) } // when modifying the group, some user options are forbidden @@ -287,7 +293,8 @@ func changePassword(conf *viper.Viper, userId string) error { return err } - ctx := context.Background() + ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer ctxCancel() txn := dc.NewTxn() defer func() { if err := txn.Discard(ctx); err != nil { @@ -329,7 +336,8 @@ func userMod(conf *viper.Viper, userId string, groups string) error { } defer cancel() - ctx := context.Background() + ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer ctxCancel() txn := dc.NewTxn() defer func() { if err := txn.Discard(ctx); err != nil { @@ -423,7 +431,8 @@ func chMod(conf *viper.Viper) error { } defer cancel() - ctx := context.Background() + ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer ctxCancel() txn := dc.NewTxn() defer func() { if err := txn.Discard(ctx); err != nil { @@ -589,3 +598,81 @@ func updateAcl(acls []Acl, newAcl Acl) ([]Acl, bool) { // 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 { + 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, groupId, err := getUserAndGroup(conf) + if err != nil { + return err + } + + dc, cancel, err := getClientWithAdminCtx(conf) + defer cancel() + if err != nil { + return fmt.Errorf("unable to get admin context: %v\n", err) + } + + ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer ctxCancel() + txn := dc.NewTxn() + defer func() { + if err := txn.Discard(ctx); err != nil { + fmt.Printf("Unable to discard transaction: %v\n", err) + } + }() + + if len(userId) != 0 { + return queryAndPrintUser(ctx, txn, userId) + } + + return queryAndPrintGroup(ctx, txn, groupId) +} diff --git a/ee/acl/run_ee.go b/ee/acl/run_ee.go index ff6b263fd27..a2c27a06a25 100644 --- a/ee/acl/run_ee.go +++ b/ee/acl/run_ee.go @@ -13,14 +13,9 @@ package acl import ( - "context" - "encoding/json" "fmt" "os" - "strings" - "github.com/dgraph-io/dgo" - "github.com/dgraph-io/dgo/protos/api" "github.com/dgraph-io/dgraph/x" "github.com/golang/glog" "github.com/spf13/cobra" @@ -117,7 +112,7 @@ func initSubcommands() []*x.SubCommand { modFlags := cmdMod.Cmd.Flags() modFlags.StringP("user", "u", "", "The user id to be changed") - modFlags.BoolP("new_password", "", false, "Whether to reset password for the user") + modFlags.BoolP("new_password", "n", 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") @@ -144,106 +139,3 @@ func initSubcommands() []*x.SubCommand { infoFlags.StringP("group", "g", "", "The group to be shown") return []*x.SubCommand{&cmdAdd, &cmdDel, &cmdMod, &cmdInfo} } - -type CloseFunc func() - -func getDgraphClient(conf *viper.Viper) (*dgo.Dgraph, CloseFunc) { - opt = options{ - dgraph: conf.GetString("dgraph"), - } - fmt.Printf("\nRunning transaction with dgraph endpoint: %v\n", opt.dgraph) - - if len(opt.dgraph) == 0 { - glog.Fatalf("The --dgraph option must be set in order to connect to dgraph") - } - - x.LoadTLSConfig(&tlsConf, CmdAcl.Conf, x.TlsClientCert, x.TlsClientKey) - tlsConf.ServerName = CmdAcl.Conf.GetString("tls_server_name") - - conn, err := x.SetupConnection(opt.dgraph, &tlsConf, false) - x.Checkf(err, "While trying to setup connection to Dgraph alpha.") - - dc := api.NewDgraphClient(conn) - return dgo.NewDgraphClient(dc), func() { - if err := conn.Close(); err != nil { - fmt.Printf("Error while closing connection: %v\n", err) - } - } -} - -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, groupId, err := getUserAndGroup(conf) - if err != nil { - return err - } - - dc, cancel, err := getClientWithAdminCtx(conf) - defer cancel() - if err != nil { - return fmt.Errorf("unable to get admin context: %v\n", err) - } - - ctx := context.Background() - txn := dc.NewTxn() - defer func() { - if err := txn.Discard(ctx); err != nil { - fmt.Printf("Unable to discard transaction: %v\n", err) - } - }() - - if len(userId) != 0 { - return queryAndPrintUser(ctx, txn, userId) - } - - return queryAndPrintGroup(ctx, txn, groupId) -} diff --git a/ee/acl/utils.go b/ee/acl/utils.go index 9dc3c9e3e5b..14ed77ee3ad 100644 --- a/ee/acl/utils.go +++ b/ee/acl/utils.go @@ -176,6 +176,32 @@ func askUserPassword(userid string, pwdType string, times int) (string, error) { return password, nil } +type CloseFunc func() + +func getDgraphClient(conf *viper.Viper) (*dgo.Dgraph, CloseFunc) { + opt = options{ + dgraph: conf.GetString("dgraph"), + } + fmt.Printf("\nRunning transaction with dgraph endpoint: %v\n", opt.dgraph) + + if len(opt.dgraph) == 0 { + glog.Fatalf("The --dgraph option must be set in order to connect to dgraph") + } + + x.LoadTLSConfig(&tlsConf, CmdAcl.Conf, x.TlsClientCert, x.TlsClientKey) + tlsConf.ServerName = CmdAcl.Conf.GetString("tls_server_name") + + conn, err := x.SetupConnection(opt.dgraph, &tlsConf, false) + x.Checkf(err, "While trying to setup connection to Dgraph alpha.") + + dc := api.NewDgraphClient(conn) + return dgo.NewDgraphClient(dc), func() { + if err := conn.Close(); err != nil { + fmt.Printf("Error while closing connection: %v\n", err) + } + } +} + func getClientWithUserCtx(userid string, passwordOpt string, conf *viper.Viper) (*dgo.Dgraph, CloseFunc, error) { password := conf.GetString(passwordOpt) @@ -188,7 +214,7 @@ func getClientWithUserCtx(userid string, passwordOpt string, conf *viper.Viper) } dc, closeClient := getDgraphClient(conf) - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) cleanFunc := func() { cancel()