Send login_hint with the device authorization request - #841
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
RFC 8628 §3.1 lets a device client tell the authorization server which user it expects to approve the code. Basecamp's device verification page is growing support for it, so basecamp-cli can steer a shared machine (or a person with several identities) to the right sign-in without the server ever treating the hint as authentication. WithDeviceLoginHint adds the parameter to the device authorization POST; unset leaves the form unchanged. Go only for now — the other SDKs have no caller asking for it yet, and SPEC §16 records the parameter as optional.
51652e1 to
7eb7c34
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The specification and public Go documentation incorrectly attribute login_hint to RFC 8628.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds optional login_hint support to Go device authorization requests.
Changes:
- Adds
WithDeviceLoginHint. - Sends and tests optional
login_hint. - Updates the device-flow specification.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
SPEC.md |
Documents the optional parameter. |
go/pkg/basecamp/oauth/device.go |
Implements the option and request field. |
go/pkg/basecamp/oauth/device_test.go |
Tests inclusion and omission. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A bot's Basecamp identity should never come from a browser sign-in on a shared machine: whoever's session the browser holds is who the profile quietly becomes, and nothing checks. Basecamp now mints personal access tokens, so the CLI can take one from a secret store and refuse to keep it unless it authenticates as the expected identity. `basecamp auth login --with-token -P <profile> --account <id>` reads the token from stdin (a terminal is refused, with the `op read … |` shape shown), stores it under the profile as a non-expiring bc5 credential — ExpiresAt 0 is the path AccessToken already never refreshes — and creates the profile when --account names its account. The profile entry is written only after /my/profile.json and the authorization endpoint have answered for the token; a rejected token leaves the profile exactly as it was found, previous credential included. Profile registration is factored out of `profile create` so both paths write the same entry, and the root pre-run lets a may-create command name a profile that does not exist yet. `--expect-identity <id>` makes every login assertive — browser and device flows included — discarding the new credential on a mismatch. `--json` returns the envelope (profile, account, identity, person, oauth_type, scope, expires_at null), and the interactive flows now refuse machine output modes up front instead of printing prose and blocking (the machine-mode half of #669). `BASECAMP_OAUTH_ISSUER` pins the BC5 authorization server and skips discovery, so a device login reaches a server that is piloting the client while its metadata is still dark. It is a temporary escape hatch and is documented as one. `--login-hint <email>` is wired through LoginOptions; the pinned SDK cannot put it on the wire yet (basecamp/basecamp-sdk#841 adds the option), so this build tells the user the hint instead of sending it, and Launchpad ignores it.
There was a problem hiding this comment.
All reported issues were addressed
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
RFC 8628 §3.1 defines only client_id and scope for the device authorization request and leaves room for extension parameters; login_hint is Basecamp's, named after the OpenID Connect parameter. Say so in the option's doc and in SPEC §16, and thread loginHint through performDeviceLogin's signature there, since Go's PerformDeviceLogin forwards the option.
RFC 8628 §3.1 lets a device client name the user it expects to approve the code (
login_hint). Basecamp's device verification page is growing support for it, sobasecamp auth login --login-hint <email>can steer a shared machine — or a person with several identities — to the right sign-in. The server never treats the hint as authentication; approval still comes from whoever holds the session.What
oauth.WithDeviceLoginHint(hint)— aDeviceOptionthat addslogin_hintto the device authorization POST. Unset leaves the form exactly as it was.requestDeviceAuthorizationgains the optionalloginHint?parameter, marked Go-only for now: no other SDK has a caller asking for it, and the option is additive, so parity can follow when one does.Consumer
basecamp-cli wires
--login-hintthroughLoginOptions.LoginHintand appends this option once it bumps to an SDK that carries it (basecamp/basecamp-cli — "auth login --with-token" PR).Verification
cd go && go vet ./pkg/basecamp/oauth && go test ./pkg/basecamp/oauth— green. NewTestRequestDeviceAuthorization_SendsLoginHintWhenSetcovers both the sent form and the omitted-when-unset form.gofmt -lclean.make lintdid not run locally (golangci-lint on this machine predates the Go 1.27 export-data format and fails typecheck on every package,mainincluded); CI's lint is the gate.Summary by cubic
Adds
oauth.WithDeviceLoginHintsooauth.RequestDeviceAuthorizationcan include Basecamp’s optional RFC 8628login_hint, steering shared or multi-identity machines to the intended sign-in. Without the option, requests are unchanged; the hint never authenticates, and approval still comes from whoever holds the session.Spec
loginHint?support for device authorization and device login as Go-only until another SDK needs it.Written for commit f9426ed. Summary will update on new commits.