Skip to content

Commit

Permalink
401 if an unauthorized user attempts to download a workflow restricte…
Browse files Browse the repository at this point in the history
…d file (#5921)

* Return 401s if an unauthorized user attempts to download a workflow restricted file

* Add method for finding parent

* Removed expectation that thumbnail be retrieved without contacting fedora, since this seems to be required in order to verify whether it is in a workflow

* Use ActiveFedora::Base.where to find the FileSet to save trip to fedora

* Move test into context where it should have permission, and remove no fedora calls test

* Switch back to using valkyrie friendly lookup of fileset and parent

* Make the lines super long to make rubocop happy
  • Loading branch information
bbpennel committed Dec 7, 2022
1 parent 6c0e57f commit 00643e0
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 6 deletions.
17 changes: 16 additions & 1 deletion app/controllers/hyrax/downloads_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module Hyrax
class DownloadsController < ApplicationController
include Hydra::Controller::DownloadBehavior
include Hyrax::LocalFileDownloadsControllerBehavior
include Hyrax::WorkflowsHelper # Provides #workflow_restriction?

def self.default_content_path
:original_file
Expand Down Expand Up @@ -37,12 +38,26 @@ def derivative_download_options
{ type: mime_type_for(file), disposition: 'inline' }
end

def file_set_parent(file_set_id)
file_set = Hyrax.query_service.find_by_alternate_identifier(alternate_identifier: file_set_id, use_valkyrie: Hyrax.config.use_valkyrie?)
@parent ||=
case file_set
when Hyrax::Resource
Hyrax.query_service.find_parents(resource: file_set).first
else
file_set.parent
end
end

# Customize the :read ability in your Ability class, or override this method.
# Hydra::Ability#download_permissions can't be used in this case because it assumes
# that files are in a LDP basic container, and thus, included in the asset's uri.
def authorize_download!
authorize! :download, params[asset_param_key]
rescue CanCan::AccessDenied
# Deny access if the work containing this file is restricted by a workflow
return unless workflow_restriction?(file_set_parent(params[asset_param_key]), ability: current_ability)
raise Hyrax::WorkflowAuthorizationException
rescue CanCan::AccessDenied, Hyrax::WorkflowAuthorizationException
unauthorized_image = Rails.root.join("app", "assets", "images", "unauthorized.png")
send_file unauthorized_image, status: :unauthorized
end
Expand Down
17 changes: 12 additions & 5 deletions spec/controllers/hyrax/downloads_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,18 @@
expect(response.body).to eq file_set.original_file.content
end

context 'when restricted by workflow' do
before do
allow(subject).to receive(:workflow_restriction?).and_return(true)
end

it 'returns :unauthorized status with image content' do
get :show, params: { id: file_set.to_param }
expect(response).to have_http_status(:unauthorized)
expect(response.content_type).to eq 'image/png'
end
end

context "with an alternative file" do
context "that is persisted" do
let(:file) { File.open(fixture_path + '/world.png', 'rb') }
Expand All @@ -70,11 +82,6 @@
expect(response.headers['Accept-Ranges']).to eq "bytes"
end

it 'retrieves the thumbnail without contacting Fedora' do
expect(ActiveFedora::Base).not_to receive(:find).with(file_set.id)
get :show, params: { id: file_set, file: 'thumbnail' }
end

it 'sends 304 response when client has valid cached data' do
get :show, params: { id: file_set, file: 'thumbnail' }
expect(response).to have_http_status :success
Expand Down

0 comments on commit 00643e0

Please sign in to comment.