FEATURE: add setting `auto_approve_email_domains` to auto approve users (#9323)

* FEATURE: add setting `auto_approve_email_domains` to auto approve users

This commit adds a new site setting `auto_approve_email_domains` to
auto approve users based on their email address domain.

Note that if a domain already exists in `email_domains_whitelist` then
`auto_approve_email_domains` needs to be duplicated there as well,
since users won’t be able to register with email address that is
not allowed in `email_domains_whitelist`.

* Update config/locales/server.en.yml

Co-Authored-By: Robin Ward <robin.ward@gmail.com>
This commit is contained in:
Arpit Jalan 2020-03-31 23:59:15 +05:30 committed by GitHub
parent 0e3fa4072f
commit b2a0d34bb7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 65 additions and 5 deletions

View File

@ -447,8 +447,11 @@ class UsersController < ApplicationController
user ||= User.new user ||= User.new
user.attributes = new_user_params user.attributes = new_user_params
# Handle API approval # Handle API approval and
ReviewableUser.set_approved_fields!(user, current_user) if user.approved? # auto approve users based on auto_approve_email_domains setting
if user.approved? || EmailValidator.can_auto_approve_user?(user.email)
ReviewableUser.set_approved_fields!(user, current_user)
end
# Handle custom fields # Handle custom fields
user_fields = UserField.all user_fields = UserField.all

View File

@ -36,7 +36,9 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f
registration_ip_address: ip_address registration_ip_address: ip_address
} }
if !SiteSetting.must_approve_users? || (SiteSetting.must_approve_users? && invite.invited_by.staff?) if !SiteSetting.must_approve_users? ||
(SiteSetting.must_approve_users? && invite.invited_by.staff?) ||
EmailValidator.can_auto_approve_user?(user.email)
ReviewableUser.set_approved_fields!(user, invite.invited_by) ReviewableUser.set_approved_fields!(user, invite.invited_by)
end end

View File

@ -1582,6 +1582,7 @@ en:
allow_index_in_robots_txt: "Specify in robots.txt that this site is allowed to be indexed by web search engines. In exceptional cases you can permanently <a href='%{base_path}/admin/customize/robots'>override robots.txt</a>." allow_index_in_robots_txt: "Specify in robots.txt that this site is allowed to be indexed by web search engines. In exceptional cases you can permanently <a href='%{base_path}/admin/customize/robots'>override robots.txt</a>."
email_domains_blacklist: "A pipe-delimited list of email domains that users are not allowed to register accounts with. Example: mailinator.com|trashmail.net" email_domains_blacklist: "A pipe-delimited list of email domains that users are not allowed to register accounts with. Example: mailinator.com|trashmail.net"
email_domains_whitelist: "A pipe-delimited list of email domains that users MUST register accounts with. WARNING: Users with email domains other than those listed will not be allowed!" email_domains_whitelist: "A pipe-delimited list of email domains that users MUST register accounts with. WARNING: Users with email domains other than those listed will not be allowed!"
auto_approve_email_domains: "Users with email addresses from this list of domains will be automatically approved."
hide_email_address_taken: "Don't inform users that an account exists with a given email address during signup and from the forgot password form." hide_email_address_taken: "Don't inform users that an account exists with a given email address during signup and from the forgot password form."
log_out_strict: "When logging out, log out ALL sessions for the user on all devices" log_out_strict: "When logging out, log out ALL sessions for the user on all devices"
version_checks: "Ping the Discourse Hub for version updates and show new version messages on the <a href='%{base_path}/admin' target='_blank'>/admin</a> dashboard" version_checks: "Ping the Discourse Hub for version updates and show new version messages on the <a href='%{base_path}/admin' target='_blank'>/admin</a> dashboard"

View File

@ -459,6 +459,9 @@ login:
email_domains_whitelist: email_domains_whitelist:
default: "" default: ""
type: list type: list
auto_approve_email_domains:
default: ""
type: list
hide_email_address_taken: false hide_email_address_taken: false
log_out_strict: true log_out_strict: true
pending_users_reminder_delay: pending_users_reminder_delay:

View File

