Skip to content

Commit

Permalink
Perform ial and aal determination in the idp app
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 committed Sep 3, 2024
1 parent 19a0d92 commit 806feef
Show file tree
Hide file tree
Showing 9 changed files with 104 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
23 changes: 15 additions & 8 deletions app/models/federated_protocols/saml.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

module FederatedProtocols
class Saml
IAL_PREFIX = %r{^http://idmanagement.gov/ns/assurance/ial}
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 +17,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)
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 +54,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 +88,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 @@ -5845,7 +5845,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 @@ -5890,7 +5890,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 @@ -979,9 +979,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 806feef

Please sign in to comment.