fix: policy handling with dynamic policy variables (#1226)

Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>
This commit is contained in:
Daniel Valdivia
2021-11-15 12:24:37 -08:00
committed by GitHub
parent 70845c0ec5
commit b8417fb7a0
6 changed files with 45 additions and 43 deletions

2
go.mod
View File

@@ -24,7 +24,7 @@ require (
github.com/minio/minio-go/v7 v7.0.15
github.com/minio/operator v0.0.0-20211011212245-31460bbbc4b7
github.com/minio/operator/logsearchapi v0.0.0-20211011212245-31460bbbc4b7
github.com/minio/pkg v1.1.5
github.com/minio/pkg v1.1.7
github.com/minio/selfupdate v0.3.1
github.com/mitchellh/go-homedir v1.1.0
github.com/rs/xid v1.3.0

4
go.sum
View File

@@ -929,8 +929,8 @@ github.com/minio/pkg v1.0.3/go.mod h1:obU54TZ9QlMv0TRaDgQ/JTzf11ZSXxnSfLrm4tMtBP
github.com/minio/pkg v1.0.4/go.mod h1:obU54TZ9QlMv0TRaDgQ/JTzf11ZSXxnSfLrm4tMtBP8=
github.com/minio/pkg v1.0.11/go.mod h1:32x/3OmGB0EOi1N+3ggnp+B5VFkSBBB9svPMVfpnf14=
github.com/minio/pkg v1.1.3/go.mod h1:32x/3OmGB0EOi1N+3ggnp+B5VFkSBBB9svPMVfpnf14=
github.com/minio/pkg v1.1.5 h1:phwKkJBQdVLyxOXC3RChPVGLtebplzQJ5jJ3l/HBvnk=
github.com/minio/pkg v1.1.5/go.mod h1:32x/3OmGB0EOi1N+3ggnp+B5VFkSBBB9svPMVfpnf14=
github.com/minio/pkg v1.1.7 h1:v+2/ol/h1Sl0iJdOFN1Srk4CzksMIDsfugXCZYb5L7Y=
github.com/minio/pkg v1.1.7/go.mod h1:32x/3OmGB0EOi1N+3ggnp+B5VFkSBBB9svPMVfpnf14=
github.com/minio/selfupdate v0.3.1 h1:BWEFSNnrZVMUWXbXIgLDNDjbejkmpAmZvy/nCz1HlEs=
github.com/minio/selfupdate v0.3.1/go.mod h1:b8ThJzzH7u2MkF6PcIra7KaXO9Khf6alWPvMSyTDCFM=
github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM=

View File

@@ -69,10 +69,26 @@ func TestListPolicies(t *testing.T) {
adminClient := adminClientMock{}
// mock function response from listPolicies()
minioListPoliciesMock = func() (map[string]*iampolicy.Policy, error) {
var readonly iampolicy.Policy
var readwrite iampolicy.Policy
var diagnostis iampolicy.Policy
for _, p := range iampolicy.DefaultPolicies {
switch p.Name {
case "readonly":
readonly = p.Definition
case "readwrite":
readwrite = p.Definition
case "diagnostics":
diagnostis = p.Definition
}
}
return map[string]*iampolicy.Policy{
"readonly": &iampolicy.ReadOnly,
"readwrite": &iampolicy.ReadWrite,
"diagnostics": &iampolicy.AdminDiagnostics,
"readonly": &readonly,
"readwrite": &readwrite,
"diagnostics": &diagnostis,
}, nil
}
// Test-1 : listPolicies() Get response from minio client with three Canned Policies and return the same number on listPolicies()

View File

@@ -458,7 +458,7 @@ func listExternalBucketsResponse(params user_api.ListExternalBucketsParams) (*mo
// create a minioClient interface implementation
// defining the client to be used
remoteClient := AdminClient{Client: remoteAdmin}
buckets, err := getAccountInfo(ctx, remoteClient)
buckets, err := getAccountBuckets(ctx, remoteClient)
if err != nil {
return nil, prepareError(err)
}

View File

@@ -24,6 +24,8 @@ import (
"strings"
"time"
"github.com/minio/pkg/bucket/policy/condition"
"github.com/minio/console/pkg/acl"
"github.com/minio/mc/cmd"
@@ -287,8 +289,8 @@ func getBucketVersionedResponse(session *models.Principal, bucketName string) (*
return bucketVResponse, nil
}
// getAccountInfo fetches a list of all buckets allowed to that particular client from MinIO Servers
func getAccountInfo(ctx context.Context, client MinioAdmin) ([]*models.Bucket, error) {
// getAccountBuckets fetches a list of all buckets allowed to that particular client from MinIO Servers
func getAccountBuckets(ctx context.Context, client MinioAdmin) ([]*models.Bucket, error) {
info, err := client.AccountInfo(ctx)
if err != nil {
return []*models.Bucket{}, err
@@ -374,7 +376,7 @@ func getListBucketsResponse(session *models.Principal) (*models.ListBucketsRespo
// create a minioClient interface implementation
// defining the client to be used
adminClient := AdminClient{Client: mAdmin}
buckets, err := getAccountInfo(ctx, adminClient)
buckets, err := getAccountBuckets(ctx, adminClient)
if err != nil {
return nil, prepareError(err)
}
@@ -523,7 +525,7 @@ func getBucketSetPolicyResponse(session *models.Principal, bucketName string, re
return nil, prepareError(err)
}
// get updated bucket details and return it
bucket, err := getBucketInfo(ctx, minioClient, adminClient, bucketName)
bucket, err := getBucketInfo(ctx, minioClient, adminClient, bucketName, session.AccountAccessKey)
if err != nil {
return nil, prepareError(err)
}
@@ -581,37 +583,22 @@ func getDeleteBucketResponse(session *models.Principal, params user_api.DeleteBu
return nil
}
func getPolicyActionSetForBucket(bucketName string, statement []minioIAMPolicy.Statement) minioIAMPolicy.ActionSet {
bucketActions := minioIAMPolicy.ActionSet{}
bucketNameARN := fmt.Sprintf("arn:aws:s3:::%s/*", bucketName)
for _, st := range statement {
if st.Effect == "Allow" {
if len(st.Resources.ToSlice()) == 0 {
mergedActions := append(bucketActions.ToSlice(), st.Actions.ToSlice()...)
bucketActions = minioIAMPolicy.NewActionSet(mergedActions...)
} else {
for _, resource := range st.Resources.ToSlice() {
resourceName := resource.String()
if resourceName == bucketNameARN || resourceName == "arn:aws:s3:::*" {
mergedActions := append(bucketActions.ToSlice(), st.Actions.ToSlice()...)
bucketActions = minioIAMPolicy.NewActionSet(mergedActions...)
}
}
}
}
}
return bucketActions
}
// getBucketInfo return bucket information including name, policy access, size and creation date
func getBucketInfo(ctx context.Context, client MinioClient, adminClient MinioAdmin, bucketName string) (*models.Bucket, error) {
func getBucketInfo(ctx context.Context, client MinioClient, adminClient MinioAdmin, bucketName string, accountName string) (*models.Bucket, error) {
// Get Account Policy
policyInfo, err := getAccountPolicy(ctx, adminClient)
if err != nil {
return nil, err
}
var bucketAdminRole bool
// Retrieve list of allowed bucketActionsArray on the bucket
bucketActions := getPolicyActionSetForBucket(bucketName, policyInfo.Statements)
// TODO: Add all the possible variables
conditionValues := map[string][]string{
condition.AWSUsername.Name(): {accountName},
}
bucketActions := policyInfo.IsAllowedActions(bucketName, "", conditionValues)
// Check if one of these bucketActionsArray belongs to administrative bucketActionsArray
bucketAdminRoleActions := bucketActions.Intersection(acl.BucketAdminRole)
bucketAdminRole = len(bucketAdminRoleActions) > 0
@@ -671,7 +658,6 @@ func getBucketInfo(ctx context.Context, client MinioClient, adminClient MinioAdm
func getBucketInfoResponse(session *models.Principal, params user_api.BucketInfoParams) (*models.Bucket, *models.Error) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*20)
defer cancel()
mClient, err := newMinioClient(session)
if err != nil {
return nil, prepareError(err)
@@ -688,7 +674,7 @@ func getBucketInfoResponse(session *models.Principal, params user_api.BucketInfo
// defining the client to be used
adminClient := AdminClient{Client: mAdmin}
bucket, err := getBucketInfo(ctx, minioClient, adminClient, params.Name)
bucket, err := getBucketInfo(ctx, minioClient, adminClient, params.Name, session.AccountAccessKey)
if err != nil {
return nil, prepareError(err)
}

View File

@@ -180,7 +180,7 @@ func TestListBucket(t *testing.T) {
// get list buckets response this response should have Name, CreationDate, Size and Access
// as part of of each bucket
function := "getaAcountUsageInfo()"
bucketList, err := getAccountInfo(ctx, adminClient)
bucketList, err := getAccountBuckets(ctx, adminClient)
if err != nil {
t.Errorf("Failed on %s:, error occurred: %s", function, err.Error())
}
@@ -197,7 +197,7 @@ func TestListBucket(t *testing.T) {
minioAccountInfoMock = func(ctx context.Context) (madmin.AccountInfo, error) {
return madmin.AccountInfo{}, errors.New("error")
}
_, err = getAccountInfo(ctx, adminClient)
_, err = getAccountBuckets(ctx, adminClient)
if assert.Error(err) {
assert.Equal("error", err.Error())
}
@@ -308,7 +308,7 @@ func TestBucketInfo(t *testing.T) {
return mockBucketList, nil
}
bucketInfo, err := getBucketInfo(ctx, minClient, adminClient, bucketToSet)
bucketInfo, err := getBucketInfo(ctx, minClient, adminClient, bucketToSet, "user1")
if err != nil {
t.Errorf("Failed on %s:, error occurred: %s", function, err.Error())
}
@@ -330,7 +330,7 @@ func TestBucketInfo(t *testing.T) {
CreationDate: "", // to be implemented
Size: 0, // to be implemented
}
bucketInfo, err = getBucketInfo(ctx, minClient, adminClient, bucketToSet)
bucketInfo, err = getBucketInfo(ctx, minClient, adminClient, bucketToSet, "bucket1")
if err != nil {
t.Errorf("Failed on %s:, error occurred: %s", function, err.Error())
}
@@ -352,7 +352,7 @@ func TestBucketInfo(t *testing.T) {
CreationDate: "", // to be implemented
Size: 0, // to be implemented
}
bucketInfo, err = getBucketInfo(ctx, minClient, adminClient, bucketToSet)
bucketInfo, err = getBucketInfo(ctx, minClient, adminClient, bucketToSet, "bucket1")
if err != nil {
t.Errorf("Failed on %s:, error occurred: %s", function, err.Error())
}
@@ -373,7 +373,7 @@ func TestBucketInfo(t *testing.T) {
CreationDate: "", // to be implemented
Size: 0, // to be implemented
}
_, err = getBucketInfo(ctx, minClient, adminClient, bucketToSet)
_, err = getBucketInfo(ctx, minClient, adminClient, bucketToSet, "bucket1")
if assert.Error(err) {
assert.Equal("invalid character 'p' looking for beginning of value", err.Error())
}