diff options
Diffstat (limited to 'spec/requests')
59 files changed, 2828 insertions, 756 deletions
diff --git a/spec/requests/admin/background_migrations_controller_spec.rb b/spec/requests/admin/background_migrations_controller_spec.rb new file mode 100644 index 00000000000..c7d5d5cae08 --- /dev/null +++ b/spec/requests/admin/background_migrations_controller_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do + let(:admin) { create(:admin) } + + before do + sign_in(admin) + end + + describe 'POST #retry' do + let(:migration) { create(:batched_background_migration, status: 'failed') } + + before do + create(:batched_background_migration_job, batched_migration: migration, batch_size: 10, min_value: 6, max_value: 15, status: :failed, attempts: 3) + + allow_next_instance_of(Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchingStrategy) do |batch_class| + allow(batch_class).to receive(:next_batch).with(anything, anything, batch_min_value: 6, batch_size: 5).and_return([6, 10]) + end + end + + subject(:retry_migration) { post retry_admin_background_migration_path(migration) } + + it 'redirects the user to the admin migrations page' do + retry_migration + + expect(response).to redirect_to(admin_background_migrations_path) + end + + it 'retries the migration' do + retry_migration + + expect(migration.reload.status).to eql 'active' + end + + context 'when the migration is not failed' do + let(:migration) { create(:batched_background_migration, status: 'paused') } + + it 'keeps the same migration status' do + expect { retry_migration }.not_to change { migration.reload.status } + end + end + end +end diff --git a/spec/requests/api/admin/sidekiq_spec.rb b/spec/requests/api/admin/sidekiq_spec.rb index 3c488816bed..1e626c90e7e 100644 --- a/spec/requests/api/admin/sidekiq_spec.rb +++ b/spec/requests/api/admin/sidekiq_spec.rb @@ -36,7 +36,7 @@ RSpec.describe API::Admin::Sidekiq, :clean_gitlab_redis_queues do add_job(admin, [2]) add_job(create(:user), [3]) - delete api("/admin/sidekiq/queues/authorized_projects?user=#{admin.username}", admin) + delete api("/admin/sidekiq/queues/authorized_projects?user=#{admin.username}&worker_class=AuthorizedProjectsWorker", admin) expect(response).to have_gitlab_http_status(:ok) expect(json_response).to eq('completed' => true, diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index 640e1ee6422..7ae350885f4 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -37,24 +37,10 @@ RSpec.describe API::Ci::Pipelines do end describe 'keys in the response' do - context 'when `pipeline_source_filter` feature flag is disabled' do - before do - stub_feature_flags(pipeline_source_filter: false) - end + it 'includes pipeline source' do + get api("/projects/#{project.id}/pipelines", user) - it 'does not includes pipeline source' do - get api("/projects/#{project.id}/pipelines", user) - - expect(json_response.first.keys).to contain_exactly(*%w[id project_id sha ref status web_url created_at updated_at]) - end - end - - context 'when `pipeline_source_filter` feature flag is disabled' do - it 'includes pipeline source' do - get api("/projects/#{project.id}/pipelines", user) - - expect(json_response.first.keys).to contain_exactly(*%w[id project_id sha ref status web_url created_at updated_at source]) - end + expect(json_response.first.keys).to contain_exactly(*%w[id project_id sha ref status web_url created_at updated_at source]) end end @@ -182,30 +168,6 @@ RSpec.describe API::Ci::Pipelines do end end - context 'when name is specified' do - let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } - - context 'when name exists' do - it 'returns matched pipelines' do - get api("/projects/#{project.id}/pipelines", user), params: { name: user.name } - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response.first['id']).to eq(pipeline.id) - end - end - - context 'when name does not exist' do - it 'returns empty' do - get api("/projects/#{project.id}/pipelines", user), params: { name: 'invalid-name' } - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_empty - end - end - end - context 'when username is specified' do let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } @@ -323,37 +285,20 @@ RSpec.describe API::Ci::Pipelines do create(:ci_pipeline, project: project, source: :api) end - context 'when `pipeline_source_filter` feature flag is disabled' do - before do - stub_feature_flags(pipeline_source_filter: false) - end - - it 'returns all pipelines' do - get api("/projects/#{project.id}/pipelines", user), params: { source: 'web' } + it 'returns matched pipelines' do + get api("/projects/#{project.id}/pipelines", user), params: { source: 'web' } - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).not_to be_empty - expect(json_response.length).to be >= 3 - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).not_to be_empty + json_response.each { |r| expect(r['source']).to eq('web') } end - context 'when `pipeline_source_filter` feature flag is enabled' do - it 'returns matched pipelines' do - get api("/projects/#{project.id}/pipelines", user), params: { source: 'web' } - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).not_to be_empty - json_response.each { |r| expect(r['source']).to eq('web') } - end - - context 'when source is invalid' do - it 'returns bad_request' do - get api("/projects/#{project.id}/pipelines", user), params: { source: 'invalid-source' } + context 'when source is invalid' do + it 'returns bad_request' do + get api("/projects/#{project.id}/pipelines", user), params: { source: 'invalid-source' } - expect(response).to have_gitlab_http_status(:bad_request) - end + expect(response).to have_gitlab_http_status(:bad_request) end end end diff --git a/spec/requests/api/ci/runners_reset_registration_token_spec.rb b/spec/requests/api/ci/runners_reset_registration_token_spec.rb new file mode 100644 index 00000000000..7623d3f1b17 --- /dev/null +++ b/spec/requests/api/ci/runners_reset_registration_token_spec.rb @@ -0,0 +1,149 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Ci::Runners do + subject { post api("#{prefix}/runners/reset_registration_token", user) } + + shared_examples 'bad request' do |result| + it 'returns 400 error' do + expect { subject }.not_to change { get_token } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to eq(result) + end + end + + shared_examples 'unauthenticated' do + it 'returns 401 error' do + expect { subject }.not_to change { get_token } + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + shared_examples 'unauthorized' do + it 'returns 403 error' do + expect { subject }.not_to change { get_token } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + shared_examples 'not found' do |scope| + it 'returns 404 error' do + expect { subject }.not_to change { get_token } + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response).to eq({ 'message' => "404 #{scope.capitalize} Not Found" }) + end + end + + shared_context 'when unauthorized' do |scope| + context 'when unauthorized' do + let_it_be(:user) { create(:user) } + + context "when not a #{scope} member" do + it_behaves_like 'not found', scope + end + + context "with a non-admin #{scope} member" do + before do + target.add_developer(user) + end + + it_behaves_like 'unauthorized' + end + end + end + + shared_context 'when authorized' do |scope| + it 'resets runner registration token' do + expect { subject }.to change { get_token } + + expect(response).to have_gitlab_http_status(:success) + expect(json_response).to eq({ 'token' => get_token }) + end + + if scope != 'instance' + context 'when malformed id is provided' do + let(:prefix) { "/#{scope.pluralize}/some%20string" } + + it_behaves_like 'not found', scope + end + end + end + + describe '/api/v4/runners/reset_registration_token' do + describe 'POST /api/v4/runners/reset_registration_token' do + before do + ApplicationSetting.create_from_defaults + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + end + + let(:prefix) { '' } + + context 'when unauthenticated' do + let(:user) { nil } + + it_behaves_like 'unauthenticated' + end + + context 'when unauthorized' do + let(:user) { create(:user) } + + context "with a non-admin instance member" do + it_behaves_like 'unauthorized' + end + end + + include_context 'when authorized', 'instance' do + let_it_be(:user) { create(:user, :admin) } + + def get_token + ApplicationSetting.current_without_cache.runners_registration_token + end + end + end + end + + describe '/api/v4/groups/:id/runners/reset_registration_token' do + describe 'POST /api/v4/groups/:id/runners/reset_registration_token' do + let_it_be(:group) { create_default(:group, :private) } + + let(:prefix) { "/groups/#{group.id}" } + + include_context 'when unauthorized', 'group' do + let(:target) { group } + end + + include_context 'when authorized', 'group' do + let_it_be(:user) { create_default(:group_member, :maintainer, user: create(:user), group: group ).user } + + def get_token + group.reload.runners_token + end + end + end + end + + describe '/api/v4/projects/:id/runners/reset_registration_token' do + describe 'POST /api/v4/projects/:id/runners/reset_registration_token' do + let_it_be(:project) { create_default(:project) } + + let(:prefix) { "/projects/#{project.id}" } + + include_context 'when unauthorized', 'project' do + let(:target) { project } + end + + include_context 'when authorized', 'project' do + let_it_be(:user) { project.owner } + + def get_token + project.reload.runners_token + end + end + end + end +end diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index ccc9f8c50c4..47bc3eb74a6 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -345,38 +345,12 @@ RSpec.describe API::CommitStatuses do expect(json_response['status']).to eq('success') end - context 'feature flags' do - using RSpec::Parameterized::TableSyntax - - where(:ci_fix_commit_status_retried, :ci_remove_update_retried_from_process_pipeline, :previous_statuses_retried) do - true | true | true - true | false | true - false | true | false - false | false | true - end - - with_them do - before do - stub_feature_flags( - ci_fix_commit_status_retried: ci_fix_commit_status_retried, - ci_remove_update_retried_from_process_pipeline: ci_remove_update_retried_from_process_pipeline - ) - end - - it 'retries a commit status', :sidekiq_might_not_need_inline do - post_request - - expect(CommitStatus.count).to eq 2 + it 'retries the commit status', :sidekiq_might_not_need_inline do + post_request - if previous_statuses_retried - expect(CommitStatus.first).to be_retried - expect(CommitStatus.last.pipeline).to be_success - else - expect(CommitStatus.first).not_to be_retried - expect(CommitStatus.last.pipeline).to be_failed - end - end - end + expect(CommitStatus.count).to eq 2 + expect(CommitStatus.first).to be_retried + expect(CommitStatus.last.pipeline).to be_success end end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 1162ae76d15..1d76c281dee 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -1879,6 +1879,26 @@ RSpec.describe API::Commits do expect(json_response['line_type']).to eq('new') end + it 'correctly adds a note for the "old" line type' do + commit = project.repository.commit("markdown") + commit_id = commit.id + route = "/projects/#{project_id}/repository/commits/#{commit_id}/comments" + + post api(route, current_user), params: { + note: 'My comment', + path: commit.raw_diffs.first.old_path, + line: 4, + line_type: 'old' + } + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/commit_note') + expect(json_response['note']).to eq('My comment') + expect(json_response['path']).to eq(commit.raw_diffs.first.old_path) + expect(json_response['line']).to eq(4) + expect(json_response['line_type']).to eq('old') + end + context 'when ref does not exist' do let(:commit_id) { 'unknown' } diff --git a/spec/requests/api/dependency_proxy_spec.rb b/spec/requests/api/dependency_proxy_spec.rb index d59f2bf06e3..2837d1c02c4 100644 --- a/spec/requests/api/dependency_proxy_spec.rb +++ b/spec/requests/api/dependency_proxy_spec.rb @@ -13,60 +13,74 @@ RSpec.describe API::DependencyProxy, api: true do group.add_owner(user) stub_config(dependency_proxy: { enabled: true }) stub_last_activity_update - group.create_dependency_proxy_setting!(enabled: true) end describe 'DELETE /groups/:id/dependency_proxy/cache' do - subject { delete api("/groups/#{group.id}/dependency_proxy/cache", user) } + subject { delete api("/groups/#{group_id}/dependency_proxy/cache", user) } - context 'with feature available and enabled' do - let_it_be(:lease_key) { "dependency_proxy:delete_group_blobs:#{group.id}" } + shared_examples 'responding to purge requests' do + context 'with feature available and enabled' do + let_it_be(:lease_key) { "dependency_proxy:delete_group_blobs:#{group.id}" } - context 'an admin user' do - it 'deletes the blobs and returns no content' do - stub_exclusive_lease(lease_key, timeout: 1.hour) - expect(PurgeDependencyProxyCacheWorker).to receive(:perform_async) + context 'an admin user' do + it 'deletes the blobs and returns no content' do + stub_exclusive_lease(lease_key, timeout: 1.hour) + expect(PurgeDependencyProxyCacheWorker).to receive(:perform_async) - subject + subject - expect(response).to have_gitlab_http_status(:no_content) - end + expect(response).to have_gitlab_http_status(:accepted) + expect(response.body).to eq('202') + end - context 'called multiple times in one hour', :clean_gitlab_redis_shared_state do - it 'returns 409 with an error message' do - stub_exclusive_lease_taken(lease_key, timeout: 1.hour) + context 'called multiple times in one hour', :clean_gitlab_redis_shared_state do + it 'returns 409 with an error message' do + stub_exclusive_lease_taken(lease_key, timeout: 1.hour) - subject + subject - expect(response).to have_gitlab_http_status(:conflict) - expect(response.body).to include('This request has already been made.') + expect(response).to have_gitlab_http_status(:conflict) + expect(response.body).to include('This request has already been made.') + end + + it 'executes service only for the first time' do + expect(PurgeDependencyProxyCacheWorker).to receive(:perform_async).once + + 2.times { subject } + end end + end - it 'executes service only for the first time' do - expect(PurgeDependencyProxyCacheWorker).to receive(:perform_async).once + context 'a non-admin' do + let(:user) { create(:user) } - 2.times { subject } + before do + group.add_maintainer(user) end + + it_behaves_like 'returning response status', :forbidden end end - context 'a non-admin' do - let(:user) { create(:user) } - + context 'depencency proxy is not enabled in the config' do before do - group.add_maintainer(user) + stub_config(dependency_proxy: { enabled: false }) end - it_behaves_like 'returning response status', :forbidden + it_behaves_like 'returning response status', :not_found end end - context 'depencency proxy is not enabled' do - before do - stub_config(dependency_proxy: { enabled: false }) - end + context 'with a group id' do + let(:group_id) { group.id } + + it_behaves_like 'responding to purge requests' + end + + context 'with an url encoded group id' do + let(:group_id) { ERB::Util.url_encode(group.full_path) } - it_behaves_like 'returning response status', :not_found + it_behaves_like 'responding to purge requests' end end end diff --git a/spec/requests/api/error_tracking_client_keys_spec.rb b/spec/requests/api/error_tracking_client_keys_spec.rb new file mode 100644 index 00000000000..886ec5ade3d --- /dev/null +++ b/spec/requests/api/error_tracking_client_keys_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::ErrorTrackingClientKeys do + let_it_be(:guest) { create(:user) } + let_it_be(:maintainer) { create(:user) } + let_it_be(:setting) { create(:project_error_tracking_setting) } + let_it_be(:project) { setting.project } + + let!(:client_key) { create(:error_tracking_client_key, project: project) } + + before do + project.add_guest(guest) + project.add_maintainer(maintainer) + end + + shared_examples 'endpoint with authorization' do + context 'when unauthenticated' do + let(:user) { nil } + + it { expect(response).to have_gitlab_http_status(:unauthorized) } + end + + context 'when authenticated as non-maintainer' do + let(:user) { guest } + + it { expect(response).to have_gitlab_http_status(:forbidden) } + end + end + + describe "GET /projects/:id/error_tracking/client_keys" do + before do + get api("/projects/#{project.id}/error_tracking/client_keys", user) + end + + it_behaves_like 'endpoint with authorization' + + context 'when authenticated as maintainer' do + let(:user) { maintainer } + + it 'returns client keys' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.size).to eq(1) + expect(json_response.first['id']).to eq(client_key.id) + end + end + end + + describe "POST /projects/:id/error_tracking/client_keys" do + before do + post api("/projects/#{project.id}/error_tracking/client_keys", user) + end + + it_behaves_like 'endpoint with authorization' + + context 'when authenticated as maintainer' do + let(:user) { maintainer } + + it 'returns a newly created client key' do + new_key = project.error_tracking_client_keys.last + + expect(json_response['id']).to eq(new_key.id) + expect(json_response['public_key']).to eq(new_key.public_key) + expect(json_response['sentry_dsn']).to eq(new_key.sentry_dsn) + end + end + end + + describe "DELETE /projects/:id/error_tracking/client_keys/:key_id" do + before do + delete api("/projects/#{project.id}/error_tracking/client_keys/#{client_key.id}", user) + end + + it_behaves_like 'endpoint with authorization' + + context 'when authenticated as maintainer' do + let(:user) { maintainer } + + it 'returns a correct status' do + expect(response).to have_gitlab_http_status(:ok) + end + end + end +end diff --git a/spec/requests/api/error_tracking_collector_spec.rb b/spec/requests/api/error_tracking_collector_spec.rb index 4b186657c4a..35d3ea01f87 100644 --- a/spec/requests/api/error_tracking_collector_spec.rb +++ b/spec/requests/api/error_tracking_collector_spec.rb @@ -7,6 +7,30 @@ RSpec.describe API::ErrorTrackingCollector do let_it_be(:setting) { create(:project_error_tracking_setting, :integrated, project: project) } let_it_be(:client_key) { create(:error_tracking_client_key, project: project) } + RSpec.shared_examples 'not found' do + it 'reponds with 404' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + RSpec.shared_examples 'bad request' do + it 'responds with 400' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + RSpec.shared_examples 'successful request' do + it 'writes to the database and returns no content' do + expect { subject }.to change { ErrorTracking::ErrorEvent.count }.by(1) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + describe "POST /error_tracking/collector/api/:id/envelope" do let_it_be(:raw_event) { fixture_file('error_tracking/event.txt') } let_it_be(:url) { "/error_tracking/collector/api/#{project.id}/envelope" } @@ -16,22 +40,6 @@ RSpec.describe API::ErrorTrackingCollector do subject { post api(url), params: params, headers: headers } - RSpec.shared_examples 'not found' do - it 'reponds with 404' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - RSpec.shared_examples 'bad request' do - it 'responds with 400' do - subject - - expect(response).to have_gitlab_http_status(:bad_request) - end - end - context 'error tracking feature is disabled' do before do setting.update!(enabled: false) @@ -48,14 +56,6 @@ RSpec.describe API::ErrorTrackingCollector do it_behaves_like 'not found' end - context 'feature flag is disabled' do - before do - stub_feature_flags(integrated_error_tracking: false) - end - - it_behaves_like 'not found' - end - context 'auth headers are missing' do let(:headers) { {} } @@ -96,10 +96,53 @@ RSpec.describe API::ErrorTrackingCollector do end end - it 'writes to the database and returns no content' do - expect { subject }.to change { ErrorTracking::ErrorEvent.count }.by(1) + it_behaves_like 'successful request' + end - expect(response).to have_gitlab_http_status(:no_content) + describe "POST /error_tracking/collector/api/:id/store" do + let_it_be(:raw_event) { fixture_file('error_tracking/parsed_event.json') } + let_it_be(:url) { "/error_tracking/collector/api/#{project.id}/store" } + + let(:params) { raw_event } + let(:headers) { { 'X-Sentry-Auth' => "Sentry sentry_key=#{client_key.public_key}" } } + + subject { post api(url), params: params, headers: headers } + + it_behaves_like 'successful request' + + context 'empty headers' do + let(:headers) { {} } + + it_behaves_like 'bad request' + end + + context 'empty body' do + let(:params) { '' } + + it_behaves_like 'bad request' + end + + context 'sentry_key as param and empty headers' do + let(:url) { "/error_tracking/collector/api/#{project.id}/store?sentry_key=#{sentry_key}" } + let(:headers) { {} } + + context 'key is wrong' do + let(:sentry_key) { 'glet_1fedb514e17f4b958435093deb02048c' } + + it_behaves_like 'not found' + end + + context 'key is empty' do + let(:sentry_key) { '' } + + it_behaves_like 'bad request' + end + + context 'key is correct' do + let(:sentry_key) { client_key.public_key } + + it_behaves_like 'successful request' + end end end end diff --git a/spec/requests/api/feature_flags_spec.rb b/spec/requests/api/feature_flags_spec.rb index 8c8c6803a38..a1aedc1d6b2 100644 --- a/spec/requests/api/feature_flags_spec.rb +++ b/spec/requests/api/feature_flags_spec.rb @@ -116,21 +116,6 @@ RSpec.describe API::FeatureFlags do }]) 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 - end end describe 'GET /projects/:id/feature_flags/:name' do @@ -185,22 +170,13 @@ RSpec.describe API::FeatureFlags do 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] + name: 'awesome-feature' } end @@ -215,14 +191,14 @@ RSpec.describe API::FeatureFlags do expect(feature_flag.description).to eq(params[:description]) end - it 'defaults to a version 1 (legacy) feature flag' do + it 'defaults to a version 2 (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.version).to eq('legacy_flag') + expect(feature_flag.version).to eq('new_version_flag') end it_behaves_like 'check user permission' @@ -232,38 +208,7 @@ RSpec.describe API::FeatureFlags do 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 - - 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 + expect(json_response['version']).to eq('new_version_flag') end context 'when there is a feature flag with the same name already' do @@ -278,43 +223,6 @@ RSpec.describe API::FeatureFlags do 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 = { @@ -455,23 +363,6 @@ RSpec.describe API::FeatureFlags do 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' do - 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(json_response).to eq({ 'message' => '404 Not Found' }) - 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, @@ -781,7 +672,7 @@ RSpec.describe API::FeatureFlags do params: params end - let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, project: project) } + let!(:feature_flag) { create(:operations_feature_flag, project: project) } let(:params) { {} } it 'destroys the feature flag' do @@ -794,7 +685,7 @@ RSpec.describe API::FeatureFlags do subject expect(response).to have_gitlab_http_status(:ok) - expect(json_response['version']).to eq('legacy_flag') + expect(json_response['version']).to eq('new_version_flag') end context 'with a version 2 feature flag' do diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 869df06b60c..0b898496dd6 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -95,6 +95,19 @@ RSpec.describe API::Files do expect(response.headers['X-Gitlab-Content-Sha256']).to eq('c440cd09bae50c4632cc58638ad33c6aa375b6109d811e76a9cc3a613c1e8887') end + it 'caches sha256 of the content', :use_clean_rails_redis_caching do + head api(route(file_path), current_user, **options), params: params + + expect(Rails.cache.fetch("blob_content_sha256:#{project.full_path}:#{response.headers['X-Gitlab-Blob-Id']}")) + .to eq('c440cd09bae50c4632cc58638ad33c6aa375b6109d811e76a9cc3a613c1e8887') + + expect_next_instance_of(Gitlab::Git::Blob) do |instance| + expect(instance).not_to receive(:load_all_data!) + end + + head api(route(file_path), current_user, **options), params: params + end + it 'returns file by commit sha' do # This file is deleted on HEAD file_path = "files%2Fjs%2Fcommit%2Ejs%2Ecoffee" diff --git a/spec/requests/api/generic_packages_spec.rb b/spec/requests/api/generic_packages_spec.rb index 4091253fb54..7e439a22e4b 100644 --- a/spec/requests/api/generic_packages_spec.rb +++ b/spec/requests/api/generic_packages_spec.rb @@ -18,7 +18,7 @@ RSpec.describe API::GenericPackages do let_it_be(:project_deploy_token_wo) { create(:project_deploy_token, deploy_token: deploy_token_wo, project: project) } let(:user) { personal_access_token.user } - let(:ci_build) { create(:ci_build, :running, user: user) } + let(:ci_build) { create(:ci_build, :running, user: user, project: project) } let(:snowplow_standard_context_params) { { user: user, project: project, namespace: project.namespace } } def auth_header @@ -388,9 +388,11 @@ RSpec.describe API::GenericPackages do end context 'event tracking' do + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: user } } + subject { upload_file(params, workhorse_headers.merge(personal_access_token_header)) } - 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 'rejects request without a file from workhorse' do @@ -542,13 +544,15 @@ RSpec.describe API::GenericPackages do end context 'event tracking' do + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: user } } + 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' + it_behaves_like 'a package tracking event', described_class.name, 'pull_package' end it 'rejects a malicious file name request' do diff --git a/spec/requests/api/go_proxy_spec.rb b/spec/requests/api/go_proxy_spec.rb index e678b6cf1c8..0143340de11 100644 --- a/spec/requests/api/go_proxy_spec.rb +++ b/spec/requests/api/go_proxy_spec.rb @@ -11,7 +11,7 @@ RSpec.describe API::GoProxy do let_it_be(:base) { "#{Settings.build_gitlab_go_url}/#{project.full_path}" } let_it_be(:oauth) { create :oauth_access_token, scopes: 'api', resource_owner: user } - let_it_be(:job) { create :ci_build, user: user, status: :running } + let_it_be(:job) { create :ci_build, user: user, status: :running, project: project } let_it_be(:pa_token) { create :personal_access_token, user: user } let_it_be(:modules) do diff --git a/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb b/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb index 3628171fcc1..008241b8055 100644 --- a/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb +++ b/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb @@ -48,13 +48,18 @@ RSpec.describe 'get board lists' do issues_data.map { |i| i['title'] } end + def issue_relative_positions + issues_data.map { |i| i['relativePosition'] } + end + shared_examples 'group and project board list issues query' do let!(:board) { create(:board, resource_parent: board_parent) } let!(:label_list) { create(:list, board: board, label: label, position: 10) } let!(:issue1) { create(:issue, project: issue_project, labels: [label, label2], relative_position: 9) } let!(:issue2) { create(:issue, project: issue_project, labels: [label, label2], relative_position: 2) } - let!(:issue3) { create(:issue, project: issue_project, labels: [label], relative_position: 9) } - let!(:issue4) { create(:issue, project: issue_project, labels: [label2], relative_position: 432) } + let!(:issue3) { create(:issue, project: issue_project, labels: [label, label2], relative_position: nil) } + let!(:issue4) { create(:issue, project: issue_project, labels: [label], relative_position: 9) } + let!(:issue5) { create(:issue, project: issue_project, labels: [label2], relative_position: 432) } context 'when the user does not have access to the board' do it 'returns nil' do @@ -69,10 +74,11 @@ RSpec.describe 'get board lists' do board_parent.add_reporter(user) end - it 'can access the issues' do + it 'can access the issues', :aggregate_failures do post_graphql(query("id: \"#{global_id_of(label_list)}\""), current_user: user) - expect(issue_titles).to eq([issue2.title, issue1.title]) + expect(issue_titles).to eq([issue2.title, issue1.title, issue3.title]) + expect(issue_relative_positions).not_to include(nil) end end end diff --git a/spec/requests/api/graphql/ci/stages_spec.rb b/spec/requests/api/graphql/ci/stages_spec.rb index cd48a24b9c8..50d2cf75097 100644 --- a/spec/requests/api/graphql/ci/stages_spec.rb +++ b/spec/requests/api/graphql/ci/stages_spec.rb @@ -4,11 +4,13 @@ require 'spec_helper' RSpec.describe 'Query.project.pipeline.stages' do include GraphqlHelpers - let(:project) { create(:project, :repository, :public) } - let(:user) { create(:user) } - let(:pipeline) { create(:ci_pipeline, project: project, user: user) } - let(:stage_graphql_data) { graphql_data['project']['pipeline']['stages'] } + subject(:post_query) { post_graphql(query, current_user: user) } + let_it_be(:project) { create(:project, :repository, :public) } + let_it_be(:user) { create(:user) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } + + let(:stage_nodes) { graphql_data_at(:project, :pipeline, :stages, :nodes) } let(:params) { {} } let(:fields) do @@ -33,14 +35,42 @@ RSpec.describe 'Query.project.pipeline.stages' do ) end - before do + before_all do create(:ci_stage_entity, pipeline: pipeline, name: 'deploy') - post_graphql(query, current_user: user) + create_list(:ci_build, 2, pipeline: pipeline, stage: 'deploy') end - it_behaves_like 'a working graphql query' + it_behaves_like 'a working graphql query' do + before do + post_query + end + end it 'returns the stage of a pipeline' do - expect(stage_graphql_data['nodes'].first['name']).to eq('deploy') + post_query + + expect(stage_nodes.first['name']).to eq('deploy') + end + + describe 'job pagination' do + let(:job_nodes) { graphql_dig_at(stage_nodes, :jobs, :nodes) } + + it 'returns up to default limit jobs per stage' do + post_query + + expect(job_nodes.count).to eq(2) + end + + context 'when the limit is manually set' do + before do + stub_application_setting(jobs_per_stage_page_size: 1) + end + + it 'returns up to custom limit jobs per stage' do + post_query + + expect(job_nodes.count).to eq(1) + end + end end end diff --git a/spec/requests/api/graphql/current_user/groups_query_spec.rb b/spec/requests/api/graphql/current_user/groups_query_spec.rb new file mode 100644 index 00000000000..39f323b21a3 --- /dev/null +++ b/spec/requests/api/graphql/current_user/groups_query_spec.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Query current user groups' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:guest_group) { create(:group, name: 'public guest', path: 'public-guest') } + let_it_be(:private_maintainer_group) { create(:group, :private, name: 'b private maintainer', path: 'b-private-maintainer') } + let_it_be(:public_developer_group) { create(:group, :private, project_creation_level: nil, name: 'c public developer', path: 'c-public-developer') } + let_it_be(:public_maintainer_group) { create(:group, :private, name: 'a public maintainer', path: 'a-public-maintainer') } + + let(:group_arguments) { {} } + let(:current_user) { user } + + let(:fields) do + <<~GRAPHQL + nodes { id path fullPath name } + GRAPHQL + end + + let(:query) do + graphql_query_for('currentUser', {}, query_graphql_field('groups', group_arguments, fields)) + end + + before_all do + guest_group.add_guest(user) + private_maintainer_group.add_maintainer(user) + public_developer_group.add_developer(user) + public_maintainer_group.add_maintainer(user) + end + + subject { graphql_data.dig('currentUser', 'groups', 'nodes') } + + before do + post_graphql(query, current_user: current_user) + end + + it_behaves_like 'a working graphql query' + + it 'avoids N+1 queries', :request_store do + control = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } + + new_group = create(:group, :private) + new_group.add_maintainer(current_user) + + expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control) + end + + it 'returns all groups where the user is a direct member' do + is_expected.to match( + expected_group_hash( + public_maintainer_group, + private_maintainer_group, + public_developer_group, + guest_group + ) + ) + end + + context 'when permission_scope is CREATE_PROJECTS' do + let(:group_arguments) { { permission_scope: :CREATE_PROJECTS } } + + specify do + is_expected.to match( + expected_group_hash( + public_maintainer_group, + private_maintainer_group, + public_developer_group + ) + ) + end + + context 'when search is provided' do + let(:group_arguments) { { permission_scope: :CREATE_PROJECTS, search: 'maintainer' } } + + specify do + is_expected.to match( + expected_group_hash( + public_maintainer_group, + private_maintainer_group + ) + ) + end + end + end + + context 'when search is provided' do + let(:group_arguments) { { search: 'maintainer' } } + + specify do + is_expected.to match( + expected_group_hash( + public_maintainer_group, + private_maintainer_group + ) + ) + end + end + + def expected_group_hash(*groups) + groups.map do |group| + { + 'id' => group.to_global_id.to_s, + 'name' => group.name, + 'path' => group.path, + 'fullPath' => group.full_path + } + end + end +end diff --git a/spec/requests/api/graphql/group/dependency_proxy_blobs_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_blobs_spec.rb new file mode 100644 index 00000000000..cdb21512894 --- /dev/null +++ b/spec/requests/api/graphql/group/dependency_proxy_blobs_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe 'getting dependency proxy blobs in a group' do + using RSpec::Parameterized::TableSyntax + include GraphqlHelpers + + let_it_be(:owner) { create(:user) } + let_it_be_with_reload(:group) { create(:group) } + let_it_be(:blob) { create(:dependency_proxy_blob, group: group) } + let_it_be(:blob2) { create(:dependency_proxy_blob, file_name: 'blob2.json', group: group) } + let_it_be(:blobs) { [blob, blob2].flatten } + + let(:dependency_proxy_blob_fields) do + <<~GQL + edges { + node { + #{all_graphql_fields_for('dependency_proxy_blobs'.classify, max_depth: 1)} + } + } + GQL + end + + let(:fields) do + <<~GQL + #{query_graphql_field('dependency_proxy_blobs', {}, dependency_proxy_blob_fields)} + dependencyProxyBlobCount + dependencyProxyTotalSize + GQL + end + + let(:query) do + graphql_query_for( + 'group', + { 'fullPath' => group.full_path }, + fields + ) + end + + let(:user) { owner } + let(:variables) { {} } + let(:dependency_proxy_blobs_response) { graphql_data.dig('group', 'dependencyProxyBlobs', 'edges') } + let(:dependency_proxy_blob_count_response) { graphql_data.dig('group', 'dependencyProxyBlobCount') } + let(:dependency_proxy_total_size_response) { graphql_data.dig('group', 'dependencyProxyTotalSize') } + + before do + stub_config(dependency_proxy: { enabled: true }) + group.add_owner(owner) + end + + subject { post_graphql(query, current_user: user, variables: variables) } + + it_behaves_like 'a working graphql query' do + before do + subject + end + end + + context 'with different permissions' do + let_it_be(:user) { create(:user) } + + where(:group_visibility, :role, :access_granted) do + :private | :maintainer | true + :private | :developer | true + :private | :reporter | true + :private | :guest | true + :private | :anonymous | false + :public | :maintainer | true + :public | :developer | true + :public | :reporter | true + :public | :guest | true + :public | :anonymous | false + end + + with_them do + before do + group.update_column(:visibility_level, Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) + group.add_user(user, role) unless role == :anonymous + end + + it 'return the proper response' do + subject + + if access_granted + expect(dependency_proxy_blobs_response.size).to eq(blobs.size) + else + expect(dependency_proxy_blobs_response).to be_blank + end + end + end + end + + context 'limiting the number of blobs' do + let(:limit) { 1 } + let(:variables) do + { path: group.full_path, n: limit } + end + + let(:query) do + <<~GQL + query($path: ID!, $n: Int) { + group(fullPath: $path) { + dependencyProxyBlobs(first: $n) { #{dependency_proxy_blob_fields} } + } + } + GQL + end + + it 'only returns N blobs' do + subject + + expect(dependency_proxy_blobs_response.size).to eq(limit) + end + end + + it 'returns the total count of blobs' do + subject + + expect(dependency_proxy_blob_count_response).to eq(blobs.size) + end + + it 'returns the total size' do + subject + expected_size = blobs.inject(0) { |sum, blob| sum + blob.size } + expect(dependency_proxy_total_size_response).to eq(ActiveSupport::NumberHelper.number_to_human_size(expected_size)) + end +end diff --git a/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb new file mode 100644 index 00000000000..c5c6d85d1e6 --- /dev/null +++ b/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe 'getting dependency proxy settings for a group' do + using RSpec::Parameterized::TableSyntax + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be_with_reload(:group) { create(:group) } + + let(:dependency_proxy_group_setting_fields) do + <<~GQL + #{all_graphql_fields_for('dependency_proxy_setting'.classify, max_depth: 1)} + GQL + end + + let(:fields) do + <<~GQL + #{query_graphql_field('dependency_proxy_setting', {}, dependency_proxy_group_setting_fields)} + GQL + end + + let(:query) do + graphql_query_for( + 'group', + { 'fullPath' => group.full_path }, + fields + ) + end + + let(:variables) { {} } + let(:dependency_proxy_group_setting_response) { graphql_data.dig('group', 'dependencyProxySetting') } + + before do + stub_config(dependency_proxy: { enabled: true }) + group.create_dependency_proxy_setting!(enabled: true) + end + + subject { post_graphql(query, current_user: user, variables: variables) } + + it_behaves_like 'a working graphql query' do + before do + subject + end + end + + context 'with different permissions' do + where(:group_visibility, :role, :access_granted) do + :private | :maintainer | true + :private | :developer | true + :private | :reporter | true + :private | :guest | true + :private | :anonymous | false + :public | :maintainer | true + :public | :developer | true + :public | :reporter | true + :public | :guest | true + :public | :anonymous | false + end + + with_them do + before do + group.update_column(:visibility_level, Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) + group.add_user(user, role) unless role == :anonymous + end + + it 'return the proper response' do + subject + + if access_granted + expect(dependency_proxy_group_setting_response).to eq('enabled' => true) + else + expect(dependency_proxy_group_setting_response).to be_blank + end + end + end + end +end diff --git a/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb new file mode 100644 index 00000000000..c8797d84906 --- /dev/null +++ b/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe 'getting dependency proxy image ttl policy for a group' do + using RSpec::Parameterized::TableSyntax + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be_with_reload(:group) { create(:group) } + + let(:dependency_proxy_image_ttl_policy_fields) do + <<~GQL + #{all_graphql_fields_for('dependency_proxy_image_ttl_group_policy'.classify, max_depth: 1)} + GQL + end + + let(:fields) do + <<~GQL + #{query_graphql_field('dependency_proxy_image_ttl_policy', {}, dependency_proxy_image_ttl_policy_fields)} + GQL + end + + let(:query) do + graphql_query_for( + 'group', + { 'fullPath' => group.full_path }, + fields + ) + end + + let(:variables) { {} } + let(:dependency_proxy_image_ttl_policy_response) { graphql_data.dig('group', 'dependencyProxyImageTtlPolicy') } + + before do + stub_config(dependency_proxy: { enabled: true }) + end + + subject { post_graphql(query, current_user: user, variables: variables) } + + it_behaves_like 'a working graphql query' do + before do + subject + end + end + + context 'with different permissions' do + where(:group_visibility, :role, :access_granted) do + :private | :maintainer | true + :private | :developer | true + :private | :reporter | true + :private | :guest | true + :private | :anonymous | false + :public | :maintainer | true + :public | :developer | true + :public | :reporter | true + :public | :guest | true + :public | :anonymous | false + end + + with_them do + before do + group.update_column(:visibility_level, Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) + group.add_user(user, role) unless role == :anonymous + end + + it 'return the proper response' do + subject + + if access_granted + expect(dependency_proxy_image_ttl_policy_response).to eq("createdAt" => nil, "enabled" => false, "ttl" => 90, "updatedAt" => nil) + else + expect(dependency_proxy_image_ttl_policy_response).to be_blank + end + end + end + end +end diff --git a/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb new file mode 100644 index 00000000000..30e704adb92 --- /dev/null +++ b/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe 'getting dependency proxy manifests in a group' do + using RSpec::Parameterized::TableSyntax + include GraphqlHelpers + + let_it_be(:owner) { create(:user) } + let_it_be_with_reload(:group) { create(:group) } + let_it_be(:manifest) { create(:dependency_proxy_manifest, group: group) } + let_it_be(:manifest2) { create(:dependency_proxy_manifest, file_name: 'image2.json', group: group) } + let_it_be(:manifests) { [manifest, manifest2].flatten } + + let(:dependency_proxy_manifest_fields) do + <<~GQL + edges { + node { + #{all_graphql_fields_for('dependency_proxy_manifests'.classify, max_depth: 1)} + } + } + GQL + end + + let(:fields) do + <<~GQL + #{query_graphql_field('dependency_proxy_manifests', {}, dependency_proxy_manifest_fields)} + dependencyProxyImageCount + GQL + end + + let(:query) do + graphql_query_for( + 'group', + { 'fullPath' => group.full_path }, + fields + ) + end + + let(:user) { owner } + let(:variables) { {} } + let(:dependency_proxy_manifests_response) { graphql_data.dig('group', 'dependencyProxyManifests', 'edges') } + let(:dependency_proxy_image_count_response) { graphql_data.dig('group', 'dependencyProxyImageCount') } + + before do + stub_config(dependency_proxy: { enabled: true }) + group.add_owner(owner) + end + + subject { post_graphql(query, current_user: user, variables: variables) } + + it_behaves_like 'a working graphql query' do + before do + subject + end + end + + context 'with different permissions' do + let_it_be(:user) { create(:user) } + + where(:group_visibility, :role, :access_granted) do + :private | :maintainer | true + :private | :developer | true + :private | :reporter | true + :private | :guest | true + :private | :anonymous | false + :public | :maintainer | true + :public | :developer | true + :public | :reporter | true + :public | :guest | true + :public | :anonymous | false + end + + with_them do + before do + group.update_column(:visibility_level, Gitlab::VisibilityLevel.const_get(group_visibility.to_s.upcase, false)) + group.add_user(user, role) unless role == :anonymous + end + + it 'return the proper response' do + subject + + if access_granted + expect(dependency_proxy_manifests_response.size).to eq(manifests.size) + else + expect(dependency_proxy_manifests_response).to be_blank + end + end + end + end + + context 'limiting the number of manifests' do + let(:limit) { 1 } + let(:variables) do + { path: group.full_path, n: limit } + end + + let(:query) do + <<~GQL + query($path: ID!, $n: Int) { + group(fullPath: $path) { + dependencyProxyManifests(first: $n) { #{dependency_proxy_manifest_fields} } + } + } + GQL + end + + it 'only returns N manifests' do + subject + + expect(dependency_proxy_manifests_response.size).to eq(limit) + end + end + + it 'returns the total count of manifests' do + subject + + expect(dependency_proxy_image_count_response).to eq(manifests.size) + end +end diff --git a/spec/requests/api/graphql/mutations/admin/sidekiq_queues/delete_jobs_spec.rb b/spec/requests/api/graphql/mutations/admin/sidekiq_queues/delete_jobs_spec.rb index 1692cfbcf84..f992e46879f 100644 --- a/spec/requests/api/graphql/mutations/admin/sidekiq_queues/delete_jobs_spec.rb +++ b/spec/requests/api/graphql/mutations/admin/sidekiq_queues/delete_jobs_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'Deleting Sidekiq jobs', :clean_gitlab_redis_queues do let(:queue) { 'authorized_projects' } - let(:variables) { { user: admin.username, queue_name: queue } } + let(:variables) { { user: admin.username, worker_class: 'AuthorizedProjectsWorker', queue_name: queue } } let(:mutation) { graphql_mutation(:admin_sidekiq_queues_delete_jobs, variables) } def mutation_response diff --git a/spec/requests/api/graphql/mutations/custom_emoji/destroy_spec.rb b/spec/requests/api/graphql/mutations/custom_emoji/destroy_spec.rb new file mode 100644 index 00000000000..07fd57a2cee --- /dev/null +++ b/spec/requests/api/graphql/mutations/custom_emoji/destroy_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Deletion of custom emoji' do + include GraphqlHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:current_user) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be_with_reload(:custom_emoji) { create(:custom_emoji, group: group, creator: user2) } + + let(:mutation) do + variables = { + id: GitlabSchema.id_from_object(custom_emoji).to_s + } + + graphql_mutation(:destroy_custom_emoji, variables) + end + + shared_examples 'does not delete custom emoji' do + it 'does not change count' do + expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(CustomEmoji, :count) + end + end + + shared_examples 'deletes custom emoji' do + it 'changes count' do + expect { post_graphql_mutation(mutation, current_user: current_user) }.to change(CustomEmoji, :count).by(-1) + end + end + + context 'when the user' do + context 'has no permissions' do + it_behaves_like 'does not delete custom emoji' + end + + context 'when the user is developer and not creator of custom emoji' do + before do + group.add_developer(current_user) + end + + it_behaves_like 'does not delete custom emoji' + end + end + + context 'when user' do + context 'is maintainer' do + before do + group.add_maintainer(current_user) + end + + it_behaves_like 'deletes custom emoji' + end + + context 'is owner' do + before do + group.add_owner(current_user) + end + + it_behaves_like 'deletes custom emoji' + end + + context 'is developer and creator of the emoji' do + before do + group.add_developer(current_user) + custom_emoji.update_attribute(:creator, current_user) + end + + it_behaves_like 'deletes custom emoji' + end + end +end diff --git a/spec/requests/api/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb b/spec/requests/api/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb new file mode 100644 index 00000000000..c9e9a22ee0b --- /dev/null +++ b/spec/requests/api/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Updating the dependency proxy image ttl policy' do + include GraphqlHelpers + using RSpec::Parameterized::TableSyntax + + let_it_be(:user) { create(:user) } + + let(:params) do + { + group_path: group.full_path, + enabled: false, + ttl: 2 + } + end + + let(:mutation) do + graphql_mutation(:update_dependency_proxy_image_ttl_group_policy, params) do + <<~QL + dependencyProxyImageTtlPolicy { + enabled + ttl + } + errors + QL + end + end + + let(:mutation_response) { graphql_mutation_response(:update_dependency_proxy_image_ttl_group_policy) } + let(:ttl_policy_response) { mutation_response['dependencyProxyImageTtlPolicy'] } + + before do + stub_config(dependency_proxy: { enabled: true }) + end + + describe 'post graphql mutation' do + subject { post_graphql_mutation(mutation, current_user: user) } + + let_it_be(:ttl_policy, reload: true) { create(:image_ttl_group_policy) } + let_it_be(:group, reload: true) { ttl_policy.group } + + context 'without permission' do + it 'returns no response' do + subject + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response).to be_nil + end + end + + context 'with permission' do + before do + group.add_developer(user) + end + + it 'returns the updated dependency proxy image ttl policy', :aggregate_failures do + subject + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['errors']).to be_empty + expect(ttl_policy_response).to include( + 'enabled' => params[:enabled], + 'ttl' => params[:ttl] + ) + 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 index 66450f8c604..886f3140086 100644 --- a/spec/requests/api/graphql/mutations/issues/create_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/create_spec.rb @@ -39,11 +39,14 @@ RSpec.describe 'Create an issue' do end it 'creates the issue' do - post_graphql_mutation(mutation, current_user: current_user) + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change(Issue, :count).by(1) expect(response).to have_gitlab_http_status(:success) expect(mutation_response['issue']).to include(input) expect(mutation_response['issue']).to include('discussionLocked' => true) + expect(Issue.last.work_item_type.base_type).to eq('issue') 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 c3aaf090703..0f2eeb90894 100644 --- a/spec/requests/api/graphql/mutations/issues/update_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/update_spec.rb @@ -44,6 +44,19 @@ RSpec.describe 'Update of an existing issue' do expect(mutation_response['issue']).to include('discussionLocked' => true) end + context 'when issue_type is updated' do + let(:input) { { 'iid' => issue.iid.to_s, 'type' => 'INCIDENT' } } + + it 'updates issue_type and work_item_type' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + issue.reload + end.to change { issue.work_item_type.base_type }.from('issue').to('incident').and( + change(issue, :issue_type).from('issue').to('incident') + ) + end + end + context 'setting labels' do let(:mutation) do graphql_mutation(:update_issue, input_params) do diff --git a/spec/requests/api/graphql/project/error_tracking/sentry_errors_request_spec.rb b/spec/requests/api/graphql/project/error_tracking/sentry_errors_request_spec.rb index 80376f56ee8..a540386a9de 100644 --- a/spec/requests/api/graphql/project/error_tracking/sentry_errors_request_spec.rb +++ b/spec/requests/api/graphql/project/error_tracking/sentry_errors_request_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe 'sentry errors requests' do include GraphqlHelpers + let_it_be(:project) { create(:project, :repository) } let_it_be(:project_setting) { create(:project_error_tracking_setting, project: project) } let_it_be(:current_user) { project.owner } @@ -30,7 +31,7 @@ RSpec.describe 'sentry errors requests' do let(:error_data) { graphql_data.dig('project', 'sentryErrors', 'detailedError') } - it 'returns a successful response', :aggregate_failures, :quarantine do + it 'returns a successful response', :aggregate_failures do post_graphql(query, current_user: current_user) expect(response).to have_gitlab_http_status(:success) @@ -48,11 +49,9 @@ RSpec.describe 'sentry errors requests' do end end - context 'reactive cache returns data' do + context 'when reactive cache returns data' do before do - allow_any_instance_of(ErrorTracking::ProjectErrorTrackingSetting) - .to receive(:issue_details) - .and_return(issue: sentry_detailed_error) + stub_setting_for(:issue_details, issue: sentry_detailed_error) post_graphql(query, current_user: current_user) end @@ -72,7 +71,7 @@ RSpec.describe 'sentry errors requests' do end end - context 'user does not have permission' do + context 'when user does not have permission' do let(:current_user) { create(:user) } it 'is expected to return an empty error' do @@ -81,11 +80,9 @@ RSpec.describe 'sentry errors requests' do end end - context 'sentry api returns an error' do + context 'when sentry api returns an error' do before do - expect_any_instance_of(ErrorTracking::ProjectErrorTrackingSetting) - .to receive(:issue_details) - .and_return(error: 'error message') + stub_setting_for(:issue_details, error: 'error message') post_graphql(query, current_user: current_user) end @@ -140,11 +137,11 @@ RSpec.describe 'sentry errors requests' do end end - context 'reactive cache returns data' do + context 'when reactive cache returns data' do before do - expect_any_instance_of(ErrorTracking::ProjectErrorTrackingSetting) - .to receive(:list_sentry_issues) - .and_return(issues: [sentry_error], pagination: pagination) + stub_setting_for(:list_sentry_issues, + issues: [sentry_error], + pagination: pagination) post_graphql(query, current_user: current_user) end @@ -177,11 +174,9 @@ RSpec.describe 'sentry errors requests' do end end - context 'sentry api itself errors out' do + context 'when sentry api itself errors out' do before do - expect_any_instance_of(ErrorTracking::ProjectErrorTrackingSetting) - .to receive(:list_sentry_issues) - .and_return(error: 'error message') + stub_setting_for(:list_sentry_issues, error: 'error message') post_graphql(query, current_user: current_user) end @@ -223,18 +218,16 @@ RSpec.describe 'sentry errors requests' do end end - context 'reactive cache returns data' do + context 'when reactive cache returns data' do before do - allow_any_instance_of(ErrorTracking::ProjectErrorTrackingSetting) - .to receive(:issue_latest_event) - .and_return(latest_event: sentry_stack_trace) + stub_setting_for(:issue_latest_event, latest_event: sentry_stack_trace) post_graphql(query, current_user: current_user) end it_behaves_like 'setting stack trace error' - context 'user does not have permission' do + context 'when user does not have permission' do let(:current_user) { create(:user) } it 'is expected to return an empty error' do @@ -243,11 +236,9 @@ RSpec.describe 'sentry errors requests' do end end - context 'sentry api returns an error' do + context 'when sentry api returns an error' do before do - expect_any_instance_of(ErrorTracking::ProjectErrorTrackingSetting) - .to receive(:issue_latest_event) - .and_return(error: 'error message') + stub_setting_for(:issue_latest_event, error: 'error message') post_graphql(query, current_user: current_user) end @@ -257,4 +248,12 @@ RSpec.describe 'sentry errors requests' do end end end + + private + + def stub_setting_for(method, **return_value) + allow_next_found_instance_of(ErrorTracking::ProjectErrorTrackingSetting) do |setting| + allow(setting).to receive(method).and_return(**return_value) + end + end end diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb index ff0d7ecceb5..c6b4d82bf15 100644 --- a/spec/requests/api/graphql/project/issues_spec.rb +++ b/spec/requests/api/graphql/project/issues_spec.rb @@ -61,6 +61,34 @@ RSpec.describe 'getting an issue list for a project' do end end + context 'filtering by my_reaction_emoji' do + using RSpec::Parameterized::TableSyntax + + let_it_be(:upvote_award) { create(:award_emoji, :upvote, user: current_user, awardable: issue_a) } + + let(:issue_a_gid) { issue_a.to_global_id.to_s } + let(:issue_b_gid) { issue_b.to_global_id.to_s } + + where(:value, :gids) do + 'thumbsup' | lazy { [issue_a_gid] } + 'ANY' | lazy { [issue_a_gid] } + 'any' | lazy { [issue_a_gid] } + 'AnY' | lazy { [issue_a_gid] } + 'NONE' | lazy { [issue_b_gid] } + 'thumbsdown' | lazy { [] } + end + + with_them do + let(:issue_filter_params) { { my_reaction_emoji: value } } + + it 'returns correctly filtered issues' do + post_graphql(query, current_user: current_user) + + expect(graphql_dig_at(issues_data, :node, :id)).to eq(gids) + end + end + end + context 'when limiting the number of results' do let(:query) do <<~GQL diff --git a/spec/requests/api/graphql/project/pipeline_spec.rb b/spec/requests/api/graphql/project/pipeline_spec.rb index cb6755640a9..d46ef313563 100644 --- a/spec/requests/api/graphql/project/pipeline_spec.rb +++ b/spec/requests/api/graphql/project/pipeline_spec.rb @@ -311,6 +311,10 @@ RSpec.describe 'getting pipeline information nested in a project' do end it 'does not generate N+1 queries', :request_store, :use_sql_query_cache do + # create extra statuses + create(:generic_commit_status, :pending, name: 'generic-build-a', pipeline: pipeline, stage_idx: 0, stage: 'build') + create(:ci_bridge, :failed, name: 'deploy-a', pipeline: pipeline, stage_idx: 2, stage: 'deploy') + # warm up post_graphql(query, current_user: current_user) @@ -318,9 +322,11 @@ RSpec.describe 'getting pipeline information nested in a project' do post_graphql(query, current_user: current_user) end - create(:ci_build, name: 'test-a', pipeline: pipeline, stage_idx: 1, stage: 'test') - create(:ci_build, name: 'test-b', pipeline: pipeline, stage_idx: 1, stage: 'test') - create(:ci_build, name: 'deploy-a', pipeline: pipeline, stage_idx: 2, stage: 'deploy') + create(:generic_commit_status, :pending, name: 'generic-build-b', pipeline: pipeline, stage_idx: 0, stage: 'build') + create(:ci_build, :failed, name: 'test-a', pipeline: pipeline, stage_idx: 1, stage: 'test') + create(:ci_build, :running, name: 'test-b', pipeline: pipeline, stage_idx: 1, stage: 'test') + create(:ci_build, :pending, name: 'deploy-b', pipeline: pipeline, stage_idx: 2, stage: 'deploy') + create(:ci_bridge, :failed, name: 'deploy-c', pipeline: pipeline, stage_idx: 2, stage: 'deploy') expect do post_graphql(query, current_user: current_user) diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 30df47ccc41..38abedde7da 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -158,6 +158,127 @@ RSpec.describe API::Groups do end end + context 'pagination strategies' do + let_it_be(:group_1) { create(:group, name: '1_group') } + let_it_be(:group_2) { create(:group, name: '2_group') } + + context 'when the user is anonymous' do + context 'offset pagination' do + context 'on making requests beyond the allowed offset pagination threshold' do + it 'returns error and suggests to use keyset pagination' do + get api('/groups'), params: { page: 3000, per_page: 25 } + + expect(response).to have_gitlab_http_status(:method_not_allowed) + expect(json_response['error']).to eq( + 'Offset pagination has a maximum allowed offset of 50000 for requests that return objects of type Group. '\ + 'Remaining records can be retrieved using keyset pagination.' + ) + end + + context 'when the feature flag `keyset_pagination_for_groups_api` is disabled' do + before do + stub_feature_flags(keyset_pagination_for_groups_api: false) + end + + it 'returns successful response' do + get api('/groups'), params: { page: 3000, per_page: 25 } + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + context 'on making requests below the allowed offset pagination threshold' do + it 'paginates the records' do + get api('/groups'), params: { page: 1, per_page: 1 } + + expect(response).to have_gitlab_http_status(:ok) + records = json_response + expect(records.size).to eq(1) + expect(records.first['id']).to eq(group_1.id) + + # next page + + get api('/groups'), params: { page: 2, per_page: 1 } + + expect(response).to have_gitlab_http_status(:ok) + records = Gitlab::Json.parse(response.body) + expect(records.size).to eq(1) + expect(records.first['id']).to eq(group_2.id) + end + end + end + + context 'keyset pagination' do + def pagination_links(response) + link = response.headers['LINK'] + return unless link + + link.split(',').map do |link| + match = link.match(/<(?<url>.*)>; rel="(?<rel>\w+)"/) + break nil unless match + + { url: match[:url], rel: match[:rel] } + end.compact + end + + def params_for_next_page(response) + next_url = pagination_links(response).find { |link| link[:rel] == 'next' }[:url] + Rack::Utils.parse_query(URI.parse(next_url).query) + end + + context 'on making requests with supported ordering structure' do + it 'paginates the records correctly' do + # first page + get api('/groups'), params: { pagination: 'keyset', per_page: 1 } + + expect(response).to have_gitlab_http_status(:ok) + records = json_response + expect(records.size).to eq(1) + expect(records.first['id']).to eq(group_1.id) + + params_for_next_page = params_for_next_page(response) + expect(params_for_next_page).to include('cursor') + + get api('/groups'), params: params_for_next_page + + expect(response).to have_gitlab_http_status(:ok) + records = Gitlab::Json.parse(response.body) + expect(records.size).to eq(1) + expect(records.first['id']).to eq(group_2.id) + end + + context 'when the feature flag `keyset_pagination_for_groups_api` is disabled' do + before do + stub_feature_flags(keyset_pagination_for_groups_api: false) + end + + it 'ignores the keyset pagination params and performs offset pagination' do + get api('/groups'), params: { pagination: 'keyset', per_page: 1 } + + expect(response).to have_gitlab_http_status(:ok) + records = json_response + expect(records.size).to eq(1) + expect(records.first['id']).to eq(group_1.id) + + params_for_next_page = params_for_next_page(response) + expect(params_for_next_page).not_to include('cursor') + end + end + end + + context 'on making requests with unsupported ordering structure' do + it 'returns error' do + get api('/groups'), params: { pagination: 'keyset', per_page: 1, order_by: 'path', sort: 'desc' } + + expect(response).to have_gitlab_http_status(:method_not_allowed) + expect(json_response['error']).to eq('Keyset pagination is not yet available for this type of request') + end + end + end + end + end + context "when authenticated as admin" do it "admin: returns an array of all groups" do get api("/groups", admin) diff --git a/spec/requests/api/helm_packages_spec.rb b/spec/requests/api/helm_packages_spec.rb index 08b4489a6e3..3236857c5fc 100644 --- a/spec/requests/api/helm_packages_spec.rb +++ b/spec/requests/api/helm_packages_spec.rb @@ -9,16 +9,32 @@ RSpec.describe API::HelmPackages do let_it_be_with_reload(:project) { create(:project, :public) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } - let_it_be(:package) { create(:helm_package, project: project) } + let_it_be(:package) { create(:helm_package, project: project, without_package_files: true) } + let_it_be(:package_file1) { create(:helm_package_file, package: package) } + let_it_be(:package_file2) { create(:helm_package_file, package: package) } + let_it_be(:package2) { create(:helm_package, project: project, without_package_files: true) } + let_it_be(:package_file2_1) { create(:helm_package_file, package: package2, file_sha256: 'file2', file_name: 'filename2.tgz', description: 'hello from stable channel') } + let_it_be(:package_file2_2) { create(:helm_package_file, package: package2, file_sha256: 'file2', file_name: 'filename2.tgz', channel: 'test', description: 'hello from test channel') } + let_it_be(:other_package) { create(:npm_package, project: project) } describe 'GET /api/v4/projects/:id/packages/helm/:channel/index.yaml' do - it_behaves_like 'handling helm chart index requests' do - let(:url) { "/projects/#{project.id}/packages/helm/#{package.package_files.first.helm_channel}/index.yaml" } + let(:url) { "/projects/#{project_id}/packages/helm/stable/index.yaml" } + + context 'with a project id' do + let(:project_id) { project.id } + + it_behaves_like 'handling helm chart index requests' + end + + context 'with an url encoded project id' do + let(:project_id) { ERB::Util.url_encode(project.full_path) } + + it_behaves_like 'handling helm chart index requests' end end describe 'GET /api/v4/projects/:id/packages/helm/:channel/charts/:file_name.tgz' do - let(:url) { "/projects/#{project.id}/packages/helm/#{package.package_files.first.helm_channel}/charts/#{package.name}-#{package.version}.tgz" } + let(:url) { "/projects/#{project.id}/packages/helm/stable/charts/#{package.name}-#{package.version}.tgz" } subject { get api(url), headers: headers } diff --git a/spec/requests/api/internal/kubernetes_spec.rb b/spec/requests/api/internal/kubernetes_spec.rb index 2acf6951d50..24422f7b0dd 100644 --- a/spec/requests/api/internal/kubernetes_spec.rb +++ b/spec/requests/api/internal/kubernetes_spec.rb @@ -93,6 +93,48 @@ RSpec.describe API::Internal::Kubernetes do end end + describe 'POST /internal/kubernetes/agent_configuration' do + def send_request(headers: {}, params: {}) + post api('/internal/kubernetes/agent_configuration'), params: params, headers: headers.reverse_merge(jwt_auth_headers) + end + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, namespace: group) } + let_it_be(:agent) { create(:cluster_agent, project: project) } + let_it_be(:config) do + { + ci_access: { + groups: [ + { id: group.full_path, default_namespace: 'production' } + ], + projects: [ + { id: project.full_path, default_namespace: 'staging' } + ] + } + } + end + + include_examples 'authorization' + + context 'agent exists' do + it 'configures the agent and returns a 204' do + send_request(params: { agent_id: agent.id, agent_config: config }) + + expect(response).to have_gitlab_http_status(:no_content) + expect(agent.authorized_groups).to contain_exactly(group) + expect(agent.authorized_projects).to contain_exactly(project) + end + end + + context 'agent does not exist' do + it 'returns a 404' do + send_request(params: { agent_id: -1, agent_config: config }) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + describe 'GET /internal/kubernetes/agent_info' do def send_request(headers: {}, params: {}) get api('/internal/kubernetes/agent_info'), params: params, headers: headers.reverse_merge(jwt_auth_headers) diff --git a/spec/requests/api/issues/get_group_issues_spec.rb b/spec/requests/api/issues/get_group_issues_spec.rb index cebde747210..3663a82891c 100644 --- a/spec/requests/api/issues/get_group_issues_spec.rb +++ b/spec/requests/api/issues/get_group_issues_spec.rb @@ -402,14 +402,7 @@ RSpec.describe API::Issues do expect_paginated_array_response([group_closed_issue.id, group_issue.id]) end - shared_examples 'labels parameter' do - it 'returns an array of labeled group issues' do - get api(base_url, user), params: { labels: group_label.title } - - expect_paginated_array_response(group_issue.id) - expect(json_response.first['labels']).to eq([group_label.title]) - end - + context 'labels parameter' do it 'returns an array of labeled group issues' do get api(base_url, user), params: { labels: group_label.title } @@ -458,22 +451,6 @@ RSpec.describe API::Issues do end end - context 'when `optimized_issuable_label_filter` feature flag is off' do - before do - stub_feature_flags(optimized_issuable_label_filter: false) - end - - it_behaves_like 'labels parameter' - end - - context 'when `optimized_issuable_label_filter` feature flag is on' do - before do - stub_feature_flags(optimized_issuable_label_filter: true) - end - - it_behaves_like 'labels parameter' - end - it 'returns issues matching given search string for title' do get api(base_url, user), params: { search: group_issue.title } diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index 125db58ed69..8a33e63b80b 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -3,21 +3,25 @@ require 'spec_helper' RSpec.describe API::Issues do + using RSpec::Parameterized::TableSyntax + let_it_be(:user) { create(:user) } let_it_be(:project, reload: true) { create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace) } let_it_be(:private_mrs_project) do create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace, merge_requests_access_level: ProjectFeature::PRIVATE) end - let(:user2) { create(:user) } - let(:non_member) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:non_member) { create(:user) } let_it_be(:guest) { create(:user) } let_it_be(:author) { create(:author) } let_it_be(:assignee) { create(:assignee) } - let(:admin) { create(:user, :admin) } - let(:issue_title) { 'foo' } - let(:issue_description) { 'closed' } - let!(:closed_issue) do + let_it_be(:admin) { create(:user, :admin) } + + let_it_be(:milestone) { create(:milestone, title: '1.0.0', project: project) } + let_it_be(:empty_milestone) { create(:milestone, title: '2.0.0', project: project) } + + let_it_be(:closed_issue) do create :closed_issue, author: user, assignees: [user], @@ -29,7 +33,7 @@ RSpec.describe API::Issues do closed_at: 1.hour.ago end - let!(:confidential_issue) do + let_it_be(:confidential_issue) do create :issue, :confidential, project: project, @@ -39,7 +43,7 @@ RSpec.describe API::Issues do updated_at: 2.hours.ago end - let!(:issue) do + let_it_be(:issue) do create :issue, author: user, assignees: [user], @@ -47,21 +51,16 @@ RSpec.describe API::Issues do milestone: milestone, created_at: generate(:past_time), updated_at: 1.hour.ago, - title: issue_title, - description: issue_description + title: 'foo', + description: 'bar' end let_it_be(:label) do create(:label, title: 'label', color: '#FFAABB', project: project) end - let!(:label_link) { create(:label_link, label: label, target: issue) } - let(:milestone) { create(:milestone, title: '1.0.0', project: project) } - let_it_be(:empty_milestone) do - create(:milestone, title: '2.0.0', project: project) - end - - let!(:note) { create(:note_on_issue, author: user, project: project, noteable: issue) } + let_it_be(:label_link) { create(:label_link, label: label, target: issue) } + let_it_be(:note) { create(:note_on_issue, author: user, project: project, noteable: issue) } let(:no_milestone_title) { 'None' } let(:any_milestone_title) { 'Any' } @@ -683,6 +682,71 @@ RSpec.describe API::Issues do end end + context 'filtering by milestone_id' do + let_it_be(:upcoming_milestone) { create(:milestone, project: project, title: "upcoming milestone", start_date: 1.day.ago, due_date: 1.day.from_now) } + let_it_be(:started_milestone) { create(:milestone, project: project, title: "started milestone", start_date: 2.days.ago, due_date: 1.day.ago) } + let_it_be(:future_milestone) { create(:milestone, project: project, title: "future milestone", start_date: 7.days.from_now, due_date: 14.days.from_now) } + let_it_be(:issue_upcoming) { create(:issue, project: project, state: :opened, milestone: upcoming_milestone) } + let_it_be(:issue_started) { create(:issue, project: project, state: :opened, milestone: started_milestone) } + let_it_be(:issue_future) { create(:issue, project: project, state: :opened, milestone: future_milestone) } + let_it_be(:issue_none) { create(:issue, project: project, state: :opened) } + + let(:wildcard_started) { 'Started' } + let(:wildcard_upcoming) { 'Upcoming' } + let(:wildcard_any) { 'Any' } + let(:wildcard_none) { 'None' } + + where(:milestone_id, :not_milestone, :expected_issues) do + ref(:wildcard_none) | nil | lazy { [issue_none.id] } + ref(:wildcard_any) | nil | lazy { [issue_future.id, issue_started.id, issue_upcoming.id, issue.id, closed_issue.id] } + ref(:wildcard_started) | nil | lazy { [issue_started.id, issue_upcoming.id] } + ref(:wildcard_upcoming) | nil | lazy { [issue_upcoming.id] } + ref(:wildcard_any) | "upcoming milestone" | lazy { [issue_future.id, issue_started.id, issue.id, closed_issue.id] } + ref(:wildcard_upcoming) | "upcoming milestone" | [] + end + + with_them do + it "returns correct issues when filtering with 'milestone_id' and optionally negated 'milestone'" do + get api('/issues', user), params: { milestone_id: milestone_id, not: not_milestone ? { milestone: not_milestone } : {} } + + expect_paginated_array_response(expected_issues) + end + end + + context 'negated filtering' do + where(:not_milestone_id, :expected_issues) do + ref(:wildcard_started) | lazy { [issue_future.id] } + ref(:wildcard_upcoming) | lazy { [issue_started.id] } + end + + with_them do + it "returns correct issues when filtering with negated 'milestone_id'" do + get api('/issues', user), params: { not: { milestone_id: not_milestone_id } } + + expect_paginated_array_response(expected_issues) + end + end + end + + context 'when mutually exclusive params are passed' do + where(:params) do + [ + [lazy { { milestone: "foo", milestone_id: wildcard_any } }], + [lazy { { not: { milestone: "foo", milestone_id: wildcard_any } } }] + ] + end + + with_them do + it "raises an error", :aggregate_failures do + get api('/issues', user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response["error"]).to include("mutually exclusive") + end + end + end + end + it 'returns an array of issues found by iids' do get api('/issues', user), params: { iids: [closed_issue.iid] } @@ -711,8 +775,8 @@ RSpec.describe API::Issues do milestone: milestone, created_at: closed_issue.created_at, updated_at: 1.hour.ago, - title: issue_title, - description: issue_description + title: 'foo', + description: 'bar' end it 'page breaks first page correctly' do @@ -751,6 +815,18 @@ RSpec.describe API::Issues do expect_paginated_array_response([closed_issue.id, issue.id]) end + it 'sorts by title asc when requested' do + get api('/issues', user), params: { order_by: :title, sort: :asc } + + expect_paginated_array_response([issue.id, closed_issue.id]) + end + + it 'sorts by title desc when requested' do + get api('/issues', user), params: { order_by: :title, sort: :desc } + + expect_paginated_array_response([closed_issue.id, issue.id]) + end + context 'with issues list sort options' do it 'accepts only predefined order by params' do API::Helpers::IssuesHelpers.sort_options.each do |sort_opt| @@ -760,7 +836,7 @@ RSpec.describe API::Issues do end it 'fails to sort with non predefined options' do - %w(milestone title abracadabra).each do |sort_opt| + %w(milestone abracadabra).each do |sort_opt| get api('/issues', user), params: { order_by: sort_opt, sort: 'asc' } expect(response).to have_gitlab_http_status(:bad_request) end @@ -1001,13 +1077,15 @@ RSpec.describe API::Issues do end describe 'DELETE /projects/:id/issues/:issue_iid' do + let(:issue_for_deletion) { create(:issue, author: user, assignees: [user], project: project) } + it 'rejects a non member from deleting an issue' do - delete api("/projects/#{project.id}/issues/#{issue.iid}", non_member) + delete api("/projects/#{project.id}/issues/#{issue_for_deletion.iid}", non_member) expect(response).to have_gitlab_http_status(:forbidden) end it 'rejects a developer from deleting an issue' do - delete api("/projects/#{project.id}/issues/#{issue.iid}", author) + delete api("/projects/#{project.id}/issues/#{issue_for_deletion.iid}", author) expect(response).to have_gitlab_http_status(:forbidden) end @@ -1016,13 +1094,13 @@ RSpec.describe API::Issues do let(:project) { create(:project, namespace: owner.namespace) } it 'deletes the issue if an admin requests it' do - delete api("/projects/#{project.id}/issues/#{issue.iid}", owner) + delete api("/projects/#{project.id}/issues/#{issue_for_deletion.iid}", owner) expect(response).to have_gitlab_http_status(:no_content) end it_behaves_like '412 response' do - let(:request) { api("/projects/#{project.id}/issues/#{issue.iid}", owner) } + let(:request) { api("/projects/#{project.id}/issues/#{issue_for_deletion.iid}", owner) } end end @@ -1035,7 +1113,7 @@ RSpec.describe API::Issues do end it 'returns 404 when using the issue ID instead of IID' do - delete api("/projects/#{project.id}/issues/#{issue.id}", user) + delete api("/projects/#{project.id}/issues/#{issue_for_deletion.id}", user) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb index 7fe516d3daa..d7f22b9d619 100644 --- a/spec/requests/api/lint_spec.rb +++ b/spec/requests/api/lint_spec.rb @@ -113,7 +113,6 @@ RSpec.describe API::Lint do expect(response).to have_gitlab_http_status(:ok) expect(json_response['status']).to eq('valid') expect(json_response['warnings']).not_to be_empty - expect(json_response['status']).to eq('valid') expect(json_response['errors']).to eq([]) end end diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index c3fd02dad51..07111dd1d62 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -15,7 +15,7 @@ RSpec.describe API::MavenPackages do let_it_be(:package_file) { package.package_files.with_file_name_like('%.xml').first } let_it_be(:jar_file) { package.package_files.with_file_name_like('%.jar').first } let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } - let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running) } + let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running, project: project) } 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) } diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 48ded93d85f..a1daf86de31 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -198,7 +198,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) + expect(json_response['created_at'].to_time).to be_present end end end @@ -311,36 +311,6 @@ RSpec.describe API::Members do expect(json_response['status']).to eq('error') expect(json_response['message']).to eq(error_message) end - - context 'with invite_source considerations', :snowplow do - let(:params) { { user_id: user_ids, access_level: Member::DEVELOPER } } - - it 'tracks the invite source as api' do - post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), - params: params - - expect_snowplow_event( - category: 'Members::CreateService', - action: 'create_member', - label: 'members-api', - property: 'existing_user', - user: maintainer - ) - end - - it 'tracks the invite source from params' do - post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), - params: params.merge(invite_source: '_invite_source_') - - expect_snowplow_event( - category: 'Members::CreateService', - action: 'create_member', - label: '_invite_source_', - property: 'existing_user', - user: maintainer - ) - end - end end end @@ -410,48 +380,28 @@ RSpec.describe API::Members do end context 'with areas_of_focus considerations', :snowplow do - context 'when there is 1 user to add' do - let(:user_id) { stranger.id } + let(:user_id) { stranger.id } - context 'when areas_of_focus is present in params' do - it 'tracks the areas_of_focus' do - post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), - params: { user_id: user_id, access_level: Member::DEVELOPER, areas_of_focus: 'Other' } - - expect_snowplow_event( - category: 'Members::CreateService', - action: 'area_of_focus', - label: 'Other', - property: source.members.last.id.to_s - ) - end - end - - context 'when areas_of_focus is not present in params' do - it 'does not track the areas_of_focus' do - post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), - params: { user_id: user_id, access_level: Member::DEVELOPER } + context 'when areas_of_focus is present in params' do + it 'tracks the areas_of_focus' do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), + params: { user_id: user_id, access_level: Member::DEVELOPER, areas_of_focus: 'Other' } - expect_no_snowplow_event(category: 'Members::CreateService', action: 'area_of_focus') - end + expect_snowplow_event( + category: 'Members::CreateService', + action: 'area_of_focus', + label: 'Other', + property: source.members.last.id.to_s + ) end end - context 'when there are multiple users to add' do - let(:user_id) { [developer.id, stranger.id].join(',') } + context 'when areas_of_focus is not present in params' do + it 'does not track the areas_of_focus' do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), + params: { user_id: user_id, access_level: Member::DEVELOPER } - context 'when areas_of_focus is present in params' do - it 'tracks the areas_of_focus' do - post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), - params: { user_id: user_id, access_level: Member::DEVELOPER, areas_of_focus: 'Other' } - - expect_snowplow_event( - category: 'Members::CreateService', - action: 'area_of_focus', - label: 'Other', - property: source.members.last.id.to_s - ) - end + expect_no_snowplow_event(category: 'Members::CreateService', action: 'area_of_focus') end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 4b5fc57571b..7a587e82683 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1072,7 +1072,7 @@ RSpec.describe API::MergeRequests do end describe "GET /groups/:id/merge_requests" do - let_it_be(:group) { create(:group, :public) } + let_it_be(:group, reload: true) { create(:group, :public) } let_it_be(:project) { create(:project, :public, :repository, creator: user, namespace: group, only_allow_merge_if_pipeline_succeeds: false) } include_context 'with merge requests' diff --git a/spec/requests/api/notification_settings_spec.rb b/spec/requests/api/notification_settings_spec.rb index 7b4a58e63da..b5551c21738 100644 --- a/spec/requests/api/notification_settings_spec.rb +++ b/spec/requests/api/notification_settings_spec.rb @@ -13,7 +13,7 @@ RSpec.describe API::NotificationSettings do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_a Hash - expect(json_response['notification_email']).to eq(user.notification_email) + expect(json_response['notification_email']).to eq(user.notification_email_or_default) expect(json_response['level']).to eq(user.global_notification_setting.level) end end diff --git a/spec/requests/api/npm_project_packages_spec.rb b/spec/requests/api/npm_project_packages_spec.rb index 8c35a1642e2..0d04c2cad5b 100644 --- a/spec/requests/api/npm_project_packages_spec.rb +++ b/spec/requests/api/npm_project_packages_spec.rb @@ -120,9 +120,11 @@ RSpec.describe API::NpmProjectPackages do project.add_developer(user) end + subject(:upload_package_with_token) { upload_with_token(package_name, params) } + shared_examples 'handling invalid record with 400 error' do it 'handles an ActiveRecord::RecordInvalid exception with 400 error' do - expect { upload_package_with_token(package_name, params) } + expect { upload_package_with_token } .not_to change { project.packages.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -136,6 +138,7 @@ RSpec.describe API::NpmProjectPackages do let(:params) { upload_params(package_name: package_name) } it_behaves_like 'handling invalid record with 400 error' + it_behaves_like 'not a package tracking event' end context 'invalid package version' do @@ -157,6 +160,7 @@ RSpec.describe API::NpmProjectPackages do let(:params) { upload_params(package_name: package_name, package_version: version) } it_behaves_like 'handling invalid record with 400 error' + it_behaves_like 'not a package tracking event' end end end @@ -169,8 +173,6 @@ RSpec.describe API::NpmProjectPackages do shared_examples 'handling upload with different authentications' do context 'with access token' do - subject { upload_package_with_token(package_name, params) } - it_behaves_like 'a package tracking event', 'API::NpmPackages', 'push_package' it 'creates npm package with file' do @@ -184,7 +186,7 @@ RSpec.describe API::NpmProjectPackages do end it 'creates npm package with file with job token' do - expect { upload_package_with_job_token(package_name, params) } + expect { upload_with_job_token(package_name, params) } .to change { project.packages.count }.by(1) .and change { Packages::PackageFile.count }.by(1) @@ -205,7 +207,7 @@ RSpec.describe API::NpmProjectPackages do end it 'creates the package metadata' do - upload_package_with_token(package_name, params) + upload_package_with_token expect(response).to have_gitlab_http_status(:ok) expect(project.reload.packages.find(json_response['id']).original_build_info.pipeline).to eq job.pipeline @@ -215,7 +217,7 @@ RSpec.describe API::NpmProjectPackages do shared_examples 'uploading the package' do it 'uploads the package' do - expect { upload_package_with_token(package_name, params) } + expect { upload_package_with_token } .to change { project.packages.count }.by(1) expect(response).to have_gitlab_http_status(:ok) @@ -249,6 +251,7 @@ RSpec.describe API::NpmProjectPackages do let(:package_name) { "@#{group.path}/test" } it_behaves_like 'handling invalid record with 400 error' + it_behaves_like 'not a package tracking event' context 'with a new version' do let_it_be(:version) { '4.5.6' } @@ -271,9 +274,14 @@ RSpec.describe API::NpmProjectPackages do let(:package_name) { "@#{group.path}/my_package_name" } let(:params) { upload_params(package_name: package_name) } - it 'returns an error if the package already exists' do + before do create(:npm_package, project: project, version: '1.0.1', name: "@#{group.path}/my_package_name") - expect { upload_package_with_token(package_name, params) } + end + + it_behaves_like 'not a package tracking event' + + it 'returns an error if the package already exists' do + expect { upload_package_with_token } .not_to change { project.packages.count } expect(response).to have_gitlab_http_status(:forbidden) @@ -285,7 +293,7 @@ RSpec.describe API::NpmProjectPackages do let(:params) { upload_params(package_name: package_name, file: 'npm/payload_with_duplicated_packages.json') } it 'creates npm package with file and dependencies' do - expect { upload_package_with_token(package_name, params) } + expect { upload_package_with_token } .to change { project.packages.count }.by(1) .and change { Packages::PackageFile.count }.by(1) .and change { Packages::Dependency.count}.by(4) @@ -297,11 +305,11 @@ RSpec.describe API::NpmProjectPackages do context 'with existing dependencies' do before do name = "@#{group.path}/existing_package" - upload_package_with_token(name, upload_params(package_name: name, file: 'npm/payload_with_duplicated_packages.json')) + upload_with_token(name, upload_params(package_name: name, file: 'npm/payload_with_duplicated_packages.json')) end it 'reuses them' do - expect { upload_package_with_token(package_name, params) } + expect { upload_package_with_token } .to change { project.packages.count }.by(1) .and change { Packages::PackageFile.count }.by(1) .and not_change { Packages::Dependency.count} @@ -317,11 +325,11 @@ RSpec.describe API::NpmProjectPackages do put api("/projects/#{project.id}/packages/npm/#{package_name.sub('/', '%2f')}"), params: params, headers: headers end - def upload_package_with_token(package_name, params = {}) + def upload_with_token(package_name, params = {}) upload_package(package_name, params.merge(access_token: token.token)) end - def upload_package_with_job_token(package_name, params = {}) + def upload_with_job_token(package_name, params = {}) upload_package(package_name, params.merge(job_token: job.token)) end diff --git a/spec/requests/api/pages/pages_spec.rb b/spec/requests/api/pages/pages_spec.rb index f4c6de00e40..0eb2ae64f43 100644 --- a/spec/requests/api/pages/pages_spec.rb +++ b/spec/requests/api/pages/pages_spec.rb @@ -36,12 +36,7 @@ RSpec.describe API::Pages do end it 'removes the pages' do - expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return true - expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, project.namespace.full_path, anything) - - Sidekiq::Testing.inline! do - delete api("/projects/#{project.id}/pages", admin ) - end + delete api("/projects/#{project.id}/pages", admin ) expect(project.reload.pages_metadatum.deployed?).to be(false) end diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index c5bcedd491a..9174356f123 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -32,6 +32,7 @@ itself: # project - pages_https_only - pending_delete - pool_repository_id + - project_namespace_id - pull_mirror_available_overridden - pull_mirror_branch_prefix - remote_mirror_available_overridden @@ -55,6 +56,7 @@ itself: # project - can_create_merge_request_in - compliance_frameworks - container_expiration_policy + - container_registry_enabled - container_registry_image_prefix - default_branch - empty_repo @@ -149,6 +151,7 @@ build_service_desk_setting: # service_desk_setting unexposed_attributes: - project_id - issue_template_key + - file_template_project_id - outgoing_name remapped_attributes: project_key: service_desk_address diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 3622eedfed5..80bccdfee0c 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -2616,6 +2616,23 @@ RSpec.describe API::Projects do expect(json_response).to have_key 'service_desk_enabled' expect(json_response).to have_key 'service_desk_address' end + + context 'when project is shared to multiple groups' do + it 'avoids N+1 queries' do + create(:project_group_link, project: project) + get api("/projects/#{project.id}", user) + + control = ActiveRecord::QueryRecorder.new do + get api("/projects/#{project.id}", user) + end + + create(:project_group_link, project: project) + + expect do + get api("/projects/#{project.id}", user) + end.not_to exceed_query_limit(control) + end + end end describe 'GET /projects/:id/users' do diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index 8df2460a2b6..c17d0600aca 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -13,7 +13,7 @@ RSpec.describe API::PypiPackages do let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } - let_it_be(:job) { create(:ci_build, :running, user: user) } + let_it_be(:job) { create(:ci_build, :running, user: user, project: project) } let(:headers) { {} } diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 87b08587904..90b03a480a8 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -839,7 +839,7 @@ RSpec.describe API::Releases do context 'when a valid token is provided' do it 'creates the release for a running job' do - job.update!(status: :running) + job.update!(status: :running, project: project) post api("/projects/#{project.id}/releases"), params: params.merge(job_token: job.token) expect(response).to have_gitlab_http_status(:created) diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index d3262b8056b..a576e1ab1ee 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -22,7 +22,7 @@ RSpec.describe API::Repositories 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).to be_an(Array) first_commit = json_response.first expect(first_commit['name']).to eq('bar') @@ -73,6 +73,25 @@ RSpec.describe API::Repositories do end end end + + context 'keyset pagination mode' do + let(:first_response) do + get api(route, current_user), params: { pagination: "keyset" } + + Gitlab::Json.parse(response.body) + end + + it 'paginates using keysets' do + page_token = first_response.last["id"] + + get api(route, current_user), params: { pagination: "keyset", page_token: page_token } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an(Array) + expect(json_response).not_to eq(first_response) + expect(json_response.map { |t| t["id"] }).not_to include(page_token) + end + end end context 'when unauthenticated', 'and project is public' do @@ -354,6 +373,7 @@ RSpec.describe API::Repositories do expect(response).to have_gitlab_http_status(:ok) expect(json_response['commits']).to be_present expect(json_response['diffs']).to be_present + expect(json_response['web_url']).to be_present end it "compares branches with explicit merge-base mode" do @@ -365,6 +385,7 @@ RSpec.describe API::Repositories do expect(response).to have_gitlab_http_status(:ok) expect(json_response['commits']).to be_present expect(json_response['diffs']).to be_present + expect(json_response['web_url']).to be_present end it "compares branches with explicit straight mode" do @@ -376,6 +397,7 @@ RSpec.describe API::Repositories do expect(response).to have_gitlab_http_status(:ok) expect(json_response['commits']).to be_present expect(json_response['diffs']).to be_present + expect(json_response['web_url']).to be_present end it "compares tags" do @@ -384,6 +406,7 @@ RSpec.describe API::Repositories do expect(response).to have_gitlab_http_status(:ok) expect(json_response['commits']).to be_present expect(json_response['diffs']).to be_present + expect(json_response['web_url']).to be_present end it "compares commits" do @@ -393,6 +416,7 @@ RSpec.describe API::Repositories do expect(json_response['commits']).to be_empty expect(json_response['diffs']).to be_empty expect(json_response['compare_same_ref']).to be_falsey + expect(json_response['web_url']).to be_present end it "compares commits in reverse order" do @@ -401,6 +425,7 @@ RSpec.describe API::Repositories do expect(response).to have_gitlab_http_status(:ok) expect(json_response['commits']).to be_present expect(json_response['diffs']).to be_present + expect(json_response['web_url']).to be_present end it "compare commits between different projects with non-forked relation" do diff --git a/spec/requests/api/rubygem_packages_spec.rb b/spec/requests/api/rubygem_packages_spec.rb index afa7adad80c..9b104520b52 100644 --- a/spec/requests/api/rubygem_packages_spec.rb +++ b/spec/requests/api/rubygem_packages_spec.rb @@ -10,7 +10,7 @@ RSpec.describe API::RubygemPackages do let_it_be_with_reload(:project) { create(:project) } let_it_be(:personal_access_token) { create(:personal_access_token) } let_it_be(:user) { personal_access_token.user } - let_it_be(:job) { create(:ci_build, :running, user: user) } + let_it_be(:job) { create(:ci_build, :running, user: user, project: project) } 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(:headers) { {} } diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 4008b57a1cf..f5d261ba4c6 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -47,6 +47,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['personal_access_token_prefix']).to be_nil expect(json_response['admin_mode']).to be(false) expect(json_response['whats_new_variant']).to eq('all_tiers') + expect(json_response['user_deactivation_emails_enabled']).to be(true) end end @@ -133,6 +134,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do import_sources: 'github,bitbucket', wiki_page_max_content_bytes: 12345, personal_access_token_prefix: "GL-", + user_deactivation_emails_enabled: false, admin_mode: true } @@ -184,6 +186,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['wiki_page_max_content_bytes']).to eq(12345) expect(json_response['personal_access_token_prefix']).to eq("GL-") expect(json_response['admin_mode']).to be(true) + expect(json_response['user_deactivation_emails_enabled']).to be(false) end end @@ -222,6 +225,45 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['asset_proxy_allowlist']).to eq(['example.com', '*.example.com', 'localhost']) end + it 'supports the deprecated `throttle_unauthenticated_*` attributes' do + put api('/application/settings', admin), params: { + throttle_unauthenticated_enabled: true, + throttle_unauthenticated_period_in_seconds: 123, + throttle_unauthenticated_requests_per_period: 456 + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include( + 'throttle_unauthenticated_enabled' => true, + 'throttle_unauthenticated_period_in_seconds' => 123, + 'throttle_unauthenticated_requests_per_period' => 456, + 'throttle_unauthenticated_web_enabled' => true, + 'throttle_unauthenticated_web_period_in_seconds' => 123, + 'throttle_unauthenticated_web_requests_per_period' => 456 + ) + end + + it 'prefers the new `throttle_unauthenticated_web_*` attributes' do + put api('/application/settings', admin), params: { + throttle_unauthenticated_enabled: false, + throttle_unauthenticated_period_in_seconds: 0, + throttle_unauthenticated_requests_per_period: 0, + throttle_unauthenticated_web_enabled: true, + throttle_unauthenticated_web_period_in_seconds: 123, + throttle_unauthenticated_web_requests_per_period: 456 + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include( + 'throttle_unauthenticated_enabled' => true, + 'throttle_unauthenticated_period_in_seconds' => 123, + 'throttle_unauthenticated_requests_per_period' => 456, + 'throttle_unauthenticated_web_enabled' => true, + 'throttle_unauthenticated_web_period_in_seconds' => 123, + 'throttle_unauthenticated_web_requests_per_period' => 456 + ) + end + it 'disables ability to switch to legacy storage' do put api("/application/settings", admin), params: { hashed_storage_enabled: false } @@ -552,5 +594,20 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['error']).to eq('whats_new_variant does not have a valid value') end end + + context 'sidekiq job limit settings' do + it 'updates the settings' do + settings = { + sidekiq_job_limiter_mode: 'track', + sidekiq_job_limiter_compression_threshold_bytes: 1, + sidekiq_job_limiter_limit_bytes: 2 + }.stringify_keys + + put api("/application/settings", admin), params: settings + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.slice(*settings.keys)).to eq(settings) + end + end end end diff --git a/spec/requests/api/terraform/modules/v1/packages_spec.rb b/spec/requests/api/terraform/modules/v1/packages_spec.rb index 6803c09b8c2..b04f5ad9a94 100644 --- a/spec/requests/api/terraform/modules/v1/packages_spec.rb +++ b/spec/requests/api/terraform/modules/v1/packages_spec.rb @@ -12,7 +12,7 @@ RSpec.describe API::Terraform::Modules::V1::Packages do let_it_be(:package) { create(:terraform_module_package, project: project) } let_it_be(:personal_access_token) { create(:personal_access_token) } let_it_be(:user) { personal_access_token.user } - let_it_be(:job) { create(:ci_build, :running, user: user) } + let_it_be(:job) { create(:ci_build, :running, user: user, project: project) } 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) } diff --git a/spec/requests/api/unleash_spec.rb b/spec/requests/api/unleash_spec.rb index 0718710f15c..6cb801538c6 100644 --- a/spec/requests/api/unleash_spec.rb +++ b/spec/requests/api/unleash_spec.rb @@ -176,25 +176,6 @@ RSpec.describe API::Unleash do it_behaves_like 'authenticated request' - context 'with version 1 (legacy) feature flags' do - let(:feature_flag) { create(:operations_feature_flag, :legacy_flag, project: project, name: 'feature1', active: true, version: 1) } - - it 'does not return a legacy feature flag' 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']).to be_empty - 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, diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 383940ce34a..527e548ad19 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -9,9 +9,13 @@ RSpec.describe API::Users do let_it_be(:gpg_key) { create(:gpg_key, user: user) } let_it_be(:email) { create(:email, user: user) } + let(:blocked_user) { create(:user, :blocked) } let(:omniauth_user) { create(:omniauth_user) } let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') } let(:private_user) { create(:user, private_profile: true) } + let(:deactivated_user) { create(:user, state: 'deactivated') } + let(:banned_user) { create(:user, :banned) } + let(:internal_user) { create(:user, :bot) } context 'admin notes' do let_it_be(:admin) { create(:admin, note: '2019-10-06 | 2FA added | user requested | www.gitlab.com') } @@ -1199,7 +1203,7 @@ RSpec.describe API::Users do it 'updates user with a new email' do old_email = user.email - old_notification_email = user.notification_email + old_notification_email = user.notification_email_or_default put api("/users/#{user.id}", admin), params: { email: 'new@email.com' } user.reload @@ -1207,7 +1211,7 @@ RSpec.describe API::Users do expect(response).to have_gitlab_http_status(:ok) expect(user).to be_confirmed expect(user.email).to eq(old_email) - expect(user.notification_email).to eq(old_notification_email) + expect(user.notification_email_or_default).to eq(old_notification_email) expect(user.unconfirmed_email).to eq('new@email.com') end @@ -2599,15 +2603,13 @@ RSpec.describe API::Users do let(:api_user) { admin } context 'for a deactivated user' do - before do - user.deactivate - end + let(:user_id) { deactivated_user.id } it 'activates a deactivated user' do activate expect(response).to have_gitlab_http_status(:created) - expect(user.reload.state).to eq('active') + expect(deactivated_user.reload.state).to eq('active') end end @@ -2625,16 +2627,14 @@ RSpec.describe API::Users do end context 'for a blocked user' do - before do - user.block - end + let(:user_id) { blocked_user.id } 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') + expect(blocked_user.reload.state).to eq('blocked') end end @@ -2711,29 +2711,25 @@ RSpec.describe API::Users do end context 'for a deactivated user' do - before do - user.deactivate - end + let(:user_id) { deactivated_user.id } it 'returns 201' do deactivate expect(response).to have_gitlab_http_status(:created) - expect(user.reload.state).to eq('deactivated') + expect(deactivated_user.reload.state).to eq('deactivated') end end context 'for a blocked user' do - before do - user.block - end + let(:user_id) { blocked_user.id } 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') + expect(blocked_user.reload.state).to eq('blocked') end end @@ -2775,7 +2771,9 @@ RSpec.describe API::Users do end end - context 'approve pending user' do + context 'approve and reject pending user' do + let(:pending_user) { create(:user, :blocked_pending_approval) } + shared_examples '404' do it 'returns 404' do expect(response).to have_gitlab_http_status(:not_found) @@ -2786,10 +2784,6 @@ RSpec.describe API::Users do describe 'POST /users/:id/approve' do subject(:approve) { post api("/users/#{user_id}/approve", api_user) } - let_it_be(:pending_user) { create(:user, :blocked_pending_approval) } - let_it_be(:deactivated_user) { create(:user, :deactivated) } - let_it_be(:blocked_user) { create(:user, :blocked) } - context 'performed by a non-admin user' do let(:api_user) { user } let(:user_id) { pending_user.id } @@ -2865,102 +2859,403 @@ RSpec.describe API::Users do end end end - end - describe 'POST /users/:id/block' do - let(:blocked_user) { create(:user, state: 'blocked') } + describe 'POST /users/:id/reject', :aggregate_failures do + subject(:reject) { post api("/users/#{user_id}/reject", api_user) } - it 'blocks existing user' do - post api("/users/#{user.id}/block", admin) + shared_examples 'returns 409' do + it 'returns 409' do + reject - aggregate_failures do - expect(response).to have_gitlab_http_status(:created) - expect(response.body).to eq('true') - expect(user.reload.state).to eq('blocked') + expect(response).to have_gitlab_http_status(:conflict) + expect(json_response['message']).to eq('User does not have a pending request') + end + end + + context 'performed by a non-admin user' do + let(:api_user) { user } + let(:user_id) { pending_user.id } + + it 'returns 403' do + expect { reject }.not_to change { pending_user.reload.state } + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('You are not allowed to reject a user') + end + end + + context 'performed by an admin user' do + let(:api_user) { admin } + + context 'for an pending approval user' do + let(:user_id) { pending_user.id } + + it 'returns 200' do + reject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['message']).to eq('Success') + end + end + + context 'for a deactivated user' do + let(:user_id) { deactivated_user.id } + + it 'does not reject a deactivated user' do + expect { reject }.not_to change { deactivated_user.reload.state } + end + + it_behaves_like 'returns 409' + end + + context 'for an active user' do + let(:user_id) { user.id } + + it 'does not reject an active user' do + expect { reject }.not_to change { user.reload.state } + end + + it_behaves_like 'returns 409' + end + + context 'for a blocked user' do + let(:user_id) { blocked_user.id } + + it 'does not reject a blocked user' do + expect { reject }.not_to change { blocked_user.reload.state } + end + + it_behaves_like 'returns 409' + end + + context 'for a ldap blocked user' do + let(:user_id) { ldap_blocked_user.id } + + it 'does not reject a ldap blocked user' do + expect { reject }.not_to change { ldap_blocked_user.reload.state } + end + + it_behaves_like 'returns 409' + end + + context 'for a user that does not exist' do + let(:user_id) { non_existing_record_id } + + before do + reject + end + + it_behaves_like '404' + end end end + end - it 'does not re-block ldap blocked users' do - post api("/users/#{ldap_blocked_user.id}/block", admin) - expect(response).to have_gitlab_http_status(:forbidden) - expect(ldap_blocked_user.reload.state).to eq('ldap_blocked') + describe 'POST /users/:id/block', :aggregate_failures do + context 'when admin' do + subject(:block_user) { post api("/users/#{user_id}/block", admin) } + + context 'with an existing user' do + let(:user_id) { user.id } + + it 'blocks existing user' do + block_user + + expect(response).to have_gitlab_http_status(:created) + expect(response.body).to eq('true') + expect(user.reload.state).to eq('blocked') + end + end + + context 'with an ldap blocked user' do + let(:user_id) { ldap_blocked_user.id } + + it 'does not re-block ldap blocked users' do + block_user + + expect(response).to have_gitlab_http_status(:forbidden) + expect(ldap_blocked_user.reload.state).to eq('ldap_blocked') + end + end + + context 'with a non existent user' do + let(:user_id) { non_existing_record_id } + + it 'does not block non existent user, returns 404' do + block_user + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 User Not Found') + end + end + + context 'with an internal user' do + let(:user_id) { internal_user.id } + + it 'does not block internal user, returns 403' do + block_user + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('An internal user cannot be blocked') + end + end + + context 'with a blocked user' do + let(:user_id) { blocked_user.id } + + it 'returns a 201 if user is already blocked' do + block_user + + expect(response).to have_gitlab_http_status(:created) + expect(response.body).to eq('null') + end + end end - it 'does not be available for non admin users' do + it 'is not available for non admin users' do post api("/users/#{user.id}/block", user) + expect(response).to have_gitlab_http_status(:forbidden) expect(user.reload.state).to eq('active') end + end - it 'returns a 404 error if user id not found' do - post api('/users/0/block', admin) - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 User Not Found') - end + describe 'POST /users/:id/unblock', :aggregate_failures do + context 'when admin' do + subject(:unblock_user) { post api("/users/#{user_id}/unblock", admin) } - it 'returns a 403 error if user is internal' do - internal_user = create(:user, :bot) + context 'with an existing user' do + let(:user_id) { user.id } - post api("/users/#{internal_user.id}/block", admin) + it 'unblocks existing user' do + unblock_user - expect(response).to have_gitlab_http_status(:forbidden) - expect(json_response['message']).to eq('An internal user cannot be blocked') - end + expect(response).to have_gitlab_http_status(:created) + expect(user.reload.state).to eq('active') + end + end - it 'returns a 201 if user is already blocked' do - post api("/users/#{blocked_user.id}/block", admin) + context 'with a blocked user' do + let(:user_id) { blocked_user.id } - aggregate_failures do - expect(response).to have_gitlab_http_status(:created) - expect(response.body).to eq('null') + it 'unblocks a blocked user' do + unblock_user + + expect(response).to have_gitlab_http_status(:created) + expect(blocked_user.reload.state).to eq('active') + end end - end - end - describe 'POST /users/:id/unblock' do - let(:blocked_user) { create(:user, state: 'blocked') } - let(:deactivated_user) { create(:user, state: 'deactivated') } + context 'with a ldap blocked user' do + let(:user_id) { ldap_blocked_user.id } - it 'unblocks existing user' do - post api("/users/#{user.id}/unblock", admin) - expect(response).to have_gitlab_http_status(:created) - expect(user.reload.state).to eq('active') - end + it 'does not unblock ldap blocked users' do + unblock_user - it 'unblocks a blocked user' do - post api("/users/#{blocked_user.id}/unblock", admin) - expect(response).to have_gitlab_http_status(:created) - expect(blocked_user.reload.state).to eq('active') + expect(response).to have_gitlab_http_status(:forbidden) + expect(ldap_blocked_user.reload.state).to eq('ldap_blocked') + end + end + + context 'with a deactivated user' do + let(:user_id) { deactivated_user.id } + + it 'does not unblock deactivated users' do + unblock_user + + expect(response).to have_gitlab_http_status(:forbidden) + expect(deactivated_user.reload.state).to eq('deactivated') + end + end + + context 'with a non existent user' do + let(:user_id) { non_existing_record_id } + + it 'returns a 404 error if user id not found' do + unblock_user + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 User Not Found') + end + end + + context 'with an invalid user id' do + let(:user_id) { 'ASDF' } + + it 'returns a 404' do + unblock_user + + expect(response).to have_gitlab_http_status(:not_found) + end + end end - it 'does not unblock ldap blocked users' do - post api("/users/#{ldap_blocked_user.id}/unblock", admin) + it 'is not available for non admin users' do + post api("/users/#{user.id}/unblock", user) expect(response).to have_gitlab_http_status(:forbidden) - expect(ldap_blocked_user.reload.state).to eq('ldap_blocked') + expect(user.reload.state).to eq('active') end + end - it 'does not unblock deactivated users' do - post api("/users/#{deactivated_user.id}/unblock", admin) - expect(response).to have_gitlab_http_status(:forbidden) - expect(deactivated_user.reload.state).to eq('deactivated') + describe 'POST /users/:id/ban', :aggregate_failures do + context 'when admin' do + subject(:ban_user) { post api("/users/#{user_id}/ban", admin) } + + context 'with an active user' do + let(:user_id) { user.id } + + it 'bans an active user' do + ban_user + + expect(response).to have_gitlab_http_status(:created) + expect(response.body).to eq('true') + expect(user.reload.state).to eq('banned') + end + end + + context 'with an ldap blocked user' do + let(:user_id) { ldap_blocked_user.id } + + it 'does not ban ldap blocked users' do + ban_user + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('You cannot ban ldap_blocked users.') + expect(ldap_blocked_user.reload.state).to eq('ldap_blocked') + end + end + + context 'with a deactivated user' do + let(:user_id) { deactivated_user.id } + + it 'does not ban deactivated users' do + ban_user + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('You cannot ban deactivated users.') + expect(deactivated_user.reload.state).to eq('deactivated') + end + end + + context 'with a banned user' do + let(:user_id) { banned_user.id } + + it 'does not ban banned users' do + ban_user + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('You cannot ban banned users.') + expect(banned_user.reload.state).to eq('banned') + end + end + + context 'with a non existent user' do + let(:user_id) { non_existing_record_id } + + it 'does not ban non existent users' do + ban_user + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 User Not Found') + end + end + + context 'with an invalid id' do + let(:user_id) { 'ASDF' } + + it 'does not ban invalid id users' do + ban_user + + expect(response).to have_gitlab_http_status(:not_found) + end + end end - it 'is not available for non admin users' do - post api("/users/#{user.id}/unblock", user) + it 'is not available for non-admin users' do + post api("/users/#{user.id}/ban", user) + expect(response).to have_gitlab_http_status(:forbidden) expect(user.reload.state).to eq('active') end + end - it 'returns a 404 error if user id not found' do - post api('/users/0/block', admin) - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 User Not Found') + describe 'POST /users/:id/unban', :aggregate_failures do + context 'when admin' do + subject(:unban_user) { post api("/users/#{user_id}/unban", admin) } + + context 'with a banned user' do + let(:user_id) { banned_user.id } + + it 'activates a banned user' do + unban_user + + expect(response).to have_gitlab_http_status(:created) + expect(banned_user.reload.state).to eq('active') + end + end + + context 'with an ldap_blocked user' do + let(:user_id) { ldap_blocked_user.id } + + it 'does not unban ldap_blocked users' do + unban_user + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('You cannot unban ldap_blocked users.') + expect(ldap_blocked_user.reload.state).to eq('ldap_blocked') + end + end + + context 'with a deactivated user' do + let(:user_id) { deactivated_user.id } + + it 'does not unban deactivated users' do + unban_user + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('You cannot unban deactivated users.') + expect(deactivated_user.reload.state).to eq('deactivated') + end + end + + context 'with an active user' do + let(:user_id) { user.id } + + it 'does not unban active users' do + unban_user + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('You cannot unban active users.') + expect(user.reload.state).to eq('active') + end + end + + context 'with a non existent user' do + let(:user_id) { non_existing_record_id } + + it 'does not unban non existent users' do + unban_user + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 User Not Found') + end + end + + context 'with an invalid id user' do + let(:user_id) { 'ASDF' } + + it 'does not unban invalid id users' do + unban_user + + expect(response).to have_gitlab_http_status(:not_found) + end + end end - it "returns a 404 for invalid ID" do - post api("/users/ASDF/block", admin) + it 'is not available for non admin users' do + post api("/users/#{banned_user.id}/unban", user) - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:forbidden) + expect(user.reload.state).to eq('active') end end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index e4a0c034b20..a16f5abf608 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -882,6 +882,10 @@ RSpec.describe 'Git HTTP requests' do before do build.update!(user: user) project.add_reporter(user) + create(:ci_job_token_project_scope_link, + source_project: project, + target_project: other_project, + added_by: user) end shared_examples 'can download code only' do @@ -1447,6 +1451,10 @@ RSpec.describe 'Git HTTP requests' do before do build.update!(project: project) # can't associate it on factory create + create(:ci_job_token_project_scope_link, + source_project: project, + target_project: other_project, + added_by: user) end context 'when build created by system is authenticated' do diff --git a/spec/requests/jira_connect/installations_controller_spec.rb b/spec/requests/jira_connect/installations_controller_spec.rb new file mode 100644 index 00000000000..6315c66a41a --- /dev/null +++ b/spec/requests/jira_connect/installations_controller_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::InstallationsController do + let_it_be(:installation) { create(:jira_connect_installation) } + + describe 'GET /-/jira_connect/installations' do + before do + get '/-/jira_connect/installations', params: { jwt: jwt } + end + + context 'without JWT' do + let(:jwt) { nil } + + it 'returns 403' do + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'with valid JWT' do + let(:qsh) { Atlassian::Jwt.create_query_string_hash('https://gitlab.test/installations', 'GET', 'https://gitlab.test') } + let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key, qsh: qsh }, installation.shared_secret) } + + it 'returns status ok' do + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns the installation as json' do + expect(json_response).to eq({ + 'gitlab_com' => true, + 'instance_url' => nil + }) + end + + context 'with instance_url' do + let_it_be(:installation) { create(:jira_connect_installation, instance_url: 'https://example.com') } + + it 'returns the installation as json' do + expect(json_response).to eq({ + 'gitlab_com' => false, + 'instance_url' => 'https://example.com' + }) + end + end + end + end + + describe 'PUT /-/jira_connect/installations' do + before do + put '/-/jira_connect/installations', params: { jwt: jwt, installation: { instance_url: update_instance_url } } + end + + let(:update_instance_url) { 'https://example.com' } + + context 'without JWT' do + let(:jwt) { nil } + + it 'returns 403' do + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'with valid JWT' do + let(:qsh) { Atlassian::Jwt.create_query_string_hash('https://gitlab.test/subscriptions', 'GET', 'https://gitlab.test') } + let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key, qsh: qsh }, installation.shared_secret) } + + it 'returns 200' do + expect(response).to have_gitlab_http_status(:ok) + end + + it 'updates the instance_url' do + expect(json_response).to eq({ + 'gitlab_com' => false, + 'instance_url' => 'https://example.com' + }) + end + + context 'invalid URL' do + let(:update_instance_url) { 'invalid url' } + + it 'returns 422 and errors', :aggregate_failures do + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response).to eq({ + 'errors' => { + 'instance_url' => [ + 'is blocked: Only allowed schemes are http, https' + ] + } + }) + end + end + end + end +end diff --git a/spec/requests/members/mailgun/permanent_failure_spec.rb b/spec/requests/members/mailgun/permanent_failure_spec.rb new file mode 100644 index 00000000000..e47aedf8e94 --- /dev/null +++ b/spec/requests/members/mailgun/permanent_failure_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'receive a permanent failure' do + describe 'POST /members/mailgun/permanent_failures', :aggregate_failures do + let_it_be(:member) { create(:project_member, :invited) } + + let(:raw_invite_token) { member.raw_invite_token } + let(:mailgun_events) { true } + let(:mailgun_signing_key) { 'abc123' } + + subject(:post_request) { post members_mailgun_permanent_failures_path(standard_params) } + + before do + stub_application_setting(mailgun_events_enabled: mailgun_events, mailgun_signing_key: mailgun_signing_key) + end + + it 'marks the member invite email success as false' do + expect { post_request }.to change { member.reload.invite_email_success }.from(true).to(false) + + expect(response).to have_gitlab_http_status(:ok) + end + + context 'when the change to a member is not made' do + context 'with incorrect signing key' do + context 'with incorrect signing key' do + let(:mailgun_signing_key) { '_foobar_' } + + it 'does not change member status and responds as not_found' do + expect { post_request }.not_to change { member.reload.invite_email_success } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with nil signing key' do + let(:mailgun_signing_key) { nil } + + it 'does not change member status and responds as not_found' do + expect { post_request }.not_to change { member.reload.invite_email_success } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when the feature is not enabled' do + let(:mailgun_events) { false } + + it 'does not change member status and responds as expected' do + expect { post_request }.not_to change { member.reload.invite_email_success } + + expect(response).to have_gitlab_http_status(:not_acceptable) + end + end + + context 'when it is not an invite email' do + before do + stub_const('::Members::Mailgun::INVITE_EMAIL_TAG', '_foobar_') + end + + it 'does not change member status and responds as expected' do + expect { post_request }.not_to change { member.reload.invite_email_success } + + expect(response).to have_gitlab_http_status(:not_acceptable) + end + end + end + + def standard_params + { + "signature": { + "timestamp": "1625056677", + "token": "eb944d0ace7227667a1b97d2d07276ae51d2b849ed2cfa68f3", + "signature": "9790cc6686eb70f0b1f869180d906870cdfd496d27fee81da0aa86b9e539e790" + }, + "event-data": { + "severity": "permanent", + "tags": ["invite_email"], + "timestamp": 1521233195.375624, + "storage": { + "url": "_anything_", + "key": "_anything_" + }, + "log-level": "error", + "id": "_anything_", + "campaigns": [], + "reason": "suppress-bounce", + "user-variables": { + "invite_token": raw_invite_token + }, + "flags": { + "is-routed": false, + "is-authenticated": true, + "is-system-test": false, + "is-test-mode": false + }, + "recipient-domain": "example.com", + "envelope": { + "sender": "bob@mg.gitlab.com", + "transport": "smtp", + "targets": "alice@example.com" + }, + "message": { + "headers": { + "to": "Alice <alice@example.com>", + "message-id": "20130503192659.13651.20287@mg.gitlab.com", + "from": "Bob <bob@mg.gitlab.com>", + "subject": "Test permanent_fail webhook" + }, + "attachments": [], + "size": 111 + }, + "recipient": "alice@example.com", + "event": "failed", + "delivery-status": { + "attempt-no": 1, + "message": "", + "code": 605, + "description": "Not delivering to previously bounced address", + "session-seconds": 0 + } + } + } + end + end +end diff --git a/spec/requests/oauth_tokens_spec.rb b/spec/requests/oauth_tokens_spec.rb index 6d944bbc783..fdcc76f42cc 100644 --- a/spec/requests/oauth_tokens_spec.rb +++ b/spec/requests/oauth_tokens_spec.rb @@ -55,5 +55,29 @@ RSpec.describe 'OAuth Tokens requests' do expect(json_response['access_token']).not_to be_nil end + + context 'when the application is configured to use expiring tokens' do + before do + application.update!(expire_access_tokens: true) + end + + it 'generates an access token with an expiration' do + request_access_token(user) + + expect(json_response['expires_in']).not_to be_nil + end + end + + context 'when the application is configured not to use expiring tokens' do + before do + application.update!(expire_access_tokens: false) + end + + it 'generates an access token without an expiration' do + request_access_token(user) + + expect(json_response.key?('expires_in')).to eq(false) + end + end end end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 5bf786f2290..5ec23382698 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -149,7 +149,15 @@ RSpec.describe 'OpenID Connect requests' do end context 'ID token payload' do + let!(:group1) { create :group } + let!(:group2) { create :group } + let!(:group3) { create :group, parent: group2 } + let!(:group4) { create :group, parent: group3 } + before do + group1.add_user(user, Gitlab::Access::OWNER) + group3.add_user(user, Gitlab::Access::DEVELOPER) + request_access_token! @payload = JSON::JWT.decode(json_response['id_token'], :skip_verification) end @@ -175,7 +183,12 @@ RSpec.describe 'OpenID Connect requests' do end it 'does not include any unknown properties' do - expect(@payload.keys).to eq %w[iss sub aud exp iat auth_time sub_legacy email email_verified] + expect(@payload.keys).to eq %w[iss sub aud exp iat auth_time sub_legacy email email_verified groups_direct] + end + + it 'does include groups' do + expected_groups = [group1.full_path, group3.full_path] + expect(@payload['groups_direct']).to match_array(expected_groups) end end @@ -331,7 +344,15 @@ RSpec.describe 'OpenID Connect requests' do end context 'ID token payload' do + let!(:group1) { create :group } + let!(:group2) { create :group } + let!(:group3) { create :group, parent: group2 } + let!(:group4) { create :group, parent: group3 } + before do + group1.add_user(user, Gitlab::Access::OWNER) + group3.add_user(user, Gitlab::Access::DEVELOPER) + request_access_token! @payload = JSON::JWT.decode(json_response['id_token'], :skip_verification) end @@ -343,6 +364,11 @@ RSpec.describe 'OpenID Connect requests' do it 'has true in email_verified claim' do expect(@payload['email_verified']).to eq(true) end + + it 'does include groups' do + expected_groups = [group1.full_path, group3.full_path] + expect(@payload['groups_direct']).to match_array(expected_groups) + end end end end diff --git a/spec/requests/projects/merge_requests_discussions_spec.rb b/spec/requests/projects/merge_requests_discussions_spec.rb index c68745b9271..8057a091bba 100644 --- a/spec/requests/projects/merge_requests_discussions_spec.rb +++ b/spec/requests/projects/merge_requests_discussions_spec.rb @@ -59,6 +59,7 @@ RSpec.describe 'merge requests discussions' do let!(:first_note) { create(:diff_note_on_merge_request, author: author, noteable: merge_request, project: project, note: "reference: #{reference.to_reference}") } let!(:second_note) { create(:diff_note_on_merge_request, in_reply_to: first_note, noteable: merge_request, project: project) } let!(:award_emoji) { create(:award_emoji, awardable: first_note) } + let!(:author_membership) { project.add_maintainer(author) } before do # Make a request to cache the discussions @@ -229,6 +230,16 @@ RSpec.describe 'merge requests discussions' do end end + context 'when author role changes' do + before do + Members::UpdateService.new(user, access_level: Gitlab::Access::GUEST).execute(author_membership) + end + + it_behaves_like 'cache miss' do + let(:changed_notes) { [first_note, second_note] } + end + end + context 'when merge_request_discussion_cache is disabled' do before do stub_feature_flags(merge_request_discussion_cache: false) diff --git a/spec/requests/projects/usage_quotas_spec.rb b/spec/requests/projects/usage_quotas_spec.rb new file mode 100644 index 00000000000..04e01da61ef --- /dev/null +++ b/spec/requests/projects/usage_quotas_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Project Usage Quotas' do + let_it_be(:project) { create(:project) } + let_it_be(:role) { :maintainer } + let_it_be(:user) { create(:user) } + + before do + project.add_role(user, role) + login_as(user) + end + + shared_examples 'response with 404 status' do + it 'renders :not_found' do + get project_usage_quotas_path(project) + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.body).not_to include(project_usage_quotas_path(project)) + end + end + + describe 'GET /:namespace/:project/usage_quotas' do + context 'with project_storage_ui feature flag enabled' do + before do + stub_feature_flags(project_storage_ui: true) + end + + it 'renders usage quotas path' do + mock_storage_app_data = { + project_path: project.full_path, + usage_quotas_help_page_path: help_page_path('user/usage_quotas'), + build_artifacts_help_page_path: help_page_path('ci/pipelines/job_artifacts', anchor: 'when-job-artifacts-are-deleted'), + packages_help_page_path: help_page_path('user/packages/package_registry/index.md', anchor: 'delete-a-package'), + repository_help_page_path: help_page_path('user/project/repository/reducing_the_repo_size_using_git'), + snippets_help_page_path: help_page_path('user/snippets', anchor: 'reduce-snippets-repository-size'), + wiki_help_page_path: help_page_path('administration/wikis/index.md', anchor: 'reduce-wiki-repository-size') + } + get project_usage_quotas_path(project) + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to include(project_usage_quotas_path(project)) + expect(assigns[:storage_app_data]).to eq(mock_storage_app_data) + expect(response.body).to include("Usage of project resources across the <strong>#{project.name}</strong> project") + end + + context 'renders :not_found for user without permission' do + let(:role) { :developer } + + it_behaves_like 'response with 404 status' + end + end + + context 'with project_storage_ui feature flag disabled' do + before do + stub_feature_flags(project_storage_ui: false) + end + + it_behaves_like 'response with 404 status' + end + end +end diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index a0f9d4c11ed..87ef6fa1a18 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -11,6 +11,8 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac # the right settings are being exercised let(:settings_to_set) do { + throttle_unauthenticated_api_requests_per_period: 100, + throttle_unauthenticated_api_period_in_seconds: 1, throttle_unauthenticated_requests_per_period: 100, throttle_unauthenticated_period_in_seconds: 1, throttle_authenticated_api_requests_per_period: 100, @@ -22,7 +24,13 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac throttle_unauthenticated_packages_api_requests_per_period: 100, throttle_unauthenticated_packages_api_period_in_seconds: 1, throttle_authenticated_packages_api_requests_per_period: 100, - throttle_authenticated_packages_api_period_in_seconds: 1 + throttle_authenticated_packages_api_period_in_seconds: 1, + throttle_authenticated_git_lfs_requests_per_period: 100, + throttle_authenticated_git_lfs_period_in_seconds: 1, + throttle_unauthenticated_files_api_requests_per_period: 100, + throttle_unauthenticated_files_api_period_in_seconds: 1, + throttle_authenticated_files_api_requests_per_period: 100, + throttle_authenticated_files_api_period_in_seconds: 1 } end @@ -33,186 +41,21 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac include_context 'rack attack cache store' - describe 'unauthenticated requests' do - let(:url_that_does_not_require_authentication) { '/users/sign_in' } - let(:url_api_internal) { '/api/v4/internal/check' } - - before do - # Disabling protected paths throttle, otherwise requests to - # '/users/sign_in' are caught by this throttle. - settings_to_set[:throttle_protected_paths_enabled] = false - - # Set low limits - settings_to_set[:throttle_unauthenticated_requests_per_period] = requests_per_period - settings_to_set[:throttle_unauthenticated_period_in_seconds] = period_in_seconds - end - - context 'when the throttle is enabled' do - before do - settings_to_set[:throttle_unauthenticated_enabled] = true - stub_application_setting(settings_to_set) - end - - it 'rejects requests over the rate limit' do - # At first, allow requests under the rate limit. - requests_per_period.times do - get url_that_does_not_require_authentication - expect(response).to have_gitlab_http_status(:ok) - end - - # the last straw - expect_rejection { get url_that_does_not_require_authentication } - end - - context 'with custom response text' do - before do - stub_application_setting(rate_limiting_response_text: 'Custom response') - end - - it 'rejects requests over the rate limit' do - # At first, allow requests under the rate limit. - requests_per_period.times do - get url_that_does_not_require_authentication - expect(response).to have_gitlab_http_status(:ok) - end - - # the last straw - expect_rejection { get url_that_does_not_require_authentication } - expect(response.body).to eq("Custom response\n") - end - end - - it 'allows requests after throttling and then waiting for the next period' do - requests_per_period.times do - get url_that_does_not_require_authentication - expect(response).to have_gitlab_http_status(:ok) - end - - expect_rejection { get url_that_does_not_require_authentication } - - travel_to(period.from_now) do - requests_per_period.times do - get url_that_does_not_require_authentication - expect(response).to have_gitlab_http_status(:ok) - end - - expect_rejection { get url_that_does_not_require_authentication } - end - end - - it 'counts requests from different IPs separately' do - requests_per_period.times do - get url_that_does_not_require_authentication - expect(response).to have_gitlab_http_status(:ok) - end - - expect_next_instance_of(Rack::Attack::Request) do |instance| - expect(instance).to receive(:ip).at_least(:once).and_return('1.2.3.4') - end - - # would be over limit for the same IP - get url_that_does_not_require_authentication - expect(response).to have_gitlab_http_status(:ok) - end - - context 'when the request is to the api internal endpoints' do - it 'allows requests over the rate limit' do - (1 + requests_per_period).times do - get url_api_internal, params: { secret_token: Gitlab::Shell.secret_token } - expect(response).to have_gitlab_http_status(:ok) - end - end - end - - context 'when the request is authenticated by a runner token' do - let(:request_jobs_url) { '/api/v4/jobs/request' } - let(:runner) { create(:ci_runner) } - - it 'does not count as unauthenticated' do - (1 + requests_per_period).times do - post request_jobs_url, params: { token: runner.token } - expect(response).to have_gitlab_http_status(:no_content) - end - end - end - - context 'when the request is to a health endpoint' do - let(:health_endpoint) { '/-/metrics' } - - it 'does not throttle the requests' do - (1 + requests_per_period).times do - get health_endpoint - expect(response).to have_gitlab_http_status(:ok) - end - end - end - - context 'when the request is to a container registry notification endpoint' do - let(:secret_token) { 'secret_token' } - let(:events) { [{ action: 'push' }] } - let(:registry_endpoint) { '/api/v4/container_registry_event/events' } - let(:registry_headers) { { 'Content-Type' => ::API::ContainerRegistryEvent::DOCKER_DISTRIBUTION_EVENTS_V1_JSON } } - - before do - allow(Gitlab.config.registry).to receive(:notification_secret) { secret_token } - - event = spy(:event) - allow(::ContainerRegistry::Event).to receive(:new).and_return(event) - allow(event).to receive(:supported?).and_return(true) - end - - it 'does not throttle the requests' do - (1 + requests_per_period).times do - post registry_endpoint, - params: { events: events }.to_json, - headers: registry_headers.merge('Authorization' => secret_token) - - expect(response).to have_gitlab_http_status(:ok) - end - end - end - - it 'logs RackAttack info into structured logs' do - requests_per_period.times do - get url_that_does_not_require_authentication - expect(response).to have_gitlab_http_status(:ok) - end - - arguments = a_hash_including({ - message: 'Rack_Attack', - env: :throttle, - remote_ip: '127.0.0.1', - request_method: 'GET', - path: '/users/sign_in', - matched: 'throttle_unauthenticated' - }) - - expect(Gitlab::AuthLogger).to receive(:error).with(arguments) - - get url_that_does_not_require_authentication - end - - it_behaves_like 'tracking when dry-run mode is set' do - let(:throttle_name) { 'throttle_unauthenticated' } - - def do_request - get url_that_does_not_require_authentication - end - end + describe 'unauthenticated API requests' do + it_behaves_like 'rate-limited unauthenticated requests' do + let(:throttle_name) { 'throttle_unauthenticated_api' } + let(:throttle_setting_prefix) { 'throttle_unauthenticated_api' } + let(:url_that_does_not_require_authentication) { '/api/v4/projects' } + let(:url_that_is_not_matched) { '/users/sign_in' } end + end - context 'when the throttle is disabled' do - before do - settings_to_set[:throttle_unauthenticated_enabled] = false - stub_application_setting(settings_to_set) - end - - it 'allows requests over the rate limit' do - (1 + requests_per_period).times do - get url_that_does_not_require_authentication - expect(response).to have_gitlab_http_status(:ok) - end - end + describe 'unauthenticated web requests' do + it_behaves_like 'rate-limited unauthenticated requests' do + let(:throttle_name) { 'throttle_unauthenticated_web' } + let(:throttle_setting_prefix) { 'throttle_unauthenticated' } + let(:url_that_does_not_require_authentication) { '/users/sign_in' } + let(:url_that_is_not_matched) { '/api/v4/projects' } end end @@ -473,9 +316,9 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac context 'when unauthenticated api throttle is enabled' do before do - settings_to_set[:throttle_unauthenticated_requests_per_period] = requests_per_period - settings_to_set[:throttle_unauthenticated_period_in_seconds] = period_in_seconds - settings_to_set[:throttle_unauthenticated_enabled] = true + settings_to_set[:throttle_unauthenticated_api_requests_per_period] = requests_per_period + settings_to_set[:throttle_unauthenticated_api_period_in_seconds] = period_in_seconds + settings_to_set[:throttle_unauthenticated_api_enabled] = true stub_application_setting(settings_to_set) end @@ -488,6 +331,22 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac expect_rejection { do_request } end end + + context 'when unauthenticated web throttle is enabled' do + before do + settings_to_set[:throttle_unauthenticated_web_requests_per_period] = requests_per_period + settings_to_set[:throttle_unauthenticated_web_period_in_seconds] = period_in_seconds + settings_to_set[:throttle_unauthenticated_web_enabled] = true + stub_application_setting(settings_to_set) + end + + it 'ignores unauthenticated web throttle' do + (1 + requests_per_period).times do + do_request + expect(response).to have_gitlab_http_status(:ok) + end + end + end end context 'when unauthenticated packages api throttle is enabled' do @@ -509,9 +368,9 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac context 'when unauthenticated api throttle is lower' do before do - settings_to_set[:throttle_unauthenticated_requests_per_period] = 0 - settings_to_set[:throttle_unauthenticated_period_in_seconds] = period_in_seconds - settings_to_set[:throttle_unauthenticated_enabled] = true + settings_to_set[:throttle_unauthenticated_api_requests_per_period] = 0 + settings_to_set[:throttle_unauthenticated_api_period_in_seconds] = period_in_seconds + settings_to_set[:throttle_unauthenticated_api_enabled] = true stub_application_setting(settings_to_set) end @@ -620,6 +479,317 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac end end + describe 'authenticated git lfs requests', :api do + let_it_be(:project) { create(:project, :internal) } + let_it_be(:user) { create(:user) } + let_it_be(:token) { create(:personal_access_token, user: user) } + let_it_be(:other_user) { create(:user) } + let_it_be(:other_user_token) { create(:personal_access_token, user: other_user) } + + let(:request_method) { 'GET' } + let(:throttle_setting_prefix) { 'throttle_authenticated_git_lfs' } + let(:git_lfs_url) { "/#{project.full_path}.git/info/lfs/locks" } + + before do + allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) + stub_application_setting(settings_to_set) + end + + context 'with regular login' do + let(:url_that_requires_authentication) { git_lfs_url } + + it_behaves_like 'rate-limited web authenticated requests' + end + + context 'with the token in the headers' do + let(:request_args) { [git_lfs_url, { headers: basic_auth_headers(user, token) }] } + let(:other_user_request_args) { [git_lfs_url, { headers: basic_auth_headers(other_user, other_user_token) }] } + + it_behaves_like 'rate-limited token-authenticated requests' + end + + context 'precedence over authenticated web throttle' do + before do + settings_to_set[:throttle_authenticated_git_lfs_requests_per_period] = requests_per_period + settings_to_set[:throttle_authenticated_git_lfs_period_in_seconds] = period_in_seconds + end + + def do_request + get git_lfs_url, headers: basic_auth_headers(user, token) + end + + context 'when authenticated git lfs throttle is enabled' do + before do + settings_to_set[:throttle_authenticated_git_lfs_enabled] = true + end + + context 'when authenticated web throttle is lower' do + before do + settings_to_set[:throttle_authenticated_web_requests_per_period] = 0 + settings_to_set[:throttle_authenticated_web_period_in_seconds] = period_in_seconds + settings_to_set[:throttle_authenticated_web_enabled] = true + stub_application_setting(settings_to_set) + end + + it 'ignores authenticated web throttle' do + requests_per_period.times do + do_request + expect(response).to have_gitlab_http_status(:ok) + end + + expect_rejection { do_request } + end + end + end + + context 'when authenticated git lfs throttle is disabled' do + before do + settings_to_set[:throttle_authenticated_git_lfs_enabled] = false + end + + context 'when authenticated web throttle is enabled' do + before do + settings_to_set[:throttle_authenticated_web_requests_per_period] = requests_per_period + settings_to_set[:throttle_authenticated_web_period_in_seconds] = period_in_seconds + settings_to_set[:throttle_authenticated_web_enabled] = true + stub_application_setting(settings_to_set) + end + + it 'rejects requests over the authenticated web rate limit' do + requests_per_period.times do + do_request + expect(response).to have_gitlab_http_status(:ok) + end + + expect_rejection { do_request } + end + end + end + end + end + + describe 'Files API' do + let(:request_method) { 'GET' } + + context 'unauthenticated' do + let_it_be(:project) { create(:project, :public, :custom_repo, files: { 'README' => 'foo' }) } + + let(:throttle_setting_prefix) { 'throttle_unauthenticated_files_api' } + let(:files_path_that_does_not_require_authentication) { "/api/v4/projects/#{project.id}/repository/files/README?ref=master" } + + def do_request + get files_path_that_does_not_require_authentication + end + + before do + settings_to_set[:throttle_unauthenticated_files_api_requests_per_period] = requests_per_period + settings_to_set[:throttle_unauthenticated_files_api_period_in_seconds] = period_in_seconds + end + + context 'when unauthenticated files api throttle is disabled' do + before do + settings_to_set[:throttle_unauthenticated_files_api_enabled] = false + stub_application_setting(settings_to_set) + end + + it 'allows requests over the rate limit' do + (1 + requests_per_period).times do + do_request + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when unauthenticated api throttle is enabled' do + before do + settings_to_set[:throttle_unauthenticated_api_requests_per_period] = requests_per_period + settings_to_set[:throttle_unauthenticated_api_period_in_seconds] = period_in_seconds + settings_to_set[:throttle_unauthenticated_api_enabled] = true + stub_application_setting(settings_to_set) + end + + it 'rejects requests over the unauthenticated api rate limit' do + requests_per_period.times do + do_request + expect(response).to have_gitlab_http_status(:ok) + end + + expect_rejection { do_request } + end + end + + context 'when unauthenticated web throttle is enabled' do + before do + settings_to_set[:throttle_unauthenticated_web_requests_per_period] = requests_per_period + settings_to_set[:throttle_unauthenticated_web_period_in_seconds] = period_in_seconds + settings_to_set[:throttle_unauthenticated_web_enabled] = true + stub_application_setting(settings_to_set) + end + + it 'ignores unauthenticated web throttle' do + (1 + requests_per_period).times do + do_request + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end + + context 'when unauthenticated files api throttle is enabled' do + before do + settings_to_set[:throttle_unauthenticated_files_api_requests_per_period] = requests_per_period # 1 + settings_to_set[:throttle_unauthenticated_files_api_period_in_seconds] = period_in_seconds # 10_000 + settings_to_set[:throttle_unauthenticated_files_api_enabled] = true + stub_application_setting(settings_to_set) + end + + it 'rejects requests over the rate limit' do + requests_per_period.times do + do_request + expect(response).to have_gitlab_http_status(:ok) + end + + expect_rejection { do_request } + end + + context 'when feature flag is off' do + before do + stub_feature_flags(files_api_throttling: false) + end + + it 'allows requests over the rate limit' do + (1 + requests_per_period).times do + do_request + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + context 'when unauthenticated api throttle is lower' do + before do + settings_to_set[:throttle_unauthenticated_api_requests_per_period] = 0 + settings_to_set[:throttle_unauthenticated_api_period_in_seconds] = period_in_seconds + settings_to_set[:throttle_unauthenticated_api_enabled] = true + stub_application_setting(settings_to_set) + end + + it 'ignores unauthenticated api throttle' do + requests_per_period.times do + do_request + expect(response).to have_gitlab_http_status(:ok) + end + + expect_rejection { do_request } + end + end + + it_behaves_like 'tracking when dry-run mode is set' do + let(:throttle_name) { 'throttle_unauthenticated_files_api' } + end + end + end + + context 'authenticated', :api do + let_it_be(:project) { create(:project, :internal, :custom_repo, files: { 'README' => 'foo' }) } + let_it_be(:user) { create(:user) } + let_it_be(:token) { create(:personal_access_token, user: user) } + let_it_be(:other_user) { create(:user) } + let_it_be(:other_user_token) { create(:personal_access_token, user: other_user) } + + let(:throttle_setting_prefix) { 'throttle_authenticated_files_api' } + let(:api_partial_url) { "/projects/#{project.id}/repository/files/README?ref=master" } + + before do + stub_application_setting(settings_to_set) + end + + context 'with the token in the query string' do + let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } + let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } + + it_behaves_like 'rate-limited token-authenticated requests' + end + + context 'with the token in the headers' do + let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } + let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } + + it_behaves_like 'rate-limited token-authenticated requests' + end + + context 'precedence over authenticated api throttle' do + before do + settings_to_set[:throttle_authenticated_files_api_requests_per_period] = requests_per_period + settings_to_set[:throttle_authenticated_files_api_period_in_seconds] = period_in_seconds + end + + def do_request + get api(api_partial_url, personal_access_token: token) + end + + context 'when authenticated files api throttle is enabled' do + before do + settings_to_set[:throttle_authenticated_files_api_enabled] = true + end + + context 'when authenticated api throttle is lower' do + before do + settings_to_set[:throttle_authenticated_api_requests_per_period] = 0 + settings_to_set[:throttle_authenticated_api_period_in_seconds] = period_in_seconds + settings_to_set[:throttle_authenticated_api_enabled] = true + stub_application_setting(settings_to_set) + end + + it 'ignores authenticated api throttle' do + requests_per_period.times do + do_request + expect(response).to have_gitlab_http_status(:ok) + end + + expect_rejection { do_request } + end + end + + context 'when feature flag is off' do + before do + stub_feature_flags(files_api_throttling: false) + end + + it 'allows requests over the rate limit' do + (1 + requests_per_period).times do + do_request + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end + + context 'when authenticated files api throttle is disabled' do + before do + settings_to_set[:throttle_authenticated_files_api_enabled] = false + end + + context 'when authenticated api throttle is enabled' do + before do + settings_to_set[:throttle_authenticated_api_requests_per_period] = requests_per_period + settings_to_set[:throttle_authenticated_api_period_in_seconds] = period_in_seconds + settings_to_set[:throttle_authenticated_api_enabled] = true + stub_application_setting(settings_to_set) + end + + it 'rejects requests over the authenticated api rate limit' do + requests_per_period.times do + do_request + expect(response).to have_gitlab_http_status(:ok) + end + + expect_rejection { do_request } + end + end + end + end + end + end + describe 'throttle bypass header' do let(:headers) { {} } let(:bypass_header) { 'gitlab-bypass-rate-limiting' } diff --git a/spec/requests/users/group_callouts_spec.rb b/spec/requests/users/group_callouts_spec.rb new file mode 100644 index 00000000000..a8680c3add4 --- /dev/null +++ b/spec/requests/users/group_callouts_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Group callouts' do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + + before do + sign_in(user) + end + + describe 'POST /-/users/group_callouts' do + let(:params) { { feature_name: feature_name, group_id: group.id } } + + subject { post group_callouts_path, params: params, headers: { 'ACCEPT' => 'application/json' } } + + context 'with valid feature name and group' do + let(:feature_name) { Users::GroupCallout.feature_names.each_key.first } + + context 'when callout entry does not exist' do + it 'creates a callout entry with dismissed state' do + expect { subject }.to change { Users::GroupCallout.count }.by(1) + end + + it 'returns success' do + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when callout entry already exists' do + let!(:callout) do + create(:group_callout, + feature_name: Users::GroupCallout.feature_names.each_key.first, + user: user, + group: group) + end + + it 'returns success', :aggregate_failures do + expect { subject }.not_to change { Users::GroupCallout.count } + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + context 'with invalid feature name' do + let(:feature_name) { 'bogus_feature_name' } + + it 'returns bad request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end +end |