s3api: support group inline policies + Condition enforcement (#9569)

* test(s3api): cover IAM inline policy aws:SourceIp + group inline gap

Unit tests under weed/s3api/ drive PutUserPolicy / PutGroupPolicy → reload
→ VerifyActionPermission with a synthetic 127.0.0.1 request and assert that
the policy's IpAddress condition flips the outcome.

The user-policy cases pass on master (hydrateRuntimePolicies already routes
inline docs through the policy engine, so Condition blocks are honored end-
to-end). The group-policy case fails: PutGroupPolicy still returns
NotImplemented, so a group inline doc never lands in the engine.

Integration counterparts live under test/s3/iam/ and exercise the same
paths against a live SeaweedFS S3+IAM endpoint.

* s3api: support group inline policies + Condition enforcement

PutGroupPolicy/GetGroupPolicy/DeleteGroupPolicy/ListGroupPolicies used to
return NotImplemented in embedded IAM mode, so anything attached to a
group as an inline doc — including aws:SourceIp or any other Condition —
was simply unreachable.

Wire the four endpoints to the credential-store methods that were
already in place (memory, postgres, filer_etc all implement
GroupInlinePolicyStore). On every config reload, hydrateRuntimePolicies
now also walks LoadGroupInlinePolicies, registers each doc in the IAM
policy engine under __inline_group_policy__/<group>/<policy>, and
appends that key to Group.PolicyNames so evaluateIAMPolicies picks it up
through its existing group walk. PutGroupPolicy/DeleteGroupPolicy are
added to the ReloadConfiguration trigger list in DoActions.

Side fix: MemoryStore.LoadConfiguration now surfaces store.groups too.
Without it iam.groups never repopulated on a memory-store reload, so
group policy evaluation silently no-op'd whether the policy was inline
or attached. The existing tests didn't notice because no test reloaded
through cm after creating a group.

The NotImplemented unit test is inverted to drive the new round-trip.

* s3api: drop redundant refreshIAMConfiguration from Put/DeleteGroupPolicy

DoActions already triggers ReloadConfiguration for both actions via the
explicit reload list, so calling refreshIAMConfiguration inline runs the
load twice per request. Per PR review.

* s3api: scope group-policy resource names per test; tighten deny polling

- Integration test resource names get a per-test suffix so retried or
  parallel CI jobs don't trip EntityAlreadyExists / BucketAlreadyExists.
- Deny-path Eventually loops gate on AccessDenied via a typed helper
  rather than any non-nil error; transient setup errors no longer end
  the wait prematurely.
- ListGroupPolicies returns ServiceFailure when the credential manager
  is nil, matching Put/Get/DeleteGroupPolicy.

* test(s3 iam): cover both IPv4 and IPv6 loopback in allow CIDRs

CI runners with happy-eyeballs resolve `localhost` to ::1 first, in
which case a 127.0.0.0/8-only allow would silently never match and the
deny-driven enforcement test would hang for the allow case. Add ::1/128
to every loopback-matching policy so the allow path works regardless of
which loopback family the SDK lands on.
This commit is contained in:
Chris Lu
2026-05-19 16:03:45 -07:00
committed by GitHub
parent 77ac781bbd
commit 285025eb73
6 changed files with 809 additions and 61 deletions
@@ -0,0 +1,309 @@
package iam
import (
"fmt"
mathrand "math/rand"
"strings"
"testing"
"time"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/aws/aws-sdk-go/service/s3"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// uniqueResourceSuffix returns a lowercased per-test, per-invocation suffix
// safe for use in IAM resource and S3 bucket names. Avoids EntityAlreadyExists
// / BucketAlreadyExists collisions when integration jobs retry or run in
// parallel against a shared stack.
func uniqueResourceSuffix(t *testing.T) string {
name := strings.ToLower(t.Name())
name = strings.ReplaceAll(name, "/", "-")
name = strings.ReplaceAll(name, "_", "-")
return fmt.Sprintf("%s-%d", name, mathrand.Intn(10000))
}
// isAccessDenied returns true when err is an AWS error with code "AccessDenied".
// Used to gate deny-path polling so transient setup errors don't end the
// Eventually loop prematurely.
func isAccessDenied(err error) bool {
if err == nil {
return false
}
awsErr, ok := err.(awserr.Error)
return ok && awsErr.Code() == "AccessDenied"
}
// TestIAMUserInlinePolicySourceIpCondition verifies that an aws:SourceIp condition
// on a user inline policy is honored. Tests run from localhost (127.0.0.1), so a
// policy that only allows access from a non-loopback CIDR must deny the request,
// and a policy that allows access from 127.0.0.0/8 must allow it.
func TestIAMUserInlinePolicySourceIpCondition(t *testing.T) {
framework := NewS3IAMTestFramework(t)
defer framework.Cleanup()
iamClient, err := framework.CreateIAMClientWithJWT("admin-user", "TestAdminRole")
require.NoError(t, err)
suffix := uniqueResourceSuffix(t)
userName := "user-" + suffix
policyName := "policy-" + suffix
bucketName := "bucket-" + suffix
_, err = iamClient.CreateUser(&iam.CreateUserInput{UserName: aws.String(userName)})
require.NoError(t, err)
keyResp, err := iamClient.CreateAccessKey(&iam.CreateAccessKeyInput{
UserName: aws.String(userName),
})
require.NoError(t, err)
accessKeyId := *keyResp.AccessKey.AccessKeyId
secretKey := *keyResp.AccessKey.SecretAccessKey
userS3 := createS3Client(t, accessKeyId, secretKey)
adminS3, err := framework.CreateS3ClientWithJWT("admin-user", "TestAdminRole")
require.NoError(t, err)
require.NoError(t, framework.CreateBucketWithCleanup(adminS3, bucketName))
t.Cleanup(func() {
if _, err := iamClient.DeleteUserPolicy(&iam.DeleteUserPolicyInput{
UserName: aws.String(userName),
PolicyName: aws.String(policyName),
}); err != nil {
t.Logf("cleanup: failed to delete user policy: %v", err)
}
if _, err := iamClient.DeleteAccessKey(&iam.DeleteAccessKeyInput{
UserName: aws.String(userName),
AccessKeyId: keyResp.AccessKey.AccessKeyId,
}); err != nil {
t.Logf("cleanup: failed to delete access key: %v", err)
}
if _, err := iamClient.DeleteUser(&iam.DeleteUserInput{UserName: aws.String(userName)}); err != nil {
t.Logf("cleanup: failed to delete user: %v", err)
}
})
policyDoc := func(cidrs ...string) string {
quoted := make([]string, len(cidrs))
for i, c := range cidrs {
quoted[i] = `"` + c + `"`
}
return `{
"Version":"2012-10-17",
"Statement":[{
"Effect":"Allow",
"Action":"s3:*",
"Resource":["arn:aws:s3:::` + bucketName + `","arn:aws:s3:::` + bucketName + `/*"],
"Condition":{"IpAddress":{"aws:SourceIp":[` + strings.Join(quoted, ",") + `]}}
}]
}`
}
t.Run("denies_when_source_ip_does_not_match", func(t *testing.T) {
// SourceIp 198.51.100.0/24 is RFC5737 TEST-NET-2; the test client is on
// loopback (127.0.0.1 or ::1 depending on resolver), so the condition
// must fail and the action must be denied.
_, err = iamClient.PutUserPolicy(&iam.PutUserPolicyInput{
UserName: aws.String(userName),
PolicyName: aws.String(policyName),
PolicyDocument: aws.String(policyDoc("198.51.100.0/24")),
})
require.NoError(t, err)
var lastErr error
require.Eventually(t, func() bool {
_, lastErr = userS3.PutObject(&s3.PutObjectInput{
Bucket: aws.String(bucketName),
Key: aws.String("denied.txt"),
Body: aws.ReadSeekCloser(strings.NewReader("nope")),
})
return isAccessDenied(lastErr)
}, 10*time.Second, 500*time.Millisecond,
"PutObject must be denied with AccessDenied when aws:SourceIp condition does not match (last error: %v)", lastErr)
})
t.Run("allows_when_source_ip_matches", func(t *testing.T) {
// Cover both IPv4 and IPv6 loopback: on CI runners `localhost` may
// resolve to ::1 first, in which case a 127.0.0.0/8-only allow would
// silently never match and the test would hang.
_, err = iamClient.PutUserPolicy(&iam.PutUserPolicyInput{
UserName: aws.String(userName),
PolicyName: aws.String(policyName),
PolicyDocument: aws.String(policyDoc("127.0.0.0/8", "::1/128")),
})
require.NoError(t, err)
require.Eventually(t, func() bool {
_, err := userS3.PutObject(&s3.PutObjectInput{
Bucket: aws.String(bucketName),
Key: aws.String("allowed.txt"),
Body: aws.ReadSeekCloser(strings.NewReader("ok")),
})
return err == nil
}, 10*time.Second, 500*time.Millisecond,
"PutObject must succeed when aws:SourceIp condition matches the loopback range")
})
}
// TestIAMGroupInlinePolicyEnforcement verifies that PutGroupPolicy is supported
// and that the resulting inline policy is enforced for members of the group,
// including its Condition block.
func TestIAMGroupInlinePolicyEnforcement(t *testing.T) {
framework := NewS3IAMTestFramework(t)
defer framework.Cleanup()
iamClient, err := framework.CreateIAMClientWithJWT("admin-user", "TestAdminRole")
require.NoError(t, err)
suffix := uniqueResourceSuffix(t)
groupName := "group-" + suffix
userName := "user-" + suffix
policyName := "policy-" + suffix
bucketName := "bucket-" + suffix
_, err = iamClient.CreateUser(&iam.CreateUserInput{UserName: aws.String(userName)})
require.NoError(t, err)
keyResp, err := iamClient.CreateAccessKey(&iam.CreateAccessKeyInput{
UserName: aws.String(userName),
})
require.NoError(t, err)
_, err = iamClient.CreateGroup(&iam.CreateGroupInput{GroupName: aws.String(groupName)})
require.NoError(t, err)
_, err = iamClient.AddUserToGroup(&iam.AddUserToGroupInput{
GroupName: aws.String(groupName),
UserName: aws.String(userName),
})
require.NoError(t, err)
userS3 := createS3Client(t, *keyResp.AccessKey.AccessKeyId, *keyResp.AccessKey.SecretAccessKey)
adminS3, err := framework.CreateS3ClientWithJWT("admin-user", "TestAdminRole")
require.NoError(t, err)
require.NoError(t, framework.CreateBucketWithCleanup(adminS3, bucketName))
t.Cleanup(func() {
if _, err := iamClient.DeleteGroupPolicy(&iam.DeleteGroupPolicyInput{
GroupName: aws.String(groupName),
PolicyName: aws.String(policyName),
}); err != nil {
t.Logf("cleanup: failed to delete group policy: %v", err)
}
if _, err := iamClient.RemoveUserFromGroup(&iam.RemoveUserFromGroupInput{
GroupName: aws.String(groupName),
UserName: aws.String(userName),
}); err != nil {
t.Logf("cleanup: failed to remove user from group: %v", err)
}
if _, err := iamClient.DeleteAccessKey(&iam.DeleteAccessKeyInput{
UserName: aws.String(userName),
AccessKeyId: keyResp.AccessKey.AccessKeyId,
}); err != nil {
t.Logf("cleanup: failed to delete access key: %v", err)
}
if _, err := iamClient.DeleteUser(&iam.DeleteUserInput{UserName: aws.String(userName)}); err != nil {
t.Logf("cleanup: failed to delete user: %v", err)
}
if _, err := iamClient.DeleteGroup(&iam.DeleteGroupInput{GroupName: aws.String(groupName)}); err != nil {
t.Logf("cleanup: failed to delete group: %v", err)
}
})
// Cover both IPv4 and IPv6 loopback in the allow CIDR list: on CI runners
// `localhost` may resolve to ::1 first, in which case a 127.0.0.0/8-only
// allow would silently never match and the test would hang.
allowDoc := `{
"Version":"2012-10-17",
"Statement":[{
"Effect":"Allow",
"Action":"s3:*",
"Resource":["arn:aws:s3:::` + bucketName + `","arn:aws:s3:::` + bucketName + `/*"],
"Condition":{"IpAddress":{"aws:SourceIp":["127.0.0.0/8","::1/128"]}}
}]
}`
denyDoc := `{
"Version":"2012-10-17",
"Statement":[{
"Effect":"Allow",
"Action":"s3:*",
"Resource":["arn:aws:s3:::` + bucketName + `","arn:aws:s3:::` + bucketName + `/*"],
"Condition":{"IpAddress":{"aws:SourceIp":"198.51.100.0/24"}}
}]
}`
t.Run("crud_round_trip", func(t *testing.T) {
_, err := iamClient.PutGroupPolicy(&iam.PutGroupPolicyInput{
GroupName: aws.String(groupName),
PolicyName: aws.String(policyName),
PolicyDocument: aws.String(allowDoc),
})
require.NoError(t, err, "PutGroupPolicy must succeed (no longer NotImplemented)")
listResp, err := iamClient.ListGroupPolicies(&iam.ListGroupPoliciesInput{
GroupName: aws.String(groupName),
})
require.NoError(t, err)
found := false
for _, name := range listResp.PolicyNames {
if name != nil && *name == policyName {
found = true
break
}
}
assert.True(t, found, "ListGroupPolicies must return the freshly added policy")
getResp, err := iamClient.GetGroupPolicy(&iam.GetGroupPolicyInput{
GroupName: aws.String(groupName),
PolicyName: aws.String(policyName),
})
require.NoError(t, err)
require.NotNil(t, getResp.PolicyDocument)
assert.Contains(t, *getResp.PolicyDocument, "aws:SourceIp",
"GetGroupPolicy must round-trip the Condition block")
})
t.Run("enforces_allow_when_condition_matches", func(t *testing.T) {
_, err := iamClient.PutGroupPolicy(&iam.PutGroupPolicyInput{
GroupName: aws.String(groupName),
PolicyName: aws.String(policyName),
PolicyDocument: aws.String(allowDoc),
})
require.NoError(t, err)
require.Eventually(t, func() bool {
_, err := userS3.PutObject(&s3.PutObjectInput{
Bucket: aws.String(bucketName),
Key: aws.String("group-allowed.txt"),
Body: aws.ReadSeekCloser(strings.NewReader("ok")),
})
return err == nil
}, 10*time.Second, 500*time.Millisecond,
"group member must be allowed when the group policy condition matches")
})
t.Run("enforces_deny_when_condition_does_not_match", func(t *testing.T) {
_, err := iamClient.PutGroupPolicy(&iam.PutGroupPolicyInput{
GroupName: aws.String(groupName),
PolicyName: aws.String(policyName),
PolicyDocument: aws.String(denyDoc),
})
require.NoError(t, err)
var lastErr error
require.Eventually(t, func() bool {
_, lastErr = userS3.PutObject(&s3.PutObjectInput{
Bucket: aws.String(bucketName),
Key: aws.String("group-denied.txt"),
Body: aws.ReadSeekCloser(strings.NewReader("nope")),
})
return isAccessDenied(lastErr)
}, 10*time.Second, 500*time.Millisecond,
"group member must be denied with AccessDenied when the group policy condition does not match (last error: %v)", lastErr)
})
}