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."

- [ ] Figure out how to test this and roll it out.
  • Loading branch information
mjgiarlo committed Jan 29, 2024
1 parent f82549e commit 8d06c36
Show file tree
Hide file tree
Showing 43 changed files with 192 additions and 243 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ fake_ldap_attributes:
this will assign you the relevant LDAP attributes when you run a server using `REMOTE_USER=(your sunet id) rails s`.

### token encryption
there is a token encryption library that handles encrypting and decrypting tokens given to users who only submit a Name/Email or Library ID for identification purposes. to keep these tokens secure we require a secret and a salt configured of moderate complexity and randomness (`SecureRandom.hex(128)` can be useful). once configured, these keys (or the tokens generated in the app) **MUST NOT** change, otherwise the tokens that users have been given will no longer be valid.
there is a token encryption library that handles encrypting and decrypting tokens given to users who only submit a Name/Email or University ID for identification purposes. to keep these tokens secure we require a secret and a salt configured of moderate complexity and randomness (`SecureRandom.hex(128)` can be useful). once configured, these keys (or the tokens generated in the app) **MUST NOT** change, otherwise the tokens that users have been given will no longer be valid.
## testing
the test suite (with RuboCop style enforcement) will be run with the default rake task (also run in CI):
```sh
Expand Down
2 changes: 1 addition & 1 deletion app/assets/stylesheets/modules/forms.scss
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ hr {
display: none;
}

.with-library-id-error {
.with-university-id-error {
#sunetid-form {
display: none;
}
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/concerns/request_strong_params.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def create_params
:proxy,
barcodes: {},
public_notes: {},
user_attributes: [:name, :email, :library_id])
user_attributes: [:name, :email, :univ_id])
end

def update_params
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def current_ability
end

def request_specific_user
user_attributes = params.dig(:request, :user_attributes)&.permit(:name, :email, :library_id).to_h.reject { |_k, v| v.blank? }
user_attributes = params.dig(:request, :user_attributes)&.permit(:name, :email, :univ_id).to_h.reject { |_k, v| v.blank? }

User.new(**user_attributes, ip_address: request.remote_ip) if user_attributes.present?
end
Expand Down Expand Up @@ -89,8 +89,8 @@ def create_via_post?
params[:action].to_sym == :create && request.post?
end

# if the patron is trying to submit a request and didn't provide a library id or name/email,
# authenticate them. Since we only provide the library id or name/email option for requests where
# if the patron is trying to submit a request and didn't provide a university id or name/email,
# authenticate them. Since we only provide the university id or name/email option for requests where
# it will succeed, we should only end up here if the request requires authentication.
#
# if we ever validate patron data from the ILS, we'll need to add more logic here
Expand Down
4 changes: 2 additions & 2 deletions app/helpers/requests_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ def requester_info(user)

if user.sso_user? || user.email_address.present?
mail_to user.email_address, user.to_email_string
elsif user.library_id_user?
user.library_id
elsif user.univ_id_user?
user.univ_id
end
end
end
8 changes: 4 additions & 4 deletions app/javascript/additional_user_validation_fields.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ var additionalUserValidationFields = (function() {
fieldsAreValid: function() {
if (this.groupedUserFields().length > 0) { // If we have a name + email field
return this.validateGroupUserFields(); // Validate name + email field are filled out only (they are always required when present)
} else { // Else, we should only have a Library ID field
return this.validateSingleUserFields(); // Validate Library ID field because it should be required.
} else { // Else, we should only have a University ID field
return this.validateSingleUserFields(); // Validate University ID field because it should be required.
}
},

Expand All @@ -66,7 +66,7 @@ var additionalUserValidationFields = (function() {

if ( field.val().length < field.attr('minlength') ) {
field[0].setCustomValidity(
'Stanford Library ID must have 10 digits (you have entered ' + field.val().length + ' ).'
'Stanford University ID must have between 8 and 10 digits (you have entered ' + field.val().length + ' ).'
);
} else {
field[0].setCustomValidity('');
Expand All @@ -84,7 +84,7 @@ var additionalUserValidationFields = (function() {
addTooltipToButtonWrapper: function() {
this.buttonWrapper().attr('data-toggle', 'tooltip')
.attr('data-placement', 'top')
.attr('data-title', 'Enter your Library ID or your name and email to complete you request.')
.attr('data-title', 'Enter your University ID or your name and email to complete you request.')
.tooltip();
},

Expand Down
4 changes: 2 additions & 2 deletions app/jobs/submit_folio_request_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ def patron_comment
def patron
@patron ||= case request
when Scan
Folio::Patron.find_by(library_id: scan_destination.patron_barcode)
Folio::Patron.find_by(univ_id: scan_destination.patron_barcode)
when Page, MediatedPage, HoldRecall
if user.patron&.make_request_as_patron?
user.patron
Expand All @@ -192,7 +192,7 @@ def patron
def find_hold_pseudo_patron_for(key)
pseudopatron_barcode = Settings.libraries[key]&.hold_pseudopatron || raise("no hold pseudopatron for '#{key}'")

Folio::Patron.find_by(library_id: pseudopatron_barcode)
Folio::Patron.find_by(univ_id: pseudopatron_barcode)
end

def barcodes
Expand Down
10 changes: 5 additions & 5 deletions app/models/ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ def self.anonymous
@anonymous ||= Ability.new(User.new(name: 'generic', email: '[email protected]'))
end

def self.with_a_library_id
@with_a_library_id ||= Ability.new(User.new(library_id: '0000000000'))
def self.with_a_univ_id
@with_a_univ_id ||= Ability.new(User.new(univ_id: '000000000'))
end

def self.sso
Expand Down Expand Up @@ -63,18 +63,18 @@ def initialize(user, token = nil)
can :new, Request

# ... but only some types of users can actually submit the request successfully
if user.sso_user? || user.library_id_user? || user.name_email_user?
if user.sso_user? || user.univ_id_user? || user.name_email_user?
can :create, MediatedPage
can :create, Page
end

if user.name_email_user? && !user.library_id_user?
if user.name_email_user? && !user.univ_id_user?
cannot :create, Page, origin: 'BUSINESS'
cannot :create, Page, origin: 'MEDIA-MTXT'
cannot :create, Page, origin: 'MEDIA-CENTER'
end

can :create, HoldRecall if user.library_id_user? || user.sso_user?
can :create, HoldRecall if user.univ_id_user? || user.sso_user?
can :create, Scan if user.super_admin? || in_scan_pilot_group?(user)

# ... and to check the status, you either need to be logged in or include a special token in the URL
Expand Down
12 changes: 6 additions & 6 deletions app/models/concerns/request_validations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module RequestValidations
:requested_item_is_not_scannable_only,
on: :create
validate :needed_date_is_not_in_the_past, on: :create, if: :needed_date
validate :library_id_exists, on: :create
validate :univ_id_exists, on: :create
end

protected
Expand Down Expand Up @@ -47,17 +47,17 @@ def needed_date_is_not_in_the_past
end

# rubocop:disable Metrics/CyclomaticComplexity
def library_id_exists
def univ_id_exists
return unless user

# Ensure we don't contact the ILS if we don't need to validate the library ID
# Ensure we don't contact the ILS if we don't need to validate the university ID
return if user.sso_user? || (requestable_with_name_email? && user.name_email_user?)

# We require the library ID is on the client side when neccesary
# We require the university ID is on the client side when neccesary
# required when necessary, so if it's blank here, it's not required
return if user.library_id.blank?
return if user.univ_id.blank?

errors.add(:library_id, 'This ID was not found in our records') unless user.patron&.exists?
errors.add(:univ_id, 'This ID was not found in our records') unless user.patron&.exists?
end
# rubocop:enable Metrics/CyclomaticComplexity
end
4 changes: 2 additions & 2 deletions app/models/concerns/requestable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def requestable_with_name_email?
Ability.anonymous.can?(:create, self)
end

def requestable_with_library_id?
Ability.with_a_library_id.can?(:create, self)
def requestable_with_university_id?
Ability.with_a_univ_id.can?(:create, self)
end
end
7 changes: 0 additions & 7 deletions app/models/current_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,16 @@ def sso_user
end
end

# rubocop:disable Metrics/AbcSize
def update_ldap_attributes(user)
user.name = ldap_name
user.ldap_group_string = ldap_group_string
user.sucard_number = ldap_sucard_number
user.univ_id = ldap_univ_id
user.affiliation = ldap_affiliation
user.email = ldap_email
user.student_type = ldap_student_type

user.save if user.changed?
end
# rubocop:enable Metrics/AbcSize

def ldap_name
ldap_attributes['displayName']
Expand All @@ -59,10 +56,6 @@ def ldap_univ_id
ldap_attributes['suUnivID']
end

def ldap_sucard_number
ldap_attributes['suCardNumber']
end

def ldap_affiliation
ldap_attributes['suAffiliation']
end
Expand Down
10 changes: 5 additions & 5 deletions app/models/folio/patron.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ module Folio
# Model for working with FOLIO Patron information
class Patron
# rubocop:disable Metrics/CyclomaticComplexity
def self.find_by(sunetid: nil, library_id: nil, **_kwargs)
# if no sunet or library_id they are probably a general public (name/email) user.
return unless sunetid || library_id.present?
def self.find_by(sunetid: nil, univ_id: nil, **_kwargs)
# if no sunet or univ_id they are probably a general public (name/email) user.
return unless sunetid || univ_id.present?

response = folio_client.login_by_sunetid(sunetid) if sunetid.present?
response ||= folio_client.login_by_library_id(library_id) if library_id.present?
response ||= folio_client.login_by_university_id(univ_id) if univ_id.present?

return new(response) if response.present?

Honeybadger.notify("Unable to find patron (looked up by sunetid: #{sunetid} / barcode: #{library_id}")
Honeybadger.notify("Unable to find patron (looked up by sunetid: #{sunetid} / university ID: #{univ_id}")

nil
rescue HTTP::Error
Expand Down
14 changes: 7 additions & 7 deletions app/models/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,23 +105,23 @@ def find_existing_user

case
when user.sso_user? then User.find_by_sunetid(user.sunetid)
when user.library_id_user? then find_existing_library_id_user
when user.univ_id_user? then find_existing_univ_id_user
when user.name_email_user? then find_existing_email_user
end
end

def find_existing_library_id_user
def find_existing_univ_id_user
if user.email
User.find_by(library_id: user.library_id, email: user.email)
User.find_by(univ_id: user.univ_id, email: user.email)
else
User.find_by_library_id(user.library_id)
User.find_by_univ_id(user.univ_id)
end
end

def find_existing_email_user
return unless user.email

User.find_by(email: user.email, library_id: user.library_id).tap do |u|
User.find_by(email: user.email, univ_id: user.univ_id).tap do |u|
next unless u

u.update(name: user.name)
Expand Down Expand Up @@ -166,8 +166,8 @@ def check_remote_ip?
mediateable?
end

def library_id_error?
errors[:library_id].present?
def univ_id_error?
errors[:univ_id].present?
end

# rubocop:disable Metrics/MethodLength
Expand Down
35 changes: 10 additions & 25 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
###
class User < ActiveRecord::Base
validates :sunetid, uniqueness: true, allow_blank: true
validates :univ_id, format: { with: /\A\d{8,10}\z/ }, allow_blank: true

has_many :requests

Expand All @@ -29,37 +30,21 @@ def to_email_string
end
end

def sucard_number=(card_number)
return unless card_number.present?

self.library_id = card_number[/\d{5}(\d+)/, 1]
end

# Prefer the patron barcode from the ILS, but fall back to the library ID as-provided
# so that the system can function when the ILS is offline
def barcode
patron&.barcode || library_id
end

# Prefer the patron information from the ILS, but fall back to the univ ID
# Prefer the patron university ID from the ILS, but fall back to the univ ID as-provided
# so that the system can function when the ILS is offline
def university_id
patron&.university_id || univ_id
end

def library_id=(library_id)
super(library_id.to_s.upcase)
end

def email_address
case
when sso_user? && !email
# Fallback for users who were created before we started
# setting the email attribute for SSO users from LDAP
notify_honeybadger_of_missing_sso_email!
"#{sunetid}@stanford.edu"
when library_id_user?
email_from_symphony
when univ_id_user?
email_from_ils
else
email
end
Expand All @@ -69,8 +54,8 @@ def sso_user?
sunetid.present?
end

def library_id_user?
library_id.present?
def univ_id_user?
univ_id.present?
end

def name_email_user?
Expand Down Expand Up @@ -99,18 +84,18 @@ def student_type
(super || '').split(/[|;]/)
end

def email_from_symphony
def email_from_ils
self.email ||= begin
patron.email if library_id_user? && patron.present?
patron.email if univ_id_user? && patron.present?
end
end

def patron
@patron ||= begin
if sso_user?
patron_model_class.find_by(sunetid:)
elsif library_id_user?
patron_model_class.find_by(library_id:)
elsif univ_id_user?
patron_model_class.find_by(univ_id:)
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions app/services/folio_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ def login_by_sunetid(sunetid)
response&.dig('users', 0)
end

# Return the FOLIO user object given a library id (e.g. barcode)
# Return the FOLIO user object given a university ID
# See https://s3.amazonaws.com/foliodocs/api/mod-users/p/users.html#users__userid__get
def login_by_library_id(library_id)
response = get_json('/users', params: { query: CqlQuery.new(barcode: library_id).to_query })
def login_by_university_id(university_id)
response = get_json('/users', params: { query: CqlQuery.new(externalSystemId: university_id).to_query })
response&.dig('users', 0)
end

Expand Down
Loading

0 comments on commit 8d06c36

Please sign in to comment.