From 00643e04c09d975465ba9939b90b04718da65112 Mon Sep 17 00:00:00 2001 From: Ben Pennell Date: Wed, 7 Dec 2022 12:56:36 -0500 Subject: [PATCH] 401 if an unauthorized user attempts to download a workflow restricted 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 --- app/controllers/hyrax/downloads_controller.rb | 17 ++++++++++++++++- .../hyrax/downloads_controller_spec.rb | 17 ++++++++++++----- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/app/controllers/hyrax/downloads_controller.rb b/app/controllers/hyrax/downloads_controller.rb index 6edfdfe1f7..bc816c20a6 100644 --- a/app/controllers/hyrax/downloads_controller.rb +++ b/app/controllers/hyrax/downloads_controller.rb @@ -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 @@ -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 diff --git a/spec/controllers/hyrax/downloads_controller_spec.rb b/spec/controllers/hyrax/downloads_controller_spec.rb index d00debdd64..8a65565388 100644 --- a/spec/controllers/hyrax/downloads_controller_spec.rb +++ b/spec/controllers/hyrax/downloads_controller_spec.rb @@ -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') } @@ -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