From df9b35d7aa3f89a74a1a95ee0b96306f730d3f15 Mon Sep 17 00:00:00 2001 From: Fredrik Thulin Date: Wed, 8 May 2019 16:33:47 +0200 Subject: Don't add AllowCreate for default transient name ids http://docs.oasis-open.org/security/saml/v2.0/errata05/os/saml-v2.0-errata05-os.html#__RefHeading__8058_1983180497: "The use of the AllowCreate attribute MUST NOT be used and SHOULD be ignored in conjunction with requests for or assertions issued with name identifiers with a Format of urn:oasis:names:tc:SAML:2.0:nameid-format:transient (they preclude any such state in and of themselves)." --- src/saml2/client_base.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/saml2/client_base.py b/src/saml2/client_base.py index 39a7d0ed..15e3b0ec 100644 --- a/src/saml2/client_base.py +++ b/src/saml2/client_base.py @@ -339,6 +339,10 @@ class Base(Entity): # If no nameid_format has been set in the configuration # or passed in then transient is the default. if nameid_format is None: + # SAML 2.0 errata says AllowCreate MUST NOT be used for + # transient ids - to make a conservative change this is + # only applied for the default cause + allow_create = None nameid_format = NAMEID_FORMAT_TRANSIENT # If a list has been configured or passed in choose the -- cgit v1.2.1 From 6f65014dfb7c4ba4cb9edd6e7f0c16889e2d0e60 Mon Sep 17 00:00:00 2001 From: Fredrik Thulin Date: Wed, 8 May 2019 16:36:30 +0200 Subject: implement match_local_id Implement MongoDB version of function to look for an existing persistent NameId for a user. --- src/saml2/mongo_store.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/saml2/mongo_store.py b/src/saml2/mongo_store.py index 6a8f9f45..6bbaaf52 100644 --- a/src/saml2/mongo_store.py +++ b/src/saml2/mongo_store.py @@ -5,6 +5,8 @@ from pymongo import MongoClient from pymongo.mongo_replica_set_client import MongoReplicaSetClient import pymongo.uri_parser import pymongo.errors +from saml2.saml import NAMEID_FORMAT_PERSISTENT + from saml2.eptid import Eptid from saml2.mdstore import InMemoryMetaData from saml2.mdstore import metadata_modules @@ -163,6 +165,20 @@ class IdentMDB(IdentDB): return item[self.mdb.primary_key] return None + def match_local_id(self, userid, sp_name_qualifier, name_qualifier): + """ + Look for an existing persistent NameID matching userid, + sp_name_qualifier and name_qualifier. + """ + filter = {"name_id.sp_name_qualifier": sp_name_qualifier, + "name_id.name_qualifier": name_qualifier, + "name_id.format": NAMEID_FORMAT_PERSISTENT, + } + res = self.mdb.get(value=userid, **filter) + if not res: + return None + return from_dict(res[0]["name_id"], ONTS, True) + def remove_remote(self, name_id): cnid = to_dict(name_id, MMODS, True) self.mdb.remove(name_id=cnid) -- cgit v1.2.1 From 161a5cbd4bc15a8d6481a95d4271ddfb214233f5 Mon Sep 17 00:00:00 2001 From: Fredrik Thulin Date: Wed, 8 May 2019 16:37:43 +0200 Subject: Look for existing persistent id's before creating new ones. --- src/saml2/ident.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/saml2/ident.py b/src/saml2/ident.py index db8365bc..f24c0390 100644 --- a/src/saml2/ident.py +++ b/src/saml2/ident.py @@ -155,6 +155,14 @@ class IdentDB(object): pass def get_nameid(self, userid, nformat, sp_name_qualifier, name_qualifier): + if nformat == NAMEID_FORMAT_PERSISTENT: + nameid = self.match_local_id(userid, sp_name_qualifier, + name_qualifier) + if nameid: + logger.debug("Found existing persistent NameId %s " + "for user %s" % (nameid, userid)) + return nameid + _id = self.create_id(nformat, name_qualifier, sp_name_qualifier) if nformat == NAMEID_FORMAT_EMAILADDRESS: @@ -163,9 +171,6 @@ class IdentDB(object): _id = "%s@%s" % (_id, self.domain) - # if nformat == NAMEID_FORMAT_PERSISTENT: - # _id = userid - nameid = NameID(format=nformat, sp_name_qualifier=sp_name_qualifier, name_qualifier=name_qualifier, text=_id) -- cgit v1.2.1 From 41806ddd7612af238d8637ee51959e58a90d8cc6 Mon Sep 17 00:00:00 2001 From: Fredrik Thulin Date: Wed, 8 May 2019 16:38:40 +0200 Subject: improve comment --- src/saml2/ident.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/saml2/ident.py b/src/saml2/ident.py index f24c0390..95835239 100644 --- a/src/saml2/ident.py +++ b/src/saml2/ident.py @@ -241,7 +241,7 @@ class IdentDB(object): def construct_nameid(self, userid, local_policy=None, sp_name_qualifier=None, name_id_policy=None, name_qualifier=""): - """ Returns a name_id for the object. How the name_id is + """ Returns a name_id for the userid. How the name_id is constructed depends on the context. :param local_policy: The policy the server is configured to follow -- cgit v1.2.1 From aa5c4207a828b94cfd90b6ecc78566632a324852 Mon Sep 17 00:00:00 2001 From: Fredrik Thulin Date: Wed, 8 May 2019 18:39:43 +0200 Subject: update tests with regards to AllowCreate AllowCreate is not supposed to be present for transient Name IDs. --- tests/test_50_server.py | 4 ++-- tests/test_51_client.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_50_server.py b/tests/test_50_server.py index dc6cbf42..ecef319e 100644 --- a/tests/test_50_server.py +++ b/tests/test_50_server.py @@ -267,7 +267,7 @@ class TestServer1(): assert resp_args["destination"] == "http://lingon.catalogix.se:8087/" assert resp_args["in_response_to"] == "id1" name_id_policy = resp_args["name_id_policy"] - assert _eq(name_id_policy.keyswv(), ["format", "allow_create"]) + assert _eq(name_id_policy.keyswv(), ["format"]) assert name_id_policy.format == saml.NAMEID_FORMAT_TRANSIENT assert resp_args[ "sp_entity_id"] == "urn:mace:example.com:saml:roland:sp" @@ -1341,7 +1341,7 @@ class TestServer1NonAsciiAva(): assert resp_args["destination"] == "http://lingon.catalogix.se:8087/" assert resp_args["in_response_to"] == "id1" name_id_policy = resp_args["name_id_policy"] - assert _eq(name_id_policy.keyswv(), ["format", "allow_create"]) + assert _eq(name_id_policy.keyswv(), ["format"]) assert name_id_policy.format == saml.NAMEID_FORMAT_TRANSIENT assert resp_args[ "sp_entity_id"] == "urn:mace:example.com:saml:roland:sp" diff --git a/tests/test_51_client.py b/tests/test_51_client.py index 2e2b7f1c..75dd8f75 100644 --- a/tests/test_51_client.py +++ b/tests/test_51_client.py @@ -269,7 +269,7 @@ class TestClient: assert ar.provider_name == "urn:mace:example.com:saml:roland:sp" assert ar.issuer.text == "urn:mace:example.com:saml:roland:sp" nid_policy = ar.name_id_policy - assert nid_policy.allow_create == "false" + assert nid_policy.allow_create is None assert nid_policy.format == saml.NAMEID_FORMAT_TRANSIENT node_requested_attributes = None @@ -1757,7 +1757,7 @@ class TestClientNonAsciiAva: assert ar.provider_name == "urn:mace:example.com:saml:roland:sp" assert ar.issuer.text == "urn:mace:example.com:saml:roland:sp" nid_policy = ar.name_id_policy - assert nid_policy.allow_create == "false" + assert nid_policy.allow_create is None assert nid_policy.format == saml.NAMEID_FORMAT_TRANSIENT node_requested_attributes = None -- cgit v1.2.1 From 70084f518537e8b82278de1b4aa1988d79498a09 Mon Sep 17 00:00:00 2001 From: Ivan Kanakarakis Date: Tue, 14 May 2019 12:25:13 +0200 Subject: Format code Signed-off-by: Ivan Kanakarakis --- src/saml2/ident.py | 18 ++++++++++++------ src/saml2/mongo_store.py | 11 +++++++---- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/saml2/ident.py b/src/saml2/ident.py index 95835239..d6a6620a 100644 --- a/src/saml2/ident.py +++ b/src/saml2/ident.py @@ -156,11 +156,13 @@ class IdentDB(object): def get_nameid(self, userid, nformat, sp_name_qualifier, name_qualifier): if nformat == NAMEID_FORMAT_PERSISTENT: - nameid = self.match_local_id(userid, sp_name_qualifier, - name_qualifier) + nameid = self.match_local_id(userid, sp_name_qualifier, name_qualifier) if nameid: - logger.debug("Found existing persistent NameId %s " - "for user %s" % (nameid, userid)) + logger.debug( + "Found existing persistent NameId {nid} for user {uid}".format( + nid=nameid, uid=userid + ) + ) return nameid _id = self.create_id(nformat, name_qualifier, sp_name_qualifier) @@ -171,8 +173,12 @@ class IdentDB(object): _id = "%s@%s" % (_id, self.domain) - nameid = NameID(format=nformat, sp_name_qualifier=sp_name_qualifier, - name_qualifier=name_qualifier, text=_id) + nameid = NameID( + format=nformat, + sp_name_qualifier=sp_name_qualifier, + name_qualifier=name_qualifier, + text=_id, + ) self.store(userid, nameid) return nameid diff --git a/src/saml2/mongo_store.py b/src/saml2/mongo_store.py index 6bbaaf52..4120e9e0 100644 --- a/src/saml2/mongo_store.py +++ b/src/saml2/mongo_store.py @@ -167,13 +167,16 @@ class IdentMDB(IdentDB): def match_local_id(self, userid, sp_name_qualifier, name_qualifier): """ + Match a local persistent identifier. + Look for an existing persistent NameID matching userid, sp_name_qualifier and name_qualifier. """ - filter = {"name_id.sp_name_qualifier": sp_name_qualifier, - "name_id.name_qualifier": name_qualifier, - "name_id.format": NAMEID_FORMAT_PERSISTENT, - } + filter = { + "name_id.sp_name_qualifier": sp_name_qualifier, + "name_id.name_qualifier": name_qualifier, + "name_id.format": NAMEID_FORMAT_PERSISTENT, + } res = self.mdb.get(value=userid, **filter) if not res: return None -- cgit v1.2.1