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
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ gem 'rqrcode'
gem 'ruby-progressbar'
gem 'ruby-saml'
gem 'safe_target_blank', '>= 1.0.2'
gem 'saml_idp', github: '18F/saml_idp', tag: '0.18.3-18f'
gem 'saml_idp', github: '18F/saml_idp', tag: '0.19.1-18f'
gem 'scrypt'
gem 'simple_form', '>= 5.0.2'
gem 'stringex', require: false
Expand Down
6 changes: 3 additions & 3 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ GIT

GIT
remote: https://github.com/18F/saml_idp.git
revision: 26d550cd249e52304aecbb53add32cbec4001e2f
tag: 0.18.3-18f
revision: f7dd59de0c860e9d78ab4f05e8365153a2a9b25a
tag: 0.19.1-18f
specs:
saml_idp (0.18.3.pre.18f)
saml_idp (0.19.1.pre.18f)
activesupport
builder
faraday
Expand Down
20 changes: 9 additions & 11 deletions app/controllers/concerns/saml_idp_auth_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module SamlIdpAuthConcern
# rubocop:disable Rails/LexicallyScopedActionFilter
before_action :validate_and_create_saml_request_object, only: :auth
before_action :validate_service_provider_and_authn_context, only: :auth
before_action :block_biometric_requests_in_production, only: :auth
before_action :check_sp_active, only: :auth
before_action :log_external_saml_auth_request, only: [:auth]
# this must take place _before_ the store_saml_request action or the SAML
Expand All @@ -19,6 +20,13 @@ module SamlIdpAuthConcern

private

def block_biometric_requests_in_production
Comment thread
solipet marked this conversation as resolved.
if @saml_request_validator.parsed_vector_of_trust&.biometric_comparison? &&
!FeatureManagement.idv_allow_selfie_check?
render_not_acceptable
end
end

def sign_out_if_forceauthn_is_true_and_user_is_signed_in
if !saml_request.force_authn?
set_issuer_forced_reauthentication(
Expand Down Expand Up @@ -139,21 +147,11 @@ def link_identity_from_session_data
end

def identity_needs_verification?
ial2_requested? &&
resolved_authn_context_result.identity_proofing? &&
(current_user.identity_not_verified? ||
current_user.reproof_for_irs?(service_provider: current_sp))
end

def_delegators :ial_context, :ial2_requested?

def ial_context
@ial_context ||= IalContext.new(
ial: resolved_authn_context_int_ial,
service_provider: saml_request_service_provider,
user: current_user,
)
end

def active_identity
current_user.last_identity
end
Expand Down
34 changes: 24 additions & 10 deletions app/controllers/saml_idp_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@ class SamlIdpController < ApplicationController

def auth
capture_analytics
if ial_context.ial2_or_greater?
if resolved_authn_context_result.identity_proofing?
return redirect_to reactivate_account_url if user_needs_to_reactivate_account?
return redirect_to url_for_pending_profile_reason if user_has_pending_profile?
return redirect_to idv_url if identity_needs_verification?
return redirect_to idv_url if selfie_needed?
end
return redirect_to sign_up_completed_url if needs_completion_screen_reason
if auth_count == 1 && first_visit_for_sp?
Expand Down Expand Up @@ -109,6 +110,11 @@ def prompt_for_password_if_ial2_request_and_pii_locked
redirect_to capture_password_url
end

def selfie_needed?
decorated_sp_session.selfie_required? &&
!current_user.identity_verified_with_selfie?
end

def set_devise_failure_redirect_for_concurrent_session_logout
request.env['devise_session_limited_failure_redirect_url'] = request.url
end
Expand Down Expand Up @@ -169,6 +175,23 @@ def render_template_for(message, action_url, type)
)
end

def track_events
analytics.sp_redirect_initiated(
ial: resolved_authn_context_int_ial,
billed_ial: ial_context.bill_for_ial_1_or_2,
sign_in_flow: session[:sign_in_flow],
)
track_billing_events
end

