diff --git a/src/applications/auth/application/PhabricatorApplicationAuth.php b/src/applications/auth/application/PhabricatorApplicationAuth.php index 1858b95037..b47da354a4 100644 --- a/src/applications/auth/application/PhabricatorApplicationAuth.php +++ b/src/applications/auth/application/PhabricatorApplicationAuth.php @@ -66,7 +66,8 @@ final class PhabricatorApplicationAuth extends PhabricatorApplication { 'start/' => 'PhabricatorAuthStartController', 'validate/' => 'PhabricatorAuthValidateController', 'unlink/(?P[^/]+)/' => 'PhabricatorAuthUnlinkController', - 'link/(?P[^/]+)/' => 'PhabricatorAuthLinkController', + '(?Plink|refresh)/(?P[^/]+)/' + => 'PhabricatorAuthLinkController', 'confirmlink/(?P[^/]+)/' => 'PhabricatorAuthConfirmLinkController', ), diff --git a/src/applications/auth/controller/PhabricatorAuthLinkController.php b/src/applications/auth/controller/PhabricatorAuthLinkController.php index 20e309337e..cb7d29c0d2 100644 --- a/src/applications/auth/controller/PhabricatorAuthLinkController.php +++ b/src/applications/auth/controller/PhabricatorAuthLinkController.php @@ -3,10 +3,12 @@ final class PhabricatorAuthLinkController extends PhabricatorAuthController { + private $action; private $providerKey; public function willProcessRequest(array $data) { $this->providerKey = $data['pkey']; + $this->action = $data['action']; } public function processRequest() { @@ -19,12 +21,27 @@ final class PhabricatorAuthLinkController return new Aphront404Response(); } - if (!$provider->shouldAllowAccountLink()) { - return $this->renderErrorPage( - pht('Account Not Linkable'), - array( - pht('This provider is not configured to allow linking.'), - )); + switch ($this->action) { + case 'link': + if (!$provider->shouldAllowAccountLink()) { + return $this->renderErrorPage( + pht('Account Not Linkable'), + array( + pht('This provider is not configured to allow linking.'), + )); + } + break; + case 'refresh': + if (!$provider->shouldAllowAccountRefresh()) { + return $this->renderErrorPage( + pht('Account Not Refreshable'), + array( + pht('This provider does not allow refreshing.'), + )); + } + break; + default: + return new Aphront400Response(); } $account = id(new PhabricatorExternalAccount())->loadOneWhere( @@ -32,20 +49,47 @@ final class PhabricatorAuthLinkController $provider->getProviderType(), $provider->getProviderDomain(), $viewer->getPHID()); - if ($account) { - return $this->renderErrorPage( - pht('Account Already Linked'), - array( - pht( - 'Your Phabricator account is already linked to an external '. - 'account for this provider.'), - )); + + switch ($this->action) { + case 'link': + if ($account) { + return $this->renderErrorPage( + pht('Account Already Linked'), + array( + pht( + 'Your Phabricator account is already linked to an external '. + 'account for this provider.'), + )); + } + break; + case 'refresh': + if (!$account) { + return $this->renderErrorPage( + pht('No Account Linked'), + array( + pht( + 'You do not have a linked account on this provider, and thus '. + 'can not refresh it.'), + )); + } + break; + default: + return new Aphront400Response(); } $panel_uri = '/settings/panel/external/'; $request->setCookie('phcid', Filesystem::readRandomCharacters(16)); - $form = $provider->buildLinkForm($this); + switch ($this->action) { + case 'link': + $form = $provider->buildLinkForm($this); + break; + case 'refresh': + $form = $provider->buildRefreshForm($this); + break; + default: + return new Aphront400Response(); + } if ($provider->isLoginFormAButton()) { require_celerity_resource('auth-css'); @@ -57,6 +101,19 @@ final class PhabricatorAuthLinkController $form); } + switch ($this->action) { + case 'link': + $name = pht('Link Account'); + $title = pht('Link %s Account', $provider->getProviderName()); + break; + case 'refresh': + $name = pht('Refresh Account'); + $title = pht('Refresh %s Account', $provider->getProviderName()); + break; + default: + return new Aphront400Response(); + } + $crumbs = $this->buildApplicationCrumbs(); $crumbs->addCrumb( id(new PhabricatorCrumbView()) @@ -64,7 +121,7 @@ final class PhabricatorAuthLinkController ->setHref($panel_uri)); $crumbs->addCrumb( id(new PhabricatorCrumbView()) - ->setName($provider->getProviderName())); + ->setName($provider->getProviderName($name))); return $this->buildApplicationPage( array( @@ -72,7 +129,7 @@ final class PhabricatorAuthLinkController $form, ), array( - 'title' => pht('Link %s Account', $provider->getProviderName()), + 'title' => $title, 'dust' => true, 'device' => true, )); diff --git a/src/applications/auth/controller/PhabricatorAuthLoginController.php b/src/applications/auth/controller/PhabricatorAuthLoginController.php index c94180c999..2e99ac82db 100644 --- a/src/applications/auth/controller/PhabricatorAuthLoginController.php +++ b/src/applications/auth/controller/PhabricatorAuthLoginController.php @@ -50,10 +50,11 @@ final class PhabricatorAuthLoginController $provider->getProviderName())); } } else if ($viewer->getPHID() == $account->getUserPHID()) { - return $this->renderError( - pht( - 'This external account ("%s") is already linked to your '. - 'Phabricator account.')); + // This is either an attempt to re-link an existing and already + // linked account (which is silly) or a refresh of an external account + // (e.g., an OAuth account). + return id(new AphrontRedirectResponse()) + ->setURI('/settings/panel/external/'); } else { return $this->renderError( pht( diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php index e73cda8e36..4f7e0bad5d 100644 --- a/src/applications/auth/provider/PhabricatorAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorAuthProvider.php @@ -154,6 +154,15 @@ abstract class PhabricatorAuthProvider { return $this->renderLoginForm($controller->getRequest(), $mode = 'link'); } + public function shouldAllowAccountRefresh() { + return true; + } + + public function buildRefreshForm( + PhabricatorAuthLinkController $controller) { + return $this->renderLoginForm($controller->getRequest(), $mode = 'refresh'); + } + protected function renderLoginForm( AphrontRequest $request, $mode) { diff --git a/src/applications/auth/provider/PhabricatorAuthProviderLDAP.php b/src/applications/auth/provider/PhabricatorAuthProviderLDAP.php index 5b6e97b47d..a3f0432937 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderLDAP.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderLDAP.php @@ -68,6 +68,10 @@ final class PhabricatorAuthProviderLDAP $dialog->setTitle(pht('Link LDAP Account')); $dialog->addSubmitButton(pht('Link Accounts')); $dialog->addCancelButton($this->getSettingsURI()); + } else if ($mode == 'refresh') { + $dialog->setTitle(pht('Refresh LDAP Account')); + $dialog->addSubmitButton(pht('Refresh Account')); + $dialog->addCancelButton($this->getSettingsURI()); } else { if ($this->shouldAllowRegistration()) { $dialog->setTitle(pht('Login or Register with LDAP')); diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php index 97c2687edb..7c9ed7edac 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php @@ -38,6 +38,8 @@ abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider { if ($mode == 'link') { $button_text = pht('Link External Account'); + } else if ($mode == 'refresh') { + $button_text = pht('Refresh Account Link'); } else if ($this->shouldAllowRegistration()) { $button_text = pht('Login or Register'); } else { diff --git a/src/applications/auth/provider/PhabricatorAuthProviderPassword.php b/src/applications/auth/provider/PhabricatorAuthProviderPassword.php index e5a40752b0..c56f8ea26f 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderPassword.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderPassword.php @@ -246,4 +246,8 @@ final class PhabricatorAuthProviderPassword return; } + public function shouldAllowAccountRefresh() { + return false; + } + } diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelExternalAccounts.php b/src/applications/settings/panel/PhabricatorSettingsPanelExternalAccounts.php index 755fb1edd6..86278c275e 100644 --- a/src/applications/settings/panel/PhabricatorSettingsPanelExternalAccounts.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelExternalAccounts.php @@ -70,6 +70,14 @@ final class PhabricatorSettingsPanelExternalAccounts $can_unlink = $can_unlink && (!$can_login || ($login_accounts > 1)); + $can_refresh = $provider && $provider->shouldAllowAccountRefresh(); + if ($can_refresh) { + $item->addAction( + id(new PHUIListItemView()) + ->setIcon('refresh') + ->setHref('/auth/refresh/'.$account->getProviderKey().'/')); + } + $item->addAction( id(new PHUIListItemView()) ->setIcon('delete')