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
4 changes: 2 additions & 2 deletions app/controllers/concerns/saml_idp_auth_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ def default_authn_context
end

def default_aal_context
if current_service_provider.aal
Saml::Idp::Constants::AUTHN_CONTEXT_AAL_TO_CLASSREF[current_service_provider.aal]
if current_service_provider.default_aal
Saml::Idp::Constants::AUTHN_CONTEXT_AAL_TO_CLASSREF[current_service_provider.default_aal]
else
Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF
end
Expand Down
2 changes: 1 addition & 1 deletion app/decorators/service_provider_session_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def requested_more_recent_verification?
attr_reader :sp, :view_context, :sp_session, :service_provider_request

def sp_aal
sp.aal || 1
sp.default_aal || 1
end

def sp_ial
Expand Down
1 change: 1 addition & 0 deletions app/models/null_service_provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class NullServiceProvider
block_encryption
cert
created_at
default_aal
description
failure_to_proof_url
fingerprint
Expand Down
8 changes: 7 additions & 1 deletion app/policies/service_provider_mfa_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ def auth_method_confirms_to_sp_request?
end

def aal3_required?
aal3_requested? || service_provider&.aal == 3
return aal3_requested? if aal_requested?

service_provider&.default_aal == 3
end

def piv_cac_required?
Expand All @@ -62,6 +64,10 @@ def aal3_enabled?
piv_cac_enabled? || webauthn_enabled?
end

def aal_requested?
@aal_level_requested.present?
end

def aal3_requested?
@aal_level_requested == 3
end
Expand Down
2 changes: 1 addition & 1 deletion app/services/attribute_asserter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def add_verified_at(attrs)
def add_aal(attrs)
requested_context = authn_request.requested_aal_authn_context
requested_aal_level = Saml::Idp::Constants::AUTHN_CONTEXT_CLASSREF_TO_AAL[requested_context]
aal_level = requested_aal_level || service_provider.aal || ::Idp::Constants::DEFAULT_AAL
aal_level = requested_aal_level || service_provider.default_aal || ::Idp::Constants::DEFAULT_AAL
context = Saml::Idp::Constants::AUTHN_CONTEXT_AAL_TO_CLASSREF[aal_level]
attrs[:aal] = { getter: aal_getter_function(context) } if context
end
Expand Down
4 changes: 2 additions & 2 deletions config/service_providers.localdev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ test:
cert: 'saml_test_sp'
logo: 'generic.svg'
ial: 2
aal: 3
default_aal: 3
attribute_bundle:
- first_name
- last_name
Expand Down Expand Up @@ -183,7 +183,7 @@ test:
friendly_name: 'Test SP'
assertion_consumer_logout_service_url: ''
ial: 2
aal: 3
default_aal: 3
allow_prompt_login: true

'test_sp_with_default_help_text':
Expand Down
12 changes: 12 additions & 0 deletions db/migrate/20210120220857_add_default_aal_to_service_provider.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
class AddDefaultAalToServiceProvider < ActiveRecord::Migration[6.1]
def up
add_column :service_providers, :default_aal, :integer
ServiceProvider.all.each do |sp|
sp.update!(default_aal: sp.aal) if sp.aal
end
end

def down
remove_column :service_providers, :default_aal
end
end
11 changes: 6 additions & 5 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
# of editing this file, please use the migrations feature of Active Record to
# incrementally modify your database, and then regenerate this schema definition.
#
# This file is the source Rails uses to define your schema when running `rails
# db:schema:load`. When creating a new database, `rails db:schema:load` tends to
# This file is the source Rails uses to define your schema when running `bin/rails
# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to
# be faster and is potentially less error prone than running all of your
# migrations from scratch. Old migrations may fail to apply correctly if those
# migrations use external dependencies or application code.
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2020_12_07_191837) do
ActiveRecord::Schema.define(version: 2021_01_20_220857) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand Down Expand Up @@ -166,12 +166,12 @@
t.integer "choose_method_view_count", default: 0
t.datetime "present_cac_view_at"
t.integer "present_cac_view_count", default: 0
t.integer "present_cac_submit_count", default: 0
t.integer "present_cac_error_count", default: 0
t.datetime "enter_info_view_at"
t.integer "enter_info_view_count", default: 0
t.datetime "success_view_at"
t.integer "success_view_count", default: 0
t.integer "present_cac_submit_count", default: 0
t.integer "present_cac_error_count", default: 0
t.datetime "selfie_view_at"
t.integer "selfie_view_count", default: 0
t.integer "selfie_submit_count", default: 0
Expand Down Expand Up @@ -492,6 +492,7 @@
t.date "iaa_start_date"
t.date "iaa_end_date"
t.string "app_id"
t.integer "default_aal"
t.index ["issuer"], name: "index_service_providers_on_issuer", unique: true
end

Expand Down
2 changes: 1 addition & 1 deletion spec/decorators/service_provider_session_decorator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@
describe '#mfa_expiration_interval' do
context 'with an AAL2 sp' do
before do
allow(sp).to receive(:aal).and_return(2)
allow(sp).to receive(:default_aal).and_return(2)
end

it { expect(subject.mfa_expiration_interval).to eq(12.hours) }
Expand Down
8 changes: 4 additions & 4 deletions spec/features/openid_connect/aal3_required_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@
end
end

describe 'ServiceProvider configured to require AAL3 authentication' do
describe 'ServiceProvider configured to default to AAL3 authentication' do
context 'user does not have aal3 auth configured' do
it 'sends user to set up AAL3 auth' do
user = user_with_2fa

