diff --git a/resources/celerity/map.php b/resources/celerity/map.php index cb44dd5585..2d402dbaa3 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => '7a73ffc5', + 'core.pkg.css' => '85a1da99', 'core.pkg.js' => '5c737607', 'differential.pkg.css' => 'b8df73d4', 'differential.pkg.js' => '67c9ea4c', @@ -30,7 +30,7 @@ return array( 'rsrc/css/aphront/notification.css' => '30240bd2', 'rsrc/css/aphront/panel-view.css' => '46923d46', 'rsrc/css/aphront/phabricator-nav-view.css' => 'f8a0c1bf', - 'rsrc/css/aphront/table-view.css' => 'daa1f9df', + 'rsrc/css/aphront/table-view.css' => '205053cd', 'rsrc/css/aphront/tokenizer.css' => 'b52d0668', 'rsrc/css/aphront/tooltip.css' => 'e3f2412f', 'rsrc/css/aphront/typeahead-browse.css' => 'b7ed02d2', @@ -46,7 +46,7 @@ return array( 'rsrc/css/application/config/config-options.css' => '16c920ae', 'rsrc/css/application/config/config-template.css' => '20babf50', 'rsrc/css/application/config/setup-issue.css' => '5eed85b2', - 'rsrc/css/application/config/unhandled-exception.css' => '9da8fdab', + 'rsrc/css/application/config/unhandled-exception.css' => '9ecfc00d', 'rsrc/css/application/conpherence/color.css' => 'b17746b0', 'rsrc/css/application/conpherence/durable-column.css' => '2d57072b', 'rsrc/css/application/conpherence/header-pane.css' => 'c9a3db8e', @@ -151,7 +151,7 @@ return array( 'rsrc/css/phui/phui-document.css' => '52b748a5', 'rsrc/css/phui/phui-feed-story.css' => 'a0c05029', 'rsrc/css/phui/phui-fontkit.css' => '9b714a5e', - 'rsrc/css/phui/phui-form-view.css' => '0807e7ac', + 'rsrc/css/phui/phui-form-view.css' => '01b796c0', 'rsrc/css/phui/phui-form.css' => '159e2d9c', 'rsrc/css/phui/phui-head-thing.css' => 'd7f293df', 'rsrc/css/phui/phui-header-view.css' => '93cea4ec', @@ -164,7 +164,7 @@ return array( 'rsrc/css/phui/phui-left-right.css' => '68513c34', 'rsrc/css/phui/phui-lightbox.css' => '4ebf22da', 'rsrc/css/phui/phui-list.css' => '470b1adb', - 'rsrc/css/phui/phui-object-box.css' => '9b58483d', + 'rsrc/css/phui/phui-object-box.css' => 'f434b6be', 'rsrc/css/phui/phui-pager.css' => 'd022c7ad', 'rsrc/css/phui/phui-pinboard-view.css' => '1f08f5d8', 'rsrc/css/phui/phui-property-list-view.css' => 'cad62236', @@ -502,6 +502,7 @@ return array( 'rsrc/js/phui/behavior-phui-selectable-list.js' => 'b26a41e4', 'rsrc/js/phui/behavior-phui-submenu.js' => 'b5e9bff9', 'rsrc/js/phui/behavior-phui-tab-group.js' => '242aa08b', + 'rsrc/js/phui/behavior-phui-timer-control.js' => 'f84bcbf4', 'rsrc/js/phuix/PHUIXActionListView.js' => 'c68f183f', 'rsrc/js/phuix/PHUIXActionView.js' => 'aaa08f3b', 'rsrc/js/phuix/PHUIXAutocomplete.js' => '58cc4ab8', @@ -519,7 +520,7 @@ return array( 'aphront-list-filter-view-css' => 'feb64255', 'aphront-multi-column-view-css' => 'fbc00ba3', 'aphront-panel-view-css' => '46923d46', - 'aphront-table-view-css' => 'daa1f9df', + 'aphront-table-view-css' => '205053cd', 'aphront-tokenizer-control-css' => 'b52d0668', 'aphront-tooltip-css' => 'e3f2412f', 'aphront-typeahead-control-css' => '8779483d', @@ -650,6 +651,7 @@ return array( 'javelin-behavior-phui-selectable-list' => 'b26a41e4', 'javelin-behavior-phui-submenu' => 'b5e9bff9', 'javelin-behavior-phui-tab-group' => '242aa08b', + 'javelin-behavior-phui-timer-control' => 'f84bcbf4', 'javelin-behavior-phuix-example' => 'c2c500a7', 'javelin-behavior-policy-control' => '0eaa33a9', 'javelin-behavior-policy-rule-editor' => '9347f172', @@ -817,7 +819,7 @@ return array( 'phui-font-icon-base-css' => 'd7994e06', 'phui-fontkit-css' => '9b714a5e', 'phui-form-css' => '159e2d9c', - 'phui-form-view-css' => '0807e7ac', + 'phui-form-view-css' => '01b796c0', 'phui-head-thing-view-css' => 'd7f293df', 'phui-header-view-css' => '93cea4ec', 'phui-hovercard' => '074f0783', @@ -831,7 +833,7 @@ return array( 'phui-left-right-css' => '68513c34', 'phui-lightbox-css' => '4ebf22da', 'phui-list-view-css' => '470b1adb', - 'phui-object-box-css' => '9b58483d', + 'phui-object-box-css' => 'f434b6be', 'phui-oi-big-ui-css' => '9e037c7a', 'phui-oi-color-css' => 'b517bfa0', 'phui-oi-drag-ui-css' => 'da15d3dc', @@ -877,7 +879,7 @@ return array( 'syntax-highlighting-css' => '8a16f91b', 'tokens-css' => 'ce5a50bd', 'typeahead-browse-css' => 'b7ed02d2', - 'unhandled-exception-css' => '9da8fdab', + 'unhandled-exception-css' => '9ecfc00d', ), 'requires' => array( '01384686' => array( @@ -2111,6 +2113,11 @@ return array( 'javelin-stratcom', 'javelin-dom', ), + 'f84bcbf4' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-dom', + ), 'f8c4e135' => array( 'javelin-install', 'javelin-dom', diff --git a/resources/sql/autopatches/20190206.external.03.providerphid.sql b/resources/sql/autopatches/20190206.external.03.providerphid.sql new file mode 100644 index 0000000000..0b2f498e02 --- /dev/null +++ b/resources/sql/autopatches/20190206.external.03.providerphid.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_user.user_externalaccount + ADD providerConfigPHID VARBINARY(64) NOT NULL; diff --git a/resources/sql/autopatches/20190206.external.04.providerlink.php b/resources/sql/autopatches/20190206.external.04.providerlink.php new file mode 100644 index 0000000000..e4a2e2d4bf --- /dev/null +++ b/resources/sql/autopatches/20190206.external.04.providerlink.php @@ -0,0 +1,36 @@ +establishConnection('w'); +$table_name = $account_table->getTableName(); + +$config_table = new PhabricatorAuthProviderConfig(); +$config_conn = $config_table->establishConnection('w'); + +foreach (new LiskRawMigrationIterator($account_conn, $table_name) as $row) { + if (strlen($row['providerConfigPHID'])) { + continue; + } + + $config_row = queryfx_one( + $config_conn, + 'SELECT phid + FROM %R + WHERE providerType = %s AND providerDomain = %s + LIMIT 1', + $config_table, + $row['accountType'], + $row['accountDomain']); + if (!$config_row) { + continue; + } + + queryfx( + $account_conn, + 'UPDATE %R + SET providerConfigPHID = %s + WHERE id = %d', + $account_table, + $config_row['phid'], + $row['id']); +} diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index b704a7cc2e..3c9a329d99 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1714,6 +1714,7 @@ phutil_register_library_map(array( 'ManiphestTaskFerretEngine' => 'applications/maniphest/search/ManiphestTaskFerretEngine.php', 'ManiphestTaskFulltextEngine' => 'applications/maniphest/search/ManiphestTaskFulltextEngine.php', 'ManiphestTaskGraph' => 'infrastructure/graph/ManiphestTaskGraph.php', + 'ManiphestTaskGraphController' => 'applications/maniphest/controller/ManiphestTaskGraphController.php', 'ManiphestTaskHasCommitEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasCommitEdgeType.php', 'ManiphestTaskHasCommitRelationship' => 'applications/maniphest/relationship/ManiphestTaskHasCommitRelationship.php', 'ManiphestTaskHasDuplicateTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasDuplicateTaskEdgeType.php', @@ -2195,6 +2196,8 @@ phutil_register_library_map(array( 'PhabricatorAuthChallengeGarbageCollector' => 'applications/auth/garbagecollector/PhabricatorAuthChallengeGarbageCollector.php', 'PhabricatorAuthChallengePHIDType' => 'applications/auth/phid/PhabricatorAuthChallengePHIDType.php', 'PhabricatorAuthChallengeQuery' => 'applications/auth/query/PhabricatorAuthChallengeQuery.php', + 'PhabricatorAuthChallengeStatusController' => 'applications/auth/controller/mfa/PhabricatorAuthChallengeStatusController.php', + 'PhabricatorAuthChallengeUpdate' => 'applications/auth/view/PhabricatorAuthChallengeUpdate.php', 'PhabricatorAuthChangePasswordAction' => 'applications/auth/action/PhabricatorAuthChangePasswordAction.php', 'PhabricatorAuthConduitAPIMethod' => 'applications/auth/conduit/PhabricatorAuthConduitAPIMethod.php', 'PhabricatorAuthConduitTokenRevoker' => 'applications/auth/revoker/PhabricatorAuthConduitTokenRevoker.php', @@ -2270,6 +2273,7 @@ phutil_register_library_map(array( 'PhabricatorAuthInviteVerifyException' => 'applications/auth/exception/PhabricatorAuthInviteVerifyException.php', 'PhabricatorAuthInviteWorker' => 'applications/auth/worker/PhabricatorAuthInviteWorker.php', 'PhabricatorAuthLinkController' => 'applications/auth/controller/PhabricatorAuthLinkController.php', + 'PhabricatorAuthLinkMessageType' => 'applications/auth/message/PhabricatorAuthLinkMessageType.php', 'PhabricatorAuthListController' => 'applications/auth/controller/config/PhabricatorAuthListController.php', 'PhabricatorAuthLoginController' => 'applications/auth/controller/PhabricatorAuthLoginController.php', 'PhabricatorAuthLoginMessageType' => 'applications/auth/message/PhabricatorAuthLoginMessageType.php', @@ -2368,6 +2372,7 @@ phutil_register_library_map(array( 'PhabricatorAuthSessionPHIDType' => 'applications/auth/phid/PhabricatorAuthSessionPHIDType.php', 'PhabricatorAuthSessionQuery' => 'applications/auth/query/PhabricatorAuthSessionQuery.php', 'PhabricatorAuthSessionRevoker' => 'applications/auth/revoker/PhabricatorAuthSessionRevoker.php', + 'PhabricatorAuthSetExternalController' => 'applications/auth/controller/PhabricatorAuthSetExternalController.php', 'PhabricatorAuthSetPasswordController' => 'applications/auth/controller/PhabricatorAuthSetPasswordController.php', 'PhabricatorAuthSetupCheck' => 'applications/config/check/PhabricatorAuthSetupCheck.php', 'PhabricatorAuthStartController' => 'applications/auth/controller/PhabricatorAuthStartController.php', @@ -3867,7 +3872,6 @@ phutil_register_library_map(array( 'PhabricatorPeopleInviteController' => 'applications/people/controller/PhabricatorPeopleInviteController.php', 'PhabricatorPeopleInviteListController' => 'applications/people/controller/PhabricatorPeopleInviteListController.php', 'PhabricatorPeopleInviteSendController' => 'applications/people/controller/PhabricatorPeopleInviteSendController.php', - 'PhabricatorPeopleLdapController' => 'applications/people/controller/PhabricatorPeopleLdapController.php', 'PhabricatorPeopleListController' => 'applications/people/controller/PhabricatorPeopleListController.php', 'PhabricatorPeopleLogQuery' => 'applications/people/query/PhabricatorPeopleLogQuery.php', 'PhabricatorPeopleLogSearchEngine' => 'applications/people/query/PhabricatorPeopleLogSearchEngine.php', @@ -4987,6 +4991,7 @@ phutil_register_library_map(array( 'PhortuneAccountViewController' => 'applications/phortune/controller/account/PhortuneAccountViewController.php', 'PhortuneAdHocCart' => 'applications/phortune/cart/PhortuneAdHocCart.php', 'PhortuneAdHocProduct' => 'applications/phortune/product/PhortuneAdHocProduct.php', + 'PhortuneAddPaymentMethodAction' => 'applications/phortune/action/PhortuneAddPaymentMethodAction.php', 'PhortuneCart' => 'applications/phortune/storage/PhortuneCart.php', 'PhortuneCartAcceptController' => 'applications/phortune/controller/cart/PhortuneCartAcceptController.php', 'PhortuneCartCancelController' => 'applications/phortune/controller/cart/PhortuneCartCancelController.php', @@ -7397,6 +7402,7 @@ phutil_register_library_map(array( 'ManiphestTaskFerretEngine' => 'PhabricatorFerretEngine', 'ManiphestTaskFulltextEngine' => 'PhabricatorFulltextEngine', 'ManiphestTaskGraph' => 'PhabricatorObjectGraph', + 'ManiphestTaskGraphController' => 'ManiphestController', 'ManiphestTaskHasCommitEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskHasCommitRelationship' => 'ManiphestTaskRelationship', 'ManiphestTaskHasDuplicateTaskEdgeType' => 'PhabricatorEdgeType', @@ -7925,6 +7931,8 @@ phutil_register_library_map(array( 'PhabricatorAuthChallengeGarbageCollector' => 'PhabricatorGarbageCollector', 'PhabricatorAuthChallengePHIDType' => 'PhabricatorPHIDType', 'PhabricatorAuthChallengeQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorAuthChallengeStatusController' => 'PhabricatorAuthController', + 'PhabricatorAuthChallengeUpdate' => 'Phobject', 'PhabricatorAuthChangePasswordAction' => 'PhabricatorSystemAction', 'PhabricatorAuthConduitAPIMethod' => 'ConduitAPIMethod', 'PhabricatorAuthConduitTokenRevoker' => 'PhabricatorAuthRevoker', @@ -8019,6 +8027,7 @@ phutil_register_library_map(array( 'PhabricatorAuthInviteVerifyException' => 'PhabricatorAuthInviteDialogException', 'PhabricatorAuthInviteWorker' => 'PhabricatorWorker', 'PhabricatorAuthLinkController' => 'PhabricatorAuthController', + 'PhabricatorAuthLinkMessageType' => 'PhabricatorAuthMessageType', 'PhabricatorAuthListController' => 'PhabricatorAuthProviderConfigController', 'PhabricatorAuthLoginController' => 'PhabricatorAuthController', 'PhabricatorAuthLoginMessageType' => 'PhabricatorAuthMessageType', @@ -8138,6 +8147,7 @@ phutil_register_library_map(array( 'PhabricatorAuthSessionPHIDType' => 'PhabricatorPHIDType', 'PhabricatorAuthSessionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorAuthSessionRevoker' => 'PhabricatorAuthRevoker', + 'PhabricatorAuthSetExternalController' => 'PhabricatorAuthController', 'PhabricatorAuthSetPasswordController' => 'PhabricatorAuthController', 'PhabricatorAuthSetupCheck' => 'PhabricatorSetupCheck', 'PhabricatorAuthStartController' => 'PhabricatorAuthController', @@ -9866,7 +9876,6 @@ phutil_register_library_map(array( 'PhabricatorPeopleInviteController' => 'PhabricatorPeopleController', 'PhabricatorPeopleInviteListController' => 'PhabricatorPeopleInviteController', 'PhabricatorPeopleInviteSendController' => 'PhabricatorPeopleInviteController', - 'PhabricatorPeopleLdapController' => 'PhabricatorPeopleController', 'PhabricatorPeopleListController' => 'PhabricatorPeopleController', 'PhabricatorPeopleLogQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorPeopleLogSearchEngine' => 'PhabricatorApplicationSearchEngine', @@ -11229,6 +11238,7 @@ phutil_register_library_map(array( 'PhortuneAccountViewController' => 'PhortuneAccountProfileController', 'PhortuneAdHocCart' => 'PhortuneCartImplementation', 'PhortuneAdHocProduct' => 'PhortuneProductImplementation', + 'PhortuneAddPaymentMethodAction' => 'PhabricatorSystemAction', 'PhortuneCart' => array( 'PhortuneDAO', 'PhabricatorApplicationTransactionInterface', diff --git a/src/aphront/AphrontRequest.php b/src/aphront/AphrontRequest.php index a65566e867..46d1266b08 100644 --- a/src/aphront/AphrontRequest.php +++ b/src/aphront/AphrontRequest.php @@ -594,7 +594,7 @@ final class AphrontRequest extends Phobject { $request_uri = idx($_SERVER, 'REQUEST_URI', '/'); $uri = new PhutilURI($request_uri); - $uri->setQueryParam('__path__', null); + $uri->removeQueryParam('__path__'); $path = phutil_escape_uri($this->getPath()); $uri->setPath($path); @@ -829,7 +829,10 @@ final class AphrontRequest extends Phobject { } $uri->setPath($this->getPath()); - $uri->setQueryParams(self::flattenData($_GET)); + $uri->removeAllQueryParams(); + foreach (self::flattenData($_GET) as $query_key => $query_value) { + $uri->appendQueryParam($query_key, $query_value); + } $input = PhabricatorStartup::getRawInput(); diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php index 8d36bbc880..8cd27fa62b 100644 --- a/src/aphront/configuration/AphrontApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontApplicationConfiguration.php @@ -118,6 +118,12 @@ final class AphrontApplicationConfiguration $database_exception = $ex; } + // If we're in developer mode, set a flag so that top-level exception + // handlers can add more information. + if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { + $sink->setShowStackTraces(true); + } + if ($database_exception) { $issue = PhabricatorSetupIssue::newDatabaseConnectionIssue( $database_exception, @@ -282,23 +288,69 @@ final class AphrontApplicationConfiguration } } catch (Exception $ex) { $original_exception = $ex; - $response = $this->handleThrowable($ex); } catch (Throwable $ex) { $original_exception = $ex; - $response = $this->handleThrowable($ex); } + $response_exception = null; try { + if ($original_exception) { + $response = $this->handleThrowable($original_exception); + } + $response = $this->produceResponse($request, $response); $response = $controller->willSendResponse($response); $response->setRequest($request); self::writeResponse($sink, $response); } catch (Exception $ex) { + $response_exception = $ex; + } catch (Throwable $ex) { + $response_exception = $ex; + } + + if ($response_exception) { + // If we encountered an exception while building a normal response, then + // encountered another exception while building a response for the first + // exception, just throw the original exception. It is more likely to be + // useful and point at a root cause than the second exception we ran into + // while telling the user about it. if ($original_exception) { throw $original_exception; } - throw $ex; + + // If we built a response successfully and then ran into an exception + // trying to render it, try to handle and present that exception to the + // user using the standard handler. + + // The problem here might be in rendering (more common) or in the actual + // response mechanism (less common). If it's in rendering, we can likely + // still render a nice exception page: the majority of rendering issues + // are in main page content, not content shared with the exception page. + + $handling_exception = null; + try { + $response = $this->handleThrowable($response_exception); + + $response = $this->produceResponse($request, $response); + $response = $controller->willSendResponse($response); + $response->setRequest($request); + + self::writeResponse($sink, $response); + } catch (Exception $ex) { + $handling_exception = $ex; + } catch (Throwable $ex) { + $handling_exception = $ex; + } + + // If we didn't have any luck with that, raise the original response + // exception. As above, this is the root cause exception and more likely + // to be useful. This will go to the fallback error handler at top + // level. + + if ($handling_exception) { + throw $response_exception; + } } return $response; diff --git a/src/aphront/response/AphrontAjaxResponse.php b/src/aphront/response/AphrontAjaxResponse.php index 1ccb3fe97e..2187defc8f 100644 --- a/src/aphront/response/AphrontAjaxResponse.php +++ b/src/aphront/response/AphrontAjaxResponse.php @@ -32,22 +32,21 @@ final class AphrontAjaxResponse extends AphrontResponse { } public function buildResponseString() { + $request = $this->getRequest(); $console = $this->getConsole(); if ($console) { // NOTE: We're stripping query parameters here both for readability and // to mitigate BREACH and similar attacks. The parameters are available // in the "Request" tab, so this should not impact usability. See T3684. - $uri = $this->getRequest()->getRequestURI(); - $uri = new PhutilURI($uri); - $uri->setQueryParams(array()); + $path = $request->getPath(); Javelin::initBehavior( 'dark-console', array( - 'uri' => (string)$uri, - 'key' => $console->getKey($this->getRequest()), + 'uri' => $path, + 'key' => $console->getKey($request), 'color' => $console->getColor(), - 'quicksand' => $this->getRequest()->isQuicksand(), + 'quicksand' => $request->isQuicksand(), )); } @@ -60,7 +59,6 @@ final class AphrontAjaxResponse extends AphrontResponse { $response = CelerityAPI::getStaticResourceResponse(); - $request = $this->getRequest(); if ($request) { $viewer = $request->getViewer(); if ($viewer) { diff --git a/src/aphront/response/AphrontResponse.php b/src/aphront/response/AphrontResponse.php index 0eab91b2af..9cfda0f5c1 100644 --- a/src/aphront/response/AphrontResponse.php +++ b/src/aphront/response/AphrontResponse.php @@ -218,7 +218,7 @@ abstract class AphrontResponse extends Phobject { $uri = id(new PhutilURI($uri)) ->setPath(null) ->setFragment(null) - ->setQueryParams(array()); + ->removeAllQueryParams(); $uri = (string)$uri; if (preg_match('/[ ;\']/', $uri)) { diff --git a/src/aphront/response/AphrontUnhandledExceptionResponse.php b/src/aphront/response/AphrontUnhandledExceptionResponse.php index efd9d70ead..32d612ca50 100644 --- a/src/aphront/response/AphrontUnhandledExceptionResponse.php +++ b/src/aphront/response/AphrontUnhandledExceptionResponse.php @@ -4,8 +4,20 @@ final class AphrontUnhandledExceptionResponse extends AphrontStandaloneHTMLResponse { private $exception; + private $showStackTraces; + + public function setShowStackTraces($show_stack_traces) { + $this->showStackTraces = $show_stack_traces; + return $this; + } + + public function getShowStackTraces() { + return $this->showStackTraces; + } + + public function setException($exception) { + // NOTE: We accept an Exception or a Throwable. - public function setException(Exception $exception) { // Log the exception unless it's specifically a silent malformed request // exception. @@ -61,10 +73,36 @@ final class AphrontUnhandledExceptionResponse $body = $ex->getMessage(); $body = phutil_escape_html_newlines($body); + $classes = array(); + $classes[] = 'unhandled-exception-detail'; + + $stack = null; + if ($this->getShowStackTraces()) { + try { + $stack = id(new AphrontStackTraceView()) + ->setTrace($ex->getTrace()); + + $stack = hsprintf('%s', $stack); + + $stack = phutil_tag( + 'div', + array( + 'class' => 'unhandled-exception-stack', + ), + $stack); + + $classes[] = 'unhandled-exception-with-stack'; + } catch (Exception $trace_exception) { + $stack = null; + } catch (Throwable $trace_exception) { + $stack = null; + } + } + return phutil_tag( 'div', array( - 'class' => 'unhandled-exception-detail', + 'class' => implode(' ', $classes), ), array( phutil_tag( @@ -79,6 +117,7 @@ final class AphrontUnhandledExceptionResponse 'class' => 'unhandled-exception-body', ), $body), + $stack, )); } diff --git a/src/aphront/sink/AphrontHTTPSink.php b/src/aphront/sink/AphrontHTTPSink.php index 7c9cc3fc0f..9e43e4a687 100644 --- a/src/aphront/sink/AphrontHTTPSink.php +++ b/src/aphront/sink/AphrontHTTPSink.php @@ -5,14 +5,22 @@ * Normally this is just @{class:AphrontPHPHTTPSink}, which uses "echo" and * "header()" to emit responses. * - * Mostly, this class allows us to do install security or metrics hooks in the - * output pipeline. - * * @task write Writing Response Components * @task emit Emitting the Response */ abstract class AphrontHTTPSink extends Phobject { + private $showStackTraces = false; + + final public function setShowStackTraces($show_stack_traces) { + $this->showStackTraces = $show_stack_traces; + return $this; + } + + final public function getShowStackTraces() { + return $this->showStackTraces; + } + /* -( Writing Response Components )---------------------------------------- */ diff --git a/src/applications/almanac/controller/AlmanacController.php b/src/applications/almanac/controller/AlmanacController.php index 24a8986766..918b3a4ad9 100644 --- a/src/applications/almanac/controller/AlmanacController.php +++ b/src/applications/almanac/controller/AlmanacController.php @@ -67,19 +67,13 @@ abstract class AlmanacController $is_builtin = isset($builtins[$key]); $is_persistent = (bool)$property->getID(); - $delete_uri = id(new PhutilURI($delete_base)) - ->setQueryParams( - array( - 'key' => $key, - 'objectPHID' => $object->getPHID(), - )); + $params = array( + 'key' => $key, + 'objectPHID' => $object->getPHID(), + ); - $edit_uri = id(new PhutilURI($edit_base)) - ->setQueryParams( - array( - 'key' => $key, - 'objectPHID' => $object->getPHID(), - )); + $delete_uri = new PhutilURI($delete_base, $params); + $edit_uri = new PhutilURI($edit_base, $params); $delete = javelin_tag( 'a', @@ -143,7 +137,7 @@ abstract class AlmanacController $phid = $object->getPHID(); $add_uri = id(new PhutilURI($edit_base)) - ->setQueryParam('objectPHID', $object->getPHID()); + ->replaceQueryParam('objectPHID', $object->getPHID()); $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, diff --git a/src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php b/src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php index 23583422fb..7d14df7239 100644 --- a/src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php +++ b/src/applications/audit/management/PhabricatorAuditManagementDeleteWorkflow.php @@ -105,10 +105,10 @@ final class PhabricatorAuditManagementDeleteWorkflow $query->withPHIDs(mpull($commits, 'getPHID')); } - $commits = $query->execute(); - $commits = mpull($commits, null, 'getPHID'); + $commit_iterator = new PhabricatorQueryIterator($query); + $audits = array(); - foreach ($commits as $commit) { + foreach ($commit_iterator as $commit) { $commit_audits = $commit->getAudits(); foreach ($commit_audits as $key => $audit) { if ($id_map && empty($id_map[$audit->getID()])) { @@ -131,51 +131,87 @@ final class PhabricatorAuditManagementDeleteWorkflow continue; } } - $audits[] = $commit_audits; - } - $audits = array_mergev($audits); - $console = PhutilConsole::getConsole(); + if (!$commit_audits) { + continue; + } - if (!$audits) { - $console->writeErr("%s\n", pht('No audits match the query.')); - return 0; - } + $handles = id(new PhabricatorHandleQuery()) + ->setViewer($viewer) + ->withPHIDs(mpull($commit_audits, 'getAuditorPHID')) + ->execute(); - $handles = id(new PhabricatorHandleQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs(mpull($audits, 'getAuditorPHID')) - ->execute(); + foreach ($commit_audits as $audit) { + $audit_id = $audit->getID(); - - foreach ($audits as $audit) { - $commit = $commits[$audit->getCommitPHID()]; - - $console->writeOut( - "%s\n", - sprintf( + $description = sprintf( '%10d %-16s %-16s %s: %s', - $audit->getID(), + $audit_id, $handles[$audit->getAuditorPHID()]->getName(), PhabricatorAuditStatusConstants::getStatusName( $audit->getAuditStatus()), $commit->getRepository()->formatCommitName( $commit->getCommitIdentifier()), - trim($commit->getSummary()))); + trim($commit->getSummary())); + + $audits[] = array( + 'auditID' => $audit_id, + 'commitPHID' => $commit->getPHID(), + 'description' => $description, + ); + } } - if (!$is_dry_run) { - $message = pht( - 'Really delete these %d audit(s)? They will be permanently deleted '. - 'and can not be recovered.', - count($audits)); - if ($console->confirm($message)) { - foreach ($audits as $audit) { - $id = $audit->getID(); - $console->writeOut("%s\n", pht('Deleting audit %d...', $id)); - $audit->delete(); - } + if (!$audits) { + echo tsprintf( + "%s\n", + pht('No audits match the query.')); + return 0; + } + + foreach ($audits as $audit_spec) { + echo tsprintf( + "%s\n", + $audit_spec['description']); + } + + if ($is_dry_run) { + echo tsprintf( + "%s\n", + pht('This is a dry run, so no changes will be made.')); + return 0; + } + + $message = pht( + 'Really delete these %s audit(s)? They will be permanently deleted '. + 'and can not be recovered.', + phutil_count($audits)); + if (!phutil_console_confirm($message)) { + echo tsprintf( + "%s\n", + pht('User aborted the workflow.')); + return 1; + } + + $audits_by_commit = igroup($audits, 'commitPHID'); + foreach ($audits_by_commit as $commit_phid => $audit_specs) { + $audit_ids = ipull($audit_specs, 'auditID'); + + $audits = id(new PhabricatorRepositoryAuditRequest())->loadAllWhere( + 'id IN (%Ld)', + $audit_ids); + + foreach ($audits as $audit) { + $id = $audit->getID(); + + echo tsprintf( + "%s\n", + pht('Deleting audit %d...', $id)); + + $audit->delete(); } + + $this->synchronizeCommitAuditState($commit_phid); } return 0; diff --git a/src/applications/audit/management/PhabricatorAuditManagementWorkflow.php b/src/applications/audit/management/PhabricatorAuditManagementWorkflow.php index 6112a38e1d..b9d90bddc8 100644 --- a/src/applications/audit/management/PhabricatorAuditManagementWorkflow.php +++ b/src/applications/audit/management/PhabricatorAuditManagementWorkflow.php @@ -87,4 +87,39 @@ abstract class PhabricatorAuditManagementWorkflow return $commits; } + protected function synchronizeCommitAuditState($commit_phid) { + $viewer = $this->getViewer(); + + $commit = id(new DiffusionCommitQuery()) + ->setViewer($viewer) + ->withPHIDs(array($commit_phid)) + ->needAuditRequests(true) + ->executeOne(); + if (!$commit) { + return; + } + + $old_status = $commit->getAuditStatusObject(); + $commit->updateAuditStatus($commit->getAudits()); + $new_status = $commit->getAuditStatusObject(); + + if ($old_status->getKey() == $new_status->getKey()) { + echo tsprintf( + "%s\n", + pht( + 'No synchronization changes for "%s".', + $commit->getDisplayName())); + } else { + echo tsprintf( + "%s\n", + pht( + 'Synchronizing "%s": "%s" -> "%s".', + $commit->getDisplayName(), + $old_status->getName(), + $new_status->getName())); + + $commit->save(); + } + } + } diff --git a/src/applications/audit/management/PhabricatorAuditSynchronizeManagementWorkflow.php b/src/applications/audit/management/PhabricatorAuditSynchronizeManagementWorkflow.php index 96d06e65c2..abd0a3c637 100644 --- a/src/applications/audit/management/PhabricatorAuditSynchronizeManagementWorkflow.php +++ b/src/applications/audit/management/PhabricatorAuditSynchronizeManagementWorkflow.php @@ -6,8 +6,16 @@ final class PhabricatorAuditSynchronizeManagementWorkflow protected function didConstruct() { $this ->setName('synchronize') - ->setExamples('**synchronize** ...') - ->setSynopsis(pht('Update audit status for commits.')) + ->setExamples( + "**synchronize** __repository__ ...\n". + "**synchronize** __commit__ ...\n". + "**synchronize** --all") + ->setSynopsis( + pht( + 'Update commits to make their summary audit state reflect the '. + 'state of their actual audit requests. This can fix inconsistencies '. + 'in database state if audit requests have been mangled '. + 'accidentally (or on purpose).')) ->setArguments( array_merge( $this->getCommitConstraintArguments(), @@ -21,36 +29,7 @@ final class PhabricatorAuditSynchronizeManagementWorkflow foreach ($objects as $object) { $commits = $this->loadCommitsForConstraintObject($object); foreach ($commits as $commit) { - $commit = id(new DiffusionCommitQuery()) - ->setViewer($viewer) - ->withPHIDs(array($commit->getPHID())) - ->needAuditRequests(true) - ->executeOne(); - if (!$commit) { - continue; - } - - $old_status = $commit->getAuditStatusObject(); - $commit->updateAuditStatus($commit->getAudits()); - $new_status = $commit->getAuditStatusObject(); - - if ($old_status->getKey() == $new_status->getKey()) { - echo tsprintf( - "%s\n", - pht( - 'No changes for "%s".', - $commit->getDisplayName())); - } else { - echo tsprintf( - "%s\n", - pht( - 'Updating "%s": "%s" -> "%s".', - $commit->getDisplayName(), - $old_status->getName(), - $new_status->getName())); - - $commit->save(); - } + $this->synchronizeCommitAuditState($commit->getPHID()); } } } diff --git a/src/applications/auth/application/PhabricatorAuthApplication.php b/src/applications/auth/application/PhabricatorAuthApplication.php index 307cab61c8..3446ad597d 100644 --- a/src/applications/auth/application/PhabricatorAuthApplication.php +++ b/src/applications/auth/application/PhabricatorAuthApplication.php @@ -61,8 +61,8 @@ final class PhabricatorAuthApplication extends PhabricatorApplication { 'start/' => 'PhabricatorAuthStartController', 'validate/' => 'PhabricatorAuthValidateController', 'finish/' => 'PhabricatorAuthFinishController', - 'unlink/(?P[^/]+)/' => 'PhabricatorAuthUnlinkController', - '(?Plink|refresh)/(?P[^/]+)/' + 'unlink/(?P\d+)/' => 'PhabricatorAuthUnlinkController', + '(?Plink|refresh)/(?P\d+)/' => 'PhabricatorAuthLinkController', 'confirmlink/(?P[^/]+)/' => 'PhabricatorAuthConfirmLinkController', @@ -86,7 +86,9 @@ final class PhabricatorAuthApplication extends PhabricatorApplication { => 'PhabricatorAuthSSHKeyRevokeController', 'view/(?P\d+)/' => 'PhabricatorAuthSSHKeyViewController', ), + 'password/' => 'PhabricatorAuthSetPasswordController', + 'external/' => 'PhabricatorAuthSetExternalController', 'mfa/' => array( $this->getQueryRoutePattern() => @@ -97,6 +99,8 @@ final class PhabricatorAuthApplication extends PhabricatorApplication { 'PhabricatorAuthFactorProviderViewController', 'message/(?P[1-9]\d*)/' => 'PhabricatorAuthFactorProviderMessageController', + 'challenge/status/(?P[1-9]\d*)/' => + 'PhabricatorAuthChallengeStatusController', ), 'message/' => array( diff --git a/src/applications/auth/controller/PhabricatorAuthConfirmLinkController.php b/src/applications/auth/controller/PhabricatorAuthConfirmLinkController.php index 664a97885f..9ceb10df8b 100644 --- a/src/applications/auth/controller/PhabricatorAuthConfirmLinkController.php +++ b/src/applications/auth/controller/PhabricatorAuthConfirmLinkController.php @@ -20,7 +20,15 @@ final class PhabricatorAuthConfirmLinkController $panel_uri = '/settings/panel/external/'; - if ($request->isFormPost()) { + if ($request->isFormOrHisecPost()) { + $workflow_key = sprintf( + 'account.link(%s)', + $account->getPHID()); + + $hisec_token = id(new PhabricatorAuthSessionEngine()) + ->setWorkflowKey($workflow_key) + ->requireHighSecurityToken($viewer, $request, $panel_uri); + $account->setUserPHID($viewer->getPHID()); $account->save(); @@ -31,14 +39,7 @@ final class PhabricatorAuthConfirmLinkController return id(new AphrontRedirectResponse())->setURI($panel_uri); } - // TODO: Provide more information about the external account. Clicking - // through this form blindly is dangerous. - - // TODO: If the user has password authentication, require them to retype - // their password here. - - $dialog = id(new AphrontDialogView()) - ->setUser($viewer) + $dialog = $this->newDialog() ->setTitle(pht('Confirm %s Account Link', $provider->getProviderName())) ->addCancelButton($panel_uri) ->addSubmitButton(pht('Confirm Account Link')); diff --git a/src/applications/auth/controller/PhabricatorAuthController.php b/src/applications/auth/controller/PhabricatorAuthController.php index 9b7267ec96..cda56d34b1 100644 --- a/src/applications/auth/controller/PhabricatorAuthController.php +++ b/src/applications/auth/controller/PhabricatorAuthController.php @@ -95,7 +95,7 @@ abstract class PhabricatorAuthController extends PhabricatorController { private function buildLoginValidateResponse(PhabricatorUser $user) { $validate_uri = new PhutilURI($this->getApplicationURI('validate/')); - $validate_uri->setQueryParam('expect', $user->getUsername()); + $validate_uri->replaceQueryParam('expect', $user->getUsername()); return id(new AphrontRedirectResponse())->setURI((string)$validate_uri); } @@ -213,19 +213,19 @@ abstract class PhabricatorAuthController extends PhabricatorController { return array($account, $provider, $response); } - $provider = PhabricatorAuthProvider::getEnabledProviderByKey( - $account->getProviderKey()); - - if (!$provider) { + $config = $account->getProviderConfig(); + if (!$config->getIsEnabled()) { $response = $this->renderError( pht( - 'The account you are attempting to register with uses a nonexistent '. - 'or disabled authentication provider (with key "%s"). An '. - 'administrator may have recently disabled this provider.', - $account->getProviderKey())); + 'The account you are attempting to register with uses a disabled '. + 'authentication provider ("%s"). An administrator may have '. + 'recently disabled this provider.', + $config->getDisplayName())); return array($account, $provider, $response); } + $provider = $config->getProvider(); + return array($account, $provider, null); } diff --git a/src/applications/auth/controller/PhabricatorAuthLinkController.php b/src/applications/auth/controller/PhabricatorAuthLinkController.php index 44176a278e..4b127b9ad1 100644 --- a/src/applications/auth/controller/PhabricatorAuthLinkController.php +++ b/src/applications/auth/controller/PhabricatorAuthLinkController.php @@ -6,14 +6,20 @@ final class PhabricatorAuthLinkController public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); $action = $request->getURIData('action'); - $provider_key = $request->getURIData('pkey'); - $provider = PhabricatorAuthProvider::getEnabledProviderByKey( - $provider_key); - if (!$provider) { + $id = $request->getURIData('id'); + + $config = id(new PhabricatorAuthProviderConfigQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->withIsEnabled(true) + ->executeOne(); + if (!$config) { return new Aphront404Response(); } + $provider = $config->getProvider(); + switch ($action) { case 'link': if (!$provider->shouldAllowAccountLink()) { @@ -37,15 +43,15 @@ final class PhabricatorAuthLinkController return new Aphront400Response(); } - $account = id(new PhabricatorExternalAccount())->loadOneWhere( - 'accountType = %s AND accountDomain = %s AND userPHID = %s', - $provider->getProviderType(), - $provider->getProviderDomain(), - $viewer->getPHID()); + $accounts = id(new PhabricatorExternalAccountQuery()) + ->setViewer($viewer) + ->withUserPHIDs(array($viewer->getPHID())) + ->withProviderConfigPHIDs(array($config->getPHID())) + ->execute(); switch ($action) { case 'link': - if ($account) { + if ($accounts) { return $this->renderErrorPage( pht('Account Already Linked'), array( @@ -56,7 +62,7 @@ final class PhabricatorAuthLinkController } break; case 'refresh': - if (!$account) { + if (!$accounts) { return $this->renderErrorPage( pht('No Account Linked'), array( @@ -76,11 +82,6 @@ final class PhabricatorAuthLinkController switch ($action) { case 'link': - id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession( - $viewer, - $request, - $panel_uri); - $form = $provider->buildLinkForm($this); break; case 'refresh': diff --git a/src/applications/auth/controller/PhabricatorAuthLoginController.php b/src/applications/auth/controller/PhabricatorAuthLoginController.php index 54649a6a69..e7dabd9340 100644 --- a/src/applications/auth/controller/PhabricatorAuthLoginController.php +++ b/src/applications/auth/controller/PhabricatorAuthLoginController.php @@ -35,6 +35,7 @@ final class PhabricatorAuthLoginController return $response; } + $invite = $this->loadInvite(); $provider = $this->provider; try { @@ -103,7 +104,7 @@ final class PhabricatorAuthLoginController // The account is not yet attached to a Phabricator user, so this is // either a registration or an account link request. if (!$viewer->isLoggedIn()) { - if ($provider->shouldAllowRegistration()) { + if ($provider->shouldAllowRegistration() || $invite) { return $this->processRegisterUser($account); } else { return $this->renderError( diff --git a/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php b/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php index f51f379d2b..d176a67119 100644 --- a/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php +++ b/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php @@ -14,11 +14,6 @@ final class PhabricatorAuthOneTimeLoginController $key = $request->getURIData('key'); $email_id = $request->getURIData('emailID'); - if ($request->getUser()->isLoggedIn()) { - return $this->renderError( - pht('You are already logged in.')); - } - $target_user = id(new PhabricatorPeopleQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withIDs(array($id)) @@ -27,6 +22,19 @@ final class PhabricatorAuthOneTimeLoginController return new Aphront404Response(); } + // NOTE: We allow you to use a one-time login link for your own current + // login account. This supports the "Set Password" flow. + + $is_logged_in = false; + if ($viewer->isLoggedIn()) { + if ($viewer->getPHID() !== $target_user->getPHID()) { + return $this->renderError( + pht('You are already logged in.')); + } else { + $is_logged_in = true; + } + } + // NOTE: As a convenience to users, these one-time login URIs may also // be associated with an email address which will be verified when the // URI is used. @@ -100,7 +108,7 @@ final class PhabricatorAuthOneTimeLoginController ->addCancelButton('/'); } - if ($request->isFormPost()) { + if ($request->isFormPost() || $is_logged_in) { // If we have an email bound into this URI, verify email so that clicking // the link in the "Welcome" email is good enough, without requiring users // to go through a second round of email verification. @@ -121,6 +129,12 @@ final class PhabricatorAuthOneTimeLoginController $next_uri = $this->getNextStepURI($target_user); + // If the user is already logged in, we're just doing a "password set" + // flow. Skip directly to the next step. + if ($is_logged_in) { + return id(new AphrontRedirectResponse())->setURI($next_uri); + } + PhabricatorCookies::setNextURICookie($request, $next_uri, $force = true); $force_full_session = false; @@ -204,24 +218,52 @@ final class PhabricatorAuthOneTimeLoginController $request->setTemporaryCookie(PhabricatorCookies::COOKIE_HISEC, 'yes'); - return (string)id(new PhutilURI($panel_uri)) - ->setQueryParams( - array( - 'key' => $key, - )); + $params = array( + 'key' => $key, + ); + + return (string)new PhutilURI($panel_uri, $params); } - $providers = id(new PhabricatorAuthProviderConfigQuery()) + // Check if the user already has external accounts linked. If they do, + // it's not obvious why they aren't using them to log in, but assume they + // know what they're doing. We won't send them to the link workflow. + $accounts = id(new PhabricatorExternalAccountQuery()) + ->setViewer($user) + ->withUserPHIDs(array($user->getPHID())) + ->execute(); + + $configs = id(new PhabricatorAuthProviderConfigQuery()) ->setViewer($user) ->withIsEnabled(true) ->execute(); + $linkable = array(); + foreach ($configs as $config) { + if (!$config->getShouldAllowLink()) { + continue; + } + + $provider = $config->getProvider(); + if (!$provider->isLoginFormAButton()) { + continue; + } + + $linkable[] = $provider; + } + + // If there's at least one linkable provider, and the user doesn't already + // have accounts, send the user to the link workflow. + if (!$accounts && $linkable) { + return '/auth/external/'; + } + // If there are no configured providers and the user is an administrator, // send them to Auth to configure a provider. This is probably where they // want to go. You can end up in this state by accidentally losing your // first session during initial setup, or after restoring exported data // from a hosted instance. - if (!$providers && $user->getIsAdmin()) { + if (!$configs && $user->getIsAdmin()) { return '/auth/'; } diff --git a/src/applications/auth/controller/PhabricatorAuthRegisterController.php b/src/applications/auth/controller/PhabricatorAuthRegisterController.php index 9e1aef592c..5a46d0e604 100644 --- a/src/applications/auth/controller/PhabricatorAuthRegisterController.php +++ b/src/applications/auth/controller/PhabricatorAuthRegisterController.php @@ -11,21 +11,25 @@ final class PhabricatorAuthRegisterController $viewer = $this->getViewer(); $account_key = $request->getURIData('akey'); - if ($request->getUser()->isLoggedIn()) { + if ($viewer->isLoggedIn()) { return id(new AphrontRedirectResponse())->setURI('/'); } + $invite = $this->loadInvite(); + $is_setup = false; if (strlen($account_key)) { $result = $this->loadAccountForRegistrationOrLinking($account_key); list($account, $provider, $response) = $result; $is_default = false; } else if ($this->isFirstTimeSetup()) { - list($account, $provider, $response) = $this->loadSetupAccount(); + $account = null; + $provider = null; + $response = null; $is_default = true; $is_setup = true; } else { - list($account, $provider, $response) = $this->loadDefaultAccount(); + list($account, $provider, $response) = $this->loadDefaultAccount($invite); $is_default = true; } @@ -33,24 +37,24 @@ final class PhabricatorAuthRegisterController return $response; } - $invite = $this->loadInvite(); + if (!$is_setup) { + if (!$provider->shouldAllowRegistration()) { + if ($invite) { + // If the user has an invite, we allow them to register with any + // provider, even a login-only provider. + } else { + // TODO: This is a routine error if you click "Login" on an external + // auth source which doesn't allow registration. The error should be + // more tailored. - if (!$provider->shouldAllowRegistration()) { - if ($invite) { - // If the user has an invite, we allow them to register with any - // provider, even a login-only provider. - } else { - // TODO: This is a routine error if you click "Login" on an external - // auth source which doesn't allow registration. The error should be - // more tailored. - - return $this->renderError( - pht( - 'The account you are attempting to register with uses an '. - 'authentication provider ("%s") which does not allow '. - 'registration. An administrator may have recently disabled '. - 'registration with this provider.', - $provider->getProviderName())); + return $this->renderError( + pht( + 'The account you are attempting to register with uses an '. + 'authentication provider ("%s") which does not allow '. + 'registration. An administrator may have recently disabled '. + 'registration with this provider.', + $provider->getProviderName())); + } } } @@ -58,14 +62,19 @@ final class PhabricatorAuthRegisterController $user = new PhabricatorUser(); - $default_username = $account->getUsername(); - $default_realname = $account->getRealName(); + if ($is_setup) { + $default_username = null; + $default_realname = null; + $default_email = null; + } else { + $default_username = $account->getUsername(); + $default_realname = $account->getRealName(); + $default_email = $account->getEmail(); + } $account_type = PhabricatorAuthPassword::PASSWORD_TYPE_ACCOUNT; $content_source = PhabricatorContentSource::newFromRequest($request); - $default_email = $account->getEmail(); - if ($invite) { $default_email = $invite->getEmailAddress(); } @@ -212,7 +221,11 @@ final class PhabricatorAuthRegisterController $can_edit_email = $profile->getCanEditEmail(); $can_edit_realname = $profile->getCanEditRealName(); - $must_set_password = $provider->shouldRequireRegistrationPassword(); + if ($is_setup) { + $must_set_password = false; + } else { + $must_set_password = $provider->shouldRequireRegistrationPassword(); + } $can_edit_anything = $profile->getCanEditAnything() || $must_set_password; $force_verify = $profile->getShouldVerifyEmail(); @@ -334,9 +347,11 @@ final class PhabricatorAuthRegisterController } if (!$errors) { - $image = $this->loadProfilePicture($account); - if ($image) { - $user->setProfileImagePHID($image->getPHID()); + if (!$is_setup) { + $image = $this->loadProfilePicture($account); + if ($image) { + $user->setProfileImagePHID($image->getPHID()); + } } try { @@ -346,17 +361,19 @@ final class PhabricatorAuthRegisterController $verify_email = true; } - if ($value_email === $default_email) { - if ($account->getEmailVerified()) { - $verify_email = true; - } + if (!$is_setup) { + if ($value_email === $default_email) { + if ($account->getEmailVerified()) { + $verify_email = true; + } - if ($provider->shouldTrustEmails()) { - $verify_email = true; - } + if ($provider->shouldTrustEmails()) { + $verify_email = true; + } - if ($invite) { - $verify_email = true; + if ($invite) { + $verify_email = true; + } } } @@ -438,9 +455,11 @@ final class PhabricatorAuthRegisterController $transaction_editor->applyTransactions($user, $xactions); } - $account->setUserPHID($user->getPHID()); - $provider->willRegisterAccount($account); - $account->save(); + if (!$is_setup) { + $account->setUserPHID($user->getPHID()); + $provider->willRegisterAccount($account); + $account->save(); + } $user->saveTransaction(); @@ -501,7 +520,6 @@ final class PhabricatorAuthRegisterController ->setAuthProvider($provider))); } - if ($can_edit_username) { $form->appendChild( id(new AphrontFormTextControl()) @@ -595,7 +613,7 @@ final class PhabricatorAuthRegisterController pht( 'Installation is complete. Register your administrator account '. 'below to log in. You will be able to configure options and add '. - 'other authentication mechanisms (like LDAP or OAuth) later on.')); + 'authentication mechanisms later on.')); } $object_box = id(new PHUIObjectBoxView()) @@ -612,11 +630,12 @@ final class PhabricatorAuthRegisterController $view = id(new PHUITwoColumnView()) ->setHeader($header) - ->setFooter(array( - $welcome_view, - $invite_header, - $object_box, - )); + ->setFooter( + array( + $welcome_view, + $invite_header, + $object_box, + )); return $this->newPage() ->setTitle($title) @@ -624,17 +643,20 @@ final class PhabricatorAuthRegisterController ->appendChild($view); } - private function loadDefaultAccount() { + private function loadDefaultAccount($invite) { $providers = PhabricatorAuthProvider::getAllEnabledProviders(); $account = null; $provider = null; $response = null; foreach ($providers as $key => $candidate_provider) { - if (!$candidate_provider->shouldAllowRegistration()) { - unset($providers[$key]); - continue; + if (!$invite) { + if (!$candidate_provider->shouldAllowRegistration()) { + unset($providers[$key]); + continue; + } } + if (!$candidate_provider->isDefaultRegistrationProvider()) { unset($providers[$key]); } @@ -652,24 +674,11 @@ final class PhabricatorAuthRegisterController } $provider = head($providers); - $account = $provider->getDefaultExternalAccount(); + $account = $provider->newDefaultExternalAccount(); return array($account, $provider, $response); } - private function loadSetupAccount() { - $provider = new PhabricatorPasswordAuthProvider(); - $provider->attachProviderConfig( - id(new PhabricatorAuthProviderConfig()) - ->setShouldAllowRegistration(1) - ->setShouldAllowLogin(1) - ->setIsEnabled(true)); - - $account = $provider->getDefaultExternalAccount(); - $response = null; - return array($account, $provider, $response); - } - private function loadProfilePicture(PhabricatorExternalAccount $account) { $phid = $account->getProfileImagePHID(); if (!$phid) { diff --git a/src/applications/auth/controller/PhabricatorAuthSetExternalController.php b/src/applications/auth/controller/PhabricatorAuthSetExternalController.php new file mode 100644 index 0000000000..51dfcab53f --- /dev/null +++ b/src/applications/auth/controller/PhabricatorAuthSetExternalController.php @@ -0,0 +1,110 @@ +getViewer(); + + $configs = id(new PhabricatorAuthProviderConfigQuery()) + ->setViewer($viewer) + ->withIsEnabled(true) + ->execute(); + + $linkable = array(); + foreach ($configs as $config) { + if (!$config->getShouldAllowLink()) { + continue; + } + + // For now, only buttons get to appear here: for example, we can't + // reasonably embed an entire LDAP form into this UI. + $provider = $config->getProvider(); + if (!$provider->isLoginFormAButton()) { + continue; + } + + $linkable[] = $config; + } + + if (!$linkable) { + return $this->newDialog() + ->setTitle(pht('No Linkable External Providers')) + ->appendParagraph( + pht( + 'Currently, there are no configured external auth providers '. + 'which you can link your account to.')) + ->addCancelButton('/'); + } + + $text = PhabricatorAuthMessage::loadMessageText( + $viewer, + PhabricatorAuthLinkMessageType::MESSAGEKEY); + if (!strlen($text)) { + $text = pht( + 'You can link your Phabricator account to an external account to '. + 'allow you to log in more easily in the future. To continue, choose '. + 'an account to link below. If you prefer not to link your account, '. + 'you can skip this step.'); + } + + $remarkup_view = new PHUIRemarkupView($viewer, $text); + $remarkup_view = phutil_tag( + 'div', + array( + 'class' => 'phui-object-box-instructions', + ), + $remarkup_view); + + PhabricatorCookies::setClientIDCookie($request); + + $view = array(); + foreach ($configs as $config) { + $provider = $config->getProvider(); + + $form = $provider->buildLinkForm($this); + + if ($provider->isLoginFormAButton()) { + require_celerity_resource('auth-css'); + $form = phutil_tag( + 'div', + array( + 'class' => 'phabricator-link-button pl', + ), + $form); + } + + $view[] = $form; + } + + $form = id(new AphrontFormView()) + ->setViewer($viewer) + ->appendControl( + id(new AphrontFormSubmitControl()) + ->addCancelButton('/', pht('Skip This Step'))); + + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Link External Account')); + + $box = id(new PHUIObjectBoxView()) + ->setViewer($viewer) + ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($remarkup_view) + ->appendChild($view) + ->appendChild($form); + + $main_view = id(new PHUITwoColumnView()) + ->setFooter($box); + + $crumbs = $this->buildApplicationCrumbs() + ->addTextCrumb(pht('Link External Account')) + ->setBorder(true); + + return $this->newPage() + ->setTitle(pht('Link External Account')) + ->setCrumbs($crumbs) + ->appendChild($main_view); + } + +} diff --git a/src/applications/auth/controller/PhabricatorAuthStartController.php b/src/applications/auth/controller/PhabricatorAuthStartController.php index 0b823098d7..72cbbea5a8 100644 --- a/src/applications/auth/controller/PhabricatorAuthStartController.php +++ b/src/applications/auth/controller/PhabricatorAuthStartController.php @@ -54,7 +54,7 @@ final class PhabricatorAuthStartController } $redirect_uri = $request->getRequestURI(); - $redirect_uri->setQueryParam('cleared', 1); + $redirect_uri->replaceQueryParam('cleared', 1); return id(new AphrontRedirectResponse())->setURI($redirect_uri); } } @@ -64,7 +64,7 @@ final class PhabricatorAuthStartController // the workflow will continue normally. if ($did_clear) { $redirect_uri = $request->getRequestURI(); - $redirect_uri->setQueryParam('cleared', null); + $redirect_uri->removeQueryParam('cleared'); return id(new AphrontRedirectResponse())->setURI($redirect_uri); } diff --git a/src/applications/auth/controller/PhabricatorAuthUnlinkController.php b/src/applications/auth/controller/PhabricatorAuthUnlinkController.php index 1e3023e5d2..43e7b1b362 100644 --- a/src/applications/auth/controller/PhabricatorAuthUnlinkController.php +++ b/src/applications/auth/controller/PhabricatorAuthUnlinkController.php @@ -3,48 +3,45 @@ final class PhabricatorAuthUnlinkController extends PhabricatorAuthController { - private $providerKey; - public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); - $this->providerKey = $request->getURIData('pkey'); + $id = $request->getURIData('id'); - list($type, $domain) = explode(':', $this->providerKey, 2); - - // Check that this account link actually exists. We don't require the - // provider to exist because we want users to be able to delete links to - // dead accounts if they want. - $account = id(new PhabricatorExternalAccount())->loadOneWhere( - 'accountType = %s AND accountDomain = %s AND userPHID = %s', - $type, - $domain, - $viewer->getPHID()); + $account = id(new PhabricatorExternalAccountQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->executeOne(); if (!$account) { - return $this->renderNoAccountErrorDialog(); + return new Aphront404Response(); } - // Check that the provider (if it exists) allows accounts to be unlinked. - $provider_key = $this->providerKey; - $provider = PhabricatorAuthProvider::getEnabledProviderByKey($provider_key); - if ($provider) { - if (!$provider->shouldAllowAccountUnlink()) { - return $this->renderNotUnlinkableErrorDialog($provider); - } + $done_uri = '/settings/panel/external/'; + + $config = $account->getProviderConfig(); + $provider = $config->getProvider(); + if (!$provider->shouldAllowAccountUnlink()) { + return $this->renderNotUnlinkableErrorDialog($provider, $done_uri); } $confirmations = $request->getStrList('confirmations'); $confirmations = array_fuse($confirmations); - if (!$request->isFormPost() || !isset($confirmations['unlink'])) { - return $this->renderConfirmDialog($confirmations); + if (!$request->isFormOrHisecPost() || !isset($confirmations['unlink'])) { + return $this->renderConfirmDialog($confirmations, $config, $done_uri); } // Check that this account isn't the only account which can be used to // login. We warn you when you remove your only login account. if ($account->isUsableForLogin()) { - $other_accounts = id(new PhabricatorExternalAccount())->loadAllWhere( - 'userPHID = %s', - $viewer->getPHID()); + $other_accounts = id(new PhabricatorExternalAccountQuery()) + ->setViewer($viewer) + ->withUserPHIDs(array($viewer->getPHID())) + ->execute(); $valid_accounts = 0; foreach ($other_accounts as $other_account) { @@ -55,11 +52,21 @@ final class PhabricatorAuthUnlinkController if ($valid_accounts < 2) { if (!isset($confirmations['only'])) { - return $this->renderOnlyUsableAccountConfirmDialog($confirmations); + return $this->renderOnlyUsableAccountConfirmDialog( + $confirmations, + $done_uri); } } } + $workflow_key = sprintf( + 'account.unlink(%s)', + $account->getPHID()); + + $hisec_token = id(new PhabricatorAuthSessionEngine()) + ->setWorkflowKey($workflow_key) + ->requireHighSecurityToken($viewer, $request, $done_uri); + $account->delete(); id(new PhabricatorAuthSessionEngine())->terminateLoginSessions( @@ -67,42 +74,27 @@ final class PhabricatorAuthUnlinkController new PhutilOpaqueEnvelope( $request->getCookie(PhabricatorCookies::COOKIE_SESSION))); - return id(new AphrontRedirectResponse())->setURI($this->getDoneURI()); - } - - private function getDoneURI() { - return '/settings/panel/external/'; - } - - private function renderNoAccountErrorDialog() { - $dialog = id(new AphrontDialogView()) - ->setUser($this->getRequest()->getUser()) - ->setTitle(pht('No Such Account')) - ->appendChild( - pht( - 'You can not unlink this account because it is not linked.')) - ->addCancelButton($this->getDoneURI()); - - return id(new AphrontDialogResponse())->setDialog($dialog); + return id(new AphrontRedirectResponse())->setURI($done_uri); } private function renderNotUnlinkableErrorDialog( - PhabricatorAuthProvider $provider) { + PhabricatorAuthProvider $provider, + $done_uri) { - $dialog = id(new AphrontDialogView()) - ->setUser($this->getRequest()->getUser()) + return $this->newDialog() ->setTitle(pht('Permanent Account Link')) ->appendChild( pht( 'You can not unlink this account because the administrator has '. - 'configured Phabricator to make links to %s accounts permanent.', + 'configured Phabricator to make links to "%s" accounts permanent.', $provider->getProviderName())) - ->addCancelButton($this->getDoneURI()); - - return id(new AphrontDialogResponse())->setDialog($dialog); + ->addCancelButton($done_uri); } - private function renderOnlyUsableAccountConfirmDialog(array $confirmations) { + private function renderOnlyUsableAccountConfirmDialog( + array $confirmations, + $done_uri) { + $confirmations[] = 'only'; return $this->newDialog() @@ -116,28 +108,23 @@ final class PhabricatorAuthUnlinkController pht( 'If you lose access to your account, you can recover access by '. 'sending yourself an email login link from the login screen.')) - ->addCancelButton($this->getDoneURI()) + ->addCancelButton($done_uri) ->addSubmitButton(pht('Unlink External Account')); } - private function renderConfirmDialog(array $confirmations) { + private function renderConfirmDialog( + array $confirmations, + PhabricatorAuthProviderConfig $config, + $done_uri) { + $confirmations[] = 'unlink'; + $provider = $config->getProvider(); - $provider_key = $this->providerKey; - $provider = PhabricatorAuthProvider::getEnabledProviderByKey($provider_key); - - if ($provider) { - $title = pht('Unlink "%s" Account?', $provider->getProviderName()); - $body = pht( - 'You will no longer be able to use your %s account to '. - 'log in to Phabricator.', - $provider->getProviderName()); - } else { - $title = pht('Unlink Account?'); - $body = pht( - 'You will no longer be able to use this account to log in '. - 'to Phabricator.'); - } + $title = pht('Unlink "%s" Account?', $provider->getProviderName()); + $body = pht( + 'You will no longer be able to use your %s account to '. + 'log in to Phabricator.', + $provider->getProviderName()); return $this->newDialog() ->setTitle($title) @@ -148,7 +135,7 @@ final class PhabricatorAuthUnlinkController 'Note: Unlinking an authentication provider will terminate any '. 'other active login sessions.')) ->addSubmitButton(pht('Unlink Account')) - ->addCancelButton($this->getDoneURI()); + ->addCancelButton($done_uri); } } diff --git a/src/applications/auth/controller/PhabricatorEmailLoginController.php b/src/applications/auth/controller/PhabricatorEmailLoginController.php index eef30e6989..76b288f059 100644 --- a/src/applications/auth/controller/PhabricatorEmailLoginController.php +++ b/src/applications/auth/controller/PhabricatorEmailLoginController.php @@ -9,20 +9,38 @@ final class PhabricatorEmailLoginController public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); + $is_logged_in = $viewer->isLoggedIn(); $e_email = true; $e_captcha = true; $errors = array(); - $v_email = $request->getStr('email'); + if ($is_logged_in) { + if (!$this->isPasswordAuthEnabled()) { + return $this->newDialog() + ->setTitle(pht('No Password Auth')) + ->appendParagraph( + pht( + 'Password authentication is not enabled and you are already '. + 'logged in. There is nothing for you here.')) + ->addCancelButton('/', pht('Continue')); + } + + $v_email = $viewer->loadPrimaryEmailAddress(); + } else { + $v_email = $request->getStr('email'); + } + if ($request->isFormPost()) { $e_email = null; $e_captcha = pht('Again'); - $captcha_ok = AphrontFormRecaptchaControl::processCaptcha($request); - if (!$captcha_ok) { - $errors[] = pht('Captcha response is incorrect, try again.'); - $e_captcha = pht('Invalid'); + if (!$is_logged_in) { + $captcha_ok = AphrontFormRecaptchaControl::processCaptcha($request); + if (!$captcha_ok) { + $errors[] = pht('Captcha response is incorrect, try again.'); + $e_captcha = pht('Invalid'); + } } if (!strlen($v_email)) { @@ -76,10 +94,24 @@ final class PhabricatorEmailLoginController } if (!$errors) { - $body = $this->newAccountLoginMailBody($target_user); + $body = $this->newAccountLoginMailBody( + $target_user, + $is_logged_in); + + if ($is_logged_in) { + $subject = pht('[Phabricator] Account Password Link'); + $instructions = pht( + 'An email has been sent containing a link you can use to set '. + 'a password for your account.'); + } else { + $subject = pht('[Phabricator] Account Login Link'); + $instructions = pht( + 'An email has been sent containing a link you can use to log '. + 'in to your account.'); + } $mail = id(new PhabricatorMetaMTAMail()) - ->setSubject(pht('[Phabricator] Account Login Link')) + ->setSubject($subject) ->setForceDelivery(true) ->addRawTos(array($target_email->getAddress())) ->setBody($body) @@ -88,8 +120,7 @@ final class PhabricatorEmailLoginController return $this->newDialog() ->setTitle(pht('Check Your Email')) ->setShortTitle(pht('Email Sent')) - ->appendParagraph( - pht('An email has been sent with a link you can use to log in.')) + ->appendParagraph($instructions) ->addCancelButton('/', pht('Done')); } } @@ -99,33 +130,47 @@ final class PhabricatorEmailLoginController ->setViewer($viewer); if ($this->isPasswordAuthEnabled()) { - $form->appendRemarkupInstructions( - pht( - 'To reset your password, provide your email address. An email '. - 'with a login link will be sent to you.')); + if ($is_logged_in) { + $title = pht('Set Password'); + $form->appendRemarkupInstructions( + pht( + 'A password reset link will be sent to your primary email '. + 'address. Follow the link to set an account password.')); + } else { + $title = pht('Password Reset'); + $form->appendRemarkupInstructions( + pht( + 'To reset your password, provide your email address. An email '. + 'with a login link will be sent to you.')); + } } else { + $title = pht('Email Login'); $form->appendRemarkupInstructions( pht( 'To access your account, provide your email address. An email '. 'with a login link will be sent to you.')); } + if ($is_logged_in) { + $address_control = new AphrontFormStaticControl(); + } else { + $address_control = id(new AphrontFormTextControl()) + ->setName('email') + ->setError($e_email); + } + + $address_control + ->setLabel(pht('Email Address')) + ->setValue($v_email); + $form - ->appendControl( - id(new AphrontFormTextControl()) - ->setLabel(pht('Email Address')) - ->setName('email') - ->setValue($v_email) - ->setError($e_email)) - ->appendControl( + ->appendControl($address_control); + + if (!$is_logged_in) { + $form->appendControl( id(new AphrontFormRecaptchaControl()) ->setLabel(pht('Captcha')) ->setError($e_captcha)); - - if ($this->isPasswordAuthEnabled()) { - $title = pht('Password Reset'); - } else { - $title = pht('Email Login'); } return $this->newDialog() @@ -137,7 +182,10 @@ final class PhabricatorEmailLoginController ->addSubmitButton(pht('Send Email')); } - private function newAccountLoginMailBody(PhabricatorUser $user) { + private function newAccountLoginMailBody( + PhabricatorUser $user, + $is_logged_in) { + $engine = new PhabricatorAuthSessionEngine(); $uri = $engine->getOneTimeLoginURI( $user, @@ -148,7 +196,12 @@ final class PhabricatorEmailLoginController $have_passwords = $this->isPasswordAuthEnabled(); if ($have_passwords) { - if ($is_serious) { + if ($is_logged_in) { + $body = pht( + 'You can use this link to set a password on your account:'. + "\n\n %s\n", + $uri); + } else if ($is_serious) { $body = pht( "You can use this link to reset your Phabricator password:". "\n\n %s\n", diff --git a/src/applications/auth/controller/config/PhabricatorAuthNewController.php b/src/applications/auth/controller/config/PhabricatorAuthNewController.php index c8fd0ad8a5..770c43208d 100644 --- a/src/applications/auth/controller/config/PhabricatorAuthNewController.php +++ b/src/applications/auth/controller/config/PhabricatorAuthNewController.php @@ -32,7 +32,7 @@ final class PhabricatorAuthNewController $provider_class = get_class($provider); $provider_uri = id(new PhutilURI('/config/edit/')) - ->setQueryParam('provider', $provider_class); + ->replaceQueryParam('provider', $provider_class); $provider_uri = $this->getApplicationURI($provider_uri); $already_exists = isset($configured_classes[get_class($provider)]); diff --git a/src/applications/auth/controller/mfa/PhabricatorAuthChallengeStatusController.php b/src/applications/auth/controller/mfa/PhabricatorAuthChallengeStatusController.php new file mode 100644 index 0000000000..884bbaad6d --- /dev/null +++ b/src/applications/auth/controller/mfa/PhabricatorAuthChallengeStatusController.php @@ -0,0 +1,40 @@ +getViewer(); + $id = $request->getURIData('id'); + $now = PhabricatorTime::getNow(); + + $result = new PhabricatorAuthChallengeUpdate(); + + $challenge = id(new PhabricatorAuthChallengeQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->withUserPHIDs(array($viewer->getPHID())) + ->withChallengeTTLBetween($now, null) + ->executeOne(); + if ($challenge) { + $config = id(new PhabricatorAuthFactorConfigQuery()) + ->setViewer($viewer) + ->withPHIDs(array($challenge->getFactorPHID())) + ->executeOne(); + if ($config) { + $provider = $config->getFactorProvider(); + $factor = $provider->getFactor(); + + $result = $factor->newChallengeStatusView( + $config, + $provider, + $viewer, + $challenge); + } + } + + return id(new AphrontAjaxResponse()) + ->setContent($result->newContent()); + } + +} diff --git a/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderEditController.php b/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderEditController.php index a8d87e2ead..a1636396ac 100644 --- a/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderEditController.php +++ b/src/applications/auth/controller/mfa/PhabricatorAuthFactorProviderEditController.php @@ -45,7 +45,7 @@ final class PhabricatorAuthFactorProviderEditController foreach ($factors as $factor_key => $factor) { $factor_uri = id(new PhutilURI('/mfa/edit/')) - ->setQueryParam('providerFactorKey', $factor_key); + ->replaceQueryParam('providerFactorKey', $factor_key); $factor_uri = $this->getApplicationURI($factor_uri); $is_enabled = $factor->canCreateNewProvider(); diff --git a/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php b/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php index d9fb5d013b..d49a447df6 100644 --- a/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php +++ b/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php @@ -42,7 +42,7 @@ final class PhabricatorAuthMainMenuBarExtension $uri = new PhutilURI('/auth/start/'); if ($controller) { $path = $controller->getRequest()->getPath(); - $uri->setQueryParam('next', $path); + $uri->replaceQueryParam('next', $path); } return id(new PHUIButtonView()) diff --git a/src/applications/auth/factor/PhabricatorAuthFactor.php b/src/applications/auth/factor/PhabricatorAuthFactor.php index 912a2c31c9..d7e6e60ecc 100644 --- a/src/applications/auth/factor/PhabricatorAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorAuthFactor.php @@ -80,6 +80,14 @@ abstract class PhabricatorAuthFactor extends Phobject { return array(); } + public function newChallengeStatusView( + PhabricatorAuthFactorConfig $config, + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $viewer, + PhabricatorAuthChallenge $challenge) { + return null; + } + /** * Is this a factor which depends on the user's contact number? * @@ -210,8 +218,6 @@ abstract class PhabricatorAuthFactor extends Phobject { get_class($this))); } - $result->setIssuedChallenges($challenges); - return $result; } @@ -242,8 +248,6 @@ abstract class PhabricatorAuthFactor extends Phobject { get_class($this))); } - $result->setIssuedChallenges($challenges); - return $result; } @@ -339,9 +343,18 @@ abstract class PhabricatorAuthFactor extends Phobject { ->setIcon('fa-commenting', 'green'); } - return id(new PHUIFormTimerControl()) + $control = id(new PHUIFormTimerControl()) ->setIcon($icon) ->appendChild($error); + + $status_challenge = $result->getStatusChallenge(); + if ($status_challenge) { + $id = $status_challenge->getID(); + $uri = "/auth/mfa/challenge/status/{$id}/"; + $control->setUpdateURI($uri); + } + + return $control; } diff --git a/src/applications/auth/factor/PhabricatorAuthFactorResult.php b/src/applications/auth/factor/PhabricatorAuthFactorResult.php index f03c3674da..b5da379545 100644 --- a/src/applications/auth/factor/PhabricatorAuthFactorResult.php +++ b/src/applications/auth/factor/PhabricatorAuthFactorResult.php @@ -11,6 +11,7 @@ final class PhabricatorAuthFactorResult private $value; private $issuedChallenges = array(); private $icon; + private $statusChallenge; public function setAnsweredChallenge(PhabricatorAuthChallenge $challenge) { if (!$challenge->getIsAnsweredChallenge()) { @@ -34,6 +35,15 @@ final class PhabricatorAuthFactorResult return $this->answeredChallenge; } + public function setStatusChallenge(PhabricatorAuthChallenge $challenge) { + $this->statusChallenge = $challenge; + return $this; + } + + public function getStatusChallenge() { + return $this->statusChallenge; + } + public function getIsValid() { return (bool)$this->getAnsweredChallenge(); } @@ -83,16 +93,6 @@ final class PhabricatorAuthFactorResult return $this->value; } - public function setIssuedChallenges(array $issued_challenges) { - assert_instances_of($issued_challenges, 'PhabricatorAuthChallenge'); - $this->issuedChallenges = $issued_challenges; - return $this; - } - - public function getIssuedChallenges() { - return $this->issuedChallenges; - } - public function setIcon(PHUIIconView $icon) { $this->icon = $icon; return $this; diff --git a/src/applications/auth/factor/PhabricatorDuoAuthFactor.php b/src/applications/auth/factor/PhabricatorDuoAuthFactor.php index 4be4c15ea8..66bd7c9ebd 100644 --- a/src/applications/auth/factor/PhabricatorDuoAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorDuoAuthFactor.php @@ -585,7 +585,7 @@ final class PhabricatorDuoAuthFactor $result = $this->newDuoFuture($provider) ->setHTTPMethod('GET') ->setMethod('auth_status', $parameters) - ->setTimeout(5) + ->setTimeout(3) ->resolve(); $state = $result['response']['result']; @@ -661,15 +661,6 @@ final class PhabricatorDuoAuthFactor 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')) @@ -689,7 +680,27 @@ final class PhabricatorDuoAuthFactor PhabricatorUser $viewer, AphrontRequest $request, array $challenges) { - return $this->newResult(); + + $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.')); + + $challenge = $this->getChallengeForCurrentContext( + $config, + $viewer, + $challenges); + if ($challenge) { + $result + ->setStatusChallenge($challenge) + ->setIcon( + id(new PHUIIconView()) + ->setIcon('fa-refresh', 'green ph-spin')); + } + + return $result; } private function newDuoFuture(PhabricatorAuthFactorProvider $provider) { @@ -790,4 +801,54 @@ final class PhabricatorDuoAuthFactor $hostname)); } + public function newChallengeStatusView( + PhabricatorAuthFactorConfig $config, + PhabricatorAuthFactorProvider $provider, + PhabricatorUser $viewer, + PhabricatorAuthChallenge $challenge) { + + $duo_xaction = $challenge->getChallengeKey(); + + $parameters = array( + 'txid' => $duo_xaction, + ); + + $default_result = id(new PhabricatorAuthChallengeUpdate()) + ->setRetry(true); + + try { + $result = $this->newDuoFuture($provider) + ->setHTTPMethod('GET') + ->setMethod('auth_status', $parameters) + ->setTimeout(5) + ->resolve(); + + $state = $result['response']['result']; + } catch (HTTPFutureCURLResponseStatus $exception) { + // If we failed or timed out, retry. Usually, this is a timeout. + return id(new PhabricatorAuthChallengeUpdate()) + ->setRetry(true); + } + + // For now, don't update the view for anything but an "Allow". Updates + // here are just about providing more visual feedback for user convenience. + if ($state !== 'allow') { + return id(new PhabricatorAuthChallengeUpdate()) + ->setRetry(false); + } + + $icon = id(new PHUIIconView()) + ->setIcon('fa-check-circle-o', 'green'); + + $view = id(new PHUIFormTimerControl()) + ->setIcon($icon) + ->appendChild(pht('You responded to this challenge correctly.')) + ->newTimerView(); + + return id(new PhabricatorAuthChallengeUpdate()) + ->setState('allow') + ->setRetry(false) + ->setMarkup($view); + } + } diff --git a/src/applications/auth/future/PhabricatorDuoFuture.php b/src/applications/auth/future/PhabricatorDuoFuture.php index 81a5a2a2b8..1e70ec2a57 100644 --- a/src/applications/auth/future/PhabricatorDuoFuture.php +++ b/src/applications/auth/future/PhabricatorDuoFuture.php @@ -80,11 +80,6 @@ final class PhabricatorDuoFuture $host = $this->apiHostname; $host = phutil_utf8_strtolower($host); - $uri = id(new PhutilURI('')) - ->setProtocol('https') - ->setDomain($host) - ->setPath($path); - $data = $this->parameters; $date = date('r'); @@ -109,11 +104,19 @@ final class PhabricatorDuoFuture $signature = new PhutilOpaqueEnvelope($signature); if ($http_method === 'GET') { - $uri->setQueryParams($data); - $data = array(); + $uri_data = $data; + $body_data = array(); + } else { + $uri_data = array(); + $body_data = $data; } - $future = id(new HTTPSFuture($uri, $data)) + $uri = id(new PhutilURI('', $uri_data)) + ->setProtocol('https') + ->setDomain($host) + ->setPath($path); + + $future = id(new HTTPSFuture($uri, $body_data)) ->setHTTPBasicAuthCredentials($this->integrationKey, $signature) ->setMethod($http_method) ->addHeader('Accept', 'application/json') diff --git a/src/applications/auth/message/PhabricatorAuthLinkMessageType.php b/src/applications/auth/message/PhabricatorAuthLinkMessageType.php new file mode 100644 index 0000000000..17991231c1 --- /dev/null +++ b/src/applications/auth/message/PhabricatorAuthLinkMessageType.php @@ -0,0 +1,18 @@ +renderLoginForm($controller->getRequest(), $mode = 'link'); } @@ -220,9 +220,7 @@ abstract class PhabricatorAuthProvider extends Phobject { $adapter->getAdapterDomain(), $account_id); if (!$account) { - $account = id(new PhabricatorExternalAccount()) - ->setAccountType($adapter->getAdapterType()) - ->setAccountDomain($adapter->getAdapterDomain()) + $account = $this->newExternalAccount() ->setAccountID($account_id); } @@ -299,8 +297,18 @@ abstract class PhabricatorAuthProvider extends Phobject { return false; } - public function getDefaultExternalAccount() { - throw new PhutilMethodNotImplementedException(); + public function newDefaultExternalAccount() { + return $this->newExternalAccount(); + } + + protected function newExternalAccount() { + $config = $this->getProviderConfig(); + $adapter = $this->getAdapter(); + + return id(new PhabricatorExternalAccount()) + ->setAccountType($adapter->getAdapterType()) + ->setAccountDomain($adapter->getAdapterDomain()) + ->setProviderConfigPHID($config->getPHID()); } public function getLoginOrder() { @@ -438,12 +446,13 @@ abstract class PhabricatorAuthProvider extends Phobject { $uri = $attributes['uri']; $uri = new PhutilURI($uri); - $params = $uri->getQueryParams(); - $uri->setQueryParams(array()); + $params = $uri->getQueryParamsAsPairList(); + $uri->removeAllQueryParams(); $content = array($button); - foreach ($params as $key => $value) { + foreach ($params as $pair) { + list($key, $value) = $pair; $content[] = phutil_tag( 'input', array( diff --git a/src/applications/auth/provider/PhabricatorPasswordAuthProvider.php b/src/applications/auth/provider/PhabricatorPasswordAuthProvider.php index ec5720e078..d39e378798 100644 --- a/src/applications/auth/provider/PhabricatorPasswordAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorPasswordAuthProvider.php @@ -159,8 +159,7 @@ final class PhabricatorPasswordAuthProvider extends PhabricatorAuthProvider { return $dialog; } - public function buildLinkForm( - PhabricatorAuthLinkController $controller) { + public function buildLinkForm($controller) { throw new Exception(pht("Password providers can't be linked.")); } @@ -359,14 +358,6 @@ final class PhabricatorPasswordAuthProvider extends PhabricatorAuthProvider { return true; } - public function getDefaultExternalAccount() { - $adapter = $this->getAdapter(); - - return id(new PhabricatorExternalAccount()) - ->setAccountType($adapter->getAdapterType()) - ->setAccountDomain($adapter->getAdapterDomain()); - } - protected function willSaveAccount(PhabricatorExternalAccount $account) { parent::willSaveAccount($account); $account->setUserPHID($account->getAccountID()); diff --git a/src/applications/auth/query/PhabricatorExternalAccountQuery.php b/src/applications/auth/query/PhabricatorExternalAccountQuery.php index c4a53c12f8..1c5b3fe14f 100644 --- a/src/applications/auth/query/PhabricatorExternalAccountQuery.php +++ b/src/applications/auth/query/PhabricatorExternalAccountQuery.php @@ -21,6 +21,7 @@ final class PhabricatorExternalAccountQuery private $userPHIDs; private $needImages; private $accountSecrets; + private $providerConfigPHIDs; public function withUserPHIDs(array $user_phids) { $this->userPHIDs = $user_phids; @@ -62,6 +63,11 @@ final class PhabricatorExternalAccountQuery return $this; } + public function withProviderConfigPHIDs(array $phids) { + $this->providerConfigPHIDs = $phids; + return $this; + } + public function newResultObject() { return new PhabricatorExternalAccount(); } @@ -71,6 +77,26 @@ final class PhabricatorExternalAccountQuery } protected function willFilterPage(array $accounts) { + $viewer = $this->getViewer(); + + $configs = id(new PhabricatorAuthProviderConfigQuery()) + ->setViewer($viewer) + ->withPHIDs(mpull($accounts, 'getProviderConfigPHID')) + ->execute(); + $configs = mpull($configs, null, 'getPHID'); + + foreach ($accounts as $key => $account) { + $config_phid = $account->getProviderConfigPHID(); + $config = idx($configs, $config_phid); + + if (!$config) { + unset($accounts[$key]); + continue; + } + + $account->attachProviderConfig($config); + } + if ($this->needImages) { $file_phids = mpull($accounts, 'getProfileImagePHID'); $file_phids = array_filter($file_phids); @@ -161,6 +187,13 @@ final class PhabricatorExternalAccountQuery $this->accountSecrets); } + if ($this->providerConfigPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'providerConfigPHID IN (%Ls)', + $this->providerConfigPHIDs); + } + return $where; } diff --git a/src/applications/auth/view/PhabricatorAuthChallengeUpdate.php b/src/applications/auth/view/PhabricatorAuthChallengeUpdate.php new file mode 100644 index 0000000000..a8ae5b8825 --- /dev/null +++ b/src/applications/auth/view/PhabricatorAuthChallengeUpdate.php @@ -0,0 +1,44 @@ +retry = $retry; + return $this; + } + + public function getRetry() { + return $this->retry; + } + + public function setState($state) { + $this->state = $state; + return $this; + } + + public function getState() { + return $this->state; + } + + public function setMarkup($markup) { + $this->markup = $markup; + return $this; + } + + public function getMarkup() { + return $this->markup; + } + + public function newContent() { + return array( + 'retry' => $this->getRetry(), + 'state' => $this->getState(), + 'markup' => $this->getMarkup(), + ); + } +} diff --git a/src/applications/calendar/controller/PhabricatorCalendarImportViewController.php b/src/applications/calendar/controller/PhabricatorCalendarImportViewController.php index 9c07d371d0..6d92f00a97 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarImportViewController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarImportViewController.php @@ -234,7 +234,7 @@ final class PhabricatorCalendarImportViewController $all_uri = $this->getApplicationURI('import/log/'); $all_uri = (string)id(new PhutilURI($all_uri)) - ->setQueryParam('importSourcePHID', $import->getPHID()); + ->replaceQueryParam('importSourcePHID', $import->getPHID()); $all_button = id(new PHUIButtonView()) ->setTag('a') @@ -273,8 +273,8 @@ final class PhabricatorCalendarImportViewController $all_uri = $this->getApplicationURI(); $all_uri = (string)id(new PhutilURI($all_uri)) - ->setQueryParam('importSourcePHID', $import->getPHID()) - ->setQueryParam('display', 'list'); + ->replaceQueryParam('importSourcePHID', $import->getPHID()) + ->replaceQueryParam('display', 'list'); $all_button = id(new PHUIButtonView()) ->setTag('a') diff --git a/src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php b/src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php index 2c6e58da50..b9893f6924 100644 --- a/src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php +++ b/src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php @@ -11,8 +11,7 @@ final class PhabricatorChatLogChannelLogController $viewer = $request->getViewer(); $id = $request->getURIData('channelID'); - $uri = clone $request->getRequestURI(); - $uri->setQueryParams(array()); + $uri = new PhutilURI($request->getPath()); $pager = new AphrontCursorPagerView(); $pager->setURI($uri); diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php index 932e04db4b..a742e3b82b 100644 --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -15,6 +15,9 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { $defined_keys = PhabricatorApplicationConfigOptions::loadAllOptions(); + $stack = PhabricatorEnv::getConfigSourceStack(); + $stack = $stack->getStack(); + foreach ($all_keys as $key) { if (isset($defined_keys[$key])) { continue; @@ -48,9 +51,6 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { ->setName($name) ->setSummary($summary); - $stack = PhabricatorEnv::getConfigSourceStack(); - $stack = $stack->getStack(); - $found = array(); $found_local = false; $found_database = false; @@ -85,6 +85,101 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { } } + $options = PhabricatorApplicationConfigOptions::loadAllOptions(); + foreach ($defined_keys as $key => $value) { + $option = idx($options, $key); + if (!$option) { + continue; + } + + if (!$option->getLocked()) { + continue; + } + + $found_database = false; + foreach ($stack as $source_key => $source) { + $value = $source->getKeys(array($key)); + if ($value) { + if ($source instanceof PhabricatorConfigDatabaseSource) { + $found_database = true; + break; + } + } + } + + if (!$found_database) { + continue; + } + + // NOTE: These are values which we don't let you edit directly, but edit + // via other UI workflows. For now, don't raise this warning about them. + // In the future, before we stop reading database configuration for + // locked values, we either need to add a flag which lets these values + // continue reading from the database or move them to some other storage + // mechanism. + $soft_locks = array( + 'phabricator.uninstalled-applications', + 'phabricator.application-settings', + 'config.ignore-issues', + ); + $soft_locks = array_fuse($soft_locks); + if (isset($soft_locks[$key])) { + continue; + } + + $doc_name = 'Configuration Guide: Locked and Hidden Configuration'; + $doc_href = PhabricatorEnv::getDoclink($doc_name); + + $set_command = phutil_tag( + 'tt', + array(), + csprintf( + 'bin/config set %R ', + $key)); + + $summary = pht( + 'Configuration value "%s" is locked, but has a value in the database.', + $key); + $message = pht( + 'The configuration value "%s" is locked (so it can not be edited '. + 'from the web UI), but has a database value. Usually, this means '. + 'that it was previously not locked, you set it using the web UI, '. + 'and it later became locked.'. + "\n\n". + 'You should copy this configuration value in a local configuration '. + 'source (usually by using %s) and then remove it from the database '. + 'with the command below.'. + "\n\n". + 'For more information on locked and hidden configuration, including '. + 'details about this setup issue, see %s.'. + "\n\n". + 'This database value is currently respected, but a future version '. + 'of Phabricator will stop respecting database values for locked '. + 'configuration options.', + $key, + $set_command, + phutil_tag( + 'a', + array( + 'href' => $doc_href, + 'target' => '_blank', + ), + $doc_name)); + $command = csprintf( + 'phabricator/ $ ./bin/config delete --database %R', + $key); + + $this->newIssue('config.locked.'.$key) + ->setShortName(pht('Deprecated Config Source')) + ->setName( + pht( + 'Locked Configuration Option "%s" Has Database Value', + $key)) + ->setSummary($summary) + ->setMessage($message) + ->addCommand($command) + ->addPhabricatorConfig($key); + } if (PhabricatorEnv::getEnvConfig('feed.http-hooks')) { $this->newIssue('config.deprecated.feed.http-hooks') diff --git a/src/applications/config/check/PhabricatorWebServerSetupCheck.php b/src/applications/config/check/PhabricatorWebServerSetupCheck.php index 398ebd6376..8f6885e8e8 100644 --- a/src/applications/config/check/PhabricatorWebServerSetupCheck.php +++ b/src/applications/config/check/PhabricatorWebServerSetupCheck.php @@ -40,7 +40,7 @@ final class PhabricatorWebServerSetupCheck extends PhabricatorSetupCheck { $base_uri = id(new PhutilURI($base_uri)) ->setPath($send_path) - ->setQueryParam($expect_key, $expect_value); + ->replaceQueryParam($expect_key, $expect_value); $self_future = id(new HTTPSFuture($base_uri)) ->addHeader('X-Phabricator-SelfCheck', 1) diff --git a/src/applications/config/option/PhabricatorPHDConfigOptions.php b/src/applications/config/option/PhabricatorPHDConfigOptions.php index 37fae45dfb..e04353876a 100644 --- a/src/applications/config/option/PhabricatorPHDConfigOptions.php +++ b/src/applications/config/option/PhabricatorPHDConfigOptions.php @@ -41,7 +41,12 @@ final class PhabricatorPHDConfigOptions "If you are running a cluster, this limit applies separately ". "to each instance of `phd`. For example, if this limit is set ". "to `4` and you have three hosts running daemons, the effective ". - "global limit will be 12.")), + "global limit will be 12.". + "\n\n". + "After changing this value, you must restart the daemons. Most ". + "configuration changes are picked up by the daemons ". + "automatically, but pool sizes can not be changed without a ". + "restart.")), $this->newOption('phd.verbose', 'bool', false) ->setLocked(true) ->setBoolOptions( diff --git a/src/applications/conpherence/controller/ConpherenceViewController.php b/src/applications/conpherence/controller/ConpherenceViewController.php index 996417a307..357d07631a 100644 --- a/src/applications/conpherence/controller/ConpherenceViewController.php +++ b/src/applications/conpherence/controller/ConpherenceViewController.php @@ -188,7 +188,7 @@ final class ConpherenceViewController extends } else { // user not logged in so give them a login button. $login_href = id(new PhutilURI('/auth/start/')) - ->setQueryParam('next', '/'.$conpherence->getMonogram()); + ->replaceQueryParam('next', '/'.$conpherence->getMonogram()); return id(new PHUIFormLayoutView()) ->addClass('login-to-participate') ->appendInstructions(pht('Log in to join this room and participate.')) diff --git a/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php b/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php index dfe328933a..fc62c4d5cb 100644 --- a/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php +++ b/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php @@ -287,7 +287,7 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject { $edit_uri = "/dashboard/panel/edit/{$panel_id}/"; $edit_uri = new PhutilURI($edit_uri); if ($dashboard_id) { - $edit_uri->setQueryParam('dashboardID', $dashboard_id); + $edit_uri->replaceQueryParam('dashboardID', $dashboard_id); } $action_edit = id(new PHUIIconView()) @@ -303,7 +303,7 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject { $remove_uri = "/dashboard/removepanel/{$dashboard_id}/"; $remove_uri = id(new PhutilURI($remove_uri)) - ->setQueryParam('panelPHID', $panel_phid); + ->replaceQueryParam('panelPHID', $panel_phid); $action_remove = id(new PHUIIconView()) ->setIcon('fa-trash-o') diff --git a/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php b/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php index ba6aace971..9f6481c05b 100644 --- a/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php +++ b/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php @@ -113,11 +113,11 @@ final class PhabricatorDashboardRenderingEngine extends Phobject { $dashboard_id = $this->dashboard->getID(); $create_uri = id(new PhutilURI('/dashboard/panel/create/')) - ->setQueryParam('dashboardID', $dashboard_id) - ->setQueryParam('column', $column); + ->replaceQueryParam('dashboardID', $dashboard_id) + ->replaceQueryParam('column', $column); $add_uri = id(new PhutilURI('/dashboard/addpanel/'.$dashboard_id.'/')) - ->setQueryParam('column', $column); + ->replaceQueryParam('column', $column); $create_button = id(new PHUIButtonView()) ->setTag('a') diff --git a/src/applications/differential/controller/DifferentialDiffCreateController.php b/src/applications/differential/controller/DifferentialDiffCreateController.php index 284edf49d3..9aaf407e28 100644 --- a/src/applications/differential/controller/DifferentialDiffCreateController.php +++ b/src/applications/differential/controller/DifferentialDiffCreateController.php @@ -71,7 +71,7 @@ final class DifferentialDiffCreateController extends DifferentialController { $uri = $this->getApplicationURI("diff/{$diff_id}/"); $uri = new PhutilURI($uri); if ($revision) { - $uri->setQueryParam('revisionID', $revision->getID()); + $uri->replaceQueryParam('revisionID', $revision->getID()); } return id(new AphrontRedirectResponse())->setURI($uri); diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 8216c11557..9bc6345576 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -1098,7 +1098,8 @@ final class DifferentialRevisionViewController // D123.vs123.id123.whitespaceignore-all.diff // lame but nice to include these options $file_name = ltrim($request_uri->getPath(), '/').'.'; - foreach ($request_uri->getQueryParams() as $key => $value) { + foreach ($request_uri->getQueryParamsAsPairList() as $pair) { + list($key, $value) = $pair; if ($key == 'download') { continue; } diff --git a/src/applications/differential/harbormaster/DifferentialBuildableEngine.php b/src/applications/differential/harbormaster/DifferentialBuildableEngine.php index 8554f7be25..8565c2dcad 100644 --- a/src/applications/differential/harbormaster/DifferentialBuildableEngine.php +++ b/src/applications/differential/harbormaster/DifferentialBuildableEngine.php @@ -7,7 +7,11 @@ final class DifferentialBuildableEngine $object = $this->getObject(); if ($object instanceof DifferentialDiff) { - return $object->getRevision(); + if ($object->getRevisionID()) { + return $object->getRevision(); + } else { + return null; + } } return $object; diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php index 14de553e59..367991497c 100644 --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -358,7 +358,7 @@ final class DifferentialChangesetListView extends AphrontView { if ($this->standaloneURI) { $uri = new PhutilURI($this->standaloneURI); - $uri->setQueryParams($uri->getQueryParams() + $qparams); + $uri = $this->appendDefaultQueryParams($uri, $qparams); $meta['standaloneURI'] = (string)$uri; } @@ -381,7 +381,7 @@ final class DifferentialChangesetListView extends AphrontView { if ($this->leftRawFileURI) { if ($change != DifferentialChangeType::TYPE_ADD) { $uri = new PhutilURI($this->leftRawFileURI); - $uri->setQueryParams($uri->getQueryParams() + $qparams); + $uri = $this->appendDefaultQueryParams($uri, $qparams); $meta['leftURI'] = (string)$uri; } } @@ -390,7 +390,7 @@ final class DifferentialChangesetListView extends AphrontView { if ($change != DifferentialChangeType::TYPE_DELETE && $change != DifferentialChangeType::TYPE_MULTICOPY) { $uri = new PhutilURI($this->rightRawFileURI); - $uri->setQueryParams($uri->getQueryParams() + $qparams); + $uri = $this->appendDefaultQueryParams($uri, $qparams); $meta['rightURI'] = (string)$uri; } } @@ -421,4 +421,23 @@ final class DifferentialChangesetListView extends AphrontView { } + private function appendDefaultQueryParams(PhutilURI $uri, array $params) { + // Add these default query parameters to the query string if they do not + // already exist. + + $have = array(); + foreach ($uri->getQueryParamsAsPairList() as $pair) { + list($key, $value) = $pair; + $have[$key] = true; + } + + foreach ($params as $key => $value) { + if (!isset($have[$key])) { + $uri->appendQueryParam($key, $value); + } + } + + return $uri; + } + } diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index 6a863a4a92..fcef87b7ef 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -709,8 +709,6 @@ final class DiffusionBrowseController extends DiffusionController { 'path' => $path, )); - $before_uri->setQueryParams($request->getRequestURI()->getQueryParams()); - $before_uri = $before_uri->alter('before', null); $before_uri = $before_uri->alter('renamed', $renamed); $before_uri = $before_uri->alter('follow', $follow); diff --git a/src/applications/diffusion/view/DiffusionView.php b/src/applications/diffusion/view/DiffusionView.php index eb7f3eb72f..123d22af5e 100644 --- a/src/applications/diffusion/view/DiffusionView.php +++ b/src/applications/diffusion/view/DiffusionView.php @@ -81,12 +81,12 @@ abstract class DiffusionView extends AphrontView { } if (isset($details['external'])) { - $href = id(new PhutilURI('/diffusion/external/')) - ->setQueryParams( - array( - 'uri' => idx($details, 'external'), - 'id' => idx($details, 'hash'), - )); + $params = array( + 'uri' => idx($details, 'external'), + 'id' => idx($details, 'hash'), + ); + + $href = new PhutilURI('/diffusion/external/', $params); $tip = pht('Browse External'); } else { $href = $this->getDiffusionRequest()->generateURI( diff --git a/src/applications/diviner/markup/DivinerSymbolRemarkupRule.php b/src/applications/diviner/markup/DivinerSymbolRemarkupRule.php index 7fdb4cb37e..db68927625 100644 --- a/src/applications/diviner/markup/DivinerSymbolRemarkupRule.php +++ b/src/applications/diviner/markup/DivinerSymbolRemarkupRule.php @@ -111,15 +111,15 @@ final class DivinerSymbolRemarkupRule extends PhutilRemarkupRule { // Here, we're generating comment text or something like that. Just // link to Diviner and let it sort things out. - $href = id(new PhutilURI('/diviner/find/')) - ->setQueryParams( - array( - 'book' => $ref->getBook(), - 'name' => $ref->getName(), - 'type' => $ref->getType(), - 'context' => $ref->getContext(), - 'jump' => true, - )); + $params = array( + 'book' => $ref->getBook(), + 'name' => $ref->getName(), + 'type' => $ref->getType(), + 'context' => $ref->getContext(), + 'jump' => true, + ); + + $href = new PhutilURI('/diviner/find/', $params); } // TODO: This probably is not the best place to do this. Move it somewhere diff --git a/src/applications/fact/controller/PhabricatorFactHomeController.php b/src/applications/fact/controller/PhabricatorFactHomeController.php index 82f6a0905b..56ffe3930b 100644 --- a/src/applications/fact/controller/PhabricatorFactHomeController.php +++ b/src/applications/fact/controller/PhabricatorFactHomeController.php @@ -11,7 +11,7 @@ final class PhabricatorFactHomeController extends PhabricatorFactController { if ($request->isFormPost()) { $uri = new PhutilURI('/fact/chart/'); - $uri->setQueryParam('y1', $request->getStr('y1')); + $uri->replaceQueryParam('y1', $request->getStr('y1')); return id(new AphrontRedirectResponse())->setURI($uri); } diff --git a/src/applications/files/controller/PhabricatorFileDataController.php b/src/applications/files/controller/PhabricatorFileDataController.php index dd5e0adb7d..8ceb1e7907 100644 --- a/src/applications/files/controller/PhabricatorFileDataController.php +++ b/src/applications/files/controller/PhabricatorFileDataController.php @@ -135,7 +135,7 @@ final class PhabricatorFileDataController extends PhabricatorFileController { $request_uri = id(clone $request->getAbsoluteRequestURI()) ->setPath(null) ->setFragment(null) - ->setQueryParams(array()); + ->removeAllQueryParams(); $response->addContentSecurityPolicyURI( 'object-src', diff --git a/src/applications/files/controller/PhabricatorFileLightboxController.php b/src/applications/files/controller/PhabricatorFileLightboxController.php index 1f679d621b..59a826dd42 100644 --- a/src/applications/files/controller/PhabricatorFileLightboxController.php +++ b/src/applications/files/controller/PhabricatorFileLightboxController.php @@ -70,7 +70,7 @@ final class PhabricatorFileLightboxController if (!$viewer->isLoggedIn()) { $login_href = id(new PhutilURI('/auth/start/')) - ->setQueryParam('next', '/'.$file->getMonogram()); + ->replaceQueryParam('next', '/'.$file->getMonogram()); return id(new PHUIFormLayoutView()) ->addClass('phui-comment-panel-empty') ->appendChild( diff --git a/src/applications/files/controller/PhabricatorFileTransformListController.php b/src/applications/files/controller/PhabricatorFileTransformListController.php index ab5322fc1a..7b5bc9299d 100644 --- a/src/applications/files/controller/PhabricatorFileTransformListController.php +++ b/src/applications/files/controller/PhabricatorFileTransformListController.php @@ -61,7 +61,7 @@ final class PhabricatorFileTransformListController $view_href = $file->getURIForTransform($xform); $view_href = new PhutilURI($view_href); - $view_href->setQueryParam('regenerate', 'true'); + $view_href->replaceQueryParam('regenerate', 'true'); $view_text = pht('Regenerate'); diff --git a/src/applications/files/markup/PhabricatorImageRemarkupRule.php b/src/applications/files/markup/PhabricatorImageRemarkupRule.php index 5d1979ed3c..57ad75bbc5 100644 --- a/src/applications/files/markup/PhabricatorImageRemarkupRule.php +++ b/src/applications/files/markup/PhabricatorImageRemarkupRule.php @@ -149,7 +149,7 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule { )); } else { $src_uri = id(new PhutilURI('/file/imageproxy/')) - ->setQueryParam('uri', $uri); + ->replaceQueryParam('uri', $uri); $img = id(new PHUIRemarkupImageView()) ->setURI($src_uri) diff --git a/src/applications/herald/controller/HeraldNewController.php b/src/applications/herald/controller/HeraldNewController.php index fbaf1aeb9e..f571aeb395 100644 --- a/src/applications/herald/controller/HeraldNewController.php +++ b/src/applications/herald/controller/HeraldNewController.php @@ -81,13 +81,13 @@ final class HeraldNewController extends HeraldController { } if (!$errors && $done) { - $uri = id(new PhutilURI('edit/')) - ->setQueryParams( - array( - 'content_type' => $content_type, - 'rule_type' => $rule_type, - 'targetPHID' => $target_phid, - )); + $params = array( + 'content_type' => $content_type, + 'rule_type' => $rule_type, + 'targetPHID' => $target_phid, + ); + + $uri = new PhutilURI('edit/', $params); $uri = $this->getApplicationURI($uri); return id(new AphrontRedirectResponse())->setURI($uri); } @@ -126,13 +126,13 @@ final class HeraldNewController extends HeraldController { ->addHiddenInput('step', 2) ->appendChild($rule_types); + $params = array( + 'content_type' => $content_type, + 'step' => '0', + ); + $cancel_text = pht('Back'); - $cancel_uri = id(new PhutilURI('new/')) - ->setQueryParams( - array( - 'content_type' => $content_type, - 'step' => 0, - )); + $cancel_uri = new PhutilURI('new/', $params); $cancel_uri = $this->getApplicationURI($cancel_uri); $title = pht('Create Herald Rule: %s', idx($content_type_map, $content_type)); @@ -173,14 +173,14 @@ final class HeraldNewController extends HeraldController { ->setValue($request->getStr('objectName')) ->setLabel(pht('Object'))); + $params = array( + 'content_type' => $content_type, + 'rule_type' => $rule_type, + 'step' => 1, + ); + $cancel_text = pht('Back'); - $cancel_uri = id(new PhutilURI('new/')) - ->setQueryParams( - array( - 'content_type' => $content_type, - 'rule_type' => $rule_type, - 'step' => 1, - )); + $cancel_uri = new PhutilURI('new/', $params); $cancel_uri = $this->getApplicationURI($cancel_uri); $title = pht('Create Herald Rule: %s', idx($content_type_map, $content_type)); diff --git a/src/applications/herald/storage/HeraldWebhookRequest.php b/src/applications/herald/storage/HeraldWebhookRequest.php index 3381f6a99c..bc916fd60b 100644 --- a/src/applications/herald/storage/HeraldWebhookRequest.php +++ b/src/applications/herald/storage/HeraldWebhookRequest.php @@ -120,7 +120,7 @@ final class HeraldWebhookRequest public function getErrorTypeForDisplay() { $map = array( self::ERRORTYPE_HOOK => pht('Hook Error'), - self::ERRORTYPE_HTTP => pht('HTTP Error'), + self::ERRORTYPE_HTTP => pht('HTTP Status Code'), self::ERRORTYPE_TIMEOUT => pht('Request Timeout'), ); diff --git a/src/applications/macro/engine/PhabricatorMemeEngine.php b/src/applications/macro/engine/PhabricatorMemeEngine.php index 7433a4e8bc..afee0f9b18 100644 --- a/src/applications/macro/engine/PhabricatorMemeEngine.php +++ b/src/applications/macro/engine/PhabricatorMemeEngine.php @@ -47,10 +47,13 @@ final class PhabricatorMemeEngine extends Phobject { } public function getGenerateURI() { - return id(new PhutilURI('/macro/meme/')) - ->alter('macro', $this->getTemplate()) - ->alter('above', $this->getAboveText()) - ->alter('below', $this->getBelowText()); + $params = array( + 'macro' => $this->getTemplate(), + 'above' => $this->getAboveText(), + 'below' => $this->getBelowText(), + ); + + return new PhutilURI('/macro/meme/', $params); } public function newAsset() { diff --git a/src/applications/maniphest/application/PhabricatorManiphestApplication.php b/src/applications/maniphest/application/PhabricatorManiphestApplication.php index 35c1efb6e8..ec732791fa 100644 --- a/src/applications/maniphest/application/PhabricatorManiphestApplication.php +++ b/src/applications/maniphest/application/PhabricatorManiphestApplication.php @@ -55,6 +55,7 @@ final class PhabricatorManiphestApplication extends PhabricatorApplication { 'subtask/(?P[1-9]\d*)/' => 'ManiphestTaskSubtaskController', ), 'subpriority/' => 'ManiphestSubpriorityController', + 'graph/(?P[1-9]\d*)/' => 'ManiphestTaskGraphController', ), ); } diff --git a/src/applications/maniphest/controller/ManiphestController.php b/src/applications/maniphest/controller/ManiphestController.php index c80a1a462a..872d3f7b38 100644 --- a/src/applications/maniphest/controller/ManiphestController.php +++ b/src/applications/maniphest/controller/ManiphestController.php @@ -61,4 +61,102 @@ abstract class ManiphestController extends PhabricatorController { return $view; } + final protected function newTaskGraphDropdownMenu( + ManiphestTask $task, + $has_parents, + $has_subtasks, + $include_standalone) { + $viewer = $this->getViewer(); + + $parents_uri = urisprintf( + '/?subtaskIDs=%d#R', + $task->getID()); + $parents_uri = $this->getApplicationURI($parents_uri); + + $subtasks_uri = urisprintf( + '/?parentIDs=%d#R', + $task->getID()); + $subtasks_uri = $this->getApplicationURI($subtasks_uri); + + $dropdown_menu = id(new PhabricatorActionListView()) + ->setViewer($viewer) + ->addAction( + id(new PhabricatorActionView()) + ->setHref($parents_uri) + ->setName(pht('Search Parent Tasks')) + ->setDisabled(!$has_parents) + ->setIcon('fa-chevron-circle-up')) + ->addAction( + id(new PhabricatorActionView()) + ->setHref($subtasks_uri) + ->setName(pht('Search Subtasks')) + ->setDisabled(!$has_subtasks) + ->setIcon('fa-chevron-circle-down')); + + if ($include_standalone) { + $standalone_uri = urisprintf('/graph/%d/', $task->getID()); + $standalone_uri = $this->getApplicationURI($standalone_uri); + + $dropdown_menu->addAction( + id(new PhabricatorActionView()) + ->setHref($standalone_uri) + ->setName(pht('View Standalone Graph')) + ->setIcon('fa-code-fork')); + } + + $graph_menu = id(new PHUIButtonView()) + ->setTag('a') + ->setIcon('fa-search') + ->setText(pht('Search...')) + ->setDropdownMenu($dropdown_menu); + + return $graph_menu; + } + + final protected function newTaskGraphOverflowView( + ManiphestTask $task, + $overflow_message, + $include_standalone) { + + $id = $task->getID(); + + if ($include_standalone) { + $standalone_uri = $this->getApplicationURI("graph/{$id}/"); + + $standalone_link = id(new PHUIButtonView()) + ->setTag('a') + ->setHref($standalone_uri) + ->setColor(PHUIButtonView::GREY) + ->setIcon('fa-code-fork') + ->setText(pht('View Standalone Graph')); + } else { + $standalone_link = null; + } + + $standalone_icon = id(new PHUIIconView()) + ->setIcon('fa-exclamation-triangle', 'yellow') + ->addClass('object-graph-header-icon'); + + $standalone_view = phutil_tag( + 'div', + array( + 'class' => 'object-graph-header', + ), + array( + $standalone_link, + $standalone_icon, + phutil_tag( + 'div', + array( + 'class' => 'object-graph-header-message', + ), + array( + $overflow_message, + )), + )); + + return $standalone_view; + } + + } diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index 0f96d76b91..c5dba7d3b5 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -80,7 +80,8 @@ final class ManiphestTaskDetailController extends ManiphestController { $related_tabs = array(); $graph_menu = null; - $graph_limit = 100; + $graph_limit = 200; + $overflow_message = null; $task_graph = id(new ManiphestTaskGraph()) ->setViewer($viewer) ->setSeedPHID($task->getPHID()) @@ -96,61 +97,55 @@ final class ManiphestTaskDetailController extends ManiphestController { $has_parents = (bool)$parent_list; $has_subtasks = (bool)$subtask_list; - $search_text = pht('Search...'); - // First, get a count of direct parent tasks and subtasks. If there // are too many of these, we just don't draw anything. You can use // the search button to browse tasks with the search UI instead. $direct_count = count($parent_list) + count($subtask_list); if ($direct_count > $graph_limit) { - $message = pht( - 'Task graph too large to display (this task is directly connected '. - 'to more than %s other tasks). Use %s to explore connected tasks.', - $graph_limit, - phutil_tag('strong', array(), $search_text)); - $message = phutil_tag('em', array(), $message); - $graph_table = id(new PHUIPropertyListView()) - ->addTextContent($message); + $overflow_message = pht( + 'This task is directly connected to more than %s other tasks. '. + 'Use %s to browse parents or subtasks, or %s to show more of the '. + 'graph.', + new PhutilNumber($graph_limit), + phutil_tag('strong', array(), pht('Search...')), + phutil_tag('strong', array(), pht('View Standalone Graph'))); + + $graph_table = null; } else { // If there aren't too many direct tasks, but there are too many total // tasks, we'll only render directly connected tasks. if ($task_graph->isOverLimit()) { $task_graph->setRenderOnlyAdjacentNodes(true); + + $overflow_message = pht( + 'This task is connected to more than %s other tasks. '. + 'Only direct parents and subtasks are shown here. Use '. + '%s to show more of the graph.', + new PhutilNumber($graph_limit), + phutil_tag('strong', array(), pht('View Standalone Graph'))); } + $graph_table = $task_graph->newGraphTable(); } - $parents_uri = urisprintf( - '/?subtaskIDs=%d#R', - $task->getID()); - $parents_uri = $this->getApplicationURI($parents_uri); + if ($overflow_message) { + $overflow_view = $this->newTaskGraphOverflowView( + $task, + $overflow_message, + true); - $subtasks_uri = urisprintf( - '/?parentIDs=%d#R', - $task->getID()); - $subtasks_uri = $this->getApplicationURI($subtasks_uri); + $graph_table = array( + $overflow_view, + $graph_table, + ); + } - $dropdown_menu = id(new PhabricatorActionListView()) - ->setViewer($viewer) - ->addAction( - id(new PhabricatorActionView()) - ->setHref($parents_uri) - ->setName(pht('Search Parent Tasks')) - ->setDisabled(!$has_parents) - ->setIcon('fa-chevron-circle-up')) - ->addAction( - id(new PhabricatorActionView()) - ->setHref($subtasks_uri) - ->setName(pht('Search Subtasks')) - ->setDisabled(!$has_subtasks) - ->setIcon('fa-chevron-circle-down')); - - $graph_menu = id(new PHUIButtonView()) - ->setTag('a') - ->setIcon('fa-search') - ->setText($search_text) - ->setDropdownMenu($dropdown_menu); + $graph_menu = $this->newTaskGraphDropdownMenu( + $task, + $has_parents, + $has_subtasks, + true); $related_tabs[] = id(new PHUITabView()) ->setName(pht('Task Graph')) @@ -300,9 +295,9 @@ final class ManiphestTaskDetailController extends ManiphestController { $subtask_form = head($subtask_options); $form_key = $subtask_form->getIdentifier(); $subtask_uri = id(new PhutilURI("/task/edit/form/{$form_key}/")) - ->setQueryParam('parent', $id) - ->setQueryParam('template', $id) - ->setQueryParam('status', ManiphestTaskStatus::getDefaultStatus()); + ->replaceQueryParam('parent', $id) + ->replaceQueryParam('template', $id) + ->replaceQueryParam('status', ManiphestTaskStatus::getDefaultStatus()); $subtask_workflow = false; } diff --git a/src/applications/maniphest/controller/ManiphestTaskGraphController.php b/src/applications/maniphest/controller/ManiphestTaskGraphController.php new file mode 100644 index 0000000000..2f342a2d0f --- /dev/null +++ b/src/applications/maniphest/controller/ManiphestTaskGraphController.php @@ -0,0 +1,125 @@ +getViewer(); + $id = $request->getURIData('id'); + + $task = id(new ManiphestTaskQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->executeOne(); + if (!$task) { + return new Aphront404Response(); + } + + $crumbs = $this->buildApplicationCrumbs() + ->addTextCrumb($task->getMonogram(), $task->getURI()) + ->addTextCrumb(pht('Graph')) + ->setBorder(true); + + $graph_limit = 2000; + $overflow_message = null; + $task_graph = id(new ManiphestTaskGraph()) + ->setViewer($viewer) + ->setSeedPHID($task->getPHID()) + ->setLimit($graph_limit) + ->loadGraph(); + if (!$task_graph->isEmpty()) { + $parent_type = ManiphestTaskDependedOnByTaskEdgeType::EDGECONST; + $subtask_type = ManiphestTaskDependsOnTaskEdgeType::EDGECONST; + $parent_map = $task_graph->getEdges($parent_type); + $subtask_map = $task_graph->getEdges($subtask_type); + $parent_list = idx($parent_map, $task->getPHID(), array()); + $subtask_list = idx($subtask_map, $task->getPHID(), array()); + $has_parents = (bool)$parent_list; + $has_subtasks = (bool)$subtask_list; + + // First, get a count of direct parent tasks and subtasks. If there + // are too many of these, we just don't draw anything. You can use + // the search button to browse tasks with the search UI instead. + $direct_count = count($parent_list) + count($subtask_list); + + if ($direct_count > $graph_limit) { + $overflow_message = pht( + 'This task is directly connected to more than %s other tasks, '. + 'which is too many tasks to display. Use %s to browse parents '. + 'or subtasks.', + new PhutilNumber($graph_limit), + phutil_tag('strong', array(), pht('Search...'))); + + $graph_table = null; + } else { + // If there aren't too many direct tasks, but there are too many total + // tasks, we'll only render directly connected tasks. + if ($task_graph->isOverLimit()) { + $task_graph->setRenderOnlyAdjacentNodes(true); + + $overflow_message = pht( + 'This task is connected to more than %s other tasks. '. + 'Only direct parents and subtasks are shown here.', + new PhutilNumber($graph_limit)); + } + + $graph_table = $task_graph->newGraphTable(); + } + + $graph_menu = $this->newTaskGraphDropdownMenu( + $task, + $has_parents, + $has_subtasks, + false); + } else { + $graph_menu = null; + $graph_table = null; + + $overflow_message = pht( + 'This task has no parent tasks and no subtasks, so there is no '. + 'graph to draw.'); + } + + if ($overflow_message) { + $overflow_view = $this->newTaskGraphOverflowView( + $task, + $overflow_message, + false); + + $graph_table = array( + $overflow_view, + $graph_table, + ); + } + + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Task Graph')); + + if ($graph_menu) { + $header->addActionLink($graph_menu); + } + + $tab_view = id(new PHUIObjectBoxView()) + ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($graph_table); + + $view = id(new PHUITwoColumnView()) + ->setFooter($tab_view); + + return $this->newPage() + ->setTitle( + array( + $task->getMonogram(), + pht('Graph'), + )) + ->setCrumbs($crumbs) + ->appendChild($view); + } + + +} diff --git a/src/applications/maniphest/controller/ManiphestTaskSubtaskController.php b/src/applications/maniphest/controller/ManiphestTaskSubtaskController.php index 5256c1bd26..3105cf661d 100644 --- a/src/applications/maniphest/controller/ManiphestTaskSubtaskController.php +++ b/src/applications/maniphest/controller/ManiphestTaskSubtaskController.php @@ -47,9 +47,9 @@ final class ManiphestTaskSubtaskController $subtype = $subtype_map->getSubtype($subtype_key); $subtask_uri = id(new PhutilURI("/task/edit/form/{$form_key}/")) - ->setQueryParam('parent', $id) - ->setQueryParam('template', $id) - ->setQueryParam('status', ManiphestTaskStatus::getDefaultStatus()); + ->replaceQueryParam('parent', $id) + ->replaceQueryParam('template', $id) + ->replaceQueryParam('status', ManiphestTaskStatus::getDefaultStatus()); $subtask_uri = $this->getApplicationURI($subtask_uri); $item = id(new PHUIObjectItemView()) diff --git a/src/applications/maniphest/view/ManiphestTaskListView.php b/src/applications/maniphest/view/ManiphestTaskListView.php index e7128fb850..6bf5daf29b 100644 --- a/src/applications/maniphest/view/ManiphestTaskListView.php +++ b/src/applications/maniphest/view/ManiphestTaskListView.php @@ -135,7 +135,7 @@ final class ManiphestTaskListView extends ManiphestView { if ($this->showBatchControls) { $href = new PhutilURI('/maniphest/task/edit/'.$task->getID().'/'); if (!$this->showSubpriorityControls) { - $href->setQueryParam('ungrippable', 'true'); + $href->replaceQueryParam('ungrippable', 'true'); } $item->addAction( id(new PHUIListItemView()) diff --git a/src/applications/metamta/adapter/PhabricatorMailMailgunAdapter.php b/src/applications/metamta/adapter/PhabricatorMailMailgunAdapter.php index 9eb478efc5..8223ee8102 100644 --- a/src/applications/metamta/adapter/PhabricatorMailMailgunAdapter.php +++ b/src/applications/metamta/adapter/PhabricatorMailMailgunAdapter.php @@ -24,6 +24,7 @@ final class PhabricatorMailMailgunAdapter array( 'api-key' => 'string', 'domain' => 'string', + 'api-hostname' => 'string', )); } @@ -31,12 +32,14 @@ final class PhabricatorMailMailgunAdapter return array( 'api-key' => null, 'domain' => null, + 'api-hostname' => 'api.mailgun.net', ); } public function sendMessage(PhabricatorMailExternalMessage $message) { $api_key = $this->getOption('api-key'); $domain = $this->getOption('domain'); + $api_hostname = $this->getOption('api-hostname'); $params = array(); $subject = $message->getSubject(); @@ -92,7 +95,8 @@ final class PhabricatorMailMailgunAdapter } $mailgun_uri = urisprintf( - 'https://api.mailgun.net/v2/%s/messages', + 'https://%s/v2/%s/messages', + $api_hostname, $domain); $future = id(new HTTPSFuture($mailgun_uri, $params)) diff --git a/src/applications/metamta/applicationpanel/PhabricatorMetaMTAApplicationEmailPanel.php b/src/applications/metamta/applicationpanel/PhabricatorMetaMTAApplicationEmailPanel.php index c13835e6f4..2f9ddcf22b 100644 --- a/src/applications/metamta/applicationpanel/PhabricatorMetaMTAApplicationEmailPanel.php +++ b/src/applications/metamta/applicationpanel/PhabricatorMetaMTAApplicationEmailPanel.php @@ -54,8 +54,7 @@ final class PhabricatorMetaMTAApplicationEmailPanel return new Aphront404Response(); } - $uri = $request->getRequestURI(); - $uri->setQueryParams(array()); + $uri = new PhutilURI($request->getPath()); $new = $request->getStr('new'); $edit = $request->getInt('edit'); diff --git a/src/applications/multimeter/controller/MultimeterSampleController.php b/src/applications/multimeter/controller/MultimeterSampleController.php index da09641d22..73c9ba530e 100644 --- a/src/applications/multimeter/controller/MultimeterSampleController.php +++ b/src/applications/multimeter/controller/MultimeterSampleController.php @@ -302,11 +302,11 @@ final class MultimeterSampleController extends MultimeterController { if (!strlen($group)) { $group = null; } - $uri->setQueryParam('group', $group); + $uri->replaceQueryParam('group', $group); if ($wipe) { foreach ($this->getColumnMap() as $key => $column) { - $uri->setQueryParam($key, null); + $uri->removeQueryParam($key); } } @@ -317,7 +317,7 @@ final class MultimeterSampleController extends MultimeterController { $value = (array)$value; $uri = clone $this->getRequest()->getRequestURI(); - $uri->setQueryParam($key, implode(',', $value)); + $uri->replaceQueryParam($key, implode(',', $value)); return phutil_tag( 'a', diff --git a/src/applications/notification/client/PhabricatorNotificationServerRef.php b/src/applications/notification/client/PhabricatorNotificationServerRef.php index b183221eee..46d03a5c3a 100644 --- a/src/applications/notification/client/PhabricatorNotificationServerRef.php +++ b/src/applications/notification/client/PhabricatorNotificationServerRef.php @@ -153,7 +153,7 @@ final class PhabricatorNotificationServerRef $instance = PhabricatorEnv::getEnvConfig('cluster.instance'); if (strlen($instance)) { - $uri->setQueryParam('instance', $instance); + $uri->replaceQueryParam('instance', $instance); } return $uri; diff --git a/src/applications/notification/controller/PhabricatorNotificationPanelController.php b/src/applications/notification/controller/PhabricatorNotificationPanelController.php index 1e956a60ea..5991e8db7b 100644 --- a/src/applications/notification/controller/PhabricatorNotificationPanelController.php +++ b/src/applications/notification/controller/PhabricatorNotificationPanelController.php @@ -25,7 +25,7 @@ final class PhabricatorNotificationPanelController $notifications_view = $builder->buildView(); $content = $notifications_view->render(); - $clear_uri->setQueryParam( + $clear_uri->replaceQueryParam( 'chronoKey', head($stories)->getChronologicalKey()); } else { diff --git a/src/applications/notification/query/PhabricatorNotificationSearchEngine.php b/src/applications/notification/query/PhabricatorNotificationSearchEngine.php index 0ee7327bfc..c7e1998333 100644 --- a/src/applications/notification/query/PhabricatorNotificationSearchEngine.php +++ b/src/applications/notification/query/PhabricatorNotificationSearchEngine.php @@ -111,7 +111,7 @@ final class PhabricatorNotificationSearchEngine ->setUser($viewer); $view = $builder->buildView(); - $clear_uri->setQueryParam( + $clear_uri->replaceQueryParam( 'chronoKey', head($notifications)->getChronologicalKey()); } else { diff --git a/src/applications/oauthserver/PhabricatorOAuthResponse.php b/src/applications/oauthserver/PhabricatorOAuthResponse.php index 62c0fc9821..e0fca827b4 100644 --- a/src/applications/oauthserver/PhabricatorOAuthResponse.php +++ b/src/applications/oauthserver/PhabricatorOAuthResponse.php @@ -36,7 +36,7 @@ final class PhabricatorOAuthResponse extends AphrontResponse { $base_uri = $this->getClientURI(); $query_params = $this->buildResponseDict(); foreach ($query_params as $key => $value) { - $base_uri->setQueryParam($key, $value); + $base_uri->replaceQueryParam($key, $value); } return $base_uri; } diff --git a/src/applications/oauthserver/PhabricatorOAuthServer.php b/src/applications/oauthserver/PhabricatorOAuthServer.php index f5c074f4eb..889e960213 100644 --- a/src/applications/oauthserver/PhabricatorOAuthServer.php +++ b/src/applications/oauthserver/PhabricatorOAuthServer.php @@ -256,8 +256,8 @@ final class PhabricatorOAuthServer extends Phobject { // Any query parameters present in the first URI must be exactly present // in the second URI. - $need_params = $primary_uri->getQueryParams(); - $have_params = $secondary_uri->getQueryParams(); + $need_params = $primary_uri->getQueryParamsAsMap(); + $have_params = $secondary_uri->getQueryParamsAsMap(); foreach ($need_params as $key => $value) { if (!array_key_exists($key, $have_params)) { diff --git a/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php b/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php index 745be3e820..2b454e00ef 100644 --- a/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php +++ b/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php @@ -306,7 +306,7 @@ final class PhabricatorOAuthServerAuthController foreach ($params as $key => $value) { if (strlen($value)) { - $full_uri->setQueryParam($key, $value); + $full_uri->replaceQueryParam($key, $value); } } diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php index e28ae2b3bb..c458e4dbd1 100644 --- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php +++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php @@ -65,11 +65,11 @@ final class PhabricatorOwnersDetailController $commit_views = array(); - $commit_uri = id(new PhutilURI('/diffusion/commit/')) - ->setQueryParams( - array( - 'package' => $package->getPHID(), - )); + $params = array( + 'package' => $package->getPHID(), + ); + + $commit_uri = new PhutilURI('/diffusion/commit/', $params); $status_concern = DiffusionCommitAuditStatus::CONCERN_RAISED; diff --git a/src/applications/people/application/PhabricatorPeopleApplication.php b/src/applications/people/application/PhabricatorPeopleApplication.php index 06740be26e..9cc3607930 100644 --- a/src/applications/people/application/PhabricatorPeopleApplication.php +++ b/src/applications/people/application/PhabricatorPeopleApplication.php @@ -63,7 +63,6 @@ final class PhabricatorPeopleApplication extends PhabricatorApplication { 'welcome/(?P[1-9]\d*)/' => 'PhabricatorPeopleWelcomeController', 'create/' => 'PhabricatorPeopleCreateController', 'new/(?P[^/]+)/' => 'PhabricatorPeopleNewController', - 'ldap/' => 'PhabricatorPeopleLdapController', 'editprofile/(?P[1-9]\d*)/' => 'PhabricatorPeopleProfileEditController', 'badges/(?P[1-9]\d*)/' => diff --git a/src/applications/people/controller/PhabricatorPeopleController.php b/src/applications/people/controller/PhabricatorPeopleController.php index e3b60eff2b..c2c262f9f4 100644 --- a/src/applications/people/controller/PhabricatorPeopleController.php +++ b/src/applications/people/controller/PhabricatorPeopleController.php @@ -28,10 +28,6 @@ abstract class PhabricatorPeopleController extends PhabricatorController { if ($viewer->getIsAdmin()) { $nav->addLabel(pht('User Administration')); - if (PhabricatorLDAPAuthProvider::getLDAPProvider()) { - $nav->addFilter('ldap', pht('Import from LDAP')); - } - $nav->addFilter('logs', pht('Activity Logs')); $nav->addFilter('invite', pht('Email Invitations')); } diff --git a/src/applications/people/controller/PhabricatorPeopleLdapController.php b/src/applications/people/controller/PhabricatorPeopleLdapController.php deleted file mode 100644 index 876bf986ad..0000000000 --- a/src/applications/people/controller/PhabricatorPeopleLdapController.php +++ /dev/null @@ -1,214 +0,0 @@ -requireApplicationCapability( - PeopleCreateUsersCapability::CAPABILITY); - $admin = $request->getUser(); - - $content = array(); - - $form = id(new AphrontFormView()) - ->setAction($request->getRequestURI() - ->alter('search', 'true')->alter('import', null)) - ->setUser($admin) - ->appendChild( - id(new AphrontFormTextControl()) - ->setLabel(pht('LDAP username')) - ->setName('username')) - ->appendChild( - id(new AphrontFormPasswordControl()) - ->setDisableAutocomplete(true) - ->setLabel(pht('Password')) - ->setName('password')) - ->appendChild( - id(new AphrontFormTextControl()) - ->setLabel(pht('LDAP query')) - ->setCaption(pht('A filter such as %s.', '(objectClass=*)')) - ->setName('query')) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->setValue(pht('Search'))); - - $panel = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Import LDAP Users')) - ->setForm($form); - - $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addTextCrumb( - pht('Import LDAP Users'), - $this->getApplicationURI('/ldap/')); - - $nav = $this->buildSideNavView(); - $nav->selectFilter('ldap'); - $nav->appendChild($content); - - if ($request->getStr('import')) { - $nav->appendChild($this->processImportRequest($request)); - } - - $nav->appendChild($panel); - - if ($request->getStr('search')) { - $nav->appendChild($this->processSearchRequest($request)); - } - - return $this->newPage() - ->setTitle(pht('Import LDAP Users')) - ->setCrumbs($crumbs) - ->setNavigation($nav); - } - - private function processImportRequest($request) { - $admin = $request->getUser(); - $usernames = $request->getArr('usernames'); - $emails = $request->getArr('email'); - $names = $request->getArr('name'); - - $notice_view = new PHUIInfoView(); - $notice_view->setSeverity(PHUIInfoView::SEVERITY_NOTICE); - $notice_view->setTitle(pht('Import Successful')); - $notice_view->setErrors(array( - pht('Successfully imported users from LDAP'), - )); - - $list = new PHUIObjectItemListView(); - $list->setNoDataString(pht('No users imported?')); - - foreach ($usernames as $username) { - $user = new PhabricatorUser(); - $user->setUsername($username); - $user->setRealname($names[$username]); - - $email_obj = id(new PhabricatorUserEmail()) - ->setAddress($emails[$username]) - ->setIsVerified(1); - try { - id(new PhabricatorUserEditor()) - ->setActor($admin) - ->createNewUser($user, $email_obj); - - id(new PhabricatorExternalAccount()) - ->setUserPHID($user->getPHID()) - ->setAccountType('ldap') - ->setAccountDomain('self') - ->setAccountID($username) - ->save(); - - $header = pht('Successfully added %s', $username); - $attribute = null; - $color = 'fa-check green'; - } catch (Exception $ex) { - $header = pht('Failed to add %s', $username); - $attribute = $ex->getMessage(); - $color = 'fa-times red'; - } - - $item = id(new PHUIObjectItemView()) - ->setHeader($header) - ->addAttribute($attribute) - ->setStatusIcon($color); - - $list->addItem($item); - } - - return array( - $notice_view, - $list, - ); - - } - - private function processSearchRequest($request) { - $panel = new PHUIBoxView(); - $admin = $request->getUser(); - - $search = $request->getStr('query'); - - $ldap_provider = PhabricatorLDAPAuthProvider::getLDAPProvider(); - if (!$ldap_provider) { - throw new Exception(pht('No LDAP provider enabled!')); - } - - $ldap_adapter = $ldap_provider->getAdapter(); - $ldap_adapter->setLoginUsername($request->getStr('username')); - $ldap_adapter->setLoginPassword( - new PhutilOpaqueEnvelope($request->getStr('password'))); - - // This causes us to connect and bind. - // TODO: Clean up this discard mode stuff. - DarkConsoleErrorLogPluginAPI::enableDiscardMode(); - $ldap_adapter->getAccountID(); - DarkConsoleErrorLogPluginAPI::disableDiscardMode(); - - $results = $ldap_adapter->searchLDAP('%Q', $search); - - foreach ($results as $key => $record) { - $account_id = $ldap_adapter->readLDAPRecordAccountID($record); - if (!$account_id) { - unset($results[$key]); - continue; - } - - $info = array( - $account_id, - $ldap_adapter->readLDAPRecordEmail($record), - $ldap_adapter->readLDAPRecordRealName($record), - ); - $results[$key] = $info; - $results[$key][] = $this->renderUserInputs($info); - } - - $form = id(new AphrontFormView()) - ->setUser($admin); - - $table = new AphrontTableView($results); - $table->setHeaders( - array( - pht('Username'), - pht('Email'), - pht('Real Name'), - pht('Import?'), - )); - $form->appendChild($table); - $form->setAction($request->getRequestURI() - ->alter('import', 'true')->alter('search', null)) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->setValue(pht('Import'))); - - $panel->appendChild($form); - - return $panel; - } - - private function renderUserInputs($user) { - $username = $user[0]; - return hsprintf( - '%s%s%s', - phutil_tag( - 'input', - array( - 'type' => 'checkbox', - 'name' => 'usernames[]', - 'value' => $username, - )), - phutil_tag( - 'input', - array( - 'type' => 'hidden', - 'name' => "email[$username]", - 'value' => $user[1], - )), - phutil_tag( - 'input', - array( - 'type' => 'hidden', - 'name' => "name[$username]", - 'value' => $user[2], - ))); - } - -} diff --git a/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php b/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php index 5cab924255..92bb2e0b86 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php @@ -157,13 +157,10 @@ final class PhabricatorPeopleProfilePictureController continue; } - $provider = PhabricatorAuthProvider::getEnabledProviderByKey( - $account->getProviderKey()); - if ($provider) { - $tip = pht('Picture From %s', $provider->getProviderName()); - } else { - $tip = pht('Picture From External Account'); - } + $config = $account->getProviderConfig(); + $provider = $config->getProvider(); + + $tip = pht('Picture From %s', $provider->getProviderName()); if ($file->isTransformableImage()) { $images[$file->getPHID()] = array( diff --git a/src/applications/people/storage/PhabricatorExternalAccount.php b/src/applications/people/storage/PhabricatorExternalAccount.php index 4bc0fcae98..bde9588339 100644 --- a/src/applications/people/storage/PhabricatorExternalAccount.php +++ b/src/applications/people/storage/PhabricatorExternalAccount.php @@ -16,8 +16,10 @@ final class PhabricatorExternalAccount extends PhabricatorUserDAO protected $accountURI; protected $profileImagePHID; protected $properties = array(); + protected $providerConfigPHID; private $profileImageFile = self::ATTACHABLE; + private $providerConfig = self::ATTACHABLE; public function getProfileImageFile() { return $this->assertAttached($this->profileImageFile); @@ -65,13 +67,6 @@ final class PhabricatorExternalAccount extends PhabricatorUserDAO ) + parent::getConfiguration(); } - public function getPhabricatorUser() { - $tmp_usr = id(new PhabricatorUser()) - ->makeEphemeral() - ->setPHID($this->getPHID()); - return $tmp_usr; - } - public function getProviderKey() { return $this->getAccountType().':'.$this->getAccountDomain(); } @@ -93,13 +88,12 @@ final class PhabricatorExternalAccount extends PhabricatorUserDAO } public function isUsableForLogin() { - $key = $this->getProviderKey(); - $provider = PhabricatorAuthProvider::getEnabledProviderByKey($key); - - if (!$provider) { + $config = $this->getProviderConfig(); + if (!$config->getIsEnabled()) { return false; } + $provider = $config->getProvider(); if (!$provider->shouldAllowLogin()) { return false; } @@ -125,6 +119,14 @@ final class PhabricatorExternalAccount extends PhabricatorUserDAO return idx($map, $type, pht('"%s" User', $type)); } + public function attachProviderConfig(PhabricatorAuthProviderConfig $config) { + $this->providerConfig = $config; + return $this; + } + + public function getProviderConfig() { + return $this->assertAttached($this->providerConfig); + } /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index 055df8b79e..c675878747 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -557,7 +557,7 @@ final class PhabricatorUser public static function describeValidUsername() { return pht( - 'Usernames must contain only numbers, letters, period, underscore and '. + 'Usernames must contain only numbers, letters, period, underscore, and '. 'hyphen, and can not end with a period. They must have no more than %d '. 'characters.', new PhutilNumber(self::MAXIMUM_USERNAME_LENGTH)); diff --git a/src/applications/people/storage/PhabricatorUserEmail.php b/src/applications/people/storage/PhabricatorUserEmail.php index 42946015de..572c7d6e8b 100644 --- a/src/applications/people/storage/PhabricatorUserEmail.php +++ b/src/applications/people/storage/PhabricatorUserEmail.php @@ -83,9 +83,8 @@ final class PhabricatorUserEmail extends PhabricatorUserDAO { */ public static function describeValidAddresses() { return pht( - "Email addresses should be in the form '%s'. The maximum ". - "length of an email address is %s character(s).", - 'user@domain.com', + 'Email addresses should be in the form "user@domain.com". The maximum '. + 'length of an email address is %s characters.', new PhutilNumber(self::MAX_ADDRESS_LENGTH)); } diff --git a/src/applications/pholio/view/PholioMockImagesView.php b/src/applications/pholio/view/PholioMockImagesView.php index 99645c4a91..786de07cfd 100644 --- a/src/applications/pholio/view/PholioMockImagesView.php +++ b/src/applications/pholio/view/PholioMockImagesView.php @@ -133,7 +133,7 @@ final class PholioMockImagesView extends AphrontView { ); $login_uri = id(new PhutilURI('/login/')) - ->setQueryParam('next', (string)$this->getRequestURI()); + ->replaceQueryParam('next', (string)$this->getRequestURI()); $config = array( 'mockID' => $mock->getID(), diff --git a/src/applications/phortune/action/PhortuneAddPaymentMethodAction.php b/src/applications/phortune/action/PhortuneAddPaymentMethodAction.php new file mode 100644 index 0000000000..09a8cd2f5d --- /dev/null +++ b/src/applications/phortune/action/PhortuneAddPaymentMethodAction.php @@ -0,0 +1,22 @@ +getID(); + $params = array( + 'merchantID' => $merchant->getID(), + 'cartID' => $cart->getID(), + ); + $payment_method_uri = $this->getApplicationURI("{$account_id}/card/new/"); - $payment_method_uri = new PhutilURI($payment_method_uri); - $payment_method_uri->setQueryParams( - array( - 'merchantID' => $merchant->getID(), - 'cartID' => $cart->getID(), - )); + $payment_method_uri = new PhutilURI($payment_method_uri, $params); $form = id(new AphrontFormView()) ->setUser($viewer) diff --git a/src/applications/phortune/controller/payment/PhortunePaymentMethodCreateController.php b/src/applications/phortune/controller/payment/PhortunePaymentMethodCreateController.php index 521508e0e8..c068862631 100644 --- a/src/applications/phortune/controller/payment/PhortunePaymentMethodCreateController.php +++ b/src/applications/phortune/controller/payment/PhortunePaymentMethodCreateController.php @@ -82,6 +82,15 @@ final class PhortunePaymentMethodCreateController ->setProviderPHID($provider->getProviderConfig()->getPHID()) ->setStatus(PhortunePaymentMethod::STATUS_ACTIVE); + // Limit the rate at which you can attempt to add payment methods. This + // is intended as a line of defense against using Phortune to validate a + // large list of stolen credit card numbers. + + PhabricatorSystemActionEngine::willTakeAction( + array($viewer->getPHID()), + new PhortuneAddPaymentMethodAction(), + 1); + if (!$errors) { $errors = $this->processClientErrors( $provider, @@ -134,7 +143,7 @@ final class PhortunePaymentMethodCreateController "cart/{$cart_id}/checkout/?paymentMethodID=".$method->getID()); } else if ($subscription_id) { $next_uri = new PhutilURI($cancel_uri); - $next_uri->setQueryParam('added', true); + $next_uri->replaceQueryParam('added', true); } else { $account_uri = $this->getApplicationURI($account->getID().'/'); $next_uri = new PhutilURI($account_uri); diff --git a/src/applications/phortune/controller/subscription/PhortuneSubscriptionEditController.php b/src/applications/phortune/controller/subscription/PhortuneSubscriptionEditController.php index e7287f3d29..04367a88a0 100644 --- a/src/applications/phortune/controller/subscription/PhortuneSubscriptionEditController.php +++ b/src/applications/phortune/controller/subscription/PhortuneSubscriptionEditController.php @@ -118,8 +118,8 @@ final class PhortuneSubscriptionEditController extends PhortuneController { $uri = $this->getApplicationURI($account->getID().'/card/new/'); $uri = new PhutilURI($uri); - $uri->setQueryParam('merchantID', $merchant->getID()); - $uri->setQueryParam('subscriptionID', $subscription->getID()); + $uri->replaceQueryParam('merchantID', $merchant->getID()); + $uri->replaceQueryParam('subscriptionID', $subscription->getID()); $add_method_button = phutil_tag( 'a', diff --git a/src/applications/phortune/provider/PhortunePayPalPaymentProvider.php b/src/applications/phortune/provider/PhortunePayPalPaymentProvider.php index 078141f8a1..262606ca62 100644 --- a/src/applications/phortune/provider/PhortunePayPalPaymentProvider.php +++ b/src/applications/phortune/provider/PhortunePayPalPaymentProvider.php @@ -348,12 +348,14 @@ final class PhortunePayPalPaymentProvider extends PhortunePaymentProvider { ->setRawPayPalQuery('SetExpressCheckout', $params) ->resolve(); - $uri = new PhutilURI('https://www.sandbox.paypal.com/cgi-bin/webscr'); - $uri->setQueryParams( - array( - 'cmd' => '_express-checkout', - 'token' => $result['TOKEN'], - )); + $params = array( + 'cmd' => '_express-checkout', + 'token' => $result['TOKEN'], + ); + + $uri = new PhutilURI( + 'https://www.sandbox.paypal.com/cgi-bin/webscr', + $params); $cart->setMetadataValue('provider.checkoutURI', (string)$uri); $cart->save(); diff --git a/src/applications/phortune/provider/PhortunePaymentProvider.php b/src/applications/phortune/provider/PhortunePaymentProvider.php index 90e354c5dc..57b2956ecb 100644 --- a/src/applications/phortune/provider/PhortunePaymentProvider.php +++ b/src/applications/phortune/provider/PhortunePaymentProvider.php @@ -273,8 +273,7 @@ abstract class PhortunePaymentProvider extends Phobject { $app = PhabricatorApplication::getByClass('PhabricatorPhortuneApplication'); $path = $app->getBaseURI().'provider/'.$id.'/'.$action.'/'; - $uri = new PhutilURI($path); - $uri->setQueryParams($params); + $uri = new PhutilURI($path, $params); if ($local) { return $uri; diff --git a/src/applications/phriction/editor/PhrictionTransactionEditor.php b/src/applications/phriction/editor/PhrictionTransactionEditor.php index d09ff5d556..1476b24c46 100644 --- a/src/applications/phriction/editor/PhrictionTransactionEditor.php +++ b/src/applications/phriction/editor/PhrictionTransactionEditor.php @@ -229,9 +229,14 @@ final class PhrictionTransactionEditor foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case PhrictionDocumentContentTransaction::TRANSACTIONTYPE: - $uri = id(new PhutilURI('/phriction/diff/'.$object->getID().'/')) - ->alter('l', $this->getOldContent()->getVersion()) - ->alter('r', $this->getNewContent()->getVersion()); + $params = array( + 'l' => $this->getOldContent()->getVersion(), + 'r' => $this->getNewContent()->getVersion(), + ); + + $path = '/phriction/diff/'.$object->getID().'/'; + $uri = new PhutilURI($path, $params); + $this->contentDiffURI = (string)$uri; break 2; default: diff --git a/src/applications/ponder/view/PonderAddAnswerView.php b/src/applications/ponder/view/PonderAddAnswerView.php index 43bfd0d6ba..20c52dac8e 100644 --- a/src/applications/ponder/view/PonderAddAnswerView.php +++ b/src/applications/ponder/view/PonderAddAnswerView.php @@ -66,7 +66,7 @@ final class PonderAddAnswerView extends AphrontView { if (!$viewer->isLoggedIn()) { $login_href = id(new PhutilURI('/auth/start/')) - ->setQueryParam('next', '/Q'.$question->getID()); + ->replaceQueryParam('next', '/Q'.$question->getID()); $form = id(new PHUIFormLayoutView()) ->addClass('login-to-participate') ->appendChild( diff --git a/src/applications/project/controller/PhabricatorProjectBoardViewController.php b/src/applications/project/controller/PhabricatorProjectBoardViewController.php index 15e0c5d075..ee239035da 100644 --- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php +++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php @@ -284,7 +284,7 @@ final class PhabricatorProjectBoardViewController $query_key = $saved_query->getQueryKey(); $bulk_uri = new PhutilURI("/maniphest/bulk/query/{$query_key}/"); - $bulk_uri->setQueryParam('board', $this->id); + $bulk_uri->replaceQueryParam('board', $this->id); return id(new AphrontRedirectResponse()) ->setURI($bulk_uri); @@ -878,7 +878,7 @@ final class PhabricatorProjectBoardViewController } $uri = $this->getURIWithState($uri) - ->setQueryParam('filter', null); + ->removeQueryParam('filter'); $item->setHref($uri); $items[] = $item; @@ -966,12 +966,12 @@ final class PhabricatorProjectBoardViewController if ($show_hidden) { $hidden_uri = $this->getURIWithState() - ->setQueryParam('hidden', null); + ->removeQueryParam('hidden'); $hidden_icon = 'fa-eye-slash'; $hidden_text = pht('Hide Hidden Columns'); } else { $hidden_uri = $this->getURIWithState() - ->setQueryParam('hidden', 'true'); + ->replaceQueryParam('hidden', 'true'); $hidden_icon = 'fa-eye'; $hidden_text = pht('Show Hidden Columns'); } @@ -999,7 +999,7 @@ final class PhabricatorProjectBoardViewController ->setHref($manage_uri); $batch_edit_uri = $request->getRequestURI(); - $batch_edit_uri->setQueryParam('batch', self::BATCH_EDIT_ALL); + $batch_edit_uri->replaceQueryParam('batch', self::BATCH_EDIT_ALL); $can_batch_edit = PhabricatorPolicyFilter::hasCapability( $viewer, PhabricatorApplication::getByClass('PhabricatorManiphestApplication'), @@ -1090,7 +1090,7 @@ final class PhabricatorProjectBoardViewController } $batch_edit_uri = $request->getRequestURI(); - $batch_edit_uri->setQueryParam('batch', $column->getID()); + $batch_edit_uri->replaceQueryParam('batch', $column->getID()); $can_batch_edit = PhabricatorPolicyFilter::hasCapability( $viewer, PhabricatorApplication::getByClass('PhabricatorManiphestApplication'), @@ -1103,7 +1103,7 @@ final class PhabricatorProjectBoardViewController ->setDisabled(!$can_batch_edit); $batch_move_uri = $request->getRequestURI(); - $batch_move_uri->setQueryParam('move', $column->getID()); + $batch_move_uri->replaceQueryParam('move', $column->getID()); $column_items[] = id(new PhabricatorActionView()) ->setIcon('fa-arrow-right') ->setName(pht('Move Tasks to Column...')) @@ -1111,7 +1111,7 @@ final class PhabricatorProjectBoardViewController ->setWorkflow(true); $query_uri = $request->getRequestURI(); - $query_uri->setQueryParam('queryColumnID', $column->getID()); + $query_uri->replaceQueryParam('queryColumnID', $column->getID()); $column_items[] = id(new PhabricatorActionView()) ->setName(pht('View as Query')) @@ -1188,18 +1188,22 @@ final class PhabricatorProjectBoardViewController $base = new PhutilURI($base); if ($force || ($this->sortKey != $this->getDefaultSort($project))) { - $base->setQueryParam('order', $this->sortKey); + $base->replaceQueryParam('order', $this->sortKey); } else { - $base->setQueryParam('order', null); + $base->removeQueryParam('order'); } if ($force || ($this->queryKey != $this->getDefaultFilter($project))) { - $base->setQueryParam('filter', $this->queryKey); + $base->replaceQueryParam('filter', $this->queryKey); } else { - $base->setQueryParam('filter', null); + $base->removeQueryParam('filter'); } - $base->setQueryParam('hidden', $this->showHidden ? 'true' : null); + if ($this->showHidden) { + $base->replaceQueryParam('hidden', 'true'); + } else { + $base->removeQueryParam('hidden'); + } return $base; } diff --git a/src/applications/project/controller/PhabricatorProjectColumnHideController.php b/src/applications/project/controller/PhabricatorProjectColumnHideController.php index 1dd5e47ecb..fbda2feb1e 100644 --- a/src/applications/project/controller/PhabricatorProjectColumnHideController.php +++ b/src/applications/project/controller/PhabricatorProjectColumnHideController.php @@ -41,7 +41,7 @@ final class PhabricatorProjectColumnHideController $view_uri = $this->getApplicationURI('/board/'.$project_id.'/'); $view_uri = new PhutilURI($view_uri); foreach ($request->getPassthroughRequestData() as $key => $value) { - $view_uri->setQueryParam($key, $value); + $view_uri->replaceQueryParam($key, $value); } if ($column->isDefaultColumn()) { diff --git a/src/applications/project/controller/PhabricatorProjectDefaultController.php b/src/applications/project/controller/PhabricatorProjectDefaultController.php index 8f42ff9736..2c7a47b2df 100644 --- a/src/applications/project/controller/PhabricatorProjectDefaultController.php +++ b/src/applications/project/controller/PhabricatorProjectDefaultController.php @@ -54,7 +54,7 @@ final class PhabricatorProjectDefaultController $view_uri = $this->getApplicationURI("board/{$id}/"); $view_uri = new PhutilURI($view_uri); foreach ($request->getPassthroughRequestData() as $key => $value) { - $view_uri->setQueryParam($key, $value); + $view_uri->replaceQueryParam($key, $value); } if ($request->isFormPost()) { diff --git a/src/applications/releeph/controller/request/ReleephRequestDifferentialCreateController.php b/src/applications/releeph/controller/request/ReleephRequestDifferentialCreateController.php index ffd6388284..1835e6f7f9 100644 --- a/src/applications/releeph/controller/request/ReleephRequestDifferentialCreateController.php +++ b/src/applications/releeph/controller/request/ReleephRequestDifferentialCreateController.php @@ -96,7 +96,7 @@ final class ReleephRequestDifferentialCreateController private function buildReleephRequestURI(ReleephBranch $branch) { $uri = $branch->getURI('request/'); return id(new PhutilURI($uri)) - ->setQueryParam('D', $this->revision->getID()); + ->replaceQueryParam('D', $this->revision->getID()); } } diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index e66fb78afa..d5ad83b6d6 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -820,8 +820,6 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return $uri; } - $uri = new PhutilURI($uri); - if (isset($params['lint'])) { $params['params'] = idx($params, 'params', array()) + array( 'lint' => $params['lint'], @@ -830,11 +828,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO $query = idx($params, 'params', array()) + $query; - if ($query) { - $uri->setQueryParams($query); - } - - return $uri; + return new PhutilURI($uri, $query); } public function updateURIIndex() { diff --git a/src/applications/search/controller/PhabricatorApplicationSearchController.php b/src/applications/search/controller/PhabricatorApplicationSearchController.php index cf4f95c16d..6b43883c78 100644 --- a/src/applications/search/controller/PhabricatorApplicationSearchController.php +++ b/src/applications/search/controller/PhabricatorApplicationSearchController.php @@ -905,7 +905,7 @@ final class PhabricatorApplicationSearchController $engine = $this->getSearchEngine(); $nux_uri = $engine->getQueryBaseURI(); $nux_uri = id(new PhutilURI($nux_uri)) - ->setQueryParam('nux', true); + ->replaceQueryParam('nux', true); $actions[] = id(new PhabricatorActionView()) ->setIcon('fa-user-plus') @@ -915,7 +915,7 @@ final class PhabricatorApplicationSearchController if ($is_dev) { $overheated_uri = $this->getRequest()->getRequestURI() - ->setQueryParam('overheated', true); + ->replaceQueryParam('overheated', true); $actions[] = id(new PhabricatorActionView()) ->setIcon('fa-fire') diff --git a/src/applications/search/worker/PhabricatorSearchWorker.php b/src/applications/search/worker/PhabricatorSearchWorker.php index f93df63981..361d9c9b6f 100644 --- a/src/applications/search/worker/PhabricatorSearchWorker.php +++ b/src/applications/search/worker/PhabricatorSearchWorker.php @@ -14,7 +14,7 @@ final class PhabricatorSearchWorker extends PhabricatorWorker { 'parameters' => $parameters, ), array( - 'priority' => parent::PRIORITY_IMPORT, + 'priority' => parent::PRIORITY_INDEX, 'objectPHID' => $phid, )); } diff --git a/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php b/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php index 1b69adcd62..8f7f633e7e 100644 --- a/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php @@ -31,8 +31,7 @@ final class PhabricatorEmailAddressesSettingsPanel $user = $this->getUser(); $editable = PhabricatorEnv::getEnvConfig('account.editable'); - $uri = $request->getRequestURI(); - $uri->setQueryParams(array()); + $uri = new PhutilURI($request->getPath()); if ($editable) { $new = $request->getStr('new'); diff --git a/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php b/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php index 29ef9fa2c7..60389e1590 100644 --- a/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php @@ -44,17 +44,13 @@ final class PhabricatorExternalAccountsSettingsPanel foreach ($accounts as $account) { $item = new PHUIObjectItemView(); - $provider = idx($providers, $account->getProviderKey()); - if ($provider) { - $item->setHeader($provider->getProviderName()); - $can_unlink = $provider->shouldAllowAccountUnlink(); - if (!$can_unlink) { - $item->addAttribute(pht('Permanently Linked')); - } - } else { - $item->setHeader( - pht('Unknown Account ("%s")', $account->getProviderKey())); - $can_unlink = true; + $config = $account->getProviderConfig(); + $provider = $config->getProvider(); + + $item->setHeader($provider->getProviderName()); + $can_unlink = $provider->shouldAllowAccountUnlink(); + if (!$can_unlink) { + $item->addAttribute(pht('Permanently Linked')); } $can_login = $account->isUsableForLogin(); @@ -65,12 +61,12 @@ final class PhabricatorExternalAccountsSettingsPanel 'account provider).')); } - $can_refresh = $provider && $provider->shouldAllowAccountRefresh(); + $can_refresh = $provider->shouldAllowAccountRefresh(); if ($can_refresh) { $item->addAction( id(new PHUIListItemView()) ->setIcon('fa-refresh') - ->setHref('/auth/refresh/'.$account->getProviderKey().'/')); + ->setHref('/auth/refresh/'.$config->getID().'/')); } $item->addAction( @@ -78,7 +74,7 @@ final class PhabricatorExternalAccountsSettingsPanel ->setIcon('fa-times') ->setWorkflow(true) ->setDisabled(!$can_unlink) - ->setHref('/auth/unlink/'.$account->getProviderKey().'/')); + ->setHref('/auth/unlink/'.$account->getID().'/')); if ($provider) { $provider->willRenderLinkedAccount($viewer, $item, $account); @@ -94,14 +90,15 @@ final class PhabricatorExternalAccountsSettingsPanel ->setNoDataString( pht('Your account is linked with all available providers.')); - $accounts = mpull($accounts, null, 'getProviderKey'); - $configs = id(new PhabricatorAuthProviderConfigQuery()) ->setViewer($viewer) ->withIsEnabled(true) ->execute(); $configs = msort($configs, 'getSortVector'); + $account_map = mgroup($accounts, 'getProviderConfigPHID'); + + foreach ($configs as $config) { $provider = $config->getProvider(); @@ -110,12 +107,11 @@ final class PhabricatorExternalAccountsSettingsPanel } // Don't show the user providers they already have linked. - $provider_key = $config->getProvider()->getProviderKey(); - if (isset($accounts[$provider_key])) { + if (isset($account_map[$config->getPHID()])) { continue; } - $link_uri = '/auth/link/'.$provider->getProviderKey().'/'; + $link_uri = '/auth/link/'.$config->getID().'/'; $link_button = id(new PHUIButtonView()) ->setTag('a') diff --git a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php index 6809b51334..09193f3c96 100644 --- a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php @@ -219,7 +219,7 @@ final class PhabricatorMultiFactorSettingsPanel foreach ($providers as $provider_phid => $provider) { $provider_uri = id(new PhutilURI($this->getPanelURI())) - ->setQueryParam('providerPHID', $provider_phid); + ->replaceQueryParam('providerPHID', $provider_phid); $is_enabled = $provider->canCreateNewConfiguration($viewer); diff --git a/src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php b/src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php index 37393d5d4f..77f32f977d 100644 --- a/src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php @@ -34,11 +34,6 @@ final class PhabricatorPasswordSettingsPanel extends PhabricatorSettingsPanel { $content_source = PhabricatorContentSource::newFromRequest($request); - $token = id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession( - $viewer, - $request, - '/settings/'); - $min_len = PhabricatorEnv::getEnvConfig('account.minimum-password-length'); $min_len = (int)$min_len; @@ -55,20 +50,25 @@ final class PhabricatorPasswordSettingsPanel extends PhabricatorSettingsPanel { ->withPasswordTypes(array($account_type)) ->withIsRevoked(false) ->execute(); - if ($password_objects) { - $password_object = head($password_objects); - } else { - $password_object = PhabricatorAuthPassword::initializeNewPassword( - $user, - $account_type); + if (!$password_objects) { + return $this->newSetPasswordView($request); } + $password_object = head($password_objects); $e_old = true; $e_new = true; $e_conf = true; $errors = array(); - if ($request->isFormPost()) { + if ($request->isFormOrHisecPost()) { + $workflow_key = sprintf( + 'password.change(%s)', + $user->getPHID()); + + $hisec_token = id(new PhabricatorAuthSessionEngine()) + ->setWorkflowKey($workflow_key) + ->requireHighSecurityToken($viewer, $request, '/settings/'); + // Rate limit guesses about the old password. This page requires MFA and // session compromise already, so this is mostly just to stop researchers // from reporting this as a vulnerability. @@ -218,5 +218,27 @@ final class PhabricatorPasswordSettingsPanel extends PhabricatorSettingsPanel { ); } + private function newSetPasswordView(AphrontRequest $request) { + $viewer = $request->getUser(); + $user = $this->getUser(); + + $form = id(new AphrontFormView()) + ->setViewer($viewer) + ->appendRemarkupInstructions( + pht( + 'Your account does not currently have a password set. You can '. + 'choose a password by performing a password reset.')) + ->appendControl( + id(new AphrontFormSubmitControl()) + ->addCancelButton('/login/email/', pht('Reset Password'))); + + $form_box = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Set Password')) + ->setBackground(PHUIObjectBoxView::WHITE_CONFIG) + ->setForm($form); + + return $form_box; + } + } diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index feb783e724..353d7ee385 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -1279,8 +1279,7 @@ abstract class PhabricatorEditEngine $fields = $this->willBuildEditForm($object, $fields); - $request_path = $request->getRequestURI() - ->setQueryParams(array()); + $request_path = $request->getPath(); $form = id(new AphrontFormView()) ->setUser($viewer) @@ -1542,8 +1541,7 @@ abstract class PhabricatorEditEngine $config_uri = $config->getCreateURI(); if ($parameters) { - $config_uri = (string)id(new PhutilURI($config_uri)) - ->setQueryParams($parameters); + $config_uri = (string)new PhutilURI($config_uri, $parameters); } $specs[] = array( diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php index f6a27d4bcd..115c7b950e 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php @@ -198,7 +198,7 @@ final class PhabricatorApplicationTransactionCommentView $viewer = $this->getViewer(); if (!$viewer->isLoggedIn()) { $uri = id(new PhutilURI('/login/')) - ->setQueryParam('next', (string)$this->getRequestURI()); + ->replaceQueryParam('next', (string)$this->getRequestURI()); return id(new PHUIObjectBoxView()) ->setFlush(true) ->appendChild( @@ -254,11 +254,12 @@ final class PhabricatorApplicationTransactionCommentView require_celerity_resource('phui-comment-form-css'); $image_uri = $viewer->getProfileImageURI(); - $image = phutil_tag( + $image = javelin_tag( 'div', array( 'style' => 'background-image: url('.$image_uri.')', - 'class' => 'phui-comment-image visual-only', + 'class' => 'phui-comment-image', + 'aural' => false, )); $wedge = phutil_tag( 'div', diff --git a/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php b/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php index 7c2205df46..efc9ea5f65 100644 --- a/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php +++ b/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php @@ -126,10 +126,10 @@ final class PhabricatorTypeaheadModularDatasourceController $results = array_slice($results, 0, $limit, $preserve_keys = true); if (($offset + (2 * $limit)) < $hard_limit) { $next_uri = id(new PhutilURI($request->getRequestURI())) - ->setQueryParam('offset', $offset + $limit) - ->setQueryParam('q', $query) - ->setQueryParam('raw', $raw_query) - ->setQueryParam('format', 'html'); + ->replaceQueryParam('offset', $offset + $limit) + ->replaceQueryParam('q', $query) + ->replaceQueryParam('raw', $raw_query) + ->replaceQueryParam('format', 'html'); $next_link = javelin_tag( 'a', @@ -248,7 +248,9 @@ final class PhabricatorTypeaheadModularDatasourceController $parameters = $source->getParameters(); if ($parameters) { $reference_uri = (string)id(new PhutilURI($reference_uri)) - ->setQueryParam('parameters', phutil_json_encode($parameters)); + ->replaceQueryParam( + 'parameters', + phutil_json_encode($parameters)); } $reference_link = phutil_tag( diff --git a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php index 196ad1b98b..e077d7a7ec 100644 --- a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php +++ b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php @@ -99,9 +99,9 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject { } public function getDatasourceURI() { - $uri = new PhutilURI('/typeahead/class/'.get_class($this).'/'); - $uri->setQueryParams($this->newURIParameters()); - return (string)$uri; + $params = $this->newURIParameters(); + $uri = new PhutilURI('/typeahead/class/'.get_class($this).'/', $params); + return phutil_string_cast($uri); } public function getBrowseURI() { @@ -109,9 +109,9 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject { return null; } - $uri = new PhutilURI('/typeahead/browse/'.get_class($this).'/'); - $uri->setQueryParams($this->newURIParameters()); - return (string)$uri; + $params = $this->newURIParameters(); + $uri = new PhutilURI('/typeahead/browse/'.get_class($this).'/', $params); + return phutil_string_cast($uri); } private function newURIParameters() { diff --git a/src/docs/user/configuration/configuration_locked.diviner b/src/docs/user/configuration/configuration_locked.diviner index 958124c381..f96adc2d82 100644 --- a/src/docs/user/configuration/configuration_locked.diviner +++ b/src/docs/user/configuration/configuration_locked.diviner @@ -111,6 +111,55 @@ phabricator/ $ ./bin/config set ``` +Locked Configuration With Database Values +========================================= + +You may receive a setup issue warning you that a locked configuration key has a +value set in the database. Most commonly, this is because: + + - In some earlier version of Phabricator, this configuration was not locked. + - In the past, you or some other administrator used the web UI to set a + value. This value was written to the database. + - In a later version of the software, the value became locked. + +When Phabricator was originally released, locked configuration did not yet +exist. Locked configuration was introduced later, and then configuration options +were gradually locked for a long time after that. + +In some cases the meaning of a value changed and it became possible to use it +to break an install or the configuration became a security risk. In other +cases, we identified an existing security risk or arrived at some other reason +to lock the value. + +Locking values was more common in the past, and it is now relatively rare for +an unlocked value to become locked: when new values are introduced, they are +generally locked or hidden appropriately. In most cases, this setup issue only +affects installs that have used Phabricator for a long time. + +At time of writing (February 2019), Phabricator currently respects these old +database values. However, some future version of Phabricator will refuse to +read locked configuration from the database, because this improves security if +an attacker manages to find a way to bypass restrictions on editing locked +configuration from the web UI. + +To clear this setup warning and avoid surprise behavioral changes in the future, +you should move these configuration values from the database to a local config +file. Usually, you'll do this by first copying the value from the database: + +``` +phabricator/ $ ./bin/config set +``` + +...and then removing the database value: + +``` +phabricator/ $ ./bin/config delete --database +``` + +See @{Configuration User Guide: Advanced Configuration} for some more detailed +discussion of different configuration sources. + + Next Steps ========== diff --git a/src/docs/user/configuration/configuring_accounts_and_registration.diviner b/src/docs/user/configuration/configuring_accounts_and_registration.diviner index 05d11b11f3..a56d7377cb 100644 --- a/src/docs/user/configuration/configuring_accounts_and_registration.diviner +++ b/src/docs/user/configuration/configuring_accounts_and_registration.diviner @@ -3,7 +3,8 @@ Describes how to configure user access to Phabricator. -= Overview = +Overview +======== Phabricator supports a number of login systems. You can enable or disable these systems to configure who can register for and access your install, and how users @@ -28,24 +29,37 @@ After you add a provider, you can link it to existing accounts (for example, associate an existing Phabricator account with a GitHub OAuth account) or users can use it to register new accounts (assuming you enable these options). -= Recovering Inaccessible Accounts = + +Recovering Inaccessible Accounts +================================ If you accidentally lock yourself out of Phabricator (for example, by disabling -all authentication providers), you can use the `bin/auth` -script to recover access to an account. To recover access, run: +all authentication providers), you can normally use the "send a login link" +action from the login screen to email yourself a login link and regain access +to your account. - phabricator/ $ ./bin/auth recover +If that isn't working (perhaps because you haven't configured email yet), you +can use the `bin/auth` script to recover access to an account. To recover +access, run: + +``` +phabricator/ $ ./bin/auth recover +``` ...where `` is the account username you want to recover access to. This will generate a link which will log you in as the specified user. -= Managing Accounts with the Web Console = + +Managing Accounts with the Web Console +====================================== To manage accounts from the web, login as an administrator account and go to `/people/` or click "People" on the homepage. Provided you're an admin, you'll see options to create or edit accounts. -= Manually Creating New Accounts = + +Manually Creating New Accounts +============================== There are two ways to manually create new accounts: via the web UI using the "People" application (this is easiest), or via the CLI using the @@ -60,7 +74,9 @@ the CLI. You can also use this script to make a user an administrator (if you accidentally remove your admin flag) or to create an administrative account. -= Next Steps = + +Next Steps +========== Continue by: diff --git a/src/docs/user/configuration/configuring_outbound_email.diviner b/src/docs/user/configuration/configuring_outbound_email.diviner index 6f5212680e..b77d761f80 100644 --- a/src/docs/user/configuration/configuring_outbound_email.diviner +++ b/src/docs/user/configuration/configuring_outbound_email.diviner @@ -227,6 +227,9 @@ To use this mailer, set `type` to `mailgun`, then configure these `options`: - `api-key`: Required string. Your Mailgun API key. - `domain`: Required string. Your Mailgun domain. + - `api-hostname`: Optional string. Defaults to "api.mailgun.net". If your + account is in another region (like the EU), you may need to specify a + different hostname. Consult the Mailgun documentation. Mailer: Amazon SES diff --git a/src/docs/user/userguide/audit.diviner b/src/docs/user/userguide/audit.diviner index 0d10867906..5223f6c969 100644 --- a/src/docs/user/userguide/audit.diviner +++ b/src/docs/user/userguide/audit.diviner @@ -175,16 +175,13 @@ You can use this command to forcibly delete requests which may have triggered incorrectly (for example, because a package or Herald rule was configured in an overbroad way). -After deleting audits, you may want to run `bin/audit synchronize` to -synchronize audit state. - **Synchronize Audit State**: Synchronize the audit state of commits to the current open audit requests with `bin/audit synchronize`. Normally, overall audit state is automatically kept up to date as changes are -made to an audit. However, if you delete audits or manually update the database -to make changes to audit request state, the state of corresponding commits may -no longer be correct. +made to an audit. However, if you manually update the database to make changes +to audit request state, the state of corresponding commits may no longer be +consistent. This command will update commits so their overall audit state reflects the cumulative state of their actual audit requests. diff --git a/src/infrastructure/daemon/workers/PhabricatorWorker.php b/src/infrastructure/daemon/workers/PhabricatorWorker.php index 1b9821b68d..f055544b7b 100644 --- a/src/infrastructure/daemon/workers/PhabricatorWorker.php +++ b/src/infrastructure/daemon/workers/PhabricatorWorker.php @@ -18,6 +18,7 @@ abstract class PhabricatorWorker extends Phobject { const PRIORITY_DEFAULT = 2000; const PRIORITY_COMMIT = 2500; const PRIORITY_BULK = 3000; + const PRIORITY_INDEX = 3500; const PRIORITY_IMPORT = 4000; /** diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php index 1236c620f1..ff6a3db04f 100644 --- a/src/infrastructure/env/PhabricatorEnv.php +++ b/src/infrastructure/env/PhabricatorEnv.php @@ -483,11 +483,17 @@ final class PhabricatorEnv extends Phobject { * @task read */ public static function getDoclink($resource, $type = 'article') { - $uri = new PhutilURI('https://secure.phabricator.com/diviner/find/'); - $uri->setQueryParam('name', $resource); - $uri->setQueryParam('type', $type); - $uri->setQueryParam('jump', true); - return (string)$uri; + $params = array( + 'name' => $resource, + 'type' => $type, + 'jump' => true, + ); + + $uri = new PhutilURI( + 'https://secure.phabricator.com/diviner/find/', + $params); + + return phutil_string_cast($uri); } diff --git a/src/infrastructure/markup/rule/PhabricatorYoutubeRemarkupRule.php b/src/infrastructure/markup/rule/PhabricatorYoutubeRemarkupRule.php index cbf322b2d9..9d79d223e0 100644 --- a/src/infrastructure/markup/rule/PhabricatorYoutubeRemarkupRule.php +++ b/src/infrastructure/markup/rule/PhabricatorYoutubeRemarkupRule.php @@ -18,12 +18,22 @@ final class PhabricatorYoutubeRemarkupRule extends PhutilRemarkupRule { return $text; } - $params = $uri->getQueryParams(); - $v_param = idx($params, 'v'); - if (!strlen($v_param)) { + $v_params = array(); + + $params = $uri->getQueryParamsAsPairList(); + foreach ($params as $pair) { + list($k, $v) = $pair; + if ($k === 'v') { + $v_params[] = $v; + } + } + + if (count($v_params) !== 1) { return $text; } + $v_param = head($v_params); + $text_mode = $this->getEngine()->isTextMode(); $mail_mode = $this->getEngine()->isHTMLMailMode(); diff --git a/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php b/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php index e47d701df9..99603da567 100644 --- a/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php +++ b/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php @@ -6,6 +6,7 @@ abstract class PhabricatorLiskDAO extends LiskDAO { private static $namespaceStack = array(); + private $forcedNamespace; const ATTACHABLE = ''; const CONFIG_APPLICATION_SERIALIZERS = 'phabricator/serializers'; @@ -47,6 +48,11 @@ abstract class PhabricatorLiskDAO extends LiskDAO { return $namespace; } + public function setForcedStorageNamespace($namespace) { + $this->forcedNamespace = $namespace; + return $this; + } + /** * @task config */ @@ -188,7 +194,13 @@ abstract class PhabricatorLiskDAO extends LiskDAO { abstract public function getApplicationName(); protected function getDatabaseName() { - return self::getStorageNamespace().'_'.$this->getApplicationName(); + if ($this->forcedNamespace) { + $namespace = $this->forcedNamespace; + } else { + $namespace = self::getStorageNamespace(); + } + + return $namespace.'_'.$this->getApplicationName(); } /** diff --git a/src/view/control/AphrontCursorPagerView.php b/src/view/control/AphrontCursorPagerView.php index 4608158481..cdb9562624 100644 --- a/src/view/control/AphrontCursorPagerView.php +++ b/src/view/control/AphrontCursorPagerView.php @@ -99,9 +99,9 @@ final class AphrontCursorPagerView extends AphrontView { return null; } - return $this->uri - ->alter('before', null) - ->alter('after', null); + return id(clone $this->uri) + ->removeQueryParam('after') + ->removeQueryParam('before'); } public function getPrevPageURI() { @@ -113,9 +113,9 @@ final class AphrontCursorPagerView extends AphrontView { return null; } - return $this->uri - ->alter('after', null) - ->alter('before', $this->prevPageID); + return id(clone $this->uri) + ->removeQueryParam('after') + ->replaceQueryParam('before', $this->prevPageID); } public function getNextPageURI() { @@ -127,9 +127,9 @@ final class AphrontCursorPagerView extends AphrontView { return null; } - return $this->uri - ->alter('after', $this->nextPageID) - ->alter('before', null); + return id(clone $this->uri) + ->replaceQueryParam('after', $this->nextPageID) + ->removeQueryParam('before'); } public function render() { diff --git a/src/view/form/control/PHUIFormTimerControl.php b/src/view/form/control/PHUIFormTimerControl.php index 7229d649e9..090de2c8e4 100644 --- a/src/view/form/control/PHUIFormTimerControl.php +++ b/src/view/form/control/PHUIFormTimerControl.php @@ -3,6 +3,7 @@ final class PHUIFormTimerControl extends AphrontFormControl { private $icon; + private $updateURI; public function setIcon(PHUIIconView $icon) { $this->icon = $icon; @@ -13,11 +14,24 @@ final class PHUIFormTimerControl extends AphrontFormControl { return $this->icon; } + public function setUpdateURI($update_uri) { + $this->updateURI = $update_uri; + return $this; + } + + public function getUpdateURI() { + return $this->updateURI; + } + protected function getCustomControlClass() { return 'phui-form-timer'; } protected function renderInput() { + return $this->newTimerView(); + } + + public function newTimerView() { $icon_cell = phutil_tag( 'td', array( @@ -34,7 +48,21 @@ final class PHUIFormTimerControl extends AphrontFormControl { $row = phutil_tag('tr', array(), array($icon_cell, $content_cell)); - return phutil_tag('table', array(), $row); + $node_id = null; + + $update_uri = $this->getUpdateURI(); + if ($update_uri) { + $node_id = celerity_generate_unique_node_id(); + + Javelin::initBehavior( + 'phui-timer-control', + array( + 'nodeID' => $node_id, + 'uri' => $update_uri, + )); + } + + return phutil_tag('table', array('id' => $node_id), $row); } } diff --git a/src/view/phui/PHUIHeadThingView.php b/src/view/phui/PHUIHeadThingView.php index ab2feee984..219ed28be0 100644 --- a/src/view/phui/PHUIHeadThingView.php +++ b/src/view/phui/PHUIHeadThingView.php @@ -52,12 +52,13 @@ final class PHUIHeadThingView extends AphrontTagView { protected function getTagContent() { - $image = phutil_tag( + $image = javelin_tag( 'a', array( - 'class' => 'phui-head-thing-image visual-only', + 'class' => 'phui-head-thing-image', 'style' => 'background-image: url('.$this->image.');', 'href' => $this->imageHref, + 'aural' => false, )); if ($this->image) { diff --git a/src/view/phui/PHUITimelineEventView.php b/src/view/phui/PHUITimelineEventView.php index 78a75a2063..5013611084 100644 --- a/src/view/phui/PHUITimelineEventView.php +++ b/src/view/phui/PHUITimelineEventView.php @@ -420,12 +420,13 @@ final class PHUITimelineEventView extends AphrontView { $image = null; $badges = null; if ($image_uri) { - $image = phutil_tag( + $image = javelin_tag( ($this->userHandle->getURI()) ? 'a' : 'div', array( 'style' => 'background-image: url('.$image_uri.')', - 'class' => 'phui-timeline-image visual-only', + 'class' => 'phui-timeline-image', 'href' => $this->userHandle->getURI(), + 'aural' => false, ), ''); if ($this->badges && $show_badges) { diff --git a/src/view/phui/PHUITimelineView.php b/src/view/phui/PHUITimelineView.php index d0e942f461..2e6d8298c8 100644 --- a/src/view/phui/PHUITimelineView.php +++ b/src/view/phui/PHUITimelineView.php @@ -154,8 +154,21 @@ final class PHUITimelineView extends AphrontView { } $uri = $this->getPager()->getNextPageURI(); - $uri->setQueryParam('quoteTargetID', $this->getQuoteTargetID()); - $uri->setQueryParam('quoteRef', $this->getQuoteRef()); + + $target_id = $this->getQuoteTargetID(); + if ($target_id === null) { + $uri->removeQueryParam('quoteTargetID'); + } else { + $uri->replaceQueryParam('quoteTargetID', $target_id); + } + + $quote_ref = $this->getQuoteRef(); + if ($quote_ref === null) { + $uri->removeQueryParam('quoteRef'); + } else { + $uri->replaceQueryParam('quoteRef', $quote_ref); + } + $events[] = javelin_tag( 'div', array( diff --git a/src/view/widget/AphrontStackTraceView.php b/src/view/widget/AphrontStackTraceView.php index 1d0616df3d..edb805af8f 100644 --- a/src/view/widget/AphrontStackTraceView.php +++ b/src/view/widget/AphrontStackTraceView.php @@ -10,7 +10,6 @@ final class AphrontStackTraceView extends AphrontView { } public function render() { - $user = $this->getUser(); $trace = $this->trace; $libraries = PhutilBootloader::getInstance()->getAllLibraries(); diff --git a/support/startup/PhabricatorStartup.php b/support/startup/PhabricatorStartup.php index 1bfb74d886..4c577ca20c 100644 --- a/support/startup/PhabricatorStartup.php +++ b/support/startup/PhabricatorStartup.php @@ -315,7 +315,7 @@ final class PhabricatorStartup { * * @param string Brief description of the exception context, like * `"Rendering Exception"`. - * @param Exception The exception itself. + * @param Throwable The exception itself. * @param bool True if it's okay to show the exception's stack trace * to the user. The trace will always be logged. * @return exit This method **does not return**. @@ -324,7 +324,7 @@ final class PhabricatorStartup { */ public static function didEncounterFatalException( $note, - Exception $ex, + $ex, $show_trace) { $message = '['.$note.'/'.get_class($ex).'] '.$ex->getMessage(); diff --git a/webroot/index.php b/webroot/index.php index 5c7d79bfa1..0014edfa2c 100644 --- a/webroot/index.php +++ b/webroot/index.php @@ -2,6 +2,7 @@ phabricator_startup(); +$fatal_exception = null; try { PhabricatorStartup::beginStartupPhase('libraries'); PhabricatorStartup::loadCoreLibraries(); @@ -12,25 +13,66 @@ try { PhabricatorStartup::beginStartupPhase('sink'); $sink = new AphrontPHPHTTPSink(); + // PHP introduced a "Throwable" interface in PHP 7 and began making more + // runtime errors throw as "Throwable" errors. This is generally good, but + // makes top-level exception handling that is compatible with both PHP 5 + // and PHP 7 a bit tricky. + + // In PHP 5, "Throwable" does not exist, so "catch (Throwable $ex)" catches + // nothing. + + // In PHP 7, various runtime conditions raise an Error which is a Throwable + // but NOT an Exception, so "catch (Exception $ex)" will not catch them. + + // To cover both cases, we "catch (Exception $ex)" to catch everything in + // PHP 5, and most things in PHP 7. Then, we "catch (Throwable $ex)" to catch + // everything else in PHP 7. For the most part, we only need to do this at + // the top level. + + $main_exception = null; try { PhabricatorStartup::beginStartupPhase('run'); AphrontApplicationConfiguration::runHTTPRequest($sink); } catch (Exception $ex) { + $main_exception = $ex; + } catch (Throwable $ex) { + $main_exception = $ex; + } + + if ($main_exception) { + $response_exception = null; try { $response = new AphrontUnhandledExceptionResponse(); - $response->setException($ex); + $response->setException($main_exception); + $response->setShowStackTraces($sink->getShowStackTraces()); PhabricatorStartup::endOutputCapture(); $sink->writeResponse($response); - } catch (Exception $response_exception) { - // If we hit a rendering exception, ignore it and throw the original - // exception. It is generally more interesting and more likely to be - // the root cause. - throw $ex; + } catch (Exception $ex) { + $response_exception = $ex; + } catch (Throwable $ex) { + $response_exception = $ex; + } + + // If we hit a rendering exception, ignore it and throw the original + // exception. It is generally more interesting and more likely to be + // the root cause. + + if ($response_exception) { + throw $main_exception; } } } catch (Exception $ex) { - PhabricatorStartup::didEncounterFatalException('Core Exception', $ex, false); + $fatal_exception = $ex; +} catch (Throwable $ex) { + $fatal_exception = $ex; +} + +if ($fatal_exception) { + PhabricatorStartup::didEncounterFatalException( + 'Core Exception', + $fatal_exception, + false); } function phabricator_startup() { diff --git a/webroot/rsrc/css/aphront/table-view.css b/webroot/rsrc/css/aphront/table-view.css index 9bc8536054..a08674cf14 100644 --- a/webroot/rsrc/css/aphront/table-view.css +++ b/webroot/rsrc/css/aphront/table-view.css @@ -327,3 +327,40 @@ span.single-display-line-content { .phui-object-box .aphront-table-view { border: none; } + +.object-graph-header { + padding: 8px 12px; + overflow: hidden; + background: {$lightyellow}; + border-bottom: 1px solid {$lightblueborder}; + vertical-align: middle; +} + +.object-graph-header .object-graph-header-icon { + float: left; + margin-top: 10px; +} + +.object-graph-header a.button { + float: right; +} + +.object-graph-header-message { + margin: 8px 200px 8px 20px; +} + +.device .object-graph-header .object-graph-header-icon { + display: none; +} + +.device .object-graph-header-message { + clear: both; + margin: 0; +} + +.device .object-graph-header a.button { + margin: 0 auto 12px; + display: block; + width: 180px; + float: none; +} diff --git a/webroot/rsrc/css/application/config/unhandled-exception.css b/webroot/rsrc/css/application/config/unhandled-exception.css index 831148cdad..cd8ad313dc 100644 --- a/webroot/rsrc/css/application/config/unhandled-exception.css +++ b/webroot/rsrc/css/application/config/unhandled-exception.css @@ -8,12 +8,12 @@ background: #fff; border: 1px solid #c0392b; border-radius: 3px; - padding: 0 8px; + padding: 8px; } .unhandled-exception-detail .unhandled-exception-title { color: #c0392b; - padding: 12px 8px; + padding: 4px 8px 12px; border-bottom: 1px solid #f4dddb; font-size: 16px; font-weight: 500; @@ -23,3 +23,31 @@ .unhandled-exception-detail .unhandled-exception-body { padding: 16px 12px; } + +.unhandled-exception-with-stack { + max-width: 95%; +} + +.unhandled-exception-stack { + background: #fcfcfc; + overflow-x: auto; + overflow-y: hidden; +} + +.unhandled-exception-stack table { + border-spacing: 0; + border-collapse: collapse; + width: 100%; + border: 1px solid #d7d7d7; +} + +.unhandled-exception-stack th { + background: #e7e7e7; + border-bottom: 1px solid #d7d7d7; + padding: 8px; +} + +.unhandled-exception-stack td { + padding: 4px 8px; + white-space: nowrap; +} diff --git a/webroot/rsrc/css/phui/phui-form-view.css b/webroot/rsrc/css/phui/phui-form-view.css index 3368bcaafb..accce86819 100644 --- a/webroot/rsrc/css/phui/phui-form-view.css +++ b/webroot/rsrc/css/phui/phui-form-view.css @@ -578,3 +578,17 @@ properly, and submit values. */ .mfa-form-enroll-button { text-align: center; } + +.phui-form-timer-updated { + animation: phui-form-timer-fade-in 2s linear; +} + + +@keyframes phui-form-timer-fade-in { + 0% { + background-color: {$lightyellow}; + } + 100% { + background-color: transparent; + } +} diff --git a/webroot/rsrc/css/phui/phui-object-box.css b/webroot/rsrc/css/phui/phui-object-box.css index 4999a4c2c2..f95e36eedc 100644 --- a/webroot/rsrc/css/phui/phui-object-box.css +++ b/webroot/rsrc/css/phui/phui-object-box.css @@ -158,3 +158,8 @@ div.phui-object-box.phui-object-box-flush { margin-top: 8px; margin-bottom: 8px; } + +.phui-object-box-instructions { + padding: 16px; + border-bottom: 1px solid {$thinblueborder}; +} diff --git a/webroot/rsrc/js/phui/behavior-phui-timer-control.js b/webroot/rsrc/js/phui/behavior-phui-timer-control.js new file mode 100644 index 0000000000..d5b73a5ee2 --- /dev/null +++ b/webroot/rsrc/js/phui/behavior-phui-timer-control.js @@ -0,0 +1,41 @@ +/** + * @provides javelin-behavior-phui-timer-control + * @requires javelin-behavior + * javelin-stratcom + * javelin-dom + */ + +JX.behavior('phui-timer-control', function(config) { + var node = JX.$(config.nodeID); + var uri = config.uri; + var state = null; + + function onupdate(result) { + var markup = result.markup; + if (markup) { + var new_node = JX.$H(markup).getFragment().firstChild; + JX.DOM.replace(node, new_node); + node = new_node; + + // If the overall state has changed from the previous display state, + // animate the control to draw the user's attention to the state change. + if (result.state !== state) { + state = result.state; + JX.DOM.alterClass(node, 'phui-form-timer-updated', true); + } + } + + var retry = result.retry; + if (retry) { + setTimeout(update, 1000); + } + } + + function update() { + new JX.Request(uri, onupdate) + .setTimeout(10000) + .send(); + } + + update(); +});