Skip to content

Commit

Permalink
🎁 Add AuthN/AuthZ for CDL assets
Browse files Browse the repository at this point in the history
Prior to this commit, we had not wired in the logic for authorization to
controlled digital lending (CDL) assets.  With this change, we're adding
the logic for handling a user authenticating to access a work.

The logic flow is as follows:

An unauthenticated user goes to a CDL asset page.  They are neither
authorized nor authenticated, so we direct them to the new session
route.  There we capture we're the URL of the requested resource and
send them on their way to authenticate.

When we perform the handshake with OpenID, we provide (as scope and
client design decision) the URL of the CDL asset.  The OpenID provider
authenticates and returns to the OmniAuth callback.  We use the returned
scope to then grant authorization to the CDL asset (and revoke any
expired authorizations).

Related to:

- #633
- #647
  • Loading branch information
jeremyf committed Sep 5, 2023
1 parent 42be90d commit dec2354
Show file tree
Hide file tree
Showing 8 changed files with 257 additions and 3 deletions.
18 changes: 18 additions & 0 deletions app/controllers/devise_decorator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
##
# OVERRIDE
# This decorator splices into Devise to set the store URL for potential WorkAuthorization.
module DeviseSessionDecorator
##
# OVERRIDE
#
# @see OmniAuth::Strategies::OpenIDConnectDecorator#requested_work_url
# @see # https://github.com/scientist-softserv/palni-palci/issues/633
def new
prepare_for_conditional_work_authorization!

super
end
end

Devise::SessionsController.prepend(WorkAuthorization::StoreUrlForScope)
Devise::SessionsController.prepend(DeviseSessionDecorator)
3 changes: 3 additions & 0 deletions app/controllers/single_signon_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# frozen_string_literal: true

class SingleSignonController < DeviseController
include WorkAuthorization::StoreUrlForScope

def index
prepare_for_conditional_work_authorization!
@identity_providers = IdentityProvider.all
render && return unless @identity_providers.count.zero?
redirect_to main_app.new_user_session_path
Expand Down
3 changes: 3 additions & 0 deletions app/controllers/users/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ def callback
@user = User.from_omniauth(request.env['omniauth.auth'])

if @user.persisted?
# https://github.com/scientist-softserv/palni-palci/issues/633
WorkAuthorization.handle_signin_for!(user: @user, scope: params[:scope])

# We need to render a loading page here just to set the sesion properly
# otherwise the logged in session gets lost during the redirect
if params[:action] == 'saml'
Expand Down
77 changes: 74 additions & 3 deletions app/models/work_authorization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,60 @@ def initialize(user:, work_pid:)
validates :work_pid, presence: true

##
# When a :user signs in, we want to re-authorize works that are part of their latest
# authentication. We want to de-authorize access to any works that are not part of their recent
# authentication.
#
# @param user [User]
# @param authorize_until [Time] authorize the given work_pid(s) until this point in time.
# @param revoke_expirations_before [Time] expire all authorizations that have expires_at less than or equal to this parameter.
# @param work_pid [String, Array<String>]
def self.handle_signin_for!(user:, authorize_until: 1.day.from_now, work_pid: nil, revoke_expirations_before: Time.zone.now)
# @param scope [String] the OpenID scope string that might include additional works to authorize.
#
# @see OmniAuth::Strategies::OpenIDConnectDecorator
# @see .extract_pids_from
def self.handle_signin_for!(user:, authorize_until: 1.day.from_now, work_pid: nil, scope: nil, revoke_expirations_before: Time.zone.now)
Rails.logger.info("#{self.class}.#{__method__} granting authorization to work_pid: #{work_pid.inspect} and scope: #{scope.inspect}.")

pids = Array.wrap(work_pid) + extract_pids_from(scope: scope)

# Maybe we get multiple pids; let's handle that accordingly
Array.wrap(work_pid).each do |pid|
pids.each do |pid|
begin
authorize!(user: user, work_pid: pid, expires_at: authorize_until)
rescue WorkNotFoundError
Rails.logger.info("Unable to find work_pid of #{pid.inspect}.")
end
end

# We re-authorized the above work_pid, so it should not be in this query.
# We re-authorized the above pids, so it should not be in this query.
where("user_id = :user_id AND expires_at <= :expires_at", user_id: user.id, expires_at: revoke_expirations_before).pluck(:work_pid).each do |pid|
revoke!(user: user, work_pid: pid)
end
end

##
# A regular expression that identifies the :work_pid for Hyrax work.
#
# @see .extract_pids_from
REGEXP_TO_MATCH_PID = %r{/concern/(?<work_type>[^\/]+)/(?<work_pid>[^\?]+)(?<query_param>\?.*)?}

