Improvement/cldsrv 966/support anon req limiting

lib/api/api.js

@@ -90,7 +90,6 @@ const {
     buildRateChecksFromConfig,
     checkRateLimitsForRequest,
 } = require('./apiUtils/rateLimit/helpers');
-const rateLimitCache = require('./apiUtils/rateLimit/cache');
 
 const monitoringMap = policies.actionMaps.actionMonitoringMapS3;
 
@@ -288,12 +287,8 @@ function callApiHandler(apiMethod, apiHandler, request, response, log, callback)
     // Vault as a hint so the returned rate limit config is the target account.
     // Only included when the cache has a hit.
     const authOptions = {};
-    if (request.bucketName) {
-        const cachedOwner = rateLimitCache.getCachedBucketOwner(request.bucketName);
-        if (cachedOwner) {
-            request.rateLimitTargetAccount = cachedOwner;
-            authOptions.targetAccount = cachedOwner;
-        }
+    if (request.rateLimitTargetAccount !== undefined) {
+        authOptions.targetAccount = request.rateLimitTargetAccount;
     }
 
     return async.waterfall(
@@ -419,7 +414,7 @@ function callApiHandler(apiMethod, apiHandler, request, response, log, callback)
                     log,
                     (err, res) => {
                         request.accountQuotas = infos?.accountQuota;
-                        request.accountLimits = infos?.limits;
+                        request.rateLimitTargetAccountLimits = infos?.limits;
                         if (err) {
                             return next(err);
                         }
@@ -509,6 +504,10 @@ const api = {
             checks.push(...buildRateChecksFromConfig('bucket', request.bucketName, rateLimitConfig.bucket));
         }
 
+        if (rateLimitConfig.bucketOwner !== undefined) {
+            request.rateLimitTargetAccount = rateLimitConfig.bucketOwner;
+        }
+
         if (rateLimitConfig.account !== undefined) {
             request.rateLimitAccountAlreadyChecked = true;
             checks.push(...buildRateChecksFromConfig('account', rateLimitConfig.bucketOwner, rateLimitConfig.account));

lib/api/apiUtils/rateLimit/config.js

@@ -156,17 +156,17 @@ const { rateLimitDefaultConfigCacheTTL, rateLimitDefaultBurstCapacity } = requir
  * @property {object} defaultConfig - Default config applied if no resource specific configuration is found.
  * @property {number} configCacheTTL - Number of milliseconds to cache per resource configs
  * @property {number} defaultBurstCapacity - Default used if resource does not specify a burst capacity.
-*/
+ */
 
 const rateLimitClassConfigSchema = Joi.object({
     defaultConfig: Joi.object({
         requestsPerSecond: Joi.object({
             limit: Joi.number().integer().min(0).required(),
-            burstCapacity: Joi.number().positive(),
+            burstCapacity: Joi.number().min(0),
         }),
     }),
     configCacheTTL: Joi.number().integer().positive().default(rateLimitDefaultConfigCacheTTL),
-    defaultBurstCapacity: Joi.number().positive().default(rateLimitDefaultBurstCapacity),
+    defaultBurstCapacity: Joi.number().min(0).default(rateLimitDefaultBurstCapacity),
 }).default({
     defaultConfig: undefined,
     configCacheTTL: rateLimitDefaultConfigCacheTTL,
@@ -233,17 +233,17 @@ function transformClassConfig(resourceClass, validatedCfg, nodes) {
         if (limit > 0 && limit < nodes) {
             throw new Error(
                 `rateLimiting.${resourceClass}.defaultConfig.` +
-                `requestsPerSecond.limit (${limit}) must be >= ` +
-                `nodes (${nodes}) ` +
-                'or 0 (unlimited). Each node enforces limit/nodes locally. ' +
-                `With limit < ${nodes}, per-node rate would be < 1 req/s, effectively blocking traffic.`
+                    `requestsPerSecond.limit (${limit}) must be >= ` +
+                    `nodes (${nodes}) ` +
+                    'or 0 (unlimited). Each node enforces limit/nodes locally. ' +
+                    `With limit < ${nodes}, per-node rate would be < 1 req/s, effectively blocking traffic.`,
             );
         }
 
         // Store both the original limit and the calculated values
         defaultConfig.RequestsPerSecond = {
             Limit: limit,
-            BurstCapacity: burstCapacity || validatedCfg.defaultBurstCapacity,
+            BurstCapacity: burstCapacity ?? validatedCfg.defaultBurstCapacity,
         };
     }
 
@@ -263,14 +263,11 @@ function transformClassConfig(resourceClass, validatedCfg, nodes) {
  */
 function parseRateLimitConfig(rateLimitingConfig) {
     // Validate configuration using Joi schema
-    const { error: validationError, value: validated } = rateLimitConfigSchema.validate(
-        rateLimitingConfig,
-        {
-            abortEarly: false,   // Return all validation errors at once
-            allowUnknown: false, // Don't allow key not present in schema
-            convert: false,      // Don't do type coercion (e.g. "1" -> 1)
-        }
-    );
+    const { error: validationError, value: validated } = rateLimitConfigSchema.validate(rateLimitingConfig, {
+        abortEarly: false, // Return all validation errors at once
+        allowUnknown: false, // Don't allow key not present in schema
+        convert: false, // Don't do type coercion (e.g. "1" -> 1)
+    });
 
     if (validationError) {
         const details = validationError.details.map(d => d.message).join('; ');
@@ -284,11 +281,7 @@ function parseRateLimitConfig(rateLimitingConfig) {
         nodes: validated.nodes,
         tokenBucketBufferSize: validated.tokenBucketBufferSize,
         tokenBucketRefillThreshold: validated.tokenBucketRefillThreshold,
-        error: new ArsenalError(
-            validated.error.code,
-            validated.error.statusCode,
-            validated.error.message,
-        ),
+        error: new ArsenalError(validated.error.code, validated.error.statusCode, validated.error.message),
     };
 
     parsed.bucket = transformClassConfig('bucket', validated.bucket, parsed.nodes);

lib/api/apiUtils/rateLimit/helpers.js

@@ -1,7 +1,12 @@
 const { config } = require('../../../Config');
+const vault = require('../../../auth/vault');
 const cache = require('./cache');
 const { getTokenBucket } = require('./tokenBucket');
-const { policies: { actionMaps: { actionMapBucketRateLimit } } } = require('arsenal');
+const {
+    policies: {
+        actionMaps: { actionMapBucketRateLimit },
+    },
+} = require('arsenal');
 
 const rateLimitApiActions = Object.keys(actionMapBucketRateLimit);
 
@@ -58,7 +63,7 @@ function extractBucketRateLimitConfig(bucketMD, log) {
         cfg: {
             ...config.rateLimiting.bucket.defaultConfig,
             source: 'global',
-        }
+        },
     });
 
     return {
@@ -76,24 +81,24 @@ function extractBucketRateLimitConfig(bucketMD, log) {
  * 1. Per-account configuration (from request)
  * 2. Global default configuration
  *
- * @param {object} authInfo - Instance of AuthInfo class with requester's info
- * @param {object} request - request object given by router
+ * @param {string} canonicalId - canonicalId of target account
+ * @param {object} accountLimits - account rate limit config
  * @param {object} log - Logger instance
  * @returns {object} Rate limit config
  */
-function extractAccountRateLimitConfig(authInfo, request, log) {
+function extractAccountRateLimitConfig(canonicalId, accountLimits, log) {
     // Try per-account config first
-    if (request.accountLimits) {
+    if (accountLimits) {
         const merged = {
             RequestsPerSecond: {
                 ...config.rateLimiting.account.defaultConfig.RequestsPerSecond,
-                ...(request.accountLimits.RequestsPerSecond || {}),
-                source: request.accountLimits.RequestsPerSecond !== undefined ? 'resource' : 'global',
+                ...(accountLimits.RequestsPerSecond || {}),
+                source: accountLimits.RequestsPerSecond !== undefined ? 'resource' : 'global',
             },
         };
 
         log.debug('Extracted per-account rate limit config', {
-            accountId: authInfo.getCanonicalID(),
+            canonicalId,
             cfg: merged,
         });
 
@@ -108,21 +113,13 @@ function extractAccountRateLimitConfig(authInfo, request, log) {
     };
 
     log.debug('Using global default rate limit config', {
-        accountId: authInfo.getCanonicalID(),
+        canonicalId,
         cfg,
     });
 
     return cfg;
 }
 
-function extractRateLimitConfigFromRequest(request, authInfo, bucketMD, log) {
-    const limitConfig = {
-        bucket: extractBucketRateLimitConfig(bucketMD, log),
-        account: extractAccountRateLimitConfig(authInfo, request, log),
-    };
-    return limitConfig;
-}
-
 function getCachedRateLimitConfig(request) {
     const cachedConfig = {};
     const cachedBucketConfig = cache.getCachedConfig(cache.namespace.bucket, request.bucketName);
@@ -132,10 +129,10 @@ function getCachedRateLimitConfig(request) {
 
     const cachedOwner = cache.getCachedBucketOwner(request.bucketName);
     if (cachedOwner !== undefined) {
+        cachedConfig.bucketOwner = cachedOwner;
         const cachedAccountConfig = cache.getCachedConfig(cache.namespace.account, cachedOwner);
         if (cachedAccountConfig !== undefined) {
             cachedConfig.account = cachedAccountConfig;
-            cachedConfig.bucketOwner = cachedOwner;
         }
     }
 
@@ -173,7 +170,7 @@ function checkRateLimitsForRequest(checks, log) {
                 source: check.source,
             });
 
-            return { allowed: false, rateLimitSource: `${check.resourceClass}:${check.source}`};
+            return { allowed: false, rateLimitSource: `${check.resourceClass}:${check.source}` };
         }
 
         buckets.push(bucket);
@@ -192,12 +189,44 @@ function checkRateLimitsForRequest(checks, log) {
     return { allowed: true };
 }
 
+async function fetchAccountRateLimitConfig(canonicalId, log) {
+    return new Promise((resolve, reject) =>
+        vault.getAccountLimitsByCanonicalId(canonicalId, log, (err, res) => {
+            if (err) {
+                reject(err);
+            } else {
+                resolve(res);
+            }
+        }),
+    );
+}
+
+async function resolveRateLimitConfig(request, authInfo, bucketMD, log) {
+    let accountLimits = request.rateLimitTargetAccountLimits;
+    // Account limits need to be fetched from Vault in 2 cases
+    // 1) A cross-account request where the bucket owner was not found in the cache.
+    // 2) An anonymous request as no previous call to Vault was made.
+    if (
+        (!request.rateLimitTargetAccount && authInfo.getCanonicalID() !== bucketMD.getOwner()) ||
+        (authInfo.isRequesterPublicUser && authInfo.isRequesterPublicUser())
+    ) {
+        accountLimits = await fetchAccountRateLimitConfig(bucketMD.getOwner(), log);
+    }
+
+    const limitConfig = {
+        bucket: extractBucketRateLimitConfig(bucketMD, log),
+        account: extractAccountRateLimitConfig(bucketMD.getOwner(), accountLimits, log),
+    };
+
+    return limitConfig;
+}
+
 module.exports = {
     rateLimitApiActions,
     extractBucketRateLimitConfig,
-    extractRateLimitConfigFromRequest,
     buildRateChecksFromConfig,
     checkRateLimitsForRequest,
     getCachedRateLimitConfig,
     requestNeedsRateCheck,
+    resolveRateLimitConfig,
 };

lib/metadata/metadataUtils.js

@@ -16,7 +16,7 @@ const cache = require('../api/apiUtils/rateLimit/cache');
 const {
     rateLimitApiActions,
     requestNeedsRateCheck,
-    extractRateLimitConfigFromRequest,
+    resolveRateLimitConfig,
     buildRateChecksFromConfig,
     checkRateLimitsForRequest,
 } = require('../api/apiUtils/rateLimit/helpers');
@@ -273,65 +273,68 @@ function validateBucket(bucket, params, log, actionImplicitDenies = {}) {
 function checkRateLimitIfNeeded(request, authInfo, bucketMD, log, callback) {
     // Skip if already checked or not enabled
     if (!requestNeedsRateCheck(request)) {
-        return process.nextTick(callback, null);
+        process.nextTick(callback, null);
+        return;
     }
 
-    // Extract rate limit config from bucket metadata and cache it
-    const checks = [];
-    const rateLimitConfig = extractRateLimitConfigFromRequest(request, authInfo, bucketMD, log);
+    resolveRateLimitConfig(request, authInfo, bucketMD, log)
+        .then(rateLimitConfig => {
+            const checks = [];
+            cache.setCachedBucketOwner(
+                bucketMD.getName(),
+                bucketMD.getOwner(),
+                config.rateLimiting.bucket.configCacheTTL,
+            );
+
+            if (!request.rateLimitBucketAlreadyChecked && rateLimitConfig.bucket !== undefined) {
+                cache.setCachedConfig(
+                    cache.namespace.bucket,
+                    bucketMD.getName(),
+                    rateLimitConfig.bucket,
+                    config.rateLimiting.bucket.configCacheTTL,
+                );
+                checks.push(...buildRateChecksFromConfig('bucket', bucketMD.getName(), rateLimitConfig.bucket));
+                // eslint-disable-next-line no-param-reassign
+                request.rateLimitBucketAlreadyChecked = true;
+            }
 
-    cache.setCachedBucketOwner(bucketMD.getName(), bucketMD.getOwner(), config.rateLimiting.bucket.configCacheTTL);
+            if (!request.rateLimitAccountAlreadyChecked && rateLimitConfig.account !== undefined) {
+                const targetAccount = request.rateLimitTargetAccount
+                    ? request.rateLimitTargetAccount
+                    : bucketMD.getOwner();
 
-    if (!request.rateLimitBucketAlreadyChecked && rateLimitConfig.bucket !== undefined) {
-        cache.setCachedConfig(
-            cache.namespace.bucket,
-            bucketMD.getName(),
-            rateLimitConfig.bucket,
-            config.rateLimiting.bucket.configCacheTTL,
-        );
-        checks.push(...buildRateChecksFromConfig('bucket', bucketMD.getName(), rateLimitConfig.bucket));
-        // eslint-disable-next-line no-param-reassign
-        request.rateLimitBucketAlreadyChecked = true;
-    }
-
-    if (
-        !request.rateLimitAccountAlreadyChecked &&
-        rateLimitConfig.account !== undefined &&
-        !(authInfo.isRequesterPublicUser && authInfo.isRequesterPublicUser())
-    ) {
-        const targetAccount = request.rateLimitTargetAccount
-            ? request.rateLimitTargetAccount
-            : authInfo.getCanonicalID();
-
-        cache.setCachedConfig(
-            cache.namespace.account,
-            targetAccount,
-            rateLimitConfig.account,
-            config.rateLimiting.account.configCacheTTL,
-        );
-        checks.push(...buildRateChecksFromConfig('account', targetAccount, rateLimitConfig.account));
-        // eslint-disable-next-line no-param-reassign
-        request.rateLimitAccountAlreadyChecked = true;
-    }
+                cache.setCachedConfig(
+                    cache.namespace.account,
+                    targetAccount,
+                    rateLimitConfig.account,
+                    config.rateLimiting.account.configCacheTTL,
+                );
+                checks.push(...buildRateChecksFromConfig('account', targetAccount, rateLimitConfig.account));
+                // eslint-disable-next-line no-param-reassign
+                request.rateLimitAccountAlreadyChecked = true;
+            }
 
-    const { allowed, rateLimitSource } = checkRateLimitsForRequest(checks, log);
-    if (!allowed) {
-        log.addDefaultFields({
-            rateLimited: true,
-            rateLimitSource,
-        });
+            const { allowed, rateLimitSource } = checkRateLimitsForRequest(checks, log);
+            if (!allowed) {
+                log.addDefaultFields({
+                    rateLimited: true,
+                    rateLimitSource,
+                });
 
-        if (request.serverAccessLog) {
-            /* eslint-disable no-param-reassign */
-            request.serverAccessLog.rateLimited = true;
-            request.serverAccessLog.rateLimitSource = rateLimitSource;
-            /* eslint-enable no-param-reassign */
-        }
+                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);
-    }
+                process.nextTick(callback, config.rateLimiting.error);
+                return;
+            }
 
-    return process.nextTick(callback, null);
+            process.nextTick(callback, null);
+        })
+        .catch(err => callback(err));
 }
 
 /** standardMetadataValidateBucketAndObj - retrieve bucket and object md from metadata