-
Notifications
You must be signed in to change notification settings - Fork 92
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
Additional Named Roles are Not Populating #26
Comments
Hi @bgshacklett Its not the SAML response that is used to fetch credentials for those additional roles. How it works is as follows:
This feature comes from an user who mentioned they have an AWS environment where they login to one master IAM account, and hop between accounts/role from there by assuming other roles. This is actually a security best practice from AWS, so I see this setup more often. So as you can see, you'll only see the additional profiles in your credentials file if the default profile has the permissions to assume the other roles. |
I found that as I was looking at the code the end of last week. Sadly I didn't ever push the branch I was on and I lost the work I did experimenting with enabling it to assume all of the roles as named profiles. The way I've done this in my Node.js module is to use the assumeRoleWithSAML function for all available roles in the SAML response from the IDP: https://github.com/bgshacklett/samlogin/blob/master/index.js#L93 I'm hoping to put a PR in for this early next week. In the interim, I'd be interested in any feedback you have on that method being utilized in your extension. |
Well, first of all, I definitely welcome contributions to the code, so lets see how we can make this work. But on first glance I'm thinking I might need to refactor the code to make it easier to integrate new features. The majority of the extension was written before I was even aware of Javascript Promises for example. I guess if I would start from scratch today I would do things differently. Maybe somewhere during the xmas holidays I'll do that. I can tell you from a conceptual point of view how I'd like to see it integrated.
In the current state I guess it would mean to intervene somewhere around this point: samltoawsstskeys/background/script.js Line 171 in 20133da
Yes, it will be a bit messy and contain duplicate code, but until some refactoring is done its probably the best way to integrate. |
I think this is still unsolved, isn't it? Also it would be nice to be able to asign a name to the default profile, instead of "default" |
I have forked and created a new version of the extension with some of the features we are discussing here. https://github.com/penchala-services-inc/chrome-samltoawsstskeys-mp/tree/master |
This may help you.I have forked and created a new version of the extension with some of the features you are discussing. https://github.com/penchala-services-inc/chrome-samltoawsstskeys-mp/tree/master |
I have forked and created a new version of the extension with what you are looking for here. https://chrome.google.com/webstore/detail/saml-to-aws-sts-keys-conv/nhcoglopkiacbogcjdcbhooedkaddimi https://github.com/penchala-services-inc/chrome-samltoawsstskeys-mp/tree/master |
Summary:
Testing with some of my coworkers has shown that at least some additional named roles are not populating in the downloaded Credentials file for us.
Steps to Reproduce:
Expected behavior:
Actual behavior:
Other notes:
Looking at locations in the source code where changes occurred, I suspect this might have started happening around 35e4917.
The text was updated successfully, but these errors were encountered: