diff --git a/cmd/admin-handlers-users.go b/cmd/admin-handlers-users.go index 779da5dc5..77eb24f52 100644 --- a/cmd/admin-handlers-users.go +++ b/cmd/admin-handlers-users.go @@ -502,11 +502,15 @@ func (a adminAPIHandlers) AddUser(w http.ResponseWriter, r *http.Request) { } checkDenyOnly := accessKey == cred.AccessKey + action := policy.Action(policy.CreateUserAdminAction) + if checkDenyOnly { + action = policy.ChangeMyPasswordAdminAction + } if !globalIAMSys.IsAllowed(policy.Args{ AccountName: cred.AccessKey, Groups: cred.Groups, - Action: policy.CreateUserAdminAction, + Action: action, ConditionValues: getConditionValues(r, "", cred), IsOwner: owner, Claims: cred.Claims, diff --git a/cmd/admin-handlers-users_test.go b/cmd/admin-handlers-users_test.go index 0759822d7..7b400a75b 100644 --- a/cmd/admin-handlers-users_test.go +++ b/cmd/admin-handlers-users_test.go @@ -243,6 +243,7 @@ func TestIAMInternalIDPServerSuite(t *testing.T) { suite.SetUpSuite(c) suite.TestUserCreate(c) + suite.TestUserPasswordActionAuthorization(c) suite.TestUserStatusActionAuthorization(c) suite.TestGroupStatusActionAuthorization(c) suite.TestUserPolicyEscalationBug(c) @@ -356,6 +357,106 @@ func (s *TestSuiteIAM) TestUserCreate(c *check) { } } +func (s *TestSuiteIAM) TestUserPasswordActionAuthorization(c *check) { + for _, tt := range []struct { + name string + statements string + self bool + other bool + }{ + {"readonly", "", true, false}, + {"consolereadonly", "", true, false}, + {"password grant", `{"Effect":"Allow","Action":"admin:ChangeMyPassword"}`, true, false}, + {"legacy CreateUser deny", `{"Effect":"Deny","Action":"admin:CreateUser","Resource":"arn:aws:s3:::*"}`, true, false}, + {"password deny", `{"Effect":"Deny","Action":"admin:ChangeMyPassword"}`, false, false}, + {"user admin", `{"Effect":"Allow","Action":"admin:CreateUser"}`, true, true}, + {"user admin with password deny", `{"Effect":"Allow","Action":"admin:CreateUser"},{"Effect":"Deny","Action":"admin:ChangeMyPassword"}`, false, true}, + {"password deny overrides grant", `{"Effect":"Allow","Action":"admin:ChangeMyPassword"},{"Effect":"Deny","Action":"admin:ChangeMyPassword"}`, false, false}, + {"wildcard deny", `{"Effect":"Deny","Action":"admin:*"}`, false, false}, + } { + c.Run(tt.name, func(t *testing.T) { + c := &check{t, s.serverType} + ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout) + defer cancel() + var users []string + policyName := tt.name + defer func() { + for _, user := range users { + if err := s.adm.RemoveUser(ctx, user); err != nil { + c.Errorf("remove test user: %v", err) + } + } + if tt.statements != "" { + if err := s.adm.RemoveCannedPolicy(ctx, policyName); err != nil { + c.Errorf("remove test policy: %v", err) + } + } + }() + createUser := func() (string, string) { + accessKey, secretKey := mustGenerateCredentials(c) + if err := s.adm.SetUser(ctx, accessKey, secretKey, madmin.AccountEnabled); err != nil { + c.Fatalf("create test user: %v", err) + } + users = append(users, accessKey) + return accessKey, secretKey + } + client := func(accessKey, secretKey string) *madmin.AdminClient { + adm, err := madmin.New(s.endpoint, accessKey, secretKey, s.secure) + if err != nil { + c.Fatal(err) + } + adm.SetCustomTransport(s.TestSuiteCommon.client.Transport) + return adm + } + if tt.statements != "" { + policyName = getRandomBucketName() + doc := []byte(`{"Version":"2012-10-17","Statement":[` + tt.statements + `]}`) + if err := s.adm.AddCannedPolicy(ctx, policyName, doc); err != nil { + c.Fatalf("save test policy: %v", err) + } + } + accessKey, secretKey := createUser() + if _, err := s.adm.AttachPolicy(ctx, madmin.PolicyAssociationReq{ + User: accessKey, Policies: []string{policyName}, + }); err != nil { + c.Fatalf("attach test policy: %v", err) + } + adm := client(accessKey, secretKey) + _, newSecretKey := mustGenerateCredentials(c) + err := adm.SetUser(ctx, accessKey, newSecretKey, madmin.AccountEnabled) + if tt.self { + if err != nil { + c.Fatalf("change own password: %v", err) + } + if _, err = adm.AccountInfo(ctx, madmin.AccountOpts{}); err == nil { + c.Fatal("old password still authenticates") + } + adm = client(accessKey, newSecretKey) + } else if err == nil || madmin.ToErrorResponse(err).Code != "AccessDenied" { + c.Fatalf("self password change: expected AccessDenied, got %v", err) + } + if _, err := adm.AccountInfo(ctx, madmin.AccountOpts{}); err != nil { + c.Fatalf("current password no longer authenticates: %v", err) + } + target, _ := createUser() + newUser, newUserSecret := mustGenerateCredentials(c) + for _, key := range []string{target, newUser} { + err := adm.SetUser(ctx, key, newUserSecret, madmin.AccountEnabled) + if tt.other { + if err != nil { + c.Fatalf("create or update another user: %v", err) + } + if key == newUser { + users = append(users, newUser) + } + } else if err == nil || madmin.ToErrorResponse(err).Code != "AccessDenied" { + c.Fatalf("create or update another user: expected AccessDenied, got %v", err) + } + } + }) + } +} + func (s *TestSuiteIAM) TestUserStatusActionAuthorization(c *check) { ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout) defer cancel() @@ -946,6 +1047,7 @@ func (s *TestSuiteIAM) TestCannedPolicies(c *check) { defaultPolicies := []string{ "readwrite", "readonly", + "consolereadonly", "writeonly", "diagnostics", "consoleAdmin", diff --git a/cmd/sdk-streaming-signature_test.go b/cmd/sdk-streaming-signature_test.go new file mode 100644 index 000000000..1edd0862d --- /dev/null +++ b/cmd/sdk-streaming-signature_test.go @@ -0,0 +1,64 @@ +// Copyright (c) 2026 Feng Ruohang +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cmd + +import ( + "bytes" + "crypto/sha256" + "hash" + "net/http" + "net/http/httptest" + "slices" + "strings" + "testing" + + "github.com/minio/minio-go/v7/pkg/signer" + "github.com/minio/minio/internal/auth" +) + +type sdkStreamingHasher struct{ hash.Hash } + +func (sdkStreamingHasher) Close() {} + +func TestAPIUpstreamSDKStreamingContentType(t *testing.T) { + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ + t: t, + objAPITest: func(obj ObjectLayer, instanceType, bucketName string, router http.Handler, cred auth.Credentials, t *testing.T) { + payload := bytes.Repeat([]byte("sdk-streaming-"), 8192) + for _, tamper := range []bool{false, true} { + req, err := http.NewRequest(http.MethodPut, getPutObjectURL("http://localhost", bucketName, "sdk-streaming"), bytes.NewReader(payload)) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Content-Type", "application/x-silo-test") + hasher := sdkStreamingHasher{sha256.New()} + req = signer.StreamingSignV4(req, cred.AccessKey, cred.SecretKey, "", globalSite.Region(), int64(len(payload)), UTCNow(), hasher) + _, signedHeaders, _ := strings.Cut(req.Header.Get("Authorization"), "SignedHeaders=") + signedHeaders, _, _ = strings.Cut(signedHeaders, ",") + if !slices.Contains(strings.Split(signedHeaders, ";"), "content-type") { + t.Fatal("upstream SDK did not sign Content-Type") + } + if tamper { + req.Header.Set("Content-Type", "application/x-tampered") + } + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + want := http.StatusOK + if tamper { + want = http.StatusForbidden + } + if rec.Code != want { + t.Fatalf("%s tamper=%v: status %d, want %d: %s", instanceType, tamper, rec.Code, want, rec.Body.String()) + } + } + info, err := obj.GetObjectInfo(t.Context(), bucketName, "sdk-streaming", ObjectOptions{}) + if err != nil { + t.Fatal(err) + } + if info.Size != int64(len(payload)) || info.ContentType != "application/x-silo-test" { + t.Fatalf("stored size/type = %d/%q", info.Size, info.ContentType) + } + }, + }) +} diff --git a/docs/iam/password-permissions.md b/docs/iam/password-permissions.md new file mode 100644 index 000000000..c0336e527 --- /dev/null +++ b/docs/iam/password-permissions.md @@ -0,0 +1,67 @@ +# Password and user-management permissions + +SILO separates a user's own password change from creating users or resetting +another user's password. The same `add-user` administration endpoint and mcli +commands continue to work; the authenticated caller and target access key +determine which permission is checked. + +| Request | Permission | Evaluation | +| --- | --- | --- | +| Change the caller's own password | `admin:ChangeMyPassword` | Allowed for an internal user with an attached policy unless explicitly denied. | +| Create another user or reset another user's password | `admin:CreateUser` | Requires an explicit Allow; an explicit Deny wins. | + +The Console's Change Password button uses `admin:ChangeMyPassword`. +`admin:CreateUser` continues to control user administration. The password change +still requires the current password. STS and service-account credentials cannot +change their parent user's password; root credentials and external identity +provider passwords remain outside this endpoint. + +## Existing policies + +A saved `Deny admin:CreateUser` still prevents user creation and password resets +for other users. It no longer prevents the caller from changing their own +password. If an existing policy used that deny to lock the caller's password, +add the new action to the same Deny statement before upgrading: + +```json +{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Deny", + "Action": ["admin:CreateUser", "admin:ChangeMyPassword"] + } + ] +} +``` + +Preserve any existing conditions on that statement. To lock only the caller's +password while allowing separately granted user administration, deny only +`admin:ChangeMyPassword`. An existing `Deny admin:*` denies both actions. +An Allow cannot override a matching Deny. + +The built-in `readonly` policy now grants its original S3 read operations +without a CreateUser deny. The added `consolereadonly` policy also grants +ListBucket for Console browsing. Neither grants user administration or S3 +writes. Both permit self-service password changes unless another policy denies +them. Combining either read-only policy with an explicit CreateUser Allow is +supported. + +Saved policies and user overrides of canned policies are preserved on upgrade. +A saved copy of the old read-only policy therefore retains its CreateUser deny: +it still blocks a separate CreateUser Allow, even though self-service password +changes now use the new action. Review that deny explicitly if combining old +read-only policies with user-administration grants. + +## Coordinated upgrade + +Upgrade SILO Server, silo-pkg and Console together, including the Console +embedded in Server. Update mcli's shared package and SDK pins as part of the +same maintained stack. Mixed versions disagree about the self-service action +and may show a button the Server refuses, hide an allowed operation, or fail to +enforce a new password-specific deny on an old Server. Complete a rolling +Server upgrade before relying on the new permission split. + +This adopts [minio/pkg #262](https://github.com/minio/pkg/pull/262). Upstream +MinIO compatibility remains best effort; the supported integration target is +`pgsty/silo`. diff --git a/go.mod b/go.mod index eb82832f7..402c645dd 100644 --- a/go.mod +++ b/go.mod @@ -4,9 +4,9 @@ go 1.27.1 // Console and MC retain their historical module paths for best-effort upstream // compatibility. Pin the maintained PGSTY implementations used by SILO. -replace github.com/minio/console => github.com/pgsty/silo-console v0.0.0-20260908142700-c103d08ec36a +replace github.com/minio/console => github.com/pgsty/silo-console v0.0.0-20260910071402-1b95b6cec652 -replace github.com/minio/mc => github.com/pgsty/mc v0.0.0-20260909015522-fcd5cad8247f +replace github.com/minio/mc => github.com/pgsty/mc v0.0.0-20260910070158-fa22b40b4eb7 // v22.7.0 does not compile on NetBSD because its unix implementation uses // CLOCK_MONOTONIC, which is unavailable there. Keep the last portable release @@ -68,7 +68,7 @@ require ( github.com/minio/kms-go/kes v0.3.1 github.com/minio/kms-go/kms v0.6.0 github.com/minio/madmin-go/v3 v3.0.110 - github.com/minio/minio-go/v7 v7.3.1-0.20260828014306-0e78d3f18efe + github.com/minio/minio-go/v7 v7.3.1-0.20260909183557-78bfa91607c2 github.com/minio/mux v1.10.1 github.com/minio/selfupdate v0.6.0 github.com/minio/simdjson-go v0.4.5 @@ -81,7 +81,7 @@ require ( github.com/nats-io/stan.go v0.10.4 github.com/ncw/directio v1.0.5 github.com/nsqio/go-nsq v1.1.0 - github.com/pgsty/silo-pkg/v3 v3.13.3 + github.com/pgsty/silo-pkg/v3 v3.13.4-0.20260910065859-b3760f56ec23 github.com/philhofer/fwd v1.2.0 github.com/pierrec/lz4/v4 v4.1.29 github.com/pkg/errors v0.9.1 diff --git a/go.sum b/go.sum index 7d50bb93c..eac952fed 100644 --- a/go.sum +++ b/go.sum @@ -474,8 +474,8 @@ github.com/minio/madmin-go/v3 v3.0.110 h1:FIYekj7YPc430ffpXFWiUtyut3qBt/unIAcDzJ github.com/minio/madmin-go/v3 v3.0.110/go.mod h1:WOe2kYmYl1OIlY2DSRHVQ8j1v4OItARQ6jGyQqcCud8= github.com/minio/md5-simd v1.1.2 h1:Gdi1DZK69+ZVMoNHRXJyNcxrMA4dSxoYHZSQbirFg34= github.com/minio/md5-simd v1.1.2/go.mod h1:MzdKDxYpY2BT9XQFocsiZf/NKVtR7nkE4RoEpN+20RM= -github.com/minio/minio-go/v7 v7.3.1-0.20260828014306-0e78d3f18efe h1:By2FKNSOUGLOeb0x4D7xJMHr8x/X1ZW8PG780SpKUwQ= -github.com/minio/minio-go/v7 v7.3.1-0.20260828014306-0e78d3f18efe/go.mod h1:KUPWdecEO1LWyUz+sTGXAuf2jZHrPh5fCsRH86QbPfk= +github.com/minio/minio-go/v7 v7.3.1-0.20260909183557-78bfa91607c2 h1:nvX7IksPFOF/cBvkSg/Z+urBZkmhZrfUVmzdNFLjZ5Y= +github.com/minio/minio-go/v7 v7.3.1-0.20260909183557-78bfa91607c2/go.mod h1:KUPWdecEO1LWyUz+sTGXAuf2jZHrPh5fCsRH86QbPfk= github.com/minio/mux v1.10.1 h1:grrK8SwRKbkNFE6qG7WAvFGH09bB46d5teOOtKfQ14s= github.com/minio/mux v1.10.1/go.mod h1:INYT4sMSTJy0QWUEA/E2DZNxJ5sAxIwbnyZjkzNFRfE= github.com/minio/pkg/v3 v3.6.1 h1:gaNT80BS/iuIany5ylTkVmfN4s6UYY30OtImFv4GQA8= @@ -545,12 +545,12 @@ github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde/go.mod h1:nZgzb github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= -github.com/pgsty/mc v0.0.0-20260909015522-fcd5cad8247f h1:JiL/FcsGMsAhA+Iv+0Jzk9VnEAVVv4HUNbR0cGf+/CE= -github.com/pgsty/mc v0.0.0-20260909015522-fcd5cad8247f/go.mod h1:VHif+uy3s+nOcQKy808NlNpYawINOeGSyisgjoSBNVs= -github.com/pgsty/silo-console v0.0.0-20260908142700-c103d08ec36a h1:aHLqQ7INozqGLEOB1tr+n/eKgrBhlQ20fHyLmeNu+ao= -github.com/pgsty/silo-console v0.0.0-20260908142700-c103d08ec36a/go.mod h1:d16nCLu7flJ6Fv0hPTW+JsyjhgyqZN87//URYO0nETU= -github.com/pgsty/silo-pkg/v3 v3.13.3 h1:d2xYTn4LXoWIAIjBlW/17wtA/Ut1ap29t+1ww4TFa8o= -github.com/pgsty/silo-pkg/v3 v3.13.3/go.mod h1:0GmaDA0ArQ8bkAI/obiSNTBQzdgBu6W0e0olDCbyXXo= +github.com/pgsty/mc v0.0.0-20260910070158-fa22b40b4eb7 h1:Mo624rA8j/5ulJbLOF39vOLw0ZmrTW3g+zbGULY48SE= +github.com/pgsty/mc v0.0.0-20260910070158-fa22b40b4eb7/go.mod h1:WKsTzLhaAeAHKib3NKxfgWmy/B9MaVIoz8g5GxA+7EY= +github.com/pgsty/silo-console v0.0.0-20260910071402-1b95b6cec652 h1:6/7Yoix9Zfg8ffF9CaKdrCZE9TixYLHpiMLdJcYhCGw= +github.com/pgsty/silo-console v0.0.0-20260910071402-1b95b6cec652/go.mod h1:ogDu8XGYwzE0LfhbguQljY6A+CNEmkcSSkFH3AfP+GY= +github.com/pgsty/silo-pkg/v3 v3.13.4-0.20260910065859-b3760f56ec23 h1:/9tqYXyTU7OkZCYa0NxsPTUQEM5ehy5GoIK6ivQlmgY= +github.com/pgsty/silo-pkg/v3 v3.13.4-0.20260910065859-b3760f56ec23/go.mod h1:1JdcUcM+TRXObb9QrC8FWhav0RKk6/u8RT2RgimfxRQ= github.com/philhofer/fwd v1.2.0 h1:e6DnBTl7vGY+Gz322/ASL4Gyp1FspeMvx1RNDoToZuM= github.com/philhofer/fwd v1.2.0/go.mod h1:RqIHx9QI14HlwKwm98g9Re5prTQ6LdeRQn+gXJFxsJM= github.com/pierrec/lz4/v4 v4.1.29 h1:CDQY6qZOLI4DW0Nx6R1vRrifrCeQHnNXkMb0hZWXFjg=