diff options
Diffstat (limited to 'spec/requests')
87 files changed, 3285 insertions, 703 deletions
diff --git a/spec/requests/admin/background_migrations_controller_spec.rb b/spec/requests/admin/background_migrations_controller_spec.rb index 0fd2ba26cb8..884448fdd95 100644 --- a/spec/requests/admin/background_migrations_controller_spec.rb +++ b/spec/requests/admin/background_migrations_controller_spec.rb @@ -40,15 +40,17 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do describe 'GET #index' do let(:default_model) { ActiveRecord::Base } + let(:db_config) { instance_double(ActiveRecord::DatabaseConfigurations::HashConfig, name: 'fake_db') } before do + allow(Gitlab::Database).to receive(:db_config_for_connection).and_return(db_config) allow(Gitlab::Database).to receive(:database_base_models).and_return(base_models) end let!(:main_database_migration) { create(:batched_background_migration, :active) } context 'when no database is provided' do - let(:base_models) { { 'fake_db' => default_model } } + let(:base_models) { { 'fake_db' => default_model }.with_indifferent_access } before do stub_const('Gitlab::Database::MAIN_DATABASE_NAME', 'fake_db') @@ -68,7 +70,7 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do end context 'when multiple database is enabled', :add_ci_connection do - let(:base_models) { { 'fake_db' => default_model, 'ci' => ci_model } } + let(:base_models) { { 'fake_db' => default_model, 'ci' => ci_model }.with_indifferent_access } let(:ci_model) { Ci::ApplicationRecord } context 'when CI database is provided' do diff --git a/spec/requests/admin/batched_jobs_controller_spec.rb b/spec/requests/admin/batched_jobs_controller_spec.rb index 9a0654c64b4..fb51b3bce88 100644 --- a/spec/requests/admin/batched_jobs_controller_spec.rb +++ b/spec/requests/admin/batched_jobs_controller_spec.rb @@ -42,7 +42,7 @@ RSpec.describe Admin::BatchedJobsController, :enable_admin_mode do end context 'when multiple database is enabled', :add_ci_connection do - let(:base_models) { { 'fake_db' => default_model, 'ci' => ci_model } } + let(:base_models) { { 'main' => default_model, 'ci' => ci_model }.with_indifferent_access } let(:ci_model) { Ci::ApplicationRecord } before do diff --git a/spec/requests/api/bulk_imports_spec.rb b/spec/requests/api/bulk_imports_spec.rb index 3b8136f265b..9f9907f4f00 100644 --- a/spec/requests/api/bulk_imports_spec.rb +++ b/spec/requests/api/bulk_imports_spec.rb @@ -62,7 +62,7 @@ RSpec.describe API::BulkImports do entities: [ source_type: 'group_entity', source_full_path: 'full_path', - destination_name: 'destination_name', + destination_name: 'destination_slug', destination_namespace: 'destination_namespace' ] } @@ -82,7 +82,7 @@ RSpec.describe API::BulkImports do entities: [ source_type: 'group_entity', source_full_path: 'full_path', - destination_name: 'destination_name', + destination_name: 'destination_slug', destination_namespace: 'destination_namespace' ] } diff --git a/spec/requests/api/ci/job_artifacts_spec.rb b/spec/requests/api/ci/job_artifacts_spec.rb index 1dd1ca4e115..2fa1ffb4974 100644 --- a/spec/requests/api/ci/job_artifacts_spec.rb +++ b/spec/requests/api/ci/job_artifacts_spec.rb @@ -41,42 +41,58 @@ RSpec.describe API::Ci::JobArtifacts do describe 'DELETE /projects/:id/jobs/:job_id/artifacts' do let!(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) } - before do - delete api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) - end + context 'when project is not undergoing stats refresh' do + before do + delete api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) + end - context 'when user is anonymous' do - let(:api_user) { nil } + context 'when user is anonymous' do + let(:api_user) { nil } - it 'does not delete artifacts' do - expect(job.job_artifacts.size).to eq 2 - end + it 'does not delete artifacts' do + expect(job.job_artifacts.size).to eq 2 + end - it 'returns status 401 (unauthorized)' do - expect(response).to have_gitlab_http_status(:unauthorized) + it 'returns status 401 (unauthorized)' do + expect(response).to have_gitlab_http_status(:unauthorized) + end end - end - context 'with developer' do - it 'does not delete artifacts' do - expect(job.job_artifacts.size).to eq 2 + context 'with developer' do + it 'does not delete artifacts' do + expect(job.job_artifacts.size).to eq 2 + end + + it 'returns status 403 (forbidden)' do + expect(response).to have_gitlab_http_status(:forbidden) + end end - it 'returns status 403 (forbidden)' do - expect(response).to have_gitlab_http_status(:forbidden) + context 'with authorized user' do + let(:maintainer) { create(:project_member, :maintainer, project: project).user } + let!(:api_user) { maintainer } + + it 'deletes artifacts' do + expect(job.job_artifacts.size).to eq 0 + end + + it 'returns status 204 (no content)' do + expect(response).to have_gitlab_http_status(:no_content) + end end end - context 'with authorized user' do - let(:maintainer) { create(:project_member, :maintainer, project: project).user } - let!(:api_user) { maintainer } + context 'when project is undergoing stats refresh' do + it_behaves_like 'preventing request because of ongoing project stats refresh' do + let(:maintainer) { create(:project_member, :maintainer, project: project).user } + let(:api_user) { maintainer } + let(:make_request) { delete api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) } - it 'deletes artifacts' do - expect(job.job_artifacts.size).to eq 0 - end + it 'does not delete artifacts' do + make_request - it 'returns status 204 (no content)' do - expect(response).to have_gitlab_http_status(:no_content) + expect(job.job_artifacts.size).to eq 2 + end end end end @@ -131,6 +147,22 @@ RSpec.describe API::Ci::JobArtifacts do expect(response).to have_gitlab_http_status(:accepted) end + + context 'when project is undergoing stats refresh' do + let!(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) } + + it_behaves_like 'preventing request because of ongoing project stats refresh' do + let(:maintainer) { create(:project_member, :maintainer, project: project).user } + let(:api_user) { maintainer } + let(:make_request) { delete api("/projects/#{project.id}/artifacts", api_user) } + + it 'does not delete artifacts' do + make_request + + expect(job.job_artifacts.size).to eq 2 + end + end + end end end diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index 4bd9f81fd1d..84ef9f8db1b 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -239,6 +239,17 @@ RSpec.describe API::Ci::Jobs do end end + context 'when non-deployment environment action' do + let(:job) do + create(:environment, name: 'review', project_id: project.id) + create(:ci_build, :artifacts, :stop_review_app, environment: 'review', pipeline: pipeline, user: api_user, status: job_status) + end + + it 'includes environment slug' do + expect(json_response.dig('environment', 'slug')).to eq('review') + end + end + context 'when passing the token as params' do let(:headers) { {} } let(:params) { { job_token: job.token } } @@ -655,62 +666,80 @@ RSpec.describe API::Ci::Jobs do before do project.add_role(user, role) - - post api("/projects/#{project.id}/jobs/#{job.id}/erase", user) end - shared_examples_for 'erases job' do - it 'erases job content' do - expect(response).to have_gitlab_http_status(:created) - expect(job.job_artifacts.count).to eq(0) - expect(job.trace.exist?).to be_falsy - expect(job.artifacts_file.present?).to be_falsy - expect(job.artifacts_metadata.present?).to be_falsy - expect(job.has_job_artifacts?).to be_falsy + context 'when project is not undergoing stats refresh' do + before do + post api("/projects/#{project.id}/jobs/#{job.id}/erase", user) end - end - context 'job is erasable' do - let(:job) { create(:ci_build, :trace_artifact, :artifacts, :test_reports, :success, project: project, pipeline: pipeline) } + shared_examples_for 'erases job' do + it 'erases job content' do + expect(response).to have_gitlab_http_status(:created) + expect(job.job_artifacts.count).to eq(0) + expect(job.trace.exist?).to be_falsy + expect(job.artifacts_file.present?).to be_falsy + expect(job.artifacts_metadata.present?).to be_falsy + expect(job.has_job_artifacts?).to be_falsy + end + end + + context 'job is erasable' do + let(:job) { create(:ci_build, :trace_artifact, :artifacts, :test_reports, :success, project: project, pipeline: pipeline) } - it_behaves_like 'erases job' + it_behaves_like 'erases job' - it 'updates job' do - job.reload + it 'updates job' do + job.reload - expect(job.erased_at).to be_truthy - expect(job.erased_by).to eq(user) + expect(job.erased_at).to be_truthy + expect(job.erased_by).to eq(user) + end end - end - context 'when job has an unarchived trace artifact' do - let(:job) { create(:ci_build, :success, :trace_live, :unarchived_trace_artifact, project: project, pipeline: pipeline) } + context 'when job has an unarchived trace artifact' do + let(:job) { create(:ci_build, :success, :trace_live, :unarchived_trace_artifact, project: project, pipeline: pipeline) } - it_behaves_like 'erases job' - end + it_behaves_like 'erases job' + end - context 'job is not erasable' do - let(:job) { create(:ci_build, :trace_live, project: project, pipeline: pipeline) } + context 'job is not erasable' do + let(:job) { create(:ci_build, :trace_live, project: project, pipeline: pipeline) } - it 'responds with forbidden' do - expect(response).to have_gitlab_http_status(:forbidden) + it 'responds with forbidden' do + expect(response).to have_gitlab_http_status(:forbidden) + end end - end - context 'when a developer erases a build' do - let(:role) { :developer } - let(:job) { create(:ci_build, :trace_artifact, :artifacts, :success, project: project, pipeline: pipeline, user: owner) } + context 'when a developer erases a build' do + let(:role) { :developer } + let(:job) { create(:ci_build, :trace_artifact, :artifacts, :success, project: project, pipeline: pipeline, user: owner) } - context 'when the build was created by the developer' do - let(:owner) { user } + context 'when the build was created by the developer' do + let(:owner) { user } + + it { expect(response).to have_gitlab_http_status(:created) } + end - it { expect(response).to have_gitlab_http_status(:created) } + context 'when the build was created by another user' do + let(:owner) { create(:user) } + + it { expect(response).to have_gitlab_http_status(:forbidden) } + end end + end - context 'when the build was created by the other' do - let(:owner) { create(:user) } + context 'when project is undergoing stats refresh' do + let(:job) { create(:ci_build, :trace_artifact, :artifacts, :test_reports, :success, project: project, pipeline: pipeline) } - it { expect(response).to have_gitlab_http_status(:forbidden) } + it_behaves_like 'preventing request because of ongoing project stats refresh' do + let(:make_request) { post api("/projects/#{project.id}/jobs/#{job.id}/erase", user) } + + it 'does not delete artifacts' do + make_request + + expect(job.reload.job_artifacts).not_to be_empty + end end end end diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index 12faeec94da..697fe16e222 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -1018,6 +1018,18 @@ RSpec.describe API::Ci::Pipelines do expect { build.reload }.to raise_error(ActiveRecord::RecordNotFound) end end + + context 'when project is undergoing stats refresh' do + it_behaves_like 'preventing request because of ongoing project stats refresh' do + let(:make_request) { delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) } + + it 'does not delete the pipeline' do + make_request + + expect(pipeline.reload).to be_persisted + end + end + end end context 'unauthorized user' do diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index dbc5f0e74e2..3c6f9ac2816 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -216,7 +216,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(json_response['token']).to eq(job.token) expect(json_response['job_info']).to eq(expected_job_info) expect(json_response['git_info']).to eq(expected_git_info) - expect(json_response['image']).to eq({ 'name' => 'image:1.0', 'entrypoint' => '/bin/sh', 'ports' => [] }) + expect(json_response['image']).to eq({ 'name' => 'image:1.0', 'entrypoint' => '/bin/sh', 'ports' => [], 'pull_policy' => nil }) expect(json_response['services']).to eq([{ 'name' => 'postgres', 'entrypoint' => nil, 'alias' => nil, 'command' => nil, 'ports' => [], 'variables' => nil }, { 'name' => 'docker:stable-dind', 'entrypoint' => '/bin/sh', @@ -810,6 +810,45 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end end + context 'when image has pull_policy' do + let(:job) { create(:ci_build, :pending, :queued, pipeline: pipeline, options: options) } + + let(:options) do + { + image: { + name: 'ruby', + pull_policy: ['if-not-present'] + } + } + end + + it 'returns the image with pull policy' do + request_job + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to include( + 'id' => job.id, + 'image' => { 'name' => 'ruby', 'pull_policy' => ['if-not-present'], 'entrypoint' => nil, 'ports' => [] } + ) + end + + context 'when the FF ci_docker_image_pull_policy is disabled' do + before do + stub_feature_flags(ci_docker_image_pull_policy: false) + end + + it 'returns the image without pull policy' do + request_job + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to include( + 'id' => job.id, + 'image' => { 'name' => 'ruby', 'entrypoint' => nil, 'ports' => [] } + ) + end + end + end + describe 'a job with excluded artifacts' do context 'when excluded paths are defined' do let(:job) do diff --git a/spec/requests/api/clusters/agent_tokens_spec.rb b/spec/requests/api/clusters/agent_tokens_spec.rb index ba26faa45a3..2dca21ca6f1 100644 --- a/spec/requests/api/clusters/agent_tokens_spec.rb +++ b/spec/requests/api/clusters/agent_tokens_spec.rb @@ -37,7 +37,7 @@ RSpec.describe API::Clusters::AgentTokens do it 'cannot access agent tokens' do get api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens", unauthorized_user) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -85,7 +85,7 @@ RSpec.describe API::Clusters::AgentTokens do it 'cannot access single agent token' do get api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens/#{agent_token_one.id}", unauthorized_user) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:not_found) end it 'cannot access token from agent of another project' do diff --git a/spec/requests/api/clusters/agents_spec.rb b/spec/requests/api/clusters/agents_spec.rb index e29be255289..72d4266b9e3 100644 --- a/spec/requests/api/clusters/agents_spec.rb +++ b/spec/requests/api/clusters/agents_spec.rb @@ -11,6 +11,7 @@ RSpec.describe API::Clusters::Agents do before do project.add_maintainer(user) + project.add_guest(unauthorized_user) end describe 'GET /projects/:id/cluster_agents' do @@ -26,6 +27,19 @@ RSpec.describe API::Clusters::Agents do expect(json_response.first['name']).to eq(agent.name) end end + + it 'returns empty list when no agents registered' do + no_agents_project = create(:project, namespace: user.namespace) + + get api("/projects/#{no_agents_project.id}/cluster_agents", user) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(response).to match_response_schema('public_api/v4/agents') + expect(json_response.count).to eq(0) + end + end end context 'unauthorized user' do @@ -140,10 +154,10 @@ RSpec.describe API::Clusters::Agents do expect(response).to have_gitlab_http_status(:not_found) end - it 'returns a 404 if the user is unauthorized to delete' do + it 'returns a 403 if the user is unauthorized to delete' do delete api("/projects/#{project.id}/cluster_agents/#{agent.id}", unauthorized_user) - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:forbidden) end it_behaves_like '412 response' do diff --git a/spec/requests/api/environments_spec.rb b/spec/requests/api/environments_spec.rb index 8328b454122..93f21c880a4 100644 --- a/spec/requests/api/environments_spec.rb +++ b/spec/requests/api/environments_spec.rb @@ -113,7 +113,7 @@ RSpec.describe API::Environments do end context 'when filtering' do - let_it_be(:environment2) { create(:environment, project: project) } + let_it_be(:stopped_environment) { create(:environment, :stopped, project: project) } it 'returns environment by name' do get api("/projects/#{project.id}/environments?name=#{environment.name}", user) @@ -152,11 +152,32 @@ RSpec.describe API::Environments do expect(json_response.size).to eq(0) end - it 'returns a 400 status code with invalid states' do + it 'returns environment by valid state' do + get api("/projects/#{project.id}/environments?states=available", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.size).to eq(1) + expect(json_response.first['name']).to eq(environment.name) + end + + it 'returns all environments when state is not specified' do + get api("/projects/#{project.id}/environments", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.size).to eq(2) + expect(json_response.first['name']).to eq(environment.name) + expect(json_response.last['name']).to eq(stopped_environment.name) + end + + it 'returns a 400 when filtering by invalid state' do get api("/projects/#{project.id}/environments?states=test", user) expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to include('Requested states are invalid') + expect(json_response['error']).to eq('states does not have a valid value') end end end diff --git a/spec/requests/api/error_tracking/collector_spec.rb b/spec/requests/api/error_tracking/collector_spec.rb index c0d7eb5460f..dfca994d1c3 100644 --- a/spec/requests/api/error_tracking/collector_spec.rb +++ b/spec/requests/api/error_tracking/collector_spec.rb @@ -106,17 +106,30 @@ RSpec.describe API::ErrorTracking::Collector do end context 'gzip body' do - let(:headers) do + let(:standard_headers) do { 'X-Sentry-Auth' => "Sentry sentry_key=#{client_key.public_key}", - 'HTTP_CONTENT_ENCODING' => 'gzip', - 'CONTENT_TYPE' => 'application/x-sentry-envelope' + 'HTTP_CONTENT_ENCODING' => 'gzip' } end let(:params) { ActiveSupport::Gzip.compress(raw_event) } - it_behaves_like 'successful request' + context 'with application/x-sentry-envelope Content-Type' do + let(:headers) { standard_headers.merge({ 'CONTENT_TYPE' => 'application/x-sentry-envelope' }) } + + it_behaves_like 'successful request' + end + + context 'with unexpected Content-Type' do + let(:headers) { standard_headers.merge({ 'CONTENT_TYPE' => 'application/gzip' }) } + + it 'responds with 415' do + subject + + expect(response).to have_gitlab_http_status(:unsupported_media_type) + end + end end end diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb index 4e75b0510d0..b54be4f5258 100644 --- a/spec/requests/api/features_spec.rb +++ b/spec/requests/api/features_spec.rb @@ -168,19 +168,15 @@ RSpec.describe API::Features, stub_feature_flags: false do end end - shared_examples 'does not enable the flag' do |actor_type, actor_path| + shared_examples 'does not enable the flag' do |actor_type| + let(:actor_path) { raise NotImplementedError } + let(:expected_inexistent_path) { actor_path } + it 'returns the current state of the flag without changes' do post api("/features/#{feature_name}", admin), params: { value: 'true', actor_type => actor_path } - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - "name" => feature_name, - "state" => "off", - "gates" => [ - { "key" => "boolean", "value" => false } - ], - 'definition' => known_feature_flag_definition_hash - ) + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq("400 Bad request - #{expected_inexistent_path} is not found!") end end @@ -201,6 +197,19 @@ RSpec.describe API::Features, stub_feature_flags: false do end end + shared_examples 'creates an enabled feature for the specified entries' do + it do + post api("/features/#{feature_name}", admin), params: { value: 'true', **gate_params } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['name']).to eq(feature_name) + expect(json_response['gates']).to contain_exactly( + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'actors', 'value' => array_including(expected_gate_params) } + ) + end + end + context 'when enabling for a project by path' do context 'when the project exists' do it_behaves_like 'enables the flag for the actor', :project do @@ -209,7 +218,9 @@ RSpec.describe API::Features, stub_feature_flags: false do end context 'when the project does not exist' do - it_behaves_like 'does not enable the flag', :project, 'mep/to/the/mep/mep' + it_behaves_like 'does not enable the flag', :project do + let(:actor_path) { 'mep/to/the/mep/mep' } + end end end @@ -221,7 +232,9 @@ RSpec.describe API::Features, stub_feature_flags: false do end context 'when the group does not exist' do - it_behaves_like 'does not enable the flag', :group, 'not/a/group' + it_behaves_like 'does not enable the flag', :group do + let(:actor_path) { 'not/a/group' } + end end end @@ -239,7 +252,9 @@ RSpec.describe API::Features, stub_feature_flags: false do end context 'when the user namespace does not exist' do - it_behaves_like 'does not enable the flag', :namespace, 'not/a/group' + it_behaves_like 'does not enable the flag', :namespace do + let(:actor_path) { 'not/a/group' } + end end context 'when a project namespace exists' do @@ -251,6 +266,98 @@ RSpec.describe API::Features, stub_feature_flags: false do end end + context 'with multiple users' do + let_it_be(:users) { create_list(:user, 3) } + + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { user: users.map(&:username).join(',') } } + let(:expected_gate_params) { users.map(&:flipper_id) } + end + + context 'when empty value exists between comma' do + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { user: "#{users.first.username},,,," } } + let(:expected_gate_params) { users.first.flipper_id } + end + end + + context 'when one of the users does not exist' do + it_behaves_like 'does not enable the flag', :user do + let(:actor_path) { "#{users.first.username},inexistent-entry" } + let(:expected_inexistent_path) { "inexistent-entry" } + end + end + end + + context 'with multiple projects' do + let_it_be(:projects) { create_list(:project, 3) } + + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { project: projects.map(&:full_path).join(',') } } + let(:expected_gate_params) { projects.map(&:flipper_id) } + end + + context 'when empty value exists between comma' do + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { project: "#{projects.first.full_path},,,," } } + let(:expected_gate_params) { projects.first.flipper_id } + end + end + + context 'when one of the projects does not exist' do + it_behaves_like 'does not enable the flag', :project do + let(:actor_path) { "#{projects.first.full_path},inexistent-entry" } + let(:expected_inexistent_path) { "inexistent-entry" } + end + end + end + + context 'with multiple groups' do + let_it_be(:groups) { create_list(:group, 3) } + + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { group: groups.map(&:full_path).join(',') } } + let(:expected_gate_params) { groups.map(&:flipper_id) } + end + + context 'when empty value exists between comma' do + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { group: "#{groups.first.full_path},,,," } } + let(:expected_gate_params) { groups.first.flipper_id } + end + end + + context 'when one of the groups does not exist' do + it_behaves_like 'does not enable the flag', :group do + let(:actor_path) { "#{groups.first.full_path},inexistent-entry" } + let(:expected_inexistent_path) { "inexistent-entry" } + end + end + end + + context 'with multiple namespaces' do + let_it_be(:namespaces) { create_list(:namespace, 3) } + + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { namespace: namespaces.map(&:full_path).join(',') } } + let(:expected_gate_params) { namespaces.map(&:flipper_id) } + end + + context 'when empty value exists between comma' do + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { namespace: "#{namespaces.first.full_path},,,," } } + let(:expected_gate_params) { namespaces.first.flipper_id } + end + end + + context 'when one of the namespaces does not exist' do + it_behaves_like 'does not enable the flag', :namespace do + let(:actor_path) { "#{namespaces.first.full_path},inexistent-entry" } + let(:expected_inexistent_path) { "inexistent-entry" } + end + end + end + it 'creates a feature with the given percentage of time if passed an integer' do post api("/features/#{feature_name}", admin), params: { value: '50' } diff --git a/spec/requests/api/graphql/ci/jobs_spec.rb b/spec/requests/api/graphql/ci/jobs_spec.rb index 2d1bb45390b..d1737fc22ae 100644 --- a/spec/requests/api/graphql/ci/jobs_spec.rb +++ b/spec/requests/api/graphql/ci/jobs_spec.rb @@ -258,4 +258,81 @@ RSpec.describe 'Query.project.pipeline' do end end end + + describe '.jobs.count' do + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + let_it_be(:successful_job) { create(:ci_build, :success, pipeline: pipeline) } + let_it_be(:pending_job) { create(:ci_build, :pending, pipeline: pipeline) } + let_it_be(:failed_job) { create(:ci_build, :failed, pipeline: pipeline) } + + let(:query) do + %( + query { + project(fullPath: "#{project.full_path}") { + pipeline(iid: "#{pipeline.iid}") { + jobs { + count + } + } + } + } + ) + end + + before do + post_graphql(query, current_user: user) + end + + it 'returns the number of jobs' do + expect(graphql_data_at(:project, :pipeline, :jobs, :count)).to eq(3) + end + + context 'with limit value' do + let(:limit) { 1 } + + let(:query) do + %( + query { + project(fullPath: "#{project.full_path}") { + pipeline(iid: "#{pipeline.iid}") { + jobs { + count(limit: #{limit}) + } + } + } + } + ) + end + + it 'returns a limited number of jobs' do + expect(graphql_data_at(:project, :pipeline, :jobs, :count)).to eq(2) + end + + context 'with invalid value' do + let(:limit) { 1500 } + + it 'returns a validation error' do + expect(graphql_errors).to include(a_hash_including('message' => 'limit must be less than or equal to 1000')) + end + end + end + + context 'with jobs filter' do + let(:query) do + %( + query { + project(fullPath: "#{project.full_path}") { + jobs(statuses: FAILED) { + count + } + } + } + ) + end + + it 'returns the number of failed jobs' do + expect(graphql_data_at(:project, :jobs, :count)).to eq(1) + end + end + end end diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index 6fa455cbfca..446d1fb1bdb 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -12,7 +12,7 @@ RSpec.describe 'Query.runner(id)' do create(:ci_runner, :instance, description: 'Runner 1', contacted_at: 2.hours.ago, active: true, version: 'adfe156', revision: 'a', locked: true, ip_address: '127.0.0.1', maximum_timeout: 600, access_level: 0, tag_list: %w[tag1 tag2], run_untagged: true, executor_type: :custom, - maintenance_note: 'Test maintenance note') + maintenance_note: '**Test maintenance note**') end let_it_be(:inactive_instance_runner) do @@ -66,6 +66,8 @@ RSpec.describe 'Query.runner(id)' do 'architectureName' => runner.architecture, 'platformName' => runner.platform, 'maintenanceNote' => runner.maintenance_note, + 'maintenanceNoteHtml' => + runner.maintainer_note.present? ? a_string_including('<strong>Test maintenance note</strong>') : '', 'jobCount' => 0, 'jobs' => a_hash_including("count" => 0, "nodes" => [], "pageInfo" => anything), 'projectCount' => nil, @@ -150,34 +152,72 @@ RSpec.describe 'Query.runner(id)' do end describe 'for project runner' do - using RSpec::Parameterized::TableSyntax + describe 'locked' do + using RSpec::Parameterized::TableSyntax - where(is_locked: [true, false]) + where(is_locked: [true, false]) - with_them do - let(:project_runner) do - create(:ci_runner, :project, description: 'Runner 3', contacted_at: 1.day.ago, active: false, locked: is_locked, - version: 'adfe157', revision: 'b', ip_address: '10.10.10.10', access_level: 1, run_untagged: true) - end + with_them do + let(:project_runner) do + create(:ci_runner, :project, description: 'Runner 3', contacted_at: 1.day.ago, active: false, locked: is_locked, + version: 'adfe157', revision: 'b', ip_address: '10.10.10.10', access_level: 1, run_untagged: true) + end - let(:query) do - wrap_fields(query_graphql_path(query_path, all_graphql_fields_for('CiRunner'))) + let(:query) do + wrap_fields(query_graphql_path(query_path, 'id locked')) + end + + let(:query_path) do + [ + [:runner, { id: project_runner.to_global_id.to_s }] + ] + end + + it 'retrieves correct locked value' do + post_graphql(query, current_user: user) + + runner_data = graphql_data_at(:runner) + + expect(runner_data).to match a_hash_including( + 'id' => project_runner.to_global_id.to_s, + 'locked' => is_locked + ) + end end + end - let(:query_path) do - [ - [:runner, { id: project_runner.to_global_id.to_s }] - ] + describe 'ownerProject' do + let_it_be(:project1) { create(:project) } + let_it_be(:project2) { create(:project) } + let_it_be(:runner1) { create(:ci_runner, :project, projects: [project2, project1]) } + let_it_be(:runner2) { create(:ci_runner, :project, projects: [project1, project2]) } + + let(:runner_query_fragment) { 'id ownerProject { id }' } + let(:query) do + %( + query { + runner1: runner(id: "#{runner1.to_global_id}") { #{runner_query_fragment} } + runner2: runner(id: "#{runner2.to_global_id}") { #{runner_query_fragment} } + } + ) end - it 'retrieves correct locked value' do + it 'retrieves correct ownerProject.id values' do post_graphql(query, current_user: user) - runner_data = graphql_data_at(:runner) - - expect(runner_data).to match a_hash_including( - 'id' => project_runner.to_global_id.to_s, - 'locked' => is_locked + expect(graphql_data).to match a_hash_including( + 'runner1' => { + 'id' => runner1.to_global_id.to_s, + 'ownerProject' => { + 'id' => project2.to_global_id.to_s + } + }, + 'runner2' => { + 'id' => runner2.to_global_id.to_s, + 'ownerProject' => { + 'id' => project1.to_global_id.to_s + } + } ) end end @@ -405,17 +445,35 @@ RSpec.describe 'Query.runner(id)' do <<~SINGLE runner(id: "#{runner.to_global_id}") { #{all_graphql_fields_for('CiRunner', excluded: excluded_fields)} + groups { + nodes { + id + } + } + projects { + nodes { + id + } + } + ownerProject { + id + } } SINGLE end - # Currently excluding a known N+1 issue, see https://gitlab.com/gitlab-org/gitlab/-/issues/334759 - let(:excluded_fields) { %w[jobCount] } + let(:active_project_runner2) { create(:ci_runner, :project) } + let(:active_group_runner2) { create(:ci_runner, :group) } + + # Currently excluding known N+1 issues, see https://gitlab.com/gitlab-org/gitlab/-/issues/334759 + let(:excluded_fields) { %w[jobCount groups projects ownerProject] } let(:single_query) do <<~QUERY { - active: #{runner_query(active_instance_runner)} + instance_runner1: #{runner_query(active_instance_runner)} + project_runner1: #{runner_query(active_project_runner)} + group_runner1: #{runner_query(active_group_runner)} } QUERY end @@ -423,22 +481,51 @@ RSpec.describe 'Query.runner(id)' do let(:double_query) do <<~QUERY { - active: #{runner_query(active_instance_runner)} - inactive: #{runner_query(inactive_instance_runner)} + instance_runner1: #{runner_query(active_instance_runner)} + instance_runner2: #{runner_query(inactive_instance_runner)} + group_runner1: #{runner_query(active_group_runner)} + group_runner2: #{runner_query(active_group_runner2)} + project_runner1: #{runner_query(active_project_runner)} + project_runner2: #{runner_query(active_project_runner2)} } QUERY end it 'does not execute more queries per runner', :aggregate_failures do # warm-up license cache and so on: - post_graphql(single_query, current_user: user) + post_graphql(double_query, current_user: user) control = ActiveRecord::QueryRecorder.new { post_graphql(single_query, current_user: user) } expect { post_graphql(double_query, current_user: user) } .not_to exceed_query_limit(control) - expect(graphql_data_at(:active)).not_to be_nil - expect(graphql_data_at(:inactive)).not_to be_nil + + expect(graphql_data.count).to eq 6 + expect(graphql_data).to match( + a_hash_including( + 'instance_runner1' => a_hash_including('id' => active_instance_runner.to_global_id.to_s), + 'instance_runner2' => a_hash_including('id' => inactive_instance_runner.to_global_id.to_s), + 'group_runner1' => a_hash_including( + 'id' => active_group_runner.to_global_id.to_s, + 'groups' => { 'nodes' => [a_hash_including('id' => group.to_global_id.to_s)] } + ), + 'group_runner2' => a_hash_including( + 'id' => active_group_runner2.to_global_id.to_s, + 'groups' => { 'nodes' => [a_hash_including('id' => active_group_runner2.groups[0].to_global_id.to_s)] } + ), + 'project_runner1' => a_hash_including( + 'id' => active_project_runner.to_global_id.to_s, + 'projects' => { 'nodes' => [a_hash_including('id' => active_project_runner.projects[0].to_global_id.to_s)] }, + 'ownerProject' => a_hash_including('id' => active_project_runner.projects[0].to_global_id.to_s) + ), + 'project_runner2' => a_hash_including( + 'id' => active_project_runner2.to_global_id.to_s, + 'projects' => { + 'nodes' => [a_hash_including('id' => active_project_runner2.projects[0].to_global_id.to_s)] + }, + 'ownerProject' => a_hash_including('id' => active_project_runner2.projects[0].to_global_id.to_s) + ) + )) end end end diff --git a/spec/requests/api/graphql/ci/runners_spec.rb b/spec/requests/api/graphql/ci/runners_spec.rb index d3e94671724..a5b8115286e 100644 --- a/spec/requests/api/graphql/ci/runners_spec.rb +++ b/spec/requests/api/graphql/ci/runners_spec.rb @@ -18,7 +18,10 @@ RSpec.describe 'Query.runners' do let(:fields) do <<~QUERY nodes { - #{all_graphql_fields_for('CiRunner')} + #{all_graphql_fields_for('CiRunner', excluded: %w[ownerProject])} + ownerProject { + id + } } QUERY end @@ -123,3 +126,47 @@ RSpec.describe 'Query.runners' do end end end + +RSpec.describe 'Group.runners' do + include GraphqlHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:group_owner) { create_default(:user) } + + before do + group.add_owner(group_owner) + end + + describe 'edges' do + let_it_be(:runner) do + create(:ci_runner, :group, active: false, version: 'def', revision: '456', + description: 'Project runner', groups: [group], ip_address: '127.0.0.1') + end + + let(:query) do + %( + query($path: ID!) { + group(fullPath: $path) { + runners { + edges { + webUrl + editUrl + node { #{all_graphql_fields_for('CiRunner')} } + } + } + } + } + ) + end + + it 'contains custom edge information' do + r = GitlabSchema.execute(query, + context: { current_user: group_owner }, + variables: { path: group.full_path }) + + edges = graphql_dig_at(r.to_h, :data, :group, :runners, :edges) + + expect(edges).to contain_exactly(a_graphql_entity_for(web_url: be_present, edit_url: be_present)) + end + end +end diff --git a/spec/requests/api/graphql/gitlab_schema_spec.rb b/spec/requests/api/graphql/gitlab_schema_spec.rb index e80f5e0e0ff..c1beadb6c45 100644 --- a/spec/requests/api/graphql/gitlab_schema_spec.rb +++ b/spec/requests/api/graphql/gitlab_schema_spec.rb @@ -190,7 +190,7 @@ RSpec.describe 'GitlabSchema configurations' do let(:query) { File.read(Rails.root.join('spec/fixtures/api/graphql/introspection.graphql')) } it 'logs the query complexity and depth' do - expect_any_instance_of(Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer).to receive(:duration).and_return(7) + expect_any_instance_of(Gitlab::Graphql::QueryAnalyzers::AST::LoggerAnalyzer).to receive(:duration).and_return(7) expect(Gitlab::GraphqlLogger).to receive(:info).with( hash_including( diff --git a/spec/requests/api/graphql/issue/issue_spec.rb b/spec/requests/api/graphql/issue/issue_spec.rb index 05fd6bf3022..6e2d736f244 100644 --- a/spec/requests/api/graphql/issue/issue_spec.rb +++ b/spec/requests/api/graphql/issue/issue_spec.rb @@ -129,6 +129,29 @@ RSpec.describe 'Query.issue(id)' do expect(graphql_errors.first['message']).to eq("\"#{gid}\" does not represent an instance of Issue") end end + + context 'when selecting `closed_as_duplicate_of`' do + let(:issue_fields) { ['closedAsDuplicateOf { id }'] } + let(:duplicate_issue) { create(:issue, project: project) } + + before do + issue.update!(duplicated_to_id: duplicate_issue.id) + + post_graphql(query, current_user: current_user) + end + + it 'returns the related issue' do + expect(issue_data['closedAsDuplicateOf']['id']).to eq(duplicate_issue.to_global_id.to_s) + end + + context 'no permission to related issue' do + let(:duplicate_issue) { create(:issue) } + + it 'does not return the related issue' do + expect(issue_data['closedAsDuplicateOf']).to eq(nil) + end + end + end end context 'when there is a confidential issue' do diff --git a/spec/requests/api/graphql/milestone_spec.rb b/spec/requests/api/graphql/milestone_spec.rb index 59de116fa2b..f6835936418 100644 --- a/spec/requests/api/graphql/milestone_spec.rb +++ b/spec/requests/api/graphql/milestone_spec.rb @@ -5,43 +5,125 @@ require 'spec_helper' RSpec.describe 'Querying a Milestone' do include GraphqlHelpers - let_it_be(:current_user) { create(:user) } + let_it_be(:guest) { create(:user) } let_it_be(:project) { create(:project) } let_it_be(:milestone) { create(:milestone, project: project) } + let_it_be(:release_a) { create(:release, project: project) } + let_it_be(:release_b) { create(:release, project: project) } - let(:query) do - graphql_query_for('milestone', { id: milestone.to_global_id.to_s }, 'title') + before_all do + milestone.releases << [release_a, release_b] + project.add_guest(guest) end - subject { graphql_data['milestone'] } - - before do - post_graphql(query, current_user: current_user) + let(:expected_release_nodes) do + contain_exactly(a_graphql_entity_for(release_a), a_graphql_entity_for(release_b)) end - context 'when the user has access to the milestone' do - before_all do - project.add_guest(current_user) + context 'when we post the query' do + let(:current_user) { nil } + let(:query) do + graphql_query_for('milestone', { id: milestone.to_global_id.to_s }, all_graphql_fields_for('Milestone')) end - it_behaves_like 'a working graphql query' + subject { graphql_data['milestone'] } - it { is_expected.to include('title' => milestone.name) } - end + before do + post_graphql(query, current_user: current_user) + end - context 'when the user does not have access to the milestone' do - it_behaves_like 'a working graphql query' + context 'when the user has access to the milestone' do + let(:current_user) { guest } - it { is_expected.to be_nil } + it_behaves_like 'a working graphql query' + + it { is_expected.to include('title' => milestone.name) } + + it 'contains release information' do + is_expected.to include('releases' => include('nodes' => expected_release_nodes)) + end + end + + context 'when the user does not have access to the milestone' do + it_behaves_like 'a working graphql query' + + it { is_expected.to be_nil } + end + + context 'when ID argument is missing' do + let(:query) do + graphql_query_for('milestone', {}, 'title') + end + + it 'raises an exception' do + expect(graphql_errors).to include(a_hash_including('message' => "Field 'milestone' is missing required arguments: id")) + end + end end - context 'when ID argument is missing' do - let(:query) do - graphql_query_for('milestone', {}, 'title') + context 'when there are two milestones' do + let_it_be(:milestone_b) { create(:milestone, project: project) } + + let(:current_user) { guest } + let(:milestone_fields) do + <<~GQL + fragment milestoneFields on Milestone { + #{all_graphql_fields_for('Milestone', max_depth: 1)} + releases { nodes { #{all_graphql_fields_for('Release', max_depth: 1)} } } + } + GQL + end + + let(:single_query) do + <<~GQL + query ($id_a: MilestoneID!) { + a: milestone(id: $id_a) { ...milestoneFields } + } + + #{milestone_fields} + GQL + end + + let(:multi_query) do + <<~GQL + query ($id_a: MilestoneID!, $id_b: MilestoneID!) { + a: milestone(id: $id_a) { ...milestoneFields } + b: milestone(id: $id_b) { ...milestoneFields } + } + #{milestone_fields} + GQL + end + + it 'produces correct results' do + r = run_with_clean_state(multi_query, + context: { current_user: current_user }, + variables: { + id_a: global_id_of(milestone).to_s, + id_b: milestone_b.to_global_id.to_s + }) + + expect(r.to_h['errors']).to be_blank + expect(graphql_dig_at(r.to_h, :data, :a, :releases, :nodes)).to match expected_release_nodes + expect(graphql_dig_at(r.to_h, :data, :b, :releases, :nodes)).to be_empty end - it 'raises an exception' do - expect(graphql_errors).to include(a_hash_including('message' => "Field 'milestone' is missing required arguments: id")) + it 'does not suffer from N+1 performance issues' do + baseline = ActiveRecord::QueryRecorder.new do + run_with_clean_state(single_query, + context: { current_user: current_user }, + variables: { id_a: milestone.to_global_id.to_s }) + end + + multi = ActiveRecord::QueryRecorder.new do + run_with_clean_state(multi_query, + context: { current_user: current_user }, + variables: { + id_a: milestone.to_global_id.to_s, + id_b: milestone_b.to_global_id.to_s + }) + end + + expect(multi).not_to exceed_query_limit(baseline) end 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 index 37656ab4eea..7abd5ca8772 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb @@ -28,4 +28,21 @@ RSpec.describe 'PipelineDestroy' do expect(response).to have_gitlab_http_status(:success) expect { pipeline.reload }.to raise_error(ActiveRecord::RecordNotFound) end + + context 'when project is undergoing stats refresh' do + before do + create(:project_build_artifacts_size_refresh, :pending, project: pipeline.project) + end + + it 'returns an error and does not destroy the pipeline' do + expect(Gitlab::ProjectStatsRefreshConflictsLogger) + .to receive(:warn_request_rejected_during_stats_refresh) + .with(pipeline.project.id) + + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_mutation_response(:pipeline_destroy)['errors']).not_to be_empty + expect(pipeline.reload).to be_persisted + end + end end diff --git a/spec/requests/api/graphql/mutations/container_repository/destroy_tags_spec.rb b/spec/requests/api/graphql/mutations/container_repository/destroy_tags_spec.rb index decb2e7bccc..ef00f45ef18 100644 --- a/spec/requests/api/graphql/mutations/container_repository/destroy_tags_spec.rb +++ b/spec/requests/api/graphql/mutations/container_repository/destroy_tags_spec.rb @@ -91,7 +91,7 @@ RSpec.describe 'Destroying a container repository tags' do it 'returns too many tags error' do expect { subject }.not_to change { ::Packages::Event.count } - explanation = graphql_errors.dig(0, 'extensions', 'problems', 0, 'explanation') + explanation = graphql_errors.dig(0, 'message') expect(explanation).to eq(Mutations::ContainerRepositories::DestroyTags::TOO_MANY_TAGS_ERROR_MESSAGE) end end diff --git a/spec/requests/api/graphql/mutations/design_management/delete_spec.rb b/spec/requests/api/graphql/mutations/design_management/delete_spec.rb index 1f43f113e65..e2ab08b301b 100644 --- a/spec/requests/api/graphql/mutations/design_management/delete_spec.rb +++ b/spec/requests/api/graphql/mutations/design_management/delete_spec.rb @@ -47,7 +47,7 @@ RSpec.describe "deleting designs" do context 'the designs list is empty' do it_behaves_like 'a failed request' do let(:designs) { [] } - let(:the_error) { a_string_matching %r/was provided invalid value/ } + let(:the_error) { a_string_matching %r/no filenames/ } end end diff --git a/spec/requests/api/graphql/mutations/incident_management/timeline_event/create_spec.rb b/spec/requests/api/graphql/mutations/incident_management/timeline_event/create_spec.rb index 3ea8b38e20f..923e12a3c06 100644 --- a/spec/requests/api/graphql/mutations/incident_management/timeline_event/create_spec.rb +++ b/spec/requests/api/graphql/mutations/incident_management/timeline_event/create_spec.rb @@ -53,7 +53,7 @@ RSpec.describe 'Creating an incident timeline event' do }, 'note' => note, 'action' => 'comment', - 'editable' => false, + 'editable' => true, 'occurredAt' => event_occurred_at.iso8601 ) end diff --git a/spec/requests/api/graphql/mutations/incident_management/timeline_event/destroy_spec.rb b/spec/requests/api/graphql/mutations/incident_management/timeline_event/destroy_spec.rb index faff3bfe23a..85208869ad9 100644 --- a/spec/requests/api/graphql/mutations/incident_management/timeline_event/destroy_spec.rb +++ b/spec/requests/api/graphql/mutations/incident_management/timeline_event/destroy_spec.rb @@ -56,7 +56,7 @@ RSpec.describe 'Removing an incident timeline event' do }, 'note' => timeline_event.note, 'noteHtml' => timeline_event.note_html, - 'editable' => false, + 'editable' => true, 'action' => timeline_event.action, 'occurredAt' => timeline_event.occurred_at.iso8601, 'createdAt' => timeline_event.created_at.iso8601, diff --git a/spec/requests/api/graphql/mutations/incident_management/timeline_event/promote_from_note_spec.rb b/spec/requests/api/graphql/mutations/incident_management/timeline_event/promote_from_note_spec.rb index b92f6af1d3d..9272e218172 100644 --- a/spec/requests/api/graphql/mutations/incident_management/timeline_event/promote_from_note_spec.rb +++ b/spec/requests/api/graphql/mutations/incident_management/timeline_event/promote_from_note_spec.rb @@ -55,7 +55,7 @@ RSpec.describe 'Promote an incident timeline event from a comment' do }, 'note' => comment.note, 'action' => 'comment', - 'editable' => false, + 'editable' => true, 'occurredAt' => comment.created_at.iso8601 ) end diff --git a/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb b/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb index 715507c3cc5..395a490bfc3 100644 --- a/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb @@ -102,18 +102,6 @@ RSpec.describe 'Setting issues crm contacts' do group.add_reporter(user) end - context 'when the feature is disabled' do - before do - stub_feature_flags(customer_relations: false) - end - - it 'raises expected error' do - post_graphql_mutation(mutation, current_user: user) - - expect(graphql_errors).to include(a_hash_including('message' => 'Feature disabled')) - end - end - it_behaves_like 'successful mutation' context 'when the contact does not exist' do diff --git a/spec/requests/api/graphql/mutations/issues/set_escalation_status_spec.rb b/spec/requests/api/graphql/mutations/issues/set_escalation_status_spec.rb index 0166871502b..a81364d37b2 100644 --- a/spec/requests/api/graphql/mutations/issues/set_escalation_status_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/set_escalation_status_spec.rb @@ -49,14 +49,6 @@ RSpec.describe 'Setting the escalation status of an incident' do it_behaves_like 'a mutation that returns top-level errors', errors: ['Feature unavailable for provided issue'] end - context 'with feature disabled' do - before do - stub_feature_flags(incident_escalations: false) - end - - it_behaves_like 'a mutation that returns top-level errors', errors: ['Feature unavailable for provided issue'] - end - it 'sets given escalation_policy to the escalation status for the issue' do post_graphql_mutation(mutation, current_user: current_user) diff --git a/spec/requests/api/graphql/mutations/jira_import/import_users_spec.rb b/spec/requests/api/graphql/mutations/jira_import/import_users_spec.rb index 45cc70f09fd..b438e1ba881 100644 --- a/spec/requests/api/graphql/mutations/jira_import/import_users_spec.rb +++ b/spec/requests/api/graphql/mutations/jira_import/import_users_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe 'Importing Jira Users' do - include JiraServiceHelper + include JiraIntegrationHelpers include GraphqlHelpers let_it_be(:user) { create(:user) } diff --git a/spec/requests/api/graphql/mutations/jira_import/start_spec.rb b/spec/requests/api/graphql/mutations/jira_import/start_spec.rb index b14305281af..1508ba31e37 100644 --- a/spec/requests/api/graphql/mutations/jira_import/start_spec.rb +++ b/spec/requests/api/graphql/mutations/jira_import/start_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe 'Starting a Jira Import' do - include JiraServiceHelper + include JiraIntegrationHelpers include GraphqlHelpers let_it_be(:user) { create(:user) } 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 5bc3c68cf26..9ef443af76a 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 @@ -76,7 +76,6 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do context 'when environment_id is missing' do let(:mutation) do variables = { - environment_id: nil, starting_at: starting_at, ending_at: ending_at, dashboard_path: dashboard_path, @@ -147,7 +146,6 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do context 'when cluster_id is missing' do let(:mutation) do variables = { - cluster_id: nil, starting_at: starting_at, ending_at: ending_at, dashboard_path: dashboard_path, diff --git a/spec/requests/api/graphql/mutations/notes/reposition_image_diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/reposition_image_diff_note_spec.rb index 0f7ccac3179..c4674155aa0 100644 --- a/spec/requests/api/graphql/mutations/notes/reposition_image_diff_note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/reposition_image_diff_note_spec.rb @@ -68,15 +68,7 @@ RSpec.describe 'Repositioning an ImageDiffNote' do let(:new_position) { { x: nil } } it_behaves_like 'a mutation that returns top-level errors' do - let(:match_errors) { include(/RepositionImageDiffNoteInput! was provided invalid value/) } - end - - it 'contains an explanation for the error' do - post_graphql_mutation(mutation, current_user: current_user) - - explanation = graphql_errors.first['extensions']['problems'].first['explanation'] - - expect(explanation).to eq('At least one property of `UpdateDiffImagePositionInput` must be set') + let(:match_errors) { include(/At least one property of `UpdateDiffImagePositionInput` must be set/) } end end end diff --git a/spec/requests/api/graphql/mutations/packages/cleanup/policy/update_spec.rb b/spec/requests/api/graphql/mutations/packages/cleanup/policy/update_spec.rb new file mode 100644 index 00000000000..7e00f3ca53a --- /dev/null +++ b/spec/requests/api/graphql/mutations/packages/cleanup/policy/update_spec.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Updating the packages cleanup policy' do + include GraphqlHelpers + using RSpec::Parameterized::TableSyntax + + let_it_be(:project, reload: true) { create(:project) } + let_it_be(:user) { create(:user) } + + let(:params) do + { + project_path: project.full_path, + keep_n_duplicated_package_files: 'TWENTY_PACKAGE_FILES' + } + end + + let(:mutation) do + graphql_mutation(:update_packages_cleanup_policy, params, + <<~QUERY + packagesCleanupPolicy { + keepNDuplicatedPackageFiles + nextRunAt + } + errors + QUERY + ) + end + + let(:mutation_response) { graphql_mutation_response(:update_packages_cleanup_policy) } + let(:packages_cleanup_policy_response) { mutation_response['packagesCleanupPolicy'] } + + shared_examples 'accepting the mutation request and updates the existing policy' do + it 'returns the updated packages cleanup policy' do + expect { subject }.not_to change { ::Packages::Cleanup::Policy.count } + + expect(project.packages_cleanup_policy.keep_n_duplicated_package_files).to eq('20') + expect_graphql_errors_to_be_empty + expect(packages_cleanup_policy_response['keepNDuplicatedPackageFiles']) + .to eq(params[:keep_n_duplicated_package_files]) + expect(packages_cleanup_policy_response['nextRunAt']).not_to eq(nil) + end + end + + shared_examples 'accepting the mutation request and creates a policy' do + it 'returns the created packages cleanup policy' do + expect { subject }.to change { ::Packages::Cleanup::Policy.count }.by(1) + + expect(project.packages_cleanup_policy.keep_n_duplicated_package_files).to eq('20') + expect_graphql_errors_to_be_empty + expect(packages_cleanup_policy_response['keepNDuplicatedPackageFiles']) + .to eq(params[:keep_n_duplicated_package_files]) + expect(packages_cleanup_policy_response['nextRunAt']).not_to eq(nil) + end + end + + shared_examples 'denying the mutation request' do + it 'returns an error' do + expect { subject }.not_to change { ::Packages::Cleanup::Policy.count } + + expect(project.packages_cleanup_policy.keep_n_duplicated_package_files).not_to eq('20') + expect(mutation_response).to be_nil + expect_graphql_errors_to_include(/you don't have permission to perform this action/) + end + end + + describe 'post graphql mutation' do + subject { post_graphql_mutation(mutation, current_user: user) } + + context 'with existing packages cleanup policy' do + let_it_be(:project_packages_cleanup_policy) { create(:packages_cleanup_policy, project: project) } + + where(:user_role, :shared_examples_name) do + :maintainer | 'accepting the mutation request and updates the existing policy' + :developer | 'denying the mutation request' + :reporter | 'denying the mutation request' + :guest | 'denying the mutation request' + :anonymous | 'denying the mutation request' + end + + with_them do + before do + project.send("add_#{user_role}", user) unless user_role == :anonymous + end + + it_behaves_like params[:shared_examples_name] + end + end + + context 'without existing packages cleanup policy' do + where(:user_role, :shared_examples_name) do + :maintainer | 'accepting the mutation request and creates a policy' + :developer | 'denying the mutation request' + :reporter | 'denying the mutation request' + :guest | 'denying the mutation request' + :anonymous | 'denying the mutation request' + end + + with_them do + before do + project.send("add_#{user_role}", user) unless user_role == :anonymous + end + + it_behaves_like params[:shared_examples_name] + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/packages/destroy_files_spec.rb b/spec/requests/api/graphql/mutations/packages/destroy_files_spec.rb new file mode 100644 index 00000000000..002cd634ebd --- /dev/null +++ b/spec/requests/api/graphql/mutations/packages/destroy_files_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Destroying multiple package files' do + using RSpec::Parameterized::TableSyntax + + include GraphqlHelpers + + let_it_be_with_reload(:package) { create(:maven_package) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { package.project } + + let(:ids) { package.package_files.first(2).map { |pf| pf.to_global_id.to_s } } + + let(:query) do + <<~GQL + errors + GQL + end + + let(:params) do + { + project_path: project.full_path, + ids: ids + } + end + + let(:mutation) { graphql_mutation(:destroy_package_files, params, query) } + + describe 'post graphql mutation' do + subject(:mutation_request) { post_graphql_mutation(mutation, current_user: user) } + + shared_examples 'destroying the package files' do + it 'marks the package file as pending destruction' do + expect { mutation_request }.to change { ::Packages::PackageFile.pending_destruction.count }.by(2) + end + + it_behaves_like 'returning response status', :success + end + + shared_examples 'denying the mutation request' do |response = "you don't have permission to perform this action"| + it 'does not mark the package file as pending destruction' do + expect { mutation_request }.not_to change { ::Packages::PackageFile.pending_destruction.count } + + expect_graphql_errors_to_include(response) + end + + it_behaves_like 'returning response status', :success + end + + context 'with valid params' do + where(:user_role, :shared_examples_name) do + :maintainer | 'destroying the package files' + :developer | 'denying the mutation request' + :reporter | 'denying the mutation request' + :guest | 'denying the mutation request' + :anonymous | 'denying the mutation request' + end + + with_them do + before do + project.send("add_#{user_role}", user) unless user_role == :anonymous + end + + it_behaves_like params[:shared_examples_name] + end + + context 'with more than 100 files' do + let(:ids) { package.package_files.map { |pf| pf.to_global_id.to_s } } + + before do + project.add_maintainer(user) + create_list(:package_file, 99, package: package) + end + + it_behaves_like 'denying the mutation request', 'Cannot delete more than 100 files' + end + + context 'with files outside of the project' do + let_it_be(:package2) { create(:maven_package) } + + let(:ids) { super().push(package2.package_files.first.to_global_id.to_s) } + + before do + project.add_maintainer(user) + end + + it_behaves_like 'denying the mutation request', 'All files must be in the requested project' + end + end + + context 'with invalid params' do + let(:params) { { id: 'foo' } } + + before do + project.add_maintainer(user) + end + + it_behaves_like 'denying the mutation request', 'invalid value for id' + end + end +end diff --git a/spec/requests/api/graphql/mutations/releases/create_spec.rb b/spec/requests/api/graphql/mutations/releases/create_spec.rb index 86995c10f10..1e62942c29d 100644 --- a/spec/requests/api/graphql/mutations/releases/create_spec.rb +++ b/spec/requests/api/graphql/mutations/releases/create_spec.rb @@ -17,6 +17,7 @@ RSpec.describe 'Creation of a new release' do let(:mutation_name) { :release_create } let(:tag_name) { 'v7.12.5'} + let(:tag_message) { nil } let(:ref) { 'master'} let(:name) { 'Version 7.12.5'} let(:description) { 'Release 7.12.5 :rocket:' } @@ -29,6 +30,7 @@ RSpec.describe 'Creation of a new release' do { projectPath: project.full_path, tagName: tag_name, + tagMessage: tag_message, ref: ref, name: name, description: description, @@ -191,10 +193,26 @@ RSpec.describe 'Creation of a new release' do context 'when the provided tag does not already exist' do let(:tag_name) { 'v7.12.5-alpha' } + after do + project.repository.rm_tag(developer, tag_name) + end + it_behaves_like 'no errors' - it 'creates a new tag' do + it 'creates a new lightweight tag' do expect { create_release }.to change { Project.find_by_id(project.id).repository.tag_count }.by(1) + expect(project.repository.find_tag(tag_name).message).to be_blank + end + + context 'and tag_message is provided' do + let(:tag_message) { 'Annotated tag message' } + + it_behaves_like 'no errors' + + it 'creates a new annotated tag with the message' do + expect { create_release }.to change { Project.find_by_id(project.id).repository.tag_count }.by(1) + expect(project.repository.find_tag(tag_name).message).to eq(tag_message) + end end end diff --git a/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb b/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb index 85194e6eb20..e1c7fd9d60d 100644 --- a/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb +++ b/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb @@ -28,17 +28,6 @@ RSpec.describe Mutations::UserPreferences::Update do expect(current_user.user_preference.persisted?).to eq(true) expect(current_user.user_preference.issues_sort).to eq(Types::IssueSortEnum.values[sort_value].value.to_s) end - - context 'when incident_escalations feature flag is disabled' do - let(:sort_value) { 'ESCALATION_STATUS_ASC' } - - before do - stub_feature_flags(incident_escalations: false) - end - - it_behaves_like 'a mutation that returns top-level errors', - errors: ['Feature flag `incident_escalations` must be enabled to use this sort order.'] - end end context 'when user has existing preference' do @@ -56,16 +45,5 @@ RSpec.describe Mutations::UserPreferences::Update do expect(current_user.user_preference.issues_sort).to eq(Types::IssueSortEnum.values[sort_value].value.to_s) end - - context 'when incident_escalations feature flag is disabled' do - let(:sort_value) { 'ESCALATION_STATUS_DESC' } - - before do - stub_feature_flags(incident_escalations: false) - end - - it_behaves_like 'a mutation that returns top-level errors', - errors: ['Feature flag `incident_escalations` must be enabled to use this sort order.'] - end end end diff --git a/spec/requests/api/graphql/mutations/work_items/update_task_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_task_spec.rb new file mode 100644 index 00000000000..32468a46ace --- /dev/null +++ b/spec/requests/api/graphql/mutations/work_items/update_task_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Update a work item task' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:developer) { create(:user).tap { |user| project.add_developer(user) } } + let_it_be(:unauthorized_work_item) { create(:work_item) } + let_it_be(:referenced_work_item, refind: true) { create(:work_item, project: project, title: 'REFERENCED') } + let_it_be(:parent_work_item) do + create(:work_item, project: project, description: "- [ ] #{referenced_work_item.to_reference}+") + end + + let(:task) { referenced_work_item } + let(:work_item) { parent_work_item } + let(:task_params) { { 'title' => 'UPDATED' } } + let(:task_input) { { 'id' => task.to_global_id.to_s }.merge(task_params) } + let(:input) { { 'id' => work_item.to_global_id.to_s, 'taskData' => task_input } } + let(:mutation) { graphql_mutation(:workItemUpdateTask, input) } + let(:mutation_response) { graphql_mutation_response(:work_item_update_task) } + + context 'the user is not allowed to read a work item' do + let(:current_user) { create(:user) } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permissions to update a work item' do + let(:current_user) { developer } + + it 'updates the work item and invalidates markdown cache on the original work item' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + referenced_work_item.reload + end.to change(referenced_work_item, :title).from(referenced_work_item.title).to('UPDATED') + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response).to include( + 'workItem' => hash_including( + 'title' => work_item.title, + 'descriptionHtml' => a_string_including('UPDATED') + ), + 'task' => hash_including( + 'title' => 'UPDATED' + ) + ) + end + + context 'when providing invalid task params' do + let(:task_params) { { 'title' => '' } } + + it 'makes no changes to the DB and returns an error message' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + task.reload + end.to not_change(task, :title).and( + not_change(work_item, :description_html) + ) + + expect(mutation_response['errors']).to contain_exactly("Title can't be blank") + end + end + + context 'when user cannot update the task' do + let(:task) { unauthorized_work_item } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + it_behaves_like 'has spam protection' do + let(:mutation_class) { ::Mutations::WorkItems::UpdateTask } + end + + context 'when the work_items feature flag is disabled' do + before do + stub_feature_flags(work_items: false) + end + + it 'does not update the task item and returns and error' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + task.reload + end.to not_change(task, :title) + + expect(mutation_response['errors']).to contain_exactly('`work_items` feature flag disabled for this project') + end + end + end + + context 'when user does not have permissions to update a work item' do + let(:current_user) { developer } + let(:work_item) { unauthorized_work_item } + + it_behaves_like 'a mutation that returns a top-level access error' + end +end diff --git a/spec/requests/api/graphql/mutations/work_items/update_widgets_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_widgets_spec.rb new file mode 100644 index 00000000000..595d8fe97ed --- /dev/null +++ b/spec/requests/api/graphql/mutations/work_items/update_widgets_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Update work item widgets' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:developer) { create(:user).tap { |user| project.add_developer(user) } } + let_it_be(:work_item, refind: true) { create(:work_item, project: project) } + + let(:input) do + { + 'descriptionWidget' => { 'description' => 'updated description' } + } + end + + let(:mutation) { graphql_mutation(:workItemUpdateWidgets, input.merge('id' => work_item.to_global_id.to_s)) } + + let(:mutation_response) { graphql_mutation_response(:work_item_update_widgets) } + + context 'the user is not allowed to update a work item' do + let(:current_user) { create(:user) } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permissions to update a work item', :aggregate_failures do + let(:current_user) { developer } + + context 'when the updated work item is not valid' do + it 'returns validation errors without the work item' do + errors = ActiveModel::Errors.new(work_item).tap { |e| e.add(:description, 'error message') } + + allow_next_found_instance_of(::WorkItem) do |instance| + allow(instance).to receive(:valid?).and_return(false) + allow(instance).to receive(:errors).and_return(errors) + end + + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['workItem']).to be_nil + expect(mutation_response['errors']).to match_array(['Description error message']) + end + end + + it 'updates the work item widgets' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to change(work_item, :description).from(nil).to('updated description') + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']).to include( + 'title' => work_item.title + ) + end + + it_behaves_like 'has spam protection' do + let(:mutation_class) { ::Mutations::WorkItems::UpdateWidgets } + end + + context 'when the work_items feature flag is disabled' do + before do + stub_feature_flags(work_items: false) + end + + it 'does not update the work item and returns and error' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to not_change(work_item, :title) + + expect(mutation_response['errors']).to contain_exactly('`work_items` feature flag disabled for this project') + end + end + end +end diff --git a/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb b/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb index 708fa96986c..31fef75f679 100644 --- a/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb +++ b/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb @@ -94,7 +94,7 @@ RSpec.describe 'getting incident timeline events' do 'id' => promoted_from_note.to_global_id.to_s, 'body' => promoted_from_note.note }, - 'editable' => false, + 'editable' => true, 'action' => timeline_event.action, 'occurredAt' => timeline_event.occurred_at.iso8601, 'createdAt' => timeline_event.created_at.iso8601, diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb index f358ec3e53f..69e14eace66 100644 --- a/spec/requests/api/graphql/project/issues_spec.rb +++ b/spec/requests/api/graphql/project/issues_spec.rb @@ -635,6 +635,18 @@ RSpec.describe 'getting an issue list for a project' do include_examples 'N+1 query check' end + context 'when requesting `closed_as_duplicate_of`' do + let(:requested_fields) { 'closedAsDuplicateOf { id }' } + let(:issue_a_dup) { create(:issue, project: project) } + let(:issue_b_dup) { create(:issue, project: project) } + + before do + issue_a.update!(duplicated_to_id: issue_a_dup) + issue_b.update!(duplicated_to_id: issue_a_dup) + end + + include_examples 'N+1 query check' + end end def issues_ids diff --git a/spec/requests/api/graphql/project/merge_request/pipelines_spec.rb b/spec/requests/api/graphql/project/merge_request/pipelines_spec.rb index 820a5d818c7..4dc272b5c2e 100644 --- a/spec/requests/api/graphql/project/merge_request/pipelines_spec.rb +++ b/spec/requests/api/graphql/project/merge_request/pipelines_spec.rb @@ -7,6 +7,7 @@ RSpec.describe 'Query.project.mergeRequests.pipelines' do let_it_be(:project) { create(:project, :public, :repository) } let_it_be(:author) { create(:user) } + let_it_be(:mr_nodes_path) { [:data, :project, :merge_requests, :nodes] } let_it_be(:merge_requests) do [ create(:merge_request, author: author, source_project: project), @@ -33,8 +34,49 @@ RSpec.describe 'Query.project.mergeRequests.pipelines' do GQL end - def run_query(first = nil) - post_graphql(query, current_user: author, variables: { path: project.full_path, first: first }) + before do + merge_requests.first(2).each do |mr| + shas = mr.recent_diff_head_shas + + shas.each do |sha| + create(:ci_pipeline, :success, project: project, ref: mr.source_branch, sha: sha) + end + end + end + + it 'produces correct results' do + r = run_query(3) + + nodes = graphql_dig_at(r, *mr_nodes_path) + + expect(nodes).to all(match('iid' => be_present, 'pipelines' => include('count' => be_a(Integer)))) + expect(graphql_dig_at(r, *mr_nodes_path, :pipelines, :count)).to contain_exactly(1, 1, 0) + end + + it 'is scalable', :request_store, :use_clean_rails_memory_store_caching do + baseline = ActiveRecord::QueryRecorder.new { run_query(1) } + + expect { run_query(2) }.not_to exceed_query_limit(baseline) + end + end + + describe '.nodes' do + let(:query) do + <<~GQL + query($path: ID!, $first: Int) { + project(fullPath: $path) { + mergeRequests(first: $first) { + nodes { + iid + pipelines { + count + nodes { id } + } + } + } + } + } + GQL end before do @@ -48,18 +90,27 @@ RSpec.describe 'Query.project.mergeRequests.pipelines' do end it 'produces correct results' do - run_query(2) - - p_nodes = graphql_data_at(:project, :merge_requests, :nodes) + r = run_query - expect(p_nodes).to all(match('iid' => be_present, 'pipelines' => match('count' => 1))) + expect(graphql_dig_at(r, *mr_nodes_path, :pipelines, :nodes, :id).uniq.size).to eq 3 end it 'is scalable', :request_store, :use_clean_rails_memory_store_caching do - # warm up - run_query + baseline = ActiveRecord::QueryRecorder.new { run_query(1) } - expect { run_query(2) }.to(issue_same_number_of_queries_as { run_query(1) }.ignoring_cached_queries) + expect { run_query(2) }.not_to exceed_query_limit(baseline) end + + it 'requests merge_request_diffs at most once' do + r = ActiveRecord::QueryRecorder.new { run_query(2) } + + expect(r.log.grep(/merge_request_diffs/)).to contain_exactly(a_string_including('SELECT')) + end + end + + def run_query(first = nil) + run_with_clean_state(query, + context: { current_user: author }, + variables: { path: project.full_path, first: first }) end end diff --git a/spec/requests/api/graphql/project/milestones_spec.rb b/spec/requests/api/graphql/project/milestones_spec.rb index 3e8948d83b1..d1ee157fc74 100644 --- a/spec/requests/api/graphql/project/milestones_spec.rb +++ b/spec/requests/api/graphql/project/milestones_spec.rb @@ -59,6 +59,27 @@ RSpec.describe 'getting milestone listings nested in a project' do end end + context 'the user does not have access' do + let_it_be(:project) { create(:project) } + let_it_be(:milestones) { create_list(:milestone, 2, project: project) } + + it 'is nil' do + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:project)).to be_nil + end + + context 'the user has access' do + let(:expected) { milestones } + + before do + project.add_guest(current_user) + end + + it_behaves_like 'searching with parameters' + end + end + context 'there are no search params' do let(:search_params) { nil } let(:expected) { all_milestones } diff --git a/spec/requests/api/graphql/project/packages_cleanup_policy_spec.rb b/spec/requests/api/graphql/project/packages_cleanup_policy_spec.rb new file mode 100644 index 00000000000..a025c57d4b8 --- /dev/null +++ b/spec/requests/api/graphql/project/packages_cleanup_policy_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe 'getting the packages cleanup policy linked to a project' do + using RSpec::Parameterized::TableSyntax + include GraphqlHelpers + + let_it_be_with_reload(:project) { create(:project) } + let_it_be(:current_user) { project.first_owner } + + let(:fields) do + <<~QUERY + #{all_graphql_fields_for('packages_cleanup_policy'.classify)} + QUERY + end + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('packagesCleanupPolicy', {}, fields) + ) + end + + subject { post_graphql(query, current_user: current_user) } + + it_behaves_like 'a working graphql query' do + before do + subject + end + end + + context 'with an existing policy' do + let_it_be(:policy) { create(:packages_cleanup_policy, project: project) } + + it_behaves_like 'a working graphql query' do + before do + subject + end + end + end + + context 'with different permissions' do + let_it_be(:current_user) { create(:user) } + + let(:packages_cleanup_policy_response) { graphql_data_at('project', 'packagesCleanupPolicy') } + + where(:visibility, :role, :policy_visible) do + :private | :maintainer | true + :private | :developer | false + :private | :reporter | false + :private | :guest | false + :private | :anonymous | false + :public | :maintainer | true + :public | :developer | false + :public | :reporter | false + :public | :guest | false + :public | :anonymous | false + end + + with_them do + before do + project.update!(visibility: visibility.to_s) + project.add_user(current_user, role) unless role == :anonymous + end + + it 'return the proper response' do + subject + + if policy_visible + expect(packages_cleanup_policy_response) + .to eq('keepNDuplicatedPackageFiles' => 'ALL_PACKAGE_FILES', 'nextRunAt' => nil) + else + expect(packages_cleanup_policy_response).to be_blank + end + end + end + end +end diff --git a/spec/requests/api/graphql/project/work_items_spec.rb b/spec/requests/api/graphql/project/work_items_spec.rb new file mode 100644 index 00000000000..66742fcbeb6 --- /dev/null +++ b/spec/requests/api/graphql/project/work_items_spec.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting an work item list for a project' do + include GraphqlHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :repository, :public, group: group) } + let_it_be(:current_user) { create(:user) } + + let_it_be(:item1) { create(:work_item, project: project, discussion_locked: true, title: 'item1') } + let_it_be(:item2) { create(:work_item, project: project, title: 'item2') } + let_it_be(:confidential_item) { create(:work_item, confidential: true, project: project, title: 'item3') } + let_it_be(:other_item) { create(:work_item) } + + let(:items_data) { graphql_data['project']['workItems']['edges'] } + let(:item_filter_params) { {} } + + let(:fields) do + <<~QUERY + edges { + node { + #{all_graphql_fields_for('workItems'.classify)} + } + } + QUERY + end + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('workItems', item_filter_params, 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 item' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'returns an empty list' do + post_graphql(query) + + expect(items_data).to eq([]) + end + end + + context 'when work_items flag is disabled' do + before do + stub_feature_flags(work_items: false) + end + + it 'returns an empty list' do + post_graphql(query) + + expect(items_data).to eq([]) + end + end + + it 'returns only items visible to user' do + post_graphql(query, current_user: current_user) + + expect(item_ids).to eq([item2.to_global_id.to_s, item1.to_global_id.to_s]) + end + + context 'when the user can see confidential items' do + before do + project.add_developer(current_user) + end + + it 'returns also confidential items' do + post_graphql(query, current_user: current_user) + + expect(item_ids).to eq([confidential_item.to_global_id.to_s, item2.to_global_id.to_s, item1.to_global_id.to_s]) + end + end + + describe 'sorting and pagination' do + let(:data_path) { [:project, :work_items] } + + def pagination_query(params) + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('workItems', params, "#{page_info} nodes { id }") + ) + end + + before do + project.add_developer(current_user) + end + + context 'when sorting by title ascending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { :TITLE_ASC } + let(:first_param) { 2 } + let(:all_records) { [item1, item2, confidential_item].map { |item| item.to_global_id.to_s } } + end + end + + context 'when sorting by title descending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { :TITLE_DESC } + let(:first_param) { 2 } + let(:all_records) { [confidential_item, item2, item1].map { |item| item.to_global_id.to_s } } + end + end + end + + def item_ids + graphql_dig_at(items_data, :node, :id) + end +end diff --git a/spec/requests/api/graphql/terraform/state/delete_spec.rb b/spec/requests/api/graphql/terraform/state/delete_spec.rb index 35927d03b49..ba0619ea611 100644 --- a/spec/requests/api/graphql/terraform/state/delete_spec.rb +++ b/spec/requests/api/graphql/terraform/state/delete_spec.rb @@ -12,12 +12,12 @@ RSpec.describe 'delete a terraform state' do let(:mutation) { graphql_mutation(:terraform_state_delete, id: state.to_global_id.to_s) } before do + expect_next_instance_of(Terraform::States::TriggerDestroyService, state, current_user: user) do |service| + expect(service).to receive(:execute).once.and_return(ServiceResponse.success) + end + post_graphql_mutation(mutation, current_user: user) end include_examples 'a working graphql query' - - it 'deletes the state' do - expect { state.reload }.to raise_error(ActiveRecord::RecordNotFound) - end end diff --git a/spec/requests/api/graphql/user/starred_projects_query_spec.rb b/spec/requests/api/graphql/user/starred_projects_query_spec.rb index 37a85b98e5f..75a17ed34c4 100644 --- a/spec/requests/api/graphql/user/starred_projects_query_spec.rb +++ b/spec/requests/api/graphql/user/starred_projects_query_spec.rb @@ -17,7 +17,6 @@ RSpec.describe 'Getting starredProjects of the user' do let_it_be(:user, reload: true) { create(:user) } let(:user_fields) { 'starredProjects { nodes { id } }' } - let(:current_user) { nil } let(:starred_projects) do post_graphql(query, current_user: current_user) @@ -34,21 +33,23 @@ RSpec.describe 'Getting starredProjects of the user' do user.toggle_star(project_c) end - it_behaves_like 'a working graphql query' do - before do - post_graphql(query) - end - end + context 'anonymous access' do + let(:current_user) { nil } - it 'found only public project' do - expect(starred_projects).to contain_exactly( - a_graphql_entity_for(project_a) - ) + it 'returns nothing' do + expect(starred_projects).to be_nil + end end context 'the current user is the user' do let(:current_user) { user } + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + it 'found all projects' do expect(starred_projects).to contain_exactly( a_graphql_entity_for(project_a), diff --git a/spec/requests/api/graphql/work_item_spec.rb b/spec/requests/api/graphql/work_item_spec.rb index 5b34c21989a..09bda8ee0d5 100644 --- a/spec/requests/api/graphql/work_item_spec.rb +++ b/spec/requests/api/graphql/work_item_spec.rb @@ -6,8 +6,13 @@ RSpec.describe 'Query.work_item(id)' do include GraphqlHelpers let_it_be(:developer) { create(:user) } - let_it_be(:project) { create(:project, :private).tap { |project| project.add_developer(developer) } } - let_it_be(:work_item) { create(:work_item, project: project) } + let_it_be(:guest) { create(:user) } + let_it_be(:project) { create(:project, :private) } + let_it_be(:work_item) { create(:work_item, project: project, description: '- List item') } + let_it_be(:child_item1) { create(:work_item, :task, project: project) } + let_it_be(:child_item2) { create(:work_item, :task, confidential: true, project: project) } + let_it_be(:child_link1) { create(:parent_link, work_item_parent: work_item, work_item: child_item1) } + let_it_be(:child_link2) { create(:parent_link, work_item_parent: work_item, work_item: child_item2) } let(:current_user) { developer } let(:work_item_data) { graphql_data['workItem'] } @@ -20,6 +25,9 @@ RSpec.describe 'Query.work_item(id)' do context 'when the user can read the work item' do before do + project.add_developer(developer) + project.add_guest(guest) + post_graphql(query, current_user: current_user) end @@ -38,6 +46,136 @@ RSpec.describe 'Query.work_item(id)' do ) end + context 'when querying widgets' do + describe 'description widget' do + let(:work_item_fields) do + <<~GRAPHQL + id + widgets { + type + ... on WorkItemWidgetDescription { + description + descriptionHtml + } + } + GRAPHQL + end + + it 'returns widget information' do + expect(work_item_data).to include( + 'id' => work_item.to_gid.to_s, + 'widgets' => match_array([ + hash_including( + 'type' => 'DESCRIPTION', + 'description' => work_item.description, + 'descriptionHtml' => ::MarkupHelper.markdown_field(work_item, :description, {}) + ), + hash_including( + 'type' => 'HIERARCHY' + ) + ]) + ) + end + end + + describe 'hierarchy widget' do + let(:work_item_fields) do + <<~GRAPHQL + id + widgets { + type + ... on WorkItemWidgetHierarchy { + parent { + id + } + children { + nodes { + id + } + } + } + } + GRAPHQL + end + + it 'returns widget information' do + expect(work_item_data).to include( + 'id' => work_item.to_gid.to_s, + 'widgets' => match_array([ + hash_including( + 'type' => 'DESCRIPTION' + ), + hash_including( + 'type' => 'HIERARCHY', + 'parent' => nil, + 'children' => { 'nodes' => match_array([ + hash_including('id' => child_link1.work_item.to_gid.to_s), + hash_including('id' => child_link2.work_item.to_gid.to_s) + ]) } + ) + ]) + ) + end + + it 'avoids N+1 queries' do + post_graphql(query, current_user: current_user) # warm up + + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + post_graphql(query, current_user: current_user) + end + + create_list(:parent_link, 3, work_item_parent: work_item) + + expect do + post_graphql(query, current_user: current_user) + end.not_to exceed_all_query_limit(control_count) + end + + context 'when user is guest' do + let(:current_user) { guest } + + it 'filters out not accessible children or parent' do + expect(work_item_data).to include( + 'id' => work_item.to_gid.to_s, + 'widgets' => match_array([ + hash_including( + 'type' => 'DESCRIPTION' + ), + hash_including( + 'type' => 'HIERARCHY', + 'parent' => nil, + 'children' => { 'nodes' => match_array([ + hash_including('id' => child_link1.work_item.to_gid.to_s) + ]) } + ) + ]) + ) + end + end + + context 'when requesting child item' do + let_it_be(:work_item) { create(:work_item, :task, project: project, description: '- List item') } + let_it_be(:parent_link) { create(:parent_link, work_item: work_item) } + + it 'returns parent information' do + expect(work_item_data).to include( + 'id' => work_item.to_gid.to_s, + 'widgets' => match_array([ + hash_including( + 'type' => 'DESCRIPTION' + ), + hash_including( + 'type' => 'HIERARCHY', + 'parent' => hash_including('id' => parent_link.work_item_parent.to_gid.to_s), + 'children' => { 'nodes' => match_array([]) } + ) + ]) + ) + end + end + end + end + context 'when an Issue Global ID is provided' do let(:global_id) { Issue.find(work_item.id).to_gid.to_s } diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb index 3bd59450d49..d94257c61eb 100644 --- a/spec/requests/api/graphql_spec.rb +++ b/spec/requests/api/graphql_spec.rb @@ -67,10 +67,10 @@ RSpec.describe 'GraphQL' do context 'when there is an error in the logger' do before do - logger_analyzer = GitlabSchema.query_analyzers.find do |qa| - qa.is_a? Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer - end - allow(logger_analyzer).to receive(:process_variables) + allow(GraphQL::Analysis::AST).to receive(:analyze_query) + .and_call_original + allow(GraphQL::Analysis::AST).to receive(:analyze_query) + .with(anything, Gitlab::Graphql::QueryAnalyzers::AST::LoggerAnalyzer::ALL_ANALYZERS, anything) .and_raise(StandardError.new("oh noes!")) end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index ffc5d353958..56f08249bdd 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -1177,7 +1177,6 @@ RSpec.describe API::Groups do it "only looks up root ancestor once and returns projects including those in subgroups" do expect(Namespace).to receive(:find_by).with(id: group1.id.to_s).once.and_call_original # For the group sent in the API call - expect(Namespace).to receive(:find_by).with(id: group1.traversal_ids.first).once.and_call_original # root_ancestor direct lookup expect(Namespace).to receive(:joins).with(start_with('INNER JOIN (SELECT id, traversal_ids[1]')).once.and_call_original # All-in-one root_ancestor query get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } @@ -1187,25 +1186,6 @@ RSpec.describe API::Groups do expect(json_response).to be_an(Array) expect(json_response.length).to eq(6) end - - context 'when group_projects_api_preload_groups feature is disabled' do - before do - stub_feature_flags(group_projects_api_preload_groups: false) - end - - it 'looks up the root ancestor multiple times' do - expect(Namespace).to receive(:find_by).with(id: group1.id.to_s).once.and_call_original - expect(Namespace).to receive(:find_by).with(id: group1.traversal_ids.first).at_least(:twice).and_call_original - expect(Namespace).not_to receive(:joins).with(start_with('INNER JOIN (SELECT id, traversal_ids[1]')) - - get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an(Array) - expect(json_response.length).to eq(6) - end - end end context 'when include_ancestor_groups is true' do diff --git a/spec/requests/api/integrations/jira_connect/subscriptions_spec.rb b/spec/requests/api/integrations/jira_connect/subscriptions_spec.rb index 86f8992a624..8a222a99b34 100644 --- a/spec/requests/api/integrations/jira_connect/subscriptions_spec.rb +++ b/spec/requests/api/integrations/jira_connect/subscriptions_spec.rb @@ -41,6 +41,7 @@ RSpec.describe API::Integrations::JiraConnect::Subscriptions do post_subscriptions expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response).to eq('message' => '401 Unauthorized - JWT authentication failed') end end diff --git a/spec/requests/api/integrations/slack/events_spec.rb b/spec/requests/api/integrations/slack/events_spec.rb new file mode 100644 index 00000000000..176e9eded31 --- /dev/null +++ b/spec/requests/api/integrations/slack/events_spec.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Integrations::Slack::Events do + describe 'POST /integrations/slack/events' do + let(:params) { {} } + let(:headers) do + { + ::API::Integrations::Slack::Request::VERIFICATION_TIMESTAMP_HEADER => Time.current.to_i.to_s, + ::API::Integrations::Slack::Request::VERIFICATION_SIGNATURE_HEADER => 'mock_verified_signature' + } + end + + before do + allow(ActiveSupport::SecurityUtils).to receive(:secure_compare) do |signature| + signature == 'mock_verified_signature' + end + + stub_application_setting(slack_app_signing_secret: 'mock_key') + end + + subject { post api('/integrations/slack/events'), params: params, headers: headers } + + shared_examples 'an unauthorized request' do + specify do + subject + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + shared_examples 'a successful request that generates a tracked error' do + specify do + expect(Gitlab::ErrorTracking).to receive(:track_exception).once + + subject + + expect(response).to have_gitlab_http_status(:no_content) + expect(response.body).to be_empty + end + end + + context 'when the slack_app_signing_secret setting is not set' do + before do + stub_application_setting(slack_app_signing_secret: nil) + end + + it_behaves_like 'an unauthorized request' + end + + context 'when the timestamp header has expired' do + before do + headers[::API::Integrations::Slack::Request::VERIFICATION_TIMESTAMP_HEADER] = 5.minutes.ago.to_i.to_s + end + + it_behaves_like 'an unauthorized request' + end + + context 'when the timestamp header is missing' do + before do + headers.delete(::API::Integrations::Slack::Request::VERIFICATION_TIMESTAMP_HEADER) + end + + it_behaves_like 'an unauthorized request' + end + + context 'when the signature header is missing' do + before do + headers.delete(::API::Integrations::Slack::Request::VERIFICATION_SIGNATURE_HEADER) + end + + it_behaves_like 'an unauthorized request' + end + + context 'when the signature is not verified' do + before do + headers[::API::Integrations::Slack::Request::VERIFICATION_SIGNATURE_HEADER] = 'unverified_signature' + end + + it_behaves_like 'an unauthorized request' + end + + context 'when type param is missing' do + it_behaves_like 'a successful request that generates a tracked error' + end + + context 'when type param is unknown' do + let(:params) do + { type: 'unknown_type' } + end + + it_behaves_like 'a successful request that generates a tracked error' + end + + context 'when type param is url_verification' do + let(:params) do + { + type: 'url_verification', + challenge: '3eZbrw1aBm2rZgRNFdxV2595E9CY3gmdALWMmHkvFXO7tYXAYM8P' + } + end + + it 'responds in-request with the challenge' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'challenge' => '3eZbrw1aBm2rZgRNFdxV2595E9CY3gmdALWMmHkvFXO7tYXAYM8P' }) + end + end + end +end diff --git a/spec/requests/api/integrations_spec.rb b/spec/requests/api/integrations_spec.rb index 96cc101e73a..cd9a0746581 100644 --- a/spec/requests/api/integrations_spec.rb +++ b/spec/requests/api/integrations_spec.rb @@ -55,33 +55,49 @@ RSpec.describe API::Integrations do describe "PUT /projects/:id/#{endpoint}/#{integration.dasherize}" do include_context integration + # NOTE: Some attributes are not supported for PUT requests, even though in most cases they should be. + # For some of them the problem is somewhere else, i.e. most chat integrations don't support the `*_channel` + # fields but they're incorrectly included in `#fields`. + # + # We can fix these manually, or with a generic approach like https://gitlab.com/gitlab-org/gitlab/-/issues/348208 + let(:missing_channel_attributes) { %i[push_channel issue_channel confidential_issue_channel merge_request_channel note_channel confidential_note_channel tag_push_channel pipeline_channel wiki_page_channel] } + let(:missing_attributes) do + { + datadog: %i[archive_trace_events], + discord: missing_channel_attributes + %i[branches_to_be_notified notify_only_broken_pipelines], + hangouts_chat: missing_channel_attributes + %i[notify_only_broken_pipelines], + jira: %i[issues_enabled project_key vulnerabilities_enabled vulnerabilities_issuetype], + mattermost: %i[deployment_channel labels_to_be_notified], + microsoft_teams: missing_channel_attributes, + mock_ci: %i[enable_ssl_verification], + prometheus: %i[manual_configuration], + slack: %i[alert_events alert_channel deployment_channel labels_to_be_notified], + unify_circuit: missing_channel_attributes + %i[branches_to_be_notified notify_only_broken_pipelines], + webex_teams: missing_channel_attributes + %i[branches_to_be_notified notify_only_broken_pipelines] + } + end + it "updates #{integration} settings and returns the correct fields" do - put api("/projects/#{project.id}/#{endpoint}/#{dashed_integration}", user), params: integration_attrs + supported_attrs = integration_attrs.without(missing_attributes.fetch(integration.to_sym, [])) + + put api("/projects/#{project.id}/#{endpoint}/#{dashed_integration}", user), params: supported_attrs expect(response).to have_gitlab_http_status(:ok) + expect(json_response['slug']).to eq(dashed_integration) current_integration = project.integrations.first - events = current_integration.event_names.empty? ? ["foo"].freeze : current_integration.event_names - query_strings = [] - events.map(&:to_sym).each do |event| - event_value = !current_integration[event] - query_strings << "#{event}=#{event_value}" - integration_attrs[event] = event_value if integration_attrs[event].present? + expect(current_integration).to have_attributes(supported_attrs) + assert_correct_response_fields(json_response['properties'].keys, current_integration) + + # Flip all booleans and verify that we can set these too + flipped_attrs = supported_attrs.transform_values do |value| + [true, false].include?(value) ? !value : value end - query_strings = query_strings.join('&') - put api("/projects/#{project.id}/#{endpoint}/#{dashed_integration}?#{query_strings}", user), params: integration_attrs + put api("/projects/#{project.id}/#{endpoint}/#{dashed_integration}", user), params: flipped_attrs expect(response).to have_gitlab_http_status(:ok) - expect(json_response['slug']).to eq(dashed_integration) - events.each do |event| - next if event == "foo" - - expect(project.integrations.first[event]).not_to eq(current_integration[event]), - "expected #{!current_integration[event]} for event #{event} for #{endpoint} #{current_integration.title}, got #{current_integration[event]}" - end - - assert_correct_response_fields(json_response['properties'].keys, current_integration) + expect(project.integrations.first).to have_attributes(flipped_attrs) end it "returns if required fields missing" do diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index acfe476a864..93e4e72f78f 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -51,6 +51,64 @@ RSpec.describe API::Internal::Base do end end + describe 'GET /internal/error_tracking_allowed' do + let_it_be(:project) { create(:project) } + + let(:params) { { project_id: project.id, public_key: 'key' } } + + context 'when the secret header is missing' do + it 'responds with unauthorized entity' do + post api("/internal/error_tracking_allowed"), params: params + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when some params are missing' do + it 'responds with unprocessable entity' do + post api("/internal/error_tracking_allowed"), params: params.except(:public_key), + headers: { API::Helpers::GITLAB_SHARED_SECRET_HEADER => Base64.encode64(secret_token) } + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + end + + context 'when the error tracking is disabled' do + it 'returns enabled: false' do + create(:error_tracking_client_key, project: project, active: false) + + post api("/internal/error_tracking_allowed"), params: params, + headers: { API::Helpers::GITLAB_SHARED_SECRET_HEADER => Base64.encode64(secret_token) } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'enabled' => false }) + end + + context 'when the error tracking record does not exist' do + it 'returns enabled: false' do + post api("/internal/error_tracking_allowed"), params: params, + headers: { API::Helpers::GITLAB_SHARED_SECRET_HEADER => Base64.encode64(secret_token) } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'enabled' => false }) + end + end + end + + context 'when the error tracking is enabled' do + it 'returns enabled: true' do + client_key = create(:error_tracking_client_key, project: project, active: true) + params[:public_key] = client_key.public_key + + post api("/internal/error_tracking_allowed"), params: params, + headers: { API::Helpers::GITLAB_SHARED_SECRET_HEADER => Base64.encode64(secret_token) } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'enabled' => true }) + end + end + end + describe 'GET /internal/two_factor_recovery_codes' do let(:key_id) { key.id } diff --git a/spec/requests/api/internal/mail_room_spec.rb b/spec/requests/api/internal/mail_room_spec.rb index 67ea617f90d..a0a9c1f9cb3 100644 --- a/spec/requests/api/internal/mail_room_spec.rb +++ b/spec/requests/api/internal/mail_room_spec.rb @@ -33,7 +33,7 @@ RSpec.describe API::Internal::MailRoom do let(:incoming_email_secret) { 'incoming_email_secret' } let(:service_desk_email_secret) { 'service_desk_email_secret' } - let(:email_content) { fixture_file("emails/commands_in_reply.eml") } + let(:email_content) { fixture_file("emails/service_desk_reply.eml") } before do allow(Gitlab::MailRoom::Authenticator).to receive(:secret).with(:incoming_email).and_return(incoming_email_secret) @@ -117,7 +117,7 @@ RSpec.describe API::Internal::MailRoom do email = ActionMailer::Base.deliveries.last expect(email).not_to be_nil - expect(email.to).to match_array(["jake@adventuretime.ooo"]) + expect(email.to).to match_array(["alan@adventuretime.ooo"]) expect(email.subject).to include("Rejected") expect(email.body.parts.last.to_s).to include("We couldn't process your email") end @@ -190,5 +190,54 @@ RSpec.describe API::Internal::MailRoom do expect(response).to have_gitlab_http_status(:unauthorized) end end + + context 'handle invalid utf-8 email content' do + let(:email_content) do + File.open(expand_fixture_path("emails/service_desk_reply_illegal_utf8.eml"), "r:SHIFT_JIS") { |f| f.read } + end + + let(:encoded_email_content) { Gitlab::EncodingHelper.encode_utf8(email_content) } + let(:auth_headers) do + jwt_token = JWT.encode(auth_payload, incoming_email_secret, 'HS256') + { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => jwt_token } + end + + it 'schedules a EmailReceiverWorker job with email content encoded to utf-8 forcefully' do + Sidekiq::Testing.fake! do + expect do + post api("/internal/mail_room/incoming_email"), headers: auth_headers, params: email_content + end.to change { EmailReceiverWorker.jobs.size }.by(1) + end + + expect(response).to have_gitlab_http_status(:ok) + + job = EmailReceiverWorker.jobs.last + expect(job).to match a_hash_including('args' => [encoded_email_content]) + end + end + + context 'handle text/plain request content type' do + let(:auth_headers) do + jwt_token = JWT.encode(auth_payload, incoming_email_secret, 'HS256') + { + Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => jwt_token, + 'Content-Type' => 'text/plain' + } + end + + it 'schedules a EmailReceiverWorker job with email content encoded to utf-8 forcefully' do + Sidekiq::Testing.fake! do + expect do + post api("/internal/mail_room/incoming_email"), headers: auth_headers, params: email_content + end.to change { EmailReceiverWorker.jobs.size }.by(1) + end + + expect(response).to have_gitlab_http_status(:ok) + expect(response.content_type).to eql('application/json') + + job = EmailReceiverWorker.jobs.last + expect(job).to match a_hash_including('args' => [email_content]) + end + end end end diff --git a/spec/requests/api/internal/workhorse_spec.rb b/spec/requests/api/internal/workhorse_spec.rb new file mode 100644 index 00000000000..d40c14cc0fd --- /dev/null +++ b/spec/requests/api/internal/workhorse_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Internal::Workhorse, :allow_forgery_protection do + include WorkhorseHelpers + + context '/authorize_upload' do + let_it_be(:user) { create(:user) } + + let(:headers) { {} } + + subject { post(api('/internal/workhorse/authorize_upload'), headers: headers) } + + def expect_status(status) + subject + expect(response).to have_gitlab_http_status(status) + end + + context 'without workhorse internal header' do + it { expect_status(:forbidden) } + end + + context 'with workhorse internal header' do + let(:headers) { workhorse_internal_api_request_header } + + it { expect_status(:unauthorized) } + + context 'as a logged in user' do + before do + login_as(user) + end + + it { expect_status(:success) } + it 'returns the temp upload path' do + subject + expect(json_response['TempPath']).to eq(Rails.root.join('tmp/tests/public/uploads/tmp').to_s) + end + end + end + end +end diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb index d093894720e..64ad5733c1b 100644 --- a/spec/requests/api/invitations_spec.rb +++ b/spec/requests/api/invitations_spec.rb @@ -59,13 +59,13 @@ RSpec.describe API::Invitations do context 'when authenticated as a maintainer/owner' do context 'and new member is already a requester' do - it 'does not transform the requester into a proper member' do + it 'transforms the requester into a proper member' do expect do post invitations_url(source, maintainer), params: { email: access_requester.email, access_level: Member::MAINTAINER } expect(response).to have_gitlab_http_status(:created) - end.not_to change { source.members.count } + end.to change { source.members.count }.by(1) end end @@ -258,12 +258,13 @@ RSpec.describe API::Invitations do end end - it "returns a message if member already exists" do + it "updates an already existing active member" do post invitations_url(source, maintainer), params: { email: developer.email, access_level: Member::MAINTAINER } expect(response).to have_gitlab_http_status(:created) - expect(json_response['message'][developer.email]).to eq("User already exists in source") + expect(json_response['status']).to eq("success") + expect(source.members.find_by(user: developer).access_level).to eq Member::MAINTAINER end it 'returns 400 when the invite params of email and user_id are not sent' do @@ -328,7 +329,7 @@ RSpec.describe API::Invitations do emails = 'email3@example.com,email4@example.com,email5@example.com,email6@example.com,email7@example.com' - unresolved_n_plus_ones = 44 # old 48 with 12 per new email, currently there are 11 queries added per email + unresolved_n_plus_ones = 40 # currently there are 10 queries added per email expect do post invitations_url(project, maintainer), params: { email: emails, access_level: Member::DEVELOPER } @@ -351,7 +352,7 @@ RSpec.describe API::Invitations do emails = 'email3@example.com,email4@example.com,email5@example.com,email6@example.com,email7@example.com' - unresolved_n_plus_ones = 67 # currently there are 11 queries added per email + unresolved_n_plus_ones = 59 # currently there are 10 queries added per email expect do post invitations_url(project, maintainer), params: { email: emails, access_level: Member::DEVELOPER } @@ -373,7 +374,7 @@ RSpec.describe API::Invitations do emails = 'email3@example.com,email4@example.com,email5@example.com,email6@example.com,email7@example.com' - unresolved_n_plus_ones = 36 # old 40 with 10 per new email, currently there are 9 queries added per email + unresolved_n_plus_ones = 32 # currently there are 8 queries added per email expect do post invitations_url(group, maintainer), params: { email: emails, access_level: Member::DEVELOPER } @@ -396,7 +397,7 @@ RSpec.describe API::Invitations do emails = 'email3@example.com,email4@example.com,email5@example.com,email6@example.com,email7@example.com' - unresolved_n_plus_ones = 62 # currently there are 9 queries added per email + unresolved_n_plus_ones = 56 # currently there are 8 queries added per email expect do post invitations_url(group, maintainer), params: { email: emails, access_level: Member::DEVELOPER } diff --git a/spec/requests/api/issue_links_spec.rb b/spec/requests/api/issue_links_spec.rb index 81dd4c3dfa0..90238c8bf76 100644 --- a/spec/requests/api/issue_links_spec.rb +++ b/spec/requests/api/issue_links_spec.rb @@ -156,6 +156,87 @@ RSpec.describe API::IssueLinks do end end + describe 'GET /links/:issue_link_id' do + def perform_request(issue_link_id, user = nil, params = {}) + get api("/projects/#{project.id}/issues/#{issue.iid}/links/#{issue_link_id}", user), params: params + end + + context 'when unauthenticated' do + it 'returns 401' do + issue_link = create(:issue_link) + + perform_request(issue_link.id) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when authenticated' do + context 'when issue link does not exist' do + it 'returns 404' do + perform_request(non_existing_record_id, user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + let_it_be(:target_issue) { create(:issue, project: project) } + + context 'when issue link does not belong to the specified issue' do + it 'returns 404' do + other_issue = create(:issue, project: project) + # source is different than the given API route issue + issue_link = create(:issue_link, source: other_issue, target: target_issue) + + perform_request(issue_link.id, user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user has ability to read the issue link' do + it 'returns 200' do + issue_link = create(:issue_link, source: issue, target: target_issue) + + perform_request(issue_link.id, user) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/issue_link') + end + end + end + + context 'when user cannot read issue link' do + let(:private_project) { create(:project) } + let(:public_project) { create(:project, :public) } + let(:public_issue) { create(:issue, project: public_project) } + + context 'when the issue link targets an issue in a non-accessible project' do + it 'returns 404' do + private_issue = create(:issue, project: private_project) + issue_link = create(:issue_link, source: public_issue, target: private_issue) + + perform_request(issue_link.id, user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when issue link targets a non-accessible issue' do + it 'returns 404' do + confidential_issue = create(:issue, :confidential, project: public_project) + issue_link = create(:issue_link, source: public_issue, target: confidential_issue) + + perform_request(issue_link.id, user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + end + describe 'DELETE /links/:issue_link_id' do context 'when unauthenticated' do it 'returns 401' do diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index 1419d39981a..480baff6eed 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -575,6 +575,26 @@ RSpec.describe API::Issues do end end + context 'with issues closed as duplicates' do + let_it_be(:dup_issue_1) { create(:issue, :closed_as_duplicate, project: project) } + + it 'avoids N+1 queries' do + get api('/issues', user) # warm up + + control = ActiveRecord::QueryRecorder.new do + get api('/issues', user) + end + + create(:issue, :closed_as_duplicate, project: project) + + expect do + get api('/issues', user) + end.not_to exceed_query_limit(control) + # 2 pre-existed issues + 2 duplicated incidents (2 closed, 2 new) + expect(json_response.count).to eq(6) + end + end + context 'filter by labels or label_name param' do context 'N+1' do let(:label_b) { create(:label, title: 'foo', project: project) } @@ -1101,6 +1121,51 @@ RSpec.describe API::Issues do expect(json_response['references']['relative']).to eq("##{issue.iid}") expect(json_response['references']['full']).to eq("#{project.parent.path}/#{project.path}##{issue.iid}") end + + context 'when issue is closed as duplicate' do + let(:new_issue) { create(:issue) } + let!(:issue_closed_as_dup) { create(:issue, project: project, duplicated_to: new_issue) } + + before do + project.add_developer(user) + end + + context 'user does not have permission to view new issue' do + it 'does not return the issue as closed_as_duplicate_of' do + get api("/projects/#{project.id}/issues/#{issue_closed_as_dup.iid}", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.dig('_links', 'closed_as_duplicate_of')).to eq(nil) + end + end + + context 'when user has access to new issue' do + before do + new_issue.project.add_guest(user) + end + + it 'returns the issue as closed_as_duplicate_of' do + get api("/projects/#{project.id}/issues/#{issue_closed_as_dup.iid}", user) + + expect(response).to have_gitlab_http_status(:ok) + expected_url = expose_url(api_v4_project_issue_path(id: new_issue.project_id, issue_iid: new_issue.iid)) + expect(json_response.dig('_links', 'closed_as_duplicate_of')).to eq(expected_url) + end + + context 'feature flag is disabled' do + before do + stub_feature_flags(closed_as_duplicate_of_issues_api: false) + end + + it 'does not return the issue as closed_as_duplicate_of' do + get api("/projects/#{project.id}/issues/#{issue_closed_as_dup.iid}", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.dig('_links', 'closed_as_duplicate_of')).to eq(nil) + end + end + end + end end describe "POST /projects/:id/issues" do diff --git a/spec/requests/api/markdown_snapshot_spec.rb b/spec/requests/api/markdown_snapshot_spec.rb new file mode 100644 index 00000000000..37607a4e866 --- /dev/null +++ b/spec/requests/api/markdown_snapshot_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# See https://docs.gitlab.com/ee/development/gitlab_flavored_markdown/specification_guide/#markdown-snapshot-testing +# for documentation on this spec. +RSpec.describe API::Markdown, 'Snapshot' do + glfm_specification_dir = File.expand_path('../../../glfm_specification', __dir__) + glfm_example_snapshots_dir = File.expand_path('../../fixtures/glfm/example_snapshots', __dir__) + include_context 'with API::Markdown Snapshot shared context', glfm_specification_dir, glfm_example_snapshots_dir +end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 63ef8643088..e4c2f17af47 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -4,13 +4,14 @@ require 'spec_helper' RSpec.describe API::Members do let(:maintainer) { create(:user, username: 'maintainer_user') } + let(:maintainer2) { create(:user, username: 'user-with-maintainer-role') } let(:developer) { create(:user) } let(:access_requester) { create(:user) } let(:stranger) { create(:user) } let(:user_with_minimal_access) { create(:user) } let(:project) do - create(:project, :public, creator_id: maintainer.id, namespace: maintainer.namespace) do |project| + create(:project, :public, creator_id: maintainer.id, group: create(:group, :public)) do |project| project.add_maintainer(maintainer) project.add_developer(developer, current_user: maintainer) project.request_access(access_requester) @@ -253,21 +254,48 @@ RSpec.describe API::Members do expect(response).to have_gitlab_http_status(:forbidden) end end + + context 'adding a member of higher access level' do + before do + # the other 'maintainer' is in fact an owner of the group! + source.add_maintainer(maintainer2) + end + + context 'when an access requester' do + it 'is not successful' do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer2), + params: { user_id: access_requester.id, access_level: Member::OWNER } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when a totally new user' do + it 'is not successful' do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer2), + params: { user_id: stranger.id, access_level: Member::OWNER } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end end end - context 'when authenticated as a maintainer/owner' do + context 'when authenticated as a member with membership management rights' do context 'and new member is already a requester' do - it 'transforms the requester into a proper member' do - expect do - post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), - params: { user_id: access_requester.id, access_level: Member::MAINTAINER } - - expect(response).to have_gitlab_http_status(:created) - end.to change { source.members.count }.by(1) - expect(source.requesters.count).to eq(0) - expect(json_response['id']).to eq(access_requester.id) - expect(json_response['access_level']).to eq(Member::MAINTAINER) + context 'when the requester is of equal or lower access level' do + it 'transforms the requester into a proper member' do + expect do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), + params: { user_id: access_requester.id, access_level: Member::MAINTAINER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.count }.by(1) + expect(source.requesters.count).to eq(0) + expect(json_response['id']).to eq(access_requester.id) + expect(json_response['access_level']).to eq(Member::MAINTAINER) + end end end @@ -445,7 +473,7 @@ RSpec.describe API::Members do it 'returns 404 when the user_id is not valid' do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), - params: { user_id: 0, access_level: Member::MAINTAINER } + params: { user_id: non_existing_record_id, access_level: Member::MAINTAINER } expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') @@ -515,16 +543,49 @@ RSpec.describe API::Members do end end end + + context 'as a maintainer updating a member to one with higher access level than themselves' do + before do + # the other 'maintainer' is in fact an owner of the group! + source.add_maintainer(maintainer2) + end + + it 'returns 403' do + put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer2), + params: { access_level: Member::OWNER } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end end context 'when authenticated as a maintainer/owner' do - it 'updates the member' do - put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer), - params: { access_level: Member::MAINTAINER } + context 'when updating a member with the same or lower access level' do + it 'updates the member' do + put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer), + params: { access_level: Member::MAINTAINER } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['id']).to eq(developer.id) - expect(json_response['access_level']).to eq(Member::MAINTAINER) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['id']).to eq(developer.id) + expect(json_response['access_level']).to eq(Member::MAINTAINER) + end + end + + context 'when updating a member with higher access level' do + let(:owner) { create(:user) } + + before do + source.add_owner(owner) + # the other 'maintainer' is in fact an owner of the group! + source.add_maintainer(maintainer2) + end + + it 'returns 403' do + put api("/#{source_type.pluralize}/#{source.id}/members/#{owner.id}", maintainer2), + params: { access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:forbidden) + end end end @@ -619,6 +680,23 @@ RSpec.describe API::Members do end end + context 'when attempting to delete a member with higher access level' do + let(:owner) { create(:user) } + + before do + source.add_owner(owner) + # the other 'maintainer' is in fact an owner of the group! + source.add_maintainer(maintainer2) + end + + it 'returns 403' do + delete api("/#{source_type.pluralize}/#{source.id}/members/#{owner.id}", maintainer2), + params: { access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + it 'deletes the member' do expect do delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer) @@ -694,13 +772,11 @@ RSpec.describe API::Members do end context 'adding owner to project' do - it 'returns created status' do - expect do - post api("/projects/#{project.id}/members", maintainer), - params: { user_id: stranger.id, access_level: Member::OWNER } + it 'returns 403' do + post api("/projects/#{project.id}/members", maintainer), + params: { user_id: stranger.id, access_level: Member::OWNER } - expect(response).to have_gitlab_http_status(:created) - end.to change { project.members.count }.by(1) + expect(response).to have_gitlab_http_status(:forbidden) end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index a7ede7f4150..695c0ed1749 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -2134,54 +2134,6 @@ RSpec.describe API::MergeRequests do expect(response).to have_gitlab_http_status(:created) end end - - describe 'SSE counter' do - let(:headers) { {} } - let(:params) do - { - title: 'Test merge_request', - source_branch: 'feature_conflict', - target_branch: 'master', - author_id: user.id, - milestone_id: milestone.id, - squash: true - } - end - - subject { post api("/projects/#{project.id}/merge_requests", user), params: params, headers: headers } - - it 'does not increase the SSE counter by default' do - expect(Gitlab::UsageDataCounters::EditorUniqueCounter).not_to receive(:track_sse_edit_action) - - subject - - expect(response).to have_gitlab_http_status(:created) - end - - context 'when referer is not the SSE' do - let(:headers) { { 'HTTP_REFERER' => 'https://gitlab.com' } } - - it 'does not increase the SSE counter by default' do - expect(Gitlab::UsageDataCounters::EditorUniqueCounter).not_to receive(:track_sse_edit_action) - - subject - - expect(response).to have_gitlab_http_status(:created) - end - end - - context 'when referer is the SSE' do - let(:headers) { { 'HTTP_REFERER' => project_show_sse_url(project, 'master/README.md') } } - - it 'increases the SSE counter by default' do - expect(Gitlab::UsageDataCounters::EditorUniqueCounter).to receive(:track_sse_edit_action).with(author: user) - - subject - - expect(response).to have_gitlab_http_status(:created) - end - end - end end describe 'PUT /projects/:id/merge_requests/:merge_request_iid' do @@ -2535,14 +2487,34 @@ RSpec.describe API::MergeRequests do expect(response).to have_gitlab_http_status(:ok) end - it "returns 406 if branch can't be merged" do - allow_any_instance_of(MergeRequest) - .to receive(:can_be_merged?).and_return(false) + context 'when change_response_code_merge_status is enabled' do + it "returns 422 if branch can't be merged" do + allow_next_found_instance_of(MergeRequest) do |merge_request| + allow(merge_request).to receive(:can_be_merged?).and_return(false) + end - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Branch cannot be merged') + end + end + + context 'when change_response_code_merge_status is disabled' do + before do + stub_feature_flags(change_response_code_merge_status: false) + end - expect(response).to have_gitlab_http_status(:not_acceptable) - expect(json_response['message']).to eq('Branch cannot be merged') + it "returns 406 if branch can't be merged" do + allow_next_found_instance_of(MergeRequest) do |merge_request| + allow(merge_request).to receive(:can_be_merged?).and_return(false) + end + + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) + + expect(response).to have_gitlab_http_status(:not_acceptable) + expect(json_response['message']).to eq('Branch cannot be merged') + end end it "returns 405 if merge_request is not open" do @@ -2693,6 +2665,7 @@ RSpec.describe API::MergeRequests do expect(response).to have_gitlab_http_status(:ok) expect(source_repository.branch_exists?(source_branch)).to be false + expect(merge_request.reload.should_remove_source_branch?).to be true end end @@ -2711,6 +2684,7 @@ RSpec.describe API::MergeRequests do expect(response).to have_gitlab_http_status(:ok) expect(source_repository.branch_exists?(source_branch)).to be false + expect(merge_request.reload.should_remove_source_branch?).to be nil end it 'does not remove the source branch' do @@ -2721,6 +2695,7 @@ RSpec.describe API::MergeRequests do expect(response).to have_gitlab_http_status(:ok) expect(source_repository.branch_exists?(source_branch)).to be_truthy + expect(merge_request.reload.should_remove_source_branch?).to be false end end diff --git a/spec/requests/api/namespaces_spec.rb b/spec/requests/api/namespaces_spec.rb index 01dbf523071..09b87f41b82 100644 --- a/spec/requests/api/namespaces_spec.rb +++ b/spec/requests/api/namespaces_spec.rb @@ -325,6 +325,24 @@ RSpec.describe API::Namespaces do expect(response.body).to eq(expected_json) end + it 'ignores paths of groups present in other hierarchies when making suggestions' do + (1..2).to_a.each do |suffix| + create(:group, name: "mygroup#{suffix}", path: "mygroup#{suffix}", parent: namespace2) + end + + create(:group, name: 'mygroup', path: 'mygroup', parent: namespace1) + + get api("/namespaces/mygroup/exists", user), params: { parent_id: namespace1.id } + + # if the paths of groups present in hierachies aren't ignored, the suggestion generated would have + # been `mygroup3`, just because groups with path `mygroup1` and `mygroup2` exists somewhere else. + # But there is no reason for those groups that exists elsewhere to cause a conflict because + # their hierarchies differ. Hence, the correct suggestion to be generated would be `mygroup1` + expected_json = { exists: true, suggests: ["mygroup1"] }.to_json + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(expected_json) + end + it 'ignores top-level namespaces when checking with parent_id' do get api("/namespaces/#{namespace1.path}/exists", user), params: { parent_id: namespace1.id } diff --git a/spec/requests/api/personal_access_tokens_spec.rb b/spec/requests/api/personal_access_tokens_spec.rb index 01f69f0aae2..403c646ee32 100644 --- a/spec/requests/api/personal_access_tokens_spec.rb +++ b/spec/requests/api/personal_access_tokens_spec.rb @@ -73,6 +73,61 @@ RSpec.describe API::PersonalAccessTokens do end end + describe 'GET /personal_access_tokens/:id' do + let_it_be(:user_token) { create(:personal_access_token, user: current_user) } + let_it_be(:user_token_path) { "/personal_access_tokens/#{user_token.id}" } + let_it_be(:invalid_path) { "/personal_access_tokens/#{non_existing_record_id}" } + + context 'when current_user is an administrator', :enable_admin_mode do + let_it_be(:admin_user) { create(:admin) } + let_it_be(:admin_token) { create(:personal_access_token, user: admin_user) } + let_it_be(:admin_path) { "/personal_access_tokens/#{admin_token.id}" } + + it 'returns admins own PAT by id' do + get api(admin_path, admin_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['id']).to eq(admin_token.id) + end + + it 'returns a different users PAT by id' do + get api(user_token_path, admin_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['id']).to eq(user_token.id) + end + + it 'fails to return PAT because no PAT exists with this id' do + get api(invalid_path, admin_user) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when current_user is not an administrator' do + let_it_be(:other_users_path) { "/personal_access_tokens/#{token1.id}" } + + it 'returns users own PAT by id' do + get api(user_token_path, current_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['id']).to eq(user_token.id) + end + + it 'fails to return other users PAT by id' do + get api(other_users_path, current_user) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + + it 'fails to return PAT because no PAT exists with this id' do + get api(invalid_path, current_user) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + describe 'DELETE /personal_access_tokens/self' do let(:path) { '/personal_access_tokens/self' } let(:token) { create(:personal_access_token, user: current_user) } diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index eb6f81c2810..35844631287 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -122,6 +122,7 @@ project_feature: - id - created_at - metrics_dashboard_access_level + - package_registry_access_level - project_id - updated_at computed_attributes: diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index b5aedde2b2e..26e0adc11b3 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -44,6 +44,8 @@ RSpec.describe API::ProjectHooks, 'ProjectHooks' do expect(json_response.first['releases_events']).to eq(true) expect(json_response.first['enable_ssl_verification']).to eq(true) expect(json_response.first['push_events_branch_filter']).to eq('master') + expect(json_response.first['alert_status']).to eq('executable') + expect(json_response.first['disabled_until']).to be_nil end end @@ -76,6 +78,8 @@ RSpec.describe API::ProjectHooks, 'ProjectHooks' do expect(json_response['releases_events']).to eq(hook.releases_events) expect(json_response['deployment_events']).to eq(true) expect(json_response['enable_ssl_verification']).to eq(hook.enable_ssl_verification) + expect(json_response['alert_status']).to eq(hook.alert_status.to_s) + expect(json_response['disabled_until']).to be_nil end it "returns a 404 error if hook id is not available" do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index d2189ab02ea..431d2e56cb5 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -3106,6 +3106,13 @@ RSpec.describe API::Projects do expect(json_response['error']).to eq 'group_access does not have a valid value' end + it "returns a 400 error when the project-group share is created with an OWNER access level" do + post api("/projects/#{project.id}/share", user), params: { group_id: group.id, group_access: Gitlab::Access::OWNER } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq 'group_access does not have a valid value' + end + it "returns a 409 error when link is not saved" do allow(::Projects::GroupLinks::CreateService).to receive_message_chain(:new, :execute) .and_return({ status: :error, http_status: 409, message: 'error' }) diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index 8fa5f409298..a24b852cdac 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -17,7 +17,68 @@ RSpec.describe API::PypiPackages do let(:headers) { {} } - context 'simple API endpoint' do + context 'simple index API endpoint' do + let_it_be(:package) { create(:pypi_package, project: project) } + let_it_be(:package2) { create(:pypi_package, project: project) } + + subject { get api(url), headers: headers } + + describe 'GET /api/v4/groups/:id/-/packages/pypi/simple' do + let(:url) { "/groups/#{group.id}/-/packages/pypi/simple" } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } + + it_behaves_like 'pypi simple index API endpoint' + it_behaves_like 'rejects PyPI access with unknown group id' + + context 'deploy tokens' do + let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: deploy_token, group: group) } + + before do + project.update_column(:visibility_level, Gitlab::VisibilityLevel::PRIVATE) + group.update_column(:visibility_level, Gitlab::VisibilityLevel::PRIVATE) + end + + it_behaves_like 'deploy token for package GET requests' + + context 'with group path as id' do + let(:url) { "/groups/#{CGI.escape(group.full_path)}/-/packages/pypi/simple"} + + it_behaves_like 'deploy token for package GET requests' + end + end + + context 'job token' do + before do + project.update_column(:visibility_level, Gitlab::VisibilityLevel::PRIVATE) + group.update_column(:visibility_level, Gitlab::VisibilityLevel::PRIVATE) + group.add_developer(user) + end + + it_behaves_like 'job token for package GET requests' + end + + it_behaves_like 'a pypi user namespace endpoint' + end + + describe 'GET /api/v4/projects/:id/packages/pypi/simple' do + let(:package_name) { package.name } + let(:url) { "/projects/#{project.id}/packages/pypi/simple" } + let(:snowplow_gitlab_standard_context) { { project: nil, namespace: group } } + + it_behaves_like 'pypi simple index API endpoint' + it_behaves_like 'rejects PyPI access with unknown project id' + it_behaves_like 'deploy token for package GET requests' + it_behaves_like 'job token for package GET requests' + + context 'with project path as id' do + let(:url) { "/projects/#{CGI.escape(project.full_path)}/packages/pypi/simple" } + + it_behaves_like 'deploy token for package GET requests' + end + end + end + + context 'simple package API endpoint' do let_it_be(:package) { create(:pypi_package, project: project) } subject { get api(url), headers: headers } @@ -25,7 +86,7 @@ RSpec.describe API::PypiPackages do describe 'GET /api/v4/groups/:id/-/packages/pypi/simple/:package_name' do let(:package_name) { package.name } let(:url) { "/groups/#{group.id}/-/packages/pypi/simple/#{package_name}" } - let(:snowplow_gitlab_standard_context) { {} } + let(:snowplow_gitlab_standard_context) { { project: nil, namespace: group } } it_behaves_like 'pypi simple API endpoint' it_behaves_like 'rejects PyPI access with unknown group id' diff --git a/spec/requests/api/release/links_spec.rb b/spec/requests/api/release/links_spec.rb index 00326426af5..2345c0063dd 100644 --- a/spec/requests/api/release/links_spec.rb +++ b/spec/requests/api/release/links_spec.rb @@ -49,6 +49,20 @@ RSpec.describe API::Release::Links do expect(response).to match_response_schema('release/links') end + + context 'when using JOB-TOKEN auth' do + let(:job) { create(:ci_build, :running, user: maintainer) } + + it 'returns releases links' do + get api("/projects/#{project.id}/releases/v0.1/assets/links", job_token: job.token) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('release/links') + expect(json_response.count).to eq(2) + end + end + end end context 'when release does not exist' do @@ -116,6 +130,20 @@ RSpec.describe API::Release::Links do expect(response).to match_response_schema('release/link') end + context 'when using JOB-TOKEN auth' do + let(:job) { create(:ci_build, :running, user: maintainer) } + + it 'returns releases link' do + get api("/projects/#{project.id}/releases/v0.1/assets/links/#{release_link.id}", job_token: job.token) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('release/link') + expect(json_response['name']).to eq(release_link.name) + end + end + end + context 'when specified tag is not found in the project' do it_behaves_like '404 response' do let(:request) { get api("/projects/#{project.id}/releases/non_existing_tag/assets/links/#{release_link.id}", maintainer) } @@ -198,6 +226,25 @@ RSpec.describe API::Release::Links do expect(response).to match_response_schema('release/link') end + context 'when using JOB-TOKEN auth' do + let(:job) { create(:ci_build, :running, user: maintainer) } + + it 'creates a new release link' do + expect do + post api("/projects/#{project.id}/releases/v0.1/assets/links"), params: params.merge(job_token: job.token) + end.to change { Releases::Link.count }.by(1) + + release.reload + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:created) + expect(last_release_link.name).to eq('awesome-app.dmg') + expect(last_release_link.filepath).to eq('/binaries/awesome-app.dmg') + expect(last_release_link.url).to eq('https://example.com/download/awesome-app.dmg') + end + end + end + context 'with protected tag' do context 'when user has access to the protected tag' do let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) } @@ -314,6 +361,20 @@ RSpec.describe API::Release::Links do expect(response).to match_response_schema('release/link') end + context 'when using JOB-TOKEN auth' do + let(:job) { create(:ci_build, :running, user: maintainer) } + + it 'updates the release link' do + put api("/projects/#{project.id}/releases/v0.1/assets/links/#{release_link.id}"), params: params.merge(job_token: job.token) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('release/link') + expect(json_response['name']).to eq('awesome-app.msi') + end + end + end + context 'with protected tag' do context 'when user has access to the protected tag' do let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) } @@ -411,6 +472,21 @@ RSpec.describe API::Release::Links do expect(response).to match_response_schema('release/link') end + context 'when using JOB-TOKEN auth' do + let(:job) { create(:ci_build, :running, user: maintainer) } + + it 'deletes the release link' do + expect do + delete api("/projects/#{project.id}/releases/v0.1/assets/links/#{release_link.id}", job_token: job.token) + end.to change { Releases::Link.count }.by(-1) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('release/link') + end + end + end + context 'with protected tag' do context 'when user has access to the protected tag' do let!(:protected_tag) { create(:protected_tag, :developers_can_create, name: '*', project: project) } diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 3c0f3a75f10..c050214ff50 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -227,6 +227,7 @@ RSpec.describe API::Releases do get api("/projects/#{project.id}/releases", maintainer) expect(response).to have_gitlab_http_status(:ok) + expect(json_response[0]['tag_path']).to include('%2F') # properly escape the slash end end @@ -928,6 +929,22 @@ RSpec.describe API::Releases do expect(json_response['message']).to eq('Tag name invalid') end end + + context 'when tag_message is provided' do + let(:tag_message) { 'Annotated tag message created by Release API' } + + before do + params.merge!(tag_message: tag_message) + end + + it 'creates an annotated tag with the tag message' do + expect do + post api("/projects/#{project.id}/releases", maintainer), params: params + end.to change { Project.find_by_id(project.id).repository.tag_count }.by(1) + + expect(project.repository.find_tag(tag_name).message).to eq(tag_message) + end + end end context 'when release already exists' do diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb index d94b70ec0f9..2460a98129f 100644 --- a/spec/requests/api/system_hooks_spec.rb +++ b/spec/requests/api/system_hooks_spec.rb @@ -44,6 +44,8 @@ RSpec.describe API::SystemHooks do expect(json_response.first['merge_requests_events']).to be false expect(json_response.first['repository_update_events']).to be true expect(json_response.first['enable_ssl_verification']).to be true + expect(json_response.first['disabled_until']).to be nil + expect(json_response.first['alert_status']).to eq 'executable' end end end @@ -79,10 +81,43 @@ RSpec.describe API::SystemHooks do 'tag_push_events' => be(hook.tag_push_events), 'merge_requests_events' => be(hook.merge_requests_events), 'repository_update_events' => be(hook.repository_update_events), - 'enable_ssl_verification' => be(hook.enable_ssl_verification) + 'enable_ssl_verification' => be(hook.enable_ssl_verification), + 'alert_status' => eq(hook.alert_status.to_s), + 'disabled_until' => eq(hook.disabled_until&.iso8601(3)) ) end + context 'the hook is disabled' do + before do + hook.disable! + end + + it "has the correct alert status", :aggregate_failures do + get api("/hooks/#{hook.id}", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/system_hook') + expect(json_response).to include('alert_status' => 'disabled') + end + end + + context 'the hook is backed-off' do + before do + hook.backoff! + end + + it "has the correct alert status", :aggregate_failures do + get api("/hooks/#{hook.id}", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/system_hook') + expect(json_response).to include( + 'alert_status' => 'temporarily_disabled', + 'disabled_until' => hook.disabled_until.iso8601(3) + ) + end + end + it 'returns 404 if the system hook does not exist' do get api("/hooks/#{non_existing_record_id}", admin) diff --git a/spec/requests/api/terraform/modules/v1/packages_spec.rb b/spec/requests/api/terraform/modules/v1/packages_spec.rb index 7d86244cb1b..12bce4da011 100644 --- a/spec/requests/api/terraform/modules/v1/packages_spec.rb +++ b/spec/requests/api/terraform/modules/v1/packages_spec.rb @@ -17,12 +17,14 @@ RSpec.describe API::Terraform::Modules::V1::Packages do let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } let(:headers) { {} } + let(:token) { tokens[token_type] } let(:tokens) do { personal_access_token: personal_access_token.token, deploy_token: deploy_token.token, - job_token: job.token + job_token: job.token, + invalid: 'invalid-token123' } end @@ -48,44 +50,43 @@ RSpec.describe API::Terraform::Modules::V1::Packages do end context 'with valid namespace' do - where(:visibility, :user_role, :member, :token_type, :valid_token, :shared_examples_name, :expected_status) do - :public | :developer | true | :personal_access_token | true | 'returns terraform module packages' | :success - :public | :guest | true | :personal_access_token | true | 'returns terraform module packages' | :success - :public | :developer | true | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | true | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :developer | false | :personal_access_token | true | 'returns no terraform module packages' | :success - :public | :guest | false | :personal_access_token | true | 'returns no terraform module packages' | :success - :public | :developer | false | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | false | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :anonymous | false | :personal_access_token | true | 'returns no terraform module packages' | :success - :private | :developer | true | :personal_access_token | true | 'returns terraform module packages' | :success - :private | :guest | true | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | true | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | true | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | false | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :private | :guest | false | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | false | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | false | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :anonymous | false | :personal_access_token | true | 'rejects terraform module packages access' | :unauthorized - :public | :developer | true | :job_token | true | 'returns terraform module packages' | :success - :public | :guest | true | :job_token | true | 'returns no terraform module packages' | :success - :public | :guest | true | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :developer | false | :job_token | true | 'returns no terraform module packages' | :success - :public | :guest | false | :job_token | true | 'returns no terraform module packages' | :success - :public | :developer | false | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | false | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | true | :job_token | true | 'returns terraform module packages' | :success - :private | :guest | true | :job_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | true | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | true | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | false | :job_token | true | 'rejects terraform module packages access' | :forbidden - :private | :guest | false | :job_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | false | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | false | :job_token | false | 'rejects terraform module packages access' | :unauthorized + where(:visibility, :user_role, :member, :token_type, :shared_examples_name, :expected_status) do + :public | :developer | true | :personal_access_token | 'returns terraform module packages' | :success + :public | :guest | true | :personal_access_token | 'returns terraform module packages' | :success + :public | :developer | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :developer | false | :personal_access_token | 'returns no terraform module packages' | :success + :public | :guest | false | :personal_access_token | 'returns no terraform module packages' | :success + :public | :developer | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :anonymous | false | nil | 'returns no terraform module packages' | :success + :private | :developer | true | :personal_access_token | 'returns terraform module packages' | :success + :private | :guest | true | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | false | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :private | :guest | false | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :anonymous | false | nil | 'rejects terraform module packages access' | :unauthorized + :public | :developer | true | :job_token | 'returns terraform module packages' | :success + :public | :guest | true | :job_token | 'returns no terraform module packages' | :success + :public | :guest | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :developer | false | :job_token | 'returns no terraform module packages' | :success + :public | :guest | false | :job_token | 'returns no terraform module packages' | :success + :public | :developer | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | true | :job_token | 'returns terraform module packages' | :success + :private | :guest | true | :job_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | false | :job_token | 'rejects terraform module packages access' | :forbidden + :private | :guest | false | :job_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | false | :invalid | 'rejects terraform module packages access' | :unauthorized end with_them do - let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } let(:headers) { user_role == :anonymous ? {} : { 'Authorization' => "Bearer #{token}" } } before do @@ -104,48 +105,48 @@ RSpec.describe API::Terraform::Modules::V1::Packages do subject { get(url, headers: headers) } context 'with valid namespace' do - where(:visibility, :user_role, :member, :token_type, :valid_token, :shared_examples_name, :expected_status) do - :public | :developer | true | :personal_access_token | true | 'grants terraform module download' | :success - :public | :guest | true | :personal_access_token | true | 'rejects terraform module packages access' | :not_found - :public | :developer | true | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | true | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :developer | false | :personal_access_token | true | 'rejects terraform module packages access' | :not_found - :public | :guest | false | :personal_access_token | true | 'rejects terraform module packages access' | :not_found - :public | :developer | false | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | false | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :anonymous | false | :personal_access_token | true | 'rejects terraform module packages access' | :not_found - :private | :developer | true | :personal_access_token | true | 'grants terraform module download' | :success - :private | :guest | true | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | true | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | true | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | false | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :private | :guest | false | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | false | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | false | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :anonymous | false | :personal_access_token | true | 'rejects terraform module packages access' | :unauthorized - :public | :developer | true | :job_token | true | 'grants terraform module download' | :success - :public | :guest | true | :job_token | true | 'rejects terraform module packages access' | :not_found - :public | :guest | true | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :developer | false | :job_token | true | 'rejects terraform module packages access' | :not_found - :public | :guest | false | :job_token | true | 'rejects terraform module packages access' | :not_found - :public | :developer | false | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | false | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | true | :job_token | true | 'grants terraform module download' | :success - :private | :guest | true | :job_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | true | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | true | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | false | :job_token | true | 'rejects terraform module packages access' | :forbidden - :private | :guest | false | :job_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | false | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | false | :job_token | false | 'rejects terraform module packages access' | :unauthorized + where(:visibility, :user_role, :member, :token_type, :shared_examples_name, :expected_status) do + :public | :developer | true | :personal_access_token | 'grants terraform module download' | :success + :public | :guest | true | :personal_access_token | 'grants terraform module download' | :success + :public | :developer | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :developer | false | :personal_access_token | 'grants terraform module download' | :success + :public | :guest | false | :personal_access_token | 'grants terraform module download' | :success + :public | :developer | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :anonymous | false | nil | 'grants terraform module download' | :success + :private | :developer | true | :personal_access_token | 'grants terraform module download' | :success + :private | :guest | true | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | false | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :private | :guest | false | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :anonymous | false | nil | 'rejects terraform module packages access' | :unauthorized + :public | :developer | true | :job_token | 'grants terraform module download' | :success + :public | :guest | true | :job_token | 'grants terraform module download' | :success + :public | :guest | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :developer | false | :job_token | 'grants terraform module download' | :success + :public | :guest | false | :job_token | 'grants terraform module download' | :success + :public | :developer | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | true | :job_token | 'grants terraform module download' | :success + :private | :guest | true | :job_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | false | :job_token | 'rejects terraform module packages access' | :forbidden + :private | :guest | false | :job_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | false | :invalid | 'rejects terraform module packages access' | :unauthorized end with_them do - let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } let(:headers) { user_role == :anonymous ? {} : { 'Authorization' => "Bearer #{token}" } } before do group.update!(visibility: visibility.to_s) + project.update!(visibility: visibility.to_s) end it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] @@ -158,55 +159,62 @@ RSpec.describe API::Terraform::Modules::V1::Packages do let(:tokens) do { personal_access_token: ::Gitlab::JWTToken.new.tap { |jwt| jwt['token'] = personal_access_token.id }.encoded, - job_token: ::Gitlab::JWTToken.new.tap { |jwt| jwt['token'] = job.token }.encoded + job_token: ::Gitlab::JWTToken.new.tap { |jwt| jwt['token'] = job.token }.encoded, + invalid: 'invalid-token123' } end subject { get(url, headers: headers) } context 'with valid namespace' do - where(:visibility, :user_role, :member, :token_type, :valid_token, :shared_examples_name, :expected_status) do - :public | :developer | true | :personal_access_token | true | 'grants terraform module package file access' | :success - :public | :guest | true | :personal_access_token | true | 'rejects terraform module packages access' | :not_found - :public | :developer | true | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | true | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :developer | false | :personal_access_token | true | 'rejects terraform module packages access' | :not_found - :public | :guest | false | :personal_access_token | true | 'rejects terraform module packages access' | :not_found - :public | :developer | false | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | false | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :anonymous | false | :personal_access_token | true | 'rejects terraform module packages access' | :not_found - :private | :developer | true | :personal_access_token | true | 'grants terraform module package file access' | :success - :private | :guest | true | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | true | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | true | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | false | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :private | :guest | false | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | false | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | false | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :anonymous | false | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :public | :developer | true | :job_token | true | 'grants terraform module package file access' | :success - :public | :guest | true | :job_token | true | 'rejects terraform module packages access' | :not_found - :public | :guest | true | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :developer | false | :job_token | true | 'rejects terraform module packages access' | :not_found - :public | :guest | false | :job_token | true | 'rejects terraform module packages access' | :not_found - :public | :developer | false | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | false | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | true | :job_token | true | 'grants terraform module package file access' | :success - :private | :guest | true | :job_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | true | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | true | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | false | :job_token | true | 'rejects terraform module packages access' | :forbidden - :private | :guest | false | :job_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | false | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | false | :job_token | false | 'rejects terraform module packages access' | :unauthorized + where(:visibility, :user_role, :member, :token_type, :shared_examples_name, :expected_status) do + :public | :developer | true | :personal_access_token | 'grants terraform module package file access' | :success + :public | :guest | true | :personal_access_token | 'grants terraform module package file access' | :success + :public | :developer | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :developer | false | :personal_access_token | 'grants terraform module package file access' | :success + :public | :guest | false | :personal_access_token | 'grants terraform module package file access' | :success + :public | :developer | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :anonymous | false | nil | 'grants terraform module package file access' | :success + :private | :developer | true | :personal_access_token | 'grants terraform module package file access' | :success + :private | :guest | true | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | false | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :private | :guest | false | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :anonymous | false | nil | 'rejects terraform module packages access' | :unauthorized + :public | :developer | true | :job_token | 'grants terraform module package file access' | :success + :public | :guest | true | :job_token | 'grants terraform module package file access' | :success + :public | :guest | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :developer | false | :job_token | 'grants terraform module package file access' | :success + :public | :guest | false | :job_token | 'grants terraform module package file access' | :success + :public | :developer | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | true | :job_token | 'grants terraform module package file access' | :success + :private | :guest | true | :job_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | true | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | false | :job_token | 'rejects terraform module packages access' | :forbidden + :private | :guest | false | :job_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | false | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | false | :invalid | 'rejects terraform module packages access' | :unauthorized end with_them do - let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } - let(:snowplow_gitlab_standard_context) { { project: project, user: user, namespace: project.namespace } } + let(:snowplow_gitlab_standard_context) do + { + project: project, + user: user_role == :anonymous ? nil : user, + namespace: project.namespace + } + end before do group.update!(visibility: visibility.to_s) + project.update!(visibility: visibility.to_s) end it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] @@ -244,49 +252,48 @@ RSpec.describe API::Terraform::Modules::V1::Packages do subject { put(url, headers: headers) } context 'with valid project' do - where(:visibility, :user_role, :member, :token_header, :token_type, :valid_token, :shared_examples_name, :expected_status) do - :public | :developer | true | 'PRIVATE-TOKEN' | :personal_access_token | true | 'process terraform module workhorse authorization' | :success - :public | :guest | true | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :public | :developer | true | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | true | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :developer | false | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :public | :guest | false | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :public | :developer | false | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | false | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :anonymous | false | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :unauthorized - :private | :developer | true | 'PRIVATE-TOKEN' | :personal_access_token | true | 'process terraform module workhorse authorization' | :success - :private | :guest | true | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | true | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | true | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | false | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :not_found - :private | :guest | false | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :not_found - :private | :developer | false | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | false | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :anonymous | false | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :unauthorized - :public | :developer | true | 'JOB-TOKEN' | :job_token | true | 'process terraform module workhorse authorization' | :success - :public | :guest | true | 'JOB-TOKEN' | :job_token | true | 'rejects terraform module packages access' | :forbidden - :public | :developer | true | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | true | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :developer | false | 'JOB-TOKEN' | :job_token | true | 'rejects terraform module packages access' | :forbidden - :public | :guest | false | 'JOB-TOKEN' | :job_token | true | 'rejects terraform module packages access' | :forbidden - :public | :developer | false | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | false | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | true | 'JOB-TOKEN' | :job_token | true | 'process terraform module workhorse authorization' | :success - :private | :guest | true | 'JOB-TOKEN' | :job_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | true | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | true | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | false | 'JOB-TOKEN' | :job_token | true | 'rejects terraform module packages access' | :not_found - :private | :guest | false | 'JOB-TOKEN' | :job_token | true | 'rejects terraform module packages access' | :not_found - :private | :developer | false | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | false | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :developer | true | 'DEPLOY-TOKEN' | :deploy_token | true | 'process terraform module workhorse authorization' | :success - :public | :developer | true | 'DEPLOY-TOKEN' | :deploy_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | true | 'DEPLOY-TOKEN' | :deploy_token | true | 'process terraform module workhorse authorization' | :success - :private | :developer | true | 'DEPLOY-TOKEN' | :deploy_token | false | 'rejects terraform module packages access' | :unauthorized + where(:visibility, :user_role, :member, :token_header, :token_type, :shared_examples_name, :expected_status) do + :public | :developer | true | 'PRIVATE-TOKEN' | :personal_access_token | 'process terraform module workhorse authorization' | :success + :public | :guest | true | 'PRIVATE-TOKEN' | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :public | :developer | true | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | true | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :developer | false | 'PRIVATE-TOKEN' | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :public | :guest | false | 'PRIVATE-TOKEN' | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :public | :developer | false | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | false | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :anonymous | false | nil | nil | 'rejects terraform module packages access' | :unauthorized + :private | :developer | true | 'PRIVATE-TOKEN' | :personal_access_token | 'process terraform module workhorse authorization' | :success + :private | :guest | true | 'PRIVATE-TOKEN' | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | true | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | true | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | false | 'PRIVATE-TOKEN' | :personal_access_token | 'rejects terraform module packages access' | :not_found + :private | :guest | false | 'PRIVATE-TOKEN' | :personal_access_token | 'rejects terraform module packages access' | :not_found + :private | :developer | false | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | false | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :anonymous | false | nil | nil | 'rejects terraform module packages access' | :unauthorized + :public | :developer | true | 'JOB-TOKEN' | :job_token | 'process terraform module workhorse authorization' | :success + :public | :guest | true | 'JOB-TOKEN' | :job_token | 'rejects terraform module packages access' | :forbidden + :public | :developer | true | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | true | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :developer | false | 'JOB-TOKEN' | :job_token | 'rejects terraform module packages access' | :forbidden + :public | :guest | false | 'JOB-TOKEN' | :job_token | 'rejects terraform module packages access' | :forbidden + :public | :developer | false | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | false | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | true | 'JOB-TOKEN' | :job_token | 'process terraform module workhorse authorization' | :success + :private | :guest | true | 'JOB-TOKEN' | :job_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | true | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | true | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | false | 'JOB-TOKEN' | :job_token | 'rejects terraform module packages access' | :not_found + :private | :guest | false | 'JOB-TOKEN' | :job_token | 'rejects terraform module packages access' | :not_found + :private | :developer | false | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | false | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :developer | true | 'DEPLOY-TOKEN' | :deploy_token | 'process terraform module workhorse authorization' | :success + :public | :developer | true | 'DEPLOY-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | true | 'DEPLOY-TOKEN' | :deploy_token | 'process terraform module workhorse authorization' | :success + :private | :developer | true | 'DEPLOY-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized end with_them do - let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } let(:headers) { user_headers.merge(workhorse_headers) } let(:user_headers) { user_role == :anonymous ? {} : { token_header => token } } @@ -322,49 +329,48 @@ RSpec.describe API::Terraform::Modules::V1::Packages do end context 'with valid project' do - where(:visibility, :user_role, :member, :token_header, :token_type, :valid_token, :shared_examples_name, :expected_status) do - :public | :developer | true | 'PRIVATE-TOKEN' | :personal_access_token | true | 'process terraform module upload' | :created - :public | :guest | true | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :public | :developer | true | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | true | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :developer | false | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :public | :guest | false | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :public | :developer | false | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | false | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :anonymous | false | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :unauthorized - :private | :developer | true | 'PRIVATE-TOKEN' | :personal_access_token | true | 'process terraform module upload' | :created - :private | :guest | true | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | true | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | true | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | false | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :not_found - :private | :guest | false | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :not_found - :private | :developer | false | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | false | 'PRIVATE-TOKEN' | :personal_access_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :anonymous | false | 'PRIVATE-TOKEN' | :personal_access_token | true | 'rejects terraform module packages access' | :unauthorized - :public | :developer | true | 'JOB-TOKEN' | :job_token | true | 'process terraform module upload' | :created - :public | :guest | true | 'JOB-TOKEN' | :job_token | true | 'rejects terraform module packages access' | :forbidden - :public | :developer | true | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | true | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :developer | false | 'JOB-TOKEN' | :job_token | true | 'rejects terraform module packages access' | :forbidden - :public | :guest | false | 'JOB-TOKEN' | :job_token | true | 'rejects terraform module packages access' | :forbidden - :public | :developer | false | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :guest | false | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | true | 'JOB-TOKEN' | :job_token | true | 'process terraform module upload' | :created - :private | :guest | true | 'JOB-TOKEN' | :job_token | true | 'rejects terraform module packages access' | :forbidden - :private | :developer | true | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | true | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | false | 'JOB-TOKEN' | :job_token | true | 'rejects terraform module packages access' | :not_found - :private | :guest | false | 'JOB-TOKEN' | :job_token | true | 'rejects terraform module packages access' | :not_found - :private | :developer | false | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :guest | false | 'JOB-TOKEN' | :job_token | false | 'rejects terraform module packages access' | :unauthorized - :public | :developer | true | 'DEPLOY-TOKEN' | :deploy_token | true | 'process terraform module upload' | :created - :public | :developer | true | 'DEPLOY-TOKEN' | :deploy_token | false | 'rejects terraform module packages access' | :unauthorized - :private | :developer | true | 'DEPLOY-TOKEN' | :deploy_token | true | 'process terraform module upload' | :created - :private | :developer | true | 'DEPLOY-TOKEN' | :deploy_token | false | 'rejects terraform module packages access' | :unauthorized + where(:visibility, :user_role, :member, :token_header, :token_type, :shared_examples_name, :expected_status) do + :public | :developer | true | 'PRIVATE-TOKEN' | :personal_access_token | 'process terraform module upload' | :created + :public | :guest | true | 'PRIVATE-TOKEN' | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :public | :developer | true | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | true | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :developer | false | 'PRIVATE-TOKEN' | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :public | :guest | false | 'PRIVATE-TOKEN' | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :public | :developer | false | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | false | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :anonymous | false | nil | nil | 'rejects terraform module packages access' | :unauthorized + :private | :developer | true | 'PRIVATE-TOKEN' | :personal_access_token | 'process terraform module upload' | :created + :private | :guest | true | 'PRIVATE-TOKEN' | :personal_access_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | true | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | true | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | false | 'PRIVATE-TOKEN' | :personal_access_token | 'rejects terraform module packages access' | :not_found + :private | :guest | false | 'PRIVATE-TOKEN' | :personal_access_token | 'rejects terraform module packages access' | :not_found + :private | :developer | false | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | false | 'PRIVATE-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :anonymous | false | nil | nil | 'rejects terraform module packages access' | :unauthorized + :public | :developer | true | 'JOB-TOKEN' | :job_token | 'process terraform module upload' | :created + :public | :guest | true | 'JOB-TOKEN' | :job_token | 'rejects terraform module packages access' | :forbidden + :public | :developer | true | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | true | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :developer | false | 'JOB-TOKEN' | :job_token | 'rejects terraform module packages access' | :forbidden + :public | :guest | false | 'JOB-TOKEN' | :job_token | 'rejects terraform module packages access' | :forbidden + :public | :developer | false | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :guest | false | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | true | 'JOB-TOKEN' | :job_token | 'process terraform module upload' | :created + :private | :guest | true | 'JOB-TOKEN' | :job_token | 'rejects terraform module packages access' | :forbidden + :private | :developer | true | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | true | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | false | 'JOB-TOKEN' | :job_token | 'rejects terraform module packages access' | :not_found + :private | :guest | false | 'JOB-TOKEN' | :job_token | 'rejects terraform module packages access' | :not_found + :private | :developer | false | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :guest | false | 'JOB-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :public | :developer | true | 'DEPLOY-TOKEN' | :deploy_token | 'process terraform module upload' | :created + :public | :developer | true | 'DEPLOY-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized + :private | :developer | true | 'DEPLOY-TOKEN' | :deploy_token | 'process terraform module upload' | :created + :private | :developer | true | 'DEPLOY-TOKEN' | :invalid | 'rejects terraform module packages access' | :unauthorized end with_them do - let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } let(:user_headers) { user_role == :anonymous ? {} : { token_header => token } } let(:headers) { user_headers.merge(workhorse_headers) } let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: snowplow_user } } diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb index ae1e461d433..e8458db4a4d 100644 --- a/spec/requests/api/terraform/state_spec.rb +++ b/spec/requests/api/terraform/state_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Terraform::State do +RSpec.describe API::Terraform::State, :snowplow do include HttpBasicAuthHelpers let_it_be(:project) { create(:project) } @@ -25,11 +25,17 @@ RSpec.describe API::Terraform::State do context 'without authentication' do let(:auth_header) { basic_auth_header('bad', 'token') } - it 'does not track unique event' do + it 'does not track unique hll event' do expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) request end + + it 'does not track Snowplow event' do + request + + expect_no_snowplow_event + end end context 'with maintainer permissions' do @@ -39,6 +45,41 @@ RSpec.describe API::Terraform::State do let(:target_event) { 'p_terraform_state_api_unique_users' } let(:expected_value) { instance_of(Integer) } end + + it 'tracks Snowplow event' do + request + + expect_snowplow_event( + category: described_class.to_s, + action: 'p_terraform_state_api_unique_users', + namespace: project.namespace.reload, + user: current_user + ) + end + + context 'when route_hll_to_snowplow_phase2 FF is disabled' do + before do + stub_feature_flags(route_hll_to_snowplow_phase2: false) + end + + it 'does not track Snowplow event' do + request + + expect_no_snowplow_event + end + end + end + end + + shared_context 'cannot access a state that is scheduled for deletion' do + before do + state.update!(deleted_at: Time.current) + end + + it 'returns unprocessable entity' do + request + + expect(response).to have_gitlab_http_status(:unprocessable_entity) end end @@ -77,6 +118,8 @@ RSpec.describe API::Terraform::State do expect(response).to have_gitlab_http_status(:not_found) end end + + it_behaves_like 'cannot access a state that is scheduled for deletion' end context 'with developer permissions' do @@ -162,6 +205,8 @@ RSpec.describe API::Terraform::State do expect(response).to have_gitlab_http_status(:unprocessable_entity) end end + + it_behaves_like 'cannot access a state that is scheduled for deletion' end context 'without body' do @@ -240,13 +285,19 @@ RSpec.describe API::Terraform::State do context 'with maintainer permissions' do let(:current_user) { maintainer } + let(:deletion_service) { instance_double(Terraform::States::TriggerDestroyService) } + + it 'schedules the state for deletion and returns empty body' do + expect(Terraform::States::TriggerDestroyService).to receive(:new).and_return(deletion_service) + expect(deletion_service).to receive(:execute).once - it 'deletes the state and returns empty body' do - expect { request }.to change { Terraform::State.count }.by(-1) + request expect(response).to have_gitlab_http_status(:ok) expect(Gitlab::Json.parse(response.body)).to be_empty end + + it_behaves_like 'cannot access a state that is scheduled for deletion' end context 'with developer permissions' do @@ -276,6 +327,7 @@ RSpec.describe API::Terraform::State do subject(:request) { post api("#{state_path}/lock"), headers: auth_header, params: params } it_behaves_like 'endpoint with unique user tracking' + it_behaves_like 'cannot access a state that is scheduled for deletion' it 'locks the terraform state' do request @@ -330,6 +382,10 @@ RSpec.describe API::Terraform::State do let(:lock_id) { 'irrelevant to this test, just needs to be present' } end + it_behaves_like 'cannot access a state that is scheduled for deletion' do + let(:lock_id) { 'irrelevant to this test, just needs to be present' } + end + context 'with the correct lock id' do let(:lock_id) { '123-456' } diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 2c5a734a0e1..d4dc7375e9e 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1694,6 +1694,111 @@ RSpec.describe API::Users do end end + describe 'GET /users/:id/project_deploy_keys' do + let(:project) { create(:project) } + + before do + project.add_maintainer(user) + + deploy_key = create(:deploy_key, user: user) + create(:deploy_keys_project, project: project, deploy_key_id: deploy_key.id) + end + + it 'returns 404 for non-existing user' do + get api("/users/#{non_existing_record_id}/project_deploy_keys") + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 User Not Found') + end + + it 'returns array of project deploy keys with pagination' do + get api("/users/#{user.id}/project_deploy_keys", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.first['title']).to eq(user.deploy_keys.first.title) + end + + it 'forbids when a developer fetches maintainer keys' do + dev_user = create(:user) + project.add_developer(dev_user) + + get api("/users/#{user.id}/project_deploy_keys", dev_user) + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('403 Forbidden - No common authorized project found') + end + + context 'with multiple projects' do + let(:second_project) { create(:project) } + let(:second_user) { create(:user) } + + before do + second_project.add_maintainer(second_user) + + deploy_key = create(:deploy_key, user: second_user) + create(:deploy_keys_project, project: second_project, deploy_key_id: deploy_key.id) + end + + context 'when no common projects for user and current_user' do + it 'forbids' do + get api("/users/#{user.id}/project_deploy_keys", second_user) + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('403 Forbidden - No common authorized project found') + end + end + + context 'when there are common projects for user and current_user' do + before do + project.add_maintainer(second_user) + end + + it 'lists only common project keys' do + expect(second_user.project_deploy_keys).to contain_exactly( + project.deploy_keys.first, second_project.deploy_keys.first) + + get api("/users/#{second_user.id}/project_deploy_keys", user) + + expect(json_response.count).to eq(1) + expect(json_response.first['key']).to eq(project.deploy_keys.first.key) + end + + it 'lists only project_deploy_keys and not user deploy_keys' do + third_user = create(:user) + + project.add_maintainer(third_user) + second_project.add_maintainer(third_user) + + create(:deploy_key, user: second_user) + create(:deploy_key, user: third_user) + + get api("/users/#{second_user.id}/project_deploy_keys", third_user) + + expect(json_response.count).to eq(2) + expect([json_response.first['key'], json_response.second['key']]).to contain_exactly( + project.deploy_keys.first.key, second_project.deploy_keys.first.key) + end + + it 'avoids N+1 queries' do + second_project.add_maintainer(user) + + control_count = ActiveRecord::QueryRecorder.new do + get api("/users/#{second_user.id}/project_deploy_keys", user) + end.count + + deploy_key = create(:deploy_key, user: second_user) + create(:deploy_keys_project, project: second_project, deploy_key_id: deploy_key.id) + + expect do + get api("/users/#{second_user.id}/project_deploy_keys", user) + end.not_to exceed_query_limit(control_count) + end + end + end + end + describe 'GET /user/:id/keys' do it 'returns 404 for non-existing user' do get api("/users/#{non_existing_record_id}/keys") diff --git a/spec/requests/api/wikis_spec.rb b/spec/requests/api/wikis_spec.rb index 06ae61ca5eb..f4096eef8d0 100644 --- a/spec/requests/api/wikis_spec.rb +++ b/spec/requests/api/wikis_spec.rb @@ -256,6 +256,24 @@ RSpec.describe API::Wikis do include_examples 'wiki API 404 Wiki Page Not Found' end end + + context 'when content contains a reference' do + let(:issue) { create(:issue, project: project) } + let(:params) { { render_html: true } } + let(:page) { create(:wiki_page, wiki: project.wiki, title: 'page_with_ref', content: issue.to_reference) } + let(:expected_content) { %r{<a href=".*#{issue.iid}".*>#{issue.to_reference}</a>} } + + before do + project.add_developer(user) + + request + end + + it 'expands the reference in the content' do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['content']).to match(expected_content) + end + end end end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 38c8d43376e..05b16119a0e 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -245,6 +245,16 @@ RSpec.describe 'Git HTTP requests' do end end + context 'when project name is missing' do + let(:path) { "/#{user.namespace.path}/info/refs" } + + it 'does not redirect to the incorrect path' do + get path + + expect(response).not_to redirect_to("/#{user.namespace.path}.git/info/refs") + end + end + it_behaves_like 'project path without .git suffix' do let(:repository_path) { "#{user.namespace.path}/project.git-project" } end diff --git a/spec/requests/groups/crm/contacts_controller_spec.rb b/spec/requests/groups/crm/contacts_controller_spec.rb index 0ee72233418..70086ddbbba 100644 --- a/spec/requests/groups/crm/contacts_controller_spec.rb +++ b/spec/requests/groups/crm/contacts_controller_spec.rb @@ -42,14 +42,6 @@ RSpec.describe Groups::Crm::ContactsController do it_behaves_like 'response with 404 status' end - context 'when feature flag is disabled' do - before do - stub_feature_flags(customer_relations: false) - end - - it_behaves_like 'response with 404 status' - end - context 'when subgroup' do let(:group) { create(:group, :private, :crm_enabled, parent: create(:group)) } diff --git a/spec/requests/groups/crm/organizations_controller_spec.rb b/spec/requests/groups/crm/organizations_controller_spec.rb index 410fc979262..e841dd80b67 100644 --- a/spec/requests/groups/crm/organizations_controller_spec.rb +++ b/spec/requests/groups/crm/organizations_controller_spec.rb @@ -42,14 +42,6 @@ RSpec.describe Groups::Crm::OrganizationsController do it_behaves_like 'response with 404 status' end - context 'when feature flag is disabled' do - before do - stub_feature_flags(customer_relations: false) - end - - it_behaves_like 'response with 404 status' - end - context 'when subgroup' do let(:group) { create(:group, :private, :crm_enabled, parent: create(:group)) } diff --git a/spec/requests/ide_controller_spec.rb b/spec/requests/ide_controller_spec.rb index 4bf1e43ba40..151fa89b819 100644 --- a/spec/requests/ide_controller_spec.rb +++ b/spec/requests/ide_controller_spec.rb @@ -208,6 +208,31 @@ RSpec.describe IdeController do it_behaves_like 'user access rights check' end + + describe 'Snowplow view event', :snowplow do + it 'is tracked' do + subject + + expect_snowplow_event( + category: described_class.to_s, + action: 'web_ide_views', + namespace: project.namespace, + user: user + ) + end + + context 'when route_hll_to_snowplow_phase2 FF is disabled' do + before do + stub_feature_flags(route_hll_to_snowplow_phase2: false) + end + + it 'does not track Snowplow event' do + subject + + expect_no_snowplow_event + end + end + end end end end diff --git a/spec/requests/jira_connect/oauth_application_ids_controller_spec.rb b/spec/requests/jira_connect/oauth_application_ids_controller_spec.rb new file mode 100644 index 00000000000..ffeaf1075f3 --- /dev/null +++ b/spec/requests/jira_connect/oauth_application_ids_controller_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::OauthApplicationIdsController do + describe 'GET /-/jira_connect/oauth_application_id' do + before do + stub_application_setting(jira_connect_application_key: '123456') + end + + it 'renders the jira connect application id' do + get '/-/jira_connect/oauth_application_id' + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ "application_id" => "123456" }) + end + + context 'application ID is empty' do + before do + stub_application_setting(jira_connect_application_key: '') + end + + it 'renders not found' do + get '/-/jira_connect/oauth_application_id' + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when jira_connect_oauth_self_managed disabled' do + before do + stub_feature_flags(jira_connect_oauth_self_managed: false) + end + + it 'renders not found' do + get '/-/jira_connect/oauth_application_id' + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/requests/mailgun/webhooks_controller_spec.rb b/spec/requests/mailgun/webhooks_controller_spec.rb new file mode 100644 index 00000000000..ae6dc89d003 --- /dev/null +++ b/spec/requests/mailgun/webhooks_controller_spec.rb @@ -0,0 +1,149 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mailgun::WebhooksController do + let(:mailgun_signing_key) { 'abc123' } + let(:valid_signature) do + { + timestamp: "1625056677", + token: "eb944d0ace7227667a1b97d2d07276ae51d2b849ed2cfa68f3", + signature: "9790cc6686eb70f0b1f869180d906870cdfd496d27fee81da0aa86b9e539e790" + } + end + + let(:event_data) { {} } + + before do + stub_application_setting(mailgun_events_enabled: true, mailgun_signing_key: mailgun_signing_key) + end + + def post_request(override_params = {}) + post mailgun_webhooks_path, params: standard_params.merge(override_params) + end + + describe '#process_webhook' do + it 'returns 406 when integration is not enabled' do + stub_application_setting(mailgun_events_enabled: false) + + post_request + + expect(response).to have_gitlab_http_status(:not_acceptable) + end + + it 'returns 404 when signing key is not configured' do + stub_application_setting(mailgun_signing_key: nil) + + post_request + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns 404 when signature invalid' do + post_request( + 'signature' => valid_signature.merge('signature' => 'xxx') + ) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns 200 when signature is valid' do + post_request + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'invite email failures' do + let_it_be(:member) { create(:project_member, :invited) } + + let(:event_data) do + { + 'event': 'failed', + 'severity': 'permanent', + 'tags': [Members::Mailgun::INVITE_EMAIL_TAG], + 'user-variables': { + Members::Mailgun::INVITE_EMAIL_TOKEN_KEY => member.raw_invite_token + } + } + end + + it 'marks the member invite email success as false' do + expect { post_request }.to change { member.reload.invite_email_success }.from(true).to(false) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'supports legacy URL' do + expect do + post members_mailgun_permanent_failures_path, params: { + 'signature' => valid_signature, + 'event-data' => event_data + } + end.to change { member.reload.invite_email_success }.from(true).to(false) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'does not change the invite status if failure is temporary' do + expect do + post_request({ 'event-data' => event_data.merge(severity: 'temporary') }) + end.not_to change { member.reload.invite_email_success } + + expect(response).to have_gitlab_http_status(:ok) + end + end + + def standard_params + { + "signature": valid_signature, + "event-data": { + "severity": "permanent", + "tags": ["invite_email"], + "timestamp": 1521233195.375624, + "storage": { + "url": "_anything_", + "key": "_anything_" + }, + "log-level": "error", + "id": "_anything_", + "campaigns": [], + "reason": "suppress-bounce", + "user-variables": { + "invite_token": '12345' + }, + "flags": { + "is-routed": false, + "is-authenticated": true, + "is-system-test": false, + "is-test-mode": false + }, + "recipient-domain": "example.com", + "envelope": { + "sender": "bob@mg.gitlab.com", + "transport": "smtp", + "targets": "alice@example.com" + }, + "message": { + "headers": { + "to": "Alice <alice@example.com>", + "message-id": "20130503192659.13651.20287@mg.gitlab.com", + "from": "Bob <bob@mg.gitlab.com>", + "subject": "Test permanent_fail webhook" + }, + "attachments": [], + "size": 111 + }, + "recipient": "alice@example.com", + "event": "failed", + "delivery-status": { + "attempt-no": 1, + "message": "", + "code": 605, + "description": "Not delivering to previously bounced address", + "session-seconds": 0 + } + }.merge(event_data) + } + end +end diff --git a/spec/requests/members/mailgun/permanent_failure_spec.rb b/spec/requests/members/mailgun/permanent_failure_spec.rb deleted file mode 100644 index e47aedf8e94..00000000000 --- a/spec/requests/members/mailgun/permanent_failure_spec.rb +++ /dev/null @@ -1,128 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'receive a permanent failure' do - describe 'POST /members/mailgun/permanent_failures', :aggregate_failures do - let_it_be(:member) { create(:project_member, :invited) } - - let(:raw_invite_token) { member.raw_invite_token } - let(:mailgun_events) { true } - let(:mailgun_signing_key) { 'abc123' } - - subject(:post_request) { post members_mailgun_permanent_failures_path(standard_params) } - - before do - stub_application_setting(mailgun_events_enabled: mailgun_events, mailgun_signing_key: mailgun_signing_key) - end - - it 'marks the member invite email success as false' do - expect { post_request }.to change { member.reload.invite_email_success }.from(true).to(false) - - expect(response).to have_gitlab_http_status(:ok) - end - - context 'when the change to a member is not made' do - context 'with incorrect signing key' do - context 'with incorrect signing key' do - let(:mailgun_signing_key) { '_foobar_' } - - it 'does not change member status and responds as not_found' do - expect { post_request }.not_to change { member.reload.invite_email_success } - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'with nil signing key' do - let(:mailgun_signing_key) { nil } - - it 'does not change member status and responds as not_found' do - expect { post_request }.not_to change { member.reload.invite_email_success } - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - context 'when the feature is not enabled' do - let(:mailgun_events) { false } - - it 'does not change member status and responds as expected' do - expect { post_request }.not_to change { member.reload.invite_email_success } - - expect(response).to have_gitlab_http_status(:not_acceptable) - end - end - - context 'when it is not an invite email' do - before do - stub_const('::Members::Mailgun::INVITE_EMAIL_TAG', '_foobar_') - end - - it 'does not change member status and responds as expected' do - expect { post_request }.not_to change { member.reload.invite_email_success } - - expect(response).to have_gitlab_http_status(:not_acceptable) - end - end - end - - def standard_params - { - "signature": { - "timestamp": "1625056677", - "token": "eb944d0ace7227667a1b97d2d07276ae51d2b849ed2cfa68f3", - "signature": "9790cc6686eb70f0b1f869180d906870cdfd496d27fee81da0aa86b9e539e790" - }, - "event-data": { - "severity": "permanent", - "tags": ["invite_email"], - "timestamp": 1521233195.375624, - "storage": { - "url": "_anything_", - "key": "_anything_" - }, - "log-level": "error", - "id": "_anything_", - "campaigns": [], - "reason": "suppress-bounce", - "user-variables": { - "invite_token": raw_invite_token - }, - "flags": { - "is-routed": false, - "is-authenticated": true, - "is-system-test": false, - "is-test-mode": false - }, - "recipient-domain": "example.com", - "envelope": { - "sender": "bob@mg.gitlab.com", - "transport": "smtp", - "targets": "alice@example.com" - }, - "message": { - "headers": { - "to": "Alice <alice@example.com>", - "message-id": "20130503192659.13651.20287@mg.gitlab.com", - "from": "Bob <bob@mg.gitlab.com>", - "subject": "Test permanent_fail webhook" - }, - "attachments": [], - "size": 111 - }, - "recipient": "alice@example.com", - "event": "failed", - "delivery-status": { - "attempt-no": 1, - "message": "", - "code": 605, - "description": "Not delivering to previously bounced address", - "session-seconds": 0 - } - } - } - end - end -end diff --git a/spec/requests/oauth/authorizations_controller_spec.rb b/spec/requests/oauth/authorizations_controller_spec.rb new file mode 100644 index 00000000000..8d19c92865e --- /dev/null +++ b/spec/requests/oauth/authorizations_controller_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Oauth::AuthorizationsController do + let_it_be(:user) { create(:user) } + let_it_be(:application) { create(:oauth_application, redirect_uri: 'custom://test') } + let_it_be(:oauth_authorization_path) do + Gitlab::Routing.url_helpers.oauth_authorization_url( + client_id: application.uid, + response_type: 'code', + scope: application.scopes, + redirect_uri: application.redirect_uri, + state: SecureRandom.hex + ) + end + + before do + sign_in(user) + end + + describe 'GET #new' do + context 'when application redirect URI has a custom scheme' do + context 'when CSP is disabled' do + before do + allow_next_instance_of(ActionDispatch::Request) do |instance| + allow(instance).to receive(:content_security_policy).and_return(nil) + end + end + + it 'does not add a CSP' do + get oauth_authorization_path + + expect(response.headers['Content-Security-Policy']).to be_nil + end + end + + context 'when CSP contains form-action' do + before do + csp = ActionDispatch::ContentSecurityPolicy.new do |p| + p.form_action "'self'" + end + + allow_next_instance_of(ActionDispatch::Request) do |instance| + allow(instance).to receive(:content_security_policy).and_return(csp) + end + end + + it 'adds custom scheme to CSP form-action' do + get oauth_authorization_path + + expect(response.headers['Content-Security-Policy']).to include("form-action 'self' custom:") + end + end + + context 'when CSP does not contain form-action' do + before do + csp = ActionDispatch::ContentSecurityPolicy.new do |p| + p.script_src :self, 'https://some-cdn.test' + p.style_src :self, 'https://some-cdn.test' + end + + allow_next_instance_of(ActionDispatch::Request) do |instance| + allow(instance).to receive(:content_security_policy).and_return(csp) + end + end + + it 'does not add form-action to the CSP' do + get oauth_authorization_path + + expect(response.headers['Content-Security-Policy']).not_to include('form-action') + end + end + end + end +end diff --git a/spec/requests/oauth/tokens_controller_spec.rb b/spec/requests/oauth/tokens_controller_spec.rb index 1967d0ba8b1..3895304dbde 100644 --- a/spec/requests/oauth/tokens_controller_spec.rb +++ b/spec/requests/oauth/tokens_controller_spec.rb @@ -6,11 +6,12 @@ RSpec.describe Oauth::TokensController do let(:cors_request_headers) { { 'Origin' => 'http://notgitlab.com' } } let(:other_headers) { {} } let(:headers) { cors_request_headers.merge(other_headers)} + let(:allowed_methods) { 'POST, OPTIONS' } shared_examples 'cross-origin POST request' do it 'allows cross-origin requests' do expect(response.headers['Access-Control-Allow-Origin']).to eq '*' - expect(response.headers['Access-Control-Allow-Methods']).to eq 'POST' + expect(response.headers['Access-Control-Allow-Methods']).to eq allowed_methods expect(response.headers['Access-Control-Allow-Headers']).to be_nil expect(response.headers['Access-Control-Allow-Credentials']).to be_nil end @@ -23,7 +24,7 @@ RSpec.describe Oauth::TokensController do it 'allows cross-origin requests' do expect(response.headers['Access-Control-Allow-Origin']).to eq '*' - expect(response.headers['Access-Control-Allow-Methods']).to eq 'POST' + expect(response.headers['Access-Control-Allow-Methods']).to eq allowed_methods expect(response.headers['Access-Control-Allow-Headers']).to eq 'Authorization' expect(response.headers['Access-Control-Allow-Credentials']).to be_nil end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 70a310ba0d5..c647fee1564 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -98,7 +98,7 @@ RSpec.describe 'OpenID Connect requests' do shared_examples 'cross-origin GET and POST request' do it 'allows cross-origin request' do expect(response.headers['Access-Control-Allow-Origin']).to eq '*' - expect(response.headers['Access-Control-Allow-Methods']).to eq 'GET, HEAD, POST' + expect(response.headers['Access-Control-Allow-Methods']).to eq 'GET, HEAD, POST, OPTIONS' expect(response.headers['Access-Control-Allow-Headers']).to be_nil expect(response.headers['Access-Control-Allow-Credentials']).to be_nil end diff --git a/spec/requests/projects/environments_controller_spec.rb b/spec/requests/projects/environments_controller_spec.rb new file mode 100644 index 00000000000..5cdf507abef --- /dev/null +++ b/spec/requests/projects/environments_controller_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::EnvironmentsController do + let_it_be(:project) { create(:project, :repository) } + + let(:environment) { create(:environment, name: 'production', project: project) } + + describe 'GET #show' do + subject { get project_environment_path(project, environment) } + + before do + sign_in(project.owner) + end + + include_examples 'avoids N+1 queries on environment detail page' + end + + def environment_params(opts = {}) + opts.reverse_merge(namespace_id: project.namespace, + project_id: project, + id: environment.id) + end + + def create_deployment_with_associations(commit_depth:) + commit = project.commit("HEAD~#{commit_depth}") + create(:user, email: commit.author_email) unless User.find_by(email: commit.author_email) + + deployer = create(:user) + pipeline = create(:ci_pipeline, project: environment.project) + build = create(:ci_build, environment: environment.name, pipeline: pipeline, user: deployer) + create(:deployment, :success, environment: environment, deployable: build, user: deployer, + project: project, sha: commit.sha) + end +end diff --git a/spec/requests/projects/issue_links_controller_spec.rb b/spec/requests/projects/issue_links_controller_spec.rb index 3447ff83ed8..81fd1adb1fd 100644 --- a/spec/requests/projects/issue_links_controller_spec.rb +++ b/spec/requests/projects/issue_links_controller_spec.rb @@ -32,7 +32,10 @@ RSpec.describe Projects::IssueLinksController do get namespace_project_issue_links_path(issue_links_params) expect(json_response.count).to eq(1) - expect(json_response.first).to include('path' => project_work_items_path(issue_b.project, issue_b.id)) + expect(json_response.first).to include( + 'path' => project_work_items_path(issue_b.project, issue_b.id), + 'type' => 'TASK' + ) end end end diff --git a/spec/requests/projects/merge_requests_controller_spec.rb b/spec/requests/projects/merge_requests_controller_spec.rb index 3b1ce569033..6580fc8b80f 100644 --- a/spec/requests/projects/merge_requests_controller_spec.rb +++ b/spec/requests/projects/merge_requests_controller_spec.rb @@ -3,6 +3,70 @@ require 'spec_helper' RSpec.describe Projects::MergeRequestsController do + describe 'GET #discussions' do + let_it_be(:merge_request) { create(:merge_request) } + let_it_be(:project) { merge_request.project } + let_it_be(:user) { merge_request.author } + let_it_be(:discussion) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + let_it_be(:discussion_reply) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: discussion) } + let_it_be(:state_event) { create(:resource_state_event, merge_request: merge_request) } + let_it_be(:discussion_2) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + let_it_be(:discussion_3) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) } + + before do + login_as(user) + end + + context 'pagination' do + def get_discussions(**params) + get discussions_project_merge_request_path(project, merge_request, params: params.merge(format: :json)) + end + + it 'returns paginated notes and cursor based on per_page param' do + get_discussions(per_page: 2) + + discussions = Gitlab::Json.parse(response.body) + notes = discussions.flat_map { |d| d['notes'] } + + expect(discussions.count).to eq(2) + expect(notes).to match([ + a_hash_including('id' => discussion.id.to_s), + a_hash_including('id' => discussion_reply.id.to_s), + a_hash_including('type' => 'StateNote') + ]) + + cursor = response.header['X-Next-Page-Cursor'] + expect(cursor).to be_present + + get_discussions(per_page: 1, cursor: cursor) + + discussions = Gitlab::Json.parse(response.body) + notes = discussions.flat_map { |d| d['notes'] } + + expect(discussions.count).to eq(1) + expect(notes).to match([ + a_hash_including('id' => discussion_2.id.to_s) + ]) + end + + context 'when paginated_mr_discussions is disabled' do + before do + stub_feature_flags(paginated_mr_discussions: false) + end + + it 'returns all discussions and ignores per_page param' do + get_discussions(per_page: 2) + + discussions = Gitlab::Json.parse(response.body) + notes = discussions.flat_map { |d| d['notes'] } + + expect(discussions.count).to eq(4) + expect(notes.count).to eq(5) + end + end + end + end + context 'token authentication' do context 'when public project' do let_it_be(:public_project) { create(:project, :public) } diff --git a/spec/requests/pwa_controller_spec.rb b/spec/requests/pwa_controller_spec.rb index f74f37ea9d0..7a295b17231 100644 --- a/spec/requests/pwa_controller_spec.rb +++ b/spec/requests/pwa_controller_spec.rb @@ -3,6 +3,15 @@ require 'spec_helper' RSpec.describe PwaController do + describe 'GET #manifest' do + it 'responds with json' do + get manifest_path(format: :json) + + expect(response.body).to include('The complete DevOps platform.') + expect(response).to have_gitlab_http_status(:success) + end + end + describe 'GET #offline' do it 'responds with static HTML page' do get offline_path diff --git a/spec/requests/robots_txt_spec.rb b/spec/requests/robots_txt_spec.rb index f6c9b018c68..7c0b7d8117a 100644 --- a/spec/requests/robots_txt_spec.rb +++ b/spec/requests/robots_txt_spec.rb @@ -37,6 +37,9 @@ RSpec.describe 'Robots.txt Requests', :aggregate_failures do '/dashboard', '/users', '/users/foo', + '/users/foo@email.com/captcha_check', + '/users/foo/captcha_check', + '/api/v1/users/foo/captcha_check', '/help', '/s/', '/-/profile', |