-
Notifications
You must be signed in to change notification settings - Fork 14
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
Load saml from responses #52
Conversation
@johananl and any chance of a review in this, also? |
I have to rebase because of 8f13fe8 (breaking change by OneLogin). |
cfd91b8
to
23717e7
Compare
@jspc - could you maybe share how you've configured OneLogin to return multiple IAM roles in the SAML assertion? I've been struggling with this for a while now. |
@johananl you need a different version of OneLogin for this, if I remember correctly... |
Have a look at https://onelogin.service-now.com/support/?id=kb_article&sys_id=66a91d03db109700d5505eea4b9619a5 from the linked issue |
23717e7
to
e57bd46
Compare
Wrong OneLogin config can lead to a role attribute with an empty value in the SAML assertion. Example: <saml:Attribute NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic" Name="https://aws.amazon.com/SAML/Attributes/Role"> <saml:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string"/> </saml:Attribute> We check for empty value before parsing ARNs to avoid index error.
e57bd46
to
0f1b7a0
Compare
Rebased over master. |
The following dependency is missing: github.com/edaniels/go-saml
Enumerate roles starting at 1 instead of 0. This is more human-friendly and consistent with MFA device selection behavior.
Thanks so much for this contribution @jspc. Superb work on this one. I took the liberty to refactor the role selection part - it would panic when selecting the last role in the list, plus I wanted to be consistent with enumerating from 1 instead of 0. I've also added handling for a SAML response which contains roles sections without a value (strange, but I've got this while testing using some OneLogin config). Lastly, I've added Thanks again! |
As per #49, SAML responses from AWS return a list of 1+ role/principal ARNs that can be assumed.
This PR will:
This looks like: