Merge branch...

Merge branch '212539-it-is-not-possible-to-force-user-confirmation-from-admin-area-if-confirmation-was-expired' into 'master'

Resolve "It is not possible to force user confirmation from Admin Area if confirmation was expired"

See merge request gitlab-org/gitlab!77287

app/controllers/admin/users_controller.rb

@@ -158,7 +158,7 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def confirm
-    if update_user { |user| user.confirm }
+    if update_user { |user| user.force_confirm }
       redirect_back_or_admin_user(notice: _("Successfully confirmed"))
     else
       redirect_back_or_admin_user(alert: _("Error occurred. User was not confirmed"))

app/models/concerns/forced_email_confirmation.rb

+# frozen_string_literal: true
+
+module ForcedEmailConfirmation
+  extend ActiveSupport::Concern
+
+  included do
+    attr_accessor :skip_confirmation_period_expiry_check
+  end
+
+  def force_confirm(args = {})
+    self.skip_confirmation_period_expiry_check = true
+    confirm(args)
+  ensure
+    self.skip_confirmation_period_expiry_check = nil
+  end
+
+  protected
+
+  # Override, from Devise::Models::Confirmable
+  # Link: https://github.com/heartcombo/devise/blob/main/lib/devise/models/confirmable.rb
+  def confirmation_period_expired?
+    return false if skip_confirmation_period_expiry_check
+
+    super
+  end
+end

app/models/email.rb

@@ -19,6 +19,7 @@ class Email < ApplicationRecord
   # This module adds async behaviour to Devise emails
   # and should be added after Devise modules are initialized.
   include AsyncDeviseEmail
+  include ForcedEmailConfirmation
 
   self.reconfirmable = false # currently email can't be changed, no need to reconfirm
 

app/models/user.rb

@@ -81,6 +81,7 @@ class User < ApplicationRecord
   # This module adds async behaviour to Devise emails
   # and should be added after Devise modules are initialized.
   include AsyncDeviseEmail
+  include ForcedEmailConfirmation
 
   MINIMUM_INACTIVE_DAYS = 90
 
@@ -1974,18 +1975,22 @@ class User < ApplicationRecord
     ci_job_token_scope.present?
   end
 
-  # override from Devise::Confirmable
+  # override from Devise::Models::Confirmable
   #
   # Add the primary email to user.emails (or confirm it if it was already
   # present) when the primary email is confirmed.
-  def confirm(*args)
-    saved = super(*args)
+  def confirm(args = {})
+    saved = super(args)
     return false unless saved
 
     email_to_confirm = self.emails.find_by(email: self.email)
 
     if email_to_confirm.present?
-      email_to_confirm.confirm(*args)
+      if skip_confirmation_period_expiry_check
+        email_to_confirm.force_confirm(args)
+      else
+        email_to_confirm.confirm(args)
+      end
     else
       add_primary_email_to_emails!
     end

spec/controllers/admin/users_controller_spec.rb

@@ -421,16 +421,37 @@ RSpec.describe Admin::UsersController do
   end
 
   describe 'PUT confirm/:id' do
-    let(:user) { create(:user, confirmed_at: nil) }
+    shared_examples_for 'confirms the user' do
+      it 'confirms the user' do
+        put :confirm, params: { id: user.username }
+        user.reload
+        expect(user.confirmed?).to be_truthy
+      end
+    end
+
+    let(:expired_confirmation_sent_at) { Date.today - User.confirm_within - 7.days }
+    let(:extant_confirmation_sent_at) { Date.today }
+
+    let(:user) do
+      create(:user, :unconfirmed).tap do |user|
+        user.update!(confirmation_sent_at: confirmation_sent_at)
+      end
+    end
 
     before do
       request.env["HTTP_REFERER"] = "/"
     end
 
-    it 'confirms user' do
-      put :confirm, params: { id: user.username }
-      user.reload
-      expect(user.confirmed?).to be_truthy
+    context 'when the confirmation period has expired' do
+      let(:confirmation_sent_at) {  expired_confirmation_sent_at }
+
+      it_behaves_like 'confirms the user'
+    end
+
+    context 'when the confirmation period has not expired' do
+      let(:confirmation_sent_at) {  extant_confirmation_sent_at }
+
+      it_behaves_like 'confirms the user'
     end
   end
 

spec/models/email_spec.rb

@@ -71,4 +71,84 @@ RSpec.describe Email do
       end
     end
   end
