Add admin users #2554

harshavardhana · 2018-09-25T22:00:18Z

No description provided.

codecov · 2018-09-27T02:47:57Z

Codecov Report

Merging #2554 into master will decrease coverage by 0.26%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2554      +/-   ##
==========================================
- Coverage   10.49%   10.22%   -0.27%     
==========================================
  Files         107      117      +10     
  Lines       10828    11114     +286     
==========================================
  Hits         1136     1136              
- Misses       9542     9828     +286     
  Partials      150      150

Impacted Files	Coverage Δ
cmd/admin-main.go	`0% <ø> (ø)`	⬆️
cmd/admin-users-disable.go	`0% <0%> (ø)`
cmd/find.go	`43.28% <0%> (ø)`	⬆️
cmd/admin-policies-remove.go	`0% <0%> (ø)`
cmd/admin-policies-add.go	`0% <0%> (ø)`
cmd/admin-users-remove.go	`0% <0%> (ø)`
cmd/admin-users.go	`0% <0%> (ø)`
cmd/admin-policies.go	`0% <0%> (ø)`
cmd/admin-policies-list.go	`0% <0%> (ø)`
cmd/admin-credentials.go	`0% <0%> (ø)`	⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7bbc81f...fe368dd. Read the comment docs.

vadmeste

Some comments

vadmeste · 2018-10-11T21:17:24Z

cmd/admin-users-add.go

+	e := client.AddUser(args.Get(1), args.Get(2))
+	fatalIf(probe.NewError(e).Trace(args...), "Cannot add new user")
+
+	policy, e := ioutil.ReadFile(args.Get(3))


Let's load policy first to fail before adding a user above.

vadmeste · 2018-10-11T21:18:27Z

cmd/admin-users-remove.go

+
+// checkAdminUsersRemoveSyntax - validate all the passed arguments
+func checkAdminUsersRemoveSyntax(ctx *cli.Context) {
+	if len(ctx.Args()) != 4 {


I think two arguments not four

vadmeste · 2018-10-11T21:19:05Z

cmd/admin-users-revoke.go

+
+// checkAdminUsersRevokeSyntax - validate all the passed arguments
+func checkAdminUsersRevokeSyntax(ctx *cli.Context) {
+	if len(ctx.Args()) != 4 {


Two arguments

vadmeste · 2018-10-11T21:20:33Z

cmd/admin-users-revoke.go

+	"github.com/minio/minio/pkg/madmin"
+)
+
+var adminUsersRevokeCmd = cli.Command{


Not sure, if we revoke the user, how can we reenable it again ?

poornas · 2018-10-11T21:26:29Z

cmd/admin-users-revoke.go

+	}
+}
+
+// mainAdminUsersRevoke is the handle for "mc admin users add" command.


"mc admin users add" -> "mc admin users revoke"

cmd/admin-users.go

poornas · 2018-10-11T21:29:25Z

cmd/admin-users-remove.go

+	}
+}
+
+// mainAdminUsersRemove is the handle for "mc admin users add" command.


mc admin users add" -> mc admin users remove"

harshavardhana · 2018-10-11T21:38:18Z

This PR is changing a bit

@vadmeste

PTAL @vadmeste @poornas

poornas · 2018-10-12T20:35:40Z

docs/minio-admin-complete-guide.md

