Skip to content

Commit

Permalink
Rename "Library ID" to "University ID"
Browse files Browse the repository at this point in the history
Connects to sul-dlss/SearchWorks#3773

This commit helps us move the access portfolio beyond using library barcode numbers, leaning instead on the university-assigned and -managed "university ID."
  • Loading branch information
mjgiarlo committed Feb 28, 2024
1 parent 4be4b37 commit 1727350
Show file tree
Hide file tree
Showing 13 changed files with 74 additions and 53 deletions.
12 changes: 7 additions & 5 deletions app/controllers/reset_pins_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,19 @@ def index; end
# a token for completing the reset
#
# Ignore errors indicating the patron wasn't found in the ILS; we don't want
# to leak information about presence/validity of library IDs
# to leak information about presence/validity of university IDs
#
# POST /reset_pin
def reset
suppress ActiveRecord::RecordNotFound do
patron = FolioClient.new.find_patron_by_barcode(library_id_param, patron_info: false)
patron = FolioClient.new.find_patron_by_university_id(university_id_param, patron_info: false)

ResetPinsMailer.with(patron:).reset_pin.deliver_now
end

flash[:success] = t 'mylibrary.reset_pin.success_html', library_id: params['library_id']
flash[:success] = t('mylibrary.reset_pin.success_html',
university_id: params['university_id'],
university_id_label: t('mylibrary.university_id.label'))
redirect_to login_path
end

Expand All @@ -48,8 +50,8 @@ def change

private

def library_id_param
params.require(:library_id)
def university_id_param
params.require(:university_id)
end

def change_pin_with_token_params
Expand Down
12 changes: 6 additions & 6 deletions app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,21 @@ def index
redirect_to summaries_url if current_user?
end

# Render a login form for Barcode + PIN users (Stanford single-sign-on are
# Render a login form for University ID + PIN users (Stanford single-sign-on are
# authenticated using a different route)
#
# GET /login
def form; end

# Handle login for Barcode + PIN users by authenticating them with the
# Handle login for University ID + PIN users by authenticating them with the
# ILS using the Warden configuration.
#
# GET /sessions/login_by_library_id
def login_by_library_id
if request.env['warden'].authenticate(:library_id)
# GET /sessions/login_by_university_id
def login_by_university_id
if request.env['warden'].authenticate(:university_id)
redirect_to summaries_url
else
redirect_to login_url, alert: t('mylibrary.sessions.login_by_library_id.alert')
redirect_to login_url, alert: t('mylibrary.sessions.login_by_university_id.alert')
end
end

Expand Down
26 changes: 14 additions & 12 deletions app/services/folio_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ def inspect
"#<#{self.class.name}:#{object_id} @base_url=\"#{base_url}\">"
end

def login(library_id, pin)
user_response = get_json('/users', params: { query: CqlQuery.new(barcode: library_id).to_query })
def login(university_id, pin)
user_response = get_json('/users', params: { query: CqlQuery.new(externalSystemId: university_id).to_query })
user = user_response.dig('users', 0)

return unless user && validate_patron_pin(user['id'], pin)
Expand All @@ -57,6 +57,18 @@ def user_info(user_id)
get_json("/users/#{CGI.escape(user_id)}")
end

# Look up a patron by university_id and return a Patron object
# If 'patron_info' is false, don't run the full patron info GraphQL query
def find_patron_by_university_id(university_id, patron_info: true)
response = get_json('/users', params: { query: CqlQuery.new(externalSystemId: university_id).to_query })

if (user = response.dig('users', 0))
return patron_info ? Folio::Patron.find(user['id']) : Folio::Patron.new({ 'user' => user })
end

raise ActiveRecord::RecordNotFound, "User with #{I18n.t('mylibrary.university_id.label')} #{university_id} not found"
end

# rubocop:disable Lint/UnusedMethodArgument
# FOLIO graphql call, compare to #patron_account
def patron_info(patron_key, item_details: {})
Expand Down Expand Up @@ -176,16 +188,6 @@ def change_pin(token, new_pin)
check_response(response, title: 'Assign pin', context: { user_id: patron_key })
end

# Look up a patron by barcode and return a Patron object
# If 'patron_info' is false, don't run the full patron info GraphQL query
def find_patron_by_barcode(barcode, patron_info: true)
response = get_json('/users', params: { query: CqlQuery.new(barcode:).to_query })
user = response.dig('users', 0)
raise ActiveRecord::RecordNotFound, "User with barcode #{barcode} not found" unless user

patron_info ? Folio::Patron.find(user['id']) : Folio::Patron.new({ 'user' => user })
end

