From 255d8f3f8c6ece6a9f1fce9ea901d71577b92099 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 18 Aug 2025 18:37:13 +0200 Subject: [PATCH] Fix e-mail confirmation bypassing username approval (#35806) --- app/models/user.rb | 8 +++-- .../models/concerns/user/confirmation.rb | 36 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index aca72daff0..8e0785e7fd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -412,7 +412,7 @@ class User < ApplicationRecord def set_approved self.approved = begin - if sign_up_from_ip_requires_approval? || sign_up_email_requires_approval? || sign_up_username_requires_approval? + if requires_approval? false else open_registrations? || valid_invitation? || external? @@ -426,7 +426,11 @@ class User < ApplicationRecord def grant_approval_on_confirmation? # Re-check approval on confirmation if the server has switched to open registrations - open_registrations? && !sign_up_from_ip_requires_approval? && !sign_up_email_requires_approval? + open_registrations? && !requires_approval? + end + + def requires_approval? + sign_up_from_ip_requires_approval? || sign_up_email_requires_approval? || sign_up_username_requires_approval? end def wrap_email_confirmation diff --git a/spec/support/examples/models/concerns/user/confirmation.rb b/spec/support/examples/models/concerns/user/confirmation.rb index 4edc402f95..bb4d1b52b9 100644 --- a/spec/support/examples/models/concerns/user/confirmation.rb +++ b/spec/support/examples/models/concerns/user/confirmation.rb @@ -86,6 +86,42 @@ RSpec.shared_examples 'User::Confirmation' do end end + context 'when the user requires explicit approval because of signup IP address' do + let(:user) { Fabricate(:user, confirmed_at: nil, unconfirmed_email: new_email, approved: false, sign_up_ip: '192.0.2.5') } + + before do + Setting.registrations_mode = 'open' + Fabricate(:ip_block, ip: '192.0.2.5', severity: :sign_up_requires_approval) + end + + it 'sets email to new_email and marks user as confirmed, but not as approved and does not trigger `account.approved` web hook' do + expect { subject } + .to change { user.reload.email }.to(new_email) + .and change { user.reload.confirmed_at }.from(nil) + .and not_change { user.reload.approved }.from(false) + expect(TriggerWebhookWorker) + .to_not have_received(:perform_async).with('account.approved', 'Account', user.account_id) + end + end + + context 'when the user requires explicit approval because of username' do + let(:user) { Fabricate(:user, confirmed_at: nil, unconfirmed_email: new_email, approved: false, account_attributes: { username: 'VeryStrangeUsername' }) } + + before do + Setting.registrations_mode = 'open' + Fabricate(:username_block, username: 'StrangeUser', exact: false, allow_with_approval: true) + end + + it 'sets email to new_email and marks user as confirmed, but not as approved and does not trigger `account.approved` web hook' do + expect { subject } + .to change { user.reload.email }.to(new_email) + .and change { user.reload.confirmed_at }.from(nil) + .and not_change { user.reload.approved }.from(false) + expect(TriggerWebhookWorker) + .to_not have_received(:perform_async).with('account.approved', 'Account', user.account_id) + end + end + context 'when registrations mode is approved' do before { Setting.registrations_mode = 'approved' }