From 0cd388c666f8211f552604ae73448a066e9c25ce Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sun, 18 Oct 2026 22:39:12 +0800 Subject: [PATCH] fix(iam): persist and propagate scoped STS revocation Enumerate durable STS identities for parent/type revocation, retain the operation cutoff, and notify sibling caches for each committed deletion. Repair missed notifications during periodic object/etcd IAM refresh. Carry immutable STS tombstones and expiry through the existing IAM revision protocol and deletion healing. Delayed creation cannot undo revocation, while unrelated and newly issued keys stay valid. Keep partial failures visible while propagating the successfully committed deletions. Cover uncached credentials, warm sibling caches, dropped notifications, object/etcd backends, reordered replay, concurrent issuance and partial persistence failure. No new dependencies or authentication-path I/O. Signed-off-by: Feng Ruohang --- cmd/iam-replication-protocol.go | 18 +++ cmd/iam-revision.go | 40 ++++- cmd/iam-store.go | 70 ++++++--- cmd/iam-sts-revocation_test.go | 254 ++++++++++++++++++++++++++++++++ cmd/iam.go | 9 +- 5 files changed, 365 insertions(+), 26 deletions(-) create mode 100644 cmd/iam-sts-revocation_test.go diff --git a/cmd/iam-replication-protocol.go b/cmd/iam-replication-protocol.go index c26d3586b..ec2061e99 100644 --- a/cmd/iam-replication-protocol.go +++ b/cmd/iam-replication-protocol.go @@ -29,6 +29,7 @@ const ( iamRevisionPeerPath = "/v3/site-replication/peer/iam-revisions" iamUserBoundaryType = "silo-user-revocation" iamGroupBoundaryType = "silo-group-revocation" + iamSTSDeleteType = "silo-sts-revocation" maxIAMRevisionBatch = 128 ) @@ -49,6 +50,7 @@ type iamGroupBoundary struct { // before applying any change that would lose revocation or member metadata. type iamReplicationItem struct { madmin.SRIAMItem + STSExpiresAt time.Time `json:"stsExpiresAt,omitempty"` GroupGrants map[string]time.Time `json:"groupGrants,omitempty"` GroupSnapshot bool `json:"groupSnapshot,omitempty"` UserRevocation *iamUserBoundary `json:"userRevocation,omitempty"` @@ -155,6 +157,13 @@ type ( func (c *SiteReplicationSys) replicationItem(ctx context.Context, item madmin.SRIAMItem) (iamReplicationItem, error) { out := iamReplicationItem{SRIAMItem: item} + if item.Type == iamSTSDeleteType { + r, err := loadIAMRevision(ctx, globalIAMSys.store, getUserIdentityPath(item.Name, stsUser)) + if err != nil { + return out, err + } + out.STSExpiresAt = r.ExpiresAt + } if item.Type == madmin.SRIAMItemSvcAcc && item.SvcAccChange != nil { var key string if item.SvcAccChange.Create != nil { @@ -242,6 +251,15 @@ func applyIAMReplicationItem(ctx context.Context, item iamReplicationItem) error ctx = context.WithValue(ctx, iamRecordBoundaryKey{}, item.RevokedBefore) } switch item.Type { + case iamSTSDeleteType: + if item.Name == "" || strings.ContainsAny(item.Name, "/\\") || item.UpdatedAt.IsZero() { + return errSRInvalidRequest(errInvalidArgument) + } + if err := globalIAMSys.store.revokeTempUser(withIAMReplicationTime(ctx, item.UpdatedAt), item.Name, item.STSExpiresAt); err != nil { + return iamReplicationError(err) + } + globalIAMSys.notifyForUser(ctx, item.Name, true) + return nil case iamUserBoundaryType: if item.UserRevocation == nil || item.UserRevocation.User == "" || item.UserRevocation.Before.IsZero() { return errSRInvalidRequest(errInvalidArgument) diff --git a/cmd/iam-revision.go b/cmd/iam-revision.go index 8e4a106a2..3ce8cb639 100644 --- a/cmd/iam-revision.go +++ b/cmd/iam-revision.go @@ -348,6 +348,13 @@ func saveIAMRevision(ctx context.Context, store IAMStorageAPI, path string, item // STS access keys identify immutable tokens, not reusable user names. return errIAMStaleUpdate } + if strings.HasPrefix(path, iamConfigSTSPrefix) && deleted { + // STS keys are immutable. A delayed revocation must also defeat a + // replayed creation that was recorded with a newer event timestamp. + if origin, replicated := iamReplicationTime(ctx); replicated && previous.timestamp().After(origin) { + ctx = withIAMReplicationTime(ctx, previous.timestamp()) + } + } if origin, replicated := iamReplicationTime(ctx); replicated { *at = origin if previous.timestamp().After(origin) || (previous.Deleted && !deleted && !origin.After(previous.timestamp())) { @@ -394,11 +401,18 @@ func saveIAMRevision(ctx context.Context, store IAMStorageAPI, path string, item // Retain only the parent name for root-account exclusion during heal. v.Credentials = auth.Credentials{ParentUser: previous.Credentials.ParentUser} v.RevokedBefore = *at - if strings.HasPrefix(path, iamConfigSTSPrefix) && !previous.Credentials.Expiration.IsZero() && !previous.Credentials.Expiration.Equal(timeSentinel) { - // The signed STS token cannot authorize beyond this time, even - // if an offline site replays it with a newer event timestamp. - v.ExpiresAt = previous.Credentials.Expiration.Add(globalMaxSkewTime) - opts = []options{{ttl: max(1, int64(time.Until(v.ExpiresAt).Seconds())+1)}} + if strings.HasPrefix(path, iamConfigSTSPrefix) { + // Carry expiry when a revocation arrives before the token itself. + // It is safe to forget this key only after its signed lifetime. + if expiration := previous.Credentials.Expiration; !expiration.IsZero() && !expiration.Equal(timeSentinel) && expiration.Add(globalMaxSkewTime).After(v.ExpiresAt) { + v.ExpiresAt = expiration.Add(globalMaxSkewTime) + } + if previous.ExpiresAt.After(v.ExpiresAt) { + v.ExpiresAt = previous.ExpiresAt + } + if !v.ExpiresAt.IsZero() { + opts = []options{{ttl: max(1, int64(time.Until(v.ExpiresAt).Seconds())+1)}} + } } } else { if v.Credentials.SessionToken != "" && v.Credentials.Claims == nil { @@ -489,6 +503,12 @@ func iamDeletionItem(path string, r iamRevision) (item madmin.SRIAMItem, ok bool name := strings.TrimSuffix(strings.TrimPrefix(path, iamConfigUsersPrefix), "/"+iamIdentityFile) item.Type = madmin.SRIAMItemIAMUser item.IAMUser = &madmin.SRIAMUser{AccessKey: name, IsDeleteReq: true} + case strings.HasPrefix(path, iamConfigSTSPrefix): + if !r.ExpiresAt.IsZero() && UTCNow().After(r.ExpiresAt) { + return item, false + } + item.Type = iamSTSDeleteType + item.Name = strings.TrimSuffix(strings.TrimPrefix(path, iamConfigSTSPrefix), "/"+iamIdentityFile) case strings.HasPrefix(path, iamConfigServiceAccountsPrefix): name := strings.TrimSuffix(strings.TrimPrefix(path, iamConfigServiceAccountsPrefix), "/"+iamIdentityFile) if name == siteReplicatorSvcAcc || r.Credentials.ParentUser == globalActiveCred.AccessKey { @@ -518,8 +538,7 @@ func iamDeletionItem(path string, r iamRevision) (item madmin.SRIAMItem, ok bool item.Type = madmin.SRIAMItemPolicyMapping item.PolicyMapping = &madmin.SRPolicyMapping{UserOrGroup: strings.TrimSuffix(name, ".json"), UserType: int(typ), IsGroup: prefix == "groups"} default: - // Expired STS credentials are not replayed. Parent revocations and - // their retained timestamp reject delayed copies of derived tokens. + // Other config paths do not represent replicated IAM deletions. return item, false } return item, true @@ -527,6 +546,8 @@ func iamDeletionItem(path string, r iamRevision) (item madmin.SRIAMItem, ok bool func iamDeletionPath(item madmin.SRIAMItem) string { switch item.Type { + case iamSTSDeleteType: + return getUserIdentityPath(item.Name, stsUser) case madmin.SRIAMItemIAMUser: if item.IAMUser != nil && item.IAMUser.IsDeleteReq { return getUserIdentityPath(item.IAMUser.AccessKey, regUser) @@ -582,6 +603,9 @@ func (c *SiteReplicationSys) healIAMDeletions(ctx context.Context) (err error) { continue } out := iamReplicationItem{SRIAMItem: item} + if item.Type == iamSTSDeleteType { + out.STSExpiresAt = r.ExpiresAt + } if item.Type == madmin.SRIAMItemIAMUser && !r.Deleted { out.Type, out.IAMUser = iamUserBoundaryType, nil out.UserRevocation = &iamUserBoundary{User: item.IAMUser.AccessKey, Before: r.RevokedBefore} @@ -593,7 +617,7 @@ func (c *SiteReplicationSys) healIAMDeletions(ctx context.Context) (err error) { byType[item.Type] = append(byType[item.Type], out) } var items []iamReplicationItem - for _, typ := range []string{madmin.SRIAMItemPolicyMapping, madmin.SRIAMItemIAMUser, madmin.SRIAMItemSvcAcc, madmin.SRIAMItemGroupInfo, madmin.SRIAMItemPolicy} { + for _, typ := range []string{madmin.SRIAMItemPolicyMapping, madmin.SRIAMItemIAMUser, madmin.SRIAMItemSvcAcc, iamSTSDeleteType, madmin.SRIAMItemGroupInfo, madmin.SRIAMItemPolicy} { items = append(items, byType[typ]...) } if len(items) == 0 { diff --git a/cmd/iam-store.go b/cmd/iam-store.go index fca9cbb1e..05ec9e6d9 100644 --- a/cmd/iam-store.go +++ b/cmd/iam-store.go @@ -711,6 +711,11 @@ func (store *IAMStoreSys) LoadIAMCache(ctx context.Context, firstTime bool) erro if err := store.loadUsers(ctx, svcUser, newCache.iamUsersMap); err != nil { return err } + // Rebuild STS revisions as well: a missed etcd watch event must not + // leave a cached token valid or hide its tombstone after a restart. + if err := store.loadUsers(ctx, stsUser, newCache.iamSTSAccountsMap); err != nil { + return err + } newCache.buildUserGroupMemberships() } @@ -754,6 +759,14 @@ func (store *IAMStoreSys) LoadIAMCache(ctx context.Context, firstTime bool) erro cache.updatedAt = time.Now() } + // Revision loading is independent of cache replacement. Even if ongoing + // IAM writes defer the full swap, a missed STS notification must converge. + for key := range cache.iamSTSAccountsMap { + if store.revisionIndex().get(getUserIdentityPath(key, stsUser)).Deleted { + delete(cache.iamSTSAccountsMap, key) + } + } + return nil } @@ -2184,45 +2197,68 @@ func (store *IAMStoreSys) SetTempUser(ctx context.Context, accessKey string, cre // RevokeTokens - revokes all temporary credentials, or those with matching type, // associated with the parent user. -func (store *IAMStoreSys) RevokeTokens(ctx context.Context, parentUser string, tokenRevokeType string) error { +func (store *IAMStoreSys) RevokeTokens(ctx context.Context, parentUser string, tokenRevokeType string) ([]string, error) { if parentUser == "" { - return errInvalidArgument + return nil, errInvalidArgument } - + before := UTCNow() cache := store.lock() defer store.unlock() - secret, err := getTokenSigningKey() if err != nil { - return err + return nil, err } - - var revoked bool - for _, ui := range cache.iamSTSAccountsMap { - if ui.Credentials.ParentUser != parentUser { + // Another node may have issued a token whose notification was lost. + // Enumerate durable identities, retaining cached entries for retrying a + // partially propagated revocation. Do not hold up cached authentication. + accounts := maps.Clone(cache.iamSTSAccountsMap) + if err := store.withIAMStorage(ctx, func(ctx context.Context) error { + return store.loadUsers(ctx, stsUser, accounts) + }); err != nil { + return nil, err + } + var revoked []string + var failures []error + for key, ui := range accounts { + if ui.Credentials.ParentUser != parentUser || ui.UpdatedAt.After(before) || ui.Credentials.IsExpired() { continue } if tokenRevokeType != "" { claims, err := getClaimsFromTokenWithSecret(ui.Credentials.SessionToken, secret) if err != nil { - continue // skip if token is invalid + continue } - // skip if token type is given and does not match if v, _ := claims.Lookup(tokenRevokeTypeClaim); v != tokenRevokeType { continue } } - if err := store.deleteUserIdentity(ctx, ui.Credentials.AccessKey, stsUser); err != nil { - return err + if err := store.deleteUserIdentity(ctx, key, stsUser); err != nil { + failures = append(failures, err) + continue } - delete(cache.iamSTSAccountsMap, ui.Credentials.AccessKey) - revoked = true + delete(cache.iamSTSAccountsMap, key) + revoked = append(revoked, key) } - - if revoked { + if len(revoked) != 0 { cache.updatedAt = time.Now() } + return revoked, errors.Join(failures...) +} +// revokeTempUser applies an immutable token's replicated tombstone without +// touching the parent, its policies, or other (including newer) tokens. +func (store *IAMStoreSys) revokeTempUser(ctx context.Context, key string, expiresAt time.Time) error { + cache := store.lock() + defer store.unlock() + u := UserIdentity{Version: 1, Deleted: true, ExpiresAt: expiresAt} + if err := store.saveUserIdentity(ctx, key, stsUser, &u); err != nil { + return err + } + delete(cache.iamSTSAccountsMap, key) + cache.updatedAt = time.Now() + if store.group != nil { + store.group.Forget(key) + } return nil } diff --git a/cmd/iam-sts-revocation_test.go b/cmd/iam-sts-revocation_test.go new file mode 100644 index 000000000..6b9931ef0 --- /dev/null +++ b/cmd/iam-sts-revocation_test.go @@ -0,0 +1,254 @@ +// Copyright (c) 2026 Feng Ruohang +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cmd + +import ( + "context" + "errors" + "fmt" + "strings" + "testing" + "time" + + "github.com/minio/madmin-go/v3" + "github.com/minio/minio/internal/auth" + "github.com/minio/minio/internal/grid" + xnet "github.com/pgsty/silo-pkg/v3/net" +) + +func TestIAMSTSRevocationDurableScope(t *testing.T) { + for _, backend := range []string{"object", "etcd"} { + for _, notify := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/notify=%t", backend, notify), func(t *testing.T) { + ctx, sys, obj := prepareIAMRevisionFixture(t, backend) + for _, parent := range []string{"sts-parent", "other-parent"} { + _, err := sys.CreateUser(ctx, parent, madmin.AddOrUpdateUserReq{SecretKey: "synthetic-parent-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + } + sibling := &IAMStoreSys{IAMStorageAPI: newIAMObjectStore(obj, MinIOUsersSysType)} + if backend == "etcd" { + sibling.IAMStorageAPI = newIAMEtcdStore(sys.store.IAMStorageAPI.(*IAMEtcdStore).client, MinIOUsersSysType) + } + mustIAM(t, sibling.LoadIAMCache(ctx, false)) + secret, err := getTokenSigningKey() + mustIAM(t, err) + issue := func(store *IAMStoreSys, parent, typ string) auth.Credentials { + cred, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: parent, tokenRevokeTypeClaim: typ}, secret) + mustIAM(t, err) + cred.ParentUser = parent + _, err = store.SetTempUser(ctx, cred.AccessKey, cred, "") + mustIAM(t, err) + return cred + } + first := issue(sys.store, "sts-parent", "selected") + uncached := issue(sibling, "sts-parent", "selected") + otherType := issue(sys.store, "sts-parent", "other-type") + otherUser := issue(sys.store, "other-parent", "selected") + if _, ok := sys.store.GetUser(uncached.AccessKey); ok { + t.Fatal("fixture cached the remote token") + } + for _, c := range []auth.Credentials{first, otherType, otherUser} { + mustIAM(t, sibling.UserNotificationHandler(ctx, c.AccessKey, stsUser)) + } + prior := globalNotificationSys + globalNotificationSys = &NotificationSys{} + t.Cleanup(func() { globalNotificationSys = prior }) + if notify { + tg, err := grid.SetupTestGrid(2) + mustIAM(t, err) + t.Cleanup(tg.Cleanup) + mustIAM(t, loadUserRPC.Register(tg.Managers[1], func(r *grid.MSS) (grid.NoPayload, *grid.RemoteErr) { + if err := sibling.UserNotificationHandler(ctx, r.Get(peerRESTUser), stsUser); err != nil { + return grid.NoPayload{}, grid.NewRemoteErr(err) + } + return grid.NoPayload{}, nil + })) + host, err := xnet.ParseHost(strings.TrimPrefix(tg.Hosts[1], "http://")) + mustIAM(t, err) + globalNotificationSys = &NotificationSys{peerClients: []*peerRESTClient{{host: host, gridConn: func() *grid.Connection { return tg.Managers[0].Connection(tg.Hosts[1]) }}}} + } + mustIAM(t, sys.RevokeTokens(ctx, "sts-parent", "selected")) + for _, c := range []auth.Credentials{first, uncached} { + r, err := loadIAMRevision(ctx, sys.store, getUserIdentityPath(c.AccessKey, stsUser)) + mustIAM(t, err) + if !r.Deleted { + t.Errorf("durable token was omitted: %s", c.AccessKey) + } + if _, ok := iamDeletionItem(getUserIdentityPath(c.AccessKey, stsUser), r); !ok { + t.Error("STS revocation cannot be replayed") + } + if notify && backend == "object" { + if _, ok := sibling.GetUser(c.AccessKey); ok { + t.Error("notified sibling accepted revoked token") + } + } + } + fresh := issue(sibling, "sts-parent", "selected") + // A missed notification must converge through the normal cache refresh. + mustIAM(t, sibling.LoadIAMCache(ctx, false)) + for _, c := range []auth.Credentials{first, uncached} { + if _, ok := sibling.GetUser(c.AccessKey); ok { + t.Error("refresh retained revoked token") + } + } + for _, c := range []auth.Credentials{otherType, otherUser, fresh} { + mustIAM(t, sibling.UserNotificationHandler(ctx, c.AccessKey, stsUser)) + if _, ok := sibling.GetUser(c.AccessKey); !ok { + t.Error("revocation removed an out-of-scope or new token") + } + } + if _, ok := sibling.GetUser("sts-parent"); !ok { + t.Error("revocation removed parent") + } + }) + } + } +} + +// Deletion is keyed by the immutable STS access key, so delayed delivery cannot +// affect a newly issued token and a delayed creation cannot undo revocation. +func TestIAMSTSRevocationReorderedReplay(t *testing.T) { + for _, backend := range []string{"object", "etcd"} { + for _, missing := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/missing=%t", backend, missing), func(t *testing.T) { + ctx, sys, _ := prepareIAMRevisionFixture(t, backend) + const parent = "sts-replay-parent" + _, err := sys.CreateUser(ctx, parent, madmin.AddOrUpdateUserReq{SecretKey: "synthetic-parent-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + secret, err := getTokenSigningKey() + mustIAM(t, err) + cred, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: parent}, secret) + mustIAM(t, err) + cred.ParentUser = parent + created := UTCNow().Add(-time.Minute) + peer := madmin.SRSTSCredential{AccessKey: cred.AccessKey, SecretKey: cred.SecretKey, SessionToken: cred.SessionToken, ParentUser: parent} + if !missing { + mustIAM(t, globalSiteReplicationSys.PeerSTSAccHandler(ctx, &peer, created)) + } + fresh, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: parent}, secret) + mustIAM(t, err) + fresh.ParentUser = parent + _, err = sys.SetTempUser(ctx, fresh.AccessKey, fresh, "") + mustIAM(t, err) + // Older than the live record on this peer: deletion must still win. + item := iamReplicationItem{SRIAMItem: madmin.SRIAMItem{Type: iamSTSDeleteType, Name: cred.AccessKey, UpdatedAt: created.Add(-time.Second)}, STSExpiresAt: cred.Expiration.Add(globalMaxSkewTime)} + for range 2 { + mustIAM(t, applyIAMReplicationItem(ctx, item)) + mustIAM(t, globalSiteReplicationSys.PeerSTSAccHandler(ctx, &peer, UTCNow())) + mustIAM(t, sys.store.LoadIAMCache(ctx, false)) + if _, ok := sys.GetUser(ctx, cred.AccessKey); ok { + t.Fatal("delayed STS creation undid revocation") + } + if _, ok := sys.GetUser(ctx, fresh.AccessKey); !ok { + t.Fatal("delayed revocation removed new token") + } + } + r, err := loadIAMRevision(ctx, sys.store, getUserIdentityPath(cred.AccessKey, stsUser)) + mustIAM(t, err) + if !r.Deleted || !r.ExpiresAt.Equal(item.STSExpiresAt) || r.Credentials.SessionToken != "" || r.Credentials.SecretKey != "" { + t.Fatal("revocation lost expiry or retained secret material") + } + for _, status := range []madmin.AccountStatus{madmin.AccountDisabled, madmin.AccountEnabled} { + _, err = sys.SetUserStatus(ctx, parent, status) + mustIAM(t, err) + } + if _, ok := sys.GetUser(ctx, cred.AccessKey); ok { + t.Fatal("parent re-enable revived revoked token") + } + }) + } + } +} + +// Pause the durable scan while another node persists a new credential. The +// revocation's original cutoff must survive the scan and leave that token live. +type iamSTSScanGateStore struct { + IAMStorageAPI + entered chan struct{} + resume chan struct{} +} + +func (s *iamSTSScanGateStore) loadUsers(ctx context.Context, typ IAMUserType, users map[string]UserIdentity) error { + close(s.entered) + select { + case <-s.resume: + case <-ctx.Done(): + return ctx.Err() + } + return s.IAMStorageAPI.loadUsers(ctx, typ, users) +} + +func TestIAMSTSRevocationConcurrentIssuance(t *testing.T) { + ctx, sys, obj := prepareIAMRevisionFixture(t) + _, err := sys.CreateUser(ctx, "sts-race-parent", madmin.AddOrUpdateUserReq{SecretKey: "synthetic-parent-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + sibling := &IAMStoreSys{IAMStorageAPI: newIAMObjectStore(obj, MinIOUsersSysType)} + secret, err := getTokenSigningKey() + mustIAM(t, err) + issue := func() auth.Credentials { + c, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: "sts-race-parent"}, secret) + mustIAM(t, err) + c.ParentUser = "sts-race-parent" + _, err = sibling.SetTempUser(ctx, c.AccessKey, c, "") + mustIAM(t, err) + return c + } + old := issue() + gate := &iamSTSScanGateStore{IAMStorageAPI: sys.store.IAMStorageAPI, entered: make(chan struct{}), resume: make(chan struct{})} + sys.store.IAMStorageAPI = gate + done := make(chan error, 1) + go func() { done <- sys.RevokeTokens(ctx, "sts-race-parent", "") }() + <-gate.entered + fresh := issue() + close(gate.resume) + mustIAM(t, <-done) + mustIAM(t, sibling.UserNotificationHandler(ctx, old.AccessKey, stsUser)) + if _, ok := sibling.GetUser(old.AccessKey); ok { + t.Fatal("old uncached token survived") + } + mustIAM(t, sibling.UserNotificationHandler(ctx, fresh.AccessKey, stsUser)) + if _, ok := sibling.GetUser(fresh.AccessKey); !ok { + t.Fatal("new token was included in earlier revocation") + } +} + +type iamSTSFailureStore struct { + IAMStorageAPI + failedPath string +} + +func (s *iamSTSFailureStore) saveIAMConfig(ctx context.Context, item any, path string, opts ...options) error { + if path == s.failedPath { + return errIAMInjectedWrite + } + return s.IAMStorageAPI.saveIAMConfig(ctx, item, path, opts...) +} + +func TestIAMSTSRevocationPartialFailure(t *testing.T) { + ctx, sys, _ := prepareIAMRevisionFixture(t) + _, err := sys.CreateUser(ctx, "sts-partial-parent", madmin.AddOrUpdateUserReq{SecretKey: "synthetic-parent-password", Status: madmin.AccountEnabled}) + mustIAM(t, err) + secret, err := getTokenSigningKey() + mustIAM(t, err) + var keys []string + for range 2 { + c, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: "sts-partial-parent"}, secret) + mustIAM(t, err) + c.ParentUser = "sts-partial-parent" + _, err = sys.SetTempUser(ctx, c.AccessKey, c, "") + mustIAM(t, err) + keys = append(keys, c.AccessKey) + } + sys.store.IAMStorageAPI = &iamSTSFailureStore{IAMStorageAPI: sys.store.IAMStorageAPI, failedPath: getUserIdentityPath(keys[0], stsUser)} + revoked, err := sys.store.RevokeTokens(ctx, "sts-partial-parent", "") + if !errors.Is(err, errIAMInjectedWrite) || len(revoked) != 1 || revoked[0] != keys[1] { + t.Fatalf("committed keys or failure lost: %v %v", revoked, err) + } + if _, ok := sys.store.GetUser(keys[1]); ok { + t.Fatal("committed revocation retained cache entry") + } + if _, ok := sys.store.GetUser(keys[0]); !ok { + t.Fatal("failed revocation claimed cache success") + } +} diff --git a/cmd/iam.go b/cmd/iam.go index 4e52b72ba..1566d81d0 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -717,7 +717,14 @@ func (sys *IAMSys) RevokeTokens(ctx context.Context, accessKey, tokenRevokeType return errServerNotInitialized } - return sys.store.RevokeTokens(ctx, accessKey, tokenRevokeType) + keys, err := sys.store.RevokeTokens(ctx, accessKey, tokenRevokeType) + // Publish every committed tombstone even if another token failed to save. + for _, key := range keys { + sys.notifyForUser(ctx, key, true) + // Offline peers recover this same record through IAM deletion healing. + iamLogIf(ctx, globalSiteReplicationSys.IAMChangeHook(ctx, madmin.SRIAMItem{Type: iamSTSDeleteType, Name: key})) + } + return err } // DeleteUser - delete user (only for long-term users not STS users).