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

fix(credential-provider-ini): support sso-session based profile as source_profile #4820

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

sjakthol
Copy link
Contributor

@sjakthol sjakthol commented Jun 10, 2023

Issue

Fixes #4757.

Description

Previously, when credential-provider-ini resolved credentials for a source profile, it loaded the profile properties, checked if the profile was an SSO profile, validated that all required sso_* properties were present and then resolved credentials with credential-provider-sso module.

When source profile was an SSO profile that did not use an SSO session, this logic worked fine as the profile must include all the sso_* properties for SSO to succeed. However, when source profile was an SSO profile that used an SSO session, credential-provider-ini only resolved profile properties directly from the profile but not from the related SSO session.

This caused sso-session based profiles to fail validation as some of the required sso_* properties are only defined in the referenced sso-session section. And hence the provider failed to resolve credentials for SSO session based source_profiles.

This commit changes credential-provider-ini module to not resolve or validate SSO profile properties but delegate that all to the credential-provider-sso module that already contains all the logic needed to load and resolve profile properties and AWS credentials for both sso-session and non-sso-session based profiles.

Instead of passing profile properties to credential-provider-sso fromSSO() method, new version only passes the profile name there and lets the credential-provider-sso module load and resolve the SSO profile with the logic that already exist in that module.

Testing

Unit tests have been adjusted to function with the updated behavior.

Following scenarios were also tested manually:

  • Resolve credentials for SSO profile without sso_session option
  • Resolve credentials for SSO profile with sso_session option
  • Resolve credentials for an assumed role profile whose source profile is an SSO profile without sso_session option
  • Resolve credentials for an assumed role profile whose source profile is an SSO profile with sso_session option

AWS SDK resolved credentials correctly in all four cases.

Additional context

N/A


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@sjakthol sjakthol requested a review from a team as a code owner June 10, 2023 14:08
@gastonsilva
Copy link

+1
What is needed to move this forward?

@pbredenberg
Copy link

This is an issue preventing some tooling we're trying to build for our internal processes from moving forward. It would be great to get this merged.

…urce_profile

Previously, when credential-provider-ini resolved credentials for a
source profile, it loaded the profile properties, checked if the profile
was an SSO profile, validated that all required sso_* properties were
present and then resolved credentials with credential-provider-sso
module.

When source profile was an SSO profile that did not use an SSO session,
this logic worked fine as the profile must include all the sso_*
properties for SSO to succeed. However, when source profile was an SSO
profile that used an SSO session, credential-provider-ini only resolved
profile properties directly from the profile but not from the related
SSO session.

This caused sso-session based profiles to fail validation as some of the
required sso_* properties are only defined in the referenced sso-session
section. And hence the provider failed to resolve credentials for SSO
session based source_profiles.

This commit changes credential-provider-ini module to not resolve or
validate SSO profile properties but delegate that all to the
credential-provider-sso module that already contains all the logic
needed to load and resolve profile properties and AWS credentials for
both sso-session and non-sso-session based profiles.

Instead of passing profile properties to credential-provider-sso
fromSSO() method, new version only passes the profile name there and
lets the credential-provider-sso module load and resolve the SSO
profile with the logic that already exist in that module.

Unit tests have been adjusted to function with the updated behavior.

Following scenarios were also tested manually:

* Resolve credentials for SSO profile without sso_session option
* Resolve credentials for SSO profile with sso_session option
* Resolve credentials for an assumed role profile whose source profile
  is an SSO profile without sso_session option
* Resolve credentials for an assumed role profile whose source profile
  is an SSO profile with sso_session option

AWS SDK resolved credentials correctly in all four cases.

Fixes aws#4757.
@sjakthol sjakthol force-pushed the source-profile-sso-session branch from f674cc1 to b3fd438 Compare January 29, 2024 20:26
@sjakthol
Copy link
Contributor Author

sjakthol commented Jan 29, 2024

I've rebased this on top of changes from #5681 that were merged in earlier today. I've stashed the original changes to sjakthol@a53d2ba in case they are still needed for some reason.

@siddsriv siddsriv merged commit b87f5e0 into aws:main Jan 30, 2024
3 checks passed
@sjakthol sjakthol deleted the source-profile-sso-session branch January 30, 2024 16:22
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2024
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.

Cannot use AWS SSO with assume role when source_profile uses the sso_session option
5 participants