From 3e71597c5b43a6447342e4981eb01529af7eebe2 Mon Sep 17 00:00:00 2001
From: Justin Coyne <jcoyne@justincoyne.com>
Date: Sat, 4 Nov 2023 10:41:01 -0500
Subject: [PATCH] StacksMediaStream is never authorized for read or download

So we don't need to grant these permissions in the Ability class
---
 app/models/ability.rb          | 28 ++++++++++++-------------
 spec/abilities/ability_spec.rb | 37 ----------------------------------
 2 files changed, 13 insertions(+), 52 deletions(-)

diff --git a/app/models/ability.rb b/app/models/ability.rb
index eb4daba3..aeb2a93d 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -41,28 +41,29 @@ def initialize(user)
     # NOTE: the below ability definitions which reference StacksFile also implicitly
     # cover StacksImage and StacksMediaStream, and any other subclasses of StacksFile.
 
-    models = [StacksFile, StacksImage, StacksMediaStream]
+    downloadable_models = [StacksFile, StacksImage]
+    access_models = downloadable_models + [StacksMediaStream]
 
-    can :download, models do |f|
+    can :download, downloadable_models do |f|
       value, rule = f.rights.world_rights_for_file f.file_name
 
       value && (rule.nil? || rule != Dor::RightsAuth::NO_DOWNLOAD_RULE)
     end
 
-    can [:access], models do |f|
+    can [:access], access_models do |f|
       value, _rule = f.rights.world_rights_for_file f.file_name
 
       value
     end
 
     if user.stanford?
-      can :download, models do |f|
+      can :download, downloadable_models do |f|
         value, rule = f.rights.stanford_only_rights_for_file f.file_name
 
         value && (rule.nil? || rule != Dor::RightsAuth::NO_DOWNLOAD_RULE)
       end
 
-      can [:access], models do |f|
+      can [:access], access_models do |f|
         value, _rule = f.rights.stanford_only_rights_for_file f.file_name
 
         value
@@ -70,13 +71,13 @@ def initialize(user)
     end
 
     if user.app_user?
-      can :download, models do |f|
+      can :download, downloadable_models do |f|
         value, rule = f.rights.agent_rights_for_file f.file_name, user.id
 
         value && (rule.nil? || rule != Dor::RightsAuth::NO_DOWNLOAD_RULE)
       end
 
-      can [:access], models do |f|
+      can [:access], access_models do |f|
         value, _rule = f.rights.agent_rights_for_file f.file_name, user.id
 
         value
@@ -84,14 +85,14 @@ def initialize(user)
     end
 
     if user.locations.present?
-      can :download, models do |f|
+      can :download, downloadable_models do |f|
         user.locations.any? do |location|
           value, rule = f.rights.location_rights_for_file(f.file_name, location)
           value && (rule.nil? || rule != Dor::RightsAuth::NO_DOWNLOAD_RULE)
         end
       end
 
-      can [:access], models do |f|
+      can [:access], access_models do |f|
         user.locations.any? do |location|
           value, _rule = f.rights.location_rights_for_file(f.file_name, location)
           value
@@ -105,7 +106,7 @@ def initialize(user)
       #   ...
       # end
 
-      can [:access], models do |f|
+      can [:access], access_models do |f|
         value, _rule = f.rights.cdl_rights_for_file(f.file_name)
         next unless value
 
@@ -125,7 +126,7 @@ def initialize(user)
       (projection.tile? || projection.thumbnail?) && can?(:access, projection.image)
     end
 
-    can [:access], Projection do |projection|
+    can :access, Projection do |projection|
       can?(:access, projection.image)
     end
 
@@ -135,10 +136,7 @@ def initialize(user)
       projection.thumbnail? && projection.object_thumbnail?
     end
 
-    can :stream, StacksMediaStream do |f|
-      can? :access, f
-    end
-
+    alias_action :stream, to: :access
     can :read_metadata, StacksImage
   end
 end
diff --git a/spec/abilities/ability_spec.rb b/spec/abilities/ability_spec.rb
index 62c4d7d4..6961d83a 100644
--- a/spec/abilities/ability_spec.rb
+++ b/spec/abilities/ability_spec.rb
@@ -60,7 +60,6 @@
       end
       it { is_expected.to be_able_to(:download, file) }
       it { is_expected.to be_able_to(:download, image) }
-      it { is_expected.to be_able_to(:download, media) }
       it { is_expected.to be_able_to(:read, tile) }
       it { is_expected.to be_able_to(:stream, media) }
       it { is_expected.to be_able_to(:access, file) }
@@ -84,7 +83,6 @@
       end
       it { is_expected.to be_able_to(:download, file) }
       it { is_expected.to be_able_to(:download, image) }