+
+  describe '#confirm' do
+    let(:expired_confirmation_sent_at) { Date.today - described_class.confirm_within - 7.days }
+    let(:extant_confirmation_sent_at) { Date.today }
+
+    let(:email) do
+      create(:email, email: 'test@gitlab.com').tap do |email|
+        email.update!(confirmation_sent_at: confirmation_sent_at)
+      end
+    end
+
+    shared_examples_for 'unconfirmed email' do
+      it 'returns unconfirmed' do
+        expect(email.confirmed?).to be_falsey
+      end
+    end
+
+    context 'when the confirmation period has expired' do
+      let(:confirmation_sent_at) {  expired_confirmation_sent_at }
+
+      it_behaves_like 'unconfirmed email'
+
+      it 'does not confirm the email' do
+        email.confirm
+
+        expect(email.confirmed?).to be_falsey
+      end
+    end
+
+    context 'when the confirmation period has not expired' do
+      let(:confirmation_sent_at) {  extant_confirmation_sent_at }
+
+      it_behaves_like 'unconfirmed email'
+
+      it 'confirms the email' do
+        email.confirm
+
+        expect(email.confirmed?).to be_truthy
+      end
+    end
+  end
+
+  describe '#force_confirm' do
+    let(:expired_confirmation_sent_at) { Date.today - described_class.confirm_within - 7.days }
+    let(:extant_confirmation_sent_at) { Date.today }
+
+    let(:email) do
+      create(:email, email: 'test@gitlab.com').tap do |email|
+        email.update!(confirmation_sent_at: confirmation_sent_at)
+      end
+    end
+
+    shared_examples_for 'unconfirmed email' do
+      it 'returns unconfirmed' do
+        expect(email.confirmed?).to be_falsey
+      end
+    end
+
+    shared_examples_for 'confirms the email on force_confirm' do
+      it 'confirms an email' do
+        email.force_confirm
+
+        expect(email.reload.confirmed?).to be_truthy
+      end
+    end
+
+    context 'when the confirmation period has expired' do
+      let(:confirmation_sent_at) {  expired_confirmation_sent_at }
+
+      it_behaves_like 'unconfirmed email'
+      it_behaves_like 'confirms the email on force_confirm'
+    end
+
+    context 'when the confirmation period has not expired' do
+      let(:confirmation_sent_at) {  extant_confirmation_sent_at }
+
+      it_behaves_like 'unconfirmed email'
+      it_behaves_like 'confirms the email on force_confirm'
+    end
+  end
 end

spec/models/user_spec.rb

@@ -1481,27 +1481,176 @@ RSpec.describe User do
   end
 
   describe '#confirm' do
+    let(:expired_confirmation_sent_at) { Date.today - described_class.confirm_within - 7.days }
+    let(:extant_confirmation_sent_at) { Date.today }
+
     before do
       allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true)
     end
 
-    let(:user) { create(:user, :unconfirmed, unconfirmed_email: 'test@gitlab.com') }
+    let(:user) do
+      create(:user, :unconfirmed, unconfirmed_email: 'test@gitlab.com').tap do |user|
+        user.update!(confirmation_sent_at: confirmation_sent_at)
+      end
+    end
 
-    it 'returns unconfirmed' do
-      expect(user.confirmed?).to be_falsey
+    shared_examples_for 'unconfirmed user' do
+      it 'returns unconfirmed' do
+        expect(user.confirmed?).to be_falsey
+      end
     end
 
-    it 'confirms a user' do
-      user.confirm
-      expect(user.confirmed?).to be_truthy
+    context 'when the confirmation period has expired' do
+      let(:confirmation_sent_at) {  expired_confirmation_sent_at }
+
+      it_behaves_like 'unconfirmed user'
+
+      it 'does not confirm the user' do
+        user.confirm
+
+        expect(user.confirmed?).to be_falsey
+      end
+
+      it 'does not add the confirmed primary email to emails' do
+        user.confirm
+
+        expect(user.emails.confirmed.map(&:email)).not_to include(user.email)
+      end
     end
 
-    it 'adds the confirmed primary email to emails' do
-      expect(user.emails.confirmed.map(&:email)).not_to include(user.email)
+    context 'when the confirmation period has not expired' do
+      let(:confirmation_sent_at) {  extant_confirmation_sent_at }
 
-      user.confirm
+      it_behaves_like 'unconfirmed user'
 
