Skip to content

Commit a9fe345

Browse files
committed
Update behaviour of subject-id requirements entity attribute
When the subject-id requiment is "any", both the subject-id and pairwise-id should be processsed. Signed-off-by: Ivan Kanakarakis <[email protected]>
1 parent 936ce58 commit a9fe345

File tree

3 files changed

+71
-33
lines changed

3 files changed

+71
-33
lines changed

src/saml2/assertion.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -556,11 +556,12 @@ def restrict(self, ava, sp_entity_id, metadata=None):
556556

557557
metadata_store = metadata or self.metadata_store
558558
spec = metadata_store.attribute_requirement(sp_entity_id) or {} if metadata_store else {}
559-
required_attributes = spec.get("required", [])
560-
optional_attributes = spec.get("optional", [])
561-
required_subject_id = metadata_store.subject_id_requirement(sp_entity_id) if metadata_store else None
562-
if required_subject_id and required_subject_id not in required_attributes:
563-
required_attributes.append(required_subject_id)
559+
required_attributes = spec.get("required") or []
560+
optional_attributes = spec.get("optional") or []
561+
requirements_subject_id = metadata_store.subject_id_requirement(sp_entity_id) if metadata_store else []
562+
for r in requirements_subject_id:
563+
if r not in required_attributes:
564+
required_attributes.extend(r)
564565
return self.filter(
565566
ava,
566567
sp_entity_id,

src/saml2/mdstore.py

Lines changed: 47 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -200,14 +200,15 @@ def all_locations(srvs):
200200
return values
201201

202202

203-
def attribute_requirement(entity, index=None):
203+
def attribute_requirement(entity_descriptor, index=None):
204204
res = {"required": [], "optional": []}
205-
for acs in entity["attribute_consuming_service"]:
205+
acss = entity_descriptor.get("attribute_consuming_service") or []
206+
for acs in acss:
206207
if index is not None and acs["index"] != index:
207208
continue
208209

209210
for attr in acs["requested_attribute"]:
210-
if "is_required" in attr and attr["is_required"] == "true":
211+
if attr.get("is_required") == "true":
211212
res["required"].append(attr)
212213
else:
213214
res["optional"].append(attr)
@@ -676,24 +677,26 @@ def service(self, entity_id, typ, service, binding=None):
676677
return res
677678

678679
def attribute_requirement(self, entity_id, index=None):
679-
"""Returns what attributes the SP requires and which are optional
680+
"""
681+
Returns what attributes the SP requires and which are optional
680682
if any such demands are registered in the Metadata.
681683
684+
In case the metadata have multiple SPSSODescriptor elements,
685+
the sum of the required and optional attributes is returned.
686+
682687
:param entity_id: The entity id of the SP
683688
:param index: which of the attribute consumer services its all about
684689
if index=None then return all attributes expected by all
685690
attribute_consuming_services.
686-
:return: 2-tuple, list of required and list of optional attributes
691+
:return: dict of required and optional list of attributes
687692
"""
688693
res = {"required": [], "optional": []}
689694

690-
try:
691-
for sp in self[entity_id]["spsso_descriptor"]:
692-
_res = attribute_requirement(sp, index)
693-
res["required"].extend(_res["required"])
694-
res["optional"].extend(_res["optional"])
695-
except KeyError:
696-
return None
695+
sp_descriptors = self[entity_id].get("spsso_descriptor") or []
696+
for sp_desc in sp_descriptors:
697+
_res = attribute_requirement(sp_desc, index)
698+
res["required"].extend(_res.get("required") or [])
699+
res["optional"].extend(_res.get("optional") or [])
697700

698701
return res
699702

@@ -1297,35 +1300,56 @@ def discovery_response(self, entity_id, binding=None, _="spsso"):
12971300
)
12981301

12991302
def attribute_requirement(self, entity_id, index=None):
1300-
for _md in self.metadata.values():
1301-
if entity_id in _md:
1302-
return _md.attribute_requirement(entity_id, index)
1303+
for md_source in self.metadata.values():
1304+
if entity_id in md_source:
1305+
return md_source.attribute_requirement(entity_id, index)
13031306

13041307
def subject_id_requirement(self, entity_id):
13051308
try:
13061309
entity_attributes = self.entity_attributes(entity_id)
13071310
except KeyError:
1308-
return None
1311+
return []
13091312

1310-
if "urn:oasis:names:tc:SAML:profiles:subject-id:req" in entity_attributes:
1311-
subject_id_req = entity_attributes["urn:oasis:names:tc:SAML:profiles:subject-id:req"][0]
1312-
if subject_id_req == "any" or subject_id_req == "pairwise-id":
1313-
return {
1313+
subject_id_reqs = entity_attributes.get("urn:oasis:names:tc:SAML:profiles:subject-id:req") or []
1314+
subject_id_req = next(iter(subject_id_reqs), None)
1315+
if subject_id_req == "any":
1316+
return [
1317+
{
1318+
"__class__": "urn:oasis:names:tc:SAML:2.0:metadata&RequestedAttribute",
1319+
"name": "urn:oasis:names:tc:SAML:attribute:pairwise-id",
1320+
"name_format": "urn:oasis:names:tc:SAML:2.0:attrname-format:uri",
1321+
"friendly_name": "pairwise-id",
1322+
"is_required": "true",
1323+
},
1324+
{
1325+
"__class__": "urn:oasis:names:tc:SAML:2.0:metadata&RequestedAttribute",
1326+
"name": "urn:oasis:names:tc:SAML:attribute:subject-id",
1327+
"name_format": "urn:oasis:names:tc:SAML:2.0:attrname-format:uri",
1328+
"friendly_name": "subject-id",
1329+
"is_required": "true",
1330+
}
1331+
]
1332+
elif subject_id_req == "pairwise-id":
1333+
return [
1334+
{
13141335
"__class__": "urn:oasis:names:tc:SAML:2.0:metadata&RequestedAttribute",
13151336
"name": "urn:oasis:names:tc:SAML:attribute:pairwise-id",
13161337
"name_format": "urn:oasis:names:tc:SAML:2.0:attrname-format:uri",
13171338
"friendly_name": "pairwise-id",
13181339
"is_required": "true",
13191340
}
1320-
elif subject_id_req == "subject-id":
1321-
return {
1341+
]
1342+
elif subject_id_req == "subject-id":
1343+
return [
1344+
{
13221345
"__class__": "urn:oasis:names:tc:SAML:2.0:metadata&RequestedAttribute",
13231346
"name": "urn:oasis:names:tc:SAML:attribute:subject-id",
13241347
"name_format": "urn:oasis:names:tc:SAML:2.0:attrname-format:uri",
13251348
"friendly_name": "subject-id",
13261349
"is_required": "true",
13271350
}
1328-
return None
1351+
]
1352+
return []
13291353

13301354
def keys(self):
13311355
res = []

tests/test_30_mdstore.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -664,11 +664,24 @@ def test_subject_id_requirement():
664664
mds = MetadataStore(ATTRCONV, sec_config, disable_ssl_certificate_validation=True)
665665
mds.imp(METADATACONF["17"])
666666
required_subject_id = mds.subject_id_requirement(entity_id="https://esi-coco.example.edu/saml2/metadata/")
667-
assert required_subject_id["__class__"] == "urn:oasis:names:tc:SAML:2.0:metadata&RequestedAttribute"
668-
assert required_subject_id["name"] == "urn:oasis:names:tc:SAML:attribute:pairwise-id"
669-
assert required_subject_id["name_format"] == "urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
670-
assert required_subject_id["friendly_name"] == "pairwise-id"
671-
assert required_subject_id["is_required"] == "true"
667+
expected = [
668+
{
669+
"__class__": "urn:oasis:names:tc:SAML:2.0:metadata&RequestedAttribute",
670+
"name": "urn:oasis:names:tc:SAML:attribute:pairwise-id",
671+
"name_format": "urn:oasis:names:tc:SAML:2.0:attrname-format:uri",
672+
"friendly_name": "pairwise-id",
673+
"is_required": "true",
674+
},
675+
{
676+
"__class__": "urn:oasis:names:tc:SAML:2.0:metadata&RequestedAttribute",
677+
"name": "urn:oasis:names:tc:SAML:attribute:subject-id",
678+
"name_format": "urn:oasis:names:tc:SAML:2.0:attrname-format:uri",
679+
"friendly_name": "subject-id",
680+
"is_required": "true",
681+
},
682+
]
683+
assert required_subject_id
684+
assert all(e in expected for e in required_subject_id)
672685

673686

674687
def test_extension():

0 commit comments

Comments
 (0)