Skip to content

Commit

Permalink
Merge pull request #3220 from manyfold3d/approve-signups
Browse files Browse the repository at this point in the history
Add option to require moderator approval for new signups
  • Loading branch information
Floppy authored Nov 22, 2024
2 parents 9301a69 + cbecdf2 commit e3217f6
Show file tree
Hide file tree
Showing 18 changed files with 142 additions and 8 deletions.
4 changes: 4 additions & 0 deletions app/controllers/settings/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ def update
if params[:reset]
@user.send_reset_password_instructions
redirect_to [:settings, @user], notice: t(".reset_link_sent")
elsif params[:approve]
@user.update(approved: true)
UserMailer.with(user: @user).account_approved.deliver_later if SiteSettings.email_configured?
redirect_to [:settings, @user], notice: t(".approved")
elsif @user.update(user_params)
redirect_to [:settings, @user], notice: t(".success")
else
Expand Down
1 change: 1 addition & 0 deletions app/controllers/settings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def update_analysis_settings(settings)

def update_multiuser_settings(settings)
return unless settings
SiteSettings.approve_signups = (settings[:approve_signups])
SiteSettings.default_viewer_role = (settings[:default_viewer_role].presence)
end

Expand Down
4 changes: 3 additions & 1 deletion app/controllers/users/registrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ def edit
# POST /users
def create
authorize User
super
super do |user|
user.update(approved: false) if SiteSettings.approve_signups
end
end

# PUT /resource
Expand Down
6 changes: 6 additions & 0 deletions app/mailers/user_mailer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class UserMailer < ApplicationMailer
def account_approved
@user = params[:user]
mail to: @user.email
end
end
1 change: 1 addition & 0 deletions app/models/site_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class SiteSettings < RailsSettings::Base
field :analyse_manifold, type: :boolean, default: false
field :anonymous_usage_id, type: :string, default: nil
field :default_viewer_role, type: :string, default: "member"
field :approve_signups, type: :boolean, default: false

def self.registration_enabled?
Rails.application.config.manyfold_features[:registration]
Expand Down
21 changes: 21 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,27 @@ def self.user_count(range)
User.where(updated_at: range).count
end

# Devise approval checks
def active_for_authentication?
super && approved?
end

def inactive_message
approved? ? super : :not_approved
end

def self.send_reset_password_instructions(attributes = {})
recoverable = find_or_initialize_with_errors(reset_password_keys, attributes, :not_found)
if recoverable.persisted?
if recoverable.approved?
recoverable.send_reset_password_instructions
else
recoverable.errors.add(:base, :not_approved)
end
end
recoverable
end

private

