diff --git a/cmd/iam-peer-reload_test.go b/cmd/iam-peer-reload_test.go new file mode 100644 index 000000000..ca4565e94 --- /dev/null +++ b/cmd/iam-peer-reload_test.go @@ -0,0 +1,133 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-only + +package cmd + +import ( + "context" + "os" + "testing" + "time" + + "github.com/minio/madmin-go/v3" + "github.com/minio/minio/internal/auth" + "github.com/minio/minio/internal/grid" + "github.com/pgsty/silo-pkg/v3/policy" +) + +// Two independent IAM caches share the same real object backend, as sibling +// nodes do. Deliver the actual peer handler only after the source committed. +func TestIAMPeerDeleteNotificationReloadsCommittedState(t *testing.T) { + for _, name := range []string{"deleted", "recreated", "recreated_without_grant"} { + recreate := name != "deleted" + t.Run(name, func(t *testing.T) { + resetTestGlobals() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + obj, disk, err := prepareFS(ctx) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(disk) + defer obj.Shutdown(ctx) + defer resetTestGlobals() + globalObjLayerMutex.Lock() + globalObjectAPI = obj + globalObjLayerMutex.Unlock() + must := func(err error) { + t.Helper() + if err != nil { + t.Fatal(err) + } + } + source := globalIAMSys + const user = "peer-reload-user" + req := madmin.AddOrUpdateUserReq{SecretKey: "original-test-password", Status: madmin.AccountEnabled} + _, err = source.CreateUser(ctx, user, req) + must(err) + _, err = source.PolicyDBSet(ctx, user, "readwrite", regUser, false) + must(err) + _, err = source.AddUsersToGroup(ctx, "peer-reload-group", []string{user}) + must(err) + _, err = source.PolicyDBSet(ctx, "peer-reload-group", "readwrite", regUser, true) + must(err) + svc, _, err := source.NewServiceAccount(ctx, user, nil, newServiceAccountOpts{accessKey: "peer-reload-service", secretKey: "service-test-password"}) + must(err) + signingKey, err := getTokenSigningKey() + must(err) + sts, err := auth.GetNewCredentialsWithMetadata(map[string]any{"exp": UTCNow().Add(time.Hour).Unix(), parentClaim: user}, signingKey) + must(err) + sts.ParentUser = user + _, err = source.SetTempUser(ctx, sts.AccessKey, sts, "") + must(err) + + siblingStore := &IAMStoreSys{IAMStorageAPI: newIAMObjectStore(obj, MinIOUsersSysType)} + must(siblingStore.LoadIAMCache(ctx, true)) + must(siblingStore.UserNotificationHandler(ctx, sts.AccessKey, stsUser)) + for _, key := range []string{user, svc.AccessKey, sts.AccessKey} { + if _, ok := siblingStore.GetUser(key); !ok { + t.Fatalf("fixture did not load %s", key) + } + } + must(source.DeleteUser(ctx, user, false)) + if recreate { + req.SecretKey = "recreated-test-password" + _, err = source.CreateUser(ctx, user, req) + must(err) + if name == "recreated" { + _, err = source.PolicyDBSet(ctx, user, "readonly", regUser, false) + must(err) + } + } + + sibling := &IAMSys{store: siblingStore, usersSysType: MinIOUsersSysType} + globalIAMSys = sibling + defer func() { globalIAMSys = source }() + server := &peerRESTServer{} + for range 2 { + _, remoteErr := server.DeleteUserHandler(grid.NewMSSWith(map[string]string{peerRESTUser: user})) + if remoteErr != nil { + t.Fatal(remoteErr) + } + } + if recreate { + u, ok := siblingStore.GetUser(user) + if !ok || u.Credentials.SecretKey != req.SecretKey { + t.Fatal("delayed deletion notification removed the recreated user") + } + loaded := make(map[string]UserIdentity) + must(source.store.loadUser(ctx, user, regUser, loaded)) + if loaded[user].Credentials.SecretKey != req.SecretKey { + t.Fatal("notification changed the persisted recreated identity") + } + if allowed := sibling.IsAllowed(policy.Args{AccountName: user, Action: policy.GetObjectAction, BucketName: "bucket", ObjectName: "object"}); allowed != (name == "recreated") { + t.Fatal("notification did not load the recreated user's current grant") + } + } + if sibling.IsAllowed(policy.Args{AccountName: user, Action: policy.PutObjectAction, BucketName: "bucket", ObjectName: "object"}) { + t.Fatal("notification retained an old direct or group grant") + } + for _, key := range []string{svc.AccessKey, sts.AccessKey} { + if _, ok := siblingStore.GetUser(key); ok { + t.Fatalf("notification retained a revoked child: %s", key) + } + } + if !recreate { + for _, key := range []string{user} { + if _, ok := siblingStore.GetUser(key); ok { + t.Fatalf("notification retained a revoked cached identity: %s", key) + } + } + if sibling.IsAllowed(policy.Args{AccountName: user, Action: policy.GetObjectAction, BucketName: "bucket", ObjectName: "object"}) { + t.Fatal("notification retained the user's old grant") + } + cache := siblingStore.rlock() + member := cache.iamUserGroupMemberships[user].Contains("peer-reload-group") + siblingStore.runlock() + if member { + t.Fatal("notification retained the deleted user's group membership") + } + } + }) + } +} diff --git a/cmd/iam-store.go b/cmd/iam-store.go index ec992b5d0..2d017a808 100644 --- a/cmd/iam-store.go +++ b/cmd/iam-store.go @@ -1814,6 +1814,52 @@ func (store *IAMStoreSys) PolicyMappingNotificationHandler(ctx context.Context, return err } +// UserDeletionNotificationHandler refreshes all cached state affected by a +// parent deletion. Reloading only a recreated parent would retain the older +// children, policy, and group memberships in this node's cache. +func (store *IAMStoreSys) UserDeletionNotificationHandler(ctx context.Context, accessKey string) error { + if accessKey == "" { + return errInvalidArgument + } + cache := store.rlock() + groups := cache.iamUserGroupMemberships[accessKey].ToSlice() + children := make(map[string]IAMUserType) + for key, u := range cache.iamUsersMap { + if u.Credentials.ParentUser == accessKey && u.Credentials.IsServiceAccount() { + children[key] = svcUser + } + } + store.runlock() + + if err := store.UserNotificationHandler(ctx, accessKey, regUser); err != nil { + return err + } + if err := store.PolicyMappingNotificationHandler(ctx, accessKey, false, regUser); err != nil { + return err + } + // Temporary credentials are loaded on demand. Invalidate their cached + // copies; a later request must revalidate them against persisted state. + cache = store.lock() + for key, u := range cache.iamSTSAccountsMap { + if u.Credentials.ParentUser == accessKey { + delete(cache.iamSTSAccountsMap, key) + } + } + cache.updatedAt = time.Now() + store.unlock() + for key, userType := range children { + if err := store.UserNotificationHandler(ctx, key, userType); err != nil { + return err + } + } + for _, group := range groups { + if err := store.GroupNotificationHandler(ctx, group); err != nil { + return err + } + } + return nil +} + // UserNotificationHandler - handles updating a user/STS account/service account // from storage. func (store *IAMStoreSys) UserNotificationHandler(ctx context.Context, accessKey string, userType IAMUserType) error { diff --git a/cmd/iam.go b/cmd/iam.go index f27a20062..e912de903 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -162,6 +162,15 @@ func (sys *IAMSys) LoadUser(ctx context.Context, objAPI ObjectLayer, accessKey s return sys.store.UserNotificationHandler(ctx, accessKey, userType) } +// LoadUserAfterDelete reloads a parent's identity and cached dependents after a +// sibling committed a deletion. Each record may already have been recreated. +func (sys *IAMSys) LoadUserAfterDelete(ctx context.Context, accessKey string) error { + if !sys.Initialized() { + return errServerNotInitialized + } + return sys.store.UserDeletionNotificationHandler(ctx, accessKey) +} + // LoadServiceAccount - reloads a specific service account from backend disks or etcd. func (sys *IAMSys) LoadServiceAccount(ctx context.Context, accessKey string) error { if !sys.Initialized() { diff --git a/cmd/peer-rest-server.go b/cmd/peer-rest-server.go index e17bd4b2e..0ef0eb0c0 100644 --- a/cmd/peer-rest-server.go +++ b/cmd/peer-rest-server.go @@ -230,7 +230,8 @@ func (s *peerRESTServer) LoadServiceAccountHandler(mss *grid.MSS) (np grid.NoPay return np, nerr } -// DeleteUserHandler - deletes a user on the server. +// DeleteUserHandler reloads the state committed by another node. A delayed +// notification must not delete an identity recreated since that commit. func (s *peerRESTServer) DeleteUserHandler(mss *grid.MSS) (np grid.NoPayload, nerr *grid.RemoteErr) { objAPI := newObjectLayerFn() if objAPI == nil { @@ -242,7 +243,9 @@ func (s *peerRESTServer) DeleteUserHandler(mss *grid.MSS) (np grid.NoPayload, ne return np, grid.NewRemoteErr(errors.New("username is missing")) } - if err := globalIAMSys.DeleteUser(context.Background(), accessKey, false); err != nil { + ctx, cancel := context.WithTimeout(GlobalContext, defaultContextTimeout) + defer cancel() + if err := globalIAMSys.LoadUserAfterDelete(ctx, accessKey); err != nil { return np, grid.NewRemoteErr(err) }