FEATURE: Import facebook avatars when logging in via facebook

FIX: warning about popup dimensions when using facebook login

Rules are:

- On account creation we always import
- If you already have an avatar uploaded, nothing is changed
- If you have no avatar uploaded, we upload from facebook on login
- If you have no avatar uploaded, we select facebook unless gravatar already selected

This also fixes SSO issues where on account creation accounts had missing avatar uploads

app/assets/javascripts/discourse/controllers/login.js.es6

@@ -130,7 +130,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
       if(customLogin){
         customLogin();
       } else {
-        const authUrl = loginMethod.get('customUrl') || Discourse.getURL("/auth/" + name);
+        let authUrl = loginMethod.get('customUrl') || Discourse.getURL("/auth/" + name);
         if (loginMethod.get("fullScreenLogin")) {
           document.cookie = "fsl=true";
           window.location = authUrl;
@@ -141,6 +141,11 @@ export default Ember.Controller.extend(ModalFunctionality, {
 
           const height = loginMethod.get("frameHeight") || 400;
           const width = loginMethod.get("frameWidth") || 800;
+
+          if (loginMethod.get("displayPopup")) {
+            authUrl = authUrl + "?display=popup";
+          }
+
           const w = window.open(authUrl, "_blank",
               "menubar=no,status=no,height=" + height + ",width=" + width +  ",left=" + left + ",top=" + top);
           const self = this;

app/assets/javascripts/discourse/models/login-method.js.es6

@@ -34,7 +34,9 @@ export function findAll(siteSettings, capabilities, isMobileDevice) {
         params.frameWidth = 850;
         params.frameHeight = 500;
       } else if (name === "facebook") {
-        params.frameHeight = 450;
+        params.frameWidth= 580;
+        params.frameHeight = 400;
+        params.displayPopup = true;
       }
 
       params.siteSettings = siteSettings;

app/models/user_avatar.rb

@@ -60,23 +60,33 @@ class UserAvatar < ActiveRecord::Base
     "#{upload_id}_#{OptimizedImage::VERSION}"
   end
 
-  def self.import_url_for_user(avatar_url, user)
+  def self.import_url_for_user(avatar_url, user, options=nil)
     tempfile = FileHelper.download(avatar_url, SiteSetting.max_image_size_kb.kilobytes, "sso-avatar", true)
 
     ext = FastImage.type(tempfile).to_s
     tempfile.rewind
 
     upload = Upload.create_for(user.id, tempfile, "external-avatar." + ext, File.size(tempfile.path), origin: avatar_url, image_type: "avatar")
-    user.uploaded_avatar_id = upload.id
 
     unless user.user_avatar
-      user.build_user_avatar
+      user.create_user_avatar
     end
 
     if !user.user_avatar.contains_upload?(upload.id)
-      user.user_avatar.custom_upload_id = upload.id
+      user.user_avatar.update_columns(custom_upload_id: upload.id)
+
+      override_gravatar = !options || options[:override_gravatar]
+
+      if user.uploaded_avatar_id.nil? ||
+          !user.user_avatar.contains_upload?(user.uploaded_avatar_id) ||
+          override_gravatar
+        user.update_columns(uploaded_avatar_id: upload.id)
       end
+    end
+
+
   rescue => e
+    p e
     # skip saving, we are not connected to the net
     Rails.logger.warn "#{e}: Failed to download external avatar: #{avatar_url}, user id #{ user.id }"
   ensure

db/migrate/20160919003141_add_avatar_url_to_facebook_info.rb

@@ -0,0 +1,5 @@
+class AddAvatarUrlToFacebookInfo < ActiveRecord::Migration
+  def change
+    add_column :facebook_user_infos, :avatar_url, :string
+  end
+end

lib/auth/facebook_authenticator.rb

@@ -24,6 +24,17 @@ class Auth::FacebookAuthenticator < Auth::Authenticator
       FacebookUserInfo.create({user_id: result.user.id}.merge(facebook_hash))
     end
 
+    if user_info
+      user_info.update_columns(facebook_hash)
+    end
+
+    user = result.user
+    if user && (!user.user_avatar || user.user_avatar.custom_upload_id.nil?)
+      if (avatar_url = facebook_hash[:avatar_url]).present?
+        UserAvatar.import_url_for_user(avatar_url, user, override_gravatar: false)
+      end
+    end
+
     if email.blank?
       UserHistory.create(
         action: UserHistory.actions[:facebook_no_email],
@@ -37,14 +48,24 @@ class Auth::FacebookAuthenticator < Auth::Authenticator
   def after_create_account(user, auth)
     data = auth[:extra_data]
     FacebookUserInfo.create({user_id: user.id}.merge(data))
+
+
+    if (avatar_url = data[:avatar_url]).present?
+      UserAvatar.import_url_for_user(avatar_url, user)
+      user.save
+    end
+
+    true
   end
 
   def register_middleware(omniauth)
+
     omniauth.provider :facebook,
            :setup => lambda { |env|
               strategy = env["omniauth.strategy"]
               strategy.options[:client_id] = SiteSetting.facebook_app_id
               strategy.options[:client_secret] = SiteSetting.facebook_app_secret
+              strategy.options[:info_fields] = 'gender,email,name,bio,first_name,link,last_name'
            },
            :scope => "email"
   end
@@ -54,6 +75,8 @@ class Auth::FacebookAuthenticator < Auth::Authenticator
   def parse_auth_token(auth_token)
 
     raw_info = auth_token["extra"]["raw_info"]
+    info = auth_token["info"]
+
     email = auth_token["info"][:email]
 
     {
@@ -65,7 +88,8 @@ class Auth::FacebookAuthenticator < Auth::Authenticator
         last_name: raw_info["last_name"],
         email: email,
         gender: raw_info["gender"],
-        name: raw_info["name"]
+        name: raw_info["name"],
+        avatar_url: info["image"]
       },
       email: email,
       email_valid: true

spec/models/user_avatar_spec.rb

@@ -17,4 +17,34 @@ describe UserAvatar do
     temp.unlink
     expect(avatar.gravatar_upload).not_to eq(nil)
   end
+
+  context '#import_url_for_user' do
+
+    it 'creates user_avatar record if missing' do
+      user = Fabricate(:user)
+      user.user_avatar.destroy
+      user.reload
+
+
+      FileHelper.stubs(:download).returns(file_from_fixtures("logo.png"))
+
+      UserAvatar.import_url_for_user("logo.png", user)
+      user.reload
+
+      expect(user.uploaded_avatar_id).not_to eq(nil)
+      expect(user.user_avatar.custom_upload_id).to eq(user.uploaded_avatar_id)
+    end
+
+    it 'can leave gravatar alone' do
+      user = Fabricate(:user, uploaded_avatar_id: 1)
+      user.user_avatar.update_columns(gravatar_upload_id: 1)
+
+      FileHelper.stubs(:download).returns(file_from_fixtures("logo.png"))
+      UserAvatar.import_url_for_user("logo.png", user, override_gravatar: false)
+
+      user.reload
+      expect(user.uploaded_avatar_id).to eq(1)
+      expect(user.user_avatar.custom_upload_id).not_to eq(nil)
+    end
+  end
 end