- Notifications
You must be signed in to change notification settings - Fork1k
feat: Login via CLI#298
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedFeb 16, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## main #298 +/- ##==========================================+ Coverage 64.81% 67.47% +2.66%========================================== Files 67 140 +73 Lines 756 7407 +6651 Branches 74 77 +3 ==========================================+ Hits 490 4998 +4508- Misses 251 1899 +1648- Partials 15 510 +495
Continue to review full report at Codecov.
|
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.
Minor nits. Overall, wonderful change!
Uh oh!
There was an error while loading.Please reload this page.
cli/login.go Outdated
| } | ||
| } | ||
| funcsaveSessionToken(cmd*cobra.Command,client*codersdk.Client,sessionTokenstring,serverURL*url.URL)error { |
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.
I'm hesitant to rip this into a distinct function. We already validate the "me" user request above, so feels like that should be reused.
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.
The alternative is to duplicate the persistence code both in the 'create initial user' and 'login' flows - inlined this function in13696a7
Another thing we could consider is bringing the"me" check above (since it's only required for initial login), and factor out the common save-session-token-and-url logic into this function. Happy with whichever you prefer.
Uh oh!
There was an error while loading.Please reload this page.
…307)Fixes#304 Unblocks#298 After logging in, the login flow should redirect to a whatever path is specified by the `?redirect` query parameter. This is important for cases like#298 - where we need to set `?redirect=%2Fcli_auth`, but also really any case where the user is linked and might have to go back to the login screen.The fix is simple - just check if the `redirect` query parameter is set, and if it is, use that as the path to redirect to on success. Also adds a test case - we had one checking that we redirect to the default (root `/`) url, but not one of the `?redirect` param
Uh oh!
There was an error while loading.Please reload this page.
Fixes#210 - this isPR implements
coder loginin the case where the default user is already created.This change adds:
openURLandisWSLfunctions to support opening the browser/api/v2/api-keysendpoint that can bePOST'd to in order to request a new api key for a usercli-authroute + page that shows the generated api keyThe
/cli-authroute, like in v1, is very minimal:And the terminal UX looks like this:
TODO:
http(s)://some-server/cli-authPOSTto the Api keys route400cli.OpenURLdata race:https://github.com/coder/coder/runs/5239455795?check_suite_focus=true#step:7:165