AP-610 calnet attribute validate is too strict

Author: davezuckerman

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) }

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'