Add bin/auth unlimit and clean up a TODO

Summary:
I stumbled across this TODO and was worried that there was a glaring hole in MFA that I'd somehow forgotten about, but the TODO is just out of date.

These actions are rate limited properly by `PhabricatorAuthTryFactorAction`, which permits a maximum of 10 actions per hour.

  - Remove the TODO.
  - Add `bin/auth unlimit` to make it easier to reset rate limits if someone needs to do that for whatever reason.

Test Plan:
  - Tried to brute force through MFA.
  - Got rate limited properly after 10 failures.
  - Reset rate limit with `bin/auth unlimit`.
  - Saw the expected number of actions clear.

{F805288}

Reviewers: chad

Reviewed By: chad

Subscribers: joshuaspence

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

src/__phutil_library_map__.php

@@ -1621,6 +1621,7 @@ phutil_register_library_map(array(
     'PhabricatorAuthManagementRefreshWorkflow' => 'applications/auth/management/PhabricatorAuthManagementRefreshWorkflow.php',
     'PhabricatorAuthManagementStripWorkflow' => 'applications/auth/management/PhabricatorAuthManagementStripWorkflow.php',
     'PhabricatorAuthManagementTrustOAuthClientWorkflow' => 'applications/auth/management/PhabricatorAuthManagementTrustOAuthClientWorkflow.php',
+    'PhabricatorAuthManagementUnlimitWorkflow' => 'applications/auth/management/PhabricatorAuthManagementUnlimitWorkflow.php',
     'PhabricatorAuthManagementUntrustOAuthClientWorkflow' => 'applications/auth/management/PhabricatorAuthManagementUntrustOAuthClientWorkflow.php',
     'PhabricatorAuthManagementVerifyWorkflow' => 'applications/auth/management/PhabricatorAuthManagementVerifyWorkflow.php',
     'PhabricatorAuthManagementWorkflow' => 'applications/auth/management/PhabricatorAuthManagementWorkflow.php',
@@ -5472,6 +5473,7 @@ phutil_register_library_map(array(
     'PhabricatorAuthManagementRefreshWorkflow' => 'PhabricatorAuthManagementWorkflow',
     'PhabricatorAuthManagementStripWorkflow' => 'PhabricatorAuthManagementWorkflow',
     'PhabricatorAuthManagementTrustOAuthClientWorkflow' => 'PhabricatorAuthManagementWorkflow',
+    'PhabricatorAuthManagementUnlimitWorkflow' => 'PhabricatorAuthManagementWorkflow',
     'PhabricatorAuthManagementUntrustOAuthClientWorkflow' => 'PhabricatorAuthManagementWorkflow',
     'PhabricatorAuthManagementVerifyWorkflow' => 'PhabricatorAuthManagementWorkflow',
     'PhabricatorAuthManagementWorkflow' => 'PhabricatorManagementWorkflow',

src/applications/auth/factor/PhabricatorTOTPAuthFactor.php

@@ -192,9 +192,6 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor {
     PhutilOpaqueEnvelope $key,
     $code) {
 
-    // TODO: This should use rate limiting to prevent multiple attempts in a
-    // short period of time.
-
     $now = (int)(time() / 30);
 
     // Allow the user to enter a code a few minutes away on either side, in

src/applications/auth/management/PhabricatorAuthManagementUnlimitWorkflow.php

@@ -0,0 +1,67 @@
+<?php
+
+final class PhabricatorAuthManagementUnlimitWorkflow
+  extends PhabricatorAuthManagementWorkflow {
+
+  protected function didConstruct() {
+    $this
+      ->setName('unlimit')
+      ->setExamples('**unlimit** --user __username__ --all')
+      ->setSynopsis(
+        pht(
+          'Reset action counters so a user can continue taking '.
+          'rate-limited actions.'))
+      ->setArguments(
+        array(
+          array(
+            'name' => 'user',
+            'param' => 'username',
+            'help' => pht('Reset action counters for this user.'),
+          ),
+          array(
+            'name' => 'all',
+            'help' => pht('Reset all counters.'),
+          ),
+        ));
+  }
+
+  public function execute(PhutilArgumentParser $args) {
+    $username = $args->getArg('user');
+    if (!strlen($username)) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'Use %s to choose a user to reset actions for.', '--user'));
+    }
+
+    $user = id(new PhabricatorPeopleQuery())
+      ->setViewer($this->getViewer())
+      ->withUsernames(array($username))
+      ->executeOne();
+    if (!$user) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'No user exists with username "%s".',
+          $username));
+    }
+
+    $all = $args->getArg('all');
+    if (!$all) {
+      // TODO: Eventually, let users reset specific actions. For now, we
+      // require `--all` so that usage won't change when you can reset in a
+      // more tailored way.
+      throw new PhutilArgumentUsageException(
+        pht(
+          'Specify %s to reset all action counters.', '--all'));
+    }
+
+    $count = PhabricatorSystemActionEngine::resetActions(
+      array(
+        $user->getPHID(),
+      ));
+
+    echo pht('Reset %s action(s).', new PhutilNumber($count))."\n";
+
+    return 0;
+  }
+
+}

src/applications/system/engine/PhabricatorSystemActionEngine.php

@@ -167,4 +167,35 @@ final class PhabricatorSystemActionEngine extends Phobject {
     return phutil_units('1 hour in seconds');
   }
 
+
+  /**
+   * Reset all action counts for actions taken by some set of actors in the
+   * previous action window.
+   *
+   * @param list<string> Actors to reset counts for.
+   * @return int Number of actions cleared.
+   */
+  public static function resetActions(array $actors) {
+    $log = new PhabricatorSystemActionLog();
+    $conn_w = $log->establishConnection('w');
+
+    $now = PhabricatorTime::getNow();
+
+    $hashes = array();
+    foreach ($actors as $actor) {
+      $hashes[] = PhabricatorHash::digestForIndex($actor);
+    }
+
+    queryfx(
+      $conn_w,
+      'DELETE FROM %T
+        WHERE actorHash IN (%Ls) AND epoch BETWEEN %d AND %d',
+      $log->getTableName(),
+      $hashes,
+      $now - self::getWindow(),
+      $now);
+
+    return $conn_w->getAffectedRows();
+  }
+
 }