def ial_context
@ial_context ||= IalContext.new(
ial: resolved_authn_context_int_ial,
service_provider: saml_request_service_provider,
user: current_user,
)
end

def resolved_authn_context_int_ial
if resolved_authn_context_result.ialmax?
0
Expand All @@ -179,15 +202,6 @@ def resolved_authn_context_int_ial
end
end

def track_events
analytics.sp_redirect_initiated(
ial: resolved_authn_context_int_ial,
billed_ial: ial_context.bill_for_ial_1_or_2,
sign_in_flow: session[:sign_in_flow],
)
track_billing_events
end

def require_path_year
render_not_found if params[:path_year].blank?
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/federated_protocols/saml.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def acr_values
end

def vtr
nil
[request.requested_vtr_authn_context] if request.requested_vtr_authn_context.present?
end

def requested_attributes
Expand Down
43 changes: 33 additions & 10 deletions app/services/saml_request_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class SamlRequestValidator
validate :cert_exists
validate :authorized_service_provider
validate :authorized_authn_context
validate :parsable_vtr
validate :authorized_email_nameid_format

def initialize(blank_cert: false)
Expand All @@ -19,6 +20,16 @@ def call(service_provider:, authn_context:, nameid_format:, authn_context_compar
FormResponse.new(success: valid?, errors: errors, extra: extra_analytics_attributes)
end

def parsed_vector_of_trust
return @parsed_vector_of_trust if defined?(@parsed_vector_of_trust)

@parsed_vector_of_trust = begin
Vot::Parser.new(vector_of_trust: vtr.first).parse if !vtr.blank?
rescue Vot::Parser::ParseException
nil
end
end

private

attr_accessor :service_provider, :authn_context, :authn_context_comparison, :nameid_format
Expand All @@ -44,13 +55,19 @@ def authorized_service_provider

def authorized_authn_context
if !valid_authn_context? ||
(ial2_context_requested? && service_provider&.ial != 2) ||
(identity_proofing_requested? && service_provider&.ial != 2) ||
(ial_max_requested? &&
!IdentityConfig.store.allowed_ialmax_providers.include?(service_provider&.issuer))
errors.add(:authn_context, :unauthorized_authn_context, type: :unauthorized_authn_context)
end
end

def parsable_vtr
if !vtr.blank? && parsed_vector_of_trust.blank?
errors.add(:authn_context, :unauthorized_authn_context, type: :unauthorized_authn_context)
end
end

def cert_exists
if @blank_cert
errors.add(:service_provider, :blank_cert_element_req, type: :blank_cert_element_req)
Expand All @@ -62,7 +79,9 @@ def valid_authn_context?
valid_contexts += Saml::Idp::Constants::PASSWORD_AUTHN_CONTEXT_CLASSREFS if step_up_comparison?

authn_contexts = authn_context.reject do |classref|
classref.include?(Saml::Idp::Constants::REQUESTED_ATTRIBUTES_CLASSREF)
next true if classref.include?(Saml::Idp::Constants::REQUESTED_ATTRIBUTES_CLASSREF)
next true if classref.match?(SamlIdp::Request::VTR_REGEXP) &&
IdentityConfig.store.use_vot_in_sp_requests
end
authn_contexts.all? do |classref|
valid_contexts.include?(classref)
Expand All @@ -73,14 +92,18 @@ def step_up_comparison?
%w[minimum better].include? authn_context_comparison
end

def ial2_context_requested?
case authn_context
when Array
authn_context.any? do |classref|
Saml::Idp::Constants::IAL2_AUTHN_CONTEXTS.include?(classref)
end
else
Saml::Idp::Constants::IAL2_AUTHN_CONTEXTS.include?(authn_context)
def identity_proofing_requested?
return true if parsed_vector_of_trust&.identity_proofing?

authn_context.each do |classref|
return true if Saml::Idp::Constants::IAL2_AUTHN_CONTEXTS.include?(classref)
end
false
end

def vtr
@vtr ||= Array(authn_context).select do |classref|
classref.match?(SamlIdp::Request::VTR_REGEXP)
end
end

Expand Down
125 changes: 117 additions & 8 deletions spec/controllers/saml_idp_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -541,15 +541,124 @@ def name_id_version(format_urn)
)
end

