diff --git a/src/applications/auth/application/PhabricatorAuthApplication.php b/src/applications/auth/application/PhabricatorAuthApplication.php --- a/src/applications/auth/application/PhabricatorAuthApplication.php +++ b/src/applications/auth/application/PhabricatorAuthApplication.php @@ -61,7 +61,7 @@ 'start/' => 'PhabricatorAuthStartController', 'validate/' => 'PhabricatorAuthValidateController', 'finish/' => 'PhabricatorAuthFinishController', - 'unlink/(?P[^/]+)/' => 'PhabricatorAuthUnlinkController', + 'unlink/(?P\d+)/' => 'PhabricatorAuthUnlinkController', '(?Plink|refresh)/(?P[^/]+)/' => 'PhabricatorAuthLinkController', 'confirmlink/(?P[^/]+)/' diff --git a/src/applications/auth/controller/PhabricatorAuthUnlinkController.php b/src/applications/auth/controller/PhabricatorAuthUnlinkController.php --- 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'); - - 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()); + $id = $request->getURIData('id'); + + $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); + 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,7 +52,9 @@ if ($valid_accounts < 2) { if (!isset($confirmations['only'])) { - return $this->renderOnlyUsableAccountConfirmDialog($confirmations); + return $this->renderOnlyUsableAccountConfirmDialog( + $confirmations, + $done_uri); } } } @@ -67,42 +66,27 @@ 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 +100,23 @@ 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 +127,7 @@ '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/settings/panel/PhabricatorExternalAccountsSettingsPanel.php b/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php --- a/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php @@ -78,7 +78,7 @@ ->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);