CLDSRV-774: Add aclRequired field to server access logs

lib/api/apiUtils/authorization/permissionChecks.js

@@ -509,8 +509,10 @@ function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner,
     request, aclPermission, results, actionImplicitDenies) {
     const bucketPolicy = bucket.getBucketPolicy();
     let processedResult = results[requestType];
+    let aclRequired = false;
     if (!bucketPolicy || request?.bypassUserBucketPolicies) {
         processedResult = actionImplicitDenies[requestType] === false && aclPermission;
+        aclRequired = true;
     } else {
         const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType, canonicalID, arn,
             bucketOwner, log, request, actionImplicitDenies);
@@ -525,9 +527,10 @@ function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner,
             processedResult = true;
         } else {
             processedResult = actionImplicitDenies[requestType] === false && aclPermission;
+            aclRequired = true;
         }
     }
-    return processedResult;
+    return { allowed: processedResult, aclRequired };
 }
 
 function isBucketAuthorized(bucket, requestTypesInput, canonicalID, authInfo, log, request,
@@ -564,8 +567,13 @@ function isBucketAuthorized(bucket, requestTypesInput, canonicalID, authInfo, lo
             _requestType = 'objectGet';
             actionImplicitDenies.objectGet = actionImplicitDenies.objectGet || false;
         }
-        return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucket.getOwner(), log,
-            request, aclPermission, results, actionImplicitDenies);
+        const { allowed, aclRequired } = processBucketPolicy(_requestType, bucket, canonicalID, arn,
+            bucket.getOwner(), log, request, aclPermission, results, actionImplicitDenies);
+        if (aclRequired && request?.serverAccessLog) {
+            // eslint-disable-next-line no-param-reassign
+            request.serverAccessLog.aclRequired = 'Yes';
+        }
+        return allowed;
     });
 }
 
@@ -582,8 +590,9 @@ function evaluateBucketPolicyWithIAM(bucket, requestTypesInput, canonicalID, aut
         if (authInfo) {
             arn = authInfo.getArn();
         }
-        return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucket.getOwner(), log,
+        const { allowed } = processBucketPolicy(_requestType, bucket, canonicalID, arn, bucket.getOwner(), log,
             request, true, results, actionImplicitDenies);
+        return allowed;
     });
 }
 
@@ -641,8 +650,13 @@ function isObjAuthorized(bucket, objectMD, requestTypesInput, canonicalID, authI
         }
         const aclPermission = checkObjectAcls(bucket, objectMD, parsedMethodName,
             canonicalID, requesterIsNotUser, isUserUnauthenticated, mainApiCall);
-        return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucketOwner,
+        const { allowed, aclRequired } = processBucketPolicy(_requestType, bucket, canonicalID, arn, bucketOwner,
             log, request, aclPermission, results, actionImplicitDenies);
+        if (aclRequired && request?.serverAccessLog) {
+            // eslint-disable-next-line no-param-reassign
+            request.serverAccessLog.aclRequired = 'Yes';
+        }
+        return allowed;
     });
 }
 

lib/metadata/metadataUtils.js

@@ -50,6 +50,11 @@ function storeServerAccessLogInfo(request, bucket, raftSessionId, options = {})
             target.enabled = true;
             target.loggingEnabled = bucket.getBucketLoggingStatus().getLoggingEnabled();
         }
