diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-09-20 14:18:08 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-09-20 14:18:08 +0300 |
commit | 5afcbe03ead9ada87621888a31a62652b10a7e4f (patch) | |
tree | 9918b67a0d0f0bafa6542e839a8be37adf73102d /spec/requests | |
parent | c97c0201564848c1f53226fe19d71fdcc472f7d0 (diff) |
Add latest changes from gitlab-org/gitlab@16-4-stable-eev16.4.0-rc42
Diffstat (limited to 'spec/requests')
75 files changed, 2339 insertions, 863 deletions
diff --git a/spec/requests/admin/abuse_reports_controller_spec.rb b/spec/requests/admin/abuse_reports_controller_spec.rb index c443a441af8..c25fb18e5b8 100644 --- a/spec/requests/admin/abuse_reports_controller_spec.rb +++ b/spec/requests/admin/abuse_reports_controller_spec.rb @@ -30,16 +30,14 @@ RSpec.describe Admin::AbuseReportsController, type: :request, feature_category: expect(assigns(:abuse_reports).first.closed?).to eq true end - context 'when abuse_reports_list flag is disabled' do - before do - stub_feature_flags(abuse_reports_list: false) - end + it 'labels does not introduce N+1 queries' do + get admin_abuse_reports_path # warm up - it 'returns all reports by default' do - get admin_abuse_reports_path + control = ActiveRecord::QueryRecorder.new { get admin_abuse_reports_path } - expect(assigns(:abuse_reports).count).to eq 2 - end + create_list(:abuse_report, 2) + + expect { get admin_abuse_reports_path }.to issue_same_number_of_queries_as(control).ignoring_cached_queries end end @@ -53,13 +51,62 @@ RSpec.describe Admin::AbuseReportsController, type: :request, feature_category: end end - shared_examples 'moderates user' do + describe 'PUT #update' do + let_it_be(:report) { create(:abuse_report) } + let_it_be(:label1) { create(:abuse_report_label, title: 'Uno') } + + let(:params) { { label_ids: [Gitlab::GlobalId.build(label1, id: label1.id).to_s] } } + let(:expected_params) { ActionController::Parameters.new(params).permit! } + + subject(:request) { put admin_abuse_report_path(report, params) } + + it 'invokes the Admin::AbuseReports::UpdateService' do + expect_next_instance_of(Admin::AbuseReports::UpdateService, report, admin, expected_params) do |service| + expect(service).to receive(:execute).and_call_original + end + + request + end + + context 'when the service response is a success' do + before do + allow_next_instance_of(Admin::AbuseReports::UpdateService, report, admin, expected_params) do |service| + allow(service).to receive(:execute).and_return(ServiceResponse.success) + end + + request + end + + it 'returns with a success status' do + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when the service response is an error' do + let(:error_message) { 'Error updating abuse report' } + + before do + allow_next_instance_of(Admin::AbuseReports::UpdateService, report, admin, expected_params) do |service| + allow(service).to receive(:execute).and_return(ServiceResponse.error(message: error_message)) + end + + request + end + + it 'returns the service response message with a failed status' do + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq(error_message) + end + end + end + + describe 'PUT #moderate_user' do let(:report) { create(:abuse_report) } let(:params) { { user_action: 'block_user', close: 'true', reason: 'spam', comment: 'obvious spam' } } let(:expected_params) { ActionController::Parameters.new(params).permit! } let(:message) { 'Service response' } - subject(:request) { put path } + subject(:request) { put moderate_user_admin_abuse_report_path(report, params) } it 'invokes the Admin::AbuseReports::ModerateUserService' do expect_next_instance_of(Admin::AbuseReports::ModerateUserService, report, admin, expected_params) do |service| @@ -100,18 +147,6 @@ RSpec.describe Admin::AbuseReportsController, type: :request, feature_category: end end - describe 'PUT #update' do - let(:path) { admin_abuse_report_path(report, params) } - - it_behaves_like 'moderates user' - end - - describe 'PUT #moderate_user' do - let(:path) { moderate_user_admin_abuse_report_path(report, params) } - - it_behaves_like 'moderates user' - end - describe 'DELETE #destroy' do let!(:report) { create(:abuse_report) } let(:params) { {} } diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb index 21cf8ab2c79..e525d615b50 100644 --- a/spec/requests/admin/users_controller_spec.rb +++ b/spec/requests/admin/users_controller_spec.rb @@ -10,6 +10,29 @@ RSpec.describe Admin::UsersController, :enable_admin_mode, feature_category: :us sign_in(admin) end + describe 'PATCH #update' do + let(:user) { create(:user) } + + context "when admin changes user email" do + let(:new_email) { 'new-email@example.com' } + + subject(:request) { patch admin_user_path(user), params: { user: { email: new_email } } } + + it 'allows change user email', :aggregate_failures do + expect { request } + .to change { user.reload.email }.from(user.email).to(new_email) + + expect(response).to redirect_to(admin_user_path(user)) + expect(flash[:notice]).to eq('User was successfully updated.') + end + + it 'does not email the user with confirmation_instructions' do + expect { request } + .not_to have_enqueued_mail(DeviseMailer, :confirmation_instructions) + end + end + end + describe 'PUT #block' do context 'when request format is :json' do subject(:request) { put block_admin_user_path(user, format: :json) } diff --git a/spec/requests/api/bulk_imports_spec.rb b/spec/requests/api/bulk_imports_spec.rb index fdbfbf052d0..8aad56c9fc3 100644 --- a/spec/requests/api/bulk_imports_spec.rb +++ b/spec/requests/api/bulk_imports_spec.rb @@ -254,11 +254,10 @@ RSpec.describe API::BulkImports, feature_category: :importers do request expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to include('entities[0][destination_namespace] must have a relative path ' \ - 'structure with no HTTP protocol characters, or leading or ' \ - 'trailing forward slashes. Path segments must not start or end ' \ - 'with a special character, and must not contain consecutive ' \ - 'special characters.') + expect(json_response['error']).to include('entities[0][destination_namespace] must be a relative path ' \ + 'and not include protocol, sub-domain, or domain information. ' \ + "For example, 'destination/full/path' not " \ + "'https://example.com/destination/full/path'") end end @@ -311,12 +310,11 @@ RSpec.describe API::BulkImports, feature_category: :importers do } end - it 'returns blocked url message in the error' do + it 'returns blocked url message in the error', :aggregate_failures do request expect(response).to have_gitlab_http_status(:unprocessable_entity) - - expect(json_response['message']).to include("Url is blocked: Only allowed schemes are http, https") + expect(json_response['message']).to eq("URL is blocked: Only allowed schemes are http, https") end end @@ -336,16 +334,16 @@ RSpec.describe API::BulkImports, feature_category: :importers do } end - it 'returns blocked url error' do + it 'returns blocked url error', :aggregate_failures do stub_request(:get, "http://gitlab.example/api/v4/#{source_entity_type}/#{source_entity_identifier}/export_relations/status?page=1&per_page=30&private_token=access_token") - .to_return(status: 404, body: "", headers: {}) + .to_return(status: 404, body: "{'error':'404 Not Found'}") request expect(response).to have_gitlab_http_status(:unprocessable_entity) - - expect(json_response['message']).to include("Group import disabled on source or destination instance. " \ - "Ask an administrator to enable it on both instances and try again.") + expect(json_response['message']).to eq( + "Unsuccessful response 404 from /api/v4/groups/full_path/export_relations/status. Body: {'error':'404 Not Found'}" + ) end end diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index c7b7131a600..19ac673308b 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -463,6 +463,14 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do it { expect(response).to have_gitlab_http_status(:bad_request) } end + + it_behaves_like 'an endpoint with keyset pagination' do + let_it_be(:another_build) { create(:ci_build, :success, :tags, project: project, pipeline: pipeline) } + + let(:first_record) { project.builds.last } + let(:second_record) { project.builds.first } + let(:api_call) { api("/projects/#{project.id}/jobs", user) } + end end context 'unauthorized user' do diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index ca57208eb1d..7f9c9a13311 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -260,7 +260,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego expect(runner.reload.platform).to eq('darwin') expect(json_response['id']).to eq(job.id) expect(json_response['token']).to eq(job.token) - expect(json_response['job_info']).to eq(expected_job_info) + expect(json_response['job_info']).to include(expected_job_info) expect(json_response['git_info']).to eq(expected_git_info) expect(json_response['image']).to eq( { 'name' => 'image:1.0', 'entrypoint' => '/bin/sh', 'ports' => [], 'pull_policy' => nil } @@ -672,7 +672,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego expect(runner.reload.platform).to eq('darwin') expect(json_response['id']).to eq(job.id) expect(json_response['token']).to eq(job.token) - expect(json_response['job_info']).to eq(expected_job_info) + expect(json_response['job_info']).to include(expected_job_info) expect(json_response['git_info']).to eq(expected_git_info) expect(json_response['artifacts']).to eq(expected_artifacts) end @@ -785,6 +785,63 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego end end end + + describe 'time_in_queue_seconds support' do + let(:job) do + create(:ci_build, :pending, :queued, pipeline: pipeline, + name: 'spinach', stage: 'test', stage_idx: 0, + queued_at: 60.seconds.ago) + end + + it 'presents the time_in_queue_seconds info in the payload' do + request_job + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['job_info']['time_in_queue_seconds']).to be >= 60.seconds + end + end + + describe 'project_jobs_running_on_instance_runners_count support' do + context 'when runner is not instance_type' do + it 'presents the project_jobs_running_on_instance_runners_count info in the payload as +Inf' do + request_job + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['job_info']['project_jobs_running_on_instance_runners_count']).to eq('+Inf') + end + end + + context 'when runner is instance_type' do + let(:project) { create(:project, namespace: group, shared_runners_enabled: true) } + let(:runner) { create(:ci_runner, :instance) } + + context 'when less than Project::INSTANCE_RUNNER_RUNNING_JOBS_MAX_BUCKET running jobs assigned to an instance runner are on the list' do + it 'presents the project_jobs_running_on_instance_runners_count info in the payload as a correct number in a string format' do + request_job + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['job_info']['project_jobs_running_on_instance_runners_count']).to eq('0') + end + end + + context 'when at least Project::INSTANCE_RUNNER_RUNNING_JOBS_MAX_BUCKET running jobs assigned to an instance runner are on the list' do + let(:other_runner) { create(:ci_runner, :instance) } + + before do + stub_const('Project::INSTANCE_RUNNER_RUNNING_JOBS_MAX_BUCKET', 1) + + create(:ci_running_build, runner: other_runner, runner_type: other_runner.runner_type, project: project) + end + + it 'presents the project_jobs_running_on_instance_runners_count info in the payload as Project::INSTANCE_RUNNER_RUNNING_JOBS_MAX_BUCKET+' do + request_job + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['job_info']['project_jobs_running_on_instance_runners_count']).to eq('1+') + end + end + end + end end describe 'port support' do diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index 2f0e64cd4da..9247d9366b2 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::CommitStatuses, feature_category: :continuous_integration do +RSpec.describe API::CommitStatuses, :clean_gitlab_redis_cache, feature_category: :continuous_integration do let_it_be(:project) { create(:project, :repository) } let_it_be(:commit) { project.repository.commit } let_it_be(:guest) { create_user(:guest) } @@ -120,6 +120,7 @@ RSpec.describe API::CommitStatuses, feature_category: :continuous_integration do it "does not return project commits" do expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('403 Forbidden') end end @@ -139,7 +140,8 @@ RSpec.describe API::CommitStatuses, feature_category: :continuous_integration do context 'developer user' do context 'uses only required parameters' do - %w[pending running success failed canceled].each do |status| + valid_statues = %w[pending running success failed canceled] + valid_statues.each do |status| context "for #{status}" do context 'when pipeline for sha does not exists' do it 'creates commit status and sets pipeline iid' do @@ -248,12 +250,13 @@ RSpec.describe API::CommitStatuses, feature_category: :continuous_integration do end end - context 'transitions status from pending' do + context 'when status transitions from pending' do before do post api(post_url, developer), params: { state: 'pending' } end - %w[running success failed canceled].each do |status| + valid_statues = %w[running success failed canceled] + valid_statues.each do |status| it "to #{status}" do expect { post api(post_url, developer), params: { state: status } }.not_to change { CommitStatus.count } @@ -366,6 +369,7 @@ RSpec.describe API::CommitStatuses, feature_category: :continuous_integration do send_request expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq("Cannot transition status via :run from :running (Reason(s): Status cannot transition via \"run\")") commit_status = project.commit_statuses.find_by!(name: 'coverage') @@ -440,6 +444,7 @@ RSpec.describe API::CommitStatuses, feature_category: :continuous_integration do it 'does not create commit status' do expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(nil) end end @@ -450,6 +455,7 @@ RSpec.describe API::CommitStatuses, feature_category: :continuous_integration do it 'does not create commit status' do expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(nil) end end @@ -464,6 +470,7 @@ RSpec.describe API::CommitStatuses, feature_category: :continuous_integration do it 'does not create commit status' do expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('403 Forbidden') end end @@ -485,15 +492,16 @@ RSpec.describe API::CommitStatuses, feature_category: :continuous_integration do it 'returns not found error' do expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Commit Not Found') end end context 'when target URL is an invalid address' do before do post api(post_url, developer), params: { - state: 'pending', - target_url: 'invalid url' - } + state: 'pending', + target_url: 'invalid url' + } end it 'responds with bad request status and validation errors' do @@ -506,9 +514,9 @@ RSpec.describe API::CommitStatuses, feature_category: :continuous_integration do context 'when target URL is an unsupported scheme' do before do post api(post_url, developer), params: { - state: 'pending', - target_url: 'git://example.com' - } + state: 'pending', + target_url: 'git://example.com' + } end it 'responds with bad request status and validation errors' do @@ -562,6 +570,7 @@ RSpec.describe API::CommitStatuses, feature_category: :continuous_integration do it 'does not create commit status' do expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('403 Forbidden') end end @@ -572,6 +581,7 @@ RSpec.describe API::CommitStatuses, feature_category: :continuous_integration do it 'does not create commit status' do expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('403 Forbidden') end end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 687ce333ca5..8b9ac7cd588 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -589,7 +589,7 @@ RSpec.describe API::Commits, feature_category: :source_code_management do let(:namespace) { project.namespace.reload } let(:label) { 'counts.web_ide_commits' } let(:context) do - [Gitlab::Tracking::ServicePingContext.new(data_source: :redis, key_path: 'counts.web_ide_commits').to_context.to_json] + [Gitlab::Usage::MetricDefinition.context_for('counts.web_ide_commits').to_context.to_json] end end end diff --git a/spec/requests/api/discussions_spec.rb b/spec/requests/api/discussions_spec.rb index a65dc6e0175..aebdcebbc5a 100644 --- a/spec/requests/api/discussions_spec.rb +++ b/spec/requests/api/discussions_spec.rb @@ -116,6 +116,17 @@ RSpec.describe API::Discussions, feature_category: :team_planning do it_behaves_like 'diff discussions API', 'projects', 'merge_requests', 'iid' it_behaves_like 'resolvable discussions API', 'projects', 'merge_requests', 'iid' + context "when position_type is file" do + it "creates a new diff note" do + position = diff_note.position.to_h.merge({ position_type: 'file' }).except(:ignore_whitespace_change) + + post api("/projects/#{parent.id}/merge_requests/#{noteable['iid']}/discussions", user), + params: { body: 'hi!', position: position } + + expect(response).to have_gitlab_http_status(:created) + end + end + context "when position is for a previous commit on the merge request" do it "returns a 400 bad request error because the line_code is old" do # SHA taken from an earlier commit listed in spec/factories/merge_requests.rb diff --git a/spec/requests/api/feature_flags_spec.rb b/spec/requests/api/feature_flags_spec.rb index 69e3633de57..4fb0dfbb070 100644 --- a/spec/requests/api/feature_flags_spec.rb +++ b/spec/requests/api/feature_flags_spec.rb @@ -111,7 +111,57 @@ RSpec.describe API::FeatureFlags, feature_category: :feature_flags do 'scopes' => [{ 'id' => scope.id, 'environment_scope' => 'production' - }] + }], + 'user_list' => nil + }] + }]) + end + end + + context 'with user_list strategy feature flags' do + let!(:feature_flag) do + create(:operations_feature_flag, :new_version_flag, project: project, name: 'feature1') + end + + let!(:user_list) do + create(:operations_feature_flag_user_list, project: project) + end + + let!(:strategy) do + create(:operations_strategy, :gitlab_userlist, user_list: user_list, feature_flag: feature_flag, name: 'gitlabUserList', parameters: {}) + end + + let!(:scope) do + create(:operations_scope, strategy: strategy, environment_scope: 'production') + end + + it 'returns the feature flags', :aggregate_failures do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flags') + expect(json_response).to eq([{ + 'name' => 'feature1', + 'description' => nil, + 'active' => true, + 'version' => 'new_version_flag', + 'updated_at' => feature_flag.updated_at.as_json, + 'created_at' => feature_flag.created_at.as_json, + 'scopes' => [], + 'strategies' => [{ + 'id' => strategy.id, + 'name' => 'gitlabUserList', + 'parameters' => {}, + 'scopes' => [{ + 'id' => scope.id, + 'environment_scope' => 'production' + }], + 'user_list' => { + 'id' => user_list.id, + 'iid' => user_list.iid, + 'name' => user_list.name, + 'user_xids' => user_list.user_xids + } }] }]) end @@ -162,7 +212,57 @@ RSpec.describe API::FeatureFlags, feature_category: :feature_flags do 'scopes' => [{ 'id' => scope.id, 'environment_scope' => 'production' - }] + }], + 'user_list' => nil + }] + }) + end + end + + context 'with user_list strategy feature flag' do + let!(:feature_flag) do + create(:operations_feature_flag, :new_version_flag, project: project, name: 'feature1') + end + + let(:user_list) do + create(:operations_feature_flag_user_list, project: project) + end + + let!(:strategy) do + create(:operations_strategy, :gitlab_userlist, user_list: user_list, feature_flag: feature_flag, name: 'gitlabUserList', parameters: {}) + end + + let!(:scope) do + create(:operations_scope, strategy: strategy, environment_scope: 'production') + end + + it 'returns the feature flag', :aggregate_failures do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + expect(json_response).to eq({ + 'name' => 'feature1', + 'description' => nil, + 'active' => true, + 'version' => 'new_version_flag', + 'updated_at' => feature_flag.updated_at.as_json, + 'created_at' => feature_flag.created_at.as_json, + 'scopes' => [], + 'strategies' => [{ + 'id' => strategy.id, + 'name' => 'gitlabUserList', + 'parameters' => {}, + 'scopes' => [{ + 'id' => scope.id, + 'environment_scope' => 'production' + }], + 'user_list' => { + 'id' => user_list.id, + 'iid' => user_list.iid, + 'name' => user_list.name, + 'user_xids' => user_list.user_xids + } }] }) end @@ -224,6 +324,10 @@ RSpec.describe API::FeatureFlags, feature_category: :feature_flags do end context 'when creating a version 2 feature flag' do + let(:user_list) do + create(:operations_feature_flag_user_list, project: project) + end + it 'creates a new feature flag' do params = { name: 'new-feature', @@ -348,6 +452,32 @@ RSpec.describe API::FeatureFlags, feature_category: :feature_flags do environment_scope: 'staging' }]) end + + it 'creates a new feature flag with user list strategy', :aggregate_failures do + params = { + name: 'new-feature', + version: 'new_version_flag', + strategies: [{ + name: 'gitlabUserList', + parameters: {}, + user_list_id: user_list.id + }] + } + + post api("/projects/#{project.id}/feature_flags", user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/feature_flag') + + feature_flag = project.operations_feature_flags.last + expect(feature_flag.name).to eq(params[:name]) + expect(feature_flag.version).to eq('new_version_flag') + expect(feature_flag.strategies.map { |s| s.slice(:name, :parameters).deep_symbolize_keys }).to eq([{ + name: 'gitlabUserList', + parameters: {} + }]) + expect(feature_flag.strategies.first.user_list).to eq(user_list) + end end context 'when given invalid parameters' do @@ -369,6 +499,10 @@ RSpec.describe API::FeatureFlags, feature_category: :feature_flags do project: project, active: true, name: 'feature1', description: 'old description') end + let(:user_list) do + create(:operations_feature_flag_user_list, project: project) + end + it 'returns a 404 if the feature flag does not exist' do params = { description: 'new description' } @@ -537,6 +671,30 @@ RSpec.describe API::FeatureFlags, feature_category: :feature_flags do }]) end + it 'updates an existing feature flag strategy to be gitlab user list strategy', :aggregate_failures do + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) + params = { + strategies: [{ + id: strategy.id, + name: 'gitlabUserList', + user_list_id: user_list.id, + parameters: {} + }] + } + + put api("/projects/#{project.id}/feature_flags/feature1", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag') + result = feature_flag.reload.strategies.map { |s| s.slice(:id, :name, :parameters).deep_symbolize_keys } + expect(result).to eq([{ + id: strategy.id, + name: 'gitlabUserList', + parameters: {} + }]) + expect(feature_flag.strategies.first.user_list).to eq(user_list) + end + it 'adds a new gradual rollout strategy to a feature flag' do strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {}) params = { diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb index 2571e3b1e6a..d922947f3e9 100644 --- a/spec/requests/api/features_spec.rb +++ b/spec/requests/api/features_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Features, stub_feature_flags: false, feature_category: :feature_flags do +RSpec.describe API::Features, :clean_gitlab_redis_feature_flag, stub_feature_flags: false, feature_category: :feature_flags do let_it_be(:user) { create(:user) } let_it_be(:opted_out) { create(:user) } let_it_be(:admin) { create(:admin) } diff --git a/spec/requests/api/graphql/ci/jobs_spec.rb b/spec/requests/api/graphql/ci/jobs_spec.rb index 756fcd8b7cd..ab2ebf134d7 100644 --- a/spec/requests/api/graphql/ci/jobs_spec.rb +++ b/spec/requests/api/graphql/ci/jobs_spec.rb @@ -139,7 +139,10 @@ RSpec.describe 'Query.project.pipeline', feature_category: :continuous_integrati let(:pipeline) do pipeline = create(:ci_pipeline, project: project, user: user) stage = create(:ci_stage, project: project, pipeline: pipeline, name: 'first', position: 1) - create(:ci_build, stage_id: stage.id, pipeline: pipeline, name: 'my test job', scheduling_type: :stage) + create( + :ci_build, pipeline: pipeline, name: 'my test job', + scheduling_type: :stage, stage_id: stage.id, stage_idx: stage.position + ) pipeline end @@ -180,10 +183,10 @@ RSpec.describe 'Query.project.pipeline', feature_category: :continuous_integrati previousStageJobsOrNeeds { nodes { ... on CiBuildNeed { - #{all_graphql_fields_for('CiBuildNeed')} + name } ... on CiJob { - #{all_graphql_fields_for('CiJob', excluded: %w[aiFailureAnalysis])} + name } } } @@ -211,10 +214,12 @@ RSpec.describe 'Query.project.pipeline', feature_category: :continuous_integrati before do build_stage = create(:ci_stage, position: 2, name: 'build', project: project, pipeline: pipeline) test_stage = create(:ci_stage, position: 3, name: 'test', project: project, pipeline: pipeline) + deploy_stage = create(:ci_stage, position: 4, name: 'deploy', project: project, pipeline: pipeline) create(:ci_build, pipeline: pipeline, name: 'docker 1 2', scheduling_type: :stage, ci_stage: build_stage, stage_idx: build_stage.position) create(:ci_build, pipeline: pipeline, name: 'docker 2 2', ci_stage: build_stage, stage_idx: build_stage.position, scheduling_type: :dag) create(:ci_build, pipeline: pipeline, name: 'rspec 1 2', scheduling_type: :stage, ci_stage: test_stage, stage_idx: test_stage.position) + create(:ci_build, pipeline: pipeline, name: 'deploy', scheduling_type: :stage, ci_stage: deploy_stage, stage_idx: deploy_stage.position) test_job = create(:ci_build, pipeline: pipeline, name: 'rspec 2 2', scheduling_type: :dag, ci_stage: test_stage, stage_idx: test_stage.position) create(:ci_build_need, build: test_job, name: 'my test job') @@ -255,6 +260,14 @@ RSpec.describe 'Query.project.pipeline', feature_category: :continuous_integrati 'previousStageJobsOrNeeds' => { 'nodes' => [ a_hash_including('name' => 'my test job') ] } + ), + a_hash_including( + 'name' => 'deploy', + 'needs' => { 'nodes' => [] }, + 'previousStageJobsOrNeeds' => { 'nodes' => [ + a_hash_including('name' => 'rspec 1 2'), + a_hash_including('name' => 'rspec 2 2') + ] } ) ) end @@ -613,3 +626,87 @@ RSpec.describe 'Query.project.pipeline', feature_category: :continuous_integrati end end end + +RSpec.describe 'previousStageJobs', feature_category: :pipeline_composition do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :public) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + + let(:query) do + <<~QUERY + { + project(fullPath: "#{project.full_path}") { + pipeline(iid: "#{pipeline.iid}") { + stages { + nodes { + groups { + nodes { + jobs { + nodes { + name + previousStageJobs { + nodes { + name + downstreamPipeline { + id + } + } + } + } + } + } + } + } + } + } + } + } + QUERY + end + + it 'does not produce N+1 queries', :request_store, :use_sql_query_cache do + user1 = create(:user) + user2 = create(:user) + + create_stage_with_build_and_bridge('build', 0) + create_stage_with_build_and_bridge('test', 1) + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + post_graphql(query, current_user: user1) + end + + expect(graphql_data_previous_stage_jobs).to eq( + 'build_build' => [], + 'test_build' => %w[build_build] + ) + + create_stage_with_build_and_bridge('deploy', 2) + + expect do + post_graphql(query, current_user: user2) + end.to issue_same_number_of_queries_as(control) + + expect(graphql_data_previous_stage_jobs).to eq( + 'build_build' => [], + 'test_build' => %w[build_build], + 'deploy_build' => %w[test_build] + ) + end + + def create_stage_with_build_and_bridge(stage_name, stage_position) + stage = create(:ci_stage, position: stage_position, name: "#{stage_name}_stage", project: project, pipeline: pipeline) + + create(:ci_build, pipeline: pipeline, name: "#{stage_name}_build", ci_stage: stage, stage_idx: stage.position) + end + + def graphql_data_previous_stage_jobs + stages = graphql_data.dig('project', 'pipeline', 'stages', 'nodes') + groups = stages.flat_map { |stage| stage.dig('groups', 'nodes') } + jobs = groups.flat_map { |group| group.dig('jobs', 'nodes') } + + jobs.each_with_object({}) do |job, previous_stage_jobs| + previous_stage_jobs[job['name']] = job.dig('previousStageJobs', 'nodes').pluck('name') + end + end +end diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index 3cfb98c57fd..6f1eb77fa9b 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -2,9 +2,11 @@ require 'spec_helper' -RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do +RSpec.describe 'Query.runner(id)', :freeze_time, feature_category: :runner_fleet do include GraphqlHelpers + using RSpec::Parameterized::TableSyntax + let_it_be(:user) { create(:user, :admin) } let_it_be(:another_admin) { create(:user, :admin) } let_it_be_with_reload(:group) { create(:group) } @@ -144,23 +146,6 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do ) expect(runner_data['tagList']).to match_array runner.tag_list end - - it 'does not execute more queries per runner', :use_sql_query_cache, :aggregate_failures do - # warm-up license cache and so on: - personal_access_token = create(:personal_access_token, user: user) - args = { current_user: user, token: { personal_access_token: personal_access_token } } - post_graphql(query, **args) - expect(graphql_data_at(:runner)).not_to be_nil - - personal_access_token = create(:personal_access_token, user: another_admin) - args = { current_user: another_admin, token: { personal_access_token: personal_access_token } } - control = ActiveRecord::QueryRecorder.new(skip_cached: false) { post_graphql(query, **args) } - - create(:ci_runner, :instance, version: '14.0.0', tag_list: %w[tag5 tag6], creator: another_admin) - create(:ci_runner, :project, version: '14.0.1', projects: [project1], tag_list: %w[tag3 tag8], creator: another_admin) - - expect { post_graphql(query, **args) }.not_to exceed_all_query_limit(control) - end end shared_examples 'retrieval with no admin url' do @@ -228,7 +213,7 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do end end - context 'with build running', :freeze_time do + context 'with build running' do let!(:pipeline) { create(:ci_pipeline, project: project1) } let!(:runner_manager) do create(:ci_runner_machine, @@ -248,23 +233,25 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do end describe 'for project runner' do - describe 'locked' do - using RSpec::Parameterized::TableSyntax + let_it_be_with_refind(:project_runner) do + create(:ci_runner, :project, + description: 'Runner 3', + contacted_at: 1.day.ago, + active: false, + locked: false, + version: 'adfe157', + revision: 'b', + ip_address: '10.10.10.10', + access_level: 1, + run_untagged: true) + end + describe 'locked' do where(is_locked: [true, false]) with_them do - let(:project_runner) do - create(:ci_runner, :project, - description: 'Runner 3', - contacted_at: 1.day.ago, - active: false, - locked: is_locked, - version: 'adfe157', - revision: 'b', - ip_address: '10.10.10.10', - access_level: 1, - run_untagged: true) + before do + project_runner.update!(locked: is_locked) end let(:query) do @@ -357,6 +344,109 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do ) end end + + describe 'jobs' do + let(:query) do + %( + query { + runner(id: "#{project_runner.to_global_id}") { #{runner_query_fragment} } + } + ) + end + + context 'with a job from a non-owned project' do + let(:runner_query_fragment) do + %( + id + jobs { + nodes { + id status shortSha finishedAt duration queuedDuration tags webPath + project { id } + runner { id } + } + } + ) + end + + let_it_be(:owned_project_owner) { create(:user) } + let_it_be(:owned_project) { create(:project) } + let_it_be(:other_project) { create(:project) } + let_it_be(:project_runner) { create(:ci_runner, :project_type, projects: [other_project, owned_project]) } + let_it_be(:owned_project_pipeline) { create(:ci_pipeline, project: owned_project) } + let_it_be(:other_project_pipeline) { create(:ci_pipeline, project: other_project) } + let_it_be(:owned_build) do + create(:ci_build, :running, runner: project_runner, pipeline: owned_project_pipeline, + tag_list: %i[a b c], created_at: 1.hour.ago, started_at: 59.minutes.ago, finished_at: 30.minutes.ago) + end + + let_it_be(:other_build) do + create(:ci_build, :success, runner: project_runner, pipeline: other_project_pipeline, + tag_list: %i[d e f], created_at: 30.minutes.ago, started_at: 19.minutes.ago, finished_at: 1.minute.ago) + end + + before_all do + owned_project.add_owner(owned_project_owner) + end + + it 'returns empty values for sensitive fields in non-owned jobs' do + post_graphql(query, current_user: owned_project_owner) + + jobs_data = graphql_data_at(:runner, :jobs, :nodes) + expect(jobs_data).not_to be_nil + expect(jobs_data).to match([ + a_graphql_entity_for(other_build, + status: other_build.status.upcase, + project: nil, tags: nil, web_path: nil, + runner: a_graphql_entity_for(project_runner), + short_sha: 'Unauthorized', finished_at: other_build.finished_at&.iso8601, + duration: a_value_within(0.001).of(other_build.duration), + queued_duration: a_value_within(0.001).of((other_build.started_at - other_build.queued_at).to_f)), + a_graphql_entity_for(owned_build, + status: owned_build.status.upcase, + project: a_graphql_entity_for(owned_project), + tags: owned_build.tag_list.map(&:to_s), + web_path: ::Gitlab::Routing.url_helpers.project_job_path(owned_project, owned_build), + runner: a_graphql_entity_for(project_runner), + short_sha: owned_build.short_sha, + finished_at: owned_build.finished_at&.iso8601, + duration: a_value_within(0.001).of(owned_build.duration), + queued_duration: a_value_within(0.001).of((owned_build.started_at - owned_build.queued_at).to_f)) + ]) + end + end + end + + describe 'a query fetching all fields' do + let(:query) do + wrap_fields(query_graphql_path(query_path, all_graphql_fields_for('CiRunner'))) + end + + let(:query_path) do + [ + [:runner, { id: project_runner.to_global_id.to_s }] + ] + end + + it 'does not execute more queries per runner', :use_sql_query_cache, :aggregate_failures do + create(:ci_build, :failed, runner: project_runner) + create(:ci_runner_machine, runner: project_runner, version: '16.4.0') + + # warm-up license cache and so on: + personal_access_token = create(:personal_access_token, user: user) + args = { current_user: user, token: { personal_access_token: personal_access_token } } + post_graphql(query, **args) + expect(graphql_data_at(:runner)).not_to be_nil + + personal_access_token = create(:personal_access_token, user: another_admin) + args = { current_user: another_admin, token: { personal_access_token: personal_access_token } } + control = ActiveRecord::QueryRecorder.new(skip_cached: false) { post_graphql(query, **args) } + + create(:ci_build, :failed, runner: project_runner) + create(:ci_runner_machine, runner: project_runner, version: '16.4.1') + + expect { post_graphql(query, **args) }.not_to exceed_all_query_limit(control) + end + end end describe 'for inactive runner' do @@ -501,8 +591,14 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do end describe 'for runner with status' do - let_it_be(:stale_runner) { create(:ci_runner, description: 'Stale runner 1', created_at: 3.months.ago) } - let_it_be(:never_contacted_instance_runner) { create(:ci_runner, description: 'Missing runner 1', created_at: 1.month.ago, contacted_at: nil) } + let_it_be(:stale_runner) do + create(:ci_runner, description: 'Stale runner 1', + created_at: (3.months + 1.second).ago, contacted_at: (3.months + 1.second).ago) + end + + let_it_be(:never_contacted_instance_runner) do + create(:ci_runner, description: 'Missing runner 1', created_at: 1.month.ago, contacted_at: nil) + end let(:query) do %( @@ -918,8 +1014,6 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do end context 'when requesting individual fields' do - using RSpec::Parameterized::TableSyntax - where(:field) do [ 'detailedStatus { id detailsPath group icon text }', diff --git a/spec/requests/api/graphql/ci/runner_web_url_edge_spec.rb b/spec/requests/api/graphql/ci/runner_web_url_edge_spec.rb index e84a1ca4cc4..76e2dda4ce2 100644 --- a/spec/requests/api/graphql/ci/runner_web_url_edge_spec.rb +++ b/spec/requests/api/graphql/ci/runner_web_url_edge_spec.rb @@ -25,16 +25,22 @@ RSpec.describe 'RunnerWebUrlEdge', feature_category: :runner_fleet do GQL end - before do + subject(:request) do post_graphql(query, current_user: user, variables: { path: group.full_path }) end context 'with an authorized user' do let(:user) { create_default(:user, :admin) } - it_behaves_like 'a working graphql query' + it_behaves_like 'a working graphql query' do + before do + request + end + end it 'returns correct URLs' do + request + expect(edges_graphql_data).to match_array [ { 'editUrl' => Gitlab::Routing.url_helpers.edit_group_runner_url(group, group_runner), @@ -47,10 +53,14 @@ RSpec.describe 'RunnerWebUrlEdge', feature_category: :runner_fleet do context 'with an unauthorized user' do let(:user) { create(:user) } - it_behaves_like 'a working graphql query' + it 'returns nil runners and an error' do + request - it 'returns no edges' do - expect(edges_graphql_data).to be_empty + expect(graphql_data.dig('group', 'runners')).to be_nil + expect(graphql_errors).to contain_exactly(a_hash_including( + 'message' => a_string_including("you don't have permission to perform this action"), + 'path' => %w[group runners] + )) end end end diff --git a/spec/requests/api/graphql/ci/runners_spec.rb b/spec/requests/api/graphql/ci/runners_spec.rb index 3f6d39435fd..c5571086700 100644 --- a/spec/requests/api/graphql/ci/runners_spec.rb +++ b/spec/requests/api/graphql/ci/runners_spec.rb @@ -72,10 +72,16 @@ RSpec.describe 'Query.runners', feature_category: :runner_fleet do args = { current_user: admin2, token: { personal_access_token: personal_access_token } } control = ActiveRecord::QueryRecorder.new { post_graphql(query, **args) } - create(:ci_runner, :instance, version: '14.0.0', tag_list: %w[tag5 tag6], creator: admin2) - create(:ci_runner, :project, version: '14.0.1', projects: [project], tag_list: %w[tag3 tag8], + runner2 = create(:ci_runner, :instance, version: '14.0.0', tag_list: %w[tag5 tag6], creator: admin2) + runner3 = create(:ci_runner, :project, version: '14.0.1', projects: [project], tag_list: %w[tag3 tag8], creator: current_user) + create(:ci_build, :failed, runner: runner2) + create(:ci_runner_machine, runner: runner2, version: '16.4.1') + + create(:ci_build, :failed, runner: runner3) + create(:ci_runner_machine, runner: runner3, version: '16.4.0') + expect { post_graphql(query, **args) }.not_to exceed_query_limit(control) 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 index 961de84234c..869147f17b3 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_blobs_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_blobs_spec.rb @@ -26,7 +26,6 @@ RSpec.describe 'getting dependency proxy blobs in a group', feature_category: :d #{query_graphql_field('dependency_proxy_blobs', {}, dependency_proxy_blob_fields)} dependencyProxyBlobCount dependencyProxyTotalSize - dependencyProxyTotalSizeInBytes dependencyProxyTotalSizeBytes GQL end @@ -44,7 +43,7 @@ RSpec.describe 'getting dependency proxy blobs in a group', feature_category: :d 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') } - let(:dependency_proxy_total_size_in_bytes_response) { graphql_data.dig('group', 'dependencyProxyTotalSizeInBytes') } + let(:dependency_proxy_total_size_bytes_response) { graphql_data.dig('group', 'dependencyProxyTotalSizeBytes') } before do stub_config(dependency_proxy: { enabled: true }) @@ -131,7 +130,7 @@ RSpec.describe 'getting dependency proxy blobs in a group', feature_category: :d it 'returns the total size in bytes' do subject expected_size = blobs.inject(0) { |sum, blob| sum + blob.size } - expect(dependency_proxy_total_size_in_bytes_response).to eq(expected_size) + expect(dependency_proxy_total_size_bytes_response.to_i).to eq(expected_size) end context 'with a giant size blob' do diff --git a/spec/requests/api/graphql/group/work_item_spec.rb b/spec/requests/api/graphql/group/work_item_spec.rb new file mode 100644 index 00000000000..9adb0acbf6b --- /dev/null +++ b/spec/requests/api/graphql/group/work_item_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting a single work item associated with a group', feature_category: :team_planning do + include GraphqlHelpers + + let_it_be(:group) { create(:group, :public) } + let_it_be(:user) { create(:user) } + let_it_be(:reporter) { create(:user).tap { |user| group.add_reporter(user) } } + + let_it_be(:group_work_item) do + create( + :work_item, + namespace: group, + author: reporter + ) + end + + let_it_be(:confidential_work_item) do + create(:work_item, :confidential, namespace: group, author: reporter) + end + + let(:work_item_data) { graphql_data.dig('group', 'workItem') } + let(:query_group) { group } + let(:query_work_item) { group_work_item } + let(:params) { { iid: query_work_item.iid.to_s } } + let(:query) do + graphql_query_for( + 'group', + { 'fullPath' => query_group.full_path }, + query_graphql_field('workItem', params, all_graphql_fields_for('workItems'.classify, max_depth: 2)) + ) + end + + context 'when the user cannot read the work item' do + let(:current_user) { user } + let(:query_work_item) { confidential_work_item } + + it 'returns does not return the work item' do + post_graphql(query, current_user: current_user) + + expect(work_item_data).to be_nil + end + end + + context 'when the user can read the work item' do + let(:current_user) { reporter } + + it 'returns the work item' do + post_graphql(query, current_user: current_user) + + expect(work_item_data).to include( + 'id' => query_work_item.to_gid.to_s, + 'iid' => query_work_item.iid.to_s + ) + end + + context 'when the namespace_level_work_items feature flag is disabled' do + before do + stub_feature_flags(namespace_level_work_items: false) + end + + it 'does not return the work item' do + post_graphql(query, current_user: current_user) + + expect(work_item_data).to be_nil + end + end + end +end diff --git a/spec/requests/api/graphql/group/work_items_spec.rb b/spec/requests/api/graphql/group/work_items_spec.rb index f6dad577b5e..ef96ef4754a 100644 --- a/spec/requests/api/graphql/group/work_items_spec.rb +++ b/spec/requests/api/graphql/group/work_items_spec.rb @@ -35,7 +35,7 @@ RSpec.describe 'getting a work_item list for a group', feature_category: :team_p let_it_be(:other_work_item) { create(:work_item) } let(:work_items_data) { graphql_data['group']['workItems']['nodes'] } - let(:work_item_filter_params) { {} } + let(:item_filter_params) { {} } let(:current_user) { user } let(:query_group) { group } @@ -47,6 +47,28 @@ RSpec.describe 'getting a work_item list for a group', feature_category: :team_p QUERY end + it_behaves_like 'graphql work item list request spec' do + let_it_be(:container_build_params) { { namespace: group } } + let(:work_item_node_path) { %w[group workItems nodes] } + + def post_query(request_user = current_user) + post_graphql(query, current_user: request_user) + end + end + + context 'when filtering by search' do + let(:item_filter_params) { { search: 'search_term', in: [:DESCRIPTION] } } + + # TODO: https://gitlab.com/gitlab-org/gitlab/-/work_items/393126 + it 'returns an error since search is not implemented at the group level yet' do + post_graphql(query, current_user: current_user) + + expect(graphql_errors).to contain_exactly( + hash_including('message' => 'Searching is not available for work items at the namespace level yet') + ) + end + end + context 'when the user can not see confidential work_items' do it_behaves_like 'a working graphql query' do before do @@ -66,12 +88,6 @@ RSpec.describe 'getting a work_item list for a group', feature_category: :team_p context 'when the user can see confidential work_items' do let(:current_user) { reporter } - it_behaves_like 'a working graphql query' do - before do - post_graphql(query, current_user: current_user) - end - end - it 'returns also confidential work_items' do post_graphql(query, current_user: current_user) @@ -96,7 +112,7 @@ RSpec.describe 'getting a work_item list for a group', feature_category: :team_p graphql_dig_at(work_items_data, :id) end - def query(params = work_item_filter_params) + def query(params = item_filter_params) graphql_query_for( 'group', { 'fullPath' => query_group.full_path }, diff --git a/spec/requests/api/graphql/group_query_spec.rb b/spec/requests/api/graphql/group_query_spec.rb index 6debe2d3d67..1dcbc44c587 100644 --- a/spec/requests/api/graphql/group_query_spec.rb +++ b/spec/requests/api/graphql/group_query_spec.rb @@ -17,7 +17,7 @@ RSpec.describe 'getting group information', :with_license, feature_category: :gr # similar to the API "GET /groups/:id" describe "Query group(fullPath)" do def group_query(group) - fields = all_graphql_fields_for('Group') + fields = all_graphql_fields_for('Group', excluded: %w[runners]) # TODO: Set required timelogs args elsewhere https://gitlab.com/gitlab-org/gitlab/-/issues/325499 fields.selection['timelogs(startDate: "2021-03-01" endDate: "2021-03-30")'] = fields.selection.delete('timelogs') diff --git a/spec/requests/api/graphql/groups_query_spec.rb b/spec/requests/api/graphql/groups_query_spec.rb index 460cb40b68a..7310382553f 100644 --- a/spec/requests/api/graphql/groups_query_spec.rb +++ b/spec/requests/api/graphql/groups_query_spec.rb @@ -12,7 +12,7 @@ RSpec.describe 'searching groups', :with_license, feature_category: :groups_and_ let(:fields) do <<~FIELDS nodes { - #{all_graphql_fields_for('Group')} + #{all_graphql_fields_for('Group', excluded: %w[runners])} } FIELDS end diff --git a/spec/requests/api/graphql/jobs_query_spec.rb b/spec/requests/api/graphql/jobs_query_spec.rb index 4248a03fa74..7607aeac6e0 100644 --- a/spec/requests/api/graphql/jobs_query_spec.rb +++ b/spec/requests/api/graphql/jobs_query_spec.rb @@ -10,7 +10,7 @@ RSpec.describe 'getting job information', feature_category: :continuous_integrat :jobs, {}, %( count nodes { - #{all_graphql_fields_for(::Types::Ci::JobType, max_depth: 1, excluded: %w[aiFailureAnalysis])} + #{all_graphql_fields_for(::Types::Ci::JobType, max_depth: 1)} }) ) end diff --git a/spec/requests/api/graphql/merge_requests/codequality_reports_comparer_spec.rb b/spec/requests/api/graphql/merge_requests/codequality_reports_comparer_spec.rb new file mode 100644 index 00000000000..2939e9307e9 --- /dev/null +++ b/spec/requests/api/graphql/merge_requests/codequality_reports_comparer_spec.rb @@ -0,0 +1,185 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Query.project.mergeRequest.codequalityReportsComparer', feature_category: :code_quality do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:merge_request) { create(:merge_request, :with_codequality_reports, source_project: project) } + + let(:mock_report) do + { + status: :parsed, + data: { + status: 'failed', + new_errors: [ + { + description: "Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.", + fingerprint: "15cdb5c53afd42bc22f8ca366a08d547", + severity: "major", + file_path: "foo.rb", + line: 10, + engine_name: "structure" + }, + { + description: "Method `backwards_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.", + fingerprint: "f3bdc1e8c102ba5fbd9e7f6cda51c95e", + severity: "major", + file_path: "foo.rb", + line: 14, + engine_name: "structure" + }, + { + description: "Avoid parameter lists longer than 5 parameters. [12/5]", + fingerprint: "ab5f8b935886b942d621399f5a2ca16e", + severity: "minor", + file_path: "foo.rb", + line: 14, + engine_name: "rubocop" + } + ], + resolved_errors: [], + existing_errors: [], + summary: { + total: 3, + resolved: 0, + errored: 3 + } + }.deep_stringify_keys + } + end + + let(:codequality_reports_comparer_fields) do + <<~QUERY + codequalityReportsComparer { + report { + status + newErrors { + description + fingerprint + severity + filePath + line + webUrl + engineName + } + resolvedErrors { + description + fingerprint + severity + filePath + line + webUrl + engineName + } + existingErrors { + description + fingerprint + severity + filePath + line + webUrl + engineName + } + summary { + errored + resolved + total + } + } + } + QUERY + end + + let(:merge_request_fields) do + query_graphql_field(:merge_request, { iid: merge_request.iid.to_s }, codequality_reports_comparer_fields) + end + + let(:query) { graphql_query_for(:project, { full_path: project.full_path }, merge_request_fields) } + + subject(:result) { graphql_data_at(:project, :merge_request, :codequality_reports_comparer) } + + before do + allow_next_found_instance_of(MergeRequest) do |merge_request| + allow(merge_request).to receive(:compare_codequality_reports).and_return(mock_report) + end + end + + context 'when the user is not authorized to read the field' do + before do + post_graphql(query, current_user: user) + end + + it { is_expected.to be_nil } + end + + context 'when the user is authorized to read the field' do + before_all do + project.add_reporter(user) + end + + before do + post_graphql(query, current_user: user) + end + + context 'when when sast_reports_in_inline_diff FF is disabled' do + before_all do + stub_feature_flags(sast_reports_in_inline_diff: false) + end + + it 'returns null for codequality_reports_comparer field' do + expect(result).to be_nil + end + end + + it 'returns expected data' do + expect(result).to match( + a_hash_including( + { + report: { + status: 'FAILED', + newErrors: [ + { + description: 'Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.', + fingerprint: '15cdb5c53afd42bc22f8ca366a08d547', + severity: 'MAJOR', + filePath: 'foo.rb', + line: 10, + webUrl: nil, + engineName: 'structure' + }, + { + description: 'Method `backwards_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.', + fingerprint: 'f3bdc1e8c102ba5fbd9e7f6cda51c95e', + severity: 'MAJOR', + filePath: 'foo.rb', + line: 14, + webUrl: nil, + engineName: 'structure' + }, + { + description: 'Avoid parameter lists longer than 5 parameters. [12/5]', + fingerprint: 'ab5f8b935886b942d621399f5a2ca16e', + severity: 'MINOR', + filePath: 'foo.rb', + line: 14, + webUrl: nil, + engineName: 'rubocop' + } + ], + resolvedErrors: [], + existingErrors: [], + summary: { + errored: 3, + resolved: 0, + total: 3 + } + } + }.deep_stringify_keys + ) + ) + end + end +end diff --git a/spec/requests/api/graphql/mutations/admin/abuse_report_labels/create_spec.rb b/spec/requests/api/graphql/mutations/admin/abuse_report_labels/create_spec.rb new file mode 100644 index 00000000000..2a20d96d9c8 --- /dev/null +++ b/spec/requests/api/graphql/mutations/admin/abuse_report_labels/create_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::Admin::AbuseReportLabels::Create, feature_category: :insider_threat do + include GraphqlHelpers + + let(:params) do + { + 'title' => 'foo', + 'color' => '#FF0000' + } + end + + let(:mutation) { graphql_mutation(:abuse_report_label_create, params) } + + subject { post_graphql_mutation(mutation, current_user: current_user) } + + def mutation_response + graphql_mutation_response(:abuse_report_label_create) + end + + context 'when the user does not have permission to create a label', :enable_admin_mode do + let_it_be(:current_user) { create(:user) } + + it_behaves_like 'a mutation that returns a top-level access error' + + it 'does not create the label' do + expect { subject }.not_to change { Admin::AbuseReportLabel.count } + end + end + + context 'when the user has permission to create a label', :enable_admin_mode do + let_it_be(:current_user) { create(:admin) } + + it 'creates the label' do + expect { subject }.to change { Admin::AbuseReportLabel.count }.to(1) + + expect(mutation_response).to include('label' => a_hash_including(params)) + end + + context 'when there are errors' do + it 'does not create the label', :aggregate_failures do + create(:abuse_report_label, title: params['title']) + + expect { subject }.not_to change { Label.count } + + expect(mutation_response).to include({ + 'label' => nil, + 'errors' => ['Title has already been taken'] + }) + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/create_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/create_spec.rb index b2fe2754198..0e49fc389c8 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/create_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/create_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe 'PipelineSchedulecreate', feature_category: :continuous_integration do include GraphqlHelpers - let_it_be(:user) { create(:user) } + let_it_be(:current_user) { create(:user) } let_it_be(:project) { create(:project, :public, :repository) } let(:mutation) do @@ -56,30 +56,21 @@ RSpec.describe 'PipelineSchedulecreate', feature_category: :continuous_integrati let(:mutation_response) { graphql_mutation_response(:pipeline_schedule_create) } context 'when unauthorized' do - it 'returns an error' do - post_graphql_mutation(mutation, current_user: user) - - expect(graphql_errors).not_to be_empty - expect(graphql_errors[0]['message']) - .to eq( - "The resource that you are attempting to access does not exist " \ - "or you don't have permission to perform this action" - ) - end + it_behaves_like 'a mutation on an unauthorized resource' end context 'when authorized' do before_all do - project.add_developer(user) + project.add_developer(current_user) end context 'when success' do it do - post_graphql_mutation(mutation, current_user: user) + post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) - expect(mutation_response['pipelineSchedule']['owner']['id']).to eq(user.to_global_id.to_s) + expect(mutation_response['pipelineSchedule']['owner']['id']).to eq(current_user.to_global_id.to_s) %w[description cron cronTimezone active].each do |key| expect(mutation_response['pipelineSchedule'][key]).to eq(pipeline_schedule_parameters[key.to_sym]) @@ -90,7 +81,7 @@ RSpec.describe 'PipelineSchedulecreate', feature_category: :continuous_integrati expect(mutation_response['pipelineSchedule']['variables']['nodes'][0]['key']).to eq('AAA') expect(mutation_response['pipelineSchedule']['variables']['nodes'][0]['value']).to eq('AAA123') - expect(mutation_response['pipelineSchedule']['owner']['id']).to eq(user.to_global_id.to_s) + expect(mutation_response['pipelineSchedule']['owner']['id']).to eq(current_user.to_global_id.to_s) expect(mutation_response['errors']).to eq([]) end @@ -110,7 +101,7 @@ RSpec.describe 'PipelineSchedulecreate', feature_category: :continuous_integrati end it do - post_graphql_mutation(mutation, current_user: user) + post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) @@ -134,7 +125,7 @@ RSpec.describe 'PipelineSchedulecreate', feature_category: :continuous_integrati end it 'returns error' do - post_graphql_mutation(mutation, current_user: user) + post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/delete_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/delete_spec.rb index e79395bb52c..7b1a21971df 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/delete_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/delete_spec.rb @@ -5,9 +5,9 @@ require 'spec_helper' RSpec.describe 'PipelineScheduleDelete', feature_category: :continuous_integration do include GraphqlHelpers - let_it_be(:user) { create(:user) } + let_it_be(:current_user) { create(:user) } let_it_be(:project) { create(:project) } - let_it_be(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } + let_it_be(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: current_user) } let(:mutation) do graphql_mutation( @@ -23,26 +23,17 @@ RSpec.describe 'PipelineScheduleDelete', feature_category: :continuous_integrati let(:mutation_response) { graphql_mutation_response(:pipeline_schedule_delete) } context 'when unauthorized' do - it 'returns an error' do - post_graphql_mutation(mutation, current_user: create(:user)) - - expect(graphql_errors).not_to be_empty - expect(graphql_errors[0]['message']) - .to eq( - "The resource that you are attempting to access does not exist " \ - "or you don't have permission to perform this action" - ) - end + it_behaves_like 'a mutation on an unauthorized resource' end context 'when authorized' do before_all do - project.add_maintainer(user) + project.add_maintainer(current_user) end context 'when success' do it do - post_graphql_mutation(mutation, current_user: user) + post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) expect(mutation_response['errors']).to eq([]) @@ -58,7 +49,7 @@ RSpec.describe 'PipelineScheduleDelete', feature_category: :continuous_integrati end it do - post_graphql_mutation(mutation, current_user: user) + post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) @@ -70,7 +61,7 @@ RSpec.describe 'PipelineScheduleDelete', feature_category: :continuous_integrati let(:pipeline_schedule_id) { 'gid://gitlab/Ci::PipelineSchedule/0' } it do - post_graphql_mutation(mutation, current_user: user) + post_graphql_mutation(mutation, current_user: current_user) expect(graphql_errors).not_to be_empty expect(graphql_errors[0]['message']) diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/play_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/play_spec.rb index 55ecf8f287e..28e8913d262 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/play_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/play_spec.rb @@ -5,14 +5,14 @@ require 'spec_helper' RSpec.describe 'PipelineSchedulePlay', feature_category: :continuous_integration do include GraphqlHelpers - let_it_be(:user) { create(:user) } + let_it_be(:current_user) { create(:user) } let_it_be(:project) { create(:project) } let_it_be(:pipeline_schedule) do create( :ci_pipeline_schedule, :every_minute, project: project, - owner: user + owner: current_user ) end @@ -30,21 +30,12 @@ RSpec.describe 'PipelineSchedulePlay', feature_category: :continuous_integration let(:mutation_response) { graphql_mutation_response(:pipeline_schedule_play) } context 'when unauthorized' do - it 'returns an error' do - post_graphql_mutation(mutation, current_user: create(:user)) - - expect(graphql_errors).not_to be_empty - expect(graphql_errors[0]['message']) - .to eq( - "The resource that you are attempting to access does not exist " \ - "or you don't have permission to perform this action" - ) - end + it_behaves_like 'a mutation on an unauthorized resource' end context 'when authorized', :sidekiq_inline do before_all do - project.add_maintainer(user) + project.add_maintainer(current_user) pipeline_schedule.update_columns(next_run_at: 2.hours.ago) end @@ -54,7 +45,7 @@ RSpec.describe 'PipelineSchedulePlay', feature_category: :continuous_integration it do expect(Ci::CreatePipelineService).to receive_message_chain(:new, :execute).and_return(service_response) - post_graphql_mutation(mutation, current_user: user) + post_graphql_mutation(mutation, current_user: current_user) expect(mutation_response['pipelineSchedule']['id']).to include(pipeline_schedule.id.to_s) new_next_run_at = DateTime.parse(mutation_response['pipelineSchedule']['nextRunAt']) @@ -68,9 +59,9 @@ RSpec.describe 'PipelineSchedulePlay', feature_category: :continuous_integration it do expect(RunPipelineScheduleWorker) .to receive(:perform_async) - .with(pipeline_schedule.id, user.id).and_return(nil) + .with(pipeline_schedule.id, current_user.id).and_return(nil) - post_graphql_mutation(mutation, current_user: user) + post_graphql_mutation(mutation, current_user: current_user) expect(mutation_response['pipelineSchedule']).to be_nil expect(mutation_response['errors']).to match_array(['Unable to schedule a pipeline to run immediately.']) diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/update_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/update_spec.rb index ec1595f393f..12f6e09913a 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/update_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/update_spec.rb @@ -5,9 +5,9 @@ require 'spec_helper' RSpec.describe 'PipelineScheduleUpdate', feature_category: :continuous_integration do include GraphqlHelpers - let_it_be(:user) { create(:user) } + let_it_be(:current_user) { create(:user) } let_it_be(:project) { create(:project, :public, :repository) } - let_it_be(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } + let_it_be(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: current_user) } let_it_be(:variable_one) do create(:ci_pipeline_schedule_variable, key: 'foo', value: 'foovalue', pipeline_schedule: pipeline_schedule) @@ -51,21 +51,12 @@ RSpec.describe 'PipelineScheduleUpdate', feature_category: :continuous_integrati let(:mutation_response) { graphql_mutation_response(:pipeline_schedule_update) } context 'when unauthorized' do - it 'returns an error' do - post_graphql_mutation(mutation, current_user: create(:user)) - - expect(graphql_errors).not_to be_empty - expect(graphql_errors[0]['message']) - .to eq( - "The resource that you are attempting to access does not exist " \ - "or you don't have permission to perform this action" - ) - end + it_behaves_like 'a mutation on an unauthorized resource' end context 'when authorized' do before_all do - project.add_developer(user) + project.add_developer(current_user) end context 'when success' do @@ -83,7 +74,7 @@ RSpec.describe 'PipelineScheduleUpdate', feature_category: :continuous_integrati end it do - post_graphql_mutation(mutation, current_user: user) + post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) @@ -117,7 +108,7 @@ RSpec.describe 'PipelineScheduleUpdate', feature_category: :continuous_integrati end it 'processes variables correctly' do - post_graphql_mutation(mutation, current_user: user) + post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) @@ -145,7 +136,7 @@ RSpec.describe 'PipelineScheduleUpdate', feature_category: :continuous_integrati end it do - post_graphql_mutation(mutation, current_user: user) + post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) @@ -172,7 +163,7 @@ RSpec.describe 'PipelineScheduleUpdate', feature_category: :continuous_integrati end it 'returns error' do - post_graphql_mutation(mutation, current_user: user) + post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_trigger/create_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_trigger/create_spec.rb index 1af12d51e1e..2711bb9a4bd 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_trigger/create_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_trigger/create_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe 'PipelineTriggerCreate', feature_category: :continuous_integration do include GraphqlHelpers - let_it_be(:user) { create(:user) } + let_it_be(:current_user) { create(:user) } let_it_be_with_reload(:project) { create(:project) } let(:mutation) { graphql_mutation(:pipeline_trigger_create, params) } @@ -19,24 +19,15 @@ RSpec.describe 'PipelineTriggerCreate', feature_category: :continuous_integratio } end - subject { post_graphql_mutation(mutation, current_user: user) } + subject { post_graphql_mutation(mutation, current_user: current_user) } context 'when unauthorized' do - it 'returns an error' do - subject - - expect(graphql_errors).not_to be_empty - expect(graphql_errors[0]['message']) - .to eq( - "The resource that you are attempting to access does not exist " \ - "or you don't have permission to perform this action" - ) - end + it_behaves_like 'a mutation on an unauthorized resource' end context 'when authorized' do before_all do - project.add_owner(user) + project.add_owner(current_user) end context 'when the params are invalid' do @@ -60,9 +51,9 @@ RSpec.describe 'PipelineTriggerCreate', feature_category: :continuous_integratio expect(graphql_data_at(:pipeline_trigger_create, :pipeline_trigger)).to match a_hash_including( 'owner' => a_hash_including( - 'id' => user.to_global_id.to_s, - 'username' => user.username, - 'name' => user.name + 'id' => current_user.to_global_id.to_s, + 'username' => current_user.username, + 'name' => current_user.name ), 'description' => description, "canAccessProject" => true, diff --git a/spec/requests/api/graphql/mutations/merge_requests/update_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/update_spec.rb new file mode 100644 index 00000000000..48db23569b6 --- /dev/null +++ b/spec/requests/api/graphql/mutations/merge_requests/update_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Update of an existing merge request', feature_category: :code_review_workflow do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:project) { create(:project, :public) } + let_it_be_with_reload(:merge_request) { create(:merge_request, source_project: project) } + + let(:input) { { 'iid' => merge_request.iid.to_s } } + let(:extra_params) { { project_path: project.full_path } } + let(:input_params) { input.merge(extra_params) } + let(:mutation) { graphql_mutation(:merge_request_update, input_params, nil, ['productAnalyticsState']) } + let(:mutation_response) { graphql_mutation_response(:merge_request_update) } + + context 'when the user is not allowed to update the merge request' do + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when the user has permissions to update the merge request' do + before_all do + project.add_developer(current_user) + end + + it_behaves_like 'updating time estimate' do + let(:resource) { merge_request } + let(:mutation_name) { 'mergeRequestUpdate' } + end + end +end diff --git a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb index 0e55b6f2c9f..d05cc19de96 100644 --- a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb +++ b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb @@ -19,10 +19,6 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create, feature_categ graphql_mutation_response(:create_annotation) end - before do - stub_feature_flags(remove_monitor_metrics: false) - end - specify { expect(described_class).to require_graphql_authorizations(:admin_metrics_dashboard_annotation) } context 'when annotation source is environment' do @@ -38,18 +34,6 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create, feature_categ graphql_mutation(:create_annotation, variables) end - context 'when the user does not have permission' do - before do - project.add_reporter(current_user) - end - - it 'does not create the annotation' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - end.not_to change { Metrics::Dashboard::Annotation.count } - end - end - context 'when the user has permission' do before do project.add_developer(current_user) @@ -67,7 +51,8 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create, feature_categ graphql_mutation(:create_annotation, variables) end - it_behaves_like 'a mutation that returns top-level errors', errors: [described_class::ANNOTATION_SOURCE_ARGUMENT_ERROR] + it_behaves_like 'a mutation that returns top-level errors', + errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] end context 'when environment_id is invalid' do @@ -87,10 +72,6 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create, feature_categ end context 'when metrics dashboard feature is unavailable' do - before do - stub_feature_flags(remove_monitor_metrics: true) - end - it_behaves_like 'a mutation that returns top-level errors', errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] end @@ -127,19 +108,8 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create, feature_categ graphql_mutation(:create_annotation, variables) end - it_behaves_like 'a mutation that returns top-level errors', errors: [described_class::ANNOTATION_SOURCE_ARGUMENT_ERROR] - end - end - - context 'without permission' do - before do - project.add_guest(current_user) - end - - it 'does not create the annotation' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - end.not_to change { Metrics::Dashboard::Annotation.count } + it_behaves_like 'a mutation that returns top-level errors', + errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] end end @@ -174,7 +144,8 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create, feature_categ graphql_mutation(:create_annotation, variables) end - it_behaves_like 'a mutation that returns top-level errors', errors: [described_class::ANNOTATION_SOURCE_ARGUMENT_ERROR] + it_behaves_like 'a mutation that returns top-level errors', + errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] end [:environment_id, :cluster_id].each do |arg_name| diff --git a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb index c81f6381398..6768998b31c 100644 --- a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb +++ b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb @@ -7,19 +7,14 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Delete, feature_categ let_it_be(:current_user) { create(:user) } let_it_be(:project) { create(:project, :private, :repository) } - let_it_be(:annotation) { create(:metrics_dashboard_annotation) } - let(:variables) { { id: GitlabSchema.id_from_object(annotation).to_s } } + let(:variables) { { id: 'ids-dont-matter' } } let(:mutation) { graphql_mutation(:delete_annotation, variables) } def mutation_response graphql_mutation_response(:delete_annotation) end - before do - stub_feature_flags(remove_monitor_metrics: false) - end - specify { expect(described_class).to require_graphql_authorizations(:admin_metrics_dashboard_annotation) } context 'when the user has permission to delete the annotation' do @@ -30,16 +25,11 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Delete, feature_categ context 'with invalid params' do let(:variables) { { id: GitlabSchema.id_from_object(project).to_s } } - it_behaves_like 'a mutation that returns top-level errors' do - let(:match_errors) { contain_exactly(include('invalid value for id')) } - end + it_behaves_like 'a mutation that returns top-level errors', + errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] end context 'when metrics dashboard feature is unavailable' do - before do - stub_feature_flags(remove_monitor_metrics: true) - end - it_behaves_like 'a mutation that returns top-level errors', errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] end @@ -51,11 +41,5 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Delete, feature_categ end it_behaves_like 'a mutation that returns top-level errors', errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] - - it 'does not delete the annotation' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - end.not_to change { Metrics::Dashboard::Annotation.count } - end end end diff --git a/spec/requests/api/graphql/mutations/work_items/linked_items/add_spec.rb b/spec/requests/api/graphql/mutations/work_items/linked_items/add_spec.rb index f18e0e44905..f30b7d0ea73 100644 --- a/spec/requests/api/graphql/mutations/work_items/linked_items/add_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/linked_items/add_spec.rb @@ -104,18 +104,15 @@ RSpec.describe "Add linked items to a work item", feature_category: :portfolio_m context 'when there are more than the max allowed items to link' do let(:max_work_items) { Mutations::WorkItems::LinkedItems::Base::MAX_WORK_ITEMS } - let(:error_msg) { "No more than #{max_work_items} work items can be linked at the same time." } - - before do - max_work_items.times { |i| ids_to_link.push("gid://gitlab/WorkItem/#{i}") } - end + let(:ids_to_link) { (0..max_work_items).map { |i| "gid://gitlab/WorkItem/#{i}" } } + let(:error_msg) { "No more than #{max_work_items} work items can be modified at the same time." } it 'returns an error message' do expect do post_graphql_mutation(mutation, current_user: current_user) end.not_to change { WorkItems::RelatedWorkItemLink.count } - expect_graphql_errors_to_include("No more than #{max_work_items} work items can be linked at the same time.") + expect_graphql_errors_to_include(error_msg) end end end diff --git a/spec/requests/api/graphql/mutations/work_items/linked_items/remove_spec.rb b/spec/requests/api/graphql/mutations/work_items/linked_items/remove_spec.rb new file mode 100644 index 00000000000..2ed4e1b4602 --- /dev/null +++ b/spec/requests/api/graphql/mutations/work_items/linked_items/remove_spec.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe "Remove items linked to a work item", feature_category: :portfolio_management do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :private) } + let_it_be(:guest) { create(:user).tap { |user| project.add_guest(user) } } + let_it_be(:work_item) { create(:work_item, project: project) } + let_it_be(:related1) { create(:work_item, project: project) } + let_it_be(:related2) { create(:work_item, project: project) } + let_it_be(:link1) { create(:work_item_link, source: work_item, target: related1) } + let_it_be(:link2) { create(:work_item_link, source: work_item, target: related2) } + + let(:mutation_response) { graphql_mutation_response(:work_item_remove_linked_items) } + let(:mutation) { graphql_mutation(:workItemRemoveLinkedItems, input, fields) } + let(:ids_to_unlink) { [related1.to_global_id.to_s, related2.to_global_id.to_s] } + let(:input) { { 'id' => work_item.to_global_id.to_s, 'workItemsIds' => ids_to_unlink } } + + let(:fields) do + <<~FIELDS + workItem { + id + widgets { + type + ... on WorkItemWidgetLinkedItems { + linkedItems { + edges { + node { + linkType + workItem { + id + } + } + } + } + } + } + } + errors + message + FIELDS + end + + context 'when the user is not allowed to read the work item' do + let(:current_user) { create(:user) } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permissions to read the work item' do + let(:current_user) { guest } + + it 'unlinks the work items' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { WorkItems::RelatedWorkItemLink.count }.by(-2) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']).to include('id' => work_item.to_global_id.to_s) + expect(mutation_response['message']).to eq("Successfully unlinked IDs: #{related1.id} and #{related2.id}.") + expect(mutation_response['workItem']['widgets']).to include( + { + 'linkedItems' => { 'edges' => [] }, 'type' => 'LINKED_ITEMS' + } + ) + end + + context 'when some items fail' do + let_it_be(:other_project) { create(:project, :private) } + let_it_be(:not_related) { create(:work_item, project: project) } + let_it_be(:no_access) { create(:work_item, project: other_project) } + let_it_be(:no_access_link) { create(:work_item_link, source: work_item, target: no_access) } + + let(:ids_to_unlink) { [related1.to_global_id.to_s, not_related.to_global_id.to_s, no_access.to_global_id.to_s] } + let(:error_msg) do + "Successfully unlinked IDs: #{related1.id}. " \ + "IDs with errors: #{no_access.id} could not be removed due to insufficient permissions, " \ + "#{not_related.id} could not be removed due to not being linked." + end + + it 'remove valid item and include failing ids in response message' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { WorkItems::RelatedWorkItemLink.count }.by(-1) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['message']).to eq(error_msg) + end + end + + context 'when there are more than the max allowed items to unlink' do + let(:max_work_items) { Mutations::WorkItems::LinkedItems::Base::MAX_WORK_ITEMS } + let(:ids_to_unlink) { (0..max_work_items).map { |i| "gid://gitlab/WorkItem/#{i}" } } + + it 'returns an error message' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { WorkItems::RelatedWorkItemLink.count } + + expect_graphql_errors_to_include("No more than #{max_work_items} work items can be modified at the same time.") + end + end + + context 'when workItemsIds is empty' do + let(:ids_to_unlink) { [] } + + it_behaves_like 'a mutation that returns top-level errors', errors: ['workItemsIds cannot be empty'] + end + + context 'when `linked_work_items` feature flag is disabled' do + before do + stub_feature_flags(linked_work_items: false) + end + + it_behaves_like 'a mutation that returns a top-level access error' + end + end +end diff --git a/spec/requests/api/graphql/mutations/work_items/update_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_spec.rb index cff21c10a5a..c7c68696888 100644 --- a/spec/requests/api/graphql/mutations/work_items/update_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/update_spec.rb @@ -1431,6 +1431,20 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do 'User has not awarded emoji of type thumbsdown on the awardable' end end + + context 'when toggling award emoji' do + let(:award_action) { 'TOGGLE' } + + context 'when emoji award is present' do + let(:award_name) { 'thumbsup' } + + it_behaves_like 'request that removes emoji' + end + + context 'when emoji award is not present' do + it_behaves_like 'request that adds emoji' + end + end end end diff --git a/spec/requests/api/graphql/organizations/organization_query_spec.rb b/spec/requests/api/graphql/organizations/organization_query_spec.rb new file mode 100644 index 00000000000..d02158382eb --- /dev/null +++ b/spec/requests/api/graphql/organizations/organization_query_spec.rb @@ -0,0 +1,178 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting organization information', feature_category: :cell do + include GraphqlHelpers + + let(:query) { graphql_query_for(:organization, { id: organization.to_global_id }, organization_fields) } + let(:current_user) { user } + let(:groups) { graphql_data_at(:organization, :groups, :nodes) } + let(:organization_fields) do + <<~FIELDS + id + path + groups { + nodes { + id + } + } + FIELDS + end + + let_it_be(:organization_user) { create(:organization_user) } + let_it_be(:organization) { organization_user.organization } + let_it_be(:user) { organization_user.user } + let_it_be(:public_group) { create(:group, name: 'public-group', organization: organization) } + let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } + let_it_be(:outside_organization_group) { create(:group) } + + let_it_be(:private_group) do + create(:group, :private, name: 'private-group', organization: organization) + end + + let_it_be(:no_access_group_in_org) do + create(:group, :private, name: 'no-access', organization: organization) + end + + before_all do + private_group.add_developer(user) + public_group.add_developer(user) + other_group.add_developer(user) + outside_organization_group.add_developer(user) + end + + subject(:request_organization) { post_graphql(query, current_user: current_user) } + + context 'when the user does not have access to the organization' do + let(:current_user) { create(:user) } + + it 'returns the organization as all organizations are public' do + request_organization + + expect(graphql_data_at(:organization, :id)).to eq(organization.to_global_id.to_s) + end + end + + context 'when user has access to the organization' do + it_behaves_like 'a working graphql query' do + before do + request_organization + end + end + + context 'when resolve_organization_groups feature flag is disabled' do + before do + stub_feature_flags(resolve_organization_groups: false) + end + + it 'returns no groups' do + request_organization + + expect(graphql_data_at(:organization)).not_to be_nil + expect(graphql_data_at(:organization, :groups, :nodes)).to be_empty + end + end + + context 'when requesting organization user' do + let(:organization_fields) do + <<~FIELDS + organizationUsers { + nodes { + badges + id + user { + id + } + } + } + FIELDS + end + + it 'returns correct organization user fields' do + request_organization + + organization_user_node = graphql_data_at(:organization, :organizationUsers, :nodes).first + expected_attributes = { + "badges" => ["It's you!"], + "id" => organization_user.to_global_id.to_s, + "user" => { "id" => user.to_global_id.to_s } + } + expect(organization_user_node).to match(expected_attributes) + end + + it 'avoids N+1 queries for all the fields' do + base_query_count = ActiveRecord::QueryRecorder.new { run_query } + + organization_user_2 = create(:organization_user, organization: organization) + other_group.add_developer(organization_user_2.user) + + expect { run_query }.not_to exceed_query_limit(base_query_count) + end + + private + + def run_query + run_with_clean_state(query, context: { current_user: current_user }) + end + end + + context 'with `search` argument' do + let(:search) { 'oth' } + let(:organization_fields) do + <<~FIELDS + id + path + groups(search: "#{search}") { + nodes { + id + name + } + } + FIELDS + end + + it 'filters groups by name' do + request_organization + + expect(groups).to contain_exactly(a_graphql_entity_for(other_group)) + end + end + + context 'with `sort` argument' do + using RSpec::Parameterized::TableSyntax + + let(:authorized_groups) { [public_group, private_group, other_group] } + + where(:field, :direction, :sorted_groups) do + 'id' | 'asc' | lazy { authorized_groups.sort_by(&:id) } + 'id' | 'desc' | lazy { authorized_groups.sort_by(&:id).reverse } + 'name' | 'asc' | lazy { authorized_groups.sort_by(&:name) } + 'name' | 'desc' | lazy { authorized_groups.sort_by(&:name).reverse } + 'path' | 'asc' | lazy { authorized_groups.sort_by(&:path) } + 'path' | 'desc' | lazy { authorized_groups.sort_by(&:path).reverse } + end + + with_them do + let(:sort) { "#{field}_#{direction}".upcase } + let(:organization_fields) do + <<~FIELDS + id + path + groups(sort: #{sort}) { + nodes { + id + } + } + FIELDS + end + + it 'sorts the groups' do + request_organization + + expect(groups.pluck('id')).to eq(sorted_groups.map(&:to_global_id).map(&:to_s)) + end + end + end + end +end diff --git a/spec/requests/api/graphql/packages/package_spec.rb b/spec/requests/api/graphql/packages/package_spec.rb index 7610a4aaac1..c8cef07c4ff 100644 --- a/spec/requests/api/graphql/packages/package_spec.rb +++ b/spec/requests/api/graphql/packages/package_spec.rb @@ -16,7 +16,7 @@ RSpec.describe 'package details', feature_category: :package_registry do end let(:depth) { 3 } - let(:excluded) { %w[metadata apiFuzzingCiConfiguration pipeline packageFiles] } + let(:excluded) { %w[metadata apiFuzzingCiConfiguration pipeline packageFiles runners] } let(:metadata) { query_graphql_fragment('ComposerMetadata') } let(:package_files) { all_graphql_fields_for('PackageFile') } let(:package_global_id) { global_id_of(composer_package) } diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb index 80c7258c05d..9ca5df95d30 100644 --- a/spec/requests/api/graphql/project/merge_request_spec.rb +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -21,8 +21,11 @@ RSpec.describe 'getting merge request information nested in a project', feature_ end it_behaves_like 'a working graphql query' do - # we exclude Project.pipeline because it needs arguments - let(:mr_fields) { all_graphql_fields_for('MergeRequest', excluded: %w[jobs pipeline]) } + # we exclude Project.pipeline because it needs arguments, + # codequalityReportsComparer because no pipeline exist yet + # and runners because the user is not an admin and therefore has no access + let(:excluded) { %w[jobs pipeline runners codequalityReportsComparer] } + let(:mr_fields) { all_graphql_fields_for('MergeRequest', excluded: excluded) } before do post_graphql(query, current_user: current_user) diff --git a/spec/requests/api/graphql/project/runners_spec.rb b/spec/requests/api/graphql/project/runners_spec.rb index bee7ce2e372..68e95de49bc 100644 --- a/spec/requests/api/graphql/project/runners_spec.rb +++ b/spec/requests/api/graphql/project/runners_spec.rb @@ -28,6 +28,8 @@ RSpec.describe 'Project.runners', feature_category: :runner do ) end + subject(:request) { post_graphql(query, current_user: user) } + context 'when the user is a project admin' do before do project.add_maintainer(user) @@ -36,7 +38,7 @@ RSpec.describe 'Project.runners', feature_category: :runner do let(:expected_ids) { [project_runner, group_runner, instance_runner].map { |g| g.to_global_id.to_s } } it 'returns all runners available to project' do - post_graphql(query, current_user: user) + request expect(graphql_data_at(:project, :runners, :nodes).pluck('id')).to match_array(expected_ids) end @@ -47,10 +49,14 @@ RSpec.describe 'Project.runners', feature_category: :runner do project.add_developer(user) end - it 'returns no runners' do - post_graphql(query, current_user: user) + it 'returns nil runners and an error' do + request - expect(graphql_data_at(:project, :runners, :nodes)).to be_empty + expect(graphql_data_at(:project, :runners)).to be_nil + expect(graphql_errors).to contain_exactly(a_hash_including( + 'message' => a_string_including("you don't have permission to perform this action"), + 'path' => %w[project runners] + )) end end end diff --git a/spec/requests/api/graphql/project/work_items_spec.rb b/spec/requests/api/graphql/project/work_items_spec.rb index 4aba83dae92..d5d3d6c578f 100644 --- a/spec/requests/api/graphql/project/work_items_spec.rb +++ b/spec/requests/api/graphql/project/work_items_spec.rb @@ -30,16 +30,14 @@ RSpec.describe 'getting a work item list for a project', feature_category: :team let_it_be(:confidential_item) { create(:work_item, confidential: true, project: project, title: 'item3') } let_it_be(:other_item) { create(:work_item) } - let(:items_data) { graphql_data['project']['workItems']['edges'] } + let(:items_data) { graphql_data['project']['workItems']['nodes'] } let(:item_filter_params) { {} } let(:fields) do <<~QUERY - edges { - node { + nodes { #{all_graphql_fields_for('workItems'.classify, max_depth: 2)} } - } QUERY end @@ -69,6 +67,15 @@ RSpec.describe 'getting a work item list for a project', feature_category: :team end end + it_behaves_like 'graphql work item list request spec' do + let_it_be(:container_build_params) { { project: project } } + let(:work_item_node_path) { %w[project workItems nodes] } + + def post_query(request_user = current_user) + post_graphql(query, current_user: request_user) + end + end + describe 'N + 1 queries' do context 'when querying root fields' do it_behaves_like 'work items resolver without N + 1 queries' @@ -199,12 +206,6 @@ RSpec.describe 'getting a work item list for a project', feature_category: :team end end - it_behaves_like 'a working graphql query' do - before do - post_graphql(query, current_user: current_user) - end - end - context 'when the user does not have access to the item' do before do project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) @@ -237,25 +238,12 @@ RSpec.describe 'getting a work item list for a project', feature_category: :team context 'when filtering by search' do it_behaves_like 'query with a search term' do - let(:issuable_data) { items_data } + let(:ids) { item_ids } let(:user) { current_user } let_it_be(:issuable) { create(:work_item, project: project, description: 'bar') } end end - context 'when filtering by author username' do - let_it_be(:author) { create(:author) } - let_it_be(:item_3) { create(:work_item, project: project, author: author) } - - let(:item_filter_params) { { author_username: item_3.author.username } } - - it 'returns correct results' do - post_graphql(query, current_user: current_user) - - expect(item_ids).to match_array([item_3.to_global_id.to_s]) - end - end - describe 'sorting and pagination' do let(:data_path) { [:project, :work_items] } @@ -415,7 +403,7 @@ RSpec.describe 'getting a work item list for a project', feature_category: :team end def item_ids - graphql_dig_at(items_data, :node, :id) + graphql_dig_at(items_data, :id) end def query(params = item_filter_params) diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb index 783e96861b1..2d9c6367676 100644 --- a/spec/requests/api/graphql/project_query_spec.rb +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -56,15 +56,21 @@ RSpec.describe 'getting project information', feature_category: :groups_and_proj expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(baseline) end - context 'when other project member is not authorized to see the full token' do + context 'when another project member or owner who is not also the token owner' do before do - project.add_maintainer(other_user) + project.add_owner(other_user) post_graphql(query, current_user: other_user) end - it 'shows truncated token' do - expect(graphql_data_at(:project, :pipeline_triggers, - :nodes).first['token']).to eql pipeline_trigger.token[0, 4] + it 'is not authorized and shows truncated token' do + expect(graphql_data_at(:project, :pipeline_triggers, :nodes).first).to match({ + 'id' => pipeline_trigger.to_global_id.to_s, + 'canAccessProject' => true, + 'description' => pipeline_trigger.description, + 'hasTokenExposed' => false, + 'lastUsed' => nil, + 'token' => pipeline_trigger.token[0, 4] + }) end end @@ -199,7 +205,7 @@ RSpec.describe 'getting project information', feature_category: :groups_and_proj context 'when the project is a catalog resource' do before do - create(:catalog_resource, project: project) + create(:ci_catalog_resource, project: project) end it 'is true' do diff --git a/spec/requests/api/graphql/work_item_spec.rb b/spec/requests/api/graphql/work_item_spec.rb index fa354bc1f66..3691e023a53 100644 --- a/spec/requests/api/graphql/work_item_spec.rb +++ b/spec/requests/api/graphql/work_item_spec.rb @@ -70,7 +70,8 @@ RSpec.describe 'Query.work_item(id)', feature_category: :team_planning do 'adminWorkItem' => true, 'adminParentLink' => true, 'setWorkItemMetadata' => true, - 'createNote' => true + 'createNote' => true, + 'adminWorkItemLink' => true }, 'project' => hash_including('id' => project.to_gid.to_s, 'fullPath' => project.full_path) ) @@ -541,13 +542,10 @@ RSpec.describe 'Query.work_item(id)', feature_category: :team_planning do end describe 'linked items widget' do - let_it_be(:related_item1) { create(:work_item, project: project) } - let_it_be(:related_item2) { create(:work_item, project: project) } - let_it_be(:related_item3) { create(:work_item) } - let_it_be(:link1) { create(:work_item_link, source: work_item, target: related_item1, link_type: 'relates_to') } - let_it_be(:link2) { create(:work_item_link, source: work_item, target: related_item2, link_type: 'relates_to') } - let_it_be(:link3) { create(:work_item_link, source: work_item, target: related_item3, link_type: 'relates_to') } - + let_it_be(:related_item) { create(:work_item, project: project) } + let_it_be(:blocked_item) { create(:work_item, project: project) } + let_it_be(:link1) { create(:work_item_link, source: work_item, target: related_item, link_type: 'relates_to') } + let_it_be(:link2) { create(:work_item_link, source: work_item, target: blocked_item, link_type: 'blocks') } let(:work_item_fields) do <<~GRAPHQL id @@ -580,12 +578,12 @@ RSpec.describe 'Query.work_item(id)', feature_category: :team_planning do hash_including( 'linkId' => link1.to_gid.to_s, 'linkType' => 'relates_to', 'linkCreatedAt' => link1.created_at.iso8601, 'linkUpdatedAt' => link1.updated_at.iso8601, - 'workItem' => { 'id' => related_item1.to_gid.to_s } + 'workItem' => { 'id' => related_item.to_gid.to_s } ), hash_including( - 'linkId' => link2.to_gid.to_s, 'linkType' => 'relates_to', + 'linkId' => link2.to_gid.to_s, 'linkType' => 'blocks', 'linkCreatedAt' => link2.created_at.iso8601, 'linkUpdatedAt' => link2.updated_at.iso8601, - 'workItem' => { 'id' => related_item2.to_gid.to_s } + 'workItem' => { 'id' => blocked_item.to_gid.to_s } ) ] ) } @@ -594,6 +592,30 @@ RSpec.describe 'Query.work_item(id)', feature_category: :team_planning do ) end + context 'when filtering by link type' do + let(:work_item_fields) do + <<~GRAPHQL + widgets { + type + ... on WorkItemWidgetLinkedItems { + linkedItems(filter: RELATED) { + nodes { + linkType + } + } + } + } + GRAPHQL + end + + it 'returns items with specified type' do + widget_data = work_item_data["widgets"].find { |widget| widget.key?("linkedItems") }["linkedItems"] + + expect(widget_data["nodes"].size).to eq(1) + expect(widget_data.dig("nodes", 0, "linkType")).to eq('relates_to') + end + end + context 'when `linked_work_items` feature flag is disabled' do before do stub_feature_flags(linked_work_items: false) diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 5296a8b3e93..7b1da1c691d 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -6,7 +6,6 @@ RSpec.describe API::Groups, feature_category: :groups_and_projects do include GroupAPIHelpers include UploadHelpers include WorkhorseHelpers - include KeysetPaginationHelpers let_it_be(:user1) { create(:user, can_create_group: false) } let_it_be(:user2) { create(:user) } @@ -196,37 +195,10 @@ RSpec.describe API::Groups, feature_category: :groups_and_projects do end end - context 'keyset pagination' do - context 'on making requests with supported ordering structure' do - it 'paginates the records correctly', :aggregate_failures 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 = pagination_params_from_next_url(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 - end - - context 'on making requests with unsupported ordering structure' do - it 'returns error', :aggregate_failures 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 + it_behaves_like 'an endpoint with keyset pagination', invalid_order: 'path' do + let(:first_record) { group_1 } + let(:second_record) { group_2 } + let(:api_call) { api('/groups') } end end end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index fa35e367420..cf0cd9a2e85 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -881,7 +881,6 @@ RSpec.describe API::Internal::Base, feature_category: :system_access do end context "custom action" do - let(:access_checker) { double(Gitlab::GitAccess) } let(:payload) do { 'action' => 'geo_proxy_to_primary', @@ -898,7 +897,8 @@ RSpec.describe API::Internal::Base, feature_category: :system_access do before do project.add_guest(user) - expect(Gitlab::GitAccess).to receive(:new).with( + + expect_next_instance_of(Gitlab::GitAccess, key, project, 'ssh', @@ -907,11 +907,12 @@ RSpec.describe API::Internal::Base, feature_category: :system_access do repository_path: "#{project.full_path}.git", redirected_path: nil } - ).and_return(access_checker) - expect(access_checker).to receive(:check).with( - 'git-receive-pack', - 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master' - ).and_return(custom_action_result) + ) do |access_checker| + expect(access_checker).to receive(:check).with( + 'git-receive-pack', + 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master' + ).and_return(custom_action_result) + end end context "git push" do diff --git a/spec/requests/api/internal/kubernetes_spec.rb b/spec/requests/api/internal/kubernetes_spec.rb index ec30840dfd8..1e8397773be 100644 --- a/spec/requests/api/internal/kubernetes_spec.rb +++ b/spec/requests/api/internal/kubernetes_spec.rb @@ -131,7 +131,8 @@ RSpec.describe API::Internal::Kubernetes, feature_category: :deployment_manageme k8s_api_proxy_request: 5, flux_git_push_notifications_total: 42, k8s_api_proxy_requests_via_ci_access: 43, - k8s_api_proxy_requests_via_user_access: 44 + k8s_api_proxy_requests_via_user_access: 44, + k8s_api_proxy_requests_via_pat_access: 45 } unique_counters = { agent_users_using_ci_tunnel: [10, 999, 777, 10], @@ -139,6 +140,8 @@ RSpec.describe API::Internal::Kubernetes, feature_category: :deployment_manageme k8s_api_proxy_requests_unique_agents_via_ci_access: [10, 999, 777, 10], k8s_api_proxy_requests_unique_users_via_user_access: [10, 999, 777, 10], k8s_api_proxy_requests_unique_agents_via_user_access: [10, 999, 777, 10], + k8s_api_proxy_requests_unique_users_via_pat_access: [10, 999, 777, 10], + k8s_api_proxy_requests_unique_agents_via_pat_access: [10, 999, 777, 10], flux_git_push_notified_unique_projects: [10, 999, 777, 10] } expected_counters = { @@ -146,7 +149,8 @@ RSpec.describe API::Internal::Kubernetes, feature_category: :deployment_manageme kubernetes_agent_k8s_api_proxy_request: request_count * counters[:k8s_api_proxy_request], kubernetes_agent_flux_git_push_notifications_total: request_count * counters[:flux_git_push_notifications_total], kubernetes_agent_k8s_api_proxy_requests_via_ci_access: request_count * counters[:k8s_api_proxy_requests_via_ci_access], - kubernetes_agent_k8s_api_proxy_requests_via_user_access: request_count * counters[:k8s_api_proxy_requests_via_user_access] + kubernetes_agent_k8s_api_proxy_requests_via_user_access: request_count * counters[:k8s_api_proxy_requests_via_user_access], + kubernetes_agent_k8s_api_proxy_requests_via_pat_access: request_count * counters[:k8s_api_proxy_requests_via_pat_access] } request_count.times do @@ -492,73 +496,125 @@ RSpec.describe API::Internal::Kubernetes, feature_category: :deployment_manageme Clusters::Agents::Authorizations::UserAccess::RefreshService.new(agent, config: user_access_config).execute end - it 'returns 400 when cookie is invalid' do - send_request(params: { agent_id: agent.id, access_type: 'session_cookie', access_key: '123', csrf_token: mask_token(new_token) }) + context 'when the access type is access_token' do + let(:personal_access_token) { create(:personal_access_token, user: user, scopes: [Gitlab::Auth::K8S_PROXY_SCOPE]) } - expect(response).to have_gitlab_http_status(:bad_request) - end + it 'returns 200 when the user has access' do + deployment_project.add_member(user, :developer) - it 'returns 401 when session is not found' do - access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id('abc') - send_request(params: { agent_id: agent.id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(new_token) }) + send_request(params: { agent_id: agent.id, access_type: 'personal_access_token', access_key: personal_access_token.token }) - expect(response).to have_gitlab_http_status(:unauthorized) - end + expect(response).to have_gitlab_http_status(:success) + end - it 'returns 401 when CSRF token does not match' do - public_id = stub_user_session(user, new_token) - access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id) - send_request(params: { agent_id: agent.id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(new_token) }) + it 'returns 400 when the feature flag is disabled' do + deployment_project.add_member(user, :developer) + stub_feature_flags(k8s_proxy_pat: false) - expect(response).to have_gitlab_http_status(:unauthorized) - end + send_request(params: { agent_id: agent.id, access_type: 'personal_access_token', access_key: personal_access_token.token }) - it 'returns 404 for non-existent agent' do - token = new_token - public_id = stub_user_session(user, token) - access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id) - send_request(params: { agent_id: non_existing_record_id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(token) }) + expect(response).to have_gitlab_http_status(:bad_request) + end - expect(response).to have_gitlab_http_status(:not_found) - end + it 'returns 403 when user has no access' do + send_request(params: { agent_id: agent.id, access_type: 'personal_access_token', access_key: personal_access_token.token }) - it 'returns 403 when user has no access' do - token = new_token - public_id = stub_user_session(user, token) - access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id) - send_request(params: { agent_id: agent.id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(token) }) + expect(response).to have_gitlab_http_status(:forbidden) + end - expect(response).to have_gitlab_http_status(:forbidden) - end + it 'returns 403 when user has incorrect token scope' do + personal_access_token.update!(scopes: [Gitlab::Auth::READ_API_SCOPE]) + deployment_project.add_member(user, :developer) - it 'returns 200 when user has access' do - deployment_project.add_member(user, :developer) - token = new_token - public_id = stub_user_session(user, token) - access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id) - send_request(params: { agent_id: agent.id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(token) }) + send_request(params: { agent_id: agent.id, access_type: 'personal_access_token', access_key: personal_access_token.token }) - expect(response).to have_gitlab_http_status(:success) - end + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'returns 403 when user has no access to requested agent' do + deployment_project.add_member(user, :developer) + + send_request(params: { agent_id: another_agent.id, access_type: 'personal_access_token', access_key: personal_access_token.token }) - it 'returns 401 when user has valid KAS cookie and CSRF token but has no access to requested agent' do - deployment_project.add_member(user, :developer) - token = new_token - public_id = stub_user_session(user, token) - access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id) - send_request(params: { agent_id: another_agent.id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(token) }) + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'returns 404 for non-existent agent' do + send_request(params: { agent_id: non_existing_record_id, access_type: 'personal_access_token', access_key: personal_access_token.token }) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:not_found) + end end - it 'returns 401 when user id is not found in session' do - deployment_project.add_member(user, :developer) - token = new_token - public_id = stub_user_session_with_no_user_id(user, token) - access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id) - send_request(params: { agent_id: agent.id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(token) }) + context 'when the access type is session_cookie' do + it 'returns 400 when cookie is invalid' do + send_request(params: { agent_id: agent.id, access_type: 'session_cookie', access_key: '123', csrf_token: mask_token(new_token) }) - expect(response).to have_gitlab_http_status(:unauthorized) + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns 401 when session is not found' do + access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id('abc') + send_request(params: { agent_id: agent.id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(new_token) }) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + + it 'returns 401 when CSRF token does not match' do + public_id = stub_user_session(user, new_token) + access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id) + send_request(params: { agent_id: agent.id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(new_token) }) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + + it 'returns 404 for non-existent agent' do + token = new_token + public_id = stub_user_session(user, token) + access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id) + send_request(params: { agent_id: non_existing_record_id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(token) }) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns 403 when user has no access' do + token = new_token + public_id = stub_user_session(user, token) + access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id) + send_request(params: { agent_id: agent.id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(token) }) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'returns 200 when user has access' do + deployment_project.add_member(user, :developer) + token = new_token + public_id = stub_user_session(user, token) + access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id) + send_request(params: { agent_id: agent.id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(token) }) + + expect(response).to have_gitlab_http_status(:success) + end + + it 'returns 401 when user has valid KAS cookie and CSRF token but has no access to requested agent' do + deployment_project.add_member(user, :developer) + token = new_token + public_id = stub_user_session(user, token) + access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id) + send_request(params: { agent_id: another_agent.id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(token) }) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'returns 401 when user id is not found in session' do + deployment_project.add_member(user, :developer) + token = new_token + public_id = stub_user_session_with_no_user_id(user, token) + access_key = Gitlab::Kas::UserAccess.encrypt_public_session_id(public_id) + send_request(params: { agent_id: agent.id, access_type: 'session_cookie', access_key: access_key, csrf_token: mask_token(token) }) + + expect(response).to have_gitlab_http_status(:unauthorized) + end end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 4edcd66e91a..d3f8aeb3e76 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -113,32 +113,12 @@ RSpec.describe API::MergeRequests, :aggregate_failures, feature_category: :sourc end context 'with merge status recheck projection' do - context 'with batched_api_mergeability_checks FF on' do - it 'checks mergeability asynchronously in batch', :sidekiq_inline do - get(api(endpoint_path, user2), params: { with_merge_status_recheck: true }) - - expect_successful_response_with_paginated_array - - expect(merge_request.reload.merge_status).to eq('can_be_merged') - end - end - - context 'with batched_api_mergeability_checks FF off' do - before do - stub_feature_flags(batched_api_mergeability_checks: false) - end - - it 'checks mergeability asynchronously' do - expect_next_instances_of(check_service_class, (1..2)) do |service| - expect(service).not_to receive(:execute) - expect(service).to receive(:async_execute).and_call_original - end + it 'checks mergeability asynchronously in batch', :sidekiq_inline do + get(api(endpoint_path, user2), params: { with_merge_status_recheck: true }) - get(api(endpoint_path, user2), params: { with_merge_status_recheck: true }) + expect_successful_response_with_paginated_array - expect_successful_response_with_paginated_array - expect(mr_entity['merge_status']).to eq('checking') - end + expect(merge_request.reload.merge_status).to eq('can_be_merged') end end diff --git a/spec/requests/api/metadata_spec.rb b/spec/requests/api/metadata_spec.rb index e15186c48a5..b81fe3f51b5 100644 --- a/spec/requests/api/metadata_spec.rb +++ b/spec/requests/api/metadata_spec.rb @@ -41,6 +41,22 @@ RSpec.describe API::Metadata, feature_category: :shared do end end + context 'with ai_features scope' do + let(:scopes) { %i(ai_features) } + + it 'returns the metadata information' do + get api(endpoint, personal_access_token: personal_access_token) + + expect_metadata + end + + it 'returns "200" response on head requests' do + head api(endpoint, personal_access_token: personal_access_token) + + expect(response).to have_gitlab_http_status(:ok) + end + end + context 'with read_user scope' do let(:scopes) { %i(read_user) } @@ -57,7 +73,7 @@ RSpec.describe API::Metadata, feature_category: :shared do end end - context 'with neither api nor read_user scope' do + context 'with neither api, ai_features nor read_user scope' do let(:scopes) { %i(read_repository) } it 'returns authorization error' do diff --git a/spec/requests/api/metrics/dashboard/annotations_spec.rb b/spec/requests/api/metrics/dashboard/annotations_spec.rb index 6000fc2a6b7..0d4a112c527 100644 --- a/spec/requests/api/metrics/dashboard/annotations_spec.rb +++ b/spec/requests/api/metrics/dashboard/annotations_spec.rb @@ -10,13 +10,12 @@ RSpec.describe API::Metrics::Dashboard::Annotations, feature_category: :metrics let(:dashboard) { 'config/prometheus/common_metrics.yml' } let(:starting_at) { Time.now.iso8601 } let(:ending_at) { 1.hour.from_now.iso8601 } - let(:params) { attributes_for(:metrics_dashboard_annotation, environment: environment, starting_at: starting_at, ending_at: ending_at, dashboard_path: dashboard) } + let(:params) { { environment: environment, starting_at: starting_at, ending_at: ending_at, dashboard_path: dashboard, description: 'desc' } } shared_examples 'POST /:source_type/:id/metrics_dashboard/annotations' do |source_type| let(:url) { "/#{source_type.pluralize}/#{source.id}/metrics_dashboard/annotations" } before do - stub_feature_flags(remove_monitor_metrics: false) project.add_developer(user) end diff --git a/spec/requests/api/metrics/user_starred_dashboards_spec.rb b/spec/requests/api/metrics/user_starred_dashboards_spec.rb index bdeba777350..d42cec7af30 100644 --- a/spec/requests/api/metrics/user_starred_dashboards_spec.rb +++ b/spec/requests/api/metrics/user_starred_dashboards_spec.rb @@ -58,11 +58,6 @@ RSpec.describe API::Metrics::UserStarredDashboards, feature_category: :metrics d end describe 'DELETE /projects/:id/metrics/user_starred_dashboards' do - let_it_be(:user_starred_dashboard_1) { create(:metrics_users_starred_dashboard, user: user, project: project, dashboard_path: dashboard) } - let_it_be(:user_starred_dashboard_2) { create(:metrics_users_starred_dashboard, user: user, project: project) } - let_it_be(:other_user_starred_dashboard) { create(:metrics_users_starred_dashboard, project: project) } - let_it_be(:other_project_starred_dashboard) { create(:metrics_users_starred_dashboard, user: user) } - before do project.add_reporter(user) end diff --git a/spec/requests/api/ml/mlflow/experiments_spec.rb b/spec/requests/api/ml/mlflow/experiments_spec.rb index fc2e814752c..409b4529699 100644 --- a/spec/requests/api/ml/mlflow/experiments_spec.rb +++ b/spec/requests/api/ml/mlflow/experiments_spec.rb @@ -179,7 +179,7 @@ RSpec.describe API::Ml::Mlflow::Experiments, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' end end @@ -203,7 +203,7 @@ RSpec.describe API::Ml::Mlflow::Experiments, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' it_behaves_like 'MLflow|Bad Request on missing required', [:key, :value] end end diff --git a/spec/requests/api/ml/mlflow/runs_spec.rb b/spec/requests/api/ml/mlflow/runs_spec.rb index 45479666e9a..af04c387830 100644 --- a/spec/requests/api/ml/mlflow/runs_spec.rb +++ b/spec/requests/api/ml/mlflow/runs_spec.rb @@ -129,7 +129,7 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' end end @@ -185,6 +185,132 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do end end + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/runs/search' do + let_it_be(:search_experiment) { create(:ml_experiments, user: nil, project: project) } + let_it_be(:first_candidate) do + create(:ml_candidates, experiment: search_experiment, name: 'c', user: nil).tap do |c| + c.metrics.create!(name: 'metric1', value: 0.3) + end + end + + let_it_be(:second_candidate) do + create(:ml_candidates, experiment: search_experiment, name: 'a', user: nil).tap do |c| + c.metrics.create!(name: 'metric1', value: 0.2) + end + end + + let_it_be(:third_candidate) do + create(:ml_candidates, experiment: search_experiment, name: 'b', user: nil).tap do |c| + c.metrics.create!(name: 'metric1', value: 0.6) + end + end + + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/search" } + let(:order_by) { nil } + let(:default_params) do + { + 'max_results' => 2, + 'experiment_ids' => [search_experiment.iid], + 'order_by' => order_by + } + end + + it 'searches runs for a project', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + is_expected.to match_response_schema('ml/search_runs') + end + + describe 'pagination and ordering' do + RSpec.shared_examples 'a paginated search runs request with order' do + it 'paginates respecting the provided order by' do + first_page_runs = json_response['runs'] + expect(first_page_runs.size).to eq(2) + + expect(first_page_runs[0]['info']['run_id']).to eq(expected_order[0].eid) + expect(first_page_runs[1]['info']['run_id']).to eq(expected_order[1].eid) + + params = default_params.merge(page_token: json_response['next_page_token']) + + get api(route), params: params, headers: headers + + second_page_response = Gitlab::Json.parse(response.body) + second_page_runs = second_page_response['runs'] + + expect(second_page_response['next_page_token']).to be_nil + expect(second_page_runs.size).to eq(1) + expect(second_page_runs[0]['info']['run_id']).to eq(expected_order[2].eid) + end + end + + let(:default_order) { [third_candidate, second_candidate, first_candidate] } + + context 'when ordering is not provided' do + let(:expected_order) { default_order } + + it_behaves_like 'a paginated search runs request with order' + end + + context 'when order by column is provided', 'and column exists' do + let(:order_by) { 'name ASC' } + let(:expected_order) { [second_candidate, third_candidate, first_candidate] } + + it_behaves_like 'a paginated search runs request with order' + end + + context 'when order by column is provided', 'and column does not exist' do + let(:order_by) { 'something DESC' } + let(:expected_order) { default_order } + + it_behaves_like 'a paginated search runs request with order' + end + + context 'when order by metric is provided', 'and metric exists' do + let(:order_by) { 'metrics.metric1' } + let(:expected_order) { [third_candidate, first_candidate, second_candidate] } + + it_behaves_like 'a paginated search runs request with order' + end + + context 'when order by metric is provided', 'and metric does not exist' do + let(:order_by) { 'metrics.something' } + + it 'returns no results' do + expect(json_response['runs']).to be_empty + end + end + + context 'when order by params is provided' do + let(:order_by) { 'params.something' } + let(:expected_order) { default_order } + + it_behaves_like 'a paginated search runs request with order' + end + end + + describe 'Error States' do + context 'when experiment_ids is not passed' do + let(:default_params) { {} } + + it_behaves_like 'MLflow|Bad Request' + end + + context 'when experiment_ids is empty' do + let(:default_params) { { 'experiment_ids' => [] } } + + it_behaves_like 'MLflow|Not Found - Resource Does Not Exist' + end + + context 'when experiment_ids is invalid' do + let(:default_params) { { 'experiment_ids' => [non_existing_record_id] } } + + it_behaves_like 'MLflow|Not Found - Resource Does Not Exist' + end + + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires read_api scope' + end + end + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/update' do let(:default_params) { { run_id: candidate.eid.to_s, status: 'FAILED', end_time: Time.now.to_i } } let(:request) { post api(route), params: params, headers: headers } @@ -220,7 +346,7 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' it_behaves_like 'MLflow|run_id param error cases' end end @@ -238,7 +364,7 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do describe 'Error Cases' do it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' it_behaves_like 'MLflow|run_id param error cases' it_behaves_like 'MLflow|Bad Request on missing required', [:key, :value, :timestamp] end @@ -263,7 +389,7 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' it_behaves_like 'MLflow|run_id param error cases' it_behaves_like 'MLflow|Bad Request on missing required', [:key, :value] end @@ -288,7 +414,7 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' it_behaves_like 'MLflow|run_id param error cases' it_behaves_like 'MLflow|Bad Request on missing required', [:key, :value] end @@ -358,7 +484,7 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do end it_behaves_like 'MLflow|shared error cases' - it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Requires api scope and write permission' it_behaves_like 'MLflow|run_id param error cases' end end diff --git a/spec/requests/api/npm_group_packages_spec.rb b/spec/requests/api/npm_group_packages_spec.rb index fe0bf1d8b46..7fba75b0630 100644 --- a/spec/requests/api/npm_group_packages_spec.rb +++ b/spec/requests/api/npm_group_packages_spec.rb @@ -11,43 +11,12 @@ RSpec.describe API::NpmGroupPackages, feature_category: :package_registry do let(:url) { api("/groups/#{group.id}/-/packages/npm/#{package_name}") } it_behaves_like 'handling get metadata requests', scope: :group - - context 'with a duplicate package name in another project' do + it_behaves_like 'rejects invalid package names' do subject { get(url) } - - before do - group.add_developer(user) - end - - let_it_be(:project2) { create(:project, :public, namespace: namespace) } - let_it_be(:package2) do - create(:npm_package, - project: project2, - name: "@#{group.path}/scoped_package", - version: '1.2.0') - end - - it_behaves_like 'rejects invalid package names' - - it 'includes all matching package versions in the response' do - subject - - expect(json_response['versions'].keys).to match_array([package.version, package2.version]) - end - - context 'with the feature flag disabled' do - before do - stub_feature_flags(npm_allow_packages_in_multiple_projects: false) - end - - it 'returns matching package versions from only one project' do - subject - - expect(json_response['versions'].keys).to match_array([package2.version]) - end - end end + it_behaves_like 'handling get metadata requests for packages in multiple projects' + context 'with mixed group and project visibilities' do subject { get(url, headers: headers) } @@ -162,13 +131,13 @@ RSpec.describe API::NpmGroupPackages, feature_category: :package_registry do end end - describe 'GET /api/v4/packages/npm/-/package/*package_name/dist-tags' do + describe 'GET /api/v4/groups/:id/-/packages/npm/-/package/*package_name/dist-tags' do it_behaves_like 'handling get dist tags requests', scope: :group do let(:url) { api("/groups/#{group.id}/-/packages/npm/-/package/#{package_name}/dist-tags") } end end - describe 'PUT /api/v4/packages/npm/-/package/*package_name/dist-tags/:tag' do + describe 'PUT /api/v4/groups/:id/-/packages/npm/-/package/*package_name/dist-tags/:tag' do it_behaves_like 'handling create dist tag requests', scope: :group do let(:url) { api("/groups/#{group.id}/-/packages/npm/-/package/#{package_name}/dist-tags/#{tag_name}") } end @@ -183,7 +152,7 @@ RSpec.describe API::NpmGroupPackages, feature_category: :package_registry do end end - describe 'DELETE /api/v4/packages/npm/-/package/*package_name/dist-tags/:tag' do + describe 'DELETE /api/v4/groups/:id/-/packages/npm/-/package/*package_name/dist-tags/:tag' do it_behaves_like 'handling delete dist tag requests', scope: :group do let(:url) { api("/groups/#{group.id}/-/packages/npm/-/package/#{package_name}/dist-tags/#{tag_name}") } end diff --git a/spec/requests/api/npm_instance_packages_spec.rb b/spec/requests/api/npm_instance_packages_spec.rb index 4f965d86d66..7b74a052860 100644 --- a/spec/requests/api/npm_instance_packages_spec.rb +++ b/spec/requests/api/npm_instance_packages_spec.rb @@ -17,34 +17,7 @@ RSpec.describe API::NpmInstancePackages, feature_category: :package_registry do it_behaves_like 'handling get metadata requests', scope: :instance it_behaves_like 'rejects invalid package names' - - context 'with a duplicate package name in another project' do - let_it_be(:project2) { create(:project, :public, namespace: namespace) } - let_it_be(:package2) do - create(:npm_package, - project: project2, - name: "@#{group.path}/scoped_package", - version: '1.2.0') - end - - it 'includes all matching package versions in the response' do - subject - - expect(json_response['versions'].keys).to match_array([package.version, package2.version]) - end - - context 'with the feature flag disabled' do - before do - stub_feature_flags(npm_allow_packages_in_multiple_projects: false) - end - - it 'returns matching package versions from only one project' do - subject - - expect(json_response['versions'].keys).to match_array([package2.version]) - end - end - end + it_behaves_like 'handling get metadata requests for packages in multiple projects' context 'when metadata cache exists' do let_it_be(:npm_metadata_cache) { create(:npm_metadata_cache, package_name: package.name, project_id: project.id) } diff --git a/spec/requests/api/nuget_project_packages_spec.rb b/spec/requests/api/nuget_project_packages_spec.rb index da74409cd77..b55d992c1e4 100644 --- a/spec/requests/api/nuget_project_packages_spec.rb +++ b/spec/requests/api/nuget_project_packages_spec.rb @@ -34,6 +34,37 @@ RSpec.describe API::NugetProjectPackages, feature_category: :package_registry do it_behaves_like 'returning response status', :ok end + shared_examples 'nuget serialize odata package endpoint' do + subject { get api(url), params: params } + + it { is_expected.to have_request_urgency(:low) } + + it_behaves_like 'returning response status', :success + + it 'returns a valid xml response and invokes OdataPackageEntryService' do + expect(Packages::Nuget::OdataPackageEntryService).to receive(:new).with(target, service_params).and_call_original + + subject + + expect(response.media_type).to eq('application/xml') + end + + [nil, '', '%20', '..%2F..', '../..'].each do |value| + context "with invalid package name #{value}" do + let(:package_name) { value } + + it_behaves_like 'returning response status', :bad_request + end + end + + context 'with missing required params' do + let(:params) { {} } + let(:package_version) { nil } + + it_behaves_like 'returning response status', :bad_request + end + end + describe 'GET /api/v4/projects/:id/packages/nuget' do let(:url) { "/projects/#{target.id}/packages/nuget/index.json" } @@ -228,6 +259,43 @@ RSpec.describe API::NugetProjectPackages, feature_category: :package_registry do it_behaves_like 'rejects nuget access with invalid target id' end + describe 'GET /api/v4/projects/:id/packages/nuget/v2/FindPackagesById()' do + it_behaves_like 'nuget serialize odata package endpoint' do + let(:url) { "/projects/#{target.id}/packages/nuget/v2/FindPackagesById()" } + let(:params) { { id: "'#{package_name}'" } } + let(:service_params) { { package_name: package_name } } + end + end + + describe 'GET /api/v4/projects/:id/packages/nuget/v2/Packages()' do + it_behaves_like 'nuget serialize odata package endpoint' do + let(:url) { "/projects/#{target.id}/packages/nuget/v2/Packages()" } + let(:params) { { '$filter' => "(tolower(Id) eq '#{package_name&.downcase}')" } } + let(:service_params) { { package_name: package_name&.downcase } } + end + end + + describe 'GET /api/v4/projects/:id/packages/nuget/v2/Packages(Id=\'*\',Version=\'*\')' do + let(:package_version) { '1.0.0' } + let(:url) { "/projects/#{target.id}/packages/nuget/v2/Packages(Id='#{package_name}',Version='#{package_version}')" } + let(:params) { {} } + let(:service_params) { { package_name: package_name, package_version: package_version } } + + it_behaves_like 'nuget serialize odata package endpoint' + + context 'with invalid package version' do + subject { get api(url) } + + ['', '1', '1./2.3', '%20', '..%2F..', '../..'].each do |value| + context "with invalid package version #{value}" do + let(:package_version) { value } + + it_behaves_like 'returning response status', :bad_request + end + end + end + end + describe 'PUT /api/v4/projects/:id/packages/nuget/authorize' do it_behaves_like 'nuget authorize upload endpoint' do let(:url) { "/projects/#{target.id}/packages/nuget/authorize" } diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index aa8568d4951..d95f96c25d6 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -44,6 +44,7 @@ itself: # project - storage_version - topic_list - verification_checksum + - organization_id remapped_attributes: avatar: avatar_url build_allow_git_fetch: build_git_strategy @@ -93,6 +94,7 @@ ci_cd_settings: - id - project_id - merge_trains_enabled + - merge_trains_skip_train_allowed - merge_pipelines_enabled - auto_rollback_enabled - inbound_job_token_scope_enabled @@ -167,6 +169,7 @@ project_setting: - allow_pipeline_trigger_approve_deployment - pages_unique_domain_enabled - pages_unique_domain + - pages_multiple_versions_enabled - runner_registration_enabled - product_analytics_instrumentation_key - jitsu_host diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index 4496e3aa7c3..49471b98eba 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -470,12 +470,14 @@ RSpec.describe API::ProjectImport, :aggregate_failures, feature_category: :impor end describe 'GET /projects/:id/import' do - it 'public project accessible for an unauthenticated user' do - project = create(:project, :public) + context 'with an unauthenticated user' do + it 'returns unauthorized response for public project import status' do + project = create(:project, :public) - get api("/projects/#{project.id}/import", nil) + get api("/projects/#{project.id}/import", nil) - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:unauthorized) + end end it 'returns the import status' do diff --git a/spec/requests/api/project_packages_spec.rb b/spec/requests/api/project_packages_spec.rb index 09991be998a..219c748c9a6 100644 --- a/spec/requests/api/project_packages_spec.rb +++ b/spec/requests/api/project_packages_spec.rb @@ -553,6 +553,12 @@ RSpec.describe API::ProjectPackages, feature_category: :package_registry do let(:per_page) { 2 } + it_behaves_like 'an endpoint with keyset pagination' do + let(:first_record) { pipeline3 } + let(:second_record) { pipeline2 } + let(:api_call) { api(package_pipelines_url, user) } + end + context 'with no cursor supplied' do subject { get api(package_pipelines_url, user), params: { per_page: per_page } } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 601e8cb3081..e3e8df79a1d 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -713,7 +713,9 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and let!(:project9) { create(:project, :public, path: 'gitlab9') } before do - user.update!(starred_projects: [project5, project7, project8, project9]) + [project5, project7, project8, project9].each do |project| + user.users_star_projects.create!(project_id: project.id) + end end context 'including owned filter' do @@ -4657,8 +4659,8 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and end before do - user.update!(starred_projects: [public_project]) - private_user.update!(starred_projects: [public_project]) + user.users_star_projects.create!(project_id: public_project.id) + private_user.users_star_projects.create!(project_id: public_project.id) end it 'returns not_found(404) for not existing project' do diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index 0feff90d088..6a57cf52466 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -473,6 +473,21 @@ RSpec.describe API::Search, :clean_gitlab_redis_rate_limiting, feature_category: get api(endpoint, current_user), params: { scope: 'users', search: 'foo@bar.com' } end end + + context 'when request exceeds the rate limit', :freeze_time, :clean_gitlab_redis_rate_limiting do + before do + stub_application_setting(search_rate_limit: 1) + end + + it 'allows user whose username is in the allowlist' do + stub_application_setting(search_rate_limit_allowlist: [user.username]) + + get api(endpoint, user), params: { scope: 'users', search: 'foo@bar.com' } + get api(endpoint, user), params: { scope: 'users', search: 'foo@bar.com' } + + expect(response).to have_gitlab_http_status(:ok) + end + end end describe "GET /groups/:id/search" do @@ -658,6 +673,21 @@ RSpec.describe API::Search, :clean_gitlab_redis_rate_limiting, feature_category: get api(endpoint, current_user), params: { scope: 'users', search: 'foo@bar.com' } end end + + context 'when request exceeds the rate limit', :freeze_time, :clean_gitlab_redis_rate_limiting do + before do + stub_application_setting(search_rate_limit: 1) + end + + it 'allows user whose username is in the allowlist' do + stub_application_setting(search_rate_limit_allowlist: [user.username]) + + get api(endpoint, user), params: { scope: 'users', search: 'foo@bar.com' } + get api(endpoint, user), params: { scope: 'users', search: 'foo@bar.com' } + + expect(response).to have_gitlab_http_status(:ok) + end + end end end @@ -1057,6 +1087,21 @@ RSpec.describe API::Search, :clean_gitlab_redis_rate_limiting, feature_category: get api(endpoint, current_user), params: { scope: 'users', search: 'foo@bar.com' } end end + + context 'when request exceeds the rate limit', :freeze_time, :clean_gitlab_redis_rate_limiting do + before do + stub_application_setting(search_rate_limit: 1) + end + + it 'allows user whose username is in the allowlist' do + stub_application_setting(search_rate_limit_allowlist: [user.username]) + + get api(endpoint, user), params: { scope: 'users', search: 'foo@bar.com' } + get api(endpoint, user), params: { scope: 'users', search: 'foo@bar.com' } + + expect(response).to have_gitlab_http_status(:ok) + end + end end end end diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 12af1fc1b79..ad52076523c 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -27,6 +27,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting, featu expect(json_response['secret_detection_token_revocation_url']).to be_nil expect(json_response['secret_detection_revocation_token_types_url']).to be_nil expect(json_response['sourcegraph_public_only']).to be_truthy + expect(json_response['decompress_archive_file_timeout']).to eq(210) expect(json_response['default_preferred_language']).to be_a String expect(json_response['default_project_visibility']).to be_a String expect(json_response['default_snippet_visibility']).to be_a String @@ -153,6 +154,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting, featu enforce_terms: true, terms: 'Hello world!', performance_bar_allowed_group_path: group.full_path, + decompress_archive_file_timeout: 60, diff_max_patch_bytes: 300_000, diff_max_files: 2000, diff_max_lines: 50000, @@ -234,6 +236,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting, featu expect(json_response['enforce_terms']).to be(true) expect(json_response['terms']).to eq('Hello world!') expect(json_response['performance_bar_allowed_group_id']).to eq(group.id) + expect(json_response['decompress_archive_file_timeout']).to eq(60) expect(json_response['diff_max_patch_bytes']).to eq(300_000) expect(json_response['diff_max_files']).to eq(2000) expect(json_response['diff_max_lines']).to eq(50000) @@ -851,7 +854,8 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting, featu sentry_enabled: true, sentry_dsn: 'http://sentry.example.com', sentry_clientside_dsn: 'http://sentry.example.com', - sentry_environment: 'production' + sentry_environment: 'production', + sentry_clientside_traces_sample_rate: 0.25 } end diff --git a/spec/requests/api/usage_data_queries_spec.rb b/spec/requests/api/usage_data_queries_spec.rb index 584b0f31a07..fdd186439a6 100644 --- a/spec/requests/api/usage_data_queries_spec.rb +++ b/spec/requests/api/usage_data_queries_spec.rb @@ -6,8 +6,8 @@ require 'rake_helper' RSpec.describe API::UsageDataQueries, :aggregate_failures, feature_category: :service_ping do include UsageDataHelpers - let_it_be(:admin) { create(:user, admin: true) } - let_it_be(:user) { create(:user) } + let!(:admin) { create(:user, admin: true) } + let!(:user) { create(:user) } before do stub_usage_data_connections @@ -70,7 +70,7 @@ RSpec.describe API::UsageDataQueries, :aggregate_failures, feature_category: :se end end - context 'when querying sql metrics' do + context 'when querying sql metrics', type: :task do let(:file) { Rails.root.join('tmp', 'test', 'sql_metrics_queries.json') } before do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 81881532240..5973649a9d7 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe API::Users, :aggregate_failures, feature_category: :user_profile do include WorkhorseHelpers + include KeysetPaginationHelpers let_it_be(:admin) { create(:admin) } let_it_be(:user, reload: true) { create(:user, username: 'user.withdot') } @@ -258,6 +259,48 @@ RSpec.describe API::Users, :aggregate_failures, feature_category: :user_profile end.not_to exceed_all_query_limit(control_count) end end + + context 'when api_keyset_pagination_multi_order FF is enabled' do + before do + stub_feature_flags(api_keyset_pagination_multi_order: true) + end + + it_behaves_like 'an endpoint with keyset pagination', invalid_order: nil do + let(:first_record) { user } + let(:second_record) { admin } + let(:api_call) { api(path, user) } + end + + it 'still supports offset pagination when keyset pagination params are not provided' do + get api(path, user) + + expect(response).to include_pagination_headers + end + end + + context 'when api_keyset_pagination_multi_order FF is disabled' do + before do + stub_feature_flags(api_keyset_pagination_multi_order: false) + end + + it 'paginates the records correctly using offset pagination' do + get api(path, user), params: { pagination: 'keyset', per_page: 1 } + + params_for_next_page = pagination_params_from_next_url(response) + expect(response).to include_pagination_headers + expect(params_for_next_page).not_to include('cursor') + end + + context 'on making requests with unsupported ordering structure' do + it 'does not return error' do + get api(path, user), + params: { pagination: 'keyset', per_page: 1, order_by: 'created_at', sort: 'asc' } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + end + end + end end end @@ -494,7 +537,7 @@ RSpec.describe API::Users, :aggregate_failures, feature_category: :user_profile context "when admin" do context 'exclude_internal param' do - let_it_be(:internal_user) { User.alert_bot } + let_it_be(:internal_user) { Users::Internal.alert_bot } it 'returns all users when it is not set' do get api("/users?exclude_internal=false", admin) @@ -3602,7 +3645,7 @@ RSpec.describe API::Users, :aggregate_failures, feature_category: :user_profile end context 'for an internal user' do - let(:user) { User.alert_bot } + let(:user) { Users::Internal.alert_bot } it 'returns 403' do deactivate diff --git a/spec/requests/clusters/agents/dashboard_controller_spec.rb b/spec/requests/clusters/agents/dashboard_controller_spec.rb new file mode 100644 index 00000000000..c3c16d9b385 --- /dev/null +++ b/spec/requests/clusters/agents/dashboard_controller_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Clusters::Agents::DashboardController, feature_category: :deployment_management do + describe 'GET show' do + let_it_be(:organization) { create(:group) } + let_it_be(:agent_management_project) { create(:project, group: organization) } + let_it_be(:agent) { create(:cluster_agent, project: agent_management_project) } + let_it_be(:deployment_project) { create(:project, group: organization) } + let(:user) { create(:user) } + let(:stub_ff) { true } + + before do + allow(::Gitlab::Kas).to receive(:enabled?).and_return(true) + end + + context 'with authorized user' do + let!(:authorization) do + create( + :agent_user_access_project_authorization, + agent: agent, + project: deployment_project + ) + end + + before do + stub_feature_flags(k8s_dashboard: stub_ff) + deployment_project.add_member(user, :developer) + sign_in(user) + get kubernetes_dashboard_path(agent.id) + end + + it 'sets the kas cookie' do + expect( + request.env['action_dispatch.cookies'][Gitlab::Kas::COOKIE_KEY] + ).to be_present + end + + it 'returns not found' do + expect(response).to have_gitlab_http_status(:ok) + end + + context 'with k8s_dashboard feature flag disabled' do + let(:stub_ff) { false } + + it 'does not set the kas cookie' do + expect( + request.env['action_dispatch.cookies'][Gitlab::Kas::COOKIE_KEY] + ).not_to be_present + end + + it 'returns not found' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'with unauthorized user' do + before do + sign_in(user) + get kubernetes_dashboard_path(agent.id) + end + + it 'does not set the kas cookie' do + expect( + request.env['action_dispatch.cookies'][Gitlab::Kas::COOKIE_KEY] + ).not_to be_present + end + + it 'returns not found' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/requests/content_security_policy_spec.rb b/spec/requests/content_security_policy_spec.rb index 3f0665f1ce5..3ce7e33d88a 100644 --- a/spec/requests/content_security_policy_spec.rb +++ b/spec/requests/content_security_policy_spec.rb @@ -7,6 +7,7 @@ require 'spec_helper' # of testing in application_controller_spec. RSpec.describe 'Content Security Policy', feature_category: :application_instrumentation do let(:snowplow_host) { 'snowplow.example.com' } + let(:vite_origin) { "#{ViteRuby.instance.config.host}:#{ViteRuby.instance.config.port}" } shared_examples 'snowplow is not in the CSP' do it 'does not add the snowplow collector hostname to the CSP' do @@ -46,5 +47,33 @@ RSpec.describe 'Content Security Policy', feature_category: :application_instrum it_behaves_like 'snowplow is not in the CSP' end + + context 'when vite enabled during development', + quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/424334' do + before do + stub_rails_env('development') + stub_feature_flags(vite: true) + + get explore_root_url + end + + it 'adds vite csp' do + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers['Content-Security-Policy']).to include(vite_origin) + end + end + + context 'when vite disabled' do + before do + stub_feature_flags(vite: false) + + get explore_root_url + end + + it "doesn't add vite csp" do + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers['Content-Security-Policy']).not_to include(vite_origin) + end + end end end diff --git a/spec/requests/groups/email_campaigns_controller_spec.rb b/spec/requests/groups/email_campaigns_controller_spec.rb deleted file mode 100644 index b6e765eba37..00000000000 --- a/spec/requests/groups/email_campaigns_controller_spec.rb +++ /dev/null @@ -1,127 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Groups::EmailCampaignsController, feature_category: :navigation do - using RSpec::Parameterized::TableSyntax - - describe 'GET #index', :snowplow do - let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, group: group) } - let_it_be(:user) { create(:user) } - - let(:track) { 'create' } - let(:series) { '0' } - let(:schema) { described_class::EMAIL_CAMPAIGNS_SCHEMA_URL } - let(:subject_line_text) { Gitlab::Email::Message::InProductMarketing.for(track.to_sym).new(group: group, user: user, series: series.to_i).subject_line } - let(:data) do - { - namespace_id: group.id, - track: track.to_sym, - series: series.to_i, - subject_line: subject_line_text - } - end - - before do - sign_in(user) - group.add_developer(user) - end - - subject do - get group_email_campaigns_url(group, track: track, series: series) - response - end - - shared_examples 'track and redirect' do - it 'redirects' do - expect(subject).to have_gitlab_http_status(:redirect) - end - - context 'on SaaS', :saas do - it 'emits a snowplow event', :snowplow do - subject - - expect_snowplow_event( - category: described_class.name, - action: 'click', - context: [{ - schema: described_class::EMAIL_CAMPAIGNS_SCHEMA_URL, - data: { namespace_id: group.id, series: series.to_i, subject_line: subject_line_text, track: track.to_s } - }], - user: user, - namespace: group - ) - end - - it 'does not save the cta_click' do - expect(Users::InProductMarketingEmail).not_to receive(:save_cta_click) - - subject - end - end - - context 'when not on.com' do - it 'saves the cta_click' do - expect(Users::InProductMarketingEmail).to receive(:save_cta_click) - - subject - end - - it 'does not track snowplow events' do - subject - - expect_no_snowplow_event - end - end - end - - shared_examples 'no track and 404' do - it 'returns 404' do - expect(subject).to have_gitlab_http_status(:not_found) - end - - it 'does not emit a snowplow event', :snowplow do - subject - - expect_no_snowplow_event - end - end - - describe 'track parameter' do - context 'when valid' do - where(track: Namespaces::InProductMarketingEmailsService::TRACKS.keys.without(:experience)) - - with_them do - it_behaves_like 'track and redirect' - end - end - - context 'when invalid' do - where(track: [nil, 'xxxx']) - - with_them do - it_behaves_like 'no track and 404' - end - end - end - - describe 'series parameter' do - context 'when valid' do - where(series: (0..Namespaces::InProductMarketingEmailsService::TRACKS[:create][:interval_days].length - 1).to_a) - - with_them do - it_behaves_like 'track and redirect' - end - end - - context 'when invalid' do - where(series: [-1, nil, Namespaces::InProductMarketingEmailsService::TRACKS[:create][:interval_days].length]) - - with_them do - it_behaves_like 'no track and 404' - end - end - end - end -end diff --git a/spec/requests/groups/settings/access_tokens_controller_spec.rb b/spec/requests/groups/settings/access_tokens_controller_spec.rb index 0204af8ea8e..8d386d8c1b7 100644 --- a/spec/requests/groups/settings/access_tokens_controller_spec.rb +++ b/spec/requests/groups/settings/access_tokens_controller_spec.rb @@ -112,5 +112,27 @@ RSpec.describe Groups::Settings::AccessTokensController, feature_category: :syst expect(assigns(:active_access_tokens).to_json).to eq(active_access_tokens.to_json) end + + it 'sets available scopes' do + expect(assigns(:scopes)).to include(Gitlab::Auth::K8S_PROXY_SCOPE) + end + + context 'with feature flag k8s_proxy_pat disabled' do + before do + stub_feature_flags(k8s_proxy_pat: false) + get group_settings_access_tokens_path(resource) + end + + it 'includes details of the active group access tokens' do + active_access_tokens = + ::GroupAccessTokenSerializer.new.represent(resource_access_tokens.reverse, group: resource) + + expect(assigns(:active_access_tokens).to_json).to eq(active_access_tokens.to_json) + end + + it 'sets available scopes' do + expect(assigns(:scopes)).not_to include(Gitlab::Auth::K8S_PROXY_SCOPE) + end + end end end diff --git a/spec/requests/groups/work_items_controller_spec.rb b/spec/requests/groups/work_items_controller_spec.rb index c47b3f03ec1..e5dd88a5471 100644 --- a/spec/requests/groups/work_items_controller_spec.rb +++ b/spec/requests/groups/work_items_controller_spec.rb @@ -4,7 +4,6 @@ require 'spec_helper' RSpec.describe 'Group Level Work Items', feature_category: :team_planning do let_it_be(:group) { create(:group, :private) } - let_it_be(:project) { create(:project, group: group) } let_it_be(:developer) { create(:user).tap { |u| group.add_developer(u) } } describe 'GET /groups/:group/-/work_items' do @@ -46,4 +45,47 @@ RSpec.describe 'Group Level Work Items', feature_category: :team_planning do end end end + + describe 'GET /groups/:group/-/work_items/:iid' do + let_it_be(:work_item) { create(:work_item, :group_level, namespace: group) } + let(:work_items_path) do + url_for(controller: 'groups/work_items', action: :show, group_id: group.full_path, iid: work_item.iid) + end + + before do + sign_in(current_user) + end + + context 'when the user can read the group' do + let(:current_user) { developer } + + it 'renders index' do + get work_items_path + + expect(response).to have_gitlab_http_status(:ok) + end + + context 'when the namespace_level_work_items feature flag is disabled' do + before do + stub_feature_flags(namespace_level_work_items: false) + end + + it 'returns not found' do + get work_items_path + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when the user cannot read the group' do + let(:current_user) { create(:user) } + + it 'returns not found' do + get work_items_path + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 217241200ff..6573fe570db 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -273,7 +273,7 @@ RSpec.describe 'OpenID Connect requests', feature_category: :system_access do let(:expected_scopes) do %w[ admin_mode api read_user read_api read_repository write_repository sudo openid profile email - read_observability write_observability create_runner + read_observability write_observability create_runner k8s_proxy ai_features ] end diff --git a/spec/requests/organizations/organizations_controller_spec.rb b/spec/requests/organizations/organizations_controller_spec.rb index 788d740504a..953adb2cbf6 100644 --- a/spec/requests/organizations/organizations_controller_spec.rb +++ b/spec/requests/organizations/organizations_controller_spec.rb @@ -13,25 +13,30 @@ RSpec.describe Organizations::OrganizationsController, feature_category: :cell d end end - shared_examples 'action disabled by `ui_for_organizations` feature flag' do - before do - stub_feature_flags(ui_for_organizations: false) - end - - it 'renders 404' do + shared_examples 'redirects to sign in page' do + it 'redirects to sign in page' do gitlab_request - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to redirect_to(new_user_session_path) end end - shared_examples 'basic organization controller action' do - context 'when the user is not logged in' do - it_behaves_like 'successful response' - it_behaves_like 'action disabled by `ui_for_organizations` feature flag' + shared_examples 'action disabled by `ui_for_organizations` feature flag' do + context 'when `ui_for_organizations` feature flag is disabled' do + before do + stub_feature_flags(ui_for_organizations: false) + end + + it 'renders 404' do + gitlab_request + + expect(response).to have_gitlab_http_status(:not_found) + end end + end - context 'when the user is logged in' do + shared_examples 'when the user is signed in' do + context 'when the user is signed in' do before do sign_in(user) end @@ -63,15 +68,52 @@ RSpec.describe Organizations::OrganizationsController, feature_category: :cell d end end + shared_examples 'controller action that requires authentication' do + context 'when the user is not signed in' do + it_behaves_like 'redirects to sign in page' + + context 'when `ui_for_organizations` feature flag is disabled' do + before do + stub_feature_flags(ui_for_organizations: false) + end + + it_behaves_like 'redirects to sign in page' + end + end + + it_behaves_like 'when the user is signed in' + end + + shared_examples 'controller action that does not require authentication' do + context 'when the user is not logged in' do + it_behaves_like 'successful response' + it_behaves_like 'action disabled by `ui_for_organizations` feature flag' + end + + it_behaves_like 'when the user is signed in' + end + describe 'GET #show' do subject(:gitlab_request) { get organization_path(organization) } - it_behaves_like 'basic organization controller action' + it_behaves_like 'controller action that does not require authentication' end describe 'GET #groups_and_projects' do subject(:gitlab_request) { get groups_and_projects_organization_path(organization) } - it_behaves_like 'basic organization controller action' + it_behaves_like 'controller action that does not require authentication' + end + + describe 'GET #new' do + subject(:gitlab_request) { get new_organization_path } + + it_behaves_like 'controller action that requires authentication' + end + + describe 'GET #index' do + subject(:gitlab_request) { get organizations_path } + + it_behaves_like 'controller action that requires authentication' end end diff --git a/spec/requests/projects/noteable_notes_spec.rb b/spec/requests/projects/noteable_notes_spec.rb deleted file mode 100644 index a490e059680..00000000000 --- a/spec/requests/projects/noteable_notes_spec.rb +++ /dev/null @@ -1,78 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Project noteable notes', feature_category: :team_planning do - describe '#index' do - let_it_be(:merge_request) { create(:merge_request) } - - let(:etag_store) { Gitlab::EtagCaching::Store.new } - let(:notes_path) { project_noteable_notes_path(project, target_type: merge_request.class.name.underscore, target_id: merge_request.id) } - let(:project) { merge_request.project } - let(:user) { project.first_owner } - - let(:response_etag) { response.headers['ETag'] } - let(:stored_etag) { "W/\"#{etag_store.get(notes_path)}\"" } - - let(:default_headers) { { 'X-Last-Fetched-At' => 0 } } - - before do - login_as(user) - end - - it 'does not set a Gitlab::EtagCaching ETag if there is a note' do - create(:note_on_merge_request, noteable: merge_request, project: merge_request.project) - - get notes_path, headers: default_headers - - expect(response).to have_gitlab_http_status(:ok) - - # Rack::ETag will set an etag based on the body digest, but that doesn't - # interfere with notes pagination - expect(response_etag).not_to eq(stored_etag) - end - - it 'sets a Gitlab::EtagCaching ETag if there is no note' do - get notes_path, headers: default_headers - - expect(response).to have_gitlab_http_status(:ok) - expect(response_etag).to eq(stored_etag) - end - - it "instruments cache hits correctly" do - etag_store.touch(notes_path) - - expect(Gitlab::Metrics::RailsSlis.request_apdex).to( - receive(:increment).with( - labels: { - request_urgency: :medium, - feature_category: "team_planning", - endpoint_id: "Projects::NotesController#index" - }, - success: be_in([true, false]) - ) - ) - allow(ActiveSupport::Notifications).to receive(:instrument).and_call_original - - expect(ActiveSupport::Notifications).to( - receive(:instrument).with( - 'process_action.action_controller', - a_hash_including( - { - request_urgency: :medium, - target_duration_s: 0.5, - metadata: a_hash_including({ - 'meta.feature_category' => 'team_planning', - 'meta.caller_id' => "Projects::NotesController#index" - }) - } - ) - ) - ) - - get notes_path, headers: default_headers.merge("if-none-match": stored_etag) - - expect(response).to have_gitlab_http_status(:not_modified) - end - end -end diff --git a/spec/requests/projects/settings/access_tokens_controller_spec.rb b/spec/requests/projects/settings/access_tokens_controller_spec.rb index 666dc42bcab..b4cfa964ac8 100644 --- a/spec/requests/projects/settings/access_tokens_controller_spec.rb +++ b/spec/requests/projects/settings/access_tokens_controller_spec.rb @@ -113,5 +113,27 @@ RSpec.describe Projects::Settings::AccessTokensController, feature_category: :sy expect(assigns(:active_access_tokens).to_json).to eq(active_access_tokens.to_json) end + + it 'sets available scopes' do + expect(assigns(:scopes)).to include(Gitlab::Auth::K8S_PROXY_SCOPE) + end + + context 'with feature flag k8s_proxy_pat disabled' do + before do + stub_feature_flags(k8s_proxy_pat: false) + get project_settings_access_tokens_path(resource) + end + + it 'includes details of the active project access tokens' do + active_access_tokens = + ::ProjectAccessTokenSerializer.new.represent(resource_access_tokens.reverse, project: resource) + + expect(assigns(:active_access_tokens).to_json).to eq(active_access_tokens.to_json) + end + + it 'sets available scopes' do + expect(assigns(:scopes)).not_to include(Gitlab::Auth::K8S_PROXY_SCOPE) + end + end end end diff --git a/spec/requests/projects/tracing_controller_spec.rb b/spec/requests/projects/tracing_controller_spec.rb deleted file mode 100644 index 8996ea7f8d6..00000000000 --- a/spec/requests/projects/tracing_controller_spec.rb +++ /dev/null @@ -1,104 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::TracingController, feature_category: :tracing do - let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, group: group) } - let_it_be(:user) { create(:user) } - let(:path) { nil } - let(:observability_tracing_ff) { true } - - subject do - get path - response - end - - before do - stub_feature_flags(observability_tracing: observability_tracing_ff) - sign_in(user) - end - - shared_examples 'tracing route request' do - it_behaves_like 'observability csp policy' do - before_all do - project.add_developer(user) - end - - let(:tested_path) { path } - end - - context 'when user does not have permissions' do - it 'returns 404' do - expect(subject).to have_gitlab_http_status(:not_found) - end - end - - context 'when user has permissions' do - before_all do - project.add_developer(user) - end - - it 'returns 200' do - expect(subject).to have_gitlab_http_status(:ok) - end - - context 'when feature is disabled' do - let(:observability_tracing_ff) { false } - - it 'returns 404' do - expect(subject).to have_gitlab_http_status(:not_found) - end - end - end - end - - describe 'GET #index' do - let(:path) { project_tracing_index_path(project) } - - it_behaves_like 'tracing route request' - - describe 'html response' do - before_all do - project.add_developer(user) - end - - it 'renders the js-tracing element correctly' do - element = Nokogiri::HTML.parse(subject.body).at_css('#js-tracing') - - expected_view_model = { - tracingUrl: Gitlab::Observability.tracing_url(project), - provisioningUrl: Gitlab::Observability.provisioning_url(project), - oauthUrl: Gitlab::Observability.oauth_url - }.to_json - expect(element.attributes['data-view-model'].value).to eq(expected_view_model) - end - end - end - - describe 'GET #show' do - let(:path) { project_tracing_path(project, id: "test-trace-id") } - - it_behaves_like 'tracing route request' - - describe 'html response' do - before_all do - project.add_developer(user) - end - - it 'renders the js-tracing element correctly' do - element = Nokogiri::HTML.parse(subject.body).at_css('#js-tracing-details') - - expected_view_model = { - tracingIndexUrl: project_tracing_index_path(project), - traceId: 'test-trace-id', - tracingUrl: Gitlab::Observability.tracing_url(project), - provisioningUrl: Gitlab::Observability.provisioning_url(project), - oauthUrl: Gitlab::Observability.oauth_url - }.to_json - - expect(element.attributes['data-view-model'].value).to eq(expected_view_model) - end - end - end -end diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index 0dd8a15c3a4..3f5cd24f3dd 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -320,6 +320,120 @@ feature_category: :system_access do end end + describe 'protected paths for get' do + let(:request_method) { 'GET' } + + context 'unauthenticated requests' do + let(:protected_path_for_get_request_that_does_not_require_authentication) do + '/users/sign_in' + end + + def do_request + get protected_path_for_get_request_that_does_not_require_authentication + end + + before do + settings_to_set[:throttle_protected_paths_requests_per_period] = requests_per_period # 1 + settings_to_set[:throttle_protected_paths_period_in_seconds] = period_in_seconds # 10_000 + settings_to_set[:protected_paths_for_get_request] = %w[/users/sign_in] + end + + context 'when protected paths throttle is disabled' do + before do + settings_to_set[:throttle_protected_paths_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 + end + + context 'when protected paths throttle is enabled' do + before do + settings_to_set[:throttle_protected_paths_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 { get protected_path_for_get_request_that_does_not_require_authentication } + end + + it 'allows GET requests to unprotected paths over the rate limit' do + (1 + requests_per_period).times do + get '/api/graphql' + expect(response).to have_gitlab_http_status(:ok) + end + end + + it_behaves_like 'tracking when dry-run mode is set' do + let(:throttle_name) { 'throttle_unauthenticated_get_protected_paths' } + end + end + end + + context 'API requests authenticated with personal access token', :api do + let(:user) { create(:user) } + let(:token) { create(:personal_access_token, user: user) } + let(:other_user) { create(:user) } + let(:other_user_token) { create(:personal_access_token, user: other_user) } + let(:throttle_setting_prefix) { 'throttle_protected_paths' } + let(:api_partial_url) { '/user/emails' } + + let(:protected_paths_for_get_request) do + [ + '/api/v4/user/emails' + ] + end + + before do + settings_to_set[:protected_paths_for_get_request] = protected_paths_for_get_request + 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 user based 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 user based token-authenticated requests' + end + end + + describe 'web requests authenticated with regular login' do + let(:throttle_setting_prefix) { 'throttle_protected_paths' } + let(:user) { create(:user) } + let(:url_that_requires_authentication) { '/users/confirmation' } + + let(:protected_paths_for_get_request) do + [ + url_that_requires_authentication + ] + end + + before do + settings_to_set[:protected_paths_for_get_request] = protected_paths_for_get_request + stub_application_setting(settings_to_set) + end + + it_behaves_like 'rate-limited web authenticated requests' + end + end + describe 'Packages API' do let(:request_method) { 'GET' } diff --git a/spec/requests/search_controller_spec.rb b/spec/requests/search_controller_spec.rb index 365b20ad4aa..37474aee1ee 100644 --- a/spec/requests/search_controller_spec.rb +++ b/spec/requests/search_controller_spec.rb @@ -9,6 +9,7 @@ RSpec.describe SearchController, type: :request, feature_category: :global_searc let_it_be(:projects) { create_list(:project, 5, :public, :repository, :wiki_repo) } before do + stub_feature_flags(super_sidebar_nav_enrolled: false) login_as(user) end diff --git a/spec/requests/sessions_spec.rb b/spec/requests/sessions_spec.rb index 8e069427678..3428e607305 100644 --- a/spec/requests/sessions_spec.rb +++ b/spec/requests/sessions_spec.rb @@ -5,9 +5,9 @@ require 'spec_helper' RSpec.describe 'Sessions', feature_category: :system_access do include SessionHelpers - context 'authentication', :allow_forgery_protection do - let(:user) { create(:user) } + let(:user) { create(:user) } + context 'for authentication', :allow_forgery_protection do it 'logout does not require a csrf token' do login_as(user) @@ -17,29 +17,36 @@ RSpec.describe 'Sessions', feature_category: :system_access do end end - describe 'about_gitlab_active_user' do - before do - allow(::Gitlab).to receive(:com?).and_return(true) - end - - let(:user) { create(:user) } + describe 'gitlab_user cookie', :saas do + let_it_be(:user) { create(:user) } context 'when user signs in' do it 'sets marketing cookie' do post user_session_path(user: { login: user.username, password: user.password }) - expect(response.cookies['about_gitlab_active_user']).to be_present + expect(response.cookies['gitlab_user']).to be_present end end context 'when user uses remember_me' do it 'sets marketing cookie' do post user_session_path(user: { login: user.username, password: user.password, remember_me: true }) - expect(response.cookies['about_gitlab_active_user']).to be_present + expect(response.cookies['gitlab_user']).to be_present + end + end + + context 'when user has pending invitations' do + it 'accepts the invitations and stores a user location' do + create(:group_member, :invited, invite_email: user.email) + member = create(:group_member, :invited, invite_email: user.email) + + post user_session_path(user: { login: user.username, password: user.password }) + + expect(response).to redirect_to(activity_group_path(member.source)) end end context 'when using two-factor authentication via OTP' do - let(:user) { create(:user, :two_factor, :invalid) } + let_it_be(:user) { create(:user, :two_factor, :invalid) } let(:user_params) { { login: user.username, password: user.password } } def authenticate_2fa(otp_attempt:) @@ -67,17 +74,6 @@ RSpec.describe 'Sessions', feature_category: :system_access do end end - context 'when user signs out' do - before do - post user_session_path(user: { login: user.username, password: user.password }) - end - - it 'deletes marketing cookie' do - post(destroy_user_session_path) - expect(response.cookies['about_gitlab_active_user']).to be_nil - end - end - context 'when user is not using GitLab SaaS' do before do allow(::Gitlab).to receive(:com?).and_return(false) @@ -85,7 +81,7 @@ RSpec.describe 'Sessions', feature_category: :system_access do it 'does not set marketing cookie' do post user_session_path(user: { login: user.username, password: user.password }) - expect(response.cookies['about_gitlab_active_user']).to be_nil + expect(response.cookies['gitlab_user']).to be_nil end end end diff --git a/spec/requests/users/namespace_visits_controller_spec.rb b/spec/requests/users/namespace_visits_controller_spec.rb new file mode 100644 index 00000000000..eeeffcce67d --- /dev/null +++ b/spec/requests/users/namespace_visits_controller_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::NamespaceVisitsController, type: :request, feature_category: :navigation do + describe "POST /" do + let_it_be(:path) { track_namespace_visits_path } + let_it_be(:request_params) { nil } + + subject(:request) { post path, params: request_params } + + context "when user is not signed-in" do + it 'throws an error 302' do + subject + + expect(response).to have_gitlab_http_status(:redirect) + end + end + + context "when user is signed-in" do + let_it_be(:user) { create(:user) } + let(:server_side_frecent_namespaces) { true } + + before do + stub_feature_flags(server_side_frecent_namespaces: server_side_frecent_namespaces) + sign_in(user) + end + + context "when the server_side_frecent_namespaces feature flag is disabled" do + let(:server_side_frecent_namespaces) { false } + + it 'throws an error 302' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context "when entity type is not provided" do + let_it_be(:request_params) { { id: '1' } } + + it 'responds with a code 400' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context "when entity ID is not provided" do + let_it_be(:request_params) { { type: 'projects' } } + + it 'responds with a code 400' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context "when entity type and ID are provided" do + let_it_be(:request_params) { { type: 'projects', id: 1 } } + + it 'calls the worker and responds with a code 200' do + expect(Users::TrackNamespaceVisitsWorker).to receive(:perform_async) + + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end +end diff --git a/spec/requests/verifies_with_email_spec.rb b/spec/requests/verifies_with_email_spec.rb index cc85ebc7ade..c8a0c0975a3 100644 --- a/spec/requests/verifies_with_email_spec.rb +++ b/spec/requests/verifies_with_email_spec.rb @@ -171,8 +171,8 @@ RSpec.describe 'VerifiesWithEmail', :clean_gitlab_redis_sessions, :clean_gitlab_ it 'adds a verification error message' do expect(json_response) - .to include('message' => (s_('IdentityVerification|The code is incorrect. '\ - 'Enter it again, or send a new code.'))) + .to include('message' => s_('IdentityVerification|The code is incorrect. '\ + 'Enter it again, or send a new code.')) end end @@ -184,7 +184,7 @@ RSpec.describe 'VerifiesWithEmail', :clean_gitlab_redis_sessions, :clean_gitlab_ it 'adds a verification error message' do expect(json_response) - .to include('message' => (s_('IdentityVerification|The code has expired. Send a new code and try again.'))) + .to include('message' => s_('IdentityVerification|The code has expired. Send a new code and try again.')) end end |