From 203ac8edaaaf82d18681fb5df7c893fab8a13657 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 27 Oct 2017 16:14:06 -0700 Subject: [PATCH] Bucket policies should use minio-go/pkg/policy instead. (#5090) --- cmd/api-errors.go | 2 + cmd/bucket-handlers.go | 18 +-- cmd/bucket-policy-handlers.go | 44 +++---- cmd/bucket-policy-handlers_test.go | 23 ++-- cmd/bucket-policy-parser.go | 118 ++++-------------- cmd/bucket-policy-parser_test.go | 194 +++++++++++++++-------------- cmd/bucket-policy.go | 53 ++++---- cmd/test-utils_test.go | 9 +- cmd/web-handlers.go | 6 +- cmd/web-handlers_test.go | 74 +++++------ 10 files changed, 248 insertions(+), 293 deletions(-) diff --git a/cmd/api-errors.go b/cmd/api-errors.go index 8540b6f88..9ab9a51b7 100644 --- a/cmd/api-errors.go +++ b/cmd/api-errors.go @@ -763,6 +763,8 @@ func toAPIErrorCode(err error) (apiErr APIErrorCode) { apiErr = ErrUnsupportedMetadata case PartsSizeUnequal: apiErr = ErrPartsSizeUnequal + case BucketPolicyNotFound: + apiErr = ErrNoSuchBucketPolicy default: apiErr = ErrInternalError } diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index a17988e4d..de596a112 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -24,10 +24,12 @@ import ( "net/http" "net/url" "path" + "reflect" "strings" "sync" mux "github.com/gorilla/mux" + "github.com/minio/minio-go/pkg/policy" "github.com/minio/minio-go/pkg/set" "github.com/minio/minio/pkg/hash" ) @@ -56,8 +58,8 @@ func enforceBucketPolicy(bucket, action, resource, referer, sourceIP string, que } // Fetch bucket policy, if policy is not set return access denied. - policy := globalBucketPolicies.GetBucketPolicy(bucket) - if policy == nil { + p := globalBucketPolicies.GetBucketPolicy(bucket) + if reflect.DeepEqual(p, emptyBucketPolicy) { return ErrAccessDenied } @@ -65,7 +67,7 @@ func enforceBucketPolicy(bucket, action, resource, referer, sourceIP string, que arn := bucketARNPrefix + strings.TrimSuffix(strings.TrimPrefix(resource, "/"), "/") // Get conditions for policy verification. - conditionKeyMap := make(map[string]set.StringSet) + conditionKeyMap := make(policy.ConditionKeyMap) for queryParam := range queryParams { conditionKeyMap[queryParam] = set.CreateStringSet(queryParams.Get(queryParam)) } @@ -78,7 +80,7 @@ func enforceBucketPolicy(bucket, action, resource, referer, sourceIP string, que conditionKeyMap["ip"] = set.CreateStringSet(sourceIP) // Validate action, resource and conditions with current policy statements. - if !bucketPolicyEvalStatements(action, arn, conditionKeyMap, policy.Statements) { + if !bucketPolicyEvalStatements(action, arn, conditionKeyMap, p.Statements) { return ErrAccessDenied } return ErrNone @@ -90,14 +92,14 @@ func isBucketActionAllowed(action, bucket, prefix string) bool { return false } - policy := globalBucketPolicies.GetBucketPolicy(bucket) - if policy == nil { + bp := globalBucketPolicies.GetBucketPolicy(bucket) + if reflect.DeepEqual(bp, emptyBucketPolicy) { return false } resource := bucketARNPrefix + path.Join(bucket, prefix) var conditionKeyMap map[string]set.StringSet // Validate action, resource and conditions with current policy statements. - return bucketPolicyEvalStatements(action, resource, conditionKeyMap, policy.Statements) + return bucketPolicyEvalStatements(action, resource, conditionKeyMap, bp.Statements) } // GetBucketLocationHandler - GET Bucket location. @@ -690,7 +692,7 @@ func (api objectAPIHandlers) DeleteBucketHandler(w http.ResponseWriter, r *http. _ = removeBucketPolicy(bucket, objectAPI) // Notify all peers (including self) to update in-memory state - S3PeersUpdateBucketPolicy(bucket, policyChange{true, nil}) + S3PeersUpdateBucketPolicy(bucket, policyChange{true, policy.BucketAccessPolicy{}}) // Delete notification config, if present - ignore any errors. _ = removeNotificationConfig(bucket, objectAPI) diff --git a/cmd/bucket-policy-handlers.go b/cmd/bucket-policy-handlers.go index 66593e55f..b731320c4 100644 --- a/cmd/bucket-policy-handlers.go +++ b/cmd/bucket-policy-handlers.go @@ -17,7 +17,7 @@ package cmd import ( - "fmt" + "encoding/json" "io" "io/ioutil" "net" @@ -27,7 +27,7 @@ import ( humanize "github.com/dustin/go-humanize" mux "github.com/gorilla/mux" - "github.com/minio/minio-go/pkg/set" + "github.com/minio/minio-go/pkg/policy" "github.com/minio/minio/pkg/wildcard" ) @@ -36,7 +36,8 @@ const maxAccessPolicySize = 20 * humanize.KiByte // Verify if a given action is valid for the url path based on the // existing bucket access policy. -func bucketPolicyEvalStatements(action string, resource string, conditions map[string]set.StringSet, statements []policyStatement) bool { +func bucketPolicyEvalStatements(action string, resource string, conditions policy.ConditionKeyMap, + statements []policy.Statement) bool { for _, statement := range statements { if bucketPolicyMatchStatement(action, resource, conditions, statement) { if statement.Effect == "Allow" { @@ -52,7 +53,8 @@ func bucketPolicyEvalStatements(action string, resource string, conditions map[s } // Verify if action, resource and conditions match input policy statement. -func bucketPolicyMatchStatement(action string, resource string, conditions map[string]set.StringSet, statement policyStatement) bool { +func bucketPolicyMatchStatement(action string, resource string, conditions policy.ConditionKeyMap, + statement policy.Statement) bool { // Verify if action, resource and condition match in given statement. return (bucketPolicyActionMatch(action, statement) && bucketPolicyResourceMatch(resource, statement) && @@ -60,7 +62,7 @@ func bucketPolicyMatchStatement(action string, resource string, conditions map[s } // Verify if given action matches with policy statement. -func bucketPolicyActionMatch(action string, statement policyStatement) bool { +func bucketPolicyActionMatch(action string, statement policy.Statement) bool { return !statement.Actions.FuncMatch(actionMatch, action).IsEmpty() } @@ -95,7 +97,7 @@ func isIPInCIDR(cidr, ip string) bool { } // Verify if given resource matches with policy statement. -func bucketPolicyResourceMatch(resource string, statement policyStatement) bool { +func bucketPolicyResourceMatch(resource string, statement policy.Statement) bool { // the resource rule for object could contain "*" wild card. // the requested object can be given access based on the already set bucket policy if // the match is successful. @@ -104,7 +106,7 @@ func bucketPolicyResourceMatch(resource string, statement policyStatement) bool } // Verify if given condition matches with policy statement. -func bucketPolicyConditionMatch(conditions map[string]set.StringSet, statement policyStatement) bool { +func bucketPolicyConditionMatch(conditions policy.ConditionKeyMap, statement policy.Statement) bool { // Supports following conditions. // - StringEquals // - StringNotEquals @@ -300,13 +302,11 @@ func (api objectAPIHandlers) DeleteBucketPolicyHandler(w http.ResponseWriter, r // Delete bucket access policy, by passing an empty policy // struct. - if err := persistAndNotifyBucketPolicyChange(bucket, policyChange{true, nil}, objAPI); err != nil { - switch err.(type) { - case BucketPolicyNotFound: - writeErrorResponse(w, ErrNoSuchBucketPolicy, r.URL) - default: - writeErrorResponse(w, ErrInternalError, r.URL) - } + err = persistAndNotifyBucketPolicyChange(bucket, policyChange{ + true, policy.BucketAccessPolicy{}, + }, objAPI) + if err != nil { + writeErrorResponse(w, toAPIErrorCode(err), r.URL) return } @@ -345,15 +345,17 @@ func (api objectAPIHandlers) GetBucketPolicyHandler(w http.ResponseWriter, r *ht policy, err := readBucketPolicy(bucket, objAPI) if err != nil { errorIf(err, "Unable to read bucket policy.") - switch err.(type) { - case BucketPolicyNotFound: - writeErrorResponse(w, ErrNoSuchBucketPolicy, r.URL) - default: - writeErrorResponse(w, ErrInternalError, r.URL) - } + writeErrorResponse(w, toAPIErrorCode(err), r.URL) + return + } + + policyBytes, err := json.Marshal(&policy) + if err != nil { + errorIf(err, "Unable to marshal bucket policy.") + writeErrorResponse(w, toAPIErrorCode(err), r.URL) return } // Write to client. - fmt.Fprint(w, policy) + w.Write(policyBytes) } diff --git a/cmd/bucket-policy-handlers_test.go b/cmd/bucket-policy-handlers_test.go index 6a607818d..bdcf14c65 100644 --- a/cmd/bucket-policy-handlers_test.go +++ b/cmd/bucket-policy-handlers_test.go @@ -25,6 +25,7 @@ import ( "net/http/httptest" "testing" + "github.com/minio/minio-go/pkg/policy" "github.com/minio/minio-go/pkg/set" ) @@ -32,8 +33,8 @@ import ( func TestBucketPolicyResourceMatch(t *testing.T) { // generates statement with given resource.. - generateStatement := func(resource string) policyStatement { - statement := policyStatement{} + generateStatement := func(resource string) policy.Statement { + statement := policy.Statement{} statement.Resources = set.CreateStringSet([]string{resource}...) return statement } @@ -45,7 +46,7 @@ func TestBucketPolicyResourceMatch(t *testing.T) { testCases := []struct { resourceToMatch string - statement policyStatement + statement policy.Statement expectedResourceMatch bool }{ // Test case 1-4. @@ -85,7 +86,7 @@ func TestBucketPolicyResourceMatch(t *testing.T) { } // TestBucketPolicyActionMatch - Test validates whether given action on the -// bucket/object matches the allowed actions in policyStatement. +// bucket/object matches the allowed actions in policy.Statement. // This test preserves the allowed actions for all 3 sets of policies, that is read-write,read-only, write-only. // The intention of the test is to catch any changes made to allowed action for on eof the above 3 major policy groups mentioned. func TestBucketPolicyActionMatch(t *testing.T) { @@ -94,7 +95,7 @@ func TestBucketPolicyActionMatch(t *testing.T) { testCases := []struct { action string - statement policyStatement + statement policy.Statement expectedResult bool }{ // s3:GetBucketLocation is the action necessary to be present in the bucket policy to allow @@ -843,28 +844,28 @@ func testDeleteBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName str // TestBucketPolicyConditionMatch - Tests to validate whether bucket policy conditions match. func TestBucketPolicyConditionMatch(t *testing.T) { - // obtain the inner map[string]set.StringSet for policyStatement.Conditions. + // obtain the inner map[string]set.StringSet for policy.Statement.Conditions. getInnerMap := func(key2, value string) map[string]set.StringSet { innerMap := make(map[string]set.StringSet) innerMap[key2] = set.CreateStringSet(value) return innerMap } - // obtain policyStatement with Conditions set. - getStatementWithCondition := func(key1, key2, value string) policyStatement { + // obtain policy.Statement with Conditions set. + getStatementWithCondition := func(key1, key2, value string) policy.Statement { innerMap := getInnerMap(key2, value) // to set policyStatment.Conditions . - conditions := make(map[string]map[string]set.StringSet) + conditions := make(policy.ConditionMap) conditions[key1] = innerMap // new policy statement. - statement := policyStatement{} + statement := policy.Statement{} // set the condition. statement.Conditions = conditions return statement } testCases := []struct { - statementCondition policyStatement + statementCondition policy.Statement condition map[string]set.StringSet expectedMatch bool diff --git a/cmd/bucket-policy-parser.go b/cmd/bucket-policy-parser.go index 084e9aeae..4ab53606e 100644 --- a/cmd/bucket-policy-parser.go +++ b/cmd/bucket-policy-parser.go @@ -26,9 +26,12 @@ import ( "sort" "strings" + "github.com/minio/minio-go/pkg/policy" "github.com/minio/minio-go/pkg/set" ) +var emptyBucketPolicy = policy.BucketAccessPolicy{} + var conditionKeyActionMap = map[string]set.StringSet{ "s3:prefix": set.CreateStringSet("s3:ListBucket"), "s3:max-keys": set.CreateStringSet("s3:ListBucket"), @@ -49,41 +52,16 @@ var supportedConditionsKey = set.CreateStringSet("s3:prefix", "s3:max-keys", "aw // supportedEffectMap - supported effects. var supportedEffectMap = set.CreateStringSet("Allow", "Deny") -// Statement - minio policy statement -type policyStatement struct { - Actions set.StringSet `json:"Action"` - Conditions map[string]map[string]set.StringSet `json:"Condition,omitempty"` - Effect string - Principal interface{} `json:"Principal"` - Resources set.StringSet `json:"Resource"` - Sid string -} - -// bucketPolicy - collection of various bucket policy statements. -type bucketPolicy struct { - Version string // date in YYYY-MM-DD format - Statements []policyStatement `json:"Statement"` -} - -// Stringer implementation for the bucket policies. -func (b bucketPolicy) String() string { - bbytes, err := json.Marshal(&b) - if err != nil { - errorIf(err, "Unable to marshal bucket policy into JSON %#v", b) - return "" - } - return string(bbytes) -} - // isValidActions - are actions valid. func isValidActions(actions set.StringSet) (err error) { // Statement actions cannot be empty. - if len(actions) == 0 { + if actions.IsEmpty() { err = errors.New("Action list cannot be empty") return err } if unsupportedActions := actions.Difference(supportedActionMap); !unsupportedActions.IsEmpty() { - err = fmt.Errorf("Unsupported actions found: ‘%#v’, please validate your policy document", unsupportedActions) + err = fmt.Errorf("Unsupported actions found: ‘%#v’, please validate your policy document", + unsupportedActions) return err } return nil @@ -106,7 +84,7 @@ func isValidEffect(effect string) (err error) { // isValidResources - are valid resources. func isValidResources(resources set.StringSet) (err error) { // Statement resources cannot be empty. - if len(resources) == 0 { + if resources.IsEmpty() { err = errors.New("Resource list cannot be empty") return err } @@ -124,60 +102,17 @@ func isValidResources(resources set.StringSet) (err error) { return nil } -// Parse principals parses a incoming json. Handles cases for -// these three combinations. -// - "Principal": "*", -// - "Principal": { "AWS" : "*" } -// - "Principal": { "AWS" : [ "*" ]} -func parsePrincipals(principal interface{}) set.StringSet { - principals, ok := principal.(map[string]interface{}) - if !ok { - var principalStr string - principalStr, ok = principal.(string) - if ok { - return set.CreateStringSet(principalStr) - } - } // else { - var principalStrs []string - for _, p := range principals { - principalStr, isStr := p.(string) - if !isStr { - principalsAdd, isInterface := p.([]interface{}) - if !isInterface { - principalStrsAddr, isStrs := p.([]string) - if !isStrs { - continue - } - principalStrs = append(principalStrs, principalStrsAddr...) - } else { - for _, pa := range principalsAdd { - var pstr string - pstr, isStr = pa.(string) - if !isStr { - continue - } - principalStrs = append(principalStrs, pstr) - } - } - continue - } // else { - principalStrs = append(principalStrs, principalStr) - } - return set.CreateStringSet(principalStrs...) -} - // isValidPrincipals - are valid principals. -func isValidPrincipals(principal interface{}) (err error) { - principals := parsePrincipals(principal) - // Statement principal should have a value. - if len(principals) == 0 { - err = errors.New("Principal cannot be empty") - return err +func isValidPrincipals(principal policy.User) (err error) { + if principal.AWS.IsEmpty() { + return errors.New("Principal cannot be empty") } - if unsuppPrincipals := principals.Difference(set.CreateStringSet([]string{"*"}...)); !unsuppPrincipals.IsEmpty() { + if diff := principal.AWS.Difference(set.CreateStringSet("*")); !diff.IsEmpty() { // Minio does not support or implement IAM, "*" is the only valid value. - // Amazon s3 doc on principals: http://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements.html#Principal - err = fmt.Errorf("Unsupported principals found: ‘%#v’, please validate your policy document", unsuppPrincipals) + // Amazon s3 doc on principal: + // http://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements.html#Principal + err = fmt.Errorf("Unsupported principals found: ‘%#v’, please validate your policy document", + diff) return err } return nil @@ -185,7 +120,7 @@ func isValidPrincipals(principal interface{}) (err error) { // isValidConditions - returns nil if the given conditions valid and // corresponding error otherwise. -func isValidConditions(actions set.StringSet, conditions map[string]map[string]set.StringSet) (err error) { +func isValidConditions(actions set.StringSet, conditions policy.ConditionMap) (err error) { // Verify conditions should be valid. Validate if only // supported condition keys are present and return error // otherwise. @@ -239,7 +174,7 @@ func resourcePrefix(resource string) string { // checkBucketPolicyResources validates Resources in unmarshalled bucket policy structure. // - Resources are validated against the given set of Actions. // - -func checkBucketPolicyResources(bucket string, bucketPolicy *bucketPolicy) APIErrorCode { +func checkBucketPolicyResources(bucket string, bucketPolicy policy.BucketAccessPolicy) APIErrorCode { // Validate statements for special actions and collect resources // for others to validate nesting. var resourceMap = set.NewStringSet() @@ -294,27 +229,27 @@ func checkBucketPolicyResources(bucket string, bucketPolicy *bucketPolicy) APIEr // parseBucketPolicy - parses and validates if bucket policy is of // proper JSON and follows allowed restrictions with policy standards. -func parseBucketPolicy(bucketPolicyReader io.Reader, policy *bucketPolicy) (err error) { +func parseBucketPolicy(bucketPolicyReader io.Reader, bktPolicy *policy.BucketAccessPolicy) (err error) { // Parse bucket policy reader. decoder := json.NewDecoder(bucketPolicyReader) - if err = decoder.Decode(&policy); err != nil { + if err = decoder.Decode(bktPolicy); err != nil { return err } // Policy version cannot be empty. - if len(policy.Version) == 0 { + if len(bktPolicy.Version) == 0 { err = errors.New("Policy version cannot be empty") return err } // Policy statements cannot be empty. - if len(policy.Statements) == 0 { + if len(bktPolicy.Statements) == 0 { err = errors.New("Policy statement cannot be empty") return err } // Loop through all policy statements and validate entries. - for _, statement := range policy.Statements { + for _, statement := range bktPolicy.Statements { // Statement effect should be valid. if err := isValidEffect(statement.Effect); err != nil { return err @@ -339,19 +274,20 @@ func parseBucketPolicy(bucketPolicyReader io.Reader, policy *bucketPolicy) (err // Separate deny and allow statements, so that we can apply deny // statements in the beginning followed by Allow statements. - var denyStatements []policyStatement - var allowStatements []policyStatement - for _, statement := range policy.Statements { + var denyStatements []policy.Statement + var allowStatements []policy.Statement + for _, statement := range bktPolicy.Statements { if statement.Effect == "Deny" { denyStatements = append(denyStatements, statement) continue } + // else if statement.Effect == "Allow" allowStatements = append(allowStatements, statement) } // Deny statements are enforced first once matched. - policy.Statements = append(denyStatements, allowStatements...) + bktPolicy.Statements = append(denyStatements, allowStatements...) // Return successfully parsed policy structure. return nil diff --git a/cmd/bucket-policy-parser_test.go b/cmd/bucket-policy-parser_test.go index d89479c64..13c3c3101 100644 --- a/cmd/bucket-policy-parser_test.go +++ b/cmd/bucket-policy-parser_test.go @@ -20,6 +20,7 @@ import ( "encoding/json" "errors" "fmt" + "reflect" "testing" "github.com/minio/minio-go/pkg/policy" @@ -74,11 +75,11 @@ var ( ) // Obtain bucket statement for read-write bucketPolicy. -func getReadWriteObjectStatement(bucketName, objectPrefix string) policyStatement { - objectResourceStatement := policyStatement{} +func getReadWriteObjectStatement(bucketName, objectPrefix string) policy.Statement { + objectResourceStatement := policy.Statement{} objectResourceStatement.Effect = "Allow" - objectResourceStatement.Principal = map[string]interface{}{ - "AWS": "*", + objectResourceStatement.Principal = policy.User{ + AWS: set.StringSet{"*": struct{}{}}, } objectResourceStatement.Resources = set.CreateStringSet([]string{fmt.Sprintf("%s%s", bucketARNPrefix, bucketName+"/"+objectPrefix+"*")}...) objectResourceStatement.Actions = set.CreateStringSet(readWriteObjectActions...) @@ -86,11 +87,11 @@ func getReadWriteObjectStatement(bucketName, objectPrefix string) policyStatemen } // Obtain object statement for read-write bucketPolicy. -func getReadWriteBucketStatement(bucketName, objectPrefix string) policyStatement { - bucketResourceStatement := policyStatement{} +func getReadWriteBucketStatement(bucketName, objectPrefix string) policy.Statement { + bucketResourceStatement := policy.Statement{} bucketResourceStatement.Effect = "Allow" - bucketResourceStatement.Principal = map[string]interface{}{ - "AWS": "*", + bucketResourceStatement.Principal = policy.User{ + AWS: set.StringSet{"*": struct{}{}}, } bucketResourceStatement.Resources = set.CreateStringSet([]string{fmt.Sprintf("%s%s", bucketARNPrefix, bucketName)}...) bucketResourceStatement.Actions = set.CreateStringSet(readWriteBucketActions...) @@ -98,19 +99,19 @@ func getReadWriteBucketStatement(bucketName, objectPrefix string) policyStatemen } // Obtain statements for read-write bucketPolicy. -func getReadWriteStatement(bucketName, objectPrefix string) []policyStatement { - statements := []policyStatement{} +func getReadWriteStatement(bucketName, objectPrefix string) []policy.Statement { + statements := []policy.Statement{} // Save the read write policy. statements = append(statements, getReadWriteBucketStatement(bucketName, objectPrefix), getReadWriteObjectStatement(bucketName, objectPrefix)) return statements } // Obtain bucket statement for read only bucketPolicy. -func getReadOnlyBucketStatement(bucketName, objectPrefix string) policyStatement { - bucketResourceStatement := policyStatement{} +func getReadOnlyBucketStatement(bucketName, objectPrefix string) policy.Statement { + bucketResourceStatement := policy.Statement{} bucketResourceStatement.Effect = "Allow" - bucketResourceStatement.Principal = map[string]interface{}{ - "AWS": "*", + bucketResourceStatement.Principal = policy.User{ + AWS: set.StringSet{"*": struct{}{}}, } bucketResourceStatement.Resources = set.CreateStringSet([]string{fmt.Sprintf("%s%s", bucketARNPrefix, bucketName)}...) bucketResourceStatement.Actions = set.CreateStringSet(readOnlyBucketActions...) @@ -118,11 +119,11 @@ func getReadOnlyBucketStatement(bucketName, objectPrefix string) policyStatement } // Obtain object statement for read only bucketPolicy. -func getReadOnlyObjectStatement(bucketName, objectPrefix string) policyStatement { - objectResourceStatement := policyStatement{} +func getReadOnlyObjectStatement(bucketName, objectPrefix string) policy.Statement { + objectResourceStatement := policy.Statement{} objectResourceStatement.Effect = "Allow" - objectResourceStatement.Principal = map[string]interface{}{ - "AWS": "*", + objectResourceStatement.Principal = policy.User{ + AWS: set.StringSet{"*": struct{}{}}, } objectResourceStatement.Resources = set.CreateStringSet([]string{fmt.Sprintf("%s%s", bucketARNPrefix, bucketName+"/"+objectPrefix+"*")}...) objectResourceStatement.Actions = set.CreateStringSet(readOnlyObjectActions...) @@ -130,20 +131,20 @@ func getReadOnlyObjectStatement(bucketName, objectPrefix string) policyStatement } // Obtain statements for read only bucketPolicy. -func getReadOnlyStatement(bucketName, objectPrefix string) []policyStatement { - statements := []policyStatement{} +func getReadOnlyStatement(bucketName, objectPrefix string) []policy.Statement { + statements := []policy.Statement{} // Save the read only policy. statements = append(statements, getReadOnlyBucketStatement(bucketName, objectPrefix), getReadOnlyObjectStatement(bucketName, objectPrefix)) return statements } // Obtain bucket statements for write only bucketPolicy. -func getWriteOnlyBucketStatement(bucketName, objectPrefix string) policyStatement { +func getWriteOnlyBucketStatement(bucketName, objectPrefix string) policy.Statement { - bucketResourceStatement := policyStatement{} + bucketResourceStatement := policy.Statement{} bucketResourceStatement.Effect = "Allow" - bucketResourceStatement.Principal = map[string]interface{}{ - "AWS": "*", + bucketResourceStatement.Principal = policy.User{ + AWS: set.StringSet{"*": struct{}{}}, } bucketResourceStatement.Resources = set.CreateStringSet([]string{fmt.Sprintf("%s%s", bucketARNPrefix, bucketName)}...) bucketResourceStatement.Actions = set.CreateStringSet(writeOnlyBucketActions...) @@ -151,11 +152,11 @@ func getWriteOnlyBucketStatement(bucketName, objectPrefix string) policyStatemen } // Obtain object statements for write only bucketPolicy. -func getWriteOnlyObjectStatement(bucketName, objectPrefix string) policyStatement { - objectResourceStatement := policyStatement{} +func getWriteOnlyObjectStatement(bucketName, objectPrefix string) policy.Statement { + objectResourceStatement := policy.Statement{} objectResourceStatement.Effect = "Allow" - objectResourceStatement.Principal = map[string]interface{}{ - "AWS": "*", + objectResourceStatement.Principal = policy.User{ + AWS: set.StringSet{"*": struct{}{}}, } objectResourceStatement.Resources = set.CreateStringSet([]string{fmt.Sprintf("%s%s", bucketARNPrefix, bucketName+"/"+objectPrefix+"*")}...) objectResourceStatement.Actions = set.CreateStringSet(writeOnlyObjectActions...) @@ -163,8 +164,8 @@ func getWriteOnlyObjectStatement(bucketName, objectPrefix string) policyStatemen } // Obtain statements for write only bucketPolicy. -func getWriteOnlyStatement(bucketName, objectPrefix string) []policyStatement { - statements := []policyStatement{} +func getWriteOnlyStatement(bucketName, objectPrefix string) []policy.Statement { + statements := []policy.Statement{} // Write only policy. // Save the write only policy. statements = append(statements, getWriteOnlyBucketStatement(bucketName, objectPrefix), getWriteOnlyBucketStatement(bucketName, objectPrefix)) @@ -325,9 +326,10 @@ func TestIsValidPrincipals(t *testing.T) { {[]string{"*"}, nil, true}, } for i, testCase := range testCases { - err := isValidPrincipals(map[string]interface{}{ - "AWS": testCase.principals, - }) + u := policy.User{ + AWS: set.CreateStringSet(testCase.principals...), + } + err := isValidPrincipals(u) if err != nil && testCase.shouldPass { t.Errorf("Test %d: Expected to pass, but failed with: %s", i+1, err.Error()) } @@ -343,86 +345,86 @@ func TestIsValidPrincipals(t *testing.T) { } } -// getEmptyConditionKeyMap - returns a function that generates a +// getEmptyConditionMap - returns a function that generates a // condition key map for a given key. -func getEmptyConditionKeyMap(conditionKey string) func() map[string]map[string]set.StringSet { - emptyConditonGenerator := func() map[string]map[string]set.StringSet { - emptyMap := make(map[string]set.StringSet) - conditions := make(map[string]map[string]set.StringSet) +func getEmptyConditionMap(conditionKey string) func() policy.ConditionMap { + emptyConditonGenerator := func() policy.ConditionMap { + emptyMap := make(policy.ConditionKeyMap) + conditions := make(policy.ConditionMap) conditions[conditionKey] = emptyMap return conditions } return emptyConditonGenerator } -// Tests validate policyStatement condition validator. +// Tests validate policy.Statement condition validator. func TestIsValidConditions(t *testing.T) { // returns empty conditions map. - setEmptyConditions := func() map[string]map[string]set.StringSet { - return make(map[string]map[string]set.StringSet) + setEmptyConditions := func() policy.ConditionMap { + return make(policy.ConditionMap) } // returns map with the "StringEquals" set to empty map. - setEmptyStringEquals := getEmptyConditionKeyMap("StringEquals") + setEmptyStringEquals := getEmptyConditionMap("StringEquals") // returns map with the "StringNotEquals" set to empty map. - setEmptyStringNotEquals := getEmptyConditionKeyMap("StringNotEquals") + setEmptyStringNotEquals := getEmptyConditionMap("StringNotEquals") // returns map with the "StringLike" set to empty map. - setEmptyStringLike := getEmptyConditionKeyMap("StringLike") + setEmptyStringLike := getEmptyConditionMap("StringLike") // returns map with the "StringNotLike" set to empty map. - setEmptyStringNotLike := getEmptyConditionKeyMap("StringNotLike") + setEmptyStringNotLike := getEmptyConditionMap("StringNotLike") // returns map with the "IpAddress" set to empty map. - setEmptyIPAddress := getEmptyConditionKeyMap("IpAddress") + setEmptyIPAddress := getEmptyConditionMap("IpAddress") // returns map with "NotIpAddress" set to empty map. - setEmptyNotIPAddress := getEmptyConditionKeyMap("NotIpAddress") + setEmptyNotIPAddress := getEmptyConditionMap("NotIpAddress") // Generate conditions. - generateConditions := func(key1, key2, value string) map[string]map[string]set.StringSet { - innerMap := make(map[string]set.StringSet) + generateConditions := func(key1, key2, value string) policy.ConditionMap { + innerMap := make(policy.ConditionKeyMap) innerMap[key2] = set.CreateStringSet(value) - conditions := make(map[string]map[string]set.StringSet) + conditions := make(policy.ConditionMap) conditions[key1] = innerMap return conditions } // generate ambigious conditions. - generateAmbigiousConditions := func() map[string]map[string]set.StringSet { - prefixMap := make(map[string]set.StringSet) + generateAmbigiousConditions := func() policy.ConditionMap { + prefixMap := make(policy.ConditionKeyMap) prefixMap["s3:prefix"] = set.CreateStringSet("Asia/") - conditions := make(map[string]map[string]set.StringSet) + conditions := make(policy.ConditionMap) conditions["StringEquals"] = prefixMap conditions["StringNotEquals"] = prefixMap return conditions } // generate valid and non valid type in the condition map. - generateValidInvalidConditions := func() map[string]map[string]set.StringSet { - innerMap := make(map[string]set.StringSet) + generateValidInvalidConditions := func() policy.ConditionMap { + innerMap := make(policy.ConditionKeyMap) innerMap["s3:prefix"] = set.CreateStringSet("Asia/") - conditions := make(map[string]map[string]set.StringSet) + conditions := make(policy.ConditionMap) conditions["StringEquals"] = innerMap conditions["InvalidType"] = innerMap return conditions } // generate valid and invalid keys for valid types in the same condition map. - generateValidInvalidConditionKeys := func() map[string]map[string]set.StringSet { - innerMapValid := make(map[string]set.StringSet) + generateValidInvalidConditionKeys := func() policy.ConditionMap { + innerMapValid := make(policy.ConditionKeyMap) innerMapValid["s3:prefix"] = set.CreateStringSet("Asia/") innerMapInValid := make(map[string]set.StringSet) innerMapInValid["s3:invalid"] = set.CreateStringSet("Asia/") - conditions := make(map[string]map[string]set.StringSet) + conditions := make(policy.ConditionMap) conditions["StringEquals"] = innerMapValid conditions["StringEquals"] = innerMapInValid return conditions } // List of Conditions used for test cases. - testConditions := []map[string]map[string]set.StringSet{ + testConditions := []policy.ConditionMap{ generateConditions("StringValues", "s3:max-keys", "100"), generateConditions("StringEquals", "s3:Object", "100"), generateAmbigiousConditions(), @@ -447,7 +449,7 @@ func TestIsValidConditions(t *testing.T) { "please validate your policy document", "s3:max-keys", getObjectActionSet) testCases := []struct { inputActions set.StringSet - inputCondition map[string]map[string]set.StringSet + inputCondition policy.ConditionMap // expected result. expectedErr error // flag indicating whether test should pass. @@ -518,26 +520,26 @@ func TestIsValidConditions(t *testing.T) { // Tests validate Policy Action and Resource fields. func TestCheckbucketPolicyResources(t *testing.T) { // constructing policy statement without invalidPrefixActions (check bucket-policy-parser.go). - setValidPrefixActions := func(statements []policyStatement) []policyStatement { + setValidPrefixActions := func(statements []policy.Statement) []policy.Statement { statements[0].Actions = set.CreateStringSet([]string{"s3:DeleteObject", "s3:PutObject"}...) return statements } // contracting policy statement with recursive resources. // should result in ErrMalformedPolicy - setRecurseResource := func(statements []policyStatement) []policyStatement { + setRecurseResource := func(statements []policy.Statement) []policy.Statement { statements[0].Resources = set.CreateStringSet([]string{"arn:aws:s3:::minio-bucket/Asia/*", "arn:aws:s3:::minio-bucket/Asia/India/*"}...) return statements } // constructing policy statement with lexically close characters. // should not result in ErrMalformedPolicy - setResourceLexical := func(statements []policyStatement) []policyStatement { + setResourceLexical := func(statements []policy.Statement) []policy.Statement { statements[0].Resources = set.CreateStringSet([]string{"arn:aws:s3:::minio-bucket/op*", "arn:aws:s3:::minio-bucket/oo*"}...) return statements } // List of bucketPolicy used for tests. - bucketAccessPolicies := []bucketPolicy{ + bucketAccessPolicies := []policy.BucketAccessPolicy{ // bucketPolicy - 1. // Contains valid read only policy statement. {Version: "1.0", Statements: getReadOnlyStatement("minio-bucket", "")}, @@ -568,7 +570,7 @@ func TestCheckbucketPolicyResources(t *testing.T) { } testCases := []struct { - inputPolicy bucketPolicy + inputPolicy policy.BucketAccessPolicy // expected results. apiErrCode APIErrorCode // Flag indicating whether the test should pass. @@ -599,7 +601,7 @@ func TestCheckbucketPolicyResources(t *testing.T) { {bucketAccessPolicies[6], ErrNone, true}, } for i, testCase := range testCases { - apiErrCode := checkBucketPolicyResources("minio-bucket", &testCase.inputPolicy) + apiErrCode := checkBucketPolicyResources("minio-bucket", testCase.inputPolicy) if apiErrCode != ErrNone && testCase.shouldPass { t.Errorf("Test %d: Expected to pass, but failed with Errocode %v", i+1, apiErrCode) } @@ -618,39 +620,39 @@ func TestCheckbucketPolicyResources(t *testing.T) { // Tests validate parsing of BucketAccessPolicy. func TestParseBucketPolicy(t *testing.T) { // set Unsupported Actions. - setUnsupportedActions := func(statements []policyStatement) []policyStatement { + setUnsupportedActions := func(statements []policy.Statement) []policy.Statement { // "s3:DeleteEverything"" is an Unsupported Action. statements[0].Actions = set.CreateStringSet([]string{"s3:GetObject", "s3:ListBucket", "s3:PutObject", "s3:DeleteEverything"}...) return statements } // set unsupported Effect. - setUnsupportedEffect := func(statements []policyStatement) []policyStatement { + setUnsupportedEffect := func(statements []policy.Statement) []policy.Statement { // Effect "Don't allow" is Unsupported. statements[0].Effect = "DontAllow" return statements } // set unsupported principals. - setUnsupportedPrincipals := func(statements []policyStatement) []policyStatement { + setUnsupportedPrincipals := func(statements []policy.Statement) []policy.Statement { // "User1111"" is an Unsupported Principal. - statements[0].Principal = map[string]interface{}{ - "AWS": []string{"*", "User1111"}, + statements[0].Principal = policy.User{ + AWS: set.CreateStringSet([]string{"*", "User1111"}...), } return statements } // set unsupported Resources. - setUnsupportedResources := func(statements []policyStatement) []policyStatement { + setUnsupportedResources := func(statements []policy.Statement) []policy.Statement { // "s3:DeleteEverything"" is an Unsupported Action. statements[0].Resources = set.CreateStringSet([]string{"my-resource"}...) return statements } // List of bucketPolicy used for test cases. - bucketAccesPolicies := []bucketPolicy{ + bucketAccesPolicies := []policy.BucketAccessPolicy{ // bucketPolicy - 0. // bucketPolicy statement empty. {Version: "1.0"}, // bucketPolicy - 1. // bucketPolicy version empty. - {Version: "", Statements: []policyStatement{}}, + {Version: "", Statements: []policy.Statement{}}, // bucketPolicy - 2. // Readonly bucketPolicy. {Version: "1.0", Statements: getReadOnlyStatement("minio-bucket", "")}, @@ -675,19 +677,19 @@ func TestParseBucketPolicy(t *testing.T) { } testCases := []struct { - inputPolicy bucketPolicy + inputPolicy policy.BucketAccessPolicy // expected results. - expectedPolicy bucketPolicy + expectedPolicy policy.BucketAccessPolicy err error // Flag indicating whether the test should pass. shouldPass bool }{ // Test case - 1. // bucketPolicy statement empty. - {bucketAccesPolicies[0], bucketPolicy{}, errors.New("Policy statement cannot be empty"), false}, + {bucketAccesPolicies[0], policy.BucketAccessPolicy{}, errors.New("Policy statement cannot be empty"), false}, // Test case - 2. // bucketPolicy version empty. - {bucketAccesPolicies[1], bucketPolicy{}, errors.New("Policy version cannot be empty"), false}, + {bucketAccesPolicies[1], policy.BucketAccessPolicy{}, errors.New("Policy version cannot be empty"), false}, // Test case - 3. // Readonly bucketPolicy. {bucketAccesPolicies[2], bucketAccesPolicies[2], nil, true}, @@ -718,8 +720,8 @@ func TestParseBucketPolicy(t *testing.T) { t.Fatalf("Test %d: Couldn't Marshal bucket policy %s", i+1, err) } - var actualAccessPolicy = &bucketPolicy{} - err = parseBucketPolicy(&buffer, actualAccessPolicy) + var actualAccessPolicy = policy.BucketAccessPolicy{} + err = parseBucketPolicy(&buffer, &actualAccessPolicy) if err != nil && testCase.shouldPass { t.Errorf("Test %d: Expected to pass, but failed with: %s", i+1, err.Error()) } @@ -734,7 +736,7 @@ func TestParseBucketPolicy(t *testing.T) { } // Test passes as expected, but the output values are verified for correctness here. if err == nil && testCase.shouldPass { - if testCase.expectedPolicy.String() != actualAccessPolicy.String() { + if !reflect.DeepEqual(testCase.expectedPolicy, actualAccessPolicy) { t.Errorf("Test %d: The expected statements from resource statement generator doesn't match the actual statements", i+1) } } @@ -751,8 +753,8 @@ func TestAWSRefererCondition(t *testing.T) { set.CreateStringSet("www.example.com", "http://www.example.com")) - requestConditionKeyMap := make(map[string]set.StringSet) - requestConditionKeyMap["referer"] = set.CreateStringSet("www.example.com") + requestConditionMap := make(policy.ConditionKeyMap) + requestConditionMap["referer"] = set.CreateStringSet("www.example.com") testCases := []struct { effect string @@ -782,20 +784,20 @@ func TestAWSRefererCondition(t *testing.T) { } for i, test := range testCases { - conditions := make(map[string]map[string]set.StringSet) + conditions := make(policy.ConditionMap) conditions[test.conditionKey] = conditionsKeyMap - allowStatement := policyStatement{ + allowStatement := policy.Statement{ Sid: "Testing AWS referer condition", Effect: test.effect, - Principal: map[string]interface{}{ - "AWS": "*", + Principal: policy.User{ + AWS: set.CreateStringSet("*"), }, Resources: resource, Conditions: conditions, } - if result := bucketPolicyConditionMatch(requestConditionKeyMap, allowStatement); result != test.match { + if result := bucketPolicyConditionMatch(requestConditionMap, allowStatement); result != test.match { t.Errorf("Test %d - Expected conditons to evaluate to %v but got %v", i+1, test.match, result) } @@ -813,8 +815,8 @@ func TestAWSSourceIPCondition(t *testing.T) { set.CreateStringSet("54.240.143.0/24", "2001:DB8:1234:5678::/64")) - requestConditionKeyMap := make(map[string]set.StringSet) - requestConditionKeyMap["ip"] = set.CreateStringSet("54.240.143.2") + requestConditionMap := make(policy.ConditionKeyMap) + requestConditionMap["ip"] = set.CreateStringSet("54.240.143.2") testCases := []struct { effect string @@ -844,20 +846,20 @@ func TestAWSSourceIPCondition(t *testing.T) { } for i, test := range testCases { - conditions := make(map[string]map[string]set.StringSet) + conditions := make(policy.ConditionMap) conditions[test.conditionKey] = conditionsKeyMap - allowStatement := policyStatement{ + allowStatement := policy.Statement{ Sid: "Testing AWS referer condition", Effect: test.effect, - Principal: map[string]interface{}{ - "AWS": "*", + Principal: policy.User{ + AWS: set.CreateStringSet("*"), }, Resources: resource, Conditions: conditions, } - if result := bucketPolicyConditionMatch(requestConditionKeyMap, allowStatement); result != test.match { + if result := bucketPolicyConditionMatch(requestConditionMap, allowStatement); result != test.match { t.Errorf("Test %d - Expected conditons to evaluate to %v but got %v", i+1, test.match, result) } diff --git a/cmd/bucket-policy.go b/cmd/bucket-policy.go index 655326b02..241988f44 100644 --- a/cmd/bucket-policy.go +++ b/cmd/bucket-policy.go @@ -20,8 +20,10 @@ import ( "bytes" "encoding/json" "io" + "reflect" "sync" + "github.com/minio/minio-go/pkg/policy" "github.com/minio/minio/pkg/hash" ) @@ -43,7 +45,7 @@ type bucketPolicies struct { rwMutex *sync.RWMutex // Collection of 'bucket' policies. - bucketPolicyConfigs map[string]*bucketPolicy + bucketPolicyConfigs map[string]policy.BucketAccessPolicy } // Represent a policy change @@ -53,11 +55,11 @@ type policyChange struct { IsRemove bool // represents the new policy for the bucket - BktPolicy *bucketPolicy + BktPolicy policy.BucketAccessPolicy } // Fetch bucket policy for a given bucket. -func (bp bucketPolicies) GetBucketPolicy(bucket string) *bucketPolicy { +func (bp bucketPolicies) GetBucketPolicy(bucket string) policy.BucketAccessPolicy { bp.rwMutex.RLock() defer bp.rwMutex.RUnlock() return bp.bucketPolicyConfigs[bucket] @@ -72,7 +74,7 @@ func (bp *bucketPolicies) SetBucketPolicy(bucket string, pCh policyChange) error if pCh.IsRemove { delete(bp.bucketPolicyConfigs, bucket) } else { - if pCh.BktPolicy == nil { + if reflect.DeepEqual(pCh.BktPolicy, emptyBucketPolicy) { return errInvalidArgument } bp.bucketPolicyConfigs[bucket] = pCh.BktPolicy @@ -81,19 +83,19 @@ func (bp *bucketPolicies) SetBucketPolicy(bucket string, pCh policyChange) error } // Loads all bucket policies from persistent layer. -func loadAllBucketPolicies(objAPI ObjectLayer) (policies map[string]*bucketPolicy, err error) { +func loadAllBucketPolicies(objAPI ObjectLayer) (policies map[string]policy.BucketAccessPolicy, err error) { // List buckets to proceed loading all notification configuration. buckets, err := objAPI.ListBuckets() - errorIf(err, "Unable to list buckets.") if err != nil { + errorIf(err, "Unable to list buckets.") return nil, errorCause(err) } - policies = make(map[string]*bucketPolicy) + policies = make(map[string]policy.BucketAccessPolicy) var pErrs []error // Loads bucket policy. for _, bucket := range buckets { - policy, pErr := readBucketPolicy(bucket.Name, objAPI) + bp, pErr := readBucketPolicy(bucket.Name, objAPI) if pErr != nil { // net.Dial fails for rpc client or any // other unexpected errors during net.Dial. @@ -105,7 +107,7 @@ func loadAllBucketPolicies(objAPI ObjectLayer) (policies map[string]*bucketPolic // Continue to load other bucket policies if possible. continue } - policies[bucket.Name] = policy + policies[bucket.Name] = bp } // Look for any errors occurred while reading bucket policies. @@ -168,21 +170,20 @@ func readBucketPolicyJSON(bucket string, objAPI ObjectLayer) (bucketPolicyReader // readBucketPolicy - reads bucket policy for an input bucket, returns BucketPolicyNotFound // if bucket policy is not found. This function also parses the bucket policy into an object. -func readBucketPolicy(bucket string, objAPI ObjectLayer) (*bucketPolicy, error) { +func readBucketPolicy(bucket string, objAPI ObjectLayer) (policy.BucketAccessPolicy, error) { // Read bucket policy JSON. bucketPolicyReader, err := readBucketPolicyJSON(bucket, objAPI) if err != nil { - return nil, err + return emptyBucketPolicy, err } // Parse the saved policy. - var policy = &bucketPolicy{} - err = parseBucketPolicy(bucketPolicyReader, policy) - if err != nil { - return nil, err + var bp policy.BucketAccessPolicy + if err = parseBucketPolicy(bucketPolicyReader, &bp); err != nil { + return emptyBucketPolicy, err } - return policy, nil + return bp, nil } // removeBucketPolicy - removes any previously written bucket policy. Returns BucketPolicyNotFound @@ -195,7 +196,8 @@ func removeBucketPolicy(bucket string, objAPI ObjectLayer) error { return err } defer objLock.Unlock() - if err := objAPI.DeleteObject(minioMetaBucket, policyPath); err != nil { + err := objAPI.DeleteObject(minioMetaBucket, policyPath) + if err != nil { errorIf(err, "Unable to remove bucket-policy on bucket %s.", bucket) err = errorCause(err) if _, ok := err.(ObjectNotFound); ok { @@ -207,10 +209,10 @@ func removeBucketPolicy(bucket string, objAPI ObjectLayer) error { } // writeBucketPolicy - save a bucket policy that is assumed to be validated. -func writeBucketPolicy(bucket string, objAPI ObjectLayer, bpy *bucketPolicy) error { +func writeBucketPolicy(bucket string, objAPI ObjectLayer, bpy policy.BucketAccessPolicy) error { buf, err := json.Marshal(bpy) if err != nil { - errorIf(err, "Unable to marshal bucket policy '%v' to JSON", *bpy) + errorIf(err, "Unable to marshal bucket policy '%#v' to JSON", bpy) return err } policyPath := pathJoin(bucketConfigPrefix, bucket, bucketPolicyConfig) @@ -236,15 +238,15 @@ func writeBucketPolicy(bucket string, objAPI ObjectLayer, bpy *bucketPolicy) err func parseAndPersistBucketPolicy(bucket string, policyBytes []byte, objAPI ObjectLayer) APIErrorCode { // Parse bucket policy. - var policy = &bucketPolicy{} - err := parseBucketPolicy(bytes.NewReader(policyBytes), policy) + var bktPolicy policy.BucketAccessPolicy + err := parseBucketPolicy(bytes.NewReader(policyBytes), &bktPolicy) if err != nil { errorIf(err, "Unable to parse bucket policy.") return ErrInvalidPolicyDocument } // Parse check bucket policy. - if s3Error := checkBucketPolicyResources(bucket, policy); s3Error != ErrNone { + if s3Error := checkBucketPolicyResources(bucket, bktPolicy); s3Error != ErrNone { return s3Error } @@ -257,7 +259,7 @@ func parseAndPersistBucketPolicy(bucket string, policyBytes []byte, objAPI Objec defer bucketLock.Unlock() // Save bucket policy. - if err = persistAndNotifyBucketPolicyChange(bucket, policyChange{false, policy}, objAPI); err != nil { + if err = persistAndNotifyBucketPolicyChange(bucket, policyChange{false, bktPolicy}, objAPI); err != nil { switch err.(type) { case BucketNameInvalid: return ErrInvalidBucketName @@ -276,11 +278,12 @@ func parseAndPersistBucketPolicy(bucket string, policyBytes []byte, objAPI Objec // change. In-memory state is updated in response to the notification. func persistAndNotifyBucketPolicyChange(bucket string, pCh policyChange, objAPI ObjectLayer) error { if pCh.IsRemove { - if err := removeBucketPolicy(bucket, objAPI); err != nil { + err := removeBucketPolicy(bucket, objAPI) + if err != nil { return err } } else { - if pCh.BktPolicy == nil { + if reflect.DeepEqual(pCh.BktPolicy, emptyBucketPolicy) { return errInvalidArgument } if err := writeBucketPolicy(bucket, objAPI, pCh.BktPolicy); err != nil { diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 9e4932dab..79b40e987 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -52,6 +52,7 @@ import ( "github.com/fatih/color" router "github.com/gorilla/mux" + "github.com/minio/minio-go/pkg/policy" "github.com/minio/minio-go/pkg/s3signer" "github.com/minio/minio/pkg/hash" ) @@ -1761,7 +1762,7 @@ func prepareTestBackend(instanceType string) (ObjectLayer, []string, error) { // STEP 2: Set the policy to allow the unsigned request, use the policyFunc to obtain the relevant statement and call // the handler again to verify its success. func ExecObjectLayerAPIAnonTest(t *testing.T, testName, bucketName, objectName, instanceType string, apiRouter http.Handler, - anonReq *http.Request, policyFunc func(string, string) policyStatement) { + anonReq *http.Request, policyFunc func(string, string) policy.Statement) { anonTestStr := "Anonymous HTTP request test" unknownSignTestStr := "Unknown HTTP signature test" @@ -1814,12 +1815,12 @@ func ExecObjectLayerAPIAnonTest(t *testing.T, testName, bucketName, objectName, } // Set write only policy on bucket to allow anonymous HTTP request for the operation under test. // request to go through. - policy := bucketPolicy{ + bp := policy.BucketAccessPolicy{ Version: "1.0", - Statements: []policyStatement{policyFunc(bucketName, "")}, + Statements: []policy.Statement{policyFunc(bucketName, "")}, } - globalBucketPolicies.SetBucketPolicy(bucketName, policyChange{false, &policy}) + globalBucketPolicies.SetBucketPolicy(bucketName, policyChange{false, bp}) // now call the handler again with the unsigned/anonymous request, it should be accepted. rec = httptest.NewRecorder() diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index c4e7d987b..9e888e4df 100644 --- a/cmd/web-handlers.go +++ b/cmd/web-handlers.go @@ -857,12 +857,14 @@ func (web *webAPIHandlers) SetBucketPolicy(r *http.Request, args *SetBucketPolic } if len(policyInfo.Statements) == 0 { - err = persistAndNotifyBucketPolicyChange(args.BucketName, policyChange{true, nil}, objectAPI) - if err != nil { + if err = persistAndNotifyBucketPolicyChange(args.BucketName, policyChange{ + true, policy.BucketAccessPolicy{}, + }, objectAPI); err != nil { return toJSONError(err, args.BucketName) } return nil } + data, err := json.Marshal(policyInfo) if err != nil { return toJSONError(err) diff --git a/cmd/web-handlers_test.go b/cmd/web-handlers_test.go index 02b58e335..7c6b4896c 100644 --- a/cmd/web-handlers_test.go +++ b/cmd/web-handlers_test.go @@ -433,12 +433,12 @@ func testListObjectsWebHandler(obj ObjectLayer, instanceType string, t TestErrHa t.Fatalf("Expected error `%s`", err) } - policy := bucketPolicy{ + policy := policy.BucketAccessPolicy{ Version: "1.0", - Statements: []policyStatement{getReadOnlyObjectStatement(bucketName, "")}, + Statements: []policy.Statement{getReadOnlyObjectStatement(bucketName, "")}, } - globalBucketPolicies.SetBucketPolicy(bucketName, policyChange{false, &policy}) + globalBucketPolicies.SetBucketPolicy(bucketName, policyChange{false, policy}) // Unauthenticated ListObjects with READ bucket policy should succeed. err, reply = test("") @@ -807,12 +807,12 @@ func testUploadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandler t.Fatalf("Expected the response status to be 403, but instead found `%d`", code) } - policy := bucketPolicy{ + bp := policy.BucketAccessPolicy{ Version: "1.0", - Statements: []policyStatement{getWriteOnlyObjectStatement(bucketName, "")}, + Statements: []policy.Statement{getWriteOnlyObjectStatement(bucketName, "")}, } - globalBucketPolicies.SetBucketPolicy(bucketName, policyChange{false, &policy}) + globalBucketPolicies.SetBucketPolicy(bucketName, policyChange{false, bp}) // Unauthenticated upload with WRITE policy should succeed. code = test("", true) @@ -914,12 +914,12 @@ func testDownloadWebHandler(obj ObjectLayer, instanceType string, t TestErrHandl t.Fatalf("Expected the response status to be 403, but instead found `%d`", code) } - policy := bucketPolicy{ + bp := policy.BucketAccessPolicy{ Version: "1.0", - Statements: []policyStatement{getReadOnlyObjectStatement(bucketName, "")}, + Statements: []policy.Statement{getReadOnlyObjectStatement(bucketName, "")}, } - globalBucketPolicies.SetBucketPolicy(bucketName, policyChange{false, &policy}) + globalBucketPolicies.SetBucketPolicy(bucketName, policyChange{false, bp}) // Unauthenticated download with READ policy should succeed. code, bodyContent = test("") @@ -1142,26 +1142,30 @@ func testWebGetBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestE t.Fatal("Unexpected error: ", err) } - policyVal := bucketPolicy{ + policyVal := policy.BucketAccessPolicy{ Version: "2012-10-17", - Statements: []policyStatement{ + Statements: []policy.Statement{ { - Actions: set.CreateStringSet("s3:GetBucketLocation", "s3:ListBucket"), - Effect: "Allow", - Principal: map[string][]string{"AWS": {"*"}}, + Actions: set.CreateStringSet("s3:GetBucketLocation", "s3:ListBucket"), + Effect: "Allow", + Principal: policy.User{ + AWS: set.CreateStringSet("*"), + }, Resources: set.CreateStringSet(bucketARNPrefix + bucketName), Sid: "", }, { - Actions: set.CreateStringSet("s3:GetObject"), - Effect: "Allow", - Principal: map[string][]string{"AWS": {"*"}}, + Actions: set.CreateStringSet("s3:GetObject"), + Effect: "Allow", + Principal: policy.User{ + AWS: set.CreateStringSet("*"), + }, Resources: set.CreateStringSet(bucketARNPrefix + bucketName + "/*"), Sid: "", }, }, } - if err := writeBucketPolicy(bucketName, obj, &policyVal); err != nil { + if err := writeBucketPolicy(bucketName, obj, policyVal); err != nil { t.Fatal("Unexpected error: ", err) } @@ -1216,32 +1220,32 @@ func testWebListAllBucketPoliciesHandler(obj ObjectLayer, instanceType string, t t.Fatal("Unexpected error: ", err) } - policyVal := bucketPolicy{ + stringEqualsConditions := policy.ConditionMap{} + stringEqualsConditions["StringEquals"] = make(policy.ConditionKeyMap) + stringEqualsConditions["StringEquals"].Add("s3:prefix", set.CreateStringSet("hello")) + + policyVal := policy.BucketAccessPolicy{ Version: "2012-10-17", - Statements: []policyStatement{ + Statements: []policy.Statement{ { Actions: set.CreateStringSet("s3:GetBucketLocation"), Effect: "Allow", - Principal: map[string][]string{"AWS": {"*"}}, + Principal: policy.User{AWS: set.CreateStringSet("*")}, Resources: set.CreateStringSet(bucketARNPrefix + bucketName), Sid: "", }, { - Actions: set.CreateStringSet("s3:ListBucket"), - Conditions: map[string]map[string]set.StringSet{ - "StringEquals": { - "s3:prefix": set.CreateStringSet("hello"), - }, - }, - Effect: "Allow", - Principal: map[string][]string{"AWS": {"*"}}, - Resources: set.CreateStringSet(bucketARNPrefix + bucketName), - Sid: "", + Actions: set.CreateStringSet("s3:ListBucket"), + Conditions: stringEqualsConditions, + Effect: "Allow", + Principal: policy.User{AWS: set.CreateStringSet("*")}, + Resources: set.CreateStringSet(bucketARNPrefix + bucketName), + Sid: "", }, { Actions: set.CreateStringSet("s3:ListBucketMultipartUploads"), Effect: "Allow", - Principal: map[string][]string{"AWS": {"*"}}, + Principal: policy.User{AWS: set.CreateStringSet("*")}, Resources: set.CreateStringSet(bucketARNPrefix + bucketName), Sid: "", }, @@ -1249,13 +1253,13 @@ func testWebListAllBucketPoliciesHandler(obj ObjectLayer, instanceType string, t Actions: set.CreateStringSet("s3:AbortMultipartUpload", "s3:DeleteObject", "s3:GetObject", "s3:ListMultipartUploadParts", "s3:PutObject"), Effect: "Allow", - Principal: map[string][]string{"AWS": {"*"}}, + Principal: policy.User{AWS: set.CreateStringSet("*")}, Resources: set.CreateStringSet(bucketARNPrefix + bucketName + "/hello*"), Sid: "", }, }, } - if err := writeBucketPolicy(bucketName, obj, &policyVal); err != nil { + if err := writeBucketPolicy(bucketName, obj, policyVal); err != nil { t.Fatal("Unexpected error: ", err) } @@ -1348,7 +1352,7 @@ func testWebSetBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestE // Parse RPC response err = getTestWebRPCResponse(rec, &reply) if testCase.pass && err != nil { - t.Fatalf("Test %d: Should succeed but it didn't, %v", i+1, err) + t.Fatalf("Test %d: Should succeed but it didn't, %#v", i+1, err) } if !testCase.pass && err == nil { t.Fatalf("Test %d: Should fail it didn't", i+1)