summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTimothy Andrew <mail@timothyandrew.net>2017-04-21 09:47:58 +0000
committerTimothy Andrew <mail@timothyandrew.net>2017-04-25 09:46:05 +0000
commit34b71e734b0b01dd28e18be4728f93fbd4d1a561 (patch)
tree730ad04bf186b803c88d58c4b65e4a15cc9d99e1
parent7d2e2bd3505e27f4b8838a5140af96c1d54d5875 (diff)
downloadgitlab-ce-34b71e734b0b01dd28e18be4728f93fbd4d1a561.tar.gz
Don't display the `is_admin?` flag for user API responses.
- To prevent an attacker from enumerating the `/users` API to get a list of all the admins. - Display the `is_admin?` flag wherever we display the `private_token` - at the moment, there are two instances: - When an admin uses `sudo` to view the `/user` endpoint - When logging in using the `/session` endpoint
-rw-r--r--lib/api/entities.rb4
-rw-r--r--lib/api/session.rb4
-rw-r--r--lib/api/users.rb2
-rw-r--r--spec/fixtures/api/schemas/public_api/v4/user/public.json2
-rw-r--r--spec/requests/api/keys_spec.rb6
-rw-r--r--spec/requests/api/users_spec.rb8
-rw-r--r--spec/requests/api/v3/users_spec.rb6
7 files changed, 23 insertions, 9 deletions
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 64ab6f01eb5..6d6ccefe877 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -14,7 +14,6 @@ module API
class User < UserBasic
expose :created_at
- expose :admin?, as: :is_admin
expose :bio, :location, :skype, :linkedin, :twitter, :website_url, :organization
end
@@ -41,8 +40,9 @@ module API
expose :external
end
- class UserWithPrivateToken < UserPublic
+ class UserWithPrivateDetails < UserPublic
expose :private_token
+ expose :admin?, as: :is_admin
end
class Email < Grape::Entity
diff --git a/lib/api/session.rb b/lib/api/session.rb
index 002ffd1d154..016415c3023 100644
--- a/lib/api/session.rb
+++ b/lib/api/session.rb
@@ -1,7 +1,7 @@
module API
class Session < Grape::API
desc 'Login to get token' do
- success Entities::UserWithPrivateToken
+ success Entities::UserWithPrivateDetails
end
params do
optional :login, type: String, desc: 'The username'
@@ -14,7 +14,7 @@ module API
return unauthorized! unless user
return render_api_error!('401 Unauthorized. You have 2FA enabled. Please use a personal access token to access the API', 401) if user.two_factor_enabled?
- present user, with: Entities::UserWithPrivateToken
+ present user, with: Entities::UserWithPrivateDetails
end
end
end
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 46f221f68fe..40acaebf670 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -433,7 +433,7 @@ module API
success Entities::UserPublic
end
get do
- present current_user, with: sudo? ? Entities::UserWithPrivateToken : Entities::UserPublic
+ present current_user, with: sudo? ? Entities::UserWithPrivateDetails : Entities::UserPublic
end
desc "Get the currently authenticated user's SSH keys" do
diff --git a/spec/fixtures/api/schemas/public_api/v4/user/public.json b/spec/fixtures/api/schemas/public_api/v4/user/public.json
index 5587cfec61a..faa126b65f2 100644
--- a/spec/fixtures/api/schemas/public_api/v4/user/public.json
+++ b/spec/fixtures/api/schemas/public_api/v4/user/public.json
@@ -9,7 +9,6 @@
"avatar_url",
"web_url",
"created_at",
- "is_admin",
"bio",
"location",
"skype",
@@ -43,7 +42,6 @@
"avatar_url": { "type": "string" },
"web_url": { "type": "string" },
"created_at": { "type": "date" },
- "is_admin": { "type": "boolean" },
"bio": { "type": ["string", "null"] },
"location": { "type": ["string", "null"] },
"skype": { "type": "string" },
diff --git a/spec/requests/api/keys_spec.rb b/spec/requests/api/keys_spec.rb
index 4c80987d680..adb33166332 100644
--- a/spec/requests/api/keys_spec.rb
+++ b/spec/requests/api/keys_spec.rb
@@ -34,6 +34,12 @@ describe API::Keys, api: true do
expect(json_response['user']['id']).to eq(user.id)
expect(json_response['user']['username']).to eq(user.username)
end
+
+ it "does not include the user's `is_admin` flag" do
+ get api("/keys/#{key.id}", admin)
+
+ expect(json_response['user']['is_admin']).to be_nil
+ end
end
end
end
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 165ab389917..1db85da5c2c 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -137,6 +137,12 @@ describe API::Users, api: true do
expect(json_response['username']).to eq(user.username)
end
+ it "does not return the user's `is_admin` flag" do
+ get api("/users/#{user.id}", user)
+
+ expect(json_response['is_admin']).to be_nil
+ end
+
it "returns a 401 if unauthenticated" do
get api("/users/9998")
expect(response).to have_http_status(401)
@@ -399,7 +405,6 @@ describe API::Users, api: true do
it "updates admin status" do
put api("/users/#{user.id}", admin), { admin: true }
expect(response).to have_http_status(200)
- expect(json_response['is_admin']).to eq(true)
expect(user.reload.admin).to eq(true)
end
@@ -413,7 +418,6 @@ describe API::Users, api: true do
it "does not update admin status" do
put api("/users/#{admin_user.id}", admin), { can_create_group: false }
expect(response).to have_http_status(200)
- expect(json_response['is_admin']).to eq(true)
expect(admin_user.reload.admin).to eq(true)
expect(admin_user.can_create_group).to eq(false)
end
diff --git a/spec/requests/api/v3/users_spec.rb b/spec/requests/api/v3/users_spec.rb
index b38cbe74b85..19465a9a4ea 100644
--- a/spec/requests/api/v3/users_spec.rb
+++ b/spec/requests/api/v3/users_spec.rb
@@ -276,5 +276,11 @@ describe API::V3::Users, api: true do
expect(new_user).to be_confirmed
end
+
+ it 'does not reveal the `is_admin` flag of the user' do
+ post v3_api('/users', admin), attributes_for(:user)
+
+ expect(json_response['is_admin']).to be_nil
+ end
end
end