From 258bf3e0b24ee828cd635b2c9c641dd12420f375 Mon Sep 17 00:00:00 2001 From: zhouyu Date: Fri, 13 Feb 2026 14:21:29 -0800 Subject: [PATCH 01/14] Add Calnet schema attribute name validation --- app/lib/error/calnet_error.rb | 6 ++++ app/models/user.rb | 61 ++++++++++++++++++++++++++++------- spec/models/user_spec.rb | 22 +++++++++++++ 3 files changed, 78 insertions(+), 11 deletions(-) create mode 100644 app/lib/error/calnet_error.rb diff --git a/app/lib/error/calnet_error.rb b/app/lib/error/calnet_error.rb new file mode 100644 index 00000000..9d95cff1 --- /dev/null +++ b/app/lib/error/calnet_error.rb @@ -0,0 +1,6 @@ +module Error + # Raised calnet error when it returns an unexpected response, + # e.g. missing email value because of the schema attribute name changed unexpected by Calnet from 'berkeleyEduAlternateId' to 'berkeleyEduAlternateID' . + class CalnetError < ApplicationError + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 227b170c..3bef2cf1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -7,6 +7,21 @@ class User FRAMEWORK_ADMIN_GROUP = 'cn=edu:berkeley:org:libr:framework:LIBR-framework-admins,ou=campus groups,dc=berkeley,dc=edu'.freeze ALMA_ADMIN_GROUP = 'cn=edu:berkeley:org:libr:framework:alma-admins,ou=campus groups,dc=berkeley,dc=edu'.freeze + CALNET_ATTRS = { + affiliations: 'berkeleyEduAffiliations', + cs_id: 'berkeleyEduCSID', + groups: 'berkeleyEduIsMemberOf', + # student_id: 'berkeleyEduStuID', + ucpath_id: 'berkeleyEduUCPathID', + email: 'berkeleyEduAlternateID', + department_number: 'departmentNumber', + display_name: 'displayName', + employee_id: 'employeeNumber', + given_name: 'givenName', + surname: 'surname', + uid: 'uid' + }.freeze + class << self # Returns a new user object from the given "omniauth.auth" hash. That's a # hash of all data returned by the auth provider (in our case, calnet). @@ -26,26 +41,50 @@ def from_omniauth(auth) # rubocop:disable Metrics/MethodLength def auth_params_from(auth) auth_extra = auth['extra'] + verify_calnet_attributes!(auth_extra) cal_groups = auth_extra['berkeleyEduIsMemberOf'] || [] # NOTE: berkeleyEduCSID should be same as berkeleyEduStuID for students { - affiliations: auth_extra['berkeleyEduAffiliations'], - cs_id: auth_extra['berkeleyEduCSID'], - department_number: auth_extra['departmentNumber'], - display_name: auth_extra['displayName'], - email: auth_extra['berkeleyEduAlternateID'], - employee_id: auth_extra['employeeNumber'], - given_name: auth_extra['givenName'], - student_id: auth_extra['berkeleyEduStuID'], - surname: auth_extra['surname'], - ucpath_id: auth_extra['berkeleyEduUCPathID'], - uid: auth_extra['uid'] || auth['uid'], + affiliations: auth_extra[CALNET_ATTRS[:affiliations]], + cs_id: auth_extra[CALNET_ATTRS[:cs_id]], + department_number: auth_extra[CALNET_ATTRS[:department_number]], + display_name: auth_extra[CALNET_ATTRS[:display_name]], + email: auth_extra[CALNET_ATTRS[:email]], + employee_id: auth_extra[CALNET_ATTRS[:employee_id]], + given_name: auth_extra[CALNET_ATTRS[:given_name]], + student_id: auth_extra[CALNET_ATTRS[:cs_id]], + surname: auth_extra[CALNET_ATTRS[:surname]], + ucpath_id: auth_extra[CALNET_ATTRS[:ucpath_id]], + uid: auth_extra[CALNET_ATTRS[:uid]] || auth['uid'], framework_admin: cal_groups.include?(FRAMEWORK_ADMIN_GROUP), alma_admin: cal_groups.include?(ALMA_ADMIN_GROUP) } end # rubocop:enable Metrics/MethodLength + + # Verifies that auth_extra contains all required CalNet attributes with exact case-sensitive names + # Raise [Error::CalnetError] if any required attributes are missing + def verify_calnet_attributes!(auth_extra) + required_attributes = CALNET_ATTRS.values + + missing = required_attributes.reject { |attr| auth_extra.key?(attr) } + + return if missing.empty? + + current_calnet_keys = list_auth_extra_keys(auth_extra) + msg = "Cannot find CalNet schema attribute(s) (case-sensitive): #{missing.join(', ')}. The current CalNet schema attributes: #{current_calnet_keys.join(', ')}." + Rails.logger.error(msg) + raise Error::CalnetError, msg + end + + # list all keys except duo keys + def list_auth_extra_keys(auth_extra) + keys = auth_extra.keys.reject { |k| k.start_with?('duo') }.sort + Rails.logger.info("CalNet auth_extra keys: #{keys.join(', ')}") + keys + end + end # Affiliations per CalNet (attribute `berkeleyEduAffiliations` e.g. diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 058c343d..44bd026b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -21,6 +21,28 @@ expect { User.from_omniauth(auth) }.to raise_error(Error::InvalidAuthProviderError) end + it 'rejects calnet when a required schema attribute is missing or renamed' do + auth = { + 'provider' => 'calnet', + 'extra' => { + 'berkeleyEduAffiliations' => 'expected affiliation', + 'berkeleyEduCSID' => 'expected cs id', + 'berkeleyEduIsMemberOf' => [], + 'berkeleyEduStuID' => 'expected student id', + 'berkeleyEduUCPathID' => 'expected UC Path ID', + 'berkeleyEduAlternatid' => 'expected email', # intentionally wrong case to simulate renamed attribute: berkeleyEduAlternatid instead of berkeleyEduAlternateId + 'departmentNumber' => 'expected dept. number', + 'displayName' => 'expected display name', + 'employeeNumber' => 'expected employee ID', + 'givenName' => 'expected given name', + 'surname' => 'expected surname', + 'uid' => 'expected UID' + } + } + + expect { User.from_omniauth(auth) }.to raise_error(Error::CalnetError, /Missing required CalNet attributes/) + end + it 'populates a User object' do framework_admin_ldap = 'cn=edu:berkeley:org:libr:framework:LIBR-framework-admins,ou=campus groups,dc=berkeley,dc=edu' auth = { From a0a4788a7e15cdae635b1ff055e7a31df96351b1 Mon Sep 17 00:00:00 2001 From: zhouyu Date: Wed, 18 Feb 2026 11:58:11 -0800 Subject: [PATCH 02/14] tested with all extra fields --- app/models/user.rb | 68 +++++++++++++++++++++++++--------------- cleanup_calnet_extra.rb | 0 spec/data/calnet/abc.yml | 14 +++++++++ spec/models/user_spec.rb | 13 ++++---- 4 files changed, 62 insertions(+), 33 deletions(-) create mode 100644 cleanup_calnet_extra.rb create mode 100644 spec/data/calnet/abc.yml diff --git a/app/models/user.rb b/app/models/user.rb index 3bef2cf1..1976a80c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -8,18 +8,18 @@ class User ALMA_ADMIN_GROUP = 'cn=edu:berkeley:org:libr:framework:alma-admins,ou=campus groups,dc=berkeley,dc=edu'.freeze CALNET_ATTRS = { - affiliations: 'berkeleyEduAffiliations', - cs_id: 'berkeleyEduCSID', - groups: 'berkeleyEduIsMemberOf', + affiliations: 'berkeleyEduAffiliations'.freeze, + cs_id: ['berkeleyEduStuID', 'berkeleyEduCSID'].freeze, + # groups: 'berkeleyEduIsMemberOf'.freeze, # student_id: 'berkeleyEduStuID', - ucpath_id: 'berkeleyEduUCPathID', - email: 'berkeleyEduAlternateID', - department_number: 'departmentNumber', - display_name: 'displayName', - employee_id: 'employeeNumber', - given_name: 'givenName', - surname: 'surname', - uid: 'uid' + ucpath_id: 'berkeleyEduUCPathID'.freeze, + email: ['berkeleyEduAlternateID', 'berkeleyEduAlternateId'].freeze, + department_number: 'departmentNumber'.freeze, + display_name: 'displayName'.freeze, + employee_id: 'employeeNumber'.freeze, + given_name: 'givenName'.freeze, + surname: 'surname'.freeze, + uid: 'uid'.freeze }.freeze class << self @@ -46,17 +46,17 @@ def auth_params_from(auth) # NOTE: berkeleyEduCSID should be same as berkeleyEduStuID for students { - affiliations: auth_extra[CALNET_ATTRS[:affiliations]], - cs_id: auth_extra[CALNET_ATTRS[:cs_id]], - department_number: auth_extra[CALNET_ATTRS[:department_number]], - display_name: auth_extra[CALNET_ATTRS[:display_name]], - email: auth_extra[CALNET_ATTRS[:email]], - employee_id: auth_extra[CALNET_ATTRS[:employee_id]], - given_name: auth_extra[CALNET_ATTRS[:given_name]], - student_id: auth_extra[CALNET_ATTRS[:cs_id]], - surname: auth_extra[CALNET_ATTRS[:surname]], - ucpath_id: auth_extra[CALNET_ATTRS[:ucpath_id]], - uid: auth_extra[CALNET_ATTRS[:uid]] || auth['uid'], + affiliations: get_attribute_from_auth(auth_extra, :affiliations), + cs_id: get_attribute_from_auth(auth_extra, :cs_id), + department_number: get_attribute_from_auth(auth_extra, :department_number), + display_name: get_attribute_from_auth(auth_extra, :display_name), + email: get_attribute_from_auth(auth_extra, :email), + employee_id: get_attribute_from_auth(auth_extra, :employee_id), + given_name: get_attribute_from_auth(auth_extra, :given_name), + student_id: get_attribute_from_auth(auth_extra, :cs_id), + surname: get_attribute_from_auth(auth_extra, :surname), + ucpath_id: get_attribute_from_auth(auth_extra, :ucpath_id), + uid: get_attribute_from_auth(auth_extra, :uid) || auth['uid'], framework_admin: cal_groups.include?(FRAMEWORK_ADMIN_GROUP), alma_admin: cal_groups.include?(ALMA_ADMIN_GROUP) } @@ -64,11 +64,18 @@ def auth_params_from(auth) # rubocop:enable Metrics/MethodLength # Verifies that auth_extra contains all required CalNet attributes with exact case-sensitive names + # For array attributes, at least one value in the array must be present in auth_extra # Raise [Error::CalnetError] if any required attributes are missing def verify_calnet_attributes!(auth_extra) required_attributes = CALNET_ATTRS.values - missing = required_attributes.reject { |attr| auth_extra.key?(attr) } + missing = required_attributes.reject do |attr| + if attr.is_a?(Array) + attr.any? { |a| auth_extra.key?(a) } + else + auth_extra.key?(attr) + end + end return if missing.empty? @@ -80,11 +87,20 @@ def verify_calnet_attributes!(auth_extra) # list all keys except duo keys def list_auth_extra_keys(auth_extra) - keys = auth_extra.keys.reject { |k| k.start_with?('duo') }.sort - Rails.logger.info("CalNet auth_extra keys: #{keys.join(', ')}") - keys + auth_extra.keys.reject { |k| k.start_with?('duo') }.sort end + # Gets an attribute value from auth_extra, handling both string and array attribute names + # If attribute is an array, tries each key in order and returns the first match + # If attribute is a string, returns the value for that key + def get_attribute_from_auth(auth_extra, attr_key) + attrs = CALNET_ATTRS[attr_key] + return auth_extra[attrs] unless attrs.is_a?(Array) + + attrs.find { |attr| auth_extra.key?(attr) }.yield_self { |attr| attr && auth_extra[attr] } + end + + end # Affiliations per CalNet (attribute `berkeleyEduAffiliations` e.g. diff --git a/cleanup_calnet_extra.rb b/cleanup_calnet_extra.rb new file mode 100644 index 00000000..e69de29b diff --git a/spec/data/calnet/abc.yml b/spec/data/calnet/abc.yml new file mode 100644 index 00000000..cc2cc109 --- /dev/null +++ b/spec/data/calnet/abc.yml @@ -0,0 +1,14 @@ +--- + +extra: + berkeleyEduAffiliations: fake_berkeleyEduAffiliations + berkeleyEduCSID: fake_berkeleyEduCSID + berkeleyEduIsMemberOf: fake_berkeleyEduIsMemberOf + berkeleyEduUCPathID: fake_berkeleyEduUCPathID + berkeleyEduAlternateID: fake_berkeleyEduAlternateID + departmentNumber: fake_departmentNumber + displayName: fake_displayName + employeeNumber: fake_employeeNumber + givenName: fake_givenName + surname: fake_surname + uid: fake_uid \ No newline at end of file diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 44bd026b..3fea32b3 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -28,9 +28,8 @@ 'berkeleyEduAffiliations' => 'expected affiliation', 'berkeleyEduCSID' => 'expected cs id', 'berkeleyEduIsMemberOf' => [], - 'berkeleyEduStuID' => 'expected student id', 'berkeleyEduUCPathID' => 'expected UC Path ID', - 'berkeleyEduAlternatid' => 'expected email', # intentionally wrong case to simulate renamed attribute: berkeleyEduAlternatid instead of berkeleyEduAlternateId + 'berkeleyEduAlternatid' => 'expected email', # intentionally wrong case to simulate renamed attribute: berkeleyEduAlternatid instead of berkeleyEduAlternateID 'departmentNumber' => 'expected dept. number', 'displayName' => 'expected display name', 'employeeNumber' => 'expected employee ID', @@ -40,7 +39,7 @@ } } - expect { User.from_omniauth(auth) }.to raise_error(Error::CalnetError, /Missing required CalNet attributes/) + expect { User.from_omniauth(auth) }.to raise_error(Error::CalnetError, /Cannot find CalNet schema attribute/) end it 'populates a User object' do @@ -54,7 +53,7 @@ 'berkeleyEduAlternateID' => 'expected email', 'employeeNumber' => 'expected employee ID', 'givenName' => 'expected given name', - 'berkeleyEduStuID' => 'expected student ID', + 'berkeleyEduCSID' => 'expected cs id', 'surname' => 'expected surname', 'berkeleyEduUCPathID' => 'expected UC Path ID', 'uid' => 'expected UID', @@ -71,7 +70,7 @@ expect(user.email).to eq('expected email') expect(user.employee_id).to eq('expected employee ID') expect(user.given_name).to eq('expected given name') - expect(user.student_id).to eq('expected student ID') + expect(user.student_id).to eq('expected cs id') expect(user.surname).to eq('expected surname') expect(user.ucpath_id).to eq('expected UC Path ID') expect(user.uid).to eq('expected UID') @@ -89,7 +88,7 @@ 'berkeleyEduAlternateID' => 'expected email', 'employeeNumber' => 'expected employee ID', 'givenName' => 'expected given name', - 'berkeleyEduStuID' => 'expected student ID', + 'berkeleyEduCSID' => 'expected cs id', 'surname' => 'expected surname', 'berkeleyEduUCPathID' => 'expected UC Path ID', 'uid' => 'expected UID' @@ -103,7 +102,7 @@ expect(user.email).to eq('expected email') expect(user.employee_id).to eq('expected employee ID') expect(user.given_name).to eq('expected given name') - expect(user.student_id).to eq('expected student ID') + expect(user.student_id).to eq('expected cs id') expect(user.surname).to eq('expected surname') expect(user.ucpath_id).to eq('expected UC Path ID') expect(user.uid).to eq('expected UID') From ddc5ca3c810701dbe8181548c44d2a17c22f45c0 Mon Sep 17 00:00:00 2001 From: zhouyu Date: Wed, 18 Feb 2026 13:40:22 -0800 Subject: [PATCH 03/14] use a base yml --- spec/calnet_helper.rb | 10 +++++++++- spec/data/calnet/{abc.yml => base.yml} | 0 2 files changed, 9 insertions(+), 1 deletion(-) rename spec/data/calnet/{abc.yml => base.yml} (100%) diff --git a/spec/calnet_helper.rb b/spec/calnet_helper.rb index 636497af..5a18c087 100644 --- a/spec/calnet_helper.rb +++ b/spec/calnet_helper.rb @@ -54,7 +54,15 @@ def auth_hash_for(uid) calnet_yml_file = "spec/data/calnet/#{uid}.yml" raise IOError, "No such file: #{calnet_yml_file}" unless File.file?(calnet_yml_file) - YAML.load_file(calnet_yml_file) + auth_hash = YAML.load_file(calnet_yml_file) + + # Merge in default extra fields from application config + if Rails.application.config.respond_to?(:calnet_test_defaults) + defaults = Rails.application.config.calnet_test_defaults.stringify_keys + auth_hash['extra'] = defaults.merge(auth_hash['extra'] || {}) + end + + auth_hash end # Logs out. Suitable for calling in an after() block. diff --git a/spec/data/calnet/abc.yml b/spec/data/calnet/base.yml similarity index 100% rename from spec/data/calnet/abc.yml rename to spec/data/calnet/base.yml From 63a17d08d28dd2eb78fef43accf96c8a52c653f0 Mon Sep 17 00:00:00 2001 From: zhouyu Date: Wed, 18 Feb 2026 13:41:56 -0800 Subject: [PATCH 04/14] use a base yml --- spec/calnet_helper.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/spec/calnet_helper.rb b/spec/calnet_helper.rb index 5a18c087..28b5161b 100644 --- a/spec/calnet_helper.rb +++ b/spec/calnet_helper.rb @@ -56,10 +56,11 @@ def auth_hash_for(uid) auth_hash = YAML.load_file(calnet_yml_file) - # Merge in default extra fields from application config - if Rails.application.config.respond_to?(:calnet_test_defaults) - defaults = Rails.application.config.calnet_test_defaults.stringify_keys - auth_hash['extra'] = defaults.merge(auth_hash['extra'] || {}) + # Merge in default extra fields from base.yml + base_yml_file = 'spec/data/calnet/base.yml' + if File.file?(base_yml_file) + base_defaults = YAML.load_file(base_yml_file)['extra'] || {} + auth_hash['extra'] = base_defaults.merge(auth_hash['extra'] || {}) end auth_hash From e44ecd27d7971e1682bcc01e6d1cba4e13f1b0ea Mon Sep 17 00:00:00 2001 From: zhouyu Date: Wed, 18 Feb 2026 14:39:14 -0800 Subject: [PATCH 05/14] Use config for calnet attributes --- app/models/user.rb | 21 ++++----------------- config/application.rb | 16 ++++++++++++++++ spec/calnet_helper.rb | 14 ++++++-------- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 1976a80c..a02a7381 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -7,20 +7,8 @@ class User FRAMEWORK_ADMIN_GROUP = 'cn=edu:berkeley:org:libr:framework:LIBR-framework-admins,ou=campus groups,dc=berkeley,dc=edu'.freeze ALMA_ADMIN_GROUP = 'cn=edu:berkeley:org:libr:framework:alma-admins,ou=campus groups,dc=berkeley,dc=edu'.freeze - CALNET_ATTRS = { - affiliations: 'berkeleyEduAffiliations'.freeze, - cs_id: ['berkeleyEduStuID', 'berkeleyEduCSID'].freeze, - # groups: 'berkeleyEduIsMemberOf'.freeze, - # student_id: 'berkeleyEduStuID', - ucpath_id: 'berkeleyEduUCPathID'.freeze, - email: ['berkeleyEduAlternateID', 'berkeleyEduAlternateId'].freeze, - department_number: 'departmentNumber'.freeze, - display_name: 'displayName'.freeze, - employee_id: 'employeeNumber'.freeze, - given_name: 'givenName'.freeze, - surname: 'surname'.freeze, - uid: 'uid'.freeze - }.freeze + # CalNet attribute mapping derived from configuration + CALNET_ATTRS = Rails.application.config.calnet_attrs.freeze class << self # Returns a new user object from the given "omniauth.auth" hash. That's a @@ -62,7 +50,7 @@ def auth_params_from(auth) } end # rubocop:enable Metrics/MethodLength - + # Verifies that auth_extra contains all required CalNet attributes with exact case-sensitive names # For array attributes, at least one value in the array must be present in auth_extra # Raise [Error::CalnetError] if any required attributes are missing @@ -97,10 +85,9 @@ def get_attribute_from_auth(auth_extra, attr_key) attrs = CALNET_ATTRS[attr_key] return auth_extra[attrs] unless attrs.is_a?(Array) - attrs.find { |attr| auth_extra.key?(attr) }.yield_self { |attr| attr && auth_extra[attr] } + attrs.find { |attr| auth_extra.key?(attr) }.then { |attr| attr && auth_extra[attr] } end - end # Affiliations per CalNet (attribute `berkeleyEduAffiliations` e.g. diff --git a/config/application.rb b/config/application.rb index 13d2b032..2f5ffbaa 100644 --- a/config/application.rb +++ b/config/application.rb @@ -111,6 +111,22 @@ def log_active_storage_root!(active_storage_root) config.x.healthcheck_urls.whois = 'https://whois.arin.net/rest/poc/1AD-ARIN' config.x.healthcheck_urls.berkeley_service_now = 'https://berkeley.service-now.com/kb_view.do?sysparm_article=KB0011960' + # CalNet attribute mapping - shared between User model and test calnet_helper + # Maps hash values to CalNet attribute name(s) + # Array values indicate fallback/alternative attribute names + config.calnet_attrs = { + affiliations: 'berkeleyEduAffiliations', + cs_id: %w[berkeleyEduStuID berkeleyEduCSID], + ucpath_id: 'berkeleyEduUCPathID', + email: %w[berkeleyEduAlternateID berkeleyEduAlternateId], + department_number: 'departmentNumber', + display_name: 'displayName', + employee_id: 'employeeNumber', + given_name: 'givenName', + surname: 'surname', + uid: 'uid' + }.freeze + config.to_prepare do GoodJob::JobsController.class_eval do include AuthSupport diff --git a/spec/calnet_helper.rb b/spec/calnet_helper.rb index 28b5161b..cc46b432 100644 --- a/spec/calnet_helper.rb +++ b/spec/calnet_helper.rb @@ -55,14 +55,12 @@ def auth_hash_for(uid) raise IOError, "No such file: #{calnet_yml_file}" unless File.file?(calnet_yml_file) auth_hash = YAML.load_file(calnet_yml_file) - - # Merge in default extra fields from base.yml - base_yml_file = 'spec/data/calnet/base.yml' - if File.file?(base_yml_file) - base_defaults = YAML.load_file(base_yml_file)['extra'] || {} - auth_hash['extra'] = base_defaults.merge(auth_hash['extra'] || {}) - end - + + # Merge in default extra testing fields using attribute names from config + attr_names = Rails.application.config.calnet_attrs.values.map { |v| v.is_a?(Array) ? v.first : v }.freeze + default_extra_subfields = attr_names.to_h { |attr| [attr, "fake_#{attr}"] } + auth_hash['extra'] = default_extra_subfields.merge(auth_hash['extra'] || {}) + auth_hash end From 6f1b76457d5413e7e6e01c6bd4e117e10b63bfc4 Mon Sep 17 00:00:00 2001 From: zhouyu Date: Wed, 18 Feb 2026 15:22:14 -0800 Subject: [PATCH 06/14] Fix rubcopo errors --- app/lib/error/calnet_error.rb | 3 +-- app/models/user.rb | 18 +++++++++--------- cleanup_calnet_extra.rb | 0 spec/models/user_spec.rb | 2 +- 4 files changed, 11 insertions(+), 12 deletions(-) delete mode 100644 cleanup_calnet_extra.rb diff --git a/app/lib/error/calnet_error.rb b/app/lib/error/calnet_error.rb index 9d95cff1..4aacc0df 100644 --- a/app/lib/error/calnet_error.rb +++ b/app/lib/error/calnet_error.rb @@ -1,6 +1,5 @@ module Error - # Raised calnet error when it returns an unexpected response, - # e.g. missing email value because of the schema attribute name changed unexpected by Calnet from 'berkeleyEduAlternateId' to 'berkeleyEduAlternateID' . + # Raised calnet error when it returns an unexpected response, such as missing expected attributes class CalnetError < ApplicationError end end diff --git a/app/models/user.rb b/app/models/user.rb index a02a7381..7247463b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -26,7 +26,7 @@ def from_omniauth(auth) private - # rubocop:disable Metrics/MethodLength + # rubocop:disable Metrics/AbcSize, Metrics/MethodLength def auth_params_from(auth) auth_extra = auth['extra'] verify_calnet_attributes!(auth_extra) @@ -49,7 +49,7 @@ def auth_params_from(auth) alma_admin: cal_groups.include?(ALMA_ADMIN_GROUP) } end - # rubocop:enable Metrics/MethodLength + # rubocop:enable Metrics/AbcSize, Metrics/MethodLength # Verifies that auth_extra contains all required CalNet attributes with exact case-sensitive names # For array attributes, at least one value in the array must be present in auth_extra @@ -67,15 +67,15 @@ def verify_calnet_attributes!(auth_extra) return if missing.empty? - current_calnet_keys = list_auth_extra_keys(auth_extra) - msg = "Cannot find CalNet schema attribute(s) (case-sensitive): #{missing.join(', ')}. The current CalNet schema attributes: #{current_calnet_keys.join(', ')}." - Rails.logger.error(msg) - raise Error::CalnetError, msg + raise_missing_calnet_attribute_error(auth_extra, missing) end - # list all keys except duo keys - def list_auth_extra_keys(auth_extra) - auth_extra.keys.reject { |k| k.start_with?('duo') }.sort + def raise_missing_calnet_attribute_error(auth_extra) + missing_attrs = "Expected Calnet attribute(s) not found (case-sensitive): #{missing.join(', ')}." + actual_calnet_keys = auth_extra.keys.reject { |k| k.start_with?('duo') }.sort + msg = "#{missing_attrs} The actual CalNet attributes: #{actual_calnet_keys.join(', ')}. The user is #{auth_extra['displayname']}" + Rails.logger.error(msg) + raise Error::CalnetError, msg end # Gets an attribute value from auth_extra, handling both string and array attribute names diff --git a/cleanup_calnet_extra.rb b/cleanup_calnet_extra.rb deleted file mode 100644 index e69de29b..00000000 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3fea32b3..55ecd1c9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -29,7 +29,7 @@ 'berkeleyEduCSID' => 'expected cs id', 'berkeleyEduIsMemberOf' => [], 'berkeleyEduUCPathID' => 'expected UC Path ID', - 'berkeleyEduAlternatid' => 'expected email', # intentionally wrong case to simulate renamed attribute: berkeleyEduAlternatid instead of berkeleyEduAlternateID + 'berkeleyEduAlternatid' => 'expected email', # intentionally wrong case to simulate wrong attribute 'departmentNumber' => 'expected dept. number', 'displayName' => 'expected display name', 'employeeNumber' => 'expected employee ID', From 9cefd5be8c329730744d753f5279b72ed6de8a96 Mon Sep 17 00:00:00 2001 From: zhouyu Date: Wed, 18 Feb 2026 15:56:10 -0800 Subject: [PATCH 07/14] pass rubocop check --- app/models/user.rb | 4 ++-- spec/models/user_spec.rb | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 7247463b..e3beca87 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -70,10 +70,10 @@ def verify_calnet_attributes!(auth_extra) raise_missing_calnet_attribute_error(auth_extra, missing) end - def raise_missing_calnet_attribute_error(auth_extra) + def raise_missing_calnet_attribute_error(auth_extra, missing) missing_attrs = "Expected Calnet attribute(s) not found (case-sensitive): #{missing.join(', ')}." actual_calnet_keys = auth_extra.keys.reject { |k| k.start_with?('duo') }.sort - msg = "#{missing_attrs} The actual CalNet attributes: #{actual_calnet_keys.join(', ')}. The user is #{auth_extra['displayname']}" + msg = "#{missing_attrs} The actual CalNet attributes: #{actual_calnet_keys.join(', ')}. The user is #{auth_extra['displayName']}" Rails.logger.error(msg) raise Error::CalnetError, msg end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 55ecd1c9..2c1107d6 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -39,7 +39,13 @@ } } - expect { User.from_omniauth(auth) }.to raise_error(Error::CalnetError, /Cannot find CalNet schema attribute/) + missing = %w[berkeleyEduAlternateID berkeleyEduAlternateId] + actual = %w[berkeleyEduAffiliations berkeleyEduAlternatid berkeleyEduCSID berkeleyEduIsMemberOf berkeleyEduUCPathID departmentNumber + displayName employeeNumber givenName surname uid] + # rubocop:disable Layout/LineLength + msg = "Expected Calnet attribute(s) not found (case-sensitive): #{missing.join(', ')}. The actual CalNet attributes: #{actual.join(', ')}. The user is expected display name" + # rubocop:enable Layout/LineLength + expect { User.from_omniauth(auth) }.to raise_error(Error::CalnetError, msg) end it 'populates a User object' do From d66e649b9d0ca0833d913e67a00a822e3d82f65e Mon Sep 17 00:00:00 2001 From: zhouyu Date: Wed, 18 Feb 2026 16:54:16 -0800 Subject: [PATCH 08/14] remove base.yml --- spec/data/calnet/base.yml | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 spec/data/calnet/base.yml diff --git a/spec/data/calnet/base.yml b/spec/data/calnet/base.yml deleted file mode 100644 index cc2cc109..00000000 --- a/spec/data/calnet/base.yml +++ /dev/null @@ -1,14 +0,0 @@ ---- - -extra: - berkeleyEduAffiliations: fake_berkeleyEduAffiliations - berkeleyEduCSID: fake_berkeleyEduCSID - berkeleyEduIsMemberOf: fake_berkeleyEduIsMemberOf - berkeleyEduUCPathID: fake_berkeleyEduUCPathID - berkeleyEduAlternateID: fake_berkeleyEduAlternateID - departmentNumber: fake_departmentNumber - displayName: fake_displayName - employeeNumber: fake_employeeNumber - givenName: fake_givenName - surname: fake_surname - uid: fake_uid \ No newline at end of file From b06f0c820d5b188c28a125de55955059142421f6 Mon Sep 17 00:00:00 2001 From: zhouyu Date: Thu, 19 Feb 2026 20:17:02 -0800 Subject: [PATCH 09/14] Validate employee and student differently --- app/models/user.rb | 79 ++++++++++++++++++++++++++++++++++--------- config/application.rb | 4 +-- 2 files changed, 65 insertions(+), 18 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index e3beca87..adda218d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -29,22 +29,23 @@ def from_omniauth(auth) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength def auth_params_from(auth) auth_extra = auth['extra'] + # log_auth_extra(auth_extra) verify_calnet_attributes!(auth_extra) cal_groups = auth_extra['berkeleyEduIsMemberOf'] || [] # NOTE: berkeleyEduCSID should be same as berkeleyEduStuID for students { - affiliations: get_attribute_from_auth(auth_extra, :affiliations), - cs_id: get_attribute_from_auth(auth_extra, :cs_id), - department_number: get_attribute_from_auth(auth_extra, :department_number), - display_name: get_attribute_from_auth(auth_extra, :display_name), + affiliations: auth_extra['berkeleyEduAffiliations'], + cs_id: auth_extra['berkeleyEduCSID'], + department_number: auth_extra['departmentNumber'], + display_name: auth_extra['displayName'], email: get_attribute_from_auth(auth_extra, :email), - employee_id: get_attribute_from_auth(auth_extra, :employee_id), - given_name: get_attribute_from_auth(auth_extra, :given_name), - student_id: get_attribute_from_auth(auth_extra, :cs_id), - surname: get_attribute_from_auth(auth_extra, :surname), - ucpath_id: get_attribute_from_auth(auth_extra, :ucpath_id), - uid: get_attribute_from_auth(auth_extra, :uid) || auth['uid'], + employee_id: auth_extra['employeeNumber'], + given_name: auth_extra['givenName'], + student_id: auth_extra['berkeleyEduStuID'], + surname: auth_extra['surname'], + ucpath_id: auth_extra['berkeleyEduUCPathID'], + uid: auth_extra['uid'] || auth['uid'], framework_admin: cal_groups.include?(FRAMEWORK_ADMIN_GROUP), alma_admin: cal_groups.include?(ALMA_ADMIN_GROUP) } @@ -55,14 +56,13 @@ def auth_params_from(auth) # For array attributes, at least one value in the array must be present in auth_extra # Raise [Error::CalnetError] if any required attributes are missing def verify_calnet_attributes!(auth_extra) - required_attributes = CALNET_ATTRS.values + affiliations = affiliations_from(auth_extra) + raise_missing_calnet_attribute_error(auth_extra, ['berkeleyEduAffiliations']) if affiliations.blank? + + required_attributes = required_attributes_for(affiliations) missing = required_attributes.reject do |attr| - if attr.is_a?(Array) - attr.any? { |a| auth_extra.key?(a) } - else - auth_extra.key?(attr) - end + present_in_auth_extra?(auth_extra, attr) end return if missing.empty? @@ -78,6 +78,53 @@ def raise_missing_calnet_attribute_error(auth_extra, missing) raise Error::CalnetError, msg end + # def log_auth_extra(auth_extra) + # return if auth_extra.nil? + + # keys = auth_extra.keys.reject { |k| k.start_with?('duo') }.sort + # Rails.logger.info("CalNet auth_extra keys!!! student 5 - #{auth_extra['berkeleyEduAffiliations']}: #{keys.join(', ')}") + # end + + def affiliations_from(auth_extra) + Array(auth_extra['berkeleyEduAffiliations']) + end + + def employee_affiliated?(affiliations) + affiliations.include?('EMPLOYEE-TYPE-STAFF') || + affiliations.include?('EMPLOYEE-TYPE-ACADEMIC') + end + + def student_affiliated?(affiliations) + affiliations.include?('STUDENT-TYPE-NOT-REGISTERED') || + affiliations.include?('STUDENT-TYPE-REGISTERED') + end + + def required_attributes_for(affiliations) + required_cal_attrs = CALNET_ATTRS.dup + + # only employee afflication will validate employee_id and ucpath_id attributes. + unless employee_affiliated?(affiliations) + required_cal_attrs.delete(:employee_id) + required_cal_attrs.delete(:ucpath_id) + + end + + # only student registered and not-registered affiliation will validate student_id attribute. + unless student_affiliated?(affiliations) + required_cal_attrs.delete(:student_id) + end + + required_cal_attrs.values + end + + def present_in_auth_extra?(auth_extra, attr) + if attr.is_a?(Array) + attr.any? { |a| auth_extra.key?(a) } + else + auth_extra.key?(attr) + end + end + # Gets an attribute value from auth_extra, handling both string and array attribute names # If attribute is an array, tries each key in order and returns the first match # If attribute is a string, returns the value for that key diff --git a/config/application.rb b/config/application.rb index 2f5ffbaa..021f56f3 100644 --- a/config/application.rb +++ b/config/application.rb @@ -115,9 +115,9 @@ def log_active_storage_root!(active_storage_root) # Maps hash values to CalNet attribute name(s) # Array values indicate fallback/alternative attribute names config.calnet_attrs = { - affiliations: 'berkeleyEduAffiliations', - cs_id: %w[berkeleyEduStuID berkeleyEduCSID], + cs_id: 'berkeleyEduCSID', ucpath_id: 'berkeleyEduUCPathID', + student_id: 'berkeleyEduStuID', email: %w[berkeleyEduAlternateID berkeleyEduAlternateId], department_number: 'departmentNumber', display_name: 'displayName', From 0a0c50ab9b9e0946c4fb479189b52a7386274f3c Mon Sep 17 00:00:00 2001 From: zhouyu Date: Fri, 20 Feb 2026 13:31:28 -0800 Subject: [PATCH 10/14] Update tests --- app/models/user.rb | 1 + config/application.rb | 1 + spec/models/user_spec.rb | 74 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index adda218d..05da713b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -101,6 +101,7 @@ def student_affiliated?(affiliations) def required_attributes_for(affiliations) required_cal_attrs = CALNET_ATTRS.dup + required_cal_attrs.delete(:affiliations) # only employee afflication will validate employee_id and ucpath_id attributes. unless employee_affiliated?(affiliations) diff --git a/config/application.rb b/config/application.rb index 021f56f3..1cb6ba74 100644 --- a/config/application.rb +++ b/config/application.rb @@ -115,6 +115,7 @@ def log_active_storage_root!(active_storage_root) # Maps hash values to CalNet attribute name(s) # Array values indicate fallback/alternative attribute names config.calnet_attrs = { + affiliations: 'berkeleyEduAffiliations', cs_id: 'berkeleyEduCSID', ucpath_id: 'berkeleyEduUCPathID', student_id: 'berkeleyEduStuID', diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2c1107d6..3c9a9292 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -76,7 +76,7 @@ expect(user.email).to eq('expected email') expect(user.employee_id).to eq('expected employee ID') expect(user.given_name).to eq('expected given name') - expect(user.student_id).to eq('expected cs id') + expect(user.student_id).to eq(nil) expect(user.surname).to eq('expected surname') expect(user.ucpath_id).to eq('expected UC Path ID') expect(user.uid).to eq('expected UID') @@ -108,7 +108,7 @@ expect(user.email).to eq('expected email') expect(user.employee_id).to eq('expected employee ID') expect(user.given_name).to eq('expected given name') - expect(user.student_id).to eq('expected cs id') + expect(user.student_id).to eq(nil) expect(user.surname).to eq('expected surname') expect(user.ucpath_id).to eq('expected UC Path ID') expect(user.uid).to eq('expected UID') @@ -129,4 +129,74 @@ end end + describe :verify_calnet_attributes! do + it 'allows employee-affiliated users without berkeleyEduStuID' do + auth_extra = { + 'berkeleyEduAffiliations' => ['EMPLOYEE-TYPE-ACADEMIC'], + 'berkeleyEduCSID' => 'cs123', + 'berkeleyEduIsMemberOf' => [], + 'berkeleyEduUCPathID' => 'ucpath456', + 'berkeleyEduAlternateID' => 'email@berkeley.edu', + 'departmentNumber' => 'dept1', + 'displayName' => 'Test Faculty', + 'employeeNumber' => 'emp789', + 'givenName' => 'Test', + 'surname' => 'Faculty', + 'uid' => 'faculty1' + } + + expect { User.from_omniauth({ 'provider' => 'calnet', 'extra' => auth_extra }) }.not_to raise_error + end + + it 'allows student-affiliated users without employeeNumber and berkeleyEduUCPathID' do + auth_extra = { + 'berkeleyEduAffiliations' => ['STUDENT-TYPE-REGISTERED'], + 'berkeleyEduCSID' => 'cs123', + 'berkeleyEduIsMemberOf' => [], + 'berkeleyEduStuID' => 'stu456', + 'berkeleyEduAlternateID' => 'email@berkeley.edu', + 'departmentNumber' => 'dept1', + 'displayName' => 'Test Student', + 'givenName' => 'Test', + 'surname' => 'Student', + 'uid' => 'student1' + } + + expect { User.from_omniauth({ 'provider' => 'calnet', 'extra' => auth_extra }) }.not_to raise_error + end + + it 'rejects student-affiliated users if berkeleyEduStuID is missing' do + auth_extra = { + 'berkeleyEduAffiliations' => ['STUDENT-TYPE-REGISTERED'], + 'berkeleyEduCSID' => 'cs123', + 'berkeleyEduIsMemberOf' => [], + 'berkeleyEduAlternateID' => 'email@berkeley.edu', + 'departmentNumber' => 'dept1', + 'displayName' => 'Test Student', + 'givenName' => 'Test', + 'surname' => 'Student', + 'uid' => 'student1' + } + + expect { User.from_omniauth({ 'provider' => 'calnet', 'extra' => auth_extra }) }.to raise_error(Error::CalnetError) + end + + it 'rejects employee-affiliated users if employeeNumber is missing' do + auth_extra = { + 'berkeleyEduAffiliations' => ['EMPLOYEE-TYPE-STAFF'], + 'berkeleyEduCSID' => 'cs123', + 'berkeleyEduIsMemberOf' => [], + 'berkeleyEduUCPathID' => 'ucpath456', + 'berkeleyEduAlternateID' => 'email@berkeley.edu', + 'departmentNumber' => 'dept1', + 'displayName' => 'Test Staff', + 'givenName' => 'Test', + 'surname' => 'Staff', + 'uid' => 'staff1' + } + + expect { User.from_omniauth({ 'provider' => 'calnet', 'extra' => auth_extra }) }.to raise_error(Error::CalnetError) + end + end + end From 5713176e73dbddab2e7d4e2977644ff3c7b913d7 Mon Sep 17 00:00:00 2001 From: zhouyu Date: Fri, 20 Feb 2026 13:35:23 -0800 Subject: [PATCH 11/14] format --- app/models/user.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 05da713b..ebe34131 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -58,7 +58,7 @@ def auth_params_from(auth) def verify_calnet_attributes!(auth_extra) affiliations = affiliations_from(auth_extra) raise_missing_calnet_attribute_error(auth_extra, ['berkeleyEduAffiliations']) if affiliations.blank? - + required_attributes = required_attributes_for(affiliations) missing = required_attributes.reject do |attr| @@ -107,13 +107,10 @@ def required_attributes_for(affiliations) unless employee_affiliated?(affiliations) required_cal_attrs.delete(:employee_id) required_cal_attrs.delete(:ucpath_id) - end - + # only student registered and not-registered affiliation will validate student_id attribute. - unless student_affiliated?(affiliations) - required_cal_attrs.delete(:student_id) - end + required_cal_attrs.delete(:student_id) unless student_affiliated?(affiliations) required_cal_attrs.values end From b4321667da7a73e3a4c53bff5675ce0c1ac17558 Mon Sep 17 00:00:00 2001 From: zhouyu Date: Fri, 20 Feb 2026 13:38:27 -0800 Subject: [PATCH 12/14] remove test method --- app/models/user.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index ebe34131..3c783511 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -29,7 +29,6 @@ def from_omniauth(auth) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength def auth_params_from(auth) auth_extra = auth['extra'] - # log_auth_extra(auth_extra) verify_calnet_attributes!(auth_extra) cal_groups = auth_extra['berkeleyEduIsMemberOf'] || [] @@ -78,13 +77,6 @@ def raise_missing_calnet_attribute_error(auth_extra, missing) raise Error::CalnetError, msg end - # def log_auth_extra(auth_extra) - # return if auth_extra.nil? - - # keys = auth_extra.keys.reject { |k| k.start_with?('duo') }.sort - # Rails.logger.info("CalNet auth_extra keys!!! student 5 - #{auth_extra['berkeleyEduAffiliations']}: #{keys.join(', ')}") - # end - def affiliations_from(auth_extra) Array(auth_extra['berkeleyEduAffiliations']) end From 3b513b174e31bb655469a924289feedbc141ec7b Mon Sep 17 00:00:00 2001 From: zhouyu Date: Fri, 20 Feb 2026 14:47:09 -0800 Subject: [PATCH 13/14] Temporarily fix rubocop violation --- app/models/user.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index 3c783511..e566a341 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,7 @@ # Represents a user in our system # # This is closely coupled to CalNet's user schema. +# rubocop:disable Metrics/ClassLength class User include ActiveModel::Model @@ -226,3 +227,4 @@ def find_primary_record uid_patron_record end end +# rubocop:enable Metrics/ClassLength From 42d670a342823bba0fcff30297442e59f7fdd457 Mon Sep 17 00:00:00 2001 From: zhouyu Date: Fri, 20 Feb 2026 16:13:47 -0800 Subject: [PATCH 14/14] fix test and rubocop issue --- app/models/user.rb | 1 - spec/models/user_spec.rb | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index 963e66f3..e566a341 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -27,7 +27,6 @@ def from_omniauth(auth) private - # rubocop:disable Metrics/AbcSize, Metrics/MethodLength # rubocop:disable Metrics/AbcSize, Metrics/MethodLength def auth_params_from(auth) auth_extra = auth['extra'] diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 22912bec..d55eb6ec 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -129,6 +129,7 @@ 'berkeleyEduStuID' => 'expected student ID', 'surname' => 'expected surname', 'berkeleyEduUCPathID' => 'expected UC Path ID', + 'berkeleyEduCSID' => 'expected cs id', 'uid' => 'expected UID' } }