Skip to content

Commit

Permalink
🎁 Adding WorkAuthorization model for CDL
Browse files Browse the repository at this point in the history
With this commit, we're adding a mechanism to "lend" folks books.  This
change accounts for granting a user read rights to a work.

It does not introduce the logic/functionality that "authorizes" or
"revokes" this loaned resource.  With this commit, there are no changed
production logic paths.

Related to:

- #633

Co-authored-by: Summer Cook <[email protected]>
  • Loading branch information
jeremyf and summer-cook committed Aug 7, 2023
1 parent a9a6f2f commit 98cc8aa
Show file tree
Hide file tree
Showing 4 changed files with 167 additions and 1 deletion.
72 changes: 72 additions & 0 deletions app/models/work_authorization.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# frozen_string_literal: true

# WorkAuthorization models users granted access to works. The instigator of the authorizations is
# outside the model. In the case of PALNI/PALCI there will be a Shibboleth/SAML authentication that
# indicates we should create a WorkAuthorization entry.
#
# @note Transactions across data storage layers (e.g. postgres and fedora) are precarious. Fedora
# doesn't have proper transactions and there is not a clear concept of Postgres and Fedora
# sharing a transaction pool. However, we can emulate one by having a postgres transaction that:
# first does all of the postgres and then does one (ideally single) fedora change. It is
# not bullet proof but does hopefully improve the chances of data integrity.
#
# @see https://github.com/scientist-softserv/palni-palci/issues/633
class WorkAuthorization < ActiveRecord::Base # rubocop:disable ApplicationRecord
class WorkNotFoundError < StandardError
def initialize(user:, work_pid:)
"Unable to authorize #{user.class} #{user.user_key.inspect} for work with ID=#{work_pid} because work does not exist."
end
end

belongs_to :user

# This will be a non-ActiveRecord resource
validates :work_pid, presence: true

##
# Grant the given :user permission to read the work associated with the given :work_pid.
#
# @param user [User]
# @param work_pid [String]
#
# @raise [WorkAuthorization::WorkNotFoundError] when the given :work_pid is not found.
#
# @see .revoke!
# rubocop:disable Rails/FindBy
def self.authorize!(user:, work_pid:)
work = ActiveFedora::Base.where(id: work_pid).first
raise WorkNotFoundError.new(user: user, work_pid: work_pid) unless work

transaction do
authorization = find_or_create_by!(user_id: user.id, work_pid: work.id)
authorization.update!(work_title: work.title)
work.set_read_users([user.user_key], [user.user_key])
work.save!
end
end
# rubocop:enable Rails/FindBy

##
# Remove permission for the given :user to read the work associated with the given :work_pid.
#
# @param user [User]
# @param work_pid [String]
#
# @see .authorize!
# rubocop:disable Rails/FindBy
def self.revoke!(user:, work_pid:)
# When we delete the authorizations, we want to ensure that we've tidied up the corresponding
# work's read users. If for some reason the ActiveFedora save fails, this the destruction of
# the authorizations will rollback. Meaning we still have a record of what we've authorized.
transaction do
where(user_id: user.id, work_pid: work_pid).destroy_all
work = ActiveFedora::Base.where(id: work_pid).first
if work
work.set_read_users([], [user.user_key])
work.save!
end
true
end
end
# rubocop:enable Rails/FindBy
end
13 changes: 13 additions & 0 deletions db/migrate/20230804202804_add_work_authorizations.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class AddWorkAuthorizations < ActiveRecord::Migration[5.2]
def change
create_table "work_authorizations", force: :cascade do |t|
t.string "work_title"
t.belongs_to "user"
t.datetime "expires_at", index: true
t.string "work_pid", index: true, null: false
t.string "scope"
t.string "error", default: nil
t.timestamps
end
end
end
24 changes: 23 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2023_07_18_202804) do
ActiveRecord::Schema.define(version: 2023_08_04_202804) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand Down Expand Up @@ -83,6 +83,9 @@
t.datetime "last_succeeded_at"
t.string "importerexporter_type", default: "Bulkrax::Importer"
t.integer "import_attempts", default: 0
t.index ["identifier"], name: "index_bulkrax_entries_on_identifier"
t.index ["importerexporter_id", "importerexporter_type"], name: "bulkrax_entries_importerexporter_idx"
t.index ["type"], name: "index_bulkrax_entries_on_type"
end

