aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorchrislu <chris.lu@gmail.com>2025-12-09 00:24:54 -0800
committerchrislu <chris.lu@gmail.com>2025-12-09 00:24:54 -0800
commitd93c90fdb3d6123ad9398bb595fb4bf16d5918a7 (patch)
tree03b9f1946a1ed9236607863bf0be6494fbf364e1
parentb198a0e8d1b874cb5ead7a7b6ef1af3c304341b6 (diff)
downloadseaweedfs-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.md77
-rw-r--r--weed/s3api/policy_engine/engine.go13
-rw-r--r--weed/s3api/policy_engine/engine_test.go46
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