diff --git a/app/models/concerns/calnet_authentication.rb b/app/models/concerns/calnet_authentication.rb index dcf1f498..0b39ecb5 100644 --- a/app/models/concerns/calnet_authentication.rb +++ b/app/models/concerns/calnet_authentication.rb @@ -51,63 +51,24 @@ def auth_params_from(auth) end # rubocop:enable Metrics/AbcSize, Metrics/MethodLength - # Verifies that auth_extra contains all required CalNet attributes with exact case-sensitive names + # Verifies that auth_extra contains the required email CalNet attribute # For array attributes, at least one value in the array must be present in auth_extra - # Not all users have the same attributes, so the required attributes are determined based on the user's affiliations (e.g. employee vs student) - # Raise [Error::CalnetError] if any required attributes are missing + # Raise [Error::CalnetError] if the email attribute is missing def verify_calnet_attributes!(auth_extra) - affiliations = affiliations_from(auth_extra) - raise_missing_calnet_attribute_error(auth_extra, ['berkeleyEduAffiliations']) if affiliations.blank? + email_attrs = CALNET_ATTRS[:email] + return if present_in_auth_extra?(auth_extra, email_attrs) - required_attributes = required_attributes_for(affiliations) - - missing = required_attributes.reject do |attr| - present_in_auth_extra?(auth_extra, attr) - end - - return if missing.empty? - - raise_missing_calnet_attribute_error(auth_extra, missing) + raise_missing_calnet_attribute_error(auth_extra, Array(email_attrs)) end 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['uid']}" Rails.logger.error(msg) raise Error::CalnetError, msg 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 - required_cal_attrs.delete(:affiliations) - - # 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. - required_cal_attrs.delete(:student_id) unless student_affiliated?(affiliations) - - 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) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 86f6ed98..8bf5e353 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -43,7 +43,7 @@ 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" + msg = "Expected CalNet attribute(s) not found (case-sensitive): #{missing.join(', ')}. The actual CalNet attributes: #{actual.join(', ')}. The user is expected UID" # rubocop:enable Layout/LineLength expect { User.from_omniauth(auth) }.to raise_error(Error::CalnetError, msg) end @@ -163,14 +163,13 @@ end describe :verify_calnet_attributes! do - it 'allows employee-affiliated users without berkeleyEduStuID' do + it 'allows users without departmentNumber' 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', @@ -181,14 +180,12 @@ expect { User.from_omniauth({ 'provider' => 'calnet', 'extra' => auth_extra }) }.not_to raise_error end - it 'allows student-affiliated users without employeeNumber and berkeleyEduUCPathID' do + it 'allows users without berkeleyEduStuID, employeeNumber, or 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', @@ -198,31 +195,29 @@ expect { User.from_omniauth({ 'provider' => 'calnet', 'extra' => auth_extra }) }.not_to raise_error end - it 'rejects student-affiliated users if berkeleyEduStuID is missing' do + it 'allows users without berkeleyEduAffiliations' do auth_extra = { - 'berkeleyEduAffiliations' => ['STUDENT-TYPE-REGISTERED'], 'berkeleyEduCSID' => 'cs123', 'berkeleyEduIsMemberOf' => [], 'berkeleyEduAlternateID' => 'email@berkeley.edu', - 'departmentNumber' => 'dept1', - 'displayName' => 'Test Student', + 'displayName' => 'Test User', 'givenName' => 'Test', - 'surname' => 'Student', - 'uid' => 'student1' + 'surname' => 'User', + 'uid' => 'user1' } - expect { User.from_omniauth({ 'provider' => 'calnet', 'extra' => auth_extra }) }.to raise_error(Error::CalnetError) + expect { User.from_omniauth({ 'provider' => 'calnet', 'extra' => auth_extra }) }.not_to raise_error end - it 'rejects employee-affiliated users if employeeNumber is missing' do + it 'rejects users if email attribute is missing' do auth_extra = { 'berkeleyEduAffiliations' => ['EMPLOYEE-TYPE-STAFF'], 'berkeleyEduCSID' => 'cs123', 'berkeleyEduIsMemberOf' => [], 'berkeleyEduUCPathID' => 'ucpath456', - 'berkeleyEduAlternateID' => 'email@berkeley.edu', 'departmentNumber' => 'dept1', 'displayName' => 'Test Staff', + 'employeeNumber' => 'emp789', 'givenName' => 'Test', 'surname' => 'Staff', 'uid' => 'staff1'