diff options
| author | chrislu <chris.lu@gmail.com> | 2025-12-09 00:24:54 -0800 |
|---|---|---|
| committer | chrislu <chris.lu@gmail.com> | 2025-12-09 00:24:54 -0800 |
| commit | d93c90fdb3d6123ad9398bb595fb4bf16d5918a7 (patch) | |
| tree | 03b9f1946a1ed9236607863bf0be6494fbf364e1 | |
| parent | b198a0e8d1b874cb5ead7a7b6ef1af3c304341b6 (diff) | |
| download | seaweedfs-d93c90fdb3d6123ad9398bb595fb4bf16d5918a7.tar.xz seaweedfs-d93c90fdb3d6123ad9398bb595fb4bf16d5918a7.zip | |
Address code review nitpicks
- Remove unused extractObjectTags placeholder function (engine.go)
- Add clarifying comment about s3:ExistingObjectTag/<key> evaluation
- Consolidate duplicate tag-based examples in README
- Factor out tagsToEntry helper to package level in tests
| -rw-r--r-- | weed/s3api/policy_engine/README_POLICY_ENGINE.md | 77 | ||||
| -rw-r--r-- | weed/s3api/policy_engine/engine.go | 13 | ||||
| -rw-r--r-- | weed/s3api/policy_engine/engine_test.go | 46 |
3 files changed, 42 insertions, 94 deletions
diff --git a/weed/s3api/policy_engine/README_POLICY_ENGINE.md b/weed/s3api/policy_engine/README_POLICY_ENGINE.md index 13d2e0029..ddd872cfd 100644 --- a/weed/s3api/policy_engine/README_POLICY_ENGINE.md +++ b/weed/s3api/policy_engine/README_POLICY_ENGINE.md @@ -163,6 +163,33 @@ You can control access based on object tags using `s3:ExistingObjectTag/<tag-key This allows anonymous access only to objects that have a tag `status=public`. +**Deny access to confidential objects:** + +```json +{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": "*", + "Action": "s3:GetObject", + "Resource": "arn:aws:s3:::my-bucket/*" + }, + { + "Effect": "Deny", + "Principal": "*", + "Action": "s3:GetObject", + "Resource": "arn:aws:s3:::my-bucket/*", + "Condition": { + "StringEquals": { + "s3:ExistingObjectTag/classification": ["confidential", "secret"] + } + } + } + ] +} +``` + **Supported Operations for Tag-Based Conditions:** Tag-based conditions (`s3:ExistingObjectTag/<key>`) are evaluated for the following operations: @@ -247,56 +274,6 @@ Note: For these conditions to be evaluated, the object must exist and the policy } ``` -### Tag-Based Access Control - -Allow public read only for objects tagged as public: - -```json -{ - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Principal": "*", - "Action": "s3:GetObject", - "Resource": "arn:aws:s3:::my-bucket/*", - "Condition": { - "StringEquals": { - "s3:ExistingObjectTag/visibility": ["public"] - } - } - } - ] -} -``` - -Deny access to confidential objects: - -```json -{ - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Principal": "*", - "Action": "s3:GetObject", - "Resource": "arn:aws:s3:::my-bucket/*" - }, - { - "Effect": "Deny", - "Principal": "*", - "Action": "s3:GetObject", - "Resource": "arn:aws:s3:::my-bucket/*", - "Condition": { - "StringEquals": { - "s3:ExistingObjectTag/classification": ["confidential", "secret"] - } - } - } - ] -} -``` - ## Integration ### For Existing SeaweedFS Users diff --git a/weed/s3api/policy_engine/engine.go b/weed/s3api/policy_engine/engine.go index db4317b62..62e375eff 100644 --- a/weed/s3api/policy_engine/engine.go +++ b/weed/s3api/policy_engine/engine.go @@ -209,10 +209,8 @@ func ExtractConditionValuesFromRequest(r *http.Request) map[string][]string { values["aws:Referer"] = []string{referer} } - // S3 object-level conditions - if r.Method == "GET" || r.Method == "HEAD" { - values["s3:ExistingObjectTag"] = extractObjectTags(r) - } + // Note: s3:ExistingObjectTag/<key> conditions are evaluated using objectEntry + // passed to EvaluatePolicy, not extracted from the request. // S3 bucket-level conditions if delimiter := r.URL.Query().Get("delimiter"); delimiter != "" { @@ -251,13 +249,6 @@ func ExtractConditionValuesFromRequest(r *http.Request) map[string][]string { return values } -// extractObjectTags extracts object tags from request (placeholder implementation) -func extractObjectTags(r *http.Request) []string { - // This would need to be implemented based on how object tags are stored - // For now, return empty slice - return []string{} -} - // BuildResourceArn builds an ARN for the given bucket and object func BuildResourceArn(bucketName, objectName string) string { if objectName == "" { diff --git a/weed/s3api/policy_engine/engine_test.go b/weed/s3api/policy_engine/engine_test.go index 7ad2ca35b..4de537ac1 100644 --- a/weed/s3api/policy_engine/engine_test.go +++ b/weed/s3api/policy_engine/engine_test.go @@ -9,6 +9,19 @@ import ( "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" ) +// tagsToEntry converts a map of tag key-value pairs to the entry.Extended format +// used for s3:ExistingObjectTag/<key> condition evaluation +func tagsToEntry(tags map[string]string) map[string][]byte { + if tags == nil { + return nil + } + entry := make(map[string][]byte) + for k, v := range tags { + entry[s3_constants.AmzObjectTaggingPrefix+k] = []byte(v) + } + return entry +} + func TestPolicyEngine(t *testing.T) { engine := NewPolicyEngine() @@ -743,18 +756,6 @@ func TestExistingObjectTagCondition(t *testing.T) { t.Fatalf("Failed to set bucket policy: %v", err) } - // Helper to convert tags to entry.Extended format - tagsToEntry := func(tags map[string]string) map[string][]byte { - if tags == nil { - return nil - } - entry := make(map[string][]byte) - for k, v := range tags { - entry[s3_constants.AmzObjectTaggingPrefix+k] = []byte(v) - } - return entry - } - tests := []struct { name string objectTags map[string]string @@ -837,15 +838,6 @@ func TestExistingObjectTagConditionMultipleTags(t *testing.T) { t.Fatalf("Failed to set bucket policy: %v", err) } - // Helper to convert tags to entry.Extended format - tagsToEntry := func(tags map[string]string) map[string][]byte { - entry := make(map[string][]byte) - for k, v := range tags { - entry[s3_constants.AmzObjectTaggingPrefix+k] = []byte(v) - } - return entry - } - tests := []struct { name string objectTags map[string]string @@ -928,18 +920,6 @@ func TestExistingObjectTagDenyPolicy(t *testing.T) { t.Fatalf("Failed to set bucket policy: %v", err) } - // Helper to convert tags to entry.Extended format - tagsToEntry := func(tags map[string]string) map[string][]byte { - if tags == nil { - return nil - } - entry := make(map[string][]byte) - for k, v := range tags { - entry[s3_constants.AmzObjectTaggingPrefix+k] = []byte(v) - } - return entry - } - tests := []struct { name string objectTags map[string]string |
