Skip to content

AP-610 calnet attribute validate is too strict #32

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
davezuckerman wants to merge 1 commit into from
Closed

AP-610 calnet attribute validate is too strict #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 6 additions & 45 deletions app/models/concerns/calnet_authentication.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand Down
25 changes: 10 additions & 15 deletions spec/models/user_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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'
Expand Down