1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-19 16:58:48 +02:00

Add an "Authority" control to Packages to support "Watcher" packages

Summary: See T13657. An install has "watcher" packages which should not allow owners to "Force Accept" other packages.

Test Plan:
  - Created package A, which I own, on "/", with "Weak" authority.
  - Created package B, which I do not own, on "/src".
  - Created a revision which touches "/src" and added package B as a reviewer.
  - Attempted to accept the revision...
    - Before patch: permitted to "Force Accept" for package B.
    - After patch: not allowed to "Force Accept" for package B.
  - Verified that setting package "A" back to "Strong" authority allows a force-accept for package B.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D21674
This commit is contained in:
epriestley 2021-06-25 08:42:40 -07:00
parent bf889c1c08
commit a641ec82a3
9 changed files with 148 additions and 0 deletions

View file

@ -0,0 +1,2 @@
ALTER TABLE {$NAMESPACE}_owners.owners_package
ADD authorityMode VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT};

View file

@ -0,0 +1,3 @@
UPDATE {$NAMESPACE}_owners.owners_package
SET authorityMode = 'strong'
WHERE authorityMode = '';

View file

@ -3966,6 +3966,7 @@ phutil_register_library_map(array(
'PhabricatorOwnersOwner' => 'applications/owners/storage/PhabricatorOwnersOwner.php', 'PhabricatorOwnersOwner' => 'applications/owners/storage/PhabricatorOwnersOwner.php',
'PhabricatorOwnersPackage' => 'applications/owners/storage/PhabricatorOwnersPackage.php', 'PhabricatorOwnersPackage' => 'applications/owners/storage/PhabricatorOwnersPackage.php',
'PhabricatorOwnersPackageAuditingTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php', 'PhabricatorOwnersPackageAuditingTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php',
'PhabricatorOwnersPackageAuthorityTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageAuthorityTransaction.php',
'PhabricatorOwnersPackageAutoreviewTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageAutoreviewTransaction.php', 'PhabricatorOwnersPackageAutoreviewTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageAutoreviewTransaction.php',
'PhabricatorOwnersPackageContextFreeGrammar' => 'applications/owners/lipsum/PhabricatorOwnersPackageContextFreeGrammar.php', 'PhabricatorOwnersPackageContextFreeGrammar' => 'applications/owners/lipsum/PhabricatorOwnersPackageContextFreeGrammar.php',
'PhabricatorOwnersPackageDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php', 'PhabricatorOwnersPackageDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php',
@ -10582,6 +10583,7 @@ phutil_register_library_map(array(
'PhabricatorNgramsInterface', 'PhabricatorNgramsInterface',
), ),
'PhabricatorOwnersPackageAuditingTransaction' => 'PhabricatorOwnersPackageTransactionType', 'PhabricatorOwnersPackageAuditingTransaction' => 'PhabricatorOwnersPackageTransactionType',
'PhabricatorOwnersPackageAuthorityTransaction' => 'PhabricatorOwnersPackageTransactionType',
'PhabricatorOwnersPackageAutoreviewTransaction' => 'PhabricatorOwnersPackageTransactionType', 'PhabricatorOwnersPackageAutoreviewTransaction' => 'PhabricatorOwnersPackageTransactionType',
'PhabricatorOwnersPackageContextFreeGrammar' => 'PhutilContextFreeGrammar', 'PhabricatorOwnersPackageContextFreeGrammar' => 'PhutilContextFreeGrammar',
'PhabricatorOwnersPackageDatasource' => 'PhabricatorTypeaheadDatasource', 'PhabricatorOwnersPackageDatasource' => 'PhabricatorTypeaheadDatasource',

View file

@ -311,9 +311,17 @@ final class DifferentialRevision extends DifferentialDAO
// which the actor may be able to use their authority over to gain the // which the actor may be able to use their authority over to gain the
// ability to force-accept for other packages. This query doesn't apply // ability to force-accept for other packages. This query doesn't apply
// dominion rules yet, and we'll bypass those rules later on. // dominion rules yet, and we'll bypass those rules later on.
// See T13657. We ignore "watcher" packages which don't grant their owners
// permission to force accept anything.
$authority_query = id(new PhabricatorOwnersPackageQuery()) $authority_query = id(new PhabricatorOwnersPackageQuery())
->setViewer($viewer) ->setViewer($viewer)
->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE))
->withAuthorityModes(
array(
PhabricatorOwnersPackage::AUTHORITY_STRONG,
))
->withAuthorityPHIDs(array($viewer->getPHID())) ->withAuthorityPHIDs(array($viewer->getPHID()))
->withControl($repository_phid, $paths); ->withControl($repository_phid, $paths);
$authority_packages = $authority_query->execute(); $authority_packages = $authority_query->execute();

View file

