Merge branch 'dblessing_account_deletion_com' into 'master'

GitLab.com users without password must contact to delete account

See merge request gitlab-org/gitlab!49626

app/models/user.rb

@@ -1493,6 +1493,10 @@ class User < ApplicationRecord
     !solo_owned_groups.present?
   end
 
+  def can_remove_self?
+    true
+  end
+
   def ci_owned_runners
     @ci_owned_runners ||= begin
       project_runners = Ci::RunnerProject

app/views/profiles/accounts/show.html.haml

@@ -79,6 +79,11 @@
           %strong= current_user.solo_owned_groups.map(&:name).join(', ')
         %p
           = s_('Profiles|You must transfer ownership or delete these groups before you can delete your account.')
+      - elsif !current_user.can_remove_self?
+        %p
+          = s_('Profiles|GitLab is unable to verify your identity automatically.')
+        %p
+          = s_('Profiles|Please email %{data_request} to begin the account deletion process.').html_safe % { data_request: mail_to('personal-data-request@gitlab.com') }
       - else
         %p
           = s_("Profiles|You don't have access to delete this user.")

ee/app/controllers/ee/registrations_controller.rb

@@ -5,11 +5,23 @@ module EE
     extend ActiveSupport::Concern
     extend ::Gitlab::Utils::Override
 
+    prepended do
+      before_action :ensure_can_remove_self, only: [:destroy]
+    end
+
     private
 
     override :set_blocked_pending_approval?
     def set_blocked_pending_approval?
       super || ::Gitlab::CurrentSettings.should_apply_user_signup_cap?
     end
+
+    def ensure_can_remove_self
+      unless current_user&.can_remove_self?
+        redirect_to profile_account_path,
+                    status: :see_other,
+                    alert: s_('Profiles|Account could not be deleted. GitLab was unable to verify your identity.')
+      end
+    end
   end
 end

ee/app/models/ee/user.rb

@@ -377,6 +377,16 @@ module EE
         board_id: board_id, epic_id: epic_id)
     end
 
+    # GitLab.com users should not be able to remove themselves
+    # when they cannot verify their local password, because it
+    # isn't set (using third party authentication).
+    override :can_remove_self?
+    def can_remove_self?
+      return true unless ::Gitlab.com?
+
+      !password_automatically_set?
+    end
+
     protected
 
     override :password_required?

ee/app/policies/ee/user_policy.rb

@@ -10,12 +10,18 @@ module EE
         ::Gitlab::CurrentSettings.current_application_settings.updating_name_disabled_for_users
       end
 
+      condition(:can_remove_self, scope: :subject) do
+        @subject.can_remove_self?
+      end
+
       rule { can?(:update_user) }.enable :update_name
 
       rule do
         updating_name_disabled_for_users &
         ~admin
       end.prevent :update_name
+
+      rule { user_is_self & ~can_remove_self }.prevent :destroy_user
     end
   end
 end

ee/changelogs/unreleased/dblessing_account_deletion_com.yml

+---
+title: GitLab.com users without password must contact to delete account
+merge_request: 49626
+author:
+type: added

ee/spec/controllers/ee/registrations_controller_spec.rb

@@ -126,4 +126,31 @@ RSpec.describe RegistrationsController do
       end
     end
   end
+
+  describe '#destroy' do
+    let(:user) { create(:user) }
+
+    before do
+      user.update!(password_automatically_set: true)
+      sign_in(user)
+    end
+
+    context 'on GitLab.com when the password is automatically set' do
+      before do
+        stub_application_setting(password_authentication_enabled_for_web: false)
+        stub_application_setting(password_authentication_enabled_for_git: false)
+        allow(::Gitlab).to receive(:com?).and_return(true)
+      end
+
+      it 'redirects without deleting the account' do
+        expect(DeleteUserWorker).not_to receive(:perform_async)
+
+        post :destroy, params: { username: user.username }
+
+        expect(flash[:alert]).to eq 'Account could not be deleted. GitLab was unable to verify your identity.'
+        expect(response).to have_gitlab_http_status(:see_other)
+        expect(response).to redirect_to profile_account_path
+      end
+    end
+  end
 end

