Commit 34b71e73 authored by Timothy Andrew's avatar Timothy Andrew

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
parent 7d2e2bd3
...@@ -14,7 +14,6 @@ module API ...@@ -14,7 +14,6 @@ module API
class User < UserBasic class User < UserBasic
expose :created_at expose :created_at
expose :admin?, as: :is_admin
expose :bio, :location, :skype, :linkedin, :twitter, :website_url, :organization expose :bio, :location, :skype, :linkedin, :twitter, :website_url, :organization
end end
...@@ -41,8 +40,9 @@ module API ...@@ -41,8 +40,9 @@ module API
expose :external expose :external
end end
class UserWithPrivateToken < UserPublic class UserWithPrivateDetails < UserPublic
expose :private_token expose :private_token
expose :admin?, as: :is_admin
end end
class Email < Grape::Entity class Email < Grape::Entity
......
module API module API
class Session < Grape::API class Session < Grape::API
desc 'Login to get token' do desc 'Login to get token' do
success Entities::UserWithPrivateToken success Entities::UserWithPrivateDetails
end end
params do params do
optional :login, type: String, desc: 'The username' optional :login, type: String, desc: 'The username'
...@@ -14,7 +14,7 @@ module API ...@@ -14,7 +14,7 @@ module API
return unauthorized! unless user 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? 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 end
end end
...@@ -433,7 +433,7 @@ module API ...@@ -433,7 +433,7 @@ module API
success Entities::UserPublic success Entities::UserPublic
end end
get do get do
present current_user, with: sudo? ? Entities::UserWithPrivateToken : Entities::UserPublic present current_user, with: sudo? ? Entities::UserWithPrivateDetails : Entities::UserPublic
end end
desc "Get the currently authenticated user's SSH keys" do desc "Get the currently authenticated user's SSH keys" do
......
...@@ -9,7 +9,6 @@ ...@@ -9,7 +9,6 @@
"avatar_url", "avatar_url",
"web_url", "web_url",
"created_at", "created_at",
"is_admin",
"bio", "bio",
"location", "location",
"skype", "skype",
...@@ -43,7 +42,6 @@ ...@@ -43,7 +42,6 @@
"avatar_url": { "type": "string" }, "avatar_url": { "type": "string" },
"web_url": { "type": "string" }, "web_url": { "type": "string" },
"created_at": { "type": "date" }, "created_at": { "type": "date" },
"is_admin": { "type": "boolean" },
"bio": { "type": ["string", "null"] }, "bio": { "type": ["string", "null"] },
"location": { "type": ["string", "null"] }, "location": { "type": ["string", "null"] },
"skype": { "type": "string" }, "skype": { "type": "string" },
......
...@@ -34,6 +34,12 @@ describe API::Keys, api: true do ...@@ -34,6 +34,12 @@ describe API::Keys, api: true do
expect(json_response['user']['id']).to eq(user.id) expect(json_response['user']['id']).to eq(user.id)
expect(json_response['user']['username']).to eq(user.username) expect(json_response['user']['username']).to eq(user.username)
end 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 end
end end
...@@ -137,6 +137,12 @@ describe API::Users, api: true do ...@@ -137,6 +137,12 @@ describe API::Users, api: true do
expect(json_response['username']).to eq(user.username) expect(json_response['username']).to eq(user.username)
end 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 it "returns a 401 if unauthenticated" do
get api("/users/9998") get api("/users/9998")
expect(response).to have_http_status(401) expect(response).to have_http_status(401)
...@@ -399,7 +405,6 @@ describe API::Users, api: true do ...@@ -399,7 +405,6 @@ describe API::Users, api: true do
it "updates admin status" do it "updates admin status" do
put api("/users/#{user.id}", admin), { admin: true } put api("/users/#{user.id}", admin), { admin: true }
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(json_response['is_admin']).to eq(true)
expect(user.reload.admin).to eq(true) expect(user.reload.admin).to eq(true)
end end
...@@ -413,7 +418,6 @@ describe API::Users, api: true do ...@@ -413,7 +418,6 @@ describe API::Users, api: true do
it "does not update admin status" do it "does not update admin status" do
put api("/users/#{admin_user.id}", admin), { can_create_group: false } put api("/users/#{admin_user.id}", admin), { can_create_group: false }
expect(response).to have_http_status(200) 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.reload.admin).to eq(true)
expect(admin_user.can_create_group).to eq(false) expect(admin_user.can_create_group).to eq(false)
end end
......
...@@ -276,5 +276,11 @@ describe API::V3::Users, api: true do ...@@ -276,5 +276,11 @@ describe API::V3::Users, api: true do
expect(new_user).to be_confirmed expect(new_user).to be_confirmed
end 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
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment