Skip to content

Treat requested subject id as requested attribute #888

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

Conversation

johanlundberg
Copy link
Contributor

Description

If an SP requests a subject id using SAML V2.0 Subject Identifier Attributes Profile Version we should not require the SP to also add the request to requested attributes.

The feature or problem addressed by this PR

When using entity categories that specifies attributes that should only be released when required an SP needs to request subject id in both entity attributes and in requested attributes. With this PR the SP only needs to request the subject id in entity attributes as defined by the committee specification.

What your changes do and why you chose this solution

I found that this way play nice with entity category filtering.

Checklist

  • Checked that no other issues or pull requests exist for the same issue/change
  • Added tests covering the new functionality
  • Updated documentation OR the change is too minor to be documented
  • Updated CHANGELOG.md OR changes are insignificant


if "urn:oasis:names:tc:SAML:profiles:subject-id:req" in entity_attributes:
subject_id_req = entity_attributes["urn:oasis:names:tc:SAML:profiles:subject-id:req"][0]
if subject_id_req == "any" or subject_id_req == "pairwise-id":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that "any" results to a pairwise identifier. I don't mind this, but it is also good to mention that this choice was made here. This should be mentioned in the changelog.

@c00kiemon5ter c00kiemon5ter merged commit 63b2faa into IdentityPython:master Dec 11, 2022
@c00kiemon5ter
Copy link
Member

Updated by a9fe345

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants