mirror of
https://we.phorge.it/source/phorge.git
synced 2025-01-08 22:01:03 +01:00
Add a "refresh" action for external accounts
Summary: Ref T1536. This is equivalent to logging out and logging back in again, but a bit less disruptive for users. For some providers (like Google), this may eventually do something different (Google has a "force" parameter which forces re-auth and is ostensibly required to refresh long-lived tokens). Broadly, this process fixes OAuth accounts with busted access tokens so we can do API stuff. For other accounts, it mostly just syncs profile pictures. Test Plan: Refreshed LDAP and Oauth accounts, linked OAuth accounts, hit error conditions. {F47390} {F47391} {F47392} {F47393} {F47394} {F47395} Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T1536 Differential Revision: https://secure.phabricator.com/D6290
This commit is contained in:
parent
e826842179
commit
fe71b34c68
8 changed files with 108 additions and 22 deletions
|
@ -66,7 +66,8 @@ final class PhabricatorApplicationAuth extends PhabricatorApplication {
|
|||
'start/' => 'PhabricatorAuthStartController',
|
||||
'validate/' => 'PhabricatorAuthValidateController',
|
||||
'unlink/(?P<pkey>[^/]+)/' => 'PhabricatorAuthUnlinkController',
|
||||
'link/(?P<pkey>[^/]+)/' => 'PhabricatorAuthLinkController',
|
||||
'(?P<action>link|refresh)/(?P<pkey>[^/]+)/'
|
||||
=> 'PhabricatorAuthLinkController',
|
||||
'confirmlink/(?P<akey>[^/]+)/'
|
||||
=> 'PhabricatorAuthConfirmLinkController',
|
||||
),
|
||||
|
|
|
@ -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,
|
||||
));
|
||||
|
|
|
@ -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(
|
||||
|
|
|
@ -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) {
|
||||
|
|
|
@ -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'));
|
||||
|
|
|
@ -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 {
|
||||
|
|
|
@ -246,4 +246,8 @@ final class PhabricatorAuthProviderPassword
|
|||
return;
|
||||
}
|
||||
|
||||
public function shouldAllowAccountRefresh() {
|
||||
return false;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -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')
|
||||
|
|
Loading…
Reference in a new issue