Skip to content

Commit

Permalink
Add CanCanCan for authorization
Browse files Browse the repository at this point in the history
Why these changes are being introduced:

We will need authorization for TACOS, as it will contain a variety
of features that should not be avilable to every user.

Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/TCO-31

How this addresses that need:

This adds the CanCanCan gem, which [we agreed to use for authorization'](https://github.com/MITLibraries/tacos/blob/main/docs/architecture-decisions/0006-use-cancancan-for-authorization.md).
It also introduces a very rudimentary Ability model -- mostly a placeholder
until we know what we need -- and a boolean `admin` field to the User
model.

It does not implement any rules in the Ability model beyond 'admins
can manage anything.'' I debated whether to go even that far, as there's
nothing yet to manage.

It's also worth noting that while I was initially opposed
to the admin boolean, it does appear to be an idiom within
CanCanCan, so I'm comfortable using it here. I think things
become more complicated when we combine that pattern with
role-based authorization (more on that in side effects, below).

Side effects of this change:

Unless we implement [role-based authorization](https://github.com/CanCanCommunity/cancancan/blob/develop/docs/role_based_authorization.md),
as we've done in ETD, CanCanCan seems to want us to include all of
our rules in the `initialize` method. That feels like it would get
out of hand quickly.

We had briefly discussed in a team meeting that we _don't_ want to
implement authorization similarly to ETD, with increasing permissions
at each role. (Though, that type of cascading is where
[CanCanCan excels](https://github.com/CanCanCommunity/cancancan/blob/develop/docs/define_check_abilities.md).)
It is also possible to assign multiple roles to a single user,
[albeit kludgily](https://github.com/CanCanCommunity/cancancan/blob/develop/docs/role_based_authorization.md#many-roles-per-user).

I do think roles are the way to go here, but we'll want to proceed
with caution. As mentioned in the ADR, we may learn early on that
CanCanCan is not actually the best authorization gem; finding that
we need a dozen or more roles may be one trigger to consider an
alternative gem.
  • Loading branch information
jazairi committed Aug 5, 2024
1 parent 0ed43fa commit f323491
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 2 deletions.
3 changes: 3 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ end
# Reduces boot times through caching; required in config/boot.rb
gem 'bootsnap', require: false

# Use CanCanCan for authorization
gem 'cancancan'

# Use Devise for authentication
gem 'devise'

Expand Down
2 changes: 2 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ GEM
bootsnap (1.18.3)
msgpack (~> 1.2)
builder (3.3.0)
cancancan (3.6.1)
capybara (3.40.0)
addressable
matrix
Expand Down Expand Up @@ -430,6 +431,7 @@ PLATFORMS
DEPENDENCIES
annotate
bootsnap
cancancan
capybara
climate_control
debug
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
class ApplicationController < ActionController::Base
helper Mitlibraries::Theme::Engine.helpers

rescue_from CanCan::AccessDenied do
redirect_to root_path, alert: 'Not authorized.'
end

def new_session_path(_scope)
root_path
end
Expand Down
16 changes: 16 additions & 0 deletions app/models/ability.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

class Ability
include CanCan::Ability

# Define abilities for the user here.
# See the wiki for details:
# https://github.com/CanCanCommunity/cancancan/blob/develop/docs/define_check_abilities.md
def initialize(user)
return unless user.present?
# Rules will go here.

return unless user.admin?
can :manage, :all
end
end
1 change: 1 addition & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
# email :string not null
# created_at :datetime not null
# updated_at :datetime not null
# admin :boolean default(FALSE)
#
class User < ApplicationRecord
include FakeAuthConfig
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20240805203949_add_admin_to_users.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddAdminToUsers < ActiveRecord::Migration[7.1]
def change
add_column :users, :admin, :boolean, default: false
end
end
3 changes: 2 additions & 1 deletion db/schema.rb

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

11 changes: 11 additions & 0 deletions test/fixtures/users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
# email :string not null
# created_at :datetime not null
# updated_at :datetime not null
# admin :boolean default(FALSE)
#

# This model initially had no columns defined. If you add columns to the
Expand All @@ -16,3 +17,13 @@
valid:
uid: "[email protected]"
email: "[email protected]"

basic:
uid: "[email protected]"
email: "[email protected]"
admin: false

admin:
uid: "[email protected]"
email: "[email protected]"
admin: true
15 changes: 14 additions & 1 deletion test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
# email :string not null
# created_at :datetime not null
# updated_at :datetime not null
# admin :boolean default(FALSE)
#
require "test_helper"
require 'test_helper'

class UserTest < ActiveSupport::TestCase
test 'user valid with uid and email' do
Expand All @@ -35,4 +36,16 @@ class UserTest < ActiveSupport::TestCase
user.save
assert_not user.valid?
end

test 'admin user is valid' do
user = users(:admin)
assert user.admin?
assert user.valid?
end

test 'non-admin user is valid' do
user = users(:basic)
assert_not user.admin?
assert user.valid?
end
end

0 comments on commit f323491

Please sign in to comment.