Skip to content
This repository has been archived by the owner on May 18, 2021. It is now read-only.

Allow assuming a source profile. #14

Merged
merged 3 commits into from
Nov 16, 2017
Merged

Allow assuming a source profile. #14

merged 3 commits into from
Nov 16, 2017

Conversation

ejcx
Copy link
Contributor

@ejcx ejcx commented Nov 15, 2017

If the source profile is itself, then don't do the second assume.
Instead, just jump in to the okta enabled role.

@ejcx ejcx changed the title Allow assuming a source profiles. Allow assuming a source profile. Nov 15, 2017
If the source profile is itself, then don't do the second assume.
Instead, just jump in to the okta enabled role.
@ejcx ejcx force-pushed the ej/support-source branch from bd03a77 to 4a026ce Compare November 15, 2017 19:57
@ejcx ejcx requested a review from dfuentes November 15, 2017 19:57
lib/provider.go Outdated
// If sourceProfile returns the same source then we do not need to assume a
// second role. Not assuming a second role allows us to assume IDP enabled
// roles directly.
if source != sourceProfile(p.profile, p.profiles) {
Copy link
Contributor

@dfuentes dfuentes Nov 15, 2017

Choose a reason for hiding this comment

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

I think this will always be false. Instead, you want if source != p.profile

p.profile is the chosen profile that is typed in.
It should coorespond to an aws config profile. In
the case that the profile's source profile is itself
then we have found an idp enabled role and we should
just log in to it instead of performing a second hop
@ejcx
Copy link
Contributor Author

ejcx commented Nov 16, 2017

@dfuentes nice catch. Source isn't actually what I wanted at all I don't think. When I switched the logic a different bug popped up

@ejcx
Copy link
Contributor Author

ejcx commented Nov 16, 2017

Oh. I misread your comment a few hours ago which held this up. I have p.profile != sourceProfile() but Ill do source = p.profile like you suggest, which I assume is a little more optimized, and the same result.

Copy link
Contributor

@dfuentes dfuentes left a comment

Choose a reason for hiding this comment

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

lgtm

@ejcx ejcx merged commit 86d56e5 into master Nov 16, 2017
@ejcx ejcx deleted the ej/support-source branch November 16, 2017 21:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants