mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-16 20:26:45 +00:00
filer-conditional-create
62
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Chris LuandGitHub
|
cc5ef1b741 |
feat(s3): add TagUser, UntagUser, ListUserTags IAM actions (#9572)
* feat(s3): add TagUser, UntagUser, ListUserTags IAM actions Adds AWS IAM-compatible user tag operations on the embedded IAM endpoint. Tags persist in the Identity proto as a repeated UserTag field; the existing 50-tag / 128-byte-key / 256-byte-value AWS limits are enforced. Pagination is stubbed (IsTruncated=false) since the 50-tag cap means all tags fit in a single response. * review: validate UntagUser TagKeys entries parseTagKeysParams now rejects empty keys and keys past MaxUserTagKeyLength; UntagUser additionally requires at least one TagKeys.member.N entry to match AWS validation behavior. * review: pre-allocate user-tag merge and filter slices mergeUserTags now allocates the combined existing+incoming capacity up front; UntagUser builds the filtered slice via make with the full ident.Tags capacity instead of ident.Tags[:0:0], which forced a reallocation on every append. * review: cover duplicate-in-request and invalid TagKeys cases Regression tests assert TagUser rejects two members with the same key in one request, and UntagUser rejects missing/empty/oversized TagKeys entries. |
||
|
Chris LuandGitHub
|
12f283357f |
fix(iam): four phase-3 follow-ups (provider scoping, public path wrapper, static mirror, claim-mode RoleArn) (#9333)
* fix(iam): scope IAM-managed OIDC provider lookup by role account Two account-scoped OIDC records sharing an issuer were collapsed into a single map slot keyed only by the URL. The last-write-wins entry then served every AssumeRoleWithWebIdentity, so a token destined for account B's role could be validated by account A's record (its clientIDs and thumbprints), defeating the per-account isolation the records exist for. The role-account check in enforceProviderAccountScope still rejected the cross-account assumption, but only after the wrong record's audience and TLS pin had already accepted the token. Refresh now keys IAM-managed records as (issuer, account), and validation parses the requested role's account up front and matches the record under that issuer in this order: exact account, global (account-less), static-config fallback. An unknown account hint deliberately skips account-scoped entries — picking one arbitrarily is the bug this commit fixes — and falls through to global or static. * fix(iam): route public AssumeRoleWithWebIdentity through IAMManager handleAssumeRoleWithWebIdentity called stsService.AssumeRoleWithWebIdentity directly, bypassing the IAMManager wrapper. The wrapper is where enforceProviderAccountScope rejects cross-account assumption attempts and capDurationByRole clamps to the role's MaxSessionDuration; both silently became no-ops for any AWS-SDK caller hitting the public endpoint. Dispatch through the IAMManager (via the existing IAMManagerProvider interface that other handlers in this file already use) when one is wired. Embedded test setups without an IAM integration fall back to the bare STS service unchanged. * fix(iam): mirror thumbprints, principal-tag keys, and policy claim from static OIDC config initOIDCProviderStore mirrored only URL and ClientIDs. Once RefreshOIDCProvidersFromStore ran (on any IAM-managed mutation, or on boot once the metadata-subscribe loop kicked in), buildOIDCProviderFromRecord rebuilt the runtime provider from this truncated record. Because IAM-managed entries take precedence over the static-config map, the rebuild silently shadowed the bootstrap with a weaker provider: - Thumbprints: dropped, so TLS-pinned issuers fell back to the system trust store. - AllowedPrincipalTagKeys: dropped, so principal-tag claims stopped reaching the session. - PolicyClaim: dropped, so claim-based policy mode stopped triggering. Pull all three from the provider's static Config map at mirror time so the stored record round-trips to a runtime provider equivalent to the one the static config produced directly. * fix(iam): allow empty RoleArn in AssumeRoleWithWebIdentity HTTP handler Phase 3b advertises that RoleArn MAY be omitted in claim-based policy mode — the STS service then derives the assumed-role ARN from the configured policy claim. The HTTP handler still rejected empty RoleArn up front with MissingParameter, so SDK callers using the documented omitted-role flow never reached the STS layer. Drop the pre-check; STS still validates that claim-based mode is configured and that the IDP emits policies, returning a precise error when either is missing. The existing error mapping below this point surfaces those as InvalidParameterValue, matching what an AWS SDK expects. * test(iam): update missing-RoleArn STS integration test for the new contract The previous commit drops the HTTP-layer RoleArn pre-check so claim-based mode can derive the ARN from a JWT claim. The integration test still asserted MissingParameter for the missing-RoleArn case, which now reaches the STS layer and surfaces a JWT-parse error instead. Update the assertion to match: missing RoleArn alone must no longer surface as MissingParameter, but a bogus JWT must still be rejected. |
||
|
Chris LuandGitHub
|
9af1b212d3 |
feat(iam): OIDC provider audit trail (Phase 3e) (#9325)
* feat(iam): claim-based policy mode for AssumeRoleWithWebIdentity When the caller passes the sentinel RoleArn arn:aws:iam:::role/sts-claim-based (or omits it entirely) and the matched OIDC provider has policyClaim set, mint a session whose effective policies come from that JWT claim instead of from a server-side role mapping. Accepts string, comma-separated string, or array shapes — MinIO-compatible behaviour for IDPs that already attach policies to the user. Trust-policy validation is skipped in claim-mode: the IDP is the sole authority for both authentication and authorization, mirroring the contract MinIO documents for its DummyRoleARN flow. Concrete-role mode is unchanged and still requires the role definition + trust policy. * fix(iam): trim policy-claim array elements + clean up stale comments Three medium-priority cleanups gemini flagged on the claim-based path: - extractClaimPolicies's array branch was leaving whitespace on each element while the string/comma-separated branch trimmed via splitPolicyClaimString. An IDP that emits ["readonly", " billing "] would create a "billing" policy lookup that didn't match the stored name. Trim every array element, drop empties. - The "synthetic ARN keyed on the session name" comment was wrong — effectiveRoleArn here is the literal sentinel; it's the assumed-role ARN generated downstream that's session-keyed. Reword. - The empty if/else block at the start of validateAssumeRoleWithWebIdentityRequest existed only to host a comment about deferred validation; the comment now lives in the function godoc and the empty branch is gone. Addresses three gemini medium reviews on PR #9322. * feat(iam): account-scoped OIDC providers Add OIDCProviderRecord.AccountID enforcement: when a role lives in account A, the OIDC provider validating the assume-role token must be either global (AccountID="") or also live in account A. Cross-account use is rejected at the IAM-manager layer before reaching the trust policy validator. OIDCProviderStore gains GetProviderByIssuerAndAccount; both the in- memory and filer-backed stores implement it. Static-config-only deployments are unaffected since they don't populate the store. * fix(iam): account-scoped lookup for cross-account check enforceProviderAccountScope was calling GetProviderByIssuer, which returns the first match arbitrarily when multiple providers share an issuer (one global + one per tenant is the canonical setup). On a two-record collision the wrong record could come back first and falsely reject a valid same-account or global-provider request. Use GetProviderByIssuerAndAccount as the primary lookup so the filter happens in the store. On miss, fall back to GetProviderByIssuer purely to distinguish "issuer entirely unknown" (let the STS layer reject) from "issuer registered in a different account" (surface a precise cross-account error). Addresses gemini high-priority review on PR #9323. * feat(iam): opt-in session revocation via JTI blocklist Add SessionRevocationStore (memory + filer implementations) and wire it into the IAMManager.IsActionAllowed path so a revoked session is rejected on the next signed request. Session JWTs now embed the session id as the JTI claim, giving the blocklist a stable key without requiring a second secret. Operators who don't configure a store keep the existing fully-stateless behaviour: every session stays valid until natural expiry. Operators who do configure one accept one filer lookup per signed request in exchange for being able to invalidate compromised tokens before expiry. Revocation entries carry the original session expiry so the blocklist self-trims via PurgeRevokedSessions. * fix(iam): hash JTI filenames + paginate Purge with proper EOF handling Three reviewer-flagged issues on the filer-backed revocation store: 1. Path traversal (security-medium): RevokeSession is exported and takes an arbitrary string. Using the JTI verbatim as a filename meant a caller could pass "../../etc/passwd" to write outside the basePath. SHA-1 hash the JTI to a fixed-width hex name; lookups still find the entry because Revoke and IsRevoked share the same hash function. 2. Purge swallowed errors. The inner `err` from stream.Recv() shadowed the outer err and the loop just broke on any failure, so a mid-stream gRPC error returned (count, nil) and the caller had no idea the purge was incomplete. Switch to errors.Is(io.EOF) for end-of-stream and propagate everything else. 3. Purge had a hardcoded 10000-entry cap. Stream-paginate via StartFromFileName so the operator-cron can clean a backlog larger than that without losing rows. * feat(iam): OIDC provider audit trail Emit one structured event per IAM-managed OIDC provider lifecycle mutation (Create, Delete, Add/Remove ClientID, UpdateThumbprints, Tag, Untag). Three sinks ship in-tree: - GlogAuditSink — default; events become structured log lines. - MemoryAuditSink — in-process buffer for tests / inspection. - FilerAuditSink — durable, one file per event under /etc/iam/audit/oidc-providers (operator-overridable basePath). Audit emission is best-effort: a failing sink never blocks an IAM mutation that has already succeeded. Use-events (per token validation) are intentionally not emitted yet — too hot for unconditional sinks. * fix(iam): collision-free audit filenames + correct file mtime Two issues gemini flagged on FilerAuditSink.Emit: 1. Filename was %d-%s.json (UnixNano + Type). Two mutations at the same nano against different ARNs (think batch script touching several providers) collided on the same path and the second CreateEntry failed silently. Append a short ARN hash to make the name unique per-event without leaking the ARN. 2. File Mtime/Crtime were set to time.Now() at write time. For audit integrity those should reflect the event's occurrence time so filer-level "ls -lt" output matches the contents. Addresses three medium-priority gemini reviews on PR #9325. |
||
|
Chris LuandGitHub
|
9d6a699b94 |
feat(iam): opt-in session revocation via JTI blocklist (Phase 3d) (#9324)
* feat(iam): claim-based policy mode for AssumeRoleWithWebIdentity When the caller passes the sentinel RoleArn arn:aws:iam:::role/sts-claim-based (or omits it entirely) and the matched OIDC provider has policyClaim set, mint a session whose effective policies come from that JWT claim instead of from a server-side role mapping. Accepts string, comma-separated string, or array shapes — MinIO-compatible behaviour for IDPs that already attach policies to the user. Trust-policy validation is skipped in claim-mode: the IDP is the sole authority for both authentication and authorization, mirroring the contract MinIO documents for its DummyRoleARN flow. Concrete-role mode is unchanged and still requires the role definition + trust policy. * fix(iam): trim policy-claim array elements + clean up stale comments Three medium-priority cleanups gemini flagged on the claim-based path: - extractClaimPolicies's array branch was leaving whitespace on each element while the string/comma-separated branch trimmed via splitPolicyClaimString. An IDP that emits ["readonly", " billing "] would create a "billing" policy lookup that didn't match the stored name. Trim every array element, drop empties. - The "synthetic ARN keyed on the session name" comment was wrong — effectiveRoleArn here is the literal sentinel; it's the assumed-role ARN generated downstream that's session-keyed. Reword. - The empty if/else block at the start of validateAssumeRoleWithWebIdentityRequest existed only to host a comment about deferred validation; the comment now lives in the function godoc and the empty branch is gone. Addresses three gemini medium reviews on PR #9322. * feat(iam): account-scoped OIDC providers Add OIDCProviderRecord.AccountID enforcement: when a role lives in account A, the OIDC provider validating the assume-role token must be either global (AccountID="") or also live in account A. Cross-account use is rejected at the IAM-manager layer before reaching the trust policy validator. OIDCProviderStore gains GetProviderByIssuerAndAccount; both the in- memory and filer-backed stores implement it. Static-config-only deployments are unaffected since they don't populate the store. * fix(iam): account-scoped lookup for cross-account check enforceProviderAccountScope was calling GetProviderByIssuer, which returns the first match arbitrarily when multiple providers share an issuer (one global + one per tenant is the canonical setup). On a two-record collision the wrong record could come back first and falsely reject a valid same-account or global-provider request. Use GetProviderByIssuerAndAccount as the primary lookup so the filter happens in the store. On miss, fall back to GetProviderByIssuer purely to distinguish "issuer entirely unknown" (let the STS layer reject) from "issuer registered in a different account" (surface a precise cross-account error). Addresses gemini high-priority review on PR #9323. * feat(iam): opt-in session revocation via JTI blocklist Add SessionRevocationStore (memory + filer implementations) and wire it into the IAMManager.IsActionAllowed path so a revoked session is rejected on the next signed request. Session JWTs now embed the session id as the JTI claim, giving the blocklist a stable key without requiring a second secret. Operators who don't configure a store keep the existing fully-stateless behaviour: every session stays valid until natural expiry. Operators who do configure one accept one filer lookup per signed request in exchange for being able to invalidate compromised tokens before expiry. Revocation entries carry the original session expiry so the blocklist self-trims via PurgeRevokedSessions. * fix(iam): hash JTI filenames + paginate Purge with proper EOF handling Three reviewer-flagged issues on the filer-backed revocation store: 1. Path traversal (security-medium): RevokeSession is exported and takes an arbitrary string. Using the JTI verbatim as a filename meant a caller could pass "../../etc/passwd" to write outside the basePath. SHA-1 hash the JTI to a fixed-width hex name; lookups still find the entry because Revoke and IsRevoked share the same hash function. 2. Purge swallowed errors. The inner `err` from stream.Recv() shadowed the outer err and the loop just broke on any failure, so a mid-stream gRPC error returned (count, nil) and the caller had no idea the purge was incomplete. Switch to errors.Is(io.EOF) for end-of-stream and propagate everything else. 3. Purge had a hardcoded 10000-entry cap. Stream-paginate via StartFromFileName so the operator-cron can clean a backlog larger than that without losing rows. |
||
|
Chris LuandGitHub
|
6483583491 |
feat(iam): account-scoped OIDC providers (Phase 3c) (#9323)
* feat(iam): claim-based policy mode for AssumeRoleWithWebIdentity When the caller passes the sentinel RoleArn arn:aws:iam:::role/sts-claim-based (or omits it entirely) and the matched OIDC provider has policyClaim set, mint a session whose effective policies come from that JWT claim instead of from a server-side role mapping. Accepts string, comma-separated string, or array shapes — MinIO-compatible behaviour for IDPs that already attach policies to the user. Trust-policy validation is skipped in claim-mode: the IDP is the sole authority for both authentication and authorization, mirroring the contract MinIO documents for its DummyRoleARN flow. Concrete-role mode is unchanged and still requires the role definition + trust policy. * fix(iam): trim policy-claim array elements + clean up stale comments Three medium-priority cleanups gemini flagged on the claim-based path: - extractClaimPolicies's array branch was leaving whitespace on each element while the string/comma-separated branch trimmed via splitPolicyClaimString. An IDP that emits ["readonly", " billing "] would create a "billing" policy lookup that didn't match the stored name. Trim every array element, drop empties. - The "synthetic ARN keyed on the session name" comment was wrong — effectiveRoleArn here is the literal sentinel; it's the assumed-role ARN generated downstream that's session-keyed. Reword. - The empty if/else block at the start of validateAssumeRoleWithWebIdentityRequest existed only to host a comment about deferred validation; the comment now lives in the function godoc and the empty branch is gone. Addresses three gemini medium reviews on PR #9322. * feat(iam): account-scoped OIDC providers Add OIDCProviderRecord.AccountID enforcement: when a role lives in account A, the OIDC provider validating the assume-role token must be either global (AccountID="") or also live in account A. Cross-account use is rejected at the IAM-manager layer before reaching the trust policy validator. OIDCProviderStore gains GetProviderByIssuerAndAccount; both the in- memory and filer-backed stores implement it. Static-config-only deployments are unaffected since they don't populate the store. * fix(iam): account-scoped lookup for cross-account check enforceProviderAccountScope was calling GetProviderByIssuer, which returns the first match arbitrarily when multiple providers share an issuer (one global + one per tenant is the canonical setup). On a two-record collision the wrong record could come back first and falsely reject a valid same-account or global-provider request. Use GetProviderByIssuerAndAccount as the primary lookup so the filter happens in the store. On miss, fall back to GetProviderByIssuer purely to distinguish "issuer entirely unknown" (let the STS layer reject) from "issuer registered in a different account" (surface a precise cross-account error). Addresses gemini high-priority review on PR #9323. |
||
|
Chris LuandGitHub
|
bc1d458fe6 |
fix(iam): reject empty issuer in ComputeParentUser (#9326)
Without iss, the same `sub` from two different IDPs would collapse to the same parent_user hash. Short-circuit to empty when either input is missing so callers see "no identity" instead of a colliding hash. Addresses coderabbit review on PR #9318. |
||
|
Chris LuandGitHub
|
1d3454ca5c |
feat(iam): claim-based policy mode for AssumeRoleWithWebIdentity (Phase 3b) (#9322)
* feat(iam): claim-based policy mode for AssumeRoleWithWebIdentity When the caller passes the sentinel RoleArn arn:aws:iam:::role/sts-claim-based (or omits it entirely) and the matched OIDC provider has policyClaim set, mint a session whose effective policies come from that JWT claim instead of from a server-side role mapping. Accepts string, comma-separated string, or array shapes — MinIO-compatible behaviour for IDPs that already attach policies to the user. Trust-policy validation is skipped in claim-mode: the IDP is the sole authority for both authentication and authorization, mirroring the contract MinIO documents for its DummyRoleARN flow. Concrete-role mode is unchanged and still requires the role definition + trust policy. * fix(iam): trim policy-claim array elements + clean up stale comments Three medium-priority cleanups gemini flagged on the claim-based path: - extractClaimPolicies's array branch was leaving whitespace on each element while the string/comma-separated branch trimmed via splitPolicyClaimString. An IDP that emits ["readonly", " billing "] would create a "billing" policy lookup that didn't match the stored name. Trim every array element, drop empties. - The "synthetic ARN keyed on the session name" comment was wrong — effectiveRoleArn here is the literal sentinel; it's the assumed-role ARN generated downstream that's session-keyed. Reword. - The empty if/else block at the start of validateAssumeRoleWithWebIdentityRequest existed only to host a comment about deferred validation; the comment now lives in the function godoc and the empty branch is gone. Addresses three gemini medium reviews on PR #9322. |
||
|
Chris LuandGitHub
|
6554ab7928 |
feat(iam): principal session tags from OIDC tokens (Phase 3a) (#9321)
* feat(iam): principal session tags from OIDC tokens Extract the AWS principal-tags namespace claim (`https://aws.amazon.com/tags/principal_tags`) from validated OIDC tokens, filter through a per-provider AllowedPrincipalTagKeys allowlist, and surface as `aws:PrincipalTag/<key>` in the STS session request context. Empty allowlist means "no tags surfaced" — operators must opt keys in explicitly so a misconfigured IDP can't pollute policy evaluation. Policy engine now accepts `aws:PrincipalTag/...` and `aws:RequestTag/...` as substitutable variables so resource-level ABAC policies can reference them. * fix(iam): case-insensitive principal-tag allowlist + sharper comment filterPrincipalTags compared keys case-sensitively, but AWS IAM session tag keys are case-insensitive (the docs are explicit). An IDP whose claim casing drifts from the operator-configured allowlist string would silently filter the value out — surprising failure mode. Lowercase both sides during the lookup; the original key casing is preserved on the output so policy variables still match what the caller sees. Also reword the "anything the IDP signs is acceptable" comment in sts_service.go: it predates the per-provider allowlist that's already filtering before this point. The reality is now "everything reaching here is on the operator's opt-in list, dropped entirely if the allowlist is empty." Addresses two gemini medium reviews on PR #9321. |
||
|
Chris LuandGitHub
|
f8973b3ed6 |
feat(iam): OIDC provider mutations + multi-client + TLS thumbprints (Phase 2b) (#9320)
* feat(iam): OIDC provider mutations + multi-client + TLS thumbprints - Mutating IAM actions: CreateOpenIDConnectProvider, DeleteOpenIDConnectProvider, AddClientIDToOpenIDConnectProvider, RemoveClientIDFromOpenIDConnectProvider, UpdateOpenIDConnectProviderThumbprint, TagOpenIDConnectProvider, UntagOpenIDConnectProvider. Each enforces AWS-shape input bounds and the read-only mode rejects all mutations. - Multiple client_ids per provider in OIDCConfig (clientIds list, plural) with full backward compatibility — singular clientId still works and is merged into the audience allowlist. Provider factory accepts both. - AWS-compatible TLS thumbprint pinning: when OIDCConfig.Thumbprints is non-empty, JWKS fetches enforce that the negotiated TLS chain contains a certificate whose SHA-1 hex matches the allowlist. Empty list keeps the existing system-trust path. * fix(iam): factor Tags.member.N parser into a helper CreateOpenIDConnectProvider and TagOpenIDConnectProvider were both walking the AWS Tags.member.N.Key / Tags.member.N.Value query-string convention with copy-pasted loops. Factor into extractTags so the parsing rules and the "no tags present" semantics live in one place. Addresses gemini medium review on PR #9320. * fix(iam): sentinel errors for OIDC provider not-found / already-exists The s3api dispatcher was using strings.Contains(err.Error(), "not found") and "already exists" to map IAM-manager errors back to AWS error codes. Substring matching on a formatted message couples the API error code to the exact wording of the upstream message — touching the message silently changes the IAM API contract. Define ErrOIDCProviderNotFound and ErrOIDCProviderAlreadyExists in the integration package, fmt.Errorf("%w: ...") them at the four return sites in iam_manager.go and oidc_provider_store.go, and use errors.Is at the s3api call sites. Same control flow, no string-match fragility. Addresses gemini medium review on PR #9320. * fix(iam): surface non-NotFound errors from CreateOIDCProvider lookup Previously CreateOIDCProvider only treated GetProviderByARN's success path as "exists" and silently fell through on any error, including transient backend failures. That hid real problems and still attempted a write. Distinguish ErrOIDCProviderNotFound (the only "safe to create" case) from other errors so we don't mask filer outages or partition issues. * fix(iam): enforce 100-client-ID cap on AddClientIDToOIDCProvider CreateOIDCProvider and the implicit update path through validateOIDC- ProviderRecord both reject lists with more than 100 client IDs, but AddClientIDToOIDCProvider could grow the list past that bound one ID at a time. Refuse the add when the list is already at the cap so the invariant holds across every mutation entry point. * feat(iam): IAM-managed OIDC provider live view in STS service Add a separate, mutex-guarded map of admin-managed OIDC providers on the STS service. The map can be atomically replaced via SetIAMManagedOIDCProvidersByIssuer; AssumeRoleWithWebIdentity lookups consult it first and fall back to the existing static-config map, so records persisted through the IAM API can shadow bootstrap entries without a restart. This is the runtime hook the IAM API and the metadata-subscribe path will both call when the OIDCProviderStore changes (next two commits). * feat(iam): refresh STS service runtime view after OIDC mutations Add IAMManager.RefreshOIDCProvidersFromStore: lists every persisted OIDCProviderRecord, builds a runtime OIDCProvider for each, and atomically publishes the issuer-keyed map into the STS service. Each mutating IAM API call (Create / Delete / AddClientID / RemoveClientID / UpdateThumbprints) now triggers this refresh inline so the local instance picks up the change without waiting for a metadata-subscribe round trip. Tag mutations skip the refresh because tags do not affect token validation. Refresh failures only log; the persisted write has already succeeded by that point, so a transient list error must not surface to the API caller. The peer-instance update path (filer metadata subscription) is added in a follow-up commit. * feat(iam): subscribe to OIDC provider changes on the filer Watch /etc/iam/oidc-providers under the existing s3 metadata-subscribe loop and call RefreshOIDCProvidersFromStore on any create / update / delete / rename. This is the cross-instance update path: S3 server A writes via the IAM API, the filer fans out the metadata change, and S3 servers B..N pick up the new runtime view without a restart. Mirrors the existing onIamConfigChange / onCircuitBreakerConfigChange pattern. The handler short-circuits when the path is unrelated, and when no IAMManager is wired in (static-only configurations). |
||
|
Chris LuandGitHub
|
4ded97a321 |
feat(iam): OIDC provider store + read-only IAM API (Phase 2a) (#9319)
* feat(iam): STS web-identity AWS-fidelity polish - OIDC discovery via .well-known/openid-configuration; falls back to /.well-known/jwks.json when discovery is absent. Reject discovery docs whose issuer claim does not match the configured issuer to defend against issuer-substitution. - ComputeParentUser derives a stable per-identity hash from (sub, iss). Surface as aws:userid in the request context and as a parent_user claim in the session JWT so per-user state survives token rotation. - Per-role MaxSessionDuration (3600..43200) clamps requested DurationSeconds before the STS service applies its own caps. - Tighten RoleSessionName to the AWS contract: 2..64 chars from [\w+=,.@-]. - Populate PackedPolicySize in AssumeRole / AssumeRoleWithWebIdentity / AssumeRoleWithLDAPIdentity responses as a percentage of the 2048-byte inline session policy budget. * fix(iam): leave omitted DurationSeconds nil so STS default applies capDurationByRole was substituting the role's MaxSessionDuration when the caller omitted DurationSeconds entirely. AWS returns the configured default (typically 1 hour) in that case, not the role's upper bound — a 12h MaxSessionDuration shouldn't silently make every no-duration assume-role mint a 12h session. Return nil when requested is nil; let the downstream calculateSessionDuration in the STS service apply its TokenDuration default. The role-max upper bound still clamps when the request arrives with a concrete value above the cap. Addresses gemini high-priority review on PR #9318. * fix(iam): synchronize OIDCProvider JWKS cache fields jwksCache, jwksFetchedAt, resolvedJWKSUri, and discoveryFailed are mutated lazily on the first token-validate call and refreshed afterwards on TTL expiry. Multiple S3 requests can land here in parallel, so the writes were racing against subsequent reads on every other goroutine. resolvedJWKSUri/discoveryFailed inherited the same un-protected pattern when discovery shipped. Add sync.RWMutex; getPublicKey takes the read lock for the common cache-hit path and promotes to the write lock for misses + refreshes. fetchJWKSLocked / resolveJWKSUriLocked assume the write lock is held by the caller; fetchJWKS keeps the test-friendly entry point that acquires the lock itself. Addresses gemini high-priority review on PR #9318. * fix(iam): trim trailing slash + retry discovery after transient failure Two OIDC discovery edge cases reviewers flagged: 1. Issuer comparison was sensitive to trailing slashes. resolveJWKSUri trims them when building the discovery URL, but the doc.Issuer ↔ p.config.Issuer check did not, so an IDP whose issuer claim drops or adds the slash relative to the configured value would be falsely rejected. Trim a single trailing slash on each side before comparing. 2. discoveryFailed flipped to true on any error and stayed there for the process lifetime. A transient 5xx at startup permanently locked the provider into the /.well-known/jwks.json fallback. Reset the flag at the top of fetchJWKSLocked when no URI has been cached yet, so each JWKS refresh (typically once per TTL = 1h) reattempts discovery. Successful discovery remains cached via resolvedJWKSUri so we don't pay the discovery RTT on every refresh. Addresses gemini security-medium + medium reviews on PR #9318. * fix(iam): require non-empty issuer in OIDC discovery doc The previous "doc.Issuer != "" && ..." guard let a discovery document that omitted the issuer field bypass the issuer-mismatch check entirely, letting the doc steer fetchJWKS at any URL it provided. OIDC Discovery 1.0 §3 mandates the issuer field; treat missing as a hard failure same as mismatched. Trailing-slash equivalence still applies. Adds TestDiscoveryRejectsMissingIssuer alongside the existing TestDiscoveryRejectsIssuerMismatch via a new omitDiscoveryIssuer toggle on fakeIDP. * feat(iam): OIDC provider store + read-only IAM API Add OIDCProviderRecord — the persisted, IAM-managed view of an OIDC identity provider — and an OIDCProviderStore interface with memory and filer implementations mirroring the existing role-store pattern. The store is hydrated at boot from the static STS.Providers list so the new IAM API surfaces the same set the STS service already validates against. Two read-only actions land now: - ListOpenIDConnectProviders -> ARN-only list, AWS-shape XML. - GetOpenIDConnectProvider -> URL, ClientIDList, ThumbprintList, Tags, CreateDate. Mutations (Create/Delete/Add-Remove ClientID/Update Thumbprint), multiple client_ids per provider, and TLS thumbprint pinning come in Phase 2b. * fix(iam): preserve CreatedAt across boots + paginate ListProviders Two medium-priority issues gemini flagged on the read-only IAM API: 1. The static-config bootstrap was setting CreatedAt = time.Now() on every server start, so the IAM GetOpenIDConnectProvider response's CreateDate shifted on each restart even when backed by a persistent store. Look up the existing record via GetProviderByARN first and preserve its CreatedAt; only the UpdatedAt advances. 2. FilerOIDCProviderStore.ListProviders had a hardcoded Limit: 1000 that silently truncated above that. Stream-paginate via StartFromFileName, returning io.EOF naturally and surfacing all other errors instead of swallowing them. Addresses two gemini medium reviews on PR #9319. |
||
|
Chris LuandGitHub
|
d951a8df5a |
feat(iam): STS web-identity AWS-fidelity polish (Phase 1) (#9318)
* feat(iam): STS web-identity AWS-fidelity polish - OIDC discovery via .well-known/openid-configuration; falls back to /.well-known/jwks.json when discovery is absent. Reject discovery docs whose issuer claim does not match the configured issuer to defend against issuer-substitution. - ComputeParentUser derives a stable per-identity hash from (sub, iss). Surface as aws:userid in the request context and as a parent_user claim in the session JWT so per-user state survives token rotation. - Per-role MaxSessionDuration (3600..43200) clamps requested DurationSeconds before the STS service applies its own caps. - Tighten RoleSessionName to the AWS contract: 2..64 chars from [\w+=,.@-]. - Populate PackedPolicySize in AssumeRole / AssumeRoleWithWebIdentity / AssumeRoleWithLDAPIdentity responses as a percentage of the 2048-byte inline session policy budget. * fix(iam): leave omitted DurationSeconds nil so STS default applies capDurationByRole was substituting the role's MaxSessionDuration when the caller omitted DurationSeconds entirely. AWS returns the configured default (typically 1 hour) in that case, not the role's upper bound — a 12h MaxSessionDuration shouldn't silently make every no-duration assume-role mint a 12h session. Return nil when requested is nil; let the downstream calculateSessionDuration in the STS service apply its TokenDuration default. The role-max upper bound still clamps when the request arrives with a concrete value above the cap. Addresses gemini high-priority review on PR #9318. * fix(iam): synchronize OIDCProvider JWKS cache fields jwksCache, jwksFetchedAt, resolvedJWKSUri, and discoveryFailed are mutated lazily on the first token-validate call and refreshed afterwards on TTL expiry. Multiple S3 requests can land here in parallel, so the writes were racing against subsequent reads on every other goroutine. resolvedJWKSUri/discoveryFailed inherited the same un-protected pattern when discovery shipped. Add sync.RWMutex; getPublicKey takes the read lock for the common cache-hit path and promotes to the write lock for misses + refreshes. fetchJWKSLocked / resolveJWKSUriLocked assume the write lock is held by the caller; fetchJWKS keeps the test-friendly entry point that acquires the lock itself. Addresses gemini high-priority review on PR #9318. * fix(iam): trim trailing slash + retry discovery after transient failure Two OIDC discovery edge cases reviewers flagged: 1. Issuer comparison was sensitive to trailing slashes. resolveJWKSUri trims them when building the discovery URL, but the doc.Issuer ↔ p.config.Issuer check did not, so an IDP whose issuer claim drops or adds the slash relative to the configured value would be falsely rejected. Trim a single trailing slash on each side before comparing. 2. discoveryFailed flipped to true on any error and stayed there for the process lifetime. A transient 5xx at startup permanently locked the provider into the /.well-known/jwks.json fallback. Reset the flag at the top of fetchJWKSLocked when no URI has been cached yet, so each JWKS refresh (typically once per TTL = 1h) reattempts discovery. Successful discovery remains cached via resolvedJWKSUri so we don't pay the discovery RTT on every refresh. Addresses gemini security-medium + medium reviews on PR #9318. * fix(iam): require non-empty issuer in OIDC discovery doc The previous "doc.Issuer != "" && ..." guard let a discovery document that omitted the issuer field bypass the issuer-mismatch check entirely, letting the doc steer fetchJWKS at any URL it provided. OIDC Discovery 1.0 §3 mandates the issuer field; treat missing as a hard failure same as mismatched. Trailing-slash equivalence still applies. Adds TestDiscoveryRejectsMissingIssuer alongside the existing TestDiscoveryRejectsIssuerMismatch via a new omitDiscoveryIssuer toggle on fakeIDP. |
||
|
Chris LuandGitHub
|
66d9b89cd2 |
fix(iam): deny IAM users with no policies instead of granting full access (#9317)
* fix(iam): deny IAM users with zero policies instead of falling through to DefaultEffect=Allow A user created via the S3 IAM API with no policies attached was inheriting full S3 access. With `weed s3 -iam` and no explicit IAM config, the policy engine's DefaultEffect defaults to Allow for the in-memory zero-config path. The "no matching statement" guard in IsActionAllowed only triggered when the user already had at least one policy, so a fresh user with PolicyNames=[] slipped through and got allow-all. Track hasManagedSubject whenever the principal resolves to a registered user or role (or PolicyNames are supplied directly) and deny on no-match. The DefaultEffect=Allow fallback now only applies to truly unmanaged callers. * test(iam): cover non-matching attached-policy case for managed-subject deny * test(iam): cover role-with-empty-AttachedPolicies deny path Sibling case to TestIsActionAllowed_RegisteredUserWithoutPoliciesIsDenied: a managed role that resolves but has zero AttachedPolicies must also fall through to deny under DefaultEffect=Allow, not inherit full access. The fix already handles this branch via the hasManagedSubject flag; this test pins the regression class so we don't lose coverage on either side of the user/role split. Addresses coderabbit nitpick on PR #9317. |
||
|
Chris LuandGitHub
|
fe1d7a404d |
fix(iam): substitute dynamic jwt:/saml:/oidc: claim variables in policies (#9217)
* fix(iam): expand arbitrary jwt:/saml:/oidc: claim variables in policies
The policy engine gated variable substitution on a fixed allowlist
(jwt:sub, jwt:iss, jwt:aud, jwt:preferred_username), so patterns like
arn:aws:s3:::softs/${jwt:project_path}/* were passed through as literals
and never matched the requested resource. Dynamic claims from OIDC
providers (e.g. GitLab CI's project_path / namespace_path) could not be
used to scope policies.
Allow any jwt:/saml:/oidc: prefixed variable to be substituted when the
claim is present in RequestContext. These values originate from a
cryptographically verified identity token (the STS session JWT or
federated assertion), and the claim names are controlled by the trusted
identity provider, so the dynamic prefix is safe. Missing claims keep
the placeholder intact so the statement still fails to match.
Numeric JWT claims (JSON-decoded as float64) are now stringified so
patterns like ${jwt:project_id} work the same as string claims.
Fixes #9214
* fix(iam): cover all integer widths in claim stringification
Address PR review: stringifyClaimValue only handled int/int32/int64 on
the signed side and nothing on the unsigned side, so int8, int16, uint,
uint8, uint16, uint32, and uint64 claim values fell through to the
default branch and the placeholder was left unsubstituted.
JSON's generic decoder produces float64/json.Number for numbers, but
RequestContext can also be populated from typed sources (custom
providers or internal code), so cover all common integer widths -
signed and unsigned - explicitly. Extend TestStringifyClaimValue to
assert each supported type.
|
||
 
|
c6302fcb54 |
feat(iam): allow caller-supplied AccessKeyId and SecretAccessKey in CreateAccessKey (#9172)
* feat(iam): support caller-supplied AccessKeyId and SecretAccessKey in CreateAccessKey Both IAM implementations (standalone and embedded) now check for caller-supplied AccessKeyId and SecretAccessKey form parameters before generating random credentials. If provided, the caller-supplied values are used. If empty, random keys are generated as before. This enables programmatic identity provisioning where the caller needs to control the S3 credentials. Backward-compatible: no behavior change for callers that omit these parameters. * refactor(iam): extract shared caller-supplied credential validation Move the AccessKeyId/SecretAccessKey format checks and the in-memory collision scan into weed/iam so the standalone IAM API, the embedded IAM in s3api, and the admin dashboard all enforce the same rules. - ValidateCallerSuppliedAccessKeyId: 4-128 alphanumeric (rejects SigV4-breaking characters like '/' and '='). - ValidateCallerSuppliedSecretAccessKey: 8-128 chars. - FindAccessKeyOwner: scans identities and service accounts and returns the owning entity type + name for debug logging, without exposing the owner in caller-facing error messages. The admin dashboard previously only length-checked caller-supplied keys; it now enforces the same alphanumeric rule, which matches what SigV4 actually accepts anyway. * fix(iam): reject partial caller-supplied AccessKeyId/SecretAccessKey Previously, if a caller supplied only one of AccessKeyId or SecretAccessKey, CreateAccessKey logged a warning and auto-generated the missing half. That silently returns a credential the caller did not fully choose, which is surprising and easy to miss in a response they expected to echo back their input. Return ErrCodeInvalidInputException instead: either both are supplied or neither is. Updates the mixed-supply tests in weed/iamapi and weed/s3api to assert the rejection. * chore(iam): centralize and broaden sensitive form redaction DoActions and ExecuteAction both had an inline loop that redacted SecretAccessKey from their debug-level request log. Replace the two copies with iam.RedactSensitiveFormValues, backed by an explicit sensitive-keys set. The set now also covers Password, OldPassword, NewPassword, PrivateKey, and SessionToken. None of those parameters are used by today's IAM actions, but naming them here makes the log-safety guarantee survive future additions such as LoginProfile / STS. * test(iam): cover the upper length bound for CreateAccessKey TestCreateAccessKeyBoundary / TestEmbeddedIamCreateAccessKeyBoundary only exercised the 3/4-char lower edge. Add cases for 128 (accepted) and 129 (rejected) for AccessKeyId, plus 7 / 128 / 129-char cases for SecretAccessKey, so both ends of the validator are locked in at the handler level (the pure validators in weed/iam already cover this). * fix(s3api/iam): verify user existence before RNG and collision scan In the embedded IAM CreateAccessKey, the user lookup ran last: a request for a non-existent user still walked the whole identity / service-account list for collisions and, if no caller-supplied keys were present, generated fresh random credentials with crypto/rand before the NoSuchEntity error finally surfaced. Reorder: validate inputs, then find the target identity, then do the collision scan, then generate keys. A missing user now fails fast and consumes no entropy, and the handler returns NoSuchEntity instead of a misleading EntityAlreadyExists when both the user is missing and the supplied AccessKeyId happens to collide with another identity's key. Add TestEmbeddedIamCreateAccessKeyRejectsMissingUser to lock in the "no mutation on unknown user" guarantee. The standalone iamapi CreateAccessKey intentionally keeps its pre-existing "create-or-attach" semantics where a missing user is implicitly provisioned — that is a behavior change beyond the scope of this PR. * test(iam): tighten collision leak assertion and cover 8-char secret - Rename the collision-owner identity in TestCreateAccessKeyRejectsCollision (both iamapi and the embedded s3api test) from "existing" / "ExistingUser" to "ownerAlpha". The old assert.NotContains check was effectively a no-op because the error message never contained those substrings; a distinctive name shared with no part of the expected error body makes the leak guard actually meaningful if the wording ever drifts. The embedded test also adds a NotContains assertion that was previously missing entirely. - Add an explicit 8-char SecretAccessKey pass case to both boundary tests so the lower edge of the validator is locked in at the handler level alongside the 7 / 128 / 129-char cases. * fix(iamapi): enforce both-or-none before the collision lookup In the standalone IAM CreateAccessKey, FindAccessKeyOwner ran before the partial-credential check. If a caller supplied only AccessKeyId and it happened to collide with an existing key, the response was EntityAlreadyExists instead of the more fundamental InvalidInput for omitting SecretAccessKey — wrong error class, and leaked the fact that the probed key is already in use. Swap the order: validate both-or-none first, then do the collision scan. Matches the embedded IAM path and AWS behavior. Add a case to TestCreateAccessKeyRejectsPartialSupply that combines partial supply with a collision to lock in the ordering. * fix(admin): reject partial caller-supplied AccessKey/SecretKey The admin dashboard path silently generated the missing half when a caller supplied only one of AccessKey or SecretKey, while the IAM API and embedded IAM paths now reject this. Align the three: if exactly one is provided, return ErrInvalidInput. Also simplifies the generator block — either both are provided or neither is, so there is no mixed path to handle. * test(s3api/iam): guard dereferences in caller-supplied-keys test TestEmbeddedIamCreateAccessKeyWithCallerSuppliedKeys dereferenced *AccessKeyId/*SecretAccessKey/*UserName and indexed Identities[0].Credentials[0] without first verifying shape, so any future regression that returns a partial response or skips the config mutation would panic mid-assertion instead of failing with a clear message. Add require.NotNil on the response pointers and require.Len on the identities/credentials slices before the asserts. * test(iamapi): exercise the service-account branch of the collision check FindAccessKeyOwner scans both Identities[*].Credentials and ServiceAccounts[*].Credential, but TestCreateAccessKeyRejectsCollision only covered the identity branch. Split the test into two subtests — one per branch — so a future refactor that drops the service-account scan (or mutates the existing credential) trips a failure. Also asserts the existing service-account credential is not mutated and no credential is attached to the target identity on rejection. * test(iam): isolate 129-char secret subcase from prior credential In both TestCreateAccessKeyBoundary (iamapi) and TestEmbeddedIamCreateAccessKeyBoundary (s3api), the 129-char SecretAccessKey subcase reused the "validkey" AccessKeyId that the preceding 8-char subcase had just persisted into the config. The test still asserted the right outcome because the handler validates secret length before running the collision scan — but if the two checks ever swap, the subcase would pass (or fail) for the wrong reason. Reset the in-memory credentials before the 129-char subcase, matching the pattern already used by the 3/128/129-char AccessKeyId and 7-char secret subcases. No behavior change; purely test isolation. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris LuandGitHub
|
e21d7602c3 |
feat(iam): implement group inline policy actions (#8992)
* feat(iam): implement group inline policy actions Add PutGroupPolicy, GetGroupPolicy, DeleteGroupPolicy, and ListGroupPolicies to both embedded and standalone IAM servers. The standalone IAM stores group inline policies in a new GroupInlinePolicies field in the Policies JSON, mirroring the existing user inline policy pattern. DeleteGroup now also checks for inline policies before allowing deletion. * fix: address review feedback for group inline policies - Embedded IAM: return NotImplemented for group inline policies instead of silently succeeding as no-ops (Gemini + CodeRabbit) - Standalone IAM: recompute member actions after PutGroupPolicy and DeleteGroupPolicy (Gemini) - Add parameter validation for GroupName/PolicyName/PolicyDocument on PutGroupPolicy, DeleteGroupPolicy, ListGroupPolicies (Gemini) - Add UserName validation for ListUserPolicies in standalone IAM - Call cleanupGroupInlinePolicies from DeleteGroup (Gemini) - Migrate GroupInlinePolicies on group rename in UpdateGroup (CodeRabbit) - Fix integration test cleanup order (CodeRabbit) * fix: persist recomputed actions and improve error handling - Set changed=true for PutGroupPolicy/DeleteGroupPolicy in standalone IAM DoActions so recomputed member actions are persisted (Gemini critical) - Make cleanupGroupInlinePolicies accept policies parameter to avoid redundant I/O, return error (Gemini) - Make migrateGroupInlinePolicies return error, handle in caller (Gemini) * fix: include group policies in action recomputation Extend computeAllActionsForUser to also aggregate group inline policies and group managed policies when s3cfg is provided. Previously, group inline policies were stored but never reflected in member Identity.Actions. (CodeRabbit critical) * perf: use identity index in recomputeActionsForGroupMembers for O(N+M) * fix: skip group inline policy integration test on embedded IAM The embedded IAM returns NotImplemented for group inline policies. Skip TestIAMGroupInlinePolicy when running against embedded mode to avoid CI failures in the group integration test matrix. |
||
|
Chris LuandGitHub
|
45ee2ab4b9 |
feat(iam): implement ListUserPolicies API action (#8991)
* feat(iam): implement ListUserPolicies API action (#8987) Add ListUserPolicies support to both embedded and standalone IAM servers, resolving the NotImplemented error when calling `aws iam list-user-policies`. * fix: address review feedback for ListUserPolicies - Add handleImplicitUsername for ListUserPolicies in both IAM servers so omitting UserName defaults to the calling user (Gemini review) - Assert synthetic policy name in unit test (CodeRabbit) - Use require.True for error type assertion in integration test (CodeRabbit) |
||
|
Chris LuandGitHub
|
79a48256f5 |
fix(s3): populate s3:prefix from query param for ListObjects policy conditions (#8971)
* fix(s3): populate s3:prefix from query param for ListObjects policy conditions (#8969) ListObjectsV2/V1 requests with prefix-restricted STS session policies were denied because: 1. s3:prefix was derived from objectKey, which the auth middleware set to the prefix value, but the resource ARN then included the prefix (e.g. arn:aws:s3:::bucket/prefix) instead of staying at bucket level (arn:aws:s3:::bucket) as AWS requires for ListBucket. 2. When objectKey was empty (no middleware propagation), s3:prefix was never populated from the query parameter at all. Now AuthorizeAction extracts the prefix query parameter directly, sets it as s3:prefix in the request context, and uses a bucket-level resource ARN when the objectKey matches the propagated prefix. * fix(s3): use AWS-style wildcard matching for StringLike policy conditions filepath.Match treats * as not matching /, which breaks IAM StringLike conditions on paths (e.g. arn:aws:s3:::bucket/* won't match nested keys). Replace with a case-sensitive variant of AwsWildcardMatch that correctly treats * as matching any character including /. * refactor(s3): replace regex wildcard matching with string-based matcher Use the existing wildcard.MatchesWildcard utility instead of compiling and caching regexes for IAM wildcard matching. Removes the regexCache, its mutex, and the sync import. * refactor(s3): inline and remove AwsWildcardMatch wrapper functions Replace all call sites with direct wildcard.MatchesWildcard calls. * fix(s3): scope s3:prefix condition key to list operations only The s3:prefix logic was running for all actions, so a GetObject on "foo/bar" would wrongly populate s3:prefix. Restrict it to action "List" and always reset resourceObjectKey to "" so the resource ARN stays at bucket level. Also set s3:prefix to "" when no prefix is provided, so policies with StringEquals {"s3:prefix": ""} evaluate correctly. |
||
|
Chris LuandGitHub
|
b8fc99a9cd |
fix(s3): apply PutObject multipart expansion to STS session policies (#8932)
* fix(s3): apply PutObject multipart expansion to STS session policy evaluation (#8929) PR #8445 added logic to implicitly grant multipart upload actions when s3:PutObject is authorized, but only in the S3 API policy engine's CompiledStatement.MatchesAction(). STS session policies are evaluated through the IAM policy engine's matchesActions() -> awsIAMMatch() path, which did plain pattern matching without the multipart expansion. Add the same multipart expansion logic to the IAM policy engine's matchesActions() so that session policies containing s3:PutObject correctly allow multipart upload operations. * fix: make multipart action set lookup case-insensitive and optimize Address PR review feedback: - Lowercase multipartActionSet keys and use strings.ToLower for lookup, since AWS IAM actions are case-insensitive - Only check for s3:PutObject permission when the requested action is actually a multipart action, avoiding unnecessary awsIAMMatch calls - Add test case for case-insensitive multipart action matching |
||
|
Chris LuandGitHub
|
995dfc4d5d |
chore: remove ~50k lines of unreachable dead code (#8913)
* chore: remove unreachable dead code across the codebase Remove ~50,000 lines of unreachable code identified by static analysis. Major removals: - weed/filer/redis_lua: entire unused Redis Lua filer store implementation - weed/wdclient/net2, resource_pool: unused connection/resource pool packages - weed/plugin/worker/lifecycle: unused lifecycle plugin worker - weed/s3api: unused S3 policy templates, presigned URL IAM, streaming copy, multipart IAM, key rotation, and various SSE helper functions - weed/mq/kafka: unused partition mapping, compression, schema, and protocol functions - weed/mq/offset: unused SQL storage and migration code - weed/worker: unused registry, task, and monitoring functions - weed/query: unused SQL engine, parquet scanner, and type functions - weed/shell: unused EC proportional rebalance functions - weed/storage/erasure_coding/distribution: unused distribution analysis functions - Individual unreachable functions removed from 150+ files across admin, credential, filer, iam, kms, mount, mq, operation, pb, s3api, server, shell, storage, topology, and util packages * fix(s3): reset shared memory store in IAM test to prevent flaky failure TestLoadIAMManagerFromConfig_EmptyConfigWithFallbackKey was flaky because the MemoryStore credential backend is a singleton registered via init(). Earlier tests that create anonymous identities pollute the shared store, causing LookupAnonymous() to unexpectedly return true. Fix by calling Reset() on the memory store before the test runs. * style: run gofmt on changed files * fix: restore KMS functions used by integration tests * fix(plugin): prevent panic on send to closed worker session channel The Plugin.sendToWorker method could panic with "send on closed channel" when a worker disconnected while a message was being sent. The race was between streamSession.close() closing the outgoing channel and sendToWorker writing to it concurrently. Add a done channel to streamSession that is closed before the outgoing channel, and check it in sendToWorker's select to safely detect closed sessions without panicking. |
||
|
Chris LuandGitHub
|
059bee683f |
feat(s3): add STS GetFederationToken support (#8891)
* feat(s3): add STS GetFederationToken support Implement the AWS STS GetFederationToken API, which allows long-term IAM users to obtain temporary credentials scoped down by an optional inline session policy. This is useful for server-side applications that mint per-user temporary credentials. Key behaviors: - Requires SigV4 authentication from a long-term IAM user - Rejects calls from temporary credentials (session tokens) - Name parameter (2-64 chars) identifies the federated user - DurationSeconds supports 900-129600 (15 min to 36 hours, default 12h) - Optional inline session policy for permission scoping - Caller's attached policies are embedded in the JWT token - Returns federated user ARN: arn:aws:sts::<account>:federated-user/<Name> No performance impact on the S3 hot path — credential vending is a separate control-plane operation, and all policy data is embedded in the stateless JWT token. * fix(s3): address GetFederationToken PR review feedback - Fix Name validation: max 32 chars (not 64) per AWS spec, add regex validation for [\w+=,.@-]+ character whitelist - Refactor parseDurationSeconds into parseDurationSecondsWithBounds to eliminate duplicated duration parsing logic - Add sts:GetFederationToken permission check via VerifyActionPermission mirroring the AssumeRole authorization pattern - Change GetPoliciesForUser to return ([]string, error) so callers fail closed on policy-resolution failures instead of silently returning nil - Move temporary-credentials rejection before SigV4 verification for early rejection and proper test coverage - Update tests: verify specific error message for temp cred rejection, add regex validation test cases (spaces, slashes rejected) * refactor(s3): use sts.Action* constants instead of hard-coded strings Replace hard-coded "sts:AssumeRole" and "sts:GetFederationToken" strings in VerifyActionPermission calls with sts.ActionAssumeRole and sts.ActionGetFederationToken package constants. * fix(s3): pass through sts: prefix in action resolver and merge policies Two fixes: 1. mapBaseActionToS3Format now passes through "sts:" prefix alongside "s3:" and "iam:", preventing sts:GetFederationToken from being rewritten to s3:sts:GetFederationToken in VerifyActionPermission. This also fixes the existing sts:AssumeRole permission checks. 2. GetFederationToken policy embedding now merges identity.PolicyNames (from SigV4 identity) with policies from the IAM manager (which may include group-attached policies), deduplicated via a map. Previously the IAM manager lookup was skipped when identity.PolicyNames was non-empty, causing group policies to be omitted from the token. * test(s3): add integration tests for sts: action passthrough and policy merge Action resolver tests: - TestMapBaseActionToS3Format_ServicePrefixPassthrough: verifies s3:, iam:, and sts: prefixed actions pass through unchanged while coarse actions (Read, Write) are mapped to S3 format - TestResolveS3Action_STSActionsPassthrough: verifies sts:AssumeRole, sts:GetFederationToken, sts:GetCallerIdentity pass through ResolveS3Action unchanged with both nil and real HTTP requests Policy merge tests: - TestGetFederationToken_GetPoliciesForUser: tests IAMManager.GetPoliciesForUser with no user store (error), missing user, user with policies, user without - TestGetFederationToken_PolicyMergeAndDedup: tests that identity.PolicyNames and IAM-manager-resolved policies are merged and deduplicated (SharedPolicy appears in both sources, result has 3 unique policies) - TestGetFederationToken_PolicyMergeNoManager: tests that when IAM manager is unavailable, identity.PolicyNames alone are embedded * test(s3): add end-to-end integration tests for GetFederationToken Add integration tests that call GetFederationToken using real AWS SigV4 signed HTTP requests against a running SeaweedFS instance, following the existing pattern in test/s3/iam/s3_sts_assume_role_test.go. Tests: - TestSTSGetFederationTokenValidation: missing name, name too short/long, invalid characters, duration too short/long, malformed policy, anonymous rejection (7 subtests) - TestSTSGetFederationTokenRejectTemporaryCredentials: obtains temp creds via AssumeRole then verifies GetFederationToken rejects them - TestSTSGetFederationTokenSuccess: basic success, custom 1h duration, 36h max duration with expiration time verification - TestSTSGetFederationTokenWithSessionPolicy: creates a bucket, obtains federated creds with GetObject-only session policy, verifies GetObject succeeds and PutObject is denied using the AWS SDK S3 client |
||
 
|
8cde3d4486 |
Add data file compaction to iceberg maintenance (Phase 2) (#8503)
* Add iceberg_maintenance plugin worker handler (Phase 1) Implement automated Iceberg table maintenance as a new plugin worker job type. The handler scans S3 table buckets for tables needing maintenance and executes operations in the correct Iceberg order: expire snapshots, remove orphan files, and rewrite manifests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add data file compaction to iceberg maintenance handler (Phase 2) Implement bin-packing compaction for small Parquet data files: - Enumerate data files from manifests, group by partition - Merge small files using parquet-go (read rows, write merged output) - Create new manifest with ADDED/DELETED/EXISTING entries - Commit new snapshot with compaction metadata Add 'compact' operation to maintenance order (runs before expire_snapshots), configurable via target_file_size_bytes and min_input_files thresholds. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix memory exhaustion in mergeParquetFiles by processing files sequentially Previously all source Parquet files were loaded into memory simultaneously, risking OOM when a compaction bin contained many small files. Now each file is loaded, its rows are streamed into the output writer, and its data is released before the next file is loaded — keeping peak memory proportional to one input file plus the output buffer. * Validate bucket/namespace/table names against path traversal Reject names containing '..', '/', or '\' in Execute to prevent directory traversal via crafted job parameters. * Add filer address failover in iceberg maintenance handler Try each filer address from cluster context in order instead of only using the first one. This improves resilience when the primary filer is temporarily unreachable. * Add separate MinManifestsToRewrite config for manifest rewrite threshold The rewrite_manifests operation was reusing MinInputFiles (meant for compaction bin file counts) as its manifest count threshold. Add a dedicated MinManifestsToRewrite field with its own config UI section and default value (5) so the two thresholds can be tuned independently. * Fix risky mtime fallback in orphan removal that could delete new files When entry.Attributes is nil, mtime defaulted to Unix epoch (1970), which would always be older than the safety threshold, causing the file to be treated as eligible for deletion. Skip entries with nil Attributes instead, matching the safer logic in operations.go. * Fix undefined function references in iceberg_maintenance_handler.go Use the exported function names (ShouldSkipDetectionByInterval, BuildDetectorActivity, BuildExecutorActivity) matching their definitions in vacuum_handler.go. * Remove duplicated iceberg maintenance handler in favor of iceberg/ subpackage The IcebergMaintenanceHandler and its compaction code in the parent pluginworker package duplicated the logic already present in the iceberg/ subpackage (which self-registers via init()). The old code lacked stale-plan guards, proper path normalization, CAS-based xattr updates, and error-returning parseOperations. Since the registry pattern (default "all") makes the old handler unreachable, remove it entirely. All functionality is provided by iceberg.Handler with the reviewed improvements. * Fix MinManifestsToRewrite clamping to match UI minimum of 2 The clamp reset values below 2 to the default of 5, contradicting the UI's advertised MinValue of 2. Clamp to 2 instead. * Sort entries by size descending in splitOversizedBin for better packing Entries were processed in insertion order which is non-deterministic from map iteration. Sorting largest-first before the splitting loop improves bin packing efficiency by filling bins more evenly. * Add context cancellation check to drainReader loop The row-streaming loop in drainReader did not check ctx between iterations, making long compaction merges uncancellable. Check ctx.Done() at the top of each iteration. * Fix splitOversizedBin to always respect targetSize limit The minFiles check in the split condition allowed bins to grow past targetSize when they had fewer than minFiles entries, defeating the OOM protection. Now bins always split at targetSize, and a trailing runt with fewer than minFiles entries is merged into the previous bin. * Add integration tests for iceberg table maintenance plugin worker Tests start a real weed mini cluster, create S3 buckets and Iceberg table metadata via filer gRPC, then exercise the iceberg.Handler operations (ExpireSnapshots, RemoveOrphans, RewriteManifests) against the live filer. A full maintenance cycle test runs all operations in sequence and verifies metadata consistency. Also adds exported method wrappers (testing_api.go) so the integration test package can call the unexported handler methods. * Fix splitOversizedBin dropping files and add source path to drainReader errors The runt-merge step could leave leading bins with fewer than minFiles entries (e.g. [80,80,10,10] with targetSize=100, minFiles=2 would drop the first 80-byte file). Replace the filter-based approach with an iterative merge that folds any sub-minFiles bin into its smallest neighbor, preserving all eligible files. Also add the source file path to drainReader error messages so callers can identify which Parquet file caused a read/write failure. * Harden integration test error handling - s3put: fail immediately on HTTP 4xx/5xx instead of logging and continuing - lookupEntry: distinguish NotFound (return nil) from unexpected RPC errors (fail the test) - writeOrphan and orphan creation in FullMaintenanceCycle: check CreateEntryResponse.Error in addition to the RPC error * go fmt --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Chris LuandGitHub
|
f950a941e3 |
Fix trust policy validation for specific AWS user principals (#8597)
* Add tests for AWS user principal in AssumeRole trust policies Add test cases that verify trust policy validation when using specific AWS user principals (e.g., "arn:aws:iam::000000000000:user/backend") in the Principal field of trust policies for AssumeRole. Covers single user, multiple users (array), wildcard, and plain string principal formats. These tests demonstrate the bug reported in #8588 where specific user principals always fail validation. * Populate RequestContext in ValidateTrustPolicyForPrincipal ValidateTrustPolicyForPrincipal was creating an EvaluationContext with a nil RequestContext. The policy engine's principal matching logic looks up "aws:PrincipalArn" in RequestContext for non-wildcard principals, so specific user ARNs like "arn:aws:iam::000000000000:user/backend" always failed to match, while wildcard "*" worked because it short-circuits before the lookup. Populate RequestContext with both "principal" and "aws:PrincipalArn" keys, consistent with how IsActionAllowed already does it. Fixes #8588 * Remove GitHub discussion URL from source code comments * Add specific error message assertions in trust policy tests |
||
|
Chris LuandGitHub
|
992db11d2b |
iam: add IAM group management (#8560)
* iam: add Group message to protobuf schema Add Group message (name, members, policy_names, disabled) and add groups field to S3ApiConfiguration for IAM group management support (issue #7742). * iam: add group CRUD to CredentialStore interface and all backends Add group management methods (CreateGroup, GetGroup, DeleteGroup, ListGroups, UpdateGroup) to the CredentialStore interface with implementations for memory, filer_etc, postgres, and grpc stores. Wire group loading/saving into filer_etc LoadConfiguration and SaveConfiguration. * iam: add group IAM response types Add XML response types for group management IAM actions: CreateGroup, DeleteGroup, GetGroup, ListGroups, AddUserToGroup, RemoveUserFromGroup, AttachGroupPolicy, DetachGroupPolicy, ListAttachedGroupPolicies, ListGroupsForUser. * iam: add group management handlers to embedded IAM API Add CreateGroup, DeleteGroup, GetGroup, ListGroups, AddUserToGroup, RemoveUserFromGroup, AttachGroupPolicy, DetachGroupPolicy, ListAttachedGroupPolicies, and ListGroupsForUser handlers with dispatch in ExecuteAction. * iam: add group management handlers to standalone IAM API Add group handlers (CreateGroup, DeleteGroup, GetGroup, ListGroups, AddUserToGroup, RemoveUserFromGroup, AttachGroupPolicy, DetachGroupPolicy, ListAttachedGroupPolicies, ListGroupsForUser) and wire into DoActions dispatch. Also add helper functions for user/policy side effects. * iam: integrate group policies into authorization Add groups and userGroups reverse index to IdentityAccessManagement. Populate both maps during ReplaceS3ApiConfiguration and MergeS3ApiConfiguration. Modify evaluateIAMPolicies to evaluate policies from user's enabled groups in addition to user policies. Update VerifyActionPermission to consider group policies when checking hasAttachedPolicies. * iam: add group side effects on user deletion and rename When a user is deleted, remove them from all groups they belong to. When a user is renamed, update group membership references. Applied to both embedded and standalone IAM handlers. * iam: watch /etc/iam/groups directory for config changes Add groups directory to the filer subscription watcher so group file changes trigger IAM configuration reloads. * admin: add group management page to admin UI Add groups page with CRUD operations, member management, policy attachment, and enable/disable toggle. Register routes in admin handlers and add Groups entry to sidebar navigation. * test: add IAM group management integration tests Add comprehensive integration tests for group CRUD, membership, policy attachment, policy enforcement, disabled group behavior, user deletion side effects, and multi-group membership. Add "group" test type to CI matrix in s3-iam-tests workflow. * iam: address PR review comments for group management - Fix XSS vulnerability in groups.templ: replace innerHTML string concatenation with DOM APIs (createElement/textContent) for rendering member and policy lists - Use userGroups reverse index in embedded IAM ListGroupsForUser for O(1) lookup instead of iterating all groups - Add buildUserGroupsIndex helper in standalone IAM handlers; use it in ListGroupsForUser and removeUserFromAllGroups for efficient lookup - Add note about gRPC store load-modify-save race condition limitation * iam: add defensive copies, validation, and XSS fixes for group management - Memory store: clone groups on store/retrieve to prevent mutation - Admin dash: deep copy groups before mutation, validate user/policy exists - HTTP handlers: translate credential errors to proper HTTP status codes, use *bool for Enabled field to distinguish missing vs false - Groups templ: use data attributes + event delegation instead of inline onclick for XSS safety, prevent stale async responses * iam: add explicit group methods to PropagatingCredentialStore Add CreateGroup, GetGroup, DeleteGroup, ListGroups, and UpdateGroup methods instead of relying on embedded interface fallthrough. Group changes propagate via filer subscription so no RPC propagation needed. * iam: detect postgres unique constraint violation and add groups index Return ErrGroupAlreadyExists when INSERT hits SQLState 23505 instead of a generic error. Add index on groups(disabled) for filtered queries. * iam: add Marker field to group list response types Add Marker string field to GetGroupResult, ListGroupsResult, ListAttachedGroupPoliciesResult, and ListGroupsForUserResult to match AWS IAM pagination response format. * iam: check group attachment before policy deletion Reject DeletePolicy if the policy is attached to any group, matching AWS IAM behavior. Add PolicyArn to ListAttachedGroupPolicies response. * iam: include group policies in IAM authorization Merge policy names from user's enabled groups into the IAMIdentity used for authorization, so group-attached policies are evaluated alongside user-attached policies. * iam: check for name collision before renaming user in UpdateUser Scan identities and inline policies for newUserName before mutating, returning EntityAlreadyExists if a collision is found. Reuse the already-loaded policies instead of loading them again inside the loop. * test: use t.Cleanup for bucket cleanup in group policy test * iam: wrap ErrUserNotInGroup sentinel in RemoveGroupMember error Wrap credential.ErrUserNotInGroup so errors.Is works in groupErrorToHTTPStatus, returning proper 400 instead of 500. * admin: regenerate groups_templ.go with XSS-safe data attributes Regenerated from groups.templ which uses data-group-name attributes instead of inline onclick with string interpolation. * iam: add input validation and persist groups during migration - Validate nil/empty group name in CreateGroup and UpdateGroup - Save groups in migrateToMultiFile so they survive legacy migration * admin: use groupErrorToHTTPStatus in GetGroupMembers and GetGroupPolicies * iam: short-circuit UpdateUser when newUserName equals current name * iam: require empty PolicyNames before group deletion Reject DeleteGroup when group has attached policies, matching the existing members check. Also fix GetGroup error handling in DeletePolicy to only skip ErrGroupNotFound, not all errors. * ci: add weed/pb/** to S3 IAM test trigger paths * test: replace time.Sleep with require.Eventually for propagation waits Use polling with timeout instead of fixed sleeps to reduce flakiness in integration tests waiting for IAM policy propagation. * fix: use credentialManager.GetPolicy for AttachGroupPolicy validation Policies created via CreatePolicy through credentialManager are stored in the credential store, not in s3cfg.Policies (which only has static config policies). Change AttachGroupPolicy to use credentialManager.GetPolicy() for policy existence validation. * feat: add UpdateGroup handler to embedded IAM API Add UpdateGroup action to enable/disable groups and rename groups via the IAM API. This is a SeaweedFS extension (not in AWS SDK) used by tests to toggle group disabled status. * fix: authenticate raw IAM API calls in group tests The embedded IAM endpoint rejects anonymous requests. Replace callIAMAPI with callIAMAPIAuthenticated that uses JWT bearer token authentication via the test framework. * feat: add UpdateGroup handler to standalone IAM API Mirror the embedded IAM UpdateGroup handler in the standalone IAM API for parity. * fix: add omitempty to Marker XML tags in group responses Non-truncated responses should not emit an empty <Marker/> element. * fix: distinguish backend errors from missing policies in AttachGroupPolicy Return ServiceFailure for credential manager errors instead of masking them as NoSuchEntity. Also switch ListGroupsForUser to use s3cfg.Groups instead of in-memory reverse index to avoid stale data. Add duplicate name check to UpdateGroup rename. * fix: standalone IAM AttachGroupPolicy uses persisted policy store Check managed policies from GetPolicies() instead of s3cfg.Policies so dynamically created policies are found. Also add duplicate name check to UpdateGroup rename. * fix: rollback inline policies on UpdateUser PutPolicies failure If PutPolicies fails after moving inline policies to the new username, restore both the identity name and the inline policies map to their original state to avoid a partial-write window. * fix: correct test cleanup ordering for group tests Replace scattered defers with single ordered t.Cleanup in each test to ensure resources are torn down in reverse-creation order: remove membership, detach policies, delete access keys, delete users, delete groups, delete policies. Move bucket cleanup to parent test scope and delete objects before bucket. * fix: move identity nil check before map lookup and refine hasAttachedPolicies Move the nil check on identity before accessing identity.Name to prevent panic. Also refine hasAttachedPolicies to only consider groups that are enabled and have actual policies attached, so membership in a no-policy group doesn't incorrectly trigger IAM authorization. * fix: fail group reload on unreadable or corrupt group files Return errors instead of logging and continuing when group files cannot be read or unmarshaled. This prevents silently applying a partial IAM config with missing group memberships or policies. * fix: use errors.Is for sql.ErrNoRows comparison in postgres group store * docs: explain why group methods skip propagateChange Group changes propagate to S3 servers via filer subscription (watching /etc/iam/groups/) rather than gRPC RPCs, since there are no group-specific RPCs in the S3 cache protocol. * fix: remove unused policyNameFromArn and strings import * fix: update service account ParentUser on user rename When renaming a user via UpdateUser, also update ParentUser references in service accounts to prevent them from becoming orphaned after the next configuration reload. * fix: wrap DetachGroupPolicy error with ErrPolicyNotAttached sentinel Use credential.ErrPolicyNotAttached so groupErrorToHTTPStatus maps it to 400 instead of falling back to 500. * fix: use admin S3 client for bucket cleanup in enforcement test The user S3 client may lack permissions by cleanup time since the user is removed from the group in an earlier subtest. Use the admin S3 client to ensure bucket and object cleanup always succeeds. * fix: add nil guard for group param in propagating store log calls Prevent potential nil dereference when logging group.Name in CreateGroup and UpdateGroup of PropagatingCredentialStore. * fix: validate Disabled field in UpdateGroup handlers Reject values other than "true" or "false" with InvalidInputException instead of silently treating them as false. * fix: seed mergedGroups from existing groups in MergeS3ApiConfiguration Previously the merge started with empty group maps, dropping any static-file groups. Now seeds from existing iam.groups before overlaying dynamic config, and builds the reverse index after merging to avoid stale entries from overridden groups. * fix: use errors.Is for filer_pb.ErrNotFound comparison in group loading Replace direct equality (==) with errors.Is() to correctly match wrapped errors, consistent with the rest of the codebase. * fix: add ErrUserNotFound and ErrPolicyNotFound to groupErrorToHTTPStatus Map these sentinel errors to 404 so AddGroupMember and AttachGroupPolicy return proper HTTP status codes. * fix: log cleanup errors in group integration tests Replace fire-and-forget cleanup calls with error-checked versions that log failures via t.Logf for debugging visibility. * fix: prevent duplicate group test runs in CI matrix The basic lane's -run "TestIAM" regex also matched TestIAMGroup* tests, causing them to run in both the basic and group lanes. Replace with explicit test function names. * fix: add GIN index on groups.members JSONB for membership lookups Without this index, ListGroupsForUser and membership queries require full table scans on the groups table. * fix: handle cross-directory moves in IAM config subscription When a file is moved out of an IAM directory (e.g., /etc/iam/groups), the dir variable was overwritten with NewParentPath, causing the source directory change to be missed. Now also notifies handlers about the source directory for cross-directory moves. * fix: validate members/policies before deleting group in admin handler AdminServer.DeleteGroup now checks for attached members and policies before delegating to credentialManager, matching the IAM handler guards. * fix: merge groups by name instead of blind append during filer load Match the identity loader's merge behavior: find existing group by name and replace, only append when no match exists. Prevents duplicates when legacy and multi-file configs overlap. * fix: check DeleteEntry response error when cleaning obsolete group files Capture and log resp.Error from filer DeleteEntry calls during group file cleanup, matching the pattern used in deleteGroupFile. * fix: verify source user exists before no-op check in UpdateUser Reorder UpdateUser to find the source identity first and return NoSuchEntityException if not found, before checking if the rename is a no-op. Previously a non-existent user renamed to itself would incorrectly return success. * fix: update service account parent refs on user rename in embedded IAM The embedded IAM UpdateUser handler updated group membership but not service account ParentUser fields, unlike the standalone handler. * fix: replay source-side events for all handlers on cross-dir moves Pass nil newEntry to bucket, IAM, and circuit-breaker handlers for the source directory during cross-directory moves, so all watchers can clear caches for the moved-away resource. * fix: don't seed mergedGroups from existing iam.groups in merge Groups are always dynamic (from filer), never static (from s3.config). Seeding from iam.groups caused stale deleted groups to persist. Now only uses config.Groups from the dynamic filer config. * fix: add deferred user cleanup in TestIAMGroupUserDeletionSideEffect Register t.Cleanup for the created user so it gets cleaned up even if the test fails before the inline DeleteUser call. * fix: assert UpdateGroup HTTP status in disabled group tests Add require.Equal checks for 200 status after UpdateGroup calls so the test fails immediately on API errors rather than relying on the subsequent Eventually timeout. * fix: trim whitespace from group name in filer store operations Trim leading/trailing whitespace from group.Name before validation in CreateGroup and UpdateGroup to prevent whitespace-only filenames. Also merge groups by name during multi-file load to prevent duplicates. * fix: add nil/empty group validation in gRPC store Guard CreateGroup and UpdateGroup against nil group or empty name to prevent panics and invalid persistence. * fix: add nil/empty group validation in postgres store Guard CreateGroup and UpdateGroup against nil group or empty name to prevent panics from nil member access and empty-name row inserts. * fix: add name collision check in embedded IAM UpdateUser The embedded IAM handler renamed users without checking if the target name already existed, unlike the standalone handler. * fix: add ErrGroupNotEmpty sentinel and map to HTTP 409 AdminServer.DeleteGroup now wraps conflict errors with ErrGroupNotEmpty, and groupErrorToHTTPStatus maps it to 409 Conflict instead of 500. * fix: use appropriate error message in GetGroupDetails based on status Return "Group not found" only for 404, use "Failed to retrieve group" for other error statuses instead of always saying "Group not found". * fix: use backend-normalized group.Name in CreateGroup response After credentialManager.CreateGroup may normalize the name (e.g., trim whitespace), use group.Name instead of the raw input for the returned GroupData to ensure consistency. * fix: add nil/empty group validation in memory store Guard CreateGroup and UpdateGroup against nil group or empty name to prevent panics from nil pointer dereference on map access. * fix: reorder embedded IAM UpdateUser to verify source first Find the source identity before checking for collisions, matching the standalone handler's logic. Previously a non-existent user renamed to an existing name would get EntityAlreadyExists instead of NoSuchEntity. * fix: handle same-directory renames in metadata subscription Replay a delete event for the old entry name during same-directory renames so handlers like onBucketMetadataChange can clean up stale state for the old name. * fix: abort GetGroups on non-ErrGroupNotFound errors Only skip groups that return ErrGroupNotFound. Other errors (e.g., transient backend failures) now abort the handler and return the error to the caller instead of silently producing partial results. * fix: add aria-label and title to icon-only group action buttons Add accessible labels to View and Delete buttons so screen readers and tooltips provide meaningful context. * fix: validate group name in saveGroup to prevent invalid filenames Trim whitespace and reject empty names before writing group JSON files, preventing creation of files like ".json". * fix: add /etc/iam/groups to filer subscription watched directories The groups directory was missing from the watched directories list, so S3 servers in a cluster would not detect group changes made by other servers via filer. The onIamConfigChange handler already had code to handle group directory changes but it was never triggered. * add direct gRPC propagation for group changes to S3 servers Groups now have the same dual propagation as identities and policies: direct gRPC push via propagateChange + async filer subscription. - Add PutGroup/RemoveGroup proto messages and RPCs - Add PutGroup/RemoveGroup in-memory cache methods on IAM - Add PutGroup/RemoveGroup gRPC server handlers - Update PropagatingCredentialStore to call propagateChange on group mutations * reduce log verbosity for config load summary Change ReplaceS3ApiConfiguration log from Infof to V(1).Infof to avoid noisy output on every config reload. * admin: show user groups in view and edit user modals - Add Groups field to UserDetails and populate from credential manager - Show groups as badges in user details view modal - Add group management to edit user modal: display current groups, add to group via dropdown, remove from group via badge x button * fix: remove duplicate showAlert that broke modal-alerts.js admin.js defined showAlert(type, message) which overwrote the modal-alerts.js version showAlert(message, type), causing broken unstyled alert boxes. Remove the duplicate and swap all callers in admin.js to use the correct (message, type) argument order. * fix: unwrap groups API response in edit user modal The /api/groups endpoint returns {"groups": [...]}, not a bare array. * Update object_store_users_templ.go * test: assert AccessDenied error code in group denial tests Replace plain assert.Error checks with awserr.Error type assertion and AccessDenied code verification, matching the pattern used in other IAM integration tests. * fix: propagate GetGroups errors in ShowGroups handler getGroupsPageData was swallowing errors and returning an empty page with 200 status. Now returns the error so ShowGroups can respond with a proper error status. * fix: reject AttachGroupPolicy when credential manager is nil Previously skipped policy existence validation when credentialManager was nil, allowing attachment of nonexistent policies. Now returns a ServiceFailureException error. * fix: preserve groups during partial MergeS3ApiConfiguration updates UpsertIdentity calls MergeS3ApiConfiguration with a partial config containing only the updated identity (nil Groups). This was wiping all in-memory group state. Now only replaces groups when config.Groups is non-nil (full config reload). * fix: propagate errors from group lookup in GetObjectStoreUserDetails ListGroups and GetGroup errors were silently ignored, potentially showing incomplete group data in the UI. * fix: use DOM APIs for group badge remove button to prevent XSS Replace innerHTML with onclick string interpolation with DOM createElement + addEventListener pattern. Also add aria-label and title to the add-to-group button. * fix: snapshot group policies under RLock to prevent concurrent map access evaluateIAMPolicies was copying the map reference via groupMap := iam.groups under RLock then iterating after RUnlock, while PutGroup mutates the map in-place. Now copies the needed policy names into a slice while holding the lock. * fix: add nil IAM check to PutGroup and RemoveGroup gRPC handlers Match the nil guard pattern used by PutPolicy/DeletePolicy to prevent nil pointer dereference when IAM is not initialized. |
||
|
Chris LuandGitHub
|
540fc97e00 |
s3/iam: reuse one request id per request (#8538)
* request_id: add shared request middleware
* s3err: preserve request ids in responses and logs
* iam: reuse request ids in XML responses
* sts: reuse request ids in XML responses
* request_id: drop legacy header fallback
* request_id: use AWS-style request id format
* iam: fix AWS-compatible XML format for ErrorResponse and field ordering
- ErrorResponse uses bare <RequestId> at root level instead of
<ResponseMetadata> wrapper, matching the AWS IAM error response spec
- Move CommonResponse to last field in success response structs so
<ResponseMetadata> serializes after result elements
- Add randomness to request ID generation to avoid collisions
- Add tests for XML ordering and ErrorResponse format
* iam: remove duplicate error_response_test.go
Test is already covered by responses_test.go.
* address PR review comments
- Guard against typed nil pointers in SetResponseRequestID before
interface assertion (CodeRabbit)
- Use regexp instead of strings.Index in test helpers for extracting
request IDs (Gemini)
* request_id: prevent spoofing, fix nil-error branch, thread reqID to error writers
- Ensure() now always generates a server-side ID, ignoring client-sent
x-amz-request-id headers to prevent request ID spoofing. Uses a
private context key (contextKey{}) instead of the header string.
- writeIamErrorResponse in both iamapi and embedded IAM now accepts
reqID as a parameter instead of calling Ensure() internally, ensuring
a single request ID per request lifecycle.
- The nil-iamError branch in writeIamErrorResponse now writes a 500
Internal Server Error response instead of returning silently.
- Updated tests to set request IDs via context (not headers) and added
tests for spoofing prevention and context reuse.
* sts: add request-id consistency assertions to ActionInBody tests
* test: update admin test to expect server-generated request IDs
The test previously sent a client x-amz-request-id header and expected
it echoed back. Since Ensure() now ignores client headers to prevent
spoofing, update the test to verify the server returns a non-empty
server-generated request ID instead.
* iam: add generic WithRequestID helper alongside reflection-based fallback
Add WithRequestID[T] that uses generics to take the address of a value
type, satisfying the pointer receiver on SetRequestId without reflection.
The existing SetResponseRequestID is kept for the two call sites that
operate on interface{} (from large action switches where the concrete
type varies at runtime). Generics cannot replace reflection there since
Go cannot infer type parameters from interface{}.
* Remove reflection and generics from request ID setting
Call SetRequestId directly on concrete response types in each switch
branch before boxing into interface{}, eliminating the need for
WithRequestID (generics) and SetResponseRequestID (reflection).
* iam: return pointer responses in action dispatch
* Fix IAM error handling consistency and ensure request IDs on all responses
- UpdateUser/CreatePolicy error branches: use writeIamErrorResponse instead
of s3err.WriteErrorResponse to preserve IAM formatting and request ID
- ExecuteAction: accept reqID parameter and generate one if empty, ensuring
every response carries a RequestId regardless of caller
* Clean up inline policies on DeleteUser and UpdateUser rename
DeleteUser: remove InlinePolicies[userName] from policy storage before
removing the identity, so policies are not orphaned.
UpdateUser: move InlinePolicies[userName] to InlinePolicies[newUserName]
when renaming, so GetUserPolicy/DeleteUserPolicy work under the new name.
Both operations persist the updated policies and return an error if
the storage write fails, preventing partial state.
|
||
 
|
14cd0f53ba |
Places the CommonResponse struct at the *end* of all IAM responses. (#8537)
* Places the CommonResponse struct at the end of all IAM responses, rather than the start. * iam: fix error response request id layout * iam: add XML ordering regression test * iam: share request id generation --------- Co-authored-by: Aaron Segal <aaron.segal@rpsolutions.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris LuandGitHub
|
f9311a3422 |
s3api: fix static IAM policy enforcement after reload (#8532)
* s3api: honor attached IAM policies over legacy actions * s3api: hydrate IAM policy docs during config reload * s3api: use policy-aware auth when listing buckets * credential: propagate context through filer_etc policy reads * credential: make legacy policy deletes durable * s3api: exercise managed policy runtime loader * s3api: allow static IAM users without session tokens * iam: deny unmatched attached policies under default allow * iam: load embedded policy files from filer store * s3api: require session tokens for IAM presigning * s3api: sync runtime policies into zero-config IAM * credential: respect context in policy file loads * credential: serialize legacy policy deletes * iam: align filer policy store naming * s3api: use authenticated principals for presigning * iam: deep copy policy conditions * s3api: require request creation in policy tests * filer: keep ReadInsideFiler as the context-aware API * iam: harden filer policy store writes * credential: strengthen legacy policy serialization test * credential: forward runtime policy loaders through wrapper * s3api: harden runtime policy merging * iam: require typed already-exists errors |
||
|
Chris LuandGitHub
|
2d65d7f499 |
Embed role policies in AssumeRole STS tokens (#8421)
* Embed role policies in AssumeRole STS tokens * Log STS policy lookup failures * Use IAMManager provider * Guard policy embedding role lookup |
||
|
Chris Lu
|
e4b70c2521 | go fix | ||
|
Chris Lu
|
f7c27cc81f | go fmt | ||
|
Chris LuandGitHub
|
bd0b1fe9d5 |
S3 IAM: Added ListPolicyVersions and GetPolicyVersion support (#8395)
* test(s3/iam): add managed policy CRUD lifecycle integration coverage * s3/iam: add ListPolicyVersions and GetPolicyVersion support * test(s3/iam): cover ListPolicyVersions and GetPolicyVersion |
||
|
Chris LuandGitHub
|
e9c45144cf |
Implement managed policy storage (#8385)
* Persist managed IAM policies * Add IAM list/get policy integration test * Faster marker lookup and cleanup * Handle delete conflict and improve listing * Add delete-in-use policy integration test * Stabilize policy ID and guard path prefix * Tighten CreatePolicy guard and reload * Add ListPolicyNames to credential store |
||
|
Chris LuandGitHub
|
7b8df39cf7 |
s3api: add AttachUserPolicy/DetachUserPolicy/ListAttachedUserPolicies (#8379)
* iam: add XML responses for managed user policy APIs * s3api: implement attach/detach/list attached user policies * s3api: add embedded IAM tests for managed user policies * iam: update CredentialStore interface and Manager for managed policies Updated the `CredentialStore` interface to include `AttachUserPolicy`, `DetachUserPolicy`, and `ListAttachedUserPolicies` methods. The `CredentialManager` was updated to delegate these calls to the store. Added common error variables for policy management. * iam: implement managed policy methods in MemoryStore Implemented `AttachUserPolicy`, `DetachUserPolicy`, and `ListAttachedUserPolicies` in the MemoryStore. Also ensured deep copying of identities includes PolicyNames. * iam: implement managed policy methods in PostgresStore Modified Postgres schema to include `policy_names` JSONB column in `users`. Implemented `AttachUserPolicy`, `DetachUserPolicy`, and `ListAttachedUserPolicies`. Updated user CRUD operations to handle policy names persistence. * iam: implement managed policy methods in remaining stores Implemented user policy management in: - `FilerEtcStore` (partial implementation) - `IamGrpcStore` (delegated via GetUser/UpdateUser) - `PropagatingCredentialStore` (to broadcast updates) Ensures cluster-wide consistency for policy attachments. * s3api: refactor EmbeddedIamApi to use managed policy APIs - Refactored `AttachUserPolicy`, `DetachUserPolicy`, and `ListAttachedUserPolicies` to use `e.credentialManager` directly. - Fixed a critical error suppression bug in `ExecuteAction` that always returned success even on failure. - Implemented robust error matching using string comparison fallbacks. - Improved consistency by reloading configuration after policy changes. * s3api: update and refine IAM integration tests - Updated tests to use a real `MemoryStore`-backed `CredentialManager`. - Refined test configuration synchronization using `sync.Once` and manual deep-copying to prevent state corruption. - Improved `extractEmbeddedIamErrorCodeAndMessage` to handle more XML formats robustly. - Adjusted test expectations to match current AWS IAM behavior. * fix compilation * visibility * ensure 10 policies * reload * add integration tests * Guard raft command registration * Allow IAM actions in policy tests * Validate gRPC policy attachments * Revert Validate gRPC policy attachments * Tighten gRPC policy attach/detach * Improve IAM managed policy handling * Improve managed policy filters |
||
|
Chris LuandGitHub
|
0d8588e3ae |
S3: Implement IAM defaults and STS signing key fallback (#8348)
* S3: Implement IAM defaults and STS signing key fallback logic * S3: Refactor startup order to init SSE-S3 key manager before IAM * S3: Derive STS signing key from KEK using HKDF for security isolation * S3: Document STS signing key fallback in security.toml * fix(s3api): refine anonymous access logic and secure-by-default behavior - Initialize anonymous identity by default in `NewIdentityAccessManagement` to prevent nil pointer exceptions. - Ensure `ReplaceS3ApiConfiguration` preserves the anonymous identity if not present in the new configuration. - Update `NewIdentityAccessManagement` signature to accept `filerClient`. - In legacy mode (no policy engine), anonymous defaults to Deny (no actions), preserving secure-by-default behavior. - Use specific `LookupAnonymous` method instead of generic map lookup. - Update tests to accommodate signature changes and verify improved anonymous handling. * feat(s3api): make IAM configuration optional - Start S3 API server without a configuration file if `EnableIam` option is set. - Default to `Allow` effect for policy engine when no configuration is provided (Zero-Config mode). - Handle empty configuration path gracefully in `loadIAMManagerFromConfig`. - Add integration test `iam_optional_test.go` to verify empty config behavior. * fix(iamapi): fix signature mismatch in NewIdentityAccessManagementWithStore * fix(iamapi): properly initialize FilerClient instead of passing nil * fix(iamapi): properly initialize filer client for IAM management - Instead of passing `nil`, construct a `wdclient.FilerClient` using the provided `Filers` addresses. - Ensure `NewIdentityAccessManagementWithStore` receives a valid `filerClient` to avoid potential nil pointer dereferences or limited functionality. * clean: remove dead code in s3api_server.go * refactor(s3api): improve IAM initialization, safety and anonymous access security * fix(s3api): ensure IAM config loads from filer after client init * fix(s3): resolve test failures in integration, CORS, and tagging tests - Fix CORS tests by providing explicit anonymous permissions config - Fix S3 integration tests by setting admin credentials in init - Align tagging test credentials in CI with IAM defaults - Added goroutine to retry IAM config load in iamapi server * fix(s3): allow anonymous access to health targets and S3 Tables when identities are present * fix(ci): use /healthz for Caddy health check in awscli tests * iam, s3api: expose DefaultAllow from IAM and Policy Engine This allows checking the global "Open by Default" configuration from other components like S3 Tables. * s3api/s3tables: support DefaultAllow in permission logic and handler Updated CheckPermissionWithContext to respect the DefaultAllow flag in PolicyContext. This enables "Open by Default" behavior for unauthenticated access in zero-config environments. Added a targeted unit test to verify the logic. * s3api/s3tables: propagate DefaultAllow through handlers Propagated the DefaultAllow flag to individual handlers for namespaces, buckets, tables, policies, and tagging. This ensures consistent "Open by Default" behavior across all S3 Tables API endpoints. * s3api: wire up DefaultAllow for S3 Tables API initialization Updated registerS3TablesRoutes to query the global IAM configuration and set the DefaultAllow flag on the S3 Tables API server. This completes the end-to-end propagation required for anonymous access in zero-config environments. Added a SetDefaultAllow method to S3TablesApiServer to facilitate this. * s3api: fix tests by adding DefaultAllow to mock IAM integrations The IAMIntegration interface was updated to include DefaultAllow(), breaking several mock implementations in tests. This commit fixes the build errors by adding the missing method to the mocks. * env * ensure ports * env * env * fix default allow * add one more test using non-anonymous user * debug * add more debug * less logs |
||
|
Chris LuandGitHub
|
cf8e383e1e |
STS: Fallback to Caller Identity when RoleArn is missing in AssumeRole (#8345)
* s3api: make RoleArn optional in AssumeRole * s3api: address PR feedback for optional RoleArn * iam: add configurable default role for AssumeRole * S3 STS: Use caller identity when RoleArn is missing - Fallback to PrincipalArn/Context in AssumeRole if RoleArn is empty - Handle User ARNs in prepareSTSCredentials - Fix PrincipalArn generation for env var credentials * Test: Add unit test for AssumeRole caller identity fallback * fix(s3api): propagate admin permissions to assumed role session when using caller identity fallback * STS: Fix is_admin propagation and optimize IAM policy evaluation for assumed roles - Restore is_admin propagation via JWT req_ctx - Optimize IsActionAllowed to skip role lookups for admin sessions - Ensure session policies are still applied for downscoping - Remove debug logging - Fix syntax errors in cleanup * fix(iam): resolve STS policy bypass for admin sessions - Fixed IsActionAllowed in iam_manager.go to correctly identify and validate internal STS tokens, ensuring session policies are enforced. - Refactored VerifyActionPermission in auth_credentials.go to properly handle session tokens and avoid legacy authorization short-circuits. - Added debug logging for better tracing of policy evaluation and session validation. |
||
|
Chris LuandGitHub
|
49a64f50f1 |
Add session policy support to IAM (#8338)
* Add session policy support to IAM - Implement policy evaluation for session tokens in policy_engine.go - Add session_policy field to session claims for tracking applied policies - Update STS service to include session policies in token generation - Add IAM integration tests for session policy validation - Update IAM manager to support policy attachment to sessions - Extend S3 API STS endpoint to handle session policy restrictions * fix: optimize session policy evaluation and add documentation * sts: add NormalizeSessionPolicy helper for inline session policies * sts: support inline session policies for AssumeRoleWithWebIdentity and credential-based flows * s3api: parse and normalize Policy parameter for STS HTTP handlers * tests: add session policy unit tests and integration tests for inline policy downscoping * tests: add s3tables STS inline policy integration * iam: handle user principals and validate tokens * sts: enforce inline session policy size limit * tests: harden s3tables STS integration config * iam: clarify principal policy resolution errors * tests: improve STS integration endpoint selection |
||
|
Chris LuandGitHub
|
25ea48227f |
Fix STS temporary credentials to use ASIA prefix instead of AKIA (#8326)
Temporary credentials from STS AssumeRole were using "AKIA" prefix (permanent IAM user credentials) instead of "ASIA" prefix (temporary security credentials). This violates AWS conventions and may cause compatibility issues with AWS SDKs that validate credential types. Changes: - Rename generateAccessKeyId to generateTemporaryAccessKeyId for clarity - Update function to use ASIA prefix for temporary credentials - Add unit tests to verify ASIA prefix format (weed/iam/sts/credential_prefix_test.go) - Add integration test to verify ASIA prefix in S3 API (test/s3/iam/s3_sts_credential_prefix_test.go) - Ensure AWS-compatible credential format (ASIA + 16 hex chars) The credentials are already deterministic (SHA256-based from session ID) and the SessionToken is correctly set to the JWT token, so this is just a prefix fix to follow AWS standards. Fixes #8312 |
||
|
23c25379ca |
iam: add ECDSA support for OIDC token validation (#8166)
* iam: add ECDSA support for OIDC token validation Fixes seaweedfs/seaweedfs#8148 * iam: refactor OIDC ECDSA tests and add failure cases - Refactored TestOIDCProviderJWTValidationECDSA to use t.Run - Added sub-tests for expired token, wrong key, invalid issuer, and invalid audience * Update weed/iam/oidc/oidc_provider_test.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * iam: improve error type assertions for OIDC invalid signature tests - Updated both RSA and ECDSA tests to specifically check for ErrProviderInvalidToken * iam: pad EC coordinates in OIDC tests to comply with RFC 7518 - Coordinates are now zero-padded to the full field size (e.g., 32 bytes for P-256) - Ensures interoperability with strict OIDC providers --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> |
||
|
Chris LuandGitHub
|
8814c2a07d |
iam: support ForAnyValue and ForAllValues condition set operators (#8105)
* iam: support ForAnyValue and ForAllValues condition set operators
This implementation adds support for AWS-style IAM condition set operators
`ForAnyValue:` and `ForAllValues:`. These are essential for trust policies
that evaluate collection-based claims like `oidc:roles` or groups.
- Updated EvaluateStringCondition to handle set operators.
- Added set operator support to numeric, date, and boolean conditions.
- ForAnyValue matches if any request value matches any condition value (default).
- ForAllValues matches if every request value matches at least one condition value.
* iam: add test suite for condition set operators
* iam: ensure ForAllValues is vacuously true for all condition types
Aligned Numeric, Date, and Boolean conditions with AWS IAM behavior
where ForAllValues returns true when the request context values are empty.
* iam: add Date vacuously true test case for ForAllValues
* iam: expand policy variables in case-insensitive string conditions
Added expandPolicyVariables support to evaluateStringConditionIgnoreCase
to ensure consistency with case-sensitive counterparts.
* iam: fix negation issues in string set operators
Refactored EvaluateStringCondition and evaluateStringConditionIgnoreCase
to evaluate operators (including negation) per context value before
aggregating. This ensures StringNotEquals and StringNotLike work
correctly with ForAllValues and ForAnyValue.
* iam: add []string support for Date and Boolean context values
Ensures consistency with Numeric conditions by allowing context values
to be provided as slices of strings, which is common in JSON/OIDC claims.
* iam: simplify redundant type check in policy engine
The `evaluateStringConditionIgnoreCase` function had a redundant type
check for `string` in the `default` block of a type switch that
already handled the `string` case.
* iam: remove outdated "currently fails" comment in negation tests
* iam: add StringLikeIgnoreCase condition support
* iam: explicitly handle empty context sets for ForAnyValue
AWS IAM treats empty request sets as "no match" for ForAnyValue.
Added an explicit check and comment to make this behavior clear.
* iam: refactor EvaluateStringCondition to expand policy variables once
Avoid redundant calls to expandPolicyVariables by expanding them once
per condition value instead of inside awsIAMMatch or in the exact
matching branch.
* iam: fix StringLike case sensitivity to match AWS IAM specs
StringLike and StringNotLike condition operators are case-sensitive in
AWS IAM. Changed the implementation to use filepath.Match for
case-sensitive wildcard matching instead of the case-insensitive
awsIAMMatch.
* iam: integrate StringLike case-sensitivity test into suite
Integrated the case-sensitivity verification into condition_set_test.go
and updated the consistency test to use StringLikeIgnoreCase to maintain
its case-insensitive matching verification.
* iam: fix NumericNotEquals logic to follow "not equal to any" semantics
Updated evaluateNumericCondition to correctly handle NumericNotEquals by
ensuring a context value matches only if it is not equal to ANY of the
provided expected values. Also added support for []string expected
values.
* iam: fix DateNotEquals logic and integrate tests
Updated evaluateDateCondition to correctly handle DateNotEquals logic.
Integrated the new test cases for NumericNotEquals and DateNotEquals into
condition_set_test.go.
* iam: fix validation error in integrated NotEquals tests
Added missing Resource field to IAM policy statements in
condition_set_test.go to satisfy validation requirements.
* iam: add set operator support for IP and Null conditions
Implemented ForAllValues and ForAnyValue support for IpAddress,
NotIpAddress, and Null condition operators. Also added test coverage for
ForAnyValue with an empty context to ensure correct behavior.
* iam: refine IP condition evaluation to handle multiple policy value types
Updated evaluateIPCondition to correctly handle string, []string, and
[]interface{} values for IP address conditions in policy documents.
Added IpAddress:SingleStringValue test case to verify consistency.
* iam: refine Null and case-insensitive string conditions
- Reverted evaluateNullCondition to standard AWS behavior (no set operators).
- Refactored evaluateStringConditionIgnoreCase to use idiomatic helpers
(strings.EqualFold and AwsWildcardMatch).
- Cleaned up tests in condition_set_test.go.
* iam: normalize policy value handling across condition evaluators
- Implemented normalizeRanges helper for consistent IP range extraction.
- Expanded type switches in IP, Bool, and String condition evaluators to
support string, []string, and []interface{} policy values.
- Fixed ForAnyValue bool matching to support string slices.
- Added targeted tests for []string policy values in condition_set_test.go.
* iam: refactor IP condition to support arbitrary context keys
Refactored evaluateIPCondition to iterate through all keys in the
condition block instead of hardcoding aws:SourceIp. This ensures
consistency with other condition types and allows custom context keys.
Added IpAddress:CustomContextKey test case to verify the change.
|
||
|
Chris LuandGitHub
|
6394e2f6a5 |
Fix IAM OIDC role mapping and OIDC claims in trust policy (#8104)
* Fix IAM OIDC role mapping and OIDC claims in trust policy * Address PR review: Add config safety checks and refactor tests |
||
|
Chris LuandGitHub
|
cd2e93bf2b |
fix: propagate OIDC attributes to STS session token for IAM policies (#8079)
* fix: propagate OIDC attributes to STS session token * refactor: apply PR suggestions for STS session claims |
||
|
Chris LuandGitHub
|
bc8a077561 |
Fix: Propagate OIDC claims for dynamic IAM policies (#8060)
Fix: Propagate OIDC claims to IAM identity for dynamic policy variables Fixes #8037. Ensures additional OIDC claims (like preferred_username) are preserved in ExternalIdentity attributes and propagated to IAM tokens, enabling substitution in dynamic policies. |
||
|
Chris LuandGitHub
|
ee3813787e |
feat(s3api): Implement S3 Policy Variables (#8039)
* feat: Add AWS IAM Policy Variables support to S3 API
Implements policy variables for dynamic access control in bucket policies.
Supported variables:
- aws:username - Extracted from principal ARN
- aws:userid - User identifier (same as username in SeaweedFS)
- aws:principaltype - IAMUser, IAMRole, or AssumedRole
- jwt:* - Any JWT claim (e.g., jwt:preferred_username, jwt:sub)
Key changes:
- Added PolicyVariableRegex to detect ${...} patterns
- Extended CompiledStatement with DynamicResourcePatterns, DynamicPrincipalPatterns, DynamicActionPatterns
- Added Claims field to PolicyEvaluationArgs for JWT claim access
- Implemented SubstituteVariables() for variable replacement from context and JWT claims
- Implemented extractPrincipalVariables() for ARN parsing
- Updated EvaluateConditions() to support variable substitution
- Comprehensive unit and integration tests
Resolves #8037
* feat: Add LDAP and PrincipalAccount variable support
Completes future enhancements for policy variables:
- Added ldap:* variable support for LDAP claims
- ldap:username - LDAP username from claims
- ldap:dn - LDAP distinguished name from claims
- ldap:* - Any LDAP claim
- Added aws:PrincipalAccount extraction from ARN
- Extracts account ID from principal ARN
- Available as ${aws:PrincipalAccount} in policies
Updated SubstituteVariables() to check LDAP claims
Updated extractPrincipalVariables() to extract account ID
Added comprehensive tests for new variables
* feat(s3api): implement IAM policy variables core logic and optimization
* feat(s3api): integrate policy variables with S3 authentication and handlers
* test(s3api): add integration tests for policy variables
* cleanup: remove unused policy conversion files
* Add S3 policy variables integration tests and path support
- Add comprehensive integration tests for policy variables
- Test username isolation, JWT claims, LDAP claims
- Add support for IAM paths in principal ARN parsing
- Add tests for principals with paths
* Fix IAM Role principal variable extraction
IAM Roles should not have aws:userid or aws:PrincipalAccount
according to AWS behavior. Only IAM Users and Assumed Roles
should have these variables.
Fixes TestExtractPrincipalVariables test failures.
* Security fixes and bug fixes for S3 policy variables
SECURITY FIXES:
- Prevent X-SeaweedFS-Principal header spoofing by clearing internal
headers at start of authentication (auth_credentials.go)
- Restrict policy variable substitution to safe allowlist to prevent
client header injection (iam/policy/policy_engine.go)
- Add core policy validation before storing bucket policies
BUG FIXES:
- Remove unused sid variable in evaluateStatement
- Fix LDAP claim lookup to check both prefixed and unprefixed keys
- Add ValidatePolicy call in PutBucketPolicyHandler
These fixes prevent privilege escalation via header injection and
ensure only validated identity claims are used in policy evaluation.
* Additional security fixes and code cleanup
SECURITY FIXES:
- Fixed X-Forwarded-For spoofing by only trusting proxy headers from
private/localhost IPs (s3_iam_middleware.go)
- Changed context key from "sourceIP" to "aws:SourceIp" for proper
policy variable substitution
CODE IMPROVEMENTS:
- Kept aws:PrincipalAccount for IAM Roles to support condition evaluations
- Removed redundant STS principaltype override
- Removed unused service variable
- Cleaned up commented-out debug logging statements
- Updated tests to reflect new IAM Role behavior
These changes prevent IP spoofing attacks and ensure policy variables
work correctly with the safe allowlist.
* Add security documentation for ParseJWTToken
Added comprehensive security comments explaining that ParseJWTToken
is safe despite parsing without verification because:
- It's only used for routing to the correct verification method
- All code paths perform cryptographic verification before trusting claims
- OIDC tokens: validated via validateExternalOIDCToken
- STS tokens: validated via ValidateSessionToken
Enhanced function documentation with clear security warnings about
proper usage to prevent future misuse.
* Fix IP condition evaluation to use aws:SourceIp key
Fixed evaluateIPCondition in IAM policy engine to use "aws:SourceIp"
instead of "sourceIP" to match the updated extractRequestContext.
This fixes the failing IP-restricted role test where IP-based policy
conditions were not being evaluated correctly.
Updated all test cases to use the correct "aws:SourceIp" key.
* Address code review feedback: optimize and clarify
PERFORMANCE IMPROVEMENT:
- Optimized expandPolicyVariables to use regexp.ReplaceAllStringFunc
for single-pass variable substitution instead of iterating through
all safe variables. This improves performance from O(n*m) to O(m)
where n is the number of safe variables and m is the pattern length.
CODE CLARITY:
- Added detailed comment explaining LDAP claim fallback mechanism
(checks both prefixed and unprefixed keys for compatibility)
- Enhanced TODO comment for trusted proxy configuration with rationale
and recommendations for supporting cloud load balancers, CDNs, and
complex network topologies
All tests passing.
* Address Copilot code review feedback
BUG FIXES:
- Fixed type switch for int/int32/int64 - separated into individual cases
since interface type switches only match the first type in multi-type cases
- Fixed grammatically incorrect error message in types.go
CODE QUALITY:
- Removed duplicate Resource/NotResource validation (already in ValidateStatement)
- Added comprehensive comment explaining isEnabled() logic and security implications
- Improved trusted proxy NOTE comment to be more concise while noting limitations
All tests passing.
* Fix test failures after extractSourceIP security changes
Updated tests to work with the security fix that only trusts
X-Forwarded-For/X-Real-IP headers from private IP addresses:
- Set RemoteAddr to 127.0.0.1 in tests to simulate trusted proxy
- Changed context key from "sourceIP" to "aws:SourceIp"
- Added test case for untrusted proxy (public RemoteAddr)
- Removed invalid ValidateStatement call (validation happens in ValidatePolicy)
All tests now passing.
* Address remaining Gemini code review feedback
CODE SAFETY:
- Deep clone Action field in CompileStatement to prevent potential data races
if the original policy document is modified after compilation
TEST CLEANUP:
- Remove debug logging (fmt.Fprintf) from engine_notresource_test.go
- Remove unused imports in engine_notresource_test.go
All tests passing.
* Fix insecure JWT parsing in IAM auth flow
SECURITY FIX:
- Renamed ParseJWTToken to ParseUnverifiedJWTToken with explicit security warnings.
- Refactored AuthenticateJWT to use the trusted SessionInfo returned by ValidateSessionToken
instead of relying on unverified claims from the initial parse.
- Refactored ValidatePresignedURLWithIAM to reuse the robust AuthenticateJWT logic, removing
duplicated and insecure manual token parsing.
This ensures all identity information (Role, Principal, Subject) used for authorization
decisions is derived solely from cryptographically verified tokens.
* Security: Fix insecure JWT claim extraction in policy engine
- Refactored EvaluatePolicy to accept trusted claims from verified Identity instead of parsing unverified tokens
- Updated AuthenticateJWT to populate Claims in IAMIdentity from verified sources (SessionInfo/ExternalIdentity)
- Updated s3api_server and handlers to pass claims correctly
- Improved isPrivateIP to support IPv6 loopback, link-local, and ULA
- Fixed flaky distributed_session_consistency test with retry logic
* fix(iam): populate Subject in STSSessionInfo to ensure correct identity propagation
This fixes the TestS3IAMAuthentication/valid_jwt_token_authentication failure by ensuring the session subject (sub) is correctly mapped to the internal SessionInfo struct, allowing bucket ownership validation to succeed.
* Optimized isPrivateIP
* Create s3-policy-tests.yml
* fix tests
* fix tests
* tests(s3/iam): simplify policy to resource-based \ (step 1)
* tests(s3/iam): add explicit Deny NotResource for isolation (step 2)
* fixes
* policy: skip resource matching for STS trust policies to allow AssumeRole evaluation
* refactor: remove debug logging and hoist policy variables for performance
* test: fix TestS3IAMBucketPolicyIntegration cleanup to handle per-subtest object lifecycle
* test: fix bucket name generation to comply with S3 63-char limit
* test: skip TestS3IAMPolicyEnforcement until role setup is implemented
* test: use weed mini for simpler test server deployment
Replace 'weed server' with 'weed mini' for IAM tests to avoid port binding issues
and simplify the all-in-one server deployment. This improves test reliability
and execution time.
* security: prevent allocation overflow in policy evaluation
Add maxPoliciesForEvaluation constant to cap the number of policies evaluated
in a single request. This prevents potential integer overflow when allocating
slices for policy lists that may be influenced by untrusted input.
Changes:
- Add const maxPoliciesForEvaluation = 1024 to set an upper bound
- Validate len(policies) < maxPoliciesForEvaluation before appending bucket policy
- Use append() instead of make([]string, len+1) to avoid arithmetic overflow
- Apply fix to both IsActionAllowed policy evaluation paths
|
||
|
06391701ed |
Add AssumeRole and AssumeRoleWithLDAPIdentity STS actions (#8003)
* test: add integration tests for AssumeRole and AssumeRoleWithLDAPIdentity STS actions - Add s3_sts_assume_role_test.go with comprehensive tests for AssumeRole: * Parameter validation (missing RoleArn, RoleSessionName, invalid duration) * AWS SigV4 authentication with valid/invalid credentials * Temporary credential generation and usage - Add s3_sts_ldap_test.go with tests for AssumeRoleWithLDAPIdentity: * Parameter validation (missing LDAP credentials, RoleArn) * LDAP authentication scenarios (valid/invalid credentials) * Integration with LDAP server (when configured) - Update Makefile with new test targets: * test-sts: run all STS tests * test-sts-assume-role: run AssumeRole tests only * test-sts-ldap: run LDAP STS tests only * test-sts-suite: run tests with full service lifecycle - Enhance setup_all_tests.sh: * Add OpenLDAP container setup for LDAP testing * Create test LDAP users (testuser, ldapadmin) * Set LDAP environment variables for tests * Update cleanup to remove LDAP container - Fix setup_keycloak.sh: * Enable verbose error logging for realm creation * Improve error diagnostics Tests use fail-fast approach (t.Fatal) when server not configured, ensuring clear feedback when infrastructure is missing. * feat: implement AssumeRole and AssumeRoleWithLDAPIdentity STS actions Implement two new STS actions to match MinIO's STS feature set: **AssumeRole Implementation:** - Add handleAssumeRole with full AWS SigV4 authentication - Integrate with existing IAM infrastructure via verifyV4Signature - Validate required parameters (RoleArn, RoleSessionName) - Validate DurationSeconds (900-43200 seconds range) - Generate temporary credentials with expiration - Return AWS-compatible XML response **AssumeRoleWithLDAPIdentity Implementation:** - Add handleAssumeRoleWithLDAPIdentity handler (stub) - Validate LDAP-specific parameters (LDAPUsername, LDAPPassword) - Validate common STS parameters (RoleArn, RoleSessionName, DurationSeconds) - Return proper error messages for missing LDAP provider - Ready for LDAP provider integration **Routing Fixes:** - Add explicit routes for AssumeRole and AssumeRoleWithLDAPIdentity - Prevent IAM handler from intercepting authenticated STS requests - Ensure proper request routing priority **Handler Infrastructure:** - Add IAM field to STSHandlers for SigV4 verification - Update NewSTSHandlers to accept IAM reference - Add STS-specific error codes and response types - Implement writeSTSErrorResponse for AWS-compatible errors The AssumeRole action is fully functional and tested. AssumeRoleWithLDAPIdentity requires LDAP provider implementation. * fix: update IAM matcher to exclude STS actions from interception Update the IAM handler matcher to check for STS actions (AssumeRole, AssumeRoleWithWebIdentity, AssumeRoleWithLDAPIdentity) and exclude them from IAM handler processing. This allows STS requests to be handled by the STS fallback handler even when they include AWS SigV4 authentication. The matcher now parses the form data to check the Action parameter and returns false for STS actions, ensuring they are routed to the correct handler. Note: This is a work-in-progress fix. Tests are still showing some routing issues that need further investigation. * fix: address PR review security issues for STS handlers This commit addresses all critical security issues from PR review: Security Fixes: - Use crypto/rand for cryptographically secure credential generation instead of time.Now().UnixNano() (fixes predictable credentials) - Add sts:AssumeRole permission check via VerifyActionPermission to prevent unauthorized role assumption - Generate proper session tokens using crypto/rand instead of placeholder strings Code Quality Improvements: - Refactor DurationSeconds parsing into reusable parseDurationSeconds() helper function used by all three STS handlers - Create generateSecureCredentials() helper for consistent and secure temporary credential generation - Fix iamMatcher to check query string as fallback when Action not found in form data LDAP Provider Implementation: - Add go-ldap/ldap/v3 dependency - Create LDAPProvider implementing IdentityProvider interface with full LDAP authentication support (connect, bind, search, groups) - Update ProviderFactory to create real LDAP providers - Wire LDAP provider into AssumeRoleWithLDAPIdentity handler Test Infrastructure: - Add LDAP user creation verification step in setup_all_tests.sh * fix: address PR feedback (Round 2) - config validation & provider improvements - Implement `validateLDAPConfig` in `ProviderFactory` - Improve `LDAPProvider.Initialize`: - Support `connectionTimeout` parsing (string/int/float) from config map - Warn if `BindDN` is present but `BindPassword` is empty - Improve `LDAPProvider.GetUserInfo`: - Add fallback to `searchUserGroups` if `memberOf` returns no groups (consistent with Authenticate) * fix: address PR feedback (Round 3) - LDAP connection improvements & build fix - Improve `LDAPProvider` connection handling: - Use `net.Dialer` with configured timeout for connection establishment - Enforce TLS 1.2+ (`MinVersion: tls.VersionTLS12`) for both LDAPS and StartTLS - Fix build error in `s3api_sts.go` (format verb for ErrorCode) * fix: address PR feedback (Round 4) - LDAP hardening, Authz check & Routing fix - LDAP Provider Hardening: - Prevent re-initialization - Enforce single user match in `GetUserInfo` (was explicit only in Authenticate) - Ensure connection closure if StartTLS fails - STS Handlers: - Add robust provider detection using type assertion - **Security**: Implement authorization check (`VerifyActionPermission`) after LDAP authentication - Routing: - Update tests to reflect that STS actions are handled by STS handler, not generic IAM * fix: address PR feedback (Round 5) - JWT tokens, ARN formatting, PrincipalArn CRITICAL FIXES: - Replace standalone credential generation with STS service JWT tokens - handleAssumeRole now generates proper JWT session tokens - handleAssumeRoleWithLDAPIdentity now generates proper JWT session tokens - Session tokens can be validated across distributed instances - Fix ARN formatting in responses - Extract role name from ARN using utils.ExtractRoleNameFromArn() - Prevents malformed ARNs like "arn:aws:sts::assumed-role/arn:aws:iam::..." - Add configurable AccountId for federated users - Add AccountId field to STSConfig (defaults to "111122223333") - PrincipalArn now uses configured account ID instead of hardcoded "aws" - Enables proper trust policy validation IMPROVEMENTS: - Sanitize LDAP authentication error messages (don't leak internal details) - Remove duplicate comment in provider detection - Add utils import for ARN parsing utilities * feat: implement LDAP connection pooling to prevent resource exhaustion PERFORMANCE IMPROVEMENT: - Add connection pool to LDAPProvider (default size: 10 connections) - Reuse LDAP connections across authentication requests - Prevent file descriptor exhaustion under high load IMPLEMENTATION: - connectionPool struct with channel-based connection management - getConnection(): retrieves from pool or creates new connection - returnConnection(): returns healthy connections to pool - createConnection(): establishes new LDAP connection with TLS support - Close(): cleanup method to close all pooled connections - Connection health checking (IsClosing()) before reuse BENEFITS: - Reduced connection overhead (no TCP handshake per request) - Better resource utilization under load - Prevents "too many open files" errors - Non-blocking pool operations (creates new conn if pool empty) * fix: correct TokenGenerator access in STS handlers CRITICAL FIX: - Make TokenGenerator public in STSService (was private tokenGenerator) - Update all references from Config.TokenGenerator to TokenGenerator - Remove TokenGenerator from STSConfig (it belongs in STSService) This fixes the "NotImplemented" errors in distributed and Keycloak tests. The issue was that Round 5 changes tried to access Config.TokenGenerator which didn't exist - TokenGenerator is a field in STSService, not STSConfig. The TokenGenerator is properly initialized in STSService.Initialize() and is now accessible for JWT token generation in AssumeRole handlers. * fix: update tests to use public TokenGenerator field Following the change to make TokenGenerator public in STSService, this commit updates the test files to reference the correct public field name. This resolves compilation errors in the IAM STS test suite. * fix: update distributed tests to use valid Keycloak users Updated s3_iam_distributed_test.go to use 'admin-user' and 'read-user' which exist in the standard Keycloak setup provided by setup_keycloak.sh. This resolves 'unknown test user' errors in distributed integration tests. * fix: ensure iam_config.json exists in setup target for CI The GitHub Actions workflow calls 'make setup' which was not creating iam_config.json, causing the server to start without IAM integration enabled (iamIntegration = nil), resulting in NotImplemented errors. Now 'make setup' copies iam_config.local.json to iam_config.json if it doesn't exist, ensuring IAM is properly configured in CI. * fix(iam/ldap): fix connection pool race and rebind corruption - Add atomic 'closed' flag to connection pool to prevent racing on Close() - Rebind authenticated user connections back to service account before returning to pool - Close connections on error instead of returning potentially corrupted state to pool * fix(iam/ldap): populate standard TokenClaims fields in ValidateToken - Set Subject, Issuer, Audience, IssuedAt, and ExpiresAt to satisfy the interface - Use time.Time for timestamps as required by TokenClaims struct - Default to 1 hour TTL for LDAP tokens * fix(s3api): include account ID in STS AssumedRoleUser ARN - Consistent with AWS, include the account ID in the assumed-role ARN - Use the configured account ID from STS service if available, otherwise default to '111122223333' - Apply to both AssumeRole and AssumeRoleWithLDAPIdentity handlers - Also update .gitignore to ignore IAM test environment files * refactor(s3api): extract shared STS credential generation logic - Move common logic for session claims and credential generation to prepareSTSCredentials - Update handleAssumeRole and handleAssumeRoleWithLDAPIdentity to use the helper - Remove stale comments referencing outdated line numbers * feat(iam/ldap): make pool size configurable and add audience support - Add PoolSize to LDAPConfig (default 10) - Add Audience to LDAPConfig to align with OIDC validation - Update initialization and ValidateToken to use new fields * update tests * debug * chore(iam): cleanup debug prints and fix test config port * refactor(iam): use mapstructure for LDAP config parsing * feat(sts): implement strict trust policy validation for AssumeRole * test(iam): refactor STS tests to use AWS SDK signer * test(s3api): implement ValidateTrustPolicyForPrincipal in MockIAMIntegration * fix(s3api): ensure IAM matcher checks query string on ParseForm error * fix(sts): use crypto/rand for secure credentials and extract constants * fix(iam): fix ldap connection leaks and add insecure warning * chore(iam): improved error wrapping and test parameterization * feat(sts): add support for LDAPProviderName parameter * Update weed/iam/ldap/ldap_provider.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update weed/s3api/s3api_sts.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(sts): use STSErrSTSNotReady when LDAP provider is missing * fix(sts): encapsulate TokenGenerator in STSService and add getter --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
Chris LuandGitHub
|
ad76487e9d |
Fix special characters in admin-generated secret keys (#7994)
Fixes #7990 The issue was that the Charset constant used for generating secret keys included the '/' character, which is URL-unsafe. When secret keys containing '/' were used in HTTP requests, they would be URL-encoded, causing a mismatch during signature verification. Changes: - Removed '/' from the Charset constant in weed/iam/constants.go - Added TestGenerateSecretAccessKey_URLSafe to verify generated keys don't contain URL-unsafe characters like '/' or '+' This ensures all newly generated secret keys are URL-safe and will work correctly with S3 authentication. Existing keys continue to work. |
||
|
9012069bd7 |
chore: execute goimports to format the code (#7983)
* chore: execute goimports to format the code Signed-off-by: promalert <promalert@outlook.com> * goimports -w . --------- Signed-off-by: promalert <promalert@outlook.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris LuandGitHub
|
e67973dc53 |
Support Policy Attachment for Object Store Users (#7981)
* Implement Policy Attachment support for Object Store Users
- Added policy_names field to iam.proto and regenerated protos.
- Updated S3 API and IAM integration to support direct policy evaluation for users.
- Enhanced Admin UI to allow attaching policies to users via modals.
- Renamed 'policies' to 'policy_names' to clarify that it stores identifiers.
- Fixed syntax error in user_management.go.
* Fix policy dropdown not populating
The API returns {policies: [...]} but JavaScript was treating response as direct array.
Updated loadPolicies() to correctly access data.policies property.
* Add null safety checks for policy dropdowns
Added checks to prevent "undefined" errors when:
- Policy select elements don't exist
- Policy dropdowns haven't loaded yet
- User is being edited before policies are loaded
* Fix policy dropdown by using correct JSON field name
JSON response has lowercase 'name' field but JavaScript was accessing 'Name'.
Changed policy.Name to policy.name to match the IAMPolicy JSON structure.
* Fix policy names not being saved on user update
Changed condition from len(req.PolicyNames) > 0 to req.PolicyNames != nil
to ensure policy names are always updated when present in the request,
even if it's an empty array (to allow clearing policies).
* Add debug logging for policy names update flow
Added console.log in frontend and glog in backend to trace
policy_names data through the update process.
* Temporarily disable auto-reload for debugging
Commented out window.location.reload() so console logs are visible
when updating a user.
* Add detailed debug logging and alert for policy selection
Added console.log for each step and an alert to show policy_names value
to help diagnose why it's not being included in the request.
* Regenerate templ files for object_store_users
Ran templ generate to ensure _templ.go files are up to date with
the latest .templ changes including debug logging.
* Remove debug logging and restore normal functionality
Cleaned up temporary debug code (console.log and alert statements)
and re-enabled automatic page reload after user update.
* Add step-by-step alert debugging for policy update
Added 5 alert checkpoints to trace policy data through the update flow:
1. Check if policiesSelect element exists
2. Show selected policy values
3. Show userData.policy_names
4. Show full request body
5. Confirm server response
Temporarily disabled auto-reload to see alerts.
* Add version check alert on page load
Added alert on DOMContentLoaded to verify new JavaScript is being executed
and not cached by the browser.
* Compile templates using make
Ran make to compile all template files and install the weed binary.
* Add button click detection and make handleUpdateUser global
- Added inline alert on button click to verify click is detected
- Made handleUpdateUser a window-level function to ensure it's accessible
- Added alert at start of handleUpdateUser function
* Fix handleUpdateUser scope issue - remove duplicate definition
Removed duplicate function definition that was inside DOMContentLoaded.
Now handleUpdateUser is defined only once in global scope (line 383)
making it accessible when button onclick fires.
* Remove all duplicate handleUpdateUser definitions
Now handleUpdateUser is defined only once at the very top of the script
block (line 352), before DOMContentLoaded, ensuring it's available when
the button onclick fires.
* Add function existence check and error catching
Added alerts to check if handleUpdateUser is defined and wrapped
the function call in try-catch to capture any JavaScript errors.
Also added console.log statements to verify function definition.
* Simplify handleUpdateUser to non-async for testing
Removed async/await and added early return to test if function
can be called at all. This will help identify if async is causing
the issue.
* Add cache-control headers to prevent browser caching
Added no-cache headers to ShowObjectStoreUsers handler to prevent
aggressive browser caching of inline JavaScript in the HTML page.
* Fix syntax error - make handleUpdateUser async
Changed function back to async to fix 'await is only valid in async functions' error.
The cache-control headers are working - browser is now loading new code.
* Update version check to v3 to verify cache busting
Changed version alert to 'v3 - WITH EARLY RETURN' to confirm
the new code with early return statement is being loaded.
* Remove all debug code - clean implementation
Removed all alerts, console.logs, and test code.
Implemented clean policy update functionality with proper error handling.
* Add ETag header for cache-busting and update walkthrough
* Fix policy pre-selection in Edit User modal
- Updated admin.js editUser function to pre-select policies
- Root cause: duplicate editUser in admin.js overwrote inline version
- Added policy pre-selection logic to match inline template
- Verified working in browser: policies now pre-select correctly
* Fix policy persistence in handleUpdateUser
- Added policy_names field to userData payload in handleUpdateUser
- Policies were being lost because handleUpdateUser only sent email and actions
- Now collects selected policies from editPolicies dropdown
- Verified working: policies persist correctly across updates
* Fix XSS vulnerability in access keys display
- Escape HTML in access key display using escapeHtml utility
- Replace inline onclick handlers with data attributes
- Add event delegation for delete access key buttons
- Prevents script injection via malicious access key values
* Fix additional XSS vulnerabilities in user details display
- Escape HTML in actions badges (line 626)
- Escape HTML in policy_names badges (line 636)
- Prevents script injection via malicious action or policy names
* Fix XSS vulnerability in loadPolicies function
- Replace innerHTML string concatenation with DOM API
- Use createElement and textContent for safe policy name insertion
- Prevents script injection via malicious policy names
- Apply same pattern to both create and edit select elements
* Remove debug logging from UpdateObjectStoreUser
- Removed glog.V(0) debug statements
- Clean up temporary debugging code before production
* Remove duplicate handleUpdateUser function
- Removed inline handleUpdateUser that duplicated admin.js logic
- Removed debug console.log statement
- admin.js version is now the single source of truth
- Eliminates maintenance burden of keeping two versions in sync
* Refine user management and address code review feedback
- Preserve PolicyNames in UpdateUserPolicies
- Allow clearing actions in UpdateObjectStoreUser by checking for nil
- Remove version comment from object_store_users.templ
- Refactor loadPolicies for DRYness using cloneNode while keeping DOM API security
* IAM Authorization for Static Access Keys
* verified XSS Fixes in Templates
* fix div
|
||
|
Chris LuandGitHub
|
d75162370c |
Fix trust policy wildcard principal handling (#7970)
* Fix trust policy wildcard principal handling
This change fixes the trust policy validation to properly support
AWS-standard wildcard principals like {"Federated": "*"}.
Previously, the evaluatePrincipalValue() function would check for
context existence before evaluating wildcards, causing wildcard
principals to fail when the context key didn't exist. This forced
users to use the plain "*" workaround instead of the more specific
{"Federated": "*"} format.
Changes:
- Modified evaluatePrincipalValue() to check for "*" FIRST before
validating against context
- Added support for wildcards in principal arrays
- Added comprehensive tests for wildcard principal handling
- All existing tests continue to pass (no regressions)
This matches AWS IAM behavior where "*" in a principal field means
"allow any value" without requiring context validation.
Fixes: https://github.com/seaweedfs/seaweedfs/issues/7917
* Refactor: Move Principal matching to PolicyEngine
This refactoring consolidates all policy evaluation logic into the
PolicyEngine, improving code organization and eliminating duplication.
Changes:
- Added matchesPrincipal() and evaluatePrincipalValue() to PolicyEngine
- Added EvaluateTrustPolicy() method for direct trust policy evaluation
- Updated statementMatches() to check Principal field when present
- Made resource matching optional (trust policies don't have Resources)
- Simplified evaluateTrustPolicy() in iam_manager.go to delegate to PolicyEngine
- Removed ~170 lines of duplicate code from iam_manager.go
Benefits:
- Single source of truth for all policy evaluation
- Better code reusability and maintainability
- Consistent evaluation rules for all policy types
- Easier to test and debug
All tests pass with no regressions.
* Make PolicyEngine AWS-compatible and add unit tests
Changes:
1. AWS-Compatible Context Keys:
- Changed "seaweed:FederatedProvider" -> "aws:FederatedProvider"
- Changed "seaweed:AWSPrincipal" -> "aws:PrincipalArn"
- Changed "seaweed:ServicePrincipal" -> "aws:PrincipalServiceName"
- This ensures 100% AWS compatibility for trust policies
2. Added Comprehensive Unit Tests:
- TestPrincipalMatching: 8 test cases for Principal matching
- TestEvaluatePrincipalValue: 7 test cases for value evaluation
- TestTrustPolicyEvaluation: 6 test cases for trust policy evaluation
- TestGetPrincipalContextKey: 4 test cases for context key mapping
- Total: 25 new unit tests for PolicyEngine
All tests pass:
- Policy engine tests: 54 passed
- Integration tests: 9 passed
- Total: 63 tests passing
* Update context keys to standard AWS/OIDC formats
Replaced remaining seaweed: context keys with standard AWS and OIDC
keys to ensure 100% compatibility with AWS IAM policies.
Mappings:
- seaweed:TokenIssuer -> oidc:iss
- seaweed:Issuer -> oidc:iss
- seaweed:Subject -> oidc:sub
- seaweed:SourceIP -> aws:SourceIp
Also updated unit tests to reflect these changes.
All 63 tests pass successfully.
* Add advanced policy tests for variable substitution and conditions
Added comprehensive tests inspired by AWS IAM patterns:
- TestPolicyVariableSubstitution: Tests ${oidc:sub} variable in resources
- TestConditionWithNumericComparison: Tests sts:DurationSeconds condition
- TestMultipleConditionOperators: Tests combining StringEquals and StringLike
Results:
- TestMultipleConditionOperators: ✅ All 3 subtests pass
- Other tests reveal need for sts:DurationSeconds context population
These tests validate the PolicyEngine's ability to handle complex
AWS-compatible policy scenarios.
* Fix federated provider context and add DurationSeconds support
Changes:
- Use iss claim as aws:FederatedProvider (AWS standard)
- Add sts:DurationSeconds to trust policy evaluation context
- TestPolicyVariableSubstitution now passes ✅
Remaining work:
- TestConditionWithNumericComparison partially works (1/3 pass)
- Need to investigate NumericLessThanEquals evaluation
* Update trust policies to use issuer URL for AWS compatibility
Changed trust policy from using provider name ("test-oidc") to
using the issuer URL ("https://test-issuer.com") to match AWS
standard behavior where aws:FederatedProvider contains the OIDC
issuer URL.
Test Results:
- 10/12 test suites passing
- TestFullOIDCWorkflow: ✅ All subtests pass
- TestPolicyEnforcement: ✅ All subtests pass
- TestSessionExpiration: ✅ Pass
- TestPolicyVariableSubstitution: ✅ Pass
- TestMultipleConditionOperators: ✅ All subtests pass
Remaining work:
- TestConditionWithNumericComparison needs investigation
- One subtest in TestTrustPolicyValidation needs fix
* Fix S3 API tests for AWS compatibility
Updated all S3 API tests to use AWS-compatible context keys and
trust policy principals:
Changes:
- seaweed:SourceIP → aws:SourceIp (IP-based conditions)
- Federated: "test-oidc" → "https://test-issuer.com" (trust policies)
Test Results:
- TestS3EndToEndWithJWT: ✅ All 13 subtests pass
- TestIPBasedPolicyEnforcement: ✅ All 3 subtests pass
This ensures policies are 100% AWS-compatible and portable.
* Fix ValidateTrustPolicy for AWS compatibility
Updated ValidateTrustPolicy method to check for:
- OIDC: issuer URL ("https://test-issuer.com")
- LDAP: provider name ("test-ldap")
- Wildcard: "*"
Test Results:
- TestTrustPolicyValidation: ✅ All 3 subtests pass
This ensures trust policy validation uses the same AWS-compatible
principals as the PolicyEngine.
* Fix multipart and presigned URL tests for AWS compatibility
Updated trust policies in:
- s3_multipart_iam_test.go
- s3_presigned_url_iam_test.go
Changed "Federated": "test-oidc" → "https://test-issuer.com"
Test Results:
- TestMultipartIAMValidation: ✅ All 7 subtests pass
- TestPresignedURLIAMValidation: ✅ All 4 subtests pass
- TestPresignedURLGeneration: ✅ All 4 subtests pass
- TestPresignedURLExpiration: ✅ All 4 subtests pass
- TestPresignedURLSecurityPolicy: ✅ All 4 subtests pass
All S3 API tests now use AWS-compatible trust policies.
* Fix numeric condition evaluation and trust policy validation interface
Major updates to ensure robust AWS-compatible policy evaluation:
1. **Policy Engine**: Added support for `int` and `int64` types in `evaluateNumericCondition`, fixing issues where raw numbers in policy documents caused evaluation failures.
2. **Trust Policy Validation**: Updated `TrustPolicyValidator` interface and `STSService` to propagate `DurationSeconds` correctly during the double-validation flow (Validation -> STS -> Validation callback).
3. **IAM Manager**: Updated implementation to match the new interface and correctly pass `sts:DurationSeconds` context key.
Test Results:
- TestConditionWithNumericComparison: ✅ All 3 subtests pass
- All IAM and S3 integration tests pass (100%)
This resolves the final edge case with DurationSeconds numeric conditions.
* Fix MockTrustPolicyValidator interface and unreachable code warnings
Updates:
1. Updated MockTrustPolicyValidator.ValidateTrustPolicyForWebIdentity to match new interface signature with durationSeconds parameter
2. Removed unreachable code after infinite loops in filer_backup.go and filer_meta_backup.go to satisfy linter
Test Results:
- All STS tests pass ✅
- Build warnings resolved ✅
* Refactor matchesPrincipal to consolidate array handling logic
Consolidated duplicated logic for []interface{} and []string types by converting them to a unified []interface{} upfront.
* Fix malformed AWS docs URL in iam_manager.go comment
* dup
* Enhance IAM integration tests with negative cases and interface array support
Added test cases to TestTrustPolicyWildcardPrincipal to:
1. Verify rejection of roles when principal context does not match (negative test)
2. Verify support for principal arrays as []interface{} (simulating JSON unmarshaled roles)
* Fix syntax errors in filer_backup and filer_meta_backup
Restored missing closing braces for for-loops and re-added return statements.
The previous attempt to remove unreachable code accidentally broke the function structure.
Build now passes successfully.
|
||
|
Chris LuandGitHub
|
54de32f207 |
Support AWS standard IAM role ARN formats (issue #7946) (#7948)
* fix(iam): support both AWS standard and legacy IAM role ARN formats Fix issue #7946 where SeaweedFS only recognized legacy IAM role ARN format (arn:aws:iam::role/RoleName) but not the standard AWS format with account ID (arn:aws:iam::ACCOUNT:role/RoleName). This was breaking EKS pod identity integration which expects the standard format. Changes: - Update ExtractRoleNameFromArn() to handle both formats by searching for 'role/' marker instead of matching a fixed prefix - Update ExtractRoleNameFromPrincipal() to clearly document both STS and IAM formats it supports with or without account ID - Simplify role ARN validation in validateRoleAssumptionForWebIdentity() and validateRoleAssumptionForCredentials() to use the extraction function - Add comprehensive test coverage with 25 test cases covering both formats The fix maintains backward compatibility with legacy format while adding support for standard AWS format with account ID. Fixes: https://github.com/seaweedfs/seaweedfs/issues/7946 * docs: improve docstring coverage for ARN utility functions - Add comprehensive package-level documentation - Enhance ExtractRoleNameFromPrincipal docstring with parameter and return descriptions - Enhance ExtractRoleNameFromArn docstring with detailed format documentation - Add docstrings to test functions explaining test coverage - Update all docstrings to 80%+ coverage for code review compliance * refactor: improve ARN parsing code maintainability and error messages - Define constants for ARN prefixes and markers (stsPrefix, stsAssumedRoleMarker, iamPrefix, iamRoleMarker) - Replace hardcoded magic strings with named constants in ExtractRoleNameFromPrincipal and ExtractRoleNameFromArn - Enhance error messages in sts_service.go to show expected ARN format when validation fails - Error message now shows: 'arn:aws:iam::[ACCOUNT_ID:]role/ROLE_NAME' format - Improves code readability and maintainability - Facilitates future ARN format changes and debugging * feat: add structured ARN type for better debugging and extensibility Implements Option 2 (Structured ARN Type) from ARN handling comparison: New Features: - ARNInfo struct with Original, RoleName, AccountID, and Format fields - ARNFormat enum (Legacy, Standard, Invalid) for type-safe format tracking - ParseRoleARN() function for structured IAM role ARN parsing - ParsePrincipalARN() function for structured STS/IAM principal parsing Benefits: - Better debugging: Can see original ARN, extracted components, and format type - Extensible: Easy to add more fields (Region, Service, etc.) in future - Type-safe: Format is an enum, not a string - Backward compatible: Kept original string-based functions STS Service Updates: - Uses ParseRoleARN() for structured validation - Logs ARN components at V(4) level for debugging (role, account, format) - Better error context when validation fails Test Coverage: - 7 new tests for ParseRoleARN (legacy, standard, invalid formats) - 7 new tests for ParsePrincipalARN (STS/IAM, legacy/standard) - All 39 existing tests still pass - Total: 53 ARN-related tests Comparison with MinIO: - More flexible: Supports both AWS formats (MinIO only supports MinIO format) - Better tested: 53 tests vs MinIO's 8 tests - Structured like MinIO but more practical for AWS use cases * security: fix ARN parsing to prevent malicious ARN acceptance Fix critical security vulnerability where malicious ARNs could bypass validation: - ARNs like 'arn:aws:iam::123456789012:user/role/malicious' were incorrectly accepted - The previous implementation used strings.Index to find 'role/' anywhere in the ARN - This allowed non-role resource types to be accepted if they contained 'role/' in their path Changes: 1. Updated ExtractRoleNameFromArn() to validate resource type is exactly 'role/' 2. Updated ExtractRoleNameFromPrincipal() to validate resource type is exactly 'assumed-role/' 3. Updated ParseRoleARN() to validate structure before extracting fields 4. Updated ParsePrincipalARN() to validate structure before extracting fields 5. Added 6 security test cases to prevent regression The fix validates ARN structure by: - Splitting on ':' to separate account ID from resource type - Verifying resource type starts with exact marker ('role/' or 'assumed-role/') - Only then extracting role name, account ID, and format All 59 tests pass, including new security tests that verify malicious ARNs are rejected. Fixes: GitHub Copilot review #3624499048 * test: add test cases for empty role names and improve validation Address review feedback to improve edge case coverage: 1. Added test case for standard format with empty role name - TestExtractRoleNameFromArn: arn:aws:iam::123456789012:role/ - TestParseRoleARN: arn:aws:iam::123456789012:role/ 2. Added empty role name validation for STS ARNs in ParsePrincipalARN - Now matches ParseRoleARN behavior - Prevents ARNs like arn:aws:sts::assumed-role/ from having valid Format 3. Added test cases for empty STS role names - TestParsePrincipalARN: arn:aws:sts::assumed-role/ - TestParsePrincipalARN: arn:aws:sts::123456789012:assumed-role/ All 65 tests pass (15 for ExtractRoleNameFromArn, 10 for ExtractRoleNameFromPrincipal, 8 for ParseRoleARN, 9 for ParsePrincipalARN, 4 security user ARNs, 2 security STS, plus existing tests). * refactor: simplify ARNInfo by removing Format enum Remove ARNFormat enum (ARNFormatLegacy, ARNFormatStandard, ARNFormatInvalid) as it's not needed for backward compatibility. Simplifications: 1. Removed ARNFormat type and all format constants 2. Removed Format field from ARNInfo struct 3. Validation now checks if RoleName is empty (simpler and clearer) 4. AccountID presence already distinguishes legacy (empty) from standard (non-empty) formats 5. Updated STS service to check RoleName emptiness instead of Format field 6. Improved debug logging to explicitly show "(legacy format)" or "(standard format)" Benefits: - Simpler code with fewer concepts - AccountID field already provides format information - Validation is clearer: empty RoleName = invalid ARN - All 65 tests still pass This change maintains the same functionality while reducing code complexity. No backward compatibility concerns as the structured ARN parsing is new. * test: add comprehensive edge case tests for ARN parsing Add 4 new test functions covering: - Multiple role markers in paths (e.g., role/role/name) - Consecutive slashes in role paths (preserved as valid components) - Special characters valid in AWS role names (+=,.@-_) - Extremely long role names near AWS limits These tests verify the parser's resilience to edge cases and ensure proper handling of various valid role name formats and special characters. |
||
|
Chris LuandGitHub
|
23fc3f2621 |
Fix AWS SDK Signature V4 with STS credentials (issue #7941) (#7944)
* Add documentation for issue #7941 fix * ensure auth * rm FIX_ISSUE_7941.md * Integrate STS session token validation into V4 signature verification - Check for X-Amz-Security-Token header in verifyV4Signature - Call validateSTSSessionToken for STS requests - Skip regular access key lookup and expiration check for STS sessions * Fix variable scoping in verifyV4Signature for STS session token validation * Add ErrExpiredToken error for better AWS S3 compatibility with STS session tokens * Support STS session token in query parameters for presigned URLs * Fix nil pointer dereference in validateSTSSessionToken * Enhance STS token validation with detailed error diagnostics and logging * Fix missing credentials in STSSessionClaims.ToSessionInfo() * test: Add comprehensive STS session claims validation tests - TestSTSSessionClaimsToSessionInfo: Validates basic claims conversion - TestSTSSessionClaimsToSessionInfoCredentialGeneration: Verifies credential generation - TestSTSSessionClaimsToSessionInfoPreservesAllFields: Ensures all fields are preserved - TestSTSSessionClaimsToSessionInfoEmptyFields: Tests handling of empty/nil fields - TestSTSSessionClaimsToSessionInfoCredentialExpiration: Validates expiration handling All tests pass with proper timing tolerance for credential generation. * perf: Reuse CredentialGenerator instance for STS session claims Optimize ToSessionInfo() to reuse a package-level defaultCredentialGenerator instead of allocating a new CredentialGenerator on every call. This reduces allocation overhead since this method is called frequently during signature verification (potentially once per request). The CredentialGenerator is stateless and deterministic, making it safe to reuse across concurrent calls without synchronization. * refactor: Surface credential generation errors and remove sensitive logging Two improvements to error handling and security: 1. weed/iam/sts/session_claims.go: - Add logging for credential generation failures in ToSessionInfo() - Wrap errors with context (session ID) to aid debugging - Use glog.Warningf() to surface errors instead of silently swallowing them - Add fmt import for error wrapping 2. weed/s3api/auth_signature_v4.go: - Remove debug logging of actual access key IDs (glog.V(2) call) - Security improvement: avoid exposing sensitive access keys even at debug level - Keep warning-level logging that shows only count of available keys This ensures credential generation failures are observable while protecting sensitive authentication material from logs. * test: Verify deterministic credential generation in session claims tests Update TestSTSSessionClaimsToSessionInfoCredentialGeneration to properly verify deterministic credential generation: - Remove misleading comment about 'randomness' - parts of credentials ARE deterministic - Add assertions that AccessKeyId is identical for same SessionId (hash-based, deterministic) - Add assertions that SessionToken is identical for same SessionId (hash-based, deterministic) - Verify Expiration matches when SessionId is identical - Document that SecretAccessKey is NOT deterministic (uses random.Read) - Truncate expiresAt to second precision to avoid timing issues This test now properly verifies that the deterministic components of credential generation work correctly while acknowledging the cryptographic randomness of the secret access key. * test(sts): Assert credentials expiration relative to now in credential expiration tests Replace wallclock assertions comparing tc.expiresAt to time.Now() (which only verified test setup) with assertions that check sessionInfo.Credentials.Expiration relative to time.Now(), thus exercising the code under test. Include clarifying comment for intent. * feat(sts): Add IsExpired helpers and use them in expiration tests - Add Credentials.IsExpired() and SessionInfo.IsExpired() in new file session_helpers.go. - Update TestSTSSessionClaimsToSessionInfoCredentialExpiration to use helpers for clearer intent. * test: revert test-only IsExpired helpers; restore direct expiration assertions Remove session_helpers.go and update TestSTSSessionClaimsToSessionInfoCredentialExpiration to assert against sessionInfo.Credentials.Expiration directly as requested by reviewer., * fix(s3api): restore error return when access key not found Critical fix: The previous cleanup of sensitive logging inadvertently removed the error return statement when access key lookup fails. This caused the code to continue and call isCredentialExpired() on nil pointer, crashing the server. This explains EOF errors in CORS tests - server was panicking on requests with invalid keys. * fix(sts): make secret access key deterministic based on sessionId CRITICAL FIX: The secret access key was being randomly generated, causing signature verification failures when the same session token was used twice: 1. AssumeRoleWithWebIdentity generates random secret key X 2. Client signs request using secret key X 3. Server validates token, regenerates credentials via ToSessionInfo() 4. ToSessionInfo() calls generateSecretAccessKey(), which generates random key Y 5. Server tries to verify signature using key Y, but signature was made with X 6. Signature verification fails (SignatureDoesNotMatch) Solution: Make generateSecretAccessKey() deterministic by using SHA256 hash of 'secret-key:' + sessionId, just like generateAccessKeyId() already does. This ensures: - AssumeRoleWithWebIdentity generates deterministic secret key from sessionId - ToSessionInfo() regenerates the same secret key from the same sessionId - Client signature verification succeeds because keys match Fixes: AWS SDK v2 CORS tests failing with 'ExpiredToken' errors Affected files: - weed/iam/sts/token_utils.go: Updated generateSecretAccessKey() signature and implementation to be deterministic - Updated GenerateTemporaryCredentials() to pass sessionId parameter Tests: All 54 STS tests pass with this fix * test(sts): add comprehensive secret key determinism test coverage Updated tests to verify that secret access keys are now deterministic: 1. Updated TestSTSSessionClaimsToSessionInfoCredentialGeneration: - Changed comment from 'NOT deterministic' to 'NOW deterministic' - Added assertion that same sessionId produces identical secret key - Explains why this is critical for signature verification 2. Added TestSecretAccessKeyDeterminism (new dedicated test): - Verifies secret key is identical across multiple calls with same sessionId - Verifies access key ID and session token are also identical - Verifies different sessionIds produce different credentials - Includes detailed comments explaining why determinism is critical These tests ensure that the STS implementation correctly regenerates deterministic credentials during signature verification. Without determinism, signature verification would always fail because the server would use different secret keys than the client used to sign. * refactor(sts): add explicit zero-time expiration handling Improved defensive programming in IsExpired() methods: 1. Credentials.IsExpired(): - Added explicit check for zero-time expiration (time.Time{}) - Treats uninitialized credentials as expired - Prevents accidentally treating uninitialized creds as valid 2. SessionInfo.IsExpired(): - Added same explicit zero-time check - Treats uninitialized sessions as expired - Protects against bugs where sessions might not be properly initialized This is important because time.Now().After(time.Time{}) returns true, but explicitly checking for zero time makes the intent clear and helps catch initialization bugs during code review and debugging. * refactor(sts): remove unused IsExpired() helper functions The session_helpers.go file contained two unused IsExpired() methods: - Credentials.IsExpired() - SessionInfo.IsExpired() These were never called anywhere in the codebase. The actual expiration checks use: - isCredentialExpired() in weed/s3api/auth_credentials.go (S3 auth) - Direct time.Now().After() checks Removing unused code improves code clarity and reduces maintenance burden. * fix(auth): pass STS session token to IAM authorization for V4 signature auth CRITICAL FIX: Session tokens were not being passed to the authorization check when using AWS Signature V4 authentication with STS credentials. The bug: 1. AWS SDK sends request with X-Amz-Security-Token header (V4 signature) 2. validateSTSSessionToken validates the token, creates Identity with PrincipalArn 3. authorizeWithIAM only checked X-SeaweedFS-Session-Token (JWT auth header) 4. Since it was empty, fell into 'static V4' branch which set SessionToken = '' 5. AuthorizeAction returned ErrAccessDenied because SessionToken was empty The fix (in authorizeWithIAM): - Check X-SeaweedFS-Session-Token first (JWT auth) - If empty, fallback to X-Amz-Security-Token header (V4 STS auth) - If still empty, check X-Amz-Security-Token query param (presigned URLs) - When session token is found with PrincipalArn, use 'STS V4 signature' path - Only use 'static V4' path when there's no session token This ensures: - JWT Bearer auth with session tokens works (existing path) - STS V4 signature auth with session tokens works (new path) - Static V4 signature auth without session tokens works (existing path) Logging updated to distinguish: - 'JWT-based IAM authorization' - 'STS V4 signature IAM authorization' (new) - 'static V4 signature IAM authorization' (clarified) * test(s3api): add comprehensive STS session token authorization test coverage Added new test file auth_sts_v4_test.go with comprehensive tests for the STS session token authorization fix: 1. TestAuthorizeWithIAMSessionTokenExtraction: - Verifies X-SeaweedFS-Session-Token is extracted from JWT auth headers - Verifies X-Amz-Security-Token is extracted from V4 STS auth headers - Verifies X-Amz-Security-Token is extracted from query parameters (presigned URLs) - Verifies JWT tokens take precedence when both are present - Regression test for the bug where V4 STS tokens were not being passed to authorization 2. TestSTSSessionTokenIntoCredentials: - Verifies STS credentials have all required fields (AccessKeyId, SecretAccessKey, SessionToken) - Verifies deterministic generation from sessionId (same sessionId = same credentials) - Verifies different sessionIds produce different credentials - Critical for signature verification: same session must regenerate same secret key 3. TestActionConstantsForV4Auth: - Verifies S3 action constants are available for authorization checks - Ensures ACTION_READ, ACTION_WRITE, etc. are properly defined These tests ensure that: - V4 Signature auth with STS tokens properly extracts and uses session tokens - Session tokens are prioritized correctly (JWT > X-Amz-Security-Token header > query param) - STS credentials are deterministically generated for signature verification - The fix for passing STS session tokens to authorization is properly covered All 3 test functions pass (6 test cases total). * refactor(s3api): improve code quality and performance - Rename authorization path constants to avoid conflict with existing authType enum - Replace nested if/else with clean switch statement in authorizeWithIAM() - Add determineIAMAuthPath() helper for clearer intent and testability - Optimize key counting in auth_signature_v4.go: remove unnecessary slice allocation - Fix timing assertion in session_claims_test.go: use WithinDuration for symmetric tolerance These changes improve code readability, maintainability, and performance while maintaining full backward compatibility and test coverage. * refactor(s3api): use typed iamAuthPath for authorization path constants - Define iamAuthPath as a named string type (similar to existing authType enum) - Update constants to use explicit type: iamAuthPathJWT, iamAuthPathSTS_V4, etc. - Update determineIAMAuthPath() to return typed iamAuthPath - Improves type safety and prevents accidental string value misuse |
||
|
Chris LuandGitHub
|
c405ff1374 |
feat(iam): add TLS configuration support for OIDC provider (#7929)
* feat(iam): add TLS configuration support for OIDC provider Adds tlsCaCert and tlsInsecureSkipVerify options to OIDC provider configuration to allow using custom CA certificates and skipping verification in development environments. * fix: use SystemCertPool for custom CA and add security warning - Use x509.SystemCertPool() to preserve trust in public CAs - Add warning log when TLSInsecureSkipVerify is enabled - Addresses code review feedback from gemini-code-assist * docs: enhance TLS configuration field documentation - Add explicit warning about TLSInsecureSkipVerify production usage - Clarify TLSCACert is for custom/self-signed certificates * security: enforce TLS 1.2 minimum version - Set MinVersion to TLS 1.2 to prevent downgrade attacks - Ensures secure communication with OIDC providers * security: validate CA cert path is absolute - Add filepath.IsAbs check before reading CA certificate - Prevents reading unintended files from relative paths - Fail fast on misconfigured paths |