fix(iam): deny IAM users with no policies instead of granting full access (#9317)

* fix(iam): deny IAM users with zero policies instead of falling through to DefaultEffect=Allow

A user created via the S3 IAM API with no policies attached was inheriting
full S3 access. With `weed s3 -iam` and no explicit IAM config, the policy
engine's DefaultEffect defaults to Allow for the in-memory zero-config path.
The "no matching statement" guard in IsActionAllowed only triggered when the
user already had at least one policy, so a fresh user with PolicyNames=[]
slipped through and got allow-all.

Track hasManagedSubject whenever the principal resolves to a registered user
or role (or PolicyNames are supplied directly) and deny on no-match. The
DefaultEffect=Allow fallback now only applies to truly unmanaged callers.

* test(iam): cover non-matching attached-policy case for managed-subject deny

* test(iam): cover role-with-empty-AttachedPolicies deny path

Sibling case to TestIsActionAllowed_RegisteredUserWithoutPoliciesIsDenied:
a managed role that resolves but has zero AttachedPolicies must also
fall through to deny under DefaultEffect=Allow, not inherit full access.
The fix already handles this branch via the hasManagedSubject flag;
this test pins the regression class so we don't lose coverage on
either side of the user/role split.

Addresses coderabbit nitpick on PR #9317.

weed/iam/integration/iam_manager.go

@@ -450,13 +450,20 @@ func (m *IAMManager) IsActionAllowed(ctx context.Context, request *ActionRequest
 
 	var baseResult *policy.EvaluationResult
 	var err error
-	subjectPolicyCount := 0
+	// hasManagedSubject is true once we've resolved the principal to a registered
+	// IAM user or role (or the caller has supplied PolicyNames directly). For a
+	// managed subject, "no matching statement" must deny — the DefaultEffect=Allow
+	// fallback is only meant for the unmanaged zero-config startup case.
+	hasManagedSubject := false
 
 	if isAdmin {
 		// Admin always has base access allowed
 		baseResult = &policy.EvaluationResult{Effect: policy.EffectAllow}
 	} else {
 		policies := request.PolicyNames
+		if len(policies) > 0 {
+			hasManagedSubject = true
+		}
 		if len(policies) == 0 {
 			// Extract role name from principal ARN
 			roleName := utils.ExtractRoleNameFromPrincipal(request.Principal)
@@ -472,6 +479,7 @@ func (m *IAMManager) IsActionAllowed(ctx context.Context, request *ActionRequest
 				if err != nil || user == nil {
 					return false, fmt.Errorf("user not found for principal: %s (user=%s)", request.Principal, userName)
 				}
+				hasManagedSubject = true
 				policies = user.GetPolicyNames()
 			} else {
 				// Get role definition
@@ -480,11 +488,10 @@ func (m *IAMManager) IsActionAllowed(ctx context.Context, request *ActionRequest
 					return false, fmt.Errorf("role not found: %s", roleName)
 				}
 
+				hasManagedSubject = true
 				policies = roleDef.AttachedPolicies
 			}
 		}
-		subjectPolicyCount = len(policies)
-
 		if bucketPolicyName != "" {
 			// Enforce an upper bound on the number of policies to avoid excessive allocations
 			if len(policies) >= maxPoliciesForEvaluation {
@@ -508,10 +515,11 @@ func (m *IAMManager) IsActionAllowed(ctx context.Context, request *ActionRequest
 	}
 
 	// Zero-config IAM uses DefaultEffect=Allow to preserve open-by-default behavior
-	// for requests without any subject policies. Once a user or role has attached
-	// policies, "no matching statement" must fall back to deny so the attachment
-	// actually scopes access.
-	if subjectPolicyCount > 0 && len(baseResult.MatchingStatements) == 0 {
+	// for requests without any subject policies. Once we resolve the principal to
+	// a registered IAM user or role (or the caller hands us policy names),
+	// "no matching statement" must fall back to deny — otherwise a freshly
+	// created user with zero policies would inherit full access.
+	if hasManagedSubject && len(baseResult.MatchingStatements) == 0 {
 		return false, nil
 	}
 

weed/iam/integration/iam_manager_no_policy_test.go

@@ -0,0 +1,145 @@
+package integration
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/seaweedfs/seaweedfs/weed/iam/policy"
+	"github.com/seaweedfs/seaweedfs/weed/iam/sts"
+	"github.com/seaweedfs/seaweedfs/weed/pb/iam_pb"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+type fakeUserStore struct {
+	users map[string]*iam_pb.Identity
+}
+
+func (f *fakeUserStore) GetUser(_ context.Context, name string) (*iam_pb.Identity, error) {
+	if u, ok := f.users[name]; ok {
+		return u, nil
+	}
+	return nil, nil
+}
+
+// TestIsActionAllowed_RegisteredUserWithoutPoliciesIsDenied is a regression test
+// for a security issue where a user created via the S3 IAM API with no policies
+// attached was granted access to every S3 action. The default-allow path was
+// intended for unmanaged zero-config startup, not for explicitly registered IAM
+// users with an empty policy set.
+func TestIsActionAllowed_RegisteredUserWithoutPoliciesIsDenied(t *testing.T) {
+	manager := NewIAMManager()
+	require.NoError(t, manager.Initialize(&IAMConfig{
+		STS: &sts.STSConfig{
+			TokenDuration:    sts.FlexibleDuration{Duration: time.Hour},
+			MaxSessionLength: sts.FlexibleDuration{Duration: 12 * time.Hour},
+			Issuer:           "test-sts",
+			SigningKey:       []byte("test-signing-key-32-characters-long"),
+		},
+		Policy: &policy.PolicyEngineConfig{
+			// Reproduce the zero-config setup: with no IAM config file, the
+			// loader picks DefaultEffect=Allow for in-memory mode.
+			DefaultEffect: string(policy.EffectAllow),
+			StoreType:     sts.StoreTypeMemory,
+		},
+		Roles: &RoleStoreConfig{StoreType: sts.StoreTypeMemory},
+	}, func() string { return "localhost:8888" }))
+
+	manager.SetUserStore(&fakeUserStore{users: map[string]*iam_pb.Identity{
+		"newuser": {Name: "newuser"}, // no PolicyNames, no Actions
+	}})
+
+	allowed, err := manager.IsActionAllowed(context.Background(), &ActionRequest{
+		Principal: "arn:aws:iam::seaweedfs:user/newuser",
+		Action:    "s3:DeleteBucket",
+		Resource:  "arn:aws:s3:::any-bucket",
+	})
+	require.NoError(t, err)
+	assert.False(t, allowed, "registered user with no policies must not get default-allow access")
+}
+
+// TestIsActionAllowed_RegisteredRoleWithoutPoliciesIsDenied is the role-side
+// counterpart of the registered-user regression: a managed role that exists
+// but has zero AttachedPolicies must not inherit full access from the
+// zero-config DefaultEffect=Allow fallback. Coderabbit flagged this as an
+// uncovered branch of the same fix on PR #9317.
+func TestIsActionAllowed_RegisteredRoleWithoutPoliciesIsDenied(t *testing.T) {
+	manager := NewIAMManager()
+	require.NoError(t, manager.Initialize(&IAMConfig{
+		STS: &sts.STSConfig{
+			TokenDuration:    sts.FlexibleDuration{Duration: time.Hour},
+			MaxSessionLength: sts.FlexibleDuration{Duration: 12 * time.Hour},
+			Issuer:           "test-sts",
+			SigningKey:       []byte("test-signing-key-32-characters-long"),
+		},
+		Policy: &policy.PolicyEngineConfig{
+			DefaultEffect: string(policy.EffectAllow),
+			StoreType:     sts.StoreTypeMemory,
+		},
+		Roles: &RoleStoreConfig{StoreType: sts.StoreTypeMemory},
+	}, func() string { return "localhost:8888" }))
+
+	ctx := context.Background()
+	require.NoError(t, manager.CreateRole(ctx, "", "EmptyRole", &RoleDefinition{
+		RoleName:         "EmptyRole",
+		RoleArn:          "arn:aws:iam::seaweedfs:role/EmptyRole",
+		AttachedPolicies: nil, // explicit: managed role with no policies attached
+	}))
+
+	allowed, err := manager.IsActionAllowed(ctx, &ActionRequest{
+		// Use the assumed-role principal form the STS path produces. The
+		// IAM manager extracts role name via ExtractRoleNameFromPrincipal,
+		// finds the role definition, and must deny because AttachedPolicies
+		// is empty even though DefaultEffect is Allow.
+		Principal: "arn:aws:sts::seaweedfs:assumed-role/EmptyRole/some-session",
+		Action:    "s3:DeleteBucket",
+		Resource:  "arn:aws:s3:::any-bucket",
+	})
+	require.NoError(t, err)
+	assert.False(t, allowed, "registered role with no attached policies must not get default-allow access")
+}
+
+// TestIsActionAllowed_RegisteredUserWithNonMatchingPolicyIsDenied covers the
+// adjacent case: the user has policies attached, but none of the policy
+// statements match the requested action/resource. The DefaultEffect=Allow
+// fallback must not silently grant access when an attached policy simply
+// does not say anything about this action.
+func TestIsActionAllowed_RegisteredUserWithNonMatchingPolicyIsDenied(t *testing.T) {
+	manager := NewIAMManager()
+	require.NoError(t, manager.Initialize(&IAMConfig{
+		STS: &sts.STSConfig{
+			TokenDuration:    sts.FlexibleDuration{Duration: time.Hour},
+			MaxSessionLength: sts.FlexibleDuration{Duration: 12 * time.Hour},
+			Issuer:           "test-sts",
+			SigningKey:       []byte("test-signing-key-32-characters-long"),
+		},
+		Policy: &policy.PolicyEngineConfig{
+			DefaultEffect: string(policy.EffectAllow),
+			StoreType:     sts.StoreTypeMemory,
+		},
+		Roles: &RoleStoreConfig{StoreType: sts.StoreTypeMemory},
+	}, func() string { return "localhost:8888" }))
+
+	ctx := context.Background()
+	require.NoError(t, manager.CreatePolicy(ctx, "", "ReadOnlyOneBucket", &policy.PolicyDocument{
+		Version: "2012-10-17",
+		Statement: []policy.Statement{{
+			Effect:   "Allow",
+			Action:   []string{"s3:GetObject"},
+			Resource: []string{"arn:aws:s3:::reports/*"},
+		}},
+	}))
+
+	manager.SetUserStore(&fakeUserStore{users: map[string]*iam_pb.Identity{
+		"alice": {Name: "alice", PolicyNames: []string{"ReadOnlyOneBucket"}},
+	}})
+
+	allowed, err := manager.IsActionAllowed(ctx, &ActionRequest{
+		Principal: "arn:aws:iam::seaweedfs:user/alice",
+		Action:    "s3:DeleteBucket",
+		Resource:  "arn:aws:s3:::other-bucket",
+	})
+	require.NoError(t, err)
+	assert.False(t, allowed, "action outside the user's attached policy must be denied")
+}