Support other providers

[md5]

This PR contains some fixes for supporting providers other than Okta. I ran into the following issues while trying to implement support for our corporate IdP based on PingFederate:

1. Our integration uses RoleArn,PrincipalArn for the values of the https://aws.amazon.com/SAML/Attributes/Role attribute. This is actually what the AWS SAML docs recommend. It appears the existing code was originally written to support PrincipalArn,RoleArn, presumably because Okta uses that.
2. Our integration uses samlp and saml2 as its XML namespace prefixes, while the existing code was written to assume saml2p and saml2. My fix is to strip the prefixes in the xml2js processing, but it would probably be better to be more thorough about actually checking the namespace URIs and local names of each tag and attribute instead of making assumptions.

In addition to those two issues, which are have proposed fixes in this PR, I also thought that the handling of accounts and defaultAccount was odd. In our case, our SAML assertion in the case that a user has roles in more than one account will have all the account/role pairs as attribute values for https://aws.amazon.com/SAML/Attributes/Role. We do not have cross-account federation in place between the "default" account and other accounts, so it is not possible to use AssumeRole as this code does.

I would think that the right thing to do would actually be to present the user with a list of roles only for the chosen account when they're choosing a role. This would make sense in the case that --account is specified or when defaultAccount is set. When there is no default or user-specified account, I would think that the thing to is to show the user a list of accounts to choose from first. Either that or to present the list of roles grouped by account as is done at https://signin.aws.amazon.com/saml.

I consider this a work in progress, but I wanted to open the PR nonetheless to start some discussion.

Thanks for making such a great start on this!

[bromanko]

Thanks for this PR. These look like great additions. This is the first anyone has attempted to expand the code to another provider so I'm unsurprised that there were some deficiencies. I'm planning to take a closer look this weekend and back-test the changes in our environment. Would love to get this merged in.

[bromanko]

In our case, our SAML assertion in the case that a user has roles in more than one account will have all the account/role pairs as attribute values for https://aws.amazon.com/SAML/Attributes/Role. We do not have cross-account federation in place between the "default" account and other accounts, so it is not possible to use AssumeRole as this code does.

This is interesting. Can you provide an example of what this looks like in the SAML response? Is this something you were able to configure on the AWS side or is it done via PingFederate?

[md5]

This is interesting. Can you provide an example of what this looks like in the SAML response? Is this something you were able to configure on the AWS side or is it done via PingFederate?

I don't currently have roles in more than one account, but I believe it looks like this:

<saml:Attribute Name="https://aws.amazon.com/SAML/Attributes/Role"
    NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic">
    <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema"
        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:type="xs:string">arn:aws:iam::000000000000:role/role-1,arn:aws:iam::000000000000:saml-provider/idp.example.com</saml:AttributeValue>
    <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema"
        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:type="xs:string">arn:aws:iam::000000000000:role/role-2,arn:aws:iam::000000000000:saml-provider/idp.example.com</saml:AttributeValue>
    <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema"
        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:type="xs:string">arn:aws:iam::1111111111111:role/role-1,arn:aws:iam::1111111111111:saml-provider/idp.example.com</saml:AttributeValue>
    <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema"
        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:type="xs:string">arn:aws:iam::1111111111111:role/role-2,arn:aws:iam::1111111111111:saml-provider/idp.example.com</saml:AttributeValue>
</saml:Attribute>

I can confirm that with the relevant folks internally. I can also ask about how the configuration works, but I think this is standard functionality of PingFederate's Amazon Connector (cf. the docs). I'm pretty sure that if the AWS side sees more than one account in the assertion and the ProviderARNs are all valid for the accounts in question, then it gives the user a list of roles to choose from that is grouped by account.

[bromanko]

This is definitely interesting. We'll have to see if there's a way to get Okta to do this. It would simplify the approach.

This PR is safely merge-able without breaking existing functionality, and is an improvement. I suggest merging it in and then you can make additional improvements in a new one.

[md5]

Sounds good. I'll take some time tomorrow to make sure it's ready for merge, remove the “WIP” label, and ping you guys in a comment when I'm done.

[md5]

@bromanko I just pushed https://github.com/meetearnest/aws-sts/pull/4/commits/70678d90de1ac20eafe9cb5b26ce95a99e418909 to include the account id in parentheses after the role name if there is more than one account.

I still think a better UX would be to let the user choose the account first and then choose a role for that account, but that can be done separately as you said.

The code is unfortunately only able to show the account ID since the account aliases are not available until you have an access key. I think this could be improved by retrieving the account names from config.json.

[md5]

Thanks for the merge @bromanko! :+1: