diff --git a/resources/sql/autopatches/20190127.project.01.subtype.sql b/resources/sql/autopatches/20190127.project.01.subtype.sql new file mode 100644 index 0000000000..107f9202d4 --- /dev/null +++ b/resources/sql/autopatches/20190127.project.01.subtype.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_project.project + ADD subtype VARCHAR(64) COLLATE {$COLLATE_TEXT} NOT NULL; diff --git a/resources/sql/autopatches/20190127.project.02.default.sql b/resources/sql/autopatches/20190127.project.02.default.sql new file mode 100644 index 0000000000..1a74506cf7 --- /dev/null +++ b/resources/sql/autopatches/20190127.project.02.default.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_project.project + SET subtype = 'default' WHERE subtype = ''; diff --git a/resources/sql/autopatches/20190129.project.01.spaces.php b/resources/sql/autopatches/20190129.project.01.spaces.php new file mode 100644 index 0000000000..845b4ff25d --- /dev/null +++ b/resources/sql/autopatches/20190129.project.01.spaces.php @@ -0,0 +1,18 @@ +establishConnection('w'); +$table_name = $table->getTableName(); + +foreach (new LiskRawMigrationIterator($conn, $table_name) as $project_row) { + queryfx( + $conn, + 'UPDATE %R SET spacePHID = %ns + WHERE parentProjectPHID = %s AND milestoneNumber IS NOT NULL', + $table, + $project_row['spacePHID'], + $project_row['phid']); +} diff --git a/scripts/mail/mail_handler.php b/scripts/mail/mail_handler.php index 1c3c71f305..bf6f315f3a 100755 --- a/scripts/mail/mail_handler.php +++ b/scripts/mail/mail_handler.php @@ -55,8 +55,8 @@ foreach (array('text', 'html') as $part) { } $headers = $parser->getHeaders(); -$headers['subject'] = iconv_mime_decode($headers['subject'], 0, 'UTF-8'); -$headers['from'] = iconv_mime_decode($headers['from'], 0, 'UTF-8'); +$headers['subject'] = phutil_decode_mime_header($headers['subject']); +$headers['from'] = phutil_decode_mime_header($headers['from']); if ($args->getArg('process-duplicates')) { $headers['message-id'] = Filesystem::readRandomCharacters(64); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 593c526218..36b4442065 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -183,7 +183,6 @@ phutil_register_library_map(array( 'AphrontCalendarEventView' => 'applications/calendar/view/AphrontCalendarEventView.php', 'AphrontController' => 'aphront/AphrontController.php', 'AphrontCursorPagerView' => 'view/control/AphrontCursorPagerView.php', - 'AphrontDefaultApplicationConfiguration' => 'aphront/configuration/AphrontDefaultApplicationConfiguration.php', 'AphrontDialogResponse' => 'aphront/response/AphrontDialogResponse.php', 'AphrontDialogView' => 'view/AphrontDialogView.php', 'AphrontEpochHTTPParameterType' => 'aphront/httpparametertype/AphrontEpochHTTPParameterType.php', @@ -2231,18 +2230,26 @@ phutil_register_library_map(array( 'PhabricatorAuthFactorConfigQuery' => 'applications/auth/query/PhabricatorAuthFactorConfigQuery.php', 'PhabricatorAuthFactorProvider' => 'applications/auth/storage/PhabricatorAuthFactorProvider.php', 'PhabricatorAuthFactorProviderController' => 'applications/auth/controller/mfa/PhabricatorAuthFactorProviderController.php', + 'PhabricatorAuthFactorProviderDuoCredentialTransaction' => 'applications/auth/xaction/PhabricatorAuthFactorProviderDuoCredentialTransaction.php', + 'PhabricatorAuthFactorProviderDuoEnrollTransaction' => 'applications/auth/xaction/PhabricatorAuthFactorProviderDuoEnrollTransaction.php', + 'PhabricatorAuthFactorProviderDuoHostnameTransaction' => 'applications/auth/xaction/PhabricatorAuthFactorProviderDuoHostnameTransaction.php', + 'PhabricatorAuthFactorProviderDuoUsernamesTransaction' => 'applications/auth/xaction/PhabricatorAuthFactorProviderDuoUsernamesTransaction.php', 'PhabricatorAuthFactorProviderEditController' => 'applications/auth/controller/mfa/PhabricatorAuthFactorProviderEditController.php', 'PhabricatorAuthFactorProviderEditEngine' => 'applications/auth/editor/PhabricatorAuthFactorProviderEditEngine.php', 'PhabricatorAuthFactorProviderEditor' => 'applications/auth/editor/PhabricatorAuthFactorProviderEditor.php', + 'PhabricatorAuthFactorProviderEnrollMessageTransaction' => 'applications/auth/xaction/PhabricatorAuthFactorProviderEnrollMessageTransaction.php', 'PhabricatorAuthFactorProviderListController' => 'applications/auth/controller/mfa/PhabricatorAuthFactorProviderListController.php', + 'PhabricatorAuthFactorProviderMFAEngine' => 'applications/auth/engine/PhabricatorAuthFactorProviderMFAEngine.php', + 'PhabricatorAuthFactorProviderMessageController' => 'applications/auth/controller/mfa/PhabricatorAuthFactorProviderMessageController.php', 'PhabricatorAuthFactorProviderNameTransaction' => 'applications/auth/xaction/PhabricatorAuthFactorProviderNameTransaction.php', 'PhabricatorAuthFactorProviderQuery' => 'applications/auth/query/PhabricatorAuthFactorProviderQuery.php', + 'PhabricatorAuthFactorProviderStatus' => 'applications/auth/constants/PhabricatorAuthFactorProviderStatus.php', + 'PhabricatorAuthFactorProviderStatusTransaction' => 'applications/auth/xaction/PhabricatorAuthFactorProviderStatusTransaction.php', 'PhabricatorAuthFactorProviderTransaction' => 'applications/auth/storage/PhabricatorAuthFactorProviderTransaction.php', 'PhabricatorAuthFactorProviderTransactionQuery' => 'applications/auth/query/PhabricatorAuthFactorProviderTransactionQuery.php', 'PhabricatorAuthFactorProviderTransactionType' => 'applications/auth/xaction/PhabricatorAuthFactorProviderTransactionType.php', 'PhabricatorAuthFactorProviderViewController' => 'applications/auth/controller/mfa/PhabricatorAuthFactorProviderViewController.php', 'PhabricatorAuthFactorResult' => 'applications/auth/factor/PhabricatorAuthFactorResult.php', - 'PhabricatorAuthFactorResultException' => 'applications/auth/exception/PhabricatorAuthFactorResultException.php', 'PhabricatorAuthFactorTestCase' => 'applications/auth/factor/__tests__/PhabricatorAuthFactorTestCase.php', 'PhabricatorAuthFinishController' => 'applications/auth/controller/PhabricatorAuthFinishController.php', 'PhabricatorAuthHMACKey' => 'applications/auth/storage/PhabricatorAuthHMACKey.php', @@ -2802,6 +2809,7 @@ phutil_register_library_map(array( 'PhabricatorCountdownTransactionType' => 'applications/countdown/xaction/PhabricatorCountdownTransactionType.php', 'PhabricatorCountdownView' => 'applications/countdown/view/PhabricatorCountdownView.php', 'PhabricatorCountdownViewController' => 'applications/countdown/controller/PhabricatorCountdownViewController.php', + 'PhabricatorCredentialEditField' => 'applications/transactions/editfield/PhabricatorCredentialEditField.php', 'PhabricatorCursorPagedPolicyAwareQuery' => 'infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php', 'PhabricatorCustomField' => 'infrastructure/customfield/field/PhabricatorCustomField.php', 'PhabricatorCustomFieldApplicationSearchAnyFunctionDatasource' => 'infrastructure/customfield/datasource/PhabricatorCustomFieldApplicationSearchAnyFunctionDatasource.php', @@ -2988,6 +2996,7 @@ phutil_register_library_map(array( 'PhabricatorDraftEngine' => 'applications/transactions/draft/PhabricatorDraftEngine.php', 'PhabricatorDraftInterface' => 'applications/transactions/draft/PhabricatorDraftInterface.php', 'PhabricatorDrydockApplication' => 'applications/drydock/application/PhabricatorDrydockApplication.php', + 'PhabricatorDuoAuthFactor' => 'applications/auth/factor/PhabricatorDuoAuthFactor.php', 'PhabricatorDuoFuture' => 'applications/auth/future/PhabricatorDuoFuture.php', 'PhabricatorEdgeChangeRecord' => 'infrastructure/edges/util/PhabricatorEdgeChangeRecord.php', 'PhabricatorEdgeChangeRecordTestCase' => 'infrastructure/edges/__tests__/PhabricatorEdgeChangeRecordTestCase.php', @@ -3057,6 +3066,8 @@ phutil_register_library_map(array( 'PhabricatorEditPage' => 'applications/transactions/editengine/PhabricatorEditPage.php', 'PhabricatorEditType' => 'applications/transactions/edittype/PhabricatorEditType.php', 'PhabricatorEditor' => 'infrastructure/PhabricatorEditor.php', + 'PhabricatorEditorExtension' => 'applications/transactions/engineextension/PhabricatorEditorExtension.php', + 'PhabricatorEditorExtensionModule' => 'applications/transactions/engineextension/PhabricatorEditorExtensionModule.php', 'PhabricatorEditorMailEngineExtension' => 'applications/transactions/engineextension/PhabricatorEditorMailEngineExtension.php', 'PhabricatorEditorMultipleSetting' => 'applications/settings/setting/PhabricatorEditorMultipleSetting.php', 'PhabricatorEditorSetting' => 'applications/settings/setting/PhabricatorEditorSetting.php', @@ -4110,6 +4121,8 @@ phutil_register_library_map(array( 'PhabricatorProjectSubprojectWarningController' => 'applications/project/controller/PhabricatorProjectSubprojectWarningController.php', 'PhabricatorProjectSubprojectsController' => 'applications/project/controller/PhabricatorProjectSubprojectsController.php', 'PhabricatorProjectSubprojectsProfileMenuItem' => 'applications/project/menuitem/PhabricatorProjectSubprojectsProfileMenuItem.php', + 'PhabricatorProjectSubtypeDatasource' => 'applications/project/typeahead/PhabricatorProjectSubtypeDatasource.php', + 'PhabricatorProjectSubtypesConfigType' => 'applications/project/config/PhabricatorProjectSubtypesConfigType.php', 'PhabricatorProjectTestDataGenerator' => 'applications/project/lipsum/PhabricatorProjectTestDataGenerator.php', 'PhabricatorProjectTransaction' => 'applications/project/storage/PhabricatorProjectTransaction.php', 'PhabricatorProjectTransactionEditor' => 'applications/project/editor/PhabricatorProjectTransactionEditor.php', @@ -5624,7 +5637,6 @@ phutil_register_library_map(array( 'AphrontCalendarEventView' => 'AphrontView', 'AphrontController' => 'Phobject', 'AphrontCursorPagerView' => 'AphrontView', - 'AphrontDefaultApplicationConfiguration' => 'AphrontApplicationConfiguration', 'AphrontDialogResponse' => 'AphrontResponse', 'AphrontDialogView' => array( 'AphrontView', @@ -7963,20 +7975,29 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionInterface', 'PhabricatorPolicyInterface', 'PhabricatorExtendedPolicyInterface', + 'PhabricatorEditEngineMFAInterface', ), 'PhabricatorAuthFactorProviderController' => 'PhabricatorAuthProviderController', + 'PhabricatorAuthFactorProviderDuoCredentialTransaction' => 'PhabricatorAuthFactorProviderTransactionType', + 'PhabricatorAuthFactorProviderDuoEnrollTransaction' => 'PhabricatorAuthFactorProviderTransactionType', + 'PhabricatorAuthFactorProviderDuoHostnameTransaction' => 'PhabricatorAuthFactorProviderTransactionType', + 'PhabricatorAuthFactorProviderDuoUsernamesTransaction' => 'PhabricatorAuthFactorProviderTransactionType', 'PhabricatorAuthFactorProviderEditController' => 'PhabricatorAuthFactorProviderController', 'PhabricatorAuthFactorProviderEditEngine' => 'PhabricatorEditEngine', 'PhabricatorAuthFactorProviderEditor' => 'PhabricatorApplicationTransactionEditor', + 'PhabricatorAuthFactorProviderEnrollMessageTransaction' => 'PhabricatorAuthFactorProviderTransactionType', 'PhabricatorAuthFactorProviderListController' => 'PhabricatorAuthProviderController', + 'PhabricatorAuthFactorProviderMFAEngine' => 'PhabricatorEditEngineMFAEngine', + 'PhabricatorAuthFactorProviderMessageController' => 'PhabricatorAuthFactorProviderController', 'PhabricatorAuthFactorProviderNameTransaction' => 'PhabricatorAuthFactorProviderTransactionType', 'PhabricatorAuthFactorProviderQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorAuthFactorProviderStatus' => 'Phobject', + 'PhabricatorAuthFactorProviderStatusTransaction' => 'PhabricatorAuthFactorProviderTransactionType', 'PhabricatorAuthFactorProviderTransaction' => 'PhabricatorModularTransaction', 'PhabricatorAuthFactorProviderTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorAuthFactorProviderTransactionType' => 'PhabricatorModularTransactionType', 'PhabricatorAuthFactorProviderViewController' => 'PhabricatorAuthFactorProviderController', 'PhabricatorAuthFactorResult' => 'Phobject', - 'PhabricatorAuthFactorResultException' => 'Exception', 'PhabricatorAuthFactorTestCase' => 'PhabricatorTestCase', 'PhabricatorAuthFinishController' => 'PhabricatorAuthController', 'PhabricatorAuthHMACKey' => 'PhabricatorAuthDAO', @@ -8639,6 +8660,7 @@ phutil_register_library_map(array( 'PhabricatorCountdownTransactionType' => 'PhabricatorModularTransactionType', 'PhabricatorCountdownView' => 'AphrontView', 'PhabricatorCountdownViewController' => 'PhabricatorCountdownController', + 'PhabricatorCredentialEditField' => 'PhabricatorEditField', 'PhabricatorCursorPagedPolicyAwareQuery' => 'PhabricatorPolicyAwareQuery', 'PhabricatorCustomField' => 'Phobject', 'PhabricatorCustomFieldApplicationSearchAnyFunctionDatasource' => 'PhabricatorTypeaheadDatasource', @@ -8843,6 +8865,7 @@ phutil_register_library_map(array( 'PhabricatorDraftDAO' => 'PhabricatorLiskDAO', 'PhabricatorDraftEngine' => 'Phobject', 'PhabricatorDrydockApplication' => 'PhabricatorApplication', + 'PhabricatorDuoAuthFactor' => 'PhabricatorAuthFactor', 'PhabricatorDuoFuture' => 'FutureProxy', 'PhabricatorEdgeChangeRecord' => 'Phobject', 'PhabricatorEdgeChangeRecordTestCase' => 'PhabricatorTestCase', @@ -8919,6 +8942,8 @@ phutil_register_library_map(array( 'PhabricatorEditPage' => 'Phobject', 'PhabricatorEditType' => 'Phobject', 'PhabricatorEditor' => 'Phobject', + 'PhabricatorEditorExtension' => 'Phobject', + 'PhabricatorEditorExtensionModule' => 'PhabricatorConfigModule', 'PhabricatorEditorMailEngineExtension' => 'PhabricatorMailEngineExtension', 'PhabricatorEditorMultipleSetting' => 'PhabricatorSelectSetting', 'PhabricatorEditorSetting' => 'PhabricatorStringSetting', @@ -10023,6 +10048,7 @@ phutil_register_library_map(array( 'PhabricatorConduitResultInterface', 'PhabricatorColumnProxyInterface', 'PhabricatorSpacesInterface', + 'PhabricatorEditEngineSubtypeInterface', ), 'PhabricatorProjectAddHeraldAction' => 'PhabricatorProjectHeraldAction', 'PhabricatorProjectApplication' => 'PhabricatorApplication', @@ -10150,6 +10176,8 @@ phutil_register_library_map(array( 'PhabricatorProjectSubprojectWarningController' => 'PhabricatorProjectController', 'PhabricatorProjectSubprojectsController' => 'PhabricatorProjectController', 'PhabricatorProjectSubprojectsProfileMenuItem' => 'PhabricatorProfileMenuItem', + 'PhabricatorProjectSubtypeDatasource' => 'PhabricatorTypeaheadDatasource', + 'PhabricatorProjectSubtypesConfigType' => 'PhabricatorJSONConfigType', 'PhabricatorProjectTestDataGenerator' => 'PhabricatorTestDataGenerator', 'PhabricatorProjectTransaction' => 'PhabricatorModularTransaction', 'PhabricatorProjectTransactionEditor' => 'PhabricatorApplicationTransactionEditor', diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php index 60b12557c9..8d36bbc880 100644 --- a/src/aphront/configuration/AphrontApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontApplicationConfiguration.php @@ -5,55 +5,81 @@ * @task response Response Handling * @task exception Exception Handling */ -abstract class AphrontApplicationConfiguration extends Phobject { +final class AphrontApplicationConfiguration + extends Phobject { private $request; private $host; private $path; private $console; - abstract public function buildRequest(); - abstract public function build404Controller(); - abstract public function buildRedirectController($uri, $external); + public function buildRequest() { + $parser = new PhutilQueryStringParser(); - final public function setRequest(AphrontRequest $request) { + $data = array(); + $data += $_POST; + $data += $parser->parseQueryString(idx($_SERVER, 'QUERY_STRING', '')); + + $cookie_prefix = PhabricatorEnv::getEnvConfig('phabricator.cookie-prefix'); + + $request = new AphrontRequest($this->getHost(), $this->getPath()); + $request->setRequestData($data); + $request->setApplicationConfiguration($this); + $request->setCookiePrefix($cookie_prefix); + + return $request; + } + + public function build404Controller() { + return array(new Phabricator404Controller(), array()); + } + + public function buildRedirectController($uri, $external) { + return array( + new PhabricatorRedirectController(), + array( + 'uri' => $uri, + 'external' => $external, + ), + ); + } + + public function setRequest(AphrontRequest $request) { $this->request = $request; return $this; } - final public function getRequest() { + public function getRequest() { return $this->request; } - final public function getConsole() { + public function getConsole() { return $this->console; } - final public function setConsole($console) { + public function setConsole($console) { $this->console = $console; return $this; } - final public function setHost($host) { + public function setHost($host) { $this->host = $host; return $this; } - final public function getHost() { + public function getHost() { return $this->host; } - final public function setPath($path) { + public function setPath($path) { $this->path = $path; return $this; } - final public function getPath() { + public function getPath() { return $this->path; } - public function willBuildRequest() {} - /** * @phutil-external-symbol class PhabricatorStartup @@ -83,6 +109,8 @@ abstract class AphrontApplicationConfiguration extends Phobject { PhabricatorStartup::beginStartupPhase('env.init'); + self::readHTTPPOSTData(); + try { PhabricatorEnv::initializeWebEnvironment(); $database_exception = null; @@ -142,16 +170,10 @@ abstract class AphrontApplicationConfiguration extends Phobject { $host = AphrontRequest::getHTTPHeader('Host'); $path = $_REQUEST['__path__']; - switch ($host) { - default: - $config_key = 'aphront.default-application-configuration-class'; - $application = PhabricatorEnv::newObjectFromConfig($config_key); - break; - } + $application = new self(); $application->setHost($host); $application->setPath($path); - $application->willBuildRequest(); $request = $application->buildRequest(); // Now that we have a request, convert the write guard into one which @@ -313,7 +335,7 @@ abstract class AphrontApplicationConfiguration extends Phobject { * parameters. * @task routing */ - final private function buildController() { + private function buildController() { $request = $this->getRequest(); // If we're configured to operate in cluster mode, reject requests which @@ -708,4 +730,88 @@ abstract class AphrontApplicationConfiguration extends Phobject { ->setContent($result); } + private static function readHTTPPOSTData() { + $request_method = idx($_SERVER, 'REQUEST_METHOD'); + if ($request_method === 'PUT') { + // For PUT requests, do nothing: in particular, do NOT read input. This + // allows us to stream input later and process very large PUT requests, + // like those coming from Git LFS. + return; + } + + + // For POST requests, we're going to read the raw input ourselves here + // if we can. Among other things, this corrects variable names with + // the "." character in them, which PHP normally converts into "_". + + // There are two major considerations here: whether the + // `enable_post_data_reading` option is set, and whether the content + // type is "multipart/form-data" or not. + + // If `enable_post_data_reading` is off, we're free to read the entire + // raw request body and parse it -- and we must, because $_POST and + // $_FILES are not built for us. If `enable_post_data_reading` is on, + // which is the default, we may not be able to read the body (the + // documentation says we can't, but empirically we can at least some + // of the time). + + // If the content type is "multipart/form-data", we need to build both + // $_POST and $_FILES, which is involved. The body itself is also more + // difficult to parse than other requests. + $raw_input = PhabricatorStartup::getRawInput(); + $parser = new PhutilQueryStringParser(); + + if (strlen($raw_input)) { + $content_type = idx($_SERVER, 'CONTENT_TYPE'); + $is_multipart = preg_match('@^multipart/form-data@i', $content_type); + if ($is_multipart && !ini_get('enable_post_data_reading')) { + $multipart_parser = id(new AphrontMultipartParser()) + ->setContentType($content_type); + + $multipart_parser->beginParse(); + $multipart_parser->continueParse($raw_input); + $parts = $multipart_parser->endParse(); + + // We're building and then parsing a query string so that requests + // with arrays (like "x[]=apple&x[]=banana") work correctly. This also + // means we can't use "phutil_build_http_querystring()", since it + // can't build a query string with duplicate names. + + $query_string = array(); + foreach ($parts as $part) { + if (!$part->isVariable()) { + continue; + } + + $name = $part->getName(); + $value = $part->getVariableValue(); + $query_string[] = rawurlencode($name).'='.rawurlencode($value); + } + $query_string = implode('&', $query_string); + $post = $parser->parseQueryString($query_string); + + $files = array(); + foreach ($parts as $part) { + if ($part->isVariable()) { + continue; + } + + $files[$part->getName()] = $part->getPHPFileDictionary(); + } + $_FILES = $files; + } else { + $post = $parser->parseQueryString($raw_input); + } + + $_POST = $post; + PhabricatorStartup::rebuildRequest(); + } else if ($_POST) { + $post = filter_input_array(INPUT_POST, FILTER_UNSAFE_RAW); + if (is_array($post)) { + $_POST = $post; + PhabricatorStartup::rebuildRequest(); + } + } + } + } diff --git a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php deleted file mode 100644 index fb67919576..0000000000 --- a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php +++ /dev/null @@ -1,121 +0,0 @@ -setContentType($content_type); - - $multipart_parser->beginParse(); - $multipart_parser->continueParse($raw_input); - $parts = $multipart_parser->endParse(); - - $query_string = array(); - foreach ($parts as $part) { - if (!$part->isVariable()) { - continue; - } - - $name = $part->getName(); - $value = $part->getVariableValue(); - - $query_string[] = urlencode($name).'='.urlencode($value); - } - $query_string = implode('&', $query_string); - $post = $parser->parseQueryString($query_string); - - $files = array(); - foreach ($parts as $part) { - if ($part->isVariable()) { - continue; - } - - $files[$part->getName()] = $part->getPHPFileDictionary(); - } - $_FILES = $files; - } else { - $post = $parser->parseQueryString($raw_input); - } - - $_POST = $post; - PhabricatorStartup::rebuildRequest(); - - $data += $post; - } else if ($_POST) { - $post = filter_input_array(INPUT_POST, FILTER_UNSAFE_RAW); - if (is_array($post)) { - $_POST = $post; - PhabricatorStartup::rebuildRequest(); - } - $data += $_POST; - } - } - - $data += $parser->parseQueryString(idx($_SERVER, 'QUERY_STRING', '')); - - $cookie_prefix = PhabricatorEnv::getEnvConfig('phabricator.cookie-prefix'); - - $request = new AphrontRequest($this->getHost(), $this->getPath()); - $request->setRequestData($data); - $request->setApplicationConfiguration($this); - $request->setCookiePrefix($cookie_prefix); - - return $request; - } - - public function build404Controller() { - return array(new Phabricator404Controller(), array()); - } - - public function buildRedirectController($uri, $external) { - return array( - new PhabricatorRedirectController(), - array( - 'uri' => $uri, - 'external' => $external, - ), - ); - } - -} diff --git a/src/aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php b/src/aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php index 2a737ecf5c..7f4eddad45 100644 --- a/src/aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php +++ b/src/aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php @@ -78,15 +78,13 @@ final class PhabricatorHighSecurityRequestExceptionHandler $form_layout = $form->buildLayoutView(); if ($is_upgrade) { - $messages = array( - pht( - 'You are taking an action which requires you to enter '. - 'high security.'), - ); + $message = pht( + 'You are taking an action which requires you to enter '. + 'high security.'); $info_view = id(new PHUIInfoView()) ->setSeverity(PHUIInfoView::SEVERITY_MFA) - ->setErrors($messages); + ->setErrors(array($message)); $dialog ->appendChild($info_view) @@ -100,12 +98,18 @@ final class PhabricatorHighSecurityRequestExceptionHandler 'period of time. When you are finished taking sensitive '. 'actions, you should leave high security.')); } else { + $message = pht( + 'You are taking an action which requires you to provide '. + 'multi-factor credentials.'); + + $info_view = id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_MFA) + ->setErrors(array($message)); + $dialog + ->appendChild($info_view) ->setErrors( array( - pht( - 'You are taking an action which requires you to provide '. - 'multi-factor credentials.'), )) ->appendChild($form_layout); } diff --git a/src/applications/auth/application/PhabricatorAuthApplication.php b/src/applications/auth/application/PhabricatorAuthApplication.php index a9ab3be181..df86595b46 100644 --- a/src/applications/auth/application/PhabricatorAuthApplication.php +++ b/src/applications/auth/application/PhabricatorAuthApplication.php @@ -95,6 +95,8 @@ final class PhabricatorAuthApplication extends PhabricatorApplication { 'PhabricatorAuthFactorProviderEditController', '(?P[1-9]\d*)/' => 'PhabricatorAuthFactorProviderViewController', + 'message/(?P[1-9]\d*)/' => + 'PhabricatorAuthFactorProviderMessageController', ), 'message/' => array( diff --git a/src/applications/auth/constants/PhabricatorAuthFactorProviderStatus.php b/src/applications/auth/constants/PhabricatorAuthFactorProviderStatus.php new file mode 100644 index 0000000000..61d4a12576 --- /dev/null +++ b/src/applications/auth/constants/PhabricatorAuthFactorProviderStatus.php @@ -0,0 +1,103 @@ +key = $status; + $result->spec = self::newSpecification($status); + + return $result; + } + + public function getName() { + return idx($this->spec, 'name', $this->key); + } + + public function getStatusHeaderIcon() { + return idx($this->spec, 'header.icon'); + } + + public function getStatusHeaderColor() { + return idx($this->spec, 'header.color'); + } + + public function isActive() { + return ($this->key === self::STATUS_ACTIVE); + } + + public function getListIcon() { + return idx($this->spec, 'list.icon'); + } + + public function getListColor() { + return idx($this->spec, 'list.color'); + } + + public function getFactorIcon() { + return idx($this->spec, 'factor.icon'); + } + + public function getFactorColor() { + return idx($this->spec, 'factor.color'); + } + + public function getOrder() { + return idx($this->spec, 'order', 0); + } + + public static function getMap() { + $specs = self::newSpecifications(); + return ipull($specs, 'name'); + } + + private static function newSpecification($key) { + $specs = self::newSpecifications(); + return idx($specs, $key, array()); + } + + private static function newSpecifications() { + return array( + self::STATUS_ACTIVE => array( + 'name' => pht('Active'), + 'header.icon' => 'fa-check', + 'header.color' => null, + 'list.icon' => null, + 'list.color' => null, + 'factor.icon' => 'fa-check', + 'factor.color' => 'green', + 'order' => 1, + ), + self::STATUS_DEPRECATED => array( + 'name' => pht('Deprecated'), + 'header.icon' => 'fa-ban', + 'header.color' => 'indigo', + 'list.icon' => 'fa-ban', + 'list.color' => 'indigo', + 'factor.icon' => 'fa-ban', + 'factor.color' => 'indigo', + 'order' => 2, + ), + self::STATUS_DISABLED => array( + 'name' => pht('Disabled'), + 'header.icon' => 'fa-times', + 'header.color' => 'red', + 'list.icon' => 'fa-times', + 'list.color' => 'red', + 'factor.icon' => 'fa-times', + 'factor.color' => 'grey', + 'order' => 3, + ), + ); + } + +} diff --git a/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php index 92328b2000..259e4c6743 100644 --- a/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php +++ b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php @@ -197,6 +197,8 @@ final class PhabricatorAuthNeedsMultiFactorController ->addCancelButton('/', pht('Continue')); } + $views = array(); + $messages = array(); $messages[] = pht( @@ -210,7 +212,39 @@ final class PhabricatorAuthNeedsMultiFactorController ->setSeverity(PHUIInfoView::SEVERITY_WARNING) ->setErrors($messages); - return $view; + $views[] = $view; + + + $providers = id(new PhabricatorAuthFactorProviderQuery()) + ->setViewer($viewer) + ->withStatuses( + array( + PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE, + )) + ->execute(); + if (!$providers) { + $messages = array(); + + $required_key = 'security.require-multi-factor-auth'; + + $messages[] = pht( + 'This install has the configuration option "%s" enabled, but does '. + 'not have any active multifactor providers configured. This means '. + 'you are required to add MFA, but are also prevented from doing so. '. + 'An administrator must disable "%s" or enable an MFA provider to '. + 'allow you to continue.', + $required_key, + $required_key); + + $view = id(new PHUIInfoView()) + ->setTitle(pht('Multi-Factor Authentication is Misconfigured')) + ->setSeverity(PHUIInfoView::SEVERITY_ERROR) + ->setErrors($messages); + + $views[] = $view; + } + + return $views; } } diff --git a/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderListController.php b/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderListController.php index 293728cf36..d19671c3ce 100644 --- a/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderListController.php +++ b/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderListController.php @@ -20,6 +20,16 @@ final class PhabricatorAuthFactorProviderListController ->setHeader($provider->getDisplayName()) ->setHref($provider->getURI()); + $status = $provider->newStatus(); + + $icon = $status->getListIcon(); + $color = $status->getListColor(); + if ($icon !== null) { + $item->setStatusIcon("{$icon} {$color}", $status->getName()); + } + + $item->setDisabled(!$status->isActive()); + $list->addItem($item); } diff --git a/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderMessageController.php b/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderMessageController.php new file mode 100644 index 0000000000..563ee39931 --- /dev/null +++ b/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderMessageController.php @@ -0,0 +1,84 @@ +requireApplicationCapability( + AuthManageProvidersCapability::CAPABILITY); + + $viewer = $request->getViewer(); + $id = $request->getURIData('id'); + + $provider = id(new PhabricatorAuthFactorProviderQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->executeOne(); + if (!$provider) { + return new Aphront404Response(); + } + + $cancel_uri = $provider->getURI(); + $enroll_key = + PhabricatorAuthFactorProviderEnrollMessageTransaction::TRANSACTIONTYPE; + + $message = $provider->getEnrollMessage(); + + if ($request->isFormOrHisecPost()) { + $message = $request->getStr('message'); + + $xactions = array(); + + $xactions[] = id(new PhabricatorAuthFactorProviderTransaction()) + ->setTransactionType($enroll_key) + ->setNewValue($message); + + $editor = id(new PhabricatorAuthFactorProviderEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true) + ->setCancelURI($cancel_uri); + + $editor->applyTransactions($provider, $xactions); + + return id(new AphrontRedirectResponse())->setURI($cancel_uri); + } + + $default_message = $provider->getEnrollDescription($viewer); + $default_message = new PHUIRemarkupView($viewer, $default_message); + + $form = id(new AphrontFormView()) + ->setViewer($viewer) + ->appendRemarkupInstructions( + pht( + 'When users add a factor for this provider, they are given this '. + 'enrollment guidance by default:')) + ->appendControl( + id(new AphrontFormMarkupControl()) + ->setLabel(pht('Default Message')) + ->setValue($default_message)) + ->appendRemarkupInstructions( + pht( + 'You may optionally customize the enrollment message users are '. + 'presented with by providing a replacement message below:')) + ->appendControl( + id(new PhabricatorRemarkupControl()) + ->setLabel(pht('Custom Message')) + ->setName('message') + ->setValue($message)); + + return $this->newDialog() + ->setTitle(pht('Change Enroll Message')) + ->setWidth(AphrontDialogView::WIDTH_FORM) + ->appendForm($form) + ->addCancelButton($cancel_uri) + ->addSubmitButton(pht('Save')); + } + +} diff --git a/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderViewController.php b/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderViewController.php index 67edf2f81b..1dac49bcf9 100644 --- a/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderViewController.php +++ b/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderViewController.php @@ -58,6 +58,15 @@ final class PhabricatorAuthFactorProviderViewController ->setHeader($provider->getDisplayName()) ->setPolicyObject($provider); + $status = $provider->newStatus(); + + $header_icon = $status->getStatusHeaderIcon(); + $header_color = $status->getStatusHeaderColor(); + $header_name = $status->getName(); + if ($header_icon !== null) { + $view->setStatus($header_icon, $header_color, $header_name); + } + return $view; } @@ -72,6 +81,16 @@ final class PhabricatorAuthFactorProviderViewController pht('Factor Type'), $provider->getFactor()->getFactorName()); + + $custom_enroll = $provider->getEnrollMessage(); + if (strlen($custom_enroll)) { + $view->addSectionHeader( + pht('Custom Enroll Message'), + PHUIPropertyListView::ICON_SUMMARY); + $view->addTextContent( + new PHUIRemarkupView($viewer, $custom_enroll)); + } + return $view; } @@ -94,6 +113,14 @@ final class PhabricatorAuthFactorProviderViewController ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); + $curtain->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Customize Enroll Message')) + ->setIcon('fa-commenting-o') + ->setHref($this->getApplicationURI("mfa/message/{$id}/")) + ->setDisabled(!$can_edit) + ->setWorkflow(true)); + return $curtain; } diff --git a/src/applications/auth/editor/PhabricatorAuthFactorProviderEditEngine.php b/src/applications/auth/editor/PhabricatorAuthFactorProviderEditEngine.php index a0b5988589..ab74350cc9 100644 --- a/src/applications/auth/editor/PhabricatorAuthFactorProviderEditEngine.php +++ b/src/applications/auth/editor/PhabricatorAuthFactorProviderEditEngine.php @@ -93,9 +93,12 @@ final class PhabricatorAuthFactorProviderEditEngine } protected function buildCustomEditFields($object) { - $factor_name = $object->getFactor()->getFactorName(); + $factor = $object->getFactor(); + $factor_name = $factor->getFactorName(); - return array( + $status_map = PhabricatorAuthFactorProviderStatus::getMap(); + + $fields = array( id(new PhabricatorStaticEditField()) ->setKey('displayType') ->setLabel(pht('Factor Type')) @@ -109,7 +112,22 @@ final class PhabricatorAuthFactorProviderEditEngine ->setDescription(pht('Display name for the MFA provider.')) ->setValue($object->getName()) ->setPlaceholder($factor_name), + id(new PhabricatorSelectEditField()) + ->setKey('status') + ->setTransactionType( + PhabricatorAuthFactorProviderStatusTransaction::TRANSACTIONTYPE) + ->setLabel(pht('Status')) + ->setDescription(pht('Status of the MFA provider.')) + ->setValue($object->getStatus()) + ->setOptions($status_map), ); + + $factor_fields = $factor->newEditEngineFields($this, $object); + foreach ($factor_fields as $field) { + $fields[] = $field; + } + + return $fields; } } diff --git a/src/applications/auth/engine/PhabricatorAuthFactorProviderMFAEngine.php b/src/applications/auth/engine/PhabricatorAuthFactorProviderMFAEngine.php new file mode 100644 index 0000000000..39f80bb5b8 --- /dev/null +++ b/src/applications/auth/engine/PhabricatorAuthFactorProviderMFAEngine.php @@ -0,0 +1,10 @@ +setViewer($viewer) ->withUserPHIDs(array($viewer->getPHID())) - ->setOrderVector(array('-id')) + ->withFactorProviderStatuses( + array( + PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE, + PhabricatorAuthFactorProviderStatus::STATUS_DEPRECATED, + )) ->execute(); + // Sort factors in the same order that they appear in on the Settings + // panel. This means that administrators changing provider statuses may + // change the order of prompts for users, but the alternative is that the + // Settings panel order disagrees with the prompt order, which seems more + // disruptive. + $factors = msort($factors, 'newSortVector'); + // If the account has no associated multi-factor auth, just issue a token // without putting the session into high security mode. This is generally // easier for users. A minor but desirable side effect is that when a user @@ -529,14 +540,22 @@ final class PhabricatorAuthSessionEngine extends Phobject { $provider = $factor->getFactorProvider(); $impl = $provider->getFactor(); - try { - $new_challenges = $impl->getNewIssuedChallenges( - $factor, - $viewer, - $issued_challenges); - } catch (PhabricatorAuthFactorResultException $ex) { - $ok = false; - $validation_results[$factor_phid] = $ex->getResult(); + $new_challenges = $impl->getNewIssuedChallenges( + $factor, + $viewer, + $issued_challenges); + + // NOTE: We may get a list of challenges back, or may just get an early + // result. For example, this can happen on an SMS factor if all SMS + // mailers have been disabled. + if ($new_challenges instanceof PhabricatorAuthFactorResult) { + $result = $new_challenges; + + if (!$result->getIsValid()) { + $ok = false; + } + + $validation_results[$factor_phid] = $result; $challenge_map[$factor_phid] = $issued_challenges; continue; } diff --git a/src/applications/auth/exception/PhabricatorAuthFactorResultException.php b/src/applications/auth/exception/PhabricatorAuthFactorResultException.php deleted file mode 100644 index 61595266e5..0000000000 --- a/src/applications/auth/exception/PhabricatorAuthFactorResultException.php +++ /dev/null @@ -1,17 +0,0 @@ -result = $result; - parent::__construct(); - } - - public function getResult() { - return $this->result; - } - -} diff --git a/src/applications/auth/factor/PhabricatorAuthFactor.php b/src/applications/auth/factor/PhabricatorAuthFactor.php index cf8087625f..ec49f7f748 100644 --- a/src/applications/auth/factor/PhabricatorAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorAuthFactor.php @@ -3,6 +3,7 @@ abstract class PhabricatorAuthFactor extends Phobject { abstract public function getFactorName(); + abstract public function getFactorShortName(); abstract public function getFactorKey(); abstract public function getFactorCreateHelp(); abstract public function getFactorDescription(); @@ -54,14 +55,31 @@ abstract class PhabricatorAuthFactor extends Phobject { return null; } - public function canCreateNewConfiguration(PhabricatorUser $user) { + public function canCreateNewConfiguration( + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $user) { return true; } - public function getConfigurationCreateDescription(PhabricatorUser $user) { + public function getConfigurationCreateDescription( + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $user) { return null; } + public function getConfigurationListDetails( + PhabricatorAuthFactorConfig $config, + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $viewer) { + return null; + } + + public function newEditEngineFields( + PhabricatorEditEngine $engine, + PhabricatorAuthFactorProvider $provider) { + return array(); + } + /** * Is this a factor which depends on the user's contact number? * @@ -129,6 +147,11 @@ abstract class PhabricatorAuthFactor extends Phobject { $viewer, $challenges); + if ($this->isAuthResult($new_challenges)) { + unset($unguarded); + return $new_challenges; + } + assert_instances_of($new_challenges, 'PhabricatorAuthChallenge'); foreach ($new_challenges as $new_challenge) { @@ -177,7 +200,7 @@ abstract class PhabricatorAuthFactor extends Phobject { return $result; } - if (!($result instanceof PhabricatorAuthFactorResult)) { + if (!$this->isAuthResult($result)) { throw new Exception( pht( 'Expected "newResultFromIssuedChallenges()" to return null or '. @@ -209,7 +232,7 @@ abstract class PhabricatorAuthFactor extends Phobject { $request, $challenges); - if (!($result instanceof PhabricatorAuthFactorResult)) { + if (!$this->isAuthResult($result)) { throw new Exception( pht( 'Expected "newResultFromChallengeResponse()" to return an object '. @@ -314,6 +337,7 @@ abstract class PhabricatorAuthFactor extends Phobject { final protected function loadMFASyncToken( + PhabricatorAuthFactorProvider $provider, AphrontRequest $request, AphrontFormView $form, PhabricatorUser $user) { @@ -380,7 +404,13 @@ abstract class PhabricatorAuthFactor extends Phobject { ->setTokenCode($sync_key_digest) ->setTokenExpires($now + $sync_ttl); - $properties = $this->newMFASyncTokenProperties($user); + $properties = $this->newMFASyncTokenProperties( + $provider, + $user); + + if ($this->isAuthResult($properties)) { + return $properties; + } foreach ($properties as $key => $value) { $sync_token->setTemporaryTokenProperty($key, $value); @@ -394,7 +424,9 @@ abstract class PhabricatorAuthFactor extends Phobject { return $sync_token; } - protected function newMFASyncTokenProperties(PhabricatorUser $user) { + protected function newMFASyncTokenProperties( + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $user) { return array(); } @@ -481,10 +513,6 @@ abstract class PhabricatorAuthFactor extends Phobject { $rows); } - final protected function throwResult(PhabricatorAuthFactorResult $result) { - throw new PhabricatorAuthFactorResultException($result); - } - final protected function getInstallDisplayName() { $uri = PhabricatorEnv::getURI('/'); $uri = new PhutilURI($uri); @@ -520,4 +548,19 @@ abstract class PhabricatorAuthFactor extends Phobject { return $request->validateCSRF(); } + final protected function loadConfigurationsForProvider( + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $user) { + + return id(new PhabricatorAuthFactorConfigQuery()) + ->setViewer($user) + ->withUserPHIDs(array($user->getPHID())) + ->withFactorProviderPHIDs(array($provider->getPHID())) + ->execute(); + } + + final protected function isAuthResult($object) { + return ($object instanceof PhabricatorAuthFactorResult); + } + } diff --git a/src/applications/auth/factor/PhabricatorDuoAuthFactor.php b/src/applications/auth/factor/PhabricatorDuoAuthFactor.php new file mode 100644 index 0000000000..187e011953 --- /dev/null +++ b/src/applications/auth/factor/PhabricatorDuoAuthFactor.php @@ -0,0 +1,813 @@ +loadConfigurationsForProvider($provider, $user)) { + return false; + } + + return true; + } + + public function getConfigurationCreateDescription( + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $user) { + + $messages = array(); + + if ($this->loadConfigurationsForProvider($provider, $user)) { + $messages[] = id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING) + ->setErrors( + array( + pht( + 'You already have Duo authentication attached to your account '. + 'for this provider.'), + )); + } + + return $messages; + } + + public function getConfigurationListDetails( + PhabricatorAuthFactorConfig $config, + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $viewer) { + + $duo_user = $config->getAuthFactorConfigProperty('duo.username'); + + return pht('Duo Username: %s', $duo_user); + } + + + public function newEditEngineFields( + PhabricatorEditEngine $engine, + PhabricatorAuthFactorProvider $provider) { + + $viewer = $engine->getViewer(); + + $credential_phid = $provider->getAuthFactorProviderProperty( + self::PROP_CREDENTIAL); + + $hostname = $provider->getAuthFactorProviderProperty(self::PROP_HOSTNAME); + $usernames = $provider->getAuthFactorProviderProperty(self::PROP_USERNAMES); + $enroll = $provider->getAuthFactorProviderProperty(self::PROP_ENROLL); + + $credential_type = PassphrasePasswordCredentialType::CREDENTIAL_TYPE; + $provides_type = PassphrasePasswordCredentialType::PROVIDES_TYPE; + + $credentials = id(new PassphraseCredentialQuery()) + ->setViewer($viewer) + ->withIsDestroyed(false) + ->withProvidesTypes(array($provides_type)) + ->execute(); + + $xaction_hostname = + PhabricatorAuthFactorProviderDuoHostnameTransaction::TRANSACTIONTYPE; + $xaction_credential = + PhabricatorAuthFactorProviderDuoCredentialTransaction::TRANSACTIONTYPE; + $xaction_usernames = + PhabricatorAuthFactorProviderDuoUsernamesTransaction::TRANSACTIONTYPE; + $xaction_enroll = + PhabricatorAuthFactorProviderDuoEnrollTransaction::TRANSACTIONTYPE; + + return array( + id(new PhabricatorTextEditField()) + ->setLabel(pht('Duo API Hostname')) + ->setKey('duo.hostname') + ->setValue($hostname) + ->setTransactionType($xaction_hostname) + ->setIsRequired(true), + id(new PhabricatorCredentialEditField()) + ->setLabel(pht('Duo API Credential')) + ->setKey('duo.credential') + ->setValue($credential_phid) + ->setTransactionType($xaction_credential) + ->setCredentialType($credential_type) + ->setCredentials($credentials), + id(new PhabricatorSelectEditField()) + ->setLabel(pht('Duo Username')) + ->setKey('duo.usernames') + ->setValue($usernames) + ->setTransactionType($xaction_usernames) + ->setOptions( + array( + 'username' => pht('Use Phabricator Username'), + 'email' => pht('Use Primary Email Address'), + )), + id(new PhabricatorSelectEditField()) + ->setLabel(pht('Create Accounts')) + ->setKey('duo.enroll') + ->setValue($enroll) + ->setTransactionType($xaction_enroll) + ->setOptions( + array( + 'deny' => pht('Require Existing Duo Account'), + 'allow' => pht('Create New Duo Account'), + )), + ); + } + + + public function processAddFactorForm( + PhabricatorAuthFactorProvider $provider, + AphrontFormView $form, + AphrontRequest $request, + PhabricatorUser $user) { + + $token = $this->loadMFASyncToken($provider, $request, $form, $user); + if ($this->isAuthResult($token)) { + $form->appendChild($this->newAutomaticControl($token)); + return; + } + + $enroll = $token->getTemporaryTokenProperty('duo.enroll'); + $duo_id = $token->getTemporaryTokenProperty('duo.user-id'); + $duo_uri = $token->getTemporaryTokenProperty('duo.uri'); + $duo_user = $token->getTemporaryTokenProperty('duo.username'); + + $is_external = ($enroll === 'external'); + $is_auto = ($enroll === 'auto'); + $is_blocked = ($enroll === 'blocked'); + + if (!$token->getIsNewTemporaryToken()) { + if ($is_auto) { + return $this->newDuoConfig($user, $duo_user); + } else if ($is_external || $is_blocked) { + $parameters = array( + 'username' => $duo_user, + ); + + $result = $this->newDuoFuture($provider) + ->setMethod('preauth', $parameters) + ->resolve(); + + $result_code = $result['response']['result']; + switch ($result_code) { + case 'auth': + case 'allow': + return $this->newDuoConfig($user, $duo_user); + case 'enroll': + if ($is_blocked) { + // We'll render an equivalent static control below, so skip + // rendering here. We explicitly don't want to give the user + // an enroll workflow. + break; + } + + $duo_uri = $result['response']['enroll_portal_url']; + + $waiting_icon = id(new PHUIIconView()) + ->setIcon('fa-mobile', 'red'); + + $waiting_control = id(new PHUIFormTimerControl()) + ->setIcon($waiting_icon) + ->setError(pht('Not Complete')) + ->appendChild( + pht( + 'You have not completed Duo enrollment yet. '. + 'Complete enrollment, then click continue.')); + + $form->appendControl($waiting_control); + break; + default: + case 'deny': + break; + } + } else { + $parameters = array( + 'user_id' => $duo_id, + 'activation_code' => $duo_uri, + ); + + $future = $this->newDuoFuture($provider) + ->setMethod('enroll_status', $parameters); + + $result = $future->resolve(); + $response = $result['response']; + + switch ($response) { + case 'success': + return $this->newDuoConfig($user, $duo_user); + case 'waiting': + $waiting_icon = id(new PHUIIconView()) + ->setIcon('fa-mobile', 'red'); + + $waiting_control = id(new PHUIFormTimerControl()) + ->setIcon($waiting_icon) + ->setError(pht('Not Complete')) + ->appendChild( + pht( + 'You have not activated this enrollment in the Duo '. + 'application on your phone yet. Complete activation, then '. + 'click continue.')); + + $form->appendControl($waiting_control); + break; + case 'invalid': + default: + throw new Exception( + pht( + 'This Duo enrollment attempt is invalid or has '. + 'expired ("%s"). Cancel the workflow and try again.', + $response)); + } + } + } + + if ($is_blocked) { + $blocked_icon = id(new PHUIIconView()) + ->setIcon('fa-times', 'red'); + + $blocked_control = id(new PHUIFormTimerControl()) + ->setIcon($blocked_icon) + ->appendChild( + pht( + 'Your Duo account ("%s") has not completed Duo enrollment. '. + 'Check your email and complete enrollment to continue.', + phutil_tag('strong', array(), $duo_user))); + + $form->appendControl($blocked_control); + } else if ($is_auto) { + $auto_icon = id(new PHUIIconView()) + ->setIcon('fa-check', 'green'); + + $auto_control = id(new PHUIFormTimerControl()) + ->setIcon($auto_icon) + ->appendChild( + pht( + 'Duo account ("%s") is fully enrolled.', + phutil_tag('strong', array(), $duo_user))); + + $form->appendControl($auto_control); + } else { + $duo_button = phutil_tag( + 'a', + array( + 'href' => $duo_uri, + 'class' => 'button button-grey', + 'target' => ($is_external ? '_blank' : null), + ), + pht('Enroll Duo Account: %s', $duo_user)); + + $duo_button = phutil_tag( + 'div', + array( + 'class' => 'mfa-form-enroll-button', + ), + $duo_button); + + if ($is_external) { + $form->appendRemarkupInstructions( + pht( + 'Complete enrolling your phone with Duo:')); + + $form->appendControl( + id(new AphrontFormMarkupControl()) + ->setValue($duo_button)); + } else { + + $form->appendRemarkupInstructions( + pht( + 'Scan this QR code with the Duo application on your mobile '. + 'phone:')); + + + $qr_code = $this->newQRCode($duo_uri); + $form->appendChild($qr_code); + + $form->appendRemarkupInstructions( + pht( + 'If you are currently using your phone to view this page, '. + 'click this button to open the Duo application:')); + + $form->appendControl( + id(new AphrontFormMarkupControl()) + ->setValue($duo_button)); + } + + $form->appendRemarkupInstructions( + pht( + 'Once you have completed setup on your phone, click continue.')); + } + } + + + protected function newMFASyncTokenProperties( + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $user) { + + $duo_user = $this->getDuoUsername($provider, $user); + + // Duo automatically normalizes usernames to lowercase. Just do that here + // so that our value agrees more closely with Duo. + $duo_user = phutil_utf8_strtolower($duo_user); + + $parameters = array( + 'username' => $duo_user, + ); + + $result = $this->newDuoFuture($provider) + ->setMethod('preauth', $parameters) + ->resolve(); + + $external_uri = null; + $result_code = $result['response']['result']; + $status_message = $result['response']['status_msg']; + switch ($result_code) { + case 'auth': + case 'allow': + // If the user already has a Duo account, they don't need to do + // anything. + return array( + 'duo.enroll' => 'auto', + 'duo.username' => $duo_user, + ); + case 'enroll': + if (!$this->shouldAllowDuoEnrollment($provider)) { + return array( + 'duo.enroll' => 'blocked', + 'duo.username' => $duo_user, + ); + } + + $external_uri = $result['response']['enroll_portal_url']; + + // Otherwise, enrollment is permitted so we're going to continue. + break; + default: + case 'deny': + return $this->newResult() + ->setIsError(true) + ->setErrorMessage( + pht( + 'Your Duo account ("%s") is not permitted to access this '. + 'system. Contact your Duo administrator for help. '. + 'The Duo preauth API responded with status message ("%s"): %s', + $duo_user, + $result_code, + $status_message)); + } + + // Duo's "/enroll" API isn't repeatable for the same username. If we're + // the first call, great: we can do inline enrollment, which is way more + // user friendly. Otherwise, we have to send the user on an adventure. + + $parameters = array( + 'username' => $duo_user, + 'valid_secs' => phutil_units('1 hour in seconds'), + ); + + try { + $result = $this->newDuoFuture($provider) + ->setMethod('enroll', $parameters) + ->resolve(); + } catch (HTTPFutureHTTPResponseStatus $ex) { + return array( + 'duo.enroll' => 'external', + 'duo.username' => $duo_user, + 'duo.uri' => $external_uri, + ); + } + + return array( + 'duo.enroll' => 'inline', + 'duo.uri' => $result['response']['activation_code'], + 'duo.username' => $duo_user, + 'duo.user-id' => $result['response']['user_id'], + ); + } + + protected function newIssuedChallenges( + PhabricatorAuthFactorConfig $config, + PhabricatorUser $viewer, + array $challenges) { + + // If we already issued a valid challenge for this workflow and session, + // don't issue a new one. + + $challenge = $this->getChallengeForCurrentContext( + $config, + $viewer, + $challenges); + if ($challenge) { + return array(); + } + + if (!$this->hasCSRF($config)) { + return $this->newResult() + ->setIsContinue(true) + ->setErrorMessage( + pht( + 'An authorization request will be pushed to the Duo '. + 'application on your phone.')); + } + + $provider = $config->getFactorProvider(); + + // Otherwise, issue a new challenge. + $duo_user = (string)$config->getAuthFactorConfigProperty('duo.username'); + + $parameters = array( + 'username' => $duo_user, + ); + + $response = $this->newDuoFuture($provider) + ->setMethod('preauth', $parameters) + ->resolve(); + $response = $response['response']; + + $next_step = $response['result']; + $status_message = $response['status_msg']; + switch ($next_step) { + case 'auth': + // We're good to go. + break; + case 'allow': + // Duo is telling us to bypass MFA. For now, refuse. + return $this->newResult() + ->setIsError(true) + ->setErrorMessage( + pht( + 'Duo is not requiring a challenge, which defeats the '. + 'purpose of MFA. Duo must be configured to challenge you.')); + case 'enroll': + return $this->newResult() + ->setIsError(true) + ->setErrorMessage( + pht( + 'Your Duo account ("%s") requires enrollment. Contact your '. + 'Duo administrator for help. Duo status message: %s', + $duo_user, + $status_message)); + case 'deny': + default: + return $this->newResult() + ->setIsError(true) + ->setErrorMessage( + pht( + 'Your Duo account ("%s") is not permitted to access this '. + 'system. Contact your Duo administrator for help. The Duo '. + 'preauth API responded with status message ("%s"): %s', + $duo_user, + $next_step, + $status_message)); + } + + $has_push = false; + $devices = $response['devices']; + foreach ($devices as $device) { + $capabilities = array_fuse($device['capabilities']); + if (isset($capabilities['push'])) { + $has_push = true; + break; + } + } + + if (!$has_push) { + return $this->newResult() + ->setIsError(true) + ->setErrorMessage( + pht( + 'This factor has been removed from your device, so Phabricator '. + 'can not send you a challenge. To continue, an administrator '. + 'must strip this factor from your account.')); + } + + $push_info = array( + pht('Domain') => $this->getInstallDisplayName(), + ); + $push_info = phutil_build_http_querystring($push_info); + + $parameters = array( + 'username' => $duo_user, + 'factor' => 'push', + 'async' => '1', + + // Duo allows us to specify a device, or to pass "auto" to have it pick + // the first one. For now, just let it pick. + 'device' => 'auto', + + // This is a hard-coded prefix for the word "... request" in the Duo UI, + // which defaults to "Login". We could pass richer information from + // workflows here, but it's not very flexible anyway. + 'type' => 'Authentication', + + 'display_username' => $viewer->getUsername(), + 'pushinfo' => $push_info, + ); + + $result = $this->newDuoFuture($provider) + ->setMethod('auth', $parameters) + ->resolve(); + + $duo_xaction = $result['response']['txid']; + + // The Duo push timeout is 60 seconds. Set our challenge to expire slightly + // more quickly so that we'll re-issue a new challenge before Duo times out. + // This should keep users away from a dead-end where they can't respond to + // Duo but Phabricator won't issue a new challenge yet. + $ttl_seconds = 55; + + return array( + $this->newChallenge($config, $viewer) + ->setChallengeKey($duo_xaction) + ->setChallengeTTL(PhabricatorTime::getNow() + $ttl_seconds), + ); + } + + protected function newResultFromIssuedChallenges( + PhabricatorAuthFactorConfig $config, + PhabricatorUser $viewer, + array $challenges) { + + $challenge = $this->getChallengeForCurrentContext( + $config, + $viewer, + $challenges); + + if ($challenge->getIsAnsweredChallenge()) { + return $this->newResult() + ->setAnsweredChallenge($challenge); + } + + $provider = $config->getFactorProvider(); + $duo_xaction = $challenge->getChallengeKey(); + + $parameters = array( + 'txid' => $duo_xaction, + ); + + // This endpoint always long-polls, so use a timeout to force it to act + // more asynchronously. + try { + $result = $this->newDuoFuture($provider) + ->setHTTPMethod('GET') + ->setMethod('auth_status', $parameters) + ->setTimeout(5) + ->resolve(); + + $state = $result['response']['result']; + $status = $result['response']['status']; + } catch (HTTPFutureCURLResponseStatus $exception) { + if ($exception->isTimeout()) { + $state = 'waiting'; + $status = 'poll'; + } else { + throw $exception; + } + } + + $now = PhabricatorTime::getNow(); + + switch ($state) { + case 'allow': + $ttl = PhabricatorTime::getNow() + + phutil_units('15 minutes in seconds'); + + $challenge + ->markChallengeAsAnswered($ttl); + + return $this->newResult() + ->setAnsweredChallenge($challenge); + case 'waiting': + // No result yet, we'll render a default state later on. + break; + default: + case 'deny': + if ($status === 'timeout') { + return $this->newResult() + ->setIsError(true) + ->setErrorMessage( + pht( + 'This request has timed out because you took too long to '. + 'respond.')); + } else { + $wait_duration = ($challenge->getChallengeTTL() - $now) + 1; + + return $this->newResult() + ->setIsWait(true) + ->setErrorMessage( + pht( + 'You denied this request. Wait %s second(s) to try again.', + new PhutilNumber($wait_duration))); + } + break; + } + + return null; + } + + public function renderValidateFactorForm( + PhabricatorAuthFactorConfig $config, + AphrontFormView $form, + PhabricatorUser $viewer, + PhabricatorAuthFactorResult $result) { + + $control = $this->newAutomaticControl($result); + if (!$control) { + $result = $this->newResult() + ->setIsContinue(true) + ->setErrorMessage( + pht( + 'A challenge has been sent to your phone. Open the Duo '. + 'application and confirm the challenge, then continue.')); + $control = $this->newAutomaticControl($result); + } + + $control + ->setLabel(pht('Duo')) + ->setCaption(pht('Factor Name: %s', $config->getFactorName())); + + $form->appendChild($control); + } + + public function getRequestHasChallengeResponse( + PhabricatorAuthFactorConfig $config, + AphrontRequest $request) { + $value = $this->getChallengeResponseFromRequest($config, $request); + return (bool)strlen($value); + } + + protected function newResultFromChallengeResponse( + PhabricatorAuthFactorConfig $config, + PhabricatorUser $viewer, + AphrontRequest $request, + array $challenges) { + + $challenge = $this->getChallengeForCurrentContext( + $config, + $viewer, + $challenges); + + $code = $this->getChallengeResponseFromRequest( + $config, + $request); + + $result = $this->newResult() + ->setValue($code); + + if ($challenge->getIsAnsweredChallenge()) { + return $result->setAnsweredChallenge($challenge); + } + + if (phutil_hashes_are_identical($code, $challenge->getChallengeKey())) { + $ttl = PhabricatorTime::getNow() + phutil_units('15 minutes in seconds'); + + $challenge + ->markChallengeAsAnswered($ttl); + + return $result->setAnsweredChallenge($challenge); + } + + if (strlen($code)) { + $error_message = pht('Invalid'); + } else { + $error_message = pht('Required'); + } + + $result->setErrorMessage($error_message); + + return $result; + } + + private function newDuoFuture(PhabricatorAuthFactorProvider $provider) { + $credential_phid = $provider->getAuthFactorProviderProperty( + self::PROP_CREDENTIAL); + + $omnipotent = PhabricatorUser::getOmnipotentUser(); + + $credential = id(new PassphraseCredentialQuery()) + ->setViewer($omnipotent) + ->withPHIDs(array($credential_phid)) + ->needSecrets(true) + ->executeOne(); + if (!$credential) { + throw new Exception( + pht( + 'Unable to load Duo API credential ("%s").', + $credential_phid)); + } + + $duo_key = $credential->getUsername(); + $duo_secret = $credential->getSecret(); + if (!$duo_secret) { + throw new Exception( + pht( + 'Duo API credential ("%s") has no secret key.', + $credential_phid)); + } + + $duo_host = $provider->getAuthFactorProviderProperty( + self::PROP_HOSTNAME); + self::requireDuoAPIHostname($duo_host); + + return id(new PhabricatorDuoFuture()) + ->setIntegrationKey($duo_key) + ->setSecretKey($duo_secret) + ->setAPIHostname($duo_host) + ->setTimeout(10) + ->setHTTPMethod('POST'); + } + + private function getDuoUsername( + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $user) { + + $mode = $provider->getAuthFactorProviderProperty(self::PROP_USERNAMES); + switch ($mode) { + case 'username': + return $user->getUsername(); + case 'email': + return $user->loadPrimaryEmailAddress(); + default: + throw new Exception( + pht( + 'Duo username pairing mode ("%s") is not supported.', + $mode)); + } + } + + private function shouldAllowDuoEnrollment( + PhabricatorAuthFactorProvider $provider) { + + $mode = $provider->getAuthFactorProviderProperty(self::PROP_ENROLL); + switch ($mode) { + case 'deny': + return false; + case 'allow': + return true; + default: + throw new Exception( + pht( + 'Duo enrollment mode ("%s") is not supported.', + $mode)); + } + } + + private function newDuoConfig(PhabricatorUser $user, $duo_user) { + $config_properties = array( + 'duo.username' => $duo_user, + ); + + $config = $this->newConfigForUser($user) + ->setFactorName(pht('Duo (%s)', $duo_user)) + ->setProperties($config_properties); + + return $config; + } + + public static function requireDuoAPIHostname($hostname) { + if (preg_match('/\.duosecurity\.com\z/', $hostname)) { + return; + } + + throw new Exception( + pht( + 'Duo API hostname ("%s") is invalid, hostname must be '. + '"*.duosecurity.com".', + $hostname)); + } + +} diff --git a/src/applications/auth/factor/PhabricatorSMSAuthFactor.php b/src/applications/auth/factor/PhabricatorSMSAuthFactor.php index a6f648af02..ba46de980e 100644 --- a/src/applications/auth/factor/PhabricatorSMSAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorSMSAuthFactor.php @@ -8,6 +8,10 @@ final class PhabricatorSMSAuthFactor } public function getFactorName() { + return pht('Text Message (SMS)'); + } + + public function getFactorShortName() { return pht('SMS'); } @@ -67,15 +71,24 @@ final class PhabricatorSMSAuthFactor return $messages; } - public function canCreateNewConfiguration(PhabricatorUser $user) { + public function canCreateNewConfiguration( + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $user) { + if (!$this->loadUserContactNumber($user)) { return false; } + if ($this->loadConfigurationsForProvider($provider, $user)) { + return false; + } + return true; } - public function getConfigurationCreateDescription(PhabricatorUser $user) { + public function getConfigurationCreateDescription( + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $user) { $messages = array(); @@ -91,6 +104,16 @@ final class PhabricatorSMSAuthFactor )); } + if ($this->loadConfigurationsForProvider($provider, $user)) { + $messages[] = id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING) + ->setErrors( + array( + pht( + 'You already have SMS authentication attached to your account.'), + )); + } + return $messages; } @@ -117,7 +140,7 @@ final class PhabricatorSMSAuthFactor AphrontRequest $request, PhabricatorUser $user) { - $token = $this->loadMFASyncToken($request, $form, $user); + $token = $this->loadMFASyncToken($provider, $request, $form, $user); $code = $request->getStr('sms.code'); $e_code = true; @@ -172,35 +195,29 @@ final class PhabricatorSMSAuthFactor } if (!$this->loadUserContactNumber($viewer)) { - $result = $this->newResult() + return $this->newResult() ->setIsError(true) ->setErrorMessage( pht( 'Your account has no primary contact number.')); - - $this->throwResult($result); } if (!$this->isSMSMailerConfigured()) { - $result = $this->newResult() + return $this->newResult() ->setIsError(true) ->setErrorMessage( pht( 'No outbound mailer which can deliver SMS messages is '. 'configured.')); - - $this->throwResult($result); } if (!$this->hasCSRF($config)) { - $result = $this->newResult() + return $this->newResult() ->setIsContinue(true) ->setErrorMessage( pht( 'A text message with an authorization code will be sent to your '. 'primary contact number.')); - - $this->throwResult($result); } // Otherwise, issue a new challenge. @@ -347,7 +364,10 @@ final class PhabricatorSMSAuthFactor return head($contact_numbers); } - protected function newMFASyncTokenProperties(PhabricatorUser $user) { + protected function newMFASyncTokenProperties( + PhabricatorAuthFactorProvider $providerr, + PhabricatorUser $user) { + $sms_code = $this->newSMSChallengeCode(); $envelope = new PhutilOpaqueEnvelope($sms_code); diff --git a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php index 1401724125..ba6613c014 100644 --- a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php @@ -10,6 +10,10 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor { return pht('Mobile Phone App (TOTP)'); } + public function getFactorShortName() { + return pht('TOTP'); + } + public function getFactorCreateHelp() { return pht( 'Allow users to attach a mobile authenticator application (like '. @@ -38,6 +42,15 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor { 'to add a new TOTP code, continue to the next step.'); } + public function getConfigurationListDetails( + PhabricatorAuthFactorConfig $config, + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $viewer) { + + $bits = strlen($config->getFactorSecret()) * 8; + return pht('%d-Bit Secret', $bits); + } + public function processAddFactorForm( PhabricatorAuthFactorProvider $provider, AphrontFormView $form, @@ -45,6 +58,7 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor { PhabricatorUser $user) { $sync_token = $this->loadMFASyncToken( + $provider, $request, $form, $user); @@ -427,7 +441,9 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor { return null; } - protected function newMFASyncTokenProperties(PhabricatorUser $user) { + protected function newMFASyncTokenProperties( + PhabricatorAuthFactorProvider $providerr, + PhabricatorUser $user) { return array( 'secret' => self::generateNewTOTPKey(), ); diff --git a/src/applications/auth/future/PhabricatorDuoFuture.php b/src/applications/auth/future/PhabricatorDuoFuture.php index fd95906da1..81a5a2a2b8 100644 --- a/src/applications/auth/future/PhabricatorDuoFuture.php +++ b/src/applications/auth/future/PhabricatorDuoFuture.php @@ -91,11 +91,7 @@ final class PhabricatorDuoFuture $http_method = $this->getHTTPMethod(); ksort($data); - $data_parts = array(); - foreach ($data as $key => $value) { - $data_parts[] = rawurlencode($key).'='.rawurlencode($value); - } - $data_parts = implode('&', $data_parts); + $data_parts = phutil_build_http_querystring($data); $corpus = array( $date, diff --git a/src/applications/auth/query/PhabricatorAuthFactorConfigQuery.php b/src/applications/auth/query/PhabricatorAuthFactorConfigQuery.php index 1674573b68..5f838f66ba 100644 --- a/src/applications/auth/query/PhabricatorAuthFactorConfigQuery.php +++ b/src/applications/auth/query/PhabricatorAuthFactorConfigQuery.php @@ -7,6 +7,7 @@ final class PhabricatorAuthFactorConfigQuery private $phids; private $userPHIDs; private $factorProviderPHIDs; + private $factorProviderStatuses; public function withIDs(array $ids) { $this->ids = $ids; @@ -28,6 +29,11 @@ final class PhabricatorAuthFactorConfigQuery return $this; } + public function withFactorProviderStatuses(array $statuses) { + $this->factorProviderStatuses = $statuses; + return $this; + } + public function newResultObject() { return new PhabricatorAuthFactorConfig(); } @@ -42,34 +48,54 @@ final class PhabricatorAuthFactorConfigQuery if ($this->ids !== null) { $where[] = qsprintf( $conn, - 'id IN (%Ld)', + 'config.id IN (%Ld)', $this->ids); } if ($this->phids !== null) { $where[] = qsprintf( $conn, - 'phid IN (%Ls)', + 'config.phid IN (%Ls)', $this->phids); } if ($this->userPHIDs !== null) { $where[] = qsprintf( $conn, - 'userPHID IN (%Ls)', + 'config.userPHID IN (%Ls)', $this->userPHIDs); } if ($this->factorProviderPHIDs !== null) { $where[] = qsprintf( $conn, - 'factorProviderPHID IN (%Ls)', + 'config.factorProviderPHID IN (%Ls)', $this->factorProviderPHIDs); } + if ($this->factorProviderStatuses !== null) { + $where[] = qsprintf( + $conn, + 'provider.status IN (%Ls)', + $this->factorProviderStatuses); + } + return $where; } + protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) { + $joins = parent::buildJoinClauseParts($conn); + + if ($this->factorProviderStatuses !== null) { + $joins[] = qsprintf( + $conn, + 'JOIN %R provider ON config.factorProviderPHID = provider.phid', + new PhabricatorAuthFactorProvider()); + } + + return $joins; + } + protected function willFilterPage(array $configs) { $provider_phids = mpull($configs, 'getFactorProviderPHID'); @@ -94,6 +120,10 @@ final class PhabricatorAuthFactorConfigQuery return $configs; } + protected function getPrimaryTableAlias() { + return 'config'; + } + public function getQueryApplicationClass() { return 'PhabricatorAuthApplication'; } diff --git a/src/applications/auth/storage/PhabricatorAuthFactorConfig.php b/src/applications/auth/storage/PhabricatorAuthFactorConfig.php index e9a30a757e..ed5a27f543 100644 --- a/src/applications/auth/storage/PhabricatorAuthFactorConfig.php +++ b/src/applications/auth/storage/PhabricatorAuthFactorConfig.php @@ -80,6 +80,12 @@ final class PhabricatorAuthFactorConfig return $this; } + public function newSortVector() { + return id(new PhutilSortVector()) + ->addInt($this->getFactorProvider()->newStatus()->getOrder()) + ->addInt($this->getID()); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/auth/storage/PhabricatorAuthFactorProvider.php b/src/applications/auth/storage/PhabricatorAuthFactorProvider.php index 40a51e741e..2213535dff 100644 --- a/src/applications/auth/storage/PhabricatorAuthFactorProvider.php +++ b/src/applications/auth/storage/PhabricatorAuthFactorProvider.php @@ -5,7 +5,8 @@ final class PhabricatorAuthFactorProvider implements PhabricatorApplicationTransactionInterface, PhabricatorPolicyInterface, - PhabricatorExtendedPolicyInterface { + PhabricatorExtendedPolicyInterface, + PhabricatorEditEngineMFAInterface { protected $providerFactorKey; protected $name; @@ -14,15 +15,11 @@ final class PhabricatorAuthFactorProvider private $factor = self::ATTACHABLE; - const STATUS_ACTIVE = 'active'; - const STATUS_DEPRECATED = 'deprecated'; - const STATUS_DISABLED = 'disabled'; - public static function initializeNewProvider(PhabricatorAuthFactor $factor) { return id(new self()) ->setProviderFactorKey($factor->getFactorKey()) ->attachFactor($factor) - ->setStatus(self::STATUS_ACTIVE); + ->setStatus(PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE); } protected function getConfiguration() { @@ -60,6 +57,14 @@ final class PhabricatorAuthFactorProvider return $this; } + public function getEnrollMessage() { + return $this->getAuthFactorProviderProperty('enroll-message'); + } + + public function setEnrollMessage($message) { + return $this->setAuthFactorProviderProperty('enroll-message', $message); + } + public function attachFactor(PhabricatorAuthFactor $factor) { $this->factor = $factor; return $this; @@ -117,6 +122,29 @@ final class PhabricatorAuthFactorProvider return $this->getFactor()->getEnrollButtonText($this, $user); } + public function newStatus() { + $status_key = $this->getStatus(); + return PhabricatorAuthFactorProviderStatus::newForStatus($status_key); + } + + public function canCreateNewConfiguration(PhabricatorUser $user) { + return $this->getFactor()->canCreateNewConfiguration($this, $user); + } + + public function getConfigurationCreateDescription(PhabricatorUser $user) { + return $this->getFactor()->getConfigurationCreateDescription($this, $user); + } + + public function getConfigurationListDetails( + PhabricatorAuthFactorConfig $config, + PhabricatorUser $viewer) { + return $this->getFactor()->getConfigurationListDetails( + $config, + $this, + $viewer); + } + + /* -( PhabricatorApplicationTransactionInterface )------------------------- */ @@ -169,4 +197,11 @@ final class PhabricatorAuthFactorProvider } +/* -( PhabricatorEditEngineMFAInterface )---------------------------------- */ + + + public function newEditEngineMFAEngine() { + return new PhabricatorAuthFactorProviderMFAEngine(); + } + } diff --git a/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoCredentialTransaction.php b/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoCredentialTransaction.php new file mode 100644 index 0000000000..f5a52cb90f --- /dev/null +++ b/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoCredentialTransaction.php @@ -0,0 +1,69 @@ +getAuthFactorProviderProperty($key); + } + + public function applyInternalEffects($object, $value) { + $key = PhabricatorDuoAuthFactor::PROP_CREDENTIAL; + $object->setAuthFactorProviderProperty($key, $value); + } + + public function getTitle() { + return pht( + '%s changed the credential for this provider from %s to %s.', + $this->renderAuthor(), + $this->renderOldHandle(), + $this->renderNewHandle()); + } + + public function validateTransactions($object, array $xactions) { + $actor = $this->getActor(); + $errors = array(); + + if (!$this->isDuoProvider($object)) { + return $errors; + } + + $old_value = $this->generateOldValue($object); + if ($this->isEmptyTextTransaction($old_value, $xactions)) { + $errors[] = $this->newRequiredError( + pht('Duo providers must have an API credential.')); + } + + foreach ($xactions as $xaction) { + $new_value = $xaction->getNewValue(); + + if (!strlen($new_value)) { + continue; + } + + if ($new_value === $old_value) { + continue; + } + + $credential = id(new PassphraseCredentialQuery()) + ->setViewer($actor) + ->withIsDestroyed(false) + ->withPHIDs(array($new_value)) + ->executeOne(); + if (!$credential) { + $errors[] = $this->newInvalidError( + pht( + 'Credential ("%s") is not valid.', + $new_value), + $xaction); + continue; + } + } + + return $errors; + } + +} diff --git a/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoEnrollTransaction.php b/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoEnrollTransaction.php new file mode 100644 index 0000000000..e1823274b8 --- /dev/null +++ b/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoEnrollTransaction.php @@ -0,0 +1,26 @@ +getAuthFactorProviderProperty($key); + } + + public function applyInternalEffects($object, $value) { + $key = PhabricatorDuoAuthFactor::PROP_ENROLL; + $object->setAuthFactorProviderProperty($key, $value); + } + + public function getTitle() { + return pht( + '%s changed the enrollment policy for this provider from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + +} diff --git a/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoHostnameTransaction.php b/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoHostnameTransaction.php new file mode 100644 index 0000000000..27ae271137 --- /dev/null +++ b/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoHostnameTransaction.php @@ -0,0 +1,63 @@ +getAuthFactorProviderProperty($key); + } + + public function applyInternalEffects($object, $value) { + $key = PhabricatorDuoAuthFactor::PROP_HOSTNAME; + $object->setAuthFactorProviderProperty($key, $value); + } + + public function getTitle() { + return pht( + '%s changed the hostname for this provider from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + if (!$this->isDuoProvider($object)) { + return $errors; + } + + $old_value = $this->generateOldValue($object); + if ($this->isEmptyTextTransaction($old_value, $xactions)) { + $errors[] = $this->newRequiredError( + pht('Duo providers must have an API hostname.')); + } + + foreach ($xactions as $xaction) { + $new_value = $xaction->getNewValue(); + + if (!strlen($new_value)) { + continue; + } + + if ($new_value === $old_value) { + continue; + } + + try { + PhabricatorDuoAuthFactor::requireDuoAPIHostname($new_value); + } catch (Exception $ex) { + $errors[] = $this->newInvalidError( + $ex->getMessage(), + $xaction); + continue; + } + } + + return $errors; + } + +} diff --git a/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoUsernamesTransaction.php b/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoUsernamesTransaction.php new file mode 100644 index 0000000000..8d9be1244f --- /dev/null +++ b/src/applications/auth/xaction/PhabricatorAuthFactorProviderDuoUsernamesTransaction.php @@ -0,0 +1,26 @@ +getAuthFactorProviderProperty($key); + } + + public function applyInternalEffects($object, $value) { + $key = PhabricatorDuoAuthFactor::PROP_USERNAMES; + $object->setAuthFactorProviderProperty($key, $value); + } + + public function getTitle() { + return pht( + '%s changed the username policy for this provider from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + +} diff --git a/src/applications/auth/xaction/PhabricatorAuthFactorProviderEnrollMessageTransaction.php b/src/applications/auth/xaction/PhabricatorAuthFactorProviderEnrollMessageTransaction.php new file mode 100644 index 0000000000..d6d26143c1 --- /dev/null +++ b/src/applications/auth/xaction/PhabricatorAuthFactorProviderEnrollMessageTransaction.php @@ -0,0 +1,39 @@ +getEnrollMessage(); + } + + public function applyInternalEffects($object, $value) { + $object->setEnrollMessage($value); + } + + public function getTitle() { + return pht( + '%s updated the enroll message.', + $this->renderAuthor()); + } + + public function hasChangeDetailView() { + return true; + } + + public function getMailDiffSectionHeader() { + return pht('CHANGES TO ENROLL MESSAGE'); + } + + public function newChangeDetailView() { + $viewer = $this->getViewer(); + + return id(new PhabricatorApplicationTransactionTextDiffDetailView()) + ->setViewer($viewer) + ->setOldText($this->getOldValue()) + ->setNewText($this->getNewValue()); + } + +} diff --git a/src/applications/auth/xaction/PhabricatorAuthFactorProviderStatusTransaction.php b/src/applications/auth/xaction/PhabricatorAuthFactorProviderStatusTransaction.php new file mode 100644 index 0000000000..37674f7b38 --- /dev/null +++ b/src/applications/auth/xaction/PhabricatorAuthFactorProviderStatusTransaction.php @@ -0,0 +1,103 @@ +getStatus(); + } + + public function applyInternalEffects($object, $value) { + $object->setStatus($value); + } + + public function getTitle() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + $old_display = PhabricatorAuthFactorProviderStatus::newForStatus($old) + ->getName(); + $new_display = PhabricatorAuthFactorProviderStatus::newForStatus($new) + ->getName(); + + return pht( + '%s changed the status of this provider from %s to %s.', + $this->renderAuthor(), + $this->renderValue($old_display), + $this->renderValue($new_display)); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + $actor = $this->getActor(); + + $map = PhabricatorAuthFactorProviderStatus::getMap(); + foreach ($xactions as $xaction) { + $new_value = $xaction->getNewValue(); + + if (!isset($map[$new_value])) { + $errors[] = $this->newInvalidError( + pht( + 'Status "%s" is invalid. Valid statuses are: %s.', + $new_value, + implode(', ', array_keys($map))), + $xaction); + continue; + } + + $require_key = 'security.require-multi-factor-auth'; + $require_mfa = PhabricatorEnv::getEnvConfig($require_key); + + if ($require_mfa) { + $status_active = PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE; + if ($new_value !== $status_active) { + $active_providers = id(new PhabricatorAuthFactorProviderQuery()) + ->setViewer($actor) + ->withStatuses( + array( + $status_active, + )) + ->execute(); + $active_providers = mpull($active_providers, null, 'getID'); + unset($active_providers[$object->getID()]); + + if (!$active_providers) { + $errors[] = $this->newInvalidError( + pht( + 'You can not deprecate or disable the last active MFA '. + 'provider while "%s" is enabled, because new users would '. + 'be unable to enroll in MFA. Disable the MFA requirement '. + 'in Config, or create or enable another MFA provider first.', + $require_key)); + continue; + } + } + } + } + + return $errors; + } + + public function didCommitTransaction($object, $value) { + $status = PhabricatorAuthFactorProviderStatus::newForStatus($value); + + // If a provider has undergone a status change, reset the MFA enrollment + // cache for all users. This may immediately force a lot of users to redo + // MFA enrollment. + + // We could be more surgical about this: we only really need to affect + // users who had a factor under the provider, and only really need to + // do anything if a provider was disabled. This is just a little simpler. + + $table = new PhabricatorUser(); + $conn = $table->establishConnection('w'); + + queryfx( + $conn, + 'UPDATE %R SET isEnrolledInMultiFactor = 0', + $table); + } + +} diff --git a/src/applications/auth/xaction/PhabricatorAuthFactorProviderTransactionType.php b/src/applications/auth/xaction/PhabricatorAuthFactorProviderTransactionType.php index fe17eee545..3f16249e0c 100644 --- a/src/applications/auth/xaction/PhabricatorAuthFactorProviderTransactionType.php +++ b/src/applications/auth/xaction/PhabricatorAuthFactorProviderTransactionType.php @@ -1,4 +1,12 @@ getFactorKey(); + return ($provider->getProviderFactorKey() === $duo_key); + } + +} diff --git a/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php index 98fa948722..7d763d6e64 100644 --- a/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php +++ b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php @@ -12,7 +12,7 @@ final class PhabricatorAccessControlTestCase extends PhabricatorTestCase { $root = dirname(phutil_get_library_root('phabricator')); require_once $root.'/support/startup/PhabricatorStartup.php'; - $application_configuration = new AphrontDefaultApplicationConfiguration(); + $application_configuration = new AphrontApplicationConfiguration(); $host = 'meow.example.com'; diff --git a/src/applications/config/check/PhabricatorExtensionsSetupCheck.php b/src/applications/config/check/PhabricatorExtensionsSetupCheck.php index 973c80629b..51105de1c5 100644 --- a/src/applications/config/check/PhabricatorExtensionsSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtensionsSetupCheck.php @@ -11,14 +11,13 @@ final class PhabricatorExtensionsSetupCheck extends PhabricatorSetupCheck { } protected function executeChecks() { - // TODO: Make 'mbstring' and 'iconv' soft requirements. + // TODO: Make 'mbstring' a soft requirement. $required = array( 'hash', 'json', 'openssl', 'mbstring', - 'iconv', 'ctype', // There is a tiny chance we might not need this, but a significant diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php index d5f44c87c8..1c8a593a78 100644 --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -416,6 +416,10 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { 'metamta.pholio.subject-prefix' => $prefix_reason, 'metamta.phriction.subject-prefix' => $prefix_reason, + 'aphront.default-application-configuration-class' => pht( + 'This ancient extension point has been replaced with other '. + 'mechanisms, including "AphrontSite".'), + ); return $ancient_config; diff --git a/src/applications/config/controller/PhabricatorConfigVersionController.php b/src/applications/config/controller/PhabricatorConfigVersionController.php index 8a87dec5cc..a9571a1f85 100644 --- a/src/applications/config/controller/PhabricatorConfigVersionController.php +++ b/src/applications/config/controller/PhabricatorConfigVersionController.php @@ -63,6 +63,8 @@ final class PhabricatorConfigVersionController $version_from_file); } + $version_property_list->addProperty('php', phpversion()); + $binaries = PhutilBinaryAnalyzer::getAllBinaries(); foreach ($binaries as $binary) { if (!$binary->isBinaryAvailable()) { diff --git a/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php b/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php index 28a0f619dd..0a3ea32e44 100644 --- a/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php +++ b/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php @@ -36,14 +36,6 @@ final class PhabricatorExtendingPhabricatorConfigOptions 'occur. Specify a list of classes which extend '. 'PhabricatorEventListener here.')) ->addExample('MyEventListener', pht('Valid Setting')), - $this->newOption( - 'aphront.default-application-configuration-class', - 'class', - 'AphrontDefaultApplicationConfiguration') - ->setLocked(true) - ->setBaseClass('AphrontApplicationConfiguration') - // TODO: This could probably use some better documentation. - ->setDescription(pht('Application configuration class.')), ); } diff --git a/src/applications/differential/editor/DifferentialRevisionEditEngine.php b/src/applications/differential/editor/DifferentialRevisionEditEngine.php index 07b693044f..9c399036d5 100644 --- a/src/applications/differential/editor/DifferentialRevisionEditEngine.php +++ b/src/applications/differential/editor/DifferentialRevisionEditEngine.php @@ -235,6 +235,32 @@ final class DifferentialRevisionEditEngine ->setConduitTypeDescription(pht('List of tasks.')) ->setValue(array()); + $fields[] = id(new PhabricatorHandlesEditField()) + ->setKey('parents') + ->setUseEdgeTransactions(true) + ->setIsFormField(false) + ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) + ->setMetadataValue( + 'edge:type', + DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST) + ->setDescription(pht('Parent revisions of this revision.')) + ->setConduitDescription(pht('Change associated parent revisions.')) + ->setConduitTypeDescription(pht('List of revisions.')) + ->setValue(array()); + + $fields[] = id(new PhabricatorHandlesEditField()) + ->setKey('children') + ->setUseEdgeTransactions(true) + ->setIsFormField(false) + ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) + ->setMetadataValue( + 'edge:type', + DifferentialRevisionDependedOnByRevisionEdgeType::EDGECONST) + ->setDescription(pht('Child revisions of this revision.')) + ->setConduitDescription(pht('Change associated child revisions.')) + ->setConduitTypeDescription(pht('List of revisions.')) + ->setValue(array()); + $actions = DifferentialRevisionActionTransaction::loadAllActions(); $actions = msortv($actions, 'getRevisionActionOrderVector'); diff --git a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php index d7d7c767e1..861d2ad220 100644 --- a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php +++ b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php @@ -177,14 +177,21 @@ final class DifferentialDiffExtractionEngine extends Phobject { 'repository' => $repository, )); - $response = DiffusionQuery::callConduitWithDiffusionRequest( - $viewer, - $drequest, - 'diffusion.filecontentquery', - array( - 'commit' => $identifier, - 'path' => $path, - )); + try { + $response = DiffusionQuery::callConduitWithDiffusionRequest( + $viewer, + $drequest, + 'diffusion.filecontentquery', + array( + 'commit' => $identifier, + 'path' => $path, + )); + } catch (Exception $ex) { + // TODO: See PHI1044. This call may fail if the diff deleted the + // file. If the call fails, just detect a change for now. This should + // generally be made cleaner in the future. + return true; + } $new_file_phid = $response['filePHID']; if (!$new_file_phid) { diff --git a/src/applications/diffusion/controller/DiffusionServeController.php b/src/applications/diffusion/controller/DiffusionServeController.php index 5a5c446a29..cb4ad0ba95 100644 --- a/src/applications/diffusion/controller/DiffusionServeController.php +++ b/src/applications/diffusion/controller/DiffusionServeController.php @@ -528,7 +528,7 @@ final class DiffusionServeController extends DiffusionController { unset($query_data[$key]); } } - $query_string = http_build_query($query_data, '', '&'); + $query_string = phutil_build_http_querystring($query_data); // We're about to wipe out PATH with the rest of the environment, so // resolve the binary first. diff --git a/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php b/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php index c72021f0c1..717e730ab1 100644 --- a/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php +++ b/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php @@ -188,7 +188,7 @@ final class DiffusionRepositoryClusterEngine extends Phobject { if ($this_version) { $this_version = (int)$this_version->getRepositoryVersion(); } else { - $this_version = -1; + $this_version = null; } if ($versions) { @@ -197,7 +197,7 @@ final class DiffusionRepositoryClusterEngine extends Phobject { // leader, we want to fetch from a leader and then update our version. $max_version = (int)max(mpull($versions, 'getRepositoryVersion')); - if ($max_version > $this_version) { + if (($this_version === null) || ($max_version > $this_version)) { if ($repository->isHosted()) { $fetchable = array(); foreach ($versions as $version) { @@ -206,6 +206,7 @@ final class DiffusionRepositoryClusterEngine extends Phobject { } } + $this->synchronizeWorkingCopyFromDevices( $fetchable, $this_version, @@ -445,10 +446,10 @@ final class DiffusionRepositoryClusterEngine extends Phobject { if ($this_version) { $this_version = (int)$this_version->getRepositoryVersion(); } else { - $this_version = -1; + $this_version = null; } - if ($new_version > $this_version) { + if (($this_version === null) || ($new_version > $this_version)) { PhabricatorRepositoryWorkingCopyVersion::updateVersion( $repository_phid, $device_phid, diff --git a/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php index 02cacbfe94..b2d1d25f44 100644 --- a/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php +++ b/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php @@ -222,8 +222,10 @@ abstract class DiffusionSSHWorkflow extends PhabricatorSSHWorkflow { pht('No repository "%s" exists!', $identifier)); } + $is_cluster = $this->getIsClusterRequest(); + $protocol = PhabricatorRepositoryURI::BUILTIN_PROTOCOL_SSH; - if (!$repository->canServeProtocol($protocol, false)) { + if (!$repository->canServeProtocol($protocol, false, $is_cluster)) { throw new Exception( pht( 'This repository ("%s") is not available over SSH.', diff --git a/src/applications/passphrase/controller/PassphraseCredentialRevealController.php b/src/applications/passphrase/controller/PassphraseCredentialRevealController.php index 3a40d253c9..99b6711ae6 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialRevealController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialRevealController.php @@ -21,12 +21,8 @@ final class PassphraseCredentialRevealController return new Aphront404Response(); } - $view_uri = '/K'.$credential->getID(); + $view_uri = $credential->getURI(); - $token = id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession( - $viewer, - $request, - $view_uri); $is_locked = $credential->getIsLocked(); if ($is_locked) { @@ -39,7 +35,7 @@ final class PassphraseCredentialRevealController ->addCancelButton($view_uri); } - if ($request->isFormPost()) { + if ($request->isFormOrHisecPost()) { $secret = $credential->getSecret(); if (!$secret) { $body = pht('This credential has no associated secret.'); @@ -76,6 +72,7 @@ final class PassphraseCredentialRevealController $editor = id(new PassphraseCredentialTransactionEditor()) ->setActor($viewer) + ->setCancelURI($view_uri) ->setContinueOnNoEffect(true) ->setContentSourceFromRequest($request) ->applyTransactions($credential, $xactions); diff --git a/src/applications/passphrase/storage/PassphraseCredential.php b/src/applications/passphrase/storage/PassphraseCredential.php index b10d392d36..c470ea661f 100644 --- a/src/applications/passphrase/storage/PassphraseCredential.php +++ b/src/applications/passphrase/storage/PassphraseCredential.php @@ -52,6 +52,10 @@ final class PassphraseCredential extends PassphraseDAO return 'K'.$this->getID(); } + public function getURI() { + return '/'.$this->getMonogram(); + } + protected function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, diff --git a/src/applications/passphrase/xaction/PassphraseCredentialLookedAtTransaction.php b/src/applications/passphrase/xaction/PassphraseCredentialLookedAtTransaction.php index 3d8cb36f31..fc76ab0d56 100644 --- a/src/applications/passphrase/xaction/PassphraseCredentialLookedAtTransaction.php +++ b/src/applications/passphrase/xaction/PassphraseCredentialLookedAtTransaction.php @@ -30,4 +30,10 @@ final class PassphraseCredentialLookedAtTransaction return 'blue'; } + public function shouldTryMFA( + $object, + PhabricatorApplicationTransaction $xaction) { + return true; + } + } diff --git a/src/applications/people/controller/PhabricatorPeopleRenameController.php b/src/applications/people/controller/PhabricatorPeopleRenameController.php index 42ff2e7988..42eebfc8ae 100644 --- a/src/applications/people/controller/PhabricatorPeopleRenameController.php +++ b/src/applications/people/controller/PhabricatorPeopleRenameController.php @@ -17,14 +17,9 @@ final class PhabricatorPeopleRenameController $done_uri = $this->getApplicationURI("manage/{$id}/"); - id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession( - $viewer, - $request, - $done_uri); - $validation_exception = null; $username = $user->getUsername(); - if ($request->isFormPost()) { + if ($request->isFormOrHisecPost()) { $username = $request->getStr('username'); $xactions = array(); @@ -36,6 +31,7 @@ final class PhabricatorPeopleRenameController $editor = id(new PhabricatorUserTransactionEditor()) ->setActor($viewer) ->setContentSourceFromRequest($request) + ->setCancelURI($done_uri) ->setContinueOnMissingFields(true); try { diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index ec1890cdb0..429d4f7a7c 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -978,9 +978,15 @@ final class PhabricatorUser * @task factors */ public function updateMultiFactorEnrollment() { - $factors = id(new PhabricatorAuthFactorConfig())->loadAllWhere( - 'userPHID = %s', - $this->getPHID()); + $factors = id(new PhabricatorAuthFactorConfigQuery()) + ->setViewer($this) + ->withUserPHIDs(array($this->getPHID())) + ->withFactorProviderStatuses( + array( + PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE, + PhabricatorAuthFactorProviderStatus::STATUS_DEPRECATED, + )) + ->execute(); $enrolled = count($factors) ? 1 : 0; if ($enrolled !== $this->isEnrolledInMultiFactor) { diff --git a/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php b/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php index db134a5c78..b6d23b3511 100644 --- a/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php +++ b/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php @@ -89,4 +89,11 @@ final class PhabricatorUserUsernameTransaction return null; } + + public function shouldTryMFA( + $object, + PhabricatorApplicationTransaction $xaction) { + return true; + } + } diff --git a/src/applications/project/config/PhabricatorProjectConfigOptions.php b/src/applications/project/config/PhabricatorProjectConfigOptions.php index c61faa64fb..2d87bf159f 100644 --- a/src/applications/project/config/PhabricatorProjectConfigOptions.php +++ b/src/applications/project/config/PhabricatorProjectConfigOptions.php @@ -83,6 +83,34 @@ EOTEXT $custom_field_type = 'custom:PhabricatorCustomFieldConfigOptionType'; + + $subtype_type = 'projects.subtypes'; + $subtype_default_key = PhabricatorEditEngineSubtype::SUBTYPE_DEFAULT; + $subtype_example = array( + array( + 'key' => $subtype_default_key, + 'name' => pht('Project'), + ), + array( + 'key' => 'team', + 'name' => pht('Team'), + ), + ); + $subtype_example = id(new PhutilJSON())->encodeAsList($subtype_example); + + $subtype_default = array( + array( + 'key' => $subtype_default_key, + 'name' => pht('Project'), + ), + ); + + $subtype_description = $this->deformat(pht(<<newOption('projects.custom-field-definitions', 'wild', array()) ->setSummary(pht('Custom Projects fields.')) @@ -102,6 +130,11 @@ EOTEXT $this->newOption('projects.colors', $colors_type, $default_colors) ->setSummary(pht('Adjust project colors.')) ->setDescription($colors_description), + $this->newOption('projects.subtypes', $subtype_type, $subtype_default) + ->setSummary(pht('Define project subtypes.')) + ->setDescription($subtype_description) + ->addExample($subtype_example, pht('Simple Subtypes')), + ); } diff --git a/src/applications/project/config/PhabricatorProjectSubtypesConfigType.php b/src/applications/project/config/PhabricatorProjectSubtypesConfigType.php new file mode 100644 index 0000000000..7603ad7683 --- /dev/null +++ b/src/applications/project/config/PhabricatorProjectSubtypesConfigType.php @@ -0,0 +1,14 @@ +renderWatchAction($project); $header->addActionLink($watch_action); + $subtype = $project->newSubtypeObject(); + if ($subtype && $subtype->hasTagView()) { + $subtype_tag = $subtype->newTagView(); + $header->addTag($subtype_tag); + } + $milestone_list = $this->buildMilestoneList($project); $subproject_list = $this->buildSubprojectList($project); diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php index ee2c087085..b714f66830 100644 --- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php @@ -249,6 +249,17 @@ final class PhabricatorProjectTransactionEditor ->rematerialize($new_parent); } + // See PHI1046. Milestones are always in the Space of their parent project. + // Synchronize the database values to match the application values. + $conn = $object->establishConnection('w'); + queryfx( + $conn, + 'UPDATE %R SET spacePHID = %ns + WHERE parentProjectPHID = %s AND milestoneNumber IS NOT NULL', + $object, + $object->getSpacePHID(), + $object->getPHID()); + return parent::applyFinalEffects($object, $xactions); } diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php index 6f81bc5e68..f6087f7d2a 100644 --- a/src/applications/project/query/PhabricatorProjectQuery.php +++ b/src/applications/project/query/PhabricatorProjectQuery.php @@ -24,6 +24,7 @@ final class PhabricatorProjectQuery private $maxDepth; private $minMilestoneNumber; private $maxMilestoneNumber; + private $subtypes; private $status = 'status-any'; const STATUS_ANY = 'status-any'; @@ -131,6 +132,11 @@ final class PhabricatorProjectQuery return $this; } + public function withSubtypes(array $subtypes) { + $this->subtypes = $subtypes; + return $this; + } + public function needMembers($need_members) { $this->needMembers = $need_members; return $this; @@ -618,6 +624,13 @@ final class PhabricatorProjectQuery $this->maxMilestoneNumber); } + if ($this->subtypes !== null) { + $where[] = qsprintf( + $conn, + 'subtype IN (%Ls)', + $this->subtypes); + } + return $where; } diff --git a/src/applications/project/query/PhabricatorProjectSearchEngine.php b/src/applications/project/query/PhabricatorProjectSearchEngine.php index 8b69a86fac..1357233b11 100644 --- a/src/applications/project/query/PhabricatorProjectSearchEngine.php +++ b/src/applications/project/query/PhabricatorProjectSearchEngine.php @@ -19,6 +19,9 @@ final class PhabricatorProjectSearchEngine } protected function buildCustomSearchFields() { + $subtype_map = id(new PhabricatorProject())->newEditEngineSubtypeMap(); + $hide_subtypes = ($subtype_map->getCount() == 1); + return array( id(new PhabricatorSearchTextField()) ->setLabel(pht('Name')) @@ -62,6 +65,14 @@ final class PhabricatorProjectSearchEngine pht( 'Pass true to find only milestones, or false to omit '. 'milestones.')), + id(new PhabricatorSearchDatasourceField()) + ->setLabel(pht('Subtypes')) + ->setKey('subtypes') + ->setAliases(array('subtype')) + ->setDescription( + pht('Search for projects with given subtypes.')) + ->setDatasource(new PhabricatorProjectSubtypeDatasource()) + ->setIsHidden($hide_subtypes), id(new PhabricatorSearchCheckboxesField()) ->setLabel(pht('Icons')) ->setKey('icons') @@ -134,6 +145,10 @@ final class PhabricatorProjectSearchEngine $query->withAncestorProjectPHIDs($map['ancestorPHIDs']); } + if ($map['subtypes']) { + $query->withSubtypes($map['subtypes']); + } + return $query; } diff --git a/src/applications/project/storage/PhabricatorProject.php b/src/applications/project/storage/PhabricatorProject.php index 4dae13f3c6..5182a941bf 100644 --- a/src/applications/project/storage/PhabricatorProject.php +++ b/src/applications/project/storage/PhabricatorProject.php @@ -12,7 +12,8 @@ final class PhabricatorProject extends PhabricatorProjectDAO PhabricatorFerretInterface, PhabricatorConduitResultInterface, PhabricatorColumnProxyInterface, - PhabricatorSpacesInterface { + PhabricatorSpacesInterface, + PhabricatorEditEngineSubtypeInterface { protected $name; protected $status = PhabricatorProjectStatus::STATUS_ACTIVE; @@ -40,6 +41,7 @@ final class PhabricatorProject extends PhabricatorProjectDAO protected $properties = array(); protected $spacePHID; + protected $subtype; private $memberPHIDs = self::ATTACHABLE; private $watcherPHIDs = self::ATTACHABLE; @@ -102,6 +104,7 @@ final class PhabricatorProject extends PhabricatorProjectDAO ->setHasWorkboard(0) ->setHasMilestones(0) ->setHasSubprojects(0) + ->setSubtype(PhabricatorEditEngineSubtype::SUBTYPE_DEFAULT) ->attachParentProject(null); } @@ -237,6 +240,7 @@ final class PhabricatorProject extends PhabricatorProjectDAO 'projectPath' => 'hashpath64', 'projectDepth' => 'uint32', 'projectPathKey' => 'bytes4', + 'subtype' => 'text64', ), self::CONFIG_KEY_SCHEMA => array( 'key_icon' => array( @@ -765,6 +769,10 @@ final class PhabricatorProject extends PhabricatorProjectDAO ->setKey('slug') ->setType('string') ->setDescription(pht('Primary slug/hashtag.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('subtype') + ->setType('string') + ->setDescription(pht('Subtype of the project.')), id(new PhabricatorConduitSearchFieldSpecification()) ->setKey('milestone') ->setType('int?') @@ -814,6 +822,7 @@ final class PhabricatorProject extends PhabricatorProjectDAO return array( 'name' => $this->getName(), 'slug' => $this->getPrimarySlug(), + 'subtype' => $this->getSubtype(), 'milestone' => $milestone, 'depth' => (int)$this->getProjectDepth(), 'parent' => $parent_ref, @@ -873,4 +882,26 @@ final class PhabricatorProject extends PhabricatorProjectDAO } +/* -( PhabricatorEditEngineSubtypeInterface )------------------------------ */ + + + public function getEditEngineSubtype() { + return $this->getSubtype(); + } + + public function setEditEngineSubtype($value) { + return $this->setSubtype($value); + } + + public function newEditEngineSubtypeMap() { + $config = PhabricatorEnv::getEnvConfig('projects.subtypes'); + return PhabricatorEditEngineSubtype::newSubtypeMap($config); + } + + public function newSubtypeObject() { + $subtype_key = $this->getEditEngineSubtype(); + $subtype_map = $this->newEditEngineSubtypeMap(); + return $subtype_map->getSubtype($subtype_key); + } + } diff --git a/src/applications/project/typeahead/PhabricatorProjectSubtypeDatasource.php b/src/applications/project/typeahead/PhabricatorProjectSubtypeDatasource.php new file mode 100644 index 0000000000..68de11e630 --- /dev/null +++ b/src/applications/project/typeahead/PhabricatorProjectSubtypeDatasource.php @@ -0,0 +1,45 @@ +buildResults(); + return $this->filterResultsAgainstTokens($results); + } + + protected function renderSpecialTokens(array $values) { + return $this->renderTokensFromResults($this->buildResults(), $values); + } + + private function buildResults() { + $results = array(); + + $subtype_map = id(new PhabricatorProject())->newEditEngineSubtypeMap(); + foreach ($subtype_map->getSubtypes() as $key => $subtype) { + + $result = id(new PhabricatorTypeaheadResult()) + ->setIcon($subtype->getIcon()) + ->setColor($subtype->getColor()) + ->setPHID($key) + ->setName($subtype->getName()); + + $results[$key] = $result; + } + + return $results; + } + +} diff --git a/src/applications/project/view/PhabricatorProjectListView.php b/src/applications/project/view/PhabricatorProjectListView.php index 645440d831..d8fb011c2e 100644 --- a/src/applications/project/view/PhabricatorProjectListView.php +++ b/src/applications/project/view/PhabricatorProjectListView.php @@ -87,6 +87,13 @@ final class PhabricatorProjectListView extends AphrontView { } } + $subtype = $project->newSubtypeObject(); + if ($subtype && $subtype->hasTagView()) { + $subtype_tag = $subtype->newTagView() + ->setSlimShady(true); + $item->addAttribute($subtype_tag); + } + $list->addItem($item); } diff --git a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php index 332d67f7af..125ee833f8 100644 --- a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php +++ b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php @@ -73,6 +73,9 @@ final class PhabricatorRepositoryPullLocalDaemon $futures = array(); $queue = array(); + $sync_wait = phutil_units('2 minutes in seconds'); + $last_sync = array(); + while (!$this->shouldExit()) { PhabricatorCaches::destroyRequestCache(); $device = AlmanacKeys::getLiveDevice(); @@ -96,6 +99,37 @@ final class PhabricatorRepositoryPullLocalDaemon $retry_after[$message->getRepositoryID()] = time(); } + if ($device) { + $unsynchronized = $this->loadUnsynchronizedRepositories($device); + $now = PhabricatorTime::getNow(); + foreach ($unsynchronized as $repository) { + $id = $repository->getID(); + + $this->log( + pht( + 'Cluster repository ("%s") is out of sync on this node ("%s").', + $repository->getDisplayName(), + $device->getName())); + + // Don't let out-of-sync conditions trigger updates too frequently, + // since we don't want to get trapped in a death spiral if sync is + // failing. + $sync_at = idx($last_sync, $id, 0); + $wait_duration = ($now - $sync_at); + if ($wait_duration < $sync_wait) { + $this->log( + pht( + 'Skipping forced out-of-sync update because the last update '. + 'was too recent (%s seconds ago).', + $wait_duration)); + continue; + } + + $last_sync[$id] = $now; + $retry_after[$id] = $now; + } + } + // If any repositories were deleted, remove them from the retry timer map // so we don't end up with a retry timer that never gets updated and // causes us to sleep for the minimum amount of time. @@ -521,4 +555,41 @@ final class PhabricatorRepositoryPullLocalDaemon return false; } + private function loadUnsynchronizedRepositories(AlmanacDevice $device) { + $viewer = $this->getViewer(); + $table = new PhabricatorRepositoryWorkingCopyVersion(); + $conn = $table->establishConnection('r'); + + $our_versions = queryfx_all( + $conn, + 'SELECT repositoryPHID, repositoryVersion FROM %R WHERE devicePHID = %s', + $table, + $device->getPHID()); + $our_versions = ipull($our_versions, 'repositoryVersion', 'repositoryPHID'); + + $max_versions = queryfx_all( + $conn, + 'SELECT repositoryPHID, MAX(repositoryVersion) maxVersion FROM %R + GROUP BY repositoryPHID', + $table); + $max_versions = ipull($max_versions, 'maxVersion', 'repositoryPHID'); + + $unsynchronized_phids = array(); + foreach ($max_versions as $repository_phid => $max_version) { + $our_version = idx($our_versions, $repository_phid); + if (($our_version === null) || ($our_version < $max_version)) { + $unsynchronized_phids[] = $repository_phid; + } + } + + if (!$unsynchronized_phids) { + return array(); + } + + return id(new PhabricatorRepositoryQuery()) + ->setViewer($viewer) + ->withPHIDs($unsynchronized_phids) + ->execute(); + } + } diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementWorkflow.php index 654a974e6c..6d48d7c5d1 100644 --- a/src/applications/repository/management/PhabricatorRepositoryManagementWorkflow.php +++ b/src/applications/repository/management/PhabricatorRepositoryManagementWorkflow.php @@ -7,7 +7,7 @@ abstract class PhabricatorRepositoryManagementWorkflow $identifiers = $args->getArg($param); if (!$identifiers) { - return null; + return array(); } $query = id(new PhabricatorRepositoryQuery()) diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index ed90f47f13..e66fb78afa 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -1506,9 +1506,18 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return $this->setDetail('hosting-enabled', $enabled); } - public function canServeProtocol($protocol, $write) { - if (!$this->isTracked()) { - return false; + public function canServeProtocol( + $protocol, + $write, + $is_intracluster = false) { + + // See T13192. If a repository is inactive, don't serve it to users. We + // still synchronize it within the cluster and serve it to other repository + // nodes. + if (!$is_intracluster) { + if (!$this->isTracked()) { + return false; + } } $clone_uris = $this->getCloneURIs(); diff --git a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php index d5cbf75951..6809b51334 100644 --- a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php @@ -53,8 +53,8 @@ final class PhabricatorMultiFactorSettingsPanel $factors = id(new PhabricatorAuthFactorConfigQuery()) ->setViewer($viewer) ->withUserPHIDs(array($user->getPHID())) - ->setOrderVector(array('-id')) ->execute(); + $factors = msort($factors, 'newSortVector'); $rows = array(); $rowc = array(); @@ -69,7 +69,18 @@ final class PhabricatorMultiFactorSettingsPanel $rowc[] = null; } + $status = $provider->newStatus(); + $status_icon = $status->getFactorIcon(); + $status_color = $status->getFactorColor(); + + $icon = id(new PHUIIconView()) + ->setIcon("{$status_icon} {$status_color}") + ->setTooltip(pht('Provider: %s', $status->getName())); + + $details = $provider->getConfigurationListDetails($factor, $viewer); + $rows[] = array( + $icon, javelin_tag( 'a', array( @@ -77,7 +88,9 @@ final class PhabricatorMultiFactorSettingsPanel 'sigil' => 'workflow', ), $factor->getFactorName()), + $provider->getFactor()->getFactorShortName(), $provider->getDisplayName(), + $details, phabricator_datetime($factor->getDateCreated(), $viewer), javelin_tag( 'a', @@ -95,15 +108,21 @@ final class PhabricatorMultiFactorSettingsPanel pht("You haven't added any authentication factors to your account yet.")); $table->setHeaders( array( + null, pht('Name'), pht('Type'), + pht('Provider'), + pht('Details'), pht('Created'), - '', + null, )); $table->setColumnClasses( array( + null, 'wide pri', - '', + null, + null, + null, 'right', 'action', )); @@ -111,6 +130,9 @@ final class PhabricatorMultiFactorSettingsPanel $table->setDeviceVisibility( array( true, + true, + false, + false, false, false, true, @@ -129,12 +151,15 @@ final class PhabricatorMultiFactorSettingsPanel $add_color = PHUIButtonView::GREY; } + $can_add = (bool)$this->loadActiveMFAProviders(); + $buttons[] = id(new PHUIButtonView()) ->setTag('a') ->setIcon('fa-plus') ->setText(pht('Add Auth Factor')) ->setHref($this->getPanelURI('?new=true')) ->setWorkflow(true) + ->setDisabled(!$can_add) ->setColor($add_color); $buttons[] = id(new PHUIButtonView()) @@ -155,21 +180,18 @@ final class PhabricatorMultiFactorSettingsPanel // Check that we have providers before we send the user through the MFA // gate, so you don't authenticate and then immediately get roadblocked. - $providers = id(new PhabricatorAuthFactorProviderQuery()) - ->setViewer($viewer) - ->withStatuses(array(PhabricatorAuthFactorProvider::STATUS_ACTIVE)) - ->execute(); + $providers = $this->loadActiveMFAProviders(); + if (!$providers) { return $this->newDialog() ->setTitle(pht('No MFA Providers')) ->appendParagraph( pht( - 'There are no active MFA providers. At least one active provider '. - 'must be available to add new MFA factors.')) + 'This install does not have any active MFA providers configured. '. + 'At least one provider must be configured and active before you '. + 'can add new MFA factors.')) ->addCancelButton($cancel_uri); } - $providers = mpull($providers, null, 'getPHID'); - $proivders = msortv($providers, 'newSortVector'); $token = id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession( $viewer, @@ -184,8 +206,7 @@ final class PhabricatorMultiFactorSettingsPanel // Only let the user continue creating a factor for a given provider if // they actually pass the provider's checks. - $selected_factor = $selected_provider->getFactor(); - if (!$selected_factor->canCreateNewConfiguration($viewer)) { + if (!$selected_provider->canCreateNewConfiguration($viewer)) { $selected_provider = null; } } @@ -200,8 +221,7 @@ final class PhabricatorMultiFactorSettingsPanel $provider_uri = id(new PhutilURI($this->getPanelURI())) ->setQueryParam('providerPHID', $provider_phid); - $factor = $provider->getFactor(); - $is_enabled = $factor->canCreateNewConfiguration($viewer); + $is_enabled = $provider->canCreateNewConfiguration($viewer); $item = id(new PHUIObjectItemView()) ->setHeader($provider->getDisplayName()) @@ -216,7 +236,7 @@ final class PhabricatorMultiFactorSettingsPanel $item->setDisabled(true); } - $create_description = $factor->getConfigurationCreateDescription( + $create_description = $provider->getConfigurationCreateDescription( $viewer); if ($create_description) { $item->appendChild($create_description); @@ -236,13 +256,16 @@ final class PhabricatorMultiFactorSettingsPanel // sometimes requires us to push a challenge to them as a side effect (for // example, with SMS). if (!$request->isFormPost() || !$request->getBool('mfa.start')) { - $description = $selected_provider->getEnrollDescription($viewer); + $enroll = $selected_provider->getEnrollMessage(); + if (!strlen($enroll)) { + $enroll = $selected_provider->getEnrollDescription($viewer); + } return $this->newDialog() ->addHiddenInput('providerPHID', $selected_provider->getPHID()) ->addHiddenInput('mfa.start', 1) ->setTitle(pht('Add Authentication Factor')) - ->appendChild(new PHUIRemarkupView($viewer, $description)) + ->appendChild(new PHUIRemarkupView($viewer, $enroll)) ->addCancelButton($cancel_uri) ->addSubmitButton($selected_provider->getEnrollButtonText($viewer)); } @@ -424,5 +447,22 @@ final class PhabricatorMultiFactorSettingsPanel ->setDialog($dialog); } + private function loadActiveMFAProviders() { + $viewer = $this->getViewer(); + + $providers = id(new PhabricatorAuthFactorProviderQuery()) + ->setViewer($viewer) + ->withStatuses( + array( + PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE, + )) + ->execute(); + + $providers = mpull($providers, null, 'getPHID'); + $providers = msortv($providers, 'newSortVector'); + + return $providers; + } + } diff --git a/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php b/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php index 0394432ff3..0edc0b3f5a 100644 --- a/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php +++ b/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php @@ -68,6 +68,7 @@ final class TransactionSearchConduitAPIMethod $object); $xaction_query + ->needHandles(false) ->withObjectPHIDs(array($object->getPHID())) ->setViewer($viewer); diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index b3106d27b2..feb783e724 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -1279,14 +1279,41 @@ abstract class PhabricatorEditEngine $fields = $this->willBuildEditForm($object, $fields); + $request_path = $request->getRequestURI() + ->setQueryParams(array()); + $form = id(new AphrontFormView()) ->setUser($viewer) + ->setAction($request_path) ->addHiddenInput('editEngine', 'true'); foreach ($this->contextParameters as $param) { $form->addHiddenInput($param, $request->getStr($param)); } + $requires_mfa = false; + if ($object instanceof PhabricatorEditEngineMFAInterface) { + $mfa_engine = PhabricatorEditEngineMFAEngine::newEngineForObject($object) + ->setViewer($viewer); + $requires_mfa = $mfa_engine->shouldRequireMFA(); + } + + if ($requires_mfa) { + $message = pht( + 'You will be required to provide multi-factor credentials to make '. + 'changes.'); + $form->appendChild( + id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_MFA) + ->setErrors(array($message))); + + // TODO: This should also set workflow on the form, so the user doesn't + // lose any form data if they "Cancel". However, Maniphest currently + // overrides "newEditResponse()" if the request is Ajax and returns a + // bag of view data. This can reasonably be cleaned up when workboards + // get their next iteration. + } + foreach ($fields as $field) { if (!$field->getIsFormField()) { continue; @@ -1565,11 +1592,19 @@ abstract class PhabricatorEditEngine $comment_uri = $this->getEditURI($object, 'comment/'); + $requires_mfa = false; + if ($object instanceof PhabricatorEditEngineMFAInterface) { + $mfa_engine = PhabricatorEditEngineMFAEngine::newEngineForObject($object) + ->setViewer($viewer); + $requires_mfa = $mfa_engine->shouldRequireMFA(); + } + $view = id(new PhabricatorApplicationTransactionCommentView()) ->setUser($viewer) ->setObjectPHID($object_phid) ->setHeaderText($header_text) ->setAction($comment_uri) + ->setRequiresMFA($requires_mfa) ->setSubmitButtonName($button_text); $draft = PhabricatorVersionedDraft::loadDraft( diff --git a/src/applications/transactions/editfield/PhabricatorCredentialEditField.php b/src/applications/transactions/editfield/PhabricatorCredentialEditField.php new file mode 100644 index 0000000000..7c70bf288e --- /dev/null +++ b/src/applications/transactions/editfield/PhabricatorCredentialEditField.php @@ -0,0 +1,43 @@ +credentialType = $credential_type; + return $this; + } + + public function getCredentialType() { + return $this->credentialType; + } + + public function setCredentials(array $credentials) { + $this->credentials = $credentials; + return $this; + } + + public function getCredentials() { + return $this->credentials; + } + + protected function newControl() { + $control = id(new PassphraseCredentialControl()) + ->setCredentialType($this->getCredentialType()) + ->setOptions($this->getCredentials()); + + return $control; + } + + protected function newHTTPParameterType() { + return new AphrontPHIDHTTPParameterType(); + } + + protected function newConduitParameterType() { + return new ConduitPHIDParameterType(); + } + +} diff --git a/src/applications/transactions/editfield/PhabricatorSpaceEditField.php b/src/applications/transactions/editfield/PhabricatorSpaceEditField.php index ee15f0b19e..c15213bd93 100644 --- a/src/applications/transactions/editfield/PhabricatorSpaceEditField.php +++ b/src/applications/transactions/editfield/PhabricatorSpaceEditField.php @@ -28,7 +28,6 @@ final class PhabricatorSpaceEditField return new ConduitPHIDParameterType(); } - public function shouldReadValueFromRequest() { return $this->getPolicyField()->shouldReadValueFromRequest(); } diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 532edb6742..37ed1547fb 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -88,6 +88,7 @@ abstract class PhabricatorApplicationTransactionEditor private $hasRequiredMFA = false; private $request; private $cancelURI; + private $extensions; const STORAGE_ENCODING_BINARY = 'binary'; @@ -1013,6 +1014,7 @@ abstract class PhabricatorApplicationTransactionEditor } $errors[] = $this->validateAllTransactions($object, $xactions); + $errors[] = $this->validateTransactionsWithExtensions($object, $xactions); $errors = array_mergev($errors); $continue_on_missing = $this->getContinueOnMissingFields(); @@ -2670,9 +2672,15 @@ abstract class PhabricatorApplicationTransactionEditor $transaction_type) { $errors = array(); - $factors = id(new PhabricatorAuthFactorConfig())->loadAllWhere( - 'userPHID = %s', - $this->getActingAsPHID()); + $factors = id(new PhabricatorAuthFactorConfigQuery()) + ->setViewer($this->getActor()) + ->withUserPHIDs(array($this->getActingAsPHID())) + ->withFactorProviderStatuses( + array( + PhabricatorAuthFactorProviderStatus::STATUS_ACTIVE, + PhabricatorAuthFactorProviderStatus::STATUS_DEPRECATED, + )) + ->execute(); foreach ($xactions as $xaction) { if (!$factors) { @@ -3289,7 +3297,7 @@ abstract class PhabricatorApplicationTransactionEditor // move the other transactions down so they provide context above the // actual comment. - $comment = $xaction->getBodyForMail(); + $comment = $this->getBodyForTextMail($xaction); if ($comment !== null) { $is_comment = true; $comments[] = array( @@ -3302,12 +3310,12 @@ abstract class PhabricatorApplicationTransactionEditor } if (!$is_comment || !$seen_comment) { - $header = $xaction->getTitleForTextMail(); + $header = $this->getTitleForTextMail($xaction); if ($header !== null) { $headers[] = $header; } - $header_html = $xaction->getTitleForHTMLMail(); + $header_html = $this->getTitleForHTMLMail($xaction); if ($header_html !== null) { $headers_html[] = $header_html; } @@ -3387,12 +3395,12 @@ abstract class PhabricatorApplicationTransactionEditor // If this is not the first comment in the mail, add the header showing // who wrote the comment immediately above the comment. if (!$is_initial) { - $header = $xaction->getTitleForTextMail(); + $header = $this->getTitleForTextMail($xaction); if ($header !== null) { $body->addRawPlaintextSection($header); } - $header_html = $xaction->getTitleForHTMLMail(); + $header_html = $this->getTitleForHTMLMail($xaction); if ($header_html !== null) { $body->addRawHTMLSection($header_html); } @@ -4851,6 +4859,13 @@ abstract class PhabricatorApplicationTransactionEditor } private function requireMFA(PhabricatorLiskDAO $object, array $xactions) { + $actor = $this->getActor(); + + // Let omnipotent editors skip MFA. This is mostly aimed at scripts. + if ($actor->isOmnipotent()) { + return; + } + $editor_class = get_class($this); $object_phid = $object->getPHID(); @@ -4865,8 +4880,6 @@ abstract class PhabricatorApplicationTransactionEditor $editor_class); } - $actor = $this->getActor(); - $request = $this->getRequest(); if ($request === null) { $source_type = $this->getContentSource()->getSourceTypeConstant(); @@ -4909,20 +4922,47 @@ abstract class PhabricatorApplicationTransactionEditor PhabricatorLiskDAO $object, array $xactions) { - $is_mfa = ($object instanceof PhabricatorEditEngineMFAInterface); - if (!$is_mfa) { + $has_engine = ($object instanceof PhabricatorEditEngineMFAInterface); + if ($has_engine) { + $engine = PhabricatorEditEngineMFAEngine::newEngineForObject($object) + ->setViewer($this->getActor()); + $require_mfa = $engine->shouldRequireMFA(); + $try_mfa = $engine->shouldTryMFA(); + } else { + $require_mfa = false; + $try_mfa = false; + } + + // If the user is mentioning an MFA object on another object or creating + // a relationship like "parent" or "child" to this object, we always + // allow the edit to move forward without requiring MFA. + if ($this->getIsInverseEdgeEditor()) { return $xactions; } - $engine = PhabricatorEditEngineMFAEngine::newEngineForObject($object) - ->setViewer($this->getActor()); - $require_mfa = $engine->shouldRequireMFA(); - if (!$require_mfa) { - $try_mfa = $engine->shouldTryMFA(); + // If the object hasn't already opted into MFA, see if any of the + // transactions want it. + if (!$try_mfa) { + foreach ($xactions as $xaction) { + $type = $xaction->getTransactionType(); + + $xtype = $this->getModularTransactionType($type); + if ($xtype) { + $xtype = clone $xtype; + $xtype->setStorage($xaction); + if ($xtype->shouldTryMFA($object, $xaction)) { + $try_mfa = true; + break; + } + } + } + } + if ($try_mfa) { $this->setShouldRequireMFA(true); } + return $xactions; } @@ -4940,13 +4980,6 @@ abstract class PhabricatorApplicationTransactionEditor return $xactions; } - // If the user is mentioning an MFA object on another object or creating - // a relationship like "parent" or "child" to this object, we allow the - // edit to move forward without requiring MFA. - if ($this->getIsInverseEdgeEditor()) { - return $xactions; - } - $template = $object->getApplicationTransactionTemplate(); $mfa_xaction = id(clone $template) @@ -4958,4 +4991,112 @@ abstract class PhabricatorApplicationTransactionEditor return $xactions; } + private function getTitleForTextMail( + PhabricatorApplicationTransaction $xaction) { + $type = $xaction->getTransactionType(); + + $xtype = $this->getModularTransactionType($type); + if ($xtype) { + $xtype = clone $xtype; + $xtype->setStorage($xaction); + $comment = $xtype->getTitleForTextMail(); + if ($comment !== false) { + return $comment; + } + } + + return $xaction->getTitleForTextMail(); + } + + private function getTitleForHTMLMail( + PhabricatorApplicationTransaction $xaction) { + $type = $xaction->getTransactionType(); + + $xtype = $this->getModularTransactionType($type); + if ($xtype) { + $xtype = clone $xtype; + $xtype->setStorage($xaction); + $comment = $xtype->getTitleForHTMLMail(); + if ($comment !== false) { + return $comment; + } + } + + return $xaction->getTitleForHTMLMail(); + } + + + private function getBodyForTextMail( + PhabricatorApplicationTransaction $xaction) { + $type = $xaction->getTransactionType(); + + $xtype = $this->getModularTransactionType($type); + if ($xtype) { + $xtype = clone $xtype; + $xtype->setStorage($xaction); + $comment = $xtype->getBodyForTextMail(); + if ($comment !== false) { + return $comment; + } + } + + return $xaction->getBodyForMail(); + } + + +/* -( Extensions )--------------------------------------------------------- */ + + + private function validateTransactionsWithExtensions( + PhabricatorLiskDAO $object, + array $xactions) { + $errors = array(); + + $extensions = $this->getEditorExtensions(); + foreach ($extensions as $extension) { + $extension_errors = $extension + ->setObject($object) + ->validateTransactions($object, $xactions); + + assert_instances_of( + $extension_errors, + 'PhabricatorApplicationTransactionValidationError'); + + $errors[] = $extension_errors; + } + + return array_mergev($errors); + } + + private function getEditorExtensions() { + if ($this->extensions === null) { + $this->extensions = $this->newEditorExtensions(); + } + return $this->extensions; + } + + private function newEditorExtensions() { + $extensions = PhabricatorEditorExtension::getAllExtensions(); + + $actor = $this->getActor(); + $object = $this->object; + foreach ($extensions as $key => $extension) { + + $extension = id(clone $extension) + ->setViewer($actor) + ->setEditor($this) + ->setObject($object); + + if (!$extension->supportsObject($this, $object)) { + unset($extensions[$key]); + continue; + } + + $extensions[$key] = $extension; + } + + return $extensions; + } + + } diff --git a/src/applications/transactions/engineextension/PhabricatorEditorExtension.php b/src/applications/transactions/engineextension/PhabricatorEditorExtension.php new file mode 100644 index 0000000000..6ffac522c2 --- /dev/null +++ b/src/applications/transactions/engineextension/PhabricatorEditorExtension.php @@ -0,0 +1,83 @@ +getPhobjectClassConstant('EXTENSIONKEY'); + } + + final public function setEditor( + PhabricatorApplicationTransactionEditor $editor) { + $this->editor = $editor; + return $this; + } + + final public function getEditor() { + return $this->editor; + } + + final public function setViewer(PhabricatorUser $viewer) { + $this->viewer = $viewer; + return $this; + } + + final public function getViewer() { + return $this->viewer; + } + + final public function setObject( + PhabricatorApplicationTransactionInterface $object) { + $this->object = $object; + return $this; + } + + final public static function getAllExtensions() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getExtensionKey') + ->execute(); + } + + abstract public function getExtensionName(); + + public function supportsObject( + PhabricatorApplicationTransactionEditor $editor, + PhabricatorApplicationTransactionInterface $object) { + return true; + } + + public function validateTransactions($object, array $xactions) { + return array(); + } + + final protected function newTransactionError( + PhabricatorApplicationTransaction $xaction, + $title, + $message) { + return new PhabricatorApplicationTransactionValidationError( + $xaction->getTransactionType(), + $title, + $message, + $xaction); + } + + final protected function newRequiredTransasctionError( + PhabricatorApplicationTransaction $xaction, + $message) { + return $this->newError($xaction, pht('Required'), $message) + ->setIsMissingFieldError(true); + } + + final protected function newInvalidTransactionError( + PhabricatorApplicationTransaction $xaction, + $message) { + return $this->newTransactionError($xaction, pht('Invalid'), $message); + } + + +} diff --git a/src/applications/transactions/engineextension/PhabricatorEditorExtensionModule.php b/src/applications/transactions/engineextension/PhabricatorEditorExtensionModule.php new file mode 100644 index 0000000000..e34a0bb3a7 --- /dev/null +++ b/src/applications/transactions/engineextension/PhabricatorEditorExtensionModule.php @@ -0,0 +1,40 @@ +getViewer(); + + $extensions = PhabricatorEditorExtension::getAllExtensions(); + + $rows = array(); + foreach ($extensions as $extension) { + $rows[] = array( + get_class($extension), + $extension->getExtensionName(), + ); + } + + return id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Class'), + pht('Name'), + )) + ->setColumnClasses( + array( + null, + 'wide pri', + )); + } + +} diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index efbbdb4a09..6d047fc823 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -740,8 +740,9 @@ abstract class PhabricatorApplicationTransaction switch ($this->getTransactionType()) { case PhabricatorTransactions::TYPE_TOKEN: + case PhabricatorTransactions::TYPE_MFA: return true; - case PhabricatorTransactions::TYPE_EDGE: + case PhabricatorTransactions::TYPE_EDGE: $edge_type = $this->getMetadataValue('edge:type'); switch ($edge_type) { case PhabricatorObjectMentionsObjectEdgeType::EDGECONST: diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php index 3d2efe0501..2d0cb8e7c1 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php +++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php @@ -425,4 +425,74 @@ abstract class PhabricatorModularTransactionType return PhabricatorPolicyCapability::CAN_EDIT; } + public function shouldTryMFA( + $object, + PhabricatorApplicationTransaction $xaction) { + return false; + } + + // NOTE: See T12921. These APIs are somewhat aspirational. For now, all of + // these use "TARGET_TEXT" (even the HTML methods!) and the body methods + // actually return Remarkup, not text or HTML. + + final public function getTitleForTextMail() { + return $this->getTitleForMailWithRenderingTarget( + PhabricatorApplicationTransaction::TARGET_TEXT); + } + + final public function getTitleForHTMLMail() { + return $this->getTitleForMailWithRenderingTarget( + PhabricatorApplicationTransaction::TARGET_TEXT); + } + + final public function getBodyForTextMail() { + return $this->getBodyForMailWithRenderingTarget( + PhabricatorApplicationTransaction::TARGET_TEXT); + } + + final public function getBodyForHTMLMail() { + return $this->getBodyForMailWithRenderingTarget( + PhabricatorApplicationTransaction::TARGET_TEXT); + } + + private function getTitleForMailWithRenderingTarget($target) { + $storage = $this->getStorage(); + + $old_target = $storage->getRenderingTarget(); + try { + $storage->setRenderingTarget($target); + $result = $this->getTitleForMail(); + } catch (Exception $ex) { + $storage->setRenderingTarget($old_target); + throw $ex; + } + $storage->setRenderingTarget($old_target); + + return $result; + } + + private function getBodyForMailWithRenderingTarget($target) { + $storage = $this->getStorage(); + + $old_target = $storage->getRenderingTarget(); + try { + $storage->setRenderingTarget($target); + $result = $this->getBodyForMail(); + } catch (Exception $ex) { + $storage->setRenderingTarget($old_target); + throw $ex; + } + $storage->setRenderingTarget($old_target); + + return $result; + } + + protected function getTitleForMail() { + return false; + } + + protected function getBodyForMail() { + return false; + } + } diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php index d830309119..f6a27d4bcd 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php @@ -1,9 +1,7 @@ editEngineLock; } + public function setRequiresMFA($requires_mfa) { + $this->requiresMFA = $requires_mfa; + return $this; + } + + public function getRequiresMFA() { + return $this->requiresMFA; + } + public function setTransactionTimeline( PhabricatorApplicationTransactionView $timeline) { @@ -187,8 +195,8 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { )); } - $user = $this->getUser(); - if (!$user->isLoggedIn()) { + $viewer = $this->getViewer(); + if (!$viewer->isLoggedIn()) { $uri = id(new PhutilURI('/login/')) ->setQueryParam('next', (string)$this->getRequestURI()); return id(new PHUIObjectBoxView()) @@ -203,6 +211,25 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { pht('Log In to Comment'))); } + if ($this->getRequiresMFA()) { + if (!$viewer->getIsEnrolledInMultiFactor()) { + $viewer->updateMultiFactorEnrollment(); + if (!$viewer->getIsEnrolledInMultiFactor()) { + $messages = array(); + $messages[] = pht( + 'You must provide multi-factor credentials to comment or make '. + 'changes, but you do not have multi-factor authentication '. + 'configured on your account.'); + $messages[] = pht( + 'To continue, configure multi-factor authentication in Settings.'); + + return id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_MFA) + ->setErrors($messages); + } + } + } + $data = array(); $comment = $this->renderCommentPanel(); @@ -226,7 +253,7 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { } require_celerity_resource('phui-comment-form-css'); - $image_uri = $user->getProfileImageURI(); + $image_uri = $viewer->getProfileImageURI(); $image = phutil_tag( 'div', array( @@ -388,6 +415,17 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { $form->appendChild($info_view); } + if ($this->getRequiresMFA()) { + $message = pht( + 'You will be required to provide multi-factor credentials to '. + 'comment or make changes.'); + + $form->appendChild( + id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_MFA) + ->setErrors(array($message))); + } + $form->appendChild($invisi_bar); $form->addClass('phui-comment-has-actions'); diff --git a/src/docs/user/configuration/configuring_outbound_email.diviner b/src/docs/user/configuration/configuring_outbound_email.diviner index b8b3835e18..4d18ba0eb2 100644 --- a/src/docs/user/configuration/configuring_outbound_email.diviner +++ b/src/docs/user/configuration/configuring_outbound_email.diviner @@ -1,33 +1,45 @@ @title Configuring Outbound Email @group config -Instructions for configuring Phabricator to send mail. +Instructions for configuring Phabricator to send email and other types of +messages, like text messages. Overview ======== -Phabricator can send outbound email through several different mail services, +Phabricator sends outbound messages through "mailers". Most mailers send +email and most messages are email messages, but mailers may also send other +types of messages (like text messages). + +Phabricator can send outbound messages through multiple different mailers, including a local mailer or various third-party services. Options include: -| Send Mail With | Setup | Cost | Inbound | Notes | -|---------|-------|------|---------|-------| -| Postmark | Easy | Cheap | Yes | Recommended | -| Mailgun | Easy | Cheap | Yes | Recommended | -| Amazon SES | Easy | Cheap | No | Recommended | -| SendGrid | Medium | Cheap | Yes | Discouraged | -| External SMTP | Medium | Varies | No | Gmail, etc. | -| Local SMTP | Hard | Free | No | sendmail, postfix, etc | -| Custom | Hard | Free | No | Write a custom mailer for some other service. | -| Drop in a Hole | Easy | Free | No | Drops mail in a deep, dark hole. | +| Send Mail With | Setup | Cost | Inbound | Media | Notes | +|----------------|-------|------|---------|-------|-------| +| Postmark | Easy | Cheap | Yes | Email | Recommended | +| Mailgun | Easy | Cheap | Yes | Email | Recommended | +| Amazon SES | Easy | Cheap | No | Email | | +| SendGrid | Medium | Cheap | Yes | Email | | +| Twilio | Easy | Cheap | No | SMS | Recommended | +| Amazon SNS | Easy | Cheap | No | SMS | Recommended | +| External SMTP | Medium | Varies | No | Email | Gmail, etc. | +| Local SMTP | Hard | Free | No | Email | sendmail, postfix, etc | +| Custom | Hard | Free | No | All | Write a custom mailer. | +| Drop in a Hole | Easy | Free | No | All | Drops mail in a deep, dark hole. | See below for details on how to select and configure mail delivery for each mailer. -Overall, Postmark and Mailgun are much easier to set up, and using one of them -is recommended. Both will also let you set up inbound email easily. +For email, Postmark or Mailgun are recommended because they make it easy to +set up inbound and outbound mail and have good track records in our production +services. Other services will also generally work well, but they may be more +difficult to set up. -If you have some internal mail service you'd like to use you can also write a -custom mailer, but this requires digging into the code. +For SMS, Twilio or SNS are recommended. They're also your only upstream +options. + +If you have some internal mail or messaging service you'd like to use you can +also write a custom mailer, but this requires digging into the code. Phabricator sends mail in the background, so the daemons need to be running for it to be able to deliver mail. You should receive setup warnings if they are @@ -91,13 +103,14 @@ The supported keys for each mailer are: types. Normally, you do not need to configure this. See below for a list of media types. -The `type` field can be used to select these third-party mailers: +The `type` field can be used to select these mailer services: - `mailgun`: Use Mailgun. - `ses`: Use Amazon SES. - `sendgrid`: Use SendGrid. - `postmark`: Use Postmark. - - `sns`: Use Amazon SNS (only for sending SMS messages). + - `twilio`: Use Twilio. + - `sns`: Use Amazon SNS. It also supports these local mailers: @@ -153,6 +166,12 @@ For alternatives and more information on configuration, see Mailer: Postmark ================ +| Media | Email +|---------| +| Inbound | Yes +|---------| + + Postmark is a third-party email delivery service. You can learn more at . @@ -183,8 +202,13 @@ documented at: Mailer: Mailgun =============== +| Media | Email +|---------| +| Inbound | Yes +|---------| + Mailgun is a third-party email delivery service. You can learn more at -. Mailgun is easy to configure and works well. +. Mailgun is easy to configure and works well. To use this mailer, set `type` to `mailgun`, then configure these `options`: @@ -195,8 +219,13 @@ To use this mailer, set `type` to `mailgun`, then configure these `options`: Mailer: Amazon SES ================== +| Media | Email +|---------| +| Inbound | No +|---------| + Amazon SES is Amazon's cloud email service. You can learn more at -. +. To use this mailer, set `type` to `ses`, then configure these `options`: @@ -209,21 +238,58 @@ which "From" address to use by setting `metamta.default-address` in your config, then follow the Amazon SES verification process to verify it. You won't be able to send email until you do this! +Mailer: Twilio +================== + +| Media | SMS +|---------| +| Inbound | No +|---------| + +Twilio is a third-party notification service. You can learn more at +. + + +To use this mailer, set `type` to `twilio`, then configure these options: + + - `account-sid`: Your Twilio Account SID. + - `auth-token`: Your Twilio Auth Token. + - `from-number`: Number to send text messages from, in E.164 format + (like `+15551237890`). + Mailer: Amazon SNS ================== +| Media | SMS +|---------| +| Inbound | No +|---------| + + Amazon SNS is Amazon's cloud notification service. You can learn more at -. Note that this mailer is only able to send +. Note that this mailer is only able to send SMS messages, not emails. -To use this mailer, set `type` to `sns`, then configure the options similarly -to the SES configuration above. +To use this mailer, set `type` to `sns`, then configure these options: + + - `access-key`: Required string. Your Amazon SNS access key. + - `secret-key`: Required string. Your Amazon SNS secret key. + - `endpoint`: Required string. Your Amazon SNS endpoint. + - `region`: Required string. Your Amazon SNS region. + +You can find the correct `region` value for your endpoint in the SNS +documentation. Mailer: SendGrid ================ +| Media | Email +|---------| +| Inbound | Yes +|---------| + SendGrid is a third-party email delivery service. You can learn more at -. +. You can configure SendGrid in two ways: you can send via SMTP or via the REST API. To use SMTP, configure Phabricator to use an `smtp` mailer. @@ -240,10 +306,16 @@ including an "API User". Make sure you're configuring your "API Key". Mailer: Sendmail ================ +| Media | Email +|---------| +| Inbound | Requires Configuration +|---------| + + This requires a `sendmail` binary to be installed on the system. Most MTAs -(e.g., sendmail, qmail, postfix) should do this, but your machine may not have -one installed by default. For install instructions, consult the documentation -for your favorite MTA. +(e.g., sendmail, qmail, postfix) should install one for you, but your machine +may not have one installed by default. For install instructions, consult the +documentation for your favorite MTA. Since you'll be sending the mail yourself, you are subject to things like SPF rules, blackholes, and MTA configuration which are beyond the scope of this @@ -258,6 +330,11 @@ configure. Mailer: SMTP ============ +| Media | Email +|---------| +| Inbound | Requires Configuration +|---------| + You can use this adapter to send mail via an external SMTP server, like Gmail. To use this mailer, set `type` to `smtp`, then configure these `options`: @@ -273,7 +350,15 @@ To use this mailer, set `type` to `smtp`, then configure these `options`: Disable Mail ============ -To disable mail, just don't configure any mailers. +| Media | All +|---------| +| Inbound | No +|---------| + + +To disable mail, just don't configure any mailers. (You can safely ignore the +setup warning reminding you to set up mailers if you don't plan to configure +any.) Testing and Debugging Outbound Email @@ -288,6 +373,9 @@ particular: Run `bin/mail help ` for more help on using these commands. +By default, `bin/mail send-test` sends email messages, but you can use +the `--type` flag to send different types of messages. + You can monitor daemons using the Daemon Console (`/daemon/`, or click **Daemon Console** from the homepage). diff --git a/src/docs/user/userguide/multi_factor_auth.diviner b/src/docs/user/userguide/multi_factor_auth.diviner index 44811e16f2..eca85d0f92 100644 --- a/src/docs/user/userguide/multi_factor_auth.diviner +++ b/src/docs/user/userguide/multi_factor_auth.diviner @@ -9,40 +9,39 @@ Overview Multi-factor authentication allows you to add additional credentials to your account to make it more secure. -This sounds complicated, but in most cases it just means that Phabricator will -make sure you have your mobile phone (by sending you a text message or having -you enter a code from a mobile application) before allowing you to log in or -take certain "high security" actions (like changing your password). +Once multi-factor authentication is configured on your account, you'll usually +use your mobile phone to provide an authorization code or an extra confirmation +when you try to log in to a new session or take certain actions (like changing +your password). Requiring you to prove you're really you by asking for something you know (your password) //and// something you have (your mobile phone) makes it much harder for attackers to access your account. The phone is an additional "factor" which protects your account from attacks. -Requiring re-authentication before performing high security actions further -limits the damage an attacker can do even if they manage to compromise a -login session. - How Multi-Factor Authentication Works ===================================== If you've configured multi-factor authentication and try to log in to your -account or take certain high security actions (like changing your password), +account or take certain sensitive actions (like changing your password), you'll be stopped and asked to enter additional credentials. -Usually, this means you'll receive an SMS with a security code on your phone, or -you'll open an app on your phone which will show you a security code. -In both cases, you'll enter the security code into Phabricator. +Usually, this means you'll receive an SMS with a authorization code on your +phone, or you'll open an app on your phone which will show you a authorization +code or ask you to confirm the action. If you're given a authorization code, +you'll enter it into Phabricator. If you're logging in, Phabricator will log you in after you enter the code. -If you're taking a high security action, Phabricator will put your account in -"high security" mode for a few minutes. In this mode, you can take high security -actions like changing passwords or SSH keys freely without entering any more -credentials. You can explicitly leave high security once you're done performing -account management, or your account will naturally return to normal security -after a short period of time. +If you're taking a sensitive action, Phabricator will sometimes put your +account in "high security" mode for a few minutes. In this mode, you can take +sensitive actions like changing passwords or SSH keys freely, without +entering any more credentials. + +You can explicitly leave high security once you're done performing account +management, or your account will naturally return to normal security after a +short period of time. While your account is in high security, you'll see a notification on screen with instructions for returning to normal security. @@ -52,8 +51,8 @@ Configuring Multi-Factor Authentication ======================================= To manage authentication factors for your account, go to -Settings > Multi-Factor Auth. You can use this control panel to add or remove -authentication factors from your account. +{nav Settings > Multi-Factor Auth}. You can use this control panel to add +or remove authentication factors from your account. You can also rename a factor by clicking the name. This can help you identify factors if you have several similar factors attached to your account. @@ -65,7 +64,7 @@ Factor: Mobile Phone App (TOTP) =============================== TOTP stands for "Time-based One-Time Password". This factor operates by having -you enter security codes from your mobile phone into Phabricator. The codes +you enter authorization codes from your mobile phone into Phabricator. The codes change every 30 seconds, so you will need to have your phone with you in order to enter them. @@ -79,23 +78,91 @@ application, so check any in-house documentation for details. In general, any TOTP application should work properly. After you've downloaded the application onto your phone, use the Phabricator -settings panel to add a factor to your account. You'll be prompted to enter a -master key into your phone, and then read a security code from your phone and -type it into Phabricator. +settings panel to add a factor to your account. You'll be prompted to scan a +QR code, and then read an authorization code from your phone and type it into +Phabricator. Later, when you need to authenticate, you'll follow this same process: launch -the application, read the security code, and type it into Phabricator. This will -prove you have your phone. +the application, read the authorization code, and type it into Phabricator. +This will prove you have your phone. Don't lose your phone! You'll need it to log into Phabricator in the future. -Recovering from Lost Factors -============================ +Factor: SMS +=========== -If you've lost a factor associated with your account (for example, your phone -has been lost or damaged), an administrator can strip the factor off your -account so that you can log in without it. +This factor operates by texting you a short authorization code when you try to +log in or perform a sensitive action. + +To use SMS, first add your phone number in {nav Settings > Contact Numbers}. +Once a primary contact number is configured on your account, you'll be able +to add an SMS factor. + +To enroll in SMS, you'll be sent a confirmation code to make sure your contact +number is correct and SMS is being delivered properly. Enter it when prompted. + +When you're asked to confirm your identity in the future, you'll be texted +an authorization code to enter into the prompt. + +(WARNING) SMS is a very weak factor and can be compromised or intercepted. For +details, see: . + + +Factor: Duo +=========== + +This factor supports integration with [[ https://duo.com/ | Duo Security ]], a +third-party authentication service popular with enterprises that have a lot of +policies to enforce. + +To use Duo, you'll install the Duo application on your phone. When you try +to take a sensitive action, you'll be asked to confirm it in the application. + + +Administration: Configuration +============================= + +New Phabricator installs start without any multi-factor providers enabled. +Users won't be able to add new factors until you set up multi-factor +authentication by configuring at least one provider. + +Configure new providers in {nav Auth > Multi-Factor}. + +Providers may be in these states: + + - **Active**: Users may add new factors. Users will be prompted to respond + to challenges from these providers when they take a sensitive action. + - **Deprecated**: Users may not add new factors, but they will still be + asked to respond to challenges from exising factors. + - **Disabled**: Users may not add new factors, and existing factors will + not be used. If MFA is required and a user only has disabled factors, + they will be forced to add a new factor. + +If you want to change factor types for your organization, the process will +normally look something like this: + + - Configure and test a new provider. + - Deprecate the old provider. + - Notify users that the old provider is deprecated and that they should move + to the new provider at their convenience, but before some upcoming + deadline. + - Once the deadline arrives, disable the old provider. + + +Administration: Requiring MFA +============================= + +As an administrator, you can require all users to add MFA to their accounts by +setting the `security.require-multi-factor-auth` option in Config. + + +Administration: Recovering from Lost Factors +============================================ + +If a user has lost a factor associated with their account (for example, their +phone has been lost or damaged), an administrator with host access can strip +the factor off their account so that they can log in without it. IMPORTANT: Before stripping factors from a user account, be absolutely certain that the user is who they claim to be! @@ -113,9 +180,10 @@ advance and require them to perform it. But no matter what you do, be certain the user (not an attacker //pretending// to be the user) is really the one making the request before stripping factors. -After verifying identity, administrators can strip authentication factors from -user accounts using the `bin/auth strip` command. For example, to strip all -factors from the account of a user who has lost their phone, run this command: +After verifying identity, administrators with host access can strip +authentication factors from user accounts using the `bin/auth strip` command. +For example, to strip all factors from the account of a user who has lost +their phone, run this command: ```lang=console # Strip all factors from a given user account. @@ -125,7 +193,7 @@ phabricator/ $ ./bin/auth strip --user --all-types You can run `bin/auth help strip` for more detail and all available flags and arguments. -This command can selectively strip types of factors. You can use +This command can selectively strip factors by factor type. You can use `bin/auth list-factors` to get a list of available factor types. ```lang=console @@ -133,8 +201,9 @@ This command can selectively strip types of factors. You can use phabricator/ $ ./bin/auth list-factors ``` -Once you've identified the factor types you want to strip, you can strip them -using the `--type` flag to specify one or more factor types: +Once you've identified the factor types you want to strip, you can strip +matching factors by using the `--type` flag to specify one or more factor +types: ```lang=console # Strip all SMS and TOTP factors for a user. diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php index 931840e8fb..9f7a69909a 100644 --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -2855,6 +2855,13 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery } } + // See T13240. If this query raises policy exceptions, don't filter objects + // in the MySQL layer. We want them to reach the application layer so we + // can reject them and raise an exception. + if ($this->shouldRaisePolicyExceptions()) { + return null; + } + $space_phids = array(); $include_null = false;