Fix handling of StringNotEquals condition operator (#3660)

cmd/bucket-policy-handlers.go

@@ -98,22 +98,32 @@ func bucketPolicyConditionMatch(conditions map[string]set.StringSet, statement p
 	// - s3:aws-Referer
 
 	for condition, conditionKeyVal := range statement.Conditions {
+		prefixConditon := conditionKeyVal["s3:prefix"]
+		maxKeyCondition := conditionKeyVal["s3:max-keys"]
 		if condition == "StringEquals" {
-			if !conditionKeyVal["s3:prefix"].Equals(conditions["prefix"]) {
+			// If there is no condition with "s3:prefix" or "s3:max-keys" condition key
+			// then there is nothing to check condition against.
+			if !prefixConditon.IsEmpty() && !prefixConditon.Equals(conditions["prefix"]) {
 				return false
 			}
-			if !conditionKeyVal["s3:max-keys"].Equals(conditions["max-keys"]) {
+			if !maxKeyCondition.IsEmpty() && !maxKeyCondition.Equals(conditions["max-keys"]) {
 				return false
 			}
 		} else if condition == "StringNotEquals" {
-			if !conditionKeyVal["s3:prefix"].Equals(conditions["prefix"]) {
+			// If there is no condition with "s3:prefix" or "s3:max-keys" condition key
+			// then there is nothing to check condition against.
+			if !prefixConditon.IsEmpty() && prefixConditon.Equals(conditions["prefix"]) {
 				return false
 			}
-			if !conditionKeyVal["s3:max-keys"].Equals(conditions["max-keys"]) {
+			if !maxKeyCondition.IsEmpty() && maxKeyCondition.Equals(conditions["max-keys"]) {
 				return false
 			}
 		} else if condition == "StringLike" {
 			awsReferers := conditionKeyVal["aws:Referer"]
+			// Skip empty condition, it is trivially satisfied.
+			if awsReferers.IsEmpty() {
+				continue
+			}
 			// wildcard match of referer in statement was not empty.
 			// StringLike has a match, i.e, condition evaluates to true.
 			for referer := range conditions["referer"] {
@@ -125,6 +135,10 @@ func bucketPolicyConditionMatch(conditions map[string]set.StringSet, statement p
 			return false
 		} else if condition == "StringNotLike" {
 			awsReferers := conditionKeyVal["aws:Referer"]
+			// Skip empty condition, it is trivially satisfied.
+			if awsReferers.IsEmpty() {
+				continue
+			}
 			// wildcard match of referer in statement was not empty.
 			// StringNotLike has a match, i.e, condition evaluates to false.
 			for referer := range conditions["referer"] {

cmd/bucket-policy-handlers_test.go

@@ -913,7 +913,7 @@ func TestBucketPolicyConditionMatch(t *testing.T) {
 			statementCondition: getStatementWithCondition("StringNotEquals", "s3:prefix", "Asia/"),
 			condition:          getInnerMap("prefix", "Asia/"),
 
-			expectedMatch: true,
+			expectedMatch: false,
 		},
 		// Test case - 6.
 		// StringNotEquals condition doesn't match.
@@ -922,7 +922,7 @@ func TestBucketPolicyConditionMatch(t *testing.T) {
 			statementCondition: getStatementWithCondition("StringNotEquals", "s3:prefix", "Asia/"),
 			condition:          getInnerMap("prefix", "Africa/"),
 
-			expectedMatch: false,
+			expectedMatch: true,
 		},
 		// Test case - 7.
 		// StringNotEquals condition matches.
@@ -931,7 +931,7 @@ func TestBucketPolicyConditionMatch(t *testing.T) {
 			statementCondition: getStatementWithCondition("StringNotEquals", "s3:max-keys", "Asia/"),
 			condition:          getInnerMap("max-keys", "Asia/"),
 
-			expectedMatch: true,
+			expectedMatch: false,
 		},
 		// Test case - 8.
 		// StringNotEquals condition doesn't match.
@@ -940,7 +940,7 @@ func TestBucketPolicyConditionMatch(t *testing.T) {
 			statementCondition: getStatementWithCondition("StringNotEquals", "s3:max-keys", "Asia/"),
 			condition:          getInnerMap("max-keys", "Africa/"),
 
-			expectedMatch: false,
+			expectedMatch: true,
 		},
 		// Test case - 9.
 		// StringLike condition matches.
@@ -977,7 +977,8 @@ func TestBucketPolicyConditionMatch(t *testing.T) {
 			// call the function under test and assert the result with the expected result.
 			doesMatch := bucketPolicyConditionMatch(tc.condition, tc.statementCondition)
 			if tc.expectedMatch != doesMatch {
-				t.Errorf("Expected the match to be `%v`; got `%v`.", tc.expectedMatch, doesMatch)
+				t.Errorf("Expected the match to be `%v`; got `%v` - %v %v.",
+					tc.expectedMatch, doesMatch, tc.condition, tc.statementCondition)
 			}
 		})
 	}