Fix registrations not checking MX records for email domain blocks requiring approval (#28608)
parent
a2f02a0775
commit
e621c1c44c
|
@ -149,6 +149,10 @@ class User < ApplicationRecord
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def self.skip_mx_check?
|
||||||
|
Rails.env.local?
|
||||||
|
end
|
||||||
|
|
||||||
def role
|
def role
|
||||||
if role_id.nil?
|
if role_id.nil?
|
||||||
UserRole.everyone
|
UserRole.everyone
|
||||||
|
@ -436,9 +440,24 @@ class User < ApplicationRecord
|
||||||
end
|
end
|
||||||
|
|
||||||
def sign_up_email_requires_approval?
|
def sign_up_email_requires_approval?
|
||||||
return false unless email.present? || unconfirmed_email.present?
|
return false if email.blank?
|
||||||
|
|
||||||
EmailDomainBlock.requires_approval?(email.presence || unconfirmed_email, attempt_ip: sign_up_ip)
|
_, domain = email.split('@', 2)
|
||||||
|
return false if domain.blank?
|
||||||
|
|
||||||
|
records = []
|
||||||
|
|
||||||
|
# Doing this conditionally is not very satisfying, but this is consistent
|
||||||
|
# with the MX records validations we do and keeps the specs tractable.
|
||||||
|
unless self.class.skip_mx_check?
|
||||||
|
Resolv::DNS.open do |dns|
|
||||||
|
dns.timeouts = 5
|
||||||
|
|
||||||
|
records = dns.getresources(domain, Resolv::DNS::Resource::IN::MX).to_a.map { |e| e.exchange.to_s }.compact_blank
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
EmailDomainBlock.requires_approval?(records + [domain], attempt_ip: sign_up_ip)
|
||||||
end
|
end
|
||||||
|
|
||||||
def open_registrations?
|
def open_registrations?
|
||||||
|
@ -489,7 +508,7 @@ class User < ApplicationRecord
|
||||||
end
|
end
|
||||||
|
|
||||||
def validate_email_dns?
|
def validate_email_dns?
|
||||||
email_changed? && !external? && !Rails.env.local?
|
email_changed? && !external? && !self.class.skip_mx_check?
|
||||||
end
|
end
|
||||||
|
|
||||||
def validate_role_elevation
|
def validate_role_elevation
|
||||||
|
|
|
@ -137,12 +137,49 @@ RSpec.describe Auth::RegistrationsController do
|
||||||
|
|
||||||
context 'when user has an email address requiring approval' do
|
context 'when user has an email address requiring approval' do
|
||||||
subject do
|
subject do
|
||||||
Setting.registrations_mode = 'open'
|
|
||||||
Fabricate(:email_domain_block, allow_with_approval: true, domain: 'example.com')
|
|
||||||
request.headers['Accept-Language'] = accept_language
|
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' } }
|
post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', agreement: 'true' } }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
before do
|
||||||
|
Setting.registrations_mode = 'open'
|
||||||
|
Fabricate(:email_domain_block, allow_with_approval: true, domain: 'example.com')
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'creates unapproved user and redirects to setup' do
|
||||||
|
subject
|
||||||
|
expect(response).to redirect_to auth_setup_path
|
||||||
|
|
||||||
|
user = User.find_by(email: 'test@example.com')
|
||||||
|
expect(user).to_not be_nil
|
||||||
|
expect(user.locale).to eq(accept_language)
|
||||||
|
expect(user.approved).to be(false)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when user has an email address requiring approval through a MX record' do
|
||||||
|
subject do
|
||||||
|
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
|
||||||
|
|
||||||
|
before do
|
||||||
|
Setting.registrations_mode = 'open'
|
||||||
|
Fabricate(:email_domain_block, allow_with_approval: true, domain: 'mail.example.com')
|
||||||
|
allow(User).to receive(:skip_mx_check?).and_return(false)
|
||||||
|
|
||||||
|
resolver = instance_double(Resolv::DNS, :timeouts= => nil)
|
||||||
|
|
||||||
|
allow(resolver).to receive(:getresources)
|
||||||
|
.with('example.com', Resolv::DNS::Resource::IN::MX)
|
||||||
|
.and_return([instance_double(Resolv::DNS::Resource::MX, exchange: 'mail.example.com')])
|
||||||
|
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([])
|
||||||
|
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([])
|
||||||
|
allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::A).and_return([instance_double(Resolv::DNS::Resource::IN::A, address: '2.3.4.5')])
|
||||||
|
allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::AAAA).and_return([instance_double(Resolv::DNS::Resource::IN::AAAA, address: 'fd00::2')])
|
||||||
|
allow(Resolv::DNS).to receive(:open).and_yield(resolver)
|
||||||
|
end
|
||||||
|
|
||||||
it 'creates unapproved user and redirects to setup' do
|
it 'creates unapproved user and redirects to setup' do
|
||||||
subject
|
subject
|
||||||
expect(response).to redirect_to auth_setup_path
|
expect(response).to redirect_to auth_setup_path
|
||||||
|
|
|
@ -48,6 +48,39 @@ RSpec.describe AppSignUpService, type: :service do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when the email address requires approval through MX records' do
|
||||||
|
before do
|
||||||
|
Setting.registrations_mode = 'open'
|
||||||
|
Fabricate(:email_domain_block, allow_with_approval: true, domain: 'smtp.email.com')
|
||||||
|
allow(User).to receive(:skip_mx_check?).and_return(false)
|
||||||
|
|
||||||
|
resolver = instance_double(Resolv::DNS, :timeouts= => nil)
|
||||||
|
|
||||||
|
allow(resolver).to receive(:getresources)
|
||||||
|
.with('email.com', Resolv::DNS::Resource::IN::MX)
|
||||||
|
.and_return([instance_double(Resolv::DNS::Resource::MX, exchange: 'smtp.email.com')])
|
||||||
|
allow(resolver).to receive(:getresources).with('email.com', Resolv::DNS::Resource::IN::A).and_return([])
|
||||||
|
allow(resolver).to receive(:getresources).with('email.com', Resolv::DNS::Resource::IN::AAAA).and_return([])
|
||||||
|
allow(resolver).to receive(:getresources).with('smtp.email.com', Resolv::DNS::Resource::IN::A).and_return([instance_double(Resolv::DNS::Resource::IN::A, address: '2.3.4.5')])
|
||||||
|
allow(resolver).to receive(:getresources).with('smtp.email.com', Resolv::DNS::Resource::IN::AAAA).and_return([instance_double(Resolv::DNS::Resource::IN::AAAA, address: 'fd00::2')])
|
||||||
|
allow(Resolv::DNS).to receive(:open).and_yield(resolver)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'creates an unapproved user', :aggregate_failures do
|
||||||
|
access_token = subject.call(app, remote_ip, params)
|
||||||
|
expect(access_token).to_not be_nil
|
||||||
|
expect(access_token.scopes.to_s).to eq 'read write'
|
||||||
|
|
||||||
|
user = User.find_by(id: access_token.resource_owner_id)
|
||||||
|
expect(user).to_not be_nil
|
||||||
|
expect(user.confirmed?).to be false
|
||||||
|
expect(user.approved?).to be false
|
||||||
|
|
||||||
|
expect(user.account).to_not be_nil
|
||||||
|
expect(user.invite_request).to be_nil
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context 'when registrations are closed' do
|
context 'when registrations are closed' do
|
||||||
before do
|
before do
|
||||||
Setting.registrations_mode = 'none'
|
Setting.registrations_mode = 'none'
|
||||||
|
|
Loading…
Reference in New Issue