Fix: handling of no inline policy for service acc. (#3221)

This commit is contained in:
Aditya Manthramurthy
2024-02-29 12:51:12 -08:00
committed by GitHub
parent 0df9487527
commit 0afea63994
5 changed files with 15 additions and 74 deletions

View File

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

View File

@@ -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,

View File

@@ -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
}

View File

@@ -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, "")

View File

@@ -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))