Merge pull request #1479 from sul-dlss/auth-check-cleanup

remove legacy auth check route, tests, controllers

Author: jcoyne

app/controllers/media_controller.rb

@@ -5,12 +5,6 @@
 class MediaController < ApplicationController
   skip_forgery_protection
 
-  before_action :load_media
-
-  rescue_from ActionController::MissingFile do
-    render plain: 'File not found', status: :not_found
-  end
-
   def verify_token
     # the media service calling verify_token provides the end-user IP address,
     # as we care about the (user) IP address that made a request to the media service with the
@@ -22,44 +16,12 @@ def verify_token
     end
   end
 
-  # jsonp response
-  def auth_check
-    # IE 11 and Edge on Windows 10 doesn't request the correct format. So just hardcode
-    # JSON as the return format since that's what we always do.
-    render json: hash_for_auth_check, callback: allowed_params[:callback]
-  end
-
   private
 
   def allowed_params
     params.permit(:action, :callback, :id, :file_name, :format, :stacks_token, :user_ip)
   end
 
-  def hash_for_auth_check
-    if can? :stream, current_media
-      # we use IP from which request originated -- we want the end user IP, not
-      #   a service on the user's behalf (load-balancer, etc.)
-      token = URI.encode_www_form_component(current_media.encrypted_token(ip: request.remote_ip))
-      {
-        status: :success,
-        token:,
-        access_restrictions: {
-          stanford_restricted: current_media.stanford_restricted?,
-          restricted_by_location: current_media.restricted_by_location?,
-          embargoed: current_media.embargoed?,
-          embargo_release_date: current_media.embargo_release_date
-        }
-      }
-    else
-      MediaAuthenticationJson.new(
-        user: current_user,
-        ability: current_ability,
-        media: current_media,
-        auth_url: iiif_auth_api_url
-      )
-    end
-  end
-
   def id
     allowed_params[:id]
   end
@@ -68,18 +30,6 @@ def file_name
     allowed_params[:file_name]
   end
 
-  def load_media
-    @media ||= StacksMediaStream.new(stacks_file:)
-  end
-
-  def stacks_file
-    StacksFile.new(file_name: params[:file_name], cocina: Cocina.find(params[:id]))
-  end
-
-  def current_media
-    @media
-  end
-
   def token_valid?(token, expected_id, expected_file_name, expected_user_ip)
     StacksMediaToken.verify_encrypted_token? token, expected_id, expected_file_name, expected_user_ip
   end

config/routes.rb

@@ -25,16 +25,11 @@
     end
   end
 
-  if Settings.features.streaming_media
-    # stream file_name must include format extension, eg .../oo000oo0000.mp4/verify_token
-    #  other dots do not need to be URL encoded (see media routing specs)
-    constraints id: druid_regex, file_name: %r{[^/]+\.\w+} do
-      get '/media/:id/:file_name/verify_token' => 'media#verify_token'
-      get '/media/:id/:file_name/auth_check' => 'media#auth_check'
-
-      get '/media/druid::id/:file_name/verify_token' => 'media#verify_token'
-      get '/media/druid::id/:file_name/auth_check' => 'media#auth_check'
-    end
+  # stream file_name must include format extension, eg .../oo000oo0000.mp4/verify_token
+  #  other dots do not need to be URL encoded (see media routing specs)
+  constraints id: druid_regex, file_name: %r{[^/]+\.\w+} do
+    get '/media/:id/:file_name/verify_token' => 'media#verify_token'
+    get '/media/druid::id/:file_name/verify_token' => 'media#verify_token'
   end
 
   root 'stacks#index'

config/settings.yml

@@ -1,5 +1,4 @@
 features:
-  streaming_media: true
   metrics: false
   wowza_token: false
 

spec/controllers/media_controller_spec.rb

@@ -3,31 +3,6 @@
 require 'rails_helper'
 
 RSpec.describe MediaController do
-  before do
-    allow(Cocina).to receive(:find).and_return(Cocina.new(public_json))
-  end
-
-  let(:public_json) do
-    {
-      'structural' => {
-        'contains' => [
-          {
-            'structural' => {
-              'contains' => [
-                {
-                  'filename' => 'bb582xs1304_sl.mp4',
-                  'access' => {
-                    'view' => 'world',
-                    'download' => 'world'
-                  }
-                }
-              ]
-            }
-          }
-        ]
-      }
-    }
-  end
   let(:video) { StacksMediaStream.new(id: 'bb582xs1304', file_name: 'bb582xs1304_sl', format: 'mp4') }
 
   describe '#verify_token' do
@@ -104,47 +79,4 @@
       expect(controller.send(:token_valid?, 'stacks_token', 'id', 'file_name', 'ip_addr')).to be true
     end
   end
-
-  describe '#auth_check' do
-    let(:id) { 'bd786fy6312' }
-    let(:file_name) { 'some_file.mp4' }
-
-    it 'returns JSON from hash_for_auth_check' do
-      test_hash = { foo: :bar }
-      expect(controller).to receive(:hash_for_auth_check).and_return(test_hash)
-      get :auth_check, params: { id:, file_name:, format: :js }
-      body = response.parsed_body
-      expect(body).to eq('foo' => 'bar')
-    end
-
-    context 'success' do
-      before do
-        # We could be more integration-y and instead e.g. stub_request(:get, "https://purl.stanford.edu/bd786fy6312.json").to_return(...).
-        # But the StacksMediaStream code (and the metadata fetching/parsing code it uses) that'd be exercised by that approach is already
-        # tested elsewhere. This approach is a bit more readable, and less brittle since it doesn't break the StacksMediaStream abstraction.
-        stacks_media_stream = instance_double(StacksMediaStream, stanford_restricted?: false, restricted_by_location?: false,
-                                                                 embargoed?: false, embargo_release_date: nil,
-                                                                 encrypted_token: 'sekret-token')
-        allow(controller).to receive_messages(can?: true, current_media: stacks_media_stream)
-      end
-
-      it 'returns json that indicates a successful auth check (including token)' do
-        get :auth_check, params: { id:, file_name:, format: :js }
-        body = response.parsed_body
-        expect(body['status']).to eq 'success'
-        expect(body['token']).to eq 'sekret-token'
-      end
-
-      it 'returns info about applicable access restrictions' do
-        get :auth_check, params: { id:, file_name:, format: :js }
-        body = response.parsed_body
-        expect(body['access_restrictions']).to eq({
-                                                    'stanford_restricted' => false,
-                                                    'restricted_by_location' => false,
-                                                    'embargoed' => false,
-                                                    'embargo_release_date' => nil
-                                                  })
-      end
-    end
-  end
 end