From fbba52068a4cba53804c7e356e2868da8a0354ff Mon Sep 17 00:00:00 2001 From: Dan Schomburg <94418270+dschom@users.noreply.github.com> Date: Tue, 19 Nov 2024 12:48:26 -0800 Subject: [PATCH] task(customs): Alter customs rules for getCredentialStatus action Because: - We've seen an increase in 'unblock sign in' page views, which led to investigating the customs config. - If a key stretching upgrade occurs, we will trigger a getCredentialsStatus, passwordChange & accountLogin action. - Therefore we potentially have 3x the number actions during an upgrade scenario. This Commit: - Moves the getCredentialStatus action into the ACCOUNT_STATUS_ACTION set, which more correctly fits the nature of this action. - The default for maxAccountStatusCheck has been increased, because key stretching changes need a bit more overhead for these types of actions. --- packages/fxa-customs-server/lib/actions.js | 1 + .../fxa-customs-server/lib/config/config.js | 29 +------------------ .../test/remote/user_defined_rules.js | 5 +--- 3 files changed, 3 insertions(+), 32 deletions(-) diff --git a/packages/fxa-customs-server/lib/actions.js b/packages/fxa-customs-server/lib/actions.js index b2e5bf6116..43bdeccad8 100644 --- a/packages/fxa-customs-server/lib/actions.js +++ b/packages/fxa-customs-server/lib/actions.js @@ -40,6 +40,7 @@ const ACCOUNT_STATUS_ACTION = { accountStatusCheck: true, sendUnblockCode: true, recoveryKeyExists: true, + getCredentialsStatus: true, }; // Actions that send an email, and hence might make diff --git a/packages/fxa-customs-server/lib/config/config.js b/packages/fxa-customs-server/lib/config/config.js index 60248d4d9d..f8cc4c7777 100644 --- a/packages/fxa-customs-server/lib/config/config.js +++ b/packages/fxa-customs-server/lib/config/config.js @@ -163,7 +163,7 @@ module.exports = function (fs, path, url, convict) { }, maxAccountStatusCheck: { doc: 'Number of account status checks within rateLimitIntervalSeconds before throttling', - default: 5, + default: 20, format: 'nat', env: 'MAX_ACCOUNT_STATUS_CHECK', }, @@ -414,33 +414,6 @@ module.exports = function (fs, path, url, convict) { }, tracing: tracingConfig, userDefinedRateLimitRules: { - getCredentialsStatusRules: { - actions: { - doc: 'Array of actions that this rule should be applied to', - default: ['getCredentialsStatus'], - format: Array, - }, - limits: { - max: { - doc: 'max actions during `period` that can occur before rate limit is applied', - format: 'nat', - default: 120, - env: 'GET_CREDENTIALS_STATUS_RULE_MAX', - }, - periodMs: { - doc: 'period needed before rate limit is reset', - format: 'duration', - default: '60 seconds', - env: 'GET_CREDENTIALS_STATUS_RULE_PERIOD_MS', - }, - rateLimitIntervalMs: { - doc: 'how long rate limit is applied', - format: 'duration', - default: '15 minutes', - env: 'GET_CREDENTIALS_STATUS_RULE_LIMIT_INTERVAL_MS', - }, - }, - }, totpCodeRules: { actions: { doc: 'Array of actions that this rule should be applied to', diff --git a/packages/fxa-customs-server/test/remote/user_defined_rules.js b/packages/fxa-customs-server/test/remote/user_defined_rules.js index 5f04d364bc..72bd487ddd 100644 --- a/packages/fxa-customs-server/test/remote/user_defined_rules.js +++ b/packages/fxa-customs-server/test/remote/user_defined_rules.js @@ -19,16 +19,13 @@ function randomIp() { } const config = require('../../lib/config').getProperties(); -config.userDefinedRateLimitRules.getCredentialsStatusRules.limits.max = 2; -config.userDefinedRateLimitRules.getCredentialsStatusRules.limits.periodMs = 1000; -config.userDefinedRateLimitRules.getCredentialsStatusRules.limits.rateLimitIntervalMs = 1000; config.userDefinedRateLimitRules.totpCodeRules.limits.periodMs = 1000; config.userDefinedRateLimitRules.totpCodeRules.limits.rateLimitIntervalMs = 1000; config.userDefinedRateLimitRules.tokenCodeRules.limits.max = 2; config.userDefinedRateLimitRules.tokenCodeRules.limits.periodMs = 1000; config.userDefinedRateLimitRules.tokenCodeRules.limits.rateLimitIntervalMs = 1000; -const ACTIONS = ['verifyTotpCode', 'verifyTokenCode', 'getCredentialsStatus']; +const ACTIONS = ['verifyTotpCode', 'verifyTokenCode']; const testServer = new TestServer(config);