diff options
Diffstat (limited to 'spec/requests')
54 files changed, 3214 insertions, 1042 deletions
diff --git a/spec/requests/api/api_spec.rb b/spec/requests/api/api_spec.rb index 3cc8764de4a..9fd30213133 100644 --- a/spec/requests/api/api_spec.rb +++ b/spec/requests/api/api_spec.rb @@ -102,6 +102,7 @@ RSpec.describe API::API do Labkit::Context.current.to_h.tap do |log_context| expect(log_context).to match('correlation_id' => an_instance_of(String), 'meta.caller_id' => '/api/:version/projects/:id/issues', + 'meta.remote_ip' => an_instance_of(String), 'meta.project' => project.full_path, 'meta.root_namespace' => project.namespace.full_path, 'meta.user' => user.username, @@ -117,6 +118,7 @@ RSpec.describe API::API do Labkit::Context.current.to_h.tap do |log_context| expect(log_context).to match('correlation_id' => an_instance_of(String), 'meta.caller_id' => '/api/:version/users', + 'meta.remote_ip' => an_instance_of(String), 'meta.feature_category' => 'users') end end diff --git a/spec/requests/api/boards_spec.rb b/spec/requests/api/boards_spec.rb index 36fc6101b84..ca6492396cd 100644 --- a/spec/requests/api/boards_spec.rb +++ b/spec/requests/api/boards_spec.rb @@ -53,17 +53,6 @@ RSpec.describe API::Boards do end end - describe "PUT /projects/:id/boards/:board_id" do - let(:url) { "/projects/#{board_parent.id}/boards/#{board.id}" } - - it 'updates the issue board' do - put api(url, user), params: { name: 'changed board name' } - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['name']).to eq('changed board name') - end - end - describe "DELETE /projects/:id/boards/:board_id" do let(:url) { "/projects/#{board_parent.id}/boards/#{board.id}" } diff --git a/spec/requests/api/ci/runner/jobs_put_spec.rb b/spec/requests/api/ci/runner/jobs_put_spec.rb index e9d793d5a22..f4c99307b1a 100644 --- a/spec/requests/api/ci/runner/jobs_put_spec.rb +++ b/spec/requests/api/ci/runner/jobs_put_spec.rb @@ -78,6 +78,33 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end end + context 'when an exit_code is provided' do + context 'when the exit_codes are acceptable' do + before do + job.options[:allow_failure_criteria] = { exit_codes: [1] } + job.save! + end + + it 'accepts an exit code' do + update_job(state: 'failed', exit_code: 1) + + expect(job.reload).to be_failed + expect(job.allow_failure).to be_truthy + expect(job).to be_unknown_failure + end + end + + context 'when the exit_codes are not defined' do + it 'ignore the exit code' do + update_job(state: 'failed', exit_code: 1) + + expect(job.reload).to be_failed + expect(job.allow_failure).to be_falsy + expect(job).to be_unknown_failure + end + end + end + context 'when failure_reason is script_failure' do before do update_job(state: 'failed', failure_reason: 'script_failure') 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 2dc92417892..74d8e3f7ae8 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -156,7 +156,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do 'sha' => job.sha, 'before_sha' => job.before_sha, 'ref_type' => 'branch', - 'refspecs' => ["+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}", + 'refspecs' => ["+#{pipeline.sha}:refs/pipelines/#{pipeline.id}", "+refs/heads/#{job.ref}:refs/remotes/origin/#{job.ref}"], 'depth' => project.ci_default_git_depth } end @@ -284,7 +284,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(response).to have_gitlab_http_status(:created) expect(json_response['git_info']['refspecs']) - .to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}", + .to contain_exactly("+#{pipeline.sha}:refs/pipelines/#{pipeline.id}", '+refs/tags/*:refs/tags/*', '+refs/heads/*:refs/remotes/origin/*') end @@ -346,7 +346,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(response).to have_gitlab_http_status(:created) expect(json_response['git_info']['refspecs']) - .to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}", + .to contain_exactly("+#{pipeline.sha}:refs/pipelines/#{pipeline.id}", '+refs/tags/*:refs/tags/*', '+refs/heads/*:refs/remotes/origin/*') end diff --git a/spec/requests/api/debian_project_packages_spec.rb b/spec/requests/api/debian_project_packages_spec.rb index d2f208d0079..663b69b1b76 100644 --- a/spec/requests/api/debian_project_packages_spec.rb +++ b/spec/requests/api/debian_project_packages_spec.rb @@ -42,5 +42,12 @@ RSpec.describe API::DebianProjectPackages do it_behaves_like 'Debian project repository PUT endpoint', :created, nil end + + describe 'PUT projects/:id/-/packages/debian/incoming/:file_name/authorize' do + let(:method) { :put } + let(:url) { "/projects/#{project.id}/-/packages/debian/incoming/#{file_name}/authorize" } + + it_behaves_like 'Debian project repository PUT endpoint', :created, nil, is_authorize: true + end end end diff --git a/spec/requests/api/generic_packages_spec.rb b/spec/requests/api/generic_packages_spec.rb index b8e79853486..d162d288129 100644 --- a/spec/requests/api/generic_packages_spec.rb +++ b/spec/requests/api/generic_packages_spec.rb @@ -3,8 +3,16 @@ require 'spec_helper' RSpec.describe API::GenericPackages do + include HttpBasicAuthHelpers + let_it_be(:personal_access_token) { create(:personal_access_token) } let_it_be(:project, reload: true) { create(:project) } + let_it_be(:deploy_token_rw) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } + let_it_be(:project_deploy_token_rw) { create(:project_deploy_token, deploy_token: deploy_token_rw, project: project) } + let_it_be(:deploy_token_ro) { create(:deploy_token, read_package_registry: true, write_package_registry: false) } + let_it_be(:project_deploy_token_ro) { create(:project_deploy_token, deploy_token: deploy_token_ro, project: project) } + let_it_be(:deploy_token_wo) { create(:deploy_token, read_package_registry: false, write_package_registry: true) } + let_it_be(:project_deploy_token_wo) { create(:project_deploy_token, deploy_token: deploy_token_wo, project: project) } let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } let(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } let(:user) { personal_access_token.user } @@ -22,6 +30,23 @@ RSpec.describe API::GenericPackages do personal_access_token_header('wrong token') when :invalid_job_token job_token_header('wrong token') + when :user_basic_auth + user_basic_auth_header(user) + when :invalid_user_basic_auth + basic_auth_header('invalid user', 'invalid password') + end + end + + def deploy_token_auth_header + case authenticate_with + when :deploy_token_rw + deploy_token_header(deploy_token_rw.token) + when :deploy_token_ro + deploy_token_header(deploy_token_ro.token) + when :deploy_token_wo + deploy_token_header(deploy_token_wo.token) + when :invalid_deploy_token + deploy_token_header('wrong token') end end @@ -33,6 +58,10 @@ RSpec.describe API::GenericPackages do { Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER => value || ci_build.token } end + def deploy_token_header(value) + { Gitlab::Auth::AuthFinders::DEPLOY_TOKEN_HEADER => value } + end + shared_examples 'secure endpoint' do before do project.add_developer(user) @@ -54,19 +83,35 @@ RSpec.describe API::GenericPackages do 'PUBLIC' | :guest | true | :personal_access_token | :forbidden 'PUBLIC' | :developer | true | :invalid_personal_access_token | :unauthorized 'PUBLIC' | :guest | true | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :developer | true | :user_basic_auth | :success + 'PUBLIC' | :guest | true | :user_basic_auth | :forbidden + 'PUBLIC' | :developer | true | :invalid_user_basic_auth | :unauthorized + 'PUBLIC' | :guest | true | :invalid_user_basic_auth | :unauthorized 'PUBLIC' | :developer | false | :personal_access_token | :forbidden 'PUBLIC' | :guest | false | :personal_access_token | :forbidden 'PUBLIC' | :developer | false | :invalid_personal_access_token | :unauthorized 'PUBLIC' | :guest | false | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :developer | false | :user_basic_auth | :forbidden + 'PUBLIC' | :guest | false | :user_basic_auth | :forbidden + 'PUBLIC' | :developer | false | :invalid_user_basic_auth | :unauthorized + 'PUBLIC' | :guest | false | :invalid_user_basic_auth | :unauthorized 'PUBLIC' | :anonymous | false | :none | :unauthorized 'PRIVATE' | :developer | true | :personal_access_token | :success 'PRIVATE' | :guest | true | :personal_access_token | :forbidden 'PRIVATE' | :developer | true | :invalid_personal_access_token | :unauthorized 'PRIVATE' | :guest | true | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :developer | true | :user_basic_auth | :success + 'PRIVATE' | :guest | true | :user_basic_auth | :forbidden + 'PRIVATE' | :developer | true | :invalid_user_basic_auth | :unauthorized + 'PRIVATE' | :guest | true | :invalid_user_basic_auth | :unauthorized 'PRIVATE' | :developer | false | :personal_access_token | :not_found 'PRIVATE' | :guest | false | :personal_access_token | :not_found 'PRIVATE' | :developer | false | :invalid_personal_access_token | :unauthorized 'PRIVATE' | :guest | false | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :developer | false | :user_basic_auth | :not_found + 'PRIVATE' | :guest | false | :user_basic_auth | :not_found + 'PRIVATE' | :developer | false | :invalid_user_basic_auth | :unauthorized + 'PRIVATE' | :guest | false | :invalid_user_basic_auth | :unauthorized 'PRIVATE' | :anonymous | false | :none | :unauthorized 'PUBLIC' | :developer | true | :job_token | :success 'PUBLIC' | :developer | true | :invalid_job_token | :unauthorized @@ -90,6 +135,21 @@ RSpec.describe API::GenericPackages do expect(response).to have_gitlab_http_status(expected_status) end end + + where(:authenticate_with, :expected_status) do + :deploy_token_rw | :success + :deploy_token_wo | :success + :deploy_token_ro | :forbidden + :invalid_deploy_token | :unauthorized + end + + with_them do + it "responds with #{params[:expected_status]}" do + authorize_upload_file(workhorse_header.merge(deploy_token_auth_header)) + + expect(response).to have_gitlab_http_status(expected_status) + end + end end context 'application security' do @@ -138,20 +198,34 @@ RSpec.describe API::GenericPackages do where(:project_visibility, :user_role, :member?, :authenticate_with, :expected_status) do 'PUBLIC' | :guest | true | :personal_access_token | :forbidden + 'PUBLIC' | :guest | true | :user_basic_auth | :forbidden 'PUBLIC' | :developer | true | :invalid_personal_access_token | :unauthorized 'PUBLIC' | :guest | true | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :developer | true | :invalid_user_basic_auth | :unauthorized + 'PUBLIC' | :guest | true | :invalid_user_basic_auth | :unauthorized 'PUBLIC' | :developer | false | :personal_access_token | :forbidden 'PUBLIC' | :guest | false | :personal_access_token | :forbidden + 'PUBLIC' | :developer | false | :user_basic_auth | :forbidden + 'PUBLIC' | :guest | false | :user_basic_auth | :forbidden 'PUBLIC' | :developer | false | :invalid_personal_access_token | :unauthorized 'PUBLIC' | :guest | false | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :developer | false | :invalid_user_basic_auth | :unauthorized + 'PUBLIC' | :guest | false | :invalid_user_basic_auth | :unauthorized 'PUBLIC' | :anonymous | false | :none | :unauthorized 'PRIVATE' | :guest | true | :personal_access_token | :forbidden + 'PRIVATE' | :guest | true | :user_basic_auth | :forbidden 'PRIVATE' | :developer | true | :invalid_personal_access_token | :unauthorized 'PRIVATE' | :guest | true | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :developer | true | :invalid_user_basic_auth | :unauthorized + 'PRIVATE' | :guest | true | :invalid_user_basic_auth | :unauthorized 'PRIVATE' | :developer | false | :personal_access_token | :not_found 'PRIVATE' | :guest | false | :personal_access_token | :not_found + 'PRIVATE' | :developer | false | :user_basic_auth | :not_found + 'PRIVATE' | :guest | false | :user_basic_auth | :not_found 'PRIVATE' | :developer | false | :invalid_personal_access_token | :unauthorized 'PRIVATE' | :guest | false | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :developer | false | :invalid_user_basic_auth | :unauthorized + 'PRIVATE' | :guest | false | :invalid_user_basic_auth | :unauthorized 'PRIVATE' | :anonymous | false | :none | :unauthorized 'PUBLIC' | :developer | true | :invalid_job_token | :unauthorized 'PUBLIC' | :developer | false | :job_token | :forbidden @@ -175,6 +249,21 @@ RSpec.describe API::GenericPackages do expect(response).to have_gitlab_http_status(expected_status) end end + + where(:authenticate_with, :expected_status) do + :deploy_token_ro | :forbidden + :invalid_deploy_token | :unauthorized + end + + with_them do + it "responds with #{params[:expected_status]}" do + headers = workhorse_header.merge(deploy_token_auth_header) + + upload_file(params, headers) + + expect(response).to have_gitlab_http_status(expected_status) + end + end end context 'when user can upload packages and has valid credentials' do @@ -182,43 +271,58 @@ RSpec.describe API::GenericPackages do project.add_developer(user) end - it 'creates package and package file when valid personal access token is used' do - headers = workhorse_header.merge(personal_access_token_header) + shared_examples 'creates a package and package file' do + it 'creates a package and package file' do + headers = workhorse_header.merge(auth_header) - expect { upload_file(params, headers) } - .to change { project.packages.generic.count }.by(1) - .and change { Packages::PackageFile.count }.by(1) + expect { upload_file(params, headers) } + .to change { project.packages.generic.count }.by(1) + .and change { Packages::PackageFile.count }.by(1) - aggregate_failures do - expect(response).to have_gitlab_http_status(:created) + aggregate_failures do + expect(response).to have_gitlab_http_status(:created) - package = project.packages.generic.last - expect(package.name).to eq('mypackage') - expect(package.version).to eq('0.0.1') - expect(package.original_build_info).to be_nil + package = project.packages.generic.last + expect(package.name).to eq('mypackage') + expect(package.version).to eq('0.0.1') - package_file = package.package_files.last - expect(package_file.file_name).to eq('myfile.tar.gz') + if should_set_build_info + expect(package.original_build_info.pipeline).to eq(ci_build.pipeline) + else + expect(package.original_build_info).to be_nil + end + + package_file = package.package_files.last + expect(package_file.file_name).to eq('myfile.tar.gz') + end end end - it 'creates package, package file, and package build info when valid job token is used' do - headers = workhorse_header.merge(job_token_header) - - expect { upload_file(params, headers) } - .to change { project.packages.generic.count }.by(1) - .and change { Packages::PackageFile.count }.by(1) + context 'when valid personal access token is used' do + it_behaves_like 'creates a package and package file' do + let(:auth_header) { personal_access_token_header } + let(:should_set_build_info) { false } + end + end - aggregate_failures do - expect(response).to have_gitlab_http_status(:created) + context 'when valid basic auth is used' do + it_behaves_like 'creates a package and package file' do + let(:auth_header) { user_basic_auth_header(user) } + let(:should_set_build_info) { false } + end + end - package = project.packages.generic.last - expect(package.name).to eq('mypackage') - expect(package.version).to eq('0.0.1') - expect(package.original_build_info.pipeline).to eq(ci_build.pipeline) + context 'when valid deploy token is used' do + it_behaves_like 'creates a package and package file' do + let(:auth_header) { deploy_token_header(deploy_token_wo.token) } + let(:should_set_build_info) { false } + end + end - package_file = package.package_files.last - expect(package_file.file_name).to eq('myfile.tar.gz') + context 'when valid job token is used' do + it_behaves_like 'creates a package and package file' do + let(:auth_header) { job_token_header } + let(:should_set_build_info) { true } end end @@ -309,21 +413,37 @@ RSpec.describe API::GenericPackages do where(:project_visibility, :user_role, :member?, :authenticate_with, :expected_status) do 'PUBLIC' | :developer | true | :personal_access_token | :success 'PUBLIC' | :guest | true | :personal_access_token | :success + 'PUBLIC' | :developer | true | :user_basic_auth | :success + 'PUBLIC' | :guest | true | :user_basic_auth | :success 'PUBLIC' | :developer | true | :invalid_personal_access_token | :unauthorized 'PUBLIC' | :guest | true | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :developer | true | :invalid_user_basic_auth | :unauthorized + 'PUBLIC' | :guest | true | :invalid_user_basic_auth | :unauthorized 'PUBLIC' | :developer | false | :personal_access_token | :success 'PUBLIC' | :guest | false | :personal_access_token | :success + 'PUBLIC' | :developer | false | :user_basic_auth | :success + 'PUBLIC' | :guest | false | :user_basic_auth | :success 'PUBLIC' | :developer | false | :invalid_personal_access_token | :unauthorized 'PUBLIC' | :guest | false | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :developer | false | :invalid_user_basic_auth | :unauthorized + 'PUBLIC' | :guest | false | :invalid_user_basic_auth | :unauthorized 'PUBLIC' | :anonymous | false | :none | :unauthorized 'PRIVATE' | :developer | true | :personal_access_token | :success 'PRIVATE' | :guest | true | :personal_access_token | :forbidden + 'PRIVATE' | :developer | true | :user_basic_auth | :success + 'PRIVATE' | :guest | true | :user_basic_auth | :forbidden 'PRIVATE' | :developer | true | :invalid_personal_access_token | :unauthorized 'PRIVATE' | :guest | true | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :developer | true | :invalid_user_basic_auth | :unauthorized + 'PRIVATE' | :guest | true | :invalid_user_basic_auth | :unauthorized 'PRIVATE' | :developer | false | :personal_access_token | :not_found 'PRIVATE' | :guest | false | :personal_access_token | :not_found + 'PRIVATE' | :developer | false | :user_basic_auth | :not_found + 'PRIVATE' | :guest | false | :user_basic_auth | :not_found 'PRIVATE' | :developer | false | :invalid_personal_access_token | :unauthorized 'PRIVATE' | :guest | false | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :developer | false | :invalid_user_basic_auth | :unauthorized + 'PRIVATE' | :guest | false | :invalid_user_basic_auth | :unauthorized 'PRIVATE' | :anonymous | false | :none | :unauthorized 'PUBLIC' | :developer | true | :job_token | :success 'PUBLIC' | :developer | true | :invalid_job_token | :unauthorized @@ -347,6 +467,21 @@ RSpec.describe API::GenericPackages do expect(response).to have_gitlab_http_status(expected_status) end end + + where(:authenticate_with, :expected_status) do + :deploy_token_rw | :success + :deploy_token_wo | :success + :deploy_token_ro | :success + :invalid_deploy_token | :unauthorized + end + + with_them do + it "responds with #{params[:expected_status]}" do + download_file(deploy_token_auth_header) + + expect(response).to have_gitlab_http_status(expected_status) + end + end end context 'event tracking' do diff --git a/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb b/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb index e086ce02942..db8a412e45c 100644 --- a/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb +++ b/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'Getting Ci Cd Setting' do let(:fields) do <<~QUERY - #{all_graphql_fields_for('ProjectCiCdSetting')} + #{all_graphql_fields_for('ProjectCiCdSetting', max_depth: 1)} QUERY end @@ -43,8 +43,10 @@ RSpec.describe 'Getting Ci Cd Setting' do it_behaves_like 'a working graphql query' - specify { expect(settings_data['mergePipelinesEnabled']).to eql project.ci_cd_settings.merge_pipelines_enabled? } - specify { expect(settings_data['mergeTrainsEnabled']).to eql project.ci_cd_settings.merge_trains_enabled? } - specify { expect(settings_data['project']['id']).to eql "gid://gitlab/Project/#{project.id}" } + it 'fetches the settings data' do + expect(settings_data['mergePipelinesEnabled']).to eql project.ci_cd_settings.merge_pipelines_enabled? + expect(settings_data['mergeTrainsEnabled']).to eql project.ci_cd_settings.merge_trains_enabled? + expect(settings_data['keepLatestArtifact']).to eql project.ci_keep_latest_artifact? + end end end diff --git a/spec/requests/api/graphql/ci/config_spec.rb b/spec/requests/api/graphql/ci/config_spec.rb index b682470e0a1..8ede6e1538c 100644 --- a/spec/requests/api/graphql/ci/config_spec.rb +++ b/spec/requests/api/graphql/ci/config_spec.rb @@ -7,7 +7,8 @@ RSpec.describe 'Query.ciConfig' do subject(:post_graphql_query) { post_graphql(query, current_user: user) } - let(:user) { create(:user) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository, creator: user, namespace: user.namespace) } let_it_be(:content) do File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci_includes.yml')) @@ -16,20 +17,41 @@ RSpec.describe 'Query.ciConfig' do let(:query) do %( query { - ciConfig(content: "#{content}") { + ciConfig(projectPath: "#{project.full_path}", content: "#{content}", dryRun: false) { status errors stages { - name - groups { + nodes { name - size - jobs { - name - groupName - stage - needs { + groups { + nodes { name + size + jobs { + nodes { + name + groupName + stage + script + beforeScript + afterScript + allowFailure + only { + refs + } + when + except { + refs + } + environment + tags + needs { + nodes { + name + } + } + } + } } } } @@ -39,53 +61,259 @@ RSpec.describe 'Query.ciConfig' do ) end - before do - post_graphql_query + it_behaves_like 'a working graphql query' do + before do + post_graphql_query + end end - it_behaves_like 'a working graphql query' - it 'returns the correct structure' do + post_graphql_query + expect(graphql_data['ciConfig']).to eq( "status" => "VALID", "errors" => [], "stages" => - [ - { - "name" => "build", - "groups" => - [ + { + "nodes" => + [ + { + "name" => "build", + "groups" => { - "name" => "rspec", - "size" => 2, - "jobs" => + "nodes" => [ - { "name" => "rspec 0 1", "groupName" => "rspec", "stage" => "build", "needs" => [] }, - { "name" => "rspec 0 2", "groupName" => "rspec", "stage" => "build", "needs" => [] } + { + "name" => "rspec", + "size" => 2, + "jobs" => + { + "nodes" => + [ + { + "name" => "rspec 0 1", + "groupName" => "rspec", + "stage" => "build", + "script" => ["rake spec"], + "beforeScript" => ["bundle install", "bundle exec rake db:create"], + "afterScript" => ["echo 'run this after'"], + "allowFailure" => false, + "only" => { "refs" => %w[branches master] }, + "when" => "on_success", + "except" => nil, + "environment" => nil, + "tags" => %w[ruby postgres], + "needs" => { "nodes" => [] } + }, + { + "name" => "rspec 0 2", + "groupName" => "rspec", + "stage" => "build", + "script" => ["rake spec"], + "beforeScript" => ["bundle install", "bundle exec rake db:create"], + "afterScript" => ["echo 'run this after'"], + "allowFailure" => true, + "only" => { "refs" => %w[branches tags] }, + "when" => "on_failure", + "except" => nil, + "environment" => nil, + "tags" => [], + "needs" => { "nodes" => [] } + } + ] + } + }, + { + "name" => "spinach", "size" => 1, "jobs" => + { + "nodes" => + [ + { + "name" => "spinach", + "groupName" => "spinach", + "stage" => "build", + "script" => ["rake spinach"], + "beforeScript" => ["bundle install", "bundle exec rake db:create"], + "afterScript" => ["echo 'run this after'"], + "allowFailure" => false, + "only" => { "refs" => %w[branches tags] }, + "when" => "on_success", + "except" => { "refs" => ["tags"] }, + "environment" => nil, + "tags" => [], + "needs" => { "nodes" => [] } + } + ] + } + } ] - }, + } + }, + { + "name" => "test", + "groups" => { - "name" => "spinach", "size" => 1, "jobs" => + "nodes" => [ - { "name" => "spinach", "groupName" => "spinach", "stage" => "build", "needs" => [] } + { + "name" => "docker", + "size" => 1, + "jobs" => + { + "nodes" => [ + { + "name" => "docker", + "groupName" => "docker", + "stage" => "test", + "script" => ["curl http://dockerhub/URL"], + "beforeScript" => ["bundle install", "bundle exec rake db:create"], + "afterScript" => ["echo 'run this after'"], + "allowFailure" => true, + "only" => { "refs" => %w[branches tags] }, + "when" => "manual", + "except" => { "refs" => ["branches"] }, + "environment" => nil, + "tags" => [], + "needs" => { "nodes" => [{ "name" => "spinach" }, { "name" => "rspec 0 1" }] } + } + ] + } + } ] } - ] - }, + }, + { + "name" => "deploy", + "groups" => + { + "nodes" => + [ + { + "name" => "deploy_job", + "size" => 1, + "jobs" => + { + "nodes" => [ + { + "name" => "deploy_job", + "groupName" => "deploy_job", + "stage" => "deploy", + "script" => ["echo 'done'"], + "beforeScript" => ["bundle install", "bundle exec rake db:create"], + "afterScript" => ["echo 'run this after'"], + "allowFailure" => false, + "only" => { "refs" => %w[branches tags] }, + "when" => "on_success", + "except" => nil, + "environment" => "production", + "tags" => [], + "needs" => { "nodes" => [] } + } + ] + } + } + ] + } + } + ] + } + ) + end + + context 'when the config file includes other files' do + let_it_be(:content) do + YAML.dump( + include: 'other_file.yml', + rspec: { + script: 'rspec' + } + ) + end + + before do + allow_next_instance_of(Repository) do |repository| + allow(repository).to receive(:blob_data_at).with(an_instance_of(String), 'other_file.yml') do + YAML.dump( + build: { + script: 'build' + } + ) + end + end + + post_graphql_query + end + + it_behaves_like 'a working graphql query' + + it 'returns the correct structure with included files' do + expect(graphql_data['ciConfig']).to eq( + "status" => "VALID", + "errors" => [], + "stages" => { - "name" => "test", - "groups" => + "nodes" => [ { - "name" => "docker", - "size" => 1, - "jobs" => [ - { "name" => "docker", "groupName" => "docker", "stage" => "test", "needs" => [{ "name" => "spinach" }, { "name" => "rspec 0 1" }] } - ] + "name" => "test", + "groups" => + { + "nodes" => + [ + { + "name" => "build", + "size" => 1, + "jobs" => + { + "nodes" => + [ + { + "name" => "build", + "stage" => "test", + "groupName" => "build", + "script" => ["build"], + "afterScript" => [], + "beforeScript" => [], + "allowFailure" => false, + "environment" => nil, + "except" => nil, + "only" => { "refs" => %w[branches tags] }, + "when" => "on_success", + "tags" => [], + "needs" => { "nodes" => [] } +} + ] + } + }, + { + "name" => "rspec", + "size" => 1, + "jobs" => + { + "nodes" => + [ + { "name" => "rspec", + "stage" => "test", + "groupName" => "rspec", + "script" => ["rspec"], + "afterScript" => [], + "beforeScript" => [], + "allowFailure" => false, + "environment" => nil, + "except" => nil, + "only" => { "refs" => %w[branches tags] }, + "when" => "on_success", + "tags" => [], + "needs" => { "nodes" => [] } } + ] + } + } + ] + } } ] } - ] - ) + ) + end end end diff --git a/spec/requests/api/graphql/ci/jobs_spec.rb b/spec/requests/api/graphql/ci/jobs_spec.rb index 19954c4e52f..3fb89d6e815 100644 --- a/spec/requests/api/graphql/ci/jobs_spec.rb +++ b/spec/requests/api/graphql/ci/jobs_spec.rb @@ -7,48 +7,74 @@ RSpec.describe 'Query.project.pipeline' do let_it_be(:project) { create(:project, :repository, :public) } let_it_be(:user) { create(:user) } - def first(field) - [field.pluralize, 'nodes', 0] + def all(*fields) + fields.flat_map { |f| [f, :nodes] } end describe '.stages.groups.jobs' do let(:pipeline) do pipeline = create(:ci_pipeline, project: project, user: user) - stage = create(:ci_stage_entity, pipeline: pipeline, name: 'first') - create(:commit_status, stage_id: stage.id, pipeline: pipeline, name: 'my test job') + stage = create(:ci_stage_entity, project: project, pipeline: pipeline, name: 'first') + create(:ci_build, stage_id: stage.id, pipeline: pipeline, name: 'my test job') pipeline end - let(:jobs_graphql_data) { graphql_data.dig(*%w[project pipeline], *first('stage'), *first('group'), 'jobs', 'nodes') } + let(:jobs_graphql_data) { graphql_data_at(:project, :pipeline, *all(:stages, :groups, :jobs)) } + + let(:first_n) { var('Int') } let(:query) do - %( - query { - project(fullPath: "#{project.full_path}") { - pipeline(iid: "#{pipeline.iid}") { - stages { - nodes { - name - groups { - nodes { - name - jobs { - nodes { - name - pipeline { - id - } - } - } - } - } + with_signature([first_n], wrap_fields(query_graphql_path([ + [:project, { full_path: project.full_path }], + [:pipeline, { iid: pipeline.iid.to_s }], + [:stages, { first: first_n }] + ], stage_fields))) + end + + let(:stage_fields) do + <<~FIELDS + nodes { + name + groups { + nodes { + name + jobs { + nodes { + name + needs { + nodes { #{all_graphql_fields_for('CiBuildNeed')} } + } + pipeline { + id } } } } } - ) + } + FIELDS + end + + context 'when there are build needs' do + before do + pipeline.statuses.each do |build| + create_list(:ci_build_need, 2, build: build) + end + end + + it 'reports the build needs' do + post_graphql(query, current_user: user) + + expect(jobs_graphql_data).to contain_exactly a_hash_including( + 'needs' => a_hash_including( + 'nodes' => contain_exactly( + a_hash_including('name' => String), + a_hash_including('name' => String) + ) + ) + ) + end end it 'returns the jobs of a pipeline stage' do @@ -57,60 +83,43 @@ RSpec.describe 'Query.project.pipeline' do expect(jobs_graphql_data).to contain_exactly(a_hash_including('name' => 'my test job')) end - it 'avoids N+1 queries', :aggregate_failures do - control_count = ActiveRecord::QueryRecorder.new do - post_graphql(query, current_user: user) + describe 'performance' do + before do + build_stage = create(:ci_stage_entity, position: 2, name: 'build', project: project, pipeline: pipeline) + test_stage = create(:ci_stage_entity, position: 3, name: 'test', project: project, pipeline: pipeline) + create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 1 2') + create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 2 2') + create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 1 2') + create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 2 2') end - build_stage = create(:ci_stage_entity, name: 'build', pipeline: pipeline) - test_stage = create(:ci_stage_entity, name: 'test', pipeline: pipeline) - create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 1 2') - create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 2 2') - create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 1 2') - create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 2 2') + it 'can find the first stage' do + post_graphql(query, current_user: user, variables: first_n.with(1)) - expect do - post_graphql(query, current_user: user) - end.not_to exceed_query_limit(control_count) + expect(jobs_graphql_data).to contain_exactly(a_hash_including('name' => 'my test job')) + end - expect(response).to have_gitlab_http_status(:ok) + it 'can find all stages' do + post_graphql(query, current_user: user, variables: first_n.with(3)) - build_stage = graphql_data.dig('project', 'pipeline', 'stages', 'nodes').find do |stage| - stage['name'] == 'build' + expect(jobs_graphql_data).to contain_exactly( + a_hash_including('name' => 'my test job'), + a_hash_including('name' => 'docker 1 2'), + a_hash_including('name' => 'docker 2 2'), + a_hash_including('name' => 'rspec 1 2'), + a_hash_including('name' => 'rspec 2 2') + ) end - test_stage = graphql_data.dig('project', 'pipeline', 'stages', 'nodes').find do |stage| - stage['name'] == 'test' + + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new do + post_graphql(query, current_user: user, variables: first_n.with(1)) + end + + expect do + post_graphql(query, current_user: user, variables: first_n.with(3)) + end.not_to exceed_query_limit(control_count) end - docker_group = build_stage.dig('groups', 'nodes').first - rspec_group = test_stage.dig('groups', 'nodes').first - - expect(docker_group['name']).to eq('docker') - expect(rspec_group['name']).to eq('rspec') - - docker_jobs = docker_group.dig('jobs', 'nodes') - rspec_jobs = rspec_group.dig('jobs', 'nodes') - - expect(docker_jobs).to eq([ - { - 'name' => 'docker 1 2', - 'pipeline' => { 'id' => pipeline.to_global_id.to_s } - }, - { - 'name' => 'docker 2 2', - 'pipeline' => { 'id' => pipeline.to_global_id.to_s } - } - ]) - - expect(rspec_jobs).to eq([ - { - 'name' => 'rspec 1 2', - 'pipeline' => { 'id' => pipeline.to_global_id.to_s } - }, - { - 'name' => 'rspec 2 2', - 'pipeline' => { 'id' => pipeline.to_global_id.to_s } - } - ]) end end diff --git a/spec/requests/api/graphql/ci/pipelines_spec.rb b/spec/requests/api/graphql/ci/pipelines_spec.rb index 414ddabbac9..7933251b8e9 100644 --- a/spec/requests/api/graphql/ci/pipelines_spec.rb +++ b/spec/requests/api/graphql/ci/pipelines_spec.rb @@ -6,53 +6,59 @@ RSpec.describe 'Query.project(fullPath).pipelines' do include GraphqlHelpers let_it_be(:project) { create(:project, :repository, :public) } - let_it_be(:first_user) { create(:user) } - let_it_be(:second_user) { create(:user) } + let_it_be(:user) { create(:user) } describe '.jobs' do - let_it_be(:query) do - %( - query { - project(fullPath: "#{project.full_path}") { - pipelines { - nodes { - jobs { - nodes { - name - } - } - } - } - } - } - ) + let(:first_n) { var('Int') } + let(:query_path) do + [ + [:project, { full_path: project.full_path }], + [:pipelines, { first: first_n }], + [:nodes], + [:jobs], + [:nodes] + ] end - it 'fetches the jobs without an N+1' do + let(:query) do + with_signature([first_n], wrap_fields(query_graphql_path(query_path, :name))) + end + + before_all do pipeline = create(:ci_pipeline, project: project) create(:ci_build, pipeline: pipeline, name: 'Job 1') - - control_count = ActiveRecord::QueryRecorder.new do - post_graphql(query, current_user: first_user) - end - pipeline = create(:ci_pipeline, project: project) create(:ci_build, pipeline: pipeline, name: 'Job 2') + end - expect do - post_graphql(query, current_user: second_user) - end.not_to exceed_query_limit(control_count) + it 'limits the results' do + post_graphql(query, current_user: user, variables: first_n.with(1)) - expect(response).to have_gitlab_http_status(:ok) + expect(graphql_data_at(*query_path.map(&:first))).to contain_exactly a_hash_including( + 'name' => 'Job 2' + ) + end - pipelines_data = graphql_data.dig('project', 'pipelines', 'nodes') + it 'fetches all results' do + post_graphql(query, current_user: user) - job_names = pipelines_data.map do |pipeline_data| - jobs_data = pipeline_data.dig('jobs', 'nodes') - jobs_data.map { |job_data| job_data['name'] } - end.flatten + expect(graphql_data_at(*query_path.map(&:first))).to contain_exactly( + a_hash_including('name' => 'Job 1'), + a_hash_including('name' => 'Job 2') + ) + end + + it 'fetches the jobs without an N+1' do + first_user = create(:personal_access_token).user + second_user = create(:personal_access_token).user + + control_count = ActiveRecord::QueryRecorder.new do + post_graphql(query, current_user: first_user, variables: first_n.with(1)) + end - expect(job_names).to contain_exactly('Job 1', 'Job 2') + expect do + post_graphql(query, current_user: second_user) + end.not_to exceed_query_limit(control_count) end end @@ -80,7 +86,7 @@ RSpec.describe 'Query.project(fullPath).pipelines' do create(:ci_build, :dast, name: 'DAST Job 1', pipeline: pipeline) create(:ci_build, :sast, name: 'SAST Job 1', pipeline: pipeline) - post_graphql(query, current_user: first_user) + post_graphql(query, current_user: user) expect(response).to have_gitlab_http_status(:ok) @@ -96,9 +102,9 @@ RSpec.describe 'Query.project(fullPath).pipelines' do end describe 'upstream' do - let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: first_user) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } let_it_be(:upstream_project) { create(:project, :repository, :public) } - let_it_be(:upstream_pipeline) { create(:ci_pipeline, project: upstream_project, user: first_user) } + let_it_be(:upstream_pipeline) { create(:ci_pipeline, project: upstream_project, user: user) } let(:upstream_pipelines_graphql_data) { graphql_data.dig(*%w[project pipelines nodes]).first['upstream'] } let(:query) do @@ -120,7 +126,7 @@ RSpec.describe 'Query.project(fullPath).pipelines' do before do create(:ci_sources_pipeline, source_pipeline: upstream_pipeline, pipeline: pipeline ) - post_graphql(query, current_user: first_user) + post_graphql(query, current_user: user) end it_behaves_like 'a working graphql query' @@ -131,15 +137,18 @@ RSpec.describe 'Query.project(fullPath).pipelines' do context 'when fetching the upstream pipeline from the pipeline' do it 'avoids N+1 queries' do + first_user = create(:user) + second_user = create(:user) + control_count = ActiveRecord::QueryRecorder.new do post_graphql(query, current_user: first_user) end - pipeline_2 = create(:ci_pipeline, project: project, user: first_user) - upstream_pipeline_2 = create(:ci_pipeline, project: upstream_project, user: first_user) + pipeline_2 = create(:ci_pipeline, project: project, user: user) + upstream_pipeline_2 = create(:ci_pipeline, project: upstream_project, user: user) create(:ci_sources_pipeline, source_pipeline: upstream_pipeline_2, pipeline: pipeline_2 ) - pipeline_3 = create(:ci_pipeline, project: project, user: first_user) - upstream_pipeline_3 = create(:ci_pipeline, project: upstream_project, user: first_user) + pipeline_3 = create(:ci_pipeline, project: project, user: user) + upstream_pipeline_3 = create(:ci_pipeline, project: upstream_project, user: user) create(:ci_sources_pipeline, source_pipeline: upstream_pipeline_3, pipeline: pipeline_3 ) expect do @@ -152,12 +161,12 @@ RSpec.describe 'Query.project(fullPath).pipelines' do end describe 'downstream' do - let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: first_user) } - let(:pipeline_2) { create(:ci_pipeline, project: project, user: first_user) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } + let(:pipeline_2) { create(:ci_pipeline, project: project, user: user) } let_it_be(:downstream_project) { create(:project, :repository, :public) } - let_it_be(:downstream_pipeline_a) { create(:ci_pipeline, project: downstream_project, user: first_user) } - let_it_be(:downstream_pipeline_b) { create(:ci_pipeline, project: downstream_project, user: first_user) } + let_it_be(:downstream_pipeline_a) { create(:ci_pipeline, project: downstream_project, user: user) } + let_it_be(:downstream_pipeline_b) { create(:ci_pipeline, project: downstream_project, user: user) } let(:pipelines_graphql_data) { graphql_data.dig(*%w[project pipelines nodes]) } @@ -183,7 +192,7 @@ RSpec.describe 'Query.project(fullPath).pipelines' do create(:ci_sources_pipeline, source_pipeline: pipeline, pipeline: downstream_pipeline_a) create(:ci_sources_pipeline, source_pipeline: pipeline_2, pipeline: downstream_pipeline_b) - post_graphql(query, current_user: first_user) + post_graphql(query, current_user: user) end it_behaves_like 'a working graphql query' @@ -198,16 +207,19 @@ RSpec.describe 'Query.project(fullPath).pipelines' do context 'when fetching the downstream pipelines from the pipeline' do it 'avoids N+1 queries' do + first_user = create(:user) + second_user = create(:user) + control_count = ActiveRecord::QueryRecorder.new do post_graphql(query, current_user: first_user) end - downstream_pipeline_2a = create(:ci_pipeline, project: downstream_project, user: first_user) + downstream_pipeline_2a = create(:ci_pipeline, project: downstream_project, user: user) create(:ci_sources_pipeline, source_pipeline: pipeline, pipeline: downstream_pipeline_2a) - downsteam_pipeline_3a = create(:ci_pipeline, project: downstream_project, user: first_user) + downsteam_pipeline_3a = create(:ci_pipeline, project: downstream_project, user: user) create(:ci_sources_pipeline, source_pipeline: pipeline, pipeline: downsteam_pipeline_3a) - downstream_pipeline_2b = create(:ci_pipeline, project: downstream_project, user: first_user) + downstream_pipeline_2b = create(:ci_pipeline, project: downstream_project, user: user) create(:ci_sources_pipeline, source_pipeline: pipeline_2, pipeline: downstream_pipeline_2b) downsteam_pipeline_3b = create(:ci_pipeline, project: downstream_project, user: first_user) create(:ci_sources_pipeline, source_pipeline: pipeline_2, pipeline: downsteam_pipeline_3b) diff --git a/spec/requests/api/graphql/group/group_members_spec.rb b/spec/requests/api/graphql/group/group_members_spec.rb index 3554e22cdf2..452610ab18f 100644 --- a/spec/requests/api/graphql/group/group_members_spec.rb +++ b/spec/requests/api/graphql/group/group_members_spec.rb @@ -10,15 +10,13 @@ RSpec.describe 'getting group members information' do let_it_be(:user_1) { create(:user, username: 'user') } let_it_be(:user_2) { create(:user, username: 'test') } - let(:member_data) { graphql_data['group']['groupMembers']['edges'] } - before_all do [user_1, user_2].each { |user| parent_group.add_guest(user) } end context 'when the request is correct' do it_behaves_like 'a working graphql query' do - before_all do + before do fetch_members end end @@ -80,12 +78,10 @@ RSpec.describe 'getting group members information' do end context 'when unauthenticated' do - it 'returns nothing' do + it 'returns visible members' do fetch_members(current_user: nil) - expect(graphql_errors).to be_nil - expect(response).to have_gitlab_http_status(:success) - expect(member_data).to be_empty + expect_array_response(user_1, user_2) end end @@ -112,8 +108,8 @@ RSpec.describe 'getting group members information' do def expect_array_response(*items) expect(response).to have_gitlab_http_status(:success) - expect(member_data).to be_an Array - expect(member_data.map { |node| node["node"]["user"]["id"] }) - .to match_array(items.map { |u| global_id_of(u) }) + member_gids = graphql_data_at(:group, :group_members, :edges, :node, :user, :id) + + expect(member_gids).to match_array(items.map { |u| global_id_of(u) }) end end diff --git a/spec/requests/api/graphql/mutations/alert_management/alerts/todo/create_spec.rb b/spec/requests/api/graphql/mutations/alert_management/alerts/todo/create_spec.rb index e5803f50474..cd423d7764a 100644 --- a/spec/requests/api/graphql/mutations/alert_management/alerts/todo/create_spec.rb +++ b/spec/requests/api/graphql/mutations/alert_management/alerts/todo/create_spec.rb @@ -42,6 +42,8 @@ RSpec.describe 'Creating a todo for the alert' do context 'todo already exists' do before do + stub_feature_flags(multiple_todos: false) + create(:todo, :pending, project: project, user: user, target: alert) end diff --git a/spec/requests/api/graphql/mutations/alert_management/http_integration/create_spec.rb b/spec/requests/api/graphql/mutations/alert_management/http_integration/create_spec.rb index a285cebc805..e594d67aab4 100644 --- a/spec/requests/api/graphql/mutations/alert_management/http_integration/create_spec.rb +++ b/spec/requests/api/graphql/mutations/alert_management/http_integration/create_spec.rb @@ -39,21 +39,7 @@ RSpec.describe 'Creating a new HTTP Integration' do project.add_maintainer(current_user) end - it 'creates a new integration' do - post_graphql_mutation(mutation, current_user: current_user) - - new_integration = ::AlertManagement::HttpIntegration.last! - integration_response = mutation_response['integration'] - - expect(response).to have_gitlab_http_status(:success) - expect(integration_response['id']).to eq(GitlabSchema.id_from_object(new_integration).to_s) - expect(integration_response['type']).to eq('HTTP') - expect(integration_response['name']).to eq(new_integration.name) - expect(integration_response['active']).to eq(new_integration.active) - expect(integration_response['token']).to eq(new_integration.token) - expect(integration_response['url']).to eq(new_integration.url) - expect(integration_response['apiUrl']).to eq(nil) - end + it_behaves_like 'creating a new HTTP integration' [:project_path, :active, :name].each do |argument| context "without required argument #{argument}" do diff --git a/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb b/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb new file mode 100644 index 00000000000..283badeaf33 --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'CiCdSettingsUpdate' do + include GraphqlHelpers + + let_it_be(:project) { create(:project, ci_keep_latest_artifact: true) } + let(:variables) { { full_path: project.full_path, keep_latest_artifact: false } } + let(:mutation) { graphql_mutation(:ci_cd_settings_update, variables) } + + context 'when unauthorized' do + let(:user) { create(:user) } + + shared_examples 'unauthorized' do + it 'returns an error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).not_to be_empty + end + end + + context 'when not a project member' do + it_behaves_like 'unauthorized' + end + + context 'when a non-admin project member' do + before do + project.add_developer(user) + end + + it_behaves_like 'unauthorized' + end + end + + context 'when authorized' do + let_it_be(:user) { project.owner } + + it 'updates ci cd settings' do + post_graphql_mutation(mutation, current_user: user) + + project.reload + + expect(response).to have_gitlab_http_status(:success) + expect(project.ci_keep_latest_artifact).to eq(false) + end + + context 'when bad arguments are provided' do + let(:variables) { { full_path: '', keep_latest_artifact: false } } + + it 'returns the errors' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).not_to be_empty + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb new file mode 100644 index 00000000000..749373e7b8d --- /dev/null +++ b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Updating the package settings' do + include GraphqlHelpers + using RSpec::Parameterized::TableSyntax + + let_it_be(:user) { create(:user) } + + let(:params) do + { + namespace_path: namespace.full_path, + maven_duplicates_allowed: false, + maven_duplicate_exception_regex: 'foo-.*' + } + end + + let(:mutation) do + graphql_mutation(:update_namespace_package_settings, params) do + <<~QL + packageSettings { + mavenDuplicatesAllowed + mavenDuplicateExceptionRegex + } + errors + QL + end + end + + let(:mutation_response) { graphql_mutation_response(:update_namespace_package_settings) } + let(:package_settings_response) { mutation_response['packageSettings'] } + + RSpec.shared_examples 'returning a success' do + it_behaves_like 'returning response status', :success + + it 'returns the updated package settings', :aggregate_failures do + subject + + expect(mutation_response['errors']).to be_empty + expect(package_settings_response['mavenDuplicatesAllowed']).to eq(params[:maven_duplicates_allowed]) + expect(package_settings_response['mavenDuplicateExceptionRegex']).to eq(params[:maven_duplicate_exception_regex]) + end + end + + RSpec.shared_examples 'rejecting invalid regex' do + context "for field mavenDuplicateExceptionRegex" do + let_it_be(:invalid_regex) { '][' } + + let(:params) do + { + :namespace_path => namespace.full_path, + 'mavenDuplicateExceptionRegex' => invalid_regex + } + end + + it_behaves_like 'returning response status', :success + + it_behaves_like 'not creating the namespace package setting' + + it 'returns an error', :aggregate_failures do + subject + + expect(graphql_errors.size).to eq(1) + expect(graphql_errors.first['message']).to include("#{invalid_regex} is an invalid regexp") + end + end + end + + RSpec.shared_examples 'accepting the mutation request updating the package settings' do + it_behaves_like 'updating the namespace package setting attributes', + from: { maven_duplicates_allowed: true, maven_duplicate_exception_regex: 'SNAPSHOT' }, + to: { maven_duplicates_allowed: false, maven_duplicate_exception_regex: 'foo-.*' } + + it_behaves_like 'returning a success' + it_behaves_like 'rejecting invalid regex' + end + + RSpec.shared_examples 'accepting the mutation request creating the package settings' do + it_behaves_like 'creating the namespace package setting' + it_behaves_like 'returning a success' + it_behaves_like 'rejecting invalid regex' + end + + RSpec.shared_examples 'denying the mutation request' do + it_behaves_like 'not creating the namespace package setting' + + it_behaves_like 'returning response status', :success + + it 'returns no response' do + subject + + expect(mutation_response).to be_nil + end + end + + describe 'post graphql mutation' do + subject { post_graphql_mutation(mutation, current_user: user) } + + context 'with existing package settings' do + let_it_be(:package_settings, reload: true) { create(:namespace_package_setting, :group) } + let_it_be(:namespace, reload: true) { package_settings.namespace } + + where(:user_role, :shared_examples_name) do + :maintainer | 'accepting the mutation request updating the package settings' + :developer | 'accepting the mutation request updating the package settings' + :reporter | 'denying the mutation request' + :guest | 'denying the mutation request' + :anonymous | 'denying the mutation request' + end + + with_them do + before do + namespace.send("add_#{user_role}", user) unless user_role == :anonymous + end + + it_behaves_like params[:shared_examples_name] + end + end + + context 'without existing package settings' do + let_it_be(:namespace, reload: true) { create(:group) } + let(:package_settings) { namespace.package_settings } + + where(:user_role, :shared_examples_name) do + :maintainer | 'accepting the mutation request creating the package settings' + :developer | 'accepting the mutation request creating the package settings' + :reporter | 'denying the mutation request' + :guest | 'denying the mutation request' + :anonymous | 'denying the mutation request' + end + + with_them do + before do + namespace.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/releases/create_spec.rb b/spec/requests/api/graphql/mutations/releases/create_spec.rb index d745eb3083d..79bdcec7944 100644 --- a/spec/requests/api/graphql/mutations/releases/create_spec.rb +++ b/spec/requests/api/graphql/mutations/releases/create_spec.rb @@ -116,11 +116,9 @@ RSpec.describe 'Creation of a new release' do context 'when all available mutation arguments are provided' do it_behaves_like 'no errors' - # rubocop: disable CodeReuse/ActiveRecord it 'returns the new release data' do create_release - release = mutation_response[:release] expected_direct_asset_url = Gitlab::Routing.url_helpers.project_release_url(project, Release.find_by(tag: tag_name)) << "/downloads#{asset_link[:directAssetPath]}" expected_attributes = { @@ -139,21 +137,17 @@ RSpec.describe 'Creation of a new release' do directAssetUrl: expected_direct_asset_url }] } + }, + milestones: { + nodes: [ + { title: '12.3' }, + { title: '12.4' } + ] } - } - - expect(release).to include(expected_attributes) + }.with_indifferent_access - # Right now the milestones are returned in a non-deterministic order. - # This `milestones` test should be moved up into the expect(release) - # above (and `.to include` updated to `.to eq`) once - # https://gitlab.com/gitlab-org/gitlab/-/issues/259012 is addressed. - expect(release['milestones']['nodes']).to match_array([ - { 'title' => '12.4' }, - { 'title' => '12.3' } - ]) + expect(mutation_response[:release]).to eq(expected_attributes) end - # rubocop: enable CodeReuse/ActiveRecord end context 'when only the required mutation arguments are provided' do diff --git a/spec/requests/api/graphql/mutations/releases/update_spec.rb b/spec/requests/api/graphql/mutations/releases/update_spec.rb index 19320c3393c..c9a6c3abd57 100644 --- a/spec/requests/api/graphql/mutations/releases/update_spec.rb +++ b/spec/requests/api/graphql/mutations/releases/update_spec.rb @@ -116,15 +116,7 @@ RSpec.describe 'Updating an existing release' do it 'updates the correct field and returns the release' do update_release - expect(mutation_response[:release]).to include(expected_attributes.merge(updates).except(:milestones)) - - # Right now the milestones are returned in a non-deterministic order. - # Because of this, we need to test milestones separately to allow - # for them to be returned in any order. - # Once https://gitlab.com/gitlab-org/gitlab/-/issues/259012 has been - # fixed, this special milestone handling can be removed. - expected_milestones = expected_attributes.merge(updates)[:milestones] - expect(mutation_response[:release][:milestones][:nodes]).to match_array(expected_milestones[:nodes]) + expect(mutation_response[:release]).to eq(expected_attributes.merge(updates)) end end diff --git a/spec/requests/api/graphql/namespace/package_settings_spec.rb b/spec/requests/api/graphql/namespace/package_settings_spec.rb new file mode 100644 index 00000000000..6af098e902f --- /dev/null +++ b/spec/requests/api/graphql/namespace/package_settings_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting namespace package settings in a namespace' do + include GraphqlHelpers + + let_it_be(:package_settings) { create(:namespace_package_setting) } + let_it_be(:namespace) { package_settings.namespace } + let_it_be(:current_user) { namespace.owner } + let(:package_settings_response) { graphql_data.dig('namespace', 'packageSettings') } + let(:fields) { all_graphql_fields_for('PackageSettings') } + + let(:query) do + graphql_query_for( + 'namespace', + { 'fullPath' => namespace.full_path }, + query_graphql_field('package_settings', {}, fields) + ) + end + + subject { post_graphql(query, current_user: current_user) } + + it_behaves_like 'a working graphql query' do + before do + subject + end + + it 'matches the JSON schema' do + expect(package_settings_response).to match_schema('graphql/namespace/package_settings') + end + end +end diff --git a/spec/requests/api/graphql/packages/package_composer_details_spec.rb b/spec/requests/api/graphql/packages/package_composer_details_spec.rb new file mode 100644 index 00000000000..1a2cf4a972a --- /dev/null +++ b/spec/requests/api/graphql/packages/package_composer_details_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe 'package composer details' do + using RSpec::Parameterized::TableSyntax + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:package) { create(:composer_package, project: project) } + let_it_be(:composer_metadatum) do + # we are forced to manually create the metadatum, without using the factory to force the sha to be a string + # and avoid an error where gitaly can't find the repository + create(:composer_metadatum, package: package, target_sha: 'foo_sha', composer_json: { name: 'name', type: 'type', license: 'license', version: 1 }) + end + + let(:query) do + graphql_query_for( + 'packageComposerDetails', + { id: package_global_id }, + all_graphql_fields_for('PackageComposerDetails', max_depth: 2) + ) + end + + let(:user) { project.owner } + let(:package_global_id) { package.to_global_id.to_s } + let(:package_composer_details_response) { graphql_data.dig('packageComposerDetails') } + + subject { post_graphql(query, current_user: user) } + + it_behaves_like 'a working graphql query' do + before do + subject + end + + it 'matches the JSON schema' do + expect(package_composer_details_response).to match_schema('graphql/packages/package_composer_details') + end + end +end diff --git a/spec/requests/api/graphql/project/alert_management/alert/assignees_spec.rb b/spec/requests/api/graphql/project/alert_management/alert/assignees_spec.rb index dd001a73349..9ab94f1d749 100644 --- a/spec/requests/api/graphql/project/alert_management/alert/assignees_spec.rb +++ b/spec/requests/api/graphql/project/alert_management/alert/assignees_spec.rb @@ -60,19 +60,35 @@ RSpec.describe 'getting Alert Management Alert Assignees' do expect(second_assignees).to be_empty end - it 'avoids N+1 queries' do - base_count = ActiveRecord::QueryRecorder.new do - post_graphql(query, current_user: current_user) + describe 'performance' do + let(:first_n) { var('Int') } + let(:params) { { first: first_n } } + let(:limited_query) { with_signature([first_n], query) } + + before do + create(:alert_management_alert, project: project, assignees: [current_user]) + end + + it 'can limit results' do + post_graphql(limited_query, current_user: current_user, variables: first_n.with(1)) + + expect(alerts.size).to eq 1 + expect(alerts).not_to include(a_hash_including('iid' => first_alert.iid.to_s)) end - # An N+1 would mean a new alert would increase the query count - third_alert = create(:alert_management_alert, project: project, assignees: [current_user]) + it 'can include all results' do + post_graphql(limited_query, current_user: current_user) - expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(base_count) + expect(alerts.size).to be > 1 + expect(alerts).to include(a_hash_including('iid' => first_alert.iid.to_s)) + end - third_assignees = assignees[third_alert.iid.to_s] + it 'avoids N+1 queries' do + base_count = ActiveRecord::QueryRecorder.new do + post_graphql(limited_query, current_user: current_user, variables: first_n.with(1)) + end - expect(third_assignees.length).to eq(1) - expect(third_assignees.first).to include('username' => current_user.username) + expect { post_graphql(limited_query, current_user: current_user) }.not_to exceed_query_limit(base_count) + end end end diff --git a/spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb b/spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb index 1350cba119b..5d46f370756 100644 --- a/spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb +++ b/spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb @@ -65,16 +65,28 @@ RSpec.describe 'getting Alert Management Alert Notes' do expect(second_notes_result).to be_empty end - it 'avoids N+1 queries' do - base_count = ActiveRecord::QueryRecorder.new do - post_graphql(query, current_user: current_user) + describe 'performance' do + let(:first_n) { var('Int') } + let(:params) { { first: first_n } } + + before do + # An N+1 would mean a new alert would increase the query count + create(:alert_management_alert, project: project) end - # An N+1 would mean a new alert would increase the query count - create(:alert_management_alert, project: project) + it 'avoids N+1 queries' do + q = with_signature([first_n], query) - expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(base_count) - expect(alerts_result.length).to eq(3) + base_count = ActiveRecord::QueryRecorder.new do + post_graphql(q, current_user: current_user, variables: first_n.with(1)) + expect(alerts_result.length).to eq(1) + end + + expect do + post_graphql(q, current_user: current_user, variables: first_n.with(3)) + expect(alerts_result.length).to eq(3) + end.not_to exceed_query_limit(base_count) + end end context 'for non-system notes' do diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb index fae52fe814d..e1b867ad097 100644 --- a/spec/requests/api/graphql/project/merge_request_spec.rb +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -9,12 +9,13 @@ RSpec.describe 'getting merge request information nested in a project' do let(:current_user) { create(:user) } let(:merge_request_graphql_data) { graphql_data['project']['mergeRequest'] } let!(:merge_request) { create(:merge_request, source_project: project) } + let(:mr_fields) { all_graphql_fields_for('MergeRequest') } let(:query) do graphql_query_for( 'project', { 'fullPath' => project.full_path }, - query_graphql_field('mergeRequest', iid: merge_request.iid.to_s) + query_graphql_field('mergeRequest', { iid: merge_request.iid.to_s }, mr_fields) ) end @@ -43,6 +44,38 @@ RSpec.describe 'getting merge request information nested in a project' do expect(merge_request_graphql_data['author']['username']).to eq(merge_request.author.username) end + context 'the merge_request has reviewers' do + let(:mr_fields) do + <<~SELECT + reviewers { nodes { id username } } + participants { nodes { id username } } + SELECT + end + + before do + merge_request.reviewers << create_list(:user, 2) + end + + it 'includes reviewers' do + expected = merge_request.reviewers.map do |r| + a_hash_including('id' => global_id_of(r), 'username' => r.username) + end + + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:project, :merge_request, :reviewers, :nodes)).to match_array(expected) + expect(graphql_data_at(:project, :merge_request, :participants, :nodes)).to include(*expected) + end + + it 'suppresses reviewers if reviewers are not allowed' do + stub_feature_flags(merge_request_reviewers: false) + + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:project, :merge_request, :reviewers)).to be_nil + end + end + it 'includes diff stats' do be_natural = an_instance_of(Integer).and(be >= 0) @@ -219,4 +252,41 @@ RSpec.describe 'getting merge request information nested in a project' do expect(merge_request_graphql_data['mergeStatus']).to eq('checking') end end + + # see: https://gitlab.com/gitlab-org/gitlab/-/issues/297358 + context 'when the notes have been preloaded (by participants)' do + let(:query) do + <<~GQL + query($path: ID!) { + project(fullPath: $path) { + mrs: mergeRequests(first: 1) { + nodes { + participants { nodes { id } } + notes(first: 1) { + pageInfo { endCursor hasPreviousPage hasNextPage } + nodes { id } + } + } + } + } + } + GQL + end + + before do + create_list(:note_on_merge_request, 3, project: project, noteable: merge_request) + end + + it 'does not error' do + post_graphql(query, + current_user: current_user, + variables: { path: project.full_path }) + + expect(graphql_data_at(:project, :mrs, :nodes, :notes, :pageInfo)).to contain_exactly a_hash_including( + 'endCursor' => String, + 'hasNextPage' => true, + 'hasPreviousPage' => false + ) + end + end end diff --git a/spec/requests/api/graphql/project/merge_requests_spec.rb b/spec/requests/api/graphql/project/merge_requests_spec.rb index c05a620bb62..c85cb8b2ffe 100644 --- a/spec/requests/api/graphql/project/merge_requests_spec.rb +++ b/spec/requests/api/graphql/project/merge_requests_spec.rb @@ -23,9 +23,7 @@ RSpec.describe 'getting merge request listings nested in a project' do graphql_query_for( :project, { full_path: project.full_path }, - query_graphql_field(:merge_requests, search_params, [ - query_graphql_field(:nodes, nil, fields) - ]) + query_nodes(:merge_requests, fields, args: search_params) ) end @@ -50,24 +48,22 @@ RSpec.describe 'getting merge request listings nested in a project' do project.repository.expire_branches_cache end + let(:graphql_data) do + GitlabSchema.execute(query, context: { current_user: current_user }).to_h['data'] + end + context 'selecting any single scalar field' do where(:field) do scalar_fields_of('MergeRequest').map { |name| [name] } end with_them do - it_behaves_like 'a working graphql query' do - let(:query) do - query_merge_requests([:iid, field].uniq) - end - - before do - post_graphql(query, current_user: current_user) - end - - it 'selects the correct MR' do - expect(results).to contain_exactly(a_hash_including('iid' => fresh_mr.iid.to_s)) - end + let(:query) do + query_merge_requests([:iid, field].uniq) + end + + it 'selects the correct MR' do + expect(results).to contain_exactly(a_hash_including('iid' => fresh_mr.iid.to_s)) end end end @@ -87,19 +83,13 @@ RSpec.describe 'getting merge request listings nested in a project' do end with_them do - it_behaves_like 'a working graphql query' do - let(:query) do - fld = is_connection ? query_graphql_field(:nodes, nil, [subfield]) : subfield - query_merge_requests([:iid, query_graphql_field(field, nil, [fld])]) - end - - before do - post_graphql(query, current_user: current_user) - end - - it 'selects the correct MR' do - expect(results).to contain_exactly(a_hash_including('iid' => fresh_mr.iid.to_s)) - end + let(:query) do + fld = is_connection ? query_graphql_field(:nodes, nil, [subfield]) : subfield + query_merge_requests([:iid, query_graphql_field(field, nil, [fld])]) + end + + it 'selects the correct MR' do + expect(results).to contain_exactly(a_hash_including('iid' => fresh_mr.iid.to_s)) end end end @@ -254,6 +244,115 @@ RSpec.describe 'getting merge request listings nested in a project' do include_examples 'N+1 query check' end + + context 'when requesting reviewers' do + let(:requested_fields) { ['reviewers { nodes { username } }'] } + + before do + merge_request_a.reviewers << create(:user) + merge_request_a.reviewers << create(:user) + merge_request_c.reviewers << create(:user) + end + + it 'returns the reviewers' do + execute_query + + expect(results).to include a_hash_including('reviewers' => { + 'nodes' => match_array(merge_request_a.reviewers.map do |r| + a_hash_including('username' => r.username) + end) + }) + end + + context 'the feature flag is disabled' do + before do + stub_feature_flags(merge_request_reviewers: false) + end + + it 'does not return reviewers' do + execute_query + + expect(results).to all(match a_hash_including('reviewers' => be_nil)) + end + end + + include_examples 'N+1 query check' + end + end + + describe 'performance' do + let(:mr_fields) do + <<~SELECT + assignees { nodes { username } } + reviewers { nodes { username } } + participants { nodes { username } } + headPipeline { status } + SELECT + end + + let(:query) do + <<~GQL + query($first: Int) { + project(fullPath: "#{project.full_path}") { + mergeRequests(first: $first) { + nodes { #{mr_fields} } + } + } + } + GQL + end + + before_all do + project.add_developer(current_user) + mrs = create_list(:merge_request, 10, :closed, :with_head_pipeline, + source_project: project, + author: current_user) + mrs.each do |mr| + mr.assignees << create(:user) + mr.assignees << current_user + mr.reviewers << create(:user) + mr.reviewers << current_user + end + end + + before do + # Confounding factor: makes DB calls in EE + allow(Gitlab::Database).to receive(:read_only?).and_return(false) + end + + def run_query(number) + # Ensure that we have a fresh request store and batch-context between runs + result = run_with_clean_state(query, + context: { current_user: current_user }, + variables: { first: number } + ) + + graphql_dig_at(result.to_h, :data, :project, :merge_requests, :nodes) + end + + def user_collection + { 'nodes' => all(match(a_hash_including('username' => be_present))) } + end + + it 'returns appropriate results' do + mrs = run_query(2) + + expect(mrs.size).to eq(2) + expect(mrs).to all( + match( + a_hash_including( + 'assignees' => user_collection, + 'reviewers' => user_collection, + 'participants' => user_collection, + 'headPipeline' => { 'status' => be_present } + ))) + end + + it 'can lookahead to eliminate N+1 queries' do + baseline = ActiveRecord::QueryRecorder.new { run_query(1) } + + expect { run_query(10) }.not_to exceed_query_limit(baseline) + end end describe 'sorting and pagination' do diff --git a/spec/requests/api/graphql/project/project_members_spec.rb b/spec/requests/api/graphql/project/project_members_spec.rb index cb937432ef7..984a0adb8c6 100644 --- a/spec/requests/api/graphql/project/project_members_spec.rb +++ b/spec/requests/api/graphql/project/project_members_spec.rb @@ -11,15 +11,13 @@ RSpec.describe 'getting project members information' do let_it_be(:user_1) { create(:user, username: 'user') } let_it_be(:user_2) { create(:user, username: 'test') } - let(:member_data) { graphql_data['project']['projectMembers']['edges'] } - before_all do [user_1, user_2].each { |user| parent_group.add_guest(user) } end context 'when the request is correct' do it_behaves_like 'a working graphql query' do - before_all do + before do fetch_members(project: parent_project) end end @@ -114,8 +112,7 @@ RSpec.describe 'getting project members information' do def expect_array_response(*items) expect(response).to have_gitlab_http_status(:success) - expect(member_data).to be_an Array - expect(member_data.map { |node| node['node']['user']['id'] }) - .to match_array(items.map { |u| global_id_of(u) }) + member_gids = graphql_data_at(:project, :project_members, :edges, :node, :user, :id) + expect(member_gids).to match_array(items.map { |u| global_id_of(u) }) end end diff --git a/spec/requests/api/graphql/project/release_spec.rb b/spec/requests/api/graphql/project/release_spec.rb index 99b15ff00b1..ccc2825da25 100644 --- a/spec/requests/api/graphql/project/release_spec.rb +++ b/spec/requests/api/graphql/project/release_spec.rb @@ -76,11 +76,11 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do it 'finds all milestones associated to a release' do post_query - expected = release.milestones.map do |milestone| + expected = release.milestones.order_by_dates_and_title.map do |milestone| { 'id' => global_id_of(milestone), 'title' => milestone.title } end - expect(data).to match_array(expected) + expect(data).to eq(expected) end end @@ -427,4 +427,33 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do end end end + + describe 'milestone order' do + let(:path) { path_prefix } + let(:current_user) { stranger } + let_it_be(:project) { create(:project, :public) } + let_it_be_with_reload(:release) { create(:release, project: project) } + + let(:release_fields) do + query_graphql_field(%{ + milestones { + nodes { + title + } + } + }) + end + + let(:actual_milestone_title_order) do + post_query + + data.dig('milestones', 'nodes').map { |m| m['title'] } + end + + before do + release.update!(milestones: [milestone_2, milestone_1]) + end + + it_behaves_like 'correct release milestone order' + end end diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb index b29f9ae913f..2cdd7273b18 100644 --- a/spec/requests/api/graphql/project_query_spec.rb +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -6,26 +6,21 @@ RSpec.describe 'getting project information' do include GraphqlHelpers let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, :repository, group: group) } + let_it_be(:project, reload: true) { create(:project, :repository, group: group) } let_it_be(:current_user) { create(:user) } - let(:fields) { all_graphql_fields_for(Project, max_depth: 2, excluded: %w(jiraImports services)) } + let(:project_fields) { all_graphql_fields_for('project'.to_s.classify, max_depth: 1) } let(:query) do - graphql_query_for(:project, { full_path: project.full_path }, fields) + graphql_query_for(:project, { full_path: project.full_path }, project_fields) end context 'when the user has full access to the project' do - let(:full_access_query) do - graphql_query_for(:project, { full_path: project.full_path }, - all_graphql_fields_for('Project', max_depth: 2)) - end - before do project.add_maintainer(current_user) end it 'includes the project', :use_clean_rails_memory_store_caching, :request_store do - post_graphql(full_access_query, current_user: current_user) + post_graphql(query, current_user: current_user) expect(graphql_data['project']).not_to be_nil end @@ -49,12 +44,12 @@ RSpec.describe 'getting project information' do end context 'when there are pipelines present' do + let(:project_fields) { query_nodes(:pipelines) } + before do create(:ci_pipeline, project: project) end - let(:fields) { query_nodes(:pipelines) } - it 'is included in the pipelines connection' do post_graphql(query, current_user: current_user) @@ -108,55 +103,6 @@ RSpec.describe 'getting project information' do end end - describe 'performance' do - before_all do - project.add_developer(current_user) - mrs = create_list(:merge_request, 10, :closed, :with_head_pipeline, - source_project: project, - author: current_user) - mrs.each do |mr| - mr.assignees << create(:user) - mr.assignees << current_user - end - end - - def run_query(number) - q = <<~GQL - query { - project(fullPath: "#{project.full_path}") { - mergeRequests(first: #{number}) { - nodes { - assignees { nodes { username } } - headPipeline { status } - } - } - } - } - GQL - - post_graphql(q, current_user: current_user) - end - - it 'returns appropriate results' do - run_query(2) - - mrs = graphql_data.dig('project', 'mergeRequests', 'nodes') - - expect(mrs.size).to eq(2) - expect(mrs).to all( - match( - a_hash_including( - 'assignees' => { 'nodes' => all(match(a_hash_including('username' => be_present))) }, - 'headPipeline' => { 'status' => be_present } - ))) - end - - it 'can lookahead to eliminate N+1 queries' do - baseline = ActiveRecord::QueryRecorder.new { run_query(1) } - expect { run_query(10) }.not_to exceed_query_limit(baseline) - end - end - context 'when the user does not have access to the project' do it 'returns an empty field' do post_graphql(query, current_user: current_user) 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 b098058a735..6cb02068f2a 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,13 @@ RSpec.describe 'Getting starredProjects of the user' do let_it_be(:user, reload: true) { create(:user) } let(:user_fields) { 'starredProjects { nodes { id } }' } - let(:starred_projects) { graphql_data_at(:user, :starred_projects, :nodes) } + let(:current_user) { nil } + + let(:starred_projects) do + post_graphql(query, current_user: current_user) + + graphql_data_at(:user, :starred_projects, :nodes) + end before do project_b.add_reporter(user) @@ -26,11 +32,13 @@ RSpec.describe 'Getting starredProjects of the user' do user.toggle_star(project_a) user.toggle_star(project_b) user.toggle_star(project_c) - - post_graphql(query) end - it_behaves_like 'a working graphql query' + it_behaves_like 'a working graphql query' do + before do + post_graphql(query) + end + end it 'found only public project' do expect(starred_projects).to contain_exactly( @@ -41,10 +49,6 @@ RSpec.describe 'Getting starredProjects of the user' do context 'the current user is the user' do let(:current_user) { user } - before do - post_graphql(query, current_user: current_user) - end - it 'found all projects' do expect(starred_projects).to contain_exactly( a_hash_including('id' => global_id_of(project_a)), @@ -56,11 +60,10 @@ RSpec.describe 'Getting starredProjects of the user' do context 'the current user is a member of a private project the user starred' do let_it_be(:other_user) { create(:user) } + let(:current_user) { other_user } before do project_b.add_reporter(other_user) - - post_graphql(query, current_user: other_user) end it 'finds public and member projects' do @@ -74,7 +77,6 @@ RSpec.describe 'Getting starredProjects of the user' do context 'the user has a private profile' do before do user.update!(private_profile: true) - post_graphql(query, current_user: current_user) end context 'the current user does not have access to view the private profile of the user' do diff --git a/spec/requests/api/graphql/users_spec.rb b/spec/requests/api/graphql/users_spec.rb index 72d86c10df1..22b68fbc9bb 100644 --- a/spec/requests/api/graphql/users_spec.rb +++ b/spec/requests/api/graphql/users_spec.rb @@ -54,6 +54,52 @@ RSpec.describe 'Users' do ) end end + + context 'when admins is true' do + let_it_be(:admin) { create(:user, :admin) } + let_it_be(:another_admin) { create(:user, :admin) } + + let(:query) { graphql_query_for(:users, { admins: true }, 'nodes { id }') } + + context 'current user is not an admin' do + let(:post_query) { post_graphql(query, current_user: current_user) } + + it_behaves_like 'a working users query' + + it 'includes all non-admin users', :aggregate_failures do + post_graphql(query) + + expect(graphql_data.dig('users', 'nodes')).to include( + { "id" => user1.to_global_id.to_s }, + { "id" => user2.to_global_id.to_s }, + { "id" => user3.to_global_id.to_s }, + { "id" => current_user.to_global_id.to_s }, + { "id" => admin.to_global_id.to_s }, + { "id" => another_admin.to_global_id.to_s } + ) + end + end + + context 'when current user is an admin' do + it_behaves_like 'a working users query' + + it 'includes only admins', :aggregate_failures do + post_graphql(query, current_user: admin) + + expect(graphql_data.dig('users', 'nodes')).to include( + { "id" => another_admin.to_global_id.to_s }, + { "id" => admin.to_global_id.to_s } + ) + + expect(graphql_data.dig('users', 'nodes')).not_to include( + { "id" => user1.to_global_id.to_s }, + { "id" => user2.to_global_id.to_s }, + { "id" => user3.to_global_id.to_s }, + { "id" => current_user.to_global_id.to_s } + ) + end + end + end end describe 'sorting and pagination' do diff --git a/spec/requests/api/group_packages_spec.rb b/spec/requests/api/group_packages_spec.rb index 72ba25c59af..26895e473de 100644 --- a/spec/requests/api/group_packages_spec.rb +++ b/spec/requests/api/group_packages_spec.rb @@ -6,8 +6,9 @@ RSpec.describe API::GroupPackages do let_it_be(:group) { create(:group, :public) } let_it_be(:project) { create(:project, :public, namespace: group, name: 'project A') } let_it_be(:user) { create(:user) } + let(:params) { {} } - subject { get api(url) } + subject { get api(url), params: params } describe 'GET /groups/:id/packages' do let(:url) { "/groups/#{group.id}/packages" } @@ -142,6 +143,7 @@ RSpec.describe API::GroupPackages do it_behaves_like 'returning response status', :bad_request end + it_behaves_like 'with versionless packages' it_behaves_like 'does not cause n^2 queries' end end diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb index aeb8e3642ed..2ea237469b1 100644 --- a/spec/requests/api/invitations_spec.rb +++ b/spec/requests/api/invitations_spec.rb @@ -30,6 +30,10 @@ RSpec.describe API::Invitations do api("/#{source.model_name.plural}/#{source.id}/invitations", user) end + def invite_member_by_email(source, source_type, email, created_by) + create(:"#{source_type}_member", invite_token: '123', invite_email: email, source: source, user: nil, created_by: created_by) + end + shared_examples 'POST /:source_type/:id/invitations' do |source_type| context "with :source_type == #{source_type.pluralize}" do it_behaves_like 'a 404 response when source is private' do @@ -280,10 +284,6 @@ RSpec.describe API::Invitations do expect(json_response.first['created_by_name']).to eq(developer.name) expect(json_response.first['user_name']).to eq(nil) end - - def invite_member_by_email(source, source_type, email, created_by) - create(:"#{source_type}_member", invite_token: '123', invite_email: email, source: source, user: nil, created_by: created_by) - end end end @@ -298,4 +298,80 @@ RSpec.describe API::Invitations do let(:source) { group } end end + + shared_examples 'DELETE /:source_type/:id/invitations/:email' do |source_type| + def invite_api(source, user, email) + api("/#{source.model_name.plural}/#{source.id}/invitations/#{email}", user) + end + + context "with :source_type == #{source_type.pluralize}" do + let!(:invite) { invite_member_by_email(source, source_type, developer.email, developer) } + + it_behaves_like 'a 404 response when source is private' do + let(:route) { delete api("/#{source_type.pluralize}/#{source.id}/invitations/#{invite.invite_email}", stranger) } + end + + context 'when authenticated as a non-member or member with insufficient rights' do + %i[access_requester stranger].each do |type| + context "as a #{type}" do + it 'returns 403' do + user = public_send(type) + + delete invite_api(source, user, invite.invite_email) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + + context 'when authenticated as a member and deleting themself' do + it 'does not delete the member' do + expect do + delete invite_api(source, developer, invite.invite_email) + + expect(response).to have_gitlab_http_status(:forbidden) + end.not_to change { source.members.count } + end + end + + context 'when authenticated as a maintainer/owner' do + it 'deletes the member and returns 204 with no content' do + expect do + delete invite_api(source, maintainer, invite.invite_email) + + expect(response).to have_gitlab_http_status(:no_content) + end.to change { source.members.count }.by(-1) + end + end + + it 'returns 404 if member does not exist' do + delete invite_api(source, maintainer, non_existing_record_id) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns 422 for a valid request if the resource was not destroyed' do + allow_next_instance_of(::Members::DestroyService) do |instance| + allow(instance).to receive(:execute).with(invite).and_return(invite) + end + + delete invite_api(source, maintainer, invite.invite_email) + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + end + end + + describe 'DELETE /projects/:id/inviations/:email' do + it_behaves_like 'DELETE /:source_type/:id/invitations/:email', 'project' do + let(:source) { project } + end + end + + describe 'DELETE /groups/:id/inviations/:email' do + it_behaves_like 'DELETE /:source_type/:id/invitations/:email', 'group' do + let(:source) { group } + end + end end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index f8521818845..6f854a28cec 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -260,6 +260,36 @@ RSpec.describe API::Jobs do end end + context 'when project is public with artifacts that are non public' do + let(:job) { create(:ci_build, :artifacts, :non_public_artifacts, pipeline: pipeline) } + + it 'rejects access to artifacts' do + project.update_column(:visibility_level, + Gitlab::VisibilityLevel::PUBLIC) + project.update_column(:public_builds, true) + + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + context 'with the non_public_artifacts feature flag disabled' do + before do + stub_feature_flags(non_public_artifacts: false) + end + + it 'allows access to artifacts' do + project.update_column(:visibility_level, + Gitlab::VisibilityLevel::PUBLIC) + project.update_column(:public_builds, true) + + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + context 'when project is public with builds access disabled' do it 'rejects access to artifacts' do project.update_column(:visibility_level, @@ -396,6 +426,33 @@ RSpec.describe API::Jobs do end end + context 'when public project guest and artifacts are non public' do + let(:api_user) { guest } + let(:job) { create(:ci_build, :artifacts, :non_public_artifacts, pipeline: pipeline) } + + before do + project.update_column(:visibility_level, + Gitlab::VisibilityLevel::PUBLIC) + project.update_column(:public_builds, true) + get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) + end + + it 'rejects access and hides existence of artifacts' do + expect(response).to have_gitlab_http_status(:forbidden) + end + + context 'with the non_public_artifacts feature flag disabled' do + before do + stub_feature_flags(non_public_artifacts: false) + get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) + end + + it 'allows access to artifacts' do + expect(response).to have_gitlab_http_status(:ok) + end + end + end + it 'does not return job artifacts if not uploaded' do get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) @@ -580,6 +637,33 @@ RSpec.describe API::Jobs do end end + context 'when project is public with non public artifacts' do + let(:job) { create(:ci_build, :artifacts, :non_public_artifacts, pipeline: pipeline, user: api_user) } + let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } + let(:public_builds) { true } + + it 'rejects access and hides existence of artifacts', :sidekiq_might_not_need_inline do + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response).to have_key('message') + expect(response.headers.to_h) + .not_to include('Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + end + + context 'with the non_public_artifacts feature flag disabled' do + before do + stub_feature_flags(non_public_artifacts: false) + end + + it 'allows access to artifacts', :sidekiq_might_not_need_inline do + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + context 'when project is private' do let(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE } let(:public_builds) { true } @@ -743,32 +827,6 @@ RSpec.describe API::Jobs do expect(response).to have_gitlab_http_status(expected_status) end end - - context 'with restrict_access_to_build_debug_mode feature disabled' do - before do - stub_feature_flags(restrict_access_to_build_debug_mode: false) - end - - where(:public_builds, :user_project_role, :expected_status) do - true | 'developer' | :ok - true | 'guest' | :ok - false | 'developer' | :ok - false | 'guest' | :forbidden - end - - with_them do - before do - project.update!(public_builds: public_builds) - project.add_role(user, user_project_role) - - get api("/projects/#{project.id}/jobs/#{job.id}/trace", api_user) - end - - it 'renders trace to authorized users' do - expect(response).to have_gitlab_http_status(expected_status) - end - end - end end end @@ -923,15 +981,32 @@ RSpec.describe API::Jobs do post api("/projects/#{project.id}/jobs/#{job.id}/play", api_user) end - context 'on an playable job' do - let(:job) { create(:ci_build, :manual, project: project, pipeline: pipeline) } + context 'on a playable job' do + let_it_be(:job) { create(:ci_bridge, :playable, pipeline: pipeline, downstream: project) } + + before do + project.add_developer(user) + end context 'when user is authorized to trigger a manual action' do - it 'plays the job' do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['user']['id']).to eq(user.id) - expect(json_response['id']).to eq(job.id) - expect(job.reload).to be_pending + context 'that is a bridge' do + it 'plays the job' do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['user']['id']).to eq(user.id) + expect(json_response['id']).to eq(job.id) + expect(job.reload).to be_pending + end + end + + context 'that is a build' do + let_it_be(:job) { create(:ci_build, :manual, project: project, pipeline: pipeline) } + + it 'plays the job' do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['user']['id']).to eq(user.id) + expect(json_response['id']).to eq(job.id) + expect(job.reload).to be_pending + end end end diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index b368f6e329c..6db6de4b533 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -520,14 +520,29 @@ RSpec.describe API::Labels do expect(json_response['color']).to eq(label1.color) end - it 'returns 200 if group label already exists' do - create(:group_label, title: label1.name, group: group) + context 'if group label already exists' do + let!(:group_label) { create(:group_label, title: label1.name, group: group) } - expect { put api("/projects/#{project.id}/labels/promote", user), params: { name: label1.name } } - .to change(project.labels, :count).by(-1) - .and change(group.labels, :count).by(0) + it 'returns a status of 200' do + put api("/projects/#{project.id}/labels/promote", user), params: { name: label1.name } - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:ok) + end + + it 'does not change the group label count' do + expect { put api("/projects/#{project.id}/labels/promote", user), params: { name: label1.name } } + .not_to change(group.labels, :count) + end + + it 'does not change the group label max (reuses the same ID)' do + expect { put api("/projects/#{project.id}/labels/promote", user), params: { name: label1.name } } + .not_to change(group.labels, :max) + end + + it 'changes the project label count' do + expect { put api("/projects/#{project.id}/labels/promote", user), params: { name: label1.name } } + .to change(project.labels, :count).by(-1) + end end it 'returns 403 if guest promotes label' do diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb index aecbcfb5b5a..2653653c896 100644 --- a/spec/requests/api/lint_spec.rb +++ b/spec/requests/api/lint_spec.rb @@ -63,7 +63,7 @@ RSpec.describe API::Lint do end context 'with invalid configuration' do - let(:yaml_content) { '{ image: "ruby:2.7", services: ["postgres"] }' } + let(:yaml_content) { '{ image: "ruby:2.7", services: ["postgres"], invalid }' } it 'responds with errors about invalid configuration' do post api('/ci/lint'), params: { content: yaml_content } @@ -71,7 +71,7 @@ RSpec.describe API::Lint do expect(response).to have_gitlab_http_status(:ok) expect(json_response['status']).to eq('invalid') expect(json_response['warnings']).to eq([]) - expect(json_response['errors']).to eq(['jobs config should contain at least one visible job']) + expect(json_response['errors']).to eq(['jobs invalid config should implement a script: or a trigger: keyword', 'jobs config should contain at least one visible job']) end it 'outputs expanded yaml content' do diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index f9ba819c9aa..e5d11fb1218 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -4,7 +4,8 @@ require 'spec_helper' RSpec.describe API::MavenPackages do include WorkhorseHelpers - let_it_be(:group) { create(:group) } + let_it_be_with_refind(:package_settings) { create(:namespace_package_setting, :group) } + let_it_be(:group) { package_settings.namespace } let_it_be(:user) { create(:user) } let_it_be(:project, reload: true) { create(:project, :public, namespace: group) } let_it_be(:package, reload: true) { create(:maven_package, project: project, name: project.full_path) } @@ -18,6 +19,7 @@ RSpec.describe API::MavenPackages do let_it_be(:deploy_token_for_group) { create(:deploy_token, :group, read_package_registry: true, write_package_registry: true) } let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: deploy_token_for_group, group: group) } + let(:package_name) { 'com/example/my-app' } let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } let(:headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } let(:headers_with_token) { headers.merge('Private-Token' => personal_access_token.token) } @@ -669,6 +671,50 @@ RSpec.describe API::MavenPackages do end end + context 'when package duplicates are not allowed' do + let(:package_name) { package.name } + let(:version) { package.version } + + before do + package_settings.update!(maven_duplicates_allowed: false) + end + + shared_examples 'storing the package file' do + it 'stores the file', :aggregate_failures do + expect { upload_file_with_token(params: params) }.to change { package.package_files.count }.by(1) + + expect(response).to have_gitlab_http_status(:ok) + expect(jar_file.file_name).to eq(file_upload.original_filename) + end + end + + it 'rejects the request', :aggregate_failures do + expect { upload_file_with_token(params: params) }.not_to change { package.package_files.count } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to include('Duplicate package is not allowed') + end + + context 'when uploading different non-duplicate files to the same package' do + let!(:package) { create(:maven_package, project: project, name: project.full_path) } + + before do + package_file = package.package_files.find_by(file_name: 'my-app-1.0-20180724.124855-1.jar') + package_file.destroy! + end + + it_behaves_like 'storing the package file' + end + + context 'when the package name matches the exception regex' do + before do + package_settings.update!(maven_duplicate_exception_regex: '.*') + end + + it_behaves_like 'storing the package file' + end + end + context 'for sha1 file' do let(:dummy_package) { double(Packages::Package) } @@ -698,7 +744,7 @@ RSpec.describe API::MavenPackages do end def upload_file(params: {}, request_headers: headers, file_extension: 'jar') - url = "/projects/#{project.id}/packages/maven/com/example/my-app/#{version}/my-app-1.0-20180724.124855-1.#{file_extension}" + url = "/projects/#{project.id}/packages/maven/#{package_name}/#{version}/my-app-1.0-20180724.124855-1.#{file_extension}" workhorse_finalize( api(url), method: :put, diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 4339f1dd830..3a3eae73932 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -440,6 +440,7 @@ RSpec.describe API::MergeRequests do milestone: milestone, author: user, assignees: [user], + reviewers: [user2], source_project: project, target_project: project, source_branch: 'what', @@ -498,6 +499,71 @@ RSpec.describe API::MergeRequests do expect(mr['assignee']['id']).not_to eq(user2.id) end end + + context 'filter by reviewer' do + context 'with reviewer_id' do + context 'with an id' do + let(:params) { { not: { reviewer_id: user2.id } } } + + it 'returns merge requests that do not have the given reviewer' do + get api(endpoint_path, user), params: { not: { reviewer_id: user2.id } } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(4) + expect(json_response.map { |mr| mr['id'] }).not_to include(merge_request2) + end + end + + context 'with Any' do + let(:params) { { not: { reviewer_id: 'Any' } } } + + it 'returns a 400' do + # Any is not supported for negated filter + get api(endpoint_path, user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('not[reviewer_id] is invalid') + end + end + + context 'with None' do + let(:params) { { not: { reviewer_id: 'None' } } } + + it 'returns a 400' do + # None is not supported for negated filter + get api(endpoint_path, user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('not[reviewer_id] is invalid') + end + end + end + + context 'with reviewer_username' do + let(:params) { { not: { reviewer_username: user2.username } } } + + it 'returns merge requests that do not have the given reviewer' do + get api(endpoint_path, user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(4) + expect(json_response.map { |mr| mr['id'] }).not_to include(merge_request2) + end + end + + context 'when both reviewer_id and reviewer_username' do + let(:params) { { not: { reviewer_id: user2.id, reviewer_username: user2.username } } } + + it 'returns a 400' do + get api('/merge_requests', user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('not[reviewer_id], not[reviewer_username] are mutually exclusive') + end + end + end end context 'source_branch param' do @@ -666,6 +732,79 @@ RSpec.describe API::MergeRequests do end end + context 'filter by reviewer' do + let_it_be(:review_requested_mr1) do + create(:merge_request, :unique_branches, author: user, reviewers: [user2], source_project: project2, target_project: project2) + end + + let_it_be(:review_requested_mr2) do + create(:merge_request, :unique_branches, author: user2, reviewers: [user], source_project: project2, target_project: project2) + end + + let(:params) { { scope: :all } } + + context 'with reviewer_id' do + let(:params) { super().merge(reviewer_id: reviewer_id) } + + context 'with an id' do + let(:reviewer_id) { user2.id } + + it 'returns review requested merge requests for the given user' do + get api('/merge_requests', user), params: params + + expect_response_contain_exactly(review_requested_mr1.id) + end + end + + context 'with Any' do + let(:reviewer_id) { 'Any' } + + it 'returns review requested merge requests for any user' do + get api('/merge_requests', user), params: params + + expect_response_contain_exactly(review_requested_mr1.id, review_requested_mr2.id) + end + end + + context 'with None' do + let(:reviewer_id) { 'None' } + + it 'returns merge requests that has no assigned reviewers' do + get api('/merge_requests', user), params: params + + expect_response_contain_exactly( + merge_request.id, + merge_request_closed.id, + merge_request_merged.id, + merge_request_locked.id, + merge_request2.id + ) + end + end + end + + context 'with reviewer_username' do + let(:params) { super().merge(reviewer_username: user2.username) } + + it 'returns review requested merge requests for the given user' do + get api('/merge_requests', user), params: params + + expect_response_contain_exactly(review_requested_mr1.id) + end + end + + context 'with both reviewer_id and reviewer_username' do + let(:params) { super().merge(reviewer_id: user2.id, reviewer_username: user2.username) } + + it 'returns a 400' do + get api('/merge_requests', user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('reviewer_id, reviewer_username are mutually exclusive') + end + end + end + it 'returns an array of merge requests assigned to the given user' do merge_request3 = create(:merge_request, :simple, author: user, assignees: [user2], source_project: project2, target_project: project2, source_branch: 'other-branch') diff --git a/spec/requests/api/nuget_group_packages_spec.rb b/spec/requests/api/nuget_group_packages_spec.rb new file mode 100644 index 00000000000..f7e81494660 --- /dev/null +++ b/spec/requests/api/nuget_group_packages_spec.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe API::NugetGroupPackages do + include_context 'nuget api setup' + + using RSpec::Parameterized::TableSyntax + + let_it_be_with_reload(:group) { create(:group) } + let_it_be_with_reload(:subgroup) { create(:group, parent: group) } + let_it_be_with_reload(:project) { create(:project, namespace: subgroup) } + let_it_be(:deploy_token) { create(:deploy_token, :group, read_package_registry: true, write_package_registry: true) } + let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: deploy_token, group: group) } + + let(:target_type) { 'groups' } + + shared_examples 'handling all endpoints' do + describe 'GET /api/v4/groups/:id/-/packages/nuget' do + it_behaves_like 'handling nuget service requests', anonymous_requests_example_name: 'rejects nuget packages access', anonymous_requests_status: :unauthorized do + let(:url) { "/groups/#{target.id}/-/packages/nuget/index.json" } + end + end + + describe 'GET /api/v4/groups/:id/-/packages/nuget/metadata/*package_name/index' do + it_behaves_like 'handling nuget metadata requests with package name', anonymous_requests_example_name: 'rejects nuget packages access', anonymous_requests_status: :unauthorized do + let(:url) { "/groups/#{target.id}/-/packages/nuget/metadata/#{package_name}/index.json" } + end + end + + describe 'GET /api/v4/groups/:id/-/packages/nuget/metadata/*package_name/*package_version' do + it_behaves_like 'handling nuget metadata requests with package name and package version', anonymous_requests_example_name: 'rejects nuget packages access', anonymous_requests_status: :unauthorized do + let(:url) { "/groups/#{target.id}/-/packages/nuget/metadata/#{package_name}/#{package.version}.json" } + end + end + + describe 'GET /api/v4/groups/:id/-/packages/nuget/query' do + it_behaves_like 'handling nuget search requests', anonymous_requests_example_name: 'rejects nuget packages access', anonymous_requests_status: :unauthorized do + let(:url) { "/groups/#{target.id}/-/packages/nuget/query?#{query_parameters.to_query}" } + end + end + end + + context 'with a subgroup' do + # Bug: deploy tokens at parent group will not see the subgroup. + # https://gitlab.com/gitlab-org/gitlab/-/issues/285495 + let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: deploy_token, group: subgroup) } + + let(:target) { subgroup } + + it_behaves_like 'handling all endpoints' + + def update_visibility_to(visibility) + project.update!(visibility_level: visibility) + subgroup.update!(visibility_level: visibility) + end + end + + context 'a group' do + let(:target) { group } + + it_behaves_like 'handling all endpoints' + + context 'with dummy packages and anonymous request' do + let_it_be(:package_name) { 'Dummy.Package' } + let_it_be(:packages) { create_list(:nuget_package, 5, :with_metadatum, name: package_name, project: project) } + let_it_be(:tags) { packages.each { |pkg| create(:packages_tag, package: pkg, name: 'test') } } + + let(:search_term) { 'umm' } + let(:take) { 26 } + let(:skip) { 0 } + let(:include_prereleases) { true } + let(:query_parameters) { { q: search_term, take: take, skip: skip, prerelease: include_prereleases } } + + subject { get api(url), headers: {}} + + shared_examples 'handling mixed visibilities' do + where(:group_visibility, :subgroup_visibility, :expected_status) do + 'PUBLIC' | 'PUBLIC' | :unauthorized + 'PUBLIC' | 'INTERNAL' | :unauthorized + 'PUBLIC' | 'PRIVATE' | :unauthorized + 'INTERNAL' | 'INTERNAL' | :unauthorized + 'INTERNAL' | 'PRIVATE' | :unauthorized + 'PRIVATE' | 'PRIVATE' | :unauthorized + end + + with_them do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(subgroup_visibility, false)) + subgroup.update!(visibility_level: Gitlab::VisibilityLevel.const_get(subgroup_visibility, false)) + group.update!(visibility_level: Gitlab::VisibilityLevel.const_get(group_visibility, false)) + end + + it_behaves_like 'returning response status', params[:expected_status] + end + end + + describe 'GET /api/v4/groups/:id/-/packages/nuget/metadata/*package_name/index' do + it_behaves_like 'handling mixed visibilities' do + let(:url) { "/groups/#{target.id}/-/packages/nuget/metadata/#{package_name}/index.json" } + end + end + + describe 'GET /api/v4/groups/:id/-/packages/nuget/metadata/*package_name/*package_version' do + it_behaves_like 'handling mixed visibilities' do + let(:url) { "/groups/#{target.id}/-/packages/nuget/metadata/#{package_name}/#{packages.first.version}.json" } + end + end + + describe 'GET /api/v4/groups/:id/-/packages/nuget/query' do + it_behaves_like 'handling mixed visibilities' do + let(:url) { "/groups/#{target.id}/-/packages/nuget/query?#{query_parameters.to_query}" } + end + end + end + + def update_visibility_to(visibility) + project.update!(visibility_level: visibility) + subgroup.update!(visibility_level: visibility) + group.update!(visibility_level: visibility) + end + end +end diff --git a/spec/requests/api/nuget_project_packages_spec.rb b/spec/requests/api/nuget_project_packages_spec.rb index df1daf39144..813ebc35ede 100644 --- a/spec/requests/api/nuget_project_packages_spec.rb +++ b/spec/requests/api/nuget_project_packages_spec.rb @@ -2,99 +2,202 @@ require 'spec_helper' RSpec.describe API::NugetProjectPackages do - include WorkhorseHelpers - include PackagesManagerApiSpecHelpers - include HttpBasicAuthHelpers + include_context 'nuget api setup' - let_it_be(:user) { create(:user) } - let_it_be(:project, reload: true) { create(:project, :public) } - let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + using RSpec::Parameterized::TableSyntax + + let_it_be_with_reload(:project) { create(:project, :public) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } + let(:target) { project } + let(:target_type) { 'projects' } + describe 'GET /api/v4/projects/:id/packages/nuget' do it_behaves_like 'handling nuget service requests' do - let(:url) { "/projects/#{project.id}/packages/nuget/index.json" } + let(:url) { "/projects/#{target.id}/packages/nuget/index.json" } end end describe 'GET /api/v4/projects/:id/packages/nuget/metadata/*package_name/index' do it_behaves_like 'handling nuget metadata requests with package name' do - let(:url) { "/projects/#{project.id}/packages/nuget/metadata/#{package_name}/index.json" } + let(:url) { "/projects/#{target.id}/packages/nuget/metadata/#{package_name}/index.json" } end end describe 'GET /api/v4/projects/:id/packages/nuget/metadata/*package_name/*package_version' do it_behaves_like 'handling nuget metadata requests with package name and package version' do - let(:url) { "/projects/#{project.id}/packages/nuget/metadata/#{package_name}/#{package.version}.json" } + let(:url) { "/projects/#{target.id}/packages/nuget/metadata/#{package_name}/#{package.version}.json" } end end describe 'GET /api/v4/projects/:id/packages/nuget/query' do it_behaves_like 'handling nuget search requests' do - let(:url) { "/projects/#{project.id}/packages/nuget/query?#{query_parameters.to_query}" } + let(:url) { "/projects/#{target.id}/packages/nuget/query?#{query_parameters.to_query}" } end end + describe 'GET /api/v4/projects/:id/packages/nuget/download/*package_name/index' do + let_it_be(:package_name) { 'Dummy.Package' } + let_it_be(:packages) { create_list(:nuget_package, 5, name: package_name, project: project) } + + let(:url) { "/projects/#{target.id}/packages/nuget/download/#{package_name}/index.json" } + + subject { get api(url) } + + context 'with valid target' do + where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do + 'PUBLIC' | :developer | true | true | 'process nuget download versions request' | :success + 'PUBLIC' | :guest | true | true | 'process nuget download versions request' | :success + 'PUBLIC' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :developer | false | true | 'process nuget download versions request' | :success + 'PUBLIC' | :guest | false | true | 'process nuget download versions request' | :success + 'PUBLIC' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :anonymous | false | true | 'process nuget download versions request' | :success + 'PRIVATE' | :developer | true | true | 'process nuget download versions request' | :success + 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden + 'PRIVATE' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :guest | true | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :developer | false | true | 'rejects nuget packages access' | :not_found + 'PRIVATE' | :guest | false | true | 'rejects nuget packages access' | :not_found + 'PRIVATE' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :guest | false | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :anonymous | false | true | 'rejects nuget packages access' | :unauthorized + end + + with_them do + let(:token) { user_token ? personal_access_token.token : 'wrong' } + let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } + + subject { get api(url), headers: headers } + + before do + update_visibility_to(Gitlab::VisibilityLevel.const_get(visibility_level, false)) + end + + it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] + end + end + + it_behaves_like 'deploy token for package GET requests' + + it_behaves_like 'rejects nuget access with unknown target id' + + it_behaves_like 'rejects nuget access with invalid target id' + end + + describe 'GET /api/v4/projects/:id/packages/nuget/download/*package_name/*package_version/*package_filename' do + let_it_be(:package_name) { 'Dummy.Package' } + let_it_be(:package) { create(:nuget_package, project: project, name: package_name) } + + let(:url) { "/projects/#{target.id}/packages/nuget/download/#{package.name}/#{package.version}/#{package.name}.#{package.version}.nupkg" } + + subject { get api(url) } + + context 'with valid target' do + where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do + 'PUBLIC' | :developer | true | true | 'process nuget download content request' | :success + 'PUBLIC' | :guest | true | true | 'process nuget download content request' | :success + 'PUBLIC' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :developer | false | true | 'process nuget download content request' | :success + 'PUBLIC' | :guest | false | true | 'process nuget download content request' | :success + 'PUBLIC' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :anonymous | false | true | 'process nuget download content request' | :success + 'PRIVATE' | :developer | true | true | 'process nuget download content request' | :success + 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden + 'PRIVATE' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :guest | true | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :developer | false | true | 'rejects nuget packages access' | :not_found + 'PRIVATE' | :guest | false | true | 'rejects nuget packages access' | :not_found + 'PRIVATE' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :guest | false | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :anonymous | false | true | 'rejects nuget packages access' | :unauthorized + end + + with_them do + let(:token) { user_token ? personal_access_token.token : 'wrong' } + let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } + + subject { get api(url), headers: headers } + + before do + update_visibility_to(Gitlab::VisibilityLevel.const_get(visibility_level, false)) + end + + it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] + end + end + + it_behaves_like 'deploy token for package GET requests' do + before do + update_visibility_to(Gitlab::VisibilityLevel::PRIVATE) + end + end + + it_behaves_like 'rejects nuget access with unknown target id' + + it_behaves_like 'rejects nuget access with invalid target id' + end + describe 'PUT /api/v4/projects/:id/packages/nuget/authorize' do let_it_be(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } let_it_be(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } - let(:url) { "/projects/#{project.id}/packages/nuget/authorize" } + let(:url) { "/projects/#{target.id}/packages/nuget/authorize" } let(:headers) { {} } subject { put api(url), headers: headers } - context 'without the need for a license' do - context 'with valid project' do - using RSpec::Parameterized::TableSyntax - - where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do - 'PUBLIC' | :developer | true | true | 'process nuget workhorse authorization' | :success - 'PUBLIC' | :guest | true | true | 'rejects nuget packages access' | :forbidden - 'PUBLIC' | :developer | true | false | 'rejects nuget packages access' | :unauthorized - 'PUBLIC' | :guest | true | false | 'rejects nuget packages access' | :unauthorized - 'PUBLIC' | :developer | false | true | 'rejects nuget packages access' | :forbidden - 'PUBLIC' | :guest | false | true | 'rejects nuget packages access' | :forbidden - 'PUBLIC' | :developer | false | false | 'rejects nuget packages access' | :unauthorized - 'PUBLIC' | :guest | false | false | 'rejects nuget packages access' | :unauthorized - 'PUBLIC' | :anonymous | false | true | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :developer | true | true | 'process nuget workhorse authorization' | :success - 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden - 'PRIVATE' | :developer | true | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :guest | true | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :developer | false | true | 'rejects nuget packages access' | :not_found - 'PRIVATE' | :guest | false | true | 'rejects nuget packages access' | :not_found - 'PRIVATE' | :developer | false | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :guest | false | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :anonymous | false | true | 'rejects nuget packages access' | :unauthorized - end - - with_them do - let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:user_headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } - let(:headers) { user_headers.merge(workhorse_header) } + context 'with valid project' do + where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do + 'PUBLIC' | :developer | true | true | 'process nuget workhorse authorization' | :success + 'PUBLIC' | :guest | true | true | 'rejects nuget packages access' | :forbidden + 'PUBLIC' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :developer | false | true | 'rejects nuget packages access' | :forbidden + 'PUBLIC' | :guest | false | true | 'rejects nuget packages access' | :forbidden + 'PUBLIC' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :anonymous | false | true | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :developer | true | true | 'process nuget workhorse authorization' | :success + 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden + 'PRIVATE' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :guest | true | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :developer | false | true | 'rejects nuget packages access' | :not_found + 'PRIVATE' | :guest | false | true | 'rejects nuget packages access' | :not_found + 'PRIVATE' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :guest | false | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :anonymous | false | true | 'rejects nuget packages access' | :unauthorized + end - before do - project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility_level, false)) - end + with_them do + let(:token) { user_token ? personal_access_token.token : 'wrong' } + let(:user_headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } + let(:headers) { user_headers.merge(workhorse_header) } - it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] + before do + update_visibility_to(Gitlab::VisibilityLevel.const_get(visibility_level, false)) end + + it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] end + end - it_behaves_like 'deploy token for package uploads' + it_behaves_like 'deploy token for package uploads' - it_behaves_like 'rejects nuget access with unknown project id' + it_behaves_like 'rejects nuget access with unknown target id' - it_behaves_like 'rejects nuget access with invalid project id' - end + it_behaves_like 'rejects nuget access with invalid target id' end describe 'PUT /api/v4/projects/:id/packages/nuget' do let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } let(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } let_it_be(:file_name) { 'package.nupkg' } - let(:url) { "/projects/#{project.id}/packages/nuget" } + let(:url) { "/projects/#{target.id}/packages/nuget" } let(:headers) { {} } let(:params) { { package: temp_file(file_name) } } let(:file_key) { :package } @@ -111,170 +214,61 @@ RSpec.describe API::NugetProjectPackages do ) end - context 'without the need for a license' do - context 'with valid project' do - using RSpec::Parameterized::TableSyntax - - where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do - 'PUBLIC' | :developer | true | true | 'process nuget upload' | :created - 'PUBLIC' | :guest | true | true | 'rejects nuget packages access' | :forbidden - 'PUBLIC' | :developer | true | false | 'rejects nuget packages access' | :unauthorized - 'PUBLIC' | :guest | true | false | 'rejects nuget packages access' | :unauthorized - 'PUBLIC' | :developer | false | true | 'rejects nuget packages access' | :forbidden - 'PUBLIC' | :guest | false | true | 'rejects nuget packages access' | :forbidden - 'PUBLIC' | :developer | false | false | 'rejects nuget packages access' | :unauthorized - 'PUBLIC' | :guest | false | false | 'rejects nuget packages access' | :unauthorized - 'PUBLIC' | :anonymous | false | true | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :developer | true | true | 'process nuget upload' | :created - 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden - 'PRIVATE' | :developer | true | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :guest | true | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :developer | false | true | 'rejects nuget packages access' | :not_found - 'PRIVATE' | :guest | false | true | 'rejects nuget packages access' | :not_found - 'PRIVATE' | :developer | false | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :guest | false | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :anonymous | false | true | 'rejects nuget packages access' | :unauthorized - end - - with_them do - let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:user_headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } - let(:headers) { user_headers.merge(workhorse_header) } - - before do - project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility_level, false)) - end - - it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] - end + context 'with valid project' do + where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do + 'PUBLIC' | :developer | true | true | 'process nuget upload' | :created + 'PUBLIC' | :guest | true | true | 'rejects nuget packages access' | :forbidden + 'PUBLIC' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | true | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :developer | false | true | 'rejects nuget packages access' | :forbidden + 'PUBLIC' | :guest | false | true | 'rejects nuget packages access' | :forbidden + 'PUBLIC' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :guest | false | false | 'rejects nuget packages access' | :unauthorized + 'PUBLIC' | :anonymous | false | true | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :developer | true | true | 'process nuget upload' | :created + 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden + 'PRIVATE' | :developer | true | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :guest | true | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :developer | false | true | 'rejects nuget packages access' | :not_found + 'PRIVATE' | :guest | false | true | 'rejects nuget packages access' | :not_found + 'PRIVATE' | :developer | false | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :guest | false | false | 'rejects nuget packages access' | :unauthorized + 'PRIVATE' | :anonymous | false | true | 'rejects nuget packages access' | :unauthorized end - it_behaves_like 'deploy token for package uploads' - - it_behaves_like 'rejects nuget access with unknown project id' - - it_behaves_like 'rejects nuget access with invalid project id' - - context 'file size above maximum limit' do - let(:headers) { basic_auth_header(deploy_token.username, deploy_token.token).merge(workhorse_header) } + with_them do + let(:token) { user_token ? personal_access_token.token : 'wrong' } + let(:user_headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } + let(:headers) { user_headers.merge(workhorse_header) } before do - allow_next_instance_of(UploadedFile) do |uploaded_file| - allow(uploaded_file).to receive(:size).and_return(project.actual_limits.nuget_max_file_size + 1) - end + update_visibility_to(Gitlab::VisibilityLevel.const_get(visibility_level, false)) end - it_behaves_like 'returning response status', :bad_request + it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] end end - end - - describe 'GET /api/v4/projects/:id/packages/nuget/download/*package_name/index' do - let_it_be(:package_name) { 'Dummy.Package' } - let_it_be(:packages) { create_list(:nuget_package, 5, name: package_name, project: project) } - let(:url) { "/projects/#{project.id}/packages/nuget/download/#{package_name}/index.json" } - - subject { get api(url) } - context 'without the need for a license' do - context 'with valid project' do - using RSpec::Parameterized::TableSyntax - - where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do - 'PUBLIC' | :developer | true | true | 'process nuget download versions request' | :success - 'PUBLIC' | :guest | true | true | 'process nuget download versions request' | :success - 'PUBLIC' | :developer | true | false | 'process nuget download versions request' | :success - 'PUBLIC' | :guest | true | false | 'process nuget download versions request' | :success - 'PUBLIC' | :developer | false | true | 'process nuget download versions request' | :success - 'PUBLIC' | :guest | false | true | 'process nuget download versions request' | :success - 'PUBLIC' | :developer | false | false | 'process nuget download versions request' | :success - 'PUBLIC' | :guest | false | false | 'process nuget download versions request' | :success - 'PUBLIC' | :anonymous | false | true | 'process nuget download versions request' | :success - 'PRIVATE' | :developer | true | true | 'process nuget download versions request' | :success - 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden - 'PRIVATE' | :developer | true | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :guest | true | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :developer | false | true | 'rejects nuget packages access' | :not_found - 'PRIVATE' | :guest | false | true | 'rejects nuget packages access' | :not_found - 'PRIVATE' | :developer | false | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :guest | false | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :anonymous | false | true | 'rejects nuget packages access' | :unauthorized - end + it_behaves_like 'deploy token for package uploads' - with_them do - let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } + it_behaves_like 'rejects nuget access with unknown target id' - subject { get api(url), headers: headers } + it_behaves_like 'rejects nuget access with invalid target id' - before do - project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility_level, false)) - end + context 'file size above maximum limit' do + let(:headers) { basic_auth_header(deploy_token.username, deploy_token.token).merge(workhorse_header) } - it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] + before do + allow_next_instance_of(UploadedFile) do |uploaded_file| + allow(uploaded_file).to receive(:size).and_return(project.actual_limits.nuget_max_file_size + 1) end end - it_behaves_like 'deploy token for package GET requests' - - it_behaves_like 'rejects nuget access with unknown project id' - - it_behaves_like 'rejects nuget access with invalid project id' + it_behaves_like 'returning response status', :bad_request end end - describe 'GET /api/v4/projects/:id/packages/nuget/download/*package_name/*package_version/*package_filename' do - let_it_be(:package_name) { 'Dummy.Package' } - let_it_be(:package) { create(:nuget_package, project: project, name: package_name) } - - let(:url) { "/projects/#{project.id}/packages/nuget/download/#{package.name}/#{package.version}/#{package.name}.#{package.version}.nupkg" } - - subject { get api(url) } - - context 'without the need for a license' do - context 'with valid project' do - using RSpec::Parameterized::TableSyntax - - where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do - 'PUBLIC' | :developer | true | true | 'process nuget download content request' | :success - 'PUBLIC' | :guest | true | true | 'process nuget download content request' | :success - 'PUBLIC' | :developer | true | false | 'process nuget download content request' | :success - 'PUBLIC' | :guest | true | false | 'process nuget download content request' | :success - 'PUBLIC' | :developer | false | true | 'process nuget download content request' | :success - 'PUBLIC' | :guest | false | true | 'process nuget download content request' | :success - 'PUBLIC' | :developer | false | false | 'process nuget download content request' | :success - 'PUBLIC' | :guest | false | false | 'process nuget download content request' | :success - 'PUBLIC' | :anonymous | false | true | 'process nuget download content request' | :success - 'PRIVATE' | :developer | true | true | 'process nuget download content request' | :success - 'PRIVATE' | :guest | true | true | 'rejects nuget packages access' | :forbidden - 'PRIVATE' | :developer | true | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :guest | true | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :developer | false | true | 'rejects nuget packages access' | :not_found - 'PRIVATE' | :guest | false | true | 'rejects nuget packages access' | :not_found - 'PRIVATE' | :developer | false | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :guest | false | false | 'rejects nuget packages access' | :unauthorized - 'PRIVATE' | :anonymous | false | true | 'rejects nuget packages access' | :unauthorized - end - - with_them do - let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } - - subject { get api(url), headers: headers } - - before do - project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility_level, false)) - end - - it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] - end - end - - it_behaves_like 'deploy token for package GET requests' - - it_behaves_like 'rejects nuget access with unknown project id' - - it_behaves_like 'rejects nuget access with invalid project id' - end + def update_visibility_to(visibility) + project.update!(visibility_level: visibility) end end diff --git a/spec/requests/api/project_packages_spec.rb b/spec/requests/api/project_packages_spec.rb index 4c8599d1a20..eb86df36dbb 100644 --- a/spec/requests/api/project_packages_spec.rb +++ b/spec/requests/api/project_packages_spec.rb @@ -11,12 +11,13 @@ RSpec.describe API::ProjectPackages do let!(:another_package) { create(:npm_package) } let(:no_package_url) { "/projects/#{project.id}/packages/0" } let(:wrong_package_url) { "/projects/#{project.id}/packages/#{another_package.id}" } + let(:params) { {} } describe 'GET /projects/:id/packages' do let(:url) { "/projects/#{project.id}/packages" } let(:package_schema) { 'public_api/v4/packages/packages' } - subject { get api(url) } + subject { get api(url), params: params } context 'without the need for a license' do context 'project is public' do @@ -118,6 +119,7 @@ RSpec.describe API::ProjectPackages do end end + it_behaves_like 'with versionless packages' it_behaves_like 'does not cause n^2 queries' end end diff --git a/spec/requests/api/project_repository_storage_moves_spec.rb b/spec/requests/api/project_repository_storage_moves_spec.rb index 15e69c2aa16..5e200312d1f 100644 --- a/spec/requests/api/project_repository_storage_moves_spec.rb +++ b/spec/requests/api/project_repository_storage_moves_spec.rb @@ -3,220 +3,10 @@ require 'spec_helper' RSpec.describe API::ProjectRepositoryStorageMoves do - include AccessMatchersForRequest - - let_it_be(:user) { create(:admin) } - let_it_be(:project) { create(:project, :repository).tap { |project| project.track_project_repository } } - let_it_be(:storage_move) { create(:project_repository_storage_move, :scheduled, container: project) } - - shared_examples 'get single project repository storage move' do - let(:project_repository_storage_move_id) { storage_move.id } - - def get_project_repository_storage_move - get api(url, user) - end - - it 'returns a project repository storage move' do - get_project_repository_storage_move - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('public_api/v4/project_repository_storage_move') - expect(json_response['id']).to eq(storage_move.id) - expect(json_response['state']).to eq(storage_move.human_state_name) - end - - context 'non-existent project repository storage move' do - let(:project_repository_storage_move_id) { non_existing_record_id } - - it 'returns not found' do - get_project_repository_storage_move - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - describe 'permissions' do - it { expect { get_project_repository_storage_move }.to be_allowed_for(:admin) } - it { expect { get_project_repository_storage_move }.to be_denied_for(:user) } - end - end - - shared_examples 'get project repository storage move list' do - def get_project_repository_storage_moves - get api(url, user) - end - - it 'returns project repository storage moves' do - get_project_repository_storage_moves - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(response).to match_response_schema('public_api/v4/project_repository_storage_moves') - expect(json_response.size).to eq(1) - expect(json_response.first['id']).to eq(storage_move.id) - expect(json_response.first['state']).to eq(storage_move.human_state_name) - end - - it 'avoids N+1 queries', :request_store do - # prevent `let` from polluting the control - get_project_repository_storage_moves - - control = ActiveRecord::QueryRecorder.new { get_project_repository_storage_moves } - - create(:project_repository_storage_move, :scheduled, container: project) - - expect { get_project_repository_storage_moves }.not_to exceed_query_limit(control) - end - - it 'returns the most recently created first' do - storage_move_oldest = create(:project_repository_storage_move, :scheduled, container: project, created_at: 2.days.ago) - storage_move_middle = create(:project_repository_storage_move, :scheduled, container: project, created_at: 1.day.ago) - - get_project_repository_storage_moves - - json_ids = json_response.map {|storage_move| storage_move['id'] } - expect(json_ids).to eq([ - storage_move.id, - storage_move_middle.id, - storage_move_oldest.id - ]) - end - - describe 'permissions' do - it { expect { get_project_repository_storage_moves }.to be_allowed_for(:admin) } - it { expect { get_project_repository_storage_moves }.to be_denied_for(:user) } - end - end - - describe 'GET /project_repository_storage_moves' do - it_behaves_like 'get project repository storage move list' do - let(:url) { '/project_repository_storage_moves' } - end - end - - describe 'GET /project_repository_storage_moves/:repository_storage_move_id' do - it_behaves_like 'get single project repository storage move' do - let(:url) { "/project_repository_storage_moves/#{project_repository_storage_move_id}" } - end - end - - describe 'GET /projects/:id/repository_storage_moves' do - it_behaves_like 'get project repository storage move list' do - let(:url) { "/projects/#{project.id}/repository_storage_moves" } - end - end - - describe 'GET /projects/:id/repository_storage_moves/:repository_storage_move_id' do - it_behaves_like 'get single project repository storage move' do - let(:url) { "/projects/#{project.id}/repository_storage_moves/#{project_repository_storage_move_id}" } - end - end - - describe 'POST /projects/:id/repository_storage_moves' do - let(:url) { "/projects/#{project.id}/repository_storage_moves" } - let(:destination_storage_name) { 'test_second_storage' } - - def create_project_repository_storage_move - post api(url, user), params: { destination_storage_name: destination_storage_name } - end - - before do - stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/extra_storage' }) - end - - it 'schedules a project repository storage move' do - create_project_repository_storage_move - - storage_move = project.repository_storage_moves.last - - expect(response).to have_gitlab_http_status(:created) - expect(response).to match_response_schema('public_api/v4/project_repository_storage_move') - expect(json_response['id']).to eq(storage_move.id) - expect(json_response['state']).to eq('scheduled') - expect(json_response['source_storage_name']).to eq('default') - expect(json_response['destination_storage_name']).to eq(destination_storage_name) - end - - describe 'permissions' do - it { expect { create_project_repository_storage_move }.to be_allowed_for(:admin) } - it { expect { create_project_repository_storage_move }.to be_denied_for(:user) } - end - - context 'destination_storage_name is missing' do - let(:destination_storage_name) { nil } - - it 'schedules a project repository storage move' do - create_project_repository_storage_move - - storage_move = project.repository_storage_moves.last - - expect(response).to have_gitlab_http_status(:created) - expect(response).to match_response_schema('public_api/v4/project_repository_storage_move') - expect(json_response['id']).to eq(storage_move.id) - expect(json_response['state']).to eq('scheduled') - expect(json_response['source_storage_name']).to eq('default') - expect(json_response['destination_storage_name']).to be_present - end - end - end - - describe 'POST /project_repository_storage_moves' do - let(:source_storage_name) { 'default' } - let(:destination_storage_name) { 'test_second_storage' } - - def create_project_repository_storage_moves - post api('/project_repository_storage_moves', user), params: { - source_storage_name: source_storage_name, - destination_storage_name: destination_storage_name - } - end - - before do - stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/extra_storage' }) - end - - it 'schedules the worker' do - expect(ProjectScheduleBulkRepositoryShardMovesWorker).to receive(:perform_async).with(source_storage_name, destination_storage_name) - - create_project_repository_storage_moves - - expect(response).to have_gitlab_http_status(:accepted) - end - - context 'source_storage_name is invalid' do - let(:destination_storage_name) { 'not-a-real-storage' } - - it 'gives an error' do - create_project_repository_storage_moves - - expect(response).to have_gitlab_http_status(:bad_request) - end - end - - context 'destination_storage_name is missing' do - let(:destination_storage_name) { nil } - - it 'schedules the worker' do - expect(ProjectScheduleBulkRepositoryShardMovesWorker).to receive(:perform_async).with(source_storage_name, destination_storage_name) - - create_project_repository_storage_moves - - expect(response).to have_gitlab_http_status(:accepted) - end - end - - context 'destination_storage_name is invalid' do - let(:destination_storage_name) { 'not-a-real-storage' } - - it 'gives an error' do - create_project_repository_storage_moves - - expect(response).to have_gitlab_http_status(:bad_request) - end - end - - describe 'normal user' do - it { expect { create_project_repository_storage_moves }.to be_denied_for(:user) } - end + it_behaves_like 'repository_storage_moves API', 'projects' do + let_it_be(:container) { create(:project, :repository).tap { |project| project.track_project_repository } } + let_it_be(:storage_move) { create(:project_repository_storage_move, :scheduled, container: container) } + let(:repository_storage_move_factory) { :project_repository_storage_move } + let(:bulk_worker_klass) { ProjectScheduleBulkRepositoryShardMovesWorker } end end diff --git a/spec/requests/api/project_templates_spec.rb b/spec/requests/api/project_templates_spec.rb index d242d49fc1b..fc1035fc17d 100644 --- a/spec/requests/api/project_templates_spec.rb +++ b/spec/requests/api/project_templates_spec.rb @@ -53,6 +53,15 @@ RSpec.describe API::ProjectTemplates do expect(json_response).to satisfy_one { |template| template['key'] == 'Android' } end + it 'returns gitlab_ci_syntax_ymls' do + get api("/projects/#{public_project.id}/templates/gitlab_ci_syntax_ymls") + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(response).to match_response_schema('public_api/v4/template_list') + expect(json_response).to satisfy_one { |template| template['key'] == 'Artifacts example' } + end + it 'returns licenses' do get api("/projects/#{public_project.id}/templates/licenses") @@ -163,6 +172,14 @@ RSpec.describe API::ProjectTemplates do expect(json_response['name']).to eq('Android') end + it 'returns a specific gitlab_ci_syntax_yml' do + get api("/projects/#{public_project.id}/templates/gitlab_ci_syntax_ymls/Artifacts%20example") + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/template') + expect(json_response['name']).to eq('Artifacts example') + end + it 'returns a specific metrics_dashboard_yml' do get api("/projects/#{public_project.id}/templates/metrics_dashboard_ymls/Default") diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index ad5468fb54c..4acd0eea448 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1583,6 +1583,7 @@ RSpec.describe API::Projects do expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access) expect(json_response['only_allow_merge_if_pipeline_succeeds']).to eq(project.only_allow_merge_if_pipeline_succeeds) expect(json_response['allow_merge_on_skipped_pipeline']).to eq(project.allow_merge_on_skipped_pipeline) + expect(json_response['restrict_user_defined_variables']).to eq(project.restrict_user_defined_variables?) expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved) expect(json_response['operations_access_level']).to be_present end @@ -1654,6 +1655,7 @@ RSpec.describe API::Projects do expect(json_response['shared_with_groups'][0]).to have_key('expires_at') expect(json_response['only_allow_merge_if_pipeline_succeeds']).to eq(project.only_allow_merge_if_pipeline_succeeds) expect(json_response['allow_merge_on_skipped_pipeline']).to eq(project.allow_merge_on_skipped_pipeline) + expect(json_response['restrict_user_defined_variables']).to eq(project.restrict_user_defined_variables?) expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved) expect(json_response['ci_default_git_depth']).to eq(project.ci_default_git_depth) expect(json_response['ci_forward_deployment_enabled']).to eq(project.ci_forward_deployment_enabled) @@ -2597,6 +2599,18 @@ RSpec.describe API::Projects do expect(response).to have_gitlab_http_status(:bad_request) end + it 'updates restrict_user_defined_variables', :aggregate_failures do + project_param = { restrict_user_defined_variables: true } + + put api("/projects/#{project3.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(:ok) + + project_param.each_pair do |k, v| + expect(json_response[k.to_s]).to eq(v) + end + end + it 'updates avatar' do project_param = { avatar: fixture_file_upload('spec/fixtures/banana_sample.gif', @@ -2711,6 +2725,22 @@ RSpec.describe API::Projects do expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']['container_expiration_policy.name_regex_keep']).to contain_exactly('not valid RE2 syntax: missing ]: [') end + + it "doesn't update container_expiration_policy with invalid keep_n" do + project_param = { + container_expiration_policy_attributes: { + cadence: '1month', + enabled: true, + keep_n: 'not_int', + name_regex_keep: 'foo.*' + } + } + + put api("/projects/#{project3.id}", user4), params: project_param + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('container_expiration_policy_attributes[keep_n] is invalid') + end end context 'when authenticated as project developer' do @@ -3364,10 +3394,10 @@ RSpec.describe API::Projects do end describe 'POST /projects/:id/housekeeping' do - let(:housekeeping) { Projects::HousekeepingService.new(project) } + let(:housekeeping) { Repositories::HousekeepingService.new(project) } before do - allow(Projects::HousekeepingService).to receive(:new).with(project, :gc).and_return(housekeeping) + allow(Repositories::HousekeepingService).to receive(:new).with(project, :gc).and_return(housekeeping) end context 'when authenticated as owner' do @@ -3381,12 +3411,12 @@ RSpec.describe API::Projects do context 'when housekeeping lease is taken' do it 'returns conflict' do - expect(housekeeping).to receive(:execute).once.and_raise(Projects::HousekeepingService::LeaseTaken) + expect(housekeeping).to receive(:execute).once.and_raise(Repositories::HousekeepingService::LeaseTaken) post api("/projects/#{project.id}/housekeeping", user) expect(response).to have_gitlab_http_status(:conflict) - expect(json_response['message']).to match(/Somebody already triggered housekeeping for this project/) + expect(json_response['message']).to match(/Somebody already triggered housekeeping for this resource/) end end end diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 58b321a255e..70de2e5330b 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -16,9 +16,6 @@ RSpec.describe API::Releases do project.add_reporter(reporter) project.add_guest(guest) project.add_developer(developer) - - project.repository.add_tag(maintainer, 'v0.1', commit.id) - project.repository.add_tag(maintainer, 'v0.2', commit.id) end describe 'GET /projects/:id/releases' do @@ -294,6 +291,25 @@ RSpec.describe API::Releases do end end + context 'when release is associated to mutiple milestones' do + context 'milestones order' do + let_it_be(:project) { create(:project, :repository, :public) } + let_it_be_with_reload(:release_with_milestones) { create(:release, tag: 'v3.14', project: project) } + + let(:actual_milestone_title_order) do + get api("/projects/#{project.id}/releases/#{release_with_milestones.tag}", non_project_member) + + json_response['milestones'].map { |m| m['title'] } + end + + before do + release_with_milestones.update!(milestones: [milestone_2, milestone_1]) + end + + it_behaves_like 'correct release milestone order' + end + end + context 'when release has link asset' do let!(:link) do create(:release_link, @@ -461,6 +477,10 @@ RSpec.describe API::Releases do } end + before do + initialize_tags + end + it 'accepts the request' do post api("/projects/#{project.id}/releases", maintainer), params: params @@ -858,6 +878,10 @@ RSpec.describe API::Releases do description: 'Super nice release') end + before do + initialize_tags + end + it 'accepts the request' do put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params @@ -1108,4 +1132,9 @@ RSpec.describe API::Releases do end end end + + def initialize_tags + project.repository.add_tag(maintainer, 'v0.1', commit.id) + project.repository.add_tag(maintainer, 'v0.2', commit.id) + end end diff --git a/spec/requests/api/snippet_repository_storage_moves_spec.rb b/spec/requests/api/snippet_repository_storage_moves_spec.rb new file mode 100644 index 00000000000..edb92569823 --- /dev/null +++ b/spec/requests/api/snippet_repository_storage_moves_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::SnippetRepositoryStorageMoves do + it_behaves_like 'repository_storage_moves API', 'snippets' do + let_it_be(:container) { create(:snippet, :repository).tap { |snippet| snippet.create_repository } } + let_it_be(:storage_move) { create(:snippet_repository_storage_move, :scheduled, container: container) } + let(:repository_storage_move_factory) { :snippet_repository_storage_move } + let(:bulk_worker_klass) { SnippetScheduleBulkRepositoryShardMovesWorker } + end +end diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb index 0fa088a641e..bfdb5458fd1 100644 --- a/spec/requests/api/terraform/state_spec.rb +++ b/spec/requests/api/terraform/state_spec.rb @@ -21,9 +21,36 @@ RSpec.describe API::Terraform::State do stub_terraform_state_object_storage end + shared_examples 'endpoint with unique user tracking' do + context 'without authentication' do + let(:auth_header) { basic_auth_header('bad', 'token') } + + before do + stub_feature_flags(usage_data_p_terraform_state_api_unique_users: false) + end + + it 'does not track unique event' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) + + request + end + end + + context 'with maintainer permissions' do + let(:current_user) { maintainer } + + it_behaves_like 'tracking unique hll events', :usage_data_p_terraform_state_api_unique_users do + let(:target_id) { 'p_terraform_state_api_unique_users' } + let(:expected_type) { instance_of(Integer) } + end + end + end + describe 'GET /projects/:id/terraform/state/:name' do subject(:request) { get api(state_path), headers: auth_header } + it_behaves_like 'endpoint with unique user tracking' + context 'without authentication' do let(:auth_header) { basic_auth_header('bad', 'token') } @@ -117,6 +144,8 @@ RSpec.describe API::Terraform::State do subject(:request) { post api(state_path), headers: auth_header, as: :json, params: params } + it_behaves_like 'endpoint with unique user tracking' + context 'when terraform state with a given name is already present' do context 'with maintainer permissions' do let(:current_user) { maintainer } @@ -219,6 +248,8 @@ RSpec.describe API::Terraform::State do describe 'DELETE /projects/:id/terraform/state/:name' do subject(:request) { delete api(state_path), headers: auth_header } + it_behaves_like 'endpoint with unique user tracking' + context 'with maintainer permissions' do let(:current_user) { maintainer } @@ -256,6 +287,8 @@ 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 'locks the terraform state' do request @@ -305,6 +338,10 @@ RSpec.describe API::Terraform::State do subject(:request) { delete api("#{state_path}/lock"), headers: auth_header, params: params } + it_behaves_like 'endpoint with unique user tracking' 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/todos_spec.rb b/spec/requests/api/todos_spec.rb index bc315c5b3c6..eaffa49fc9d 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -302,6 +302,8 @@ RSpec.describe API::Todos do end it 'returns 304 there already exist a todo on that issuable' do + stub_feature_flags(multiple_todos: false) + create(:todo, project: project_1, author: author_1, user: john_doe, target: issuable) post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.iid}/todo", john_doe) diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 2cd1483f486..94fba451860 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -368,6 +368,16 @@ RSpec.describe API::Users do expect(json_response.map { |u| u['id'] }).not_to include(internal_user.id) end end + + context 'admins param' do + it 'returns all users' do + get api("/users?admins=true", user) + + expect(response).to match_response_schema('public_api/v4/user/basics') + expect(json_response.size).to eq(2) + expect(json_response.map { |u| u['id'] }).to include(user.id, admin.id) + end + end end context "when admin" do @@ -487,6 +497,16 @@ RSpec.describe API::Users do expect(response).to have_gitlab_http_status(:bad_request) end end + + context 'admins param' do + it 'returns only admins' do + get api("/users?admins=true", admin) + + expect(response).to match_response_schema('public_api/v4/user/basics') + expect(json_response.size).to eq(1) + expect(json_response.first['id']).to eq(admin.id) + end + end end describe "GET /users/:id" do @@ -2368,7 +2388,7 @@ RSpec.describe API::Users do activate expect(response).to have_gitlab_http_status(:forbidden) - expect(json_response['message']).to eq('403 Forbidden - A blocked user must be unblocked to be activated') + expect(json_response['message']).to eq('403 Forbidden - A blocked user must be unblocked to be activated') expect(user.reload.state).to eq('blocked') end end @@ -2382,7 +2402,7 @@ RSpec.describe API::Users do activate expect(response).to have_gitlab_http_status(:forbidden) - expect(json_response['message']).to eq('403 Forbidden - A blocked user must be unblocked to be activated') + expect(json_response['message']).to eq('403 Forbidden - A blocked user must be unblocked to be activated') expect(user.reload.state).to eq('ldap_blocked') end end @@ -2439,7 +2459,7 @@ RSpec.describe API::Users do deactivate expect(response).to have_gitlab_http_status(:forbidden) - expect(json_response['message']).to eq("403 Forbidden - The user you are trying to deactivate has been active in the past #{::User::MINIMUM_INACTIVE_DAYS} days and cannot be deactivated") + expect(json_response['message']).to eq("403 Forbidden - The user you are trying to deactivate has been active in the past #{::User::MINIMUM_INACTIVE_DAYS} days and cannot be deactivated") expect(user.reload.state).to eq('active') end end @@ -2467,7 +2487,7 @@ RSpec.describe API::Users do deactivate expect(response).to have_gitlab_http_status(:forbidden) - expect(json_response['message']).to eq('403 Forbidden - A blocked user cannot be deactivated by the API') + expect(json_response['message']).to eq('403 Forbidden - A blocked user cannot be deactivated by the API') expect(user.reload.state).to eq('blocked') end end @@ -2481,7 +2501,7 @@ RSpec.describe API::Users do deactivate expect(response).to have_gitlab_http_status(:forbidden) - expect(json_response['message']).to eq('403 Forbidden - A blocked user cannot be deactivated by the API') + expect(json_response['message']).to eq('403 Forbidden - A blocked user cannot be deactivated by the API') expect(user.reload.state).to eq('ldap_blocked') end end @@ -2493,7 +2513,7 @@ RSpec.describe API::Users do deactivate expect(response).to have_gitlab_http_status(:forbidden) - expect(json_response['message']).to eq('403 Forbidden - An internal user cannot be deactivated by the API') + expect(json_response['message']).to eq('403 Forbidden - An internal user cannot be deactivated by the API') end end @@ -2853,115 +2873,91 @@ RSpec.describe API::Users do let(:expires_at) { 3.days.from_now.to_date.to_s } let(:scopes) { %w(api read_user) } - context 'when feature flag is enabled' do - before do - stub_feature_flags(pat_creation_api_for_admin: true) - end - - it 'returns error if required attributes are missing' do - post api("/users/#{user.id}/personal_access_tokens", admin) - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('name is missing, scopes is missing, scopes does not have a valid value') - end - - it 'returns a 404 error if user not found' do - post api("/users/#{non_existing_record_id}/personal_access_tokens", admin), - params: { - name: name, - scopes: scopes, - expires_at: expires_at - } + it 'returns error if required attributes are missing' do + post api("/users/#{user.id}/personal_access_tokens", admin) - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 User Not Found') - end + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('name is missing, scopes is missing, scopes does not have a valid value') + end - it 'returns a 401 error when not authenticated' do - post api("/users/#{user.id}/personal_access_tokens"), - params: { - name: name, - scopes: scopes, - expires_at: expires_at - } + it 'returns a 404 error if user not found' do + post api("/users/#{non_existing_record_id}/personal_access_tokens", admin), + params: { + name: name, + scopes: scopes, + expires_at: expires_at + } - expect(response).to have_gitlab_http_status(:unauthorized) - expect(json_response['message']).to eq('401 Unauthorized') - end + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 User Not Found') + end - it 'returns a 403 error when authenticated as normal user' do - post api("/users/#{user.id}/personal_access_tokens", user), - params: { - name: name, - scopes: scopes, - expires_at: expires_at - } + it 'returns a 401 error when not authenticated' do + post api("/users/#{user.id}/personal_access_tokens"), + params: { + name: name, + scopes: scopes, + expires_at: expires_at + } - expect(response).to have_gitlab_http_status(:forbidden) - expect(json_response['message']).to eq('403 Forbidden') - end + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['message']).to eq('401 Unauthorized') + end - it 'creates a personal access token when authenticated as admin' do - post api("/users/#{user.id}/personal_access_tokens", admin), - params: { - name: name, - expires_at: expires_at, - scopes: scopes - } + it 'returns a 403 error when authenticated as normal user' do + post api("/users/#{user.id}/personal_access_tokens", user), + params: { + name: name, + scopes: scopes, + expires_at: expires_at + } - expect(response).to have_gitlab_http_status(:created) - expect(json_response['name']).to eq(name) - expect(json_response['scopes']).to eq(scopes) - expect(json_response['expires_at']).to eq(expires_at) - expect(json_response['id']).to be_present - expect(json_response['created_at']).to be_present - expect(json_response['active']).to be_truthy - expect(json_response['revoked']).to be_falsey - expect(json_response['token']).to be_present - end + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('403 Forbidden') + end - context 'when an error is thrown by the model' do - let!(:admin_personal_access_token) { create(:personal_access_token, user: admin) } - let(:error_message) { 'error message' } + it 'creates a personal access token when authenticated as admin' do + post api("/users/#{user.id}/personal_access_tokens", admin), + params: { + name: name, + expires_at: expires_at, + scopes: scopes + } - before do - allow_next_instance_of(PersonalAccessToken) do |personal_access_token| - allow(personal_access_token).to receive_message_chain(:errors, :full_messages) - .and_return([error_message]) + expect(response).to have_gitlab_http_status(:created) + expect(json_response['name']).to eq(name) + expect(json_response['scopes']).to eq(scopes) + expect(json_response['expires_at']).to eq(expires_at) + expect(json_response['id']).to be_present + expect(json_response['created_at']).to be_present + expect(json_response['active']).to be_truthy + expect(json_response['revoked']).to be_falsey + expect(json_response['token']).to be_present + end - allow(personal_access_token).to receive(:save).and_return(false) - end - end + context 'when an error is thrown by the model' do + let!(:admin_personal_access_token) { create(:personal_access_token, user: admin) } + let(:error_message) { 'error message' } - it 'returns the error' do - post api("/users/#{user.id}/personal_access_tokens", personal_access_token: admin_personal_access_token), - params: { - name: name, - expires_at: expires_at, - scopes: scopes - } + before do + allow_next_instance_of(PersonalAccessToken) do |personal_access_token| + allow(personal_access_token).to receive_message_chain(:errors, :full_messages) + .and_return([error_message]) - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq(error_message) + allow(personal_access_token).to receive(:save).and_return(false) end end - end - context 'when feature flag is disabled' do - before do - stub_feature_flags(pat_creation_api_for_admin: false) - end - - it 'returns a 404' do - post api("/users/#{user.id}/personal_access_tokens", admin), + it 'returns the error' do + post api("/users/#{user.id}/personal_access_tokens", personal_access_token: admin_personal_access_token), params: { name: name, expires_at: expires_at, scopes: scopes } - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 Not Found') + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq(error_message) end end end diff --git a/spec/requests/jwks_controller_spec.rb b/spec/requests/jwks_controller_spec.rb new file mode 100644 index 00000000000..5eda1979027 --- /dev/null +++ b/spec/requests/jwks_controller_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JwksController do + describe 'GET /-/jwks' do + let(:ci_jwt_signing_key) { OpenSSL::PKey::RSA.generate(1024) } + let(:ci_jwk) { ci_jwt_signing_key.to_jwk } + let(:oidc_jwk) { OpenSSL::PKey::RSA.new(Rails.application.secrets.openid_connect_signing_key).to_jwk } + + before do + stub_application_setting(ci_jwt_signing_key: ci_jwt_signing_key.to_s) + end + + it 'returns signing keys used to sign CI_JOB_JWT' do + get jwks_url + + expect(response).to have_gitlab_http_status(:ok) + + ids = json_response['keys'].map { |jwk| jwk['kid'] } + expect(ids).to contain_exactly(ci_jwk['kid'], oidc_jwk['kid']) + end + + it 'does not leak private key data' do + get jwks_url + + aggregate_failures do + json_response['keys'].each do |jwk| + expect(jwk.keys).to contain_exactly('kty', 'kid', 'e', 'n', 'use', 'alg') + expect(jwk['use']).to eq('sig') + expect(jwk['alg']).to eq('RS256') + end + end + end + end +end diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 535d511a459..aeec18aee2b 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -193,10 +193,8 @@ RSpec.describe 'Git LFS API and storage' do subject(:request) { post_lfs_json batch_url(project), body, headers } let(:response) { request && super() } - let(:lfs_chunked_encoding) { true } before do - stub_feature_flags(lfs_chunked_encoding: lfs_chunked_encoding) project.lfs_objects << lfs_object end @@ -480,20 +478,6 @@ RSpec.describe 'Git LFS API and storage' do expect(headers['Transfer-Encoding']).to eq('chunked') end - context 'when lfs_chunked_encoding feature is disabled' do - let(:lfs_chunked_encoding) { false } - - it 'responds with upload hypermedia link' do - expect(json_response['objects']).to be_kind_of(Array) - expect(json_response['objects'].first).to include(sample_object) - expect(json_response['objects'].first['actions']['upload']['href']).to eq(objects_url(project, sample_oid, sample_size)) - - headers = json_response['objects'].first['actions']['upload']['header'] - expect(headers['Content-Type']).to eq('application/octet-stream') - expect(headers['Transfer-Encoding']).to be_nil - end - end - it_behaves_like 'process authorization header', renew_authorization: renew_authorization end diff --git a/spec/requests/profiles/notifications_controller_spec.rb b/spec/requests/profiles/notifications_controller_spec.rb index 87669b3594c..d7dfb1c675d 100644 --- a/spec/requests/profiles/notifications_controller_spec.rb +++ b/spec/requests/profiles/notifications_controller_spec.rb @@ -24,7 +24,7 @@ RSpec.describe 'view user notifications' do get profile_notifications_path end - describe 'GET /profile/notifications' do + describe 'GET /-/profile/notifications' do it 'does not have an N+1 due to an additional groups (with no parent group)' do get_profile_notifications diff --git a/spec/requests/projects/cycle_analytics_events_spec.rb b/spec/requests/projects/cycle_analytics_events_spec.rb index f8fa9459467..565576f3091 100644 --- a/spec/requests/projects/cycle_analytics_events_spec.rb +++ b/spec/requests/projects/cycle_analytics_events_spec.rb @@ -7,113 +7,95 @@ RSpec.describe 'value stream analytics events' do let(:project) { create(:project, :repository, public_builds: false) } let(:issue) { create(:issue, project: project, created_at: 2.days.ago) } - shared_examples 'value stream analytics events examples' do - describe 'GET /:namespace/:project/value_stream_analytics/events/issues' do - before do - project.add_developer(user) + describe 'GET /:namespace/:project/value_stream_analytics/events/issues' do + before do + project.add_developer(user) - 3.times do |count| - travel_to(Time.now + count.days) do - create_cycle - end + 3.times do |count| + travel_to(Time.now + count.days) do + create_cycle end - - deploy_master(user, project) - - login_as(user) end - it 'lists the issue events' do - get project_cycle_analytics_issue_path(project, format: :json) + deploy_master(user, project) - first_issue_iid = project.issues.sort_by_attribute(:created_desc).pluck(:iid).first.to_s + login_as(user) + end - expect(json_response['events']).not_to be_empty - expect(json_response['events'].first['iid']).to eq(first_issue_iid) - end + it 'lists the issue events' do + get project_cycle_analytics_issue_path(project, format: :json) - it 'lists the plan events' do - get project_cycle_analytics_plan_path(project, format: :json) + first_issue_iid = project.issues.sort_by_attribute(:created_desc).pluck(:iid).first.to_s - first_issue_iid = project.issues.sort_by_attribute(:created_desc).pluck(:iid).first.to_s + expect(json_response['events']).not_to be_empty + expect(json_response['events'].first['iid']).to eq(first_issue_iid) + end - expect(json_response['events']).not_to be_empty - expect(json_response['events'].first['iid']).to eq(first_issue_iid) - end + it 'lists the plan events' do + get project_cycle_analytics_plan_path(project, format: :json) - it 'lists the code events' do - get project_cycle_analytics_code_path(project, format: :json) + first_issue_iid = project.issues.sort_by_attribute(:created_desc).pluck(:iid).first.to_s - expect(json_response['events']).not_to be_empty + expect(json_response['events']).not_to be_empty + expect(json_response['events'].first['iid']).to eq(first_issue_iid) + end - first_mr_iid = project.merge_requests.sort_by_attribute(:created_desc).pluck(:iid).first.to_s + it 'lists the code events' do + get project_cycle_analytics_code_path(project, format: :json) - expect(json_response['events'].first['iid']).to eq(first_mr_iid) - end + expect(json_response['events']).not_to be_empty - it 'lists the test events', :sidekiq_inline do - get project_cycle_analytics_test_path(project, format: :json) + first_mr_iid = project.merge_requests.sort_by_attribute(:created_desc).pluck(:iid).first.to_s - expect(json_response['events']).not_to be_empty - expect(json_response['events'].first['date']).not_to be_empty - end + expect(json_response['events'].first['iid']).to eq(first_mr_iid) + end - it 'lists the review events' do - get project_cycle_analytics_review_path(project, format: :json) + it 'lists the test events', :sidekiq_inline do + get project_cycle_analytics_test_path(project, format: :json) - first_mr_iid = project.merge_requests.sort_by_attribute(:created_desc).pluck(:iid).first.to_s + expect(json_response['events']).not_to be_empty + expect(json_response['events'].first['date']).not_to be_empty + end - expect(json_response['events']).not_to be_empty - expect(json_response['events'].first['iid']).to eq(first_mr_iid) - end + it 'lists the review events' do + get project_cycle_analytics_review_path(project, format: :json) - it 'lists the staging events', :sidekiq_inline do - get project_cycle_analytics_staging_path(project, format: :json) + first_mr_iid = project.merge_requests.sort_by_attribute(:created_desc).pluck(:iid).first.to_s - expect(json_response['events']).not_to be_empty - expect(json_response['events'].first['date']).not_to be_empty - end + expect(json_response['events']).not_to be_empty + expect(json_response['events'].first['iid']).to eq(first_mr_iid) + end - context 'with private project and builds' do - before do - project.members.last.update(access_level: Gitlab::Access::GUEST) - end + it 'lists the staging events', :sidekiq_inline do + get project_cycle_analytics_staging_path(project, format: :json) - it 'does not list the test events' do - get project_cycle_analytics_test_path(project, format: :json) + expect(json_response['events']).not_to be_empty + expect(json_response['events'].first['date']).not_to be_empty + end - expect(response).to have_gitlab_http_status(:not_found) - end + context 'with private project and builds' do + before do + project.members.last.update(access_level: Gitlab::Access::GUEST) + end - it 'does not list the staging events' do - get project_cycle_analytics_staging_path(project, format: :json) + it 'does not list the test events' do + get project_cycle_analytics_test_path(project, format: :json) - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:not_found) + end - it 'lists the issue events' do - get project_cycle_analytics_issue_path(project, format: :json) + it 'does not list the staging events' do + get project_cycle_analytics_staging_path(project, format: :json) - expect(response).to have_gitlab_http_status(:ok) - end + expect(response).to have_gitlab_http_status(:not_found) end - end - end - describe 'when new_project_level_vsa_backend feature flag is off' do - before do - stub_feature_flags(new_project_level_vsa_backend: false, thing: project) - end - - it_behaves_like 'value stream analytics events examples' - end + it 'lists the issue events' do + get project_cycle_analytics_issue_path(project, format: :json) - describe 'when new_project_level_vsa_backend feature flag is on' do - before do - stub_feature_flags(new_project_level_vsa_backend: true, thing: project) + expect(response).to have_gitlab_http_status(:ok) + end end - - it_behaves_like 'value stream analytics events examples' end def create_cycle diff --git a/spec/requests/projects/incident_management/pagerduty_incidents_spec.rb b/spec/requests/projects/incident_management/pagerduty_incidents_spec.rb index b18bffdb110..32434435475 100644 --- a/spec/requests/projects/incident_management/pagerduty_incidents_spec.rb +++ b/spec/requests/projects/incident_management/pagerduty_incidents_spec.rb @@ -23,7 +23,7 @@ RSpec.describe 'PagerDuty webhook' do it 'calls PagerDuty webhook processor with correct parameters' do make_request - expect(webhook_processor_class).to have_received(:new).with(project, nil, payload) + expect(webhook_processor_class).to have_received(:new).with(project, payload) expect(webhook_processor).to have_received(:execute).with('VALID-TOKEN') end diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index c2e68df2c40..34f34c0b850 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -60,6 +60,24 @@ RSpec.describe 'Rack Attack global throttles' do expect_rejection { get url_that_does_not_require_authentication } end + context 'with custom response text' do + before do + stub_application_setting(rate_limiting_response_text: 'Custom response') + end + + it 'rejects requests over the rate limit' do + # At first, allow requests under the rate limit. + requests_per_period.times do + get url_that_does_not_require_authentication + expect(response).to have_gitlab_http_status(:ok) + end + + # the last straw + expect_rejection { get url_that_does_not_require_authentication } + expect(response.body).to eq("Custom response\n") + end + end + it 'allows requests after throttling and then waiting for the next period' do requests_per_period.times do get url_that_does_not_require_authentication @@ -170,16 +188,16 @@ RSpec.describe 'Rack Attack global throttles' do end describe 'API requests authenticated with personal access token', :api do - let(:user) { create(:user) } - let(:token) { create(:personal_access_token, user: user) } - let(:other_user) { create(:user) } - let(:other_user_token) { create(:personal_access_token, user: other_user) } + let_it_be(:user) { create(:user) } + let_it_be(:token) { create(:personal_access_token, user: user) } + let_it_be(:other_user) { create(:user) } + let_it_be(:other_user_token) { create(:personal_access_token, user: other_user) } let(:throttle_setting_prefix) { 'throttle_authenticated_api' } let(:api_partial_url) { '/todos' } context 'with the token in the query string' do - let(:request_args) { [api(api_partial_url, personal_access_token: token)] } - let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token)] } + let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } + let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } it_behaves_like 'rate-limited token-authenticated requests' end @@ -190,6 +208,41 @@ RSpec.describe 'Rack Attack global throttles' do it_behaves_like 'rate-limited token-authenticated requests' end + + context 'with the token in the OAuth headers' do + let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(token)) } + let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_token)) } + + it_behaves_like 'rate-limited token-authenticated requests' + end + + context 'with the token in basic auth' do + let(:request_args) { api_get_args_with_token_headers(api_partial_url, basic_auth_headers(user, token)) } + let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, basic_auth_headers(other_user, other_user_token)) } + + it_behaves_like 'rate-limited token-authenticated requests' + end + + context 'with a read_api scope' do + before do + token.update!(scopes: ['read_api']) + other_user_token.update!(scopes: ['read_api']) + end + + context 'with the token in the headers' do + let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } + let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } + + it_behaves_like 'rate-limited token-authenticated requests' + end + + context 'with the token in the OAuth headers' do + let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(token)) } + let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_token)) } + + it_behaves_like 'rate-limited token-authenticated requests' + end + end end describe 'API requests authenticated with OAuth token', :api do @@ -205,8 +258,8 @@ RSpec.describe 'Rack Attack global throttles' do let(:api_partial_url) { '/todos' } context 'with the token in the query string' do - let(:request_args) { [api(api_partial_url, oauth_access_token: token)] } - let(:other_user_request_args) { [api(api_partial_url, oauth_access_token: other_user_token)] } + let(:request_args) { [api(api_partial_url, oauth_access_token: token), {}] } + let(:other_user_request_args) { [api(api_partial_url, oauth_access_token: other_user_token), {}] } it_behaves_like 'rate-limited token-authenticated requests' end @@ -217,6 +270,15 @@ RSpec.describe 'Rack Attack global throttles' do it_behaves_like 'rate-limited token-authenticated requests' end + + context 'with a read_api scope' do + let(:read_token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "read_api") } + let(:other_user_read_token) { Doorkeeper::AccessToken.create!(application_id: other_user_application.id, resource_owner_id: other_user.id, scopes: "read_api") } + let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(read_token)) } + let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_read_token)) } + + it_behaves_like 'rate-limited token-authenticated requests' + end end describe '"web" (non-API) requests authenticated with RSS token' do @@ -314,8 +376,8 @@ RSpec.describe 'Rack Attack global throttles' do end context 'with the token in the query string' do - let(:request_args) { [api(api_partial_url, personal_access_token: token)] } - let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token)] } + let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } + let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } it_behaves_like 'rate-limited token-authenticated requests' end diff --git a/spec/requests/runner_setup_controller_spec.rb b/spec/requests/runner_setup_controller_spec.rb new file mode 100644 index 00000000000..665c896e30d --- /dev/null +++ b/spec/requests/runner_setup_controller_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RunnerSetupController do + let(:user) { create(:user) } + + before do + sign_in(user) + end + + describe 'GET /-/runner_setup/platforms' do + it 'renders the platforms' do + get runner_setup_platforms_url + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to have_key("windows") + expect(json_response).to have_key("kubernetes") + end + end +end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb new file mode 100644 index 00000000000..b224ef87229 --- /dev/null +++ b/spec/requests/users_controller_spec.rb @@ -0,0 +1,833 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe UsersController do + # This user should have the same e-mail address associated with the GPG key prepared for tests + let(:user) { create(:user, email: GpgHelpers::User1.emails[0]) } + let(:private_user) { create(:user, private_profile: true) } + let(:public_user) { create(:user) } + + describe 'GET #show' do + shared_examples_for 'renders the show template' do + it 'renders the show template' do + get user_url user.username + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('show') + end + end + + context 'when the user exists and has public visibility' do + context 'when logged in' do + before do + sign_in(user) + end + + it_behaves_like 'renders the show template' + end + + context 'when logged out' do + it_behaves_like 'renders the show template' + end + end + + context 'when public visibility level is restricted' do + before do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + end + + context 'when logged out' do + it 'redirects to login page' do + get user_url user.username + + expect(response).to redirect_to new_user_session_path + end + end + + context 'when logged in' do + before do + sign_in(user) + end + + it_behaves_like 'renders the show template' + end + end + + context 'when a user by that username does not exist' do + context 'when logged out' do + it 'redirects to login page' do + get user_url 'nonexistent' + + expect(response).to redirect_to new_user_session_path + end + end + + context 'when logged in' do + before do + sign_in(user) + end + + it 'renders 404' do + get user_url 'nonexistent' + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'requested in json format' do + let(:project) { create(:project) } + + before do + project.add_developer(user) + Gitlab::DataBuilder::Push.build_sample(project, user) + + sign_in(user) + end + + it 'returns 404 with deprecation message' do + # Requesting "/username?format=json" instead of "/username.json" + get user_url user.username, params: { format: :json } + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.media_type).to eq('application/json') + expect(Gitlab::Json.parse(response.body)['message']).to include('This endpoint is deprecated.') + end + end + end + + describe 'GET /users/:username (deprecated user top)' do + it 'redirects to /user1' do + get '/users/user1' + + expect(response).to redirect_to user_path('user1') + end + end + + describe 'GET #activity' do + shared_examples_for 'renders the show template' do + it 'renders the show template' do + get user_activity_url user.username + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('show') + end + end + + context 'when the user exists and has public visibility' do + context 'when logged in' do + before do + sign_in(user) + end + + it_behaves_like 'renders the show template' + end + + context 'when logged out' do + it_behaves_like 'renders the show template' + end + end + + context 'when public visibility level is restricted' do + before do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + end + + context 'when logged out' do + it 'redirects to login page' do + get user_activity_url user.username + + expect(response).to redirect_to new_user_session_path + end + end + + context 'when logged in' do + before do + sign_in(user) + end + + it_behaves_like 'renders the show template' + end + end + + context 'when a user by that username does not exist' do + context 'when logged out' do + it 'redirects to login page' do + get user_activity_url 'nonexistent' + + expect(response).to redirect_to new_user_session_path + end + end + + context 'when logged in' do + before do + sign_in(user) + end + + it 'renders 404' do + get user_activity_url 'nonexistent' + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'requested in json format' do + let(:project) { create(:project) } + + before do + project.add_developer(user) + Gitlab::DataBuilder::Push.build_sample(project, user) + + sign_in(user) + end + + it 'loads events' do + get user_activity_url user.username, format: :json + + expect(response.media_type).to eq('application/json') + expect(Gitlab::Json.parse(response.body)['count']).to eq(1) + end + + it 'hides events if the user cannot read cross project' do + allow(Ability).to receive(:allowed?).and_call_original + expect(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } + + get user_activity_url user.username, format: :json + + expect(response.media_type).to eq('application/json') + expect(Gitlab::Json.parse(response.body)['count']).to eq(0) + end + + it 'hides events if the user has a private profile' do + Gitlab::DataBuilder::Push.build_sample(project, private_user) + + get user_activity_url private_user.username, format: :json + + expect(response.media_type).to eq('application/json') + expect(Gitlab::Json.parse(response.body)['count']).to eq(0) + end + end + end + + describe 'GET #ssh_keys' do + context 'non existent user' do + it 'does not generally work' do + get '/not-existent.keys' + + expect(response).not_to be_successful + end + end + + context 'user with no keys' do + it 'responds the empty body with text/plain content type' do + get "/#{user.username}.keys" + + expect(response).to be_successful + expect(response.media_type).to eq("text/plain") + expect(response.body).to eq("") + end + end + + context 'user with keys' do + let!(:key) { create(:key, user: user) } + let!(:another_key) { create(:another_key, user: user) } + let!(:deploy_key) { create(:deploy_key, user: user) } + + shared_examples_for 'renders all public keys' do + it 'renders all non-deploy keys separated with a new line with text/plain content type without the comment key' do + get "/#{user.username}.keys" + + expect(response).to be_successful + expect(response.media_type).to eq("text/plain") + + expect(response.body).not_to eq('') + expect(response.body).to eq(user.all_ssh_keys.join("\n")) + + expect(response.body).to include(key.key.sub(' dummy@gitlab.com', '')) + expect(response.body).to include(another_key.key.sub(' dummy@gitlab.com', '')) + + expect(response.body).not_to match(/dummy@gitlab.com/) + + expect(response.body).not_to include(deploy_key.key) + end + end + + context 'while signed in' do + before do + sign_in(user) + end + + it_behaves_like 'renders all public keys' + end + + context 'when logged out' do + before do + sign_out(user) + end + + it_behaves_like 'renders all public keys' + end + end + end + + describe 'GET #gpg_keys' do + context 'non existent user' do + it 'does not generally work' do + get '/not-existent.keys' + + expect(response).not_to be_successful + end + end + + context 'user with no keys' do + it 'responds the empty body with text/plain content type' do + get "/#{user.username}.gpg" + + expect(response).to be_successful + expect(response.media_type).to eq("text/plain") + expect(response.body).to eq("") + end + end + + context 'user with keys' do + let!(:gpg_key) { create(:gpg_key, user: user) } + let!(:another_gpg_key) { create(:another_gpg_key, user: user) } + + shared_examples_for 'renders all verified GPG keys' do + it 'renders all verified keys separated with a new line with text/plain content type' do + get "/#{user.username}.gpg" + + expect(response).to be_successful + + expect(response.media_type).to eq("text/plain") + + expect(response.body).not_to eq('') + expect(response.body).to eq(user.gpg_keys.select(&:verified?).map(&:key).join("\n")) + + expect(response.body).to include(gpg_key.key) + expect(response.body).to include(another_gpg_key.key) + end + end + + context 'while signed in' do + before do + sign_in(user) + end + + it_behaves_like 'renders all verified GPG keys' + end + + context 'when logged out' do + before do + sign_out(user) + end + + it_behaves_like 'renders all verified GPG keys' + end + + context 'when revoked' do + shared_examples_for 'doesn\'t render revoked keys' do + it 'doesn\'t render revoked keys' do + get "/#{user.username}.gpg" + + expect(response.body).not_to eq('') + + expect(response.body).to include(gpg_key.key) + expect(response.body).not_to include(another_gpg_key.key) + end + end + + before do + sign_in(user) + another_gpg_key.revoke + end + + context 'while signed in' do + it_behaves_like 'doesn\'t render revoked keys' + end + + context 'when logged out' do + before do + sign_out(user) + end + + it_behaves_like 'doesn\'t render revoked keys' + end + end + end + end + + describe 'GET #calendar' do + context 'for user' do + let(:project) { create(:project) } + + before do + sign_in(user) + project.add_developer(user) + end + + context 'with public profile' do + it 'renders calendar' do + push_data = Gitlab::DataBuilder::Push.build_sample(project, public_user) + EventCreateService.new.push(project, public_user, push_data) + + get user_calendar_url public_user.username, format: :json + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'with private profile' do + it 'does not render calendar' do + push_data = Gitlab::DataBuilder::Push.build_sample(project, private_user) + EventCreateService.new.push(project, private_user, push_data) + + get user_calendar_url private_user.username, format: :json + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'forked project' do + let(:project) { create(:project) } + let(:forked_project) { Projects::ForkService.new(project, user).execute } + + before do + sign_in(user) + project.add_developer(user) + + push_data = Gitlab::DataBuilder::Push.build_sample(project, user) + + fork_push_data = Gitlab::DataBuilder::Push + .build_sample(forked_project, user) + + EventCreateService.new.push(project, user, push_data) + EventCreateService.new.push(forked_project, user, fork_push_data) + end + + it 'includes forked projects' do + get user_calendar_url user.username + + expect(assigns(:contributions_calendar).projects.count).to eq(2) + end + end + end + + describe 'GET #calendar_activities' do + let!(:project) { create(:project) } + let(:user) { create(:user) } + + before do + allow_next_instance_of(User) do |instance| + allow(instance).to receive(:contributed_projects_ids).and_return([project.id]) + end + + sign_in(user) + project.add_developer(user) + end + + it 'renders activities on the specified day' do + get user_calendar_activities_url user.username, date: '2014-07-31' + + expect(response.media_type).to eq('text/html') + expect(response.body).to include('Jul 31, 2014') + end + + context 'for user' do + context 'with public profile' do + let(:issue) { create(:issue, project: project, author: user) } + let(:note) { create(:note, noteable: issue, author: user, project: project) } + + before do + create_push_event + create_note_event + end + + it 'renders calendar_activities' do + get user_calendar_activities_url public_user.username + + expect(response.body).not_to be_empty + end + + it 'avoids N+1 queries', :request_store do + get user_calendar_activities_url public_user.username + + control = ActiveRecord::QueryRecorder.new { get user_calendar_activities_url public_user.username } + + create_push_event + create_note_event + + expect { get user_calendar_activities_url public_user.username }.not_to exceed_query_limit(control) + end + end + + context 'with private profile' do + it 'does not render calendar_activities' do + push_data = Gitlab::DataBuilder::Push.build_sample(project, private_user) + EventCreateService.new.push(project, private_user, push_data) + + get user_calendar_activities_url private_user.username + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'external authorization' do + subject { get user_calendar_activities_url user.username } + + it_behaves_like 'disabled when using an external authorization service' + end + + def create_push_event + push_data = Gitlab::DataBuilder::Push.build_sample(project, public_user) + EventCreateService.new.push(project, public_user, push_data) + end + + def create_note_event + EventCreateService.new.leave_note(note, public_user) + end + end + end + + describe 'GET #contributed' do + let(:project) { create(:project, :public) } + + subject do + get user_contributed_projects_url author.username, format: format + end + + before do + sign_in(user) + + project.add_developer(public_user) + project.add_developer(private_user) + create(:push_event, project: project, author: author) + + subject + end + + shared_examples_for 'renders contributed projects' do + it 'renders contributed projects' do + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).not_to be_empty + end + end + + %i(html json).each do |format| + context "format: #{format}" do + let(:format) { format } + + context 'with public profile' do + let(:author) { public_user } + + it_behaves_like 'renders contributed projects' + end + + context 'with private profile' do + let(:author) { private_user } + + it 'returns 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'with a user that has the ability to read private profiles', :enable_admin_mode do + let(:user) { create(:admin) } + + it_behaves_like 'renders contributed projects' + end + end + end + end + end + + describe 'GET #starred' do + let(:project) { create(:project, :public) } + + subject do + get user_starred_projects_url author.username, format: format + end + + before do + author.toggle_star(project) + + sign_in(user) + subject + end + + shared_examples_for 'renders starred projects' do + it 'renders starred projects' do + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).not_to be_empty + end + end + + %i(html json).each do |format| + context "format: #{format}" do + let(:format) { format } + + context 'with public profile' do + let(:author) { public_user } + + it_behaves_like 'renders starred projects' + end + + context 'with private profile' do + let(:author) { private_user } + + it 'returns 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'with a user that has the ability to read private profiles', :enable_admin_mode do + let(:user) { create(:admin) } + + it_behaves_like 'renders starred projects' + end + end + end + end + end + + describe 'GET #snippets' do + before do + sign_in(user) + end + + context 'format html' do + it 'renders snippets page' do + get user_snippets_url user.username + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('show') + end + end + + context 'format json' do + it 'response with snippets json data' do + get user_snippets_url user.username, format: :json + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to have_key('html') + end + end + + context 'external authorization' do + subject { get user_snippets_url user.username } + + it_behaves_like 'disabled when using an external authorization service' + end + end + + describe 'GET #exists' do + before do + sign_in(user) + end + + context 'when user exists' do + it 'returns JSON indicating the user exists' do + get user_exists_url user.username + + expected_json = { exists: true }.to_json + expect(response.body).to eq(expected_json) + end + + context 'when the casing is different' do + let(:user) { create(:user, username: 'CamelCaseUser') } + + it 'returns JSON indicating the user exists' do + get user_exists_url user.username.downcase + + expected_json = { exists: true }.to_json + expect(response.body).to eq(expected_json) + end + end + end + + context 'when the user does not exist' do + it 'returns JSON indicating the user does not exist' do + get user_exists_url 'foo' + + expected_json = { exists: false }.to_json + expect(response.body).to eq(expected_json) + end + + context 'when a user changed their username' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') } + + it 'returns JSON indicating a user by that username does not exist' do + get user_exists_url 'old-username' + + expected_json = { exists: false }.to_json + expect(response.body).to eq(expected_json) + end + end + end + end + + describe 'GET #suggests' do + context 'when user exists' do + it 'returns JSON indicating the user exists and a suggestion' do + get user_suggests_url user.username + + expected_json = { exists: true, suggests: ["#{user.username}1"] }.to_json + expect(response.body).to eq(expected_json) + end + + context 'when the casing is different' do + let(:user) { create(:user, username: 'CamelCaseUser') } + + it 'returns JSON indicating the user exists and a suggestion' do + get user_suggests_url user.username.downcase + + expected_json = { exists: true, suggests: ["#{user.username.downcase}1"] }.to_json + expect(response.body).to eq(expected_json) + end + end + end + + context 'when the user does not exist' do + it 'returns JSON indicating the user does not exist' do + get user_suggests_url 'foo' + + expected_json = { exists: false, suggests: [] }.to_json + expect(response.body).to eq(expected_json) + end + + context 'when a user changed their username' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') } + + it 'returns JSON indicating a user by that username does not exist' do + get user_suggests_url 'old-username' + + expected_json = { exists: false, suggests: [] }.to_json + expect(response.body).to eq(expected_json) + end + end + end + end + + describe '#ensure_canonical_path' do + before do + sign_in(user) + end + + context 'for a GET request' do + context 'when requesting users at the root path' do + context 'when requesting the canonical path' do + let(:user) { create(:user, username: 'CamelCaseUser') } + + context 'with exactly matching casing' do + it 'responds with success' do + get user_url user.username + + expect(response).to be_successful + end + end + + context 'with different casing' do + it 'redirects to the correct casing' do + get user_url user.username.downcase + + expect(response).to redirect_to(user) + expect(flash[:notice]).to be_nil + end + end + end + + shared_examples_for 'redirects to the canonical path' do + it 'redirects to the canonical path' do + get user_url redirect_route.path + + expect(response).to redirect_to(user) + expect(flash[:notice]).to eq(user_moved_message(redirect_route, user)) + end + end + + context 'when requesting a redirected path' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-path') } + + it_behaves_like 'redirects to the canonical path' + + context 'when the old path is a substring of the scheme or host' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'http') } + + # it does not modify the requested host and ... + it_behaves_like 'redirects to the canonical path' + end + + context 'when the old path is substring of users' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'ser') } + + it_behaves_like 'redirects to the canonical path' + end + end + end + + context 'when requesting users under the /users path' do + context 'when requesting the canonical path' do + let(:user) { create(:user, username: 'CamelCaseUser') } + + context 'with exactly matching casing' do + it 'responds with success' do + get user_projects_url user.username + + expect(response).to be_successful + end + end + + context 'with different casing' do + it 'redirects to the correct casing' do + get user_projects_url user.username.downcase + + expect(response).to redirect_to(user_projects_path(user)) + expect(flash[:notice]).to be_nil + end + end + end + + shared_examples_for 'redirects to the canonical path' do + it 'redirects to the canonical path' do + get user_projects_url redirect_route.path + + expect(response).to redirect_to(user_projects_path(user)) + expect(flash[:notice]).to eq(user_moved_message(redirect_route, user)) + end + end + + context 'when requesting a redirected path' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-path') } + + it_behaves_like 'redirects to the canonical path' + + context 'when the old path is a substring of the scheme or host' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'http') } + + # it does not modify the requested host and ... + it_behaves_like 'redirects to the canonical path' + end + + context 'when the old path is substring of users' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'ser') } + + # it does not modify the /users part of the path + # (i.e. /users/ser should not become /ufoos/ser) and ... + it_behaves_like 'redirects to the canonical path' + end + end + end + end + end + + context 'token authentication' do + let(:url) { user_url(user.username, format: :atom) } + + it_behaves_like 'authenticates sessionless user for the request spec', public: true + end + + def user_moved_message(redirect_route, user) + "User '#{redirect_route.path}' was moved to '#{user.full_path}'. Please update any links and bookmarks that may still have the old path." + end +end |