diff --git a/app/forms/openid_connect_authorize_form.rb b/app/forms/openid_connect_authorize_form.rb index 0a5674474cc..1fe9f08b782 100644 --- a/app/forms/openid_connect_authorize_form.rb +++ b/app/forms/openid_connect_authorize_form.rb @@ -53,11 +53,11 @@ class OpenidConnectAuthorizeForm validate :validate_verified_within_duration, if: :verified_within_allowed? def initialize(params) - @acr_values = parse_to_values(params[:acr_values], Saml::Idp::Constants::VALID_AUTHN_CONTEXTS) + @acr_values = parse_acr_values(params[:acr_values], Saml::Idp::Constants::VALID_AUTHN_CONTEXTS) @vtr = parse_vtr(params[:vtr]) SIMPLE_ATTRS.each { |key| instance_variable_set(:"@#{key}", params[key]) } @prompt ||= 'select_account' - @scope = parse_to_values(params[:scope], scopes) + @scope = parse_scope(params[:scope], scopes) @unauthorized_scope = check_for_unauthorized_scope(params) if verified_within_allowed? @@ -95,7 +95,7 @@ def link_identity_to_service_provider( nonce: nonce, rails_session_id: rails_session_id, ial: ial, - acr_values: Vot::AcrComponentValues.join(acr_values), + acr_values: Vot::AcrComponentValues.build(acr_values), vtr: vtr, requested_aal_value: requested_aal_value, scope: scope.join(' '), @@ -124,7 +124,9 @@ def requested_aal_value private - attr_reader :identity, :success + # @return [ServiceProviderIdentity] + attr_reader :identity + attr_reader :success def code identity&.session_uuid @@ -149,8 +151,12 @@ def parsed_vectors_of_trust end end - def parse_to_values(param_value, possible_values) + def parse_scope(param_value, possible_values) return [] if param_value.blank? + param_value.split(' ').compact & possible_values + end + + def parse_acr_values(param_value, possible_values = Saml::Idp::Constants::VALID_AUTHN_CONTEXTS) Vot::AcrComponentValues.order_by_priority(param_value) & possible_values end @@ -256,7 +262,7 @@ def extra_analytics_attributes allow_prompt_login: service_provider&.allow_prompt_login, redirect_uri: result_uri, scope: scope&.sort&.join(' '), - acr_values: Vot::AcrComponentValues.join(acr_values), + acr_values: Vot::AcrComponentValues.build(acr_values), vtr: vtr, unauthorized_scope: @unauthorized_scope, code_digest: code ? Digest::SHA256.hexdigest(code) : nil, @@ -309,14 +315,7 @@ def sp_defaults_to_identity_proofing? end def identity_proofing_requested? - if parsed_vectors_of_trust.present? - parsed_vectors_of_trust.any?(&:identity_proofing?) - else - Vot::AcrComponentValues. - by_name[highest_level_ial]&. - requirements&. - include?(:identity_proofing) - end + requested_authn_context.identity_proofing? end def identity_proofing_service_provider? @@ -328,11 +327,11 @@ def ialmax_allowed_for_sp? end def ialmax_requested? - ial_values.include?(Saml::Idp::Constants::IALMAX_AUTHN_CONTEXT_CLASSREF) + requested_authn_context.ialmax? end def biometric_ial_requested? - ial_values.intersect?(Saml::Idp::Constants::BIOMETRIC_IAL_CONTEXTS) + requested_authn_context.biometric_comparison? end def highest_level_ial @@ -347,13 +346,20 @@ def verified_within_allowed? IdentityConfig.store.allowed_verified_within_providers.include?(client_id) end - def default_aal_acr - ctx = AuthnContextResolver.new( + def request_authn_context_resolver + @request_authn_context_resolver ||= AuthnContextResolver.new( service_provider: service_provider, user: nil, - vtr: nil, - acr_values: nil, + vtr: parsed_vectors_of_trust.present? && vtr, + acr_values: acr_values, ) - ctx.asserted_aal_acr + end + + def requested_authn_context + @requested_authn_context ||= request_authn_context_resolver.result + end + + def default_aal_acr + request_authn_context_resolver.default_aal_acr end end diff --git a/app/models/federated_protocols/oidc.rb b/app/models/federated_protocols/oidc.rb index 0ca2971d168..20c90154f78 100644 --- a/app/models/federated_protocols/oidc.rb +++ b/app/models/federated_protocols/oidc.rb @@ -12,11 +12,11 @@ def issuer end def ial - request.ial_values.sort.max + request.ial_values.first end def aal - request.aal_values.sort.max + request.aal_values.first end def acr_values @@ -41,6 +41,7 @@ def service_provider private + # @return [OpenidConnectAuthorizeForm] attr_reader :request end end diff --git a/app/services/authn_context_resolver.rb b/app/services/authn_context_resolver.rb index 2973759199c..2b8a7896bce 100644 --- a/app/services/authn_context_resolver.rb +++ b/app/services/authn_context_resolver.rb @@ -21,11 +21,12 @@ class AuthnContextResolver Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, Saml::Idp::Constants::LOA1_AUTHN_CONTEXT_CLASSREF, ].freeze + def initialize(user:, service_provider:, vtr:, acr_values:) @user = user @service_provider = service_provider @vtr = vtr - @acr_values = acr_values + @acr_values = Vot::AcrComponentValues.build(acr_values) end def result @@ -51,13 +52,25 @@ def asserted_ial_acr def asserted_aal_acr return if vtr.present? - if acr_aal_component_values.any? - highest_aal_acr(acr_aal_component_values.map(&:name)) || acr_aal_component_values.first.name - elsif service_provider&.default_aal.to_i >= 3 + if acr_aal_component_values.present? + highest_aal_acr(acr_aal_component_values) || acr_aal_component_values.first.name + # elsif service_provider&.default_aal.to_i >= 3 + # Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF + # elsif service_provider&.default_aal.to_i == 2 + # Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF + # elsif acr_result.identity_proofing_or_ialmax? + # Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF + else + default_aal_acr + end + end + + def default_aal_acr + if service_provider&.default_aal.to_i >= 3 Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF elsif service_provider&.default_aal.to_i == 2 Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF - elsif acr_result.identity_proofing_or_ialmax? + elsif acr_result_with_sp_defaults.identity_proofing_or_ialmax? Saml::Idp::Constants::AAL2_AUTHN_CONTEXT_CLASSREF else Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF @@ -159,9 +172,11 @@ def result_with_sp_aal_defaults(result) def acr_aal_component_values @acr_aal_component_values ||= - acr_result_without_sp_defaults.component_values.filter do |component_value| - Saml::Idp::Constants::AUTHN_CONTEXT_CLASSREF_TO_AAL.include?(component_value.name) - end + Vot::AcrComponentValues.order_by_priority( + Vot::AcrComponentValues.aal_component_values( + acr_result_without_sp_defaults.component_values, + ), + ) end def acr_ial_component_values @@ -178,6 +193,6 @@ def biometric_is_required?(result) end def highest_aal_acr(aals) - AALS_BY_PRIORITY.find { |aal| aals.include?(aal) } + Vot::AcrComponentValues.find_highest_priority(aals) end end diff --git a/app/services/vot/acr_component_values.rb b/app/services/vot/acr_component_values.rb index 661915810a1..a6070a6d43c 100644 --- a/app/services/vot/acr_component_values.rb +++ b/app/services/vot/acr_component_values.rb @@ -160,6 +160,12 @@ def self.by_name NAME_HASH end + def self.includes_requirements?(name, *requirements) + component = NAME_HASH[name] + component.present? && + component.requirements.intersection(requirements).size == requirements.length + end + # Get the highest priority ACR value # @return [String, nil] def self.find_highest_priority(values) @@ -193,32 +199,56 @@ def self.ial_values(values) to_names(values).filter { |acr| AcrComponentValues.ial?(acr) } end + def self.ial_component_values(values) + to_components(values).filter { |acr| AcrComponentValues.ial?(acr) } + end + def self.aal_values(values) to_names(values).filter { |acr| AcrComponentValues.aal?(acr) } end + def self.aal_component_values(values) + to_components(values).filter { |acr| AcrComponentValues.aal?(acr) } + end + # Convert list of strings or {Vot::ComponentValue} to ACR values # @return [Array] - def self.to_names(values) + def self.to_names(values = '') + [] unless values.present? && + (values.is_a?(String) || values.is_a?(Enumerable)) + values_ary = values.is_a?(String) && values.split(DELIM) || values.presence || [] + + values_ary. + map { |v| AcrComponentValues.to_name(v) }. + compact_blank. + uniq + end + + # Convert list of strings or {Vot::ComponentValue} to an array of ComponentValue + # @param values [String, Enumerable] + # @return [Array] + def self.to_components(values) [] unless values.present? && - !(values.is_a?(String) || values.is_a?(Enumerable)) - names = - if values.is_a?(Enumerable) - values. - map { |v| AcrComponentValues.to_name(v) }. - to_a - else - values.split(DELIM) - end - names.compact_blank.uniq + (values.is_a?(String) || values.is_a?(Enumerable)) + + values_ary = values.is_a?(String) && values.split(DELIM) || values + + values_ary. + map { |v| AcrComponentValues.to_component(v) }. + compact_blank. + uniq end def self.to_name(value) value.is_a?(ComponentValue) ? value.name : value.to_s end - def self.join(values) - to_names(values).join(DELIM) + def self.to_component(value) + value.is_a?(String) && by_name[value] || value + end + + def self.build(values) + values.is_a?(Enumerable) && to_names(values).join(DELIM) || values end def self.ial?(value) diff --git a/spec/forms/openid_connect_authorize_form_spec.rb b/spec/forms/openid_connect_authorize_form_spec.rb index 2a2cfa99cc4..a437e72a342 100644 --- a/spec/forms/openid_connect_authorize_form_spec.rb +++ b/spec/forms/openid_connect_authorize_form_spec.rb @@ -149,6 +149,7 @@ let(:vtr) { ['A1.B2.C3'].to_json } it 'has errors' do + raise_error expect(valid?).to eq(false) expect(form.errors[:vtr]). to include(t('openid_connect.authorization.errors.no_valid_vtr')) @@ -213,6 +214,7 @@ shared_examples 'allows biometric IAL only if sp is authorized' do |biometric_ial| let(:acr_values) { biometric_ial } + let(:vtr) { nil } context "when the IAL requested is #{biometric_ial}" do context 'when the service provider is allowed to use biometric ials' do @@ -472,7 +474,7 @@ it 'returns AAL3' do expect(form.requested_aal_value).to eq( - Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF, + Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF, ) end end @@ -604,8 +606,10 @@ let(:verified_within) { '45d' } it 'parses the value as a number of days' do - expect(form.valid?).to eq(true) - expect(form.verified_within).to eq(45.days) + aggregate_failures 'verified within verified_within' do + expect(form.valid?).to eq(true) + expect(form.verified_within).to eq(45.days) + end end end