Fix broken TTY after manual auth code prompt. #758
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #758 +/- ##
==========================================
+ Coverage 79.86% 79.89% +0.02%
==========================================
Files 124 124
Lines 8357 8369 +12
==========================================
+ Hits 6674 6686 +12
Misses 1464 1464
Partials 219 219
Continue to review full report at Codecov.
|
cfryanr
reviewed
Jul 29, 2021
cfryanr
reviewed
Jul 29, 2021
cfryanr
reviewed
Jul 29, 2021
mattmoyer
force-pushed
the
use-plain-authcode-prompt
branch
2 times, most recently
from
46706eb
to
3899cda
Compare
This may be a temporary fix. It switches the manual auth code prompt to use `promptForValue()` instead of `promptForSecret()`. The `promptForSecret()` function no longer supports cancellation (the v0.9.2 behavior) and the method of cancelling in `promptForValue()` is now based on running the blocking read in a background goroutine, which is allowed to block forever or leak (which is not important for our CLI use case). This means that the authorization code is now visible in the user's terminal, but this is really not a big deal because of PKCE and the limited lifetime of an auth code. The main goroutine now correctly waits for the "manual prompt" goroutine to clean up, which now includes printing the extra newline that would normally have been entered by the user in the manual flow. The text of the manual login prompt is updated to be more concise and less scary (don't use the word "fail"). Signed-off-by: Matt Moyer <moyerm@vmware.com>
mattmoyer
force-pushed
the
use-plain-authcode-prompt
branch
from
3899cda
to
1e32530
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This may be a temporary fix. It switches the manual auth code prompt to use
promptForValue()instead ofpromptForSecret(). ThepromptForSecret()function no longer supports cancellation (the v0.9.2 behavior) and the method of cancelling inpromptForValue()is now based on running the blocking read in a background goroutine, which is allowed to block forever or leak (which is not important for our CLI use case).This means that the authorization code is now visible in the user's terminal, but this is really not a big deal because of PKCE and the limited lifetime of an auth code.
The main goroutine now correctly waits for the "manual prompt" goroutine to clean up, which now includes printing the extra newline that would normally have been entered by the user in the manual flow.
Release note:
This functionality with the bug has not yet been released.