From 92a0c03c4212e1be84c43cf963f32f417d217025 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 9 Jun 2026 09:13:52 +0200 Subject: [PATCH] Fix fetching of SAML metadata for large aggregate endpoints (#23531) * Fix fetching of SAML metadata for large aggregate endpoints https://community.openproject.org/work_packages/OP-19420 * Use XML pull parser to avoid text parsing --- .../contracts/saml/providers/base_contract.rb | 2 + .../forms/saml/providers/metadata_url_form.rb | 8 + .../forms/saml/providers/metadata_xml_form.rb | 8 + modules/auth_saml/app/models/saml/provider.rb | 1 + .../app/services/saml/metadata_document.rb | 152 ++++++ .../app/services/saml/metadata_fetcher.rb | 69 +++ .../services/saml/update_metadata_service.rb | 16 +- modules/auth_saml/config/locales/en.yml | 8 + .../spec/fixtures/federation_metadata.xml | 443 ++++++++++++++++++ .../services/saml/metadata_document_spec.rb | 141 ++++++ .../services/saml/metadata_fetcher_spec.rb | 97 ++++ .../saml/update_metadata_service_spec.rb | 23 +- 12 files changed, 961 insertions(+), 7 deletions(-) create mode 100644 modules/auth_saml/app/services/saml/metadata_document.rb create mode 100644 modules/auth_saml/app/services/saml/metadata_fetcher.rb create mode 100644 modules/auth_saml/spec/fixtures/federation_metadata.xml create mode 100644 modules/auth_saml/spec/services/saml/metadata_document_spec.rb create mode 100644 modules/auth_saml/spec/services/saml/metadata_fetcher_spec.rb diff --git a/modules/auth_saml/app/contracts/saml/providers/base_contract.rb b/modules/auth_saml/app/contracts/saml/providers/base_contract.rb index eaee4f8e5d5..834e1c6ee19 100644 --- a/modules/auth_saml/app/contracts/saml/providers/base_contract.rb +++ b/modules/auth_saml/app/contracts/saml/providers/base_contract.rb @@ -47,6 +47,8 @@ module Saml url: { allow_blank: true, allow_nil: true, schemes: %w[http https] }, if: -> { model.metadata_url_changed? } + attribute :idp_entity_id + attribute :idp_sso_service_url validates :idp_sso_service_url, url: { schemes: %w[http https] }, diff --git a/modules/auth_saml/app/forms/saml/providers/metadata_url_form.rb b/modules/auth_saml/app/forms/saml/providers/metadata_url_form.rb index 4be72e20603..e3e7c47325d 100644 --- a/modules/auth_saml/app/forms/saml/providers/metadata_url_form.rb +++ b/modules/auth_saml/app/forms/saml/providers/metadata_url_form.rb @@ -40,6 +40,14 @@ module Saml caption: I18n.t("saml.instructions.metadata_url"), input_width: :xlarge ) + f.text_field( + name: :idp_entity_id, + label: I18n.t("activerecord.attributes.saml/provider.idp_entity_id"), + required: false, + disabled: provider.seeded_from_env?, + caption: I18n.t("saml.instructions.idp_entity_id"), + input_width: :xlarge + ) end end end diff --git a/modules/auth_saml/app/forms/saml/providers/metadata_xml_form.rb b/modules/auth_saml/app/forms/saml/providers/metadata_xml_form.rb index 51024a6dc1c..f4ecd4425fc 100644 --- a/modules/auth_saml/app/forms/saml/providers/metadata_xml_form.rb +++ b/modules/auth_saml/app/forms/saml/providers/metadata_xml_form.rb @@ -42,6 +42,14 @@ module Saml rows: 10, input_width: :medium ) + f.text_field( + name: :idp_entity_id, + label: I18n.t("activerecord.attributes.saml/provider.idp_entity_id"), + required: false, + disabled: provider.seeded_from_env?, + caption: I18n.t("saml.instructions.idp_entity_id"), + input_width: :xlarge + ) end end end diff --git a/modules/auth_saml/app/models/saml/provider.rb b/modules/auth_saml/app/models/saml/provider.rb index e9aeae7fc51..1843c2e81e7 100644 --- a/modules/auth_saml/app/models/saml/provider.rb +++ b/modules/auth_saml/app/models/saml/provider.rb @@ -11,6 +11,7 @@ module Saml store_attribute :options, :metadata_xml, :string store_attribute :options, :last_metadata_update, :datetime + store_attribute :options, :idp_entity_id, :string store_attribute :options, :idp_sso_service_url, :string store_attribute :options, :idp_slo_service_url, :string diff --git a/modules/auth_saml/app/services/saml/metadata_document.rb b/modules/auth_saml/app/services/saml/metadata_document.rb new file mode 100644 index 00000000000..5d7855d46c9 --- /dev/null +++ b/modules/auth_saml/app/services/saml/metadata_document.rb @@ -0,0 +1,152 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module Saml + # Prepares SAML metadata XML for parsing by ruby-saml. + # + # Federation aggregates MAY contain thousands of individual entities. + # Using ruby-saml directly would load the full document into REXML, which is extremely slow. + # This class streams the XML and tries to extract the matching single EntityDescriptor when we can. + class MetadataDocument + class MetadataTooLargeError < StandardError; end + + class FederationMetadataError < StandardError; end + + MAX_SIZE = 150.megabytes + + def self.prepare(source, entity_id: nil) + new(source, entity_id:).prepare + end + + def initialize(source, entity_id: nil) + @source = source + @entity_id = entity_id.presence + end + + def prepare + if aggregate? + read_entity_fragment! + else + read_all + end + end + + def read_entity_fragment! + fragment = extract_entity_fragment + if fragment.nil? + message = + if @entity_id + "Entity '#{@entity_id}' not found in federation aggregate" + else + "No identity provider found in federation aggregate" + end + raise FederationMetadataError, message + end + + fragment + end + + # Decide whether the document is a federation aggregate by inspecting its root element. + # Using +Nokogiri::XML::Reader+, we only advance to the root element and stop based on it. + def aggregate? + with_reader_io do |io| + Nokogiri::XML::Reader(io).each do |node| + next unless node.node_type == Nokogiri::XML::Reader::TYPE_ELEMENT + + return node.local_name == "EntitiesDescriptor" + end + end + + false + end + + private + + def extract_entity_fragment + if @entity_id + find_entity_by_id + else + find_first_idp_entity + end + end + + # We try to prevent calling outer_xml on all entities when looking. + # Instead, we can only look at entityID attribute until the target is found, + # and then call outer_xml only on that fragment. + def find_entity_by_id + with_reader_io do |io| + Nokogiri::XML::Reader(io).each do |node| + next unless entity_descriptor_element?(node) + next unless node.attribute("entityID") == @entity_id + + return node.outer_xml + end + end + + nil + end + + def find_first_idp_entity + with_reader_io do |io| + Nokogiri::XML::Reader(io).each do |node| + next unless entity_descriptor_element?(node) + + fragment = node.outer_xml + return fragment if idp_descriptor_fragment?(fragment) + end + end + + nil + end + + def idp_descriptor_fragment?(fragment) + Nokogiri::XML.fragment(fragment).at_xpath(".//*[local-name()='IDPSSODescriptor']").present? + end + + def entity_descriptor_element?(node) + node.node_type == Nokogiri::XML::Reader::TYPE_ELEMENT && node.local_name == "EntityDescriptor" + end + + def read_all + return @source if @source.is_a?(String) + + with_reader_io(&:read) + end + + def with_reader_io(&) + if @source.is_a?(String) + StringIO.open(@source, &) + else + @source.rewind + yield @source + end + end + end +end diff --git a/modules/auth_saml/app/services/saml/metadata_fetcher.rb b/modules/auth_saml/app/services/saml/metadata_fetcher.rb new file mode 100644 index 00000000000..32711f7e4fd --- /dev/null +++ b/modules/auth_saml/app/services/saml/metadata_fetcher.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module Saml + class MetadataFetcher + include ActionView::Helpers::NumberHelper + + def self.fetch(url, &) + new(url).fetch(&) + end + + def initialize(url) + @url = url + end + + def fetch + Tempfile.create("saml-metadata") do |file| + file.binmode + + OpenProject::SsrfProtection.get(@url) do |response| + unless response.is_a?(Net::HTTPSuccess) + raise OneLogin::RubySaml::HttpError, + "Failed to fetch idp metadata: #{response.code}: #{response.message}" + end + + bytes_written = 0 + response.read_body do |chunk| + file.write(chunk) + bytes_written += chunk.bytesize + if bytes_written > MetadataDocument::MAX_SIZE + raise MetadataDocument::MetadataTooLargeError, + "Metadata exceeds max size of #{number_to_human_size(MetadataDocument::MAX_SIZE, precision: 2)}" + end + end + end + + file.rewind + yield file + end + end + end +end diff --git a/modules/auth_saml/app/services/saml/update_metadata_service.rb b/modules/auth_saml/app/services/saml/update_metadata_service.rb index 3bb26271904..aff7267ec1f 100644 --- a/modules/auth_saml/app/services/saml/update_metadata_service.rb +++ b/modules/auth_saml/app/services/saml/update_metadata_service.rb @@ -39,8 +39,14 @@ module Saml @provider = provider end - def call + def call # rubocop:disable Metrics/AbcSize apply_metadata(merge_certificates(fetch_metadata)) + rescue MetadataDocument::FederationMetadataError => e + OpenProject.logger.error(e) + ServiceResult.failure(result: provider, message: I18n.t("saml.metadata_parser.federation_metadata")) + rescue MetadataDocument::MetadataTooLargeError => e + OpenProject.logger.error(e) + ServiceResult.failure(result: provider, message: I18n.t("saml.metadata_parser.metadata_too_large")) rescue StandardError => e OpenProject.logger.error(e) ServiceResult.failure(result: provider, @@ -69,12 +75,16 @@ module Saml end def parse_xml - parser_instance.parse_to_hash(provider.metadata_xml) + xml = MetadataDocument.prepare(provider.metadata_xml, entity_id: provider.idp_entity_id) + parser_instance.parse_to_hash(xml) end def parse_url validate_metadata_url_host! - parser_instance.parse_remote_to_hash(provider.metadata_url) + MetadataFetcher.fetch(provider.metadata_url) do |file| + xml = MetadataDocument.prepare(file, entity_id: provider.idp_entity_id) + parser_instance.parse_to_hash(xml) + end end def validate_metadata_url_host! diff --git a/modules/auth_saml/config/locales/en.yml b/modules/auth_saml/config/locales/en.yml index 2ac0332c26e..611a4e662dd 100644 --- a/modules/auth_saml/config/locales/en.yml +++ b/modules/auth_saml/config/locales/en.yml @@ -12,6 +12,7 @@ en: sp_entity_id: Service entity ID metadata_url: Identity provider metadata URL name_identifier_format: Name identifier format + idp_entity_id: Identity provider entity ID idp_sso_service_url: Identity provider login endpoint idp_slo_service_url: Identity provider logout endpoint idp_cert: Public certificate of identity provider @@ -43,6 +44,10 @@ en: success: "Successfully updated the configuration using the identity provider metadata." invalid_url: "Provided metadata URL is invalid. Provide a HTTP(s) URL." error: "Failed to retrieve the identity provider metadata: %{error}" + federation_metadata: > + The metadata URL points to a federation aggregate (many identity providers). + Use your institution's direct metadata URL, or enter your institution's IdP entity ID in the metadata form and try again. + metadata_too_large: "The metadata file exceeds the maximum allowed size." providers: label_empty_title: "No SAML providers configured yet." label_empty_description: "Add a provider to see them here." @@ -107,6 +112,9 @@ en: Your identity provider does not have a metadata endpoint or XML download option. You can configure it manually. metadata_url: > Your identity provider provides a metadata URL. + idp_entity_id: > + Optional: Useful when the metadata URL points to a federation aggregate with a lot of entries. + Enter the entity ID of your institution's identity provider. Leave blank for single-entity metadata URLs. metadata_xml: > Your identity provider provides a metadata XML download. limit_self_registration: > diff --git a/modules/auth_saml/spec/fixtures/federation_metadata.xml b/modules/auth_saml/spec/fixtures/federation_metadata.xml new file mode 100644 index 00000000000..7ab347030a6 --- /dev/null +++ b/modules/auth_saml/spec/fixtures/federation_metadata.xml @@ -0,0 +1,443 @@ + + + + + + + + + + State University Portal + Student and staff portal of State University + + + + + + MIIDKzCCAhOgAwIBAgIUNKP1WT7mAMRllZ3z8EDR2to1VNYwDQYJKoZIhvcNAQELBQAwJTEjMCEG +A1UEAwwaaWRwLmV4YW1wbGUtdW5pdmVyc2l0eS5lZHUwHhcNMjYwNjAzMDg1NDI3WhcNMzYwNTMx +MDg1NDI3WjAlMSMwIQYDVQQDDBppZHAuZXhhbXBsZS11bml2ZXJzaXR5LmVkdTCCASIwDQYJKoZI +hvcNAQEBBQADggEPADCCAQoCggEBAK+dgrVLuW+JCM2ST/SfA1J8XX3aCgtuU18Z1Xb8PcmMjQCJ +Ow0PDIApqp/JJAyt7KDiv8cgJgAmPMGbVQpcmMcOInBu+AvTsndagVqV7V3ePLwN/WOm+NPVoB2g +EEYGhF32gwZSQ0SHtJphxy4KoJv8CspVWRTviFU/pi1t8HsWJBLW6U6Jb7eLySM/G//6AFWFULdH +GImkAk/9BfT7iCINYHsOW0MO237UKw90qShxtFCB/fqPRC6eldC2kFkod9eIw9x7cMuH74QGVsCd +XAv+HUUtd6ov8Vn0xwaDiDxneXgKEBMwdVl97B+s9egUuif2TuMSYF09Qx7KREyayC0CAwEAAaNT +MFEwHQYDVR0OBBYEFHh0ON9CYmfpcEQYlxixtd6kkmHVMB8GA1UdIwQYMBaAFHh0ON9CYmfpcEQY +lxixtd6kkmHVMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBABSLwFVGf8L9+alT +RU3n3D+vyUOvb5zb3hMER3WN0p323nQyeZ7WVSfIy3vsD2PcdsDinYpM9NV9P5zU+11PgAMRwqGv +UnQLbklJ3d/5ciwdCVPUk+/c8pVwOnrCa5D6m1C8Or4Pnoxihz7sdmGoaGXdlrDhcwzosKx/AcT1 +aJNor+3SzkXJLzNfrYypBeUS8XzlzH3lTY+J1aYrfzXNK06XPCMlckKVL4nFVe0yDrSUk9DiLojy +b90TaOSCqAGzoecoV8MOCxef6EvFgHmPpq8oMycWal4Ud2zdugUOtnNWxUVvWS15L7O7VG1eley/z +UrWMUi2pWJ1Gk1NAFs8/0A= + + + + + + + State University + State University + https://www.state-university.edu + + + Alex + Smith + mailto:asmith@state-university.edu + + + + + + + + + + + Tech Institute Library + Electronic resources portal for Tech Institute staff + + + + + + MIIDKzCCAhOgAwIBAgIUNKP1WT7mAMRllZ3z8EDR2to1VNYwDQYJKoZIhvcNAQELBQAwJTEjMCEG +A1UEAwwaaWRwLmV4YW1wbGUtdW5pdmVyc2l0eS5lZHUwHhcNMjYwNjAzMDg1NDI3WhcNMzYwNTMx +MDg1NDI3WjAlMSMwIQYDVQQDDBppZHAuZXhhbXBsZS11bml2ZXJzaXR5LmVkdTCCASIwDQYJKoZI +hvcNAQEBBQADggEPADCCAQoCggEBAK+dgrVLuW+JCM2ST/SfA1J8XX3aCgtuU18Z1Xb8PcmMjQCJ +Ow0PDIApqp/JJAyt7KDiv8cgJgAmPMGbVQpcmMcOInBu+AvTsndagVqV7V3ePLwN/WOm+NPVoB2g +EEYGhF32gwZSQ0SHtJphxy4KoJv8CspVWRTviFU/pi1t8HsWJBLW6U6Jb7eLySM/G//6AFWFULdH +GImkAk/9BfT7iCINYHsOW0MO237UKw90qShxtFCB/fqPRC6eldC2kFkod9eIw9x7cMuH74QGVsCd +XAv+HUUtd6ov8Vn0xwaDiDxneXgKEBMwdVl97B+s9egUuif2TuMSYF09Qx7KREyayC0CAwEAAaNT +MFEwHQYDVR0OBBYEFHh0ON9CYmfpcEQYlxixtd6kkmHVMB8GA1UdIwQYMBaAFHh0ON9CYmfpcEQY +lxixtd6kkmHVMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBABSLwFVGf8L9+alT +RU3n3D+vyUOvb5zb3hMER3WN0p323nQyeZ7WVSfIy3vsD2PcdsDinYpM9NV9P5zU+11PgAMRwqGv +UnQLbklJ3d/5ciwdCVPUk+/c8pVwOnrCa5D6m1C8Or4Pnoxihz7sdmGoaGXdlrDhcwzosKx/AcT1 +aJNor+3SzkXJLzNfrYypBeUS8XzlzH3lTY+J1aYrfzXNK06XPCMlckKVL4nFVe0yDrSUk9DiLojy +b90TaOSCqAGzoecoV8MOCxef6EvFgHmPpq8oMycWal4Ud2zdugUOtnNWxUVvWS15L7O7VG1eley/z +UrWMUi2pWJ1Gk1NAFs8/0A= + + + + + + + Tech Institute + Tech Institute + https://www.tech-institute.ac.uk + + + Jamie + Taylor + mailto:jtaylor@tech-institute.ac.uk + + + + + + + + + + + Research Center Data Portal + Scientific data access portal + + + + + + MIIDKzCCAhOgAwIBAgIUNKP1WT7mAMRllZ3z8EDR2to1VNYwDQYJKoZIhvcNAQELBQAwJTEjMCEG +A1UEAwwaaWRwLmV4YW1wbGUtdW5pdmVyc2l0eS5lZHUwHhcNMjYwNjAzMDg1NDI3WhcNMzYwNTMx +MDg1NDI3WjAlMSMwIQYDVQQDDBppZHAuZXhhbXBsZS11bml2ZXJzaXR5LmVkdTCCASIwDQYJKoZI +hvcNAQEBBQADggEPADCCAQoCggEBAK+dgrVLuW+JCM2ST/SfA1J8XX3aCgtuU18Z1Xb8PcmMjQCJ +Ow0PDIApqp/JJAyt7KDiv8cgJgAmPMGbVQpcmMcOInBu+AvTsndagVqV7V3ePLwN/WOm+NPVoB2g +EEYGhF32gwZSQ0SHtJphxy4KoJv8CspVWRTviFU/pi1t8HsWJBLW6U6Jb7eLySM/G//6AFWFULdH +GImkAk/9BfT7iCINYHsOW0MO237UKw90qShxtFCB/fqPRC6eldC2kFkod9eIw9x7cMuH74QGVsCd +XAv+HUUtd6ov8Vn0xwaDiDxneXgKEBMwdVl97B+s9egUuif2TuMSYF09Qx7KREyayC0CAwEAAaNT +MFEwHQYDVR0OBBYEFHh0ON9CYmfpcEQYlxixtd6kkmHVMB8GA1UdIwQYMBaAFHh0ON9CYmfpcEQY +lxixtd6kkmHVMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBABSLwFVGf8L9+alT +RU3n3D+vyUOvb5zb3hMER3WN0p323nQyeZ7WVSfIy3vsD2PcdsDinYpM9NV9P5zU+11PgAMRwqGv +UnQLbklJ3d/5ciwdCVPUk+/c8pVwOnrCa5D6m1C8Or4Pnoxihz7sdmGoaGXdlrDhcwzosKx/AcT1 +aJNor+3SzkXJLzNfrYypBeUS8XzlzH3lTY+J1aYrfzXNK06XPCMlckKVL4nFVe0yDrSUk9DiLojy +b90TaOSCqAGzoecoV8MOCxef6EvFgHmPpq8oMycWal4Ud2zdugUOtnNWxUVvWS15L7O7VG1eley/z +UrWMUi2pWJ1Gk1NAFs8/0A= + + + + + + + Research Center + Research Center + https://www.research-center.de + + + Maria + Weber + mailto:mweber@research-center.de + + + + + + + + + + + + + State University + Identity provider of State University + https://www.state-university.edu + + + + + + MIIDJzCCAg+gAwIBAgIULPAnn8haVhQcUbxjrZLjtbFX5RkwDQYJKoZIhvcNAQELBQAwIzEhMB8G +A1UEAwwYaWRwLnN0YXRlLXVuaXZlcnNpdHkuZWR1MB4XDTI2MDYwMzA4NTQ0OVoXDTM2MDUzMTA4 +NTQ0OVowIzEhMB8GA1UEAwwYaWRwLnN0YXRlLXVuaXZlcnNpdHkuZWR1MIIBIjANBgkqhkiG9w0B +AQEFAAOCAQ8AMIIBCgKCAQEAwo21vefLRM/2iK6LO4Sbl++0h+zrLuSSusY/0Wu3QjPAGklaqzQ0j +Dc2jbY5bXei4ebWuGxT5PCqPOFIaTQM8uJ5q7uVAo06ab6/uSfwjDJTNE7jokhMeilNh4GuoWTsb +CyGMDUbSdfTNTDCC/ELoWlTCErce+TuIZulG8Jzmmm3AKTmMjOW+Ch3HMLoKCmEyniKs7m0eB7Az +298bFCFKp0dpqkfre5U5WTYeFe39RBbZfDPl0bN0UxF1T7ITOf6tkryXPJXfQAnXDiCnB22A936M +5ou1XFTozkE8RBbgOjq34zBbM0V69rPnhh0yqFDIVUVtULcgyJQEIB5qpigDQIDAQABo1MwUTAdBg +NVHQ4EFgQUQ4UIhO5Q3RmegmL27mKP0j5QJQ4wHwYDVR0jBBgwFoAUQ4UIhO5Q3RmegmL27mKP0j +5QJQ4wDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEALUR5WnK0mbu2lPO6CXd7LTs +IcRdEq/qXRQC+KqofuS5dyZ0N2ip0zD50tG6y8YjWQHERQ89+ZEKv5VmD9+k4PnZpNfVjfmyeivq +bWohIn/0iNAUNmY/bam0qbz8SCiLPLVrKvu9BAGWcoV7UX9zf+qe+NNT8z1bFVR3+WcnnO/AEWyu +UVab0ud6HYp5xc36Jj3e98T50h5OaXpQpwfyuM7/p4WX3Ri5FRa0tNxlAy685cuL6B5zrbUOoc+Q +sc3FqNtGFM/TExrW9VY/kRQVDpuhqv/eUO7RfQJmv1FBhP+XMdXUudJ6aNFswI0P1UO/H9YGm93U +CXnNW8NGo/MwFSg== + + + + + urn:oasis:names:tc:SAML:2.0:nameid-format:transient + urn:oasis:names:tc:SAML:2.0:nameid-format:persistent + + + + + State University + State University + https://www.state-university.edu + + + Alex + Smith + mailto:asmith@state-university.edu + + + + + + + + + + + Tech Institute + Identity provider of Tech Institute + https://www.tech-institute.ac.uk + + + + + + MIIDJzCCAg+gAwIBAgIUFozhkEI22FNzfgPfPg6tXhOCsLMwDQYJKoZIhvcNAQELBQAwIzEhMB8G +A1UEAwwYaWRwLnRlY2gtaW5zdGl0dXRlLmFjLnVrMB4XDTI2MDYwMzA4NTQ0OVoXDTM2MDUzMTA4 +NTQ0OVowIzEhMB8GA1UEAwwYaWRwLnRlY2gtaW5zdGl0dXRlLmFjLnVrMIIBIjANBgkqhkiG9w0B +AQEFAAOCAQ8AMIIBCgKCAQEAodVdylnMKJ5d3p7vZqX4za/12IJVI7/Xsyi7S1UxfB52vktX9EZo +IxqO9GLMe9lIrHFOB8jU1I7BdYp8r0E4mo44YYikncOMKNaOuHF5J1WaJOUZd807jriGO9agZor9 +AmABRpl3w/WYVL9OZPj1mHngQPWaVcosM3OkxXwdULPdcxfe3VATHcNWi7uoLZsB0hpBwua47ldS +wThmZkVNjdjlD0BRiUCSynFtSY2vGxHz+D/aBvYhrGlBX4uTPCOxqybyC3dfadD9ZlvLfY0d1Yic +ZvNEybP8bPcu8xUwnsyL6bBsdat7/XXbawOsxEXxSR9wcv42ZfgKmKyTyUdpLQIDAQABo1MwUTAd +BgNVHQ4EFgQUDVmR5rn9fYj2XkwffcZAwCWKvfowHwYDVR0jBBgwFoAUDVmR5rn9fYj2XkwffcZA +wCWKvfowDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAnVbAE4SsEYhodobnZEWGu +z/747XH/m7DM9KM4lvKq6QOd/C/aJ7Lp9nwfl1gXIHdwdYozlOpak2+zfVFFbepvDOldldpScJzZ +sC+80PTta+h91OTug6Q+viRJV/OzQPozLJ6jz5Tox8dYn7T2XIfmM49jX+tAR861QopVaHLmZ3Lg +EHzsk+aIV6I6yyJfBmpy4+tO5OhKv3UHLCHmo7NPVhiX+pxe3+XCghdRtKMFCB9gwqJxYoybHb+a +NrbgZmhp6hjzd9tsFQDMSdkCSTHWCaqua5Hi4EfMIR75SKxUZvrCkTe4FOa+rG5bq26vdeKBZUCs +sd+OCMN3dWtH446Lf7Gw== + + + + + urn:oasis:names:tc:SAML:2.0:nameid-format:transient + + + + + Tech Institute + Tech Institute + https://www.tech-institute.ac.uk + + + Jamie + Taylor + mailto:jtaylor@tech-institute.ac.uk + + + + + + + + + + + Research Center + Identity provider of Research Center + https://www.research-center.de + + + + + + MIIDIzCCAgugAwIBAgIUcxlF9USQY3nQvvO8uqXJL7C8/XgwDQYJKoZIhvcNAQELBQAwITEfMB0G +A1UEAwwWaWRwLnJlc2VhcmNoLWNlbnRlci5kZTAeFw0yNjA2MDMwODU0NDlaFw0zNjA1MzEwODU0 +NDlaMCExHzAdBgNVBAMMFmlkcC5yZXNlYXJjaC1jZW50ZXIuZGUwggEiMA0GCSqGSIb3DQEBAQUA +A4IBDwAwggEKAoIBAQDPxGQuYQjgdNedu3dgelQQ5t+1SxM4an4xQav4rXqG2ZtmEmDS3WDfy7Kc +RKtSfW/6+zWmNUd6wQQnc+3VgnmOb+qRTSh9XCD3WFSrr/hG0HfrmzKtjo4nYK5gWLdXQwgwznu +R40IOxsKEEtkp2UgkFLMAtGjJ15Kyu6J3eV8MIpB468G+mXXXhbLw7y8uB+cyCTWGsG1aa1ahmZS +Rz7K9H1TafUSuarUis/+oXZO9+YJHNEi9iU4U6h9iTlqPkly146FefQlHsBys7T0Es46aIqiaCMC +ygCDxhKK4Rwf0E67I9QMFbTW1y8LF5hKs42NBzcf46iAkwYgPzisbqZtLAgMBAAGjUzBRMB0GA1Ud +DgQWBBSfqZUJOP+vllNKi2xk3Lbo8St4sDAfBgNVHSMEGDAWgBSfqZUJOP+vllNKi2xk3Lbo8St4 +sDAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBAJPRIMKcogeICnsF2pfca25Gln +5w13G95as+du3wx5ZVqQLH+JYLKcM6Uq7FVvWIrBwbOjCa9FEyUs1lATOtTkCZ3q05X/sw+sGMXI +r8H/vkDhOPkwIlKRaSRJHCBGN3xk8lUxev6fwwlU3Ht0fQEiOAWlc9N231wDYFDIAXmJbN/tG3/l +OfJUT1MXHdWe7QuJzmwLQ9OXogfdV2SskSQSqH9MKYcQ+qADc1lf0J0Or0GKSLvjz8YPLkwhXLGr +NhM+VyjD50hrg0DkUtco/8RTnkC3FkAvI/6WjpSiejpjW8Muh3rRAzxnfA9FQV0S0YXmlb/isM/l +Z6NiIkQ/8eB + + + + + urn:oasis:names:tc:SAML:2.0:nameid-format:transient + + + + + Research Center + Research Center + https://www.research-center.de + + + Maria + Weber + mailto:mweber@research-center.de + + + + + + + + + + + National Laboratory + Laboratoire National + Identity provider of National Laboratory + https://www.national-lab.fr + + + + + + MIIDHTCCAgWgAwIBAgIUUl2dXqMr4hJ6wf4fj/GOQlZAcpEwDQYJKoZIhvcNAQELBQAwHjEcMBoG +A1UEAwwTaWRwLm5hdGlvbmFsLWxhYi5mcjAeFw0yNjA2MDMwODU0NDlaFw0zNjA1MzEwODU0NDla +MB4xHDAaBgNVBAMME2lkcC5uYXRpb25hbC1sYWIuZnIwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAw +ggEKAoIBAQDAlY882lrAlhuadc5xMT6U8ELf5zH7adGzGjtP76Kx9SLYewDASPXY7nXUx3aM2kqs +o+MWX0iaVpnWYvV+LMFyH6Mg/12pZhRx8S/cQysYEV3loWVSi3lESkOK9ELR2BeWOrYyZtk3Pz3q +AJ/BoSFegxe7OxC866m20LMKG9sDBB88i9svxmxXYxSR/HkAEfarSruk37aLOD++Iud+FBfJvRJR +174MZj6gecIsGcXy2Zn41WLgjGl3/sWUK27SWoa9Up5vDTIeQ8lZ5ySbRT58M+eHCOxSVf/gFf7s +49+v5i/9iDKDMo5Hy+MDbdovUaDo6KQmiPg6cfzqT52AT39lAgMBAAGjUzBRMB0GA1UdDgQWBBTj +Gvmk9xkzVe57Cte2KOEnf3k2hzAfBgNVHSMEGDAWgBTjGvmk9xkzVe57Cte2KOEnf3k2hzAPBgNV +HRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQAMvHuuB2KGKl0n7d0MtJ2/fzKtJmJMMja2 +o1Mqj/rg2Nkq31wrFdXqayxypYDSXQgLoABkWYyZkWIhoWOnml2J9bUDJ8uYLulp4VbNZnAylAS5 +9hu/35ZEc9hmUrm9Q8JaevKdULneiIiR2R269oqf0QEJgF26ZzEEodH0oj7ZNOOuF2OXy+8R893Y +DSnLHCEtW6uZWjEQTsfbGVcppvYYho6/K0/saO0IPb1/LZ8z1uB7CWbsn5KRnqR0AvT2+R5xlIxE +k7uWFpPOkdfgfnJ9UbfN2tgDyiieyME3PZZ5sjGWs/StRfnt/z9FeB00T3chiF4ldtuIysmUFDLu +3fm5 + + + + + urn:oasis:names:tc:SAML:2.0:nameid-format:transient + urn:oasis:names:tc:SAML:2.0:nameid-format:persistent + + + + + National Laboratory + National Laboratory + https://www.national-lab.fr + + + Sophie + Martin + mailto:smartin@national-lab.fr + + + + + + + + + + + Polytechnic University + Universidade Politécnica + Identity provider of Polytechnic University + https://www.polytechnic.edu.br + + + + + + MIIDIzCCAgugAwIBAgIUcX6lAS/Bq/lth/N/EA1dj6b7aI4wDQYJKoZIhvcNAQELBQAwITEfMB0G +A1UEAwwWaWRwLnBvbHl0ZWNobmljLmVkdS5icjAeFw0yNjA2MDMwODU0NDlaFw0zNjA1MzEwODU0 +NDlaMCExHzAdBgNVBAMMFmlkcC5wb2x5dGVjaG5pYy5lZHUuYnIwggEiMA0GCSqGSIb3DQEBAQUA +A4IBDwAwggEKAoIBAQDgmBYTodZcgpqaimV8nFH/YH6I6Y2euIgEIIPTHVboHUXAgErOrSU1jzjF +2S1UX7uHiyg00GSJyQPBPn2i73nTplDSyj0TzmcDdI2uWyY8kygozuGrsjVv7T+krTswlCYD+GhF +09VK18Gk8VHX0DB/5GFFB03yORHSOi0MieJ5cIWNxDpSaj3UJyVnZdvA9jfxvJKIjVE3niNG82Gc +Y70VglYRr5Gm814eDAYxhfqJNfSZxKowCmxrcTiKRN/E/hrEzFAYxXaCsyaO5WYs49+IOt2pNkJH +t86ErHQEKjTF3sf10WsN9UzYz1zv74b6sqfT8GztSjmI7XcDN36ZooJBAgMBAAGjUzBRMB0GA1Ud +DgQWBBTF0YvxRtWXlt/f1Y3//c6aX/gxxzAfBgNVHSMEGDAWgBTF0YvxRtWXlt/f1Y3//c6aX/gx +xzAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQDVu6t9PvJ7ewKeJ05q7sFdNw9k +MEjDdim/m3Y6j31YGjSdBXmKnaH898hliATkgv7EnzgFkCT45NtT8aX1pETDEl9kgxL/TjhdwdHK +1q+Skou1NmJM+Zd7Zm8yvreA7Gj+YTiTL2iF4jeRTR+k04KZaQCLMgoxTQNgF/uxlitapX2+L+d +IyTikGTkfINXNRjhvx2t3rUqJoxn/bl1qM6uS4A/R5aOne6xve5Kd+lyUCOxB/TZuHpDZgB6O1FT +03HXTn4lmoCIiCPsjXhXqQOgTq+g8JPMUYtGAi4diNF+CWcD9Zv/1rdqCuGlk1O105HRIPcST5LF +4MzCa1nJBoVPj + + + + + urn:oasis:names:tc:SAML:2.0:nameid-format:transient + + + + + Polytechnic University + Polytechnic University + https://www.polytechnic.edu.br + + + Carlos + Silva + mailto:csilva@polytechnic.edu.br + + + + diff --git a/modules/auth_saml/spec/services/saml/metadata_document_spec.rb b/modules/auth_saml/spec/services/saml/metadata_document_spec.rb new file mode 100644 index 00000000000..3f7073a2e58 --- /dev/null +++ b/modules/auth_saml/spec/services/saml/metadata_document_spec.rb @@ -0,0 +1,141 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe Saml::MetadataDocument do + # Fixture: 3 SPs then 5 IdPs — see spec/fixtures/federation_metadata.xml + let(:federation_xml) { Rails.root.join("modules/auth_saml/spec/fixtures/federation_metadata.xml").read } + + # Minimal single EntityDescriptor — not an aggregate, so prepare returns it unchanged + let(:single_idp_xml) do + <<~XML + + + + + + XML + end + + # Entity IDs present in the fixture + let(:first_idp_entity_id) { "https://idp.state-university.edu/idp/shibboleth" } + let(:last_idp_entity_id) { "https://idp.polytechnic.edu.br/idp/shibboleth" } + let(:sp_entity_id) { "https://sp.state-university.edu/shibboleth" } + + describe ".prepare on a single-entity document" do + it "returns the XML unchanged" do + result = described_class.prepare(single_idp_xml) + expect(result).to eq(single_idp_xml) + end + end + + describe ".prepare on a federation aggregate" do + it "detects the aggregate" do + expect(described_class.new(federation_xml).aggregate?).to be(true) + end + + context "without an entity_id" do + it "returns the first IdP entity, skipping SP-only entries" do + result = described_class.prepare(federation_xml) + + expect(result).to include(first_idp_entity_id) + expect(result).to include("IDPSSODescriptor") + expect(result).not_to include("SPSSODescriptor") + end + end + + context "with a matching entity_id" do + it "extracts the requested IdP" do + result = described_class.prepare(federation_xml, entity_id: last_idp_entity_id) + + expect(result).to include(last_idp_entity_id) + expect(result).to include("IDPSSODescriptor") + end + + it "can also extract an SP entity by entity_id" do + result = described_class.prepare(federation_xml, entity_id: sp_entity_id) + + expect(result).to include(sp_entity_id) + expect(result).to include("SPSSODescriptor") + end + end + + context "with an entity_id not present in the aggregate" do + it "raises FederationMetadataError with the missing entity_id in the message" do + expect do + described_class.prepare(federation_xml, entity_id: "https://missing.example.com/idp/shibboleth") + end.to raise_error(described_class::FederationMetadataError, /missing\.example\.com/) + end + end + + context "when the aggregate contains no IdPs and no entity_id is given" do + let(:sp_only_aggregate) do + <<~XML + + + + + + XML + end + + it "raises FederationMetadataError" do + expect do + described_class.prepare(sp_only_aggregate) + end.to raise_error(described_class::FederationMetadataError) + end + end + end + + describe ".prepare on an IO source (Tempfile)" do + it "reads a single-entity file correctly" do + Tempfile.create("spec-metadata") do |f| + f.write(single_idp_xml) + f.rewind + expect(described_class.prepare(f)).to eq(single_idp_xml) + end + end + + it "extracts the first IdP from a federation aggregate file" do + Tempfile.create("spec-metadata") do |f| + f.write(federation_xml) + f.rewind + result = described_class.prepare(f) + expect(result).to include(first_idp_entity_id) + expect(result).to include("IDPSSODescriptor") + end + end + end +end diff --git a/modules/auth_saml/spec/services/saml/metadata_fetcher_spec.rb b/modules/auth_saml/spec/services/saml/metadata_fetcher_spec.rb new file mode 100644 index 00000000000..0a989534c01 --- /dev/null +++ b/modules/auth_saml/spec/services/saml/metadata_fetcher_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe Saml::MetadataFetcher do + let(:url) { "https://example.com/metadata" } + let(:response) { instance_double(Net::HTTPSuccess) } + + before do + allow(response).to receive(:is_a?).with(Net::HTTPSuccess).and_return(true) + allow(OpenProject::SsrfProtection).to receive(:get).and_yield(response) + end + + describe ".fetch" do + context "with a successful response" do + before do + allow(response).to receive(:read_body).and_yield("") + end + + it "yields a file with the response body rewound to the start" do + described_class.fetch(url) do |file| + expect(file).to be_a(File) + expect(file.pos).to eq(0) + expect(file.read).to eq("") + end + end + + it "removes the tempfile after the block" do + path = nil + described_class.fetch(url) do |file| + path = file.path + expect(File.exist?(path)).to be(true) + end + expect(File.exist?(path)).to be(false) + end + end + + context "when the response is not successful" do + let(:response) { instance_double(Net::HTTPNotFound, code: "404", message: "Not Found") } + + before do + allow(response).to receive(:is_a?).with(Net::HTTPSuccess).and_return(false) + allow(OpenProject::SsrfProtection).to receive(:get).and_yield(response) + end + + it "raises HttpError without yielding" do + yielded = false + expect do + described_class.fetch(url) { yielded = true } + end.to raise_error(OneLogin::RubySaml::HttpError, /404/) + expect(yielded).to be(false) + end + end + + context "when the response body exceeds MAX_SIZE" do + before do + allow(response).to receive(:read_body).and_yield("x" * (Saml::MetadataDocument::MAX_SIZE + 1)) + end + + it "raises MetadataTooLargeError without yielding" do + yielded = false + expect do + described_class.fetch(url) { yielded = true } + end.to raise_error(Saml::MetadataDocument::MetadataTooLargeError) + expect(yielded).to be(false) + end + end + end +end diff --git a/modules/auth_saml/spec/services/saml/update_metadata_service_spec.rb b/modules/auth_saml/spec/services/saml/update_metadata_service_spec.rb index 342d894b2a3..bc2c9ac1f47 100644 --- a/modules/auth_saml/spec/services/saml/update_metadata_service_spec.rb +++ b/modules/auth_saml/spec/services/saml/update_metadata_service_spec.rb @@ -231,22 +231,37 @@ RSpec.describe Saml::UpdateMetadataService do let(:metadata_url) { "https://example.com/metadata" } let(:provider) { Saml::Provider.new(metadata_url:) } let(:parser_instance) { instance_double(OneLogin::RubySaml::IdpMetadataParser) } + let(:http_response) { instance_double(Net::HTTPSuccess) } before do allow(OneLogin::RubySaml::IdpMetadataParser).to receive(:new).and_return(parser_instance) - allow(parser_instance).to receive(:parse_remote_to_hash).and_return({}) + allow(parser_instance).to receive(:parse_to_hash).and_return({}) + allow(Saml::MetadataDocument).to receive(:prepare).and_return("") + allow(http_response).to receive(:is_a?).with(Net::HTTPSuccess).and_return(true) + allow(http_response).to receive(:read_body).and_yield("") + allow(OpenProject::SsrfProtection).to receive(:get) end context "when the URL host resolves to a safe IP" do before do allow(OpenProject::SsrfProtection).to receive(:safe_ip?).with("example.com").and_return(IPAddr.new("93.184.216.34")) + allow(OpenProject::SsrfProtection).to receive(:get).and_yield(http_response) end - it "checks the host and fetches metadata remotely" do + it "checks the host, fetches metadata via MetadataFetcher, and cleans up the tempfile" do + fetched_file = nil + allow(Saml::MetadataDocument).to receive(:prepare) do |file, **| + fetched_file = file + "" + end + parse_metadata expect(OpenProject::SsrfProtection).to have_received(:safe_ip?).with("example.com") - expect(parser_instance).to have_received(:parse_remote_to_hash).with(metadata_url) + expect(OpenProject::SsrfProtection).to have_received(:get).with(metadata_url) + expect(Saml::MetadataDocument).to have_received(:prepare).with(instance_of(File), entity_id: nil) + expect(parser_instance).to have_received(:parse_to_hash).with("") + expect(fetched_file).to be_closed end end @@ -261,7 +276,7 @@ RSpec.describe Saml::UpdateMetadataService do expect(result).not_to be_success expect(result.message).to include("MetadataHostNotAllowedError") expect(OpenProject::SsrfProtection).to have_received(:safe_ip?).with("example.com") - expect(parser_instance).not_to have_received(:parse_remote_to_hash) + expect(OpenProject::SsrfProtection).not_to have_received(:get) end end end