diff options
Diffstat (limited to 'spec/controllers')
58 files changed, 2044 insertions, 626 deletions
diff --git a/spec/controllers/admin/cohorts_controller_spec.rb b/spec/controllers/admin/cohorts_controller_spec.rb new file mode 100644 index 00000000000..9eb2a713517 --- /dev/null +++ b/spec/controllers/admin/cohorts_controller_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Admin::CohortsController do + context 'as admin' do + let(:user) { create(:admin) } + + before do + sign_in(user) + end + + it 'renders 200' do + get :index + + expect(response).to have_gitlab_http_status(:success) + end + + describe 'GET #index' do + it_behaves_like 'tracking unique visits', :index do + let(:target_id) { 'i_analytics_cohorts' } + end + end + end + + context 'as normal user' do + let(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'renders a 404' do + get :index + + expect(response).to have_gitlab_http_status(:not_found) + end + end +end diff --git a/spec/controllers/admin/dev_ops_report_controller_spec.rb b/spec/controllers/admin/dev_ops_report_controller_spec.rb new file mode 100644 index 00000000000..0be30fff0c2 --- /dev/null +++ b/spec/controllers/admin/dev_ops_report_controller_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Admin::DevOpsReportController do + describe 'GET #show' do + context 'as admin' do + let(:user) { create(:admin) } + + before do + sign_in(user) + end + + it 'responds with success' do + get :show + + expect(response).to have_gitlab_http_status(:success) + end + + it_behaves_like 'tracking unique visits', :show do + let(:target_id) { 'i_analytics_dev_ops_score' } + end + end + end + + context 'as normal user' do + let(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'responds with 404' do + get :show + + expect(response).to have_gitlab_http_status(:not_found) + end + end +end diff --git a/spec/controllers/admin/instance_statistics_controller_spec.rb b/spec/controllers/admin/instance_statistics_controller_spec.rb new file mode 100644 index 00000000000..c589e46857f --- /dev/null +++ b/spec/controllers/admin/instance_statistics_controller_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Admin::InstanceStatisticsController do + let(:admin) { create(:user, :admin) } + + before do + sign_in(admin) + end + + describe 'GET #show' do + it_behaves_like 'tracking unique visits', :index do + let(:target_id) { 'i_analytics_instance_statistics' } + end + end +end diff --git a/spec/controllers/admin/integrations_controller_spec.rb b/spec/controllers/admin/integrations_controller_spec.rb index 4a5d5ede728..4b1806a43d2 100644 --- a/spec/controllers/admin/integrations_controller_spec.rb +++ b/spec/controllers/admin/integrations_controller_spec.rb @@ -23,12 +23,15 @@ RSpec.describe Admin::IntegrationsController do end describe '#update' do + include JiraServiceHelper + let(:integration) { create(:jira_service, :instance) } before do + stub_jira_service_test allow(PropagateIntegrationWorker).to receive(:perform_async) - put :update, params: { id: integration.class.to_param, overwrite: true, service: { url: url } } + put :update, params: { id: integration.class.to_param, service: { url: url } } end context 'valid params' do @@ -40,7 +43,7 @@ RSpec.describe Admin::IntegrationsController do end it 'calls to PropagateIntegrationWorker' do - expect(PropagateIntegrationWorker).to have_received(:perform_async).with(integration.id, true) + expect(PropagateIntegrationWorker).to have_received(:perform_async).with(integration.id, false) end end diff --git a/spec/controllers/admin/plan_limits_controller_spec.rb b/spec/controllers/admin/plan_limits_controller_spec.rb new file mode 100644 index 00000000000..2666925c2b7 --- /dev/null +++ b/spec/controllers/admin/plan_limits_controller_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Admin::PlanLimitsController do + let_it_be(:plan) { create(:plan) } + let_it_be(:plan_limits) { create(:plan_limits, plan: plan) } + + describe 'POST create' do + let(:params) do + { + plan_limits: { + plan_id: plan.id, + conan_max_file_size: file_size, id: plan_limits.id + } + } + end + + context 'with an authenticated admin user' do + let(:file_size) { 10.megabytes } + + it 'updates the plan limits', :aggregate_failures do + sign_in(create(:admin)) + + post :create, params: params + + expect(response).to redirect_to(general_admin_application_settings_path) + expect(plan_limits.reload.conan_max_file_size).to eq(file_size) + end + end + + context 'without admin access' do + let(:file_size) { 1.megabytes } + + it 'returns `not_found`' do + sign_in(create(:user)) + + post :create, params: params + + expect(response).to have_gitlab_http_status(:not_found) + expect(plan_limits.conan_max_file_size).not_to eq(file_size) + end + end + end +end diff --git a/spec/controllers/admin/sessions_controller_spec.rb b/spec/controllers/admin/sessions_controller_spec.rb index 82366cc6952..35982e57034 100644 --- a/spec/controllers/admin/sessions_controller_spec.rb +++ b/spec/controllers/admin/sessions_controller_spec.rb @@ -220,10 +220,8 @@ RSpec.describe Admin::SessionsController, :do_not_mock_admin_mode do end end - context 'when using two-factor authentication via U2F' do - let(:user) { create(:admin, :two_factor_via_u2f) } - - def authenticate_2fa_u2f(user_params) + shared_examples 'when using two-factor authentication via hardware device' do + def authenticate_2fa(user_params) post(:create, params: { user: user_params }, session: { otp_user_id: user.id }) end @@ -239,14 +237,18 @@ RSpec.describe Admin::SessionsController, :do_not_mock_admin_mode do end it 'can login with valid auth' do + # we can stub both without an differentiation between webauthn / u2f + # as these not interfere with each other und this saves us passing aroud + # parameters allow(U2fRegistration).to receive(:authenticate).and_return(true) + allow_any_instance_of(Webauthn::AuthenticateService).to receive(:execute).and_return(true) expect(controller.current_user_mode.admin_mode?).to be(false) controller.store_location_for(:redirect, admin_root_path) controller.current_user_mode.request_admin_mode! - authenticate_2fa_u2f(login: user.username, device_response: '{}') + authenticate_2fa(login: user.username, device_response: '{}') expect(response).to redirect_to admin_root_path expect(controller.current_user_mode.admin_mode?).to be(true) @@ -254,16 +256,33 @@ RSpec.describe Admin::SessionsController, :do_not_mock_admin_mode do it 'cannot login with invalid auth' do allow(U2fRegistration).to receive(:authenticate).and_return(false) + allow_any_instance_of(Webauthn::AuthenticateService).to receive(:execute).and_return(false) expect(controller.current_user_mode.admin_mode?).to be(false) controller.current_user_mode.request_admin_mode! - authenticate_2fa_u2f(login: user.username, device_response: '{}') + authenticate_2fa(login: user.username, device_response: '{}') expect(response).to render_template('admin/sessions/two_factor') expect(controller.current_user_mode.admin_mode?).to be(false) end end + + context 'when using two-factor authentication via U2F' do + it_behaves_like 'when using two-factor authentication via hardware device' do + let(:user) { create(:admin, :two_factor_via_u2f) } + + before do + stub_feature_flags(webauthn: false) + end + end + end + + context 'when using two-factor authentication via WebAuthn' do + it_behaves_like 'when using two-factor authentication via hardware device' do + let(:user) { create(:admin, :two_factor_via_webauthn) } + end + end end end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 08a1d7c9fa9..e4cdcda756b 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -218,28 +218,44 @@ RSpec.describe Admin::UsersController do end describe 'PATCH disable_two_factor' do - it 'disables 2FA for the user' do - expect(user).to receive(:disable_two_factor!) - allow(subject).to receive(:user).and_return(user) + subject { patch :disable_two_factor, params: { id: user.to_param } } - go - end + context 'for a user that has 2FA enabled' do + let(:user) { create(:user, :two_factor) } - it 'redirects back' do - go + it 'disables 2FA for the user' do + subject - expect(response).to redirect_to(admin_user_path(user)) - end + expect(user.reload.two_factor_enabled?).to eq(false) + end + + it 'redirects back' do + subject + + expect(response).to redirect_to(admin_user_path(user)) + end - it 'displays an alert' do - go + it 'displays a notice on success' do + subject - expect(flash[:notice]) - .to eq _('Two-factor Authentication has been disabled for this user') + expect(flash[:notice]) + .to eq _('Two-factor authentication has been disabled for this user') + end end - def go - patch :disable_two_factor, params: { id: user.to_param } + context 'for a user that does not have 2FA enabled' do + it 'redirects back' do + subject + + expect(response).to redirect_to(admin_user_path(user)) + end + + it 'displays an alert on failure' do + subject + + expect(flash[:alert]) + .to eq _('Two-factor authentication is not enabled for this user') + end end end @@ -270,82 +286,111 @@ RSpec.describe Admin::UsersController do describe 'POST update' do context 'when the password has changed' do - def update_password(user, password, password_confirmation = nil) + def update_password(user, password = User.random_password, password_confirmation = password) params = { id: user.to_param, user: { password: password, - password_confirmation: password_confirmation || password + password_confirmation: password_confirmation } } post :update, params: params end - context 'when the admin changes their own password' do - it 'updates the password' do - expect { update_password(admin, 'AValidPassword1') } - .to change { admin.reload.encrypted_password } - end - - it 'does not set the new password to expire immediately' do - expect { update_password(admin, 'AValidPassword1') } - .not_to change { admin.reload.password_expires_at } + context 'when admin changes their own password' do + context 'when password is valid' do + it 'updates the password' do + expect { update_password(admin) } + .to change { admin.reload.encrypted_password } + end + + it 'does not set the new password to expire immediately' do + expect { update_password(admin) } + .not_to change { admin.reload.password_expired? } + end + + it 'does not enqueue the `admin changed your password` email' do + expect { update_password(admin) } + .not_to have_enqueued_mail(DeviseMailer, :password_change_by_admin) + end + + it 'enqueues the `password changed` email' do + expect { update_password(admin) } + .to have_enqueued_mail(DeviseMailer, :password_change) + end end end - context 'when the new password is valid' do - it 'redirects to the user' do - update_password(user, 'AValidPassword1') - - expect(response).to redirect_to(admin_user_path(user)) - end - - it 'updates the password' do - expect { update_password(user, 'AValidPassword1') } - .to change { user.reload.encrypted_password } - end - - it 'sets the new password to expire immediately' do - expect { update_password(user, 'AValidPassword1') } - .to change { user.reload.password_expires_at }.to be_within(2.seconds).of(Time.current) + context 'when admin changes the password of another user' do + context 'when the new password is valid' do + it 'redirects to the user' do + update_password(user) + + expect(response).to redirect_to(admin_user_path(user)) + end + + it 'updates the password' do + expect { update_password(user) } + .to change { user.reload.encrypted_password } + end + + it 'sets the new password to expire immediately' do + expect { update_password(user) } + .to change { user.reload.password_expired? }.from(false).to(true) + end + + it 'enqueues the `admin changed your password` email' do + expect { update_password(user) } + .to have_enqueued_mail(DeviseMailer, :password_change_by_admin) + end + + it 'does not enqueue the `password changed` email' do + expect { update_password(user) } + .not_to have_enqueued_mail(DeviseMailer, :password_change) + end end end context 'when the new password is invalid' do + let(:password) { 'invalid' } + it 'shows the edit page again' do - update_password(user, 'invalid') + update_password(user, password) expect(response).to render_template(:edit) end it 'returns the error message' do - update_password(user, 'invalid') + update_password(user, password) expect(assigns[:user].errors).to contain_exactly(a_string_matching(/too short/)) end it 'does not update the password' do - expect { update_password(user, 'invalid') } + expect { update_password(user, password) } .not_to change { user.reload.encrypted_password } end end context 'when the new password does not match the password confirmation' do + let(:password) { 'some_password' } + let(:password_confirmation) { 'not_same_as_password' } + it 'shows the edit page again' do - update_password(user, 'AValidPassword1', 'AValidPassword2') + update_password(user, password, password_confirmation) expect(response).to render_template(:edit) end it 'returns the error message' do - update_password(user, 'AValidPassword1', 'AValidPassword2') + update_password(user, password, password_confirmation) expect(assigns[:user].errors).to contain_exactly(a_string_matching(/doesn't match/)) end it 'does not update the password' do - expect { update_password(user, 'AValidPassword1', 'AValidPassword2') } + expect { update_password(user, password, password_confirmation) } .not_to change { user.reload.encrypted_password } end end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index f8d4690e9ce..188a4cb04af 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -795,13 +795,8 @@ RSpec.describe ApplicationController do end let(:user) { create(:user) } - let(:experiment_enabled) { true } - before do - stub_experiment_for_user(signup_flow: experiment_enabled) - end - - context 'experiment enabled and user with required role' do + context 'user with required role' do before do user.set_role_required! sign_in(user) @@ -811,7 +806,7 @@ RSpec.describe ApplicationController do it { is_expected.to redirect_to users_sign_up_welcome_path } end - context 'experiment enabled and user without a required role' do + context 'user without a required role' do before do sign_in(user) get :index @@ -819,43 +814,31 @@ RSpec.describe ApplicationController do it { is_expected.not_to redirect_to users_sign_up_welcome_path } end + end - context 'experiment disabled' do - let(:experiment_enabled) { false } + describe 'rescue_from Gitlab::Auth::IpBlacklisted' do + controller(described_class) do + skip_before_action :authenticate_user! - before do - user.set_role_required! - sign_in(user) - get :index + def index + raise Gitlab::Auth::IpBlacklisted end - - it { is_expected.not_to redirect_to users_sign_up_welcome_path } end - describe 'rescue_from Gitlab::Auth::IpBlacklisted' do - controller(described_class) do - skip_before_action :authenticate_user! - - def index - raise Gitlab::Auth::IpBlacklisted - end - end - - it 'returns a 403 and logs the request' do - expect(Gitlab::AuthLogger).to receive(:error).with({ - message: 'Rack_Attack', - env: :blocklist, - remote_ip: '1.2.3.4', - request_method: 'GET', - path: '/anonymous' - }) + it 'returns a 403 and logs the request' do + expect(Gitlab::AuthLogger).to receive(:error).with({ + message: 'Rack_Attack', + env: :blocklist, + remote_ip: '1.2.3.4', + request_method: 'GET', + path: '/anonymous' + }) - request.remote_addr = '1.2.3.4' + request.remote_addr = '1.2.3.4' - get :index + get :index - expect(response).to have_gitlab_http_status(:forbidden) - end + expect(response).to have_gitlab_http_status(:forbidden) end end diff --git a/spec/controllers/concerns/redis_tracking_spec.rb b/spec/controllers/concerns/redis_tracking_spec.rb new file mode 100644 index 00000000000..3795fca5576 --- /dev/null +++ b/spec/controllers/concerns/redis_tracking_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe RedisTracking do + let(:event_name) { 'g_compliance_dashboard' } + let(:feature) { 'g_compliance_dashboard_feature' } + let(:user) { create(:user) } + + controller(ApplicationController) do + include RedisTracking + + skip_before_action :authenticate_user!, only: :show + track_redis_hll_event :index, :show, name: 'i_analytics_dev_ops_score', feature: :g_compliance_dashboard_feature, feature_default_enabled: true + + def index + render html: 'index' + end + + def new + render html: 'new' + end + + def show + render html: 'show' + end + end + + context 'with feature disabled' do + it 'does not track the event' do + stub_feature_flags(feature => false) + + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) + + get :index + end + end + + context 'with usage ping disabled' do + it 'does not track the event' do + stub_feature_flags(feature => true) + allow(Gitlab::CurrentSettings).to receive(:usage_ping_enabled?).and_return(false) + + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) + + get :index + end + end + + context 'with feature enabled and usage ping enabled' do + before do + stub_feature_flags(feature => true) + allow(Gitlab::CurrentSettings).to receive(:usage_ping_enabled?).and_return(true) + end + + context 'when user is logged in' do + it 'tracks the event' do + sign_in(user) + + expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event) + + get :index + end + + it 'passes default_enabled flag' do + sign_in(user) + + expect(controller).to receive(:metric_feature_enabled?).with(feature.to_sym, true) + + get :index + end + end + + context 'when user is not logged in and there is a visitor_id' do + let(:visitor_id) { SecureRandom.uuid } + + before do + routes.draw { get 'show' => 'anonymous#show' } + end + + it 'tracks the event' do + cookies[:visitor_id] = { value: visitor_id, expires: 24.months } + + expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event) + + get :show + end + end + + context 'when user is not logged in and there is no visitor_id' do + it 'does not tracks the event' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) + + get :index + end + end + + context 'for untracked action' do + it 'does not tracks the event' do + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) + + get :new + end + end + end +end diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb index e24e4cbf5e7..747ccd7ba1b 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -50,9 +50,14 @@ RSpec.describe SendFileUpload do shared_examples 'handles image resize requests' do let(:headers) { double } + let(:image_requester) { build(:user) } + let(:image_owner) { build(:user) } + let(:params) do + { attachment: 'avatar.png' } + end before do - allow(uploader).to receive(:image?).and_return(true) + allow(uploader).to receive(:image_safe_for_scaling?).and_return(true) allow(uploader).to receive(:mounted_as).and_return(:avatar) allow(controller).to receive(:headers).and_return(headers) @@ -60,70 +65,127 @@ RSpec.describe SendFileUpload do # local or remote files allow(controller).to receive(:send_file) allow(controller).to receive(:redirect_to) + + allow(controller).to receive(:current_user).and_return(image_requester) + allow(uploader).to receive(:model).and_return(image_owner) end - context 'when feature is enabled for current user' do - let(:user) { build(:user) } + 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: user) - allow(controller).to receive(:current_user).and_return(user) + stub_feature_flags(dynamic_image_resizing_requester: true) + stub_feature_flags(dynamic_image_resizing_owner: true) end - context 'with valid width parameter' do - it 'renders OK with workhorse command header' do - expect(controller).not_to receive(:send_file) - expect(controller).to receive(:params).at_least(:once).and_return(width: '64') - expect(controller).to receive(:head).with(:ok) - expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) + it_behaves_like 'handles image resize requests allowed by FFs' - subject + 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 'with missing width parameter' do - it 'does not write workhorse command header' do - expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) + 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 - subject - 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 - context 'with invalid width parameter' do - it 'does not write workhorse command header' do - expect(controller).to receive(:params).at_least(:once).and_return(width: 'not a number') - expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) + it_behaves_like 'bypasses image resize requests not allowed by FFs' + end - subject - end + context 'when both FFs are disabled' do + before do + stub_feature_flags(dynamic_image_resizing_requester: false) + stub_feature_flags(dynamic_image_resizing_owner: false) end - context 'with width that is not allowed' do - it 'does not write workhorse command header' do - expect(controller).to receive(:params).at_least(:once).and_return(width: '63') - expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) + it_behaves_like 'bypasses image resize requests not allowed by FFs' + end + end - subject - end + shared_examples 'bypasses image resize requests not allowed by FFs' do + it 'does not write workhorse command header' do + expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) + + subject + end + end + + shared_examples 'handles image resize requests allowed by FFs' do + context 'with valid width parameter' do + it 'renders OK with workhorse command header' do + expect(controller).not_to receive(:send_file) + expect(controller).to receive(:params).at_least(:once).and_return(width: '64') + expect(controller).to receive(:head).with(:ok) + + expect(Gitlab::Workhorse).to receive(:send_scaled_image).with(a_string_matching('^(/.+|https://.+)'), 64, 'image/png').and_return([ + Gitlab::Workhorse::SEND_DATA_HEADER, "send-scaled-img:faux" + ]) + expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, "send-scaled-img:faux") + + subject end + end - context 'when image file is not an avatar' do - it 'does not write workhorse command header' do - expect(uploader).to receive(:mounted_as).and_return(nil) # FileUploader is not mounted - expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) + context 'with missing width parameter' do + it 'does not write workhorse command header' do + expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) - subject - end + subject end end - context 'when feature is disabled' do - before do - stub_feature_flags(dynamic_image_resizing: false) + context 'with invalid width parameter' do + it 'does not write workhorse command header' do + expect(controller).to receive(:params).at_least(:once).and_return(width: 'not a number') + expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) + + subject end + end + context 'with width that is not allowed' do it 'does not write workhorse command header' do - expect(controller).to receive(:params).at_least(:once).and_return(width: '64') + expect(controller).to receive(:params).at_least(:once).and_return(width: '63') + expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) + + subject + end + end + + context 'when image file is not an avatar' do + it 'does not write workhorse command header' do + expect(uploader).to receive(:mounted_as).and_return(nil) # FileUploader is not mounted + expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) + + subject + end + end + + context 'when image file type is not considered safe for scaling' do + it 'does not write workhorse command header' do + expect(uploader).to receive(:image_safe_for_scaling?).and_return(false) expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) subject diff --git a/spec/controllers/dashboard/projects_controller_spec.rb b/spec/controllers/dashboard/projects_controller_spec.rb index 1e1d9519f78..2719b7c8a24 100644 --- a/spec/controllers/dashboard/projects_controller_spec.rb +++ b/spec/controllers/dashboard/projects_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Dashboard::ProjectsController do +RSpec.describe Dashboard::ProjectsController, :aggregate_failures do include ExternalAuthorizationServiceHelpers let_it_be(:user) { create(:user) } @@ -15,6 +15,7 @@ RSpec.describe Dashboard::ProjectsController do context 'user logged in' do let_it_be(:project) { create(:project) } let_it_be(:project2) { create(:project) } + let(:projects) { [project, project2] } before_all do project.add_developer(user) @@ -41,7 +42,7 @@ RSpec.describe Dashboard::ProjectsController do get :index - expect(assigns(:projects)).to eq([project, project2]) + expect(assigns(:projects)).to eq(projects) end context 'project sorting' do @@ -66,6 +67,20 @@ RSpec.describe Dashboard::ProjectsController do it_behaves_like 'search and sort parameters', sort end end + + context 'with deleted project' do + let!(:pending_delete_project) do + project.tap { |p| p.update!(pending_delete: true) } + end + + it 'does not display deleted project' do + get :index + projects_result = assigns(:projects) + + expect(projects_result).not_to include(pending_delete_project) + expect(projects_result).to include(project2) + end + end end end @@ -153,7 +168,7 @@ RSpec.describe Dashboard::ProjectsController do project.add_developer(user) end - it 'renders all kinds of event without error', :aggregate_failures do + it 'renders all kinds of event without error' do get :index, format: :atom expect(assigns(:events)).to include(design_event, wiki_page_event, issue_event) @@ -165,6 +180,21 @@ RSpec.describe Dashboard::ProjectsController do "closed issue #{issue.to_reference}" ) end + + context 'with deleted project' do + let(:pending_deleted_project) { projects.last.tap { |p| p.update!(pending_delete: true) } } + + before do + pending_deleted_project.add_developer(user) + end + + it 'does not display deleted project' do + get :index, format: :atom + + expect(response.body).not_to include(pending_deleted_project.full_name) + expect(response.body).to include(project.full_name) + end + end end end end diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index c5643f96b7a..405f1eae482 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -60,14 +60,28 @@ RSpec.describe GraphqlController do it 'updates the users last_activity_on field' do expect { post :execute }.to change { user.reload.last_activity_on } end + + it "sets context's sessionless value as false" do + post :execute + + expect(assigns(:context)[:is_sessionless_user]).to be false + end end context 'when user uses an API token' do let(:user) { create(:user, last_activity_on: Date.yesterday) } let(:token) { create(:personal_access_token, user: user, scopes: [:api]) } + subject { post :execute, params: { access_token: token.token } } + it 'updates the users last_activity_on field' do - expect { post :execute, params: { access_token: token.token } }.to change { user.reload.last_activity_on } + expect { subject }.to change { user.reload.last_activity_on } + end + + it "sets context's sessionless value as true" do + subject + + expect(assigns(:context)[:is_sessionless_user]).to be true end end @@ -77,6 +91,12 @@ RSpec.describe GraphqlController do expect(response).to have_gitlab_http_status(:ok) end + + it "sets context's sessionless value as false" do + post :execute + + expect(assigns(:context)[:is_sessionless_user]).to be false + end end end @@ -127,4 +147,22 @@ RSpec.describe GraphqlController do end end end + + describe '#append_info_to_payload' do + let(:graphql_query) { graphql_query_for('project', { 'fullPath' => 'foo' }, %w(id name)) } + let(:log_payload) { {} } + + before do + allow(controller).to receive(:append_info_to_payload).and_wrap_original do |method, *| + method.call(log_payload) + end + end + + it 'appends metadata for logging' do + post :execute, params: { query: graphql_query, operationName: 'Foo' } + + expect(controller).to have_received(:append_info_to_payload) + expect(log_payload.dig(:metadata, :graphql, :operation_name)).to eq('Foo') + end + end end diff --git a/spec/controllers/groups/settings/integrations_controller_spec.rb b/spec/controllers/groups/settings/integrations_controller_spec.rb index d079f3f077e..cdcdfde175f 100644 --- a/spec/controllers/groups/settings/integrations_controller_spec.rb +++ b/spec/controllers/groups/settings/integrations_controller_spec.rb @@ -3,7 +3,6 @@ require 'spec_helper' RSpec.describe Groups::Settings::IntegrationsController do - let_it_be(:project) { create(:project) } let(:user) { create(:user) } let(:group) { create(:group) } @@ -82,10 +81,13 @@ RSpec.describe Groups::Settings::IntegrationsController do end describe '#update' do - let(:integration) { create(:jira_service, project: project) } + include JiraServiceHelper + + let(:integration) { create(:jira_service, project: nil, group_id: group.id) } before do group.add_owner(user) + stub_jira_service_test put :update, params: { group_id: group, id: integration.class.to_param, service: { url: url } } end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 78de89de796..35d8c0b7c6d 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -422,10 +422,6 @@ RSpec.describe GroupsController do end context 'searching' do - before do - stub_feature_flags(attempt_group_search_optimizations: true) - end - it 'works with popularity sort' do get :issues, params: { id: group.to_param, search: 'foo', sort: 'popularity' } diff --git a/spec/controllers/instance_statistics/cohorts_controller_spec.rb b/spec/controllers/instance_statistics/cohorts_controller_spec.rb deleted file mode 100644 index c16ac0dced1..00000000000 --- a/spec/controllers/instance_statistics/cohorts_controller_spec.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe InstanceStatistics::CohortsController do - let(:user) { create(:user) } - - before do - sign_in(user) - end - - it_behaves_like 'instance statistics availability' - - it 'renders a 404 when the usage ping is disabled' do - stub_application_setting(usage_ping_enabled: false) - - get :index - - expect(response).to have_gitlab_http_status(:not_found) - end - - describe 'GET #index' do - it_behaves_like 'tracking unique visits', :index do - let(:request_params) { {} } - let(:target_id) { 'i_analytics_cohorts' } - end - end -end diff --git a/spec/controllers/instance_statistics/dev_ops_score_controller_spec.rb b/spec/controllers/instance_statistics/dev_ops_score_controller_spec.rb deleted file mode 100644 index c9677a64eef..00000000000 --- a/spec/controllers/instance_statistics/dev_ops_score_controller_spec.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe InstanceStatistics::DevOpsScoreController do - it_behaves_like 'instance statistics availability' - - describe 'GET #index' do - let(:user) { create(:user) } - - before do - sign_in(user) - end - - it_behaves_like 'tracking unique visits', :index do - let(:request_params) { {} } - let(:target_id) { 'i_analytics_dev_ops_score' } - end - end -end diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index 2b222331b55..a083cfac981 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -2,36 +2,142 @@ require 'spec_helper' -RSpec.describe InvitesController do - let(:token) { '123456' } +RSpec.describe InvitesController, :snowplow do let_it_be(:user) { create(:user) } - let(:member) { create(:project_member, :invited, invite_token: token, invite_email: user.email) } + let(:member) { create(:project_member, :invited, invite_email: user.email) } + let(:raw_invite_token) { member.raw_invite_token } 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 before do controller.instance_variable_set(:@member, member) - sign_in(user) end describe 'GET #show' do - it 'accepts user if invite email matches signed in user' do - expect do - get :show, params: { id: token } - end.to change { project_members.include?(user) }.from(false).to(true) + subject(:request) { get :show, params: params } + + context 'when logged in' do + before do + sign_in(user) + end + + it 'accepts user if invite email matches signed in user' do + expect do + request + end.to change { project_members.include?(user) }.from(false).to(true) + + expect(response).to have_gitlab_http_status(:found) + expect(flash[:notice]).to include 'You have been granted' + end + + it 'forces re-confirmation if email does not match signed in user' do + member.invite_email = 'bogus@email.com' + + expect do + request + end.not_to change { project_members.include?(user) } + + expect(response).to have_gitlab_http_status(:ok) + 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 + end + + context 'when not logged in' do + context 'when inviter is a member' do + it 'is redirected to a new session with invite email param' do + request + + expect(response).to redirect_to(new_user_session_path(invite_email: member.invite_email)) + end + end + + context 'when inviter is not a member' do + let(:params) { { id: '_bogus_token_' } } + + it 'is redirected to a new session' do + request + + expect(response).to redirect_to(new_user_session_path) + end + end + end + end + + describe 'POST #accept' do + before do + sign_in(user) + end + + 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(response).to have_gitlab_http_status(:found) - expect(flash[:notice]).to include 'You have been granted' + expect_snowplow_event(snowplow_event.merge(action: 'accepted')) + end end - it 'forces re-confirmation if email does not match signed in user' do - member.invite_email = 'bogus@email.com' + context 'when new_user_invite is control' do + let(:params) { { id: raw_invite_token, new_user_invite: 'control' } } + let(:group_type) { 'control_group' } - expect do - get :show, params: { id: token } - end.not_to change { project_members.include?(user) } + it 'tracks the user as control group' do + request - expect(response).to have_gitlab_http_status(:ok) - expect(flash[:notice]).to be_nil + expect_snowplow_event(snowplow_event.merge(action: 'accepted')) + end end end end diff --git a/spec/controllers/jira_connect/app_descriptor_controller_spec.rb b/spec/controllers/jira_connect/app_descriptor_controller_spec.rb new file mode 100644 index 00000000000..55bafa938a7 --- /dev/null +++ b/spec/controllers/jira_connect/app_descriptor_controller_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::AppDescriptorController do + describe '#show' do + it 'returns JSON app descriptor' do + get :show + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include( + 'baseUrl' => 'https://test.host/-/jira_connect', + 'lifecycle' => { + 'installed' => '/events/installed', + 'uninstalled' => '/events/uninstalled' + }, + 'links' => { + 'documentation' => 'http://test.host/help/integration/jira_development_panel#gitlabcom-1' + } + ) + end + end +end diff --git a/spec/controllers/jira_connect/events_controller_spec.rb b/spec/controllers/jira_connect/events_controller_spec.rb new file mode 100644 index 00000000000..d1a2dd6e7af --- /dev/null +++ b/spec/controllers/jira_connect/events_controller_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::EventsController do + describe '#installed' do + subject do + post :installed, params: { + clientKey: '1234', + sharedSecret: 'secret', + baseUrl: 'https://test.atlassian.net' + } + end + + it 'saves the jira installation data' do + expect { subject }.to change { JiraConnectInstallation.count }.by(1) + end + + it 'saves the correct values' do + subject + + installation = JiraConnectInstallation.find_by_client_key('1234') + + expect(installation.shared_secret).to eq('secret') + expect(installation.base_url).to eq('https://test.atlassian.net') + end + + context 'client key already exists' do + it 'returns 422' do + create(:jira_connect_installation, client_key: '1234') + + subject + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + end + + describe '#uninstalled' do + let!(:installation) { create(:jira_connect_installation) } + let(:qsh) { Atlassian::Jwt.create_query_string_hash('https://gitlab.test/events/uninstalled', 'POST', 'https://gitlab.test') } + + before do + request.headers['Authorization'] = "JWT #{auth_token}" + end + + subject { post :uninstalled } + + context 'when JWT is invalid' do + let(:auth_token) { 'invalid_token' } + + it 'returns 403' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'does not delete the installation' do + expect { subject }.not_to change { JiraConnectInstallation.count } + end + end + + context 'when JWT is valid' do + let(:auth_token) do + Atlassian::Jwt.encode({ iss: installation.client_key, qsh: qsh }, installation.shared_secret) + end + + it 'deletes the installation' do + expect { subject }.to change { JiraConnectInstallation.count }.by(-1) + end + end + end + end +end diff --git a/spec/controllers/jira_connect/subscriptions_controller_spec.rb b/spec/controllers/jira_connect/subscriptions_controller_spec.rb new file mode 100644 index 00000000000..95b359a989a --- /dev/null +++ b/spec/controllers/jira_connect/subscriptions_controller_spec.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::SubscriptionsController do + let_it_be(:installation) { create(:jira_connect_installation) } + + describe '#index' do + before do + get :index, params: { jwt: jwt } + end + + context 'without JWT' do + let(:jwt) { nil } + + it 'returns 403' do + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'with valid JWT' do + let(:qsh) { Atlassian::Jwt.create_query_string_hash('https://gitlab.test/subscriptions', 'GET', 'https://gitlab.test') } + let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key, qsh: qsh }, installation.shared_secret) } + + it 'returns 200' do + expect(response).to have_gitlab_http_status(:ok) + end + + it 'removes X-Frame-Options to allow rendering in iframe' do + expect(response.headers['X-Frame-Options']).to be_nil + end + end + end + + describe '#create' do + let(:group) { create(:group) } + let(:user) { create(:user) } + let(:current_user) { user } + + before do + group.add_maintainer(user) + end + + subject { post :create, params: { jwt: jwt, namespace_path: group.path, format: :json } } + + context 'without JWT' do + let(:jwt) { nil } + + it 'returns 403' do + sign_in(user) + + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'with valid JWT' do + let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key }, installation.shared_secret) } + + context 'signed in to GitLab' do + before do + sign_in(user) + end + + context 'dev panel integration is available' do + it 'creates a subscription' do + expect { subject }.to change { installation.subscriptions.count }.from(0).to(1) + end + + it 'returns 200' do + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + context 'not signed in to GitLab' do + it 'returns 401' do + subject + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + end + + describe '#destroy' do + let(:subscription) { create(:jira_connect_subscription, installation: installation) } + + before do + delete :destroy, params: { jwt: jwt, id: subscription.id } + end + + context 'without JWT' do + let(:jwt) { nil } + + it 'returns 403' do + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'with valid JWT' do + let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key }, installation.shared_secret) } + + it 'deletes the subscription' do + expect { subscription.reload }.to raise_error ActiveRecord::RecordNotFound + expect(response).to have_gitlab_http_status(:ok) + end + end + end +end diff --git a/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb b/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb index 2de824bbf3c..ecff173b8ac 100644 --- a/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb @@ -11,6 +11,11 @@ RSpec.describe Ldap::OmniauthCallbacksController do expect(request.env['warden']).to be_authenticated end + it 'creates an authentication event record' do + expect { post provider }.to change { AuthenticationEvent.count }.by(1) + expect(AuthenticationEvent.last.provider).to eq(provider.to_s) + end + context 'with sign in prevented' do let(:ldap_settings) { ldap_setting_defaults.merge(prevent_ldap_sign_in: true) } diff --git a/spec/controllers/oauth/jira/authorizations_controller_spec.rb b/spec/controllers/oauth/jira/authorizations_controller_spec.rb new file mode 100644 index 00000000000..0b4a691d7ec --- /dev/null +++ b/spec/controllers/oauth/jira/authorizations_controller_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Oauth::Jira::AuthorizationsController do + describe 'GET new' do + it 'redirects to OAuth authorization with correct params' do + get :new, params: { client_id: 'client-123', redirect_uri: 'http://example.com/' } + + expect(response).to redirect_to(oauth_authorization_url(client_id: 'client-123', + response_type: 'code', + redirect_uri: oauth_jira_callback_url)) + end + end + + describe 'GET callback' do + it 'redirects to redirect_uri on session with code param' do + session['redirect_uri'] = 'http://example.com' + + get :callback, params: { code: 'hash-123' } + + expect(response).to redirect_to('http://example.com?code=hash-123') + end + + it 'redirects to redirect_uri on session with code param preserving existing query' do + session['redirect_uri'] = 'http://example.com?foo=bar' + + get :callback, params: { code: 'hash-123' } + + expect(response).to redirect_to('http://example.com?foo=bar&code=hash-123') + end + end + + describe 'POST access_token' do + it 'returns oauth params in a format Jira expects' do + expect_any_instance_of(Doorkeeper::Request::AuthorizationCode).to receive(:authorize) do + double(status: :ok, body: { 'access_token' => 'fake-123', 'scope' => 'foo', 'token_type' => 'bar' }) + end + + post :access_token, params: { code: 'code-123', client_id: 'client-123', client_secret: 'secret-123' } + + expect(response.body).to eq('access_token=fake-123&scope=foo&token_type=bar') + end + end +end diff --git a/spec/controllers/oauth/token_info_controller_spec.rb b/spec/controllers/oauth/token_info_controller_spec.rb index 91a986db251..6d01a534673 100644 --- a/spec/controllers/oauth/token_info_controller_spec.rb +++ b/spec/controllers/oauth/token_info_controller_spec.rb @@ -5,10 +5,10 @@ require 'spec_helper' RSpec.describe Oauth::TokenInfoController do describe '#show' do context 'when the user is not authenticated' do - it 'responds with a 401' do + it 'responds with a 400' do get :show - expect(response).to have_gitlab_http_status(:unauthorized) + expect(response).to have_gitlab_http_status(:bad_request) expect(Gitlab::Json.parse(response.body)).to include('error' => 'invalid_request') end end @@ -36,10 +36,10 @@ RSpec.describe Oauth::TokenInfoController do end context 'when the doorkeeper_token is not recognised' do - it 'responds with a 401' do + it 'responds with a 400' do get :show, params: { access_token: 'unknown_token' } - expect(response).to have_gitlab_http_status(:unauthorized) + expect(response).to have_gitlab_http_status(:bad_request) expect(Gitlab::Json.parse(response.body)).to include('error' => 'invalid_request') end end @@ -49,10 +49,10 @@ RSpec.describe Oauth::TokenInfoController do create(:oauth_access_token, created_at: 2.days.ago, expires_in: 10.minutes) end - it 'responds with a 401' do + it 'responds with a 400' do get :show, params: { access_token: access_token.token } - expect(response).to have_gitlab_http_status(:unauthorized) + expect(response).to have_gitlab_http_status(:bad_request) expect(Gitlab::Json.parse(response.body)).to include('error' => 'invalid_request') end end @@ -60,10 +60,10 @@ RSpec.describe Oauth::TokenInfoController do context 'when the token is revoked' do let(:access_token) { create(:oauth_access_token, revoked_at: 2.days.ago) } - it 'responds with a 401' do + it 'responds with a 400' do get :show, params: { access_token: access_token.token } - expect(response).to have_gitlab_http_status(:unauthorized) + expect(response).to have_gitlab_http_status(:bad_request) expect(Gitlab::Json.parse(response.body)).to include('error' => 'invalid_request') end end diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index 3f7f0c55f38..291d51348e6 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -170,6 +170,11 @@ RSpec.describe OmniauthCallbacksController, type: :controller do expect(request.env['warden']).to be_authenticated end + it 'creates an authentication event record' do + expect { post provider }.to change { AuthenticationEvent.count }.by(1) + expect(AuthenticationEvent.last.provider).to eq(provider.to_s) + end + context 'when user has no linked provider' do let(:user) { create(:user) } @@ -276,6 +281,51 @@ RSpec.describe OmniauthCallbacksController, type: :controller do end end + context 'atlassian_oauth2' do + let(:provider) { :atlassian_oauth2 } + let(:extern_uid) { 'my-uid' } + + context 'when the user and identity already exist' do + let(:user) { create(:atlassian_user, extern_uid: extern_uid) } + + it 'allows sign-in' do + post :atlassian_oauth2 + + expect(request.env['warden']).to be_authenticated + end + end + + context 'for a new user' do + before do + stub_omniauth_setting(enabled: true, auto_link_user: true, allow_single_sign_on: ['atlassian_oauth2']) + + user.destroy + end + + it 'denies sign-in if sign-up is enabled, but block_auto_created_users is set' do + post :atlassian_oauth2 + + expect(flash[:alert]).to start_with 'Your account has been blocked.' + end + + it 'accepts sign-in if sign-up is enabled' do + stub_omniauth_setting(block_auto_created_users: false) + + post :atlassian_oauth2 + + expect(request.env['warden']).to be_authenticated + end + + it 'denies sign-in if sign-up is not enabled' do + stub_omniauth_setting(allow_single_sign_on: false, block_auto_created_users: false) + + post :atlassian_oauth2 + + expect(flash[:alert]).to start_with 'Signing in using your Atlassian account without a pre-existing GitLab account is not allowed.' + end + end + end + context 'salesforce' do let(:extern_uid) { 'my-uid' } let(:provider) { :salesforce } diff --git a/spec/controllers/passwords_controller_spec.rb b/spec/controllers/passwords_controller_spec.rb index ba2c0c0455d..e9883107456 100644 --- a/spec/controllers/passwords_controller_spec.rb +++ b/spec/controllers/passwords_controller_spec.rb @@ -3,11 +3,13 @@ require 'spec_helper' RSpec.describe PasswordsController do - describe '#check_password_authentication_available' do - before do - @request.env["devise.mapping"] = Devise.mappings[:user] - end + include DeviseHelpers + before do + set_devise_mapping(context: @request) + end + + describe '#check_password_authentication_available' do context 'when password authentication is disabled for the web interface and Git' do it 'prevents a password reset' do stub_application_setting(password_authentication_enabled_for_web: false) @@ -30,4 +32,51 @@ RSpec.describe PasswordsController do end end end + + describe '#update' do + render_views + + context 'updating the password' do + subject do + put :update, params: { + user: { + password: password, + password_confirmation: password_confirmation, + reset_password_token: reset_password_token + } + } + end + + let(:password) { User.random_password } + let(:password_confirmation) { password } + let(:reset_password_token) { user.send_reset_password_instructions } + let(:user) { create(:user, password_automatically_set: true, password_expires_at: 10.minutes.ago) } + + context 'password update is successful' do + it 'updates the password-related flags' do + subject + user.reload + + expect(response).to redirect_to(new_user_session_path) + expect(flash[:notice]).to include('password has been changed successfully') + expect(user.password_automatically_set).to eq(false) + expect(user.password_expires_at).to be_nil + end + end + + context 'password update is unsuccessful' do + let(:password_confirmation) { 'not_the_same_as_password' } + + it 'does not update the password-related flags' do + subject + user.reload + + expect(response).to render_template(:edit) + expect(response.body).to have_content("Password confirmation doesn't match Password") + expect(user.password_automatically_set).to eq(true) + expect(user.password_expires_at).not_to be_nil + end + end + end + end end diff --git a/spec/controllers/profiles/accounts_controller_spec.rb b/spec/controllers/profiles/accounts_controller_spec.rb index 52a7a1609a1..c6e7866a659 100644 --- a/spec/controllers/profiles/accounts_controller_spec.rb +++ b/spec/controllers/profiles/accounts_controller_spec.rb @@ -45,5 +45,18 @@ RSpec.describe Profiles::AccountsController do end end end + + describe 'atlassian_oauth2 provider' do + let(:user) { create(:atlassian_user) } + + it 'allows a user to unlink a connected account' do + expect(user.atlassian_identity).not_to be_nil + + delete :unlink, params: { provider: 'atlassian_oauth2' } + + expect(response).to have_gitlab_http_status(:found) + expect(user.reload.atlassian_identity).to be_nil + end + end end end diff --git a/spec/controllers/profiles/emails_controller_spec.rb b/spec/controllers/profiles/emails_controller_spec.rb index 246f8a6cd76..08552cc28fa 100644 --- a/spec/controllers/profiles/emails_controller_spec.rb +++ b/spec/controllers/profiles/emails_controller_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Profiles::EmailsController do - let(:user) { create(:user) } + let_it_be(:user) { create(:user) } before do sign_in(user) @@ -16,36 +16,43 @@ RSpec.describe Profiles::EmailsController do end describe '#create' do - context 'when email address is valid' do - let(:email_params) { { email: "add_email@example.com" } } + let(:email) { 'add_email@example.com' } + let(:params) { { email: { email: email } } } - it 'sends an email confirmation' do - expect { post(:create, params: { email: email_params }) }.to change { ActionMailer::Base.deliveries.size } - end + subject { post(:create, params: params) } + + it 'sends an email confirmation' do + expect { subject }.to change { ActionMailer::Base.deliveries.size } end context 'when email address is invalid' do - let(:email_params) { { email: "test.@example.com" } } + let(:email) { 'invalid.@example.com' } it 'does not send an email confirmation' do - expect { post(:create, params: { email: email_params }) }.not_to change { ActionMailer::Base.deliveries.size } + expect { subject }.not_to change { ActionMailer::Base.deliveries.size } end end end describe '#resend_confirmation_instructions' do - let(:email_params) { { email: "add_email@example.com" } } + let_it_be(:email) { create(:email, user: user) } + let(:params) { { id: email.id } } + + subject { put(:resend_confirmation_instructions, params: params) } it 'resends an email confirmation' do - email = user.emails.create(email: 'add_email@example.com') + expect { subject }.to change { ActionMailer::Base.deliveries.size } - expect { put(:resend_confirmation_instructions, params: { id: email }) }.to change { ActionMailer::Base.deliveries.size } - expect(ActionMailer::Base.deliveries.last.to).to eq [email_params[:email]] - expect(ActionMailer::Base.deliveries.last.subject).to match "Confirmation instructions" + expect(ActionMailer::Base.deliveries.last.to).to eq [email.email] + expect(ActionMailer::Base.deliveries.last.subject).to match 'Confirmation instructions' end - it 'unable to resend an email confirmation' do - expect { put(:resend_confirmation_instructions, params: { id: 1 }) }.not_to change { ActionMailer::Base.deliveries.size } + context 'email does not exist' do + let(:params) { { id: non_existing_record_id } } + + it 'does not send an email confirmation' do + expect { subject }.not_to change { ActionMailer::Base.deliveries.size } + end end end end diff --git a/spec/controllers/profiles/keys_controller_spec.rb b/spec/controllers/profiles/keys_controller_spec.rb index 66f6135df1e..17964ef4a43 100644 --- a/spec/controllers/profiles/keys_controller_spec.rb +++ b/spec/controllers/profiles/keys_controller_spec.rb @@ -20,4 +20,108 @@ RSpec.describe Profiles::KeysController do expect(Key.last.expires_at).to be_like_time(expires_at) end end + + describe "#get_keys" do + describe "non existent user" do + it "does not generally work" do + get :get_keys, params: { username: 'not-existent' } + + expect(response).not_to be_successful + end + end + + describe "user with no keys" do + it "does generally work" do + get :get_keys, params: { username: user.username } + + expect(response).to be_successful + end + + it "renders all keys separated with a new line" do + get :get_keys, params: { username: user.username } + + expect(response.body).to eq("") + end + it "responds with text/plain content type" do + get :get_keys, params: { username: user.username } + expect(response.content_type).to eq("text/plain") + end + end + + describe "user with keys" do + let!(:key) { create(:key, user: user) } + let!(:another_key) { create(:another_key, user: user) } + let!(:deploy_key) { create(:deploy_key, user: user) } + + describe "while signed in" do + before do + sign_in(user) + end + + it "does generally work" do + get :get_keys, params: { username: user.username } + + expect(response).to be_successful + end + + it "renders all non deploy keys separated with a new line" do + get :get_keys, params: { username: user.username } + + expect(response.body).not_to eq('') + expect(response.body).to eq(user.all_ssh_keys.join("\n")) + + expect(response.body).to include(key.key.sub(' dummy@gitlab.com', '')) + expect(response.body).to include(another_key.key.sub(' dummy@gitlab.com', '')) + + expect(response.body).not_to include(deploy_key.key) + end + + it "does not render the comment of the key" do + get :get_keys, params: { username: user.username } + expect(response.body).not_to match(/dummy@gitlab.com/) + end + + it "responds with text/plain content type" do + get :get_keys, params: { username: user.username } + + expect(response.content_type).to eq("text/plain") + end + end + + describe 'when logged out' do + before do + sign_out(user) + end + + it "still does generally work" do + get :get_keys, params: { username: user.username } + + expect(response).to be_successful + end + + it "renders all non deploy keys separated with a new line" do + get :get_keys, params: { username: user.username } + + expect(response.body).not_to eq('') + expect(response.body).to eq(user.all_ssh_keys.join("\n")) + + expect(response.body).to include(key.key.sub(' dummy@gitlab.com', '')) + expect(response.body).to include(another_key.key.sub(' dummy@gitlab.com', '')) + + expect(response.body).not_to include(deploy_key.key) + end + + it "does not render the comment of the key" do + get :get_keys, params: { username: user.username } + expect(response.body).not_to match(/dummy@gitlab.com/) + end + + it "responds with text/plain content type" do + get :get_keys, params: { username: user.username } + + expect(response.content_type).to eq("text/plain") + end + end + end + end end diff --git a/spec/controllers/profiles/notifications_controller_spec.rb b/spec/controllers/profiles/notifications_controller_spec.rb index 40b4c8f0371..90df7cc0991 100644 --- a/spec/controllers/profiles/notifications_controller_spec.rb +++ b/spec/controllers/profiles/notifications_controller_spec.rb @@ -37,7 +37,7 @@ RSpec.describe Profiles::NotificationsController do expect(assigns(:group_notifications).map(&:source_id)).to include(subgroup.id) end - it 'has an N+1 (but should not)' do + it 'does not have an N+1' do sign_in(user) control = ActiveRecord::QueryRecorder.new do @@ -46,10 +46,42 @@ RSpec.describe Profiles::NotificationsController do create_list(:group, 2, parent: group) - # We currently have an N + 1, switch to `not_to` once fixed expect do get :show - end.to exceed_query_limit(control) + end.not_to exceed_query_limit(control) + end + end + + context 'with group notifications' do + let(:notifications_per_page) { 5 } + + let_it_be(:group) { create(:group) } + let_it_be(:subgroups) { create_list(:group, 10, parent: group) } + + before do + group.add_developer(user) + sign_in(user) + allow(Kaminari.config).to receive(:default_per_page).and_return(notifications_per_page) + end + + it 'paginates the groups' do + get :show + + expect(assigns(:group_notifications).count).to eq(5) + end + + context 'when the user is not a member' do + let(:notifications_per_page) { 20 } + + let_it_be(:public_group) { create(:group, :public) } + + it 'does not show public groups', :aggregate_failures do + get :show + + # Let's make sure we're grabbing all groups in one page, just in case + expect(assigns(:user_groups).count).to eq(11) + expect(assigns(:user_groups)).not_to include(public_group) + end end end diff --git a/spec/controllers/profiles/two_factor_auths_controller_spec.rb b/spec/controllers/profiles/two_factor_auths_controller_spec.rb index 1fb0b18622b..59eb33f4bc6 100644 --- a/spec/controllers/profiles/two_factor_auths_controller_spec.rb +++ b/spec/controllers/profiles/two_factor_auths_controller_spec.rb @@ -120,18 +120,46 @@ RSpec.describe Profiles::TwoFactorAuthsController do end describe 'DELETE destroy' do - let(:user) { create(:user, :two_factor) } + subject { delete :destroy } + + context 'for a user that has 2FA enabled' do + let(:user) { create(:user, :two_factor) } + + it 'disables two factor' do + subject + + expect(user.reload.two_factor_enabled?).to eq(false) + end + + it 'redirects to profile_account_path' do + subject + + expect(response).to redirect_to(profile_account_path) + end - it 'disables two factor' do - expect(user).to receive(:disable_two_factor!) + it 'displays a notice on success' do + subject - delete :destroy + expect(flash[:notice]) + .to eq _('Two-factor authentication has been disabled successfully!') + end end - it 'redirects to profile_account_path' do - delete :destroy + context 'for a user that does not have 2FA enabled' do + let(:user) { create(:user) } - expect(response).to redirect_to(profile_account_path) + it 'redirects to profile_account_path' do + subject + + expect(response).to redirect_to(profile_account_path) + end + + it 'displays an alert on failure' do + subject + + expect(flash[:alert]) + .to eq _('Two-factor authentication is not enabled for this user') + end end end end diff --git a/spec/controllers/profiles/webauthn_registrations_controller_spec.rb b/spec/controllers/profiles/webauthn_registrations_controller_spec.rb new file mode 100644 index 00000000000..0c475039963 --- /dev/null +++ b/spec/controllers/profiles/webauthn_registrations_controller_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Profiles::WebauthnRegistrationsController do + let(:user) { create(:user, :two_factor_via_webauthn) } + + before do + sign_in(user) + end + + describe '#destroy' do + it 'deletes the given webauthn registration' do + registration_to_delete = user.webauthn_registrations.first + + expect { delete :destroy, params: { id: registration_to_delete.id } }.to change { user.webauthn_registrations.count }.by(-1) + expect(response).to be_redirect + end + end +end diff --git a/spec/controllers/projects/badges_controller_spec.rb b/spec/controllers/projects/badges_controller_spec.rb index 7e7a630921f..242b2fd3ec6 100644 --- a/spec/controllers/projects/badges_controller_spec.rb +++ b/spec/controllers/projects/badges_controller_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' RSpec.describe Projects::BadgesController do - let(:project) { pipeline.project } - let!(:pipeline) { create(:ci_empty_pipeline) } - let(:user) { create(:user) } + let_it_be(:project, reload: true) { create(:project, :repository) } + let_it_be(:pipeline, reload: true) { create(:ci_empty_pipeline, project: project) } + let_it_be(:user) { create(:user) } shared_examples 'a badge resource' do |badge_type| context 'when pipelines are public' do @@ -137,6 +137,26 @@ RSpec.describe Projects::BadgesController do describe '#pipeline' do it_behaves_like 'a badge resource', :pipeline + + context 'with ignore_skipped param' do + render_views + + before do + pipeline.update!(status: :skipped) + project.add_maintainer(user) + sign_in(user) + end + + it 'returns skipped badge if set to false' do + get_badge(:pipeline, ignore_skipped: false) + expect(response.body).to include('skipped') + end + + it 'does not return skipped badge if set to true' do + get_badge(:pipeline, ignore_skipped: true) + expect(response.body).to include('unknown') + end + end end describe '#coverage' do @@ -148,7 +168,7 @@ RSpec.describe Projects::BadgesController do namespace_id: project.namespace.to_param, project_id: project, ref: pipeline.ref - }.merge(args.slice(:style, :key_text, :key_width)) + }.merge(args.slice(:style, :key_text, :key_width, :ignore_skipped)) get badge, params: params, format: :svg end diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 9fee97f938c..b998dee09b2 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -347,6 +347,13 @@ RSpec.describe Projects::BlobController do end end end + + it_behaves_like 'tracking unique hll events', :track_editor_edit_actions do + subject { put :update, params: default_params, format: format } + + let(:target_id) { 'g_edit_by_sfe' } + let(:expected_type) { instance_of(Integer) } + end end describe 'DELETE destroy' do @@ -436,4 +443,32 @@ RSpec.describe Projects::BlobController do end end end + + describe 'POST create' do + let(:user) { create(:user) } + let(:default_params) do + { + namespace_id: project.namespace, + project_id: project, + id: 'master', + branch_name: 'master', + file_name: 'docs/EXAMPLE_FILE', + content: 'Added changes', + commit_message: 'Create CHANGELOG' + } + end + + before do + project.add_developer(user) + + sign_in(user) + end + + it_behaves_like 'tracking unique hll events', :track_editor_edit_actions do + subject { post :create, params: default_params, format: format } + + let(:target_id) { 'g_edit_by_sfe' } + let(:expected_type) { instance_of(Integer) } + end + end end diff --git a/spec/controllers/projects/ci/lints_controller_spec.rb b/spec/controllers/projects/ci/lints_controller_spec.rb index b3e08292546..22f052e39b7 100644 --- a/spec/controllers/projects/ci/lints_controller_spec.rb +++ b/spec/controllers/projects/ci/lints_controller_spec.rb @@ -5,8 +5,8 @@ require 'spec_helper' RSpec.describe Projects::Ci::LintsController do include StubRequests - let(:project) { create(:project, :repository) } - let(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } before do sign_in(user) @@ -20,7 +20,7 @@ RSpec.describe Projects::Ci::LintsController do get :show, params: { namespace_id: project.namespace, project_id: project } end - it { expect(response).to be_successful } + it { expect(response).to have_gitlab_http_status(:ok) } it 'renders show page' do expect(response).to render_template :show @@ -47,7 +47,8 @@ RSpec.describe Projects::Ci::LintsController do describe 'POST #create' do subject { post :create, params: params } - let(:params) { { namespace_id: project.namespace, project_id: project, content: content } } + let(:format) { :html } + let(:params) { { namespace_id: project.namespace, project_id: project, content: content, format: format } } let(:remote_file_path) { 'https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.gitlab-ci-1.yml' } let(:remote_file_content) do @@ -71,6 +72,20 @@ 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) } + + 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 + end + end + context 'with a valid gitlab-ci.yml' do before do stub_full_request(remote_file_path).to_return(body: remote_file_content) @@ -78,27 +93,30 @@ RSpec.describe Projects::Ci::LintsController do end shared_examples 'returns a successful validation' do - it 'returns successfully' do + before do subject - expect(response).to be_successful end - it 'render show page' do - subject + 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 - subject expect(assigns(:project)).to eq(project) end + + it_behaves_like 'successful request with format json' end context 'using legacy validation (YamlProcessor)' do it_behaves_like 'returns a successful validation' it 'runs validations through YamlProcessor' do - expect(Gitlab::Ci::YamlProcessor).to receive(:new_with_validation_errors).and_call_original + expect(Gitlab::Ci::YamlProcessor).to receive(:new).and_call_original subject end @@ -126,7 +144,7 @@ RSpec.describe Projects::Ci::LintsController do it_behaves_like 'returns a successful validation' it 'runs validations through YamlProcessor' do - expect(Gitlab::Ci::YamlProcessor).to receive(:new_with_validation_errors).and_call_original + expect(Gitlab::Ci::YamlProcessor).to receive(:new).and_call_original subject end @@ -145,22 +163,30 @@ RSpec.describe Projects::Ci::LintsController do before do project.add_developer(user) + subject end - it 'assigns errors' do - subject + it 'assigns result with errors' do + expect(assigns[:result].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 - expect(assigns[:errors]).to eq(['root config contains unknown keys: rubocop']) + 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 errors' do - subject - - expect(assigns[:errors]).to eq(['root config contains unknown keys: rubocop']) + it 'assigns result with errors' do + expect(assigns[:result].errors).to eq(['jobs rubocop config should implement a script: or a trigger: keyword']) end + + it_behaves_like 'successful request with format json' end end @@ -174,6 +200,14 @@ 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/graphs_controller_spec.rb b/spec/controllers/projects/graphs_controller_spec.rb index 49def8f80b0..be89fa0d361 100644 --- a/spec/controllers/projects/graphs_controller_spec.rb +++ b/spec/controllers/projects/graphs_controller_spec.rb @@ -44,7 +44,7 @@ RSpec.describe Projects::GraphsController do context 'when anonymous users can read build report results' do it 'sets the daily coverage options' do - Timecop.freeze do + freeze_time do get(:charts, params: { namespace_id: project.namespace.path, project_id: project.path, id: 'master' }) expect(assigns[:daily_coverage_options]).to eq( diff --git a/spec/controllers/projects/issue_links_controller_spec.rb b/spec/controllers/projects/issue_links_controller_spec.rb new file mode 100644 index 00000000000..bce109b7c79 --- /dev/null +++ b/spec/controllers/projects/issue_links_controller_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::IssueLinksController do + let_it_be(:namespace) { create(:group, :public) } + let_it_be(:project) { create(:project, :public, namespace: namespace) } + let_it_be(:user) { create(:user) } + let_it_be(:issue1) { create(:issue, project: project) } + let_it_be(:issue2) { create(:issue, project: project) } + + describe 'GET #index' do + let_it_be(:issue_link) { create(:issue_link, source: issue1, target: issue2, link_type: 'relates_to') } + + def get_link(user, issue) + sign_in(user) + + params = { + namespace_id: issue.project.namespace.to_param, + project_id: issue.project, + issue_id: issue.iid + } + + get :index, params: params, as: :json + end + + before do + project.add_developer(user) + end + + it 'returns success response' do + get_link(user, issue1) + + expect(response).to have_gitlab_http_status(:ok) + + link = json_response.first + expect(link['id']).to eq(issue2.id) + expect(link['link_type']).to eq('relates_to') + end + end + + describe 'POST #create' do + def create_link(user, issue, target) + sign_in(user) + + post_params = { + namespace_id: issue.project.namespace.to_param, + project_id: issue.project, + issue_id: issue.iid, + issuable_references: [target.to_reference], + link_type: 'relates_to' + } + + post :create, params: post_params, as: :json + end + + before do + project.add_developer(user) + end + + it 'returns success response' do + create_link(user, issue1, issue2) + + expect(response).to have_gitlab_http_status(:ok) + + link = json_response['issuables'].first + expect(link['id']).to eq(issue2.id) + expect(link['link_type']).to eq('relates_to') + end + end +end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index a0e478ef368..ed5198bf015 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -6,9 +6,9 @@ RSpec.describe Projects::IssuesController do include ProjectForksHelper include_context 'includes Spam constants' - let(:project) { create(:project) } - let(:user) { create(:user) } - let(:issue) { create(:issue, project: project) } + let_it_be(:project, reload: true) { create(:project) } + let_it_be(:user, reload: true) { create(:user) } + let(:issue) { create(:issue, project: project) } describe "GET #index" do context 'external issue tracker' do @@ -39,8 +39,8 @@ RSpec.describe Projects::IssuesController do end context 'when project has moved' do - let(:new_project) { create(:project) } - let(:issue) { create(:issue, project: new_project) } + let_it_be(:new_project) { create(:project) } + let_it_be(:issue) { create(:issue, project: new_project) } before do project.route.destroy @@ -82,6 +82,16 @@ RSpec.describe Projects::IssuesController do expect(response).to have_gitlab_http_status(:ok) end + it 'returns only list type issues' do + issue = create(:issue, project: project) + incident = create(:issue, project: project, issue_type: 'incident') + create(:issue, project: project, issue_type: 'test_case') + + get :index, params: { namespace_id: project.namespace, project_id: project } + + expect(assigns(:issues)).to contain_exactly(issue, incident) + end + it "returns 301 if request path doesn't match project path" do get :index, params: { namespace_id: project.namespace, project_id: project.path.upcase } @@ -297,6 +307,7 @@ RSpec.describe Projects::IssuesController do project.add_developer(developer) end + let_it_be(:issue) { create(:issue, project: project) } let(:developer) { user } let(:params) do { @@ -401,7 +412,7 @@ RSpec.describe Projects::IssuesController do end context 'when moving issue to another private project' do - let(:another_project) { create(:project, :private) } + let_it_be(:another_project) { create(:project, :private) } context 'when user has access to move issue' do before do @@ -438,10 +449,10 @@ RSpec.describe Projects::IssuesController do end describe 'PUT #reorder' do - let(:group) { create(:group, projects: [project]) } - let!(:issue1) { create(:issue, project: project, relative_position: 10) } - let!(:issue2) { create(:issue, project: project, relative_position: 20) } - let!(:issue3) { create(:issue, project: project, relative_position: 30) } + let_it_be(:group) { create(:group, projects: [project]) } + let_it_be(:issue1) { create(:issue, project: project, relative_position: 10) } + let_it_be(:issue2) { create(:issue, project: project, relative_position: 20) } + let_it_be(:issue3) { create(:issue, project: project, relative_position: 30) } before do sign_in(user) @@ -657,15 +668,15 @@ RSpec.describe Projects::IssuesController do end describe 'Confidential Issues' do - let(:project) { create(:project_empty_repo, :public) } - let(:assignee) { create(:assignee) } - let(:author) { create(:user) } - let(:non_member) { create(:user) } - let(:member) { create(:user) } - let(:admin) { create(:admin) } - let!(:issue) { create(:issue, project: project) } - let!(:unescaped_parameter_value) { create(:issue, :confidential, project: project, author: author) } - let!(:request_forgery_timing_attack) { create(:issue, :confidential, project: project, assignees: [assignee]) } + let_it_be(:project) { create(:project_empty_repo, :public) } + let_it_be(:assignee) { create(:assignee) } + let_it_be(:author) { create(:user) } + let_it_be(:non_member) { create(:user) } + let_it_be(:member) { create(:user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:unescaped_parameter_value) { create(:issue, :confidential, project: project, author: author) } + let_it_be(:request_forgery_timing_attack) { create(:issue, :confidential, project: project, assignees: [assignee]) } describe 'GET #index' do it 'does not list confidential issues for guests' do @@ -1010,6 +1021,25 @@ RSpec.describe Projects::IssuesController do end end end + + it 'logs the view with Gitlab::Search::RecentIssues' do + sign_in(user) + recent_issues_double = instance_double(::Gitlab::Search::RecentIssues, log_view: nil) + expect(::Gitlab::Search::RecentIssues).to receive(:new).with(user: user).and_return(recent_issues_double) + + go(id: issue.to_param) + + expect(response).to be_successful + expect(recent_issues_double).to have_received(:log_view).with(issue) + end + + context 'when not logged in' do + it 'does not log the view with Gitlab::Search::RecentIssues' do + expect(::Gitlab::Search::RecentIssues).not_to receive(:new) + + go(id: issue.to_param) + end + end end describe 'GET #realtime_changes' do @@ -1077,7 +1107,7 @@ RSpec.describe Projects::IssuesController do end context 'resolving discussions in MergeRequest' do - let(:discussion) { create(:diff_note_on_merge_request).to_discussion } + let_it_be(:discussion) { create(:diff_note_on_merge_request).to_discussion } let(:merge_request) { discussion.noteable } let(:project) { merge_request.source_project } @@ -1376,9 +1406,9 @@ RSpec.describe Projects::IssuesController do end context "when the user is owner" do - let(:owner) { create(:user) } - let(:namespace) { create(:namespace, owner: owner) } - let(:project) { create(:project, namespace: namespace) } + let_it_be(:owner) { create(:user) } + let_it_be(:namespace) { create(:namespace, owner: owner) } + let_it_be(:project) { create(:project, namespace: namespace) } before do sign_in(owner) @@ -1461,7 +1491,8 @@ RSpec.describe Projects::IssuesController do describe 'POST create_merge_request' do let(:target_project_id) { nil } - let(:project) { create(:project, :repository, :public) } + + let_it_be(:project) { create(:project, :repository, :public) } before do project.add_developer(user) @@ -1539,7 +1570,7 @@ RSpec.describe Projects::IssuesController do end describe 'POST #import_csv' do - let(:project) { create(:project, :public) } + let_it_be(:project) { create(:project, :public) } let(:file) { fixture_file_upload('spec/fixtures/csv_comma.csv') } context 'unauthorized' do @@ -1621,7 +1652,7 @@ RSpec.describe Projects::IssuesController do end context 'when not logged in' do - let(:project) { create(:project_empty_repo, :public) } + let(:empty_project) { create(:project_empty_repo, :public) } it 'redirects to the sign in page' do request_csv @@ -1738,7 +1769,7 @@ RSpec.describe Projects::IssuesController do end context 'with cross-reference system note', :request_store do - let(:new_issue) { create(:issue) } + let_it_be(:new_issue) { create(:issue) } let(:cross_reference) { "mentioned in #{new_issue.to_reference(issue.project)}" } before do @@ -1836,7 +1867,7 @@ RSpec.describe Projects::IssuesController do end context 'private project with token authentication' do - let(:private_project) { create(:project, :private) } + let_it_be(:private_project) { create(:project, :private) } it_behaves_like 'authenticates sessionless user', :index, :atom, ignore_incrementing: true do before do @@ -1856,7 +1887,7 @@ RSpec.describe Projects::IssuesController do end context 'public project with token authentication' do - let(:public_project) { create(:project, :public) } + let_it_be(:public_project) { create(:project, :public) } it_behaves_like 'authenticates sessionless user', :index, :atom, public: true do before do diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 818b1c30b37..94cce1964ca 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -201,33 +201,61 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do end context 'when job has artifacts' do - before do - get_show_json - end - context 'with not expiry date' do let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) } it 'exposes needed information' do + get_show_json + expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('job/job_details') expect(json_response['artifact']['download_path']).to match(%r{artifacts/download}) expect(json_response['artifact']['browse_path']).to match(%r{artifacts/browse}) + expect(json_response['artifact']).not_to have_key('keep_path') expect(json_response['artifact']).not_to have_key('expired') expect(json_response['artifact']).not_to have_key('expired_at') end end - context 'with expiry date' do + context 'with expired artifacts' do let(:job) { create(:ci_build, :success, :artifacts, :expired, pipeline: pipeline) } - it 'exposes needed information' do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('job/job_details') - expect(json_response['artifact']).not_to have_key('download_path') - expect(json_response['artifact']).not_to have_key('browse_path') - expect(json_response['artifact']['expired']).to eq(true) - expect(json_response['artifact']['expire_at']).not_to be_empty + context 'when artifacts are unlocked' do + before do + job.pipeline.unlocked! + end + + it 'exposes needed information' do + get_show_json + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['artifact']).not_to have_key('download_path') + expect(json_response['artifact']).not_to have_key('browse_path') + expect(json_response['artifact']).not_to have_key('keep_path') + expect(json_response['artifact']['expired']).to eq(true) + expect(json_response['artifact']['expire_at']).not_to be_empty + expect(json_response['artifact']['locked']).to eq(false) + end + end + + context 'when artifacts are locked' do + before do + job.pipeline.artifacts_locked! + end + + it 'exposes needed information' do + get_show_json + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['artifact']).to have_key('download_path') + expect(json_response['artifact']).to have_key('browse_path') + expect(json_response['artifact']).to have_key('keep_path') + expect(json_response['artifact']['expired']).to eq(true) + expect(json_response['artifact']['expire_at']).not_to be_empty + expect(json_response['artifact']['locked']).to eq(true) + end end end end diff --git a/spec/controllers/projects/merge_requests/content_controller_spec.rb b/spec/controllers/projects/merge_requests/content_controller_spec.rb index 7fb20b4666a..67d3ef6f4f0 100644 --- a/spec/controllers/projects/merge_requests/content_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/content_controller_spec.rb @@ -48,6 +48,9 @@ RSpec.describe Projects::MergeRequests::ContentController do expect(merge_request).to receive(:check_mergeability) do_request(:widget) + + expect(response).to match_response_schema('entities/merge_request_poll_widget') + expect(response.headers['Poll-Interval']).to eq('10000') end context 'merged merge request' do @@ -59,6 +62,21 @@ RSpec.describe Projects::MergeRequests::ContentController do do_request(:widget) expect(response).to match_response_schema('entities/merge_request_poll_widget') + expect(response.headers['Poll-Interval']).to eq('300000') + end + end + + context 'with coverage data' do + let(:merge_request) { create(:merge_request, target_project: project, source_project: project, head_pipeline: head_pipeline) } + let!(:base_pipeline) { create(:ci_empty_pipeline, project: project, ref: merge_request.target_branch, sha: merge_request.diff_base_sha) } + let!(:head_pipeline) { create(:ci_empty_pipeline, project: project) } + let!(:rspec_base) { create(:ci_build, name: 'rspec', coverage: 93.1, pipeline: base_pipeline) } + let!(:rspec_head) { create(:ci_build, name: 'rspec', coverage: 97.1, pipeline: head_pipeline) } + + it 'renders widget MR entity as json' do + do_request(:widget) + + expect(response).to match_response_schema('entities/merge_request_poll_widget') 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 217447c2ad6..91770a00081 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -58,39 +58,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do end end - shared_examples 'persisted preferred diff view cookie' do - context 'with view param' do - before do - go(view: 'parallel') - end - - it 'saves the preferred diff view in a cookie' do - expect(response.cookies['diff_view']).to eq('parallel') - end - - it 'only renders the required view', :aggregate_failures do - diff_files_without_deletions = json_response['diff_files'].reject { |f| f['deleted_file'] } - have_no_inline_diff_lines = satisfy('have no inline diff lines') do |diff_file| - !diff_file.has_key?('highlighted_diff_lines') - end - - expect(diff_files_without_deletions).to all(have_key('parallel_diff_lines')) - expect(diff_files_without_deletions).to all(have_no_inline_diff_lines) - end - end - - context 'when the user cannot view the merge request' do - before do - project.team.truncate - go - end - - it 'returns a 404' do - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - shared_examples "diff note on-demand position creation" do it "updates diff discussion positions" do service = double("service") @@ -155,7 +122,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like 'forked project with submodules' end - it_behaves_like 'persisted preferred diff view cookie' it_behaves_like 'cached diff collection' it_behaves_like 'diff note on-demand position creation' end @@ -414,6 +380,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do def collection_arguments(pagination_data = {}) { + environment: nil, merge_request: merge_request, diff_view: :inline, pagination_data: { @@ -439,18 +406,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like '404 for unexistent diffable' - context 'when feature is disabled' do - before do - stub_feature_flags(diffs_batch_load: false) - end - - it 'returns 404' do - go - - expect(response).to have_gitlab_http_status(:not_found) - end - end - context 'when not authorized' do let(:other_user) { create(:user) } @@ -522,7 +477,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do end it_behaves_like 'forked project with submodules' - it_behaves_like 'persisted preferred diff view cookie' it_behaves_like 'cached diff collection' context 'diff unfolding' do diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 8e1b250cd3c..ee194e5ff2f 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -44,6 +44,16 @@ RSpec.describe Projects::MergeRequestsController do get :show, params: params.merge(extra_params) end + context 'with view param' do + before do + go(view: 'parallel') + end + + it 'saves the preferred diff view in a cookie' do + expect(response.cookies['diff_view']).to eq('parallel') + end + end + context 'when merge request is unchecked' do before do merge_request.mark_as_unchecked! @@ -77,6 +87,22 @@ RSpec.describe Projects::MergeRequestsController do end end + context 'with `default_merge_ref_for_diffs` feature flag enabled' do + before do + stub_feature_flags(default_merge_ref_for_diffs: true) + go + end + + it 'adds the diff_head parameter' do + expect(assigns["endpoint_metadata_url"]).to eq( + diffs_metadata_project_json_merge_request_path( + project, + merge_request, + 'json', + diff_head: true)) + end + end + context 'when diff is missing' do render_views @@ -97,6 +123,16 @@ RSpec.describe Projects::MergeRequestsController do expect(response).to be_successful end + it 'logs the view with Gitlab::Search::RecentMergeRequests' do + recent_merge_requests_double = instance_double(::Gitlab::Search::RecentMergeRequests, log_view: nil) + expect(::Gitlab::Search::RecentMergeRequests).to receive(:new).with(user: user).and_return(recent_merge_requests_double) + + go(format: :html) + + expect(response).to be_successful + expect(recent_merge_requests_double).to have_received(:log_view).with(merge_request) + end + context "that is invalid" do let(:merge_request) { create(:invalid_merge_request, target_project: project, source_project: project) } diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 0c7391c1b9c..fa32d32f552 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -135,10 +135,6 @@ RSpec.describe Projects::MilestonesController do end describe "#destroy" do - before do - stub_feature_flags(track_resource_milestone_change_events: false) - end - it "removes milestone" do expect(issue.milestone_id).to eq(milestone.id) @@ -153,10 +149,6 @@ RSpec.describe Projects::MilestonesController do merge_request.reload expect(merge_request.milestone_id).to eq(nil) - - # Check system note left for milestone removal - last_note = project.issues.find(issue.id).notes[-1].note - expect(last_note).to eq('removed milestone') end end diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 570d65dba4f..8c59b2b378f 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -98,7 +98,7 @@ RSpec.describe Projects::NotesController do let(:page_2_boundary) { microseconds(page_2.last.updated_at + NotesFinder::FETCH_OVERLAP) } around do |example| - Timecop.freeze do + freeze_time do example.run end end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index ef560f6426b..c3be7de25a8 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -43,7 +43,7 @@ RSpec.describe Projects::PipelinesController do end end - it 'executes N+1 queries' do + it 'does not execute N+1 queries' do get_pipelines_index_json control_count = ActiveRecord::QueryRecorder.new do @@ -53,7 +53,7 @@ RSpec.describe Projects::PipelinesController do create_all_pipeline_types # There appears to be one extra query for Pipelines#has_warnings? for some reason - expect { get_pipelines_index_json }.not_to exceed_query_limit(control_count + 7) + expect { get_pipelines_index_json }.not_to exceed_query_limit(control_count + 1) expect(response).to have_gitlab_http_status(:ok) expect(json_response['pipelines'].count).to eq 12 end @@ -763,6 +763,71 @@ RSpec.describe Projects::PipelinesController do end end + describe 'POST create.json' do + let(:project) { create(:project, :public, :repository) } + + subject do + post :create, params: { + namespace_id: project.namespace, + project_id: project, + pipeline: { ref: 'master' } + }, + format: :json + end + + before do + project.add_developer(user) + project.project_feature.update(builds_access_level: feature) + end + + context 'with a valid .gitlab-ci.yml file' do + before do + stub_ci_pipeline_yaml_file(YAML.dump({ + test: { + stage: 'test', + script: 'echo' + } + })) + end + + it 'creates a pipeline' do + expect { subject }.to change { project.ci_pipelines.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['id']).to eq(project.ci_pipelines.last.id) + end + end + + context 'with an invalid .gitlab-ci.yml file' do + before do + stub_ci_pipeline_yaml_file(YAML.dump({ + build: { + stage: 'build', + script: 'echo', + rules: [{ when: 'always' }] + }, + test: { + stage: 'invalid', + script: 'echo' + } + })) + end + + it 'does not create a pipeline' do + expect { subject }.not_to change { project.ci_pipelines.count } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['errors']).to eq([ + 'test job: chosen stage does not exist; available stages are .pre, build, test, deploy, .post' + ]) + expect(json_response['warnings'][0]).to include( + 'jobs:build may allow multiple pipelines to run for a single action due to `rules:when`' + ) + expect(json_response['total_warnings']).to eq(1) + end + end + end + describe 'POST retry.json' do let!(:pipeline) { create(:ci_pipeline, :failed, project: project) } let!(:build) { create(:ci_build, :failed, pipeline: pipeline) } diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index 50f474c0222..8f928cf3382 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Projects::ServicesController do + include JiraServiceHelper + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:service) { create(:jira_service, project: project) } @@ -54,8 +56,7 @@ RSpec.describe Projects::ServicesController do end it 'returns success' do - stub_request(:get, 'http://example.com/rest/api/2/serverInfo') - .to_return(status: 200, body: '{}') + stub_jira_service_test expect(Gitlab::HTTP).to receive(:get).with('/rest/api/2/serverInfo', any_args).and_call_original @@ -66,8 +67,7 @@ RSpec.describe Projects::ServicesController do end it 'returns success' do - stub_request(:get, 'http://example.com/rest/api/2/serverInfo') - .to_return(status: 200, body: '{}') + stub_jira_service_test expect(Gitlab::HTTP).to receive(:get).with('/rest/api/2/serverInfo', any_args).and_call_original @@ -123,7 +123,7 @@ RSpec.describe Projects::ServicesController do expect(response).to be_successful expect(json_response).to eq( 'error' => true, - 'message' => 'Test failed.', + 'message' => 'Connection failed. Please check your settings.', 'service_response' => '', 'test_failed' => true ) @@ -136,7 +136,7 @@ RSpec.describe Projects::ServicesController do let(:service_params) { { active: true } } let(:params) { project_params(service: service_params) } - let(:message) { 'Jira activated.' } + let(:message) { 'Jira settings saved and active.' } let(:redirect_url) { edit_project_service_path(project, service) } before do @@ -175,7 +175,7 @@ RSpec.describe Projects::ServicesController do context 'when param `active` is set to false' do let(:service_params) { { active: false } } - let(:message) { 'Jira settings saved, but not activated.' } + let(:message) { 'Jira settings saved, but not active.' } it_behaves_like 'service update' end @@ -200,6 +200,7 @@ RSpec.describe Projects::ServicesController do describe 'as JSON' do before do + stub_jira_service_test put :update, params: project_params(service: service_params, format: :json) end diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index 191b718af56..ca1b0d2fe15 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/spec/controllers/projects/settings/operations_controller_spec.rb @@ -152,7 +152,8 @@ RSpec.describe Projects::Settings::OperationsController do create_issue: 'false', send_email: 'false', issue_template_key: 'some-other-template', - pagerduty_active: 'true' + pagerduty_active: 'true', + auto_close_incident: 'true' } } end @@ -171,7 +172,7 @@ RSpec.describe Projects::Settings::OperationsController do new_incident_management_settings = params expect(Gitlab::Tracking).to receive(:event) - .with('IncidentManagement::Settings', event_key, kind_of(Hash)) + .with('IncidentManagement::Settings', event_key, any_args) patch :update, params: project_params(project, incident_management_setting_attributes: new_incident_management_settings) @@ -187,6 +188,8 @@ RSpec.describe Projects::Settings::OperationsController do 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' it_behaves_like 'a gitlab tracking event', { pagerduty_active: '0' }, 'disabled_pagerduty_webhook' + it_behaves_like 'a gitlab tracking event', { auto_close_incident: '1' }, 'enabled_auto_close_incident' + it_behaves_like 'a gitlab tracking event', { auto_close_incident: '0' }, 'disabled_auto_close_incident' end end diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb index bb9b556f442..d0e412dfdb8 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -6,12 +6,12 @@ RSpec.describe Projects::SnippetsController do include Gitlab::Routing let_it_be(:user) { create(:user) } - let_it_be(:user2) { create(:user) } - let(:project) { create(:project_empty_repo, :public) } + let_it_be(:other_user) { create(:user) } + let_it_be(:project) { create(:project_empty_repo, :public) } before do project.add_maintainer(user) - project.add_maintainer(user2) + project.add_developer(other_user) end describe 'GET #index' do @@ -47,7 +47,7 @@ RSpec.describe Projects::SnippetsController do end context 'when the project snippet is private' do - let!(:project_snippet) { create(:project_snippet, :private, project: project, author: user) } + let_it_be(:project_snippet) { create(:project_snippet, :private, project: project, author: user) } context 'when anonymous' do it 'does not include the private snippet' do @@ -59,11 +59,9 @@ RSpec.describe Projects::SnippetsController do end context 'when signed in as the author' do - before do + it 'renders the snippet' do sign_in(user) - end - it 'renders the snippet' do subject expect(assigns(:snippets)).to include(project_snippet) @@ -72,11 +70,9 @@ RSpec.describe Projects::SnippetsController do end context 'when signed in as a project member' do - before do - sign_in(user2) - end - it 'renders the snippet' do + sign_in(other_user) + subject expect(assigns(:snippets)).to include(project_snippet) @@ -171,7 +167,6 @@ RSpec.describe Projects::SnippetsController do end describe 'PUT #update' do - let(:project) { create :project, :public } let(:visibility_level) { Snippet::PUBLIC } let(:snippet) { create :project_snippet, author: user, project: project, visibility_level: visibility_level } @@ -190,20 +185,6 @@ RSpec.describe Projects::SnippetsController do snippet.reload end - it_behaves_like 'updating snippet checks blob is binary' do - let_it_be(:title) { 'Foo' } - let(:params) do - { - namespace_id: project.namespace.to_param, - project_id: project, - id: snippet.id, - project_snippet: { title: title } - } - end - - subject { put :update, params: params } - end - context 'when the snippet is spam' do before do allow_next_instance_of(Spam::AkismetService) do |instance| @@ -311,7 +292,7 @@ RSpec.describe Projects::SnippetsController do end describe 'POST #mark_as_spam' do - let(:snippet) { create(:project_snippet, :private, project: project, author: user) } + let_it_be(:snippet) { create(:project_snippet, :private, project: project, author: user) } before do allow_next_instance_of(Spam::AkismetService) do |instance| @@ -359,7 +340,7 @@ RSpec.describe Projects::SnippetsController do %w[show raw].each do |action| describe "GET ##{action}" do context 'when the project snippet is private' do - let(:project_snippet) { create(:project_snippet, :private, :repository, project: project, author: user) } + let_it_be(:project_snippet) { create(:project_snippet, :private, :repository, project: project, author: user) } subject { get action, params: { namespace_id: project.namespace, project_id: project, id: project_snippet.to_param } } @@ -381,7 +362,7 @@ RSpec.describe Projects::SnippetsController do context 'when signed in as a project member' do before do - sign_in(user2) + sign_in(other_user) end it_behaves_like 'successful response' @@ -494,9 +475,8 @@ RSpec.describe Projects::SnippetsController do subject { get :raw, params: params } context 'when repository is empty' do - let(:content) { "first line\r\nsecond line\r\nthird line" } - let(:formatted_content) { content.gsub(/\r\n/, "\n") } - let(:project_snippet) do + let_it_be(:content) { "first line\r\nsecond line\r\nthird line" } + let_it_be(:project_snippet) do create( :project_snippet, :public, :empty_repo, project: project, @@ -505,6 +485,8 @@ RSpec.describe Projects::SnippetsController do ) end + let(:formatted_content) { content.gsub(/\r\n/, "\n") } + context 'CRLF line ending' do before do allow_next_instance_of(Blob) do |instance| @@ -531,7 +513,7 @@ RSpec.describe Projects::SnippetsController do end context 'when repository is not empty' do - let(:project_snippet) do + let_it_be(:project_snippet) do create( :project_snippet, :public, :repository, project: project, @@ -553,7 +535,7 @@ RSpec.describe Projects::SnippetsController do end describe 'DELETE #destroy' do - let!(:snippet) { create(:project_snippet, :private, project: project, author: user) } + let_it_be(:snippet) { create(:project_snippet, :private, project: project, author: user) } let(:params) do { @@ -563,20 +545,22 @@ RSpec.describe Projects::SnippetsController do } end + subject { delete :destroy, params: params } + context 'when current user has ability to destroy the snippet' do before do sign_in(user) end it 'removes the snippet' do - delete :destroy, params: params + subject expect { snippet.reload }.to raise_error(ActiveRecord::RecordNotFound) end context 'when snippet is succesfuly destroyed' do it 'redirects to the project snippets page' do - delete :destroy, params: params + subject expect(response).to redirect_to(project_snippets_path(project)) end @@ -589,7 +573,7 @@ RSpec.describe Projects::SnippetsController do end it 'renders the snippet page with errors' do - delete :destroy, params: params + subject expect(flash[:alert]).to eq('Failed to remove snippet.') expect(response).to redirect_to(project_snippet_path(project, snippet)) @@ -598,32 +582,13 @@ RSpec.describe Projects::SnippetsController do end context 'when current_user does not have ability to destroy the snippet' do - let(:another_user) { create(:user) } - - before do - sign_in(another_user) - end - it 'responds with status 404' do - delete :destroy, params: params + sign_in(other_user) - expect(response).to have_gitlab_http_status(:not_found) - end - end - end + subject - describe 'GET #edit' do - it_behaves_like 'editing snippet checks blob is binary' do - let(:snippet) { create(:project_snippet, :private, project: project, author: user) } - let(:params) do - { - namespace_id: project.namespace, - project_id: project, - id: snippet - } + expect(response).to have_gitlab_http_status(:not_found) end - - subject { get :edit, params: params } end end end diff --git a/spec/controllers/projects/static_site_editor_controller_spec.rb b/spec/controllers/projects/static_site_editor_controller_spec.rb index 384218504b9..7883c7e6f81 100644 --- a/spec/controllers/projects/static_site_editor_controller_spec.rb +++ b/spec/controllers/projects/static_site_editor_controller_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Projects::StaticSiteEditorController do let_it_be(:project) { create(:project, :public, :repository) } let_it_be(:user) { create(:user) } + let(:data) { instance_double(Hash) } describe 'GET show' do let(:default_params) do @@ -16,6 +17,16 @@ RSpec.describe Projects::StaticSiteEditorController do } end + let(:service_response) do + ServiceResponse.success(payload: data) + end + + before do + allow_next_instance_of(::StaticSiteEditor::ConfigService) do |instance| + allow(instance).to receive(:execute).and_return(service_response) + end + end + context 'User roles' do context 'anonymous' do before do @@ -55,18 +66,26 @@ RSpec.describe Projects::StaticSiteEditorController do end it 'assigns a required variables' do - expect(assigns(:config)).to be_a(Gitlab::StaticSiteEditor::Config) + expect(assigns(:data)).to eq(data) expect(assigns(:ref)).to eq('master') expect(assigns(:path)).to eq('README.md') end - context 'when combination of ref and file path is incorrect' do + context 'when combination of ref and path is incorrect' do let(:default_params) { super().merge(id: 'unknown') } it 'responds with 404 page' do expect(response).to have_gitlab_http_status(:not_found) end end + + context 'when invalid config file' do + let(:service_response) { ServiceResponse.error(message: 'invalid') } + + it 'returns 422' do + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + end end end end diff --git a/spec/controllers/projects/todos_controller_spec.rb b/spec/controllers/projects/todos_controller_spec.rb index e1e1e455094..0e35f401bc8 100644 --- a/spec/controllers/projects/todos_controller_spec.rb +++ b/spec/controllers/projects/todos_controller_spec.rb @@ -3,13 +3,14 @@ require('spec_helper') RSpec.describe Projects::TodosController do - let(:user) { create(:user) } - let(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } let(:issue) { create(:issue, project: project) } let(:merge_request) { create(:merge_request, source_project: project) } + let(:design) { create(:design, project: project, issue: issue) } let(:parent) { project } - shared_examples 'project todos actions' do + shared_examples 'issuable todo actions' do it_behaves_like 'todos actions' context 'when not authorized for resource' do @@ -40,7 +41,7 @@ RSpec.describe Projects::TodosController do format: 'html' end - it_behaves_like 'project todos actions' + it_behaves_like 'issuable todo actions' end end @@ -57,7 +58,31 @@ RSpec.describe Projects::TodosController do format: 'html' end - it_behaves_like 'project todos actions' + it_behaves_like 'issuable todo actions' + end + end + + context 'Designs' do + include DesignManagementTestHelpers + + before do + enable_design_management + end + + describe 'POST create' do + def post_create + post :create, + params: { + namespace_id: project.namespace, + project_id: project, + issue_id: issue.id, + issuable_id: design.id, + issuable_type: 'design' + }, + format: 'html' + end + + it_behaves_like 'todos actions' end end end diff --git a/spec/controllers/projects/web_ide_schemas_controller_spec.rb b/spec/controllers/projects/web_ide_schemas_controller_spec.rb new file mode 100644 index 00000000000..fbec941aecc --- /dev/null +++ b/spec/controllers/projects/web_ide_schemas_controller_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::WebIdeSchemasController do + let_it_be(:developer) { create(:user) } + let_it_be(:project) { create(:project, :private, :repository, namespace: developer.namespace) } + + before do + project.add_developer(developer) + + sign_in(user) + end + + describe 'GET show' do + let(:user) { developer } + let(:branch) { 'master' } + + subject do + get :show, params: { + namespace_id: project.namespace.to_param, + project_id: project, + branch: branch, + filename: 'package.json' + } + end + + before do + allow_next_instance_of(::Ide::SchemasConfigService) do |instance| + allow(instance).to receive(:execute).and_return(result) + end + end + + context 'when branch is invalid' do + let(:branch) { 'non-existent' } + + it 'returns 422' do + subject + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + end + + context 'when a valid schema exists' do + let(:result) { { status: :success, schema: { schema: 'Sample Schema' } } } + + it 'returns the schema' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq('{"schema":"Sample Schema"}') + end + end + + context 'when an error occurs parsing the schema' do + let(:result) { { status: :error, message: 'Some error occured' } } + + it 'returns 422 with the error' do + subject + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(response.body).to eq('{"status":"error","message":"Some error occured"}') + end + end + end +end diff --git a/spec/controllers/projects/web_ide_terminals_controller_spec.rb b/spec/controllers/projects/web_ide_terminals_controller_spec.rb index 2ae5899c258..3eb3d5da351 100644 --- a/spec/controllers/projects/web_ide_terminals_controller_spec.rb +++ b/spec/controllers/projects/web_ide_terminals_controller_spec.rb @@ -113,7 +113,7 @@ RSpec.describe Projects::WebIdeTerminalsController do let(:result) { { status: :success } } before do - allow_next_instance_of(::Ci::WebIdeConfigService) do |instance| + allow_next_instance_of(::Ide::TerminalConfigService) do |instance| allow(instance).to receive(:execute).and_return(result) end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index e59493827ba..e4374a8f104 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -555,26 +555,55 @@ RSpec.describe ProjectsController do end shared_examples_for 'updating a project' do + context 'when there is a conflicting project path' do + let(:random_name) { "project-#{SecureRandom.hex(8)}" } + let!(:conflict_project) { create(:project, name: random_name, path: random_name, namespace: project.namespace) } + + it 'does not show any references to the conflicting path' do + expect { update_project(path: random_name) }.not_to change { project.reload.path } + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).not_to include(random_name) + end + end + context 'when only renaming a project path' do - it "sets the repository to the right path after a rename" do + it "doesnt change the disk_path when using hashed storage" do + skip unless project.hashed_storage?(:repository) + + hashed_storage_path = ::Storage::Hashed.new(project).disk_path original_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do project.repository.path end - expect { update_project path: 'renamed_path' } - .to change { project.reload.path } + expect { update_project path: 'renamed_path' }.to change { project.reload.path } expect(project.path).to include 'renamed_path' assign_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do assigns(:repository).path end - if project.hashed_storage?(:repository) - expect(assign_repository_path).to eq(original_repository_path) - else - expect(assign_repository_path).to include(project.path) + expect(original_repository_path).to include(hashed_storage_path) + expect(assign_repository_path).to include(hashed_storage_path) + end + + it "upgrades and move project to hashed storage when project was originally legacy" do + skip if project.hashed_storage?(:repository) + + hashed_storage_path = Storage::Hashed.new(project).disk_path + original_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.path + end + + expect { update_project path: 'renamed_path' }.to change { project.reload.path } + expect(project.path).to include 'renamed_path' + + assign_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + assigns(:repository).path end + expect(original_repository_path).not_to include(hashed_storage_path) + expect(assign_repository_path).to include(hashed_storage_path) expect(response).to have_gitlab_http_status(:found) end end diff --git a/spec/controllers/registrations/experience_levels_controller_spec.rb b/spec/controllers/registrations/experience_levels_controller_spec.rb index cd46cec1641..ee1acf3d93d 100644 --- a/spec/controllers/registrations/experience_levels_controller_spec.rb +++ b/spec/controllers/registrations/experience_levels_controller_spec.rb @@ -59,8 +59,6 @@ RSpec.describe Registrations::ExperienceLevelsController do end context 'when user is successfully updated' do - it { is_expected.to set_flash[:message].to('Welcome! You have signed up successfully.') } - context 'when no experience_level is sent' do before do user.user_preference.update_attribute(:experience_level, :novice) diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 2c766035d87..f80e18df22e 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -128,7 +128,7 @@ RSpec.describe RegistrationsController do post(:create, params: user_params) expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email]) - expect(response).to redirect_to(dashboard_projects_path) + expect(response).to redirect_to(users_sign_up_welcome_path) end end end @@ -164,10 +164,10 @@ RSpec.describe RegistrationsController do expect(flash[:alert]).to eq(_('There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.')) end - it 'redirects to the dashboard when the reCAPTCHA is solved' do + it 'redirects to the welcome page when the reCAPTCHA is solved' do post(:create, params: user_params) - expect(flash[:notice]).to eq(I18n.t('devise.registrations.signed_up')) + expect(response).to redirect_to(users_sign_up_welcome_path) end end @@ -316,6 +316,12 @@ RSpec.describe RegistrationsController do stub_experiment(signup_flow: true, terms_opt_in: true) end + it 'records user for the terms_opt_in experiment' do + expect(controller).to receive(:record_experiment_user).with(:terms_opt_in) + + subject + end + context 'when user is not part of the experiment' do before do stub_experiment_for_user(signup_flow: true, terms_opt_in: false) @@ -458,42 +464,35 @@ RSpec.describe RegistrationsController do describe '#welcome' do subject { get :welcome } - context 'signup_flow experiment enabled' do - before do - stub_experiment_for_user(signup_flow: true) - end + it 'renders the devise_experimental_separate_sign_up_flow layout' do + sign_in(create(:user)) - 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 - expected_layout = Gitlab.ee? ? :checkout : :devise_experimental_separate_sign_up_flow + expect(subject).to render_template(expected_layout) + end - expect(subject).to render_template(expected_layout) + context '2FA is required from group' do + before do + user = create(:user, require_two_factor_authentication_from_group: true) + sign_in(user) 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 + it 'does not perform a redirect' do + expect(subject).not_to redirect_to(profile_two_factor_auth_path) end end + end - context 'signup_flow experiment disabled' do - before do - sign_in(create(:user)) - stub_experiment_for_user(signup_flow: false) - end - - it 'renders the devise layout' do - expected_layout = Gitlab.ee? ? :checkout : :devise + describe '#update_registration' do + subject(:update_registration) do + patch :update_registration, params: { user: { role: 'software_developer', setup_for_company: 'false' } } + end - expect(subject).to render_template(expected_layout) - end + before do + sign_in(create(:user)) end + + it { is_expected.to redirect_to(dashboard_projects_path)} end end diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index a41ff28841d..f244392bbad 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -95,49 +95,77 @@ RSpec.describe SearchController do using RSpec::Parameterized::TableSyntax render_views - it 'omits pipeline status from load' do - project = create(:project, :public) - expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects) + context 'when block_anonymous_global_searches is disabled' do + before do + stub_feature_flags(block_anonymous_global_searches: false) + end - get :show, params: { scope: 'projects', search: project.name } + it 'omits pipeline status from load' do + project = create(:project, :public) + expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects) - expect(assigns[:search_objects].first).to eq project - end + get :show, params: { scope: 'projects', search: project.name } - context 'check search term length' do - let(:search_queries) do - char_limit = SearchService::SEARCH_CHAR_LIMIT - term_limit = SearchService::SEARCH_TERM_LIMIT - { - chars_under_limit: ('a' * (char_limit - 1)), - chars_over_limit: ('a' * (char_limit + 1)), - terms_under_limit: ('abc ' * (term_limit - 1)), - terms_over_limit: ('abc ' * (term_limit + 1)) - } + expect(assigns[:search_objects].first).to eq project end - where(:string_name, :expectation) do - :chars_under_limit | :not_to_set_flash - :chars_over_limit | :set_chars_flash - :terms_under_limit | :not_to_set_flash - :terms_over_limit | :set_terms_flash - end + context 'check search term length' do + let(:search_queries) do + char_limit = SearchService::SEARCH_CHAR_LIMIT + term_limit = SearchService::SEARCH_TERM_LIMIT + { + chars_under_limit: ('a' * (char_limit - 1)), + chars_over_limit: ('a' * (char_limit + 1)), + terms_under_limit: ('abc ' * (term_limit - 1)), + terms_over_limit: ('abc ' * (term_limit + 1)) + } + end + + where(:string_name, :expectation) do + :chars_under_limit | :not_to_set_flash + :chars_over_limit | :set_chars_flash + :terms_under_limit | :not_to_set_flash + :terms_over_limit | :set_terms_flash + end - with_them do - it do - get :show, params: { scope: 'projects', search: search_queries[string_name] } - - case expectation - when :not_to_set_flash - expect(controller).not_to set_flash[:alert] - when :set_chars_flash - expect(controller).to set_flash[:alert].to(/characters/) - when :set_terms_flash - expect(controller).to set_flash[:alert].to(/terms/) + with_them do + it do + get :show, params: { scope: 'projects', search: search_queries[string_name] } + + case expectation + when :not_to_set_flash + expect(controller).not_to set_flash[:alert] + when :set_chars_flash + expect(controller).to set_flash[:alert].to(/characters/) + when :set_terms_flash + expect(controller).to set_flash[:alert].to(/terms/) + end end end end end + + context 'when block_anonymous_global_searches is enabled' do + context 'for unauthenticated user' do + before do + sign_out(user) + end + + it 'redirects to login page' do + get :show, params: { scope: 'projects', search: '*' } + + expect(response).to redirect_to new_user_session_path + end + end + + context 'for authenticated user' do + it 'succeeds' do + get :show, params: { scope: 'projects', search: '*' } + + expect(response).to have_gitlab_http_status(:ok) + end + end + end end it 'finds issue comments' do @@ -149,6 +177,19 @@ RSpec.describe SearchController do expect(assigns[:search_objects].first).to eq note end + context 'unique users tracking' do + before do + allow(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event) + end + + it_behaves_like 'tracking unique hll events', :search_track_unique_users do + subject { get :show, params: { scope: 'projects', search: 'term' }, format: format } + + let(:target_id) { 'i_search_total' } + let(:expected_type) { instance_of(String) } + end + end + context 'on restricted projects' do context 'when signed out' do before do diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index f2e16baaccf..688539f2a03 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -6,11 +6,11 @@ RSpec.describe SessionsController do include DeviseHelpers include LdapHelpers - describe '#new' do - before do - set_devise_mapping(context: @request) - end + before do + set_devise_mapping(context: @request) + end + describe '#new' do context 'when auto sign-in is enabled' do before do stub_omniauth_setting(auto_sign_in_with_provider: :saml) @@ -59,13 +59,19 @@ RSpec.describe SessionsController do end end end - end - describe '#create' do - before do - set_devise_mapping(context: @request) + it "redirects correctly for referer on same host with params" do + host = "test.host" + search_path = "/search?search=seed_project" + request.headers[:HTTP_REFERER] = "http://#{host}#{search_path}" + + get(:new, params: { redirect_to_referer: :yes }) + + expect(controller.stored_location_for(:redirect)).to eq(search_path) end + end + describe '#create' do it_behaves_like 'known sign in' do let(:user) { create(:user) } let(:post_action) { post(:create, params: { user: { login: user.username, password: user.password } }) } @@ -130,8 +136,13 @@ RSpec.describe SessionsController do end it 'creates an audit log record' do - expect { post(:create, params: { user: user_params }) }.to change { SecurityEvent.count }.by(1) - expect(SecurityEvent.last.details[:with]).to eq('standard') + expect { post(:create, params: { user: user_params }) }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details[:with]).to eq('standard') + end + + it 'creates an authentication event record' do + expect { post(:create, params: { user: user_params }) }.to change { AuthenticationEvent.count }.by(1) + expect(AuthenticationEvent.last.provider).to eq('standard') end include_examples 'user login request with unique ip limit', 302 do @@ -229,7 +240,7 @@ RSpec.describe SessionsController do context 'when there are more than 5 anonymous session with the same IP' do before do - allow(Gitlab::AnonymousSession).to receive_message_chain(:new, :stored_sessions).and_return(6) + allow(Gitlab::AnonymousSession).to receive_message_chain(:new, :session_count).and_return(6) end it 'displays an error when the reCAPTCHA is not solved' do @@ -241,7 +252,7 @@ RSpec.describe SessionsController do end it 'successfully logs in a user when reCAPTCHA is solved' do - expect(Gitlab::AnonymousSession).to receive_message_chain(:new, :cleanup_session_per_ip_entries) + expect(Gitlab::AnonymousSession).to receive_message_chain(:new, :cleanup_session_per_ip_count) succesful_login(user_params) @@ -398,8 +409,13 @@ RSpec.describe SessionsController do end it "creates an audit log record" do - expect { authenticate_2fa(login: user.username, otp_attempt: user.current_otp) }.to change { SecurityEvent.count }.by(1) - expect(SecurityEvent.last.details[:with]).to eq("two-factor") + expect { authenticate_2fa(login: user.username, otp_attempt: user.current_otp) }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details[:with]).to eq("two-factor") + end + + it "creates an authentication event record" do + expect { authenticate_2fa(login: user.username, otp_attempt: user.current_otp) }.to change { AuthenticationEvent.count }.by(1) + expect(AuthenticationEvent.last.provider).to eq("two-factor") end end @@ -410,6 +426,10 @@ RSpec.describe SessionsController do post(:create, params: { user: user_params }, session: { otp_user_id: user.id }) end + before do + stub_feature_flags(webauthn: false) + end + context 'remember_me field' do it 'sets a remember_user_token cookie when enabled' do allow(U2fRegistration).to receive(:authenticate).and_return(true) @@ -435,31 +455,21 @@ RSpec.describe SessionsController do it "creates an audit log record" do allow(U2fRegistration).to receive(:authenticate).and_return(true) - expect { authenticate_2fa_u2f(login: user.username, device_response: "{}") }.to change { SecurityEvent.count }.by(1) - expect(SecurityEvent.last.details[:with]).to eq("two-factor-via-u2f-device") + expect { authenticate_2fa_u2f(login: user.username, device_response: "{}") }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details[:with]).to eq("two-factor-via-u2f-device") end - end - end - - describe "#new" do - before do - set_devise_mapping(context: @request) - end - it "redirects correctly for referer on same host with params" do - host = "test.host" - search_path = "/search?search=seed_project" - request.headers[:HTTP_REFERER] = "http://#{host}#{search_path}" - - get(:new, params: { redirect_to_referer: :yes }) + it "creates an authentication event record" do + allow(U2fRegistration).to receive(:authenticate).and_return(true) - expect(controller.stored_location_for(:redirect)).to eq(search_path) + expect { authenticate_2fa_u2f(login: user.username, device_response: "{}") }.to change { AuthenticationEvent.count }.by(1) + expect(AuthenticationEvent.last.provider).to eq("two-factor-via-u2f-device") + end end end context 'when login fails' do before do - set_devise_mapping(context: @request) @request.env["warden.options"] = { action: 'unauthenticated' } end @@ -473,10 +483,6 @@ RSpec.describe SessionsController do describe '#set_current_context' do let_it_be(:user) { create(:user) } - before do - set_devise_mapping(context: @request) - end - context 'when signed in' do before do sign_in(user) @@ -530,4 +536,21 @@ RSpec.describe SessionsController do end end end + + describe '#destroy' do + before do + sign_in(user) + end + + context 'for a user whose password has expired' do + let(:user) { create(:user, password_expires_at: 2.days.ago) } + + it 'allows to sign out successfully' do + delete :destroy + + expect(response).to redirect_to(new_user_session_path) + expect(controller.current_user).to be_nil + end + end + end end diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 92370b3381a..6517922d92a 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -334,12 +334,6 @@ RSpec.describe SnippetsController do snippet.reload end - it_behaves_like 'updating snippet checks blob is binary' do - let_it_be(:title) { 'Foo' } - - subject { put :update, params: { id: snippet, personal_snippet: { title: title } } } - end - context 'when the snippet is spam' do before do allow_next_instance_of(Spam::AkismetService) do |instance| @@ -746,12 +740,4 @@ RSpec.describe SnippetsController do end end end - - describe 'GET #edit' do - it_behaves_like 'editing snippet checks blob is binary' do - let_it_be(:snippet) { create(:personal_snippet, :public, :repository, author: user) } - - subject { get :edit, params: { id: snippet } } - end - end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 99c3b82bd0d..bec4b24484a 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -114,71 +114,6 @@ RSpec.describe UsersController do end end - describe "#ssh_keys" do - describe "non existent user" do - it "does not generally work" do - get :ssh_keys, params: { username: 'not-existent' } - - expect(response).not_to be_successful - end - end - - describe "user with no keys" do - it "does generally work" do - get :ssh_keys, params: { username: user.username } - - expect(response).to be_successful - end - - it "renders all keys separated with a new line" do - get :ssh_keys, params: { username: user.username } - - expect(response.body).to eq("") - end - - it "responds with text/plain content type" do - get :ssh_keys, params: { username: user.username } - expect(response.content_type).to eq("text/plain") - end - end - - describe "user with keys" do - let!(:key) { create(:key, user: user) } - let!(:another_key) { create(:another_key, user: user) } - let!(:deploy_key) { create(:deploy_key, user: user) } - - it "does generally work" do - get :ssh_keys, params: { username: user.username } - - expect(response).to be_successful - end - - it "renders all non deploy keys separated with a new line" do - get :ssh_keys, params: { username: user.username } - - expect(response.body).not_to eq('') - expect(response.body).to eq(user.all_ssh_keys.join("\n")) - - expect(response.body).to include(key.key.sub(' dummy@gitlab.com', '')) - expect(response.body).to include(another_key.key.sub(' dummy@gitlab.com', '')) - - expect(response.body).not_to include(deploy_key.key) - end - - it "does not render the comment of the key" do - get :ssh_keys, params: { username: user.username } - - expect(response.body).not_to match(/dummy@gitlab.com/) - end - - it "responds with text/plain content type" do - get :ssh_keys, params: { username: user.username } - - expect(response.content_type).to eq("text/plain") - end - end - end - describe 'GET #calendar' do context 'for user' do let(:project) { create(:project) } |