Move all IAM storage functionality into iam store type (#13567)

This reverts commit 091a7ae359.

- Ensure all actions accessing storage lock properly.

- Behavior change: policies can be deleted only when they
  are not associated with any active credentials.

Also adds fix for accidental canned policy removal that was present in the
reverted version of the change.

cmd/admin-handler-utils.go

@@ -103,6 +103,12 @@ func toAdminAPIErr(ctx context.Context, err error) APIError {
 				Description:    err.Error(),
 				HTTPStatusCode: http.StatusServiceUnavailable,
 			}
+		case errors.Is(err, errPolicyInUse):
+			apiErr = APIError{
+				Code:           "XMinioAdminPolicyInUse",
+				Description:    "The policy cannot be removed, as it is in use",
+				HTTPStatusCode: http.StatusBadRequest,
+			}
 		case errors.Is(err, kes.ErrKeyExists):
 			apiErr = APIError{
 				Code:           "XMinioKMSKeyExists",

cmd/admin-handlers-users_test.go

@@ -258,10 +258,20 @@ func (s *TestSuiteIAM) TestPolicyCreate(c *check) {
 		c.Fatalf("policy was missing!")
 	}
 
-	// 5. Check that policy can be deleted.
+	// 5. Check that policy cannot be deleted when attached to a user.
+	err = s.adm.RemoveCannedPolicy(ctx, policy)
+	if err == nil {
+		c.Fatalf("policy could be unexpectedly deleted!")
+	}
+
+	// 6. Delete the user and then delete the policy.
+	err = s.adm.RemoveUser(ctx, accessKey)
+	if err != nil {
+		c.Fatalf("user could not be deleted: %v", err)
+	}
 	err = s.adm.RemoveCannedPolicy(ctx, policy)
 	if err != nil {
-		c.Fatalf("policy delete err: %v", err)
+		c.Fatalf("policy del err: %v", err)
 	}
 }
 
@@ -627,7 +637,8 @@ func (c *check) mustListObjects(ctx context.Context, client *minio.Client, bucke
 	res := client.ListObjects(ctx, bucket, minio.ListObjectsOptions{})
 	v, ok := <-res
 	if ok && v.Err != nil {
-		c.Fatalf("user was unable to list unexpectedly!")
+		msg := fmt.Sprintf("user was unable to list: %v", v.Err)
+		c.Fatalf(msg)
 	}
 }
 

cmd/iam-dummy-store.go

@@ -27,22 +27,37 @@ import (
 
 type iamDummyStore struct {
 	sync.RWMutex
+	*iamCache
+	usersSysType UsersSysType
 }
 
-func (ids *iamDummyStore) lock() {
+func newIAMDummyStore(usersSysType UsersSysType) *iamDummyStore {
+	return &iamDummyStore{
+		iamCache:     newIamCache(),
+		usersSysType: usersSysType,
+	}
+}
+
+func (ids *iamDummyStore) rlock() *iamCache {
+	ids.RLock()
+	return ids.iamCache
+}
+
+func (ids *iamDummyStore) runlock() {
+	ids.RUnlock()
+}
+
+func (ids *iamDummyStore) lock() *iamCache {
 	ids.Lock()
+	return ids.iamCache
 }
 
 func (ids *iamDummyStore) unlock() {
 	ids.Unlock()
 }
 
-func (ids *iamDummyStore) rlock() {
-	ids.RLock()
-}
-
-func (ids *iamDummyStore) runlock() {
-	ids.RUnlock()
+func (ids *iamDummyStore) getUsersSysType() UsersSysType {
+	return ids.usersSysType
 }
 
 func (ids *iamDummyStore) migrateBackendFormat(context.Context) error {

cmd/iam-etcd-store.go

@@ -62,27 +62,37 @@ func extractPathPrefixAndSuffix(s string, prefix string, suffix string) string {
 type IAMEtcdStore struct {
 	sync.RWMutex
 
+	*iamCache
+
+	usersSysType UsersSysType
+
 	client *etcd.Client
 }
 
-func newIAMEtcdStore(client *etcd.Client) *IAMEtcdStore {
-	return &IAMEtcdStore{client: client}
+func newIAMEtcdStore(client *etcd.Client, usersSysType UsersSysType) *IAMEtcdStore {
+	return &IAMEtcdStore{client: client, usersSysType: usersSysType}
 }
 
-func (ies *IAMEtcdStore) lock() {
+func (ies *IAMEtcdStore) rlock() *iamCache {
+	ies.RLock()
+	return ies.iamCache
+}
+
+func (ies *IAMEtcdStore) runlock() {
+	ies.RUnlock()
+}
+
+func (ies *IAMEtcdStore) lock() *iamCache {
 	ies.Lock()
+	return ies.iamCache
 }
 
 func (ies *IAMEtcdStore) unlock() {
 	ies.Unlock()
 }
 
-func (ies *IAMEtcdStore) rlock() {
-	ies.RLock()
-}
-
-func (ies *IAMEtcdStore) runlock() {
-	ies.RUnlock()
+func (ies *IAMEtcdStore) getUsersSysType() UsersSysType {
+	return ies.usersSysType
 }
 
 func (ies *IAMEtcdStore) saveIAMConfig(ctx context.Context, item interface{}, itemPath string, opts ...options) error {
@@ -244,6 +254,8 @@ func (ies *IAMEtcdStore) migrateToV1(ctx context.Context) error {
 
 // Should be called under config migration lock
 func (ies *IAMEtcdStore) migrateBackendFormat(ctx context.Context) error {
+	ies.Lock()
+	defer ies.Unlock()
 	return ies.migrateToV1(ctx)
 }
 
@@ -260,7 +272,7 @@ func (ies *IAMEtcdStore) loadPolicyDoc(ctx context.Context, policy string, m map
 	return nil
 }
 
-func (ies *IAMEtcdStore) getPolicyDoc(ctx context.Context, kvs *mvccpb.KeyValue, m map[string]iampolicy.Policy) error {
+func (ies *IAMEtcdStore) getPolicyDocKV(ctx context.Context, kvs *mvccpb.KeyValue, m map[string]iampolicy.Policy) error {
 	var p iampolicy.Policy
 	err := getIAMConfig(&p, kvs.Value, string(kvs.Key))
 	if err != nil {
@@ -286,14 +298,14 @@ func (ies *IAMEtcdStore) loadPolicyDocs(ctx context.Context, m map[string]iampol
 
 	// Parse all values to construct the policies data model.
 	for _, kvs := range r.Kvs {
-		if err = ies.getPolicyDoc(ctx, kvs, m); err != nil && err != errNoSuchPolicy {
+		if err = ies.getPolicyDocKV(ctx, kvs, m); err != nil && err != errNoSuchPolicy {
 			return err
 		}
 	}
 	return nil
 }
 
-func (ies *IAMEtcdStore) getUser(ctx context.Context, userkv *mvccpb.KeyValue, userType IAMUserType, m map[string]auth.Credentials, basePrefix string) error {
+func (ies *IAMEtcdStore) getUserKV(ctx context.Context, userkv *mvccpb.KeyValue, userType IAMUserType, m map[string]auth.Credentials, basePrefix string) error {
 	var u UserIdentity
 	err := getIAMConfig(&u, userkv.Value, string(userkv.Key))
 	if err != nil {
@@ -355,7 +367,7 @@ func (ies *IAMEtcdStore) loadUsers(ctx context.Context, userType IAMUserType, m
 
 	// Parse all users values to create the proper data model
 	for _, userKv := range r.Kvs {
-		if err = ies.getUser(ctx, userKv, userType, m, basePrefix); err != nil && err != errNoSuchUser {
+		if err = ies.getUserKV(ctx, userKv, userType, m, basePrefix); err != nil && err != errNoSuchUser {
 			return err
 		}
 	}

cmd/iam-object-store.go

@@ -34,30 +34,44 @@ import (
 
 // IAMObjectStore implements IAMStorageAPI
 type IAMObjectStore struct {
-	// Protect assignment to objAPI
+	// Protect access to storage within the current server.
 	sync.RWMutex
 
+	*iamCache
+
+	usersSysType UsersSysType
+
 	objAPI ObjectLayer
 }
 
-func newIAMObjectStore(objAPI ObjectLayer) *IAMObjectStore {
-	return &IAMObjectStore{objAPI: objAPI}
+func newIAMObjectStore(objAPI ObjectLayer, usersSysType UsersSysType) *IAMObjectStore {
+	return &IAMObjectStore{
+		iamCache:     newIamCache(),
+		objAPI:       objAPI,
+		usersSysType: usersSysType,
+	}
 }
 
-func (iamOS *IAMObjectStore) lock() {
+func (iamOS *IAMObjectStore) rlock() *iamCache {
+	iamOS.RLock()
+	return iamOS.iamCache
+}
+
+func (iamOS *IAMObjectStore) runlock() {
+	iamOS.RUnlock()
+}
+
+func (iamOS *IAMObjectStore) lock() *iamCache {
 	iamOS.Lock()
+	return iamOS.iamCache
 }
 
 func (iamOS *IAMObjectStore) unlock() {
 	iamOS.Unlock()
 }
 
-func (iamOS *IAMObjectStore) rlock() {
-	iamOS.RLock()
-}
-
-func (iamOS *IAMObjectStore) runlock() {
-	iamOS.RUnlock()
+func (iamOS *IAMObjectStore) getUsersSysType() UsersSysType {
+	return iamOS.usersSysType
 }
 
 // Migrate users directory in a single scan.
@@ -182,6 +196,8 @@ func (iamOS *IAMObjectStore) migrateToV1(ctx context.Context) error {
 
 // Should be called under config migration lock
 func (iamOS *IAMObjectStore) migrateBackendFormat(ctx context.Context) error {
+	iamOS.Lock()
+	defer iamOS.Unlock()
 	return iamOS.migrateToV1(ctx)
 }
 

cmd/site-replication.go

@@ -20,7 +20,6 @@ package cmd
 import (
 	"bytes"
 	"context"
-	"crypto/rand"
 	"crypto/tls"
 	"encoding/base64"
 	"encoding/json"
@@ -355,20 +354,12 @@ func (c *SiteReplicationSys) AddPeerClusters(ctx context.Context, sites []madmin
 
 	// Generate a secret key for the service account.
 	var secretKey string
-	{
-		secretKeyBuf := make([]byte, 40)
-		n, err := rand.Read(secretKeyBuf)
-		if err == nil && n != 40 {
-			err = fmt.Errorf("Unable to read 40 random bytes to generate secret key")
+	_, secretKey, err := auth.GenerateCredentials()
+	if err != nil {
+		return madmin.ReplicateAddStatus{}, SRError{
+			Cause: err,
+			Code:  ErrInternalError,
 		}
-		if err != nil {
-			return madmin.ReplicateAddStatus{}, SRError{
-				Cause: err,
-				Code:  ErrInternalError,
-			}
-		}
-		secretKey = strings.Replace(string([]byte(base64.StdEncoding.EncodeToString(secretKeyBuf))[:40]),
-			"/", "+", -1)
 	}
 
 	svcCred, err := globalIAMSys.NewServiceAccount(ctx, sites[selfIdx].AccessKey, nil, newServiceAccountOpts{
@@ -1270,9 +1261,7 @@ func (c *SiteReplicationSys) getAdminClient(ctx context.Context, deploymentID st
 }
 
 func (c *SiteReplicationSys) getPeerCreds() (*auth.Credentials, error) {
-	globalIAMSys.store.rlock()
-	defer globalIAMSys.store.runlock()
-	creds, ok := globalIAMSys.iamUsersMap[c.state.ServiceAccountAccessKey]
+	creds, ok := globalIAMSys.store.GetUser(c.state.ServiceAccountAccessKey)
 	if !ok {
 		return nil, errors.New("site replication service account not found!")
 	}

cmd/sts-handlers_test.go

@@ -95,6 +95,10 @@ func (s *TestSuiteIAM) TestSTS(c *check) {
 		c.Fatalf("Unable to set policy: %v", err)
 	}
 
+	// confirm that the user is able to access the bucket
+	uClient := s.getUserClient(c, accessKey, secretKey, "")
+	c.mustListObjects(ctx, uClient, bucket)
+
 	assumeRole := cr.STSAssumeRole{
 		Client:      s.TestSuiteCommon.client,
 		STSEndpoint: s.endPoint,

cmd/typed-errors.go

@@ -81,6 +81,9 @@ var errGroupNotEmpty = errors.New("Specified group is not empty - cannot remove
 // error returned in IAM subsystem when policy doesn't exist.
 var errNoSuchPolicy = errors.New("Specified canned policy does not exist")
 
+// error returned when policy to be deleted is in use.
+var errPolicyInUse = errors.New("Specified policy is in use and cannot be deleted.")
+
 // error returned in IAM subsystem when an external users systems is configured.
 var errIAMActionNotAllowed = errors.New("Specified IAM action is not allowed")
 

go.mod

@@ -50,7 +50,7 @@ require (
 	github.com/minio/madmin-go v1.1.11-0.20211102182201-e51fd3d6b104
 	github.com/minio/minio-go/v7 v7.0.15
 	github.com/minio/parquet-go v1.0.0
-	github.com/minio/pkg v1.1.5
+	github.com/minio/pkg v1.1.6-0.20211103212545-951bbd71498c
 	github.com/minio/selfupdate v0.3.1
 	github.com/minio/sha256-simd v1.0.0
 	github.com/minio/simdjson-go v0.2.1

go.sum

@@ -1077,6 +1077,10 @@ github.com/minio/pkg v1.0.11/go.mod h1:32x/3OmGB0EOi1N+3ggnp+B5VFkSBBB9svPMVfpnf
 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.6-0.20211102234044-cd6b7b169e31 h1:nZkTtdcp4JgClBFI+mZJNO1J+8bEpcrOumdsbgdtF0A=
+github.com/minio/pkg v1.1.6-0.20211102234044-cd6b7b169e31/go.mod h1:32x/3OmGB0EOi1N+3ggnp+B5VFkSBBB9svPMVfpnf14=
+github.com/minio/pkg v1.1.6-0.20211103212545-951bbd71498c h1:zP0nEhOBjJRu6fP8nrNMUoGVZGIHbFKY1Ln5V/6Djbg=
+github.com/minio/pkg v1.1.6-0.20211103212545-951bbd71498c/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=