diff --git a/api/admin_client_mock.go b/api/admin_client_mock.go index 13e3be48f..531dcfc45 100644 --- a/api/admin_client_mock.go +++ b/api/admin_client_mock.go @@ -80,7 +80,7 @@ var ( minioSetUserStatusMock func(accessKey string, status madmin.AccountStatus) error minioAccountInfoMock func(ctx context.Context) (madmin.AccountInfo, error) - minioAddServiceAccountMock func(ctx context.Context, policy *iampolicy.Policy, user string, accessKey string, secretKey string, description string, name string, expiry *time.Time, status string) (madmin.Credentials, error) + minioAddServiceAccountMock func(ctx context.Context, policy string, user string, accessKey string, secretKey string, description string, name string, expiry *time.Time, status string) (madmin.Credentials, error) minioListServiceAccountsMock func(ctx context.Context, user string) (madmin.ListServiceAccountsResp, error) minioDeleteServiceAccountMock func(ctx context.Context, serviceAccount string) error minioInfoServiceAccountMock func(ctx context.Context, serviceAccount string) (madmin.InfoServiceAccountResp, error) @@ -377,7 +377,7 @@ func (ac AdminClientMock) AccountInfo(ctx context.Context) (madmin.AccountInfo, return minioAccountInfoMock(ctx) } -func (ac AdminClientMock) addServiceAccount(ctx context.Context, policy *iampolicy.Policy, user string, accessKey string, secretKey string, description string, name string, expiry *time.Time, status string) (madmin.Credentials, error) { +func (ac AdminClientMock) addServiceAccount(ctx context.Context, policy string, user string, accessKey string, secretKey string, description string, name string, expiry *time.Time, status string) (madmin.Credentials, error) { return minioAddServiceAccountMock(ctx, policy, user, accessKey, secretKey, description, name, expiry, status) } diff --git a/api/client-admin.go b/api/client-admin.go index a0f304dae..f02ddc364 100644 --- a/api/client-admin.go +++ b/api/client-admin.go @@ -70,7 +70,7 @@ type MinioAdmin interface { heal(ctx context.Context, bucket, prefix string, healOpts madmin.HealOpts, clientToken string, forceStart, forceStop bool) (healStart madmin.HealStartSuccess, healTaskStatus madmin.HealTaskStatus, err error) // Service Accounts - addServiceAccount(ctx context.Context, policy *iampolicy.Policy, user string, accessKey string, secretKey string, name string, description string, expiry *time.Time, comment string) (madmin.Credentials, error) + addServiceAccount(ctx context.Context, policy string, user string, accessKey string, secretKey string, name string, description string, expiry *time.Time, comment string) (madmin.Credentials, error) listServiceAccounts(ctx context.Context, user string) (madmin.ListServiceAccountsResp, error) deleteServiceAccount(ctx context.Context, serviceAccount string) error infoServiceAccount(ctx context.Context, serviceAccount string) (madmin.InfoServiceAccountResp, error) @@ -305,13 +305,9 @@ func (ac AdminClient) getLogs(ctx context.Context, node string, lineCnt int, log } // implements madmin.AddServiceAccount() -func (ac AdminClient) addServiceAccount(ctx context.Context, policy *iampolicy.Policy, user string, accessKey string, secretKey string, name string, description string, expiry *time.Time, comment string) (madmin.Credentials, error) { - buf, err := json.Marshal(policy) - if err != nil { - return madmin.Credentials{}, err - } +func (ac AdminClient) addServiceAccount(ctx context.Context, policy string, user string, accessKey string, secretKey string, name string, description string, expiry *time.Time, comment string) (madmin.Credentials, error) { return ac.Client.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ - Policy: buf, + Policy: []byte(policy), TargetUser: user, AccessKey: accessKey, SecretKey: secretKey, diff --git a/api/service_accounts_handlers.go b/api/service_accounts_handlers.go index d85022eea..db960cbcd 100644 --- a/api/service_accounts_handlers.go +++ b/api/service_accounts_handlers.go @@ -17,11 +17,9 @@ package api import ( - "bytes" "context" "encoding/json" "errors" - "strings" "time" "github.com/go-openapi/runtime/middleware" @@ -123,36 +121,17 @@ func registerServiceAccountsHandlers(api *operations.ConsoleAPI) { // createServiceAccount adds a service account to the userClient and assigns a policy to him if defined. func createServiceAccount(ctx context.Context, userClient MinioAdmin, policy string, name string, description string, expiry *time.Time, comment string) (*models.ServiceAccountCreds, error) { - // By default a nil policy will be used so the service account inherit the parent account policy, otherwise - // we override with the user provided iam policy - var iamPolicy *iampolicy.Policy - if strings.TrimSpace(policy) != "" { - iamp, err := iampolicy.ParseConfig(bytes.NewReader([]byte(policy))) - if err != nil { - return nil, err - } - iamPolicy = iamp - } - creds, err := userClient.addServiceAccount(ctx, iamPolicy, "", "", "", name, description, expiry, comment) + creds, err := userClient.addServiceAccount(ctx, policy, "", "", "", name, description, expiry, comment) if err != nil { return nil, err } return &models.ServiceAccountCreds{AccessKey: creds.AccessKey, SecretKey: creds.SecretKey, URL: getMinIOServer()}, nil } -// createServiceAccount adds a service account with the given credentials to the userClient and assigns a policy to him if defined. +// createServiceAccount adds a service account with the given credentials to the +// userClient and assigns a policy to him if defined. func createServiceAccountCreds(ctx context.Context, userClient MinioAdmin, policy string, accessKey string, secretKey string, name string, description string, expiry *time.Time, comment string) (*models.ServiceAccountCreds, error) { - // By default a nil policy will be used so the service account inherit the parent account policy, otherwise - // we override with the user provided iam policy - var iamPolicy *iampolicy.Policy - if strings.TrimSpace(policy) != "" { - iamp, err := iampolicy.ParseConfig(bytes.NewReader([]byte(policy))) - if err != nil { - return nil, err - } - iamPolicy = iamp - } - creds, err := userClient.addServiceAccount(ctx, iamPolicy, "", accessKey, secretKey, name, description, expiry, comment) + creds, err := userClient.addServiceAccount(ctx, policy, "", accessKey, secretKey, name, description, expiry, comment) if err != nil { return nil, err } @@ -190,18 +169,7 @@ func getCreateServiceAccountResponse(session *models.Principal, params saApi.Cre // createServiceAccount adds a service account to a given user and assigns a policy to him if defined. func createAUserServiceAccount(ctx context.Context, userClient MinioAdmin, policy string, user string, name string, description string, expiry *time.Time, comment string) (*models.ServiceAccountCreds, error) { - // By default a nil policy will be used so the service account inherit the parent account policy, otherwise - // we override with the user provided iam policy - var iamPolicy *iampolicy.Policy - if strings.TrimSpace(policy) != "" { - iamp, err := iampolicy.ParseConfig(bytes.NewReader([]byte(policy))) - if err != nil { - return nil, err - } - iamPolicy = iamp - } - - creds, err := userClient.addServiceAccount(ctx, iamPolicy, user, "", "", name, description, expiry, comment) + creds, err := userClient.addServiceAccount(ctx, policy, user, "", "", name, description, expiry, comment) if err != nil { return nil, err } @@ -209,18 +177,7 @@ func createAUserServiceAccount(ctx context.Context, userClient MinioAdmin, polic } func createAUserServiceAccountCreds(ctx context.Context, userClient MinioAdmin, policy string, user string, accessKey string, secretKey string, name string, description string, expiry *time.Time, comment string) (*models.ServiceAccountCreds, error) { - // By default a nil policy will be used so the service account inherit the parent account policy, otherwise - // we override with the user provided iam policy - var iamPolicy *iampolicy.Policy - if strings.TrimSpace(policy) != "" { - iamp, err := iampolicy.ParseConfig(bytes.NewReader([]byte(policy))) - if err != nil { - return nil, err - } - iamPolicy = iamp - } - - creds, err := userClient.addServiceAccount(ctx, iamPolicy, user, accessKey, secretKey, name, description, expiry, comment) + creds, err := userClient.addServiceAccount(ctx, policy, user, accessKey, secretKey, name, description, expiry, comment) if err != nil { return nil, err } diff --git a/api/service_accounts_handlers_test.go b/api/service_accounts_handlers_test.go index cdfd655cc..f1f5ede00 100644 --- a/api/service_accounts_handlers_test.go +++ b/api/service_accounts_handlers_test.go @@ -24,7 +24,6 @@ import ( "time" "github.com/minio/madmin-go/v3" - iampolicy "github.com/minio/pkg/v2/policy" "github.com/stretchr/testify/assert" ) @@ -41,7 +40,7 @@ func TestAddServiceAccount(t *testing.T) { AccessKey: "minio", SecretKey: "minio123", } - minioAddServiceAccountMock = func(_ context.Context, _ *iampolicy.Policy, _ string, _ string, _ string, _ string, _ string, _ *time.Time, _ string) (madmin.Credentials, error) { + minioAddServiceAccountMock = func(_ context.Context, _ string, _ string, _ string, _ string, _ string, _ string, _ *time.Time, _ string) (madmin.Credentials, error) { return mockResponse, nil } saCreds, err := createServiceAccount(ctx, client, policyDefinition, "", "", nil, "") @@ -51,25 +50,13 @@ func TestAddServiceAccount(t *testing.T) { assert.Equal(mockResponse.AccessKey, saCreds.AccessKey, fmt.Sprintf("Failed on %s:, error occurred: AccessKey differ", function)) assert.Equal(mockResponse.SecretKey, saCreds.SecretKey, fmt.Sprintf("Failed on %s:, error occurred: SecretKey differ", function)) - // Test-2: if an invalid policy is assigned to the service account, this will raise an error - policyDefinition = "invalid policy" - mockResponse = madmin.Credentials{ - AccessKey: "minio", - SecretKey: "minio123", - } - minioAddServiceAccountMock = func(_ context.Context, _ *iampolicy.Policy, _ string, _ string, _ string, _ string, _ string, _ *time.Time, _ string) (madmin.Credentials, error) { - return mockResponse, nil - } - _, err = createServiceAccount(ctx, client, policyDefinition, "", "", nil, "") - assert.Error(err) - - // Test-3: if an error occurs on server while creating service account (valid policy), handle it + // Test-2: if an error occurs on server while creating service account (valid policy), handle it policyDefinition = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Action\":[\"s3:GetBucketLocation\",\"s3:GetObject\",\"s3:ListAllMyBuckets\"],\"Resource\":[\"arn:aws:s3:::bucket1/*\"]}]}" mockResponse = madmin.Credentials{ AccessKey: "minio", SecretKey: "minio123", } - minioAddServiceAccountMock = func(_ context.Context, _ *iampolicy.Policy, _ string, _ string, _ string, _ string, _ string, _ *time.Time, _ string) (madmin.Credentials, error) { + minioAddServiceAccountMock = func(_ context.Context, _ string, _ string, _ string, _ string, _ string, _ string, _ *time.Time, _ string) (madmin.Credentials, error) { return madmin.Credentials{}, errors.New("error") } _, err = createServiceAccount(ctx, client, policyDefinition, "", "", nil, "") diff --git a/integration/objects_test.go b/integration/objects_test.go index 38aa261e3..2c304bbe3 100644 --- a/integration/objects_test.go +++ b/integration/objects_test.go @@ -186,6 +186,7 @@ func TestObjectGet(t *testing.T) { } response, err := client.Do(request) + fmt.Printf("Console server Response: %v\nErr: %v\n", response, err) assert.NotNil(response, fmt.Sprintf("%s response object is nil", tt.name)) assert.Nil(err, fmt.Sprintf("%s returned an error: %v", tt.name, err))