create_table "bulkrax_exporter_runs", force: :cascade do |t|
Expand Down Expand Up @@ -165,7 +168,9 @@
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.integer "order", default: 0
t.index ["child_id"], name: "index_bulkrax_pending_relationships_on_child_id"
t.index ["importer_run_id"], name: "index_bulkrax_pending_relationships_on_importer_run_id"
t.index ["parent_id"], name: "index_bulkrax_pending_relationships_on_parent_id"
end

create_table "bulkrax_statuses", force: :cascade do |t|
Expand All @@ -179,6 +184,9 @@
t.string "runnable_type"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["error_class"], name: "index_bulkrax_statuses_on_error_class"
t.index ["runnable_id", "runnable_type"], name: "bulkrax_statuses_runnable_idx"
t.index ["statusable_id", "statusable_type"], name: "bulkrax_statuses_statusable_idx"
end

create_table "checksum_audit_logs", id: :serial, force: :cascade do |t|
Expand Down Expand Up @@ -857,6 +865,20 @@
t.datetime "updated_at"
end

create_table "work_authorizations", force: :cascade do |t|
t.string "work_title"
t.bigint "user_id"
t.datetime "expires_at"
t.string "work_pid", null: false
t.string "scope"
t.string "error"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["expires_at"], name: "index_work_authorizations_on_expires_at"
t.index ["user_id"], name: "index_work_authorizations_on_user_id"
t.index ["work_pid"], name: "index_work_authorizations_on_work_pid"
end

create_table "work_view_stats", id: :serial, force: :cascade do |t|
t.datetime "date"
t.integer "work_views"
Expand Down
59 changes: 59 additions & 0 deletions spec/models/work_authorization_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
require 'spec_helper'

require 'cancan/matchers'

RSpec.describe WorkAuthorization, type: :model do
let(:work) { FactoryBot.create(:generic_work) }
let(:borrowing_user) { FactoryBot.create(:user) }
let(:ability) { ::Ability.new(borrowing_user) }

describe '.authorize!' do
it 'gives the borrowing user the ability to "read" the work' do
# We re-instantiate an ability class because CanCan caches many of the ability checks. By
# both passing the id and reinstantiating, we ensure that we have the most fresh data; that is
# no cached ability "table" nor cached values on the work.
expect { described_class.authorize!(user: borrowing_user, work_pid: work.id) }
.to change { ::Ability.new(borrowing_user).can?(:read, work.id) }.from(false).to(true)
end

context 'when the work_pid is not found' do
it 'raises a WorkAuthorization::WorkNotFoundError' do
expect { described_class.authorize!(user: borrowing_user, work_pid: "oh-so-404") }.to raise_error(WorkAuthorization::WorkNotFoundError)
end
end
end

describe '.revoke!' do
it 'revokes an authorized user from being able to "read" the work' do
# Ensuring we're authorized
described_class.authorize!(user: borrowing_user, work_pid: work.id)

expect { described_class.revoke!(user: borrowing_user, work_pid: work.id) }
.to change { ::Ability.new(borrowing_user).can?(:read, work.id) }.from(true).to(false)
end

it 'gracefully handles revocation of non-existent works' do
expect(described_class.revoke!(user: borrowing_user, work_pid: "oh-so-404")).to be_truthy
end

it 'gracefully handles revoking that which was never authorized' do
expect { described_class.revoke!(user: borrowing_user, work_pid: work.id) }
.not_to change { ::Ability.new(borrowing_user).can?(:read, work.id) }.from(false)
end
end

xdescribe 'adding errors' do
let(:authorization) { WorkAuthorization.new }

it 'adds the error' do
authorization.update_error 'test error'
expect(authorization.error).to eq('test error')
end

it 'clears error' do
authorization.update_error 'test error'
authorization.clear_error
expect(authorization.error).to eq(nil)
end
end
end

0 comments on commit 98cc8aa

Please sign in to comment.