Page MenuHomePhabricator
Files F15382472

D11049.diff
No OneTemporary
Actions

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

Mime Type
text/plain
Expires
Sat, Mar 15, 12:32 PM (1 w, 3 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7693133
Default Alt Text
D11049.diff (10 KB)

Event Timeline

Phabricator · Built by Phacility