diff options
Diffstat (limited to 'spec/requests/api')
102 files changed, 2061 insertions, 665 deletions
diff --git a/spec/requests/api/api_spec.rb b/spec/requests/api/api_spec.rb index b6cb790bb71..260f7cbc226 100644 --- a/spec/requests/api/api_spec.rb +++ b/spec/requests/api/api_spec.rb @@ -262,4 +262,54 @@ RSpec.describe API::API do end end end + + describe 'content security policy header' do + let_it_be(:user) { create(:user) } + + let(:csp) { nil } + let(:report_only) { false } + + subject { get api("/users/#{user.id}", user) } + + before do + allow(Rails.application.config).to receive(:content_security_policy).and_return(csp) + allow(Rails.application.config).to receive(:content_security_policy_report_only).and_return(report_only) + end + + context 'when CSP is not configured globally' do + it 'does not set the CSP header' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers['Content-Security-Policy']).to be_nil + end + end + + context 'when CSP is configured globally' do + let(:csp) do + ActionDispatch::ContentSecurityPolicy.new do |p| + p.default_src :self + end + end + + it 'sets a stricter CSP header' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers['Content-Security-Policy']).to eq("default-src 'none'") + end + + context 'when report_only is true' do + let(:report_only) { true } + + it 'does not set any CSP header' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers['Content-Security-Policy']).to be_nil + expect(response.headers['Content-Security-Policy-Report-Only']).to be_nil + end + end + end + end end diff --git a/spec/requests/api/boards_spec.rb b/spec/requests/api/boards_spec.rb index ca6492396cd..817e1324c7c 100644 --- a/spec/requests/api/boards_spec.rb +++ b/spec/requests/api/boards_spec.rb @@ -57,9 +57,11 @@ RSpec.describe API::Boards do let(:url) { "/projects/#{board_parent.id}/boards/#{board.id}" } it 'delete the issue board' do - delete api(url, user) + expect do + delete api(url, user) - expect(response).to have_gitlab_http_status(:no_content) + expect(response).to have_gitlab_http_status(:no_content) + end.to change { board_parent.boards.count }.by(-1) end end diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 780e45cf443..cc696d76a02 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe API::Branches do let_it_be(:user) { create(:user) } - let(:project) { create(:project, :repository, creator: user, path: 'my.project') } + let(:project) { create(:project, :repository, creator: user, path: 'my.project', create_branch: 'ends-with.txt') } let(:guest) { create(:user).tap { |u| project.add_guest(u) } } let(:branch_name) { 'feature' } let(:branch_sha) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } @@ -17,7 +17,6 @@ RSpec.describe API::Branches do before do project.add_maintainer(user) - project.repository.add_branch(user, 'ends-with.txt', branch_sha) stub_feature_flags(branch_list_keyset_pagination: false) end @@ -201,7 +200,7 @@ RSpec.describe API::Branches do context 'when sort value is not supported' do it_behaves_like '400 response' do - let(:request) { get api(route, user), params: { sort: 'unknown' }} + let(:request) { get api(route, user), params: { sort: 'unknown' } } end end end diff --git a/spec/requests/api/bulk_imports_spec.rb b/spec/requests/api/bulk_imports_spec.rb index 9f9907f4f00..6a3d13567bd 100644 --- a/spec/requests/api/bulk_imports_spec.rb +++ b/spec/requests/api/bulk_imports_spec.rb @@ -53,23 +53,80 @@ RSpec.describe API::BulkImports do end end - it 'starts a new migration' do - post api('/bulk_imports', user), params: { - configuration: { - url: 'http://gitlab.example', - access_token: 'access_token' - }, - entities: [ - source_type: 'group_entity', - source_full_path: 'full_path', - destination_name: 'destination_slug', - destination_namespace: 'destination_namespace' - ] - } - - expect(response).to have_gitlab_http_status(:created) - - expect(json_response['status']).to eq('created') + shared_examples 'starting a new migration' do + it 'starts a new migration' do + post api('/bulk_imports', user), params: { + configuration: { + url: 'http://gitlab.example', + access_token: 'access_token' + }, + entities: [ + { + source_type: 'group_entity', + source_full_path: 'full_path', + destination_namespace: 'destination_namespace' + }.merge(destination_param) + ] + } + + expect(response).to have_gitlab_http_status(:created) + + expect(json_response['status']).to eq('created') + end + end + + include_examples 'starting a new migration' do + let(:destination_param) { { destination_slug: 'destination_slug' } } + end + + include_examples 'starting a new migration' do + let(:destination_param) { { destination_name: 'destination_name' } } + end + + context 'when both destination_name & destination_slug are provided' do + it 'returns a mutually exclusive error' do + post api('/bulk_imports', user), params: { + configuration: { + url: 'http://gitlab.example', + access_token: 'access_token' + }, + entities: [ + { + source_type: 'group_entity', + source_full_path: 'full_path', + destination_name: 'destination_name', + destination_slug: 'destination_slug', + destination_namespace: 'destination_namespace' + } + ] + } + + expect(response).to have_gitlab_http_status(:bad_request) + + expect(json_response['error']).to eq('entities[0][destination_slug], entities[0][destination_name] are mutually exclusive') + end + end + + context 'when neither destination_name nor destination_slug is provided' do + it 'returns at_least_one_of error' do + post api('/bulk_imports', user), params: { + configuration: { + url: 'http://gitlab.example', + access_token: 'access_token' + }, + entities: [ + { + source_type: 'group_entity', + source_full_path: 'full_path', + destination_namespace: 'destination_namespace' + } + ] + } + + expect(response).to have_gitlab_http_status(:bad_request) + + expect(json_response['error']).to eq('entities[0][destination_slug], entities[0][destination_name] are missing, at least one parameter must be provided') + end end context 'when provided url is blocked' do @@ -82,7 +139,7 @@ RSpec.describe API::BulkImports do entities: [ source_type: 'group_entity', source_full_path: 'full_path', - destination_name: 'destination_slug', + destination_slug: 'destination_slug', destination_namespace: 'destination_namespace' ] } diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index 84ef9f8db1b..57828e50320 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -158,7 +158,7 @@ RSpec.describe API::Ci::Jobs do context 'with basic auth header' do let(:personal_access_token) { create(:personal_access_token, user: user) } - let(:token) { personal_access_token.token} + let(:token) { personal_access_token.token } include_context 'with auth headers' do let(:header) { { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => token } } diff --git a/spec/requests/api/ci/pipeline_schedules_spec.rb b/spec/requests/api/ci/pipeline_schedules_spec.rb index 5fb94976c5f..30badadde13 100644 --- a/spec/requests/api/ci/pipeline_schedules_spec.rb +++ b/spec/requests/api/ci/pipeline_schedules_spec.rb @@ -98,7 +98,7 @@ RSpec.describe API::Ci::PipelineSchedules do end matcher :return_pipeline_schedule_sucessfully do - match_unless_raises do |reponse| + match_unless_raises do |response| expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('pipeline_schedule') end @@ -207,6 +207,110 @@ RSpec.describe API::Ci::PipelineSchedules do end end + describe 'GET /projects/:id/pipeline_schedules/:pipeline_schedule_id/pipelines' do + let(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: developer) } + + before do + create_list(:ci_pipeline, 2, project: project, pipeline_schedule: pipeline_schedule, source: :schedule) + end + + let(:url) { "/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/pipelines" } + + matcher :return_pipeline_schedule_pipelines_successfully do + match_unless_raises do |reponse| + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(response).to match_response_schema('public_api/v4/pipelines') + end + end + + shared_examples 'request with project permissions' do + context 'authenticated user with project permissions' do + before do + project.add_maintainer(user) + end + + it 'returns the details of pipelines triggered from the pipeline schedule' do + get api(url, user) + + expect(response).to return_pipeline_schedule_pipelines_successfully + end + end + end + + shared_examples 'request with schedule ownership' do + context 'authenticated user with pipeline schedule ownership' do + it 'returns the details of pipelines triggered from the pipeline schedule' do + get api(url, developer) + + expect(response).to return_pipeline_schedule_pipelines_successfully + end + end + end + + shared_examples 'request with unauthenticated user' do + context 'with unauthenticated user' do + it 'does not return the details of pipelines triggered from the pipeline schedule' do + get api(url) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + shared_examples 'request with non-existing pipeline_schedule' do + it "responds with 404 Not Found if requesting for a non-existing pipeline schedule's pipelines" do + get api("/projects/#{project.id}/pipeline_schedules/#{non_existing_record_id}/pipelines", developer) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with private project' do + it_behaves_like 'request with schedule ownership' + it_behaves_like 'request with project permissions' + it_behaves_like 'request with unauthenticated user' + it_behaves_like 'request with non-existing pipeline_schedule' + + context 'authenticated user with no project permissions' do + it 'does not return the details of pipelines triggered from the pipeline schedule' do + get api(url, user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'authenticated user with insufficient project permissions' do + before do + project.add_guest(user) + end + + it 'does not return the details of pipelines triggered from the pipeline schedule' do + get api(url, user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'with public project' do + let_it_be(:project) { create(:project, :repository, :public, public_builds: false) } + + it_behaves_like 'request with schedule ownership' + it_behaves_like 'request with project permissions' + it_behaves_like 'request with unauthenticated user' + it_behaves_like 'request with non-existing pipeline_schedule' + + context 'authenticated user with no project permissions' do + it 'returns the details of pipelines triggered from the pipeline schedule' do + get api(url, user) + + expect(response).to return_pipeline_schedule_pipelines_successfully + end + end + end + end + describe 'POST /projects/:id/pipeline_schedules' do let(:params) { attributes_for(:ci_pipeline_schedule) } diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index 746be1ccc44..cd58251cfcc 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -29,7 +29,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do describe 'POST /api/v4/jobs/request' do let!(:last_update) {} - let!(:new_update) { } + let!(:new_update) {} let(:user_agent) { 'gitlab-runner 9.0.0 (9-0-stable; go1.7.4; linux/amd64)' } before do diff --git a/spec/requests/api/ci/runner/jobs_trace_spec.rb b/spec/requests/api/ci/runner/jobs_trace_spec.rb index c3c074d80d9..d42043a7fe5 100644 --- a/spec/requests/api/ci/runner/jobs_trace_spec.rb +++ b/spec/requests/api/ci/runner/jobs_trace_spec.rb @@ -61,7 +61,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_trace_chunks do end context 'when job has been updated recently' do - it { expect { patch_the_trace }.not_to change { job.updated_at }} + it { expect { patch_the_trace }.not_to change { job.updated_at } } it "changes the job's trace" do patch_the_trace @@ -70,7 +70,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_trace_chunks do end context 'when Runner makes a force-patch' do - it { expect { force_patch_the_trace }.not_to change { job.updated_at }} + it { expect { force_patch_the_trace }.not_to change { job.updated_at } } it "doesn't change the build.trace" do force_patch_the_trace diff --git a/spec/requests/api/ci/runner/runners_post_spec.rb b/spec/requests/api/ci/runner/runners_post_spec.rb index 50ace7adccb..47302046865 100644 --- a/spec/requests/api/ci/runner/runners_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_post_spec.rb @@ -16,7 +16,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do context 'when invalid token is provided' do it 'returns 403 error' do allow_next_instance_of(::Ci::Runners::RegisterRunnerService) do |service| - allow(service).to receive(:execute).and_return(nil) + allow(service).to receive(:execute) + .and_return(ServiceResponse.error(message: 'invalid token supplied', http_status: :forbidden)) end post api('/runners'), params: { token: 'invalid' } @@ -58,7 +59,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(service).to receive(:execute) .once .with('valid token', a_hash_including(expected_params)) - .and_return(new_runner) + .and_return(ServiceResponse.success(payload: { runner: new_runner })) end end @@ -113,7 +114,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do .once .with('valid token', a_hash_including('maintenance_note' => 'Some maintainer notes') .and(excluding('maintainter_note' => anything))) - .and_return(new_runner) + .and_return(ServiceResponse.success(payload: { runner: new_runner })) end request @@ -139,7 +140,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(service).to receive(:execute) .once .with('valid token', a_hash_including(expected_params)) - .and_return(new_runner) + .and_return(ServiceResponse.success(payload: { runner: new_runner })) end request diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb index 6f16fe5460b..f1f22dfadc2 100644 --- a/spec/requests/api/ci/secure_files_spec.rb +++ b/spec/requests/api/ci/secure_files_spec.rb @@ -59,7 +59,7 @@ RSpec.describe API::Ci::SecureFiles do expect do post api("/projects/#{project.id}/secure_files", maintainer), params: file_params - end.not_to change {project.secure_files.count} + end.not_to change { project.secure_files.count } expect(response).to have_gitlab_http_status(:service_unavailable) end @@ -78,7 +78,7 @@ RSpec.describe API::Ci::SecureFiles do it 'returns a 201 when uploading a file when the ci_secure_files_read_only feature flag is disabled' do expect do post api("/projects/#{project.id}/secure_files", maintainer), params: file_params - end.to change {project.secure_files.count}.by(1) + end.to change { project.secure_files.count }.by(1) expect(response).to have_gitlab_http_status(:created) end @@ -249,7 +249,7 @@ RSpec.describe API::Ci::SecureFiles do it 'creates a secure file' do expect do post api("/projects/#{project.id}/secure_files", maintainer), params: file_params - end.to change {project.secure_files.count}.by(1) + 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') diff --git a/spec/requests/api/ci/triggers_spec.rb b/spec/requests/api/ci/triggers_spec.rb index a036a55f5f3..953dcb8a483 100644 --- a/spec/requests/api/ci/triggers_spec.rb +++ b/spec/requests/api/ci/triggers_spec.rb @@ -136,8 +136,8 @@ RSpec.describe API::Ci::Triggers do end context 'when triggered from another running job' do - let!(:trigger) { } - let!(:trigger_request) { } + let!(:trigger) {} + let!(:trigger_request) {} context 'when other job is triggered by a user' do let(:trigger_token) { create(:ci_build, :running, project: project, user: user).token } @@ -242,7 +242,7 @@ RSpec.describe API::Ci::Triggers do expect do post api("/projects/#{project.id}/triggers", user), params: { description: 'trigger' } - end.to change {project.triggers.count}.by(1) + end.to change { project.triggers.count }.by(1) expect(response).to have_gitlab_http_status(:created) expect(json_response).to include('description' => 'trigger') @@ -335,7 +335,7 @@ RSpec.describe API::Ci::Triggers do delete api("/projects/#{project.id}/triggers/#{trigger.id}", user) expect(response).to have_gitlab_http_status(:no_content) - end.to change {project.triggers.count}.by(-1) + end.to change { project.triggers.count }.by(-1) end it 'responds with 404 Not Found if requesting non-existing trigger' do diff --git a/spec/requests/api/ci/variables_spec.rb b/spec/requests/api/ci/variables_spec.rb index dc524e121d5..74ed8c1551d 100644 --- a/spec/requests/api/ci/variables_spec.rb +++ b/spec/requests/api/ci/variables_spec.rb @@ -116,7 +116,7 @@ RSpec.describe API::Ci::Variables do it 'creates variable' do expect do post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'PROTECTED_VALUE_2', protected: true, masked: true } - end.to change {project.variables.count}.by(1) + end.to change { project.variables.count }.by(1) expect(response).to have_gitlab_http_status(:created) expect(json_response['key']).to eq('TEST_VARIABLE_2') @@ -129,7 +129,7 @@ RSpec.describe API::Ci::Variables do it 'creates variable with optional attributes' do expect do post api("/projects/#{project.id}/variables", user), params: { variable_type: 'file', key: 'TEST_VARIABLE_2', value: 'VALUE_2' } - end.to change {project.variables.count}.by(1) + end.to change { project.variables.count }.by(1) expect(response).to have_gitlab_http_status(:created) expect(json_response['key']).to eq('TEST_VARIABLE_2') @@ -142,7 +142,7 @@ RSpec.describe API::Ci::Variables do it 'does not allow to duplicate variable key' do expect do post api("/projects/#{project.id}/variables", user), params: { key: variable.key, value: 'VALUE_2' } - end.to change {project.variables.count}.by(0) + end.to change { project.variables.count }.by(0) expect(response).to have_gitlab_http_status(:bad_request) end @@ -268,7 +268,7 @@ RSpec.describe API::Ci::Variables do delete api("/projects/#{project.id}/variables/#{variable.key}", user) expect(response).to have_gitlab_http_status(:no_content) - end.to change {project.variables.count}.by(-1) + end.to change { project.variables.count }.by(-1) end it 'responds with 404 Not Found if requesting non-existing variable' do @@ -295,7 +295,7 @@ RSpec.describe API::Ci::Variables do delete api("/projects/#{project.id}/variables/key1", user), params: { 'filter[environment_scope]': 'production' } expect(response).to have_gitlab_http_status(:no_content) - end.to change {project.variables.count}.by(-1) + end.to change { project.variables.count }.by(-1) expect(var1.reload).to be_present expect { var2.reload }.to raise_error(ActiveRecord::RecordNotFound) diff --git a/spec/requests/api/clusters/agents_spec.rb b/spec/requests/api/clusters/agents_spec.rb index 72d4266b9e3..5e3bdd69529 100644 --- a/spec/requests/api/clusters/agents_spec.rb +++ b/spec/requests/api/clusters/agents_spec.rb @@ -101,7 +101,7 @@ RSpec.describe API::Clusters::Agents do expect do post(api("/projects/#{project.id}/cluster_agents", user), params: { name: 'some-agent' }) - end.to change {project.cluster_agents.count}.by(1) + end.to change { project.cluster_agents.count }.by(1) aggregate_failures "testing response" do expect(response).to have_gitlab_http_status(:created) @@ -139,7 +139,7 @@ RSpec.describe API::Clusters::Agents do delete api("/projects/#{project.id}/cluster_agents/#{agent.id}", user) expect(response).to have_gitlab_http_status(:no_content) - end.to change {project.cluster_agents.count}.by(-1) + end.to change { project.cluster_agents.count }.by(-1) end it 'returns a 404 error when deleting non existent agent' do diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 9ef845f06bf..68fe45cd026 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -206,11 +206,13 @@ RSpec.describe API::Commits do let(:page) { 1 } let(:per_page) { 5 } let(:ref_name) { 'master' } - let!(:request) do + let(:request) do get api("/projects/#{project_id}/repository/commits?page=#{page}&per_page=#{per_page}&ref_name=#{ref_name}", user) end it 'returns correct headers' do + request + expect(response).to include_limited_pagination_headers expect(response.headers['Link']).to match(/page=1&per_page=5/) expect(response.headers['Link']).to match(/page=2&per_page=5/) @@ -218,6 +220,8 @@ RSpec.describe API::Commits do context 'viewing the first page' do it 'returns the first 5 commits' do + request + commit = project.repository.commit expect(json_response.size).to eq(per_page) @@ -230,6 +234,8 @@ RSpec.describe API::Commits do let(:page) { 3 } it 'returns the third 5 commits' do + request + commit = project.repository.commits('HEAD', limit: per_page, offset: (page - 1) * per_page).first expect(json_response.size).to eq(per_page) @@ -238,10 +244,55 @@ RSpec.describe API::Commits do end end - context 'when per_page is 0' do - let(:per_page) { 0 } + context 'when pagination params are invalid' do + let_it_be(:project) { create(:project, :repository) } + + using RSpec::Parameterized::TableSyntax + + where(:page, :per_page, :error_message) do + 0 | nil | 'page does not have a valid value' + -1 | nil | 'page does not have a valid value' + 'a' | nil | 'page is invalid' + nil | 0 | 'per_page does not have a valid value' + nil | -1 | 'per_page does not have a valid value' + nil | 'a' | 'per_page is invalid' + end + + with_them do + it 'returns 400 response' do + request + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq(error_message) + end + end - it_behaves_like '400 response' + context 'when FF is off' do + before do + stub_feature_flags(only_positive_pagination_values: false) + end + + where(:page, :per_page, :error_message, :status) do + 0 | nil | nil | :success + -10 | nil | nil | :internal_server_error + 'a' | nil | 'page is invalid' | :bad_request + nil | 0 | 'per_page has a value not allowed' | :bad_request + nil | -1 | nil | :success + nil | 'a' | 'per_page is invalid' | :bad_request + end + + with_them do + it 'returns a response' do + request + + expect(response).to have_gitlab_http_status(status) + + if error_message + expect(json_response['error']).to eq(error_message) + end + end + end + end end end @@ -928,6 +979,40 @@ RSpec.describe API::Commits do end end + context 'when action is missing' do + let(:params) do + { + branch: 'master', + commit_message: 'Invalid', + actions: [{ action: nil, file_path: 'files/ruby/popen.rb' }] + } + end + + it 'responds with 400 bad request' do + post api(url, user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('actions[0][action] is empty') + end + end + + context 'when action is not supported' do + let(:params) do + { + branch: 'master', + commit_message: 'Invalid', + actions: [{ action: 'unknown', file_path: 'files/ruby/popen.rb' }] + } + end + + it 'responds with 400 bad request' do + post api(url, user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('actions[0][action] does not have a valid value') + end + end + context 'when committing into a fork as a maintainer' do include_context 'merge request allowing collaboration' @@ -988,8 +1073,8 @@ RSpec.describe API::Commits do it 'returns all refs with no scope' do get api(route, current_user), params: { per_page: 100 } - refs = project.repository.branch_names_contains(commit_id).map {|name| ['branch', name]} - refs.concat(project.repository.tag_names_contains(commit_id).map {|name| ['tag', name]}) + refs = project.repository.branch_names_contains(commit_id).map { |name| ['branch', name] } + refs.concat(project.repository.tag_names_contains(commit_id).map { |name| ['tag', name] }) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_limited_pagination_headers @@ -1000,8 +1085,8 @@ RSpec.describe API::Commits do it 'returns all refs' do get api(route, current_user), params: { type: 'all', per_page: 100 } - refs = project.repository.branch_names_contains(commit_id).map {|name| ['branch', name]} - refs.concat(project.repository.tag_names_contains(commit_id).map {|name| ['tag', name]}) + refs = project.repository.branch_names_contains(commit_id).map { |name| ['branch', name] } + refs.concat(project.repository.tag_names_contains(commit_id).map { |name| ['tag', name] }) expect(response).to have_gitlab_http_status(:ok) expect(json_response.map { |r| [r['type'], r['name']] }.compact).to eq(refs) @@ -1010,7 +1095,7 @@ RSpec.describe API::Commits do it 'returns the branch refs' do get api(route, current_user), params: { type: 'branch', per_page: 100 } - refs = project.repository.branch_names_contains(commit_id).map {|name| ['branch', name]} + refs = project.repository.branch_names_contains(commit_id).map { |name| ['branch', name] } expect(response).to have_gitlab_http_status(:ok) expect(json_response.map { |r| [r['type'], r['name']] }.compact).to eq(refs) @@ -1019,7 +1104,7 @@ RSpec.describe API::Commits do it 'returns the tag refs' do get api(route, current_user), params: { type: 'tag', per_page: 100 } - refs = project.repository.tag_names_contains(commit_id).map {|name| ['tag', name]} + refs = project.repository.tag_names_contains(commit_id).map { |name| ['tag', name] } expect(response).to have_gitlab_http_status(:ok) expect(json_response.map { |r| [r['type'], r['name']] }.compact).to eq(refs) @@ -2036,7 +2121,7 @@ RSpec.describe API::Commits do context 'unsigned commit' do it_behaves_like '404 response' do let(:request) { get api(route, current_user) } - let(:message) { '404 Signature Not Found'} + let(:message) { '404 Signature Not Found' } end end diff --git a/spec/requests/api/conan_instance_packages_spec.rb b/spec/requests/api/conan_instance_packages_spec.rb index 54cad3093d7..e4747e0eb99 100644 --- a/spec/requests/api/conan_instance_packages_spec.rb +++ b/spec/requests/api/conan_instance_packages_spec.rb @@ -94,7 +94,7 @@ RSpec.describe API::ConanInstancePackages do end describe 'DELETE /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel' do - subject { delete api("/packages/conan/v1/conans/#{recipe_path}"), headers: headers} + subject { delete api("/packages/conan/v1/conans/#{recipe_path}"), headers: headers } it_behaves_like 'delete package endpoint' end diff --git a/spec/requests/api/conan_project_packages_spec.rb b/spec/requests/api/conan_project_packages_spec.rb index e28105eb8eb..48e36b55a68 100644 --- a/spec/requests/api/conan_project_packages_spec.rb +++ b/spec/requests/api/conan_project_packages_spec.rb @@ -93,7 +93,7 @@ RSpec.describe API::ConanProjectPackages do end describe 'DELETE /api/v4/projects/:id/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel' do - subject { delete api("/projects/#{project_id}/packages/conan/v1/conans/#{recipe_path}"), headers: headers} + subject { delete api("/projects/#{project_id}/packages/conan/v1/conans/#{recipe_path}"), headers: headers } it_behaves_like 'delete package endpoint' end diff --git a/spec/requests/api/dependency_proxy_spec.rb b/spec/requests/api/dependency_proxy_spec.rb index 067852ef1e9..a8617fcb0bf 100644 --- a/spec/requests/api/dependency_proxy_spec.rb +++ b/spec/requests/api/dependency_proxy_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe API::DependencyProxy, api: true do let_it_be(:user) { create(:user) } - let_it_be(:blob) { create(:dependency_proxy_blob )} + let_it_be(:blob) { create(:dependency_proxy_blob ) } let_it_be(:group, reload: true) { blob.group } before do diff --git a/spec/requests/api/deployments_spec.rb b/spec/requests/api/deployments_spec.rb index 69f7b54c277..24c3ee59c18 100644 --- a/spec/requests/api/deployments_spec.rb +++ b/spec/requests/api/deployments_spec.rb @@ -448,6 +448,90 @@ RSpec.describe API::Deployments do end end + describe 'DELETE /projects/:id/deployments/:deployment_id' do + let(:project) { create(:project, :repository) } + let(:environment) { create(:environment, project: project) } + let(:commits) { project.repository.commits(nil, { limit: 3 }) } + let!(:deploy) do + create( + :deployment, + :success, + project: project, + environment: environment, + deployable: nil, + sha: commits[1].sha + ) + end + + let!(:old_deploy) do + create( + :deployment, + :success, + project: project, + environment: environment, + deployable: nil, + sha: commits[0].sha, + finished_at: 1.year.ago + ) + end + + let!(:running_deploy) do + create( + :deployment, + :running, + project: project, + environment: environment, + deployable: nil, + sha: commits[2].sha + ) + end + + context 'as an maintainer' do + it 'deletes a deployment' do + delete api("/projects/#{project.id}/deployments/#{old_deploy.id}", user) + + expect(response).to have_gitlab_http_status(:no_content) + end + + it 'will not delete a running deployment' do + delete api("/projects/#{project.id}/deployments/#{running_deploy.id}", user) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to include("Cannot destroy running deployment") + end + end + + context 'as a developer' do + let(:developer) { create(:user) } + + before do + project.add_developer(developer) + end + + it 'is forbidden' do + delete api("/projects/#{project.id}/deployments/#{deploy.id}", developer) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'as non member' do + it 'is not found' do + delete api("/projects/#{project.id}/deployments/#{deploy.id}", non_member) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'for non-existent deployment' do + it 'is not found' do + delete api("/projects/#{project.id}/deployments/#{non_existing_record_id}", project.first_owner) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + describe 'GET /projects/:id/deployments/:deployment_id/merge_requests' do let(:project) { create(:project, :repository) } let!(:deployment) { create(:deployment, :success, project: project) } diff --git a/spec/requests/api/doorkeeper_access_spec.rb b/spec/requests/api/doorkeeper_access_spec.rb index 77f1dadff46..14da9a600cd 100644 --- a/spec/requests/api/doorkeeper_access_spec.rb +++ b/spec/requests/api/doorkeeper_access_spec.rb @@ -9,13 +9,13 @@ RSpec.describe 'doorkeeper access' do describe "unauthenticated" do it "returns authentication success" do - get api("/user"), params: { access_token: token.token } + get api("/user"), params: { access_token: token.plaintext_token } expect(response).to have_gitlab_http_status(:ok) end include_examples 'user login request with unique ip limit' do def request - get api('/user'), params: { access_token: token.token } + get api('/user'), params: { access_token: token.plaintext_token } end end end @@ -42,7 +42,7 @@ RSpec.describe 'doorkeeper access' do shared_examples 'forbidden request' do it 'returns 403 response' do - get api("/user"), params: { access_token: token.token } + get api("/user"), params: { access_token: token.plaintext_token } expect(response).to have_gitlab_http_status(:forbidden) end diff --git a/spec/requests/api/go_proxy_spec.rb b/spec/requests/api/go_proxy_spec.rb index 0143340de11..7c44fddc303 100644 --- a/spec/requests/api/go_proxy_spec.rb +++ b/spec/requests/api/go_proxy_spec.rb @@ -318,7 +318,7 @@ RSpec.describe API::GoProxy do context 'with a case sensitive project and versions' do let_it_be(:project) { create :project_empty_repo, :public, creator: user, path: 'MyGoLib' } let_it_be(:base) { "#{Settings.build_gitlab_go_url}/#{project.full_path}" } - let_it_be(:base_encoded) { base.gsub(/[A-Z]/) { |s| "!#{s.downcase}"} } + let_it_be(:base_encoded) { base.gsub(/[A-Z]/) { |s| "!#{s.downcase}" } } let_it_be(:modules) do create(:go_module_commit, :files, project: project, files: { 'README.md' => "Hi" }) @@ -376,7 +376,7 @@ RSpec.describe API::GoProxy do end it 'returns ok with a job token' do - get_resource(oauth_access_token: job) + get_resource(access_token: job) expect(response).to have_gitlab_http_status(:ok) end @@ -395,7 +395,7 @@ RSpec.describe API::GoProxy do it 'returns unauthorized with a failed job token' do job.update!(status: :failed) - get_resource(oauth_access_token: job) + get_resource(access_token: job) expect(response).to have_gitlab_http_status(:unauthorized) end @@ -445,7 +445,7 @@ RSpec.describe API::GoProxy do end it 'returns not found with a job token' do - get_resource(oauth_access_token: job) + get_resource(access_token: job) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb b/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb index 6324db0be4a..484ddc3469b 100644 --- a/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb +++ b/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'get board lists' do let_it_be(:group_label2) { create(:group_label, group: group, name: 'Testing') } let(:params) { '' } - let(:board) { } + let(:board) {} let(:confidential) { false } let(:board_parent_type) { board_parent.class.to_s.downcase } let(:board_data) { graphql_data[board_parent_type]['boards']['nodes'][0] } diff --git a/spec/requests/api/graphql/boards/board_lists_query_spec.rb b/spec/requests/api/graphql/boards/board_lists_query_spec.rb index 39ff108a9e1..6fe2e41cf35 100644 --- a/spec/requests/api/graphql/boards/board_lists_query_spec.rb +++ b/spec/requests/api/graphql/boards/board_lists_query_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'get board lists' do let_it_be(:group_label2) { create(:group_label, group: group, name: 'Testing') } let(:params) { '' } - let(:board) { } + let(:board) {} let(:board_parent_type) { board_parent.class.to_s.downcase } let(:board_data) { graphql_data[board_parent_type]['boards']['edges'].first['node'] } let(:lists_data) { board_data['lists']['edges'] } diff --git a/spec/requests/api/graphql/ci/instance_variables_spec.rb b/spec/requests/api/graphql/ci/instance_variables_spec.rb index 7acf73a4e7a..c5c88697bf4 100644 --- a/spec/requests/api/graphql/ci/instance_variables_spec.rb +++ b/spec/requests/api/graphql/ci/instance_variables_spec.rb @@ -57,4 +57,16 @@ RSpec.describe 'Query.ciVariables' do expect(graphql_data.dig('ciVariables')).to be_nil end end + + context 'when the user is unauthenticated' do + let_it_be(:user) { nil } + + it 'returns nothing' do + create(:ci_instance_variable, value: 'verysecret', masked: true) + + post_graphql(query, current_user: user) + + expect(graphql_data.dig('ciVariables')).to be_nil + end + end end diff --git a/spec/requests/api/graphql/ci/manual_variables_spec.rb b/spec/requests/api/graphql/ci/manual_variables_spec.rb index b7aa76511a3..a15bac2b8bd 100644 --- a/spec/requests/api/graphql/ci/manual_variables_spec.rb +++ b/spec/requests/api/graphql/ci/manual_variables_spec.rb @@ -35,8 +35,8 @@ RSpec.describe 'Query.project(fullPath).pipelines.jobs.manualVariables' do project.add_maintainer(user) end - it 'returns the manual variables for the jobs' do - job = create(:ci_build, :manual, pipeline: pipeline) + it 'returns the manual variables for actionable jobs' do + job = create(:ci_build, :actionable, pipeline: pipeline) create(:ci_job_variable, key: 'MANUAL_TEST_VAR', job: job) post_graphql(query, current_user: user) @@ -46,8 +46,8 @@ RSpec.describe 'Query.project(fullPath).pipelines.jobs.manualVariables' do expect(variables_data.map { |var| var['key'] }).to match_array(['MANUAL_TEST_VAR']) end - it 'does not fetch job variables for jobs that are not manual' do - job = create(:ci_build, pipeline: pipeline) + it 'does not fetch job variables for jobs that are not actionable' do + job = create(:ci_build, pipeline: pipeline, status: :manual) create(:ci_job_variable, key: 'THIS_VAR_WOULD_SHOULD_NEVER_EXIST', job: job) post_graphql(query, current_user: user) diff --git a/spec/requests/api/graphql/ci/pipelines_spec.rb b/spec/requests/api/graphql/ci/pipelines_spec.rb index a968e5508cb..f471a152603 100644 --- a/spec/requests/api/graphql/ci/pipelines_spec.rb +++ b/spec/requests/api/graphql/ci/pipelines_spec.rb @@ -166,6 +166,35 @@ RSpec.describe 'Query.project(fullPath).pipelines' do end end + describe '.job' do + let(:first_n) { var('Int') } + let(:query_path) do + [ + [:project, { full_path: project.full_path }], + [:pipelines], + [:nodes], + [:job, { name: 'Job 1' }] + ] + end + + let(:query) do + wrap_fields(query_graphql_path(query_path, :status)) + end + + before_all do + pipeline = create(:ci_pipeline, project: project) + create(:ci_build, pipeline: pipeline, name: 'Job 1', status: :failed, retried: true) + create(:ci_build, pipeline: pipeline, name: 'Job 1', status: :success) + end + + it 'fetches the latest job with the given name' do + post_graphql(query, current_user: user) + expect(graphql_data_at(*query_path.map(&:first))).to contain_exactly a_hash_including( + 'status' => 'SUCCESS' + ) + end + end + describe '.jobs' do let(:first_n) { var('Int') } let(:query_path) do diff --git a/spec/requests/api/graphql/ci/runners_spec.rb b/spec/requests/api/graphql/ci/runners_spec.rb index a5b8115286e..749f6839cb5 100644 --- a/spec/requests/api/graphql/ci/runners_spec.rb +++ b/spec/requests/api/graphql/ci/runners_spec.rb @@ -37,7 +37,9 @@ RSpec.describe 'Query.runners' do end before do - allow(Gitlab::Ci::RunnerUpgradeCheck.instance).to receive(:check_runner_upgrade_status) + allow_next_instance_of(::Gitlab::Ci::RunnerUpgradeCheck) do |instance| + allow(instance).to receive(:check_runner_upgrade_suggestion) + end post_graphql(query, current_user: current_user) end diff --git a/spec/requests/api/graphql/crm/contacts_spec.rb b/spec/requests/api/graphql/crm/contacts_spec.rb index 7e824140894..a676e92dc3b 100644 --- a/spec/requests/api/graphql/crm/contacts_spec.rb +++ b/spec/requests/api/graphql/crm/contacts_spec.rb @@ -12,11 +12,11 @@ RSpec.describe 'getting CRM contacts' do create( :contact, group: group, - first_name: "ABC", - last_name: "DEF", - email: "ghi@test.com", - description: "LMNO", - state: "inactive" + first_name: "PQR", + last_name: "STU", + email: "aaa@test.com", + description: "YZ", + state: "active" ) end @@ -26,9 +26,9 @@ RSpec.describe 'getting CRM contacts' do group: group, first_name: "ABC", last_name: "DEF", - email: "vwx@test.com", - description: "YZ", - state: "active" + email: "ghi@test.com", + description: "LMNO", + state: "inactive" ) end @@ -36,9 +36,9 @@ RSpec.describe 'getting CRM contacts' do create( :contact, group: group, - first_name: "PQR", - last_name: "STU", - email: "aaa@test.com", + first_name: "JKL", + last_name: "MNO", + email: "vwx@test.com", description: "YZ", state: "active" ) @@ -51,7 +51,7 @@ RSpec.describe 'getting CRM contacts' do it_behaves_like 'sorted paginated query' do let(:sort_argument) { {} } let(:first_param) { 2 } - let(:all_records) { [contact_a, contact_b, contact_c] } + let(:all_records) { [contact_b, contact_c, contact_a] } let(:data_path) { [:group, :contacts] } def pagination_query(params) diff --git a/spec/requests/api/graphql/current_user/groups_query_spec.rb b/spec/requests/api/graphql/current_user/groups_query_spec.rb index ef0f32bacf0..6e36beb2afc 100644 --- a/spec/requests/api/graphql/current_user/groups_query_spec.rb +++ b/spec/requests/api/graphql/current_user/groups_query_spec.rb @@ -6,10 +6,11 @@ RSpec.describe 'Query current user groups' do include GraphqlHelpers let_it_be(:user) { create(:user) } + let_it_be(:root_group) { create(:group, name: 'Root group', path: 'root-group') } let_it_be(:guest_group) { create(:group, name: 'public guest', path: 'public-guest') } - let_it_be(:private_maintainer_group) { create(:group, :private, name: 'b private maintainer', path: 'b-private-maintainer') } + let_it_be(:private_maintainer_group) { create(:group, :private, name: 'b private maintainer', path: 'b-private-maintainer', parent: root_group) } let_it_be(:public_developer_group) { create(:group, project_creation_level: nil, name: 'c public developer', path: 'c-public-developer') } - let_it_be(:public_maintainer_group) { create(:group, name: 'a public maintainer', path: 'a-public-maintainer') } + let_it_be(:public_maintainer_group) { create(:group, name: 'a public maintainer', path: 'a-public-maintainer', parent: root_group) } let_it_be(:public_owner_group) { create(:group, name: 'a public owner', path: 'a-public-owner') } let(:group_arguments) { {} } @@ -77,7 +78,7 @@ RSpec.describe 'Query current user groups' do end context 'when search is provided' do - let(:group_arguments) { { permission_scope: :CREATE_PROJECTS, search: 'maintainer' } } + let(:group_arguments) { { permission_scope: :CREATE_PROJECTS, search: 'root-group maintainer' } } specify do is_expected.to match( @@ -127,6 +128,18 @@ RSpec.describe 'Query current user groups' do ) ) end + + context 'when searching for a full path (including parent)' do + let(:group_arguments) { { search: 'root-group/b-private-maintainer' } } + + specify do + is_expected.to match( + expected_group_hash( + private_maintainer_group + ) + ) + end + end end def expected_group_hash(*groups) diff --git a/spec/requests/api/graphql/custom_emoji_query_spec.rb b/spec/requests/api/graphql/custom_emoji_query_spec.rb index 874357d9eef..13b7a22e791 100644 --- a/spec/requests/api/graphql/custom_emoji_query_spec.rb +++ b/spec/requests/api/graphql/custom_emoji_query_spec.rb @@ -31,8 +31,8 @@ RSpec.describe 'getting custom emoji within namespace' do post_graphql(custom_emoji_query(group), current_user: current_user) expect(response).to have_gitlab_http_status(:ok) - expect(graphql_data['group']['customEmoji']['nodes'].count). to eq(1) - expect(graphql_data['group']['customEmoji']['nodes'].first['name']). to eq(custom_emoji.name) + expect(graphql_data['group']['customEmoji']['nodes'].count).to eq(1) + expect(graphql_data['group']['customEmoji']['nodes'].first['name']).to eq(custom_emoji.name) end it 'returns nil when unauthorised' do diff --git a/spec/requests/api/graphql/group/group_members_spec.rb b/spec/requests/api/graphql/group/group_members_spec.rb index 1ff5b134e92..bab8d5b770c 100644 --- a/spec/requests/api/graphql/group/group_members_spec.rb +++ b/spec/requests/api/graphql/group/group_members_spec.rb @@ -64,24 +64,6 @@ RSpec.describe 'getting group members information' do expect_array_response(user_2) end - - context 'when the use_keyset_aware_user_search_query FF is off' do - before do - stub_feature_flags(use_keyset_aware_user_search_query: false) - end - - it 'raises error on the 2nd page due to missing cursor data' do - fetch_members(args: { search: 'Same Name', first: 1 }) - - # user_2 because the "old" order was undeterministic (insert order), no tie-breaker column - expect_array_response(user_2) - - next_cursor = graphql_data_at(:group, :groupMembers, :pageInfo, :endCursor) - fetch_members(args: { search: 'Same Name', first: 1, after: next_cursor }) - - expect(graphql_errors.first['message']).to include('PG::UndefinedColumn') - end - end end end end diff --git a/spec/requests/api/graphql/group_query_spec.rb b/spec/requests/api/graphql/group_query_spec.rb index fd0ee5d52b9..8ee5c3c5d73 100644 --- a/spec/requests/api/graphql/group_query_spec.rb +++ b/spec/requests/api/graphql/group_query_spec.rb @@ -122,6 +122,87 @@ RSpec.describe 'getting group information' do end end + context 'with timelog categories' do + let_it_be(:group) { create(:group) } + let_it_be(:timelog_category) { create(:timelog_category, namespace: group, name: 'TimelogCategoryTest') } + + context 'when user is guest' do + it 'includes empty timelog categories array' do + post_graphql(group_query(group), current_user: user2) + + expect(graphql_data_at(:group, :timelogCategories, :nodes)).to match([]) + end + end + + context 'when user has reporter role' do + before do + group.add_reporter(user2) + end + + it 'returns the timelog category with all its fields' do + post_graphql(group_query(group), current_user: user2) + + expect(graphql_data_at(:group, :timelogCategories, :nodes)) + .to contain_exactly(a_graphql_entity_for(timelog_category)) + end + + context 'when timelog_categories flag is disabled' do + before do + stub_feature_flags(timelog_categories: false) + end + + it 'returns no timelog categories' do + post_graphql(group_query(group), current_user: user2) + + expect(graphql_data_at(:group, :timelogCategories)).to be_nil + end + end + end + + context 'for N+1 queries' do + let!(:group1) { create(:group) } + let!(:group2) { create(:group) } + + before do + group1.add_reporter(user2) + group2.add_reporter(user2) + end + + it 'avoids N+1 database queries' do + pending('See: https://gitlab.com/gitlab-org/gitlab/-/issues/369396') + + ctx = { current_user: user2 } + + baseline_query = <<~GQL + query { + a: group(fullPath: "#{group1.full_path}") { ... g } + } + + fragment g on Group { + timelogCategories { nodes { name } } + } + GQL + + query = <<~GQL + query { + a: group(fullPath: "#{group1.full_path}") { ... g } + b: group(fullPath: "#{group2.full_path}") { ... g } + } + + fragment g on Group { + timelogCategories { nodes { name } } + } + GQL + + control = ActiveRecord::QueryRecorder.new do + run_with_clean_state(baseline_query, context: ctx) + end + + expect { run_with_clean_state(query, context: ctx) }.not_to exceed_query_limit(control) + end + end + end + context "when authenticated as admin" do it "returns any existing group" do post_graphql(group_query(private_group), current_user: admin) diff --git a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb index fdf5503a3a2..3879e58cecf 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb @@ -74,7 +74,7 @@ RSpec.describe 'Adding an AwardEmoji' do end describe 'marking Todos as done' do - let(:user) { current_user} + let(:user) { current_user } subject { post_graphql_mutation(mutation, current_user: user) } diff --git a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb index 6b26e37e30c..7ddffa1ab0a 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb @@ -84,7 +84,7 @@ RSpec.describe 'Toggling an AwardEmoji' do end describe 'marking Todos as done' do - let(:user) { current_user} + let(:user) { current_user } subject { post_graphql_mutation(mutation, current_user: user) } diff --git a/spec/requests/api/graphql/mutations/boards/destroy_spec.rb b/spec/requests/api/graphql/mutations/boards/destroy_spec.rb index 23e099e94b6..7620da3e7e0 100644 --- a/spec/requests/api/graphql/mutations/boards/destroy_spec.rb +++ b/spec/requests/api/graphql/mutations/boards/destroy_spec.rb @@ -65,15 +65,8 @@ RSpec.describe Mutations::Boards::Destroy do other_board.destroy! end - it 'does not destroy the board' do - expect { subject }.not_to change { Board.count }.from(1) - end - - it 'returns an error and not nil board' do - subject - - expect(mutation_response['errors']).not_to be_empty - expect(mutation_response['board']).not_to be_nil + it 'does destroy the board' do + expect { subject }.to change { Board.count }.by(-1) end end end diff --git a/spec/requests/api/graphql/mutations/ci/job_retry_spec.rb b/spec/requests/api/graphql/mutations/ci/job_retry_spec.rb index ef640183bd8..8cf559a372a 100644 --- a/spec/requests/api/graphql/mutations/ci/job_retry_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/job_retry_spec.rb @@ -47,6 +47,38 @@ RSpec.describe 'JobRetry' do expect(new_job).not_to be_retried end + context 'when given CI variables' do + let(:job) { create(:ci_build, :success, :actionable, pipeline: pipeline, name: 'build') } + + let(:mutation) do + variables = { + id: job.to_global_id.to_s, + variables: { key: 'MANUAL_VAR', value: 'test manual var' } + } + + graphql_mutation(:job_retry, variables, + <<-QL + errors + job { + id + } + QL + ) + end + + it 'applies them to a retried manual job' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + + new_job_id = GitlabSchema.object_from_id(mutation_response['job']['id']).sync.id + new_job = ::Ci::Build.find(new_job_id) + expect(new_job.job_variables.count).to be(1) + expect(new_job.job_variables.first.key).to eq('MANUAL_VAR') + expect(new_job.job_variables.first.value).to eq('test manual var') + end + end + context 'when the job is not retryable' do let(:job) { create(:ci_build, :retried, pipeline: pipeline) } diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb index d9106aa42c4..6ec1b7ce9b6 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb @@ -40,7 +40,7 @@ RSpec.describe 'PipelineCancel' do expect(build).not_to be_canceled end - it "cancels all cancelable builds from a pipeline" do + it 'cancels all cancelable builds from a pipeline', :sidekiq_inline do build = create(:ci_build, :running, pipeline: pipeline) post_graphql_mutation(mutation, current_user: user) diff --git a/spec/requests/api/graphql/mutations/merge_requests/request_attention_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/request_attention_spec.rb deleted file mode 100644 index 9c751913827..00000000000 --- a/spec/requests/api/graphql/mutations/merge_requests/request_attention_spec.rb +++ /dev/null @@ -1,79 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Request attention' do - include GraphqlHelpers - - let_it_be(:current_user) { create(:user) } - let_it_be(:user) { create(:user) } - let_it_be(:merge_request) { create(:merge_request, reviewers: [user]) } - let_it_be(:project) { merge_request.project } - - let(:input) { { user_id: global_id_of(user) } } - - let(:mutation) do - variables = { - project_path: project.full_path, - iid: merge_request.iid.to_s - } - graphql_mutation(:merge_request_request_attention, variables.merge(input), - <<-QL.strip_heredoc - clientMutationId - errors - QL - ) - end - - def mutation_response - graphql_mutation_response(:merge_request_request_attention) - end - - def mutation_errors - mutation_response['errors'] - end - - before_all do - project.add_developer(current_user) - project.add_developer(user) - end - - it 'is successful' do - post_graphql_mutation(mutation, current_user: current_user) - - expect(response).to have_gitlab_http_status(:success) - expect(mutation_errors).to be_empty - end - - context 'when current user is not allowed to update the merge request' do - it 'returns an error' do - post_graphql_mutation(mutation, current_user: create(:user)) - - expect(graphql_errors).not_to be_empty - end - end - - context 'when user is not a reviewer' do - let(:input) { { user_id: global_id_of(create(:user)) } } - - it 'returns an error' do - post_graphql_mutation(mutation, current_user: current_user) - - expect(response).to have_gitlab_http_status(:success) - expect(mutation_errors).not_to be_empty - end - end - - context 'feature flag is disabled' do - before do - stub_feature_flags(mr_attention_requests: false) - end - - it 'returns an error' do - post_graphql_mutation(mutation, current_user: current_user) - - expect(response).to have_gitlab_http_status(:success) - expect(graphql_errors[0]["message"]).to eq "Feature disabled" - end - end -end diff --git a/spec/requests/api/graphql/mutations/merge_requests/set_reviewers_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/set_reviewers_spec.rb new file mode 100644 index 00000000000..be786256ef2 --- /dev/null +++ b/spec/requests/api/graphql/mutations/merge_requests/set_reviewers_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Setting reviewers of a merge request', :assume_throttled do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:current_user) { create(:user, developer_projects: [project]) } + let_it_be(:reviewer) { create(:user) } + let_it_be(:reviewer2) { create(:user) } + let_it_be_with_reload(:merge_request) { create(:merge_request, source_project: project) } + + let(:input) { { reviewer_usernames: [reviewer.username] } } + let(:expected_result) do + [{ 'username' => reviewer.username }] + end + + let(:mutation) do + variables = { + project_path: project.full_path, + iid: merge_request.iid.to_s + } + graphql_mutation(:merge_request_set_reviewers, variables.merge(input), + <<-QL.strip_heredoc + clientMutationId + errors + mergeRequest { + id + reviewers { + nodes { + username + } + } + } + QL + ) + end + + def mutation_response + graphql_mutation_response(:merge_request_set_reviewers) + end + + def mutation_reviewer_nodes + mutation_response['mergeRequest']['reviewers']['nodes'] + end + + def run_mutation! + post_graphql_mutation(mutation, current_user: current_user) + end + + before do + project.add_developer(reviewer) + project.add_developer(reviewer2) + + merge_request.update!(reviewers: []) + end + + it 'returns an error if the user is not allowed to update the merge request' do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + end + + context 'when the current user does not have permission to add reviewers' do + let(:current_user) { create(:user) } + + it 'does not change the reviewers' do + project.add_guest(current_user) + + expect { run_mutation! }.not_to change { merge_request.reset.reviewers.pluck(:id) } + + expect(graphql_errors).not_to be_empty + end + end + + context 'with reviewers already assigned' do + before do + merge_request.reviewers = [reviewer2] + merge_request.save! + end + + it 'replaces the reviewer' do + run_mutation! + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_reviewer_nodes).to match_array(expected_result) + end + end + + context 'when passing an empty list of reviewers' do + let(:input) { { reviewer_usernames: [] } } + + before do + merge_request.reviewers = [reviewer2] + merge_request.save! + end + + it 'removes reviewer' do + run_mutation! + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_reviewer_nodes).to eq([]) + end + end +end diff --git a/spec/requests/api/graphql/mutations/merge_requests/update_reviewer_state_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/update_reviewer_state_spec.rb deleted file mode 100644 index cf497cb2579..00000000000 --- a/spec/requests/api/graphql/mutations/merge_requests/update_reviewer_state_spec.rb +++ /dev/null @@ -1,65 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Toggle attention requested for reviewer' do - include GraphqlHelpers - - let(:current_user) { create(:user) } - let(:merge_request) { create(:merge_request, reviewers: [user]) } - let(:project) { merge_request.project } - let(:user) { create(:user) } - let(:input) { { user_id: global_id_of(user) } } - - let(:mutation) do - variables = { - project_path: project.full_path, - iid: merge_request.iid.to_s - } - graphql_mutation(:merge_request_toggle_attention_requested, variables.merge(input), - <<-QL.strip_heredoc - clientMutationId - errors - QL - ) - end - - def mutation_response - graphql_mutation_response(:merge_request_toggle_attention_requested) - end - - def mutation_errors - mutation_response['errors'] - end - - before do - project.add_developer(current_user) - project.add_developer(user) - end - - it 'returns an error if the user is not allowed to update the merge request' do - post_graphql_mutation(mutation, current_user: create(:user)) - - expect(graphql_errors).not_to be_empty - end - - describe 'reviewer does not exist' do - let(:input) { { user_id: global_id_of(create(:user)) } } - - it 'returns an error' do - post_graphql_mutation(mutation, current_user: current_user) - - expect(response).to have_gitlab_http_status(:success) - expect(mutation_errors).not_to be_empty - end - end - - describe 'reviewer exists' do - it 'does not return an error' do - post_graphql_mutation(mutation, current_user: current_user) - - expect(response).to have_gitlab_http_status(:success) - expect(mutation_errors).to be_empty - end - end -end diff --git a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb index 22b5f2d5112..9c3842db31a 100644 --- a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb @@ -79,21 +79,29 @@ RSpec.describe 'Adding a Note' do context 'for an issue' do let(:noteable) { create(:issue, project: project) } - let(:mutation) do - variables = { + let(:mutation) { graphql_mutation(:create_note, variables) } + let(:variables) do + { noteable_id: GitlabSchema.id_from_object(noteable).to_s, - body: body, - confidential: true - } - - graphql_mutation(:create_note, variables) + body: body + }.merge(variables_extra) end before do project.add_developer(current_user) end - it_behaves_like 'a Note mutation with confidential notes' + context 'when using internal param' do + let(:variables_extra) { { internal: true } } + + it_behaves_like 'a Note mutation with confidential notes' + end + + context 'when using deprecated confidential param' do + let(:variables_extra) { { confidential: true } } + + it_behaves_like 'a Note mutation with confidential notes' + end end context 'when body only contains quick actions' do diff --git a/spec/requests/api/graphql/mutations/releases/create_spec.rb b/spec/requests/api/graphql/mutations/releases/create_spec.rb index 1e62942c29d..2541072b766 100644 --- a/spec/requests/api/graphql/mutations/releases/create_spec.rb +++ b/spec/requests/api/graphql/mutations/releases/create_spec.rb @@ -16,10 +16,10 @@ RSpec.describe 'Creation of a new release' do let(:mutation_name) { :release_create } - let(:tag_name) { 'v7.12.5'} + let(:tag_name) { 'v7.12.5' } let(:tag_message) { nil } - let(:ref) { 'master'} - let(:name) { 'Version 7.12.5'} + let(:ref) { 'master' } + let(:name) { 'Version 7.12.5' } let(:description) { 'Release 7.12.5 :rocket:' } let(:released_at) { '2018-12-10' } let(:milestones) { [milestone_12_3.title, milestone_12_4.title] } diff --git a/spec/requests/api/graphql/mutations/releases/update_spec.rb b/spec/requests/api/graphql/mutations/releases/update_spec.rb index 0fa3d7de299..33d4e57904c 100644 --- a/spec/requests/api/graphql/mutations/releases/update_spec.rb +++ b/spec/requests/api/graphql/mutations/releases/update_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'Updating an existing release' do let_it_be(:milestone_12_4) { create(:milestone, project: project, title: '12.4') } let_it_be(:tag_name) { 'v1.1.0' } - let_it_be(:name) { 'Version 7.12.5'} + let_it_be(:name) { 'Version 7.12.5' } let_it_be(:description) { 'Release 7.12.5 :rocket:' } let_it_be(:released_at) { '2018-12-10' } let_it_be(:created_at) { '2018-11-05' } diff --git a/spec/requests/api/graphql/mutations/remove_attention_request_spec.rb b/spec/requests/api/graphql/mutations/remove_attention_request_spec.rb deleted file mode 100644 index 053559b039d..00000000000 --- a/spec/requests/api/graphql/mutations/remove_attention_request_spec.rb +++ /dev/null @@ -1,79 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Remove attention request' do - include GraphqlHelpers - - let_it_be(:current_user) { create(:user) } - let_it_be(:user) { create(:user) } - let_it_be(:merge_request) { create(:merge_request, reviewers: [user]) } - let_it_be(:project) { merge_request.project } - - let(:input) { { user_id: global_id_of(user) } } - - let(:mutation) do - variables = { - project_path: project.full_path, - iid: merge_request.iid.to_s - } - graphql_mutation(:merge_request_remove_attention_request, variables.merge(input), - <<-QL.strip_heredoc - clientMutationId - errors - QL - ) - end - - def mutation_response - graphql_mutation_response(:merge_request_remove_attention_request) - end - - def mutation_errors - mutation_response['errors'] - end - - before_all do - project.add_developer(current_user) - project.add_developer(user) - end - - it 'is successful' do - post_graphql_mutation(mutation, current_user: current_user) - - expect(response).to have_gitlab_http_status(:success) - expect(mutation_errors).to be_empty - end - - context 'when current user is not allowed to update the merge request' do - it 'returns an error' do - post_graphql_mutation(mutation, current_user: create(:user)) - - expect(graphql_errors).not_to be_empty - end - end - - context 'when user is not a reviewer' do - let(:input) { { user_id: global_id_of(create(:user)) } } - - it 'returns an error' do - post_graphql_mutation(mutation, current_user: current_user) - - expect(response).to have_gitlab_http_status(:success) - expect(mutation_errors).not_to be_empty - end - end - - context 'feature flag is disabled' do - before do - stub_feature_flags(mr_attention_requests: false) - end - - it 'returns an error' do - post_graphql_mutation(mutation, current_user: current_user) - - expect(response).to have_gitlab_http_status(:success) - expect(graphql_errors[0]["message"]).to eq "Feature disabled" - end - end -end diff --git a/spec/requests/api/graphql/mutations/snippets/create_spec.rb b/spec/requests/api/graphql/mutations/snippets/create_spec.rb index 9a3cea3ca14..264fa5732c3 100644 --- a/spec/requests/api/graphql/mutations/snippets/create_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/create_spec.rb @@ -12,8 +12,8 @@ RSpec.describe 'Creating a Snippet' do let(:title) { 'Initial title' } let(:visibility_level) { 'public' } let(:action) { :create } - let(:file_1) { { filePath: 'example_file1', content: 'This is the example file 1' }} - let(:file_2) { { filePath: 'example_file2', content: 'This is the example file 2' }} + let(:file_1) { { filePath: 'example_file1', content: 'This is the example file 1' } } + let(:file_2) { { filePath: 'example_file2', content: 'This is the example file 2' } } let(:actions) { [{ action: action }.merge(file_1), { action: action }.merge(file_2)] } let(:project_path) { nil } let(:uploaded_files) { nil } @@ -149,7 +149,7 @@ RSpec.describe 'Creating a Snippet' do end context 'when there non ActiveRecord errors' do - let(:file_1) { { filePath: 'invalid://file/path', content: 'foobar' }} + let(:file_1) { { filePath: 'invalid://file/path', content: 'foobar' } } it_behaves_like 'a mutation that returns errors in the response', errors: ['Repository Error creating the snippet - Invalid file name'] it_behaves_like 'does not create snippet' diff --git a/spec/requests/api/graphql/mutations/timelogs/create_spec.rb b/spec/requests/api/graphql/mutations/timelogs/create_spec.rb new file mode 100644 index 00000000000..eea04b89783 --- /dev/null +++ b/spec/requests/api/graphql/mutations/timelogs/create_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Create a timelog' do + include GraphqlHelpers + + let_it_be(:author) { create(:user) } + let_it_be(:project) { create(:project, :public) } + let_it_be(:time_spent) { '1h' } + + let(:current_user) { nil } + let(:users_container) { project } + let(:mutation) do + graphql_mutation(:timelogCreate, { + 'time_spent' => time_spent, + 'spent_at' => '2022-07-08', + 'summary' => 'Test summary', + 'issuable_id' => issuable.to_global_id.to_s + }) + end + + let(:mutation_response) { graphql_mutation_response(:timelog_create) } + + context 'when issuable is an Issue' do + let_it_be(:issuable) { create(:issue, project: project) } + + it_behaves_like 'issuable supports timelog creation mutation' + end + + context 'when issuable is a MergeRequest' do + let_it_be(:issuable) { create(:merge_request, source_project: project) } + + it_behaves_like 'issuable supports timelog creation mutation' + end + + context 'when issuable is a WorkItem' do + let_it_be(:issuable) { create(:work_item, project: project, title: 'WorkItem') } + + it_behaves_like 'issuable supports timelog creation mutation' + end + + context 'when issuable is an Incident' do + let_it_be(:issuable) { create(:incident, project: project) } + + it_behaves_like 'issuable supports timelog creation mutation' + end +end diff --git a/spec/requests/api/graphql/mutations/timelogs/delete_spec.rb b/spec/requests/api/graphql/mutations/timelogs/delete_spec.rb index b674e77f093..d304bfbdf00 100644 --- a/spec/requests/api/graphql/mutations/timelogs/delete_spec.rb +++ b/spec/requests/api/graphql/mutations/timelogs/delete_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'Delete a timelog' do let_it_be(:author) { create(:user) } let_it_be(:project) { create(:project, :public) } let_it_be(:issue) { create(:issue, project: project) } - let_it_be(:timelog) { create(:timelog, user: author, issue: issue, time_spent: 1800)} + let_it_be(:timelog) { create(:timelog, user: author, issue: issue, time_spent: 1800) } let(:current_user) { nil } let(:mutation) { graphql_mutation(:timelogDelete, { 'id' => timelog.to_global_id.to_s }) } diff --git a/spec/requests/api/graphql/mutations/uploads/delete_spec.rb b/spec/requests/api/graphql/mutations/uploads/delete_spec.rb new file mode 100644 index 00000000000..f44bf179397 --- /dev/null +++ b/spec/requests/api/graphql/mutations/uploads/delete_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Delete an upload' do + include GraphqlHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:developer) { create(:user).tap { |user| group.add_developer(user) } } + let_it_be(:maintainer) { create(:user).tap { |user| group.add_maintainer(user) } } + + let(:extra_params) { {} } + let(:params) { { filename: File.basename(upload.path), secret: upload.secret }.merge(extra_params) } + let(:mutation) { graphql_mutation(:uploadDelete, params) } + let(:mutation_response) { graphql_mutation_response(:upload_delete) } + + shared_examples_for 'upload deletion' do + context 'when the user is not allowed to delete uploads' do + let(:current_user) { developer } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when the user is anonymous' do + let(:current_user) { nil } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permissions to delete uploads' do + let(:current_user) { maintainer } + + it 'deletes the upload' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['upload']).to include('id' => upload.to_global_id.to_s) + expect(mutation_response['errors']).to be_empty + end + + context 'when upload does not exist' do + let(:params) { { filename: 'invalid', secret: upload.secret }.merge(extra_params) } + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['upload']).to be_nil + expect(mutation_response['errors']).to match_array([ + "The resource that you are attempting to access does not "\ + "exist or you don't have permission to perform this action." + ]) + end + end + end + end + + context 'when deleting project upload' do + let_it_be_with_reload(:upload) { create(:upload, :issuable_upload, model: project) } + + let(:extra_params) { { project_path: project.full_path } } + + it_behaves_like 'upload deletion' + end + + context 'when deleting group upload' do + let_it_be_with_reload(:upload) { create(:upload, :namespace_upload, model: group) } + + let(:extra_params) { { group_path: group.full_path } } + + it_behaves_like 'upload deletion' + end +end diff --git a/spec/requests/api/graphql/mutations/work_items/create_from_task_spec.rb b/spec/requests/api/graphql/mutations/work_items/create_from_task_spec.rb index b1356bbe6fd..e7f4917ddde 100644 --- a/spec/requests/api/graphql/mutations/work_items/create_from_task_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/create_from_task_spec.rb @@ -7,7 +7,7 @@ RSpec.describe "Create a work item from a task in a work item's description" do 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, description: '- [ ] A task in a list', lock_version: 3) } + let_it_be(:work_item, refind: true) { create(:work_item, :confidential, project: project, description: '- [ ] A task in a list', lock_version: 3) } let(:lock_version) { work_item.lock_version } let(:input) do @@ -48,6 +48,7 @@ RSpec.describe "Create a work item from a task in a work item's description" do expect(created_work_item.issue_type).to eq('task') expect(created_work_item.work_item_type.base_type).to eq('task') expect(created_work_item.work_item_parent).to eq(work_item) + expect(created_work_item).to be_confidential expect(mutation_response['workItem']).to include('id' => work_item.to_global_id.to_s) expect(mutation_response['newWorkItem']).to include('id' => created_work_item.to_global_id.to_s) 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 911568bc39f..8233821053f 100644 --- a/spec/requests/api/graphql/mutations/work_items/create_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/create_spec.rb @@ -12,6 +12,7 @@ RSpec.describe 'Create a work item' do { 'title' => 'new title', 'description' => 'new description', + 'confidential' => true, 'workItemTypeId' => WorkItems::Type.default_by_type(:task).to_global_id.to_s } end @@ -38,6 +39,7 @@ RSpec.describe 'Create a work item' do expect(response).to have_gitlab_http_status(:success) expect(created_work_item.issue_type).to eq('task') + expect(created_work_item).to be_confidential expect(created_work_item.work_item_type.base_type).to eq('task') expect(mutation_response['workItem']).to include( input.except('workItemTypeId').merge( @@ -127,7 +129,7 @@ RSpec.describe 'Create a work item' do end context 'when parent work item is not found' do - let_it_be(:parent) { build_stubbed(:work_item, id: non_existing_record_id)} + let_it_be(:parent) { build_stubbed(:work_item, id: non_existing_record_id) } it 'returns a top level error' do post_graphql_mutation(mutation, current_user: current_user) diff --git a/spec/requests/api/graphql/mutations/work_items/update_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_spec.rb index 77f7b9bacef..909d6549fa5 100644 --- a/spec/requests/api/graphql/mutations/work_items/update_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/update_spec.rb @@ -34,6 +34,10 @@ RSpec.describe 'Update a work item' do context 'when user has permissions to update a work item' do let(:current_user) { developer } + it_behaves_like 'has spam protection' do + let(:mutation_class) { ::Mutations::WorkItems::Update } + end + context 'when the work item is open' do it 'closes and updates the work item' do expect do @@ -71,36 +75,48 @@ RSpec.describe 'Update a work item' do end end - context 'when unsupported widget input is sent' do - let_it_be(:test_case) { create(:work_item_type, :default, :test_case, name: 'some_test_case_name') } - let_it_be(:work_item) { create(:work_item, work_item_type: test_case, project: project) } - - let(:input) do - { - 'hierarchyWidget' => {} + context 'when updating confidentiality' do + let(:fields) do + <<~FIELDS + workItem { + confidential } + errors + FIELDS end - it_behaves_like 'a mutation that returns top-level errors', - errors: ["Following widget keys are not supported by some_test_case_name type: [:hierarchy_widget]"] - end + shared_examples 'toggling confidentiality' do + it 'successfully updates work item' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to change(work_item, :confidential).from(values[:old]).to(values[:new]) - it_behaves_like 'has spam protection' do - let(:mutation_class) { ::Mutations::WorkItems::Update } - end + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']).to include( + 'confidential' => values[:new] + ) + end + end - context 'when the work_items feature flag is disabled' do - before do - stub_feature_flags(work_items: false) + context 'when setting as confidential' do + let(:input) { { 'confidential' => true } } + + it_behaves_like 'toggling confidentiality' do + let(:values) { { old: false, new: true } } + end 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) + context 'when setting as non-confidential' do + let(:input) { { 'confidential' => false } } - expect(mutation_response['errors']).to contain_exactly('`work_items` feature flag disabled for this project') + before do + work_item.update!(confidential: true) + end + + it_behaves_like 'toggling confidentiality' do + let(:values) { { old: true, new: false } } + end end end @@ -128,26 +144,90 @@ RSpec.describe 'Update a work item' do end end - context 'with weight widget input' do + context 'with due and start date widget input' do + let(:start_date) { Date.today } + let(:due_date) { 1.week.from_now.to_date } let(:fields) do <<~FIELDS - workItem { - widgets { - type - ... on WorkItemWidgetWeight { - weight + workItem { + widgets { + type + ... on WorkItemWidgetStartAndDueDate { + startDate + dueDate + } } } - } - errors + errors FIELDS end - it_behaves_like 'update work item weight widget' do - let(:new_weight) { 2 } + let(:input) do + { 'startAndDueDateWidget' => { 'startDate' => start_date.to_s, 'dueDate' => due_date.to_s } } + end - let(:input) do - { 'weightWidget' => { 'weight' => new_weight } } + it 'updates start and due date' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to change(work_item, :start_date).from(nil).to(start_date).and( + change(work_item, :due_date).from(nil).to(due_date) + ) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']['widgets']).to include( + { + 'startDate' => start_date.to_s, + 'dueDate' => due_date.to_s, + 'type' => 'START_AND_DUE_DATE' + } + ) + end + + context 'when provided input is invalid' do + let(:due_date) { 1.week.ago.to_date } + + it 'returns validation errors without the work item' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['workItem']).to be_nil + expect(mutation_response['errors']).to contain_exactly('Due date must be greater than or equal to start date') + end + end + + context 'when dates were already set for the work item' do + before do + work_item.update!(start_date: start_date, due_date: due_date) + end + + context 'when updating only start date' do + let(:input) do + { 'startAndDueDateWidget' => { 'startDate' => nil } } + end + + it 'allows setting a single date to null' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to change(work_item, :start_date).from(start_date).to(nil).and( + not_change(work_item, :due_date).from(due_date) + ) + end + end + + context 'when updating only due date' do + let(:input) do + { 'startAndDueDateWidget' => { 'dueDate' => nil } } + end + + it 'allows setting a single date to null' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to change(work_item, :due_date).from(due_date).to(nil).and( + not_change(work_item, :start_date).from(start_date) + ) + end end end end @@ -179,7 +259,7 @@ RSpec.describe 'Update a work item' do end context 'when updating parent' do - let_it_be(:work_item) { create(:work_item, :task, project: project) } + let_it_be(:work_item, reload: true) { create(:work_item, :task, project: project) } let_it_be(:valid_parent) { create(:work_item, project: project) } let_it_be(:invalid_parent) { create(:work_item, :task, project: project) } @@ -346,5 +426,78 @@ RSpec.describe 'Update a work item' do end end end + + context 'when updating assignees' do + let(:fields) do + <<~FIELDS + workItem { + widgets { + type + ... on WorkItemWidgetAssignees { + assignees { + nodes { + id + username + } + } + } + } + } + errors + FIELDS + end + + let(:input) do + { 'assigneesWidget' => { 'assigneeIds' => [developer.to_global_id.to_s] } } + end + + it 'updates the work item assignee' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to change(work_item, :assignee_ids).from([]).to([developer.id]) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']['widgets']).to include( + { + 'type' => 'ASSIGNEES', + 'assignees' => { + 'nodes' => [ + { 'id' => developer.to_global_id.to_s, 'username' => developer.username } + ] + } + } + ) + end + end + + context 'when unsupported widget input is sent' do + let_it_be(:test_case) { create(:work_item_type, :default, :test_case, name: 'some_test_case_name') } + let_it_be(:work_item) { create(:work_item, work_item_type: test_case, project: project) } + + let(:input) do + { + 'hierarchyWidget' => {} + } + end + + it_behaves_like 'a mutation that returns top-level errors', + errors: ["Following widget keys are not supported by some_test_case_name type: [:hierarchy_widget]"] + 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/namespace/root_storage_statistics_spec.rb b/spec/requests/api/graphql/namespace/root_storage_statistics_spec.rb index 37cc502103d..8d8a0baae36 100644 --- a/spec/requests/api/graphql/namespace/root_storage_statistics_spec.rb +++ b/spec/requests/api/graphql/namespace/root_storage_statistics_spec.rb @@ -49,7 +49,7 @@ RSpec.describe 'rendering namespace statistics' do it_behaves_like 'a working namespace with storage statistics query' context 'when the namespace is public' do - let(:group) { create(:group, :public)} + let(:group) { create(:group, :public) } it 'hides statistics for unauthenticated requests' do post_graphql(query, current_user: nil) diff --git a/spec/requests/api/graphql/packages/conan_spec.rb b/spec/requests/api/graphql/packages/conan_spec.rb index 1f3732980d9..5bd5a71bbeb 100644 --- a/spec/requests/api/graphql/packages/conan_spec.rb +++ b/spec/requests/api/graphql/packages/conan_spec.rb @@ -8,7 +8,7 @@ RSpec.describe 'conan package details' do let_it_be(:package) { create(:conan_package, project: project) } let(:metadata) { query_graphql_fragment('ConanMetadata') } - let(:package_files_metadata) {query_graphql_fragment('ConanFileMetadata')} + let(:package_files_metadata) { query_graphql_fragment('ConanFileMetadata') } let(:query) do graphql_query_for(:package, { id: package_global_id }, <<~FIELDS) diff --git a/spec/requests/api/graphql/packages/helm_spec.rb b/spec/requests/api/graphql/packages/helm_spec.rb index 397096f70db..1675b8faa23 100644 --- a/spec/requests/api/graphql/packages/helm_spec.rb +++ b/spec/requests/api/graphql/packages/helm_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'helm package details' do let_it_be(:package) { create(:helm_package, project: project) } - let(:package_files_metadata) {query_graphql_fragment('HelmFileMetadata')} + let(:package_files_metadata) { query_graphql_fragment('HelmFileMetadata') } let(:query) do graphql_query_for(:package, { id: package_global_id }, <<~FIELDS) diff --git a/spec/requests/api/graphql/packages/package_spec.rb b/spec/requests/api/graphql/packages/package_spec.rb index 0335c1085b4..c28b37db5af 100644 --- a/spec/requests/api/graphql/packages/package_spec.rb +++ b/spec/requests/api/graphql/packages/package_spec.rb @@ -18,7 +18,7 @@ RSpec.describe 'package details' do let(:depth) { 3 } let(:excluded) { %w[metadata apiFuzzingCiConfiguration pipeline packageFiles] } let(:metadata) { query_graphql_fragment('ComposerMetadata') } - let(:package_files) {all_graphql_fields_for('PackageFile')} + let(:package_files) { all_graphql_fields_for('PackageFile') } let(:package_global_id) { global_id_of(composer_package) } let(:package_details) { graphql_data_at(:package) } diff --git a/spec/requests/api/graphql/project/base_service_spec.rb b/spec/requests/api/graphql/project/base_service_spec.rb index 5dc0f55db88..58d10ade8cf 100644 --- a/spec/requests/api/graphql/project/base_service_spec.rb +++ b/spec/requests/api/graphql/project/base_service_spec.rb @@ -26,7 +26,7 @@ RSpec.describe 'query Jira service' do ) end - let(:services) { graphql_data.dig('project', 'services', 'nodes')} + let(:services) { graphql_data.dig('project', 'services', 'nodes') } it_behaves_like 'unauthorized users cannot read services' 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 2b85704f479..2fe5fb593fe 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 @@ -34,6 +34,8 @@ RSpec.describe 'getting a detailed sentry error' do context 'when data is loading via reactive cache' do before do + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) + post_graphql(query, current_user: current_user) end @@ -48,6 +50,10 @@ RSpec.describe 'getting a detailed sentry error' do .to receive(:issue_details) .and_return({ issue: sentry_detailed_error }) + expect(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event) + .with('error_tracking_view_details', values: current_user.id) + post_graphql(query, current_user: current_user) end diff --git a/spec/requests/api/graphql/project/fork_targets_spec.rb b/spec/requests/api/graphql/project/fork_targets_spec.rb new file mode 100644 index 00000000000..b21a11ff4dc --- /dev/null +++ b/spec/requests/api/graphql/project/fork_targets_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting a list of fork targets for a project' do + include GraphqlHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:another_group) { create(:group) } + let_it_be(:project) { create(:project, :private, group: group) } + let_it_be(:user) { create(:user, developer_projects: [project]) } + + let(:current_user) { user } + let(:fields) do + <<~GRAPHQL + forkTargets{ + nodes { id name fullPath visibility } + } + GRAPHQL + end + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + fields + ) + end + + before_all do + group.add_owner(user) + another_group.add_owner(user) + end + + context 'when user has access to the project' do + before do + post_graphql(query, current_user: current_user) + end + + it_behaves_like 'a working graphql query' + + it 'returns fork targets for the project' do + expect(graphql_data.dig('project', 'forkTargets', 'nodes')).to match_array( + [user.namespace, project.namespace, another_group].map do |target| + hash_including( + { + 'id' => target.to_global_id.to_s, + 'name' => target.name, + 'fullPath' => target.full_path, + 'visibility' => target.visibility + } + ) + end + ) + end + end + + context "when user doesn't have access to the project" do + let(:current_user) { create(:user) } + + before do + post_graphql(query, current_user: current_user) + end + + it 'does not return the project' do + expect(graphql_data).to eq('project' => nil) + end + end +end diff --git a/spec/requests/api/graphql/project/jira_import_spec.rb b/spec/requests/api/graphql/project/jira_import_spec.rb index 98a3f08baa6..202220f4bf6 100644 --- a/spec/requests/api/graphql/project/jira_import_spec.rb +++ b/spec/requests/api/graphql/project/jira_import_spec.rb @@ -56,8 +56,8 @@ RSpec.describe 'query Jira import data' do ) end - let(:jira_imports) { graphql_data.dig('project', 'jiraImports', 'nodes')} - let(:jira_import_status) { graphql_data.dig('project', 'jiraImportStatus')} + let(:jira_imports) { graphql_data.dig('project', 'jiraImports', 'nodes') } + let(:jira_import_status) { graphql_data.dig('project', 'jiraImportStatus') } context 'when user cannot read Jira import data' do before do @@ -89,11 +89,11 @@ RSpec.describe 'query Jira import data' do context 'list of jira imports sorted ascending by scheduledAt time' do it 'retuns list of jira imports' do - jira_proket_keys = jira_imports.map {|ji| ji['jiraProjectKey']} - usernames = jira_imports.map {|ji| ji.dig('scheduledBy', 'username')} - imported_issues_count = jira_imports.map {|ji| ji.dig('importedIssuesCount')} - failed_issues_count = jira_imports.map {|ji| ji.dig('failedToImportCount')} - total_issue_count = jira_imports.map {|ji| ji.dig('totalIssueCount')} + jira_proket_keys = jira_imports.map { |ji| ji['jiraProjectKey'] } + usernames = jira_imports.map { |ji| ji.dig('scheduledBy', 'username') } + imported_issues_count = jira_imports.map { |ji| ji.dig('importedIssuesCount') } + failed_issues_count = jira_imports.map { |ji| ji.dig('failedToImportCount') } + total_issue_count = jira_imports.map { |ji| ji.dig('totalIssueCount') } expect(jira_imports.size).to eq 2 expect(jira_proket_keys).to eq %w(BB AA) diff --git a/spec/requests/api/graphql/project/project_members_spec.rb b/spec/requests/api/graphql/project/project_members_spec.rb index 4225c3ad3e8..97a79ab3b0e 100644 --- a/spec/requests/api/graphql/project/project_members_spec.rb +++ b/spec/requests/api/graphql/project/project_members_spec.rb @@ -48,24 +48,6 @@ RSpec.describe 'getting project members information' do expect_array_response(user_2) end - - context 'when the use_keyset_aware_user_search_query FF is off' do - before do - stub_feature_flags(use_keyset_aware_user_search_query: false) - end - - it 'raises error on the 2nd page due to missing cursor data' do - fetch_members(project: parent_project, args: { search: 'Same Name', first: 1 }) - - # user_2 because the "old" order was undeterministic (insert order), no tie-breaker column - expect_array_response(user_2) - - next_cursor = graphql_data_at(:project, :projectMembers, :pageInfo, :endCursor) - fetch_members(project: parent_project, args: { search: 'Same Name', first: 1, after: next_cursor }) - - expect(graphql_errors.first['message']).to include('PG::UndefinedColumn') - end - end end end end diff --git a/spec/requests/api/graphql/project/work_items_spec.rb b/spec/requests/api/graphql/project/work_items_spec.rb index 66742fcbeb6..6ef28392b8b 100644 --- a/spec/requests/api/graphql/project/work_items_spec.rb +++ b/spec/requests/api/graphql/project/work_items_spec.rb @@ -21,7 +21,7 @@ RSpec.describe 'getting an work item list for a project' do <<~QUERY edges { node { - #{all_graphql_fields_for('workItems'.classify)} + #{all_graphql_fields_for('workItems'.classify, max_depth: 2)} } } QUERY diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb index 310a8e9fa33..d1b990629a1 100644 --- a/spec/requests/api/graphql/project_query_spec.rb +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -190,4 +190,100 @@ RSpec.describe 'getting project information' do end end end + + context 'with timelog categories' do + let_it_be(:timelog_category) do + create(:timelog_category, namespace: project.project_namespace, name: 'TimelogCategoryTest') + end + + let(:project_fields) do + <<~GQL + timelogCategories { + nodes { + #{all_graphql_fields_for('TimeTrackingTimelogCategory')} + } + } + GQL + end + + context 'when user is guest and the project is public' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it 'includes empty timelog categories array' do + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:project, :timelogCategories, :nodes)).to match([]) + end + end + + context 'when user has reporter role' do + before do + project.add_reporter(current_user) + end + + it 'returns the timelog category with all its fields' do + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:project, :timelogCategories, :nodes)) + .to contain_exactly(a_graphql_entity_for(timelog_category)) + end + + context 'when timelog_categories flag is disabled' do + before do + stub_feature_flags(timelog_categories: false) + end + + it 'returns no timelog categories' do + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:project, :timelogCategories)).to be_nil + end + end + end + + context 'for N+1 queries' do + let!(:project1) { create(:project) } + let!(:project2) { create(:project) } + + before do + project1.add_reporter(current_user) + project2.add_reporter(current_user) + end + + it 'avoids N+1 database queries' do + pending('See: https://gitlab.com/gitlab-org/gitlab/-/issues/369396') + + ctx = { current_user: current_user } + + baseline_query = <<~GQL + query { + a: project(fullPath: "#{project1.full_path}") { ... p } + } + + fragment p on Project { + timelogCategories { nodes { name } } + } + GQL + + query = <<~GQL + query { + a: project(fullPath: "#{project1.full_path}") { ... p } + b: project(fullPath: "#{project2.full_path}") { ... p } + } + + fragment p on Project { + timelogCategories { nodes { name } } + } + GQL + + control = ActiveRecord::QueryRecorder.new do + run_with_clean_state(baseline_query, context: ctx) + end + + expect { run_with_clean_state(query, context: ctx) }.not_to exceed_query_limit(control) + end + end + end end diff --git a/spec/requests/api/graphql/work_item_spec.rb b/spec/requests/api/graphql/work_item_spec.rb index f17d2ebbb7e..34644e5893a 100644 --- a/spec/requests/api/graphql/work_item_spec.rb +++ b/spec/requests/api/graphql/work_item_spec.rb @@ -8,7 +8,16 @@ RSpec.describe 'Query.work_item(id)' do let_it_be(:developer) { create(:user) } let_it_be(:guest) { create(:user) } let_it_be(:project) { create(:project, :private) } - let_it_be(:work_item) { create(:work_item, project: project, description: '- List item', weight: 1) } + let_it_be(:work_item) do + create( + :work_item, + project: project, + description: '- List item', + start_date: Date.today, + due_date: 1.week.from_now + ) + end + let_it_be(:child_item1) { create(:work_item, :task, project: project) } let_it_be(:child_item2) { create(:work_item, :task, confidential: true, project: project) } let_it_be(:child_link1) { create(:parent_link, work_item_parent: work_item, work_item: child_item1) } @@ -16,7 +25,7 @@ RSpec.describe 'Query.work_item(id)' do let(:current_user) { developer } let(:work_item_data) { graphql_data['workItem'] } - let(:work_item_fields) { all_graphql_fields_for('WorkItem') } + let(:work_item_fields) { all_graphql_fields_for('WorkItem', max_depth: 2) } let(:global_id) { work_item.to_gid.to_s } let(:query) do @@ -41,8 +50,10 @@ RSpec.describe 'Query.work_item(id)' do 'lockVersion' => work_item.lock_version, 'state' => "OPEN", 'title' => work_item.title, + 'confidential' => work_item.confidential, 'workItemType' => hash_including('id' => work_item.work_item_type.to_gid.to_s), - 'userPermissions' => { 'readWorkItem' => true, 'updateWorkItem' => true, 'deleteWorkItem' => false } + 'userPermissions' => { 'readWorkItem' => true, 'updateWorkItem' => true, 'deleteWorkItem' => false }, + 'project' => hash_including('id' => project.to_gid.to_s, 'fullPath' => project.full_path) ) end @@ -163,14 +174,24 @@ RSpec.describe 'Query.work_item(id)' do end end - describe 'weight widget' do + describe 'assignees widget' do + let(:assignees) { create_list(:user, 2) } + let(:work_item) { create(:work_item, project: project, assignees: assignees) } + let(:work_item_fields) do <<~GRAPHQL id widgets { type - ... on WorkItemWidgetWeight { - weight + ... on WorkItemWidgetAssignees { + allowsMultipleAssignees + canInviteMembers + assignees { + nodes { + id + username + } + } } } GRAPHQL @@ -181,30 +202,34 @@ RSpec.describe 'Query.work_item(id)' do 'id' => work_item.to_gid.to_s, 'widgets' => include( hash_including( - 'type' => 'WEIGHT', - 'weight' => work_item.weight + 'type' => 'ASSIGNEES', + 'allowsMultipleAssignees' => boolean, + 'canInviteMembers' => boolean, + 'assignees' => { + 'nodes' => match_array( + assignees.map { |a| { 'id' => a.to_gid.to_s, 'username' => a.username } } + ) + } ) ) ) end end - describe 'assignees widget' do - let(:assignees) { create_list(:user, 2) } - let(:work_item) { create(:work_item, project: project, assignees: assignees) } + describe 'labels widget' do + let(:labels) { create_list(:label, 2, project: project) } + let(:work_item) { create(:work_item, project: project, labels: labels) } let(:work_item_fields) do <<~GRAPHQL id widgets { type - ... on WorkItemWidgetAssignees { - allowsMultipleAssignees - canInviteMembers - assignees { + ... on WorkItemWidgetLabels { + labels { nodes { id - username + title } } } @@ -217,12 +242,10 @@ RSpec.describe 'Query.work_item(id)' do 'id' => work_item.to_gid.to_s, 'widgets' => include( hash_including( - 'type' => 'ASSIGNEES', - 'allowsMultipleAssignees' => boolean, - 'canInviteMembers' => boolean, - 'assignees' => { + 'type' => 'LABELS', + 'labels' => { 'nodes' => match_array( - assignees.map { |a| { 'id' => a.to_gid.to_s, 'username' => a.username } } + labels.map { |a| { 'id' => a.to_gid.to_s, 'title' => a.title } } ) } ) @@ -230,6 +253,34 @@ RSpec.describe 'Query.work_item(id)' do ) end end + + describe 'start and due date widget' do + let(:work_item_fields) do + <<~GRAPHQL + id + widgets { + type + ... on WorkItemWidgetStartAndDueDate { + startDate + dueDate + } + } + GRAPHQL + end + + it 'returns widget information' do + expect(work_item_data).to include( + 'id' => work_item.to_gid.to_s, + 'widgets' => include( + hash_including( + 'type' => 'START_AND_DUE_DATE', + 'startDate' => work_item.start_date.to_s, + 'dueDate' => work_item.due_date.to_s + ) + ) + ) + end + end end context 'when an Issue Global ID is provided' do diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb index d94257c61eb..1c1ae73ddfe 100644 --- a/spec/requests/api/graphql_spec.rb +++ b/spec/requests/api/graphql_spec.rb @@ -463,50 +463,21 @@ RSpec.describe 'GraphQL' do ) end - context 'when new_graphql_keyset_pagination feature flag is off' do - before do - stub_feature_flags(new_graphql_keyset_pagination: false) - end - - it 'paginates datetimes correctly when they have millisecond data' do - # let's make sure we're actually querying a timestamp, just in case - expect(Gitlab::Graphql::Pagination::Keyset::QueryBuilder) - .to receive(:new).with(anything, anything, hash_including('created_at'), anything).and_call_original + it 'paginates datetimes correctly when they have millisecond data' do + execute_query + first_page = graphql_data + edges = first_page.dig(*issues_edges) + cursor = first_page.dig(*end_cursor) - execute_query - first_page = graphql_data - edges = first_page.dig(*issues_edges) - cursor = first_page.dig(*end_cursor) + expect(edges.count).to eq(6) + expect(edges.last['node']['iid']).to eq(issues[4].iid.to_s) - expect(edges.count).to eq(6) - expect(edges.last['node']['iid']).to eq(issues[4].iid.to_s) + execute_query(after: cursor) + second_page = graphql_data + edges = second_page.dig(*issues_edges) - execute_query(after: cursor) - second_page = graphql_data - edges = second_page.dig(*issues_edges) - - expect(edges.count).to eq(4) - expect(edges.last['node']['iid']).to eq(issues[0].iid.to_s) - end - end - - context 'when new_graphql_keyset_pagination feature flag is on' do - it 'paginates datetimes correctly when they have millisecond data' do - execute_query - first_page = graphql_data - edges = first_page.dig(*issues_edges) - cursor = first_page.dig(*end_cursor) - - expect(edges.count).to eq(6) - expect(edges.last['node']['iid']).to eq(issues[4].iid.to_s) - - execute_query(after: cursor) - second_page = graphql_data - edges = second_page.dig(*issues_edges) - - expect(edges.count).to eq(4) - expect(edges.last['node']['iid']).to eq(issues[0].iid.to_s) - end + expect(edges.count).to eq(4) + expect(edges.last['node']['iid']).to eq(issues[0].iid.to_s) end end end diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index a7b4bea362f..4fed7dd7624 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -91,7 +91,7 @@ RSpec.describe API::GroupVariables do it 'creates variable' do expect do post api("/groups/#{group.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'PROTECTED_VALUE_2', protected: true, masked: true } - end.to change {group.variables.count}.by(1) + end.to change { group.variables.count }.by(1) expect(response).to have_gitlab_http_status(:created) expect(json_response['key']).to eq('TEST_VARIABLE_2') @@ -105,7 +105,7 @@ RSpec.describe API::GroupVariables do it 'creates variable with optional attributes' do expect do post api("/groups/#{group.id}/variables", user), params: { variable_type: 'file', key: 'TEST_VARIABLE_2', value: 'VALUE_2' } - end.to change {group.variables.count}.by(1) + end.to change { group.variables.count }.by(1) expect(response).to have_gitlab_http_status(:created) expect(json_response['key']).to eq('TEST_VARIABLE_2') @@ -119,7 +119,7 @@ RSpec.describe API::GroupVariables do it 'does not allow to duplicate variable key' do expect do post api("/groups/#{group.id}/variables", user), params: { key: variable.key, value: 'VALUE_2' } - end.to change {group.variables.count}.by(0) + end.to change { group.variables.count }.by(0) expect(response).to have_gitlab_http_status(:bad_request) end @@ -207,7 +207,7 @@ RSpec.describe API::GroupVariables do delete api("/groups/#{group.id}/variables/#{variable.key}", user) expect(response).to have_gitlab_http_status(:no_content) - end.to change {group.variables.count}.by(-1) + end.to change { group.variables.count }.by(-1) end it 'responds with 404 Not Found if requesting non-existing variable' do diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 3bc3cce5310..bc37f8e4655 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -864,7 +864,7 @@ RSpec.describe API::Groups do end describe 'PUT /groups/:id' do - let(:new_group_name) { 'New Group'} + let(:new_group_name) { 'New Group' } let(:file_path) { 'spec/fixtures/dk.png' } it_behaves_like 'group avatar upload' do diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 8961f3177b6..e29e5c31a34 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -26,8 +26,8 @@ RSpec.describe API::Helpers do } end - let(:header) { } - let(:request) { Grape::Request.new(env)} + let(:header) {} + let(:request) { Grape::Request.new(env) } let(:params) { request.params } before do @@ -539,7 +539,7 @@ RSpec.describe API::Helpers do let(:token) { create(:oauth_access_token) } before do - env['HTTP_AUTHORIZATION'] = "Bearer #{token.token}" + env['HTTP_AUTHORIZATION'] = "Bearer #{token.plaintext_token}" end it_behaves_like 'sudo' diff --git a/spec/requests/api/integrations_spec.rb b/spec/requests/api/integrations_spec.rb index b2db7f7caef..1e8061f9606 100644 --- a/spec/requests/api/integrations_spec.rb +++ b/spec/requests/api/integrations_spec.rb @@ -66,6 +66,7 @@ RSpec.describe API::Integrations do mattermost: %i[deployment_channel labels_to_be_notified], mock_ci: %i[enable_ssl_verification], prometheus: %i[manual_configuration], + pumble: %i[branches_to_be_notified notify_only_broken_pipelines], slack: %i[alert_events alert_channel deployment_channel labels_to_be_notified], unify_circuit: %i[branches_to_be_notified notify_only_broken_pipelines], webex_teams: %i[branches_to_be_notified notify_only_broken_pipelines] diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index acfe476a864..e100684018a 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -376,10 +376,17 @@ RSpec.describe API::Internal::Base do shared_examples 'rate limited request' do let(:action) { 'git-upload-pack' } let(:actor) { key } + let(:rate_limiter) { double(:rate_limiter, ip: "127.0.0.1", trusted_ip?: false) } + + before do + allow(::Gitlab::Auth::IpRateLimiter).to receive(:new).with("127.0.0.1").and_return(rate_limiter) + end it 'is throttled by rate limiter' do allow(::Gitlab::ApplicationRateLimiter).to receive(:threshold).and_return(1) + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:gitlab_shell_operation, scope: [action, project.full_path, actor]).twice.and_call_original + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:gitlab_shell_operation, scope: [action, project.full_path, "127.0.0.1"]).and_call_original request @@ -402,6 +409,28 @@ RSpec.describe API::Internal::Base do subject end end + + context 'when rate_limit_gitlab_shell_by_ip feature flag is disabled' do + before do + stub_feature_flags(rate_limit_gitlab_shell_by_ip: false) + end + + it 'is not throttled by rate limiter' do + expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?) + + subject + end + end + + context 'when the IP is in a trusted range' do + let(:rate_limiter) { double(:rate_limiter, ip: "127.0.0.1", trusted_ip?: true) } + + it 'is not throttled by rate limiter' do + expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?) + + subject + end + end end context "access granted" do @@ -1451,7 +1480,7 @@ RSpec.describe API::Internal::Base do describe 'POST /internal/two_factor_otp_check' do let(:key_id) { key.id } - let(:otp) { '123456'} + let(:otp) { '123456' } subject do post api('/internal/two_factor_otp_check'), @@ -1472,7 +1501,7 @@ RSpec.describe API::Internal::Base do describe 'POST /internal/two_factor_manual_otp_check' do let(:key_id) { key.id } - let(:otp) { '123456'} + let(:otp) { '123456' } subject do post api('/internal/two_factor_manual_otp_check'), @@ -1493,7 +1522,7 @@ RSpec.describe API::Internal::Base do describe 'POST /internal/two_factor_push_otp_check' do let(:key_id) { key.id } - let(:otp) { '123456'} + let(:otp) { '123456' } subject do post api('/internal/two_factor_push_otp_check'), @@ -1514,7 +1543,7 @@ RSpec.describe API::Internal::Base do describe 'POST /internal/two_factor_manual_otp_check' do let(:key_id) { key.id } - let(:otp) { '123456'} + let(:otp) { '123456' } subject do post api('/internal/two_factor_manual_otp_check'), @@ -1534,7 +1563,7 @@ RSpec.describe API::Internal::Base do describe 'POST /internal/two_factor_push_otp_check' do let(:key_id) { key.id } - let(:otp) { '123456'} + let(:otp) { '123456' } subject do post api('/internal/two_factor_push_otp_check'), diff --git a/spec/requests/api/internal/error_tracking_spec.rb b/spec/requests/api/internal/error_tracking_spec.rb index 69eb54d5ed2..4c420eb8505 100644 --- a/spec/requests/api/internal/error_tracking_spec.rb +++ b/spec/requests/api/internal/error_tracking_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe API::Internal::ErrorTracking do let(:secret_token) { Gitlab::CurrentSettings.error_tracking_access_token } let(:headers) do - { ::API::Internal::ErrorTracking::GITLAB_ERROR_TRACKING_TOKEN_HEADER => Base64.encode64(secret_token) } + { ::API::Internal::ErrorTracking::GITLAB_ERROR_TRACKING_TOKEN_HEADER => secret_token } end describe 'GET /internal/error_tracking/allowed' do diff --git a/spec/requests/api/internal/kubernetes_spec.rb b/spec/requests/api/internal/kubernetes_spec.rb index c0a979995c9..67d8a18dfd8 100644 --- a/spec/requests/api/internal/kubernetes_spec.rb +++ b/spec/requests/api/internal/kubernetes_spec.rb @@ -59,7 +59,7 @@ RSpec.describe API::Internal::Kubernetes do end end - describe 'POST /internal/kubernetes/usage_metrics' do + describe 'POST /internal/kubernetes/usage_metrics', :clean_gitlab_redis_shared_state do def send_request(headers: {}, params: {}) post api('/internal/kubernetes/usage_metrics'), params: params, headers: headers.reverse_merge(jwt_auth_headers) end @@ -69,29 +69,102 @@ RSpec.describe API::Internal::Kubernetes do context 'is authenticated for an agent' do let!(:agent_token) { create(:cluster_agent_token) } + # Todo: Remove gitops_sync_count and k8s_api_proxy_request_count in the next milestone + # https://gitlab.com/gitlab-org/gitlab/-/issues/369489 + # We're only keeping it for backwards compatibility until KAS is released + # using `counts:` instead + context 'deprecated events' do + it 'returns no_content for valid events' do + send_request(params: { gitops_sync_count: 10, k8s_api_proxy_request_count: 5 }) + + expect(response).to have_gitlab_http_status(:no_content) + end + + it 'returns no_content for counts of zero' do + send_request(params: { gitops_sync_count: 0, k8s_api_proxy_request_count: 0 }) + + expect(response).to have_gitlab_http_status(:no_content) + end + + it 'returns 400 for non number' do + send_request(params: { gitops_sync_count: 'string', k8s_api_proxy_request_count: 1 }) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns 400 for negative number' do + send_request(params: { gitops_sync_count: -1, k8s_api_proxy_request_count: 1 }) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'tracks events' do + counters = { gitops_sync_count: 10, k8s_api_proxy_request_count: 5 } + expected_counters = { + kubernetes_agent_gitops_sync: counters[:gitops_sync_count], + kubernetes_agent_k8s_api_proxy_request: counters[:k8s_api_proxy_request_count] + } + + send_request(params: counters) + + expect(Gitlab::UsageDataCounters::KubernetesAgentCounter.totals).to eq(expected_counters) + end + end + it 'returns no_content for valid events' do - send_request(params: { gitops_sync_count: 10, k8s_api_proxy_request_count: 5 }) + counters = { gitops_sync: 10, k8s_api_proxy_request: 5 } + unique_counters = { agent_users_using_ci_tunnel: [10] } + + send_request(params: { counters: counters, unique_counters: unique_counters }) expect(response).to have_gitlab_http_status(:no_content) end it 'returns no_content for counts of zero' do - send_request(params: { gitops_sync_count: 0, k8s_api_proxy_request_count: 0 }) + counters = { gitops_sync: 0, k8s_api_proxy_request: 0 } + unique_counters = { agent_users_using_ci_tunnel: [] } + + send_request(params: { counters: counters, unique_counters: unique_counters }) expect(response).to have_gitlab_http_status(:no_content) end - it 'returns 400 for non number' do - send_request(params: { gitops_sync_count: 'string', k8s_api_proxy_request_count: 1 }) + it 'returns 400 for non counter number' do + counters = { gitops_sync: 'string', k8s_api_proxy_request: 0 } + + send_request(params: { counters: counters }) expect(response).to have_gitlab_http_status(:bad_request) end - it 'returns 400 for negative number' do - send_request(params: { gitops_sync_count: -1, k8s_api_proxy_request_count: 1 }) + it 'returns 400 for non unique_counter set' do + unique_counters = { agent_users_using_ci_tunnel: 1 } + + send_request(params: { unique_counters: unique_counters }) expect(response).to have_gitlab_http_status(:bad_request) end + + it 'tracks events' do + counters = { gitops_sync: 10, k8s_api_proxy_request: 5 } + unique_counters = { agent_users_using_ci_tunnel: [10] } + expected_counters = { + kubernetes_agent_gitops_sync: counters[:gitops_sync], + kubernetes_agent_k8s_api_proxy_request: counters[:k8s_api_proxy_request] + } + + send_request(params: { counters: counters, unique_counters: unique_counters }) + + expect(Gitlab::UsageDataCounters::KubernetesAgentCounter.totals).to eq(expected_counters) + + expect( + Gitlab::UsageDataCounters::HLLRedisCounter + .unique_events( + event_names: 'agent_users_using_ci_tunnel', + start_date: Date.current, end_date: Date.current + 10 + ) + ).to eq(1) + end end end @@ -180,4 +253,95 @@ RSpec.describe API::Internal::Kubernetes do end end end + + describe 'GET /internal/kubernetes/project_info' do + def send_request(headers: {}, params: {}) + get api('/internal/kubernetes/project_info'), params: params, headers: headers.reverse_merge(jwt_auth_headers) + end + + include_examples 'authorization' + include_examples 'agent authentication' + + context 'an agent is found' do + let_it_be(:agent_token) { create(:cluster_agent_token) } + + shared_examples 'agent token tracking' + + context 'project is public' do + let(:project) { create(:project, :public) } + + it 'returns expected data', :aggregate_failures do + send_request(params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:success) + + expect(json_response).to match( + a_hash_including( + 'project_id' => project.id, + 'gitaly_info' => a_hash_including( + 'address' => match(/\.socket$/), + 'token' => 'secret', + 'features' => {} + ), + 'gitaly_repository' => a_hash_including( + 'storage_name' => project.repository_storage, + 'relative_path' => project.disk_path + '.git', + 'gl_repository' => "project-#{project.id}", + 'gl_project_path' => project.full_path + ), + 'default_branch' => project.default_branch_or_main + ) + ) + end + + context 'repository is for project members only' do + let(:project) { create(:project, :public, :repository_private) } + + it 'returns 404' do + send_request(params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'project is private' do + let(:project) { create(:project, :private) } + + it 'returns 404' do + send_request(params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'and agent belongs to project' do + let(:agent_token) { create(:cluster_agent_token, agent: create(:cluster_agent, project: project)) } + + it 'returns 200' do + send_request(params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:success) + end + end + end + + context 'project is internal' do + let(:project) { create(:project, :internal) } + + it 'returns 404' do + send_request(params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'project does not exist' do + it 'returns 404' do + send_request(params: { id: non_existing_record_id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end end diff --git a/spec/requests/api/internal/workhorse_spec.rb b/spec/requests/api/internal/workhorse_spec.rb index d40c14cc0fd..bcf63bf7c2f 100644 --- a/spec/requests/api/internal/workhorse_spec.rb +++ b/spec/requests/api/internal/workhorse_spec.rb @@ -32,6 +32,7 @@ RSpec.describe API::Internal::Workhorse, :allow_forgery_protection do end it { expect_status(:success) } + it 'returns the temp upload path' do subject expect(json_response['TempPath']).to eq(Rails.root.join('tmp/tests/public/uploads/tmp').to_s) diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb index cb351635081..a795b49c44e 100644 --- a/spec/requests/api/invitations_spec.rb +++ b/spec/requests/api/invitations_spec.rb @@ -447,7 +447,7 @@ RSpec.describe API::Invitations do emails = 'email3@example.com,email4@example.com,email5@example.com,email6@example.com,email7@example.com' - unresolved_n_plus_ones = 32 # currently there are 8 queries added per email + unresolved_n_plus_ones = 36 # currently there are 9 queries added per email expect do post invitations_url(group, maintainer), params: { email: emails, access_level: Member::DEVELOPER } diff --git a/spec/requests/api/issue_links_spec.rb b/spec/requests/api/issue_links_spec.rb index 90238c8bf76..98f72f22cdc 100644 --- a/spec/requests/api/issue_links_spec.rb +++ b/spec/requests/api/issue_links_spec.rb @@ -162,12 +162,29 @@ RSpec.describe API::IssueLinks do end context 'when unauthenticated' do - it 'returns 401' do - issue_link = create(:issue_link) + context 'when accessing an issue of a private project' do + it 'returns 401' do + issue_link = create(:issue_link) - perform_request(issue_link.id) + perform_request(issue_link.id) - expect(response).to have_gitlab_http_status(:unauthorized) + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + # This isn't ideal, see https://gitlab.com/gitlab-org/gitlab/-/issues/364077 + context 'when accessing an issue of a public project' do + let(:project) { create(:project, :public) } + let(:issue) { create(:issue, project: project) } + let(:public_issue) { create(:issue, project: project) } + + it 'returns 401' do + issue_link = create(:issue_link, source: issue, target: public_issue) + + perform_request(issue_link.id) + + expect(response).to have_gitlab_http_status(:unauthorized) + end end end diff --git a/spec/requests/api/issues/get_project_issues_spec.rb b/spec/requests/api/issues/get_project_issues_spec.rb index 346f8975835..ec6cc060c83 100644 --- a/spec/requests/api/issues/get_project_issues_spec.rb +++ b/spec/requests/api/issues/get_project_issues_spec.rb @@ -9,6 +9,8 @@ RSpec.describe API::Issues do create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace, merge_requests_access_level: ProjectFeature::PRIVATE) end + let_it_be(:group) { create(:group, :public) } + let(:user2) { create(:user) } let(:non_member) { create(:user) } let_it_be(:guest) { create(:user) } @@ -85,6 +87,8 @@ RSpec.describe API::Issues do end before_all do + group.add_reporter(user) + group.add_guest(guest) project.add_reporter(user) project.add_guest(guest) private_mrs_project.add_reporter(user) @@ -107,6 +111,22 @@ RSpec.describe API::Issues do end end + shared_examples 'returns project issues without confidential issues for guests' do + specify do + get api(api_url, guest) + + expect_paginated_array_response_contain_exactly(open_issue.id, closed_issue.id) + end + end + + shared_examples 'returns all project issues for reporters' do + specify do + get api(api_url, user) + + expect_paginated_array_response_contain_exactly(open_issue.id, confidential_issue.id, closed_issue.id) + end + end + describe "GET /projects/:id/issues" do let(:base_url) { "/projects/#{project.id}" } @@ -183,6 +203,30 @@ RSpec.describe API::Issues do end end + context 'when user is an inherited member from the group' do + let!(:open_issue) { create(:issue, project: group_project) } + let!(:confidential_issue) { create(:issue, :confidential, project: group_project) } + let!(:closed_issue) { create(:issue, state: :closed, project: group_project) } + + let!(:api_url) { "/projects/#{group_project.id}/issues" } + + context 'and group project is public and issues are private' do + let_it_be(:group_project) do + create(:project, :public, issues_access_level: ProjectFeature::PRIVATE, group: group) + end + + it_behaves_like 'returns project issues without confidential issues for guests' + it_behaves_like 'returns all project issues for reporters' + end + + context 'and group project is private' do + let_it_be(:group_project) { create(:project, :private, group: group) } + + it_behaves_like 'returns project issues without confidential issues for guests' + it_behaves_like 'returns all project issues for reporters' + end + end + it 'avoids N+1 queries' do get api("/projects/#{project.id}/issues", user) diff --git a/spec/requests/api/markdown_spec.rb b/spec/requests/api/markdown_spec.rb index 47e1f007daa..3e702b05bc9 100644 --- a/spec/requests/api/markdown_spec.rb +++ b/spec/requests/api/markdown_spec.rb @@ -5,9 +5,11 @@ require "spec_helper" RSpec.describe API::Markdown do describe "POST /markdown" do let(:user) {} # No-op. It gets overwritten in the contexts below. + let(:disable_authenticate_markdown_api) { false } before do stub_commonmark_sourcepos_disabled + stub_feature_flags(authenticate_markdown_api: false) if disable_authenticate_markdown_api post api("/markdown", user), params: params end @@ -21,27 +23,53 @@ RSpec.describe API::Markdown do end end - shared_examples "404 Project Not Found" do - it "responses with 404 Not Found" do + shared_examples '404 Project Not Found' do + it 'responds with 404 Not Found' do expect(response).to have_gitlab_http_status(:not_found) expect(response.headers["Content-Type"]).to eq("application/json") expect(json_response).to be_a(Hash) - expect(json_response["message"]).to eq("404 Project Not Found") + expect(json_response['message']).to eq('404 Project Not Found') end end - context "when arguments are invalid" do - context "when text is missing" do - let(:params) { {} } + shared_examples '400 Bad Request' do + it 'responds with 400 Bad Request' do + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.headers['Content-Type']).to eq('application/json') + expect(json_response).to be_a(Hash) + expect(json_response['error']).to eq('text is missing') + end + end + + context 'when not logged in' do + let(:user) {} + let(:params) { {} } - it "responses with 400 Bad Request" do - expect(response).to have_gitlab_http_status(:bad_request) - expect(response.headers["Content-Type"]).to eq("application/json") + context 'and authenticate_markdown_api turned on' do + it 'responds with 401 Unathorized' do + expect(response).to have_gitlab_http_status(:unauthorized) + expect(response.headers['Content-Type']).to eq('application/json') expect(json_response).to be_a(Hash) - expect(json_response["error"]).to eq("text is missing") + expect(json_response['message']).to eq('401 Unauthorized') end end + context 'and authenticate_markdown_api turned off' do + let(:disable_authenticate_markdown_api) { true } + + it_behaves_like '400 Bad Request' + end + end + + context 'when arguments are invalid' do + let(:user) { create(:user) } + + context 'when text is missing' do + let(:params) { {} } + + it_behaves_like '400 Bad Request' + end + context "when project is not found" do let(:params) { { text: "Hello world!", gfm: true, project: "Dummy project" } } @@ -53,6 +81,7 @@ RSpec.describe API::Markdown do let_it_be(:project) { create(:project) } let_it_be(:issue) { create(:issue, project: project) } + let(:user) { create(:user) } let(:issue_url) { "http://#{Gitlab.config.gitlab.host}/#{issue.project.namespace.path}/#{issue.project.path}/-/issues/#{issue.iid}" } let(:text) { ":tada: Hello world! :100: #{issue.to_reference}" } @@ -131,14 +160,13 @@ RSpec.describe API::Markdown do end context 'when not logged in' do - let(:user) { } + let(:user) {} + let(:disable_authenticate_markdown_api) { true } it_behaves_like 'user without proper access' end context 'when logged in as user without access' do - let(:user) { create(:user) } - it_behaves_like 'user without proper access' end @@ -175,8 +203,9 @@ RSpec.describe API::Markdown do end end - context 'when not logged in' do - let(:user) { } + context 'when not logged in and authenticate_markdown_api turned off' do + let(:user) {} + let(:disable_authenticate_markdown_api) { true } it_behaves_like 'user without proper access' end diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index ba82d2facc6..1b378788b6a 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -34,7 +34,7 @@ RSpec.describe API::MavenPackages do end let(:version) { '1.0-SNAPSHOT' } - let(:param_path) { "#{package_name}/#{version}"} + let(:param_path) { "#{package_name}/#{version}" } before do project.add_developer(user) @@ -1000,20 +1000,45 @@ RSpec.describe API::MavenPackages do context 'for sha1 file' do let(:dummy_package) { double(Packages::Package) } + let(:file_upload) { fixture_file_upload('spec/fixtures/packages/maven/my-app-1.0-20180724.124855-1.pom.sha1') } + let(:stored_sha1) { File.read(file_upload.path) } - it 'checks the sha1' do + subject(:upload) { upload_file_with_token(params: params, file_extension: 'pom.sha1') } + + before do # The sha verification done by the maven api is between: # - the sha256 set by workhorse helpers # - the sha256 of the sha1 of the uploaded package file # We're going to send `file_upload` for the sha1 and stub the sha1 of the package file so that # both sha256 being the same - expect(::Packages::PackageFileFinder).to receive(:new).and_return(double(execute!: dummy_package)) - expect(dummy_package).to receive(:file_sha1).and_return(File.read(file_upload.path)) + allow(::Packages::PackageFileFinder).to receive(:new).and_return(double(execute!: dummy_package)) + allow(dummy_package).to receive(:file_sha1).and_return(stored_sha1) + end - upload_file_with_token(params: params, file_extension: 'jar.sha1') + it 'returns no content' do + upload expect(response).to have_gitlab_http_status(:no_content) end + + context 'when the stored sha1 is not the same' do + let(:sent_sha1) { File.read(file_upload.path) } + let(:stored_sha1) { 'wrong sha1' } + + it 'logs an error and returns conflict' do + expect(Gitlab::ErrorTracking).to receive(:log_exception).with( + instance_of(ArgumentError), + message: 'maven package file sha1 conflict', + stored_sha1: stored_sha1, + received_sha256: Digest::SHA256.hexdigest(sent_sha1), + sha256_hexdigest_of_stored_sha1: Digest::SHA256.hexdigest(stored_sha1) + ) + + upload + + expect(response).to have_gitlab_http_status(:conflict) + end + end end context 'for md5 file' do diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index e4c2f17af47..9df9c75b020 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -3,14 +3,14 @@ require 'spec_helper' RSpec.describe API::Members do - let(:maintainer) { create(:user, username: 'maintainer_user') } - let(:maintainer2) { create(:user, username: 'user-with-maintainer-role') } - let(:developer) { create(:user) } - let(:access_requester) { create(:user) } - let(:stranger) { create(:user) } - let(:user_with_minimal_access) { create(:user) } - - let(:project) do + let_it_be(:maintainer) { create(:user, username: 'maintainer_user') } + let_it_be(:maintainer2) { create(:user, username: 'user-with-maintainer-role') } + let_it_be(:developer) { create(:user) } + let_it_be(:access_requester) { create(:user) } + let_it_be(:stranger) { create(:user) } + let_it_be(:user_with_minimal_access) { create(:user) } + + let_it_be(:project, refind: true) do create(:project, :public, creator_id: maintainer.id, group: create(:group, :public)) do |project| project.add_maintainer(maintainer) project.add_developer(developer, current_user: maintainer) @@ -18,7 +18,7 @@ RSpec.describe API::Members do end end - let!(:group) do + let_it_be(:group, refind: true) do create(:group, :public) do |group| group.add_owner(maintainer) group.add_developer(developer, maintainer) @@ -187,8 +187,8 @@ RSpec.describe API::Members do end context 'with a subgroup' do - let(:group) { create(:group, :private)} - let(:subgroup) { create(:group, :private, parent: group)} + let(:group) { create(:group, :private) } + let(:subgroup) { create(:group, :private, parent: group) } let(:project) { create(:project, group: subgroup) } before do @@ -231,6 +231,33 @@ RSpec.describe API::Members do end end end + + context 'with ancestral membership' do + shared_examples 'response with correct access levels' do + it do + get api("/#{source_type.pluralize}/#{source.id}/members/#{all ? 'all/' : ''}#{developer.id}", developer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['access_level']).to eq(Member::MAINTAINER) + end + end + + before do + source.add_maintainer(developer) + end + + include_examples 'response with correct access levels' + + context 'having email invite' do + before do + Member + .find_by(source: group, user: developer) + .update!(invite_email: 'email@email.com') + end + + include_examples 'response with correct access levels' + end + end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 695c0ed1749..2a03ae89389 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -88,7 +88,7 @@ RSpec.describe API::MergeRequests do context 'with merge status recheck projection' do it 'checks mergeability asynchronously' do - expect_next_instance_of(check_service_class) do |service| + expect_next_instances_of(check_service_class, (1..2)) do |service| expect(service).not_to receive(:execute) expect(service).to receive(:async_execute).and_call_original end @@ -595,6 +595,22 @@ RSpec.describe API::MergeRequests do end end + RSpec.shared_examples 'a non-cached MergeRequest api request' do |call_count| + it 'serializes merge request' do + expect(API::Entities::MergeRequestBasic).to receive(:represent).exactly(call_count).times.and_call_original + + get api(endpoint_path) + end + end + + RSpec.shared_examples 'a cached MergeRequest api request' do + it 'serializes merge request' do + expect(API::Entities::MergeRequestBasic).not_to receive(:represent) + + get api(endpoint_path) + end + end + describe 'route shadowing' do include GrapePathHelpers::NamedRouteMatcher @@ -979,13 +995,43 @@ RSpec.describe API::MergeRequests do end end - describe "GET /projects/:id/merge_requests" do + describe "GET /projects/:id/merge_requests", :use_clean_rails_memory_store_caching do include_context 'with merge requests' let(:endpoint_path) { "/projects/#{project.id}/merge_requests" } it_behaves_like 'merge requests list' + context 'caching' do + let(:params) { {} } + + before do + get api(endpoint_path) + end + + context 'when it is cached' do + it_behaves_like 'a cached MergeRequest api request' + end + + context 'when it is not cached' do + context 'when the status changes' do + before do + merge_request.mark_as_unchecked! + end + + it_behaves_like 'a non-cached MergeRequest api request', 1 + end + + context 'when another user requests' do + before do + sign_in(user2) + end + + it_behaves_like 'a non-cached MergeRequest api request', 4 + end + end + end + it "returns 404 for non public projects" do project = create(:project, :private) @@ -1466,6 +1512,45 @@ RSpec.describe API::MergeRequests do end end + describe 'GET /projects/:id/merge_requests/:merge_request_iid/reviewers' do + it 'returns reviewers' do + reviewer = create(:user) + merge_request.merge_request_reviewers.create!(reviewer: reviewer) + + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/reviewers", user) + + 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(merge_request.merge_request_reviewers.size) + + expect(json_response.last['user']['id']).to eq(reviewer.id) + expect(json_response.last['user']['name']).to eq(reviewer.name) + expect(json_response.last['user']['username']).to eq(reviewer.username) + expect(json_response.last['state']).to eq('unreviewed') + expect(json_response.last['updated_state_by']).to be_nil + expect(json_response.last['created_at']).to be_present + end + + it 'returns a 404 when iid does not exist' do + get api("/projects/#{project.id}/merge_requests/#{non_existing_record_iid}/reviewers", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns a 404 when id is used instead of iid' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/reviewers", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when merge request author has only guest access' do + it_behaves_like 'rejects user from accessing merge request info' do + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/reviewers" } + end + end + end + describe 'GET /projects/:id/merge_requests/:merge_request_iid/commits' do include_context 'with merge requests' @@ -2482,39 +2567,37 @@ RSpec.describe API::MergeRequests do let(:pipeline) { create(:ci_pipeline, project: project) } it "returns merge_request in case of success" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) + expect { put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) } + .to change { merge_request.reload.merged? } + .from(false) + .to(true) expect(response).to have_gitlab_http_status(:ok) end - context 'when change_response_code_merge_status is enabled' do - it "returns 422 if branch can't be merged" do - allow_next_found_instance_of(MergeRequest) do |merge_request| - allow(merge_request).to receive(:can_be_merged?).and_return(false) + context 'when the merge request fails to merge' do + it 'returns 422' do + expect_next_instance_of(::MergeRequests::MergeService) do |service| + expect(service).to receive(:execute) end - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) + expect { put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) } + .not_to change { merge_request.reload.merged? } expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Branch cannot be merged') + expect(json_response['message']).to eq("Branch cannot be merged") end end - context 'when change_response_code_merge_status is disabled' do - before do - stub_feature_flags(change_response_code_merge_status: false) + it "returns 422 if branch can't be merged" do + allow_next_found_instance_of(MergeRequest) do |merge_request| + allow(merge_request).to receive(:can_be_merged?).and_return(false) end - it "returns 406 if branch can't be merged" do - allow_next_found_instance_of(MergeRequest) do |merge_request| - allow(merge_request).to receive(:can_be_merged?).and_return(false) - end - - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) - expect(response).to have_gitlab_http_status(:not_acceptable) - expect(json_response['message']).to eq('Branch cannot be merged') - end + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Branch cannot be merged') end it "returns 405 if merge_request is not open" do diff --git a/spec/requests/api/metrics/dashboard/annotations_spec.rb b/spec/requests/api/metrics/dashboard/annotations_spec.rb index 79a38702354..5e64ac7d481 100644 --- a/spec/requests/api/metrics/dashboard/annotations_spec.rb +++ b/spec/requests/api/metrics/dashboard/annotations_spec.rb @@ -10,7 +10,7 @@ RSpec.describe API::Metrics::Dashboard::Annotations do let(:dashboard) { 'config/prometheus/common_metrics.yml' } let(:starting_at) { Time.now.iso8601 } let(:ending_at) { 1.hour.from_now.iso8601 } - let(:params) { attributes_for(:metrics_dashboard_annotation, environment: environment, starting_at: starting_at, ending_at: ending_at, dashboard_path: dashboard)} + let(:params) { attributes_for(:metrics_dashboard_annotation, environment: environment, starting_at: starting_at, ending_at: ending_at, dashboard_path: dashboard) } shared_examples 'POST /:source_type/:id/metrics_dashboard/annotations' do |source_type| let(:url) { "/#{source_type.pluralize}/#{source.id}/metrics_dashboard/annotations" } diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index f6a65274ca2..89abb28140a 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -111,7 +111,7 @@ RSpec.describe API::Notes do system: false end - let(:test_url) {"/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes"} + let(:test_url) { "/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes" } shared_examples 'a notes request' do it 'is a note array response' do diff --git a/spec/requests/api/npm_project_packages_spec.rb b/spec/requests/api/npm_project_packages_spec.rb index 62809b432af..3bcffac2760 100644 --- a/spec/requests/api/npm_project_packages_spec.rb +++ b/spec/requests/api/npm_project_packages_spec.rb @@ -59,7 +59,7 @@ RSpec.describe API::NpmProjectPackages do end context 'with access token' do - let(:headers) { build_token_auth_header(token.token) } + let(:headers) { build_token_auth_header(token.plaintext_token) } it_behaves_like 'successfully downloads the file' it_behaves_like 'a package tracking event', 'API::NpmPackages', 'pull_package' @@ -95,7 +95,7 @@ RSpec.describe API::NpmProjectPackages do it_behaves_like 'a package file that requires auth' context 'with guest' do - let(:headers) { build_token_auth_header(token.token) } + let(:headers) { build_token_auth_header(token.plaintext_token) } it 'denies download when not enough permissions' do project.add_guest(user) @@ -307,8 +307,8 @@ RSpec.describe API::NpmProjectPackages do expect { upload_package_with_token } .to change { project.packages.count }.by(1) .and change { Packages::PackageFile.count }.by(1) - .and change { Packages::Dependency.count}.by(4) - .and change { Packages::DependencyLink.count}.by(6) + .and change { Packages::Dependency.count }.by(4) + .and change { Packages::DependencyLink.count }.by(6) expect(response).to have_gitlab_http_status(:ok) end @@ -323,8 +323,8 @@ RSpec.describe API::NpmProjectPackages do expect { upload_package_with_token } .to change { project.packages.count }.by(1) .and change { Packages::PackageFile.count }.by(1) - .and not_change { Packages::Dependency.count} - .and change { Packages::DependencyLink.count}.by(6) + .and not_change { Packages::Dependency.count } + .and change { Packages::DependencyLink.count }.by(6) end end end @@ -356,7 +356,7 @@ RSpec.describe API::NpmProjectPackages do end def upload_with_token(package_name, params = {}) - upload_package(package_name, params.merge(access_token: token.token)) + upload_package(package_name, params.merge(access_token: token.plaintext_token)) end def upload_with_job_token(package_name, params = {}) diff --git a/spec/requests/api/nuget_group_packages_spec.rb b/spec/requests/api/nuget_group_packages_spec.rb index 1b71f0f9de1..c1375288809 100644 --- a/spec/requests/api/nuget_group_packages_spec.rb +++ b/spec/requests/api/nuget_group_packages_spec.rb @@ -73,7 +73,7 @@ RSpec.describe API::NugetGroupPackages do let(:include_prereleases) { true } let(:query_parameters) { { q: search_term, take: take, skip: skip, prerelease: include_prereleases }.compact } - subject { get api(url), headers: {}} + subject { get api(url), headers: {} } shared_examples 'handling mixed visibilities' do where(:group_visibility, :subgroup_visibility, :expected_status) do diff --git a/spec/requests/api/pages/pages_spec.rb b/spec/requests/api/pages/pages_spec.rb index 0eb2ae64f43..7d44ff533aa 100644 --- a/spec/requests/api/pages/pages_spec.rb +++ b/spec/requests/api/pages/pages_spec.rb @@ -19,7 +19,7 @@ RSpec.describe API::Pages do end it_behaves_like '404 response' do - let(:request) { delete api("/projects/#{project.id}/pages", admin)} + let(:request) { delete api("/projects/#{project.id}/pages", admin) } end end diff --git a/spec/requests/api/pages_domains_spec.rb b/spec/requests/api/pages_domains_spec.rb index 75183156c9d..cd4e8b30d8f 100644 --- a/spec/requests/api/pages_domains_spec.rb +++ b/spec/requests/api/pages_domains_spec.rb @@ -19,8 +19,8 @@ RSpec.describe API::PagesDomains do end let(:pages_domain_secure_params) { build(:pages_domain, domain: 'ssl.other-domain.test', project: project).slice(:domain, :certificate, :key) } - let(:pages_domain_secure_key_missmatch_params) {build(:pages_domain, :with_trusted_chain, project: project).slice(:domain, :certificate, :key) } - let(:pages_domain_secure_missing_chain_params) {build(:pages_domain, :with_missing_chain, project: project).slice(:certificate) } + let(:pages_domain_secure_key_missmatch_params) { build(:pages_domain, :with_trusted_chain, project: project).slice(:domain, :certificate, :key) } + let(:pages_domain_secure_missing_chain_params) { build(:pages_domain, :with_missing_chain, project: project).slice(:certificate) } let(:route) { "/projects/#{project.id}/pages/domains" } let(:route_domain) { "/projects/#{project.id}/pages/domains/#{pages_domain.domain}" } diff --git a/spec/requests/api/personal_access_tokens_spec.rb b/spec/requests/api/personal_access_tokens_spec.rb index 403c646ee32..8d8998cfdd6 100644 --- a/spec/requests/api/personal_access_tokens_spec.rb +++ b/spec/requests/api/personal_access_tokens_spec.rb @@ -34,7 +34,7 @@ RSpec.describe API::PersonalAccessTokens do context 'logged in as a non-Administrator' do let_it_be(:current_user) { create(:user) } let_it_be(:user) { create(:user) } - let_it_be(:token) { create(:personal_access_token, user: current_user)} + let_it_be(:token) { create(:personal_access_token, user: current_user) } let_it_be(:other_token) { create(:personal_access_token, user: user) } let_it_be(:token_impersonated) { create(:personal_access_token, impersonation: true, user: current_user) } @@ -100,7 +100,7 @@ RSpec.describe API::PersonalAccessTokens do it 'fails to return PAT because no PAT exists with this id' do get api(invalid_path, admin_user) - expect(response).to have_gitlab_http_status(:unauthorized) + expect(response).to have_gitlab_http_status(:not_found) end end diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index 8d3622ca17d..670035187cb 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -101,6 +101,7 @@ ci_cd_settings: job_token_scope_enabled: ci_job_token_scope_enabled separated_caches: ci_separated_caches opt_in_jwt: ci_opt_in_jwt + allow_fork_pipelines_to_run_in_parent_project: ci_allow_fork_pipelines_to_run_in_parent_project build_import_state: # import_state unexposed_attributes: @@ -157,6 +158,7 @@ project_setting: - cve_id_request_enabled - mr_default_target_self - target_platforms + - selective_code_owner_removals build_service_desk_setting: # service_desk_setting unexposed_attributes: diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index 8655e5b0238..afe5a7d4a21 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -47,7 +47,7 @@ RSpec.describe API::ProjectImport, :aggregate_failures do it 'executes a limited number of queries' do control_count = ActiveRecord::QueryRecorder.new { subject }.count - expect(control_count).to be <= 108 + expect(control_count).to be <= 109 end it 'schedules an import using a namespace' do diff --git a/spec/requests/api/project_packages_spec.rb b/spec/requests/api/project_packages_spec.rb index 5f4b8899a33..7a05da8e13f 100644 --- a/spec/requests/api/project_packages_spec.rb +++ b/spec/requests/api/project_packages_spec.rb @@ -86,6 +86,18 @@ RSpec.describe API::ProjectPackages do expect(json_response).to include(a_hash_including('_links' => a_hash_including('web_path' => include(nested_project.namespace.full_path)))) end end + + context 'with JOB-TOKEN auth' do + let(:job) { create(:ci_build, :running, user: user) } + + subject { get api(url, job_token: job.token) } + + it_behaves_like 'returns packages', :project, :maintainer + it_behaves_like 'returns packages', :project, :developer + it_behaves_like 'returns packages', :project, :reporter + it_behaves_like 'returns packages', :project, :no_type + it_behaves_like 'returns packages', :project, :guest + end end context 'project is private' do @@ -116,6 +128,19 @@ RSpec.describe API::ProjectPackages do end end end + + context 'with JOB-TOKEN auth' do + let(:job) { create(:ci_build, :running, user: user) } + + subject { get api(url, job_token: job.token) } + + it_behaves_like 'returns packages', :project, :maintainer + it_behaves_like 'returns packages', :project, :developer + it_behaves_like 'returns packages', :project, :reporter + it_behaves_like 'rejects packages access', :project, :no_type, :not_found + # TODO uncomment when https://gitlab.com/gitlab-org/gitlab/-/issues/370998 is resolved + # it_behaves_like 'rejects packages access', :project, :guest, :not_found + end end context 'with pagination params' do @@ -177,6 +202,8 @@ RSpec.describe API::ProjectPackages do end describe 'GET /projects/:id/packages/:package_id' do + let(:single_package_schema) { 'public_api/v4/packages/package' } + subject { get api(package_url, user) } shared_examples 'no destroy url' do @@ -217,7 +244,7 @@ RSpec.describe API::ProjectPackages do subject expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('public_api/v4/packages/package') + expect(response).to match_response_schema(single_package_schema) end it 'returns 404 when the package does not exist' do @@ -233,6 +260,18 @@ RSpec.describe API::ProjectPackages do end it_behaves_like 'no destroy url' + + context 'with JOB-TOKEN auth' do + let(:job) { create(:ci_build, :running, user: user) } + + subject { get api(package_url, job_token: job.token) } + + it_behaves_like 'returns package', :project, :maintainer + it_behaves_like 'returns package', :project, :developer + it_behaves_like 'returns package', :project, :reporter + it_behaves_like 'returns package', :project, :no_type + it_behaves_like 'returns package', :project, :guest + end end context 'project is private' do @@ -259,7 +298,7 @@ RSpec.describe API::ProjectPackages do subject expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('public_api/v4/packages/package') + expect(response).to match_response_schema(single_package_schema) end it_behaves_like 'no destroy url' @@ -273,6 +312,19 @@ RSpec.describe API::ProjectPackages do it_behaves_like 'destroy url' end + context 'with JOB-TOKEN auth' do + let(:job) { create(:ci_build, :running, user: user) } + + subject { get api(package_url, job_token: job.token) } + + it_behaves_like 'returns package', :project, :maintainer + it_behaves_like 'returns package', :project, :developer + it_behaves_like 'returns package', :project, :reporter + # TODO uncomment when https://gitlab.com/gitlab-org/gitlab/-/issues/370998 is resolved + # it_behaves_like 'rejects packages access', :project, :guest, :not_found + it_behaves_like 'rejects packages access', :project, :no_type, :not_found + end + context 'with pipeline' do let!(:package1) { create(:npm_package, :with_build, project: project) } @@ -355,6 +407,26 @@ RSpec.describe API::ProjectPackages do expect(response).to have_gitlab_http_status(:no_content) end + + context 'with JOB-TOKEN auth' do + let(:job) { create(:ci_build, :running, user: user) } + + it 'returns 403 for a user without enough permissions' do + project.add_developer(user) + + expect { delete api(package_url, job_token: job.token) }.not_to change { ::Packages::Package.pending_destruction.count } + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'returns 204' do + project.add_maintainer(user) + + expect { delete api(package_url, job_token: job.token) }.to change { ::Packages::Package.pending_destruction.count }.by(1) + + expect(response).to have_gitlab_http_status(:no_content) + end + end end context 'with a maven package' do diff --git a/spec/requests/api/project_templates_spec.rb b/spec/requests/api/project_templates_spec.rb index 070fd6db3dc..87d70a87f42 100644 --- a/spec/requests/api/project_templates_spec.rb +++ b/spec/requests/api/project_templates_spec.rb @@ -77,7 +77,7 @@ RSpec.describe API::ProjectTemplates do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(response).to match_response_schema('public_api/v4/template_list') - expect(json_response.map {|t| t['key']}).to match_array(%w(bug feature_proposal template_test)) + expect(json_response.map { |t| t['key'] }).to match_array(%w(bug feature_proposal template_test)) end it 'returns merge request templates' do @@ -86,7 +86,7 @@ RSpec.describe API::ProjectTemplates do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(response).to match_response_schema('public_api/v4/template_list') - expect(json_response.map {|t| t['key']}).to match_array(%w(bug feature_proposal template_test)) + expect(json_response.map { |t| t['key'] }).to match_array(%w(bug feature_proposal template_test)) end it 'returns 400 for an unknown template type' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index ae689d7327b..94688833d88 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1093,7 +1093,7 @@ RSpec.describe API::Projects do it 'does not create new project and respond with 403' do allow_any_instance_of(User).to receive(:projects_limit_left).and_return(0) expect { post api('/projects', user2), params: { name: 'foo' } } - .to change {Project.count}.by(0) + .to change { Project.count }.by(0) expect(response).to have_gitlab_http_status(:forbidden) end end @@ -2427,6 +2427,7 @@ RSpec.describe API::Projects do expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved) expect(json_response['ci_default_git_depth']).to eq(project.ci_default_git_depth) expect(json_response['ci_forward_deployment_enabled']).to eq(project.ci_forward_deployment_enabled) + expect(json_response['ci_allow_fork_pipelines_to_run_in_parent_project']).to eq(project.ci_allow_fork_pipelines_to_run_in_parent_project) expect(json_response['ci_separated_caches']).to eq(project.ci_separated_caches) expect(json_response['merge_method']).to eq(project.merge_method.to_s) expect(json_response['squash_option']).to eq(project.squash_option.to_s) @@ -3692,6 +3693,7 @@ RSpec.describe API::Projects do merge_method: 'ff', ci_default_git_depth: 20, ci_forward_deployment_enabled: false, + ci_allow_fork_pipelines_to_run_in_parent_project: false, ci_separated_caches: false, description: 'new description' } diff --git a/spec/requests/api/protected_branches_spec.rb b/spec/requests/api/protected_branches_spec.rb index 8efb822cb83..9f10eb1bb9f 100644 --- a/spec/requests/api/protected_branches_spec.rb +++ b/spec/requests/api/protected_branches_spec.rb @@ -3,14 +3,22 @@ require 'spec_helper' RSpec.describe API::ProtectedBranches do - let(:user) { create(:user) } - let!(:project) { create(:project, :repository) } + let_it_be_with_reload(:project) { create(:project, :repository) } + let_it_be(:maintainer) { create(:user) } + let_it_be(:guest) { create(:user) } + let(:protected_name) { 'feature' } let(:branch_name) { protected_name } + let!(:protected_branch) do create(:protected_branch, project: project, name: protected_name) end + before_all do + project.add_maintainer(maintainer) + project.add_guest(guest) + end + describe "GET /projects/:id/protected_branches" do let(:params) { {} } let(:route) { "/projects/#{project.id}/protected_branches" } @@ -29,9 +37,7 @@ RSpec.describe API::ProtectedBranches do end context 'when authenticated as a maintainer' do - before do - project.add_maintainer(user) - end + let(:user) { maintainer } context 'when search param is not present' do it_behaves_like 'protected branches' do @@ -49,9 +55,7 @@ RSpec.describe API::ProtectedBranches do end context 'when authenticated as a guest' do - before do - project.add_guest(user) - end + let(:user) { guest } it_behaves_like '403 response' do let(:request) { get api(route, user) } @@ -84,9 +88,7 @@ RSpec.describe API::ProtectedBranches do end context 'when authenticated as a maintainer' do - before do - project.add_maintainer(user) - end + let(:user) { maintainer } it_behaves_like 'protected branch' @@ -104,9 +106,7 @@ RSpec.describe API::ProtectedBranches do end context 'when authenticated as a guest' do - before do - project.add_guest(user) - end + let(:user) { guest } it_behaves_like '403 response' do let(:request) { get api(route, user) } @@ -124,9 +124,7 @@ RSpec.describe API::ProtectedBranches do end context 'when authenticated as a maintainer' do - before do - project.add_maintainer(user) - end + let(:user) { maintainer } it 'protects a single branch' do post post_endpoint, params: { name: branch_name } @@ -226,13 +224,10 @@ RSpec.describe API::ProtectedBranches do end end - context 'when a policy restricts rule deletion' do - before do - policy = instance_double(ProtectedBranchPolicy, allowed?: false) - expect(ProtectedBranchPolicy).to receive(:new).and_return(policy) - end + context 'when a policy restricts rule creation' do + it "prevents creations of the protected branch rule" do + disallow(:create_protected_branch, an_instance_of(ProtectedBranch)) - it "prevents deletion of the protected branch rule" do post post_endpoint, params: { name: branch_name } expect(response).to have_gitlab_http_status(:forbidden) @@ -241,9 +236,7 @@ RSpec.describe API::ProtectedBranches do end context 'when authenticated as a guest' do - before do - project.add_guest(user) - end + let(:user) { guest } it "returns a 403 error if guest" do post post_endpoint, params: { name: branch_name } @@ -254,12 +247,9 @@ RSpec.describe API::ProtectedBranches do end describe "DELETE /projects/:id/protected_branches/unprotect/:branch" do + let(:user) { maintainer } let(:delete_endpoint) { api("/projects/#{project.id}/protected_branches/#{branch_name}", user) } - before do - project.add_maintainer(user) - end - it "unprotects a single branch" do delete delete_endpoint @@ -277,12 +267,9 @@ RSpec.describe API::ProtectedBranches do end context 'when a policy restricts rule deletion' do - before do - policy = instance_double(ProtectedBranchPolicy, allowed?: false) - expect(ProtectedBranchPolicy).to receive(:new).and_return(policy) - end - it "prevents deletion of the protected branch rule" do + disallow(:destroy_protected_branch, protected_branch) + delete delete_endpoint expect(response).to have_gitlab_http_status(:forbidden) @@ -299,4 +286,9 @@ RSpec.describe API::ProtectedBranches do end end end + + def disallow(ability, protected_branch) + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, ability, protected_branch).and_return(false) + end end diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index 9e0d3780fd8..6c130bb4963 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -41,7 +41,7 @@ RSpec.describe API::PypiPackages do it_behaves_like 'deploy token for package GET requests' context 'with group path as id' do - let(:url) { "/groups/#{CGI.escape(group.full_path)}/-/packages/pypi/simple"} + let(:url) { "/groups/#{CGI.escape(group.full_path)}/-/packages/pypi/simple" } it_behaves_like 'deploy token for package GET requests' end @@ -102,7 +102,7 @@ RSpec.describe API::PypiPackages do it_behaves_like 'deploy token for package GET requests' context 'with group path as id' do - let(:url) { "/groups/#{CGI.escape(group.full_path)}/-/packages/pypi/simple/#{package_name}"} + let(:url) { "/groups/#{CGI.escape(group.full_path)}/-/packages/pypi/simple/#{package_name}" } it_behaves_like 'deploy token for package GET requests' end diff --git a/spec/requests/api/release/links_spec.rb b/spec/requests/api/release/links_spec.rb index 2345c0063dd..57b2e005929 100644 --- a/spec/requests/api/release/links_spec.rb +++ b/spec/requests/api/release/links_spec.rb @@ -66,7 +66,7 @@ RSpec.describe API::Release::Links do end context 'when release does not exist' do - let!(:release) { } + let!(:release) {} it_behaves_like '404 response' do let(:request) { get api("/projects/#{project.id}/releases/v0.1/assets/links", maintainer) } @@ -98,7 +98,7 @@ RSpec.describe API::Release::Links do end context 'when the release does not exists' do - let!(:release) { } + let!(:release) {} it_behaves_like '403 response' do let(:request) { get api("/projects/#{project.id}/releases/v0.1/assets/links", non_project_member) } @@ -409,7 +409,7 @@ RSpec.describe API::Release::Links do end context 'when there are no corresponding release link' do - let!(:release_link) { } + let!(:release_link) {} it_behaves_like '404 response' do let(:request) do @@ -510,7 +510,7 @@ RSpec.describe API::Release::Links do end context 'when there are no corresponding release link' do - let!(:release_link) { } + let!(:release_link) {} it_behaves_like '404 response' do let(:request) do diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index c050214ff50..1d9e3a6c887 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -962,7 +962,7 @@ RSpec.describe API::Releases do context 'with milestones' do let(:subject) { post api("/projects/#{project.id}/releases", maintainer), params: params } let(:milestone) { create(:milestone, project: project, title: 'v1.0') } - let(:returned_milestones) { json_response['milestones'].map {|m| m['title']} } + let(:returned_milestones) { json_response['milestones'].map { |m| m['title'] } } before do params.merge!(milestone_params) @@ -1120,7 +1120,7 @@ RSpec.describe API::Releases do end context 'when there are no corresponding releases' do - let!(:release) { } + let!(:release) {} it 'forbids the request' do put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params @@ -1158,7 +1158,7 @@ RSpec.describe API::Releases do end context 'with milestones' do - let(:returned_milestones) { json_response['milestones'].map {|m| m['title']} } + let(:returned_milestones) { json_response['milestones'].map { |m| m['title'] } } subject { put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params } @@ -1310,7 +1310,7 @@ RSpec.describe API::Releases do end context 'when there are no corresponding releases' do - let!(:release) { } + let!(:release) {} it 'forbids the request' do delete api("/projects/#{project.id}/releases/v0.1", maintainer) diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index cf0165d123f..3c22f918af5 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -92,6 +92,32 @@ RSpec.describe API::Repositories do expect(json_response.map { |t| t["id"] }).not_to include(page_token) end end + + context 'with pagination=none' do + context 'with recursive=1' do + it 'returns unpaginated recursive project paths tree' do + get api("#{route}?recursive=1&pagination=none", current_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(response).not_to include_pagination_headers + expect(json_response[4]['name']).to eq('html') + expect(json_response[4]['path']).to eq('files/html') + expect(json_response[4]['type']).to eq('tree') + expect(json_response[4]['mode']).to eq('040000') + end + end + + context 'with recursive=0' do + it 'returns 400' do + get api("#{route}?recursive=0&pagination=none", current_user) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']) + .to eq('pagination cannot be "none" unless "recursive" is true') + end + end + end end context 'when unauthenticated', 'and project is public' do diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index 4d2a69cd85b..66b78829e0d 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -350,6 +350,17 @@ RSpec.describe API::Search do include_examples 'pagination', scope: :snippet_titles end end + + it 'sets global search information for logging' do + expect(Gitlab::Instrumentation::GlobalSearchApi).to receive(:set_information).with( + type: 'basic', + level: 'global', + scope: 'issues', + search_duration_s: a_kind_of(Numeric) + ) + + get api(endpoint, user), params: { scope: 'issues', search: 'john doe' } + end end it_behaves_like 'rate limited endpoint', rate_limit_key: :search_rate_limit do diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index d4a8e591622..6f0d5827a80 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -46,7 +46,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['spam_check_api_key']).to be_nil expect(json_response['wiki_page_max_content_bytes']).to be_a(Integer) expect(json_response['require_admin_approval_after_user_signup']).to eq(true) - expect(json_response['personal_access_token_prefix']).to be_nil + expect(json_response['personal_access_token_prefix']).to eq('glpat-') expect(json_response['admin_mode']).to be(false) expect(json_response['whats_new_variant']).to eq('all_tiers') expect(json_response['user_deactivation_emails_enabled']).to be(true) diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 0ba1011684a..0dd6e484e8d 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -28,7 +28,7 @@ RSpec.describe API::Snippets, factory_default: :keep do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(json_response.map { |snippet| snippet['id']} ).to contain_exactly( + expect(json_response.map { |snippet| snippet['id'] } ).to contain_exactly( public_snippet.id, internal_snippet.id, private_snippet.id) @@ -75,7 +75,7 @@ RSpec.describe API::Snippets, factory_default: :keep do it 'returns snippets available for user in given time range' do get api(path, personal_access_token: user_token) - expect(json_response.map { |snippet| snippet['id']} ).to contain_exactly( + expect(json_response.map { |snippet| snippet['id'] } ).to contain_exactly( private_snippet_in_time_range1.id, private_snippet_in_time_range2.id) end @@ -99,10 +99,10 @@ RSpec.describe API::Snippets, factory_default: :keep do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(json_response.map { |snippet| snippet['id']} ).to contain_exactly( + expect(json_response.map { |snippet| snippet['id'] } ).to contain_exactly( public_snippet.id, public_snippet_other.id) - expect(json_response.map { |snippet| snippet['web_url']} ).to contain_exactly( + expect(json_response.map { |snippet| snippet['web_url'] } ).to contain_exactly( "http://localhost/-/snippets/#{public_snippet.id}", "http://localhost/-/snippets/#{public_snippet_other.id}") expect(json_response[0]['files'].first).to eq snippet_blob_file(public_snippet_other.blobs.first) @@ -126,7 +126,7 @@ RSpec.describe API::Snippets, factory_default: :keep do it 'returns public snippets available to user in given time range' do get api(path, personal_access_token: user_token) - expect(json_response.map { |snippet| snippet['id']} ).to contain_exactly( + expect(json_response.map { |snippet| snippet['id'] } ).to contain_exactly( public_snippet_in_time_range.id) end end diff --git a/spec/requests/api/topics_spec.rb b/spec/requests/api/topics_spec.rb index e711414a895..72221e3fb6a 100644 --- a/spec/requests/api/topics_spec.rb +++ b/spec/requests/api/topics_spec.rb @@ -36,6 +36,22 @@ RSpec.describe API::Topics do expect(json_response[2]['total_projects_count']).to eq(1) end + context 'with without_projects' do + let_it_be(:topic_4) { create(:topic, name: 'unassigned topic', total_projects_count: 0) } + + it 'returns topics without assigned projects' do + get api('/topics'), params: { without_projects: true } + + expect(json_response.map { |t| t['id'] }).to contain_exactly(topic_4.id) + end + + it 'returns topics without assigned projects' do + get api('/topics'), params: { without_projects: false } + + expect(json_response.map { |t| t['id'] }).to contain_exactly(topic_1.id, topic_2.id, topic_3.id, topic_4.id) + end + end + context 'with search' do using RSpec::Parameterized::TableSyntax diff --git a/spec/requests/api/unleash_spec.rb b/spec/requests/api/unleash_spec.rb index 7bdb89fb286..3ee895d9421 100644 --- a/spec/requests/api/unleash_spec.rb +++ b/spec/requests/api/unleash_spec.rb @@ -8,8 +8,8 @@ RSpec.describe API::Unleash do let_it_be(:project, refind: true) { create(:project) } let(:project_id) { project.id } - let(:params) { } - let(:headers) { } + let(:params) {} + let(:headers) {} shared_examples 'authenticated request' do context 'when using instance id' do @@ -57,7 +57,7 @@ RSpec.describe API::Unleash do context 'when using header' do let(:client) { create(:operations_feature_flags_client, project: project) } - let(:headers) { { "UNLEASH-INSTANCEID" => client.token }} + let(:headers) { { "UNLEASH-INSTANCEID" => client.token } } it 'responds with OK' do subject diff --git a/spec/requests/api/user_counts_spec.rb b/spec/requests/api/user_counts_spec.rb index 2d4705920cf..ab2aa87d1b7 100644 --- a/spec/requests/api/user_counts_spec.rb +++ b/spec/requests/api/user_counts_spec.rb @@ -43,21 +43,6 @@ RSpec.describe API::UserCounts do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_a Hash expect(json_response['merge_requests']).to eq(2) - expect(json_response['attention_requests']).to eq(0) - end - - describe 'mr_attention_requests is disabled' do - before do - stub_feature_flags(mr_attention_requests: false) - end - - it 'does not include attention_requests count' do - create(:merge_request, source_project: project, author: user, assignees: [user]) - - get api('/user_counts', user) - - expect(json_response.key?('attention_requests')).to be(false) - end end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 81ca2548995..26238a87209 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1184,7 +1184,7 @@ RSpec.describe API::Users do post api('/users', admin), params: { email: 'invalid email', - password: 'password', + password: User.random_password, name: 'test' } expect(response).to have_gitlab_http_status(:bad_request) @@ -1250,7 +1250,7 @@ RSpec.describe API::Users do post api('/users', admin), params: { email: 'test@example.com', - password: 'password', + password: User.random_password, username: 'test', name: 'foo' } @@ -1262,7 +1262,7 @@ RSpec.describe API::Users do params: { name: 'foo', email: 'test@example.com', - password: 'password', + password: User.random_password, username: 'foo' } end.to change { User.count }.by(0) @@ -1276,7 +1276,7 @@ RSpec.describe API::Users do params: { name: 'foo', email: 'foo@example.com', - password: 'password', + password: User.random_password, username: 'test' } end.to change { User.count }.by(0) @@ -1290,7 +1290,7 @@ RSpec.describe API::Users do params: { name: 'foo', email: 'foo@example.com', - password: 'password', + password: User.random_password, username: 'TEST' } end.to change { User.count }.by(0) @@ -1710,8 +1710,8 @@ RSpec.describe API::Users do context "with existing user" do before do - post api("/users", admin), params: { email: 'test@example.com', password: 'password', username: 'test', name: 'test' } - post api("/users", admin), params: { email: 'foo@bar.com', password: 'password', username: 'john', name: 'john' } + post api("/users", admin), params: { email: 'test@example.com', password: User.random_password, username: 'test', name: 'test' } + post api("/users", admin), params: { email: 'foo@bar.com', password: User.random_password, username: 'john', name: 'john' } @user = User.all.last end |