@ -197,6 +197,12 @@ final class PhabricatorOwnersDetailController
$name = idx($spec, 'short', $dominion); $name = idx($spec, 'short', $dominion);
$view->addProperty(pht('Dominion'), $name); $view->addProperty(pht('Dominion'), $name);
$authority_mode = $package->getAuthorityMode();
$authority_map = PhabricatorOwnersPackage::getAuthorityOptionsMap();
$spec = idx($authority_map, $authority_mode, array());
$name = idx($spec, 'short', $authority_mode);
$view->addProperty(pht('Authority'), $name);
$auto = $package->getAutoReview(); $auto = $package->getAutoReview();
$autoreview_map = PhabricatorOwnersPackage::getAutoreviewOptionsMap(); $autoreview_map = PhabricatorOwnersPackage::getAutoreviewOptionsMap();
$spec = idx($autoreview_map, $auto, array()); $spec = idx($autoreview_map, $auto, array());

View file

@ -90,6 +90,9 @@ EOTEXT
$dominion_map = PhabricatorOwnersPackage::getDominionOptionsMap(); $dominion_map = PhabricatorOwnersPackage::getDominionOptionsMap();
$dominion_map = ipull($dominion_map, 'name'); $dominion_map = ipull($dominion_map, 'name');
$authority_map = PhabricatorOwnersPackage::getAuthorityOptionsMap();
$authority_map = ipull($authority_map, 'name');
return array( return array(
id(new PhabricatorTextEditField()) id(new PhabricatorTextEditField())
->setKey('name') ->setKey('name')
@ -118,6 +121,16 @@ EOTEXT
->setIsCopyable(true) ->setIsCopyable(true)
->setValue($object->getDominion()) ->setValue($object->getDominion())
->setOptions($dominion_map), ->setOptions($dominion_map),
id(new PhabricatorSelectEditField())
->setKey('authority')
->setLabel(pht('Authority'))
->setDescription(
pht('Change package authority rules.'))
->setTransactionType(
PhabricatorOwnersPackageAuthorityTransaction::TRANSACTIONTYPE)
->setIsCopyable(true)
->setValue($object->getAuthorityMode())
->setOptions($authority_map),
id(new PhabricatorSelectEditField()) id(new PhabricatorSelectEditField())
->setKey('autoReview') ->setKey('autoReview')
->setLabel(pht('Auto Review')) ->setLabel(pht('Auto Review'))

View file

@ -10,6 +10,7 @@ final class PhabricatorOwnersPackageQuery
private $repositoryPHIDs; private $repositoryPHIDs;
private $paths; private $paths;
private $statuses; private $statuses;
private $authorityModes;
private $controlMap = array(); private $controlMap = array();
private $controlResults; private $controlResults;
@ -77,6 +78,11 @@ final class PhabricatorOwnersPackageQuery
return $this; return $this;
} }
public function withAuthorityModes(array $modes) {
$this->authorityModes = $modes;
return $this;
}
public function withNameNgrams($ngrams) { public function withNameNgrams($ngrams) {
return $this->withNgramsConstraint( return $this->withNgramsConstraint(
new PhabricatorOwnersPackageNameNgrams(), new PhabricatorOwnersPackageNameNgrams(),
@ -231,6 +237,13 @@ final class PhabricatorOwnersPackageQuery
$where[] = qsprintf($conn, '%LO', $clauses); $where[] = qsprintf($conn, '%LO', $clauses);
} }
if ($this->authorityModes !== null) {
$where[] = qsprintf(
$conn,
'authorityMode IN (%Ls)',
$this->authorityModes);
}
return $where; return $where;
} }

View file

