From f733b10a280d216636e9230768e46697b35783f7 Mon Sep 17 00:00:00 2001 From: enriquepablo Date: Wed, 27 Nov 2024 13:57:07 +0100 Subject: [PATCH 1/3] Do not crash when entity-selection-profile attribute has invalid JSON --- src/pyff/samlmd.py | 19 ++- .../metadata/test-sp-trustinfo-in-attr.xml | 112 ++++++++++++++++++ 2 files changed, 127 insertions(+), 4 deletions(-) diff --git a/src/pyff/samlmd.py b/src/pyff/samlmd.py index 9be47176..c6f445b3 100644 --- a/src/pyff/samlmd.py +++ b/src/pyff/samlmd.py @@ -1047,14 +1047,25 @@ def discojson_sp_attr(e): if b64_trustinfos is None: return None + entityID = e.get('entityID', None) + if entityID is None: + return None + sp = {} - sp['entityID'] = e.get('entityID', None) + sp['entityID'] = entityID sp['profiles'] = {} for b64_trustinfo in b64_trustinfos: - str_trustinfo = b64decode(b64_trustinfo.encode('ascii')) - trustinfo = json.loads(str_trustinfo.decode('utf8')) - sp['profiles'].update(trustinfo['profiles']) + try: + str_trustinfo = b64decode(b64_trustinfo.encode('ascii')) + trustinfo = json.loads(str_trustinfo.decode('utf8')) + sp['profiles'].update(trustinfo['profiles']) + + except Exception as e: + log.warning(f"Invalid entity-selection-profile attribute for {entityID}: {e}") + + if not sp['profiles']: + return None return sp diff --git a/src/pyff/test/data/metadata/test-sp-trustinfo-in-attr.xml b/src/pyff/test/data/metadata/test-sp-trustinfo-in-attr.xml index b1532fda..12e0d268 100644 --- a/src/pyff/test/data/metadata/test-sp-trustinfo-in-attr.xml +++ b/src/pyff/test/data/metadata/test-sp-trustinfo-in-attr.xml @@ -71,6 +71,118 @@ fMou5aW0mZ+QgJNKOrxY5vFxUq6pn3OiYbBu3m1C9ajbU/nx2evzt4+qUwTfHFb+ ZgXpOtmxRekFzVvGZ18BSPJKwAAqqZ11X7skT/NwEAhbgplVPv9WkDmDzqNvHqQJ nyRgD2ZqUPU9nEOjGy0gI07dciVcYZQ+CiZeSECIWgQwjDEBDuwMCVAZA6gfdz6C KJuN+RUSKPEcxPxle1MiB4MU0ei5X4xUbvLWKn9Ok7TOXg2BpnMAv6eON1wVo0Aa +D265cqy6Le/toVg= + + + + + + + + + + + + + + + ICOS Carbon Portal SAML service + ICOS Kolportalens SAML tjänst + + + + + + + ICOS Carbon Portal + ICOS Kolportalen + Carbon Portal + Kolportalen + https://www.icos-cp.eu/ + https://www.icos-cp.eu/ + + + Oleg + Mirzov + mailto:oleg.mirzov@nateko.lu.se + + + Alex + Vermeulen + mailto:alex.vermeulen@nateko.lu.se + + + + + + http://swamid.se/policy/mdrps + + + + + + + + + + + + + + + + + + + + http://www.geant.net/uri/dataprotection-code-of-conduct/v1 + + + invalidValueForAttribute + + + + + + + + Carbon Portal authentication service + Kolportalens autentiseringstjänst + Single Sign On for services of ICOS Carbon Portal. Maintained by the Carbon Portal team at Physical Geography department (nateko.lu.se). + Single Sign On tjänst för ICOS Kolportalen. Hanteras av Carbon Portal teamet på INES (nateko.lu.se). + https://cpauth.icos-cp.eu/saml/privacyStatement + https://www.icos-cp.eu/ + https://www.icos-cp.eu/ + https://cpauth.icos-cp.eu/saml/privacyStatement + + + + + cpauth.icos-cp.eu + + CN=cpauth.icos-cp.eu + MIIEJzCCAw+gAwIBAgIJANC3VWNs7fbTMA0GCSqGSIb3DQEBCwUAMIGpMQswCQYD +VQQGEwJTRTERMA8GA1UECAwIU2vDg8KlbmUxDTALBgNVBAcMBEx1bmQxGzAZBgNV +BAoMEklDT1MgQ2FyYm9uIFBvcnRhbDEfMB0GA1UECwwWQXV0aGVudGljYXRpb24g +U2VydmljZTEaMBgGA1UEAwwRY3BhdXRoLmljb3MtY3AuZXUxHjAcBgkqhkiG9w0B +CQEWD2luZm9AaWNvcy1jcC5ldTAeFw0xNTAyMDUxMjI0MzZaFw0yNTAyMDIxMjI0 +MzZaMIGpMQswCQYDVQQGEwJTRTERMA8GA1UECAwIU2vDg8KlbmUxDTALBgNVBAcM +BEx1bmQxGzAZBgNVBAoMEklDT1MgQ2FyYm9uIFBvcnRhbDEfMB0GA1UECwwWQXV0 +aGVudGljYXRpb24gU2VydmljZTEaMBgGA1UEAwwRY3BhdXRoLmljb3MtY3AuZXUx +HjAcBgkqhkiG9w0BCQEWD2luZm9AaWNvcy1jcC5ldTCCASIwDQYJKoZIhvcNAQEB +BQADggEPADCCAQoCggEBAM2QN1jaZJeuPAH+4sVMZKk7vg4JIbUuTMKk0+KIAg5M +XiVsRiEUjY+LtIncrvA/kf2CIySI0WkbwZMjcDd03hNj4kLWhuyxfOCwDO6DsUbG +MbyI6HIYWXJp5ljfEEFgtMqT3dDtD5vwq8h4Zy20ukxOoIokKczrAvn4JjkMsj6Z +0CEAFBC29o4E8PWQbUBgvt6Z+2ao+RHMLD7nZVBx98Occ9KfnYnDDd9Oi1XFe009 +zaSbcqY2RpN8I9hcW/KQf3KnGW5xZ5dr4rhGklCkYr+h0W3xKu+hin8bk91t1Dkr +gaKl/N7M3Oof3k+7ZBlwaV97es5InWCeNgDxCGkBRNsCAwEAAaNQME4wHQYDVR0O +BBYEFDcD7MVudooGaNRYqXBYqQi3VzGxMB8GA1UdIwQYMBaAFDcD7MVudooGaNRY +qXBYqQi3VzGxMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBABS02eZS +weXGMJ2fEIy2JH0VhCbjuX/rz+8Hfh9LjzNb3QwKHuwP83yvPqRulV9FYmvOoK8T +fMou5aW0mZ+QgJNKOrxY5vFxUq6pn3OiYbBu3m1C9ajbU/nx2evzt4+qUwTfHFb+ +ZgXpOtmxRekFzVvGZ18BSPJKwAAqqZ11X7skT/NwEAhbgplVPv9WkDmDzqNvHqQJ +nyRgD2ZqUPU9nEOjGy0gI07dciVcYZQ+CiZeSECIWgQwjDEBDuwMCVAZA6gfdz6C +KJuN+RUSKPEcxPxle1MiB4MU0ei5X4xUbvLWKn9Ok7TOXg2BpnMAv6eON1wVo0Aa D265cqy6Le/toVg= From b39220353b3bd1b618b7fd500175665a525a65dc Mon Sep 17 00:00:00 2001 From: enriquepablo Date: Mon, 13 Jan 2025 11:51:36 +0100 Subject: [PATCH 2/3] warn when trust info has profiles with same name. Also allow extra metadata in trust info in entity attribute. --- src/pyff/samlmd.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/pyff/samlmd.py b/src/pyff/samlmd.py index c6f445b3..6593ee52 100644 --- a/src/pyff/samlmd.py +++ b/src/pyff/samlmd.py @@ -1048,18 +1048,27 @@ def discojson_sp_attr(e): return None entityID = e.get('entityID', None) - if entityID is None: - return None - sp = {} sp['entityID'] = entityID sp['profiles'] = {} + sp['extra_md'] = {} for b64_trustinfo in b64_trustinfos: try: str_trustinfo = b64decode(b64_trustinfo.encode('ascii')) trustinfo = json.loads(str_trustinfo.decode('utf8')) - sp['profiles'].update(trustinfo['profiles']) + for profile in trustinfo['profiles']: + if profile in sp['profiles']: + log.warning(f"SP Entity {entityID} has a duplicate trust profile {profile}") + else: + sp['profiles'][profile] = trustinfo['profiles'][profile] + + if 'extra_md' in trustinfo: + for extra_id in trustinfo['extra_md']: + if extra_id in sp['extra_md']: + log.warning(f"SP Entity {entityID} has a duplicate extra IdP metadata {extra_id}") + else: + sp['extra_md'][extra_id] = trustinfo['extra_md'][extra_id] except Exception as e: log.warning(f"Invalid entity-selection-profile attribute for {entityID}: {e}") From f939db30992bd25e551c466cd52b092147f94d54 Mon Sep 17 00:00:00 2001 From: enriquepablo Date: Wed, 8 Jan 2025 16:36:27 +0100 Subject: [PATCH 3/3] fix problem with non-standard OA ScopedUIInfo, close #287 --- src/pyff/builtins.py | 6 +- src/pyff/samlmd.py | 20 +++---- src/pyff/store.py | 6 +- .../metadata/test-scoped-display-name.xml | 60 +++++++++++++++++++ src/pyff/test/test_pipeline.py | 33 ++++++++++ 5 files changed, 109 insertions(+), 16 deletions(-) create mode 100644 src/pyff/test/data/metadata/test-scoped-display-name.xml diff --git a/src/pyff/builtins.py b/src/pyff/builtins.py index 8bdd3a2d..4fee0ab8 100644 --- a/src/pyff/builtins.py +++ b/src/pyff/builtins.py @@ -844,14 +844,14 @@ def select(req: Plumbing.Request, *opts): def _strings(elt): lst = [] for attr in [ - '{%s}DisplayName' % NS['mdui'], + './/{%s}UIInfo/{%s}DisplayName' % (NS['mdui'], NS['mdui']), '{%s}ServiceName' % NS['md'], '{%s}OrganizationDisplayName' % NS['md'], '{%s}OrganizationName' % NS['md'], - '{%s}Keywords' % NS['mdui'], + './/{%s}UIInfo/{%s}Keywords' % (NS['mdui'], NS['mdui']), '{%s}Scope' % NS['shibmd'], ]: - lst.extend([s.text for s in elt.iter(attr)]) + lst.extend([s.text for s in elt.iterfind(attr)]) lst.append(elt.get('entityID')) return [item for item in lst if item is not None] diff --git a/src/pyff/samlmd.py b/src/pyff/samlmd.py index 6593ee52..6c34bffb 100644 --- a/src/pyff/samlmd.py +++ b/src/pyff/samlmd.py @@ -698,12 +698,12 @@ def gen_icon(e): def entity_icon_url(e, langs=None): - for ico in filter_lang(e.iter("{%s}Logo" % NS['mdui']), langs=langs): + for ico in filter_lang(e.iterfind(".//{%s}UIInfo/{%s}Logo" % (NS['mdui'], NS['mdui'])), langs=langs): return dict(url=ico.text, width=ico.get('width'), height=ico.get('height')) def privacy_statement_url(entity, langs): - for url in filter_lang(entity.iter("{%s}PrivacyStatementURL" % NS['mdui']), langs=langs): + for url in filter_lang(entity.iterfind(".//{%s}UIInfo/{%s}PrivacyStatementURL" % (NS['mdui'], NS['mdui'])), langs=langs): return url.text @@ -732,12 +732,12 @@ def entity_extended_display_i18n(entity, default_lang=None): ) name_dict.update(lang_dict(entity.iter("{%s}ServiceName" % NS['md']), lambda e: e.text, default_lang=default_lang)) name_dict.update( - lang_dict(entity.iter("{%s}DisplayName" % NS['mdui']), lambda e: e.text, default_lang=default_lang) + lang_dict(entity.iterfind(".//{%s}UIInfo/{%s}DisplayName" % (NS['mdui'], NS['mdui'])), lambda e: e.text, default_lang=default_lang) ) desc_dict = lang_dict(entity.iter("{%s}OrganizationURL" % NS['md']), lambda e: e.text, default_lang=default_lang) desc_dict.update( - lang_dict(entity.iter("{%s}Description" % NS['mdui']), lambda e: e.text, default_lang=default_lang) + lang_dict(entity.iterfind(".//{%s}UIInfo/{%s}Description" % (NS['mdui'], NS['mdui'])), lambda e: e.text, default_lang=default_lang) ) return name_dict, desc_dict @@ -825,7 +825,7 @@ def entity_extended_display(entity, langs=None): display = serviceName.text break - for displayName in filter_lang(entity.iter("{%s}DisplayName" % NS['mdui']), langs=langs): + for displayName in filter_lang(entity.iterfind(".//{%s}UIInfo/{%s}DisplayName" % (NS['mdui'], NS['mdui'])), langs=langs): info = display display = displayName.text break @@ -834,7 +834,7 @@ def entity_extended_display(entity, langs=None): info = organizationUrl.text break - for description in filter_lang(entity.iter("{%s}Description" % NS['mdui']), langs=langs): + for description in filter_lang(entity.iterfind(".//{%s}UIInfo/{%s}Description" % (NS['mdui'], NS['mdui'])), langs=langs): info = description.text break @@ -850,7 +850,7 @@ def entity_display_name(entity: Element, langs=None) -> str: :param entity: An EntityDescriptor element :param langs: The list of languages to search in priority order """ - for displayName in filter_lang(entity.iter("{%s}DisplayName" % NS['mdui']), langs=langs): + for displayName in filter_lang(entity.iterfind(".//{%s}UIInfo/{%s}DisplayName" % (NS['mdui'], NS['mdui'])), langs=langs): return displayName.text.strip() for serviceName in filter_lang(entity.iter("{%s}ServiceName" % NS['md']), langs=langs): @@ -946,7 +946,7 @@ def discojson(e, sources=None, langs=None, fallback_to_favicon=False, icon_store icon_info['url'] = ico d['entity_icon_url'] = icon_info - keywords = filter_lang(e.iter("{%s}Keywords" % NS['mdui']), langs=langs) + keywords = filter_lang(e.iterfind(".//{%s}UIInfo/{%s}Keywords" % (NS['mdui'], NS['mdui'])), langs=langs) if keywords is not None: lst = [elt.text for elt in keywords] if len(lst) > 0: @@ -1223,7 +1223,7 @@ def entity_simple_info(e, langs=None): d['service_name'] = entity_service_name(e, langs) d['service_descr'] = entity_service_description(e, langs) d['entity_attributes'] = entity_attribute_dict(e) - keywords = filter_lang(e.iter("{%s}Keywords" % NS['mdui']), langs=langs) + keywords = filter_lang(e.iterfind(".//{%s}UIInfo/{%s}Keywords" % (NS['mdui'], NS['mdui'])), langs=langs) if keywords is not None: lst = [elt.text for elt in keywords] if len(lst) > 0: @@ -1233,7 +1233,7 @@ def entity_simple_info(e, langs=None): def entity_info(e, langs=None): d = entity_simple_summary(e) - keywords = filter_lang(e.iter("{%s}Keywords" % NS['mdui']), langs=langs) + keywords = filter_lang(e.iterfind(".//{%s}UIInfo/{%s}Keywords" % (NS['mdui'], NS['mdui'])), langs=langs) if keywords is not None: lst = [elt.text for elt in keywords] if len(lst) > 0: diff --git a/src/pyff/store.py b/src/pyff/store.py index c4c31b33..f0f197a1 100644 --- a/src/pyff/store.py +++ b/src/pyff/store.py @@ -413,14 +413,14 @@ def search(self, query=None, path=None, entity_filter=None, related=None): def _strings(elt): lst = [] for attr in [ - '{%s}DisplayName' % NS['mdui'], + './/{%s}UIInfo/{%s}DisplayName' % (NS['mdui'], NS['mdui']), '{%s}ServiceName' % NS['md'], '{%s}OrganizationDisplayName' % NS['md'], '{%s}OrganizationName' % NS['md'], - '{%s}Keywords' % NS['mdui'], + './/{%s}UIInfo/{%s}Keywords' % (NS['mdui'], NS['mdui']), '{%s}Scope' % NS['shibmd'], ]: - lst.extend([s.text for s in elt.iter(attr)]) + lst.extend([s.text for s in elt.iterfind(attr)]) lst.append(elt.get('entityID')) return [item for item in lst if item is not None] diff --git a/src/pyff/test/data/metadata/test-scoped-display-name.xml b/src/pyff/test/data/metadata/test-scoped-display-name.xml new file mode 100644 index 00000000..c3aa6a68 --- /dev/null +++ b/src/pyff/test/data/metadata/test-scoped-display-name.xml @@ -0,0 +1,60 @@ + + + + + example.com + 81098135.example.com + + Scoped Example Universitet + Scoped Example University + + + Example universitet + Example University + Identity Provider för Example universitet + Identity Provider for Example University + http://www.example.com/ + http://www.example.com/english/ + https://www.example.com/static/images/umu_logo.jpg + https://www.example.com/static/images/logo.jpg + https://www.example.com/static/images/logo_eng.jpg + exempel + example + + + example.com + example.net + 10.0.0.0/8 + + + + + urn:oasis:names:tc:SAML:2.0:nameid-format:transient + + + + ExempelU + ExampleU + Exempel Universitetet + The Example University + http://www.example.com + http://www.example.com/english + + + Example University + Example helpdesk + helpdesk@example.com + + + Example University + Example helpdesk + helpdesk@example.com + + + Example University + Servicedesk Example universitet + support@example.com + + diff --git a/src/pyff/test/test_pipeline.py b/src/pyff/test/test_pipeline.py index d4c687f4..44e95094 100644 --- a/src/pyff/test/test_pipeline.py +++ b/src/pyff/test/test_pipeline.py @@ -795,3 +795,36 @@ def test_discojson_sp_trustinfo_in_attr(self): pass finally: shutil.rmtree(tmpdir) + + def test_discojson_scoped_display_name(self): + with patch.multiple("sys", exit=self.sys_exit): + tmpdir = tempfile.mkdtemp() + os.rmdir(tmpdir) # lets make sure 'store' can recreate it + try: + self.exec_pipeline(""" +- load: + - file://%s/metadata/test-scoped-display-name.xml +- select +- discojson +- publish: + output: %s/disco.json + raw: true + update_store: false +""" % (self.datadir, tmpdir)) + fn = "%s/disco.json" % tmpdir + assert os.path.exists(fn) + with open(fn, 'r') as f: + idps_json = json.load(f) + + idp_json = idps_json[0] + assert idp_json['title'] == 'Example University' + assert idp_json['title_langs']['sv'] == 'Example universitet' + assert idp_json['title_langs']['en'] == 'Example University' + assert idp_json['descr'] == 'Identity Provider for Example University' + assert idp_json['descr_langs']['sv'] == 'Identity Provider för Example universitet' + assert idp_json['descr_langs']['en'] == 'Identity Provider for Example University' + assert idp_json['keywords'] == 'example' + except IOError: + pass + finally: + shutil.rmtree(tmpdir)