diff options
Diffstat (limited to 'spec/controllers')
49 files changed, 1225 insertions, 757 deletions
diff --git a/spec/controllers/admin/dashboard_controller_spec.rb b/spec/controllers/admin/dashboard_controller_spec.rb index 283d82a3ab8..bfbd2ca946f 100644 --- a/spec/controllers/admin/dashboard_controller_spec.rb +++ b/spec/controllers/admin/dashboard_controller_spec.rb @@ -4,12 +4,20 @@ require 'spec_helper' RSpec.describe Admin::DashboardController do describe '#index' do + before do + sign_in(create(:admin)) + end + + it 'retrieves Redis versions' do + get :index + + expect(assigns[:redis_versions].length).to eq(1) + end + context 'with pending_delete projects' do render_views it 'does not retrieve projects that are pending deletion' do - sign_in(create(:admin)) - project = create(:project) pending_delete_project = create(:project, pending_delete: true) diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 5312a0db7f5..d0d1fa6a6bc 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -107,49 +107,40 @@ RSpec.describe Admin::UsersController do subject { put :approve, params: { id: user.username } } - context 'when feature is disabled' do - before do - stub_feature_flags(admin_approval_for_new_user_signups: false) - end - - it 'responds with access denied' do + context 'when successful' do + it 'activates the user' do subject - expect(response).to have_gitlab_http_status(:not_found) + user.reload + + expect(user).to be_active + expect(flash[:notice]).to eq('Successfully approved') end - end - context 'when feature is enabled' do - before do - stub_feature_flags(admin_approval_for_new_user_signups: true) + it 'emails the user on approval' do + expect(DeviseMailer).to receive(:user_admin_approval).with(user).and_call_original + expect { subject }.to have_enqueued_mail(DeviseMailer, :user_admin_approval) end + end - context 'when successful' do - it 'activates the user' do - subject + context 'when unsuccessful' do + let(:user) { create(:user, :blocked) } - user.reload + it 'displays the error' do + subject - expect(user).to be_active - expect(flash[:notice]).to eq('Successfully approved') - end + expect(flash[:alert]).to eq('The user you are trying to approve is not pending an approval') end - context 'when unsuccessful' do - let(:user) { create(:user, :blocked) } - - it 'displays the error' do - subject - - expect(flash[:alert]).to eq('The user you are trying to approve is not pending an approval') - end + it 'does not activate the user' do + subject - it 'does not activate the user' do - subject + user.reload + expect(user).not_to be_active + end - user.reload - expect(user).not_to be_active - end + it 'does not email the pending user' do + expect { subject }.not_to have_enqueued_mail(DeviseMailer, :user_admin_approval) end end end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index d95aac2f386..9342513d224 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -171,6 +171,8 @@ RSpec.describe ApplicationController do describe '#route_not_found' do controller(described_class) do + skip_before_action :authenticate_user!, only: :index + def index route_not_found end @@ -184,6 +186,14 @@ RSpec.describe ApplicationController do expect(response).to have_gitlab_http_status(:not_found) end + it 'renders 404 if client is a search engine crawler' do + request.env['HTTP_USER_AGENT'] = 'Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)' + + get :index + + expect(response).to have_gitlab_http_status(:not_found) + end + it 'redirects to login page if not authenticated' do get :index diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index e7c0bc43e86..c2eb9d54303 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -382,6 +382,17 @@ RSpec.describe AutocompleteController do sign_in(user) end + context 'and they cannot read the project' do + it 'returns a not found response' do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(false) + + get(:deploy_keys_with_owners, params: { project_id: project.id }) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + it 'renders the deploy key in a json payload, with its owner' do get(:deploy_keys_with_owners, params: { project_id: project.id }) diff --git a/spec/controllers/concerns/controller_with_feature_category_spec.rb b/spec/controllers/concerns/controller_with_feature_category_spec.rb deleted file mode 100644 index 55e84755f5c..00000000000 --- a/spec/controllers/concerns/controller_with_feature_category_spec.rb +++ /dev/null @@ -1,60 +0,0 @@ -# frozen_string_literal: true - -require 'fast_spec_helper' -require_relative "../../../app/controllers/concerns/controller_with_feature_category" - -RSpec.describe ControllerWithFeatureCategory do - describe ".feature_category_for_action" do - let(:base_controller) do - Class.new do - include ControllerWithFeatureCategory - end - end - - let(:controller) do - Class.new(base_controller) do - feature_category :foo, %w(update edit) - feature_category :bar, %w(index show) - feature_category :quux, %w(destroy) - end - end - - let(:subclass) do - Class.new(controller) do - feature_category :baz, %w(subclass_index) - end - end - - it "is nil when nothing was defined" do - expect(base_controller.feature_category_for_action("hello")).to be_nil - end - - it "returns the expected category", :aggregate_failures do - expect(controller.feature_category_for_action("update")).to eq(:foo) - expect(controller.feature_category_for_action("index")).to eq(:bar) - expect(controller.feature_category_for_action("destroy")).to eq(:quux) - end - - it "returns the expected category for categories defined in subclasses" do - expect(subclass.feature_category_for_action("subclass_index")).to eq(:baz) - end - - it "raises an error when defining for the controller and for individual actions" do - expect do - Class.new(base_controller) do - feature_category :hello - feature_category :goodbye, [:world] - end - end.to raise_error(ArgumentError, "hello is defined for all actions, but other categories are set") - end - - it "raises an error when multiple calls define the same action" do - expect do - Class.new(base_controller) do - feature_category :hello, [:world] - feature_category :goodbye, ["world"] - end - end.to raise_error(ArgumentError, "Actions have multiple feature categories: world") - end - end -end diff --git a/spec/controllers/concerns/lfs_request_spec.rb b/spec/controllers/concerns/lfs_request_spec.rb deleted file mode 100644 index 3bafd761a3e..00000000000 --- a/spec/controllers/concerns/lfs_request_spec.rb +++ /dev/null @@ -1,75 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe LfsRequest do - include ProjectForksHelper - - controller(Repositories::GitHttpClientController) do - # `described_class` is not available in this context - include LfsRequest - - def show - head :ok - end - - def project - @project ||= Project.find_by(id: params[:id]) - end - - def download_request? - true - end - - def upload_request? - false - end - - def ci? - false - end - end - - let(:project) { create(:project, :public) } - - before do - stub_lfs_setting(enabled: true) - end - - context 'user is authenticated without access to lfs' do - before do - allow(controller).to receive(:authenticate_user) - allow(controller).to receive(:authentication_result) do - Gitlab::Auth::Result.new - end - end - - context 'with access to the project' do - it 'returns 403' do - get :show, params: { id: project.id } - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'without access to the project' do - context 'project does not exist' do - it 'returns 404' do - get :show, params: { id: 'does not exist' } - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'project is private' do - let(:project) { create(:project, :private) } - - it 'returns 404' do - get :show, params: { id: project.id } - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - end -end diff --git a/spec/controllers/concerns/metrics_dashboard_spec.rb b/spec/controllers/concerns/metrics_dashboard_spec.rb index 8a4d8828aaa..83546403ce5 100644 --- a/spec/controllers/concerns/metrics_dashboard_spec.rb +++ b/spec/controllers/concerns/metrics_dashboard_spec.rb @@ -155,6 +155,7 @@ RSpec.describe MetricsDashboard do '.gitlab/dashboards/errors.yml' => dashboard_yml } end + let_it_be(:project) { create(:project, :custom_repo, files: dashboards) } before do diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb index 747ccd7ba1b..f9a6afb95ea 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -70,61 +70,18 @@ RSpec.describe SendFileUpload do allow(uploader).to receive(:model).and_return(image_owner) end - context 'when boths FFs are enabled' do - before do - stub_feature_flags(dynamic_image_resizing_requester: image_requester) - stub_feature_flags(dynamic_image_resizing_owner: image_owner) - end - - it_behaves_like 'handles image resize requests allowed by FFs' - end - - context 'when boths FFs are enabled globally' do - before do - stub_feature_flags(dynamic_image_resizing_requester: true) - stub_feature_flags(dynamic_image_resizing_owner: true) - end - - it_behaves_like 'handles image resize requests allowed by FFs' - - context 'when current_user is nil' do - before do - allow(controller).to receive(:current_user).and_return(nil) - end - - it_behaves_like 'handles image resize requests allowed by FFs' - end - end - - context 'when only FF based on content requester is enabled for current user' do - before do - stub_feature_flags(dynamic_image_resizing_requester: image_requester) - stub_feature_flags(dynamic_image_resizing_owner: false) - end - - it_behaves_like 'bypasses image resize requests not allowed by FFs' - end - - context 'when only FF based on content owner is enabled for requested avatar owner' do - before do - stub_feature_flags(dynamic_image_resizing_requester: false) - stub_feature_flags(dynamic_image_resizing_owner: image_owner) - end - - it_behaves_like 'bypasses image resize requests not allowed by FFs' - end + it_behaves_like 'handles image resize requests allowed by FF' - context 'when both FFs are disabled' do + context 'when FF is disabled' do before do - stub_feature_flags(dynamic_image_resizing_requester: false) - stub_feature_flags(dynamic_image_resizing_owner: false) + stub_feature_flags(dynamic_image_resizing: false) end - it_behaves_like 'bypasses image resize requests not allowed by FFs' + it_behaves_like 'bypasses image resize requests not allowed by FF' end end - shared_examples 'bypasses image resize requests not allowed by FFs' do + shared_examples 'bypasses image resize requests not allowed by FF' do it 'does not write workhorse command header' do expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) @@ -132,7 +89,7 @@ RSpec.describe SendFileUpload do end end - shared_examples 'handles image resize requests allowed by FFs' do + shared_examples 'handles image resize requests allowed by FF' do context 'with valid width parameter' do it 'renders OK with workhorse command header' do expect(controller).not_to receive(:send_file) diff --git a/spec/controllers/every_controller_spec.rb b/spec/controllers/every_controller_spec.rb index b1519c4ef1e..a1c377eff76 100644 --- a/spec/controllers/every_controller_spec.rb +++ b/spec/controllers/every_controller_spec.rb @@ -17,7 +17,7 @@ RSpec.describe "Every controller" do .compact .select { |route| route[:controller].present? && route[:action].present? } .map { |route| [constantize_controller(route[:controller]), route[:action]] } - .select { |(controller, action)| controller&.include?(ControllerWithFeatureCategory) } + .select { |(controller, action)| controller&.include?(::Gitlab::WithFeatureCategory) } .reject { |(controller, action)| controller == ApplicationController || controller == Devise::UnlocksController } end diff --git a/spec/controllers/groups/dependency_proxies_controller_spec.rb b/spec/controllers/groups/dependency_proxies_controller_spec.rb new file mode 100644 index 00000000000..35bd7d47aed --- /dev/null +++ b/spec/controllers/groups/dependency_proxies_controller_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::DependencyProxiesController do + let(:group) { create(:group) } + let(:user) { create(:user) } + + before do + group.add_owner(user) + sign_in(user) + end + + describe 'GET #show' do + context 'feature enabled' do + before do + enable_dependency_proxy + end + + it 'returns 200 and renders the view' do + get :show, params: { group_id: group.to_param } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('groups/dependency_proxies/show') + end + end + + it 'returns 404 when feature is disabled' do + disable_dependency_proxy + + get :show, params: { group_id: group.to_param } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + describe 'PUT #update' do + context 'feature enabled' do + before do + enable_dependency_proxy + end + + it 'redirects back to show page' do + put :update, params: update_params + + expect(response).to have_gitlab_http_status(:found) + end + end + + it 'returns 404 when feature is disabled' do + put :update, params: update_params + + expect(response).to have_gitlab_http_status(:not_found) + end + + def update_params + { + group_id: group.to_param, + dependency_proxy_group_setting: { enabled: true } + } + end + end + + def enable_dependency_proxy + stub_config(dependency_proxy: { enabled: true }) + + group.create_dependency_proxy_setting!(enabled: true) + end + + def disable_dependency_proxy + group.create_dependency_proxy_setting!(enabled: false) + end +end diff --git a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb new file mode 100644 index 00000000000..615b56ff22f --- /dev/null +++ b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb @@ -0,0 +1,161 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::DependencyProxyForContainersController do + let(:group) { create(:group) } + let(:token_response) { { status: :success, token: 'abcd1234' } } + + shared_examples 'not found when disabled' do + context 'feature disabled' do + before do + disable_dependency_proxy + end + + it 'returns 404' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + before do + allow(Gitlab.config.dependency_proxy) + .to receive(:enabled).and_return(true) + + allow_next_instance_of(DependencyProxy::RequestTokenService) do |instance| + allow(instance).to receive(:execute).and_return(token_response) + end + end + + describe 'GET #manifest' do + let(:manifest) { { foo: 'bar' }.to_json } + let(:pull_response) { { status: :success, manifest: manifest } } + + before do + allow_next_instance_of(DependencyProxy::PullManifestService) do |instance| + allow(instance).to receive(:execute).and_return(pull_response) + end + end + + subject { get_manifest } + + context 'feature enabled' do + before do + enable_dependency_proxy + end + + context 'remote token request fails' do + let(:token_response) do + { + status: :error, + http_status: 503, + message: 'Service Unavailable' + } + end + + it 'proxies status from the remote token request' do + subject + + expect(response).to have_gitlab_http_status(:service_unavailable) + expect(response.body).to eq('Service Unavailable') + end + end + + context 'remote manifest request fails' do + let(:pull_response) do + { + status: :error, + http_status: 400, + message: '' + } + end + + it 'proxies status from the remote manifest request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to be_empty + end + end + + it 'returns 200 with manifest file' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(manifest) + end + end + + it_behaves_like 'not found when disabled' + + def get_manifest + get :manifest, params: { group_id: group.to_param, image: 'alpine', tag: '3.9.2' } + end + end + + describe 'GET #blob' do + let(:blob) { create(:dependency_proxy_blob) } + let(:blob_sha) { blob.file_name.sub('.gz', '') } + let(:blob_response) { { status: :success, blob: blob } } + + before do + allow_next_instance_of(DependencyProxy::FindOrCreateBlobService) do |instance| + allow(instance).to receive(:execute).and_return(blob_response) + end + end + + subject { get_blob } + + context 'feature enabled' do + before do + enable_dependency_proxy + end + + context 'remote blob request fails' do + let(:blob_response) do + { + status: :error, + http_status: 400, + message: '' + } + end + + it 'proxies status from the remote blob request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to be_empty + end + end + + it 'sends a file' do + expect(controller).to receive(:send_file).with(blob.file.path, {}) + + subject + end + + it 'returns Content-Disposition: attachment' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers['Content-Disposition']).to match(/^attachment/) + end + end + + it_behaves_like 'not found when disabled' + + def get_blob + get :blob, params: { group_id: group.to_param, image: 'alpine', sha: blob_sha } + end + end + + def enable_dependency_proxy + group.create_dependency_proxy_setting!(enabled: true) + end + + def disable_dependency_proxy + group.create_dependency_proxy_setting!(enabled: false) + end +end diff --git a/spec/controllers/groups/registry/repositories_controller_spec.rb b/spec/controllers/groups/registry/repositories_controller_spec.rb index ae982b02a4f..70125087f30 100644 --- a/spec/controllers/groups/registry/repositories_controller_spec.rb +++ b/spec/controllers/groups/registry/repositories_controller_spec.rb @@ -64,12 +64,11 @@ RSpec.describe Groups::Registry::RepositoriesController do context 'html format' do let(:format) { :html } - it 'show index page' do - expect(Gitlab::Tracking).not_to receive(:event) - + it 'show index page', :snowplow do subject expect(response).to have_gitlab_http_status(:ok) + expect_no_snowplow_event end end diff --git a/spec/controllers/groups/settings/integrations_controller_spec.rb b/spec/controllers/groups/settings/integrations_controller_spec.rb index cdcdfde175f..beb2ad3afec 100644 --- a/spec/controllers/groups/settings/integrations_controller_spec.rb +++ b/spec/controllers/groups/settings/integrations_controller_spec.rb @@ -46,7 +46,7 @@ RSpec.describe Groups::Settings::IntegrationsController do describe '#edit' do context 'when user is not owner' do it 'renders not_found' do - get :edit, params: { group_id: group, id: Service.available_services_names.sample } + get :edit, params: { group_id: group, id: Service.available_services_names(include_project_specific: false).sample } expect(response).to have_gitlab_http_status(:not_found) end @@ -61,13 +61,13 @@ RSpec.describe Groups::Settings::IntegrationsController do it 'returns not_found' do stub_feature_flags(group_level_integrations: false) - get :edit, params: { group_id: group, id: Service.available_services_names.sample } + get :edit, params: { group_id: group, id: Service.available_services_names(include_project_specific: false).sample } expect(response).to have_gitlab_http_status(:not_found) end end - Service.available_services_names.each do |integration_name| + Service.available_services_names(include_project_specific: false).each do |integration_name| context "#{integration_name}" do it 'successfully displays the template' do get :edit, params: { group_id: group, id: integration_name } diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index df7e018b35e..55833ee3aad 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -319,10 +319,10 @@ RSpec.describe GroupsController, factory_default: :keep do stub_experiment(onboarding_issues: false) end - it 'does not track anything' do - expect(Gitlab::Tracking).not_to receive(:event) - + it 'does not track anything', :snowplow do create_namespace + + expect_no_snowplow_event end end @@ -336,15 +336,15 @@ RSpec.describe GroupsController, factory_default: :keep do stub_experiment_for_user(onboarding_issues: false) end - it 'tracks the event with the "created_namespace" action with the "control_group" property' do - expect(Gitlab::Tracking).to receive(:event).with( - 'Growth::Conversion::Experiment::OnboardingIssues', - 'created_namespace', + it 'tracks the event with the "created_namespace" action with the "control_group" property', :snowplow do + create_namespace + + expect_snowplow_event( + category: 'Growth::Conversion::Experiment::OnboardingIssues', + action: 'created_namespace', label: anything, property: 'control_group' ) - - create_namespace end end @@ -353,15 +353,15 @@ RSpec.describe GroupsController, factory_default: :keep do stub_experiment_for_user(onboarding_issues: true) end - it 'tracks the event with the "created_namespace" action with the "experimental_group" property' do - expect(Gitlab::Tracking).to receive(:event).with( - 'Growth::Conversion::Experiment::OnboardingIssues', - 'created_namespace', + it 'tracks the event with the "created_namespace" action with the "experimental_group" property', :snowplow do + create_namespace + + expect_snowplow_event( + category: 'Growth::Conversion::Experiment::OnboardingIssues', + action: 'created_namespace', label: anything, property: 'experimental_group' ) - - create_namespace end end end @@ -1213,4 +1213,60 @@ RSpec.describe GroupsController, factory_default: :keep do it_behaves_like 'disabled when using an external authorization service' end end + + describe 'GET #unfoldered_environment_names' do + it 'shows the environment names of a public project to an anonymous user' do + public_project = create(:project, :public, namespace: group) + + create(:environment, project: public_project, name: 'foo') + + get( + :unfoldered_environment_names, + params: { id: group, format: :json } + ) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq(%w[foo]) + end + + it 'does not show environment names of private projects to anonymous users' do + create(:environment, project: project, name: 'foo') + + get( + :unfoldered_environment_names, + params: { id: group, format: :json } + ) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + end + + it 'shows environment names of a private project to a group member' do + create(:environment, project: project, name: 'foo') + sign_in(developer) + + get( + :unfoldered_environment_names, + params: { id: group, format: :json } + ) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq(%w[foo]) + end + + it 'does not show environment names of private projects to a logged-in non-member' do + alice = create(:user) + + create(:environment, project: project, name: 'foo') + sign_in(alice) + + get( + :unfoldered_environment_names, + params: { id: group, format: :json } + ) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + end + end end diff --git a/spec/controllers/import/bulk_imports_controller_spec.rb b/spec/controllers/import/bulk_imports_controller_spec.rb index f3850ff844e..dd850a86227 100644 --- a/spec/controllers/import/bulk_imports_controller_spec.rb +++ b/spec/controllers/import/bulk_imports_controller_spec.rb @@ -24,7 +24,7 @@ RSpec.describe Import::BulkImportsController do expect(session[:bulk_import_gitlab_url]).to be_nil expect(response).to have_gitlab_http_status(:found) - expect(response).to redirect_to(status_import_bulk_import_url) + expect(response).to redirect_to(status_import_bulk_imports_url) end end @@ -37,7 +37,7 @@ RSpec.describe Import::BulkImportsController do expect(session[:bulk_import_gitlab_access_token]).to eq(token) expect(session[:bulk_import_gitlab_url]).to eq(url) expect(response).to have_gitlab_http_status(:found) - expect(response).to redirect_to(status_import_bulk_import_url) + expect(response).to redirect_to(status_import_bulk_imports_url) end it 'strips access token with spaces' do @@ -46,19 +46,21 @@ RSpec.describe Import::BulkImportsController do post :configure, params: { bulk_import_gitlab_access_token: " #{token} " } expect(session[:bulk_import_gitlab_access_token]).to eq(token) - expect(controller).to redirect_to(status_import_bulk_import_url) + expect(controller).to redirect_to(status_import_bulk_imports_url) end end describe 'GET status' do - let(:client) { Gitlab::BulkImport::Client.new(uri: 'http://gitlab.example', token: 'token') } + let(:client) { BulkImports::Clients::Http.new(uri: 'http://gitlab.example', token: 'token') } describe 'serialized group data' do let(:client_response) do - [ - { 'id' => 1, 'full_name' => 'group1', 'full_path' => 'full/path/group1' }, - { 'id' => 2, 'full_name' => 'group2', 'full_path' => 'full/path/group2' } - ] + double( + parsed_response: [ + { 'id' => 1, 'full_name' => 'group1', 'full_path' => 'full/path/group1' }, + { 'id' => 2, 'full_name' => 'group2', 'full_path' => 'full/path/group2' } + ] + ) end before do @@ -69,7 +71,7 @@ RSpec.describe Import::BulkImportsController do it 'returns serialized group data' do get :status, format: :json - expect(response.parsed_body).to eq({ importable_data: client_response }.as_json) + expect(json_response).to eq({ importable_data: client_response.parsed_response }.as_json) end end @@ -111,7 +113,7 @@ RSpec.describe Import::BulkImportsController do context 'when connection error occurs' do before do allow(controller).to receive(:client).and_return(client) - allow(client).to receive(:get).and_raise(Gitlab::BulkImport::Client::ConnectionError) + allow(client).to receive(:get).and_raise(BulkImports::Clients::Http::ConnectionError) end it 'returns 422' do @@ -128,9 +130,21 @@ RSpec.describe Import::BulkImportsController do end end end + + describe 'POST create' do + it 'executes BulkImportService' do + expect_next_instance_of(BulkImportService) do |service| + expect(service).to receive(:execute) + end + + post :create + + expect(response).to have_gitlab_http_status(:ok) + end + end end - context 'when gitlab_api_imports feature flag is disabled' do + context 'when bulk_import feature flag is disabled' do before do stub_feature_flags(bulk_import: false) end diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index e19b6caca5b..a408d821833 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -144,6 +144,58 @@ RSpec.describe Import::GithubController do expect(json_response.dig('provider_repos', 0, 'id')).to eq(repo_1.id) expect(json_response.dig('provider_repos', 1, 'id')).to eq(repo_2.id) end + + context 'when filtering' do + let(:filter) { 'test' } + let(:user_login) { 'user' } + let(:collaborations_subquery) { 'repo:repo1 repo:repo2' } + let(:organizations_subquery) { 'org:org1 org:org2' } + + before do + allow_next_instance_of(Octokit::Client) do |client| + allow(client).to receive(:user).and_return(double(login: user_login)) + end + end + + it 'makes request to github search api' do + expected_query = "test in:name is:public,private user:#{user_login} #{collaborations_subquery} #{organizations_subquery}" + + expect_next_instance_of(Gitlab::GithubImport::Client) do |client| + expect(client).to receive(:collaborations_subquery).and_return(collaborations_subquery) + expect(client).to receive(:organizations_subquery).and_return(organizations_subquery) + expect(client).to receive(:each_page).with(:search_repositories, expected_query).and_return([].to_enum) + end + + get :status, params: { filter: filter }, format: :json + end + + context 'when user input contains colons and spaces' do + before do + stub_client(search_repos_by_name: []) + end + + it 'sanitizes user input' do + filter = ' test1:test2 test3 : test4 ' + expected_filter = 'test1test2test3test4' + + get :status, params: { filter: filter }, format: :json + + expect(assigns(:filter)).to eq(expected_filter) + end + end + + context 'when rate limit threshold is exceeded' do + before do + allow(controller).to receive(:status).and_raise(Gitlab::GithubImport::RateLimitError) + end + + it 'returns 429' do + get :status, params: { filter: 'test' }, format: :json + + expect(response).to have_gitlab_http_status(:too_many_requests) + end + end + end end end diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index 75a972d2f95..2d13b942c31 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -9,13 +9,6 @@ RSpec.describe InvitesController, :snowplow do let(:project_members) { member.source.users } let(:md5_member_global_id) { Digest::MD5.hexdigest(member.to_global_id.to_s) } let(:params) { { id: raw_invite_token } } - let(:snowplow_event) do - { - category: 'Growth::Acquisition::Experiment::InviteEmail', - label: md5_member_global_id, - property: group_type - } - end shared_examples 'invalid token' do context 'when invite token is not valid' do @@ -94,38 +87,6 @@ RSpec.describe InvitesController, :snowplow do expect(flash[:notice]).to be_nil end - context 'when new_user_invite is not set' do - it 'does not track the user as experiment group' do - request - - expect_no_snowplow_event - end - end - - context 'when new_user_invite is experiment' do - let(:params) { { id: raw_invite_token, new_user_invite: 'experiment' } } - let(:group_type) { 'experiment_group' } - - it 'tracks the user as experiment group' do - request - - expect_snowplow_event(**snowplow_event.merge(action: 'opened')) - expect_snowplow_event(**snowplow_event.merge(action: 'accepted')) - end - end - - context 'when new_user_invite is control' do - let(:params) { { id: raw_invite_token, new_user_invite: 'control' } } - let(:group_type) { 'control_group' } - - it 'tracks the user as control group' do - request - - expect_snowplow_event(**snowplow_event.merge(action: 'opened')) - expect_snowplow_event(**snowplow_event.merge(action: 'accepted')) - end - end - it_behaves_like "tracks the 'accepted' event for the invitation reminders experiment" it_behaves_like 'invalid token' end @@ -158,36 +119,6 @@ RSpec.describe InvitesController, :snowplow do subject(:request) { post :accept, params: params } - context 'when new_user_invite is not set' do - it 'does not track an event' do - request - - expect_no_snowplow_event - end - end - - context 'when new_user_invite is experiment' do - let(:params) { { id: raw_invite_token, new_user_invite: 'experiment' } } - let(:group_type) { 'experiment_group' } - - it 'tracks the user as experiment group' do - request - - expect_snowplow_event(**snowplow_event.merge(action: 'accepted')) - end - end - - context 'when new_user_invite is control' do - let(:params) { { id: raw_invite_token, new_user_invite: 'control' } } - let(:group_type) { 'control_group' } - - it 'tracks the user as control group' do - request - - expect_snowplow_event(**snowplow_event.merge(action: 'accepted')) - end - end - it_behaves_like "tracks the 'accepted' event for the invitation reminders experiment" it_behaves_like 'invalid token' end diff --git a/spec/controllers/jwks_controller_spec.rb b/spec/controllers/jwks_controller_spec.rb new file mode 100644 index 00000000000..013ec01eba2 --- /dev/null +++ b/spec/controllers/jwks_controller_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JwksController do + describe 'GET #index' do + let(:ci_jwt_signing_key) { OpenSSL::PKey::RSA.generate(1024) } + let(:ci_jwk) { ci_jwt_signing_key.to_jwk } + let(:oidc_jwk) { OpenSSL::PKey::RSA.new(Rails.application.secrets.openid_connect_signing_key).to_jwk } + + before do + stub_application_setting(ci_jwt_signing_key: ci_jwt_signing_key.to_s) + end + + it 'returns signing keys used to sign CI_JOB_JWT' do + get :index + + expect(response).to have_gitlab_http_status(:ok) + + ids = json_response['keys'].map { |jwk| jwk['kid'] } + expect(ids).to contain_exactly(ci_jwk['kid'], oidc_jwk['kid']) + end + + it 'does not leak private key data' do + get :index + + aggregate_failures do + json_response['keys'].each do |jwk| + expect(jwk.keys).to contain_exactly('kty', 'kid', 'e', 'n', 'use', 'alg') + expect(jwk['use']).to eq('sig') + expect(jwk['alg']).to eq('RS256') + end + end + end + end +end diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb index 249e6322d1c..7a72a13febe 100644 --- a/spec/controllers/profiles_controller_spec.rb +++ b/spec/controllers/profiles_controller_spec.rb @@ -84,9 +84,10 @@ RSpec.describe ProfilesController, :request_store do it 'allows setting a user status' do sign_in(user) - put :update, params: { user: { status: { message: 'Working hard!' } } } + put :update, params: { user: { status: { message: 'Working hard!', availability: 'busy' } } } expect(user.reload.status.message).to eq('Working hard!') + expect(user.reload.status.availability).to eq('busy') expect(response).to have_gitlab_http_status(:found) end diff --git a/spec/controllers/projects/alerting/notifications_controller_spec.rb b/spec/controllers/projects/alerting/notifications_controller_spec.rb index 33fd73c762a..b3d37723ccf 100644 --- a/spec/controllers/projects/alerting/notifications_controller_spec.rb +++ b/spec/controllers/projects/alerting/notifications_controller_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Projects::Alerting::NotificationsController do let_it_be(:project) { create(:project) } let_it_be(:environment) { create(:environment, project: project) } + let(:params) { project_params } describe 'POST #create' do around do |example| @@ -20,7 +21,7 @@ RSpec.describe Projects::Alerting::NotificationsController do end def make_request - post :create, params: project_params, body: payload.to_json, as: :json + post :create, params: params, body: payload.to_json, as: :json end context 'when notification service succeeds' do @@ -53,26 +54,69 @@ RSpec.describe Projects::Alerting::NotificationsController do context 'bearer token' do context 'when set' do - it 'extracts bearer token' do - request.headers['HTTP_AUTHORIZATION'] = 'Bearer some token' + context 'when extractable' do + before do + request.headers['HTTP_AUTHORIZATION'] = 'Bearer some token' + end - expect(notify_service).to receive(:execute).with('some token') + it 'extracts bearer token' do + expect(notify_service).to receive(:execute).with('some token', nil) - make_request + make_request + end + + context 'with a corresponding integration' do + context 'with integration parameters specified' do + let_it_be_with_reload(:integration) { create(:alert_management_http_integration, project: project) } + let(:params) { project_params(endpoint_identifier: integration.endpoint_identifier, name: integration.name) } + + context 'the integration is active' do + it 'extracts and finds the integration' do + expect(notify_service).to receive(:execute).with('some token', integration) + + make_request + end + end + + context 'when the integration is inactive' do + before do + integration.update!(active: false) + end + + it 'does not find an integration' do + expect(notify_service).to receive(:execute).with('some token', nil) + + make_request + end + end + end + + context 'without integration parameters specified' do + let_it_be(:integration) { create(:alert_management_http_integration, :legacy, project: project) } + + it 'extracts and finds the legacy integration' do + expect(notify_service).to receive(:execute).with('some token', integration) + + make_request + end + end + end end - it 'pass nil if cannot extract a non-bearer token' do - request.headers['HTTP_AUTHORIZATION'] = 'some token' + context 'when inextractable' do + it 'passes nil for a non-bearer token' do + request.headers['HTTP_AUTHORIZATION'] = 'some token' - expect(notify_service).to receive(:execute).with(nil) + expect(notify_service).to receive(:execute).with(nil, nil) - make_request + make_request + end end end context 'when missing' do it 'passes nil' do - expect(notify_service).to receive(:execute).with(nil) + expect(notify_service).to receive(:execute).with(nil, nil) make_request end diff --git a/spec/controllers/projects/avatars_controller_spec.rb b/spec/controllers/projects/avatars_controller_spec.rb index 16e9c845307..35878fe4c2d 100644 --- a/spec/controllers/projects/avatars_controller_spec.rb +++ b/spec/controllers/projects/avatars_controller_spec.rb @@ -3,14 +3,14 @@ require 'spec_helper' RSpec.describe Projects::AvatarsController do - let_it_be(:project) { create(:project, :repository) } + describe 'GET #show' do + let_it_be(:project) { create(:project, :public, :repository) } - before do - controller.instance_variable_set(:@project, project) - end + before do + controller.instance_variable_set(:@project, project) + end - describe 'GET #show' do - subject { get :show, params: { namespace_id: project.namespace, project_id: project } } + subject { get :show, params: { namespace_id: project.namespace, project_id: project.path } } context 'when repository has no avatar' do it 'shows 404' do @@ -37,6 +37,15 @@ RSpec.describe Projects::AvatarsController do expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq 'true' end + it 'sets appropriate caching headers' do + sign_in(project.owner) + subject + + expect(response.cache_control[:public]).to eq(true) + expect(response.cache_control[:max_age]).to eq(60) + expect(response.cache_control[:no_store]).to be_nil + end + it_behaves_like 'project cache control headers' end @@ -51,9 +60,16 @@ RSpec.describe Projects::AvatarsController do end describe 'DELETE #destroy' do + let(:project) { create(:project, :repository, avatar: fixture_file_upload("spec/fixtures/dk.png", "image/png")) } + + before do + sign_in(project.owner) + end + it 'removes avatar from DB by calling destroy' do - delete :destroy, params: { namespace_id: project.namespace.id, project_id: project.id } + delete :destroy, params: { namespace_id: project.namespace.path, project_id: project.path } + expect(response).to redirect_to(edit_project_path(project, anchor: 'js-general-project-settings')) expect(project.avatar.present?).to be_falsey expect(project).to be_valid end diff --git a/spec/controllers/projects/ci/lints_controller_spec.rb b/spec/controllers/projects/ci/lints_controller_spec.rb index 22f052e39b7..c4e040b0287 100644 --- a/spec/controllers/projects/ci/lints_controller_spec.rb +++ b/spec/controllers/projects/ci/lints_controller_spec.rb @@ -47,9 +47,9 @@ RSpec.describe Projects::Ci::LintsController do describe 'POST #create' do subject { post :create, params: params } - let(:format) { :html } - let(:params) { { namespace_id: project.namespace, project_id: project, content: content, format: format } } + let(:params) { { namespace_id: project.namespace, project_id: project, content: content, format: :json } } let(:remote_file_path) { 'https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.gitlab-ci-1.yml' } + let(:parsed_body) { Gitlab::Json.parse(response.body) } let(:remote_file_content) do <<~HEREDOC @@ -72,17 +72,23 @@ RSpec.describe Projects::Ci::LintsController do HEREDOC end - shared_examples 'successful request with format json' do - context 'with format json' do - let(:format) { :json } - let(:parsed_body) { Gitlab::Json.parse(response.body) } + shared_examples 'returns a successful validation' do + before do + subject + end - it 'renders json' do - expect(response).to have_gitlab_http_status :ok - expect(response.content_type).to eq 'application/json' - expect(parsed_body).to include('errors', 'warnings', 'jobs', 'valid') - expect(parsed_body).to match_schema('entities/lint_result_entity') - end + it 'returns successfully' do + expect(response).to have_gitlab_http_status :ok + end + + it 'renders json' do + expect(response.content_type).to eq 'application/json' + expect(parsed_body).to include('errors', 'warnings', 'jobs', 'valid') + expect(parsed_body).to match_schema('entities/lint_result_entity') + end + + it 'retrieves project' do + expect(assigns(:project)).to eq(project) end end @@ -92,25 +98,7 @@ RSpec.describe Projects::Ci::LintsController do project.add_developer(user) end - shared_examples 'returns a successful validation' do - before do - subject - end - - it 'returns successfully' do - expect(response).to have_gitlab_http_status :ok - end - - it 'renders show page' do - expect(response).to render_template :show - end - - it 'retrieves project' do - expect(assigns(:project)).to eq(project) - end - - it_behaves_like 'successful request with format json' - end + it_behaves_like 'returns a successful validation' context 'using legacy validation (YamlProcessor)' do it_behaves_like 'returns a successful validation' @@ -135,20 +123,6 @@ RSpec.describe Projects::Ci::LintsController do subject end - - context 'when dry_run feature flag is disabled' do - before do - stub_feature_flags(ci_lint_creates_pipeline_with_dry_run: false) - end - - it_behaves_like 'returns a successful validation' - - it 'runs validations through YamlProcessor' do - expect(Gitlab::Ci::YamlProcessor).to receive(:new).and_call_original - - subject - end - end end end @@ -166,27 +140,23 @@ RSpec.describe Projects::Ci::LintsController do subject end + it_behaves_like 'returns a successful validation' + it 'assigns result with errors' do - expect(assigns[:result].errors).to match_array([ + expect(parsed_body['errors']).to match_array([ 'jobs rubocop config should implement a script: or a trigger: keyword', 'jobs config should contain at least one visible job' ]) end - it 'render show page' do - expect(response).to render_template :show - end - - it_behaves_like 'successful request with format json' - context 'with dry_run mode' do subject { post :create, params: params.merge(dry_run: 'true') } it 'assigns result with errors' do - expect(assigns[:result].errors).to eq(['jobs rubocop config should implement a script: or a trigger: keyword']) + expect(parsed_body['errors']).to eq(['jobs rubocop config should implement a script: or a trigger: keyword']) end - it_behaves_like 'successful request with format json' + it_behaves_like 'returns a successful validation' end end @@ -200,14 +170,6 @@ RSpec.describe Projects::Ci::LintsController do it 'responds with 404' do expect(response).to have_gitlab_http_status(:not_found) end - - context 'with format json' do - let(:format) { :json } - - it 'responds with 404' do - expect(response).to have_gitlab_http_status :not_found - end - end end end end diff --git a/spec/controllers/projects/ci/pipeline_editor_controller_spec.rb b/spec/controllers/projects/ci/pipeline_editor_controller_spec.rb new file mode 100644 index 00000000000..1bf6ff95c44 --- /dev/null +++ b/spec/controllers/projects/ci/pipeline_editor_controller_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::Ci::PipelineEditorController do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + + before do + sign_in(user) + end + + describe 'GET #show' do + context 'with enough privileges' do + before do + project.add_developer(user) + + get :show, params: { namespace_id: project.namespace, project_id: project } + end + + it { expect(response).to have_gitlab_http_status(:ok) } + + it 'renders show page' do + expect(response).to render_template :show + end + end + + context 'without enough privileges' do + before do + project.add_reporter(user) + + get :show, params: { namespace_id: project.namespace, project_id: project } + end + + it 'responds with 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when ci_pipeline_editor_page feature flag is disabled' do + before do + stub_feature_flags(ci_pipeline_editor_page: false) + project.add_developer(user) + + get :show, params: { namespace_id: project.namespace, project_id: project } + end + + it 'responds with 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/controllers/projects/cycle_analytics/events_controller_spec.rb b/spec/controllers/projects/cycle_analytics/events_controller_spec.rb index c5b72ff2b3b..f940da7ea35 100644 --- a/spec/controllers/projects/cycle_analytics/events_controller_spec.rb +++ b/spec/controllers/projects/cycle_analytics/events_controller_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Projects::CycleAnalytics::EventsController do project.add_maintainer(user) end - describe 'cycle analytics not set up flag' do + describe 'value stream analytics not set up flag' do context 'with no data' do it 'is empty' do get_issue diff --git a/spec/controllers/projects/cycle_analytics_controller_spec.rb b/spec/controllers/projects/cycle_analytics_controller_spec.rb index e956065972f..24c2d568d9a 100644 --- a/spec/controllers/projects/cycle_analytics_controller_spec.rb +++ b/spec/controllers/projects/cycle_analytics_controller_spec.rb @@ -32,7 +32,7 @@ RSpec.describe Projects::CycleAnalyticsController do end end - describe 'cycle analytics not set up flag' do + describe 'value stream analytics not set up flag' do context 'with no data' do it 'is true' do get(:show, diff --git a/spec/controllers/projects/imports_controller_spec.rb b/spec/controllers/projects/imports_controller_spec.rb index 029b4210f19..5e09a50aa36 100644 --- a/spec/controllers/projects/imports_controller_spec.rb +++ b/spec/controllers/projects/imports_controller_spec.rb @@ -7,10 +7,21 @@ RSpec.describe Projects::ImportsController do let(:project) { create(:project) } before do - sign_in(user) + sign_in(user) if user end describe 'GET #show' do + context 'when user is not authenticated and the project is public' do + let(:user) { nil } + let(:project) { create(:project, :public) } + + it 'returns 404 response' do + get :show, params: { namespace_id: project.namespace.to_param, project_id: project } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + context 'when the user has maintainer rights' do before do project.add_maintainer(user) diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index f956baa0e22..26e1842468b 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -52,14 +52,14 @@ RSpec.describe Projects::IssuesController do get :index, params: { namespace_id: project.namespace, project_id: project } expect(response).to redirect_to(project_issues_path(new_project)) - expect(response).to have_gitlab_http_status(:found) + expect(response).to have_gitlab_http_status(:moved_permanently) end it 'redirects from an old issue correctly' do get :show, params: { namespace_id: project.namespace, project_id: project, id: issue } expect(response).to redirect_to(project_issue_path(new_project, issue)) - expect(response).to have_gitlab_http_status(:found) + expect(response).to have_gitlab_http_status(:moved_permanently) end end end @@ -1869,7 +1869,7 @@ RSpec.describe Projects::IssuesController do } expect(response).to redirect_to(designs_project_issue_path(new_project, issue)) - expect(response).to have_gitlab_http_status(:found) + expect(response).to have_gitlab_http_status(:moved_permanently) end end end diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 91770a00081..bda1f1a3b1c 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -383,6 +383,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do environment: nil, merge_request: merge_request, diff_view: :inline, + merge_ref_head_diff: nil, pagination_data: { current_page: nil, next_page: nil, @@ -454,6 +455,31 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like 'successful request' end + context 'with paths param' do + let(:example_file_path) { "README" } + let(:file_path_option) { { paths: [example_file_path] } } + + subject do + go(file_path_option) + end + + it_behaves_like 'serializes diffs with expected arguments' do + let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch } + let(:expected_options) do + collection_arguments(current_page: 1, total_pages: 1) + end + end + + it_behaves_like 'successful request' + + it 'filters down the response to the expected file path' do + subject + + expect(json_response["diff_files"].size).to eq(1) + expect(json_response["diff_files"].first["file_path"]).to eq(example_file_path) + end + end + context 'with default params' do subject { go } diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index ee194e5ff2f..f159f0e6099 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -6,14 +6,10 @@ RSpec.describe Projects::MergeRequestsController do include ProjectForksHelper include Gitlab::Routing - let(:project) { create(:project, :repository) } - let(:user) { project.owner } + let_it_be_with_refind(:project) { create(:project, :repository) } + let_it_be_with_reload(:project_public_with_private_builds) { create(:project, :repository, :public, :builds_private) } + let(:user) { project.owner } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } - let(:merge_request_with_conflicts) do - create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start', source_project: project, merge_status: :unchecked) do |mr| - mr.mark_as_unmergeable - end - end before do sign_in(user) @@ -99,7 +95,8 @@ RSpec.describe Projects::MergeRequestsController do project, merge_request, 'json', - diff_head: true)) + diff_head: true, + view: 'inline')) end end @@ -107,7 +104,7 @@ RSpec.describe Projects::MergeRequestsController do render_views it 'renders merge request page' do - merge_request.merge_request_diff.destroy + merge_request.merge_request_diff.destroy! go(format: :html) @@ -147,7 +144,7 @@ RSpec.describe Projects::MergeRequestsController do let(:new_project) { create(:project) } before do - project.route.destroy + project.route.destroy! new_project.redirect_routes.create!(path: project.full_path) new_project.add_developer(user) end @@ -161,7 +158,7 @@ RSpec.describe Projects::MergeRequestsController do } expect(response).to redirect_to(project_merge_request_path(new_project, merge_request)) - expect(response).to have_gitlab_http_status(:found) + expect(response).to have_gitlab_http_status(:moved_permanently) end it 'redirects from an old merge request commits correctly' do @@ -173,7 +170,7 @@ RSpec.describe Projects::MergeRequestsController do } expect(response).to redirect_to(commits_project_merge_request_path(new_project, merge_request)) - expect(response).to have_gitlab_http_status(:found) + expect(response).to have_gitlab_http_status(:moved_permanently) end end end @@ -359,12 +356,11 @@ RSpec.describe Projects::MergeRequestsController do end context 'there is no source project' do - let(:project) { create(:project, :repository) } let(:forked_project) { fork_project_with_submodules(project) } let!(:merge_request) { create(:merge_request, source_project: forked_project, source_branch: 'add-submodule-version-bump', target_branch: 'master', target_project: project) } before do - forked_project.destroy + forked_project.destroy! end it 'closes MR without errors' do @@ -435,7 +431,7 @@ RSpec.describe Projects::MergeRequestsController do context 'when the merge request is not mergeable' do before do - merge_request.update(title: "WIP: #{merge_request.title}") + merge_request.update!(title: "WIP: #{merge_request.title}") post :merge, params: base_params end @@ -475,7 +471,7 @@ RSpec.describe Projects::MergeRequestsController do context 'when squash is passed as 1' do it 'updates the squash attribute on the MR to true' do - merge_request.update(squash: false) + merge_request.update!(squash: false) merge_with_sha(squash: '1') expect(merge_request.reload.squash_on_merge?).to be_truthy @@ -484,7 +480,7 @@ RSpec.describe Projects::MergeRequestsController do context 'when squash is passed as 0' do it 'updates the squash attribute on the MR to false' do - merge_request.update(squash: true) + merge_request.update!(squash: true) merge_with_sha(squash: '0') expect(merge_request.reload.squash_on_merge?).to be_falsey @@ -547,7 +543,7 @@ RSpec.describe Projects::MergeRequestsController do context 'and head pipeline is not the current one' do before do - head_pipeline.update(sha: 'not_current_sha') + head_pipeline.update!(sha: 'not_current_sha') end it 'returns :failed' do @@ -667,9 +663,9 @@ RSpec.describe Projects::MergeRequestsController do end context "when the user is owner" do - let(:owner) { create(:user) } - let(:namespace) { create(:namespace, owner: owner) } - let(:project) { create(:project, :repository, namespace: namespace) } + let_it_be(:owner) { create(:user) } + let_it_be(:namespace) { create(:namespace, owner: owner) } + let_it_be(:project) { create(:project, :repository, namespace: namespace) } before do sign_in owner @@ -765,7 +761,7 @@ RSpec.describe Projects::MergeRequestsController do end context 'with private builds on a public project' do - let(:project) { create(:project, :repository, :public, :builds_private) } + let(:project) { project_public_with_private_builds } context 'for a project owner' do it 'responds with serialized pipelines' do @@ -813,7 +809,7 @@ RSpec.describe Projects::MergeRequestsController do context 'with public builds' do let(:forked_project) do fork_project(project, fork_user, repository: true).tap do |new_project| - new_project.project_feature.update(builds_access_level: ProjectFeature::ENABLED) + new_project.project_feature.update!(builds_access_level: ProjectFeature::ENABLED) end end @@ -855,7 +851,7 @@ RSpec.describe Projects::MergeRequestsController do end describe 'GET exposed_artifacts' do - let(:merge_request) do + let_it_be(:merge_request) do create(:merge_request, :with_merge_request_pipeline, target_project: project, @@ -993,7 +989,7 @@ RSpec.describe Projects::MergeRequestsController do end describe 'GET coverage_reports' do - let(:merge_request) do + let_it_be(:merge_request) do create(:merge_request, :with_merge_request_pipeline, target_project: project, @@ -1123,7 +1119,7 @@ RSpec.describe Projects::MergeRequestsController do end describe 'GET terraform_reports' do - let(:merge_request) do + let_it_be(:merge_request) do create(:merge_request, :with_merge_request_pipeline, target_project: project, @@ -1271,7 +1267,7 @@ RSpec.describe Projects::MergeRequestsController do end describe 'GET test_reports' do - let(:merge_request) do + let_it_be(:merge_request) do create(:merge_request, :with_diffs, :with_merge_request_pipeline, @@ -1382,7 +1378,7 @@ RSpec.describe Projects::MergeRequestsController do end describe 'GET accessibility_reports' do - let(:merge_request) do + let_it_be(:merge_request) do create(:merge_request, :with_diffs, :with_merge_request_pipeline, @@ -1419,7 +1415,7 @@ RSpec.describe Projects::MergeRequestsController do end context 'permissions on a public project with private CI/CD' do - let(:project) { create(:project, :repository, :public, :builds_private) } + let(:project) { project_public_with_private_builds } let(:accessibility_comparison) { { status: :parsed, data: { summary: 1 } } } context 'while signed out' do @@ -1505,7 +1501,7 @@ RSpec.describe Projects::MergeRequestsController do describe 'POST remove_wip' do before do merge_request.title = merge_request.wip_title - merge_request.save + merge_request.save! post :remove_wip, params: { @@ -1626,7 +1622,7 @@ RSpec.describe Projects::MergeRequestsController do it 'links to the environment on that project', :sidekiq_might_not_need_inline do get_ci_environments_status - expect(json_response.first['url']).to match /#{forked.full_path}/ + expect(json_response.first['url']).to match(/#{forked.full_path}/) end context "when environment_target is 'merge_commit'", :sidekiq_might_not_need_inline do @@ -1653,7 +1649,7 @@ RSpec.describe Projects::MergeRequestsController do get_ci_environments_status(environment_target: 'merge_commit') expect(response).to have_gitlab_http_status(:ok) - expect(json_response.first['url']).to match /#{project.full_path}/ + expect(json_response.first['url']).to match(/#{project.full_path}/) end end end @@ -1773,7 +1769,7 @@ RSpec.describe Projects::MergeRequestsController do context 'with project member visibility on a public project' do let(:user) { create(:user) } - let(:project) { create(:project, :repository, :public, :builds_private) } + let(:project) { project_public_with_private_builds } it 'returns pipeline data to project members' do project.add_developer(user) @@ -1999,4 +1995,21 @@ RSpec.describe Projects::MergeRequestsController do expect(assigns(:noteable)).not_to be_nil end end + + describe 'POST export_csv' do + subject { post :export_csv, params: { namespace_id: project.namespace, project_id: project } } + + it 'redirects to the merge request index' do + subject + + expect(response).to redirect_to(project_merge_requests_path(project)) + expect(response.flash[:notice]).to match(/\AYour CSV export has started/i) + end + + it 'enqueues an IssuableExportCsvWorker worker' do + expect(IssuableExportCsvWorker).to receive(:perform_async).with(:merge_request, user.id, project.id, anything) + + subject + end + end end diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 8c59b2b378f..d76432f71b3 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -113,6 +113,8 @@ RSpec.describe Projects::NotesController do end it 'returns the first page of notes' do + expect(Gitlab::EtagCaching::Middleware).to receive(:skip!) + get :index, params: request_params expect(json_response['notes'].count).to eq(page_1.count) @@ -122,6 +124,8 @@ RSpec.describe Projects::NotesController do end it 'returns the second page of notes' do + expect(Gitlab::EtagCaching::Middleware).to receive(:skip!) + request.headers['X-Last-Fetched-At'] = page_1_boundary get :index, params: request_params @@ -133,6 +137,8 @@ RSpec.describe Projects::NotesController do end it 'returns the final page of notes' do + expect(Gitlab::EtagCaching::Middleware).to receive(:skip!) + request.headers['X-Last-Fetched-At'] = page_2_boundary get :index, params: request_params @@ -142,6 +148,19 @@ RSpec.describe Projects::NotesController do expect(json_response['last_fetched_at']).to eq(microseconds(Time.zone.now)) expect(response.headers['Poll-Interval'].to_i).to be > 1 end + + it 'returns an empty page of notes' do + expect(Gitlab::EtagCaching::Middleware).not_to receive(:skip!) + + request.headers['X-Last-Fetched-At'] = microseconds(Time.zone.now) + + get :index, params: request_params + + expect(json_response['notes']).to be_empty + expect(json_response['more']).to be_falsy + expect(json_response['last_fetched_at']).to eq(microseconds(Time.zone.now)) + expect(response.headers['Poll-Interval'].to_i).to be > 1 + end end context 'feature flag disabled' do diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index b3921164c81..43cf1a16051 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -33,11 +33,6 @@ RSpec.describe Projects::RawController do it_behaves_like 'project cache control headers' it_behaves_like 'content disposition headers' - it_behaves_like 'uncached response' do - before do - subject - end - end end context 'image header' do @@ -225,6 +220,32 @@ RSpec.describe Projects::RawController do end end end + + describe 'caching' do + def request_file + get(:show, params: { namespace_id: project.namespace, project_id: project, id: 'master/README.md' }) + end + + it 'sets appropriate caching headers' do + sign_in create(:user) + request_file + + expect(response.cache_control[:public]).to eq(true) + expect(response.cache_control[:max_age]).to eq(60) + expect(response.cache_control[:no_store]).to be_nil + end + + context 'when If-None-Match header is set' do + it 'returns a 304 status' do + request_file + + request.headers['If-None-Match'] = response.headers['ETag'] + request_file + + expect(response).to have_gitlab_http_status(:not_modified) + end + end + end end def execute_raw_requests(requests:, project:, file_path:, **params) diff --git a/spec/controllers/projects/registry/repositories_controller_spec.rb b/spec/controllers/projects/registry/repositories_controller_spec.rb index 098fa9bac2c..9b803edd463 100644 --- a/spec/controllers/projects/registry/repositories_controller_spec.rb +++ b/spec/controllers/projects/registry/repositories_controller_spec.rb @@ -50,18 +50,17 @@ RSpec.describe Projects::Registry::RepositoriesController do tags: %w[rc1 latest]) end - it 'successfully renders container repositories' do - expect(Gitlab::Tracking).not_to receive(:event) - + it 'successfully renders container repositories', :snowplow do go_to_index + expect_no_snowplow_event expect(response).to have_gitlab_http_status(:ok) end - it 'tracks the event' do - expect(Gitlab::Tracking).to receive(:event).with(anything, 'list_repositories', {}) - + it 'tracks the event', :snowplow do go_to_index(format: :json) + + expect_snowplow_event(category: anything, action: 'list_repositories') end it 'creates a root container repository' do @@ -132,11 +131,12 @@ RSpec.describe Projects::Registry::RepositoriesController do expect(response).to have_gitlab_http_status(:no_content) end - it 'tracks the event' do - expect(Gitlab::Tracking).to receive(:event).with(anything, 'delete_repository', {}) + it 'tracks the event', :snowplow do allow(DeleteContainerRepositoryWorker).to receive(:perform_async).with(user.id, repository.id) delete_repository(repository) + + expect_snowplow_event(category: anything, action: 'delete_repository') end end end diff --git a/spec/controllers/projects/registry/tags_controller_spec.rb b/spec/controllers/projects/registry/tags_controller_spec.rb index 59df9e78a3c..5bff89b4308 100644 --- a/spec/controllers/projects/registry/tags_controller_spec.rb +++ b/spec/controllers/projects/registry/tags_controller_spec.rb @@ -39,10 +39,10 @@ RSpec.describe Projects::Registry::TagsController do expect(response).to include_pagination_headers end - it 'tracks the event' do - expect(Gitlab::Tracking).to receive(:event).with(anything, 'list_tags', {}) - + it 'tracks the event', :snowplow do get_tags + + expect_snowplow_event(category: anything, action: 'list_tags') end end @@ -109,7 +109,7 @@ RSpec.describe Projects::Registry::TagsController do it 'tracks the event' do expect_delete_tags(%w[test.]) - expect(controller).to receive(:track_event).with(:delete_tag, {}) + expect(controller).to receive(:track_event).with(:delete_tag) destroy_tag('test.') end @@ -148,11 +148,11 @@ RSpec.describe Projects::Registry::TagsController do bulk_destroy_tags(tags) end - it 'tracks the event' do + it 'tracks the event', :snowplow do expect_delete_tags(tags) - expect(Gitlab::Tracking).to receive(:event).with(anything, 'delete_tag_bulk', {}) - bulk_destroy_tags(tags) + + expect_snowplow_event(category: anything, action: 'delete_tag_bulk') end end end diff --git a/spec/controllers/projects/releases_controller_spec.rb b/spec/controllers/projects/releases_controller_spec.rb index 420d818daeb..07fb03b39c6 100644 --- a/spec/controllers/projects/releases_controller_spec.rb +++ b/spec/controllers/projects/releases_controller_spec.rb @@ -201,102 +201,7 @@ RSpec.describe Projects::ReleasesController do end end - context 'GET #downloads' do - subject do - get :downloads, params: { namespace_id: project.namespace, project_id: project, tag: tag, filepath: filepath } - end - - before do - sign_in(user) - end - - let(:release) { create(:release, project: project, tag: tag ) } - let!(:link) { create(:release_link, release: release, name: 'linux-amd64 binaries', filepath: '/binaries/linux-amd64', url: 'https://downloads.example.com/bin/gitlab-linux-amd64') } - let(:tag) { 'v11.9.0-rc2' } - - context 'valid filepath' do - let(:filepath) { CGI.escape('/binaries/linux-amd64') } - - it 'redirects to the asset direct link' do - subject - - expect(response).to redirect_to('https://downloads.example.com/bin/gitlab-linux-amd64') - end - - it 'redirects with a status of 302' do - subject - - expect(response).to have_gitlab_http_status(:redirect) - end - end - - context 'invalid filepath' do - let(:filepath) { CGI.escape('/binaries/win32') } - - it 'is not found' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - context 'GET #downloads' do - subject do - get :downloads, params: { - namespace_id: project.namespace, - project_id: project, - tag: tag, - filepath: filepath - } - end - - before do - sign_in(user) - end - - let(:release) { create(:release, project: project, tag: tag ) } - let(:tag) { 'v11.9.0-rc2' } - let(:db_filepath) { '/binaries/linux-amd64' } - let!(:link) do - create :release_link, - release: release, - name: 'linux-amd64 binaries', - filepath: db_filepath, - url: 'https://downloads.example.com/bin/gitlab-linux-amd64' - end - - context 'valid filepath' do - let(:filepath) { CGI.escape('/binaries/linux-amd64') } - - it 'redirects to the asset direct link' do - subject - - expect(response).to redirect_to(link.url) - end - end - - context 'invalid filepath' do - let(:filepath) { CGI.escape('/binaries/win32') } - - it 'is not found' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'ignores filepath extension' do - let(:db_filepath) { '/binaries/linux-amd64.json' } - let(:filepath) { CGI.escape(db_filepath) } - - it 'redirects to the asset direct link' do - subject - - expect(response).to redirect_to(link.url) - end - end - end + # `GET #downloads` is addressed in spec/requests/projects/releases_controller_spec.rb private diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index 97eea7c7e9d..e7f4a8a1422 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -122,7 +122,9 @@ RSpec.describe Projects::RepositoriesController do expect(response).to have_gitlab_http_status(:ok) expect(response.header['ETag']).to be_present - expect(response.header['Cache-Control']).to include('max-age=60, private') + expect(response.cache_control[:public]).to eq(false) + expect(response.cache_control[:max_age]).to eq(60) + expect(response.cache_control[:no_store]).to be_nil end context 'when project is public' do diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index 9fc9da1265e..46f69eaf96a 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/spec/controllers/projects/settings/operations_controller_spec.rb @@ -166,23 +166,22 @@ RSpec.describe Projects::Settings::OperationsController do context 'updating each incident management setting' do let(:new_incident_management_settings) { {} } - shared_examples 'a gitlab tracking event' do |params, event_key| - it "creates a gitlab tracking event #{event_key}" do + shared_examples 'a gitlab tracking event' do |params, event_key, **args| + it "creates a gitlab tracking event #{event_key}", :snowplow do new_incident_management_settings = params - expect(Gitlab::Tracking).to receive(:event) - .with('IncidentManagement::Settings', event_key, any_args) - patch :update, params: project_params(project, incident_management_setting_attributes: new_incident_management_settings) project.reload + + expect_snowplow_event(category: 'IncidentManagement::Settings', action: event_key, **args) end end it_behaves_like 'a gitlab tracking event', { create_issue: '1' }, 'enabled_issue_auto_creation_on_alerts' it_behaves_like 'a gitlab tracking event', { create_issue: '0' }, 'disabled_issue_auto_creation_on_alerts' - it_behaves_like 'a gitlab tracking event', { issue_template_key: 'template' }, 'enabled_issue_template_on_alerts' - it_behaves_like 'a gitlab tracking event', { issue_template_key: nil }, 'disabled_issue_template_on_alerts' + it_behaves_like 'a gitlab tracking event', { issue_template_key: 'template' }, 'enabled_issue_template_on_alerts', label: "Template name", property: "template" + it_behaves_like 'a gitlab tracking event', { issue_template_key: nil }, 'disabled_issue_template_on_alerts', label: "Template name", property: "" it_behaves_like 'a gitlab tracking event', { send_email: '1' }, 'enabled_sending_emails' it_behaves_like 'a gitlab tracking event', { send_email: '0' }, 'disabled_sending_emails' it_behaves_like 'a gitlab tracking event', { pagerduty_active: '1' }, 'enabled_pagerduty_webhook' diff --git a/spec/controllers/projects/settings/repository_controller_spec.rb b/spec/controllers/projects/settings/repository_controller_spec.rb index d93f23ae142..394f1ff28f2 100644 --- a/spec/controllers/projects/settings/repository_controller_spec.rb +++ b/spec/controllers/projects/settings/repository_controller_spec.rb @@ -23,13 +23,15 @@ RSpec.describe Projects::Settings::RepositoryController do describe 'PUT cleanup' do let(:object_map) { fixture_file_upload('spec/fixtures/bfg_object_map.txt') } - it 'enqueues a RepositoryCleanupWorker' do - allow(RepositoryCleanupWorker).to receive(:perform_async) + it 'enqueues a project cleanup' do + expect(Projects::CleanupService) + .to receive(:enqueue) + .with(project, user, anything) + .and_return(status: :success) - put :cleanup, params: { namespace_id: project.namespace, project_id: project, project: { object_map: object_map } } + put :cleanup, params: { namespace_id: project.namespace, project_id: project, project: { bfg_object_map: object_map } } expect(response).to redirect_to project_settings_repository_path(project) - expect(RepositoryCleanupWorker).to have_received(:perform_async).once end end diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb index 6b394fab14c..f9221c5a4ef 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -180,16 +180,6 @@ RSpec.describe Projects::SnippetsController do end end - describe 'GET #show as JSON' do - it 'renders the blob from the repository' do - project_snippet = create(:project_snippet, :public, :repository, project: project, author: user) - - get :show, params: { namespace_id: project.namespace, project_id: project, id: project_snippet.to_param }, format: :json - - expect(assigns(:blob)).to eq(project_snippet.blobs.first) - end - end - describe "GET #show for embeddable content" do let(:project_snippet) { create(:project_snippet, :repository, snippet_permission, project: project, author: user) } let(:extra_params) { {} } diff --git a/spec/controllers/projects/static_site_editor_controller_spec.rb b/spec/controllers/projects/static_site_editor_controller_spec.rb index 6ea730cbf27..867b2b51039 100644 --- a/spec/controllers/projects/static_site_editor_controller_spec.rb +++ b/spec/controllers/projects/static_site_editor_controller_spec.rb @@ -105,7 +105,8 @@ RSpec.describe Projects::StaticSiteEditorController do foo: 'bar' } }, - a_boolean: true + a_boolean: true, + a_nil: nil } end @@ -130,6 +131,10 @@ RSpec.describe Projects::StaticSiteEditorController do it 'serializes data values which are hashes to JSON' do expect(assigns_data[:a_hash]).to eq('{"a_deeper_hash":{"foo":"bar"}}') end + + it 'serializes data values which are nil to an empty string' do + expect(assigns_data[:a_nil]).to eq('') + end end end end diff --git a/spec/controllers/projects/tags_controller_spec.rb b/spec/controllers/projects/tags_controller_spec.rb index 57760088183..efb57494f82 100644 --- a/spec/controllers/projects/tags_controller_spec.rb +++ b/spec/controllers/projects/tags_controller_spec.rb @@ -9,18 +9,75 @@ RSpec.describe Projects::TagsController do let(:user) { create(:user) } describe 'GET index' do - before do - get :index, params: { namespace_id: project.namespace.to_param, project_id: project } - end + subject { get :index, params: { namespace_id: project.namespace.to_param, project_id: project } } it 'returns the tags for the page' do + subject + expect(assigns(:tags).map(&:name)).to include('v1.1.0', 'v1.0.0') end it 'returns releases matching those tags' do + subject + expect(assigns(:releases)).to include(release) expect(assigns(:releases)).not_to include(invalid_release) end + + context '@tag_pipeline_status' do + context 'when no pipelines exist' do + it 'is empty' do + subject + + expect(assigns(:tag_pipeline_statuses)).to be_empty + end + end + + context 'when multiple tags exist' do + before do + create(:ci_pipeline, + project: project, + ref: 'v1.1.0', + sha: project.commit('v1.1.0').sha, + status: :running) + create(:ci_pipeline, + project: project, + ref: 'v1.0.0', + sha: project.commit('v1.0.0').sha, + status: :success) + end + + it 'all relevant commit statuses are received' do + subject + + expect(assigns(:tag_pipeline_statuses)['v1.1.0'].group).to eq("running") + expect(assigns(:tag_pipeline_statuses)['v1.0.0'].group).to eq("success") + end + end + + context 'when a tag has multiple pipelines' do + before do + create(:ci_pipeline, + project: project, + ref: 'v1.0.0', + sha: project.commit('v1.0.0').sha, + status: :running, + created_at: 6.months.ago) + create(:ci_pipeline, + project: project, + ref: 'v1.0.0', + sha: project.commit('v1.0.0').sha, + status: :success, + created_at: 2.months.ago) + end + + it 'chooses the latest to determine status' do + subject + + expect(assigns(:tag_pipeline_statuses)['v1.0.0'].group).to eq("success") + end + end + end end describe 'GET show' do @@ -70,7 +127,8 @@ RSpec.describe Projects::TagsController do end let(:release_description) { nil } - let(:request) do + + subject(:request) do post(:create, params: { namespace_id: project.namespace.to_param, project_id: project, @@ -81,7 +139,7 @@ RSpec.describe Projects::TagsController do end it 'creates tag' do - request + subject expect(response).to have_gitlab_http_status(:found) expect(project.repository.find_tag('1.0')).to be_present @@ -92,7 +150,7 @@ RSpec.describe Projects::TagsController do let(:release_description) { 'some release description' } it 'creates tag and release' do - request + subject expect(response).to have_gitlab_http_status(:found) expect(project.repository.find_tag('1.0')).to be_present @@ -118,7 +176,7 @@ RSpec.describe Projects::TagsController do expect(service).to receive(:execute).and_call_original end - request + subject aggregate_failures do expect(response).to have_gitlab_http_status(:found) diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb index 40632e0dea7..01593f4133c 100644 --- a/spec/controllers/projects/templates_controller_spec.rb +++ b/spec/controllers/projects/templates_controller_spec.rb @@ -5,35 +5,84 @@ require 'spec_helper' RSpec.describe Projects::TemplatesController do let(:project) { create(:project, :repository, :private) } let(:user) { create(:user) } - let(:file_path_1) { '.gitlab/issue_templates/issue_template.md' } - let(:file_path_2) { '.gitlab/merge_request_templates/merge_request_template.md' } - let!(:file_1) { project.repository.create_file(user, file_path_1, 'issue content', message: 'message', branch_name: 'master') } - let!(:file_2) { project.repository.create_file(user, file_path_2, 'merge request content', message: 'message', branch_name: 'master') } + let(:issue_template_path_1) { '.gitlab/issue_templates/issue_template_1.md' } + let(:issue_template_path_2) { '.gitlab/issue_templates/issue_template_2.md' } + let(:merge_request_template_path_1) { '.gitlab/merge_request_templates/merge_request_template_1.md' } + let(:merge_request_template_path_2) { '.gitlab/merge_request_templates/merge_request_template_2.md' } + let!(:issue_template_file_1) { project.repository.create_file(user, issue_template_path_1, 'issue content 1', message: 'message 1', branch_name: 'master') } + let!(:issue_template_file_2) { project.repository.create_file(user, issue_template_path_2, 'issue content 2', message: 'message 2', branch_name: 'master') } + let!(:merge_request_template_file_1) { project.repository.create_file(user, merge_request_template_path_1, 'merge request content 1', message: 'message 1', branch_name: 'master') } + let!(:merge_request_template_file_2) { project.repository.create_file(user, merge_request_template_path_2, 'merge request content 2', message: 'message 2', branch_name: 'master') } + let(:expected_issue_template_1) { { 'key' => 'issue_template_1', 'name' => 'issue_template_1', 'content' => 'issue content 1' } } + let(:expected_issue_template_2) { { 'key' => 'issue_template_2', 'name' => 'issue_template_2', 'content' => 'issue content 2' } } + let(:expected_merge_request_template_1) { { 'key' => 'merge_request_template_1', 'name' => 'merge_request_template_1', 'content' => 'merge request content 1' } } + let(:expected_merge_request_template_2) { { 'key' => 'merge_request_template_2', 'name' => 'merge_request_template_2', 'content' => 'merge request content 2' } } + + describe '#index' do + before do + project.add_developer(user) + sign_in(user) + end + + shared_examples 'templates request' do + it 'returns the templates' do + get(:index, params: { namespace_id: project.namespace, template_type: template_type, project_id: project }, format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to match(expected_templates) + end + + it 'fails for user with no access' do + other_user = create(:user) + sign_in(other_user) + + get(:index, params: { namespace_id: project.namespace, template_type: template_type, project_id: project }, format: :json) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when querying for issue templates' do + it_behaves_like 'templates request' do + let(:template_type) { 'issue' } + let(:expected_templates) { [expected_issue_template_1, expected_issue_template_2] } + end + end + + context 'when querying for merge_request templates' do + it_behaves_like 'templates request' do + let(:template_type) { 'merge_request' } + let(:expected_templates) { [expected_merge_request_template_1, expected_merge_request_template_2] } + end + end + end describe '#show' do shared_examples 'renders issue templates as json' do + let(:expected_issue_template) { expected_issue_template_2 } + it do - get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json) + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template_2', project_id: project }, format: :json) expect(response).to have_gitlab_http_status(:ok) - expect(json_response['name']).to eq('issue_template') - expect(json_response['content']).to eq('issue content') + expect(json_response).to match(expected_issue_template) end end shared_examples 'renders merge request templates as json' do + let(:expected_merge_request_template) { expected_merge_request_template_2 } + it do - get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json) + get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template_2', project_id: project }, format: :json) expect(response).to have_gitlab_http_status(:ok) - expect(json_response['name']).to eq('merge_request_template') - expect(json_response['content']).to eq('merge request content') + expect(json_response).to match(expected_merge_request_template) end end shared_examples 'renders 404 when requesting an issue template' do it do - get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json) + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template_1', project_id: project }, format: :json) expect(response).to have_gitlab_http_status(:not_found) end @@ -41,21 +90,23 @@ RSpec.describe Projects::TemplatesController do shared_examples 'renders 404 when requesting a merge request template' do it do - get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json) + get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template_1', project_id: project }, format: :json) expect(response).to have_gitlab_http_status(:not_found) end end - shared_examples 'renders 404 when params are invalid' do + shared_examples 'raises error when template type is invalid' do it 'does not route when the template type is invalid' do expect do - get(:show, params: { namespace_id: project.namespace, template_type: 'invalid_type', key: 'issue_template', project_id: project }, format: :json) + get(:show, params: { namespace_id: project.namespace, template_type: 'invalid_type', key: 'issue_template_1', project_id: project }, format: :json) end.to raise_error(ActionController::UrlGenerationError) end + end + shared_examples 'renders 404 when params are invalid' do it 'renders 404 when the format type is invalid' do - get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :html) + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template_1', project_id: project }, format: :html) expect(response).to have_gitlab_http_status(:not_found) end @@ -74,7 +125,6 @@ RSpec.describe Projects::TemplatesController do include_examples 'renders 404 when requesting an issue template' include_examples 'renders 404 when requesting a merge request template' - include_examples 'renders 404 when params are invalid' end context 'when user is a member of the project' do @@ -85,7 +135,11 @@ RSpec.describe Projects::TemplatesController do include_examples 'renders issue templates as json' include_examples 'renders merge request templates as json' - include_examples 'renders 404 when params are invalid' + + context 'when params are invalid' do + include_examples 'raises error when template type is invalid' + include_examples 'renders 404 when params are invalid' + end end context 'when user is a guest of the project' do @@ -96,7 +150,6 @@ RSpec.describe Projects::TemplatesController do include_examples 'renders issue templates as json' include_examples 'renders 404 when requesting a merge request template' - include_examples 'renders 404 when params are invalid' end end @@ -111,8 +164,8 @@ RSpec.describe Projects::TemplatesController do get(:names, params: { namespace_id: project.namespace, template_type: template_type, project_id: project }, format: :json) expect(response).to have_gitlab_http_status(:ok) - expect(json_response.size).to eq(1) - expect(json_response[0]['name']).to eq(expected_template_name) + expect(json_response.size).to eq(2) + expect(json_response).to match(expected_template_names) end it 'fails for user with no access' do @@ -128,14 +181,14 @@ RSpec.describe Projects::TemplatesController do context 'when querying for issue templates' do it_behaves_like 'template names request' do let(:template_type) { 'issue' } - let(:expected_template_name) { 'issue_template' } + let(:expected_template_names) { [{ 'name' => 'issue_template_1' }, { 'name' => 'issue_template_2' }] } end end context 'when querying for merge_request templates' do it_behaves_like 'template names request' do let(:template_type) { 'merge_request' } - let(:expected_template_name) { 'merge_request_template' } + let(:expected_template_names) { [{ 'name' => 'merge_request_template_1' }, { 'name' => 'merge_request_template_2' }] } end end end diff --git a/spec/controllers/projects/terraform_controller_spec.rb b/spec/controllers/projects/terraform_controller_spec.rb new file mode 100644 index 00000000000..1978b9494fa --- /dev/null +++ b/spec/controllers/projects/terraform_controller_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::TerraformController do + let_it_be(:project) { create(:project) } + + describe 'GET index' do + subject { get :index, params: { namespace_id: project.namespace, project_id: project } } + + context 'when user is authorized' do + let(:user) { project.creator } + + before do + sign_in(user) + subject + end + + it 'renders content' do + expect(response).to be_successful + end + end + + context 'when user is unauthorized' do + let(:user) { create(:user) } + + before do + project.add_guest(user) + sign_in(user) + subject + end + + it 'shows 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 0640f9e5724..012a98f433e 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -1437,4 +1437,55 @@ RSpec.describe ProjectsController do def project_moved_message(redirect_route, project) "Project '#{redirect_route.path}' was moved to '#{project.full_path}'. Please update any links and bookmarks that may still have the old path." end + + describe 'GET #unfoldered_environment_names' do + it 'shows the environment names of a public project to an anonymous user' do + create(:environment, project: public_project, name: 'foo') + + get( + :unfoldered_environment_names, + params: { namespace_id: public_project.namespace, id: public_project, format: :json } + ) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq(%w[foo]) + end + + it 'does not show environment names of a private project to anonymous users' do + create(:environment, project: project, name: 'foo') + + get( + :unfoldered_environment_names, + params: { namespace_id: project.namespace, id: project, format: :json } + ) + + expect(response).to redirect_to(new_user_session_path) + end + + it 'shows environment names of a private project to a project member' do + create(:environment, project: project, name: 'foo') + project.add_developer(user) + sign_in(user) + + get( + :unfoldered_environment_names, + params: { namespace_id: project.namespace, id: project, format: :json } + ) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq(%w[foo]) + end + + it 'does not show environment names of a private project to a logged-in non-member' do + create(:environment, project: project, name: 'foo') + sign_in(user) + + get( + :unfoldered_environment_names, + params: { namespace_id: project.namespace, id: project, format: :json } + ) + + expect(response).to have_gitlab_http_status(:not_found) + end + end end diff --git a/spec/controllers/registrations/welcome_controller_spec.rb b/spec/controllers/registrations/welcome_controller_spec.rb new file mode 100644 index 00000000000..d32c936b8c9 --- /dev/null +++ b/spec/controllers/registrations/welcome_controller_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Registrations::WelcomeController do + let(:user) { create(:user) } + + describe '#welcome' do + subject(:show) { get :show } + + context 'without a signed in user' do + it { is_expected.to redirect_to new_user_registration_path } + end + + context 'when role or setup_for_company is not set' do + before do + sign_in(user) + end + + it { is_expected.to render_template(:show) } + end + + context 'when role is required and setup_for_company is not set' do + before do + user.set_role_required! + sign_in(user) + end + + it { is_expected.to render_template(:show) } + end + + context 'when role and setup_for_company is set' do + before do + user.update!(setup_for_company: false) + sign_in(user) + end + + it { is_expected.to redirect_to(dashboard_projects_path)} + end + + context 'when role is set and setup_for_company is not set' do + before do + user.update!(role: :software_developer) + sign_in(user) + end + + it { is_expected.to render_template(:show) } + end + + context '2FA is required from group' do + before do + user = create(:user, require_two_factor_authentication_from_group: true) + sign_in(user) + end + + it 'does not perform a redirect' do + expect(subject).not_to redirect_to(profile_two_factor_auth_path) + end + end + end + + describe '#update' do + subject(:update) do + patch :update, params: { user: { role: 'software_developer', setup_for_company: 'false' } } + end + + context 'without a signed in user' do + it { is_expected.to redirect_to new_user_registration_path } + end + + context 'with a signed in user' do + before do + sign_in(user) + end + + it { is_expected.to redirect_to(dashboard_projects_path)} + end + end +end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 501d8d4a78d..2fb17e56f37 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -7,35 +7,16 @@ RSpec.describe RegistrationsController do before do stub_feature_flags(invisible_captcha: false) + stub_application_setting(require_admin_approval_after_user_signup: false) end describe '#new' do subject { get :new } - context 'with the experimental signup flow enabled and the user is part of the experimental group' do - before do - stub_experiment(signup_flow: true) - stub_experiment_for_user(signup_flow: true) - end - - it 'renders new template and sets the resource variable' do - expect(subject).to render_template(:new) - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:resource)).to be_a(User) - end - end - - context 'with the experimental signup flow enabled and the user is part of the control group' do - before do - stub_experiment(signup_flow: true) - stub_experiment_for_user(signup_flow: false) - end - - it 'renders new template and sets the resource variable' do - subject - expect(response).to have_gitlab_http_status(:found) - expect(response).to redirect_to(new_user_session_path(anchor: 'register-pane')) - end + it 'renders new template and sets the resource variable' do + expect(subject).to render_template(:new) + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:resource)).to be_a(User) end end @@ -46,102 +27,86 @@ RSpec.describe RegistrationsController do subject { post(:create, params: user_params) } context '`blocked_pending_approval` state' do - context 'when the feature is enabled' do + context 'when the `require_admin_approval_after_user_signup` setting is turned on' do before do - stub_feature_flags(admin_approval_for_new_user_signups: true) + stub_application_setting(require_admin_approval_after_user_signup: true) end - context 'when the `require_admin_approval_after_user_signup` setting is turned on' do - before do - stub_application_setting(require_admin_approval_after_user_signup: true) - end + it 'signs up the user in `blocked_pending_approval` state' do + subject + created_user = User.find_by(email: 'new@user.com') - it 'signs up the user in `blocked_pending_approval` state' do - subject - created_user = User.find_by(email: 'new@user.com') + expect(created_user).to be_present + expect(created_user.blocked_pending_approval?).to eq(true) + end - expect(created_user).to be_present - expect(created_user.blocked_pending_approval?).to eq(true) - end + it 'does not log in the user after sign up' do + subject - it 'does not log in the user after sign up' do - subject + expect(controller.current_user).to be_nil + end - expect(controller.current_user).to be_nil - end + it 'shows flash message after signing up' do + subject - it 'shows flash message after signing up' do - subject + expect(response).to redirect_to(new_user_session_path(anchor: 'login-pane')) + expect(flash[:notice]) + .to eq('You have signed up successfully. However, we could not sign you in because your account is awaiting approval from your GitLab administrator.') + end - expect(response).to redirect_to(new_user_session_path(anchor: 'login-pane')) - expect(flash[:notice]) - .to eq('You have signed up successfully. However, we could not sign you in because your account is awaiting approval from your GitLab administrator.') + it 'emails the access request to approvers' do + expect_next_instance_of(NotificationService) do |notification| + allow(notification).to receive(:new_instance_access_request).with(User.find_by(email: 'new@user.com')) end - context 'email confirmation' do - context 'when `send_user_confirmation_email` is true' do - before do - stub_application_setting(send_user_confirmation_email: true) - end - - it 'does not send a confirmation email' do - expect { subject } - .not_to have_enqueued_mail(DeviseMailer, :confirmation_instructions) - end - end - end + subject end - context 'when the `require_admin_approval_after_user_signup` setting is turned off' do - before do - stub_application_setting(require_admin_approval_after_user_signup: false) - end - - it 'signs up the user in `active` state' do - subject - created_user = User.find_by(email: 'new@user.com') + context 'email confirmation' do + context 'when `send_user_confirmation_email` is true' do + before do + stub_application_setting(send_user_confirmation_email: true) + end - expect(created_user).to be_present - expect(created_user.active?).to eq(true) + it 'does not send a confirmation email' do + expect { subject } + .not_to have_enqueued_mail(DeviseMailer, :confirmation_instructions) + end end + end + end - it 'does not show any flash message after signing up' do - subject + context 'when the `require_admin_approval_after_user_signup` setting is turned off' do + it 'signs up the user in `active` state' do + subject + created_user = User.find_by(email: 'new@user.com') - expect(flash[:notice]).to be_nil - end + expect(created_user).to be_present + expect(created_user.active?).to eq(true) + end - context 'email confirmation' do - context 'when `send_user_confirmation_email` is true' do - before do - stub_application_setting(send_user_confirmation_email: true) - end + it 'does not show any flash message after signing up' do + subject - it 'sends a confirmation email' do - expect { subject } - .to have_enqueued_mail(DeviseMailer, :confirmation_instructions) - end - end - end + expect(flash[:notice]).to be_nil end - end - context 'when the feature is disabled' do - before do - stub_feature_flags(admin_approval_for_new_user_signups: false) - end + it 'does not email the approvers' do + expect(NotificationService).not_to receive(:new) - context 'when the `require_admin_approval_after_user_signup` setting is turned on' do - before do - stub_application_setting(require_admin_approval_after_user_signup: true) - end + subject + end - it 'signs up the user in `active` state' do - subject + context 'email confirmation' do + context 'when `send_user_confirmation_email` is true' do + before do + stub_application_setting(send_user_confirmation_email: true) + end - created_user = User.find_by(email: 'new@user.com') - expect(created_user).to be_present - expect(created_user.active?).to eq(true) + it 'sends a confirmation email' do + expect { subject } + .to have_enqueued_mail(DeviseMailer, :confirmation_instructions) + end end end end @@ -448,45 +413,4 @@ RSpec.describe RegistrationsController do end end end - - describe '#welcome' do - subject { get :welcome } - - it 'renders the devise_experimental_separate_sign_up_flow layout' do - sign_in(create(:user)) - - expected_layout = Gitlab.ee? ? :checkout : :devise_experimental_separate_sign_up_flow - - expect(subject).to render_template(expected_layout) - end - - context '2FA is required from group' do - before do - user = create(:user, require_two_factor_authentication_from_group: true) - sign_in(user) - end - - it 'does not perform a redirect' do - expect(subject).not_to redirect_to(profile_two_factor_auth_path) - end - end - end - - describe '#update_registration' do - subject(:update_registration) do - patch :update_registration, params: { user: { role: 'software_developer', setup_for_company: 'false' } } - end - - context 'without a signed in user' do - it { is_expected.to redirect_to new_user_registration_path } - end - - context 'with a signed in user' do - before do - sign_in(create(:user)) - end - - it { is_expected.to redirect_to(dashboard_projects_path)} - end - end end diff --git a/spec/controllers/repositories/lfs_storage_controller_spec.rb b/spec/controllers/repositories/lfs_storage_controller_spec.rb index 0201e73728f..4f9d049cf87 100644 --- a/spec/controllers/repositories/lfs_storage_controller_spec.rb +++ b/spec/controllers/repositories/lfs_storage_controller_spec.rb @@ -127,6 +127,41 @@ RSpec.describe Repositories::LfsStorageController do end end + context 'when existing file has been deleted' do + let(:lfs_object) { create(:lfs_object, :with_file) } + + before do + FileUtils.rm(lfs_object.file.path) + params[:oid] = lfs_object.oid + params[:size] = lfs_object.size + end + + it 'replaces the file' do + expect(Gitlab::AppJsonLogger).to receive(:info).with(message: "LFS file replaced because it did not exist", oid: lfs_object.oid, size: lfs_object.size) + + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(lfs_object.reload.file).to exist + end + + context 'with invalid file' do + before do + allow_next_instance_of(ActionController::Parameters) do |params| + allow(params).to receive(:[]).and_call_original + allow(params).to receive(:[]).with(:file).and_return({}) + end + end + + it 'renders LFS forbidden' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + expect(lfs_object.reload.file).not_to exist + end + end + end + context 'when file is not stored' do it 'renders unprocessable entity' do expect(controller).to receive(:store_file!).and_return(nil) diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index a0cb696828d..afebc6982c1 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -268,12 +268,15 @@ RSpec.describe SearchController do last_payload = payload end - get :show, params: { scope: 'issues', search: 'hello world', group_id: '123', project_id: '456' } + get :show, params: { scope: 'issues', search: 'hello world', group_id: '123', project_id: '456', confidential: true, state: true, force_search_results: true } expect(last_payload[:metadata]['meta.search.group_id']).to eq('123') expect(last_payload[:metadata]['meta.search.project_id']).to eq('456') expect(last_payload[:metadata]['meta.search.search']).to eq('hello world') expect(last_payload[:metadata]['meta.search.scope']).to eq('issues') + expect(last_payload[:metadata]['meta.search.force_search_results']).to eq('true') + expect(last_payload[:metadata]['meta.search.filters.confidential']).to eq('true') + expect(last_payload[:metadata]['meta.search.filters.state']).to eq('true') end end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 75bcc32e6f3..c31ba6fe156 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -86,7 +86,7 @@ RSpec.describe SessionsController do post(:create, params: { user: { login: 'invalid', password: 'invalid' } }) expect(response) - .to set_flash.now[:alert].to /Invalid Login or password/ + .to set_flash.now[:alert].to(/Invalid Login or password/) end end @@ -299,7 +299,7 @@ RSpec.describe SessionsController do context 'when using two-factor authentication via OTP' do let(:user) { create(:user, :two_factor) } - def authenticate_2fa(user_params, otp_user_id: user.id) + def authenticate_2fa(otp_user_id: user.id, **user_params) post(:create, params: { user: user_params }, session: { otp_user_id: otp_user_id }) end @@ -343,11 +343,12 @@ RSpec.describe SessionsController do it 'favors login over otp_user_id when password is present and does not authenticate the user' do authenticate_2fa( - { login: 'random_username', password: user.password }, + login: 'random_username', + password: user.password, otp_user_id: user.id ) - expect(response).to set_flash.now[:alert].to /Invalid Login or password/ + expect(response).to set_flash.now[:alert].to(/Invalid Login or password/) end end @@ -396,7 +397,7 @@ RSpec.describe SessionsController do it 'warns about invalid OTP code' do expect(response).to set_flash.now[:alert] - .to /Invalid two-factor code/ + .to(/Invalid two-factor code/) end end end diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 1ccba7f9114..993ab5d1c72 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -205,14 +205,6 @@ RSpec.describe SnippetsController do end end end - - context 'when requesting JSON' do - it 'renders the blob from the repository' do - get :show, params: { id: public_snippet.to_param }, format: :json - - expect(assigns(:blob)).to eq(public_snippet.blobs.first) - end - end end describe 'POST #mark_as_spam' do |