Replace old rate limiting in password login flow with "SystemAction" rate limiting

Summary:
Depends on D20667. Ref T13343. Password auth currently uses an older rate limiting mechanism, upgrade it to the modern "SystemAction" mechanism.

This mostly just improves consistency, although there are some tangential/theoretical benefits:

- it's not obvious that making the user log GC very quickly could disable rate limiting;
- if we let you configure action limits in the future, which we might, this would become configurable for free.

Test Plan:
- With CAPTCHAs off, made a bunch of invalid login attempts. Got rate limited.
- With CAPTCHAs on, made a bunch of invalid login attempts. Got downgraded to CAPTCHAs after a few.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13343

Differential Revision: https://secure.phabricator.com/D20668

epriestley 7 years ago a75766c0 e090b32c

+53 -30

4 changed files

expand all

src

__phutil_library_map__.php

applications

auth

action

PhabricatorAuthTryPasswordAction.php

PhabricatorAuthTryPasswordWithoutCAPTCHAAction.php

provider

PhabricatorPasswordAuthProvider.php

src/__phutil_library_map__.php

··· 2431 2431 'PhabricatorAuthTestSMSAction' => 'applications/auth/action/PhabricatorAuthTestSMSAction.php', 2432 2432 'PhabricatorAuthTryEmailLoginAction' => 'applications/auth/action/PhabricatorAuthTryEmailLoginAction.php', 2433 2433 'PhabricatorAuthTryFactorAction' => 'applications/auth/action/PhabricatorAuthTryFactorAction.php', 2434 + 'PhabricatorAuthTryPasswordAction' => 'applications/auth/action/PhabricatorAuthTryPasswordAction.php', 2435 + 'PhabricatorAuthTryPasswordWithoutCAPTCHAAction' => 'applications/auth/action/PhabricatorAuthTryPasswordWithoutCAPTCHAAction.php', 2434 2436 'PhabricatorAuthUnlinkController' => 'applications/auth/controller/PhabricatorAuthUnlinkController.php', 2435 2437 'PhabricatorAuthValidateController' => 'applications/auth/controller/PhabricatorAuthValidateController.php', 2436 2438 'PhabricatorAuthWaitForApprovalMessageType' => 'applications/auth/message/PhabricatorAuthWaitForApprovalMessageType.php', ··· 8427 8429 'PhabricatorAuthTestSMSAction' => 'PhabricatorSystemAction', 8428 8430 'PhabricatorAuthTryEmailLoginAction' => 'PhabricatorSystemAction', 8429 8431 'PhabricatorAuthTryFactorAction' => 'PhabricatorSystemAction', 8432 + 'PhabricatorAuthTryPasswordAction' => 'PhabricatorSystemAction', 8433 + 'PhabricatorAuthTryPasswordWithoutCAPTCHAAction' => 'PhabricatorSystemAction', 8430 8434 'PhabricatorAuthUnlinkController' => 'PhabricatorAuthController', 8431 8435 'PhabricatorAuthValidateController' => 'PhabricatorAuthController', 8432 8436 'PhabricatorAuthWaitForApprovalMessageType' => 'PhabricatorAuthMessageType',

+22

src/applications/auth/action/PhabricatorAuthTryPasswordAction.php

··· 1 + <?php 2 + 3 + final class PhabricatorAuthTryPasswordAction 4 + extends PhabricatorSystemAction { 5 + 6 + const TYPECONST = 'auth.password'; 7 + 8 + public function getActionConstant() { 9 + return self::TYPECONST; 10 + } 11 + 12 + public function getScoreThreshold() { 13 + return 100 / phutil_units('1 hour in seconds'); 14 + } 15 + 16 + public function getLimitExplanation() { 17 + return pht( 18 + 'Your remote address has made too many login attempts in a short '. 19 + 'period of time.'); 20 + } 21 + 22 + }

+16

src/applications/auth/action/PhabricatorAuthTryPasswordWithoutCAPTCHAAction.php

··· 1 + <?php 2 + 3 + final class PhabricatorAuthTryPasswordWithoutCAPTCHAAction 4 + extends PhabricatorSystemAction { 5 + 6 + const TYPECONST = 'auth.password-without-captcha'; 7 + 8 + public function getActionConstant() { 9 + return self::TYPECONST; 10 + } 11 + 12 + public function getScoreThreshold() { 13 + return 10 / phutil_units('1 hour in seconds'); 14 + } 15 + 16 + }

+11 -30

src/applications/auth/provider/PhabricatorPasswordAuthProvider.php

··· 255 255 $viewer = $request->getUser(); 256 256 $content_source = PhabricatorContentSource::newFromRequest($request); 257 257 258 - $captcha_limit = 5; 259 - $hard_limit = 32; 260 - $limit_window = phutil_units('15 minutes in seconds'); 258 + $rate_actor = PhabricatorSystemActionEngine::newActorFromRequest($request); 261 259 262 - $failed_attempts = PhabricatorUserLog::loadRecentEventsFromThisIP( 263 - PhabricatorUserLog::ACTION_LOGIN_FAILURE, 264 - $limit_window); 260 + PhabricatorSystemActionEngine::willTakeAction( 261 + array($rate_actor), 262 + new PhabricatorAuthTryPasswordAction(), 263 + 1); 265 264 266 265 // If the same remote address has submitted several failed login attempts 267 266 // recently, require they provide a CAPTCHA response for new attempts. 268 267 $require_captcha = false; 269 268 $captcha_valid = false; 270 269 if (AphrontFormRecaptchaControl::isRecaptchaEnabled()) { 271 - if (count($failed_attempts) > $captcha_limit) { 270 + try { 271 + PhabricatorSystemActionEngine::willTakeAction( 272 + array($rate_actor), 273 + new PhabricatorAuthTryPasswordWithoutCAPTCHAAction(), 274 + 1); 275 + } catch (PhabricatorSystemActionRateLimitException $ex) { 272 276 $require_captcha = true; 273 277 $captcha_valid = AphrontFormRecaptchaControl::processCaptcha($request); 274 278 } 275 - } 276 - 277 - // If the user has submitted quite a few failed login attempts recently, 278 - // give them a hard limit. 279 - if (count($failed_attempts) > $hard_limit) { 280 - $guidance = array(); 281 - 282 - $guidance[] = pht( 283 - 'Your remote address has failed too many login attempts recently. '. 284 - 'Wait a few minutes before trying again.'); 285 - 286 - $guidance[] = pht( 287 - 'If you are unable to log in to your account, you can '. 288 - '[[ /login/email | send a reset link to your email address ]].'); 289 - 290 - $guidance = implode("\n\n", $guidance); 291 - 292 - $dialog = $controller->newDialog() 293 - ->setTitle(pht('Too Many Login Attempts')) 294 - ->appendChild(new PHUIRemarkupView($viewer, $guidance)) 295 - ->addCancelButton('/auth/start/', pht('Wait Patiently')); 296 - 297 - return array(null, $dialog); 298 279 } 299 280 300 281 $response = null;

Configure Feed

Configure Feed