Improving ACL #3124
Improving ACL #3124
Conversation
There was a problem hiding this comment.
Reviewed 3 of 4 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gitlw and @srfrog)
ee/acl/acl.go, line 239 at r2 (raw file):
} func isFlagPassed(f *pflag.FlagSet, name string) bool {
avoid checking for this.
ee/acl/lib/acl_lib.go, line 17 at r2 (raw file):
import "github.com/dgraph-io/dgo/protos/api" func GetCreateUserNQuads(userId string, password string) []*api.NQuad {
Just move it to some file within the acl package. No need for a new pkg.
CreateUserNQuads(...)
There was a problem hiding this comment.
Reviewable status: 0 of 4 files reviewed, 7 unresolved discussions (waiting on @gitlw, @manishrjain, and @srfrog)
edgraph/access_ee.go, line 372 at r3 (raw file):
// Insert Groot. createUserNQuads := acl.CreateUserNQuads(x.GrootId, "password")
Should we use a random password value instead? it looks like it's setting the default password to "password"
ee/acl/acl.go, line 387 at r3 (raw file):
} fmt.Printf("Successfully modified groups for user %v.\n", userId) fmt.Println("The latest info is:")
move this Println into queryAndPrint() ?
ee/acl/acl.go, line 484 at r3 (raw file):
fmt.Printf("Successfully changed permission for group %v on predicate %v to %v\n", groupId, predicate, perm) fmt.Println("The latest info is:")
same
ee/acl/utils.go, line 220 at r3 (raw file):
Subject: "_:newuser", Predicate: "dgraph.password", ObjectValue: &api.Value{Val: &api.Value_StrVal{StrVal: password}},
how about api.Value_PasswordVal{} for password?
There was a problem hiding this comment.
Reviewable status: 0 of 5 files reviewed, 7 unresolved discussions (waiting on @golangcibot, @manishrjain, and @srfrog)
edgraph/access_ee.go, line 372 at r3 (raw file):
Previously, srfrog (Gus) wrote…
Should we use a random password value instead? it looks like it's setting the default password to "password"
Yes we intentionally set the default password to be well known, so that operators can use it for doing the password reset operation for groot.
ee/acl/acl.go, line 413 at r1 (raw file):
Previously, golangcibot (Bot from GolangCI) wrote…
line is 101 characters (from
lll)
Done.
ee/acl/acl.go, line 239 at r2 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
avoid checking for this.
Done.
ee/acl/acl.go, line 387 at r3 (raw file):
Previously, srfrog (Gus) wrote…
move this Println into
queryAndPrint()?
The queryAndPrintUser function is also used by the info subcommand, for which I feel it's unnecessary to say "the latest info is".
ee/acl/acl.go, line 484 at r3 (raw file):
Previously, srfrog (Gus) wrote…
same
ditto
ee/acl/utils.go, line 220 at r3 (raw file):
Previously, srfrog (Gus) wrote…
how about
api.Value_PasswordVal{}for password?
If I understand correctly, Value_PasswordVal is for the value that has already been hashed. Here I'm providing the plaintext password before hashing.
ee/acl/lib/acl_lib.go, line 17 at r2 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
Just move it to some file within the acl package. No need for a new pkg.
CreateUserNQuads(...)
Done.
There was a problem hiding this comment.
Reviewable status: 0 of 5 files reviewed, 7 unresolved discussions (waiting on @gitlw, @golangcibot, @manishrjain, and @srfrog)
ee/acl/run_ee.go, line 120 at r4 (raw file):
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")
shorthand -n
ee/acl/run_ee.go, line 236 at r4 (raw file):
} ctx := context.Background()
ctx := context.WithTimeout(, 10*time.Second)
There was a problem hiding this comment.
Reviewable status: 0 of 5 files reviewed, 7 unresolved discussions (waiting on @gitlw, @golangcibot, @manishrjain, and @srfrog)
This change is