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
This commit is contained in:
@@ -153,13 +153,6 @@ func (s *AdminServer) DeleteObjectStoreUser(username string) error {
|
|||||||
return fmt.Errorf("credential manager not available")
|
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()
|
ctx := context.Background()
|
||||||
|
|
||||||
// Delete user using credential manager
|
// Delete user using credential manager
|
||||||
|
|||||||
@@ -250,24 +250,6 @@ func NewIdentityAccessManagementWithStore(option *S3ApiServerOption, filerClient
|
|||||||
iam.stopChan = make(chan struct{})
|
iam.stopChan = make(chan struct{})
|
||||||
iam.grpcDialOption = option.GrpcDialOption
|
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
|
// First, try to load configurations from file or filer
|
||||||
startConfigFile := option.Config
|
startConfigFile := option.Config
|
||||||
if startConfigFile == "" {
|
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
|
// atomically switch
|
||||||
iam.identities = identities
|
iam.identities = identities
|
||||||
iam.identityAnonymous = identityAnonymous
|
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)
|
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 {
|
for _, policy := range config.Policies {
|
||||||
policies[policy.Name] = policy
|
policies[policy.Name] = policy
|
||||||
}
|
}
|
||||||
@@ -1131,11 +1132,11 @@ func (iam *IdentityAccessManagement) LookupByAccessKey(accessKey string) (identi
|
|||||||
return iam.lookupByAccessKey(accessKey)
|
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) {
|
func (iam *IdentityAccessManagement) LookupAnonymous() (identity *Identity, found bool) {
|
||||||
iam.m.RLock()
|
iam.m.RLock()
|
||||||
defer iam.m.RUnlock()
|
defer iam.m.RUnlock()
|
||||||
if iam.identityAnonymous != nil {
|
if iam.identityAnonymous != nil && !iam.identityAnonymous.Disabled {
|
||||||
return iam.identityAnonymous, true
|
return iam.identityAnonymous, true
|
||||||
}
|
}
|
||||||
return nil, false
|
return nil, false
|
||||||
@@ -1450,8 +1451,10 @@ func (iam *IdentityAccessManagement) AuthSignatureOnly(r *http.Request) (*Identi
|
|||||||
return identity, s3err.ErrNotImplemented
|
return identity, s3err.ErrNotImplemented
|
||||||
}
|
}
|
||||||
case authTypeAnonymous:
|
case authTypeAnonymous:
|
||||||
// Anonymous users can be authenticated, but authorization is handled separately
|
if ident, found := iam.LookupAnonymous(); found {
|
||||||
return iam.identityAnonymous, s3err.ErrNone
|
return ident, s3err.ErrNone
|
||||||
|
}
|
||||||
|
return nil, s3err.ErrAccessDenied
|
||||||
default:
|
default:
|
||||||
return identity, s3err.ErrNotImplemented
|
return identity, s3err.ErrNotImplemented
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,36 +4,21 @@ import (
|
|||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestLoadIAMManagerFromConfig_OptionalConfig(t *testing.T) {
|
func TestLoadIAMManagerWithNoConfig(t *testing.T) {
|
||||||
// Mock dependencies
|
// Verify that IAM can be initialized without any config
|
||||||
filerAddressProvider := func() string { return "localhost:8888" }
|
option := &S3ApiServerOption{
|
||||||
getFilerSigningKey := func() string { return "test-signing-key" }
|
Config: "",
|
||||||
|
}
|
||||||
// Test Case 1: Empty config path should load defaults
|
iamManager := NewIdentityAccessManagementWithStore(option, nil, "memory")
|
||||||
iamManager, err := loadIAMManagerFromConfig("", filerAddressProvider, getFilerSigningKey)
|
assert.NotNil(t, iamManager)
|
||||||
require.NoError(t, err)
|
// Internal state might be hard to access directly, but successful init implies defaults worked.
|
||||||
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 TestLoadIAMManagerFromConfig_EmptyConfigWithFallbackKey(t *testing.T) {
|
func TestLoadIAMManagerFromConfig_EmptyConfigWithFallbackKey(t *testing.T) {
|
||||||
// Mock dependencies where getFilerSigningKey returns empty, forcing fallback logic
|
// Initialize IAM with empty config — no anonymous identity is configured,
|
||||||
// Initialize IAM with empty config (should trigger defaults)
|
// so LookupAnonymous should return not-found.
|
||||||
// We pass empty string for config file path
|
|
||||||
option := &S3ApiServerOption{
|
option := &S3ApiServerOption{
|
||||||
Config: "",
|
Config: "",
|
||||||
IamConfig: "",
|
IamConfig: "",
|
||||||
@@ -41,10 +26,6 @@ func TestLoadIAMManagerFromConfig_EmptyConfigWithFallbackKey(t *testing.T) {
|
|||||||
}
|
}
|
||||||
iamManager := NewIdentityAccessManagementWithStore(option, nil, "memory")
|
iamManager := NewIdentityAccessManagementWithStore(option, nil, "memory")
|
||||||
|
|
||||||
// Verify identityAnonymous is initialized
|
_, found := iamManager.LookupAnonymous()
|
||||||
// This confirms the fix for anonymous access in zero-config mode
|
assert.False(t, found, "Anonymous identity should not be found when not explicitly configured")
|
||||||
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)
|
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user