CLDSRV-256 fix and test tag conditions updateRequestContexts() helper

lib/api/apiUtils/authorization/tagConditionKeys.js

@@ -1,5 +1,5 @@
-const assert = require('assert');
 const async = require('async');
+
 const { auth, s3middleware } = require('arsenal');
 const metadata = require('../../../metadata/wrapper');
 const { decodeVersionId } = require('../object/versioning');
@@ -12,64 +12,64 @@ function makeTagQuery(tags) {
         .join('&');
 }
 
-function updateRequestContexts(request, requestContexts, apiMethod, log, cb) {
-    requestContexts.forEach(rc => {
-        rc.setNeedTagEval(true);
-
-        async.series([
-            next => {
-                if (request.headers['x-amz-tagging']) {
-                    rc.setRequestObjTags(request.headers['x-amz-tagging']);
-                    process.nextTick(() => next());
-                } else if (request.post && apiMethod === 'objectPutTagging') {
-                    parseTagXml(request.post, log, (err, tags) => {
-                        if (err) {
-                            log.trace('error parsing request tags');
-                            return next(err);
-                        }
-                        rc.setRequestObjTags(makeTagQuery(tags));
-                        return next();
-                    });
-                } else {
-                    process.nextTick(() => next());
-                }
-            },
-            next => {
-                const objectKey = request.objectKey;
-                const bucketName = request.bucketName;
-                const decodedVidResult = decodeVersionId(request.query);
-                if (decodedVidResult instanceof Error) {
-                    log.trace('invalid versionId query', {
-                        versionId: request.query.versionId,
-                        error: decodedVidResult,
-                    });
-                    return process.nextTick(() => next(decodedVidResult));
-                }
-                const reqVersionId = decodedVidResult;
-                return metadata.getObjectMD(bucketName, objectKey, { versionId: reqVersionId }, log,
-                (err, objMD) => {
+function updateRequestContextsWithTags(request, requestContexts, apiMethod, log, cb) {
+    async.waterfall([
+        next => {
+            if (request.headers['x-amz-tagging']) {
+                return next(null, request.headers['x-amz-tagging']);
+            }
+            if (request.post && apiMethod === 'objectPutTagging') {
+                return parseTagXml(request.post, log, (err, tags) => {
+                    if (err) {
+                        log.trace('error parsing request tags');
+                        return next(err);
+                    }
+                    return next(null, makeTagQuery(tags));
+                });
+            }
+            return next(null, null);
+        },
+        (requestTagsQuery, next) => {
+            const objectKey = request.objectKey;
+            const bucketName = request.bucketName;
+            const decodedVidResult = decodeVersionId(request.query);
+            if (decodedVidResult instanceof Error) {
+                log.trace('invalid versionId query', {
+                    versionId: request.query.versionId,
+                    error: decodedVidResult,
+                });
+                return next(decodedVidResult);
+            }
+            const reqVersionId = decodedVidResult;
+            return metadata.getObjectMD(
+                bucketName, objectKey, { versionId: reqVersionId }, log, (err, objMD) => {
                     if (err) {
                         // TODO: move to `.is` once BKTCLT-9 is done and bumped in Cloudserver
                         if (err.NoSuchKey) {
-                            return next();
+                            return next(null, requestTagsQuery, null);
                         }
                         log.trace('error getting request object tags');
                         return next(err);
                     }
-                    const existingTags = objMD.tags;
-                    if (existingTags) {
-                        rc.setExistingObjTag(makeTagQuery(existingTags));
-                    }
-                    return next();
+                    const existingTagsQuery = objMD.tags && makeTagQuery(objMD.tags);
+                    return next(null, requestTagsQuery, existingTagsQuery);
                 });
-            },
-        ], err => {
-            if (err) {
-                log.trace('error processing tag condition key evaluation');
-                return cb(err);
+        },
+    ], (err, requestTagsQuery, existingTagsQuery) => {
+        if (err) {
+            log.trace('error processing tag condition key evaluation');
+            return cb(err);
+        }
+        for (const rc of requestContexts) {
+            rc.setNeedTagEval(true);
+            if (requestTagsQuery) {
+                rc.setRequestObjTags(requestTagsQuery);
             }
-            return cb(null, requestContexts);
-        });
+            if (existingTagsQuery) {
+                rc.setExistingObjTag(existingTagsQuery);
+            }
+        }
+        return cb();
     });
 }
 
@@ -81,20 +81,17 @@ function tagConditionKeyAuth(authorizationResults, request, requestContexts, api
         return cb();
     }
 
-    return updateRequestContexts(request, requestContexts, apiMethod, log, (err, updatedContexts) => {
+    return updateRequestContextsWithTags(request, requestContexts, apiMethod, log, err => {
         if (err) {
             return cb(err);
         }
-        if (assert.deepStrictEqual(requestContexts, updatedContexts)) {
-            return cb();
-        }
         return auth.server.doAuth(request, log,
-            (err, userInfo, tagAuthResults) => cb(err, tagAuthResults), 's3', updatedContexts);
+            (err, userInfo, authResults) => cb(err, authResults), 's3', requestContexts);
     });
 }
 
 module.exports = {
     tagConditionKeyAuth,
-    updateRequestContexts,
+    updateRequestContextsWithTags,
     makeTagQuery,
 };

tests/unit/api/apiUtils/tagConditionKeys.js

@@ -8,9 +8,10 @@ const {
     TaggingConfigTester,
     createRequestContext,
 } = require('../../helpers');
-const { tagConditionKeyAuth, updateRequestContexts, makeTagQuery } =
+const { tagConditionKeyAuth, updateRequestContextsWithTags, makeTagQuery } =
     require('../../../../lib/api/apiUtils/authorization/tagConditionKeys');
 const { bucketPut } = require('../../../../lib/api/bucketPut');
+const objectPut = require('../../../../lib/api/objectPut');
 
 const log = new DummyRequestLogger();
 const bucketName = 'tagconditionkeybuckettester';
@@ -39,27 +40,41 @@ const objectPutReq = new DummyRequest({
     calculatedHash: 'vnR+tLdVF79rPPfF+7YvOg==',
 }, postBody);
 
-const requestContexts = [
+const objectPutRequestContexts = [
     createRequestContext('objectPut', objectPutReq),
 ];
 
+const objectGetReq = {
+    bucketName,
+    headers: {
+        host: `${bucketName}.s3.amazonaws.com`,
+    },
+    objectKey,
+    url: `/${bucketName}/${objectKey}`,
+    query: {},
+};
+const objectGetRequestContexts = [
+    createRequestContext('objectGet', objectGetReq),
+    createRequestContext('objectGetTagging', objectGetReq),
+];
+
 describe('tagConditionKeyAuth', () => {
     it('should return empty if no previous auth results', done => {
-        tagConditionKeyAuth([], objectPutReq, requestContexts, 'bucketPut', log, err => {
+        tagConditionKeyAuth([], objectPutReq, objectPutRequestContexts, 'bucketPut', log, err => {
             assert.ifError(err);
             done();
         });
     });
     it('should return empty if auth results do not contain checkTagConditions key', done => {
         const authResults = [{ isAllowed: true }, { isAllowed: true }];
-        tagConditionKeyAuth(authResults, objectPutReq, requestContexts, 'bucketPut', log, err => {
+        tagConditionKeyAuth(authResults, objectPutReq, objectPutRequestContexts, 'bucketPut', log, err => {
             assert.ifError(err);
             done();
         });
     });
 });
 
-describe('updateRequestContexts', () => {
+describe('updateRequestContextsWithTags', () => {
     before(done => {
         cleanup();
         bucketPut(authInfo, bucketPutReq, log, done);
@@ -68,11 +83,30 @@ describe('updateRequestContexts', () => {
     after(cleanup);
 
     it('should update request context with request object tags', done => {
-        updateRequestContexts(objectPutReq, requestContexts, 'objectPut', log, (err, newRequestContexts) => {
+        updateRequestContextsWithTags(objectPutReq, objectPutRequestContexts, 'objectPut', log, err => {
             assert.ifError(err);
-            assert(newRequestContexts[0].getNeedTagEval());
-            assert.strictEqual(newRequestContexts[0].getRequestObjTags(), makeTagQuery(taggingUtil.getTags()));
+            assert(objectPutRequestContexts[0].getNeedTagEval());
+            assert.strictEqual(objectPutRequestContexts[0].getRequestObjTags(),
+                               makeTagQuery(taggingUtil.getTags()));
+            assert.strictEqual(objectPutRequestContexts[0].getExistingObjTag(), null);
             done();
         });
     });
+
+    it('should update multiple request contexts with existing object tags', done => {
+        objectPut(authInfo, objectPutReq, 'foobar', log, err => {
+            assert.ifError(err);
+            updateRequestContextsWithTags(objectGetReq, objectGetRequestContexts, 'objectGet', log,
+            err => {
+                assert.ifError(err);
+                for (const requestContext of objectGetRequestContexts) {
+                    assert(requestContext.getNeedTagEval());
+                    assert.strictEqual(requestContext.getExistingObjTag(),
+                                       makeTagQuery(taggingUtil.getTags()));
+                    assert.strictEqual(requestContext.getRequestObjTags(), null);
+                }
+                done();
+            });
+        });
+    });
 });