Fix account activation being triggered before email confirmation (#23245)
* Add tests * Fix account activation being triggered before email confirmation Fixes #23098gh/stable
parent
4725191d3c
commit
6883fddb19
|
@ -195,10 +195,16 @@ class User < ApplicationRecord
|
|||
|
||||
super
|
||||
|
||||
if new_user && approved?
|
||||
prepare_new_user!
|
||||
elsif new_user
|
||||
notify_staff_about_pending_account!
|
||||
if new_user
|
||||
# Avoid extremely unlikely race condition when approving and confirming
|
||||
# the user at the same time
|
||||
reload unless approved?
|
||||
|
||||
if approved?
|
||||
prepare_new_user!
|
||||
else
|
||||
notify_staff_about_pending_account!
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -209,7 +215,13 @@ class User < ApplicationRecord
|
|||
skip_confirmation!
|
||||
save!
|
||||
|
||||
prepare_new_user! if new_user && approved?
|
||||
if new_user
|
||||
# Avoid extremely unlikely race condition when approving and confirming
|
||||
# the user at the same time
|
||||
reload unless approved?
|
||||
|
||||
prepare_new_user! if approved?
|
||||
end
|
||||
end
|
||||
|
||||
def update_sign_in!(new_sign_in: false)
|
||||
|
@ -260,7 +272,11 @@ class User < ApplicationRecord
|
|||
return if approved?
|
||||
|
||||
update!(approved: true)
|
||||
prepare_new_user!
|
||||
|
||||
# Avoid extremely unlikely race condition when approving and confirming
|
||||
# the user at the same time
|
||||
reload unless confirmed?
|
||||
prepare_new_user! if confirmed?
|
||||
end
|
||||
|
||||
def otp_enabled?
|
||||
|
|
|
@ -142,10 +142,136 @@ RSpec.describe User, type: :model do
|
|||
end
|
||||
|
||||
describe '#confirm' do
|
||||
it 'sets email to unconfirmed_email' do
|
||||
user = Fabricate.build(:user, confirmed_at: Time.now.utc, unconfirmed_email: 'new-email@example.com')
|
||||
user.confirm
|
||||
expect(user.email).to eq 'new-email@example.com'
|
||||
let(:new_email) { 'new-email@example.com' }
|
||||
|
||||
subject { user.confirm }
|
||||
|
||||
before do
|
||||
allow(TriggerWebhookWorker).to receive(:perform_async)
|
||||
end
|
||||
|
||||
context 'when the user is already confirmed' do
|
||||
let!(:user) { Fabricate(:user, confirmed_at: Time.now.utc, approved: true, unconfirmed_email: new_email) }
|
||||
|
||||
it 'sets email to unconfirmed_email' do
|
||||
expect { subject }.to change { user.reload.email }.to(new_email)
|
||||
end
|
||||
|
||||
it 'does not trigger the account.approved Web Hook' do
|
||||
subject
|
||||
expect(TriggerWebhookWorker).not_to have_received(:perform_async).with('account.approved', 'Account', user.account_id)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the user is a new user' do
|
||||
let(:user) { Fabricate(:user, confirmed_at: nil, unconfirmed_email: new_email) }
|
||||
|
||||
context 'when the user is already approved' do
|
||||
around(:example) do |example|
|
||||
registrations_mode = Setting.registrations_mode
|
||||
Setting.registrations_mode = 'approved'
|
||||
|
||||
example.run
|
||||
|
||||
Setting.registrations_mode = registrations_mode
|
||||
end
|
||||
|
||||
before do
|
||||
user.approve!
|
||||
end
|
||||
|
||||
it 'sets email to unconfirmed_email' do
|
||||
expect { subject }.to change { user.reload.email }.to(new_email)
|
||||
end
|
||||
|
||||
it 'triggers the account.approved Web Hook' do
|
||||
user.confirm
|
||||
expect(TriggerWebhookWorker).to have_received(:perform_async).with('account.approved', 'Account', user.account_id).once
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the user does not require explicit approval' do
|
||||
around(:example) do |example|
|
||||
registrations_mode = Setting.registrations_mode
|
||||
Setting.registrations_mode = 'open'
|
||||
|
||||
example.run
|
||||
|
||||
Setting.registrations_mode = registrations_mode
|
||||
end
|
||||
|
||||
it 'sets email to unconfirmed_email' do
|
||||
expect { subject }.to change { user.reload.email }.to(new_email)
|
||||
end
|
||||
|
||||
it 'triggers the account.approved Web Hook' do
|
||||
user.confirm
|
||||
expect(TriggerWebhookWorker).to have_received(:perform_async).with('account.approved', 'Account', user.account_id).once
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the user requires explicit approval but is not approved' do
|
||||
around(:example) do |example|
|
||||
registrations_mode = Setting.registrations_mode
|
||||
Setting.registrations_mode = 'approved'
|
||||
|
||||
example.run
|
||||
|
||||
Setting.registrations_mode = registrations_mode
|
||||
end
|
||||
|
||||
it 'sets email to unconfirmed_email' do
|
||||
expect { subject }.to change { user.reload.email }.to(new_email)
|
||||
end
|
||||
|
||||
it 'does not trigger the account.approved Web Hook' do
|
||||
subject
|
||||
expect(TriggerWebhookWorker).to_not have_received(:perform_async).with('account.approved', 'Account', user.account_id)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#approve!' do
|
||||
subject { user.approve! }
|
||||
|
||||
around(:example) do |example|
|
||||
registrations_mode = Setting.registrations_mode
|
||||
Setting.registrations_mode = 'approved'
|
||||
|
||||
example.run
|
||||
|
||||
Setting.registrations_mode = registrations_mode
|
||||
end
|
||||
|
||||
before do
|
||||
allow(TriggerWebhookWorker).to receive(:perform_async)
|
||||
end
|
||||
|
||||
context 'when the user is already confirmed' do
|
||||
let(:user) { Fabricate(:user, confirmed_at: Time.now.utc, approved: false) }
|
||||
|
||||
it 'sets the approved flag' do
|
||||
expect { subject }.to change { user.reload.approved? }.to(true)
|
||||
end
|
||||
|
||||
it 'triggers the account.approved Web Hook' do
|
||||
subject
|
||||
expect(TriggerWebhookWorker).to have_received(:perform_async).with('account.approved', 'Account', user.account_id).once
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the user is not confirmed' do
|
||||
let(:user) { Fabricate(:user, confirmed_at: nil, approved: false) }
|
||||
|
||||
it 'sets the approved flag' do
|
||||
expect { subject }.to change { user.reload.approved? }.to(true)
|
||||
end
|
||||
|
||||
it 'does not trigger the account.approved Web Hook' do
|
||||
subject
|
||||
expect(TriggerWebhookWorker).not_to have_received(:perform_async).with('account.approved', 'Account', user.account_id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
Reference in New Issue