-      it { is_expected.to be_able_to(:download, media) }
       it { is_expected.to be_able_to(:read, tile) }
       it { is_expected.to be_able_to(:stream, media) }
       it { is_expected.to be_able_to(:access, file) }
@@ -108,7 +106,6 @@
       end
       it { is_expected.not_to be_able_to(:download, file) }
       it { is_expected.not_to be_able_to(:download, image) }
-      it { is_expected.not_to be_able_to(:download, media) }
       it { is_expected.not_to be_able_to(:read, big_image) }
       it { is_expected.to be_able_to(:read, tile) }
       it { is_expected.to be_able_to(:stream, media) }
@@ -133,7 +130,6 @@
       end
       it { is_expected.not_to be_able_to(:download, file) }
       it { is_expected.not_to be_able_to(:download, image) }
-      it { is_expected.not_to be_able_to(:download, media) }
       it { is_expected.not_to be_able_to(:read, tile) }
       it { is_expected.not_to be_able_to(:stream, media) }
       it { is_expected.not_to be_able_to(:access, file) }
@@ -160,7 +156,6 @@
       end
       it { is_expected.to be_able_to(:download, file) }
       it { is_expected.to be_able_to(:download, image) }
-      it { is_expected.to be_able_to(:download, media) }
       it { is_expected.to be_able_to(:read, tile) }
       it { is_expected.to be_able_to(:stream, media) }
       it { is_expected.to be_able_to(:access, file) }
@@ -183,7 +178,6 @@
       end
       it { is_expected.not_to be_able_to(:download, file) }
       it { is_expected.not_to be_able_to(:download, image) }
-      it { is_expected.not_to be_able_to(:download, media) }
       it { is_expected.not_to be_able_to(:read, tile) }
       it { is_expected.not_to be_able_to(:stream, media) }
       it { is_expected.not_to be_able_to(:access, file) }
@@ -251,7 +245,6 @@
       end
       it { is_expected.not_to be_able_to(:download, file) }
       it { is_expected.not_to be_able_to(:download, image) }
-      it { is_expected.not_to be_able_to(:download, media) }
       it { is_expected.not_to be_able_to(:read, tile) }
       it { is_expected.not_to be_able_to(:stream, media) }
       it { is_expected.not_to be_able_to(:access, file) }
@@ -295,7 +288,6 @@
       let(:user) { User.new(ip_address: 'ip.address2') }
       it { is_expected.to be_able_to(:download, file) }
       it { is_expected.to be_able_to(:download, image) }
-      it { is_expected.to be_able_to(:download, media) }
       it { is_expected.to be_able_to(:read, tile) }
       it { is_expected.to be_able_to(:stream, media) }
       it { is_expected.to be_able_to(:access, file) }
@@ -308,7 +300,6 @@
       let(:user) { User.new(ip_address: 'some.unknown.ip') }
       it { is_expected.not_to be_able_to(:download, file) }
       it { is_expected.not_to be_able_to(:download, image) }
-      it { is_expected.not_to be_able_to(:download, media) }
       it { is_expected.not_to be_able_to(:read, tile) }
       it { is_expected.not_to be_able_to(:stream, media) }
       it { is_expected.not_to be_able_to(:access, file) }
@@ -333,7 +324,6 @@
         let(:user) { User.new(ip_address: 'ip.address2') }
         it { is_expected.not_to be_able_to(:download, file) }
         it { is_expected.not_to be_able_to(:download, image) }
-        it { is_expected.not_to be_able_to(:download, media) }
         it { is_expected.to be_able_to(:read, tile) }
         it { is_expected.to be_able_to(:stream, media) }
         it { is_expected.to be_able_to(:access, file) }
@@ -345,7 +335,6 @@
         let(:user) { User.new(ip_address: 'some.unknown.ip') }
         it { is_expected.not_to be_able_to(:download, file) }
         it { is_expected.not_to be_able_to(:download, image) }
-        it { is_expected.not_to be_able_to(:download, media) }
         it { is_expected.not_to be_able_to(:read, tile) }
         it { is_expected.not_to be_able_to(:stream, media) }
         it { is_expected.not_to be_able_to(:access, file) }
@@ -373,7 +362,6 @@
       end
       it { is_expected.to be_able_to(:download, file) }
       it { is_expected.to be_able_to(:download, image) }
-      it { is_expected.to be_able_to(:download, media) }
       it { is_expected.to be_able_to(:read, tile) }
       it { is_expected.to be_able_to(:stream, media) }
       it { is_expected.to be_able_to(:access, file) }
@@ -397,7 +385,6 @@
       end
       it { is_expected.to be_able_to(:download, file) }
       it { is_expected.to be_able_to(:download, image) }
-      it { is_expected.to be_able_to(:download, media) }
       it { is_expected.to be_able_to(:read, tile) }
       it { is_expected.to be_able_to(:stream, media) }
       it { is_expected.to be_able_to(:access, file) }
