Add user roles

.rubocop.yml

@@ -19,7 +19,7 @@ AllCops:
 # Join tables don't really need timestamps
 Rails/CreateTableWithTimestamps:
   Exclude:
-    # - 'db/migrate/20180128231930_create_organizations_and_events.rb'
+    - db/migrate/20220919194501_rolify_create_roles.rb
 
 # Rails generates this file
 Style/BlockComments:
@@ -64,6 +64,10 @@ Rails/HasManyOrHasOneDependent:
 
 Rails/HasAndBelongsToMany:
   Enabled: true
+  Exclude:
+    # Rolify uses HABTM. Despite a decade of the community attempting to implement
+    # `has_many: through`, it still struggles mightily with it. Let's make an exception.
+    - app/models/role.rb
 
 Style/NumericPredicate:
   Enabled: true

Gemfile

@@ -178,3 +178,6 @@ gem "country_select", "~> 8.0"
 
 # Used for sending email through Mailgun
 gem "mailgun-ruby", "~> 1.2"
+
+# Rolify is used for user roles
+gem "rolify", "~> 6.0"

Gemfile.lock

@@ -326,6 +326,7 @@ GEM
       mime-types (>= 1.16, < 4.0)
       netrc (~> 0.8)
     rexml (3.2.5)
+    rolify (6.0.0)
     rubocop (1.28.1)
       parallel (~> 1.10)
       parser (>= 3.1.0.0)
@@ -506,6 +507,7 @@ DEPENDENCIES
   rails (~> 7.0.2.3)
   rake
   redis (~> 4.0)
+  rolify (~> 6.0)
   rubocop
   rubocop-minitest
   rubocop-performance

app/controllers/accounts_controller.rb

@@ -87,6 +87,8 @@ def create
     raise InvalidTokenError if @user.new_record?
     raise InvalidUpdatePasswordError if typed_params.password.blank? || @user.invalid?
 
+    @user.remove_role :new_user
+
     sign_in @user
     redirect_to after_sign_in_path_for(@user)
   end

app/controllers/application_controller.rb

@@ -75,15 +75,15 @@ def authenticate_super_user
     # First we make sure they're logged in at all, this also sets the current user so we can check it
     return false unless authenticate_user!
 
-    current_user.super_admin?
+    current_user.is_admin?
   end
 
   sig { void }
   def authenticate_super_user!
     # First we make sure they're logged in at all, this also sets the current user so we can check it
     authenticate_user!
 
-    unless current_user.super_admin?
+    unless current_user.is_admin?
       redirect_back_or_to "/", allow_other_host: false, alert: "You must be a super user/admin to access this page."
     end
   end

app/models/role.rb

@@ -0,0 +1,13 @@
+class Role < ApplicationRecord
+  has_and_belongs_to_many :users, join_table: :users_roles
+
+  belongs_to :resource,
+             polymorphic: true,
+             optional: true
+
+  validates :resource_type,
+            inclusion: { in: Rolify.resource_types },
+            allow_nil: true
+
+  scopify
+end

app/models/user.rb

@@ -1,6 +1,11 @@
 # typed: strict
 
 class User < ApplicationRecord
+  rolify
+  devise :database_authenticatable, :registerable,
+         :recoverable, :rememberable, :validatable,
+         :trackable, :lockable, :confirmable
+
   has_many :api_keys, dependent: :delete_all
   has_many :archive_items, foreign_key: :submitter_id, dependent: :nullify
 
@@ -9,17 +14,6 @@ class User < ApplicationRecord
 
   has_one :applicant, dependent: :destroy
 
-  # Include default devise modules. Others available are:
-  # :timeoutable and :omniauthable
-  devise :database_authenticatable, :registerable,
-         :recoverable, :rememberable, :validatable,
-         :trackable, :lockable, :confirmable
-
-  sig { returns(T::Boolean) }
-  def super_admin?
-    self.super_admin
-  end
-
   # `Devise::Recoverable#set_reset_password_token` is a protected method, which prevents us from
   # calling it directly. Since we need to be able to do that for tests and for duck-punching other
   # `Devise::Recoverable` methods, we pull it into the public space here.
