Page MenuHomePhabricator
Files F13180716

D11049.id26537.diff
No OneTemporary
Actions

D11049.id26537.diff
View Options

diff --git a/src/applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php b/src/applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php
--- a/src/applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php
+++ b/src/applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php
@@ -13,7 +13,8 @@
$output = PhabricatorMarkupEngine::renderOneObject(
$task,
ManiphestTask::MARKUP_FIELD_DESCRIPTION,
- $request->getUser());
+ $request->getUser(),
+ $task);
$content = phutil_tag_div('phabricator-remarkup', $output);
diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php
--- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php
+++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php
@@ -124,6 +124,7 @@
$engine = new PhabricatorMarkupEngine();
$engine->setViewer($user);
+ $engine->setContextObject($task);
$engine->addObject($task, ManiphestTask::MARKUP_FIELD_DESCRIPTION);
$timeline = $this->buildTransactionTimeline(
diff --git a/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php b/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php
--- a/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php
+++ b/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php
@@ -113,6 +113,7 @@
$engine = new PhabricatorMarkupEngine();
$engine->setViewer($user);
+ $engine->setContextObject($task);
if ($transaction->hasComment()) {
$engine->addObject(
$transaction->getComment(),
diff --git a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php
--- a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php
+++ b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php
@@ -147,10 +147,30 @@
$task->getSubscriberPHIDs()));
if ($added_ccs) {
- // We've added CCs, so include a CC transaction.
- $all_ccs = array_merge($task->getSubscriberPHIDs(), $added_ccs);
- $cc_transaction->setNewValue(array('=' => $all_ccs));
- $transactions[] = $cc_transaction;
+ $users = id(new PhabricatorPeopleQuery())
+ ->setViewer($user)
+ ->withPHIDs($added_ccs)
+ ->execute();
+ $users = mpull($users, null, 'getPHID');
+
+ // Check if the added_ccs have CAN_VIEW permission on the object
+ foreach ($added_ccs as $key => $added_cc) {
+ if ($task instanceof PhabricatorPolicyInterface
+ && !PhabricatorPolicyFilter::hasCapability(
+ $users[$added_cc],
+ $task,
+ PhabricatorPolicyCapability::CAN_VIEW)
+ ) {
+ unset($added_ccs[$key]);
+ }
+ }
+
+ if ($added_ccs) {
+ // We've added CCs, so include a CC transaction.
+ $all_ccs = array_merge($task->getSubscriberPHIDs(), $added_ccs);
+ $cc_transaction->setNewValue(array('=' => $all_ccs));
+ $transactions[] = $cc_transaction;
+ }
}
$comments = $request->getStr('comments');
diff --git a/src/applications/people/markup/PhabricatorMentionRemarkupRule.php b/src/applications/people/markup/PhabricatorMentionRemarkupRule.php
--- a/src/applications/people/markup/PhabricatorMentionRemarkupRule.php
+++ b/src/applications/people/markup/PhabricatorMentionRemarkupRule.php
@@ -92,18 +92,33 @@
}
$engine->setTextMetadata($mentioned_key, $mentioned);
+ $contextObject = $engine->getConfig('contextObject');
foreach ($metadata as $username => $tokens) {
$exists = isset($actual_users[$username]);
+ $user_has_no_permission = false;
if ($exists) {
$user = $actual_users[$username];
Javelin::initBehavior('phabricator-hovercards');
+ // Check if the user has view access to the object she was mentioned in
+ if ($contextObject
+ && $contextObject instanceof PhabricatorPolicyInterface) {
+ if (!PhabricatorPolicyFilter::hasCapability(
+ $user,
+ $contextObject,
+ PhabricatorPolicyCapability::CAN_VIEW)) {
+ // User mentioned has no permission to this object
+ $user_has_no_permission = true;
+ }
+ }
+
$user_href = '/p/'.$user->getUserName().'/';
if ($engine->isHTMLMailMode()) {
$user_href = PhabricatorEnv::getProductionURI($user_href);
+ // FIXME: Change the style for users without permission here too
$tag = phutil_tag(
'a',
array(
@@ -124,6 +139,10 @@
->setName('@'.$user->getUserName())
->setHref($user_href);
+ if ($user_has_no_permission) {
+ $tag->setBackgroundColor(PHUITagView::COLOR_GREY);
+ }
+
if (!$user->isUserActivated()) {
$tag->setDotColor(PHUITagView::COLOR_GREY);
} else {
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -1053,13 +1053,32 @@
$phids = array_diff($phids, $this->subscribers);
}
- foreach ($phids as $key => $phid) {
- if ($object->isAutomaticallySubscribed($phid)) {
- unset($phids[$key]);
+ if ($phids) {
+ $users = id(new PhabricatorPeopleQuery())
+ ->setViewer($this->getActor())
+ ->withPHIDs($phids)
+ ->execute();
+ $users = mpull($users, null, 'getPHID');
+
+ foreach ($phids as $key => $phid) {
+ // Do not subscribe mentioned users
+ // who do not have VIEW Permissions
+ if ($object instanceof PhabricatorPolicyInterface
+ && !PhabricatorPolicyFilter::hasCapability(
+ $users[$phid],
+ $object,
+ PhabricatorPolicyCapability::CAN_VIEW)
+ ) {
+ unset($phids[$key]);
+ } else {
+ if ($object->isAutomaticallySubscribed($phid)) {
+ unset($phids[$key]);
+ }
+ }
}
+ $phids = array_values($phids);
}
- $phids = array_values($phids);
-
+ // No else here to properly return null should we unset all subscriber
if (!$phids) {
return null;
}
diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php
--- a/src/infrastructure/markup/PhabricatorMarkupEngine.php
+++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php
@@ -41,6 +41,7 @@
private $objects = array();
private $viewer;
+ private $contextObject;
private $version = 14;
@@ -54,15 +55,18 @@
* @param PhabricatorMarkupInterface The object to render.
* @param string The field to render.
* @param PhabricatorUser User viewing the markup.
+ * @param object A context object for policy checks
* @return string Marked up output.
* @task markup
*/
public static function renderOneObject(
PhabricatorMarkupInterface $object,
$field,
- PhabricatorUser $viewer) {
+ PhabricatorUser $viewer,
+ $context_object = null) {
return id(new PhabricatorMarkupEngine())
->setViewer($viewer)
+ ->setContextObject($context_object)
->addObject($object, $field)
->process()
->getOutput($object, $field);
@@ -116,6 +120,7 @@
foreach ($objects as $key => $info) {
$engines[$key] = $info['object']->newMarkupEngine($info['field']);
$engines[$key]->setConfig('viewer', $this->viewer);
+ $engines[$key]->setConfig('contextObject', $this->contextObject);
}
// Load or build the preprocessor caches.
@@ -290,6 +295,18 @@
return $this;
}
+ /**
+ * Set the context object. Used to implement object permissions.
+ *
+ * @param The object in which context this remarkup is used.
+ * @return this
+ * @task markup
+ */
+ public function setContextObject($object) {
+ $this->contextObject = $object;
+ return $this;
+ }
+
/* -( Engine Construction )------------------------------------------------ */

File Metadata

Mime Type
text/plain
Expires
Fri, May 10, 2:59 AM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6281436
Default Alt Text
D11049.id26537.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility