Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 2 additions & 3 deletions app/controllers/concerns/saml_idp_auth_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def link_identity_from_session_data
IdentityLinker.
new(current_user, saml_request_service_provider).
link_identity(
ial: ial_context.ial,
ial: resolved_authn_context_int_ial,
rails_session_id: session.id,
)
end
Expand All @@ -148,9 +148,8 @@ def identity_needs_verification?

def ial_context
@ial_context ||= IalContext.new(
ial: requested_ial_authn_context,
ial: resolved_authn_context_int_ial,
Comment on lines 150 to +151
Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT of renaming the attribute in the IalContext constructor to be int_ial for clarity?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IalContext actually takes an integer or a string.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am making sure that we pass the integer value here because that will properly compute IALMax. In some IALMax cases the value is the string value for IAL1 with the minimum context comparison.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it!

service_provider: saml_request_service_provider,
authn_context_comparison: saml_request.requested_authn_context_comparison,
user: current_user,
)
end
Expand Down
18 changes: 15 additions & 3 deletions app/controllers/saml_idp_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def set_devise_failure_redirect_for_concurrent_session_logout
end

def pii_requested_but_locked?
if (sp_session && sp_session_ial > 1) || ial_context.ialmax_requested?
if resolved_authn_context_result.identity_proofing_or_ialmax?
current_user.identity_verified? &&
!Pii::Cacher.new(current_user, user_session).exists_in_session?
end
Expand Down Expand Up @@ -146,7 +146,9 @@ def log_external_saml_auth_request
end

def requested_ial
return 'ialmax' if ial_context.ialmax_requested?
requested_ial_acr = FederatedProtocols::Saml.new(saml_request).ial
requested_ial_component = Vot::LegacyComponentValues.by_name[requested_ial_acr]
return 'ialmax' if requested_ial_component&.requirements&.include?(:ialmax)

saml_request&.requested_ial_authn_context || 'none'
end
Expand Down Expand Up @@ -174,9 +176,19 @@ def render_template_for(message, action_url, type)
)
end

def resolved_authn_context_int_ial
if resolved_authn_context_result.ialmax?
0
elsif resolved_authn_context_result.identity_proofing?
2
else
1
end
end

def track_events
analytics.sp_redirect_initiated(
ial: ial_context.ial,
ial: resolved_authn_context_int_ial,
billed_ial: ial_context.bill_for_ial_1_or_2,
sign_in_flow: session[:sign_in_flow],
)
Expand Down
30 changes: 29 additions & 1 deletion app/models/federated_protocols/saml.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ def issuer
end

def ial
request.requested_ial_authn_context || default_authn_context
if ialmax_requested_with_authn_context_comparison?
::Saml::Idp::Constants::IALMAX_AUTHN_CONTEXT_CLASSREF
else
request.requested_ial_authn_context || default_authn_context
end
end

def aal
Expand Down Expand Up @@ -54,5 +58,29 @@ def current_service_provider
return @current_service_provider if defined?(@current_service_provider)
@current_service_provider = ServiceProvider.find_by(issuer: issuer)
end

##
# A ServiceProvider can request an IAL authn context with a mimimum context comparison . In this
# case the IdP is expected to return a result with that IAL or a higher one.
#
# If a SP requests IAL1 with the mimium context comparison then the IdP can response with a
# IAL2 response. In order for this to happen the following need to be true:
#
# - The service provider is authorized to make IAL2 requests
# - The user has a verified account
#
# This methods checks that we are in a situation where the authn context comparison situation
# described above exists and the SP requirements are met (the requirement that the user is
# verified occurs as part of the IALMax functionality).
#
def ialmax_requested_with_authn_context_comparison?
return unless (current_service_provider&.ial || 1) > 1

acr_component_value = Vot::LegacyComponentValues.by_name[request.requested_ial_authn_context]
return unless acr_component_value.present?

!acr_component_value.requirements.include?(:identity_proofing) &&
request.requested_authn_context_comparison == 'minimum'
end
end
end
4 changes: 2 additions & 2 deletions app/presenters/saml_request_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ def ial2_authn_context?
end

def ialmax_authn_context?
ial_context.ialmax_requested?
ial_acr_value = FederatedProtocols::Saml.new(request).ial
Vot::LegacyComponentValues.by_name[ial_acr_value]&.requirements&.include?(:ialmax)
end

