From 6ccda3e8097f81a5d4b68cff8b7162586e2269de Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 19 Mar 2026 18:10:20 -0700 Subject: [PATCH] fix(s3): allow deleting the anonymous user from admin webui (#8706) Remove the block that prevented deleting the "anonymous" identity and stop auto-creating it when absent. If no anonymous identity exists (or it is disabled), LookupAnonymous returns not-found and both auth paths return ErrAccessDenied for anonymous requests. To enable anonymous access, explicitly create the "anonymous" user. To revoke it, delete the user like any other identity. Closes #8694 --- weed/admin/dash/user_management.go | 7 ---- weed/s3api/auth_credentials.go | 67 ++++++++++++++++-------------- weed/s3api/iam_optional_test.go | 43 ++++++------------- 3 files changed, 47 insertions(+), 70 deletions(-) diff --git a/weed/admin/dash/user_management.go b/weed/admin/dash/user_management.go index 685bf4937..7832e501f 100644 --- a/weed/admin/dash/user_management.go +++ b/weed/admin/dash/user_management.go @@ -153,13 +153,6 @@ func (s *AdminServer) DeleteObjectStoreUser(username string) error { return fmt.Errorf("credential manager not available") } - // Prevent deletion of the anonymous identity — it is a system identity - // used for unauthenticated S3 access. Removing it would break anonymous - // request handling in the IAM layer. - if username == "anonymous" { - return fmt.Errorf("cannot delete the system identity 'anonymous'") - } - ctx := context.Background() // Delete user using credential manager diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 04309add5..a09f6c6d4 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -250,24 +250,6 @@ func NewIdentityAccessManagementWithStore(option *S3ApiServerOption, filerClient iam.stopChan = make(chan struct{}) iam.grpcDialOption = option.GrpcDialOption - // Initialize default anonymous identity - // This ensures consistent behavior for anonymous access: - // 1. In simple auth mode (no IAM integration): - // - lookupAnonymous returns this identity - // - VerifyActionPermission checks actions (which are empty) -> Denies access - // - This preserves the secure-by-default behavior for simple auth - // 2. In advanced IAM mode (with Policy Engine): - // - lookupAnonymous returns this identity - // - VerifyActionPermission proceeds to Policy Engine - // - Policy Engine evaluates against policies (DefaultEffect=Allow if no config) - // - This enables the flexible "Open by Default" for zero-config startup - iam.identityAnonymous = &Identity{ - Name: "anonymous", - Account: &AccountAnonymous, - Actions: []Action{}, - IsStatic: true, - } - // First, try to load configurations from file or filer startConfigFile := option.Config if startConfigFile == "" { @@ -657,16 +639,6 @@ func (iam *IdentityAccessManagement) ReplaceS3ApiConfiguration(config *iam_pb.S3 } } - // Ensure anonymous identity exists - if identityAnonymous == nil { - identityAnonymous = &Identity{ - Name: "anonymous", - Account: accounts[AccountAnonymous.Id], - Actions: []Action{}, - IsStatic: true, - } - } - // atomically switch iam.identities = identities iam.identityAnonymous = identityAnonymous @@ -921,6 +893,35 @@ func (iam *IdentityAccessManagement) MergeS3ApiConfiguration(config *iam_pb.S3Ap glog.V(3).Infof("Loaded service account %s for dynamic parent %s (expiration: %d)", sa.Id, sa.ParentUser, sa.Expiration) } + // If the anonymous identity was carried over from the previous state but is + // no longer present in the credential-manager snapshot, clear it so that + // deleted anonymous users do not persist across merges. + if identityAnonymous != nil && !identityAnonymous.IsStatic { + stillPresent := false + for _, ident := range config.Identities { + if ident.Name == s3_constants.AccountAnonymousId { + stillPresent = true + break + } + } + if !stillPresent { + // Remove from identities slice and maps + for i, ident := range identities { + if ident == identityAnonymous { + identities = append(identities[:i], identities[i+1:]...) + break + } + } + delete(nameToIdentity, identityAnonymous.Name) + for _, cred := range identityAnonymous.Credentials { + if accessKeyIdent[cred.AccessKey] == identityAnonymous { + delete(accessKeyIdent, cred.AccessKey) + } + } + identityAnonymous = nil + } + } + for _, policy := range config.Policies { policies[policy.Name] = policy } @@ -1131,11 +1132,11 @@ func (iam *IdentityAccessManagement) LookupByAccessKey(accessKey string) (identi return iam.lookupByAccessKey(accessKey) } -// LookupAnonymous returns the anonymous identity if it exists +// LookupAnonymous returns the anonymous identity if it exists and is not disabled. func (iam *IdentityAccessManagement) LookupAnonymous() (identity *Identity, found bool) { iam.m.RLock() defer iam.m.RUnlock() - if iam.identityAnonymous != nil { + if iam.identityAnonymous != nil && !iam.identityAnonymous.Disabled { return iam.identityAnonymous, true } return nil, false @@ -1450,8 +1451,10 @@ func (iam *IdentityAccessManagement) AuthSignatureOnly(r *http.Request) (*Identi return identity, s3err.ErrNotImplemented } case authTypeAnonymous: - // Anonymous users can be authenticated, but authorization is handled separately - return iam.identityAnonymous, s3err.ErrNone + if ident, found := iam.LookupAnonymous(); found { + return ident, s3err.ErrNone + } + return nil, s3err.ErrAccessDenied default: return identity, s3err.ErrNotImplemented } diff --git a/weed/s3api/iam_optional_test.go b/weed/s3api/iam_optional_test.go index 087bfd675..583b14791 100644 --- a/weed/s3api/iam_optional_test.go +++ b/weed/s3api/iam_optional_test.go @@ -4,36 +4,21 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -func TestLoadIAMManagerFromConfig_OptionalConfig(t *testing.T) { - // Mock dependencies - filerAddressProvider := func() string { return "localhost:8888" } - getFilerSigningKey := func() string { return "test-signing-key" } - - // Test Case 1: Empty config path should load defaults - iamManager, err := loadIAMManagerFromConfig("", filerAddressProvider, getFilerSigningKey) - require.NoError(t, err) - require.NotNil(t, iamManager) - - // Verify STS Service is initialized with defaults - stsService := iamManager.GetSTSService() - assert.NotNil(t, stsService) - - // Verify defaults are applied - // Since we can't easily access the internal config of stsService, - // we rely on the fact that initialization succeeded without error. - // We can also verify that the policy engine uses memory store by default. - - // Verify Policy Engine is initialized with defaults (Memory store, Deny effect) - // Again, internal state might be hard to access directly, but successful init implies defaults worked. +func TestLoadIAMManagerWithNoConfig(t *testing.T) { + // Verify that IAM can be initialized without any config + option := &S3ApiServerOption{ + Config: "", + } + iamManager := NewIdentityAccessManagementWithStore(option, nil, "memory") + assert.NotNil(t, iamManager) + // Internal state might be hard to access directly, but successful init implies defaults worked. } func TestLoadIAMManagerFromConfig_EmptyConfigWithFallbackKey(t *testing.T) { - // Mock dependencies where getFilerSigningKey returns empty, forcing fallback logic - // Initialize IAM with empty config (should trigger defaults) - // We pass empty string for config file path + // Initialize IAM with empty config — no anonymous identity is configured, + // so LookupAnonymous should return not-found. option := &S3ApiServerOption{ Config: "", IamConfig: "", @@ -41,10 +26,6 @@ func TestLoadIAMManagerFromConfig_EmptyConfigWithFallbackKey(t *testing.T) { } iamManager := NewIdentityAccessManagementWithStore(option, nil, "memory") - // Verify identityAnonymous is initialized - // This confirms the fix for anonymous access in zero-config mode - anonIdentity, found := iamManager.LookupAnonymous() - assert.True(t, found, "Anonymous identity should be found by default") - assert.NotNil(t, anonIdentity, "Anonymous identity should not be nil") - assert.Equal(t, "anonymous", anonIdentity.Name) + _, found := iamManager.LookupAnonymous() + assert.False(t, found, "Anonymous identity should not be found when not explicitly configured") }