Adding ability to block domains from registration (#15397)

Prior to this commit, we had filtering options for email registration:

1. Specific allowed domains.
2. Implicitly allow all domains if no allowed domains specified.

With this commit, we add another option: The adminstrator may block one
or more domains from registering.

Closes forem/rfcs#281
This commit is contained in:
Jeremy Friesen 2021-11-16 18:56:01 -05:00 committed by GitHub
parent ad0c5c8ae3
commit 3782db891a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 83 additions and 3 deletions

View file

@ -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

View file

@ -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?"
},

View file

@ -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

View file

@ -95,6 +95,14 @@
<%= admin_config_description Constants::Settings::Authentication::DETAILS[:display_email_domain_allow_list_publicly][:description] %>
</div>
</div>
<div class="crayons-field">
<%= 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] %>
</div>
<div class="crayons-field--checkbox">
<%= f.check_box :require_captcha_for_email_password_registration,
checked: Settings::Authentication.require_captcha_for_email_password_registration,

View file

@ -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"