diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index f3d51b938..26942f5d1 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -64,10 +64,12 @@ class RegistrationsController < Devise::RegistrationsController def check_allowed_email(resource) domain = resource.email.split("@").last - allow_list = Settings::Authentication.allowed_registration_email_domains - return if allow_list.empty? || allow_list.include?(domain) + return true if Settings::Authentication.acceptable_domain?(domain: domain) resource.email = nil + # Alright, this error message isn't quite correct. Is the email + # from a blocked domain? Or an explicitly allowed domain. I + # think this is enough. resource.errors.add(:email, "is not included in allowed domains.") end diff --git a/app/lib/constants/settings/authentication.rb b/app/lib/constants/settings/authentication.rb index fca778e43..1d14231b8 100644 --- a/app/lib/constants/settings/authentication.rb +++ b/app/lib/constants/settings/authentication.rb @@ -26,6 +26,10 @@ module Constants "The \"PEM\" key from the Authentication Service configured in the Apple Developer Portal", placeholder: "-----BEGIN PRIVATE KEY-----\nMIGTAQrux...QPe8Yb\n-----END PRIVATE KEY-----\\n" }, + blocked_registration_email_domains: { + description: "Block registration from specified domains? (comma-separated list)", + placeholder: "seo-hunt.com" + }, display_email_domain_allow_list_publicly: { description: "Do you want to display the list of allowed domains, or keep it private?" }, diff --git a/app/models/settings/authentication.rb b/app/models/settings/authentication.rb index e98f0afd3..8fff8121d 100644 --- a/app/models/settings/authentication.rb +++ b/app/models/settings/authentication.rb @@ -11,6 +11,9 @@ module Settings setting :apple_key_id, type: :string setting :apple_pem, type: :string setting :apple_team_id, type: :string + setting :blocked_registration_email_domains, type: :array, default: %(), validates: { + valid_domain_csv: true + } setting :display_email_domain_allow_list_publicly, type: :boolean, default: false setting :facebook_key, type: :string setting :facebook_secret, type: :string @@ -38,5 +41,16 @@ module Settings "present" end singleton_class.alias_method(:apple_secret, :apple_key) + + # @param domain [String] The domain to check for acceptability + # + # @return [Boolean] do we allow this domain? + def self.acceptable_domain?(domain:) + return false if blocked_registration_email_domains.include?(domain) + return true if allowed_registration_email_domains.empty? + return true if allowed_registration_email_domains.include?(domain) + + false + end end end diff --git a/app/views/admin/settings/forms/_authentication.html.erb b/app/views/admin/settings/forms/_authentication.html.erb index 2dc85975f..66dcd0f08 100644 --- a/app/views/admin/settings/forms/_authentication.html.erb +++ b/app/views/admin/settings/forms/_authentication.html.erb @@ -95,6 +95,14 @@ <%= admin_config_description Constants::Settings::Authentication::DETAILS[:display_email_domain_allow_list_publicly][:description] %> +
+ <%= admin_config_label :blocked_registration_email_domains, "Block email domains", model: Settings::Authentication %> + <%= admin_config_description Constants::Settings::Authentication::DETAILS[:blocked_registration_email_domains][:description] %> + <%= f.text_field :blocked_registration_email_domains, + class: "crayons-textfield", + value: Settings::Authentication.blocked_registration_email_domains.join(","), + placeholder: Constants::Settings::Authentication::DETAILS[:blocked_registration_email_domains][:placeholder] %> +
<%= f.check_box :require_captcha_for_email_password_registration, checked: Settings::Authentication.require_captcha_for_email_password_registration, diff --git a/spec/models/settings/authentication_spec.rb b/spec/models/settings/authentication_spec.rb index ca7b686f8..af062c21f 100644 --- a/spec/models/settings/authentication_spec.rb +++ b/spec/models/settings/authentication_spec.rb @@ -1,8 +1,60 @@ require "rails_helper" RSpec.describe Settings::Authentication, type: :model do + describe "#acceptable_domain?" do + subject { described_class.acceptable_domain?(domain: domain) } + + let(:domain) { "hello.com" } + + context "with blocked domain" do + before do + allow(described_class).to receive(:blocked_registration_email_domains).and_return([domain]) + end + + it { is_expected.to be_falsey } + end + + context "with allowed domain" do + before do + allow(described_class).to receive(:allowed_registration_email_domains).and_return([domain]) + end + + it { is_expected.to be_truthy } + end + + context "with no domains blocked nor explicitly allowed" do + before do + allow(described_class).to receive(:allowed_registration_email_domains).and_return([]) + end + + it { is_expected.to be_truthy } + end + + context "with no domains blocked but an explicitly allowed domain" do + before do + allow(described_class).to receive(:allowed_registration_email_domains).and_return(["wonka.vision"]) + end + + it { is_expected.to be_falsey } + end + end + describe "validations" do - describe "validating domain lists" do + describe "#blocked_registration_email_domains" do + it "allows valid domain lists" do + expect do + described_class.blocked_registration_email_domains = "example.com, example2.com" + end.not_to raise_error + end + + it "rejects invalid domain lists" do + expect do + described_class.blocked_registration_email_domains = "example.com, e.c" + end.to raise_error(/must be a comma-separated list of valid domains/) + end + end + + describe "#allowed_registration_email_domains" do it "allows valid domain lists" do expect do described_class.allowed_registration_email_domains = "example.com, example2.com"