# Mark all of a user's fines (accounts) as having been paid
# The payment will show as being made from the 'Online' service point
# rubocop:disable Metrics/MethodLength
Expand Down
4 changes: 2 additions & 2 deletions app/views/reset_pins/change_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<div class="form-group">
<%= label_tag :pin, 'New PIN' %>
<div class="input-group" data-showpassword="pin">
<%= password_field_tag :pin, nil, class: 'form-control', 'aria-describedby': 'libraryIdHelp', autofocus: true, autocomplete: 'off' %>
<%= password_field_tag :pin, nil, class: 'form-control', 'aria-describedby': 'new-pin-help', autofocus: true, autocomplete: 'off' %>
<div class="input-group-append">
<button class="btn btn-link" data-visibility disabled>
<%= sul_icon(:'sharp-visibility-24px') %>
Expand All @@ -22,7 +22,7 @@
</button>
</div>
</div>
<small id="libraryIdHelp" class="form-text text-muted">Your new PIN</small>
<small id="new-pin-help" class="form-text text-muted">Your new PIN</small>
</div>
<%= submit_tag 'Change PIN', class: 'btn btn-primary' %>
<%= link_to 'Cancel', root_url, class: 'btn btn-link' %>
Expand Down
6 changes: 3 additions & 3 deletions app/views/reset_pins/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<h1>Reset/Request PIN</h1>
<%= form_tag reset_pin_url, method: :post do %>
<div class="form-group">
<%= label_tag :library_id, 'Library ID' %>
<%= text_field_tag :library_id, nil, class: 'form-control', 'aria-describedby': 'libraryIdHelp', autofocus: true %>
<small id="libraryIdHelp" class="form-text text-muted">Last 10 digits above the barcode on your library card</small>
<%= label_tag :university_id, t('mylibrary.university_id.label') %>
<%= text_field_tag :university_id, nil, class: 'form-control', 'aria-describedby': 'university-id-help', autofocus: true %>
<small id="university-id-help" class="form-text text-muted"><%= t('mylibrary.university_id.help_text') %></small>
</div>

<%= submit_tag 'Reset/Request PIN', class: 'btn btn-primary' %>
Expand Down
8 changes: 4 additions & 4 deletions app/views/sessions/form.html.erb
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
<h1><%= proxy_login_header %></h1>
<%= form_tag login_by_library_id_url, method: :post do %>
<%= form_tag login_by_university_id_url, method: :post do %>

<div class="form-group">
<%= label_tag :library_id, 'Library ID' %>
<%= text_field_tag :library_id, nil, class: 'form-control col-8 col-sm-4', 'aria-describedby': 'libraryIdHelp', autofocus: true %>
<small id="libraryIdHelp" class="form-text text-muted">Last 10 digits above the barcode on your library card</small>
<%= label_tag :university_id, t('mylibrary.university_id.label') %>
<%= text_field_tag :university_id, nil, class: 'form-control col-8 col-sm-4', 'aria-describedby': 'university-id-help', autofocus: true %>
<small id="university-id-help" class="form-text text-muted"><%= t('mylibrary.university_id.help_text') %></small>
</div>

<div class="form-group">
Expand Down
19 changes: 15 additions & 4 deletions config/initializers/warden.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,27 @@ def uid
end
end

Warden::Strategies.add(:library_id) do
Warden::Strategies.add(:university_id) do
# Reviewing numbers input in the FOLIO "External System ID", it appears that
# we have user records with 8, 9 and 10 digits.
#
# * Core community users (who presumably would be accessing our applications
# using auth and NOT inputting an ID) have 8 digits in the FOLIO External
# System ID field
# * Courtesy card holders (the primary user group that we're trying to help
# gain access in sul-requests & My Account) have either 9 or 10 digits in
# FOLIO External System ID field.
def valid?
params['library_id'].present? && params['pin'].present?
params['university_id'].present? &&
params['university_id'].match?(/\d{8,10}/) &&
params['pin'].present?
end

def authenticate!
response = ApplicationController.ils_client_class.new.login(params['library_id'], params['pin'])
response = ApplicationController.ils_client_class.new.login(params['university_id'], params['pin'])

if response&.key?('patronKey') || response&.key?('id')
u = { username: params['library_id'], patron_key: response['patronKey'] || response['id'] }
u = { username: params['university_id'], patron_key: response['patronKey'] || response['id'] }
success!(u)
else
fail!('Could not log in')
Expand Down
7 changes: 5 additions & 2 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ en:
time_tomorrow: 'Tomorrow at %-l:%M%P'
hello: "Hello world"
mylibrary:
university_id:
label: 'University ID'
help_text: 'This is an 8- to 10-digit number that serves as a replacement for the Library ID that was previously utilized.'
renew_item:
success_html: <span class="font-weight-bold">Success!</span> "%{title}" was renewed.
error_html: <span class="font-weight-bold">Sorry!</span> Something went wrong and "%{title}" was not renewed.
Expand All @@ -64,7 +67,7 @@ en:
success_html: <span class="font-weight-bold">Success!</span> Your new PIN is ready to use.
invalid_token_html: <span class="font-weight-bold">Sorry!</span> That reset PIN link is invalid or expired. Enter your library ID to request a new link.
reset_pin:
success_html: <span class="font-weight-bold">Check your email!</span> A PIN reset link has been sent to the address associated with library ID %{library_id}
success_html: <span class="font-weight-bold">Check your email!</span> A PIN reset link has been sent to the address associated with %{university_id_label} %{university_id}
request_failed_html: <span class="font-weight-bold">Sorry!</span> Something went wrong, possibly due to a connection failure. Try again or contact us for help.
reset_pins_mailer:
reset_pin:
Expand All @@ -83,7 +86,7 @@ en:
request_failed_html: <span class="font-weight-bold">Sorry!</span> Something went wrong, possibly due to a connection failure. Please <a href="mailto:[email protected]">contact us</a> for help resolving your fines.
payment_failed_html: <span class="font-weight-bold">Sorry!</span> Payment failed. Please <a href="mailto:[email protected]">contact us</a> for help resolving your fines.
sessions:
login_by_library_id:
login_by_university_id:
alert: Unable to authenticate.
login_by_sunetid:
error_html: <p class="h3">Unable to log in. Your SUNet ID is not linked to a library account.</p>
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
get 'feedback' => 'feedback_forms#new'

