Skip to content

Commit

Permalink
Perform ial and aal determination in the idp app (#11160)
Browse files Browse the repository at this point in the history
This was unnecessarily leaking into the saml_idp gem.
A separate PR (18F/saml_idp#117) in
that repo will remove these concerns from there.

See https://gitlab.login.gov/lg-people/Melba/backlog-fy24/-/issues/95

changelog: Internal, Refactoring, Refactor saml_request.requested_ial_authn_context calls to single place
  • Loading branch information
vrajmohan authored Sep 3, 2024
1 parent 652f19e commit 6e8b29e
Show file tree
Hide file tree
Showing 9 changed files with 105 additions and 38 deletions.
6 changes: 1 addition & 5 deletions app/controllers/concerns/saml_idp_auth_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,11 @@ def response_authn_context
if saml_request.requested_vtr_authn_contexts.present?
resolved_authn_context_result.expanded_component_values
else
saml_request.requested_aal_authn_context ||
FederatedProtocols::Saml.new(saml_request).aal ||
default_aal_context
end
end

def requested_ial_authn_context
saml_request.requested_ial_authn_context || default_ial_context
end

def link_identity_from_session_data
IdentityLinker.
new(current_user, saml_request_service_provider).
Expand Down
14 changes: 8 additions & 6 deletions app/controllers/saml_idp_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def log_external_saml_auth_request
analytics.saml_auth_request(
requested_ial: requested_ial,
authn_context: saml_request&.requested_authn_contexts,
requested_aal_authn_context: saml_request&.requested_aal_authn_context,
requested_aal_authn_context: FederatedProtocols::Saml.new(saml_request).aal,
requested_vtr_authn_contexts: saml_request&.requested_vtr_authn_contexts.presence,
force_authn: saml_request&.force_authn?,
final_auth_request: sp_session[:final_auth_request],
Expand All @@ -156,11 +156,13 @@ def log_external_saml_auth_request
end

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

saml_request&.requested_ial_authn_context || 'none'
saml_protocol = FederatedProtocols::Saml.new(saml_request)
requested_ial_acr = saml_protocol.ial
if requested_ial_acr == ::Saml::Idp::Constants::IALMAX_AUTHN_CONTEXT_CLASSREF
return 'ialmax'
else
saml_protocol.requested_ial_authn_context.presence || 'none'
end
end

def handle_successful_handoff
Expand Down
4 changes: 0 additions & 4 deletions app/models/federated_protocols/oidc.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ def requested_attributes
OpenidConnectAttributeScoper.new(request.scope).requested_attributes
end

def biometric_comparison_required?
request.biometric_comparison_required?
end

def service_provider
request.service_provider
end
Expand Down
24 changes: 16 additions & 8 deletions app/models/federated_protocols/saml.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

module FederatedProtocols
class Saml
IAL_PREFIX = %r{^http://idmanagement.gov/ns/assurance/ial}
LOA_PREFIX = %r{^http://idmanagement.gov/ns/assurance/loa}
AAL_PREFIX = %r{^http://idmanagement.gov/ns/assurance/aal|urn:gov:gsa:ac:classes:sp:PasswordProtectedTransport:duo}

def initialize(request)
@request = request
end
Expand All @@ -14,12 +18,20 @@ def ial
if ialmax_requested_with_authn_context_comparison?
::Saml::Idp::Constants::IALMAX_AUTHN_CONTEXT_CLASSREF
else
request.requested_ial_authn_context || default_authn_context
requested_ial_authn_context || default_ial_authn_context
end
end

def requested_ial_authn_context
request.requested_authn_contexts.find do |classref|
IAL_PREFIX.match?(classref) || LOA_PREFIX.match?(classref)
end
end

def aal
request.requested_aal_authn_context
request.requested_authn_contexts.find do |classref|
AAL_PREFIX.match?(classref)
end
end

def acr_values
Expand All @@ -43,15 +55,11 @@ def service_provider
current_service_provider
end

def biometric_comparison_required?
false
end

private

attr_reader :request

def default_authn_context
def default_ial_authn_context
if current_service_provider&.ial
::Saml::Idp::Constants::AUTHN_CONTEXT_IAL_TO_CLASSREF[current_service_provider.ial]
else
Expand Down Expand Up @@ -81,7 +89,7 @@ def current_service_provider
def ialmax_requested_with_authn_context_comparison?
return unless (current_service_provider&.ial || 1) > 1

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

!acr_component_value.requirements.include?(:identity_proofing) &&
Expand Down
4 changes: 2 additions & 2 deletions app/services/analytics_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5873,7 +5873,7 @@ def rules_of_use_visit
# @param [String] endpoint
# @param [Boolean] idv
# @param [Boolean] finish_profile
# @param [Integer] requested_ial
# @param [String] requested_ial
# @param [Boolean] request_signed
# @param [String] matching_cert_serial
# matches the request certificate in a successful, signed request
Expand Down Expand Up @@ -5918,7 +5918,7 @@ def saml_auth(
)
end

# @param [Integer] requested_ial
# @param [String] requested_ial
# @param [Array] authn_context
# @param [String, nil] requested_aal_authn_context
# @param [String, nil] requested_vtr_authn_contexts
Expand Down
16 changes: 10 additions & 6 deletions app/services/attribute_asserter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def build
add_vot(attrs)
else
add_aal(attrs)
add_ial(attrs) if authn_request.requested_ial_authn_context || !service_provider.ial.nil?
add_ial(attrs)
end

add_x509(attrs) if bundle.include?(:x509_presented) && x509_data
Expand Down Expand Up @@ -145,7 +145,7 @@ def add_vot(attrs)
end

def add_aal(attrs)
requested_context = authn_request.requested_aal_authn_context
requested_context = requested_aal_authn_context
requested_aal_level = Saml::Idp::Constants::AUTHN_CONTEXT_CLASSREF_TO_AAL[requested_context]
aal_level = requested_aal_level || service_provider.default_aal || ::Idp::Constants::DEFAULT_AAL
context = Saml::Idp::Constants::AUTHN_CONTEXT_AAL_TO_CLASSREF[aal_level]
Expand Down Expand Up @@ -220,12 +220,16 @@ def bundle
).map(&:to_sym)
end

def authn_request_bundle
SamlRequestParser.new(authn_request).requested_attributes
def requested_ial_authn_context
FederatedProtocols::Saml.new(authn_request).requested_ial_authn_context
end

def requested_aal_authn_context
FederatedProtocols::Saml.new(authn_request).aal
end

def authn_context
authn_request.requested_ial_authn_context
def authn_request_bundle
SamlRequestParser.new(authn_request).requested_attributes
end

def x509_data
Expand Down
3 changes: 0 additions & 3 deletions spec/features/idv/analytics_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -981,9 +981,6 @@ def wait_for_event(event, wait)

context 'Happy selfie path' do
before do
allow_any_instance_of(FederatedProtocols::Oidc).
to receive(:biometric_comparison_required?).
and_return(true)
allow_any_instance_of(DocAuth::Response).to receive(:selfie_status).and_return(:success)

perform_in_browser(:desktop) do
Expand Down
4 changes: 0 additions & 4 deletions spec/features/idv/hybrid_mobile/hybrid_mobile_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -362,10 +362,6 @@
end

context 'barcode read error on desktop, redo document capture on mobile' do
before do
allow_any_instance_of(FederatedProtocols::Oidc).
to receive(:biometric_comparison_required?).and_return(true)
end
it 'continues to ssn on desktop when user selects Continue', js: true do
user = nil

Expand Down
68 changes: 68 additions & 0 deletions spec/models/federated_protocols/saml_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
require 'rails_helper'

module FederatedProtocols
RSpec.describe Saml do
let(:authn_contexts) { [] }
let(:saml_request) { instance_double(SamlIdp::Request) }

subject { FederatedProtocols::Saml.new saml_request }

before do
allow(saml_request).to receive(:requested_authn_contexts).and_return(authn_contexts)
end

describe '#aal' do
context 'when no aal context is requested' do
it 'returns nil' do
expect(subject.aal).to be_nil
end
end

context 'when the only context requested is aal' do
let(:aal) { 'http://idmanagement.gov/ns/assurance/aal/2' }
let(:authn_contexts) { [aal] }

it 'returns the requested aal' do
expect(subject.aal).to eq(aal)
end
end

context 'when multiple contexts are requested including aal' do
let(:aal) { 'http://idmanagement.gov/ns/assurance/aal/2' }
let(:ial) { 'http://idmanagement.gov/ns/assurance/ial/1' }
let(:authn_contexts) { [ial, aal] }

it 'returns the requested aal' do
expect(subject.aal).to eq(aal)
end
end
end

describe '#requested_ial_authn_context' do
context 'when no ial context is requested' do
it 'returns nil' do
expect(subject.requested_ial_authn_context).to be_nil
end
end

context 'when the only context requested is ial' do
let(:ial) { 'http://idmanagement.gov/ns/assurance/ial/2' }
let(:authn_contexts) { [ial] }

it 'returns the requested ial' do
expect(subject.requested_ial_authn_context).to eq(ial)
end
end

context 'when multiple contexts are requested including ial' do
let(:aal) { 'http://idmanagement.gov/ns/assurance/aal/2' }
let(:ial) { 'http://idmanagement.gov/ns/assurance/ial/1' }
let(:authn_contexts) { [ial, aal] }

it 'returns the requested ial' do
expect(subject.requested_ial_authn_context).to eq(ial)
end
end
end
end
end

0 comments on commit 6e8b29e

Please sign in to comment.