diff options
-rw-r--r-- | doc/api/users.md | 4 | ||||
-rw-r--r-- | lib/api/users.rb | 8 | ||||
-rw-r--r-- | spec/requests/api/users_spec.rb | 171 |
3 files changed, 96 insertions, 87 deletions
diff --git a/doc/api/users.md b/doc/api/users.md index dfd2f6cc87d..4ec609e62e9 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -259,7 +259,7 @@ GET /users?with_custom_attributes=true ## Single user -Get a single user. This endpoint can be accessed without authentication. +Get a single user. ### For user @@ -806,7 +806,7 @@ Example response: ### Followers and following -Get the followers of a user. This endpoint can be accessed without authentication. +Get the followers of a user. ```plaintext GET /users/:id/followers diff --git a/lib/api/users.rb b/lib/api/users.rb index e3271b8b9b2..944be990c2f 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -140,7 +140,10 @@ module API end # rubocop: disable CodeReuse/ActiveRecord get ":id", feature_category: :users do + forbidden!('Not authorized!') unless current_user + user = User.find_by(id: params[:id]) + not_found!('User') unless user && can?(current_user, :read_user, user) opts = { with: current_user&.admin? ? Entities::UserDetailsWithAdmin : Entities::User, current_user: current_user } @@ -156,6 +159,7 @@ module API end get ":user_id/status", requirements: API::USER_REQUIREMENTS, feature_category: :users do user = find_user(params[:user_id]) + not_found!('User') unless user && can?(current_user, :read_user, user) present user.status || {}, with: Entities::UserStatus @@ -203,6 +207,8 @@ module API use :pagination end get ':id/following', feature_category: :users do + forbidden!('Not authorized!') unless current_user + user = find_user(params[:id]) not_found!('User') unless user && can?(current_user, :read_user_profile, user) @@ -217,6 +223,8 @@ module API use :pagination end get ':id/followers', feature_category: :users do + forbidden!('Not authorized!') unless current_user + user = find_user(params[:id]) not_found!('User') unless user && can?(current_user, :read_user_profile, user) diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 527e548ad19..ee1911b0a26 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -53,37 +53,6 @@ RSpec.describe API::Users do end end - describe 'GET /users/:id' do - context 'when unauthenticated' do - it 'does not contain the note of the user' do - get api("/users/#{user.id}") - - expect(json_response).not_to have_key('note') - end - end - - context 'when authenticated' do - context 'as an admin' do - it 'contains the note of the user' do - get api("/users/#{user.id}", admin) - - expect(json_response).to have_key('note') - expect(json_response['note']).to eq(user.note) - expect(json_response).to have_key('sign_in_count') - end - end - - context 'as a regular user' do - it 'does not contain the note of the user' do - get api("/users/#{user.id}", user) - - expect(json_response).not_to have_key('note') - expect(json_response).not_to have_key('sign_in_count') - end - end - end - end - describe "PUT /users/:id" do context 'when user is an admin' do it "updates note of the user" do @@ -527,6 +496,8 @@ RSpec.describe API::Users do end describe "GET /users/:id" do + let_it_be(:user2, reload: true) { create(:user, username: 'another_user') } + it "returns a user by id" do get api("/users/#{user.id}", user) @@ -564,6 +535,64 @@ RSpec.describe API::Users do expect(json_response.keys).not_to include 'trial' end + it 'returns a 404 if the target user is present but inaccessible' do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :read_user, user2).and_return(false) + + get api("/users/#{user2.id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns the `created_at` field for public users' do + get api("/users/#{user2.id}", user) + + expect(response).to match_response_schema('public_api/v4/user/basic') + expect(json_response.keys).to include('created_at') + end + + it 'does not return the `created_at` field for private users' do + get api("/users/#{private_user.id}", user) + + expect(response).to match_response_schema('public_api/v4/user/basic') + expect(json_response.keys).not_to include('created_at') + end + + it 'returns the `followers` field for public users' do + get api("/users/#{user2.id}", user) + + expect(response).to match_response_schema('public_api/v4/user/basic') + expect(json_response.keys).to include('followers') + end + + it 'does not return the `followers` field for private users' do + get api("/users/#{private_user.id}", user) + + expect(response).to match_response_schema('public_api/v4/user/basic') + expect(json_response.keys).not_to include('followers') + end + + it 'returns the `following` field for public users' do + get api("/users/#{user2.id}", user) + + expect(response).to match_response_schema('public_api/v4/user/basic') + expect(json_response.keys).to include('following') + end + + it 'does not return the `following` field for private users' do + get api("/users/#{private_user.id}", user) + + expect(response).to match_response_schema('public_api/v4/user/basic') + expect(json_response.keys).not_to include('following') + end + + it 'does not contain the note of the user' do + get api("/users/#{user.id}", user) + + expect(json_response).not_to have_key('note') + expect(json_response).not_to have_key('sign_in_count') + end + context 'when job title is present' do let(:job_title) { 'Fullstack Engineer' } @@ -580,6 +609,14 @@ RSpec.describe API::Users do end context 'when authenticated as admin' do + it 'contains the note of the user' do + get api("/users/#{user.id}", admin) + + expect(json_response).to have_key('note') + expect(json_response['note']).to eq(user.note) + expect(json_response).to have_key('sign_in_count') + end + it 'includes the `is_admin` field' do get api("/users/#{user.id}", admin) @@ -640,62 +677,10 @@ RSpec.describe API::Users do end context 'for an anonymous user' do - it "returns a user by id" do - get api("/users/#{user.id}") - - expect(response).to match_response_schema('public_api/v4/user/basic') - expect(json_response['username']).to eq(user.username) - end - - it "returns a 404 if the target user is present but inaccessible" do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?).with(nil, :read_user, user).and_return(false) - - get api("/users/#{user.id}") - - expect(response).to have_gitlab_http_status(:not_found) - end - - it "returns the `created_at` field for public users" do - get api("/users/#{user.id}") - - expect(response).to match_response_schema('public_api/v4/user/basic') - expect(json_response.keys).to include 'created_at' - end - - it "does not return the `created_at` field for private users" do - get api("/users/#{private_user.id}") - - expect(response).to match_response_schema('public_api/v4/user/basic') - expect(json_response.keys).not_to include 'created_at' - end - - it "returns the `followers` field for public users" do - get api("/users/#{user.id}") - - expect(response).to match_response_schema('public_api/v4/user/basic') - expect(json_response.keys).to include 'followers' - end - - it "does not return the `followers` field for private users" do - get api("/users/#{private_user.id}") - - expect(response).to match_response_schema('public_api/v4/user/basic') - expect(json_response.keys).not_to include 'followers' - end - - it "returns the `following` field for public users" do + it 'returns 403' do get api("/users/#{user.id}") - expect(response).to match_response_schema('public_api/v4/user/basic') - expect(json_response.keys).to include 'following' - end - - it "does not return the `following` field for private users" do - get api("/users/#{private_user.id}") - - expect(response).to match_response_schema('public_api/v4/user/basic') - expect(json_response.keys).not_to include 'following' + expect(response).to have_gitlab_http_status(:forbidden) end end @@ -788,6 +773,14 @@ RSpec.describe API::Users do describe 'GET /users/:id/followers' do let(:follower) { create(:user) } + context 'for an anonymous user' do + it 'returns 403' do + get api("/users/#{user.id}") + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + context 'user has followers' do it 'lists followers' do follower.follow(user) @@ -823,6 +816,14 @@ RSpec.describe API::Users do describe 'GET /users/:id/following' do let(:followee) { create(:user) } + context 'for an anonymous user' do + it 'returns 403' do + get api("/users/#{user.id}") + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + context 'user has followers' do it 'lists following user' do user.follow(followee) |