maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Optionally limit new users based on admin setting (#20149)

Browse source 
* create admin setting for new user status

* limiting on signup working 🎉

* update registration specs

* sort out OAuth registrations
This commit is contained in:
PJ 2023-09-26 10:46:36 +01:00 • committed by GitHub
parent 25d1cdd2e0
commit 47deeefe57
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 265 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
app/controllers/registrations_controller.rb
View file

@ -22,17 +22,17 @@ class RegistrationsController < Devise::RegistrationsController
build_devise_resource
if resource.persisted?
update_first_user_permissions(resource)
resource.set_initial_roles!
if ForemInstance.smtp_enabled?
if resource.creator?
prepare_new_forem_instance
sign_in(resource)
redirect_to new_admin_creator_setting_path
elsif ForemInstance.smtp_enabled?
redirect_to confirm_email_path(email: resource.email)
else
sign_in(resource)
if resource.roles.includes(:creator).any?
redirect_to new_admin_creator_setting_path
else
redirect_to root_path
end
redirect_to root_path
end
else
render action: "by_email"
@ -41,13 +41,7 @@ class RegistrationsController < Devise::RegistrationsController
private
def update_first_user_permissions(resource)
return unless Settings::General.waiting_on_first_user
resource.add_role(:creator)
resource.add_role(:super_admin)
resource.add_role(:trusted)
resource.skip_confirmation!
def prepare_new_forem_instance
Settings::General.waiting_on_first_user = false
Users::CreateMascotAccount.call
Discover::RegisterWorker.perform_async # Register Forem instance on https://discover.forem.com

6
app/helpers/admin/settings_helper.rb
View file

@ -7,5 +7,11 @@ module Admin
def billboard_all_countries_for_editing
ISO3166::Country.all.to_h { |country| [country.alpha2, country.common_name] }
end
def new_user_status_options
::Settings::Authentication::NEW_USER_STATUSES.map do |status|
[status.humanize, status]
end
end
end
end

4
app/lib/constants/settings/authentication.rb
View file

@ -66,6 +66,10 @@ module Constants
description: I18n.t("lib.constants.settings.authentication.invite_only.description"),
placeholder: ""
},
new_user_status: {
description: I18n.t("lib.constants.settings.authentication.new_user_status.description"),
placeholder: I18n.t("lib.constants.settings.authentication.new_user_status.placeholder")
},
recaptcha_site_key: {
description: I18n.t("lib.constants.settings.authentication.recaptcha_site.description"),
placeholder: I18n.t("lib.constants.settings.authentication.recaptcha_site.placeholder")

9
app/models/settings/authentication.rb
View file

@ -2,6 +2,8 @@ module Settings
class Authentication < Base
self.table_name = :settings_authentications
NEW_USER_STATUSES = %w[good_standing limited].freeze
setting :allow_email_password_login, type: :boolean, default: true
setting :allow_email_password_registration, type: :boolean, default: false
setting :allowed_registration_email_domains, type: :array, default: %w[], validates: {
@ -24,6 +26,9 @@ module Settings
setting :google_oauth2_key, type: :string
setting :google_oauth2_secret, type: :string
setting :invite_only_mode, type: :boolean, default: false
setting :new_user_status, type: :string, default: "good_standing", validates: {
inclusion: { in: NEW_USER_STATUSES }
}
setting :providers, type: :array, default: %w[]
setting :require_captcha_for_email_password_registration, type: :boolean, default: false
setting :twitter_key, type: :string, default: ApplicationConfig["TWITTER_KEY"]
@ -57,5 +62,9 @@ module Settings
false
end
def self.limit_new_users?
new_user_status == "limited"
end
end
end

15
app/models/user.rb
View file

@ -277,6 +277,21 @@ class User < ApplicationRecord
self.remember_created_at ||= Time.now.utc
end
def set_initial_roles!
# Avoid overwriting roles for users who already exist but are e.g. logging in
# through a new identity provider
return unless valid? && previously_new_record?
if Settings::General.waiting_on_first_user
add_role(:creator)
add_role(:super_admin)
add_role(:trusted)
elsif Settings::Authentication.limit_new_users?
add_role(:limited)
# Otherwise just leave the new user in good standing
end
end
def calculate_score
# User score is used to mitigate spam by reducing visibility of flagged users
# It can generally be used as a baseline for affecting certain functionality which

1
app/services/authentication/authenticator.rb
View file

@ -51,6 +51,7 @@ module Authentication
else
update_user(user)
end
user.set_initial_roles!
identity.user = user if identity.user_id.blank?
new_identity = identity.new_record?

9
app/views/admin/settings/forms/_authentication.html.erb
View file

@ -207,6 +207,15 @@
<hr class="my-0" />
<% end %>
</section>
<section class="crayons-field mb-6">
<%= admin_config_label :new_user_status, model: Settings::Authentication %>
<%= admin_config_description Constants::Settings::Authentication.details[:new_user_status][:description] %>
<%= select_tag "settings_authentication[new_user_status]",
options_for_select(new_user_status_options, Settings::Authentication.new_user_status),
multiple: false,
class: "crayons-select" %>
</section>
</section>
<%= render "update_setting_button", f: f %>

2
app/views/admin/settings/forms/_user_experience.html.erb
View file

@ -5,7 +5,7 @@
<summary class="crayons-subtitle-2 p-6">User Experience and Brand</summary>
<div class="p-6 pt-0">
<fieldset class="grid gap-4">
<div class="crayons-fieldx">
<div class="crayons-field">
<%= admin_config_label :feed_style, model: Settings::UserExperience %>
<%= admin_config_description Constants::Settings::UserExperience.details[:feed_style][:description] %>
<%= select_tag "settings_user_experience[feed_style]",

3
config/locales/lib/en.yml
View file

@ -37,6 +37,9 @@ en:
description: The "Client Secret" portion of the OAuth 2.0 page on the Google Cloud Platform portal
invite_only:
description: Only users invited by email can join this community.
new_user_status:
description: What status do you want to apply to new users?
placeholder: Good standing
providers:
description: How can users sign in?
recaptcha_secret:

3
config/locales/lib/fr.yml
View file

@ -37,6 +37,9 @@ fr:
description: The "Client Secret" portion of the OAuth 2.0 page on the Google Cloud Platform portal
invite_only:
description: Only users invited by email can join this community.
new_user_status:
description: What status do you want to apply to new users?
placeholder: Good standing
providers:
description: How can users sign in?
recaptcha_secret:

63
spec/models/user_spec.rb
View file

@ -663,6 +663,69 @@ RSpec.describe User do
end
end
describe "#set_initial_roles!" do
it "adds creator roles if waiting on first user" do
allow(Settings::General).to receive(:waiting_on_first_user).and_return(true)
user = create(:user)
user.set_initial_roles!
expect(user).to be_creator
expect(user).to be_super_admin
expect(user).to be_trusted
expect(user).not_to be_limited
end
it "does not add any roles if not waiting on first user" do
user = create(:user)
user.set_initial_roles!
expect(user).not_to be_creator
expect(user).not_to be_super_admin
expect(user).not_to be_trusted
expect(user).not_to be_limited
end
it "adds the limited role to a new user if the new user status setting is limited" do
allow(Settings::Authentication).to receive(:new_user_status).and_return("limited")
user = create(:user)
user.set_initial_roles!
expect(user).not_to be_creator
expect(user).not_to be_super_admin
expect(user).not_to be_trusted
expect(user).to be_limited
end
it "does not change any roles if the user is not a new user" do
create(:user, :trusted, email: "trusted-user@forem.test")
# Now considered an already existing record to ActiveRecord
user = described_class.find_by(email: "trusted-user@forem.test")
expect(UserRole.count).to eq(1)
expect { user.set_initial_roles! }.not_to change(UserRole, :count)
expect(user).to be_trusted
end
it "does not change any roles if the user is not valid" do
user = create(:user, :tag_moderator)
expect(UserRole.count).to eq(1)
user.username = ""
expect { user.set_initial_roles! }.not_to change(UserRole, :count)
expect(user).to be_tag_moderator
end
it "does nothing if the user has not been persisted" do
user = build(:user)
expect(UserRole.count).to eq(0)
expect { user.set_initial_roles! }.not_to change(UserRole, :count)
end
end
describe "#calculate_score" do
it "calculates a score" do
user.update_column(:badge_achievements_count, 3)

94
spec/requests/registrations_spec.rb
View file

@ -232,15 +232,46 @@ RSpec.describe "Registrations" do
expect(User.all.size).to be 1
end
it "marks as registered" do
it "registers a user in good standing" do
post "/users", params:
{ user: { name: "test #{rand(10)}",
username: "haha_#{rand(10)}",
email: "yoooo#{rand(100)}@yo.co",
password: "PaSSw0rd_yo000",
password_confirmation: "PaSSw0rd_yo000" } }
expect(User.last.registered).to be true
expect(User.last.registered_at).not_to be_nil
new_user = User.last
expect(new_user.registered).to be true
expect(new_user.registered_at).not_to be_nil
expect(new_user).not_to be_limited
end
it "limits the user if the admins have set new user status to limited" do
allow(Settings::Authentication).to receive(:new_user_status).and_return("limited")
user = build(:user)
user_attributes = user.slice(:name, :username, :email)
post "/users", params:
{ user: { **user_attributes, password: "Passw0rd!", password_confirmation: "Passw0rd!" } }
new_user = User.last
expect(new_user.registered).to be true
expect(new_user.registered_at).not_to be_nil
expect(new_user).to be_limited
end
it "logs in user and redirects to the root path" do
user = build(:user)
user_attributes = user.slice(:name, :username, :email)
post "/users", params:
{ user: { **user_attributes, password: "Passw0rd!", password_confirmation: "Passw0rd!" } }
new_user = User.last
expect(new_user).to have_attributes(user_attributes)
expect(controller.current_user).to eq(new_user)
expect(response).to redirect_to(root_path)
end
it "does not create user with password confirmation mismatch" do
@ -264,6 +295,44 @@ RSpec.describe "Registrations" do
end
end
context "when email registration is allowed and confirmation is required" do
before do
allow(Settings::Authentication).to receive(:allow_email_password_registration).and_return(true)
allow(ForemInstance).to receive(:smtp_enabled?).and_return(true)
allow_any_instance_of(ProfileImageUploader).to receive(:download!)
end
it "registers the user but does not log them in" do
user = build(:user)
user_attributes = user.slice(:name, :username, :email)
post "/users", params:
{ user: { **user_attributes, password: "Passw0rd!", password_confirmation: "Passw0rd!" } }
new_user = User.last
expect(new_user.registered).to be true
expect(new_user.registered_at).not_to be_nil
expect(new_user).not_to be_limited
expect(controller.current_user).to be_nil
expect(response).to redirect_to(confirm_email_path(email: user.email))
end
it "also limits the user first if the admins have set new user status to limited" do
allow(Settings::Authentication).to receive(:new_user_status).and_return("limited")
user = build(:user)
user_attributes = user.slice(:name, :username, :email)
post "/users", params:
{ user: { **user_attributes, password: "Passw0rd!", password_confirmation: "Passw0rd!" } }
new_user = User.last
expect(new_user).to be_limited
expect(controller.current_user).to be_nil
expect(response).to redirect_to(confirm_email_path(email: user.email))
end
end
context "when email registration allowed and email allow list empty" do
before do
allow_any_instance_of(ProfileImageUploader).to receive(:download!)
@ -379,6 +448,20 @@ RSpec.describe "Registrations" do
expect(User.first.email).to eq user_email
end
it "logs in user and redirects them to the creator settings path" do
user = build(:user)
user_attributes = user.slice(:name, :username, :email)
post "/users", params:
{ user: { **user_attributes, password: "Passw0rd!", password_confirmation: "Passw0rd!" } }
new_user = User.first
expect(new_user.registered).to be true
expect(new_user.registered_at).not_to be_nil
expect(controller.current_user).to eq(new_user)
expect(response).to redirect_to(new_admin_creator_setting_path)
end
it "makes user super admin and config admin" do
post "/users", params:
{ user: { name: "test #{rand(10)}",
@ -388,6 +471,8 @@ RSpec.describe "Registrations" do
password_confirmation: "PaSSw0rd_yo000" } }
expect(User.first.super_admin?).to be true
expect(User.first.trusted?).to be true
expect(User.first.creator?).to be true
expect(User.first.limited?).to be false
end
it "creates mascot user" do
@ -471,6 +556,9 @@ RSpec.describe "Registrations" do
password: "PaSSw0rd_yo000",
password_confirmation: "PaSSw0rd_yo000" } }
expect(User.first.super_admin?).to be true
expect(User.first.trusted?).to be true
expect(User.first.creator?).to be true
expect(User.first.limited?).to be false
end
end
end

