s3: enforce authentication and JSON error format for Iceberg REST Catalog (#8192)
* s3: enforce authentication and JSON error format for Iceberg REST Catalog * s3/iceberg: align error exception types with OpenAPI spec examples * s3api: refactor AuthenticateRequest to return identity object * s3/iceberg: propagate full identity object to request context * s3/iceberg: differentiate NotAuthorizedException and ForbiddenException * s3/iceberg: reject requests if authenticator is nil to prevent auth bypass * s3/iceberg: refactor Auth middleware to build context incrementally and use switch for error mapping * s3api: update misleading comment for authRequestWithAuthType * s3api: return ErrAccessDenied if IAM is not configured to prevent auth bypass * s3/iceberg: optimize context update in Auth middleware * s3api: export CanDo for external authorization use * s3/iceberg: enforce identity-based authorization in all API handlers * s3api: fix compilation errors by updating internal CanDo references * s3/iceberg: robust identity validation and consistent action usage in handlers * s3api: complete CanDo rename across tests and policy engine integration * s3api: fix integration tests by allowing admin access when auth is disabled and explicit gRPC ports * duckdb * create test bucket
This commit is contained in:
@@ -86,11 +86,11 @@ func TestCanDo(t *testing.T) {
|
||||
},
|
||||
}
|
||||
// object specific
|
||||
assert.Equal(t, true, ident1.canDo(ACTION_WRITE, "bucket1", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident1.canDo(ACTION_WRITE, "bucket1", "/a/b/c/d/e.txt"))
|
||||
assert.Equal(t, false, ident1.canDo(ACTION_DELETE_BUCKET, "bucket1", ""))
|
||||
assert.Equal(t, false, ident1.canDo(ACTION_WRITE, "bucket1", "/a/b/other/some"), "action without *")
|
||||
assert.Equal(t, false, ident1.canDo(ACTION_WRITE, "bucket1", "/a/b/*"), "action on parent directory")
|
||||
assert.Equal(t, true, ident1.CanDo(ACTION_WRITE, "bucket1", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident1.CanDo(ACTION_WRITE, "bucket1", "/a/b/c/d/e.txt"))
|
||||
assert.Equal(t, false, ident1.CanDo(ACTION_DELETE_BUCKET, "bucket1", ""))
|
||||
assert.Equal(t, false, ident1.CanDo(ACTION_WRITE, "bucket1", "/a/b/other/some"), "action without *")
|
||||
assert.Equal(t, false, ident1.CanDo(ACTION_WRITE, "bucket1", "/a/b/*"), "action on parent directory")
|
||||
|
||||
// bucket specific
|
||||
ident2 := &Identity{
|
||||
@@ -101,11 +101,11 @@ func TestCanDo(t *testing.T) {
|
||||
"WriteAcp:bucket1",
|
||||
},
|
||||
}
|
||||
assert.Equal(t, true, ident2.canDo(ACTION_READ, "bucket1", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident2.canDo(ACTION_WRITE, "bucket1", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident2.canDo(ACTION_WRITE_ACP, "bucket1", ""))
|
||||
assert.Equal(t, false, ident2.canDo(ACTION_READ_ACP, "bucket1", ""))
|
||||
assert.Equal(t, false, ident2.canDo(ACTION_LIST, "bucket1", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident2.CanDo(ACTION_READ, "bucket1", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident2.CanDo(ACTION_WRITE, "bucket1", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident2.CanDo(ACTION_WRITE_ACP, "bucket1", ""))
|
||||
assert.Equal(t, false, ident2.CanDo(ACTION_READ_ACP, "bucket1", ""))
|
||||
assert.Equal(t, false, ident2.CanDo(ACTION_LIST, "bucket1", "/a/b/c/d.txt"))
|
||||
|
||||
// across buckets
|
||||
ident3 := &Identity{
|
||||
@@ -115,10 +115,10 @@ func TestCanDo(t *testing.T) {
|
||||
"Write",
|
||||
},
|
||||
}
|
||||
assert.Equal(t, true, ident3.canDo(ACTION_READ, "bucket1", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident3.canDo(ACTION_WRITE, "bucket1", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, false, ident3.canDo(ACTION_LIST, "bucket1", "/a/b/other/some"))
|
||||
assert.Equal(t, false, ident3.canDo(ACTION_WRITE_ACP, "bucket1", ""))
|
||||
assert.Equal(t, true, ident3.CanDo(ACTION_READ, "bucket1", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident3.CanDo(ACTION_WRITE, "bucket1", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, false, ident3.CanDo(ACTION_LIST, "bucket1", "/a/b/other/some"))
|
||||
assert.Equal(t, false, ident3.CanDo(ACTION_WRITE_ACP, "bucket1", ""))
|
||||
|
||||
// partial buckets
|
||||
ident4 := &Identity{
|
||||
@@ -128,9 +128,9 @@ func TestCanDo(t *testing.T) {
|
||||
"ReadAcp:special_*",
|
||||
},
|
||||
}
|
||||
assert.Equal(t, true, ident4.canDo(ACTION_READ, "special_bucket", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident4.canDo(ACTION_READ_ACP, "special_bucket", ""))
|
||||
assert.Equal(t, false, ident4.canDo(ACTION_READ, "bucket1", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident4.CanDo(ACTION_READ, "special_bucket", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident4.CanDo(ACTION_READ_ACP, "special_bucket", ""))
|
||||
assert.Equal(t, false, ident4.CanDo(ACTION_READ, "bucket1", "/a/b/c/d.txt"))
|
||||
|
||||
// admin buckets
|
||||
ident5 := &Identity{
|
||||
@@ -139,10 +139,10 @@ func TestCanDo(t *testing.T) {
|
||||
"Admin:special_*",
|
||||
},
|
||||
}
|
||||
assert.Equal(t, true, ident5.canDo(ACTION_READ, "special_bucket", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident5.canDo(ACTION_READ_ACP, "special_bucket", ""))
|
||||
assert.Equal(t, true, ident5.canDo(ACTION_WRITE, "special_bucket", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident5.canDo(ACTION_WRITE_ACP, "special_bucket", ""))
|
||||
assert.Equal(t, true, ident5.CanDo(ACTION_READ, "special_bucket", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident5.CanDo(ACTION_READ_ACP, "special_bucket", ""))
|
||||
assert.Equal(t, true, ident5.CanDo(ACTION_WRITE, "special_bucket", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident5.CanDo(ACTION_WRITE_ACP, "special_bucket", ""))
|
||||
|
||||
// anonymous buckets
|
||||
ident6 := &Identity{
|
||||
@@ -151,7 +151,7 @@ func TestCanDo(t *testing.T) {
|
||||
"Read",
|
||||
},
|
||||
}
|
||||
assert.Equal(t, true, ident6.canDo(ACTION_READ, "anything_bucket", "/a/b/c/d.txt"))
|
||||
assert.Equal(t, true, ident6.CanDo(ACTION_READ, "anything_bucket", "/a/b/c/d.txt"))
|
||||
|
||||
//test deleteBucket operation
|
||||
ident7 := &Identity{
|
||||
@@ -160,7 +160,7 @@ func TestCanDo(t *testing.T) {
|
||||
"DeleteBucket:bucket1",
|
||||
},
|
||||
}
|
||||
assert.Equal(t, true, ident7.canDo(ACTION_DELETE_BUCKET, "bucket1", ""))
|
||||
assert.Equal(t, true, ident7.CanDo(ACTION_DELETE_BUCKET, "bucket1", ""))
|
||||
}
|
||||
|
||||
func TestMatchWildcardPattern(t *testing.T) {
|
||||
@@ -580,7 +580,7 @@ func TestBucketLevelListPermissions(t *testing.T) {
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
result := identity.canDo(tc.action, tc.bucket, tc.object)
|
||||
result := identity.CanDo(tc.action, tc.bucket, tc.object)
|
||||
assert.Equal(t, tc.shouldAllow, result, tc.description)
|
||||
})
|
||||
}
|
||||
@@ -599,7 +599,7 @@ func TestBucketLevelListPermissions(t *testing.T) {
|
||||
testCases := []string{"anybucket", "mybucket", "test-bucket", "prod-data"}
|
||||
|
||||
for _, bucket := range testCases {
|
||||
result := identity.canDo("List", bucket, "")
|
||||
result := identity.CanDo("List", bucket, "")
|
||||
assert.True(t, result, "Global List permission should allow access to bucket %s", bucket)
|
||||
}
|
||||
})
|
||||
@@ -614,9 +614,9 @@ func TestBucketLevelListPermissions(t *testing.T) {
|
||||
}
|
||||
|
||||
// Should only allow access to the exact bucket
|
||||
assert.True(t, identity.canDo("List", "specificbucket", ""), "Should allow access to exact bucket")
|
||||
assert.False(t, identity.canDo("List", "specificbucket-test", ""), "Should deny access to bucket with suffix")
|
||||
assert.False(t, identity.canDo("List", "otherbucket", ""), "Should deny access to different bucket")
|
||||
assert.True(t, identity.CanDo("List", "specificbucket", ""), "Should allow access to exact bucket")
|
||||
assert.False(t, identity.CanDo("List", "specificbucket-test", ""), "Should deny access to bucket with suffix")
|
||||
assert.False(t, identity.CanDo("List", "otherbucket", ""), "Should deny access to different bucket")
|
||||
})
|
||||
|
||||
t.Log("This test validates the fix for issue #7066")
|
||||
@@ -639,26 +639,26 @@ func TestListBucketsAuthRequest(t *testing.T) {
|
||||
}
|
||||
|
||||
// Test 1: ListBuckets operation should succeed (bucket = "")
|
||||
// This would have failed before the fix because canDo("List", "", "") would return false
|
||||
// After the fix, it bypasses the canDo check for ListBuckets operations
|
||||
// This would have failed before the fix because CanDo("List", "", "") would return false
|
||||
// After the fix, it bypasses the CanDo check for ListBuckets operations
|
||||
|
||||
// Simulate what happens in authRequest for ListBuckets:
|
||||
// action = ACTION_LIST, bucket = "", object = ""
|
||||
|
||||
// Before fix: identity.canDo(ACTION_LIST, "", "") would fail
|
||||
// After fix: the canDo check should be bypassed
|
||||
// Before fix: identity.CanDo(ACTION_LIST, "", "") would fail
|
||||
// After fix: the CanDo check should be bypassed
|
||||
|
||||
// Test the individual canDo method to show it would fail without the special case
|
||||
result := identity.canDo(Action(ACTION_LIST), "", "")
|
||||
assert.False(t, result, "canDo should return false for empty bucket with bucket-specific permissions")
|
||||
// Test the individual CanDo method to show it would fail without the special case
|
||||
result := identity.CanDo(Action(ACTION_LIST), "", "")
|
||||
assert.False(t, result, "CanDo should return false for empty bucket with bucket-specific permissions")
|
||||
|
||||
// Test with a specific bucket that matches the permission
|
||||
result2 := identity.canDo(Action(ACTION_LIST), "mybucket", "")
|
||||
assert.True(t, result2, "canDo should return true for matching bucket")
|
||||
result2 := identity.CanDo(Action(ACTION_LIST), "mybucket", "")
|
||||
assert.True(t, result2, "CanDo should return true for matching bucket")
|
||||
|
||||
// Test with a specific bucket that doesn't match
|
||||
result3 := identity.canDo(Action(ACTION_LIST), "otherbucket", "")
|
||||
assert.False(t, result3, "canDo should return false for non-matching bucket")
|
||||
result3 := identity.CanDo(Action(ACTION_LIST), "otherbucket", "")
|
||||
assert.False(t, result3, "CanDo should return false for non-matching bucket")
|
||||
})
|
||||
|
||||
t.Run("Object listing maintains permission enforcement", func(t *testing.T) {
|
||||
@@ -675,14 +675,14 @@ func TestListBucketsAuthRequest(t *testing.T) {
|
||||
// These operations have a specific bucket in the URL
|
||||
|
||||
// Should succeed for allowed bucket
|
||||
result1 := identity.canDo(Action(ACTION_LIST), "mybucket", "prefix/")
|
||||
result1 := identity.CanDo(Action(ACTION_LIST), "mybucket", "prefix/")
|
||||
assert.True(t, result1, "Should allow listing objects in permitted bucket")
|
||||
|
||||
result2 := identity.canDo(Action(ACTION_LIST), "mybucket-prod", "")
|
||||
result2 := identity.CanDo(Action(ACTION_LIST), "mybucket-prod", "")
|
||||
assert.True(t, result2, "Should allow listing objects in wildcard-matched bucket")
|
||||
|
||||
// Should fail for disallowed bucket
|
||||
result3 := identity.canDo(Action(ACTION_LIST), "otherbucket", "")
|
||||
result3 := identity.CanDo(Action(ACTION_LIST), "otherbucket", "")
|
||||
assert.False(t, result3, "Should deny listing objects in non-permitted bucket")
|
||||
})
|
||||
|
||||
@@ -734,11 +734,11 @@ func TestSignatureVerificationDoesNotCheckPermissions(t *testing.T) {
|
||||
assert.Equal(t, "list_secret_key", cred.SecretKey)
|
||||
|
||||
// User should have the correct permissions
|
||||
assert.True(t, identity.canDo(Action(ACTION_LIST), "bucket-123", ""))
|
||||
assert.True(t, identity.canDo(Action(ACTION_READ), "bucket-123", ""))
|
||||
assert.True(t, identity.CanDo(Action(ACTION_LIST), "bucket-123", ""))
|
||||
assert.True(t, identity.CanDo(Action(ACTION_READ), "bucket-123", ""))
|
||||
|
||||
// User should NOT have write permissions
|
||||
assert.False(t, identity.canDo(Action(ACTION_WRITE), "bucket-123", ""))
|
||||
assert.False(t, identity.CanDo(Action(ACTION_WRITE), "bucket-123", ""))
|
||||
})
|
||||
|
||||
t.Log("This test validates the fix for issue #7334")
|
||||
|
||||
Reference in New Issue
Block a user