Add basic support for editing project policies

Summary: This case is unusually complicated because there are more rules than most objects will have. - Edits are either "joins", "leaves" or "other edits". - "Joins" require "can join" or "can edit". - "Leaves" don't require any policy. - "Other edits" require "can edit". - You can't edit away your ability to edit. - You //can// leave a project that you wouldn't be able to rejoin. Things I'm going to add: - Global log of policy changes. - `bin/policy` script for undoing policy changes. - Test coverage for these rules. Test Plan: Made various project visibility edits with various users, joined / left projects, etc. I'll add more complete coverage in the next diff. Reviewers: btrahan, vrana Reviewed By: btrahan CC: aran Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D3270
2024-09-20 01:08:50 +02:00 · 2012-08-15 10:44:58 -07:00 · 2012-08-15 10:44:58 -07:00 · 42461b5f06
commit 42461b5f06
parent 51c5a9b067
7 changed files with 253 additions and 24 deletions
--- a/src/applications/project/constants/PhabricatorProjectTransactionType.php
+++ b/src/applications/project/constants/PhabricatorProjectTransactionType.php
@ -22,5 +22,8 @@ final class PhabricatorProjectTransactionType
  const TYPE_NAME       = 'name';
  const TYPE_MEMBERS    = 'members';
  const TYPE_STATUS     = 'status';
+  const TYPE_CAN_VIEW   = 'canview';
+  const TYPE_CAN_EDIT   = 'canedit';
+  const TYPE_CAN_JOIN   = 'canjoin';

 }
--- a/src/applications/project/controller/PhabricatorProjectController.php
+++ b/src/applications/project/controller/PhabricatorProjectController.php
@ -1,7 +1,7 @@
 <?php

 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
@ -53,9 +53,28 @@ abstract class PhabricatorProjectController extends PhabricatorController {
    $nav_view->addFilter(null, 'Wiki '.$external_arrow, $phriction_uri);
    $nav_view->addFilter('people', 'People');
    $nav_view->addFilter('about', 'About');
+
+    $user = $this->getRequest()->getUser();
+    $can_edit = PhabricatorPolicyCapability::CAN_EDIT;
+
    $nav_view->addSpacer();
+    if (PhabricatorPolicyFilter::hasCapability($user, $project, $can_edit)) {
      $nav_view->addFilter('edit', "Edit Project\xE2\x80\xA6", $edit_uri);
      $nav_view->addFilter('members', "Edit Members\xE2\x80\xA6", $members_uri);
+    } else {
+      $nav_view->addFilter(
+        'edit',
+        "Edit Project\xE2\x80\xA6",
+        $edit_uri,
+        $relative = false,
+        'disabled');
+      $nav_view->addFilter(
+        'members',
+        "Edit Members\xE2\x80\xA6",
+        $members_uri,
+        $relative = false,
+        'disabled');
+    }

    return $nav_view;
  }
--- a/src/applications/project/controller/PhabricatorProjectMembersEditController.php
+++ b/src/applications/project/controller/PhabricatorProjectMembersEditController.php
@ -27,7 +27,15 @@ final class PhabricatorProjectMembersEditController
    $request = $this->getRequest();
    $user = $request->getUser();

-    $project = id(new PhabricatorProject())->load($this->id);
+    $project = id(new PhabricatorProjectQuery())
+      ->setViewer($user)
+      ->withIDs(array($this->id))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->executeOne();
    if (!$project) {
      return new Aphront404Response();
    }
--- a/src/applications/project/controller/PhabricatorProjectProfileController.php
+++ b/src/applications/project/controller/PhabricatorProjectProfileController.php
@ -31,24 +31,30 @@ final class PhabricatorProjectProfileController
    $request = $this->getRequest();
    $user = $request->getUser();

-    $project = id(new PhabricatorProject())->load($this->id);
+    $query = id(new PhabricatorProjectQuery())
+      ->setViewer($user)
+      ->withIDs(array($this->id));
+
+    if ($this->page == 'people') {
+      $query->needMembers(true);
+    }
+
+    $project = $query->executeOne();
    if (!$project) {
      return new Aphront404Response();
    }