def has_any_role_of?(*args)
Expand Down
1 change: 1 addition & 0 deletions app/views/devise/registrations/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<%= text_input_row form, :username, autocomplete: "username" %>
<%= password_input_row form, :password, autocomplete: "new-password" %>
<%= password_input_row form, :password_confirmation, autocomplete: "new-password" %>
<%= content_tag :div, t(".approval_help"), class: "alert alert-info" if SiteSettings.approve_signups %>
<%= form.submit translate(".sign_up"), class: "btn btn-primary" %>
<% end %>
Expand Down
15 changes: 13 additions & 2 deletions app/views/settings/multiuser.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<td><%= SiteSettings.multiuser_enabled? ? "✅" : "❌" %></td>
</tr>
<tr>
<td><%= t(".registration") %></td>
<td><%= t(".registration_open") %></td>
<td><%= SiteSettings.registration_enabled? ? "✅" : "❌" %></td>
</tr>
<tr>
Expand All @@ -29,10 +29,21 @@
<%= t(".details_html") %>
</p>
<hr>
<% if SiteSettings.registration_enabled? %>
<h3><%= t(".registration") %></h3>
<div class="row">
<%= form.label nil, t(".approve_signups.label"), for: "multiuser[approve_signups]", class: "col-sm-4 col-form-label" %>
<div class="col-sm-8 form-check form-switch">
<%= form.check_box "multiuser[approve_signups]", checked: SiteSettings.approve_signups, class: "form-check-input" %>
<small><%= t(".approve_signups.help") %></small>
</div>
</div>
<hr>
<% end %>
<h3><%= t(".permissions") %></h3>
<div class="row">
<%= form.label nil, t(".default_viewer_role.label"), for: "multiuser[default_viewer_role]", class: "col-sm-4 col-form-label" %>
<div class="col-sm-8 form-check form-switch">
<div class="col-sm-8">
<%= form.select "multiuser[default_viewer_role]",
[[translate(".default_viewer_role.options.none"), nil],
[translate(".default_viewer_role.options.member"), "member"]],
Expand Down
7 changes: 5 additions & 2 deletions app/views/settings/users/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@
<th></th>
</tr>
<% @users.each do |user| %>
<tr>
<td><%= user.entity.is_a?(User) ? user.entity.username : user.username %></td>
<tr class="<%= user.entity.approved? ? "" : "table-info" %>">
<td>
<%= icon("hourglass", "") unless user.entity.approved? %>
<%= user.entity.is_a?(User) ? user.entity.username : user.username %>
</td>
<td><%= user.entity.is_a?(User) ? masked_email(user.entity.email) : nil %></td>
<%= content_tag :td, (user.entity.auth_uid ? "✅" : "❌") if SiteSettings.oidc_enabled? %>
<%= content_tag :td, (user.local? ? "✅" : "❌") if SiteSettings.federation_enabled? %>
Expand Down
8 changes: 8 additions & 0 deletions app/views/settings/users/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@
<th><%= User.human_attribute_name(:email) %></th>
<td><%= @user.email %></td>
</tr>
<% unless @user.approved? %>
<tr>
<th><%= User.human_attribute_name(:approved) %></th>
<td>
<%= link_to safe_join([icon("person-check", t(".approve")), t(".approve")], " "), settings_user_path(@user, approve: true), method: :patch, class: "btn btn-primary btn-sm" if policy(@user).edit? %>
</td>
</tr>
<% end %>
<tr>
<th><%= User.human_attribute_name(:created_at) %></th>
<td><%= @user.created_at.to_fs(:long) %></td>
Expand Down
3 changes: 3 additions & 0 deletions app/views/user_mailer/account_approved.text.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<%= translate(".greeting") %>
<%= translate(".message", link: new_user_session_url) %>
4 changes: 4 additions & 0 deletions config/locales/devise/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ en:
invalid: Invalid %{authentication_keys} or password.
last_attempt: You have one more attempt before your account is temporarily locked.
locked: Your account is locked for an hour; try again later.
not_approved: Your account has not yet been approved by a moderator.
not_found_in_database: Invalid %{authentication_keys} or password.
timeout: Your session expired. Please sign in again to continue.
unauthenticated: You need to sign in or sign up before continuing.
Expand Down Expand Up @@ -103,6 +104,7 @@ en:
mask: Blur in lists
show: Show as normal
new:
approval_help: After creating your account, it will be manually reviewed by our moderators. We'll let you know when approval is complete and you can sign in.
sign_up: Sign up
pagination_settings:
collections:
Expand Down Expand Up @@ -168,6 +170,8 @@ en:
update_needs_confirmation: You updated your account successfully, but we need to verify your new email address. Please check your email and follow the confirmation link to confirm your new email address.
updated: Your account has been updated successfully.
updated_but_not_signed_in: Your account has been updated successfully, but since your password was changed, you need to sign in again.
user:
signed_up_but_not_approved: You have signed up successfully. A moderator will review your account soon, after which you will be able to sign in.
sessions:
already_signed_out: Signed out successfully.
new:
Expand Down
6 changes: 6 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ en:
problematic_type: Object Type
severity: Severity
user:
approved: Account pending
confirmation_sent_at: Confirmation sent at
confirmation_token: Confirmation token
confirmed_at: Confirmed at
Expand Down Expand Up @@ -393,6 +394,11 @@ en:
myminifactory: MyMiniFactory
theminiindex: The Mini Index
thingiverse: Thingiverse
user_mailer:
account_approved:
subject: Account approved
greeting: Hi!
message: Your account has been approved; you may now sign in at %{link}.
views:
pagination:
first: "« First"
Expand Down
8 changes: 7 additions & 1 deletion config/locales/settings/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ en:
general:
submit: Save Settings
multiuser:
approve_signups:
help: All new accounts must be approved by a moderator before they can log in.
label: Approve accounts
default_viewer_role:
help: Who can view new models / creators / collections by default?
label: Default visibility
Expand All @@ -42,7 +45,8 @@ en:
multiuser_mode: Manyfold is running in multiuser mode.
oidc: OpenID Connect configured?
permissions: Permissions
registration: Account registration open?
registration: Account registration
registration_open: Account registration open?
single_user_mode: Manyfold is running in single user mode.
reporting:
description_html: If you enable usage tracking, the following information will be sent once a day to <code>%{endpoint}</code>. The <code>id</code> is randomly generated when you enable tracking.
Expand Down Expand Up @@ -88,9 +92,11 @@ en:
new:
title: Create user
show:
approve: Approve user
confirm_destroy: Are you sure you want to remove this user account immediately? This cannot be undone!
reset_password: Reset password
title: 'User details: %{username}'
update:
approved: User was approved successfully.
reset_link_sent: A link has been sent to the user for them to set a new password.
success: User updated successfully.
8 changes: 8 additions & 0 deletions db/migrate/20241122121621_add_approved_to_user.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class AddApprovedToUser < ActiveRecord::Migration[7.2]
def change
change_table :users do |t|
t.boolean :approved, default: true, null: false
t.index :approved
end
end
end
4 changes: 3 additions & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions spec/mailers/user_mailer_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
require "rails_helper"

RSpec.describe UserMailer do
describe "account_approved" do
let(:user) { create(:user) }
let(:mail) { described_class.with(user: user).account_approved }

it "sets correct subject" do
expect(mail.subject).to eq("Account approved")
end

it "sends to user" do
expect(mail.to).to eq([user.email])
end

it "renders the body" do
expect(mail.body.encoded).to match("Hi")
end
end
end
29 changes: 28 additions & 1 deletion spec/requests/users/registrations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,17 @@
end
end

describe "POST /users" do
describe "POST /users with approval disabled" do
before { post "/users", params: post_options }

it "creates a new user" do
expect(User.count).to eq 2
end

it "creates user in approved state" do
expect(User.last).to be_approved
end

it "redirects to root" do
expect(response).to redirect_to("/")
end
Expand All @@ -286,6 +290,29 @@
end
end

describe "POST /users with approval enabled" do
before {
allow(SiteSettings).to receive(:approve_signups).and_return(true)
post "/users", params: post_options
}

it "creates a new user" do
expect(User.count).to eq 2
end

it "creates user in pending state" do
expect(User.last).not_to be_approved
end

it "redirects to root" do
expect(response).to redirect_to("/")
end

it "does not sign in the user" do
expect(controller.current_user).to be_nil
end
end

describe "GET /users/cancel without a signup in progress" do
before { get "/users/cancel" }

Expand Down

0 comments on commit e3217f6

Please sign in to comment.