Skip to content

impr(CLDSRV-855): Update rate limit post check #6129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tmacro wants to merge 3 commits into
base: improvement/CLDSRV-855/update_helpers
Choose a base branch
from
+52 −48
Open

impr(CLDSRV-855): Update rate limit post check #6129

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
db880cf
impr(CLDSRV-855): Update post check
tmacro Apr 1, 2026
70bb7c5
fix jsdoc
tmacro Apr 1, 2026
a15c7e4
fix jsdoc round 2
tmacro Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 52 additions & 48 deletions lib/metadata/metadataUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@ const { onlyOwnerAllowed } = require('../../constants');
const { actionNeedQuotaCheck, actionWithDataDeletion } = require('arsenal/build/lib/policyEvaluator/RequestContext');
const { processBytesToWrite, validateQuotas } = require('../api/apiUtils/quotas/quotaUtils');
const { config } = require('../Config');
const cache = require('../api/apiUtils/rateLimit/cache');
const {
extractAndCacheRateLimitConfig,
checkRateLimitWithConfig,
rateLimitApiActions,
requestNeedsRateCheck,
extractRateLimitConfigFromRequest,
buildRateChecksFromConfig,
checkRateLimitsForRequest,
} = require('../api/apiUtils/rateLimit/helpers');

/**
Expand Down Expand Up @@ -242,67 +245,68 @@ function validateBucket(bucket, params, log, actionImplicitDenies = {}) {
* Extracts rate limit config from bucket metadata, caches it, and enforces limit.
* Calls callback with error if rate limited, null if allowed or no rate limiting.
*
* @param {object} bucket - Bucket metadata object
* @param {string} bucketName - Bucket name
* @param {object} request - Request object with rateLimitAlreadyChecked tracker
* @param {object} request - Request object
* @param {AuthInfo} authInfo - AuthInfo class instance, requester's info
* @param {BucketInfo} bucketMD - Bucket metadata
* @param {object} log - Logger instance
* @param {function} callback - Callback(err) - err if rate limited, null if allowed
* @returns {undefined}
*/
Copy link
Copy Markdown

@claude claude bot Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSDoc is stale — still documents the old (bucket, bucketName, request, log, callback) signature. Should be updated to match the new (request, authInfo, bucketMD, log, callback) params and mention rateLimitBucketAlreadyChecked/rateLimitAccountAlreadyChecked instead of rateLimitAlreadyChecked.

— Claude Code

function checkRateLimitIfNeeded(bucket, bucketName, request, log, callback) {
function checkRateLimitIfNeeded(request, authInfo, bucketMD, log, callback) {
// Skip if already checked or not enabled
if (request.rateLimitAlreadyChecked
|| !config.rateLimiting?.enabled
|| rateLimitApiActions.includes(request.apiMethod)
|| request.isInternalServiceRequest) {
if (!requestNeedsRateCheck(request)) {
return process.nextTick(callback, null);
}

// Extract rate limit config from bucket metadata and cache it
const rateLimitConfig = extractAndCacheRateLimitConfig(bucket, bucketName, log);
const checks = [];
const rateLimitConfig = extractRateLimitConfigFromRequest(request, authInfo, bucketMD, log);

// No rate limiting configured
if (!rateLimitConfig) {
if (!request.rateLimitBucketAlreadyChecked && rateLimitConfig.bucket !== undefined) {
cache.setCachedConfig(
cache.namespace.bucket,
bucketMD.getName(),
rateLimitConfig.bucket,
config.rateLimiting.bucket.configCacheTTL
);
cache.setCachedBucketOwner(bucketMD.getName(), bucketMD.getOwner(), config.rateLimiting.bucket.configCacheTTL);
checks.push(...buildRateChecksFromConfig('bucket', bucketMD.getName(), rateLimitConfig.bucket));
// eslint-disable-next-line no-param-reassign
request.rateLimitAlreadyChecked = true;
return process.nextTick(callback, null);
request.rateLimitBucketAlreadyChecked = true;
}

// Check rate limit with GCRA
return checkRateLimitWithConfig(
bucketName,
rateLimitConfig,
log,
(rateLimitErr, rateLimited) => {
if (rateLimitErr) {
log.error('Rate limit check error in metadata validation', {
error: rateLimitErr,
});
}
if (!request.rateLimitAccountAlreadyChecked && rateLimitConfig.account !== undefined) {
cache.setCachedConfig(
cache.namespace.account,
authInfo.getCanonicalID(),
rateLimitConfig.account,
config.rateLimiting.account.configCacheTTL
);
checks.push(...buildRateChecksFromConfig('account', authInfo.getCanonicalID(), rateLimitConfig.account));
// eslint-disable-next-line no-param-reassign
request.rateLimitAccountAlreadyChecked = true;
}

if (rateLimited) {
log.addDefaultFields({
rateLimited: true,
rateLimitSource: rateLimitConfig.source,
});
// Add to server access log
/* eslint-disable no-param-reassign */
if (request.serverAccessLog) {
request.serverAccessLog.rateLimited = true;
request.serverAccessLog.rateLimitSource = rateLimitConfig.source;
}
request.rateLimitAlreadyChecked = true;
/* eslint-enable no-param-reassign */
return callback(config.rateLimiting.error);
}
const { allowed, rateLimitSource } = checkRateLimitsForRequest(checks, log);
if (!allowed) {
log.addDefaultFields({
rateLimited: true,
rateLimitSource,
});

// Allowed - set tracker and continue
// eslint-disable-next-line no-param-reassign
request.rateLimitAlreadyChecked = true;
return callback(null);
if (request.serverAccessLog) {
/* eslint-disable no-param-reassign */
request.serverAccessLog.rateLimited = true;
request.serverAccessLog.rateLimitSource = rateLimitSource;
/* eslint-enable no-param-reassign */
}
);

return process.nextTick(callback, config.rateLimiting.error);
}

return process.nextTick(callback, null);
}

/** standardMetadataValidateBucketAndObj - retrieve bucket and object md from metadata
* and check if user is authorized to access them.
* @param {object} params - function parameters
Expand Down Expand Up @@ -362,7 +366,7 @@ function standardMetadataValidateBucketAndObj(params, actionImplicitDenies, log,
}

// Rate limiting check if not already done in api.js
return checkRateLimitIfNeeded(bucket, bucketName, request, log, err => {
return checkRateLimitIfNeeded(request, authInfo, bucket, log, err => {
if (err) {
return next(err, bucket);
}
Expand Down Expand Up @@ -456,7 +460,7 @@ function standardMetadataValidateBucket(params, actionImplicitDenies, log, callb
}

// Rate limiting check if not already done in api.js
return checkRateLimitIfNeeded(bucket, bucketName, request, log, err => {
return checkRateLimitIfNeeded(request, params.authInfo, bucket, log, err => {
if (err) {
return callback(err);
}
Expand Down
Loading