Improve Phortune policy behavior

Summary:
Currently, PhortuneAccounts have a very open default policy to allow merchants to see and interact with them.

This has the undesirable side effect of leaking their names in too many places, because all users are allowed to load the handles for the accounts. Although this information is not super sensitive, we shouldn't expose it.

I went through about 5 really messy diffs trying to fix this. It's very complicated because there are a lot of objects and many of them are related to PhortuneAccounts, but PhortuneAccounts are not bound to a specific merchant. This lead to a lot of threading viewers and merchants all over the place through the call stack and some really sketchy diffs with OmnipotentUsers that weren't going anywhere good.

This is the cleanest approach I came up with, by far:

  - Introduce the concept of an "Authority", which gives a user more powers as a viewer. For now, since we only have one use case, this is pretty open-ended.
  - When a viewer is acting as a merchant, grant them authority through the merchant.
  - Have Accounts check if the viewer is acting with merchant authority. This lets us easily implement the rule "merchants can see this stuff" without being too broad.

Then update the Subscription view to respect Merchant Authority.

I partially updated the Cart views to respect it. I'll finish this up in a separate diff, but this seemed like a good checkpoint that introduced the concept without too much extra baggage.

This feels pretty good/clean to me, overall, even ignoring the series of horrible messes I made on my way here.

Test Plan:
  - Verified I can see everything I need to as a merchant (modulo un-updated Cart UIs).
  - Verified I can see nothing when acting as a normal user.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

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

src/applications/people/storage/PhabricatorUser.php

@@ -50,6 +50,8 @@ final class PhabricatorUser
   private $alternateCSRFString = self::ATTACHABLE;
   private $session = self::ATTACHABLE;
 
+  private $authorities = array();
+
   protected function readField($field) {
     switch ($field) {
       case 'timezoneIdentifier':
@@ -694,6 +696,25 @@ EOBODY;
       $email->getUserPHID());
   }
 
+
+  /**
+   * Grant a user a source of authority, to let them bypass policy checks they
+   * could not otherwise.
+   */
+  public function grantAuthority($authority) {
+    $this->authorities[] = $authority;
+    return $this;
+  }
+
+
+  /**
+   * Get authorities granted to the user.
+   */
+  public function getAuthorities() {
+    return $this->authorities;
+  }
+
+
 /* -(  Multi-Factor Authentication  )---------------------------------------- */
 
 

src/applications/phortune/application/PhabricatorPhortuneApplication.php

@@ -85,6 +85,12 @@ final class PhabricatorPhortuneApplication extends PhabricatorApplication {
           'edit/(?:(?P<id>\d+)/)?' => 'PhortuneMerchantEditController',
           'orders/(?P<merchantID>\d+)/(?:query/(?P<queryKey>[^/]+)/)?'
             => 'PhortuneCartListController',
+          '(?P<merchantID>\d+)/cart/(?P<id>\d+)/' => array(
+            '' => 'PhortuneCartViewController',
+            '(?P<action>cancel|refund)/' => 'PhortuneCartCancelController',
+            'update/' => 'PhortuneCartUpdateController',
+            'accept/' => 'PhortuneCartAcceptController',
+          ),
           '(?P<merchantID>\d+)/subscription/' => array(
             '(?:query/(?P<queryKey>[^/]+)/)?'
               => 'PhortuneSubscriptionListController',

src/applications/phortune/controller/PhortuneCartListController.php

@@ -42,6 +42,7 @@ final class PhortuneCartListController
         return new Aphront404Response();
       }
       $this->merchant = $merchant;
+      $viewer->grantAuthority($merchant);
       $engine->setMerchant($merchant);
     } else if ($account_id) {
       $account = id(new PhortuneAccountQuery())

src/applications/phortune/controller/PhortuneCartViewController.php

@@ -13,6 +13,11 @@ final class PhortuneCartViewController
     $request = $this->getRequest();
     $viewer = $request->getUser();
 
+    $authority = $this->loadMerchantAuthority();
+
+    // TODO: This (and the rest of the Cart controllers) need to be updated
+    // to use merchant URIs and merchant authority.
+
     $cart = id(new PhortuneCartQuery())
       ->setViewer($viewer)
       ->withIDs(array($this->id))
@@ -157,7 +162,11 @@ final class PhortuneCartViewController
     $account = $cart->getAccount();
 
     $crumbs = $this->buildApplicationCrumbs();
-    $this->addAccountCrumb($crumbs, $cart->getAccount());
+    if ($authority) {
+      $this->addMerchantCrumb($crumbs, $authority);
+    } else {
+      $this->addAccountCrumb($crumbs, $cart->getAccount());
+    }
     $crumbs->addTextCrumb(pht('Cart %d', $cart->getID()));
 
     $timeline = $this->buildTransactionTimeline(

src/applications/phortune/controller/PhortuneController.php

@@ -84,4 +84,30 @@ abstract class PhortuneController extends PhabricatorController {
     return $providers;
   }
 
+  protected function loadMerchantAuthority() {
+    $request = $this->getRequest();
+    $viewer = $this->getViewer();
+
+    $is_merchant = (bool)$request->getURIData('merchantID');
+    if (!$is_merchant) {
+      return null;
+    }
+
+    $merchant = id(new PhortuneMerchantQuery())
+      ->setViewer($viewer)
+      ->withIDs(array($request->getURIData('merchantID')))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->executeOne();
+    if (!$merchant) {
+      return null;
+    }
+
+    $viewer->grantAuthority($merchant);
+    return $merchant;
+  }
+
 }

src/applications/phortune/controller/PhortuneSubscriptionListController.php

@@ -36,6 +36,7 @@ final class PhortuneSubscriptionListController
         return new Aphront404Response();
       }
       $this->merchant = $merchant;
