Skip to content

Fix SAML auth context handling for IAL2 strict request #6253

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

Merged
aduth merged 2 commits into from
Apr 28, 2022
Merged

Fix SAML auth context handling for IAL2 strict request #6253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
848cb9e
Fix SAML auth context handling for IAL2 strict request
aduth Apr 26, 2022
42fa86f
Check explicitly for ial2_strict in SAML requested_attributes
aduth Apr 27, 2022
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
2 changes: 1 addition & 1 deletion app/controllers/saml_idp_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def redirect_to_verification_url
end

def profile_or_identity_needs_verification_or_decryption?
return false unless ial2_requested?
return false unless ial_context.ial2_or_greater?
profile_needs_verification? || identity_needs_verification? || identity_needs_decryption?
end

Expand Down
6 changes: 5 additions & 1 deletion app/presenters/saml_request_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def initialize(request:, service_provider:)
end

def requested_attributes
if ial2_authn_context? || ialmax_authn_context?
if ial2_authn_context? || ial2_strict_authn_context? || ialmax_authn_context?
bundle.map { |attr| ATTRIBUTE_TO_FRIENDLY_NAME_MAP[attr] }.compact.uniq
else
attrs = [:email]
Expand All @@ -40,6 +40,10 @@ def ial2_authn_context?
(Saml::Idp::Constants::IAL2_AUTHN_CONTEXTS & authn_context).present?
end

def ial2_strict_authn_context?
authn_context.include? Saml::Idp::Constants::IAL2_STRICT_AUTHN_CONTEXT_CLASSREF
end

def ialmax_authn_context?
authn_context.include? Saml::Idp::Constants::IALMAX_AUTHN_CONTEXT_CLASSREF
end
Expand Down
42 changes: 33 additions & 9 deletions spec/controllers/saml_idp_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,15 @@ def name_id_version(format_urn)
)
end

context 'with IAL2 and the identity is already verified' do
shared_examples 'a verified identity' do |authn_context, ial|
Copy link
Copy Markdown
Contributor

@solipet solipet Apr 26, 2022

Choose a reason for hiding this comment

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

nice.

let(:ial2_settings) do
saml_settings(
overrides: {
issuer: sp1_issuer,
authn_context: authn_context,
},
)
end
let(:user) { create(:profile, :active, :verified).user }
let(:pii) do
Pii::Attributes.new_from_hash(
Expand All @@ -457,7 +465,7 @@ def name_id_version(format_urn)
ial2_authnrequest = saml_authn_request_url(
overrides: {
issuer: sp1_issuer,
authn_context: Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF,
authn_context: authn_context,
},
)
raw_req = CGI.unescape ial2_authnrequest.split('SAMLRequest').last
Expand All @@ -476,7 +484,7 @@ def name_id_version(format_urn)

before do
stub_sign_in(user)
IdentityLinker.new(user, sp1).link_identity(ial: 2)
IdentityLinker.new(user, sp1).link_identity(ial: ial)
user.identities.last.update!(
verified_attributes: %w[given_name family_name social_security_number address],
)
Expand All @@ -491,9 +499,9 @@ def name_id_version(format_urn)
saml_get_auth(ial2_settings)
end

it 'sets identity ial to 2' do
it 'sets identity ial' do
saml_get_auth(ial2_settings)
expect(user.identities.last.ial).to eq(2)
expect(user.identities.last.ial).to eq(ial)
end

it 'does not redirect the user to the IdV URL' do
Expand Down Expand Up @@ -521,23 +529,23 @@ def name_id_version(format_urn)
stub_analytics
expect(@analytics).to receive(:track_event).
with('SAML Auth Request',
requested_ial: Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF,
requested_ial: authn_context,
service_provider: sp1_issuer)
expect(@analytics).to receive(:track_event).
with(Analytics::SAML_AUTH,
success: true,
errors: {},
nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT,
authn_context: ['http://idmanagement.gov/ns/assurance/ial/2'],
authn_context: [authn_context],
authn_context_comparison: 'exact',
requested_ial: Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF,
requested_ial: authn_context,
service_provider: sp1_issuer,
endpoint: '/api/saml/auth2022',
idv: false,
finish_profile: false)
expect(@analytics).to receive(:track_event).
with(Analytics::SP_REDIRECT_INITIATED,
ial: 2)
ial: ial)

allow(controller).to receive(:identity_needs_verification?).and_return(false)
saml_get_auth(ial2_settings)
Expand All @@ -553,6 +561,22 @@ def name_id_version(format_urn)
end
end

context 'with IAL2 and the identity is already verified' do
it_behaves_like 'a verified identity',
Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF,
Idp::Constants::IAL2
end

context 'with IAL2 strict and the identity is already verified' do
before do
allow(IdentityConfig.store).to receive(:liveness_checking_enabled).and_return(true)
end

it_behaves_like 'a verified identity',
Saml::Idp::Constants::IAL2_STRICT_AUTHN_CONTEXT_CLASSREF,
Idp::Constants::IAL2_STRICT
end

context 'with IAL2 and the identity is not already verified' do
it 'redirects to IdV URL for IAL2 proofer' do
user = create(:user, :signed_up)
Expand Down