Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving ACL #3124

Merged
merged 12 commits into from
Mar 13, 2019
Merged

Improving ACL #3124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0818557
Print latest info about a user or group after modifications
Mar 9, 2019
29b1fd3
Using types to distinguish user and group
Mar 11, 2019
78d233a
Using type to distinguish user and group
Mar 11, 2019
efd8d72
Checking if the option --group_list is provided to invoke userMod
Mar 11, 2019
01426ee
Updating error message to include the perm value 7
Mar 11, 2019
7499c77
Reduce line width to below 100 chars
Mar 11, 2019
c35c6af
Addressed Manish's comments
Mar 12, 2019
12f29e6
Fixed tests, and correctly checking if the group list is set
Mar 12, 2019
2c1c072
Removed unused parameter
Mar 12, 2019
9d1dbae
Adding timeout on context for all commands
Mar 13, 2019
e9acbd9
Merge branch 'master' into gitlw/distinguish_xid
Mar 13, 2019
8a150c2
Merge branch 'master' into gitlw/distinguish_xid
Mar 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions edgraph/access_ee.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
164 changes: 130 additions & 34 deletions ee/acl/acl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"fmt"
"regexp"
"strings"
"time"

"github.com/dgraph-io/dgo"
"github.com/dgraph-io/dgo/protos/api"
Expand All @@ -41,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)
}
Expand Down Expand Up @@ -87,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 {
Expand All @@ -103,17 +112,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,
Expand All @@ -135,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 {
Expand All @@ -157,6 +157,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{
Expand Down Expand Up @@ -202,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 {
Expand Down Expand Up @@ -244,17 +250,25 @@ 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)
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 newPassword {
return changePassword(conf, userId)
}
return changePassword(conf, userId)

return userMod(conf, userId, groupList)
}

// when modifying the group, some user options are forbidden
Expand All @@ -274,16 +288,13 @@ 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()
ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Second)
defer ctxCancel()
txn := dc.NewTxn()
defer func() {
if err := txn.Discard(ctx); err != nil {
Expand Down Expand Up @@ -325,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 {
Expand Down Expand Up @@ -385,8 +397,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 {
Expand All @@ -401,6 +414,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 {
Expand All @@ -415,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 {
Expand Down Expand Up @@ -479,13 +496,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 {
Expand Down Expand Up @@ -533,7 +551,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, ", "))

Expand Down Expand Up @@ -580,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)
}
37 changes: 6 additions & 31 deletions ee/acl/acl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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")
Expand Down Expand Up @@ -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 {
Expand All @@ -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",
Expand Down
Loading