diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-03 01:30:45 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-03 01:30:45 +0300 |
commit | 66e26f113a97590b3cb49db87eec749312073c90 (patch) | |
tree | 4684e536e62b0b963f5d19ab29d04fed07bf4811 /spec | |
parent | 42a3f91387d2e1e8914b76817a8be34f67d9b84c (diff) |
Add latest changes from gitlab-org/security/gitlab@13-12-stable-ee
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/invites_controller_spec.rb | 84 | ||||
-rw-r--r-- | spec/controllers/projects/pipelines_controller_spec.rb | 49 | ||||
-rw-r--r-- | spec/features/invites_spec.rb | 39 | ||||
-rw-r--r-- | spec/features/projects/pipelines/pipeline_spec.rb | 5 | ||||
-rw-r--r-- | spec/frontend/members/store/mutations_spec.js | 6 | ||||
-rw-r--r-- | spec/policies/personal_access_token_policy_spec.rb | 7 | ||||
-rw-r--r-- | spec/requests/api/personal_access_tokens_spec.rb | 12 |
7 files changed, 112 insertions, 90 deletions
diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index 6b94d186d5f..bca59813a23 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -24,9 +24,64 @@ RSpec.describe InvitesController do end end + shared_examples 'invite email match enforcement' do |error_status:, flash_alert: nil| + it 'accepts user if invite email matches signed in user' do + expect do + request + end.to change { project_members.include?(user) }.from(false).to(true) + + expect(response).to have_gitlab_http_status(:found) + expect(flash[:notice]).to include 'You have been granted' + end + + it 'accepts invite if invite email matches confirmed secondary email' do + secondary_email = create(:email, :confirmed, user: user) + member.update!(invite_email: secondary_email.email) + + expect do + request + end.to change { project_members.include?(user) }.from(false).to(true) + + expect(response).to have_gitlab_http_status(:found) + expect(flash[:notice]).to include 'You have been granted' + end + + it 'does not accept if invite email matches unconfirmed secondary email' do + secondary_email = create(:email, user: user) + member.update!(invite_email: secondary_email.email) + + expect do + request + end.not_to change { project_members.include?(user) } + + expect(response).to have_gitlab_http_status(error_status) + expect(flash[:alert]).to eq(flash_alert) + end + + it 'does not accept if invite email does not match signed in user' do + member.update!(invite_email: 'bogus@email.com') + + expect do + request + end.not_to change { project_members.include?(user) } + + expect(response).to have_gitlab_http_status(error_status) + expect(flash[:alert]).to eq(flash_alert) + end + end + describe 'GET #show' do subject(:request) { get :show, params: params } + context 'when logged in' do + before do + sign_in(user) + end + + it_behaves_like 'invite email match enforcement', error_status: :ok + it_behaves_like 'invalid token' + end + context 'when it is part of our invite email experiment' do let(:extra_params) { { invite_type: 'initial_email' } } @@ -58,34 +113,6 @@ RSpec.describe InvitesController do end end - context 'when logged in' do - before do - sign_in(user) - end - - it 'accepts user if invite email matches signed in user' do - expect do - request - end.to change { project_members.include?(user) }.from(false).to(true) - - expect(response).to have_gitlab_http_status(:found) - expect(flash[:notice]).to include 'You have been granted' - end - - it 'forces re-confirmation if email does not match signed in user' do - member.update!(invite_email: 'bogus@email.com') - - expect do - request - end.not_to change { project_members.include?(user) } - - expect(response).to have_gitlab_http_status(:ok) - expect(flash[:notice]).to be_nil - end - - it_behaves_like 'invalid token' - end - context 'when not logged in' do context 'when invite token belongs to a valid member' do context 'when instance allows sign up' do @@ -239,6 +266,7 @@ RSpec.describe InvitesController do subject(:request) { post :accept, params: params } + it_behaves_like 'invite email match enforcement', error_status: :redirect, flash_alert: 'The invitation could not be accepted.' it_behaves_like 'invalid token' end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 0e6b5e84d85..7faf6412944 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -302,35 +302,46 @@ RSpec.describe Projects::PipelinesController do end describe 'GET #show' do - render_views - - let_it_be(:pipeline) { create(:ci_pipeline, project: project) } - - subject { get_pipeline_html } - def get_pipeline_html get :show, params: { namespace_id: project.namespace, project_id: project, id: pipeline }, format: :html end - def create_build_with_artifacts(stage, stage_idx, name) - create(:ci_build, :artifacts, :tags, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name) - end + context 'when the project is public' do + render_views - before do - create_build_with_artifacts('build', 0, 'job1') - create_build_with_artifacts('build', 0, 'job2') + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + + def create_build_with_artifacts(stage, stage_idx, name) + create(:ci_build, :artifacts, :tags, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name) + end + + before do + create_build_with_artifacts('build', 0, 'job1') + create_build_with_artifacts('build', 0, 'job2') + end + + it 'avoids N+1 database queries', :request_store do + control_count = ActiveRecord::QueryRecorder.new { get_pipeline_html }.count + expect(response).to have_gitlab_http_status(:ok) + + create_build_with_artifacts('build', 0, 'job3') + + expect { get_pipeline_html }.not_to exceed_query_limit(control_count) + expect(response).to have_gitlab_http_status(:ok) + end end - it 'avoids N+1 database queries', :request_store do - get_pipeline_html + context 'when the project is private' do + let(:project) { create(:project, :private, :repository) } + let(:pipeline) { create(:ci_pipeline, project: project) } - control_count = ActiveRecord::QueryRecorder.new { get_pipeline_html }.count - expect(response).to have_gitlab_http_status(:ok) + it 'returns `not_found` when the user does not have access' do + sign_in(create(:user)) - create_build_with_artifacts('build', 0, 'job3') + get_pipeline_html - expect { get_pipeline_html }.not_to exceed_query_limit(control_count) - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:not_found) + end end end diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb index a72cf033d61..be01488e3f2 100644 --- a/spec/features/invites_spec.rb +++ b/spec/features/invites_spec.rb @@ -90,48 +90,17 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do end context 'when signed in and an invite link is clicked' do - context 'when an invite email is a secondary email for the user' do - let(:invite_email) { 'user_secondary@example.com' } - - before do - sign_in(user) - visit invite_path(group_invite.raw_invite_token) - end - - it 'sends user to the invite url and allows them to decline' do - expect(current_path).to eq(invite_path(group_invite.raw_invite_token)) - expect(page).to have_content("Note that this invitation was sent to #{invite_email}") - expect(page).to have_content("but you are signed in as #{user.to_reference} with email #{user.email}") - - click_link('Decline') - - expect(page).to have_content('You have declined the invitation') - expect(current_path).to eq(dashboard_projects_path) - expect { group_invite.reload }.to raise_error ActiveRecord::RecordNotFound - end - - it 'sends uer to the invite url and allows them to accept' do - expect(current_path).to eq(invite_path(group_invite.raw_invite_token)) - expect(page).to have_content("Note that this invitation was sent to #{invite_email}") - expect(page).to have_content("but you are signed in as #{user.to_reference} with email #{user.email}") - - click_link('Accept invitation') - - expect(page).to have_content('You have been granted') - expect(current_path).to eq(activity_group_path(group)) - end - end - context 'when user is an existing member' do before do - sign_in(owner) + group.add_developer(user) + sign_in(user) visit invite_path(group_invite.raw_invite_token) end it 'shows message user already a member' do expect(current_path).to eq(invite_path(group_invite.raw_invite_token)) - expect(page).to have_link(owner.name, href: user_url(owner)) - expect(page).to have_content('However, you are already a member of this group.') + expect(page).to have_link(user.name, href: user_path(user)) + expect(page).to have_content('You are already a member of this group.') end end end diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 70dc0bd04e8..b93cbddf553 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -361,9 +361,8 @@ RSpec.describe 'Pipeline', :js do let(:project) { create(:project, :public, :repository, public_builds: false) } let(:role) { :guest } - it 'does not show failed jobs tab pane' do - expect(page).to have_link('Pipeline') - expect(page).not_to have_content('Failed Jobs') + it 'does not show the pipeline details page' do + expect(page).to have_content('Not Found') end end end diff --git a/spec/frontend/members/store/mutations_spec.js b/spec/frontend/members/store/mutations_spec.js index 7ad7034eb6d..78bbad394a0 100644 --- a/spec/frontend/members/store/mutations_spec.js +++ b/spec/frontend/members/store/mutations_spec.js @@ -44,8 +44,7 @@ describe('Vuex members mutations', () => { describe('when error has a message', () => { it('shows error message', () => { const error = new Error('Request failed with status code 422'); - const message = - 'User email "john.smith@gmail.com" does not match the allowed domain of example.com'; + const message = 'User email does not match the allowed domain of example.com'; error.response = { data: { message }, @@ -88,8 +87,7 @@ describe('Vuex members mutations', () => { describe('when error has a message', () => { it('shows error message', () => { const error = new Error('Request failed with status code 422'); - const message = - 'User email "john.smith@gmail.com" does not match the allowed domain of example.com'; + const message = 'User email does not match the allowed domain of example.com'; error.response = { data: { message }, diff --git a/spec/policies/personal_access_token_policy_spec.rb b/spec/policies/personal_access_token_policy_spec.rb index b5e8d40b133..e146133429b 100644 --- a/spec/policies/personal_access_token_policy_spec.rb +++ b/spec/policies/personal_access_token_policy_spec.rb @@ -41,6 +41,13 @@ RSpec.describe PersonalAccessTokenPolicy do it { is_expected.to be_allowed(:read_token) } it { is_expected.to be_allowed(:revoke_token) } end + + context 'subject of the impersonated token' do + let_it_be(:token) { build_stubbed(:personal_access_token, user: current_user, impersonation: true) } + + it { is_expected.to be_disallowed(:read_token) } + it { is_expected.to be_disallowed(:revoke_token) } + end end context 'current_user is a blocked administrator', :enable_admin_mode do diff --git a/spec/requests/api/personal_access_tokens_spec.rb b/spec/requests/api/personal_access_tokens_spec.rb index ccc5f322ff9..0ff2c46e693 100644 --- a/spec/requests/api/personal_access_tokens_spec.rb +++ b/spec/requests/api/personal_access_tokens_spec.rb @@ -6,6 +6,7 @@ RSpec.describe API::PersonalAccessTokens do let_it_be(:path) { '/personal_access_tokens' } let_it_be(:token1) { create(:personal_access_token) } let_it_be(:token2) { create(:personal_access_token) } + let_it_be(:token_impersonated) { create(:personal_access_token, impersonation: true, user: token1.user) } let_it_be(:current_user) { create(:user) } describe 'GET /personal_access_tokens' do @@ -24,8 +25,9 @@ RSpec.describe API::PersonalAccessTokens do get api(path, current_user), params: { user_id: token1.user.id } expect(response).to have_gitlab_http_status(:ok) - expect(json_response.count).to eq(1) + expect(json_response.count).to eq(2) expect(json_response.first['user_id']).to eq(token1.user.id) + expect(json_response.last['id']).to eq(token_impersonated.id) end end @@ -34,6 +36,7 @@ RSpec.describe API::PersonalAccessTokens do let_it_be(:user) { create(:user) } let_it_be(:token) { create(:personal_access_token, user: current_user)} let_it_be(:other_token) { create(:personal_access_token, user: user) } + let_it_be(:token_impersonated) { create(:personal_access_token, impersonation: true, user: current_user) } it 'returns all PATs belonging to the signed-in user' do get api(path, current_user, personal_access_token: token) @@ -95,6 +98,7 @@ RSpec.describe API::PersonalAccessTokens do context 'when current_user is not an administrator' do let_it_be(:user_token) { create(:personal_access_token, user: current_user) } let_it_be(:user_token_path) { "/personal_access_tokens/#{user_token.id}" } + let_it_be(:token_impersonated) { create(:personal_access_token, impersonation: true, user: current_user) } it 'fails revokes a different users token' do delete api(path, current_user) @@ -107,6 +111,12 @@ RSpec.describe API::PersonalAccessTokens do expect(response).to have_gitlab_http_status(:no_content) end + + it 'cannot revoke impersonation token' do + delete api("/personal_access_tokens/#{token_impersonated.id}", current_user) + + expect(response).to have_gitlab_http_status(:bad_request) + end end end end |