@ -22,6 +22,14 @@ class EmailValidator < ActiveModel::EachValidator
true true
end end
def self.can_auto_approve_user?(email)
if (setting = SiteSetting.auto_approve_email_domains).present?
return !!(EmailValidator.allowed?(email) && email_in_restriction_setting?(setting, email))
end
false
end
def self.email_in_restriction_setting?(setting, value) def self.email_in_restriction_setting?(setting, value)
domains = setting.gsub('.', '\.') domains = setting.gsub('.', '\.')
regexp = Regexp.new("@(.+\\.)?(#{domains})$", true) regexp = Regexp.new("@(.+\\.)?(#{domains})$", true)

View File

@ -41,6 +41,23 @@ describe EmailValidator do
end end
end end
context "auto approve email domains" do
it "works as expected" do
SiteSetting.auto_approve_email_domains = "example.com"
expect(EmailValidator.can_auto_approve_user?("foobar@example.com.fr")).to eq(false)
expect(EmailValidator.can_auto_approve_user?("foobar@example.com")).to eq(true)
end
it "returns false if domain not present in email_domains_whitelist" do
SiteSetting.email_domains_whitelist = "googlemail.com"
SiteSetting.auto_approve_email_domains = "example.com|googlemail.com"
expect(EmailValidator.can_auto_approve_user?("foobar@example.com")).to eq(false)
expect(EmailValidator.can_auto_approve_user?("foobar@googlemail.com")).to eq(true)
end
end
context '.email_regex' do context '.email_regex' do
it 'should match valid emails' do it 'should match valid emails' do
expect(!!('test@discourse.org' =~ EmailValidator.email_regex)).to eq(true) expect(!!('test@discourse.org' =~ EmailValidator.email_regex)).to eq(true)

View File

@ -59,7 +59,7 @@ describe InviteRedeemer do
end end
describe "#redeem" do describe "#redeem" do
fab!(:invite) { Fabricate(:invite) } fab!(:invite) { Fabricate(:invite, email: "foobar@example.com") }
let(:name) { 'john snow' } let(:name) { 'john snow' }
let(:username) { 'kingofthenorth' } let(:username) { 'kingofthenorth' }
let(:password) { 'know5nOthiNG' } let(:password) { 'know5nOthiNG' }
@ -102,6 +102,16 @@ describe InviteRedeemer do
expect(user.approved).to eq(true) expect(user.approved).to eq(true)
end end
it "should redeem the invite if invited by non staff and approve if email in auto_approve_email_domains setting" do
SiteSetting.must_approve_users = true
SiteSetting.auto_approve_email_domains = "example.com"
user = invite_redeemer.redeem
expect(user.name).to eq(name)
expect(user.username).to eq(username)
expect(user.approved).to eq(true)
end
it "should not blow up if invited_by user has been removed" do it "should not blow up if invited_by user has been removed" do
invite.invited_by.destroy! invite.invited_by.destroy!
invite.reload invite.reload

View File

@ -583,7 +583,7 @@ describe UsersController do
UsersController.any_instance.stubs(:honeypot_value).returns(nil) UsersController.any_instance.stubs(:honeypot_value).returns(nil)
UsersController.any_instance.stubs(:challenge_value).returns(nil) UsersController.any_instance.stubs(:challenge_value).returns(nil)
SiteSetting.allow_new_registrations = true SiteSetting.allow_new_registrations = true
@user = Fabricate.build(:user, password: "strongpassword") @user = Fabricate.build(:user, email: "foobar@example.com", password: "strongpassword")
end end
let(:post_user_params) do let(:post_user_params) do
@ -826,6 +826,22 @@ describe UsersController do
new_user = User.find(json["user_id"]) new_user = User.find(json["user_id"])
expect(new_user.locale).not_to eq("fr") expect(new_user.locale).not_to eq("fr")
end end
it "will auto approve user if the user email domain matches auto_approve_email_domains setting" do
Jobs.run_immediately!
SiteSetting.must_approve_users = true
SiteSetting.auto_approve_email_domains = "example.com"
post "/u.json", params: post_user_params.merge(active: true, api_key: api_key.key)
expect(response.status).to eq(200)
json = JSON.parse(response.body)
new_user = User.find(json["user_id"])
expect(json['active']).to be_truthy
expect(new_user.approved).to be_truthy
expect(ReviewableUser.pending.find_by(target: new_user)).to be_blank
end
end end
end end