From e090b32c7528498fa5e742efeb797f18b3acfdc2 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 19 Jul 2019 09:56:48 -0700
Subject: [PATCH] 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
---
 src/__phutil_library_map__.php                |  2 ++
 .../PhabricatorAuthTryEmailLoginAction.php    | 22 +++++++++++++++++++
 .../PhabricatorEmailLoginController.php       |  8 +++++++
 .../engine/PhabricatorSystemActionEngine.php  |  4 ++++
 4 files changed, 36 insertions(+)
 create mode 100644 src/applications/auth/action/PhabricatorAuthTryEmailLoginAction.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 3120eb01b6..6ccaf1ec97 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2429,6 +2429,7 @@ phutil_register_library_map(array(
     'PhabricatorAuthTemporaryTokenTypeModule' => 'applications/auth/tokentype/PhabricatorAuthTemporaryTokenTypeModule.php',
     'PhabricatorAuthTerminateSessionController' => 'applications/auth/controller/PhabricatorAuthTerminateSessionController.php',
     'PhabricatorAuthTestSMSAction' => 'applications/auth/action/PhabricatorAuthTestSMSAction.php',
+    'PhabricatorAuthTryEmailLoginAction' => 'applications/auth/action/PhabricatorAuthTryEmailLoginAction.php',
     'PhabricatorAuthTryFactorAction' => 'applications/auth/action/PhabricatorAuthTryFactorAction.php',
     'PhabricatorAuthUnlinkController' => 'applications/auth/controller/PhabricatorAuthUnlinkController.php',
     'PhabricatorAuthValidateController' => 'applications/auth/controller/PhabricatorAuthValidateController.php',
@@ -8424,6 +8425,7 @@ phutil_register_library_map(array(
     'PhabricatorAuthTemporaryTokenTypeModule' => 'PhabricatorConfigModule',
     'PhabricatorAuthTerminateSessionController' => 'PhabricatorAuthController',
     'PhabricatorAuthTestSMSAction' => 'PhabricatorSystemAction',
+    'PhabricatorAuthTryEmailLoginAction' => 'PhabricatorSystemAction',
     'PhabricatorAuthTryFactorAction' => 'PhabricatorSystemAction',
     'PhabricatorAuthUnlinkController' => 'PhabricatorAuthController',
     'PhabricatorAuthValidateController' => 'PhabricatorAuthController',
diff --git a/src/applications/auth/action/PhabricatorAuthTryEmailLoginAction.php b/src/applications/auth/action/PhabricatorAuthTryEmailLoginAction.php
new file mode 100644
index 0000000000..001358e3f6
--- /dev/null
+++ b/src/applications/auth/action/PhabricatorAuthTryEmailLoginAction.php
@@ -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.');
+  }
+
+}
diff --git a/src/applications/auth/controller/PhabricatorEmailLoginController.php b/src/applications/auth/controller/PhabricatorEmailLoginController.php
index 234829634b..492526edf5 100644
--- a/src/applications/auth/controller/PhabricatorEmailLoginController.php
+++ b/src/applications/auth/controller/PhabricatorEmailLoginController.php
@@ -53,6 +53,14 @@ final class PhabricatorEmailLoginController
         // it expensive to fish for valid email addresses while giving the user
         // 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(
           'address = %s',
           $v_email);
diff --git a/src/applications/system/engine/PhabricatorSystemActionEngine.php b/src/applications/system/engine/PhabricatorSystemActionEngine.php
index 6b8352a29e..c097fa04a4 100644
--- a/src/applications/system/engine/PhabricatorSystemActionEngine.php
+++ b/src/applications/system/engine/PhabricatorSystemActionEngine.php
@@ -198,4 +198,8 @@ final class PhabricatorSystemActionEngine extends Phobject {
     return $conn_w->getAffectedRows();
   }
 
+  public static function newActorFromRequest(AphrontRequest $request) {
+    return $request->getRemoteAddress();
+  }
+
 }