1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00

Phriction - validateTransactions that need parent ancestry to complete successfully

Summary:
Fixes T6651, T6682. Since policy is defined by ancestry, you can't make things outside the core tree.

An alternative fix would be to automagically stub everything in these cases. This has potential negative policy implications - consider making a public document with several levels of depth that automagically stubs out its ancestry as public - and additionally the PhabricatorApplicationTransactionEditor framework would make this very tricky code (i.e. you are expected to validateTransactions in said hook *and* return an error if things aren't valid and not do some automagic stubbing, etc.)

Test Plan: tried to move a doc from location/that/exists to locationz/thatz/dontz/existz/ and got an error message with links to each missing doc. tried to create a doc at locatonz/thatz/dontz/existsz/ and got an error message with links to each missing doc.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6682, T6651

Differential Revision: https://secure.phabricator.com/D10978
This commit is contained in:
Bob Trahan 2014-12-12 11:35:43 -08:00
parent 3a85d0bf5a
commit 905fc217b8

View file

@ -3,6 +3,9 @@
final class PhrictionTransactionEditor
extends PhabricatorApplicationTransactionEditor {
const VALIDATE_CREATE_ANCESTRY = 'create';
const VALIDATE_MOVE_ANCESTRY = 'move';
private $description;
private $oldContent;
private $newContent;
@ -519,6 +522,18 @@ final class PhrictionTransactionEditor
$errors[] = $error;
}
}
if ($this->getIsNewObject()) {
$ancestry_errors = $this->validateAncestry(
$object,
$type,
$xaction,
self::VALIDATE_CREATE_ANCESTRY);
if ($ancestry_errors) {
$errors = array_merge($errors, $ancestry_errors);
}
}
break;
case PhrictionTransaction::TYPE_MOVE_TO:
@ -547,8 +562,15 @@ final class PhrictionTransactionEditor
$errors[] = $error;
}
// NOTE: We use the ominpotent user because we can't let users
// overwrite documents even if they can't see them.
$ancestry_errors = $this->validateAncestry(
$object,
$type,
$xaction,
self::VALIDATE_MOVE_ANCESTRY);
if ($ancestry_errors) {
$errors = array_merge($errors, $ancestry_errors);
}
$target_document = id(new PhrictionDocumentQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
->withSlugs(array($object->getSlug()))
@ -606,6 +628,59 @@ final class PhrictionTransactionEditor
return $errors;
}
private function validateAncestry(
PhabricatorLiskDAO $object,
$type,
PhabricatorApplicationTransaction $xaction,
$verb) {
$errors = array();
// NOTE: We use the ominpotent user for these checks because policy
// doesn't matter; existence does.
$other_doc_viewer = PhabricatorUser::getOmnipotentUser();
$ancestral_slugs = PhabricatorSlug::getAncestry($object->getSlug());
if ($ancestral_slugs) {
$ancestors = id(new PhrictionDocumentQuery())
->setViewer($other_doc_viewer)
->withSlugs($ancestral_slugs)
->execute();
$ancestors = mpull($ancestors, null, 'getSlug');
foreach ($ancestral_slugs as $slug) {
$ancestor_doc = idx($ancestors, $slug);
if (!$ancestor_doc) {
$create_uri = '/phriction/edit/?slug='.$slug;
$create_link = phutil_tag(
'a',
array(
'href' => $create_uri,
),
$slug);
switch ($verb) {
case self::VALIDATE_MOVE_ANCESTRY:
$message = pht(
'Can not move document because the parent document with '.
'slug %s does not exist!',
$create_link);
break;
case self::VALIDATE_CREATE_ANCESTRY:
$message = pht(
'Can not create document because the parent document with '.
'slug %s does not exist!',
$create_link);
break;
}
$error = new PhabricatorApplicationTransactionValidationError(
$type,
pht('Missing Ancestor'),
$message,
$xaction);
$errors[] = $error;
}
}
}
return $errors;
}
private function validateContentVersion(
PhabricatorLiskDAO $object,
$type,