From 58735ee3829e36e24735587e2212b97c4149e0d1 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Wed, 26 Aug 2026 14:18:50 +0800 Subject: [PATCH] fix: authorize user status changes by target status Signed-off-by: Feng Ruohang --- cmd/admin-handlers-users.go | 17 ++++-- cmd/admin-handlers-users_test.go | 100 +++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 5 deletions(-) diff --git a/cmd/admin-handlers-users.go b/cmd/admin-handlers-users.go index eb032672d4eee..c9ac0f4d88bef 100644 --- a/cmd/admin-handlers-users.go +++ b/cmd/admin-handlers-users.go @@ -398,18 +398,25 @@ func (a adminAPIHandlers) SetGroupStatus(w http.ResponseWriter, r *http.Request) } // SetUserStatus - PUT /minio/admin/v3/set-user-status?accessKey=&status=[enabled|disabled] +func setUserStatusAdminAction(status string) policy.AdminAction { + if madmin.AccountStatus(status) == madmin.AccountDisabled { + return policy.DisableUserAdminAction + } + return policy.EnableUserAdminAction +} + func (a adminAPIHandlers) SetUserStatus(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - objectAPI, creds := validateAdminReq(ctx, w, r, policy.EnableUserAdminAction) - if objectAPI == nil { - return - } - vars := mux.Vars(r) accessKey := vars["accessKey"] status := vars["status"] + objectAPI, creds := validateAdminReq(ctx, w, r, setUserStatusAdminAction(status)) + if objectAPI == nil { + return + } + // you cannot enable or disable yourself. if accessKey == creds.AccessKey { writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, errInvalidArgument), r.URL) diff --git a/cmd/admin-handlers-users_test.go b/cmd/admin-handlers-users_test.go index 350edf82381e2..74ad6b8a33e6c 100644 --- a/cmd/admin-handlers-users_test.go +++ b/cmd/admin-handlers-users_test.go @@ -41,12 +41,33 @@ import ( "github.com/minio/minio-go/v7/pkg/signer" "github.com/minio/minio/internal/auth" "github.com/minio/pkg/v3/env" + "github.com/minio/pkg/v3/policy" ) const ( testDefaultTimeout = 30 * time.Second ) +func TestSetUserStatusAdminAction(t *testing.T) { + tests := []struct { + name string + status string + want policy.AdminAction + }{ + {name: "enable", status: string(madmin.AccountEnabled), want: policy.EnableUserAdminAction}, + {name: "disable", status: string(madmin.AccountDisabled), want: policy.DisableUserAdminAction}, + {name: "invalid preserves authenticated default", status: "invalid", want: policy.EnableUserAdminAction}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := setUserStatusAdminAction(tt.status); got != tt.want { + t.Fatalf("setUserStatusAdminAction(%q) = %q, want %q", tt.status, got, tt.want) + } + }) + } +} + // API suite container for IAM type TestSuiteIAM struct { TestSuiteCommon @@ -202,6 +223,7 @@ func TestIAMInternalIDPServerSuite(t *testing.T) { suite.SetUpSuite(c) suite.TestUserCreate(c) + suite.TestUserStatusActionAuthorization(c) suite.TestUserPolicyEscalationBug(c) suite.TestPolicyCreate(c) suite.TestServiceAccountBareARNPolicyRejected(c) @@ -313,6 +335,84 @@ func (s *TestSuiteIAM) TestUserCreate(c *check) { } } +func (s *TestSuiteIAM) TestUserStatusActionAuthorization(c *check) { + ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout) + defer cancel() + + var createdUsers []string + var createdPolicies []string + defer func() { + for _, user := range createdUsers { + if err := s.adm.RemoveUser(ctx, user); err != nil { + c.Errorf("unable to remove test user %s: %v", user, err) + } + } + for _, policyName := range createdPolicies { + if err := s.adm.RemoveCannedPolicy(ctx, policyName); err != nil { + c.Errorf("unable to remove test policy %s: %v", policyName, err) + } + } + }() + + createUser := func() (string, string) { + accessKey, secretKey := mustGenerateCredentials(c) + if err := s.adm.SetUser(ctx, accessKey, secretKey, madmin.AccountEnabled); err != nil { + c.Fatalf("unable to create test user: %v", err) + } + createdUsers = append(createdUsers, accessKey) + return accessKey, secretKey + } + + createStatusClient := func(action policy.AdminAction) *madmin.AdminClient { + accessKey, secretKey := createUser() + policyName := getRandomBucketName() + policyBytes := fmt.Appendf(nil, `{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Action": ["%s"] + }] +}`, action) + if err := s.adm.AddCannedPolicy(ctx, policyName, policyBytes); err != nil { + c.Fatalf("unable to add status policy: %v", err) + } + createdPolicies = append(createdPolicies, policyName) + if _, err := s.adm.AttachPolicy(ctx, madmin.PolicyAssociationReq{ + Policies: []string{policyName}, + User: accessKey, + }); err != nil { + c.Fatalf("unable to attach status policy: %v", err) + } + + client, err := madmin.NewWithOptions(s.endpoint, &madmin.Options{ + Creds: credentials.NewStaticV4(accessKey, secretKey, ""), + Secure: s.secure, + }) + if err != nil { + c.Fatalf("unable to create status admin client: %v", err) + } + client.SetCustomTransport(s.TestSuiteCommon.client.Transport) + return client + } + + targetAccessKey, _ := createUser() + disableClient := createStatusClient(policy.DisableUserAdminAction) + if err := disableClient.SetUserStatus(ctx, targetAccessKey, madmin.AccountDisabled); err != nil { + c.Fatalf("DisableUser-only client could not disable a user: %v", err) + } + if err := disableClient.SetUserStatus(ctx, targetAccessKey, madmin.AccountEnabled); err == nil || madmin.ToErrorResponse(err).Code != "AccessDenied" { + c.Fatalf("DisableUser-only client unexpectedly enabled a user: %v", err) + } + + enableClient := createStatusClient(policy.EnableUserAdminAction) + if err := enableClient.SetUserStatus(ctx, targetAccessKey, madmin.AccountEnabled); err != nil { + c.Fatalf("EnableUser-only client could not enable a user: %v", err) + } + if err := enableClient.SetUserStatus(ctx, targetAccessKey, madmin.AccountDisabled); err == nil || madmin.ToErrorResponse(err).Code != "AccessDenied" { + c.Fatalf("EnableUser-only client unexpectedly disabled a user: %v", err) + } +} + func (s *TestSuiteIAM) TestUserPolicyEscalationBug(c *check) { ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout) defer cancel()