Implement most of the administrative UI for approval queues

Summary:
Nothing fancy here, just:

  - UI to show users needing approval.
  - "Approve" and "Disable" actions.
  - Send "Approved" email on approve.
  - "Approve" edit + log operations.
  - "Wait for Approval" state for users who need approval.

There's still no natural way for users to end up not-approved -- you have to write directly to the database.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

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

src/__phutil_library_map__.php

@@ -1082,6 +1082,7 @@ phutil_register_library_map(array(
     'PhabricatorAuthManagementRecoverWorkflow' => 'applications/auth/management/PhabricatorAuthManagementRecoverWorkflow.php',
     'PhabricatorAuthManagementRefreshWorkflow' => 'applications/auth/management/PhabricatorAuthManagementRefreshWorkflow.php',
     'PhabricatorAuthManagementWorkflow' => 'applications/auth/management/PhabricatorAuthManagementWorkflow.php',
+    'PhabricatorAuthNeedsApprovalController' => 'applications/auth/controller/PhabricatorAuthNeedsApprovalController.php',
     'PhabricatorAuthNewController' => 'applications/auth/controller/config/PhabricatorAuthNewController.php',
     'PhabricatorAuthOldOAuthRedirectController' => 'applications/auth/controller/PhabricatorAuthOldOAuthRedirectController.php',
     'PhabricatorAuthProvider' => 'applications/auth/provider/PhabricatorAuthProvider.php',
@@ -1600,7 +1601,9 @@ phutil_register_library_map(array(
     'PhabricatorPasteTransactionComment' => 'applications/paste/storage/PhabricatorPasteTransactionComment.php',
     'PhabricatorPasteTransactionQuery' => 'applications/paste/query/PhabricatorPasteTransactionQuery.php',
     'PhabricatorPasteViewController' => 'applications/paste/controller/PhabricatorPasteViewController.php',
+    'PhabricatorPeopleApproveController' => 'applications/people/controller/PhabricatorPeopleApproveController.php',
     'PhabricatorPeopleController' => 'applications/people/controller/PhabricatorPeopleController.php',
+    'PhabricatorPeopleDisableController' => 'applications/people/controller/PhabricatorPeopleDisableController.php',
     'PhabricatorPeopleEditController' => 'applications/people/controller/PhabricatorPeopleEditController.php',
     'PhabricatorPeopleHovercardEventListener' => 'applications/people/event/PhabricatorPeopleHovercardEventListener.php',
     'PhabricatorPeopleLdapController' => 'applications/people/controller/PhabricatorPeopleLdapController.php',
@@ -3445,6 +3448,7 @@ phutil_register_library_map(array(
     'PhabricatorAuthManagementRecoverWorkflow' => 'PhabricatorAuthManagementWorkflow',
     'PhabricatorAuthManagementRefreshWorkflow' => 'PhabricatorAuthManagementWorkflow',
     'PhabricatorAuthManagementWorkflow' => 'PhutilArgumentWorkflow',
+    'PhabricatorAuthNeedsApprovalController' => 'PhabricatorAuthController',
     'PhabricatorAuthNewController' => 'PhabricatorAuthProviderConfigController',
     'PhabricatorAuthOldOAuthRedirectController' => 'PhabricatorAuthController',
     'PhabricatorAuthProviderConfig' =>
@@ -4011,7 +4015,9 @@ phutil_register_library_map(array(
     'PhabricatorPasteTransactionComment' => 'PhabricatorApplicationTransactionComment',
     'PhabricatorPasteTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'PhabricatorPasteViewController' => 'PhabricatorPasteController',
+    'PhabricatorPeopleApproveController' => 'PhabricatorPeopleController',
     'PhabricatorPeopleController' => 'PhabricatorController',
+    'PhabricatorPeopleDisableController' => 'PhabricatorPeopleController',
     'PhabricatorPeopleEditController' => 'PhabricatorPeopleController',
     'PhabricatorPeopleHovercardEventListener' => 'PhabricatorEventListener',
     'PhabricatorPeopleLdapController' => 'PhabricatorPeopleController',

src/applications/auth/controller/PhabricatorAuthNeedsApprovalController.php

@@ -0,0 +1,36 @@
+<?php
+
+final class PhabricatorAuthNeedsApprovalController
+  extends PhabricatorAuthController {
+
+  public function shouldRequireLogin() {
+    return false;
+  }
+
+  public function shouldRequireEmailVerification() {
+    return false;
+  }
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    $user = $request->getUser();
+
+    $wait_for_approval = pht(
+      "Your account has been created, but needs to be approved by an ".
+      "administrator. You'll receive an email once your account is approved.");
+
+    $dialog = id(new AphrontDialogView())
+      ->setUser($user)
+      ->setTitle(pht('Wait for Approval'))
+      ->appendChild($wait_for_approval)
+      ->addCancelButton('/', pht('Wait Patiently'));
+
+    return $this->buildApplicationPage(
+      $dialog,
+      array(
+        'title' => pht('Wait For Approval'),
+        'device' => true,
+      ));
+  }
+
+}

src/applications/base/controller/PhabricatorController.php

@@ -120,6 +120,11 @@ abstract class PhabricatorController extends AphrontController {
             return $this->delegateToController($controller);
           }
         }
+        if (!$user->getIsApproved()) {
+          $controller = new PhabricatorAuthNeedsApprovalController($request);
+          $this->setCurrentApplication($auth_application);
+          return $this->delegateToController($controller);
+        }
       }
 
       // If the user doesn't have access to the application, don't let them use

src/applications/people/application/PhabricatorApplicationPeople.php

@@ -41,6 +41,8 @@ final class PhabricatorApplicationPeople extends PhabricatorApplication {
       '/people/' => array(
         '(query/(?P<key>[^/]+)/)?' => 'PhabricatorPeopleListController',
         'logs/' => 'PhabricatorPeopleLogsController',
+        'approve/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleApproveController',
+        'disable/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleDisableController',
         'edit/(?:(?P<id>[1-9]\d*)/(?:(?P<view>\w+)/)?)?'
           => 'PhabricatorPeopleEditController',
         'ldap/' => 'PhabricatorPeopleLdapController',

src/applications/people/controller/PhabricatorPeopleApproveController.php

@@ -0,0 +1,66 @@
+<?php
+
+final class PhabricatorPeopleApproveController
+  extends PhabricatorPeopleController {
+
+  private $id;
+
+  public function willProcessRequest(array $data) {
+    $this->id = idx($data, 'id');
+  }
+
+  public function processRequest() {
+
+    $request = $this->getRequest();
+    $admin = $request->getUser();
+
+    $user = id(new PhabricatorPeopleQuery())
+      ->setViewer($admin)
+      ->withIDs(array($this->id))
+      ->executeOne();
+    if (!$user) {
+      return new Aphront404Response();
+    }
+
+    $done_uri = $this->getApplicationURI('query/approval/');
+
+    if ($request->isFormPost()) {
+      id(new PhabricatorUserEditor())
+        ->setActor($admin)
+        ->approveUser($user, true);
+
+      $title = pht(
+        'Phabricator Account "%s" Approved',
+        $user->getUsername(),
+        $admin->getUsername());
+
+      $body = pht(
+        "Your Phabricator account (%s) has been approved by %s. You can ".
+        "login here:\n\n  %s\n\n",
+        $user->getUsername(),
+        $admin->getUsername(),
+        PhabricatorEnv::getProductionURI('/'));
+
+      $mail = id(new PhabricatorMetaMTAMail())
+        ->addTos(array($user->getPHID()))
+        ->addCCs(array($admin->getPHID()))
+        ->setSubject('[Phabricator] '.$title)
+        ->setBody($body)
+        ->saveAndSend();
+
+      return id(new AphrontRedirectResponse())->setURI($done_uri);
+    }
+
+    $dialog = id(new AphrontDialogView())
+      ->setUser($admin)
+      ->setTitle(pht('Confirm Approval'))
+      ->appendChild(
+        pht(
+          'Allow %s to access this Phabricator install?',
+          phutil_tag('strong', array(), $user->getUsername())))
+      ->addCancelButton($done_uri)
+      ->addSubmitButton(pht('Approve Account'));
+
+    return id(new AphrontDialogResponse())->setDialog($dialog);
+  }
+}

src/applications/people/controller/PhabricatorPeopleDisableController.php

@@ -0,0 +1,48 @@
+<?php
+
+final class PhabricatorPeopleDisableController
+  extends PhabricatorPeopleController {
+
+  private $id;
+
+  public function willProcessRequest(array $data) {
+    $this->id = idx($data, 'id');
+  }
+
+  public function processRequest() {
+
+    $request = $this->getRequest();
+    $admin = $request->getUser();
+
+    $user = id(new PhabricatorPeopleQuery())
+      ->setViewer($admin)
+      ->withIDs(array($this->id))
+      ->executeOne();
+    if (!$user) {
+      return new Aphront404Response();
+    }
+
+    $done_uri = $this->getApplicationURI('query/approval/');
+
+    if ($request->isFormPost()) {
+      id(new PhabricatorUserEditor())
+        ->setActor($admin)
+        ->disableUser($user, true);
+
+      return id(new AphrontRedirectResponse())->setURI($done_uri);
+    }
+
+    $dialog = id(new AphrontDialogView())
+      ->setUser($admin)
+      ->setTitle(pht('Confirm Disable'))
+      ->appendChild(
+        pht(
+          'Disable %s? They will no longer be able to access Phabricator or '.
+          'receive email.',
+          phutil_tag('strong', array(), $user->getUsername())))
+      ->addCancelButton($done_uri)
+      ->addSubmitButton(pht('Disable Account'));
+
+    return id(new AphrontDialogResponse())->setDialog($dialog);
+  }
+}

src/applications/people/controller/PhabricatorPeopleListController.php

@@ -38,6 +38,8 @@ final class PhabricatorPeopleListController extends PhabricatorPeopleController
 
     $list = new PHUIObjectItemListView();
 
+    $is_approval = ($query->getQueryKey() == 'approval');
+
     foreach ($users as $user) {
       $primary_email = $user->loadPrimaryEmail();
       if ($primary_email && $primary_email->getIsVerified()) {
@@ -61,8 +63,10 @@ final class PhabricatorPeopleListController extends PhabricatorPeopleController
         $item->addIcon('disable', pht('Disabled'));
       }
 
-      if (!$user->getIsApproved()) {
+      if (!$is_approval) {
-        $item->addIcon('raise-priority', pht('Not Approved'));
+        if (!$user->getIsApproved()) {
+          $item->addIcon('perflab-grey', pht('Needs Approval'));
+        }
       }
 
       if ($user->getIsAdmin()) {
@@ -74,11 +78,26 @@ final class PhabricatorPeopleListController extends PhabricatorPeopleController
       }
 
       if ($viewer->getIsAdmin()) {
-        $uid = $user->getID();
+        $user_id = $user->getID();
-        $item->addAction(
+        if ($is_approval) {
-          id(new PHUIListItemView())
+          $item->addAction(
-            ->setIcon('edit')
+            id(new PHUIListItemView())
-            ->setHref($this->getApplicationURI('edit/'.$uid.'/')));
+              ->setIcon('disable')
+              ->setName(pht('Disable'))
+              ->setWorkflow(true)
+              ->setHref($this->getApplicationURI('disable/'.$user_id.'/')));
+          $item->addAction(
+            id(new PHUIListItemView())
+              ->setIcon('like')
+              ->setName(pht('Approve'))
+              ->setWorkflow(true)
+              ->setHref($this->getApplicationURI('approve/'.$user_id.'/')));
+        } else {
+          $item->addAction(
+            id(new PHUIListItemView())
+              ->setIcon('edit')
+              ->setHref($this->getApplicationURI('edit/'.$user_id.'/')));
+        }
       }
 
       $list->addItem($item);

src/applications/people/editor/PhabricatorUserEditor.php

@@ -294,6 +294,44 @@ final class PhabricatorUserEditor extends PhabricatorEditor {
   }
 
 
+  /**
+   * @task role
+   */
+  public function approveUser(PhabricatorUser $user, $approve) {
+    $actor = $this->requireActor();
+
+    if (!$user->getID()) {
+      throw new Exception("User has not been created yet!");
+    }
+
+    $user->openTransaction();
+      $user->beginWriteLocking();
+
+        $user->reload();
+        if ($user->getIsApproved() == $approve) {
+          $user->endWriteLocking();
+          $user->killTransaction();
+          return $this;
+        }
+
+        $log = PhabricatorUserLog::newLog(
+          $actor,
+          $user,
+          PhabricatorUserLog::ACTION_APPROVE);
+        $log->setOldValue($user->getIsApproved());
+        $log->setNewValue($approve);
+
+        $user->setIsApproved($approve);
+        $user->save();
+
+        $log->save();
+
+      $user->endWriteLocking();
+    $user->saveTransaction();
+
+    return $this;
+  }
+
   /**
    * @task role
    */

src/applications/people/query/PhabricatorPeopleQuery.php

@@ -13,6 +13,7 @@ final class PhabricatorPeopleQuery
   private $isAdmin;
   private $isSystemAgent;
   private $isDisabled;
+  private $isApproved;
   private $nameLike;
 
   private $needPrimaryEmail;
@@ -70,6 +71,11 @@ final class PhabricatorPeopleQuery
     return $this;
   }
 
+  public function withIsApproved($approved) {
+    $this->isApproved = $approved;
+    return $this;
+  }
+
   public function withNameLike($like) {
     $this->nameLike = $like;
     return $this;
@@ -249,10 +255,18 @@ final class PhabricatorPeopleQuery
         'user.isAdmin = 1');
     }
 
-    if ($this->isDisabled) {
+    if ($this->isDisabled !== null) {
       $where[] = qsprintf(
         $conn_r,
-        'user.isDisabled = 1');
+        'user.isDisabled = %d',
+        (int)$this->isDisabled);
+    }
+
+    if ($this->isApproved !== null) {
+      $where[] = qsprintf(
+        $conn_r,
+        'user.isApproved = %d',
+        (int)$this->isApproved);
     }
 
     if ($this->isSystemAgent) {

src/applications/people/query/PhabricatorPeopleSearchEngine.php

@@ -15,6 +15,7 @@ final class PhabricatorPeopleSearchEngine
     $saved->setParameter('isAdmin', $request->getStr('isAdmin'));
     $saved->setParameter('isDisabled', $request->getStr('isDisabled'));
     $saved->setParameter('isSystemAgent', $request->getStr('isSystemAgent'));
+    $saved->setParameter('needsApproval', $request->getStr('needsApproval'));
     $saved->setParameter('createdStart', $request->getStr('createdStart'));
     $saved->setParameter('createdEnd', $request->getStr('createdEnd'));
 
@@ -40,6 +41,8 @@ final class PhabricatorPeopleSearchEngine
     $is_admin = $saved->getParameter('isAdmin');
     $is_disabled = $saved->getParameter('isDisabled');
     $is_system_agent = $saved->getParameter('isSystemAgent');
+    $needs_approval = $saved->getParameter('needsApproval');
+    $no_disabled = $saved->getParameter('noDisabled');
 
     if ($is_admin) {
       $query->withIsAdmin(true);
@@ -47,12 +50,18 @@ final class PhabricatorPeopleSearchEngine
 
     if ($is_disabled) {
       $query->withIsDisabled(true);
+    } else if ($no_disabled) {
+      $query->withIsDisabled(false);
     }
 
     if ($is_system_agent) {
       $query->withIsSystemAgent(true);
     }
 
+    if ($needs_approval) {
+      $query->withIsApproved(false);
+    }
+
     $start = $this->parseDateTime($saved->getParameter('createdStart'));
     $end = $this->parseDateTime($saved->getParameter('createdEnd'));
 
@@ -79,6 +88,7 @@ final class PhabricatorPeopleSearchEngine
     $is_admin = $saved->getParameter('isAdmin');
     $is_disabled = $saved->getParameter('isDisabled');
     $is_system_agent = $saved->getParameter('isSystemAgent');
+    $needs_approval = $saved->getParameter('needsApproval');
 
     $form
       ->appendChild(
@@ -108,7 +118,12 @@ final class PhabricatorPeopleSearchEngine
             'isSystemAgent',
             1,
             pht('Show only System Agents.'),
-            $is_system_agent));
+            $is_system_agent)
+          ->addCheckbox(
+            'needsApproval',
+            1,
+            pht('Show only users who need approval.'),
+            $needs_approval));
 
     $this->appendCustomFieldsToForm($form, $saved);
 
@@ -130,6 +145,11 @@ final class PhabricatorPeopleSearchEngine
       'all' => pht('All'),
     );
 
+    $viewer = $this->requireViewer();
+    if ($viewer->getIsAdmin()) {
+      $names['approval'] = pht('Approval Queue');
+    }
+
     return $names;
   }
 
@@ -140,6 +160,10 @@ final class PhabricatorPeopleSearchEngine
     switch ($query_key) {
       case 'all':
         return $query;
+      case 'approval':
+        return $query
+          ->setParameter('needsApproval', true)
+          ->setParameter('noDisabled', true);
     }
 
     return parent::buildSavedQueryFromBuiltin($query_key);

src/applications/people/storage/PhabricatorUserLog.php

@@ -13,6 +13,7 @@ final class PhabricatorUserLog extends PhabricatorUserDAO {
   const ACTION_ADMIN          = 'admin';
   const ACTION_SYSTEM_AGENT   = 'system-agent';
   const ACTION_DISABLE        = 'disable';
+  const ACTION_APPROVE        = 'approve';
   const ACTION_DELETE         = 'delete';
 
   const ACTION_CONDUIT_CERTIFICATE = 'conduit-cert';