def authn_context
Expand All @@ -52,7 +53,6 @@ def ial_context
@ial_context ||= IalContext.new(
ial: request.requested_ial_authn_context || default_ial_context,
service_provider: service_provider,
authn_context_comparison: request.requested_authn_context_comparison,
)
end

Expand Down
22 changes: 18 additions & 4 deletions app/services/attribute_asserter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def build
attrs = default_attrs
add_email(attrs) if bundle.include? :email
add_all_emails(attrs) if bundle.include? :all_emails
add_bundle(attrs) if user.active_profile.present? && ial_context.ial2_or_greater?
add_bundle(attrs) if should_add_proofed_attributes?
add_verified_at(attrs) if bundle.include?(:verified_at) && ial_context.ial2_service_provider?
add_aal(attrs)
add_ial(attrs) if authn_request.requested_ial_authn_context || !service_provider.ial.nil?
Expand All @@ -51,12 +51,21 @@ def build
:decrypted_pii,
:user_session

def should_add_proofed_attributes?
return false if !user.active_profile.present?
ial_context.ial2_or_greater? || ial_max_requested?
end

def ial_max_requested?
ial_acr_value = FederatedProtocols::Saml.new(authn_request).ial
Vot::LegacyComponentValues.by_name[ial_acr_value]&.requirements&.include?(:ialmax)
end

def ial_context
@ial_context ||= IalContext.new(
ial: authn_context,
service_provider: service_provider,
user: user,
authn_context_comparison: authn_request&.requested_authn_context_comparison,
)
end

Expand Down Expand Up @@ -126,14 +135,19 @@ def add_aal(attrs)

def add_ial(attrs)
requested_context = authn_request.requested_ial_authn_context
context = if ial_context.ialmax_requested? && ial_context.ial2_requested?
sp_ial # IAL2 since IALMAX only works for IAL2 SPs
context = if ialmax_requested_and_fullfilable?
# IAL2 since IALMAX only works for IAL2 SPs
sp_ial
else
requested_context.presence || sp_ial
end
attrs[:ial] = { getter: ial_getter_function(context) } if context
end

def ialmax_requested_and_fullfilable?
ial_max_requested? && user.active_profile.present?
end

def sp_ial
Saml::Idp::Constants::AUTHN_CONTEXT_IAL_TO_CLASSREF[service_provider.ial]
end
Expand Down
20 changes: 3 additions & 17 deletions app/services/ial_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,15 @@

# Wraps up logic for querying the IAL level of an authorization request
class IalContext
attr_reader :ial, :service_provider, :user, :authn_context_comparison
attr_reader :ial, :service_provider, :user

# @param ial [String, Integer] IAL level as either an integer (see ::Idp::Constants::IAL2, etc)
# or a string see Saml::Idp::Constants contexts
# @param service_provider [ServiceProvider, nil]
def initialize(ial:, service_provider:, user: nil, authn_context_comparison: nil)
@authn_context_comparison = authn_context_comparison
def initialize(ial:, service_provider:, user: nil)
@service_provider = service_provider
@user = user
@ial = int_ial(ial)
@ial = convert_ial_to_int(ial)
end

def ial2_service_provider?
Expand Down Expand Up @@ -44,19 +43,6 @@ def ial2_or_greater?

private

def int_ial(input)
return 0 if saml_ialmax?(input)

convert_ial_to_int(input)
end

def saml_ialmax?(input)
int_ial_from_request = convert_ial_to_int(input)
return false unless int_ial_from_request.present?

service_provider&.ial == 2 && authn_context_comparison == 'minimum' && int_ial_from_request < 2
end

def convert_ial_to_int(input)
Integer(input)
rescue TypeError # input was nil
Expand Down
4 changes: 4 additions & 0 deletions app/services/vot/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ def self.no_sp_result
ialmax?: false,
)
end

def identity_proofing_or_ialmax?
identity_proofing? || ialmax?
end
end

attr_reader :vector_of_trust, :acr_values
Expand Down
25 changes: 25 additions & 0 deletions spec/presenters/saml_request_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
phone address1 address2 city state zipcode foo
]
service_provider = ServiceProvider.new(attribute_bundle: all_attributes)
allow(request).to receive(
:service_provider,
).and_return(
double(identifier: service_provider.issuer),
)
presenter = SamlRequestPresenter.new(request: request, service_provider: service_provider)

