mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-21 06:36:54 +00:00
fix(s3): keep anonymous access working with EnableIam default `docker run seaweedfs` (and `weed mini` with no config) start with EnableIam=true but no IAM config file and no identities. The advanced-IAM init path was failing in 4.25 because of the missing STS signing key, which masked a latent bug: SetIAMIntegration unconditionally flipped isAuthEnabled to true, and isEnabled() also treated a non-nil iamIntegration as auth-on. Once the mini SSE-S3 KEK landed in 4.26 the STS fallback started succeeding, the integration got installed end to end, and every anonymous S3 request bounced as AccessDenied. Separate the two concerns: SetIAMIntegration just plumbs in the OIDC / embedded-IAM machinery, and a new EnableAuthEnforcement opts in to enforcement. The startup path calls it only when -s3.iam.config is actually provided, so operators with explicit IAM configs still get auth (preserves #7726). isEnabled() now reads isAuthEnabled only.
This commit is contained in:
@@ -1042,18 +1042,22 @@ func (iam *IdentityAccessManagement) UpsertIdentity(ident *iam_pb.Identity) erro
|
||||
|
||||
// isEnabled reports whether S3 auth should be enforced for this server.
|
||||
//
|
||||
// Auth is considered enabled if either:
|
||||
// - we have any locally managed identities/credentials (iam.isAuthEnabled), or
|
||||
// - an external IAM integration has been configured (iam.iamIntegration != nil).
|
||||
// Driven solely by isAuthEnabled, which is set when:
|
||||
// - any locally managed identities/credentials are loaded (file/filer/env), or
|
||||
// - the operator passes -s3.iam.config, which triggers EnableAuthEnforcement
|
||||
// at startup time even before any identities sync in.
|
||||
//
|
||||
// The iamIntegration check is intentionally included so that when an external
|
||||
// IAM provider is configured (and the server relies solely on it), auth is
|
||||
// still treated as enabled even if there are no local identities yet or
|
||||
// before any sync logic flips isAuthEnabled to true. Removing this check or
|
||||
// relying only on isAuthEnabled would change when auth is enforced and could
|
||||
// unintentionally allow unauthenticated access in integration-only setups.
|
||||
// Earlier versions also treated `iam.iamIntegration != nil` as a signal to
|
||||
// enforce auth, on the assumption that a non-nil integration implied an
|
||||
// explicitly configured external IAM provider. That assumption broke once
|
||||
// EnableIam (mini's default) started initialising the integration with empty
|
||||
// defaults so the embedded IAM API and OIDC-subscribe paths have somewhere
|
||||
// to plug in. With no signing key and no identities, that path still forced
|
||||
// auth on and rejected every anonymous request to `docker run seaweedfs`
|
||||
// (#9557). The fix is to keep the integration available without flipping
|
||||
// enforcement; explicit setups call EnableAuthEnforcement themselves.
|
||||
func (iam *IdentityAccessManagement) isEnabled() bool {
|
||||
return iam.isAuthEnabled || iam.iamIntegration != nil
|
||||
return iam.isAuthEnabled
|
||||
}
|
||||
|
||||
func (iam *IdentityAccessManagement) updateAuthenticationState(identitiesCount int) bool {
|
||||
@@ -1954,15 +1958,34 @@ func (iam *IdentityAccessManagement) initializeKMSFromJSON(configContent []byte)
|
||||
return kms.LoadKMSFromConfig(kmsVal)
|
||||
}
|
||||
|
||||
// SetIAMIntegration sets the IAM integration for advanced authentication and authorization
|
||||
// SetIAMIntegration sets the IAM integration for advanced authentication and authorization.
|
||||
//
|
||||
// This does NOT flip isAuthEnabled on its own. The advanced IAM machinery is
|
||||
// initialised unconditionally when EnableIam is set (mini's default), even
|
||||
// without an IAM config file, so that the embedded IAM API and OIDC-provider
|
||||
// subscribe paths have somewhere to plug in. In that mode there are no roles,
|
||||
// providers or identities yet, so the legacy "no credentials = allow all"
|
||||
// startup behavior must be preserved — otherwise `docker run seaweedfs` (which
|
||||
// starts `weed mini` with no config) rejects every anonymous request.
|
||||
//
|
||||
// Callers that genuinely require authentication enforcement — an explicit
|
||||
// -s3.iam.config file, or identities loaded from file/filer/env — flip
|
||||
// isAuthEnabled themselves via EnableAuthEnforcement / updateAuthenticationState.
|
||||
func (iam *IdentityAccessManagement) SetIAMIntegration(integration *S3IAMIntegration) {
|
||||
iam.m.Lock()
|
||||
defer iam.m.Unlock()
|
||||
iam.iamIntegration = integration
|
||||
// When IAM integration is configured, authentication must be enabled
|
||||
// to ensure requests go through proper auth checks
|
||||
if integration != nil {
|
||||
}
|
||||
|
||||
// EnableAuthEnforcement turns on the auth-required mode unconditionally. Use
|
||||
// from setup paths that have evidence the operator intends to enforce auth
|
||||
// (explicit -s3.iam.config file, etc.) even before any identities are loaded.
|
||||
func (iam *IdentityAccessManagement) EnableAuthEnforcement() {
|
||||
iam.m.Lock()
|
||||
defer iam.m.Unlock()
|
||||
if !iam.isAuthEnabled {
|
||||
iam.isAuthEnabled = true
|
||||
hasAnyIdentity.Store(true)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -45,3 +45,32 @@ func TestLoadIAMManagerFromConfig_EmptyConfigWithFallbackKey(t *testing.T) {
|
||||
_, found := iamManager.LookupAnonymous()
|
||||
assert.False(t, found, "Anonymous identity should not be found when not explicitly configured")
|
||||
}
|
||||
|
||||
// TestSetIAMIntegrationKeepsAuthDisabledWithoutConfig is a regression test for
|
||||
// issue #9557. The `weed mini` defaults (and the bare `docker run seaweedfs`
|
||||
// image) start with EnableIam=true but no IAM config file and no identities.
|
||||
// The advanced-IAM init path used to also flip isAuthEnabled to true via
|
||||
// SetIAMIntegration, which then rejected every anonymous request as
|
||||
// AccessDenied — breaking out-of-the-box S3 access. Setting an integration
|
||||
// must not, on its own, enable auth enforcement; explicit configs use
|
||||
// EnableAuthEnforcement to opt in.
|
||||
func TestSetIAMIntegrationKeepsAuthDisabledWithoutConfig(t *testing.T) {
|
||||
resetMemoryStore()
|
||||
|
||||
option := &S3ApiServerOption{
|
||||
EnableIam: true,
|
||||
}
|
||||
iam := NewIdentityAccessManagementWithStore(option, nil, "memory")
|
||||
|
||||
// Simulate an integration object being plugged in (the constructor in
|
||||
// s3api_server.go does this when EnableIam=true, even with no config file).
|
||||
// We only care that auth stays off — the integration value itself is opaque.
|
||||
iam.SetIAMIntegration(&S3IAMIntegration{})
|
||||
|
||||
assert.False(t, iam.isEnabled(), "Auth must stay disabled when no identities and no IamConfig are configured")
|
||||
|
||||
// And EnableAuthEnforcement does flip it on — this is what the startup
|
||||
// path runs when the operator explicitly passes -s3.iam.config.
|
||||
iam.EnableAuthEnforcement()
|
||||
assert.True(t, iam.isEnabled(), "EnableAuthEnforcement must turn auth on")
|
||||
}
|
||||
|
||||
@@ -928,8 +928,10 @@ func TestS3IAMOnlyModeRejectsAnonymous(t *testing.T) {
|
||||
s3IAMIntegration := NewS3IAMIntegration(iamManager, "localhost:8888")
|
||||
require.NotNil(t, s3IAMIntegration)
|
||||
|
||||
// Set IAM integration - this should enable auth
|
||||
// Set IAM integration and explicitly enforce auth, matching what the
|
||||
// startup path does when an operator passes -s3.iam.config (#9557).
|
||||
iam.SetIAMIntegration(s3IAMIntegration)
|
||||
iam.EnableAuthEnforcement()
|
||||
|
||||
// Verify auth is enabled
|
||||
require.True(t, iam.isEnabled(), "Auth must be enabled when IAM integration is configured")
|
||||
|
||||
@@ -317,8 +317,16 @@ func NewS3ApiServerWithStore(router *mux.Router, option *S3ApiServerOption, expl
|
||||
// Set IAM integration in server
|
||||
s3ApiServer.iamIntegration = s3iam
|
||||
|
||||
// Set the integration in the traditional IAM for compatibility
|
||||
// Set the integration in the traditional IAM for compatibility.
|
||||
// SetIAMIntegration no longer auto-enables auth — see the function comment.
|
||||
// Only force isAuthEnabled when the operator actually pointed us at an
|
||||
// IAM config file. Without one, EnableIam is the implicit mini default
|
||||
// and we must keep the "no credentials = allow all" startup behavior so
|
||||
// `docker run seaweedfs` works out of the box (fixes #9557).
|
||||
iam.SetIAMIntegration(s3iam)
|
||||
if option.IamConfig != "" {
|
||||
iam.EnableAuthEnforcement()
|
||||
}
|
||||
|
||||
// Initialize STS HTTP handlers for AssumeRoleWithWebIdentity endpoint
|
||||
if stsService := iamManager.GetSTSService(); stsService != nil {
|
||||
|
||||
Reference in New Issue
Block a user