@@ -421,7 +408,6 @@
       end
       it { is_expected.to be_able_to(:download, file) }
       it { is_expected.to be_able_to(:download, image) }
-      it { is_expected.to be_able_to(:download, media) }
       it { is_expected.to be_able_to(:read, tile) }
       it { is_expected.to be_able_to(:stream, media) }
       it { is_expected.to be_able_to(:access, file) }
@@ -444,7 +430,6 @@
       end
       it { is_expected.to be_able_to(:download, file) }
       it { is_expected.to be_able_to(:download, image) }
-      it { is_expected.to be_able_to(:download, media) }
       it { is_expected.to be_able_to(:read, tile) }
       it { is_expected.to be_able_to(:stream, media) }
       it { is_expected.to be_able_to(:access, file) }
@@ -467,7 +452,6 @@
       end
       it { is_expected.not_to be_able_to(:download, file) }
       it { is_expected.not_to be_able_to(:download, image) }
-      it { is_expected.not_to be_able_to(:download, media) }
       it { is_expected.to be_able_to(:read, tile) }
       it { is_expected.to be_able_to(:stream, media) }
       it { is_expected.to be_able_to(:access, file) }
@@ -492,7 +476,6 @@
       end
       it { is_expected.to be_able_to(:download, file) }
       it { is_expected.to be_able_to(:download, image) }
-      it { is_expected.to be_able_to(:download, media) }
       it { is_expected.to be_able_to(:read, tile) }
       it { is_expected.to be_able_to(:stream, media) }
       it { is_expected.to be_able_to(:access, file) }
@@ -515,7 +498,6 @@
       end
       it { is_expected.not_to be_able_to(:download, file) }
       it { is_expected.not_to be_able_to(:download, image) }
-      it { is_expected.not_to be_able_to(:download, media) }
       it { is_expected.not_to be_able_to(:read, tile) }
       it { is_expected.not_to be_able_to(:stream, media) }
       it { is_expected.not_to be_able_to(:access, file) }
@@ -538,7 +520,6 @@
       end
       it { is_expected.not_to be_able_to(:download, file) }
       it { is_expected.not_to be_able_to(:download, image) }
-      it { is_expected.not_to be_able_to(:download, media) }
       it { is_expected.not_to be_able_to(:read, tile) }
       it { is_expected.not_to be_able_to(:stream, media) }
       it { is_expected.not_to be_able_to(:access, file) }
@@ -561,7 +542,6 @@
       end
       it { is_expected.not_to be_able_to(:download, file) }
       it { is_expected.not_to be_able_to(:download, image) }
-      it { is_expected.not_to be_able_to(:download, media) }
       it { is_expected.to be_able_to(:read, tile) }
       it { is_expected.to be_able_to(:stream, media) }
       it { is_expected.to be_able_to(:access, file) }
@@ -593,7 +573,6 @@
 
         it { is_expected.to be_able_to(:download, file) }
         it { is_expected.to be_able_to(:download, image) }
-        it { is_expected.to be_able_to(:download, media) }
         it { is_expected.to be_able_to(:read, tile) }
         it { is_expected.to be_able_to(:stream, media) }
         it { is_expected.to be_able_to(:access, file) }
@@ -607,7 +586,6 @@
 
         it { is_expected.not_to be_able_to(:download, file) }
         it { is_expected.not_to be_able_to(:download, image) }
-        it { is_expected.not_to be_able_to(:download, media) }
         it { is_expected.to be_able_to(:read, tile) }
         it { is_expected.to be_able_to(:stream, media) }
         it { is_expected.to be_able_to(:access, file) }
@@ -623,7 +601,6 @@
 
         it { is_expected.to be_able_to(:download, file) }
         it { is_expected.to be_able_to(:download, image) }
-        it { is_expected.to be_able_to(:download, media) }
         it { is_expected.to be_able_to(:read, tile) }
         it { is_expected.to be_able_to(:stream, media) }
         it { is_expected.to be_able_to(:access, file) }
@@ -639,7 +616,6 @@
 
         it { is_expected.not_to be_able_to(:download, file) }
         it { is_expected.not_to be_able_to(:download, image) }
-        it { is_expected.not_to be_able_to(:download, media) }
         it { is_expected.to be_able_to(:read, tile) }
         it { is_expected.to be_able_to(:stream, media) }
         it { is_expected.to be_able_to(:access, file) }
@@ -655,7 +631,6 @@
 
         it { is_expected.not_to be_able_to(:download, file) }
         it { is_expected.not_to be_able_to(:download, image) }
-        it { is_expected.not_to be_able_to(:download, media) }
         it { is_expected.not_to be_able_to(:read, tile) }
         it { is_expected.not_to be_able_to(:stream, media) }
         it { is_expected.not_to be_able_to(:access, file) }