##
# From the given :scope string extract an array of potential work_ids.
#
# @param scope [String]
# @param with_regexp [Regexp]
#
# @return [Array<String>] work pid(s) that we found in the given :scope.
def self.extract_pids_from(scope:, with_regexp: REGEXP_TO_MATCH_PID)
return [] if scope.blank?

scope.split(/\s+/).map do |scope_element|
match = with_regexp.match(scope_element)
match[:work_pid] if match
end.compact
end

##
# Grant the given :user permission to read the work associated with the given :work_pid.
#
Expand Down Expand Up @@ -91,4 +125,41 @@ def self.revoke!(user:, work_pid:)
end
end
# rubocop:enable Rails/FindBy

##
# This module is a controller mixin that assumes access to a `session` object.
#
# Via {#prepare_for_conditional_work_authorization!} provide the logic for storing a URL to later
# be available in an authorization request.
#
# @example
# class ApplicationController < ActionController::Base
# include WorkAuthorization::StoreUrlForScope
# end
module StoreUrlForScope
CDL_SESSION_KEY = 'cdl.requested_work_url'

##
# Responsible for storing in the session the URL that is a candidate for Controlled Digital
# Lending (CDL) authorization.
def prepare_for_conditional_work_authorization!
# Note: Accessing `stored_location_for` clears that location from the session; so we need to
# grab it and then set it again.
url = stored_location_for(:user)
store_location_for(:user, url)

# If this could be a controlled digital lending (CDL) work, let's not rely on the
# stored_location (e.g. something that is part of devise and the value being subject to change).
# Instead, let's set the OmniAuth::Strategies::OpenIDConnectDecorator::CDL_SESSION_KEY key in
# the session.
if WorkAuthorization::REGEXP_TO_MATCH_PID.match(url)
Rails.logger.info("Called #{self.class}##{__method__} to set session['#{WorkAuthorization::StoreUrlForScope::CDL_SESSION_KEY}'] to #{url.inspect}.")
session[WorkAuthorization::StoreUrlForScope::CDL_SESSION_KEY] = url
else
Rails.logger.info("Called #{self.class}##{__method__} with stored location of #{url.inspect}; which is not matching as a work URL.")
# We don't have work URL, let's obliterate this.
session.delete(WorkAuthorization::StoreUrlForScope::CDL_SESSION_KEY)
end
end
end
end
8 changes: 8 additions & 0 deletions app/views/hyrax/base/unauthorized.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<h1><%= t('.unauthorized') %></h1>
<% if respond_to?(:curation_concern) && curation_concern %>
<p><%= t('.is_private', type: curation_concern.human_readable_type.downcase ) %><p>
<p><%= t('.id', id: curation_concern.id) %>
<% else %>
<p><%= t('.page_is_private') %><p>
<p>You may need to <%= link_to "re-authorize to access this resource", '#' %>.<p>
<% end %>
56 changes: 56 additions & 0 deletions lib/omni_auth/strategies/open_id_connect_decorator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
module OmniAuth
module Strategies
##
# OVERRIDE to provide openid {#scope} based on the current session.
#
# @see https://github.com/scientist-softserv/palni-palci/issues/633
module OpenIDConnectDecorator
##
# In OmniAuth, the options are a tenant wide configuration. However, per
# the client's controlled digitial lending (CDL) system, in the options we
# use for authentication we must inclue the URL of the work that the
# authenticating user wants to access.
#
# @note In testing, when the scope did not include the sample noted in the
# {#requested_work_url} method, the openid provider would return the
# status 200 (e.g. Success) and a body "Failed to get response from
# patron API"
#
# @return [Hash<Symbol,Object>]
def options
# If we don't include this, we keep adding to the `options[:scope]`
return @decorated_options if defined? @decorated_options

# TODO: What if we are not in the reshare tenant but the tenant is using
# OpenID for authentication?
opts = super

opts[:scope] += [requested_work_url] unless opts[:scope].include?(requested_work_url)

Rails.logger.info("#{self.class}#options scope value is #{opts[:scope].inspect}")

@decorated_options = opts
end