+      $viewer->grantAuthority($merchant);
       $engine->setMerchant($merchant);
     } else if ($this->accountID) {
       $account = id(new PhortuneAccountQuery())

src/applications/phortune/controller/PhortuneSubscriptionViewController.php

@@ -5,6 +5,8 @@ final class PhortuneSubscriptionViewController extends PhortuneController {
   public function handleRequest(AphrontRequest $request) {
     $viewer = $this->getViewer();
 
+    $is_merchant = (bool)$this->loadMerchantAuthority();
+
     $subscription = id(new PhortuneSubscriptionQuery())
       ->setViewer($viewer)
       ->withIDs(array($request->getURIData('id')))
@@ -19,7 +21,6 @@ final class PhortuneSubscriptionViewController extends PhortuneController {
       $subscription,
       PhabricatorPolicyCapability::CAN_EDIT);
 
-    $is_merchant = (bool)$request->getURIData('merchantID');
     $merchant = $subscription->getMerchant();
     $account = $subscription->getAccount();
 

src/applications/phortune/query/PhortuneCartQuery.php

@@ -91,6 +91,10 @@ final class PhortuneCartQuery
       $cart->attachAccount($account);
     }
 
+    if (!$carts) {
+      return array();
+    }
+
     $merchants = id(new PhortuneMerchantQuery())
       ->setViewer($this->getViewer())
       ->withPHIDs(mpull($carts, 'getMerchantPHID'))
@@ -106,6 +110,10 @@ final class PhortuneCartQuery
       $cart->attachMerchant($merchant);
     }
 
+    if (!$carts) {
+      return array();
+    }
+
     $implementations = array();
 
     $cart_map = mgroup($carts, 'getCartClass');

src/applications/phortune/query/PhortuneCartSearchEngine.php

@@ -166,8 +166,21 @@ final class PhortuneCartSearchEngine
     foreach ($carts as $cart) {
       $merchant = $cart->getMerchant();
 
+      if ($this->getMerchant()) {
+        $href = $this->getApplicationURI(
+          'merchant/'.$merchant->getID().'/cart/'.$cart->getID().'/');
+      } else {
+        $href = $cart->getDetailURI();
+      }
+
       $rows[] = array(
         $cart->getID(),
+        phutil_tag(
+          'a',
+          array(
+            'href' => $href,
+          ),
+          $cart->getName()),
         $handles[$cart->getPHID()]->renderLink(),
         $handles[$merchant->getPHID()]->renderLink(),
         $handles[$cart->getAuthorPHID()]->renderLink(),

src/applications/phortune/query/PhortunePaymentMethodQuery.php

@@ -65,6 +65,10 @@ final class PhortunePaymentMethodQuery
       $method->attachAccount($account);
     }
 
+    if (!$methods) {
+      return $methods;
+    }
+
     $merchants = id(new PhortuneMerchantQuery())
       ->setViewer($this->getViewer())
       ->withPHIDs(mpull($methods, 'getMerchantPHID'))
@@ -80,6 +84,10 @@ final class PhortunePaymentMethodQuery
       $method->attachMerchant($merchant);
     }
 
+    if (!$methods) {
+      return $methods;
+    }
+
     $provider_configs = id(new PhortunePaymentProviderConfigQuery())
       ->setViewer($this->getViewer())
       ->withPHIDs(mpull($methods, 'getProviderPHID'))

src/applications/phortune/storage/PhortuneAccount.php

@@ -133,10 +133,6 @@ final class PhortuneAccount extends PhortuneDAO
   public function getPolicy($capability) {
     switch ($capability) {
       case PhabricatorPolicyCapability::CAN_VIEW:
-        // Accounts are technically visible to all users, because merchant
-        // controllers need to be able to see accounts in order to process
-        // orders. We lock things down more tightly at the application level.
-        return PhabricatorPolicies::POLICY_USER;
       case PhabricatorPolicyCapability::CAN_EDIT:
         if ($this->getPHID() === null) {
           // Allow a user to create an account for themselves.
@@ -149,7 +145,21 @@ final class PhortuneAccount extends PhortuneDAO
 
   public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
     $members = array_fuse($this->getMemberPHIDs());
-    return isset($members[$viewer->getPHID()]);
+    if (isset($members[$viewer->getPHID()])) {
+      return true;
+    }
+
+    // If the viewer is acting on behalf of a merchant, they can see
+    // payment accounts.
+    if ($capability == PhabricatorPolicyCapability::CAN_VIEW) {
+      foreach ($viewer->getAuthorities() as $authority) {
+        if ($authority instanceof PhortuneMerchant) {
+          return true;
+        }
+      }
+    }
+
+    return false;
   }
 
   public function describeAutomaticCapability($capability) {