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 Federated Signin URL Generator #6365

Closed
wants to merge 1 commit into from

Conversation

ckabalan
Copy link
Contributor

@ckabalan ckabalan commented Aug 27, 2021

Adds a signin "service" in similar fashion to the CLI's configure "service":

  • Using the AWS federation endpoint this command takes temporary
    credentials and returns a sign-in URL allowing a user to log in to the
    AWS Management Console using those temporary credentials.

  • Follows the published example code for 'Enabling custom identity
    broker access to the AWS console' in the AWS IAM User Guide.

  • The name signin, while generic and potentially confusing to new
    users, follows as close as possible to the global service naming convention
    similar to iam.amazonaws.com (signin.aws.amazon.com).

  • Resolves Login into AWS console from cli #4642 (feature request)

Description of changes:

  • Includes passing 100% test coverage (unit and functional)
  • Includes informative documentation and examples
  • Meets pep8 standards (via flake8) except one reference to a long documentation URL in a comment
  • Follows as many patterns and best practices I could find elsewhere in the project

I contributed some updated examples/documentation to the aws-cli previously, but not new functionality. I'll monitor this PR for any questions or adjustments that need to be made.

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

@codecov-commenter
Copy link

codecov-commenter commented Aug 27, 2021

Codecov Report

Merging #6365 (e6596f1) into develop (3803a80) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head e6596f1 differs from pull request most recent head 587fed5. Consider uploading reports for the commit 587fed5 to get more accurate results

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #6365   +/-   ##
========================================
  Coverage    92.85%   92.85%           
========================================
  Files          204      207    +3     
  Lines        16298    16368   +70     
========================================
+ Hits         15133    15199   +66     
- Misses        1165     1169    +4     
Impacted Files Coverage Δ
awscli/customizations/signin/__init__.py 100.00% <100.00%> (ø)
awscli/customizations/signin/exceptions.py 100.00% <100.00%> (ø)
awscli/customizations/signin/signin.py 100.00% <100.00%> (ø)
awscli/handlers.py 100.00% <100.00%> (ø)
awscli/argparser.py 92.55% <0.00%> (-1.07%) ⬇️
.../customizations/datapipeline/createdefaultroles.py 84.72% <0.00%> (-1.00%) ⬇️
awscli/customizations/codeartifact/login.py 97.48% <0.00%> (-0.36%) ⬇️
awscli/testutils.py 65.17% <0.00%> (-0.24%) ⬇️
awscli/help.py 96.98% <0.00%> (-0.02%) ⬇️
awscli/paramfile.py 100.00% <0.00%> (ø)
... and 5 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 3803a80...587fed5. Read the comment docs.

@kdaily kdaily added the needs-review This issue or pull request needs review from a core team member. label Nov 6, 2021
@aantn
Copy link

aantn commented Dec 28, 2021

This would be really useful. I wrote a tool to do this but honestly it should just be part of the AWS CLI and someone already did the hard work for a PR. Why not merge this?

@ckabalan ckabalan force-pushed the signin-url-generator branch 2 times, most recently from eeaf718 to 16a2eec Compare February 15, 2022 18:41
@ckabalan
Copy link
Contributor Author

I've rebased this PR and made some adjustments to account for codebase/convention changes since the original PR was submitted.

Adds a signin "service" in similar fashion to the CLI's configure "service":

* Using the AWS federation endpoint this command takes temporary
  credentials and returns a sign-in URL allowing a user to log in to the
  AWS Management Console using those temporary credentials.

* Follows the published example code for 'Enabling custom identity
  broker access to the AWS console' in the AWS IAM User Guide.

* The name `signin`, while generic and potentially confusing to new
  users, follows as close as possible to the global service naming convention
  similar to `iam.amazonaws.com` (`signin.aws.amazon.com`).

* Resolves aws#4642 (feature request)
@ckabalan
Copy link
Contributor Author

I submitted this as an AWS customer, but now I'm internal to AWS as a Solutions Architect, you can find me internally ckabalan@.

@Diom
Copy link

Diom commented Jun 3, 2022

@ckabalan Great work on the PR. Any idea if you will get it merged soon?

@bgshacklett
Copy link

Any updates here? It would be fantastic to have this as a native part of the CLI.

@maspotts
Copy link

maspotts commented Mar 7, 2024

Hi: was this ultimately abandoned? It would be super useful!

@tim-finnigan
Copy link
Contributor

Hi all, thanks for your patience here. I brought this PR up for discussion with the team, and the consensus was that these changes are not under consideration at this time. This feature request would require a broad internal security review and this is not something that the team has bandwidth to prioritize right now. But we can continue tracking the corresponding feature request (#4642) going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review This issue or pull request needs review from a core team member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Login into AWS console from cli
8 participants