Download as
Browse Code »

Commit ae564c97d48bf728745c57720734cb40378fd90f

Authored by Dmitriy Zaporozhets
1 parent d5b0f29c
Exists in spb-stable and in 2 other branches 7-0-stable, 7-0-stable-spb

Dont expose user email via API 

To prevent leaking of users info we reduce amount of user information
retrieved via API for normal users.

What user can get via API:

* if not admin: only id, state, name, username and avatar_url
* if admin: all user information
* about himself: all informaion

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Showing 10 changed files with 53 additions and 48 deletions   Show diff stats
app/assets/javascripts/project_users_select.js.coffee
  Diff comments   View file @ae564c9
@@ -37,13 +37,9 @@ @@ -37,13 +37,9 @@
37 37
38 projectUserFormatResult: (user) -> 38 projectUserFormatResult: (user) ->
39 if user.avatar_url 39 if user.avatar_url
40 - avatar = gon.relative_url_root + user.avatar_url  
41 - else if gon.gravatar_enabled  
42 - avatar = gon.gravatar_url  
43 - avatar = avatar.replace('%{hash}', md5(user.email))  
44 - avatar = avatar.replace('%{size}', '24') 40 + avatar = user.avatar_url
45 else 41 else
46 - avatar = gon.relative_url_root + "#{image_path('no_avatar.png')}" 42 + avatar = gon.default_avatar_url
47 43
48 if user.id == '' 44 if user.id == ''
49 avatarMarkup = '' 45 avatarMarkup = ''
app/assets/javascripts/users_select.js.coffee
  Diff comments   View file @ae564c9
1 $ -> 1 $ ->
2 userFormatResult = (user) -> 2 userFormatResult = (user) ->
3 if user.avatar_url 3 if user.avatar_url
4 - avatar = gon.relative_url_root + user.avatar_url  
5 - else if gon.gravatar_enabled  
6 - avatar = gon.gravatar_url  
7 - avatar = avatar.replace('%{hash}', md5(user.email))  
8 - avatar = avatar.replace('%{size}', '24') 4 + avatar = user.avatar_url
9 else 5 else
10 - avatar = gon.relative_url_root + "#{image_path('no_avatar.png')}" 6 + avatar = gon.default_avatar_url
11 7
12 "<div class='user-result'> 8 "<div class='user-result'>
13 <div class='user-image'><img class='avatar s24' src='#{avatar}'></div> 9 <div class='user-image'><img class='avatar s24' src='#{avatar}'></div>
app/controllers/application_controller.rb
  Diff comments   View file @ae564c9
@@ -164,9 +164,8 @@ class ApplicationController &lt; ActionController::Base @@ -164,9 +164,8 @@ class ApplicationController &lt; ActionController::Base
164 def add_gon_variables 164 def add_gon_variables
165 gon.default_issues_tracker = Project.issues_tracker.default_value 165 gon.default_issues_tracker = Project.issues_tracker.default_value
166 gon.api_version = API::API.version 166 gon.api_version = API::API.version
167 - gon.gravatar_url = request.ssl? || Gitlab.config.gitlab.https ? Gitlab.config.gravatar.ssl_url : Gitlab.config.gravatar.plain_url  
168 gon.relative_url_root = Gitlab.config.gitlab.relative_url_root 167 gon.relative_url_root = Gitlab.config.gitlab.relative_url_root
169 - gon.gravatar_enabled = Gitlab.config.gravatar.enabled 168 + gon.default_avatar_url = URI::join(Gitlab.config.gitlab.url, ActionController::Base.helpers.image_path('no_avatar.png')).to_s
170 169
171 if current_user 170 if current_user
172 gon.current_user_id = current_user.id 171 gon.current_user_id = current_user.id
app/helpers/application_helper.rb
  Diff comments   View file @ae564c9
