From 49f0119c12b24454e001b12b5af04fc2654c3add Mon Sep 17 00:00:00 2001
From: Kyle Zhao <kzhao.sw@gmail.com>
Date: Mon, 21 Aug 2017 19:14:26 -0400
Subject: [PATCH] FEATURE: import Github profile picture

---
 lib/auth/github_authenticator.rb              | 15 +++++
 .../auth/github_authenticator_spec.rb         | 57 ++++++++++++++++---
 2 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/lib/auth/github_authenticator.rb b/lib/auth/github_authenticator.rb
index 41874a04c1f..8fe20999b78 100644
--- a/lib/auth/github_authenticator.rb
+++ b/lib/auth/github_authenticator.rb
@@ -91,6 +91,8 @@ class Auth::GithubAuthenticator < Auth::Authenticator
       end
     end
 
+    retrieve_avatar(user, data)
+
     result.user = user
     result
   end
@@ -102,6 +104,8 @@ class Auth::GithubAuthenticator < Auth::Authenticator
       screen_name: data[:github_screen_name],
       github_user_id: data[:github_user_id]
     )
+
+    retrieve_avatar(user, data)
   end
 
   def register_middleware(omniauth)
@@ -113,4 +117,15 @@ class Auth::GithubAuthenticator < Auth::Authenticator
            },
            scope: "user:email"
   end
+
+  protected
+
+  def retrieve_avatar(user, data)
+    return unless user
+    return if user.user_avatar&.custom_upload_id.present?
+
+    if (avatar_url = data[:image]).present?
+      Jobs.enqueue(:download_avatar_from_url, url: avatar_url, user_id: user.id, override_gravatar: false)
+    end
+  end
 end
diff --git a/spec/components/auth/github_authenticator_spec.rb b/spec/components/auth/github_authenticator_spec.rb
index ee1edd1775f..8f81cb58af8 100644
--- a/spec/components/auth/github_authenticator_spec.rb
+++ b/spec/components/auth/github_authenticator_spec.rb
@@ -6,13 +6,33 @@ require 'rails_helper'
 Auth.send(:remove_const, :GithubAuthenticator)
 load 'auth/github_authenticator.rb'
 
+def auth_token_for(user)
+  {
+    extra: {
+      all_emails: [{
+        email: user.email,
+        primary: true,
+        verified: true,
+      }]
+    },
+    info: {
+      email: user.email,
+      email_verified: true,
+      nickname: user.username,
+      name: user.name,
+      image: "https://avatars3.githubusercontent.com/u/#{user.username}",
+    },
+    uid: '100'
+  }
+end
+
 describe Auth::GithubAuthenticator do
+  let(:authenticator) { described_class.new }
+  let(:user) { Fabricate(:user) }
 
   context 'after_authenticate' do
 
     it 'can authenticate and create a user record for already existing users' do
-      user = Fabricate(:user)
-
       hash = {
         extra: {
           all_emails: [{
@@ -30,7 +50,6 @@ describe Auth::GithubAuthenticator do
         uid: "100"
       }
 
-      authenticator = Auth::GithubAuthenticator.new
       result = authenticator.after_authenticate(hash)
 
       expect(result.user.id).to eq(user.id)
@@ -41,8 +60,6 @@ describe Auth::GithubAuthenticator do
     end
 
     it 'will not authenticate for already existing users with an unverified email' do
-      user = Fabricate(:user)
-
       hash = {
         extra: {
           all_emails: [{
@@ -60,7 +77,6 @@ describe Auth::GithubAuthenticator do
         uid: "100"
       }
 
-      authenticator = Auth::GithubAuthenticator.new
       result = authenticator.after_authenticate(hash)
 
       expect(result.user).to eq(nil)
@@ -88,7 +104,6 @@ describe Auth::GithubAuthenticator do
         uid: "100"
       }
 
-      authenticator = Auth::GithubAuthenticator.new
       result = authenticator.after_authenticate(hash)
 
       expect(result.user).to eq(nil)
@@ -120,7 +135,6 @@ describe Auth::GithubAuthenticator do
         uid: "100"
       }
 
-      authenticator = Auth::GithubAuthenticator.new
       SiteSetting.email_domains_blacklist = "blacklist.com"
       result = authenticator.after_authenticate(hash)
 
@@ -157,7 +171,6 @@ describe Auth::GithubAuthenticator do
         uid: "100"
       }
 
-      authenticator = Auth::GithubAuthenticator.new
       SiteSetting.email_domains_whitelist = "whitelist.com"
       result = authenticator.after_authenticate(hash)
 
@@ -169,4 +182,30 @@ describe Auth::GithubAuthenticator do
     end
 
   end
+
+  describe 'avatar retrieval' do
+    context 'when user has a custom avatar' do
+      let(:user_avatar) { Fabricate(:user_avatar, custom_upload: Fabricate(:upload)) }
+      let(:user_with_custom_avatar) { Fabricate(:user, user_avatar: user_avatar) }
+
+      it 'does not enqueue a download_avatar_from_url job' do
+        Jobs.expects(:enqueue).with(:download_avatar_from_url, anything).never
+
+        result = authenticator.after_authenticate(auth_token_for(user_with_custom_avatar))
+      end
+    end
+
+    context 'when user does not have a custom avatar' do
+      it 'enqueues a download_avatar_from_url job' do
+        Jobs.expects(:enqueue).with(
+          :download_avatar_from_url,
+          url: "https://avatars3.githubusercontent.com/u/#{user.username}",
+          user_id: user.id,
+          override_gravatar: false
+        )
+
+        result = authenticator.after_authenticate(auth_token_for(user))
+      end
+    end
+  end
 end