From 3cac927348006edd205e1e2eed7676f62a1bf5f0 Mon Sep 17 00:00:00 2001
From: Aditya Manthramurthy <donatello@users.noreply.github.com>
Date: Tue, 19 Sep 2023 17:57:42 -0700
Subject: [PATCH] Load STS policy mappings periodically (#18061)

To ensure that policy mappings are current for service accounts
belonging to (non-derived) STS accounts (like an LDAP user's service
account) we periodically reload such mappings.

This is primarily to handle a case where a policy mapping update
notification is missed by a minio node. Such a node would continue to
have the stale mapping in memory because STS creds/mappings were never
periodically scanned from storage.
---
 cmd/iam-object-store.go | 28 +++++++++++++++++++++++++++-
 cmd/iam-store.go        | 21 +++++++++++++++++----
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/cmd/iam-object-store.go b/cmd/iam-object-store.go
index dcc252516..64bd3e01b 100644
--- a/cmd/iam-object-store.go
+++ b/cmd/iam-object-store.go
@@ -476,12 +476,38 @@ func (iamOS *IAMObjectStore) loadAllFromObjStore(ctx context.Context, cache *iam
 
 	bootstrapTraceMsg("loading service accounts")
 	svcAccList := listedConfigItems[svcAccListKey]
+	svcUsersMap := make(map[string]UserIdentity, len(svcAccList))
 	for _, item := range svcAccList {
 		userName := path.Dir(item)
-		if err := iamOS.loadUser(ctx, userName, svcUser, cache.iamUsersMap); err != nil && err != errNoSuchUser {
+		if err := iamOS.loadUser(ctx, userName, svcUser, svcUsersMap); err != nil && err != errNoSuchUser {
 			return fmt.Errorf("unable to load the service account `%s`: %w", userName, err)
 		}
 	}
+	for _, svcAcc := range svcUsersMap {
+		svcParent := svcAcc.Credentials.ParentUser
+		if _, ok := cache.iamUsersMap[svcParent]; !ok {
+			// If a service account's parent user is not in iamUsersMap, the
+			// parent is an STS account. Such accounts may have a policy mapped
+			// on the parent user, so we load them. This is not needed for the
+			// initial server startup, however, it is needed for the case where
+			// the STS account's policy mapping (for example in LDAP mode) may
+			// be changed and the user's policy mapping in memory is stale
+			// (because the policy change notification was missed by the current
+			// server).
+			//
+			// The "policy not found" error is ignored because the STS account may
+			// not have a policy mapped via its parent (for e.g. in
+			// OIDC/AssumeRoleWithCustomToken/AssumeRoleWithCertificate).
+			err := iamOS.loadMappedPolicy(ctx, svcParent, stsUser, false, cache.iamSTSPolicyMap)
+			if err != nil && !errors.Is(err, errNoSuchPolicy) {
+				return fmt.Errorf("unable to load the policy mapping for the STS user `%s`: %w", svcParent, err)
+			}
+		}
+	}
+	// Copy svcUsersMap to cache.iamUsersMap
+	for k, v := range svcUsersMap {
+		cache.iamUsersMap[k] = v
+	}
 
 	cache.buildUserGroupMemberships()
 	return nil
diff --git a/cmd/iam-store.go b/cmd/iam-store.go
index aa92279da..3f8510d95 100644
--- a/cmd/iam-store.go
+++ b/cmd/iam-store.go
@@ -560,9 +560,9 @@ func (store *IAMStoreSys) LoadIAMCache(ctx context.Context) error {
 	// were changes to the in-memory cache we should wait for the next
 	// cycle until we can safely update the in-memory cache.
 	//
-	// An in-memory cache must be replaced only if we know for sure that
-	// the values loaded from disk are not stale. They might be stale
-	// if the cached.updatedAt is recent than the refresh cycle began.
+	// An in-memory cache must be replaced only if we know for sure that the
+	// values loaded from disk are not stale. They might be stale if the
+	// cached.updatedAt is more recent than the refresh cycle began.
 	if cache.updatedAt.Before(loadedAt) {
 		// No one has updated anything since the config was loaded,
 		// so we just replace whatever is on the disk into memory.
@@ -572,6 +572,15 @@ func (store *IAMStoreSys) LoadIAMCache(ctx context.Context) error {
 		cache.iamUserGroupMemberships = newCache.iamUserGroupMemberships
 		cache.iamUserPolicyMap = newCache.iamUserPolicyMap
 		cache.iamUsersMap = newCache.iamUsersMap
+		// For STS policy map, we need to merge the new cache with the existing
+		// cache because the periodic IAM reload is partial. The periodic load
+		// here is to account for STS policy mapping changes that should apply
+		// for service accounts derived from such STS accounts (i.e. LDAP STS
+		// accounts).
+		for k, v := range newCache.iamSTSPolicyMap {
+			cache.iamSTSPolicyMap[k] = v
+		}
+
 		cache.updatedAt = time.Now()
 	}
 
@@ -2433,8 +2442,12 @@ func (store *IAMStoreSys) LoadUser(ctx context.Context, accessKey string) {
 			// Load parent user and mapped policies.
 			if store.getUsersSysType() == MinIOUsersSysType {
 				store.loadUser(ctx, svc.Credentials.ParentUser, regUser, cache.iamUsersMap)
+				store.loadMappedPolicy(ctx, svc.Credentials.ParentUser, regUser, false, cache.iamUserPolicyMap)
+			} else {
+				// In case of LDAP the parent user's policy mapping needs to be
+				// loaded into sts map
+				store.loadMappedPolicy(ctx, svc.Credentials.ParentUser, stsUser, false, cache.iamSTSPolicyMap)
 			}
-			store.loadMappedPolicy(ctx, svc.Credentials.ParentUser, regUser, false, cache.iamUserPolicyMap)
 		}
 	}