s3: keep service account keys when a pushed identity replaces its parent (#12394)

* s3: keep service account keys when a pushed identity replaces its parent

Service account changes push the parent user, whose proto carries only
the user's own keys. The merge replaced the cached parent with that
proto and dropped every service account key until a restart. Read the
parent's service accounts from the credential store so the push
rebuilds them, deleted and disabled ones included.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* s3: test that a parent identity push keeps its service account keys

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* s3: test a sibling service account survives a deletion push

Also check the restored secret and status, not just the key.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Chris LuandClaude Opus 5.5 authored and GitHub committed 2026-11-04 05:26:07 -08:00
1 parent 92bdfeaf29
commit a0f30afe5f
2 files changed
+82

No files matched your search

+11
View File
@@ -1412,6 +1412,17 @@ func (iam *IdentityAccessManagement) UpsertIdentity(ident *iam_pb.Identity) erro
config.Policies = append(config.Policies, &iam_pb.Policy{Name: runtimeName, Content: string(content)})
}
}
// The proto carries only the user's own keys; the merge replaces the
// identity, so its service account keys have to come from the store.
serviceAccounts, err := iam.credentialManager.ListServiceAccounts(context.Background())
if err != nil {
return err
}
for _, sa := range serviceAccounts {
if sa.ParentUser == ident.Name {
config.ServiceAccounts = append(config.ServiceAccounts, sa)
}
}
}
if err := iam.MergeS3ApiConfiguration(config, false, false); err != nil {
return err
@@ -367,3 +367,74 @@ func TestPutGroupRaceKeepsNewerInlinePolicy(t *testing.T) {
s3a.iam.VerifyActionPermission(req, ident, s3_constants.ACTION_READ, "bucket", "obj"),
"a stale group push must not restore a grant the newer policy revoked")
}
// Service account changes push the parent identity, whose proto carries only
// the user's own keys. The upsert must rebuild the parent's service account
// keys from the store instead of dropping them.
func TestUpsertIdentityKeepsServiceAccountKeys(t *testing.T) {
s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{{
Name: "pacs",
Credentials: []*iam_pb.Credential{{AccessKey: "AKPACS", SecretKey: "secret"}},
Actions: []string{"Read:bucket"},
}})
cm := s3a.iam.credentialManager
ctx := context.Background()
expiration := time.Now().Add(time.Hour).Unix()
newServiceAccount := func(id, accessKey string) *iam_pb.ServiceAccount {
return &iam_pb.ServiceAccount{
Id: id,
ParentUser: "pacs",
Credential: &iam_pb.Credential{AccessKey: accessKey, SecretKey: "sa-secret", Status: "Active"},
Expiration: expiration,
}
}
first := newServiceAccount("sa:pacs:1", "AKSA1")
require.NoError(t, cm.CreateServiceAccount(ctx, first))
parent, err := cm.GetUser(ctx, "pacs")
require.NoError(t, err)
require.NoError(t, s3a.iam.ReplaceS3ApiConfiguration(&iam_pb.S3ApiConfiguration{
Identities: []*iam_pb.Identity{parent},
ServiceAccounts: []*iam_pb.ServiceAccount{first},
}))
second := newServiceAccount("sa:pacs:2", "AKSA2")
require.NoError(t, cm.CreateServiceAccount(ctx, second))
pushed, err := cm.GetUser(ctx, "pacs")
require.NoError(t, err)
require.NoError(t, s3a.iam.UpsertIdentity(pushed))
for _, accessKey := range []string{"AKPACS", "AKSA1", "AKSA2"} {
ident, cred, found := s3a.iam.LookupByAccessKey(accessKey)
require.True(t, found, "access key %s must survive the parent push", accessKey)
assert.Equal(t, "pacs", ident.Name)
if accessKey != "AKPACS" {
assert.Equal(t, "sa-secret", cred.SecretKey)
assert.Equal(t, "Active", cred.Status)
assert.Equal(t, expiration, cred.Expiration)
}
}
require.NoError(t, cm.DeleteServiceAccount(ctx, first.Id))
pushed, err = cm.GetUser(ctx, "pacs")
require.NoError(t, err)
require.NoError(t, s3a.iam.UpsertIdentity(pushed))
_, _, found := s3a.iam.LookupByAccessKey("AKSA1")
assert.False(t, found, "a deleted service account must stop authenticating")
_, _, found = s3a.iam.LookupByAccessKey("AKSA2")
assert.True(t, found, "a sibling service account must survive another one's deletion")
disabled := newServiceAccount(second.Id, "AKSA2")
disabled.Disabled = true
require.NoError(t, cm.UpdateServiceAccount(ctx, second.Id, disabled))
pushed, err = cm.GetUser(ctx, "pacs")
require.NoError(t, err)
require.NoError(t, s3a.iam.UpsertIdentity(pushed))
_, _, found = s3a.iam.LookupByAccessKey("AKSA1")
assert.False(t, found)
_, _, found = s3a.iam.LookupByAccessKey("AKSA2")
assert.False(t, found, "a disabled service account must stop authenticating")
_, _, found = s3a.iam.LookupByAccessKey("AKPACS")
assert.True(t, found)
}