AP-611 raise 403 for patron lookup if active patron is not found

app/lib/error/alma_record_not_found_error.rb

@@ -0,0 +1,6 @@
+module Error
+  # Raised when Alma returns a not-found style response.
+  # Subclassing ActiveRecord::RecordNotFound preserves existing 404 handling.
+  class AlmaRecordNotFoundError < ActiveRecord::RecordNotFound
+  end
+end

app/models/alma/user.rb

@@ -42,6 +42,10 @@ def find_if_active(id)
         find_if_exists(id).tap do |rec|
           return nil unless rec && rec.active?
         end
+      rescue Error::AlmaRecordNotFoundError => e
+        return if e.message.include?('Alma query failed with response: 404')
+
+        raise
       end
     end
 

app/services/alma_services.rb

@@ -50,14 +50,14 @@ def authenticate_alma_patron?(alma_user_id, alma_password)
       def get_user(alma_user_id)
         params = { view: 'full', expand: 'fees' }
         connection.get(user_uri_for(alma_user_id), params).tap do |res|
-          raise ActiveRecord::RecordNotFound, "Alma query failed with response: #{res.status}" unless res.status == 200
+          raise Error::AlmaRecordNotFoundError, "Alma query failed with response: #{res.status}" unless res.status == 200
         end
       end
 
       def save(alma_user_id, user)
         # noinspection RubyArgCount
         connection.put(user_uri_for(alma_user_id), user.to_json, { 'Content-Type' => 'application/json' }).tap do |res|
-          raise ActiveRecord::RecordNotFound, 'Failed to save.' unless res.status == 200
+          raise Error::AlmaRecordNotFoundError, 'Failed to save.' unless res.status == 200
         end
 
         'Saved user.' # TODO: what is this for?
@@ -85,7 +85,7 @@ def fee_uri_for(alma_id, fee_id)
 
       def fetch_all(alma_user_id)
         res = connection.get(fees_uri_for(alma_user_id))
-        raise ActiveRecord::RecordNotFound, 'No fees could be found.' unless res.status == 200
+        raise Error::AlmaRecordNotFoundError, 'No fees could be found.' unless res.status == 200
 
         JSON.parse(res.body)
       end
@@ -97,7 +97,7 @@ def credit(alma_user_id, pp_ref_number, fee)
         payment_uri = URIs.append(fee_uri_for(alma_user_id, fee.id), '?', URI.encode_www_form(params))
 
         connection.post(payment_uri).tap do |res|
-          raise ActiveRecord::RecordNotFound, "Alma query failed with response: #{res.status}" unless res.status == 200
+          raise Error::AlmaRecordNotFoundError, "Alma query failed with response: #{res.status}" unless res.status == 200
         end
       end
     end
@@ -119,14 +119,14 @@ def fetch_sets(env, offset = 0)
         }
 
         res = connection(env).get(URIs.append(alma_api_url, 'conf/sets'), params)
-        raise ActiveRecord::RecordNotFound, 'No item sets could be found..' unless res.status == 200
+        raise Error::AlmaRecordNotFoundError, 'No item sets could be found..' unless res.status == 200
 
         JSON.parse(res.body)
       end
 
       def fetch_set(env, id)
         res = connection(env).get(URIs.append(alma_api_url, "conf/sets/#{id}"))
-        raise ActiveRecord::RecordNotFound, "No set with ID #{id} found..." unless res.status == 200
+        raise Error::AlmaRecordNotFoundError, "No set with ID #{id} found..." unless res.status == 200
 
         JSON.parse(res.body)
       end
@@ -137,15 +137,15 @@ def fetch_members(set_id, env, offset = 0)
           limit: 100
         }
         res = connection(env).get(URIs.append(alma_api_url, "conf/sets/#{set_id}/members"), params)
-        raise ActiveRecord::RecordNotFound, 'No item sets could be found.' unless res.status == 200
+        raise Error::AlmaRecordNotFoundError, 'No item sets could be found.' unless res.status == 200
 
         JSON.parse(res.body)
       end
 
       def fetch_item(env, mms_id, holding_id, item_pid)
         uri = URIs.append(alma_api_url, "bibs/#{mms_id}/holdings/#{holding_id}/items/#{item_pid}")
         res = connection(env).get(uri)
-        raise ActiveRecord::RecordNotFound, 'Item could be found.' unless res.status == 200
+        raise Error::AlmaRecordNotFoundError, 'Item could be found.' unless res.status == 200
 
         JSON.parse(res.body)
       end

app/views/application/patron_not_found_error.html.erb

@@ -1,2 +1,2 @@
 <h1>Forbidden</h1>
-<p>Your patron record cannot be found. Please contact the <a href="https://www.lib.berkeley.edu/find/privileges-desk">Privileges Desk</a>.</p>
+<p>This form is only available to active library patrons. Please contact the <a href="https://www.lib.berkeley.edu/find/privileges-desk">Privileges Desk</a>.</p>

spec/forms_helper.rb

@@ -56,11 +56,23 @@ def show_path(id)
     with_patron_login(patron_id) do
       get new_form_path
       expect(response).to have_http_status(:forbidden)
-      expected_msg = /Your patron record cannot be found/
+      expected_msg = /This form is only available to active library patrons/
       expect(response.body).to match(expected_msg)
     end
   end
 
+  it 'forbids users when active patron lookup returns Alma 404' do
+    patron_id = Alma::Type.sample_id_for(allowed_patron_types.first)
+    allow(AlmaServices::Patron).to receive(:get_user).with(patron_id)
+      .and_raise(Error::AlmaRecordNotFoundError,
+                 'Alma query failed with response: 404')
+
+    with_patron_login(patron_id) do
+      get new_form_path
+      expect(response).to have_http_status(:forbidden)
+    end
+  end
+
   describe 'with a valid user' do
     attr_reader :patron_id
     attr_reader :patron

spec/models/user_spec.rb

@@ -235,4 +235,23 @@
     end
   end
 
+  describe :primary_patron_record do
+    it 'returns nil when active patron lookup returns nil' do
+      user = User.new(uid: 'does_not_exist')
+      allow(Alma::User).to receive(:find_if_active).with('does_not_exist')
+        .and_return(nil)
+
+      expect(user.primary_patron_record).to be_nil
+    end
+
+    it 're-raises active patron lookup errors' do
+      user = User.new(uid: 'fake_uid')
+      allow(Alma::User).to receive(:find_if_active).with('fake_uid')
+        .and_raise(Error::AlmaRecordNotFoundError,
+                   'Alma query failed with response: 500')
+
+      expect { user.primary_patron_record }.to raise_error(Error::AlmaRecordNotFoundError)
+    end
+  end
+
 end