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
8 changes: 8 additions & 0 deletions app/controllers/openid_connect/authorization_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ def pre_validate_authorize_form
user_fully_authenticated: user_fully_authenticated?,
referer: request.referer,
vtr_param: params[:vtr],
unknown_authn_contexts:,
),
)
return if result.success?
Expand Down Expand Up @@ -258,5 +259,12 @@ def redirect_user(redirect_uri, issuer, user_uuid)
def sp_handoff_bouncer
@sp_handoff_bouncer ||= SpHandoffBouncer.new(sp_session)
end

def unknown_authn_contexts
return nil if params[:vtr].present? || params[:acr_values].blank?

(params[:acr_values].split - Saml::Idp::Constants::VALID_AUTHN_CONTEXTS).
join(' ').presence
end
end
end
25 changes: 24 additions & 1 deletion app/controllers/saml_idp_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ def capture_analytics
request_signed: saml_request.signed?,
matching_cert_serial:,
requested_nameid_format: saml_request.name_id_format,
unknown_authn_contexts:,
)

if result.success? && saml_request.signed?
Expand All @@ -151,12 +152,13 @@ def log_external_saml_auth_request

analytics.saml_auth_request(
requested_ial: requested_ial,
authn_context: saml_request&.requested_authn_contexts,
authn_context: requested_authn_contexts,
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],
service_provider: saml_request&.issuer,
unknown_authn_contexts:,
user_fully_authenticated: user_fully_authenticated?,
)
end
Expand Down Expand Up @@ -227,4 +229,25 @@ def resolved_authn_context_int_ial
def require_path_year
render_not_found if params[:path_year].blank?
end

def unknown_authn_contexts
return nil if saml_request.requested_vtr_authn_contexts.present?
return nil if requested_authn_contexts.blank?

unmatched_authn_contexts.reject do |authn_context|
authn_context.match(req_attrs_regexp)
end.join(' ').presence
end

def unmatched_authn_contexts
requested_authn_contexts - Saml::Idp::Constants::VALID_AUTHN_CONTEXTS
end

def requested_authn_contexts
@request_authn_contexts || saml_request&.requested_authn_contexts
end