##
# @return [String] The URL of the work that was requested by the
# authenticating user.
#
# @note {#session} method is `@env['rack.session']`
# @note {#env} method is the hash representation of the Rack environment.
# @note {#request} method is the {#env} as a Rack::Request object
#
# @note The following URL is known to be acceptable for the reshare.commons-archive.org tenant:
#
# https://reshare.palni-palci-staging.notch8.cloud/concern/cdls/74ebfc53-ee7c-4dc9-9dd7-693e4d840745
def requested_work_url
Rails.logger.info("#{self.class}#session['#{WorkAuthorization::StoreUrlForScope::CDL_SESSION_KEY}'] is #{session[WorkAuthorization::StoreUrlForScope::CDL_SESSION_KEY].inspect}")
Rails.logger.info("#{self.class}#params['scope'] is #{params['scope'].inspect}")
session[WorkAuthorization::StoreUrlForScope::CDL_SESSION_KEY] ||
Array.wrap(params['scope']).split(%r{\s+}).reject { |el| el.to_s == "openid" }.first
end
end
end
end

OmniAuth::Strategies::OpenIDConnect.prepend(OmniAuth::Strategies::OpenIDConnectDecorator)
30 changes: 30 additions & 0 deletions spec/models/work_authorization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,37 @@
let(:borrowing_user) { FactoryBot.create(:user) }
let(:ability) { ::Ability.new(borrowing_user) }

describe '.extract_pids_from' do
subject { described_class.extract_pids_from(scope: given_test_scope) }

[
["http://pals.hyku.test/concern/generic_works/f2af2a68-7c79-481b-815e-a91517e23761?locale=en openid", ["f2af2a68-7c79-481b-815e-a91517e23761"]],
["openid", []],
["http://pals.hyku.test/concern/generic_works/f2af2a68-7c79-481b-815e-a91517e23761?locale=en", ["f2af2a68-7c79-481b-815e-a91517e23761"]],
["http://pals.hyku.test/collection/f2af2a68-7c79-481b-815e-a91517e23761?locale=en", []],
[nil, []]
].each do |given_scope, expected_value|
context "with #{given_scope.inspect}" do
let(:given_test_scope) { given_scope }

it { is_expected.to match_array(expected_value) }
end
end
end

describe '.handle_signin_for!' do
context 'when given a work_pid and a scope' do
it 'will authorize the given work_pid and scope’s work' do
given_scope = "http://pals.hyku.test/concern/generic_works/#{other_work.id}?locale=en openid"

expect do
expect do
described_class.handle_signin_for!(user: borrowing_user, work_pid: work.id, scope: given_scope, authorize_until: 1.day.from_now)
end.to change { ::Ability.new(borrowing_user).can?(:read, work.id) }.from(false).to(true)
end.to change { ::Ability.new(borrowing_user).can?(:read, other_work.id) }.from(false).to(true)
end
end

context 'when given a work_pid' do
it 'will re-authorize the given work_pid and expire non-specified work_ids' do
described_class.authorize!(user: borrowing_user, work_pid: work.id, expires_at: 1.day.ago)
Expand Down
65 changes: 65 additions & 0 deletions spec/omni_auth/strategies/open_id_connect_decorator_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# frozen_string_literal: true

require 'spec_helper'

# rubocop:disable RSpec/MultipleDescribes
RSpec.describe OmniAuth::Strategies::OpenIDConnectDecorator do
let(:strategy) do
Class.new do
def initialize(options: {}, session: {})
@options = options
@session = session
end
attr_reader :options, :session

# Include this after the attr_reader :options to leverage super method
prepend OmniAuth::Strategies::OpenIDConnectDecorator
end
end

let(:requested_work_url) { "https://hello.world/something-special" }
let(:options) { { scope: [:openid] } }
let(:session) { { "cdl.requested_work_url" => requested_work_url } }
let(:instance) { strategy.new(options: options, session: session) }

describe '#options' do
subject { instance.options }

it "has a :scope key that appends the #requested_work_url" do
expect(subject.fetch(:scope)).to match_array([:openid, requested_work_url])
end
end

describe '#requested_work_url' do
subject { instance.requested_work_url }

it "fetches the 'cdl.requested_work_url' session" do
expect(subject).to eq(requested_work_url)
end

describe "when the 'cdl.requested_work_url' key is missing" do
let(:session) { {} }

it { is_expected.to be_nil }
end
end
end

RSpec.describe OmniAuth::Strategies::OpenIDConnect do
describe '#options method' do
subject(:options_method) { described_class.instance_method(:options) }

context 'source_location' do
subject { options_method.source_location }

it { is_expected.to match_array([Rails.root.join('lib', 'omni_auth', 'strategies', 'open_id_connect_decorator.rb').to_s, Integer]) }
end

context 'super_method' do
subject { options_method.super_method }

it { is_expected.not_to be_nil }
end
end
end
# rubocop:enable RSpec/MultipleDescribes

0 comments on commit dec2354

Please sign in to comment.