chore: remove ~50k lines of unreachable dead code (#8913)
* chore: remove unreachable dead code across the codebase Remove ~50,000 lines of unreachable code identified by static analysis. Major removals: - weed/filer/redis_lua: entire unused Redis Lua filer store implementation - weed/wdclient/net2, resource_pool: unused connection/resource pool packages - weed/plugin/worker/lifecycle: unused lifecycle plugin worker - weed/s3api: unused S3 policy templates, presigned URL IAM, streaming copy, multipart IAM, key rotation, and various SSE helper functions - weed/mq/kafka: unused partition mapping, compression, schema, and protocol functions - weed/mq/offset: unused SQL storage and migration code - weed/worker: unused registry, task, and monitoring functions - weed/query: unused SQL engine, parquet scanner, and type functions - weed/shell: unused EC proportional rebalance functions - weed/storage/erasure_coding/distribution: unused distribution analysis functions - Individual unreachable functions removed from 150+ files across admin, credential, filer, iam, kms, mount, mq, operation, pb, s3api, server, shell, storage, topology, and util packages * fix(s3): reset shared memory store in IAM test to prevent flaky failure TestLoadIAMManagerFromConfig_EmptyConfigWithFallbackKey was flaky because the MemoryStore credential backend is a singleton registered via init(). Earlier tests that create anonymous identities pollute the shared store, causing LookupAnonymous() to unexpectedly return true. Fix by calling Reset() on the memory store before the test runs. * style: run gofmt on changed files * fix: restore KMS functions used by integration tests * fix(plugin): prevent panic on send to closed worker session channel The Plugin.sendToWorker method could panic with "send on closed channel" when a worker disconnected while a message was being sent. The race was between streamSession.close() closing the outgoing channel and sendToWorker writing to it concurrently. Add a done channel to streamSession that is closed before the outgoing channel, and check it in sendToWorker's select to safely detect closed sessions without panicking.
This commit is contained in:
Chris Lu
@@ -125,22 +125,6 @@ func (c *NormalizedValueCache) evictLeastRecentlyUsed() {
|
||||
delete(c.cache, tail.key)
|
||||
}
|
||||
|
||||
// Clear clears all cached values
|
||||
func (c *NormalizedValueCache) Clear() {
|
||||
c.mu.Lock()
|
||||
defer c.mu.Unlock()
|
||||
c.cache = make(map[string]*LRUNode)
|
||||
c.head.next = c.tail
|
||||
c.tail.prev = c.head
|
||||
}
|
||||
|
||||
// GetStats returns cache statistics
|
||||
func (c *NormalizedValueCache) GetStats() (size int, maxSize int) {
|
||||
c.mu.RLock()
|
||||
defer c.mu.RUnlock()
|
||||
return len(c.cache), c.maxSize
|
||||
}
|
||||
|
||||
// Global cache instance with size limit
|
||||
var normalizedValueCache = NewNormalizedValueCache(1000)
|
||||
|
||||
@@ -769,34 +753,3 @@ func EvaluateConditions(conditions PolicyConditions, contextValues map[string][]
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
// EvaluateConditionsLegacy evaluates conditions using the old interface{} format for backward compatibility
|
||||
// objectEntry is the object's metadata from entry.Extended (can be nil)
|
||||
func EvaluateConditionsLegacy(conditions map[string]interface{}, contextValues map[string][]string, objectEntry map[string][]byte) bool {
|
||||
if len(conditions) == 0 {
|
||||
return true // No conditions means always true
|
||||
}
|
||||
|
||||
for operator, conditionMap := range conditions {
|
||||
conditionEvaluator, err := GetConditionEvaluator(operator)
|
||||
if err != nil {
|
||||
glog.Warningf("Unsupported condition operator: %s", operator)
|
||||
continue
|
||||
}
|
||||
|
||||
conditionMapTyped, ok := conditionMap.(map[string]interface{})
|
||||
if !ok {
|
||||
glog.Warningf("Invalid condition format for operator: %s", operator)
|
||||
continue
|
||||
}
|
||||
|
||||
for key, value := range conditionMapTyped {
|
||||
contextVals := getConditionContextValue(key, contextValues, objectEntry)
|
||||
if !conditionEvaluator.Evaluate(value, contextVals) {
|
||||
return false // If any condition fails, the whole condition block fails
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -610,92 +610,6 @@ func BuildActionName(action string) string {
|
||||
return fmt.Sprintf("s3:%s", action)
|
||||
}
|
||||
|
||||
// IsReadAction checks if an action is a read action
|
||||
func IsReadAction(action string) bool {
|
||||
readActions := []string{
|
||||
"s3:GetObject",
|
||||
"s3:GetObjectVersion",
|
||||
"s3:GetObjectAcl",
|
||||
"s3:GetObjectVersionAcl",
|
||||
"s3:GetObjectTagging",
|
||||
"s3:GetObjectVersionTagging",
|
||||
"s3:ListBucket",
|
||||
"s3:ListBucketVersions",
|
||||
"s3:GetBucketLocation",
|
||||
"s3:GetBucketVersioning",
|
||||
"s3:GetBucketAcl",
|
||||
"s3:GetBucketCors",
|
||||
"s3:GetBucketPolicy",
|
||||
"s3:GetBucketTagging",
|
||||
"s3:GetBucketNotification",
|
||||
"s3:GetBucketObjectLockConfiguration",
|
||||
"s3:GetObjectRetention",
|
||||
"s3:GetObjectLegalHold",
|
||||
}
|
||||
|
||||
for _, readAction := range readActions {
|
||||
if action == readAction {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// IsWriteAction checks if an action is a write action
|
||||
func IsWriteAction(action string) bool {
|
||||
writeActions := []string{
|
||||
"s3:PutObject",
|
||||
"s3:PutObjectAcl",
|
||||
"s3:PutObjectTagging",
|
||||
"s3:DeleteObject",
|
||||
"s3:DeleteObjectVersion",
|
||||
"s3:DeleteObjectTagging",
|
||||
"s3:AbortMultipartUpload",
|
||||
"s3:ListMultipartUploads",
|
||||
"s3:ListParts",
|
||||
"s3:PutBucketAcl",
|
||||
"s3:PutBucketCors",
|
||||
"s3:PutBucketPolicy",
|
||||
"s3:PutBucketTagging",
|
||||
"s3:PutBucketNotification",
|
||||
"s3:PutBucketVersioning",
|
||||
"s3:DeleteBucketPolicy",
|
||||
"s3:DeleteBucketTagging",
|
||||
"s3:DeleteBucketCors",
|
||||
"s3:PutBucketObjectLockConfiguration",
|
||||
"s3:PutObjectRetention",
|
||||
"s3:PutObjectLegalHold",
|
||||
"s3:BypassGovernanceRetention",
|
||||
}
|
||||
|
||||
for _, writeAction := range writeActions {
|
||||
if action == writeAction {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// GetBucketNameFromArn extracts bucket name from ARN
|
||||
func GetBucketNameFromArn(arn string) string {
|
||||
if strings.HasPrefix(arn, "arn:aws:s3:::") {
|
||||
parts := strings.SplitN(arn[13:], "/", 2)
|
||||
return parts[0]
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// GetObjectNameFromArn extracts object name from ARN
|
||||
func GetObjectNameFromArn(arn string) string {
|
||||
if strings.HasPrefix(arn, "arn:aws:s3:::") {
|
||||
parts := strings.SplitN(arn[13:], "/", 2)
|
||||
if len(parts) > 1 {
|
||||
return parts[1]
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// GetPolicyStatements returns all policy statements for a bucket
|
||||
func (engine *PolicyEngine) GetPolicyStatements(bucketName string) []PolicyStatement {
|
||||
engine.mutex.RLock()
|
||||
|
||||
@@ -6,7 +6,6 @@ import (
|
||||
"testing"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
|
||||
"github.com/seaweedfs/seaweedfs/weed/util/wildcard"
|
||||
)
|
||||
|
||||
@@ -226,47 +225,6 @@ func TestConditionEvaluators(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestConvertIdentityToPolicy(t *testing.T) {
|
||||
identityActions := []string{
|
||||
"Read:bucket1/*",
|
||||
"Write:bucket1/*",
|
||||
"Admin:bucket2",
|
||||
}
|
||||
|
||||
policy, err := ConvertIdentityToPolicy(identityActions)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to convert identity to policy: %v", err)
|
||||
}
|
||||
|
||||
if policy.Version != "2012-10-17" {
|
||||
t.Errorf("Expected version 2012-10-17, got %s", policy.Version)
|
||||
}
|
||||
|
||||
if len(policy.Statement) != 3 {
|
||||
t.Errorf("Expected 3 statements, got %d", len(policy.Statement))
|
||||
}
|
||||
|
||||
// Check first statement (Read)
|
||||
stmt := policy.Statement[0]
|
||||
if stmt.Effect != PolicyEffectAllow {
|
||||
t.Errorf("Expected Allow effect, got %s", stmt.Effect)
|
||||
}
|
||||
|
||||
actions := normalizeToStringSlice(stmt.Action)
|
||||
// Read action now includes: GetObject, GetObjectVersion, ListBucket, ListBucketVersions,
|
||||
// GetObjectAcl, GetObjectVersionAcl, GetObjectTagging, GetObjectVersionTagging,
|
||||
// GetBucketLocation, GetBucketVersioning, GetBucketAcl, GetBucketCors, GetBucketTagging, GetBucketNotification
|
||||
if len(actions) != 14 {
|
||||
t.Errorf("Expected 14 read actions, got %d: %v", len(actions), actions)
|
||||
}
|
||||
|
||||
resources := normalizeToStringSlice(stmt.Resource)
|
||||
// Read action now includes both bucket ARN (for ListBucket*) and object ARN (for GetObject*)
|
||||
if len(resources) != 2 {
|
||||
t.Errorf("Expected 2 resources (bucket and bucket/*), got %d: %v", len(resources), resources)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPolicyValidation(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
@@ -794,41 +752,6 @@ func TestCompilePolicy(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestNewPolicyBackedIAMWithLegacy tests the constructor overload
|
||||
func TestNewPolicyBackedIAMWithLegacy(t *testing.T) {
|
||||
// Mock legacy IAM
|
||||
mockLegacyIAM := &MockLegacyIAM{}
|
||||
|
||||
// Test the new constructor
|
||||
policyBackedIAM := NewPolicyBackedIAMWithLegacy(mockLegacyIAM)
|
||||
|
||||
// Verify that the legacy IAM is set
|
||||
if policyBackedIAM.legacyIAM != mockLegacyIAM {
|
||||
t.Errorf("Expected legacy IAM to be set, but it wasn't")
|
||||
}
|
||||
|
||||
// Verify that the policy engine is initialized
|
||||
if policyBackedIAM.policyEngine == nil {
|
||||
t.Errorf("Expected policy engine to be initialized, but it wasn't")
|
||||
}
|
||||
|
||||
// Compare with the traditional approach
|
||||
traditionalIAM := NewPolicyBackedIAM()
|
||||
traditionalIAM.SetLegacyIAM(mockLegacyIAM)
|
||||
|
||||
// Both should behave the same
|
||||
if policyBackedIAM.legacyIAM != traditionalIAM.legacyIAM {
|
||||
t.Errorf("Expected both approaches to result in the same legacy IAM")
|
||||
}
|
||||
}
|
||||
|
||||
// MockLegacyIAM implements the LegacyIAM interface for testing
|
||||
type MockLegacyIAM struct{}
|
||||
|
||||
func (m *MockLegacyIAM) authRequest(r *http.Request, action Action) (Identity, s3err.ErrorCode) {
|
||||
return nil, s3err.ErrNone
|
||||
}
|
||||
|
||||
// TestExistingObjectTagCondition tests s3:ExistingObjectTag/<tag-key> condition support
|
||||
func TestExistingObjectTagCondition(t *testing.T) {
|
||||
engine := NewPolicyEngine()
|
||||
|
||||
@@ -1,642 +0,0 @@
|
||||
package policy_engine
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/glog"
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
|
||||
)
|
||||
|
||||
// Action represents an S3 action - this should match the type in auth_credentials.go
|
||||
type Action string
|
||||
|
||||
// Identity represents a user identity - this should match the type in auth_credentials.go
|
||||
type Identity interface {
|
||||
CanDo(action Action, bucket string, objectKey string) bool
|
||||
}
|
||||
|
||||
// PolicyBackedIAM provides policy-based access control with fallback to legacy IAM
|
||||
type PolicyBackedIAM struct {
|
||||
policyEngine *PolicyEngine
|
||||
legacyIAM LegacyIAM // Interface to delegate to existing IAM system
|
||||
}
|
||||
|
||||
// LegacyIAM interface for delegating to existing IAM implementation
|
||||
type LegacyIAM interface {
|
||||
authRequest(r *http.Request, action Action) (Identity, s3err.ErrorCode)
|
||||
}
|
||||
|
||||
// NewPolicyBackedIAM creates a new policy-backed IAM system
|
||||
func NewPolicyBackedIAM() *PolicyBackedIAM {
|
||||
return &PolicyBackedIAM{
|
||||
policyEngine: NewPolicyEngine(),
|
||||
legacyIAM: nil, // Will be set when integrated with existing IAM
|
||||
}
|
||||
}
|
||||
|
||||
// NewPolicyBackedIAMWithLegacy creates a new policy-backed IAM system with legacy IAM set
|
||||
func NewPolicyBackedIAMWithLegacy(legacyIAM LegacyIAM) *PolicyBackedIAM {
|
||||
return &PolicyBackedIAM{
|
||||
policyEngine: NewPolicyEngine(),
|
||||
legacyIAM: legacyIAM,
|
||||
}
|
||||
}
|
||||
|
||||
// SetLegacyIAM sets the legacy IAM system for fallback
|
||||
func (p *PolicyBackedIAM) SetLegacyIAM(legacyIAM LegacyIAM) {
|
||||
p.legacyIAM = legacyIAM
|
||||
}
|
||||
|
||||
// SetBucketPolicy sets the policy for a bucket
|
||||
func (p *PolicyBackedIAM) SetBucketPolicy(bucketName string, policyJSON string) error {
|
||||
return p.policyEngine.SetBucketPolicy(bucketName, policyJSON)
|
||||
}
|
||||
|
||||
// GetBucketPolicy gets the policy for a bucket
|
||||
func (p *PolicyBackedIAM) GetBucketPolicy(bucketName string) (*PolicyDocument, error) {
|
||||
return p.policyEngine.GetBucketPolicy(bucketName)
|
||||
}
|
||||
|
||||
// DeleteBucketPolicy deletes the policy for a bucket
|
||||
func (p *PolicyBackedIAM) DeleteBucketPolicy(bucketName string) error {
|
||||
return p.policyEngine.DeleteBucketPolicy(bucketName)
|
||||
}
|
||||
|
||||
// CanDo checks if a principal can perform an action on a resource
|
||||
func (p *PolicyBackedIAM) CanDo(action, bucketName, objectName, principal string, r *http.Request) bool {
|
||||
// If there's a bucket policy, evaluate it
|
||||
if p.policyEngine.HasPolicyForBucket(bucketName) {
|
||||
result := p.policyEngine.EvaluatePolicyForRequest(bucketName, objectName, action, principal, r)
|
||||
switch result {
|
||||
case PolicyResultAllow:
|
||||
return true
|
||||
case PolicyResultDeny:
|
||||
return false
|
||||
case PolicyResultIndeterminate:
|
||||
// Fall through to legacy system
|
||||
}
|
||||
}
|
||||
|
||||
// No bucket policy or indeterminate result, use legacy conversion
|
||||
return p.evaluateLegacyAction(action, bucketName, objectName, principal)
|
||||
}
|
||||
|
||||
// evaluateLegacyAction evaluates actions using legacy identity-based rules
|
||||
func (p *PolicyBackedIAM) evaluateLegacyAction(action, bucketName, objectName, principal string) bool {
|
||||
// If we have a legacy IAM system to delegate to, use it
|
||||
if p.legacyIAM != nil {
|
||||
// Create a dummy request for legacy evaluation
|
||||
// In real implementation, this would use the actual request
|
||||
r := &http.Request{
|
||||
Header: make(http.Header),
|
||||
}
|
||||
|
||||
// Convert the action string to Action type
|
||||
legacyAction := Action(action)
|
||||
|
||||
// Use legacy IAM to check permission
|
||||
identity, errCode := p.legacyIAM.authRequest(r, legacyAction)
|
||||
if errCode != s3err.ErrNone {
|
||||
return false
|
||||
}
|
||||
|
||||
// If we have an identity, check if it can perform the action
|
||||
if identity != nil {
|
||||
return identity.CanDo(legacyAction, bucketName, objectName)
|
||||
}
|
||||
}
|
||||
|
||||
// No legacy IAM available, convert to policy and evaluate
|
||||
return p.evaluateUsingPolicyConversion(action, bucketName, objectName, principal)
|
||||
}
|
||||
|
||||
// evaluateUsingPolicyConversion converts legacy action to policy and evaluates
|
||||
func (p *PolicyBackedIAM) evaluateUsingPolicyConversion(action, bucketName, objectName, principal string) bool {
|
||||
// For now, use a conservative approach for legacy actions
|
||||
// In a real implementation, this would integrate with the existing identity system
|
||||
glog.V(2).Infof("Legacy action evaluation for %s on %s/%s by %s", action, bucketName, objectName, principal)
|
||||
|
||||
// Return false to maintain security until proper legacy integration is implemented
|
||||
// This ensures no unintended access is granted
|
||||
return false
|
||||
}
|
||||
|
||||
// extractBucketAndPrefix extracts bucket name and prefix from a resource pattern.
|
||||
// Examples:
|
||||
//
|
||||
// "bucket" -> bucket="bucket", prefix=""
|
||||
// "bucket/*" -> bucket="bucket", prefix=""
|
||||
// "bucket/prefix/*" -> bucket="bucket", prefix="prefix"
|
||||
// "bucket/a/b/c/*" -> bucket="bucket", prefix="a/b/c"
|
||||
func extractBucketAndPrefix(pattern string) (string, string) {
|
||||
// Validate input
|
||||
pattern = strings.TrimSpace(pattern)
|
||||
if pattern == "" || pattern == "/" {
|
||||
return "", ""
|
||||
}
|
||||
|
||||
// Remove trailing /* if present
|
||||
pattern = strings.TrimSuffix(pattern, "/*")
|
||||
|
||||
// Remove a single trailing slash to avoid empty path segments
|
||||
if strings.HasSuffix(pattern, "/") {
|
||||
pattern = pattern[:len(pattern)-1]
|
||||
}
|
||||
if pattern == "" {
|
||||
return "", ""
|
||||
}
|
||||
|
||||
// Split on the first /
|
||||
parts := strings.SplitN(pattern, "/", 2)
|
||||
bucket := strings.TrimSpace(parts[0])
|
||||
if bucket == "" {
|
||||
return "", ""
|
||||
}
|
||||
|
||||
if len(parts) == 1 {
|
||||
// No slash, entire pattern is bucket
|
||||
return bucket, ""
|
||||
}
|
||||
// Has slash, first part is bucket, rest is prefix
|
||||
prefix := strings.Trim(parts[1], "/")
|
||||
return bucket, prefix
|
||||
}
|
||||
|
||||
// buildObjectResourceArn generates ARNs for object-level access.
|
||||
// It properly handles both bucket-level (all objects) and prefix-level access.
|
||||
// Returns empty slice if bucket is invalid to prevent generating malformed ARNs.
|
||||
func buildObjectResourceArn(resourcePattern string) []string {
|
||||
bucket, prefix := extractBucketAndPrefix(resourcePattern)
|
||||
// If bucket is empty, the pattern is invalid; avoid generating malformed ARNs
|
||||
if bucket == "" {
|
||||
return []string{}
|
||||
}
|
||||
if prefix != "" {
|
||||
// Prefix-based access: restrict to objects under this prefix
|
||||
return []string{fmt.Sprintf("arn:aws:s3:::%s/%s/*", bucket, prefix)}
|
||||
}
|
||||
// Bucket-level access: all objects in bucket
|
||||
return []string{fmt.Sprintf("arn:aws:s3:::%s/*", bucket)}
|
||||
}
|
||||
|
||||
// ConvertIdentityToPolicy converts a legacy identity action to an AWS policy
|
||||
func ConvertIdentityToPolicy(identityActions []string) (*PolicyDocument, error) {
|
||||
statements := make([]PolicyStatement, 0)
|
||||
|
||||
for _, action := range identityActions {
|
||||
stmt, err := convertSingleAction(action)
|
||||
if err != nil {
|
||||
glog.Warningf("Failed to convert action %s: %v", action, err)
|
||||
continue
|
||||
}
|
||||
if stmt != nil {
|
||||
statements = append(statements, *stmt)
|
||||
}
|
||||
}
|
||||
|
||||
if len(statements) == 0 {
|
||||
return nil, fmt.Errorf("no valid statements generated")
|
||||
}
|
||||
|
||||
return &PolicyDocument{
|
||||
Version: PolicyVersion2012_10_17,
|
||||
Statement: statements,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// convertSingleAction converts a single legacy action to a policy statement.
|
||||
// action format: "ActionType:ResourcePattern" (e.g., "Write:bucket/prefix/*")
|
||||
func convertSingleAction(action string) (*PolicyStatement, error) {
|
||||
parts := strings.Split(action, ":")
|
||||
if len(parts) != 2 {
|
||||
return nil, fmt.Errorf("invalid action format: %s", action)
|
||||
}
|
||||
|
||||
actionType := parts[0]
|
||||
resourcePattern := parts[1]
|
||||
|
||||
var s3Actions []string
|
||||
var resources []string
|
||||
|
||||
switch actionType {
|
||||
case "Read":
|
||||
// Read includes both object-level (GetObject, GetObjectAcl, GetObjectTagging, GetObjectVersions)
|
||||
// and bucket-level operations (ListBucket, GetBucketLocation, GetBucketVersioning, GetBucketCors, etc.)
|
||||
s3Actions = []string{
|
||||
"s3:GetObject",
|
||||
"s3:GetObjectVersion",
|
||||
"s3:GetObjectAcl",
|
||||
"s3:GetObjectVersionAcl",
|
||||
"s3:GetObjectTagging",
|
||||
"s3:GetObjectVersionTagging",
|
||||
"s3:ListBucket",
|
||||
"s3:ListBucketVersions",
|
||||
"s3:GetBucketLocation",
|
||||
"s3:GetBucketVersioning",
|
||||
"s3:GetBucketAcl",
|
||||
"s3:GetBucketCors",
|
||||
"s3:GetBucketTagging",
|
||||
"s3:GetBucketNotification",
|
||||
}
|
||||
bucket, _ := extractBucketAndPrefix(resourcePattern)
|
||||
objectResources := buildObjectResourceArn(resourcePattern)
|
||||
// Include both bucket ARN (for ListBucket* and Get*Bucket operations) and object ARNs (for GetObject* operations)
|
||||
if bucket != "" {
|
||||
resources = append([]string{fmt.Sprintf("arn:aws:s3:::%s", bucket)}, objectResources...)
|
||||
} else {
|
||||
resources = objectResources
|
||||
}
|
||||
|
||||
case "Write":
|
||||
// Write includes object-level writes (PutObject, DeleteObject, PutObjectAcl, DeleteObjectVersion, DeleteObjectTagging, PutObjectTagging)
|
||||
// and bucket-level writes (PutBucketVersioning, PutBucketCors, DeleteBucketCors, PutBucketAcl, PutBucketTagging, DeleteBucketTagging, PutBucketNotification)
|
||||
// and multipart upload operations (AbortMultipartUpload, ListMultipartUploads, ListParts).
|
||||
// ListMultipartUploads and ListParts are included because they are part of the multipart upload workflow
|
||||
// and require Write permissions to be meaningful (no point listing uploads if you can't abort/complete them).
|
||||
s3Actions = []string{
|
||||
"s3:PutObject",
|
||||
"s3:PutObjectAcl",
|
||||
"s3:PutObjectTagging",
|
||||
"s3:DeleteObject",
|
||||
"s3:DeleteObjectVersion",
|
||||
"s3:DeleteObjectTagging",
|
||||
"s3:AbortMultipartUpload",
|
||||
"s3:ListMultipartUploads",
|
||||
"s3:ListParts",
|
||||
"s3:PutBucketAcl",
|
||||
"s3:PutBucketCors",
|
||||
"s3:PutBucketTagging",
|
||||
"s3:PutBucketNotification",
|
||||
"s3:PutBucketVersioning",
|
||||
"s3:DeleteBucketTagging",
|
||||
"s3:DeleteBucketCors",
|
||||
}
|
||||
bucket, _ := extractBucketAndPrefix(resourcePattern)
|
||||
objectResources := buildObjectResourceArn(resourcePattern)
|
||||
// Include bucket ARN so bucket-level write operations (e.g., PutBucketVersioning, PutBucketCors)
|
||||
// have the correct resource, while still allowing object-level writes.
|
||||
if bucket != "" {
|
||||
resources = append([]string{fmt.Sprintf("arn:aws:s3:::%s", bucket)}, objectResources...)
|
||||
} else {
|
||||
resources = objectResources
|
||||
}
|
||||
|
||||
case "Admin":
|
||||
s3Actions = []string{"s3:*"}
|
||||
bucket, prefix := extractBucketAndPrefix(resourcePattern)
|
||||
if bucket == "" {
|
||||
// Invalid pattern, return error
|
||||
return nil, fmt.Errorf("Admin action requires a valid bucket name")
|
||||
}
|
||||
if prefix != "" {
|
||||
// Subpath admin access: restrict to objects under this prefix
|
||||
resources = []string{
|
||||
fmt.Sprintf("arn:aws:s3:::%s", bucket),
|
||||
fmt.Sprintf("arn:aws:s3:::%s/%s/*", bucket, prefix),
|
||||
}
|
||||
} else {
|
||||
// Bucket-level admin access: full bucket permissions
|
||||
resources = []string{
|
||||
fmt.Sprintf("arn:aws:s3:::%s", bucket),
|
||||
fmt.Sprintf("arn:aws:s3:::%s/*", bucket),
|
||||
}
|
||||
}
|
||||
|
||||
case "List":
|
||||
// List includes bucket listing operations and also ListAllMyBuckets
|
||||
s3Actions = []string{"s3:ListBucket", "s3:ListBucketVersions", "s3:ListAllMyBuckets"}
|
||||
// ListBucket actions only require bucket ARN, not object-level ARNs
|
||||
bucket, _ := extractBucketAndPrefix(resourcePattern)
|
||||
if bucket != "" {
|
||||
resources = []string{fmt.Sprintf("arn:aws:s3:::%s", bucket)}
|
||||
} else {
|
||||
// Invalid pattern, return empty resources to fail validation
|
||||
resources = []string{}
|
||||
}
|
||||
|
||||
case "Tagging":
|
||||
// Tagging includes both object-level and bucket-level tagging operations
|
||||
s3Actions = []string{
|
||||
"s3:GetObjectTagging",
|
||||
"s3:PutObjectTagging",
|
||||
"s3:DeleteObjectTagging",
|
||||
"s3:GetBucketTagging",
|
||||
"s3:PutBucketTagging",
|
||||
"s3:DeleteBucketTagging",
|
||||
}
|
||||
bucket, _ := extractBucketAndPrefix(resourcePattern)
|
||||
objectResources := buildObjectResourceArn(resourcePattern)
|
||||
// Include bucket ARN so bucket-level tagging operations have the correct resource
|
||||
if bucket != "" {
|
||||
resources = append([]string{fmt.Sprintf("arn:aws:s3:::%s", bucket)}, objectResources...)
|
||||
} else {
|
||||
resources = objectResources
|
||||
}
|
||||
|
||||
case "BypassGovernanceRetention":
|
||||
s3Actions = []string{"s3:BypassGovernanceRetention"}
|
||||
resources = buildObjectResourceArn(resourcePattern)
|
||||
|
||||
case "GetObjectRetention":
|
||||
s3Actions = []string{"s3:GetObjectRetention"}
|
||||
resources = buildObjectResourceArn(resourcePattern)
|
||||
|
||||
case "PutObjectRetention":
|
||||
s3Actions = []string{"s3:PutObjectRetention"}
|
||||
resources = buildObjectResourceArn(resourcePattern)
|
||||
|
||||
case "GetObjectLegalHold":
|
||||
s3Actions = []string{"s3:GetObjectLegalHold"}
|
||||
resources = buildObjectResourceArn(resourcePattern)
|
||||
|
||||
case "PutObjectLegalHold":
|
||||
s3Actions = []string{"s3:PutObjectLegalHold"}
|
||||
resources = buildObjectResourceArn(resourcePattern)
|
||||
|
||||
case "GetBucketObjectLockConfiguration":
|
||||
s3Actions = []string{"s3:GetBucketObjectLockConfiguration"}
|
||||
bucket, _ := extractBucketAndPrefix(resourcePattern)
|
||||
if bucket != "" {
|
||||
resources = []string{fmt.Sprintf("arn:aws:s3:::%s", bucket)}
|
||||
} else {
|
||||
// Invalid pattern, return empty resources to fail validation
|
||||
resources = []string{}
|
||||
}
|
||||
|
||||
case "PutBucketObjectLockConfiguration":
|
||||
s3Actions = []string{"s3:PutBucketObjectLockConfiguration"}
|
||||
bucket, _ := extractBucketAndPrefix(resourcePattern)
|
||||
if bucket != "" {
|
||||
resources = []string{fmt.Sprintf("arn:aws:s3:::%s", bucket)}
|
||||
} else {
|
||||
// Invalid pattern, return empty resources to fail validation
|
||||
resources = []string{}
|
||||
}
|
||||
|
||||
default:
|
||||
return nil, fmt.Errorf("unknown action type: %s", actionType)
|
||||
}
|
||||
|
||||
return &PolicyStatement{
|
||||
Effect: PolicyEffectAllow,
|
||||
Action: NewStringOrStringSlice(s3Actions...),
|
||||
Resource: NewStringOrStringSlicePtr(resources...),
|
||||
}, nil
|
||||
}
|
||||
|
||||
// GetActionMappings returns the mapping of legacy actions to S3 actions
|
||||
func GetActionMappings() map[string][]string {
|
||||
return map[string][]string{
|
||||
"Read": {
|
||||
"s3:GetObject",
|
||||
"s3:GetObjectVersion",
|
||||
"s3:GetObjectAcl",
|
||||
"s3:GetObjectVersionAcl",
|
||||
"s3:GetObjectTagging",
|
||||
"s3:GetObjectVersionTagging",
|
||||
"s3:ListBucket",
|
||||
"s3:ListBucketVersions",
|
||||
"s3:GetBucketLocation",
|
||||
"s3:GetBucketVersioning",
|
||||
"s3:GetBucketAcl",
|
||||
"s3:GetBucketCors",
|
||||
"s3:GetBucketTagging",
|
||||
"s3:GetBucketNotification",
|
||||
},
|
||||
"Write": {
|
||||
"s3:PutObject",
|
||||
"s3:PutObjectAcl",
|
||||
"s3:PutObjectTagging",
|
||||
"s3:DeleteObject",
|
||||
"s3:DeleteObjectVersion",
|
||||
"s3:DeleteObjectTagging",
|
||||
"s3:AbortMultipartUpload",
|
||||
"s3:ListMultipartUploads",
|
||||
"s3:ListParts",
|
||||
"s3:PutBucketAcl",
|
||||
"s3:PutBucketCors",
|
||||
"s3:PutBucketTagging",
|
||||
"s3:PutBucketNotification",
|
||||
"s3:PutBucketVersioning",
|
||||
"s3:DeleteBucketTagging",
|
||||
"s3:DeleteBucketCors",
|
||||
},
|
||||
"Admin": {
|
||||
"s3:*",
|
||||
},
|
||||
"List": {
|
||||
"s3:ListBucket",
|
||||
"s3:ListBucketVersions",
|
||||
"s3:ListAllMyBuckets",
|
||||
},
|
||||
"Tagging": {
|
||||
"s3:GetObjectTagging",
|
||||
"s3:PutObjectTagging",
|
||||
"s3:DeleteObjectTagging",
|
||||
"s3:GetBucketTagging",
|
||||
"s3:PutBucketTagging",
|
||||
"s3:DeleteBucketTagging",
|
||||
},
|
||||
"BypassGovernanceRetention": {
|
||||
"s3:BypassGovernanceRetention",
|
||||
},
|
||||
"GetObjectRetention": {
|
||||
"s3:GetObjectRetention",
|
||||
},
|
||||
"PutObjectRetention": {
|
||||
"s3:PutObjectRetention",
|
||||
},
|
||||
"GetObjectLegalHold": {
|
||||
"s3:GetObjectLegalHold",
|
||||
},
|
||||
"PutObjectLegalHold": {
|
||||
"s3:PutObjectLegalHold",
|
||||
},
|
||||
"GetBucketObjectLockConfiguration": {
|
||||
"s3:GetBucketObjectLockConfiguration",
|
||||
},
|
||||
"PutBucketObjectLockConfiguration": {
|
||||
"s3:PutBucketObjectLockConfiguration",
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// ValidateActionMapping validates that a legacy action can be mapped to S3 actions
|
||||
func ValidateActionMapping(action string) error {
|
||||
mappings := GetActionMappings()
|
||||
|
||||
parts := strings.Split(action, ":")
|
||||
if len(parts) != 2 {
|
||||
return fmt.Errorf("invalid action format: %s, expected format: 'ActionType:Resource'", action)
|
||||
}
|
||||
|
||||
actionType := parts[0]
|
||||
resource := parts[1]
|
||||
|
||||
if _, exists := mappings[actionType]; !exists {
|
||||
return fmt.Errorf("unknown action type: %s", actionType)
|
||||
}
|
||||
|
||||
if resource == "" {
|
||||
return fmt.Errorf("resource cannot be empty")
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// ConvertLegacyActions converts an array of legacy actions to S3 actions
|
||||
func ConvertLegacyActions(legacyActions []string) ([]string, error) {
|
||||
mappings := GetActionMappings()
|
||||
s3Actions := make([]string, 0)
|
||||
|
||||
for _, legacyAction := range legacyActions {
|
||||
if err := ValidateActionMapping(legacyAction); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
parts := strings.Split(legacyAction, ":")
|
||||
actionType := parts[0]
|
||||
|
||||
if actionType == "Admin" {
|
||||
// Admin gives all permissions, so we can just return s3:*
|
||||
return []string{"s3:*"}, nil
|
||||
}
|
||||
|
||||
if mapped, exists := mappings[actionType]; exists {
|
||||
s3Actions = append(s3Actions, mapped...)
|
||||
}
|
||||
}
|
||||
|
||||
// Remove duplicates
|
||||
uniqueActions := make([]string, 0)
|
||||
seen := make(map[string]bool)
|
||||
for _, action := range s3Actions {
|
||||
if !seen[action] {
|
||||
uniqueActions = append(uniqueActions, action)
|
||||
seen[action] = true
|
||||
}
|
||||
}
|
||||
|
||||
return uniqueActions, nil
|
||||
}
|
||||
|
||||
// GetResourcesFromLegacyAction extracts resources from a legacy action.
|
||||
// It delegates to convertSingleAction to ensure consistent resource ARN generation
|
||||
// across the codebase and avoid duplicating action-type-specific logic.
|
||||
func GetResourcesFromLegacyAction(legacyAction string) ([]string, error) {
|
||||
stmt, err := convertSingleAction(legacyAction)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return stmt.Resource.Strings(), nil
|
||||
}
|
||||
|
||||
// CreatePolicyFromLegacyIdentity creates a policy document from legacy identity actions
|
||||
func CreatePolicyFromLegacyIdentity(identityName string, actions []string) (*PolicyDocument, error) {
|
||||
statements := make([]PolicyStatement, 0)
|
||||
|
||||
// Group actions by resource pattern
|
||||
resourceActions := make(map[string][]string)
|
||||
|
||||
for _, action := range actions {
|
||||
// Validate action format before processing
|
||||
if err := ValidateActionMapping(action); err != nil {
|
||||
glog.Warningf("Skipping invalid action %q for identity %q: %v", action, identityName, err)
|
||||
continue
|
||||
}
|
||||
|
||||
parts := strings.Split(action, ":")
|
||||
if len(parts) != 2 {
|
||||
continue
|
||||
}
|
||||
|
||||
resourcePattern := parts[1]
|
||||
actionType := parts[0]
|
||||
|
||||
if _, exists := resourceActions[resourcePattern]; !exists {
|
||||
resourceActions[resourcePattern] = make([]string, 0)
|
||||
}
|
||||
resourceActions[resourcePattern] = append(resourceActions[resourcePattern], actionType)
|
||||
}
|
||||
|
||||
// Create statements for each resource pattern
|
||||
for resourcePattern, actionTypes := range resourceActions {
|
||||
s3Actions := make([]string, 0)
|
||||
resourceSet := make(map[string]struct{})
|
||||
|
||||
// Collect S3 actions and aggregate resource ARNs from all action types.
|
||||
// Different action types have different resource ARN requirements:
|
||||
// - List: bucket-level ARNs only
|
||||
// - Read/Write/Tagging: object-level ARNs
|
||||
// - Admin: full bucket access
|
||||
// We must merge all required ARNs for the combined policy statement.
|
||||
for _, actionType := range actionTypes {
|
||||
if actionType == "Admin" {
|
||||
s3Actions = []string{"s3:*"}
|
||||
// Admin action determines the resources, so we can break after processing it.
|
||||
res, err := GetResourcesFromLegacyAction(fmt.Sprintf("Admin:%s", resourcePattern))
|
||||
if err != nil {
|
||||
glog.Warningf("Failed to get resources for Admin action on %s: %v", resourcePattern, err)
|
||||
resourceSet = nil // Invalidate to skip this statement
|
||||
break
|
||||
}
|
||||
for _, r := range res {
|
||||
resourceSet[r] = struct{}{}
|
||||
}
|
||||
break
|
||||
}
|
||||
|
||||
if mapped, exists := GetActionMappings()[actionType]; exists {
|
||||
s3Actions = append(s3Actions, mapped...)
|
||||
res, err := GetResourcesFromLegacyAction(fmt.Sprintf("%s:%s", actionType, resourcePattern))
|
||||
if err != nil {
|
||||
glog.Warningf("Failed to get resources for %s action on %s: %v", actionType, resourcePattern, err)
|
||||
resourceSet = nil // Invalidate to skip this statement
|
||||
break
|
||||
}
|
||||
for _, r := range res {
|
||||
resourceSet[r] = struct{}{}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if resourceSet == nil || len(s3Actions) == 0 {
|
||||
continue
|
||||
}
|
||||
|
||||
resources := make([]string, 0, len(resourceSet))
|
||||
for r := range resourceSet {
|
||||
resources = append(resources, r)
|
||||
}
|
||||
|
||||
statement := PolicyStatement{
|
||||
Sid: fmt.Sprintf("%s-%s", identityName, strings.ReplaceAll(resourcePattern, "/", "-")),
|
||||
Effect: PolicyEffectAllow,
|
||||
Action: NewStringOrStringSlice(s3Actions...),
|
||||
Resource: NewStringOrStringSlicePtr(resources...),
|
||||
}
|
||||
|
||||
statements = append(statements, statement)
|
||||
}
|
||||
|
||||
if len(statements) == 0 {
|
||||
return nil, fmt.Errorf("no valid statements generated for identity %s", identityName)
|
||||
}
|
||||
|
||||
return &PolicyDocument{
|
||||
Version: PolicyVersion2012_10_17,
|
||||
Statement: statements,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// HasPolicyForBucket checks if a bucket has a policy
|
||||
func (p *PolicyBackedIAM) HasPolicyForBucket(bucketName string) bool {
|
||||
return p.policyEngine.HasPolicyForBucket(bucketName)
|
||||
}
|
||||
|
||||
// GetPolicyEngine returns the underlying policy engine
|
||||
func (p *PolicyBackedIAM) GetPolicyEngine() *PolicyEngine {
|
||||
return p.policyEngine
|
||||
}
|
||||
@@ -1,373 +0,0 @@
|
||||
package policy_engine
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
// TestConvertSingleActionDeleteObject tests support for s3:DeleteObject action (Issue #7864)
|
||||
func TestConvertSingleActionDeleteObject(t *testing.T) {
|
||||
// Test that Write action includes DeleteObject S3 action
|
||||
stmt, err := convertSingleAction("Write:bucket")
|
||||
assert.NoError(t, err)
|
||||
assert.NotNil(t, stmt)
|
||||
|
||||
// Check that s3:DeleteObject is included in the actions
|
||||
actions := stmt.Action.Strings()
|
||||
assert.Contains(t, actions, "s3:DeleteObject", "Write action should include s3:DeleteObject")
|
||||
assert.Contains(t, actions, "s3:PutObject", "Write action should include s3:PutObject")
|
||||
}
|
||||
|
||||
// TestConvertSingleActionSubpath tests subpath handling for legacy actions (Issue #7864)
|
||||
func TestConvertSingleActionSubpath(t *testing.T) {
|
||||
testCases := []struct {
|
||||
name string
|
||||
action string
|
||||
expectedActions []string
|
||||
expectedResources []string
|
||||
description string
|
||||
}{
|
||||
{
|
||||
name: "Write_on_bucket",
|
||||
action: "Write:mybucket",
|
||||
expectedActions: []string{"s3:PutObject", "s3:DeleteObject", "s3:PutObjectAcl", "s3:DeleteObjectVersion", "s3:PutObjectTagging", "s3:DeleteObjectTagging", "s3:AbortMultipartUpload", "s3:ListMultipartUploads", "s3:ListParts", "s3:PutBucketAcl", "s3:PutBucketCors", "s3:PutBucketTagging", "s3:PutBucketNotification", "s3:PutBucketVersioning", "s3:DeleteBucketTagging", "s3:DeleteBucketCors"},
|
||||
expectedResources: []string{"arn:aws:s3:::mybucket", "arn:aws:s3:::mybucket/*"},
|
||||
description: "Write permission on bucket should include bucket and object ARNs",
|
||||
},
|
||||
{
|
||||
name: "Write_on_bucket_with_wildcard",
|
||||
action: "Write:mybucket/*",
|
||||
expectedActions: []string{"s3:PutObject", "s3:DeleteObject", "s3:PutObjectAcl", "s3:DeleteObjectVersion", "s3:PutObjectTagging", "s3:DeleteObjectTagging", "s3:AbortMultipartUpload", "s3:ListMultipartUploads", "s3:ListParts", "s3:PutBucketAcl", "s3:PutBucketCors", "s3:PutBucketTagging", "s3:PutBucketNotification", "s3:PutBucketVersioning", "s3:DeleteBucketTagging", "s3:DeleteBucketCors"},
|
||||
expectedResources: []string{"arn:aws:s3:::mybucket", "arn:aws:s3:::mybucket/*"},
|
||||
description: "Write permission with /* should include bucket and object ARNs",
|
||||
},
|
||||
{
|
||||
name: "Write_on_subpath",
|
||||
action: "Write:mybucket/sub_path/*",
|
||||
expectedActions: []string{"s3:PutObject", "s3:DeleteObject", "s3:PutObjectAcl", "s3:DeleteObjectVersion", "s3:PutObjectTagging", "s3:DeleteObjectTagging", "s3:AbortMultipartUpload", "s3:ListMultipartUploads", "s3:ListParts", "s3:PutBucketAcl", "s3:PutBucketCors", "s3:PutBucketTagging", "s3:PutBucketNotification", "s3:PutBucketVersioning", "s3:DeleteBucketTagging", "s3:DeleteBucketCors"},
|
||||
expectedResources: []string{"arn:aws:s3:::mybucket", "arn:aws:s3:::mybucket/sub_path/*"},
|
||||
description: "Write permission on subpath should include bucket and subpath objects ARNs",
|
||||
},
|
||||
{
|
||||
name: "Read_on_subpath",
|
||||
action: "Read:mybucket/documents/*",
|
||||
expectedActions: []string{"s3:GetObject", "s3:GetObjectVersion", "s3:ListBucket", "s3:ListBucketVersions", "s3:GetObjectAcl", "s3:GetObjectVersionAcl", "s3:GetObjectTagging", "s3:GetObjectVersionTagging", "s3:GetBucketLocation", "s3:GetBucketVersioning", "s3:GetBucketAcl", "s3:GetBucketCors", "s3:GetBucketTagging", "s3:GetBucketNotification"},
|
||||
expectedResources: []string{"arn:aws:s3:::mybucket", "arn:aws:s3:::mybucket/documents/*"},
|
||||
description: "Read permission on subpath should include bucket ARN and subpath objects",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
stmt, err := convertSingleAction(tc.action)
|
||||
assert.NoError(t, err, tc.description)
|
||||
assert.NotNil(t, stmt)
|
||||
|
||||
// Check actions
|
||||
actions := stmt.Action.Strings()
|
||||
for _, expectedAction := range tc.expectedActions {
|
||||
assert.Contains(t, actions, expectedAction,
|
||||
"Action %s should be included for %s", expectedAction, tc.action)
|
||||
}
|
||||
|
||||
// Check resources - verify all expected resources are present
|
||||
resources := stmt.Resource.Strings()
|
||||
assert.ElementsMatch(t, resources, tc.expectedResources,
|
||||
"Resources should match exactly for %s. Got %v, expected %v", tc.action, resources, tc.expectedResources)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestConvertSingleActionSubpathDeleteAllowed tests that DeleteObject works on subpaths
|
||||
func TestConvertSingleActionSubpathDeleteAllowed(t *testing.T) {
|
||||
// This test specifically addresses Issue #7864 part 1:
|
||||
// "when a user is granted permission to a subpath, eg s3.configure -user someuser
|
||||
// -actions Write -buckets some_bucket/sub_path/* -apply
|
||||
// the user will only be able to put, but not delete object under somebucket/sub_path"
|
||||
|
||||
stmt, err := convertSingleAction("Write:some_bucket/sub_path/*")
|
||||
assert.NoError(t, err)
|
||||
|
||||
// The fix: s3:DeleteObject should be in the allowed actions
|
||||
actions := stmt.Action.Strings()
|
||||
assert.Contains(t, actions, "s3:DeleteObject",
|
||||
"Write permission on subpath should allow deletion of objects in that path")
|
||||
|
||||
// The resource should be restricted to the subpath
|
||||
resources := stmt.Resource.Strings()
|
||||
assert.Contains(t, resources, "arn:aws:s3:::some_bucket/sub_path/*",
|
||||
"Delete permission should apply to objects under the subpath")
|
||||
}
|
||||
|
||||
// TestConvertSingleActionNestedPaths tests deeply nested paths
|
||||
func TestConvertSingleActionNestedPaths(t *testing.T) {
|
||||
testCases := []struct {
|
||||
action string
|
||||
expectedResources []string
|
||||
}{
|
||||
{
|
||||
action: "Write:bucket/a/b/c/*",
|
||||
expectedResources: []string{"arn:aws:s3:::bucket", "arn:aws:s3:::bucket/a/b/c/*"},
|
||||
},
|
||||
{
|
||||
action: "Read:bucket/data/documents/2024/*",
|
||||
expectedResources: []string{"arn:aws:s3:::bucket", "arn:aws:s3:::bucket/data/documents/2024/*"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
stmt, err := convertSingleAction(tc.action)
|
||||
assert.NoError(t, err)
|
||||
|
||||
resources := stmt.Resource.Strings()
|
||||
assert.ElementsMatch(t, resources, tc.expectedResources)
|
||||
}
|
||||
}
|
||||
|
||||
// TestGetResourcesFromLegacyAction tests that GetResourcesFromLegacyAction generates
|
||||
// action-appropriate resources consistent with convertSingleAction
|
||||
func TestGetResourcesFromLegacyAction(t *testing.T) {
|
||||
testCases := []struct {
|
||||
name string
|
||||
action string
|
||||
expectedResources []string
|
||||
description string
|
||||
}{
|
||||
// List actions - bucket-only (no object ARNs)
|
||||
{
|
||||
name: "List_on_bucket",
|
||||
action: "List:mybucket",
|
||||
expectedResources: []string{"arn:aws:s3:::mybucket"},
|
||||
description: "List action should only have bucket ARN",
|
||||
},
|
||||
{
|
||||
name: "List_on_bucket_with_wildcard",
|
||||
action: "List:mybucket/*",
|
||||
expectedResources: []string{"arn:aws:s3:::mybucket"},
|
||||
description: "List action should only have bucket ARN regardless of wildcard",
|
||||
},
|
||||
// Read actions - bucket and object-level ARNs (includes List* and Get* operations)
|
||||
{
|
||||
name: "Read_on_bucket",
|
||||
action: "Read:mybucket",
|
||||
expectedResources: []string{"arn:aws:s3:::mybucket", "arn:aws:s3:::mybucket/*"},
|
||||
description: "Read action should have both bucket and object ARNs",
|
||||
},
|
||||
{
|
||||
name: "Read_on_subpath",
|
||||
action: "Read:mybucket/documents/*",
|
||||
expectedResources: []string{"arn:aws:s3:::mybucket", "arn:aws:s3:::mybucket/documents/*"},
|
||||
description: "Read action on subpath should have bucket ARN and object ARN for subpath",
|
||||
},
|
||||
// Write actions - bucket and object ARNs (includes bucket-level operations)
|
||||
{
|
||||
name: "Write_on_subpath",
|
||||
action: "Write:mybucket/sub_path/*",
|
||||
expectedResources: []string{"arn:aws:s3:::mybucket", "arn:aws:s3:::mybucket/sub_path/*"},
|
||||
description: "Write action should have bucket and object ARNs",
|
||||
},
|
||||
// Admin actions - both bucket and object ARNs
|
||||
{
|
||||
name: "Admin_on_bucket",
|
||||
action: "Admin:mybucket",
|
||||
expectedResources: []string{"arn:aws:s3:::mybucket", "arn:aws:s3:::mybucket/*"},
|
||||
description: "Admin action should have both bucket and object ARNs",
|
||||
},
|
||||
{
|
||||
name: "Admin_on_subpath",
|
||||
action: "Admin:mybucket/admin/section/*",
|
||||
expectedResources: []string{"arn:aws:s3:::mybucket", "arn:aws:s3:::mybucket/admin/section/*"},
|
||||
description: "Admin action on subpath should restrict to subpath, preventing privilege escalation",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
resources, err := GetResourcesFromLegacyAction(tc.action)
|
||||
assert.NoError(t, err, tc.description)
|
||||
assert.ElementsMatch(t, resources, tc.expectedResources,
|
||||
"Resources should match expected. Got %v, expected %v", resources, tc.expectedResources)
|
||||
|
||||
// Also verify consistency with convertSingleAction where applicable
|
||||
stmt, err := convertSingleAction(tc.action)
|
||||
assert.NoError(t, err)
|
||||
|
||||
stmtResources := stmt.Resource.Strings()
|
||||
assert.ElementsMatch(t, resources, stmtResources,
|
||||
"GetResourcesFromLegacyAction should match convertSingleAction resources for %s", tc.action)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestExtractBucketAndPrefixEdgeCases validates edge case handling in extractBucketAndPrefix
|
||||
func TestExtractBucketAndPrefixEdgeCases(t *testing.T) {
|
||||
testCases := []struct {
|
||||
name string
|
||||
pattern string
|
||||
expectedBucket string
|
||||
expectedPrefix string
|
||||
description string
|
||||
}{
|
||||
{
|
||||
name: "Empty string",
|
||||
pattern: "",
|
||||
expectedBucket: "",
|
||||
expectedPrefix: "",
|
||||
description: "Empty pattern should return empty strings",
|
||||
},
|
||||
{
|
||||
name: "Whitespace only",
|
||||
pattern: " ",
|
||||
expectedBucket: "",
|
||||
expectedPrefix: "",
|
||||
description: "Whitespace-only pattern should return empty strings",
|
||||
},
|
||||
{
|
||||
name: "Slash only",
|
||||
pattern: "/",
|
||||
expectedBucket: "",
|
||||
expectedPrefix: "",
|
||||
description: "Slash-only pattern should return empty strings",
|
||||
},
|
||||
{
|
||||
name: "Double slash prefix",
|
||||
pattern: "bucket//prefix/*",
|
||||
expectedBucket: "bucket",
|
||||
expectedPrefix: "prefix",
|
||||
description: "Double slash should be normalized (trailing slashes removed)",
|
||||
},
|
||||
{
|
||||
name: "Normal bucket",
|
||||
pattern: "mybucket",
|
||||
expectedBucket: "mybucket",
|
||||
expectedPrefix: "",
|
||||
description: "Bucket-only pattern should work correctly",
|
||||
},
|
||||
{
|
||||
name: "Bucket with prefix",
|
||||
pattern: "mybucket/myprefix/*",
|
||||
expectedBucket: "mybucket",
|
||||
expectedPrefix: "myprefix",
|
||||
description: "Bucket with prefix should be parsed correctly",
|
||||
},
|
||||
{
|
||||
name: "Nested prefix",
|
||||
pattern: "mybucket/a/b/c/*",
|
||||
expectedBucket: "mybucket",
|
||||
expectedPrefix: "a/b/c",
|
||||
description: "Nested prefix should be preserved",
|
||||
},
|
||||
{
|
||||
name: "Bucket with trailing slash",
|
||||
pattern: "mybucket/",
|
||||
expectedBucket: "mybucket",
|
||||
expectedPrefix: "",
|
||||
description: "Trailing slash on bucket should be normalized",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
bucket, prefix := extractBucketAndPrefix(tc.pattern)
|
||||
assert.Equal(t, tc.expectedBucket, bucket, tc.description)
|
||||
assert.Equal(t, tc.expectedPrefix, prefix, tc.description)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestCreatePolicyFromLegacyIdentityMultipleActions validates correct resource ARN aggregation
|
||||
// when multiple action types target the same resource pattern
|
||||
func TestCreatePolicyFromLegacyIdentityMultipleActions(t *testing.T) {
|
||||
testCases := []struct {
|
||||
name string
|
||||
identityName string
|
||||
actions []string
|
||||
expectedStatements int
|
||||
expectedActionsInStmt1 []string
|
||||
expectedResourcesInStmt1 []string
|
||||
description string
|
||||
}{
|
||||
{
|
||||
name: "List_and_Write_on_subpath",
|
||||
identityName: "data-manager",
|
||||
actions: []string{"List:mybucket/data/*", "Write:mybucket/data/*"},
|
||||
expectedStatements: 1,
|
||||
expectedActionsInStmt1: []string{
|
||||
"s3:ListBucket", "s3:ListBucketVersions", "s3:ListAllMyBuckets",
|
||||
"s3:PutObject", "s3:DeleteObject", "s3:PutObjectAcl", "s3:DeleteObjectVersion",
|
||||
"s3:PutObjectTagging", "s3:DeleteObjectTagging", "s3:AbortMultipartUpload",
|
||||
"s3:ListMultipartUploads", "s3:ListParts", "s3:PutBucketAcl", "s3:PutBucketCors",
|
||||
"s3:PutBucketTagging", "s3:PutBucketNotification", "s3:PutBucketVersioning",
|
||||
"s3:DeleteBucketTagging", "s3:DeleteBucketCors",
|
||||
},
|
||||
expectedResourcesInStmt1: []string{
|
||||
"arn:aws:s3:::mybucket", // From List and Write actions
|
||||
"arn:aws:s3:::mybucket/data/*", // From Write action
|
||||
},
|
||||
description: "List + Write on same subpath should aggregate all actions and both bucket and object ARNs",
|
||||
},
|
||||
{
|
||||
name: "Read_and_Tagging_on_bucket",
|
||||
identityName: "tag-reader",
|
||||
actions: []string{"Read:mybucket", "Tagging:mybucket"},
|
||||
expectedStatements: 1,
|
||||
expectedActionsInStmt1: []string{
|
||||
"s3:GetObject", "s3:GetObjectVersion",
|
||||
"s3:ListBucket", "s3:ListBucketVersions",
|
||||
"s3:GetObjectAcl", "s3:GetObjectVersionAcl",
|
||||
"s3:GetObjectTagging", "s3:GetObjectVersionTagging",
|
||||
"s3:PutObjectTagging", "s3:DeleteObjectTagging",
|
||||
"s3:GetBucketLocation", "s3:GetBucketVersioning",
|
||||
"s3:GetBucketAcl", "s3:GetBucketCors", "s3:GetBucketTagging",
|
||||
"s3:GetBucketNotification", "s3:PutBucketTagging", "s3:DeleteBucketTagging",
|
||||
},
|
||||
expectedResourcesInStmt1: []string{
|
||||
"arn:aws:s3:::mybucket",
|
||||
"arn:aws:s3:::mybucket/*",
|
||||
},
|
||||
description: "Read + Tagging on same bucket should aggregate all bucket and object-level actions and ARNs",
|
||||
},
|
||||
{
|
||||
name: "Admin_with_other_actions",
|
||||
identityName: "admin-user",
|
||||
actions: []string{"Admin:mybucket/admin/*", "Write:mybucket/admin/*"},
|
||||
expectedStatements: 1,
|
||||
expectedActionsInStmt1: []string{"s3:*"},
|
||||
expectedResourcesInStmt1: []string{
|
||||
"arn:aws:s3:::mybucket",
|
||||
"arn:aws:s3:::mybucket/admin/*",
|
||||
},
|
||||
description: "Admin action should dominate and set s3:*, other actions still processed for resources",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
policy, err := CreatePolicyFromLegacyIdentity(tc.identityName, tc.actions)
|
||||
assert.NoError(t, err, tc.description)
|
||||
assert.NotNil(t, policy)
|
||||
|
||||
// Check statement count
|
||||
assert.Equal(t, tc.expectedStatements, len(policy.Statement),
|
||||
"Expected %d statement(s), got %d", tc.expectedStatements, len(policy.Statement))
|
||||
|
||||
if tc.expectedStatements > 0 {
|
||||
stmt := policy.Statement[0]
|
||||
|
||||
// Check actions
|
||||
actualActions := stmt.Action.Strings()
|
||||
for _, expectedAction := range tc.expectedActionsInStmt1 {
|
||||
assert.Contains(t, actualActions, expectedAction,
|
||||
"Action %s should be included in statement", expectedAction)
|
||||
}
|
||||
|
||||
// Check resources - all expected resources should be present
|
||||
actualResources := stmt.Resource.Strings()
|
||||
assert.ElementsMatch(t, tc.expectedResourcesInStmt1, actualResources,
|
||||
"Statement should aggregate all required resource ARNs. Got %v, expected %v",
|
||||
actualResources, tc.expectedResourcesInStmt1)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -490,11 +490,6 @@ func GetBucketFromResource(resource string) string {
|
||||
return ""
|
||||
}
|
||||
|
||||
// IsObjectResource checks if resource refers to objects
|
||||
func IsObjectResource(resource string) bool {
|
||||
return strings.Contains(resource, "/")
|
||||
}
|
||||
|
||||
// MatchesAction checks if an action matches any of the compiled action matchers.
|
||||
// It also implicitly grants multipart upload actions if s3:PutObject is allowed,
|
||||
// since multipart upload is an implementation detail of putting objects.
|
||||
|
||||
Reference in New Issue
Block a user