Conversation
Users with their own OAuth client credentials registered on Launchpad need the CLI's redirect_uri to match their app's registered callback. The CLI previously hardcoded http://127.0.0.1:8976/callback with no way to override it. Add resolveOAuthCallback() with precedence chain: LoginOptions.RedirectURI > BASECAMP_OAUTH_REDIRECT_URI env var > CallbackAddr-derived > hardcoded default Validate all redirect URIs (including default) against RFC 8252 loopback rules: http scheme, loopback host, explicit port, no userinfo/query/fragment. Rename credential env vars to BASECAMP_OAUTH_CLIENT_ID/SECRET with strict pairing (both required when either is set). BC3 DCR clients registered with a custom redirect URI are not persisted to client.json, preventing stale credentials on subsequent runs without the override. Closes #183
github-actions
bot
added
tests
There was a problem hiding this comment.
Pull request overview
Adds support for user-specified OAuth redirect_uri (via env var and programmatic options) so custom OAuth clients can complete the auth flow, with strict loopback redirect validation aligned with RFC 8252 and updated credential env var handling.
Changes:
- Add
BASECAMP_OAUTH_REDIRECT_URI+LoginOptions.RedirectURI, and resolve/validate loopback redirect + listener address viaresolveOAuthCallback(). - Rename/standardize OAuth credential env vars to
BASECAMP_OAUTH_CLIENT_ID/BASECAMP_OAUTH_CLIENT_SECRETwith strict pairing viaresolveClientCredentials(). - Adjust BC3 DCR persistence behavior: custom redirect URI clients are session-only; improve auth timeout error to include listener address; add comprehensive tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/auth/auth.go | Implements redirect URI resolution/validation, updates auth URL + token exchange to use resolved redirect URI, and adjusts BC3 DCR persistence behavior. |
| internal/auth/auth_test.go | Adds unit tests covering redirect resolution/validation, env credential pairing, redirect propagation, and BC3 persistence rules. |
| README.md | Documents new env vars for custom OAuth credentials and redirect URI configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…est isolation - Remove `customRedirect bool` parameter from `registerBC3Client`; derive it from `opts.RedirectURI != defaultRedirectURI` inside the function - Add `XDG_CONFIG_HOME` override in `TestRegisterBC3Client_UsesResolvedRedirectURI` to prevent writing to the developer's real config dir - Use full env var names in README sentence
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BASECAMP_OAUTH_REDIRECT_URIenv var andLoginOptions.RedirectURIfield so users with custom OAuth client credentials can specify a matching redirect URIresolveOAuthCallback()with full RFC 8252 loopback validation (http scheme, loopback host, explicit port, no userinfo/query/fragment)BASECAMP_OAUTH_CLIENT_ID/BASECAMP_OAUTH_CLIENT_SECRETwith strict pairingclient.json)Closes #183
Test plan
make checkpasses (fmt, vet, lint, unit tests, e2e tests)TestResolveOAuthCallback— 11 subtests covering default, env override, programmatic override, CallbackAddr compat, and all validation rejectionsTestResolveClientCredentials— 4 subtests covering both-set, ID-only error, secret-only error, neither-setTestBuildAuthURL_UsesResolvedRedirectURI— resolved URI propagates to authorization URLTestExchangeCode_UsesResolvedRedirectURI— resolved URI propagates to token exchangeTestRegisterBC3Client_UsesResolvedRedirectURI— resolved URI sent in DCR request bodyTestRegisterBC3Client_CustomRedirectNotPersisted— custom redirect skips client.json writeTestRegisterBC3Client_DefaultRedirectPersisted— default redirect persists as beforeTestLoadClientCredentials_BC3_CustomRedirect_SkipsStoredClient— custom redirect bypasses stored client, forces fresh DCR