diff --git a/conf/default.conf.php b/conf/default.conf.php index 6b856ea69f..4176dcb9b5 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -582,6 +582,9 @@ return array( // The Facebook "Application Secret" to use for Facebook API access. 'facebook.application-secret' => null, + // Should Phabricator reject requests made by users with + // Secure Browsing disabled? + 'facebook.require-https-auth' => false, // -- GitHub OAuth ---------------------------------------------------------- // diff --git a/src/applications/auth/controller/PhabricatorOAuthLoginController.php b/src/applications/auth/controller/PhabricatorOAuthLoginController.php index 62da43d8c7..d87e7a2cd4 100644 --- a/src/applications/auth/controller/PhabricatorOAuthLoginController.php +++ b/src/applications/auth/controller/PhabricatorOAuthLoginController.php @@ -70,7 +70,7 @@ final class PhabricatorOAuthLoginController } $provider->setUserData($user_data); } catch (PhabricatorOAuthProviderException $e) { - return $this->buildErrorResponse(new PhabricatorOAuthFailureView()); + return $this->buildErrorResponse(new PhabricatorOAuthFailureView(), $e); } $provider->setAccessToken($this->accessToken); @@ -243,12 +243,18 @@ final class PhabricatorOAuthLoginController return $this->delegateToController($controller); } - private function buildErrorResponse(PhabricatorOAuthFailureView $view) { + private function buildErrorResponse(PhabricatorOAuthFailureView $view, + Exception $e = null) { + $provider = $this->provider; $provider_name = $provider->getProviderName(); $view->setOAuthProvider($provider); + if ($e) { + $view->setException($e); + } + return $this->buildStandardPageResponse( $view, array( diff --git a/src/applications/auth/oauth/provider/PhabricatorOAuthProviderFacebook.php b/src/applications/auth/oauth/provider/PhabricatorOAuthProviderFacebook.php index ec396d7145..32bc418320 100644 --- a/src/applications/auth/oauth/provider/PhabricatorOAuthProviderFacebook.php +++ b/src/applications/auth/oauth/provider/PhabricatorOAuthProviderFacebook.php @@ -78,7 +78,9 @@ final class PhabricatorOAuthProviderFacebook extends PhabricatorOAuthProvider { } public function getUserInfoURI() { - return 'https://graph.facebook.com/me'; + $fields = array('id', 'name', 'email', 'link', 'security_settings'); + return 'https://graph.facebook.com/me?fields='. + implode(',', $fields); } public function getMinimumScope() { @@ -88,6 +90,17 @@ final class PhabricatorOAuthProviderFacebook extends PhabricatorOAuthProvider { public function setUserData($data) { $data = json_decode($data, true); $this->validateUserData($data); + + if (PhabricatorEnv::getEnvConfig('facebook.require-https-auth')) { + if (!$data['security_settings']['secure_browsing']['enabled']) { + throw new PhabricatorOAuthProviderException( + 'You must enable Secure Browsing on your Facebook account in'. + ' order to log in to Phabricator. For more information, check'. + ' out http://www.facebook.com/help/?faq=215897678434749' + ); + } + } + $this->userData = $data; return $this; } diff --git a/src/applications/auth/view/PhabricatorOAuthFailureView.php b/src/applications/auth/view/PhabricatorOAuthFailureView.php index b1e407ad85..1af0db2925 100644 --- a/src/applications/auth/view/PhabricatorOAuthFailureView.php +++ b/src/applications/auth/view/PhabricatorOAuthFailureView.php @@ -20,6 +20,7 @@ final class PhabricatorOAuthFailureView extends AphrontView { private $request; private $provider; + private $exception; public function setRequest(AphrontRequest $request) { $this->request = $request; @@ -31,6 +32,11 @@ final class PhabricatorOAuthFailureView extends AphrontView { return $this; } + public function setException(Exception $e) { + $this->exception = $e; + return $this; + } + public function render() { $request = $this->request; $provider = $this->provider; @@ -53,6 +59,11 @@ final class PhabricatorOAuthFailureView extends AphrontView { hsprintf( '
Error Reason: %s
', $request->getStr('error_reason'))); + } else if ($this->exception) { + $view->appendChild( + hsprintf( + 'Error Details: %s
', + $this->exception->getMessage())); } else { // TODO: We can probably refine this. $view->appendChild(