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;