Skip to content

Commit

Permalink
Refactor down the stack to make use of new batch stream token create/…
Browse files Browse the repository at this point in the history
…update method
  • Loading branch information
cjcolvar committed Nov 21, 2023
1 parent 85010d7 commit c456ebf
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 10 deletions.
7 changes: 2 additions & 5 deletions app/controllers/media_objects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -464,11 +464,8 @@ def manifest
@media_object = SpeedyAF::Proxy::MediaObject.find(params[:id])
authorize! :read, @media_object

master_files = master_file_presenters
canvas_presenters = master_files.collect do |mf|
stream_info = secure_streams(mf.stream_details, @media_object.id)
IiifCanvasPresenter.new(master_file: mf, stream_info: stream_info)
end
stream_info_hash = secure_stream_infos(master_file_presenters, @media_object.id)
canvas_presenters = master_file_presenters.collect { |mf| IiifCanvasPresenter.new(master_file: mf, stream_info: stream_info_hash[mf.id]) }
presenter = IiifManifestPresenter.new(media_object: @media_object, master_files: canvas_presenters, lending_enabled: lending_enabled?(@media_object))

manifest = IIIFManifest::V3::ManifestFactory.new(presenter).to_h
Expand Down
27 changes: 25 additions & 2 deletions app/helpers/security_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,34 @@ def secure_streams(stream_info, media_object_id)
stream_info
end

def add_stream_url(stream_info)
# Batch version of secure_streams
# media_object CDL check only happens once
# session tokens retrieved in batch then passed into add_stream_url
# Returns Hash[MasterFile.id, stream_info]
def secure_stream_infos(master_files, media_object_id)
stream_info_hash = {}
if not_checked_out?(media_object_id)
master_files.each { |mf| stream_info_hash[mf.id] = mf.stream_details }
else
stream_tokens = StreamToken.get_session_tokens_for(session: session, targets: master_files.map(&:id))
stream_token_hash = stream_tokens.pluck(:target, :token).to_h
master_files.each { |mf| stream_info_hash[mf.id] = secure_stream_info(mf.stream_details, stream_token_hash[mf.id]) }
end
stream_info_hash
end

# Same as secure_streams except without CDL checking
def secure_stream_info(stream_info, token)
add_stream_url(stream_info, token: token)
stream_info
end

# Optional token kwarg used if passed in
def add_stream_url(stream_info, token: nil)
add_stream_cookies(id: stream_info[:id])
[:stream_hls].each do |protocol|
stream_info[protocol].each do |quality|
quality[:url] = SecurityHandler.secure_url(quality[:url], session: session, target: stream_info[:id], protocol: protocol)
quality[:url] = SecurityHandler.secure_url(quality[:url], session: session, target: stream_info[:id], protocol: protocol, token: token)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/services/security_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def rewrite_url(url, context)
end
else
session = context[:session] || { media_token: nil }
token = StreamToken.find_or_create_session_token(session, context[:target])
token = context[:token] || StreamToken.find_or_create_session_token(session, context[:target])
"#{url}?token=#{token}"
end
end
Expand Down
3 changes: 2 additions & 1 deletion spec/controllers/media_objects_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1840,7 +1840,8 @@

describe '#manifest' do
context 'read from solr' do
let!(:media_object) { FactoryBot.create(:published_media_object, :with_master_file, visibility: 'public') }
let!(:master_file) { FactoryBot.create(:master_file, :with_derivative, media_object: media_object) }
let!(:media_object) { FactoryBot.create(:published_media_object, visibility: 'public') }
it 'should not read from fedora' do
perform_enqueued_jobs(only: MediaObjectIndexingJob)
WebMock.reset_executed_requests!
Expand Down
57 changes: 56 additions & 1 deletion spec/helpers/security_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@

context 'when using non-AWS streaming server' do
describe '#add_stream_cookies' do
it 'adds security tokens to cookies' do
it 'does not add security tokens to cookies' do
expect { helper.add_stream_cookies(stream_info) }.not_to change { controller.cookies.sum {|k,v| 1} }
end
end
Expand Down Expand Up @@ -158,5 +158,60 @@
end
end
end

describe '#secure_stream_infos' do
let(:master_file) { FactoryBot.create(:master_file, :with_derivative, media_object: media_object) }
let(:token) { 'dcba-4321' }
let(:secure_url_with_token) { secure_url + "?token=#{token}" }

before do
allow(SecurityHandler).to receive(:secure_url).with(String, token: token).and_return(secure_url_with_token)
end

context 'controlled digital lending is disabled' do
before { allow(Settings.controlled_digital_lending).to receive(:enable).and_return(false) }

it 'rewrites urls in the stream_infos' do
stream_info_hash = helper.secure_stream_infos([master_file], media_object.id)
stream_info = stream_info_hash[master_file.id]
[:stream_hls].each do |protocol|
stream_info[protocol].each do |quality|
expect(quality[:url]).to eq secure_url
end
end
end
end

context 'controlled digital lending is enabled' do
before { allow(Settings.controlled_digital_lending).to receive(:enable).and_return(true) }
before { allow(Settings.controlled_digital_lending).to receive(:collections_enabled).and_return(true) }

context 'the user has the item checked out' do
before { FactoryBot.create(:checkout, media_object_id: media_object.id, user_id: user.id)}

it 'rewrites urls in the stream_infos' do
stream_info_hash = helper.secure_stream_infos([master_file], media_object.id)
stream_info = stream_info_hash[master_file.id]
[:stream_hls].each do |protocol|
stream_info[protocol].each do |quality|
expect(quality[:url]).to eq secure_url
end
end
end
end

context 'the user does not have the item checked out' do
it 'does not rewrite urls in the stream_info' do
stream_info_hash = helper.secure_stream_infos([master_file], media_object.id)
stream_info = stream_info_hash[master_file.id]
[:stream_hls].each do |protocol|
stream_info[protocol].each do |quality|
expect(quality[:url]).not_to eq secure_url
end
end
end
end
end
end
end
end
8 changes: 8 additions & 0 deletions spec/services/security_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@
it 'adds a StreamToken param' do
expect(subject.rewrite_url(url, context)).to start_with "http://example.com/streaming/id?token="
end

context 'when token provided' do
let(:context) {{ session: {}, target: 'abcd1234', protocol: :stream_hls, token: 'dcba-4321' }}

it 'adds token param' do
expect(subject.rewrite_url(url, context)).to eq "http://example.com/streaming/id?token=dcba-4321"
end
end
end
end

Expand Down

0 comments on commit c456ebf

Please sign in to comment.