diff --git a/README.md b/README.md index 401ce1a12..a69e1104d 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/app/assets/stylesheets/modules/forms.scss b/app/assets/stylesheets/modules/forms.scss index a0261d65b..4eb8f8195 100644 --- a/app/assets/stylesheets/modules/forms.scss +++ b/app/assets/stylesheets/modules/forms.scss @@ -91,7 +91,7 @@ hr { display: none; } -.with-library-id-error { +.with-university-id-error { #sunetid-form { display: none; } diff --git a/app/controllers/concerns/request_strong_params.rb b/app/controllers/concerns/request_strong_params.rb index 90c5325ad..09189191e 100644 --- a/app/controllers/concerns/request_strong_params.rb +++ b/app/controllers/concerns/request_strong_params.rb @@ -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 diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 79a470d4f..6da1b8648 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -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 @@ -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 diff --git a/app/helpers/requests_helper.rb b/app/helpers/requests_helper.rb index f50d0a578..5cf2a7f8e 100644 --- a/app/helpers/requests_helper.rb +++ b/app/helpers/requests_helper.rb @@ -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 diff --git a/app/javascript/additional_user_validation_fields.js b/app/javascript/additional_user_validation_fields.js index 346297d77..1ef9abe1d 100644 --- a/app/javascript/additional_user_validation_fields.js +++ b/app/javascript/additional_user_validation_fields.js @@ -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. } }, @@ -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(''); @@ -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(); }, diff --git a/app/jobs/submit_folio_request_job.rb b/app/jobs/submit_folio_request_job.rb index 3d5047459..f2513d33f 100644 --- a/app/jobs/submit_folio_request_job.rb +++ b/app/jobs/submit_folio_request_job.rb @@ -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 @@ -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 diff --git a/app/models/ability.rb b/app/models/ability.rb index 2e46d166b..c523d3fce 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -12,8 +12,8 @@ def self.anonymous @anonymous ||= Ability.new(User.new(name: 'generic', email: 'external-user@example.com')) 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 @@ -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 diff --git a/app/models/concerns/request_validations.rb b/app/models/concerns/request_validations.rb index 304fd4a9f..f271e3e2b 100644 --- a/app/models/concerns/request_validations.rb +++ b/app/models/concerns/request_validations.rb @@ -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 @@ -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 diff --git a/app/models/concerns/requestable.rb b/app/models/concerns/requestable.rb index f6b74682b..7f1d6bd91 100644 --- a/app/models/concerns/requestable.rb +++ b/app/models/concerns/requestable.rb @@ -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 diff --git a/app/models/current_user.rb b/app/models/current_user.rb index bcda1a3dd..b0b09176e 100644 --- a/app/models/current_user.rb +++ b/app/models/current_user.rb @@ -33,11 +33,9 @@ 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 @@ -45,7 +43,6 @@ def update_ldap_attributes(user) user.save if user.changed? end - # rubocop:enable Metrics/AbcSize def ldap_name ldap_attributes['displayName'] @@ -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 diff --git a/app/models/folio/patron.rb b/app/models/folio/patron.rb index 9c46bd8a7..446e5d9a6 100644 --- a/app/models/folio/patron.rb +++ b/app/models/folio/patron.rb @@ -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 diff --git a/app/models/request.rb b/app/models/request.rb index 7bde720c5..bae8bcee2 100644 --- a/app/models/request.rb +++ b/app/models/request.rb @@ -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) @@ -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 diff --git a/app/models/user.rb b/app/models/user.rb index e5dc078b6..2ed0416d8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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 @@ -29,28 +30,12 @@ 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 @@ -58,8 +43,8 @@ def email_address # 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 @@ -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? @@ -99,9 +84,9 @@ 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 @@ -109,8 +94,8 @@ 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 diff --git a/app/services/folio_client.rb b/app/services/folio_client.rb index 967d7cae6..9087c54a9 100644 --- a/app/services/folio_client.rb +++ b/app/services/folio_client.rb @@ -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 diff --git a/app/views/requests/_no_sunet_form.html.erb b/app/views/requests/_no_sunet_form.html.erb index 48115cc2b..83c29e9c6 100644 --- a/app/views/requests/_no_sunet_form.html.erb +++ b/app/views/requests/_no_sunet_form.html.erb @@ -1,29 +1,29 @@ -<% if f.object.requestable_with_library_id? || f.object.requestable_with_name_email? %> +<% if f.object.requestable_with_university_id? || f.object.requestable_with_name_email? %>
<%= f.fields_for :user, (current_request.user unless current_request.user && current_request.user.sso_user?) || User.new do |uf| %> - <% if f.object.requestable_with_library_id? %> -
"> - <%= uf.label :library_id, 'Stanford Library ID', class: "control-label #{label_column_class} #{'required' unless f.object.requestable_with_name_email?}" %> + <% if f.object.requestable_with_university_id? %> +
"> + <%= uf.label :univ_id, t('activerecord.attributes.user.univ_id'), class: "control-label #{label_column_class} #{'required' unless f.object.requestable_with_name_email?}" %>
<%= uf.text_field_without_bootstrap( - :library_id, + :univ_id, class: 'form-control', - minlength: (10 unless f.object.requestable_with_name_email?), + minlength: (8 unless f.object.requestable_with_name_email?), maxlength: 10, - 'aria-describedby' => 'library-id-help', + 'aria-describedby' => 'university-id-help', data: { behavior: 'single-user-field' }, autocomplete: 'off' ) %> - <% if f.object.library_id_error? %> -