diff --git a/app/controllers/settings/users_controller.rb b/app/controllers/settings/users_controller.rb index 11247cbc9..c637b9990 100644 --- a/app/controllers/settings/users_controller.rb +++ b/app/controllers/settings/users_controller.rb @@ -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 diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index 9cc38361d..b25dcbe0b 100644 --- a/app/controllers/settings_controller.rb +++ b/app/controllers/settings_controller.rb @@ -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 diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index e04e5c744..6f8490866 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -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 diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb new file mode 100644 index 000000000..4bb8ff05a --- /dev/null +++ b/app/mailers/user_mailer.rb @@ -0,0 +1,6 @@ +class UserMailer < ApplicationMailer + def account_approved + @user = params[:user] + mail to: @user.email + end +end diff --git a/app/models/site_settings.rb b/app/models/site_settings.rb index 6d2d09d4a..8d6e60bd3 100644 --- a/app/models/site_settings.rb +++ b/app/models/site_settings.rb @@ -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] diff --git a/app/models/user.rb b/app/models/user.rb index 371d63e78..e65c0416b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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) diff --git a/app/views/devise/registrations/new.html.erb b/app/views/devise/registrations/new.html.erb index 54d19a138..92cc0b29d 100644 --- a/app/views/devise/registrations/new.html.erb +++ b/app/views/devise/registrations/new.html.erb @@ -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 %> diff --git a/app/views/settings/multiuser.html.erb b/app/views/settings/multiuser.html.erb index edb63f235..13e22ba7d 100644 --- a/app/views/settings/multiuser.html.erb +++ b/app/views/settings/multiuser.html.erb @@ -9,7 +9,7 @@ <%= SiteSettings.multiuser_enabled? ? "✅" : "❌" %> - <%= t(".registration") %> + <%= t(".registration_open") %> <%= SiteSettings.registration_enabled? ? "✅" : "❌" %> @@ -29,10 +29,21 @@ <%= t(".details_html") %>


+ <% if SiteSettings.registration_enabled? %> +

<%= t(".registration") %>

+
+ <%= form.label nil, t(".approve_signups.label"), for: "multiuser[approve_signups]", class: "col-sm-4 col-form-label" %> +
+ <%= form.check_box "multiuser[approve_signups]", checked: SiteSettings.approve_signups, class: "form-check-input" %> + <%= t(".approve_signups.help") %> +
+
+
+ <% end %>

<%= t(".permissions") %>

<%= form.label nil, t(".default_viewer_role.label"), for: "multiuser[default_viewer_role]", class: "col-sm-4 col-form-label" %> -
+
<%= form.select "multiuser[default_viewer_role]", [[translate(".default_viewer_role.options.none"), nil], [translate(".default_viewer_role.options.member"), "member"]], diff --git a/app/views/settings/users/index.html.erb b/app/views/settings/users/index.html.erb index 4022b3850..9495e9bac 100644 --- a/app/views/settings/users/index.html.erb +++ b/app/views/settings/users/index.html.erb @@ -13,8 +13,11 @@ <% @users.each do |user| %> - - <%= user.entity.is_a?(User) ? user.entity.username : user.username %> + "> + + <%= icon("hourglass", "") unless user.entity.approved? %> + <%= user.entity.is_a?(User) ? user.entity.username : user.username %> + <%= user.entity.is_a?(User) ? masked_email(user.entity.email) : nil %> <%= content_tag :td, (user.entity.auth_uid ? "✅" : "❌") if SiteSettings.oidc_enabled? %> <%= content_tag :td, (user.local? ? "✅" : "❌") if SiteSettings.federation_enabled? %> diff --git a/app/views/settings/users/show.html.erb b/app/views/settings/users/show.html.erb index 863878f12..4c860bbab 100644 --- a/app/views/settings/users/show.html.erb +++ b/app/views/settings/users/show.html.erb @@ -8,6 +8,14 @@ <%= User.human_attribute_name(:email) %> <%= @user.email %> + <% unless @user.approved? %> + + <%= User.human_attribute_name(:approved) %> + + <%= 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? %> + + + <% end %> <%= User.human_attribute_name(:created_at) %> <%= @user.created_at.to_fs(:long) %> diff --git a/app/views/user_mailer/account_approved.text.erb b/app/views/user_mailer/account_approved.text.erb new file mode 100644 index 000000000..cf998f555 --- /dev/null +++ b/app/views/user_mailer/account_approved.text.erb @@ -0,0 +1,3 @@ +<%= translate(".greeting") %> + +<%= translate(".message", link: new_user_session_url) %> diff --git a/config/locales/devise/en.yml b/config/locales/devise/en.yml index 8977b0ced..cdbc5a7bd 100644 --- a/config/locales/devise/en.yml +++ b/config/locales/devise/en.yml @@ -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. @@ -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: @@ -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: diff --git a/config/locales/en.yml b/config/locales/en.yml index 0df4c3c1d..8507ad2b1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -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 @@ -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" diff --git a/config/locales/settings/en.yml b/config/locales/settings/en.yml index f602d3f1e..54ad7014e 100644 --- a/config/locales/settings/en.yml +++ b/config/locales/settings/en.yml @@ -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 @@ -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 %{endpoint}. The id is randomly generated when you enable tracking. @@ -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. diff --git a/db/migrate/20241122121621_add_approved_to_user.rb b/db/migrate/20241122121621_add_approved_to_user.rb new file mode 100644 index 000000000..be9d1087e --- /dev/null +++ b/db/migrate/20241122121621_add_approved_to_user.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index 0f1d8932e..dfa396a99 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_11_05_121830) do +ActiveRecord::Schema[7.2].define(version: 2024_11_22_121621) do create_table "caber_relations", force: :cascade do |t| t.string "subject_type" t.integer "subject_id" @@ -309,6 +309,8 @@ t.string "auth_uid" t.string "sensitive_content_handling" t.string "public_id" + t.boolean "approved", default: true, null: false + t.index ["approved"], name: "index_users_on_approved" t.index ["email"], name: "index_users_on_email", unique: true t.index ["public_id"], name: "index_users_on_public_id" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb new file mode 100644 index 000000000..1f4fd9dae --- /dev/null +++ b/spec/mailers/user_mailer_spec.rb @@ -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 diff --git a/spec/requests/users/registrations_spec.rb b/spec/requests/users/registrations_spec.rb index f584e718b..5b57f3e57 100644 --- a/spec/requests/users/registrations_spec.rb +++ b/spec/requests/users/registrations_spec.rb @@ -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 @@ -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" }