From af4ab07f46b07fbc9b1e5945a4aa1e4af7c616a8 Mon Sep 17 00:00:00 2001
From: epriestley <epriestley@phabricator.com>
Date: Wed, 23 Feb 2011 10:27:33 -0800
Subject: [PATCH] Fix Facebook OAuth flow to ask for email.

Summary:

Test Plan:

Reviewers:

CC:
---
 .../auth/controller/login/PhabricatorLoginController.php      | 2 ++
 .../auth/oauth/provider/base/PhabricatorOAuthProvider.php     | 1 +
 .../provider/facebook/PhabricatorOAuthProviderFacebook.php    | 4 ++++
 .../oauth/provider/github/PhabricatorOAuthProviderGithub.php  | 4 ++++
 4 files changed, 11 insertions(+)

diff --git a/src/applications/auth/controller/login/PhabricatorLoginController.php b/src/applications/auth/controller/login/PhabricatorLoginController.php
index eca7c6c648..e6991fae06 100644
--- a/src/applications/auth/controller/login/PhabricatorLoginController.php
+++ b/src/applications/auth/controller/login/PhabricatorLoginController.php
@@ -109,6 +109,7 @@ class PhabricatorLoginController extends PhabricatorAuthController {
       $redirect_uri   = $provider->getRedirectURI();
       $client_id      = $provider->getClientID();
       $provider_name  = $provider->getProviderName();
+      $minimum_scope  = $provider->getMinimumScope();
 
       // TODO: In theory we should use 'state' to prevent CSRF, but the total
       // effect of the CSRF attack is that an attacker can cause a user to login
@@ -121,6 +122,7 @@ class PhabricatorLoginController extends PhabricatorAuthController {
         ->setAction($auth_uri)
         ->addHiddenInput('client_id', $client_id)
         ->addHiddenInput('redirect_uri', $redirect_uri)
+        ->addHiddenInput('scope', $minimum_scope)
         ->setUser($request->getUser())
         ->setMethod('GET')
         ->appendChild(
diff --git a/src/applications/auth/oauth/provider/base/PhabricatorOAuthProvider.php b/src/applications/auth/oauth/provider/base/PhabricatorOAuthProvider.php
index 2bff6ff656..dd3590a9c6 100644
--- a/src/applications/auth/oauth/provider/base/PhabricatorOAuthProvider.php
+++ b/src/applications/auth/oauth/provider/base/PhabricatorOAuthProvider.php
@@ -30,6 +30,7 @@ abstract class PhabricatorOAuthProvider {
   abstract public function getAuthURI();
   abstract public function getTokenURI();
   abstract public function getUserInfoURI();
+  abstract public function getMinimumScope();
 
   public function __construct() {
 
diff --git a/src/applications/auth/oauth/provider/facebook/PhabricatorOAuthProviderFacebook.php b/src/applications/auth/oauth/provider/facebook/PhabricatorOAuthProviderFacebook.php
index e6a0407743..8893f9a178 100644
--- a/src/applications/auth/oauth/provider/facebook/PhabricatorOAuthProviderFacebook.php
+++ b/src/applications/auth/oauth/provider/facebook/PhabricatorOAuthProviderFacebook.php
@@ -54,4 +54,8 @@ class PhabricatorOAuthProviderFacebook extends PhabricatorOAuthProvider {
     return 'https://graph.facebook.com/me';
   }
 
+  public function getMinimumScope() {
+    return 'email';
+  }
+
 }
diff --git a/src/applications/auth/oauth/provider/github/PhabricatorOAuthProviderGithub.php b/src/applications/auth/oauth/provider/github/PhabricatorOAuthProviderGithub.php
index e49ad64854..7b92956a21 100644
--- a/src/applications/auth/oauth/provider/github/PhabricatorOAuthProviderGithub.php
+++ b/src/applications/auth/oauth/provider/github/PhabricatorOAuthProviderGithub.php
@@ -54,4 +54,8 @@ class PhabricatorOAuthProviderGithub extends PhabricatorOAuthProvider {
     return 'https://github.com/api/v2/json/user/show';
   }
 
+  public function getMinimumScope() {
+    return null;
+  }
+
 }