@ -21,6 +21,7 @@ final class PhabricatorOwnersPackage
protected $dominion; protected $dominion;
protected $properties = array(); protected $properties = array();
protected $auditingState; protected $auditingState;
protected $authorityMode;
private $paths = self::ATTACHABLE; private $paths = self::ATTACHABLE;
private $owners = self::ATTACHABLE; private $owners = self::ATTACHABLE;
@ -41,6 +42,9 @@ final class PhabricatorOwnersPackage
const DOMINION_STRONG = 'strong'; const DOMINION_STRONG = 'strong';
const DOMINION_WEAK = 'weak'; const DOMINION_WEAK = 'weak';
const AUTHORITY_STRONG = 'strong';
const AUTHORITY_WEAK = 'weak';
const PROPERTY_IGNORED = 'ignored'; const PROPERTY_IGNORED = 'ignored';
public static function initializeNewPackage(PhabricatorUser $actor) { public static function initializeNewPackage(PhabricatorUser $actor) {
@ -58,6 +62,7 @@ final class PhabricatorOwnersPackage
->setAuditingState(PhabricatorOwnersAuditRule::AUDITING_NONE) ->setAuditingState(PhabricatorOwnersAuditRule::AUDITING_NONE)
->setAutoReview(self::AUTOREVIEW_NONE) ->setAutoReview(self::AUTOREVIEW_NONE)
->setDominion(self::DOMINION_STRONG) ->setDominion(self::DOMINION_STRONG)
->setAuthorityMode(self::AUTHORITY_STRONG)
->setViewPolicy($view_policy) ->setViewPolicy($view_policy)
->setEditPolicy($edit_policy) ->setEditPolicy($edit_policy)
->attachPaths(array()) ->attachPaths(array())
@ -115,6 +120,19 @@ final class PhabricatorOwnersPackage
); );
} }
public static function getAuthorityOptionsMap() {
return array(
self::AUTHORITY_STRONG => array(
'name' => pht('Strong (Package Owns Paths)'),
'short' => pht('Strong'),
),
self::AUTHORITY_WEAK => array(
'name' => pht('Weak (Package Watches Paths)'),
'short' => pht('Weak'),
),
);
}
protected function getConfiguration() { protected function getConfiguration() {
return array( return array(
// This information is better available from the history table. // This information is better available from the history table.
@ -130,6 +148,7 @@ final class PhabricatorOwnersPackage
'status' => 'text32', 'status' => 'text32',
'autoReview' => 'text32', 'autoReview' => 'text32',
'dominion' => 'text32', 'dominion' => 'text32',
'authorityMode' => 'text32',
), ),
) + parent::getConfiguration(); ) + parent::getConfiguration();
} }
@ -568,6 +587,10 @@ final class PhabricatorOwnersPackage
return PhabricatorOwnersAuditRule::newFromState($this->getAuditingState()); return PhabricatorOwnersAuditRule::newFromState($this->getAuditingState());
} }
public function getHasStrongAuthority() {
return ($this->getAuthorityMode() === self::AUTHORITY_STRONG);
}
/* -( PhabricatorPolicyInterface )----------------------------------------- */ /* -( PhabricatorPolicyInterface )----------------------------------------- */
@ -696,6 +719,10 @@ final class PhabricatorOwnersPackage
->setKey('dominion') ->setKey('dominion')
->setType('map<string, wild>') ->setType('map<string, wild>')
->setDescription(pht('Dominion setting information.')), ->setDescription(pht('Dominion setting information.')),
id(new PhabricatorConduitSearchFieldSpecification())
->setKey('authority')
->setType('map<string, wild>')
->setDescription(pht('Authority setting information.')),
id(new PhabricatorConduitSearchFieldSpecification()) id(new PhabricatorConduitSearchFieldSpecification())
->setKey('ignored') ->setKey('ignored')
->setType('map<string, wild>') ->setType('map<string, wild>')
@ -747,6 +774,23 @@ final class PhabricatorOwnersPackage
'short' => $dominion_short, 'short' => $dominion_short,
); );
$authority_value = $this->getAuthorityMode();
$authority_map = self::getAuthorityOptionsMap();
if (isset($authority_map[$authority_value])) {
$authority_label = $authority_map[$authority_value]['name'];
$authority_short = $authority_map[$authority_value]['short'];
} else {
$authority_label = pht('Unknown ("%s")', $authority_value);
$authority_short = pht('Unknown ("%s")', $authority_value);
}
$authority = array(
'value' => $authority_value,
'label' => $authority_label,
'short' => $authority_short,
);
// Force this to always emit as a JSON object even if empty, never as // Force this to always emit as a JSON object even if empty, never as
// a JSON list. // a JSON list.
$ignored = $this->getIgnoredPathAttributes(); $ignored = $this->getIgnoredPathAttributes();
@ -762,6 +806,7 @@ final class PhabricatorOwnersPackage
'review' => $review, 'review' => $review,
'audit' => $audit, 'audit' => $audit,
'dominion' => $dominion, 'dominion' => $dominion,
'authority' => $authority,
'ignored' => $ignored, 'ignored' => $ignored,
); );
} }

View file

@ -0,0 +1,56 @@
<?php
final class PhabricatorOwnersPackageAuthorityTransaction
extends PhabricatorOwnersPackageTransactionType {
const TRANSACTIONTYPE = 'owners.authority';
public function generateOldValue($object) {
return $object->getAuthorityMode();
}
public function validateTransactions($object, array $xactions) {
$errors = array();
$map = PhabricatorOwnersPackage::getAuthorityOptionsMap();
foreach ($xactions as $xaction) {
$new = $xaction->getNewValue();
if (empty($map[$new])) {
$valid = array_keys($map);
$errors[] = $this->newInvalidError(
pht(
'Authority setting "%s" is not valid. '.
'Valid settings are: %s.',
$new,
implode(', ', $valid)),
$xaction);
}
}
return $errors;
}
public function applyInternalEffects($object, $value) {
$object->setAuthorityMode($value);
}
public function getTitle() {
$map = PhabricatorOwnersPackage::getAuthorityOptionsMap();
$map = ipull($map, 'short');
$old = $this->getOldValue();
$new = $this->getNewValue();
$old = idx($map, $old, $old);
$new = idx($map, $new, $new);
return pht(
'%s adjusted package authority rules from %s to %s.',
$this->renderAuthor(),
$this->renderValue($old),
$this->renderValue($new));
}
}