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
39 changes: 30 additions & 9 deletions app/services/id_token_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,14 @@ def jwt_payload
def id_token_claims
{
acr: acr,
vot: (vot if sp_requests_vot?),
vtm: (IdentityConfig.store.vtm_url if sp_requests_vot?),
nonce: identity.nonce,
aud: identity.service_provider,
jti: SecureRandom.urlsafe_base64,
at_hash: hash_token(identity.access_token),
c_hash: hash_token(code),
}
}.compact
end

def timestamp_claims
Expand All @@ -55,24 +57,43 @@ def timestamp_claims
end

def acr
ial = identity.ial
case ial
when Idp::Constants::IAL_MAX then determine_ial_max_acr
when Idp::Constants::IAL1 then Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF
when Idp::Constants::IAL2 then Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF
return nil unless identity.acr_values.present?
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.

Currently we only return a single ACR value. I'm not sure if we want to start returning the full list because I am not sure how SPs will respond to that.

I believe the previous implementation of responding with the ACR value for the identity assurance level is probably the way to go here to ensure backwards compadibility.

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.

Actually we could probably remove all the IAL computation in my previous comment if we dressed up this method like so:

def acr
  if resolved_authn_context_result.ialmax?
    determine_ial_max_acr.name
  elsif resolved_authn_context_result.identity_proofing?
    Vot::LegacyComponentValues::IAL2.name
  else
    Vot::LegacyComponentValues::IAL1.name
  end
end

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.

And since this didn't stack up like I expected, this is the "previous comment" I am referring to: #10142 (comment)

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.

This way we:

  1. Return ial1 or ial2 correctly in the IALMAX case
  2. Return ial1 or ial2 even if the SP requests loa1 or loa3 or if the SP makes no ial related request
  3. Only ever return one value in the ACR
  4. Correctly return the ial1 or ial2 ACR values even if we introduce new ACR values or change names


if resolved_authn_context_result.ialmax?
determine_ial_max_acr.name
elsif resolved_authn_context_result.identity_proofing?
Vot::LegacyComponentValues::IAL2.name
else
raise "Unknown ial #{ial}"
Vot::LegacyComponentValues::IAL1.name
end
end

def sp_requests_vot?
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.

We probably also want to add a vot.present? clause here as well. That way we won't end up with a vtm claim when there is not vot.

return false unless identity.vtr.present?
IdentityConfig.store.use_vot_in_sp_requests
end

def vot
return nil unless identity.vtr.present?
resolved_authn_context_result.component_values.map(&:name).join('.')
end

def determine_ial_max_acr
if identity.user.identity_verified?
Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF
Vot::LegacyComponentValues::IAL2
else
Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF
Vot::LegacyComponentValues::IAL1
end
end

def resolved_authn_context_result
@resolved_authn_context_result ||= AuthnContextResolver.new(
service_provider: identity.service_provider_record,
vtr: [identity.vtr],
acr_values: identity.acr_values,
).resolve
end

def expires
now.to_i + ttl
end
Expand Down
1 change: 1 addition & 0 deletions config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ verify_gpo_key_max_attempts: 5
verify_personal_key_attempt_window_in_minutes: 15
verify_personal_key_max_attempts: 5
version_headers_enabled: false
vtm_url: 'https://developer.login.gov/vot-trust-framework'
use_dashboard_service_providers: false
use_kms: false
use_vot_in_sp_requests: false
Expand Down
1 change: 1 addition & 0 deletions lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ def self.build_store(config_map)
config.add(:version_headers_enabled, type: :boolean)
config.add(:voice_otp_pause_time)
config.add(:voice_otp_speech_rate)
config.add(:vtm_url)
config.add(:weekly_auth_funnel_report_config, type: :json)

@key_types = config.key_types
Expand Down
72 changes: 70 additions & 2 deletions spec/services/id_token_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

let(:now) { Time.zone.now }
let(:custom_expiration) { (now + 5.minutes).to_i }
let(:vtm_url) { 'https://example.com/vot-trust-framework' }
subject(:builder) do
IdTokenBuilder.new(
identity: identity,
Expand Down Expand Up @@ -61,22 +62,89 @@
expect(decoded_payload[:nonce]).to eq(identity.nonce)
end

context 'it sets the vot' do
context 'sp requests vot' do
before do
allow(IdentityConfig.store).to receive(:use_vot_in_sp_requests).
and_return(true)
allow(IdentityConfig.store).to receive(:vtm_url).
and_return(vtm_url)
end

it 'sets the vot if the sp requests it' do
identity.vtr = 'Pb'
expect(decoded_payload[:vot]).to eq('C1.C2.P1.Pb')
end

it 'sets the vtm' do
identity.vtr = 'Pb'
expect(decoded_payload[:vtm]).to eq(vtm_url)
end
end

context 'sp does not request vot' do
before do
allow(IdentityConfig.store).to receive(:use_vot_in_sp_requests).
and_return(false)
allow(IdentityConfig.store).to receive(:vtm_url).
and_return(vtm_url)
end

it 'does not set the vot if the sp does not request it' do
identity.vtr = 'Pb'
expect(decoded_payload[:vot]).to eq nil
end

it 'does not set the vtm' do
identity.vtr = nil
expect(decoded_payload[:vtm]).to eq nil
end
end
end

context 'it sets the acr' do
context 'aal and ial request' do
before do
identity.aal = 2
acr_values = [
Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF,
Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF,
].join(' ')
identity.acr_values = acr_values
end

it 'ignores the aal value' do
expect(decoded_payload[:acr]).to eq(Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF)
end
end

context 'ial2 request' do
before do
identity.ial = 2
identity.acr_values = Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF
end

it 'sets the acr to the ial2 constant' do
expect(decoded_payload[:acr]).to eq(Saml::Idp::Constants::IAL2_AUTHN_CONTEXT_CLASSREF)
end
end

context 'ial1 request' do
before { identity.ial = 1 }
before do
identity.ial = 1
identity.acr_values = Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF
end

it 'sets the acr to the ial1 constant' do
expect(decoded_payload[:acr]).to eq(Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF)
end
end

context 'ialmax request' do
before { identity.ial = 0 }
before do
identity.ial = 0
identity.acr_values = Saml::Idp::Constants::IALMAX_AUTHN_CONTEXT_CLASSREF
end

context 'non-verified user' do
it 'sets the acr to the ial1 constant' do
Expand Down