Add ability to require approval when users sign up using specific email domains (#28468)
This commit is contained in:
		
							parent
							
								
									195b89d336
								
							
						
					
					
						commit
						dfdadb92e8
					
				
					 13 changed files with 84 additions and 16 deletions
				
			
		| 
						 | 
					@ -40,7 +40,7 @@ module Admin
 | 
				
			||||||
          (@email_domain_block.other_domains || []).uniq.each do |domain|
 | 
					          (@email_domain_block.other_domains || []).uniq.each do |domain|
 | 
				
			||||||
            next if EmailDomainBlock.where(domain: domain).exists?
 | 
					            next if EmailDomainBlock.where(domain: domain).exists?
 | 
				
			||||||
 | 
					
 | 
				
			||||||
            other_email_domain_block = EmailDomainBlock.create!(domain: domain, parent: @email_domain_block)
 | 
					            other_email_domain_block = EmailDomainBlock.create!(domain: domain, allow_with_approval: @email_domain_block.allow_with_approval, parent: @email_domain_block)
 | 
				
			||||||
            log_action :create, other_email_domain_block
 | 
					            log_action :create, other_email_domain_block
 | 
				
			||||||
          end
 | 
					          end
 | 
				
			||||||
        end
 | 
					        end
 | 
				
			||||||
| 
						 | 
					@ -65,7 +65,7 @@ module Admin
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def resource_params
 | 
					    def resource_params
 | 
				
			||||||
      params.require(:email_domain_block).permit(:domain, other_domains: [])
 | 
					      params.require(:email_domain_block).permit(:domain, :allow_with_approval, other_domains: [])
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def form_email_domain_block_batch_params
 | 
					    def form_email_domain_block_batch_params
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -55,7 +55,7 @@ class Api::V1::Admin::EmailDomainBlocksController < Api::BaseController
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def resource_params
 | 
					  def resource_params
 | 
				
			||||||
    params.permit(:domain)
 | 
					    params.permit(:domain, :allow_with_approval)
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def insert_pagination_headers
 | 
					  def insert_pagination_headers
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -9,6 +9,7 @@
 | 
				
			||||||
#  created_at          :datetime         not null
 | 
					#  created_at          :datetime         not null
 | 
				
			||||||
#  updated_at          :datetime         not null
 | 
					#  updated_at          :datetime         not null
 | 
				
			||||||
#  parent_id           :bigint(8)
 | 
					#  parent_id           :bigint(8)
 | 
				
			||||||
 | 
					#  allow_with_approval :boolean          default(FALSE), not null
 | 
				
			||||||
#
 | 
					#
 | 
				
			||||||
 | 
					
 | 
				
			||||||
class EmailDomainBlock < ApplicationRecord
 | 
					class EmailDomainBlock < ApplicationRecord
 | 
				
			||||||
| 
						 | 
					@ -42,8 +43,8 @@ class EmailDomainBlock < ApplicationRecord
 | 
				
			||||||
      @attempt_ip = attempt_ip
 | 
					      @attempt_ip = attempt_ip
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def match?
 | 
					    def match?(...)
 | 
				
			||||||
      blocking? || invalid_uri?
 | 
					      blocking?(...) || invalid_uri?
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    private
 | 
					    private
 | 
				
			||||||
| 
						 | 
					@ -52,8 +53,8 @@ class EmailDomainBlock < ApplicationRecord
 | 
				
			||||||
      @uris.any?(&:nil?)
 | 
					      @uris.any?(&:nil?)
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def blocking?
 | 
					    def blocking?(allow_with_approval: false)
 | 
				
			||||||
      blocks = EmailDomainBlock.where(domain: domains_with_variants).order(Arel.sql('char_length(domain) desc'))
 | 
					      blocks = EmailDomainBlock.where(domain: domains_with_variants, allow_with_approval: allow_with_approval).order(Arel.sql('char_length(domain) desc'))
 | 
				
			||||||
      blocks.each { |block| block.history.add(@attempt_ip) } if @attempt_ip.present?
 | 
					      blocks.each { |block| block.history.add(@attempt_ip) } if @attempt_ip.present?
 | 
				
			||||||
      blocks.any?
 | 
					      blocks.any?
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
| 
						 | 
					@ -86,4 +87,8 @@ class EmailDomainBlock < ApplicationRecord
 | 
				
			||||||
  def self.block?(domain_or_domains, attempt_ip: nil)
 | 
					  def self.block?(domain_or_domains, attempt_ip: nil)
 | 
				
			||||||
    Matcher.new(domain_or_domains, attempt_ip: attempt_ip).match?
 | 
					    Matcher.new(domain_or_domains, attempt_ip: attempt_ip).match?
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  def self.requires_approval?(domain_or_domains, attempt_ip: nil)
 | 
				
			||||||
 | 
					    Matcher.new(domain_or_domains, attempt_ip: attempt_ip).match?(allow_with_approval: true)
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
end
 | 
					end
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -418,7 +418,7 @@ class User < ApplicationRecord
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def set_approved
 | 
					  def set_approved
 | 
				
			||||||
    self.approved = begin
 | 
					    self.approved = begin
 | 
				
			||||||
      if sign_up_from_ip_requires_approval?
 | 
					      if sign_up_from_ip_requires_approval? || sign_up_email_requires_approval?
 | 
				
			||||||
        false
 | 
					        false
 | 
				
			||||||
      else
 | 
					      else
 | 
				
			||||||
        open_registrations? || valid_invitation? || external?
 | 
					        open_registrations? || valid_invitation? || external?
 | 
				
			||||||
| 
						 | 
					@ -430,6 +430,12 @@ class User < ApplicationRecord
 | 
				
			||||||
    !sign_up_ip.nil? && IpBlock.where(severity: :sign_up_requires_approval).where('ip >>= ?', sign_up_ip.to_s).exists?
 | 
					    !sign_up_ip.nil? && IpBlock.where(severity: :sign_up_requires_approval).where('ip >>= ?', sign_up_ip.to_s).exists?
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  def sign_up_email_requires_approval?
 | 
				
			||||||
 | 
					    return false unless email.present? || unconfirmed_email.present?
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    EmailDomainBlock.requires_approval?(email.presence || unconfirmed_email, attempt_ip: sign_up_ip)
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def open_registrations?
 | 
					  def open_registrations?
 | 
				
			||||||
    Setting.registrations_mode == 'open'
 | 
					    Setting.registrations_mode == 'open'
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -1,7 +1,7 @@
 | 
				
			||||||
# frozen_string_literal: true
 | 
					# frozen_string_literal: true
 | 
				
			||||||
 | 
					
 | 
				
			||||||
class REST::Admin::EmailDomainBlockSerializer < ActiveModel::Serializer
 | 
					class REST::Admin::EmailDomainBlockSerializer < ActiveModel::Serializer
 | 
				
			||||||
  attributes :id, :domain, :created_at, :history
 | 
					  attributes :id, :domain, :created_at, :history, :allow_with_approval
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def id
 | 
					  def id
 | 
				
			||||||
    object.id.to_s
 | 
					    object.id.to_s
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -12,3 +12,7 @@
 | 
				
			||||||
        ·
 | 
					        ·
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      = t('admin.email_domain_blocks.attempts_over_week', count: email_domain_block.history.reduce(0) { |sum, day| sum + day.accounts })
 | 
					      = t('admin.email_domain_blocks.attempts_over_week', count: email_domain_block.history.reduce(0) { |sum, day| sum + day.accounts })
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      - if email_domain_block.allow_with_approval?
 | 
				
			||||||
 | 
					        ·
 | 
				
			||||||
 | 
					        = t('admin.email_domain_blocks.allow_registrations_with_approval')
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -7,6 +7,9 @@
 | 
				
			||||||
  .fields-group
 | 
					  .fields-group
 | 
				
			||||||
    = f.input :domain, wrapper: :with_block_label, label: t('admin.email_domain_blocks.domain'), input_html: { readonly: defined?(@resolved_records) }
 | 
					    = f.input :domain, wrapper: :with_block_label, label: t('admin.email_domain_blocks.domain'), input_html: { readonly: defined?(@resolved_records) }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  .fields-group
 | 
				
			||||||
 | 
					    = f.input :allow_with_approval, wrapper: :with_label, hint: false, label: I18n.t('admin.email_domain_blocks.allow_registrations_with_approval')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  - if defined?(@resolved_records)
 | 
					  - if defined?(@resolved_records)
 | 
				
			||||||
    %p.hint= t('admin.email_domain_blocks.resolved_dns_records_hint_html')
 | 
					    %p.hint= t('admin.email_domain_blocks.resolved_dns_records_hint_html')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -425,6 +425,7 @@ en:
 | 
				
			||||||
      view: View domain block
 | 
					      view: View domain block
 | 
				
			||||||
    email_domain_blocks:
 | 
					    email_domain_blocks:
 | 
				
			||||||
      add_new: Add new
 | 
					      add_new: Add new
 | 
				
			||||||
 | 
					      allow_registrations_with_approval: Allow registrations with approval
 | 
				
			||||||
      attempts_over_week:
 | 
					      attempts_over_week:
 | 
				
			||||||
        one: "%{count} attempt over the last week"
 | 
					        one: "%{count} attempt over the last week"
 | 
				
			||||||
        other: "%{count} sign-up attempts over the last week"
 | 
					        other: "%{count} sign-up attempts over the last week"
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -0,0 +1,7 @@
 | 
				
			||||||
 | 
					# frozen_string_literal: true
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					class AddAllowWithApprovalToEmailDomainBlocks < ActiveRecord::Migration[7.1]
 | 
				
			||||||
 | 
					  def change
 | 
				
			||||||
 | 
					    add_column :email_domain_blocks, :allow_with_approval, :boolean, default: false, null: false
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					end
 | 
				
			||||||
| 
						 | 
					@ -10,7 +10,7 @@
 | 
				
			||||||
#
 | 
					#
 | 
				
			||||||
# It's strongly recommended that you check this file into your version control system.
 | 
					# It's strongly recommended that you check this file into your version control system.
 | 
				
			||||||
 | 
					
 | 
				
			||||||
ActiveRecord::Schema[7.1].define(version: 2023_12_12_073317) do
 | 
					ActiveRecord::Schema[7.1].define(version: 2023_12_22_100226) do
 | 
				
			||||||
  # These are extensions that must be enabled in order to support this database
 | 
					  # These are extensions that must be enabled in order to support this database
 | 
				
			||||||
  enable_extension "plpgsql"
 | 
					  enable_extension "plpgsql"
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -435,6 +435,7 @@ ActiveRecord::Schema[7.1].define(version: 2023_12_12_073317) do
 | 
				
			||||||
    t.datetime "created_at", precision: nil, null: false
 | 
					    t.datetime "created_at", precision: nil, null: false
 | 
				
			||||||
    t.datetime "updated_at", precision: nil, null: false
 | 
					    t.datetime "updated_at", precision: nil, null: false
 | 
				
			||||||
    t.bigint "parent_id"
 | 
					    t.bigint "parent_id"
 | 
				
			||||||
 | 
					    t.boolean "allow_with_approval", default: false, null: false
 | 
				
			||||||
    t.index ["domain"], name: "index_email_domain_blocks_on_domain", unique: true
 | 
					    t.index ["domain"], name: "index_email_domain_blocks_on_domain", unique: true
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -12,13 +12,14 @@ RSpec.describe Admin::EmailDomainBlocksController do
 | 
				
			||||||
  describe 'GET #index' do
 | 
					  describe 'GET #index' do
 | 
				
			||||||
    around do |example|
 | 
					    around do |example|
 | 
				
			||||||
      default_per_page = EmailDomainBlock.default_per_page
 | 
					      default_per_page = EmailDomainBlock.default_per_page
 | 
				
			||||||
      EmailDomainBlock.paginates_per 1
 | 
					      EmailDomainBlock.paginates_per 2
 | 
				
			||||||
      example.run
 | 
					      example.run
 | 
				
			||||||
      EmailDomainBlock.paginates_per default_per_page
 | 
					      EmailDomainBlock.paginates_per default_per_page
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    it 'returns http success' do
 | 
					    it 'returns http success' do
 | 
				
			||||||
      2.times { Fabricate(:email_domain_block) }
 | 
					      2.times { Fabricate(:email_domain_block) }
 | 
				
			||||||
 | 
					      Fabricate(:email_domain_block, allow_with_approval: true)
 | 
				
			||||||
      get :index, params: { page: 2 }
 | 
					      get :index, params: { page: 2 }
 | 
				
			||||||
      expect(response).to have_http_status(200)
 | 
					      expect(response).to have_http_status(200)
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -135,6 +135,25 @@ RSpec.describe Auth::RegistrationsController do
 | 
				
			||||||
      end
 | 
					      end
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    context 'when user has an email address requiring approval' do
 | 
				
			||||||
 | 
					      subject do
 | 
				
			||||||
 | 
					        Setting.registrations_mode = 'open'
 | 
				
			||||||
 | 
					        Fabricate(:email_domain_block, allow_with_approval: true, domain: 'example.com')
 | 
				
			||||||
 | 
					        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 '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 'with Approval-based registrations without invite' do
 | 
					    context 'with Approval-based registrations without invite' do
 | 
				
			||||||
      subject do
 | 
					      subject do
 | 
				
			||||||
        Setting.registrations_mode = 'approved'
 | 
					        Setting.registrations_mode = 'approved'
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -27,6 +27,27 @@ RSpec.describe AppSignUpService, type: :service do
 | 
				
			||||||
      end
 | 
					      end
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    context 'when the email address requires approval' do
 | 
				
			||||||
 | 
					      before do
 | 
				
			||||||
 | 
					        Setting.registrations_mode = 'open'
 | 
				
			||||||
 | 
					        Fabricate(:email_domain_block, allow_with_approval: true, domain: 'email.com')
 | 
				
			||||||
 | 
					      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 a new issue