diff options
Diffstat (limited to 'spec/requests')
85 files changed, 1991 insertions, 527 deletions
diff --git a/spec/requests/abuse_reports_controller_spec.rb b/spec/requests/abuse_reports_controller_spec.rb new file mode 100644 index 00000000000..94c80ccb89a --- /dev/null +++ b/spec/requests/abuse_reports_controller_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe AbuseReportsController do + let(:reporter) { create(:user) } + let(:user) { create(:user) } + let(:attrs) do + attributes_for(:abuse_report) do |hash| + hash[:user_id] = user.id + end + end + + before do + sign_in(reporter) + end + + describe 'GET new' do + context 'when the user has already been deleted' do + it 'redirects the reporter to root_path' do + user_id = user.id + user.destroy! + + get new_abuse_report_path(user_id: user_id) + + expect(response).to redirect_to root_path + expect(flash[:alert]).to eq(_('Cannot create the abuse report. The user has been deleted.')) + end + end + + context 'when the user has already been blocked' do + it 'redirects the reporter to the user\'s profile' do + user.block + + get new_abuse_report_path(user_id: user.id) + + expect(response).to redirect_to user + expect(flash[:alert]).to eq(_('Cannot create the abuse report. This user has been blocked.')) + end + end + end + + describe 'POST create' do + context 'with valid attributes' do + it 'saves the abuse report' do + expect do + post abuse_reports_path(abuse_report: attrs) + end.to change { AbuseReport.count }.by(1) + end + + it 'calls notify' do + expect_next_instance_of(AbuseReport) do |instance| + expect(instance).to receive(:notify) + end + + post abuse_reports_path(abuse_report: attrs) + end + + it 'redirects back to root' do + post abuse_reports_path(abuse_report: attrs) + + expect(response).to redirect_to root_path + end + end + + context 'with invalid attributes' do + it 'redirects back to root' do + attrs.delete(:user_id) + post abuse_reports_path(abuse_report: attrs) + + expect(response).to redirect_to root_path + end + end + end +end diff --git a/spec/requests/admin/background_migrations_controller_spec.rb b/spec/requests/admin/background_migrations_controller_spec.rb index c7d5d5cae08..67c9c4df827 100644 --- a/spec/requests/admin/background_migrations_controller_spec.rb +++ b/spec/requests/admin/background_migrations_controller_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do let(:migration) { create(:batched_background_migration, status: 'failed') } before do - create(:batched_background_migration_job, batched_migration: migration, batch_size: 10, min_value: 6, max_value: 15, status: :failed, attempts: 3) + create(:batched_background_migration_job, :failed, batched_migration: migration, batch_size: 10, min_value: 6, max_value: 15, attempts: 3) allow_next_instance_of(Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchingStrategy) do |batch_class| allow(batch_class).to receive(:next_batch).with(anything, anything, batch_min_value: 6, batch_size: 5).and_return([6, 10]) diff --git a/spec/requests/api/api_spec.rb b/spec/requests/api/api_spec.rb index 6a02f81fcae..df9be2616c5 100644 --- a/spec/requests/api/api_spec.rb +++ b/spec/requests/api/api_spec.rb @@ -102,7 +102,7 @@ RSpec.describe API::API do describe 'logging', :aggregate_failures do let_it_be(:project) { create(:project, :public) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } context 'when the endpoint is handled by the application' do context 'when the endpoint supports all possible fields' do diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index ad517a05533..780e45cf443 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -188,6 +188,24 @@ RSpec.describe API::Branches do end end + context 'when sort parameter is passed' do + it 'sorts branches' do + get api(route, user), params: { sort: 'name_asc', per_page: 10 } + + sorted_branch_names = json_response.map { |branch| branch['name'] } + + project_branch_names = project.repository.branch_names.sort.take(10) + + expect(sorted_branch_names).to eq(project_branch_names) + end + + context 'when sort value is not supported' do + it_behaves_like '400 response' do + let(:request) { get api(route, user), params: { sort: 'unknown' }} + end + end + end + context 'when unauthenticated', 'and project is public' do before do project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index 13838cffd76..1b87a5e24f5 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -988,7 +988,7 @@ RSpec.describe API::Ci::Pipelines do describe 'DELETE /projects/:id/pipelines/:pipeline_id' do context 'authorized user' do - let(:owner) { project.owner } + let(:owner) { project.first_owner } it 'destroys the pipeline' do delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) diff --git a/spec/requests/api/ci/runner/runners_post_spec.rb b/spec/requests/api/ci/runner/runners_post_spec.rb index 530b601add9..5eb5d3977a3 100644 --- a/spec/requests/api/ci/runner/runners_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_post_spec.rb @@ -30,11 +30,11 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do post api('/runners'), params: { token: 'valid token', description: 'server.hostname', - maintainer_note: 'Some maintainer notes', + maintenance_note: 'Some maintainer notes', run_untagged: false, tag_list: 'tag1, tag2', locked: true, - active: true, + paused: false, access_level: 'ref_protected', maximum_timeout: 9000 } @@ -46,7 +46,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do allow_next_instance_of(::Ci::RegisterRunnerService) do |service| expected_params = { description: 'server.hostname', - maintainer_note: 'Some maintainer notes', + maintenance_note: 'Some maintainer notes', run_untagged: false, tag_list: %w(tag1 tag2), locked: true, @@ -55,19 +55,33 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do maximum_timeout: 9000 }.stringify_keys - allow(service).to receive(:execute) + expect(service).to receive(:execute) .once .with('valid token', a_hash_including(expected_params)) .and_return(new_runner) end end - it 'creates runner' do - request + context 'when token_expires_at is nil' do + it 'creates runner' do + request - expect(response).to have_gitlab_http_status(:created) - expect(json_response['id']).to eq(new_runner.id) - expect(json_response['token']).to eq(new_runner.token) + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to eq({ 'id' => new_runner.id, 'token' => new_runner.token, 'token_expires_at' => nil }) + end + end + + context 'when token_expires_at is a valid date' do + before do + new_runner.token_expires_at = DateTime.new(2022, 1, 11, 14, 39, 24) + end + + it 'creates runner' do + request + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to eq({ 'id' => new_runner.id, 'token' => new_runner.token, 'token_expires_at' => '2022-01-11T14:39:24.000Z' }) + end end it_behaves_like 'storing arguments in the application context for the API' do @@ -81,6 +95,59 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end end + context 'when deprecated maintainer_note field is provided' do + RSpec::Matchers.define_negated_matcher :excluding, :include + + def request + post api('/runners'), params: { + token: 'valid token', + maintainer_note: 'Some maintainer notes' + } + end + + let(:new_runner) { create(:ci_runner) } + + it 'converts to maintenance_note param' do + allow_next_instance_of(::Ci::RegisterRunnerService) do |service| + expect(service).to receive(:execute) + .once + .with('valid token', a_hash_including('maintenance_note' => 'Some maintainer notes') + .and(excluding('maintainter_note' => anything))) + .and_return(new_runner) + end + + request + + expect(response).to have_gitlab_http_status(:created) + end + end + + context 'when deprecated active parameter is provided' do + def request + post api('/runners'), params: { + token: 'valid token', + active: false + } + end + + let_it_be(:new_runner) { create(:ci_runner) } + + it 'uses active value in registration' do + expect_next_instance_of(::Ci::RegisterRunnerService) do |service| + expected_params = { active: false }.stringify_keys + + expect(service).to receive(:execute) + .once + .with('valid token', a_hash_including(expected_params)) + .and_return(new_runner) + end + + request + + expect(response).to have_gitlab_http_status(:created) + end + end + context 'calling actual register service' do include StubGitlabCalls diff --git a/spec/requests/api/ci/runner/runners_verify_post_spec.rb b/spec/requests/api/ci/runner/runners_verify_post_spec.rb index 4680076acae..038e126deaa 100644 --- a/spec/requests/api/ci/runner/runners_verify_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_verify_post_spec.rb @@ -49,6 +49,30 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do let(:expected_params) { { client_id: "runner/#{runner.id}" } } end end + + context 'when non-expired token is provided' do + subject { post api('/runners/verify'), params: { token: runner.token } } + + it 'verifies Runner credentials' do + runner["token_expires_at"] = 10.days.from_now + runner.save! + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when expired token is provided' do + subject { post api('/runners/verify'), params: { token: runner.token } } + + it 'does not verify Runner credentials' do + runner["token_expires_at"] = 10.days.ago + runner.save! + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end end end end diff --git a/spec/requests/api/ci/runners_reset_registration_token_spec.rb b/spec/requests/api/ci/runners_reset_registration_token_spec.rb index df64c0bd22b..e1dc347f8dd 100644 --- a/spec/requests/api/ci/runners_reset_registration_token_spec.rb +++ b/spec/requests/api/ci/runners_reset_registration_token_spec.rb @@ -138,7 +138,7 @@ RSpec.describe API::Ci::Runners do end include_context 'when authorized', 'project' do - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } def get_token project.reload.runners_token diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index 305c0bd9df0..336ce70d8d2 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -86,14 +86,24 @@ RSpec.describe API::Ci::Runners do expect(response).to have_gitlab_http_status(:bad_request) end - it 'filters runners by status' do - create(:ci_runner, :project, :inactive, description: 'Inactive project runner', projects: [project]) + context 'with an inactive runner' do + let_it_be(:runner) { create(:ci_runner, :project, :inactive, description: 'Inactive project runner', projects: [project]) } - get api('/runners?status=paused', user) + it 'filters runners by paused state' do + get api('/runners?paused=true', user) - expect(json_response).to match_array [ - a_hash_including('description' => 'Inactive project runner') - ] + expect(json_response).to match_array [ + a_hash_including('description' => 'Inactive project runner') + ] + end + + it 'filters runners by status' do + get api('/runners?status=paused', user) + + expect(json_response).to match_array [ + a_hash_including('description' => 'Inactive project runner') + ] + end end it 'does not filter by invalid status' do @@ -109,7 +119,7 @@ RSpec.describe API::Ci::Runners do get api('/runners?tag_list=tag1,tag2', user) expect(json_response).to match_array [ - a_hash_including('description' => 'Runner tagged with tag1 and tag2') + a_hash_including('description' => 'Runner tagged with tag1 and tag2', 'active' => true, 'paused' => false) ] end end @@ -137,7 +147,7 @@ RSpec.describe API::Ci::Runners do get api('/runners/all', admin) expect(json_response).to match_array [ - a_hash_including('description' => 'Project runner', 'is_shared' => false, 'runner_type' => 'project_type'), + a_hash_including('description' => 'Project runner', 'is_shared' => false, 'active' => true, 'paused' => false, 'runner_type' => 'project_type'), a_hash_including('description' => 'Two projects runner', 'is_shared' => false, 'runner_type' => 'project_type'), a_hash_including('description' => 'Group runner A', 'is_shared' => false, 'runner_type' => 'group_type'), a_hash_including('description' => 'Group runner B', 'is_shared' => false, 'runner_type' => 'group_type'), @@ -199,14 +209,24 @@ RSpec.describe API::Ci::Runners do expect(response).to have_gitlab_http_status(:bad_request) end - it 'filters runners by status' do - create(:ci_runner, :project, :inactive, description: 'Inactive project runner', projects: [project]) + context 'with an inactive runner' do + let_it_be(:runner) { create(:ci_runner, :project, :inactive, description: 'Inactive project runner', projects: [project]) } - get api('/runners/all?status=paused', admin) + it 'filters runners by status' do + get api('/runners/all?paused=true', admin) - expect(json_response).to match_array [ - a_hash_including('description' => 'Inactive project runner') - ] + expect(json_response).to match_array [ + a_hash_including('description' => 'Inactive project runner') + ] + end + + it 'filters runners by status' do + get api('/runners/all?status=paused', admin) + + expect(json_response).to match_array [ + a_hash_including('description' => 'Inactive project runner') + ] + end end it 'does not filter by invalid status' do @@ -255,6 +275,8 @@ RSpec.describe API::Ci::Runners do expect(json_response['description']).to eq(shared_runner.description) expect(json_response['maximum_timeout']).to be_nil expect(json_response['status']).to eq("not_connected") + expect(json_response['active']).to eq(true) + expect(json_response['paused']).to eq(false) end end @@ -359,6 +381,14 @@ RSpec.describe API::Ci::Runners do expect(shared_runner.reload.active).to eq(!active) end + it 'runner paused state' do + active = shared_runner.active + update_runner(shared_runner.id, admin, paused: active) + + expect(response).to have_gitlab_http_status(:ok) + expect(shared_runner.reload.active).to eq(!active) + end + it 'runner tag list' do update_runner(shared_runner.id, admin, tag_list: ['ruby2.1', 'pgsql', 'mysql']) @@ -500,6 +530,10 @@ RSpec.describe API::Ci::Runners do context 'admin user' do context 'when runner is shared' do it 'deletes runner' do + expect_next_instance_of(Ci::UnregisterRunnerService, shared_runner) do |service| + expect(service).to receive(:execute).once.and_call_original + end + expect do delete api("/runners/#{shared_runner.id}", admin) @@ -514,6 +548,10 @@ RSpec.describe API::Ci::Runners do context 'when runner is not shared' do it 'deletes used project runner' do + expect_next_instance_of(Ci::UnregisterRunnerService, project_runner) do |service| + expect(service).to receive(:execute).once.and_call_original + end + expect do delete api("/runners/#{project_runner.id}", admin) @@ -523,6 +561,10 @@ RSpec.describe API::Ci::Runners do end it 'returns 404 if runner does not exist' do + allow_next_instance_of(Ci::UnregisterRunnerService) do |service| + expect(service).not_to receive(:execute) + end + delete api('/runners/0', admin) expect(response).to have_gitlab_http_status(:not_found) @@ -604,6 +646,10 @@ RSpec.describe API::Ci::Runners do context 'unauthorized user' do it 'does not delete project runner' do + allow_next_instance_of(Ci::UnregisterRunnerService) do |service| + expect(service).not_to receive(:execute) + end + delete api("/runners/#{project_runner.id}") expect(response).to have_gitlab_http_status(:unauthorized) @@ -618,7 +664,7 @@ RSpec.describe API::Ci::Runners do post api("/runners/#{shared_runner.id}/reset_authentication_token", admin) expect(response).to have_gitlab_http_status(:success) - expect(json_response).to eq({ 'token' => shared_runner.reload.token }) + expect(json_response).to eq({ 'token' => shared_runner.reload.token, 'token_expires_at' => nil }) end.to change { shared_runner.reload.token } end @@ -642,7 +688,7 @@ RSpec.describe API::Ci::Runners do post api("/runners/#{project_runner.id}/reset_authentication_token", user) expect(response).to have_gitlab_http_status(:success) - expect(json_response).to eq({ 'token' => project_runner.reload.token }) + expect(json_response).to eq({ 'token' => project_runner.reload.token, 'token_expires_at' => nil }) end.to change { project_runner.reload.token } end @@ -683,7 +729,22 @@ RSpec.describe API::Ci::Runners do post api("/runners/#{group_runner_a.id}/reset_authentication_token", user) expect(response).to have_gitlab_http_status(:success) - expect(json_response).to eq({ 'token' => group_runner_a.reload.token }) + expect(json_response).to eq({ 'token' => group_runner_a.reload.token, 'token_expires_at' => nil }) + end.to change { group_runner_a.reload.token } + end + + it 'resets group runner authentication token with owner access with expiration time', :freeze_time do + expect(group_runner_a.reload.token_expires_at).to be_nil + + group.update!(runner_token_expiration_interval: 5.days) + + expect do + post api("/runners/#{group_runner_a.id}/reset_authentication_token", user) + group_runner_a.reload + + expect(response).to have_gitlab_http_status(:success) + expect(json_response).to eq({ 'token' => group_runner_a.token, 'token_expires_at' => group_runner_a.token_expires_at.iso8601(3) }) + expect(group_runner_a.token_expires_at).to eq(5.days.from_now) end.to change { group_runner_a.reload.token } end end @@ -908,9 +969,9 @@ RSpec.describe API::Ci::Runners do get api("/projects/#{project.id}/runners", user) expect(json_response).to match_array [ - a_hash_including('description' => 'Project runner'), - a_hash_including('description' => 'Two projects runner'), - a_hash_including('description' => 'Shared runner') + a_hash_including('description' => 'Project runner', 'active' => true, 'paused' => false), + a_hash_including('description' => 'Two projects runner', 'active' => true, 'paused' => false), + a_hash_including('description' => 'Shared runner', 'active' => true, 'paused' => false) ] end @@ -946,14 +1007,24 @@ RSpec.describe API::Ci::Runners do expect(response).to have_gitlab_http_status(:bad_request) end - it 'filters runners by status' do - create(:ci_runner, :project, :inactive, description: 'Inactive project runner', projects: [project]) + context 'with an inactive runner' do + let_it_be(:runner) { create(:ci_runner, :project, :inactive, description: 'Inactive project runner', projects: [project]) } - get api("/projects/#{project.id}/runners?status=paused", user) + it 'filters runners by status' do + get api("/projects/#{project.id}/runners?paused=true", user) - expect(json_response).to match_array [ - a_hash_including('description' => 'Inactive project runner') - ] + expect(json_response).to match_array [ + a_hash_including('description' => 'Inactive project runner') + ] + end + + it 'filters runners by status' do + get api("/projects/#{project.id}/runners?status=paused", user) + + expect(json_response).to match_array [ + a_hash_including('description' => 'Inactive project runner') + ] + end end it 'does not filter by invalid status' do @@ -980,13 +1051,14 @@ RSpec.describe API::Ci::Runners do end end - shared_context 'GET /groups/:id/runners' do + describe 'GET /groups/:id/runners' do context 'authorized user with maintainer privileges' do it 'returns all runners' do get api("/groups/#{group.id}/runners", user) expect(json_response).to match_array([ - a_hash_including('description' => 'Group runner A') + a_hash_including('description' => 'Group runner A', 'active' => true, 'paused' => false), + a_hash_including('description' => 'Shared runner', 'active' => true, 'paused' => false) ]) end @@ -999,6 +1071,15 @@ RSpec.describe API::Ci::Runners do ]) end + it 'returns instance runners when instance_type is specified' do + get api("/groups/#{group.id}/runners?type=instance_type", user) + + expect(json_response).to match_array([ + a_hash_including('description' => 'Shared runner') + ]) + end + + # TODO: Remove in %15.0 (https://gitlab.com/gitlab-org/gitlab/-/issues/351466) it 'returns empty result when type does not match' do get api("/groups/#{group.id}/runners?type=project_type", user) @@ -1012,21 +1093,31 @@ RSpec.describe API::Ci::Runners do end end - context 'filter runners by status' do - it 'returns runners by valid status' do - create(:ci_runner, :group, :inactive, description: 'Inactive group runner', groups: [group]) + context 'with an inactive runner' do + let_it_be(:runner) { create(:ci_runner, :group, :inactive, description: 'Inactive group runner', groups: [group]) } - get api("/groups/#{group.id}/runners?status=paused", user) + it 'returns runners by paused state' do + get api("/groups/#{group.id}/runners?paused=true", user) expect(json_response).to match_array([ a_hash_including('description' => 'Inactive group runner') ]) end - it 'does not filter by invalid status' do - get api("/groups/#{group.id}/runners?status=bogus", user) + context 'filter runners by status' do + it 'returns runners by valid status' do + get api("/groups/#{group.id}/runners?status=paused", user) - expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to match_array([ + a_hash_including('description' => 'Inactive group runner') + ]) + end + + it 'does not filter by invalid status' do + get api("/groups/#{group.id}/runners?status=bogus", user) + + expect(response).to have_gitlab_http_status(:bad_request) + end end end @@ -1048,16 +1139,6 @@ RSpec.describe API::Ci::Runners do end end - it_behaves_like 'GET /groups/:id/runners' - - context 'when the FF ci_find_runners_by_ci_mirrors is disabled' do - before do - stub_feature_flags(ci_find_runners_by_ci_mirrors: false) - end - - it_behaves_like 'GET /groups/:id/runners' - end - describe 'POST /projects/:id/runners' do context 'authorized user' do let_it_be(:project_runner2) { create(:ci_runner, :project, projects: [project2]) } diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb new file mode 100644 index 00000000000..5cf6999f60a --- /dev/null +++ b/spec/requests/api/ci/secure_files_spec.rb @@ -0,0 +1,314 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Ci::SecureFiles do + before do + stub_ci_secure_file_object_storage + stub_feature_flags(ci_secure_files: true) + end + + let_it_be(:user) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:project) { create(:project, creator_id: user.id) } + let_it_be(:maintainer) { create(:project_member, :maintainer, user: user, project: project) } + let_it_be(:developer) { create(:project_member, :developer, user: user2, project: project) } + let_it_be(:secure_file) { create(:ci_secure_file, project: project) } + + describe 'GET /projects/:id/secure_files' do + context 'feature flag' do + it 'returns a 503 when the feature flag is disabled' do + stub_feature_flags(ci_secure_files: false) + + get api("/projects/#{project.id}/secure_files", user) + + expect(response).to have_gitlab_http_status(:service_unavailable) + end + + it 'returns a 200 when the feature flag is enabled' do + get api("/projects/#{project.id}/secure_files", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_a(Array) + end + end + + context 'authorized user with proper permissions' do + it 'returns project secure files' do + get api("/projects/#{project.id}/secure_files", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_a(Array) + end + end + + context 'authorized user with invalid permissions' do + it 'does not return project secure files' do + get api("/projects/#{project.id}/secure_files", user2) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'unauthorized user' do + it 'does not return project secure files' do + get api("/projects/#{project.id}/secure_files") + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + describe 'GET /projects/:id/secure_files/:secure_file_id' do + context 'authorized user with proper permissions' do + it 'returns project secure file details' do + get api("/projects/#{project.id}/secure_files/#{secure_file.id}", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['name']).to eq(secure_file.name) + expect(json_response['permissions']).to eq(secure_file.permissions) + end + + it 'responds with 404 Not Found if requesting non-existing secure file' do + get api("/projects/#{project.id}/secure_files/99999", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'authorized user with invalid permissions' do + it 'does not return project secure file details' do + get api("/projects/#{project.id}/secure_files/#{secure_file.id}", user2) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'unauthorized user' do + it 'does not return project secure file details' do + get api("/projects/#{project.id}/secure_files/#{secure_file.id}") + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + describe 'GET /projects/:id/secure_files/:secure_file_id/download' do + context 'authorized user with proper permissions' do + it 'returns a secure file' do + sample_file = fixture_file('ci_secure_files/upload-keystore.jks') + secure_file.file = CarrierWaveStringFile.new(sample_file) + secure_file.save! + + get api("/projects/#{project.id}/secure_files/#{secure_file.id}/download", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(Base64.encode64(response.body)).to eq(Base64.encode64(sample_file)) + end + + it 'responds with 404 Not Found if requesting non-existing secure file' do + get api("/projects/#{project.id}/secure_files/99999/download", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'authorized user with invalid permissions' do + it 'does not return project secure file details' do + get api("/projects/#{project.id}/secure_files/#{secure_file.id}/download", user2) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'unauthorized user' do + it 'does not return project secure file details' do + get api("/projects/#{project.id}/secure_files/#{secure_file.id}/download") + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + describe 'POST /projects/:id/secure_files' do + context 'authorized user with proper permissions' do + it 'creates a secure file' do + params = { + file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'), + name: 'upload-keystore.jks', + permissions: 'execute' + } + + expect do + post api("/projects/#{project.id}/secure_files", user), params: params + end.to change {project.secure_files.count}.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['name']).to eq('upload-keystore.jks') + expect(json_response['permissions']).to eq('execute') + expect(json_response['checksum']).to eq(secure_file.checksum) + expect(json_response['checksum_algorithm']).to eq('sha256') + + secure_file = Ci::SecureFile.find(json_response['id']) + expect(secure_file.checksum).to eq( + Digest::SHA256.hexdigest(fixture_file('ci_secure_files/upload-keystore.jks')) + ) + expect(json_response['id']).to eq(secure_file.id) + end + + it 'creates a secure file with read_only permissions by default' do + params = { + file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'), + name: 'upload-keystore.jks' + } + + expect do + post api("/projects/#{project.id}/secure_files", user), params: params + end.to change {project.secure_files.count}.by(1) + + expect(json_response['permissions']).to eq('read_only') + end + + it 'uploads and downloads a secure file' do + post_params = { + file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'), + name: 'upload-keystore.jks', + permissions: 'read_write' + } + + post api("/projects/#{project.id}/secure_files", user), params: post_params + + secure_file_id = json_response['id'] + + get api("/projects/#{project.id}/secure_files/#{secure_file_id}/download", user) + + expect(Base64.encode64(response.body)).to eq(Base64.encode64(fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks').read)) + end + + it 'returns an error when the file checksum fails to validate' do + secure_file.update!(checksum: 'foo') + + get api("/projects/#{project.id}/secure_files/#{secure_file.id}/download", user) + + expect(response.code).to eq("500") + end + + it 'returns an error when no file is uploaded' do + post_params = { + name: 'upload-keystore.jks' + } + + post api("/projects/#{project.id}/secure_files", user), params: post_params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('file is missing') + end + + it 'returns an error when the file name is missing' do + post_params = { + file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks') + } + + post api("/projects/#{project.id}/secure_files", user), params: post_params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('name is missing') + end + + it 'returns an error when an unexpected permission is supplied' do + post_params = { + file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'), + name: 'upload-keystore.jks', + permissions: 'foo' + } + + post api("/projects/#{project.id}/secure_files", user), params: post_params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('permissions does not have a valid value') + end + + it 'returns an error when an unexpected validation failure happens' do + allow_next_instance_of(Ci::SecureFile) do |instance| + allow(instance).to receive(:valid?).and_return(false) + allow(instance).to receive_message_chain(:errors, :any?).and_return(true) + allow(instance).to receive_message_chain(:errors, :messages).and_return(['Error 1', 'Error 2']) + end + + post_params = { + file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'), + name: 'upload-keystore.jks' + } + + post api("/projects/#{project.id}/secure_files", user), params: post_params + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns a 413 error when the file size is too large' do + allow_next_instance_of(Ci::SecureFile) do |instance| + allow(instance).to receive_message_chain(:file, :size).and_return(6.megabytes.to_i) + end + + post_params = { + file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'), + name: 'upload-keystore.jks' + } + + post api("/projects/#{project.id}/secure_files", user), params: post_params + + expect(response).to have_gitlab_http_status(:payload_too_large) + end + end + + context 'authorized user with invalid permissions' do + it 'does not create a secure file' do + post api("/projects/#{project.id}/secure_files", user2) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'unauthorized user' do + it 'does not create a secure file' do + post api("/projects/#{project.id}/secure_files") + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + describe 'DELETE /projects/:id/secure_files/:secure_file_id' do + context 'authorized user with proper permissions' do + it 'deletes the secure file' do + expect do + delete api("/projects/#{project.id}/secure_files/#{secure_file.id}", user) + + expect(response).to have_gitlab_http_status(:no_content) + end.to change {project.secure_files.count}.by(-1) + end + + it 'responds with 404 Not Found if requesting non-existing secure_file' do + delete api("/projects/#{project.id}/secure_files/99999", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'authorized user with invalid permissions' do + it 'does not delete the secure_file' do + delete api("/projects/#{project.id}/secure_files/#{secure_file.id}", user2) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'unauthorized user' do + it 'does not delete the secure_file' do + delete api("/projects/#{project.id}/secure_files/#{secure_file.id}") + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end +end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 2bc642f8b14..156a4cf5ff3 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -5,6 +5,7 @@ require 'mime/types' RSpec.describe API::Commits do include ProjectForksHelper + include SessionHelpers let(:user) { create(:user) } let(:guest) { create(:user).tap { |u| project.add_guest(u) } } @@ -227,6 +228,12 @@ RSpec.describe API::Commits do expect(response.headers['X-Page']).to eq('3') end end + + context 'when per_page is 0' do + let(:per_page) { 0 } + + it_behaves_like '400 response' + end end context 'with order parameter' do @@ -378,14 +385,7 @@ RSpec.describe API::Commits do context 'when using warden' do it 'increments usage counters', :clean_gitlab_redis_sessions do - session_id = Rack::Session::SessionId.new('6919a6f1bb119dd7396fadc38fd18d0d') - session_hash = { 'warden.user.user.key' => [[user.id], user.encrypted_password[0, 29]] } - - Gitlab::Redis::Sessions.with do |redis| - redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_hash)) - end - - cookies[Gitlab::Application.config.session_options[:key]] = session_id.public_id + stub_session('warden.user.user.key' => [[user.id], user.encrypted_password[0, 29]]) expect(::Gitlab::UsageDataCounters::WebIdeCounter).to receive(:increment_commits_count) expect(::Gitlab::UsageDataCounters::EditorUniqueCounter).to receive(:track_web_ide_edit_action) diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb index 35dba93b766..a265f67115a 100644 --- a/spec/requests/api/features_spec.rb +++ b/spec/requests/api/features_spec.rb @@ -167,76 +167,85 @@ RSpec.describe API::Features, stub_feature_flags: false do end end + shared_examples 'does not enable the flag' do |actor_type, actor_path| + it 'returns the current state of the flag without changes' do + post api("/features/#{feature_name}", admin), params: { value: 'true', actor_type => actor_path } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + "name" => feature_name, + "state" => "off", + "gates" => [ + { "key" => "boolean", "value" => false } + ], + 'definition' => known_feature_flag_definition_hash + ) + end + end + + shared_examples 'enables the flag for the actor' do |actor_type| + it 'sets the feature gate' do + post api("/features/#{feature_name}", admin), params: { value: 'true', actor_type => actor.full_path } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + 'name' => feature_name, + 'state' => 'conditional', + 'gates' => [ + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'actors', 'value' => ["#{actor.class}:#{actor.id}"] } + ], + 'definition' => known_feature_flag_definition_hash + ) + end + end + context 'when enabling for a project by path' do context 'when the project exists' do - let!(:project) { create(:project) } - - it 'sets the feature gate' do - post api("/features/#{feature_name}", admin), params: { value: 'true', project: project.full_path } - - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'conditional', - 'gates' => [ - { 'key' => 'boolean', 'value' => false }, - { 'key' => 'actors', 'value' => ["Project:#{project.id}"] } - ], - 'definition' => known_feature_flag_definition_hash - ) + it_behaves_like 'enables the flag for the actor', :project do + let(:actor) { create(:project) } end end context 'when the project does not exist' do - it 'sets no new values' do - post api("/features/#{feature_name}", admin), params: { value: 'true', project: 'mep/to/the/mep/mep' } - - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - "name" => feature_name, - "state" => "off", - "gates" => [ - { "key" => "boolean", "value" => false } - ], - 'definition' => known_feature_flag_definition_hash - ) - end + it_behaves_like 'does not enable the flag', :project, 'mep/to/the/mep/mep' end end context 'when enabling for a group by path' do context 'when the group exists' do - it 'sets the feature gate' do - group = create(:group) - - post api("/features/#{feature_name}", admin), params: { value: 'true', group: group.full_path } - - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'conditional', - 'gates' => [ - { 'key' => 'boolean', 'value' => false }, - { 'key' => 'actors', 'value' => ["Group:#{group.id}"] } - ], - 'definition' => known_feature_flag_definition_hash - ) + it_behaves_like 'enables the flag for the actor', :group do + let(:actor) { create(:group) } end end context 'when the group does not exist' do - it 'sets no new values and keeps the feature disabled' do - post api("/features/#{feature_name}", admin), params: { value: 'true', group: 'not/a/group' } - - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - "name" => feature_name, - "state" => "off", - "gates" => [ - { "key" => "boolean", "value" => false } - ], - 'definition' => known_feature_flag_definition_hash - ) + it_behaves_like 'does not enable the flag', :group, 'not/a/group' + end + end + + context 'when enabling for a namespace by path' do + context 'when the user namespace exists' do + it_behaves_like 'enables the flag for the actor', :namespace do + let(:actor) { create(:namespace) } + end + end + + context 'when the group namespace exists' do + it_behaves_like 'enables the flag for the actor', :namespace do + let(:actor) { create(:group) } + end + end + + context 'when the user namespace does not exist' do + it_behaves_like 'does not enable the flag', :namespace, 'not/a/group' + end + + context 'when a project namespace exists' do + let(:project_namespace) { create(:project_namespace) } + + it_behaves_like 'does not enable the flag', :namespace do + let(:actor_path) { project_namespace.full_path } end end end diff --git a/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb b/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb index 578a71a7272..c19defa37e8 100644 --- a/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb +++ b/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb @@ -5,7 +5,7 @@ RSpec.describe 'Getting Ci Cd Setting' do include GraphqlHelpers let_it_be_with_reload(:project) { create(:project, :repository) } - let_it_be(:current_user) { project.owner } + let_it_be(:current_user) { project.first_owner } let(:fields) do <<~QUERY diff --git a/spec/requests/api/graphql/ci/config_spec.rb b/spec/requests/api/graphql/ci/config_spec.rb index 755585f8e0e..62b15a8396c 100644 --- a/spec/requests/api/graphql/ci/config_spec.rb +++ b/spec/requests/api/graphql/ci/config_spec.rb @@ -225,7 +225,7 @@ RSpec.describe 'Query.ciConfig' do context 'when using deprecated keywords' do let_it_be(:content) do YAML.dump( - rspec: { script: 'ls' }, + rspec: { script: 'ls', type: 'test' }, types: ['test'] ) end @@ -233,7 +233,10 @@ RSpec.describe 'Query.ciConfig' do it 'returns a warning' do post_graphql_query - expect(graphql_data['ciConfig']['warnings']).to include('root `types` is deprecated in 9.0 and will be removed in 15.0.') + expect(graphql_data['ciConfig']['warnings']).to include( + 'root `types` is deprecated in 9.0 and will be removed in 15.0.', + 'jobs:rspec `type` is deprecated in 9.0 and will be removed in 15.0.' + ) end end diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index 8c919b48849..fa16b9e1ddd 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -25,6 +25,8 @@ RSpec.describe 'Query.runner(id)' do access_level: 0, tag_list: %w[tag1 tag2], run_untagged: true, executor_type: :shell) end + let_it_be(:active_project_runner) { create(:ci_runner, :project) } + def get_runner(id) case id when :active_instance_runner @@ -33,6 +35,8 @@ RSpec.describe 'Query.runner(id)' do inactive_instance_runner when :active_group_runner active_group_runner + when :active_project_runner + active_project_runner end end @@ -55,7 +59,7 @@ RSpec.describe 'Query.runner(id)' do runner = get_runner(runner_id) expect(runner_data).to match a_hash_including( - 'id' => "gid://gitlab/Ci::Runner/#{runner.id}", + 'id' => runner.to_global_id.to_s, 'description' => runner.description, 'createdAt' => runner.created_at&.iso8601, 'contactedAt' => runner.contacted_at&.iso8601, @@ -64,6 +68,7 @@ RSpec.describe 'Query.runner(id)' do 'revision' => runner.revision, 'locked' => false, 'active' => runner.active, + 'paused' => !runner.active, 'status' => runner.status('14.5').to_s.upcase, 'maximumTimeout' => runner.maximum_timeout, 'accessLevel' => runner.access_level.to_s.upcase, @@ -72,6 +77,7 @@ RSpec.describe 'Query.runner(id)' do 'runnerType' => runner.instance_type? ? 'INSTANCE_TYPE' : 'PROJECT_TYPE', 'executorName' => runner.executor_type&.dasherize, 'jobCount' => 0, + 'jobs' => a_hash_including("count" => 0, "nodes" => [], "pageInfo" => anything), 'projectCount' => nil, 'adminUrl' => "http://localhost/admin/runners/#{runner.id}", 'userPermissions' => { @@ -103,7 +109,7 @@ RSpec.describe 'Query.runner(id)' do runner = get_runner(runner_id) expect(runner_data).to match a_hash_including( - 'id' => "gid://gitlab/Ci::Runner/#{runner.id}", + 'id' => runner.to_global_id.to_s, 'adminUrl' => nil ) expect(runner_data['tagList']).to match_array runner.tag_list @@ -179,7 +185,7 @@ RSpec.describe 'Query.runner(id)' do runner_data = graphql_data_at(:runner) expect(runner_data).to match a_hash_including( - 'id' => "gid://gitlab/Ci::Runner/#{project_runner.id}", + 'id' => project_runner.to_global_id.to_s, 'locked' => is_locked ) end @@ -216,13 +222,36 @@ RSpec.describe 'Query.runner(id)' do a_hash_including( 'webUrl' => "http://localhost/groups/#{group.full_path}/-/runners/#{active_group_runner.id}", 'node' => { - 'id' => "gid://gitlab/Ci::Runner/#{active_group_runner.id}" + 'id' => active_group_runner.to_global_id.to_s } ) ] end end + describe 'for group runner request' do + let(:query) do + %( + query { + runner(id: "#{active_group_runner.to_global_id}") { + groups { + nodes { + id + } + } + } + } + ) + end + + it 'retrieves groups field with expected value' do + post_graphql(query, current_user: user) + + runner_data = graphql_data_at(:runner, :groups) + expect(runner_data).to eq 'nodes' => [{ 'id' => group.to_global_id.to_s }] + end + end + describe 'for runner with status' do let_it_be(:stale_runner) { create(:ci_runner, description: 'Stale runner 1', created_at: 3.months.ago) } let_it_be(:never_contacted_instance_runner) { create(:ci_runner, description: 'Missing runner 1', created_at: 1.month.ago, contacted_at: nil) } @@ -279,21 +308,51 @@ RSpec.describe 'Query.runner(id)' do let!(:job) { create(:ci_build, runner: project_runner1) } - context 'requesting project and job counts' do + context 'requesting projects and counts for projects and jobs' do + let(:jobs_fragment) do + %( + jobs { + count + nodes { + id + status + } + } + ) + end + let(:query) do %( query { projectRunner1: runner(id: "#{project_runner1.to_global_id}") { projectCount jobCount + #{jobs_fragment} + projects { + nodes { + id + } + } } projectRunner2: runner(id: "#{project_runner2.to_global_id}") { projectCount jobCount + #{jobs_fragment} + projects { + nodes { + id + } + } } activeInstanceRunner: runner(id: "#{active_instance_runner.to_global_id}") { projectCount jobCount + #{jobs_fragment} + projects { + nodes { + id + } + } } } ) @@ -312,13 +371,29 @@ RSpec.describe 'Query.runner(id)' do expect(runner1_data).to match a_hash_including( 'jobCount' => 1, - 'projectCount' => 2) + 'jobs' => a_hash_including( + "count" => 1, + "nodes" => [{ "id" => job.to_global_id.to_s, "status" => job.status.upcase }] + ), + 'projectCount' => 2, + 'projects' => { + 'nodes' => [ + { 'id' => project1.to_global_id.to_s }, + { 'id' => project2.to_global_id.to_s } + ] + }) expect(runner2_data).to match a_hash_including( 'jobCount' => 0, - 'projectCount' => 0) + 'jobs' => nil, # returning jobs not allowed for more than 1 runner (see RunnerJobsResolver) + 'projectCount' => 0, + 'projects' => { + 'nodes' => [] + }) expect(runner3_data).to match a_hash_including( 'jobCount' => 0, - 'projectCount' => nil) + 'jobs' => nil, # returning jobs not allowed for more than 1 runner (see RunnerJobsResolver) + 'projectCount' => nil, + 'projects' => nil) end end end @@ -326,7 +401,17 @@ RSpec.describe 'Query.runner(id)' do describe 'by regular user' do let(:user) { create(:user) } - it_behaves_like 'retrieval by unauthorized user', :active_instance_runner + context 'on instance runner' do + it_behaves_like 'retrieval by unauthorized user', :active_instance_runner + end + + context 'on group runner' do + it_behaves_like 'retrieval by unauthorized user', :active_group_runner + end + + context 'on project runner' do + it_behaves_like 'retrieval by unauthorized user', :active_project_runner + end end describe 'by non-admin user' do diff --git a/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb b/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb index 802ab847b3d..35a70a180a2 100644 --- a/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb +++ b/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb @@ -17,7 +17,7 @@ RSpec.describe 'container repository details' do ) end - let(:user) { project.owner } + let(:user) { project.first_owner } let(:variables) { {} } let(:tags) { %w[latest tag1 tag2 tag3 tag4 tag5] } let(:container_repository_global_id) { container_repository.to_global_id.to_s } diff --git a/spec/requests/api/graphql/gitlab_schema_spec.rb b/spec/requests/api/graphql/gitlab_schema_spec.rb index 8bbeae97f57..e80f5e0e0ff 100644 --- a/spec/requests/api/graphql/gitlab_schema_spec.rb +++ b/spec/requests/api/graphql/gitlab_schema_spec.rb @@ -166,7 +166,7 @@ RSpec.describe 'GitlabSchema configurations' do end context 'authentication' do - let(:current_user) { project.owner } + let(:current_user) { project.first_owner } it 'authenticates all queries' do subject @@ -216,7 +216,7 @@ RSpec.describe 'GitlabSchema configurations' do context "global id's" do it 'uses GlobalID to expose ids' do post_graphql(graphql_query_for('project', { 'fullPath' => project.full_path }, %w(id)), - current_user: project.owner) + current_user: project.first_owner) parsed_id = GlobalID.parse(graphql_data['project']['id']) diff --git a/spec/requests/api/graphql/group/recent_issue_boards_query_spec.rb b/spec/requests/api/graphql/group/recent_issue_boards_query_spec.rb new file mode 100644 index 00000000000..4914beec870 --- /dev/null +++ b/spec/requests/api/graphql/group/recent_issue_boards_query_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting group recent issue boards' do + include GraphqlHelpers + + it_behaves_like 'querying a GraphQL type recent boards' do + let_it_be(:user) { create(:user) } + let_it_be(:parent) { create(:group, :public) } + let_it_be(:board) { create(:board, resource_parent: parent, name: 'test group board') } + let(:board_type) { 'group' } + end +end diff --git a/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb b/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb index 05f6804a208..30e7f196542 100644 --- a/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb @@ -45,7 +45,7 @@ RSpec.describe 'CiCdSettingsUpdate' do end context 'when authorized' do - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } it 'updates ci cd settings' do post_graphql_mutation(mutation, current_user: user) diff --git a/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb b/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb index b53a7ddde32..5269c60b50a 100644 --- a/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb @@ -49,7 +49,7 @@ RSpec.describe 'CiJobTokenScopeAddProject' do end context 'when authorized' do - let_it_be(:current_user) { project.owner } + let_it_be(:current_user) { project.first_owner } before do target_project.add_developer(current_user) diff --git a/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb b/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb index f1f42b00ada..b62291d1ebd 100644 --- a/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb @@ -55,7 +55,7 @@ RSpec.describe 'CiJobTokenScopeRemoveProject' do end context 'when authorized' do - let_it_be(:current_user) { project.owner } + let_it_be(:current_user) { project.first_owner } before do target_project.add_guest(current_user) diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb index 08959d354e2..37656ab4eea 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb @@ -6,7 +6,7 @@ RSpec.describe 'PipelineDestroy' do include GraphqlHelpers let_it_be(:project) { create(:project) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project, user: user) } let(:mutation) do diff --git a/spec/requests/api/graphql/mutations/ci/runners_registration_token/reset_spec.rb b/spec/requests/api/graphql/mutations/ci/runners_registration_token/reset_spec.rb index 322706be119..12368e7e9c5 100644 --- a/spec/requests/api/graphql/mutations/ci/runners_registration_token/reset_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/runners_registration_token/reset_spec.rb @@ -71,7 +71,7 @@ RSpec.describe 'RunnersRegistrationTokenReset' do end include_context 'when authorized', 'project' do - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } def get_token project.reload.runners_token diff --git a/spec/requests/api/graphql/mutations/issues/create_spec.rb b/spec/requests/api/graphql/mutations/issues/create_spec.rb index 6baed352b37..3d81b456c9c 100644 --- a/spec/requests/api/graphql/mutations/issues/create_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/create_spec.rb @@ -52,5 +52,22 @@ RSpec.describe 'Create an issue' do it_behaves_like 'has spam protection' do let(:mutation_class) { ::Mutations::Issues::Create } end + + context 'when position params are provided' do + let(:existing_issue) { create(:issue, project: project, relative_position: 50) } + + before do + input.merge!( + move_after_id: existing_issue.to_global_id.to_s + ) + end + + it 'sets the correct position' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['issue']['relativePosition']).to be < existing_issue.relative_position + end + end end end diff --git a/spec/requests/api/graphql/mutations/security/ci_configuration/configure_sast_iac_spec.rb b/spec/requests/api/graphql/mutations/security/ci_configuration/configure_sast_iac_spec.rb index 929609d4160..0c034f38dc8 100644 --- a/spec/requests/api/graphql/mutations/security/ci_configuration/configure_sast_iac_spec.rb +++ b/spec/requests/api/graphql/mutations/security/ci_configuration/configure_sast_iac_spec.rb @@ -12,7 +12,7 @@ RSpec.describe 'ConfigureSastIac' do let(:mutation_response) { graphql_mutation_response(:configureSastIac) } context 'when authorized' do - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } it 'creates a branch with sast iac configured' do post_graphql_mutation(mutation, current_user: user) diff --git a/spec/requests/api/graphql/mutations/security/ci_configuration/configure_secret_detection_spec.rb b/spec/requests/api/graphql/mutations/security/ci_configuration/configure_secret_detection_spec.rb index 23a154b71a0..8fa6e44b208 100644 --- a/spec/requests/api/graphql/mutations/security/ci_configuration/configure_secret_detection_spec.rb +++ b/spec/requests/api/graphql/mutations/security/ci_configuration/configure_secret_detection_spec.rb @@ -12,7 +12,7 @@ RSpec.describe 'ConfigureSecretDetection' do let(:mutation_response) { graphql_mutation_response(:configureSecretDetection) } context 'when authorized' do - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } it 'creates a branch with secret detection configured' do post_graphql_mutation(mutation, current_user: user) diff --git a/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb b/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb new file mode 100644 index 00000000000..e1c7fd9d60d --- /dev/null +++ b/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::UserPreferences::Update do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + + let(:sort_value) { 'TITLE_ASC' } + + let(:input) do + { + 'issuesSort' => sort_value + } + end + + let(:mutation) { graphql_mutation(:userPreferencesUpdate, input) } + let(:mutation_response) { graphql_mutation_response(:userPreferencesUpdate) } + + context 'when user has no existing preference' do + it 'creates the user preference record' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['userPreferences']['issuesSort']).to eq(sort_value) + + expect(current_user.user_preference.persisted?).to eq(true) + expect(current_user.user_preference.issues_sort).to eq(Types::IssueSortEnum.values[sort_value].value.to_s) + end + end + + context 'when user has existing preference' do + before do + current_user.create_user_preference!(issues_sort: Types::IssueSortEnum.values['TITLE_DESC'].value) + end + + it 'updates the existing value' do + post_graphql_mutation(mutation, current_user: current_user) + + current_user.user_preference.reload + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['userPreferences']['issuesSort']).to eq(sort_value) + + expect(current_user.user_preference.issues_sort).to eq(Types::IssueSortEnum.values[sort_value].value.to_s) + end + end +end diff --git a/spec/requests/api/graphql/mutations/work_items/create_spec.rb b/spec/requests/api/graphql/mutations/work_items/create_spec.rb index e7a0c7753fb..6abdaa2c850 100644 --- a/spec/requests/api/graphql/mutations/work_items/create_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/create_spec.rb @@ -47,6 +47,18 @@ RSpec.describe 'Create a work item' do ) end + context 'when input is invalid' do + let(:input) { { 'title' => '', 'workItemTypeId' => WorkItems::Type.default_by_type(:task).to_global_id.to_s } } + + it 'does not create and returns validation errors' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to not_change(WorkItem, :count) + + expect(graphql_mutation_response(:work_item_create)['errors']).to contain_exactly("Title can't be blank") + end + end + it_behaves_like 'has spam protection' do let(:mutation_class) { ::Mutations::WorkItems::Create } end @@ -56,8 +68,13 @@ RSpec.describe 'Create a work item' do stub_feature_flags(work_items: false) end - it_behaves_like 'a mutation that returns top-level errors', - errors: ["Field 'workItemCreate' doesn't exist on type 'Mutation'", "Variable $workItemCreateInput is declared by anonymous mutation but not used"] + it 'does not create the work item and returns an error' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to not_change(WorkItem, :count) + + expect(mutation_response['errors']).to contain_exactly('`work_items` feature flag disabled for this project') + end end end end diff --git a/spec/requests/api/graphql/mutations/work_items/delete_spec.rb b/spec/requests/api/graphql/mutations/work_items/delete_spec.rb new file mode 100644 index 00000000000..14c8b757a57 --- /dev/null +++ b/spec/requests/api/graphql/mutations/work_items/delete_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Delete a work item' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:developer) { create(:user).tap { |user| project.add_developer(user) } } + + let(:current_user) { developer } + let(:mutation) { graphql_mutation(:workItemDelete, { 'id' => work_item.to_global_id.to_s }) } + let(:mutation_response) { graphql_mutation_response(:work_item_delete) } + + context 'when the user is not allowed to delete a work item' do + let(:work_item) { create(:work_item, project: project) } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permissions to delete a work item' do + let_it_be(:authored_work_item, refind: true) { create(:work_item, project: project, author: developer, assignees: [developer]) } + + let(:work_item) { authored_work_item } + + it 'deletes the work item' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change(WorkItem, :count).by(-1) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['project']).to include('id' => work_item.project.to_global_id.to_s) + end + + context 'when the work_items feature flag is disabled' do + before do + stub_feature_flags(work_items: false) + end + + it 'does not delete the work item' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to not_change(WorkItem, :count) + + expect(mutation_response['errors']).to contain_exactly('`work_items` feature flag disabled for this project') + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/work_items/update_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_spec.rb new file mode 100644 index 00000000000..71b03103115 --- /dev/null +++ b/spec/requests/api/graphql/mutations/work_items/update_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Update a work item' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:developer) { create(:user).tap { |user| project.add_developer(user) } } + let_it_be(:work_item, refind: true) { create(:work_item, project: project) } + + let(:work_item_event) { 'CLOSE' } + let(:input) { { 'stateEvent' => work_item_event, 'title' => 'updated title' } } + + let(:mutation) { graphql_mutation(:workItemUpdate, input.merge('id' => work_item.to_global_id.to_s)) } + + let(:mutation_response) { graphql_mutation_response(:work_item_update) } + + context 'the user is not allowed to update a work item' do + let(:current_user) { create(:user) } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permissions to update a work item' do + let(:current_user) { developer } + + context 'when the work item is open' do + it 'closes and updates the work item' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to change(work_item, :state).from('opened').to('closed').and( + change(work_item, :title).from(work_item.title).to('updated title') + ) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']).to include( + 'state' => 'CLOSED', + 'title' => 'updated title' + ) + end + end + + context 'when the work item is closed' do + let(:work_item_event) { 'REOPEN' } + + before do + work_item.close! + end + + it 'reopens the work item' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to change(work_item, :state).from('closed').to('opened') + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']).to include( + 'state' => 'OPEN' + ) + end + end + + it_behaves_like 'has spam protection' do + let(:mutation_class) { ::Mutations::WorkItems::Update } + end + + context 'when the work_items feature flag is disabled' do + before do + stub_feature_flags(work_items: false) + end + + it 'does not update the work item and returns and error' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to not_change(work_item, :title) + + expect(mutation_response['errors']).to contain_exactly('`work_items` feature flag disabled for this project') + end + end + end +end diff --git a/spec/requests/api/graphql/packages/package_spec.rb b/spec/requests/api/graphql/packages/package_spec.rb index 2ff3bc7cc47..365efc514d4 100644 --- a/spec/requests/api/graphql/packages/package_spec.rb +++ b/spec/requests/api/graphql/packages/package_spec.rb @@ -102,18 +102,6 @@ RSpec.describe 'package details' do expect(package_file_ids).to contain_exactly(package_file.to_global_id.to_s) end - - context 'with packages_installable_package_files disabled' do - before do - stub_feature_flags(packages_installable_package_files: false) - end - - it 'returns them' do - subject - - expect(package_file_ids).to contain_exactly(package_file_pending_destruction.to_global_id.to_s, package_file.to_global_id.to_s) - end - end end context 'with a batched query' do @@ -145,8 +133,9 @@ RSpec.describe 'package details' do let(:pipeline_gids) { pipelines.sort_by(&:id).map(&:to_gid).map(&:to_s).reverse } before do - composer_package.pipelines = pipelines - composer_package.save! + pipelines.each do |pipeline| + create(:package_build_info, package: composer_package, pipeline: pipeline) + end end def run_query(args) diff --git a/spec/requests/api/graphql/project/container_expiration_policy_spec.rb b/spec/requests/api/graphql/project/container_expiration_policy_spec.rb index dc16847a669..e3ea9e46353 100644 --- a/spec/requests/api/graphql/project/container_expiration_policy_spec.rb +++ b/spec/requests/api/graphql/project/container_expiration_policy_spec.rb @@ -5,7 +5,7 @@ RSpec.describe 'getting a repository in a project' do include GraphqlHelpers let_it_be(:project) { create(:project) } - let_it_be(:current_user) { project.owner } + let_it_be(:current_user) { project.first_owner } let_it_be(:container_expiration_policy) { project.container_expiration_policy } let(:fields) do diff --git a/spec/requests/api/graphql/project/container_repositories_spec.rb b/spec/requests/api/graphql/project/container_repositories_spec.rb index 692143b2215..bbab6012f3f 100644 --- a/spec/requests/api/graphql/project/container_repositories_spec.rb +++ b/spec/requests/api/graphql/project/container_repositories_spec.rb @@ -38,7 +38,7 @@ RSpec.describe 'getting container repositories in a project' do ) end - let(:user) { project.owner } + let(:user) { project.first_owner } let(:variables) { {} } let(:container_repositories_response) { graphql_data.dig('project', 'containerRepositories', 'edges') } let(:container_repositories_count_response) { graphql_data.dig('project', 'containerRepositoriesCount') } diff --git a/spec/requests/api/graphql/project/error_tracking/sentry_detailed_error_request_spec.rb b/spec/requests/api/graphql/project/error_tracking/sentry_detailed_error_request_spec.rb index 40a3281d3b7..2b85704f479 100644 --- a/spec/requests/api/graphql/project/error_tracking/sentry_detailed_error_request_spec.rb +++ b/spec/requests/api/graphql/project/error_tracking/sentry_detailed_error_request_spec.rb @@ -6,7 +6,7 @@ RSpec.describe 'getting a detailed sentry error' do let_it_be(:project) { create(:project, :repository) } let_it_be(:project_setting) { create(:project_error_tracking_setting, project: project) } - let_it_be(:current_user) { project.owner } + let_it_be(:current_user) { project.first_owner } let_it_be(:sentry_detailed_error) { build(:error_tracking_sentry_detailed_error) } let(:sentry_gid) { sentry_detailed_error.to_global_id.to_s } diff --git a/spec/requests/api/graphql/project/error_tracking/sentry_errors_request_spec.rb b/spec/requests/api/graphql/project/error_tracking/sentry_errors_request_spec.rb index a540386a9de..3ca0e35882a 100644 --- a/spec/requests/api/graphql/project/error_tracking/sentry_errors_request_spec.rb +++ b/spec/requests/api/graphql/project/error_tracking/sentry_errors_request_spec.rb @@ -6,7 +6,7 @@ RSpec.describe 'sentry errors requests' do let_it_be(:project) { create(:project, :repository) } let_it_be(:project_setting) { create(:project_error_tracking_setting, project: project) } - let_it_be(:current_user) { project.owner } + let_it_be(:current_user) { project.first_owner } let(:query) do graphql_query_for( diff --git a/spec/requests/api/graphql/project/grafana_integration_spec.rb b/spec/requests/api/graphql/project/grafana_integration_spec.rb index 9b24698f40c..e7534945e7a 100644 --- a/spec/requests/api/graphql/project/grafana_integration_spec.rb +++ b/spec/requests/api/graphql/project/grafana_integration_spec.rb @@ -5,7 +5,7 @@ RSpec.describe 'Getting Grafana Integration' do include GraphqlHelpers let_it_be(:project) { create(:project, :repository) } - let_it_be(:current_user) { project.owner } + let_it_be(:current_user) { project.first_owner } let_it_be(:grafana_integration) { create(:grafana_integration, project: project) } let(:fields) do diff --git a/spec/requests/api/graphql/project/issue/design_collection/versions_spec.rb b/spec/requests/api/graphql/project/issue/design_collection/versions_spec.rb index 9d98498ca8a..46fd65db1c5 100644 --- a/spec/requests/api/graphql/project/issue/design_collection/versions_spec.rb +++ b/spec/requests/api/graphql/project/issue/design_collection/versions_spec.rb @@ -24,7 +24,7 @@ RSpec.describe 'Getting versions related to an issue' do create(:design_version, issue: issue) end - let_it_be(:owner) { issue.project.owner } + let_it_be(:owner) { issue.project.first_owner } def version_query(params = version_params) query_graphql_field(:versions, params, version_query_fields) diff --git a/spec/requests/api/graphql/project/issue/designs/designs_spec.rb b/spec/requests/api/graphql/project/issue/designs/designs_spec.rb index def41efddde..f0205319983 100644 --- a/spec/requests/api/graphql/project/issue/designs/designs_spec.rb +++ b/spec/requests/api/graphql/project/issue/designs/designs_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'Getting designs related to an issue' do include DesignManagementTestHelpers let_it_be(:design) { create(:design, :with_smaller_image_versions, versions_count: 1) } - let_it_be(:current_user) { design.project.owner } + let_it_be(:current_user) { design.project.first_owner } let(:design_query) do <<~NODE diff --git a/spec/requests/api/graphql/project/issue/designs/notes_spec.rb b/spec/requests/api/graphql/project/issue/designs/notes_spec.rb index 7148750b6cb..de2ace95757 100644 --- a/spec/requests/api/graphql/project/issue/designs/notes_spec.rb +++ b/spec/requests/api/graphql/project/issue/designs/notes_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'Getting designs related to an issue' do let_it_be(:project) { create(:project, :public) } let_it_be(:issue) { create(:issue, project: project) } let_it_be(:design) { create(:design, :with_file, versions_count: 1, issue: issue) } - let_it_be(:current_user) { project.owner } + let_it_be(:current_user) { project.first_owner } let_it_be(:note) { create(:diff_note_on_design, noteable: design, project: project) } before do diff --git a/spec/requests/api/graphql/project/merge_requests_spec.rb b/spec/requests/api/graphql/project/merge_requests_spec.rb index b0bedd99fce..303748bc70e 100644 --- a/spec/requests/api/graphql/project/merge_requests_spec.rb +++ b/spec/requests/api/graphql/project/merge_requests_spec.rb @@ -29,6 +29,10 @@ RSpec.describe 'getting merge request listings nested in a project' do create(:merge_request, :unique_branches, source_project: project) end + let(:all_merge_requests) do + [merge_request_a, merge_request_b, merge_request_c, merge_request_d, merge_request_e] + end + let(:results) { graphql_data.dig('project', 'mergeRequests', 'nodes') } let(:search_params) { nil } @@ -180,6 +184,39 @@ RSpec.describe 'getting merge request listings nested in a project' do it_behaves_like 'when searching with parameters' end + context 'when searching by update time' do + let(:start_time) { 10.days.ago } + let(:cutoff) { start_time + 36.hours } + + before do + all_merge_requests.each_with_index do |mr, i| + mr.updated_at = start_time + i.days + mr.save!(touch: false) + end + end + + context 'when searching by updated_after' do + let(:search_params) { { updated_after: cutoff } } + let(:mrs) { all_merge_requests[2..] } + + it_behaves_like 'when searching with parameters' + end + + context 'when searching by updated_before' do + let(:search_params) { { updated_before: cutoff } } + let(:mrs) { all_merge_requests[0..1] } + + it_behaves_like 'when searching with parameters' + end + + context 'when searching by updated_before and updated_after' do + let(:search_params) { { updated_after: cutoff, updated_before: cutoff + 2.days } } + let(:mrs) { all_merge_requests[2..3] } + + it_behaves_like 'when searching with parameters' + end + end + context 'when searching by combination' do let(:search_params) { { state: :closed, labels: [label.title] } } let(:mrs) { [merge_request_c] } diff --git a/spec/requests/api/graphql/project/project_members_spec.rb b/spec/requests/api/graphql/project/project_members_spec.rb index 466464f600c..315d44884ff 100644 --- a/spec/requests/api/graphql/project/project_members_spec.rb +++ b/spec/requests/api/graphql/project/project_members_spec.rb @@ -110,6 +110,102 @@ RSpec.describe 'getting project members information' do end end + context 'merge request interactions' do + let(:project_path) { var('ID!').with(parent_project.full_path) } + let(:mr_a) do + var('MergeRequestID!') + .with(global_id_of(create(:merge_request, source_project: parent_project, source_branch: 'branch-1'))) + end + + let(:mr_b) do + var('MergeRequestID!') + .with(global_id_of(create(:merge_request, source_project: parent_project, source_branch: 'branch-2'))) + end + + let(:interaction_query) do + <<~HEREDOC + edges { + node { + user { + id + } + mrA: #{query_graphql_field(:merge_request_interaction, { id: mr_a }, 'canMerge')} + } + } + HEREDOC + end + + let(:interaction_b_query) do + <<~HEREDOC + edges { + node { + user { + id + } + mrA: #{query_graphql_field(:merge_request_interaction, { id: mr_a }, 'canMerge')} + mrB: #{query_graphql_field(:merge_request_interaction, { id: mr_b }, 'canMerge')} + } + } + HEREDOC + end + + it 'avoids N+1 queries, when requesting multiple MRs' do + control_query = with_signature( + [project_path, mr_a], + graphql_query_for(:project, { full_path: project_path }, + query_graphql_field(:project_members, nil, interaction_query)) + ) + query_two = with_signature( + [project_path, mr_a, mr_b], + graphql_query_for(:project, { full_path: project_path }, + query_graphql_field(:project_members, nil, interaction_b_query)) + ) + + control_count = ActiveRecord::QueryRecorder.new do + post_graphql(control_query, current_user: user, variables: [project_path, mr_a]) + end + + # two project members, neither of whom can merge + expect(can_merge(:mrA)).to eq [false, false] + + expect do + post_graphql(query_two, current_user: user, variables: [project_path, mr_a, mr_b]) + + expect(can_merge(:mrA)).to eq [false, false] + expect(can_merge(:mrB)).to eq [false, false] + end.not_to exceed_query_limit(control_count) + end + + it 'avoids N+1 queries, when more users are involved' do + new_user = create(:user) + + query = with_signature( + [project_path, mr_a], + graphql_query_for(:project, { full_path: project_path }, + query_graphql_field(:project_members, nil, interaction_query)) + ) + + control_count = ActiveRecord::QueryRecorder.new do + post_graphql(query, current_user: user, variables: [project_path, mr_a]) + end + + # two project members, neither of whom can merge + expect(can_merge(:mrA)).to eq [false, false] + + parent_project.add_guest(new_user) + + expect do + post_graphql(query, current_user: user, variables: [project_path, mr_a]) + + expect(can_merge(:mrA)).to eq [false, false, false] + end.not_to exceed_query_limit(control_count) + end + + def can_merge(name) + graphql_data_at(:project, :project_members, :edges, :node, name, :can_merge) + end + end + context 'when unauthenticated' do it 'returns members' do fetch_members(current_user: nil, project: parent_project) diff --git a/spec/requests/api/graphql/project/recent_issue_boards_query_spec.rb b/spec/requests/api/graphql/project/recent_issue_boards_query_spec.rb new file mode 100644 index 00000000000..b3daf86c4af --- /dev/null +++ b/spec/requests/api/graphql/project/recent_issue_boards_query_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting project recent issue boards' do + include GraphqlHelpers + + it_behaves_like 'querying a GraphQL type recent boards' do + let_it_be(:user) { create(:user) } + let_it_be(:parent) { create(:project, :public, namespace: user.namespace) } + let_it_be(:board) { create(:board, resource_parent: parent, name: 'test project board') } + let(:board_type) { 'project' } + end +end diff --git a/spec/requests/api/graphql/project/repository/blobs_spec.rb b/spec/requests/api/graphql/project/repository/blobs_spec.rb index 12f6fbd793e..ba87f1100f2 100644 --- a/spec/requests/api/graphql/project/repository/blobs_spec.rb +++ b/spec/requests/api/graphql/project/repository/blobs_spec.rb @@ -5,7 +5,7 @@ RSpec.describe 'getting blobs in a project repository' do include GraphqlHelpers let(:project) { create(:project, :repository) } - let(:current_user) { project.owner } + let(:current_user) { project.first_owner } let(:paths) { ["CONTRIBUTING.md", "README.md"] } let(:ref) { project.default_branch } let(:fields) do diff --git a/spec/requests/api/graphql/project/repository_spec.rb b/spec/requests/api/graphql/project/repository_spec.rb index 8810f2fa3d5..b00f64c3db6 100644 --- a/spec/requests/api/graphql/project/repository_spec.rb +++ b/spec/requests/api/graphql/project/repository_spec.rb @@ -5,7 +5,7 @@ RSpec.describe 'getting a repository in a project' do include GraphqlHelpers let(:project) { create(:project, :repository) } - let(:current_user) { project.owner } + let(:current_user) { project.first_owner } let(:fields) do <<~QUERY #{all_graphql_fields_for('repository'.classify)} diff --git a/spec/requests/api/graphql/project/tree/tree_spec.rb b/spec/requests/api/graphql/project/tree/tree_spec.rb index f4cd316da96..25e878a5b1a 100644 --- a/spec/requests/api/graphql/project/tree/tree_spec.rb +++ b/spec/requests/api/graphql/project/tree/tree_spec.rb @@ -5,7 +5,7 @@ RSpec.describe 'getting a tree in a project' do include GraphqlHelpers let(:project) { create(:project, :repository) } - let(:current_user) { project.owner } + let(:current_user) { project.first_owner } let(:path) { "" } let(:ref) { "master" } let(:fields) do diff --git a/spec/requests/api/group_clusters_spec.rb b/spec/requests/api/group_clusters_spec.rb index f65f9384efa..c48b5007f91 100644 --- a/spec/requests/api/group_clusters_spec.rb +++ b/spec/requests/api/group_clusters_spec.rb @@ -6,11 +6,11 @@ RSpec.describe API::GroupClusters do include KubernetesHelpers let(:current_user) { create(:user) } - let(:developer_user) { create(:user) } + let(:unauthorized_user) { create(:user) } let(:group) { create(:group, :private) } before do - group.add_developer(developer_user) + group.add_reporter(unauthorized_user) group.add_maintainer(current_user) end @@ -24,7 +24,7 @@ RSpec.describe API::GroupClusters do context 'non-authorized user' do it 'responds with 403' do - get api("/groups/#{group.id}/clusters", developer_user) + get api("/groups/#{group.id}/clusters", unauthorized_user) expect(response).to have_gitlab_http_status(:forbidden) end @@ -68,7 +68,7 @@ RSpec.describe API::GroupClusters do context 'non-authorized user' do it 'responds with 403' do - get api("/groups/#{group.id}/clusters/#{cluster_id}", developer_user) + get api("/groups/#{group.id}/clusters/#{cluster_id}", unauthorized_user) expect(response).to have_gitlab_http_status(:forbidden) end @@ -183,7 +183,7 @@ RSpec.describe API::GroupClusters do context 'non-authorized user' do it 'responds with 403' do - post api("/groups/#{group.id}/clusters/user", developer_user), params: cluster_params + post api("/groups/#{group.id}/clusters/user", unauthorized_user), params: cluster_params expect(response).to have_gitlab_http_status(:forbidden) end @@ -290,7 +290,7 @@ RSpec.describe API::GroupClusters do context 'non-authorized user' do before do - post api("/groups/#{group.id}/clusters/user", developer_user), params: cluster_params + post api("/groups/#{group.id}/clusters/user", unauthorized_user), params: cluster_params end it 'responds with 403' do @@ -364,7 +364,7 @@ RSpec.describe API::GroupClusters do context 'non-authorized user' do it 'responds with 403' do - put api("/groups/#{group.id}/clusters/#{cluster.id}", developer_user), params: update_params + put api("/groups/#{group.id}/clusters/#{cluster.id}", unauthorized_user), params: update_params expect(response).to have_gitlab_http_status(:forbidden) end @@ -505,7 +505,7 @@ RSpec.describe API::GroupClusters do context 'non-authorized user' do it 'responds with 403' do - delete api("/groups/#{group.id}/clusters/#{cluster.id}", developer_user), params: cluster_params + delete api("/groups/#{group.id}/clusters/#{cluster.id}", unauthorized_user), params: cluster_params expect(response).to have_gitlab_http_status(:forbidden) end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 88c004345fc..7de3567dcdd 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -1163,17 +1163,33 @@ RSpec.describe API::Groups do expect(json_response.length).to eq(3) end - it "returns projects including those in subgroups" do - subgroup = create(:group, parent: group1) - create(:project, group: subgroup) - create(:project, group: subgroup) + context 'when include_subgroups is true' do + it "returns projects including those in subgroups" do + subgroup = create(:group, parent: group1) + create(:project, group: subgroup) + create(:project, group: subgroup) - get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } + get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an(Array) - expect(json_response.length).to eq(5) + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(5) + end + end + + context 'when include_ancestor_groups is true' do + it 'returns ancestors groups projects' do + subgroup = create(:group, parent: group1) + subgroup_project = create(:project, group: subgroup) + + get api("/groups/#{subgroup.id}/projects", user1), params: { include_ancestor_groups: true } + + records = Gitlab::Json.parse(response.body) + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(records.map { |r| r['id'] }).to match_array([project1.id, project3.id, subgroup_project.id, archived_project.id]) + end end it "does not return a non existing group" do diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 9aa8aaafc68..2b7963eadab 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -612,6 +612,30 @@ RSpec.describe API::Internal::Base do expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-mep-mep' => 'false') end end + + context "with a sidechannels enabled for a project" do + before do + stub_feature_flags(gitlab_shell_upload_pack_sidechannel: project) + end + + it "has the use_sidechannel field set to true for that project" do + pull(key, project) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response["gl_repository"]).to eq("project-#{project.id}") + expect(json_response["gitaly"]["use_sidechannel"]).to eq(true) + end + + it "has the use_sidechannel field set to false for other projects" do + other_project = create(:project, :public, :repository) + + pull(key, other_project) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response["gl_repository"]).to eq("project-#{other_project.id}") + expect(json_response["gitaly"]["use_sidechannel"]).to eq(false) + end + end end context "git push" do @@ -724,6 +748,30 @@ RSpec.describe API::Internal::Base do end end + context 'with a pending membership' do + let_it_be(:project) { create(:project, :repository) } + + before_all do + create(:project_member, :awaiting, :developer, source: project, user: user) + end + + it 'returns not found for git pull' do + pull(key, project) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response["status"]).to be_falsey + expect(user.reload.last_activity_on).to be_nil + end + + it 'returns not found for git push' do + push(key, project) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response["status"]).to be_falsey + expect(user.reload.last_activity_on).to be_nil + end + end + context "custom action" do let(:access_checker) { double(Gitlab::GitAccess) } let(:payload) do diff --git a/spec/requests/api/internal/container_registry/migration_spec.rb b/spec/requests/api/internal/container_registry/migration_spec.rb new file mode 100644 index 00000000000..27e99a21c65 --- /dev/null +++ b/spec/requests/api/internal/container_registry/migration_spec.rb @@ -0,0 +1,153 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Internal::ContainerRegistry::Migration do + let_it_be_with_reload(:repository) { create(:container_repository) } + + let(:secret_token) { 'secret_token' } + let(:sent_token) { secret_token } + let(:repository_path) { repository.path } + let(:status) { 'pre_import_complete' } + let(:params) { { path: repository.path, status: status } } + + before do + allow(Gitlab.config.registry).to receive(:notification_secret) { secret_token } + end + + describe 'PUT /internal/registry/repositories/:path/migration/status' do + subject do + put api("/internal/registry/repositories/#{repository_path}/migration/status"), + params: params, + headers: { 'Authorization' => sent_token } + end + + shared_examples 'returning an error' do |with_message: nil, returning_status: :bad_request| + it "returns bad request response" do + expect { subject } + .not_to change { repository.reload.migration_state } + + expect(response).to have_gitlab_http_status(returning_status) + expect(response.body).to include(with_message) if with_message + end + end + + context 'with a valid sent token' do + shared_examples 'updating the repository migration status' do |from:, to:| + it "updates the migration status from #{from} to #{to}" do + expect { subject } + .to change { repository.reload.migration_state }.from(from).to(to) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'with status pre_import_complete' do + let(:status) { 'pre_import_complete' } + + it_behaves_like 'returning an error', with_message: 'Wrong migration state (default)' + + context 'with repository in pre_importing migration state' do + let(:repository) { create(:container_repository, :pre_importing) } + + before do + allow_next_found_instance_of(ContainerRepository) do |found_repository| + allow(found_repository).to receive(:migration_import).and_return(:ok) + end + end + + it_behaves_like 'updating the repository migration status', from: 'pre_importing', to: 'importing' + + context 'with a failing transition' do + before do + allow_next_found_instance_of(ContainerRepository) do |found_repository| + allow(found_repository).to receive(:finish_pre_import_and_start_import).and_return(false) + end + end + + it_behaves_like 'returning an error', with_message: "Couldn't transition from pre_importing to importing" + end + end + + context 'with repository in importing migration state' do + let(:repository) { create(:container_repository, :importing) } + + it_behaves_like 'returning an error', with_message: "Couldn't transition from pre_importing to importing" + end + end + + context 'with status import_complete' do + let(:status) { 'import_complete' } + + it_behaves_like 'returning an error', with_message: 'Wrong migration state (default)' + + context 'with repository in importing migration state' do + let(:repository) { create(:container_repository, :importing) } + let(:transition_result) { true } + + it_behaves_like 'updating the repository migration status', from: 'importing', to: 'import_done' + + context 'with a failing transition' do + before do + allow_next_found_instance_of(ContainerRepository) do |found_repository| + allow(found_repository).to receive(:finish_import).and_return(false) + end + end + + it_behaves_like 'returning an error', with_message: "Couldn't transition from importing to import_done" + end + end + + context 'with repository in pre_importing migration state' do + let(:repository) { create(:container_repository, :pre_importing) } + + it_behaves_like 'returning an error', with_message: "Couldn't transition from importing to import_done" + end + end + + %w[pre_import_failed import_failed].each do |status| + context 'with status pre_import_failed' do + let(:status) { 'pre_import_failed' } + + it_behaves_like 'returning an error', with_message: 'Wrong migration state (default)' + + context 'with repository in importing migration state' do + let(:repository) { create(:container_repository, :importing) } + + it_behaves_like 'updating the repository migration status', from: 'importing', to: 'import_aborted' + end + + context 'with repository in pre_importing migration state' do + let(:repository) { create(:container_repository, :pre_importing) } + + it_behaves_like 'updating the repository migration status', from: 'pre_importing', to: 'import_aborted' + end + end + end + + context 'with a non existing path' do + let(:repository_path) { 'this/does/not/exist' } + + it_behaves_like 'returning an error', returning_status: :not_found + end + + context 'with invalid status' do + let(:params) { super().merge(status: nil).compact } + + it_behaves_like 'returning an error', returning_status: :bad_request + end + + context 'with invalid path' do + let(:repository_path) { nil } + + it_behaves_like 'returning an error', returning_status: :not_found + end + end + + context 'with an invalid sent token' do + let(:sent_token) { 'not_valid' } + + it_behaves_like 'returning an error', returning_status: :unauthorized + end + end +end diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index 9204ee4d7f0..c5e57b5b18b 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -488,6 +488,8 @@ RSpec.describe API::Issues do let_it_be(:issue3) { create(:issue, project: project, author: user, due_date: frozen_time + 10.days) } let_it_be(:issue4) { create(:issue, project: project, author: user, due_date: frozen_time + 34.days) } let_it_be(:issue5) { create(:issue, project: project, author: user, due_date: frozen_time - 8.days) } + let_it_be(:issue6) { create(:issue, project: project, author: user, due_date: frozen_time) } + let_it_be(:issue7) { create(:issue, project: project, author: user, due_date: frozen_time + 1.day) } before do travel_to(frozen_time) @@ -500,7 +502,13 @@ RSpec.describe API::Issues do it 'returns them all when argument is empty' do get api('/issues?due_date=', user) - expect_paginated_array_response(issue5.id, issue4.id, issue3.id, issue2.id, issue.id, closed_issue.id) + expect_paginated_array_response(issue7.id, issue6.id, issue5.id, issue4.id, issue3.id, issue2.id, issue.id, closed_issue.id) + end + + it 'returns issues with due date' do + get api('/issues?due_date=any', user) + + expect_paginated_array_response(issue7.id, issue6.id, issue5.id, issue4.id, issue3.id, issue2.id) end it 'returns issues without due date' do @@ -512,19 +520,31 @@ RSpec.describe API::Issues do it 'returns issues due for this week' do get api('/issues?due_date=week', user) - expect_paginated_array_response(issue2.id) + expect_paginated_array_response(issue7.id, issue6.id, issue2.id) end it 'returns issues due for this month' do get api('/issues?due_date=month', user) - expect_paginated_array_response(issue3.id, issue2.id) + expect_paginated_array_response(issue7.id, issue6.id, issue3.id, issue2.id) end it 'returns issues that are due previous two weeks and next month' do get api('/issues?due_date=next_month_and_previous_two_weeks', user) - expect_paginated_array_response(issue5.id, issue4.id, issue3.id, issue2.id) + expect_paginated_array_response(issue7.id, issue6.id, issue5.id, issue4.id, issue3.id, issue2.id) + end + + it 'returns issues that are due today' do + get api('/issues?due_date=today', user) + + expect_paginated_array_response(issue6.id) + end + + it 'returns issues that are due tomorrow' do + get api('/issues?due_date=tomorrow', user) + + expect_paginated_array_response(issue7.id) end it 'returns issues that are overdue' do @@ -1164,14 +1184,15 @@ RSpec.describe API::Issues do end describe 'PUT /projects/:id/issues/:issue_iid/reorder' do - let_it_be(:project) { create(:project) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } let_it_be(:issue1) { create(:issue, project: project, relative_position: 10) } let_it_be(:issue2) { create(:issue, project: project, relative_position: 20) } let_it_be(:issue3) { create(:issue, project: project, relative_position: 30) } context 'when user has access' do - before do - project.add_developer(user) + before_all do + group.add_developer(user) end context 'with valid params' do @@ -1197,6 +1218,19 @@ RSpec.describe API::Issues do expect(response).to have_gitlab_http_status(:not_found) end end + + context 'with issue in different project' do + let(:other_project) { create(:project, group: group) } + let(:other_issue) { create(:issue, project: other_project, relative_position: 80) } + + it 'reorders issues and returns a successful 200 response' do + put api("/projects/#{other_project.id}/issues/#{other_issue.iid}/reorder", user), params: { move_after_id: issue2.id, move_before_id: issue3.id } + + expect(response).to have_gitlab_http_status(:ok) + expect(other_issue.reload.relative_position) + .to be_between(issue2.reload.relative_position, issue3.reload.relative_position) + end + end end context 'with unauthorized user' do diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb index 7c1e731a99a..73bc4a5d1f3 100644 --- a/spec/requests/api/lint_spec.rb +++ b/spec/requests/api/lint_spec.rb @@ -110,7 +110,7 @@ RSpec.describe API::Lint do context 'when authenticated' do let_it_be(:api_user) { create(:user) } - context 'with valid .gitlab-ci.yaml content' do + context 'with valid .gitlab-ci.yml content' do let(:yaml_content) do File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) end @@ -140,7 +140,7 @@ RSpec.describe API::Lint do end end - context 'with valid .gitlab-ci.yaml with warnings' do + context 'with valid .gitlab-ci.yml with warnings' do let(:yaml_content) { { job: { script: 'ls', rules: [{ when: 'always' }] } }.to_yaml } it 'passes validation but returns warnings' do @@ -153,8 +153,8 @@ RSpec.describe API::Lint do end end - context 'with valid .gitlab-ci.yaml using deprecated keywords' do - let(:yaml_content) { { job: { script: 'ls' }, types: ['test'] }.to_yaml } + context 'with valid .gitlab-ci.yml using deprecated keywords' do + let(:yaml_content) { { job: { script: 'ls', type: 'test' }, types: ['test'] }.to_yaml } it 'passes validation but returns warnings' do post api('/ci/lint', api_user), params: { content: yaml_content } @@ -166,7 +166,7 @@ RSpec.describe API::Lint do end end - context 'with an invalid .gitlab_ci.yml' do + context 'with an invalid .gitlab-ci.yml' do context 'with invalid syntax' do let(:yaml_content) { 'invalid content' } @@ -384,6 +384,15 @@ RSpec.describe API::Lint do project.add_developer(api_user) end + context 'with no commit' do + it 'returns error about providing content' do + ci_lint + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['errors']).to match_array(['Please provide content of .gitlab-ci.yml']) + end + end + context 'with valid .gitlab-ci.yml content' do let(:yaml_content) do { include: { local: 'another-gitlab-ci.yml' }, test: { stage: 'test', script: 'echo 1' } }.to_yaml diff --git a/spec/requests/api/markdown_spec.rb b/spec/requests/api/markdown_spec.rb index faf671d350f..0488bce4663 100644 --- a/spec/requests/api/markdown_spec.rb +++ b/spec/requests/api/markdown_spec.rb @@ -71,7 +71,7 @@ RSpec.describe API::Markdown do end context "when authorized" do - let(:user) { project.owner } + let(:user) { project.first_owner } it_behaves_like "rendered markdown text without GFM" end @@ -97,7 +97,7 @@ RSpec.describe API::Markdown do context "with project" do let(:params) { { text: text, gfm: true, project: project.full_path } } - let(:user) { project.owner } + let(:user) { project.first_owner } it "renders markdown text" do expect(response).to have_gitlab_http_status(:created) diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 02061bb8ab6..6186a43f992 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -416,6 +416,8 @@ RSpec.describe API::Members do end it "returns 409 if member already exists" do + source.add_guest(stranger) + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), params: { user_id: maintainer.id, access_level: Member::MAINTAINER } diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index a751f785913..9e6fea9e5b4 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -436,6 +436,26 @@ RSpec.describe API::MergeRequests do response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) end + + context 'returns an array of merge_requests ordered by title' do + it 'asc when requested' do + path = endpoint_path + '?order_by=title&sort=asc' + + get api(path, user) + + response_titles = json_response.map { |merge_request| merge_request['title'] } + expect(response_titles).to eq(response_titles.sort) + end + + it 'desc when requested' do + path = endpoint_path + '?order_by=title&sort=desc' + + get api(path, user) + + response_titles = json_response.map { |merge_request| merge_request['title'] } + expect(response_titles).to eq(response_titles.sort.reverse) + end + end end context 'NOT params' do @@ -985,14 +1005,6 @@ RSpec.describe API::MergeRequests do it_behaves_like 'merge requests list' - context 'when :api_caching_merge_requests is disabled' do - before do - stub_feature_flags(api_caching_merge_requests: false) - end - - it_behaves_like 'merge requests list' - end - it "returns 404 for non public projects" do project = create(:project, :private) @@ -2876,7 +2888,7 @@ RSpec.describe API::MergeRequests do it 'is false for an unauthorized user' do expect do - put api("/projects/#{target_project.id}/merge_requests/#{merge_request.iid}", target_project.owner), params: { state_event: 'close', remove_source_branch: true } + put api("/projects/#{target_project.id}/merge_requests/#{merge_request.iid}", target_project.first_owner), params: { state_event: 'close', remove_source_branch: true } end.not_to change { merge_request.reload.merge_params } expect(response).to have_gitlab_http_status(:ok) @@ -3324,6 +3336,18 @@ RSpec.describe API::MergeRequests do end end + context 'when merge request branch does not allow force push' do + before do + create(:protected_branch, project: project, name: merge_request.source_branch, allow_force_push: false) + end + + it 'returns 403' do + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/rebase", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + it 'returns 403 if the user cannot push to the branch' do guest = create(:user) project.add_guest(guest) diff --git a/spec/requests/api/package_files_spec.rb b/spec/requests/api/package_files_spec.rb index a7e6a97fd0e..01c7ef1476f 100644 --- a/spec/requests/api/package_files_spec.rb +++ b/spec/requests/api/package_files_spec.rb @@ -87,18 +87,6 @@ RSpec.describe API::PackageFiles do expect(package_file_ids).not_to include(package_file_pending_destruction.id) end - - context 'with packages_installable_package_files disabled' do - before do - stub_feature_flags(packages_installable_package_files: false) - end - - it 'returns them' do - get api(url, user) - - expect(package_file_ids).to include(package_file_pending_destruction.id) - end - end end end end @@ -186,18 +174,6 @@ RSpec.describe API::PackageFiles do expect(response).to have_gitlab_http_status(:not_found) end - - context 'with packages_installable_package_files disabled' do - before do - stub_feature_flags(packages_installable_package_files: false) - end - - it 'can be accessed', :aggregate_failures do - expect { api_request }.not_to change { package.package_files.pending_destruction.count } - - expect(response).to have_gitlab_http_status(:no_content) - end - end end end end diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index 01d2fb18f00..8a6e87944ec 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -121,7 +121,6 @@ project_feature: - created_at - metrics_dashboard_access_level - project_id - - requirements_access_level - security_and_compliance_access_level - updated_at computed_attributes: @@ -139,6 +138,7 @@ project_setting: - has_confluence - has_shimo - has_vulnerabilities + - legacy_open_source_license_available - prevent_merge_without_jira_issue - warn_about_potentially_unwanted_characters - previous_default_branch diff --git a/spec/requests/api/project_clusters_spec.rb b/spec/requests/api/project_clusters_spec.rb index 253b61e5865..b83b41a881a 100644 --- a/spec/requests/api/project_clusters_spec.rb +++ b/spec/requests/api/project_clusters_spec.rb @@ -5,13 +5,15 @@ require 'spec_helper' RSpec.describe API::ProjectClusters do include KubernetesHelpers - let_it_be(:current_user) { create(:user) } + let_it_be(:maintainer_user) { create(:user) } let_it_be(:developer_user) { create(:user) } + let_it_be(:reporter_user) { create(:user) } let_it_be(:project) { create(:project) } before do - project.add_maintainer(current_user) + project.add_maintainer(maintainer_user) project.add_developer(developer_user) + project.add_reporter(reporter_user) end describe 'GET /projects/:id/clusters' do @@ -24,7 +26,7 @@ RSpec.describe API::ProjectClusters do context 'non-authorized user' do it 'responds with 403' do - get api("/projects/#{project.id}/clusters", developer_user) + get api("/projects/#{project.id}/clusters", reporter_user) expect(response).to have_gitlab_http_status(:forbidden) end @@ -32,7 +34,7 @@ RSpec.describe API::ProjectClusters do context 'authorized user' do before do - get api("/projects/#{project.id}/clusters", current_user) + get api("/projects/#{project.id}/clusters", developer_user) end it 'includes pagination headers' do @@ -61,13 +63,13 @@ RSpec.describe API::ProjectClusters do let(:cluster) do create(:cluster, :project, :provided_by_gcp, :with_domain, platform_kubernetes: platform_kubernetes, - user: current_user, + user: maintainer_user, projects: [project]) end context 'non-authorized user' do it 'responds with 403' do - get api("/projects/#{project.id}/clusters/#{cluster_id}", developer_user) + get api("/projects/#{project.id}/clusters/#{cluster_id}", reporter_user) expect(response).to have_gitlab_http_status(:forbidden) end @@ -75,7 +77,7 @@ RSpec.describe API::ProjectClusters do context 'authorized user' do before do - get api("/projects/#{project.id}/clusters/#{cluster_id}", current_user) + get api("/projects/#{project.id}/clusters/#{cluster_id}", developer_user) end it 'returns specific cluster' do @@ -111,8 +113,8 @@ RSpec.describe API::ProjectClusters do it 'returns user information' do user = json_response['user'] - expect(user['id']).to eq(current_user.id) - expect(user['username']).to eq(current_user.username) + expect(user['id']).to eq(maintainer_user.id) + expect(user['username']).to eq(maintainer_user.username) end it 'returns GCP provider information' do @@ -156,7 +158,7 @@ RSpec.describe API::ProjectClusters do let(:management_project_id) { management_project.id } before do - management_project.add_maintainer(current_user) + management_project.add_maintainer(maintainer_user) end let(:platform_kubernetes_attributes) do @@ -190,7 +192,7 @@ RSpec.describe API::ProjectClusters do context 'authorized user' do before do - post api("/projects/#{project.id}/clusters/user", current_user), params: cluster_params + post api("/projects/#{project.id}/clusters/user", maintainer_user), params: cluster_params end context 'with valid params' do @@ -317,7 +319,7 @@ RSpec.describe API::ProjectClusters do create(:cluster, :provided_by_gcp, :project, projects: [project]) - post api("/projects/#{project.id}/clusters/user", current_user), params: cluster_params + post api("/projects/#{project.id}/clusters/user", maintainer_user), params: cluster_params end it 'responds with 201' do @@ -369,9 +371,9 @@ RSpec.describe API::ProjectClusters do context 'authorized user' do before do - management_project.add_maintainer(current_user) + management_project.add_maintainer(maintainer_user) - put api("/projects/#{project.id}/clusters/#{cluster.id}", current_user), params: update_params + put api("/projects/#{project.id}/clusters/#{cluster.id}", maintainer_user), params: update_params cluster.reload end @@ -501,7 +503,7 @@ RSpec.describe API::ProjectClusters do context 'authorized user' do before do - delete api("/projects/#{project.id}/clusters/#{cluster.id}", current_user), params: cluster_params + delete api("/projects/#{project.id}/clusters/#{cluster.id}", maintainer_user), params: cluster_params end it 'deletes the cluster' do diff --git a/spec/requests/api/project_export_spec.rb b/spec/requests/api/project_export_spec.rb index b9c458373a8..2bc31153f2c 100644 --- a/spec/requests/api/project_export_spec.rb +++ b/spec/requests/api/project_export_spec.rb @@ -450,7 +450,7 @@ RSpec.describe API::ProjectExport, :clean_gitlab_redis_cache do expect_next_instance_of(Projects::ImportExport::ExportService) do |service| expect(service).to receive(:execute) end - post api(path, project.owner), params: params + post api(path, project.first_owner), params: params expect(response).to have_gitlab_http_status(:accepted) end diff --git a/spec/requests/api/project_snapshots_spec.rb b/spec/requests/api/project_snapshots_spec.rb index 33c86d56ed4..bf78ff56206 100644 --- a/spec/requests/api/project_snapshots_spec.rb +++ b/spec/requests/api/project_snapshots_spec.rb @@ -33,7 +33,7 @@ RSpec.describe API::ProjectSnapshots do end it 'returns authentication error as project owner' do - get api("/projects/#{project.id}/snapshot", project.owner) + get api("/projects/#{project.id}/snapshot", project.first_owner) expect(response).to have_gitlab_http_status(:forbidden) end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index bf41a808219..02df82d14a8 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -30,7 +30,7 @@ RSpec.shared_examples 'languages and percentages JSON response' do context 'when the languages were detected before' do before do - Projects::DetectRepositoryLanguagesService.new(project, project.owner).execute + Projects::DetectRepositoryLanguagesService.new(project, project.first_owner).execute end it 'returns the detection from the database' do @@ -2166,6 +2166,7 @@ RSpec.describe API::Projects do approvals_before_merge compliance_frameworks mirror + requirements_access_level requirements_enabled security_and_compliance_enabled issues_template @@ -2710,7 +2711,7 @@ RSpec.describe API::Projects do it 'returns the project users' do get api("/projects/#{project.id}/users", current_user) - user = project.namespace.owner + user = project.namespace.first_owner expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 21a8622e08d..f42fc7aabc2 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -561,17 +561,6 @@ RSpec.describe API::Repositories do let(:request) { get api(route, guest) } end end - - context 'api_caching_rate_limit_repository_compare is disabled' do - before do - stub_feature_flags(api_caching_rate_limit_repository_compare: false) - end - - it_behaves_like 'repository compare' do - let(:project) { create(:project, :public, :repository) } - let(:current_user) { nil } - end - end end describe 'GET /projects/:id/repository/contributors' do diff --git a/spec/requests/api/rubygem_packages_spec.rb b/spec/requests/api/rubygem_packages_spec.rb index 0e63a7269e7..f0408d94137 100644 --- a/spec/requests/api/rubygem_packages_spec.rb +++ b/spec/requests/api/rubygem_packages_spec.rb @@ -187,19 +187,6 @@ RSpec.describe API::RubygemPackages do expect(response).to have_gitlab_http_status(:ok) expect(response.body).not_to eq(package_file_pending_destruction.file.file.read) end - - context 'with packages_installable_package_files disabled' do - before do - stub_feature_flags(packages_installable_package_files: false) - end - - it 'returns them' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to eq(package_file_pending_destruction.file.file.read) - end - end end end diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 7e940d52a41..f7048a1ca6b 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -32,6 +32,8 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['dsa_key_restriction']).to eq(0) expect(json_response['ecdsa_key_restriction']).to eq(0) expect(json_response['ed25519_key_restriction']).to eq(0) + expect(json_response['ecdsa_sk_key_restriction']).to eq(0) + expect(json_response['ed25519_sk_key_restriction']).to eq(0) expect(json_response['performance_bar_allowed_group_id']).to be_nil expect(json_response['allow_local_requests_from_hooks_and_services']).to be(false) expect(json_response['allow_local_requests_from_web_hooks_and_services']).to be(false) @@ -49,6 +51,9 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['whats_new_variant']).to eq('all_tiers') expect(json_response['user_deactivation_emails_enabled']).to be(true) expect(json_response['suggest_pipeline_enabled']).to be(true) + expect(json_response['runner_token_expiration_interval']).to be_nil + expect(json_response['group_runner_token_expiration_interval']).to be_nil + expect(json_response['project_runner_token_expiration_interval']).to be_nil end end @@ -111,6 +116,8 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do dsa_key_restriction: 2048, ecdsa_key_restriction: 384, ed25519_key_restriction: 256, + ecdsa_sk_key_restriction: 256, + ed25519_sk_key_restriction: 256, enforce_terms: true, terms: 'Hello world!', performance_bar_allowed_group_path: group.full_path, @@ -137,7 +144,8 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do personal_access_token_prefix: "GL-", user_deactivation_emails_enabled: false, admin_mode: true, - suggest_pipeline_enabled: false + suggest_pipeline_enabled: false, + users_get_by_id_limit: 456 } expect(response).to have_gitlab_http_status(:ok) @@ -163,6 +171,8 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['dsa_key_restriction']).to eq(2048) expect(json_response['ecdsa_key_restriction']).to eq(384) expect(json_response['ed25519_key_restriction']).to eq(256) + expect(json_response['ecdsa_sk_key_restriction']).to eq(256) + expect(json_response['ed25519_sk_key_restriction']).to eq(256) expect(json_response['enforce_terms']).to be(true) expect(json_response['terms']).to eq('Hello world!') expect(json_response['performance_bar_allowed_group_id']).to eq(group.id) @@ -190,6 +200,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['admin_mode']).to be(true) expect(json_response['user_deactivation_emails_enabled']).to be(false) expect(json_response['suggest_pipeline_enabled']).to be(false) + expect(json_response['users_get_by_id_limit']).to eq(456) end end @@ -644,5 +655,37 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do end end end + + context 'runner token expiration_intervals' do + it 'updates the settings' do + put api("/application/settings", admin), params: { + runner_token_expiration_interval: 3600, + group_runner_token_expiration_interval: 3600 * 2, + project_runner_token_expiration_interval: 3600 * 3 + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include( + 'runner_token_expiration_interval' => 3600, + 'group_runner_token_expiration_interval' => 3600 * 2, + 'project_runner_token_expiration_interval' => 3600 * 3 + ) + end + + it 'updates the settings with empty values' do + put api("/application/settings", admin), params: { + runner_token_expiration_interval: nil, + group_runner_token_expiration_interval: nil, + project_runner_token_expiration_interval: nil + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include( + 'runner_token_expiration_interval' => nil, + 'group_runner_token_expiration_interval' => nil, + 'project_runner_token_expiration_interval' => nil + ) + end + end end end diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index bb56192a2ff..3558babf2f1 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -16,250 +16,232 @@ RSpec.describe API::Tags do project.add_developer(user) end - describe 'GET /projects/:id/repository/tags' do + describe 'GET /projects/:id/repository/tags', :use_clean_rails_memory_store_caching do before do stub_feature_flags(tag_list_keyset_pagination: false) end - shared_examples "get repository tags" do - let(:route) { "/projects/#{project_id}/repository/tags" } + let(:route) { "/projects/#{project_id}/repository/tags" } - context 'sorting' do - let(:current_user) { user } + context 'sorting' do + let(:current_user) { user } - it 'sorts by descending order by default' do - get api(route, current_user) + it 'sorts by descending order by default' do + get api(route, current_user) - desc_order_tags = project.repository.tags.sort_by { |tag| tag.dereferenced_target.committed_date } - desc_order_tags.reverse!.map! { |tag| tag.dereferenced_target.id } + desc_order_tags = project.repository.tags.sort_by { |tag| tag.dereferenced_target.committed_date } + desc_order_tags.reverse!.map! { |tag| tag.dereferenced_target.id } - expect(json_response.map { |tag| tag['commit']['id'] }).to eq(desc_order_tags) - end + expect(json_response.map { |tag| tag['commit']['id'] }).to eq(desc_order_tags) + end - it 'sorts by ascending order if specified' do - get api("#{route}?sort=asc", current_user) + it 'sorts by ascending order if specified' do + get api("#{route}?sort=asc", current_user) - asc_order_tags = project.repository.tags.sort_by { |tag| tag.dereferenced_target.committed_date } - asc_order_tags.map! { |tag| tag.dereferenced_target.id } + asc_order_tags = project.repository.tags.sort_by { |tag| tag.dereferenced_target.committed_date } + asc_order_tags.map! { |tag| tag.dereferenced_target.id } - expect(json_response.map { |tag| tag['commit']['id'] }).to eq(asc_order_tags) - end + expect(json_response.map { |tag| tag['commit']['id'] }).to eq(asc_order_tags) + end - it 'sorts by name in descending order when requested' do - get api("#{route}?order_by=name", current_user) + it 'sorts by name in descending order when requested' do + get api("#{route}?order_by=name", current_user) - ordered_by_name = project.repository.tags.map { |tag| tag.name }.sort.reverse + ordered_by_name = project.repository.tags.map { |tag| tag.name }.sort.reverse - expect(json_response.map { |tag| tag['name'] }).to eq(ordered_by_name) - end + expect(json_response.map { |tag| tag['name'] }).to eq(ordered_by_name) + end - it 'sorts by name in ascending order when requested' do - get api("#{route}?order_by=name&sort=asc", current_user) + it 'sorts by name in ascending order when requested' do + get api("#{route}?order_by=name&sort=asc", current_user) - ordered_by_name = project.repository.tags.map { |tag| tag.name }.sort + ordered_by_name = project.repository.tags.map { |tag| tag.name }.sort - expect(json_response.map { |tag| tag['name'] }).to eq(ordered_by_name) - end + expect(json_response.map { |tag| tag['name'] }).to eq(ordered_by_name) end + end - context 'searching' do - it 'only returns searched tags' do - get api("#{route}", user), params: { search: 'v1.1.0' } + context 'searching' do + it 'only returns searched tags' do + get api("#{route}", user), params: { search: 'v1.1.0' } - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.size).to eq(1) - expect(json_response[0]['name']).to eq('v1.1.0') - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.size).to eq(1) + expect(json_response[0]['name']).to eq('v1.1.0') end + end - shared_examples_for 'repository tags' do - it 'returns the repository tags' do - get api(route, current_user) + shared_examples_for 'repository tags' do + it 'returns the repository tags' do + get api(route, current_user) - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('public_api/v4/tags') - expect(response).to include_pagination_headers - expect(json_response.map { |r| r['name'] }).to include(tag_name) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/tags') + expect(response).to include_pagination_headers + expect(json_response.map { |r| r['name'] }).to include(tag_name) + end - context 'when repository is disabled' do - include_context 'disabled repository' + context 'when repository is disabled' do + include_context 'disabled repository' - it_behaves_like '403 response' do - let(:request) { get api(route, current_user) } - end + it_behaves_like '403 response' do + let(:request) { get api(route, current_user) } end end + end - context 'when unauthenticated', 'and project is public' do - let(:project) { create(:project, :public, :repository) } + context 'when unauthenticated', 'and project is public' do + let(:project) { create(:project, :public, :repository) } - it_behaves_like 'repository tags' - end + it_behaves_like 'repository tags' + end - context 'when unauthenticated', 'and project is private' do - it_behaves_like '404 response' do - let(:request) { get api(route) } - let(:message) { '404 Project Not Found' } - end + context 'when unauthenticated', 'and project is private' do + it_behaves_like '404 response' do + let(:request) { get api(route) } + let(:message) { '404 Project Not Found' } end + end - context 'when authenticated', 'as a maintainer' do - let(:current_user) { user } + context 'when authenticated', 'as a maintainer' do + let(:current_user) { user } - it_behaves_like 'repository tags' + it_behaves_like 'repository tags' - context 'requesting with the escaped project full path' do - let(:project_id) { CGI.escape(project.full_path) } + context 'requesting with the escaped project full path' do + let(:project_id) { CGI.escape(project.full_path) } - it_behaves_like 'repository tags' - end + it_behaves_like 'repository tags' end + end - context 'when authenticated', 'as a guest' do - it_behaves_like '403 response' do - let(:request) { get api(route, guest) } - end + context 'when authenticated', 'as a guest' do + it_behaves_like '403 response' do + let(:request) { get api(route, guest) } end + end - context 'with releases' do - let(:description) { 'Awesome release!' } + context 'with releases' do + let(:description) { 'Awesome release!' } - let!(:release) do - create(:release, - :legacy, - project: project, - tag: tag_name, - description: description) - end + let!(:release) do + create(:release, + :legacy, + project: project, + tag: tag_name, + description: description) + end - it 'returns an array of project tags with release info' do - get api(route, user) + it 'returns an array of project tags with release info' do + get api(route, user) - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('public_api/v4/tags') - expect(response).to include_pagination_headers + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/tags') + expect(response).to include_pagination_headers - expected_tag = json_response.find { |r| r['name'] == tag_name } - expect(expected_tag['message']).to eq(tag_message) - expect(expected_tag['release']['description']).to eq(description) - end + expected_tag = json_response.find { |r| r['name'] == tag_name } + expect(expected_tag['message']).to eq(tag_message) + expect(expected_tag['release']['description']).to eq(description) end + end - context 'with keyset pagination on', :aggregate_errors do - before do - stub_feature_flags(tag_list_keyset_pagination: true) - end + context 'with keyset pagination on', :aggregate_errors do + before do + stub_feature_flags(tag_list_keyset_pagination: true) + end - context 'with keyset pagination option' do - let(:base_params) { { pagination: 'keyset' } } + context 'with keyset pagination option' do + let(:base_params) { { pagination: 'keyset' } } - context 'with gitaly pagination params' do - context 'with high limit' do - let(:params) { base_params.merge(per_page: 100) } + context 'with gitaly pagination params' do + context 'with high limit' do + let(:params) { base_params.merge(per_page: 100) } - it 'returns all repository tags' do - get api(route, user), params: params + it 'returns all repository tags' do + get api(route, user), params: params - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('public_api/v4/tags') - expect(response.headers).not_to include('Link') - tag_names = json_response.map { |x| x['name'] } - expect(tag_names).to match_array(project.repository.tag_names) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/tags') + expect(response.headers).not_to include('Link') + tag_names = json_response.map { |x| x['name'] } + expect(tag_names).to match_array(project.repository.tag_names) end + end - context 'with low limit' do - let(:params) { base_params.merge(per_page: 2) } + context 'with low limit' do + let(:params) { base_params.merge(per_page: 2) } - it 'returns limited repository tags' do - get api(route, user), params: params + it 'returns limited repository tags' do + get api(route, user), params: params - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('public_api/v4/tags') - expect(response.headers).to include('Link') - tag_names = json_response.map { |x| x['name'] } - expect(tag_names).to match_array(%w(v1.1.0 v1.1.1)) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/tags') + expect(response.headers).to include('Link') + tag_names = json_response.map { |x| x['name'] } + expect(tag_names).to match_array(%w(v1.1.0 v1.1.1)) end + end - context 'with missing page token' do - let(:params) { base_params.merge(page_token: 'unknown') } + context 'with missing page token' do + let(:params) { base_params.merge(page_token: 'unknown') } - it_behaves_like '422 response' do - let(:request) { get api(route, user), params: params } - let(:message) { 'Invalid page token: refs/tags/unknown' } - end + it_behaves_like '422 response' do + let(:request) { get api(route, user), params: params } + let(:message) { 'Invalid page token: refs/tags/unknown' } end end end end end - context ":api_caching_tags flag enabled", :use_clean_rails_memory_store_caching do + describe "cache expiry" do + let(:route) { "/projects/#{project_id}/repository/tags" } + let(:current_user) { user } + before do - stub_feature_flags(api_caching_tags: true) + # Set the cache + get api(route, current_user) end - it_behaves_like "get repository tags" - - describe "cache expiry" do - let(:route) { "/projects/#{project_id}/repository/tags" } - let(:current_user) { user } + it "is cached" do + expect(API::Entities::Tag).not_to receive(:represent) - before do - # Set the cache - get api(route, current_user) - end + get api(route, current_user) + end - it "is cached" do - expect(API::Entities::Tag).not_to receive(:represent) + shared_examples "cache expired" do + it "isn't cached" do + expect(API::Entities::Tag).to receive(:represent).exactly(3).times get api(route, current_user) end + end - shared_examples "cache expired" do - it "isn't cached" do - expect(API::Entities::Tag).to receive(:represent).exactly(3).times - - get api(route, current_user) - end - end - - context "when protected tag is changed" do - before do - create(:protected_tag, name: tag_name, project: project) - end - - it_behaves_like "cache expired" + context "when protected tag is changed" do + before do + create(:protected_tag, name: tag_name, project: project) end - context "when release is changed" do - before do - create(:release, :legacy, project: project, tag: tag_name) - end + it_behaves_like "cache expired" + end - it_behaves_like "cache expired" + context "when release is changed" do + before do + create(:release, :legacy, project: project, tag: tag_name) end - context "when project is changed" do - before do - project.touch - end + it_behaves_like "cache expired" + end - it_behaves_like "cache expired" + context "when project is changed" do + before do + project.touch end - end - end - context ":api_caching_tags flag disabled" do - before do - stub_feature_flags(api_caching_tags: false) + it_behaves_like "cache expired" end - - it_behaves_like "get repository tags" end context 'when gitaly is unavailable' do diff --git a/spec/requests/api/terraform/modules/v1/packages_spec.rb b/spec/requests/api/terraform/modules/v1/packages_spec.rb index 8160113bbde..7d86244cb1b 100644 --- a/spec/requests/api/terraform/modules/v1/packages_spec.rb +++ b/spec/requests/api/terraform/modules/v1/packages_spec.rb @@ -232,20 +232,6 @@ RSpec.describe API::Terraform::Modules::V1::Packages do expect(response.body).not_to eq(package_file_pending_destruction.file.file.read) expect(response.body).to eq(package_file.file.file.read) end - - context 'with packages_installable_package_files disabled' do - before do - stub_feature_flags(packages_installable_package_files: false) - end - - it 'returns them' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to eq(package_file_pending_destruction.file.file.read) - expect(response.body).not_to eq(package_file.file.file.read) - end - end end end diff --git a/spec/requests/api/usage_data_spec.rb b/spec/requests/api/usage_data_spec.rb index bacaf960e6a..aefccc4fbf7 100644 --- a/spec/requests/api/usage_data_spec.rb +++ b/spec/requests/api/usage_data_spec.rb @@ -57,13 +57,26 @@ RSpec.describe API::UsageData do end end - %w[merge_requests commits].each do |postfix| - context 'with correct params' do - let(:known_event_postfix) { postfix } + context 'with correct params' do + using RSpec::Parameterized::TableSyntax + + where(:prefix, :event) do + 'static_site_editor' | 'merge_requests' + 'static_site_editor' | 'commits' + end + + before do + stub_application_setting(usage_ping_enabled: true) + stub_feature_flags(usage_data_api: true) + allow(Gitlab::RequestForgeryProtection).to receive(:verified?).and_return(true) + stub_feature_flags("usage_data_#{prefix}_#{event}" => true) + end + + with_them do + it 'returns status :ok' do + expect(Gitlab::UsageDataCounters::BaseCounter).to receive(:count).with(event) - it 'returns status ok' do - expect(Gitlab::UsageDataCounters::BaseCounter).to receive(:count).with(known_event_postfix) - post api(endpoint, user), params: { event: known_event } + post api(endpoint, user), params: { event: "#{prefix}_#{event}" } expect(response).to have_gitlab_http_status(:ok) end @@ -73,6 +86,7 @@ RSpec.describe API::UsageData do context 'with unknown event' do before do skip_feature_flags_yaml_validation + skip_default_enabled_yaml_check end it 'returns status ok' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 98875d7e8d2..985e07bf174 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -499,7 +499,8 @@ RSpec.describe API::Users do let_it_be(:user2, reload: true) { create(:user, username: 'another_user') } before do - allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:users_get_by_id, scope: user).and_return(false) + allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?) + .with(:users_get_by_id, scope: user, users_allowlist: []).and_return(false) end it "returns a user by id" do @@ -600,7 +601,7 @@ RSpec.describe API::Users do context 'when the rate limit is not exceeded' do it 'returns a success status' do expect(Gitlab::ApplicationRateLimiter) - .to receive(:throttled?).with(:users_get_by_id, scope: user) + .to receive(:throttled?).with(:users_get_by_id, scope: user, users_allowlist: []) .and_return(false) get api("/users/#{user.id}", user) @@ -613,7 +614,7 @@ RSpec.describe API::Users do context 'when feature flag is enabled' do it 'returns "too many requests" status' do expect(Gitlab::ApplicationRateLimiter) - .to receive(:throttled?).with(:users_get_by_id, scope: user) + .to receive(:throttled?).with(:users_get_by_id, scope: user, users_allowlist: []) .and_return(true) get api("/users/#{user.id}", user) @@ -629,6 +630,24 @@ RSpec.describe API::Users do expect(response).to have_gitlab_http_status(:ok) end + + it 'allows users whose username is in the allowlist' do + allowlist = [user.username] + current_settings = Gitlab::CurrentSettings.current_application_settings + + # Necessary to ensure the same object is returned on each call + allow(Gitlab::CurrentSettings).to receive(:current_application_settings).and_return current_settings + + allow(current_settings).to receive(:users_get_by_id_limit_allowlist).and_return(allowlist) + + expect(Gitlab::ApplicationRateLimiter) + .to receive(:throttled?).with(:users_get_by_id, scope: user, users_allowlist: allowlist) + .and_call_original + + get api("/users/#{user.id}", user) + + expect(response).to have_gitlab_http_status(:ok) + end end context 'when feature flag is disabled' do diff --git a/spec/requests/boards/lists_controller_spec.rb b/spec/requests/boards/lists_controller_spec.rb index 4d9f1dace4d..47f4925d5b0 100644 --- a/spec/requests/boards/lists_controller_spec.rb +++ b/spec/requests/boards/lists_controller_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Boards::ListsController do describe '#index' do let(:board) { create(:board) } - let(:user) { board.project.owner } + let(:user) { board.project.first_owner } it 'does not have N+1 queries' do login_as(user) diff --git a/spec/requests/concerns/planning_hierarchy_spec.rb b/spec/requests/concerns/planning_hierarchy_spec.rb new file mode 100644 index 00000000000..ece9270b3a1 --- /dev/null +++ b/spec/requests/concerns/planning_hierarchy_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe PlanningHierarchy, type: :request do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + + before do + project.add_maintainer(user) + sign_in(user) + end + + describe 'GET #planning_hierarchy' do + it 'renders planning hierarchy' do + get project_planning_hierarchy_path(project) + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to match(/id="js-work-items-hierarchy"/) + end + end +end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 623cf24b9cb..340ed7bde53 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -836,6 +836,24 @@ RSpec.describe 'Git HTTP requests' do end end end + + context "when the user is admin" do + let(:admin) { create(:admin) } + let(:env) { { user: admin.username, password: admin.password } } + + # Currently, the admin mode is bypassed for git operations. + # Once the admin mode is considered for git operations, this test will fail. + # Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/296509 + context 'when admin mode is enabled', :enable_admin_mode do + it_behaves_like 'pulls are allowed' + it_behaves_like 'pushes are allowed' + end + + context 'when admin mode is disabled' do + it_behaves_like 'pulls are allowed' + it_behaves_like 'pushes are allowed' + end + end end end @@ -929,10 +947,10 @@ RSpec.describe 'Git HTTP requests' do context 'when admin mode is disabled' do it_behaves_like 'can download code only' - it 'downloads from other project get status 404' do + it 'downloads from other project get status 403' do clone_get "#{other_project.full_path}.git", user: 'gitlab-ci-token', password: build.token - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:forbidden) end end end @@ -1534,10 +1552,10 @@ RSpec.describe 'Git HTTP requests' do context 'when admin mode is disabled' do it_behaves_like 'can download code only' - it 'downloads from other project get status 404' do + it 'downloads from other project get status 403' do clone_get "#{other_project.full_path}.git", user: 'gitlab-ci-token', password: build.token - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:forbidden) end end end diff --git a/spec/requests/import/gitlab_projects_controller_spec.rb b/spec/requests/import/gitlab_projects_controller_spec.rb index 58843a7fec4..eed035608d0 100644 --- a/spec/requests/import/gitlab_projects_controller_spec.rb +++ b/spec/requests/import/gitlab_projects_controller_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Import::GitlabProjectsController do include_context 'workhorse headers' let_it_be(:namespace) { create(:namespace) } - let_it_be(:user) { namespace.owner } + let_it_be(:user) { namespace.first_owner } before do login_as(user) diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index f89395fccaf..4b2f11da77e 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -546,14 +546,6 @@ RSpec.describe 'Git LFS API and storage' do expect(lfs_object.reload.projects.pluck(:id)).to match_array([other_project.id, project.id]) end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(lfs_auto_link_fork_source: false) - end - - it_behaves_like 'batch upload with existing LFS object' - end end context 'when user does not have access to parent' do diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 8ee752da44e..70a310ba0d5 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -275,7 +275,7 @@ RSpec.describe 'OpenID Connect requests' do expect(response).to have_gitlab_http_status(:ok) expect(json_response['issuer']).to eq('http://localhost') expect(json_response['jwks_uri']).to eq('http://www.example.com/oauth/discovery/keys') - expect(json_response['scopes_supported']).to eq(%w[api read_user read_api read_repository write_repository sudo openid profile email]) + expect(json_response['scopes_supported']).to match_array %w[api read_user read_api read_repository write_repository sudo openid profile email] end context 'with a cross-origin request' do @@ -285,7 +285,7 @@ RSpec.describe 'OpenID Connect requests' do expect(response).to have_gitlab_http_status(:ok) expect(json_response['issuer']).to eq('http://localhost') expect(json_response['jwks_uri']).to eq('http://www.example.com/oauth/discovery/keys') - expect(json_response['scopes_supported']).to eq(%w[api read_user read_api read_repository write_repository sudo openid profile email]) + expect(json_response['scopes_supported']).to match_array %w[api read_user read_api read_repository write_repository sudo openid profile email] end it_behaves_like 'cross-origin GET request' diff --git a/spec/requests/projects/cluster_agents_controller_spec.rb b/spec/requests/projects/cluster_agents_controller_spec.rb index e4c4f537699..914d5b17ba8 100644 --- a/spec/requests/projects/cluster_agents_controller_spec.rb +++ b/spec/requests/projects/cluster_agents_controller_spec.rb @@ -14,7 +14,7 @@ RSpec.describe Projects::ClusterAgentsController do let_it_be(:user) { create(:user) } before do - project.add_developer(user) + project.add_reporter(user) sign_in(user) subject end diff --git a/spec/requests/projects/clusters/integrations_controller_spec.rb b/spec/requests/projects/clusters/integrations_controller_spec.rb index 323c61b9af3..c05e3da675c 100644 --- a/spec/requests/projects/clusters/integrations_controller_spec.rb +++ b/spec/requests/projects/clusters/integrations_controller_spec.rb @@ -28,7 +28,7 @@ RSpec.describe Projects::Clusters::IntegrationsController do describe 'POST create_or_update' do let(:cluster) { create(:cluster, :project, :provided_by_gcp) } let(:project) { cluster.project } - let(:user) { project.owner } + let(:user) { project.first_owner } it_behaves_like '#create_or_update action' do let(:path) { create_or_update_project_cluster_integration_path(project, cluster) } diff --git a/spec/requests/projects/google_cloud/deployments_controller_spec.rb b/spec/requests/projects/google_cloud/deployments_controller_spec.rb index a5eccc43147..fd356bc61c7 100644 --- a/spec/requests/projects/google_cloud/deployments_controller_spec.rb +++ b/spec/requests/projects/google_cloud/deployments_controller_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe Projects::GoogleCloud::DeploymentsController do - let_it_be(:project) { create(:project, :public) } + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:repository) { project.repository } let_it_be(:user_guest) { create(:user) } let_it_be(:user_developer) { create(:user) } @@ -36,8 +37,6 @@ RSpec.describe Projects::GoogleCloud::DeploymentsController do it 'returns not found on GET request' do urls_list.each do |url| unauthorized_members.each do |unauthorized_member| - sign_in(unauthorized_member) - get url expect(response).to have_gitlab_http_status(:not_found) @@ -65,18 +64,63 @@ RSpec.describe Projects::GoogleCloud::DeploymentsController do let_it_be(:url) { "#{project_google_cloud_deployments_cloud_run_path(project)}" } before do + sign_in(user_maintainer) + allow_next_instance_of(GoogleApi::CloudPlatform::Client) do |client| allow(client).to receive(:validate_token).and_return(true) end end - it 'renders placeholder' do - authorized_members.each do |authorized_member| - sign_in(authorized_member) + it 'redirects to google_cloud home on enable service error' do + # since GPC_PROJECT_ID is not set, enable cloud run service should return an error + + get url + + expect(response).to redirect_to(project_google_cloud_index_path(project)) + end + + it 'tracks error and redirects to gcp_error' do + mock_google_error = Google::Apis::ClientError.new('some_error') + + allow_next_instance_of(GoogleCloud::EnableCloudRunService) do |service| + allow(service).to receive(:execute).and_raise(mock_google_error) + end + + expect(Gitlab::ErrorTracking).to receive(:track_exception).with(mock_google_error, { project_id: project.id }) + + get url + + expect(response).to render_template(:gcp_error) + end + + context 'GCP_PROJECT_IDs are defined' do + it 'redirects to google_cloud home on generate pipeline error' do + allow_next_instance_of(GoogleCloud::EnableCloudRunService) do |enable_cloud_run_service| + allow(enable_cloud_run_service).to receive(:execute).and_return({ status: :success }) + end + + allow_next_instance_of(GoogleCloud::GeneratePipelineService) do |generate_pipeline_service| + allow(generate_pipeline_service).to receive(:execute).and_return({ status: :error }) + end get url - expect(response).to have_gitlab_http_status(:ok) + expect(response).to redirect_to(project_google_cloud_index_path(project)) + end + + it 'redirects to create merge request form' do + allow_next_instance_of(GoogleCloud::EnableCloudRunService) do |service| + allow(service).to receive(:execute).and_return({ status: :success }) + end + + allow_next_instance_of(GoogleCloud::GeneratePipelineService) do |service| + allow(service).to receive(:execute).and_return({ status: :success }) + end + + get url + + expect(response).to have_gitlab_http_status(:found) + expect(response.location).to include(project_new_merge_request_path(project)) end end end diff --git a/spec/requests/projects/google_cloud/service_accounts_controller_spec.rb b/spec/requests/projects/google_cloud/service_accounts_controller_spec.rb index 6b4d1c490e2..0f243a6a7a9 100644 --- a/spec/requests/projects/google_cloud/service_accounts_controller_spec.rb +++ b/spec/requests/projects/google_cloud/service_accounts_controller_spec.rb @@ -2,10 +2,6 @@ require 'spec_helper' -# Mock Types -MockGoogleOAuth2Credentials = Struct.new(:app_id, :app_secret) -MockServiceAccount = Struct.new(:project_id, :unique_id) - RSpec.describe Projects::GoogleCloud::ServiceAccountsController do let_it_be(:project) { create(:project, :public) } @@ -86,10 +82,12 @@ RSpec.describe Projects::GoogleCloud::ServiceAccountsController do context 'and user has successfully completed the google oauth2 flow' do before do allow_next_instance_of(GoogleApi::CloudPlatform::Client) do |client| + mock_service_account = Struct.new(:project_id, :unique_id, :email).new(123, 456, 'em@ai.l') allow(client).to receive(:validate_token).and_return(true) allow(client).to receive(:list_projects).and_return([{}, {}, {}]) - allow(client).to receive(:create_service_account).and_return(MockServiceAccount.new(123, 456)) + allow(client).to receive(:create_service_account).and_return(mock_service_account) allow(client).to receive(:create_service_account_key).and_return({}) + allow(client).to receive(:grant_service_account_roles) end end @@ -147,7 +145,8 @@ RSpec.describe Projects::GoogleCloud::ServiceAccountsController do context 'but gitlab instance is not configured for google oauth2' do before do - unconfigured_google_oauth2 = MockGoogleOAuth2Credentials.new('', '') + unconfigured_google_oauth2 = Struct.new(:app_id, :app_secret) + .new('', '') allow(Gitlab::Auth::OAuth::Provider).to receive(:config_for) .with('google_oauth2') .and_return(unconfigured_google_oauth2) diff --git a/spec/requests/projects/merge_requests/creations_spec.rb b/spec/requests/projects/merge_requests/creations_spec.rb index 0a3e663444f..842ad01656e 100644 --- a/spec/requests/projects/merge_requests/creations_spec.rb +++ b/spec/requests/projects/merge_requests/creations_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'merge requests creations' do include ProjectForksHelper let(:project) { create(:project, :repository) } - let(:user) { project.owner } + let(:user) { project.first_owner } before do login_as(user) diff --git a/spec/requests/projects/merge_requests_discussions_spec.rb b/spec/requests/projects/merge_requests_discussions_spec.rb index 6cf7bfb1795..c761af86c16 100644 --- a/spec/requests/projects/merge_requests_discussions_spec.rb +++ b/spec/requests/projects/merge_requests_discussions_spec.rb @@ -6,7 +6,7 @@ RSpec.describe 'merge requests discussions' do # Further tests can be found at merge_requests_controller_spec.rb describe 'GET /:namespace/:project/-/merge_requests/:iid/discussions' do let(:project) { create(:project, :repository, :public) } - let(:owner) { project.owner } + let(:owner) { project.first_owner } let(:user) { create(:user) } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } diff --git a/spec/requests/projects/merge_requests_spec.rb b/spec/requests/projects/merge_requests_spec.rb index 59fde803560..91153554e55 100644 --- a/spec/requests/projects/merge_requests_spec.rb +++ b/spec/requests/projects/merge_requests_spec.rb @@ -12,7 +12,7 @@ RSpec.describe 'merge requests actions' do reviewers: [user2]) end - let(:user) { project.owner } + let(:user) { project.first_owner } let(:user2) { create(:user) } before do diff --git a/spec/requests/projects/metrics_dashboard_spec.rb b/spec/requests/projects/metrics_dashboard_spec.rb index c248463faa3..61bfe1c6edf 100644 --- a/spec/requests/projects/metrics_dashboard_spec.rb +++ b/spec/requests/projects/metrics_dashboard_spec.rb @@ -6,7 +6,7 @@ RSpec.describe 'Projects::MetricsDashboardController' do let_it_be(:project) { create(:project) } let_it_be(:environment) { create(:environment, project: project) } let_it_be(:environment2) { create(:environment, project: project) } - let_it_be(:user) { project.owner } + let_it_be(:user) { project.first_owner } before do project.add_developer(user) diff --git a/spec/requests/projects/noteable_notes_spec.rb b/spec/requests/projects/noteable_notes_spec.rb index 2bf1ffb2edc..44ee50ca002 100644 --- a/spec/requests/projects/noteable_notes_spec.rb +++ b/spec/requests/projects/noteable_notes_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'Project noteable notes' do let(:etag_store) { Gitlab::EtagCaching::Store.new } let(:notes_path) { project_noteable_notes_path(project, target_type: merge_request.class.name.underscore, target_id: merge_request.id) } let(:project) { merge_request.project } - let(:user) { project.owner } + let(:user) { project.first_owner } let(:response_etag) { response.headers['ETag'] } let(:stored_etag) { "W/\"#{etag_store.get(notes_path)}\"" } diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index 793438808a5..f2126e3cf9c 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_caching do include RackAttackSpecHelpers + include SessionHelpers let(:settings) { Gitlab::CurrentSettings.current_application_settings } @@ -63,6 +64,22 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac end end + describe 'API requests from the frontend', :api, :clean_gitlab_redis_sessions do + context 'when unauthenticated' do + it_behaves_like 'rate-limited frontend API requests' do + let(:throttle_setting_prefix) { 'throttle_unauthenticated' } + end + end + + context 'when authenticated' do + it_behaves_like 'rate-limited frontend API requests' do + let_it_be(:personal_access_token) { create(:personal_access_token) } + + let(:throttle_setting_prefix) { 'throttle_authenticated' } + end + end + end + describe 'API requests authenticated with personal access token', :api do let_it_be(:user) { create(:user) } let_it_be(:token) { create(:personal_access_token, user: user) } @@ -184,6 +201,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac context 'unauthenticated requests' do let(:protected_path_that_does_not_require_authentication) do + # This is one of the default values for `application_settings.protected_paths` '/users/sign_in' end @@ -227,6 +245,20 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac expect_rejection { post protected_path_that_does_not_require_authentication, params: post_params } end + it 'allows non-POST requests to protected paths over the rate limit' do + (1 + requests_per_period).times do + get protected_path_that_does_not_require_authentication + expect(response).to have_gitlab_http_status(:ok) + end + end + + it 'allows POST requests to unprotected paths over the rate limit' do + (1 + requests_per_period).times do + post '/api/graphql' + expect(response).to have_gitlab_http_status(:ok) + end + end + it_behaves_like 'tracking when dry-run mode is set' do let(:throttle_name) { 'throttle_unauthenticated_protected_paths' } end diff --git a/spec/requests/recursive_webhook_detection_spec.rb b/spec/requests/recursive_webhook_detection_spec.rb index a3014bf1d73..fe27c90b6c8 100644 --- a/spec/requests/recursive_webhook_detection_spec.rb +++ b/spec/requests/recursive_webhook_detection_spec.rb @@ -11,6 +11,11 @@ RSpec.describe 'Recursive webhook detection', :sidekiq_inline, :clean_gitlab_red let_it_be(:project_hook) { create(:project_hook, project: project, merge_requests_events: true) } let_it_be(:system_hook) { create(:system_hook, merge_requests_events: true) } + let(:stubbed_project_hook_hostname) { stubbed_hostname(project_hook.url, hostname: stubbed_project_hook_ip_address) } + let(:stubbed_system_hook_hostname) { stubbed_hostname(system_hook.url, hostname: stubbed_system_hook_ip_address) } + let(:stubbed_project_hook_ip_address) { '8.8.8.8' } + let(:stubbed_system_hook_ip_address) { '8.8.8.9' } + # Trigger a change to the merge request to fire the webhooks. def trigger_web_hooks params = { merge_request: { description: FFaker::Lorem.sentence } } @@ -18,8 +23,8 @@ RSpec.describe 'Recursive webhook detection', :sidekiq_inline, :clean_gitlab_red end def stub_requests - stub_full_request(project_hook.url, method: :post, ip_address: '8.8.8.8') - stub_full_request(system_hook.url, method: :post, ip_address: '8.8.8.9') + stub_full_request(project_hook.url, method: :post, ip_address: stubbed_project_hook_ip_address) + stub_full_request(system_hook.url, method: :post, ip_address: stubbed_system_hook_ip_address) end before do @@ -37,10 +42,10 @@ RSpec.describe 'Recursive webhook detection', :sidekiq_inline, :clean_gitlab_red trigger_web_hooks - expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + expect(WebMock).to have_requested(:post, stubbed_project_hook_hostname) .with { |req| req.headers['X-Gitlab-Event-Uuid'] == uuid } .once - expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)) + expect(WebMock).to have_requested(:post, stubbed_system_hook_hostname) .with { |req| req.headers['X-Gitlab-Event-Uuid'] == uuid } .once end @@ -54,24 +59,24 @@ RSpec.describe 'Recursive webhook detection', :sidekiq_inline, :clean_gitlab_red Gitlab::WebHooks::RecursionDetection.set_request_uuid(nil) end - it 'executes all webhooks and logs an error for the recursive hook', :aggregate_failures do + it 'blocks and logs an error for the recursive webhook, but execute the non-recursive webhook', :aggregate_failures do stub_requests expect(Gitlab::AuthLogger).to receive(:error).with( include( - message: 'Webhook recursion detected and will be blocked in future', + message: 'Recursive webhook blocked from executing', hook_id: project_hook.id, recursion_detection: { uuid: uuid, ids: [project_hook.id] } ) - ).twice # Twice: once in `#async_execute`, and again in `#execute`. + ).once trigger_web_hooks - expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)).once - expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)).once + expect(WebMock).not_to have_requested(:post, stubbed_project_hook_hostname) + expect(WebMock).to have_requested(:post, stubbed_system_hook_hostname).once end end @@ -87,35 +92,35 @@ RSpec.describe 'Recursive webhook detection', :sidekiq_inline, :clean_gitlab_red Gitlab::WebHooks::RecursionDetection.set_request_uuid(nil) end - it 'executes and logs errors for all hooks', :aggregate_failures do + it 'blocks and logs errors for all hooks', :aggregate_failures do stub_requests previous_hook_ids = previous_hooks.map(&:id) expect(Gitlab::AuthLogger).to receive(:error).with( include( - message: 'Webhook recursion detected and will be blocked in future', + message: 'Recursive webhook blocked from executing', hook_id: project_hook.id, recursion_detection: { uuid: uuid, ids: include(*previous_hook_ids) } ) - ).twice + ).once expect(Gitlab::AuthLogger).to receive(:error).with( include( - message: 'Webhook recursion detected and will be blocked in future', + message: 'Recursive webhook blocked from executing', hook_id: system_hook.id, recursion_detection: { uuid: uuid, ids: include(*previous_hook_ids) } ) - ).twice + ).once trigger_web_hooks - expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)).once - expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)).once + expect(WebMock).not_to have_requested(:post, stubbed_project_hook_hostname) + expect(WebMock).not_to have_requested(:post, stubbed_system_hook_hostname) end end end @@ -156,10 +161,10 @@ RSpec.describe 'Recursive webhook detection', :sidekiq_inline, :clean_gitlab_red expect(uuid_headers).to all(be_present) expect(uuid_headers.uniq.length).to eq(2) - expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + expect(WebMock).to have_requested(:post, stubbed_project_hook_hostname) .with { |req| uuid_headers.include?(req.headers['X-Gitlab-Event-Uuid']) } .once - expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)) + expect(WebMock).to have_requested(:post, stubbed_system_hook_hostname) .with { |req| uuid_headers.include?(req.headers['X-Gitlab-Event-Uuid']) } .once end @@ -175,8 +180,8 @@ RSpec.describe 'Recursive webhook detection', :sidekiq_inline, :clean_gitlab_red expect(uuid_headers).to all(be_present) expect(uuid_headers.length).to eq(4) expect(uuid_headers.uniq.length).to eq(4) - expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)).twice - expect(WebMock).to have_requested(:post, stubbed_hostname(system_hook.url)).twice + expect(WebMock).to have_requested(:post, stubbed_project_hook_hostname).twice + expect(WebMock).to have_requested(:post, stubbed_system_hook_hostname).twice end end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index dacc11eece7..d033ce15b00 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -506,6 +506,7 @@ RSpec.describe UsersController do describe 'GET #contributed' do let(:project) { create(:project, :public) } + let(:aimed_for_deletion_project) { create(:project, :public, :archived, marked_for_deletion_at: 3.days.ago) } subject do get user_contributed_projects_url author.username, format: format @@ -516,7 +517,10 @@ RSpec.describe UsersController do project.add_developer(public_user) project.add_developer(private_user) + aimed_for_deletion_project.add_developer(public_user) + aimed_for_deletion_project.add_developer(private_user) create(:push_event, project: project, author: author) + create(:push_event, project: aimed_for_deletion_project, author: author) subject end @@ -526,6 +530,11 @@ RSpec.describe UsersController do expect(response).to have_gitlab_http_status(:ok) expect(response.body).not_to be_empty end + + it 'does not list projects aimed for deletion' do + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:contributed_projects)).to eq([project]) + end end %i(html json).each do |format| @@ -557,6 +566,7 @@ RSpec.describe UsersController do describe 'GET #starred' do let(:project) { create(:project, :public) } + let(:aimed_for_deletion_project) { create(:project, :public, :archived, marked_for_deletion_at: 3.days.ago) } subject do get user_starred_projects_url author.username, format: format @@ -574,6 +584,11 @@ RSpec.describe UsersController do expect(response).to have_gitlab_http_status(:ok) expect(response.body).not_to be_empty end + + it 'does not list projects aimed for deletion' do + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:starred_projects)).to eq([project]) + end end %i(html json).each do |format| @@ -634,13 +649,13 @@ RSpec.describe UsersController do end describe 'GET #exists' do - before do - sign_in(user) + context 'when user exists' do + before do + sign_in(user) - allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(false) - end + allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(false) + end - context 'when user exists' do it 'returns JSON indicating the user exists' do get user_exists_url user.username @@ -661,6 +676,15 @@ RSpec.describe UsersController do end context 'when the user does not exist' do + it 'will not show a signup page if registration is disabled' do + stub_application_setting(signup_enabled: false) + get user_exists_url 'foo' + + expected_json = { error: "You must be authenticated to access this path." }.to_json + expect(response).to have_gitlab_http_status(:unauthorized) + expect(response.body).to eq(expected_json) + end + it 'returns JSON indicating the user does not exist' do get user_exists_url 'foo' |