Skip to content
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

Use SAML's subject if no username is set on ATTRIBUTES_MAP #80

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pappacena
Copy link

I'm not sure how other SAML SP works, but Okta does't have a default UserName attribute, but you are able to get the username from the subject.

With this change, if the username attribute is not explicitly set on ATTRIBUTES_MAP, it is possible to use the subject text as username.

@pappacena pappacena changed the title Use SAML username if no username is set on ATTRIBUTES_MAP Use SAML's subject if no username is set on ATTRIBUTES_MAP Jan 24, 2019
@fangli
Copy link
Owner

fangli commented Mar 8, 2019

Good catch! but I'm afraid this is not the nature way we should use to get username, since SAML itself provides attributes for authentication.
For your usecase, you may just want to add the mapping in okta for SAML attributes, that's what anyone else did for integration.

@pappacena
Copy link
Author

@fangli yes, I imagine that everyone else would just map the Okta attributes. That's what I would do too.

Unfortunately, the security department from my company didn't allow to do this mapping (don't ask me why...). That's why I needed to do this fallback.

@jessesimpson36
Copy link

I agree with pappacena, service providers don't necessarily have access to change how the data sent from the identity provider's point of view.

Just because Okta uses attributes for the username, doesn't mean SAML2 spec's intended that usage. I read through the documentation and on page 18, it talks about how it's a common usage to put the NameId in the Subject body (although optional). It is also optional to have a username be a part of the AttributeStatements.

http://www.oasis-open.org/committees/download.php/35711/sstc-saml-core-errata-2.0-wd-06-diff.pdf

I would like you to reconsider merging this PR, because I'm in the same situation as pappacena. I do not have access to change how the SAML is formatted, nor do I have the right to ask them to change it when they are following the SAML spec.

If you would like me to aid in developing this PR or testing this PR, please let me know because I am happy to help.

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.

None yet

3 participants