Skip to content

Conversation

@burmudar
Copy link
Contributor

@burmudar burmudar commented Dec 3, 2025

This adds the flag --device-flow to login command which then starts the OAuth device authentication flow. gh does the same flow when you authenticate from the cli with gh auth login.

Screenshot 2025-12-03 at 15 17 20
  • add package internal/oauthdevice
  • discover oauth configuration via path .well-known/openid-configuration
  • add client to handle flow, in particular to discover the endpoints it should use and ultimately poll for the token once the user as authorized the application.

I wanted to add --client-id in 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

  • Unit tests
go test -v ./internal/oauthdevice/...
=== RUN   TestDiscover_Success
--- PASS: TestDiscover_Success (0.00s)
=== RUN   TestDiscover_Caching
--- PASS: TestDiscover_Caching (0.00s)
=== RUN   TestDiscover_Error
--- PASS: TestDiscover_Error (0.00s)
=== RUN   TestStart_Success
--- PASS: TestStart_Success (0.00s)
=== RUN   TestStart_WithScopes
--- PASS: TestStart_WithScopes (0.00s)
=== RUN   TestStart_Error
--- PASS: TestStart_Error (0.00s)
=== RUN   TestStart_NoDeviceEndpoint
--- PASS: TestStart_NoDeviceEndpoint (0.00s)
=== RUN   TestPoll_Success
--- PASS: TestPoll_Success (0.00s)
=== RUN   TestPoll_AuthorizationPending
--- PASS: TestPoll_AuthorizationPending (0.00s)
=== RUN   TestPoll_SlowDown
--- PASS: TestPoll_SlowDown (0.00s)
=== RUN   TestPoll_ExpiredToken
--- PASS: TestPoll_ExpiredToken (0.00s)
=== RUN   TestPoll_AccessDenied
--- PASS: TestPoll_AccessDenied (0.00s)
=== RUN   TestPoll_Timeout
--- PASS: TestPoll_Timeout (0.00s)
=== RUN   TestPoll_ContextCancellation
--- PASS: TestPoll_ContextCancellation (0.00s)
PASS
ok      github.com/sourcegraph/src-cli/internal/oauthdevice     0.625s
  • Tested manually against my local SG

Amp thread

@burmudar burmudar self-assigned this Dec 3, 2025
@burmudar burmudar requested review from a team and eseliger December 3, 2025 13:37
@burmudar burmudar marked this pull request as ready for review December 3, 2025 13:37
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no emojis in UI please 😬 since claude, this has an AI vibe slop feeling to it :(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

valid :) I'll remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 sg to store some secrets. It uses your OS keychain

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh nice, cross OS too :)

@burmudar burmudar marked this pull request as draft December 4, 2025 06:51
@burmudar burmudar force-pushed the wb/add-oauth-device-flow branch from 465fe85 to fd1668e Compare December 8, 2025 10:40
@burmudar
Copy link
Contributor Author

burmudar commented Dec 8, 2025

@burmudar burmudar changed the title feat(auth): use oauth device flow to authenticate with predefined src-cli OAuth client feat(oauth): use oauth device flow to authenticate with predefined src-cli OAuth client Dec 8, 2025
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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should:

  • always try to open the browser
  • always print the url

Use OAuth device flow to authenticate:
$ src login --device-flow https://sourcegraph.com
Copy link
Member

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?

Copy link
Contributor Author

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_TOKEN set

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. It's already removed just have to update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants