Skip to content

Commit 9478786

Browse files
committed
CLDSRV-774: Add aclRequired field to server access logs
aclRequired indicates whether the request required an ACL for authorization. It is "Yes" when no bucket policy exists or when the bucket policy returns DEFAULT_DENY, falling back to ACL evaluation. It is "-" for owner requests, service accounts, and requests decided by IAM or bucket policy alone. For copy operations, the source bucket auth runs on the same request object and would overwrite the destination's aclRequired. The value is saved before source auth, then moved to sourceServerAccessLog and the destination's value is restored.
1 parent c09dc78 commit 9478786

3 files changed

Lines changed: 32 additions & 7 deletions

File tree

lib/api/apiUtils/authorization/permissionChecks.js

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -509,8 +509,10 @@ function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner,
509509
request, aclPermission, results, actionImplicitDenies) {
510510
const bucketPolicy = bucket.getBucketPolicy();
511511
let processedResult = results[requestType];
512+
let aclRequired = false;
512513
if (!bucketPolicy || request?.bypassUserBucketPolicies) {
513514
processedResult = actionImplicitDenies[requestType] === false && aclPermission;
515+
aclRequired = true;
514516
} else {
515517
const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType, canonicalID, arn,
516518
bucketOwner, log, request, actionImplicitDenies);
@@ -525,9 +527,10 @@ function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner,
525527
processedResult = true;
526528
} else {
527529
processedResult = actionImplicitDenies[requestType] === false && aclPermission;
530+
aclRequired = true;
528531
}
529532
}
530-
return processedResult;
533+
return { allowed: processedResult, aclRequired };
531534
}
532535

533536
function isBucketAuthorized(bucket, requestTypesInput, canonicalID, authInfo, log, request,
@@ -564,8 +567,13 @@ function isBucketAuthorized(bucket, requestTypesInput, canonicalID, authInfo, lo
564567
_requestType = 'objectGet';
565568
actionImplicitDenies.objectGet = actionImplicitDenies.objectGet || false;
566569
}
567-
return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucket.getOwner(), log,
568-
request, aclPermission, results, actionImplicitDenies);
570+
const { allowed, aclRequired } = processBucketPolicy(_requestType, bucket, canonicalID, arn,
571+
bucket.getOwner(), log, request, aclPermission, results, actionImplicitDenies);
572+
if (aclRequired && request?.serverAccessLog) {
573+
// eslint-disable-next-line no-param-reassign
574+
request.serverAccessLog.aclRequired = 'Yes';
575+
}
576+
return allowed;
569577
});
570578
}
571579

@@ -582,8 +590,9 @@ function evaluateBucketPolicyWithIAM(bucket, requestTypesInput, canonicalID, aut
582590
if (authInfo) {
583591
arn = authInfo.getArn();
584592
}
585-
return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucket.getOwner(), log,
593+
const { allowed } = processBucketPolicy(_requestType, bucket, canonicalID, arn, bucket.getOwner(), log,
586594
request, true, results, actionImplicitDenies);
595+
return allowed;
587596
});
588597
}
589598

@@ -641,8 +650,13 @@ function isObjAuthorized(bucket, objectMD, requestTypesInput, canonicalID, authI
641650
}
642651
const aclPermission = checkObjectAcls(bucket, objectMD, parsedMethodName,
643652
canonicalID, requesterIsNotUser, isUserUnauthenticated, mainApiCall);
644-
return processBucketPolicy(_requestType, bucket, canonicalID, arn, bucketOwner,
653+
const { allowed, aclRequired } = processBucketPolicy(_requestType, bucket, canonicalID, arn, bucketOwner,
645654
log, request, aclPermission, results, actionImplicitDenies);
655+
if (aclRequired && request?.serverAccessLog) {
656+
// eslint-disable-next-line no-param-reassign
657+
request.serverAccessLog.aclRequired = 'Yes';
658+
}
659+
return allowed;
646660
});
647661
}
648662

lib/metadata/metadataUtils.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ function storeServerAccessLogInfo(request, bucket, raftSessionId, options = {})
5050
target.enabled = true;
5151
target.loggingEnabled = bucket.getBucketLoggingStatus().getLoggingEnabled();
5252
}
53+
// Source auth runs on the same request object and overwrites the
54+
// destination's aclRequired. Move the source value to its log entry
55+
// and restore the destination value that was saved before source auth.
56+
target.aclRequired = request.serverAccessLog.aclRequired;
57+
request.serverAccessLog.aclRequired = options.savedAclRequired;
5358
} else {
5459
// Default behavior: store in main serverAccessLog
5560
request.serverAccessLog.raftSessionID = raftSessionId;
@@ -326,6 +331,10 @@ function standardMetadataValidateBucketAndObj(params, actionImplicitDenies, log,
326331
if (!Array.isArray(requestType)) {
327332
requestType = [requestType];
328333
}
334+
if (params.serverAccessLogOptions?.copySource) {
335+
// eslint-disable-next-line no-param-reassign
336+
params.serverAccessLogOptions.savedAclRequired = request?.serverAccessLog?.aclRequired;
337+
}
329338
async.waterfall([
330339
next => {
331340
// versionId may be 'null', which asks metadata to fetch the null key specifically

lib/utilities/serverAccessLogger.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ function buildLogEntry(req, params, options) {
495495
tlsVersion: req.socket?.encrypted
496496
? req.socket.getCipher()['version']
497497
: req.headers?.['x-ssl-protocol'] ?? undefined,
498-
aclRequired: options.aclRequired ?? undefined, // TODO: CLDSRV-774
498+
aclRequired: options.aclRequired ?? undefined,
499499
// hostID: undefined, // NOT IMPLEMENTED
500500
// accessPointARN: undefined, // NOT IMPLEMENTED
501501

@@ -557,6 +557,7 @@ function logBatchDeleteObject(req, requestID, objectKey, objectSize, error) {
557557
errorCode,
558558
objectKey,
559559
requestID,
560+
aclRequired: req.serverAccessLog?.aclRequired,
560561
});
561562

562563
serverAccessLogger.write(`${JSON.stringify(logEntry)}\n`);
@@ -589,7 +590,7 @@ function logServerAccess(req, res) {
589590
totalTime: calculateTotalTime(params.startTime, params.onFinishEndTime),
590591
turnAroundTime: calculateTurnAroundTime(params.startTurnAroundTime, endTurnAroundTime),
591592
versionId: req.query?.versionId,
592-
aclRequired: undefined, // TODO: CLDSRV-774
593+
aclRequired: req.serverAccessLog.aclRequired,
593594
referer: req.headers?.referer,
594595
userAgent: req.headers?.['user-agent'],
595596
requestID,
@@ -659,6 +660,7 @@ function logCopySourceAccess(req, requestID, operation, sourceBucket, sourceObje
659660
httpCode,
660661
errorCode,
661662
requestID,
663+
aclRequired: params.aclRequired,
662664
});
663665

664666
serverAccessLogger.write(`${JSON.stringify(logEntry)}\n`);

0 commit comments

Comments
 (0)