diff options
Diffstat (limited to 'spec/requests')
55 files changed, 2003 insertions, 422 deletions
diff --git a/spec/requests/admin/background_migrations_controller_spec.rb b/spec/requests/admin/background_migrations_controller_spec.rb index 55971a00e55..9933008502f 100644 --- a/spec/requests/admin/background_migrations_controller_spec.rb +++ b/spec/requests/admin/background_migrations_controller_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do end describe 'POST #retry' do - let(:migration) { create(:batched_background_migration, status: 'failed') } + let(:migration) { create(:batched_background_migration, :failed) } before do create(:batched_background_migration_job, :failed, batched_migration: migration, batch_size: 10, min_value: 6, max_value: 15, attempts: 3) @@ -37,11 +37,11 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do it 'retries the migration' do retry_migration - expect(migration.reload.status).to eql 'active' + expect(migration.reload.status_name).to be :active end context 'when the migration is not failed' do - let(:migration) { create(:batched_background_migration, status: 'paused') } + let(:migration) { create(:batched_background_migration, :paused) } it 'keeps the same migration status' do expect { retry_migration }.not_to change { migration.reload.status } diff --git a/spec/requests/api/alert_management_alerts_spec.rb b/spec/requests/api/alert_management_alerts_spec.rb new file mode 100644 index 00000000000..99293e5ae95 --- /dev/null +++ b/spec/requests/api/alert_management_alerts_spec.rb @@ -0,0 +1,411 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::AlertManagementAlerts do + let_it_be(:creator) { create(:user) } + let_it_be(:project) do + create(:project, :public, creator_id: creator.id, namespace: creator.namespace) + end + + let_it_be(:user) { create(:user) } + let_it_be(:alert) { create(:alert_management_alert, project: project) } + + describe 'PUT /projects/:id/alert_management_alerts/:alert_iid/metric_images/authorize' do + include_context 'workhorse headers' + + before do + project.add_developer(user) + end + + subject do + post api("/projects/#{project.id}/alert_management_alerts/#{alert.iid}/metric_images/authorize", user), + headers: workhorse_headers + end + + it 'authorizes uploading with workhorse header' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.media_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + end + + it 'rejects requests that bypassed gitlab-workhorse' do + workhorse_headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER) + + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + + context 'when using remote storage' do + context 'when direct upload is enabled' do + before do + stub_uploads_object_storage(MetricImageUploader, enabled: true, direct_upload: true) + end + + it 'responds with status 200, location of file remote store and object details' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.media_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + expect(json_response).not_to have_key('TempPath') + expect(json_response['RemoteObject']).to have_key('ID') + expect(json_response['RemoteObject']).to have_key('GetURL') + expect(json_response['RemoteObject']).to have_key('StoreURL') + expect(json_response['RemoteObject']).to have_key('DeleteURL') + expect(json_response['RemoteObject']).to have_key('MultipartUpload') + end + end + + context 'when direct upload is disabled' do + before do + stub_uploads_object_storage(MetricImageUploader, enabled: true, direct_upload: false) + end + + it 'handles as a local file' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.media_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + expect(json_response['TempPath']).to eq(MetricImageUploader.workhorse_local_upload_path) + expect(json_response['RemoteObject']).to be_nil + end + end + end + end + + describe 'POST /projects/:id/alert_management_alerts/:alert_iid/metric_images' do + include WorkhorseHelpers + using RSpec::Parameterized::TableSyntax + + include_context 'workhorse headers' + + let(:file) { fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg') } + let(:file_name) { 'rails_sample.jpg' } + let(:url) { 'http://gitlab.com' } + let(:url_text) { 'GitLab' } + + let(:params) { { url: url, url_text: url_text } } + + subject do + workhorse_finalize( + api("/projects/#{project.id}/alert_management_alerts/#{alert.iid}/metric_images", user), + method: :post, + file_key: :file, + params: params.merge(file: file), + headers: workhorse_headers, + send_rewritten_field: true + ) + end + + shared_examples 'can_upload_metric_image' do + it 'creates a new metric image' do + subject + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['filename']).to eq(file_name) + expect(json_response['url']).to eq(url) + expect(json_response['url_text']).to eq(url_text) + expect(json_response['created_at']).not_to be_nil + expect(json_response['id']).not_to be_nil + file_path_regex = %r{/uploads/-/system/alert_management_metric_image/file/\d+/#{file_name}} + expect(json_response['file_path']).to match(file_path_regex) + end + end + + shared_examples 'unauthorized_upload' do + it 'disallows the upload' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('403 Forbidden') + end + end + + where(:user_role, :expected_status) do + :guest | :unauthorized_upload + :reporter | :unauthorized_upload + :developer | :can_upload_metric_image + end + + with_them do + before do + # Local storage + stub_uploads_object_storage(MetricImageUploader, enabled: false) + allow_next_instance_of(MetricImageUploader) do |uploader| + allow(uploader).to receive(:file_storage?).and_return(true) + end + + project.send("add_#{user_role}", user) + end + + it_behaves_like "#{params[:expected_status]}" + end + + context 'file size too large' do + before do + allow_next_instance_of(UploadedFile) do |upload_file| + allow(upload_file).to receive(:size).and_return(AlertManagement::MetricImage::MAX_FILE_SIZE + 1) + end + end + + it 'returns an error' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to match(/File is too large/) + end + end + + context 'error when saving' do + before do + project.add_developer(user) + + allow_next_instance_of(::AlertManagement::MetricImages::UploadService) do |service| + error = instance_double(ServiceResponse, success?: false, message: 'some error', http_status: :bad_request) + allow(service).to receive(:execute).and_return(error) + end + end + + it 'returns an error' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to match(/some error/) + end + end + + context 'object storage enabled' do + before do + # Object storage + stub_uploads_object_storage(MetricImageUploader) + + allow_next_instance_of(MetricImageUploader) do |uploader| + allow(uploader).to receive(:file_storage?).and_return(true) + end + project.add_developer(user) + end + + it_behaves_like 'can_upload_metric_image' + + it 'uploads to remote storage' do + subject + + last_upload = AlertManagement::MetricImage.last.uploads.last + expect(last_upload.store).to eq(::ObjectStorage::Store::REMOTE) + end + end + end + + describe 'GET /projects/:id/alert_management_alerts/:alert_iid/metric_images' do + using RSpec::Parameterized::TableSyntax + + let!(:image) { create(:alert_metric_image, alert: alert) } + + subject { get api("/projects/#{project.id}/alert_management_alerts/#{alert.iid}/metric_images", user) } + + shared_examples 'can_read_metric_image' do + it 'can read the metric images' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.first).to match( + { + id: image.id, + created_at: image.created_at.strftime('%Y-%m-%dT%H:%M:%S.%LZ'), + filename: image.filename, + file_path: image.file_path, + url: image.url, + url_text: nil + }.with_indifferent_access + ) + end + end + + shared_examples 'unauthorized_read' do + it 'cannot read the metric images' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + where(:user_role, :public_project, :expected_status) do + :not_member | false | :unauthorized_read + :not_member | true | :unauthorized_read + :guest | false | :unauthorized_read + :reporter | false | :unauthorized_read + :developer | false | :can_read_metric_image + end + + with_them do + before do + project.send("add_#{user_role}", user) unless user_role == :not_member + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) unless public_project + end + + it_behaves_like "#{params[:expected_status]}" + end + end + + describe 'PUT /projects/:id/alert_management_alerts/:alert_iid/metric_images/:metric_image_id' do + using RSpec::Parameterized::TableSyntax + + let!(:image) { create(:alert_metric_image, alert: alert) } + let(:params) { { url: 'http://test.example.com', url_text: 'Example website 123' } } + + subject do + put api("/projects/#{project.id}/alert_management_alerts/#{alert.iid}/metric_images/#{image.id}", user), + params: params + end + + shared_examples 'can_update_metric_image' do + it 'can update the metric images' do + subject + + expect(response).to have_gitlab_http_status(:success) + expect(json_response['url']).to eq(params[:url]) + expect(json_response['url_text']).to eq(params[:url_text]) + end + end + + shared_examples 'unauthorized_update' do + it 'cannot update the metric image' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + expect(image.reload).to eq(image) + end + end + + where(:user_role, :public_project, :expected_status) do + :not_member | false | :unauthorized_update + :not_member | true | :unauthorized_update + :guest | false | :unauthorized_update + :reporter | false | :unauthorized_update + :developer | false | :can_update_metric_image + end + + with_them do + before do + project.send("add_#{user_role}", user) unless user_role == :not_member + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) unless public_project + end + + it_behaves_like "#{params[:expected_status]}" + end + + context 'when user has access' do + before do + project.add_developer(user) + end + + context 'and metric image not found' do + subject do + put api("/projects/#{project.id}/alert_management_alerts/#{alert.iid}/metric_images/#{non_existing_record_id}", user) # rubocop: disable Layout/LineLength + end + + it 'returns an error' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('Metric image not found') + end + end + + context 'metric image cannot be updated' do + let(:params) { { url_text: 'something_long' * 100 } } + + it 'returns an error' do + subject + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Metric image could not be updated') + end + end + end + end + + describe 'DELETE /projects/:id/alert_management_alerts/:alert_iid/metric_images/:metric_image_id' do + using RSpec::Parameterized::TableSyntax + + let!(:image) { create(:alert_metric_image, alert: alert) } + + subject do + delete api("/projects/#{project.id}/alert_management_alerts/#{alert.iid}/metric_images/#{image.id}", user) + end + + shared_examples 'can delete metric image successfully' do + it 'can delete the metric images' do + subject + + expect(response).to have_gitlab_http_status(:no_content) + expect { image.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + shared_examples 'unauthorized delete' do + it 'cannot delete the metric image' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + expect(image.reload).to eq(image) + end + end + + where(:user_role, :public_project, :expected_status) do + :not_member | false | 'unauthorized delete' + :not_member | true | 'unauthorized delete' + :guest | false | 'unauthorized delete' + :reporter | false | 'unauthorized delete' + :developer | false | 'can delete metric image successfully' + end + + with_them do + before do + project.send("add_#{user_role}", user) unless user_role == :not_member + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) unless public_project + end + + it_behaves_like "#{params[:expected_status]}" + end + + context 'when user has access' do + before do + project.add_developer(user) + end + + context 'when metric image not found' do + subject do + delete api("/projects/#{project.id}/alert_management_alerts/#{alert.iid}/metric_images/#{non_existing_record_id}", user) # rubocop: disable Layout/LineLength + end + + it 'returns an error' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('Metric image not found') + end + end + + context 'when error when deleting' do + before do + allow_next_instance_of(AlertManagement::AlertsFinder) do |finder| + allow(finder).to receive(:execute).and_return([alert]) + end + + allow(alert).to receive_message_chain('metric_images.find_by_id') { image } + allow(image).to receive(:destroy).and_return(false) + end + + it 'returns an error' do + subject + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Metric image could not be deleted') + end + end + end + end +end diff --git a/spec/requests/api/award_emoji_spec.rb b/spec/requests/api/award_emoji_spec.rb index 07a9f7dfd74..782e14593f7 100644 --- a/spec/requests/api/award_emoji_spec.rb +++ b/spec/requests/api/award_emoji_spec.rb @@ -26,6 +26,23 @@ RSpec.describe API::AwardEmoji do expect(json_response.first['name']).to eq(award_emoji.name) end + it "includes custom emoji attributes" do + group = create(:group) + group.add_maintainer(user) + + project = create(:project, namespace: group) + custom_emoji = create(:custom_emoji, name: 'partyparrot', namespace: group) + issue = create(:issue, project: project) + create(:award_emoji, awardable: issue, user: user, name: custom_emoji.name) + + get api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.first['name']).to eq(custom_emoji.name) + expect(json_response.first['url']).to eq(custom_emoji.file) + end + it "returns a 404 error when issue id not found" do get api("/projects/#{project.id}/issues/#{non_existing_record_iid}/award_emoji", user) diff --git a/spec/requests/api/bulk_imports_spec.rb b/spec/requests/api/bulk_imports_spec.rb index 1602819a02e..3b8136f265b 100644 --- a/spec/requests/api/bulk_imports_spec.rb +++ b/spec/requests/api/bulk_imports_spec.rb @@ -18,6 +18,29 @@ RSpec.describe API::BulkImports do expect(response).to have_gitlab_http_status(:ok) expect(json_response.pluck('id')).to contain_exactly(import_1.id, import_2.id) end + + context 'sort parameter' do + it 'sorts by created_at descending by default' do + get api('/bulk_imports', user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.pluck('id')).to eq([import_2.id, import_1.id]) + end + + it 'sorts by created_at descending when explicitly specified' do + get api('/bulk_imports', user), params: { sort: 'desc' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.pluck('id')).to eq([import_2.id, import_1.id]) + end + + it 'sorts by created_at ascending when explicitly specified' do + get api('/bulk_imports', user), params: { sort: 'asc' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.pluck('id')).to eq([import_1.id, import_2.id]) + end + end end describe 'POST /bulk_imports' do diff --git a/spec/requests/api/ci/job_artifacts_spec.rb b/spec/requests/api/ci/job_artifacts_spec.rb index 0db6acbc7b8..5abff85af9c 100644 --- a/spec/requests/api/ci/job_artifacts_spec.rb +++ b/spec/requests/api/ci/job_artifacts_spec.rb @@ -82,18 +82,6 @@ RSpec.describe API::Ci::JobArtifacts do end describe 'DELETE /projects/:id/artifacts' do - context 'when feature flag is disabled' do - before do - stub_feature_flags(bulk_expire_project_artifacts: false) - end - - it 'returns 404' do - delete api("/projects/#{project.id}/artifacts", api_user) - - expect(response).to have_gitlab_http_status(:not_found) - end - end - context 'when user is anonymous' do let(:api_user) { nil } @@ -236,6 +224,8 @@ RSpec.describe API::Ci::JobArtifacts do expect(response.headers.to_h) .to include('Content-Type' => 'application/json', 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + expect(response.headers.to_h) + .not_to include('Gitlab-Workhorse-Detect-Content-Type' => 'true') expect(response.parsed_body).to be_empty end @@ -568,7 +558,8 @@ RSpec.describe API::Ci::JobArtifacts do expect(response).to have_gitlab_http_status(:ok) expect(response.headers.to_h) .to include('Content-Type' => 'application/json', - 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/, + 'Gitlab-Workhorse-Detect-Content-Type' => 'true') end end @@ -638,7 +629,8 @@ RSpec.describe API::Ci::JobArtifacts do expect(response).to have_gitlab_http_status(:ok) expect(response.headers.to_h) .to include('Content-Type' => 'application/json', - 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/, + 'Gitlab-Workhorse-Detect-Content-Type' => 'true') expect(response.parsed_body).to be_empty end end @@ -656,7 +648,8 @@ RSpec.describe API::Ci::JobArtifacts do expect(response).to have_gitlab_http_status(:ok) expect(response.headers.to_h) .to include('Content-Type' => 'application/json', - 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/, + 'Gitlab-Workhorse-Detect-Content-Type' => 'true') end end diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index f6dae7e8e23..d3820e4948e 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -623,6 +623,15 @@ RSpec.describe API::Ci::Jobs do end end + context 'when a build is not retryable' do + let(:job) { create(:ci_build, :created, pipeline: pipeline) } + + it 'responds with unprocessable entity' do + expect(json_response['message']).to eq('403 Forbidden - Job is not retryable') + expect(response).to have_gitlab_http_status(:forbidden) + end + end + context 'user without :update_build permission' do let(:api_user) { reporter } 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 d317386dc73..9e6bac41d59 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -216,7 +216,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(json_response['token']).to eq(job.token) expect(json_response['job_info']).to eq(expected_job_info) expect(json_response['git_info']).to eq(expected_git_info) - expect(json_response['image']).to eq({ 'name' => 'ruby:2.7', 'entrypoint' => '/bin/sh', 'ports' => [] }) + expect(json_response['image']).to eq({ 'name' => 'image:1.0', 'entrypoint' => '/bin/sh', 'ports' => [] }) expect(json_response['services']).to eq([{ 'name' => 'postgres', 'entrypoint' => nil, 'alias' => nil, 'command' => nil, 'ports' => [], 'variables' => nil }, { 'name' => 'docker:stable-dind', 'entrypoint' => '/bin/sh', @@ -611,6 +611,40 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end end + context 'when job has code coverage report' do + let(:job) do + create(:ci_build, :pending, :queued, :coverage_report_cobertura, + pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) + end + + let(:expected_artifacts) do + [ + { + 'name' => 'cobertura-coverage.xml', + 'paths' => ['cobertura.xml'], + 'when' => 'always', + 'expire_in' => '7d', + "artifact_type" => "cobertura", + "artifact_format" => "gzip" + } + ] + end + + it 'returns job with the correct artifact specification', :aggregate_failures do + request_job info: { platform: :darwin, features: { upload_multiple_artifacts: true } } + + expect(response).to have_gitlab_http_status(:created) + expect(response.headers['Content-Type']).to eq('application/json') + expect(response.headers).not_to have_key('X-GitLab-Last-Update') + 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['git_info']).to eq(expected_git_info) + expect(json_response['artifacts']).to eq(expected_artifacts) + end + end + context 'when triggered job is available' do let(:expected_variables) do [{ 'key' => 'CI_JOB_NAME', 'value' => 'spinach', 'public' => true, 'masked' => false }, @@ -819,11 +853,11 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do subject { request_job(id: job.id) } it_behaves_like 'storing arguments in the application context for the API' do - let(:expected_params) { { user: user.username, project: project.full_path, client_id: "user/#{user.id}" } } + let(:expected_params) { { user: user.username, project: project.full_path, client_id: "runner/#{runner.id}", job_id: job.id, pipeline_id: job.pipeline_id } } end - it_behaves_like 'not executing any extra queries for the application context', 3 do - # Extra queries: User, Project, Route + it_behaves_like 'not executing any extra queries for the application context', 4 do + # Extra queries: User, Project, Route, Runner let(:subject_proc) { proc { request_job(id: job.id) } } end end diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb index aa479cb8713..6de6d1ef222 100644 --- a/spec/requests/api/ci/secure_files_spec.rb +++ b/spec/requests/api/ci/secure_files_spec.rb @@ -6,15 +6,24 @@ RSpec.describe API::Ci::SecureFiles do before do stub_ci_secure_file_object_storage stub_feature_flags(ci_secure_files: true) + stub_feature_flags(ci_secure_files_read_only: false) end let_it_be(:maintainer) { create(:user) } let_it_be(:developer) { create(:user) } let_it_be(:guest) { create(:user) } let_it_be(:anonymous) { create(:user) } + let_it_be(:unconfirmed) { create(:user, :unconfirmed) } let_it_be(:project) { create(:project, creator_id: maintainer.id) } let_it_be(:secure_file) { create(:ci_secure_file, project: project) } + let(:file_params) do + { + file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'), + name: 'upload-keystore.jks' + } + end + before_all do project.add_maintainer(maintainer) project.add_developer(developer) @@ -39,6 +48,43 @@ RSpec.describe API::Ci::SecureFiles do end end + context 'ci_secure_files_read_only feature flag' do + context 'when the flag is enabled' do + before do + stub_feature_flags(ci_secure_files_read_only: true) + end + + it 'returns a 503 when attempting to upload a file' do + stub_feature_flags(ci_secure_files_read_only: true) + + expect do + post api("/projects/#{project.id}/secure_files", maintainer), params: file_params + end.not_to change {project.secure_files.count} + + expect(response).to have_gitlab_http_status(:service_unavailable) + end + + it 'returns a 200 when downloading a file' do + stub_feature_flags(ci_secure_files_read_only: true) + + get api("/projects/#{project.id}/secure_files", developer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_a(Array) + end + end + + context 'when the flag is disabled' do + it 'returns a 201 when uploading a file when the ci_secure_files_read_only feature flag is disabled' do + expect do + post api("/projects/#{project.id}/secure_files", maintainer), params: file_params + end.to change {project.secure_files.count}.by(1) + + expect(response).to have_gitlab_http_status(:created) + end + end + end + context 'authenticated user with admin permissions' do it 'returns project secure files' do get api("/projects/#{project.id}/secure_files", maintainer) @@ -73,6 +119,14 @@ RSpec.describe API::Ci::SecureFiles do end end + context 'unconfirmed user' do + it 'does not return project secure files' do + get api("/projects/#{project.id}/secure_files", unconfirmed) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + context 'unauthenticated user' do it 'does not return project secure files' do get api("/projects/#{project.id}/secure_files") @@ -117,6 +171,14 @@ RSpec.describe API::Ci::SecureFiles do end end + context 'unconfirmed user' do + it 'does not return project secure file details' do + get api("/projects/#{project.id}/secure_files/#{secure_file.id}", unconfirmed) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + context 'unauthenticated user' do it 'does not return project secure file details' do get api("/projects/#{project.id}/secure_files/#{secure_file.id}") @@ -167,6 +229,14 @@ RSpec.describe API::Ci::SecureFiles do end end + context 'unconfirmed user' do + it 'does not return project secure file details' do + get api("/projects/#{project.id}/secure_files/#{secure_file.id}/download", unconfirmed) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + context 'unauthenticated user' do it 'does not return project secure file details' do get api("/projects/#{project.id}/secure_files/#{secure_file.id}/download") @@ -179,14 +249,8 @@ RSpec.describe API::Ci::SecureFiles do describe 'POST /projects/:id/secure_files' do context 'authenticated user with admin permissions' do it 'creates a secure file' do - params = { - file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'), - name: 'upload-keystore.jks', - permissions: 'execute' - } - expect do - post api("/projects/#{project.id}/secure_files", maintainer), params: params + post api("/projects/#{project.id}/secure_files", maintainer), params: file_params.merge(permissions: 'execute') end.to change {project.secure_files.count}.by(1) expect(response).to have_gitlab_http_status(:created) @@ -204,26 +268,15 @@ RSpec.describe API::Ci::SecureFiles do end it 'creates a secure file with read_only permissions by default' do - params = { - file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'), - name: 'upload-keystore.jks' - } - expect do - post api("/projects/#{project.id}/secure_files", maintainer), params: params + post api("/projects/#{project.id}/secure_files", maintainer), params: file_params end.to change {project.secure_files.count}.by(1) expect(json_response['permissions']).to eq('read_only') end it 'uploads and downloads a secure file' do - post_params = { - file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'), - name: 'upload-keystore.jks', - permissions: 'read_write' - } - - post api("/projects/#{project.id}/secure_files", maintainer), params: post_params + post api("/projects/#{project.id}/secure_files", maintainer), params: file_params secure_file_id = json_response['id'] @@ -243,12 +296,8 @@ RSpec.describe API::Ci::SecureFiles do end it 'returns an error when no file is uploaded' do - post_params = { - name: 'upload-keystore.jks' - } - expect do - post api("/projects/#{project.id}/secure_files", maintainer), params: post_params + post api("/projects/#{project.id}/secure_files", maintainer), params: { name: 'upload-keystore.jks' } end.not_to change { project.secure_files.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -256,7 +305,17 @@ RSpec.describe API::Ci::SecureFiles do end it 'returns an error when the file name is missing' do + expect do + post api("/projects/#{project.id}/secure_files", maintainer), params: { file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks') } + end.not_to change { project.secure_files.count } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('name is missing') + end + + it 'returns an error when the file name has already been used' do post_params = { + name: secure_file.name, file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks') } @@ -265,18 +324,12 @@ RSpec.describe API::Ci::SecureFiles do end.not_to change { project.secure_files.count } expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('name is missing') + expect(json_response['message']['name']).to include('has already been taken') end it 'returns an error when an unexpected permission is supplied' do - post_params = { - file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'), - name: 'upload-keystore.jks', - permissions: 'foo' - } - expect do - post api("/projects/#{project.id}/secure_files", maintainer), params: post_params + post api("/projects/#{project.id}/secure_files", maintainer), params: file_params.merge(permissions: 'foo') end.not_to change { project.secure_files.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -290,13 +343,8 @@ RSpec.describe API::Ci::SecureFiles do allow(instance).to receive_message_chain(:errors, :messages).and_return(['Error 1', 'Error 2']) end - post_params = { - file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'), - name: 'upload-keystore.jks' - } - expect do - post api("/projects/#{project.id}/secure_files", maintainer), params: post_params + post api("/projects/#{project.id}/secure_files", maintainer), params: file_params end.not_to change { project.secure_files.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -307,13 +355,8 @@ RSpec.describe API::Ci::SecureFiles do allow(instance).to receive_message_chain(:file, :size).and_return(6.megabytes.to_i) end - post_params = { - file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'), - name: 'upload-keystore.jks' - } - expect do - post api("/projects/#{project.id}/secure_files", maintainer), params: post_params + post api("/projects/#{project.id}/secure_files", maintainer), params: file_params end.not_to change { project.secure_files.count } expect(response).to have_gitlab_http_status(:payload_too_large) @@ -340,6 +383,16 @@ RSpec.describe API::Ci::SecureFiles do end end + context 'unconfirmed user' do + it 'does not create a secure file' do + expect do + post api("/projects/#{project.id}/secure_files", unconfirmed) + end.not_to change { project.secure_files.count } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + context 'unauthenticated user' do it 'does not create a secure file' do expect do @@ -390,6 +443,16 @@ RSpec.describe API::Ci::SecureFiles do end end + context 'unconfirmed user' do + it 'does not delete the secure_file' do + expect do + delete api("/projects/#{project.id}/secure_files#{secure_file.id}", unconfirmed) + end.not_to change { project.secure_files.count } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + context 'unauthenticated user' do it 'does not delete the secure_file' do expect do diff --git a/spec/requests/api/clusters/agents_spec.rb b/spec/requests/api/clusters/agents_spec.rb new file mode 100644 index 00000000000..e29be255289 --- /dev/null +++ b/spec/requests/api/clusters/agents_spec.rb @@ -0,0 +1,153 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Clusters::Agents do + let_it_be(:agent) { create(:cluster_agent) } + + let(:user) { agent.created_by_user } + let(:unauthorized_user) { create(:user) } + let!(:project) { agent.project } + + before do + project.add_maintainer(user) + end + + describe 'GET /projects/:id/cluster_agents' do + context 'authorized user' do + it 'returns project agents' do + get api("/projects/#{project.id}/cluster_agents", user) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(response).to match_response_schema('public_api/v4/agents') + expect(json_response.count).to eq(1) + expect(json_response.first['name']).to eq(agent.name) + end + end + end + + context 'unauthorized user' do + it 'unable to access agents' do + get api("/projects/#{project.id}/cluster_agents", unauthorized_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + it 'avoids N+1 queries', :request_store do + # Establish baseline + get api("/projects/#{project.id}/cluster_agents", user) + + control = ActiveRecord::QueryRecorder.new do + get api("/projects/#{project.id}/cluster_agents", user) + end + + # Now create a second record and ensure that the API does not execute + # any more queries than before + create(:cluster_agent, project: project) + + expect do + get api("/projects/#{project.id}/cluster_agents", user) + end.not_to exceed_query_limit(control) + end + end + + describe 'GET /projects/:id/cluster_agents/:agent_id' do + context 'authorized user' do + it 'returns a project agent' do + get api("/projects/#{project.id}/cluster_agents/#{agent.id}", user) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/agent') + expect(json_response['name']).to eq(agent.name) + end + end + + it 'returns a 404 error if agent id is not available' do + get api("/projects/#{project.id}/cluster_agents/#{non_existing_record_id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'unauthorized user' do + it 'unable to access an existing agent' do + get api("/projects/#{project.id}/cluster_agents/#{agent.id}", unauthorized_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'POST /projects/:id/cluster_agents' do + it 'adds agent to project' do + expect do + post(api("/projects/#{project.id}/cluster_agents", user), + params: { name: 'some-agent' }) + end.to change {project.cluster_agents.count}.by(1) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/agent') + expect(json_response['name']).to eq('some-agent') + end + end + + it 'returns a 400 error if name not given' do + post api("/projects/#{project.id}/cluster_agents", user) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns a 400 error if name is invalid' do + post api("/projects/#{project.id}/cluster_agents", user), params: { name: '#4^x' } + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']) + .to include("Name can contain only lowercase letters, digits, and '-', but cannot start or end with '-'") + end + end + + it 'returns 404 error if project does not exist' do + post api("/projects/#{non_existing_record_id}/cluster_agents", user), params: { name: 'some-agent' } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + describe 'DELETE /projects/:id/cluster_agents/:agent_id' do + it 'deletes agent from project' do + expect do + delete api("/projects/#{project.id}/cluster_agents/#{agent.id}", user) + + expect(response).to have_gitlab_http_status(:no_content) + end.to change {project.cluster_agents.count}.by(-1) + end + + it 'returns a 404 error when deleting non existent agent' do + delete api("/projects/#{project.id}/cluster_agents/#{non_existing_record_id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns a 404 error if agent id not given' do + delete api("/projects/#{project.id}/cluster_agents", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns a 404 if the user is unauthorized to delete' do + delete api("/projects/#{project.id}/cluster_agents/#{agent.id}", unauthorized_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/cluster_agents/#{agent.id}", user) } + end + end +end diff --git a/spec/requests/api/composer_packages_spec.rb b/spec/requests/api/composer_packages_spec.rb index bc30fc3b230..53f3ef10743 100644 --- a/spec/requests/api/composer_packages_spec.rb +++ b/spec/requests/api/composer_packages_spec.rb @@ -430,11 +430,23 @@ RSpec.describe API::ComposerPackages do context 'with valid project' do let!(:package) { create(:composer_package, :with_metadatum, name: package_name, project: project) } + let(:headers) { basic_auth_header(user.username, personal_access_token.token) } + + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end context 'when the sha does not match the package name' do let(:sha) { '123' } + let(:headers) { basic_auth_header(user.username, personal_access_token.token) } - it_behaves_like 'process Composer api request', :anonymous, :not_found + context 'anonymous' do + let(:headers) { {} } + + it_behaves_like 'process Composer api request', :anonymous, :unauthorized + end + + it_behaves_like 'process Composer api request', :developer, :not_found end context 'when the package name does not match the sha' do @@ -442,7 +454,13 @@ RSpec.describe API::ComposerPackages do let(:sha) { branch.target } let(:url) { "/projects/#{project.id}/packages/composer/archives/unexisting-package-name.zip" } - it_behaves_like 'process Composer api request', :anonymous, :not_found + context 'anonymous' do + let(:headers) { {} } + + it_behaves_like 'process Composer api request', :anonymous, :unauthorized + end + + it_behaves_like 'process Composer api request', :developer, :not_found end context 'with a match package name and sha' do @@ -460,14 +478,14 @@ RSpec.describe API::ComposerPackages do 'PUBLIC' | :guest | false | false | :success 'PUBLIC' | :anonymous | false | true | :success 'PRIVATE' | :developer | true | true | :success - 'PRIVATE' | :developer | true | false | :success - 'PRIVATE' | :developer | false | true | :success - 'PRIVATE' | :developer | false | false | :success - 'PRIVATE' | :guest | true | true | :success - 'PRIVATE' | :guest | true | false | :success - 'PRIVATE' | :guest | false | true | :success - 'PRIVATE' | :guest | false | false | :success - 'PRIVATE' | :anonymous | false | true | :success + 'PRIVATE' | :developer | true | false | :unauthorized + 'PRIVATE' | :developer | false | true | :not_found + 'PRIVATE' | :developer | false | false | :unauthorized + 'PRIVATE' | :guest | true | true | :forbidden + 'PRIVATE' | :guest | true | false | :unauthorized + 'PRIVATE' | :guest | false | true | :not_found + 'PRIVATE' | :guest | false | false | :unauthorized + 'PRIVATE' | :anonymous | false | true | :unauthorized end with_them do @@ -480,8 +498,17 @@ RSpec.describe API::ComposerPackages do end it_behaves_like 'process Composer api request', params[:user_role], params[:expected_status], params[:member] - it_behaves_like 'a package tracking event', described_class.name, 'pull_package' + + include_context 'Composer user type', params[:user_role], params[:member] do + if params[:expected_status] == :success + it_behaves_like 'a package tracking event', described_class.name, 'pull_package' + else + it_behaves_like 'not a package tracking event' + end + end end + + it_behaves_like 'Composer publish with deploy tokens' end end diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 6aa12b6ff48..cb0b5f6bfc3 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -9,6 +9,7 @@ RSpec.describe API::Files do let!(:project) { create(:project, :repository, namespace: user.namespace ) } let(:guest) { create(:user) { |u| project.add_guest(u) } } let(:file_path) { "files%2Fruby%2Fpopen%2Erb" } + let(:executable_file_path) { "files%2Fexecutables%2Fls" } let(:rouge_file_path) { "%2e%2e%2f" } let(:absolute_path) { "%2Fetc%2Fpasswd.rb" } let(:invalid_file_message) { 'file_path should be a valid file path' } @@ -18,6 +19,12 @@ RSpec.describe API::Files do } end + let(:executable_ref_params) do + { + ref: 'with-executables' + } + end + let(:author_email) { 'user@example.org' } let(:author_name) { 'John Doe' } @@ -219,9 +226,26 @@ RSpec.describe API::Files do expect(json_response['file_name']).to eq('popen.rb') expect(json_response['last_commit_id']).to eq('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') expect(json_response['content_sha256']).to eq('c440cd09bae50c4632cc58638ad33c6aa375b6109d811e76a9cc3a613c1e8887') + expect(json_response['execute_filemode']).to eq(false) expect(Base64.decode64(json_response['content']).lines.first).to eq("require 'fileutils'\n") end + context 'for executable file' do + it 'returns file attributes as json' do + get api(route(executable_file_path), api_user, **options), params: executable_ref_params + + aggregate_failures 'testing response' do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['file_path']).to eq(CGI.unescape(executable_file_path)) + expect(json_response['file_name']).to eq('ls') + expect(json_response['last_commit_id']).to eq('6b8dc4a827797aa025ff6b8f425e583858a10d4f') + expect(json_response['content_sha256']).to eq('2c74b1181ef780dfb692c030d3a0df6e0b624135c38a9344e56b9f80007b6191') + expect(json_response['execute_filemode']).to eq(true) + expect(Base64.decode64(json_response['content']).lines.first).to eq("#!/bin/sh\n") + end + end + end + it 'returns json when file has txt extension' do file_path = "bar%2Fbranch-test.txt" @@ -386,6 +410,23 @@ RSpec.describe API::Files do expect(response.headers['X-Gitlab-Last-Commit-Id']).to eq('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') expect(response.headers['X-Gitlab-Content-Sha256']) .to eq('c440cd09bae50c4632cc58638ad33c6aa375b6109d811e76a9cc3a613c1e8887') + expect(response.headers['X-Gitlab-Execute-Filemode']).to eq("false") + end + + context 'for executable file' do + it 'returns file attributes in headers' do + head api(route(executable_file_path) + '/blame', current_user), params: executable_ref_params + + aggregate_failures 'testing response' do + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers['X-Gitlab-File-Path']).to eq(CGI.unescape(executable_file_path)) + expect(response.headers['X-Gitlab-File-Name']).to eq('ls') + expect(response.headers['X-Gitlab-Last-Commit-Id']).to eq('6b8dc4a827797aa025ff6b8f425e583858a10d4f') + expect(response.headers['X-Gitlab-Content-Sha256']) + .to eq('2c74b1181ef780dfb692c030d3a0df6e0b624135c38a9344e56b9f80007b6191') + expect(response.headers['X-Gitlab-Execute-Filemode']).to eq("true") + end + end end it 'returns 400 when file path is invalid' do @@ -642,6 +683,15 @@ RSpec.describe API::Files do } end + let(:executable_params) do + { + branch: "master", + content: "puts 8", + commit_message: "Added newfile", + execute_filemode: true + } + end + it 'returns 400 when file path is invalid' do post api(route(rouge_file_path), user), params: params @@ -661,6 +711,18 @@ RSpec.describe API::Files do last_commit = project.repository.commit.raw expect(last_commit.author_email).to eq(user.email) expect(last_commit.author_name).to eq(user.name) + expect(project.repository.blob_at_branch(params[:branch], CGI.unescape(file_path)).executable?).to eq(false) + end + + it "creates a new executable file in project repo" do + post api(route(file_path), user), params: executable_params + + expect(response).to have_gitlab_http_status(:created) + expect(json_response["file_path"]).to eq(CGI.unescape(file_path)) + last_commit = project.repository.commit.raw + expect(last_commit.author_email).to eq(user.email) + expect(last_commit.author_name).to eq(user.name) + expect(project.repository.blob_at_branch(params[:branch], CGI.unescape(file_path)).executable?).to eq(true) end it "returns a 400 bad request if no mandatory params given" do @@ -820,6 +882,44 @@ RSpec.describe API::Files do expect(last_commit.author_name).to eq(author_name) end end + + context 'when specifying the execute_filemode' do + let(:executable_params) do + { + branch: 'master', + content: 'puts 8', + commit_message: 'Changed file', + execute_filemode: true + } + end + + let(:non_executable_params) do + { + branch: 'with-executables', + content: 'puts 8', + commit_message: 'Changed file', + execute_filemode: false + } + end + + it 'updates to executable file mode' do + put api(route(file_path), user), params: executable_params + + aggregate_failures 'testing response' do + expect(response).to have_gitlab_http_status(:ok) + expect(project.repository.blob_at_branch(executable_params[:branch], CGI.unescape(file_path)).executable?).to eq(true) + end + end + + it 'updates to non-executable file mode' do + put api(route(executable_file_path), user), params: non_executable_params + + aggregate_failures 'testing response' do + expect(response).to have_gitlab_http_status(:ok) + expect(project.repository.blob_at_branch(non_executable_params[:branch], CGI.unescape(executable_file_path)).executable?).to eq(false) + end + end + end end describe "DELETE /projects/:id/repository/files" do diff --git a/spec/requests/api/graphql/ci/job_spec.rb b/spec/requests/api/graphql/ci/job_spec.rb index b0514a0a963..ddb2664d353 100644 --- a/spec/requests/api/graphql/ci/job_spec.rb +++ b/spec/requests/api/graphql/ci/job_spec.rb @@ -52,6 +52,7 @@ RSpec.describe 'Query.project(fullPath).pipelines.job(id)' do 'name' => job_2.name, 'allowFailure' => job_2.allow_failure, 'duration' => 25, + 'kind' => 'BUILD', 'queuedDuration' => 2.0, 'status' => job_2.status.upcase ) diff --git a/spec/requests/api/graphql/ci/jobs_spec.rb b/spec/requests/api/graphql/ci/jobs_spec.rb index b191b585d06..2d1bb45390b 100644 --- a/spec/requests/api/graphql/ci/jobs_spec.rb +++ b/spec/requests/api/graphql/ci/jobs_spec.rb @@ -155,6 +155,56 @@ RSpec.describe 'Query.project.pipeline' do end end + describe '.jobs.kind' do + 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 { + kind + } + } + } + } + } + } + } + } + } + ) + end + + context 'when the job is a build' do + it 'returns BUILD' do + create(:ci_build, pipeline: pipeline) + + post_graphql(query, current_user: user) + + job_data = graphql_data_at(:project, :pipeline, :stages, :nodes, :groups, :nodes, :jobs, :nodes).first + expect(job_data['kind']).to eq 'BUILD' + end + end + + context 'when the job is a bridge' do + it 'returns BRIDGE' do + create(:ci_bridge, pipeline: pipeline) + + post_graphql(query, current_user: user) + + job_data = graphql_data_at(:project, :pipeline, :stages, :nodes, :groups, :nodes, :jobs, :nodes).first + expect(job_data['kind']).to eq 'BRIDGE' + end + end + end + describe '.jobs.artifacts' do let_it_be(:pipeline) { create(:ci_pipeline, project: project) } diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index b99a3d14fb9..39f0f696b08 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -27,27 +27,18 @@ RSpec.describe 'Query.runner(id)' do let_it_be(:active_project_runner) { create(:ci_runner, :project) } - def get_runner(id) - case id - when :active_instance_runner - active_instance_runner - when :inactive_instance_runner - inactive_instance_runner - when :active_group_runner - active_group_runner - when :active_project_runner - active_project_runner - end + before do + allow(Gitlab::Ci::RunnerUpgradeCheck.instance).to receive(:check_runner_upgrade_status) end - shared_examples 'runner details fetch' do |runner_id| + shared_examples 'runner details fetch' do let(:query) do wrap_fields(query_graphql_path(query_path, all_graphql_fields_for('CiRunner'))) end let(:query_path) do [ - [:runner, { id: get_runner(runner_id).to_global_id.to_s }] + [:runner, { id: runner.to_global_id.to_s }] ] end @@ -57,7 +48,6 @@ RSpec.describe 'Query.runner(id)' do runner_data = graphql_data_at(:runner) expect(runner_data).not_to be_nil - runner = get_runner(runner_id) expect(runner_data).to match a_hash_including( 'id' => runner.to_global_id.to_s, 'description' => runner.description, @@ -90,14 +80,14 @@ RSpec.describe 'Query.runner(id)' do end end - shared_examples 'retrieval with no admin url' do |runner_id| + shared_examples 'retrieval with no admin url' do let(:query) do wrap_fields(query_graphql_path(query_path, all_graphql_fields_for('CiRunner'))) end let(:query_path) do [ - [:runner, { id: get_runner(runner_id).to_global_id.to_s }] + [:runner, { id: runner.to_global_id.to_s }] ] end @@ -107,7 +97,6 @@ RSpec.describe 'Query.runner(id)' do runner_data = graphql_data_at(:runner) expect(runner_data).not_to be_nil - runner = get_runner(runner_id) expect(runner_data).to match a_hash_including( 'id' => runner.to_global_id.to_s, 'adminUrl' => nil @@ -116,14 +105,14 @@ RSpec.describe 'Query.runner(id)' do end end - shared_examples 'retrieval by unauthorized user' do |runner_id| + shared_examples 'retrieval by unauthorized user' do let(:query) do wrap_fields(query_graphql_path(query_path, all_graphql_fields_for('CiRunner'))) end let(:query_path) do [ - [:runner, { id: get_runner(runner_id).to_global_id.to_s }] + [:runner, { id: runner.to_global_id.to_s }] ] end @@ -135,7 +124,9 @@ RSpec.describe 'Query.runner(id)' do end describe 'for active runner' do - it_behaves_like 'runner details fetch', :active_instance_runner + let(:runner) { active_instance_runner } + + it_behaves_like 'runner details fetch' context 'when tagList is not requested' do let(:query) do @@ -144,7 +135,7 @@ RSpec.describe 'Query.runner(id)' do let(:query_path) do [ - [:runner, { id: active_instance_runner.to_global_id.to_s }] + [:runner, { id: runner.to_global_id.to_s }] ] end @@ -193,7 +184,9 @@ RSpec.describe 'Query.runner(id)' do end describe 'for inactive runner' do - it_behaves_like 'runner details fetch', :inactive_instance_runner + let(:runner) { inactive_instance_runner } + + it_behaves_like 'runner details fetch' end describe 'for group runner request' do @@ -369,15 +362,21 @@ RSpec.describe 'Query.runner(id)' do let(:user) { create(:user) } context 'on instance runner' do - it_behaves_like 'retrieval by unauthorized user', :active_instance_runner + let(:runner) { active_instance_runner } + + it_behaves_like 'retrieval by unauthorized user' end context 'on group runner' do - it_behaves_like 'retrieval by unauthorized user', :active_group_runner + let(:runner) { active_group_runner } + + it_behaves_like 'retrieval by unauthorized user' end context 'on project runner' do - it_behaves_like 'retrieval by unauthorized user', :active_project_runner + let(:runner) { active_project_runner } + + it_behaves_like 'retrieval by unauthorized user' end end @@ -388,13 +387,17 @@ RSpec.describe 'Query.runner(id)' do group.add_user(user, Gitlab::Access::OWNER) end - it_behaves_like 'retrieval with no admin url', :active_group_runner + it_behaves_like 'retrieval with no admin url' do + let(:runner) { active_group_runner } + end end describe 'by unauthenticated user' do let(:user) { nil } - it_behaves_like 'retrieval by unauthorized user', :active_instance_runner + it_behaves_like 'retrieval by unauthorized user' do + let(:runner) { active_instance_runner } + end end describe 'Query limits' do diff --git a/spec/requests/api/graphql/ci/runners_spec.rb b/spec/requests/api/graphql/ci/runners_spec.rb index 267dd1b5e6f..6b88c82b025 100644 --- a/spec/requests/api/graphql/ci/runners_spec.rb +++ b/spec/requests/api/graphql/ci/runners_spec.rb @@ -34,6 +34,8 @@ RSpec.describe 'Query.runners' do end before do + allow(Gitlab::Ci::RunnerUpgradeCheck.instance).to receive(:check_runner_upgrade_status) + post_graphql(query, current_user: current_user) end diff --git a/spec/requests/api/graphql/mutations/boards/create_spec.rb b/spec/requests/api/graphql/mutations/boards/create_spec.rb index 22d05f36f0f..ca848c0c92f 100644 --- a/spec/requests/api/graphql/mutations/boards/create_spec.rb +++ b/spec/requests/api/graphql/mutations/boards/create_spec.rb @@ -4,6 +4,16 @@ require 'spec_helper' RSpec.describe Mutations::Boards::Create do let_it_be(:parent) { create(:project) } + let_it_be(:current_user, reload: true) { create(:user) } + + let(:name) { 'board name' } + let(:mutation) { graphql_mutation(:create_board, params) } + + subject { post_graphql_mutation(mutation, current_user: current_user) } + + def mutation_response + graphql_mutation_response(:create_board) + end let(:project_path) { parent.full_path } let(:params) do diff --git a/spec/requests/api/graphql/mutations/ci/job_retry_spec.rb b/spec/requests/api/graphql/mutations/ci/job_retry_spec.rb index a14935379dc..ef640183bd8 100644 --- a/spec/requests/api/graphql/mutations/ci/job_retry_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/job_retry_spec.rb @@ -8,7 +8,8 @@ RSpec.describe 'JobRetry' do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project) } let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } - let_it_be(:job) { create(:ci_build, :success, pipeline: pipeline, name: 'build') } + + let(:job) { create(:ci_build, :success, pipeline: pipeline, name: 'build') } let(:mutation) do variables = { @@ -37,10 +38,23 @@ RSpec.describe 'JobRetry' do end it 'retries a job' do - job_id = ::Gitlab::GlobalId.build(job, id: job.id).to_s post_graphql_mutation(mutation, current_user: user) expect(response).to have_gitlab_http_status(:success) - expect(mutation_response['job']['id']).to eq(job_id) + new_job_id = GitlabSchema.object_from_id(mutation_response['job']['id']).sync.id + + new_job = ::Ci::Build.find(new_job_id) + expect(new_job).not_to be_retried + end + + context 'when the job is not retryable' do + let(:job) { create(:ci_build, :retried, pipeline: pipeline) } + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: user) + + expect(mutation_response['job']).to be(nil) + expect(mutation_response['errors']).to match_array(['Job cannot be retried']) + end end end diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb index a20ac823550..d9106aa42c4 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_cancel_spec.rb @@ -47,5 +47,6 @@ RSpec.describe 'PipelineCancel' do expect(response).to have_gitlab_http_status(:success) expect(build.reload).to be_canceled + expect(pipeline.reload).to be_canceled end end diff --git a/spec/requests/api/graphql/mutations/issues/update_spec.rb b/spec/requests/api/graphql/mutations/issues/update_spec.rb index 0f2eeb90894..f38deb426b1 100644 --- a/spec/requests/api/graphql/mutations/issues/update_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/update_spec.rb @@ -8,8 +8,8 @@ RSpec.describe 'Update of an existing issue' do let_it_be(:current_user) { create(:user) } let_it_be(:project) { create(:project, :public) } let_it_be(:issue) { create(:issue, project: project) } - let_it_be(:label1) { create(:label, project: project) } - let_it_be(:label2) { create(:label, project: project) } + let_it_be(:label1) { create(:label, title: "a", project: project) } + let_it_be(:label2) { create(:label, title: "b", project: project) } let(:input) do { @@ -124,7 +124,7 @@ RSpec.describe 'Update of an existing issue' do context 'add and remove labels' do let(:input_params) { input.merge(extra_params).merge({ addLabelIds: [label1.id], removeLabelIds: [label2.id] }) } - it 'returns error for mutually exclusive arguments' do + it 'returns correct labels' do post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) @@ -132,6 +132,22 @@ RSpec.describe 'Update of an existing issue' do expect(mutation_response['issue']['labels']).to include({ "nodes" => [{ "id" => label1.to_global_id.to_s }] }) end end + + context 'add labels' do + let(:input_params) { input.merge(extra_params).merge({ addLabelIds: [label1.id] }) } + + before do + issue.update!({ labels: [label2] }) + end + + it 'adds labels and keeps the title ordering' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(json_response['errors']).to be_nil + expect(mutation_response['issue']['labels']['nodes']).to eq([{ "id" => label1.to_global_id.to_s }, { "id" => label2.to_global_id.to_s }]) + end + end end end end diff --git a/spec/requests/api/graphql/mutations/merge_requests/set_labels_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/set_labels_spec.rb index 0d0cc66c52a..e40a3cf7ce9 100644 --- a/spec/requests/api/graphql/mutations/merge_requests/set_labels_spec.rb +++ b/spec/requests/api/graphql/mutations/merge_requests/set_labels_spec.rb @@ -8,8 +8,8 @@ RSpec.describe 'Setting labels of a merge request' do let(:current_user) { create(:user) } let(:merge_request) { create(:merge_request) } let(:project) { merge_request.project } - let(:label) { create(:label, project: project) } - let(:label2) { create(:label, project: project) } + let(:label) { create(:label, title: "a", project: project) } + let(:label2) { create(:label, title: "b", project: project) } let(:input) { { label_ids: [GitlabSchema.id_from_object(label).to_s] } } let(:mutation) do @@ -81,12 +81,12 @@ RSpec.describe 'Setting labels of a merge request' do merge_request.update!(labels: [label2]) end - it 'sets the labels, without removing others' do + it 'sets the labels and resets labels to keep the title ordering, without removing others' do post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) expect(mutation_label_nodes.count).to eq(2) - expect(mutation_label_nodes).to contain_exactly({ 'id' => label.to_global_id.to_s }, { 'id' => label2.to_global_id.to_s }) + expect(mutation_label_nodes).to eq([{ 'id' => label.to_global_id.to_s }, { 'id' => label2.to_global_id.to_s }]) end end diff --git a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb index 2bc671e4ca5..63b94dccca0 100644 --- a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb @@ -17,8 +17,7 @@ RSpec.describe 'Adding a Note' do noteable_id: GitlabSchema.id_from_object(noteable).to_s, discussion_id: (GitlabSchema.id_from_object(discussion).to_s if discussion), merge_request_diff_head_sha: head_sha.presence, - body: body, - confidential: true + body: body } graphql_mutation(:create_note, variables) @@ -49,7 +48,6 @@ RSpec.describe 'Adding a Note' do post_graphql_mutation(mutation, current_user: current_user) expect(mutation_response['note']['body']).to eq('Body text') - expect(mutation_response['note']['confidential']).to eq(true) end describe 'creating Notes in reply to a discussion' do @@ -79,6 +77,25 @@ RSpec.describe 'Adding a Note' do end end + context 'for an issue' do + let(:noteable) { create(:issue, project: project) } + let(:mutation) do + variables = { + noteable_id: GitlabSchema.id_from_object(noteable).to_s, + body: body, + confidential: true + } + + graphql_mutation(:create_note, variables) + end + + before do + project.add_developer(current_user) + end + + it_behaves_like 'a Note mutation with confidential notes' + end + context 'when body only contains quick actions' do let(:head_sha) { noteable.diff_head_sha } let(:body) { '/merge' } diff --git a/spec/requests/api/graphql/mutations/notes/update/note_spec.rb b/spec/requests/api/graphql/mutations/notes/update/note_spec.rb index 5a92ffe61b8..bae5c58abff 100644 --- a/spec/requests/api/graphql/mutations/notes/update/note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/update/note_spec.rb @@ -8,7 +8,7 @@ RSpec.describe 'Updating a Note' do let!(:note) { create(:note, note: original_body) } let(:original_body) { 'Initial body text' } let(:updated_body) { 'Updated body text' } - let(:params) { { body: updated_body, confidential: true } } + let(:params) { { body: updated_body } } let(:mutation) do variables = params.merge(id: GitlabSchema.id_from_object(note).to_s) @@ -28,7 +28,6 @@ RSpec.describe 'Updating a Note' do post_graphql_mutation(mutation, current_user: current_user) expect(note.reload.note).to eq(original_body) - expect(note.confidential).to be_falsey end end @@ -41,46 +40,19 @@ RSpec.describe 'Updating a Note' do post_graphql_mutation(mutation, current_user: current_user) expect(note.reload.note).to eq(updated_body) - expect(note.confidential).to be_truthy end it 'returns the updated Note' do post_graphql_mutation(mutation, current_user: current_user) expect(mutation_response['note']['body']).to eq(updated_body) - expect(mutation_response['note']['confidential']).to be_truthy - end - - context 'when only confidential param is present' do - let(:params) { { confidential: true } } - - it 'updates only the note confidentiality' do - post_graphql_mutation(mutation, current_user: current_user) - - expect(note.reload.note).to eq(original_body) - expect(note.confidential).to be_truthy - end - end - - context 'when only body param is present' do - let(:params) { { body: updated_body } } - - before do - note.update_column(:confidential, true) - end - - it 'updates only the note body' do - post_graphql_mutation(mutation, current_user: current_user) - - expect(note.reload.note).to eq(updated_body) - expect(note.confidential).to be_truthy - end end context 'when there are ActiveRecord validation errors' do - let(:updated_body) { '' } + let(:params) { { body: '', confidential: true } } - it_behaves_like 'a mutation that returns errors in the response', errors: ["Note can't be blank"] + it_behaves_like 'a mutation that returns errors in the response', + errors: ["Note can't be blank", 'Confidential can not be changed for existing notes'] it 'does not update the Note' do post_graphql_mutation(mutation, current_user: current_user) diff --git a/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb b/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb index 9ac98db91e2..c5c34e16717 100644 --- a/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb +++ b/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb @@ -50,6 +50,37 @@ RSpec.describe 'Marking all todos done' do expect(updated_todo_ids).to contain_exactly(global_id_of(todo1), global_id_of(todo3)) end + context 'when target_id is given', :aggregate_failures do + let_it_be(:target) { create(:issue, project: project) } + let_it_be(:target_todo1) { create(:todo, user: current_user, author: author, state: :pending, target: target) } + let_it_be(:target_todo2) { create(:todo, user: current_user, author: author, state: :pending, target: target) } + + let(:input) { { 'targetId' => target.to_global_id.to_s } } + + it 'marks all pending todos for the target as done' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(target_todo1.reload.state).to eq('done') + expect(target_todo2.reload.state).to eq('done') + + expect(todo1.reload.state).to eq('pending') + expect(todo3.reload.state).to eq('pending') + + updated_todo_ids = mutation_response['todos'].map { |todo| todo['id'] } + expect(updated_todo_ids).to contain_exactly(global_id_of(target_todo1), global_id_of(target_todo2)) + end + + context 'when target does not exist' do + let(:input) { { 'targetId' => "gid://gitlab/Issue/#{non_existing_record_id}" } } + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).to include(a_hash_including('message' => include('Resource not available'))) + end + end + end + it 'behaves as expected if there are no todos for the requesting user' do post_graphql_mutation(mutation, current_user: other_user2) diff --git a/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb b/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb index e1c7fd9d60d..85194e6eb20 100644 --- a/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb +++ b/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb @@ -28,6 +28,17 @@ RSpec.describe Mutations::UserPreferences::Update do expect(current_user.user_preference.persisted?).to eq(true) expect(current_user.user_preference.issues_sort).to eq(Types::IssueSortEnum.values[sort_value].value.to_s) end + + context 'when incident_escalations feature flag is disabled' do + let(:sort_value) { 'ESCALATION_STATUS_ASC' } + + before do + stub_feature_flags(incident_escalations: false) + end + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Feature flag `incident_escalations` must be enabled to use this sort order.'] + end end context 'when user has existing preference' do @@ -45,5 +56,16 @@ RSpec.describe Mutations::UserPreferences::Update do expect(current_user.user_preference.issues_sort).to eq(Types::IssueSortEnum.values[sort_value].value.to_s) end + + context 'when incident_escalations feature flag is disabled' do + let(:sort_value) { 'ESCALATION_STATUS_DESC' } + + before do + stub_feature_flags(incident_escalations: false) + end + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Feature flag `incident_escalations` must be enabled to use this sort order.'] + end end end diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb index da0c87fcefe..3bd59450d49 100644 --- a/spec/requests/api/graphql_spec.rb +++ b/spec/requests/api/graphql_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'GraphQL' do let(:expected_execute_query_log) do { "correlation_id" => kind_of(String), - "meta.caller_id" => "graphql:anonymous", + "meta.caller_id" => "graphql:unknown", "meta.client_id" => kind_of(String), "meta.feature_category" => "not_owned", "meta.remote_ip" => kind_of(String), diff --git a/spec/requests/api/group_export_spec.rb b/spec/requests/api/group_export_spec.rb index 31eef21654a..ffa313d4464 100644 --- a/spec/requests/api/group_export_spec.rb +++ b/spec/requests/api/group_export_spec.rb @@ -30,76 +30,62 @@ RSpec.describe API::GroupExport do group.add_owner(user) end - context 'group_import_export feature flag enabled' do + context 'when export file exists' do before do - stub_feature_flags(group_import_export: true) - allow(Gitlab::ApplicationRateLimiter) .to receive(:increment) .and_return(0) - end - - context 'when export file exists' do - before do - upload.export_file = fixture_file_upload('spec/fixtures/group_export.tar.gz', "`/tar.gz") - upload.save! - end - it 'downloads exported group archive' do - get api(download_path, user) - - expect(response).to have_gitlab_http_status(:ok) - end + upload.export_file = fixture_file_upload('spec/fixtures/group_export.tar.gz', "`/tar.gz") + upload.save! + end - context 'when export_file.file does not exist' do - before do - expect_next_instance_of(ImportExportUploader) do |uploader| - expect(uploader).to receive(:file).and_return(nil) - end - end + it 'downloads exported group archive' do + get api(download_path, user) - it 'returns 404' do - get api(download_path, user) + expect(response).to have_gitlab_http_status(:ok) + end - expect(response).to have_gitlab_http_status(:not_found) + context 'when export_file.file does not exist' do + before do + expect_next_instance_of(ImportExportUploader) do |uploader| + expect(uploader).to receive(:file).and_return(nil) end end - context 'when object is not present' do - let(:other_group) { create(:group, :with_export) } - let(:other_download_path) { "/groups/#{other_group.id}/export/download" } + it 'returns 404' do + get api(download_path, user) - before do - other_group.add_owner(user) - other_group.export_file.file.delete - end + expect(response).to have_gitlab_http_status(:not_found) + end + end - it 'returns 404' do - get api(other_download_path, user) + context 'when object is not present' do + let(:other_group) { create(:group, :with_export) } + let(:other_download_path) { "/groups/#{other_group.id}/export/download" } - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('The group export file is not available yet') - end + before do + other_group.add_owner(user) + other_group.export_file.file.delete end - end - context 'when export file does not exist' do it 'returns 404' do - get api(download_path, user) + get api(other_download_path, user) expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('The group export file is not available yet') end end end - context 'group_import_export feature flag disabled' do - before do - stub_feature_flags(group_import_export: false) - end - - it 'responds with 404 Not Found' do + context 'when export file does not exist' do + it 'returns 404' do get api(download_path, user) + allow(Gitlab::ApplicationRateLimiter) + .to receive(:increment) + .and_return(0) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -122,58 +108,40 @@ RSpec.describe API::GroupExport do end describe 'POST /groups/:group_id/export' do - context 'group_import_export feature flag enabled' do + context 'when user is a group owner' do before do - stub_feature_flags(group_import_export: true) + group.add_owner(user) end - context 'when user is a group owner' do - before do - group.add_owner(user) - end - - it 'accepts download' do - post api(path, user) + it 'accepts download' do + post api(path, user) - expect(response).to have_gitlab_http_status(:accepted) - end + expect(response).to have_gitlab_http_status(:accepted) end + end - context 'when the export cannot be started' do - before do - group.add_owner(user) - allow(GroupExportWorker).to receive(:perform_async).and_return(nil) - end - - it 'returns an error' do - post api(path, user) - - expect(response).to have_gitlab_http_status(:error) - end + context 'when the export cannot be started' do + before do + group.add_owner(user) + allow(GroupExportWorker).to receive(:perform_async).and_return(nil) end - context 'when user is not a group owner' do - before do - group.add_developer(user) - end - - it 'forbids the request' do - post api(path, user) + it 'returns an error' do + post api(path, user) - expect(response).to have_gitlab_http_status(:forbidden) - end + expect(response).to have_gitlab_http_status(:error) end end - context 'group_import_export feature flag disabled' do + context 'when user is not a group owner' do before do - stub_feature_flags(group_import_export: false) + group.add_developer(user) end - it 'responds with 404 Not Found' do + it 'forbids the request' do post api(path, user) - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:forbidden) end end @@ -202,7 +170,6 @@ RSpec.describe API::GroupExport do let(:status_path) { "/groups/#{group.id}/export_relations/status" } before do - stub_feature_flags(group_import_export: true) group.add_owner(user) end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 7de3567dcdd..ffc5d353958 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -1164,17 +1164,47 @@ RSpec.describe API::Groups do end context 'when include_subgroups is true' do - it "returns projects including those in subgroups" do + before do subgroup = create(:group, parent: group1) + subgroup2 = create(:group, parent: subgroup) + create(:project, group: subgroup) create(:project, group: subgroup) + create(:project, group: subgroup2) + + group1.reload + end + + it "only looks up root ancestor once and returns projects including those in subgroups" do + expect(Namespace).to receive(:find_by).with(id: group1.id.to_s).once.and_call_original # For the group sent in the API call + expect(Namespace).to receive(:find_by).with(id: group1.traversal_ids.first).once.and_call_original # root_ancestor direct lookup + expect(Namespace).to receive(:joins).with(start_with('INNER JOIN (SELECT id, traversal_ids[1]')).once.and_call_original # All-in-one root_ancestor query get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an(Array) - expect(json_response.length).to eq(5) + expect(json_response.length).to eq(6) + end + + context 'when group_projects_api_preload_groups feature is disabled' do + before do + stub_feature_flags(group_projects_api_preload_groups: false) + end + + it 'looks up the root ancestor multiple times' do + expect(Namespace).to receive(:find_by).with(id: group1.id.to_s).once.and_call_original + expect(Namespace).to receive(:find_by).with(id: group1.traversal_ids.first).at_least(:twice).and_call_original + expect(Namespace).not_to receive(:joins).with(start_with('INNER JOIN (SELECT id, traversal_ids[1]')) + + get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(6) + end end end diff --git a/spec/requests/api/integrations_spec.rb b/spec/requests/api/integrations_spec.rb index 220c58afbe9..96cc101e73a 100644 --- a/spec/requests/api/integrations_spec.rb +++ b/spec/requests/api/integrations_spec.rb @@ -55,7 +55,7 @@ RSpec.describe API::Integrations do describe "PUT /projects/:id/#{endpoint}/#{integration.dasherize}" do include_context integration - it "updates #{integration} settings" do + it "updates #{integration} settings and returns the correct fields" do put api("/projects/#{project.id}/#{endpoint}/#{dashed_integration}", user), params: integration_attrs expect(response).to have_gitlab_http_status(:ok) @@ -80,6 +80,8 @@ RSpec.describe API::Integrations do expect(project.integrations.first[event]).not_to eq(current_integration[event]), "expected #{!current_integration[event]} for event #{event} for #{endpoint} #{current_integration.title}, got #{current_integration[event]}" end + + assert_correct_response_fields(json_response['properties'].keys, current_integration) end it "returns if required fields missing" do @@ -142,22 +144,24 @@ RSpec.describe API::Integrations do expect(response).to have_gitlab_http_status(:unauthorized) end - it "returns all properties of active integration #{integration}" do + it "returns all properties of active integration #{integration}, except password fields" do get api("/projects/#{project.id}/#{endpoint}/#{dashed_integration}", user) expect(initialized_integration).to be_active expect(response).to have_gitlab_http_status(:ok) - expect(json_response['properties'].keys).to match_array(integration_instance.api_field_names) + + assert_correct_response_fields(json_response['properties'].keys, integration_instance) end - it "returns all properties of inactive integration #{integration}" do + it "returns all properties of inactive integration #{integration}, except password fields" do deactive_integration! get api("/projects/#{project.id}/#{endpoint}/#{dashed_integration}", user) expect(initialized_integration).not_to be_active expect(response).to have_gitlab_http_status(:ok) - expect(json_response['properties'].keys).to match_array(integration_instance.api_field_names) + + assert_correct_response_fields(json_response['properties'].keys, integration_instance) end it "returns not found if integration does not exist" do @@ -369,5 +373,20 @@ RSpec.describe API::Integrations do end end end + + private + + def assert_correct_response_fields(response_keys, integration) + assert_fields_match_integration(response_keys, integration) + assert_secret_fields_filtered(response_keys, integration) + end + + def assert_fields_match_integration(response_keys, integration) + expect(response_keys).to match_array(integration.api_field_names) + end + + def assert_secret_fields_filtered(response_keys, integration) + expect(response_keys).not_to include(*integration.secret_fields) + end end end diff --git a/spec/requests/api/internal/container_registry/migration_spec.rb b/spec/requests/api/internal/container_registry/migration_spec.rb index 27e99a21c65..35113c66f11 100644 --- a/spec/requests/api/internal/container_registry/migration_spec.rb +++ b/spec/requests/api/internal/container_registry/migration_spec.rb @@ -67,12 +67,17 @@ RSpec.describe API::Internal::ContainerRegistry::Migration do it_behaves_like 'returning an error', with_message: "Couldn't transition from pre_importing to importing" end - end - context 'with repository in importing migration state' do - let(:repository) { create(:container_repository, :importing) } + context 'with repository in importing migration state' do + let(:repository) { create(:container_repository, :importing) } + + it 'returns ok and does not update the migration state' do + expect { subject } + .not_to change { repository.reload.migration_state } - it_behaves_like 'returning an error', with_message: "Couldn't transition from pre_importing to importing" + expect(response).to have_gitlab_http_status(:ok) + end + end end end @@ -101,7 +106,7 @@ RSpec.describe API::Internal::ContainerRegistry::Migration do context 'with repository in pre_importing migration state' do let(:repository) { create(:container_repository, :pre_importing) } - it_behaves_like 'returning an error', with_message: "Couldn't transition from importing to import_done" + it_behaves_like 'updating the repository migration status', from: 'pre_importing', to: 'import_done' end end diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb index 741cf793a77..d093894720e 100644 --- a/spec/requests/api/invitations_spec.rb +++ b/spec/requests/api/invitations_spec.rb @@ -69,7 +69,7 @@ RSpec.describe API::Invitations do end end - it 'invites a new member' do + it 'adds a new member by email' do expect do post invitations_url(source, maintainer), params: { email: email, access_level: Member::DEVELOPER } @@ -78,6 +78,24 @@ RSpec.describe API::Invitations do end.to change { source.members.invite.count }.by(1) end + it 'adds a new member by user_id' do + expect do + post invitations_url(source, maintainer), + params: { user_id: stranger.id, access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.non_invite.count }.by(1) + end + + it 'adds new members with email and user_id' do + expect do + post invitations_url(source, maintainer), + params: { email: email, user_id: stranger.id, access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.invite.count }.by(1).and change { source.members.non_invite.count }.by(1) + end + it 'invites a list of new email addresses' do expect do email_list = [email, email2].join(',') @@ -88,6 +106,19 @@ RSpec.describe API::Invitations do expect(response).to have_gitlab_http_status(:created) end.to change { source.members.invite.count }.by(2) end + + it 'invites a list of new email addresses and user ids' do + expect do + stranger2 = create(:user) + email_list = [email, email2].join(',') + user_id_list = "#{stranger.id},#{stranger2.id}" + + post invitations_url(source, maintainer), + params: { email: email_list, user_id: user_id_list, access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.invite.count }.by(2).and change { source.members.non_invite.count }.by(2) + end end context 'access levels' do @@ -235,27 +266,36 @@ RSpec.describe API::Invitations do expect(json_response['message'][developer.email]).to eq("User already exists in source") end - it 'returns 404 when the email is not valid' do + it 'returns 400 when the invite params of email and user_id are not sent' do + post invitations_url(source, maintainer), + params: { access_level: Member::MAINTAINER } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('400 Bad request - Must provide either email or user_id as a parameter') + end + + it 'returns 400 when the email is blank' do post invitations_url(source, maintainer), params: { email: '', access_level: Member::MAINTAINER } - expect(response).to have_gitlab_http_status(:created) - expect(json_response['message']).to eq('Emails cannot be blank') + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('400 Bad request - Must provide either email or user_id as a parameter') end - it 'returns 404 when the email list is not a valid format' do + it 'returns 400 when the user_id is blank' do post invitations_url(source, maintainer), - params: { email: 'email1@example.com,not-an-email', access_level: Member::MAINTAINER } + params: { user_id: '', access_level: Member::MAINTAINER } expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('email contains an invalid email address') + expect(json_response['message']).to eq('400 Bad request - Must provide either email or user_id as a parameter') end - it 'returns 400 when email is not given' do + it 'returns 400 when the email list is not a valid format' do post invitations_url(source, maintainer), - params: { access_level: Member::MAINTAINER } + params: { email: 'email1@example.com,not-an-email', access_level: Member::MAINTAINER } expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('email contains an invalid email address') end it 'returns 400 when access_level is not given' do @@ -278,12 +318,90 @@ RSpec.describe API::Invitations do it_behaves_like 'POST /:source_type/:id/invitations', 'project' do let(:source) { project } end + + it 'records queries', :request_store, :use_sql_query_cache do + post invitations_url(project, maintainer), params: { email: email, access_level: Member::DEVELOPER } + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + post invitations_url(project, maintainer), params: { email: email2, access_level: Member::DEVELOPER } + end + + emails = 'email3@example.com,email4@example.com,email5@example.com,email6@example.com,email7@example.com' + + unresolved_n_plus_ones = 44 # old 48 with 12 per new email, currently there are 11 queries added per email + + expect do + post invitations_url(project, maintainer), params: { email: emails, access_level: Member::DEVELOPER } + end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) + end + + it 'records queries with secondary emails', :request_store, :use_sql_query_cache do + create(:email, email: email, user: create(:user)) + + post invitations_url(project, maintainer), params: { email: email, access_level: Member::DEVELOPER } + + create(:email, email: email2, user: create(:user)) + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + post invitations_url(project, maintainer), params: { email: email2, access_level: Member::DEVELOPER } + end + + create(:email, email: 'email4@example.com', user: create(:user)) + create(:email, email: 'email6@example.com', user: create(:user)) + + emails = 'email3@example.com,email4@example.com,email5@example.com,email6@example.com,email7@example.com' + + unresolved_n_plus_ones = 67 # currently there are 11 queries added per email + + expect do + post invitations_url(project, maintainer), params: { email: emails, access_level: Member::DEVELOPER } + end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) + end end describe 'POST /groups/:id/invitations' do it_behaves_like 'POST /:source_type/:id/invitations', 'group' do let(:source) { group } end + + it 'records queries', :request_store, :use_sql_query_cache do + post invitations_url(group, maintainer), params: { email: email, access_level: Member::DEVELOPER } + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + post invitations_url(group, maintainer), params: { email: email2, access_level: Member::DEVELOPER } + end + + emails = 'email3@example.com,email4@example.com,email5@example.com,email6@example.com,email7@example.com' + + unresolved_n_plus_ones = 36 # old 40 with 10 per new email, currently there are 9 queries added per email + + expect do + post invitations_url(group, maintainer), params: { email: emails, access_level: Member::DEVELOPER } + end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) + end + + it 'records queries with secondary emails', :request_store, :use_sql_query_cache do + create(:email, email: email, user: create(:user)) + + post invitations_url(group, maintainer), params: { email: email, access_level: Member::DEVELOPER } + + create(:email, email: email2, user: create(:user)) + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + post invitations_url(group, maintainer), params: { email: email2, access_level: Member::DEVELOPER } + end + + create(:email, email: 'email4@example.com', user: create(:user)) + create(:email, email: 'email6@example.com', user: create(:user)) + + emails = 'email3@example.com,email4@example.com,email5@example.com,email6@example.com,email7@example.com' + + unresolved_n_plus_ones = 62 # currently there are 9 queries added per email + + expect do + post invitations_url(group, maintainer), params: { email: emails, access_level: Member::DEVELOPER } + end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) + end end shared_examples 'GET /:source_type/:id/invitations' do |source_type| @@ -315,23 +433,6 @@ RSpec.describe API::Invitations do end end - it 'avoids N+1 queries' do - invite_member_by_email(source, source_type, email, maintainer) - - # Establish baseline - get invitations_url(source, maintainer) - - control = ActiveRecord::QueryRecorder.new do - get invitations_url(source, maintainer) - end - - invite_member_by_email(source, source_type, email2, maintainer) - - expect do - get invitations_url(source, maintainer) - end.not_to exceed_query_limit(control) - end - it 'does not find confirmed members' do get invitations_url(source, maintainer) diff --git a/spec/requests/api/issue_links_spec.rb b/spec/requests/api/issue_links_spec.rb index 45583f5c7dc..81dd4c3dfa0 100644 --- a/spec/requests/api/issue_links_spec.rb +++ b/spec/requests/api/issue_links_spec.rb @@ -34,7 +34,7 @@ RSpec.describe API::IssueLinks do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Array expect(json_response.length).to eq(2) - expect(response).to match_response_schema('public_api/v4/issue_links') + expect(response).to match_response_schema('public_api/v4/related_issues') end it 'returns multiple links without N + 1' do @@ -205,16 +205,30 @@ RSpec.describe API::IssueLinks do end context 'when user has ability to delete the issue link' do + let_it_be(:target_issue) { create(:issue, project: project) } + + before do + project.add_reporter(user) + end + it 'returns 200' do - target_issue = create(:issue, project: project) issue_link = create(:issue_link, source: issue, target: target_issue) - project.add_reporter(user) delete api("/projects/#{project.id}/issues/#{issue.iid}/links/#{issue_link.id}", user) expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('public_api/v4/issue_link') end + + it 'returns 404 when the issue link does not belong to the specified issue' do + other_issue = create(:issue, project: project) + issue_link = create(:issue_link, source: other_issue, target: target_issue) + + delete api("/projects/#{project.id}/issues/#{issue.iid}/links/#{issue_link.id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Not found') + end end end end diff --git a/spec/requests/api/issues/get_project_issues_spec.rb b/spec/requests/api/issues/get_project_issues_spec.rb index 9948e13e9ae..346f8975835 100644 --- a/spec/requests/api/issues/get_project_issues_spec.rb +++ b/spec/requests/api/issues/get_project_issues_spec.rb @@ -877,5 +877,35 @@ RSpec.describe API::Issues do expect(response).to have_gitlab_http_status(:not_found) end + + context 'with a confidential note' do + let!(:note) do + create( + :note, + :confidential, + project: project, + noteable: issue, + author: create(:user) + ) + end + + it 'returns a full list of participants' do + get api("/projects/#{project.id}/issues/#{issue.iid}/participants", user) + + expect(response).to have_gitlab_http_status(:ok) + participant_ids = json_response.map { |el| el['id'] } + expect(participant_ids).to match_array([issue.author_id, note.author_id]) + end + + context 'when user cannot see a confidential note' do + it 'returns a limited list of participants' do + get api("/projects/#{project.id}/issues/#{issue.iid}/participants", create(:user)) + + expect(response).to have_gitlab_http_status(:ok) + participant_ids = json_response.map { |el| el['id'] } + expect(participant_ids).to match_array([issue.author_id]) + end + end + end end end diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index c5e57b5b18b..1419d39981a 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -554,6 +554,27 @@ RSpec.describe API::Issues do end end + context 'with incident issues' do + let_it_be(:incident) { create(:incident, project: project) } + + it 'avoids N+1 queries' do + get api('/issues', user) # warm up + + control = ActiveRecord::QueryRecorder.new do + get api('/issues', user) + end + + create(:incident, project: project) + create(:incident, project: project) + + expect do + get api('/issues', user) + end.not_to exceed_query_limit(control) + # 2 pre-existed issues + 3 incidents + expect(json_response.count).to eq(5) + end + end + context 'filter by labels or label_name param' do context 'N+1' do let(:label_b) { create(:label, title: 'foo', project: project) } diff --git a/spec/requests/api/keys_spec.rb b/spec/requests/api/keys_spec.rb index 49b8f4a8520..67c3de324dc 100644 --- a/spec/requests/api/keys_spec.rb +++ b/spec/requests/api/keys_spec.rb @@ -3,10 +3,11 @@ require 'spec_helper' RSpec.describe API::Keys do - let(:user) { create(:user) } - let(:admin) { create(:admin) } - let(:key) { create(:key, user: user, expires_at: 1.day.from_now) } - let(:email) { create(:email, user: user) } + let_it_be(:user) { create(:user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:email) { create(:email, user: user) } + let_it_be(:key) { create(:rsa_key_4096, user: user, expires_at: 1.day.from_now) } + let_it_be(:fingerprint_md5) { 'df:73:db:29:3c:a5:32:cf:09:17:7e:8e:9d:de:d7:f7' } describe 'GET /keys/:uid' do context 'when unauthenticated' do @@ -24,7 +25,6 @@ RSpec.describe API::Keys do end it 'returns single ssh key with user information' do - user.keys << key get api("/keys/#{key.id}", admin) expect(response).to have_gitlab_http_status(:ok) expect(json_response['title']).to eq(key.title) @@ -43,23 +43,50 @@ RSpec.describe API::Keys do describe 'GET /keys?fingerprint=' do it 'returns authentication error' do - get api("/keys?fingerprint=#{key.fingerprint}") + get api("/keys?fingerprint=#{fingerprint_md5}") expect(response).to have_gitlab_http_status(:unauthorized) end it 'returns authentication error when authenticated as user' do - get api("/keys?fingerprint=#{key.fingerprint}", user) + get api("/keys?fingerprint=#{fingerprint_md5}", user) expect(response).to have_gitlab_http_status(:forbidden) end context 'when authenticated as admin' do - it 'returns 404 for non-existing SSH md5 fingerprint' do - get api("/keys?fingerprint=11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11", admin) + context 'MD5 fingerprint' do + it 'returns 404 for non-existing SSH md5 fingerprint' do + get api("/keys?fingerprint=11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11", admin) - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 Key Not Found') + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Key Not Found') + end + + it 'returns user if SSH md5 fingerprint found' do + get api("/keys?fingerprint=#{fingerprint_md5}", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['title']).to eq(key.title) + expect(json_response['user']['id']).to eq(user.id) + expect(json_response['user']['username']).to eq(user.username) + end + + context 'with FIPS mode', :fips_mode do + it 'returns 404 for non-existing SSH md5 fingerprint' do + get api("/keys?fingerprint=11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11", admin) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('Failed to return the key') + end + + it 'returns 404 for existing SSH md5 fingerprint' do + get api("/keys?fingerprint=#{fingerprint_md5}", admin) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('Failed to return the key') + end + end end it 'returns 404 for non-existing SSH sha256 fingerprint' do @@ -69,20 +96,7 @@ RSpec.describe API::Keys do expect(json_response['message']).to eq('404 Key Not Found') end - it 'returns user if SSH md5 fingerprint found' do - user.keys << key - - get api("/keys?fingerprint=#{key.fingerprint}", admin) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['title']).to eq(key.title) - expect(json_response['user']['id']).to eq(user.id) - expect(json_response['user']['username']).to eq(user.username) - end - it 'returns user if SSH sha256 fingerprint found' do - user.keys << key - get api("/keys?fingerprint=#{URI.encode_www_form_component("SHA256:" + key.fingerprint_sha256)}", admin) expect(response).to have_gitlab_http_status(:ok) @@ -92,8 +106,6 @@ RSpec.describe API::Keys do end it 'returns user if SSH sha256 fingerprint found' do - user.keys << key - get api("/keys?fingerprint=#{URI.encode_www_form_component("sha256:" + key.fingerprint_sha256)}", admin) expect(response).to have_gitlab_http_status(:ok) @@ -103,7 +115,7 @@ RSpec.describe API::Keys do end it "does not include the user's `is_admin` flag" do - get api("/keys?fingerprint=#{key.fingerprint}", admin) + get api("/keys?fingerprint=#{URI.encode_www_form_component("sha256:" + key.fingerprint_sha256)}", admin) expect(json_response['user']['is_admin']).to be_nil end diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb index 73bc4a5d1f3..ef7f5ee87dc 100644 --- a/spec/requests/api/lint_spec.rb +++ b/spec/requests/api/lint_spec.rb @@ -195,7 +195,7 @@ RSpec.describe API::Lint do end context 'with invalid configuration' do - let(:yaml_content) { '{ image: "ruby:2.7", services: ["postgres"] }' } + let(:yaml_content) { '{ image: "image:1.0", services: ["postgres"] }' } it 'responds with errors about invalid configuration' do post api('/ci/lint', api_user), params: { content: yaml_content } @@ -465,7 +465,7 @@ RSpec.describe API::Lint do context 'with invalid .gitlab-ci.yml content' do let(:yaml_content) do - { image: 'ruby:2.7', services: ['postgres'] }.deep_stringify_keys.to_yaml + { image: 'image:1.0', services: ['postgres'] }.deep_stringify_keys.to_yaml end before do @@ -712,7 +712,7 @@ RSpec.describe API::Lint do context 'with invalid .gitlab-ci.yml content' do let(:yaml_content) do - { image: 'ruby:2.7', services: ['postgres'] }.deep_stringify_keys.to_yaml + { image: 'image:1.0', services: ['postgres'] }.deep_stringify_keys.to_yaml end context 'when running as dry run' do diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 561d81f9860..6bacb3a59b2 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -11,16 +11,16 @@ RSpec.describe API::Members do let(:project) do create(:project, :public, creator_id: maintainer.id, namespace: maintainer.namespace) do |project| - project.add_developer(developer) project.add_maintainer(maintainer) + project.add_developer(developer, current_user: maintainer) project.request_access(access_requester) end end let!(:group) do create(:group, :public) do |group| - group.add_developer(developer) group.add_owner(maintainer) + group.add_developer(developer, maintainer) create(:group_member, :minimal_access, source: group, user: user_with_minimal_access) group.request_access(access_requester) end @@ -50,6 +50,10 @@ RSpec.describe API::Members do expect(json_response).to be_an Array expect(json_response.size).to eq(2) expect(json_response.map { |u| u['id'] }).to match_array [maintainer.id, developer.id] + expect(json_response).to contain_exactly( + a_hash_including('created_by' => a_hash_including('id' => maintainer.id)), + hash_not_including('created_by') + ) end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 9e6fea9e5b4..b1183bb10fa 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1605,11 +1605,7 @@ RSpec.describe API::MergeRequests do expect(json_response['overflow']).to be_falsy end - context 'when using DB-backed diffs via feature flag' do - before do - stub_feature_flags(mrc_api_use_raw_diffs_from_gitaly: false) - end - + context 'when using DB-backed diffs' do it_behaves_like 'find an existing merge request' it 'accesses diffs via DB-backed diffs.diffs' do diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index 455400072bf..f6a65274ca2 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -22,7 +22,7 @@ RSpec.describe API::Notes do let!(:issue) { create(:issue, project: project, author: user) } let!(:issue_note) { create(:note, noteable: issue, project: project, author: user) } - it_behaves_like "noteable API", 'projects', 'issues', 'iid' do + it_behaves_like "noteable API with confidential notes", 'projects', 'issues', 'iid' do let(:parent) { project } let(:noteable) { issue } let(:note) { issue_note } diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index 02d377efd95..fbcaa404edb 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -11,8 +11,6 @@ itself: # project - has_external_wiki - hidden - import_source - - import_type - - import_url - jobs_cache_index - last_repository_check_at - last_repository_check_failed @@ -63,6 +61,8 @@ itself: # project - empty_repo - forks_count - http_url_to_repo + - import_status + - import_url - name_with_namespace - open_issues_count - owner @@ -148,6 +148,7 @@ project_setting: - updated_at - cve_id_request_enabled - mr_default_target_self + - target_platforms build_service_desk_setting: # service_desk_setting unexposed_attributes: diff --git a/spec/requests/api/project_export_spec.rb b/spec/requests/api/project_export_spec.rb index 2bc31153f2c..07efd56fef4 100644 --- a/spec/requests/api/project_export_spec.rb +++ b/spec/requests/api/project_export_spec.rb @@ -260,6 +260,29 @@ RSpec.describe API::ProjectExport, :clean_gitlab_redis_cache do expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.') end end + + context 'applies correct scope when throttling' do + before do + stub_application_setting(project_download_export_limit: 1) + end + + it 'throttles downloads within same namespaces' do + # simulate prior request to the same namespace, which increments the rate limit counter for that scope + Gitlab::ApplicationRateLimiter.throttled?(:project_download_export, scope: [user, project_finished.namespace]) + + get api(download_path_finished, user) + expect(response).to have_gitlab_http_status(:too_many_requests) + end + + it 'allows downloads from different namespaces' do + # simulate prior request to a different namespace, which increments the rate limit counter for that scope + Gitlab::ApplicationRateLimiter.throttled?(:project_download_export, + scope: [user, create(:project, :with_export).namespace]) + + get api(download_path_finished, user) + expect(response).to have_gitlab_http_status(:ok) + end + end end context 'when user is a maintainer' do diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index a0f6d3d0081..7e6d80c047c 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -13,7 +13,7 @@ RSpec.describe API::ProjectImport, :aggregate_failures do let(:namespace) { create(:group) } before do - namespace.add_owner(user) + namespace.add_owner(user) if user end shared_examples 'requires authentication' do @@ -47,7 +47,7 @@ RSpec.describe API::ProjectImport, :aggregate_failures do it 'executes a limited number of queries' do control_count = ActiveRecord::QueryRecorder.new { subject }.count - expect(control_count).to be <= 105 + expect(control_count).to be <= 108 end it 'schedules an import using a namespace' do @@ -306,63 +306,49 @@ RSpec.describe API::ProjectImport, :aggregate_failures do it_behaves_like 'requires authentication' - it 'returns NOT FOUND when the feature is disabled' do - stub_feature_flags(import_project_from_remote_file: false) - - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - - context 'when the feature flag is enabled' do - before do - stub_feature_flags(import_project_from_remote_file: true) - end - - context 'when the response is successful' do - it 'schedules the import successfully' do - project = create( - :project, - namespace: user.namespace, - name: 'test-import', - path: 'test-import' - ) + context 'when the response is successful' do + it 'schedules the import successfully' do + project = create( + :project, + namespace: user.namespace, + name: 'test-import', + path: 'test-import' + ) - service_response = ServiceResponse.success(payload: project) - expect_next(::Import::GitlabProjects::CreateProjectService) - .to receive(:execute) - .and_return(service_response) + service_response = ServiceResponse.success(payload: project) + expect_next(::Import::GitlabProjects::CreateProjectService) + .to receive(:execute) + .and_return(service_response) - subject + subject - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to include({ - 'id' => project.id, - 'name' => 'test-import', - 'name_with_namespace' => "#{user.namespace.name} / test-import", - 'path' => 'test-import', - 'path_with_namespace' => "#{user.namespace.path}/test-import" - }) - end + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to include({ + 'id' => project.id, + 'name' => 'test-import', + 'name_with_namespace' => "#{user.namespace.name} / test-import", + 'path' => 'test-import', + 'path_with_namespace' => "#{user.namespace.path}/test-import" + }) end + end - context 'when the service returns an error' do - it 'fails to schedule the import' do - service_response = ServiceResponse.error( - message: 'Failed to import', - http_status: :bad_request - ) - expect_next(::Import::GitlabProjects::CreateProjectService) - .to receive(:execute) - .and_return(service_response) + context 'when the service returns an error' do + it 'fails to schedule the import' do + service_response = ServiceResponse.error( + message: 'Failed to import', + http_status: :bad_request + ) + expect_next(::Import::GitlabProjects::CreateProjectService) + .to receive(:execute) + .and_return(service_response) - subject + subject - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response).to eq({ - 'message' => 'Failed to import' - }) - end + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to eq({ + 'message' => 'Failed to import' + }) end end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index fc1d815a64e..011300a038f 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -683,6 +683,33 @@ RSpec.describe API::Projects do end end + context 'and imported=true' do + before do + other_user = create(:user) + # imported project by other user + create(:project, creator: other_user, import_type: 'github', import_url: 'http://foo.com') + # project created by current user directly instead of importing + create(:project) + project.update_attribute(:import_url, 'http://user:password@host/path') + project.update_attribute(:import_type, 'github') + end + + it 'returns only imported projects owned by current user' do + get api('/projects?imported=true', user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |p| p['id'] }).to eq [project.id] + end + + it 'does not expose import credentials' do + get api('/projects?imported=true', user) + + expect(json_response.first['import_url']).to eq 'http://host/path' + end + end + context 'when authenticated as a different user' do it_behaves_like 'projects response' do let(:filter) { {} } @@ -777,7 +804,7 @@ RSpec.describe API::Projects do subject { get api('/projects', current_user), params: params } before do - group_with_projects.add_owner(current_user) + group_with_projects.add_owner(current_user) if current_user end it 'returns non-public items based ordered by similarity' do @@ -3116,6 +3143,29 @@ RSpec.describe API::Projects do project2.add_developer(project2_user) end + it 'records the query', :request_store, :use_sql_query_cache do + post api("/projects/#{project.id}/import_project_members/#{project2.id}", user) + + control_project = create(:project) + control_project.add_maintainer(user) + control_project.add_developer(create(:user)) + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + post api("/projects/#{project.id}/import_project_members/#{control_project.id}", user) + end + + measure_project = create(:project) + measure_project.add_maintainer(user) + measure_project.add_developer(create(:user)) + measure_project.add_developer(create(:user)) # make this 2nd one to find any n+1 + + unresolved_n_plus_ones = 21 # 21 queries added per member + + expect do + post api("/projects/#{project.id}/import_project_members/#{measure_project.id}", user) + end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) + end + it 'returns 200 when it successfully imports members from another project' do expect do post api("/projects/#{project.id}/import_project_members/#{project2.id}", user) diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 6038682de1e..c6bf72176a8 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -1358,4 +1358,95 @@ RSpec.describe API::Releases do release_cli: release_cli ) end + + describe 'GET /groups/:id/releases' do + let_it_be(:user1) { create(:user, can_create_group: false) } + let_it_be(:admin) { create(:admin) } + let_it_be(:group1) { create(:group) } + let_it_be(:group2) { create(:group, :private) } + let_it_be(:project1) { create(:project, namespace: group1) } + let_it_be(:project2) { create(:project, namespace: group2) } + let_it_be(:project3) { create(:project, namespace: group1, path: 'test', visibility_level: Gitlab::VisibilityLevel::PRIVATE) } + let_it_be(:release1) { create(:release, project: project1) } + let_it_be(:release2) { create(:release, project: project2) } + let_it_be(:release3) { create(:release, project: project3) } + + context 'when authenticated as owner' do + it 'gets releases from all projects in the group' do + get api("/groups/#{group1.id}/releases", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.length).to eq(2) + expect(json_response.pluck('name')).to match_array([release1.name, release3.name]) + end + + it 'respects order by parameters' do + create(:release, project: project1, released_at: DateTime.now + 1.day) + get api("/groups/#{group1.id}/releases", admin), params: { sort: 'desc' } + + expect(DateTime.parse(json_response[0]["released_at"])) + .to be > (DateTime.parse(json_response[1]["released_at"])) + end + + it 'respects the simple parameter' do + get api("/groups/#{group1.id}/releases", admin), params: { simple: true } + + expect(json_response[0].keys).not_to include("assets") + end + + it 'denies access to private groups' do + get api("/groups/#{group2.id}/releases", user1), params: { simple: true } + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns not found unless :group_releases_finder_inoperator feature flag enabled' do + stub_feature_flags(group_releases_finder_inoperator: false) + + get api("/groups/#{group1.id}/releases", admin) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when authenticated as guest' do + before do + group1.add_guest(guest) + end + + it "does not expose tag, commit, source code or helper paths" do + get api("/groups/#{group1.id}/releases", guest) + + expect(response).to match_response_schema('public_api/v4/release/releases_for_guest') + expect(json_response[0]['assets']['count']).to eq(release1.links.count) + expect(json_response[0]['commit_path']).to be_nil + expect(json_response[0]['tag_path']).to be_nil + end + end + + context 'performance testing' do + shared_examples 'avoids N+1 queries' do |query_params = {}| + context 'with subgroups' do + let(:group) { create(:group) } + + it 'include_subgroups avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get api("/groups/#{group.id}/releases", admin), params: query_params.merge({ include_subgroups: true }) + end.count + + subgroups = create_list(:group, 10, parent: group1) + projects = create_list(:project, 10, namespace: subgroups[0]) + create_list(:release, 10, project: projects[0], author: admin) + + expect do + get api("/groups/#{group.id}/releases", admin), params: query_params.merge({ include_subgroups: true }) + end.not_to exceed_all_query_limit(control_count) + end + end + end + + it_behaves_like 'avoids N+1 queries' + it_behaves_like 'avoids N+1 queries', { simple: true } + end + end end diff --git a/spec/requests/api/remote_mirrors_spec.rb b/spec/requests/api/remote_mirrors_spec.rb index 436efb708fd..338647224e0 100644 --- a/spec/requests/api/remote_mirrors_spec.rb +++ b/spec/requests/api/remote_mirrors_spec.rb @@ -26,6 +26,26 @@ RSpec.describe API::RemoteMirrors do end end + describe 'GET /projects/:id/remote_mirrors/:mirror_id' do + let(:route) { "/projects/#{project.id}/remote_mirrors/#{mirror.id}" } + let(:mirror) { project.remote_mirrors.first } + + it 'requires `admin_remote_mirror` permission' do + get api(route, developer) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + + it 'returns at remote mirror' do + project.add_maintainer(user) + + get api(route, user) + + expect(response).to have_gitlab_http_status(:success) + expect(response).to match_response_schema('remote_mirror') + end + end + describe 'POST /projects/:id/remote_mirrors' do let(:route) { "/projects/#{project.id}/remote_mirrors" } @@ -75,11 +95,11 @@ RSpec.describe API::RemoteMirrors do end describe 'PUT /projects/:id/remote_mirrors/:mirror_id' do - let(:route) { ->(id) { "/projects/#{project.id}/remote_mirrors/#{id}" } } + let(:route) { "/projects/#{project.id}/remote_mirrors/#{mirror.id}" } let(:mirror) { project.remote_mirrors.first } it 'requires `admin_remote_mirror` permission' do - put api(route[mirror.id], developer) + put api(route, developer) expect(response).to have_gitlab_http_status(:unauthorized) end @@ -87,7 +107,7 @@ RSpec.describe API::RemoteMirrors do it 'updates a remote mirror' do project.add_maintainer(user) - put api(route[mirror.id], user), params: { + put api(route, user), params: { enabled: '0', only_protected_branches: 'true', keep_divergent_refs: 'true' @@ -99,4 +119,44 @@ RSpec.describe API::RemoteMirrors do expect(json_response['keep_divergent_refs']).to eq(true) end end + + describe 'DELETE /projects/:id/remote_mirrors/:mirror_id' do + let(:route) { ->(id) { "/projects/#{project.id}/remote_mirrors/#{id}" } } + let(:mirror) { project.remote_mirrors.first } + + it 'requires `admin_remote_mirror` permission' do + expect { delete api(route[mirror.id], developer) }.not_to change { project.remote_mirrors.count } + + expect(response).to have_gitlab_http_status(:unauthorized) + end + + context 'when the user is a maintainer' do + before do + project.add_maintainer(user) + end + + it 'returns 404 for non existing id' do + delete api(route[non_existing_record_id], user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns bad request if the update service fails' do + expect_next_instance_of(Projects::UpdateService) do |service| + expect(service).to receive(:execute).and_return(status: :error, message: 'message') + end + + expect { delete api(route[mirror.id], user) }.not_to change { project.remote_mirrors.count } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to eq({ 'message' => 'message' }) + end + + it 'deletes a remote mirror' do + expect { delete api(route[mirror.id], user) }.to change { project.remote_mirrors.count }.from(1).to(0) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + end end diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 1d199a72d1d..d6d2bd5baf2 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -452,7 +452,7 @@ RSpec.describe API::Repositories do it "compare commits between different projects" do group = create(:group) - group.add_owner(current_user) + group.add_owner(current_user) if current_user forked_project = fork_project(project, current_user, repository: true, namespace: group) forked_project.repository.create_ref('refs/heads/improve/awesome', 'refs/heads/improve/more-awesome') diff --git a/spec/requests/api/resource_access_tokens_spec.rb b/spec/requests/api/resource_access_tokens_spec.rb index 7e3e682767f..369a8c1b0ab 100644 --- a/spec/requests/api/resource_access_tokens_spec.rb +++ b/spec/requests/api/resource_access_tokens_spec.rb @@ -29,6 +29,8 @@ RSpec.describe API::ResourceAccessTokens do token_ids = json_response.map { |token| token['id'] } expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(response).to match_response_schema('public_api/v4/resource_access_tokens') expect(token_ids).to match_array(access_tokens.pluck(:id)) end @@ -131,6 +133,103 @@ RSpec.describe API::ResourceAccessTokens do end end + context "GET #{source_type}s/:id/access_tokens/:token_id" do + subject(:get_token) { get api("/#{source_type}s/#{resource_id}/access_tokens/#{token_id}", user) } + + let_it_be(:project_bot) { create(:user, :project_bot) } + let_it_be(:token) { create(:personal_access_token, user: project_bot) } + let_it_be(:resource_id) { resource.id } + let_it_be(:token_id) { token.id } + + before do + if source_type == 'project' + resource.add_maintainer(project_bot) + else + resource.add_owner(project_bot) + end + end + + context "when the user has valid permissions" do + it "gets the #{source_type} access token from the #{source_type}" do + get_token + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/resource_access_token') + + expect(json_response["name"]).to eq(token.name) + expect(json_response["scopes"]).to eq(token.scopes) + + if source_type == 'project' + expect(json_response["access_level"]).to eq(resource.team.max_member_access(token.user.id)) + else + expect(json_response["access_level"]).to eq(resource.max_member_access_for_user(token.user)) + end + + expect(json_response["expires_at"]).to eq(token.expires_at.to_date.iso8601) + end + + context "when using #{source_type} access token to GET other #{source_type} access token" do + let_it_be(:other_project_bot) { create(:user, :project_bot) } + let_it_be(:other_token) { create(:personal_access_token, user: other_project_bot) } + let_it_be(:token_id) { other_token.id } + + before do + resource.add_maintainer(other_project_bot) + end + + it "gets the #{source_type} access token from the #{source_type}" do + get_token + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/resource_access_token') + + expect(json_response["name"]).to eq(other_token.name) + expect(json_response["scopes"]).to eq(other_token.scopes) + + if source_type == 'project' + expect(json_response["access_level"]).to eq(resource.team.max_member_access(other_token.user.id)) + else + expect(json_response["access_level"]).to eq(resource.max_member_access_for_user(other_token.user)) + end + + expect(json_response["expires_at"]).to eq(other_token.expires_at.to_date.iso8601) + end + end + + context "when attempting to get a non-existent #{source_type} access token" do + let_it_be(:token_id) { non_existing_record_id } + + it "does not get the token, and returns 404" do + get_token + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.body).to include("Could not find #{source_type} access token with token_id: #{token_id}") + end + end + + context "when attempting to get a token that does not belong to the specified #{source_type}" do + let_it_be(:resource_id) { other_resource.id } + + it "does not get the token, and returns 404" do + get_token + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.body).to include("Could not find #{source_type} access token with token_id: #{token_id}") + end + end + end + + context "when the user does not have valid permissions" do + let_it_be(:user) { user_non_priviledged } + + it "returns 401" do + get_token + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + context "DELETE #{source_type}s/:id/access_tokens/:token_id", :sidekiq_inline do subject(:delete_token) { delete api("/#{source_type}s/#{resource_id}/access_tokens/#{token_id}", user) } diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index f7048a1ca6b..c724c69045e 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -91,7 +91,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do end end - it "updates application settings" do + it "updates application settings", fips_mode: false do put api("/application/settings", admin), params: { default_ci_config_path: 'debian/salsa-ci.yml', @@ -286,6 +286,55 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['hashed_storage_enabled']).to eq(true) end + context 'SSH key restriction settings', :fips_mode do + let(:settings) do + { + dsa_key_restriction: -1, + ecdsa_key_restriction: 256, + ecdsa_sk_key_restriction: 256, + ed25519_key_restriction: 256, + ed25519_sk_key_restriction: 256, + rsa_key_restriction: 3072 + } + end + + it 'allows updating the settings' do + put api("/application/settings", admin), params: settings + + expect(response).to have_gitlab_http_status(:ok) + settings.each do |attribute, value| + expect(ApplicationSetting.current.public_send(attribute)).to eq(value) + end + end + + it 'does not allow DSA keys' do + put api("/application/settings", admin), params: { dsa_key_restriction: 1024 } + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'does not allow short RSA key values' do + put api("/application/settings", admin), params: { rsa_key_restriction: 2048 } + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'does not allow unrestricted key lengths' do + types = %w(dsa_key_restriction + ecdsa_key_restriction + ecdsa_sk_key_restriction + ed25519_key_restriction + ed25519_sk_key_restriction + rsa_key_restriction) + + types.each do |type| + put api("/application/settings", admin), params: { type => 0 } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + context 'external policy classification settings' do let(:settings) do { diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index eadceeba03b..c554463df76 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -83,19 +83,21 @@ RSpec.describe API::Users do describe 'GET /users/' do context 'when unauthenticated' do - it "does not contain the note of users" do + it "does not contain certain fields" do get api("/users"), params: { username: user.username } expect(json_response.first).not_to have_key('note') + expect(json_response.first).not_to have_key('namespace_id') end end context 'when authenticated' do context 'as a regular user' do - it 'does not contain the note of users' do + it 'does not contain certain fields' do get api("/users", user), params: { username: user.username } expect(json_response.first).not_to have_key('note') + expect(json_response.first).not_to have_key('namespace_id') end end @@ -154,6 +156,7 @@ RSpec.describe API::Users do get api("/user", user) expect(json_response).not_to have_key('note') + expect(json_response).not_to have_key('namespace_id') end end end @@ -335,12 +338,14 @@ RSpec.describe API::Users do expect(response).to match_response_schema('public_api/v4/user/basics') expect(json_response.first.keys).not_to include 'is_admin' end + end + context "when admin" do context 'exclude_internal param' do let_it_be(:internal_user) { User.alert_bot } it 'returns all users when it is not set' do - get api("/users?exclude_internal=false", user) + get api("/users?exclude_internal=false", admin) expect(response).to match_response_schema('public_api/v4/user/basics') expect(response).to include_pagination_headers @@ -356,6 +361,26 @@ RSpec.describe API::Users do end end + context 'without_project_bots param' do + let_it_be(:project_bot) { create(:user, :project_bot) } + + it 'returns all users when it is not set' do + get api("/users?without_project_bots=false", user) + + expect(response).to match_response_schema('public_api/v4/user/basics') + expect(response).to include_pagination_headers + expect(json_response.map { |u| u['id'] }).to include(project_bot.id) + end + + it 'returns all non project_bot users when it is set' do + get api("/users?without_project_bots=true", user) + + expect(response).to match_response_schema('public_api/v4/user/basics') + expect(response).to include_pagination_headers + expect(json_response.map { |u| u['id'] }).not_to include(project_bot.id) + end + end + context 'admins param' do it 'returns all users' do get api("/users?admins=true", user) @@ -384,6 +409,15 @@ RSpec.describe API::Users do expect(response).to include_pagination_headers end + it "users contain the `namespace_id` field" do + get api("/users", admin) + + expect(response).to have_gitlab_http_status(:success) + expect(response).to match_response_schema('public_api/v4/user/admins') + expect(json_response.size).to eq(2) + expect(json_response.map { |u| u['namespace_id'] }).to include(user.namespace_id, admin.namespace_id) + end + it "returns an array of external users" do create(:user, external: true) @@ -697,6 +731,14 @@ RSpec.describe API::Users do expect(json_response['highest_role']).to be(0) end + it 'includes the `namespace_id` field' do + get api("/users/#{user.id}", admin) + + expect(response).to have_gitlab_http_status(:success) + expect(response).to match_response_schema('public_api/v4/user/admin') + expect(json_response['namespace_id']).to eq(user.namespace_id) + end + if Gitlab.ee? it 'does not include values for plan or trial' do get api("/users/#{user.id}", admin) @@ -1934,7 +1976,7 @@ RSpec.describe API::Users do end end - describe "POST /users/:id/emails" do + describe "POST /users/:id/emails", :mailer do it "does not create invalid email" do post api("/users/#{user.id}/emails", admin), params: {} @@ -1944,11 +1986,15 @@ RSpec.describe API::Users do it "creates unverified email" do email_attrs = attributes_for :email - expect do - post api("/users/#{user.id}/emails", admin), params: email_attrs - end.to change { user.emails.count }.by(1) + + perform_enqueued_jobs do + expect do + post api("/users/#{user.id}/emails", admin), params: email_attrs + end.to change { user.emails.count }.by(1) + end expect(json_response['confirmed_at']).to be_nil + should_email(user) end it "returns a 400 for invalid ID" do diff --git a/spec/requests/api/v3/github_spec.rb b/spec/requests/api/v3/github_spec.rb index 838948132dd..5bfea15f0ca 100644 --- a/spec/requests/api/v3/github_spec.rb +++ b/spec/requests/api/v3/github_spec.rb @@ -9,7 +9,7 @@ RSpec.describe API::V3::Github do let_it_be_with_reload(:project) { create(:project, :repository, creator: user) } before do - project.add_maintainer(user) + project.add_maintainer(user) if user end describe 'GET /orgs/:namespace/repos' do diff --git a/spec/requests/groups/crm/contacts_controller_spec.rb b/spec/requests/groups/crm/contacts_controller_spec.rb index 4d8ca0fcd60..0ee72233418 100644 --- a/spec/requests/groups/crm/contacts_controller_spec.rb +++ b/spec/requests/groups/crm/contacts_controller_spec.rb @@ -85,28 +85,19 @@ RSpec.describe Groups::Crm::ContactsController do end describe 'GET #index' do - subject do - get group_crm_contacts_path(group) - response - end + subject { get group_crm_contacts_path(group) } it_behaves_like 'ok response with index template if authorized' end describe 'GET #new' do - subject do - get new_group_crm_contact_path(group) - response - end + subject { get new_group_crm_contact_path(group) } it_behaves_like 'ok response with index template if authorized' end describe 'GET #edit' do - subject do - get edit_group_crm_contact_path(group, id: 1) - response - end + subject { get edit_group_crm_contact_path(group, id: 1) } it_behaves_like 'ok response with index template if authorized' end diff --git a/spec/requests/groups/crm/organizations_controller_spec.rb b/spec/requests/groups/crm/organizations_controller_spec.rb index 37ffac71772..410fc979262 100644 --- a/spec/requests/groups/crm/organizations_controller_spec.rb +++ b/spec/requests/groups/crm/organizations_controller_spec.rb @@ -85,18 +85,19 @@ RSpec.describe Groups::Crm::OrganizationsController do end describe 'GET #index' do - subject do - get group_crm_organizations_path(group) - response - end + subject { get group_crm_organizations_path(group) } it_behaves_like 'ok response with index template if authorized' end describe 'GET #new' do - subject do - get new_group_crm_organization_path(group) - end + subject { get new_group_crm_organization_path(group) } + + it_behaves_like 'ok response with index template if authorized' + end + + describe 'GET #edit' do + subject { get edit_group_crm_organization_path(group, id: 1) } it_behaves_like 'ok response with index template if authorized' end diff --git a/spec/requests/groups/email_campaigns_controller_spec.rb b/spec/requests/groups/email_campaigns_controller_spec.rb index 9ed828d1a9a..4d630ef6710 100644 --- a/spec/requests/groups/email_campaigns_controller_spec.rb +++ b/spec/requests/groups/email_campaigns_controller_spec.rb @@ -94,7 +94,7 @@ RSpec.describe Groups::EmailCampaignsController do describe 'track parameter' do context 'when valid' do - where(track: [Namespaces::InProductMarketingEmailsService::TRACKS.keys.without(:experience), Namespaces::InviteTeamEmailService::TRACK].flatten) + where(track: Namespaces::InProductMarketingEmailsService::TRACKS.keys.without(:experience)) with_them do it_behaves_like 'track and redirect' @@ -117,10 +117,6 @@ RSpec.describe Groups::EmailCampaignsController do with_them do it_behaves_like 'track and redirect' end - - it_behaves_like 'track and redirect' do - let(:track) { Namespaces::InviteTeamEmailService::TRACK.to_s } - end end context 'when invalid' do @@ -128,10 +124,6 @@ RSpec.describe Groups::EmailCampaignsController do with_them do it_behaves_like 'no track and 404' - - it_behaves_like 'no track and 404' do - let(:track) { Namespaces::InviteTeamEmailService::TRACK.to_s } - end end end end diff --git a/spec/requests/import/gitlab_groups_controller_spec.rb b/spec/requests/import/gitlab_groups_controller_spec.rb index 4abf99cf994..8d5c1e3ebab 100644 --- a/spec/requests/import/gitlab_groups_controller_spec.rb +++ b/spec/requests/import/gitlab_groups_controller_spec.rb @@ -155,20 +155,6 @@ RSpec.describe Import::GitlabGroupsController do end end - context 'when group import FF is disabled' do - let(:request_params) { { path: 'test-group-import', name: 'test-group-import' } } - - before do - stub_feature_flags(group_import_export: false) - end - - it 'returns an error' do - expect { import_request }.not_to change { Group.count } - - expect(response).to have_gitlab_http_status(:not_found) - end - end - context 'when the parent group is invalid' do let(:request_params) { { path: 'test-group-import', name: 'test-group-import', parent_id: -1 } } diff --git a/spec/requests/jira_authorizations_spec.rb b/spec/requests/jira_authorizations_spec.rb index 24c6001814c..b43d36e94f4 100644 --- a/spec/requests/jira_authorizations_spec.rb +++ b/spec/requests/jira_authorizations_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe 'Jira authorization requests' do let(:user) { create :user } let(:application) { create :oauth_application, scopes: 'api' } - let(:redirect_uri) { oauth_jira_callback_url(host: "http://www.example.com") } + let(:redirect_uri) { oauth_jira_dvcs_callback_url(host: "http://www.example.com") } def generate_access_grant create :oauth_access_grant, application: application, resource_owner_id: user.id, redirect_uri: redirect_uri diff --git a/spec/requests/projects/work_items_spec.rb b/spec/requests/projects/work_items_spec.rb new file mode 100644 index 00000000000..e6365a3824a --- /dev/null +++ b/spec/requests/projects/work_items_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Work Items' do + let_it_be(:work_item) { create(:work_item) } + let_it_be(:developer) { create(:user) } + + before_all do + work_item.project.add_developer(developer) + end + + describe 'GET /:namespace/:project/work_items/:id' do + before do + sign_in(developer) + end + + context 'when the work_items feature flag is enabled' do + it 'renders index' do + get project_work_items_url(work_item.project, work_items_path: work_item.id) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when the work_items feature flag is disabled' do + before do + stub_feature_flags(work_items: false) + end + + it 'returns 404' do + get project_work_items_url(work_item.project, work_items_path: work_item.id) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end |