From f3a69bbd848fb8e56399fafeb15dfde03559319e Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Tue, 17 Nov 2020 08:43:46 -0500 Subject: [PATCH] Allow Forems to specify which domains are allowed for registration (#11442) * Allow authors to restrict which emails can sign up * Add form and tests * Check for email presence in allowed email flow * Fix test domains * Fix codeclimate issue --- app/controllers/admin/configs_controller.rb | 13 ++++++ app/controllers/registrations_controller.rb | 10 +++++ .../admin/controllers/config_controller.js | 2 +- app/lib/constants/site_config.rb | 7 +++ app/models/site_config.rb | 2 + app/views/admin/configs/show.html.erb | 18 ++++++++ .../_email_registration_form.html.erb | 17 +++++++ spec/requests/admin/configs_spec.rb | 20 +++++++++ spec/requests/registrations_spec.rb | 44 +++++++++++++++++++ 9 files changed, 132 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/configs_controller.rb b/app/controllers/admin/configs_controller.rb index e9d5e638f..51b816fd2 100644 --- a/app/controllers/admin/configs_controller.rb +++ b/app/controllers/admin/configs_controller.rb @@ -123,6 +123,8 @@ module Admin video_encoder_key tag_feed_minimum_score home_feed_minimum_score + allowed_registration_email_domains + display_email_domain_allow_list_publicly ].freeze IMAGE_FIELDS = @@ -211,6 +213,7 @@ module Admin errors = [] errors << "Brand color must be darker for accessibility." if brand_contrast_too_low errors << "Brand color must be be a 6 character hex (starting with #)." if brand_color_not_hex + errors << "Allowed emails must be list of domains." if allowed_domains_include_improper_format redirect_to admin_config_path, alert: "😭 #{errors.to_sentence}" if errors.any? end @@ -260,6 +263,16 @@ module Admin hex.present? && !hex.match?(/\A#(\h{6}|\h{3})\z/) end + def allowed_domains_include_improper_format + domains = params.dig(:site_config, :allowed_registration_email_domains) + return unless domains + + domains_array = domains.delete(" ").split(",") + valid_domains = domains_array + .select { |d| d.match?(/^[a-zA-Z0-9][a-zA-Z0-9-]{1,61}[a-zA-Z0-9](?:\.[a-zA-Z]{2,})+$/) } + valid_domains.size != domains_array.size + end + def valid_image_url(url) url.match?(VALID_URL) end diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index bf5613fb1..847c68cb6 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -23,6 +23,7 @@ class RegistrationsController < Devise::RegistrationsController resource.registered = true resource.registered_at = Time.current resource.editor_version = "v2" + check_allowed_email(resource) if resource.email.present? resource.save if resource.email.present? yield resource if block_given? if resource.persisted? @@ -57,4 +58,13 @@ class RegistrationsController < Devise::RegistrationsController recaptcha_params = { secret_key: SiteConfig.recaptcha_secret_key } params["g-recaptcha-response"] && verify_recaptcha(recaptcha_params) end + + def check_allowed_email(resource) + domain = resource.email.split("@").last + allow_list = SiteConfig.allowed_registration_email_domains + return if allow_list.empty? || allow_list.include?(domain) + + resource.email = nil + resource.errors.add(:email, "is not included in allowed domains.") + end end diff --git a/app/javascript/admin/controllers/config_controller.js b/app/javascript/admin/controllers/config_controller.js index 2344ba81d..88d68dc8f 100644 --- a/app/javascript/admin/controllers/config_controller.js +++ b/app/javascript/admin/controllers/config_controller.js @@ -12,7 +12,7 @@ const emailAuthModalTitle = 'Disable Email address registration'; // TODO: Remove the sentence "You must update site config to save this action!" // once we build more robust flow for Admin/Config const emailAuthModalBody = - '

If you disable Email address as a registration option, people cannot create an account with their email address.

However, people who have already created an account using their email address can continue to login.

Please update site config to save this action.

'; + '

If you disable Email address as a registration option, people cannot create an account with their email address.

However, people who have already created an account using their email address can continue to login.

You must confirm and update site config to save below this action.

'; export default class ConfigController extends Controller { static targets = [ diff --git a/app/lib/constants/site_config.rb b/app/lib/constants/site_config.rb index d86bfadae..1f3122bab 100644 --- a/app/lib/constants/site_config.rb +++ b/app/lib/constants/site_config.rb @@ -9,6 +9,10 @@ module Constants they're creating a new account in your community", placeholder: "" }, + allowed_registration_email_domains: { + description: "Restrict registration to only certain emails? (comma-separated list)", + placeholder: "dev.to, forem.com, codenewbie.org" + }, authentication_providers: { description: "How can users sign in?", placeholder: "" @@ -82,6 +86,9 @@ module Constants default_font: { description: "Determines the default Base Reading Font (registered users can change this in their UX settings)" }, + display_email_domain_allow_list_publicly: { + description: "Do you want to display the list of allowed domains, or keep it private?" + }, display_jobs_banner: { description: "Display a jobs banner that points users to the jobs page when they type 'job'" \ "or 'jobs' in the search box", diff --git a/app/models/site_config.rb b/app/models/site_config.rb index 62f4625d8..27efea783 100644 --- a/app/models/site_config.rb +++ b/app/models/site_config.rb @@ -27,6 +27,8 @@ class SiteConfig < RailsSettings::Base # Authentication field :allow_email_password_registration, type: :boolean, default: false field :allow_email_password_login, type: :boolean, default: true + field :allowed_registration_email_domains, type: :array, default: %w[] + field :display_email_domain_allow_list_publicly, type: :boolean, default: false field :require_captcha_for_email_password_registration, type: :boolean, default: false field :authentication_providers, type: :array, default: proc { Authentication::Providers.available } field :invite_only_mode, type: :boolean, default: false diff --git a/app/views/admin/configs/show.html.erb b/app/views/admin/configs/show.html.erb index 5c98ffc6c..269f8ab65 100644 --- a/app/views/admin/configs/show.html.erb +++ b/app/views/admin/configs/show.html.erb @@ -196,6 +196,24 @@ id: "email-registration-checkbox", class: "crayons-checkbox" %> +
+ <%= admin_config_label :allowed_registration_email_domains, "Allowed email domains" %> + <%= admin_config_description Constants::SiteConfig::DETAILS[:allowed_registration_email_domains][:description] %> + <%= f.text_field :allowed_registration_email_domains, + class: "crayons-textfield", + value: SiteConfig.allowed_registration_email_domains.join(","), + placeholder: Constants::SiteConfig::DETAILS[:allowed_registration_email_domains][:placeholder] %> +
+
+ <%= f.check_box :display_email_domain_allow_list_publicly, + checked: SiteConfig.display_email_domain_allow_list_publicly, + id: "email-display-emails-publicly-checkbox", + class: "crayons-checkbox mt-2" %> +
+ <%= admin_config_label :display_email_domain_allow_list_publicly %> + <%= admin_config_description Constants::SiteConfig::DETAILS[:display_email_domain_allow_list_publicly][:description] %> +
+
<%= f.check_box :require_captcha_for_email_password_registration, checked: SiteConfig.require_captcha_for_email_password_registration, diff --git a/app/views/shared/authentication/_email_registration_form.html.erb b/app/views/shared/authentication/_email_registration_form.html.erb index 94bfcd35d..a839598ce 100644 --- a/app/views/shared/authentication/_email_registration_form.html.erb +++ b/app/views/shared/authentication/_email_registration_form.html.erb @@ -29,6 +29,23 @@ <% else %>

Create your account

<% end %> + <% if SiteConfig.display_email_domain_allow_list_publicly && + SiteConfig.allowed_registration_email_domains.any? %> +
+ <% if SiteConfig.allowed_registration_email_domains.one? %> + Registration restricted to @<%= SiteConfig.allowed_registration_email_domains.first %> emails. + <% else %> + Registration restricted to the following emails +
    + <% SiteConfig.allowed_registration_email_domains.each do |domain| %> +
  • + @<%= domain %> +
    • + <% end %> +
+ <% end %> +
+ <% end %>
<%= f.label :profile_image, class: "crayons-field__label" %> <%= f.file_field :profile_image, accept: "image/*", class: "crayons-card crayons-card--secondary p-3 flex items-center flex-1 w-100", required: true %> diff --git a/spec/requests/admin/configs_spec.rb b/spec/requests/admin/configs_spec.rb index a207f3106..71e9b98ef 100644 --- a/spec/requests/admin/configs_spec.rb +++ b/spec/requests/admin/configs_spec.rb @@ -88,6 +88,26 @@ RSpec.describe "/admin/config", type: :request do expect(SiteConfig.authentication_providers).to eq([provider]) end + it "enables proper domains to allow list" do + proper_list = "dev.to, forem.com, forem.dev" + post "/admin/config", params: { site_config: { allowed_registration_email_domains: proper_list }, + confirmation: confirmation_message } + expect(SiteConfig.allowed_registration_email_domains).to eq(%w[dev.to forem.com forem.dev]) + end + + it "does not allow improper domain list" do + impproper_list = "dev.to, foremcom, forem.dev" + post "/admin/config", params: { site_config: { allowed_registration_email_domains: impproper_list }, + confirmation: confirmation_message } + expect(SiteConfig.allowed_registration_email_domains).not_to eq(%w[dev.to foremcom forem.dev]) + end + + it "enables display_email_domain_allow_list_publicly" do + post "/admin/config", params: { site_config: { display_email_domain_allow_list_publicly: true }, + confirmation: confirmation_message } + expect(SiteConfig.display_email_domain_allow_list_publicly).to be(true) + end + it "enables email authentication" do post "/admin/config", params: { site_config: { allow_email_password_registration: true }, confirmation: confirmation_message } diff --git a/spec/requests/registrations_spec.rb b/spec/requests/registrations_spec.rb index feace8db0..edd2c3812 100644 --- a/spec/requests/registrations_spec.rb +++ b/spec/requests/registrations_spec.rb @@ -222,6 +222,50 @@ RSpec.describe "Registrations", type: :request do end end + context "when email registration allowed and email allow list empty" do + before do + allow(SiteConfig).to receive(:allow_email_password_registration).and_return(true) + allow(SiteConfig).to receive(:allowed_registration_email_domains).and_return([]) + end + + it "creates user when email in allow list" do + post "/users", params: + { user: { name: "royal #{rand(10)}", + username: "magoo_#{rand(10)}", + email: "queenelizabeth@dev.to", + password: "PaSSw0rd_yo000", + password_confirmation: "PaSSw0rd_yo000" } } + expect(User.all.size).to be 1 + end + end + + context "when email registration allowed and email allow list present" do + before do + allow(SiteConfig).to receive(:allow_email_password_registration).and_return(true) + allow(SiteConfig).to receive(:allowed_registration_email_domains).and_return(["dev.to", "forem.com"]) + end + + it "does not create user when email not in allow list" do + post "/users", params: + { user: { name: "ronald #{rand(10)}", + username: "mcdonald_#{rand(10)}", + email: "ronald@mcdonald.com", + password: "PaSSw0rd_yo000", + password_confirmation: "PaSSw0rd_yo000" } } + expect(User.all.size).to be 0 + end + + it "creates user when email in allow list" do + post "/users", params: + { user: { name: "royal #{rand(10)}", + username: "magoo_#{rand(10)}", + email: "queenelizabeth@dev.to", + password: "PaSSw0rd_yo000", + password_confirmation: "PaSSw0rd_yo000" } } + expect(User.all.size).to be 1 + end + end + context "when site configured to accept email registration AND require captcha" do before do allow(SiteConfig).to receive(:allow_email_password_registration).and_return(true)