diff options
Diffstat (limited to 'spec/services/users')
6 files changed, 791 insertions, 245 deletions
diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index 90c4f70d749..b32599d4af8 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -10,371 +10,475 @@ RSpec.describe Users::DestroyService do let(:service) { described_class.new(admin) } let(:gitlab_shell) { Gitlab::Shell.new } - describe "Deletes a user and all their personal projects", :enable_admin_mode do - context 'no options are given' do - it 'deletes the user' do - user_data = service.execute(user) - - expect(user_data['email']).to eq(user.email) - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - expect { Namespace.find(namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) + shared_examples 'pre-migrate clean-up' do + describe "Deletes a user and all their personal projects", :enable_admin_mode do + context 'no options are given' do + it 'will delete the personal project' do + expect_next_instance_of(Projects::DestroyService) do |destroy_service| + expect(destroy_service).to receive(:execute).once.and_return(true) + end + + service.execute(user) + end end - it 'deletes user associations in batches' do - expect(user).to receive(:destroy_dependent_associations_in_batches) + context 'personal projects in pending_delete' do + before do + project.pending_delete = true + project.save! + end + + it 'destroys a personal project in pending_delete' do + expect_next_instance_of(Projects::DestroyService) do |destroy_service| + expect(destroy_service).to receive(:execute).once.and_return(true) + end - service.execute(user) + service.execute(user) + end end - it 'does not include snippets when deleting in batches' do - expect(user).to receive(:destroy_dependent_associations_in_batches).with({ exclude: [:snippets] }) + context "solo owned groups present" do + let(:solo_owned) { create(:group) } + let(:member) { create(:group_member) } + let(:user) { member.user } - service.execute(user) - end + before do + solo_owned.group_members = [member] + end - it 'will delete the project' do - expect_next_instance_of(Projects::DestroyService) do |destroy_service| - expect(destroy_service).to receive(:execute).once.and_return(true) + it 'returns the user with attached errors' do + expect(service.execute(user)).to be(user) + expect(user.errors.full_messages).to( + contain_exactly('You must transfer ownership or delete groups before you can remove user')) end - service.execute(user) + it 'does not delete the user, nor the group' do + service.execute(user) + + expect(User.find(user.id)).to eq user + expect(Group.find(solo_owned.id)).to eq solo_owned + end end - it 'calls the bulk snippet destroy service for the user personal snippets' do - repo1 = create(:personal_snippet, :repository, author: user).snippet_repository - repo2 = create(:project_snippet, :repository, project: project, author: user).snippet_repository + context "deletions with solo owned groups" do + let(:solo_owned) { create(:group) } + let(:member) { create(:group_member) } + let(:user) { member.user } - aggregate_failures do - expect(gitlab_shell.repository_exists?(repo1.shard_name, repo1.disk_path + '.git')).to be_truthy - expect(gitlab_shell.repository_exists?(repo2.shard_name, repo2.disk_path + '.git')).to be_truthy + before do + solo_owned.group_members = [member] + service.execute(user, delete_solo_owned_groups: true) end - # Call made when destroying user personal projects - expect(Snippets::BulkDestroyService).to receive(:new) - .with(admin, project.snippets).and_call_original + it 'deletes solo owned groups' do + expect { Group.find(solo_owned.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + end - # Call to remove user personal snippets and for - # project snippets where projects are not user personal - # ones - expect(Snippets::BulkDestroyService).to receive(:new) - .with(admin, user.snippets.only_personal_snippets).and_call_original + context 'deletions with inherited group owners' do + let(:group) { create(:group, :nested) } + let(:user) { create(:user) } + let(:inherited_owner) { create(:user) } + + before do + group.parent.add_owner(inherited_owner) + group.add_owner(user) - service.execute(user) + service.execute(user, delete_solo_owned_groups: true) + end - aggregate_failures do - expect(gitlab_shell.repository_exists?(repo1.shard_name, repo1.disk_path + '.git')).to be_falsey - expect(gitlab_shell.repository_exists?(repo2.shard_name, repo2.disk_path + '.git')).to be_falsey + it 'does not delete the group' do + expect(Group.exists?(id: group)).to be_truthy end end - it 'calls the bulk snippet destroy service with hard delete option if it is present' do - # this avoids getting into Projects::DestroyService as it would - # call Snippets::BulkDestroyService first! - allow(user).to receive(:personal_projects).and_return([]) + describe "user personal's repository removal" do + context 'storages' do + before do + perform_enqueued_jobs { service.execute(user) } + end - expect_next_instance_of(Snippets::BulkDestroyService) do |bulk_destroy_service| - expect(bulk_destroy_service).to receive(:execute).with({ hard_delete: true }).and_call_original - end + context 'legacy storage' do + let!(:project) { create(:project, :empty_repo, :legacy_storage, namespace: user.namespace) } - service.execute(user, { hard_delete: true }) - end + it 'removes repository' do + expect( + gitlab_shell.repository_exists?(project.repository_storage, + "#{project.disk_path}.git") + ).to be_falsey + end + end - it 'does not delete project snippets that the user is the author of' do - repo = create(:project_snippet, :repository, author: user).snippet_repository - service.execute(user) - expect(gitlab_shell.repository_exists?(repo.shard_name, repo.disk_path + '.git')).to be_truthy - expect(User.ghost.snippets).to include(repo.snippet) - end + context 'hashed storage' do + let!(:project) { create(:project, :empty_repo, namespace: user.namespace) } - context 'when an error is raised deleting snippets' do - it 'does not delete user' do - snippet = create(:personal_snippet, :repository, author: user) + it 'removes repository' do + expect( + gitlab_shell.repository_exists?(project.repository_storage, + "#{project.disk_path}.git") + ).to be_falsey + end + end + end - bulk_service = double - allow(Snippets::BulkDestroyService).to receive(:new).and_call_original - allow(Snippets::BulkDestroyService).to receive(:new).with(admin, user.snippets).and_return(bulk_service) - allow(bulk_service).to receive(:execute).and_return(ServiceResponse.error(message: 'foo')) + context 'repository removal status is taken into account' do + it 'raises exception' do + expect_next_instance_of(::Projects::DestroyService) do |destroy_service| + expect(destroy_service).to receive(:execute).and_return(false) + end - aggregate_failures do expect { service.execute(user) } - .to raise_error(Users::DestroyService::DestroyError, 'foo' ) - expect(snippet.reload).not_to be_nil - expect(gitlab_shell.repository_exists?(snippet.repository_storage, snippet.disk_path + '.git')).to be_truthy + .to raise_error(Users::DestroyService::DestroyError, + "Project #{project.id} can't be deleted" ) end end end - end - context 'projects in pending_delete' do - before do - project.pending_delete = true - project.save! - end + describe "calls the before/after callbacks" do + it 'of project_members' do + expect_any_instance_of(ProjectMember).to receive(:run_callbacks).with(:find).once + expect_any_instance_of(ProjectMember).to receive(:run_callbacks).with(:initialize).once + expect_any_instance_of(ProjectMember).to receive(:run_callbacks).with(:destroy).once - it 'destroys a project in pending_delete' do - expect_next_instance_of(Projects::DestroyService) do |destroy_service| - expect(destroy_service).to receive(:execute).once.and_return(true) + service.execute(user) end - service.execute(user) + it 'of group_members' do + group_member = create(:group_member) + group_member.group.group_members.create!(user: user, access_level: 40) - expect { Project.find(project.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect_any_instance_of(GroupMember).to receive(:run_callbacks).with(:find).once + expect_any_instance_of(GroupMember).to receive(:run_callbacks).with(:initialize).once + expect_any_instance_of(GroupMember).to receive(:run_callbacks).with(:destroy).once + + service.execute(user) + end end end + end - context "a deleted user's issues" do - let(:project) { create(:project) } + context 'when user_destroy_with_limited_execution_time_worker is disabled' do + before do + stub_feature_flags(user_destroy_with_limited_execution_time_worker: false) + end - before do - project.add_developer(user) - end + include_examples 'pre-migrate clean-up' - context "for an issue the user was assigned to" do - let!(:issue) { create(:issue, project: project, assignees: [user]) } + describe "Deletes a user and all their personal projects", :enable_admin_mode do + context 'no options are given' do + it 'deletes the user' do + user_data = service.execute(user) - before do - service.execute(user) + expect(user_data['email']).to eq(user.email) + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Namespace.find(namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) end - it 'does not delete issues the user is assigned to' do - expect(Issue.find_by_id(issue.id)).to be_present + it 'deletes user associations in batches' do + expect(user).to receive(:destroy_dependent_associations_in_batches) + + service.execute(user) end - it 'migrates the issue so that it is "Unassigned"' do - migrated_issue = Issue.find_by_id(issue.id) + it 'does not include snippets when deleting in batches' do + expect(user).to receive(:destroy_dependent_associations_in_batches).with({ exclude: [:snippets] }) - expect(migrated_issue.assignees).to be_empty + service.execute(user) end - end - end - context "a deleted user's merge_requests" do - let(:project) { create(:project, :repository) } + it 'calls the bulk snippet destroy service for the user personal snippets' do + repo1 = create(:personal_snippet, :repository, author: user).snippet_repository + repo2 = create(:project_snippet, :repository, project: project, author: user).snippet_repository - before do - project.add_developer(user) - end + aggregate_failures do + expect(gitlab_shell.repository_exists?(repo1.shard_name, repo1.disk_path + '.git')).to be_truthy + expect(gitlab_shell.repository_exists?(repo2.shard_name, repo2.disk_path + '.git')).to be_truthy + end - context "for an merge request the user was assigned to" do - let!(:merge_request) { create(:merge_request, source_project: project, assignees: [user]) } + # Call made when destroying user personal projects + expect(Snippets::BulkDestroyService).to receive(:new) + .with(admin, project.snippets).and_call_original + + # Call to remove user personal snippets and for + # project snippets where projects are not user personal + # ones + expect(Snippets::BulkDestroyService).to receive(:new) + .with(admin, user.snippets.only_personal_snippets).and_call_original - before do service.execute(user) + + aggregate_failures do + expect(gitlab_shell.repository_exists?(repo1.shard_name, repo1.disk_path + '.git')).to be_falsey + expect(gitlab_shell.repository_exists?(repo2.shard_name, repo2.disk_path + '.git')).to be_falsey + end end - it 'does not delete merge requests the user is assigned to' do - expect(MergeRequest.find_by_id(merge_request.id)).to be_present + it 'calls the bulk snippet destroy service with hard delete option if it is present' do + # this avoids getting into Projects::DestroyService as it would + # call Snippets::BulkDestroyService first! + allow(user).to receive(:personal_projects).and_return([]) + + expect_next_instance_of(Snippets::BulkDestroyService) do |bulk_destroy_service| + expect(bulk_destroy_service).to receive(:execute).with({ skip_authorization: true }).and_call_original + end + + service.execute(user, { hard_delete: true }) end - it 'migrates the merge request so that it is "Unassigned"' do - migrated_merge_request = MergeRequest.find_by_id(merge_request.id) + it 'does not delete project snippets that the user is the author of' do + repo = create(:project_snippet, :repository, author: user).snippet_repository + service.execute(user) + expect(gitlab_shell.repository_exists?(repo.shard_name, repo.disk_path + '.git')).to be_truthy + expect(User.ghost.snippets).to include(repo.snippet) + end - expect(migrated_merge_request.assignees).to be_empty + context 'when an error is raised deleting snippets' do + it 'does not delete user' do + snippet = create(:personal_snippet, :repository, author: user) + + bulk_service = double + allow(Snippets::BulkDestroyService).to receive(:new).and_call_original + allow(Snippets::BulkDestroyService).to receive(:new).with(admin, user.snippets).and_return(bulk_service) + allow(bulk_service).to receive(:execute).and_return(ServiceResponse.error(message: 'foo')) + + aggregate_failures do + expect { service.execute(user) } + .to raise_error(Users::DestroyService::DestroyError, 'foo' ) + expect(snippet.reload).not_to be_nil + expect( + gitlab_shell.repository_exists?(snippet.repository_storage, + snippet.disk_path + '.git') + ).to be_truthy + end + end end end - end - context "solo owned groups present" do - let(:solo_owned) { create(:group) } - let(:member) { create(:group_member) } - let(:user) { member.user } + context 'projects in pending_delete' do + before do + project.pending_delete = true + project.save! + end - before do - solo_owned.group_members = [member] - end + it 'destroys a project in pending_delete' do + expect_next_instance_of(Projects::DestroyService) do |destroy_service| + expect(destroy_service).to receive(:execute).once.and_return(true) + end - it 'returns the user with attached errors' do - expect(service.execute(user)).to be(user) - expect(user.errors.full_messages).to eq([ - 'You must transfer ownership or delete groups before you can remove user' - ]) - end + service.execute(user) - it 'does not delete the user' do - service.execute(user) - expect(User.find(user.id)).to eq user + expect { Project.find(project.id) }.to raise_error(ActiveRecord::RecordNotFound) + end end - end - context "deletions with solo owned groups" do - let(:solo_owned) { create(:group) } - let(:member) { create(:group_member) } - let(:user) { member.user } + context "a deleted user's issues" do + let(:project) { create(:project) } - before do - solo_owned.group_members = [member] - service.execute(user, delete_solo_owned_groups: true) - end + before do + project.add_developer(user) + end - it 'deletes solo owned groups' do - expect { Project.find(solo_owned.id) }.to raise_error(ActiveRecord::RecordNotFound) - end + context "for an issue the user was assigned to" do + let!(:issue) { create(:issue, project: project, assignees: [user]) } - it 'deletes the user' do - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - end - end + before do + service.execute(user) + end - context 'deletions with inherited group owners' do - let(:group) { create(:group, :nested) } - let(:user) { create(:user) } - let(:inherited_owner) { create(:user) } + it 'does not delete issues the user is assigned to' do + expect(Issue.find_by_id(issue.id)).to be_present + end - before do - group.parent.add_owner(inherited_owner) - group.add_owner(user) + it 'migrates the issue so that it is "Unassigned"' do + migrated_issue = Issue.find_by_id(issue.id) - service.execute(user, delete_solo_owned_groups: true) + expect(migrated_issue.assignees).to be_empty + end + end end - it 'does not delete the group' do - expect(Group.exists?(id: group)).to be_truthy - end + context "a deleted user's merge_requests" do + let(:project) { create(:project, :repository) } - it 'deletes the user' do - expect(User.exists?(id: user)).to be_falsey - end - end + before do + project.add_developer(user) + end - context 'migrating associated records' do - let!(:issue) { create(:issue, author: user) } + context "for an merge request the user was assigned to" do + let!(:merge_request) { create(:merge_request, source_project: project, assignees: [user]) } + + before do + service.execute(user) + end - it 'delegates to the `MigrateToGhostUser` service to move associated records to the ghost user' do - expect_any_instance_of(Users::MigrateToGhostUserService).to receive(:execute).once.and_call_original + it 'does not delete merge requests the user is assigned to' do + expect(MergeRequest.find_by_id(merge_request.id)).to be_present + end - service.execute(user) + it 'migrates the merge request so that it is "Unassigned"' do + migrated_merge_request = MergeRequest.find_by_id(merge_request.id) - expect(issue.reload.author).to be_ghost + expect(migrated_merge_request.assignees).to be_empty + end + end end - context 'when hard_delete option is given' do - it 'will not ghost certain records' do + context 'migrating associated records' do + let!(:issue) { create(:issue, author: user) } + + it 'delegates to the `MigrateToGhostUser` service to move associated records to the ghost user' do expect_any_instance_of(Users::MigrateToGhostUserService).to receive(:execute).once.and_call_original - service.execute(user, hard_delete: true) + service.execute(user) - expect(Issue.exists?(issue.id)).to be_falsy + expect(issue.reload.author).to be_ghost end - end - end - describe "user personal's repository removal" do - context 'storages' do - before do - perform_enqueued_jobs { service.execute(user) } - end + context 'when hard_delete option is given' do + it 'will not ghost certain records' do + expect_any_instance_of(Users::MigrateToGhostUserService).to receive(:execute).once.and_call_original - context 'legacy storage' do - let!(:project) { create(:project, :empty_repo, :legacy_storage, namespace: user.namespace) } + service.execute(user, hard_delete: true) - it 'removes repository' do - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey + expect(Issue.exists?(issue.id)).to be_falsy end end + end + end - context 'hashed storage' do - let!(:project) { create(:project, :empty_repo, namespace: user.namespace) } + describe "Deletion permission checks" do + it 'does not delete the user when user is not an admin' do + other_user = create(:user) - it 'removes repository' do - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey - end + expect { described_class.new(other_user).execute(user) }.to raise_error(Gitlab::Access::AccessDeniedError) + expect(User.exists?(user.id)).to be(true) + end + + context 'when admin mode is enabled', :enable_admin_mode do + it 'allows admins to delete anyone' do + described_class.new(admin).execute(user) + + expect(User.exists?(user.id)).to be(false) end end - context 'repository removal status is taken into account' do - it 'raises exception' do - expect_next_instance_of(::Projects::DestroyService) do |destroy_service| - expect(destroy_service).to receive(:execute).and_return(false) - end + context 'when admin mode is disabled' do + it 'disallows admins to delete anyone' do + expect { described_class.new(admin).execute(user) }.to raise_error(Gitlab::Access::AccessDeniedError) - expect { service.execute(user) } - .to raise_error(Users::DestroyService::DestroyError, "Project #{project.id} can't be deleted" ) + expect(User.exists?(user.id)).to be(true) end end - end - describe "calls the before/after callbacks" do - it 'of project_members' do - expect_any_instance_of(ProjectMember).to receive(:run_callbacks).with(:find).once - expect_any_instance_of(ProjectMember).to receive(:run_callbacks).with(:initialize).once - expect_any_instance_of(ProjectMember).to receive(:run_callbacks).with(:destroy).once + it 'allows users to delete their own account' do + described_class.new(user).execute(user) - service.execute(user) + expect(User.exists?(user.id)).to be(false) end - it 'of group_members' do - group_member = create(:group_member) - group_member.group.group_members.create!(user: user, access_level: 40) + it 'allows user to be deleted if skip_authorization: true' do + other_user = create(:user) - expect_any_instance_of(GroupMember).to receive(:run_callbacks).with(:find).once - expect_any_instance_of(GroupMember).to receive(:run_callbacks).with(:initialize).once - expect_any_instance_of(GroupMember).to receive(:run_callbacks).with(:destroy).once + described_class.new(user).execute(other_user, skip_authorization: true) - service.execute(user) + expect(User.exists?(other_user.id)).to be(false) end end - end - - describe "Deletion permission checks" do - it 'does not delete the user when user is not an admin' do - other_user = create(:user) - expect { described_class.new(other_user).execute(user) }.to raise_error(Gitlab::Access::AccessDeniedError) - expect(User.exists?(user.id)).to be(true) - end + context 'batched nullify' do + let(:other_user) { create(:user) } - context 'when admin mode is enabled', :enable_admin_mode do - it 'allows admins to delete anyone' do - described_class.new(admin).execute(user) + it 'nullifies related associations in batches' do + expect(other_user).to receive(:nullify_dependent_associations_in_batches).and_call_original - expect(User.exists?(user.id)).to be(false) + described_class.new(user).execute(other_user, skip_authorization: true) end - end - context 'when admin mode is disabled' do - it 'disallows admins to delete anyone' do - expect { described_class.new(admin).execute(user) }.to raise_error(Gitlab::Access::AccessDeniedError) + it 'nullifies last_updated_issues, closed_issues, resource_label_events' do + issue = create(:issue, closed_by: other_user, updated_by: other_user) + resource_label_event = create(:resource_label_event, user: other_user) - expect(User.exists?(user.id)).to be(true) - end - end + described_class.new(user).execute(other_user, skip_authorization: true) - it 'allows users to delete their own account' do - described_class.new(user).execute(user) + issue.reload + resource_label_event.reload - expect(User.exists?(user.id)).to be(false) + expect(issue.closed_by).to be_nil + expect(issue.updated_by).to be_nil + expect(resource_label_event.user).to be_nil + end end + end - it 'allows user to be deleted if skip_authorization: true' do - other_user = create(:user) - - described_class.new(user).execute(other_user, skip_authorization: true) + context 'when user_destroy_with_limited_execution_time_worker is enabled' do + include_examples 'pre-migrate clean-up' - expect(User.exists?(other_user.id)).to be(false) + describe "Deletes a user and all their personal projects", :enable_admin_mode do + context 'no options are given' do + it 'creates GhostUserMigration record to handle migration in a worker' do + expect { service.execute(user) } + .to( + change do + Users::GhostUserMigration.where(user: user, + initiator_user: admin) + .exists? + end.from(false).to(true)) + end + end end - end - context 'batched nullify' do - let(:other_user) { create(:user) } + describe "Deletion permission checks" do + it 'does not delete the user when user is not an admin' do + other_user = create(:user) - it 'nullifies related associations in batches' do - expect(other_user).to receive(:nullify_dependent_associations_in_batches).and_call_original + expect { described_class.new(other_user).execute(user) }.to raise_error(Gitlab::Access::AccessDeniedError) - described_class.new(user).execute(other_user, skip_authorization: true) - end + expect(Users::GhostUserMigration).not_to be_exists + end + + context 'when admin mode is enabled', :enable_admin_mode do + it 'allows admins to delete anyone' do + expect { described_class.new(admin).execute(user) } + .to( + change do + Users::GhostUserMigration.where(user: user, + initiator_user: admin) + .exists? + end.from(false).to(true)) + end + end - it 'nullifies last_updated_issues, closed_issues, resource_label_events' do - issue = create(:issue, closed_by: other_user, updated_by: other_user) - resource_label_event = create(:resource_label_event, user: other_user) + context 'when admin mode is disabled' do + it 'disallows admins to delete anyone' do + expect { described_class.new(admin).execute(user) }.to raise_error(Gitlab::Access::AccessDeniedError) - described_class.new(user).execute(other_user, skip_authorization: true) + expect(Users::GhostUserMigration).not_to be_exists + end + end - issue.reload - resource_label_event.reload + it 'allows users to delete their own account' do + expect { described_class.new(user).execute(user) } + .to( + change do + Users::GhostUserMigration.where(user: user, + initiator_user: user) + .exists? + end.from(false).to(true)) + end - expect(issue.closed_by).to be_nil - expect(issue.updated_by).to be_nil - expect(resource_label_event.user).to be_nil + it 'allows user to be deleted if skip_authorization: true' do + other_user = create(:user) + + expect do + described_class.new(user) + .execute(other_user, skip_authorization: true) + end.to( + change do + Users::GhostUserMigration.where(user: other_user, + initiator_user: user ) + .exists? + end.from(false).to(true)) + end end end end diff --git a/spec/services/users/email_verification/generate_token_service_spec.rb b/spec/services/users/email_verification/generate_token_service_spec.rb new file mode 100644 index 00000000000..e7aa1bf8306 --- /dev/null +++ b/spec/services/users/email_verification/generate_token_service_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::EmailVerification::GenerateTokenService do + using RSpec::Parameterized::TableSyntax + + let(:service) { described_class.new(attr: attr) } + let(:token) { 'token' } + let(:digest) { Devise.token_generator.digest(User, attr, token) } + + describe '#execute' do + context 'with a valid attribute' do + where(:attr) { [:unlock_token, :confirmation_token] } + + with_them do + before do + allow_next_instance_of(described_class) do |service| + allow(service).to receive(:generate_token).and_return(token) + end + end + + it "returns a token and it's digest" do + expect(service.execute).to eq([token, digest]) + end + end + end + + context 'with an invalid attribute' do + let(:attr) { :xxx } + + it 'raises an error' do + expect { service.execute }.to raise_error(ArgumentError, 'Invalid attribute') + end + end + end +end diff --git a/spec/services/users/email_verification/validate_token_service_spec.rb b/spec/services/users/email_verification/validate_token_service_spec.rb new file mode 100644 index 00000000000..44af4a4d36f --- /dev/null +++ b/spec/services/users/email_verification/validate_token_service_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::EmailVerification::ValidateTokenService, :clean_gitlab_redis_rate_limiting do + using RSpec::Parameterized::TableSyntax + + let(:service) { described_class.new(attr: attr, user: user, token: token) } + let(:token) { 'token' } + let(:encrypted_token) { Devise.token_generator.digest(User, attr, token) } + let(:generated_at_attr) { attr == :unlock_token ? :locked_at : :confirmation_sent_at } + let(:token_generated_at) { 1.minute.ago } + let(:user) { build(:user, attr => encrypted_token, generated_at_attr => token_generated_at) } + + describe '#execute' do + context 'with a valid attribute' do + where(:attr) { [:unlock_token, :confirmation_token] } + + with_them do + context 'when successful' do + it 'returns a success status' do + expect(service.execute).to eq(status: :success) + end + end + + context 'when rate limited' do + before do + allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?) + .with(:email_verification, scope: encrypted_token).and_return(true) + end + + it 'returns a failure status' do + expect(service.execute).to eq( + { + status: :failure, + reason: :rate_limited, + message: format(s_("IdentityVerification|You've reached the maximum amount of tries. "\ + 'Wait %{interval} or send a new code and try again.'), interval: '10 minutes') + } + ) + end + end + + context 'when expired' do + let(:token_generated_at) { 2.hours.ago } + + it 'returns a failure status' do + expect(service.execute).to eq( + { + status: :failure, + reason: :expired, + message: s_('IdentityVerification|The code has expired. Send a new code and try again.') + } + ) + end + end + + context 'when invalid' do + let(:encrypted_token) { 'xxx' } + + it 'returns a failure status' do + expect(service.execute).to eq( + { + status: :failure, + reason: :invalid, + message: s_('IdentityVerification|The code is incorrect. Enter it again, or send a new code.') + } + ) + end + end + + context 'when encrypted token was not set and a blank token is provided' do + let(:encrypted_token) { nil } + let(:token) { '' } + + it 'returns a failure status' do + expect(service.execute).to eq( + { + status: :failure, + reason: :invalid, + message: s_('IdentityVerification|The code is incorrect. Enter it again, or send a new code.') + } + ) + end + end + end + end + + context 'with an invalid attribute' do + let(:attr) { :username } + + it 'raises an error' do + expect { service.execute }.to raise_error(ArgumentError, 'Invalid attribute') + end + end + end +end diff --git a/spec/services/users/migrate_records_to_ghost_user_in_batches_service_spec.rb b/spec/services/users/migrate_records_to_ghost_user_in_batches_service_spec.rb new file mode 100644 index 00000000000..7366b1646b9 --- /dev/null +++ b/spec/services/users/migrate_records_to_ghost_user_in_batches_service_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::MigrateRecordsToGhostUserInBatchesService do + let(:service) { described_class.new } + + let_it_be(:ghost_user_migration) { create(:ghost_user_migration) } + + describe '#execute' do + it 'stops when execution time limit reached' do + expect_next_instance_of(::Gitlab::Utils::ExecutionTracker) do |tracker| + expect(tracker).to receive(:over_limit?).and_return(true) + end + + expect(Users::MigrateRecordsToGhostUserService).not_to receive(:new) + + service.execute + end + + it 'calls Users::MigrateRecordsToGhostUserService' do + expect_next_instance_of(Users::MigrateRecordsToGhostUserService) do |service| + expect(service).to( + receive(:execute) + .with(hard_delete: ghost_user_migration.hard_delete)) + end + + service.execute + end + end +end diff --git a/spec/services/users/migrate_records_to_ghost_user_service_spec.rb b/spec/services/users/migrate_records_to_ghost_user_service_spec.rb new file mode 100644 index 00000000000..766be51ae13 --- /dev/null +++ b/spec/services/users/migrate_records_to_ghost_user_service_spec.rb @@ -0,0 +1,259 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::MigrateRecordsToGhostUserService do + let!(:user) { create(:user) } + let(:service) { described_class.new(user, admin, execution_tracker) } + let(:execution_tracker) { instance_double(::Gitlab::Utils::ExecutionTracker, over_limit?: false) } + + let_it_be(:admin) { create(:admin) } + let_it_be(:project) { create(:project, :repository) } + + context "when migrating a user's associated records to the ghost user" do + context 'for issues' do + context 'when deleted user is present as both author and edited_user' do + include_examples 'migrating records to the ghost user', Issue, [:author, :last_edited_by] do + let(:created_record) do + create(:issue, project: project, author: user, last_edited_by: user) + end + end + end + + context 'when deleted user is present only as edited_user' do + include_examples 'migrating records to the ghost user', Issue, [:last_edited_by] do + let(:created_record) { create(:issue, project: project, author: create(:user), last_edited_by: user) } + end + end + + context "when deleted user is the assignee" do + let!(:issue) { create(:issue, project: project, assignees: [user]) } + + it 'migrates the issue so that it is "Unassigned"' do + service.execute + + migrated_issue = Issue.find_by_id(issue.id) + expect(migrated_issue).to be_present + expect(migrated_issue.assignees).to be_empty + end + end + end + + context 'for merge requests' do + context 'when deleted user is present as both author and merge_user' do + include_examples 'migrating records to the ghost user', MergeRequest, [:author, :merge_user] do + let(:created_record) do + create(:merge_request, source_project: project, + author: user, + merge_user: user, + target_branch: "first") + end + end + end + + context 'when deleted user is present only as both merge_user' do + include_examples 'migrating records to the ghost user', MergeRequest, [:merge_user] do + let(:created_record) do + create(:merge_request, source_project: project, + merge_user: user, + target_branch: "first") + end + end + end + + context "when deleted user is the assignee" do + let!(:merge_request) { create(:merge_request, source_project: project, assignees: [user]) } + + it 'migrates the merge request so that it is "Unassigned"' do + service.execute + + migrated_merge_request = MergeRequest.find_by_id(merge_request.id) + expect(migrated_merge_request).to be_present + expect(migrated_merge_request.assignees).to be_empty + end + end + end + + context 'for notes' do + include_examples 'migrating records to the ghost user', Note do + let(:created_record) { create(:note, project: project, author: user) } + end + end + + context 'for abuse reports' do + include_examples 'migrating records to the ghost user', AbuseReport do + let(:created_record) { create(:abuse_report, reporter: user, user: create(:user)) } + end + end + + context 'for award emoji' do + include_examples 'migrating records to the ghost user', AwardEmoji, [:user] do + let(:created_record) { create(:award_emoji, user: user) } + + context "when the awardable already has an award emoji of the same name assigned to the ghost user" do + let(:awardable) { create(:issue) } + let!(:existing_award_emoji) { create(:award_emoji, user: User.ghost, name: "thumbsup", awardable: awardable) } + let!(:award_emoji) { create(:award_emoji, user: user, name: "thumbsup", awardable: awardable) } + + it "migrates the award emoji regardless" do + service.execute + + migrated_record = AwardEmoji.find_by_id(award_emoji.id) + + expect(migrated_record.user).to eq(User.ghost) + end + + it "does not leave the migrated award emoji in an invalid state" do + service.execute + + migrated_record = AwardEmoji.find_by_id(award_emoji.id) + + expect(migrated_record).to be_valid + end + end + end + end + + context 'for snippets' do + include_examples 'migrating records to the ghost user', Snippet do + let(:created_record) { create(:snippet, project: project, author: user) } + end + end + + context 'for reviews' do + include_examples 'migrating records to the ghost user', Review, [:author] do + let(:created_record) { create(:review, author: user) } + end + end + end + + context 'on post-migrate cleanups' do + it 'destroys the user and personal namespace' do + namespace = user.namespace + + allow(user).to receive(:destroy).and_call_original + + service.execute + + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Namespace.find(namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'deletes user associations in batches' do + expect(user).to receive(:destroy_dependent_associations_in_batches) + + service.execute + end + + context 'for batched nullify' do + it 'nullifies related associations in batches' do + expect(user).to receive(:nullify_dependent_associations_in_batches).and_call_original + + service.execute + end + + it 'nullifies last_updated_issues, closed_issues, resource_label_events' do + issue = create(:issue, closed_by: user, updated_by: user) + resource_label_event = create(:resource_label_event, user: user) + + service.execute + + issue.reload + resource_label_event.reload + + expect(issue.closed_by).to be_nil + expect(issue.updated_by).to be_nil + expect(resource_label_event.user).to be_nil + end + end + + context 'for snippets' do + let(:gitlab_shell) { Gitlab::Shell.new } + + it 'does not include snippets when deleting in batches' do + expect(user).to receive(:destroy_dependent_associations_in_batches).with({ exclude: [:snippets] }) + + service.execute + end + + it 'calls the bulk snippet destroy service for the user personal snippets' do + repo1 = create(:personal_snippet, :repository, author: user).snippet_repository + repo2 = create(:project_snippet, :repository, project: project, author: user).snippet_repository + + aggregate_failures do + expect(gitlab_shell.repository_exists?(repo1.shard_name, "#{repo1.disk_path}.git")).to be(true) + expect(gitlab_shell.repository_exists?(repo2.shard_name, "#{repo2.disk_path}.git")).to be(true) + end + + # Call made when destroying user personal projects + expect(Snippets::BulkDestroyService).not_to( + receive(:new).with(admin, project.snippets).and_call_original) + + # Call to remove user personal snippets and for + # project snippets where projects are not user personal + # ones + expect(Snippets::BulkDestroyService).to( + receive(:new).with(admin, user.snippets.only_personal_snippets).and_call_original) + + service.execute + + aggregate_failures do + expect(gitlab_shell.repository_exists?(repo1.shard_name, "#{repo1.disk_path}.git")).to be(false) + expect(gitlab_shell.repository_exists?(repo2.shard_name, "#{repo2.disk_path}.git")).to be(true) + end + end + + it 'calls the bulk snippet destroy service with hard delete option if it is present' do + # this avoids getting into Projects::DestroyService as it would + # call Snippets::BulkDestroyService first! + allow(user).to receive(:personal_projects).and_return([]) + + expect_next_instance_of(Snippets::BulkDestroyService) do |bulk_destroy_service| + expect(bulk_destroy_service).to receive(:execute).with({ skip_authorization: true }).and_call_original + end + + service.execute(hard_delete: true) + end + + it 'does not delete project snippets that the user is the author of' do + repo = create(:project_snippet, :repository, author: user).snippet_repository + + service.execute + + expect(gitlab_shell.repository_exists?(repo.shard_name, "#{repo.disk_path}.git")).to be(true) + expect(User.ghost.snippets).to include(repo.snippet) + end + + context 'when an error is raised deleting snippets' do + it 'does not delete user' do + snippet = create(:personal_snippet, :repository, author: user) + + bulk_service = double + allow(Snippets::BulkDestroyService).to receive(:new).and_call_original + allow(Snippets::BulkDestroyService).to receive(:new).with(admin, user.snippets).and_return(bulk_service) + allow(bulk_service).to receive(:execute).and_return(ServiceResponse.error(message: 'foo')) + + aggregate_failures do + expect { service.execute }.to( + raise_error(Users::MigrateRecordsToGhostUserService::DestroyError, 'foo' )) + expect(snippet.reload).not_to be_nil + expect( + gitlab_shell.repository_exists?(snippet.repository_storage, + "#{snippet.disk_path}.git") + ).to be(true) + end + end + end + end + + context 'when hard_delete option is given' do + it 'will not ghost certain records' do + issue = create(:issue, author: user) + + service.execute(hard_delete: true) + + expect(Issue).not_to exist(issue.id) + end + end + end +end diff --git a/spec/services/users/reject_service_spec.rb b/spec/services/users/reject_service_spec.rb index 5a243e876ac..abff6b1e023 100644 --- a/spec/services/users/reject_service_spec.rb +++ b/spec/services/users/reject_service_spec.rb @@ -35,11 +35,29 @@ RSpec.describe Users::RejectService do context 'success' do context 'when the executor user is an admin in admin mode', :enable_admin_mode do - it 'deletes the user', :sidekiq_inline do - subject + context 'when user_destroy_with_limited_execution_time_worker is enabled' do + it 'initiates user removal', :sidekiq_inline do + subject + + expect(subject[:status]).to eq(:success) + expect( + Users::GhostUserMigration.where(user: user, + initiator_user: current_user) + ).to be_exists + end + end - expect(subject[:status]).to eq(:success) - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + context 'when user_destroy_with_limited_execution_time_worker is disabled' do + before do + stub_feature_flags(user_destroy_with_limited_execution_time_worker: false) + end + + it 'deletes the user', :sidekiq_inline do + subject + + expect(subject[:status]).to eq(:success) + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + end end it 'emails the user on rejection' do |