diff --git a/src/applications/auth/controller/PhabricatorAuthStartController.php b/src/applications/auth/controller/PhabricatorAuthStartController.php --- a/src/applications/auth/controller/PhabricatorAuthStartController.php +++ b/src/applications/auth/controller/PhabricatorAuthStartController.php @@ -113,17 +113,9 @@ PhabricatorCookies::setClientIDCookie($request); } - if (!$request->getURIData('loggedout') && count($providers) == 1) { - $auto_login_provider = head($providers); - $auto_login_config = $auto_login_provider->getProviderConfig(); - if ($auto_login_provider instanceof PhabricatorPhabricatorAuthProvider && - $auto_login_config->getShouldAutoLogin()) { - $auto_login_adapter = $provider->getAdapter(); - $auto_login_adapter->setState($provider->getAuthCSRFCode($request)); - return id(new AphrontRedirectResponse()) - ->setIsExternal(true) - ->setURI($provider->getAdapter()->getAuthenticateURI()); - } + $auto_response = $this->tryAutoLogin($providers); + if ($auto_response) { + return $auto_response; } $invite = $this->loadInvite(); @@ -282,4 +274,35 @@ array($message)); } + private function tryAutoLogin(array $providers) { + $request = $this->getRequest(); + + // If the user just logged out, don't immediately log them in again. + if ($request->getURIData('loggedout')) { + return null; + } + + // If we have more than one provider, we can't autologin because we + // don't know which one the user wants. + if (count($providers) != 1) { + return null; + } + + $provider = head($providers); + if (!$provider->supportsAutoLogin()) { + return null; + } + + $config = $provider->getProviderConfig(); + if (!$config->getShouldAutoLogin()) { + return null; + } + + $auto_uri = $provider->getAutoLoginURI($request); + + return id(new AphrontRedirectResponse()) + ->setIsExternal(true) + ->setURI($auto_uri); + } + } diff --git a/src/applications/auth/controller/config/PhabricatorAuthEditController.php b/src/applications/auth/controller/config/PhabricatorAuthEditController.php --- a/src/applications/auth/controller/config/PhabricatorAuthEditController.php +++ b/src/applications/auth/controller/config/PhabricatorAuthEditController.php @@ -130,7 +130,7 @@ PhabricatorAuthProviderConfigTransaction::TYPE_TRUST_EMAILS) ->setNewValue($request->getInt('trustEmails', 0)); - if ($provider instanceof PhabricatorPhabricatorAuthProvider) { + if ($provider->supportsAutoLogin()) { $xactions[] = id(new PhabricatorAuthProviderConfigTransaction()) ->setTransactionType( PhabricatorAuthProviderConfigTransaction::TYPE_AUTO_LOGIN) @@ -314,7 +314,7 @@ $v_trust_email)); } - if ($provider instanceof PhabricatorPhabricatorAuthProvider) { + if ($provider->supportsAutoLogin()) { $form->appendChild( id(new AphrontFormCheckboxControl()) ->addCheckbox( diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php --- a/src/applications/auth/provider/PhabricatorAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorAuthProvider.php @@ -495,4 +495,12 @@ } } + public function supportsAutoLogin() { + return false; + } + + public function getAutoLoginURI(AphrontRequest $request) { + throw new PhutilMethodNotImplementedException(); + } + } diff --git a/src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php b/src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php --- a/src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php +++ b/src/applications/auth/provider/PhabricatorOAuth2AuthProvider.php @@ -273,4 +273,17 @@ parent::willRenderLinkedAccount($viewer, $item, $account); } + public function supportsAutoLogin() { + return true; + } + + public function getAutoLoginURI(AphrontRequest $request) { + $csrf_code = $this->getAuthCSRFCode($request); + + $adapter = $this->getAdapter(); + $adapter->setState($csrf_code); + + return $adapter->getAuthenticateURI(); + } + }