wip: OpenidConnectAuthorizeForm specs passing

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

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

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

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<String>]
-    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<Vot::ComponentValue>]
+    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)

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