fix: authorize user status changes by target status

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=<access_key>&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)

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()