diff --git a/restapi/admin_arns_test.go b/restapi/admin_arns_test.go index f39b2f319..8f6ea0151 100644 --- a/restapi/admin_arns_test.go +++ b/restapi/admin_arns_test.go @@ -49,6 +49,6 @@ func TestArnsList(t *testing.T) { arnsList, err = getArns(ctx, adminClient) assert.Nil(arnsList, "arn list was not returned nil") - assert.NotNil(err, "An error should have ben returned") + assert.NotNil(err, "An error should have been returned") } diff --git a/restapi/admin_users.go b/restapi/admin_users.go index 11e6eccbc..cc269fd60 100644 --- a/restapi/admin_users.go +++ b/restapi/admin_users.go @@ -17,6 +17,10 @@ package restapi import ( + "sort" + + iampolicy "github.com/minio/minio/pkg/iam/policy" + "github.com/go-openapi/errors" "github.com/go-openapi/runtime/middleware" "github.com/minio/console/models" @@ -30,6 +34,10 @@ import ( "strings" ) +const Allow = 1 +const Deny = -1 +const Unknown = 0 + func registerUsersHandlers(api *operations.ConsoleAPI) { // List Users api.AdminAPIListUsersHandler = admin_api.ListUsersHandlerFunc(func(params admin_api.ListUsersParams, session *models.Principal) middleware.Responder { @@ -487,23 +495,49 @@ func getListUsersWithAccessToBucketResponse(session *models.Principal, bucket st // defining the client to be used adminClient := adminClient{client: mAdmin} - users, err := listUsers(ctx, adminClient) + return listUsersWithAccessToBucket(ctx, adminClient, bucket) +} + +func policyAllowsAndMatchesBucket(policy *iampolicy.Policy, bucket string) int { + policyStatements := policy.Statements + for i := 0; i < len(policyStatements); i++ { + resources := policyStatements[i].Resources + effect := policyStatements[i].Effect + if resources.Match(bucket, map[string][]string{}) { + if effect.IsValid() { + if effect.IsAllowed(true) { + return Allow + } + return Deny + } + } + } + return Unknown +} + +func listUsersWithAccessToBucket(ctx context.Context, adminClient MinioAdmin, bucket string) ([]string, *models.Error) { + users, err := adminClient.listUsers(ctx) if err != nil { return nil, prepareError(err) } var retval []string - seen := make(map[string]bool) - for i := 0; i < len(users); i++ { - for _, policyName := range users[i].Policy { + akHasAccess := make(map[string]bool) + akIsDenied := make(map[string]bool) + for k, v := range users { + for _, policyName := range strings.Split(v.PolicyName, ",") { + policyName = strings.TrimSpace(policyName) policy, err := adminClient.getPolicy(ctx, policyName) if err == nil { - parsedPolicy, err2 := parsePolicy(policyName, policy) - if err2 == nil && policyMatchesBucket(parsedPolicy, bucket) && !seen[users[i].AccessKey] { - retval = append(retval, users[i].AccessKey) - seen[users[i].AccessKey] = true - } - if err2 != nil { - log.Println(err2) + if !akIsDenied[k] { + switch policyAllowsAndMatchesBucket(policy, bucket) { + case Allow: + if !akHasAccess[k] { + akHasAccess[k] = true + } + case Deny: + akIsDenied[k] = true + akHasAccess[k] = false + } } } else { log.Println(err) @@ -521,14 +555,17 @@ func getListUsersWithAccessToBucketResponse(session *models.Principal, bucket st if err == nil { policy, err2 := adminClient.getPolicy(ctx, info.Policy) if err2 == nil { - parsedPolicy, err3 := parsePolicy(info.Policy, policy) for j := 0; j < len(info.Members); j++ { - if err3 == nil && !seen[info.Members[j]] && policyMatchesBucket(parsedPolicy, bucket) { - retval = append(retval, info.Members[j]) - seen[info.Members[j]] = true - } - if err3 != nil { - log.Println(err3) + if !akIsDenied[info.Members[j]] { + switch policyAllowsAndMatchesBucket(policy, bucket) { + case Allow: + if !akHasAccess[info.Members[j]] { + akHasAccess[info.Members[j]] = true + } + case Deny: + akIsDenied[info.Members[j]] = true + akHasAccess[info.Members[j]] = false + } } } } else { @@ -538,5 +575,11 @@ func getListUsersWithAccessToBucketResponse(session *models.Principal, bucket st log.Println(err) } } + for k, v := range akHasAccess { + if v { + retval = append(retval, k) + } + } + sort.Strings(retval) return retval, nil } diff --git a/restapi/admin_users_test.go b/restapi/admin_users_test.go index 078f719c8..49f3aab9e 100644 --- a/restapi/admin_users_test.go +++ b/restapi/admin_users_test.go @@ -17,11 +17,14 @@ package restapi import ( + "bytes" "context" "fmt" "strings" "testing" + iampolicy "github.com/minio/minio/pkg/iam/policy" + "github.com/minio/minio/pkg/madmin" "errors" @@ -400,3 +403,158 @@ func TestUserGroupsBulk(t *testing.T) { assert.Equal("error in users-groups assignation: \"error,error,error\"", err.Error()) } } + +func TestListUsersWithAccessToBucket(t *testing.T) { + assert := asrt.New(t) + ctx := context.Background() + adminClient := adminClientMock{} + user1 := madmin.UserInfo{SecretKey: "testtest", + PolicyName: "consoleAdmin,testPolicy,redundantPolicy", + Status: "enabled", + MemberOf: []string{"group1"}, + } + user2 := madmin.UserInfo{SecretKey: "testtest", + PolicyName: "testPolicy, otherPolicy", + Status: "enabled", + MemberOf: []string{"group1"}, + } + mockUsers := map[string]madmin.UserInfo{"testuser1": user1, "testuser2": user2} + minioListUsersMock = func() (map[string]madmin.UserInfo, error) { + return mockUsers, nil + } + policyMap := map[string]string{ + "consoleAdmin": `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "admin:*" + ] + }, + { + "Effect": "Allow", + "Action": [ + "s3:*" + ], + "Resource": [ + "arn:aws:s3:::*" + ] + } + ] +}`, + "testPolicy": `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Deny", + "Action": [ + "s3:*" + ], + "Resource": [ + "arn:aws:s3:::bucket1" + ] + } + ] + }`, + "otherPolicy": `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:*" + ], + "Resource": [ + "arn:aws:s3:::bucket2" + ] + } + ] + }`, + "thirdPolicy": `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:*" + ], + "Resource": [ + "arn:aws:s3:::bucket3" + ] + } + ] + }`, "RedundantPolicy": `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:*" + ], + "Resource": [ + "arn:aws:s3:::bucket1" + ] + } + ] + }`, + } + minioGetPolicyMock = func(name string) (*iampolicy.Policy, error) { + iamp, err := iampolicy.ParseConfig(bytes.NewReader([]byte(policyMap[name]))) + if err != nil { + return nil, err + } + return iamp, nil + } + minioListGroupsMock = func() ([]string, error) { + return []string{"group1"}, nil + } + minioGetGroupDescriptionMock = func(name string) (*madmin.GroupDesc, error) { + if name == "group1" { + mockResponse := &madmin.GroupDesc{ + Name: "group1", + Policy: "thirdPolicy", + Members: []string{"testuser1", "testuser2"}, + Status: "enabled", + } + return mockResponse, nil + } + return nil, errorGeneric + } + type args struct { + bucket string + } + tests := []struct { + name string + args args + want []string + }{ + { + name: "Test1", + args: args{bucket: "bucket0"}, + want: []string{"testuser1"}, + }, + { + name: "Test2", + args: args{bucket: "bucket1"}, + want: []string(nil), + }, + { + name: "Test3", + args: args{bucket: "bucket2"}, + want: []string{"testuser1", "testuser2"}, + }, + { + name: "Test4", + args: args{bucket: "bucket3"}, + want: []string{"testuser1", "testuser2"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, _ := listUsersWithAccessToBucket(ctx, adminClient, tt.args.bucket) + assert.Equal(got, tt.want) + }) + } + +}