+        // Source auth runs on the same request object and overwrites the
+        // destination's aclRequired. Move the source value to its log entry
+        // and restore the destination value that was saved before source auth.
+        target.aclRequired = request.serverAccessLog.aclRequired;
+        request.serverAccessLog.aclRequired = options.savedAclRequired;
     } else {
         // Default behavior: store in main serverAccessLog
         request.serverAccessLog.raftSessionID = raftSessionId;
@@ -326,6 +331,10 @@ function standardMetadataValidateBucketAndObj(params, actionImplicitDenies, log,
     if (!Array.isArray(requestType)) {
         requestType = [requestType];
     }
+    if (params.serverAccessLogOptions?.copySource) {
+        // eslint-disable-next-line no-param-reassign
+        params.serverAccessLogOptions.savedAclRequired = request?.serverAccessLog?.aclRequired;
+    }
     async.waterfall([
         next => {
             // versionId may be 'null', which asks metadata to fetch the null key specifically

lib/utilities/serverAccessLogger.js

@@ -495,7 +495,7 @@ function buildLogEntry(req, params, options) {
         tlsVersion: req.socket?.encrypted
             ? req.socket.getCipher()['version']
             : req.headers?.['x-ssl-protocol'] ?? undefined,
-        aclRequired: options.aclRequired ?? undefined,  // TODO: CLDSRV-774
+        aclRequired: options.aclRequired ?? undefined,
         // hostID: undefined,         // NOT IMPLEMENTED
         // accessPointARN: undefined, // NOT IMPLEMENTED
 
@@ -557,6 +557,7 @@ function logBatchDeleteObject(req, requestID, objectKey, objectSize, error) {
         errorCode,
         objectKey,
         requestID,
+        aclRequired: req.serverAccessLog?.aclRequired,
     });
 
     serverAccessLogger.write(`${JSON.stringify(logEntry)}\n`);
@@ -589,7 +590,7 @@ function logServerAccess(req, res) {
         totalTime: calculateTotalTime(params.startTime, params.onFinishEndTime),
         turnAroundTime: calculateTurnAroundTime(params.startTurnAroundTime, endTurnAroundTime),
         versionId: req.query?.versionId,
-        aclRequired: undefined, // TODO: CLDSRV-774
+        aclRequired: req.serverAccessLog.aclRequired,
         referer: req.headers?.referer,
         userAgent: req.headers?.['user-agent'],
         requestID,
@@ -659,6 +660,7 @@ function logCopySourceAccess(req, requestID, operation, sourceBucket, sourceObje
         httpCode,
         errorCode,
         requestID,
+        aclRequired: params.aclRequired,
     });
 
     serverAccessLogger.write(`${JSON.stringify(logEntry)}\n`);

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@zenko/cloudserver",
-  "version": "9.2.33",
+  "version": "9.2.34",
   "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
   "main": "index.js",
   "engines": {

tests/functional/aws-node-sdk/test/serverAccessLogs/testServerAccessLogFile.js

@@ -143,7 +143,7 @@ describe('Server Access Logs - File Output', async () => {
             'authenticationType': 'AuthHeader', // STATIC
             // 'hostHeader': '', // UNKNOWN
             'tlsVersion': null, // TODO: Add https tests.
-            'aclRequired': null, // TODO: Add https tests.
+            'aclRequired': null, // DYNAMIC (absent for owner, "Yes" when ACL is consulted)
             'bucketOwner': '79a59df900b949e55d96a1e698fbacedfd6e09d98eacf8f8d5218e7cd47ef2be', // DYNAMIC
             bucketName, // DYNAMIC
             // 'req_id': '', // UNKNOWN

tests/unit/api/apiUtils/permissionChecks.js

@@ -1,11 +1,15 @@
 const assert = require('assert');
 
-const { isLifecycleSession, checkBucketPolicyResult, checkBucketPolicy } =
+const { isLifecycleSession, checkBucketPolicyResult, checkBucketPolicy,
+    isBucketAuthorized, isObjAuthorized, evaluateBucketPolicyWithIAM } =
     require('../../../../lib/api/apiUtils/authorization/permissionChecks.js');
 const { DummyRequestLogger } = require('../../helpers');
 
 const stubLog = new DummyRequestLogger();
 
+const ownerCanonicalId = '79a59df900b949e55d96a1e698fbacedfd6e09d98eacf8f8d5218e7cd47ef2be';
+const otherCanonicalId = 'aaaaaaaabbbbbbbbccccccccddddddddeeeeeeeeffffffffgggggggghhhhhhh1';
+
 describe('authInfoHelper', () => {
     const tests = [
         {
@@ -708,3 +712,214 @@ describe('checkBucketPolicy Principal logic', () => {
         });
     });
 });
+
+describe('aclRequired field in isBucketAuthorized', () => {
+    function makeBucket(owner, acl, bucketPolicy) {
+        return {
+            getOwner: () => owner,
+            getAcl: () => ({
+                Canned: acl?.Canned || '',
+                FULL_CONTROL: acl?.FULL_CONTROL || [],
+                READ: acl?.READ || [],
+                READ_ACP: acl?.READ_ACP || [],
+                WRITE: acl?.WRITE || [],
+                WRITE_ACP: acl?.WRITE_ACP || [],
+            }),
+            getBucketPolicy: () => bucketPolicy || null,
+        };
+    }
+
+    function makeRequest() {
+        return { serverAccessLog: {}, socket: { remoteAddress: '127.0.0.1' }, headers: {} };
+    }
+
+    function makeAuthInfo(canonicalID, arn, isIAMUser = false) {
+        return {
+            getCanonicalID: () => canonicalID,
+            getArn: () => arn,
+            isRequesterAnIAMUser: () => isIAMUser,
+        };
+    }
+
+    it('should not set aclRequired when requester is bucket owner', () => {
+        const request = makeRequest();
+        const bucket = makeBucket(ownerCanonicalId);
+        const authInfo = makeAuthInfo(ownerCanonicalId, 'arn:aws:iam::123456789012:/owner/');
+        isBucketAuthorized(bucket, 'bucketGet', ownerCanonicalId, authInfo, stubLog,
+            request, { bucketGet: false });
+        assert.strictEqual(request.serverAccessLog.aclRequired, undefined);
+    });
+
+    it('should set aclRequired to Yes for non-owner with no bucket policy', () => {
+        const request = makeRequest();
+        const bucket = makeBucket(ownerCanonicalId, { READ: [otherCanonicalId] });
+        const authInfo = makeAuthInfo(otherCanonicalId, 'arn:aws:iam::999999999999:user/other');
+        isBucketAuthorized(bucket, 'bucketGet', otherCanonicalId, authInfo, stubLog,
+            request, { bucketGet: false });
+        assert.strictEqual(request.serverAccessLog.aclRequired, 'Yes');
+    });
+
+    it('should not set aclRequired when bucket policy explicitly allows', () => {
+        const request = makeRequest();
+        // Use an IAM user in the bucket owner's account so principal match is OK (not CROSS_ACCOUNT)
+        // and the owner early-return doesn't fire (requester is IAM user, not account root)
+        const iamUserCanonicalId = ownerCanonicalId;
+        const bucket = makeBucket(ownerCanonicalId, {}, {
+            Statement: [{
+                Effect: 'Allow',
+                Principal: { AWS: 'arn:aws:iam::123456789012:user/iamuser' },
+                Action: ['s3:ListBucket'],
+                Resource: ['arn:aws:s3:::test-bucket'],
+            }],
+        });
+        const authInfo = makeAuthInfo(iamUserCanonicalId, 'arn:aws:iam::123456789012:user/iamuser', true);
+        isBucketAuthorized(bucket, 'bucketGet', iamUserCanonicalId, authInfo, stubLog,
+            request, { bucketGet: false });
+        assert.strictEqual(request.serverAccessLog.aclRequired, undefined);
+    });
+
+    it('should not set aclRequired when bucket policy explicitly denies', () => {
+        const request = makeRequest();
+        const iamUserCanonicalId = ownerCanonicalId;
+        const bucket = makeBucket(ownerCanonicalId, { READ: [iamUserCanonicalId] }, {
+            Statement: [{
+                Effect: 'Deny',
+                Principal: { AWS: 'arn:aws:iam::123456789012:user/iamuser' },
+                Action: ['s3:ListBucket'],
+                Resource: ['arn:aws:s3:::test-bucket'],
+            }],
+        });
+        const authInfo = makeAuthInfo(iamUserCanonicalId, 'arn:aws:iam::123456789012:user/iamuser', true);
+        isBucketAuthorized(bucket, 'bucketGet', iamUserCanonicalId, authInfo, stubLog,
+            request, { bucketGet: false });
+        assert.strictEqual(request.serverAccessLog.aclRequired, undefined);
+    });
+
+    it('should set aclRequired to Yes when bucket policy returns DEFAULT_DENY', () => {
+        const request = makeRequest();
+        // Policy grants PutObject to a different principal — nothing matches
+        // the bucketGet request from otherCanonicalId, so checkBucketPolicy
+        // returns DEFAULT_DENY and falls back to ACL evaluation.
+        const bucket = makeBucket(ownerCanonicalId, { READ: [otherCanonicalId] }, {
+            Statement: [{
+                Effect: 'Allow',
+                Principal: { AWS: 'arn:aws:iam::111111111111:root' },
+                Action: ['s3:PutObject'],
+                Resource: ['arn:aws:s3:::test-bucket/*'],
+            }],
+        });
+        const authInfo = makeAuthInfo(otherCanonicalId, 'arn:aws:iam::999999999999:user/other');
+        isBucketAuthorized(bucket, 'bucketGet', otherCanonicalId, authInfo, stubLog,
+            request, { bucketGet: false });
+        assert.strictEqual(request.serverAccessLog.aclRequired, 'Yes');
+    });
+
+    it('should handle missing serverAccessLog gracefully', () => {
+        const request = {};
+        const bucket = makeBucket(ownerCanonicalId, { READ: [otherCanonicalId] });
+        const authInfo = makeAuthInfo(otherCanonicalId, 'arn:aws:iam::999999999999:user/other');
+        assert.doesNotThrow(() => {
+            isBucketAuthorized(bucket, 'bucketGet', otherCanonicalId, authInfo, stubLog,
+                request, { bucketGet: false });
+        });
+    });
+});
+
+describe('aclRequired field in isObjAuthorized', () => {
+    function makeBucket(owner, acl, bucketPolicy) {
+        return {
+            getOwner: () => owner,
+            getName: () => 'test-bucket',
+            getAcl: () => ({
+                Canned: acl?.Canned || '',
+                FULL_CONTROL: acl?.FULL_CONTROL || [],
+                READ: acl?.READ || [],
+                READ_ACP: acl?.READ_ACP || [],
+                WRITE: acl?.WRITE || [],
+                WRITE_ACP: acl?.WRITE_ACP || [],
+            }),
+            getBucketPolicy: () => bucketPolicy || null,
+        };
+    }
+
+    function makeRequest() {
+        return { serverAccessLog: {} };
+    }
+
+    function makeAuthInfo(canonicalID, arn) {
+        return {
+            getCanonicalID: () => canonicalID,
+            getArn: () => arn,
+            isRequesterAnIAMUser: () => false,
+        };
+    }
+
+    function makeObjectMD(ownerId) {
+        return {
+            'owner-id': ownerId,
+            acl: {
+                Canned: '',
+                FULL_CONTROL: [],
+                READ: [],
+                READ_ACP: [],
+                WRITE: [],
+                WRITE_ACP: [],
+            },
+        };
+    }
+
+    it('should not set aclRequired when requester is object owner', () => {
+        const request = makeRequest();
+        const bucket = makeBucket(ownerCanonicalId);
+        const objectMD = makeObjectMD(otherCanonicalId);
+        const authInfo = makeAuthInfo(otherCanonicalId, 'arn:aws:iam::999999999999:/account/');
+        isObjAuthorized(bucket, objectMD, 'objectGet', otherCanonicalId, authInfo, stubLog,
+            request, { objectGet: false });
+        assert.strictEqual(request.serverAccessLog.aclRequired, undefined);
+    });
+
+    it('should set aclRequired to Yes for non-owner with no bucket policy', () => {
+        const request = makeRequest();
+        const bucket = makeBucket(ownerCanonicalId);
+        const objectMD = makeObjectMD(ownerCanonicalId);
+        objectMD.acl.READ = [otherCanonicalId];
+        const authInfo = makeAuthInfo(otherCanonicalId, 'arn:aws:iam::999999999999:user/other');
+        isObjAuthorized(bucket, objectMD, 'objectGet', otherCanonicalId, authInfo, stubLog,
+            request, { objectGet: false });
+        assert.strictEqual(request.serverAccessLog.aclRequired, 'Yes');
+    });
+});
+
+describe('aclRequired field in evaluateBucketPolicyWithIAM', () => {
+    function makeBucket(owner, bucketPolicy) {
+        return {
+            getOwner: () => owner,
+            getAcl: () => ({
+                Canned: '',
+                FULL_CONTROL: [],
+                READ: [],
+                READ_ACP: [],
+                WRITE: [],
+                WRITE_ACP: [],
+            }),
+            getBucketPolicy: () => bucketPolicy || null,
+        };
+    }
+
+    function makeAuthInfo(canonicalID, arn) {
+        return {
+            getCanonicalID: () => canonicalID,
+            getArn: () => arn,
+            isRequesterAnIAMUser: () => false,
+        };
+    }
+
+    it('should not set aclRequired (ACLs are not actually consulted)', () => {
+        const request = { serverAccessLog: {} };
+        const bucket = makeBucket(ownerCanonicalId);
+        const authInfo = makeAuthInfo(otherCanonicalId, 'arn:aws:iam::999999999999:user/other');
+        evaluateBucketPolicyWithIAM(bucket, 'objectDelete', otherCanonicalId, authInfo,
+            { objectDelete: false }, stubLog, request);
+        assert.strictEqual(request.serverAccessLog.aclRequired, undefined);
+    });
+});