From c00cd5c2a31e22cfdbe8c39cce2242a1f0e0df94 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 28 Jan 2016 07:01:17 -0800 Subject: [PATCH] Make hidden and locked configuration even more explicit Summary: A user in IRC seemed very confused by this, and worked extremely hard to shoot themsevles in the foot by manually writing locked configuration to the database. Try to explain why configuration is locked better. Test Plan: Mostly reading. {F1078905} {F1078906} Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D15128 --- .../PhabricatorConfigEditController.php | 72 ++++++++++--- ...PhabricatorConfigManagementSetWorkflow.php | 7 +- .../configuration_locked.diviner | 101 ++++++++++++++++++ 3 files changed, 161 insertions(+), 19 deletions(-) create mode 100644 src/docs/user/configuration/configuration_locked.diviner diff --git a/src/applications/config/controller/PhabricatorConfigEditController.php b/src/applications/config/controller/PhabricatorConfigEditController.php index 370ba01f37..0c93c86f09 100644 --- a/src/applications/config/controller/PhabricatorConfigEditController.php +++ b/src/applications/config/controller/PhabricatorConfigEditController.php @@ -104,22 +104,43 @@ final class PhabricatorConfigEditController if ($errors) { $error_view = id(new PHUIInfoView()) ->setErrors($errors); - } else if ($option->getHidden()) { - $msg = pht( + } + + $status_items = array(); + if ($option->getHidden()) { + $message = pht( 'This configuration is hidden and can not be edited or viewed from '. 'the web interface.'); - $error_view = id(new PHUIInfoView()) - ->setTitle(pht('Configuration Hidden')) - ->setSeverity(PHUIInfoView::SEVERITY_WARNING) - ->appendChild(phutil_tag('p', array(), $msg)); + $status_items[] = id(new PHUIStatusItemView()) + ->setIcon('fa-eye-slash red') + ->setTarget(phutil_tag('strong', array(), pht('Configuration Hidden'))) + ->setNote($message); } else if ($option->getLocked()) { + $message = $option->getLockedMessage(); - $msg = $option->getLockedMessage(); - $error_view = id(new PHUIInfoView()) - ->setTitle(pht('Configuration Locked')) - ->setSeverity(PHUIInfoView::SEVERITY_NOTICE) - ->appendChild(phutil_tag('p', array(), $msg)); + $status_items[] = id(new PHUIStatusItemView()) + ->setIcon('fa-lock red') + ->setTarget(phutil_tag('strong', array(), pht('Configuration Locked'))) + ->setNote($message); + } + + if ($status_items) { + $doc_href = PhabricatorEnv::getDoclink( + 'Configuration Guide: Locked and Hidden Configuration'); + + $doc_link = phutil_tag( + 'a', + array( + 'href' => $doc_href, + 'target' => '_blank', + ), + pht('Configuration Guide: Locked and Hidden Configuration')); + + $status_items[] = id(new PHUIStatusItemView()) + ->setIcon('fa-book') + ->setTarget(phutil_tag('strong', array(), pht('Learn More'))) + ->setNote($doc_link); } if ($option->getHidden() || $option->getLocked()) { @@ -144,11 +165,30 @@ final class PhabricatorConfigEditController $form ->setUser($viewer) - ->addHiddenInput('issue', $request->getStr('issue')) - ->appendChild( + ->addHiddenInput('issue', $request->getStr('issue')); + + if ($status_items) { + $status_view = id(new PHUIStatusListView()); + + foreach ($status_items as $status_item) { + $status_view->addItem($status_item); + } + + $form->appendControl( id(new AphrontFormMarkupControl()) - ->setLabel(pht('Description')) - ->setValue($description)); + ->setValue($status_view)); + } + + $description = $option->getDescription(); + if (strlen($description)) { + $description_view = new PHUIRemarkupView($viewer, $description); + + $form + ->appendChild( + id(new AphrontFormMarkupControl()) + ->setLabel(pht('Description')) + ->setValue($description_view)); + } if ($group) { $extra = $group->renderContextualDescription( @@ -195,7 +235,7 @@ final class PhabricatorConfigEditController ->setForm($form); if ($error_view) { - $form_box->setInfoView($error_view); + $form_box->setInfoView($error_view); } $crumbs = $this->buildApplicationCrumbs(); diff --git a/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php b/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php index 97586a77dd..f6d40a5126 100644 --- a/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php +++ b/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php @@ -141,9 +141,10 @@ final class PhabricatorConfigManagementSetWorkflow if ($option->getLocked() && $use_database) { throw new PhutilArgumentUsageException( pht( - "Config key '%s' is locked and can only be set in local ". - "configuration.", - $key)); + 'Config key "%s" is locked and can only be set in local '. + 'configuration. To learn more, see "%s" in the documentation.', + $key, + pht('Configuration Guide: Locked and Hidden Configuration'))); } try { diff --git a/src/docs/user/configuration/configuration_locked.diviner b/src/docs/user/configuration/configuration_locked.diviner new file mode 100644 index 0000000000..040b838177 --- /dev/null +++ b/src/docs/user/configuration/configuration_locked.diviner @@ -0,0 +1,101 @@ +@title Configuration Guide: Locked and Hidden Configuration +@group config + +Details about locked and hidden configuration. + + +Overview +======== + +Some configuration options are **Locked** or **Hidden**. If an option has one +of these attributes, it means: + + - **Locked Configuration**: This setting can not be written from the web UI. + - **Hidden Configuration**: This setting can not be read or written from + the web UI. + +This document explains these attributes in more detail. + + +Locked Configuration +==================== + +**Locked Configuration** can not be edited from the web UI. In general, you +can edit it from the CLI instead, with `bin/config`: + +``` +phabricator/ $ ./bin/config set +``` + +A few settings have alternate CLI tools. Refer to the setting page for +details. + +Note that these settings can not be written to the database, even from the +CLI. + +Locked values can not be unlocked: they are locked because of what the setting +does or how the setting operates. Some of the reasons configuration options are +locked include: + + +**Required for bootstrapping**: Some options, like `mysql.host`, must be +available before Phabricator can read configuration from the database. + +If you stored `mysql.host` only in the database, Phabricator would not know how +to connect to the database in order to read the value in the first place. + +These options must be provided in a configuration source which is read earlier +in the bootstrapping process, before Phabricator connects to the database. + + +**Errors could not be fixed from the web UI**: Some options, like +`phabricator.base-uri`, can effectively disable the web UI if they are +configured incorrectly. + +If these options could be configured from the web UI, you could not fix them if +you made a mistake (because the web UI would no longer work, so you could not +load the page to change the value). + +We require these options to be edited from the CLI to make sure the editor has +access to fix any mistakes. + + +**Attackers could gain greater access**: Some options could be modified by an +attacker who has gained access to an administrator account in order to gain +greater access. + +For example, an attacker who could modify `metamta.mail-adapter` (and other +similar options), could potentially reconfigure Phabricator to send mail +through an evil server they controlled, then trigger password resets on other +user accounts to compromise them. + +We require these options to be edited from the CLI to make sure the editor +has full access to the install. + + +Hidden Configuration +==================== + +**Hidden Configuration** is similar to locked configuration, but also can not +be //read// from the web UI. + +In almost all cases, configuration is hidden because it is some sort of secret +key or access token for an external service. These values are hidden from the +web UI to prevent administrators (or attackers who have compromised +administrator accounts) from reading them. + +You can review (and edit) hidden configuration from the CLI: + +``` +phabricator/ $ ./bin/config get +phabricator/ $ ./bin/config set + +``` + + +Next Steps +========== + +Continue by: + + - returning to the @{article: Configuration Guide}.