mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-23 22:10:55 +01:00
Adding 'Secure Browsing' config setting to Facebook OAuth.
Summary: The Graph API exposes a new field, security_settings, which allows applications to see whether a user has enabled Secure Browsing. This diff adds a configuration setting to Phabricator which forces users to have Secure Browsing enabled when logging in via Facebook. Test Plan: With the configuration setting off, verify that secure browsing does not affect the ability to log in. With the configuration setting on and secure browsing off, verify that the login attempts is rejected. Then verify that the login attempt succeeds when secure browsing is enabled. Reviewers: epriestley Reviewed By: epriestley CC: arice, aran, Korvin Maniphest Tasks: T1487 Differential Revision: https://secure.phabricator.com/D2964
This commit is contained in:
parent
0d162e15f6
commit
7081923bd7
4 changed files with 36 additions and 3 deletions
|
@ -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 ---------------------------------------------------------- //
|
||||
|
||||
|
|
|
@ -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(
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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(
|
||||
'<p><strong>Error Reason:</strong> %s</p>',
|
||||
$request->getStr('error_reason')));
|
||||
} else if ($this->exception) {
|
||||
$view->appendChild(
|
||||
hsprintf(
|
||||
'<p><strong>Error Details:</strong> %s</p>',
|
||||
$this->exception->getMessage()));
|
||||
} else {
|
||||
// TODO: We can probably refine this.
|
||||
$view->appendChild(
|
||||
|
|
Loading…
Reference in a new issue