1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-03-30 22:18:13 +02:00

In Phortune, use actual merchant authority (not authority grants) to control account visibility

Summary:
Depends on D20715. Ref T13366. See that task for discussion.

Replace the unreliable "grantAuthority()"-based check with an actual "can the viewer edit any merchant this account has a relationship with?" check.

This makes these objects easier to use from a policy perspective and makes it so that the `Query` alone can fully enforce permissions properly with no setup, so general infrastructure (like handles and transactions) works properly with Phortune objects.

Test Plan: Viewed merchants and accounts as users with no authority, direct authority on the account, and indirect authority via a merchant relationship.

Maniphest Tasks: T13366

Differential Revision: https://secure.phabricator.com/D20716
This commit is contained in:
epriestley 2019-08-15 16:37:46 -07:00
parent 277bce5638
commit a3213ab20b
3 changed files with 84 additions and 14 deletions

View file

@ -58,9 +58,10 @@ final class PhortuneMerchantInvoiceCreateController
} }
if (!$target_account) { if (!$target_account) {
$accounts = PhortuneAccountQuery::loadAccountsForUser( $accounts = id(new PhortuneAccountQuery())
$target_user, ->setViewer($viewer)
PhabricatorContentSource::newFromRequest($request)); ->withMemberPHIDs(array($target_user->getPHID()))
->execute();
$form = id(new AphrontFormView()) $form = id(new AphrontFormView())
->setUser($viewer) ->setUser($viewer)

View file

@ -86,14 +86,14 @@ final class PhortuneMerchantQuery
if ($this->ids !== null) { if ($this->ids !== null) {
$where[] = qsprintf( $where[] = qsprintf(
$conn, $conn,
'id IN (%Ld)', 'merchant.id IN (%Ld)',
$this->ids); $this->ids);
} }
if ($this->phids !== null) { if ($this->phids !== null) {
$where[] = qsprintf( $where[] = qsprintf(
$conn, $conn,
'phid IN (%Ls)', 'merchant.phid IN (%Ls)',
$this->phids); $this->phids);
} }
@ -113,7 +113,7 @@ final class PhortuneMerchantQuery
if ($this->memberPHIDs !== null) { if ($this->memberPHIDs !== null) {
$joins[] = qsprintf( $joins[] = qsprintf(
$conn, $conn,
'LEFT JOIN %T e ON m.phid = e.src AND e.type = %d', 'LEFT JOIN %T e ON merchant.phid = e.src AND e.type = %d',
PhabricatorEdgeConfig::TABLE_NAME_EDGE, PhabricatorEdgeConfig::TABLE_NAME_EDGE,
PhortuneMerchantHasMemberEdgeType::EDGECONST); PhortuneMerchantHasMemberEdgeType::EDGECONST);
} }
@ -126,7 +126,68 @@ final class PhortuneMerchantQuery
} }
protected function getPrimaryTableAlias() { protected function getPrimaryTableAlias() {
return 'm'; return 'merchant';
}
public static function canViewersEditMerchants(
array $viewer_phids,
array $merchant_phids) {
// See T13366 for some discussion. This is an unusual caching construct to
// make policy filtering of Accounts easier.
foreach ($viewer_phids as $key => $viewer_phid) {
if (!$viewer_phid) {
unset($viewer_phids[$key]);
}
}
if (!$viewer_phids) {
return array();
}
$cache_key = 'phortune.merchant.can-edit';
$cache = PhabricatorCaches::getRequestCache();
$cache_data = $cache->getKey($cache_key);
if (!$cache_data) {
$cache_data = array();
}
$load_phids = array();
foreach ($viewer_phids as $viewer_phid) {
if (!isset($cache_data[$viewer_phid])) {
$load_phids[] = $viewer_phid;
}
}
$did_write = false;
foreach ($load_phids as $load_phid) {
$merchants = id(new self())
->setViewer(PhabricatorUser::getOmnipotentUser())
->withMemberPHIDs(array($load_phid))
->execute();
foreach ($merchants as $merchant) {
$cache_data[$load_phid][$merchant->getPHID()] = true;
$did_write = true;
}
}
if ($did_write) {
$cache->setKey($cache_key, $cache_data);
}
$results = array();
foreach ($viewer_phids as $viewer_phid) {
foreach ($merchant_phids as $merchant_phid) {
if (!isset($cache_data[$viewer_phid][$merchant_phid])) {
continue;
}
$results[$viewer_phid][$merchant_phid] = true;
}
}
return $results;
} }
} }

View file

@ -179,13 +179,18 @@ final class PhortuneAccount extends PhortuneDAO
return true; return true;
} }
// If the viewer is acting on behalf of a merchant, they can see // See T13366. If the viewer can edit any merchant that this payment
// payment accounts. // account has a relationship with, they can see the payment account.
if ($capability == PhabricatorPolicyCapability::CAN_VIEW) { if ($capability == PhabricatorPolicyCapability::CAN_VIEW) {
foreach ($viewer->getAuthorities() as $authority) { $viewer_phids = array($viewer->getPHID());
if ($authority instanceof PhortuneMerchant) { $merchant_phids = $this->getMerchantPHIDs();
return true;
} $any_edit = PhortuneMerchantQuery::canViewersEditMerchants(
$viewer_phids,
$merchant_phids);
if ($any_edit) {
return true;
} }
} }
@ -193,7 +198,10 @@ final class PhortuneAccount extends PhortuneDAO
} }
public function describeAutomaticCapability($capability) { public function describeAutomaticCapability($capability) {
return pht('Members of an account can always view and edit it.'); return array(
pht('Members of an account can always view and edit it.'),
pht('Merchants an account has established a relationship can view it.'),
);
} }