adding test case for bucket users (#773)

Co-authored-by: Minio Trusted <trusted@minio.io>
Co-authored-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>

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")
 
 }

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 !akIsDenied[k] {
+					switch policyAllowsAndMatchesBucket(policy, bucket) {
+					case Allow:
+						if !akHasAccess[k] {
+							akHasAccess[k] = true
+						}
+					case Deny:
+						akIsDenied[k] = true
+						akHasAccess[k] = false
 					}
-				if err2 != nil {
-					log.Println(err2)
 				}
 			} 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 !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
 						}
-					if err3 != nil {
-						log.Println(err3)
 					}
 				}
 			} 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
 }

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)
+		})
+	}
+
+}