diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-19 04:45:44 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-19 04:45:44 +0300 |
commit | 85dc423f7090da0a52c73eb66faf22ddb20efff9 (patch) | |
tree | 9160f299afd8c80c038f08e1545be119f5e3f1e1 /spec/requests/api | |
parent | 15c2c8c66dbe422588e5411eee7e68f1fa440bb8 (diff) |
Add latest changes from gitlab-org/gitlab@13-4-stable-ee
Diffstat (limited to 'spec/requests/api')
91 files changed, 3950 insertions, 2014 deletions
diff --git a/spec/requests/api/access_requests_spec.rb b/spec/requests/api/access_requests_spec.rb index 223d740a004..2af6c438fc9 100644 --- a/spec/requests/api/access_requests_spec.rb +++ b/spec/requests/api/access_requests_spec.rb @@ -90,7 +90,7 @@ RSpec.describe API::AccessRequests do context 'when authenticated as a stranger' do context "when access request is disabled for the #{source_type}" do before do - source.update(request_access_enabled: false) + source.update!(request_access_enabled: false) end it 'returns 403' do diff --git a/spec/requests/api/boards_spec.rb b/spec/requests/api/boards_spec.rb index f0d3afd0af7..a63198c5407 100644 --- a/spec/requests/api/boards_spec.rb +++ b/spec/requests/api/boards_spec.rb @@ -41,7 +41,7 @@ RSpec.describe API::Boards do it 'creates a new issue board list for group labels' do group = create(:group) group_label = create(:group_label, group: group) - board_parent.update(group: group) + board_parent.update!(group: group) post api(url, user), params: { label_id: group_label.id } @@ -54,7 +54,7 @@ RSpec.describe API::Boards do group = create(:group) sub_group = create(:group, parent: group) group_label = create(:group_label, group: group) - board_parent.update(group: sub_group) + board_parent.update!(group: sub_group) group.add_developer(user) sub_group.add_developer(user) diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 4b9b82b3a5b..5298f93886d 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -181,7 +181,7 @@ RSpec.describe API::Branches do context 'when unauthenticated', 'and project is public' do before do - project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) end it_behaves_like 'repository branches' @@ -289,7 +289,7 @@ RSpec.describe API::Branches do context 'when unauthenticated', 'and project is public' do before do - project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) end it_behaves_like 'repository branch' diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index 111bc933ea4..577b43e6e42 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe API::Ci::Pipelines do let_it_be(:user) { create(:user) } let_it_be(:non_member) { create(:user) } + let_it_be(:project2) { create(:project, creator: user) } # We need to reload as the shared example 'pipelines visibility table' is changing project let_it_be(:project, reload: true) do @@ -307,6 +308,606 @@ RSpec.describe API::Ci::Pipelines do end end + describe 'GET /projects/:id/pipelines/:pipeline_id/jobs' do + let(:query) { {} } + let(:api_user) { user } + let_it_be(:job) do + create(:ci_build, :success, pipeline: pipeline, + artifacts_expire_at: 1.day.since) + end + + let(:guest) { create(:project_member, :guest, project: project).user } + + before do |example| + unless example.metadata[:skip_before_request] + project.update!(public_builds: false) + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query + end + end + + context 'with ci_jobs_finder_refactor ff enabled' do + before do + stub_feature_flags(ci_jobs_finder_refactor: true) + end + + context 'authorized user' do + it 'returns pipeline jobs' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + end + + it 'returns correct values' do + expect(json_response).not_to be_empty + expect(json_response.first['commit']['id']).to eq project.commit.id + expect(Time.parse(json_response.first['artifacts_expire_at'])).to be_like_time(job.artifacts_expire_at) + expect(json_response.first['artifacts_file']).to be_nil + expect(json_response.first['artifacts']).to be_an Array + expect(json_response.first['artifacts']).to be_empty + end + + it_behaves_like 'a job with artifacts and trace' do + let(:api_endpoint) { "/projects/#{project.id}/pipelines/#{pipeline.id}/jobs" } + end + + it 'returns pipeline data' do + json_job = json_response.first + + expect(json_job['pipeline']).not_to be_empty + expect(json_job['pipeline']['id']).to eq job.pipeline.id + expect(json_job['pipeline']['ref']).to eq job.pipeline.ref + expect(json_job['pipeline']['sha']).to eq job.pipeline.sha + expect(json_job['pipeline']['status']).to eq job.pipeline.status + end + + context 'filter jobs with one scope element' do + let(:query) { { 'scope' => 'pending' } } + + it do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + end + end + + context 'filter jobs with hash' do + let(:query) { { scope: { hello: 'pending', world: 'running' } } } + + it { expect(response).to have_gitlab_http_status(:bad_request) } + end + + context 'filter jobs with array of scope elements' do + let(:query) { { scope: %w(pending running) } } + + it do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + end + end + + context 'respond 400 when scope contains invalid state' do + let(:query) { { scope: %w(unknown running) } } + + it { expect(response).to have_gitlab_http_status(:bad_request) } + end + + context 'jobs in different pipelines' do + let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } + let!(:job2) { create(:ci_build, pipeline: pipeline2) } + + it 'excludes jobs from other pipelines' do + json_response.each { |job| expect(job['pipeline']['id']).to eq(pipeline.id) } + end + end + + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query + end.count + + create_list(:ci_build, 3, :trace_artifact, :artifacts, :test_reports, pipeline: pipeline) + + expect do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query + end.not_to exceed_all_query_limit(control_count) + end + end + + context 'no pipeline is found' do + it 'does not return jobs' do + get api("/projects/#{project2.id}/pipelines/#{pipeline.id}/jobs", user) + + expect(json_response['message']).to eq '404 Project Not Found' + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'unauthorized user' do + context 'when user is not logged in' do + let(:api_user) { nil } + + it 'does not return jobs' do + expect(json_response['message']).to eq '404 Project Not Found' + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user is guest' do + let(:guest) { create(:project_member, :guest, project: project).user } + let(:api_user) { guest } + + it 'does not return jobs' do + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + + context 'with ci_jobs_finder ff disabled' do + before do + stub_feature_flags(ci_jobs_finder_refactor: false) + end + + context 'authorized user' do + it 'returns pipeline jobs' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + end + + it 'returns correct values' do + expect(json_response).not_to be_empty + expect(json_response.first['commit']['id']).to eq project.commit.id + expect(Time.parse(json_response.first['artifacts_expire_at'])).to be_like_time(job.artifacts_expire_at) + expect(json_response.first['artifacts_file']).to be_nil + expect(json_response.first['artifacts']).to be_an Array + expect(json_response.first['artifacts']).to be_empty + end + + it_behaves_like 'a job with artifacts and trace' do + let(:api_endpoint) { "/projects/#{project.id}/pipelines/#{pipeline.id}/jobs" } + end + + it 'returns pipeline data' do + json_job = json_response.first + + expect(json_job['pipeline']).not_to be_empty + expect(json_job['pipeline']['id']).to eq job.pipeline.id + expect(json_job['pipeline']['ref']).to eq job.pipeline.ref + expect(json_job['pipeline']['sha']).to eq job.pipeline.sha + expect(json_job['pipeline']['status']).to eq job.pipeline.status + end + + context 'filter jobs with one scope element' do + let(:query) { { 'scope' => 'pending' } } + + it do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + end + end + + context 'filter jobs with hash' do + let(:query) { { scope: { hello: 'pending', world: 'running' } } } + + it { expect(response).to have_gitlab_http_status(:bad_request) } + end + + context 'filter jobs with array of scope elements' do + let(:query) { { scope: %w(pending running) } } + + it do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + end + end + + context 'respond 400 when scope contains invalid state' do + let(:query) { { scope: %w(unknown running) } } + + it { expect(response).to have_gitlab_http_status(:bad_request) } + end + + context 'jobs in different pipelines' do + let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } + let!(:job2) { create(:ci_build, pipeline: pipeline2) } + + it 'excludes jobs from other pipelines' do + json_response.each { |job| expect(job['pipeline']['id']).to eq(pipeline.id) } + end + end + + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query + end.count + + create_list(:ci_build, 3, :trace_artifact, :artifacts, :test_reports, pipeline: pipeline) + + expect do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query + end.not_to exceed_all_query_limit(control_count) + end + end + + context 'no pipeline is found' do + it 'does not return jobs' do + get api("/projects/#{project2.id}/pipelines/#{pipeline.id}/jobs", user) + + expect(json_response['message']).to eq '404 Project Not Found' + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'unauthorized user' do + context 'when user is not logged in' do + let(:api_user) { nil } + + it 'does not return jobs' do + expect(json_response['message']).to eq '404 Project Not Found' + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user is guest' do + let(:guest) { create(:project_member, :guest, project: project).user } + let(:api_user) { guest } + + it 'does not return jobs' do + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + end + + describe 'GET /projects/:id/pipelines/:pipeline_id/bridges' do + let_it_be(:bridge) { create(:ci_bridge, pipeline: pipeline) } + let(:downstream_pipeline) { create(:ci_pipeline) } + + let!(:pipeline_source) do + create(:ci_sources_pipeline, + source_pipeline: pipeline, + source_project: project, + source_job: bridge, + pipeline: downstream_pipeline, + project: downstream_pipeline.project) + end + + let(:query) { {} } + let(:api_user) { user } + + before do |example| + unless example.metadata[:skip_before_request] + project.update!(public_builds: false) + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + end + end + + context 'with ci_jobs_finder_refactor ff enabled' do + before do + stub_feature_flags(ci_jobs_finder_refactor: true) + end + + context 'authorized user' do + it 'returns pipeline bridges' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + end + + it 'returns correct values' do + expect(json_response).not_to be_empty + expect(json_response.first['commit']['id']).to eq project.commit.id + expect(json_response.first['id']).to eq bridge.id + expect(json_response.first['name']).to eq bridge.name + expect(json_response.first['stage']).to eq bridge.stage + end + + it 'returns pipeline data' do + json_bridge = json_response.first + + expect(json_bridge['pipeline']).not_to be_empty + expect(json_bridge['pipeline']['id']).to eq bridge.pipeline.id + expect(json_bridge['pipeline']['ref']).to eq bridge.pipeline.ref + expect(json_bridge['pipeline']['sha']).to eq bridge.pipeline.sha + expect(json_bridge['pipeline']['status']).to eq bridge.pipeline.status + end + + it 'returns downstream pipeline data' do + json_bridge = json_response.first + + expect(json_bridge['downstream_pipeline']).not_to be_empty + expect(json_bridge['downstream_pipeline']['id']).to eq downstream_pipeline.id + expect(json_bridge['downstream_pipeline']['ref']).to eq downstream_pipeline.ref + expect(json_bridge['downstream_pipeline']['sha']).to eq downstream_pipeline.sha + expect(json_bridge['downstream_pipeline']['status']).to eq downstream_pipeline.status + end + + context 'filter bridges' do + before_all do + create_bridge(pipeline, :pending) + create_bridge(pipeline, :running) + end + + context 'with one scope element' do + let(:query) { { 'scope' => 'pending' } } + + it :skip_before_request do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.count).to eq 1 + expect(json_response.first["status"]).to eq "pending" + end + end + + context 'with array of scope elements' do + let(:query) { { scope: %w(pending running) } } + + it :skip_before_request do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.count).to eq 2 + json_response.each { |r| expect(%w(pending running).include?(r['status'])).to be true } + end + end + end + + context 'respond 400 when scope contains invalid state' do + context 'in an array' do + let(:query) { { scope: %w(unknown running) } } + + it { expect(response).to have_gitlab_http_status(:bad_request) } + end + + context 'in a hash' do + let(:query) { { scope: { unknown: true } } } + + it { expect(response).to have_gitlab_http_status(:bad_request) } + end + + context 'in a string' do + let(:query) { { scope: "unknown" } } + + it { expect(response).to have_gitlab_http_status(:bad_request) } + end + end + + context 'bridges in different pipelines' do + let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } + let!(:bridge2) { create(:ci_bridge, pipeline: pipeline2) } + + it 'excludes bridges from other pipelines' do + json_response.each { |bridge| expect(bridge['pipeline']['id']).to eq(pipeline.id) } + end + end + + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + end.count + + 3.times { create_bridge(pipeline) } + + expect do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + end.not_to exceed_all_query_limit(control_count) + end + end + + context 'no pipeline is found' do + it 'does not return bridges' do + get api("/projects/#{project2.id}/pipelines/#{pipeline.id}/bridges", user) + + expect(json_response['message']).to eq '404 Project Not Found' + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'unauthorized user' do + context 'when user is not logged in' do + let(:api_user) { nil } + + it 'does not return bridges' do + expect(json_response['message']).to eq '404 Project Not Found' + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user is guest' do + let(:api_user) { guest } + let(:guest) { create(:project_member, :guest, project: project).user } + + it 'does not return bridges' do + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user has no read_build access for project' do + before do + project.add_guest(api_user) + end + + it 'does not return bridges' do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user) + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + + context 'with ci_jobs_finder_refactor ff disabled' do + before do + stub_feature_flags(ci_jobs_finder_refactor: false) + end + + context 'authorized user' do + it 'returns pipeline bridges' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + end + + it 'returns correct values' do + expect(json_response).not_to be_empty + expect(json_response.first['commit']['id']).to eq project.commit.id + expect(json_response.first['id']).to eq bridge.id + expect(json_response.first['name']).to eq bridge.name + expect(json_response.first['stage']).to eq bridge.stage + end + + it 'returns pipeline data' do + json_bridge = json_response.first + + expect(json_bridge['pipeline']).not_to be_empty + expect(json_bridge['pipeline']['id']).to eq bridge.pipeline.id + expect(json_bridge['pipeline']['ref']).to eq bridge.pipeline.ref + expect(json_bridge['pipeline']['sha']).to eq bridge.pipeline.sha + expect(json_bridge['pipeline']['status']).to eq bridge.pipeline.status + end + + it 'returns downstream pipeline data' do + json_bridge = json_response.first + + expect(json_bridge['downstream_pipeline']).not_to be_empty + expect(json_bridge['downstream_pipeline']['id']).to eq downstream_pipeline.id + expect(json_bridge['downstream_pipeline']['ref']).to eq downstream_pipeline.ref + expect(json_bridge['downstream_pipeline']['sha']).to eq downstream_pipeline.sha + expect(json_bridge['downstream_pipeline']['status']).to eq downstream_pipeline.status + end + + context 'filter bridges' do + before_all do + create_bridge(pipeline, :pending) + create_bridge(pipeline, :running) + end + + context 'with one scope element' do + let(:query) { { 'scope' => 'pending' } } + + it :skip_before_request do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.count).to eq 1 + expect(json_response.first["status"]).to eq "pending" + end + end + + context 'with array of scope elements' do + let(:query) { { scope: %w(pending running) } } + + it :skip_before_request do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.count).to eq 2 + json_response.each { |r| expect(%w(pending running).include?(r['status'])).to be true } + end + end + end + + context 'respond 400 when scope contains invalid state' do + context 'in an array' do + let(:query) { { scope: %w(unknown running) } } + + it { expect(response).to have_gitlab_http_status(:bad_request) } + end + + context 'in a hash' do + let(:query) { { scope: { unknown: true } } } + + it { expect(response).to have_gitlab_http_status(:bad_request) } + end + + context 'in a string' do + let(:query) { { scope: "unknown" } } + + it { expect(response).to have_gitlab_http_status(:bad_request) } + end + end + + context 'bridges in different pipelines' do + let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } + let!(:bridge2) { create(:ci_bridge, pipeline: pipeline2) } + + it 'excludes bridges from other pipelines' do + json_response.each { |bridge| expect(bridge['pipeline']['id']).to eq(pipeline.id) } + end + end + + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + end.count + + 3.times { create_bridge(pipeline) } + + expect do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + end.not_to exceed_all_query_limit(control_count) + end + end + + context 'no pipeline is found' do + it 'does not return bridges' do + get api("/projects/#{project2.id}/pipelines/#{pipeline.id}/bridges", user) + + expect(json_response['message']).to eq '404 Project Not Found' + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'unauthorized user' do + context 'when user is not logged in' do + let(:api_user) { nil } + + it 'does not return bridges' do + expect(json_response['message']).to eq '404 Project Not Found' + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user is guest' do + let(:api_user) { guest } + let(:guest) { create(:project_member, :guest, project: project).user } + + it 'does not return bridges' do + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user has no read_build access for project' do + before do + project.add_guest(api_user) + end + + it 'does not return bridges' do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user) + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + + def create_bridge(pipeline, status = :created) + create(:ci_bridge, status: status, pipeline: pipeline).tap do |bridge| + downstream_pipeline = create(:ci_pipeline) + create(:ci_sources_pipeline, + source_pipeline: pipeline, + source_project: pipeline.project, + source_job: bridge, + pipeline: downstream_pipeline, + project: downstream_pipeline.project) + end + end + end + describe 'POST /projects/:id/pipeline ' do def expect_variables(variables, expected_variables) variables.each_with_index do |variable, index| @@ -476,17 +1077,18 @@ RSpec.describe API::Ci::Pipelines do end end - context 'when config source is not ci' do - let(:non_ci_config_source) { ::Ci::PipelineEnums.non_ci_config_source_values.first } - let(:pipeline_not_ci) do - create(:ci_pipeline, config_source: non_ci_config_source, project: project) + context 'when pipeline is a dangling pipeline' do + let(:dangling_source) { Enums::Ci::Pipeline.dangling_sources.each_value.first } + + let(:dangling_pipeline) do + create(:ci_pipeline, source: dangling_source, project: project) end it 'returns the specified pipeline' do - get api("/projects/#{project.id}/pipelines/#{pipeline_not_ci.id}", user) + get api("/projects/#{project.id}/pipelines/#{dangling_pipeline.id}", user) expect(response).to have_gitlab_http_status(:ok) - expect(json_response['sha']).to eq(pipeline_not_ci.sha) + expect(json_response['sha']).to eq(dangling_pipeline.sha) end end end @@ -624,7 +1226,7 @@ RSpec.describe API::Ci::Pipelines do end it 'does not log an audit event' do - expect { delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) }.not_to change { SecurityEvent.count } + expect { delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) }.not_to change { AuditEvent.count } end context 'when the pipeline has jobs' do diff --git a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb index e5c60bb539b..97110b63ff6 100644 --- a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb +++ b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb @@ -143,6 +143,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(response.media_type).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) expect(json_response['TempPath']).to eq(JobArtifactUploader.workhorse_local_upload_path) expect(json_response['RemoteObject']).to be_nil + expect(json_response['MaximumSize']).not_to be_nil end end @@ -167,6 +168,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(json_response['RemoteObject']).to have_key('StoreURL') expect(json_response['RemoteObject']).to have_key('DeleteURL') expect(json_response['RemoteObject']).to have_key('MultipartUpload') + expect(json_response['MaximumSize']).not_to be_nil end end @@ -188,6 +190,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(response).to have_gitlab_http_status(:ok) expect(response.media_type).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) expect(json_response['TempPath']).not_to be_nil + expect(json_response['MaximumSize']).not_to be_nil end it 'fails to post too large artifact' do @@ -235,36 +238,41 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(json_response['ProcessLsif']).to be_truthy end - it 'adds ProcessLsifReferences header' do - authorize_artifacts_with_token_in_headers(artifact_type: :lsif) + it 'tracks code_intelligence usage ping' do + tracking_params = { + event_names: 'i_source_code_code_intelligence', + start_date: Date.yesterday, + end_date: Date.today + } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['ProcessLsifReferences']).to be_truthy + expect { authorize_artifacts_with_token_in_headers(artifact_type: :lsif) } + .to change { Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(tracking_params) } + .by(1) end context 'code_navigation feature flag is disabled' do - it 'responds with a forbidden error' do + before do stub_feature_flags(code_navigation: false) + end + + it 'responds with a forbidden error' do authorize_artifacts_with_token_in_headers(artifact_type: :lsif) aggregate_failures do expect(response).to have_gitlab_http_status(:forbidden) expect(json_response['ProcessLsif']).to be_falsy - expect(json_response['ProcessLsifReferences']).to be_falsy end end - end - context 'code_navigation_references feature flag is disabled' do - it 'sets ProcessLsifReferences header to false' do - stub_feature_flags(code_navigation_references: false) - authorize_artifacts_with_token_in_headers(artifact_type: :lsif) + it 'does not track code_intelligence usage ping' do + tracking_params = { + event_names: 'i_source_code_code_intelligence', + start_date: Date.yesterday, + end_date: Date.today + } - aggregate_failures do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['ProcessLsif']).to be_truthy - expect(json_response['ProcessLsifReferences']).to be_falsy - end + expect { authorize_artifacts_with_token_in_headers(artifact_type: :lsif) } + .not_to change { Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(tracking_params) } end end end diff --git a/spec/requests/api/ci/runner/jobs_put_spec.rb b/spec/requests/api/ci/runner/jobs_put_spec.rb index 025747f2f0c..183a3b26e00 100644 --- a/spec/requests/api/ci/runner/jobs_put_spec.rb +++ b/spec/requests/api/ci/runner/jobs_put_spec.rb @@ -105,6 +105,67 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do it { expect(job).to be_unmet_prerequisites } end + + context 'when unmigrated live trace chunks exist' do + context 'when accepting trace feature is enabled' do + before do + stub_feature_flags(ci_accept_trace: true) + end + + context 'when checksum is present' do + context 'when live trace chunk is still live' do + it 'responds with 202' do + update_job(state: 'success', checksum: 'crc32:12345678') + + expect(job.pending_state).to be_present + expect(response).to have_gitlab_http_status(:accepted) + end + end + + context 'when runner retries request after receiving 202' do + it 'responds with 202 and then with 200', :sidekiq_inline do + perform_enqueued_jobs do + update_job(state: 'success', checksum: 'crc32:12345678') + end + + expect(job.reload.pending_state).to be_present + expect(response).to have_gitlab_http_status(:accepted) + + perform_enqueued_jobs do + update_job(state: 'success', checksum: 'crc32:12345678') + end + + expect(job.reload.pending_state).not_to be_present + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when live trace chunk has been migrated' do + before do + job.trace_chunks.first.update!(data_store: :database) + end + + it 'responds with 200' do + update_job(state: 'success', checksum: 'crc:12345678') + + expect(job.reload).to be_success + expect(job.pending_state).not_to be_present + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + context 'when checksum is not present' do + it 'responds with 200' do + update_job(state: 'success') + + expect(job.reload).to be_success + expect(job.pending_state).not_to be_present + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end end context 'when trace is given' do diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 21ff0a94db9..d34244771ad 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -367,10 +367,31 @@ RSpec.describe API::Commits do end end - it 'does not increment the usage counters using access token authentication' do - expect(::Gitlab::UsageDataCounters::WebIdeCounter).not_to receive(:increment_commits_count) + context 'when using access token authentication' do + it 'does not increment the usage counters' do + expect(::Gitlab::UsageDataCounters::WebIdeCounter).not_to receive(:increment_commits_count) + expect(::Gitlab::UsageDataCounters::EditorUniqueCounter).not_to receive(:track_web_ide_edit_action) - post api(url, user), params: valid_c_params + post api(url, user), params: valid_c_params + end + end + + context 'when using warden' do + it 'increments usage counters', :clean_gitlab_redis_shared_state do + session_id = Rack::Session::SessionId.new('6919a6f1bb119dd7396fadc38fd18d0d') + session_hash = { 'warden.user.user.key' => [[user.id], user.encrypted_password[0, 29]] } + + Gitlab::Redis::SharedState.with do |redis| + redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_hash)) + end + + cookies[Gitlab::Application.config.session_options[:key]] = session_id.public_id + + expect(::Gitlab::UsageDataCounters::WebIdeCounter).to receive(:increment_commits_count) + expect(::Gitlab::UsageDataCounters::EditorUniqueCounter).to receive(:track_web_ide_edit_action) + + post api(url), params: valid_c_params + end end context 'a new file in project repo' do diff --git a/spec/requests/api/composer_packages_spec.rb b/spec/requests/api/composer_packages_spec.rb index f5b8ebb545b..f5279af0483 100644 --- a/spec/requests/api/composer_packages_spec.rb +++ b/spec/requests/api/composer_packages_spec.rb @@ -26,30 +26,61 @@ RSpec.describe API::ComposerPackages do group.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) end - where(:project_visibility_level, :user_role, :member, :user_token, :include_package) do - 'PUBLIC' | :developer | true | true | :include_package - 'PUBLIC' | :developer | true | false | :include_package - 'PUBLIC' | :developer | false | false | :include_package - 'PUBLIC' | :developer | false | true | :include_package - 'PUBLIC' | :guest | true | true | :include_package - 'PUBLIC' | :guest | true | false | :include_package - 'PUBLIC' | :guest | false | true | :include_package - 'PUBLIC' | :guest | false | false | :include_package - 'PUBLIC' | :anonymous | false | true | :include_package - 'PRIVATE' | :developer | true | true | :include_package - 'PRIVATE' | :developer | true | false | :does_not_include_package - 'PRIVATE' | :developer | false | true | :does_not_include_package - 'PRIVATE' | :developer | false | false | :does_not_include_package - 'PRIVATE' | :guest | true | true | :does_not_include_package - 'PRIVATE' | :guest | true | false | :does_not_include_package - 'PRIVATE' | :guest | false | true | :does_not_include_package - 'PRIVATE' | :guest | false | false | :does_not_include_package - 'PRIVATE' | :anonymous | false | true | :does_not_include_package + context 'with basic auth' do + where(:project_visibility_level, :user_role, :member, :user_token, :include_package) do + 'PUBLIC' | :developer | true | true | :include_package + 'PUBLIC' | :developer | false | true | :include_package + 'PUBLIC' | :guest | true | true | :include_package + 'PUBLIC' | :guest | false | true | :include_package + 'PUBLIC' | :anonymous | false | true | :include_package + 'PRIVATE' | :developer | true | true | :include_package + 'PRIVATE' | :developer | false | true | :does_not_include_package + 'PRIVATE' | :guest | true | true | :does_not_include_package + 'PRIVATE' | :guest | false | true | :does_not_include_package + 'PRIVATE' | :anonymous | false | true | :does_not_include_package + 'PRIVATE' | :guest | false | false | :does_not_include_package + 'PRIVATE' | :guest | true | false | :does_not_include_package + 'PRIVATE' | :developer | false | false | :does_not_include_package + 'PRIVATE' | :developer | true | false | :does_not_include_package + 'PUBLIC' | :developer | true | false | :include_package + 'PUBLIC' | :guest | true | false | :include_package + 'PUBLIC' | :developer | false | false | :include_package + 'PUBLIC' | :guest | false | false | :include_package + end + + with_them do + include_context 'Composer api project access', params[:project_visibility_level], params[:user_role], params[:user_token], :basic do + it_behaves_like 'Composer package index', params[:user_role], :success, params[:member], params[:include_package] + end + end end - with_them do - include_context 'Composer api project access', params[:project_visibility_level], params[:user_role], params[:user_token] do - it_behaves_like 'Composer package index', params[:user_role], :success, params[:member], params[:include_package] + context 'with private token header auth' do + where(:project_visibility_level, :user_role, :member, :user_token, :expected_status, :include_package) do + 'PUBLIC' | :developer | true | true | :success | :include_package + 'PUBLIC' | :developer | false | true | :success | :include_package + 'PUBLIC' | :guest | true | true | :success | :include_package + 'PUBLIC' | :guest | false | true | :success | :include_package + 'PUBLIC' | :anonymous | false | true | :success | :include_package + 'PRIVATE' | :developer | true | true | :success | :include_package + 'PRIVATE' | :developer | false | true | :success | :does_not_include_package + 'PRIVATE' | :guest | true | true | :success | :does_not_include_package + 'PRIVATE' | :guest | false | true | :success | :does_not_include_package + 'PRIVATE' | :anonymous | false | true | :success | :does_not_include_package + 'PRIVATE' | :guest | false | false | :unauthorized | nil + 'PRIVATE' | :guest | true | false | :unauthorized | nil + 'PRIVATE' | :developer | false | false | :unauthorized | nil + 'PRIVATE' | :developer | true | false | :unauthorized | nil + 'PUBLIC' | :developer | true | false | :unauthorized | nil + 'PUBLIC' | :guest | true | false | :unauthorized | nil + 'PUBLIC' | :developer | false | false | :unauthorized | nil + 'PUBLIC' | :guest | false | false | :unauthorized | nil + end + + with_them do + include_context 'Composer api project access', params[:project_visibility_level], params[:user_role], params[:user_token], :token do + it_behaves_like 'Composer package index', params[:user_role], params[:expected_status], params[:member], params[:include_package] + end end end end @@ -105,22 +136,22 @@ RSpec.describe API::ComposerPackages do context 'with valid project' do where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do 'PUBLIC' | :developer | true | true | 'Composer provider index' | :success - 'PUBLIC' | :developer | true | false | 'Composer provider index' | :success + 'PUBLIC' | :developer | true | false | 'process Composer api request' | :unauthorized 'PUBLIC' | :developer | false | true | 'Composer provider index' | :success - 'PUBLIC' | :developer | false | false | 'Composer provider index' | :success + 'PUBLIC' | :developer | false | false | 'process Composer api request' | :unauthorized 'PUBLIC' | :guest | true | true | 'Composer provider index' | :success - 'PUBLIC' | :guest | true | false | 'Composer provider index' | :success + 'PUBLIC' | :guest | true | false | 'process Composer api request' | :unauthorized 'PUBLIC' | :guest | false | true | 'Composer provider index' | :success - 'PUBLIC' | :guest | false | false | 'Composer provider index' | :success + 'PUBLIC' | :guest | false | false | 'process Composer api request' | :unauthorized 'PUBLIC' | :anonymous | false | true | 'Composer provider index' | :success 'PRIVATE' | :developer | true | true | 'Composer provider index' | :success - 'PRIVATE' | :developer | true | false | 'process Composer api request' | :not_found + 'PRIVATE' | :developer | true | false | 'process Composer api request' | :unauthorized 'PRIVATE' | :developer | false | true | 'process Composer api request' | :not_found - 'PRIVATE' | :developer | false | false | 'process Composer api request' | :not_found + 'PRIVATE' | :developer | false | false | 'process Composer api request' | :unauthorized 'PRIVATE' | :guest | true | true | 'Composer empty provider index' | :success - 'PRIVATE' | :guest | true | false | 'process Composer api request' | :not_found + 'PRIVATE' | :guest | true | false | 'process Composer api request' | :unauthorized 'PRIVATE' | :guest | false | true | 'process Composer api request' | :not_found - 'PRIVATE' | :guest | false | false | 'process Composer api request' | :not_found + 'PRIVATE' | :guest | false | false | 'process Composer api request' | :unauthorized 'PRIVATE' | :anonymous | false | true | 'process Composer api request' | :not_found end @@ -151,22 +182,22 @@ RSpec.describe API::ComposerPackages do where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do 'PUBLIC' | :developer | true | true | 'Composer package api request' | :success - 'PUBLIC' | :developer | true | false | 'Composer package api request' | :success + 'PUBLIC' | :developer | true | false | 'process Composer api request' | :unauthorized 'PUBLIC' | :developer | false | true | 'Composer package api request' | :success - 'PUBLIC' | :developer | false | false | 'Composer package api request' | :success + 'PUBLIC' | :developer | false | false | 'process Composer api request' | :unauthorized 'PUBLIC' | :guest | true | true | 'Composer package api request' | :success - 'PUBLIC' | :guest | true | false | 'Composer package api request' | :success + 'PUBLIC' | :guest | true | false | 'process Composer api request' | :unauthorized 'PUBLIC' | :guest | false | true | 'Composer package api request' | :success - 'PUBLIC' | :guest | false | false | 'Composer package api request' | :success + 'PUBLIC' | :guest | false | false | 'process Composer api request' | :unauthorized 'PUBLIC' | :anonymous | false | true | 'Composer package api request' | :success 'PRIVATE' | :developer | true | true | 'Composer package api request' | :success - 'PRIVATE' | :developer | true | false | 'process Composer api request' | :not_found + 'PRIVATE' | :developer | true | false | 'process Composer api request' | :unauthorized 'PRIVATE' | :developer | false | true | 'process Composer api request' | :not_found - 'PRIVATE' | :developer | false | false | 'process Composer api request' | :not_found + 'PRIVATE' | :developer | false | false | 'process Composer api request' | :unauthorized 'PRIVATE' | :guest | true | true | 'process Composer api request' | :not_found - 'PRIVATE' | :guest | true | false | 'process Composer api request' | :not_found + 'PRIVATE' | :guest | true | false | 'process Composer api request' | :unauthorized 'PRIVATE' | :guest | false | true | 'process Composer api request' | :not_found - 'PRIVATE' | :guest | false | false | 'process Composer api request' | :not_found + 'PRIVATE' | :guest | false | false | 'process Composer api request' | :unauthorized 'PRIVATE' | :anonymous | false | true | 'process Composer api request' | :not_found end diff --git a/spec/requests/api/conan_instance_packages_spec.rb b/spec/requests/api/conan_instance_packages_spec.rb new file mode 100644 index 00000000000..817530f0bad --- /dev/null +++ b/spec/requests/api/conan_instance_packages_spec.rb @@ -0,0 +1,152 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::ConanInstancePackages do + include_context 'conan api setup' + + describe 'GET /api/v4/packages/conan/v1/ping' do + let_it_be(:url) { '/packages/conan/v1/ping' } + + it_behaves_like 'conan ping endpoint' + end + + describe 'GET /api/v4/packages/conan/v1/conans/search' do + let_it_be(:url) { '/packages/conan/v1/conans/search' } + + it_behaves_like 'conan search endpoint' + end + + describe 'GET /api/v4/packages/conan/v1/users/authenticate' do + let_it_be(:url) { '/packages/conan/v1/users/authenticate' } + + it_behaves_like 'conan authenticate endpoint' + end + + describe 'GET /api/v4/packages/conan/v1/users/check_credentials' do + let_it_be(:url) { "/packages/conan/v1/users/check_credentials" } + + it_behaves_like 'conan check_credentials endpoint' + end + + context 'recipe endpoints' do + include_context 'conan recipe endpoints' + + let(:project_id) { 9999 } + let(:url_prefix) { "#{Settings.gitlab.base_url}/api/v4" } + + describe 'GET /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel' do + let(:recipe_path) { package.conan_recipe_path } + let(:url) { "/packages/conan/v1/conans/#{recipe_path}" } + + it_behaves_like 'recipe snapshot endpoint' + end + + describe 'GET /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/packages/:conan_package_reference' do + let(:recipe_path) { package.conan_recipe_path } + let(:url) { "/packages/conan/v1/conans/#{recipe_path}/packages/#{conan_package_reference}" } + + it_behaves_like 'package snapshot endpoint' + end + + describe 'GET /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/digest' do + subject { get api("/packages/conan/v1/conans/#{recipe_path}/digest"), headers: headers } + + it_behaves_like 'recipe download_urls endpoint' + end + + describe 'GET /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/packages/:conan_package_reference/download_urls' do + subject { get api("/packages/conan/v1/conans/#{recipe_path}/packages/#{conan_package_reference}/download_urls"), headers: headers } + + it_behaves_like 'package download_urls endpoint' + end + + describe 'GET /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/download_urls' do + subject { get api("/packages/conan/v1/conans/#{recipe_path}/download_urls"), headers: headers } + + it_behaves_like 'recipe download_urls endpoint' + end + + describe 'GET /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/packages/:conan_package_reference/digest' do + subject { get api("/packages/conan/v1/conans/#{recipe_path}/packages/#{conan_package_reference}/digest"), headers: headers } + + it_behaves_like 'package download_urls endpoint' + end + + describe 'POST /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/upload_urls' do + subject { post api("/packages/conan/v1/conans/#{recipe_path}/upload_urls"), params: params.to_json, headers: headers } + + it_behaves_like 'recipe upload_urls endpoint' + end + + describe 'POST /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/packages/:conan_package_reference/upload_urls' do + subject { post api("/packages/conan/v1/conans/#{recipe_path}/packages/123456789/upload_urls"), params: params.to_json, headers: headers } + + it_behaves_like 'package upload_urls endpoint' + 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} + + it_behaves_like 'delete package endpoint' + end + end + + context 'file download endpoints' do + include_context 'conan file download endpoints' + + describe 'GET /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/ +:recipe_revision/export/:file_name' do + subject do + get api("/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision}/export/#{recipe_file.file_name}"), + headers: headers + end + + it_behaves_like 'recipe file download endpoint' + it_behaves_like 'project not found by recipe' + end + + describe 'GET /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/ +:recipe_revision/package/:conan_package_reference/:package_revision/:file_name' do + subject do + get api("/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision}/package/#{metadata.conan_package_reference}/#{metadata.package_revision}/#{package_file.file_name}"), + headers: headers + end + + it_behaves_like 'package file download endpoint' + it_behaves_like 'project not found by recipe' + end + end + + context 'file upload endpoints' do + include_context 'conan file upload endpoints' + + describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:file_name/authorize' do + let(:file_name) { 'conanfile.py' } + + subject { put api("/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}/authorize"), headers: headers_with_token } + + it_behaves_like 'workhorse authorize endpoint' + end + + describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:conan_package_reference/:package_revision/:file_name/authorize' do + let(:file_name) { 'conaninfo.txt' } + + subject { put api("/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}/authorize"), headers: headers_with_token } + + it_behaves_like 'workhorse authorize endpoint' + end + + describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:file_name' do + let(:url) { "/api/v4/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}" } + + it_behaves_like 'workhorse recipe file upload endpoint' + end + + describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:conan_package_reference/:package_revision/:file_name' do + let(:url) { "/api/v4/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}" } + + it_behaves_like 'workhorse package file upload endpoint' + end + end +end diff --git a/spec/requests/api/conan_packages_spec.rb b/spec/requests/api/conan_packages_spec.rb deleted file mode 100644 index d24f6b68048..00000000000 --- a/spec/requests/api/conan_packages_spec.rb +++ /dev/null @@ -1,954 +0,0 @@ -# frozen_string_literal: true -require 'spec_helper' - -RSpec.describe API::ConanPackages do - include WorkhorseHelpers - include HttpBasicAuthHelpers - include PackagesManagerApiSpecHelpers - - let(:package) { create(:conan_package) } - let_it_be(:personal_access_token) { create(:personal_access_token) } - let_it_be(:user) { personal_access_token.user } - let(:project) { package.project } - - let(:base_secret) { SecureRandom.base64(64) } - let(:auth_token) { personal_access_token.token } - let(:job) { create(:ci_build, user: user, status: :running) } - let(:job_token) { job.token } - let(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } - let(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } - - let(:headers) do - { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('foo', auth_token) } - end - - let(:jwt_secret) do - OpenSSL::HMAC.hexdigest( - OpenSSL::Digest::SHA256.new, - base_secret, - Gitlab::ConanToken::HMAC_KEY - ) - end - - before do - project.add_developer(user) - allow(Settings).to receive(:attr_encrypted_db_key_base).and_return(base_secret) - end - - describe 'GET /api/v4/packages/conan/v1/ping' do - it 'responds with 401 Unauthorized when no token provided' do - get api('/packages/conan/v1/ping') - - expect(response).to have_gitlab_http_status(:unauthorized) - end - - it 'responds with 200 OK when valid token is provided' do - jwt = build_jwt(personal_access_token) - get api('/packages/conan/v1/ping'), headers: build_token_auth_header(jwt.encoded) - - expect(response).to have_gitlab_http_status(:ok) - expect(response.headers['X-Conan-Server-Capabilities']).to eq("") - end - - it 'responds with 200 OK when valid job token is provided' do - jwt = build_jwt_from_job(job) - get api('/packages/conan/v1/ping'), headers: build_token_auth_header(jwt.encoded) - - expect(response).to have_gitlab_http_status(:ok) - expect(response.headers['X-Conan-Server-Capabilities']).to eq("") - end - - it 'responds with 200 OK when valid deploy token is provided' do - jwt = build_jwt_from_deploy_token(deploy_token) - get api('/packages/conan/v1/ping'), headers: build_token_auth_header(jwt.encoded) - - expect(response).to have_gitlab_http_status(:ok) - expect(response.headers['X-Conan-Server-Capabilities']).to eq("") - end - - it 'responds with 401 Unauthorized when invalid access token ID is provided' do - jwt = build_jwt(double(id: 12345), user_id: personal_access_token.user_id) - get api('/packages/conan/v1/ping'), headers: build_token_auth_header(jwt.encoded) - - expect(response).to have_gitlab_http_status(:unauthorized) - end - - it 'responds with 401 Unauthorized when invalid user is provided' do - jwt = build_jwt(personal_access_token, user_id: 12345) - get api('/packages/conan/v1/ping'), headers: build_token_auth_header(jwt.encoded) - - expect(response).to have_gitlab_http_status(:unauthorized) - end - - it 'responds with 401 Unauthorized when the provided JWT is signed with different secret' do - jwt = build_jwt(personal_access_token, secret: SecureRandom.base64(32)) - get api('/packages/conan/v1/ping'), headers: build_token_auth_header(jwt.encoded) - - expect(response).to have_gitlab_http_status(:unauthorized) - end - - it 'responds with 401 Unauthorized when invalid JWT is provided' do - get api('/packages/conan/v1/ping'), headers: build_token_auth_header('invalid-jwt') - - expect(response).to have_gitlab_http_status(:unauthorized) - end - - it 'responds with 401 Unauthorized when the job is not running' do - job.update!(status: :failed) - jwt = build_jwt_from_job(job) - get api('/packages/conan/v1/ping'), headers: build_token_auth_header(jwt.encoded) - - expect(response).to have_gitlab_http_status(:unauthorized) - end - - context 'packages feature disabled' do - it 'responds with 404 Not Found' do - stub_packages_setting(enabled: false) - get api('/packages/conan/v1/ping') - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - describe 'GET /api/v4/packages/conan/v1/conans/search' do - before do - project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - - get api('/packages/conan/v1/conans/search'), headers: headers, params: params - end - - subject { json_response['results'] } - - context 'returns packages with a matching name' do - let(:params) { { q: package.conan_recipe } } - - it { is_expected.to contain_exactly(package.conan_recipe) } - end - - context 'returns packages using a * wildcard' do - let(:params) { { q: "#{package.name[0, 3]}*" } } - - it { is_expected.to contain_exactly(package.conan_recipe) } - end - - context 'does not return non-matching packages' do - let(:params) { { q: "foo" } } - - it { is_expected.to be_blank } - end - end - - describe 'GET /api/v4/packages/conan/v1/users/authenticate' do - subject { get api('/packages/conan/v1/users/authenticate'), headers: headers } - - context 'when using invalid token' do - let(:auth_token) { 'invalid_token' } - - it 'responds with 401' do - subject - - expect(response).to have_gitlab_http_status(:unauthorized) - end - end - - context 'when valid JWT access token is provided' do - it 'responds with 200' do - subject - - expect(response).to have_gitlab_http_status(:ok) - end - - it 'token has valid validity time' do - Timecop.freeze do - subject - - payload = JSONWebToken::HMACToken.decode( - response.body, jwt_secret).first - expect(payload['access_token']).to eq(personal_access_token.id) - expect(payload['user_id']).to eq(personal_access_token.user_id) - - duration = payload['exp'] - payload['iat'] - expect(duration).to eq(1.hour) - end - end - end - - context 'with valid job token' do - let(:auth_token) { job_token } - - it 'responds with 200' do - subject - - expect(response).to have_gitlab_http_status(:ok) - end - end - - context 'with valid deploy token' do - let(:auth_token) { deploy_token.token } - - it 'responds with 200' do - subject - - expect(response).to have_gitlab_http_status(:ok) - end - end - end - - describe 'GET /api/v4/packages/conan/v1/users/check_credentials' do - it 'responds with a 200 OK with PAT' do - get api('/packages/conan/v1/users/check_credentials'), headers: headers - - expect(response).to have_gitlab_http_status(:ok) - end - - context 'with job token' do - let(:auth_token) { job_token } - - it 'responds with a 200 OK with job token' do - get api('/packages/conan/v1/users/check_credentials'), headers: headers - - expect(response).to have_gitlab_http_status(:ok) - end - end - - context 'with deploy token' do - let(:auth_token) { deploy_token.token } - - it 'responds with a 200 OK with job token' do - get api('/packages/conan/v1/users/check_credentials'), headers: headers - - expect(response).to have_gitlab_http_status(:ok) - end - end - - it 'responds with a 401 Unauthorized when an invalid token is used' do - get api('/packages/conan/v1/users/check_credentials'), headers: build_token_auth_header('invalid-token') - - expect(response).to have_gitlab_http_status(:unauthorized) - end - end - - shared_examples 'rejects invalid recipe' do - context 'with invalid recipe path' do - let(:recipe_path) { '../../foo++../..' } - - it 'returns 400' do - subject - - expect(response).to have_gitlab_http_status(:bad_request) - end - end - end - - shared_examples 'rejects invalid file_name' do |invalid_file_name| - let(:file_name) { invalid_file_name } - - context 'with invalid file_name' do - it 'returns 400' do - subject - - expect(response).to have_gitlab_http_status(:bad_request) - end - end - end - - shared_examples 'rejects recipe for invalid project' do - context 'with invalid recipe path' do - let(:recipe_path) { 'aa/bb/not-existing-project/ccc' } - - it 'returns forbidden' do - subject - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - end - - shared_examples 'rejects recipe for not found package' do - context 'with invalid recipe path' do - let(:recipe_path) do - 'aa/bb/%{project}/ccc' % { project: ::Packages::Conan::Metadatum.package_username_from(full_path: project.full_path) } - end - - it 'returns not found' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - shared_examples 'empty recipe for not found package' do - context 'with invalid recipe url' do - let(:recipe_path) do - 'aa/bb/%{project}/ccc' % { project: ::Packages::Conan::Metadatum.package_username_from(full_path: project.full_path) } - end - - it 'returns not found' do - allow(::Packages::Conan::PackagePresenter).to receive(:new) - .with( - 'aa/bb@%{project}/ccc' % { project: ::Packages::Conan::Metadatum.package_username_from(full_path: project.full_path) }, - user, - project, - any_args - ).and_return(presenter) - allow(presenter).to receive(:recipe_snapshot) { {} } - allow(presenter).to receive(:package_snapshot) { {} } - - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to eq("{}") - end - end - end - - shared_examples 'recipe download_urls' do - let(:recipe_path) { package.conan_recipe_path } - - it 'returns the download_urls for the recipe files' do - expected_response = { - 'conanfile.py' => "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/export/conanfile.py", - 'conanmanifest.txt' => "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/export/conanmanifest.txt" - } - - allow(presenter).to receive(:recipe_urls) { expected_response } - - subject - - expect(json_response).to eq(expected_response) - end - end - - shared_examples 'package download_urls' do - let(:recipe_path) { package.conan_recipe_path } - - it 'returns the download_urls for the package files' do - expected_response = { - 'conaninfo.txt' => "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conaninfo.txt", - 'conanmanifest.txt' => "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conanmanifest.txt", - 'conan_package.tgz' => "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conan_package.tgz" - } - - allow(presenter).to receive(:package_urls) { expected_response } - - subject - - expect(json_response).to eq(expected_response) - end - end - - context 'recipe endpoints' do - let(:jwt) { build_jwt(personal_access_token) } - let(:headers) { build_token_auth_header(jwt.encoded) } - let(:conan_package_reference) { '123456789' } - let(:presenter) { double('::Packages::Conan::PackagePresenter') } - - before do - allow(::Packages::Conan::PackagePresenter).to receive(:new) - .with(package.conan_recipe, user, package.project, any_args) - .and_return(presenter) - end - - shared_examples 'rejects invalid upload_url params' do - context 'with unaccepted json format' do - let(:params) { %w[foo bar] } - - it 'returns 400' do - subject - - expect(response).to have_gitlab_http_status(:bad_request) - end - end - end - - shared_examples 'successful response when using Unicorn' do - context 'on Unicorn', :unicorn do - it 'returns successfully' do - subject - - expect(response).to have_gitlab_http_status(:ok) - end - end - end - - describe 'GET /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel' do - let(:recipe_path) { package.conan_recipe_path } - - subject { get api("/packages/conan/v1/conans/#{recipe_path}"), headers: headers } - - it_behaves_like 'rejects invalid recipe' - it_behaves_like 'rejects recipe for invalid project' - it_behaves_like 'empty recipe for not found package' - - context 'with existing package' do - it 'returns a hash of files with their md5 hashes' do - expected_response = { - 'conanfile.py' => 'md5hash1', - 'conanmanifest.txt' => 'md5hash2' - } - - allow(presenter).to receive(:recipe_snapshot) { expected_response } - - subject - - expect(json_response).to eq(expected_response) - end - end - end - - describe 'GET /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/packages/:conan_package_reference' do - let(:recipe_path) { package.conan_recipe_path } - - subject { get api("/packages/conan/v1/conans/#{recipe_path}/packages/#{conan_package_reference}"), headers: headers } - - it_behaves_like 'rejects invalid recipe' - it_behaves_like 'rejects recipe for invalid project' - it_behaves_like 'empty recipe for not found package' - - context 'with existing package' do - it 'returns a hash of md5 values for the files' do - expected_response = { - 'conaninfo.txt' => "md5hash1", - 'conanmanifest.txt' => "md5hash2", - 'conan_package.tgz' => "md5hash3" - } - - allow(presenter).to receive(:package_snapshot) { expected_response } - - subject - - expect(json_response).to eq(expected_response) - end - end - end - - describe 'GET /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/digest' do - subject { get api("/packages/conan/v1/conans/#{recipe_path}/digest"), headers: headers } - - it_behaves_like 'rejects invalid recipe' - it_behaves_like 'rejects recipe for invalid project' - it_behaves_like 'recipe download_urls' - end - - describe 'GET /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/packages/:conan_package_reference/download_urls' do - subject { get api("/packages/conan/v1/conans/#{recipe_path}/packages/#{conan_package_reference}/download_urls"), headers: headers } - - it_behaves_like 'rejects invalid recipe' - it_behaves_like 'rejects recipe for invalid project' - it_behaves_like 'package download_urls' - end - - describe 'GET /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/download_urls' do - subject { get api("/packages/conan/v1/conans/#{recipe_path}/download_urls"), headers: headers } - - it_behaves_like 'rejects invalid recipe' - it_behaves_like 'rejects recipe for invalid project' - it_behaves_like 'recipe download_urls' - end - - describe 'GET /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/packages/:conan_package_reference/digest' do - subject { get api("/packages/conan/v1/conans/#{recipe_path}/packages/#{conan_package_reference}/digest"), headers: headers } - - it_behaves_like 'rejects invalid recipe' - it_behaves_like 'rejects recipe for invalid project' - it_behaves_like 'package download_urls' - end - - describe 'POST /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/upload_urls' do - let(:recipe_path) { package.conan_recipe_path } - - let(:params) do - { 'conanfile.py': 24, - 'conanmanifest.txt': 123 } - end - - subject { post api("/packages/conan/v1/conans/#{recipe_path}/upload_urls"), params: params.to_json, headers: headers } - - it_behaves_like 'rejects invalid recipe' - it_behaves_like 'rejects invalid upload_url params' - it_behaves_like 'successful response when using Unicorn' - - it 'returns a set of upload urls for the files requested' do - subject - - expected_response = { - 'conanfile.py': "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/export/conanfile.py", - 'conanmanifest.txt': "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/export/conanmanifest.txt" - } - - expect(response.body).to eq(expected_response.to_json) - end - - context 'with conan_sources and conan_export files' do - let(:params) do - { 'conan_sources.tgz': 345, - 'conan_export.tgz': 234, - 'conanmanifest.txt': 123 } - end - - it 'returns upload urls for the additional files' do - subject - - expected_response = { - 'conan_sources.tgz': "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/export/conan_sources.tgz", - 'conan_export.tgz': "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/export/conan_export.tgz", - 'conanmanifest.txt': "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/export/conanmanifest.txt" - } - - expect(response.body).to eq(expected_response.to_json) - end - end - - context 'with an invalid file' do - let(:params) do - { 'invalid_file.txt': 10, - 'conanmanifest.txt': 123 } - end - - it 'does not return the invalid file as an upload_url' do - subject - - expected_response = { - 'conanmanifest.txt': "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/export/conanmanifest.txt" - } - - expect(response.body).to eq(expected_response.to_json) - end - end - end - - describe 'POST /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/packages/:conan_package_reference/upload_urls' do - let(:recipe_path) { package.conan_recipe_path } - - let(:params) do - { 'conaninfo.txt': 24, - 'conanmanifest.txt': 123, - 'conan_package.tgz': 523 } - end - - subject { post api("/packages/conan/v1/conans/#{recipe_path}/packages/123456789/upload_urls"), params: params.to_json, headers: headers } - - it_behaves_like 'rejects invalid recipe' - it_behaves_like 'rejects invalid upload_url params' - it_behaves_like 'successful response when using Unicorn' - - it 'returns a set of upload urls for the files requested' do - expected_response = { - 'conaninfo.txt': "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conaninfo.txt", - 'conanmanifest.txt': "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conanmanifest.txt", - 'conan_package.tgz': "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conan_package.tgz" - } - - subject - - expect(response.body).to eq(expected_response.to_json) - end - - context 'with invalid files' do - let(:params) do - { 'conaninfo.txt': 24, - 'invalid_file.txt': 10 } - end - - it 'returns upload urls only for the valid requested files' do - expected_response = { - 'conaninfo.txt': "#{Settings.gitlab.base_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conaninfo.txt" - } - - subject - - expect(response.body).to eq(expected_response.to_json) - end - end - end - - describe 'DELETE /api/v4/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel' do - let(:recipe_path) { package.conan_recipe_path } - - subject { delete api("/packages/conan/v1/conans/#{recipe_path}"), headers: headers} - - it_behaves_like 'rejects invalid recipe' - - it 'returns unauthorized for users without valid permission' do - subject - - expect(response).to have_gitlab_http_status(:forbidden) - end - - context 'with delete permissions' do - before do - project.add_maintainer(user) - end - - it_behaves_like 'a gitlab tracking event', described_class.name, 'delete_package' - - it 'deletes a package' do - expect { subject }.to change { Packages::Package.count }.from(2).to(1) - end - end - end - end - - context 'file endpoints' do - let(:jwt) { build_jwt(personal_access_token) } - let(:headers) { build_token_auth_header(jwt.encoded) } - let(:recipe_path) { package.conan_recipe_path } - - shared_examples 'denies download with no token' do - context 'with no private token' do - let(:headers) { {} } - - it 'returns 400' do - subject - - expect(response).to have_gitlab_http_status(:unauthorized) - end - end - end - - shared_examples 'a public project with packages' do - it 'returns the file' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response.media_type).to eq('application/octet-stream') - end - end - - shared_examples 'an internal project with packages' do - before do - project.team.truncate - project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - end - - it_behaves_like 'denies download with no token' - - it 'returns the file' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response.media_type).to eq('application/octet-stream') - end - end - - shared_examples 'a private project with packages' do - before do - project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) - end - - it_behaves_like 'denies download with no token' - - it 'returns the file' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response.media_type).to eq('application/octet-stream') - end - - it 'denies download when not enough permissions' do - project.add_guest(user) - - subject - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - shared_examples 'a project is not found' do - let(:recipe_path) { 'not/package/for/project' } - - it 'returns forbidden' do - subject - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - describe 'GET /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/ -:recipe_revision/export/:file_name' do - let(:recipe_file) { package.package_files.find_by(file_name: 'conanfile.py') } - let(:metadata) { recipe_file.conan_file_metadatum } - - subject do - get api("/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision}/export/#{recipe_file.file_name}"), - headers: headers - end - - it_behaves_like 'a public project with packages' - it_behaves_like 'an internal project with packages' - it_behaves_like 'a private project with packages' - it_behaves_like 'a project is not found' - end - - describe 'GET /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/ -:recipe_revision/package/:conan_package_reference/:package_revision/:file_name' do - let(:package_file) { package.package_files.find_by(file_name: 'conaninfo.txt') } - let(:metadata) { package_file.conan_file_metadatum } - - subject do - get api("/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision}/package/#{metadata.conan_package_reference}/#{metadata.package_revision}/#{package_file.file_name}"), - headers: headers - end - - it_behaves_like 'a public project with packages' - it_behaves_like 'an internal project with packages' - it_behaves_like 'a private project with packages' - it_behaves_like 'a project is not found' - - context 'tracking the conan_package.tgz download' do - let(:package_file) { package.package_files.find_by(file_name: ::Packages::Conan::FileMetadatum::PACKAGE_BINARY) } - - it_behaves_like 'a gitlab tracking event', described_class.name, 'pull_package' - end - end - end - - context 'file uploads' do - let(:jwt) { build_jwt(personal_access_token) } - let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } - let(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } - let(:headers_with_token) { build_token_auth_header(jwt.encoded).merge(workhorse_header) } - let(:recipe_path) { "foo/bar/#{project.full_path.tr('/', '+')}/baz"} - - shared_examples 'uploads a package file' do - context 'with object storage disabled' do - context 'without a file from workhorse' do - let(:params) { { file: nil } } - - it 'rejects the request' do - subject - - expect(response).to have_gitlab_http_status(:bad_request) - end - end - - context 'with a file' do - it_behaves_like 'package workhorse uploads' - end - - context 'without a token' do - it 'rejects request without a token' do - headers_with_token.delete('HTTP_AUTHORIZATION') - - subject - - expect(response).to have_gitlab_http_status(:unauthorized) - end - end - - context 'when params from workhorse are correct' do - it 'creates package and stores package file' do - expect { subject } - .to change { project.packages.count }.by(1) - .and change { Packages::PackageFile.count }.by(1) - - expect(response).to have_gitlab_http_status(:ok) - - package_file = project.packages.last.package_files.reload.last - expect(package_file.file_name).to eq(params[:file].original_filename) - end - - it "doesn't attempt to migrate file to object storage" do - expect(ObjectStorage::BackgroundMoveWorker).not_to receive(:perform_async) - - subject - end - end - end - - context 'with object storage enabled' do - context 'and direct upload enabled' do - let!(:fog_connection) do - stub_package_file_object_storage(direct_upload: true) - end - - let(:tmp_object) do - fog_connection.directories.new(key: 'packages').files.create( - key: "tmp/uploads/#{file_name}", - body: 'content' - ) - end - - let(:fog_file) { fog_to_uploaded_file(tmp_object) } - - ['123123', '../../123123'].each do |remote_id| - context "with invalid remote_id: #{remote_id}" do - let(:params) do - { - file: fog_file, - 'file.remote_id' => remote_id - } - end - - it 'responds with status 403' do - subject - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - end - - context 'with valid remote_id' do - let(:params) do - { - file: fog_file, - 'file.remote_id' => file_name - } - end - - it 'creates package and stores package file' do - expect { subject } - .to change { project.packages.count }.by(1) - .and change { Packages::PackageFile.count }.by(1) - - expect(response).to have_gitlab_http_status(:ok) - - package_file = project.packages.last.package_files.reload.last - expect(package_file.file_name).to eq(params[:file].original_filename) - expect(package_file.file.read).to eq('content') - end - end - end - - it_behaves_like 'background upload schedules a file migration' - end - end - - shared_examples 'workhorse authorization' do - it 'authorizes posting package with a valid token' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response.media_type).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) - end - - it 'rejects request without a valid token' do - headers_with_token['HTTP_AUTHORIZATION'] = 'foo' - - subject - - expect(response).to have_gitlab_http_status(:unauthorized) - end - - it 'rejects request without a valid permission' do - project.add_guest(user) - - subject - - expect(response).to have_gitlab_http_status(:forbidden) - end - - it 'rejects requests that bypassed gitlab-workhorse' do - headers_with_token.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER) - - subject - - expect(response).to have_gitlab_http_status(:forbidden) - end - - context 'when using remote storage' do - context 'when direct upload is enabled' do - before do - stub_package_file_object_storage(enabled: true, direct_upload: true) - end - - it 'responds with status 200, location of package remote store and object details' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response.media_type).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) - expect(json_response).not_to have_key('TempPath') - expect(json_response['RemoteObject']).to have_key('ID') - expect(json_response['RemoteObject']).to have_key('GetURL') - expect(json_response['RemoteObject']).to have_key('StoreURL') - expect(json_response['RemoteObject']).to have_key('DeleteURL') - expect(json_response['RemoteObject']).not_to have_key('MultipartUpload') - end - end - - context 'when direct upload is disabled' do - before do - stub_package_file_object_storage(enabled: true, direct_upload: false) - end - - it 'handles as a local file' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response.media_type).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) - expect(json_response['TempPath']).to eq(::Packages::PackageFileUploader.workhorse_local_upload_path) - expect(json_response['RemoteObject']).to be_nil - end - end - end - end - - describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:file_name/authorize' do - let(:file_name) { 'conanfile.py' } - - subject { put api("/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}/authorize"), headers: headers_with_token } - - it_behaves_like 'rejects invalid recipe' - it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack' - it_behaves_like 'workhorse authorization' - end - - describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:conan_package_reference/:package_revision/:file_name/authorize' do - let(:file_name) { 'conaninfo.txt' } - - subject { put api("/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}/authorize"), headers: headers_with_token } - - it_behaves_like 'rejects invalid recipe' - it_behaves_like 'rejects invalid file_name', 'conaninfo.txttest' - it_behaves_like 'workhorse authorization' - end - - describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:file_name' do - let(:file_name) { 'conanfile.py' } - let(:params) { { file: temp_file(file_name) } } - - subject do - workhorse_finalize( - "/api/v4/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}", - method: :put, - file_key: :file, - params: params, - send_rewritten_field: true, - headers: headers_with_token - ) - end - - it_behaves_like 'rejects invalid recipe' - it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack' - it_behaves_like 'uploads a package file' - end - - describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:conan_package_reference/:package_revision/:file_name' do - let(:file_name) { 'conaninfo.txt' } - let(:params) { { file: temp_file(file_name) } } - - subject do - workhorse_finalize( - "/api/v4/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}", - method: :put, - file_key: :file, - params: params, - headers: headers_with_token, - send_rewritten_field: true - ) - end - - it_behaves_like 'rejects invalid recipe' - it_behaves_like 'rejects invalid file_name', 'conaninfo.txttest' - it_behaves_like 'uploads a package file' - - context 'tracking the conan_package.tgz upload' do - let(:file_name) { ::Packages::Conan::FileMetadatum::PACKAGE_BINARY } - - it_behaves_like 'a gitlab tracking event', described_class.name, 'push_package' - end - end - end -end diff --git a/spec/requests/api/conan_project_packages_spec.rb b/spec/requests/api/conan_project_packages_spec.rb new file mode 100644 index 00000000000..fefaf9790b1 --- /dev/null +++ b/spec/requests/api/conan_project_packages_spec.rb @@ -0,0 +1,152 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe API::ConanProjectPackages do + include_context 'conan api setup' + + let(:project_id) { project.id } + + describe 'GET /api/v4/projects/:id/packages/conan/v1/ping' do + let(:url) { "/projects/#{project.id}/packages/conan/v1/ping" } + + it_behaves_like 'conan ping endpoint' + end + + describe 'GET /api/v4/projects/:id/packages/conan/v1/conans/search' do + let(:url) { "/projects/#{project.id}/packages/conan/v1/conans/search" } + + it_behaves_like 'conan search endpoint' + end + + describe 'GET /api/v4/projects/:id/packages/conan/v1/users/authenticate' do + let(:url) { "/projects/#{project.id}/packages/conan/v1/users/authenticate" } + + it_behaves_like 'conan authenticate endpoint' + end + + describe 'GET /api/v4/projects/:id/packages/conan/v1/users/check_credentials' do + let(:url) { "/projects/#{project.id}/packages/conan/v1/users/check_credentials" } + + it_behaves_like 'conan check_credentials endpoint' + end + + context 'recipe endpoints' do + include_context 'conan recipe endpoints' + + let(:url_prefix) { "#{Settings.gitlab.base_url}/api/v4/projects/#{project_id}" } + + describe 'GET /api/v4/projects/:id/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel' do + let(:recipe_path) { package.conan_recipe_path } + let(:url) { "/projects/#{project_id}/packages/conan/v1/conans/#{recipe_path}" } + + it_behaves_like 'recipe snapshot endpoint' + end + + describe 'GET /api/v4/projects/:id/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/packages/:conan_package_reference' do + let(:recipe_path) { package.conan_recipe_path } + let(:url) { "/projects/#{project_id}/packages/conan/v1/conans/#{recipe_path}/packages/#{conan_package_reference}" } + + it_behaves_like 'package snapshot endpoint' + end + + describe 'GET /api/v4/projects/:id/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/digest' do + subject { get api("/projects/#{project_id}/packages/conan/v1/conans/#{recipe_path}/digest"), headers: headers } + + it_behaves_like 'recipe download_urls endpoint' + end + + describe 'GET /api/v4/projects/:id/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/packages/:conan_package_reference/download_urls' do + subject { get api("/projects/#{project_id}/packages/conan/v1/conans/#{recipe_path}/packages/#{conan_package_reference}/download_urls"), headers: headers } + + it_behaves_like 'package download_urls endpoint' + end + + describe 'GET /api/v4/projects/:id/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/download_urls' do + subject { get api("/projects/#{project_id}/packages/conan/v1/conans/#{recipe_path}/download_urls"), headers: headers } + + it_behaves_like 'recipe download_urls endpoint' + end + + describe 'GET /api/v4/projects/:id/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/packages/:conan_package_reference/digest' do + subject { get api("/projects/#{project_id}/packages/conan/v1/conans/#{recipe_path}/packages/#{conan_package_reference}/digest"), headers: headers } + + it_behaves_like 'package download_urls endpoint' + end + + describe 'POST /api/v4/projects/:id/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/upload_urls' do + subject { post api("/projects/#{project_id}/packages/conan/v1/conans/#{recipe_path}/upload_urls"), params: params.to_json, headers: headers } + + it_behaves_like 'recipe upload_urls endpoint' + end + + describe 'POST /api/v4/projects/:id/packages/conan/v1/conans/:package_name/package_version/:package_username/:package_channel/packages/:conan_package_reference/upload_urls' do + subject { post api("/projects/#{project_id}/packages/conan/v1/conans/#{recipe_path}/packages/123456789/upload_urls"), params: params.to_json, headers: headers } + + it_behaves_like 'package upload_urls endpoint' + 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} + + it_behaves_like 'delete package endpoint' + end + end + + context 'file download endpoints' do + include_context 'conan file download endpoints' + + describe 'GET /api/v4/projects/:id/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/ +:recipe_revision/export/:file_name' do + subject do + get api("/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision}/export/#{recipe_file.file_name}"), + headers: headers + end + + it_behaves_like 'recipe file download endpoint' + it_behaves_like 'project not found by project id' + end + + describe 'GET /api/v4/projects/:id/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/ +:recipe_revision/package/:conan_package_reference/:package_revision/:file_name' do + subject do + get api("/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision}/package/#{metadata.conan_package_reference}/#{metadata.package_revision}/#{package_file.file_name}"), + headers: headers + end + + it_behaves_like 'package file download endpoint' + it_behaves_like 'project not found by project id' + end + end + + context 'file upload endpoints' do + include_context 'conan file upload endpoints' + + describe 'PUT /api/v4/projects/:id/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:file_name/authorize' do + let(:file_name) { 'conanfile.py' } + + subject { put api("/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}/authorize"), headers: headers_with_token } + + it_behaves_like 'workhorse authorize endpoint' + end + + describe 'PUT /api/v4/projects/:id/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:conan_package_reference/:package_revision/:file_name/authorize' do + let(:file_name) { 'conaninfo.txt' } + + subject { put api("/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}/authorize"), headers: headers_with_token } + + it_behaves_like 'workhorse authorize endpoint' + end + + describe 'PUT /api/v4/projects/:id/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:file_name' do + let(:url) { "/api/v4/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}" } + + it_behaves_like 'workhorse recipe file upload endpoint' + end + + describe 'PUT /api/v4/projects/:id/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:conan_package_reference/:package_revision/:file_name' do + let(:url) { "/api/v4/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}" } + + it_behaves_like 'workhorse package file upload endpoint' + end + end +end diff --git a/spec/requests/api/generic_packages_spec.rb b/spec/requests/api/generic_packages_spec.rb new file mode 100644 index 00000000000..ed852fe75c7 --- /dev/null +++ b/spec/requests/api/generic_packages_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::GenericPackages do + let_it_be(:personal_access_token) { create(:personal_access_token) } + let_it_be(:project) { create(:project) } + + describe 'GET /api/v4/projects/:id/packages/generic/ping' do + let(:user) { personal_access_token.user } + let(:auth_token) { personal_access_token.token } + + before do + project.add_developer(user) + end + + context 'packages feature is disabled' do + it 'responds with 404 Not Found' do + stub_packages_setting(enabled: false) + + ping(personal_access_token: auth_token) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'generic_packages feature flag is disabled' do + it 'responds with 404 Not Found' do + stub_feature_flags(generic_packages: false) + + ping(personal_access_token: auth_token) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'generic_packages feature flag is enabled' do + before do + stub_feature_flags(generic_packages: true) + end + + context 'authenticating using personal access token' do + it 'responds with 200 OK when valid personal access token is provided' do + ping(personal_access_token: auth_token) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'responds with 401 Unauthorized when invalid personal access token provided' do + ping(personal_access_token: 'invalid-token') + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'authenticating using job token' do + it 'responds with 200 OK when valid job token is provided' do + job_token = create(:ci_build, :running, user: user).token + + ping(job_token: job_token) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'responds with 401 Unauthorized when invalid job token provided' do + ping(job_token: 'invalid-token') + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + def ping(personal_access_token: nil, job_token: nil) + headers = { + Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => personal_access_token.presence, + Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER => job_token.presence + }.compact + + get api('/projects/%d/packages/generic/ping' % project.id), headers: headers + end + end +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 ae1abb50a40..3628171fcc1 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 @@ -30,7 +30,7 @@ RSpec.describe 'get board lists' do nodes { lists { nodes { - issues { + issues(filters: {labelName: "#{label2.title}"}) { count nodes { #{all_graphql_fields_for('issues'.classify)} @@ -51,8 +51,8 @@ RSpec.describe 'get board lists' do shared_examples 'group and project board list issues query' do let!(:board) { create(:board, resource_parent: board_parent) } let!(:label_list) { create(:list, board: board, label: label, position: 10) } - let!(:issue1) { create(:issue, project: issue_project, labels: [label], relative_position: 9) } - let!(:issue2) { create(:issue, project: issue_project, labels: [label], relative_position: 2) } + let!(:issue1) { create(:issue, project: issue_project, labels: [label, label2], relative_position: 9) } + let!(:issue2) { create(:issue, project: issue_project, labels: [label, label2], relative_position: 2) } let!(:issue3) { create(:issue, project: issue_project, labels: [label], relative_position: 9) } let!(:issue4) { create(:issue, project: issue_project, labels: [label2], relative_position: 432) } @@ -72,7 +72,7 @@ RSpec.describe 'get board lists' do it 'can access the issues' do post_graphql(query("id: \"#{global_id_of(label_list)}\""), current_user: user) - expect(issue_titles).to eq([issue2.title, issue3.title, issue1.title]) + expect(issue_titles).to eq([issue2.title, issue1.title]) end end end diff --git a/spec/requests/api/graphql/current_user_todos_spec.rb b/spec/requests/api/graphql/current_user_todos_spec.rb new file mode 100644 index 00000000000..b657f15d0e9 --- /dev/null +++ b/spec/requests/api/graphql/current_user_todos_spec.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'A Todoable that implements the CurrentUserTodos interface' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:project) { create(:project, :public) } + let_it_be(:todoable) { create(:issue, project: project) } + let_it_be(:done_todo) { create(:todo, state: :done, target: todoable, user: current_user) } + let_it_be(:pending_todo) { create(:todo, state: :pending, target: todoable, user: current_user) } + let(:state) { 'null' } + + let(:todoable_response) do + graphql_data_at(:project, :issue, :currentUserTodos, :nodes) + end + + let(:query) do + <<~GQL + { + project(fullPath: "#{project.full_path}") { + issue(iid: "#{todoable.iid}") { + currentUserTodos(state: #{state}) { + nodes { + #{all_graphql_fields_for('Todo', max_depth: 1)} + } + } + } + } + } + GQL + end + + it 'returns todos of the current user' do + post_graphql(query, current_user: current_user) + + expect(todoable_response).to contain_exactly( + a_hash_including('id' => global_id_of(done_todo)), + a_hash_including('id' => global_id_of(pending_todo)) + ) + end + + it 'does not return todos of another user', :aggregate_failures do + post_graphql(query, current_user: create(:user)) + + expect(response).to have_gitlab_http_status(:success) + expect(todoable_response).to be_empty + end + + it 'does not error when there is no logged in user', :aggregate_failures do + post_graphql(query) + + expect(response).to have_gitlab_http_status(:success) + expect(todoable_response).to be_empty + end + + context 'when `state` argument is `pending`' do + let(:state) { 'pending' } + + it 'returns just the pending todo' do + post_graphql(query, current_user: current_user) + + expect(todoable_response).to contain_exactly( + a_hash_including('id' => global_id_of(pending_todo)) + ) + end + end + + context 'when `state` argument is `done`' do + let(:state) { 'done' } + + it 'returns just the done todo' do + post_graphql(query, current_user: current_user) + + expect(todoable_response).to contain_exactly( + a_hash_including('id' => global_id_of(done_todo)) + ) + end + end +end diff --git a/spec/requests/api/graphql/group/group_members_spec.rb b/spec/requests/api/graphql/group/group_members_spec.rb new file mode 100644 index 00000000000..84b2fd63d46 --- /dev/null +++ b/spec/requests/api/graphql/group/group_members_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting group members information' do + include GraphqlHelpers + + let_it_be(:group) { create(:group, :public) } + let_it_be(:user) { create(:user) } + let_it_be(:user_1) { create(:user, username: 'user') } + let_it_be(:user_2) { create(:user, username: 'test') } + + let(:member_data) { graphql_data['group']['groupMembers']['edges'] } + + before do + [user_1, user_2].each { |user| group.add_guest(user) } + end + + context 'when the request is correct' do + it_behaves_like 'a working graphql query' do + before do + fetch_members(user) + end + end + + it 'returns group members successfully' do + fetch_members(user) + + expect(graphql_errors).to be_nil + expect_array_response(user_1.to_global_id.to_s, user_2.to_global_id.to_s) + end + + it 'returns members that match the search query' do + fetch_members(user, { search: 'test' }) + + expect(graphql_errors).to be_nil + expect_array_response(user_2.to_global_id.to_s) + end + end + + def fetch_members(user = nil, args = {}) + post_graphql(members_query(args), current_user: user) + end + + def members_query(args = {}) + members_node = <<~NODE + edges { + node { + user { + id + } + } + } + NODE + + graphql_query_for("group", + { full_path: group.full_path }, + [query_graphql_field("groupMembers", args, members_node)] + ) + end + + def expect_array_response(*items) + expect(response).to have_gitlab_http_status(:success) + expect(member_data).to be_an Array + expect(member_data.map { |node| node["node"]["user"]["id"] }).to match_array(items) + end +end diff --git a/spec/requests/api/graphql/group_query_spec.rb b/spec/requests/api/graphql/group_query_spec.rb index d99bff2e349..83180c7d7a5 100644 --- a/spec/requests/api/graphql/group_query_spec.rb +++ b/spec/requests/api/graphql/group_query_spec.rb @@ -89,18 +89,13 @@ RSpec.describe 'getting group information', :do_not_mock_admin_mode do end it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new do - post_graphql(group_query(group1), current_user: admin) - end.count + pending('See: https://gitlab.com/gitlab-org/gitlab/-/issues/245272') queries = [{ query: group_query(group1) }, { query: group_query(group2) }] - expect do - post_multiplex(queries, current_user: admin) - end.not_to exceed_query_limit(control_count) - - expect(graphql_errors).to contain_exactly(nil, nil) + expect { post_multiplex(queries, current_user: admin) } + .to issue_same_number_of_queries_as { post_graphql(group_query(group1), current_user: admin) } end end diff --git a/spec/requests/api/graphql/instance_statistics_measurements_spec.rb b/spec/requests/api/graphql/instance_statistics_measurements_spec.rb new file mode 100644 index 00000000000..b8cbe54534a --- /dev/null +++ b/spec/requests/api/graphql/instance_statistics_measurements_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'InstanceStatisticsMeasurements' do + include GraphqlHelpers + + let(:current_user) { create(:user, :admin) } + let!(:instance_statistics_measurement_1) { create(:instance_statistics_measurement, :project_count, recorded_at: 20.days.ago, count: 5) } + let!(:instance_statistics_measurement_2) { create(:instance_statistics_measurement, :project_count, recorded_at: 10.days.ago, count: 10) } + + let(:query) { graphql_query_for(:instanceStatisticsMeasurements, 'identifier: PROJECTS', 'nodes { count }') } + + before do + post_graphql(query, current_user: current_user) + end + + it 'returns measurement objects' do + expect(graphql_data.dig('instanceStatisticsMeasurements', 'nodes')).to eq([{ "count" => 10 }, { "count" => 5 }]) + end +end diff --git a/spec/requests/api/graphql/issue/issue_spec.rb b/spec/requests/api/graphql/issue/issue_spec.rb new file mode 100644 index 00000000000..1c9d6b25856 --- /dev/null +++ b/spec/requests/api/graphql/issue/issue_spec.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Query.issue(id)' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:current_user) { create(:user) } + + let(:issue_data) { graphql_data['issue'] } + + let_it_be(:issue_params) { { 'id' => issue.to_global_id.to_s } } + let(:issue_fields) { all_graphql_fields_for('Issue'.classify) } + + let(:query) do + graphql_query_for('issue', issue_params, issue_fields) + end + + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + context 'when the user does not have access to the issue' do + it 'returns nil' do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + + post_graphql(query) + + expect(issue_data).to be nil + end + end + + context 'when the user does have access' do + before do + project.add_guest(current_user) + end + + it 'returns the issue' do + post_graphql(query, current_user: current_user) + + expect(issue_data).to include( + 'title' => issue.title, + 'description' => issue.description + ) + end + + context 'selecting any single field' do + where(:field) do + scalar_fields_of('Issue').map { |name| [name] } + end + + with_them do + it_behaves_like 'a working graphql query' do + let(:issue_fields) do + field + end + + before do + post_graphql(query, current_user: current_user) + end + + it "returns the Issue and field #{params['field']}" do + expect(issue_data.keys).to eq([field]) + end + end + end + end + + context 'selecting multiple fields' do + let(:issue_fields) { %w(title description) } + + it 'returns the Issue with the specified fields' do + post_graphql(query, current_user: current_user) + + expect(issue_data.keys).to eq( %w(title description) ) + expect(issue_data['title']).to eq(issue.title) + expect(issue_data['description']).to eq(issue.description) + end + end + + context 'when passed a non-Issue gid' do + let(:mr) {create(:merge_request)} + + it 'returns an error' do + gid = mr.to_global_id.to_s + issue_params['id'] = gid + + post_graphql(query, current_user: current_user) + + expect(graphql_errors).not_to be nil + expect(graphql_errors.first['message']).to eq("\"#{gid}\" does not represent an instance of Issue") + end + end + end + + context 'when there is a confidential issue' do + let!(:confidential_issue) do + create(:issue, :confidential, project: project) + end + + let(:issue_params) { { 'id' => confidential_issue.to_global_id.to_s } } + + context 'when the user cannot see confidential issues' do + it 'returns nil ' do + post_graphql(query, current_user: current_user) + + expect(issue_data).to be nil + end + end + + context 'when the user can see confidential issues' do + it 'returns the confidential issue' do + project.add_developer(current_user) + + post_graphql(query, current_user: current_user) + + expect(graphql_data.count).to eq(1) + expect(issue_data['confidential']).to be(true) + end + end + end +end diff --git a/spec/requests/api/graphql/metrics/dashboard_query_spec.rb b/spec/requests/api/graphql/metrics/dashboard_query_spec.rb index 456b0a5dea1..e01f59ee6a0 100644 --- a/spec/requests/api/graphql/metrics/dashboard_query_spec.rb +++ b/spec/requests/api/graphql/metrics/dashboard_query_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'Getting Metrics Dashboard' do let_it_be(:current_user) { create(:user) } let(:project) { create(:project) } - let!(:environment) { create(:environment, project: project) } + let(:environment) { create(:environment, project: project) } let(:query) do graphql_query_for( @@ -25,73 +25,156 @@ RSpec.describe 'Getting Metrics Dashboard' do ) end - context 'for anonymous user' do + context 'with metrics_dashboard_exhaustive_validations feature flag off' do before do - post_graphql(query, current_user: current_user) + stub_feature_flags(metrics_dashboard_exhaustive_validations: false) end - context 'requested dashboard is available' do - let(:path) { 'config/prometheus/common_metrics.yml' } + context 'for anonymous user' do + before do + post_graphql(query, current_user: current_user) + end + + context 'requested dashboard is available' do + let(:path) { 'config/prometheus/common_metrics.yml' } + + it_behaves_like 'a working graphql query' + + it 'returns nil' do + dashboard = graphql_data.dig('project', 'environments', 'nodes') + + expect(dashboard).to be_nil + end + end + end + + context 'for user with developer access' do + before do + project.add_developer(current_user) + post_graphql(query, current_user: current_user) + end + + context 'requested dashboard is available' do + let(:path) { 'config/prometheus/common_metrics.yml' } + + it_behaves_like 'a working graphql query' + + it 'returns metrics dashboard' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') + + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => nil) + end + + context 'invalid dashboard' do + let(:path) { '.gitlab/dashboards/metrics.yml' } + let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "---\ndashboard: 'test'" }) } + + it 'returns metrics dashboard' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') + + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["panel_groups: should be an array of panel_groups objects"]) + end + end + + context 'empty dashboard' do + let(:path) { '.gitlab/dashboards/metrics.yml' } + let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "" }) } + + it 'returns metrics dashboard' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') - it_behaves_like 'a working graphql query' + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["dashboard: can't be blank", "panel_groups: should be an array of panel_groups objects"]) + end + end + end + + context 'requested dashboard can not be found' do + let(:path) { 'config/prometheus/i_am_not_here.yml' } - it 'returns nil' do - dashboard = graphql_data.dig('project', 'environments', 'nodes') + it_behaves_like 'a working graphql query' - expect(dashboard).to be_nil + it 'returns nil' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') + + expect(dashboard).to be_nil + end end end end - context 'for user with developer access' do + context 'with metrics_dashboard_exhaustive_validations feature flag on' do before do - project.add_developer(current_user) - post_graphql(query, current_user: current_user) + stub_feature_flags(metrics_dashboard_exhaustive_validations: true) end - context 'requested dashboard is available' do - let(:path) { 'config/prometheus/common_metrics.yml' } + context 'for anonymous user' do + before do + post_graphql(query, current_user: current_user) + end + + context 'requested dashboard is available' do + let(:path) { 'config/prometheus/common_metrics.yml' } + + it_behaves_like 'a working graphql query' - it_behaves_like 'a working graphql query' + it 'returns nil' do + dashboard = graphql_data.dig('project', 'environments', 'nodes') - it 'returns metrics dashboard' do - dashboard = graphql_data.dig('project', 'environments', 'nodes')[0]['metricsDashboard'] + expect(dashboard).to be_nil + end + end + end - expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => nil) + context 'for user with developer access' do + before do + project.add_developer(current_user) + post_graphql(query, current_user: current_user) end - context 'invalid dashboard' do - let(:path) { '.gitlab/dashboards/metrics.yml' } - let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "---\ndashboard: 'test'" }) } + context 'requested dashboard is available' do + let(:path) { 'config/prometheus/common_metrics.yml' } + + it_behaves_like 'a working graphql query' it 'returns metrics dashboard' do dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') - expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["panel_groups: should be an array of panel_groups objects"]) + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => nil) end - end - context 'empty dashboard' do - let(:path) { '.gitlab/dashboards/metrics.yml' } - let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "" }) } + context 'invalid dashboard' do + let(:path) { '.gitlab/dashboards/metrics.yml' } + let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "---\ndashboard: 'test'" }) } - it 'returns metrics dashboard' do - dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') + it 'returns metrics dashboard' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') - expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["dashboard: can't be blank", "panel_groups: should be an array of panel_groups objects"]) + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["root is missing required keys: panel_groups"]) + end + end + + context 'empty dashboard' do + let(:path) { '.gitlab/dashboards/metrics.yml' } + let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "" }) } + + it 'returns metrics dashboard' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') + + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["root is missing required keys: dashboard, panel_groups"]) + end end end - end - context 'requested dashboard can not be found' do - let(:path) { 'config/prometheus/i_am_not_here.yml' } + context 'requested dashboard can not be found' do + let(:path) { 'config/prometheus/i_am_not_here.yml' } - it_behaves_like 'a working graphql query' + it_behaves_like 'a working graphql query' - it 'returns nil' do - dashboard = graphql_data.dig('project', 'environments', 'nodes')[0]['metricsDashboard'] + it 'returns nil' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') - expect(dashboard).to be_nil + expect(dashboard).to be_nil + end end end end 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 1891300dace..1d38bb39d59 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb @@ -32,9 +32,7 @@ RSpec.describe 'Adding an AwardEmoji' do context 'when the user does not have permission' do it_behaves_like 'a mutation that does not create an AwardEmoji' - - it_behaves_like 'a mutation that returns top-level errors', - errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + it_behaves_like 'a mutation that returns a top-level access error' end context 'when the user has permission' do diff --git a/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb index 665b511abb8..c6e8800de1f 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb @@ -33,9 +33,7 @@ RSpec.describe 'Removing an AwardEmoji' do shared_examples 'a mutation that does not authorize the user' do it_behaves_like 'a mutation that does not destroy an AwardEmoji' - - it_behaves_like 'a mutation that returns top-level errors', - errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + it_behaves_like 'a mutation that returns a top-level access error' end context 'when the current_user does not own the award emoji' do 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 ab4a213fde3..2df59ce97ca 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb @@ -143,8 +143,6 @@ RSpec.describe 'Toggling an AwardEmoji' do context 'when the user does not have permission' do it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' - - it_behaves_like 'a mutation that returns top-level errors', - errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + it_behaves_like 'a mutation that returns a top-level access error' end end diff --git a/spec/requests/api/graphql/mutations/boards/destroy_spec.rb b/spec/requests/api/graphql/mutations/boards/destroy_spec.rb new file mode 100644 index 00000000000..a6d894e698d --- /dev/null +++ b/spec/requests/api/graphql/mutations/boards/destroy_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::Boards::Destroy do + include GraphqlHelpers + + let_it_be(:current_user, reload: true) { create(:user) } + let_it_be(:project, reload: true) { create(:project) } + let_it_be(:board) { create(:board, project: project) } + let_it_be(:other_board) { create(:board, project: project) } + let(:mutation) do + variables = { + id: GitlabSchema.id_from_object(board).to_s + } + + graphql_mutation(:destroy_board, variables) + end + + subject { post_graphql_mutation(mutation, current_user: current_user) } + + def mutation_response + graphql_mutation_response(:destroy_board) + end + + context 'when the user does not have permission' do + it_behaves_like 'a mutation that returns a top-level access error' + + it 'does not destroy the board' do + expect { subject }.not_to change { Board.count } + end + end + + context 'when the user has permission' do + before do + project.add_maintainer(current_user) + end + + context 'when given id is not for a board' do + let_it_be(:board) { build_stubbed(:issue, project: project) } + + it 'returns an error' do + subject + + expect(graphql_errors.first['message']).to include('does not represent an instance of Board') + end + end + + context 'when everything is ok' do + it 'destroys the board' do + expect { subject }.to change { Board.count }.from(2).to(1) + end + + it 'returns an empty board' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('board') + expect(mutation_response['board']).to be_nil + end + end + + context 'when there is only 1 board for the parent' do + before 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 + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/boards/lists/create_spec.rb b/spec/requests/api/graphql/mutations/boards/lists/create_spec.rb new file mode 100644 index 00000000000..328f4fb7b6e --- /dev/null +++ b/spec/requests/api/graphql/mutations/boards/lists/create_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Create a label or backlog board list' do + include GraphqlHelpers + + let_it_be(:group) { create(:group, :private) } + let_it_be(:board) { create(:board, group: group) } + let_it_be(:user) { create(:user) } + let_it_be(:dev_label) do + create(:group_label, title: 'Development', color: '#FFAABB', group: group) + end + + let(:current_user) { user } + let(:mutation) { graphql_mutation(:board_list_create, input) } + let(:mutation_response) { graphql_mutation_response(:board_list_create) } + + context 'the user is not allowed to read board lists' do + let(:input) { { board_id: board.to_global_id.to_s, backlog: true } } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permissions to admin board lists' do + before do + group.add_reporter(current_user) + end + + describe 'backlog list' do + let(:input) { { board_id: board.to_global_id.to_s, backlog: true } } + + it 'creates the list' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['list']) + .to include('position' => nil, 'listType' => 'backlog') + end + end + + describe 'label list' do + let(:input) { { board_id: board.to_global_id.to_s, label_id: dev_label.to_global_id.to_s } } + + it 'creates the list' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['list']) + .to include('position' => 0, 'listType' => 'label', 'label' => include('title' => 'Development')) + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/boards/lists/update_spec.rb b/spec/requests/api/graphql/mutations/boards/lists/update_spec.rb index 8a6d2cb3994..8e24e053211 100644 --- a/spec/requests/api/graphql/mutations/boards/lists/update_spec.rb +++ b/spec/requests/api/graphql/mutations/boards/lists/update_spec.rb @@ -15,8 +15,7 @@ RSpec.describe 'Update of an existing board list' do let(:mutation_response) { graphql_mutation_response(:update_board_list) } context 'the user is not allowed to read board lists' do - it_behaves_like 'a mutation that returns top-level errors', - errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + it_behaves_like 'a mutation that returns a top-level access error' end before do diff --git a/spec/requests/api/graphql/mutations/branches/create_spec.rb b/spec/requests/api/graphql/mutations/branches/create_spec.rb index 082b445bf3e..fc09f57a389 100644 --- a/spec/requests/api/graphql/mutations/branches/create_spec.rb +++ b/spec/requests/api/graphql/mutations/branches/create_spec.rb @@ -15,8 +15,7 @@ RSpec.describe 'Creation of a new branch' do let(:mutation_response) { graphql_mutation_response(:create_branch) } context 'the user is not allowed to create a branch' do - it_behaves_like 'a mutation that returns top-level errors', - errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + it_behaves_like 'a mutation that returns a top-level access error' end context 'when user has permissions to create a branch' do diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb new file mode 100644 index 00000000000..a20ac823550 --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'PipelineCancel' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } + + let(:mutation) do + variables = { + id: pipeline.to_global_id.to_s + } + graphql_mutation(:pipeline_cancel, variables, 'errors') + end + + let(:mutation_response) { graphql_mutation_response(:pipeline_cancel) } + + before_all do + project.add_maintainer(user) + end + + it 'does not cancel any pipelines not owned by the current user' do + build = create(:ci_build, :running, pipeline: pipeline) + + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + expect(build).not_to be_canceled + end + + it 'returns a error if the pipline cannot be be canceled' do + build = create(:ci_build, :success, pipeline: pipeline) + + post_graphql_mutation(mutation, current_user: user) + + expect(mutation_response).to include('errors' => include(eq 'Pipeline is not cancelable')) + expect(build).not_to be_canceled + end + + it "cancels all cancelable builds from a pipeline" do + build = create(:ci_build, :running, pipeline: pipeline) + + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(build.reload).to be_canceled + end +end diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb new file mode 100644 index 00000000000..08959d354e2 --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'PipelineDestroy' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:user) { project.owner } + let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project, user: user) } + + let(:mutation) do + variables = { + id: pipeline.to_global_id.to_s + } + graphql_mutation(:pipeline_destroy, variables, 'errors') + end + + it 'returns an error if the user is not allowed to destroy the pipeline' do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + end + + it 'destroys a pipeline' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect { pipeline.reload }.to raise_error(ActiveRecord::RecordNotFound) + end +end diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_retry_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_retry_spec.rb new file mode 100644 index 00000000000..f6acf29c321 --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/pipeline_retry_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'PipelineRetry' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } + + let(:mutation) do + variables = { + id: pipeline.to_global_id.to_s + } + graphql_mutation(:pipeline_retry, variables, + <<-QL + errors + pipeline { + id + } + QL + ) + end + + let(:mutation_response) { graphql_mutation_response(:pipeline_retry) } + + before_all do + project.add_maintainer(user) + end + + it 'returns an error if the user is not allowed to retry the pipeline' do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + end + + it 'retries a pipeline' do + pipeline_id = ::Gitlab::GlobalId.build(pipeline, id: pipeline.id).to_s + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['pipeline']['id']).to eq(pipeline_id) + end +end diff --git a/spec/requests/api/graphql/mutations/commits/create_spec.rb b/spec/requests/api/graphql/mutations/commits/create_spec.rb index 9e4a96700bb..ac4fa7cfe83 100644 --- a/spec/requests/api/graphql/mutations/commits/create_spec.rb +++ b/spec/requests/api/graphql/mutations/commits/create_spec.rb @@ -24,8 +24,7 @@ RSpec.describe 'Creation of a new commit' do let(:mutation_response) { graphql_mutation_response(:commit_create) } context 'the user is not allowed to create a commit' do - it_behaves_like 'a mutation that returns top-level errors', - errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + it_behaves_like 'a mutation that returns a top-level access error' end context 'when user has permissions to create a commit' do diff --git a/spec/requests/api/graphql/mutations/design_management/upload_spec.rb b/spec/requests/api/graphql/mutations/design_management/upload_spec.rb index 9a9c7107b20..2189ae3c519 100644 --- a/spec/requests/api/graphql/mutations/design_management/upload_spec.rb +++ b/spec/requests/api/graphql/mutations/design_management/upload_spec.rb @@ -12,11 +12,11 @@ RSpec.describe "uploading designs" do let(:files) { [fixture_file_upload("spec/fixtures/dk.png")] } let(:variables) { {} } - let(:mutation) do + def mutation input = { project_path: project.full_path, iid: issue.iid, - files: files + files: files.dup }.merge(variables) graphql_mutation(:design_management_upload, input) end @@ -30,31 +30,15 @@ RSpec.describe "uploading designs" do end it "returns an error if the user is not allowed to upload designs" do - post_graphql_mutation(mutation, current_user: create(:user)) + post_graphql_mutation_with_uploads(mutation, current_user: create(:user)) expect(graphql_errors).to be_present end - it "succeeds (backward compatibility)" do - post_graphql_mutation(mutation, current_user: current_user) + it "succeeds, and responds with the created designs" do + post_graphql_mutation_with_uploads(mutation, current_user: current_user) expect(graphql_errors).not_to be_present - end - - it 'succeeds' do - file_path_in_params = ['designManagementUploadInput', 'files', 0] - params = mutation_to_apollo_uploads_param(mutation, files: [file_path_in_params]) - - workhorse_post_with_file(api('/', current_user, version: 'graphql'), - params: params, - file_key: '1' - ) - - expect(graphql_errors).not_to be_present - end - - it "responds with the created designs" do - post_graphql_mutation(mutation, current_user: current_user) expect(mutation_response).to include( "designs" => a_collection_containing_exactly( @@ -65,7 +49,7 @@ RSpec.describe "uploading designs" do it "can respond with skipped designs" do 2.times do - post_graphql_mutation(mutation, current_user: current_user) + post_graphql_mutation_with_uploads(mutation, current_user: current_user) files.each(&:rewind) end @@ -80,7 +64,7 @@ RSpec.describe "uploading designs" do let(:variables) { { iid: "123" } } it "returns an error" do - post_graphql_mutation(mutation, current_user: create(:user)) + post_graphql_mutation_with_uploads(mutation, current_user: create(:user)) expect(graphql_errors).not_to be_empty end @@ -92,7 +76,7 @@ RSpec.describe "uploading designs" do expect(service).to receive(:execute).and_return({ status: :error, message: "Something went wrong" }) end - post_graphql_mutation(mutation, current_user: current_user) + post_graphql_mutation_with_uploads(mutation, current_user: current_user) expect(mutation_response["errors"].first).to eq("Something went wrong") end end diff --git a/spec/requests/api/graphql/mutations/discussions/toggle_resolve_spec.rb b/spec/requests/api/graphql/mutations/discussions/toggle_resolve_spec.rb index 457c37e900b..450996bf76b 100644 --- a/spec/requests/api/graphql/mutations/discussions/toggle_resolve_spec.rb +++ b/spec/requests/api/graphql/mutations/discussions/toggle_resolve_spec.rb @@ -20,8 +20,7 @@ RSpec.describe 'Toggling the resolve status of a discussion' do context 'when the user does not have permission' do let_it_be(:current_user) { create(:user) } - it_behaves_like 'a mutation that returns top-level errors', - errors: ["The resource that you are attempting to access does not exist or you don't have permission to perform this action"] + it_behaves_like 'a mutation that returns a top-level access error' end context 'when user has permission' do diff --git a/spec/requests/api/graphql/mutations/issues/set_locked_spec.rb b/spec/requests/api/graphql/mutations/issues/set_locked_spec.rb index f1d55430e02..4989d096925 100644 --- a/spec/requests/api/graphql/mutations/issues/set_locked_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/set_locked_spec.rb @@ -32,12 +32,7 @@ RSpec.describe 'Setting an issue as locked' do end context 'when the user is not allowed to update the issue' do - it 'returns an error' do - error = "The resource that you are attempting to access does not exist or you don't have permission to perform this action" - post_graphql_mutation(mutation, current_user: current_user) - - expect(graphql_errors).to include(a_hash_including('message' => error)) - end + it_behaves_like 'a mutation that returns a top-level access error' end context 'when user is allowed to update the issue' do diff --git a/spec/requests/api/graphql/mutations/issues/set_severity_spec.rb b/spec/requests/api/graphql/mutations/issues/set_severity_spec.rb new file mode 100644 index 00000000000..96fd2368765 --- /dev/null +++ b/spec/requests/api/graphql/mutations/issues/set_severity_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Setting severity level of an incident' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let(:incident) { create(:incident) } + let(:project) { incident.project } + let(:input) { { severity: 'CRITICAL' } } + + let(:mutation) do + variables = { + project_path: project.full_path, + iid: incident.iid.to_s + } + + graphql_mutation(:issue_set_severity, variables.merge(input), + <<-QL.strip_heredoc + clientMutationId + errors + issue { + iid + severity + } + QL + ) + end + + def mutation_response + graphql_mutation_response(:issue_set_severity) + end + + context 'when the user is not allowed to update the incident' do + it 'returns an error' do + error = "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to include(a_hash_including('message' => error)) + end + end + + context 'when the user is allowed to update the incident' do + before do + project.add_developer(user) + end + + it 'updates the issue' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response.dig('issue', 'severity')).to eq('CRITICAL') + end + end +end diff --git a/spec/requests/api/graphql/mutations/issues/update_spec.rb b/spec/requests/api/graphql/mutations/issues/update_spec.rb index fd983c683be..af52f9d57a3 100644 --- a/spec/requests/api/graphql/mutations/issues/update_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/update_spec.rb @@ -20,8 +20,7 @@ RSpec.describe 'Update of an existing issue' do let(:mutation_response) { graphql_mutation_response(:update_issue) } context 'the user is not allowed to update issue' do - it_behaves_like 'a mutation that returns top-level errors', - errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + it_behaves_like 'a mutation that returns a top-level access error' end context 'when user has permissions to update issue' do diff --git a/spec/requests/api/graphql/mutations/merge_requests/create_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/create_spec.rb index 9297ca054c7..bf759521dc0 100644 --- a/spec/requests/api/graphql/mutations/merge_requests/create_spec.rb +++ b/spec/requests/api/graphql/mutations/merge_requests/create_spec.rb @@ -24,8 +24,7 @@ RSpec.describe 'Creation of a new merge request' do let(:mutation_response) { graphql_mutation_response(:merge_request_create) } context 'the user is not allowed to create a branch' do - it_behaves_like 'a mutation that returns top-level errors', - errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + it_behaves_like 'a mutation that returns a top-level access error' end context 'when user has permissions to create a merge request' do diff --git a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb index 0e2da94f0f9..10ca2cf1cf8 100644 --- a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb +++ b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb @@ -101,7 +101,7 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do graphql_mutation(:create_annotation, variables) end - it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab id.'] + it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab ID.'] end end end @@ -188,7 +188,7 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do graphql_mutation(:create_annotation, variables) end - it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab id.'] + it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab ID.'] end end diff --git a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb index 2459a6f3828..7357f3e1e35 100644 --- a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb +++ b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb @@ -45,7 +45,7 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Delete do graphql_mutation(:delete_annotation, variables) end - it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab id.'] + it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab ID.'] end context 'when the delete fails' do diff --git a/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb index e847c46be1b..21da1332465 100644 --- a/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb @@ -44,7 +44,7 @@ RSpec.describe 'Adding a DiffNote' do it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote context do - let(:diff_refs) { build(:merge_request).diff_refs } # Allow fake diff refs so arguments are valid + let(:diff_refs) { build(:commit).diff_refs } # Allow fake diff refs so arguments are valid it_behaves_like 'a Note mutation when the given resource id is not for a Noteable' end diff --git a/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb index 896a398e308..8bc68e6017c 100644 --- a/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb @@ -47,7 +47,7 @@ RSpec.describe 'Adding an image DiffNote' do it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote context do - let(:diff_refs) { build(:merge_request).diff_refs } # Allow fake diff refs so arguments are valid + let(:diff_refs) { build(:commit).diff_refs } # Allow fake diff refs so arguments are valid it_behaves_like 'a Note mutation when the given resource id is not for a Noteable' end diff --git a/spec/requests/api/graphql/mutations/notes/destroy_spec.rb b/spec/requests/api/graphql/mutations/notes/destroy_spec.rb index 6002a5b5b9d..49f09fadfea 100644 --- a/spec/requests/api/graphql/mutations/notes/destroy_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/destroy_spec.rb @@ -21,8 +21,7 @@ RSpec.describe 'Destroying a Note' do context 'when the user does not have permission' do let(:current_user) { create(:user) } - it_behaves_like 'a mutation that returns top-level errors', - errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + it_behaves_like 'a mutation that returns a top-level access error' it 'does not destroy the Note' do expect do diff --git a/spec/requests/api/graphql/mutations/notes/update/image_diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/update/image_diff_note_spec.rb index 463a872d95d..0c00906d6bf 100644 --- a/spec/requests/api/graphql/mutations/notes/update/image_diff_note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/update/image_diff_note_spec.rb @@ -59,8 +59,7 @@ RSpec.describe 'Updating an image DiffNote' do context 'when the user does not have permission' do let_it_be(:current_user) { create(:user) } - it_behaves_like 'a mutation that returns top-level errors', - errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + it_behaves_like 'a mutation that returns a top-level access error' it 'does not update the DiffNote' do post_graphql_mutation(mutation, current_user: current_user) diff --git a/spec/requests/api/graphql/mutations/notes/update/note_spec.rb b/spec/requests/api/graphql/mutations/notes/update/note_spec.rb index 0d93afe9434..5a92ffe61b8 100644 --- a/spec/requests/api/graphql/mutations/notes/update/note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/update/note_spec.rb @@ -22,8 +22,7 @@ RSpec.describe 'Updating a Note' do context 'when the user does not have permission' do let_it_be(:current_user) { create(:user) } - it_behaves_like 'a mutation that returns top-level errors', - errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + it_behaves_like 'a mutation that returns a top-level access error' it 'does not update the Note' do post_graphql_mutation(mutation, current_user: current_user) diff --git a/spec/requests/api/graphql/mutations/snippets/create_spec.rb b/spec/requests/api/graphql/mutations/snippets/create_spec.rb index 56a5f4907c1..1bb446de708 100644 --- a/spec/requests/api/graphql/mutations/snippets/create_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/create_spec.rb @@ -7,22 +7,24 @@ RSpec.describe 'Creating a Snippet' do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project) } - let(:content) { 'Initial content' } + let(:description) { 'Initial description' } let(:title) { 'Initial title' } - let(:file_name) { 'Initial file_name' } 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(:actions) { [{ action: action }.merge(file_1), { action: action }.merge(file_2)] } let(:project_path) { nil } let(:uploaded_files) { nil } let(:mutation_vars) do { - content: content, description: description, visibility_level: visibility_level, - file_name: file_name, title: title, project_path: project_path, - uploaded_files: uploaded_files + uploaded_files: uploaded_files, + blob_actions: actions } end @@ -62,24 +64,47 @@ RSpec.describe 'Creating a Snippet' do context 'when the user has permission' do let(:current_user) { user } - context 'with PersonalSnippet' do - it 'creates the Snippet' do + shared_examples 'does not create snippet' do + it 'does not create the Snippet' do expect do subject - end.to change { Snippet.count }.by(1) + end.not_to change { Snippet.count } end - it 'returns the created Snippet' do + it 'does not return Snippet' do subject - expect(mutation_response['snippet']['blob']['richData']).to be_nil - expect(mutation_response['snippet']['blob']['plainData']).to match(content) + expect(mutation_response['snippet']).to be_nil + end + end + + shared_examples 'creates snippet' do + it 'returns the created Snippet' do + expect do + subject + end.to change { Snippet.count }.by(1) + expect(mutation_response['snippet']['title']).to eq(title) expect(mutation_response['snippet']['description']).to eq(description) - expect(mutation_response['snippet']['fileName']).to eq(file_name) expect(mutation_response['snippet']['visibilityLevel']).to eq(visibility_level) - expect(mutation_response['snippet']['project']).to be_nil + expect(mutation_response['snippet']['blobs'][0]['plainData']).to match(file_1[:content]) + expect(mutation_response['snippet']['blobs'][0]['fileName']).to match(file_1[:file_path]) + expect(mutation_response['snippet']['blobs'][1]['plainData']).to match(file_2[:content]) + expect(mutation_response['snippet']['blobs'][1]['fileName']).to match(file_2[:file_path]) end + + context 'when action is invalid' do + let(:file_1) { { filePath: 'example_file1' }} + + it_behaves_like 'a mutation that returns errors in the response', errors: ['Snippet actions have invalid data'] + it_behaves_like 'does not create snippet' + end + + it_behaves_like 'snippet edit usage data counters' + end + + context 'with PersonalSnippet' do + it_behaves_like 'creates snippet' end context 'with ProjectSnippet' do @@ -89,23 +114,7 @@ RSpec.describe 'Creating a Snippet' do project.add_developer(current_user) end - it 'creates the Snippet' do - expect do - subject - end.to change { Snippet.count }.by(1) - end - - it 'returns the created Snippet' do - subject - - expect(mutation_response['snippet']['blob']['richData']).to be_nil - expect(mutation_response['snippet']['blob']['plainData']).to match(content) - expect(mutation_response['snippet']['title']).to eq(title) - expect(mutation_response['snippet']['description']).to eq(description) - expect(mutation_response['snippet']['fileName']).to eq(file_name) - expect(mutation_response['snippet']['visibilityLevel']).to eq(visibility_level) - expect(mutation_response['snippet']['project']['fullPath']).to eq(project_path) - end + it_behaves_like 'creates snippet' context 'when the project path is invalid' do let(:project_path) { 'foobar' } @@ -122,61 +131,8 @@ RSpec.describe 'Creating a Snippet' do it_behaves_like 'a mutation that returns top-level errors', errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] end - end - - shared_examples 'does not create snippet' do - it 'does not create the Snippet' do - expect do - subject - end.not_to change { Snippet.count } - end - - it 'does not return Snippet' do - subject - - expect(mutation_response['snippet']).to be_nil - end - end - - context 'when snippet is created using the files param' do - 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(:actions) { [{ action: action }.merge(file_1), { action: action }.merge(file_2)] } - let(:mutation_vars) do - { - description: description, - visibility_level: visibility_level, - project_path: project_path, - title: title, - blob_actions: actions - } - end - - it 'creates the Snippet' do - expect do - subject - end.to change { Snippet.count }.by(1) - end - - it 'returns the created Snippet' do - subject - expect(mutation_response['snippet']['title']).to eq(title) - expect(mutation_response['snippet']['description']).to eq(description) - expect(mutation_response['snippet']['visibilityLevel']).to eq(visibility_level) - expect(mutation_response['snippet']['blobs'][0]['plainData']).to match(file_1[:content]) - expect(mutation_response['snippet']['blobs'][0]['fileName']).to match(file_1[:file_path]) - expect(mutation_response['snippet']['blobs'][1]['plainData']).to match(file_2[:content]) - expect(mutation_response['snippet']['blobs'][1]['fileName']).to match(file_2[:file_path]) - end - - context 'when action is invalid' do - let(:file_1) { { filePath: 'example_file1' }} - - it_behaves_like 'a mutation that returns errors in the response', errors: ['Snippet actions have invalid data'] - it_behaves_like 'does not create snippet' - end + it_behaves_like 'snippet edit usage data counters' end context 'when there are ActiveRecord validation errors' do @@ -187,7 +143,7 @@ RSpec.describe 'Creating a Snippet' do end context 'when there non ActiveRecord errors' do - let(:file_name) { 'invalid://file/path' } + 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/snippets/destroy_spec.rb b/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb index c861564c66b..b71f87d2702 100644 --- a/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb @@ -56,7 +56,7 @@ RSpec.describe 'Destroying a Snippet' do post_graphql_mutation(mutation, current_user: current_user) expect(graphql_errors) - .to include(a_hash_including('message' => "#{snippet_gid} is not a valid id for Snippet.")) + .to include(a_hash_including('message' => "#{snippet_gid} is not a valid ID for Snippet.")) end it 'does not destroy the Snippet' do diff --git a/spec/requests/api/graphql/mutations/snippets/update_spec.rb b/spec/requests/api/graphql/mutations/snippets/update_spec.rb index 3f39c0ab851..58ce74b9263 100644 --- a/spec/requests/api/graphql/mutations/snippets/update_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/update_spec.rb @@ -12,18 +12,20 @@ RSpec.describe 'Updating a Snippet' do let(:updated_content) { 'Updated content' } let(:updated_description) { 'Updated description' } let(:updated_title) { 'Updated_title' } - let(:updated_file_name) { 'Updated file_name' } let(:current_user) { snippet.author } - + let(:updated_file) { 'CHANGELOG' } + let(:deleted_file) { 'README' } let(:snippet_gid) { GitlabSchema.id_from_object(snippet).to_s } let(:mutation_vars) do { id: snippet_gid, - content: updated_content, description: updated_description, visibility_level: 'public', - file_name: updated_file_name, - title: updated_title + title: updated_title, + blob_actions: [ + { action: :update, filePath: updated_file, content: updated_content }, + { action: :delete, filePath: deleted_file } + ] } end @@ -50,21 +52,32 @@ RSpec.describe 'Updating a Snippet' do end context 'when the user has permission' do - it 'updates the Snippet' do + it 'updates the snippet record' do post_graphql_mutation(mutation, current_user: current_user) expect(snippet.reload.title).to eq(updated_title) end - it 'returns the updated Snippet' do + it 'updates the Snippet' do + blob_to_update = blob_at(updated_file) + blob_to_delete = blob_at(deleted_file) + + expect(blob_to_update.data).not_to eq updated_content + expect(blob_to_delete).to be_present + post_graphql_mutation(mutation, current_user: current_user) - expect(mutation_response['snippet']['blob']['richData']).to be_nil - expect(mutation_response['snippet']['blob']['plainData']).to match(updated_content) - expect(mutation_response['snippet']['title']).to eq(updated_title) - expect(mutation_response['snippet']['description']).to eq(updated_description) - expect(mutation_response['snippet']['fileName']).to eq(updated_file_name) - expect(mutation_response['snippet']['visibilityLevel']).to eq('public') + blob_to_update = blob_at(updated_file) + blob_to_delete = blob_at(deleted_file) + + aggregate_failures do + expect(blob_to_update.data).to eq updated_content + expect(blob_to_delete).to be_nil + expect(blob_in_mutation_response(updated_file)['plainData']).to match(updated_content) + expect(mutation_response['snippet']['title']).to eq(updated_title) + expect(mutation_response['snippet']['description']).to eq(updated_description) + expect(mutation_response['snippet']['visibilityLevel']).to eq('public') + end end context 'when there are ActiveRecord validation errors' do @@ -79,16 +92,29 @@ RSpec.describe 'Updating a Snippet' do end it 'returns the Snippet with its original values' do + blob_to_update = blob_at(updated_file) + blob_to_delete = blob_at(deleted_file) + post_graphql_mutation(mutation, current_user: current_user) - expect(mutation_response['snippet']['blob']['richData']).to be_nil - expect(mutation_response['snippet']['blob']['plainData']).to match(original_content) - expect(mutation_response['snippet']['title']).to eq(original_title) - expect(mutation_response['snippet']['description']).to eq(original_description) - expect(mutation_response['snippet']['fileName']).to eq(original_file_name) - expect(mutation_response['snippet']['visibilityLevel']).to eq('private') + aggregate_failures do + expect(blob_at(updated_file).data).to eq blob_to_update.data + expect(blob_at(deleted_file).data).to eq blob_to_delete.data + expect(blob_in_mutation_response(deleted_file)['plainData']).not_to be_nil + expect(mutation_response['snippet']['title']).to eq(original_title) + expect(mutation_response['snippet']['description']).to eq(original_description) + expect(mutation_response['snippet']['visibilityLevel']).to eq('private') + end end end + + def blob_in_mutation_response(filename) + mutation_response['snippet']['blobs'].select { |blob| blob['name'] == filename }[0] + end + + def blob_at(filename) + snippet.repository.blob_at('HEAD', filename) + end end end @@ -96,6 +122,7 @@ RSpec.describe 'Updating a Snippet' do let(:snippet) do create(:personal_snippet, :private, + :repository, file_name: original_file_name, title: original_title, content: original_content, @@ -104,6 +131,7 @@ RSpec.describe 'Updating a Snippet' do it_behaves_like 'graphql update actions' it_behaves_like 'when the snippet is not found' + it_behaves_like 'snippet edit usage data counters' end describe 'ProjectSnippet' do @@ -111,6 +139,7 @@ RSpec.describe 'Updating a Snippet' do let(:snippet) do create(:project_snippet, :private, + :repository, project: project, author: create(:user), file_name: original_file_name, @@ -145,44 +174,10 @@ RSpec.describe 'Updating a Snippet' do expect(errors.first['message']).to eq(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) end end - end - - it_behaves_like 'when the snippet is not found' - end - - context 'when using the files params' do - let!(:snippet) { create(:personal_snippet, :private, :repository) } - let(:updated_content) { 'updated_content' } - let(:updated_file) { 'CHANGELOG' } - let(:deleted_file) { 'README' } - let(:mutation_vars) do - { - id: snippet_gid, - blob_actions: [ - { action: :update, filePath: updated_file, content: updated_content }, - { action: :delete, filePath: deleted_file } - ] - } - end - it 'updates the Snippet' do - blob_to_update = blob_at(updated_file) - expect(blob_to_update.data).not_to eq updated_content - - blob_to_delete = blob_at(deleted_file) - expect(blob_to_delete).to be_present - - post_graphql_mutation(mutation, current_user: current_user) - - blob_to_update = blob_at(updated_file) - expect(blob_to_update.data).to eq updated_content - - blob_to_delete = blob_at(deleted_file) - expect(blob_to_delete).to be_nil + it_behaves_like 'snippet edit usage data counters' end - def blob_at(filename) - snippet.repository.blob_at('HEAD', filename) - end + it_behaves_like 'when the snippet is not found' end end diff --git a/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb b/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb index ed5552f3e30..705ef28ffd4 100644 --- a/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb +++ b/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb @@ -59,7 +59,6 @@ RSpec.describe 'Marking all todos done' do context 'when user is not logged in' do let(:current_user) { nil } - it_behaves_like 'a mutation that returns top-level errors', - errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + it_behaves_like 'a mutation that returns a top-level access error' end end diff --git a/spec/requests/api/graphql/mutations/todos/mark_done_spec.rb b/spec/requests/api/graphql/mutations/todos/mark_done_spec.rb index 9c4733f6769..8bf8b96aff5 100644 --- a/spec/requests/api/graphql/mutations/todos/mark_done_spec.rb +++ b/spec/requests/api/graphql/mutations/todos/mark_done_spec.rb @@ -63,14 +63,11 @@ RSpec.describe 'Marking todos done' do context 'when todo does not belong to requesting user' do let(:input) { { id: other_user_todo.to_global_id.to_s } } - let(:access_error) { 'The resource that you are attempting to access does not exist or you don\'t have permission to perform this action' } - it 'contains the expected error' do - post_graphql_mutation(mutation, current_user: current_user) + it_behaves_like 'a mutation that returns a top-level access error' - errors = json_response['errors'] - expect(errors).not_to be_blank - expect(errors.first['message']).to eq(access_error) + it 'results in the correct todo states' do + post_graphql_mutation(mutation, current_user: current_user) expect(todo1.reload.state).to eq('pending') expect(todo2.reload.state).to eq('done') @@ -80,7 +77,7 @@ RSpec.describe 'Marking todos done' do context 'when using an invalid gid' do let(:input) { { id: 'invalid_gid' } } - let(:invalid_gid_error) { 'invalid_gid is not a valid GitLab id.' } + let(:invalid_gid_error) { 'invalid_gid is not a valid GitLab ID.' } it 'contains the expected error' do post_graphql_mutation(mutation, current_user: current_user) diff --git a/spec/requests/api/graphql/mutations/todos/restore_spec.rb b/spec/requests/api/graphql/mutations/todos/restore_spec.rb index 6dedde56e13..8451dcdf587 100644 --- a/spec/requests/api/graphql/mutations/todos/restore_spec.rb +++ b/spec/requests/api/graphql/mutations/todos/restore_spec.rb @@ -63,14 +63,11 @@ RSpec.describe 'Restoring Todos' do context 'when todo does not belong to requesting user' do let(:input) { { id: other_user_todo.to_global_id.to_s } } - let(:access_error) { 'The resource that you are attempting to access does not exist or you don\'t have permission to perform this action' } - it 'contains the expected error' do - post_graphql_mutation(mutation, current_user: current_user) + it_behaves_like 'a mutation that returns a top-level access error' - errors = json_response['errors'] - expect(errors).not_to be_blank - expect(errors.first['message']).to eq(access_error) + it 'results in the correct todo states' do + post_graphql_mutation(mutation, current_user: current_user) expect(todo1.reload.state).to eq('done') expect(todo2.reload.state).to eq('pending') @@ -80,7 +77,7 @@ RSpec.describe 'Restoring Todos' do context 'when using an invalid gid' do let(:input) { { id: 'invalid_gid' } } - let(:invalid_gid_error) { 'invalid_gid is not a valid GitLab id.' } + let(:invalid_gid_error) { 'invalid_gid is not a valid GitLab ID.' } it 'contains the expected error' do post_graphql_mutation(mutation, current_user: current_user) diff --git a/spec/requests/api/graphql/namespace/projects_spec.rb b/spec/requests/api/graphql/namespace/projects_spec.rb index 0b634e6b689..03160719389 100644 --- a/spec/requests/api/graphql/namespace/projects_spec.rb +++ b/spec/requests/api/graphql/namespace/projects_spec.rb @@ -78,4 +78,43 @@ RSpec.describe 'getting projects' do it_behaves_like 'a graphql namespace' end + + describe 'sorting and pagination' do + let(:data_path) { [:namespace, :projects] } + + def pagination_query(params, page_info) + graphql_query_for( + 'namespace', + { 'fullPath' => subject.full_path }, + <<~QUERY + projects(includeSubgroups: #{include_subgroups}, search: "#{search}", #{params}) { + #{page_info} edges { + node { + #{all_graphql_fields_for('Project')} + } + } + } + QUERY + ) + end + + def pagination_results_data(data) + data.map { |project| project.dig('node', 'name') } + end + + context 'when sorting by similarity' do + let!(:project_1) { create(:project, name: 'Project', path: 'project', namespace: subject) } + let!(:project_2) { create(:project, name: 'Test Project', path: 'test-project', namespace: subject) } + let!(:project_3) { create(:project, name: 'Test', path: 'test', namespace: subject) } + let!(:project_4) { create(:project, name: 'Test Project Other', path: 'other-test-project', namespace: subject) } + let(:search) { 'test' } + let(:current_user) { user } + + it_behaves_like 'sorted paginated query' do + let(:sort_param) { 'SIMILARITY' } + let(:first_param) { 2 } + let(:expected_results) { [project_3.name, project_2.name, project_4.name] } + end + end + end end diff --git a/spec/requests/api/graphql/project/issue/designs/notes_spec.rb b/spec/requests/api/graphql/project/issue/designs/notes_spec.rb index ae5c8363d0f..65191e057c7 100644 --- a/spec/requests/api/graphql/project/issue/designs/notes_spec.rb +++ b/spec/requests/api/graphql/project/issue/designs/notes_spec.rb @@ -14,8 +14,6 @@ RSpec.describe 'Getting designs related to an issue' do before do enable_design_management - - note end it_behaves_like 'a working graphql query' do diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb index 06e613a09bc..5d4276f47ca 100644 --- a/spec/requests/api/graphql/project/issues_spec.rb +++ b/spec/requests/api/graphql/project/issues_spec.rb @@ -5,12 +5,13 @@ require 'spec_helper' RSpec.describe 'getting an issue list for a project' do include GraphqlHelpers - let(:project) { create(:project, :repository, :public) } - let(:current_user) { create(:user) } let(:issues_data) { graphql_data['project']['issues']['edges'] } - let!(:issues) do + + let_it_be(:project) { create(:project, :repository, :public) } + let_it_be(:current_user) { create(:user) } + let_it_be(:issues, reload: true) do [create(:issue, project: project, discussion_locked: true), - create(:issue, project: project)] + create(:issue, :with_alert, project: project)] end let(:fields) do @@ -85,7 +86,7 @@ RSpec.describe 'getting an issue list for a project' do end context 'when there is a confidential issue' do - let!(:confidential_issue) do + let_it_be(:confidential_issue) do create(:issue, :confidential, project: project) end @@ -256,9 +257,140 @@ RSpec.describe 'getting an issue list for a project' do end end - def grab_iids(data = issues_data) - data.map do |issue| - issue.dig('node', 'iid').to_i + context 'fetching alert management alert' do + let(:fields) do + <<~QUERY + edges { + node { + iid + alertManagementAlert { + title + } + } + } + QUERY + end + + # Alerts need to have developer permission and above + before do + project.add_developer(current_user) + end + + it 'avoids N+1 queries' do + control = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } + + create(:alert_management_alert, :with_issue, project: project ) + + expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control) + end + + it 'returns the alert data' do + post_graphql(query, current_user: current_user) + + alert_titles = issues_data.map { |issue| issue.dig('node', 'alertManagementAlert', 'title') } + expected_titles = issues.map { |issue| issue.alert_management_alert&.title } + + expect(alert_titles).to contain_exactly(*expected_titles) + end + end + + context 'fetching labels' do + let(:fields) do + <<~QUERY + edges { + node { + id + labels { + nodes { + id + } + } + } + } + QUERY + end + + before do + issues.each do |issue| + # create a label for each issue we have to properly test N+1 + label = create(:label, project: project) + issue.update!(labels: [label]) + end + end + + def response_label_ids(response_data) + response_data.map do |edge| + edge['node']['labels']['nodes'].map { |u| u['id'] } + end.flatten + end + + def labels_as_global_ids(issues) + issues.map(&:labels).flatten.map(&:to_global_id).map(&:to_s) + end + + it 'avoids N+1 queries', :aggregate_failures do + control = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } + expect(issues_data.count).to eq(2) + expect(response_label_ids(issues_data)).to match_array(labels_as_global_ids(issues)) + + new_issues = issues + [create(:issue, project: project, labels: [create(:label, project: project)])] + + expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control) + # graphql_data is memoized (see spec/support/helpers/graphql_helpers.rb) + # so we have to parse the body ourselves the second time + issues_data = Gitlab::Json.parse(response.body)['data']['project']['issues']['edges'] + expect(issues_data.count).to eq(3) + expect(response_label_ids(issues_data)).to match_array(labels_as_global_ids(new_issues)) + end + end + + context 'fetching assignees' do + let(:fields) do + <<~QUERY + edges { + node { + id + assignees { + nodes { + id + } + } + } + } + QUERY + end + + before do + issues.each do |issue| + # create an assignee for each issue we have to properly test N+1 + assignee = create(:user) + issue.update!(assignees: [assignee]) + end + end + + def response_assignee_ids(response_data) + response_data.map do |edge| + edge['node']['assignees']['nodes'].map { |node| node['id'] } + end.flatten + end + + def assignees_as_global_ids(issues) + issues.map(&:assignees).flatten.map(&:to_global_id).map(&:to_s) + end + + it 'avoids N+1 queries', :aggregate_failures do + control = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } + expect(issues_data.count).to eq(2) + expect(response_assignee_ids(issues_data)).to match_array(assignees_as_global_ids(issues)) + + new_issues = issues + [create(:issue, project: project, assignees: [create(:user)])] + + expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control) + # graphql_data is memoized (see spec/support/helpers/graphql_helpers.rb) + # so we have to parse the body ourselves the second time + issues_data = Gitlab::Json.parse(response.body)['data']['project']['issues']['edges'] + expect(issues_data.count).to eq(3) + expect(response_assignee_ids(issues_data)).to match_array(assignees_as_global_ids(new_issues)) end end end diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb index c39358a2db1..fae52fe814d 100644 --- a/spec/requests/api/graphql/project/merge_request_spec.rb +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -124,7 +124,8 @@ RSpec.describe 'getting merge request information nested in a project' do 'removeSourceBranch' => false, 'cherryPickOnCurrentMergeRequest' => false, 'revertOnCurrentMergeRequest' => false, - 'updateMergeRequest' => false + 'updateMergeRequest' => false, + 'canMerge' => false } post_graphql(query, current_user: current_user) diff --git a/spec/requests/api/graphql/project/merge_requests_spec.rb b/spec/requests/api/graphql/project/merge_requests_spec.rb index bb63a5994b0..22b003501a1 100644 --- a/spec/requests/api/graphql/project/merge_requests_spec.rb +++ b/spec/requests/api/graphql/project/merge_requests_spec.rb @@ -8,7 +8,7 @@ RSpec.describe 'getting merge request listings nested in a project' do let_it_be(:project) { create(:project, :repository, :public) } let_it_be(:current_user) { create(:user) } - let_it_be(:label) { create(:label) } + let_it_be(:label) { create(:label, project: project) } let_it_be(:merge_request_a) { create(:labeled_merge_request, :unique_branches, source_project: project, labels: [label]) } let_it_be(:merge_request_b) { create(:merge_request, :closed, :unique_branches, source_project: project) } let_it_be(:merge_request_c) { create(:labeled_merge_request, :closed, :unique_branches, source_project: project, labels: [label]) } @@ -210,4 +210,48 @@ RSpec.describe 'getting merge request listings nested in a project' do include_examples 'N+1 query check' end end + describe 'sorting and pagination' do + let(:data_path) { [:project, :mergeRequests] } + + def pagination_query(params, page_info) + graphql_query_for( + :project, + { full_path: project.full_path }, + <<~QUERY + mergeRequests(#{params}) { + #{page_info} edges { + node { + id + } + } + } + QUERY + ) + end + + def pagination_results_data(data) + data.map { |project| project.dig('node', 'id') } + end + + context 'when sorting by merged_at DESC' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { 'MERGED_AT_DESC' } + let(:first_param) { 2 } + + let(:expected_results) do + [ + merge_request_b, + merge_request_c, + merge_request_d, + merge_request_a + ].map(&:to_gid).map(&:to_s) + end + + before do + merge_request_c.metrics.update!(merged_at: 5.days.ago) + merge_request_b.metrics.update!(merged_at: 1.day.ago) + end + end + end + end end diff --git a/spec/requests/api/graphql/project/release_spec.rb b/spec/requests/api/graphql/project/release_spec.rb index f9c19d9747d..8fce29d0dc6 100644 --- a/spec/requests/api/graphql/project/release_spec.rb +++ b/spec/requests/api/graphql/project/release_spec.rb @@ -10,6 +10,8 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do let_it_be(:guest) { create(:user) } let_it_be(:reporter) { create(:user) } let_it_be(:stranger) { create(:user) } + let_it_be(:link_filepath) { '/direct/asset/link/path' } + let_it_be(:released_at) { Time.now - 1.day } let(:params_for_issues_and_mrs) { { scope: 'all', state: 'opened', release_tag: release.tag } } let(:post_query) { post_graphql(query, current_user: current_user) } @@ -38,6 +40,7 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do name createdAt releasedAt + upcomingRelease }) end @@ -53,7 +56,8 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do 'descriptionHtml' => release.description_html, 'name' => release.name, 'createdAt' => release.created_at.iso8601, - 'releasedAt' => release.released_at.iso8601 + 'releasedAt' => release.released_at.iso8601, + 'upcomingRelease' => false }) end end @@ -127,7 +131,7 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do let(:release_fields) do query_graphql_field(:assets, nil, - query_graphql_field(:links, nil, 'nodes { id name url external }')) + query_graphql_field(:links, nil, 'nodes { id name url external, directAssetUrl }')) end it 'finds all release links' do @@ -138,7 +142,8 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do 'id' => global_id_of(link), 'name' => link.name, 'url' => link.url, - 'external' => link.external? + 'external' => link.external?, + 'directAssetUrl' => link.filepath ? Gitlab::Routing.url_helpers.project_release_url(project, release) << link.filepath : link.url } end @@ -268,9 +273,9 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do let_it_be(:project) { create(:project, :repository, :private) } let_it_be(:milestone_1) { create(:milestone, project: project) } let_it_be(:milestone_2) { create(:milestone, project: project) } - let_it_be(:release) { create(:release, :with_evidence, project: project, milestones: [milestone_1, milestone_2]) } + let_it_be(:release) { create(:release, :with_evidence, project: project, milestones: [milestone_1, milestone_2], released_at: released_at) } let_it_be(:release_link_1) { create(:release_link, release: release) } - let_it_be(:release_link_2) { create(:release_link, release: release) } + let_it_be(:release_link_2) { create(:release_link, release: release, filepath: link_filepath) } before_all do project.add_developer(developer) @@ -309,9 +314,9 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do let_it_be(:project) { create(:project, :repository, :public) } let_it_be(:milestone_1) { create(:milestone, project: project) } let_it_be(:milestone_2) { create(:milestone, project: project) } - let_it_be(:release) { create(:release, :with_evidence, project: project, milestones: [milestone_1, milestone_2]) } + let_it_be(:release) { create(:release, :with_evidence, project: project, milestones: [milestone_1, milestone_2], released_at: released_at) } let_it_be(:release_link_1) { create(:release_link, release: release) } - let_it_be(:release_link_2) { create(:release_link, release: release) } + let_it_be(:release_link_2) { create(:release_link, release: release, filepath: link_filepath) } before_all do project.add_developer(developer) @@ -371,4 +376,45 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do it_behaves_like 'no access to the release field' end end + + describe 'upcoming release' do + let(:path) { path_prefix } + let(:project) { create(:project, :repository, :private) } + let(:release) { create(:release, :with_evidence, project: project, released_at: released_at) } + let(:current_user) { developer } + + let(:release_fields) do + query_graphql_field(%{ + releasedAt + upcomingRelease + }) + end + + before do + project.add_developer(developer) + post_query + end + + context 'future release' do + let(:released_at) { Time.now + 1.day } + + it 'finds all release data' do + expect(data).to eq({ + 'releasedAt' => release.released_at.iso8601, + 'upcomingRelease' => true + }) + end + end + + context 'past release' do + let(:released_at) { Time.now - 1.day } + + it 'finds all release data' do + expect(data).to eq({ + 'releasedAt' => release.released_at.iso8601, + 'upcomingRelease' => false + }) + end + end + end end diff --git a/spec/requests/api/graphql/project/releases_spec.rb b/spec/requests/api/graphql/project/releases_spec.rb index 7e418bbaa5b..7c57c0e9177 100644 --- a/spec/requests/api/graphql/project/releases_spec.rb +++ b/spec/requests/api/graphql/project/releases_spec.rb @@ -14,6 +14,7 @@ RSpec.describe 'Query.project(fullPath).releases()' do graphql_query_for(:project, { fullPath: project.full_path }, %{ releases { + count nodes { tagName tagPath @@ -53,6 +54,20 @@ RSpec.describe 'Query.project(fullPath).releases()' do stub_default_url_options(host: 'www.example.com') end + shared_examples 'correct total count' do + let(:data) { graphql_data.dig('project', 'releases') } + + before do + create_list(:release, 2, project: project) + + post_query + end + + it 'returns the total count' do + expect(data['count']).to eq(project.releases.count) + end + end + shared_examples 'full access to all repository-related fields' do describe 'repository-related fields' do before do @@ -92,6 +107,8 @@ RSpec.describe 'Query.project(fullPath).releases()' do ) end end + + it_behaves_like 'correct total count' end shared_examples 'no access to any repository-related fields' do @@ -119,6 +136,8 @@ RSpec.describe 'Query.project(fullPath).releases()' do ) end end + + it_behaves_like 'correct total count' end # editUrl is tested separately becuase its permissions diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb index c6049e098be..4b8ffb0675c 100644 --- a/spec/requests/api/graphql/project_query_spec.rb +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -5,7 +5,8 @@ require 'spec_helper' RSpec.describe 'getting project information' do include GraphqlHelpers - let(:project) { create(:project, :repository) } + let(:group) { create(:group) } + let(:project) { create(:project, :repository, group: group) } let(:current_user) { create(:user) } let(:query) do @@ -60,6 +61,51 @@ RSpec.describe 'getting project information' do expect(graphql_data['project']['pipelines']['edges'].size).to eq(1) end end + + it 'includes inherited members in project_members' do + group_member = create(:group_member, group: group) + project_member = create(:project_member, project: project) + member_query = <<~GQL + query { + project(fullPath: "#{project.full_path}") { + projectMembers { + nodes { + id + user { + username + } + ... on ProjectMember { + project { + id + } + } + ... on GroupMember { + group { + id + } + } + } + } + } + } + GQL + + post_graphql(member_query, current_user: current_user) + + member_ids = graphql_data.dig('project', 'projectMembers', 'nodes') + expect(member_ids).to include( + a_hash_including( + 'id' => group_member.to_global_id.to_s, + 'group' => { 'id' => group.to_global_id.to_s } + ) + ) + expect(member_ids).to include( + a_hash_including( + 'id' => project_member.to_global_id.to_s, + 'project' => { 'id' => project.to_global_id.to_s } + ) + ) + end end describe 'performance' do diff --git a/spec/requests/api/graphql/user/starred_projects_query_spec.rb b/spec/requests/api/graphql/user/starred_projects_query_spec.rb new file mode 100644 index 00000000000..8a1bd3d172f --- /dev/null +++ b/spec/requests/api/graphql/user/starred_projects_query_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Getting starredProjects of the user' do + include GraphqlHelpers + + let(:query) do + graphql_query_for(:user, user_params, user_fields) + end + + let(:user_params) { { username: user.username } } + + let_it_be(:project_a) { create(:project, :public) } + let_it_be(:project_b) { create(:project, :private) } + let_it_be(:project_c) { create(:project, :private) } + let_it_be(:user, reload: true) { create(:user) } + + let(:user_fields) { 'starredProjects { nodes { id } }' } + let(:starred_projects) { graphql_data_at(:user, :starred_projects, :nodes) } + + before do + project_b.add_reporter(user) + project_c.add_reporter(user) + + user.toggle_star(project_a) + user.toggle_star(project_b) + user.toggle_star(project_c) + + post_graphql(query) + end + + it_behaves_like 'a working graphql query' + + it 'found only public project' do + expect(starred_projects).to contain_exactly( + a_hash_including('id' => global_id_of(project_a)) + ) + end + + context 'the current user is the user' do + let(:current_user) { user } + + before do + post_graphql(query, current_user: current_user) + end + + it 'found all projects' do + expect(starred_projects).to contain_exactly( + a_hash_including('id' => global_id_of(project_a)), + a_hash_including('id' => global_id_of(project_b)), + a_hash_including('id' => global_id_of(project_c)) + ) + end + end + + context 'the current user is a member of a private project the user starred' do + let_it_be(:other_user) { create(:user) } + + before do + project_b.add_reporter(other_user) + + post_graphql(query, current_user: other_user) + end + + it 'finds public and member projects' do + expect(starred_projects).to contain_exactly( + a_hash_including('id' => global_id_of(project_a)), + a_hash_including('id' => global_id_of(project_b)) + ) + end + end +end diff --git a/spec/requests/api/group_packages_spec.rb b/spec/requests/api/group_packages_spec.rb index e02f6099637..f67cafbd8f5 100644 --- a/spec/requests/api/group_packages_spec.rb +++ b/spec/requests/api/group_packages_spec.rb @@ -141,5 +141,7 @@ RSpec.describe API::GroupPackages do it_behaves_like 'returning response status', :bad_request end + + it_behaves_like 'does not cause n^2 queries' end end diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 1fa705423d2..9c0ea14e3e3 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -39,7 +39,7 @@ RSpec.describe API::Helpers do end def error!(message, status, header) - raise Exception.new("#{status} - #{message}") + raise StandardError.new("#{status} - #{message}") end def set_param(key, value) diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 873189af397..4a0a7c81781 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -415,7 +415,7 @@ RSpec.describe API::Internal::Base do let(:env) { {} } around do |example| - Timecop.freeze { example.run } + freeze_time { example.run } end before do @@ -1179,7 +1179,7 @@ RSpec.describe API::Internal::Base do let(:gl_repository) { "snippet-#{personal_snippet.id}" } it 'does not try to notify that project moved' do - allow(Gitlab::GlRepository).to receive(:parse).and_return([personal_snippet, nil, Gitlab::GlRepository::PROJECT]) + allow(Gitlab::GlRepository).to receive(:parse).and_return([personal_snippet, nil, Gitlab::GlRepository::SNIPPET]) expect(Gitlab::Checks::ProjectMoved).not_to receive(:fetch_message) diff --git a/spec/requests/api/internal/kubernetes_spec.rb b/spec/requests/api/internal/kubernetes_spec.rb index 555ca441fe7..f669483b5a4 100644 --- a/spec/requests/api/internal/kubernetes_spec.rb +++ b/spec/requests/api/internal/kubernetes_spec.rb @@ -3,24 +3,97 @@ require 'spec_helper' RSpec.describe API::Internal::Kubernetes do - describe "GET /internal/kubernetes/agent_info" do + let(:jwt_auth_headers) do + jwt_token = JWT.encode({ 'iss' => Gitlab::Kas::JWT_ISSUER }, Gitlab::Kas.secret, 'HS256') + + { Gitlab::Kas::INTERNAL_API_REQUEST_HEADER => jwt_token } + end + + let(:jwt_secret) { SecureRandom.random_bytes(Gitlab::Kas::SECRET_LENGTH) } + + before do + allow(Gitlab::Kas).to receive(:secret).and_return(jwt_secret) + end + + shared_examples 'authorization' do + context 'not authenticated' do + it 'returns 401' do + send_request(headers: { Gitlab::Kas::INTERNAL_API_REQUEST_HEADER => '' }) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + context 'kubernetes_agent_internal_api feature flag disabled' do before do stub_feature_flags(kubernetes_agent_internal_api: false) end it 'returns 404' do - get api('/internal/kubernetes/agent_info') + send_request expect(response).to have_gitlab_http_status(:not_found) end end + end + shared_examples 'agent authentication' do it 'returns 403 if Authorization header not sent' do - get api('/internal/kubernetes/agent_info') + send_request + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'returns 403 if Authorization is for non-existent agent' do + send_request(headers: { 'Authorization' => 'Bearer NONEXISTENT' }) expect(response).to have_gitlab_http_status(:forbidden) end + end + + describe 'POST /internal/kubernetes/usage_metrics' do + def send_request(headers: {}, params: {}) + post api('/internal/kubernetes/usage_metrics'), params: params, headers: headers.reverse_merge(jwt_auth_headers) + end + + include_examples 'authorization' + + context 'is authenticated for an agent' do + let!(:agent_token) { create(:cluster_agent_token) } + + it 'returns no_content for valid gitops_sync_count' do + send_request(params: { gitops_sync_count: 10 }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:no_content) + end + + it 'returns no_content 0 gitops_sync_count' do + send_request(params: { gitops_sync_count: 0 }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:no_content) + end + + it 'returns 400 for non number' do + send_request(params: { gitops_sync_count: 'string' }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns 400 for negative number' do + send_request(params: { gitops_sync_count: '-1' }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + + describe "GET /internal/kubernetes/agent_info" do + def send_request(headers: {}, params: {}) + get api('/internal/kubernetes/agent_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!(:agent_token) { create(:cluster_agent_token) } @@ -29,7 +102,7 @@ RSpec.describe API::Internal::Kubernetes do let(:project) { agent.project } it 'returns expected data', :aggregate_failures do - get api('/internal/kubernetes/agent_info'), headers: { 'Authorization' => "Bearer #{agent_token.token}" } + send_request(headers: { 'Authorization' => "Bearer #{agent_token.token}" }) expect(response).to have_gitlab_http_status(:success) @@ -53,42 +126,15 @@ RSpec.describe API::Internal::Kubernetes do ) end end - - context 'no such agent exists' do - it 'returns 404' do - get api('/internal/kubernetes/agent_info'), headers: { 'Authorization' => 'Bearer ABCD' } - - expect(response).to have_gitlab_http_status(:forbidden) - end - end end describe 'GET /internal/kubernetes/project_info' do - context 'kubernetes_agent_internal_api feature flag disabled' do - before do - stub_feature_flags(kubernetes_agent_internal_api: false) - end - - it 'returns 404' do - get api('/internal/kubernetes/project_info') - - expect(response).to have_gitlab_http_status(:not_found) - end + def send_request(headers: {}, params: {}) + get api('/internal/kubernetes/project_info'), params: params, headers: headers.reverse_merge(jwt_auth_headers) end - it 'returns 403 if Authorization header not sent' do - get api('/internal/kubernetes/project_info') - - expect(response).to have_gitlab_http_status(:forbidden) - end - - context 'no such agent exists' do - it 'returns 404' do - get api('/internal/kubernetes/project_info'), headers: { 'Authorization' => 'Bearer ABCD' } - - expect(response).to have_gitlab_http_status(:forbidden) - end - end + include_examples 'authorization' + include_examples 'agent authentication' context 'an agent is found' do let!(:agent_token) { create(:cluster_agent_token) } @@ -99,7 +145,7 @@ RSpec.describe API::Internal::Kubernetes do let(:project) { create(:project, :public) } it 'returns expected data', :aggregate_failures do - get api('/internal/kubernetes/project_info'), params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" } + send_request(params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) expect(response).to have_gitlab_http_status(:success) @@ -126,7 +172,7 @@ RSpec.describe API::Internal::Kubernetes do let(:project) { create(:project, :private) } it 'returns 404' do - get api('/internal/kubernetes/project_info'), params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" } + send_request(params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) expect(response).to have_gitlab_http_status(:not_found) end @@ -136,7 +182,7 @@ RSpec.describe API::Internal::Kubernetes do let(:project) { create(:project, :internal) } it 'returns 404' do - get api('/internal/kubernetes/project_info'), params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" } + send_request(params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) expect(response).to have_gitlab_http_status(:not_found) end @@ -144,7 +190,7 @@ RSpec.describe API::Internal::Kubernetes do context 'project does not exist' do it 'returns 404' do - get api('/internal/kubernetes/project_info'), params: { id: 0 }, headers: { 'Authorization' => "Bearer #{agent_token.token}" } + send_request(params: { id: 0 }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/api/issue_links_spec.rb b/spec/requests/api/issue_links_spec.rb new file mode 100644 index 00000000000..a4243766111 --- /dev/null +++ b/spec/requests/api/issue_links_spec.rb @@ -0,0 +1,207 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::IssueLinks do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } + + before do + project.add_guest(user) + end + + describe 'GET /links' do + context 'when unauthenticated' do + it 'returns 401' do + get api("/projects/#{project.id}/issues/#{issue.iid}/links") + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when authenticated' do + it 'returns related issues' do + target_issue = create(:issue, project: project) + create(:issue_link, source: issue, target: target_issue) + + get api("/projects/#{project.id}/issues/#{issue.iid}/links", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.length).to eq(1) + expect(response).to match_response_schema('public_api/v4/issue_links') + end + end + end + + describe 'POST /links' do + context 'when unauthenticated' do + it 'returns 401' do + target_issue = create(:issue) + + post api("/projects/#{project.id}/issues/#{issue.iid}/links"), + params: { target_project_id: target_issue.project.id, target_issue_iid: target_issue.iid } + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when authenticated' do + context 'given target project not found' do + it 'returns 404' do + target_issue = create(:issue) + + post api("/projects/#{project.id}/issues/#{issue.iid}/links", user), + params: { target_project_id: -1, target_issue_iid: target_issue.iid } + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Project Not Found') + end + end + + context 'given target issue not found' do + it 'returns 404' do + target_project = create(:project, :public) + + post api("/projects/#{project.id}/issues/#{issue.iid}/links", user), + params: { target_project_id: target_project.id, target_issue_iid: non_existing_record_iid } + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Not found') + end + end + + context 'when user does not have write access to given issue' do + it 'returns 404' do + unauthorized_project = create(:project) + target_issue = create(:issue, project: unauthorized_project) + unauthorized_project.add_guest(user) + + post api("/projects/#{project.id}/issues/#{issue.iid}/links", user), + params: { target_project_id: unauthorized_project.id, target_issue_iid: target_issue.iid } + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('No Issue found for given params') + end + end + + context 'when trying to relate to a confidential issue' do + it 'returns 404' do + project = create(:project, :public) + target_issue = create(:issue, :confidential, project: project) + + post api("/projects/#{project.id}/issues/#{issue.iid}/links", user), + params: { target_project_id: project.id, target_issue_iid: target_issue.iid } + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Not found') + end + end + + context 'when trying to relate to a private project issue' do + it 'returns 404' do + project = create(:project, :private) + target_issue = create(:issue, project: project) + + post api("/projects/#{project.id}/issues/#{issue.iid}/links", user), + params: { target_project_id: project.id, target_issue_iid: target_issue.iid } + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Project Not Found') + end + end + + context 'when user has ability to create an issue link' do + let_it_be(:target_issue) { create(:issue, project: project) } + + before do + project.add_reporter(user) + end + + it 'returns 201 status and contains the expected link response' do + post api("/projects/#{project.id}/issues/#{issue.iid}/links", user), + params: { target_project_id: project.id, target_issue_iid: target_issue.iid, link_type: 'relates_to' } + + expect_link_response(link_type: 'relates_to') + end + + it 'returns 201 when sending full path of target project' do + post api("/projects/#{project.id}/issues/#{issue.iid}/links", user), + params: { target_project_id: project.full_path, target_issue_iid: target_issue.iid } + + expect_link_response + end + + def expect_link_response(link_type: 'relates_to') + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/issue_link') + expect(json_response['link_type']).to eq(link_type) + end + end + end + end + + describe 'DELETE /links/:issue_link_id' do + context 'when unauthenticated' do + it 'returns 401' do + issue_link = create(:issue_link) + + delete api("/projects/#{project.id}/issues/#{issue.iid}/links/#{issue_link.id}") + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when authenticated' do + context 'when user does not have write access to given issue link' do + it 'returns 404' do + unauthorized_project = create(:project) + target_issue = create(:issue, project: unauthorized_project) + issue_link = create(:issue_link, source: issue, target: target_issue) + unauthorized_project.add_guest(user) + + delete api("/projects/#{project.id}/issues/#{issue.iid}/links/#{issue_link.id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('No Issue Link found') + end + end + + context 'issue link not found' do + it 'returns 404' do + delete api("/projects/#{project.id}/issues/#{issue.iid}/links/#{non_existing_record_id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Not found') + end + end + + context 'when trying to delete a link with a private project issue' do + it 'returns 404' do + project = create(:project, :private) + target_issue = create(:issue, project: project) + issue_link = create(:issue_link, source: issue, target: target_issue) + + delete api("/projects/#{project.id}/issues/#{issue.iid}/links/#{issue_link.id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Project Not Found') + end + end + + context 'when user has ability to delete the issue link' do + it 'returns 200' do + target_issue = create(:issue, project: project) + issue_link = create(:issue_link, source: issue, target: target_issue) + project.add_reporter(user) + + delete api("/projects/#{project.id}/issues/#{issue.iid}/links/#{issue_link.id}", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/issue_link') + end + end + end + end +end diff --git a/spec/requests/api/issues/get_group_issues_spec.rb b/spec/requests/api/issues/get_group_issues_spec.rb index b0fbf3bf66d..3870c78deee 100644 --- a/spec/requests/api/issues/get_group_issues_spec.rb +++ b/spec/requests/api/issues/get_group_issues_spec.rb @@ -402,30 +402,76 @@ RSpec.describe API::Issues do expect_paginated_array_response([group_closed_issue.id, group_issue.id]) end - it 'returns an array of labeled group issues' do - get api(base_url, user), params: { labels: group_label.title } + shared_examples 'labels parameter' do + it 'returns an array of labeled group issues' do + get api(base_url, user), params: { labels: group_label.title } - expect_paginated_array_response(group_issue.id) - expect(json_response.first['labels']).to eq([group_label.title]) - end + expect_paginated_array_response(group_issue.id) + expect(json_response.first['labels']).to eq([group_label.title]) + end - it 'returns an array of labeled group issues with labels param as array' do - get api(base_url, user), params: { labels: [group_label.title] } + it 'returns an array of labeled group issues' do + get api(base_url, user), params: { labels: group_label.title } - expect_paginated_array_response(group_issue.id) - expect(json_response.first['labels']).to eq([group_label.title]) + expect_paginated_array_response(group_issue.id) + expect(json_response.first['labels']).to eq([group_label.title]) + end + + it 'returns an array of labeled group issues with labels param as array' do + get api(base_url, user), params: { labels: [group_label.title] } + + expect_paginated_array_response(group_issue.id) + expect(json_response.first['labels']).to eq([group_label.title]) + end + + it 'returns an array of labeled group issues where all labels match' do + get api(base_url, user), params: { labels: "#{group_label.title},foo,bar" } + + expect_paginated_array_response([]) + end + + it 'returns an array of labeled group issues where all labels match with labels param as array' do + get api(base_url, user), params: { labels: [group_label.title, 'foo', 'bar'] } + + expect_paginated_array_response([]) + end + + context 'with labeled issues' do + let(:group_issue2) { create :issue, project: group_project } + let(:label_b) { create(:label, title: 'foo', project: group_project) } + let(:label_c) { create(:label, title: 'bar', project: group_project) } + + before do + create(:label_link, label: group_label, target: group_issue2) + create(:label_link, label: label_b, target: group_issue) + create(:label_link, label: label_b, target: group_issue2) + create(:label_link, label: label_c, target: group_issue) + + get api(base_url, user), params: params + end + + let(:issue) { group_issue } + let(:issue2) { group_issue2 } + let(:label) { group_label } + + it_behaves_like 'labeled issues with labels and label_name params' + end end - it 'returns an array of labeled group issues where all labels match' do - get api(base_url, user), params: { labels: "#{group_label.title},foo,bar" } + context 'when `optimized_issuable_label_filter` feature flag is off' do + before do + stub_feature_flags(optimized_issuable_label_filter: false) + end - expect_paginated_array_response([]) + it_behaves_like 'labels parameter' end - it 'returns an array of labeled group issues where all labels match with labels param as array' do - get api(base_url, user), params: { labels: [group_label.title, 'foo', 'bar'] } + context 'when `optimized_issuable_label_filter` feature flag is on' do + before do + stub_feature_flags(optimized_issuable_label_filter: true) + end - expect_paginated_array_response([]) + it_behaves_like 'labels parameter' end it 'returns issues matching given search string for title' do @@ -440,27 +486,6 @@ RSpec.describe API::Issues do expect_paginated_array_response(group_issue.id) end - context 'with labeled issues' do - let(:group_issue2) { create :issue, project: group_project } - let(:label_b) { create(:label, title: 'foo', project: group_project) } - let(:label_c) { create(:label, title: 'bar', project: group_project) } - - before do - create(:label_link, label: group_label, target: group_issue2) - create(:label_link, label: label_b, target: group_issue) - create(:label_link, label: label_b, target: group_issue2) - create(:label_link, label: label_c, target: group_issue) - - get api(base_url, user), params: params - end - - let(:issue) { group_issue } - let(:issue2) { group_issue2 } - let(:label) { group_label } - - it_behaves_like 'labeled issues with labels and label_name params' - end - context 'with archived projects' do let_it_be(:archived_issue) do create( diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index b638a65d65e..b8cbddd9ed4 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -87,6 +87,46 @@ RSpec.describe API::Issues do end end + describe 'GET /issues/:id' do + context 'when unauthorized' do + it 'returns unauthorized' do + get api("/issues/#{issue.id}" ) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when authorized' do + context 'as a normal user' do + it 'returns forbidden' do + get api("/issues/#{issue.id}", user ) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'as an admin' do + context 'when issue exists' do + it 'returns the issue' do + get api("/issues/#{issue.id}", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.dig('author', 'id')).to eq(issue.author.id) + expect(json_response['description']).to eq(issue.description) + end + end + + context 'when issue does not exist' do + it 'returns 404' do + get api("/issues/0", admin) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + end + describe 'GET /issues' do context 'when unauthenticated' do it 'returns an array of all issues' do @@ -128,6 +168,11 @@ RSpec.describe API::Issues do expect_paginated_array_response([issue.id, closed_issue.id]) end + it 'responds with a 401 instead of the specified issue' do + get api("/issues/#{issue.id}") + expect(response).to have_gitlab_http_status(:unauthorized) + end + context 'issues_statistics' do it 'returns authentication error without any scope' do get api('/issues_statistics') diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 77d5a4f26a8..2d57146fbc9 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -5,32 +5,6 @@ require 'spec_helper' RSpec.describe API::Jobs do include HttpIOHelpers - shared_examples 'a job with artifacts and trace' do |result_is_array: true| - context 'with artifacts and trace' do - let!(:second_job) { create(:ci_build, :trace_artifact, :artifacts, :test_reports, pipeline: pipeline) } - - it 'returns artifacts and trace data', :skip_before_request do - get api(api_endpoint, api_user) - json_job = result_is_array ? json_response.select { |job| job['id'] == second_job.id }.first : json_response - - expect(json_job['artifacts_file']).not_to be_nil - expect(json_job['artifacts_file']).not_to be_empty - expect(json_job['artifacts_file']['filename']).to eq(second_job.artifacts_file.filename) - expect(json_job['artifacts_file']['size']).to eq(second_job.artifacts_file.size) - expect(json_job['artifacts']).not_to be_nil - expect(json_job['artifacts']).to be_an Array - expect(json_job['artifacts'].size).to eq(second_job.job_artifacts.length) - json_job['artifacts'].each do |artifact| - expect(artifact).not_to be_nil - file_type = Ci::JobArtifact.file_types[artifact['file_type']] - expect(artifact['size']).to eq(second_job.job_artifacts.find_by(file_type: file_type).size) - expect(artifact['filename']).to eq(second_job.job_artifacts.find_by(file_type: file_type).filename) - expect(artifact['file_format']).to eq(second_job.job_artifacts.find_by(file_type: file_type).file_format) - end - end - end - end - let_it_be(:project, reload: true) do create(:project, :repository, public_builds: false) end @@ -56,7 +30,7 @@ RSpec.describe API::Jobs do end describe 'GET /projects/:id/jobs' do - let(:query) { Hash.new } + let(:query) { {} } before do |example| unless example.metadata[:skip_before_request] @@ -166,295 +140,6 @@ RSpec.describe API::Jobs do end end - describe 'GET /projects/:id/pipelines/:pipeline_id/jobs' do - let(:query) { Hash.new } - - before do |example| - unless example.metadata[:skip_before_request] - job - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query - end - end - - context 'authorized user' do - it 'returns pipeline jobs' do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - end - - it 'returns correct values' do - expect(json_response).not_to be_empty - expect(json_response.first['commit']['id']).to eq project.commit.id - expect(Time.parse(json_response.first['artifacts_expire_at'])).to be_like_time(job.artifacts_expire_at) - expect(json_response.first['artifacts_file']).to be_nil - expect(json_response.first['artifacts']).to be_an Array - expect(json_response.first['artifacts']).to be_empty - end - - it_behaves_like 'a job with artifacts and trace' do - let(:api_endpoint) { "/projects/#{project.id}/pipelines/#{pipeline.id}/jobs" } - end - - it 'returns pipeline data' do - json_job = json_response.first - - expect(json_job['pipeline']).not_to be_empty - expect(json_job['pipeline']['id']).to eq job.pipeline.id - expect(json_job['pipeline']['ref']).to eq job.pipeline.ref - expect(json_job['pipeline']['sha']).to eq job.pipeline.sha - expect(json_job['pipeline']['status']).to eq job.pipeline.status - end - - context 'filter jobs with one scope element' do - let(:query) { { 'scope' => 'pending' } } - - it do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - end - end - - context 'filter jobs with array of scope elements' do - let(:query) { { scope: %w(pending running) } } - - it do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - end - end - - context 'respond 400 when scope contains invalid state' do - let(:query) { { scope: %w(unknown running) } } - - it { expect(response).to have_gitlab_http_status(:bad_request) } - end - - context 'jobs in different pipelines' do - let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } - let!(:job2) { create(:ci_build, pipeline: pipeline2) } - - it 'excludes jobs from other pipelines' do - json_response.each { |job| expect(job['pipeline']['id']).to eq(pipeline.id) } - end - end - - context 'when config source not ci' do - let(:non_ci_config_source) { ::Ci::PipelineEnums.non_ci_config_source_values.first } - let(:pipeline) do - create(:ci_pipeline, config_source: non_ci_config_source, project: project) - end - - it 'returns the specified pipeline' do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response[0]['pipeline']['sha']).to eq(pipeline.sha.to_s) - end - end - - it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query - end.count - - create_list(:ci_build, 3, :trace_artifact, :artifacts, :test_reports, pipeline: pipeline) - - expect do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query - end.not_to exceed_all_query_limit(control_count) - end - end - - context 'unauthorized user' do - context 'when user is not logged in' do - let(:api_user) { nil } - - it 'does not return jobs' do - expect(response).to have_gitlab_http_status(:unauthorized) - end - end - - context 'when user is guest' do - let(:api_user) { guest } - - it 'does not return jobs' do - expect(response).to have_gitlab_http_status(:forbidden) - end - end - end - end - - describe 'GET /projects/:id/pipelines/:pipeline_id/bridges' do - let!(:bridge) { create(:ci_bridge, pipeline: pipeline) } - let(:downstream_pipeline) { create(:ci_pipeline) } - - let!(:pipeline_source) do - create(:ci_sources_pipeline, - source_pipeline: pipeline, - source_project: project, - source_job: bridge, - pipeline: downstream_pipeline, - project: downstream_pipeline.project) - end - - let(:query) { Hash.new } - - before do |example| - unless example.metadata[:skip_before_request] - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - end - end - - context 'authorized user' do - it 'returns pipeline bridges' do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - end - - it 'returns correct values' do - expect(json_response).not_to be_empty - expect(json_response.first['commit']['id']).to eq project.commit.id - expect(json_response.first['id']).to eq bridge.id - expect(json_response.first['name']).to eq bridge.name - expect(json_response.first['stage']).to eq bridge.stage - end - - it 'returns pipeline data' do - json_bridge = json_response.first - - expect(json_bridge['pipeline']).not_to be_empty - expect(json_bridge['pipeline']['id']).to eq bridge.pipeline.id - expect(json_bridge['pipeline']['ref']).to eq bridge.pipeline.ref - expect(json_bridge['pipeline']['sha']).to eq bridge.pipeline.sha - expect(json_bridge['pipeline']['status']).to eq bridge.pipeline.status - end - - it 'returns downstream pipeline data' do - json_bridge = json_response.first - - expect(json_bridge['downstream_pipeline']).not_to be_empty - expect(json_bridge['downstream_pipeline']['id']).to eq downstream_pipeline.id - expect(json_bridge['downstream_pipeline']['ref']).to eq downstream_pipeline.ref - expect(json_bridge['downstream_pipeline']['sha']).to eq downstream_pipeline.sha - expect(json_bridge['downstream_pipeline']['status']).to eq downstream_pipeline.status - end - - context 'filter bridges' do - before do - create_bridge(pipeline, :pending) - create_bridge(pipeline, :running) - end - - context 'with one scope element' do - let(:query) { { 'scope' => 'pending' } } - - it :skip_before_request do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - expect(json_response.count).to eq 1 - expect(json_response.first["status"]).to eq "pending" - end - end - - context 'with array of scope elements' do - let(:query) { { scope: %w(pending running) } } - - it :skip_before_request do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - expect(json_response.count).to eq 2 - json_response.each { |r| expect(%w(pending running).include?(r['status'])).to be true } - end - end - end - - context 'respond 400 when scope contains invalid state' do - let(:query) { { scope: %w(unknown running) } } - - it { expect(response).to have_gitlab_http_status(:bad_request) } - end - - context 'bridges in different pipelines' do - let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } - let!(:bridge2) { create(:ci_bridge, pipeline: pipeline2) } - - it 'excludes bridges from other pipelines' do - json_response.each { |bridge| expect(bridge['pipeline']['id']).to eq(pipeline.id) } - end - end - - it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - end.count - - 3.times { create_bridge(pipeline) } - - expect do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - end.not_to exceed_all_query_limit(control_count) - end - end - - context 'unauthorized user' do - context 'when user is not logged in' do - let(:api_user) { nil } - - it 'does not return bridges' do - expect(response).to have_gitlab_http_status(:unauthorized) - end - end - - context 'when user is guest' do - let(:api_user) { guest } - - it 'does not return bridges' do - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'when user has no read access for pipeline' do - before do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?).with(api_user, :read_pipeline, pipeline).and_return(false) - end - - it 'does not return bridges' do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user) - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'when user has no read_build access for project' do - before do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?).with(api_user, :read_build, project).and_return(false) - end - - it 'does not return bridges' do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user) - expect(response).to have_gitlab_http_status(:forbidden) - end - end - end - - def create_bridge(pipeline, status = :created) - create(:ci_bridge, status: status, pipeline: pipeline).tap do |bridge| - downstream_pipeline = create(:ci_pipeline) - create(:ci_sources_pipeline, - source_pipeline: pipeline, - source_project: pipeline.project, - source_job: bridge, - pipeline: downstream_pipeline, - project: downstream_pipeline.project) - end - end - end - describe 'GET /projects/:id/jobs/:job_id' do before do |example| unless example.metadata[:skip_before_request] diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index b74887762a2..0a23aed109b 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -283,7 +283,7 @@ RSpec.describe API::MavenPackages do context 'internal project' do before do - group.group_member(user).destroy + group.group_member(user).destroy! project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) end @@ -542,6 +542,18 @@ RSpec.describe API::MavenPackages do context 'when params from workhorse are correct' do let(:params) { { file: file_upload } } + context 'file size is too large' do + it 'rejects the request' do + allow_next_instance_of(UploadedFile) do |uploaded_file| + allow(uploaded_file).to receive(:size).and_return(project.actual_limits.maven_max_file_size + 1) + end + + upload_file_with_token(params) + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + it 'rejects a malicious request' do put api("/projects/#{project.id}/packages/maven/com/example/my-app/#{version}/%2e%2e%2f.ssh%2fauthorized_keys"), params: params, headers: headers_with_token diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 23889912d7a..de52087340c 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -258,8 +258,8 @@ RSpec.describe API::Members do it 'does not create the member if group level is higher' do parent = create(:group) - group.update(parent: parent) - project.update(group: group) + group.update!(parent: parent) + project.update!(group: group) parent.add_developer(stranger) post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), @@ -272,8 +272,8 @@ RSpec.describe API::Members do it 'creates the member if group level is lower' do parent = create(:group) - group.update(parent: parent) - project.update(group: group) + group.update!(parent: parent) + project.update!(group: group) parent.add_developer(stranger) post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), diff --git a/spec/requests/api/merge_request_diffs_spec.rb b/spec/requests/api/merge_request_diffs_spec.rb index 3f41a7a034d..2e6cbe7bee7 100644 --- a/spec/requests/api/merge_request_diffs_spec.rb +++ b/spec/requests/api/merge_request_diffs_spec.rb @@ -8,8 +8,8 @@ RSpec.describe API::MergeRequestDiffs, 'MergeRequestDiffs' do let!(:project) { merge_request.target_project } before do - merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') - merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') + merge_request.merge_request_diffs.create!(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') + merge_request.merge_request_diffs.create!(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') project.add_maintainer(user) end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index d4c05b4b198..2757c56e0fe 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -5,23 +5,19 @@ require "spec_helper" RSpec.describe API::MergeRequests do include ProjectForksHelper - let(:base_time) { Time.now } + let_it_be(:base_time) { Time.now } let_it_be(:user) { create(:user) } let_it_be(:user2) { create(:user) } let_it_be(:admin) { create(:user, :admin) } - let(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) } - let(:milestone) { create(:milestone, title: '1.0.0', project: project) } - let(:milestone1) { create(:milestone, title: '0.9', project: project) } - let(:merge_request_context_commit) {create(:merge_request_context_commit, message: 'test')} - let!(:merge_request) { create(:merge_request, :simple, milestone: milestone1, author: user, assignees: [user], merge_request_context_commits: [merge_request_context_commit], source_project: project, target_project: project, source_branch: 'markdown', title: "Test", created_at: base_time) } - let!(:merge_request_closed) { create(:merge_request, state: "closed", milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.second) } - let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignees: [user], source_project: project, target_project: project, title: "Merged test", created_at: base_time + 2.seconds, merge_commit_sha: '9999999999999999999999999999999999999999') } - let!(:merge_request_locked) { create(:merge_request, state: "locked", milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, title: "Locked test", created_at: base_time + 1.second) } - let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") } - let!(:note2) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") } + let_it_be(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) } + + let(:milestone1) { create(:milestone, title: '0.9', project: project) } + let(:milestone) { create(:milestone, title: '1.0.0', project: project) } let(:label) { create(:label, title: 'label', color: '#FFAABB', project: project) } let(:label2) { create(:label, title: 'a-test', color: '#FFFFFF', project: project) } + let(:merge_request) { create(:merge_request, :simple, author: user, assignees: [user], source_project: project, target_project: project, source_branch: 'markdown', title: "Test", created_at: base_time) } + before do project.add_reporter(user) project.add_reporter(user2) @@ -29,6 +25,16 @@ RSpec.describe API::MergeRequests do stub_licensed_features(multiple_merge_request_assignees: false) end + shared_context 'with merge requests' do + let_it_be(:milestone1) { create(:milestone, title: '0.9', project: project) } + let_it_be(:merge_request) { create(:merge_request, :simple, milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, source_branch: 'markdown', title: "Test", created_at: base_time) } + let_it_be(:merge_request_closed) { create(:merge_request, state: "closed", milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.second) } + let_it_be(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignees: [user], source_project: project, target_project: project, title: "Merged test", created_at: base_time + 2.seconds, merge_commit_sha: '9999999999999999999999999999999999999999') } + let_it_be(:merge_request_locked) { create(:merge_request, state: "locked", milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, title: "Locked test", created_at: base_time + 1.second) } + let_it_be(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") } + let_it_be(:note2) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") } + end + shared_context 'with labels' do before do create(:label_link, label: label, target: merge_request) @@ -68,6 +74,7 @@ RSpec.describe API::MergeRequests do context 'when merge request is unchecked' do let(:check_service_class) { MergeRequests::MergeabilityCheckService } let(:mr_entity) { json_response.find { |mr| mr['id'] == merge_request.id } } + let(:merge_request) { create(:merge_request, :simple, author: user, source_project: project, title: "Test") } before do merge_request.mark_as_unchecked! @@ -426,14 +433,13 @@ RSpec.describe API::MergeRequests do end context 'NOT params' do - let(:merge_request2) do + let!(:merge_request2) do create( :merge_request, :simple, milestone: milestone, author: user, assignees: [user], - merge_request_context_commits: [merge_request_context_commit], source_project: project, target_project: project, source_branch: 'what', @@ -442,6 +448,8 @@ RSpec.describe API::MergeRequests do ) end + let!(:merge_request_context_commit) { create(:merge_request_context_commit, merge_request: merge_request2, message: 'test') } + before do create(:label_link, label: label, target: merge_request) create(:label_link, label: label2, target: merge_request2) @@ -527,6 +535,8 @@ RSpec.describe API::MergeRequests do end describe 'GET /merge_requests' do + include_context 'with merge requests' + context 'when unauthenticated' do it 'returns an array of all merge requests' do get api('/merge_requests', user), params: { scope: 'all' } @@ -563,9 +573,9 @@ RSpec.describe API::MergeRequests do end context 'when authenticated' do - let!(:project2) { create(:project, :public, namespace: user.namespace) } - let!(:merge_request2) { create(:merge_request, :simple, author: user, assignees: [user], source_project: project2, target_project: project2) } - let(:user2) { create(:user) } + let_it_be(:project2) { create(:project, :public, :repository, namespace: user.namespace) } + let_it_be(:merge_request2) { create(:merge_request, :simple, author: user, assignees: [user], source_project: project2, target_project: project2) } + let_it_be(:user2) { create(:user) } it 'returns an array of all merge requests except unauthorized ones' do get api('/merge_requests', user), params: { scope: :all } @@ -778,8 +788,8 @@ RSpec.describe API::MergeRequests do end context 'search params' do - before do - merge_request.update(title: 'Search title', description: 'Search description') + let_it_be(:merge_request) do + create(:merge_request, :simple, author: user, source_project: project, target_project: project, title: 'Search title', description: 'Search description') end it 'returns merge requests matching given search string for title' do @@ -818,6 +828,8 @@ RSpec.describe API::MergeRequests do end describe "GET /projects/:id/merge_requests" do + include_context 'with merge requests' + let(:endpoint_path) { "/projects/#{project.id}/merge_requests" } it_behaves_like 'merge requests list' @@ -845,7 +857,9 @@ RSpec.describe API::MergeRequests do end context 'a project which enforces all discussions to be resolved' do - let!(:project) { create(:project, :repository, only_allow_merge_if_all_discussions_are_resolved: true) } + let_it_be(:project) { create(:project, :repository, only_allow_merge_if_all_discussions_are_resolved: true) } + + include_context 'with merge requests' it 'avoids N+1 queries' do control = ActiveRecord::QueryRecorder.new do @@ -864,6 +878,9 @@ RSpec.describe API::MergeRequests do describe "GET /groups/:id/merge_requests" do let_it_be(:group) { create(:group, :public) } let_it_be(:project) { create(:project, :public, :repository, creator: user, namespace: group, only_allow_merge_if_pipeline_succeeds: false) } + + include_context 'with merge requests' + let(:endpoint_path) { "/groups/#{group.id}/merge_requests" } before do @@ -877,6 +894,8 @@ RSpec.describe API::MergeRequests do let_it_be(:subgroup) { create(:group, parent: group) } let_it_be(:project) { create(:project, :public, :repository, creator: user, namespace: subgroup, only_allow_merge_if_pipeline_succeeds: false) } + include_context 'with merge requests' + it_behaves_like 'merge requests list' end @@ -893,7 +912,7 @@ RSpec.describe API::MergeRequests do let(:parent_group) { create(:group) } before do - group.update(parent_id: parent_group.id) + group.update!(parent_id: parent_group.id) merge_request_merged.reload end @@ -936,6 +955,8 @@ RSpec.describe API::MergeRequests do end describe "GET /projects/:id/merge_requests/:merge_request_iid" do + let(:merge_request) { create(:merge_request, :simple, author: user, assignees: [user], milestone: milestone, source_project: project, source_branch: 'markdown', title: "Test") } + it 'matches json schema' do merge_request = create(:merge_request, :with_test_reports, milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, title: "Test", created_at: base_time) get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) @@ -1006,7 +1027,7 @@ RSpec.describe API::MergeRequests do let(:non_member) { create(:user) } before do - merge_request.update(author: non_member) + merge_request.update!(author: non_member) end it 'exposes first_contribution as true' do @@ -1059,9 +1080,12 @@ RSpec.describe API::MergeRequests do end context 'head_pipeline' do + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request, :simple, author: user, source_project: project, source_branch: 'markdown', title: "Test") } + before do - merge_request.update(head_pipeline: create(:ci_pipeline)) - merge_request.project.project_feature.update(builds_access_level: 10) + merge_request.update!(head_pipeline: create(:ci_pipeline)) + merge_request.project.project_feature.update!(builds_access_level: 10) end context 'when user can read the pipeline' do @@ -1188,11 +1212,13 @@ RSpec.describe API::MergeRequests do describe 'GET /projects/:id/merge_requests/:merge_request_iid/participants' do it_behaves_like 'issuable participants endpoint' do - let(:entity) { merge_request } + let(:entity) { create(:merge_request, :simple, milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, source_branch: 'markdown', title: "Test", created_at: base_time) } end end describe 'GET /projects/:id/merge_requests/:merge_request_iid/commits' do + include_context 'with merge requests' + it 'returns a 200 when merge request is valid' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/commits", user) commit = merge_request.commits.first @@ -1216,6 +1242,9 @@ RSpec.describe API::MergeRequests do end describe 'GET /projects/:id/merge_requests/:merge_request_iid/:context_commits' do + let_it_be(:merge_request) { create(:merge_request, :simple, author: user, source_project: project, target_project: project, source_branch: 'markdown', title: "Test", created_at: base_time) } + let_it_be(:merge_request_context_commit) { create(:merge_request_context_commit, merge_request: merge_request, message: 'test') } + it 'returns a 200 when merge request is valid' do context_commit = merge_request.context_commits.first @@ -1234,6 +1263,8 @@ RSpec.describe API::MergeRequests do end describe 'GET /projects/:id/merge_requests/:merge_request_iid/changes' do + let_it_be(:merge_request) { create(:merge_request, :simple, author: user, assignees: [user], source_project: project, target_project: project, source_branch: 'markdown', title: "Test", created_at: base_time) } + it 'returns the change information of the merge_request' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/changes", user) @@ -1254,6 +1285,8 @@ RSpec.describe API::MergeRequests do end describe 'GET /projects/:id/merge_requests/:merge_request_iid/pipelines' do + let_it_be(:merge_request) { create(:merge_request, :simple, author: user, assignees: [user], source_project: project, target_project: project, source_branch: 'markdown', title: "Test", created_at: base_time) } + context 'when authorized' do let!(:pipeline) { create(:ci_empty_pipeline, project: project, user: user, ref: merge_request.source_branch, sha: merge_request.diff_head_sha) } let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } @@ -1308,16 +1341,15 @@ RSpec.describe API::MergeRequests do }) end - let(:project) do + let_it_be(:project) do create(:project, :private, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) end - let(:merge_request) do + let_it_be(:merge_request) do create(:merge_request, :with_detached_merge_request_pipeline, - milestone: milestone1, author: user, assignees: [user], source_project: project, @@ -1351,7 +1383,7 @@ RSpec.describe API::MergeRequests do end context 'when the merge request does not exist' do - let(:merge_request_iid) { 777 } + let(:merge_request_iid) { non_existing_record_id } it 'responds with a blank 404' do expect { request }.not_to change(Ci::Pipeline, :count) @@ -1604,7 +1636,7 @@ RSpec.describe API::MergeRequests do end.to change { MergeRequest.count }.by(0) expect(response).to have_gitlab_http_status(:conflict) - expect(json_response['message']).to eq(["Another open merge request already exists for this source branch: !5"]) + expect(json_response['message']).to eq(["Another open merge request already exists for this source branch: !1"]) end end @@ -1659,7 +1691,7 @@ RSpec.describe API::MergeRequests do end it 'returns 403 when target project has disabled merge requests' do - project.project_feature.update(merge_requests_access_level: 0) + project.project_feature.update!(merge_requests_access_level: 0) post api("/projects/#{forked_project.id}/merge_requests", user2), params: { @@ -1778,6 +1810,7 @@ RSpec.describe API::MergeRequests do before do create(:merge_request_context_commit, merge_request: merge_request, sha: commit.id) end + it 'returns 400 when the context commit is already created' do post api("/projects/#{project.id}/merge_requests/#{merge_request_iid}/context_commits", authenticated_user), params: params expect(response).to have_gitlab_http_status(:bad_request) @@ -1937,7 +1970,9 @@ RSpec.describe API::MergeRequests do end describe "PUT /projects/:id/merge_requests/:merge_request_iid/merge", :clean_gitlab_redis_cache do - let(:pipeline) { create(:ci_pipeline) } + let(:project) { create(:project, :repository, namespace: user.namespace) } + let(:merge_request) { create(:merge_request, :simple, author: user, source_project: project, source_branch: 'markdown', title: 'Test') } + 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) @@ -2111,7 +2146,7 @@ RSpec.describe API::MergeRequests do let(:source_branch) { merge_request.source_branch } before do - merge_request.update(merge_params: { 'force_remove_source_branch' => true }) + merge_request.update!(merge_params: { 'force_remove_source_branch' => true }) end it 'removes the source branch' do @@ -2138,7 +2173,7 @@ RSpec.describe API::MergeRequests do let(:merge_request) { create(:merge_request, :rebased, source_project: project, squash: true) } before do - project.update(merge_requests_ff_only_enabled: true) + project.update!(merge_requests_ff_only_enabled: true) end it "records the squash commit SHA and returns it in the response" do @@ -2169,9 +2204,7 @@ RSpec.describe API::MergeRequests do end context 'when merge-ref is not synced with merge status' do - before do - merge_request.update!(merge_status: 'cannot_be_merged') - end + let(:merge_request) { create(:merge_request, :simple, author: user, source_project: project, source_branch: 'markdown', merge_status: 'cannot_be_merged') } it 'returns 200 if MR can be merged' do get api(url, user) @@ -2230,7 +2263,7 @@ RSpec.describe API::MergeRequests do describe "PUT /projects/:id/merge_requests/:merge_request_iid" do context 'updates force_remove_source_branch properly' do it 'sets to false' do - merge_request.update(merge_params: { 'force_remove_source_branch' => true } ) + merge_request.update!(merge_params: { 'force_remove_source_branch' => true } ) expect(merge_request.force_remove_source_branch?).to be_truthy @@ -2242,7 +2275,7 @@ RSpec.describe API::MergeRequests do end it 'sets to true' do - merge_request.update(merge_params: { 'force_remove_source_branch' => false } ) + merge_request.update!(merge_params: { 'force_remove_source_branch' => false } ) expect(merge_request.force_remove_source_branch?).to be_falsey @@ -2254,6 +2287,7 @@ RSpec.describe API::MergeRequests do end context 'with a merge request across forks' do + let(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) } let(:fork_owner) { create(:user) } let(:source_project) { fork_project(project, fork_owner) } let(:target_project) { project } @@ -2320,12 +2354,46 @@ RSpec.describe API::MergeRequests do expect(json_response['squash']).to be_truthy end - it "returns merge_request with renamed target_branch" do + it "updates target_branch and returns merge_request" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { target_branch: "wiki" } expect(response).to have_gitlab_http_status(:ok) expect(json_response['target_branch']).to eq('wiki') end + context "forked projects" do + let_it_be(:user2) { create(:user) } + let(:project) { create(:project, :public, :repository) } + let!(:forked_project) { fork_project(project, user2, repository: true) } + let(:merge_request) do + create(:merge_request, + source_project: forked_project, + target_project: project, + source_branch: "fixes") + end + + shared_examples "update of allow_collaboration and allow_maintainer_to_push" do |request_value, expected_value| + %w[allow_collaboration allow_maintainer_to_push].each do |attr| + it "attempts to update #{attr} to #{request_value} and returns #{expected_value} for `allow_collaboration`" do + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user2), params: { attr => request_value } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response["allow_collaboration"]).to eq(expected_value) + expect(json_response["allow_maintainer_to_push"]).to eq(expected_value) + end + end + end + + context "when source project is public (i.e. MergeRequest#collaborative_push_possible? == true)" do + it_behaves_like "update of allow_collaboration and allow_maintainer_to_push", true, true + end + + context "when source project is private (i.e. MergeRequest#collaborative_push_possible? == false)" do + let(:project) { create(:project, :private, :repository) } + + it_behaves_like "update of allow_collaboration and allow_maintainer_to_push", true, false + end + end + it "returns merge_request that removes the source branch" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { remove_source_branch: true } @@ -2717,7 +2785,7 @@ RSpec.describe API::MergeRequests do end describe 'Time tracking' do - let(:issuable) { merge_request } + let!(:issuable) { create(:merge_request, :simple, author: user, assignees: [user], source_project: project, target_project: project, source_branch: 'markdown', title: "Test", created_at: base_time) } include_examples 'time tracking endpoints', 'merge_request' end @@ -2726,7 +2794,7 @@ RSpec.describe API::MergeRequests do merge_request merge_request.created_at += 1.hour merge_request.updated_at += 30.minutes - merge_request.save + merge_request.save! merge_request end @@ -2734,7 +2802,7 @@ RSpec.describe API::MergeRequests do merge_request_closed merge_request_closed.created_at -= 1.hour merge_request_closed.updated_at -= 30.minutes - merge_request_closed.save + merge_request_closed.save! merge_request_closed end end diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index ca4ebd3689f..baab72d106f 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -81,7 +81,7 @@ RSpec.describe API::Notes do context "issue is confidential" do before do - ext_issue.update(confidential: true) + ext_issue.update!(confidential: true) end it "returns 404" do @@ -183,7 +183,7 @@ RSpec.describe API::Notes do context "when issue is confidential" do before do - issue.update(confidential: true) + issue.update!(confidential: true) end it "returns 404" do diff --git a/spec/requests/api/nuget_packages_spec.rb b/spec/requests/api/nuget_packages_spec.rb index 87c62ec41c6..62f244c433b 100644 --- a/spec/requests/api/nuget_packages_spec.rb +++ b/spec/requests/api/nuget_packages_spec.rb @@ -220,6 +220,18 @@ RSpec.describe API::NugetPackages do it_behaves_like 'rejects nuget access with unknown project id' it_behaves_like 'rejects nuget access with invalid project id' + + context 'file size above maximum limit' do + let(:headers) { basic_auth_header(deploy_token.username, deploy_token.token).merge(workhorse_header) } + + before do + allow_next_instance_of(UploadedFile) do |uploaded_file| + allow(uploaded_file).to receive(:size).and_return(project.actual_limits.nuget_max_file_size + 1) + end + end + + it_behaves_like 'returning response status', :bad_request + end end end diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb index f5971054b3c..23d5df873d4 100644 --- a/spec/requests/api/oauth_tokens_spec.rb +++ b/spec/requests/api/oauth_tokens_spec.rb @@ -20,7 +20,7 @@ RSpec.describe 'OAuth tokens' do request_oauth_token(user, client_basic_auth_header(client)) - expect(response).to have_gitlab_http_status(:unauthorized) + expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['error']).to eq('invalid_grant') end end @@ -62,7 +62,7 @@ RSpec.describe 'OAuth tokens' do request_oauth_token(user, basic_auth_header(client.uid, 'invalid secret')) - expect(response).to have_gitlab_http_status(:unauthorized) + expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['error']).to eq('invalid_client') end end @@ -72,7 +72,7 @@ RSpec.describe 'OAuth tokens' do shared_examples 'does not create an access token' do let(:user) { create(:user) } - it { expect(response).to have_gitlab_http_status(:unauthorized) } + it { expect(response).to have_gitlab_http_status(:bad_request) } end context 'when user is blocked' do diff --git a/spec/requests/api/pages/internal_access_spec.rb b/spec/requests/api/pages/internal_access_spec.rb index c894a2d3ca4..4ac47f17b7e 100644 --- a/spec/requests/api/pages/internal_access_spec.rb +++ b/spec/requests/api/pages/internal_access_spec.rb @@ -72,7 +72,7 @@ RSpec.describe "Internal Project Pages Access" do with_them do before do - project.project_feature.update(pages_access_level: pages_access_level) + project.project_feature.update!(pages_access_level: pages_access_level) end it "correct return value" do if !with_user.nil? diff --git a/spec/requests/api/pages/pages_spec.rb b/spec/requests/api/pages/pages_spec.rb index 53e732928ff..f4c6de00e40 100644 --- a/spec/requests/api/pages/pages_spec.rb +++ b/spec/requests/api/pages/pages_spec.rb @@ -39,7 +39,9 @@ RSpec.describe API::Pages do expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return true expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, project.namespace.full_path, anything) - delete api("/projects/#{project.id}/pages", admin ) + Sidekiq::Testing.inline! do + delete api("/projects/#{project.id}/pages", admin ) + end expect(project.reload.pages_metadatum.deployed?).to be(false) end diff --git a/spec/requests/api/pages/private_access_spec.rb b/spec/requests/api/pages/private_access_spec.rb index ea5db691b14..c1c0e406508 100644 --- a/spec/requests/api/pages/private_access_spec.rb +++ b/spec/requests/api/pages/private_access_spec.rb @@ -72,7 +72,7 @@ RSpec.describe "Private Project Pages Access" do with_them do before do - project.project_feature.update(pages_access_level: pages_access_level) + project.project_feature.update!(pages_access_level: pages_access_level) end it "correct return value" do if !with_user.nil? diff --git a/spec/requests/api/pages/public_access_spec.rb b/spec/requests/api/pages/public_access_spec.rb index ae73cee91d5..c45b3a4c55e 100644 --- a/spec/requests/api/pages/public_access_spec.rb +++ b/spec/requests/api/pages/public_access_spec.rb @@ -72,7 +72,7 @@ RSpec.describe "Public Project Pages Access" do with_them do before do - project.project_feature.update(pages_access_level: pages_access_level) + project.project_feature.update!(pages_access_level: pages_access_level) end it "correct return value" do if !with_user.nil? diff --git a/spec/requests/api/project_milestones_spec.rb b/spec/requests/api/project_milestones_spec.rb index d1e5df66b3f..71535e66353 100644 --- a/spec/requests/api/project_milestones_spec.rb +++ b/spec/requests/api/project_milestones_spec.rb @@ -43,7 +43,7 @@ RSpec.describe API::ProjectMilestones do let(:milestones) { [group_milestone, ancestor_group_milestone, milestone, closed_milestone] } before_all do - project.update(namespace: group) + project.update!(namespace: group) end it_behaves_like 'listing all milestones' @@ -108,7 +108,7 @@ RSpec.describe API::ProjectMilestones do let(:group) { create(:group) } before do - project.update(namespace: group) + project.update!(namespace: group) end context 'when user does not have permission to promote milestone' do @@ -163,7 +163,7 @@ RSpec.describe API::ProjectMilestones do context 'when project does not belong to group' do before do - project.update(namespace: user.namespace) + project.update!(namespace: user.namespace) end it 'returns 403' do diff --git a/spec/requests/api/project_packages_spec.rb b/spec/requests/api/project_packages_spec.rb index 0ece3bff8f9..2f0d0fc87ec 100644 --- a/spec/requests/api/project_packages_spec.rb +++ b/spec/requests/api/project_packages_spec.rb @@ -104,6 +104,8 @@ RSpec.describe API::ProjectPackages do expect(json_response.first['name']).to include(package2.name) end end + + it_behaves_like 'does not cause n^2 queries' end end @@ -127,6 +129,22 @@ RSpec.describe API::ProjectPackages do end context 'without the need for a license' do + context 'with build info' do + it 'does not result in additional queries' do + control = ActiveRecord::QueryRecorder.new do + get api(package_url, user) + end + + pipeline = create(:ci_pipeline, user: user) + create(:ci_build, user: user, pipeline: pipeline) + create(:package_build_info, package: package1, pipeline: pipeline) + + expect do + get api(package_url, user) + end.not_to exceed_query_limit(control) + end + end + context 'project is public' do it 'returns 200 and the package information' do subject diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 9b876edae24..08c88873078 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -304,56 +304,10 @@ RSpec.describe API::ProjectSnippets do let(:visibility_level) { Snippet::PUBLIC } let(:snippet) { create(:project_snippet, :repository, author: admin, visibility_level: visibility_level, project: project) } - it 'updates snippet' do - new_content = 'New content' - new_description = 'New description' - - update_snippet(params: { content: new_content, description: new_description, visibility: 'private' }) - - expect(response).to have_gitlab_http_status(:ok) - snippet.reload - expect(snippet.content).to eq(new_content) - expect(snippet.description).to eq(new_description) - expect(snippet.visibility).to eq('private') - end - - it 'updates snippet with content parameter' do - new_content = 'New content' - new_description = 'New description' - - update_snippet(params: { content: new_content, description: new_description }) - - expect(response).to have_gitlab_http_status(:ok) - snippet.reload - expect(snippet.content).to eq(new_content) - expect(snippet.description).to eq(new_description) - end - - it 'returns 404 for invalid snippet id' do - update_snippet(snippet_id: non_existing_record_id, params: { title: 'foo' }) - - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 Snippet Not Found') - end - - it 'returns 400 for missing parameters' do - update_snippet - - expect(response).to have_gitlab_http_status(:bad_request) - end - - it 'returns 400 if content is blank' do - update_snippet(params: { content: '' }) - - expect(response).to have_gitlab_http_status(:bad_request) - end - - it 'returns 400 if title is blank' do - update_snippet(params: { title: '' }) - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq 'title is empty' - end + it_behaves_like 'snippet file updates' + it_behaves_like 'snippet non-file updates' + it_behaves_like 'snippet individual non-file updates' + it_behaves_like 'invalid snippet updates' it_behaves_like 'update with repository actions' do let(:snippet_without_repo) { create(:project_snippet, author: admin, project: project, visibility_level: visibility_level) } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 46340f86f69..831b0d6e678 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -49,15 +49,15 @@ end RSpec.describe API::Projects do include ProjectForksHelper - let(:user) { create(:user) } - let(:user2) { create(:user) } - let(:user3) { create(:user) } - let(:admin) { create(:admin) } - let(:project) { create(:project, :repository, namespace: user.namespace) } - let(:project2) { create(:project, namespace: user.namespace) } - let(:project_member) { create(:project_member, :developer, user: user3, project: project) } - let(:user4) { create(:user, username: 'user.with.dot') } - let(:project3) do + let_it_be(:user) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:user3) { create(:user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:project, reload: true) { create(:project, :repository, namespace: user.namespace) } + let_it_be(:project2, reload: true) { create(:project, namespace: user.namespace) } + let_it_be(:project_member) { create(:project_member, :developer, user: user3, project: project) } + let_it_be(:user4) { create(:user, username: 'user.with.dot') } + let_it_be(:project3, reload: true) do create(:project, :private, :repository, @@ -71,14 +71,14 @@ RSpec.describe API::Projects do snippets_enabled: false) end - let(:project_member2) do + let_it_be(:project_member2) do create(:project_member, user: user4, project: project3, access_level: ProjectMember::MAINTAINER) end - let(:project4) do + let_it_be(:project4, reload: true) do create(:project, name: 'third_project', path: 'third_project', @@ -86,6 +86,8 @@ RSpec.describe API::Projects do namespace: user4.namespace) end + let(:user_projects) { [public_project, project, project2, project3] } + shared_context 'with language detection' do let(:ruby) { create(:programming_language, name: 'Ruby') } let(:javascript) { create(:programming_language, name: 'JavaScript') } @@ -146,14 +148,7 @@ RSpec.describe API::Projects do end end - let!(:public_project) { create(:project, :public, name: 'public_project') } - - before do - project - project2 - project3 - project4 - end + let_it_be(:public_project) { create(:project, :public, name: 'public_project') } context 'when unauthenticated' do it_behaves_like 'projects response' do @@ -171,7 +166,7 @@ RSpec.describe API::Projects do it_behaves_like 'projects response' do let(:filter) { {} } let(:current_user) { user } - let(:projects) { [public_project, project, project2, project3] } + let(:projects) { user_projects } end it_behaves_like 'projects response without N + 1 queries' do @@ -208,7 +203,7 @@ RSpec.describe API::Projects do end it 'does not include projects marked for deletion' do - project.update(pending_delete: true) + project.update!(pending_delete: true) get api('/projects', user) @@ -257,7 +252,7 @@ RSpec.describe API::Projects do expect(response).to include_pagination_headers expect(json_response).to be_an Array - statistics = json_response.first['statistics'] + statistics = json_response.find { |p| p['id'] == project.id }['statistics'] expect(statistics).to be_present expect(statistics).to include('commit_count', 'storage_size', 'repository_size', 'wiki_size', 'lfs_objects_size', 'job_artifacts_size', 'snippets_size') end @@ -386,14 +381,14 @@ RSpec.describe API::Projects do it_behaves_like 'projects response' do let(:filter) { { id_after: project2.id } } let(:current_user) { user } - let(:projects) { [public_project, project, project2, project3].select { |p| p.id > project2.id } } + let(:projects) { user_projects.select { |p| p.id > project2.id } } end context 'regression: empty string is ignored' do it_behaves_like 'projects response' do let(:filter) { { id_after: '' } } let(:current_user) { user } - let(:projects) { [public_project, project, project2, project3] } + let(:projects) { user_projects } end end end @@ -402,14 +397,14 @@ RSpec.describe API::Projects do it_behaves_like 'projects response' do let(:filter) { { id_before: project2.id } } let(:current_user) { user } - let(:projects) { [public_project, project, project2, project3].select { |p| p.id < project2.id } } + let(:projects) { user_projects.select { |p| p.id < project2.id } } end context 'regression: empty string is ignored' do it_behaves_like 'projects response' do let(:filter) { { id_before: '' } } let(:current_user) { user } - let(:projects) { [public_project, project, project2, project3] } + let(:projects) { user_projects } end end end @@ -418,7 +413,7 @@ RSpec.describe API::Projects do it_behaves_like 'projects response' do let(:filter) { { id_before: project2.id, id_after: public_project.id } } let(:current_user) { user } - let(:projects) { [public_project, project, project2, project3].select { |p| p.id < project2.id && p.id > public_project.id } } + let(:projects) { user_projects.select { |p| p.id < project2.id && p.id > public_project.id } } end end @@ -481,7 +476,7 @@ RSpec.describe API::Projects 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.first['id']).to eq(project3.id) + expect(json_response.map { |p| p['id'] }).to eq(user_projects.map(&:id).sort.reverse) end end @@ -495,14 +490,32 @@ RSpec.describe API::Projects do expect(json_response.first['name']).to eq(project4.name) expect(json_response.first['owner']['username']).to eq(user4.username) end + + context 'when admin creates a project' do + before do + group = create(:group) + project_create_opts = { + name: 'GitLab', + namespace_id: group.id + } + + Projects::CreateService.new(admin, project_create_opts).execute + end + + it 'does not list as owned project for admin' do + get api('/projects', admin), params: { owned: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + end + end end context 'and with starred=true' do let(:public_project) { create(:project, :public) } before do - project_member - user3.update(starred_projects: [project, project2, project3, public_project]) + user3.update!(starred_projects: [project, project2, project3, public_project]) end it 'returns the starred projects viewable by the user' do @@ -523,7 +536,7 @@ RSpec.describe API::Projects do let!(:project9) { create(:project, :public, path: 'gitlab9') } before do - user.update(starred_projects: [project5, project7, project8, project9]) + user.update!(starred_projects: [project5, project7, project8, project9]) end context 'including owned filter' do @@ -642,7 +655,6 @@ RSpec.describe API::Projects do context 'non-admin user' do let(:current_user) { user } - let(:projects) { [public_project, project, project2, project3] } it 'returns projects ordered normally' do get api('/projects', current_user), params: { order_by: order_by } @@ -650,7 +662,7 @@ RSpec.describe API::Projects 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 { |project| project['id'] }).to eq(projects.map(&:id).reverse) + expect(json_response.map { |project| project['id'] }).to eq(user_projects.map(&:id).sort.reverse) end end end @@ -686,7 +698,8 @@ RSpec.describe API::Projects do context 'with keyset pagination' do let(:current_user) { user } - let(:projects) { [public_project, project, project2, project3] } + let(:first_project_id) { user_projects.map(&:id).min } + let(:last_project_id) { user_projects.map(&:id).max } context 'headers and records' do let(:params) { { pagination: 'keyset', order_by: :id, sort: :asc, per_page: 1 } } @@ -696,11 +709,11 @@ RSpec.describe API::Projects do expect(response.header).to include('Links') expect(response.header['Links']).to include('pagination=keyset') - expect(response.header['Links']).to include("id_after=#{public_project.id}") + expect(response.header['Links']).to include("id_after=#{first_project_id}") expect(response.header).to include('Link') expect(response.header['Link']).to include('pagination=keyset') - expect(response.header['Link']).to include("id_after=#{public_project.id}") + expect(response.header['Link']).to include("id_after=#{first_project_id}") end it 'contains only the first project with per_page = 1' do @@ -708,7 +721,7 @@ RSpec.describe API::Projects do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Array - expect(json_response.map { |p| p['id'] }).to contain_exactly(public_project.id) + expect(json_response.map { |p| p['id'] }).to contain_exactly(first_project_id) end it 'still includes a link if the end has reached and there is no more data after this page' do @@ -752,11 +765,11 @@ RSpec.describe API::Projects do expect(response.header).to include('Links') expect(response.header['Links']).to include('pagination=keyset') - expect(response.header['Links']).to include("id_before=#{project3.id}") + expect(response.header['Links']).to include("id_before=#{last_project_id}") expect(response.header).to include('Link') expect(response.header['Link']).to include('pagination=keyset') - expect(response.header['Link']).to include("id_before=#{project3.id}") + expect(response.header['Link']).to include("id_before=#{last_project_id}") end it 'contains only the last project with per_page = 1' do @@ -764,7 +777,7 @@ RSpec.describe API::Projects do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Array - expect(json_response.map { |p| p['id'] }).to contain_exactly(project3.id) + expect(json_response.map { |p| p['id'] }).to contain_exactly(last_project_id) end end @@ -793,7 +806,7 @@ RSpec.describe API::Projects do ids += Gitlab::Json.parse(response.body).map { |p| p['id'] } end - expect(ids).to contain_exactly(*projects.map(&:id)) + expect(ids).to contain_exactly(*user_projects.map(&:id)) end end end @@ -814,7 +827,7 @@ RSpec.describe API::Projects do .to change { Project.count }.by(1) expect(response).to have_gitlab_http_status(:created) - project = Project.first + project = Project.last expect(project.name).to eq('Foo Project') expect(project.path).to eq('foo-project') @@ -825,7 +838,7 @@ RSpec.describe API::Projects do .to change { Project.count }.by(1) expect(response).to have_gitlab_http_status(:created) - project = Project.first + project = Project.last expect(project.name).to eq('foo_project') expect(project.path).to eq('foo_project') @@ -836,7 +849,7 @@ RSpec.describe API::Projects do .to change { Project.count }.by(1) expect(response).to have_gitlab_http_status(:created) - project = Project.first + project = Project.last expect(project.name).to eq('Foo Project') expect(project.path).to eq('path-project-Foo') @@ -1232,7 +1245,7 @@ RSpec.describe API::Projects do describe 'GET /users/:user_id/starred_projects/' do before do - user3.update(starred_projects: [project, project2, project3]) + user3.update!(starred_projects: [project, project2, project3]) end it 'returns error when user not found' do @@ -1745,7 +1758,7 @@ RSpec.describe API::Projects do end it 'returns 404 when project is marked for deletion' do - project.update(pending_delete: true) + project.update!(pending_delete: true) get api("/projects/#{project.id}", user) @@ -1985,7 +1998,8 @@ RSpec.describe API::Projects do context 'when authenticated' do context 'valid request' do it_behaves_like 'project users response' do - let(:current_user) { user } + let(:project) { project4 } + let(:current_user) { user4 } end end @@ -2011,8 +2025,8 @@ RSpec.describe API::Projects do get api("/projects/#{project.id}/users?skip_users=#{user.id}", user) expect(response).to have_gitlab_http_status(:ok) - expect(json_response.size).to eq(1) - expect(json_response[0]['id']).to eq(other_user.id) + expect(json_response.size).to eq(2) + expect(json_response.map { |m| m['id'] }).not_to include(user.id) end end end @@ -2260,7 +2274,7 @@ RSpec.describe API::Projects do end it "returns a 400 error when sharing is disabled" do - project.namespace.update(share_with_group_lock: true) + project.namespace.update!(share_with_group_lock: true) post api("/projects/#{project.id}/share", user), params: { group_id: group.id, group_access: Gitlab::Access::DEVELOPER } expect(response).to have_gitlab_http_status(:bad_request) end @@ -2417,7 +2431,7 @@ RSpec.describe API::Projects do end it 'updates visibility_level from public to private' do - project3.update({ visibility_level: Gitlab::VisibilityLevel::PUBLIC }) + project3.update!({ visibility_level: Gitlab::VisibilityLevel::PUBLIC }) project_param = { visibility: 'private' } put api("/projects/#{project3.id}", user), params: project_param @@ -2877,8 +2891,8 @@ RSpec.describe API::Projects do let(:private_user) { create(:user, private_profile: true) } before do - user.update(starred_projects: [public_project]) - private_user.update(starred_projects: [public_project]) + user.update!(starred_projects: [public_project]) + private_user.update!(starred_projects: [public_project]) end it 'returns not_found(404) for not existing project' do diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index e2cfd87b507..e72ac002f6b 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -11,6 +11,7 @@ RSpec.describe API::PypiPackages do let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } + let_it_be(:job) { create(:ci_build, :running, user: user) } describe 'GET /api/v4/projects/:id/packages/pypi/simple/:package_name' do let_it_be(:package) { create(:pypi_package, project: project) } @@ -58,6 +59,8 @@ RSpec.describe API::PypiPackages do it_behaves_like 'deploy token for package GET requests' + it_behaves_like 'job token for package GET requests' + it_behaves_like 'rejects PyPI access with unknown project id' end @@ -108,6 +111,8 @@ RSpec.describe API::PypiPackages do it_behaves_like 'deploy token for package uploads' + it_behaves_like 'job token for package uploads' + it_behaves_like 'rejects PyPI access with unknown project id' end @@ -117,7 +122,8 @@ RSpec.describe API::PypiPackages do let_it_be(:file_name) { 'package.whl' } let(:url) { "/projects/#{project.id}/packages/pypi" } let(:headers) { {} } - let(:base_params) { { requires_python: '>=3.7', version: '1.0.0', name: 'sample-project', sha256_digest: '123' } } + let(:requires_python) { '>=3.7' } + let(:base_params) { { requires_python: requires_python, version: '1.0.0', name: 'sample-project', sha256_digest: '123' } } let(:params) { base_params.merge(content: temp_file(file_name)) } let(:send_rewritten_field) { true } @@ -169,6 +175,19 @@ RSpec.describe API::PypiPackages do end end + context 'with required_python too big' do + let(:requires_python) { 'x' * 256 } + let(:token) { personal_access_token.token } + let(:user_headers) { basic_auth_header(user.username, token) } + let(:headers) { user_headers.merge(workhorse_header) } + + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + + it_behaves_like 'process PyPi api request', :developer, :bad_request, true + end + context 'with an invalid package' do let(:token) { personal_access_token.token } let(:user_headers) { basic_auth_header(user.username, token) } @@ -184,7 +203,21 @@ RSpec.describe API::PypiPackages do it_behaves_like 'deploy token for package uploads' + it_behaves_like 'job token for package uploads' + it_behaves_like 'rejects PyPI access with unknown project id' + + context 'file size above maximum limit' do + let(:headers) { basic_auth_header(deploy_token.username, deploy_token.token).merge(workhorse_header) } + + before do + allow_next_instance_of(UploadedFile) do |uploaded_file| + allow(uploaded_file).to receive(:size).and_return(project.actual_limits.pypi_max_file_size + 1) + end + end + + it_behaves_like 'returning response status', :bad_request + end end describe 'GET /api/v4/projects/:id/packages/pypi/files/:sha256/*file_identifier' do @@ -247,6 +280,26 @@ RSpec.describe API::PypiPackages do end end + context 'with job token headers' do + let(:headers) { basic_auth_header(::Gitlab::Auth::CI_JOB_USER, job.token) } + + context 'valid token' do + it_behaves_like 'returning response status', :success + end + + context 'invalid token' do + let(:headers) { basic_auth_header(::Gitlab::Auth::CI_JOB_USER, 'bar') } + + it_behaves_like 'returning response status', :success + end + + context 'invalid user' do + let(:headers) { basic_auth_header('foo', job.token) } + + it_behaves_like 'returning response status', :success + end + end + it_behaves_like 'rejects PyPI access with unknown project id' end end diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index 1a93be98a67..af6731f3015 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -47,6 +47,17 @@ RSpec.describe API::Search do end end + shared_examples 'filter by state' do |scope:, search:| + it 'respects scope filtering' do + get api(endpoint, user), params: { scope: scope, search: search, state: state } + + documents = Gitlab::Json.parse(response.body) + + expect(documents.count).to eq(1) + expect(documents.first['state']).to eq(state) + end + end + describe 'GET /search' do let(:endpoint) { '/search' } @@ -88,42 +99,84 @@ RSpec.describe API::Search do end context 'for issues scope' do - before do - create(:issue, project: project, title: 'awesome issue') + context 'without filtering by state' do + before do + create(:issue, project: project, title: 'awesome issue') - get api(endpoint, user), params: { scope: 'issues', search: 'awesome' } - end + get api(endpoint, user), params: { scope: 'issues', search: 'awesome' } + end - it_behaves_like 'response is correct', schema: 'public_api/v4/issues' + it_behaves_like 'response is correct', schema: 'public_api/v4/issues' - it_behaves_like 'ping counters', scope: :issues + it_behaves_like 'ping counters', scope: :issues - describe 'pagination' do + describe 'pagination' do + before do + create(:issue, project: project, title: 'another issue') + end + + include_examples 'pagination', scope: :issues + end + end + + context 'filter by state' do before do - create(:issue, project: project, title: 'another issue') + create(:issue, project: project, title: 'awesome opened issue') + create(:issue, :closed, project: project, title: 'awesome closed issue') end - include_examples 'pagination', scope: :issues + context 'state: opened' do + let(:state) { 'opened' } + + include_examples 'filter by state', scope: :issues, search: 'awesome' + end + + context 'state: closed' do + let(:state) { 'closed' } + + include_examples 'filter by state', scope: :issues, search: 'awesome' + end end end context 'for merge_requests scope' do - before do - create(:merge_request, source_project: repo_project, title: 'awesome mr') + context 'without filtering by state' do + before do + create(:merge_request, source_project: repo_project, title: 'awesome mr') - get api(endpoint, user), params: { scope: 'merge_requests', search: 'awesome' } - end + get api(endpoint, user), params: { scope: 'merge_requests', search: 'awesome' } + end - it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests' + it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests' - it_behaves_like 'ping counters', scope: :merge_requests + it_behaves_like 'ping counters', scope: :merge_requests - describe 'pagination' do + describe 'pagination' do + before do + create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch') + end + + include_examples 'pagination', scope: :merge_requests + end + end + + context 'filter by state' do before do - create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch') + create(:merge_request, source_project: project, title: 'awesome opened mr') + create(:merge_request, :closed, project: project, title: 'awesome closed mr') end - include_examples 'pagination', scope: :merge_requests + context 'state: opened' do + let(:state) { 'opened' } + + include_examples 'filter by state', scope: :merge_requests, search: 'awesome' + end + + context 'state: closed' do + let(:state) { 'closed' } + + include_examples 'filter by state', scope: :merge_requests, search: 'awesome' + end end end diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 8db0cdcbc2c..ef12f6dbed3 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -31,7 +31,6 @@ RSpec.describe API::Settings, 'Settings' do expect(json_response['ecdsa_key_restriction']).to eq(0) expect(json_response['ed25519_key_restriction']).to eq(0) expect(json_response['performance_bar_allowed_group_id']).to be_nil - expect(json_response['instance_statistics_visibility_private']).to be(false) expect(json_response['allow_local_requests_from_hooks_and_services']).to be(false) expect(json_response['allow_local_requests_from_web_hooks_and_services']).to be(false) expect(json_response['allow_local_requests_from_system_hooks']).to be(true) @@ -40,6 +39,7 @@ RSpec.describe API::Settings, 'Settings' do expect(json_response['snippet_size_limit']).to eq(50.megabytes) expect(json_response['spam_check_endpoint_enabled']).to be_falsey expect(json_response['spam_check_endpoint_url']).to be_nil + expect(json_response['wiki_page_max_content_bytes']).to be_a(Integer) end end @@ -104,7 +104,6 @@ RSpec.describe API::Settings, 'Settings' do enforce_terms: true, terms: 'Hello world!', performance_bar_allowed_group_path: group.full_path, - instance_statistics_visibility_private: true, diff_max_patch_bytes: 150_000, default_branch_protection: ::Gitlab::Access::PROTECTION_DEV_CAN_MERGE, local_markdown_version: 3, @@ -118,7 +117,8 @@ RSpec.describe API::Settings, 'Settings' do spam_check_endpoint_enabled: true, spam_check_endpoint_url: 'https://example.com/spam_check', disabled_oauth_sign_in_sources: 'unknown', - import_sources: 'github,bitbucket' + import_sources: 'github,bitbucket', + wiki_page_max_content_bytes: 12345 } expect(response).to have_gitlab_http_status(:ok) @@ -146,7 +146,6 @@ RSpec.describe API::Settings, 'Settings' do expect(json_response['enforce_terms']).to be(true) expect(json_response['terms']).to eq('Hello world!') expect(json_response['performance_bar_allowed_group_id']).to eq(group.id) - expect(json_response['instance_statistics_visibility_private']).to be(true) expect(json_response['diff_max_patch_bytes']).to eq(150_000) expect(json_response['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) expect(json_response['local_markdown_version']).to eq(3) @@ -161,6 +160,7 @@ RSpec.describe API::Settings, 'Settings' do expect(json_response['spam_check_endpoint_url']).to eq('https://example.com/spam_check') expect(json_response['disabled_oauth_sign_in_sources']).to eq([]) expect(json_response['import_sources']).to match_array(%w(github bitbucket)) + expect(json_response['wiki_page_max_content_bytes']).to eq(12345) end end @@ -239,8 +239,7 @@ RSpec.describe API::Settings, 'Settings' do snowplow_collector_hostname: "snowplow.example.com", snowplow_cookie_domain: ".example.com", snowplow_enabled: true, - snowplow_app_id: "app_id", - snowplow_iglu_registry_url: 'https://example.com' + snowplow_app_id: "app_id" } end diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 4e2f6e108eb..8d77026d26c 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -111,7 +111,7 @@ RSpec.describe API::Snippets do end it 'returns 404 for invalid snippet id' do - snippet.destroy + snippet.destroy! get api("/snippets/#{snippet.id}/raw", author) @@ -201,7 +201,7 @@ RSpec.describe API::Snippets do end it 'returns 404 for invalid snippet id' do - private_snippet.destroy + private_snippet.destroy! subject @@ -368,7 +368,7 @@ RSpec.describe API::Snippets do context 'when the snippet is public' do let(:extra_params) { { visibility: 'public' } } - it 'rejects the shippet' do + it 'rejects the snippet' do expect { subject }.not_to change { Snippet.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -391,19 +391,16 @@ RSpec.describe API::Snippets do create(:personal_snippet, :repository, author: user, visibility_level: visibility_level) end - shared_examples 'snippet updates' do - it 'updates a snippet' do - new_content = 'New content' - new_description = 'New description' + it_behaves_like 'snippet file updates' + it_behaves_like 'snippet non-file updates' + it_behaves_like 'snippet individual non-file updates' + it_behaves_like 'invalid snippet updates' - update_snippet(params: { content: new_content, description: new_description, visibility: 'internal' }) + it "returns 404 for another user's snippet" do + update_snippet(requester: other_user, params: { title: 'foobar' }) - expect(response).to have_gitlab_http_status(:ok) - snippet.reload - expect(snippet.content).to eq(new_content) - expect(snippet.description).to eq(new_description) - expect(snippet.visibility).to eq('internal') - end + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Snippet Not Found') end context 'with restricted visibility settings' do @@ -413,43 +410,7 @@ RSpec.describe API::Snippets do Gitlab::VisibilityLevel::PRIVATE]) end - it_behaves_like 'snippet updates' - end - - it_behaves_like 'snippet updates' - - it 'returns 404 for invalid snippet id' do - update_snippet(snippet_id: non_existing_record_id, params: { title: 'Foo' }) - - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 Snippet Not Found') - end - - it "returns 404 for another user's snippet" do - update_snippet(requester: other_user, params: { title: 'foobar' }) - - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 Snippet Not Found') - end - - it 'returns 400 for missing parameters' do - update_snippet - - expect(response).to have_gitlab_http_status(:bad_request) - end - - it 'returns 400 if content is blank' do - update_snippet(params: { content: '' }) - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq 'content is empty' - end - - it 'returns 400 if title is blank' do - update_snippet(params: { title: '' }) - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq 'title is empty' + it_behaves_like 'snippet non-file updates' end it_behaves_like 'update with repository actions' do @@ -475,7 +436,7 @@ RSpec.describe API::Snippets do context 'when the snippet is public' do let(:visibility_level) { Snippet::PUBLIC } - it 'rejects the shippet' do + it 'rejects the snippet' do expect { update_snippet(params: { title: 'Foo' }) } .not_to change { snippet.reload.title } diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb index c47a12456c3..8d128bd911f 100644 --- a/spec/requests/api/terraform/state_spec.rb +++ b/spec/requests/api/terraform/state_spec.rb @@ -9,7 +9,7 @@ RSpec.describe API::Terraform::State do let_it_be(:developer) { create(:user, developer_projects: [project]) } let_it_be(:maintainer) { create(:user, maintainer_projects: [project]) } - let!(:state) { create(:terraform_state, :with_file, project: project) } + let!(:state) { create(:terraform_state, :with_version, project: project) } let(:current_user) { maintainer } let(:auth_header) { user_basic_auth_header(current_user) } @@ -42,7 +42,7 @@ RSpec.describe API::Terraform::State do request expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to eq(state.file.read) + expect(response.body).to eq(state.reload.latest_file.read) end context 'for a project that does not exist' do @@ -63,7 +63,7 @@ RSpec.describe API::Terraform::State do request expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to eq(state.file.read) + expect(response.body).to eq(state.reload.latest_file.read) end end end @@ -78,7 +78,7 @@ RSpec.describe API::Terraform::State do request expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to eq(state.file.read) + expect(response.body).to eq(state.reload.latest_file.read) end it 'returns unauthorized if the the job is not running' do @@ -106,14 +106,14 @@ RSpec.describe API::Terraform::State do request expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to eq(state.file.read) + expect(response.body).to eq(state.reload.latest_file.read) end end end end describe 'POST /projects/:id/terraform/state/:name' do - let(:params) { { 'instance': 'example-instance' } } + let(:params) { { 'instance': 'example-instance', 'serial': '1' } } subject(:request) { post api(state_path), headers: auth_header, as: :json, params: params } diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index dfd0e13d84c..bc315c5b3c6 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -34,6 +34,29 @@ RSpec.describe API::Todos do end context 'when authenticated' do + context 'when invalid params' do + context "invalid action" do + it 'returns 400' do + get api('/todos', john_doe), params: { action: 'InvalidAction' } + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context "invalid state" do + it 'returns 400' do + get api('/todos', john_doe), params: { state: 'InvalidState' } + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context "invalid type" do + it 'returns 400' do + get api('/todos', john_doe), params: { type: 'InvalidType' } + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + it 'returns an array of pending todos for current user' do get api('/todos', john_doe) diff --git a/spec/requests/api/usage_data_spec.rb b/spec/requests/api/usage_data_spec.rb new file mode 100644 index 00000000000..46dd54dcc73 --- /dev/null +++ b/spec/requests/api/usage_data_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::UsageData do + let_it_be(:user) { create(:user) } + + describe 'POST /usage_data/increment_unique_users' do + let(:endpoint) { '/usage_data/increment_unique_users' } + let(:known_event) { 'g_compliance_dashboard' } + let(:unknown_event) { 'unknown' } + + context 'without CSRF token' do + it 'returns forbidden' do + stub_feature_flags(usage_data_api: true) + allow(Gitlab::RequestForgeryProtection).to receive(:verified?).and_return(false) + + post api(endpoint, user), params: { event: known_event } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'usage_data_api feature not enabled' do + it 'returns not_found' do + stub_feature_flags(usage_data_api: false) + + post api(endpoint, user), params: { event: known_event } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'without authentication' do + it 'returns 401 response' do + post api(endpoint), params: { event: known_event } + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'with authentication' do + before do + stub_feature_flags(usage_data_api: true) + stub_feature_flags("usage_data_#{known_event}" => true) + stub_application_setting(usage_ping_enabled: true) + allow(Gitlab::RequestForgeryProtection).to receive(:verified?).and_return(true) + end + + context 'when event is missing from params' do + it 'returns bad request' do + post api(endpoint, user), params: {} + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'with correct params' do + it 'returns status ok' do + expect(Gitlab::Redis::HLL).to receive(:add) + + post api(endpoint, user), params: { event: known_event } + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'with unknown event' do + it 'returns status ok' do + expect(Gitlab::Redis::HLL).not_to receive(:add) + + post api(endpoint, user), params: { event: unknown_event } + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end +end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 6c6497a240b..806b586ef49 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -348,6 +348,26 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do expect(response).to match_response_schema('public_api/v4/user/basics') expect(json_response.first.keys).not_to include 'is_admin' end + + context 'exclude_internal param' do + let_it_be(:internal_user) { User.alert_bot } + + it 'returns all users when it is not set' do + get api("/users?exclude_internal=false", user) + + expect(response).to match_response_schema('public_api/v4/user/basics') + expect(response).to include_pagination_headers + expect(json_response.map { |u| u['id'] }).to include(internal_user.id) + end + + it 'returns all non internal users when it is set' do + get api("/users?exclude_internal=true", user) + + expect(response).to match_response_schema('public_api/v4/user/basics') + expect(response).to include_pagination_headers + expect(json_response.map { |u| u['id'] }).not_to include(internal_user.id) + end + end end context "when admin" do @@ -894,6 +914,50 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do expect(response).to have_gitlab_http_status(:ok) end + context 'updating password' do + def update_password(user, admin, password = User.random_password) + put api("/users/#{user.id}", admin), params: { password: password } + end + + context 'admin updates their own password' do + it 'does not force reset on next login' do + update_password(admin, admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(user.reload.password_expired?).to eq(false) + end + + it 'does not enqueue the `admin changed your password` email' do + expect { update_password(admin, admin) } + .not_to have_enqueued_mail(DeviseMailer, :password_change_by_admin) + end + + it 'enqueues the `password changed` email' do + expect { update_password(admin, admin) } + .to have_enqueued_mail(DeviseMailer, :password_change) + end + end + + context 'admin updates the password of another user' do + it 'forces reset on next login' do + update_password(user, admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(user.reload.password_expired?).to eq(true) + end + + it 'enqueues the `admin changed your password` email' do + expect { update_password(user, admin) } + .to have_enqueued_mail(DeviseMailer, :password_change_by_admin) + end + + it 'does not enqueue the `password changed` email' do + expect { update_password(user, admin) } + .not_to have_enqueued_mail(DeviseMailer, :password_change) + end + end + end + it "updates user with new bio" do put api("/users/#{user.id}", admin), params: { bio: 'new test bio' } @@ -920,13 +984,6 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do expect(user.reload.bio).to eq('') end - it "updates user with new password and forces reset on next login" do - put api("/users/#{user.id}", admin), params: { password: '12345678' } - - expect(response).to have_gitlab_http_status(:ok) - expect(user.reload.password_expires_at).to be <= Time.now - end - it "updates user with organization" do put api("/users/#{user.id}", admin), params: { organization: 'GitLab' } @@ -1377,7 +1434,7 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do end end - describe 'POST /users/:id/keys' do + describe 'POST /users/:id/gpg_keys' do it 'does not create invalid GPG key' do post api("/users/#{user.id}/gpg_keys", admin) diff --git a/spec/requests/api/v3/github_spec.rb b/spec/requests/api/v3/github_spec.rb new file mode 100644 index 00000000000..86ddf4a78d8 --- /dev/null +++ b/spec/requests/api/v3/github_spec.rb @@ -0,0 +1,516 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::V3::Github do + let(:user) { create(:user) } + let(:unauthorized_user) { create(:user) } + let(:admin) { create(:user, :admin) } + let(:project) { create(:project, :repository, creator: user) } + + before do + project.add_maintainer(user) + end + + describe 'GET /orgs/:namespace/repos' do + it 'returns an empty array' do + group = create(:group) + + jira_get v3_api("/orgs/#{group.path}/repos", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq([]) + end + + it 'returns 200 when namespace path include a dot' do + group = create(:group, path: 'foo.bar') + + jira_get v3_api("/orgs/#{group.path}/repos", user) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + describe 'GET /user/repos' do + it 'returns an empty array' do + jira_get v3_api('/user/repos', user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq([]) + end + end + + shared_examples_for 'Jira-specific mimicked GitHub endpoints' do + describe 'GET /.../issues/:id/comments' do + let(:merge_request) do + create(:merge_request, source_project: project, target_project: project) + end + + let!(:note) do + create(:note, project: project, noteable: merge_request) + end + + context 'when user has access to the merge request' do + it 'returns an array of notes' do + jira_get v3_api("/repos/#{path}/issues/#{merge_request.id}/comments", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an(Array) + expect(json_response.size).to eq(1) + end + end + + context 'when user has no access to the merge request' do + let(:project) { create(:project, :private) } + + before do + project.add_guest(user) + end + + it 'returns 404' do + jira_get v3_api("/repos/#{path}/issues/#{merge_request.id}/comments", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'GET /.../pulls/:id/commits' do + it 'returns an empty array' do + jira_get v3_api("/repos/#{path}/pulls/xpto/commits", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq([]) + end + end + + describe 'GET /.../pulls/:id/comments' do + it 'returns an empty array' do + jira_get v3_api("/repos/#{path}/pulls/xpto/comments", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq([]) + end + end + end + + # Here we test that using /-/jira as namespace/project still works, + # since that is how old Jira setups will talk to us + context 'old /-/jira endpoints' do + it_behaves_like 'Jira-specific mimicked GitHub endpoints' do + let(:path) { '-/jira' } + end + + it 'returns an empty Array for events' do + jira_get v3_api('/repos/-/jira/events', user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq([]) + end + end + + context 'new :namespace/:project jira endpoints' do + it_behaves_like 'Jira-specific mimicked GitHub endpoints' do + let(:path) { "#{project.namespace.path}/#{project.path}" } + end + + describe 'GET /users/:username' do + let!(:user1) { create(:user, username: 'jane.porter') } + + context 'user exists' do + it 'responds with the expected user' do + jira_get v3_api("/users/#{user.username}", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('entities/github/user') + end + end + + context 'user does not exist' do + it 'responds with the expected status' do + jira_get v3_api('/users/unknown_user_name', user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'no rights to request user lists' do + before do + expect(Ability).to receive(:allowed?).with(unauthorized_user, :read_users_list, :global).and_return(false) + expect(Ability).to receive(:allowed?).at_least(:once).and_call_original + end + + it 'responds with forbidden' do + jira_get v3_api("/users/#{user.username}", unauthorized_user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + + describe 'GET events' do + let(:group) { create(:group) } + let(:project) { create(:project, :empty_repo, path: 'project.with.dot', group: group) } + let(:events_path) { "/repos/#{group.path}/#{project.path}/events" } + + context 'if there are no merge requests' do + it 'returns an empty array' do + jira_get v3_api(events_path, user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq([]) + end + end + + context 'if there is a merge request' do + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } + + it 'returns an event' do + jira_get v3_api(events_path, user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an(Array) + expect(json_response.size).to eq(1) + end + end + + context 'if there are more merge requests' do + let!(:merge_request) { create(:merge_request, id: 10000, source_project: project, target_project: project, author: user) } + let!(:merge_request2) { create(:merge_request, id: 10001, source_project: project, source_branch: generate(:branch), target_project: project, author: user) } + + it 'returns the expected amount of events' do + jira_get v3_api(events_path, user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an(Array) + expect(json_response.size).to eq(2) + end + + it 'ensures each event has a unique id' do + jira_get v3_api(events_path, user) + + ids = json_response.map { |event| event['id'] }.uniq + expect(ids.size).to eq(2) + end + end + end + end + + describe 'repo pulls' do + let(:project2) { create(:project, :repository, creator: user) } + let(:assignee) { create(:user) } + let(:assignee2) { create(:user) } + let!(:merge_request) do + create(:merge_request, source_project: project, target_project: project, author: user, assignees: [assignee]) + end + + let!(:merge_request_2) do + create(:merge_request, source_project: project2, target_project: project2, author: user, assignees: [assignee, assignee2]) + end + + before do + project2.add_maintainer(user) + end + + describe 'GET /-/jira/pulls' do + it 'returns an array of merge requests with github format' do + jira_get v3_api('/repos/-/jira/pulls', user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an(Array) + expect(json_response.size).to eq(2) + expect(response).to match_response_schema('entities/github/pull_requests') + end + end + + describe 'GET /repos/:namespace/:project/pulls' do + it 'returns an array of merge requests for the proper project in github format' do + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/pulls", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an(Array) + expect(json_response.size).to eq(1) + expect(response).to match_response_schema('entities/github/pull_requests') + end + end + + describe 'GET /repos/:namespace/:project/pulls/:id' do + context 'when user has access to the merge requests' do + it 'returns the requested merge request in github format' do + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/pulls/#{merge_request.id}", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('entities/github/pull_request') + end + end + + context 'when user has no access to the merge request' do + it 'returns 404' do + project.add_guest(unauthorized_user) + + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/pulls/#{merge_request.id}", unauthorized_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when instance admin' do + it 'returns the requested merge request in github format' do + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/pulls/#{merge_request.id}", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('entities/github/pull_request') + end + end + end + end + + describe 'GET /users/:namespace/repos' do + let(:group) { create(:group, name: 'foo') } + + def expect_project_under_namespace(projects, namespace, user) + jira_get v3_api("/users/#{namespace.path}/repos", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(response).to match_response_schema('entities/github/repositories') + + projects.each do |project| + hash = json_response.find do |hash| + hash['name'] == ::Gitlab::Jira::Dvcs.encode_project_name(project) + end + + raise "Project #{project.full_path} not present in response" if hash.nil? + + expect(hash['owner']['login']).to eq(namespace.path) + end + expect(json_response.size).to eq(projects.size) + end + + context 'group namespace' do + let(:project) { create(:project, group: group) } + let!(:project2) { create(:project, :public, group: group) } + + it 'returns an array of projects belonging to group excluding the ones user is not directly a member of, even when public' do + expect_project_under_namespace([project], group, user) + end + + context 'when instance admin' do + let(:user) { create(:user, :admin) } + + it 'returns an array of projects belonging to group' do + expect_project_under_namespace([project, project2], group, user) + end + + context 'with a private group' do + let(:group) { create(:group, :private) } + let!(:project2) { create(:project, :private, group: group) } + + it 'returns an array of projects belonging to group' do + expect_project_under_namespace([project, project2], group, user) + end + end + end + end + + context 'nested group namespace' do + let(:group) { create(:group, :nested) } + let!(:parent_group_project) { create(:project, group: group.parent, name: 'parent_group_project') } + let!(:child_group_project) { create(:project, group: group, name: 'child_group_project') } + + before do + group.parent.add_maintainer(user) + end + + it 'returns an array of projects belonging to group with github format' do + expect_project_under_namespace([parent_group_project, child_group_project], group.parent, user) + end + + it 'avoids N+1 queries' do + jira_get v3_api("/users/#{group.parent.path}/repos", user) + + control = ActiveRecord::QueryRecorder.new { jira_get v3_api("/users/#{group.parent.path}/repos", user) } + + new_group = create(:group, parent: group.parent) + create(:project, :repository, group: new_group, creator: user) + + expect { jira_get v3_api("/users/#{group.parent.path}/repos", user) }.not_to exceed_query_limit(control) + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'user namespace' do + let(:project) { create(:project, namespace: user.namespace) } + + it 'returns an array of projects belonging to user namespace with github format' do + expect_project_under_namespace([project], user.namespace, user) + end + end + + context 'namespace path includes a dot' do + let(:project) { create(:project, group: group) } + let(:group) { create(:group, name: 'foo.bar') } + + before do + group.add_maintainer(user) + end + + it 'returns an array of projects belonging to group with github format' do + expect_project_under_namespace([project], group, user) + end + end + + context 'unauthenticated' do + it 'returns 401' do + jira_get v3_api('/users/foo/repos', nil) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'namespace does not exist' do + it 'responds with not found status' do + jira_get v3_api('/users/noo/repos', user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'GET /repos/:namespace/:project/branches' do + context 'authenticated' do + context 'updating project feature usage' do + it 'counts Jira Cloud integration as enabled' do + user_agent = 'Jira DVCS Connector Vertigo/4.42.0' + + Timecop.freeze do + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/branches", user), user_agent + + expect(project.reload.jira_dvcs_cloud_last_sync_at).to be_like_time(Time.now) + end + end + + it 'counts Jira Server integration as enabled' do + user_agent = 'Jira DVCS Connector/3.2.4' + + Timecop.freeze do + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/branches", user), user_agent + + expect(project.reload.jira_dvcs_server_last_sync_at).to be_like_time(Time.now) + end + end + end + + it 'returns an array of project branches with github format' do + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/branches", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an(Array) + + expect(response).to match_response_schema('entities/github/branches') + end + + it 'returns 200 when project path include a dot' do + project.update!(path: 'foo.bar') + + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/branches", user) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns 200 when namespace path include a dot' do + group = create(:group, path: 'foo.bar') + project = create(:project, :repository, group: group) + project.add_reporter(user) + + jira_get v3_api("/repos/#{group.path}/#{project.path}/branches", user) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'unauthenticated' do + it 'returns 401' do + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/branches", nil) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'unauthorized' do + it 'returns 404 when lower access level' do + project.add_guest(unauthorized_user) + + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/branches", unauthorized_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'GET /repos/:namespace/:project/commits/:sha' do + let(:commit) { project.repository.commit } + let(:commit_id) { commit.id } + + context 'authenticated' do + it 'returns commit with github format' do + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/commits/#{commit_id}", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('entities/github/commit') + end + + it 'returns 200 when project path include a dot' do + project.update!(path: 'foo.bar') + + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/commits/#{commit_id}", user) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns 200 when namespace path include a dot' do + group = create(:group, path: 'foo.bar') + project = create(:project, :repository, group: group) + project.add_reporter(user) + + jira_get v3_api("/repos/#{group.path}/#{project.path}/commits/#{commit_id}", user) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'unauthenticated' do + it 'returns 401' do + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/commits/#{commit_id}", nil) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'unauthorized' do + it 'returns 404 when lower access level' do + project.add_guest(unauthorized_user) + + jira_get v3_api("/repos/#{project.namespace.path}/#{project.path}/commits/#{commit_id}", + unauthorized_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + def jira_get(path, user_agent = 'Jira DVCS Connector/3.2.4') + get path, headers: { 'User-Agent' => user_agent } + end + + def v3_api(path, user = nil, personal_access_token: nil, oauth_access_token: nil) + api( + path, + user, + version: 'v3', + personal_access_token: personal_access_token, + oauth_access_token: oauth_access_token + ) + end +end diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb index 7bb73e9664b..1ae9b0d548d 100644 --- a/spec/requests/api/variables_spec.rb +++ b/spec/requests/api/variables_spec.rb @@ -60,25 +60,10 @@ RSpec.describe API::Variables do let!(:var2) { create(:ci_variable, project: project, key: 'key1', environment_scope: 'production') } context 'when filter[environment_scope] is not passed' do - context 'FF ci_variables_api_filter_environment_scope is enabled' do - it 'returns 409' do - get api("/projects/#{project.id}/variables/key1", user) + it 'returns 409' do + get api("/projects/#{project.id}/variables/key1", user) - expect(response).to have_gitlab_http_status(:conflict) - end - end - - context 'FF ci_variables_api_filter_environment_scope is disabled' do - before do - stub_feature_flags(ci_variables_api_filter_environment_scope: false) - end - - it 'returns random one' do - get api("/projects/#{project.id}/variables/key1", user) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['key']).to eq('key1') - end + expect(response).to have_gitlab_http_status(:conflict) end end @@ -232,25 +217,10 @@ RSpec.describe API::Variables do let!(:var2) { create(:ci_variable, project: project, key: 'key1', environment_scope: 'production') } context 'when filter[environment_scope] is not passed' do - context 'FF ci_variables_api_filter_environment_scope is enabled' do - it 'returns 409' do - get api("/projects/#{project.id}/variables/key1", user) + it 'returns 409' do + get api("/projects/#{project.id}/variables/key1", user) - expect(response).to have_gitlab_http_status(:conflict) - end - end - - context 'FF ci_variables_api_filter_environment_scope is disabled' do - before do - stub_feature_flags(ci_variables_api_filter_environment_scope: false) - end - - it 'updates random one' do - put api("/projects/#{project.id}/variables/key1", user), params: { value: 'new_val' } - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['value']).to eq('new_val') - end + expect(response).to have_gitlab_http_status(:conflict) end end @@ -312,26 +282,10 @@ RSpec.describe API::Variables do let!(:var2) { create(:ci_variable, project: project, key: 'key1', environment_scope: 'production') } context 'when filter[environment_scope] is not passed' do - context 'FF ci_variables_api_filter_environment_scope is enabled' do - it 'returns 409' do - get api("/projects/#{project.id}/variables/key1", user) - - expect(response).to have_gitlab_http_status(:conflict) - end - end - - context 'FF ci_variables_api_filter_environment_scope is disabled' do - before do - stub_feature_flags(ci_variables_api_filter_environment_scope: false) - end + it 'returns 409' do + get api("/projects/#{project.id}/variables/key1", user) - it 'deletes random one' do - expect 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 + expect(response).to have_gitlab_http_status(:conflict) end end |