mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-01 04:05:54 +00:00
* s3/iam: persist IAM-managed OIDC providers in the filer, and trust them after a restart
The S3 server's IAM config loader never read the documented
`oidcProviderStore` key, so the OIDC provider store was always in memory:
a provider created with CreateOpenIDConnectProvider lived in one gateway's
process, was lost on restart, and was never seen by peers. The
/etc/iam/oidc-providers metadata subscription refreshed from that empty
in-memory store.
- Read `oidcProviderStore` and pass it to the IAM manager. With an IAM
config file the default stays memory. With no config file (zero-config
IAM, as `weed filer -s3` and operator-managed clusters run) it defaults
to the filer: there is nothing static to shadow, and providers created at
runtime otherwise vanish on restart.
- With a store that outlives the process, load the STS runtime view from it
at startup, so providers created on an earlier boot or on a peer are
trusted without waiting for the next mutation.
- If the store cannot be read at startup (a filer not up yet), the load is
retried in the background with backoff until it succeeds: the metadata
subscription reports only later changes, so providers already stored would
otherwise stay unknown to STS until one of them changed.
- Mark records mirrored from STS.Providers as `source: static-config`, and
at startup delete such records whose provider has left the config, so
removing a provider from the config file still revokes it. Records created
through the IAM API are never pruned.
- The filer store reported every failed lookup, an unreachable filer
included, as ErrOIDCProviderNotFound, which CreateOIDCProvider reads as
"free to create". Only a confirmed absence is now not-found.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* s3/iam: keep config-file OIDC providers out of a persistent store
Review of the previous commit found that mirroring the IAM config file's
providers into a persistent store, and pruning them when they leave the
file, breaks as soon as S3 servers share a filer:
- a server prunes stored config-file providers its own file does not list,
including ones a peer's file still defines (a zero-config server prunes
them all);
- mirroring overwrites an API-created provider with the same ARN and marks
it config-owned, so a later prune deletes it;
- a failed mirror write or a failed prune leaves a stale record trusted;
- a mirrored record is loaded into STS at startup as an IAM-managed provider
and shadows the config-file provider, dropping the settings a record does
not carry (jwksUri, roleMapping, policyClaim, ...).
A persistent store now never receives the config file's providers. STS keeps
serving them from its static configuration, as it always has; the IAM API
lists and returns them from memory, refuses to change or delete them
(UnmodifiableEntity; change them in the file) and to create another provider
with their ARN (EntityAlreadyExists). The store holds only providers created
through the IAM API, and those are what startup loads into STS. There is
nothing to prune, so the source marker is gone. An in-memory store keeps its
behaviour: the config file's providers are records in it, as before.
buildOIDCProviderFromRecord also carries PolicyClaim and
AllowedPrincipalTagKeys now; they were dropped whenever an API-created
provider was loaded into STS.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* s3/iam: send UnmodifiableEntity as a 400, not an internal error
The IAM API's error writer had no case for UnmodifiableEntity, which the
previous commit returns for a change to a config-file provider, so it went
out as a 500 ServiceFailure that clients retry. AWS sends it as a 400.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* s3/iam: document stored-over-config precedence, drop invented CreateDate, cancel superseded retries
Follow-ups from review of b881982d2:
- A provider stored under the same ARN as a config-file provider takes
precedence in the IAM API, matching STS, which already prefers
IAM-managed providers so that an API call can shadow a bootstrap entry.
Deleting the stored provider brings the config-file one back. This was
already the behaviour; it is now documented and tested.
- A config-file provider no longer reports its server's start time as
CreateDate, which changed on every restart; GetOpenIDConnectProvider now
omits the date for it. An in-memory store still stamps its copies at load,
as before.
- The startup retry runs under a cancellable context, is cancelled when
another store is installed, and retries the store it was started for
rather than reading the manager's field, so replacing the store neither
leaves the old retry running nor races with it (go test -race).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* s3/iam: serialize OIDC provider refreshes so an older snapshot cannot restore a deleted provider
Refreshes run concurrently: after an IAM API change, on a peer's change
and in the startup retry. Each lists the store and then hands STS the
result, so a refresh that listed before a DeleteOIDCProvider could finish
after that call's own refresh and keep the deleted provider trusted until
the next change. Refreshes now hold a lock from the read to the hand-off,
and a startup retry cancelled by installing another store drops its
snapshot instead of applying it.
The retry-cancellation test waits for the retry by polling instead of a
fixed sleep.
* s3/iam: route SetOIDCProviderStore through installOIDCProviderStore
A store installed after Initialize skipped the static-provider overlay
and startup hydration: config-file providers disappeared from the IAM
API, ErrOIDCProviderStatic no longer protected them, and stored
providers were never trusted until the next mutation or peer event.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
309 lines
9.3 KiB
Go
309 lines
9.3 KiB
Go
package s3api
|
|
|
|
import (
|
|
"context"
|
|
"os"
|
|
"path/filepath"
|
|
"testing"
|
|
|
|
"github.com/seaweedfs/seaweedfs/weed/iam/integration"
|
|
"github.com/stretchr/testify/assert"
|
|
)
|
|
|
|
func TestLoadIAMManagerFromConfig_Defaults(t *testing.T) {
|
|
// Create a temporary config file with minimal content (just policy)
|
|
tmpDir := t.TempDir()
|
|
configPath := filepath.Join(tmpDir, "iam_config.json")
|
|
|
|
configContent := `{
|
|
"sts": {
|
|
"providers": []
|
|
},
|
|
"policy": {
|
|
"storeType": "memory",
|
|
"defaultEffect": "Allow"
|
|
}
|
|
}`
|
|
|
|
err := os.WriteFile(configPath, []byte(configContent), 0644)
|
|
assert.NoError(t, err)
|
|
|
|
// dummy filer address provider
|
|
filerProvider := func() string { return "localhost:8888" }
|
|
defaultSigningKeyProvider := func() string { return "default-secure-signing-key" }
|
|
|
|
// Load the manager
|
|
manager, err := loadIAMManagerFromConfig(configPath, filerProvider, defaultSigningKeyProvider)
|
|
assert.NoError(t, err)
|
|
assert.NotNil(t, manager)
|
|
}
|
|
|
|
func TestLoadIAMManagerFromConfig_Overrides(t *testing.T) {
|
|
// Create a temporary config file with EXPLICIT values
|
|
tmpDir := t.TempDir()
|
|
configPath := filepath.Join(tmpDir, "iam_config_explicit.json")
|
|
|
|
configContent := `{
|
|
"sts": {
|
|
"tokenDuration": "2h",
|
|
"maxSessionLength": "24h",
|
|
"issuer": "custom-issuer",
|
|
"signingKey": "ZXhwbGljaXQtc2lnbmluZy1rZXktMTIzNDU="
|
|
},
|
|
"policy": {
|
|
"storeType": "memory",
|
|
"defaultEffect": "Allow"
|
|
}
|
|
}`
|
|
// Base64 encoded "explicit-signing-key-12345" is "ZXhwbGljaXQtc2lnbmluZy1rZXktMTIzNDU="
|
|
|
|
err := os.WriteFile(configPath, []byte(configContent), 0644)
|
|
assert.NoError(t, err)
|
|
|
|
filerProvider := func() string { return "localhost:8888" }
|
|
defaultSigningKeyProvider := func() string { return "default-secure-signing-key" }
|
|
|
|
// Load
|
|
manager, err := loadIAMManagerFromConfig(configPath, filerProvider, defaultSigningKeyProvider)
|
|
assert.NoError(t, err)
|
|
assert.NotNil(t, manager)
|
|
}
|
|
|
|
func TestLoadIAMManagerFromConfig_PartialDefaults(t *testing.T) {
|
|
// Test that partial configs (e.g. providing SigningKey but not Duration) work
|
|
tmpDir := t.TempDir()
|
|
configPath := filepath.Join(tmpDir, "iam_config_partial.json")
|
|
|
|
// Signing key provided in JSON, others missing
|
|
configContent := `{
|
|
"sts": {
|
|
"signingKey": "anNvbi1wcm92aWRlZC1rZXktMTIzNDU="
|
|
},
|
|
"policy": {
|
|
"storeType": "memory",
|
|
"defaultEffect": "Allow"
|
|
}
|
|
}`
|
|
|
|
err := os.WriteFile(configPath, []byte(configContent), 0644)
|
|
assert.NoError(t, err)
|
|
|
|
filerProvider := func() string { return "localhost:8888" }
|
|
// Default signing key provided but should be IGNORED because JSON has one
|
|
defaultSigningKeyProvider := func() string { return "server-default-key-should-be-ignored" }
|
|
|
|
manager, err := loadIAMManagerFromConfig(configPath, filerProvider, defaultSigningKeyProvider)
|
|
assert.NoError(t, err)
|
|
assert.NotNil(t, manager)
|
|
}
|
|
|
|
func TestLoadIAMManagerFromConfig_ExplicitEmptyKey(t *testing.T) {
|
|
// Test that if JSON has empty signing key string, it still falls back
|
|
tmpDir := t.TempDir()
|
|
configPath := filepath.Join(tmpDir, "iam_config_empty_key.json")
|
|
|
|
// Signing key explicitly empty
|
|
configContent := `{
|
|
"sts": {
|
|
"signingKey": ""
|
|
},
|
|
"policy": {
|
|
"storeType": "memory",
|
|
"defaultEffect": "Allow"
|
|
}
|
|
}`
|
|
|
|
err := os.WriteFile(configPath, []byte(configContent), 0644)
|
|
assert.NoError(t, err)
|
|
|
|
filerProvider := func() string { return "localhost:8888" }
|
|
defaultSigningKeyProvider := func() string { return "fallback-key-should-be-used" }
|
|
|
|
manager, err := loadIAMManagerFromConfig(configPath, filerProvider, defaultSigningKeyProvider)
|
|
assert.NoError(t, err)
|
|
assert.NotNil(t, manager)
|
|
}
|
|
|
|
func TestLoadIAMManagerFromConfig_MissingKeyError(t *testing.T) {
|
|
// Test that if BOTH keys are empty, it fails with a clear error
|
|
tmpDir := t.TempDir()
|
|
configPath := filepath.Join(tmpDir, "iam_config_all_empty.json")
|
|
|
|
// Signing key explicitly empty in JSON
|
|
configContent := `{
|
|
"sts": {
|
|
"signingKey": ""
|
|
},
|
|
"policy": {
|
|
"storeType": "memory",
|
|
"defaultEffect": "Allow"
|
|
}
|
|
}`
|
|
|
|
err := os.WriteFile(configPath, []byte(configContent), 0644)
|
|
assert.NoError(t, err)
|
|
|
|
filerProvider := func() string { return "localhost:8888" }
|
|
defaultSigningKeyProvider := func() string { return "" } // Empty default too
|
|
|
|
// Ensure no SSE-S3 key interferes (global state in tests is tricky, but let's assume clean state or no mock)
|
|
// Ideally we would mock GetSSES3KeyManager().GetMasterKey() but it's a global singleton.
|
|
// For this unit test, if the global key manager has no key, it should fail.
|
|
|
|
_, err = loadIAMManagerFromConfig(configPath, filerProvider, defaultSigningKeyProvider)
|
|
|
|
// Should return a clear error
|
|
assert.Error(t, err)
|
|
assert.Contains(t, err.Error(), "no signing key found for STS service")
|
|
}
|
|
|
|
func TestLoadIAMManagerFromConfig_ExplicitFileDefaultsToDeny(t *testing.T) {
|
|
tmpDir := t.TempDir()
|
|
configPath := filepath.Join(tmpDir, "iam_config_implicit_policy_defaults.json")
|
|
|
|
// Explicit config file with no policy.defaultEffect should default to Deny.
|
|
configContent := `{
|
|
"sts": {
|
|
"issuer": "explicit-config"
|
|
}
|
|
}`
|
|
|
|
err := os.WriteFile(configPath, []byte(configContent), 0644)
|
|
assert.NoError(t, err)
|
|
|
|
filerProvider := func() string { return "localhost:8888" }
|
|
defaultSigningKeyProvider := func() string { return "fallback-key-for-explicit-config" }
|
|
|
|
manager, err := loadIAMManagerFromConfig(configPath, filerProvider, defaultSigningKeyProvider)
|
|
assert.NoError(t, err)
|
|
assert.NotNil(t, manager)
|
|
assert.False(t, manager.DefaultAllow())
|
|
}
|
|
|
|
func TestLoadIAMManagerFromConfig_NoFileDefaultsToAllow(t *testing.T) {
|
|
// No explicit IAM file should preserve zero-config startup behavior.
|
|
filerProvider := func() string { return "localhost:8888" }
|
|
defaultSigningKeyProvider := func() string { return "fallback-key-for-zero-config" }
|
|
|
|
manager, err := loadIAMManagerFromConfig("", filerProvider, defaultSigningKeyProvider)
|
|
assert.NoError(t, err)
|
|
assert.NotNil(t, manager)
|
|
assert.True(t, manager.DefaultAllow())
|
|
}
|
|
|
|
func TestLoadIAMManagerFromConfig_ExplicitFileEnforcesUserScopedPolicy(t *testing.T) {
|
|
tmpDir := t.TempDir()
|
|
configPath := filepath.Join(tmpDir, "iam_regression_8366.json")
|
|
|
|
// Regression coverage for #8366:
|
|
// with explicit IAM config and omitted policy.defaultEffect, unrestricted bucket creation
|
|
// must NOT be allowed.
|
|
configContent := `{
|
|
"sts": {
|
|
"issuer": "seaweedfs-sts"
|
|
},
|
|
"policies": [
|
|
{
|
|
"name": "S3UserPolicy",
|
|
"document": {
|
|
"Version": "2012-10-17",
|
|
"Statement": [
|
|
{
|
|
"Effect": "Allow",
|
|
"Action": ["s3:ListAllMyBuckets"],
|
|
"Resource": ["arn:aws:s3:::*"]
|
|
},
|
|
{
|
|
"Effect": "Allow",
|
|
"Action": ["s3:*"],
|
|
"Resource": [
|
|
"arn:aws:s3:::user-${jwt:preferred_username}",
|
|
"arn:aws:s3:::user-${jwt:preferred_username}/*"
|
|
]
|
|
}
|
|
]
|
|
}
|
|
}
|
|
],
|
|
"roles": [
|
|
{
|
|
"roleName": "S3UserRole",
|
|
"roleArn": "arn:aws:iam::role/S3UserRole",
|
|
"attachedPolicies": ["S3UserPolicy"],
|
|
"trustPolicy": {
|
|
"Version": "2012-10-17",
|
|
"Statement": [
|
|
{
|
|
"Effect": "Allow",
|
|
"Principal": {"Federated": "*"},
|
|
"Action": ["sts:AssumeRoleWithWebIdentity"]
|
|
}
|
|
]
|
|
}
|
|
}
|
|
]
|
|
}`
|
|
|
|
err := os.WriteFile(configPath, []byte(configContent), 0644)
|
|
assert.NoError(t, err)
|
|
|
|
filerProvider := func() string { return "localhost:8888" }
|
|
defaultSigningKeyProvider := func() string { return "fallback-key-for-regression-8366" }
|
|
|
|
manager, err := loadIAMManagerFromConfig(configPath, filerProvider, defaultSigningKeyProvider)
|
|
assert.NoError(t, err)
|
|
assert.NotNil(t, manager)
|
|
assert.False(t, manager.DefaultAllow())
|
|
|
|
ctx := context.Background()
|
|
principal := "arn:aws:sts::000000000000:assumed-role/S3UserRole/alice-session"
|
|
reqCtx := map[string]interface{}{"jwt:preferred_username": "alice"}
|
|
|
|
allowed, err := manager.IsActionAllowed(ctx, &integration.ActionRequest{
|
|
Principal: principal,
|
|
Action: "s3:CreateBucket",
|
|
Resource: "arn:aws:s3:::arbitrary-bucket",
|
|
RequestContext: reqCtx,
|
|
})
|
|
assert.NoError(t, err)
|
|
assert.False(t, allowed, "arbitrary bucket creation should be denied")
|
|
|
|
allowed, err = manager.IsActionAllowed(ctx, &integration.ActionRequest{
|
|
Principal: principal,
|
|
Action: "s3:CreateBucket",
|
|
Resource: "arn:aws:s3:::user-alice",
|
|
RequestContext: reqCtx,
|
|
})
|
|
assert.NoError(t, err)
|
|
assert.True(t, allowed, "user-scoped bucket creation should be allowed")
|
|
}
|
|
|
|
func TestLoadIAMManagerFromConfig_HonorsOIDCProviderStore(t *testing.T) {
|
|
// The documented oidcProviderStore key must reach the IAM manager; without
|
|
// it, providers created through the IAM API live in one gateway's memory.
|
|
cases := []struct {
|
|
name string
|
|
store string
|
|
filer bool
|
|
}{
|
|
{"absent keeps memory", ``, false},
|
|
{"filer persists", `,"oidcProviderStore":{"storeType":"filer"}`, true},
|
|
{"no config file persists", "no-file", true},
|
|
}
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
configPath := ""
|
|
if tc.store != "no-file" {
|
|
configPath = filepath.Join(t.TempDir(), "iam_config.json")
|
|
configContent := `{"sts":{"providers":[]},"policy":{"storeType":"memory","defaultEffect":"Deny"}` + tc.store + `}`
|
|
assert.NoError(t, os.WriteFile(configPath, []byte(configContent), 0644))
|
|
}
|
|
|
|
manager, err := loadIAMManagerFromConfig(configPath, func() string { return "localhost:8888" }, func() string { return "oidc-store-signing-key" })
|
|
assert.NoError(t, err)
|
|
_, isFiler := manager.GetOIDCProviderStore().(*integration.FilerOIDCProviderStore)
|
|
assert.Equal(t, tc.filer, isFiler)
|
|
})
|
|
}
|
|
}
|