Add a rate limit to requesting account recovery links from a given remote address
Summary:
Depends on D20666. Ref T13343. In D20666, I limited the rate at which a given user account can be sent account recovery links.
Here, add a companion limit to the rate at which a given remote address may request recovery of any account. This limit is a little more forgiving since reasonable users may plausibly try multiple variations of several email addresses, make typos, etc. The goal is just to hinder attackers from fishing for every address under the sun on installs with no CAPTCHA configured and no broad-spectrum VPN-style access controls.
Test Plan: {F6607846}
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13343
Differential Revision: https://secure.phabricator.com/D20667
This commit is contained in:
@@ -2429,6 +2429,7 @@ phutil_register_library_map(array(
|
|||||||
'PhabricatorAuthTemporaryTokenTypeModule' => 'applications/auth/tokentype/PhabricatorAuthTemporaryTokenTypeModule.php',
|
'PhabricatorAuthTemporaryTokenTypeModule' => 'applications/auth/tokentype/PhabricatorAuthTemporaryTokenTypeModule.php',
|
||||||
'PhabricatorAuthTerminateSessionController' => 'applications/auth/controller/PhabricatorAuthTerminateSessionController.php',
|
'PhabricatorAuthTerminateSessionController' => 'applications/auth/controller/PhabricatorAuthTerminateSessionController.php',
|
||||||
'PhabricatorAuthTestSMSAction' => 'applications/auth/action/PhabricatorAuthTestSMSAction.php',
|
'PhabricatorAuthTestSMSAction' => 'applications/auth/action/PhabricatorAuthTestSMSAction.php',
|
||||||
|
'PhabricatorAuthTryEmailLoginAction' => 'applications/auth/action/PhabricatorAuthTryEmailLoginAction.php',
|
||||||
'PhabricatorAuthTryFactorAction' => 'applications/auth/action/PhabricatorAuthTryFactorAction.php',
|
'PhabricatorAuthTryFactorAction' => 'applications/auth/action/PhabricatorAuthTryFactorAction.php',
|
||||||
'PhabricatorAuthUnlinkController' => 'applications/auth/controller/PhabricatorAuthUnlinkController.php',
|
'PhabricatorAuthUnlinkController' => 'applications/auth/controller/PhabricatorAuthUnlinkController.php',
|
||||||
'PhabricatorAuthValidateController' => 'applications/auth/controller/PhabricatorAuthValidateController.php',
|
'PhabricatorAuthValidateController' => 'applications/auth/controller/PhabricatorAuthValidateController.php',
|
||||||
@@ -8424,6 +8425,7 @@ phutil_register_library_map(array(
|
|||||||
'PhabricatorAuthTemporaryTokenTypeModule' => 'PhabricatorConfigModule',
|
'PhabricatorAuthTemporaryTokenTypeModule' => 'PhabricatorConfigModule',
|
||||||
'PhabricatorAuthTerminateSessionController' => 'PhabricatorAuthController',
|
'PhabricatorAuthTerminateSessionController' => 'PhabricatorAuthController',
|
||||||
'PhabricatorAuthTestSMSAction' => 'PhabricatorSystemAction',
|
'PhabricatorAuthTestSMSAction' => 'PhabricatorSystemAction',
|
||||||
|
'PhabricatorAuthTryEmailLoginAction' => 'PhabricatorSystemAction',
|
||||||
'PhabricatorAuthTryFactorAction' => 'PhabricatorSystemAction',
|
'PhabricatorAuthTryFactorAction' => 'PhabricatorSystemAction',
|
||||||
'PhabricatorAuthUnlinkController' => 'PhabricatorAuthController',
|
'PhabricatorAuthUnlinkController' => 'PhabricatorAuthController',
|
||||||
'PhabricatorAuthValidateController' => 'PhabricatorAuthController',
|
'PhabricatorAuthValidateController' => 'PhabricatorAuthController',
|
||||||
|
|||||||
@@ -0,0 +1,22 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
final class PhabricatorAuthTryEmailLoginAction
|
||||||
|
extends PhabricatorSystemAction {
|
||||||
|
|
||||||
|
const TYPECONST = 'mail.try-login';
|
||||||
|
|
||||||
|
public function getActionConstant() {
|
||||||
|
return self::TYPECONST;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getScoreThreshold() {
|
||||||
|
return 20 / phutil_units('1 hour in seconds');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getLimitExplanation() {
|
||||||
|
return pht(
|
||||||
|
'You have made too many account recovery requests in a short period '.
|
||||||
|
'of time.');
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
||||||
@@ -53,6 +53,14 @@ final class PhabricatorEmailLoginController
|
|||||||
// it expensive to fish for valid email addresses while giving the user
|
// it expensive to fish for valid email addresses while giving the user
|
||||||
// a better error if they goof their email.
|
// a better error if they goof their email.
|
||||||
|
|
||||||
|
$action_actor = PhabricatorSystemActionEngine::newActorFromRequest(
|
||||||
|
$request);
|
||||||
|
|
||||||
|
PhabricatorSystemActionEngine::willTakeAction(
|
||||||
|
array($action_actor),
|
||||||
|
new PhabricatorAuthTryEmailLoginAction(),
|
||||||
|
1);
|
||||||
|
|
||||||
$target_email = id(new PhabricatorUserEmail())->loadOneWhere(
|
$target_email = id(new PhabricatorUserEmail())->loadOneWhere(
|
||||||
'address = %s',
|
'address = %s',
|
||||||
$v_email);
|
$v_email);
|
||||||
|
|||||||
@@ -198,4 +198,8 @@ final class PhabricatorSystemActionEngine extends Phobject {
|
|||||||
return $conn_w->getAffectedRows();
|
return $conn_w->getAffectedRows();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public static function newActorFromRequest(AphrontRequest $request) {
|
||||||
|
return $request->getRemoteAddress();
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user