+
    $profile = $project->loadProfile();
    if (!$profile) {
      $profile = new PhabricatorProjectProfile();
    }

    $picture = $profile->loadProfileImageURI();
-    $members = $project->loadMemberPHIDs();
-    $member_map = array_fill_keys($members, true);

    $nav_view = $this->buildLocalNavigation($project);

    $this->page = $nav_view->selectFilter($this->page, 'dashboard');

-
    require_celerity_resource('phabricator-profile-css');
    switch ($this->page) {
      case 'dashboard':
@ -88,7 +94,15 @@ final class PhabricatorProjectProfileController
    $header->setProfilePicture($picture);

    $action = null;
-    if (empty($member_map[$user->getPHID()])) {
+    if (!$project->isUserMember($user->getPHID())) {
+      $can_join = PhabricatorPolicyCapability::CAN_JOIN;
+
+      if (PhabricatorPolicyFilter::hasCapability($user, $project, $can_join)) {
+        $class = 'green';
+      } else {
+        $class = 'grey disabled';
+      }
+
      $action = phabricator_render_form(
        $user,
        array(
@ -98,7 +112,7 @@ final class PhabricatorProjectProfileController
        phutil_render_tag(
          'button',
          array(
-            'class' => 'green',
+            'class' => $class,
          ),
          'Join Project'));
    } else {
@ -172,7 +186,7 @@ final class PhabricatorProjectProfileController
    PhabricatorProject $project,
    PhabricatorProjectProfile $profile) {

-    $member_phids = $project->loadMemberPHIDs();
+    $member_phids = $project->getMemberPHIDs();
    $handles = id(new PhabricatorObjectHandleData($member_phids))
      ->loadHandles();

--- a/src/applications/project/controller/PhabricatorProjectProfileEditController.php
+++ b/src/applications/project/controller/PhabricatorProjectProfileEditController.php
@ -28,10 +28,19 @@ final class PhabricatorProjectProfileEditController
    $request = $this->getRequest();
    $user = $request->getUser();

-    $project = id(new PhabricatorProject())->load($this->id);
+    $project = id(new PhabricatorProjectQuery())
+      ->setViewer($user)
+      ->withIDs(array($this->id))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->executeOne();
    if (!$project) {
      return new Aphront404Response();
    }
+
    $profile = $project->loadProfile();
    if (empty($profile)) {
      $profile = new PhabricatorProjectProfile();
@ -62,6 +71,24 @@ final class PhabricatorProjectProfileEditController
        $xaction->setNewValue($request->getStr('status'));
        $xactions[] = $xaction;

+        $xaction = new PhabricatorProjectTransaction();
+        $xaction->setTransactionType(
+          PhabricatorProjectTransactionType::TYPE_CAN_VIEW);
+        $xaction->setNewValue($request->getStr('can_view'));
+        $xactions[] = $xaction;
+
+        $xaction = new PhabricatorProjectTransaction();
+        $xaction->setTransactionType(
+          PhabricatorProjectTransactionType::TYPE_CAN_EDIT);
+        $xaction->setNewValue($request->getStr('can_edit'));
+        $xactions[] = $xaction;
+
+        $xaction = new PhabricatorProjectTransaction();
+        $xaction->setTransactionType(
+          PhabricatorProjectTransactionType::TYPE_CAN_JOIN);
+        $xaction->setNewValue($request->getStr('can_join'));
+        $xactions[] = $xaction;
+
        $editor = new PhabricatorProjectEditor($project);
        $editor->setUser($user);
        $editor->applyTransactions($xactions);
@ -150,6 +177,31 @@ final class PhabricatorProjectProfileEditController
          ->setLabel('Blurb')
          ->setName('blurb')
          ->setValue($profile->getBlurb()))
+      ->appendChild(
+        '<p class="aphront-form-instructions">NOTE: Policy settings are not '.
+        'yet fully implemented. Some interfaces still ignore these settings, '.
+        'particularly "Visible To".</p>')
+      ->appendChild(
+        id(new AphrontFormPolicyControl())
+          ->setUser($user)
+          ->setName('can_view')
+          ->setCaption('Members can always view a project.')
+          ->setPolicyObject($project)
+          ->setCapability(PhabricatorPolicyCapability::CAN_VIEW))
+      ->appendChild(
+        id(new AphrontFormPolicyControl())
+          ->setUser($user)
+          ->setName('can_edit')
+          ->setPolicyObject($project)
+          ->setCapability(PhabricatorPolicyCapability::CAN_EDIT))
+      ->appendChild(
+        id(new AphrontFormPolicyControl())
+          ->setUser($user)
+          ->setName('can_join')
+          ->setCaption(
+            'Users who can edit a project can always join a project.')
+          ->setPolicyObject($project)
+          ->setCapability(PhabricatorPolicyCapability::CAN_JOIN))
      ->appendChild(
        id(new AphrontFormMarkupControl())
          ->setLabel('Profile Image')
--- a/src/applications/project/controller/PhabricatorProjectUpdateController.php
+++ b/src/applications/project/controller/PhabricatorProjectUpdateController.php
@ -31,18 +31,14 @@ final class PhabricatorProjectUpdateController
    $request = $this->getRequest();
    $user = $request->getUser();

-    $project = id(new PhabricatorProjectQuery())
-      ->setViewer($user)
-      ->needMembers(true)
-      ->withIDs(array($this->id))
-      ->executeOne();
-    if (!$project) {
-      return new Aphront404Response();
-    }
+    $capabilities = array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+    );

    $process_action = false;
    switch ($this->action) {
      case 'join':
+        $capabilities[] = PhabricatorPolicyCapability::CAN_JOIN;
        $process_action = $request->isFormPost();
        break;
      case 'leave':
@ -52,6 +48,16 @@ final class PhabricatorProjectUpdateController
        return new Aphront404Response();
    }

+    $project = id(new PhabricatorProjectQuery())
+      ->setViewer($user)
+      ->withIDs(array($this->id))
+      ->needMembers(true)
+      ->requireCapabilities($capabilities)
+      ->executeOne();
+    if (!$project) {
+      return new Aphront404Response();
+    }
+
    $project_uri = '/project/view/'.$project->getID().'/';

    if ($process_action) {
--- a/src/applications/project/editor/PhabricatorProjectEditor.php
+++ b/src/applications/project/editor/PhabricatorProjectEditor.php
@ -95,22 +95,54 @@ final class PhabricatorProjectEditor {
    }

    foreach ($transactions as $key => $xaction) {
-      $type = $xaction->getTransactionType();
-
      $this->setTransactionOldValue($project, $xaction);
-
      if (!$this->transactionHasEffect($xaction)) {
        unset($transactions[$key]);
        continue;
      }
+    }

-      $this->applyTransactionEffect($project, $xaction);
+    if (!$is_new) {
+      // You must be able to view a project in order to edit it in any capacity.
+      PhabricatorPolicyFilter::requireCapability(
+        $user,
+        $project,
+        PhabricatorPolicyCapability::CAN_VIEW);
+
+      $need_edit = false;
+      $need_join = false;
+      foreach ($transactions as $key => $xaction) {
+        if ($this->getTransactionRequiresEditCapability($xaction)) {
+          $need_edit = true;
+        }
+        if ($this->getTransactionRequiresJoinCapability($xaction)) {
+          $need_join = true;
+        }
+      }
+
+      if ($need_edit) {
+        PhabricatorPolicyFilter::requireCapability(
+          $user,
+          $project,
+          PhabricatorPolicyCapability::CAN_EDIT);
+      }
+
+      if ($need_join) {
+        PhabricatorPolicyFilter::requireCapability(
+          $user,
+          $project,
+          PhabricatorPolicyCapability::CAN_JOIN);
+      }
    }

    if (!$transactions) {
      return $this;
    }

+    foreach ($transactions as $xaction) {
+      $this->applyTransactionEffect($project, $xaction);
+    }
+
    try {
      $project->openTransaction();
        $project->save();
@ -203,6 +235,15 @@ final class PhabricatorProjectEditor {
        $new_value = array_values($new_value);
        $xaction->setNewValue($new_value);
        break;
+      case PhabricatorProjectTransactionType::TYPE_CAN_VIEW:
+        $xaction->setOldValue($project->getViewPolicy());
+        break;
+      case PhabricatorProjectTransactionType::TYPE_CAN_EDIT:
+        $xaction->setOldValue($project->getEditPolicy());
+        break;
+      case PhabricatorProjectTransactionType::TYPE_CAN_JOIN:
+        $xaction->setOldValue($project->getJoinPolicy());
+        break;
      default:
        throw new Exception("Unknown transaction type '{$type}'!");
    }
@ -229,6 +270,21 @@ final class PhabricatorProjectEditor {
        $this->addEdges = array_keys(array_diff_key($new, $old));
        $this->remEdges = array_keys(array_diff_key($old, $new));
        break;
+      case PhabricatorProjectTransactionType::TYPE_CAN_VIEW:
+        $project->setViewPolicy($xaction->getNewValue());
+        break;
+      case PhabricatorProjectTransactionType::TYPE_CAN_EDIT:
+        $project->setEditPolicy($xaction->getNewValue());
+
+        // You can't edit away your ability to edit the project.
+        PhabricatorPolicyFilter::mustRetainCapability(
+          $this->user,
+          $project,
+          PhabricatorPolicyCapability::CAN_EDIT);
+        break;
+      case PhabricatorProjectTransactionType::TYPE_CAN_JOIN:
+        $project->setJoinPolicy($xaction->getNewValue());
+        break;
      default:
        throw new Exception("Unknown transaction type '{$type}'!");
    }
@ -264,4 +320,75 @@ final class PhabricatorProjectEditor {
    return ($xaction->getOldValue() !== $xaction->getNewValue());
  }

+
+  /**
+   * All transactions except joining or leaving a project require edit
+   * capability.
+   */
+  private function getTransactionRequiresEditCapability(
+    PhabricatorProjectTransaction $xaction) {
+    return ($this->isJoinOrLeaveTransaction($xaction) === null);
+  }
+
+
+  /**
+   * Joining a project requires the join capability. Anyone leave a project.
+   */
+  private function getTransactionRequiresJoinCapability(
+    PhabricatorProjectTransaction $xaction) {
+    $type = $this->isJoinOrLeaveTransaction($xaction);
+    return ($type == 'join');
+  }
+
+
+  /**
+   * Returns 'join' if this transaction causes the acting user ONLY to join the
+   * project.
+   *
+   * Returns 'leave' if this transaction causes the acting user ONLY to leave
+   * the project.
+   *
+   * Returns null in all other cases.
+   */
+  private function isJoinOrLeaveTransaction(
+    PhabricatorProjectTransaction $xaction) {
+
+    $type = $xaction->getTransactionType();
+    if ($type != PhabricatorProjectTransactionType::TYPE_MEMBERS) {
+      return null;
+    }
+
+    switch ($type) {
+      case PhabricatorProjectTransactionType::TYPE_MEMBERS:
+        $old = $xaction->getOldValue();
+        $new = $xaction->getNewValue();
+
+        $add = array_diff($new, $old);
+        $rem = array_diff($old, $new);
+
+        if (count($add) > 1) {
+          return null;
+        } else if (count($add) == 1) {
+          if (reset($add) != $this->user->getPHID()) {
+            return null;
+          } else {
+            return 'join';
+          }
+        }
+
+        if (count($rem) > 1) {
+          return null;
+        } else if (count($rem) == 1) {
+          if (reset($rem) != $this->user->getPHID()) {
+            return null;
+          } else {
+            return 'leave';
+          }
+        }
+        break;
+    }
+
+    return true;
+  }
+
 }