52
spec/services/authentication/authenticator_spec.rb
View file

@ -19,6 +19,48 @@ RSpec.describe Authentication::Authenticator, type: :service do
end
end
shared_examples "preserves existing user roles" do
it "does not make any changes to roles if the user already exists" do
existing_user = create(:user, :trusted)
auth_payload.info.email = existing_user.email
expect do
user = service.call
expect(user).to eq(existing_user)
expect(user).to be_trusted
expect(user).not_to be_limited
end.not_to change(existing_user.roles, :count)
end
end
shared_examples "appropriate new user status" do
it "registers a user in good standing by default" do
user = service.call
expect(user.registered).to be(true)
expect(user.registered_at).not_to be_nil
expect(user).not_to be_limited
end
include_examples "preserves existing user roles"
context "when new user status admin setting is limited" do
before do
allow(Settings::Authentication).to receive(:new_user_status).and_return("limited")
end
it "registers the user as limited" do
user = service.call
expect(user.registered).to be(true)
expect(user.registered_at).not_to be_nil
expect(user).to be_limited
end
include_examples "preserves existing user roles"
end
end
context "when authenticating through an unknown provider" do
it "raises ProviderNotFound" do
auth_payload = OmniAuth.config.mock_auth[:github].merge(provider: "okta")
@ -34,6 +76,8 @@ RSpec.describe Authentication::Authenticator, type: :service do
include_context "spam handling"
include_context "appropriate new user status"
describe "new user" do
it "creates a new user" do
expect do
@ -229,6 +273,8 @@ RSpec.describe Authentication::Authenticator, type: :service do
include_context "spam handling"
include_context "appropriate new user status"
describe "new user" do
it "creates a new user" do
expect do
@ -422,6 +468,8 @@ RSpec.describe Authentication::Authenticator, type: :service do
include_context "spam handling"
include_context "appropriate new user status"
describe "new user" do
it "creates a new user" do
expect do
@ -507,6 +555,8 @@ RSpec.describe Authentication::Authenticator, type: :service do
include_context "spam handling"
include_context "appropriate new user status"
describe "new user" do
it "creates a new user" do
expect do
@ -685,6 +735,8 @@ RSpec.describe Authentication::Authenticator, type: :service do
include_context "spam handling"
include_context "appropriate new user status"
describe "new user" do
it "creates a new user" do
expect do