@@ -33,7 +27,7 @@ def set_reset_password_token
   # Like the original method, it also creates the user's `reset_password_token`.
   sig { returns(String) }
   def send_setup_instructions
-    raise AlreadySetupError if sign_in_count.positive?
+    raise AlreadySetupError unless self.is_new_user?
 
     token = set_reset_password_token
 
@@ -54,7 +48,7 @@ def send_setup_instructions
   def self.create_from_applicant(applicant)
     raise ApplicantNotApprovedError unless applicant.approved?
 
-    self.create!({
+    user = self.create!({
       applicant: applicant,
       email: applicant.email,
       # The user will have to change their password immediately. This is just to pass validation.
@@ -64,6 +58,20 @@ def self.create_from_applicant(applicant)
       confirmed_at: applicant.confirmed_at,
       confirmation_sent_at: applicant.confirmation_sent_at
     })
+
+    user.assign_default_roles
+
+    user
+  end
+
+  # All new users are implicitly Insights users.
+  # All new users are also "new" until they have completed their initial setup.
+  sig { void }
+  def assign_default_roles
+    if self.roles.blank?
+      self.add_role :new_user
+      self.add_role :insights_user
+    end
   end
 end
 

app/views/layouts/_header.html.erb

@@ -18,7 +18,7 @@
             <li>
               <%= link_to "Settings", account_path, class: "block no-underline py-2 px-4 text-sm text-gray-700 hover:bg-gray-100 dark:hover:bg-gray-600 dark:text-gray-200 dark:hover:text-white" %>
             </li>
-            <% if current_user.super_admin? %>
+            <% if current_user.is_admin? %>
               <li>
                 <%= link_to jobs_status_index_path, class: "flex flex-inline gap-1 block no-underline py-2 px-4 text-sm text-gray-700 hover:bg-gray-100 dark:hover:bg-gray-600 dark:text-gray-200 dark:hover:text-white" do %>
                   <svg xmlns="http://www.w3.org/2000/svg" class="h-5 w-5" viewBox="0 0 20 20" fill="currentColor"><path fill-rule="evenodd" d="M11.3 1.046A1 1 0 0112 2v5h4a1 1 0 01.82 1.573l-7 10A1 1 0 018 18v-5H4a1 1 0 01-.82-1.573l7-10a1 1 0 011.12-.38z" clip-rule="evenodd" /></svg>

config/initializers/rolify.rb

@@ -0,0 +1,10 @@
+Rolify.configure do |config|
+  # By default ORM adapter is ActiveRecord. uncomment to use mongoid
+  # config.use_mongoid
+
+  # Dynamic shortcuts for User class (user.is_admin? like methods). Default is: false
+  config.use_dynamic_shortcuts
+
+  # Configuration to remove roles from database once the last resource is removed. Default is: true
+  # config.remove_role_if_empty = false
+end

db/migrate/20220919194501_rolify_create_roles.rb

@@ -0,0 +1,18 @@
+class RolifyCreateRoles < ActiveRecord::Migration[7.0]
+  def change
+    create_table "roles", id: :uuid do |t|
+      t.string :name
+      t.references :resource, polymorphic: true
+
+      t.timestamps
+    end
+
+    create_table "users_roles", id: false do |t|
+      t.references :user, type: :uuid
+      t.references :role, type: :uuid
+    end
+
+    add_index :roles, [ :name, :resource_type, :resource_id ]
+    add_index :users_roles, [ :user_id, :role_id ]
+  end
+end

db/migrate/20220919214503_convert_admins_to_rolify.rb

@@ -0,0 +1,15 @@
+class ConvertAdminsToRolify < ActiveRecord::Migration[7.0]
+  def up
+    User.where(super_admin: true).each { |admin| admin.add_role :admin }
+
+    remove_column :users, :super_admin
+  end
+  def down
+    add_column :users, :super_admin, :boolean, default: false
+
+    User.with_role(:admin).each do |admin|
+      admin.update super_admin: true
+      admin.remove_role :admin
+    end
+  end
+end

db/seeds.rb

@@ -11,12 +11,12 @@
 easy_password = "password123"
 
 # Super-admin account; no applicant necessary.
-User.create!({
+admin = User.create!({
   email: "[email protected]",
   password: easy_password,
-  super_admin: true,
   confirmed_at: Time.now,
 })
+admin.add_role :admin
 
 Applicant.create!([
   # This applicant is a fresh, unconfirmed applicant.
@@ -84,8 +84,6 @@
   # Override the randomized initial password.
   password: easy_password,
   password_confirmation: easy_password,
-  # Make sure they don't look fresh.
-  sign_in_count: 1,
 })
 
 # Create the restricted user
@@ -95,8 +93,6 @@
   # Override the randomized initial password.
   password: easy_password,
   password_confirmation: easy_password,
-  # Make sure they don't look fresh.
-  sign_in_count: 1,
 })
 
 Sources::Tweet.create_from_url "https://twitter.com/kairyssdal/status/1415029747826905090"

docs/ARCHITECTURE.md

@@ -21,7 +21,7 @@ Zenodotus allows its users to search its archive using image or text inputs. Sea
 
 ## User model
 
-Zenodotus' `User` model handles authentication for the app via [Devise](https://github.com/heartcombo/devise). Internal users may be indicated as such with the `super_admin` boolean.
+Zenodotus' `User` model handles authentication for the app via [Devise](https://github.com/heartcombo/devise). Roles are managed with the Rolify gem. Internal users have the `:admin` role and are recognized with the `is_admin?` helper (provided by Rolify automatically).
 
 ## MediaReview
 Coming soon

test/controllers/accounts_controller_test.rb

@@ -4,7 +4,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
   include Devise::Test::IntegrationHelpers
 
   test "can get the setup page with a valid token" do
-    user = users(:user1)
+    user = users(:user)
 
     token = user.set_reset_password_token
 
@@ -21,7 +21,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "cannot get setup page while logged in" do
-    user = users(:user1)
+    user = users(:user)
 
     # Use a valid token to make this a legitimate test.
     # The redirection will run before the token is even checked, but we want to make sure it's an
@@ -36,7 +36,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "can setup an account as a new user" do
-    user = users(:user1)
+    user = users(:user)
 
     post create_account_path({
       reset_password_token: user.set_reset_password_token,
@@ -57,7 +57,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "cannot setup an account with a previously-used token" do
-    user = users(:user1)
+    user = users(:user)
 
     create_params = {
       reset_password_token: user.set_reset_password_token,
@@ -76,7 +76,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "cannot setup an account with blank passwords" do
-    user = users(:user1)
+    user = users(:user)
 
     post create_account_path({
       reset_password_token: user.set_reset_password_token,
@@ -87,7 +87,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "cannot setup an account with short passwords" do
-    user = users(:user1)
+    user = users(:user)
     short_password = "password"[0, Devise.password_length.begin - 1]
 
     post create_account_path({
@@ -99,7 +99,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "cannot setup an account with mismatching passwords" do
-    user = users(:user1)
+    user = users(:user)
 
     post create_account_path({
       reset_password_token: user.set_reset_password_token,
@@ -108,4 +108,18 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
     })
     assert_response :bad_request
   end
+
+  test "should remove new_user role during setup" do
+    user = users(:new_user)
+
+    assert user.is_new_user?
+
+    post create_account_path({
+      reset_password_token: user.set_reset_password_token,
+      password: "password1",
+      password_confirmation: "password1"
+    })
+
+    assert_not user.is_new_user?
+  end
 end

test/controllers/archive_controller_test.rb

@@ -24,7 +24,7 @@ def after_all
   end
 
   test "load index if authenticated" do
-    sign_in users(:user1)
+    sign_in users(:user)
     get root_url
     assert_response :success
   end