IdpMetadata#name_id_format(s) should be parsed as a prioritized list

[johnnyshields]

Currently IdpMetadata#name_id_format is a singular (scalar) value.

Since the IdP metadata can include multiple nameId formats, it would be useful to add a #name_id_formats method.

Then, #name_id_format could be selected to be the one that matches the SP metadata requirement.

[pitbulk]

You see that the name_id_format is a settings value at the SP side, this value is used in order to generate the AuthNRequest.

The parser could be extended to include a:

def idp_name_id_formats
  name_id_formats = []
  nodes = REXML::XPath.match(
    @idpsso_descriptor,
    "md:NameIDFormat",
    SamlMetadata::NAMESPACE
  )
  nodes.each do |node|
    name_id_formats << Utils.element_text(node)
  end
          
  name_id_formats
end

and the to_hash could be extended as:

def to_hash(options = {})
  {
    :idp_entity_id => @entity_id,
    :name_identifier_format => idp_name_id_format,
    :idp_sso_service_url => single_signon_service_url(options),
    :idp_slo_service_url => single_logout_service_url(options),
    :idp_slo_response_service_url => single_logout_response_service_url(options),
    :idp_attribute_names => attribute_names,
    :idp_cert => nil,
    :idp_cert_fingerprint => nil,
    :idp_cert_multi => nil,
    :idp_name_identifier_format => idp_name_id_formats,
    :valid_until => valid_until,
    :cache_duration => cache_duration,
  }.tap do |response_hash|
    merge_certificates_into(response_hash) unless certificates.nil?
  end    
end

[johnnyshields]

I think the right way would be to add a new option to IdpMetadata#parse which mirrors how the slo/sso bindings are done:

# @option options [Array<String>, nil] :name_id_format an ordered list of NameIDFormats to detect a desired value. The first NameIDFormat in the list that is included in the metadata will be used.

(Note that we currently have the following:)

# @option options [Array<String>, nil] :sso_binding an ordered list of bindings to detect the single signon URL. The first binding in the list that is included in the metadata will be used.
# @option options [Array<String>, nil] :slo_binding an ordered list of bindings to detect the single logout URL. The first binding in the list that is included in the metadata will be used.
# @option options [String, nil] :entity_id when this is given, the entity descriptor for this ID is used. When ommitted, the first entity descriptor is used.

If the option is not specified, behavior will be the same at it is now. Would you accept a PR for this?

[pitbulk]

Seems reasonable

[johnnyshields]

PR raised