do not remove Sid from svcaccount policies (#14064)

fixes #13905

cmd/admin-handler-utils.go

@@ -102,6 +102,18 @@ func toAdminAPIErr(ctx context.Context, err error) APIError {
 				Description:    err.Error(),
 				HTTPStatusCode: http.StatusForbidden,
 			}
+		case errors.Is(err, errIAMServiceAccount):
+			apiErr = APIError{
+				Code:           "XMinioIAMServiceAccount",
+				Description:    err.Error(),
+				HTTPStatusCode: http.StatusBadRequest,
+			}
+		case errors.Is(err, errIAMServiceAccountUsed):
+			apiErr = APIError{
+				Code:           "XMinioIAMServiceAccountUsed",
+				Description:    err.Error(),
+				HTTPStatusCode: http.StatusBadRequest,
+			}
 		case errors.Is(err, errIAMNotInitialized):
 			apiErr = APIError{
 				Code:           "XMinioIAMNotInitialized",

cmd/admin-handlers-users.go

@@ -816,18 +816,15 @@ func (a adminAPIHandlers) InfoServiceAccount(w http.ResponseWriter, r *http.Requ
 
 	var svcAccountPolicy iampolicy.Policy
 
-	impliedPolicy := policy == nil
+	if policy != nil {
-
+		svcAccountPolicy = *policy
-	// If policy is empty, check for policy of the parent user
-	if !impliedPolicy {
-		svcAccountPolicy = svcAccountPolicy.Merge(*policy)
 	} else {
 		policiesNames, err := globalIAMSys.PolicyDBGet(svcAccount.ParentUser, false)
 		if err != nil {
 			writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
 			return
 		}
-		svcAccountPolicy = svcAccountPolicy.Merge(globalIAMSys.GetCombinedPolicy(policiesNames...))
+		svcAccountPolicy = globalIAMSys.GetCombinedPolicy(policiesNames...)
 	}
 
 	policyJSON, err := json.MarshalIndent(svcAccountPolicy, "", " ")
@@ -839,7 +836,7 @@ func (a adminAPIHandlers) InfoServiceAccount(w http.ResponseWriter, r *http.Requ
 	infoResp := madmin.InfoServiceAccountResp{
 		ParentUser:    svcAccount.ParentUser,
 		AccountStatus: svcAccount.Status,
-		ImpliedPolicy: impliedPolicy,
+		ImpliedPolicy: policy == nil,
 		Policy:        string(policyJSON),
 	}
 

cmd/bucket-policy-handlers_test.go

@@ -37,20 +37,25 @@ import (
 func getAnonReadOnlyBucketPolicy(bucketName string) *policy.Policy {
 	return &policy.Policy{
 		Version: policy.DefaultVersion,
-		Statements: []policy.Statement{policy.NewStatement(
+		Statements: []policy.Statement{
+			policy.NewStatement(
+				"",
 				policy.Allow,
 				policy.NewPrincipal("*"),
 				policy.NewActionSet(policy.GetBucketLocationAction, policy.ListBucketAction),
 				policy.NewResourceSet(policy.NewResource(bucketName, "")),
 				condition.NewFunctions(),
-		)},
+			),
+		},
 	}
 }
 
 func getAnonWriteOnlyBucketPolicy(bucketName string) *policy.Policy {
 	return &policy.Policy{
 		Version: policy.DefaultVersion,
-		Statements: []policy.Statement{policy.NewStatement(
+		Statements: []policy.Statement{
+			policy.NewStatement(
+				"",
 				policy.Allow,
 				policy.NewPrincipal("*"),
 				policy.NewActionSet(
@@ -59,27 +64,33 @@ func getAnonWriteOnlyBucketPolicy(bucketName string) *policy.Policy {
 				),
 				policy.NewResourceSet(policy.NewResource(bucketName, "")),
 				condition.NewFunctions(),
-		)},
+			),
+		},
 	}
 }
 
 func getAnonReadOnlyObjectPolicy(bucketName, prefix string) *policy.Policy {
 	return &policy.Policy{
 		Version: policy.DefaultVersion,
-		Statements: []policy.Statement{policy.NewStatement(
+		Statements: []policy.Statement{
+			policy.NewStatement(
+				"",
 				policy.Allow,
 				policy.NewPrincipal("*"),
 				policy.NewActionSet(policy.GetObjectAction),
 				policy.NewResourceSet(policy.NewResource(bucketName, prefix)),
 				condition.NewFunctions(),
-		)},
+			),
+		},
 	}
 }
 
 func getAnonWriteOnlyObjectPolicy(bucketName, prefix string) *policy.Policy {
 	return &policy.Policy{
 		Version: policy.DefaultVersion,
-		Statements: []policy.Statement{policy.NewStatement(
+		Statements: []policy.Statement{
+			policy.NewStatement(
+				"",
 				policy.Allow,
 				policy.NewPrincipal("*"),
 				policy.NewActionSet(
@@ -90,7 +101,8 @@ func getAnonWriteOnlyObjectPolicy(bucketName, prefix string) *policy.Policy {
 				),
 				policy.NewResourceSet(policy.NewResource(bucketName, prefix)),
 				condition.NewFunctions(),
-		)},
+			),
+		},
 	}
 }
 

cmd/gateway/azure/gateway-azure.go

@@ -1416,6 +1416,7 @@ func (a *azureObjects) GetBucketPolicy(ctx context.Context, bucket string) (*pol
 		Version: policy.DefaultVersion,
 		Statements: []policy.Statement{
 			policy.NewStatement(
+				"",
 				policy.Allow,
 				policy.NewPrincipal("*"),
 				policy.NewActionSet(

cmd/gateway/gcs/gateway-gcs.go

@@ -1470,6 +1470,7 @@ func (l *gcsGateway) GetBucketPolicy(ctx context.Context, bucket string) (*polic
 		Version: policy.DefaultVersion,
 		Statements: []policy.Statement{
 			policy.NewStatement(
+				"",
 				policy.Allow,
 				policy.NewPrincipal("*"),
 				actionSet,

cmd/iam-store.go

@@ -1084,13 +1084,14 @@ func filterPolicies(cache *iamCache, policyName string, bucketName string) (stri
 			continue
 		}
 		p, found := cache.iamPolicyDocsMap[policy]
-		if found {
+		if !found {
+			continue
+		}
 		if bucketName == "" || p.Policy.MatchResource(bucketName) {
 			policies = append(policies, policy)
 			combinedPolicy = combinedPolicy.Merge(p.Policy)
 		}
 	}
-	}
 	return strings.Join(policies, ","), combinedPolicy
 }
 
@@ -1511,13 +1512,16 @@ func (store *IAMStoreSys) AddServiceAccount(ctx context.Context, cred auth.Crede
 	// Found newly requested service account, to be an existing account -
 	// reject such operation (updates to the service account are handled in
 	// a different API).
-	if _, found := cache.iamUsersMap[accessKey]; found {
+	if scred, found := cache.iamUsersMap[accessKey]; found {
-		return errIAMActionNotAllowed
+		if scred.ParentUser != parentUser {
+			return errIAMServiceAccountUsed
+		}
+		return errIAMServiceAccount
 	}
 
 	// Parent user must not be a service account.
 	if cr, found := cache.iamUsersMap[parentUser]; found && cr.IsServiceAccount() {
-		return errIAMActionNotAllowed
+		return errIAMServiceAccount
 	}
 
 	u := newUserIdentity(cred)

cmd/iam.go

@@ -883,18 +883,19 @@ func (sys *IAMSys) getServiceAccount(ctx context.Context, accessKey string) (aut
 	var embeddedPolicy *iampolicy.Policy
 
 	jwtClaims, err := auth.ExtractClaims(sa.SessionToken, globalActiveCred.SecretKey)
-	if err == nil {
+	if err != nil {
+		return auth.Credentials{}, nil, err
+	}
 	pt, ptok := jwtClaims.Lookup(iamPolicyClaimNameSA())
 	sp, spok := jwtClaims.Lookup(iampolicy.SessionPolicyName)
 	if ptok && spok && pt == "embedded-policy" {
 		policyBytes, err := base64.StdEncoding.DecodeString(sp)
-			if err == nil {
+		if err != nil {
-				p, err := iampolicy.ParseConfig(bytes.NewReader(policyBytes))
+			return auth.Credentials{}, nil, err
-				if err == nil {
-					policy := iampolicy.Policy{}.Merge(*p)
-					embeddedPolicy = &policy
-				}
 		}
+		embeddedPolicy, err = iampolicy.ParseConfig(bytes.NewReader(policyBytes))
+		if err != nil {
+			return auth.Credentials{}, nil, err
 		}
 	}
 

cmd/policy_test.go

@@ -31,14 +31,14 @@ func TestPolicySysIsAllowed(t *testing.T) {
 	p := &policy.Policy{
 		Version: policy.DefaultVersion,
 		Statements: []policy.Statement{
-			policy.NewStatement(
+			policy.NewStatement("",
 				policy.Allow,
 				policy.NewPrincipal("*"),
 				policy.NewActionSet(policy.GetBucketLocationAction),
 				policy.NewResourceSet(policy.NewResource("mybucket", "")),
 				condition.NewFunctions(),
 			),
-			policy.NewStatement(
+			policy.NewStatement("",
 				policy.Allow,
 				policy.NewPrincipal("*"),
 				policy.NewActionSet(policy.PutObjectAction),
@@ -164,14 +164,14 @@ func TestPolicyToBucketAccessPolicy(t *testing.T) {
 	case1Policy := &policy.Policy{
 		Version: policy.DefaultVersion,
 		Statements: []policy.Statement{
-			policy.NewStatement(
+			policy.NewStatement("",
 				policy.Allow,
 				policy.NewPrincipal("*"),
 				policy.NewActionSet(policy.GetBucketLocationAction, policy.ListBucketAction),
 				policy.NewResourceSet(policy.NewResource("mybucket", "")),
 				condition.NewFunctions(),
 			),
-			policy.NewStatement(
+			policy.NewStatement("",
 				policy.Allow,
 				policy.NewPrincipal("*"),
 				policy.NewActionSet(policy.GetObjectAction),
@@ -199,7 +199,7 @@ func TestPolicyToBucketAccessPolicy(t *testing.T) {
 	case3Policy := &policy.Policy{
 		Version: "12-10-2012",
 		Statements: []policy.Statement{
-			policy.NewStatement(
+			policy.NewStatement("",
 				policy.Allow,
 				policy.NewPrincipal("*"),
 				policy.NewActionSet(policy.PutObjectAction),
@@ -244,14 +244,14 @@ func TestBucketAccessPolicyToPolicy(t *testing.T) {
 	case1Result := &policy.Policy{
 		Version: policy.DefaultVersion,
 		Statements: []policy.Statement{
-			policy.NewStatement(
+			policy.NewStatement("",
 				policy.Allow,
 				policy.NewPrincipal("*"),
 				policy.NewActionSet(policy.GetBucketLocationAction, policy.ListBucketAction),
 				policy.NewResourceSet(policy.NewResource("mybucket", "")),
 				condition.NewFunctions(),
 			),
-			policy.NewStatement(
+			policy.NewStatement("",
 				policy.Allow,
 				policy.NewPrincipal("*"),
 				policy.NewActionSet(policy.GetObjectAction),

cmd/typed-errors.go

@@ -91,6 +91,12 @@ var errTooManyPolicies = errors.New("Only a single policy may be specified here.
 // error returned in IAM subsystem when an external users systems is configured.
 var errIAMActionNotAllowed = errors.New("Specified IAM action is not allowed")
 
+// error returned in IAM service account
+var errIAMServiceAccount = errors.New("Specified service account cannot be updated in this API call")
+
+// error returned in IAM service account is already used.
+var errIAMServiceAccountUsed = errors.New("Specified service account is used by another user")
+
 // error returned in IAM subsystem when IAM sub-system is still being initialized.
 var errIAMNotInitialized = errors.New("IAM sub-system is being initialized, please try again")
 

go.mod

@@ -52,7 +52,7 @@ require (
 	github.com/minio/madmin-go v1.2.4
 	github.com/minio/minio-go/v7 v7.0.20
 	github.com/minio/parquet-go v1.1.0
-	github.com/minio/pkg v1.1.11
+	github.com/minio/pkg v1.1.14
 	github.com/minio/selfupdate v0.4.0
 	github.com/minio/sha256-simd v1.0.0
 	github.com/minio/simdjson-go v0.2.1

go.sum

@@ -1114,8 +1114,9 @@ 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.11 h1:FQMYrOZzZenQUVwvxM0YjoAZB15Wb+m3TZSu33Lynx0=
 github.com/minio/pkg v1.1.11/go.mod h1:2WJAxesjzmPK9MnLZKm5n1hVYfBg04f2GQs6N5ImNU8=
+github.com/minio/pkg v1.1.14 h1:9tGPH6p0YEsIOHOY3sh2Yl5Dn0JNTkNOdZW/p+MKIi4=
+github.com/minio/pkg v1.1.14/go.mod h1:2WJAxesjzmPK9MnLZKm5n1hVYfBg04f2GQs6N5ImNU8=
 github.com/minio/selfupdate v0.3.1/go.mod h1:b8ThJzzH7u2MkF6PcIra7KaXO9Khf6alWPvMSyTDCFM=
 github.com/minio/selfupdate v0.4.0 h1:A7t07pN4Ch1tBTIRStW0KhUVyykz+2muCqFsITQeEW8=
 github.com/minio/selfupdate v0.4.0/go.mod h1:mcDkzMgq8PRcpCRJo/NlPY7U45O5dfYl2Y0Rg7IustY=