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
16 changes: 15 additions & 1 deletion app/forms/openid_connect_authorize_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion app/presenters/openid_connect_user_info_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions app/services/identity_linker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddIdentitiesRequestedAalValue < ActiveRecord::Migration[7.0]
def change
add_column :identities, :requested_aal_value, :text
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
128 changes: 101 additions & 27 deletions spec/forms/openid_connect_authorize_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

question: should this return urn:gov:gsa:ac:classes:sp:PasswordProtectedTransport:duo instead of 0?

Copy link
Contributor

Choose a reason for hiding this comment

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

aal is an integer in the database, so I think this may be a bug in the current implementation to have the form return a string

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

Expand Down Expand Up @@ -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
Expand All @@ -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 }
Expand Down
3 changes: 2 additions & 1 deletion spec/presenters/openid_connect_user_info_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand Down