Files
minio/cmd/batch-submitter-edits_test.go
T
Feng Ruohang a0de92fcbe fix(iam): refresh LDAP groups on the stored credential, not a stale snapshot
The hourly LDAP group refresh, which every server runs, reads the STS and
service-account credentials from its own cache, queries the directory,
and wrote each credential whose groups changed back whole through
UpdateUserIdentity. A secret rotation or an edit made meanwhile, on this
server or one whose notification it missed, was undone, and a credential
deleted meanwhile came back enabled with its old secret.

RefreshUserGroups replaces that writer, whose only caller was the
refresh. Under the identity's cross-node revision lock it loads the
stored record, refuses one that is absent, deleted, expired or now has
another parent (whose groups the lookup did not compute), and otherwise
changes only the groups of the stored record. A record without its
access key gets it from the path, as the loaders do. A refused refresh
drops the stale entry from the cache, which loads what is stored on next
use; a written or unchanged one caches the stored record. An STS record
keeps the lease of its token's remaining lifetime.

Tests cover rotation and edit, deletion, a sibling server whose cache did
not hear of a rotation or a deletion, recreation under the same and under
another parent, unchanged groups, and a stored record without its access
key. Written back as the old snapshot write, every case fails.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-11-08 12:00:20 +08:00

125 lines
4.7 KiB
Go

// Copyright (c) 2026 PGSTY
// SPDX-License-Identifier: AGPL-3.0-or-later
package cmd
import (
"testing"
"time"
"github.com/minio/madmin-go/v3"
"github.com/minio/minio/internal/auth"
)
// Edits that keep the credential keep a job running; edits of the
// credential itself, and its status, still end it.
func TestBatchSubmitterCredentialEdits(t *testing.T) {
f := newBatchAuthzFixture(t)
batchAuthzPolicy(f.ctx, t, f.sys, "edit-parent", batchAuthzFullAccess)
parent := iamAdminUser(f.ctx, t, f.sys, "edit-parent", "edit-parent")
soon := UTCNow().Add(time.Hour)
later := UTCNow().Add(24 * time.Hour)
newService := func(t *testing.T, expiring bool) (auth.Credentials, *batchJobSubmitter) {
t.Helper()
var opts newServiceAccountOpts
if expiring {
opts.expiration = &soon
}
created, _, err := f.sys.NewServiceAccount(f.ctx, parent.AccessKey, nil, opts)
mustIAM(t, err)
stored, ok := f.sys.GetUser(f.ctx, created.AccessKey)
if !ok {
t.Fatal("service account missing")
}
sub, err := newBatchJobSubmitter(stored.Credentials)
mustIAM(t, err)
mustIAM(t, sub.authorize(f.ctx, batchContextRead(f)))
return stored.Credentials, sub
}
for _, tc := range []struct {
name string
expiring bool
edit func(t *testing.T, cred auth.Credentials)
keep bool
}{
{"description", false, func(t *testing.T, cred auth.Credentials) {
_, err := f.sys.UpdateServiceAccount(f.ctx, cred.AccessKey, updateServiceAccountOpts{description: "cosmetic"})
mustIAM(t, err)
}, true},
{"name", false, func(t *testing.T, cred auth.Credentials) {
_, err := f.sys.UpdateServiceAccount(f.ctx, cred.AccessKey, updateServiceAccountOpts{name: "renamed"})
mustIAM(t, err)
}, true},
{"expiry-extended", true, func(t *testing.T, cred auth.Credentials) {
_, err := f.sys.UpdateServiceAccount(f.ctx, cred.AccessKey, updateServiceAccountOpts{expiration: &later})
mustIAM(t, err)
}, true},
// An expiry makes a service account a temporary credential, which
// IsAllowed evaluates differently.
{"expiry-added", false, func(t *testing.T, cred auth.Credentials) {
_, err := f.sys.UpdateServiceAccount(f.ctx, cred.AccessKey, updateServiceAccountOpts{expiration: &later})
mustIAM(t, err)
}, false},
{"groups-refreshed", false, func(t *testing.T, cred auth.Credentials) {
// As the LDAP group refresh rewrites a derived credential.
u, ok := f.sys.GetUser(f.ctx, cred.AccessKey)
if !ok {
t.Fatal("service account missing")
}
wrote, err := f.sys.store.RefreshUserGroups(f.ctx, u.Credentials, append(u.Credentials.Groups, "refreshed-group"))
mustIAM(t, err)
if !wrote {
t.Fatal("control: the refresh did not write")
}
}, true},
{"secret-rotated", false, func(t *testing.T, cred auth.Credentials) {
_, err := f.sys.UpdateServiceAccount(f.ctx, cred.AccessKey, updateServiceAccountOpts{secretKey: "edit-rotated-secret-key"})
mustIAM(t, err)
}, false},
{"disabled", false, func(t *testing.T, cred auth.Credentials) {
_, err := f.sys.UpdateServiceAccount(f.ctx, cred.AccessKey, updateServiceAccountOpts{status: string(madmin.AccountDisabled)})
mustIAM(t, err)
}, false},
} {
t.Run(tc.name, func(t *testing.T) {
cred, sub := newService(t, tc.expiring)
before, _ := f.sys.GetUser(f.ctx, cred.AccessKey)
time.Sleep(2 * time.Millisecond) // a distinct UpdatedAt
tc.edit(t, cred)
after, _ := f.sys.GetUser(f.ctx, cred.AccessKey)
if after.UpdatedAt.Equal(before.UpdatedAt) {
t.Fatal("control: the edit did not change the identity record")
}
err := sub.authorize(f.ctx, batchContextRead(f))
if tc.keep && err != nil {
t.Errorf("an edit that keeps the credential ended the job: %v", err)
}
if !tc.keep && !isBatchJobAuthzError(err) {
t.Errorf("an edit of the credential must end the job, got %v", err)
}
})
}
}
// A user deleted and created again under its access key, with the same
// secret, is not the credential that submitted the job.
func TestBatchSubmitterRecreatedSameSecret(t *testing.T) {
f := newBatchAuthzFixture(t)
batchAuthzPolicy(f.ctx, t, f.sys, "recreate-full", batchAuthzFullAccess)
cred := iamAdminUser(f.ctx, t, f.sys, "recreate-user", "recreate-full")
sub, err := newBatchJobSubmitter(cred)
mustIAM(t, err)
mustIAM(t, sub.authorize(f.ctx, batchContextRead(f)))
mustIAM(t, f.sys.DeleteUser(f.ctx, cred.AccessKey, false))
time.Sleep(10 * time.Millisecond)
again := iamAdminUser(f.ctx, t, f.sys, cred.AccessKey, "recreate-full")
if again.SecretKey != cred.SecretKey {
t.Fatal("control: the user must be created again with the same secret")
}
err = sub.authorize(f.ctx, batchContextRead(f))
t.Logf("recreated: %v", err)
if !isBatchJobAuthzError(err) {
t.Errorf("a recreated user must not run the job of the deleted one: %v", err)
}
}