Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13176352
D11049.id26616.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
10 KB
Referenced Files
None
Subscribers
None
D11049.id26616.diff
View Options
diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -7,7 +7,7 @@
*/
return array(
'names' => array(
- 'core.pkg.css' => 'f588bfc3',
+ 'core.pkg.css' => 'd83f8c13',
'core.pkg.js' => '44aac665',
'darkconsole.pkg.js' => 'd326843f',
'differential.pkg.css' => '8af45893',
@@ -104,7 +104,7 @@
'rsrc/css/application/tokens/tokens.css' => '3d0f239e',
'rsrc/css/application/uiexample/example.css' => '528b19de',
'rsrc/css/core/core.css' => 'ca42b69f',
- 'rsrc/css/core/remarkup.css' => 'a2d3f9c4',
+ 'rsrc/css/core/remarkup.css' => '7604f12e',
'rsrc/css/core/syntax.css' => '56c1ba38',
'rsrc/css/core/z-index.css' => '44e1d311',
'rsrc/css/diviner/diviner-shared.css' => '38813222',
@@ -725,7 +725,7 @@
'phabricator-phtize' => 'd254d646',
'phabricator-prefab' => 'bbae734c',
'phabricator-profile-css' => '28f433ef',
- 'phabricator-remarkup-css' => 'a2d3f9c4',
+ 'phabricator-remarkup-css' => '7604f12e',
'phabricator-search-results-css' => 'f240504c',
'phabricator-shaped-request' => '7cbe244b',
'phabricator-side-menu-view-css' => '90eafc85',
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,27 +92,52 @@
}
$engine->setTextMetadata($mentioned_key, $mentioned);
+ $context_object = $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 ($context_object
+ && $context_object instanceof PhabricatorPolicyInterface) {
+ if (!PhabricatorPolicyFilter::hasCapability(
+ $user,
+ $context_object,
+ 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);
+
+ if ($user_has_no_permission) {
+ $colors = '
+ border-color: #92969D;
+ color: #92969D;
+ background-color: #F7F7F7;';
+ } else {
+ $colors = '
+ border-color: #f1f7ff;
+ color: #19558d;
+ background-color: #f1f7ff;';
+ }
+
$tag = phutil_tag(
'a',
array(
'href' => $user_href,
- 'style' => 'background-color: #f1f7ff;
- border-color: #f1f7ff;
+ 'style' => $colors.'
border: 1px solid transparent;
border-radius: 3px;
- color: #19558d;
font-weight: bold;
padding: 0 4px;',
),
@@ -124,6 +149,10 @@
->setName('@'.$user->getUserName())
->setHref($user_href);
+ if ($user_has_no_permission) {
+ $tag->addClass('phabricator-remarkup-mention-nopermission');
+ }
+
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 )------------------------------------------------ */
diff --git a/webroot/rsrc/css/core/remarkup.css b/webroot/rsrc/css/core/remarkup.css
--- a/webroot/rsrc/css/core/remarkup.css
+++ b/webroot/rsrc/css/core/remarkup.css
@@ -208,6 +208,11 @@
background: #ffaaaa;
}
+.phabricator-remarkup-mention-nopermission .phui-tag-core {
+ background: {$lightgreybackground};
+ color: {$lightgreytext};
+}
+
.phabricator-remarkup .remarkup-note {
margin: 16px 0;
padding: 12px;
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Thu, May 9, 1:03 PM (3 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6274865
Default Alt Text
D11049.id26616.diff (10 KB)
Attached To
Mode
D11049: Do not CC users without permissions to view an object
Attached
Detach File
Event Timeline
Log In to Comment