diff --git a/app/forms/openid_connect_authorize_form.rb b/app/forms/openid_connect_authorize_form.rb index b9340a5c530..45d7c128bbb 100644 --- a/app/forms/openid_connect_authorize_form.rb +++ b/app/forms/openid_connect_authorize_form.rb @@ -18,6 +18,11 @@ class OpenidConnectAuthorizeForm ].freeze ATTRS = [:unauthorized_scope, :acr_values, :scope, :verified_within, *SIMPLE_ATTRS].freeze + AALS_BY_PRIORITY = [Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF, + Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF, + Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF, + Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF, + Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF].freeze attr_reader(*ATTRS) @@ -81,6 +86,7 @@ def link_identity_to_service_provider(current_user, rails_session_id) rails_session_id: rails_session_id, ial: ial_context.ial, aal: aal, + requested_aal_value: requested_aal_value, scope: scope.join(' '), code_challenge: code_challenge, ) @@ -110,7 +116,11 @@ def aal_values end def aal - Saml::Idp::Constants::AUTHN_CONTEXT_CLASSREF_TO_AAL[aal_values.sort.max] || + Saml::Idp::Constants::AUTHN_CONTEXT_CLASSREF_TO_AAL[requested_aal_value] + end + + def requested_aal_value + highest_level_aal(aal_values) || Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF end @@ -255,4 +265,8 @@ def validate_privileges ) end end + + def highest_level_aal(aal_values) + AALS_BY_PRIORITY.find { |aal| aal_values.include?(aal) } + end end diff --git a/app/presenters/openid_connect_user_info_presenter.rb b/app/presenters/openid_connect_user_info_presenter.rb index 2b18fef1d25..2b5bf856096 100644 --- a/app/presenters/openid_connect_user_info_presenter.rb +++ b/app/presenters/openid_connect_user_info_presenter.rb @@ -22,7 +22,7 @@ def user_info info.merge!(x509_attributes) if scoper.x509_scopes_requested? info[:verified_at] = verified_at if scoper.verified_at_requested? info[:ial] = Saml::Idp::Constants::AUTHN_CONTEXT_IAL_TO_CLASSREF[identity.ial] - info[:aal] = Saml::Idp::Constants::AUTHN_CONTEXT_AAL_TO_CLASSREF[identity.aal] + info[:aal] = identity.requested_aal_value scoper.filter(info) end diff --git a/app/services/identity_linker.rb b/app/services/identity_linker.rb index c1c79dae6f0..388524abf97 100644 --- a/app/services/identity_linker.rb +++ b/app/services/identity_linker.rb @@ -6,12 +6,14 @@ def initialize(user, service_provider) @service_provider = service_provider @ial = nil @aal = nil + @requested_aal_value = nil end def link_identity( code_challenge: nil, ial: nil, aal: nil, + requested_aal_value: nil, nonce: nil, rails_session_id: nil, scope: nil, @@ -28,6 +30,7 @@ def link_identity( code_challenge: code_challenge, ial: ial, aal: aal, + requested_aal_value: requested_aal_value, nonce: nonce, rails_session_id: rails_session_id, scope: scope, diff --git a/db/primary_migrate/20230404131517_add_identities_requested_aal_value.rb b/db/primary_migrate/20230404131517_add_identities_requested_aal_value.rb new file mode 100644 index 00000000000..69d74f3f977 --- /dev/null +++ b/db/primary_migrate/20230404131517_add_identities_requested_aal_value.rb @@ -0,0 +1,5 @@ +class AddIdentitiesRequestedAalValue < ActiveRecord::Migration[7.0] + def change + add_column :identities, :requested_aal_value, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 7767e896d09..b7b2962d53f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_04_03_232935) do +ActiveRecord::Schema[7.0].define(version: 2023_04_04_131517) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" enable_extension "pgcrypto" @@ -284,6 +284,7 @@ t.datetime "last_ial2_authenticated_at", precision: nil t.datetime "deleted_at", precision: nil t.integer "aal" + t.text "requested_aal_value" t.index ["access_token"], name: "index_identities_on_access_token", unique: true t.index ["session_uuid"], name: "index_identities_on_session_uuid", unique: true t.index ["user_id", "service_provider"], name: "index_identities_on_user_id_and_service_provider", unique: true diff --git a/spec/forms/openid_connect_authorize_form_spec.rb b/spec/forms/openid_connect_authorize_form_spec.rb index 05f45dfd74b..104062636a4 100644 --- a/spec/forms/openid_connect_authorize_form_spec.rb +++ b/spec/forms/openid_connect_authorize_form_spec.rb @@ -357,37 +357,19 @@ end describe '#aal' do - context 'when DEFAULT_AAL passed' do - before do - default = Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF - IdentityConfig.store.valid_authn_contexts.push(default) - end - - after do - IdentityConfig.store.valid_authn_contexts.pop - end - - let(:acr_values) { Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF } + context 'when no AAL passed' do + let(:acr_values) { Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF } it 'returns 0' do - expect(form.aal).to eq(Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF) + expect(form.aal).to eq(0) end end - context 'when AAL1 passed' do - before do - aal1 = Saml::Idp::Constants::AAL1_AUTHN_CONTEXT_CLASSREF - IdentityConfig.store.valid_authn_contexts.push(aal1) - end - - after do - IdentityConfig.store.valid_authn_contexts.pop - end - - let(:acr_values) { Saml::Idp::Constants::AAL1_AUTHN_CONTEXT_CLASSREF } + context 'when DEFAULT_AAL passed' do + let(:acr_values) { Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF } - it 'returns 1' do - expect(form.aal).to eq(1) + it 'returns 0' do + expect(form.aal).to eq(0) end end @@ -430,9 +412,7 @@ expect(form.aal).to eq(3) end end - end - describe '#aal' do context 'when IAL and AAL passed' do aal2 = Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF ial2 = Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF @@ -448,6 +428,100 @@ end end + describe '#requested_aal_value' do + context 'when AAL2 passed' do + let(:acr_values) { Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF } + + it 'returns AAL2' do + expect(form.requested_aal_value).to eq(Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF) + end + end + + context 'when AAL2_PHISHING_RESISTANT passed' do + let(:acr_values) { Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF } + + it 'returns AAL2+Phishing Resistant' do + expect(form.requested_aal_value).to eq( + Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF, + ) + end + end + + context 'when AAL2_HSPD12 passed' do + let(:acr_values) { Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF } + + it 'returns AAL2+HSPD12' do + expect(form.requested_aal_value).to eq( + Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF, + ) + end + end + + context 'when AAL3 passed' do + let(:acr_values) { Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF } + + it 'returns AAL3' do + expect(form.requested_aal_value).to eq(Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF) + end + end + + context 'when AAL3_HSPD12 passed' do + let(:acr_values) { Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF } + + it 'returns AAL3+HSPD12' do + expect(form.requested_aal_value).to eq( + Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF, + ) + end + end + + context 'when AAL3_HSPD12 and AAL2_HSPD12 passed' do + let(:acr_values) do + [Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF, + Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF].join(' ') + end + + it 'returns AAL2+HSPD12' do + expect(form.requested_aal_value).to eq( + Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF, + ) + end + end + + context 'when AAL2 and AAL2_PHISHING_RESISTANT passed' do + let(:phishing_resistant) do + Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF + end + + let(:acr_values) do + "#{Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF} + #{phishing_resistant}" + end + + it 'returns AAL2+HSPD12' do + expect(form.requested_aal_value).to eq(phishing_resistant) + end + end + + context 'when AAL2_PHISHING_RESISTANT and AAL2 passed' do + # this is the same as the previous test, just reverse ordered + # AAL values, to ensure it doesn't just take the 2nd AAL. + let(:phishing_resistant) do + Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF + end + + let(:acr_values) do + "#{phishing_resistant} + #{Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF}" + end + + it 'returns AAL2+HSPD12' do + requested_aal_value = form.requested_aal_value + expect(requested_aal_value).to eq(phishing_resistant) + end + end + end + describe '#verified_within' do context 'without a verified_within' do let(:verified_within) { nil } diff --git a/spec/presenters/openid_connect_user_info_presenter_spec.rb b/spec/presenters/openid_connect_user_info_presenter_spec.rb index c380b2875da..eba2a630afa 100644 --- a/spec/presenters/openid_connect_user_info_presenter_spec.rb +++ b/spec/presenters/openid_connect_user_info_presenter_spec.rb @@ -18,6 +18,7 @@ service_provider: service_provider.issuer, scope: scope, aal: 2, + requested_aal_value: Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF, ) end @@ -28,7 +29,7 @@ it 'has basic attributes' do ial = Saml::Idp::Constants::AUTHN_CONTEXT_IAL_TO_CLASSREF[identity.ial] - aal = Saml::Idp::Constants::AUTHN_CONTEXT_AAL_TO_CLASSREF[identity.aal] + aal = Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF aggregate_failures do expect(user_info[:sub]).to eq(identity.uuid)