Unconfirmed user deletion skips users who have signed in
What does this MR do and why?
- If an instance has always had hard email confirmation turned on, then all users who are unconfirmed would have never signed in.
- If an instance had email confirmed off or "soft" at some point in the past, there may be users who signed in and were active on the platform for a time even though they have never confirmed their email.
- If a user wanted to sign in again, they would need to confirm their email. But we still want to remove any users with a sign_in_count > 0 here because we don't want to risk accidentally deleting dormant users rather than inactive users.
- Refinement of worker added via !122600 (merged)
- Internal discussion on this logic: #352514 (comment 1432041522)
- This also adds some validations on how many days to wait after deleting unconfirmed users so that when email confirmation is set to 'soft' this setting isn't used to delete users who are still in the window to confirm their accounts.
- This logic is all behind the
delete_unconfirmed_users_setting
feature flag
Changelog: changed
Database Migration
- Query with old index: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/19696/commands/64506
- Query with new index: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/19696/commands/64514
How to set up and validate locally
This requires a Premium license.
-
In rails console enable feature flag and needed settings
Feature.enable(:delete_unconfirmed_users_setting) Gitlab::CurrentSettings.update(delete_unconfirmed_users: true) Gitlab::CurrentSettings.update(unconfirmed_users_delete_after_days: 30) Gitlab::CurrentSettings.update(email_confirmation_setting: 'hard')
-
Create a user who is unconfirmed and created more than 30 days ago
FactoryBot.create(:user, :unconfirmed, created_at: 1.year.ago, email: 'deleteme@example.com', username: 'unconfirmed_person') FactoryBot.create(:user, :unconfirmed, created_at: 1.year.ago, email: 'donotdeleteme@example.com', username: 'unconfirmed_person_who_signed_in_once', sign_in_count: 1)
-
Wait for the hourly cron job or kick it off manually
Users::UnconfirmedUsersDeletionCronWorker.new.perform
-
The user who never signed in should have a ghost migration user (these are processed async to delete the user)
Users::GhostUserMigration.where(user_id: User.find_by_username('unconfirmed_person').id).first => present Users::GhostUserMigration.where(user_id: User.find_by_username('unconfirmed_person_who_signed_in_once').id).first => nil
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by Jessie Young