diff --git a/cmd/iam-store.go b/cmd/iam-store.go index 05eb795f1..9c9538569 100644 --- a/cmd/iam-store.go +++ b/cmd/iam-store.go @@ -2854,12 +2854,12 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st u, _, err := store.editStoredIdentity(ctx, cache, accessKey, svcUser, func(stored *UserIdentity) error { if !stored.Credentials.IsServiceAccount() { - return errNoSuchServiceAccount + return errIAMIdentityGone } if opts.parentUser != "" && stored.Credentials.ParentUser != opts.parentUser { // Not the account the caller was authorized for: it was // deleted and created again for another parent meanwhile. - return errNoSuchServiceAccount + return errIAMIdentityGone } cr, err := updatedServiceAccountCredentials(ctx, accessKey, stored.Credentials, opts) if err != nil { @@ -3168,7 +3168,7 @@ func (store *IAMStoreSys) RefreshUserGroups(ctx context.Context, cred auth.Crede stored.Credentials.Groups = groups return nil }) - if errors.Is(err, errIAMIdentityGone) { + if errors.Is(err, errIAMIdentityGone) || errors.Is(err, errIAMStaleUpdate) { return false, nil } if err != nil { @@ -3181,12 +3181,15 @@ func (store *IAMStoreSys) RefreshUserGroups(ctx context.Context, cred auth.Crede // is stored now, under its revision lock (see iamIdentityEdit), and caches // the result. The caller holds the cache lock. When the identity is absent // or deleted in storage, or edit returns errIAMIdentityGone because it is -// no longer the identity the edit addressed, the cached copy is dropped, so -// a lookup loads what is stored, and errIAMIdentityGone is returned. +// no longer the identity the edit addressed, errIAMIdentityGone is +// returned; a replicated edit older than what is stored returns +// errIAMStaleUpdate. Either way storage holds something else than the +// cached copy, which is dropped, so a lookup loads what is stored. func (store *IAMStoreSys) editStoredIdentity(ctx context.Context, cache *iamCache, accessKey string, userType IAMUserType, edit func(u *UserIdentity) error) (UserIdentity, bool, error) { op := &iamIdentityEdit{accessKey: accessKey, edit: edit} err := store.saveIAMRevision(ctx, getUserIdentityPath(accessKey, userType), op) - if errors.Is(err, errIAMIdentityGone) { + if errors.Is(err, errIAMIdentityGone) || errors.Is(err, errIAMStaleUpdate) { + // Storage holds something else than the cached copy: drop it. store.forgetCachedIdentity(cache, accessKey, userType) } if err != nil { diff --git a/cmd/iam-stored-identity-edit_test.go b/cmd/iam-stored-identity-edit_test.go index 6db73081a..42c22c437 100644 --- a/cmd/iam-stored-identity-edit_test.go +++ b/cmd/iam-stored-identity-edit_test.go @@ -231,3 +231,42 @@ func TestIAMUserStatusOnServiceAccount(t *testing.T) { t.Error("a user secret change of a service account touched the account") } } + +// A server whose cache still holds the account of the first parent drops +// it when its edit is refused, whether the edit was bound to that parent or +// replicated from before the deletion: storage says the cached copy is gone. +func TestIAMRefusedEditDropsStaleCache(t *testing.T) { + t.Run("rebound-parent", func(t *testing.T) { + f := newGroupRefreshFixture(t) + snapshot := f.service(t, "edit-rebind") + other := f.sibling(t) + next := iamAdminUser(f.ctx, t, f.sys, "edit-rebind-next", "refresh-parent") + mustIAM(t, f.sys.DeleteServiceAccount(f.ctx, snapshot.AccessKey, false)) + _, _, err := f.sys.NewServiceAccount(f.ctx, next.AccessKey, nil, newServiceAccountOpts{accessKey: snapshot.AccessKey, secretKey: "edit-rebind-secret"}) + mustIAM(t, err) + if cached, ok := other.GetUser(snapshot.AccessKey); !ok || cached.Credentials.ParentUser != snapshot.ParentUser { + t.Fatal("control: the stale server must still cache the first account") + } + _, err = other.UpdateServiceAccount(f.ctx, snapshot.AccessKey, updateServiceAccountOpts{description: "stale", parentUser: snapshot.ParentUser}) + if !errors.Is(err, errNoSuchServiceAccount) { + t.Errorf("got %v, want errNoSuchServiceAccount", err) + } + if _, ok := other.GetUser(snapshot.AccessKey); ok { + t.Error("the stale server still caches the account of the first parent") + } + }) + t.Run("replicated-stale", func(t *testing.T) { + f := newGroupRefreshFixture(t) + snapshot := f.service(t, "edit-repl-stale") + other := f.sibling(t) + before := UTCNow() + mustIAM(t, f.sys.DeleteServiceAccount(f.ctx, snapshot.AccessKey, false)) + _, err := other.UpdateServiceAccount(withIAMReplicationTime(f.ctx, before), snapshot.AccessKey, updateServiceAccountOpts{description: "late"}) + if iamReplicationError(err) != nil { + t.Errorf("an older replicated edit must be acknowledged, got %v", err) + } + if _, ok := other.GetUser(snapshot.AccessKey); ok { + t.Error("the server that acknowledged a stale edit still caches the deleted account") + } + }) +}