Merge branch 'more-secure-api' into 'master'

More secure api

Dont expose user email via API. Fixes #1314

6 files changed, 49 insertions, 24 deletions

diff --git a/app/assets/javascripts/project_users_select.js.coffee b/app/assets/javascripts/project_users_select.js.coffee
index 382f9b37992..cfbcd5108c8 100644
--- a/app/assets/javascripts/project_users_select.js.coffee
+++ b/app/assets/javascripts/project_users_select.js.coffee
@@ -37,13 +37,9 @@
 
   projectUserFormatResult: (user) ->
     if user.avatar_url
-      avatar = gon.relative_url_root + user.avatar_url
-    else if gon.gravatar_enabled
-      avatar = gon.gravatar_url
-      avatar = avatar.replace('%{hash}', md5(user.email))
-      avatar = avatar.replace('%{size}', '24')
+      avatar = user.avatar_url
     else
-      avatar = gon.relative_url_root + "#{image_path('no_avatar.png')}"
+      avatar = gon.default_avatar_url
 
     if user.id == ''
       avatarMarkup = ''
diff --git a/app/assets/javascripts/users_select.js.coffee b/app/assets/javascripts/users_select.js.coffee
index da66a4ba7f2..86318bd7d94 100644
--- a/app/assets/javascripts/users_select.js.coffee
+++ b/app/assets/javascripts/users_select.js.coffee
@@ -1,13 +1,9 @@
 $ ->
   userFormatResult = (user) ->
     if user.avatar_url
-      avatar = gon.relative_url_root + user.avatar_url
-    else if gon.gravatar_enabled
-      avatar = gon.gravatar_url
-      avatar = avatar.replace('%{hash}', md5(user.email))
-      avatar = avatar.replace('%{size}', '24')
+      avatar = user.avatar_url
     else
-      avatar = gon.relative_url_root + "#{image_path('no_avatar.png')}"
+      avatar = gon.default_avatar_url
 
     "<div class='user-result'>
        <div class='user-image'><img class='avatar s24' src='#{avatar}'></div>
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 685d41a5520..603e89a5e29 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -164,9 +164,8 @@ class ApplicationController < ActionController::Base
   def add_gon_variables
     gon.default_issues_tracker = Project.issues_tracker.default_value
     gon.api_version = API::API.version
-    gon.gravatar_url = request.ssl? || Gitlab.config.gitlab.https ? Gitlab.config.gravatar.ssl_url : Gitlab.config.gravatar.plain_url
     gon.relative_url_root = Gitlab.config.gitlab.relative_url_root
-    gon.gravatar_enabled = Gitlab.config.gravatar.enabled
+    gon.default_avatar_url = URI::join(Gitlab.config.gitlab.url, ActionController::Base.helpers.image_path('no_avatar.png')).to_s
 
     if current_user
       gon.current_user_id = current_user.id
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 13120d2e581..c3d89eb1b82 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -60,23 +60,21 @@ module ApplicationHelper
 
   def avatar_icon(user_email = '', size = nil)
     user = User.find_by(email: user_email)
-    if user && user.avatar.present?
-      user.avatar.url
+
+    if user
+      user.avatar_url(size) || default_avatar
     else
       gravatar_icon(user_email, size)
     end
   end
 
   def gravatar_icon(user_email = '', size = nil)
-    size = 40 if size.nil? || size <= 0
+    GravatarService.new.execute(user_email, size) ||
+      default_avatar
+  end
 
-    if !Gitlab.config.gravatar.enabled || user_email.blank?
-      image_path('no_avatar.png')
-    else
-      gravatar_url = request.ssl? || gitlab_config.https ? Gitlab.config.gravatar.ssl_url : Gitlab.config.gravatar.plain_url
-      user_email.strip!
-      sprintf gravatar_url, hash: Digest::MD5.hexdigest(user_email.downcase), size: size, email: user_email
-    end
+  def default_avatar
+    image_path('no_avatar.png')
   end
 
   def last_commit(project)
diff --git a/app/models/user.rb b/app/models/user.rb
index 0fbc9284dd8..2352f8c050b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -482,4 +482,12 @@ class User < ActiveRecord::Base
   def public_profile?
     authorized_projects.public_only.any?
   end
+
+  def avatar_url(size = nil)
+    if avatar.present?
+      URI::join(Gitlab.config.gitlab.url, avatar.url).to_s
+    else
+      GravatarService.new.execute(email, size)
+    end
+  end
 end
diff --git a/app/services/gravatar_service.rb b/app/services/gravatar_service.rb
new file mode 100644
index 00000000000..a69c7c78377
--- /dev/null
+++ b/app/services/gravatar_service.rb
@@ -0,0 +1,28 @@
+class GravatarService
+  def execute(email, size = nil)
+    if gravatar_config.enabled && email.present?
+      size = 40 if size.nil? || size <= 0
+
+      sprintf gravatar_url,
+        hash: Digest::MD5.hexdigest(email.strip.downcase),
+        size: size,
+        email: email.strip
+    end
+  end
+
+  def gitlab_config
+    Gitlab.config.gitlab
+  end
+
+  def gravatar_config
+    Gitlab.config.gravatar
+  end
+
+  def gravatar_url
+    if gitlab_config.https
+      gravatar_config.ssl_url
+    else
+      gravatar_config.plain_url
+    end
+  end
+end