diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-10 00:06:44 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-10 00:06:44 +0000 |
commit | 308146dc398fd4c13453048105498018459e0985 (patch) | |
tree | d843eb63c1672e4b18c483907e2cd4aa7fca708e /spec | |
parent | 4b28d5ae770c6bd332283a3f13ceae06329c409b (diff) | |
download | gitlab-ce-308146dc398fd4c13453048105498018459e0985.tar.gz |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
24 files changed, 1133 insertions, 26 deletions
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 28d53a7f830..1d1653e67e3 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -60,6 +60,96 @@ describe Admin::UsersController do end end + describe 'PUT #activate' do + shared_examples 'a request that activates the user' do + it 'activates the user' do + put :activate, params: { id: user.username } + user.reload + expect(user.active?).to be_truthy + expect(flash[:notice]).to eq('Successfully activated') + end + end + + context 'for a deactivated user' do + before do + user.deactivate + end + + it_behaves_like 'a request that activates the user' + end + + context 'for an active user' do + it_behaves_like 'a request that activates the user' + end + + context 'for a blocked user' do + before do + user.block + end + + it 'does not activate the user' do + put :activate, params: { id: user.username } + user.reload + expect(user.active?).to be_falsey + expect(flash[:notice]).to eq('Error occurred. A blocked user must be unblocked to be activated') + end + end + end + + describe 'PUT #deactivate' do + shared_examples 'a request that deactivates the user' do + it 'deactivates the user' do + put :deactivate, params: { id: user.username } + user.reload + expect(user.deactivated?).to be_truthy + expect(flash[:notice]).to eq('Successfully deactivated') + end + end + + context 'for an active user' do + let(:activity) { {} } + let(:user) { create(:user, **activity) } + + context 'with no recent activity' do + let(:activity) { { last_activity_on: ::User::MINIMUM_INACTIVE_DAYS.next.days.ago } } + + it_behaves_like 'a request that deactivates the user' + end + + context 'with recent activity' do + let(:activity) { { last_activity_on: ::User::MINIMUM_INACTIVE_DAYS.pred.days.ago } } + + it 'does not deactivate the user' do + put :deactivate, params: { id: user.username } + user.reload + expect(user.deactivated?).to be_falsey + expect(flash[:notice]).to eq("The user you are trying to deactivate has been active in the past 14 days and cannot be deactivated") + end + end + end + + context 'for a deactivated user' do + before do + user.deactivate + end + + it_behaves_like 'a request that deactivates the user' + end + + context 'for a blocked user' do + before do + user.block + end + + it 'does not deactivate the user' do + put :deactivate, params: { id: user.username } + user.reload + expect(user.deactivated?).to be_falsey + expect(flash[:notice]).to eq('Error occurred. A blocked user cannot be deactivated') + end + end + end + describe 'PUT block/:id' do it 'blocks user' do put :block, params: { id: user.username } diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index e87c5e59b3b..5e33421854b 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -460,6 +460,25 @@ describe ApplicationController do end end + context 'deactivated user' do + controller(described_class) do + def index + render html: 'authenticated' + end + end + + before do + sign_in user + user.deactivate + end + + it 'signs out a deactivated user' do + get :index + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to eq('Your account has been deactivated by your administrator. Please log back in to reactivate your account.') + end + end + context 'terms' do controller(described_class) do def index diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index 9371c434f49..521dbe7ee23 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -18,6 +18,28 @@ describe OmniauthCallbacksController, type: :controller do Rails.application.env_config['omniauth.auth'] = @original_env_config_omniauth_auth end + context 'a deactivated user' do + let(:provider) { :github } + let(:extern_uid) { 'my-uid' } + + before do + user.deactivate! + post provider + end + + it 'allows sign in' do + expect(request.env['warden']).to be_authenticated + end + + it 'activates the user' do + expect(user.reload.active?).to be_truthy + end + + it 'shows reactivation flash message after logging in' do + expect(flash[:notice]).to eq('Welcome back! Your account had been deactivated due to inactivity but is now reactivated.') + end + end + context 'when the user is on the last sign in attempt' do let(:extern_uid) { 'my-uid' } diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index ca3c802777b..302de3246c2 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -258,5 +258,26 @@ describe Projects::MergeRequests::DiffsController do it_behaves_like 'forked project with submodules' it_behaves_like 'persisted preferred diff view cookie' + + context 'diff unfolding' do + let!(:unfoldable_diff_note) do + create(:diff_note_on_merge_request, :folded_position, project: project, noteable: merge_request) + end + + let!(:diff_note) do + create(:diff_note_on_merge_request, project: project, noteable: merge_request) + end + + it 'unfolds correct diff file positions' do + expect_next_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiffBatch) do |instance| + expect(instance) + .to receive(:unfold_diff_files) + .with([unfoldable_diff_note.position]) + .and_call_original + end + + go + end + end end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 68b7bf61231..2108cf1c8ae 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -61,6 +61,25 @@ describe SessionsController do expect(subject.current_user).to eq user end + context 'a deactivated user' do + before do + user.deactivate! + post(:create, params: { user: user_params }) + end + + it 'is allowed to login' do + expect(subject.current_user).to eq user + end + + it 'activates the user' do + expect(subject.current_user.active?).to be_truthy + end + + it 'shows reactivation flash message after logging in' do + expect(flash[:notice]).to eq('Welcome back! Your account had been deactivated due to inactivity but is now reactivated.') + end + end + context 'with password authentication disabled' do before do stub_application_setting(password_authentication_enabled_for_web: false) diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index 8304b718136..2f02acca794 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -62,6 +62,18 @@ FactoryBot.define do ) end + trait :folded_position do + position do + Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: 1, + new_line: 1, + diff_refs: diff_refs + ) + end + end + trait :resolved do resolved_at { Time.now } resolved_by { create(:user) } diff --git a/spec/features/admin/admin_users_spec.rb b/spec/features/admin/admin_users_spec.rb index ebf71d8c9da..29f29e58917 100644 --- a/spec/features/admin/admin_users_spec.rb +++ b/spec/features/admin/admin_users_spec.rb @@ -31,7 +31,8 @@ describe "Admin::Users" do expect(page).to have_content(current_user.last_activity_on.strftime("%e %b, %Y")) expect(page).to have_content(user.email) expect(page).to have_content(user.name) - expect(page).to have_link('Block', href: block_admin_user_path(user)) + expect(page).to have_button('Block') + expect(page).to have_button('Deactivate') expect(page).to have_button('Delete user') expect(page).to have_button('Delete user and contributions') end @@ -277,7 +278,8 @@ describe "Admin::Users" do expect(page).to have_content(user.email) expect(page).to have_content(user.name) expect(page).to have_content(user.id) - expect(page).to have_link('Block user', href: block_admin_user_path(user)) + expect(page).to have_button('Deactivate user') + expect(page).to have_button('Block user') expect(page).to have_button('Delete user') expect(page).to have_button('Delete user and contributions') end diff --git a/spec/finders/user_finder_spec.rb b/spec/finders/user_finder_spec.rb index 4771b878b8e..c20d7850d68 100644 --- a/spec/finders/user_finder_spec.rb +++ b/spec/finders/user_finder_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe UserFinder do - set(:user) { create(:user) } + let_it_be(:user) { create(:user) } describe '#find_by_id' do context 'when the user exists' do @@ -24,7 +24,7 @@ describe UserFinder do context 'when the user does not exist' do it 'returns nil' do - found = described_class.new(1).find_by_id + found = described_class.new(-1).find_by_id expect(found).to be_nil end @@ -84,7 +84,7 @@ describe UserFinder do context 'when the user does not exist' do it 'returns nil' do - found = described_class.new(1).find_by_id_or_username + found = described_class.new(-1).find_by_id_or_username expect(found).to be_nil end @@ -110,7 +110,7 @@ describe UserFinder do context 'when the user does not exist' do it 'raises ActiveRecord::RecordNotFound' do - finder = described_class.new(1) + finder = described_class.new(-1) expect { finder.find_by_id! }.to raise_error(ActiveRecord::RecordNotFound) end @@ -170,10 +170,32 @@ describe UserFinder do context 'when the user does not exist' do it 'raises ActiveRecord::RecordNotFound' do - finder = described_class.new(1) + finder = described_class.new(-1) expect { finder.find_by_id_or_username! }.to raise_error(ActiveRecord::RecordNotFound) end end end + + describe '#find_by_ssh_key_id' do + let_it_be(:ssh_key) { create(:key, user: user) } + + it 'returns the user when passing the ssh key id' do + found = described_class.new(ssh_key.id).find_by_ssh_key_id + + expect(found).to eq(user) + end + + it 'returns the user when passing the ssh key id (string)' do + found = described_class.new(ssh_key.id.to_s).find_by_ssh_key_id + + expect(found).to eq(user) + end + + it 'returns nil when the id does not exist' do + found = described_class.new(-1).find_by_ssh_key_id + + expect(found).to be_nil + end + end end diff --git a/spec/frontend/pages/admin/users/components/__snapshots__/delete_user_modal_spec.js.snap b/spec/frontend/pages/admin/users/components/__snapshots__/delete_user_modal_spec.js.snap new file mode 100644 index 00000000000..78a736a9060 --- /dev/null +++ b/spec/frontend/pages/admin/users/components/__snapshots__/delete_user_modal_spec.js.snap @@ -0,0 +1,63 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`User Operation confirmation modal renders modal with form included 1`] = ` +<div> + <p> + content + </p> + + <p> + To confirm, type + <code> + username + </code> + </p> + + <form + action="delete-url" + method="post" + > + <input + name="_method" + type="hidden" + value="delete" + /> + + <input + name="authenticity_token" + type="hidden" + value="csrf" + /> + + <glforminput-stub + autocomplete="off" + autofocus="" + name="username" + type="text" + value="" + /> + </form> + + <glbutton-stub + variant="secondary" + > + Cancel + </glbutton-stub> + + <glbutton-stub + disabled="true" + variant="warning" + > + + secondaryAction + + </glbutton-stub> + + <glbutton-stub + disabled="true" + variant="danger" + > + action + </glbutton-stub> +</div> +`; diff --git a/spec/frontend/pages/admin/users/components/__snapshots__/user_operation_confirmation_modal_spec.js.snap b/spec/frontend/pages/admin/users/components/__snapshots__/user_operation_confirmation_modal_spec.js.snap new file mode 100644 index 00000000000..4a3989f5192 --- /dev/null +++ b/spec/frontend/pages/admin/users/components/__snapshots__/user_operation_confirmation_modal_spec.js.snap @@ -0,0 +1,33 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`User Operation confirmation modal renders modal with form included 1`] = ` +<glmodal-stub + modalclass="" + modalid="user-operation-modal" + ok-title="action" + ok-variant="warning" + title="title" + titletag="h4" +> + <form + action="/url" + method="post" + > + <span> + content + </span> + + <input + name="_method" + type="hidden" + value="method" + /> + + <input + name="authenticity_token" + type="hidden" + value="csrf" + /> + </form> +</glmodal-stub> +`; diff --git a/spec/frontend/pages/admin/users/components/delete_user_modal_spec.js b/spec/frontend/pages/admin/users/components/delete_user_modal_spec.js new file mode 100644 index 00000000000..57802a41bb5 --- /dev/null +++ b/spec/frontend/pages/admin/users/components/delete_user_modal_spec.js @@ -0,0 +1,85 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlButton, GlFormInput } from '@gitlab/ui'; +import DeleteUserModal from '~/pages/admin/users/components/delete_user_modal.vue'; +import ModalStub from './stubs/modal_stub'; + +describe('User Operation confirmation modal', () => { + let wrapper; + + const findButton = variant => + wrapper + .findAll(GlButton) + .filter(w => w.attributes('variant') === variant) + .at(0); + + const createComponent = (props = {}) => { + wrapper = shallowMount(DeleteUserModal, { + propsData: { + title: 'title', + content: 'content', + action: 'action', + secondaryAction: 'secondaryAction', + deleteUserUrl: 'delete-url', + blockUserUrl: 'block-url', + username: 'username', + csrfToken: 'csrf', + ...props, + }, + stubs: { + GlModal: ModalStub, + }, + sync: false, + }); + }; + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + it('renders modal with form included', () => { + createComponent(); + expect(wrapper.element).toMatchSnapshot(); + }); + + it.each` + variant | prop | action + ${'danger'} | ${'deleteUserUrl'} | ${'delete'} + ${'warning'} | ${'blockUserUrl'} | ${'block'} + `('closing modal with $variant button triggers $action', ({ variant, prop }) => { + createComponent(); + const form = wrapper.find('form'); + jest.spyOn(form.element, 'submit').mockReturnValue(); + const modalButton = findButton(variant); + modalButton.vm.$emit('click'); + return wrapper.vm.$nextTick().then(() => { + expect(form.element.submit).toHaveBeenCalled(); + expect(form.element.action).toContain(wrapper.props(prop)); + expect(new FormData(form.element).get('authenticity_token')).toEqual( + wrapper.props('csrfToken'), + ); + }); + }); + + it('disables buttons by default', () => { + createComponent(); + const blockButton = findButton('warning'); + const deleteButton = findButton('danger'); + expect(blockButton.attributes().disabled).toBeTruthy(); + expect(deleteButton.attributes().disabled).toBeTruthy(); + }); + + it('enables button when username is typed', () => { + createComponent({ + username: 'some-username', + }); + wrapper.find(GlFormInput).vm.$emit('input', 'some-username'); + const blockButton = findButton('warning'); + const deleteButton = findButton('danger'); + + return wrapper.vm.$nextTick().then(() => { + expect(blockButton.attributes().disabled).toBeFalsy(); + expect(deleteButton.attributes().disabled).toBeFalsy(); + }); + }); +}); diff --git a/spec/frontend/pages/admin/users/components/stubs/modal_stub.js b/spec/frontend/pages/admin/users/components/stubs/modal_stub.js new file mode 100644 index 00000000000..4dc55e909a0 --- /dev/null +++ b/spec/frontend/pages/admin/users/components/stubs/modal_stub.js @@ -0,0 +1,23 @@ +const ModalStub = { + inheritAttrs: false, + name: 'glmodal-stub', + data() { + return { + showWasCalled: false, + }; + }, + methods: { + show() { + this.showWasCalled = true; + }, + hide() {}, + }, + render(h) { + const children = [this.$slots.default, this.$slots['modal-footer']] + .filter(Boolean) + .reduce((acc, nodes) => acc.concat(nodes), []); + return h('div', children); + }, +}; + +export default ModalStub; diff --git a/spec/frontend/pages/admin/users/components/user_modal_manager_spec.js b/spec/frontend/pages/admin/users/components/user_modal_manager_spec.js new file mode 100644 index 00000000000..7653fffc502 --- /dev/null +++ b/spec/frontend/pages/admin/users/components/user_modal_manager_spec.js @@ -0,0 +1,148 @@ +import { shallowMount } from '@vue/test-utils'; +import UserModalManager from '~/pages/admin/users/components/user_modal_manager.vue'; +import ModalStub from './stubs/modal_stub'; + +describe('Users admin page Modal Manager', () => { + const modalConfiguration = { + action1: { + title: 'action1', + content: 'Action Modal 1', + }, + action2: { + title: 'action2', + content: 'Action Modal 2', + }, + }; + + const actionModals = { + action1: ModalStub, + action2: ModalStub, + }; + + let wrapper; + + const createComponent = (props = {}) => { + wrapper = shallowMount(UserModalManager, { + propsData: { + actionModals, + modalConfiguration, + csrfToken: 'dummyCSRF', + ...props, + }, + stubs: { + dummyComponent1: true, + dummyComponent2: true, + }, + sync: false, + }); + }; + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + describe('render behavior', () => { + it('does not renders modal when initialized', () => { + createComponent(); + expect(wrapper.find({ ref: 'modal' }).exists()).toBeFalsy(); + }); + + it('throws if non-existing action is requested', () => { + createComponent(); + expect(() => wrapper.vm.show({ glModalAction: 'non-existing' })).toThrow(); + }); + + it('throws if action has no proper configuration', () => { + createComponent({ + modalConfiguration: {}, + }); + expect(() => wrapper.vm.show({ glModalAction: 'action1' })).toThrow(); + }); + + it('renders modal with expected props when valid configuration is passed', () => { + createComponent(); + wrapper.vm.show({ + glModalAction: 'action1', + extraProp: 'extraPropValue', + }); + + return wrapper.vm.$nextTick().then(() => { + const modal = wrapper.find({ ref: 'modal' }); + expect(modal.exists()).toBeTruthy(); + expect(modal.vm.$attrs.csrfToken).toEqual('dummyCSRF'); + expect(modal.vm.$attrs.extraProp).toEqual('extraPropValue'); + expect(modal.vm.showWasCalled).toBeTruthy(); + }); + }); + }); + + describe('global listener', () => { + beforeEach(() => { + jest.spyOn(document, 'addEventListener'); + jest.spyOn(document, 'removeEventListener'); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + + it('registers global listener on mount', () => { + createComponent(); + expect(document.addEventListener).toHaveBeenCalledWith('click', expect.any(Function)); + }); + + it('removes global listener on destroy', () => { + createComponent(); + wrapper.destroy(); + expect(document.removeEventListener).toHaveBeenCalledWith('click', expect.any(Function)); + }); + }); + + describe('click handling', () => { + let node; + + beforeEach(() => { + node = document.createElement('div'); + document.body.appendChild(node); + }); + + afterEach(() => { + node.remove(); + node = null; + }); + + it('ignores wrong clicks', () => { + createComponent(); + const event = new window.MouseEvent('click', { + bubbles: true, + cancellable: true, + }); + jest.spyOn(event, 'preventDefault'); + node.dispatchEvent(event); + expect(event.preventDefault).not.toHaveBeenCalled(); + }); + + it('captures click with glModalAction', () => { + createComponent(); + node.dataset.glModalAction = 'action1'; + const event = new window.MouseEvent('click', { + bubbles: true, + cancellable: true, + }); + jest.spyOn(event, 'preventDefault'); + node.dispatchEvent(event); + + expect(event.preventDefault).toHaveBeenCalled(); + return wrapper.vm.$nextTick().then(() => { + const modal = wrapper.find({ ref: 'modal' }); + expect(modal.exists()).toBeTruthy(); + expect(modal.vm.showWasCalled).toBeTruthy(); + }); + }); + }); +}); diff --git a/spec/frontend/pages/admin/users/components/user_operation_confirmation_modal_spec.js b/spec/frontend/pages/admin/users/components/user_operation_confirmation_modal_spec.js new file mode 100644 index 00000000000..0ecdae2618c --- /dev/null +++ b/spec/frontend/pages/admin/users/components/user_operation_confirmation_modal_spec.js @@ -0,0 +1,47 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlModal } from '@gitlab/ui'; +import UserOperationConfirmationModal from '~/pages/admin/users/components/user_operation_confirmation_modal.vue'; + +describe('User Operation confirmation modal', () => { + let wrapper; + + const createComponent = (props = {}) => { + wrapper = shallowMount(UserOperationConfirmationModal, { + propsData: { + title: 'title', + content: 'content', + action: 'action', + url: '/url', + username: 'username', + csrfToken: 'csrf', + method: 'method', + ...props, + }, + sync: false, + }); + }; + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + it('renders modal with form included', () => { + createComponent(); + expect(wrapper.element).toMatchSnapshot(); + }); + + it('closing modal with ok button triggers form submit', () => { + createComponent(); + const form = wrapper.find('form'); + jest.spyOn(form.element, 'submit').mockReturnValue(); + wrapper.find(GlModal).vm.$emit('ok'); + return wrapper.vm.$nextTick().then(() => { + expect(form.element.submit).toHaveBeenCalled(); + expect(form.element.action).toContain(wrapper.props('url')); + expect(new FormData(form.element).get('authenticity_token')).toEqual( + wrapper.props('csrfToken'), + ); + }); + }); +}); diff --git a/spec/lib/gitlab/auth/user_access_denied_reason_spec.rb b/spec/lib/gitlab/auth/user_access_denied_reason_spec.rb index 8ec19c454d8..7045105a2c7 100644 --- a/spec/lib/gitlab/auth/user_access_denied_reason_spec.rb +++ b/spec/lib/gitlab/auth/user_access_denied_reason_spec.rb @@ -33,5 +33,13 @@ describe Gitlab::Auth::UserAccessDeniedReason do it { is_expected.to match /This action cannot be performed by internal users/ } end + + context 'when the user is deactivated' do + before do + user.deactivate! + end + + it { is_expected.to eq "Your account has been deactivated by your administrator. Please log back in from a web browser to reactivate your account at #{Gitlab.config.gitlab.url}" } + end end end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 3fc45bfc920..dc4b0b5b1b6 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -520,6 +520,12 @@ describe Gitlab::Auth do end end + it 'finds the user in deactivated state' do + user.deactivate! + + expect( gl_auth.find_with_user_password(username, password) ).to eql user + end + it "does not find user in blocked state" do user.block diff --git a/spec/lib/gitlab/diff/position_collection_spec.rb b/spec/lib/gitlab/diff/position_collection_spec.rb new file mode 100644 index 00000000000..de0e631ab03 --- /dev/null +++ b/spec/lib/gitlab/diff/position_collection_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Diff::PositionCollection do + let(:merge_request) { build(:merge_request) } + + def build_text_position(attrs = {}) + attributes = { + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: merge_request.diff_refs + }.merge(attrs) + + Gitlab::Diff::Position.new(attributes) + end + + def build_image_position(attrs = {}) + attributes = { + old_path: "files/images/any_image.png", + new_path: "files/images/any_image.png", + width: 10, + height: 10, + x: 1, + y: 1, + diff_refs: merge_request.diff_refs, + position_type: "image" + }.merge(attrs) + + Gitlab::Diff::Position.new(attributes) + end + + let(:text_position) { build_text_position } + let(:folded_text_position) { build_text_position(old_line: 1, new_line: 1) } + let(:image_position) { build_image_position } + let(:head_sha) { merge_request.diff_head_sha } + + let(:collection) do + described_class.new([text_position, folded_text_position, image_position], head_sha) + end + + describe '#to_a' do + it 'returns all positions' do + expect(collection.to_a).to eq([text_position, folded_text_position, image_position]) + end + end + + describe '#unfoldable' do + it 'returns unfoldable diff positions' do + expect(collection.unfoldable).to eq([folded_text_position]) + end + + context 'when given head_sha does not match with positions head_sha' do + let(:head_sha) { 'unknown' } + + it 'returns no position' do + expect(collection.unfoldable).to be_empty + end + end + end + + describe '#concat' do + let(:new_text_position) { build_text_position(old_line: 1, new_line: 1) } + + it 'returns a Gitlab::Diff::Position' do + expect(collection.concat([new_text_position])).to be_a(described_class) + end + + it 'concatenates the new position to the collection' do + collection.concat([new_text_position]) + + expect(collection.to_a).to eq([text_position, folded_text_position, image_position, new_text_position]) + end + end +end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index d584cdbe280..81dc96b538a 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -541,6 +541,13 @@ describe Gitlab::GitAccess do expect { pull_access_check }.to raise_unauthorized('Your account has been blocked.') end + it 'disallows deactivated users to pull' do + project.add_maintainer(user) + user.deactivate! + + expect { pull_access_check }.to raise_unauthorized("Your account has been deactivated by your administrator. Please log back in from a web browser to reactivate your account at #{Gitlab.config.gitlab.url}") + end + context 'when the project repository does not exist' do it 'returns not found' do project.add_guest(user) @@ -925,6 +932,12 @@ describe Gitlab::GitAccess do project.add_developer(user) end + it 'does not allow deactivated users to push' do + user.deactivate! + + expect { push_access_check }.to raise_unauthorized("Your account has been deactivated by your administrator. Please log back in from a web browser to reactivate your account at #{Gitlab.config.gitlab.url}") + end + it 'cleans up the files' do expect(project.repository).to receive(:clean_stale_repository_files).and_call_original expect { push_access_check }.not_to raise_error diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 58a6c62cdcf..b142942a0a7 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -650,6 +650,46 @@ describe MergeRequest do end end + describe '#note_positions_for_paths' do + let(:merge_request) { create(:merge_request, :with_diffs) } + let(:project) { merge_request.project } + let!(:diff_note) do + create(:diff_note_on_merge_request, project: project, noteable: merge_request) + end + + let(:file_paths) { merge_request.diffs.diff_files.map(&:file_path) } + + subject do + merge_request.note_positions_for_paths(file_paths) + end + + it 'returns a Gitlab::Diff::PositionCollection' do + expect(subject).to be_a(Gitlab::Diff::PositionCollection) + end + + context 'within all diff files' do + it 'returns correct positions' do + expect(subject).to match_array([diff_note.position]) + end + end + + context 'within specific diff file' do + let(:file_paths) { [diff_note.position.file_path] } + + it 'returns correct positions' do + expect(subject).to match_array([diff_note.position]) + end + end + + context 'within no diff files' do + let(:file_paths) { [] } + + it 'returns no positions' do + expect(subject.to_a).to be_empty + end + end + end + describe '#discussions_diffs' do let(:merge_request) { create(:merge_request) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 25e17f3bec4..12292dad142 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1120,6 +1120,30 @@ describe User do end end + describe 'deactivating a user' do + let(:user) { create(:user, name: 'John Smith') } + + context "an active user" do + it "can be deactivated" do + user.deactivate + + expect(user.deactivated?).to be_truthy + end + end + + context "a user who is blocked" do + before do + user.block + end + + it "cannot be deactivated" do + user.deactivate + + expect(user.reload.deactivated?).to be_falsy + end + end + end + describe '.filter_items' do let(:user) { double } @@ -1141,6 +1165,12 @@ describe User do expect(described_class.filter_items('blocked')).to include user end + it 'filters by deactivated' do + expect(described_class).to receive(:deactivated).and_return([user]) + + expect(described_class.filter_items('deactivated')).to include user + end + it 'filters by two_factor_disabled' do expect(described_class).to receive(:without_two_factor).and_return([user]) @@ -1524,15 +1554,22 @@ describe User do end describe '.find_by_ssh_key_id' do - context 'using an existing SSH key ID' do - let(:user) { create(:user) } - let(:key) { create(:key, user: user) } + let_it_be(:user) { create(:user) } + let_it_be(:key) { create(:key, user: user) } + context 'using an existing SSH key ID' do it 'returns the corresponding User' do expect(described_class.find_by_ssh_key_id(key.id)).to eq(user) end end + it 'only performs a single query' do + key # Don't count the queries for creating the key and user + + expect { described_class.find_by_ssh_key_id(key.id) } + .not_to exceed_query_limit(1) + end + context 'using an invalid SSH key ID' do it 'returns nil' do expect(described_class.find_by_ssh_key_id(-1)).to be_nil @@ -2042,6 +2079,95 @@ describe User do end end + describe "#last_active_at" do + let(:last_activity_on) { 5.days.ago.to_date } + let(:current_sign_in_at) { 8.days.ago } + + context 'for a user that has `last_activity_on` set' do + let(:user) { create(:user, last_activity_on: last_activity_on) } + + it 'returns `last_activity_on` with current time zone' do + expect(user.last_active_at).to eq(last_activity_on.to_time.in_time_zone) + end + end + + context 'for a user that has `current_sign_in_at` set' do + let(:user) { create(:user, current_sign_in_at: current_sign_in_at) } + + it 'returns `current_sign_in_at`' do + expect(user.last_active_at).to eq(current_sign_in_at) + end + end + + context 'for a user that has both `current_sign_in_at` & ``last_activity_on`` set' do + let(:user) { create(:user, current_sign_in_at: current_sign_in_at, last_activity_on: last_activity_on) } + + it 'returns the latest among `current_sign_in_at` & `last_activity_on`' do + latest_event = [current_sign_in_at, last_activity_on.to_time.in_time_zone].max + expect(user.last_active_at).to eq(latest_event) + end + end + + context 'for a user that does not have both `current_sign_in_at` & `last_activity_on` set' do + let(:user) { create(:user, current_sign_in_at: nil, last_activity_on: nil) } + + it 'returns nil' do + expect(user.last_active_at).to eq(nil) + end + end + end + + describe "#can_be_deactivated?" do + let(:activity) { {} } + let(:user) { create(:user, name: 'John Smith', **activity) } + let(:day_within_minium_inactive_days_threshold) { User::MINIMUM_INACTIVE_DAYS.pred.days.ago } + let(:day_outside_minium_inactive_days_threshold) { User::MINIMUM_INACTIVE_DAYS.next.days.ago } + + shared_examples 'not eligible for deactivation' do + it 'returns false' do + expect(user.can_be_deactivated?).to be_falsey + end + end + + shared_examples 'eligible for deactivation' do + it 'returns true' do + expect(user.can_be_deactivated?).to be_truthy + end + end + + context "a user who is not active" do + before do + user.block + end + + it_behaves_like 'not eligible for deactivation' + end + + context 'a user who has activity within the specified minimum inactive days' do + let(:activity) { { last_activity_on: day_within_minium_inactive_days_threshold } } + + it_behaves_like 'not eligible for deactivation' + end + + context 'a user who has signed in within the specified minimum inactive days' do + let(:activity) { { current_sign_in_at: day_within_minium_inactive_days_threshold } } + + it_behaves_like 'not eligible for deactivation' + end + + context 'a user who has no activity within the specified minimum inactive days' do + let(:activity) { { last_activity_on: day_outside_minium_inactive_days_threshold } } + + it_behaves_like 'eligible for deactivation' + end + + context 'a user who has not signed in within the specified minimum inactive days' do + let(:activity) { { current_sign_in_at: day_outside_minium_inactive_days_threshold } } + + it_behaves_like 'eligible for deactivation' + end + end + describe "#contributed_projects" do subject { create(:user) } let!(:project1) { create(:project) } diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb index df6cc526eb0..48f3d485f4b 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -141,6 +141,40 @@ describe GlobalPolicy do end end + describe 'receive notifications' do + describe 'regular user' do + it { is_expected.to be_allowed(:receive_notifications) } + end + + describe 'admin' do + let(:current_user) { create(:admin) } + + it { is_expected.to be_allowed(:receive_notifications) } + end + + describe 'anonymous' do + let(:current_user) { nil } + + it { is_expected.not_to be_allowed(:receive_notifications) } + end + + describe 'blocked user' do + before do + current_user.block + end + + it { is_expected.not_to be_allowed(:receive_notifications) } + end + + describe 'deactivated user' do + before do + current_user.deactivate + end + + it { is_expected.not_to be_allowed(:receive_notifications) } + end + end + describe 'git access' do describe 'regular user' do it { is_expected.to be_allowed(:access_git) } @@ -158,6 +192,14 @@ describe GlobalPolicy do it { is_expected.to be_allowed(:access_git) } end + describe 'deactivated user' do + before do + current_user.deactivate + end + + it { is_expected.not_to be_allowed(:access_git) } + end + context 'when terms are enforced' do before do enforce_terms diff --git a/spec/requests/api/doorkeeper_access_spec.rb b/spec/requests/api/doorkeeper_access_spec.rb index d74484c8d29..cfee3f6c0f8 100644 --- a/spec/requests/api/doorkeeper_access_spec.rb +++ b/spec/requests/api/doorkeeper_access_spec.rb @@ -38,21 +38,35 @@ describe 'doorkeeper access' do end end - describe "when user is blocked" do - it "returns authorization error" do - user.block + shared_examples 'forbidden request' do + it 'returns 403 response' do get api("/user"), params: { access_token: token.token } expect(response).to have_gitlab_http_status(403) end end - describe "when user is ldap_blocked" do - it "returns authorization error" do + context "when user is blocked" do + before do + user.block + end + + it_behaves_like 'forbidden request' + end + + context "when user is ldap_blocked" do + before do user.ldap_block - get api("/user"), params: { access_token: token.token } + end - expect(response).to have_gitlab_http_status(403) + it_behaves_like 'forbidden request' + end + + context "when user is deactivated" do + before do + user.deactivate end + + it_behaves_like 'forbidden request' end end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 2280d8ca9d4..7161d6f0a10 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -237,14 +237,6 @@ describe API::Internal::Base do expect(json_response['name']).to eq(user.name) end - it "finds a user by user id" do - get(api("/internal/discover"), params: { user_id: user.id, secret_token: secret_token }) - - expect(response).to have_gitlab_http_status(200) - - expect(json_response['name']).to eq(user.name) - end - it "finds a user by username" do get(api("/internal/discover"), params: { username: user.username, secret_token: secret_token }) diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index df76b62b40e..0d190ae069e 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1846,6 +1846,182 @@ describe API::Users do end end + context 'activate and deactivate' do + shared_examples '404' do + it 'returns 404' do + expect(response).to have_gitlab_http_status(404) + expect(json_response['message']).to eq('404 User Not Found') + end + end + + describe 'POST /users/:id/activate' do + context 'performed by a non-admin user' do + it 'is not authorized to perform the action' do + post api("/users/#{user.id}/activate", user) + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'performed by an admin user' do + context 'for a deactivated user' do + before do + user.deactivate + + post api("/users/#{user.id}/activate", admin) + end + + it 'activates a deactivated user' do + expect(response).to have_gitlab_http_status(201) + expect(user.reload.state).to eq('active') + end + end + + context 'for an active user' do + before do + user.activate + + post api("/users/#{user.id}/activate", admin) + end + + it 'returns 201' do + expect(response).to have_gitlab_http_status(201) + expect(user.reload.state).to eq('active') + end + end + + context 'for a blocked user' do + before do + user.block + + post api("/users/#{user.id}/activate", admin) + end + + it 'returns 403' do + expect(response).to have_gitlab_http_status(403) + expect(json_response['message']).to eq('403 Forbidden - A blocked user must be unblocked to be activated') + expect(user.reload.state).to eq('blocked') + end + end + + context 'for a ldap blocked user' do + before do + user.ldap_block + + post api("/users/#{user.id}/activate", admin) + end + + it 'returns 403' do + expect(response).to have_gitlab_http_status(403) + expect(json_response['message']).to eq('403 Forbidden - A blocked user must be unblocked to be activated') + expect(user.reload.state).to eq('ldap_blocked') + end + end + + context 'for a user that does not exist' do + before do + post api("/users/0/activate", admin) + end + + it_behaves_like '404' + end + end + end + + describe 'POST /users/:id/deactivate' do + context 'performed by a non-admin user' do + it 'is not authorized to perform the action' do + post api("/users/#{user.id}/deactivate", user) + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'performed by an admin user' do + context 'for an active user' do + let(:activity) { {} } + let(:user) { create(:user, username: 'user.with.dot', **activity) } + + context 'with no recent activity' do + let(:activity) { { last_activity_on: ::User::MINIMUM_INACTIVE_DAYS.next.days.ago } } + + before do + post api("/users/#{user.id}/deactivate", admin) + end + + it 'deactivates an active user' do + expect(response).to have_gitlab_http_status(201) + expect(user.reload.state).to eq('deactivated') + end + end + + context 'with recent activity' do + let(:activity) { { last_activity_on: ::User::MINIMUM_INACTIVE_DAYS.pred.days.ago } } + + before do + post api("/users/#{user.id}/deactivate", admin) + end + + it 'does not deactivate an active user' do + expect(response).to have_gitlab_http_status(403) + expect(json_response['message']).to eq("403 Forbidden - The user you are trying to deactivate has been active in the past #{::User::MINIMUM_INACTIVE_DAYS} days and cannot be deactivated") + expect(user.reload.state).to eq('active') + end + end + end + + context 'for a deactivated user' do + before do + user.deactivate + + post api("/users/#{user.id}/deactivate", admin) + end + + it 'returns 201' do + expect(response).to have_gitlab_http_status(201) + expect(user.reload.state).to eq('deactivated') + end + end + + context 'for a blocked user' do + before do + user.block + + post api("/users/#{user.id}/deactivate", admin) + end + + it 'returns 403' do + expect(response).to have_gitlab_http_status(403) + expect(json_response['message']).to eq('403 Forbidden - A blocked user cannot be deactivated by the API') + expect(user.reload.state).to eq('blocked') + end + end + + context 'for a ldap blocked user' do + before do + user.ldap_block + + post api("/users/#{user.id}/deactivate", admin) + end + + it 'returns 403' do + expect(response).to have_gitlab_http_status(403) + expect(json_response['message']).to eq('403 Forbidden - A blocked user cannot be deactivated by the API') + expect(user.reload.state).to eq('ldap_blocked') + end + end + + context 'for a user that does not exist' do + before do + post api("/users/0/deactivate", admin) + end + + it_behaves_like '404' + end + end + end + end + describe 'POST /users/:id/block' do before do admin @@ -1878,6 +2054,7 @@ describe API::Users do describe 'POST /users/:id/unblock' do let(:blocked_user) { create(:user, state: 'blocked') } + let(:deactivated_user) { create(:user, state: 'deactivated') } before do admin @@ -1901,7 +2078,13 @@ describe API::Users do expect(ldap_blocked_user.reload.state).to eq('ldap_blocked') end - it 'does not be available for non admin users' do + it 'does not unblock deactivated users' do + post api("/users/#{deactivated_user.id}/unblock", admin) + expect(response).to have_gitlab_http_status(403) + expect(deactivated_user.reload.state).to eq('deactivated') + end + + it 'is not available for non admin users' do post api("/users/#{user.id}/unblock", user) expect(response).to have_gitlab_http_status(403) expect(user.reload.state).to eq('active') |