Skip to content

Commit 2d8fd3c

Browse files
authored
Merge pull request #6 from pgsty/codex/upstream-sdk-password-20260910
fix(policy)!: separate password self service and update upstream SDK
2 parents b60f756 + abf03e6 commit 2d8fd3c

7 files changed

Lines changed: 196 additions & 14 deletions

File tree

UPSTREAM.md

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
# Upstream review: 2026-09-10
2+
3+
Reviewed `minio/pkg` through
4+
[`657d87e88168`](https://github.com/minio/pkg/commit/657d87e881680db68e3a9f9286569788651c968e)
5+
and `minio/minio-go` through
6+
[`78bfa91607c2`](https://github.com/minio/minio-go/commit/78bfa91607c2b9a7eb1ad5fe739de3c6c238d5ab).
7+
8+
## Adopted
9+
10+
- [pkg #262](https://github.com/minio/pkg/pull/262): report
11+
`admin:ChangeMyPassword` as an implicit self-service capability, subject to
12+
explicit Deny. `admin:CreateUser` requires an explicit Allow. Remove the
13+
legacy CreateUser deny from the built-in read-only policy.
14+
- [pkg #233](https://github.com/minio/pkg/pull/233): add `consolereadonly`,
15+
with GetObject, GetBucketLocation and ListBucket. Apply #262 to this new
16+
policy too. The original `readonly` S3 permissions remain unchanged.
17+
- Pin the upstream SDK to `v7.3.1-0.20260909183557-78bfa91607c2`. Relative to
18+
the previous `0e78d3f18efe` pin, it contains configurable upload limits
19+
([#2299](https://github.com/minio/minio-go/pull/2299)), Content-Type in
20+
streaming SignedHeaders ([#2301](https://github.com/minio/minio-go/pull/2301))
21+
and caller TLS trust on RDMA ([#2302](https://github.com/minio/minio-go/pull/2302)).
22+
23+
## Breaking authorization compatibility
24+
25+
Adopting #262 changes existing policy semantics; it is independent of updating
26+
the minio-go SDK. This must be disclosed as a breaking change in the release
27+
that includes it. The Go signatures and Go compatibility floor are unchanged,
28+
but the public `Policy.IsAllowedActions` method returns different capabilities:
29+
ChangeMyPassword is implicit unless denied, and CreateUser requires an explicit
30+
Allow. Consumers must use the matching capability for each operation.
31+
32+
With the matching Server change, a saved `Deny admin:CreateUser` no longer
33+
prevents the caller from changing their own password. A saved
34+
`Deny admin:ChangeMyPassword` now prevents it. To preserve a policy's old
35+
combined restriction, add ChangeMyPassword to the same CreateUser Deny statement
36+
before upgrading, preserving its other actions, scope and conditions. Saved
37+
policy documents are not rewritten automatically.
38+
39+
The built-in `readonly` policy also drops its old CreateUser deny. It now allows
40+
self-service password changes, and a separate CreateUser Allow can grant user
41+
administration where the old built-in deny overrode it. Saved overrides retain
42+
their existing deny; inspect the effective policy contents. `consolereadonly`
43+
is new and follows the split. Neither read-only policy grants user
44+
administration on its own.
45+
46+
Deploy password authorization with the matching SILO Server and Console.
47+
During a mixed-version rollout or rollback, retain both denies if the old
48+
combined restriction must hold: an old Server does not enforce a password-only
49+
deny for this endpoint. See the Server's
50+
[password-permission migration guide](https://github.com/pgsty/silo/blob/420340bc142b7dec00c26c28dd78102e3ed9d0f3/docs/iam/password-permissions.md)
51+
for the before/after matrix, policy migration, read-only composition and
52+
rollback limits.
53+
54+
## Already covered or deferred
55+
56+
| Upstream work | SILO decision |
57+
| --- | --- |
58+
| #265, x/crypto v0.56.0 | Already selected; no additional version bump. |
59+
| #230, RNG subkey initialization | Already fixed locally. |
60+
| #226, exact condition key lookup | Already fixed locally. |
61+
| #242, xtime.Duration JSON marshaling | Already implemented locally. |
62+
| #261, NotResource deduplication, literal-policy Deny detection and wildcard backtracking | Correctness fixes already present in SILO v3.13.3, including condition-value boundary tests; retain them. The remaining indexing and classification refactor is a separate optimization. |
63+
| #249–252, ARN and Memory resource changes | Keep SILO's strict administration writes and compatible historical-policy parsing; do not replace them with the AIStor Memory model. |
64+
| #245–246, typed action API | Breaking source changes without a current SILO requirement; defer. |
65+
| Memory, S3Tables, annotation, compression and new closed-product admin actions | No matching maintained SILO endpoints in this update; defer. |
66+
| #263, bounded deduplicating channel | New helper with no current SILO consumer; defer. |
67+
| Certificate-cache accessors, bandwidth configuration and RNG assembly | Independent features or optimizations; defer until a consumer or measured need justifies them. |
68+
69+
[minio-go #2274](https://github.com/minio/minio-go/pull/2274), trimming bucket
70+
location whitespace, is still open at this review. It is absent from the pinned
71+
commit. Take it through a later upstream commit after merge; do not revive the
72+
retired silo-go fork for this other-S3 compatibility issue. It does not block a
73+
SILO release.

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ require (
2222
github.com/lestrrat-go/jwx/v3 v3.2.0
2323
github.com/mattn/go-colorable v0.1.15
2424
github.com/mattn/go-isatty v0.0.24
25-
github.com/minio/minio-go/v7 v7.3.1-0.20260828014306-0e78d3f18efe
25+
github.com/minio/minio-go/v7 v7.3.1-0.20260909183557-78bfa91607c2
2626
github.com/minio/mux v1.10.1
2727
github.com/rjeczalik/notify v0.9.3
2828
github.com/tinylib/msgp v1.6.4

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ github.com/mattn/go-runewidth v0.0.29 h1:3oGF3R/S2N9DQ3ptftzVIvg2eicmojCzlwBEmqE
9797
github.com/mattn/go-runewidth v0.0.29/go.mod h1:3qAiGCV4Koz/yuveO58qUefmUTRm8r0IGEXZ9jeHp/8=
9898
github.com/minio/md5-simd v1.1.2 h1:Gdi1DZK69+ZVMoNHRXJyNcxrMA4dSxoYHZSQbirFg34=
9999
github.com/minio/md5-simd v1.1.2/go.mod h1:MzdKDxYpY2BT9XQFocsiZf/NKVtR7nkE4RoEpN+20RM=
100-
github.com/minio/minio-go/v7 v7.3.1-0.20260828014306-0e78d3f18efe h1:By2FKNSOUGLOeb0x4D7xJMHr8x/X1ZW8PG780SpKUwQ=
101-
github.com/minio/minio-go/v7 v7.3.1-0.20260828014306-0e78d3f18efe/go.mod h1:KUPWdecEO1LWyUz+sTGXAuf2jZHrPh5fCsRH86QbPfk=
100+
github.com/minio/minio-go/v7 v7.3.1-0.20260909183557-78bfa91607c2 h1:nvX7IksPFOF/cBvkSg/Z+urBZkmhZrfUVmzdNFLjZ5Y=
101+
github.com/minio/minio-go/v7 v7.3.1-0.20260909183557-78bfa91607c2/go.mod h1:KUPWdecEO1LWyUz+sTGXAuf2jZHrPh5fCsRH86QbPfk=
102102
github.com/minio/mux v1.10.1 h1:grrK8SwRKbkNFE6qG7WAvFGH09bB46d5teOOtKfQ14s=
103103
github.com/minio/mux v1.10.1/go.mod h1:INYT4sMSTJy0QWUEA/E2DZNxJ5sAxIwbnyZjkzNFRfE=
104104
github.com/philhofer/fwd v1.2.0 h1:e6DnBTl7vGY+Gz322/ASL4Gyp1FspeMvx1RNDoToZuM=

policy/constants.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,20 @@ var DefaultPolicies = []struct {
6060
Actions: NewActionSet(GetBucketLocationAction, GetObjectAction),
6161
Resources: NewResourceSet(NewResource("*")),
6262
},
63+
},
64+
},
65+
},
66+
67+
// ConsoleReadOnly - read only with ListBucket for console browsing.
68+
{
69+
Name: "consolereadonly",
70+
Definition: Policy{
71+
Version: DefaultVersion,
72+
Statements: []Statement{
6373
{
6474
SID: ID(""),
65-
Effect: Deny,
66-
Actions: NewActionSet(CreateUserAdminAction),
75+
Effect: Allow,
76+
Actions: NewActionSet(GetBucketLocationAction, GetObjectAction, ListBucketAction),
6777
Resources: NewResourceSet(NewResource("*")),
6878
},
6979
},

policy/password_test.go

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
// Copyright (c) 2026 Feng Ruohang
2+
// SPDX-License-Identifier: AGPL-3.0-or-later
3+
4+
package policy
5+
6+
import (
7+
"strings"
8+
"testing"
9+
)
10+
11+
func TestPasswordAndUserManagementActions(t *testing.T) {
12+
for _, tt := range []struct {
13+
name string
14+
statements string
15+
password bool
16+
createUser bool
17+
}{
18+
{"implicit self service", `{"Effect":"Allow","Action":"s3:GetObject","Resource":"arn:aws:s3:::*"}`, true, false},
19+
{"explicit password grant", `{"Effect":"Allow","Action":"admin:ChangeMyPassword"}`, true, false},
20+
{"explicit user management grant", `{"Effect":"Allow","Action":"admin:CreateUser"}`, true, true},
21+
{"legacy user management deny", `{"Effect":"Deny","Action":"admin:CreateUser","Resource":"arn:aws:s3:::*"}`, true, false},
22+
{"password deny", `{"Effect":"Deny","Action":"admin:ChangeMyPassword"}`, false, false},
23+
{"password deny with user management grant", `{"Effect":"Deny","Action":"admin:ChangeMyPassword"},{"Effect":"Allow","Action":"admin:CreateUser"}`, false, true},
24+
{"deny overrides allow", `{"Effect":"Allow","Action":"admin:ChangeMyPassword"},{"Effect":"Deny","Action":"admin:ChangeMyPassword"}`, false, false},
25+
{"wildcard deny", `{"Effect":"Deny","Action":"admin:*"}`, false, false},
26+
{"both denies", `{"Effect":"Deny","Action":["admin:CreateUser","admin:ChangeMyPassword"]}`, false, false},
27+
} {
28+
t.Run(tt.name, func(t *testing.T) {
29+
doc := `{"Version":"2012-10-17","Statement":[` + tt.statements + `]}`
30+
for _, mode := range []string{"read", "write", "merge"} {
31+
t.Run(mode, func(t *testing.T) {
32+
parser := ParseConfig
33+
if mode == "write" {
34+
parser = ParseConfigStrict
35+
}
36+
p, err := parser(strings.NewReader(doc))
37+
if err != nil {
38+
t.Fatal(err)
39+
}
40+
if mode == "merge" {
41+
merged := MergePolicies(*p, Policy{})
42+
p = &merged
43+
}
44+
actions := p.IsAllowedActions("", "", nil)
45+
for action, want := range map[Action]bool{
46+
ChangeMyPasswordAdminAction: tt.password,
47+
CreateUserAdminAction: tt.createUser,
48+
} {
49+
if got := actions.Contains(action); got != want {
50+
t.Errorf("reported %s = %v, want %v", action, got, want)
51+
}
52+
if got := p.IsAllowed(Args{Action: action, DenyOnly: action == ChangeMyPasswordAdminAction}); got != want {
53+
t.Errorf("authorized %s = %v, want %v", action, got, want)
54+
}
55+
}
56+
})
57+
}
58+
})
59+
}
60+
}
61+
62+
func TestReadOnlySelfServicePolicies(t *testing.T) {
63+
for _, name := range []string{"readonly", "consolereadonly"} {
64+
t.Run(name, func(t *testing.T) {
65+
var p *Policy
66+
for _, canned := range DefaultPolicies {
67+
if canned.Name == name {
68+
p = &canned.Definition
69+
break
70+
}
71+
}
72+
if p == nil {
73+
t.Fatal("missing canned policy")
74+
}
75+
actions := p.IsAllowedActions("bucket", "object", nil)
76+
for action, want := range map[Action]bool{
77+
GetObjectAction: true,
78+
GetBucketLocationAction: true,
79+
ListBucketAction: name == "consolereadonly",
80+
PutObjectAction: false,
81+
DeleteObjectAction: false,
82+
CreateUserAdminAction: false,
83+
ChangeMyPasswordAdminAction: true,
84+
CreateServiceAccountAdminAction: true,
85+
} {
86+
if got := actions.Contains(action); got != want {
87+
t.Errorf("%s = %v, want %v", action, got, want)
88+
}
89+
}
90+
// A read-only grant must compose with a separate user-admin policy.
91+
userAdmin := Policy{Version: DefaultVersion, Statements: []Statement{{
92+
Effect: Allow, Actions: NewActionSet(CreateUserAdminAction),
93+
}}}
94+
merged := MergePolicies(*p, userAdmin)
95+
if !merged.IsAllowed(Args{Action: CreateUserAdminAction}) {
96+
t.Error("read-only policy overrides an independent user-management grant")
97+
}
98+
})
99+
}
100+
}

policy/policy.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,8 @@ func (iamp Policy) IsAllowedActions(bucketName, objectName string, conditionValu
169169
ObjectName: objectName,
170170
Action: admAction,
171171
ConditionValues: conditionValues,
172-
// checks mainly for actions that can have explicit
173-
// deny, while without it are implicitly enabled.
174-
DenyOnly: action == CreateServiceAccountAdminAction || action == CreateUserAdminAction,
172+
// Match the server's implicit grants for self-service actions.
173+
DenyOnly: action == CreateServiceAccountAdminAction || action == ChangeMyPasswordAdminAction,
175174
}) {
176175
actionSet.Add(admAction)
177176
}

policy/policy_regression_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,12 +162,12 @@ func TestHasDenyStatementWithoutParsing(t *testing.T) {
162162
})
163163
}
164164
for _, p := range DefaultPolicies {
165-
if p.Name == "readonly" {
166-
if !p.Definition.HasDenyStatement() {
167-
t.Error("readonly's explicit Deny must be reported before parsing")
168-
}
169-
return
165+
want := false
166+
for _, statement := range p.Definition.Statements {
167+
want = want || statement.Effect == Deny
168+
}
169+
if got := p.Definition.HasDenyStatement(); got != want {
170+
t.Errorf("%s: HasDenyStatement() = %v, want %v", p.Name, got, want)
170171
}
171172
}
172-
t.Fatal("readonly policy not found")
173173
}

0 commit comments

Comments
 (0)