1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-01-02 02:40:58 +01:00

Make most Differential reads policy-aware

Summary: Ref T603. Makes the majority of reads policy aware (and pretty much all the important ones).

Test Plan:
  - Created a comment with `differential.createcomment`.
  - Created a new revision with `arc diff` in order to exercise `differential.creatediff`.
  - Created an inline comment with `differential.createinline`.
  - Added a comment to a revision.
  - Edited an inline comment.
  - Edited a revision.
  - Wrote "Depends on ..." in a summary, saved, verified link was created.
  - Browsed a file in Diffusion.
  - Got past the code I changed in the Releeph request thing.
  - Edited a Releeph request.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7136
This commit is contained in:
epriestley 2013-09-26 12:37:19 -07:00
parent 80378eb5f6
commit 9b3d7b0dba
16 changed files with 66 additions and 21 deletions

View file

@ -31,8 +31,10 @@ final class ConduitAPI_differential_createcomment_Method
} }
protected function execute(ConduitAPIRequest $request) { protected function execute(ConduitAPIRequest $request) {
$revision = id(new DifferentialRevision())->load( $revision = id(new DifferentialRevisionQuery())
$request->getValue('revision_id')); ->setViewer($request->getUser())
->withIDs(array($request->getValue('revision_id')))
->executeOne();
if (!$revision) { if (!$revision) {
throw new ConduitException('ERR_BAD_REVISION'); throw new ConduitException('ERR_BAD_REVISION');
} }

View file

@ -59,7 +59,10 @@ final class ConduitAPI_differential_creatediff_Method extends ConduitAPIMethod {
$parent_id = $request->getValue('parentRevisionID'); $parent_id = $request->getValue('parentRevisionID');
if ($parent_id) { if ($parent_id) {
$parent_rev = id(new DifferentialRevision())->load($parent_id); $parent_rev = id(new DifferentialRevisionQuery())
->setViewer($request->getUser())
->withIDs(array($parent_id))
->executeOne();
if ($parent_rev) { if ($parent_rev) {
if ($parent_rev->getStatus() != if ($parent_rev->getStatus() !=
ArcanistDifferentialRevisionStatus::CLOSED) { ArcanistDifferentialRevisionStatus::CLOSED) {

View file

@ -43,7 +43,10 @@ final class ConduitAPI_differential_createinline_Method
if ($rid) { if ($rid) {
// Given both a revision and a diff, check that they match. // Given both a revision and a diff, check that they match.
// Given only a revision, find the active diff. // Given only a revision, find the active diff.
$revision = id(new DifferentialRevision())->load($rid); $revision = id(new DifferentialRevisionQuery())
->setViewer($request->getUser())
->withIDs(array($rid))
->executeOne();
if (!$revision) { if (!$revision) {
throw new ConduitException('ERR-BAD-REVISION'); throw new ConduitException('ERR-BAD-REVISION');
} }

View file

@ -8,8 +8,13 @@ final class DifferentialCommentSaveController extends DifferentialController {
return new Aphront400Response(); return new Aphront400Response();
} }
$viewer = $request->getUser();
$revision_id = $request->getInt('revision_id'); $revision_id = $request->getInt('revision_id');
$revision = id(new DifferentialRevision())->load($revision_id); $revision = id(new DifferentialRevisionQuery())
->setViewer($viewer)
->withIDs(array($revision_id))
->executeOne();
if (!$revision) { if (!$revision) {
return new Aphront400Response(); return new Aphront400Response();
} }

View file

@ -15,7 +15,13 @@ final class DifferentialInlineCommentEditController
$revision_id = $this->revisionID; $revision_id = $this->revisionID;
$changeset_id = $this->getChangesetID(); $changeset_id = $this->getChangesetID();
if (!id(new DifferentialRevision())->load($revision_id)) { $viewer = $this->getRequest()->getUser();
$revision = id(new DifferentialRevisionQuery())
->setViewer($viewer)
->withIDs(array($revision_id))
->executeOne();
if (!$revision) {
throw new Exception("Invalid revision ID!"); throw new Exception("Invalid revision ID!");
} }

View file

@ -22,6 +22,11 @@ final class DifferentialRevisionEditController extends DifferentialController {
->withIDs(array($this->id)) ->withIDs(array($this->id))
->needRelationships(true) ->needRelationships(true)
->needReviewerStatus(true) ->needReviewerStatus(true)
->requireCapabilities(
array(
PhabricatorPolicyCapability::CAN_VIEW,
PhabricatorPolicyCapability::CAN_EDIT,
))
->executeOne(); ->executeOne();
if (!$revision) { if (!$revision) {
return new Aphront404Response(); return new Aphront404Response();

View file

@ -162,8 +162,10 @@ abstract class DifferentialFreeformFieldSpecification
$dependents = $this->findDependentRevisions($message); $dependents = $this->findDependentRevisions($message);
if ($dependents) { if ($dependents) {
$dependents = id(new DifferentialRevision()) $dependents = id(new DifferentialRevisionQuery())
->loadAllWhere('id IN (%Ld)', $dependents); ->setViewer($editor->getActor())
->withIDs($dependents)
->execute();
$this->saveFieldEdges( $this->saveFieldEdges(
$editor->getRevision(), $editor->getRevision(),
PhabricatorEdgeConfig::TYPE_DREV_DEPENDS_ON_DREV, PhabricatorEdgeConfig::TYPE_DREV_DEPENDS_ON_DREV,

View file

@ -233,6 +233,10 @@ final class ConduitAPI_diffusion_getcommits_Method
private function addDifferentialInformation(array $commits) { private function addDifferentialInformation(array $commits) {
$commit_phids = ipull($commits, 'commitPHID'); $commit_phids = ipull($commits, 'commitPHID');
// TODO: (T603) This should be policy checked, either by moving to
// DifferentialRevisionQuery or by doing a followup query to make sure
// the matched objects are visible.
$rev_conn_r = id(new DifferentialRevision())->establishConnection('r'); $rev_conn_r = id(new DifferentialRevision())->establishConnection('r');
$revs = queryfx_all( $revs = queryfx_all(
$rev_conn_r, $rev_conn_r,

View file

@ -550,18 +550,19 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
$commits = mpull($commits, null, 'getCommitIdentifier'); $commits = mpull($commits, null, 'getCommitIdentifier');
} }
$request = $this->getRequest();
$user = $request->getUser();
$revision_ids = id(new DifferentialRevision()) $revision_ids = id(new DifferentialRevision())
->loadIDsByCommitPHIDs(mpull($commits, 'getPHID')); ->loadIDsByCommitPHIDs(mpull($commits, 'getPHID'));
$revisions = array(); $revisions = array();
if ($revision_ids) { if ($revision_ids) {
$revisions = id(new DifferentialRevision())->loadAllWhere( $revisions = id(new DifferentialRevisionQuery())
'id IN (%Ld)', ->setViewer($user)
$revision_ids); ->withIDs($revision_ids)
->execute();
} }
$request = $this->getRequest();
$user = $request->getUser();
Javelin::initBehavior('phabricator-oncopy', array()); Javelin::initBehavior('phabricator-oncopy', array());
$engine = null; $engine = null;

View file

@ -211,6 +211,7 @@ final class HeraldCommitAdapter extends HeraldAdapter {
$data = $this->commitData; $data = $this->commitData;
$revision_id = $data->getCommitDetail('differential.revisionID'); $revision_id = $data->getCommitDetail('differential.revisionID');
if ($revision_id) { if ($revision_id) {
// TODO: (T603) Herald policy stuff.
$revision = id(new DifferentialRevision())->load($revision_id); $revision = id(new DifferentialRevision())->load($revision_id);
if ($revision) { if ($revision) {
$revision->loadRelationships(); $revision->loadRelationships();

View file

@ -23,6 +23,7 @@ final class ReleephCommitFinder {
$matches = array(); $matches = array();
if (preg_match('/^D([1-9]\d*)$/', $partial_string, $matches)) { if (preg_match('/^D([1-9]\d*)$/', $partial_string, $matches)) {
$diff_id = $matches[1]; $diff_id = $matches[1];
// TOOD: (T603) This is all slated for annihilation.
$diff_rev = id(new DifferentialRevision())->load($diff_id); $diff_rev = id(new DifferentialRevision())->load($diff_id);
if (!$diff_rev) { if (!$diff_rev) {
throw new ReleephCommitFinderException( throw new ReleephCommitFinderException(

View file

@ -3,21 +3,26 @@
final class ReleephRequestDifferentialCreateController final class ReleephRequestDifferentialCreateController
extends ReleephProjectController { extends ReleephProjectController {
private $revisionID;
private $revision; private $revision;
public function willProcessRequest(array $data) { public function willProcessRequest(array $data) {
$diff_rev_id = $data['diffRevID']; $this->revisionID = $data['diffRevID'];
$diff_rev = id(new DifferentialRevision())->load($diff_rev_id);
if (!$diff_rev) {
throw new Exception(sprintf('D%d not found!', $diff_rev_id));
}
$this->revision = $diff_rev;
} }
public function processRequest() { public function processRequest() {
$request = $this->getRequest(); $request = $this->getRequest();
$user = $request->getUser(); $user = $request->getUser();
$diff_rev = id(new DifferentialRevisionQuery())
->setViewer($user)
->withIDs(array($this->revisionID))
->executeOne();
if (!$diff_rev) {
return new Aphront404Response();
}
$this->revision = $diff_rev;
$arc_project = id(new PhabricatorRepositoryArcanistProject()) $arc_project = id(new PhabricatorRepositoryArcanistProject())
->loadOneWhere('phid = %s', $this->revision->getArcanistProjectPHID()); ->loadOneWhere('phid = %s', $this->revision->getArcanistProjectPHID());

View file

@ -228,7 +228,10 @@ final class ReleephRequestEditController extends ReleephProjectController {
$origin = null; $origin = null;
$diff_rev_id = $request->getStr('D'); $diff_rev_id = $request->getStr('D');
if ($diff_rev_id) { if ($diff_rev_id) {
$diff_rev = id(new DifferentialRevision())->load($diff_rev_id); $diff_rev = id(new DifferentialRevisionQuery())
->setViewer($user)
->withIDs(array($diff_rev_id))
->executeOne();
$origin = '/D'.$diff_rev->getID(); $origin = '/D'.$diff_rev->getID();
$title = sprintf( $title = sprintf(
'D%d: %s', 'D%d: %s',

View file

@ -248,6 +248,7 @@ final class ReleephRequest extends ReleephDAO
} }
} }
// TODO: (T603) Get rid of all this one-off ad-hoc loading.
public function loadDifferentialRevision() { public function loadDifferentialRevision() {
$diff_phid = $this->loadRequestCommitDiffPHID(); $diff_phid = $this->loadRequestCommitDiffPHID();
if (!$diff_phid) { if (!$diff_phid) {

View file

@ -92,6 +92,8 @@ final class PhabricatorRepositoryCommitOwnersWorker
$commit_reviewedby_phid = null; $commit_reviewedby_phid = null;
if ($revision_id) { if ($revision_id) {
// TODO: (T603) This is probably safe to use an omnipotent user on,
// but check things more closely.
$revision = id(new DifferentialRevision())->load($revision_id); $revision = id(new DifferentialRevision())->load($revision_id);
if ($revision) { if ($revision) {
$revision_author_phid = $revision->getAuthorPHID(); $revision_author_phid = $revision->getAuthorPHID();

View file

@ -93,6 +93,7 @@ final class PhabricatorSearchSelectController
switch ($this->type) { switch ($this->type) {
case DifferentialPHIDTypeRevision::TYPECONST: case DifferentialPHIDTypeRevision::TYPECONST:
// TODO: (T603) See below. This whole thing needs cleanup.
$objects = id(new DifferentialRevision())->loadAllWhere( $objects = id(new DifferentialRevision())->loadAllWhere(
'id IN (%Ld)', 'id IN (%Ld)',
$object_ids); $object_ids);