diff --git a/app/models/settings/authentication.rb b/app/models/settings/authentication.rb index 8fff8121d..b44d5cc38 100644 --- a/app/models/settings/authentication.rb +++ b/app/models/settings/authentication.rb @@ -11,7 +11,7 @@ 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: { + setting :blocked_registration_email_domains, type: :array, default: %w[], validates: { valid_domain_csv: true } setting :display_email_domain_allow_list_publicly, type: :boolean, default: false diff --git a/app/validators/valid_domain_csv_validator.rb b/app/validators/valid_domain_csv_validator.rb index 671f938d5..43f3144ef 100644 --- a/app/validators/valid_domain_csv_validator.rb +++ b/app/validators/valid_domain_csv_validator.rb @@ -1,6 +1,10 @@ +# @note While the validator implies a CSV, the implementation is that +# we have an array. Upstream implementors likely accept a CSV +# and coerce it into an array. See Authentication::Base for an +# example of coercing the CSV into an array. class ValidDomainCsvValidator < ActiveModel::EachValidator DEFAULT_MESSAGE = "must be a comma-separated list of valid domains".freeze - VALID_DOMAIN = /^[a-zA-Z0-9]{1,61}[a-zA-Z0-9](?:\.[a-zA-Z]{2,})+$/ + VALID_DOMAIN = /^[a-zA-Z0-9][a-zA-Z0-9-]{0,61}[a-zA-Z0-9](?:\.[a-zA-Z]{2,})+$/ def validate_each(record, attribute, value) return unless value diff --git a/spec/requests/admin/configs_spec.rb b/spec/requests/admin/configs_spec.rb index 0f8b8e84b..cef7f8c25 100644 --- a/spec/requests/admin/configs_spec.rb +++ b/spec/requests/admin/configs_spec.rb @@ -113,14 +113,6 @@ RSpec.describe "/admin/customization/config", type: :request do expect(Settings::Authentication.allowed_registration_email_domains).to eq(%w[dev.to forem.com forem.dev]) end - it "allows 2-character domains" do - proper_list = "dev.to, forem.com, 2u.com" - post admin_settings_authentications_path, params: { - settings_authentication: { allowed_registration_email_domains: proper_list } - } - expect(Settings::Authentication.allowed_registration_email_domains).to eq(%w[dev.to forem.com 2u.com]) - end - it "does not allow improper domain list" do impproper_list = "dev.to, foremcom, forem.dev" post admin_settings_authentications_path, params: { diff --git a/spec/validators/valid_domain_csv_validator_spec.rb b/spec/validators/valid_domain_csv_validator_spec.rb new file mode 100644 index 000000000..82c3b0969 --- /dev/null +++ b/spec/validators/valid_domain_csv_validator_spec.rb @@ -0,0 +1,41 @@ +require "rails_helper" + +RSpec.describe ValidDomainCsvValidator do + let(:validatable) do + Class.new do + def self.name + "Validatable" + end + include ActiveModel::Validations + attr_accessor :domains + + validates :domains, valid_domain_csv: true + end + end + let(:model) { validatable.new } + + it "marks valid a domain with dashes in the middle" do + model.domains = ["seo-hunt.com"] + expect(model).to be_valid + end + + it "marks valid a two character domain" do + model.domains = ["2u.com"] + expect(model).to be_valid + end + + it "marks invalid a domain with a dash as a prefix" do + model.domains = ["-seo-hunt.com"] + expect(model).to be_invalid + end + + it "marks valid an array of domains" do + model.domains = ["hello.com", "world.org"] + expect(model).to be_valid + end + + it "marks invalid an array of domains if one is invalid" do + model.domains = ["hello.com", "world.org", "notadomain"] + expect(model).to be_invalid + end +end