Merge service-account replication fix into main

Include the remaining committed IAM snapshot fix from claude/lucid-hopper-811f3d. Preserve the current contributor inventory and previously integrated fixes while adding the service-account replication release note.

Signed-off-by: Feng Ruohang <rh@vonng.com>

cmd/admin-handlers-users.go

@@ -914,21 +914,13 @@ func (a adminAPIHandlers) UpdateServiceAccount(w http.ResponseWriter, r *http.Re
 	}
 
 	// Call site replication hook - non-root user accounts are replicated.
+	// The hook sends the account as it is stored after this edit, not the
+	// fields the request changed (see replicationItem).
 	if svcAccount.ParentUser != globalActiveCred.AccessKey {
 		replLogIf(ctx, globalSiteReplicationSys.IAMChangeHook(ctx, madmin.SRIAMItem{
-			Type: madmin.SRIAMItemSvcAcc,
-			SvcAccChange: &madmin.SRSvcAccChange{
-				Update: &madmin.SRSvcAccUpdate{
-					AccessKey:     accessKey,
-					SecretKey:     opts.secretKey,
-					Status:        opts.status,
-					Name:          opts.name,
-					Description:   opts.description,
-					SessionPolicy: madmin.SRSessionPolicy(updateReq.NewPolicy),
-					Expiration:    updateReq.NewExpiration,
-				},
-			},
-			UpdatedAt: updatedAt,
+			Type:         madmin.SRIAMItemSvcAcc,
+			SvcAccChange: &madmin.SRSvcAccChange{Update: &madmin.SRSvcAccUpdate{AccessKey: accessKey}},
+			UpdatedAt:    updatedAt,
 		}))
 	}
 
@@ -2399,37 +2391,13 @@ func (a adminAPIHandlers) ExportIAM(w http.ResponseWriter, r *http.Request) {
 					// skip site-replication service account.
 					continue
 				}
-				claims, err := globalIAMSys.GetClaimsForSvcAcc(ctx, acc.Credentials.AccessKey)
+				// The stored record only: this server's cache may not have
+				// heard of an edit made through another server.
+				svcAccts[user], err = serviceAccountSnapshot(acc)
 				if err != nil {
 					writeErrorResponse(ctx, w, exportError(ctx, err, iamFile, ""), r.URL)
 					return
 				}
-				sa, policy, err := globalIAMSys.GetServiceAccount(ctx, acc.Credentials.AccessKey)
-				if err != nil {
-					writeErrorResponse(ctx, w, exportError(ctx, err, iamFile, ""), r.URL)
-					return
-				}
-
-				var policyJSON []byte
-				if policy != nil {
-					policyJSON, err = json.Marshal(policy)
-					if err != nil {
-						writeErrorResponse(ctx, w, exportError(ctx, err, iamFile, ""), r.URL)
-						return
-					}
-				}
-				svcAccts[user] = madmin.SRSvcAccCreate{
-					Parent:        acc.Credentials.ParentUser,
-					AccessKey:     user,
-					SecretKey:     acc.Credentials.SecretKey,
-					Groups:        acc.Credentials.Groups,
-					Claims:        claims,
-					SessionPolicy: policyJSON,
-					Status:        acc.Credentials.Status,
-					Name:          sa.Name,
-					Description:   sa.Description,
-					Expiration:    &sa.Expiration,
-				}
 			}
 
 			svcAccData, err := json.Marshal(svcAccts)

cmd/iam-replication-protocol.go