get '/sessions/login_by_sunetid', to: 'sessions#login_by_sunetid', as: :login_by_sunetid
post '/sessions/login_by_library_id', to: 'sessions#login_by_library_id', as: :login_by_library_id
post '/sessions/login_by_university_id', to: 'sessions#login_by_university_id', as: :login_by_university_id
get '/login', to: 'sessions#form', as: :login
get '/logout', to: 'sessions#destroy', as: :logout

Expand Down
2 changes: 1 addition & 1 deletion config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ GLOBAL_ALERT: <%= true %>
borrow_direct_reshare:
enabled: false

# Adding Illiad
# Adding Illiad
sul_illiad: https://sul-illiad-test.stanford.edu/
illiad_api_key: 'AbC123'

Expand Down
10 changes: 6 additions & 4 deletions spec/controllers/reset_pins_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require 'rails_helper'

RSpec.describe ResetPinsController do
let(:mock_client) { instance_double(FolioClient, find_patron_by_barcode: patron, ping: true) }
let(:mock_client) { instance_double(FolioClient, find_patron_by_university_id: patron, ping: true) }
let(:patron) do
instance_double(Folio::Patron, display_name: 'Patron', barcode: 'PATRON', email: '[email protected]',
pin_reset_token: 'abcdef')
Expand Down Expand Up @@ -35,13 +35,15 @@
context 'with unauthenticated requests' do
describe '#reset' do
it 'sends the reset pin email' do
expect { post :reset, params: { library_id: '123456' } }.to change { ActionMailer::Base.deliveries.count }.by(1)
expect { post :reset, params: { university_id: '123456789' } }
.to change { ActionMailer::Base.deliveries.count }
.by(1)
end

it 'sets flash messages' do
post :reset, params: { library_id: '123456' }
post :reset, params: { university_id: '123456789' }

expect(flash[:success]).to match(/associated with library ID 123456/)
expect(flash[:success]).to match(/associated with University ID 123456789/)
end
end
end
Expand Down
15 changes: 8 additions & 7 deletions spec/controllers/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,30 +83,31 @@
end
end

describe 'POST login_by_library_id' do
describe 'POST login_by_university_id' do
context 'with a valid login' do
before do
allow(mock_client).to receive(:login).with('abc', '123').and_return('patronKey' => 1)
allow(mock_client).to receive(:login).with('01234567', '123').and_return('patronKey' => 1)
end

it 'logs in the user' do
post(:login_by_library_id, params: { library_id: 'abc', pin: '123' })
post(:login_by_university_id, params: { university_id: '01234567', pin: '123' })

expect(warden.user).to include username: 'abc', patron_key: 1
expect(warden.user).to include username: '01234567', patron_key: 1
end

it 'redirects the user to the summary page' do
expect(post(:login_by_library_id, params: { library_id: 'abc', pin: '123' })).to redirect_to summaries_url
expect(post(:login_by_university_id, params: { university_id: '01234567', pin: '123' }))
.to redirect_to summaries_url
end
end

context 'with an invalid login' do
it 'redirects failed requests back to the login page' do
expect(post(:login_by_library_id)).to redirect_to login_url
expect(post(:login_by_university_id)).to redirect_to login_url
end

it 'sets an alert' do
get(:login_by_library_id)
get(:login_by_university_id)
expect(flash[:alert]).to include('Unable to authenticate.')
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/features/reset_pin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require 'rails_helper'

RSpec.describe 'Reset Pin' do
let(:mock_client) { instance_double(FolioClient, find_patron_by_barcode: patron, ping: true, change_pin: nil) }
let(:mock_client) { instance_double(FolioClient, find_patron_by_university_id: patron, ping: true, change_pin: nil) }
let(:patron) do
instance_double(Folio::Patron, email: '[email protected]', display_name: 'J Doe', barcode: '123',
pin_reset_token: 'foo')
Expand All @@ -26,7 +26,7 @@

it 'allows user to reset pin' do
visit reset_pin_path
fill_in('library_id', with: '123456')
fill_in('university_id', with: '123456')
click_on 'Reset/Request PIN'
expect(page).to have_css '.flash_messages', text: 'Check your email!'
end
Expand Down

0 comments on commit 1727350

Please sign in to comment.