@@ -273,19 +275,69 @@ FLAGS:
  Storage : Used 8.2GiB
 ```

+<a name="users"></a>
+### Command `users` - Manage regular users
+`users` command to add new, remove, enable, disable, list users on Minio server.


add new, -> add

vadmeste · 2018-10-12T22:21:45Z

cmd/admin-users-add.go

+  {{.HelpName}} - {{.Usage}}
+
+USAGE:
+  {{.HelpName}} TARGET USERNAME PASSWORD POLICYTYPE|POLICYFILE


Can we add possible values of policy type ? and a short example of policy file content

vadmeste · 2018-10-12T22:48:02Z

One little problem is when trying to upload an object, mc cp, in a bucket.. it doesn't work because for some reasons, mc want to list all buckets, readwrite policy won't guarantee it that, but users can use custom policy document probably and then fix this later

harshavardhana · 2018-10-12T22:56:18Z

One little problem is when trying to upload an object, mc cp, in a bucket.. it doesn't work because for some reasons, mc want to list all buckets, readwrite policy won't guarantee it that, but users can use custom policy document probably and then fix this later

This is something of a problem in minio-go I think it constructs policies with explicit API names but not for everything.

harshavardhana · 2018-10-13T02:23:31Z

After talking to @abperiasamy some more requirements came in blocking this for now.

poornas · 2018-10-16T00:21:39Z

Removing a revoked user gives an inconsistent error that specified key does not exist.

➜  mc git:(a52ea918) ✗ mc admin users add myss3 newus2e4r newuser123 readonly --insecure
Added used `newus2e4r` successfully.
➜  mc git:(a52ea918) ✗ mc admin users disable myss3 newus2e4r --insecure    
Disabled user `newus2e4r` successfully.
➜  mc git:(a52ea918) ✗ mc admin users remove myss3 newus2e4r --insecure     
mc: <ERROR> Cannot remove new user The specified key does not exist.

server side
Error: Object not found: .minio.sys#config/iam/users/newus2e4r/policy.json
       1: cmd/logger/logger.go:295:logger.LogIf()
       2: cmd/admin-handlers.go:733:cmd.adminAPIHandlers.RemoveUser()
       3: cmd/admin-router.go:96:cmd.(adminAPIHandlers).RemoveUser-fm()
       4: net/http/server.go:1947:http.HandlerFunc.ServeHTTP()

vadmeste

LGTM & tested

poornas

Tested & LGTM

harshavardhana mentioned this pull request Sep 25, 2018

STS, Multi-user support and OPA policy agent integration minio/minio#6168

Merged

9 tasks

harshavardhana force-pushed the users branch 2 times, most recently from 74e0409 to 35ecf85 Compare September 27, 2018 02:47

harshavardhana force-pushed the users branch from 35ecf85 to c1b79bf Compare October 2, 2018 23:27

harshavardhana mentioned this pull request Oct 10, 2018

multi user minio/minio#6607

Closed

harshavardhana force-pushed the users branch from c1b79bf to 33997b7 Compare October 11, 2018 18:50

harshavardhana changed the title ~~[WIP] Add admin users~~ Add admin users Oct 11, 2018

harshavardhana mentioned this pull request Oct 11, 2018

Add ListUsers API to list all configured users in IAM minio/minio#6619

Merged

9 tasks

deekoder requested review from vadmeste and poornas October 11, 2018 20:31

vadmeste previously requested changes Oct 11, 2018

View reviewed changes

poornas requested changes Oct 11, 2018

View reviewed changes

poornas reviewed Oct 11, 2018

View reviewed changes

cmd/admin-users.go Outdated Show resolved Hide resolved

poornas reviewed Oct 11, 2018

View reviewed changes

harshavardhana force-pushed the users branch 3 times, most recently from 4305d32 to 0de3054 Compare October 11, 2018 22:48

harshavardhana force-pushed the users branch 5 times, most recently from de9b614 to 7e731bf Compare October 12, 2018 17:20

poornas reviewed Oct 12, 2018

View reviewed changes

harshavardhana force-pushed the users branch from 7e731bf to 4d5e90f Compare October 12, 2018 21:57

deekoder added the priority: high label Oct 12, 2018

vadmeste reviewed Oct 12, 2018

View reviewed changes

harshavardhana force-pushed the users branch from 4d5e90f to a52ea91 Compare October 12, 2018 23:57

harshavardhana added the pending discussion label Oct 13, 2018

deekoder removed the pending discussion label Oct 15, 2018

harshavardhana force-pushed the users branch from a52ea91 to 593a72b Compare October 15, 2018 22:15

harshavardhana mentioned this pull request Oct 15, 2018

Add canned policy support minio/minio#6637

Merged

8 tasks

harshavardhana force-pushed the users branch 2 times, most recently from 5a675bd to 9e3f098 Compare October 15, 2018 23:54

harshavardhana force-pushed the users branch 3 times, most recently from 9df99b5 to 3edcf4f Compare October 16, 2018 18:52

harshavardhana mentioned this pull request Oct 16, 2018

Fix toggling users status minio/minio#6640

Merged

9 tasks

Add admin users

fe368dd

harshavardhana force-pushed the users branch from 3edcf4f to fe368dd Compare October 16, 2018 19:51

vadmeste approved these changes Oct 16, 2018

View reviewed changes

poornas approved these changes Oct 16, 2018

View reviewed changes

deekoder merged commit b7e87b6 into minio:master Oct 16, 2018

harshavardhana deleted the users branch October 16, 2018 21:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add admin users #2554

Add admin users #2554

harshavardhana commented Sep 25, 2018

codecov bot commented Sep 27, 2018 •

edited

Loading

vadmeste left a comment

vadmeste Oct 11, 2018

vadmeste Oct 11, 2018

vadmeste Oct 11, 2018

vadmeste Oct 11, 2018

poornas Oct 11, 2018

poornas Oct 11, 2018

harshavardhana commented Oct 11, 2018

poornas Oct 12, 2018

vadmeste Oct 12, 2018

vadmeste commented Oct 12, 2018

harshavardhana commented Oct 12, 2018

harshavardhana commented Oct 13, 2018

poornas commented Oct 16, 2018

vadmeste left a comment

poornas left a comment

Add admin users #2554

Add admin users #2554

Conversation

harshavardhana commented Sep 25, 2018

codecov bot commented Sep 27, 2018 • edited Loading

Codecov Report

vadmeste left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshavardhana commented Oct 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vadmeste commented Oct 12, 2018

harshavardhana commented Oct 12, 2018

harshavardhana commented Oct 13, 2018

poornas commented Oct 16, 2018

vadmeste left a comment

Choose a reason for hiding this comment

poornas left a comment

Choose a reason for hiding this comment

codecov bot commented Sep 27, 2018 •

edited

Loading