ee/spec/features/profiles/account_spec.rb

@@ -82,4 +82,22 @@ RSpec.describe 'Profile > Account' do
       end
     end
   end
+
+  describe 'Delete account' do
+    context "on GitLab.com when the user's password is automatically set" do
+      before do
+        allow(::Gitlab).to receive(:com?).and_return(true)
+        user.update!(password_automatically_set: true)
+        visit profile_account_path
+      end
+
+      it 'shows that the identity cannot be verified' do
+        expect(page).to have_content 'GitLab is unable to verify your identity automatically.'
+      end
+
+      it 'does not display a delete button' do
+        expect(page).not_to have_button 'Delete account'
+      end
+    end
+  end
 end

ee/spec/models/ee/user_spec.rb

@@ -1571,4 +1571,42 @@ RSpec.describe User do
       end
     end
   end
+
+  describe '#can_remove_self?' do
+    let(:user) { create(:user) }
+
+    subject { user.can_remove_self? }
+
+    context 'not on GitLab.com' do
+      context 'when the password is not automatically set' do
+        it { is_expected.to eq true }
+      end
+
+      context 'when the password is automatically set' do
+        before do
+          user.password_automatically_set = true
+        end
+
+        it { is_expected.to eq true }
+      end
+    end
+
+    context 'on GitLab.com' do
+      before do
+        allow(::Gitlab).to receive(:com?).and_return(true)
+      end
+
+      context 'when the password is not automatically set' do
+        it { is_expected.to eq true }
+      end
+
+      context 'when the password is automatically set' do
+        before do
+          user.password_automatically_set = true
+        end
+
+        it { is_expected.to eq false }
+      end
+    end
+  end
 end

ee/spec/policies/user_policy_spec.rb

@@ -96,4 +96,34 @@ RSpec.describe UserPolicy do
       it_behaves_like 'changing a user', :update_name
     end
   end
+
+  describe ':destroy_user' do
+    context 'when user is not self', :enable_admin_mode do
+      let(:current_user) { create(:user, :admin) }
+
+      it { is_expected.to be_allowed(:destroy_user) }
+    end
+
+    context 'when user is self' do
+      let(:current_user) { user }
+
+      it { is_expected.to be_allowed(:destroy_user) }
+
+      context 'when the user password is automatically set' do
+        before do
+          current_user.update!(password_automatically_set: true)
+        end
+
+        it { is_expected.to be_allowed(:destroy_user) }
+
+        context 'on GitLab.com' do
+          before do
+            allow(::Gitlab).to receive(:com?).and_return(true)
+          end
+
+          it { is_expected.not_to be_allowed(:destroy_user) }
+        end
+      end
+    end
+  end
 end

locale/gitlab.pot

@@ -21091,6 +21091,9 @@ msgstr ""
 msgid "Profiles|@username"
 msgstr ""
 
+msgid "Profiles|Account could not be deleted. GitLab was unable to verify your identity."
+msgstr ""
+
 msgid "Profiles|Account scheduled for removal."
 msgstr ""
 
@@ -21193,6 +21196,9 @@ msgstr ""
 msgid "Profiles|Full name"
 msgstr ""
 
+msgid "Profiles|GitLab is unable to verify your identity automatically."
+msgstr ""
+
 msgid "Profiles|Give your individual key a title."
 msgstr ""
 
@@ -21241,6 +21247,9 @@ msgstr ""
 msgid "Profiles|Path"
 msgstr ""
 
+msgid "Profiles|Please email %{data_request} to begin the account deletion process."
+msgstr ""
+
 msgid "Profiles|Position and size your new avatar"
 msgstr ""
 

spec/models/user_spec.rb

@@ -3098,6 +3098,14 @@ RSpec.describe User do
     end
   end
 
+  describe '#can_remove_self?' do
+    let(:user) { create(:user) }
+
+    it 'returns true' do
+      expect(user.can_remove_self?).to eq true
+    end
+  end
+
   describe "#recent_push" do
     let(:user) { build(:user) }
     let(:project) { build(:project) }