From 21a73c52a7d0d0149e1058aeec155fe1c87aaeff Mon Sep 17 00:00:00 2001 From: ThibG Date: Thu, 2 May 2019 04:30:12 +0200 Subject: Check that an invite link is valid before bypassing approval mode (#10657) * Check that an invite link is valid before bypassing approval mode Fixes #10656 * Add tests * Only consider valid invite links in registration controller * fixup --- .../auth/registrations_controller_spec.rb | 83 ++++++++++++++++++++++ 1 file changed, 83 insertions(+) (limited to 'spec') diff --git a/spec/controllers/auth/registrations_controller_spec.rb b/spec/controllers/auth/registrations_controller_spec.rb index 1095df034..a4337039e 100644 --- a/spec/controllers/auth/registrations_controller_spec.rb +++ b/spec/controllers/auth/registrations_controller_spec.rb @@ -107,6 +107,89 @@ RSpec.describe Auth::RegistrationsController, type: :controller do end end + context 'approval-based registrations without invite' do + around do |example| + registrations_mode = Setting.registrations_mode + example.run + Setting.registrations_mode = registrations_mode + end + + 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' } } + end + + it 'redirects to login page' do + subject + expect(response).to redirect_to new_user_session_path + end + + it 'creates user' do + subject + 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 eq(false) + end + end + + context 'approval-based registrations with expired invite' do + around do |example| + registrations_mode = Setting.registrations_mode + example.run + Setting.registrations_mode = registrations_mode + end + + 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 } } + end + + it 'redirects to login page' do + subject + expect(response).to redirect_to new_user_session_path + end + + it 'creates user' do + subject + 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 eq(false) + end + end + + context 'approval-based registrations with valid invite' do + around do |example| + registrations_mode = Setting.registrations_mode + example.run + Setting.registrations_mode = registrations_mode + end + + subject do + Setting.registrations_mode = 'approved' + request.headers["Accept-Language"] = accept_language + invite = Fabricate(:invite, 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 } } + end + + it 'redirects to login page' do + subject + expect(response).to redirect_to new_user_session_path + end + + it 'creates user' do + subject + 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 eq(true) + end + end + it 'does nothing if user already exists' do Fabricate(:user, account: Fabricate(:account, username: 'test')) subject -- cgit From 011b032300657ccca4a42866749afc6ec2588ecc Mon Sep 17 00:00:00 2001 From: ThibG Date: Fri, 3 May 2019 20:36:36 +0200 Subject: Provide a link to existing domain block when trying to block an already-blocked domain (#10663) * When trying to block an already-blocked domain, provide a link to the block * Fix styling for links in flash messages * Allow blocks to be upgraded but not downgraded --- app/controllers/admin/domain_blocks_controller.rb | 22 +++++++++++---- app/javascript/styles/mastodon/forms.scss | 11 ++++++++ app/models/domain_block.rb | 7 +++++ config/locales/en.yml | 1 + .../admin/domain_blocks_controller_spec.rb | 13 ++++++++- spec/models/domain_block_spec.rb | 31 ++++++++++++++++++++++ 6 files changed, 79 insertions(+), 6 deletions(-) (limited to 'spec') diff --git a/app/controllers/admin/domain_blocks_controller.rb b/app/controllers/admin/domain_blocks_controller.rb index 5f307ddee..dd3f83389 100644 --- a/app/controllers/admin/domain_blocks_controller.rb +++ b/app/controllers/admin/domain_blocks_controller.rb @@ -13,13 +13,25 @@ module Admin authorize :domain_block, :create? @domain_block = DomainBlock.new(resource_params) + existing_domain_block = resource_params[:domain].present? ? DomainBlock.find_by(domain: resource_params[:domain]) : nil - if @domain_block.save - DomainBlockWorker.perform_async(@domain_block.id) - log_action :create, @domain_block - redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.created_msg') - else + if existing_domain_block.present? && !@domain_block.stricter_than?(existing_domain_block) + @domain_block.save + flash[:alert] = I18n.t('admin.domain_blocks.existing_domain_block_html', name: existing_domain_block.domain, unblock_url: admin_domain_block_path(existing_domain_block)).html_safe # rubocop:disable Rails/OutputSafety + @domain_block.errors[:domain].clear render :new + else + if existing_domain_block.present? + @domain_block = existing_domain_block + @domain_block.update(resource_params) + end + if @domain_block.save + DomainBlockWorker.perform_async(@domain_block.id) + log_action :create, @domain_block + redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.created_msg') + else + render :new + end end end diff --git a/app/javascript/styles/mastodon/forms.scss b/app/javascript/styles/mastodon/forms.scss index 91888d305..2b8d7a682 100644 --- a/app/javascript/styles/mastodon/forms.scss +++ b/app/javascript/styles/mastodon/forms.scss @@ -533,6 +533,17 @@ code { color: $error-value-color; } + a { + display: inline-block; + color: $darker-text-color; + text-decoration: none; + + &:hover { + color: $primary-text-color; + text-decoration: underline; + } + } + p { margin-bottom: 15px; } diff --git a/app/models/domain_block.rb b/app/models/domain_block.rb index 069cda367..0b12617c6 100644 --- a/app/models/domain_block.rb +++ b/app/models/domain_block.rb @@ -29,4 +29,11 @@ class DomainBlock < ApplicationRecord def self.blocked?(domain) where(domain: domain, severity: :suspend).exists? end + + def stricter_than?(other_block) + return true if suspend? + return false if other_block.suspend? && (silence? || noop?) + return false if other_block.silence? && noop? + (reject_media || !other_block.reject_media) && (reject_reports || !other_block.reject_reports) + end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 405b0eda5..6d59411a5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -269,6 +269,7 @@ en: created_msg: Domain block is now being processed destroyed_msg: Domain block has been undone domain: Domain + existing_domain_block_html: You have already imposed stricter limits on %{name}, you need to unblock it first. new: create: Create block hint: The domain block will not prevent creation of account entries in the database, but will retroactively and automatically apply specific moderation methods on those accounts. diff --git a/spec/controllers/admin/domain_blocks_controller_spec.rb b/spec/controllers/admin/domain_blocks_controller_spec.rb index 129bf8883..2a8675c21 100644 --- a/spec/controllers/admin/domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/domain_blocks_controller_spec.rb @@ -37,7 +37,7 @@ RSpec.describe Admin::DomainBlocksController, type: :controller do end it 'renders new when failed to save' do - Fabricate(:domain_block, domain: 'example.com') + Fabricate(:domain_block, domain: 'example.com', severity: 'suspend') allow(DomainBlockWorker).to receive(:perform_async).and_return(true) post :create, params: { domain_block: { domain: 'example.com', severity: 'silence' } } @@ -45,6 +45,17 @@ RSpec.describe Admin::DomainBlocksController, type: :controller do expect(DomainBlockWorker).not_to have_received(:perform_async) expect(response).to render_template :new end + + it 'allows upgrading a block' do + Fabricate(:domain_block, domain: 'example.com', severity: 'silence') + allow(DomainBlockWorker).to receive(:perform_async).and_return(true) + + post :create, params: { domain_block: { domain: 'example.com', severity: 'silence', reject_media: true, reject_reports: true } } + + expect(DomainBlockWorker).to have_received(:perform_async) + expect(flash[:notice]).to eq I18n.t('admin.domain_blocks.created_msg') + expect(response).to redirect_to(admin_instances_path(limited: '1')) + end end describe 'DELETE #destroy' do diff --git a/spec/models/domain_block_spec.rb b/spec/models/domain_block_spec.rb index 89cadccfe..0035fd0ff 100644 --- a/spec/models/domain_block_spec.rb +++ b/spec/models/domain_block_spec.rb @@ -36,4 +36,35 @@ RSpec.describe DomainBlock, type: :model do expect(DomainBlock.blocked?('domain')).to eq false end end + + describe 'stricter_than?' do + it 'returns true if the new block has suspend severity while the old has lower severity' do + suspend = DomainBlock.new(domain: 'domain', severity: :suspend) + silence = DomainBlock.new(domain: 'domain', severity: :silence) + noop = DomainBlock.new(domain: 'domain', severity: :noop) + expect(suspend.stricter_than?(silence)).to be true + expect(suspend.stricter_than?(noop)).to be true + end + + it 'returns false if the new block has lower severity than the old one' do + suspend = DomainBlock.new(domain: 'domain', severity: :suspend) + silence = DomainBlock.new(domain: 'domain', severity: :silence) + noop = DomainBlock.new(domain: 'domain', severity: :noop) + expect(silence.stricter_than?(suspend)).to be false + expect(noop.stricter_than?(suspend)).to be false + expect(noop.stricter_than?(silence)).to be false + end + + it 'returns false if the new block does is less strict regarding reports' do + older = DomainBlock.new(domain: 'domain', severity: :silence, reject_reports: true) + newer = DomainBlock.new(domain: 'domain', severity: :silence, reject_reports: false) + expect(newer.stricter_than?(older)).to be false + end + + it 'returns false if the new block does is less strict regarding media' do + older = DomainBlock.new(domain: 'domain', severity: :silence, reject_media: true) + newer = DomainBlock.new(domain: 'domain', severity: :silence, reject_media: false) + expect(newer.stricter_than?(older)).to be false + end + end end -- cgit From 7cb369d4c66c4381c856a2714b4117d6204cd4bb Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 3 May 2019 23:44:44 +0200 Subject: Change e-mail whitelist/blacklist to not be checked when invited (#10683) * Change e-mail whitelist/blacklist to not be checked when invited And only when creating an account, not when updating it later Fix #10648 * Fix test --- app/models/user.rb | 2 +- app/validators/blacklisted_email_validator.rb | 5 ++++- spec/validators/blacklisted_email_validator_spec.rb | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/app/models/user.rb b/app/models/user.rb index 432078651..bce28aa5f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -78,7 +78,7 @@ class User < ApplicationRecord accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? } validates :locale, inclusion: I18n.available_locales.map(&:to_s), if: :locale? - validates_with BlacklistedEmailValidator, if: :email_changed? + validates_with BlacklistedEmailValidator, on: :create validates_with EmailMxValidator, if: :validate_email_dns? validates :agreement, acceptance: { allow_nil: false, accept: [true, 'true', '1'] }, on: :create diff --git a/app/validators/blacklisted_email_validator.rb b/app/validators/blacklisted_email_validator.rb index a2061fdd3..a288c20ef 100644 --- a/app/validators/blacklisted_email_validator.rb +++ b/app/validators/blacklisted_email_validator.rb @@ -2,7 +2,10 @@ class BlacklistedEmailValidator < ActiveModel::Validator def validate(user) + return if user.invited? + @email = user.email + user.errors.add(:email, I18n.t('users.invalid_email')) if blocked_email? end @@ -13,7 +16,7 @@ class BlacklistedEmailValidator < ActiveModel::Validator end def on_blacklist? - return true if EmailDomainBlock.block?(@email) + return true if EmailDomainBlock.block?(@email) return false if Rails.configuration.x.email_domains_blacklist.blank? domains = Rails.configuration.x.email_domains_blacklist.gsub('.', '\.') diff --git a/spec/validators/blacklisted_email_validator_spec.rb b/spec/validators/blacklisted_email_validator_spec.rb index d2e442f4a..84b0107dd 100644 --- a/spec/validators/blacklisted_email_validator_spec.rb +++ b/spec/validators/blacklisted_email_validator_spec.rb @@ -8,6 +8,7 @@ RSpec.describe BlacklistedEmailValidator, type: :validator do let(:errors) { double(add: nil) } before do + allow(user).to receive(:invited?) { false } allow_any_instance_of(described_class).to receive(:blocked_email?) { blocked_email } described_class.new.validate(user) end -- cgit