@@ -60,23 +60,21 @@ module ApplicationHelper @@ -60,23 +60,21 @@ module ApplicationHelper
60 60
61 def avatar_icon(user_email = '', size = nil) 61 def avatar_icon(user_email = '', size = nil)
62 user = User.find_by(email: user_email) 62 user = User.find_by(email: user_email)
63 - if user && user.avatar.present?  
64 - user.avatar.url 63 +
  64 + if user
  65 + user.avatar_url(size) || default_avatar
65 else 66 else
66 gravatar_icon(user_email, size) 67 gravatar_icon(user_email, size)
67 end 68 end
68 end 69 end
69 70
70 def gravatar_icon(user_email = '', size = nil) 71 def gravatar_icon(user_email = '', size = nil)
71 - size = 40 if size.nil? || size <= 0 72 + GravatarService.new.execute(user_email, size) ||
  73 + default_avatar
  74 + end
72 75
73 - if !Gitlab.config.gravatar.enabled || user_email.blank?  
74 - image_path('no_avatar.png')  
75 - else  
76 - gravatar_url = request.ssl? || gitlab_config.https ? Gitlab.config.gravatar.ssl_url : Gitlab.config.gravatar.plain_url  
77 - user_email.strip!  
78 - sprintf gravatar_url, hash: Digest::MD5.hexdigest(user_email.downcase), size: size, email: user_email  
79 - end 76 + def default_avatar
  77 + image_path('no_avatar.png')
80 end 78 end
81 79
82 def last_commit(project) 80 def last_commit(project)
app/models/user.rb
  Diff comments   View file @ae564c9
@@ -482,4 +482,12 @@ class User &lt; ActiveRecord::Base @@ -482,4 +482,12 @@ class User &lt; ActiveRecord::Base
482 def public_profile? 482 def public_profile?
483 authorized_projects.public_only.any? 483 authorized_projects.public_only.any?
484 end 484 end
  485 +
  486 + def avatar_url(size = nil)
  487 + if avatar.present?
  488 + URI::join(Gitlab.config.gitlab.url, avatar.url).to_s
  489 + else
  490 + GravatarService.new.execute(email)
  491 + end
  492 + end
485 end 493 end
lib/api/entities.rb
  Diff comments   View file @ae564c9
1 module API 1 module API
2 module Entities 2 module Entities
3 - class User < Grape::Entity  
4 - expose :id, :username, :email, :name, :bio, :skype, :linkedin, :twitter, :website_url,  
5 - :theme_id, :color_scheme_id, :state, :created_at, :extern_uid, :provider  
6 - expose :is_admin?, as: :is_admin  
7 - expose :can_create_group?, as: :can_create_group  
8 - expose :can_create_project?, as: :can_create_project 3 + class UserSafe < Grape::Entity
  4 + expose :name, :username
  5 + end
9 6
10 - expose :avatar_url do |user, options|  
11 - if user.avatar.present?  
12 - user.avatar.url  
13 - end  
14 - end 7 + class UserBasic < UserSafe
  8 + expose :id, :state, :avatar_url
15 end 9 end
16 10
17 - class UserSafe < Grape::Entity  
18 - expose :name, :username 11 + class User < UserBasic
  12 + expose :created_at
  13 + expose :is_admin?, as: :is_admin
  14 + expose :bio, :skype, :linkedin, :twitter, :website_url
19 end 15 end
20 16
21 - class UserBasic < Grape::Entity  
22 - expose :id, :username, :email, :name, :state, :created_at 17 + class UserFull < User
  18 + expose :email
  19 + expose :theme_id, :color_scheme_id, :extern_uid, :provider
  20 + expose :can_create_group?, as: :can_create_group
  21 + expose :can_create_project?, as: :can_create_project
23 end 22 end
24 23
25 - class UserLogin < User 24 + class UserLogin < UserFull
26 expose :private_token 25 expose :private_token
27 end 26 end
28 27
lib/api/internal.rb
  Diff comments   View file @ae564c9
@@ -59,4 +59,3 @@ module API @@ -59,4 +59,3 @@ module API
59 end 59 end
60 end 60 end
61 end 61 end
62 -  
lib/api/projects.rb
  Diff comments   View file @ae564c9
