From c787435a214fe04be772787c13ce7e5c61a8efe2 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 31 Aug 2026 12:53:26 +0000 Subject: [PATCH] Fix invited-without-approval-bypass not being asked for a textual reason (#40332) --- app/models/user.rb | 2 +- app/views/auth/registrations/new.html.haml | 4 +- .../auth/registrations_controller_spec.rb | 88 -------------- spec/system/auth/registrations_spec.rb | 114 ++++++++++++++++++ 4 files changed, 117 insertions(+), 91 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 282c5bdfb75..774aff58b24 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -525,7 +525,7 @@ class User < ApplicationRecord end def invite_text_required? - Setting.require_invite_text && !open_registrations? && !invited? && !external? && !bypass_registration_checks? + Setting.require_invite_text && !open_registrations? && !invite&.bypass_approval? && !external? && !bypass_registration_checks? end def trigger_webhooks diff --git a/app/views/auth/registrations/new.html.haml b/app/views/auth/registrations/new.html.haml index 15e1bed973f..b416d273cfe 100644 --- a/app/views/auth/registrations/new.html.haml +++ b/app/views/auth/registrations/new.html.haml @@ -60,7 +60,7 @@ required: true, wrapper: :with_block_label - - if approved_registrations? && @invite.blank? + - if approved_registrations? && !@invite&.bypass_approval? .fields-group = f.simple_fields_for :invite_request, resource.invite_request || resource.build_invite_request do |invite_request_fields| = invite_request_fields.input :text, @@ -81,4 +81,4 @@ wrapper: :with_label .actions - = f.button :button, @invite.present? ? t('auth.register') : sign_up_message, type: :submit + = f.button :button, @invite&.bypass_approval? ? t('auth.register') : sign_up_message, type: :submit diff --git a/spec/controllers/auth/registrations_controller_spec.rb b/spec/controllers/auth/registrations_controller_spec.rb index 6531608a014..c9e27f712e9 100644 --- a/spec/controllers/auth/registrations_controller_spec.rb +++ b/spec/controllers/auth/registrations_controller_spec.rb @@ -252,94 +252,6 @@ RSpec.describe Auth::RegistrationsController do end end - context 'with Approval-based registrations without invite' do - subject do - Setting.registrations_mode = 'approved' - request.headers['Accept-Language'] = accept_language - post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', agreement: 'true' } } - end - - it 'redirects to setup and creates user' do - subject - - expect(response) - .to redirect_to auth_setup_path - - expect(User.find_by(email: 'test@example.com')) - .to be_present - .and have_attributes( - locale: eq(accept_language), - approved: be(false) - ) - end - end - - context 'with Approval-based registrations with expired invite' do - subject do - Setting.registrations_mode = 'approved' - request.headers['Accept-Language'] = accept_language - invite = Fabricate(:invite, max_uses: nil, expires_at: 1.hour.ago) - post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', invite_code: invite.code, agreement: 'true' } } - end - - it 'redirects to setup and creates user' do - subject - - expect(response).to redirect_to auth_setup_path - - expect(User.find_by(email: 'test@example.com')) - .to be_present - .and have_attributes( - locale: eq(accept_language), - approved: be(false) - ) - end - end - - context 'with Approval-based registrations with valid invite and required invite text' do - subject do - Setting.registrations_mode = 'approved' - Setting.require_invite_text = true - request.headers['Accept-Language'] = accept_language - invite = Fabricate(:invite, user: inviter, max_uses: nil, expires_at: 1.hour.from_now) - post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', invite_code: invite.code, agreement: 'true' } } - end - - let!(:inviter) { Fabricate(:user, confirmed_at: 2.days.ago) } - - it 'redirects to setup and creates user in a non-approved state' do - subject - - expect(response).to redirect_to auth_setup_path - - expect(User.find_by(email: 'test@example.com')) - .to be_present - .and have_attributes( - locale: eq(accept_language), - approved: be(false) - ) - end - - context 'when the inviting user has the permission to bypass approval' do - before do - inviter.role.update!(permissions: inviter.role.permissions | UserRole::FLAGS[:invite_bypass_approval]) - end - - it 'redirects to setup and creates user in an approved state' do - subject - - expect(response).to redirect_to auth_setup_path - - expect(User.find_by(email: 'test@example.com')) - .to be_present - .and have_attributes( - locale: eq(accept_language), - approved: be(true) - ) - end - end - end - context 'with an already taken username' do subject do Setting.registrations_mode = 'open' diff --git a/spec/system/auth/registrations_spec.rb b/spec/system/auth/registrations_spec.rb index 2cf0dfe46f3..acc4d89975f 100644 --- a/spec/system/auth/registrations_spec.rb +++ b/spec/system/auth/registrations_spec.rb @@ -36,6 +36,120 @@ RSpec.describe 'Auth Registration' do end end + context 'with approval-based registrations' do + subject { visit new_user_registration_path } + + shared_examples 'approval is required' do + context 'when reason to join is not required' do + before { Setting.require_invite_text = false } + + it 'asks for an optional reason to join, creates the user as unapproved' do + subject + + expect(page) + .to have_title(I18n.t('auth.register')) + + expect(page) + .to have_text(I18n.t('simple_form.labels.invite_request.text')) + + expect { fill_in_and_submit_form(apply: true) } + .to change(User, :count).by(1) + + expect(User.find_by(email: 'test@example.com')) + .to have_attributes(approved: false) + end + end + + context 'when reason to join is required' do + before { Setting.require_invite_text = true } + + it 'asks for a required reason to join, creates the user unapproved' do + subject + + expect(page) + .to have_title(I18n.t('auth.register')) + + expect(page) + .to have_text(I18n.t('simple_form.labels.invite_request.text')) + + # Not providing the invite text results in an error + expect { fill_in_and_submit_form(apply: true) } + .to_not change(User, :count) + expect(page) + .to have_text(/error below/) + + # Providing the invite text succeeds + expect { fill_in_and_submit_form(apply: true, invite_text: 'Hello world') } + .to change(User, :count).by(1) + + expect(User.find_by(email: 'test@example.com')) + .to have_attributes(approved: false) + end + end + end + + before do + Setting.registrations_mode = 'approved' + end + + it_behaves_like 'approval is required' + + context 'with an invitation' do + subject { visit new_user_registration_path(invite_code: invite.code) } + + let!(:inviter) { Fabricate(:user, confirmed_at: 2.days.ago, bypass_registration_checks: true) } + let(:invite) { Fabricate(:invite, user: inviter) } + + before do + inviter.approve! + end + + it_behaves_like 'approval is required' + + context 'when the invite allows bypassing approval' do + before do + inviter.role.update!(permissions: inviter.role.permissions | UserRole::FLAGS[:invite_bypass_approval]) + end + + it 'does not ask for a required reason to join, creates the user approved' do + subject + + expect(page) + .to have_title(I18n.t('auth.register')) + + expect(page) + .to have_no_text(I18n.t('simple_form.labels.invite_request.text')) + + expect { fill_in_and_submit_form(apply: false) } + .to change(User, :count).by(1) + + expect(User.find_by(email: 'test@example.com')) + .to have_attributes(approved: true) + end + end + + context 'when invite has expired' do + let(:invite) { Fabricate(:invite, user: inviter, expires_at: 1.hour.ago) } + + it_behaves_like 'approval is required' + end + end + + def fill_in_and_submit_form(apply: false, invite_text: nil) + # Avoid the registration spam check + travel_to 10.seconds.from_now + + fill_in 'user_account_attributes_username', with: 'test' + fill_in 'user_email', with: 'test@example.com' + fill_in 'user_password', with: 'Test.123.Pass' + fill_in 'user_password_confirmation', with: 'Test.123.Pass' + fill_in 'user_invite_request_attributes_text', with: invite_text if invite_text.present? + check 'user_agreement' + + click_on(apply ? I18n.t('auth.apply_for_account') : I18n.t('auth.register')) + end + end + context 'when age verification is enabled' do before { Setting.min_age = 16 }