From 7409b0be8d8228d7673c5decd74f0ddcee9c39fe Mon Sep 17 00:00:00 2001
From: Aayush Chouhan <achouhan@redhat.com>
Date: Thu, 26 Dec 2024 22:10:05 +0530
Subject: [PATCH] Added a check for duplicate Id in bucket lifecycle rules

Signed-off-by: Aayush Chouhan <achouhan@redhat.com>
---
 .../s3/ops/s3_put_bucket_lifecycle.js         |  8 ++++
 src/test/lifecycle/common.js                  | 41 +++++++++++++++++++
 src/test/system_tests/test_lifecycle.js       |  1 +
 3 files changed, 50 insertions(+)

diff --git a/src/endpoint/s3/ops/s3_put_bucket_lifecycle.js b/src/endpoint/s3/ops/s3_put_bucket_lifecycle.js
index b079d2b151..6e77135df0 100644
--- a/src/endpoint/s3/ops/s3_put_bucket_lifecycle.js
+++ b/src/endpoint/s3/ops/s3_put_bucket_lifecycle.js
@@ -83,6 +83,7 @@ function parse_lifecycle_field(field, field_parser = parseInt) {
  * http://docs.aws.amazon.com/AmazonS3/latest/API/RESTBucketPUTlifecycle.html
  */
 async function put_bucket_lifecycle(req) {
+    const id_set = new Set();
     const lifecycle_rules = _.map(req.body.LifecycleConfiguration.Rule, rule => {
         const current_rule = {
             filter: {},
@@ -100,6 +101,13 @@ async function put_bucket_lifecycle(req) {
             current_rule.id = uuid();
         }
 
+        // Check for duplicate ID in the rules
+        if (id_set.has(current_rule.id)) {
+            dbg.error('Rule ID must be unique. Found same ID for more than one rule: ', current_rule.id);
+            throw new S3Error({ ...S3Error.InvalidArgument, message: 'Rule ID must be unique. Found same ID for more than one rule'});
+        }
+        id_set.add(current_rule.id);
+
         if (rule.Status?.length !== 1) {
             dbg.error('Rule should have status', rule);
             throw new S3Error(S3Error.InvalidArgument);
diff --git a/src/test/lifecycle/common.js b/src/test/lifecycle/common.js
index c4a2001287..61c06896aa 100644
--- a/src/test/lifecycle/common.js
+++ b/src/test/lifecycle/common.js
@@ -359,6 +359,36 @@ function id_lifecycle_configuration(Bucket, Key) {
     };
 }
 
+function duplicate_id_lifecycle_configuration(Bucket, Key) {
+    const ID1 = 'rule_id';
+    const ID2 = ID1; // set duplicate ID
+    return {
+        Bucket,
+        LifecycleConfiguration: {
+            Rules: [{
+                ID1,
+                Expiration: {
+                    Days: 17,
+                },
+                Filter: {
+                    Prefix: Key,
+                },
+                Status: 'Enabled',
+            },
+            {
+                ID2,
+                Expiration: {
+                    Days: 18,
+                },
+                Filter: {
+                    Prefix: Key,
+                },
+                Status: 'Enabled',
+            }, ],
+        },
+    };
+}
+
 async function put_get_lifecycle_configuration(Bucket, putLifecycleParams, s3) {
     const putLifecycleResult = await s3.putBucketLifecycleConfiguration(putLifecycleParams);
     console.log('put lifecycle params:', putLifecycleParams, 'result', putLifecycleResult);
@@ -528,3 +558,14 @@ exports.test_rule_id_length = async function(Bucket, Key, s3) {
         assert(error.code === 'InvalidArgument', `Expected InvalidArgument: id length exceeding ${s3_const.MAX_RULE_ID_LENGTH} characters`);
     }
 };
+
+exports.test_rule_duplicate_id = async function(Bucket, Key, s3) {
+    const putLifecycleParams = duplicate_id_lifecycle_configuration(Bucket, Key);
+
+    try {
+        await s3.putBucketLifecycleConfiguration(putLifecycleParams);
+        assert.fail('Expected error for duplicate rule ID, but request was successful');
+    } catch (error) {
+        assert(error.code === 'InvalidArgument', 'Expected InvalidArgument: duplicate ID found in the rules');
+    }
+};
diff --git a/src/test/system_tests/test_lifecycle.js b/src/test/system_tests/test_lifecycle.js
index ea00ee0d69..7b23c0d1d6 100644
--- a/src/test/system_tests/test_lifecycle.js
+++ b/src/test/system_tests/test_lifecycle.js
@@ -55,6 +55,7 @@ async function main() {
     await commonTests.test_filter_size(Bucket, s3);
     await commonTests.test_and_prefix_size(Bucket, Key, s3);
     await commonTests.test_rule_id_length(Bucket, Key, s3);
+    await commonTests.test_rule_duplicate_id(Bucket, Key, s3);
 
     const getObjectParams = {
         Bucket,