-
Notifications
You must be signed in to change notification settings - Fork 569
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
feat(cli): prompt user for authentication on subsequent logins (#11004, #9329) #12883
Closed
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
elasticspoon
changed the title
feat(cli): prompt on subsequent logins as authenticated user
feat(cli): prompt authenticated user on subsequent logins
Apr 6, 2024
) Prompt the user if they want to re-authenticate if they run `coder login` when already authenticated. Does not apply if they provided a token via flag or ENV variable. A new token should be generated an stored in that situation unless the `--use-token-as-session` flag was also used. fix(cli): prompt user on login attempt with invalid ENV token Fixes issue where if invalid token was set as ENV variable a user would be unable to login until the ENV variable was cleared. User will now be informed that the token is invalid and prompted to login normally.
elasticspoon
changed the title
feat(cli): prompt authenticated user on subsequent logins
feat(cli): prompt authenticated user on subsequent logins (#11004, #9329)
Apr 7, 2024
elasticspoon
changed the title
feat(cli): prompt authenticated user on subsequent logins (#11004, #9329)
feat(cli): prompt user for authentication on subsequent logins (#11004, #9329)
Apr 7, 2024
elasticspoon
force-pushed
the
validate-relogin
branch
from
April 7, 2024 00:21
9d3225b
to
f45143b
Compare
ammario
requested review from
mafredri
and removed request for
sreya and
ammario
April 11, 2024 17:29
mafredri
reviewed
Apr 15, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few minor nits but LGTM otherwise, thanks for this PR!
cliui should return nil when returning cliui.Canceled (CTRL-C) and return error others fix nits
I made an incorrect assumption, we can check if a flag/env token is valid on login and create a new token if the provided one is invalid. However, on subsequent requests the invalid ENV token will still take priority over the valid token stored on disk. Therefore, this behavior does not work.
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.
Fixes #11004 (partially)
coder login
When Already AuthenticatedCurrently if a user runs
coder login
when logged in they will get the default login flow:Instead prompt the user if they want to re-authenticate if they run
coder login
when already authenticated. Authenticated is defined as having a valid session token stored in the config directory.Does not apply if they provided a valid token via flag or ENV variable. A new token should be generated an stored in that situation unless the
--use-token-as-session
flag was also used.Invalid Token
If an invalid token was set asENV
variable a user would be unable to login until theENV
variable was cleared. (The same behavior will occur for invalid flags)User will now be informed that the token is invalid and prompted to login normally. The ENV variable won't be cleared but a valid session token will be created.I was incorrect about this part. While we can create a new valid token at login subsequent commands will still prefer the invalid ENV token. A potential solution would be to check if the ENV token if valid and fall back to the config token in that situation, however, that creates the overhead of an additional API call on every authed CLI command invocation.