@@ -209,7 +209,7 @@ module API @@ -209,7 +209,7 @@ module API
209 @users = User.where(id: user_project.team.users.map(&:id)) 209 @users = User.where(id: user_project.team.users.map(&:id))
210 @users = @users.search(params[:search]) if params[:search].present? 210 @users = @users.search(params[:search]) if params[:search].present?
211 @users = paginate @users 211 @users = paginate @users
212 - present @users, with: Entities::User 212 + present @users, with: Entities::UserBasic
213 end 213 end
214 214
215 # Get a project labels 215 # Get a project labels
lib/api/users.rb
  Diff comments   View file @ae564c9
@@ -13,7 +13,12 @@ module API @@ -13,7 +13,12 @@ module API
13 @users = @users.active if params[:active].present? 13 @users = @users.active if params[:active].present?
14 @users = @users.search(params[:search]) if params[:search].present? 14 @users = @users.search(params[:search]) if params[:search].present?
15 @users = paginate @users 15 @users = paginate @users
16 - present @users, with: Entities::User 16 +
  17 + if current_user.is_admin?
  18 + present @users, with: Entities::UserFull
  19 + else
  20 + present @users, with: Entities::UserBasic
  21 + end
17 end 22 end
18 23
19 # Get a single user 24 # Get a single user
@@ -24,7 +29,12 @@ module API @@ -24,7 +29,12 @@ module API
24 # GET /users/:id 29 # GET /users/:id
25 get ":id" do 30 get ":id" do
26 @user = User.find(params[:id]) 31 @user = User.find(params[:id])
27 - present @user, with: Entities::User 32 +
  33 + if current_user.is_admin?
  34 + present @user, with: Entities::UserFull
  35 + else
  36 + present @user, with: Entities::UserBasic
  37 + end
28 end 38 end
29 39
30 # Create user. Available only for admin 40 # Create user. Available only for admin
@@ -53,7 +63,7 @@ module API @@ -53,7 +63,7 @@ module API
53 admin = attrs.delete(:admin) 63 admin = attrs.delete(:admin)
54 user.admin = admin unless admin.nil? 64 user.admin = admin unless admin.nil?
55 if user.save 65 if user.save
56 - present user, with: Entities::User 66 + present user, with: Entities::UserFull
57 else 67 else
58 not_found! 68 not_found!
59 end 69 end
@@ -87,7 +97,7 @@ module API @@ -87,7 +97,7 @@ module API
87 admin = attrs.delete(:admin) 97 admin = attrs.delete(:admin)
88 user.admin = admin unless admin.nil? 98 user.admin = admin unless admin.nil?
89 if user.update_attributes(attrs, as: :admin) 99 if user.update_attributes(attrs, as: :admin)
90 - present user, with: Entities::User 100 + present user, with: Entities::UserFull
91 else 101 else
92 not_found! 102 not_found!
93 end 103 end
spec/requests/api/users_spec.rb
  Diff comments   View file @ae564c9
@@ -20,7 +20,7 @@ describe API::API, api: true do @@ -20,7 +20,7 @@ describe API::API, api: true do
20 get api("/users", user) 20 get api("/users", user)
21 response.status.should == 200 21 response.status.should == 200
22 json_response.should be_an Array 22 json_response.should be_an Array
23 - json_response.first['email'].should == user.email 23 + json_response.first['username'].should == user.username
24 end 24 end
25 end 25 end
26 end 26 end
@@ -29,7 +29,7 @@ describe API::API, api: true do @@ -29,7 +29,7 @@ describe API::API, api: true do
29 it "should return a user by id" do 29 it "should return a user by id" do
30 get api("/users/#{user.id}", user) 30 get api("/users/#{user.id}", user)
31 response.status.should == 200 31 response.status.should == 200
32 - json_response['email'].should == user.email 32 + json_response['username'].should == user.username
33 end 33 end
34 34
35 it "should return a 401 if unauthenticated" do 35 it "should return a 401 if unauthenticated" do