Align web UI "Disable" and "Approve/Disapprove" flows with new "Can Disable Users" permission
Summary: Depends on D19606. Ref T13189. See PHI642. - Disabling/enabling users no longer requires admin. Now, you just need "Can Disable Users". - Update the UI to appropriately show the action in black or grey depending on what clicking the button will do. - For "Approve/Disapprove", fix a couple bugs, then let them go through without respect for "Can Disable Users". This is conceptually a different action, even though it ultimately sets the "Disabled" flag. Test Plan: - Disabled/enabled users from the web UI as various users, including a non-administrator with "Can Disable Users". - Hit permissions errors from the web UI as various users, including an administrator without "Can Disable Users". - Saw the "Disable/Enable" action activate properly based on whether clicking the button would actually work. - Disapproved a user without "Can Disable Users" permission, tried to re-disapprove a user. - Approved a user, tried to reapprove a user. Reviewers: amckinley Maniphest Tasks: T13189 Differential Revision: https://secure.phabricator.com/D19607
This commit is contained in:
		@@ -16,6 +16,13 @@ final class PhabricatorPeopleApproveController
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
    $done_uri = $this->getApplicationURI('query/approval/');
 | 
					    $done_uri = $this->getApplicationURI('query/approval/');
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    if ($user->getIsApproved()) {
 | 
				
			||||||
 | 
					      return $this->newDialog()
 | 
				
			||||||
 | 
					        ->setTitle(pht('Already Approved'))
 | 
				
			||||||
 | 
					        ->appendChild(pht('This user has already been approved.'))
 | 
				
			||||||
 | 
					        ->addCancelButton($done_uri);
 | 
				
			||||||
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    if ($request->isFormPost()) {
 | 
					    if ($request->isFormPost()) {
 | 
				
			||||||
      id(new PhabricatorUserEditor())
 | 
					      id(new PhabricatorUserEditor())
 | 
				
			||||||
        ->setActor($viewer)
 | 
					        ->setActor($viewer)
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -3,10 +3,14 @@
 | 
				
			|||||||
final class PhabricatorPeopleDisableController
 | 
					final class PhabricatorPeopleDisableController
 | 
				
			||||||
  extends PhabricatorPeopleController {
 | 
					  extends PhabricatorPeopleController {
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  public function shouldRequireAdmin() {
 | 
				
			||||||
 | 
					    return false;
 | 
				
			||||||
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  public function handleRequest(AphrontRequest $request) {
 | 
					  public function handleRequest(AphrontRequest $request) {
 | 
				
			||||||
    $viewer = $this->getViewer();
 | 
					    $viewer = $this->getViewer();
 | 
				
			||||||
    $id = $request->getURIData('id');
 | 
					    $id = $request->getURIData('id');
 | 
				
			||||||
    $via = $request->getURIData('id');
 | 
					    $via = $request->getURIData('via');
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    $user = id(new PhabricatorPeopleQuery())
 | 
					    $user = id(new PhabricatorPeopleQuery())
 | 
				
			||||||
      ->setViewer($viewer)
 | 
					      ->setViewer($viewer)
 | 
				
			||||||
@@ -20,11 +24,36 @@ final class PhabricatorPeopleDisableController
 | 
				
			|||||||
    // on profiles and also via the "X" action on the approval queue. We do
 | 
					    // on profiles and also via the "X" action on the approval queue. We do
 | 
				
			||||||
    // things slightly differently depending on the context the actor is in.
 | 
					    // things slightly differently depending on the context the actor is in.
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    // In particular, disabling via "Disapprove" requires you be an
 | 
				
			||||||
 | 
					    // administrator (and bypasses the "Can Disable Users" permission).
 | 
				
			||||||
 | 
					    // Disabling via "Disable" requires the permission only.
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    $is_disapprove = ($via == 'disapprove');
 | 
					    $is_disapprove = ($via == 'disapprove');
 | 
				
			||||||
    if ($is_disapprove) {
 | 
					    if ($is_disapprove) {
 | 
				
			||||||
      $done_uri = $this->getApplicationURI('query/approval/');
 | 
					      $done_uri = $this->getApplicationURI('query/approval/');
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      if (!$viewer->getIsAdmin()) {
 | 
				
			||||||
 | 
					        return $this->newDialog()
 | 
				
			||||||
 | 
					          ->setTitle(pht('No Permission'))
 | 
				
			||||||
 | 
					          ->appendParagraph(pht('Only administrators can disapprove users.'))
 | 
				
			||||||
 | 
					          ->addCancelButton($done_uri);
 | 
				
			||||||
 | 
					      }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      if ($user->getIsApproved()) {
 | 
				
			||||||
 | 
					        return $this->newDialog()
 | 
				
			||||||
 | 
					          ->setTitle(pht('Already Approved'))
 | 
				
			||||||
 | 
					          ->appendParagraph(pht('This user has already been approved.'))
 | 
				
			||||||
 | 
					          ->addCancelButton($done_uri);
 | 
				
			||||||
 | 
					      }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      // On the "Disapprove" flow, bypass the "Can Disable Users" permission.
 | 
				
			||||||
 | 
					      $actor = PhabricatorUser::getOmnipotentUser();
 | 
				
			||||||
      $should_disable = true;
 | 
					      $should_disable = true;
 | 
				
			||||||
    } else {
 | 
					    } else {
 | 
				
			||||||
 | 
					      $this->requireApplicationCapability(
 | 
				
			||||||
 | 
					        PeopleDisableUsersCapability::CAPABILITY);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      $actor = $viewer;
 | 
				
			||||||
      $done_uri = $this->getApplicationURI("manage/{$id}/");
 | 
					      $done_uri = $this->getApplicationURI("manage/{$id}/");
 | 
				
			||||||
      $should_disable = !$user->getIsDisabled();
 | 
					      $should_disable = !$user->getIsDisabled();
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
@@ -46,7 +75,8 @@ final class PhabricatorPeopleDisableController
 | 
				
			|||||||
        ->setNewValue($should_disable);
 | 
					        ->setNewValue($should_disable);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      id(new PhabricatorUserTransactionEditor())
 | 
					      id(new PhabricatorUserTransactionEditor())
 | 
				
			||||||
        ->setActor($viewer)
 | 
					        ->setActor($actor)
 | 
				
			||||||
 | 
					        ->setActingAsPHID($viewer->getPHID())
 | 
				
			||||||
        ->setContentSourceFromRequest($request)
 | 
					        ->setContentSourceFromRequest($request)
 | 
				
			||||||
        ->setContinueOnMissingFields(true)
 | 
					        ->setContinueOnMissingFields(true)
 | 
				
			||||||
        ->setContinueOnNoEffect(true)
 | 
					        ->setContinueOnNoEffect(true)
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -75,11 +75,22 @@ final class PhabricatorPeopleProfileManageController
 | 
				
			|||||||
  private function buildCurtain(PhabricatorUser $user) {
 | 
					  private function buildCurtain(PhabricatorUser $user) {
 | 
				
			||||||
    $viewer = $this->getViewer();
 | 
					    $viewer = $this->getViewer();
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    $is_self = ($user->getPHID() === $viewer->getPHID());
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    $can_edit = PhabricatorPolicyFilter::hasCapability(
 | 
					    $can_edit = PhabricatorPolicyFilter::hasCapability(
 | 
				
			||||||
      $viewer,
 | 
					      $viewer,
 | 
				
			||||||
      $user,
 | 
					      $user,
 | 
				
			||||||
      PhabricatorPolicyCapability::CAN_EDIT);
 | 
					      PhabricatorPolicyCapability::CAN_EDIT);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    $is_admin = $viewer->getIsAdmin();
 | 
				
			||||||
 | 
					    $can_admin = ($is_admin && !$is_self);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    $has_disable = $this->hasApplicationCapability(
 | 
				
			||||||
 | 
					      PeopleDisableUsersCapability::CAPABILITY);
 | 
				
			||||||
 | 
					    $can_disable = ($has_disable && !$is_self);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    $can_welcome = ($is_admin && $user->canEstablishWebSessions());
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    $curtain = $this->newCurtainView($user);
 | 
					    $curtain = $this->newCurtainView($user);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    $curtain->addAction(
 | 
					    $curtain->addAction(
 | 
				
			||||||
@@ -114,10 +125,6 @@ final class PhabricatorPeopleProfileManageController
 | 
				
			|||||||
      $empower_name = pht('Make Administrator');
 | 
					      $empower_name = pht('Make Administrator');
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    $is_admin = $viewer->getIsAdmin();
 | 
					 | 
				
			||||||
    $is_self = ($user->getPHID() === $viewer->getPHID());
 | 
					 | 
				
			||||||
    $can_admin = ($is_admin && !$is_self);
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
    $curtain->addAction(
 | 
					    $curtain->addAction(
 | 
				
			||||||
      id(new PhabricatorActionView())
 | 
					      id(new PhabricatorActionView())
 | 
				
			||||||
        ->setIcon($empower_icon)
 | 
					        ->setIcon($empower_icon)
 | 
				
			||||||
@@ -146,7 +153,7 @@ final class PhabricatorPeopleProfileManageController
 | 
				
			|||||||
      id(new PhabricatorActionView())
 | 
					      id(new PhabricatorActionView())
 | 
				
			||||||
        ->setIcon($disable_icon)
 | 
					        ->setIcon($disable_icon)
 | 
				
			||||||
        ->setName($disable_name)
 | 
					        ->setName($disable_name)
 | 
				
			||||||
        ->setDisabled(!$can_admin)
 | 
					        ->setDisabled(!$can_disable)
 | 
				
			||||||
        ->setWorkflow(true)
 | 
					        ->setWorkflow(true)
 | 
				
			||||||
        ->setHref($this->getApplicationURI('disable/'.$user->getID().'/')));
 | 
					        ->setHref($this->getApplicationURI('disable/'.$user->getID().'/')));
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -158,8 +165,6 @@ final class PhabricatorPeopleProfileManageController
 | 
				
			|||||||
        ->setWorkflow(true)
 | 
					        ->setWorkflow(true)
 | 
				
			||||||
        ->setHref($this->getApplicationURI('delete/'.$user->getID().'/')));
 | 
					        ->setHref($this->getApplicationURI('delete/'.$user->getID().'/')));
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    $can_welcome = ($is_admin && $user->canEstablishWebSessions());
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
    $curtain->addAction(
 | 
					    $curtain->addAction(
 | 
				
			||||||
      id(new PhabricatorActionView())
 | 
					      id(new PhabricatorActionView())
 | 
				
			||||||
        ->setIcon('fa-envelope')
 | 
					        ->setIcon('fa-envelope')
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user