Skip to content

do not assert both subject-id and pairwise-id #987

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/saml2/assertion.py
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,38 @@ def not_on_or_after(self, sp_entity_id):

return in_a_while(**self.get_lifetime(sp_entity_id))

@staticmethod
def _subject_id_or_pairwise_id(
ava,
required,
):
"""
If both subject-id and pairwise-id are present in required attributes, check if both are present in
available attributes and if so default to pairwise-id only.
"""
if required is None:
return ava, None

# check if both subject-id and pairwise-id are in required attributes
subject_id = None
pairwise_id = None
for item in required:
if item["name"] == "urn:oasis:names:tc:SAML:attribute:subject-id":
subject_id = item
if item["name"] == "urn:oasis:names:tc:SAML:attribute:pairwise-id":
pairwise_id = item

# if both are in required attributes, check if both are in available attributes and if so remove subject-id
# from required attributes and ava
if subject_id and pairwise_id:
if all(
[friendly_name in ava for friendly_name in [subject_id["friendly_name"], pairwise_id["friendly_name"]]]
):
required.pop(required.index(subject_id))
ava.pop(subject_id["friendly_name"])

return ava, required

def filter(self, ava, sp_entity_id, mdstore=None, required=None, optional=None):
"""What attribute and attribute values returns depends on what
the SP or the registration authority has said it wants in the request
Expand Down Expand Up @@ -518,6 +550,10 @@ def filter(self, ava, sp_entity_id, mdstore=None, required=None, optional=None):

subject_ava = ava.copy()

# make sure we don't assert both subject-id and pairwise-id if subject-id requirement is "any"
if self.metadata_store and self.metadata_store.subject_id_requirement_type(sp_entity_id) == "any":
subject_ava, required = self._subject_id_or_pairwise_id(subject_ava, required)

# entity category restrictions
_ent_rest = self.get_entity_categories(sp_entity_id, mds=mdstore, required=required)
if _ent_rest:
Expand Down
9 changes: 6 additions & 3 deletions src/saml2/mdstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -1304,14 +1304,17 @@ def attribute_requirement(self, entity_id, index=None):
if entity_id in md_source:
return md_source.attribute_requirement(entity_id, index)

def subject_id_requirement(self, entity_id):
def subject_id_requirement_type(self, entity_id):
try:
entity_attributes = self.entity_attributes(entity_id)
except KeyError:
return []
return ""

subject_id_reqs = entity_attributes.get("urn:oasis:names:tc:SAML:profiles:subject-id:req") or []
subject_id_req = next(iter(subject_id_reqs), None)
return next(iter(subject_id_reqs), None)

def subject_id_requirement(self, entity_id):
subject_id_req = self.subject_id_requirement_type(entity_id)
if subject_id_req == "any":
return [
{
Expand Down
36 changes: 36 additions & 0 deletions tests/test_37_entity_categories.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,3 +400,39 @@ def test_filter_ava_refeds_personalized_access():
assert _eq(ava["eduPersonScopedAffiliation"], ["[email protected]"])
assert _eq(ava["eduPersonAssurance"], ["http://www.swamid.se/policy/assurance/al1"])
assert _eq(ava["schacHomeOrganization"], ["example.com"])


def test_filter_subject_id_or_pairwise_id():
entity_id = "https://esi-coco.example.edu/saml2/metadata/"
mds = MetadataStore(ATTRCONV, sec_config, disable_ssl_certificate_validation=True)
mds.imp([{"class": "saml2.mdstore.MetaDataFile", "metadata": [(full_path("entity_esi_and_coco_sp.xml"),)]}])

policy_conf = {"default": {"lifetime": {"minutes": 15}, "entity_categories": ["swamid"]}}

policy = Policy(policy_conf, mds)

ava = {
"subject-id": ["subject-id"],
"pairwise-id": ["pairwise-id"],
}

required_attributes = [
{
"__class__": "urn:oasis:names:tc:SAML:2.0:metadata&RequestedAttribute",
"name": "urn:oasis:names:tc:SAML:attribute:pairwise-id",
"name_format": "urn:oasis:names:tc:SAML:2.0:attrname-format:uri",
"friendly_name": "pairwise-id",
"is_required": "true",
},
{
"__class__": "urn:oasis:names:tc:SAML:2.0:metadata&RequestedAttribute",
"name": "urn:oasis:names:tc:SAML:attribute:subject-id",
"name_format": "urn:oasis:names:tc:SAML:2.0:attrname-format:uri",
"friendly_name": "subject-id",
"is_required": "true",
},
]

ava = policy.filter(ava, entity_id, required=required_attributes)

assert _eq(list(ava.keys()), ["pairwise-id"])