-      expect(user.emails.confirmed.map(&:email)).to include(user.email)
+      it 'confirms a user' do
+        user.confirm
+        expect(user.confirmed?).to be_truthy
+      end
+
+      it 'adds the confirmed primary email to emails' do
+        expect(user.emails.confirmed.map(&:email)).not_to include(user.email)
+
+        user.confirm
+
+        expect(user.emails.confirmed.map(&:email)).to include(user.email)
+      end
+
+      context 'when the primary email is already included in user.emails' do
+        let(:expired_confirmation_sent_at_for_email) { Date.today - Email.confirm_within - 7.days }
+        let(:extant_confirmation_sent_at_for_email) { Date.today }
+
+        let!(:email) do
+          create(:email, email: user.unconfirmed_email, user: user).tap do |email|
+            email.update!(confirmation_sent_at: confirmation_sent_at_for_email)
+          end
+        end
+
+        context 'when the confirmation period of the email record has expired' do
+          let(:confirmation_sent_at_for_email) { expired_confirmation_sent_at_for_email }
+
+          it 'does not confirm the email record' do
+            user.confirm
+
+            expect(email.reload.confirmed?).to be_falsey
+          end
+        end
+
+        context 'when the confirmation period of the email record has not expired' do
+          let(:confirmation_sent_at_for_email) { extant_confirmation_sent_at_for_email }
+
+          it 'confirms the email record' do
+            user.confirm
+
+            expect(email.reload.confirmed?).to be_truthy
+          end
+        end
+      end
+    end
+  end
+
+  describe '#force_confirm' do
+    let(:expired_confirmation_sent_at) { Date.today - described_class.confirm_within - 7.days }
+    let(:extant_confirmation_sent_at) { Date.today }
+
+    let(:user) do
+      create(:user, :unconfirmed, unconfirmed_email: 'test@gitlab.com').tap do |user|
+        user.update!(confirmation_sent_at: confirmation_sent_at)
+      end
+    end
+
+    shared_examples_for 'unconfirmed user' do
+      it 'returns unconfirmed' do
+        expect(user.confirmed?).to be_falsey
+      end
+    end
+
+    shared_examples_for 'confirms the user on force_confirm' do
+      it 'confirms a user' do
+        user.force_confirm
+        expect(user.confirmed?).to be_truthy
+      end
+    end
+
+    shared_examples_for 'adds the confirmed primary email to emails' do
+      it 'adds the confirmed primary email to emails' do
+        expect(user.emails.confirmed.map(&:email)).not_to include(user.email)
+
+        user.force_confirm
+
+        expect(user.emails.confirmed.map(&:email)).to include(user.email)
+      end
+    end
+
+    shared_examples_for 'confirms the email record if the primary email was already present in user.emails' do
+      context 'when the primary email is already included in user.emails' do
+        let(:expired_confirmation_sent_at_for_email) { Date.today - Email.confirm_within - 7.days }
+        let(:extant_confirmation_sent_at_for_email) { Date.today }
+
+        let!(:email) do
+          create(:email, email: user.unconfirmed_email, user: user).tap do |email|
+            email.update!(confirmation_sent_at: confirmation_sent_at_for_email)
+          end
+        end
+
+        shared_examples_for 'confirms the email record' do
+          it 'confirms the email record' do
+            user.force_confirm
+
+            expect(email.reload.confirmed?).to be_truthy
+          end
+        end
+
+        context 'when the confirmation period of the email record has expired' do
+          let(:confirmation_sent_at_for_email) { expired_confirmation_sent_at_for_email }
+
+          it_behaves_like 'confirms the email record'
+        end
+
+        context 'when the confirmation period of the email record has not expired' do
+          let(:confirmation_sent_at_for_email) { extant_confirmation_sent_at_for_email }
+
+          it_behaves_like 'confirms the email record'
+        end
+      end
+    end
+
+    context 'when the confirmation period has expired' do
+      let(:confirmation_sent_at) {  expired_confirmation_sent_at }
+
+      it_behaves_like 'unconfirmed user'
+      it_behaves_like 'confirms the user on force_confirm'
+      it_behaves_like 'adds the confirmed primary email to emails'
+      it_behaves_like 'confirms the email record if the primary email was already present in user.emails'
+    end
+
+    context 'when the confirmation period has not expired' do
+      let(:confirmation_sent_at) {  extant_confirmation_sent_at }
+
+      it_behaves_like 'unconfirmed user'
+      it_behaves_like 'confirms the user on force_confirm'
+      it_behaves_like 'adds the confirmed primary email to emails'
+      it_behaves_like 'confirms the email record if the primary email was already present in user.emails'
     end
   end