saml2aws
saml2aws copied to clipboard
Allow password to be saved to keychain with skip-prompt
Currently when you run the configure, the password is only saved to keychain if you don't specify the --skip-prompt
argument.
We use saml2aws as the last command in a setup script, where it doesn't make sense to prompt the user to re-enter the same data.
Fixes #485.
This would fix #485, correct?
Yes it would
@wolfeidau think we could get this merged and release? would be a big help to my team!
bump, I'm also waiting for this to resolve some issues within my org!
This would be a great help during onboarding 👍
The main concern i have with this code is the nested if / if else's it is pretty hard to read now 😞
@wolfeidau I've refactored the password prompting. Please take another look.
@wolfeidau can you evaluate this again? It's a nice feature I'd like to have.
Hi @wolfeidau, can you please take another look.
@wolfeidau kindly pinging :) we are also having issues with this, would be great to have this merged
CC: @gliptak
Codecov Report
Attention: 14 lines
in your changes are missing coverage. Please review.
Comparison is base (
d075691
) 39.28% compared to head (9748742
) 39.72%.
Files | Patch % | Lines |
---|---|---|
cmd/saml2aws/commands/configure.go | 30.00% | 13 Missing and 1 partial :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## master #547 +/- ##
==========================================
+ Coverage 39.28% 39.72% +0.43%
==========================================
Files 53 53
Lines 8025 8027 +2
==========================================
+ Hits 3153 3189 +36
+ Misses 4449 4411 -38
- Partials 423 427 +4
Flag | Coverage Δ | |
---|---|---|
unittests | 39.72% <30.00%> (+0.43%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Bump, my organisation also has an interest in seeing this merged.
@lmottaca-te That is good. Unfortunately, this will bump the coverage down, so I will need it to be accompanied with tests. If you are willing to add tests, I can work with you to merge it.
Since creating the PR my organisation has moved to AWS SSO, so unfortunately I don't have the time to add the tests. Feel free to fork my branch or let me know if you want to be added as a collaborator on my fork/repo.
@lmottaca-te Will you help implement tests for this? If not, I am inclined to close the PR
@mapkon I have added tests which has resulted in a positive coverage https://app.codecov.io/gh/Versent/saml2aws/pull/547 . Is this enough for the PR to proceed?
FYI, updated the test to utilise defer as I forgot it was a thing.