@@ -172,14 +172,41 @@ func (c *SiteReplicationSys) replicationItem(ctx context.Context, item madmin.SR
 			key = item.SvcAccChange.Update.AccessKey
 		}
 		if key != "" {
-			r, err := loadIAMRevision(ctx, globalIAMSys.store, getUserIdentityPath(key, svcUser))
+			// A create or an edit goes out as the account stored now: the
+			// whole record, under its revision, whatever the caller's item
+			// says. A peer orders items by revision and drops an older one,
+			// so an edit sent as the fields it changed would lose every
+			// earlier edit that did not reach the peer, or reaches it late.
+			// A later write that won before sending goes out in place of
+			// this one.
+			stored := make(map[string]UserIdentity, 1)
+			err := globalIAMSys.store.loadUser(ctx, key, svcUser, stored)
+			if err != nil && !errors.Is(err, errNoSuchUser) {
+				return out, err
+			}
+			u, ok := stored[key]
+			if !ok {
+				// Nothing is sent for an account that is deleted, expired or
+				// of a revoked parent meanwhile; a deletion replicates itself.
+				// A stored token that cannot be read is an error, and the
+				// account is not taken for sent.
+				var raw UserIdentity
+				err := globalIAMSys.store.loadIAMConfig(ctx, &raw, getUserIdentityPath(key, svcUser))
+				if err == nil && !raw.Deleted && !raw.Credentials.IsExpired() {
+					_, err = extractJWTClaims(raw)
+				}
+				if err != nil && !errors.Is(err, errConfigNotFound) {
+					return out, err
+				}
+				return out, errIAMStaleUpdate
+			}
+			snapshot, err := serviceAccountSnapshot(u)
 			if err != nil {
 				return out, err
 			}
-			if r.Deleted || r.timestamp().After(item.UpdatedAt) {
-				return out, errIAMStaleUpdate
-			}
-			out.RevokedBefore = r.RevokedBefore
+			out.SvcAccChange = &madmin.SRSvcAccChange{Create: &snapshot}
+			out.UpdatedAt = u.UpdatedAt
+			out.RevokedBefore = u.RevokedBefore
 		}
 	}
 	if item.Type == madmin.SRIAMItemGroupInfo && item.GroupInfo != nil && !item.GroupInfo.UpdateReq.IsRemove {

cmd/iam-svcacct-replication-snapshot_test.go

@@ -0,0 +1,482 @@
+// Copyright (c) 2026 PGSTY
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+package cmd
+
+import (
+	"archive/zip"
+	"bytes"
+	"context"
+	"encoding/base64"
+	"encoding/json"
+	"errors"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"path"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/minio/madmin-go/v3"
+	"github.com/minio/minio/internal/auth"
+	"github.com/pgsty/silo-pkg/v3/policy"
+)
+
+// Site replication sends a service account as the record its site has
+// stored, whole. A peer that applies the last item it was sent then has
+// the account of the sending site, whatever it received or lost before,
+// and whatever the sending server holds in its cache.
+
+const svcAccSnapshotPolicy = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["s3:GetObject"],"Resource":["arn:aws:s3:::snapshot/*"]}]}`
+
+// svcAccRecord is what a site has of a service account.
+type svcAccRecord struct {
+	Parent, Secret, Status, Name, Description, Policy string
+	Expiration, UpdatedAt                             time.Time
+}
+
+// svcAccPolicyText is a policy document in the one form it is compared in;
+// an account without an embedded policy has "".
+func svcAccPolicyText(t *testing.T, document []byte) string {
+	t.Helper()
+	if len(document) == 0 {
+		return ""
+	}
+	p, err := policy.ParseConfig(bytes.NewReader(document))
+	mustIAM(t, err)
+	text, err := json.Marshal(p)
+	mustIAM(t, err)
+	return string(text)
+}
+
+// svcAccStored is the account as a server that starts now loads it.
+func svcAccStored(t *testing.T, f *groupRefreshFixture, key string) svcAccRecord {
+	t.Helper()
+	u, ok := f.stored(t, key)
+	if !ok {
+		t.Fatalf("service account %s is not stored", key)
+	}
+	cr := u.Credentials
+	claims, err := getClaimsFromTokenWithSecret(cr.SessionToken, cr.SecretKey)
+	mustIAM(t, err)
+	rec := svcAccRecord{Parent: cr.ParentUser, Secret: cr.SecretKey, Status: cr.Status, Name: cr.Name, Description: cr.Description, Expiration: cr.Expiration, UpdatedAt: u.UpdatedAt}
+	if sp, ok := claims.Lookup(policy.SessionPolicyName); ok && claims.MapClaims[iamPolicyClaimNameSA()] == embeddedPolicyType {
+		document, err := base64.StdEncoding.DecodeString(sp)
+		mustIAM(t, err)
+		rec.Policy = svcAccPolicyText(t, document)
+	}
+	return rec
+}
+
+// svcAccSite is a site that replicates to one peer, which records the IAM
+// items it is sent.
+type svcAccSite struct {
+	*groupRefreshFixture
+	mu   sync.Mutex
+	sent []iamReplicationItem
+}
+
+func newSvcAccSite(t *testing.T) *svcAccSite {
+	t.Helper()
+	s := &svcAccSite{groupRefreshFixture: newGroupRefreshFixture(t)}
+	globalObjLayerMutex.Lock()
+	globalObjectAPI = s.obj
+	globalObjLayerMutex.Unlock()
+
+	peer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == http.MethodPut && strings.HasSuffix(r.URL.Path, iamRevisionPeerPath) {
+			var batch iamRevisionBatch
+			if err := json.NewDecoder(r.Body).Decode(&batch); err != nil {
+				t.Error(err)
+			}
+			s.mu.Lock()
+			s.sent = append(s.sent, batch.Items...)
+			s.mu.Unlock()
+		}
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(iamRevisionResponse{iamRevisionStatus: iamRevisionStatus{Version: iamRevisionProtocol, Node: "node", Instance: "boot", Digest: "fixture"}})
+	}))
+	globalSiteReplicatorCred.Set("snapshot-site-signing-key")
+	globalSiteReplicationSys.Lock()
+	globalSiteReplicationSys.enabled = true
+	globalSiteReplicationSys.state = srState{ServiceAccountAccessKey: s.parent.AccessKey, Peers: s.peers(peer.URL)}
+	globalSiteReplicationSys.Unlock()
+	t.Cleanup(func() {
+		s.leave()
+		peer.Close()
+	})
+	return s
+}
+
+func (s *svcAccSite) peers(endpoint string) map[string]madmin.PeerInfo {
+	return map[string]madmin.PeerInfo{
+		globalDeploymentID(): {Name: "local", DeploymentID: globalDeploymentID()},
+		"remote":             {Name: "remote", DeploymentID: "remote", Endpoint: endpoint},
+	}
+}
+
+// leave ends the replication of this process, for the site that comes next.
+func (s *svcAccSite) leave() {
+	globalSiteReplicationSys.Lock()
+	globalSiteReplicationSys.enabled = false
+	globalSiteReplicationSys.state = srState{}
+	globalSiteReplicationSys.Unlock()
+	globalSiteReplicatorCred.Set("")
+}
+
+// items are the creates and edits of account key the peer was sent, in the
+// order of sending.
+func (s *svcAccSite) items(key string) []iamReplicationItem {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	var items []iamReplicationItem
+	for _, item := range s.sent {
+		c := item.SvcAccChange
+		if c != nil && ((c.Create != nil && c.Create.AccessKey == key) || (c.Update != nil && c.Update.AccessKey == key)) {
+			items = append(items, item)
+		}
+	}
+	return items
+}
+
+// create and edit are an administrator's requests to this site.
+func (s *svcAccSite) create(t *testing.T, key string) {
+	t.Helper()
+	rec := iamAdminCall(t, globalActiveCred, http.MethodPut, "/minio/admin/v3/add-service-account",
+		madmin.AddServiceAccountReq{TargetUser: s.parent.AccessKey, AccessKey: key, SecretKey: "snapshot-original-secret"}, nil, adminAPIHandlers{}.AddServiceAccount)
+	if rec.Code != http.StatusOK {
+		t.Fatalf("add-service-account: %d %s", rec.Code, rec.Body.String())
+	}
+}
+
+func (s *svcAccSite) edit(t *testing.T, key string, req madmin.UpdateServiceAccountReq) {
+	t.Helper()
+	rec := iamAdminCall(t, globalActiveCred, http.MethodPost, "/minio/admin/v3/update-service-account?accessKey="+key,
+		req, map[string]string{"accessKey": key}, adminAPIHandlers{}.UpdateServiceAccount)
+	if rec.Code != http.StatusNoContent {
+		t.Fatalf("update-service-account: %d %s", rec.Code, rec.Body.String())
+	}
+}
+
+// svcAccPeer is the site that applies items, in the order given, and what
+// it then has of account key. It has the parent user, as a peer does.
+func svcAccPeer(t *testing.T, from *svcAccSite, key string, items ...iamReplicationItem) svcAccRecord {
+	t.Helper()
+	from.leave()
+	peer := newGroupRefreshFixture(t)
+	for i, item := range items {
+		// As the peer reads it off the wire.
+		raw, err := json.Marshal(item)
+		mustIAM(t, err)
+		var received iamReplicationItem
+		mustIAM(t, json.Unmarshal(raw, &received))
+		if err := applyIAMReplicationItem(peer.ctx, received); err != nil {
+			t.Fatalf("the peer refused item %d: %v", i, err)
+		}
+	}
+	return svcAccStored(t, peer, key)
+}
+
+func wantSvcAccRecord(t *testing.T, where string, got, want svcAccRecord) {
+	t.Helper()
+	if got != want {
+		t.Errorf("%s:\n got %+v\nwant %+v", where, got, want)
+	}
+}
+
+// An edit of a service account reaches a peer whatever became of the edits
+// before it: one that arrives late, or never, is not lost.
+func TestSiteReplicationServiceAccountEdits(t *testing.T) {
+	const key = "snapshot-edits"
+	for _, tc := range []struct {
+		name  string
+		edits []madmin.UpdateServiceAccountReq
+		took  func(svcAccRecord) bool // control: what the site has after them
+	}{
+		{"policy-secret-description", []madmin.UpdateServiceAccountReq{
+			{NewPolicy: json.RawMessage(svcAccSnapshotPolicy)},
+			{NewSecretKey: "snapshot-rotated-secret"},
+			{NewDescription: "described"},
+		}, func(r svcAccRecord) bool {
+			return r.Policy != "" && r.Secret == "snapshot-rotated-secret" && r.Description == "described"
+		}},
+		{"secret-disable", []madmin.UpdateServiceAccountReq{
+			{NewPolicy: json.RawMessage(svcAccSnapshotPolicy), NewSecretKey: "snapshot-rotated-secret"},
+			{NewStatus: string(madmin.AccountDisabled)},
+		}, func(r svcAccRecord) bool {
+			return r.Policy != "" && r.Secret == "snapshot-rotated-secret" && r.Status == auth.AccountOff
+		}},
+		// The last edit removes the embedded policy: the peer gets an
+		// account without one, and the description set before.
+		{"policy-removed", []madmin.UpdateServiceAccountReq{
+			{NewPolicy: json.RawMessage(svcAccSnapshotPolicy)},
+			{NewDescription: "described"},
+			{NewPolicy: json.RawMessage(`{"Statement":[]}`)},
+		}, func(r svcAccRecord) bool { return r.Policy == "" && r.Description == "described" }},
+		// Policy documents an edit ignores: one with a version and no
+		// statement, one with statements and no version. The site keeps the
+		// policy it has, and so does the peer.
+		{"policy-ignored", []madmin.UpdateServiceAccountReq{
+			{NewPolicy: json.RawMessage(svcAccSnapshotPolicy)},
+			{NewPolicy: json.RawMessage(`{"Version":"2012-10-17","Statement":[]}`), NewName: "named"},
+			{NewPolicy: json.RawMessage(`{"Statement":[{"Effect":"Allow","Action":["s3:PutObject"],"Resource":["arn:aws:s3:::ignored/*"]}]}`), NewDescription: "described"},
+		}, func(r svcAccRecord) bool {
+			return r.Policy == svcAccPolicyText(t, []byte(svcAccSnapshotPolicy)) && r.Name == "named" && r.Description == "described"
+		}},
+	} {
+		for _, delivery := range []string{"late", "lost"} {
+			t.Run(tc.name+"/"+delivery, func(t *testing.T) {
+				site := newSvcAccSite(t)
+				site.create(t, key)
+				for _, edit := range tc.edits {
+					site.edit(t, key, edit)
+				}
+				sent := site.items(key)
+				if len(sent) != 1+len(tc.edits) {
+					t.Fatalf("the site sent %d items for the account, want one for the create and each edit", len(sent))
+				}
+				want := svcAccStored(t, site.groupRefreshFixture, key)
+				if !tc.took(want) {
+					t.Fatalf("control: the site did not apply the edits: %+v", want)
+				}
+				// The peer has the account and gets the last edit. The edits
+				// before it were lost, or reach it after the last.
+				last := len(sent) - 1
+				deliver := []iamReplicationItem{sent[0], sent[last]}
+				if delivery == "late" {
+					deliver = append(deliver, sent[1:last]...)
+				}
+				wantSvcAccRecord(t, "peer", svcAccPeer(t, site, key, deliver...), want)
+			})
+		}
+	}
+
+	// Two requests edit the account through this site at once, and the
+	// first comes to replicate when the second is stored.
+	t.Run("concurrent", func(t *testing.T) {
+		site := newSvcAccSite(t)
+		site.create(t, key)
+		sp, err := policy.ParseConfig(strings.NewReader(svcAccSnapshotPolicy))
+		mustIAM(t, err)
+		first, err := site.sys.UpdateServiceAccount(site.ctx, key, updateServiceAccountOpts{sessionPolicy: sp})
+		mustIAM(t, err)
+		second, err := site.sys.UpdateServiceAccount(site.ctx, key, updateServiceAccountOpts{description: "described"})
+		mustIAM(t, err)
+		for _, at := range []time.Time{first, second} {
+			mustIAM(t, globalSiteReplicationSys.IAMChangeHook(site.ctx, madmin.SRIAMItem{
+				Type:         madmin.SRIAMItemSvcAcc,
+				SvcAccChange: &madmin.SRSvcAccChange{Update: &madmin.SRSvcAccUpdate{AccessKey: key}},
+				UpdatedAt:    at,
+			}))
+		}
+		want := svcAccStored(t, site.groupRefreshFixture, key)
+		if want.Policy == "" || want.Description != "described" {
+			t.Fatalf("control: the site did not apply the edits: %+v", want)
+		}
+		wantSvcAccRecord(t, "peer", svcAccPeer(t, site, key, site.items(key)...), want)
+	})
+}
+
+// Another server of the site restricted, rotated and disabled the account,
+// and this server's cache has not heard of it. What this server sends, and
+// exports, is the account that is stored.
+func TestSiteReplicationServiceAccountStaleCache(t *testing.T) {
+	const key = "snapshot-stale"
+	// prepare returns the site, the stored account and what it sent for the
+	// create.
+	prepare := func(t *testing.T) (*svcAccSite, svcAccRecord, iamReplicationItem) {
+		t.Helper()
+		site := newSvcAccSite(t)
+		site.create(t, key)
+		created := site.items(key)
+		if len(created) != 1 {
+			t.Fatalf("the site sent %d items for the create", len(created))
+		}
+		sp, err := policy.ParseConfig(strings.NewReader(svcAccSnapshotPolicy))
+		mustIAM(t, err)
+		_, err = site.sibling(t).UpdateServiceAccount(site.ctx, key, updateServiceAccountOpts{
+			sessionPolicy: sp, secretKey: "snapshot-rotated-secret", status: auth.AccountOff, description: "restricted",
+		})
+		mustIAM(t, err)
+		if cached, _ := site.sys.store.GetUser(key); cached.Credentials.SecretKey != "snapshot-original-secret" || cached.Credentials.Status != auth.AccountOn {
+			t.Fatal("control: this server's cache must not have heard of the edit")
+		}
+		want := svcAccStored(t, site.groupRefreshFixture, key)
+		if want.Policy == "" || want.Secret != "snapshot-rotated-secret" || want.Status != auth.AccountOff {
+			t.Fatalf("control: the edit is not stored: %+v", want)
+		}
+		return site, want, created[0]
+	}
+
+	t.Run("heal", func(t *testing.T) {
+		site, want, created := prepare(t)
+		local := madmin.UserInfo{Status: madmin.AccountStatus(want.Status), UpdatedAt: want.UpdatedAt}
+		remote := madmin.UserInfo{Status: madmin.AccountStatus(auth.AccountOn), UpdatedAt: created.UpdatedAt}
+		info := srStatusInfo{Sites: globalSiteReplicationSys.state.Peers, UserStats: map[string]map[string]srUserStatsSummary{key: {
+			globalDeploymentID(): {userInfo: srUserInfo{UserInfo: local}},
+			"remote":             {SRUserStatsSummary: madmin.SRUserStatsSummary{UserInfoMismatch: true}, userInfo: srUserInfo{UserInfo: remote}},
+		}}}
+		mustIAM(t, globalSiteReplicationSys.healUsers(site.ctx, site.obj, key, info))
+		sent := site.items(key)
+		if len(sent) != 2 {
+			t.Fatalf("heal sent %d items for the account, want 1", len(sent)-1)
+		}
+		wantSvcAccRecord(t, "healed peer", svcAccPeer(t, site, key, sent...), want)
+	})
+
+	t.Run("sync", func(t *testing.T) {
+		site, want, _ := prepare(t)
+		mustIAM(t, globalSiteReplicationSys.syncToAllPeers(site.ctx, madmin.SRAddOptions{}))
+		sent := site.items(key)
+		if len(sent) != 2 {
+			t.Fatalf("sync sent %d items for the account, want 1", len(sent)-1)
+		}
+		wantSvcAccRecord(t, "new peer", svcAccPeer(t, site, key, sent[1]), want)
+	})
+
+	t.Run("export", func(t *testing.T) {
+		_, want, _ := prepare(t)
+		rec := iamAdminCall(t, globalActiveCred, http.MethodGet, "/minio/admin/v3/export-iam", nil, nil, adminAPIHandlers{}.ExportIAM)
+		if rec.Code != http.StatusOK {
+			t.Fatalf("export-iam: %d %s", rec.Code, rec.Body.String())
+		}
+		archive, err := zip.NewReader(bytes.NewReader(rec.Body.Bytes()), int64(rec.Body.Len()))
+		mustIAM(t, err)
+		file, err := archive.Open(path.Join(iamAssetsDir, allSvcAcctsFile))
+		mustIAM(t, err)
+		raw, err := io.ReadAll(file)
+		mustIAM(t, err)
+		var accounts map[string]madmin.SRSvcAccCreate
+		mustIAM(t, json.Unmarshal(raw, &accounts))
+		a, ok := accounts[key]
+		if !ok || a.Expiration == nil {
+			t.Fatalf("the export has no account %s with an expiration (present=%v)", key, ok)
+		}
+		got := svcAccRecord{Parent: a.Parent, Secret: a.SecretKey, Status: a.Status, Name: a.Name, Description: a.Description, Policy: svcAccPolicyText(t, a.SessionPolicy), Expiration: *a.Expiration}
+		want.UpdatedAt = time.Time{} // an export has no revisions
+		wantSvcAccRecord(t, "export", got, want)
+		if a.Claims[iamPolicyClaimNameSA()] != embeddedPolicyType {
+			t.Errorf("exported claims say %v, want %s", a.Claims[iamPolicyClaimNameSA()], embeddedPolicyType)
+		}
+	})
+}
+
+// The parent was deleted and created again on both sites. The token of a
+// service account created afterwards names that revocation, and a peer
+// refuses a credential of the parent that does not: the create must carry
+// the claims of the token.
+func TestSiteReplicationServiceAccountOfRecreatedParent(t *testing.T) {
+	const key = "snapshot-reparent"
+	site := newSvcAccSite(t)
+	mustIAM(t, site.sys.DeleteUser(site.ctx, site.parent.AccessKey, false))
+	iamAdminUser(site.ctx, t, site.sys, site.parent.AccessKey, "refresh-parent")
+	revoked, err := loadIAMRevision(site.ctx, site.sys.store, getUserIdentityPath(site.parent.AccessKey, regUser))
+	mustIAM(t, err)
+	if revoked.RevokedBefore.IsZero() {
+		t.Fatal("control: the parent must have a revocation boundary")
+	}
+	site.create(t, key)
+	sent := site.items(key)
+	if len(sent) != 1 {
+		t.Fatalf("the site sent %d items for the create", len(sent))
+	}
+	want := svcAccStored(t, site.groupRefreshFixture, key)
+
+	// The peer's parent is newer than the deletion, which leaves it live
+	// with the boundary: a parent that was deleted and created again.
+	site.leave()
+	peer := newGroupRefreshFixture(t)
+	mustIAM(t, peer.sys.DeleteUser(withIAMReplicationTime(peer.ctx, revoked.RevokedBefore), peer.parent.AccessKey, false))
+	if r, err := loadIAMRevision(peer.ctx, peer.sys.store, getUserIdentityPath(peer.parent.AccessKey, regUser)); err != nil || r.Deleted || !r.RevokedBefore.Equal(revoked.RevokedBefore) {
+		t.Fatalf("control: the peer's parent must be live with the boundary (err=%v deleted=%v)", err, r.Deleted)
+	}
+	mustIAM(t, applyIAMReplicationItem(peer.ctx, sent[0]))
+	if _, ok := peer.stored(t, key); !ok {
+		t.Fatal("the peer dropped the new service account")
+	}
+	wantSvcAccRecord(t, "peer", svcAccStored(t, peer, key), want)
+}
+
+// An account that is deleted is not sent, and that is not an error. An
+// account whose stored token cannot be read is not taken for deleted: the
+// site signing key is not available here, and no token is read without it.
+func TestSiteReplicationServiceAccountUnreadable(t *testing.T) {
+	const key = "snapshot-unreadable"
+	site := newSvcAccSite(t)
+	site.create(t, key)
+	item := madmin.SRIAMItem{Type: madmin.SRIAMItemSvcAcc, SvcAccChange: &madmin.SRSvcAccChange{Update: &madmin.SRSvcAccUpdate{AccessKey: key}}}
+
+	globalSiteReplicatorCred.Set("")
+	if _, err := globalSiteReplicationSys.replicationItem(site.ctx, item); err == nil || errors.Is(err, errIAMStaleUpdate) {
+		t.Errorf("an account whose token cannot be read must be an error, got %v", err)
+	}
+	if err := globalSiteReplicationSys.IAMChangeHook(site.ctx, item); err == nil {
+		t.Error("the hook reported an account as sent that it could not read")
+	}
+	globalSiteReplicatorCred.Set("snapshot-site-signing-key")
+
+	mustIAM(t, site.sys.DeleteServiceAccount(site.ctx, key, false))
+	if _, err := globalSiteReplicationSys.replicationItem(site.ctx, item); !errors.Is(err, errIAMStaleUpdate) {
+		t.Errorf("a deleted account must not be sent, got %v", err)
+	}
+	if sent := site.items(key); len(sent) != 1 {
+		t.Errorf("the site sent %d items for the account, want only the create", len(sent))
+	}
+}
+
+// svcAccPausedStore holds every load of a user until proceed is closed.
+type svcAccPausedStore struct {
+	IAMStorageAPI
+	once             sync.Once
+	loading, proceed chan struct{}
+}
+
+func (s *svcAccPausedStore) loadUser(ctx context.Context, user string, userType IAMUserType, m map[string]UserIdentity) error {
+	s.once.Do(func() { close(s.loading) })
+	<-s.proceed
+	return s.IAMStorageAPI.loadUser(ctx, user, userType, m)
+}
+
+// The hook reads the account from storage without the lock of the
+// replication configuration. Loading an identity takes that lock for
+// reading; if the hook held it too, a change of the configuration that
+// arrives in between would wait for the hook, and the hook for it.
+func TestSiteReplicationHookReadsStorageUnlocked(t *testing.T) {
+	const key = "snapshot-unlocked"
+	site := newSvcAccSite(t)
+	site.create(t, key)
+	paused := &svcAccPausedStore{IAMStorageAPI: site.sys.store.IAMStorageAPI, loading: make(chan struct{}), proceed: make(chan struct{})}
+	site.sys.store.IAMStorageAPI = paused
+
+	sent := make(chan error, 1)
+	go func() {
+		sent <- globalSiteReplicationSys.IAMChangeHook(site.ctx, madmin.SRIAMItem{
+			Type:         madmin.SRIAMItemSvcAcc,
+			SvcAccChange: &madmin.SRSvcAccChange{Update: &madmin.SRSvcAccUpdate{AccessKey: key}},
+		})
+	}()
+	select {
+	case <-paused.loading:
+	case err := <-sent:
+		t.Fatalf("the hook did not read the account from storage (err=%v)", err)
+	}
+	// A writer that had to wait here would never get the lock, nor the
+	// hook its next read lock behind that writer: try, do not wait.
+	if globalSiteReplicationSys.TryLock() {
+		globalSiteReplicationSys.Unlock() //nolint:staticcheck // only the lock is of interest
+	} else {
+		t.Error("the hook holds the lock of the replication configuration while it reads storage")
+	}
+	close(paused.proceed)
+	select {
+	case err := <-sent:
+		mustIAM(t, err)
+	case <-time.After(10 * time.Second):
+		t.Fatal("the hook did not return")
+	}
+	if got := site.items(key); len(got) != 2 {
+		t.Errorf("the site sent %d items for the account, want the create and the edit", len(got))
+	}
+}

cmd/iam.go

@@ -1311,22 +1311,62 @@ func (sys *IAMSys) getServiceAccount(ctx context.Context, accessKey string) (Use
 		return UserIdentity{}, nil, errNoSuchServiceAccount
 	}
 
-	var embeddedPolicy *policy.Policy
+	embeddedPolicy, err := embeddedServiceAccountPolicy(jwtClaims)
+	if err != nil {
+		return UserIdentity{}, nil, err
+	}
+	return sa, embeddedPolicy, nil
+}
 
+// embeddedServiceAccountPolicy returns the policy embedded in the token of a
+// service account, or nil when the account has the policies of its parent.
+func embeddedServiceAccountPolicy(jwtClaims *jwt.MapClaims) (*policy.Policy, error) {
 	pt, ptok := jwtClaims.Lookup(iamPolicyClaimNameSA())
 	sp, spok := jwtClaims.Lookup(policy.SessionPolicyName)
-	if ptok && spok && pt == embeddedPolicyType {
-		policyBytes, err := base64.StdEncoding.DecodeString(sp)
-		if err != nil {
-			return UserIdentity{}, nil, err
-		}
-		embeddedPolicy, err = policy.ParseConfig(bytes.NewReader(policyBytes))
-		if err != nil {
-			return UserIdentity{}, nil, err
+	if !ptok || !spok || pt != embeddedPolicyType {
+		return nil, nil
+	}
+	policyBytes, err := base64.StdEncoding.DecodeString(sp)
+	if err != nil {
+		return nil, err
+	}
+	return policy.ParseConfig(bytes.NewReader(policyBytes))
+}
+
+// serviceAccountSnapshot returns service account u, a record loaded from
+// storage, as the create that replication, the first sync, heal and export
+// carry. Its claims and its embedded policy are read from the token of that
+// record, so that every part of the snapshot is of one revision: a server's
+// cache may hold the account as it was before an edit made through another
+// server.
+func serviceAccountSnapshot(u UserIdentity) (madmin.SRSvcAccCreate, error) {
+	jwtClaims, err := extractJWTClaims(u)
+	if err != nil {
+		return madmin.SRSvcAccCreate{}, err
+	}
+	embeddedPolicy, err := embeddedServiceAccountPolicy(jwtClaims)
+	if err != nil {
+		return madmin.SRSvcAccCreate{}, err
+	}
+	var policyJSON []byte
+	if embeddedPolicy != nil {
+		if policyJSON, err = json.Marshal(embeddedPolicy); err != nil {
+			return madmin.SRSvcAccCreate{}, err
 		}
 	}
-
-	return sa, embeddedPolicy, nil
+	cr := u.Credentials
+	return madmin.SRSvcAccCreate{
+		Parent:        cr.ParentUser,
+		AccessKey:     cr.AccessKey,
+		SecretKey:     cr.SecretKey,
+		Groups:        cr.Groups,
+		Claims:        jwtClaims.Map(),
+		SessionPolicy: policyJSON,
+		Status:        cr.Status,
+		Name:          cr.Name,
+		Description:   cr.Description,
+		Expiration:    &cr.Expiration,
+	}, nil
 }
 
 // GetTemporaryAccount - wrapper method to get information about a temporary account
@@ -1398,25 +1438,6 @@ func (sys *IAMSys) getAccountWithClaims(ctx context.Context, accessKey string) (
 	return acc, jwtClaims, nil
 }
 
-// GetClaimsForSvcAcc - gets the claims associated with the service account.
-func (sys *IAMSys) GetClaimsForSvcAcc(ctx context.Context, accessKey string) (map[string]any, error) {
-	if !sys.Initialized() {
-		return nil, errServerNotInitialized
-	}
-
-	sa, ok := sys.store.GetUser(accessKey)
-	if !ok || !sa.Credentials.IsServiceAccount() {
-		return nil, errNoSuchServiceAccount
-	}
-
-	jwtClaims, err := extractJWTClaims(sa)
-	if err != nil {
-		return nil, err
-	}
-
-	return jwtClaims.Map(), nil
-}
-
 // DeleteServiceAccount - delete a service account
 func (sys *IAMSys) DeleteServiceAccount(ctx context.Context, accessKey string, notifyPeers bool) error {
 	if !sys.Initialized() {

cmd/site-replication.go

@@ -1256,9 +1256,10 @@ func (c *SiteReplicationSys) IAMChangeHook(ctx context.Context, item madmin.SRIA
 	// The IAM item has already been applied to the local cluster at this
 	// point, and only needs to be updated on all remote peer clusters.
 
-	c.RLock()
-	defer c.RUnlock()
-	if !c.enabled {
+	// What is sent is read from storage before the lock is taken: loading
+	// an identity asks whether site replication is enabled, and a read lock
+	// asked for a second time behind a waiting writer is never granted.
+	if !c.isEnabled() {
 		return nil
 	}
 
@@ -1284,6 +1285,12 @@ func (c *SiteReplicationSys) IAMChangeHook(ctx context.Context, item madmin.SRIA
 	if err != nil {
 		return err
 	}
+
+	c.RLock()
+	defer c.RUnlock()
+	if !c.enabled {
+		return nil
+	}
 	cerr := c.concDo(nil, func(d string, p madmin.PeerInfo) error {
 		admClient, err := c.getAdminClient(ctx, d)
 		if err != nil {
@@ -2365,41 +2372,11 @@ func (c *SiteReplicationSys) syncToAllPeers(ctx context.Context, addOpts madmin.
 				continue
 			}
 
-			claims, err := globalIAMSys.GetClaimsForSvcAcc(ctx, acc.Credentials.AccessKey)
-			if err != nil {
-				return errSRBackendIssue(err)
-			}
-
-			_, policy, err := globalIAMSys.GetServiceAccount(ctx, acc.Credentials.AccessKey)
-			if err != nil {
-				return errSRBackendIssue(err)
-			}
-
-			var policyJSON []byte
-			if policy != nil {
-				policyJSON, err = json.Marshal(policy)
-				if err != nil {
-					return wrapSRErr(err)
-				}
-			}
-
+			// The hook sends the account as it is stored (see replicationItem).
 			err = c.IAMChangeHook(ctx, madmin.SRIAMItem{
-				Type: madmin.SRIAMItemSvcAcc,
-				SvcAccChange: &madmin.SRSvcAccChange{
-					Create: &madmin.SRSvcAccCreate{
-						Parent:        acc.Credentials.ParentUser,
-						AccessKey:     user,
-						SecretKey:     acc.Credentials.SecretKey,
-						Groups:        acc.Credentials.Groups,
-						Claims:        claims,
-						SessionPolicy: policyJSON,
-						Status:        acc.Credentials.Status,
-						Name:          acc.Credentials.Name,
-						Description:   acc.Credentials.Description,
-						Expiration:    &acc.Credentials.Expiration,
-					},
-				},
-				UpdatedAt: acc.UpdatedAt,
+				Type:         madmin.SRIAMItemSvcAcc,
+				SvcAccChange: &madmin.SRSvcAccChange{Create: &madmin.SRSvcAccCreate{AccessKey: user}},
+				UpdatedAt:    acc.UpdatedAt,
 			})
 			if err != nil {
 				return errSRIAMError(err)
@@ -5485,50 +5462,12 @@ func (c *SiteReplicationSys) healUsers(ctx context.Context, objAPI ObjectLayer,
 		}
 		creds := u.Credentials
 		if creds.IsServiceAccount() {
-			claims, err := globalIAMSys.GetClaimsForSvcAcc(ctx, creds.AccessKey)
-			if err != nil {
-				replLogOnceIf(ctx,
-					fmt.Errorf("Unable to heal service account from peer site %s -> %s : %w", latestPeerName, peerName, err),
-					fmt.Sprintf("heal-user-%s", user))
-				continue
-			}
-
-			_, policy, err := globalIAMSys.GetServiceAccount(ctx, creds.AccessKey)
-			if err != nil {
-				replLogOnceIf(ctx,
-					fmt.Errorf("Unable to heal service account from peer site %s -> %s : %w", latestPeerName, peerName, err),
-					fmt.Sprintf("heal-user-%s", user))
-				continue
-			}
-
-			var policyJSON []byte
-			if policy != nil {
-				policyJSON, err = json.Marshal(policy)
-				if err != nil {
-					replLogOnceIf(ctx,
-						fmt.Errorf("Unable to heal service account from peer site %s -> %s : %w", latestPeerName, peerName, err),
-						fmt.Sprintf("heal-user-%s", user))
-					continue
-				}
-			}
-
+			// The hook sends the account as it is stored (see
+			// replicationItem), not as this server has it cached.
 			if err := c.IAMChangeHook(ctx, madmin.SRIAMItem{
-				Type: madmin.SRIAMItemSvcAcc,
-				SvcAccChange: &madmin.SRSvcAccChange{
-					Create: &madmin.SRSvcAccCreate{
-						Parent:        creds.ParentUser,
-						AccessKey:     creds.AccessKey,
-						SecretKey:     creds.SecretKey,
-						Groups:        creds.Groups,
-						Claims:        claims,
-						SessionPolicy: policyJSON,
-						Status:        creds.Status,
-						Name:          creds.Name,
-						Description:   creds.Description,
-						Expiration:    &creds.Expiration,
-					},
-				},
-				UpdatedAt: lastUpdate,
+				Type:         madmin.SRIAMItemSvcAcc,
+				SvcAccChange: &madmin.SRSvcAccChange{Create: &madmin.SRSvcAccCreate{AccessKey: creds.AccessKey}},
+				UpdatedAt:    lastUpdate,
 			}); err != nil {
 				replLogOnceIf(ctx,
 					fmt.Errorf("Unable to heal service account from peer site %s -> %s : %w", latestPeerName, peerName, err),