@@ -686,7 +661,6 @@
 
         it { is_expected.not_to be_able_to(:download, file) }
         it { is_expected.not_to be_able_to(:download, image) }
-        it { is_expected.not_to be_able_to(:download, media) }
         it { is_expected.to be_able_to(:read, tile) }
         it { is_expected.to be_able_to(:stream, media) }
         it { is_expected.to be_able_to(:access, file) }
@@ -700,7 +674,6 @@
 
         it { is_expected.to be_able_to(:download, file) }
         it { is_expected.to be_able_to(:download, image) }
-        it { is_expected.to be_able_to(:download, media) }
         it { is_expected.to be_able_to(:read, tile) }
         it { is_expected.to be_able_to(:stream, media) }
         it { is_expected.to be_able_to(:access, file) }
@@ -714,7 +687,6 @@
 
         it { is_expected.not_to be_able_to(:download, file) }
         it { is_expected.not_to be_able_to(:download, image) }
-        it { is_expected.not_to be_able_to(:download, media) }
         it { is_expected.not_to be_able_to(:read, tile) }
         it { is_expected.not_to be_able_to(:stream, media) }
         it { is_expected.not_to be_able_to(:access, file) }
@@ -743,7 +715,6 @@
       context 'for an anonymous user' do
         it { is_expected.not_to be_able_to(:download, file) }
         it { is_expected.not_to be_able_to(:download, image) }
-        it { is_expected.not_to be_able_to(:download, media) }
         it { is_expected.to be_able_to(:read, tile) }
         it { is_expected.to be_able_to(:stream, media) }
         it { is_expected.to be_able_to(:access, file) }
@@ -757,7 +728,6 @@
 
         it { is_expected.to be_able_to(:download, file) }
         it { is_expected.to be_able_to(:download, image) }
-        it { is_expected.to be_able_to(:download, media) }
         it { is_expected.to be_able_to(:read, tile) }
         it { is_expected.to be_able_to(:stream, media) }
         it { is_expected.to be_able_to(:access, file) }
@@ -791,7 +761,6 @@
       context 'as an anonymous user' do
         it { is_expected.to be_able_to(:download, file) }
         it { is_expected.to be_able_to(:download, image) }
-        it { is_expected.not_to be_able_to(:download, media) }
         it { is_expected.to be_able_to(:read, tile) }
         it { is_expected.to be_able_to(:stream, media) }
         it { is_expected.to be_able_to(:access, file) }
@@ -826,7 +795,6 @@
 
         it { is_expected.to be_able_to(:download, file) }
         it { is_expected.not_to be_able_to(:download, image) }
-        it { is_expected.to be_able_to(:download, media) }
         it { is_expected.not_to be_able_to(:read, tile) }
         it { is_expected.to be_able_to(:stream, media) }
         it { is_expected.to be_able_to(:access, file) }
@@ -843,7 +811,6 @@
 
         it { is_expected.to be_able_to(:download, file) }
         it { is_expected.to be_able_to(:download, image) }
-        it { is_expected.to be_able_to(:download, media) }
         it { is_expected.to be_able_to(:read, tile) }
         it { is_expected.to be_able_to(:stream, media) }
         it { is_expected.to be_able_to(:access, file) }
@@ -856,7 +823,6 @@
       context 'as an anonymous user' do
         it { is_expected.not_to be_able_to(:download, file) }
         it { is_expected.not_to be_able_to(:download, image) }
-        it { is_expected.not_to be_able_to(:download, media) }
         it { is_expected.not_to be_able_to(:read, tile) }
         it { is_expected.not_to be_able_to(:stream, media) }
         it { is_expected.not_to be_able_to(:access, file) }
@@ -891,7 +857,6 @@
 
         it { is_expected.not_to be_able_to(:download, file) }
         it { is_expected.to be_able_to(:download, image) }
-        it { is_expected.to be_able_to(:download, media) }
         it { is_expected.to be_able_to(:read, tile) }
         it { is_expected.to be_able_to(:stream, media) }
         it { is_expected.not_to be_able_to(:access, file) }
@@ -905,7 +870,6 @@
 
         it { is_expected.not_to be_able_to(:download, file) }
         it { is_expected.not_to be_able_to(:download, image) }
-        it { is_expected.not_to be_able_to(:download, media) }
         it { is_expected.not_to be_able_to(:read, tile) }
         it { is_expected.not_to be_able_to(:stream, media) }
         it { is_expected.not_to be_able_to(:access, file) }
@@ -919,7 +883,6 @@
 
         it { is_expected.to be_able_to(:download, file) }
         it { is_expected.not_to be_able_to(:download, image) }
-        it { is_expected.not_to be_able_to(:download, media) }
         it { is_expected.not_to be_able_to(:read, tile) }
         it { is_expected.not_to be_able_to(:stream, media) }
         it { is_expected.to be_able_to(:access, file) }