feat(oauth): use oauth device flow to authenticate with predefined src-cli OAuth client #1223
feat(oauth): use oauth device flow to authenticate with predefined src-cli OAuth client #1223
Conversation
cmd/src/login.go
Outdated
| } | ||
|
|
||
| fmt.Fprintln(out) | ||
| fmt.Fprintf(out, "🔐 To authenticate, visit %s and enter the code: %s\n", authResp.VerificationURI, authResp.UserCode) |
There was a problem hiding this comment.
no emojis in UI please 😬 since claude, this has an AI vibe slop feeling to it :(
There was a problem hiding this comment.
valid :) I'll remove it
There was a problem hiding this comment.
Removed. We need to do a larger sweep of the other emojis too :|
| fmt.Fprintln(out) | ||
| fmt.Fprintf(out, "To use this access token, set the following environment variables in your terminal:\n\n") | ||
| fmt.Fprintf(out, " export SRC_ENDPOINT=%s\n", endpointArg) | ||
| fmt.Fprintf(out, " export SRC_ACCESS_TOKEN=%s\n", cfg.AccessToken) |
There was a problem hiding this comment.
what you get here is not a SG access token, it's an oauth token and it comes with an access token and refresh token (and expiry) and needs to regularly be refreshed.
I think we need to store the accesstoken/refreshtoken pair in secure storage and add some http Transport that refreshes the credential as needed.
There was a problem hiding this comment.
Will look at using https://github.com/99designs/keyring. We already use it with sg to store some secrets. It uses your OS keychain
465fe85 to
fd1668e
Compare
|
This change is part of the following stack: Change managed by git-spice. |
| fmt.Fprintf(out, "To authenticate, visit %s and enter the code: %s\n", authResp.VerificationURI, authResp.UserCode) | ||
| if authResp.VerificationURIComplete != "" { | ||
| fmt.Fprintln(out) | ||
| fmt.Fprintf(out, "Alternatively, you can open: %s\n", authResp.VerificationURIComplete) |
There was a problem hiding this comment.
Should:
- always try to open the browser
- always print the url
|
|
||
| Use OAuth device flow to authenticate: | ||
|
|
||
| $ src login --device-flow https://sourcegraph.com |
There was a problem hiding this comment.
Do other CLI's require a flag for this? If I remember they are normally interactive right? You could still interactively decide between creating an access token vs oauth flows right?
There was a problem hiding this comment.
No they don't. It just happens. The plan it to make it part of the normal flow if you don't have SRC_ACCESS_TOKEN set
There was a problem hiding this comment.
src login is a misnomer currently haha, it should probably be renamed to src whoami and then src login is the interactive flow always 😬
|
|
||
| Override the default client id used during device flow when authenticating: | ||
|
|
||
| $ src login --device-flow https://sourcegraph.com --client-id sgo_my_own_client_id |
There was a problem hiding this comment.
doesn't seem worth supporting a custom client-id given you shipped a predefined one. If a user still hasn't upgraded sourcegraph just fallback to the old flow?
There was a problem hiding this comment.
agreed. It's already removed just have to update
fd1668e to
1565618
Compare
1565618 to
9a04958
Compare
This adds the flag
--device-flowtologincommand which then starts the OAuth device authentication flow.ghdoes the same flow when you authenticate from the cli withgh auth login.internal/oauthdevice.well-known/openid-configurationI wanted to add
--client-idin case people wanted to override the default client that is used which can also be used for testing, but when I tried creating a client on S2 it doesn't have the correct configuration as set from the UI to be able to do this.Important
Seems like a lot of code but it's the tests that make up most of it
Test plan
Amp thread