From d57de6dc207283071280e8ec5f955a4fe98ce464 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 19 May 2026 13:03:30 -0700 Subject: [PATCH] fix(s3): keep anonymous access working with EnableIam default (fixes #9557) (#9567) 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. --- weed/s3api/auth_credentials.go | 51 +++++++++++++++++++++++--------- weed/s3api/iam_optional_test.go | 29 ++++++++++++++++++ weed/s3api/s3_end_to_end_test.go | 4 ++- weed/s3api/s3api_server.go | 10 ++++++- 4 files changed, 78 insertions(+), 16 deletions(-) diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index cc8e6d1d7..4f8842998 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -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) } } diff --git a/weed/s3api/iam_optional_test.go b/weed/s3api/iam_optional_test.go index 4d35e4df9..109d43579 100644 --- a/weed/s3api/iam_optional_test.go +++ b/weed/s3api/iam_optional_test.go @@ -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") +} diff --git a/weed/s3api/s3_end_to_end_test.go b/weed/s3api/s3_end_to_end_test.go index 15f1c33d5..958323f7b 100644 --- a/weed/s3api/s3_end_to_end_test.go +++ b/weed/s3api/s3_end_to_end_test.go @@ -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") diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 8056da6e5..eb3956a53 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -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 {