-
Notifications
You must be signed in to change notification settings - Fork 70
feat(oauth): use oauth device flow to authenticate with predefined src-cli OAuth client #1223
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
base: main
Are you sure you want to change the base?
Changes from all commits
004a382
9f9d240
bb43d20
85dc6a4
3c7d643
75bf1b4
fd1668e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,17 +7,19 @@ import ( | |
| "io" | ||
| "os" | ||
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/sourcegraph/src-cli/internal/api" | ||
| "github.com/sourcegraph/src-cli/internal/cmderrors" | ||
| "github.com/sourcegraph/src-cli/internal/oauthdevice" | ||
| ) | ||
|
|
||
| func init() { | ||
| usage := `'src login' helps you authenticate 'src' to access a Sourcegraph instance with your user credentials. | ||
|
|
||
| Usage: | ||
|
|
||
| src login SOURCEGRAPH_URL | ||
| src login [flags] SOURCEGRAPH_URL | ||
|
|
||
| Examples: | ||
|
|
||
|
|
@@ -28,6 +30,15 @@ Examples: | |
| Authenticate to Sourcegraph.com: | ||
|
|
||
| $ src login https://sourcegraph.com | ||
|
|
||
| Use OAuth device flow to authenticate: | ||
|
|
||
| $ src login --device-flow https://sourcegraph.com | ||
|
|
||
|
|
||
| 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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agreed. It's already removed just have to update |
||
| ` | ||
|
|
||
| flagSet := flag.NewFlagSet("login", flag.ExitOnError) | ||
|
|
@@ -37,7 +48,9 @@ Examples: | |
| } | ||
|
|
||
| var ( | ||
| apiFlags = api.NewFlags(flagSet) | ||
| apiFlags = api.NewFlags(flagSet) | ||
| useDeviceFlow = flagSet.Bool("device-flow", false, "Use OAuth device flow to obtain an access token interactively") | ||
| OAuthClientID = flagSet.String("client-id", oauthdevice.DefaultClientID, "Client ID to use with OAuth device flow. Will use the predefined src cli client ID if not specified.") | ||
| ) | ||
|
|
||
| handler := func(args []string) error { | ||
|
|
@@ -52,9 +65,21 @@ Examples: | |
| return cmderrors.Usage("expected exactly one argument: the Sourcegraph URL, or SRC_ENDPOINT to be set") | ||
| } | ||
|
|
||
| if *OAuthClientID == "" { | ||
| return cmderrors.Usage("no value specified for client-id") | ||
| } | ||
|
|
||
| client := cfg.apiClient(apiFlags, io.Discard) | ||
|
|
||
| return loginCmd(context.Background(), cfg, client, endpoint, os.Stdout) | ||
| return loginCmd(context.Background(), loginParams{ | ||
| cfg: cfg, | ||
| client: client, | ||
| endpoint: endpoint, | ||
| out: os.Stdout, | ||
| useDeviceFlow: *useDeviceFlow, | ||
| apiFlags: apiFlags, | ||
| deviceFlowClient: oauthdevice.NewClient(*OAuthClientID), | ||
| }) | ||
| } | ||
|
|
||
| commands = append(commands, &command{ | ||
|
|
@@ -64,8 +89,21 @@ Examples: | |
| }) | ||
| } | ||
|
|
||
| func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg string, out io.Writer) error { | ||
| endpointArg = cleanEndpoint(endpointArg) | ||
| type loginParams struct { | ||
| cfg *config | ||
| client api.Client | ||
| endpoint string | ||
| out io.Writer | ||
| useDeviceFlow bool | ||
| apiFlags *api.Flags | ||
| deviceFlowClient oauthdevice.Client | ||
| } | ||
|
|
||
| func loginCmd(ctx context.Context, p loginParams) error { | ||
| endpointArg := cleanEndpoint(p.endpoint) | ||
| cfg := p.cfg | ||
| client := p.client | ||
| out := p.out | ||
|
|
||
| printProblem := func(problem string) { | ||
| fmt.Fprintf(out, "❌ Problem: %s\n", problem) | ||
|
|
@@ -86,7 +124,19 @@ func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg s | |
|
|
||
| noToken := cfg.AccessToken == "" | ||
| endpointConflict := endpointArg != cfg.Endpoint | ||
| if noToken || endpointConflict { | ||
|
|
||
| if p.useDeviceFlow { | ||
| token, err := runDeviceFlow(ctx, endpointArg, out, p.deviceFlowClient) | ||
| if err != nil { | ||
| printProblem(fmt.Sprintf("Device flow authentication failed: %s", err)) | ||
| fmt.Fprintln(out, createAccessTokenMessage) | ||
| return cmderrors.ExitCode1 | ||
| } | ||
|
|
||
| cfg.AccessToken = token | ||
| cfg.Endpoint = endpointArg | ||
| client = cfg.apiClient(p.apiFlags, out) | ||
| } else if noToken || endpointConflict { | ||
| fmt.Fprintln(out) | ||
| switch { | ||
| case noToken: | ||
|
|
@@ -122,6 +172,43 @@ func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg s | |
| } | ||
| fmt.Fprintln(out) | ||
| fmt.Fprintf(out, "✔️ Authenticated as %s on %s\n", result.CurrentUser.Username, endpointArg) | ||
|
|
||
| if p.useDeviceFlow { | ||
| 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) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will look at using https://github.com/99designs/keyring. We already use it with
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh nice, cross OS too :) |
||
| } | ||
|
|
||
| fmt.Fprintln(out) | ||
| return nil | ||
| } | ||
|
|
||
| func runDeviceFlow(ctx context.Context, endpoint string, out io.Writer, client oauthdevice.Client) (string, error) { | ||
| authResp, err := client.Start(ctx, endpoint, nil) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| fmt.Fprintln(out) | ||
| 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) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should:
|
||
| } | ||
| fmt.Fprintln(out) | ||
| fmt.Fprint(out, "Waiting for authorization...") | ||
| defer fmt.Fprintf(out, "DONE\n\n") | ||
|
|
||
| interval := time.Duration(authResp.Interval) * time.Second | ||
| if interval <= 0 { | ||
| interval = 5 * time.Second | ||
| } | ||
|
|
||
| tokenResp, err := client.Poll(ctx, endpoint, authResp.DeviceCode, interval, authResp.ExpiresIn) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| return tokenResp.AccessToken, nil | ||
| } | ||
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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_TOKENsetThere 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.
src loginis a misnomer currently haha, it should probably be renamed tosrc whoamiand thensrc loginis the interactive flow always 😬