api: BucketNotification should disallow duplicate notification. (#2539)

Added checks to look for duplicated notification configs.

Fixes #2472

cmd/api-errors.go

@@ -119,6 +119,7 @@ const (
 	ErrFilterNamePrefix
 	ErrFilterNameSuffix
 	ErrFilterValueInvalid
+	ErrOverlappingConfigs
 
 	// S3 extended errors.
 	ErrContentSHA256Mismatch
@@ -505,6 +506,11 @@ var errorCodeResponse = map[APIErrorCode]APIError{
 		Description:    "Size of filter rule value cannot exceed 1024 bytes in UTF-8 representation",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
+	ErrOverlappingConfigs: {
+		Code:           "InvalidArgument",
+		Description:    "Configurations overlap. Configurations on the same bucket cannot share a common event type.",
+		HTTPStatusCode: http.StatusBadRequest,
+	},
 
 	/// S3 extensions.
 	ErrContentSHA256Mismatch: {

cmd/bucket-notification-datatypes.go

@@ -32,33 +32,30 @@ type keyFilter struct {
 	FilterRules []filterRule `xml:"FilterRule,omitempty"`
 }
 
-// Queue SQS configuration.
-type queueConfig struct {
+// Common elements of service notification.
+type serviceConfig struct {
 	Events []string `xml:"Event"`
 	Filter struct {
 		Key keyFilter `xml:"S3Key,omitempty"`
 	}
-	ID       string `xml:"Id"`
+	ID string `xml:"Id"`
+}
+
+// Queue SQS configuration.
+type queueConfig struct {
+	serviceConfig
 	QueueARN string `xml:"Queue"`
 }
 
 // Topic SNS configuration, this is a compliance field not used by minio yet.
 type topicConfig struct {
-	Events []string `xml:"Event"`
-	Filter struct {
-		Key keyFilter `xml:"S3Key"`
-	}
-	ID       string `xml:"Id"`
+	serviceConfig
 	TopicARN string `xml:"Topic"`
 }
 
 // Lambda function configuration, this is a compliance field not used by minio yet.
 type lambdaConfig struct {
-	Events []string `xml:"Event"`
-	Filter struct {
-		Key keyFilter `xml:"S3Key,omitempty"`
-	}
-	ID        string `xml:"Id"`
+	serviceConfig
 	LambdaARN string `xml:"CloudFunction"`
 }
 

cmd/bucket-notification-utils.go

@@ -266,17 +266,68 @@ func validateTopicConfigs(topicConfigs []topicConfig) APIErrorCode {
 	return ErrNone
 }
 
+// Check all the queue configs for any duplicates.
+func checkDuplicateQueueConfigs(configs []queueConfig) APIErrorCode {
+	configMaps := make(map[string]int)
+
+	// Navigate through each configs and count the entries.
+	for _, config := range configs {
+		configMaps[config.QueueARN]++
+	}
+
+	// Validate if there are any duplicate counts.
+	for _, count := range configMaps {
+		if count != 1 {
+			return ErrOverlappingConfigs
+		}
+	}
+
+	// Success.
+	return ErrNone
+}
+
+// Check all the topic configs for any duplicates.
+func checkDuplicateTopicConfigs(configs []topicConfig) APIErrorCode {
+	configMaps := make(map[string]int)
+
+	// Navigate through each configs and count the entries.
+	for _, config := range configs {
+		configMaps[config.TopicARN]++
+	}
+
+	// Validate if there are any duplicate counts.
+	for _, count := range configMaps {
+		if count != 1 {
+			return ErrOverlappingConfigs
+		}
+	}
+
+	// Success.
+	return ErrNone
+}
+
 // Validates all the bucket notification configuration for their validity,
 // if one of the config is malformed or has invalid data it is rejected.
 // Configuration is never applied partially.
 func validateNotificationConfig(nConfig notificationConfig) APIErrorCode {
+	// Validate all queue configs.
 	if s3Error := validateQueueConfigs(nConfig.QueueConfigs); s3Error != ErrNone {
 		return s3Error
 	}
+	// Validate all topic configs.
 	if s3Error := validateTopicConfigs(nConfig.TopicConfigs); s3Error != ErrNone {
 		return s3Error
 	}
 
+	// Check for duplicate queue configs.
+	if s3Error := checkDuplicateQueueConfigs(nConfig.QueueConfigs); s3Error != ErrNone {
+		return s3Error
+	}
+	// Check for duplicate topic configs.
+	if s3Error := checkDuplicateTopicConfigs(nConfig.TopicConfigs); s3Error != ErrNone {
+		return s3Error
+	}
+
 	// Add validation for other configurations.
 	return ErrNone
 }

cmd/server_test.go

@@ -75,10 +75,9 @@ func (s *TestSuiteCommon) TestAuth(c *C) {
 	c.Assert(len(accessID), Equals, minioAccessID)
 }
 
-// TestBucketNotification - Inserts the bucket notification and
-// verifies it by fetching the notification back.
+// TestBucketNotification - Inserts the bucket notification and verifies it by fetching the notification back.
 func (s *TestSuiteCommon) TestBucketNotification(c *C) {
-	// Sample bucket notification
+	// Sample bucket notification.
 	bucketNotificationBuf := `<NotificationConfiguration><TopicConfiguration><Event>s3:ObjectCreated:Put</Event><Filter><S3Key><FilterRule><Name>prefix</Name><Value>images/</Value></FilterRule></S3Key></Filter><Id>1</Id><Topic>arn:minio:sns:us-east-1:444455556666:listen</Topic></TopicConfiguration></NotificationConfiguration>`
 
 	// generate a random bucket Name.
@@ -100,7 +99,7 @@ func (s *TestSuiteCommon) TestBucketNotification(c *C) {
 	c.Assert(err, IsNil)
 
 	client = http.Client{}
-	// execute the HTTP request to create bucket.
+	// execute the HTTP request.
 	response, err = client.Do(request)
 
 	c.Assert(err, IsNil)
@@ -128,7 +127,7 @@ func (s *TestSuiteCommon) TestBucketNotification(c *C) {
 	c.Assert(err, IsNil)
 
 	client = http.Client{}
-	// execute the HTTP request to create bucket.
+	// execute the HTTP request.
 	response, err = client.Do(request)
 	c.Assert(err, IsNil)
 
@@ -140,7 +139,7 @@ func (s *TestSuiteCommon) TestBucketNotification(c *C) {
 	c.Assert(err, IsNil)
 
 	client = http.Client{}
-	// execute the HTTP request to create bucket.
+	// execute the HTTP request.
 	response, err = client.Do(request)
 	c.Assert(err, IsNil)
 
@@ -152,10 +151,21 @@ func (s *TestSuiteCommon) TestBucketNotification(c *C) {
 	c.Assert(err, IsNil)
 
 	client = http.Client{}
-	// execute the HTTP request to create bucket.
+	// execute the HTTP request.
 	response, err = client.Do(request)
 	c.Assert(err, IsNil)
 	verifyError(c, response, "InvalidArgument", "A specified event is not supported for notifications.", http.StatusBadRequest)
+
+	bucketNotificationDuplicates := `<NotificationConfiguration><TopicConfiguration><Event>s3:ObjectCreated:Put</Event><Filter><S3Key><FilterRule><Name>prefix</Name><Value>images/</Value></FilterRule></S3Key></Filter><Id>1</Id><Topic>arn:minio:sns:us-east-1:444455556666:listen</Topic></TopicConfiguration><TopicConfiguration><Event>s3:ObjectCreated:Put</Event><Filter><S3Key><FilterRule><Name>prefix</Name><Value>images/</Value></FilterRule></S3Key></Filter><Id>1</Id><Topic>arn:minio:sns:us-east-1:444455556666:listen</Topic></TopicConfiguration></NotificationConfiguration>`
+	request, err = newTestSignedRequest("PUT", getPutNotificationURL(s.endPoint, bucketName),
+		int64(len(bucketNotificationDuplicates)), bytes.NewReader([]byte(bucketNotificationDuplicates)), s.accessKey, s.secretKey)
+	c.Assert(err, IsNil)
+
+	client = http.Client{}
+	// execute the HTTP request.
+	response, err = client.Do(request)
+	c.Assert(err, IsNil)
+	verifyError(c, response, "InvalidArgument", "Configurations overlap. Configurations on the same bucket cannot share a common event type.", http.StatusBadRequest)
 }
 
 // TestBucketPolicy - Inserts the bucket policy and verifies it by fetching the policy back.