From 9a28de1588d2a5b221f4c51f3ebc8ef15d94754b Mon Sep 17 00:00:00 2001 From: Uwe Winter Date: Fri, 12 Jan 2024 15:34:07 +1100 Subject: [PATCH 1/8] add schema validation to oidc_backends_config. This mechanism can be used for all XML files. Mention accepted_audiences in oidc_backends_config.xml.sample --- lib/galaxy/authnz/managers.py | 5 +- .../sample/oidc_backends_config.xml.sample | 3 + .../config/schemas/oidc_backends_config.xsd | 151 ++++++++++++++++++ lib/galaxy/util/__init__.py | 12 +- 4 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 lib/galaxy/config/schemas/oidc_backends_config.xsd diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index f6cb4ef8b3e2..0b40570756af 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -14,6 +14,7 @@ exceptions, model, ) +from galaxy.config import GALAXY_SCHEMAS_PATH from galaxy.util import ( asbool, etree, @@ -34,6 +35,8 @@ Strategy, ) +OIDC_BACKEND_SCHEMA = GALAXY_SCHEMAS_PATH / "oidc_backends_config.xsd" + log = logging.getLogger(__name__) # Note: This if for backward compatibility. Icons can be specified in oidc_backends_config.xml. @@ -105,7 +108,7 @@ def _parse_oidc_backends_config(self, config_file): self.oidc_backends_config = {} self.oidc_backends_implementation = {} try: - tree = parse_xml(config_file) + tree = parse_xml(config_file, OIDC_BACKEND_SCHEMA) root = tree.getroot() if root.tag != "OIDC": raise etree.ParseError( diff --git a/lib/galaxy/config/sample/oidc_backends_config.xml.sample b/lib/galaxy/config/sample/oidc_backends_config.xml.sample index 5b2c558e75b6..3683c98bdfde 100644 --- a/lib/galaxy/config/sample/oidc_backends_config.xml.sample +++ b/lib/galaxy/config/sample/oidc_backends_config.xml.sample @@ -150,6 +150,9 @@ Please mind `http` and `https`. + + diff --git a/lib/galaxy/config/schemas/oidc_backends_config.xsd b/lib/galaxy/config/schemas/oidc_backends_config.xsd new file mode 100644 index 000000000000..a8ffe7a0cd5b --- /dev/null +++ b/lib/galaxy/config/schemas/oidc_backends_config.xsd @@ -0,0 +1,151 @@ + + + + + + + Root element for OpenID Connect (OIDC) configurations, encompassing multiple identity providers. + + + + + + + + Configuration for a specific OIDC Identity Provider (IdP). + + + + + + + Client ID obtained from the IdP at client registration. + + + + + + + Secret generated by the IdP for the client upon registration. + + + + + + + URI where the IdP will send the authentication response. + + + + + + + Determines whether the IdP should prompt the user for re-authorization and consent. + + + + + + + URL to an icon representing the IdP. + + + + + + + Additional scopes requested from the IdP. + + + + + + + Indicates whether a confirmation page is shown for new user creation. + + + + + + + Path to a CA bundle file or directory for SSL certificate verification. + + + + + + + Override the default OIDC configuration URI. + + + + + + + Restricts the list of allowed identity providers for authentication. + + + + + + + Enable logout from the IdP when logging out of the application. + + + + + + + Custom label for the IdP in the user interface. + + + + + + + URL of the IdP. + + + + + + + API URL for the IdP, if different from the main URL. + + + + + + + Indicates support for Proof Key for Code Exchange (PKCE). + + + + + + + Hint to preselect the IdP during authentication. + + + + + + + Specifies the accepted audiences for authentication tokens. + + + + + + + + Name of the Identity Provider (IdP). + + + + + + + + + diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index 541921b574d1..0fa4f30f7ef6 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -294,7 +294,7 @@ def unique_id(KEY_SIZE=128): return md5(random_bits).hexdigest() -def parse_xml(fname: StrPath, strip_whitespace=True, remove_comments=True) -> ElementTree: +def parse_xml(fname: StrPath, schemaFname: StrPath | None = None, strip_whitespace=True, remove_comments=True) -> ElementTree: """Returns a parsed xml tree""" parser = None if remove_comments and LXML_AVAILABLE: @@ -302,7 +302,15 @@ def parse_xml(fname: StrPath, strip_whitespace=True, remove_comments=True) -> El # but lxml doesn't do this by default parser = etree.XMLParser(remove_comments=remove_comments) try: + schema = None + if schemaFname: + with open(str(schemaFname),"rb") as schema_file: + schema_root = etree.XML(schema_file.read()) + schema = etree.XMLSchema(schema_root) + tree = etree.parse(str(fname), parser=parser) + if schema: + schema.assertValid(tree) root = tree.getroot() if strip_whitespace: for elem in root.iter("*"): @@ -318,6 +326,8 @@ def parse_xml(fname: StrPath, strip_whitespace=True, remove_comments=True) -> El except etree.ParseError: log.exception("Error parsing file %s", fname) raise + except etree.DocumentInvalid as e: + log.exception("Validation of file %s failed with error {e}"%fname) return tree From ca1b877066fa62caf8a754258b17ab35f3b67764 Mon Sep 17 00:00:00 2001 From: Uwe Winter Date: Fri, 12 Jan 2024 15:44:29 +1100 Subject: [PATCH 2/8] Use Union and lower_case param --- lib/galaxy/util/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index 0fa4f30f7ef6..82d5a74f7aa5 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -294,7 +294,7 @@ def unique_id(KEY_SIZE=128): return md5(random_bits).hexdigest() -def parse_xml(fname: StrPath, schemaFname: StrPath | None = None, strip_whitespace=True, remove_comments=True) -> ElementTree: +def parse_xml(fname: StrPath, schemafname: Union[StrPath,None] = None, strip_whitespace=True, remove_comments=True) -> ElementTree: """Returns a parsed xml tree""" parser = None if remove_comments and LXML_AVAILABLE: @@ -303,8 +303,8 @@ def parse_xml(fname: StrPath, schemaFname: StrPath | None = None, strip_whitespa parser = etree.XMLParser(remove_comments=remove_comments) try: schema = None - if schemaFname: - with open(str(schemaFname),"rb") as schema_file: + if schemafname: + with open(str(schemafname),"rb") as schema_file: schema_root = etree.XML(schema_file.read()) schema = etree.XMLSchema(schema_root) From a9f2ef1e20e24888b31976e991f125fb696248be Mon Sep 17 00:00:00 2001 From: Uwe Winter Date: Fri, 12 Jan 2024 15:50:03 +1100 Subject: [PATCH 3/8] Fix f string not being used correctly in exception logging. --- lib/galaxy/util/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index 82d5a74f7aa5..c3666316b2a3 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -327,7 +327,7 @@ def parse_xml(fname: StrPath, schemafname: Union[StrPath,None] = None, strip_whi log.exception("Error parsing file %s", fname) raise except etree.DocumentInvalid as e: - log.exception("Validation of file %s failed with error {e}"%fname) + log.exception(f"Validation of file %s failed with error {e}"%fname) return tree From e6d8087b9ecd4cc1d99d06b8d6737ff4a37c1a6f Mon Sep 17 00:00:00 2001 From: Uwe Winter Date: Fri, 12 Jan 2024 15:55:17 +1100 Subject: [PATCH 4/8] Fix linting issues --- lib/galaxy/util/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index c3666316b2a3..3eb55962ce16 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -294,7 +294,7 @@ def unique_id(KEY_SIZE=128): return md5(random_bits).hexdigest() -def parse_xml(fname: StrPath, schemafname: Union[StrPath,None] = None, strip_whitespace=True, remove_comments=True) -> ElementTree: +def parse_xml(fname: StrPath, schemafname: Union[StrPath, None] = None, strip_whitespace=True, remove_comments=True) -> ElementTree: """Returns a parsed xml tree""" parser = None if remove_comments and LXML_AVAILABLE: @@ -304,7 +304,7 @@ def parse_xml(fname: StrPath, schemafname: Union[StrPath,None] = None, strip_whi try: schema = None if schemafname: - with open(str(schemafname),"rb") as schema_file: + with open(str(schemafname), "rb") as schema_file: schema_root = etree.XML(schema_file.read()) schema = etree.XMLSchema(schema_root) @@ -327,7 +327,7 @@ def parse_xml(fname: StrPath, schemafname: Union[StrPath,None] = None, strip_whi log.exception("Error parsing file %s", fname) raise except etree.DocumentInvalid as e: - log.exception(f"Validation of file %s failed with error {e}"%fname) + log.exception(f"Validation of file %s failed with error {e}" % fname) return tree From e1d8f874b710a27304feb80462311454007326dd Mon Sep 17 00:00:00 2001 From: Uwe Winter Date: Fri, 12 Jan 2024 16:02:52 +1100 Subject: [PATCH 5/8] Fix formatting issues --- lib/galaxy/util/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index 3eb55962ce16..197639136810 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -294,7 +294,10 @@ def unique_id(KEY_SIZE=128): return md5(random_bits).hexdigest() -def parse_xml(fname: StrPath, schemafname: Union[StrPath, None] = None, strip_whitespace=True, remove_comments=True) -> ElementTree: +def parse_xml(fname: StrPath, + schemafname: Union[StrPath, None] = None, + strip_whitespace=True, + remove_comments=True) -> ElementTree: """Returns a parsed xml tree""" parser = None if remove_comments and LXML_AVAILABLE: From 777862edf682f78f18804631a3c18650f729ee20 Mon Sep 17 00:00:00 2001 From: Uwe Winter Date: Fri, 12 Jan 2024 16:13:01 +1100 Subject: [PATCH 6/8] fix final formatting issue (finally make format doesn't die on me anymore) --- lib/galaxy/util/__init__.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index 197639136810..a299d817285d 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -294,10 +294,9 @@ def unique_id(KEY_SIZE=128): return md5(random_bits).hexdigest() -def parse_xml(fname: StrPath, - schemafname: Union[StrPath, None] = None, - strip_whitespace=True, - remove_comments=True) -> ElementTree: +def parse_xml( + fname: StrPath, schemafname: Union[StrPath, None] = None, strip_whitespace=True, remove_comments=True +) -> ElementTree: """Returns a parsed xml tree""" parser = None if remove_comments and LXML_AVAILABLE: From 57997e002746d30a207d292c771380ab6e52a33d Mon Sep 17 00:00:00 2001 From: Uwe Winter Date: Mon, 15 Jan 2024 13:10:49 +1100 Subject: [PATCH 7/8] move location of XSD to lib/galaxy/authnz/xsd change processing order for validation to occur after pre-processing the XML ensure validation is only done if lxml is available raise validation error, such that issue is immediately visible to the user. --- lib/galaxy/authnz/managers.py | 9 +++++++-- .../xsd}/oidc_backends_config.xsd | 0 lib/galaxy/util/__init__.py | 18 ++++++++++-------- 3 files changed, 17 insertions(+), 10 deletions(-) rename lib/galaxy/{config/schemas => authnz/xsd}/oidc_backends_config.xsd (100%) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index 0b40570756af..411f9d39f7bf 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -5,6 +5,7 @@ import os import random import string +import sys import requests from cloudauthz import CloudAuthz @@ -14,7 +15,6 @@ exceptions, model, ) -from galaxy.config import GALAXY_SCHEMAS_PATH from galaxy.util import ( asbool, etree, @@ -35,7 +35,12 @@ Strategy, ) -OIDC_BACKEND_SCHEMA = GALAXY_SCHEMAS_PATH / "oidc_backends_config.xsd" +if sys.version_info >= (3, 9): + from importlib.resources import files +else: + from importlib_resources import files + +OIDC_BACKEND_SCHEMA = files("galaxy.authnz.xsd") / "oidc_backends_config.xsd" log = logging.getLogger(__name__) diff --git a/lib/galaxy/config/schemas/oidc_backends_config.xsd b/lib/galaxy/authnz/xsd/oidc_backends_config.xsd similarity index 100% rename from lib/galaxy/config/schemas/oidc_backends_config.xsd rename to lib/galaxy/authnz/xsd/oidc_backends_config.xsd diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index a299d817285d..69c14889a387 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -299,20 +299,19 @@ def parse_xml( ) -> ElementTree: """Returns a parsed xml tree""" parser = None + schema = None if remove_comments and LXML_AVAILABLE: # If using stdlib etree comments are always removed, # but lxml doesn't do this by default parser = etree.XMLParser(remove_comments=remove_comments) - try: - schema = None - if schemafname: - with open(str(schemafname), "rb") as schema_file: - schema_root = etree.XML(schema_file.read()) - schema = etree.XMLSchema(schema_root) + if LXML_AVAILABLE and schemafname: + with open(str(schemafname), "rb") as schema_file: + schema_root = etree.XML(schema_file.read()) + schema = etree.XMLSchema(schema_root) + + try: tree = etree.parse(str(fname), parser=parser) - if schema: - schema.assertValid(tree) root = tree.getroot() if strip_whitespace: for elem in root.iter("*"): @@ -320,6 +319,8 @@ def parse_xml( elem.text = elem.text.strip() if elem.tail is not None: elem.tail = elem.tail.strip() + if schema: + schema.assertValid(tree) except OSError as e: if e.errno is None and not os.path.exists(fname): # lxml doesn't set errno @@ -330,6 +331,7 @@ def parse_xml( raise except etree.DocumentInvalid as e: log.exception(f"Validation of file %s failed with error {e}" % fname) + raise return tree From eb36ad387311ef8f950ac2c78843431f785498f0 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 15 Jan 2024 10:52:20 +0100 Subject: [PATCH 8/8] Use galaxy.util.resources.files --- lib/galaxy/authnz/managers.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index 411f9d39f7bf..fd37c8c06cea 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -5,7 +5,6 @@ import os import random import string -import sys import requests from cloudauthz import CloudAuthz @@ -23,6 +22,7 @@ string_as_bool, unicodify, ) +from galaxy.util.resources import files from .custos_authnz import ( CustosAuthFactory, KEYCLOAK_BACKENDS, @@ -35,11 +35,6 @@ Strategy, ) -if sys.version_info >= (3, 9): - from importlib.resources import files -else: - from importlib_resources import files - OIDC_BACKEND_SCHEMA = files("galaxy.authnz.xsd") / "oidc_backends_config.xsd" log = logging.getLogger(__name__)