From 4811e6e7c1226d3c51649f47c2e7b49beb68aa19 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 9 Nov 2016 10:02:25 -0800 Subject: [PATCH] Require several advanced postgraduate degrees to understand object policies Summary: Fixes T11836. See some prior discussion in T8376#120613. The policy hint in headers in the UI is not exhaustive, and can not reasonably be exhaustive. For example, on a revision, it may say "All Users", but really mean "All users who can see the space this object is in and the repository it belongs to, plus the revision author and reviewers". These rules are explained if you click (and, often, in the documentation), but "All Users" is still at least somewhat misleading. I don't think there's any perfect solution here that balances the needs of both new and experienced users perfectly, but this change tries to do a bit better about avoiding cases where we say something very open (like "All Users") when the real policy is not very open. Specifically, I've made these changes to the header: - Spaces are now listed in the tag, so it will say `(S3 > All Users)` instead of `(All Users)`. They're already listed in the header, this just makes it more explicit that Spaces are a policy container and part of the view policy. - Extended policy objects are now listed in the tag, so it will say `(S3 > rARC > All Users)` for a revision in the Arcanist repository which is also in Space 3. - Objects can now provide a "Policy Codex", which is an object that represents a rulebook of more sophisticated policy descriptions. This codex can replace the tag with something else. - Imported calendar events now say "Uses Import Policy" instead of, e.g., "All Users". I've made these changes to the policy dialog: - Split it into more visually separate sections. - Added an explicit section for extended policies ("You must also have access to these other objects: ..."). - Broken the object policy rules into a "Special Rules" section (for rules like "you can only see a revision if you can see the repository it is part of") and an "Object Policy" section (for the actual object policy). - Tried to make it a little more readable? - The new policy dialogs are great to curl up with in front of a fire with a nice cup of cocoa. I've made these changes to infrastructure: - Implementing `PhabricatorPolicyInterface` no longer requires you to implement `describeAutomaticCapability()`. - Instead, implement `PhabricatorPolicyCodexInterface` and return a `PhabricatorPolicyCodex` object. - This "codex" is a policy rulebook which can set all the policy icons, labels, colors, rules, etc., to properly explain complex policies. - Broadly, the old method was usually either not useful (most objects have no special rules) or not powerful enough (objects with special rules often need to do more in order to explain them). Test Plan: {F1912860} {F1912861} {F1912862} {F1912863} Reviewers: chad Reviewed By: chad Subscribers: avivey Maniphest Tasks: T11836 Differential Revision: https://secure.phabricator.com/D16830 --- src/__phutil_library_map__.php | 10 + .../PhabricatorCalendarEventPolicyCodex.php | 80 +++++ .../storage/PhabricatorCalendarEvent.php | 19 +- .../storage/DifferentialRevision.php | 8 - .../policy/codex/PhabricatorPolicyCodex.php | 106 +++++++ .../codex/PhabricatorPolicyCodexInterface.php | 18 ++ .../PhabricatorPolicyCodexRuleDescription.php | 37 +++ .../PhabricatorPolicyExplainController.php | 295 +++++++++++++----- .../interface/PhabricatorPolicyInterface.php | 32 -- .../policy/storage/PhabricatorPolicy.php | 4 +- .../policy/view/PHUIPolicySectionView.php | 142 +++++++++ src/view/phui/PHUIHeaderView.php | 36 ++- webroot/rsrc/css/aphront/dialog-view.css | 4 + webroot/rsrc/css/phui/phui-header-view.css | 62 ++++ 14 files changed, 723 insertions(+), 130 deletions(-) create mode 100644 src/applications/calendar/codex/PhabricatorCalendarEventPolicyCodex.php create mode 100644 src/applications/policy/codex/PhabricatorPolicyCodex.php create mode 100644 src/applications/policy/codex/PhabricatorPolicyCodexInterface.php create mode 100644 src/applications/policy/codex/PhabricatorPolicyCodexRuleDescription.php create mode 100644 src/applications/policy/view/PHUIPolicySectionView.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a8901dadc1..071eef10e3 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1668,6 +1668,7 @@ phutil_register_library_map(array( 'PHUIPagerView' => 'view/phui/PHUIPagerView.php', 'PHUIPinboardItemView' => 'view/phui/PHUIPinboardItemView.php', 'PHUIPinboardView' => 'view/phui/PHUIPinboardView.php', + 'PHUIPolicySectionView' => 'applications/policy/view/PHUIPolicySectionView.php', 'PHUIPropertyGroupView' => 'view/phui/PHUIPropertyGroupView.php', 'PHUIPropertyListExample' => 'applications/uiexample/examples/PHUIPropertyListExample.php', 'PHUIPropertyListView' => 'view/phui/PHUIPropertyListView.php', @@ -2071,6 +2072,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarEventNameTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventNameTransaction.php', 'PhabricatorCalendarEventNotificationView' => 'applications/calendar/notifications/PhabricatorCalendarEventNotificationView.php', 'PhabricatorCalendarEventPHIDType' => 'applications/calendar/phid/PhabricatorCalendarEventPHIDType.php', + 'PhabricatorCalendarEventPolicyCodex' => 'applications/calendar/codex/PhabricatorCalendarEventPolicyCodex.php', 'PhabricatorCalendarEventQuery' => 'applications/calendar/query/PhabricatorCalendarEventQuery.php', 'PhabricatorCalendarEventRSVPEmailCommand' => 'applications/calendar/command/PhabricatorCalendarEventRSVPEmailCommand.php', 'PhabricatorCalendarEventRecurringTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php', @@ -3311,6 +3313,9 @@ phutil_register_library_map(array( 'PhabricatorPolicyCanViewCapability' => 'applications/policy/capability/PhabricatorPolicyCanViewCapability.php', 'PhabricatorPolicyCapability' => 'applications/policy/capability/PhabricatorPolicyCapability.php', 'PhabricatorPolicyCapabilityTestCase' => 'applications/policy/capability/__tests__/PhabricatorPolicyCapabilityTestCase.php', + 'PhabricatorPolicyCodex' => 'applications/policy/codex/PhabricatorPolicyCodex.php', + 'PhabricatorPolicyCodexInterface' => 'applications/policy/codex/PhabricatorPolicyCodexInterface.php', + 'PhabricatorPolicyCodexRuleDescription' => 'applications/policy/codex/PhabricatorPolicyCodexRuleDescription.php', 'PhabricatorPolicyConfigOptions' => 'applications/policy/config/PhabricatorPolicyConfigOptions.php', 'PhabricatorPolicyConstants' => 'applications/policy/constants/PhabricatorPolicyConstants.php', 'PhabricatorPolicyController' => 'applications/policy/controller/PhabricatorPolicyController.php', @@ -6447,6 +6452,7 @@ phutil_register_library_map(array( 'PHUIPagerView' => 'AphrontView', 'PHUIPinboardItemView' => 'AphrontView', 'PHUIPinboardView' => 'AphrontView', + 'PHUIPolicySectionView' => 'AphrontTagView', 'PHUIPropertyGroupView' => 'AphrontTagView', 'PHUIPropertyListExample' => 'PhabricatorUIExample', 'PHUIPropertyListView' => 'AphrontView', @@ -6870,6 +6876,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarDAO', 'PhabricatorPolicyInterface', 'PhabricatorExtendedPolicyInterface', + 'PhabricatorPolicyCodexInterface', 'PhabricatorProjectInterface', 'PhabricatorMarkupInterface', 'PhabricatorApplicationTransactionInterface', @@ -6923,6 +6930,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarEventNameTransaction' => 'PhabricatorCalendarEventTransactionType', 'PhabricatorCalendarEventNotificationView' => 'Phobject', 'PhabricatorCalendarEventPHIDType' => 'PhabricatorPHIDType', + 'PhabricatorCalendarEventPolicyCodex' => 'PhabricatorPolicyCodex', 'PhabricatorCalendarEventQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorCalendarEventRSVPEmailCommand' => 'PhabricatorCalendarEventEmailCommand', 'PhabricatorCalendarEventRecurringTransaction' => 'PhabricatorCalendarEventTransactionType', @@ -8361,6 +8369,8 @@ phutil_register_library_map(array( 'PhabricatorPolicyCanViewCapability' => 'PhabricatorPolicyCapability', 'PhabricatorPolicyCapability' => 'Phobject', 'PhabricatorPolicyCapabilityTestCase' => 'PhabricatorTestCase', + 'PhabricatorPolicyCodex' => 'Phobject', + 'PhabricatorPolicyCodexRuleDescription' => 'Phobject', 'PhabricatorPolicyConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPolicyConstants' => 'Phobject', 'PhabricatorPolicyController' => 'PhabricatorController', diff --git a/src/applications/calendar/codex/PhabricatorCalendarEventPolicyCodex.php b/src/applications/calendar/codex/PhabricatorCalendarEventPolicyCodex.php new file mode 100644 index 0000000000..97e21d901e --- /dev/null +++ b/src/applications/calendar/codex/PhabricatorCalendarEventPolicyCodex.php @@ -0,0 +1,80 @@ +getObject(); + + if (!$object->isImportedEvent()) { + return null; + } + + return pht('Uses Import Policy'); + } + + public function getPolicyIcon() { + $object = $this->getObject(); + + if (!$object->isImportedEvent()) { + return null; + } + + return 'fa-download'; + } + + public function getPolicyTagClasses() { + $object = $this->getObject(); + + if (!$object->isImportedEvent()) { + return array(); + } + + return array( + 'policy-adjusted-special', + ); + } + + public function getPolicySpecialRuleDescriptions() { + $object = $this->getObject(); + + $rules = array(); + $rules[] = $this->newRule() + ->setDescription( + pht('The host of an event can always view and edit it.')); + + $rules[] = $this->newRule() + ->setCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + )) + ->setDescription( + pht('Users who are invited to an event can always view it.')); + + + $rules[] = $this->newRule() + ->setCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + )) + ->setIsActive($object->isImportedEvent()) + ->setDescription( + pht( + 'Imported events can only be viewed by users who can view '. + 'the import source.')); + + $rules[] = $this->newRule() + ->setCapabilities( + array( + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->setIsActive($object->isImportedEvent()) + ->setDescription( + pht( + 'Imported events can not be edited in Phabricator.')); + + return $rules; + } + + +} diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index 58e3cf1dea..6d76ad6816 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -4,6 +4,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO implements PhabricatorPolicyInterface, PhabricatorExtendedPolicyInterface, + PhabricatorPolicyCodexInterface, PhabricatorProjectInterface, PhabricatorMarkupInterface, PhabricatorApplicationTransactionInterface, @@ -1217,18 +1218,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return false; } - public function describeAutomaticCapability($capability) { - if ($this->isImportedEvent()) { - return pht( - 'Events imported from external sources can not be edited in '. - 'Phabricator.'); - } - - return pht( - 'The host of an event can always view and edit it. Users who are '. - 'invited to an event can always view it.'); - } - /* -( PhabricatorExtendedPolicyInterface )--------------------------------- */ @@ -1251,6 +1240,12 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return $extended; } +/* -( PhabricatorPolicyCodexInterface )------------------------------------ */ + + public function newPolicyCodex() { + return new PhabricatorCalendarEventPolicyCodex(); + } + /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index 97d5a5e581..47443932e3 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -370,14 +370,6 @@ final class DifferentialRevision extends DifferentialDAO switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: - // NOTE: In Differential, an automatic capability on a revision (being - // an author) is sufficient to view it, even if you can not see the - // repository the revision belongs to. We can bail out early in this - // case. - if ($this->hasAutomaticCapability($capability, $viewer)) { - break; - } - $repository_phid = $this->getRepositoryPHID(); $repository = $this->getRepository(); diff --git a/src/applications/policy/codex/PhabricatorPolicyCodex.php b/src/applications/policy/codex/PhabricatorPolicyCodex.php new file mode 100644 index 0000000000..43a1cc5a1d --- /dev/null +++ b/src/applications/policy/codex/PhabricatorPolicyCodex.php @@ -0,0 +1,106 @@ +viewer = $viewer; + return $this; + } + + final public function getViewer() { + return $this->viewer; + } + + final public function setObject(PhabricatorPolicyCodexInterface $object) { + $this->object = $object; + return $this; + } + + final public function getObject() { + return $this->object; + } + + final public function setCapability($capability) { + $this->capability = $capability; + return $this; + } + + final public function getCapability() { + return $this->capability; + } + + final public function setPolicy(PhabricatorPolicy $policy) { + $this->policy = $policy; + return $this; + } + + final public function getPolicy() { + return $this->policy; + } + + final public static function newFromObject( + PhabricatorPolicyCodexInterface $object, + PhabricatorUser $viewer) { + + if (!($object instanceof PhabricatorPolicyInterface)) { + throw new Exception( + pht( + 'Object (of class "%s") implements interface "%s", but must also '. + 'implement interface "%s".', + get_class($object), + 'PhabricatorPolicyCodexInterface', + 'PhabricatorPolicyInterface')); + } + + $codex = $object->newPolicyCodex(); + if (!($codex instanceof PhabricatorPolicyCodex)) { + throw new Exception( + pht( + 'Object (of class "%s") implements interface "%s", but defines '. + 'method "%s" incorrectly: this method must return an object of '. + 'class "%s".', + get_class($object), + 'PhabricatorPolicyCodexInterface', + 'newPolicyCodex()', + __CLASS__)); + } + + $codex + ->setObject($object) + ->setViewer($viewer); + + return $codex; + } + +} diff --git a/src/applications/policy/codex/PhabricatorPolicyCodexInterface.php b/src/applications/policy/codex/PhabricatorPolicyCodexInterface.php new file mode 100644 index 0000000000..43f1e4c445 --- /dev/null +++ b/src/applications/policy/codex/PhabricatorPolicyCodexInterface.php @@ -0,0 +1,18 @@ +>PolicyCodex(); + } + +*/ diff --git a/src/applications/policy/codex/PhabricatorPolicyCodexRuleDescription.php b/src/applications/policy/codex/PhabricatorPolicyCodexRuleDescription.php new file mode 100644 index 0000000000..edc216fc66 --- /dev/null +++ b/src/applications/policy/codex/PhabricatorPolicyCodexRuleDescription.php @@ -0,0 +1,37 @@ +description = $description; + return $this; + } + + public function getDescription() { + return $this->description; + } + + public function setCapabilities(array $capabilities) { + $this->capabilities = $capabilities; + return $this; + } + + public function getCapabilities() { + return $this->capabilities; + } + + public function setIsActive($is_active) { + $this->isActive = $is_active; + return $this; + } + + public function getIsActive() { + return $this->isActive; + } + +} diff --git a/src/applications/policy/controller/PhabricatorPolicyExplainController.php b/src/applications/policy/controller/PhabricatorPolicyExplainController.php index 54d1902c36..5030c4d636 100644 --- a/src/applications/policy/controller/PhabricatorPolicyExplainController.php +++ b/src/applications/policy/controller/PhabricatorPolicyExplainController.php @@ -34,124 +34,118 @@ final class PhabricatorPolicyExplainController ->setViewer($viewer) ->withPHIDs(array($phid)) ->executeOne(); + + $object_name = $handle->getName(); $object_uri = nonempty($handle->getURI(), '/'); - $explanation = PhabricatorPolicy::getPolicyExplanation( - $viewer, - $policy->getPHID()); - - $auto_info = (array)$object->describeAutomaticCapability($capability); - - $auto_info = array_merge( - array($explanation), - $auto_info); - $auto_info = array_filter($auto_info); - - $capability_name = $capability; - $capobj = PhabricatorPolicyCapability::getCapabilityByKey($capability); - if ($capobj) { - $capability_name = $capobj->getCapabilityName(); - } - $dialog = id(new AphrontDialogView()) ->setUser($viewer) - ->setClass('aphront-access-dialog'); - - $this->appendSpaceInformation($dialog, $object, $policy, $capability); - - $intro = pht( - 'Users with the "%s" capability for this object:', - $capability_name); - - $object_name = pht( - '%s %s', - $handle->getTypeName(), - $handle->getObjectName()); - - $dialog + ->setClass('aphront-access-dialog') ->setTitle(pht('Policy Details: %s', $object_name)) - ->appendParagraph($intro) ->addCancelButton($object_uri, pht('Done')); - if ($auto_info) { - $dialog->appendList($auto_info); - } + $space_section = $this->buildSpaceSection( + $object, + $policy, + $capability); + + $extended_section = $this->buildExtendedSection( + $object, + $capability); + + $exceptions_section = $this->buildExceptionsSection( + $object, + $capability); + + $object_section = $this->buildObjectSection( + $object, + $policy, + $capability, + $handle); + + $dialog->appendChild( + array( + $space_section, + $extended_section, + $exceptions_section, + $object_section, + )); - $this->appendStrengthInformation($dialog, $object, $policy, $capability); return $dialog; } - private function appendSpaceInformation( - AphrontDialogView $dialog, + private function buildSpaceSection( PhabricatorPolicyInterface $object, PhabricatorPolicy $policy, $capability) { $viewer = $this->getViewer(); if (!($object instanceof PhabricatorSpacesInterface)) { - return; + return null; } if (!PhabricatorSpacesNamespaceQuery::getSpacesExist($viewer)) { - return; + return null; } - // NOTE: We're intentionally letting users through here, even if they only - // have access to one space. The intent is to help users in "space jail" - // understand who objects they create are visible to: - $space_phid = PhabricatorSpacesNamespaceQuery::getObjectSpacePHID( $object); - $handles = $viewer->loadHandles(array($space_phid)); - $doc_href = PhabricatorEnv::getDoclink('Spaces User Guide'); - - $dialog->appendParagraph( - array( - pht( - 'This object is in %s, and can only be seen or edited by users with '. - 'access to view objects in the space.', - $handles[$space_phid]->renderLink()), - ' ', - phutil_tag( - 'strong', - array(), - phutil_tag( - 'a', - array( - 'href' => $doc_href, - 'target' => '_blank', - ), - pht('Learn More'))), - )); - $spaces = PhabricatorSpacesNamespaceQuery::getViewerSpaces($viewer); $space = idx($spaces, $space_phid); if (!$space) { - return; + return null; } $space_policies = PhabricatorPolicyQuery::loadPolicies($viewer, $space); $space_policy = idx($space_policies, PhabricatorPolicyCapability::CAN_VIEW); if (!$space_policy) { - return; + return null; } + $doc_href = PhabricatorEnv::getDoclink('Spaces User Guide'); + $capability_name = $this->getCapabilityName($capability); + + $space_section = id(new PHUIPolicySectionView()) + ->setViewer($viewer) + ->setIcon('fa-th-large bluegrey') + ->setHeader(pht('Space')) + ->setDocumentationLink(pht('Spaces Documentation'), $doc_href) + ->appendList( + array( + array( + phutil_tag('strong', array(), pht('Space:')), + ' ', + $viewer->renderHandle($space_phid)->setAsTag(true), + ), + array( + phutil_tag('strong', array(), pht('%s:', $capability_name)), + ' ', + $space_policy->getShortName(), + ), + )) + ->appendParagraph( + pht( + 'This object is in %s and can only be seen or edited by users '. + 'with access to view objects in the space.', + $viewer->renderHandle($space_phid))); + $space_explanation = PhabricatorPolicy::getPolicyExplanation( $viewer, $space_policy->getPHID()); $items = array(); $items[] = $space_explanation; - $dialog->appendParagraph(pht('Users who can see objects in this space:')); - $dialog->appendList($items); + $space_section + ->appendParagraph(pht('Users who can see objects in this space:')) + ->appendList($items); $view_capability = PhabricatorPolicyCapability::CAN_VIEW; if ($capability == $view_capability) { $stronger = $space_policy->isStrongerThan($policy); if ($stronger) { - $dialog->appendParagraph( + $space_section->appendHint( pht( 'The space this object is in has a more restrictive view '. 'policy ("%s") than the object does ("%s"), so the space\'s '. @@ -161,14 +155,15 @@ final class PhabricatorPolicyExplainController } } - $dialog->appendParagraph( + $space_section->appendHint( pht( 'After a user passes space policy checks, they must still pass '. 'object policy checks.')); + + return $space_section; } - private function appendStrengthInformation( - AphrontDialogView $dialog, + private function getStrengthInformation( PhabricatorPolicyInterface $object, PhabricatorPolicy $policy, $capability) { @@ -206,7 +201,157 @@ final class PhabricatorPolicyExplainController $default_policy->getShortName()); } - $dialog->appendParagraph($info); + return $info; + } + + private function getCapabilityName($capability) { + $capability_name = $capability; + $capobj = PhabricatorPolicyCapability::getCapabilityByKey($capability); + if ($capobj) { + $capability_name = $capobj->getCapabilityName(); + } + + return $capability_name; + } + + private function buildExtendedSection( + PhabricatorPolicyInterface $object, + $capability) { + $viewer = $this->getViewer(); + + if (!($object instanceof PhabricatorExtendedPolicyInterface)) { + return null; + } + + $extended_rules = $object->getExtendedPolicy($capability, $viewer); + if (!$extended_rules) { + return null; + } + + $items = array(); + foreach ($extended_rules as $extended_rule) { + $extended_target = $extended_rule[0]; + $extended_capabilities = (array)$extended_rule[1]; + if (is_object($extended_target)) { + $extended_target = $extended_target->getPHID(); + } + + foreach ($extended_capabilities as $extended_capability) { + $ex_name = $this->getCapabilityName($extended_capability); + $items[] = array( + phutil_tag('strong', array(), pht('%s:', $ex_name)), + ' ', + $viewer->renderHandle($extended_target)->setAsTag(true), + ); + } + } + + return id(new PHUIPolicySectionView()) + ->setViewer($viewer) + ->setIcon('fa-link') + ->setHeader(pht('Required Capabilities on Other Objects')) + ->appendParagraph( + pht( + 'To access this object, users must have first have access '. + 'capabilties on these other objects:')) + ->appendList($items); + } + + private function buildExceptionsSection( + PhabricatorPolicyInterface $object, + $capability) { + $viewer = $this->getViewer(); + + if ($object instanceof PhabricatorPolicyCodexInterface) { + $codex = PhabricatorPolicyCodex::newFromObject($object, $viewer); + $rules = $codex->getPolicySpecialRuleDescriptions(); + + $exceptions = array(); + foreach ($rules as $rule) { + $is_active = $rule->getIsActive(); + if ($is_active) { + $rule_capabilities = $rule->getCapabilities(); + if ($rule_capabilities) { + if (!in_array($capability, $rule_capabilities)) { + $is_active = false; + } + } + } + + $description = $rule->getDescription(); + + if (!$is_active) { + $description = phutil_tag( + 'span', + array( + 'class' => 'phui-policy-section-view-inactive-rule', + ), + $description); + } + + $exceptions[] = $description; + } + } else if (method_exists($object, 'describeAutomaticCapability')) { + $exceptions = (array)$object->describeAutomaticCapability($capability); + $exceptions = array_filter($exceptions); + } else { + $exceptions = array(); + } + + if (!$exceptions) { + return null; + } + + + return id(new PHUIPolicySectionView()) + ->setViewer($viewer) + ->setIcon('fa-unlock-alt red') + ->setHeader(pht('Special Rules')) + ->appendParagraph( + pht( + 'This object has special rules which override normal object '. + 'policy rules:')) + ->appendList($exceptions); + } + + private function buildObjectSection( + PhabricatorPolicyInterface $object, + PhabricatorPolicy $policy, + $capability, + PhabricatorObjectHandle $handle) { + + $viewer = $this->getViewer(); + $capability_name = $this->getCapabilityName($capability); + + $object_section = id(new PHUIPolicySectionView()) + ->setViewer($viewer) + ->setIcon($handle->getIcon().' bluegrey') + ->setHeader(pht('Object Policy')) + ->appendList( + array( + array( + phutil_tag('strong', array(), pht('%s:', $capability_name)), + ' ', + $policy->getShortName(), + ), + )) + ->appendParagraph( + pht( + 'In detail, this means that these users can take this action, '. + 'provided they pass all of the checks described above first:')) + ->appendList( + array( + PhabricatorPolicy::getPolicyExplanation( + $viewer, + $policy->getPHID()), + )); + + $strength = $this->getStrengthInformation($object, $policy, $capability); + if ($strength) { + $object_section->appendHint($strength); + } + + return $object_section; } } diff --git a/src/applications/policy/interface/PhabricatorPolicyInterface.php b/src/applications/policy/interface/PhabricatorPolicyInterface.php index 515f0d1626..df93512f2f 100644 --- a/src/applications/policy/interface/PhabricatorPolicyInterface.php +++ b/src/applications/policy/interface/PhabricatorPolicyInterface.php @@ -6,34 +6,6 @@ interface PhabricatorPolicyInterface extends PhabricatorPHIDInterface { public function getPolicy($capability); public function hasAutomaticCapability($capability, PhabricatorUser $viewer); - /** - * Describe exceptions to an object's policy setting. - * - * The intent of this method is to explain and inform users about special - * cases which override configured policy settings. If this object has any - * such exceptions, explain them by returning one or more human-readable - * strings which describe the exception in a broad, categorical way. For - * example: - * - * - "The owner of an X can always view and edit it." - * - "Members of a Y can always view it." - * - * You can return `null`, a single string, or a list of strings. - * - * The relevant capability to explain (like "view") is passed as a parameter. - * You should tailor any messages to be relevant to that capability, although - * they do not need to exclusively describe the capability, and in some cases - * being more general ("The author can view and edit...") will be more clear. - * - * Messages should describe general rules, not specific objects, because the - * main goal is to teach the user the rules. For example, write "the author", - * not the specific author's name. - * - * @param const @{class:PhabricatorPolicyCapability} constant. - * @return wild Description of policy exceptions. See above. - */ - public function describeAutomaticCapability($capability); - } // TEMPLATE IMPLEMENTATION ///////////////////////////////////////////////////// @@ -58,8 +30,4 @@ interface PhabricatorPolicyInterface extends PhabricatorPHIDInterface { return false; } - public function describeAutomaticCapability($capability) { - return null; - } - */ diff --git a/src/applications/policy/storage/PhabricatorPolicy.php b/src/applications/policy/storage/PhabricatorPolicy.php index f2bfb8ce48..a7426ccad6 100644 --- a/src/applications/policy/storage/PhabricatorPolicy.php +++ b/src/applications/policy/storage/PhabricatorPolicy.php @@ -225,7 +225,9 @@ final class PhabricatorPolicy switch ($policy) { case PhabricatorPolicies::POLICY_PUBLIC: - return pht('This object is public.'); + return pht( + 'This object is public and can be viewed by anyone, even if they '. + 'do not have a Phabricator account.'); case PhabricatorPolicies::POLICY_USER: return pht('Logged in users can take this action.'); case PhabricatorPolicies::POLICY_ADMIN: diff --git a/src/applications/policy/view/PHUIPolicySectionView.php b/src/applications/policy/view/PHUIPolicySectionView.php new file mode 100644 index 0000000000..471e5035fb --- /dev/null +++ b/src/applications/policy/view/PHUIPolicySectionView.php @@ -0,0 +1,142 @@ +header = $header; + return $this; + } + + public function getHeader() { + return $this->header; + } + + public function setIcon($icon) { + $this->icon = $icon; + return $this; + } + + public function getIcon() { + return $this->icon; + } + + public function setDocumentationLink($name, $href) { + $link = phutil_tag( + 'a', + array( + 'href' => $href, + 'target' => '_blank', + ), + $name); + + $this->documentationLink = phutil_tag( + 'div', + array( + 'class' => 'phui-policy-section-view-link', + ), + array( + id(new PHUIIconView())->setIcon('fa-book'), + $link, + )); + + return $this; + } + + public function getDocumentationLink() { + return $this->documentationLink; + } + + public function appendList(array $items) { + foreach ($items as $key => $item) { + $items[$key] = phutil_tag( + 'li', + array( + 'class' => 'remarkup-list-item', + ), + $item); + } + + $list = phutil_tag( + 'ul', + array( + 'class' => 'remarkup-list', + ), + $items); + + return $this->appendChild($list); + } + + public function appendHint($content) { + $hint = phutil_tag( + 'p', + array( + 'class' => 'phui-policy-section-view-hint', + ), + array( + id(new PHUIIconView()) + ->setIcon('fa-sticky-note bluegrey'), + ' ', + pht('Note:'), + ' ', + $content, + )); + + return $this->appendChild($hint); + } + + public function appendParagraph($content) { + return $this->appendChild(phutil_tag('p', array(), $content)); + } + + protected function getTagAttributes() { + return array( + 'class' => 'phui-policy-section-view', + ); + } + + protected function getTagContent() { + require_celerity_resource('phui-header-view-css'); + + $icon_view = null; + $icon = $this->getIcon(); + if ($icon !== null) { + $icon_view = id(new PHUIIconView()) + ->setIcon($icon); + } + + $header_view = phutil_tag( + 'span', + array( + 'class' => 'phui-policy-section-view-header-text', + ), + $this->getHeader()); + + $header = phutil_tag( + 'div', + array( + 'class' => 'phui-policy-section-view-header', + ), + array( + $icon_view, + $header_view, + $this->getDocumentationLink(), + )); + + return array( + $header, + phutil_tag( + 'div', + array( + 'class' => 'phui-policy-section-view-body', + ), + $this->renderChildren()), + ); + } + + +} diff --git a/src/view/phui/PHUIHeaderView.php b/src/view/phui/PHUIHeaderView.php index e5e834bef5..0ae2c558d6 100644 --- a/src/view/phui/PHUIHeaderView.php +++ b/src/view/phui/PHUIHeaderView.php @@ -506,8 +506,40 @@ final class PHUIHeaderView extends AphrontTagView { } } + $policy_name = array($policy->getShortName()); + $policy_icon = $policy->getIcon().' bluegrey'; + + if ($object instanceof PhabricatorPolicyCodexInterface) { + $codex = PhabricatorPolicyCodex::newFromObject($object, $viewer); + + $codex_name = $codex->getPolicyShortName($policy, $view_capability); + if ($codex_name !== null) { + $policy_name = $codex_name; + } + + $codex_icon = $codex->getPolicyIcon($policy, $view_capability); + if ($codex_icon !== null) { + $policy_icon = $codex_icon; + } + + $codex_classes = $codex->getPolicyTagClasses($policy, $view_capability); + foreach ($codex_classes as $codex_class) { + $container_classes[] = $codex_class; + } + } + + if (!is_array($policy_name)) { + $policy_name = (array)$policy_name; + } + + $arrow = id(new PHUIIconView()) + ->setIcon('fa-angle-right') + ->addClass('policy-tier-separator'); + + $policy_name = phutil_implode_html($arrow, $policy_name); + $icon = id(new PHUIIconView()) - ->setIcon($policy->getIcon().' bluegrey'); + ->setIcon($policy_icon); $link = javelin_tag( 'a', @@ -516,7 +548,7 @@ final class PHUIHeaderView extends AphrontTagView { 'href' => '/policy/explain/'.$phid.'/'.$view_capability.'/', 'sigil' => 'workflow', ), - $policy->getShortName()); + $policy_name); return phutil_tag( 'span', diff --git a/webroot/rsrc/css/aphront/dialog-view.css b/webroot/rsrc/css/aphront/dialog-view.css index dd746dd5ee..a010c82b5a 100644 --- a/webroot/rsrc/css/aphront/dialog-view.css +++ b/webroot/rsrc/css/aphront/dialog-view.css @@ -128,6 +128,10 @@ width: 50%; } +.aphront-access-dialog .aphront-dialog-body { + padding: 0 12px; +} + .aphront-policy-rejection { font-weight: bold; } diff --git a/webroot/rsrc/css/phui/phui-header-view.css b/webroot/rsrc/css/phui/phui-header-view.css index e773668f0a..06d16f60b3 100644 --- a/webroot/rsrc/css/phui/phui-header-view.css +++ b/webroot/rsrc/css/phui/phui-header-view.css @@ -240,6 +240,26 @@ body .phui-header-shell.phui-bleed-header color: {$sh-orangetext}; } +.policy-header-callout.policy-adjusted-special { + background: {$sh-indigobackground}; +} + +.policy-header-callout.policy-adjusted-special .policy-link, +.policy-header-callout.policy-adjusted-special .phui-icon-view { + color: {$sh-indigotext}; +} + +.policy-header-callout .policy-space-container { + font-weight: bold; + color: {$sh-redtext}; +} + +.policy-header-callout .policy-tier-separator { + padding: 0 0 0 4px; + color: {$lightgreytext}; +} + + .phui-header-action-links .phui-mobile-menu { display: none; } @@ -333,3 +353,45 @@ body .phui-header-shell.phui-bleed-header .phui-header-view .phui-tag-shade-indigo a { color: {$sh-indigotext}; } + +.phui-policy-section-view { + margin-bottom: 24px; +} + +.phui-policy-section-view-header { + background: {$bluebackground}; + border-bottom: 1px solid {$lightblueborder}; + padding: 4px 8px; + color: {$darkbluetext}; + margin-bottom: 8px; +} + +.phui-policy-section-view-header-text { + font-weight: bold; +} + +.phui-policy-section-view-header .phui-icon-view { + margin-right: 8px; +} + +.phui-policy-section-view-link { + float: right; +} + +.phui-policy-section-view-link .phui-icon-view { + color: {$bluetext}; +} + +.phui-policy-section-view-hint { + color: {$greytext}; + background: {$lightbluebackground}; + padding: 8px; +} + +.phui-policy-section-view-body { + padding: 0 12px; +} + +.phui-policy-section-view-inactive-rule { + color: {$greytext}; +}