@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Use Differential policy columns to drive policies

Summary:
Ref T603. Read policies out of policy columns.

When a revision is associated with a repository (which is currently never), require view access on the repository to see the revision (or, require the viewer to be the owner). This is a blanket "do the right thing" rule which should make Differential's default policies align with user expectations.

Future diffs will populate the `repositoryPHID` when a revision is created.

Test Plan: Tooled around Differential. None of this stuff does anything yet, so nothing very exciting happened.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

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

+79 -1
+53
src/applications/differential/query/DifferentialRevisionQuery.php
··· 361 361 } 362 362 363 363 public function willFilterPage(array $revisions) { 364 + $viewer = $this->getViewer(); 365 + 366 + $repository_phids = mpull($revisions, 'getRepositoryPHID'); 367 + $repository_phids = array_filter($repository_phids); 368 + 369 + $repositories = array(); 370 + if ($repository_phids) { 371 + $repositories = id(new PhabricatorRepositoryQuery()) 372 + ->setViewer($this->getViewer()) 373 + ->withPHIDs($repository_phids) 374 + ->execute(); 375 + $repositories = mpull($repositories, null, 'getPHID'); 376 + } 377 + 378 + // If a revision is associated with a repository: 379 + // 380 + // - the viewer must be able to see the repository; or 381 + // - the viewer must have an automatic view capability. 382 + // 383 + // In the latter case, we'll load the revision but not load the repository. 384 + 385 + $can_view = PhabricatorPolicyCapability::CAN_VIEW; 386 + foreach ($revisions as $key => $revision) { 387 + $repo_phid = $revision->getRepositoryPHID(); 388 + if (!$repo_phid) { 389 + // The revision has no associated repository. Attach `null` and move on. 390 + $revision->attachRepository(null); 391 + continue; 392 + } 393 + 394 + $repository = idx($repositories, $repo_phid); 395 + if ($repository) { 396 + // The revision has an associated repository, and the viewer can see 397 + // it. Attach it and move on. 398 + $revision->attachRepository($repository); 399 + continue; 400 + } 401 + 402 + if ($revision->hasAutomaticCapability($can_view, $viewer)) { 403 + // The revision has an associated repository which the viewer can not 404 + // see, but the viewer has an automatic capability on this revision. 405 + // Load the revision without attaching a repository. 406 + $revision->attachRepository(null); 407 + continue; 408 + } 409 + 410 + // The revision has an associated repository, and the viewer can't see 411 + // it, and the viewer has no special capabilities. Filter out this 412 + // revision. 413 + unset($revisions[$key]); 414 + } 415 + 416 + 364 417 $table = new DifferentialRevision(); 365 418 $conn_r = $table->establishConnection('r'); 366 419
+26 -1
src/applications/differential/storage/DifferentialRevision.php
··· 34 34 private $activeDiff = self::ATTACHABLE; 35 35 private $diffIDs = self::ATTACHABLE; 36 36 private $hashes = self::ATTACHABLE; 37 + private $repository = self::ATTACHABLE; 37 38 38 39 private $reviewerStatus = self::ATTACHABLE; 39 40 ··· 306 307 } 307 308 308 309 public function getPolicy($capability) { 309 - return PhabricatorPolicies::POLICY_USER; 310 + switch ($capability) { 311 + case PhabricatorPolicyCapability::CAN_VIEW: 312 + return $this->getViewPolicy(); 313 + case PhabricatorPolicyCapability::CAN_EDIT: 314 + return $this->getEditPolicy(); 315 + } 310 316 } 311 317 312 318 public function hasAutomaticCapability($capability, PhabricatorUser $user) { 319 + 320 + // A revision's author (which effectively means "owner" after we added 321 + // commandeering) can always view and edit it. 322 + $author_phid = $this->getAuthorPHID(); 323 + if ($author_phid) { 324 + if ($user->getPHID() == $author_phid) { 325 + return true; 326 + } 327 + } 328 + 313 329 return false; 314 330 } 315 331 ··· 327 343 assert_instances_of($reviewers, 'DifferentialReviewer'); 328 344 329 345 $this->reviewerStatus = $reviewers; 346 + return $this; 347 + } 348 + 349 + public function getRepository() { 350 + return $this->assertAttached($this->repository); 351 + } 352 + 353 + public function attachRepository(PhabricatorRepository $repository = null) { 354 + $this->repository = $repository; 330 355 return $this; 331 356 } 332 357 }