diff --git a/test/s3/iam/s3_iam_inline_policy_condition_test.go b/test/s3/iam/s3_iam_inline_policy_condition_test.go new file mode 100644 index 000000000..a09007b7d --- /dev/null +++ b/test/s3/iam/s3_iam_inline_policy_condition_test.go @@ -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) + }) +} diff --git a/weed/credential/memory/memory_identity.go b/weed/credential/memory/memory_identity.go index 1cf0d3179..9b578b152 100644 --- a/weed/credential/memory/memory_identity.go +++ b/weed/credential/memory/memory_identity.go @@ -34,6 +34,16 @@ func (store *MemoryStore) LoadConfiguration(ctx context.Context) (*iam_pb.S3ApiC }) } + // Groups are written via CreateGroup / AddUserToGroup / AttachGroupPolicy + // (never via SaveConfiguration), so they live in store.groups regardless + // of the bulk-config path. Surface them here so consumers that reload + // through cm.LoadConfiguration see a faithful snapshot — without this, + // iam.groups would never repopulate on memory-store reloads and group + // policy evaluation would silently no-op. + for _, g := range store.groups { + config.Groups = append(config.Groups, cloneGroup(g)) + } + return config, nil } diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 4f8842998..0c09cd8db 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -1671,10 +1671,18 @@ type inlinePolicyLoader interface { LoadInlinePolicies(ctx context.Context) (map[string]map[string]policy_engine.PolicyDocument, error) } +type groupInlinePolicyLoader interface { + LoadGroupInlinePolicies(ctx context.Context) (map[string]map[string]policy_engine.PolicyDocument, error) +} + func inlinePolicyRuntimeName(userName, policyName string) string { return "__inline_policy__/" + userName + "/" + policyName } +func inlineGroupPolicyRuntimeName(groupName, policyName string) string { + return "__inline_group_policy__/" + groupName + "/" + policyName +} + func mergePoliciesIntoConfiguration(config *iam_pb.S3ApiConfiguration, policies []*iam_pb.Policy) { if len(policies) == 0 { return @@ -1759,44 +1767,73 @@ func (iam *IdentityAccessManagement) hydrateRuntimePolicies(ctx context.Context, return nil } - inlineLoader, ok := store.(inlinePolicyLoader) - if !ok { - return nil - } - - inlinePoliciesByUser, err := inlineLoader.LoadInlinePolicies(ctx) - if err != nil { - return fmt.Errorf("failed to load inline policies for runtime: %w", err) - } - - if len(inlinePoliciesByUser) == 0 { - return nil - } - - identityByName := make(map[string]*iam_pb.Identity, len(config.Identities)) - for _, identity := range config.Identities { - identityByName[identity.Name] = identity - } - inlinePolicies := make([]*iam_pb.Policy, 0) - for userName, userPolicies := range inlinePoliciesByUser { - identity, found := identityByName[userName] - if !found { - continue - } - for policyName, policyDocument := range userPolicies { - content, err := json.Marshal(policyDocument) - if err != nil { - return fmt.Errorf("failed to marshal inline policy %q for user %q: %w", policyName, userName, err) + if inlineLoader, ok := store.(inlinePolicyLoader); ok { + inlinePoliciesByUser, err := inlineLoader.LoadInlinePolicies(ctx) + if err != nil { + return fmt.Errorf("failed to load inline policies for runtime: %w", err) + } + if len(inlinePoliciesByUser) > 0 { + identityByName := make(map[string]*iam_pb.Identity, len(config.Identities)) + for _, identity := range config.Identities { + identityByName[identity.Name] = identity } - runtimePolicyName := inlinePolicyRuntimeName(userName, policyName) - inlinePolicies = append(inlinePolicies, &iam_pb.Policy{ - Name: runtimePolicyName, - Content: string(content), - }) - identity.PolicyNames = appendUniquePolicyName(identity.PolicyNames, runtimePolicyName) + for userName, userPolicies := range inlinePoliciesByUser { + identity, found := identityByName[userName] + if !found { + continue + } + + for policyName, policyDocument := range userPolicies { + content, err := json.Marshal(policyDocument) + if err != nil { + return fmt.Errorf("failed to marshal inline policy %q for user %q: %w", policyName, userName, err) + } + + runtimePolicyName := inlinePolicyRuntimeName(userName, policyName) + inlinePolicies = append(inlinePolicies, &iam_pb.Policy{ + Name: runtimePolicyName, + Content: string(content), + }) + identity.PolicyNames = appendUniquePolicyName(identity.PolicyNames, runtimePolicyName) + } + } + } + } + + if groupLoader, ok := store.(groupInlinePolicyLoader); ok { + groupPoliciesByName, err := groupLoader.LoadGroupInlinePolicies(ctx) + if err != nil { + return fmt.Errorf("failed to load group inline policies for runtime: %w", err) + } + if len(groupPoliciesByName) > 0 { + groupByName := make(map[string]*iam_pb.Group, len(config.Groups)) + for _, group := range config.Groups { + groupByName[group.Name] = group + } + + for groupName, groupPolicies := range groupPoliciesByName { + group, found := groupByName[groupName] + if !found { + continue + } + + for policyName, policyDocument := range groupPolicies { + content, err := json.Marshal(policyDocument) + if err != nil { + return fmt.Errorf("failed to marshal inline policy %q for group %q: %w", policyName, groupName, err) + } + + runtimePolicyName := inlineGroupPolicyRuntimeName(groupName, policyName) + inlinePolicies = append(inlinePolicies, &iam_pb.Policy{ + Name: runtimePolicyName, + Content: string(content), + }) + group.PolicyNames = appendUniquePolicyName(group.PolicyNames, runtimePolicyName) + } + } } } diff --git a/weed/s3api/s3api_embedded_iam.go b/weed/s3api/s3api_embedded_iam.go index 57ab588a0..d3f87eece 100644 --- a/weed/s3api/s3api_embedded_iam.go +++ b/weed/s3api/s3api_embedded_iam.go @@ -1986,29 +1986,130 @@ func (e *EmbeddedIamApi) ListGroupsForUser(s3cfg *iam_pb.S3ApiConfiguration, val return resp, nil } -// notImplementedError returns a NotImplemented IAM error for the embedded server. -func notImplementedGroupInlineError() *iamError { - return &iamError{Code: s3err.GetAPIError(s3err.ErrNotImplemented).Code, Error: fmt.Errorf("group inline policies are not supported in embedded IAM mode; use the standalone IAM server or managed policies (AttachGroupPolicy)")} +// groupExists returns true if a group of the given name is present in the +// supplied S3 IAM configuration. +func groupExists(s3cfg *iam_pb.S3ApiConfiguration, groupName string) bool { + for _, g := range s3cfg.Groups { + if g.Name == groupName { + return true + } + } + return false } -// PutGroupPolicy is not supported in embedded IAM mode. +// PutGroupPolicy attaches an inline policy to a group. The document (including +// any Condition block) is persisted via the credential store and registered +// in the IAM policy engine via hydrateRuntimePolicies on the next reload. func (e *EmbeddedIamApi) PutGroupPolicy(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (*iamPutGroupPolicyResponse, *iamError) { - return &iamPutGroupPolicyResponse{}, notImplementedGroupInlineError() + resp := &iamPutGroupPolicyResponse{} + groupName := values.Get("GroupName") + if groupName == "" { + return resp, &iamError{Code: iam.ErrCodeInvalidInputException, Error: fmt.Errorf("GroupName is required")} + } + policyName := values.Get("PolicyName") + if policyName == "" { + return resp, &iamError{Code: iam.ErrCodeInvalidInputException, Error: fmt.Errorf("PolicyName is required")} + } + if e.credentialManager == nil { + return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: fmt.Errorf("credential manager not configured")} + } + policyDocumentString := values.Get("PolicyDocument") + policyDocument, err := e.GetPolicyDocument(&policyDocumentString) + if err != nil { + return resp, &iamError{Code: iam.ErrCodeMalformedPolicyDocumentException, Error: err} + } + if !groupExists(s3cfg, groupName) { + return resp, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: fmt.Errorf("group %s does not exist", groupName)} + } + if err := e.credentialManager.PutGroupInlinePolicy(context.Background(), groupName, policyName, policyDocument); err != nil { + return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: err} + } + // DoActions reloads the IAM configuration for PutGroupPolicy, so the + // engine registration via hydrateRuntimePolicies happens there. No + // internal refresh is needed. + return resp, nil } -// GetGroupPolicy is not supported in embedded IAM mode. +// GetGroupPolicy returns the JSON document for an inline policy on a group. func (e *EmbeddedIamApi) GetGroupPolicy(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (*iamGetGroupPolicyResponse, *iamError) { - return &iamGetGroupPolicyResponse{}, notImplementedGroupInlineError() + resp := &iamGetGroupPolicyResponse{} + groupName := values.Get("GroupName") + policyName := values.Get("PolicyName") + if groupName == "" { + return resp, &iamError{Code: iam.ErrCodeInvalidInputException, Error: fmt.Errorf("GroupName is required")} + } + if policyName == "" { + return resp, &iamError{Code: iam.ErrCodeInvalidInputException, Error: fmt.Errorf("PolicyName is required")} + } + if !groupExists(s3cfg, groupName) { + return resp, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: fmt.Errorf("group %s does not exist", groupName)} + } + if e.credentialManager == nil { + return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: fmt.Errorf("credential manager not configured")} + } + doc, err := e.credentialManager.GetGroupInlinePolicy(context.Background(), groupName, policyName) + if err != nil { + return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: err} + } + if doc == nil { + return resp, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: fmt.Errorf("policy %s not found on group %s", policyName, groupName)} + } + docJSON, err := json.Marshal(doc) + if err != nil { + return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: err} + } + resp.GetGroupPolicyResult.GroupName = groupName + resp.GetGroupPolicyResult.PolicyName = policyName + resp.GetGroupPolicyResult.PolicyDocument = string(docJSON) + return resp, nil } -// DeleteGroupPolicy is not supported in embedded IAM mode. +// DeleteGroupPolicy removes an inline policy from a group. func (e *EmbeddedIamApi) DeleteGroupPolicy(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (*iamDeleteGroupPolicyResponse, *iamError) { - return &iamDeleteGroupPolicyResponse{}, notImplementedGroupInlineError() + resp := &iamDeleteGroupPolicyResponse{} + groupName := values.Get("GroupName") + policyName := values.Get("PolicyName") + if groupName == "" { + return resp, &iamError{Code: iam.ErrCodeInvalidInputException, Error: fmt.Errorf("GroupName is required")} + } + if policyName == "" { + return resp, &iamError{Code: iam.ErrCodeInvalidInputException, Error: fmt.Errorf("PolicyName is required")} + } + if !groupExists(s3cfg, groupName) { + return resp, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: fmt.Errorf("group %s does not exist", groupName)} + } + if e.credentialManager == nil { + return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: fmt.Errorf("credential manager not configured")} + } + if err := e.credentialManager.DeleteGroupInlinePolicy(context.Background(), groupName, policyName); err != nil { + return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: err} + } + // DoActions reloads the IAM configuration for DeleteGroupPolicy, so the + // engine deregistration via hydrateRuntimePolicies happens there. No + // internal refresh is needed. + return resp, nil } -// ListGroupPolicies is not supported in embedded IAM mode. +// ListGroupPolicies returns the names of inline policies attached to a group. func (e *EmbeddedIamApi) ListGroupPolicies(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (*iamListGroupPoliciesResponse, *iamError) { - return &iamListGroupPoliciesResponse{}, notImplementedGroupInlineError() + resp := &iamListGroupPoliciesResponse{} + groupName := values.Get("GroupName") + if groupName == "" { + return resp, &iamError{Code: iam.ErrCodeInvalidInputException, Error: fmt.Errorf("GroupName is required")} + } + if !groupExists(s3cfg, groupName) { + return resp, &iamError{Code: iam.ErrCodeNoSuchEntityException, Error: fmt.Errorf("group %s does not exist", groupName)} + } + if e.credentialManager == nil { + return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: fmt.Errorf("credential manager not configured")} + } + names, err := e.credentialManager.ListGroupInlinePolicies(context.Background(), groupName) + if err != nil { + return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: err} + } + resp.ListGroupPoliciesResult.PolicyNames = names + resp.ListGroupPoliciesResult.IsTruncated = false + return resp, nil } // handleImplicitUsername adds username who signs the request to values if 'username' is not specified. @@ -2492,7 +2593,7 @@ func (e *EmbeddedIamApi) ExecuteAction(ctx context.Context, values url.Values, s glog.Errorf("Failed to reload IAM configuration after mutation: %v", err) // Don't fail the request since the persistent save succeeded } - } else if action == "AttachUserPolicy" || action == "DetachUserPolicy" || action == "CreatePolicy" || action == "DeletePolicy" || action == "CreateUser" { + } else if action == "AttachUserPolicy" || action == "DetachUserPolicy" || action == "CreatePolicy" || action == "DeletePolicy" || action == "CreateUser" || action == "PutGroupPolicy" || action == "DeleteGroupPolicy" { // Even if changed=false (persisted via credentialManager), we should still reload // if we are utilizing the local in-memory cache for speed if err := e.ReloadConfiguration(); err != nil { diff --git a/weed/s3api/s3api_embedded_iam_test.go b/weed/s3api/s3api_embedded_iam_test.go index 8905241f1..c44ab5aa3 100644 --- a/weed/s3api/s3api_embedded_iam_test.go +++ b/weed/s3api/s3api_embedded_iam_test.go @@ -72,6 +72,10 @@ func NewEmbeddedIamApiForTest() *EmbeddedIamApiForTest { for i, p := range config.Policies { s3cfg.Policies[i] = proto.Clone(p).(*iam_pb.Policy) } + s3cfg.Groups = make([]*iam_pb.Group, len(config.Groups)) + for i, g := range config.Groups { + s3cfg.Groups[i] = proto.Clone(g).(*iam_pb.Group) + } } return err } @@ -619,37 +623,47 @@ func TestEmbeddedIamListUserPolicies(t *testing.T) { assert.Equal(t, http.StatusNotFound, rr3.Code) } -// TestEmbeddedIamGroupInlinePoliciesNotImplemented tests that group inline policies -// return NotImplemented in embedded IAM mode. -func TestEmbeddedIamGroupInlinePoliciesNotImplemented(t *testing.T) { +// TestEmbeddedIamGroupInlinePolicies_CRUD exercises the Put/Get/List/Delete +// round-trip for group inline policies and verifies that an unknown policy +// surfaces as NoSuchEntity (not the legacy NotImplemented). +func TestEmbeddedIamGroupInlinePolicies_CRUD(t *testing.T) { api := NewEmbeddedIamApiForTest() s3cfg := &iam_pb.S3ApiConfiguration{ Groups: []*iam_pb.Group{ {Name: "developers", Members: []string{"alice"}}, }, } + api.mockConfig = s3cfg - notImpl := s3err.GetAPIError(s3err.ErrNotImplemented).Code + policyDoc := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:GetObject","Resource":"arn:aws:s3:::*"}]}` _, iamErr := api.PutGroupPolicy(s3cfg, url.Values{ "GroupName": {"developers"}, "PolicyName": {"DevPolicy"}, - "PolicyDocument": {`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:GetObject","Resource":"arn:aws:s3:::*"}]}`}, + "PolicyDocument": {policyDoc}, }) - assert.NotNil(t, iamErr) - assert.Equal(t, notImpl, iamErr.Code) + require.Nil(t, iamErr, "PutGroupPolicy must succeed; got: %+v", iamErr) - _, iamErr = api.GetGroupPolicy(s3cfg, url.Values{"GroupName": {"developers"}, "PolicyName": {"DevPolicy"}}) - assert.NotNil(t, iamErr) - assert.Equal(t, notImpl, iamErr.Code) + listResp, iamErr := api.ListGroupPolicies(s3cfg, url.Values{"GroupName": {"developers"}}) + require.Nil(t, iamErr) + assert.Equal(t, []string{"DevPolicy"}, listResp.ListGroupPoliciesResult.PolicyNames) + + getResp, iamErr := api.GetGroupPolicy(s3cfg, url.Values{"GroupName": {"developers"}, "PolicyName": {"DevPolicy"}}) + require.Nil(t, iamErr) + assert.Equal(t, "developers", getResp.GetGroupPolicyResult.GroupName) + assert.Equal(t, "DevPolicy", getResp.GetGroupPolicyResult.PolicyName) + assert.Contains(t, getResp.GetGroupPolicyResult.PolicyDocument, "s3:GetObject") _, iamErr = api.DeleteGroupPolicy(s3cfg, url.Values{"GroupName": {"developers"}, "PolicyName": {"DevPolicy"}}) - assert.NotNil(t, iamErr) - assert.Equal(t, notImpl, iamErr.Code) + require.Nil(t, iamErr) - _, iamErr = api.ListGroupPolicies(s3cfg, url.Values{"GroupName": {"developers"}}) - assert.NotNil(t, iamErr) - assert.Equal(t, notImpl, iamErr.Code) + _, iamErr = api.GetGroupPolicy(s3cfg, url.Values{"GroupName": {"developers"}, "PolicyName": {"DevPolicy"}}) + require.NotNil(t, iamErr) + assert.Equal(t, iam.ErrCodeNoSuchEntityException, iamErr.Code) + + _, iamErr = api.GetGroupPolicy(s3cfg, url.Values{"GroupName": {"ghosts"}, "PolicyName": {"DevPolicy"}}) + require.NotNil(t, iamErr, "GetGroupPolicy on a missing group must error") + assert.Equal(t, iam.ErrCodeNoSuchEntityException, iamErr.Code) } // TestEmbeddedIamAttachUserPolicy tests attaching a managed policy to a user. diff --git a/weed/s3api/s3api_inline_policy_condition_test.go b/weed/s3api/s3api_inline_policy_condition_test.go new file mode 100644 index 000000000..9cd378285 --- /dev/null +++ b/weed/s3api/s3api_inline_policy_condition_test.go @@ -0,0 +1,277 @@ +package s3api + +import ( + "context" + "net/http" + "net/url" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" + s3_constants "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// In-process reproducers for aws:SourceIp condition handling on inline policies. +// +// These exercise the full chain: PutUserPolicy/PutGroupPolicy → reload → +// VerifyActionPermission against a synthetic HTTP request whose RemoteAddr is +// 127.0.0.1. The whole point is to prove that the Condition block is honored +// (or, before the fix lands, that it is silently dropped). + +const inlineCondTestBucket = "inline-cond-bucket" + +func inlineCondPolicyDoc(cidr string) string { + return `{ + "Version":"2012-10-17", + "Statement":[{ + "Effect":"Allow", + "Action":"s3:*", + "Resource":["arn:aws:s3:::` + inlineCondTestBucket + `","arn:aws:s3:::` + inlineCondTestBucket + `/*"], + "Condition":{"IpAddress":{"aws:SourceIp":"` + cidr + `"}} + }] + }` +} + +// inlineCondRequest builds a minimal request that VerifyActionPermission can +// evaluate: RemoteAddr set so extractSourceIP returns 127.0.0.1, and the bucket +// path so GetBucketAndObject (if reached) is unambiguous. +func inlineCondRequest(t *testing.T, method string) *http.Request { + t.Helper() + req, err := http.NewRequest(method, "http://s3.amazonaws.com/"+inlineCondTestBucket+"/obj", nil) + require.NoError(t, err) + req.Host = "s3.amazonaws.com" + req.RemoteAddr = "127.0.0.1:54321" + return req +} + +// seedInlineCondUser puts a user "alice" with a single access key into both +// api.mockConfig and the credential store, then reloads the in-memory IAM so +// identity lookups observe her. Returns the loaded Identity. +func seedInlineCondUser(t *testing.T, api *EmbeddedIamApiForTest) *Identity { + t.Helper() + if api.mockConfig == nil { + api.mockConfig = &iam_pb.S3ApiConfiguration{} + } + api.mockConfig.Identities = append(api.mockConfig.Identities, &iam_pb.Identity{ + Name: "alice", + Credentials: []*iam_pb.Credential{ + {AccessKey: "AKIAALICE", SecretKey: "alice-secret"}, + }, + }) + require.NoError(t, api.credentialManager.SaveConfiguration(context.Background(), api.mockConfig)) + require.NoError(t, api.iam.LoadS3ApiConfigurationFromCredentialManager()) + ident := api.iam.lookupByIdentityName("alice") + require.NotNil(t, ident, "alice must be loaded after reload") + return ident +} + +// TestUserInlinePolicySourceIpCondition_Denies is the primary reproducer: a +// user inline policy with an aws:SourceIp condition that does NOT match the +// caller's source IP must result in AccessDenied. Today this passes (bug) +// because PutUserPolicy flattens the document into ident.Actions and the +// Condition block is dropped. +func TestUserInlinePolicySourceIpCondition_Denies(t *testing.T) { + api := NewEmbeddedIamApiForTest() + api.mockConfig = &iam_pb.S3ApiConfiguration{} + seedInlineCondUser(t, api) + + // Allow s3:* on the bucket only when SourceIp is in 198.51.100.0/24 (TEST-NET-2). + // 127.0.0.1 must not match, so the action must be denied. + _, iamErr := api.PutUserPolicy(api.mockConfig, url.Values{ + "UserName": {"alice"}, + "PolicyName": {"OnlyFromTestNet"}, + "PolicyDocument": {inlineCondPolicyDoc("198.51.100.0/24")}, + }) + require.Nil(t, iamErr, "PutUserPolicy must succeed") + require.NoError(t, api.PutS3ApiConfiguration(api.mockConfig)) + require.NoError(t, api.iam.LoadS3ApiConfigurationFromCredentialManager()) + + ident := api.iam.lookupByIdentityName("alice") + require.NotNil(t, ident) + + req := inlineCondRequest(t, http.MethodPut) + got := api.iam.VerifyActionPermission(req, ident, s3_constants.ACTION_WRITE, inlineCondTestBucket, "obj") + assert.Equal(t, s3err.ErrAccessDenied, got, + "PutObject from 127.0.0.1 must be denied: the user inline policy's aws:SourceIp condition (198.51.100.0/24) does not match") +} + +// TestUserInlinePolicySourceIpCondition_Allows is the companion: with a +// matching CIDR (127.0.0.0/8), the same call must succeed. +func TestUserInlinePolicySourceIpCondition_Allows(t *testing.T) { + api := NewEmbeddedIamApiForTest() + api.mockConfig = &iam_pb.S3ApiConfiguration{} + seedInlineCondUser(t, api) + + _, iamErr := api.PutUserPolicy(api.mockConfig, url.Values{ + "UserName": {"alice"}, + "PolicyName": {"FromLoopback"}, + "PolicyDocument": {inlineCondPolicyDoc("127.0.0.0/8")}, + }) + require.Nil(t, iamErr) + require.NoError(t, api.PutS3ApiConfiguration(api.mockConfig)) + require.NoError(t, api.iam.LoadS3ApiConfigurationFromCredentialManager()) + + ident := api.iam.lookupByIdentityName("alice") + require.NotNil(t, ident) + + req := inlineCondRequest(t, http.MethodPut) + got := api.iam.VerifyActionPermission(req, ident, s3_constants.ACTION_WRITE, inlineCondTestBucket, "obj") + assert.Equal(t, s3err.ErrNone, got, + "PutObject from 127.0.0.1 must be allowed: the user inline policy's aws:SourceIp condition (127.0.0.0/8) matches") +} + +// TestGroupInlinePolicy_PutAndEnforce verifies that PutGroupPolicy is supported +// (no longer returns NotImplemented) and that an aws:SourceIp condition on the +// resulting inline policy is honored for group members. +func TestGroupInlinePolicy_PutAndEnforce(t *testing.T) { + api := NewEmbeddedIamApiForTest() + api.mockConfig = &iam_pb.S3ApiConfiguration{} + seedInlineCondUser(t, api) + // Groups live in their own credential-store namespace (CreateGroup, not + // SaveConfiguration). Seed devs+alice via the dedicated API and refresh + // through the framework hook so both mockConfig.Groups and iam.groups + // observe the new group. + ctx := context.Background() + require.NoError(t, api.credentialManager.CreateGroup(ctx, &iam_pb.Group{ + Name: "devs", + Members: []string{"alice"}, + })) + require.NoError(t, api.refreshIAMConfiguration()) + + // PutGroupPolicy must succeed. This is the call that returns + // NotImplemented today; this test will fail with that error. + _, iamErr := api.PutGroupPolicy(api.mockConfig, url.Values{ + "GroupName": {"devs"}, + "PolicyName": {"DevsFromLoopback"}, + "PolicyDocument": {inlineCondPolicyDoc("198.51.100.0/24")}, + }) + require.Nil(t, iamErr, "PutGroupPolicy must succeed; got: %+v", iamErr) + require.NoError(t, api.iam.LoadS3ApiConfigurationFromCredentialManager()) + + ident := api.iam.lookupByIdentityName("alice") + require.NotNil(t, ident) + + // Deny path: condition does not match the loopback caller. + req := inlineCondRequest(t, http.MethodPut) + got := api.iam.VerifyActionPermission(req, ident, s3_constants.ACTION_WRITE, inlineCondTestBucket, "obj") + assert.Equal(t, s3err.ErrAccessDenied, got, + "group member from 127.0.0.1 must be denied when the group inline policy's aws:SourceIp condition (198.51.100.0/24) does not match") + + // Round-trip via the store: list & get must observe the new inline policy. + listResp, iamErr := api.ListGroupPolicies(api.mockConfig, url.Values{"GroupName": {"devs"}}) + require.Nil(t, iamErr) + assert.Contains(t, listResp.ListGroupPoliciesResult.PolicyNames, "DevsFromLoopback") + + getResp, iamErr := api.GetGroupPolicy(api.mockConfig, url.Values{ + "GroupName": {"devs"}, "PolicyName": {"DevsFromLoopback"}, + }) + require.Nil(t, iamErr) + assert.Contains(t, getResp.GetGroupPolicyResult.PolicyDocument, "aws:SourceIp", + "GetGroupPolicy must round-trip the Condition block") + + // Flip the policy to the matching CIDR and verify the allow path. + _, iamErr = api.PutGroupPolicy(api.mockConfig, url.Values{ + "GroupName": {"devs"}, + "PolicyName": {"DevsFromLoopback"}, + "PolicyDocument": {inlineCondPolicyDoc("127.0.0.0/8")}, + }) + require.Nil(t, iamErr) + require.NoError(t, api.iam.LoadS3ApiConfigurationFromCredentialManager()) + + ident = api.iam.lookupByIdentityName("alice") + require.NotNil(t, ident) + got = api.iam.VerifyActionPermission(inlineCondRequest(t, http.MethodPut), ident, + s3_constants.ACTION_WRITE, inlineCondTestBucket, "obj") + assert.Equal(t, s3err.ErrNone, got, + "group member from 127.0.0.1 must be allowed when the group inline policy's aws:SourceIp condition (127.0.0.0/8) matches") + + // Cleanup: DeleteGroupPolicy must also be implemented and must drop the + // engine registration so the action is no longer permitted via this group. + _, iamErr = api.DeleteGroupPolicy(api.mockConfig, url.Values{ + "GroupName": {"devs"}, "PolicyName": {"DevsFromLoopback"}, + }) + require.Nil(t, iamErr, "DeleteGroupPolicy must succeed") + require.NoError(t, api.iam.LoadS3ApiConfigurationFromCredentialManager()) + ident = api.iam.lookupByIdentityName("alice") + require.NotNil(t, ident) + got = api.iam.VerifyActionPermission(inlineCondRequest(t, http.MethodPut), ident, + s3_constants.ACTION_WRITE, inlineCondTestBucket, "obj") + assert.Equal(t, s3err.ErrAccessDenied, got, + "after DeleteGroupPolicy, the group inline policy must no longer grant access") +} + +// TestUserInlinePolicy_ConditionDiscriminatesAllowVsDeny pairs allow and deny +// against the same user with the same Actions list. If the engine path is not +// in use, both calls would hit the legacy Actions branch and both would be +// allowed; the discriminator is that switching the CIDR must flip the result. +func TestUserInlinePolicy_ConditionDiscriminatesAllowVsDeny(t *testing.T) { + api := NewEmbeddedIamApiForTest() + api.mockConfig = &iam_pb.S3ApiConfiguration{} + seedInlineCondUser(t, api) + + // Allow from loopback first: must succeed. + _, iamErr := api.PutUserPolicy(api.mockConfig, url.Values{ + "UserName": {"alice"}, + "PolicyName": {"P"}, + "PolicyDocument": {inlineCondPolicyDoc("127.0.0.0/8")}, + }) + require.Nil(t, iamErr) + require.NoError(t, api.PutS3ApiConfiguration(api.mockConfig)) + require.NoError(t, api.iam.LoadS3ApiConfigurationFromCredentialManager()) + + ident := api.iam.lookupByIdentityName("alice") + require.NotNil(t, ident) + got := api.iam.VerifyActionPermission(inlineCondRequest(t, http.MethodPut), ident, + s3_constants.ACTION_WRITE, inlineCondTestBucket, "obj") + require.Equal(t, s3err.ErrNone, got, "policy with matching CIDR must allow") + + // Replace with non-matching CIDR: must now deny. Same user, same action, + // same Actions list (Admin:bucket). Only the Condition block changed. + _, iamErr = api.PutUserPolicy(api.mockConfig, url.Values{ + "UserName": {"alice"}, + "PolicyName": {"P"}, + "PolicyDocument": {inlineCondPolicyDoc("198.51.100.0/24")}, + }) + require.Nil(t, iamErr) + require.NoError(t, api.PutS3ApiConfiguration(api.mockConfig)) + require.NoError(t, api.iam.LoadS3ApiConfigurationFromCredentialManager()) + + ident = api.iam.lookupByIdentityName("alice") + require.NotNil(t, ident) + got = api.iam.VerifyActionPermission(inlineCondRequest(t, http.MethodPut), ident, + s3_constants.ACTION_WRITE, inlineCondTestBucket, "obj") + require.Equal(t, s3err.ErrAccessDenied, got, + "flipping aws:SourceIp to a non-matching CIDR must flip the decision; "+ + "this proves the engine (not the legacy Actions list) drives the outcome") +} + +// TestUserInlinePolicy_ReloadFromStore verifies that after the IAM is reloaded +// (simulating a restart), the stored user inline policy is re-registered and +// its Condition block is still enforced. Catches a regression where the engine +// state is rebuilt only from managed policies, not inline ones. +func TestUserInlinePolicy_ReloadFromStore(t *testing.T) { + api := NewEmbeddedIamApiForTest() + api.mockConfig = &iam_pb.S3ApiConfiguration{} + seedInlineCondUser(t, api) + + // Persist an inline policy via the credential store directly, bypassing + // the API. This is the on-disk state a fresh process boots into. + var doc policy_engine.PolicyDocument + require.NoError(t, doc.UnmarshalJSON([]byte(inlineCondPolicyDoc("198.51.100.0/24")))) + require.NoError(t, api.credentialManager.PutUserInlinePolicy( + context.Background(), "alice", "OnlyFromTestNet", doc)) + + // Simulate restart: full reload from credential store. + require.NoError(t, api.iam.LoadS3ApiConfigurationFromCredentialManager()) + + ident := api.iam.lookupByIdentityName("alice") + require.NotNil(t, ident) + + req := inlineCondRequest(t, http.MethodPut) + got := api.iam.VerifyActionPermission(req, ident, s3_constants.ACTION_WRITE, inlineCondTestBucket, "obj") + assert.Equal(t, s3err.ErrAccessDenied, got, + "after reload, the persisted user inline policy's aws:SourceIp condition must still be honored") +}