Merge pull request #175 from pgsty/codex/upstream-sdk-password-20260910

fix(iam)!: split self-service password permissions and update SDK stack
This commit is contained in:
Feng Ruohang
2026-09-10 17:51:08 +08:00
committed by GitHub
6 changed files with 325 additions and 13 deletions
+5 -1
View File
@@ -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,
+102
View File
@@ -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",
+64
View File
@@ -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)
}
},
})
}
+142
View File
@@ -0,0 +1,142 @@
# Password and user-management permissions
**Breaking change: the password-permission split changes the meaning of
existing IAM policies.** The same stored policy can authorize a request after
this update that it denied before, or deny a request it previously authorized.
This is a deliberate authorization change from adopting
[minio/pkg #262](https://github.com/minio/pkg/pull/262), independent of the
minio-go SDK update. It must be called out as a breaking change in the release
that first includes it; it is not a transparent dependency refresh.
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.
## What changes and why
Previously, both operations checked `admin:CreateUser`. Changing one's own
password used an implicit grant unless that action was explicitly denied;
managing other users required an explicit Allow. The split retains these two
evaluation rules but checks `admin:ChangeMyPassword` for the caller's password.
It lets an operator independently control password changes and user
administration. This is a policy-design choice, not a required mitigation for
the SDK signing or region-compatibility fixes.
The following cases assume an internal user with an attached policy, matching
statement conditions, and no other applicable grants or denies:
| Existing policy | Own password before | Own password after | Create/reset another user, before and after |
| --- | --- | --- | --- |
| S3 read grant only | Allowed | Allowed | Denied |
| `Deny admin:CreateUser` | Denied | **Allowed** | Denied |
| `Deny admin:ChangeMyPassword` | Allowed | **Denied** | Denied |
| `Allow admin:CreateUser` | Allowed | Allowed | Allowed |
| `Allow admin:CreateUser` plus `Deny admin:ChangeMyPassword` | Allowed | **Denied** | Allowed |
| Deny both actions, or `Deny admin:*` | Denied | Denied | Denied |
Wildcard denies that match `admin:CreateUser` but do not match
`admin:ChangeMyPassword`, such as `admin:Create*`, have the same password
compatibility change as the explicit CreateUser deny. An Allow never overrides
a matching Deny. Granting only `admin:ChangeMyPassword` does not grant user
administration.
The policy JSON format, stored documents and endpoint are retained, but that
does not preserve their authorization semantics. This update does not rewrite
saved policies or provide a switch that restores the old action mapping.
## Preserve the behavior of 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 `admin:ChangeMyPassword` to the **same Deny statement before upgrading**.
For example, change that statement's action list to:
```json
{
"Effect": "Deny",
"Action": ["admin:CreateUser", "admin:ChangeMyPassword"]
}
```
This is a statement fragment, not a replacement for the entire policy.
Preserve its other actions, resource scope and conditions, and all other
statements. Check policies attached through groups as well as directly to users.
The preceding SILO package version already recognizes both action names, so
this dual deny can be prepared before the Server upgrade. Saved policy files
are not migrated automatically; the operator must apply this change where the
old password restriction is intended.
To adopt the new split and lock only the caller's password while allowing
separately granted user administration, deny only `admin:ChangeMyPassword`.
That finer distinction is enforced only by Servers containing this change.
## Built-in read-only policies
The built-in `readonly` policy now grants its original S3 read operations
without its previous CreateUser deny. This has two compatibility effects:
- A user of the old `readonly` policy could not change their own password; with
the split they can, unless another applicable statement denies
`admin:ChangeMyPassword`.
- The old built-in `readonly` deny overrode a separate CreateUser Allow. The
new built-in definition allows that independently granted user administration.
This can broaden effective permissions for users with both policies attached.
The added `consolereadonly` policy also grants ListBucket for Console browsing
and follows the new split. Neither read-only policy grants user administration
or S3 writes on its own. Their S3 permissions do not implicitly lock passwords.
Saved policies and user overrides of canned policies are preserved on upgrade.
A saved copy of the old read-only policy retains its CreateUser deny and still
blocks a separate CreateUser Allow, even though it no longer blocks self-service
password changes. Where no saved override exists, Server uses the updated
built-in definition. Review the effective policy documents rather than assuming
every policy named `readonly` has the same contents. To retain both old
restrictions, attach a policy denying both actions or retain the saved readonly
override and add ChangeMyPassword to its existing deny.
## Console and package callers
`silo-pkg`'s `Policy.IsAllowedActions` now reports `admin:ChangeMyPassword` as
implicit unless denied, and reports `admin:CreateUser` only when explicitly
allowed. Its Go signature and the Go compatibility floor are unchanged, but
its returned capabilities change. Console and other consumers must stop using
the CreateUser capability as a proxy for permission to change one's own password.
## Coordinated upgrade and rollback
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.
Before upgrading, export the affected user/group policy documents, review the
cases above, and apply both denies wherever the old combined restriction must
survive. Keep both denies throughout a rolling upgrade and any rollback window.
Verify self-service password changes and other-user creation/password resets
with the affected accounts. Complete the Server rollout and update Console
before relying on the new independent permissions.
Rolling back the binaries does not convert policies. An old Server ignores
`Deny admin:ChangeMyPassword` for this endpoint, so that deny alone cannot lock
the password after rollback. Restore or retain the CreateUser deny when the
password must remain locked; on the old Server it will also deny management of
other users. The old Server cannot represent the new combination of allowing
user administration while denying only self-service password changes.
Upstream MinIO compatibility remains best effort; the supported integration
target is `pgsty/silo`.
+4 -4
View File
@@ -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-20260910093545-6a0b31b5ade2
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-20260910093317-e6a60edf0952
// 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.20260910091716-2d8fd3cbbf07
github.com/philhofer/fwd v1.2.0
github.com/pierrec/lz4/v4 v4.1.29
github.com/pkg/errors v0.9.1
+8 -8
View File
@@ -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-20260910093317-e6a60edf0952 h1:b6jpBuZWUiZhBox64eF1cmNL4HWGwdSk/LVXNc3DBQ0=
github.com/pgsty/mc v0.0.0-20260910093317-e6a60edf0952/go.mod h1:VsGNEmditljwBgmYKy7X+//dXLN8gEfSPR2m4lcMPmo=
github.com/pgsty/silo-console v0.0.0-20260910093545-6a0b31b5ade2 h1:v/AXKa/UZkDheLLGrpKmyu8bVHo11dVrDHdbDvjX1/M=
github.com/pgsty/silo-console v0.0.0-20260910093545-6a0b31b5ade2/go.mod h1:bCyOnZactQajLOqlgCn0lHwQWMNylvP6MUOqqLako00=
github.com/pgsty/silo-pkg/v3 v3.13.4-0.20260910091716-2d8fd3cbbf07 h1:IKm2AyPsvuL4NyniK3ixqqQz1CIargmhyCkL2eZvlhA=
github.com/pgsty/silo-pkg/v3 v3.13.4-0.20260910091716-2d8fd3cbbf07/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=