diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-10-21 07:08:36 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-10-21 07:08:36 +0000 |
commit | 48aff82709769b098321c738f3444b9bdaa694c6 (patch) | |
tree | e00c7c43e2d9b603a5a6af576b1685e400410dee /spec/requests/api | |
parent | 879f5329ee916a948223f8f43d77fba4da6cd028 (diff) | |
download | gitlab-ce-48aff82709769b098321c738f3444b9bdaa694c6.tar.gz |
Add latest changes from gitlab-org/gitlab@13-5-stable-eev13.5.0-rc42
Diffstat (limited to 'spec/requests/api')
73 files changed, 5604 insertions, 720 deletions
diff --git a/spec/requests/api/admin/instance_clusters_spec.rb b/spec/requests/api/admin/instance_clusters_spec.rb index b68541b5d92..9d0661089a9 100644 --- a/spec/requests/api/admin/instance_clusters_spec.rb +++ b/spec/requests/api/admin/instance_clusters_spec.rb @@ -162,6 +162,7 @@ RSpec.describe ::API::Admin::InstanceClusters do name: 'test-instance-cluster', domain: 'domain.example.com', managed: false, + namespace_per_environment: false, platform_kubernetes_attributes: platform_kubernetes_attributes, clusterable: clusterable } @@ -206,6 +207,7 @@ RSpec.describe ::API::Admin::InstanceClusters do expect(cluster_result.enabled).to eq(true) expect(platform_kubernetes.authorization_type).to eq('rbac') expect(cluster_result.managed).to be_falsy + expect(cluster_result.namespace_per_environment).to eq(false) expect(platform_kubernetes.api_url).to eq("https://example.com") expect(platform_kubernetes.token).to eq('sample-token') end @@ -235,6 +237,22 @@ RSpec.describe ::API::Admin::InstanceClusters do end end + context 'when namespace_per_environment is not set' do + let(:cluster_params) do + { + name: 'test-cluster', + domain: 'domain.example.com', + platform_kubernetes_attributes: platform_kubernetes_attributes + } + end + + it 'defaults to true' do + cluster_result = Clusters::Cluster.find(json_response['id']) + + expect(cluster_result).to be_namespace_per_environment + end + end + context 'when an instance cluster already exists' do it 'allows user to add multiple clusters' do post api('/admin/clusters/add', admin_user), params: multiple_cluster_params diff --git a/spec/requests/api/api_guard/response_coercer_middleware_spec.rb b/spec/requests/api/api_guard/response_coercer_middleware_spec.rb new file mode 100644 index 00000000000..6f3f97fe846 --- /dev/null +++ b/spec/requests/api/api_guard/response_coercer_middleware_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::APIGuard::ResponseCoercerMiddleware do + using RSpec::Parameterized::TableSyntax + + it 'is loaded' do + expect(API::API.middleware).to include([:use, described_class]) + end + + describe '#call' do + let(:app) do + Class.new(API::API) + end + + [ + nil, 201, 10.5, "test" + ].each do |val| + it 'returns a String body' do + app.get 'bodytest' do + status 200 + env['api.format'] = :binary + body val + end + + unless val.is_a?(String) + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).with(instance_of(ArgumentError)) + end + + get api('/bodytest') + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(val.to_s) + end + end + + [100, 204, 304].each do |status| + it 'allows nil body' do + app.get 'statustest' do + status status + env['api.format'] = :binary + body nil + end + + expect(Gitlab::ErrorTracking).not_to receive(:track_and_raise_for_dev_exception) + + get api('/statustest') + + expect(response.status).to eq(status) + expect(response.body).to eq('') + end + end + end +end diff --git a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb index 97110b63ff6..71be0c30f5a 100644 --- a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb +++ b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb @@ -227,10 +227,6 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end context 'authorize uploading of an lsif artifact' do - before do - stub_feature_flags(code_navigation: job.project) - end - it 'adds ProcessLsif header' do authorize_artifacts_with_token_in_headers(artifact_type: :lsif) @@ -249,32 +245,6 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do .to change { Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(tracking_params) } .by(1) end - - context 'code_navigation feature flag is disabled' do - before do - stub_feature_flags(code_navigation: false) - end - - it 'responds with a forbidden error' do - authorize_artifacts_with_token_in_headers(artifact_type: :lsif) - - aggregate_failures do - expect(response).to have_gitlab_http_status(:forbidden) - expect(json_response['ProcessLsif']).to be_falsy - end - end - - it 'does not track code_intelligence usage ping' do - tracking_params = { - event_names: 'i_source_code_code_intelligence', - start_date: Date.yesterday, - end_date: Date.today - } - - expect { authorize_artifacts_with_token_in_headers(artifact_type: :lsif) } - .not_to change { Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(tracking_params) } - end - end end def authorize_artifacts(params = {}, request_headers = headers) diff --git a/spec/requests/api/ci/runner/jobs_put_spec.rb b/spec/requests/api/ci/runner/jobs_put_spec.rb index 183a3b26e00..cbefaa2c321 100644 --- a/spec/requests/api/ci/runner/jobs_put_spec.rb +++ b/spec/requests/api/ci/runner/jobs_put_spec.rb @@ -46,64 +46,59 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end context 'when status is given' do - it 'mark job as succeeded' do + it 'marks job as succeeded' do update_job(state: 'success') - job.reload - expect(job).to be_success + expect(job.reload).to be_success + expect(response.header).not_to have_key('X-GitLab-Trace-Update-Interval') end - it 'mark job as failed' do + it 'marks job as failed' do update_job(state: 'failed') - job.reload - expect(job).to be_failed + expect(job.reload).to be_failed expect(job).to be_unknown_failure + expect(response.header).not_to have_key('X-GitLab-Trace-Update-Interval') end context 'when failure_reason is script_failure' do before do update_job(state: 'failed', failure_reason: 'script_failure') - job.reload end - it { expect(job).to be_script_failure } + it { expect(job.reload).to be_script_failure } end context 'when failure_reason is runner_system_failure' do before do update_job(state: 'failed', failure_reason: 'runner_system_failure') - job.reload end - it { expect(job).to be_runner_system_failure } + it { expect(job.reload).to be_runner_system_failure } end context 'when failure_reason is unrecognized value' do before do update_job(state: 'failed', failure_reason: 'what_is_this') - job.reload end - it { expect(job).to be_unknown_failure } + it { expect(job.reload).to be_unknown_failure } end context 'when failure_reason is job_execution_timeout' do before do update_job(state: 'failed', failure_reason: 'job_execution_timeout') - job.reload end - it { expect(job).to be_job_execution_timeout } + it { expect(job.reload).to be_job_execution_timeout } end context 'when failure_reason is unmet_prerequisites' do before do update_job(state: 'failed', failure_reason: 'unmet_prerequisites') - job.reload end - it { expect(job).to be_unmet_prerequisites } + it { expect(job.reload).to be_unmet_prerequisites } end context 'when unmigrated live trace chunks exist' do @@ -119,24 +114,21 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(job.pending_state).to be_present expect(response).to have_gitlab_http_status(:accepted) + expect(response.header['X-GitLab-Trace-Update-Interval']).to be > 0 end end context 'when runner retries request after receiving 202' do it 'responds with 202 and then with 200', :sidekiq_inline do - perform_enqueued_jobs do - update_job(state: 'success', checksum: 'crc32:12345678') - end + update_job(state: 'success', checksum: 'crc32:12345678') - expect(job.reload.pending_state).to be_present expect(response).to have_gitlab_http_status(:accepted) + expect(job.reload.pending_state).to be_present - perform_enqueued_jobs do - update_job(state: 'success', checksum: 'crc32:12345678') - end + update_job(state: 'success', checksum: 'crc32:12345678') - expect(job.reload.pending_state).not_to be_present expect(response).to have_gitlab_http_status(:ok) + expect(job.reload.pending_state).not_to be_present end end @@ -149,8 +141,9 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do update_job(state: 'success', checksum: 'crc:12345678') expect(job.reload).to be_success - expect(job.pending_state).not_to be_present + expect(job.pending_state).to be_present expect(response).to have_gitlab_http_status(:ok) + expect(response.header).not_to have_key('X-GitLab-Trace-Update-Interval') end end end @@ -248,7 +241,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end def update_job_after_time(update_interval = 20.minutes, state = 'running') - Timecop.travel(job.updated_at + update_interval) do + travel_to(job.updated_at + update_interval) do update_job(job.token, state: state) end end 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 4fa95f8ebb2..2dc92417892 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -194,7 +194,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do [{ 'key' => 'cache_key', 'untracked' => false, 'paths' => ['vendor/*'], - 'policy' => 'pull-push' }] + 'policy' => 'pull-push', + 'when' => 'on_success' }] end let(:expected_features) { { 'trace_sections' => true } } diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index d34244771ad..d455ed9c194 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -36,13 +36,9 @@ RSpec.describe API::Commits do end it 'include correct pagination headers' do - commit_count = project.repository.count_commits(ref: 'master').to_s - get api(route, current_user) - expect(response).to include_pagination_headers - expect(response.headers['X-Total']).to eq(commit_count) - expect(response.headers['X-Page']).to eql('1') + expect(response).to include_limited_pagination_headers end end @@ -79,12 +75,10 @@ RSpec.describe API::Commits do it 'include correct pagination headers' do commits = project.repository.commits("master", limit: 2) after = commits.second.created_at - commit_count = project.repository.count_commits(ref: 'master', after: after).to_s get api("/projects/#{project_id}/repository/commits?since=#{after.utc.iso8601}", user) - expect(response).to include_pagination_headers - expect(response.headers['X-Total']).to eq(commit_count) + expect(response).to include_limited_pagination_headers expect(response.headers['X-Page']).to eql('1') end end @@ -109,12 +103,10 @@ RSpec.describe API::Commits do it 'include correct pagination headers' do commits = project.repository.commits("master", limit: 2) before = commits.second.created_at - commit_count = project.repository.count_commits(ref: 'master', before: before).to_s get api("/projects/#{project_id}/repository/commits?until=#{before.utc.iso8601}", user) - expect(response).to include_pagination_headers - expect(response.headers['X-Total']).to eq(commit_count) + expect(response).to include_limited_pagination_headers expect(response.headers['X-Page']).to eql('1') end end @@ -137,49 +129,49 @@ RSpec.describe API::Commits do context "path optional parameter" do it "returns project commits matching provided path parameter" do path = 'files/ruby/popen.rb' - commit_count = project.repository.count_commits(ref: 'master', path: path).to_s get api("/projects/#{project_id}/repository/commits?path=#{path}", user) expect(json_response.size).to eq(3) expect(json_response.first["id"]).to eq("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") - expect(response).to include_pagination_headers - expect(response.headers['X-Total']).to eq(commit_count) + expect(response).to include_limited_pagination_headers end it 'include correct pagination headers' do path = 'files/ruby/popen.rb' - commit_count = project.repository.count_commits(ref: 'master', path: path).to_s get api("/projects/#{project_id}/repository/commits?path=#{path}", user) - expect(response).to include_pagination_headers - expect(response.headers['X-Total']).to eq(commit_count) + expect(response).to include_limited_pagination_headers expect(response.headers['X-Page']).to eql('1') end end context 'all optional parameter' do it 'returns all project commits' do - commit_count = project.repository.count_commits(all: true) + expected_commit_ids = project.repository.commits(nil, all: true, limit: 50).map(&:id) + + get api("/projects/#{project_id}/repository/commits?all=true&per_page=50", user) - get api("/projects/#{project_id}/repository/commits?all=true", user) + commit_ids = json_response.map { |c| c['id'] } - expect(response).to include_pagination_headers - expect(response.headers['X-Total']).to eq(commit_count.to_s) + expect(response).to include_limited_pagination_headers + expect(commit_ids).to eq(expected_commit_ids) expect(response.headers['X-Page']).to eql('1') end end context 'first_parent optional parameter' do it 'returns all first_parent commits' do - commit_count = project.repository.count_commits(ref: SeedRepo::Commit::ID, first_parent: true) + expected_commit_ids = project.repository.commits(SeedRepo::Commit::ID, limit: 50, first_parent: true).map(&:id) - get api("/projects/#{project_id}/repository/commits", user), params: { ref_name: SeedRepo::Commit::ID, first_parent: 'true' } + get api("/projects/#{project_id}/repository/commits?per_page=50", user), params: { ref_name: SeedRepo::Commit::ID, first_parent: 'true' } - expect(response).to include_pagination_headers - expect(commit_count).to eq(12) - expect(response.headers['X-Total']).to eq(commit_count.to_s) + commit_ids = json_response.map { |c| c['id'] } + + expect(response).to include_limited_pagination_headers + expect(expected_commit_ids.size).to eq(12) + expect(commit_ids).to eq(expected_commit_ids) end end @@ -209,11 +201,7 @@ RSpec.describe API::Commits do end it 'returns correct headers' do - commit_count = project.repository.count_commits(ref: ref_name).to_s - - expect(response).to include_pagination_headers - expect(response.headers['X-Total']).to eq(commit_count) - expect(response.headers['X-Page']).to eq('1') + expect(response).to include_limited_pagination_headers expect(response.headers['Link']).to match(/page=1&per_page=5/) expect(response.headers['Link']).to match(/page=2&per_page=5/) end @@ -972,7 +960,7 @@ RSpec.describe API::Commits do refs.concat(project.repository.tag_names_contains(commit_id).map {|name| ['tag', name]}) expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers + expect(response).to include_limited_pagination_headers expect(json_response).to be_an Array expect(json_response.map { |r| [r['type'], r['name']] }.compact).to eq(refs) end @@ -1262,7 +1250,7 @@ RSpec.describe API::Commits do get api(route, current_user) expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers + expect(response).to include_limited_pagination_headers expect(json_response.size).to be >= 1 expect(json_response.first.keys).to include 'diff' end @@ -1276,7 +1264,7 @@ RSpec.describe API::Commits do get api(route, current_user) expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers + expect(response).to include_limited_pagination_headers expect(json_response.size).to be <= 1 end end @@ -1914,7 +1902,7 @@ RSpec.describe API::Commits do get api("/projects/#{project.id}/repository/commits/#{commit.id}/merge_requests", user) expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers + expect(response).to include_limited_pagination_headers expect(json_response.length).to eq(1) expect(json_response[0]['id']).to eq(merged_mr.id) end diff --git a/spec/requests/api/composer_packages_spec.rb b/spec/requests/api/composer_packages_spec.rb index f5279af0483..ef4682466d5 100644 --- a/spec/requests/api/composer_packages_spec.rb +++ b/spec/requests/api/composer_packages_spec.rb @@ -289,6 +289,34 @@ RSpec.describe API::ComposerPackages do it_behaves_like 'process Composer api request', :developer, :not_found end end + + context 'with invalid composer.json' do + let(:headers) { basic_auth_header(user.username, personal_access_token.token) } + let(:params) { { tag: 'v1.2.99' } } + let(:project) { create(:project, :custom_repo, files: files, group: group) } + + before do + project.repository.add_tag(user, 'v1.2.99', 'master') + end + + context 'with a missing composer.json file' do + let(:files) { { 'some_other_file' => '' } } + + it_behaves_like 'process Composer api request', :developer, :unprocessable_entity + end + + context 'with an empty composer.json file' do + let(:files) { { 'composer.json' => '' } } + + it_behaves_like 'process Composer api request', :developer, :unprocessable_entity + end + + context 'with a malformed composer.json file' do + let(:files) { { 'composer.json' => 'not_valid_JSON' } } + + it_behaves_like 'process Composer api request', :developer, :unprocessable_entity + end + end end describe 'GET /api/v4/projects/:id/packages/composer/archives/*package_name?sha=:sha' do diff --git a/spec/requests/api/debian_group_packages_spec.rb b/spec/requests/api/debian_group_packages_spec.rb new file mode 100644 index 00000000000..8a05d20fb33 --- /dev/null +++ b/spec/requests/api/debian_group_packages_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe API::DebianGroupPackages do + include HttpBasicAuthHelpers + include WorkhorseHelpers + + include_context 'Debian repository shared context', :group do + describe 'GET groups/:id/-/packages/debian/dists/*distribution/Release.gpg' do + let(:url) { "/groups/#{group.id}/-/packages/debian/dists/#{distribution}/Release.gpg" } + + it_behaves_like 'Debian group repository GET endpoint', :not_found, nil + end + + describe 'GET groups/:id/-/packages/debian/dists/*distribution/Release' do + let(:url) { "/groups/#{group.id}/-/packages/debian/dists/#{distribution}/Release" } + + it_behaves_like 'Debian group repository GET endpoint', :success, 'TODO Release' + end + + describe 'GET groups/:id/-/packages/debian/dists/*distribution/InRelease' do + let(:url) { "/groups/#{group.id}/-/packages/debian/dists/#{distribution}/InRelease" } + + it_behaves_like 'Debian group repository GET endpoint', :not_found, nil + end + + describe 'GET groups/:id/-/packages/debian/dists/*distribution/:component/binary-:architecture/Packages' do + let(:url) { "/groups/#{group.id}/-/packages/debian/dists/#{distribution}/#{component}/binary-#{architecture}/Packages" } + + it_behaves_like 'Debian group repository GET endpoint', :success, 'TODO Packages' + end + + describe 'GET groups/:id/-/packages/debian/pool/:component/:letter/:source_package/:file_name' do + let(:url) { "/groups/#{group.id}/-/packages/debian/pool/#{component}/#{letter}/#{source_package}/#{package_name}_#{package_version}_#{architecture}.deb" } + + it_behaves_like 'Debian group repository GET endpoint', :success, 'TODO File' + end + end +end diff --git a/spec/requests/api/debian_project_packages_spec.rb b/spec/requests/api/debian_project_packages_spec.rb new file mode 100644 index 00000000000..d2f208d0079 --- /dev/null +++ b/spec/requests/api/debian_project_packages_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe API::DebianProjectPackages do + include HttpBasicAuthHelpers + include WorkhorseHelpers + + include_context 'Debian repository shared context', :project do + describe 'GET projects/:id/-/packages/debian/dists/*distribution/Release.gpg' do + let(:url) { "/projects/#{project.id}/-/packages/debian/dists/#{distribution}/Release.gpg" } + + it_behaves_like 'Debian project repository GET endpoint', :not_found, nil + end + + describe 'GET projects/:id/-/packages/debian/dists/*distribution/Release' do + let(:url) { "/projects/#{project.id}/-/packages/debian/dists/#{distribution}/Release" } + + it_behaves_like 'Debian project repository GET endpoint', :success, 'TODO Release' + end + + describe 'GET projects/:id/-/packages/debian/dists/*distribution/InRelease' do + let(:url) { "/projects/#{project.id}/-/packages/debian/dists/#{distribution}/InRelease" } + + it_behaves_like 'Debian project repository GET endpoint', :not_found, nil + end + + describe 'GET projects/:id/-/packages/debian/dists/*distribution/:component/binary-:architecture/Packages' do + let(:url) { "/projects/#{project.id}/-/packages/debian/dists/#{distribution}/#{component}/binary-#{architecture}/Packages" } + + it_behaves_like 'Debian project repository GET endpoint', :success, 'TODO Packages' + end + + describe 'GET projects/:id/-/packages/debian/pool/:component/:letter/:source_package/:file_name' do + let(:url) { "/projects/#{project.id}/-/packages/debian/pool/#{component}/#{letter}/#{source_package}/#{package_name}_#{package_version}_#{architecture}.deb" } + + it_behaves_like 'Debian project repository GET endpoint', :success, 'TODO File' + end + + describe 'PUT projects/:id/-/packages/debian/incoming/:file_name' do + let(:method) { :put } + let(:url) { "/projects/#{project.id}/-/packages/debian/incoming/#{file_name}" } + + it_behaves_like 'Debian project repository PUT endpoint', :created, nil + end + end +end diff --git a/spec/requests/api/doorkeeper_access_spec.rb b/spec/requests/api/doorkeeper_access_spec.rb index f16cd58bb34..77f1dadff46 100644 --- a/spec/requests/api/doorkeeper_access_spec.rb +++ b/spec/requests/api/doorkeeper_access_spec.rb @@ -71,4 +71,12 @@ RSpec.describe 'doorkeeper access' do it_behaves_like 'forbidden request' end + + context 'when user is blocked pending approval' do + before do + user.block_pending_approval + end + + it_behaves_like 'forbidden request' + end end diff --git a/spec/requests/api/feature_flag_scopes_spec.rb b/spec/requests/api/feature_flag_scopes_spec.rb new file mode 100644 index 00000000000..da5b2cbb7ae --- /dev/null +++ b/spec/requests/api/feature_flag_scopes_spec.rb @@ -0,0 +1,319 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe API::FeatureFlagScopes do + include FeatureFlagHelpers + + let(:project) { create(:project, :repository) } + let(:developer) { create(:user) } + let(:reporter) { create(:user) } + let(:user) { developer } + + before do + project.add_developer(developer) + project.add_reporter(reporter) + end + + shared_examples_for 'check user permission' do + context 'when user is reporter' do + let(:user) { reporter } + + it 'forbids the request' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + + shared_examples_for 'not found' do + it 'returns Not Found' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + describe 'GET /projects/:id/feature_flag_scopes' do + subject do + get api("/projects/#{project.id}/feature_flag_scopes", user), + params: params + end + + let(:feature_flag_1) { create_flag(project, 'flag_1', true) } + let(:feature_flag_2) { create_flag(project, 'flag_2', true) } + + before do + create_scope(feature_flag_1, 'staging', false) + create_scope(feature_flag_1, 'production', true) + create_scope(feature_flag_2, 'review/*', false) + end + + context 'when environment is production' do + let(:params) { { environment: 'production' } } + + it_behaves_like 'check user permission' + + it 'returns all effective feature flags under the environment' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag_detailed_scopes') + expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => true }) + expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => true }) + end + end + + context 'when environment is staging' do + let(:params) { { environment: 'staging' } } + + it 'returns all effective feature flags under the environment' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => false }) + expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => true }) + end + end + + context 'when environment is review/feature X' do + let(:params) { { environment: 'review/feature X' } } + + it 'returns all effective feature flags under the environment' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => true }) + expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => false }) + end + end + end + + describe 'GET /projects/:id/feature_flags/:name/scopes' do + subject do + get api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes", user) + end + + context 'when there are two scopes' do + let(:feature_flag) { create_flag(project, 'test') } + let!(:additional_scope) { create_scope(feature_flag, 'production', false) } + + it_behaves_like 'check user permission' + + it 'returns scopes of the feature flag' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag_scopes') + expect(json_response.count).to eq(2) + expect(json_response.first['environment_scope']).to eq(feature_flag.scopes[0].environment_scope) + expect(json_response.second['environment_scope']).to eq(feature_flag.scopes[1].environment_scope) + end + end + + context 'when there are no feature flags' do + let(:feature_flag) { double(:feature_flag, name: 'test') } + + it_behaves_like 'not found' + end + end + + describe 'POST /projects/:id/feature_flags/:name/scopes' do + subject do + post api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes", user), + params: params + end + + let(:params) do + { + environment_scope: 'staging', + active: true, + strategies: [{ name: 'userWithId', parameters: { 'userIds': 'a,b,c' } }].to_json + } + end + + context 'when there is a corresponding feature flag' do + let!(:feature_flag) { create(:operations_feature_flag, project: project) } + + it_behaves_like 'check user permission' + + it 'creates a new scope' do + subject + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/feature_flag_scope') + expect(json_response['environment_scope']).to eq(params[:environment_scope]) + expect(json_response['active']).to eq(params[:active]) + expect(json_response['strategies']).to eq(Gitlab::Json.parse(params[:strategies])) + end + + context 'when the scope already exists' do + before do + create_scope(feature_flag, params[:environment_scope]) + end + + it 'returns error' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to include('Scopes environment scope (staging) has already been taken') + end + end + end + + context 'when feature flag is not found' do + let(:feature_flag) { double(:feature_flag, name: 'test') } + + it_behaves_like 'not found' + end + end + + describe 'GET /projects/:id/feature_flags/:name/scopes/:environment_scope' do + subject do + get api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes/#{environment_scope}", + user) + end + + let(:environment_scope) { scope.environment_scope } + + shared_examples_for 'successful response' do + it 'returns a scope' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag_scope') + expect(json_response['id']).to eq(scope.id) + expect(json_response['active']).to eq(scope.active) + expect(json_response['environment_scope']).to eq(scope.environment_scope) + end + end + + context 'when there is a feature flag' do + let!(:feature_flag) { create(:operations_feature_flag, project: project) } + let(:scope) { feature_flag.default_scope } + + it_behaves_like 'check user permission' + it_behaves_like 'successful response' + + context 'when environment scope includes slash' do + let!(:scope) { create_scope(feature_flag, 'review/*', false) } + + it_behaves_like 'not found' + + context 'when URL-encoding the environment scope parameter' do + let(:environment_scope) { CGI.escape(scope.environment_scope) } + + it_behaves_like 'successful response' + end + end + end + + context 'when there are no feature flags' do + let(:feature_flag) { double(:feature_flag, name: 'test') } + let(:scope) { double(:feature_flag_scope, environment_scope: 'prd') } + + it_behaves_like 'not found' + end + end + + describe 'PUT /projects/:id/feature_flags/:name/scopes/:environment_scope' do + subject do + put api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes/#{environment_scope}", + user), params: params + end + + let(:environment_scope) { scope.environment_scope } + + let(:params) do + { + active: true, + strategies: [{ name: 'userWithId', parameters: { 'userIds': 'a,b,c' } }].to_json + } + end + + context 'when there is a corresponding feature flag' do + let!(:feature_flag) { create(:operations_feature_flag, project: project) } + let(:scope) { create_scope(feature_flag, 'staging', false, [{ name: "default", parameters: {} }]) } + + it_behaves_like 'check user permission' + + it 'returns the updated scope' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag_scope') + expect(json_response['id']).to eq(scope.id) + expect(json_response['active']).to eq(params[:active]) + expect(json_response['strategies']).to eq(Gitlab::Json.parse(params[:strategies])) + end + + context 'when there are no corresponding feature flag scopes' do + let(:scope) { double(:feature_flag_scope, environment_scope: 'prd') } + + it_behaves_like 'not found' + end + end + + context 'when there are no corresponding feature flags' do + let(:feature_flag) { double(:feature_flag, name: 'test') } + let(:scope) { double(:feature_flag_scope, environment_scope: 'prd') } + + it_behaves_like 'not found' + end + end + + describe 'DELETE /projects/:id/feature_flags/:name/scopes/:environment_scope' do + subject do + delete api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes/#{environment_scope}", + user) + end + + let(:environment_scope) { scope.environment_scope } + + shared_examples_for 'successful response' do + it 'destroys the scope' do + expect { subject } + .to change { Operations::FeatureFlagScope.exists?(environment_scope: scope.environment_scope) } + .from(true).to(false) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'when there is a feature flag' do + let!(:feature_flag) { create(:operations_feature_flag, project: project) } + + context 'when there is a targeted scope' do + let!(:scope) { create_scope(feature_flag, 'production', false) } + + it_behaves_like 'check user permission' + it_behaves_like 'successful response' + + context 'when environment scope includes slash' do + let!(:scope) { create_scope(feature_flag, 'review/*', false) } + + it_behaves_like 'not found' + + context 'when URL-encoding the environment scope parameter' do + let(:environment_scope) { CGI.escape(scope.environment_scope) } + + it_behaves_like 'successful response' + end + end + end + + context 'when there are no targeted scopes' do + let!(:scope) { double(:feature_flag_scope, environment_scope: 'production') } + + it_behaves_like 'not found' + end + end + + context 'when there are no feature flags' do + let(:feature_flag) { double(:feature_flag, name: 'test') } + let(:scope) { double(:feature_flag_scope, environment_scope: 'prd') } + + it_behaves_like 'not found' + end + end +end diff --git a/spec/requests/api/feature_flags_spec.rb b/spec/requests/api/feature_flags_spec.rb new file mode 100644 index 00000000000..90d4a7b8b21 --- /dev/null +++ b/spec/requests/api/feature_flags_spec.rb @@ -0,0 +1,1130 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe API::FeatureFlags do + include FeatureFlagHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:developer) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:non_project_member) { create(:user) } + let(:user) { developer } + + before_all do + project.add_developer(developer) + project.add_reporter(reporter) + end + + shared_examples_for 'check user permission' do + context 'when user is reporter' do + let(:user) { reporter } + + it 'forbids the request' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + + shared_examples_for 'not found' do + it 'returns Not Found' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + describe 'GET /projects/:id/feature_flags' do + subject { get api("/projects/#{project.id}/feature_flags", user) } + + context 'when there are two feature flags' do + let!(:feature_flag_1) do + create(:operations_feature_flag, project: project) + end + + let!(:feature_flag_2) do + create(:operations_feature_flag, project: project) + end + + it 'returns feature flags ordered by name' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flags') + expect(json_response.count).to eq(2) + expect(json_response.first['name']).to eq(feature_flag_1.name) + expect(json_response.second['name']).to eq(feature_flag_2.name) + end + + it 'returns the legacy flag version' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flags') + expect(json_response.map { |f| f['version'] }).to eq(%w[legacy_flag legacy_flag]) + end + + it 'does not return the legacy flag version when the feature flag is disabled' do + stub_feature_flags(feature_flags_new_version: false) + + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flags') + expect(json_response.select { |f| f.key?('version') }).to eq([]) + end + + it 'does not return strategies if the new flag is disabled' do + stub_feature_flags(feature_flags_new_version: false) + + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flags') + expect(json_response.select { |f| f.key?('strategies') }).to eq([]) + end + + it 'does not have N+1 problem' do + control_count = ActiveRecord::QueryRecorder.new { subject } + + create_list(:operations_feature_flag, 3, project: project) + + expect { get api("/projects/#{project.id}/feature_flags", user) } + .not_to exceed_query_limit(control_count) + end + + it_behaves_like 'check user permission' + end + + context 'with version 2 feature flags' do + let!(:feature_flag) do + create(:operations_feature_flag, :new_version_flag, project: project, name: 'feature1') + end + + let!(:strategy) do + create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + end + + let!(:scope) do + create(:operations_scope, strategy: strategy, environment_scope: 'production') + end + + it 'returns the feature flags' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flags') + expect(json_response).to eq([{ + 'name' => 'feature1', + 'description' => nil, + 'active' => true, + 'version' => 'new_version_flag', + 'updated_at' => feature_flag.updated_at.as_json, + 'created_at' => feature_flag.created_at.as_json, + 'scopes' => [], + 'strategies' => [{ + 'id' => strategy.id, + 'name' => 'default', + 'parameters' => {}, + 'scopes' => [{ + 'id' => scope.id, + 'environment_scope' => 'production' + }] + }] + }]) + end + + it 'does not return a version 2 flag when the feature flag is disabled' do + stub_feature_flags(feature_flags_new_version: false) + + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flags') + expect(json_response).to eq([]) + end + end + + context 'with version 1 and 2 feature flags' do + it 'returns both versions of flags ordered by name' do + create(:operations_feature_flag, project: project, name: 'legacy_flag') + feature_flag = create(:operations_feature_flag, :new_version_flag, project: project, name: 'new_version_flag') + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + create(:operations_scope, strategy: strategy, environment_scope: 'production') + + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flags') + expect(json_response.map { |f| f['name'] }).to eq(%w[legacy_flag new_version_flag]) + end + + it 'returns only version 1 flags when the feature flag is disabled' do + stub_feature_flags(feature_flags_new_version: false) + create(:operations_feature_flag, project: project, name: 'legacy_flag') + feature_flag = create(:operations_feature_flag, :new_version_flag, project: project, name: 'new_version_flag') + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + create(:operations_scope, strategy: strategy, environment_scope: 'production') + + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flags') + expect(json_response.map { |f| f['name'] }).to eq(['legacy_flag']) + end + end + end + + describe 'GET /projects/:id/feature_flags/:name' do + subject { get api("/projects/#{project.id}/feature_flags/#{feature_flag.name}", user) } + + context 'when there is a feature flag' do + let!(:feature_flag) { create_flag(project, 'awesome-feature') } + + it 'returns a feature flag entry' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(json_response['name']).to eq(feature_flag.name) + expect(json_response['description']).to eq(feature_flag.description) + expect(json_response['version']).to eq('legacy_flag') + end + + it_behaves_like 'check user permission' + end + + context 'with a version 2 feature_flag' do + it 'returns the feature flag' do + feature_flag = create(:operations_feature_flag, :new_version_flag, project: project, name: 'feature1') + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + scope = create(:operations_scope, strategy: strategy, environment_scope: 'production') + + get api("/projects/#{project.id}/feature_flags/feature1", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(json_response).to eq({ + 'name' => 'feature1', + 'description' => nil, + 'active' => true, + 'version' => 'new_version_flag', + 'updated_at' => feature_flag.updated_at.as_json, + 'created_at' => feature_flag.created_at.as_json, + 'scopes' => [], + 'strategies' => [{ + 'id' => strategy.id, + 'name' => 'default', + 'parameters' => {}, + 'scopes' => [{ + 'id' => scope.id, + 'environment_scope' => 'production' + }] + }] + }) + end + + it 'returns a 404 when the feature is disabled' do + stub_feature_flags(feature_flags_new_version: false) + feature_flag = create(:operations_feature_flag, :new_version_flag, project: project, name: 'feature1') + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + create(:operations_scope, strategy: strategy, environment_scope: 'production') + + get api("/projects/#{project.id}/feature_flags/feature1", user) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response).to eq({ 'message' => '404 Not found' }) + end + end + end + + describe 'POST /projects/:id/feature_flags' do + def scope_default + { + environment_scope: '*', + active: false, + strategies: [{ name: 'default', parameters: {} }].to_json + } + end + + subject do + post api("/projects/#{project.id}/feature_flags", user), params: params + end + + let(:params) do + { + name: 'awesome-feature', + scopes: [scope_default] + } + end + + it 'creates a new feature flag' do + subject + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/feature_flag') + + feature_flag = project.operations_feature_flags.last + expect(feature_flag.name).to eq(params[:name]) + expect(feature_flag.description).to eq(params[:description]) + end + + it 'defaults to a version 1 (legacy) feature flag' do + subject + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/feature_flag') + + feature_flag = project.operations_feature_flags.last + expect(feature_flag.version).to eq('legacy_flag') + end + + it_behaves_like 'check user permission' + + it 'returns version' do + subject + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(json_response['version']).to eq('legacy_flag') + end + + it 'does not return version when new version flags are disabled' do + stub_feature_flags(feature_flags_new_version: false) + + subject + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(json_response.key?('version')).to eq(false) + end + + context 'with active set to false in the params for a legacy flag' do + let(:params) do + { + name: 'awesome-feature', + version: 'legacy_flag', + active: 'false', + scopes: [scope_default] + } + end + + it 'creates an inactive feature flag' do + subject + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(json_response['active']).to eq(false) + end + end + + context 'when no scopes passed in parameters' do + let(:params) { { name: 'awesome-feature' } } + + it 'creates a new feature flag with active default scope' do + subject + + expect(response).to have_gitlab_http_status(:created) + feature_flag = project.operations_feature_flags.last + expect(feature_flag.default_scope).to be_active + end + end + + context 'when there is a feature flag with the same name already' do + before do + create_flag(project, 'awesome-feature') + end + + it 'fails to create a new feature flag' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when create a feature flag with two scopes' do + let(:params) do + { + name: 'awesome-feature', + description: 'this is awesome', + scopes: [ + scope_default, + scope_with_user_with_id + ] + } + end + + let(:scope_with_user_with_id) do + { + environment_scope: 'production', + active: true, + strategies: [{ + name: 'userWithId', + parameters: { userIds: 'user:1' } + }].to_json + } + end + + it 'creates a new feature flag with two scopes' do + subject + + expect(response).to have_gitlab_http_status(:created) + + feature_flag = project.operations_feature_flags.last + feature_flag.scopes.ordered.each_with_index do |scope, index| + expect(scope.environment_scope).to eq(params[:scopes][index][:environment_scope]) + expect(scope.active).to eq(params[:scopes][index][:active]) + expect(scope.strategies).to eq(Gitlab::Json.parse(params[:scopes][index][:strategies])) + end + end + end + + context 'when creating a version 2 feature flag' do + it 'creates a new feature flag' do + params = { + name: 'new-feature', + version: 'new_version_flag' + } + + post api("/projects/#{project.id}/feature_flags", user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(json_response).to match(hash_including({ + 'name' => 'new-feature', + 'description' => nil, + 'active' => true, + 'version' => 'new_version_flag', + 'scopes' => [], + 'strategies' => [] + })) + + feature_flag = project.operations_feature_flags.last + expect(feature_flag.name).to eq(params[:name]) + expect(feature_flag.version).to eq('new_version_flag') + end + + it 'creates a new feature flag that is inactive' do + params = { + name: 'new-feature', + version: 'new_version_flag', + active: false + } + + post api("/projects/#{project.id}/feature_flags", user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(json_response['active']).to eq(false) + + feature_flag = project.operations_feature_flags.last + expect(feature_flag.active).to eq(false) + end + + it 'creates a new feature flag with strategies' do + params = { + name: 'new-feature', + version: 'new_version_flag', + strategies: [{ + name: 'userWithId', + parameters: { 'userIds': 'user1' } + }] + } + + post api("/projects/#{project.id}/feature_flags", user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/feature_flag') + + feature_flag = project.operations_feature_flags.last + expect(feature_flag.name).to eq(params[:name]) + expect(feature_flag.version).to eq('new_version_flag') + expect(feature_flag.strategies.map { |s| s.slice(:name, :parameters).deep_symbolize_keys }).to eq([{ + name: 'userWithId', + parameters: { userIds: 'user1' } + }]) + end + + it 'creates a new feature flag with gradual rollout strategy with scopes' do + params = { + name: 'new-feature', + version: 'new_version_flag', + strategies: [{ + name: 'gradualRolloutUserId', + parameters: { groupId: 'default', percentage: '50' }, + scopes: [{ + environment_scope: 'staging' + }] + }] + } + + post api("/projects/#{project.id}/feature_flags", user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/feature_flag') + + feature_flag = project.operations_feature_flags.last + expect(feature_flag.name).to eq(params[:name]) + expect(feature_flag.version).to eq('new_version_flag') + expect(feature_flag.strategies.map { |s| s.slice(:name, :parameters).deep_symbolize_keys }).to eq([{ + name: 'gradualRolloutUserId', + parameters: { groupId: 'default', percentage: '50' } + }]) + expect(feature_flag.strategies.first.scopes.map { |s| s.slice(:environment_scope).deep_symbolize_keys }).to eq([{ + environment_scope: 'staging' + }]) + end + + it 'creates a new feature flag with flexible rollout strategy with scopes' do + params = { + name: 'new-feature', + version: 'new_version_flag', + strategies: [{ + name: 'flexibleRollout', + parameters: { groupId: 'default', rollout: '50', stickiness: 'DEFAULT' }, + scopes: [{ + environment_scope: 'staging' + }] + }] + } + + post api("/projects/#{project.id}/feature_flags", user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/feature_flag') + + feature_flag = project.operations_feature_flags.last + expect(feature_flag.name).to eq(params[:name]) + expect(feature_flag.version).to eq('new_version_flag') + expect(feature_flag.strategies.map { |s| s.slice(:name, :parameters).deep_symbolize_keys }).to eq([{ + name: 'flexibleRollout', + parameters: { groupId: 'default', rollout: '50', stickiness: 'DEFAULT' } + }]) + expect(feature_flag.strategies.first.scopes.map { |s| s.slice(:environment_scope).deep_symbolize_keys }).to eq([{ + environment_scope: 'staging' + }]) + end + + it 'returns a 422 when the feature flag is disabled' do + stub_feature_flags(feature_flags_new_version: false) + params = { + name: 'new-feature', + version: 'new_version_flag' + } + + post api("/projects/#{project.id}/feature_flags", user), params: params + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response).to eq({ 'message' => 'Version 2 flags are not enabled for this project' }) + expect(project.operations_feature_flags.count).to eq(0) + end + end + + context 'when given invalid parameters' do + it 'responds with a 400 when given an invalid version' do + params = { name: 'new-feature', version: 'bad_value' } + + post api("/projects/#{project.id}/feature_flags", user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to eq({ 'message' => 'Version is invalid' }) + end + end + end + + describe 'POST /projects/:id/feature_flags/:name/enable' do + subject do + post api("/projects/#{project.id}/feature_flags/#{params[:name]}/enable", user), + params: params + end + + let(:params) do + { + name: 'awesome-feature', + environment_scope: 'production', + strategy: { name: 'userWithId', parameters: { userIds: 'Project:1' } }.to_json + } + end + + context 'when feature flag does not exist yet' do + it 'creates a new feature flag with the specified scope and strategy' do + subject + + feature_flag = project.operations_feature_flags.last + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(feature_flag.name).to eq(params[:name]) + expect(scope.strategies).to eq([Gitlab::Json.parse(params[:strategy])]) + expect(feature_flag.version).to eq('legacy_flag') + end + + it 'returns the flag version and strategies in the json response' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(json_response.slice('version', 'strategies')).to eq({ + 'version' => 'legacy_flag', + 'strategies' => [] + }) + end + + it_behaves_like 'check user permission' + end + + context 'when feature flag exists already' do + let!(:feature_flag) { create_flag(project, params[:name]) } + + context 'when feature flag scope does not exist yet' do + it 'creates a new scope with the specified strategy' do + subject + + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + expect(response).to have_gitlab_http_status(:ok) + expect(scope.strategies).to eq([Gitlab::Json.parse(params[:strategy])]) + end + + it_behaves_like 'check user permission' + end + + context 'when feature flag scope exists already' do + let(:defined_strategy) { { name: 'userWithId', parameters: { userIds: 'Project:2' } } } + + before do + create_scope(feature_flag, params[:environment_scope], true, [defined_strategy]) + end + + it 'adds an additional strategy to the scope' do + subject + + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + expect(response).to have_gitlab_http_status(:ok) + expect(scope.strategies).to eq([defined_strategy.deep_stringify_keys, Gitlab::Json.parse(params[:strategy])]) + end + + context 'when the specified strategy exists already' do + let(:defined_strategy) { Gitlab::Json.parse(params[:strategy]) } + + it 'does not add a duplicate strategy' do + subject + + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + strategy_count = scope.strategies.count { |strategy| strategy['name'] == 'userWithId' } + expect(response).to have_gitlab_http_status(:ok) + expect(strategy_count).to eq(1) + end + end + end + end + + context 'with a version 2 flag' do + let!(:feature_flag) { create(:operations_feature_flag, :new_version_flag, project: project, name: params[:name]) } + + it 'does not change the flag and returns an unprocessable_entity response' do + subject + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response).to eq({ 'message' => 'Version 2 flags not supported' }) + feature_flag.reload + expect(feature_flag.scopes).to eq([]) + expect(feature_flag.strategies).to eq([]) + end + end + end + + describe 'POST /projects/:id/feature_flags/:name/disable' do + subject do + post api("/projects/#{project.id}/feature_flags/#{params[:name]}/disable", user), + params: params + end + + let(:params) do + { + name: 'awesome-feature', + environment_scope: 'production', + strategy: { name: 'userWithId', parameters: { userIds: 'Project:1' } }.to_json + } + end + + context 'when feature flag does not exist yet' do + it_behaves_like 'not found' + end + + context 'when feature flag exists already' do + let!(:feature_flag) { create_flag(project, params[:name]) } + + context 'when feature flag scope does not exist yet' do + it_behaves_like 'not found' + end + + context 'when feature flag scope exists already and has the specified strategy' do + let(:defined_strategies) do + [ + { name: 'userWithId', parameters: { userIds: 'Project:1' } }, + { name: 'userWithId', parameters: { userIds: 'Project:2' } } + ] + end + + before do + create_scope(feature_flag, params[:environment_scope], true, defined_strategies) + end + + it 'removes the strategy from the scope' do + subject + + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(scope.strategies) + .to eq([{ name: 'userWithId', parameters: { userIds: 'Project:2' } }.deep_stringify_keys]) + end + + it 'returns the flag version and strategies in the json response' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(json_response.slice('version', 'strategies')).to eq({ + 'version' => 'legacy_flag', + 'strategies' => [] + }) + end + + it_behaves_like 'check user permission' + + context 'when strategies become empty array after the removal' do + let(:defined_strategies) do + [{ name: 'userWithId', parameters: { userIds: 'Project:1' } }] + end + + it 'destroys the scope' do + subject + + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + expect(response).to have_gitlab_http_status(:ok) + expect(scope).to be_nil + end + + it_behaves_like 'check user permission' + end + end + + context 'when scope exists already but cannot find the corresponding strategy' do + let(:defined_strategy) { { name: 'userWithId', parameters: { userIds: 'Project:2' } } } + + before do + create_scope(feature_flag, params[:environment_scope], true, [defined_strategy]) + end + + it_behaves_like 'not found' + end + end + + context 'with a version 2 feature flag' do + let!(:feature_flag) { create(:operations_feature_flag, :new_version_flag, project: project, name: params[:name]) } + + it 'does not change the flag and returns an unprocessable_entity response' do + subject + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response).to eq({ 'message' => 'Version 2 flags not supported' }) + feature_flag.reload + expect(feature_flag.scopes).to eq([]) + expect(feature_flag.strategies).to eq([]) + end + end + end + + describe 'PUT /projects/:id/feature_flags/:name' do + context 'with a legacy feature flag' do + let!(:feature_flag) do + create(:operations_feature_flag, :legacy_flag, project: project, + name: 'feature1', description: 'old description') + end + + it 'returns a 404 if the feature is disabled' do + stub_feature_flags(feature_flags_new_version: false) + params = { description: 'new description' } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:not_found) + expect(feature_flag.reload.description).to eq('old description') + end + + it 'returns a 422' do + params = { description: 'new description' } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response).to eq({ 'message' => 'PUT operations are not supported for legacy feature flags' }) + expect(feature_flag.reload.description).to eq('old description') + end + end + + context 'with a version 2 feature flag' do + let!(:feature_flag) do + create(:operations_feature_flag, :new_version_flag, project: project, active: true, + name: 'feature1', description: 'old description') + end + + it 'returns a 404 if the feature is disabled' do + stub_feature_flags(feature_flags_new_version: false) + params = { description: 'new description' } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:not_found) + expect(feature_flag.reload.description).to eq('old description') + end + + it 'returns a 404 if the feature flag does not exist' do + params = { description: 'new description' } + + put api("/projects/#{project.id}/feature_flags/other_flag_name", user), params: params + + expect(response).to have_gitlab_http_status(:not_found) + expect(feature_flag.reload.description).to eq('old description') + end + + it 'forbids a request for a reporter' do + params = { description: 'new description' } + + put api("/projects/#{project.id}/feature_flags/feature1", reporter), params: params + + expect(response).to have_gitlab_http_status(:forbidden) + expect(feature_flag.reload.description).to eq('old description') + end + + it 'returns an error for an invalid update of gradual rollout' do + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + params = { + strategies: [{ + id: strategy.id, + name: 'gradualRolloutUserId', + parameters: { bad: 'params' } + }] + } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).not_to be_nil + result = feature_flag.reload.strategies.map { |s| s.slice(:id, :name, :parameters).deep_symbolize_keys } + expect(result).to eq([{ + id: strategy.id, + name: 'default', + parameters: {} + }]) + end + + it 'returns an error for an invalid update of flexible rollout' do + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + params = { + strategies: [{ + id: strategy.id, + name: 'flexibleRollout', + parameters: { bad: 'params' } + }] + } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).not_to be_nil + result = feature_flag.reload.strategies.map { |s| s.slice(:id, :name, :parameters).deep_symbolize_keys } + expect(result).to eq([{ + id: strategy.id, + name: 'default', + parameters: {} + }]) + end + + it 'updates the feature flag' do + params = { description: 'new description' } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(feature_flag.reload.description).to eq('new description') + end + + it 'updates the flag active value' do + params = { active: false } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(json_response['active']).to eq(false) + expect(feature_flag.reload.active).to eq(false) + end + + it 'updates the feature flag name' do + params = { name: 'new-name' } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(json_response['name']).to eq('new-name') + expect(feature_flag.reload.name).to eq('new-name') + end + + it 'ignores a provided version parameter' do + params = { description: 'other description', version: 'bad_value' } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(feature_flag.reload.description).to eq('other description') + end + + it 'returns the feature flag json' do + params = { description: 'new description' } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + feature_flag.reload + expect(json_response).to eq({ + 'name' => 'feature1', + 'description' => 'new description', + 'active' => true, + 'created_at' => feature_flag.created_at.as_json, + 'updated_at' => feature_flag.updated_at.as_json, + 'scopes' => [], + 'strategies' => [], + 'version' => 'new_version_flag' + }) + end + + it 'updates an existing feature flag strategy to be gradual rollout strategy' do + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + params = { + strategies: [{ + id: strategy.id, + name: 'gradualRolloutUserId', + parameters: { groupId: 'default', percentage: '10' } + }] + } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + result = feature_flag.reload.strategies.map { |s| s.slice(:id, :name, :parameters).deep_symbolize_keys } + expect(result).to eq([{ + id: strategy.id, + name: 'gradualRolloutUserId', + parameters: { groupId: 'default', percentage: '10' } + }]) + end + + it 'updates an existing feature flag strategy to be flexible rollout strategy' do + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + params = { + strategies: [{ + id: strategy.id, + name: 'flexibleRollout', + parameters: { groupId: 'default', rollout: '10', stickiness: 'DEFAULT' } + }] + } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + result = feature_flag.reload.strategies.map { |s| s.slice(:id, :name, :parameters).deep_symbolize_keys } + expect(result).to eq([{ + id: strategy.id, + name: 'flexibleRollout', + parameters: { groupId: 'default', rollout: '10', stickiness: 'DEFAULT' } + }]) + end + + it 'adds a new gradual rollout strategy to a feature flag' do + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + params = { + strategies: [{ + name: 'gradualRolloutUserId', + parameters: { groupId: 'default', percentage: '10' } + }] + } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + result = feature_flag.reload.strategies + .map { |s| s.slice(:id, :name, :parameters).deep_symbolize_keys } + .sort_by { |s| s[:name] } + expect(result.first[:id]).to eq(strategy.id) + expect(result.map { |s| s.slice(:name, :parameters) }).to eq([{ + name: 'default', + parameters: {} + }, { + name: 'gradualRolloutUserId', + parameters: { groupId: 'default', percentage: '10' } + }]) + end + + it 'adds a new gradual flexible strategy to a feature flag' do + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + params = { + strategies: [{ + name: 'flexibleRollout', + parameters: { groupId: 'default', rollout: '10', stickiness: 'DEFAULT' } + }] + } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + result = feature_flag.reload.strategies + .map { |s| s.slice(:id, :name, :parameters).deep_symbolize_keys } + .sort_by { |s| s[:name] } + expect(result.first[:id]).to eq(strategy.id) + expect(result.map { |s| s.slice(:name, :parameters) }).to eq([{ + name: 'default', + parameters: {} + }, { + name: 'flexibleRollout', + parameters: { groupId: 'default', rollout: '10', stickiness: 'DEFAULT' } + }]) + end + + it 'deletes a feature flag strategy' do + strategy_a = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + strategy_b = create(:operations_strategy, feature_flag: feature_flag, + name: 'userWithId', parameters: { userIds: 'userA,userB' }) + params = { + strategies: [{ + id: strategy_a.id, + name: 'default', + parameters: {}, + _destroy: true + }, { + id: strategy_b.id, + name: 'userWithId', + parameters: { userIds: 'userB' } + }] + } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + result = feature_flag.reload.strategies + .map { |s| s.slice(:id, :name, :parameters).deep_symbolize_keys } + .sort_by { |s| s[:name] } + expect(result).to eq([{ + id: strategy_b.id, + name: 'userWithId', + parameters: { userIds: 'userB' } + }]) + end + + it 'updates an existing feature flag scope' do + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + scope = create(:operations_scope, strategy: strategy, environment_scope: '*') + params = { + strategies: [{ + id: strategy.id, + scopes: [{ + id: scope.id, + environment_scope: 'production' + }] + }] + } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + result = feature_flag.reload.strategies.first.scopes.map { |s| s.slice(:id, :environment_scope).deep_symbolize_keys } + expect(result).to eq([{ + id: scope.id, + environment_scope: 'production' + }]) + end + + it 'deletes an existing feature flag scope' do + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + scope = create(:operations_scope, strategy: strategy, environment_scope: '*') + params = { + strategies: [{ + id: strategy.id, + scopes: [{ + id: scope.id, + _destroy: true + }] + }] + } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(feature_flag.reload.strategies.first.scopes.count).to eq(0) + end + end + end + + describe 'DELETE /projects/:id/feature_flags/:name' do + subject do + delete api("/projects/#{project.id}/feature_flags/#{feature_flag.name}", user), + params: params + end + + let!(:feature_flag) { create(:operations_feature_flag, project: project) } + let(:params) { {} } + + it 'destroys the feature flag' do + expect { subject }.to change { Operations::FeatureFlag.count }.by(-1) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns version' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['version']).to eq('legacy_flag') + end + + it 'does not return version when new version flags are disabled' do + stub_feature_flags(feature_flags_new_version: false) + + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.key?('version')).to eq(false) + end + + context 'with a version 2 feature flag' do + let!(:feature_flag) { create(:operations_feature_flag, :new_version_flag, project: project) } + + it 'destroys the flag' do + expect { subject }.to change { Operations::FeatureFlag.count }.by(-1) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns a 404 if the feature is disabled' do + stub_feature_flags(feature_flags_new_version: false) + + expect { subject }.not_to change { Operations::FeatureFlag.count } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/requests/api/feature_flags_user_lists_spec.rb b/spec/requests/api/feature_flags_user_lists_spec.rb new file mode 100644 index 00000000000..469210040dd --- /dev/null +++ b/spec/requests/api/feature_flags_user_lists_spec.rb @@ -0,0 +1,371 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::FeatureFlagsUserLists do + let_it_be(:project, refind: true) { create(:project) } + let_it_be(:developer) { create(:user) } + let_it_be(:reporter) { create(:user) } + + before_all do + project.add_developer(developer) + project.add_reporter(reporter) + end + + def create_list(name: 'mylist', user_xids: 'user1') + create(:operations_feature_flag_user_list, project: project, name: name, user_xids: user_xids) + end + + def disable_repository(project) + project.project_feature.update!( + repository_access_level: ::ProjectFeature::DISABLED, + merge_requests_access_level: ::ProjectFeature::DISABLED, + builds_access_level: ::ProjectFeature::DISABLED + ) + end + + describe 'GET /projects/:id/feature_flags_user_lists' do + it 'forbids the request for a reporter' do + get api("/projects/#{project.id}/feature_flags_user_lists", reporter) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'returns forbidden if the feature is unavailable' do + disable_repository(project) + + get api("/projects/#{project.id}/feature_flags_user_lists", developer) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'returns all the user lists' do + create_list(name: 'list_a', user_xids: 'user1') + create_list(name: 'list_b', user_xids: 'user1,user2,user3') + + get api("/projects/#{project.id}/feature_flags_user_lists", developer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.map { |list| list['name'] }.sort).to eq(%w[list_a list_b]) + end + + it 'returns all the data for a user list' do + user_list = create_list(name: 'list_a', user_xids: 'user1') + + get api("/projects/#{project.id}/feature_flags_user_lists", developer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq([{ + 'id' => user_list.id, + 'iid' => user_list.iid, + 'project_id' => project.id, + 'created_at' => user_list.created_at.as_json, + 'updated_at' => user_list.updated_at.as_json, + 'name' => 'list_a', + 'user_xids' => 'user1', + 'path' => project_feature_flags_user_list_path(user_list.project, user_list), + 'edit_path' => edit_project_feature_flags_user_list_path(user_list.project, user_list) + }]) + end + + it 'paginates user lists' do + create_list(name: 'list_a', user_xids: 'user1') + create_list(name: 'list_b', user_xids: 'user1,user2,user3') + + get api("/projects/#{project.id}/feature_flags_user_lists?page=2&per_page=1", developer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.map { |list| list['name'] }).to eq(['list_b']) + end + + it 'returns the user lists for only the specified project' do + create(:operations_feature_flag_user_list, project: project, name: 'list') + other_project = create(:project) + create(:operations_feature_flag_user_list, project: other_project, name: 'other_list') + + get api("/projects/#{project.id}/feature_flags_user_lists", developer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.map { |list| list['name'] }).to eq(['list']) + end + + it 'returns an empty list' do + get api("/projects/#{project.id}/feature_flags_user_lists", developer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq([]) + end + end + + describe 'GET /projects/:id/feature_flags_user_lists/:iid' do + it 'forbids the request for a reporter' do + list = create_list + + get api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", reporter) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'returns forbidden if the feature is unavailable' do + disable_repository(project) + list = create_list + + get api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", developer) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'returns the user list' do + list = create_list(name: 'testers', user_xids: 'test1,test2') + + get api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", developer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ + 'name' => 'testers', + 'user_xids' => 'test1,test2', + 'id' => list.id, + 'iid' => list.iid, + 'project_id' => project.id, + 'created_at' => list.created_at.as_json, + 'updated_at' => list.updated_at.as_json, + 'path' => project_feature_flags_user_list_path(list.project, list), + 'edit_path' => edit_project_feature_flags_user_list_path(list.project, list) + }) + end + + it 'returns the correct user list identified by the iid' do + create_list(name: 'list_a', user_xids: 'test1') + list_b = create_list(name: 'list_b', user_xids: 'test2') + + get api("/projects/#{project.id}/feature_flags_user_lists/#{list_b.iid}", developer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['name']).to eq('list_b') + end + + it 'scopes the iid search to the project' do + other_project = create(:project) + other_project.add_developer(developer) + create(:operations_feature_flag_user_list, project: other_project, name: 'other_list') + list = create(:operations_feature_flag_user_list, project: project, name: 'list') + + get api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", developer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['name']).to eq('list') + end + + it 'returns not found when the list does not exist' do + get api("/projects/#{project.id}/feature_flags_user_lists/1", developer) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response).to eq({ 'message' => '404 Not found' }) + end + end + + describe 'POST /projects/:id/feature_flags_user_lists' do + it 'forbids the request for a reporter' do + post api("/projects/#{project.id}/feature_flags_user_lists", reporter), params: { + name: 'mylist', user_xids: 'user1' + } + + expect(response).to have_gitlab_http_status(:forbidden) + expect(project.operations_feature_flags_user_lists.count).to eq(0) + end + + it 'returns forbidden if the feature is unavailable' do + disable_repository(project) + + post api("/projects/#{project.id}/feature_flags_user_lists", developer), params: { + name: 'mylist', user_xids: 'user1' + } + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'creates the flag' do + post api("/projects/#{project.id}/feature_flags_user_lists", developer), params: { + name: 'mylist', user_xids: 'user1' + } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response.slice('name', 'user_xids', 'project_id', 'iid')).to eq({ + 'name' => 'mylist', + 'user_xids' => 'user1', + 'project_id' => project.id, + 'iid' => 1 + }) + expect(project.operations_feature_flags_user_lists.count).to eq(1) + expect(project.operations_feature_flags_user_lists.last.name).to eq('mylist') + end + + it 'requires name' do + post api("/projects/#{project.id}/feature_flags_user_lists", developer), params: { + user_xids: 'user1' + } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to eq({ 'message' => 'name is missing' }) + expect(project.operations_feature_flags_user_lists.count).to eq(0) + end + + it 'requires user_xids' do + post api("/projects/#{project.id}/feature_flags_user_lists", developer), params: { + name: 'empty_list' + } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to eq({ 'message' => 'user_xids is missing' }) + expect(project.operations_feature_flags_user_lists.count).to eq(0) + end + + it 'returns an error when name is already taken' do + create_list(name: 'myname') + post api("/projects/#{project.id}/feature_flags_user_lists", developer), params: { + name: 'myname', user_xids: 'a' + } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to eq({ 'message' => ['Name has already been taken'] }) + expect(project.operations_feature_flags_user_lists.count).to eq(1) + end + + it 'does not create a flag for a project of which the developer is not a member' do + other_project = create(:project) + + post api("/projects/#{other_project.id}/feature_flags_user_lists", developer), params: { + name: 'mylist', user_xids: 'user1' + } + + expect(response).to have_gitlab_http_status(:not_found) + expect(other_project.operations_feature_flags_user_lists.count).to eq(0) + expect(project.operations_feature_flags_user_lists.count).to eq(0) + end + end + + describe 'PUT /projects/:id/feature_flags_user_lists/:iid' do + it 'forbids the request for a reporter' do + list = create_list(name: 'original_name') + + put api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", reporter), params: { + name: 'mylist' + } + + expect(response).to have_gitlab_http_status(:forbidden) + expect(list.reload.name).to eq('original_name') + end + + it 'returns forbidden if the feature is unavailable' do + list = create_list(name: 'original_name') + disable_repository(project) + + put api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", developer), params: { + name: 'mylist', user_xids: '456,789' + } + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'updates the list' do + list = create_list(name: 'original_name', user_xids: '123') + + put api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", developer), params: { + name: 'mylist', user_xids: '456,789' + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.slice('name', 'user_xids')).to eq({ + 'name' => 'mylist', + 'user_xids' => '456,789' + }) + expect(list.reload.name).to eq('mylist') + end + + it 'preserves attributes not listed in the request' do + list = create_list(name: 'original_name', user_xids: '123') + + put api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", developer), params: {} + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.slice('name', 'user_xids')).to eq({ + 'name' => 'original_name', + 'user_xids' => '123' + }) + expect(list.reload.name).to eq('original_name') + expect(list.reload.user_xids).to eq('123') + end + + it 'returns an error when the update is invalid' do + create_list(name: 'taken', user_xids: '123') + list = create_list(name: 'original_name', user_xids: '123') + + put api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", developer), params: { + name: 'taken' + } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to eq({ 'message' => ['Name has already been taken'] }) + end + + it 'returns not found when the list does not exist' do + list = create_list(name: 'original_name', user_xids: '123') + + put api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid + 1}", developer), params: { + name: 'new_name' + } + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response).to eq({ 'message' => '404 Not found' }) + end + end + + describe 'DELETE /projects/:id/feature_flags_user_lists/:iid' do + it 'forbids the request for a reporter' do + list = create_list + + delete api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", reporter) + + expect(response).to have_gitlab_http_status(:forbidden) + expect(project.operations_feature_flags_user_lists.count).to eq(1) + end + + it 'returns forbidden if the feature is unavailable' do + list = create_list + disable_repository(project) + + delete api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", developer) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'returns not found when the list does not exist' do + delete api("/projects/#{project.id}/feature_flags_user_lists/1", developer) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response).to eq({ 'message' => '404 Not found' }) + end + + it 'deletes the list' do + list = create_list + + delete api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", developer) + + expect(response).to have_gitlab_http_status(:no_content) + expect(response.body).to be_blank + expect(project.operations_feature_flags_user_lists.count).to eq(0) + end + + it 'does not delete the list if it is associated with a strategy' do + list = create_list + feature_flag = create(:operations_feature_flag, :new_version_flag, project: project) + create(:operations_strategy, feature_flag: feature_flag, name: 'gitlabUserList', user_list: list) + + delete api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", developer) + + expect(response).to have_gitlab_http_status(:conflict) + expect(json_response).to eq({ 'message' => ['User list is associated with a strategy'] }) + expect(list.reload).to be_persisted + end + end +end diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb index 2746e777306..3f443b4f92b 100644 --- a/spec/requests/api/features_spec.rb +++ b/spec/requests/api/features_spec.rb @@ -12,6 +12,8 @@ RSpec.describe API::Features, stub_feature_flags: false do Flipper.register(:perf_team) do |actor| actor.respond_to?(:admin) && actor.admin? end + + skip_feature_flags_yaml_validation end describe 'GET /features' do diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index bb4e88f97f8..f77f127ddc8 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -747,7 +747,7 @@ RSpec.describe API::Files do it "updates existing file in project repo with accepts correct last commit id" do last_commit = Gitlab::Git::Commit - .last_for_path(project.repository, 'master', URI.unescape(file_path)) + .last_for_path(project.repository, 'master', Addressable::URI.unencode_component(file_path)) params_with_correct_id = params.merge(last_commit_id: last_commit.id) put api(route(file_path), user), params: params_with_correct_id @@ -757,7 +757,7 @@ RSpec.describe API::Files do it "returns 400 when file path is invalid" do last_commit = Gitlab::Git::Commit - .last_for_path(project.repository, 'master', URI.unescape(file_path)) + .last_for_path(project.repository, 'master', Addressable::URI.unencode_component(file_path)) params_with_correct_id = params.merge(last_commit_id: last_commit.id) put api(route(rouge_file_path), user), params: params_with_correct_id @@ -769,7 +769,7 @@ RSpec.describe API::Files do it_behaves_like 'when path is absolute' do let(:last_commit) do Gitlab::Git::Commit - .last_for_path(project.repository, 'master', URI.unescape(file_path)) + .last_for_path(project.repository, 'master', Addressable::URI.unencode_component(file_path)) end let(:params_with_correct_id) { params.merge(last_commit_id: last_commit.id) } diff --git a/spec/requests/api/generic_packages_spec.rb b/spec/requests/api/generic_packages_spec.rb index ed852fe75c7..2cb686167f1 100644 --- a/spec/requests/api/generic_packages_spec.rb +++ b/spec/requests/api/generic_packages_spec.rb @@ -4,79 +4,432 @@ require 'spec_helper' RSpec.describe API::GenericPackages do let_it_be(:personal_access_token) { create(:personal_access_token) } - let_it_be(:project) { create(:project) } + let_it_be(:project, reload: true) { create(: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 } + let(:ci_build) { create(:ci_build, :running, user: user) } - describe 'GET /api/v4/projects/:id/packages/generic/ping' do - let(:user) { personal_access_token.user } - let(:auth_token) { personal_access_token.token } + def auth_header + return {} if user_role == :anonymous + case authenticate_with + when :personal_access_token + personal_access_token_header + when :job_token + job_token_header + when :invalid_personal_access_token + personal_access_token_header('wrong token') + when :invalid_job_token + job_token_header('wrong token') + end + end + + def personal_access_token_header(value = nil) + { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => value || personal_access_token.token } + end + + def job_token_header(value = nil) + { Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER => value || ci_build.token } + end + + shared_examples 'secure endpoint' do before do project.add_developer(user) end - context 'packages feature is disabled' do - it 'responds with 404 Not Found' do - stub_packages_setting(enabled: false) + it 'rejects malicious request' do + subject - ping(personal_access_token: auth_token) + expect(response).to have_gitlab_http_status(:bad_request) + end + end - expect(response).to have_gitlab_http_status(:not_found) + describe 'PUT /api/v4/projects/:id/packages/generic/:package_name/:package_version/:file_name/authorize' do + context 'with valid project' do + using RSpec::Parameterized::TableSyntax + + where(:project_visibility, :user_role, :member?, :authenticate_with, :expected_status) do + 'PUBLIC' | :developer | true | :personal_access_token | :success + '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 | 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' | :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 | 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' | :anonymous | false | :none | :unauthorized + 'PUBLIC' | :developer | true | :job_token | :success + 'PUBLIC' | :developer | true | :invalid_job_token | :unauthorized + 'PUBLIC' | :developer | false | :job_token | :forbidden + 'PUBLIC' | :developer | false | :invalid_job_token | :unauthorized + 'PRIVATE' | :developer | true | :job_token | :success + 'PRIVATE' | :developer | true | :invalid_job_token | :unauthorized + 'PRIVATE' | :developer | false | :job_token | :not_found + 'PRIVATE' | :developer | false | :invalid_job_token | :unauthorized + end + + with_them do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility, false)) + project.send("add_#{user_role}", user) if member? && user_role != :anonymous + end + + it "responds with #{params[:expected_status]}" do + authorize_upload_file(workhorse_header.merge(auth_header)) + + expect(response).to have_gitlab_http_status(expected_status) + end + end + end + + context 'application security' do + using RSpec::Parameterized::TableSyntax + + where(:param_name, :param_value) do + :package_name | 'my-package/../' + :package_name | 'my-package%2f%2e%2e%2f' + :file_name | '../.ssh%2fauthorized_keys' + :file_name | '%2e%2e%2f.ssh%2fauthorized_keys' + end + + with_them do + subject { authorize_upload_file(workhorse_header.merge(personal_access_token_header), param_name => param_value) } + + it_behaves_like 'secure endpoint' end end context 'generic_packages feature flag is disabled' do it 'responds with 404 Not Found' do stub_feature_flags(generic_packages: false) + project.add_developer(user) - ping(personal_access_token: auth_token) + authorize_upload_file(workhorse_header.merge(personal_access_token_header)) expect(response).to have_gitlab_http_status(:not_found) end end - context 'generic_packages feature flag is enabled' do + def authorize_upload_file(request_headers, package_name: 'mypackage', file_name: 'myfile.tar.gz') + url = "/projects/#{project.id}/packages/generic/#{package_name}/0.0.1/#{file_name}/authorize" + + put api(url), headers: request_headers + end + end + + describe 'PUT /api/v4/projects/:id/packages/generic/:package_name/:package_version/:file_name' do + include WorkhorseHelpers + + let(:file_upload) { fixture_file_upload('spec/fixtures/packages/generic/myfile.tar.gz') } + let(:params) { { file: file_upload } } + + context 'authentication' do + using RSpec::Parameterized::TableSyntax + + where(:project_visibility, :user_role, :member?, :authenticate_with, :expected_status) 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 | 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' | :anonymous | false | :none | :unauthorized + '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 | 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' | :anonymous | false | :none | :unauthorized + 'PUBLIC' | :developer | true | :invalid_job_token | :unauthorized + 'PUBLIC' | :developer | false | :job_token | :forbidden + 'PUBLIC' | :developer | false | :invalid_job_token | :unauthorized + 'PRIVATE' | :developer | true | :invalid_job_token | :unauthorized + 'PRIVATE' | :developer | false | :job_token | :not_found + 'PRIVATE' | :developer | false | :invalid_job_token | :unauthorized + end + + with_them do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility, false)) + project.send("add_#{user_role}", user) if member? && user_role != :anonymous + end + + it "responds with #{params[:expected_status]}" do + headers = workhorse_header.merge(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 before do - stub_feature_flags(generic_packages: true) + project.add_developer(user) end - context 'authenticating using personal access token' do - it 'responds with 200 OK when valid personal access token is provided' do - ping(personal_access_token: auth_token) + it 'creates package and package file when valid personal access token is used' do + headers = workhorse_header.merge(personal_access_token_header) + + 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) - expect(response).to have_gitlab_http_status(:ok) + package = project.packages.generic.last + expect(package.name).to eq('mypackage') + expect(package.version).to eq('0.0.1') + expect(package.build_info).to be_nil + + package_file = package.package_files.last + expect(package_file.file_name).to eq('myfile.tar.gz') 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) - it 'responds with 401 Unauthorized when invalid personal access token provided' do - ping(personal_access_token: 'invalid-token') + aggregate_failures do + expect(response).to have_gitlab_http_status(:created) - expect(response).to have_gitlab_http_status(:unauthorized) + package = project.packages.generic.last + expect(package.name).to eq('mypackage') + expect(package.version).to eq('0.0.1') + expect(package.build_info.pipeline).to eq(ci_build.pipeline) + + package_file = package.package_files.last + expect(package_file.file_name).to eq('myfile.tar.gz') end end - context 'authenticating using job token' do - it 'responds with 200 OK when valid job token is provided' do - job_token = create(:ci_build, :running, user: user).token + context 'event tracking' do + subject { upload_file(params, workhorse_header.merge(personal_access_token_header)) } + + it_behaves_like 'a gitlab tracking event', described_class.name, 'push_package' + end + + it 'rejects request without a file from workhorse' do + headers = workhorse_header.merge(personal_access_token_header) + upload_file({}, headers) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'rejects request without an auth token' do + upload_file(params, workhorse_header) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + + it 'rejects request without workhorse rewritten fields' do + headers = workhorse_header.merge(personal_access_token_header) + upload_file(params, headers, send_rewritten_field: false) - ping(job_token: job_token) + expect(response).to have_gitlab_http_status(:bad_request) + end - expect(response).to have_gitlab_http_status(:ok) + it 'rejects request if file size is too large' do + allow_next_instance_of(UploadedFile) do |uploaded_file| + allow(uploaded_file).to receive(:size).and_return(project.actual_limits.generic_packages_max_file_size + 1) end - it 'responds with 401 Unauthorized when invalid job token provided' do - ping(job_token: 'invalid-token') + headers = workhorse_header.merge(personal_access_token_header) + upload_file(params, headers) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'rejects request without workhorse header' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).once - expect(response).to have_gitlab_http_status(:unauthorized) + upload_file(params, personal_access_token_header) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'application security' do + using RSpec::Parameterized::TableSyntax + + where(:param_name, :param_value) do + :package_name | 'my-package/../' + :package_name | 'my-package%2f%2e%2e%2f' + :file_name | '../.ssh%2fauthorized_keys' + :file_name | '%2e%2e%2f.ssh%2fauthorized_keys' + end + + with_them do + subject { upload_file(params, workhorse_header.merge(personal_access_token_header), param_name => param_value) } + + it_behaves_like 'secure endpoint' + end + end + + def upload_file(params, request_headers, send_rewritten_field: true, package_name: 'mypackage', file_name: 'myfile.tar.gz') + url = "/projects/#{project.id}/packages/generic/#{package_name}/0.0.1/#{file_name}" + + workhorse_finalize( + api(url), + method: :put, + file_key: :file, + params: params, + headers: request_headers, + send_rewritten_field: send_rewritten_field + ) + end + end + + describe 'GET /api/v4/projects/:id/packages/generic/:package_name/:package_version/:file_name' do + using RSpec::Parameterized::TableSyntax + + let_it_be(:package) { create(:generic_package, project: project) } + let_it_be(:package_file) { create(:package_file, :generic, package: package) } + + context 'authentication' 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 | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :guest | true | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :developer | false | :personal_access_token | :success + 'PUBLIC' | :guest | false | :personal_access_token | :success + 'PUBLIC' | :developer | false | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :guest | false | :invalid_personal_access_token | :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 | 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' | :anonymous | false | :none | :unauthorized + 'PUBLIC' | :developer | true | :job_token | :success + 'PUBLIC' | :developer | true | :invalid_job_token | :unauthorized + 'PUBLIC' | :developer | false | :job_token | :success + 'PUBLIC' | :developer | false | :invalid_job_token | :unauthorized + 'PRIVATE' | :developer | true | :job_token | :success + 'PRIVATE' | :developer | true | :invalid_job_token | :unauthorized + 'PRIVATE' | :developer | false | :job_token | :not_found + 'PRIVATE' | :developer | false | :invalid_job_token | :unauthorized + end + + with_them do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility, false)) + project.send("add_#{user_role}", user) if member? && user_role != :anonymous + end + + it "responds with #{params[:expected_status]}" do + download_file(auth_header) + + expect(response).to have_gitlab_http_status(expected_status) end end end - def ping(personal_access_token: nil, job_token: nil) - headers = { - Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => personal_access_token.presence, - Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER => job_token.presence - }.compact + context 'event tracking' do + before do + project.add_developer(user) + end + + subject { download_file(personal_access_token_header) } + + it_behaves_like 'a gitlab tracking event', described_class.name, 'pull_package' + end + + it 'rejects a malicious file name request' do + project.add_developer(user) + + download_file(personal_access_token_header, file_name: '../.ssh%2fauthorized_keys') + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'rejects a malicious file name request' do + project.add_developer(user) + + download_file(personal_access_token_header, file_name: '%2e%2e%2f.ssh%2fauthorized_keys') + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'rejects a malicious package name request' do + project.add_developer(user) + + download_file(personal_access_token_header, package_name: 'my-package/../') + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'rejects a malicious package name request' do + project.add_developer(user) + + download_file(personal_access_token_header, package_name: 'my-package%2f%2e%2e%2f') + + expect(response).to have_gitlab_http_status(:bad_request) + end + + context 'application security' do + using RSpec::Parameterized::TableSyntax + + where(:param_name, :param_value) do + :package_name | 'my-package/../' + :package_name | 'my-package%2f%2e%2e%2f' + :file_name | '../.ssh%2fauthorized_keys' + :file_name | '%2e%2e%2f.ssh%2fauthorized_keys' + end + + with_them do + subject { download_file(personal_access_token_header, param_name => param_value) } + + it_behaves_like 'secure endpoint' + end + end + + it 'responds with 404 Not Found for non existing package' do + project.add_developer(user) + + download_file(personal_access_token_header, package_name: 'no-such-package') + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'responds with 404 Not Found for non existing package file' do + project.add_developer(user) + + download_file(personal_access_token_header, file_name: 'no-such-file') + + expect(response).to have_gitlab_http_status(:not_found) + end + + def download_file(request_headers, package_name: nil, file_name: nil) + package_name ||= package.name + file_name ||= package_file.file_name + url = "/projects/#{project.id}/packages/generic/#{package_name}/#{package.version}/#{file_name}" - get api('/projects/%d/packages/generic/ping' % project.id), headers: headers + get api(url), headers: request_headers end end end diff --git a/spec/requests/api/graphql/boards/board_lists_query_spec.rb b/spec/requests/api/graphql/boards/board_lists_query_spec.rb index 0838900eaba..5d5b963fed5 100644 --- a/spec/requests/api/graphql/boards/board_lists_query_spec.rb +++ b/spec/requests/api/graphql/boards/board_lists_query_spec.rb @@ -7,8 +7,8 @@ RSpec.describe 'get board lists' do let_it_be(:user) { create(:user) } let_it_be(:unauth_user) { create(:user) } - let_it_be(:project) { create(:project, creator_id: user.id, namespace: user.namespace ) } let_it_be(:group) { create(:group, :private) } + let_it_be(:project) { create(:project, creator_id: user.id, group: group) } let_it_be(:project_label) { create(:label, project: project, name: 'Development') } let_it_be(:project_label2) { create(:label, project: project, name: 'Testing') } let_it_be(:group_label) { create(:group_label, group: group, name: 'Development') } @@ -111,12 +111,19 @@ RSpec.describe 'get board lists' do board_parent.add_reporter(user) end - it 'finds the correct list' do + it 'returns the correct list with issue count for matching issue filters' do label_list = create(:list, board: board, label: label, position: 10) + create(:issue, project: project, labels: [label, label2]) + create(:issue, project: project, labels: [label]) - post_graphql(query("id: \"#{global_id_of(label_list)}\""), current_user: user) + post_graphql(query(id: global_id_of(label_list), issueFilters: { labelName: label2.title }), current_user: user) - expect(lists_data[0]['node']['title']).to eq label_list.title + aggregate_failures do + list_node = lists_data[0]['node'] + + expect(list_node['title']).to eq label_list.title + expect(list_node['issuesCount']).to eq 1 + end end end end diff --git a/spec/requests/api/graphql/gitlab_schema_spec.rb b/spec/requests/api/graphql/gitlab_schema_spec.rb index ee7dba545be..fe1c7c15de2 100644 --- a/spec/requests/api/graphql/gitlab_schema_spec.rb +++ b/spec/requests/api/graphql/gitlab_schema_spec.rb @@ -190,7 +190,9 @@ RSpec.describe 'GitlabSchema configurations' do variables: {}.to_s, complexity: 181, depth: 13, - duration_s: 7 + duration_s: 7, + used_fields: an_instance_of(Array), + used_deprecated_fields: an_instance_of(Array) } expect_any_instance_of(Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer).to receive(:duration).and_return(7) diff --git a/spec/requests/api/graphql/group/merge_requests_spec.rb b/spec/requests/api/graphql/group/merge_requests_spec.rb new file mode 100644 index 00000000000..e9a5e558b1d --- /dev/null +++ b/spec/requests/api/graphql/group/merge_requests_spec.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# Based on ee/spec/requests/api/epics_spec.rb +# Should follow closely in order to ensure all situations are covered +RSpec.describe 'Query.group.mergeRequests' do + include GraphqlHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:sub_group) { create(:group, parent: group) } + + let_it_be(:project_a) { create(:project, :repository, group: group) } + let_it_be(:project_b) { create(:project, :repository, group: group) } + let_it_be(:project_c) { create(:project, :repository, group: sub_group) } + let_it_be(:project_x) { create(:project, :repository) } + let_it_be(:user) { create(:user, developer_projects: [project_x]) } + + let_it_be(:mr_attrs) do + { target_branch: 'master' } + end + + let_it_be(:mr_traits) do + [:unique_branches, :unique_author] + end + + let_it_be(:mrs_a, reload: true) { create_list(:merge_request, 2, *mr_traits, **mr_attrs, source_project: project_a) } + let_it_be(:mrs_b, reload: true) { create_list(:merge_request, 2, *mr_traits, **mr_attrs, source_project: project_b) } + let_it_be(:mrs_c, reload: true) { create_list(:merge_request, 2, *mr_traits, **mr_attrs, source_project: project_c) } + let_it_be(:other_mr) { create(:merge_request, source_project: project_x) } + + let(:mrs_data) { graphql_data_at(:group, :merge_requests, :nodes) } + + before do + group.add_developer(user) + end + + def expected_mrs(mrs) + mrs.map { |mr| a_hash_including('id' => global_id_of(mr)) } + end + + describe 'not passing any arguments' do + let(:query) do + <<~GQL + query($path: ID!) { + group(fullPath: $path) { + mergeRequests { nodes { id } } + } + } + GQL + end + + it 'can find all merge requests in the group, excluding sub-groups' do + post_graphql(query, current_user: user, variables: { path: group.full_path }) + + expect(mrs_data).to match_array(expected_mrs(mrs_a + mrs_b)) + end + end + + describe 'restricting by author' do + let(:query) do + <<~GQL + query($path: ID!, $user: String) { + group(fullPath: $path) { + mergeRequests(authorUsername: $user) { nodes { id author { username } } } + } + } + GQL + end + + let(:author) { mrs_b.first.author } + + it 'can find all merge requests with user as author' do + post_graphql(query, current_user: user, variables: { user: author.username, path: group.full_path }) + + expect(mrs_data).to match_array(expected_mrs([mrs_b.first])) + end + end + + describe 'restricting by assignee' do + let(:query) do + <<~GQL + query($path: ID!, $user: String) { + group(fullPath: $path) { + mergeRequests(assigneeUsername: $user) { nodes { id } } + } + } + GQL + end + + let_it_be(:assignee) { create(:user) } + + before_all do + mrs_b.second.assignees << assignee + mrs_a.first.assignees << assignee + end + + it 'can find all merge requests assigned to user' do + post_graphql(query, current_user: user, variables: { user: assignee.username, path: group.full_path }) + + expect(mrs_data).to match_array(expected_mrs([mrs_a.first, mrs_b.second])) + end + end + + describe 'passing include_subgroups: true' do + let(:query) do + <<~GQL + query($path: ID!) { + group(fullPath: $path) { + mergeRequests(includeSubgroups: true) { nodes { id } } + } + } + GQL + end + + it 'can find all merge requests in the group, including sub-groups' do + post_graphql(query, current_user: user, variables: { path: group.full_path }) + + expect(mrs_data).to match_array(expected_mrs(mrs_a + mrs_b + mrs_c)) + end + end +end diff --git a/spec/requests/api/graphql/instance_statistics_measurements_spec.rb b/spec/requests/api/graphql/instance_statistics_measurements_spec.rb index b8cbe54534a..5d7dbcf2e3c 100644 --- a/spec/requests/api/graphql/instance_statistics_measurements_spec.rb +++ b/spec/requests/api/graphql/instance_statistics_measurements_spec.rb @@ -9,13 +9,16 @@ RSpec.describe 'InstanceStatisticsMeasurements' do let!(:instance_statistics_measurement_1) { create(:instance_statistics_measurement, :project_count, recorded_at: 20.days.ago, count: 5) } let!(:instance_statistics_measurement_2) { create(:instance_statistics_measurement, :project_count, recorded_at: 10.days.ago, count: 10) } - let(:query) { graphql_query_for(:instanceStatisticsMeasurements, 'identifier: PROJECTS', 'nodes { count }') } + let(:query) { graphql_query_for(:instanceStatisticsMeasurements, 'identifier: PROJECTS', 'nodes { count identifier }') } before do post_graphql(query, current_user: current_user) end it 'returns measurement objects' do - expect(graphql_data.dig('instanceStatisticsMeasurements', 'nodes')).to eq([{ "count" => 10 }, { "count" => 5 }]) + expect(graphql_data.dig('instanceStatisticsMeasurements', 'nodes')).to eq([ + { "count" => 10, 'identifier' => 'PROJECTS' }, + { "count" => 5, 'identifier' => 'PROJECTS' } + ]) end end diff --git a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb index 1d38bb39d59..3aaebb5095a 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb @@ -45,8 +45,9 @@ RSpec.describe 'Adding an AwardEmoji' do it_behaves_like 'a mutation that does not create an AwardEmoji' - it_behaves_like 'a mutation that returns top-level errors', - errors: ['Cannot award emoji to this resource'] + it_behaves_like 'a mutation that returns top-level errors' do + let(:match_errors) { include(/was provided invalid value for awardableId/) } + end end context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do diff --git a/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb index c6e8800de1f..7cd39f93ae7 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb @@ -50,8 +50,9 @@ RSpec.describe 'Removing an AwardEmoji' do it_behaves_like 'a mutation that does not destroy an AwardEmoji' - it_behaves_like 'a mutation that returns top-level errors', - errors: ['Cannot award emoji to this resource'] + it_behaves_like 'a mutation that returns top-level errors' do + let(:match_errors) { include(/was provided invalid value for awardableId/) } + end end context 'when the given awardable is an Awardable' do diff --git a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb index 2df59ce97ca..6910ad80a11 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb @@ -44,8 +44,9 @@ RSpec.describe 'Toggling an AwardEmoji' do it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' - it_behaves_like 'a mutation that returns top-level errors', - errors: ['Cannot award emoji to this resource'] + it_behaves_like 'a mutation that returns top-level errors' do + let(:match_errors) { include(/was provided invalid value for awardableId/) } + end end context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do diff --git a/spec/requests/api/graphql/mutations/boards/create_spec.rb b/spec/requests/api/graphql/mutations/boards/create_spec.rb new file mode 100644 index 00000000000..c5f981262ea --- /dev/null +++ b/spec/requests/api/graphql/mutations/boards/create_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::Boards::Create do + let_it_be(:parent) { create(:project) } + let(:project_path) { parent.full_path } + let(:params) do + { + project_path: project_path, + name: name + } + end + + it_behaves_like 'boards create mutation' +end diff --git a/spec/requests/api/graphql/mutations/boards/lists/destroy_spec.rb b/spec/requests/api/graphql/mutations/boards/lists/destroy_spec.rb new file mode 100644 index 00000000000..42f690f53ed --- /dev/null +++ b/spec/requests/api/graphql/mutations/boards/lists/destroy_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::Boards::Lists::Destroy do + include GraphqlHelpers + + let_it_be(:current_user, reload: true) { create(:user) } + let_it_be(:project, reload: true) { create(:project) } + let_it_be(:board) { create(:board, project: project) } + let_it_be(:list) { create(:list, board: board) } + let(:mutation) do + variables = { + list_id: GitlabSchema.id_from_object(list).to_s + } + + graphql_mutation(:destroy_board_list, variables) + end + + subject { post_graphql_mutation(mutation, current_user: current_user) } + + def mutation_response + graphql_mutation_response(:destroy_board_list) + end + + context 'when the user does not have permission' do + it_behaves_like 'a mutation that returns a top-level access error' + + it 'does not destroy the list' do + expect { subject }.not_to change { List.count } + end + end + + context 'when the user has permission' do + before do + project.add_maintainer(current_user) + end + + context 'when given id is not for a list' do + let_it_be(:list) { build_stubbed(:issue, project: project) } + + it 'returns an error' do + subject + + expect(graphql_errors.first['message']).to include('does not represent an instance of List') + end + end + + context 'when everything is ok' do + it 'destroys the list' do + expect { subject }.to change { List.count }.from(2).to(1) + end + + it 'returns an empty list' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('list') + expect(mutation_response['list']).to be_nil + end + end + + context 'when the list is not destroyable' do + let_it_be(:list) { create(:list, board: board, list_type: :backlog) } + + it 'does not destroy the list' do + expect { subject }.not_to change { List.count }.from(3) + end + + it 'returns an error and not nil list' do + subject + + expect(mutation_response['errors']).not_to be_empty + expect(mutation_response['list']).not_to be_nil + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/issues/create_spec.rb b/spec/requests/api/graphql/mutations/issues/create_spec.rb new file mode 100644 index 00000000000..39b408faa90 --- /dev/null +++ b/spec/requests/api/graphql/mutations/issues/create_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Create an issue' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:assignee1) { create(:user) } + let_it_be(:assignee2) { create(:user) } + let_it_be(:project_label1) { create(:label, project: project) } + let_it_be(:project_label2) { create(:label, project: project) } + let_it_be(:milestone) { create(:milestone, project: project) } + let_it_be(:new_label1) { FFaker::Lorem.word } + let_it_be(:new_label2) { FFaker::Lorem.word } + + let(:input) do + { + 'title' => 'new title', + 'description' => 'new description', + 'confidential' => true, + 'dueDate' => Date.tomorrow.strftime('%Y-%m-%d') + } + end + + let(:mutation) { graphql_mutation(:createIssue, input.merge('projectPath' => project.full_path, 'locked' => true)) } + + let(:mutation_response) { graphql_mutation_response(:create_issue) } + + context 'the user is not allowed to create an issue' do + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permissions to create an issue' do + before do + project.add_developer(current_user) + end + + it 'updates the issue' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['issue']).to include(input) + expect(mutation_response['issue']).to include('discussionLocked' => true) + end + end +end diff --git a/spec/requests/api/graphql/mutations/issues/move_spec.rb b/spec/requests/api/graphql/mutations/issues/move_spec.rb new file mode 100644 index 00000000000..5bbaff61edd --- /dev/null +++ b/spec/requests/api/graphql/mutations/issues/move_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Moving an issue' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:issue) { create(:issue) } + let_it_be(:target_project) { create(:project) } + + let(:mutation) do + variables = { + project_path: issue.project.full_path, + target_project_path: target_project.full_path, + iid: issue.iid.to_s + } + + graphql_mutation(:issue_move, variables, + <<-QL.strip_heredoc + clientMutationId + errors + issue { + title + } + QL + ) + end + + def mutation_response + graphql_mutation_response(:issue_move) + end + + context 'when the user is not allowed to read source project' do + it 'returns an error' do + error = "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to include(a_hash_including('message' => error)) + end + end + + context 'when the user is not allowed to move issue to target project' do + before do + issue.project.add_developer(user) + end + + it 'returns an error' do + error = "Cannot move issue due to insufficient permissions!" + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['errors'][0]).to eq(error) + end + end + + context 'when the user is allowed to move issue' do + before do + issue.project.add_developer(user) + target_project.add_developer(user) + end + + it 'moves the issue' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response.dig('issue', 'title')).to eq(issue.title) + expect(issue.reload.state).to eq('closed') + expect(target_project.issues.find_by_title(issue.title)).to be_present + end + end +end diff --git a/spec/requests/api/graphql/mutations/issues/update_spec.rb b/spec/requests/api/graphql/mutations/issues/update_spec.rb index af52f9d57a3..71f25dbbe49 100644 --- a/spec/requests/api/graphql/mutations/issues/update_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/update_spec.rb @@ -10,13 +10,15 @@ RSpec.describe 'Update of an existing issue' do let_it_be(:issue) { create(:issue, project: project) } let(:input) do { - project_path: project.full_path, - iid: issue.iid.to_s, - locked: true + 'iid' => issue.iid.to_s, + 'title' => 'new title', + 'description' => 'new description', + 'confidential' => true, + 'dueDate' => Date.tomorrow.strftime('%Y-%m-%d') } end - let(:mutation) { graphql_mutation(:update_issue, input) } + let(:mutation) { graphql_mutation(:update_issue, input.merge(project_path: project.full_path, locked: true)) } let(:mutation_response) { graphql_mutation_response(:update_issue) } context 'the user is not allowed to update issue' do @@ -32,9 +34,8 @@ RSpec.describe 'Update of an existing issue' do post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) - expect(mutation_response['issue']).to include( - 'discussionLocked' => true - ) + expect(mutation_response['issue']).to include(input) + expect(mutation_response['issue']).to include('discussionLocked' => true) end end end diff --git a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb index 10ca2cf1cf8..81d13b29dde 100644 --- a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb +++ b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb @@ -101,7 +101,9 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do graphql_mutation(:create_annotation, variables) end - it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab ID.'] + it_behaves_like 'a mutation that returns top-level errors' do + let(:match_errors) { include(/is not a valid Global ID/) } + end end end end @@ -109,7 +111,7 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do context 'when annotation source is cluster' do let(:mutation) do variables = { - cluster_id: GitlabSchema.id_from_object(cluster).to_s, + cluster_id: cluster.to_global_id.to_s, starting_at: starting_at, ending_at: ending_at, dashboard_path: dashboard_path, @@ -188,15 +190,17 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do graphql_mutation(:create_annotation, variables) end - it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab ID.'] + it_behaves_like 'a mutation that returns top-level errors' do + let(:match_errors) { include(/is not a valid Global ID/) } + end end end context 'when both environment_id and cluster_id are provided' do let(:mutation) do variables = { - environment_id: GitlabSchema.id_from_object(environment).to_s, - cluster_id: GitlabSchema.id_from_object(cluster).to_s, + environment_id: environment.to_global_id.to_s, + cluster_id: cluster.to_global_id.to_s, starting_at: starting_at, ending_at: ending_at, dashboard_path: dashboard_path, @@ -210,14 +214,14 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do end context 'when a non-cluster or environment id is provided' do + let(:gid) { { environment_id: project.to_global_id.to_s } } let(:mutation) do variables = { - environment_id: GitlabSchema.id_from_object(project).to_s, starting_at: starting_at, ending_at: ending_at, dashboard_path: dashboard_path, description: description - } + }.merge!(gid) graphql_mutation(:create_annotation, variables) end @@ -226,6 +230,18 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do project.add_developer(current_user) end - it_behaves_like 'a mutation that returns top-level errors', errors: [described_class::INVALID_ANNOTATION_SOURCE_ERROR] + describe 'non-environment id' do + it_behaves_like 'a mutation that returns top-level errors' do + let(:match_errors) { include(/does not represent an instance of Environment/) } + end + end + + describe 'non-cluster id' do + let(:gid) { { cluster_id: project.to_global_id.to_s } } + + it_behaves_like 'a mutation that returns top-level errors' do + let(:match_errors) { include(/does not represent an instance of Clusters::Cluster/) } + end + end end end diff --git a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb index 391ced7dc98..6d761eb0a54 100644 --- a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb @@ -60,6 +60,14 @@ RSpec.describe 'Adding a Note' do expect(mutation_response['note']['discussion']['id']).to eq(discussion.to_global_id.to_s) end + + context 'when the discussion_id is not for a Discussion' do + let(:discussion) { create(:issue) } + + it_behaves_like 'a mutation that returns top-level errors' do + let(:match_errors) { include(/ does not represent an instance of Discussion/) } + end + end end end end diff --git a/spec/requests/api/graphql/mutations/notes/update/image_diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/update/image_diff_note_spec.rb index 0c00906d6bf..efa2ceb65c2 100644 --- a/spec/requests/api/graphql/mutations/notes/update/image_diff_note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/update/image_diff_note_spec.rb @@ -178,6 +178,12 @@ RSpec.describe 'Updating an image DiffNote' do it_behaves_like 'a mutation that returns top-level errors', errors: ['body or position arguments are required'] end + context 'when the resource is not a Note' do + let(:diff_note) { note } + + it_behaves_like 'a Note mutation when the given resource id is not for a Note' + end + context 'when resource is not a DiffNote on an image' do let!(:diff_note) { create(:diff_note_on_merge_request, note: original_body) } diff --git a/spec/requests/api/graphql/mutations/snippets/create_spec.rb b/spec/requests/api/graphql/mutations/snippets/create_spec.rb index 1bb446de708..d2fa3cfc24f 100644 --- a/spec/requests/api/graphql/mutations/snippets/create_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/create_spec.rb @@ -76,21 +76,25 @@ RSpec.describe 'Creating a Snippet' do expect(mutation_response['snippet']).to be_nil end + + it_behaves_like 'spam flag is present' end shared_examples 'creates snippet' do - it 'returns the created Snippet' do + it 'returns the created Snippet', :aggregate_failures do expect do subject end.to change { Snippet.count }.by(1) + snippet = Snippet.last + created_file_1 = snippet.repository.blob_at('HEAD', file_1[:filePath]) + created_file_2 = snippet.repository.blob_at('HEAD', file_2[:filePath]) + + expect(created_file_1.data).to match(file_1[:content]) + expect(created_file_2.data).to match(file_2[:content]) expect(mutation_response['snippet']['title']).to eq(title) expect(mutation_response['snippet']['description']).to eq(description) expect(mutation_response['snippet']['visibilityLevel']).to eq(visibility_level) - expect(mutation_response['snippet']['blobs'][0]['plainData']).to match(file_1[:content]) - expect(mutation_response['snippet']['blobs'][0]['fileName']).to match(file_1[:file_path]) - expect(mutation_response['snippet']['blobs'][1]['plainData']).to match(file_2[:content]) - expect(mutation_response['snippet']['blobs'][1]['fileName']).to match(file_2[:file_path]) end context 'when action is invalid' do @@ -101,6 +105,10 @@ RSpec.describe 'Creating a Snippet' do end it_behaves_like 'snippet edit usage data counters' + it_behaves_like 'spam flag is present' + it_behaves_like 'can raise spam flag' do + let(:service) { Snippets::CreateService } + end end context 'with PersonalSnippet' do @@ -140,6 +148,9 @@ RSpec.describe 'Creating a Snippet' do it_behaves_like 'a mutation that returns errors in the response', errors: ["Title can't be blank"] it_behaves_like 'does not create snippet' + it_behaves_like 'can raise spam flag' do + let(:service) { Snippets::CreateService } + end end context 'when there non ActiveRecord errors' do diff --git a/spec/requests/api/graphql/mutations/snippets/update_spec.rb b/spec/requests/api/graphql/mutations/snippets/update_spec.rb index 58ce74b9263..21d403c6f73 100644 --- a/spec/requests/api/graphql/mutations/snippets/update_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/update_spec.rb @@ -37,6 +37,8 @@ RSpec.describe 'Updating a Snippet' do graphql_mutation_response(:update_snippet) end + subject { post_graphql_mutation(mutation, current_user: current_user) } + shared_examples 'graphql update actions' do context 'when the user does not have permission' do let(:current_user) { create(:user) } @@ -46,14 +48,14 @@ RSpec.describe 'Updating a Snippet' do it 'does not update the Snippet' do expect do - post_graphql_mutation(mutation, current_user: current_user) + subject end.not_to change { snippet.reload } end end context 'when the user has permission' do it 'updates the snippet record' do - post_graphql_mutation(mutation, current_user: current_user) + subject expect(snippet.reload.title).to eq(updated_title) end @@ -65,7 +67,7 @@ RSpec.describe 'Updating a Snippet' do expect(blob_to_update.data).not_to eq updated_content expect(blob_to_delete).to be_present - post_graphql_mutation(mutation, current_user: current_user) + subject blob_to_update = blob_at(updated_file) blob_to_delete = blob_at(deleted_file) @@ -73,20 +75,25 @@ RSpec.describe 'Updating a Snippet' do aggregate_failures do expect(blob_to_update.data).to eq updated_content expect(blob_to_delete).to be_nil - expect(blob_in_mutation_response(updated_file)['plainData']).to match(updated_content) expect(mutation_response['snippet']['title']).to eq(updated_title) expect(mutation_response['snippet']['description']).to eq(updated_description) expect(mutation_response['snippet']['visibilityLevel']).to eq('public') end end + it_behaves_like 'can raise spam flag' do + let(:service) { Snippets::UpdateService } + end + + it_behaves_like 'spam flag is present' + context 'when there are ActiveRecord validation errors' do let(:updated_title) { '' } it_behaves_like 'a mutation that returns errors in the response', errors: ["Title can't be blank"] it 'does not update the Snippet' do - post_graphql_mutation(mutation, current_user: current_user) + subject expect(snippet.reload.title).to eq(original_title) end @@ -95,21 +102,21 @@ RSpec.describe 'Updating a Snippet' do blob_to_update = blob_at(updated_file) blob_to_delete = blob_at(deleted_file) - post_graphql_mutation(mutation, current_user: current_user) + subject aggregate_failures do expect(blob_at(updated_file).data).to eq blob_to_update.data expect(blob_at(deleted_file).data).to eq blob_to_delete.data - expect(blob_in_mutation_response(deleted_file)['plainData']).not_to be_nil expect(mutation_response['snippet']['title']).to eq(original_title) expect(mutation_response['snippet']['description']).to eq(original_description) expect(mutation_response['snippet']['visibilityLevel']).to eq('private') end end - end - def blob_in_mutation_response(filename) - mutation_response['snippet']['blobs'].select { |blob| blob['name'] == filename }[0] + it_behaves_like 'spam flag is present' + it_behaves_like 'can raise spam flag' do + let(:service) { Snippets::UpdateService } + end end def blob_at(filename) @@ -150,7 +157,7 @@ RSpec.describe 'Updating a Snippet' do context 'when the author is not a member of the project' do it 'returns an an error' do - post_graphql_mutation(mutation, current_user: current_user) + subject errors = json_response['errors'] expect(errors.first['message']).to eq(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) @@ -168,7 +175,7 @@ RSpec.describe 'Updating a Snippet' do it 'returns an an error' do project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::DISABLED) - post_graphql_mutation(mutation, current_user: current_user) + subject errors = json_response['errors'] expect(errors.first['message']).to eq(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) diff --git a/spec/requests/api/graphql/mutations/todos/mark_done_spec.rb b/spec/requests/api/graphql/mutations/todos/mark_done_spec.rb index 8bf8b96aff5..8a9a0b9e845 100644 --- a/spec/requests/api/graphql/mutations/todos/mark_done_spec.rb +++ b/spec/requests/api/graphql/mutations/todos/mark_done_spec.rb @@ -76,15 +76,15 @@ RSpec.describe 'Marking todos done' do end context 'when using an invalid gid' do - let(:input) { { id: 'invalid_gid' } } - let(:invalid_gid_error) { 'invalid_gid is not a valid GitLab ID.' } + let(:input) { { id: GitlabSchema.id_from_object(author).to_s } } + let(:invalid_gid_error) { /"#{input[:id]}" does not represent an instance of #{todo1.class}/ } it 'contains the expected error' do post_graphql_mutation(mutation, current_user: current_user) errors = json_response['errors'] expect(errors).not_to be_blank - expect(errors.first['message']).to eq(invalid_gid_error) + expect(errors.first['message']).to match(invalid_gid_error) expect(todo1.reload.state).to eq('pending') expect(todo2.reload.state).to eq('done') diff --git a/spec/requests/api/graphql/mutations/todos/restore_spec.rb b/spec/requests/api/graphql/mutations/todos/restore_spec.rb index 8451dcdf587..a58c7fc69fc 100644 --- a/spec/requests/api/graphql/mutations/todos/restore_spec.rb +++ b/spec/requests/api/graphql/mutations/todos/restore_spec.rb @@ -76,15 +76,15 @@ RSpec.describe 'Restoring Todos' do end context 'when using an invalid gid' do - let(:input) { { id: 'invalid_gid' } } - let(:invalid_gid_error) { 'invalid_gid is not a valid GitLab ID.' } + let(:input) { { id: GitlabSchema.id_from_object(author).to_s } } + let(:invalid_gid_error) { /"#{input[:id]}" does not represent an instance of #{todo1.class}/ } it 'contains the expected error' do post_graphql_mutation(mutation, current_user: current_user) errors = json_response['errors'] expect(errors).not_to be_blank - expect(errors.first['message']).to eq(invalid_gid_error) + expect(errors.first['message']).to match(invalid_gid_error) expect(todo1.reload.state).to eq('done') expect(todo2.reload.state).to eq('pending') diff --git a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb index d3a2e6a1deb..8deed75a466 100644 --- a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb +++ b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb @@ -139,6 +139,19 @@ RSpec.describe 'getting Alert Management Alerts' do it { expect(alerts.size).to eq(0) } end end + + context 'assignee_username' do + let(:alert) { triggered_alert } + let(:assignee) { alert.assignees.first! } + let(:params) { { assignee_username: assignee.username } } + + it_behaves_like 'a working graphql query' + + specify do + expect(alerts.size).to eq(1) + expect(first_alert['iid']).to eq(alert.iid.to_s) + end + end end end end diff --git a/spec/requests/api/graphql/project/issue/designs/notes_spec.rb b/spec/requests/api/graphql/project/issue/designs/notes_spec.rb index 65191e057c7..e25453510d5 100644 --- a/spec/requests/api/graphql/project/issue/designs/notes_spec.rb +++ b/spec/requests/api/graphql/project/issue/designs/notes_spec.rb @@ -31,8 +31,8 @@ RSpec.describe 'Getting designs related to an issue' do post_graphql(query(note_fields), current_user: nil) designs_data = graphql_data['project']['issue']['designs']['designs'] - design_data = designs_data['edges'].first['node'] - note_data = design_data['notes']['edges'].first['node'] + design_data = designs_data['nodes'].first + note_data = design_data['notes']['nodes'].first expect(note_data['id']).to eq(note.to_global_id.to_s) end @@ -40,14 +40,10 @@ RSpec.describe 'Getting designs related to an issue' do def query(note_fields = all_graphql_fields_for(Note)) design_node = <<~NODE designs { - edges { - node { - notes { - edges { - node { - #{note_fields} - } - } + nodes { + notes { + nodes { + #{note_fields} } } } diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb index 5d4276f47ca..40fec6ba068 100644 --- a/spec/requests/api/graphql/project/issues_spec.rb +++ b/spec/requests/api/graphql/project/issues_spec.rb @@ -53,16 +53,37 @@ RSpec.describe 'getting an issue list for a project' do context 'when limiting the number of results' do let(:query) do - graphql_query_for( - 'project', - { 'fullPath' => project.full_path }, - "issues(first: 1) { #{fields} }" - ) + <<~GQL + query($path: ID!, $n: Int) { + project(fullPath: $path) { + issues(first: $n) { #{fields} } + } + } + GQL + end + + let(:issue_limit) { 1 } + let(:variables) do + { path: project.full_path, n: issue_limit } end it_behaves_like 'a working graphql query' do before do - post_graphql(query, current_user: current_user) + post_graphql(query, current_user: current_user, variables: variables) + end + + it 'only returns N issues' do + expect(issues_data.size).to eq(issue_limit) + end + end + + context 'no limit is provided' do + let(:issue_limit) { nil } + + it 'returns all issues' do + post_graphql(query, current_user: current_user, variables: variables) + + expect(issues_data.size).to be > 1 end end @@ -71,7 +92,7 @@ RSpec.describe 'getting an issue list for a project' do # Newest first, we only want to see the newest checked expect(Ability).not_to receive(:allowed?).with(current_user, :read_issue, issues.first) - post_graphql(query, current_user: current_user) + post_graphql(query, current_user: current_user, variables: variables) 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 22b003501a1..c737e0b8caf 100644 --- a/spec/requests/api/graphql/project/merge_requests_spec.rb +++ b/spec/requests/api/graphql/project/merge_requests_spec.rb @@ -13,6 +13,7 @@ RSpec.describe 'getting merge request listings nested in a project' do let_it_be(:merge_request_b) { create(:merge_request, :closed, :unique_branches, source_project: project) } let_it_be(:merge_request_c) { create(:labeled_merge_request, :closed, :unique_branches, source_project: project, labels: [label]) } let_it_be(:merge_request_d) { create(:merge_request, :locked, :unique_branches, source_project: project) } + let_it_be(:merge_request_e) { create(:merge_request, :unique_branches, source_project: project) } let(:results) { graphql_data.dig('project', 'mergeRequests', 'nodes') } @@ -118,7 +119,7 @@ RSpec.describe 'getting merge request listings nested in a project' do context 'there are no search params' do let(:search_params) { nil } - let(:mrs) { [merge_request_a, merge_request_b, merge_request_c, merge_request_d] } + let(:mrs) { [merge_request_a, merge_request_b, merge_request_c, merge_request_d, merge_request_e] } it_behaves_like 'searching with parameters' end @@ -172,6 +173,28 @@ RSpec.describe 'getting merge request listings nested in a project' do it_behaves_like 'searching with parameters' end + context 'when requesting `approved_by`' do + let(:search_params) { { iids: [merge_request_a.iid.to_s, merge_request_b.iid.to_s] } } + let(:extra_iid_for_second_query) { merge_request_c.iid.to_s } + let(:requested_fields) { query_graphql_field(:approved_by, nil, query_graphql_field(:nodes, nil, [:username])) } + + def execute_query + query = query_merge_requests(requested_fields) + post_graphql(query, current_user: current_user) + end + + it 'exposes approver username' do + merge_request_a.approved_by_users << current_user + + execute_query + + user_data = { 'username' => current_user.username } + expect(results).to include(a_hash_including('approvedBy' => { 'nodes' => array_including(user_data) })) + end + + include_examples 'N+1 query check' + end + describe 'fields' do let(:requested_fields) { nil } let(:extra_iid_for_second_query) { merge_request_c.iid.to_s } @@ -209,7 +232,19 @@ RSpec.describe 'getting merge request listings nested in a project' do include_examples 'N+1 query check' end + + context 'when requesting `user_notes_count`' do + let(:requested_fields) { [:user_notes_count] } + + before do + create_list(:note_on_merge_request, 2, noteable: merge_request_a, project: project) + create(:note_on_merge_request, noteable: merge_request_c, project: project) + end + + include_examples 'N+1 query check' + end end + describe 'sorting and pagination' do let(:data_path) { [:project, :mergeRequests] } @@ -241,16 +276,50 @@ RSpec.describe 'getting merge request listings nested in a project' do let(:expected_results) do [ merge_request_b, - merge_request_c, merge_request_d, + merge_request_c, + merge_request_e, merge_request_a ].map(&:to_gid).map(&:to_s) end before do - merge_request_c.metrics.update!(merged_at: 5.days.ago) + five_days_ago = 5.days.ago + + merge_request_d.metrics.update!(merged_at: five_days_ago) + + # same merged_at, the second order column will decide (merge_request.id) + merge_request_c.metrics.update!(merged_at: five_days_ago) + merge_request_b.metrics.update!(merged_at: 1.day.ago) end + + context 'when paginating backwards' do + let(:params) { 'first: 2, sort: MERGED_AT_DESC' } + let(:page_info) { 'pageInfo { startCursor endCursor }' } + + before do + post_graphql(pagination_query(params, page_info), current_user: current_user) + end + + it 'paginates backwards correctly' do + # first page + first_page_response_data = graphql_dig_at(Gitlab::Json.parse(response.body), :data, *data_path, :edges) + end_cursor = graphql_dig_at(Gitlab::Json.parse(response.body), :data, :project, :mergeRequests, :pageInfo, :endCursor) + + # second page + params = "first: 2, after: \"#{end_cursor}\", sort: MERGED_AT_DESC" + post_graphql(pagination_query(params, page_info), current_user: current_user) + start_cursor = graphql_dig_at(Gitlab::Json.parse(response.body), :data, :project, :mergeRequests, :pageInfo, :start_cursor) + + # going back to the first page + + params = "last: 2, before: \"#{start_cursor}\", sort: MERGED_AT_DESC" + post_graphql(pagination_query(params, page_info), current_user: current_user) + backward_paginated_response_data = graphql_dig_at(Gitlab::Json.parse(response.body), :data, *data_path, :edges) + expect(first_page_response_data).to eq(backward_paginated_response_data) + end + end end end end diff --git a/spec/requests/api/graphql/project/milestones_spec.rb b/spec/requests/api/graphql/project/milestones_spec.rb new file mode 100644 index 00000000000..2fede4c7285 --- /dev/null +++ b/spec/requests/api/graphql/project/milestones_spec.rb @@ -0,0 +1,202 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting milestone listings nested in a project' do + include GraphqlHelpers + + let_it_be(:today) { Time.now.utc.to_date } + let_it_be(:project) { create(:project, :repository, :public) } + let_it_be(:current_user) { create(:user) } + + let_it_be(:no_dates) { create(:milestone, project: project, title: 'no dates') } + let_it_be(:no_end) { create(:milestone, project: project, title: 'no end', start_date: today - 10.days) } + let_it_be(:no_start) { create(:milestone, project: project, title: 'no start', due_date: today - 5.days) } + let_it_be(:fully_past) { create(:milestone, project: project, title: 'past', start_date: today - 10.days, due_date: today - 5.days) } + let_it_be(:covers_today) { create(:milestone, project: project, title: 'present', start_date: today - 5.days, due_date: today + 5.days) } + let_it_be(:fully_future) { create(:milestone, project: project, title: 'future', start_date: today + 5.days, due_date: today + 10.days) } + let_it_be(:closed) { create(:milestone, :closed, project: project) } + + let(:results) { graphql_data_at(:project, :milestones, :nodes) } + + let(:search_params) { nil } + + def query_milestones(fields) + graphql_query_for( + :project, + { full_path: project.full_path }, + query_graphql_field(:milestones, search_params, [ + query_graphql_field(:nodes, nil, %i[id title]) + ]) + ) + end + + def result_list(expected) + expected.map do |milestone| + a_hash_including('id' => global_id_of(milestone)) + end + end + + let(:query) do + query_milestones(all_graphql_fields_for('Milestone', max_depth: 1)) + end + + let(:all_milestones) do + [no_dates, no_end, no_start, fully_past, fully_future, covers_today, closed] + end + + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + shared_examples 'searching with parameters' do + it 'finds the right milestones' do + post_graphql(query, current_user: current_user) + + expect(results).to match_array(result_list(expected)) + end + end + + context 'there are no search params' do + let(:search_params) { nil } + let(:expected) { all_milestones } + + it_behaves_like 'searching with parameters' + end + + context 'the search params do not match anything' do + let(:search_params) { { title: 'wibble' } } + let(:expected) { [] } + + it_behaves_like 'searching with parameters' + end + + context 'searching by state:closed' do + let(:search_params) { { state: :closed } } + let(:expected) { [closed] } + + it_behaves_like 'searching with parameters' + end + + context 'searching by state:active' do + let(:search_params) { { state: :active } } + let(:expected) { all_milestones - [closed] } + + it_behaves_like 'searching with parameters' + end + + context 'searching by title' do + let(:search_params) { { title: 'no start' } } + let(:expected) { [no_start] } + + it_behaves_like 'searching with parameters' + end + + context 'searching by search_title' do + let(:search_params) { { search_title: 'no' } } + let(:expected) { [no_dates, no_start, no_end] } + + it_behaves_like 'searching with parameters' + end + + context 'searching by containing_date' do + let(:search_params) { { containing_date: (today - 7.days).iso8601 } } + let(:expected) { [no_start, no_end, fully_past] } + + it_behaves_like 'searching with parameters' + end + + context 'searching by containing_date = today' do + let(:search_params) { { containing_date: today.iso8601 } } + let(:expected) { [no_end, covers_today] } + + it_behaves_like 'searching with parameters' + end + + context 'searching by custom range' do + let(:expected) { [no_end, fully_future] } + let(:search_params) do + { + start_date: (today + 6.days).iso8601, + end_date: (today + 7.days).iso8601 + } + end + + it_behaves_like 'searching with parameters' + end + + context 'using timeframe argument' do + let(:expected) { [no_end, fully_future] } + let(:search_params) do + { + timeframe: { + start: (today + 6.days).iso8601, + end: (today + 7.days).iso8601 + } + } + end + + it_behaves_like 'searching with parameters' + end + + describe 'timeframe validations' do + let(:vars) do + { + path: project.full_path, + start: (today + 6.days).iso8601, + end: (today + 7.days).iso8601 + } + end + + it_behaves_like 'a working graphql query' do + before do + query = <<~GQL + query($path: ID!, $start: Date!, $end: Date!) { + project(fullPath: $path) { + milestones(timeframe: { start: $start, end: $end }) { + nodes { id } + } + } + } + GQL + + post_graphql(query, current_user: current_user, variables: vars) + end + end + + it 'is invalid to provide timeframe and start_date/end_date' do + query = <<~GQL + query($path: ID!, $tstart: Date!, $tend: Date!, $start: Time!, $end: Time!) { + project(fullPath: $path) { + milestones(timeframe: { start: $tstart, end: $tend }, startDate: $start, endDate: $end) { + nodes { id } + } + } + } + GQL + + post_graphql(query, current_user: current_user, + variables: vars.merge(vars.transform_keys { |k| :"t#{k}" })) + + expect(graphql_errors).to contain_exactly(a_hash_including('message' => include('deprecated in favor of timeframe'))) + end + + it 'is invalid to invert the timeframe arguments' do + query = <<~GQL + query($path: ID!, $start: Date!, $end: Date!) { + project(fullPath: $path) { + milestones(timeframe: { start: $end, end: $start }) { + nodes { id } + } + } + } + GQL + + post_graphql(query, current_user: current_user, variables: vars) + + expect(graphql_errors).to contain_exactly(a_hash_including('message' => include('start must be before end'))) + end + end +end diff --git a/spec/requests/api/graphql/user_query_spec.rb b/spec/requests/api/graphql/user_query_spec.rb index 2f4dc0a9160..79debd0b7ef 100644 --- a/spec/requests/api/graphql/user_query_spec.rb +++ b/spec/requests/api/graphql/user_query_spec.rb @@ -29,15 +29,15 @@ RSpec.describe 'getting user information' do let_it_be(:unauthorized_user) { create(:user) } let_it_be(:assigned_mr) do - create(:merge_request, :unique_branches, + create(:merge_request, :unique_branches, :unique_author, source_project: project_a, assignees: [user]) end let_it_be(:assigned_mr_b) do - create(:merge_request, :unique_branches, + create(:merge_request, :unique_branches, :unique_author, source_project: project_b, assignees: [user]) end let_it_be(:assigned_mr_c) do - create(:merge_request, :unique_branches, + create(:merge_request, :unique_branches, :unique_author, source_project: project_b, assignees: [user]) end let_it_be(:authored_mr) do @@ -133,6 +133,17 @@ RSpec.describe 'getting user information' do ) end end + + context 'filtering by author' do + let(:author) { assigned_mr_b.author } + let(:mr_args) { { author_username: author.username } } + + it 'finds the authored mrs' do + expect(assigned_mrs).to contain_exactly( + a_hash_including('id' => global_id_of(assigned_mr_b)) + ) + end + end end context 'the current user does not have access' do @@ -172,6 +183,23 @@ RSpec.describe 'getting user information' do end end + context 'filtering by assignee' do + let(:assignee) { create(:user) } + let(:mr_args) { { assignee_username: assignee.username } } + + it 'finds the assigned mrs' do + authored_mr.assignees << assignee + authored_mr_c.assignees << assignee + + post_graphql(query, current_user: current_user) + + expect(authored_mrs).to contain_exactly( + a_hash_including('id' => global_id_of(authored_mr)), + a_hash_including('id' => global_id_of(authored_mr_c)) + ) + end + end + context 'filtering by project path and IID' do let(:mr_args) do { project_path: project_b.full_path, iids: [authored_mr_b.iid.to_s] } @@ -253,8 +281,10 @@ RSpec.describe 'getting user information' do let(:current_user) { user } it 'can be found' do - expect(assigned_mrs).to include( - a_hash_including('id' => global_id_of(assigned_mr)) + expect(assigned_mrs).to contain_exactly( + a_hash_including('id' => global_id_of(assigned_mr)), + a_hash_including('id' => global_id_of(assigned_mr_b)), + a_hash_including('id' => global_id_of(assigned_mr_c)) ) end end diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb index ff1a5aa1540..94a66f54e4d 100644 --- a/spec/requests/api/graphql_spec.rb +++ b/spec/requests/api/graphql_spec.rb @@ -9,7 +9,15 @@ RSpec.describe 'GraphQL' do context 'logging' do shared_examples 'logging a graphql query' do let(:expected_params) do - { query_string: query, variables: variables.to_s, duration_s: anything, depth: 1, complexity: 1 } + { + query_string: query, + variables: variables.to_s, + duration_s: anything, + depth: 1, + complexity: 1, + used_fields: ['Query.echo'], + used_deprecated_fields: [] + } end it 'logs a query with the expected params' do diff --git a/spec/requests/api/group_clusters_spec.rb b/spec/requests/api/group_clusters_spec.rb index 068af1485e2..eb21ae9468c 100644 --- a/spec/requests/api/group_clusters_spec.rb +++ b/spec/requests/api/group_clusters_spec.rb @@ -172,6 +172,7 @@ RSpec.describe API::GroupClusters do name: 'test-cluster', domain: 'domain.example.com', managed: false, + namespace_per_environment: false, platform_kubernetes_attributes: platform_kubernetes_attributes, management_project_id: management_project_id } @@ -206,6 +207,7 @@ RSpec.describe API::GroupClusters do expect(cluster_result.domain).to eq('domain.example.com') expect(cluster_result.managed).to be_falsy expect(cluster_result.management_project_id).to eq management_project_id + expect(cluster_result.namespace_per_environment).to eq(false) expect(platform_kubernetes.rbac?).to be_truthy expect(platform_kubernetes.api_url).to eq(api_url) expect(platform_kubernetes.token).to eq('sample-token') @@ -237,6 +239,22 @@ RSpec.describe API::GroupClusters do end end + context 'when namespace_per_environment is not set' do + let(:cluster_params) do + { + name: 'test-cluster', + domain: 'domain.example.com', + platform_kubernetes_attributes: platform_kubernetes_attributes + } + end + + it 'defaults to true' do + cluster_result = Clusters::Cluster.find(json_response['id']) + + expect(cluster_result).to be_namespace_per_environment + end + end + context 'current user does not have access to management_project_id' do let(:management_project_id) { create(:project).id } diff --git a/spec/requests/api/group_container_repositories_spec.rb b/spec/requests/api/group_container_repositories_spec.rb index 3128becae6d..4584ef37bd0 100644 --- a/spec/requests/api/group_container_repositories_spec.rb +++ b/spec/requests/api/group_container_repositories_spec.rb @@ -25,7 +25,6 @@ RSpec.describe API::GroupContainerRepositories do group.add_reporter(reporter) group.add_guest(guest) - stub_feature_flags(container_registry_api: true) stub_container_registry_config(enabled: true) root_repository @@ -44,7 +43,7 @@ RSpec.describe API::GroupContainerRepositories do let(:object) { group } end - it_behaves_like 'a gitlab tracking event', described_class.name, 'list_repositories' + it_behaves_like 'a package tracking event', described_class.name, 'list_repositories' context 'with invalid group id' do let(:url) { "/groups/#{non_existing_record_id}/registry/repositories" } diff --git a/spec/requests/api/group_packages_spec.rb b/spec/requests/api/group_packages_spec.rb index f67cafbd8f5..72ba25c59af 100644 --- a/spec/requests/api/group_packages_spec.rb +++ b/spec/requests/api/group_packages_spec.rb @@ -77,7 +77,7 @@ RSpec.describe API::GroupPackages do it_behaves_like 'returns packages', :group, :owner it_behaves_like 'returns packages', :group, :maintainer it_behaves_like 'returns packages', :group, :developer - it_behaves_like 'rejects packages access', :group, :reporter, :forbidden + it_behaves_like 'returns packages', :group, :reporter it_behaves_like 'rejects packages access', :group, :guest, :forbidden context 'with subgroup' do @@ -88,7 +88,7 @@ RSpec.describe API::GroupPackages do it_behaves_like 'returns packages with subgroups', :group, :owner it_behaves_like 'returns packages with subgroups', :group, :maintainer it_behaves_like 'returns packages with subgroups', :group, :developer - it_behaves_like 'rejects packages access', :group, :reporter, :forbidden + it_behaves_like 'returns packages with subgroups', :group, :reporter it_behaves_like 'rejects packages access', :group, :guest, :forbidden context 'excluding subgroup' do @@ -97,7 +97,7 @@ RSpec.describe API::GroupPackages do it_behaves_like 'returns packages', :group, :owner it_behaves_like 'returns packages', :group, :maintainer it_behaves_like 'returns packages', :group, :developer - it_behaves_like 'rejects packages access', :group, :reporter, :forbidden + it_behaves_like 'returns packages', :group, :reporter it_behaves_like 'rejects packages access', :group, :guest, :forbidden end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index da423e986c3..c7756a4fae5 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -1391,6 +1391,139 @@ RSpec.describe API::Groups do end end + describe 'GET /groups/:id/descendant_groups' do + let_it_be(:child_group1) { create(:group, parent: group1) } + let_it_be(:private_child_group1) { create(:group, :private, parent: group1) } + let_it_be(:sub_child_group1) { create(:group, parent: child_group1) } + let_it_be(:child_group2) { create(:group, :private, parent: group2) } + let_it_be(:sub_child_group2) { create(:group, :private, parent: child_group2) } + let(:response_groups) { json_response.map { |group| group['name'] } } + + context 'when unauthenticated' do + it 'returns only public descendants' do + get api("/groups/#{group1.id}/descendant_groups") + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.length).to eq(2) + expect(response_groups).to contain_exactly(child_group1.name, sub_child_group1.name) + end + + it 'returns 404 for a private group' do + get api("/groups/#{group2.id}/descendant_groups") + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when authenticated as user' do + context 'when user is not member of a public group' do + it 'returns no descendants for the public group' do + get api("/groups/#{group1.id}/descendant_groups", user2) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.length).to eq(0) + end + + context 'when using all_available in request' do + it 'returns public descendants' do + get api("/groups/#{group1.id}/descendant_groups", user2), params: { all_available: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.length).to eq(2) + expect(response_groups).to contain_exactly(child_group1.name, sub_child_group1.name) + end + end + end + + context 'when user is not member of a private group' do + it 'returns 404 for the private group' do + get api("/groups/#{group2.id}/descendant_groups", user1) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user is member of public group' do + before do + group1.add_guest(user2) + end + + it 'returns private descendants' do + get api("/groups/#{group1.id}/descendant_groups", user2) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.length).to eq(3) + expect(response_groups).to contain_exactly(child_group1.name, sub_child_group1.name, private_child_group1.name) + end + + context 'when using statistics in request' do + it 'does not include statistics' do + get api("/groups/#{group1.id}/descendant_groups", user2), params: { statistics: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.first).not_to include 'statistics' + end + end + end + + context 'when user is member of private group' do + before do + group2.add_guest(user1) + end + + it 'returns descendants' do + get api("/groups/#{group2.id}/descendant_groups", user1) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.length).to eq(2) + expect(response_groups).to contain_exactly(child_group2.name, sub_child_group2.name) + end + end + end + + context 'when authenticated as admin' do + it 'returns private descendants of a public group' do + get api("/groups/#{group1.id}/descendant_groups", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.length).to eq(3) + end + + it 'returns descendants of a private group' do + get api("/groups/#{group2.id}/descendant_groups", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.length).to eq(2) + end + + it 'does not include statistics by default' do + get api("/groups/#{group1.id}/descendant_groups", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.first).not_to include('statistics') + end + + it 'includes statistics if requested' do + get api("/groups/#{group1.id}/descendant_groups", admin), params: { statistics: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.first).to include('statistics') + end + end + end + describe "POST /groups" do it_behaves_like 'group avatar upload' do def make_upload_request diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 9c0ea14e3e3..91d10791541 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -9,7 +9,7 @@ RSpec.describe API::Helpers do include described_class include TermsHelper - let(:user) { create(:user) } + let_it_be(:user, reload: true) { create(:user) } let(:admin) { create(:admin) } let(:key) { create(:key, user: user) } @@ -243,6 +243,67 @@ RSpec.describe API::Helpers do end end end + + describe "when authenticating using a job token" do + let_it_be(:job, reload: true) do + create(:ci_build, user: user, status: :running) + end + + let(:route_authentication_setting) { {} } + + before do + allow_any_instance_of(self.class).to receive(:route_authentication_setting) + .and_return(route_authentication_setting) + end + + context 'when route is allowed to be authenticated' do + let(:route_authentication_setting) { { job_token_allowed: true } } + + it "returns a 401 response for an invalid token" do + env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = 'invalid token' + + expect { current_user }.to raise_error /401/ + end + + it "returns a 401 response for a job that's not running" do + job.update!(status: :success) + env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = job.token + + expect { current_user }.to raise_error /401/ + end + + it "returns a 403 response for a user without access" do + env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = job.token + allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false) + + expect { current_user }.to raise_error /403/ + end + + it 'returns a 403 response for a user who is blocked' do + user.block! + env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = job.token + + expect { current_user }.to raise_error /403/ + end + + it "sets current_user" do + env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = job.token + + expect(current_user).to eq(user) + end + end + + context 'when route is not allowed to be authenticated' do + let(:route_authentication_setting) { { job_token_allowed: false } } + + it "sets current_user to nil" do + env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = job.token + allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(true) + + expect(current_user).to be_nil + end + end + end end describe '.handle_api_exception' do diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 4a0a7c81781..ab5f09305ce 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe API::Internal::Base do + include APIInternalBaseHelpers + let_it_be(:user, reload: true) { create(:user) } let_it_be(:project, reload: true) { create(:project, :repository, :wiki_repo) } let_it_be(:personal_snippet) { create(:personal_snippet, :repository, author: user) } @@ -48,43 +50,63 @@ RSpec.describe API::Internal::Base do end end - describe 'GET /internal/two_factor_recovery_codes' do - it 'returns an error message when the key does not exist' do - post api('/internal/two_factor_recovery_codes'), - params: { - secret_token: secret_token, - key_id: non_existing_record_id - } + shared_examples 'actor key validations' do + context 'key id is not provided' do + let(:key_id) { nil } - expect(json_response['success']).to be_falsey - expect(json_response['message']).to eq('Could not find the given key') + it 'returns an error message' do + subject + + expect(json_response['success']).to be_falsey + expect(json_response['message']).to eq('Could not find a user without a key') + end end - it 'returns an error message when the key is a deploy key' do - deploy_key = create(:deploy_key) + context 'key does not exist' do + let(:key_id) { non_existing_record_id } - post api('/internal/two_factor_recovery_codes'), - params: { - secret_token: secret_token, - key_id: deploy_key.id - } + it 'returns an error message' do + subject - expect(json_response['success']).to be_falsey - expect(json_response['message']).to eq('Deploy keys cannot be used to retrieve recovery codes') + expect(json_response['success']).to be_falsey + expect(json_response['message']).to eq('Could not find the given key') + end + end + + context 'key without user' do + let(:key_id) { create(:key, user: nil).id } + + it 'returns an error message' do + subject + + expect(json_response['success']).to be_falsey + expect(json_response['message']).to eq('Could not find a user for the given key') + end end + end - it 'returns an error message when the user does not exist' do - key_without_user = create(:key, user: nil) + describe 'GET /internal/two_factor_recovery_codes' do + let(:key_id) { key.id } + subject do post api('/internal/two_factor_recovery_codes'), params: { secret_token: secret_token, - key_id: key_without_user.id + key_id: key_id } + end - expect(json_response['success']).to be_falsey - expect(json_response['message']).to eq('Could not find a user for the given key') - expect(json_response['recovery_codes']).to be_nil + it_behaves_like 'actor key validations' + + context 'key is a deploy key' do + let(:key_id) { create(:deploy_key).id } + + it 'returns an error message' do + subject + + expect(json_response['success']).to be_falsey + expect(json_response['message']).to eq('Deploy keys cannot be used to retrieve recovery codes') + end end context 'when two-factor is enabled' do @@ -93,11 +115,7 @@ RSpec.describe API::Internal::Base do allow_any_instance_of(User) .to receive(:generate_otp_backup_codes!).and_return(%w(119135e5a3ebce8e 34bd7b74adbc8861)) - post api('/internal/two_factor_recovery_codes'), - params: { - secret_token: secret_token, - key_id: key.id - } + subject expect(json_response['success']).to be_truthy expect(json_response['recovery_codes']).to match_array(%w(119135e5a3ebce8e 34bd7b74adbc8861)) @@ -108,11 +126,7 @@ RSpec.describe API::Internal::Base do it 'returns an error message' do allow_any_instance_of(User).to receive(:two_factor_enabled?).and_return(false) - post api('/internal/two_factor_recovery_codes'), - params: { - secret_token: secret_token, - key_id: key.id - } + subject expect(json_response['success']).to be_falsey expect(json_response['recovery_codes']).to be_nil @@ -121,42 +135,27 @@ RSpec.describe API::Internal::Base do end describe 'POST /internal/personal_access_token' do - it 'returns an error message when the key does not exist' do - post api('/internal/personal_access_token'), - params: { - secret_token: secret_token, - key_id: non_existing_record_id - } - - expect(json_response['success']).to be_falsey - expect(json_response['message']).to eq('Could not find the given key') - end - - it 'returns an error message when the key is a deploy key' do - deploy_key = create(:deploy_key) + let(:key_id) { key.id } + subject do post api('/internal/personal_access_token'), params: { secret_token: secret_token, - key_id: deploy_key.id + key_id: key_id } - - expect(json_response['success']).to be_falsey - expect(json_response['message']).to eq('Deploy keys cannot be used to create personal access tokens') end - it 'returns an error message when the user does not exist' do - key_without_user = create(:key, user: nil) + it_behaves_like 'actor key validations' - post api('/internal/personal_access_token'), - params: { - secret_token: secret_token, - key_id: key_without_user.id - } + context 'key is a deploy key' do + let(:key_id) { create(:deploy_key).id } - expect(json_response['success']).to be_falsey - expect(json_response['message']).to eq('Could not find a user for the given key') - expect(json_response['token']).to be_nil + it 'returns an error message' do + subject + + expect(json_response['success']).to be_falsey + expect(json_response['message']).to eq('Deploy keys cannot be used to create personal access tokens') + end end it 'returns an error message when given an non existent user' do @@ -459,7 +458,7 @@ RSpec.describe API::Internal::Base do end it_behaves_like 'sets hook env' do - let(:gl_repository) { Gitlab::GlRepository::WIKI.identifier_for_container(project) } + let(:gl_repository) { Gitlab::GlRepository::WIKI.identifier_for_container(project.wiki) } end end @@ -1207,86 +1206,157 @@ RSpec.describe API::Internal::Base do end end - def gl_repository_for(container) - case container - when ProjectWiki - Gitlab::GlRepository::WIKI.identifier_for_container(container.project) - when Project - Gitlab::GlRepository::PROJECT.identifier_for_container(container) - when Snippet - Gitlab::GlRepository::SNIPPET.identifier_for_container(container) - else - nil + describe 'POST /internal/two_factor_config' do + let(:key_id) { key.id } + + before do + stub_feature_flags(two_factor_for_cli: true) end - end - def full_path_for(container) - case container - when PersonalSnippet - "snippets/#{container.id}" - when ProjectSnippet - "#{container.project.full_path}/snippets/#{container.id}" - else - container.full_path + subject do + post api('/internal/two_factor_config'), + params: { + secret_token: secret_token, + key_id: key_id + } end - end - def pull(key, container, protocol = 'ssh') - post( - api("/internal/allowed"), - params: { - key_id: key.id, - project: full_path_for(container), - gl_repository: gl_repository_for(container), - action: 'git-upload-pack', - secret_token: secret_token, - protocol: protocol - } - ) - end + it_behaves_like 'actor key validations' - def push(key, container, protocol = 'ssh', env: nil, changes: nil) - push_with_path(key, - full_path: full_path_for(container), - gl_repository: gl_repository_for(container), - protocol: protocol, - env: env, - changes: changes) - end + context 'when the key is a deploy key' do + let(:key) { create(:deploy_key) } - def push_with_path(key, full_path:, gl_repository: nil, protocol: 'ssh', env: nil, changes: nil) - changes ||= 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master' + it 'does not required two factor' do + subject - params = { - changes: changes, - key_id: key.id, - project: full_path, - action: 'git-receive-pack', - secret_token: secret_token, - protocol: protocol, - env: env - } - params[:gl_repository] = gl_repository if gl_repository + expect(json_response['success']).to be_truthy + expect(json_response['two_factor_required']).to be_falsey + end + end - post( - api("/internal/allowed"), - params: params - ) + context 'when two-factor is enabled' do + it 'returns user two factor config' do + allow_any_instance_of(User).to receive(:two_factor_enabled?).and_return(true) + + subject + + expect(json_response['success']).to be_truthy + expect(json_response['two_factor_required']).to be_truthy + end + end + + context 'when two-factor is not enabled' do + it 'returns an error message' do + allow_any_instance_of(User).to receive(:two_factor_enabled?).and_return(false) + + subject + + expect(json_response['success']).to be_truthy + expect(json_response['two_factor_required']).to be_falsey + end + end + + context 'two_factor_for_cli feature is disabled' do + before do + stub_feature_flags(two_factor_for_cli: false) + end + + context 'when two-factor is enabled for the user' do + it 'returns user two factor config' do + allow_any_instance_of(User).to receive(:two_factor_enabled?).and_return(true) + + subject + + expect(json_response['success']).to be_falsey + end + end + end end - def archive(key, container) - post( - api("/internal/allowed"), - params: { - ref: 'master', - key_id: key.id, - project: full_path_for(container), - gl_repository: gl_repository_for(container), - action: 'git-upload-archive', - secret_token: secret_token, - protocol: 'ssh' - } - ) + describe 'POST /internal/two_factor_otp_check' do + let(:key_id) { key.id } + let(:otp) { '123456'} + + before do + stub_feature_flags(two_factor_for_cli: true) + end + + subject do + post api('/internal/two_factor_otp_check'), + params: { + secret_token: secret_token, + key_id: key_id, + otp_attempt: otp + } + end + + it_behaves_like 'actor key validations' + + context 'when the key is a deploy key' do + let(:key_id) { create(:deploy_key).id } + + it 'returns an error message' do + subject + + expect(json_response['success']).to be_falsey + expect(json_response['message']).to eq('Deploy keys cannot be used for Two Factor') + end + end + + context 'when the two factor is enabled' do + before do + allow_any_instance_of(User).to receive(:two_factor_enabled?).and_return(true) + end + + context 'when the OTP is valid' do + it 'returns success' do + allow_any_instance_of(Users::ValidateOtpService).to receive(:execute).with(otp).and_return(status: :success) + + subject + + expect(json_response['success']).to be_truthy + end + end + + context 'when the OTP is invalid' do + it 'is not success' do + allow_any_instance_of(Users::ValidateOtpService).to receive(:execute).with(otp).and_return(status: :error) + + subject + + expect(json_response['success']).to be_falsey + end + end + end + + context 'when the two factor is disabled' do + before do + allow_any_instance_of(User).to receive(:two_factor_enabled?).and_return(false) + end + + it 'returns an error message' do + subject + + expect(json_response['success']).to be_falsey + expect(json_response['message']).to eq 'Two-factor authentication is not enabled for this user' + end + end + + context 'two_factor_for_cli feature is disabled' do + before do + stub_feature_flags(two_factor_for_cli: false) + end + + context 'when two-factor is enabled for the user' do + it 'returns user two factor config' do + allow_any_instance_of(User).to receive(:two_factor_enabled?).and_return(true) + + subject + + expect(json_response['success']).to be_falsey + end + end + end end def lfs_auth_project(project) diff --git a/spec/requests/api/internal/lfs_spec.rb b/spec/requests/api/internal/lfs_spec.rb new file mode 100644 index 00000000000..4739ec62992 --- /dev/null +++ b/spec/requests/api/internal/lfs_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Internal::Lfs do + include APIInternalBaseHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:lfs_object) { create(:lfs_object, :with_file) } + let_it_be(:lfs_objects_project) { create(:lfs_objects_project, project: project, lfs_object: lfs_object) } + let_it_be(:gl_repository) { "project-#{project.id}" } + let_it_be(:filename) { lfs_object.file.path } + + let(:secret_token) { Gitlab::Shell.secret_token } + + describe 'GET /internal/lfs' do + let(:valid_params) do + { oid: lfs_object.oid, gl_repository: gl_repository, secret_token: secret_token } + end + + context 'with invalid auth' do + let(:invalid_params) { valid_params.merge!(secret_token: 'invalid_tokne') } + + it 'returns 401' do + get api("/internal/lfs"), params: invalid_params + end + end + + context 'with valid auth' do + context 'LFS in local storage' do + it 'sends the file' do + get api("/internal/lfs"), params: valid_params + + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers['Content-Type']).to eq('application/octet-stream') + expect(response.headers['Content-Length'].to_i).to eq(File.stat(filename).size) + expect(response.body).to eq(File.open(filename, 'rb', &:read)) + end + + # https://www.rubydoc.info/github/rack/rack/master/Rack/Sendfile + it 'delegates sending to Web server' do + get api("/internal/lfs"), params: valid_params, env: { 'HTTP_X_SENDFILE_TYPE' => 'X-Sendfile' } + + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers['Content-Type']).to eq('application/octet-stream') + expect(response.headers['Content-Length'].to_i).to eq(0) + expect(response.headers['X-Sendfile']).to be_present + expect(response.body).to eq("") + end + + it 'retuns 404 for unknown file' do + params = valid_params.merge(oid: SecureRandom.hex) + + get api("/internal/lfs"), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns 404 if LFS object does not belong to project' do + other_lfs = create(:lfs_object, :with_file) + params = valid_params.merge(oid: other_lfs.oid) + + get api("/internal/lfs"), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'LFS in object storage' do + let!(:lfs_object2) { create(:lfs_object, :with_file) } + let!(:lfs_objects_project2) { create(:lfs_objects_project, project: project, lfs_object: lfs_object2) } + let(:valid_params) do + { oid: lfs_object2.oid, gl_repository: gl_repository, secret_token: secret_token } + end + + before do + stub_lfs_object_storage(enabled: true) + lfs_object2.file.migrate!(LfsObjectUploader::Store::REMOTE) + end + + it 'notifies Workhorse to send the file' do + get api("/internal/lfs"), params: valid_params + + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("send-url:") + expect(response.headers['Content-Type']).to eq('application/octet-stream') + expect(response.headers['Content-Length'].to_i).to eq(0) + expect(response.body).to eq("") + end + end + end + end +end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 2d57146fbc9..c1498e03f76 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -465,12 +465,14 @@ RSpec.describe API::Jobs do end context 'find proper job' do + let(:job_with_artifacts) { job } + shared_examples 'a valid file' do context 'when artifacts are stored locally', :sidekiq_might_not_need_inline do let(:download_headers) do { 'Content-Transfer-Encoding' => 'binary', 'Content-Disposition' => - %Q(attachment; filename="#{job.artifacts_file.filename}"; filename*=UTF-8''#{job.artifacts_file.filename}) } + %Q(attachment; filename="#{job_with_artifacts.artifacts_file.filename}"; filename*=UTF-8''#{job.artifacts_file.filename}) } end it { expect(response).to have_gitlab_http_status(:ok) } @@ -518,6 +520,18 @@ RSpec.describe API::Jobs do it_behaves_like 'a valid file' end + + context 'with job name in a child pipeline' do + let(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) } + let!(:child_job) { create(:ci_build, :artifacts, :success, name: 'rspec', pipeline: child_pipeline) } + let(:job_with_artifacts) { child_job } + + before do + get_for_ref('master', child_job.name) + end + + it_behaves_like 'a valid file' + end end end diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb index 4c60c8bd2a3..9890cdc20c0 100644 --- a/spec/requests/api/lint_spec.rb +++ b/spec/requests/api/lint_spec.rb @@ -17,23 +17,52 @@ RSpec.describe API::Lint do expect(json_response['status']).to eq('valid') expect(json_response['errors']).to eq([]) end + + it 'outputs expanded yaml content' do + post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to have_key('merged_yaml') + end end context 'with an invalid .gitlab_ci.yml' do - it 'responds with errors about invalid syntax' do - post api('/ci/lint'), params: { content: 'invalid content' } + context 'with invalid syntax' do + let(:yaml_content) { 'invalid content' } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['status']).to eq('invalid') - expect(json_response['errors']).to eq(['Invalid configuration format']) + it 'responds with errors about invalid syntax' do + post api('/ci/lint'), params: { content: yaml_content } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['status']).to eq('invalid') + expect(json_response['errors']).to eq(['Invalid configuration format']) + end + + it 'outputs expanded yaml content' do + post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to have_key('merged_yaml') + end end - it "responds with errors about invalid configuration" do - post api('/ci/lint'), params: { content: '{ image: "ruby:2.7", services: ["postgres"] }' } + context 'with invalid configuration' do + let(:yaml_content) { '{ image: "ruby:2.7", services: ["postgres"] }' } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['status']).to eq('invalid') - expect(json_response['errors']).to eq(['jobs config should contain at least one visible job']) + it 'responds with errors about invalid configuration' do + post api('/ci/lint'), params: { content: yaml_content } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['status']).to eq('invalid') + expect(json_response['errors']).to eq(['jobs config should contain at least one visible job']) + end + + it 'outputs expanded yaml content' do + post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to have_key('merged_yaml') + end end end @@ -46,4 +75,204 @@ RSpec.describe API::Lint do end end end + + describe 'GET /projects/:id/ci/lint' do + subject(:ci_lint) { get api("/projects/#{project.id}/ci/lint", api_user), params: { dry_run: dry_run } } + + let(:project) { create(:project, :repository) } + let(:dry_run) { nil } + + RSpec.shared_examples 'valid config' do + it 'passes validation' do + ci_lint + + included_config = YAML.safe_load(included_content, [Symbol]) + root_config = YAML.safe_load(yaml_content, [Symbol]) + expected_yaml = included_config.merge(root_config).except(:include).to_yaml + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Hash + expect(json_response['merged_yaml']).to eq(expected_yaml) + expect(json_response['valid']).to eq(true) + expect(json_response['errors']).to eq([]) + end + end + + RSpec.shared_examples 'invalid config' do + it 'responds with errors about invalid configuration' do + ci_lint + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['merged_yaml']).to eq(yaml_content) + expect(json_response['valid']).to eq(false) + expect(json_response['errors']).to eq(['jobs config should contain at least one visible job']) + end + end + + context 'when unauthenticated' do + let_it_be(:api_user) { nil } + + it 'returns authentication error' do + ci_lint + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when authenticated as non-member' do + let_it_be(:api_user) { create(:user) } + + let(:yaml_content) do + { include: { local: 'another-gitlab-ci.yml' }, test: { stage: 'test', script: 'echo 1' } }.to_yaml + end + + context 'when project is private' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + stub_ci_pipeline_yaml_file(yaml_content) + end + + it 'returns authentication error' do + ci_lint + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when project is public' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + context 'when running as dry run' do + let(:dry_run) { true } + + before do + stub_ci_pipeline_yaml_file(yaml_content) + end + + it 'returns pipeline creation error' do + ci_lint + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['merged_yaml']).to eq(nil) + expect(json_response['valid']).to eq(false) + expect(json_response['errors']).to eq(['Insufficient permissions to create a new pipeline']) + end + end + + context 'when running static validation' do + let(:dry_run) { false } + + let(:included_content) do + { another_test: { stage: 'test', script: 'echo 1' } }.to_yaml + end + + before do + project.repository.create_file( + project.creator, + '.gitlab-ci.yml', + yaml_content, + message: 'Automatically created .gitlab-ci.yml', + branch_name: 'master' + ) + + project.repository.create_file( + project.creator, + 'another-gitlab-ci.yml', + included_content, + message: 'Automatically created another-gitlab-ci.yml', + branch_name: 'master' + ) + end + + it_behaves_like 'valid config' + end + end + end + + context 'when authenticated as project guest' do + let_it_be(:api_user) { create(:user) } + + before do + project.add_guest(api_user) + end + + it 'returns authentication error' do + ci_lint + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when authenticated as project developer' do + let_it_be(:api_user) { create(:user) } + + before do + project.add_developer(api_user) + end + + context 'with valid .gitlab-ci.yml content' do + let(:yaml_content) do + { include: { local: 'another-gitlab-ci.yml' }, test: { stage: 'test', script: 'echo 1' } }.to_yaml + end + + let(:included_content) do + { another_test: { stage: 'test', script: 'echo 1' } }.to_yaml + end + + before do + project.repository.create_file( + project.creator, + '.gitlab-ci.yml', + yaml_content, + message: 'Automatically created .gitlab-ci.yml', + branch_name: 'master' + ) + + project.repository.create_file( + project.creator, + 'another-gitlab-ci.yml', + included_content, + message: 'Automatically created another-gitlab-ci.yml', + branch_name: 'master' + ) + end + + context 'when running as dry run' do + let(:dry_run) { true } + + it_behaves_like 'valid config' + end + + context 'when running static validation' do + let(:dry_run) { false } + + it_behaves_like 'valid config' + end + end + + context 'with invalid .gitlab-ci.yml content' do + let(:yaml_content) do + { image: 'ruby:2.7', services: ['postgres'] }.to_yaml + end + + before do + stub_ci_pipeline_yaml_file(yaml_content) + end + + context 'when running as dry run' do + let(:dry_run) { true } + + it_behaves_like 'invalid config' + end + + context 'when running static validation' do + let(:dry_run) { false } + + it_behaves_like 'invalid config' + end + end + end + end end diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index 0a23aed109b..37748fe5ea7 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -15,10 +15,13 @@ RSpec.describe API::MavenPackages do let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } + let_it_be(: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(: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) } + let(:group_deploy_token_headers) { { Gitlab::Auth::AuthFinders::DEPLOY_TOKEN_HEADER => deploy_token_for_group.token } } let(:headers_with_deploy_token) do headers.merge( @@ -36,7 +39,7 @@ RSpec.describe API::MavenPackages do context 'with jar file' do let_it_be(:package_file) { jar_file } - it_behaves_like 'a gitlab tracking event', described_class.name, 'pull_package' + it_behaves_like 'a package tracking event', described_class.name, 'pull_package' end end @@ -342,6 +345,17 @@ RSpec.describe API::MavenPackages do it_behaves_like 'downloads with a job token' it_behaves_like 'downloads with a deploy token' + + context 'with group deploy token' do + subject { download_file_with_token(package_file.file_name, {}, group_deploy_token_headers) } + + it 'returns the file' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.media_type).to eq('application/octet-stream') + end + end end def download_file(file_name, params = {}, request_headers = headers) @@ -548,7 +562,7 @@ RSpec.describe API::MavenPackages do allow(uploaded_file).to receive(:size).and_return(project.actual_limits.maven_max_file_size + 1) end - upload_file_with_token(params) + upload_file_with_token(params: params) expect(response).to have_gitlab_http_status(:bad_request) end @@ -563,19 +577,19 @@ RSpec.describe API::MavenPackages do context 'without workhorse header' do let(:workhorse_header) { {} } - subject { upload_file_with_token(params) } + subject { upload_file_with_token(params: params) } it_behaves_like 'package workhorse uploads' end context 'event tracking' do - subject { upload_file_with_token(params) } + subject { upload_file_with_token(params: params) } - it_behaves_like 'a gitlab tracking event', described_class.name, 'push_package' + it_behaves_like 'a package tracking event', described_class.name, 'push_package' end it 'creates package and stores package file' do - expect { upload_file_with_token(params) }.to change { project.packages.count }.by(1) + expect { upload_file_with_token(params: params) }.to change { project.packages.count }.by(1) .and change { Packages::Maven::Metadatum.count }.by(1) .and change { Packages::PackageFile.count }.by(1) @@ -584,7 +598,7 @@ RSpec.describe API::MavenPackages do end it 'allows upload with running job token' do - upload_file(params.merge(job_token: job.token)) + upload_file(params: params.merge(job_token: job.token)) expect(response).to have_gitlab_http_status(:ok) expect(project.reload.packages.last.build_info.pipeline).to eq job.pipeline @@ -592,13 +606,13 @@ RSpec.describe API::MavenPackages do it 'rejects upload without running job token' do job.update!(status: :failed) - upload_file(params.merge(job_token: job.token)) + upload_file(params: params.merge(job_token: job.token)) expect(response).to have_gitlab_http_status(:unauthorized) end it 'allows upload with deploy token' do - upload_file(params, headers_with_deploy_token) + upload_file(params: params, request_headers: headers_with_deploy_token) expect(response).to have_gitlab_http_status(:ok) end @@ -612,7 +626,10 @@ RSpec.describe API::MavenPackages do # We force the id of the deploy token and the user to be the same unauthorized_deploy_token.update!(id: another_user.id) - upload_file(params, headers.merge(Gitlab::Auth::AuthFinders::DEPLOY_TOKEN_HEADER => unauthorized_deploy_token.token)) + upload_file( + params: params, + request_headers: headers.merge(Gitlab::Auth::AuthFinders::DEPLOY_TOKEN_HEADER => unauthorized_deploy_token.token) + ) expect(response).to have_gitlab_http_status(:forbidden) end @@ -621,16 +638,43 @@ RSpec.describe API::MavenPackages do let(:version) { '$%123' } it 'rejects request' do - expect { upload_file_with_token(params) }.not_to change { project.packages.count } + expect { upload_file_with_token(params: params) }.not_to change { project.packages.count } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']).to include('Validation failed') end end + + context 'for sha1 file' do + let(:dummy_package) { double(Packages::Package) } + + it 'checks the sha1' do + # The sha verification done by the maven api is between: + # - the sha256 set by workhorse helpers + # - the sha256 of the sha1 of the uploaded package file + # We're going to send `file_upload` for the sha1 and stub the sha1 of the package file so that + # both sha256 being the same + expect(::Packages::PackageFileFinder).to receive(:new).and_return(double(execute!: dummy_package)) + expect(dummy_package).to receive(:file_sha1).and_return(File.read(file_upload.path)) + + upload_file_with_token(params: params, file_extension: 'jar.sha1') + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'for md5 file' do + it 'returns an empty body' do + upload_file_with_token(params: params, file_extension: 'jar.md5') + + expect(response.body).to eq('') + expect(response).to have_gitlab_http_status(:ok) + end + end end - def upload_file(params = {}, request_headers = headers) - url = "/projects/#{project.id}/packages/maven/com/example/my-app/#{version}/my-app-1.0-20180724.124855-1.jar" + 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}" workhorse_finalize( api(url), method: :put, @@ -641,8 +685,8 @@ RSpec.describe API::MavenPackages do ) end - def upload_file_with_token(params = {}, request_headers = headers_with_token) - upload_file(params, request_headers) + def upload_file_with_token(params: {}, request_headers: headers_with_token, file_extension: 'jar') + upload_file(params: params, request_headers: request_headers, file_extension: file_extension) end end end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 55b2447fc68..047b9423906 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -196,6 +196,7 @@ RSpec.describe API::Members do # Member attributes expect(json_response['access_level']).to eq(Member::DEVELOPER) + expect(json_response['created_at'].to_time).to be_like_time(developer.created_at) end end end @@ -251,6 +252,36 @@ RSpec.describe API::Members do expect(json_response['id']).to eq(stranger.id) expect(json_response['access_level']).to eq(Member::DEVELOPER) end + + describe 'executes the Members::CreateService for multiple user_ids' do + it 'returns success when it successfully create all members' do + expect do + user_ids = [stranger.id, access_requester.id].join(',') + + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), + params: { user_id: user_ids, access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.count }.by(2) + expect(json_response['status']).to eq('success') + end + + it 'returns the error message if there was an error adding members to group' do + error_message = 'Unable to find User ID' + user_ids = [stranger.id, access_requester.id].join(',') + + allow_next_instance_of(::Members::CreateService) do |service| + expect(service).to receive(:execute).with(source).and_return({ status: :error, message: error_message }) + end + + expect do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), + params: { user_id: user_ids, access_level: Member::DEVELOPER } + end.not_to change { source.members.count } + expect(json_response['status']).to eq('error') + expect(json_response['message']).to eq(error_message) + end + end end context 'access levels' do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 2757c56e0fe..506607f4cc2 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -856,6 +856,55 @@ RSpec.describe API::MergeRequests do expect(json_response.first['id']).to eq merge_request_closed.id end + context 'when filtering by deployments' do + let_it_be(:mr) do + create(:merge_request, :merged, source_project: project, target_project: project) + end + + before do + env = create(:environment, project: project, name: 'staging') + deploy = create(:deployment, :success, environment: env, deployable: nil) + + deploy.link_merge_requests(MergeRequest.where(id: mr.id)) + end + + it 'supports getting merge requests deployed to an environment' do + get api(endpoint_path, user), params: { environment: 'staging' } + + expect(json_response.first['id']).to eq mr.id + end + + it 'does not return merge requests for an environment without deployments' do + get api(endpoint_path, user), params: { environment: 'bla' } + + expect_empty_array_response + end + + it 'supports getting merge requests deployed after a date' do + get api(endpoint_path, user), params: { deployed_after: '1990-01-01' } + + expect(json_response.first['id']).to eq mr.id + end + + it 'does not return merge requests not deployed after a given date' do + get api(endpoint_path, user), params: { deployed_after: '2100-01-01' } + + expect_empty_array_response + end + + it 'supports getting merge requests deployed before a date' do + get api(endpoint_path, user), params: { deployed_before: '2100-01-01' } + + expect(json_response.first['id']).to eq mr.id + end + + it 'does not return merge requests not deployed before a given date' do + get api(endpoint_path, user), params: { deployed_before: '1990-01-01' } + + expect_empty_array_response + end + end + context 'a project which enforces all discussions to be resolved' do let_it_be(:project) { create(:project, :repository, only_allow_merge_if_all_discussions_are_resolved: true) } @@ -1140,7 +1189,7 @@ RSpec.describe API::MergeRequests do context 'when a merge request has more than the changes limit' do it "returns a string indicating that more changes were made" do - stub_const('Commit::DIFF_HARD_LIMIT_FILES', 5) + allow(Commit).to receive(:diff_hard_limit_files).and_return(5) merge_request_overflow = create(:merge_request, :simple, author: user, diff --git a/spec/requests/api/npm_packages_spec.rb b/spec/requests/api/npm_packages_spec.rb index 108ea84b7e6..8a3ccd7c6e3 100644 --- a/spec/requests/api/npm_packages_spec.rb +++ b/spec/requests/api/npm_packages_spec.rb @@ -88,12 +88,16 @@ RSpec.describe API::NpmPackages do it_behaves_like 'returning the npm package info' context 'with unknown package' do + subject { get api("/packages/npm/unknown") } + it 'returns a redirect' do - get api("/packages/npm/unknown") + subject expect(response).to have_gitlab_http_status(:found) expect(response.headers['Location']).to eq('https://registry.npmjs.org/unknown') end + + it_behaves_like 'a gitlab tracking event', described_class.name, 'npm_request_forward' end end @@ -193,7 +197,7 @@ RSpec.describe API::NpmPackages do expect(response.media_type).to eq('application/octet-stream') end - it_behaves_like 'a gitlab tracking event', described_class.name, 'pull_package' + it_behaves_like 'a package tracking event', described_class.name, 'pull_package' end context 'private project' do @@ -301,7 +305,7 @@ RSpec.describe API::NpmPackages do context 'with access token' do subject { upload_package_with_token(package_name, params) } - it_behaves_like 'a gitlab tracking event', described_class.name, 'push_package' + it_behaves_like 'a package tracking event', described_class.name, 'push_package' it 'creates npm package with file' do expect { subject } diff --git a/spec/requests/api/project_clusters_spec.rb b/spec/requests/api/project_clusters_spec.rb index ff35e380476..7b37862af74 100644 --- a/spec/requests/api/project_clusters_spec.rb +++ b/spec/requests/api/project_clusters_spec.rb @@ -171,6 +171,7 @@ RSpec.describe API::ProjectClusters do name: 'test-cluster', domain: 'domain.example.com', managed: false, + namespace_per_environment: false, platform_kubernetes_attributes: platform_kubernetes_attributes, management_project_id: management_project_id } @@ -202,6 +203,7 @@ RSpec.describe API::ProjectClusters do expect(cluster_result.domain).to eq('domain.example.com') expect(cluster_result.managed).to be_falsy expect(cluster_result.management_project_id).to eq management_project_id + expect(cluster_result.namespace_per_environment).to eq(false) expect(platform_kubernetes.rbac?).to be_truthy expect(platform_kubernetes.api_url).to eq(api_url) expect(platform_kubernetes.namespace).to eq(namespace) @@ -235,6 +237,22 @@ RSpec.describe API::ProjectClusters do end end + context 'when namespace_per_environment is not set' do + let(:cluster_params) do + { + name: 'test-cluster', + domain: 'domain.example.com', + platform_kubernetes_attributes: platform_kubernetes_attributes + } + end + + it 'defaults to true' do + cluster_result = Clusters::Cluster.find(json_response['id']) + + expect(cluster_result).to be_namespace_per_environment + end + end + context 'current user does not have access to management_project_id' do let(:management_project_id) { create(:project).id } diff --git a/spec/requests/api/project_container_repositories_spec.rb b/spec/requests/api/project_container_repositories_spec.rb index 6cf0619cde4..34476b10576 100644 --- a/spec/requests/api/project_container_repositories_spec.rb +++ b/spec/requests/api/project_container_repositories_spec.rb @@ -31,7 +31,6 @@ RSpec.describe API::ProjectContainerRepositories do project.add_reporter(reporter) project.add_guest(guest) - stub_feature_flags(container_registry_api: true) stub_container_registry_config(enabled: true) root_repository @@ -45,7 +44,7 @@ RSpec.describe API::ProjectContainerRepositories do it_behaves_like 'rejected container repository access', :guest, :forbidden it_behaves_like 'rejected container repository access', :anonymous, :not_found - it_behaves_like 'a gitlab tracking event', described_class.name, 'list_repositories' + it_behaves_like 'a package tracking event', described_class.name, 'list_repositories' it_behaves_like 'returns repositories for allowed users', :reporter, 'project' do let(:object) { project } @@ -57,7 +56,7 @@ RSpec.describe API::ProjectContainerRepositories do it_behaves_like 'rejected container repository access', :developer, :forbidden it_behaves_like 'rejected container repository access', :anonymous, :not_found - it_behaves_like 'a gitlab tracking event', described_class.name, 'delete_repository' + it_behaves_like 'a package tracking event', described_class.name, 'delete_repository' context 'for maintainer' do let(:api_user) { maintainer } @@ -86,7 +85,7 @@ RSpec.describe API::ProjectContainerRepositories do stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA latest)) end - it_behaves_like 'a gitlab tracking event', described_class.name, 'list_tags' + it_behaves_like 'a package tracking event', described_class.name, 'list_tags' it 'returns a list of tags' do subject @@ -114,7 +113,7 @@ RSpec.describe API::ProjectContainerRepositories do it_behaves_like 'rejected container repository access', :developer, :forbidden it_behaves_like 'rejected container repository access', :anonymous, :not_found - it_behaves_like 'a gitlab tracking event', described_class.name, 'delete_tag_bulk' + it_behaves_like 'a package tracking event', described_class.name, 'delete_tag_bulk' end context 'for maintainer' do diff --git a/spec/requests/api/project_packages_spec.rb b/spec/requests/api/project_packages_spec.rb index 2f0d0fc87ec..4c8599d1a20 100644 --- a/spec/requests/api/project_packages_spec.rb +++ b/spec/requests/api/project_packages_spec.rb @@ -23,6 +23,19 @@ RSpec.describe API::ProjectPackages do it_behaves_like 'returns packages', :project, :no_type end + context 'with conan package' do + let!(:conan_package) { create(:conan_package, project: project) } + + it 'uses the conan recipe as the package name' do + subject + + response_conan_package = json_response.find { |package| package['id'] == conan_package.id } + + expect(response_conan_package['name']).to eq(conan_package.conan_recipe) + expect(response_conan_package['conan_package_name']).to eq(conan_package.name) + end + end + context 'project is private' do let(:project) { create(:project, :private) } diff --git a/spec/requests/api/project_repository_storage_moves_spec.rb b/spec/requests/api/project_repository_storage_moves_spec.rb index 4c9e058ef13..ecf4c75b52f 100644 --- a/spec/requests/api/project_repository_storage_moves_spec.rb +++ b/spec/requests/api/project_repository_storage_moves_spec.rb @@ -145,10 +145,17 @@ RSpec.describe API::ProjectRepositoryStorageMoves do context 'destination_storage_name is missing' do let(:destination_storage_name) { nil } - it 'returns a validation error' do + it 'schedules a project repository storage move' do create_project_repository_storage_move - expect(response).to have_gitlab_http_status(:bad_request) + 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 diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 08c88873078..6a9cf6e16e2 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -6,21 +6,16 @@ RSpec.describe API::ProjectSnippets do include SnippetHelpers let_it_be(:project) { create(:project, :public) } - let_it_be(:user) { create(:user) } - let_it_be(:admin) { create(:admin) } let_it_be(:project_no_snippets) { create(:project, :snippets_disabled) } - - before do - project_no_snippets.add_developer(admin) - project_no_snippets.add_developer(user) - end + let_it_be(:user) { create(:user, developer_projects: [project_no_snippets]) } + let_it_be(:admin) { create(:admin, developer_projects: [project_no_snippets]) } + let_it_be(:public_snippet, reload: true) { create(:project_snippet, :public, :repository, project: project) } describe "GET /projects/:project_id/snippets/:id/user_agent_detail" do - let(:snippet) { create(:project_snippet, :public, project: project) } - let!(:user_agent_detail) { create(:user_agent_detail, subject: snippet) } + let_it_be(:user_agent_detail) { create(:user_agent_detail, subject: public_snippet) } it 'exposes known attributes' do - get api("/projects/#{project.id}/snippets/#{snippet.id}/user_agent_detail", admin) + get api("/projects/#{project.id}/snippets/#{public_snippet.id}/user_agent_detail", admin) expect(response).to have_gitlab_http_status(:ok) expect(json_response['user_agent']).to eq(user_agent_detail.user_agent) @@ -31,29 +26,27 @@ RSpec.describe API::ProjectSnippets do it 'respects project scoping' do other_project = create(:project) - get api("/projects/#{other_project.id}/snippets/#{snippet.id}/user_agent_detail", admin) + get api("/projects/#{other_project.id}/snippets/#{public_snippet.id}/user_agent_detail", admin) expect(response).to have_gitlab_http_status(:not_found) end it "returns unauthorized for non-admin users" do - get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/user_agent_detail", user) + get api("/projects/#{public_snippet.project.id}/snippets/#{public_snippet.id}/user_agent_detail", user) expect(response).to have_gitlab_http_status(:forbidden) end context 'with snippets disabled' do it_behaves_like '403 response' do - let(:request) { get api("/projects/#{project_no_snippets.id}/snippets/123/user_agent_detail", admin) } + let(:request) { get api("/projects/#{project_no_snippets.id}/snippets/#{non_existing_record_id}/user_agent_detail", admin) } end end end describe 'GET /projects/:project_id/snippets/' do - let(:user) { create(:user) } - it 'returns all snippets available to team member' do project.add_developer(user) - public_snippet = create(:project_snippet, :public, project: project) + internal_snippet = create(:project_snippet, :internal, project: project) private_snippet = create(:project_snippet, :private, project: project) @@ -62,8 +55,7 @@ RSpec.describe API::ProjectSnippets do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(json_response.size).to eq(3) - expect(json_response.map { |snippet| snippet['id'] }).to include(public_snippet.id, internal_snippet.id, private_snippet.id) + expect(json_response.map { |snippet| snippet['id'] }).to contain_exactly(public_snippet.id, internal_snippet.id, private_snippet.id) expect(json_response.last).to have_key('web_url') end @@ -75,7 +67,7 @@ RSpec.describe API::ProjectSnippets do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(json_response.size).to eq(0) + expect(json_response.map { |snippet| snippet['id'] }).to contain_exactly(public_snippet.id) end context 'with snippets disabled' do @@ -86,8 +78,7 @@ RSpec.describe API::ProjectSnippets do end describe 'GET /projects/:project_id/snippets/:id' do - let_it_be(:user) { create(:user) } - let_it_be(:snippet) { create(:project_snippet, :public, :repository, project: project) } + let(:snippet) { public_snippet } it 'returns snippet json' do get api("/projects/#{project.id}/snippets/#{snippet.id}", user) @@ -113,12 +104,12 @@ RSpec.describe API::ProjectSnippets do context 'with snippets disabled' do it_behaves_like '403 response' do - let(:request) { get api("/projects/#{project_no_snippets.id}/snippets/123", user) } + let(:request) { get api("/projects/#{project_no_snippets.id}/snippets/#{non_existing_record_id}", user) } end end - it_behaves_like 'snippet_multiple_files feature disabled' do - subject { get api("/projects/#{project.id}/snippets/#{snippet.id}", user) } + it_behaves_like 'project snippet access levels' do + let(:path) { "/projects/#{snippet.project.id}/snippets/#{snippet.id}" } end end @@ -133,37 +124,35 @@ RSpec.describe API::ProjectSnippets do let(:file_path) { 'file_1.rb' } let(:file_content) { 'puts "hello world"' } - let(:params) { base_params.merge(file_params) } let(:file_params) { { files: [{ file_path: file_path, content: file_content }] } } + let(:params) { base_params.merge(file_params) } + + subject { post api("/projects/#{project.id}/snippets/", actor), params: params } shared_examples 'project snippet repository actions' do let(:snippet) { ProjectSnippet.find(json_response['id']) } - it 'creates repository' do - subject - - expect(snippet.repository.exists?).to be_truthy - end - it 'commit the files to the repository' do subject - blob = snippet.repository.blob_at('master', file_path) + aggregate_failures do + expect(snippet.repository.exists?).to be_truthy + + blob = snippet.repository.blob_at('master', file_path) - expect(blob.data).to eq file_content + expect(blob.data).to eq file_content + end end end context 'with an external user' do - let(:user) { create(:user, :external) } + let(:actor) { create(:user, :external) } context 'that belongs to the project' do - before do - project.add_developer(user) - end - it 'creates a new snippet' do - post api("/projects/#{project.id}/snippets/", user), params: params + project.add_developer(actor) + + subject expect(response).to have_gitlab_http_status(:created) end @@ -171,7 +160,7 @@ RSpec.describe API::ProjectSnippets do context 'that does not belong to the project' do it 'does not create a new snippet' do - post api("/projects/#{project.id}/snippets/", user), params: params + subject expect(response).to have_gitlab_http_status(:forbidden) end @@ -179,16 +168,17 @@ RSpec.describe API::ProjectSnippets do end context 'with a regular user' do - let(:user) { create(:user) } + let(:actor) { user } - before do + before_all do project.add_developer(user) + end + + before do stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC, Gitlab::VisibilityLevel::PRIVATE]) params['visibility'] = 'internal' end - subject { post api("/projects/#{project.id}/snippets/", user), params: params } - it 'creates a new snippet' do subject @@ -205,7 +195,7 @@ RSpec.describe API::ProjectSnippets do end context 'with an admin' do - subject { post api("/projects/#{project.id}/snippets/", admin), params: params } + let(:actor) { admin } it 'creates a new snippet' do subject @@ -244,6 +234,8 @@ RSpec.describe API::ProjectSnippets do end context 'when save fails because the repository could not be created' do + let(:actor) { admin } + before do allow_next_instance_of(Snippets::CreateService) do |instance| allow(instance).to receive(:create_repository).and_raise(Snippets::CreateService::CreateRepositoryError) @@ -251,43 +243,44 @@ RSpec.describe API::ProjectSnippets do end it 'returns 400' do - post api("/projects/#{project.id}/snippets", admin), params: params + subject expect(response).to have_gitlab_http_status(:bad_request) end end context 'when the snippet is spam' do - def create_snippet(project, snippet_params = {}) - project.add_developer(user) - - post api("/projects/#{project.id}/snippets", user), params: params.merge(snippet_params) - end + let(:actor) { user } before do allow_next_instance_of(Spam::AkismetService) do |instance| allow(instance).to receive(:spam?).and_return(true) end + + project.add_developer(user) end context 'when the snippet is private' do it 'creates the snippet' do - expect { create_snippet(project, visibility: 'private') } - .to change { Snippet.count }.by(1) + params['visibility'] = 'private' + + expect { subject }.to change { Snippet.count }.by(1) end end context 'when the snippet is public' do - it 'rejects the snippet' do - expect { create_snippet(project, visibility: 'public') } - .not_to change { Snippet.count } + before do + params['visibility'] = 'public' + end + it 'rejects the snippet' do + expect { subject }.not_to change { Snippet.count } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']).to eq({ "error" => "Spam detected" }) end it 'creates a spam log' do - expect { create_snippet(project, visibility: 'public') } + expect { subject } .to log_spam(title: 'Test Title', user_id: user.id, noteable_type: 'ProjectSnippet') end end @@ -363,7 +356,7 @@ RSpec.describe API::ProjectSnippets do context 'with snippets disabled' do it_behaves_like '403 response' do - let(:request) { put api("/projects/#{project_no_snippets.id}/snippets/123", admin), params: { description: 'foo' } } + let(:request) { put api("/projects/#{project_no_snippets.id}/snippets/#{non_existing_record_id}", admin), params: { description: 'foo' } } end end @@ -373,7 +366,7 @@ RSpec.describe API::ProjectSnippets do end describe 'DELETE /projects/:project_id/snippets/:id/' do - let(:snippet) { create(:project_snippet, author: admin, project: project) } + let_it_be(:snippet, refind: true) { public_snippet } it 'deletes snippet' do delete api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin) @@ -394,13 +387,13 @@ RSpec.describe API::ProjectSnippets do context 'with snippets disabled' do it_behaves_like '403 response' do - let(:request) { delete api("/projects/#{project_no_snippets.id}/snippets/123", admin) } + let(:request) { delete api("/projects/#{project_no_snippets.id}/snippets/#{non_existing_record_id}", admin) } end end end describe 'GET /projects/:project_id/snippets/:id/raw' do - let_it_be(:snippet) { create(:project_snippet, :repository, author: admin, project: project) } + let_it_be(:snippet) { create(:project_snippet, :repository, :public, author: admin, project: project) } it 'returns raw text' do get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/raw", admin) @@ -416,9 +409,13 @@ RSpec.describe API::ProjectSnippets do expect(json_response['message']).to eq('404 Snippet Not Found') end + it_behaves_like 'project snippet access levels' do + let(:path) { "/projects/#{snippet.project.id}/snippets/#{snippet.id}/raw" } + end + context 'with snippets disabled' do it_behaves_like '403 response' do - let(:request) { get api("/projects/#{project_no_snippets.id}/snippets/123/raw", admin) } + let(:request) { get api("/projects/#{project_no_snippets.id}/snippets/#{non_existing_record_id}/raw", admin) } end end @@ -435,5 +432,9 @@ RSpec.describe API::ProjectSnippets do it_behaves_like 'raw snippet files' do let(:api_path) { "/projects/#{snippet.project.id}/snippets/#{snippet_id}/files/#{ref}/#{file_path}/raw" } end + + it_behaves_like 'project snippet access levels' do + let(:path) { "/projects/#{snippet.project.id}/snippets/#{snippet.id}/files/master/%2Egitattributes/raw" } + end end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 831b0d6e678..2abcb39a1c8 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1615,6 +1615,7 @@ RSpec.describe API::Projects do expect(json_response['allow_merge_on_skipped_pipeline']).to eq(project.allow_merge_on_skipped_pipeline) expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved) expect(json_response['ci_default_git_depth']).to eq(project.ci_default_git_depth) + expect(json_response['ci_forward_deployment_enabled']).to eq(project.ci_forward_deployment_enabled) expect(json_response['merge_method']).to eq(project.merge_method.to_s) expect(json_response['readme_url']).to eq(project.readme_url) expect(json_response).to have_key 'packages_enabled' @@ -2607,6 +2608,7 @@ RSpec.describe API::Projects do merge_requests_enabled: true, merge_method: 'ff', ci_default_git_depth: 20, + ci_forward_deployment_enabled: false, description: 'new description' } put api("/projects/#{project3.id}", user4), params: project_param diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index e72ac002f6b..72a470dca4b 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -23,24 +23,24 @@ RSpec.describe API::PypiPackages do using RSpec::Parameterized::TableSyntax where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do - 'PUBLIC' | :developer | true | true | 'PyPi package versions' | :success - 'PUBLIC' | :guest | true | true | 'PyPi package versions' | :success - 'PUBLIC' | :developer | true | false | 'PyPi package versions' | :success - 'PUBLIC' | :guest | true | false | 'PyPi package versions' | :success - 'PUBLIC' | :developer | false | true | 'PyPi package versions' | :success - 'PUBLIC' | :guest | false | true | 'PyPi package versions' | :success - 'PUBLIC' | :developer | false | false | 'PyPi package versions' | :success - 'PUBLIC' | :guest | false | false | 'PyPi package versions' | :success - 'PUBLIC' | :anonymous | false | true | 'PyPi package versions' | :success - 'PRIVATE' | :developer | true | true | 'PyPi package versions' | :success - 'PRIVATE' | :guest | true | true | 'process PyPi api request' | :forbidden - 'PRIVATE' | :developer | true | false | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :guest | true | false | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :developer | false | true | 'process PyPi api request' | :not_found - 'PRIVATE' | :guest | false | true | 'process PyPi api request' | :not_found - 'PRIVATE' | :developer | false | false | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :guest | false | false | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :anonymous | false | true | 'process PyPi api request' | :unauthorized + 'PUBLIC' | :developer | true | true | 'PyPI package versions' | :success + 'PUBLIC' | :guest | true | true | 'PyPI package versions' | :success + 'PUBLIC' | :developer | true | false | 'PyPI package versions' | :success + 'PUBLIC' | :guest | true | false | 'PyPI package versions' | :success + 'PUBLIC' | :developer | false | true | 'PyPI package versions' | :success + 'PUBLIC' | :guest | false | true | 'PyPI package versions' | :success + 'PUBLIC' | :developer | false | false | 'PyPI package versions' | :success + 'PUBLIC' | :guest | false | false | 'PyPI package versions' | :success + 'PUBLIC' | :anonymous | false | true | 'PyPI package versions' | :success + 'PRIVATE' | :developer | true | true | 'PyPI package versions' | :success + 'PRIVATE' | :guest | true | true | 'process PyPI api request' | :forbidden + 'PRIVATE' | :developer | true | false | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :guest | true | false | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :developer | false | true | 'process PyPI api request' | :not_found + 'PRIVATE' | :guest | false | true | 'process PyPI api request' | :not_found + 'PRIVATE' | :developer | false | false | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :guest | false | false | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :anonymous | false | true | 'process PyPI api request' | :unauthorized end with_them do @@ -57,6 +57,16 @@ RSpec.describe API::PypiPackages do end end + context 'with a normalized package name' do + let_it_be(:package) { create(:pypi_package, project: project, name: 'my.package') } + let(:url) { "/projects/#{project.id}/packages/pypi/simple/my-package" } + let(:headers) { basic_auth_header(user.username, personal_access_token.token) } + + subject { get api(url), headers: headers } + + it_behaves_like 'PyPI package versions', :developer, :success + end + it_behaves_like 'deploy token for package GET requests' it_behaves_like 'job token for package GET requests' @@ -76,24 +86,24 @@ RSpec.describe API::PypiPackages 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 PyPi api request' | :success - 'PUBLIC' | :guest | true | true | 'process PyPi api request' | :forbidden - 'PUBLIC' | :developer | true | false | 'process PyPi api request' | :unauthorized - 'PUBLIC' | :guest | true | false | 'process PyPi api request' | :unauthorized - 'PUBLIC' | :developer | false | true | 'process PyPi api request' | :forbidden - 'PUBLIC' | :guest | false | true | 'process PyPi api request' | :forbidden - 'PUBLIC' | :developer | false | false | 'process PyPi api request' | :unauthorized - 'PUBLIC' | :guest | false | false | 'process PyPi api request' | :unauthorized - 'PUBLIC' | :anonymous | false | true | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :developer | true | true | 'process PyPi api request' | :success - 'PRIVATE' | :guest | true | true | 'process PyPi api request' | :forbidden - 'PRIVATE' | :developer | true | false | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :guest | true | false | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :developer | false | true | 'process PyPi api request' | :not_found - 'PRIVATE' | :guest | false | true | 'process PyPi api request' | :not_found - 'PRIVATE' | :developer | false | false | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :guest | false | false | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :anonymous | false | true | 'process PyPi api request' | :unauthorized + 'PUBLIC' | :developer | true | true | 'process PyPI api request' | :success + 'PUBLIC' | :guest | true | true | 'process PyPI api request' | :forbidden + 'PUBLIC' | :developer | true | false | 'process PyPI api request' | :unauthorized + 'PUBLIC' | :guest | true | false | 'process PyPI api request' | :unauthorized + 'PUBLIC' | :developer | false | true | 'process PyPI api request' | :forbidden + 'PUBLIC' | :guest | false | true | 'process PyPI api request' | :forbidden + 'PUBLIC' | :developer | false | false | 'process PyPI api request' | :unauthorized + 'PUBLIC' | :guest | false | false | 'process PyPI api request' | :unauthorized + 'PUBLIC' | :anonymous | false | true | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :developer | true | true | 'process PyPI api request' | :success + 'PRIVATE' | :guest | true | true | 'process PyPI api request' | :forbidden + 'PRIVATE' | :developer | true | false | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :guest | true | false | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :developer | false | true | 'process PyPI api request' | :not_found + 'PRIVATE' | :guest | false | true | 'process PyPI api request' | :not_found + 'PRIVATE' | :developer | false | false | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :guest | false | false | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :anonymous | false | true | 'process PyPI api request' | :unauthorized end with_them do @@ -142,24 +152,24 @@ RSpec.describe API::PypiPackages do using RSpec::Parameterized::TableSyntax where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do - 'PUBLIC' | :developer | true | true | 'PyPi package creation' | :created - 'PUBLIC' | :guest | true | true | 'process PyPi api request' | :forbidden - 'PUBLIC' | :developer | true | false | 'process PyPi api request' | :unauthorized - 'PUBLIC' | :guest | true | false | 'process PyPi api request' | :unauthorized - 'PUBLIC' | :developer | false | true | 'process PyPi api request' | :forbidden - 'PUBLIC' | :guest | false | true | 'process PyPi api request' | :forbidden - 'PUBLIC' | :developer | false | false | 'process PyPi api request' | :unauthorized - 'PUBLIC' | :guest | false | false | 'process PyPi api request' | :unauthorized - 'PUBLIC' | :anonymous | false | true | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :developer | true | true | 'process PyPi api request' | :created - 'PRIVATE' | :guest | true | true | 'process PyPi api request' | :forbidden - 'PRIVATE' | :developer | true | false | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :guest | true | false | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :developer | false | true | 'process PyPi api request' | :not_found - 'PRIVATE' | :guest | false | true | 'process PyPi api request' | :not_found - 'PRIVATE' | :developer | false | false | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :guest | false | false | 'process PyPi api request' | :unauthorized - 'PRIVATE' | :anonymous | false | true | 'process PyPi api request' | :unauthorized + 'PUBLIC' | :developer | true | true | 'PyPI package creation' | :created + 'PUBLIC' | :guest | true | true | 'process PyPI api request' | :forbidden + 'PUBLIC' | :developer | true | false | 'process PyPI api request' | :unauthorized + 'PUBLIC' | :guest | true | false | 'process PyPI api request' | :unauthorized + 'PUBLIC' | :developer | false | true | 'process PyPI api request' | :forbidden + 'PUBLIC' | :guest | false | true | 'process PyPI api request' | :forbidden + 'PUBLIC' | :developer | false | false | 'process PyPI api request' | :unauthorized + 'PUBLIC' | :guest | false | false | 'process PyPI api request' | :unauthorized + 'PUBLIC' | :anonymous | false | true | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :developer | true | true | 'process PyPI api request' | :created + 'PRIVATE' | :guest | true | true | 'process PyPI api request' | :forbidden + 'PRIVATE' | :developer | true | false | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :guest | true | false | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :developer | false | true | 'process PyPI api request' | :not_found + 'PRIVATE' | :guest | false | true | 'process PyPI api request' | :not_found + 'PRIVATE' | :developer | false | false | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :guest | false | false | 'process PyPI api request' | :unauthorized + 'PRIVATE' | :anonymous | false | true | 'process PyPI api request' | :unauthorized end with_them do @@ -185,7 +195,7 @@ RSpec.describe API::PypiPackages do project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) end - it_behaves_like 'process PyPi api request', :developer, :bad_request, true + it_behaves_like 'process PyPI api request', :developer, :bad_request, true end context 'with an invalid package' do @@ -232,24 +242,24 @@ RSpec.describe API::PypiPackages do using RSpec::Parameterized::TableSyntax where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do - 'PUBLIC' | :developer | true | true | 'PyPi package download' | :success - 'PUBLIC' | :guest | true | true | 'PyPi package download' | :success - 'PUBLIC' | :developer | true | false | 'PyPi package download' | :success - 'PUBLIC' | :guest | true | false | 'PyPi package download' | :success - 'PUBLIC' | :developer | false | true | 'PyPi package download' | :success - 'PUBLIC' | :guest | false | true | 'PyPi package download' | :success - 'PUBLIC' | :developer | false | false | 'PyPi package download' | :success - 'PUBLIC' | :guest | false | false | 'PyPi package download' | :success - 'PUBLIC' | :anonymous | false | true | 'PyPi package download' | :success - 'PRIVATE' | :developer | true | true | 'PyPi package download' | :success - 'PRIVATE' | :guest | true | true | 'PyPi package download' | :success - 'PRIVATE' | :developer | true | false | 'PyPi package download' | :success - 'PRIVATE' | :guest | true | false | 'PyPi package download' | :success - 'PRIVATE' | :developer | false | true | 'PyPi package download' | :success - 'PRIVATE' | :guest | false | true | 'PyPi package download' | :success - 'PRIVATE' | :developer | false | false | 'PyPi package download' | :success - 'PRIVATE' | :guest | false | false | 'PyPi package download' | :success - 'PRIVATE' | :anonymous | false | true | 'PyPi package download' | :success + 'PUBLIC' | :developer | true | true | 'PyPI package download' | :success + 'PUBLIC' | :guest | true | true | 'PyPI package download' | :success + 'PUBLIC' | :developer | true | false | 'PyPI package download' | :success + 'PUBLIC' | :guest | true | false | 'PyPI package download' | :success + 'PUBLIC' | :developer | false | true | 'PyPI package download' | :success + 'PUBLIC' | :guest | false | true | 'PyPI package download' | :success + 'PUBLIC' | :developer | false | false | 'PyPI package download' | :success + 'PUBLIC' | :guest | false | false | 'PyPI package download' | :success + 'PUBLIC' | :anonymous | false | true | 'PyPI package download' | :success + 'PRIVATE' | :developer | true | true | 'PyPI package download' | :success + 'PRIVATE' | :guest | true | true | 'PyPI package download' | :success + 'PRIVATE' | :developer | true | false | 'PyPI package download' | :success + 'PRIVATE' | :guest | true | false | 'PyPI package download' | :success + 'PRIVATE' | :developer | false | true | 'PyPI package download' | :success + 'PRIVATE' | :guest | false | true | 'PyPI package download' | :success + 'PRIVATE' | :developer | false | false | 'PyPI package download' | :success + 'PRIVATE' | :guest | false | false | 'PyPI package download' | :success + 'PRIVATE' | :anonymous | false | true | 'PyPI package download' | :success end with_them do diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 779ae983886..e78d05835f2 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -53,6 +53,49 @@ RSpec.describe API::Releases do expect(json_response.second['tag_name']).to eq(release_1.tag) end + RSpec.shared_examples 'release sorting' do |order_by| + subject { get api(url, access_level), params: { sort: sort, order_by: order_by } } + + context "sorting by #{order_by}" do + context 'ascending order' do + let(:sort) { 'asc' } + + it 'returns the sorted releases' do + subject + + expect(json_response.map { |release| release['name'] }).to eq(releases.map(&:name)) + end + end + + context 'descending order' do + let(:sort) { 'desc' } + + it 'returns the sorted releases' do + subject + + expect(json_response.map { |release| release['name'] }).to eq(releases.reverse.map(&:name)) + end + end + end + end + + context 'return releases in sorted order' do + before do + release_2.update_attribute(:created_at, 3.days.ago) + end + + let(:url) { "/projects/#{project.id}/releases" } + let(:access_level) { maintainer } + + it_behaves_like 'release sorting', 'released_at' do + let(:releases) { [release_1, release_2] } + end + + it_behaves_like 'release sorting', 'created_at' do + let(:releases) { [release_2, release_1] } + end + end + it 'matches response schema' do get api("/projects/#{project.id}/releases", maintainer) @@ -259,7 +302,7 @@ RSpec.describe API::Releases do end it '#collected_at' do - Timecop.freeze(Time.now.round) do + travel_to(Time.now.round) do get api("/projects/#{project.id}/releases/v0.1", maintainer) expect(json_response['evidences'].first['collected_at'].to_datetime.to_i).to be_within(1.minute).of(release.evidences.first.created_at.to_i) @@ -476,7 +519,7 @@ RSpec.describe API::Releases do it 'sets the released_at to the current time if the released_at parameter is not provided' do now = Time.zone.parse('2015-08-25 06:00:00Z') - Timecop.freeze(now) do + travel_to(now) do post api("/projects/#{project.id}/releases", maintainer), params: params expect(project.releases.last.released_at).to eq(now) @@ -598,7 +641,7 @@ RSpec.describe API::Releases do end end - context 'when create two assets' do + context 'when creating two assets' do let(:params) do base_params.merge({ assets: { @@ -758,6 +801,65 @@ RSpec.describe API::Releases do expect(response).to have_gitlab_http_status(:conflict) end end + + context 'with milestones' do + let(:subject) { post api("/projects/#{project.id}/releases", maintainer), params: params } + let(:milestone) { create(:milestone, project: project, title: 'v1.0') } + let(:returned_milestones) { json_response['milestones'].map {|m| m['title']} } + + before do + params.merge!(milestone_params) + + subject + end + + context 'with a project milestone' do + let(:milestone_params) { { milestones: [milestone.title] } } + + it 'adds the milestone' do + expect(response).to have_gitlab_http_status(:created) + expect(returned_milestones).to match_array(['v1.0']) + end + end + + context 'with multiple milestones' do + let(:milestone2) { create(:milestone, project: project, title: 'm2') } + let(:milestone_params) { { milestones: [milestone.title, milestone2.title] } } + + it 'adds all milestones' do + expect(response).to have_gitlab_http_status(:created) + expect(returned_milestones).to match_array(['v1.0', 'm2']) + end + end + + context 'with an empty milestone' do + let(:milestone_params) { { milestones: [] } } + + it 'removes all milestones' do + expect(response).to have_gitlab_http_status(:created) + expect(json_response['milestones']).to be_nil + end + end + + context 'with a non-existant milestone' do + let(:milestone_params) { { milestones: ['xyz'] } } + + it 'returns a 400 error as milestone not found' do + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq("Milestone(s) not found: xyz") + end + end + + context 'with a milestone from a different project' do + let(:milestone) { create(:milestone, title: 'v1.0') } + let(:milestone_params) { { milestones: [milestone.title] } } + + it 'returns a 400 error as milestone not found' do + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq("Milestone(s) not found: v1.0") + end + end + end end describe 'PUT /projects/:id/releases/:tag_name' do @@ -863,6 +965,83 @@ RSpec.describe API::Releases do end end end + + context 'with milestones' do + let(:returned_milestones) { json_response['milestones'].map {|m| m['title']} } + + subject { put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params } + + context 'when a milestone is passed in' do + let(:milestone) { create(:milestone, project: project, title: 'v1.0') } + let(:milestone_title) { milestone.title } + let(:params) { { milestones: [milestone_title] } } + + before do + release.milestones << milestone + end + + context 'a different milestone' do + let(:milestone_title) { 'v2.0' } + let!(:milestone2) { create(:milestone, project: project, title: milestone_title) } + + it 'replaces the milestone' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(returned_milestones).to match_array(['v2.0']) + end + end + + context 'an identical milestone' do + let(:milestone_title) { 'v1.0' } + + it 'does not change the milestone' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(returned_milestones).to match_array(['v1.0']) + end + end + + context 'an empty milestone' do + let(:milestone_title) { nil } + + it 'removes the milestone' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['milestones']).to be_nil + end + end + + context 'multiple milestones' do + context 'with one new' do + let!(:milestone2) { create(:milestone, project: project, title: 'milestone2') } + let(:params) { { milestones: [milestone.title, milestone2.title] } } + + it 'adds the new milestone' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(returned_milestones).to match_array(['v1.0', 'milestone2']) + end + end + + context 'with all new' do + let!(:milestone2) { create(:milestone, project: project, title: 'milestone2') } + let!(:milestone3) { create(:milestone, project: project, title: 'milestone3') } + let(:params) { { milestones: [milestone2.title, milestone3.title] } } + + it 'replaces the milestones' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(returned_milestones).to match_array(%w(milestone2 milestone3)) + end + end + end + end + end end describe 'DELETE /projects/:id/releases/:tag_name' do diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 36707f32d04..45bce8c8a5c 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -402,7 +402,9 @@ RSpec.describe API::Repositories do end it "returns an empty string when the diff overflows" do - stub_const('Gitlab::Git::DiffCollection::DEFAULT_LIMITS', { max_files: 2, max_lines: 2 }) + allow(Gitlab::Git::DiffCollection) + .to receive(:default_limits) + .and_return({ max_files: 2, max_lines: 2 }) get api(route, current_user), params: { from: 'master', to: 'feature' } diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index af6731f3015..05cfad9cc62 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -58,6 +58,17 @@ RSpec.describe API::Search do end end + shared_examples 'filter by confidentiality' do |scope:, search:| + it 'respects confidentiality filtering' do + get api(endpoint, user), params: { scope: scope, search: search, confidential: confidential.to_s } + + documents = Gitlab::Json.parse(response.body) + + expect(documents.count).to eq(1) + expect(documents.first['confidential']).to eq(confidential) + end + end + describe 'GET /search' do let(:endpoint) { '/search' } @@ -137,6 +148,26 @@ RSpec.describe API::Search do include_examples 'filter by state', scope: :issues, search: 'awesome' end end + + context 'filter by confidentiality' do + before do + stub_feature_flags(search_filter_by_confidential: true) + create(:issue, project: project, author: user, title: 'awesome non-confidential issue') + create(:issue, :confidential, project: project, author: user, title: 'awesome confidential issue') + end + + context 'confidential: true' do + let(:confidential) { true } + + include_examples 'filter by confidentiality', scope: :issues, search: 'awesome' + end + + context 'confidential: false' do + let(:confidential) { false } + + include_examples 'filter by confidentiality', scope: :issues, search: 'awesome' + end + end end context 'for merge_requests scope' do @@ -231,18 +262,6 @@ RSpec.describe API::Search do it_behaves_like 'pagination', scope: :users it_behaves_like 'ping counters', scope: :users - - context 'when users search feature is disabled' do - before do - stub_feature_flags(users_search: false) - - get api(endpoint, user), params: { scope: 'users', search: 'billy' } - end - - it 'returns 400 error' do - expect(response).to have_gitlab_http_status(:bad_request) - end - end end context 'for snippet_titles scope' do @@ -416,18 +435,6 @@ RSpec.describe API::Search do include_examples 'pagination', scope: :users end - - context 'when users search feature is disabled' do - before do - stub_feature_flags(users_search: false) - - get api(endpoint, user), params: { scope: 'users', search: 'billy' } - end - - it 'returns 400 error' do - expect(response).to have_gitlab_http_status(:bad_request) - end - end end context 'for users scope with group path as id' do @@ -589,18 +596,6 @@ RSpec.describe API::Search do include_examples 'pagination', scope: :users end - - context 'when users search feature is disabled' do - before do - stub_feature_flags(users_search: false) - - get api(endpoint, user), params: { scope: 'users', search: 'billy' } - end - - it 'returns 400 error' do - expect(response).to have_gitlab_http_status(:bad_request) - end - end end context 'for notes scope' do diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index 5528a0c094f..63ed57c5045 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -264,4 +264,34 @@ RSpec.describe API::Services do expect(json_response['properties']['notify_only_broken_pipelines']).to eq(true) end end + + describe 'Hangouts Chat service' do + let(:service_name) { 'hangouts-chat' } + let(:params) do + { + webhook: 'https://hook.example.com', + branches_to_be_notified: 'default' + } + end + + before do + project.create_hangouts_chat_service( + active: true, + properties: params + ) + end + + it 'accepts branches_to_be_notified for update', :aggregate_failures do + put api("/projects/#{project.id}/services/#{service_name}", user), params: params.merge(branches_to_be_notified: 'all') + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['properties']['branches_to_be_notified']).to eq('all') + end + + it 'only requires the webhook param' do + put api("/projects/#{project.id}/services/#{service_name}", user), params: { webhook: 'https://hook.example.com' } + + expect(response).to have_gitlab_http_status(:ok) + end + end end diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index ef12f6dbed3..8b5f74df8f8 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -96,6 +96,7 @@ RSpec.describe API::Settings, 'Settings' do help_page_text: 'custom help text', help_page_hide_commercial_content: true, help_page_support_url: 'http://example.com/help', + help_page_documentation_base_url: 'https://docs.gitlab.com', project_export_enabled: false, rsa_key_restriction: ApplicationSetting::FORBIDDEN_KEY_VALUE, dsa_key_restriction: 2048, @@ -138,6 +139,7 @@ RSpec.describe API::Settings, 'Settings' do expect(json_response['help_page_text']).to eq('custom help text') expect(json_response['help_page_hide_commercial_content']).to be_truthy expect(json_response['help_page_support_url']).to eq('http://example.com/help') + expect(json_response['help_page_documentation_base_url']).to eq('https://docs.gitlab.com') expect(json_response['project_export_enabled']).to be_falsey expect(json_response['rsa_key_restriction']).to eq(ApplicationSetting::FORBIDDEN_KEY_VALUE) expect(json_response['dsa_key_restriction']).to eq(2048) @@ -413,6 +415,14 @@ RSpec.describe API::Settings, 'Settings' do end end + it 'supports legacy admin_notification_email' do + put api('/application/settings', admin), + params: { admin_notification_email: 'test@example.com' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['abuse_notification_email']).to eq('test@example.com') + end + context "missing sourcegraph_url value when sourcegraph_enabled is true" do it "returns a blank parameter error message" do put api("/application/settings", admin), params: { sourcegraph_enabled: true } diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 8d77026d26c..227c53f8fb9 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -2,18 +2,28 @@ require 'spec_helper' -RSpec.describe API::Snippets do +RSpec.describe API::Snippets, factory_default: :keep do include SnippetHelpers - let_it_be(:user) { create(:user) } + let_it_be(:admin) { create(:user, :admin) } + let_it_be(:user) { create(:user) } + let_it_be(:other_user) { create(:user) } - describe 'GET /snippets/' do - it 'returns snippets available' do - public_snippet = create(:personal_snippet, :repository, :public, author: user) - private_snippet = create(:personal_snippet, :repository, :private, author: user) - internal_snippet = create(:personal_snippet, :repository, :internal, author: user) + let_it_be(:public_snippet) { create(:personal_snippet, :repository, :public, author: user) } + let_it_be(:private_snippet) { create(:personal_snippet, :repository, :private, author: user) } + let_it_be(:internal_snippet) { create(:personal_snippet, :repository, :internal, author: user) } + + let_it_be(:user_token) { create(:personal_access_token, user: user) } + let_it_be(:other_user_token) { create(:personal_access_token, user: other_user) } + let_it_be(:project) do + create_default(:project, :public).tap do |p| + p.add_maintainer(user) + end + end - get api("/snippets/", user) + describe 'GET /snippets/' do + it 'returns snippets available for user' do + get api("/snippets/", personal_access_token: user_token) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -29,9 +39,7 @@ RSpec.describe API::Snippets do end it 'hides private snippets from regular user' do - create(:personal_snippet, :private) - - get api("/snippets/", user) + get api("/snippets/", personal_access_token: other_user_token) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -39,21 +47,17 @@ RSpec.describe API::Snippets do expect(json_response.size).to eq(0) end - it 'returns 404 for non-authenticated' do - create(:personal_snippet, :internal) - + it 'returns 401 for non-authenticated' do get api("/snippets/") expect(response).to have_gitlab_http_status(:unauthorized) end it 'does not return snippets related to a project with disable feature visibility' do - project = create(:project) - create(:project_member, project: project, user: user) - public_snippet = create(:personal_snippet, :public, author: user, project: project) + public_snippet = create(:project_snippet, :public, author: user, project: project) project.project_feature.update_attribute(:snippets_access_level, 0) - get api("/snippets/", user) + get api("/snippets/", personal_access_token: user_token) json_response.each do |snippet| expect(snippet["id"]).not_to eq(public_snippet.id) @@ -62,10 +66,6 @@ RSpec.describe API::Snippets do end describe 'GET /snippets/public' do - let_it_be(:other_user) { create(:user) } - let_it_be(:public_snippet) { create(:personal_snippet, :repository, :public, author: user) } - let_it_be(:private_snippet) { create(:personal_snippet, :repository, :private, author: user) } - let_it_be(:internal_snippet) { create(:personal_snippet, :repository, :internal, author: user) } let_it_be(:public_snippet_other) { create(:personal_snippet, :repository, :public, author: other_user) } let_it_be(:private_snippet_other) { create(:personal_snippet, :repository, :private, author: other_user) } let_it_be(:internal_snippet_other) { create(:personal_snippet, :repository, :internal, author: other_user) } @@ -73,8 +73,10 @@ RSpec.describe API::Snippets do let_it_be(:private_snippet_project) { create(:project_snippet, :repository, :private, author: user) } let_it_be(:internal_snippet_project) { create(:project_snippet, :repository, :internal, author: user) } - it 'returns all snippets with public visibility from all users' do - get api("/snippets/public", user) + let(:path) { "/snippets/public" } + + it 'returns only public snippets from all users when authenticated' do + get api(path, personal_access_token: user_token) aggregate_failures do expect(response).to have_gitlab_http_status(:ok) @@ -90,20 +92,23 @@ RSpec.describe API::Snippets do expect(json_response[1]['files'].first).to eq snippet_blob_file(public_snippet.blobs.first) end end - end - - describe 'GET /snippets/:id/raw' do - let_it_be(:author) { create(:user) } - let_it_be(:snippet) { create(:personal_snippet, :repository, :private, author: author) } it 'requires authentication' do - get api("/snippets/#{snippet.id}", nil) + get api(path, nil) expect(response).to have_gitlab_http_status(:unauthorized) end + end + + describe 'GET /snippets/:id/raw' do + let(:snippet) { private_snippet } + + it_behaves_like 'snippet access with different users' do + let(:path) { "/snippets/#{snippet.id}/raw" } + end it 'returns raw text' do - get api("/snippets/#{snippet.id}/raw", author) + get api("/snippets/#{snippet.id}/raw", personal_access_token: user_token) expect(response).to have_gitlab_http_status(:ok) expect(response.media_type).to eq 'text/plain' @@ -113,69 +118,37 @@ RSpec.describe API::Snippets do it 'returns 404 for invalid snippet id' do snippet.destroy! - get api("/snippets/#{snippet.id}/raw", author) + get api("/snippets/#{snippet.id}/raw", personal_access_token: user_token) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Snippet Not Found') end - it 'hides private snippets from ordinary users' do - get api("/snippets/#{snippet.id}/raw", user) - - expect(response).to have_gitlab_http_status(:not_found) - end - - it 'shows internal snippets to ordinary users' do - internal_snippet = create(:personal_snippet, :internal, author: author) - - get api("/snippets/#{internal_snippet.id}/raw", user) - - expect(response).to have_gitlab_http_status(:ok) - end - it_behaves_like 'snippet blob content' do - let_it_be(:snippet_with_empty_repo) { create(:personal_snippet, :empty_repo, :private, author: author) } + let_it_be(:snippet_with_empty_repo) { create(:personal_snippet, :empty_repo, :private, author: user) } - subject { get api("/snippets/#{snippet.id}/raw", snippet.author) } + subject { get api("/snippets/#{snippet.id}/raw", snippet.author, personal_access_token: user_token) } end end describe 'GET /snippets/:id/files/:ref/:file_path/raw' do - let_it_be(:snippet) { create(:personal_snippet, :repository, :private) } + let_it_be(:snippet) { private_snippet } it_behaves_like 'raw snippet files' do let(:api_path) { "/snippets/#{snippet_id}/files/#{ref}/#{file_path}/raw" } end - end - - describe 'GET /snippets/:id' do - let_it_be(:admin) { create(:user, :admin) } - let_it_be(:author) { create(:user) } - let_it_be(:private_snippet) { create(:personal_snippet, :repository, :private, author: author) } - let_it_be(:internal_snippet) { create(:personal_snippet, :repository, :internal, author: author) } - let(:snippet) { private_snippet } - subject { get api("/snippets/#{snippet.id}", user) } - - it 'hides private snippets from an ordinary user' do - subject - - expect(response).to have_gitlab_http_status(:not_found) + it_behaves_like 'snippet access with different users' do + let(:path) { "/snippets/#{snippet.id}/files/master/%2Egitattributes/raw" } end + end - context 'without a user' do - let(:user) { nil } + describe 'GET /snippets/:id' do + let(:snippet_id) { private_snippet.id } - it 'requires authentication' do - subject - - expect(response).to have_gitlab_http_status(:unauthorized) - end - end + subject { get api("/snippets/#{snippet_id}", personal_access_token: user_token) } context 'with the author' do - let(:user) { author } - it 'returns snippet json' do subject @@ -191,18 +164,10 @@ RSpec.describe API::Snippets do end end - context 'with an admin' do - let(:user) { admin } - - it 'shows private snippets to an admin' do - subject - - expect(response).to have_gitlab_http_status(:ok) - end - - it 'returns 404 for invalid snippet id' do - private_snippet.destroy! + context 'with a non-existent snippet ID' do + let(:snippet_id) { 0 } + it 'returns 404' do subject expect(response).to have_gitlab_http_status(:not_found) @@ -210,18 +175,8 @@ RSpec.describe API::Snippets do end end - context 'with an internal snippet' do - let(:snippet) { internal_snippet } - - it 'shows internal snippets to an ordinary user' do - subject - - expect(response).to have_gitlab_http_status(:ok) - end - end - - it_behaves_like 'snippet_multiple_files feature disabled' do - let(:user) { author } + it_behaves_like 'snippet access with different users' do + let(:path) { "/snippets/#{snippet.id}" } end end @@ -241,7 +196,7 @@ RSpec.describe API::Snippets do let(:file_params) { { files: [{ file_path: file_path, content: file_content }] } } let(:extra_params) { {} } - subject { post api("/snippets/", user), params: params } + subject { post api("/snippets/", personal_access_token: user_token), params: params } shared_examples 'snippet creation' do let(:snippet) { Snippet.find(json_response["id"]) } @@ -305,12 +260,9 @@ RSpec.describe API::Snippets do it_behaves_like 'snippet creation' - it_behaves_like 'snippet_multiple_files feature disabled' do - let(:snippet) { Snippet.find(json_response["id"]) } - end - context 'with an external user' do let(:user) { create(:user, :external) } + let(:user_token) { create(:personal_access_token, user: user) } it 'does not create a new snippet' do subject @@ -384,8 +336,6 @@ RSpec.describe API::Snippets do end describe 'PUT /snippets/:id' do - let_it_be(:other_user) { create(:user) } - let(:visibility_level) { Snippet::PUBLIC } let(:snippet) do create(:personal_snippet, :repository, author: user, visibility_level: visibility_level) @@ -465,11 +415,10 @@ RSpec.describe API::Snippets do end context "when admin" do - let(:admin) { create(:admin) } - let(:token) { create(:personal_access_token, user: admin, scopes: [:sudo]) } + let_it_be(:token) { create(:personal_access_token, user: admin, scopes: [:sudo]) } subject do - put api("/snippets/#{snippet.id}", admin, personal_access_token: token), params: { visibility: 'private', sudo: user.id } + put api("/snippets/#{snippet.id}", personal_access_token: token), params: { visibility: 'private', sudo: user.id } end context 'when sudo is defined' do @@ -496,34 +445,32 @@ RSpec.describe API::Snippets do end describe 'DELETE /snippets/:id' do - let!(:public_snippet) { create(:personal_snippet, :public, author: user) } - it 'deletes snippet' do expect do - delete api("/snippets/#{public_snippet.id}", user) + delete api("/snippets/#{public_snippet.id}", personal_access_token: user_token) expect(response).to have_gitlab_http_status(:no_content) end.to change { PersonalSnippet.count }.by(-1) end it 'returns 404 for invalid snippet id' do - delete api("/snippets/#{non_existing_record_id}", user) + delete api("/snippets/#{non_existing_record_id}", personal_access_token: user_token) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Snippet Not Found') end it_behaves_like '412 response' do - let(:request) { api("/snippets/#{public_snippet.id}", user) } + let(:request) { api("/snippets/#{public_snippet.id}", personal_access_token: user_token) } end end describe "GET /snippets/:id/user_agent_detail" do - let(:admin) { create(:admin) } - let(:snippet) { create(:personal_snippet, :public, author: user) } - let!(:user_agent_detail) { create(:user_agent_detail, subject: snippet) } + let(:snippet) { public_snippet } it 'exposes known attributes' do + user_agent_detail = create(:user_agent_detail, subject: snippet) + get api("/snippets/#{snippet.id}/user_agent_detail", admin) expect(response).to have_gitlab_http_status(:ok) diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb index 8d128bd911f..aff41ff5974 100644 --- a/spec/requests/api/terraform/state_spec.rb +++ b/spec/requests/api/terraform/state_spec.rb @@ -18,7 +18,7 @@ RSpec.describe API::Terraform::State do let(:state_path) { "/projects/#{project_id}/terraform/state/#{state_name}" } before do - stub_terraform_state_object_storage(Terraform::StateUploader) + stub_terraform_state_object_storage end describe 'GET /projects/:id/terraform/state/:name' do diff --git a/spec/requests/api/terraform/state_version_spec.rb b/spec/requests/api/terraform/state_version_spec.rb new file mode 100644 index 00000000000..ade0aacf805 --- /dev/null +++ b/spec/requests/api/terraform/state_version_spec.rb @@ -0,0 +1,210 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Terraform::StateVersion do + include HttpBasicAuthHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:developer) { create(:user, developer_projects: [project]) } + let_it_be(:maintainer) { create(:user, maintainer_projects: [project]) } + let_it_be(:user_without_access) { create(:user) } + + let_it_be(:state) { create(:terraform_state, project: project) } + + let!(:versions) { create_list(:terraform_state_version, 3, terraform_state: state) } + + let(:current_user) { maintainer } + let(:auth_header) { user_basic_auth_header(current_user) } + let(:project_id) { project.id } + let(:state_name) { state.name } + let(:version) { versions.last } + let(:version_serial) { version.version } + let(:state_version_path) { "/projects/#{project_id}/terraform/state/#{state_name}/versions/#{version_serial}" } + + describe 'GET /projects/:id/terraform/state/:name/versions/:serial' do + subject(:request) { get api(state_version_path), headers: auth_header } + + context 'with invalid authentication' do + let(:auth_header) { basic_auth_header('bad', 'token') } + + it 'returns unauthorized status' do + request + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'with no authentication' do + let(:auth_header) { nil } + + it 'returns unauthorized status' do + request + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'personal acceess token authentication' do + context 'with maintainer permissions' do + let(:current_user) { maintainer } + + it 'returns the state contents at the given version' do + request + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(version.file.read) + end + + context 'for a project that does not exist' do + let(:project_id) { '0000' } + + it 'returns not found status' do + request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'with developer permissions' do + let(:current_user) { developer } + + it 'returns the state contents at the given version' do + request + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(version.file.read) + end + end + + context 'with no permissions' do + let(:current_user) { user_without_access } + + it 'returns not found status' do + request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'job token authentication' do + let(:auth_header) { job_basic_auth_header(job) } + + context 'with maintainer permissions' do + let(:job) { create(:ci_build, status: :running, project: project, user: maintainer) } + + it 'returns the state contents at the given version' do + request + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(version.file.read) + end + + it 'returns unauthorized status if the the job is not running' do + job.update!(status: :failed) + request + + expect(response).to have_gitlab_http_status(:unauthorized) + end + + context 'for a project that does not exist' do + let(:project_id) { '0000' } + + it 'returns not found status' do + request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'with developer permissions' do + let(:job) { create(:ci_build, status: :running, project: project, user: developer) } + + it 'returns the state contents at the given version' do + request + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(version.file.read) + end + end + + context 'with no permissions' do + let(:current_user) { user_without_access } + let(:job) { create(:ci_build, status: :running, user: current_user) } + + it 'returns not found status' do + request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + + describe 'DELETE /projects/:id/terraform/state/:name/versions/:serial' do + subject(:request) { delete api(state_version_path), headers: auth_header } + + context 'with invalid authentication' do + let(:auth_header) { basic_auth_header('bad', 'token') } + + it 'returns unauthorized status' do + request + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'with no authentication' do + let(:auth_header) { nil } + + it 'returns unauthorized status' do + request + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'with maintainer permissions' do + let(:current_user) { maintainer } + + it 'deletes the version' do + expect { request }.to change { Terraform::StateVersion.count }.by(-1) + + expect(response).to have_gitlab_http_status(:no_content) + end + + context 'version does not exist' do + let(:version_serial) { -1 } + + it 'does not delete a version' do + expect { request }.to change { Terraform::StateVersion.count }.by(0) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'with developer permissions' do + let(:current_user) { developer } + + it 'returns forbidden status' do + expect { request }.to change { Terraform::StateVersion.count }.by(0) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'with no permissions' do + let(:current_user) { user_without_access } + + it 'returns not found status' do + expect { request }.to change { Terraform::StateVersion.count }.by(0) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/requests/api/unleash_spec.rb b/spec/requests/api/unleash_spec.rb new file mode 100644 index 00000000000..0b70d62b093 --- /dev/null +++ b/spec/requests/api/unleash_spec.rb @@ -0,0 +1,608 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Unleash do + include FeatureFlagHelpers + + let_it_be(:project, refind: true) { create(:project) } + let(:project_id) { project.id } + let(:params) { } + let(:headers) { } + + shared_examples 'authenticated request' do + context 'when using instance id' do + let(:client) { create(:operations_feature_flags_client, project: project) } + let(:params) { { instance_id: client.token } } + + it 'responds with OK' do + subject + + expect(response).to have_gitlab_http_status(:ok) + end + + context 'when repository is disabled' do + before do + project.project_feature.update!( + repository_access_level: ::ProjectFeature::DISABLED, + merge_requests_access_level: ::ProjectFeature::DISABLED, + builds_access_level: ::ProjectFeature::DISABLED + ) + end + + it 'responds with forbidden' do + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when repository is private' do + before do + project.project_feature.update!( + repository_access_level: ::ProjectFeature::PRIVATE, + merge_requests_access_level: ::ProjectFeature::DISABLED, + builds_access_level: ::ProjectFeature::DISABLED + ) + end + + it 'responds with OK' do + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + context 'when using header' do + let(:client) { create(:operations_feature_flags_client, project: project) } + let(:headers) { { "UNLEASH-INSTANCEID" => client.token }} + + it 'responds with OK' do + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when using bogus instance id' do + let(:params) { { instance_id: 'token' } } + + it 'responds with unauthorized' do + subject + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when using not existing project' do + let(:project_id) { -5000 } + let(:params) { { instance_id: 'token' } } + + it 'responds with unauthorized' do + subject + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + shared_examples_for 'support multiple environments' do + let!(:client) { create(:operations_feature_flags_client, project: project) } + let!(:base_headers) { { "UNLEASH-INSTANCEID" => client.token } } + let!(:headers) { base_headers.merge({ "UNLEASH-APPNAME" => "test" }) } + + let!(:feature_flag_1) do + create(:operations_feature_flag, name: "feature_flag_1", project: project, active: true) + end + + let!(:feature_flag_2) do + create(:operations_feature_flag, name: "feature_flag_2", project: project, active: false) + end + + before do + create_scope(feature_flag_1, 'production', false) + create_scope(feature_flag_2, 'review/*', true) + end + + it 'does not have N+1 problem' do + control_count = ActiveRecord::QueryRecorder.new { get api(features_url), headers: headers }.count + + create(:operations_feature_flag, name: "feature_flag_3", project: project, active: true) + + expect { get api(features_url), headers: headers }.not_to exceed_query_limit(control_count) + end + + context 'when app name is staging' do + let(:headers) { base_headers.merge({ "UNLEASH-APPNAME" => "staging" }) } + + it 'returns correct active values' do + subject + + feature_flag_1 = json_response['features'].find { |f| f['name'] == 'feature_flag_1' } + feature_flag_2 = json_response['features'].find { |f| f['name'] == 'feature_flag_2' } + + expect(feature_flag_1['enabled']).to eq(true) + expect(feature_flag_2['enabled']).to eq(false) + end + end + + context 'when app name is production' do + let(:headers) { base_headers.merge({ "UNLEASH-APPNAME" => "production" }) } + + it 'returns correct active values' do + subject + + feature_flag_1 = json_response['features'].find { |f| f['name'] == 'feature_flag_1' } + feature_flag_2 = json_response['features'].find { |f| f['name'] == 'feature_flag_2' } + + expect(feature_flag_1['enabled']).to eq(false) + expect(feature_flag_2['enabled']).to eq(false) + end + end + + context 'when app name is review/patch-1' do + let(:headers) { base_headers.merge({ "UNLEASH-APPNAME" => "review/patch-1" }) } + + it 'returns correct active values' do + subject + + feature_flag_1 = json_response['features'].find { |f| f['name'] == 'feature_flag_1' } + feature_flag_2 = json_response['features'].find { |f| f['name'] == 'feature_flag_2' } + + expect(feature_flag_1['enabled']).to eq(true) + expect(feature_flag_2['enabled']).to eq(false) + end + end + + context 'when app name is empty' do + let(:headers) { base_headers } + + it 'returns empty list' do + subject + + expect(json_response['features'].count).to eq(0) + end + end + end + + %w(/feature_flags/unleash/:project_id/features /feature_flags/unleash/:project_id/client/features).each do |features_endpoint| + describe "GET #{features_endpoint}" do + let(:features_url) { features_endpoint.sub(':project_id', project_id.to_s) } + let(:client) { create(:operations_feature_flags_client, project: project) } + + subject { get api(features_url), params: params, headers: headers } + + it_behaves_like 'authenticated request' + + context 'with version 1 (legacy) feature flags' do + let(:feature_flag) { create(:operations_feature_flag, project: project, name: 'feature1', active: true, version: 1) } + + it_behaves_like 'support multiple environments' + + context 'with a list of feature flags' do + let(:headers) { { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" } } + let!(:enabled_feature_flag) { create(:operations_feature_flag, project: project, name: 'feature1', active: true, version: 1) } + let!(:disabled_feature_flag) { create(:operations_feature_flag, project: project, name: 'feature2', active: false, version: 1) } + + it 'responds with a list of features' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['version']).to eq(1) + expect(json_response['features']).not_to be_empty + expect(json_response['features'].map { |f| f['name'] }.sort).to eq(%w[feature1 feature2]) + expect(json_response['features'].sort_by {|f| f['name'] }.map { |f| f['enabled'] }).to eq([true, false]) + end + + it 'matches json schema' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('unleash/unleash') + end + end + + it 'returns a feature flag strategy' do + create(:operations_feature_flag_scope, + feature_flag: feature_flag, + environment_scope: 'sandbox', + active: true, + strategies: [{ name: "gradualRolloutUserId", + parameters: { groupId: "default", percentage: "50" } }]) + headers = { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "sandbox" } + + get api(features_url), headers: headers + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features'].first['enabled']).to eq(true) + strategies = json_response['features'].first['strategies'] + expect(strategies).to eq([{ + "name" => "gradualRolloutUserId", + "parameters" => { + "percentage" => "50", + "groupId" => "default" + } + }]) + end + + it 'returns a default strategy for a scope' do + create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'sandbox', active: true) + headers = { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "sandbox" } + + get api(features_url), headers: headers + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features'].first['enabled']).to eq(true) + strategies = json_response['features'].first['strategies'] + expect(strategies).to eq([{ "name" => "default", "parameters" => {} }]) + end + + it 'returns multiple strategies for a feature flag' do + create(:operations_feature_flag_scope, + feature_flag: feature_flag, + environment_scope: 'staging', + active: true, + strategies: [{ name: "userWithId", parameters: { userIds: "max,fred" } }, + { name: "gradualRolloutUserId", + parameters: { groupId: "default", percentage: "50" } }]) + headers = { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "staging" } + + get api(features_url), headers: headers + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features'].first['enabled']).to eq(true) + strategies = json_response['features'].first['strategies'].sort_by { |s| s['name'] } + expect(strategies).to eq([{ + "name" => "gradualRolloutUserId", + "parameters" => { + "percentage" => "50", + "groupId" => "default" + } + }, { + "name" => "userWithId", + "parameters" => { + "userIds" => "max,fred" + } + }]) + end + + it 'returns a disabled feature when the flag is disabled' do + flag = create(:operations_feature_flag, project: project, name: 'test_feature', active: false, version: 1) + create(:operations_feature_flag_scope, feature_flag: flag, environment_scope: 'production', active: true) + headers = { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" } + + get api(features_url), headers: headers + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features'].first['enabled']).to eq(false) + end + + context "with an inactive scope" do + let!(:scope) { create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'production', active: false, strategies: [{ name: "default", parameters: {} }]) } + let(:headers) { { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" } } + + it 'returns a disabled feature' do + get api(features_url), headers: headers + + expect(response).to have_gitlab_http_status(:ok) + feature_json = json_response['features'].first + expect(feature_json['enabled']).to eq(false) + expect(feature_json['strategies']).to eq([{ 'name' => 'default', 'parameters' => {} }]) + end + end + end + + context 'with version 2 feature flags' do + it 'does not return a flag without any strategies' do + create(:operations_feature_flag, project: project, + name: 'feature1', active: true, version: 2) + + get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features']).to be_empty + end + + it 'returns a flag with a default strategy' do + feature_flag = create(:operations_feature_flag, project: project, + name: 'feature1', active: true, version: 2) + strategy = create(:operations_strategy, feature_flag: feature_flag, + name: 'default', parameters: {}) + create(:operations_scope, strategy: strategy, environment_scope: 'production') + + get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features']).to eq([{ + 'name' => 'feature1', + 'enabled' => true, + 'strategies' => [{ + 'name' => 'default', + 'parameters' => {} + }] + }]) + end + + it 'returns a flag with a userWithId strategy' do + feature_flag = create(:operations_feature_flag, project: project, + name: 'feature1', active: true, version: 2) + strategy = create(:operations_strategy, feature_flag: feature_flag, + name: 'userWithId', parameters: { userIds: 'user123,user456' }) + create(:operations_scope, strategy: strategy, environment_scope: 'production') + + get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features']).to eq([{ + 'name' => 'feature1', + 'enabled' => true, + 'strategies' => [{ + 'name' => 'userWithId', + 'parameters' => { 'userIds' => 'user123,user456' } + }] + }]) + end + + it 'returns a flag with multiple strategies' do + feature_flag = create(:operations_feature_flag, project: project, + name: 'feature1', active: true, version: 2) + strategy_a = create(:operations_strategy, feature_flag: feature_flag, + name: 'userWithId', parameters: { userIds: 'user_a,user_b' }) + strategy_b = create(:operations_strategy, feature_flag: feature_flag, + name: 'gradualRolloutUserId', parameters: { groupId: 'default', percentage: '45' }) + create(:operations_scope, strategy: strategy_a, environment_scope: 'production') + create(:operations_scope, strategy: strategy_b, environment_scope: 'production') + + get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features'].map { |f| f['name'] }.sort).to eq(['feature1']) + features_json = json_response['features'].map do |feature| + feature.merge(feature.slice('strategies').transform_values { |v| v.sort_by { |s| s['name'] } }) + end + expect(features_json).to eq([{ + 'name' => 'feature1', + 'enabled' => true, + 'strategies' => [{ + 'name' => 'gradualRolloutUserId', + 'parameters' => { 'groupId' => 'default', 'percentage' => '45' } + }, { + 'name' => 'userWithId', + 'parameters' => { 'userIds' => 'user_a,user_b' } + }] + }]) + end + + it 'returns only flags matching the environment scope' do + feature_flag_a = create(:operations_feature_flag, project: project, + name: 'feature1', active: true, version: 2) + strategy_a = create(:operations_strategy, feature_flag: feature_flag_a) + create(:operations_scope, strategy: strategy_a, environment_scope: 'production') + feature_flag_b = create(:operations_feature_flag, project: project, + name: 'feature2', active: true, version: 2) + strategy_b = create(:operations_strategy, feature_flag: feature_flag_b) + create(:operations_scope, strategy: strategy_b, environment_scope: 'staging') + + get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'staging' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features'].map { |f| f['name'] }.sort).to eq(['feature2']) + expect(json_response['features']).to eq([{ + 'name' => 'feature2', + 'enabled' => true, + 'strategies' => [{ + 'name' => 'default', + 'parameters' => {} + }] + }]) + end + + it 'returns only strategies matching the environment scope' do + feature_flag = create(:operations_feature_flag, project: project, + name: 'feature1', active: true, version: 2) + strategy_a = create(:operations_strategy, feature_flag: feature_flag, + name: 'userWithId', parameters: { userIds: 'user2,user8,user4' }) + create(:operations_scope, strategy: strategy_a, environment_scope: 'production') + strategy_b = create(:operations_strategy, feature_flag: feature_flag, + name: 'default', parameters: {}) + create(:operations_scope, strategy: strategy_b, environment_scope: 'staging') + + get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features']).to eq([{ + 'name' => 'feature1', + 'enabled' => true, + 'strategies' => [{ + 'name' => 'userWithId', + 'parameters' => { 'userIds' => 'user2,user8,user4' } + }] + }]) + end + + it 'returns only flags for the given project' do + project_b = create(:project) + feature_flag_a = create(:operations_feature_flag, project: project, name: 'feature_a', active: true, version: 2) + strategy_a = create(:operations_strategy, feature_flag: feature_flag_a) + create(:operations_scope, strategy: strategy_a, environment_scope: 'sandbox') + feature_flag_b = create(:operations_feature_flag, project: project_b, name: 'feature_b', active: true, version: 2) + strategy_b = create(:operations_strategy, feature_flag: feature_flag_b) + create(:operations_scope, strategy: strategy_b, environment_scope: 'sandbox') + + get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'sandbox' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features']).to eq([{ + 'name' => 'feature_a', + 'enabled' => true, + 'strategies' => [{ + 'name' => 'default', + 'parameters' => {} + }] + }]) + end + + it 'returns all strategies with a matching scope' do + feature_flag = create(:operations_feature_flag, project: project, + name: 'feature1', active: true, version: 2) + strategy_a = create(:operations_strategy, feature_flag: feature_flag, + name: 'userWithId', parameters: { userIds: 'user2,user8,user4' }) + create(:operations_scope, strategy: strategy_a, environment_scope: '*') + strategy_b = create(:operations_strategy, feature_flag: feature_flag, + name: 'default', parameters: {}) + create(:operations_scope, strategy: strategy_b, environment_scope: 'review/*') + strategy_c = create(:operations_strategy, feature_flag: feature_flag, + name: 'gradualRolloutUserId', parameters: { groupId: 'default', percentage: '15' }) + create(:operations_scope, strategy: strategy_c, environment_scope: 'review/patch-1') + + get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'review/patch-1' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features'].first['strategies'].sort_by { |s| s['name'] }).to eq([{ + 'name' => 'default', + 'parameters' => {} + }, { + 'name' => 'gradualRolloutUserId', + 'parameters' => { 'groupId' => 'default', 'percentage' => '15' } + }, { + 'name' => 'userWithId', + 'parameters' => { 'userIds' => 'user2,user8,user4' } + }]) + end + + it 'returns a strategy with more than one matching scope' do + feature_flag = create(:operations_feature_flag, project: project, + name: 'feature1', active: true, version: 2) + strategy = create(:operations_strategy, feature_flag: feature_flag, + name: 'default', parameters: {}) + create(:operations_scope, strategy: strategy, environment_scope: 'production') + create(:operations_scope, strategy: strategy, environment_scope: '*') + + get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features']).to eq([{ + 'name' => 'feature1', + 'enabled' => true, + 'strategies' => [{ + 'name' => 'default', + 'parameters' => {} + }] + }]) + end + + it 'returns a disabled flag with a matching scope' do + feature_flag = create(:operations_feature_flag, project: project, + name: 'myfeature', active: false, version: 2) + strategy = create(:operations_strategy, feature_flag: feature_flag, + name: 'default', parameters: {}) + create(:operations_scope, strategy: strategy, environment_scope: 'production') + + get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features']).to eq([{ + 'name' => 'myfeature', + 'enabled' => false, + 'strategies' => [{ + 'name' => 'default', + 'parameters' => {} + }] + }]) + end + + it 'returns a userWithId strategy for a gitlabUserList strategy' do + feature_flag = create(:operations_feature_flag, :new_version_flag, project: project, + name: 'myfeature', active: true) + user_list = create(:operations_feature_flag_user_list, project: project, + name: 'My List', user_xids: 'user1,user2') + strategy = create(:operations_strategy, feature_flag: feature_flag, + name: 'gitlabUserList', parameters: {}, user_list: user_list) + create(:operations_scope, strategy: strategy, environment_scope: 'production') + + get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features']).to eq([{ + 'name' => 'myfeature', + 'enabled' => true, + 'strategies' => [{ + 'name' => 'userWithId', + 'parameters' => { 'userIds' => 'user1,user2' } + }] + }]) + end + end + + context 'when mixing version 1 and version 2 feature flags' do + it 'returns both types of flags when both match' do + feature_flag_a = create(:operations_feature_flag, project: project, + name: 'feature_a', active: true, version: 2) + strategy = create(:operations_strategy, feature_flag: feature_flag_a, + name: 'userWithId', parameters: { userIds: 'user8' }) + create(:operations_scope, strategy: strategy, environment_scope: 'staging') + feature_flag_b = create(:operations_feature_flag, project: project, + name: 'feature_b', active: true, version: 1) + create(:operations_feature_flag_scope, feature_flag: feature_flag_b, + active: true, strategies: [{ name: 'default', parameters: {} }], environment_scope: 'staging') + + get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'staging' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features'].sort_by {|f| f['name']}).to eq([{ + 'name' => 'feature_a', + 'enabled' => true, + 'strategies' => [{ + 'name' => 'userWithId', + 'parameters' => { 'userIds' => 'user8' } + }] + }, { + 'name' => 'feature_b', + 'enabled' => true, + 'strategies' => [{ + 'name' => 'default', + 'parameters' => {} + }] + }]) + end + + it 'returns legacy flags when only legacy flags match' do + feature_flag_a = create(:operations_feature_flag, project: project, + name: 'feature_a', active: true, version: 2) + strategy = create(:operations_strategy, feature_flag: feature_flag_a, + name: 'userWithId', parameters: { userIds: 'user8' }) + create(:operations_scope, strategy: strategy, environment_scope: 'production') + feature_flag_b = create(:operations_feature_flag, project: project, + name: 'feature_b', active: true, version: 1) + create(:operations_feature_flag_scope, feature_flag: feature_flag_b, + active: true, strategies: [{ name: 'default', parameters: {} }], environment_scope: 'staging') + + get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'staging' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['features']).to eq([{ + 'name' => 'feature_b', + 'enabled' => true, + 'strategies' => [{ + 'name' => 'default', + 'parameters' => {} + }] + }]) + end + end + end + end + + describe 'POST /feature_flags/unleash/:project_id/client/register' do + subject { post api("/feature_flags/unleash/#{project_id}/client/register"), params: params, headers: headers } + + it_behaves_like 'authenticated request' + end + + describe 'POST /feature_flags/unleash/:project_id/client/metrics' do + subject { post api("/feature_flags/unleash/#{project_id}/client/metrics"), params: params, headers: headers } + + it_behaves_like 'authenticated request' + end +end diff --git a/spec/requests/api/usage_data_spec.rb b/spec/requests/api/usage_data_spec.rb index 46dd54dcc73..4f4f386e9db 100644 --- a/spec/requests/api/usage_data_spec.rb +++ b/spec/requests/api/usage_data_spec.rb @@ -66,6 +66,10 @@ RSpec.describe API::UsageData do end context 'with unknown event' do + before do + skip_feature_flags_yaml_validation + end + it 'returns status ok' do expect(Gitlab::Redis::HLL).not_to receive(:add) diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 806b586ef49..7330c89fe77 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1460,39 +1460,47 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do end describe 'GET /user/:id/gpg_keys' do - context 'when unauthenticated' do - it 'returns authentication error' do - get api("/users/#{user.id}/gpg_keys") + it 'returns 404 for non-existing user' do + get api('/users/0/gpg_keys') - expect(response).to have_gitlab_http_status(:unauthorized) - end + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 User Not Found') end - context 'when authenticated' do - it 'returns 404 for non-existing user' do - get api('/users/0/gpg_keys', admin) + it 'returns array of GPG keys' do + user.gpg_keys << gpg_key - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 User Not Found') - end + get api("/users/#{user.id}/gpg_keys") - it 'returns 404 error if key not foud' do - delete api("/users/#{user.id}/gpg_keys/#{non_existing_record_id}", admin) + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.first['key']).to eq(gpg_key.key) + end + end - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 GPG Key Not Found') - end + describe 'GET /user/:id/gpg_keys/:key_id' do + it 'returns 404 for non-existing user' do + get api('/users/0/gpg_keys/1') - it 'returns array of GPG keys' do - user.gpg_keys << gpg_key + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 User Not Found') + end - get api("/users/#{user.id}/gpg_keys", admin) + it 'returns 404 for non-existing key' do + get api("/users/#{user.id}/gpg_keys/0") - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.first['key']).to eq(gpg_key.key) - end + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 GPG Key Not Found') + end + + it 'returns a single GPG key' do + user.gpg_keys << gpg_key + + get api("/users/#{user.id}/gpg_keys/#{gpg_key.id}") + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['key']).to eq(gpg_key.key) end end @@ -2308,23 +2316,31 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do end describe 'POST /users/:id/activate' do + subject(:activate) { post api("/users/#{user_id}/activate", api_user) } + + let(:user_id) { user.id } + context 'performed by a non-admin user' do + let(:api_user) { user } + it 'is not authorized to perform the action' do - post api("/users/#{user.id}/activate", user) + activate expect(response).to have_gitlab_http_status(:forbidden) end end context 'performed by an admin user' do + let(:api_user) { admin } + context 'for a deactivated user' do before do user.deactivate - - post api("/users/#{user.id}/activate", admin) end it 'activates a deactivated user' do + activate + expect(response).to have_gitlab_http_status(:created) expect(user.reload.state).to eq('active') end @@ -2333,11 +2349,11 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do context 'for an active user' do before do user.activate - - post api("/users/#{user.id}/activate", admin) end it 'returns 201' do + activate + expect(response).to have_gitlab_http_status(:created) expect(user.reload.state).to eq('active') end @@ -2346,11 +2362,11 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do context 'for a blocked user' do before do user.block - - post api("/users/#{user.id}/activate", admin) end it 'returns 403' 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(user.reload.state).to eq('blocked') @@ -2360,11 +2376,11 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do context 'for a ldap blocked user' do before do user.ldap_block - - post api("/users/#{user.id}/activate", admin) end it 'returns 403' 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(user.reload.state).to eq('ldap_blocked') @@ -2372,8 +2388,10 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do end context 'for a user that does not exist' do + let(:user_id) { 0 } + before do - post api("/users/0/activate", admin) + activate end it_behaves_like '404' @@ -2382,15 +2400,23 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do end describe 'POST /users/:id/deactivate' do + subject(:deactivate) { post api("/users/#{user_id}/deactivate", api_user) } + + let(:user_id) { user.id } + context 'performed by a non-admin user' do + let(:api_user) { user } + it 'is not authorized to perform the action' do - post api("/users/#{user.id}/deactivate", user) + deactivate expect(response).to have_gitlab_http_status(:forbidden) end end context 'performed by an admin user' do + let(:api_user) { admin } + context 'for an active user' do let(:activity) { {} } let(:user) { create(:user, **activity) } @@ -2398,11 +2424,9 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do context 'with no recent activity' do let(:activity) { { last_activity_on: ::User::MINIMUM_INACTIVE_DAYS.next.days.ago } } - before do - post api("/users/#{user.id}/deactivate", admin) - end - it 'deactivates an active user' do + deactivate + expect(response).to have_gitlab_http_status(:created) expect(user.reload.state).to eq('deactivated') end @@ -2411,11 +2435,9 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do context 'with recent activity' do let(:activity) { { last_activity_on: ::User::MINIMUM_INACTIVE_DAYS.pred.days.ago } } - before do - post api("/users/#{user.id}/deactivate", admin) - end - it 'does not deactivate an active user' 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(user.reload.state).to eq('active') @@ -2426,11 +2448,11 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do context 'for a deactivated user' do before do user.deactivate - - post api("/users/#{user.id}/deactivate", admin) end it 'returns 201' do + deactivate + expect(response).to have_gitlab_http_status(:created) expect(user.reload.state).to eq('deactivated') end @@ -2439,11 +2461,11 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do context 'for a blocked user' do before do user.block - - post api("/users/#{user.id}/deactivate", admin) end it 'returns 403' 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(user.reload.state).to eq('blocked') @@ -2453,20 +2475,33 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do context 'for a ldap blocked user' do before do user.ldap_block - - post api("/users/#{user.id}/deactivate", admin) end it 'returns 403' 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(user.reload.state).to eq('ldap_blocked') end end + context 'for an internal user' do + let(:user) { User.alert_bot } + + it 'returns 403' 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') + end + end + context 'for a user that does not exist' do + let(:user_id) { 0 } + before do - post api("/users/0/deactivate", admin) + deactivate end it_behaves_like '404' @@ -2506,6 +2541,15 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do expect(json_response['message']).to eq('404 User Not Found') end + it 'returns a 403 error if user is internal' do + internal_user = create(:user, :bot) + + post api("/users/#{internal_user.id}/block", admin) + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('An internal user cannot be blocked') + end + it 'returns a 201 if user is already blocked' do post api("/users/#{blocked_user.id}/block", admin) |