def req_attrs_regexp
Regexp.escape(Saml::Idp::Constants::REQUESTED_ATTRIBUTES_CLASSREF)
end
end
9 changes: 9 additions & 0 deletions app/services/analytics_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5498,6 +5498,7 @@ def openid_connect_bearer_token(success:, ial:, client_id:, errors:, error_detai
# @param [String, nil] vtr_param
# @param [Boolean] unauthorized_scope
# @param [Boolean] user_fully_authenticated
# @param [String] unknown_authn_contexts space separated list of unknown contexts
def openid_connect_request_authorization(
success:,
errors:,
Expand All @@ -5514,6 +5515,7 @@ def openid_connect_request_authorization(
unauthorized_scope:,
user_fully_authenticated:,
error_details: nil,
unknown_authn_contexts: nil,
**extra
)
track_event(
Expand All @@ -5533,6 +5535,7 @@ def openid_connect_request_authorization(
vtr_param:,
unauthorized_scope:,
user_fully_authenticated:,
unknown_authn_contexts:,
**extra,
)
end
Expand Down Expand Up @@ -6334,6 +6337,7 @@ def rules_of_use_visit
# matches the request certificate in a successful, signed request
# @param [Hash] cert_error_details Details for errors that occurred because of an invalid
# signature
# @param [String] unknown_authn_contexts space separated list of unknown contexts
def saml_auth(
success:,
errors:,
Expand All @@ -6350,6 +6354,7 @@ def saml_auth(
matching_cert_serial:,
error_details: nil,
cert_error_details: nil,
unknown_authn_contexts: nil,
**extra
)
track_event(
Expand All @@ -6369,6 +6374,7 @@ def saml_auth(
request_signed:,
matching_cert_serial:,
cert_error_details:,
unknown_authn_contexts:,
**extra,
)
end
Expand All @@ -6380,6 +6386,7 @@ def saml_auth(
# @param [Boolean] force_authn
# @param [Boolean] final_auth_request
# @param [String] service_provider
# @param [String] unknown_authn_contexts space separated list of unknown contexts
# @param [Boolean] user_fully_authenticated
# An external request for SAML Authentication was received
def saml_auth_request(
Expand All @@ -6390,6 +6397,7 @@ def saml_auth_request(
force_authn:,
final_auth_request:,
service_provider:,
unknown_authn_contexts:,
user_fully_authenticated:,
**extra
)
Expand All @@ -6402,6 +6410,7 @@ def saml_auth_request(
force_authn:,
final_auth_request:,
service_provider:,
unknown_authn_contexts:,
user_fully_authenticated:,
**extra,
)
Expand Down
5 changes: 4 additions & 1 deletion app/services/saml_request_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ def valid_authn_context?
next true if classref.match?(SamlIdp::Request::VTR_REGEXP) &&
IdentityConfig.store.use_vot_in_sp_requests
end
authn_contexts.all? do |classref|
# SAML requests are allowed to "default" to the integration's IAL default.
return true if authn_contexts.empty?

authn_contexts.any? do |classref|
valid_contexts.include?(classref)
end
end
Expand Down
15 changes: 1 addition & 14 deletions app/services/vot/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ module Vot
class Parser
class ParseException < StandardError; end

class UnsupportedComponentsException < ParseException; end

class DuplicateComponentsException < ParseException; end

Result = Data.define(
Expand Down Expand Up @@ -87,8 +85,7 @@ def initial_components
@initial_components ||= component_string.split(component_separator).map do |component_name|
component_map.fetch(component_name)
rescue KeyError
raise_unsupported_component_exception(component_name)
end
end.compact
end

def component_separator
Expand All @@ -113,16 +110,6 @@ def validate_component_uniqueness!(component_values)
end
end

def raise_unsupported_component_exception(component_value_name)
if vector_of_trust.present?
raise UnsupportedComponentsException,
"'#{vector_of_trust}' contains unknown component '#{component_value_name}'"
else
raise UnsupportedComponentsException,
"'#{acr_values}' contains unknown acr value '#{component_value_name}'"
end
end

def raise_duplicate_component_exception
if vector_of_trust.present?
raise DuplicateComponentsException, "'#{vector_of_trust}' contains duplicate components"
Expand Down
27 changes: 26 additions & 1 deletion spec/controllers/application_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ def index
let(:vtr) { nil }
let(:acr_values) do
[
'http://idmanagement.gov/ns/assurance/aal/1',
Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF,
].join(' ')
end

Expand All @@ -486,6 +486,31 @@ def index
expect(result.identity_proofing?).to eq(true)
end

context 'when an unknown acr value is passed in' do
let(:acr_values) { 'unknown-acr-value' }

it 'raises an exception' do
expect { result }.to raise_exception(
Vot::Parser::ParseException,
'VoT parser called without VoT or ACR values',
)
end

context 'with a known acr value' do
let(:acr_values) do
[
Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF,
'unknown-acr-value',
].join(' ')
end

it 'returns a resolved authn context result' do
expect(result.aal2?).to eq(true)
expect(result.identity_proofing?).to eq(true)
end
end
end

context 'without an SP' do
let(:sp) { nil }
let(:sp_session) { nil }
Expand Down
59 changes: 59 additions & 0 deletions spec/controllers/openid_connect/authorization_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2029,6 +2029,65 @@
end
end

context 'when there are unknown acr_values params' do
let(:unknown_value) { 'unknown-acr-value' }
let(:acr_values) { unknown_value }

context 'when there is only an unknown acr_value' do
it 'tracks the event with errors' do
stub_analytics

action

expect(@analytics).to have_logged_event(
'OpenID Connect: authorization request',
success: false,
client_id:,
prompt:,
allow_prompt_login: true,
unauthorized_scope: false,
errors: hash_including(:acr_values),
error_details: hash_including(:acr_values),
user_fully_authenticated: true,
acr_values: '',
code_challenge_present: false,
scope: 'openid profile',
unknown_authn_contexts: unknown_value,
)
end

context 'when there is also a valid acr_value' do
let(:known_value) { Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF }
let(:acr_values) do
[
unknown_value,
known_value,
].join(' ')
end

it 'tracks the event' do
stub_analytics

action
expect(@analytics).to have_logged_event(
'OpenID Connect: authorization request',
success: true,
client_id:,
prompt:,
allow_prompt_login: true,
unauthorized_scope: false,
user_fully_authenticated: true,
acr_values: known_value,
code_challenge_present: false,
scope: 'openid profile',
unknown_authn_contexts: unknown_value,
errors: {},
)
end
end
end
end

context 'vtr with invalid params that do not interfere with the redirect_uri' do
let(:acr_values) { nil }
let(:vtr) { ['C1'].to_json }
Expand Down
53 changes: 50 additions & 3 deletions spec/controllers/saml_idp_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -970,15 +970,22 @@ def name_id_version(format_urn)
end

context 'authn_context is invalid' do
it 'renders an error page' do
let(:unknown_value) do
'http://idmanagement.gov/ns/assurance/loa/5'
end
let(:authn_context) { unknown_value }

before do
stub_analytics

saml_get_auth(
saml_settings(
overrides: { authn_context: 'http://idmanagement.gov/ns/assurance/loa/5' },
overrides: { authn_context: },
),
)
end

it 'renders an error page' do
expect(controller).to render_template('saml_idp/auth/error')
expect(response.status).to eq(400)
expect(response.body).to include(t('errors.messages.unauthorized_authn_context'))
Expand All @@ -989,7 +996,7 @@ def name_id_version(format_urn)
errors: { authn_context: [t('errors.messages.unauthorized_authn_context')] },
error_details: { authn_context: { unauthorized_authn_context: true } },
nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT,
authn_context: ['http://idmanagement.gov/ns/assurance/loa/5'],
authn_context: [unknown_value],
authn_context_comparison: 'exact',
service_provider: 'http://localhost:3000',
request_signed: true,
Expand All @@ -998,9 +1005,49 @@ def name_id_version(format_urn)
idv: false,
finish_profile: false,
matching_cert_serial: saml_test_sp_cert_serial,
unknown_authn_contexts: unknown_value,
),
)
end

context 'there is also a valid authn_context' do
let(:authn_context) do
[
Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF,
unknown_value,
]
end

it 'logs the unknown authn_context value' do
expect(response.status).to eq(302)
expect(@analytics).to have_logged_event(
'SAML Auth Request',
hash_including(
unknown_authn_contexts: unknown_value,
),
)
end

context 'when it includes the ReqAttributes AuthnContext' do
let(:authn_context) do
[
Saml::Idp::Constants::REQUESTED_ATTRIBUTES_CLASSREF,
Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF,
unknown_value,
]
end

it 'logs the unknown authn_context value' do
expect(response.status).to eq(302)
expect(@analytics).to have_logged_event(
'SAML Auth Request',
hash_including(
unknown_authn_contexts: unknown_value,
),
)
end
end
end
end

context 'authn_context scenarios' do
Expand Down
Loading