visit_idp_from_ial1_oidc_sp_requiring_aal3(prompt: 'select_account')
visit_idp_from_ial1_oidc_sp_defaulting_to_aal3(prompt: 'select_account')
sign_in_live_with_2fa(user)

expect(current_url).to eq(two_factor_options_url)
Expand All @@ -52,15 +52,15 @@
context 'user has aal3 auth configured' do
it 'sends user to authenticate with AAL3 auth' do
sign_in_before_2fa(user_with_aal3_2fa)
visit_idp_from_ial1_oidc_sp_requiring_aal3(prompt: 'select_account')
visit_idp_from_ial1_oidc_sp_defaulting_to_aal3(prompt: 'select_account')
visit login_two_factor_path(otp_delivery_preference: 'sms')

expect(current_url).to eq(login_two_factor_webauthn_url)
end

it 'does not allow an already signed in user to bypass AAL3 auth' do
sign_in_and_2fa_user(user_with_aal3_2fa)
visit_idp_from_ial1_oidc_sp_requiring_aal3(prompt: 'select_account')
visit_idp_from_ial1_oidc_sp_defaulting_to_aal3(prompt: 'select_account')

expect(current_url).to eq(login_two_factor_webauthn_url)
end
Expand Down
4 changes: 2 additions & 2 deletions spec/features/remember_device/sp_expiration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@
allow(AppConfig.env).to receive(:otp_delivery_blocklist_maxretry).and_return('1000')

ServiceProvider.from_issuer('urn:gov:gsa:openidconnect:sp:server').update!(
aal: aal,
default_aal: aal,
ial: ial,
)
ServiceProvider.from_issuer('http://localhost:3000').update!(
aal: aal,
default_aal: aal,
ial: ial,
)
end
Expand Down
2 changes: 1 addition & 1 deletion spec/features/saml/aal3_required_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
end
end

describe 'SAML ServiceProvider configured to require AAL3 authentication' do
describe 'SAML ServiceProvider configured to default to AAL3 authentication' do
context 'user does not have aal3 auth configured' do
it 'sends user to set up AAL3 auth' do
sign_in_and_2fa_user(user_with_2fa)
Expand Down
2 changes: 1 addition & 1 deletion spec/features/sign_in/remember_device_default_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
context 'when signing in from an SP when the SP is AAL2' do
before do
ServiceProvider.from_issuer('urn:gov:gsa:openidconnect:sp:server').update!(
aal: 2,
default_aal: 2,
)
end

Expand Down
2 changes: 1 addition & 1 deletion spec/features/users/sign_up_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@

it 'does not show the remember device option as the default when the SP is AAL2' do
ServiceProvider.from_issuer('urn:gov:gsa:openidconnect:sp:server').update!(
aal: 2,
default_aal: 2,
)
visit_idp_from_sp_with_ial1(:oidc)
sign_up_and_set_password
Expand Down
30 changes: 30 additions & 0 deletions spec/policies/service_provider_mfa_policy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,36 @@
)
end

describe '#aal3_required?' do
context 'aal3 requested' do
let(:aal_level_requested) { 3 }
before { service_provider.default_aal = nil }

it { expect(policy.aal3_required?).to eq(true) }
end

context 'no aal level requested, SP default is aal3' do
let(:aal_level_requested) { nil }
before { service_provider.default_aal = 3 }

it { expect(policy.aal3_required?).to eq(true) }
end

context 'aal2 requested, no default set' do
let(:aal_level_requested) { 2 }
before { service_provider.default_aal = nil }

it { expect(policy.aal3_required?).to eq(false) }
end

context 'aal2 level requested, SP default is aal3' do
let(:aal_level_requested) { 2 }
before { service_provider.default_aal = 3 }

it { expect(policy.aal3_required?).to eq(false) }
end
end

describe '#user_needs_sp_auth_method_verification?' do
context 'aal3 required' do
let(:aal_level_requested) { 3 }
Expand Down
2 changes: 1 addition & 1 deletion spec/services/attribute_asserter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
ServiceProvider,
issuer: 'http://localhost:3000',
ial: service_provider_ial,
aal: service_provider_aal,
default_aal: service_provider_aal,
liveness_checking_required: false,
metadata: {},
)
Expand Down
2 changes: 1 addition & 1 deletion spec/support/oidc_auth_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def visit_idp_from_ial1_oidc_sp_requesting_aal3(**args)
oidc_path
end

def visit_idp_from_ial1_oidc_sp_requiring_aal3(**args)
def visit_idp_from_ial1_oidc_sp_defaulting_to_aal3(**args)
args[:client_id] ||= OIDC_AAL3_ISSUER
params = ial1_params args
oidc_path = openid_connect_authorize_path params
Expand Down
1 change: 1 addition & 0 deletions spec/support/saml_auth_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ def sp1_saml_settings

def aal3_sp1_saml_settings
settings = saml_settings.dup
settings.authn_context = nil
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this be something that sets AAL3? Or does it come from the issuer?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is a case where the semantics have changed. The default settings request an AAL2 auth, and would have returned AAL3 given aal: 3. Now that request would return AAL2 since the request trumps the default. These settings are now used in a test of the default (spec/features/saml/aal3_required_spec), so we need to not make an AAL request at all.

settings.issuer = 'https://aal3.serviceprovider.com/auth/saml/metadata'
settings
end
Expand Down