Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15382472
D11049.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.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
@@ -22,16 +22,7 @@
$action = $request->getStr('action');
- // Compute new CCs added by @mentions. Several things can cause CCs to
- // be added as side effects: mentions, explicit CCs, users who aren't
- // CC'd interacting with the task, and ownership changes. We build up a
- // list of all the CCs and then construct a transaction for them at the
- // end if necessary.
- $added_ccs = PhabricatorMarkupEngine::extractPHIDsFromMentions(
- $user,
- array(
- $request->getStr('comments'),
- ));
+ $added_ccs = array();
$cc_transaction = new ManiphestTransaction();
$cc_transaction
@@ -67,7 +58,7 @@
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
// Accumulate the new explicit CCs into the array that we'll add in
// the CC transaction later.
- $added_ccs = array_merge($added_ccs, $request->getArr('ccs'));
+ $added_ccs = $request->getArr('ccs');
// Throw away the primary transaction.
$transaction = null;
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
Sat, Mar 15, 12:32 PM (3 d, 18 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7693133
Default Alt Text
D11049.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