context 'with IAL2 and the identity is already verified' do
let(:ial2_settings) do
saml_settings(
overrides: {
issuer: sp1_issuer,
authn_context: Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF,
},
)
context 'when a request is made with a VTR in the authn context' do
let(:user) { create(:user, :fully_registered) }

before do
allow(IdentityConfig.store).to receive(:use_vot_in_sp_requests).and_return(true)
stub_sign_in(user)
end

context 'the request does not require identity proofing' do
it 'redirects the user' do
vtr_settings = saml_settings(
overrides: {
issuer: sp1_issuer,
authn_context: 'C1',
},
)
saml_get_auth(vtr_settings)
expect(response).to redirect_to(sign_up_completed_url)
expect(controller.session[:sp][:vtr]).to eq(['C1'])
end
end

context 'the request requires identity proofing' do
it 'redirects to identity proofing' do
vtr_settings = saml_settings(
Comment thread
solipet marked this conversation as resolved.
overrides: {
issuer: sp1_issuer,
authn_context: 'C1.C2.P1',
},
)
saml_get_auth(vtr_settings)
expect(response).to redirect_to(idv_url)
expect(controller.session[:sp][:vtr]).to eq(['C1.C2.P1'])
end
end

context 'the request requires identity proofing with a biometric' do
let(:vtr_settings) do
saml_settings(
overrides: {
issuer: sp1_issuer,
authn_context: 'C1.C2.P1.Pb',
},
)
end
let(:pii) do
Pii::Attributes.new_from_hash(
first_name: 'Some',
last_name: 'One',
ssn: '666666666',
)
end
let(:doc_auth_selfie_capture_enabled) { true }

before do
allow(IdentityConfig.store).to receive(
:doc_auth_selfie_capture_enabled,
).and_return(
doc_auth_selfie_capture_enabled,
)
create(:profile, :active, user: user, pii: pii.to_h)
Pii::Cacher.new(user, controller.user_session).save_decrypted_pii(
pii,
user.reload.active_profile.id,
)
end

context 'the user has proofed without a biometric check' do
before do
user.active_profile.update!(idv_level: :legacy_unsupervised)
end

it 'redirects to identity proofing for a user who is verified without a biometric' do
saml_get_auth(vtr_settings)
expect(response).to redirect_to(idv_url)
expect(controller.session[:sp][:vtr]).to eq(['C1.C2.P1.Pb'])
end
end

context 'the user has proofed with a biometric check' do
before do
user.active_profile.update!(idv_level: :unsupervised_with_selfie)
end

it 'does not redirect to proofing' do
saml_get_auth(vtr_settings)
expect(response).to redirect_to(sign_up_completed_url)
expect(controller.session[:sp][:vtr]).to eq(['C1.C2.P1.Pb'])
end
end

context 'selfie check is disabled for the environment' do
let(:doc_auth_selfie_capture_enabled) { false }

it 'renders an error' do
saml_get_auth(vtr_settings)
expect(response.status).to eq(406)
end
end
end

context 'the VTR is not parsable' do
it 'renders an error' do
vtr_settings = saml_settings(
overrides: {
issuer: sp1_issuer,
authn_context: 'Fa.Ke.Va.Lu.E0',
},
)
saml_get_auth(vtr_settings)
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'))
end
end
end

context 'with IAL2 and the identity is already verified' do
let(:user) { create(:profile, :active, :verified).user }
let(:pii) do
Pii::Attributes.new_from_hash(
Expand Down
Loading