Merge remote-tracking branch 'origin/w/8.6/bugfix/CLDSRV-508-fix-buck…

…et-tagging' into w/8.7/bugfix/CLDSRV-508-fix-bucket-tagging
scality · Feb 20, 2024 · 2f2f91d · 2f2f91d
2 parents 3186a97 + a28b141
commit 2f2f91d
Show file tree

Hide file tree

Showing 8 changed files with 221 additions and 4 deletions.
diff --git a/lib/api/bucketDeleteTagging.js b/lib/api/bucketDeleteTagging.js
@@ -21,15 +21,19 @@ function bucketDeleteTagging(authInfo, request, log, callback) {
         authInfo,
         bucketName,
         requestType: request.apiMethods || 'bucketDeleteTagging',
+        request,
     };
 
     let bucket = null;
     return waterfall([
         next => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log,
             (err, b) => {
+                if (err) {
+                    return next(err);
+                }
                 bucket = b;
                 bucket.setTags([]);
-                return next(err);
+                return next();
             }),
         next => metadata.updateBucket(bucket.getName(), bucket, log, next),
     ], err => {

diff --git a/lib/api/bucketGetTagging.js b/lib/api/bucketGetTagging.js
@@ -37,7 +37,7 @@ const escapeForXml = s3middleware.escapeForXml;
 function tagsToXml(tags) {
     const xml = [];
 
-    xml.push('<?xml version="1.0" encoding="UTF-8" standalone="yes"?><Tagging><TagSet>');
+    xml.push('<?xml version="1.0" encoding="UTF-8" standalone="yes"?><Tagging> <TagSet>');
 
     tags.forEach(tag => {
         xml.push('<Tag>');
@@ -46,7 +46,7 @@ function tagsToXml(tags) {
         xml.push('</Tag>');
     });
 
-    xml.push('</TagSet></Tagging>');
+    xml.push('</TagSet> </Tagging>');
 
     return xml.join('');
 }

diff --git a/lib/api/bucketPutTagging.js b/lib/api/bucketPutTagging.js
@@ -39,6 +39,7 @@ function bucketPutTagging(authInfo, request, log, callback) {
         authInfo,
         bucketName,
         requestType: request.apiMethods || 'bucketPutTagging',
+        request,
     };
     let bucket = null;
     return waterfall([

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@zenko/cloudserver",
-  "version": "8.7.45",
+  "version": "8.7.46",
   "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
   "main": "index.js",
   "engines": {

diff --git a/tests/unit/api/bucketDeleteTagging.js b/tests/unit/api/bucketDeleteTagging.js
@@ -0,0 +1,74 @@
+const assert = require('assert');
+
+const { bucketPut } = require('../../../lib/api/bucketPut');
+const { cleanup,
+    DummyRequestLogger,
+    makeAuthInfo,
+    TaggingConfigTester }
+    = require('../helpers');
+const bucketPutTagging = require('../../../lib/api/bucketPutTagging');
+const bucketGetTagging = require('../../../lib/api/bucketGetTagging');
+const bucketDeleteTagging = require('../../../lib/api/bucketDeleteTagging');
+const log = new DummyRequestLogger();
+const authInfo = makeAuthInfo('accessKey1');
+const bucketName = 'bucketGetTaggingTest';
+
+const testBucketPutRequest = {
+    bucketName,
+    headers: { host: `${bucketName}.s3.amazonaws.com` },
+    url: '/',
+    actionImplicitDenies: false,
+};
+
+describe('deleteBucketTagging API', () => {
+    beforeEach(done => {
+        cleanup();
+        bucketPut(authInfo, testBucketPutRequest, log, done);
+    });
+
+    afterEach(() => cleanup());
+
+    it('should delete tags resource', done => {
+        const taggingUtil = new TaggingConfigTester();
+        const testBucketPutTaggingRequest = taggingUtil
+            .createBucketTaggingRequest('PUT', bucketName);
+        bucketPutTagging(authInfo, testBucketPutTaggingRequest, log, err => {
+            assert.strictEqual(err, undefined);
+
+            const testBucketGetTaggingRequest = taggingUtil
+                .createBucketTaggingRequest('GET', bucketName);
+            return bucketGetTagging(authInfo, testBucketGetTaggingRequest, log,
+                (err, xml) => {
+                    assert.ifError(err);
+                    assert.strictEqual(xml, taggingUtil.constructXml());
+                    const testBucketDeleteTaggingRequest = taggingUtil
+                        .createBucketTaggingRequest('DELETE', bucketName);
+                    return bucketDeleteTagging(authInfo, testBucketDeleteTaggingRequest,
+                        log, err => {
+                            assert.ifError(err);
+                            return bucketGetTagging(authInfo, testBucketGetTaggingRequest,
+                                log, err => {
+                                    assert(err.NoSuchTagSet);
+                                    return done();
+                                });
+                        });
+                });
+        });
+    });
+
+    it('should return access denied if the authorization check fails', done => {
+        const taggingUtil = new TaggingConfigTester();
+        const testBucketPutTaggingRequest = taggingUtil
+            .createBucketTaggingRequest('PUT', bucketName);
+        bucketPutTagging(authInfo, testBucketPutTaggingRequest, log, err => {
+            assert.ifError(err);
+            const testBucketDeleteTaggingRequest = taggingUtil
+                .createBucketTaggingRequest('DELETE', bucketName, null, true);
+            return bucketDeleteTagging(authInfo, testBucketDeleteTaggingRequest,
+                log, err => {
+                    assert(err.AccessDenied);
+                    return done();
+                });
+        });
+    });
+});
diff --git a/tests/unit/api/bucketGetTagging.js b/tests/unit/api/bucketGetTagging.js
@@ -0,0 +1,67 @@
+const assert = require('assert');
+
+const { bucketPut } = require('../../../lib/api/bucketPut');
+const { cleanup,
+    DummyRequestLogger,
+    makeAuthInfo,
+    TaggingConfigTester,
+} = require('../helpers');
+const bucketPutTagging = require('../../../lib/api/bucketPutTagging');
+const bucketGetTagging = require('../../../lib/api/bucketGetTagging');
+const log = new DummyRequestLogger();
+const authInfo = makeAuthInfo('accessKey1');
+const bucketName = 'bucketGetTaggingTest';
+
+const testBucketPutRequest = {
+    bucketName,
+    headers: { host: `${bucketName}.s3.amazonaws.com` },
+    url: '/',
+    actionImplicitDenies: false,
+};
+
+describe('getBucketTagging API', () => {
+    beforeEach(done => {
+        cleanup();
+        bucketPut(authInfo, testBucketPutRequest, log, done);
+    });
+
+    afterEach(() => cleanup());
+
+    it('should return tags resource', done => {
+        const taggingUtil = new TaggingConfigTester();
+        const testBucketPutTaggingRequest = taggingUtil
+            .createBucketTaggingRequest('PUT', bucketName);
+        bucketPutTagging(authInfo, testBucketPutTaggingRequest, log, err => {
+            assert.strictEqual(err, undefined);
+            const testBucketGetTaggingRequest = taggingUtil
+                .createBucketTaggingRequest('GET', bucketName);
+            return bucketGetTagging(authInfo, testBucketGetTaggingRequest, log,
+            (err, xml) => {
+                if (err) {
+                    process.stdout.write(`Err getting object tagging ${err}`);
+                    return done(err);
+                }
+                assert.strictEqual(xml, taggingUtil.constructXml());
+                return done();
+            });
+        });
+    });
+
+    it('should return access denied if the authorization check fails', done => {
+        const taggingUtil = new TaggingConfigTester();
+        const testBucketPutTaggingRequest = taggingUtil
+            .createBucketTaggingRequest('PUT', bucketName);
+        bucketPutTagging(authInfo, testBucketPutTaggingRequest, log, err => {
+            assert.strictEqual(err, undefined);
+            const testBucketGetTaggingRequest = taggingUtil
+                .createBucketTaggingRequest('GET', bucketName, true);
+            const badAuthInfo = makeAuthInfo('accessKey2');
+            return bucketGetTagging(badAuthInfo, testBucketGetTaggingRequest, log,
+            err => {
+                assert.strictEqual(err.AccessDenied, true);
+                return done();
+            });
+        });
+    });
+});
+
diff --git a/tests/unit/api/bucketPutTagging.js b/tests/unit/api/bucketPutTagging.js
@@ -0,0 +1,53 @@
+const assert = require('assert');
+
+const { bucketPut } = require('../../../lib/api/bucketPut');
+const { cleanup,
+    DummyRequestLogger,
+    makeAuthInfo,
+    TaggingConfigTester,
+} = require('../helpers');
+const bucketPutTagging = require('../../../lib/api/bucketPutTagging');
+const log = new DummyRequestLogger();
+const authInfo = makeAuthInfo('accessKey1');
+const bucketName = 'bucketGetTaggingTest';
+
+const testBucketPutRequest = {
+    bucketName,
+    headers: { host: `${bucketName}.s3.amazonaws.com` },
+    url: '/',
+    actionImplicitDenies: false,
+};
+
+describe('putBucketTagging API', () => {
+    beforeEach(done => {
+        cleanup();
+        bucketPut(authInfo, testBucketPutRequest, log, done);
+    });
+
+    afterEach(() => cleanup());
+
+    it('should set tags resource', done => {
+        const taggingUtil = new TaggingConfigTester();
+        const testBucketPutTaggingRequest = taggingUtil
+            .createBucketTaggingRequest('PUT', bucketName);
+        bucketPutTagging(authInfo, testBucketPutTaggingRequest, log, err => {
+            if (err) {
+                process.stdout.write(`Err putting object tagging ${err}`);
+                return done(err);
+            }
+            assert.strictEqual(err, undefined);
+            return done();
+        });
+    });
+
+    it('should return access denied if the authorization check fails', done => {
+        const taggingUtil = new TaggingConfigTester();
+        const testBucketPutTaggingRequest = taggingUtil
+            .createBucketTaggingRequest('PUT', bucketName);
+        const authInfo = makeAuthInfo('accessKey2');
+        bucketPutTagging(authInfo, testBucketPutTaggingRequest, log, err => {
+            assert(err.AccessDenied);
+            return done();
+        });
+    });
+});
diff --git a/tests/unit/helpers.js b/tests/unit/helpers.js
@@ -438,6 +438,24 @@ class TaggingConfigTester {
         }
         return request;
     }
+
+    createBucketTaggingRequest(method, bucketName, body, implicitDeny = false) {
+        const request = {
+            bucketName,
+            headers: {
+                host: `${bucketName}.s3.amazonaws.com`,
+            },
+            url: '/?tagging',
+            query: { tagging: '' },
+            actionImplicitDenies: implicitDeny,
+        };
+        if (method === 'PUT') {
+            request.post = body || this.constructXml();
+            request.headers['content-md5'] = crypto.createHash('md5')
+                .update(request.post, 'utf8').digest('base64');
+        }
+        return request;
+    }
 }
 
 class AccessControlPolicy {