expect(presenter.requested_attributes).to eq(%i[email all_emails verified_at])
Expand All @@ -43,6 +48,11 @@

sp_attributes = %w[email first_name last_name ssn zipcode]
service_provider = ServiceProvider.new(attribute_bundle: sp_attributes, ial: 2)
allow(request).to receive(
:service_provider,
).and_return(
double(identifier: service_provider.issuer),
)
presenter = SamlRequestPresenter.new(request: request, service_provider: service_provider)

expect(presenter.requested_attributes).to eq(
Expand All @@ -67,6 +77,11 @@

sp_attributes = %w[email first_name last_name ssn zipcode all_emails]
service_provider = ServiceProvider.new(attribute_bundle: sp_attributes, ial: 1)
allow(request).to receive(
:service_provider,
).and_return(
double(identifier: service_provider.issuer),
)
presenter = SamlRequestPresenter.new(request: request, service_provider: service_provider)

expect(presenter.requested_attributes).to eq(%i[email all_emails])
Expand All @@ -86,6 +101,11 @@
email first_name last_name dob foo ssn phone verified_at
],
)
allow(request).to receive(
:service_provider,
).and_return(
double(identifier: service_provider.issuer),
)
presenter = SamlRequestPresenter.new(request: request, service_provider: service_provider)
valid_attributes = %i[
email given_name family_name birthdate social_security_number phone verified_at
Expand All @@ -106,6 +126,11 @@
service_provider = ServiceProvider.new(
attribute_bundle: %w[address1 address2 city state zipcode],
)
allow(request).to receive(
:service_provider,
).and_return(
double(identifier: service_provider.issuer),
)
presenter = SamlRequestPresenter.new(request: request, service_provider: service_provider)

expect(presenter.requested_attributes).to eq([:address])
Expand Down
49 changes: 48 additions & 1 deletion spec/services/attribute_asserter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -544,9 +544,14 @@
expected_ial = Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF
expect(user.asserted_attributes[:ial][:getter].call(user)).to eq expected_ial
end

it 'does not include proofed attributes' do
expect(user.asserted_attributes[:first_name]).to eq(nil)
expect(user.asserted_attributes[:phone]).to eq(nil)
end
end

context 'service provider requests IALMAX with IAL2 user' do
context 'IAL2 service provider requests IALMAX with IAL2 user' do
let(:service_provider_ial) { 2 }
let(:subject) do
described_class.new(
Expand All @@ -563,6 +568,7 @@
user.identities << identity
allow(service_provider.metadata).to receive(:[]).with(:attribute_bundle).
and_return(%w[email phone first_name])
ServiceProvider.find_by(issuer: sp1_issuer).update!(ial: 2)
subject.build
end

Expand All @@ -574,6 +580,47 @@
expected_ial = Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF
expect(user.asserted_attributes[:ial][:getter].call(user)).to eq expected_ial
end

it 'includes proofed attributes' do
expect(user.asserted_attributes[:first_name][:getter].call(user)).to eq('Jåné')
expect(user.asserted_attributes[:phone][:getter].call(user)).to eq('+18888675309')
end
end
end

context 'non-IAL2 service provider requests IALMAX with IAL2 user' do
let(:service_provider_ial) { 1 }
let(:subject) do
described_class.new(
user: user,
name_id_format: name_id_format,
service_provider: service_provider,
authn_request: ialmax_authn_request,
decrypted_pii: decrypted_pii,
user_session: user_session,
)
end

before do
user.identities << identity
allow(service_provider.metadata).to receive(:[]).with(:attribute_bundle).
and_return(%w[email phone first_name])
ServiceProvider.find_by(issuer: sp1_issuer).update!(ial: 1)
subject.build
end

it 'includes ial' do
expect(user.asserted_attributes.keys).to include(:ial)
end

it 'creates a getter function for ial attribute' do
expected_ial = Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF
expect(user.asserted_attributes[:ial][:getter].call(user)).to eq expected_ial
end

it 'includes proofed attributes' do
expect(user.asserted_attributes[:first_name]).to eq(nil)
expect(user.asserted_attributes[:phone]).to eq(nil)
end
end

Expand Down
Loading