From 7081923bd73c271faa57e375ab942cf7ad5a1797 Mon Sep 17 00:00:00 2001
From: Neal Poole <neal@fb.com>
Date: Tue, 17 Jul 2012 18:18:16 -0700
Subject: [PATCH] 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
---
 conf/default.conf.php                             |  3 +++
 .../PhabricatorOAuthLoginController.php           | 10 ++++++++--
 .../provider/PhabricatorOAuthProviderFacebook.php | 15 ++++++++++++++-
 .../auth/view/PhabricatorOAuthFailureView.php     | 11 +++++++++++
 4 files changed, 36 insertions(+), 3 deletions(-)

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(
           '<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(