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

Add support for SSO credentials provider #2118

Merged
merged 1 commit into from Nov 20, 2020

Conversation

Quanzzzz
Copy link
Contributor

@Quanzzzz Quanzzzz commented Oct 26, 2020

Add support for SSO Credentials Provider.

Description

In this pr, a credential provider that exchanges a resolved SSO login token for temporary AWS credentials is added. Besides, several new classes and tests are added to enable this credentials provider.

Main Updates since the first review:

  1. Merged the SsoCredentialsProvider into part of the ProfileCredentialsProvider so the profile addressing part can be better taken care of;
  2. Separate the generation of cached token path and resolution of access token to make it more flexible;
  3. Updated all the tests to make them match the current code and achieve better coverage;

Besides the main changes for SSO, there are also some small changes to the code:

  1. Corrected the javadocs of annotation SdkInternalApi to make it more accurate;
  2. Moved the userHomeDirectorymethod out of ProfileFileLocation and put it in the utils module, thus it can be used by the other modules;

Motivation and Context

Currently, SSO provides customers an interface where customers can copy-paste snippests to set the environment variables to temporary credentials that they generate.
While the interface is good for console access and fine for occasional API use, it is painful for repeated API use as customers have to go back to the SSO page every hour to get new credentials. Allowing customers to use their existing login username and password to fetch temporart AWS credentials without having to go to an external site is a much more seamless experience.

Testing

Since this credentials provider include interaction with SSO service and it relies on another SSO login flow, so there isn’t integration tests added here. However, to make sure the functionality is sound, multiple unit tests and functional tests are added.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed
  • A short description of the change has been added to the CHANGELOG
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

@sonarcloud
Copy link

sonarcloud bot commented Nov 5, 2020

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 1 Security Hotspot to review)
Code Smell A 11 Code Smells

15.6% 15.6% Coverage
0.0% 0.0% Duplication

@Quanzzzz Quanzzzz requested a review from millems November 5, 2020 01:29
@Quanzzzz Quanzzzz force-pushed the sso-credentials-dev branch 2 times, most recently from 7734c58 to 7fa7dba Compare November 18, 2020 23:41
@codecov-io
Copy link

codecov-io commented Nov 19, 2020

Codecov Report

Merging #2118 (a72aade) into master (040e854) will decrease coverage by 0.00%.
The diff coverage is 77.48%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2118      +/-   ##
============================================
- Coverage     77.44%   77.43%   -0.01%     
- Complexity      299      326      +27     
============================================
  Files          1214     1221       +7     
  Lines         38161    38292     +131     
  Branches       3014     3018       +4     
============================================
+ Hits          29552    29650      +98     
- Misses         7157     7187      +30     
- Partials       1452     1455       +3     
Flag Coverage Δ Complexity Δ
unittests 77.43% <77.48%> (-0.01%) 0.00 <27.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
.../credentials/internal/ProfileCredentialsUtils.java 78.72% <14.28%> (-10.57%) 0.00 <0.00> (ø)
...wssdk/services/sso/auth/ExpiredTokenException.java 53.84% <53.84%> (ø) 3.00 <3.00> (?)
...sso/auth/SsoProfileCredentialsProviderFactory.java 75.00% <75.00%> (ø) 3.00 <3.00> (?)
...wssdk/services/sso/internal/SsoTokenFileUtils.java 80.00% <80.00%> (ø) 4.00 <4.00> (?)
...ssdk/services/sso/auth/SsoCredentialsProvider.java 89.13% <89.13%> (ø) 9.00 <9.00> (?)
...re/amazon/awssdk/utils/UserHomeDirectoryUtils.java 93.33% <93.33%> (ø) 0.00 <0.00> (?)
...re/amazon/awssdk/profiles/ProfileFileLocation.java 87.50% <100.00%> (-2.83%) 0.00 <0.00> (ø)
...ervices/sso/internal/SessionCredentialsHolder.java 100.00% <100.00%> (ø) 3.00 <3.00> (?)
.../services/sso/internal/SsoAccessTokenProvider.java 100.00% <100.00%> (ø) 5.00 <5.00> (?)
...nio/netty/internal/OldConnectionReaperHandler.java 81.81% <0.00%> (-9.10%) 0.00% <0.00%> (ø%)
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 040e854...a72aade. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants