diff --git a/README.md b/README.md index dadb00e99..9192e0615 100644 --- a/README.md +++ b/README.md @@ -111,12 +111,15 @@ Breadcrumbs suggest next commands, making it easy for humans and agents to navig ## Authentication -OAuth 2.1 with automatic token refresh. First login opens your browser: +OAuth 2.1 with automatic token refresh. First login opens your browser. +When the server supports it, login uses the OAuth device flow automatically: +you approve a short code in the browser instead of a redirect. Otherwise +login falls back to Launchpad's authorization-code flow. ```bash basecamp auth login # Authenticate with Basecamp -basecamp auth login --scope read # Read-only access (BC3 OAuth only, default) -basecamp auth login --scope full # Full read+write access (BC3 OAuth only) +basecamp auth login --scope read # Read-only access (default; ignored by Launchpad) +basecamp auth login --scope full # Full read+write access (ignored by Launchpad) basecamp auth token # Print token for scripts ``` @@ -175,7 +178,6 @@ See [install.md](install.md) for step-by-step setup instructions. ``` ~/.config/basecamp/ # Your Basecamp identity ├── credentials.json # OAuth tokens (fallback when keyring unavailable) -├── client.json # DCR client registration └── config.json # Global preferences ~/.config/basecamp/theme/ # Tool display (optional) @@ -189,6 +191,9 @@ See [install.md](install.md) for step-by-step setup instructions. └── config.json # Project, account defaults ``` +A leftover `~/.config/basecamp/client.json` (from the removed development +client-registration flow) is obsolete and safe to delete. + ## Troubleshooting ```bash diff --git a/e2e/auth.bats b/e2e/auth.bats index 6fcfb8336..16f36f8c0 100644 --- a/e2e/auth.bats +++ b/e2e/auth.bats @@ -74,6 +74,20 @@ load test_helper assert_output_contains "--device-code" } +@test "basecamp auth login --help describes flags provider-neutrally" { + run basecamp auth login --help + assert_success + assert_output_contains "Headless authentication with manual browser instructions" + assert_output_contains "ignored by Launchpad" +} + +@test "basecamp profile create --help describes flags provider-neutrally" { + run basecamp profile create --help + assert_success + assert_output_contains "Headless authentication with manual browser instructions" + assert_output_contains "ignored by Launchpad" +} + @test "basecamp auth login rejects --device-code --local" { run basecamp auth login --device-code --local assert_failure diff --git a/go.mod b/go.mod index bf3e2bda0..1c3e9bd2c 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( charm.land/bubbles/v2 v2.1.1 charm.land/bubbletea/v2 v2.0.8 charm.land/lipgloss/v2 v2.0.5 - github.com/basecamp/basecamp-sdk/go v0.9.1-0.20260727173625-bb363c847b92 + github.com/basecamp/basecamp-sdk/go v0.9.1-0.20260728223259-0bdb14ac4a7b github.com/basecamp/cli v0.2.2-0.20260728023309-04e401b12c6c github.com/charmbracelet/bubbles v1.0.0 github.com/charmbracelet/glamour v1.0.0 diff --git a/go.sum b/go.sum index 11a120dd6..3be59757e 100644 --- a/go.sum +++ b/go.sum @@ -23,8 +23,8 @@ github.com/aymanbagabas/go-udiff v0.4.1 h1:OEIrQ8maEeDBXQDoGCbbTTXYJMYRCRO1fnodZ github.com/aymanbagabas/go-udiff v0.4.1/go.mod h1:0L9PGwj20lrtmEMeyw4WKJ/TMyDtvAoK9bf2u/mNo3w= github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk= github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4= -github.com/basecamp/basecamp-sdk/go v0.9.1-0.20260727173625-bb363c847b92 h1:4gQJTR8e5kBvXHDHLvef+Q00XY7KzWTxnQSLWMSPcyA= -github.com/basecamp/basecamp-sdk/go v0.9.1-0.20260727173625-bb363c847b92/go.mod h1:r83ralDQ0q9vbAby5qQ5x9hgCgUdJLDLHYpiU6jaFjE= +github.com/basecamp/basecamp-sdk/go v0.9.1-0.20260728223259-0bdb14ac4a7b h1:owC4eQieiylIAXVSwb63fOKqZNt0m/vMTLhLNnuwcsM= +github.com/basecamp/basecamp-sdk/go v0.9.1-0.20260728223259-0bdb14ac4a7b/go.mod h1:r83ralDQ0q9vbAby5qQ5x9hgCgUdJLDLHYpiU6jaFjE= github.com/basecamp/cli v0.2.2-0.20260728023309-04e401b12c6c h1:+5sQBl8sqYoD1Qhwsibn8sBCKWPyZ9NDez6mnuo9Afo= github.com/basecamp/cli v0.2.2-0.20260728023309-04e401b12c6c/go.mod h1:EK1Dba6DEw8ZAilVBpf/jri3ONDV7LQkLACSDe73f/c= github.com/bmatcuk/doublestar v1.1.1/go.mod h1:UD6OnuiIn0yFxxA2le/rnRU1G4RaI4UvFv1sNto9p6w= diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 0aa75856d..186a80609 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -4,14 +4,13 @@ package auth import ( "bufio" "context" - "encoding/json" - "errors" "fmt" "io" "net" "net/http" "net/url" "os" + "strconv" "strings" "sync" "time" @@ -22,6 +21,7 @@ import ( "github.com/basecamp/basecamp-cli/internal/config" "github.com/basecamp/basecamp-cli/internal/hostutil" "github.com/basecamp/basecamp-cli/internal/output" + "github.com/basecamp/basecamp-cli/internal/richtext" ) // ClientCredentials holds OAuth client ID and secret. @@ -37,6 +37,17 @@ const ( launchpadClientSecret = "a3dc33d78258e828efd6768ac2cd67f32ec1910a" //nolint:gosec // G101: Public OAuth client secret for native app ) +// bc5ClientID is the pre-registered public client for the BC5 device flow. +// Public client: no secret. +const bc5ClientID = "basecamp-cli" + +// OAuth provider types stored in credentials. Legacy "bc3" (the removed +// DCR/PKCE development flow) may still appear in stored credentials. +const ( + oauthTypeBC5 = "bc5" + oauthTypeLaunchpad = "launchpad" +) + // Default OAuth callback address and redirect URI. const ( defaultCallbackAddr = "127.0.0.1:8976" @@ -178,12 +189,12 @@ func (m *Manager) refreshLocked(ctx context.Context, origin string, creds *Crede // Migrate old credentials missing OAuthType if creds.OAuthType == "" { - creds.OAuthType = "launchpad" + creds.OAuthType = oauthTypeLaunchpad } // Migrate old credentials missing TokenEndpoint if creds.TokenEndpoint == "" { - if creds.OAuthType == "bc3" { + if creds.OAuthType == "bc3" || creds.OAuthType == oauthTypeBC5 { return output.ErrAuth("Stored credentials missing token endpoint — please re-authenticate: basecamp auth login") } lpURL, lpErr := m.launchpadURL() @@ -197,35 +208,23 @@ func (m *Manager) refreshLocked(ctx context.Context, origin string, creds *Crede // The token endpoint here is a persisted (possibly migrated) value from // the credential store and receives the refresh token plus client - // credentials. The SDK's RequireSecureEndpoint only checks scheme==https, - // so a poisoned store could still carry userinfo (https://user@evil/), - // empty-host, or opaque/malformed https forms that it would let through. - // Apply the same strict check used for the other OAuth endpoints before - // any POST. - if u, err := url.Parse(tokenEndpoint); err != nil { - return output.ErrAuth(fmt.Sprintf("invalid token endpoint %q: %v", tokenEndpoint, err)) - } else if !isSecureEndpointURL(u) { - return output.ErrAuth(fmt.Sprintf("invalid token endpoint %q: must be an absolute https URL (or http on loopback)", tokenEndpoint)) + // credentials. A poisoned store could carry userinfo (https://user@evil/), + // empty-host, or opaque/malformed https forms, so apply the same strict + // check used for the other OAuth endpoints before any POST. + if err := requireSecureOAuthEndpoint("token endpoint", tokenEndpoint); err != nil { + return err } // Resolve client credentials for the refresh request var clientID, clientSecret string switch creds.OAuthType { case "bc3": - cc, err := m.loadBC3Client() - if err != nil { - if os.IsNotExist(err) { - // DCR credentials from custom-redirect logins are intentionally - // not persisted (see registerBC3Client). After a process restart - // the client.json won't exist and refresh is impossible. - return output.ErrAuth("Cannot load BC3 client credentials for token refresh. " + - "This can happen after a custom-redirect login (credentials are session-only). " + - "Please re-authenticate: basecamp auth login") - } - return output.ErrAuth(fmt.Sprintf("Cannot load BC3 client credentials for token refresh: %v", err)) - } - clientID = cc.ClientID - clientSecret = cc.ClientSecret + // DCR-era development flow, removed. Its per-install dynamic clients + // can't be resolved anymore, so the refresh token is unusable. + return output.ErrAuth("Stored credentials are from a removed development flow — please re-authenticate: basecamp auth login") + case oauthTypeBC5: + // Pre-registered public client: no secret. + clientID = bc5ClientID default: // Launchpad (or old credentials defaulted to launchpad) if envCreds, err := resolveClientCredentials(func(string) {}); err != nil { @@ -242,11 +241,14 @@ func (m *Manager) refreshLocked(ctx context.Context, origin string, creds *Crede exchanger := oauth.NewExchanger(m.httpClient) req := oauth.RefreshRequest{ - TokenEndpoint: tokenEndpoint, - RefreshToken: creds.RefreshToken, - ClientID: clientID, - ClientSecret: clientSecret, - UseLegacyFormat: creds.OAuthType == "launchpad", + TokenEndpoint: tokenEndpoint, + RefreshToken: creds.RefreshToken, + ClientID: clientID, + ClientSecret: clientSecret, + // Echo the stored RFC 8707 resource indicator (sent only when set): + // BC5 multi-account refresh tokens are rejected without it. + Resource: creds.Resource, + UseLegacyFormat: creds.OAuthType == oauthTypeLaunchpad, } token, err := exchanger.Refresh(ctx, req) @@ -258,6 +260,11 @@ func (m *Manager) refreshLocked(ctx context.Context, origin string, creds *Crede if token.RefreshToken != "" { creds.RefreshToken = token.RefreshToken } + // An omitted resource preserves the stored binding (carry-forward, like an + // omitted rotated refresh token); a present one replaces it. + if token.Resource != "" { + creds.Resource = token.Resource + } if !token.ExpiresAt.IsZero() { creds.ExpiresAt = token.ExpiresAt.Unix() } else { @@ -273,8 +280,8 @@ func (m *Manager) refreshLocked(ctx context.Context, origin string, creds *Crede // LoginResult holds the outcome of a successful Login(). // Callers use this to determine the effective scope instead of their input. type LoginResult struct { - OAuthType string // "bc3" or "launchpad" - Scope string // effective scope: "read"/"full" for BC3, "" for Launchpad + OAuthType string // "bc5" or "launchpad" (stored credentials may also carry legacy "bc3") + Scope string // effective scope: "read"/"full" for BC5, "" for Launchpad } // LoginOptions configures the login flow. @@ -310,6 +317,10 @@ type LoginOptions struct { // Logger receives status messages during the login flow. // If nil, messages are suppressed for headless/SDK use. Logger func(msg string) + + // deviceOptions are appended last to the SDK device-flow options. + // Test seam: lets tests inject WithDeviceSleep/WithDeviceClock. + deviceOptions []oauth.DeviceOption } // defaults fills in default values for LoginOptions. @@ -373,7 +384,9 @@ func resolveOAuthCallback(opts *LoginOptions) (redirectURI string, listenAddr st return raw, u.Host, nil } -// Login initiates the OAuth login flow. +// Login initiates the OAuth login flow. Discovery selects the provider: +// a BC5 issuer runs the RFC 8628 device flow; the Launchpad fallback runs +// the authorization-code flow with a loopback (or pasted) callback. func (m *Manager) Login(ctx context.Context, opts LoginOptions) (*LoginResult, error) { if opts.Remote && opts.Local { return nil, output.ErrUsage("--remote and --local are mutually exclusive") @@ -386,65 +399,53 @@ func (m *Manager) Login(ctx context.Context, opts LoginOptions) (*LoginResult, e opts.defaults() - // Resolve redirect URI and listener address - redirectURI, listenAddr, err := resolveOAuthCallback(&opts) + credKey := m.credentialKey() + + disc, err := m.discoverOAuth(ctx, opts.log) if err != nil { return nil, err } - opts.RedirectURI = redirectURI - // Log overrides - if redirectURI != defaultRedirectURI { - opts.log(fmt.Sprintf("Using custom redirect URI: %s", redirectURI)) + if disc.oauthType == oauthTypeBC5 { + return m.loginDevice(ctx, credKey, disc.config, &opts) } + return m.loginLaunchpad(ctx, credKey, disc.config, &opts) +} - credKey := m.credentialKey() - - // Discover OAuth config - oauthCfg, oauthType, err := m.discoverOAuth(ctx, opts.log) +// loginLaunchpad runs the authorization-code flow against Launchpad: +// browser (or printed) auth URL, then a loopback callback or pasted +// callback URL in remote mode. +func (m *Manager) loginLaunchpad(ctx context.Context, credKey string, oauthCfg *oauth.Config, opts *LoginOptions) (*LoginResult, error) { + // Resolve redirect URI and listener address + redirectURI, listenAddr, err := resolveOAuthCallback(opts) if err != nil { return nil, err } + opts.RedirectURI = redirectURI - // Device-only authorization servers omit the authorization endpoint. - // Assert authorization-code capability before scope handling and client - // registration so an unsupported flow can't trigger DCR side effects. - if oauthCfg.AuthorizationEndpoint == nil || *oauthCfg.AuthorizationEndpoint == "" { - return nil, output.ErrAuth("OAuth server does not advertise an authorization endpoint (authorization-code flow unsupported)") + // Log overrides + if redirectURI != defaultRedirectURI { + opts.log(fmt.Sprintf("Using custom redirect URI: %s", redirectURI)) } - // Apply provider-aware scope rules - effectiveScope := opts.Scope - if oauthType == "launchpad" { - if effectiveScope != "" { - opts.log("Launchpad does not support OAuth scopes; --scope ignored") - } - effectiveScope = "" - } else { - // BC3: default to "read" when no scope specified - if effectiveScope == "" { - effectiveScope = "read" - } + if opts.Scope != "" { + opts.log("Launchpad does not support OAuth scopes; --scope ignored") } - // Load or register client credentials - clientCreds, err := m.loadClientCredentials(ctx, oauthCfg, oauthType, &opts) + clientCreds, err := launchpadClientCredentials(opts.log) if err != nil { return nil, err } - // Generate PKCE challenge (for BC3) - var codeVerifier, codeChallenge string - if oauthType == "bc3" { - codeVerifier = pkce.GenerateVerifier() - codeChallenge = pkce.GenerateChallenge(codeVerifier) - } - // Generate state for CSRF protection state := pkce.GenerateState() + if oauthCfg.AuthorizationEndpoint == nil { + return nil, output.ErrAuth("authorization server did not advertise an authorization endpoint") + } + // Build authorization URL - authURL, err := m.buildAuthURL(oauthCfg, oauthType, effectiveScope, state, codeChallenge, clientCreds.ClientID, &opts) + authURL, err := m.buildAuthURL(*oauthCfg.AuthorizationEndpoint, state, clientCreds.ClientID, opts) if err != nil { return nil, err } @@ -515,21 +516,153 @@ func (m *Manager) Login(ctx context.Context, opts LoginOptions) (*LoginResult, e defer resolve(false) // safety net: signal failure if we return without explicit resolve // Exchange code for tokens - creds, err := m.exchangeCode(ctx, oauthCfg, oauthType, code, codeVerifier, clientCreds, &opts) + creds, err := m.exchangeCode(ctx, oauthCfg, code, clientCreds, opts) if err != nil { return nil, err } - creds.OAuthType = oauthType + creds.OAuthType = oauthTypeLaunchpad creds.TokenEndpoint = oauthCfg.TokenEndpoint - creds.Scope = effectiveScope + creds.Scope = "" if err := m.store.Save(credKey, creds); err != nil { return nil, err } resolve(true) - return &LoginResult{OAuthType: oauthType, Scope: effectiveScope}, nil + return &LoginResult{OAuthType: oauthTypeLaunchpad, Scope: ""}, nil +} + +// loginDevice runs the RFC 8628 device authorization grant against a +// discovered BC5 issuer as the pre-registered public client. The display +// callback is the trust boundary for the server-controlled device +// authorization response: URIs are validated before launch and all printed +// copies are sanitized. +func (m *Manager) loginDevice(ctx context.Context, credKey string, oauthCfg *oauth.Config, opts *LoginOptions) (*LoginResult, error) { + // Both endpoints come from the server-controlled discovery document; + // reject unsafe forms before any POST. A nil device endpoint is left to + // the SDK's capability guard, which fails without making a request. + if err := requireSecureOAuthEndpoint("token endpoint", oauthCfg.TokenEndpoint); err != nil { + return nil, err + } + if oauthCfg.DeviceAuthorizationEndpoint != nil { + if err := requireSecureOAuthEndpoint("device authorization endpoint", *oauthCfg.DeviceAuthorizationEndpoint); err != nil { + return nil, err + } + } + + devOpts := []oauth.DeviceOption{oauth.WithDeviceHTTPClient(m.httpClient)} + if opts.Scope != "" { + devOpts = append(devOpts, oauth.WithDeviceScope(opts.Scope)) + } + devOpts = append(devOpts, opts.deviceOptions...) + + // The SDK display hook can't return an error, and the SDK proceeds into + // polling regardless. On malformed display data the callback records + // displayErr and cancels this derived context to abort before polling. + devCtx, cancelDev := context.WithCancel(ctx) + defer cancelDev() + + var displayErr error + display := func(devAuth oauth.DeviceAuthorization) { + // Validate the raw server-supplied URIs before printing or launching + // anything: browser target is the code-embedding URI when valid, + // falling back to the plain verification URI. + target := "" + if devAuth.VerificationURIComplete != nil { + target = validVerificationURL(*devAuth.VerificationURIComplete) + } + if target == "" { + target = validVerificationURL(devAuth.VerificationURI) + } + + // The command logger prints raw to the terminal, so strip + // ANSI/OSC/control sequences from everything displayed. Trim after + // sanitizing: a code that reduces to whitespace is as unusable as an + // empty one — without the trim it would be displayed and polled until + // expiry. + userCode := strings.TrimSpace(richtext.SanitizeSingleLine(devAuth.UserCode)) + shownURI := strings.TrimSpace(richtext.SanitizeSingleLine(target)) + if target == "" || userCode == "" || shownURI == "" { + displayErr = output.ErrAPI(0, "authorization server returned malformed device authorization") + cancelDev() + return + } + + opts.log("\nTo authenticate, open this URL in a browser on any device:") + opts.log(" " + shownURI) + opts.log("") + opts.log("and enter the code: " + userCode) + if devAuth.ExpiresIn > 0 { + opts.log(fmt.Sprintf("The code expires in %v.", time.Duration(devAuth.ExpiresIn)*time.Second)) + } + // Flag matrix: default/--local launch the browser; --remote, + // --device-code, and --no-browser (Remote implies NoBrowser) print + // only. defaults() leaves BrowserLauncher nil in headless modes, but + // honor NoBrowser too so an injected launcher can't override it. + if !opts.NoBrowser && opts.BrowserLauncher != nil { + if launchErr := opts.BrowserLauncher(target); launchErr != nil { + opts.log("\nCouldn't open browser automatically — use the URL above.") + } else { + opts.log("\nOpening browser for authentication...") + } + } + opts.log("\nWaiting for approval...") + } + + token, err := oauth.PerformDeviceLogin(devCtx, oauthCfg, bc5ClientID, display, devOpts...) + if displayErr != nil { + // The malformed display data — not the cancellation it triggered — + // is the real cause. + return nil, displayErr + } + if err != nil { + return nil, err + } + + effectiveScope := token.Scope + if effectiveScope == "" { + effectiveScope = opts.Scope + } + if effectiveScope == "" { + effectiveScope = "read" + } + + creds := &Credentials{ + AccessToken: token.AccessToken, + RefreshToken: token.RefreshToken, + OAuthType: oauthTypeBC5, + TokenEndpoint: oauthCfg.TokenEndpoint, + Scope: effectiveScope, + // The RFC 8707 account binding: a trusted-client device login mints a + // multi-account refresh token, and refreshing one without echoing this + // is rejected (400 invalid_request) — losing it here would strand the + // login at first token expiry. + Resource: token.Resource, + } + if !token.ExpiresAt.IsZero() { + creds.ExpiresAt = token.ExpiresAt.Unix() + } + + if err := m.store.Save(credKey, creds); err != nil { + return nil, err + } + + return &LoginResult{OAuthType: oauthTypeBC5, Scope: effectiveScope}, nil +} + +// validVerificationURL validates a server-supplied verification URI with the +// same policy as other OAuth browser URLs (https, or http on loopback, no +// userinfo). Returns the raw URL when valid, "" otherwise. +func validVerificationURL(raw string) string { + if raw == "" { + return "" + } + u, err := url.Parse(raw) + if err != nil || !isSecureEndpointURL(u) { + return "" + } + return raw } // Logout removes stored credentials. @@ -538,38 +671,118 @@ func (m *Manager) Logout() error { return m.store.Delete(credKey) } -func (m *Manager) discoverOAuth(ctx context.Context, log func(string)) (*oauth.Config, string, error) { - // The SDK binds the discovered issuer to this string code-point exact - // (RFC 8414), so a trailing slash in the configured base URL would - // mismatch the server's issuer and incorrectly fall back to Launchpad. - baseURL := config.NormalizeBaseURL(m.cfg.BaseURL) +// discovery is the outcome of provider selection: the OAuth config to use +// and which login flow it drives. +type discovery struct { + config *oauth.Config + oauthType string + issuer string +} + +// discoverOAuth performs resource-first OAuth discovery (RFC 9728 → RFC 8414) +// from the configured base URL's origin. Only the two soft pre-selection +// outcomes fall back to Launchpad; once a BC5 issuer is selected, every +// failure is returned loudly — never converted into a Launchpad attempt. +func (m *Manager) discoverOAuth(ctx context.Context, log func(string)) (*discovery, error) { + origin, err := resourceOrigin(m.cfg.BaseURL) + if err != nil { + return nil, err + } + discoverer := oauth.NewDiscoverer(m.httpClient) - cfg, err := discoverer.Discover(ctx, baseURL) + res, err := discoverer.DiscoverFromResource(ctx, origin) if err != nil { - log(fmt.Sprintf("warning: OAuth discovery failed for %s, using Launchpad fallback", baseURL)) - // Fallback to Launchpad + // Hard selection failure: propagate unchanged. output.AsError at the + // root maps the wrapped *basecamp.Error taxonomy to exit codes. + return nil, err + } + + if res.IsFallback() { + if res.FallbackReason == oauth.FallbackResourceDiscoveryFailed { + log(fmt.Sprintf("warning: OAuth discovery failed for %s, using Launchpad fallback", origin)) + } lpURL, lpErr := m.launchpadURL() if lpErr != nil { - return nil, "", lpErr + return nil, lpErr } - authzEndpoint := lpURL + "/authorization/new" + authz := lpURL + "/authorization/new" fallbackCfg := &oauth.Config{ - AuthorizationEndpoint: &authzEndpoint, + AuthorizationEndpoint: &authz, TokenEndpoint: lpURL + "/authorization/token", } - log(fmt.Sprintf("Authenticating via launchpad (%s)", authzEndpoint)) - return fallbackCfg, "launchpad", nil + log(fmt.Sprintf("Authenticating via launchpad (%s)", authz)) + return &discovery{config: fallbackCfg, oauthType: oauthTypeLaunchpad}, nil } - log(fmt.Sprintf("Authenticating via bc3 (%s)", strOrEmpty(cfg.AuthorizationEndpoint))) - return cfg, "bc3", nil + + log(fmt.Sprintf("Authenticating via %s (device flow)", res.Issuer)) + return &discovery{config: res.Config, oauthType: oauthTypeBC5, issuer: res.Issuer}, nil } -// strOrEmpty returns the value of p, or "" when p is nil. -func strOrEmpty(p *string) string { - if p == nil { - return "" +// resourceOrigin reduces the configured base URL to a bare scheme://host[:port] +// origin for RFC 9728 protected-resource discovery, CANONICALIZED: lowercase +// scheme and host, explicit default ports stripped. The SDK binds the +// protected-resource metadata's resource identifier to this string code-point +// exact, so an equivalent spelling (HTTPS://3.BasecampAPI.com, +// https://3.basecampapi.com:443) would otherwise mismatch the advertised +// identifier and silently soft-fall back to Launchpad. Only the path is +// stripped and case/default-port normalized; every other deviation is rejected +// rather than laundered. Validation failures never echo the raw URL: userinfo, +// query strings, and fragments can carry secrets, and parse errors can +// reproduce their input. +func resourceOrigin(baseURL string) (string, error) { + fail := func(rule string) (string, error) { + return "", output.ErrUsage("invalid base URL: " + rule) + } + + u, err := url.Parse(baseURL) + if err != nil { + return fail("not a parseable URL") + } + if u.Opaque != "" || !u.IsAbs() { + return fail("must be an absolute URL") + } + if u.Hostname() == "" { + return fail("must include a hostname") + } + // The localhost carve-out checks the LOWERCASED host: DNS names are + // case-insensitive, so http://LocalHost is as local as http://localhost. + if u.Scheme != "https" && (u.Scheme != "http" || !hostutil.IsLocalhost(strings.ToLower(u.Host))) { + return fail("scheme must be https (or http on localhost)") + } + if u.User != nil { + return fail("userinfo is not allowed") + } + // ForceQuery catches the bare-"?" form ("https://host?"), which parses + // with an empty RawQuery but still carries a query component. + if u.RawQuery != "" || u.ForceQuery { + return fail("query string is not allowed") } - return *p + if u.Fragment != "" { + return fail("fragment is not allowed") + } + // url.Parse lowercases the scheme; the host keeps its input case and must + // be lowered here (DNS names are case-insensitive, the code-point binding + // is not). Hostname() strips IPv6 brackets — restore them so the origin + // stays a parseable URL. + hostname := strings.ToLower(u.Hostname()) + if strings.Contains(hostname, ":") { + hostname = "[" + hostname + "]" + } + host := hostname + if port := u.Port(); port != "" { + // url.Parse requires a numeric port but does not range-check it. + n, portErr := strconv.Atoi(port) + if portErr != nil || n < 1 || n > 65535 { + return fail("port must be between 1 and 65535") + } + // Strip an explicit default port (the canonical origin form) and + // normalize leading zeros; any other port is kept numerically. + if (u.Scheme != "https" || n != 443) && (u.Scheme != "http" || n != 80) { + host += ":" + strconv.Itoa(n) + } + } + + return u.Scheme + "://" + host, nil } func (m *Manager) launchpadURL() (string, error) { @@ -582,25 +795,10 @@ func (m *Manager) launchpadURL() (string, error) { return "https://launchpad.37signals.com", nil } -func (m *Manager) loadClientCredentials(ctx context.Context, oauthCfg *oauth.Config, oauthType string, opts *LoginOptions) (*ClientCredentials, error) { - if oauthType == "bc3" { - // BC3 with default redirect: try stored client first - if opts.RedirectURI == defaultRedirectURI { - creds, err := m.loadBC3Client() - if err == nil { - return creds, nil - } - } - - // Register new client via DCR - if oauthCfg.RegistrationEndpoint == nil || *oauthCfg.RegistrationEndpoint == "" { - return nil, output.ErrAuth("OAuth server does not support Dynamic Client Registration") - } - return m.registerBC3Client(ctx, *oauthCfg.RegistrationEndpoint, opts) - } - - // Launchpad: resolve client credentials from env vars - creds, err := resolveClientCredentials(opts.log) +// launchpadClientCredentials resolves the Launchpad OAuth client: env var +// overrides first, then the built-in production credentials. +func launchpadClientCredentials(log func(string)) (*ClientCredentials, error) { + creds, err := resolveClientCredentials(log) if err != nil { return nil, err } @@ -636,25 +834,6 @@ func resolveClientCredentials(log func(string)) (*ClientCredentials, error) { return &ClientCredentials{ClientID: clientID, ClientSecret: clientSecret}, nil } -func (m *Manager) loadBC3Client() (*ClientCredentials, error) { - clientFile := config.GlobalConfigDir() + "/client.json" - data, err := os.ReadFile(clientFile) //nolint:gosec // G304: Path is from trusted config dir - if err != nil { - return nil, err - } - - var creds ClientCredentials - if err := json.Unmarshal(data, &creds); err != nil { - return nil, err - } - - if creds.ClientID == "" { - return nil, fmt.Errorf("no client_id in stored credentials") - } - - return &creds, nil -} - // isSecureEndpointURL reports whether u uses a scheme safe for OAuth endpoints // derived from the server-controlled discovery document: https, or http only on // loopback for local development. The URL must also be absolute with a hostname — @@ -663,8 +842,8 @@ func (m *Manager) loadBC3Client() (*ClientCredentials, error) { // otherwise slip through to the transport or browser launcher. URLs carrying // userinfo (user:pass@host) are rejected outright: they enable phishing // displays in browsers and net/http synthesizes a Basic Authorization header -// from them. Centralizing the rule keeps the registration, authorization, and -// redirect-following checks consistent. +// from them. Centralizing the rule keeps the authorization, token, device, and +// verification-URI checks consistent. func isSecureEndpointURL(u *url.URL) bool { if u.Hostname() == "" { return false @@ -674,152 +853,35 @@ func isSecureEndpointURL(u *url.URL) bool { if u.User != nil { return false } + // url.Parse requires a numeric port but does not range-check it, so + // https://host:70000/ parses cleanly yet is undialable and unlaunchable. + if port := u.Port(); port != "" { + if n, err := strconv.Atoi(port); err != nil || n < 1 || n > 65535 { + return false + } + } // IsLocalhost takes the host:port form and strips the port itself. return u.Scheme == "https" || (u.Scheme == "http" && hostutil.IsLocalhost(u.Host)) } -func (m *Manager) registerBC3Client(ctx context.Context, registrationEndpoint string, opts *LoginOptions) (*ClientCredentials, error) { - // The registration endpoint comes from the server-controlled discovery - // document. Restrict it to https (or http on loopback for local - // development) so a hostile discovery doc can't hand the DCR POST a - // file:// (or other) scheme that RequireSecureURL would let through. - // Mirrors buildAuthURL's scheme whitelist. - u, err := url.Parse(registrationEndpoint) +// requireSecureOAuthEndpoint parses and validates a server-controlled OAuth +// endpoint URL with isSecureEndpointURL, returning an auth-class error naming +// the endpoint when it fails. +func requireSecureOAuthEndpoint(name, endpoint string) error { + u, err := url.Parse(endpoint) if err != nil { - return nil, output.ErrAuth(fmt.Sprintf("invalid registration endpoint %q: %v", registrationEndpoint, err)) + return output.ErrAuth(fmt.Sprintf("invalid %s %q: %v", name, endpoint, err)) } if !isSecureEndpointURL(u) { - return nil, output.ErrAuth(fmt.Sprintf("invalid registration endpoint %q: must be an absolute https URL (or http on loopback)", registrationEndpoint)) - } - - customRedirect := opts.RedirectURI != defaultRedirectURI - regReq := map[string]any{ - "client_name": "basecamp-cli", - "client_uri": "https://github.com/basecamp/basecamp-cli", - "redirect_uris": []string{opts.RedirectURI}, - "grant_types": []string{"authorization_code"}, - "response_types": []string{"code"}, - "token_endpoint_auth_method": "none", - } - - body, err := json.Marshal(regReq) - if err != nil { - return nil, err - } - - req, err := http.NewRequestWithContext(ctx, "POST", registrationEndpoint, strings.NewReader(string(body))) - if err != nil { - return nil, err + return output.ErrAuth(fmt.Sprintf("invalid %s %q: must be an absolute https URL (or http on loopback)", name, endpoint)) } - req.Header.Set("Content-Type", "application/json") - - // Use a dedicated client with its own CheckRedirect guard. The DCR POST body - // carries only client metadata (no auth code or refresh token), so following a - // proxy-canonicalized 3xx redirect is safe — and necessary, since the manager's - // guarded client would silently fail first-time login on such redirects. But Go - // replays the POST body on 307/308, so re-validate EACH hop's target with the - // same scheme rule applied to the registration endpoint; otherwise a hostile - // server could 307 the body to a file:// or non-loopback http:// URL, escaping - // the whitelist that only covered the original endpoint. - checkRedirect := func(req *http.Request, via []*http.Request) error { - if !isSecureEndpointURL(req.URL) { - return output.ErrAuth(fmt.Sprintf("refusing DCR redirect to %q: must be an absolute https URL (or http on loopback)", req.URL.String())) - } - // Only 307/308 preserve the POST method and body. On 301/302/303 Go - // downgrades the upcoming request (req here) to a body-less GET, so the - // registration would silently arrive empty and first-time login would - // fail confusingly. Refuse instead of resending as GET. - if req.Method != http.MethodPost { - return output.ErrAuth(fmt.Sprintf("refusing DCR redirect to %q: redirect downgraded the registration POST to %s, dropping the request body; the endpoint must redirect with 307/308", req.URL.String(), req.Method)) - } - // A redirect loop is a deterministic endpoint misconfiguration, not a - // transient network failure: return an auth-class error so the - // errors.As unwrap below surfaces it directly instead of masking it - // as a retryable output.ErrNetwork. - if len(via) >= 10 { - return output.ErrAuth(fmt.Sprintf("registration endpoint redirect loop: stopped after 10 redirects at %q", req.URL.String())) - } - return nil - } - var dcrClient *http.Client - if m.httpClient != nil { - c := *m.httpClient // http.Client has no locks; value copy is safe - c.CheckRedirect = checkRedirect - dcrClient = &c - } else { - dcrClient = &http.Client{Timeout: 30 * time.Second, CheckRedirect: checkRedirect} - } - resp, err := dcrClient.Do(req) - if err != nil { - // A CheckRedirect rejection surfaces as a *url.Error wrapping our - // output.ErrAuth. Surface it directly rather than masking the security - // failure as a retryable network error. - var outErr *output.Error - if errors.As(err, &outErr) { - return nil, outErr - } - return nil, output.ErrNetwork(err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { - respBody, _ := io.ReadAll(io.LimitReader(resp.Body, 64<<10)) // 64 KB limit - return nil, output.ErrAPI(resp.StatusCode, fmt.Sprintf("DCR failed: %s", string(respBody))) - } - - var regResp struct { - ClientID string `json:"client_id"` - ClientSecret string `json:"client_secret,omitempty"` - } - if err := json.NewDecoder(io.LimitReader(resp.Body, 64<<10)).Decode(®Resp); err != nil { // 64 KB limit - return nil, err - } - - if regResp.ClientID == "" { - return nil, fmt.Errorf("no client_id in DCR response") - } - - creds := &ClientCredentials{ - ClientID: regResp.ClientID, - ClientSecret: regResp.ClientSecret, - } - - // Only persist DCR credentials when using the default redirect URI. - // Custom redirect URIs are session-only to prevent stale client.json - // entries that would fail on subsequent runs without the override. - if !customRedirect { - if err := m.saveBC3Client(creds); err != nil { - return nil, err - } - } - - return creds, nil -} - -func (m *Manager) saveBC3Client(creds *ClientCredentials) error { - configDir := config.GlobalConfigDir() - if err := os.MkdirAll(configDir, 0700); err != nil { - return err - } - - data, err := json.Marshal(creds) - if err != nil { - return err - } - - clientFile := configDir + "/client.json" - return os.WriteFile(clientFile, data, 0600) + return nil } -func (m *Manager) buildAuthURL(cfg *oauth.Config, oauthType, scope, state, codeChallenge, clientID string, opts *LoginOptions) (string, error) { - // Login asserts this before any side effects; kept as a defensive check - // for other callers. - if cfg.AuthorizationEndpoint == nil || *cfg.AuthorizationEndpoint == "" { - return "", output.ErrAuth("OAuth server does not advertise an authorization endpoint (authorization-code flow unsupported)") - } - u, err := url.Parse(*cfg.AuthorizationEndpoint) +func (m *Manager) buildAuthURL(authorizationEndpoint, state, clientID string, opts *LoginOptions) (string, error) { + u, err := url.Parse(authorizationEndpoint) if err != nil { - return "", output.ErrAuth(fmt.Sprintf("invalid authorization endpoint %q: %v", *cfg.AuthorizationEndpoint, err)) + return "", output.ErrAuth(fmt.Sprintf("invalid authorization endpoint %q: %v", authorizationEndpoint, err)) } // The authorization endpoint comes from the server-controlled discovery @@ -827,7 +889,7 @@ func (m *Manager) buildAuthURL(cfg *oauth.Config, oauthType, scope, state, codeC // open). Restrict it to https (or http on loopback for local development) // so a hostile discovery doc can't hand the OS a file:// (or other) URL. if !isSecureEndpointURL(u) { - return "", output.ErrAuth(fmt.Sprintf("invalid authorization endpoint %q: must be an absolute https URL (or http on loopback)", *cfg.AuthorizationEndpoint)) + return "", output.ErrAuth(fmt.Sprintf("invalid authorization endpoint %q: must be an absolute https URL (or http on loopback)", authorizationEndpoint)) } q := u.Query() @@ -835,33 +897,20 @@ func (m *Manager) buildAuthURL(cfg *oauth.Config, oauthType, scope, state, codeC q.Set("client_id", clientID) q.Set("redirect_uri", opts.RedirectURI) q.Set("state", state) - - if oauthType == "bc3" { - q.Set("code_challenge", codeChallenge) - q.Set("code_challenge_method", "S256") - if scope != "" { - q.Set("scope", scope) - } - } else { - q.Set("type", "web_server") - } + q.Set("type", "web_server") u.RawQuery = q.Encode() return u.String(), nil } -func (m *Manager) exchangeCode(ctx context.Context, cfg *oauth.Config, oauthType, code, codeVerifier string, clientCreds *ClientCredentials, opts *LoginOptions) (*Credentials, error) { +func (m *Manager) exchangeCode(ctx context.Context, cfg *oauth.Config, code string, clientCreds *ClientCredentials, opts *LoginOptions) (*Credentials, error) { // The token endpoint comes from the server-controlled discovery document // and receives the authorization code plus client credentials. The SDK's // RequireSecureEndpoint only checks scheme==https, which lets userinfo // (https://legit@evil.com/token) and empty-host forms through. Apply the - // same strict check used for the registration and authorization endpoints. - u, err := url.Parse(cfg.TokenEndpoint) - if err != nil { - return nil, output.ErrAuth(fmt.Sprintf("invalid token endpoint %q: %v", cfg.TokenEndpoint, err)) - } - if !isSecureEndpointURL(u) { - return nil, output.ErrAuth(fmt.Sprintf("invalid token endpoint %q: must be an absolute https URL (or http on loopback)", cfg.TokenEndpoint)) + // same strict check used for the other OAuth endpoints. + if err := requireSecureOAuthEndpoint("token endpoint", cfg.TokenEndpoint); err != nil { + return nil, err } exchanger := oauth.NewExchanger(m.httpClient) @@ -872,8 +921,7 @@ func (m *Manager) exchangeCode(ctx context.Context, cfg *oauth.Config, oauthType RedirectURI: opts.RedirectURI, ClientID: clientCreds.ClientID, ClientSecret: clientCreds.ClientSecret, - CodeVerifier: codeVerifier, - UseLegacyFormat: oauthType == "launchpad", + UseLegacyFormat: true, } token, err := exchanger.Exchange(ctx, req) @@ -999,9 +1047,9 @@ func (m *Manager) AuthorizationEndpoint(ctx context.Context) (string, error) { oauthType := m.GetOAuthType() switch oauthType { - case "bc3": + case "bc3", oauthTypeBC5: return config.NormalizeBaseURL(m.cfg.BaseURL) + "/authorization.json", nil - case "launchpad", "": + case oauthTypeLaunchpad, "": // "launchpad" = stored credentials; "" = no stored credentials and // no env token (shouldn't normally reach here since IsAuthenticated // would have caught it, but handle gracefully). @@ -1015,7 +1063,8 @@ func (m *Manager) AuthorizationEndpoint(ctx context.Context) (string, error) { } } -// GetOAuthType returns the OAuth type for the current credential key ("bc3" or "launchpad"). +// GetOAuthType returns the OAuth type for the current credential key +// ("bc5", "launchpad", or legacy "bc3"). func (m *Manager) GetOAuthType() string { credKey := m.credentialKey() creds, err := m.store.Load(credKey) diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 8e3556f3e..b745285cc 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -9,7 +9,6 @@ import ( "net/http/httptest" "net/url" "os" - "path/filepath" "strings" "sync" "sync/atomic" @@ -26,7 +25,7 @@ import ( // syncLogger is a thread-safe log collector for remote-mode login tests. // It captures all log messages under a mutex and signals authReady when it -// sees the auth URL (a line starting with "http" containing "/authorize"). +// sees the auth URL (a line starting with "http" containing "/authorization/new"). type syncLogger struct { mu sync.Mutex logs []string @@ -45,7 +44,7 @@ func (sl *syncLogger) log(msg string) { if !sl.signaled { trimmed := strings.TrimSpace(msg) - if strings.HasPrefix(trimmed, "http") && strings.Contains(trimmed, "/authorize") { + if strings.HasPrefix(trimmed, "http") && strings.Contains(trimmed, "/authorization/new") { sl.signaled = true sl.authReady <- trimmed } @@ -60,11 +59,6 @@ func (sl *syncLogger) snapshot() []string { return cp } -// strPtr returns a pointer to s, for oauth.Config optional endpoint fields. -func strPtr(s string) *string { - return &s -} - // newTestStore creates a file-backed credential store for testing. func newTestStore(t *testing.T, dir string) *Store { t.Helper() @@ -344,11 +338,13 @@ func TestCredentialsJSON(t *testing.T) { } func TestOAuthConfigJSON(t *testing.T) { + authz := "https://auth.example.com/authorize" + register := "https://auth.example.com/register" cfg := &oauth.Config{ Issuer: "https://issuer.example.com", - AuthorizationEndpoint: strPtr("https://auth.example.com/authorize"), + AuthorizationEndpoint: &authz, TokenEndpoint: "https://auth.example.com/token", - RegistrationEndpoint: strPtr("https://auth.example.com/register"), + RegistrationEndpoint: ®ister, ScopesSupported: []string{"read", "write"}, } @@ -365,22 +361,6 @@ func TestOAuthConfigJSON(t *testing.T) { assert.Len(t, loaded.ScopesSupported, 2, "ScopesSupported length mismatch") } -func TestClientCredentialsJSON(t *testing.T) { - creds := &ClientCredentials{ - ClientID: "client-id-123", - ClientSecret: "client-secret-456", - } - - data, err := json.Marshal(creds) - require.NoError(t, err, "Marshal failed") - - var loaded ClientCredentials - require.NoError(t, json.Unmarshal(data, &loaded), "Unmarshal failed") - - assert.Equal(t, creds.ClientID, loaded.ClientID) - assert.Equal(t, creds.ClientSecret, loaded.ClientSecret) -} - func TestUsingKeyring(t *testing.T) { tmpDir := t.TempDir() @@ -432,45 +412,11 @@ func TestDiscoverOAuth_PropagatesInsecureLaunchpadError(t *testing.T) { t.Setenv("BASECAMP_LAUNCHPAD_URL", "http://evil.example.com") noop := func(string) {} - _, _, err := m.discoverOAuth(context.Background(), noop) + _, err := m.discoverOAuth(context.Background(), noop) require.Error(t, err, "insecure launchpad URL error must propagate through discoverOAuth") assert.Contains(t, err.Error(), "BASECAMP_LAUNCHPAD_URL") } -// TestDiscoverOAuth_NormalizesTrailingSlash guards the issuer binding: the -// SDK binds the discovered issuer to the requested base URL code-point exact -// (RFC 8414), so a configured BaseURL with a trailing slash must be -// normalized before discovery — otherwise BC3 discovery mismatches the -// server's issuer and incorrectly falls back to Launchpad. -func TestDiscoverOAuth_NormalizesTrailingSlash(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/.well-known/oauth-authorization-server" { - http.NotFound(w, r) - return - } - w.Header().Set("Content-Type", "application/json") - base := "http://" + r.Host - fmt.Fprintf(w, `{ - "issuer": "%s", - "authorization_endpoint": "%s/authorize", - "token_endpoint": "%s/token" - }`, base, base, base) - })) - defer srv.Close() - - cfg := config.Default() - cfg.BaseURL = srv.URL + "/" - - m := &Manager{cfg: cfg, httpClient: srv.Client()} - - noop := func(string) {} - oauthCfg, oauthType, err := m.discoverOAuth(context.Background(), noop) - require.NoError(t, err) - assert.Equal(t, "bc3", oauthType, "trailing-slash BaseURL must not fall back to Launchpad") - require.NotNil(t, oauthCfg.AuthorizationEndpoint) - assert.Equal(t, srv.URL+"/authorize", *oauthCfg.AuthorizationEndpoint) -} - func TestResolveOAuthCallback(t *testing.T) { tests := []struct { name string @@ -622,12 +568,9 @@ func TestResolveClientCredentials(t *testing.T) { func TestBuildAuthURL_UsesResolvedRedirectURI(t *testing.T) { m := &Manager{cfg: config.Default(), httpClient: http.DefaultClient} - oauthCfg := &oauth.Config{ - AuthorizationEndpoint: strPtr("https://auth.example.com/authorize"), - } opts := &LoginOptions{RedirectURI: "http://localhost:9999/my-callback"} - authURL, err := m.buildAuthURL(oauthCfg, "launchpad", "", "state123", "", "client-id", opts) + authURL, err := m.buildAuthURL("https://auth.example.com/authorize", "state123", "client-id", opts) require.NoError(t, err) assert.Contains(t, authURL, "redirect_uri=http%3A%2F%2Flocalhost%3A9999%2Fmy-callback") } @@ -647,8 +590,7 @@ func TestBuildAuthURL_RejectsUnsafeScheme(t *testing.T) { } for _, endpoint := range accepted { t.Run("accepts "+endpoint, func(t *testing.T) { - oauthCfg := &oauth.Config{AuthorizationEndpoint: strPtr(endpoint)} - _, err := m.buildAuthURL(oauthCfg, "bc3", "read", "state", "challenge", "cid", opts) + _, err := m.buildAuthURL(endpoint, "state", "cid", opts) require.NoError(t, err) }) } @@ -666,8 +608,7 @@ func TestBuildAuthURL_RejectsUnsafeScheme(t *testing.T) { } for _, endpoint := range rejected { t.Run("rejects "+endpoint, func(t *testing.T) { - oauthCfg := &oauth.Config{AuthorizationEndpoint: strPtr(endpoint)} - _, err := m.buildAuthURL(oauthCfg, "bc3", "read", "state", "challenge", "cid", opts) + _, err := m.buildAuthURL(endpoint, "state", "cid", opts) require.Error(t, err) }) } @@ -688,8 +629,7 @@ func TestBuildAuthURL_RejectsMalformedEndpoint(t *testing.T) { } for _, endpoint := range malformed { t.Run("rejects "+endpoint, func(t *testing.T) { - oauthCfg := &oauth.Config{AuthorizationEndpoint: strPtr(endpoint)} - _, err := m.buildAuthURL(oauthCfg, "bc3", "read", "state", "challenge", "cid", opts) + _, err := m.buildAuthURL(endpoint, "state", "cid", opts) require.Error(t, err) assert.Contains(t, err.Error(), "invalid authorization endpoint") @@ -716,7 +656,7 @@ func TestExchangeCode_UsesResolvedRedirectURI(t *testing.T) { clientCreds := &ClientCredentials{ClientID: "cid", ClientSecret: "csecret"} opts := &LoginOptions{RedirectURI: "http://localhost:7777/cb"} - _, err := m.exchangeCode(context.Background(), oauthCfg, "launchpad", "code123", "", clientCreds, opts) + _, err := m.exchangeCode(context.Background(), oauthCfg, "code123", clientCreds, opts) require.NoError(t, err) // Body is URL-encoded form data assert.Contains(t, receivedBody, "redirect_uri=http%3A%2F%2Flocalhost%3A7777%2Fcb") @@ -750,7 +690,7 @@ func TestExchangeCode_RejectsUnsafeTokenEndpoint(t *testing.T) { transport := &recordingTransport{} m := &Manager{cfg: config.Default(), httpClient: &http.Client{Transport: transport}} oauthCfg := &oauth.Config{TokenEndpoint: endpoint} - _, err := m.exchangeCode(context.Background(), oauthCfg, "launchpad", "code123", "", clientCreds, opts) + _, err := m.exchangeCode(context.Background(), oauthCfg, "code123", clientCreds, opts) require.Error(t, err) assert.Contains(t, err.Error(), "invalid token endpoint") assert.False(t, transport.attempted.Load(), "must reject before any network POST") @@ -784,344 +724,6 @@ func TestRefreshLocked_RejectsUnsafeTokenEndpoint(t *testing.T) { } } -func TestRegisterBC3Client_UsesResolvedRedirectURI(t *testing.T) { - var receivedBody map[string]any - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - json.NewDecoder(r.Body).Decode(&receivedBody) - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"client_id":"dcr-id","client_secret":"dcr-secret"}`) - })) - defer srv.Close() - - tmpDir := t.TempDir() - t.Setenv("XDG_CONFIG_HOME", tmpDir) - - m := &Manager{ - cfg: config.Default(), - httpClient: srv.Client(), - store: newTestStore(t, tmpDir), - } - opts := &LoginOptions{RedirectURI: "http://localhost:7777/cb"} - - creds, err := m.registerBC3Client(context.Background(), srv.URL+"/register", opts) - require.NoError(t, err) - assert.Equal(t, "dcr-id", creds.ClientID) - - // Verify the redirect URI was sent in the DCR request - redirectURIs, ok := receivedBody["redirect_uris"].([]any) - require.True(t, ok) - assert.Equal(t, "http://localhost:7777/cb", redirectURIs[0]) -} - -func TestRegisterBC3Client_CustomRedirectNotPersisted(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"client_id":"dcr-id","client_secret":"dcr-secret"}`) - })) - defer srv.Close() - - tmpDir := t.TempDir() - // Override XDG_CONFIG_HOME so saveBC3Client would write to tmpDir (but shouldn't) - t.Setenv("XDG_CONFIG_HOME", tmpDir) - - m := &Manager{ - cfg: config.Default(), - httpClient: srv.Client(), - store: newTestStore(t, tmpDir), - } - opts := &LoginOptions{RedirectURI: "http://localhost:7777/cb"} - - // Custom redirect: should NOT persist - _, err := m.registerBC3Client(context.Background(), srv.URL+"/register", opts) - require.NoError(t, err) - - clientFile := filepath.Join(tmpDir, "basecamp", "client.json") - _, statErr := os.Stat(clientFile) - assert.True(t, os.IsNotExist(statErr), "client.json should not be written for custom redirect URI") -} - -func TestRegisterBC3Client_DefaultRedirectPersisted(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"client_id":"dcr-id","client_secret":"dcr-secret"}`) - })) - defer srv.Close() - - tmpDir := t.TempDir() - // Override XDG_CONFIG_HOME so saveBC3Client writes to tmpDir - t.Setenv("XDG_CONFIG_HOME", tmpDir) - - m := &Manager{ - cfg: config.Default(), - httpClient: srv.Client(), - store: newTestStore(t, tmpDir), - } - opts := &LoginOptions{RedirectURI: defaultRedirectURI} - - // Default redirect: should persist - _, err := m.registerBC3Client(context.Background(), srv.URL+"/register", opts) - require.NoError(t, err) - - clientFile := filepath.Join(tmpDir, "basecamp", "client.json") - _, statErr := os.Stat(clientFile) - assert.NoError(t, statErr, "client.json should be written for default redirect URI") -} - -// TestRegisterBC3Client_RejectsUnsafeScheme guards against a hostile discovery -// document handing the DCR POST a non-https registration endpoint (e.g. -// file://). https and http-on-loopback are accepted; everything else must error -// before any request is made. Mirrors buildAuthURL's scheme whitelist. -func TestRegisterBC3Client_RejectsUnsafeScheme(t *testing.T) { - m := &Manager{cfg: config.Default(), httpClient: http.DefaultClient} - opts := &LoginOptions{RedirectURI: defaultRedirectURI} - - rejected := []string{ - "file:///etc/passwd", - "http://evil.example.com/register", - "ftp://evil.example.com/register", - "javascript:alert(1)", - "data:text/html,foo", - "https:foo", // opaque form: right scheme, no host - "https://", // absolute form with empty host - "https://:3000/register", // non-empty Host (":3000") but empty hostname - "http://:8080/register", // port-only authority is not loopback - "https://evil.example@auth.example.com/register", // userinfo enables phishing display and Basic-auth synthesis - } - for _, endpoint := range rejected { - t.Run("rejects "+endpoint, func(t *testing.T) { - _, err := m.registerBC3Client(context.Background(), endpoint, opts) - require.Error(t, err) - assert.Contains(t, err.Error(), "registration endpoint") - }) - } -} - -// TestRegisterBC3Client_FollowsRedirect verifies the DCR POST follows a -// proxy-canonicalized 3xx on the registration endpoint (rather than silently -// failing under the manager's GET-only guard) when the redirect target stays -// within the secure-endpoint whitelist — here a loopback http:// hop. The DCR -// body carries only client metadata, so following such a redirect is safe. -func TestRegisterBC3Client_FollowsRedirect(t *testing.T) { - mux := http.NewServeMux() - mux.HandleFunc("/register", func(w http.ResponseWriter, r *http.Request) { - http.Redirect(w, r, "/register-canonical", http.StatusTemporaryRedirect) - }) - mux.HandleFunc("/register-canonical", func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"client_id":"dcr-id","client_secret":"dcr-secret"}`) - }) - srv := httptest.NewServer(mux) - defer srv.Close() - - tmpDir := t.TempDir() - t.Setenv("XDG_CONFIG_HOME", tmpDir) - - // Manager carries a guarded client (as appctx wires it) to prove the DCR - // path uses its own unguarded client rather than m.httpClient. - guarded := srv.Client() - guarded.CheckRedirect = func(_ *http.Request, via []*http.Request) error { - if len(via) > 0 && via[0].Method != http.MethodGet && via[0].Method != http.MethodHead { - return http.ErrUseLastResponse - } - return nil - } - m := &Manager{ - cfg: config.Default(), - httpClient: guarded, - store: newTestStore(t, tmpDir), - } - opts := &LoginOptions{RedirectURI: "http://localhost:7777/cb"} - - creds, err := m.registerBC3Client(context.Background(), srv.URL+"/register", opts) - require.NoError(t, err) - assert.Equal(t, "dcr-id", creds.ClientID) -} - -// TestRegisterBC3Client_FollowsHTTPSRedirect verifies an https redirect hop is -// followed: the scheme stays within the secure-endpoint whitelist, so the -// re-validation in CheckRedirect must not reject it. -func TestRegisterBC3Client_FollowsHTTPSRedirect(t *testing.T) { - mux := http.NewServeMux() - mux.HandleFunc("/register", func(w http.ResponseWriter, r *http.Request) { - http.Redirect(w, r, "/register-canonical", http.StatusTemporaryRedirect) - }) - mux.HandleFunc("/register-canonical", func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"client_id":"dcr-id","client_secret":"dcr-secret"}`) - }) - srv := httptest.NewTLSServer(mux) - defer srv.Close() - - tmpDir := t.TempDir() - t.Setenv("XDG_CONFIG_HOME", tmpDir) - - m := &Manager{ - cfg: config.Default(), - httpClient: srv.Client(), // trusts the test server's TLS cert - store: newTestStore(t, tmpDir), - } - opts := &LoginOptions{RedirectURI: "http://localhost:7777/cb"} - - creds, err := m.registerBC3Client(context.Background(), srv.URL+"/register", opts) - require.NoError(t, err) - assert.Equal(t, "dcr-id", creds.ClientID) -} - -// TestRegisterBC3Client_RejectsUnsafeRedirect guards against a hostile server -// 307/308-redirecting the DCR POST (body and all) to a scheme/host outside the -// secure-endpoint whitelist that was only enforced on the original endpoint. -// Each redirect hop must be re-validated, so file:// and non-loopback http:// -// targets are rejected before the body is replayed. -func TestRegisterBC3Client_RejectsUnsafeRedirect(t *testing.T) { - targets := map[string]string{ - "file scheme": "file:///etc/passwd", - "non-loopback http": "http://evil.example.com/register", - "other scheme": "ftp://evil.example.com/register", - } - for name, target := range targets { - t.Run(name, func(t *testing.T) { - mux := http.NewServeMux() - mux.HandleFunc("/register", func(w http.ResponseWriter, r *http.Request) { - http.Redirect(w, r, target, http.StatusTemporaryRedirect) - }) - srv := httptest.NewServer(mux) - defer srv.Close() - - tmpDir := t.TempDir() - t.Setenv("XDG_CONFIG_HOME", tmpDir) - - m := &Manager{ - cfg: config.Default(), - httpClient: srv.Client(), - store: newTestStore(t, tmpDir), - } - opts := &LoginOptions{RedirectURI: "http://localhost:7777/cb"} - - _, err := m.registerBC3Client(context.Background(), srv.URL+"/register", opts) - require.Error(t, err) - assert.Contains(t, err.Error(), "redirect") - - // The rejection is a security failure, not a transient network - // error: it must surface as an auth-class error (with its exit - // code and re-auth hint), not be masked as a retryable ErrNetwork. - var outErr *output.Error - require.ErrorAs(t, err, &outErr) - assert.Equal(t, output.CodeAuth, outErr.Code) - }) - } -} - -// TestRegisterBC3Client_RejectsMethodDowngradeRedirect guards against a -// 301/302/303 on the registration endpoint: Go follows those by downgrading -// the POST to a body-less GET, so the DCR registration would silently arrive -// empty at the target. The guard must reject the downgraded hop with an -// auth-class error, and the redirect target must never see the GET. -func TestRegisterBC3Client_RejectsMethodDowngradeRedirect(t *testing.T) { - for _, status := range []int{http.StatusMovedPermanently, http.StatusFound, http.StatusSeeOther} { - t.Run(http.StatusText(status), func(t *testing.T) { - var targetHits atomic.Int32 - mux := http.NewServeMux() - mux.HandleFunc("/register", func(w http.ResponseWriter, r *http.Request) { - http.Redirect(w, r, "/register-canonical", status) - }) - mux.HandleFunc("/register-canonical", func(w http.ResponseWriter, r *http.Request) { - targetHits.Add(1) - }) - srv := httptest.NewServer(mux) - defer srv.Close() - - tmpDir := t.TempDir() - t.Setenv("XDG_CONFIG_HOME", tmpDir) - - m := &Manager{ - cfg: config.Default(), - httpClient: srv.Client(), - store: newTestStore(t, tmpDir), - } - opts := &LoginOptions{RedirectURI: "http://localhost:7777/cb"} - - _, err := m.registerBC3Client(context.Background(), srv.URL+"/register", opts) - require.Error(t, err) - assert.Contains(t, err.Error(), "downgraded the registration POST") - - // Security failure, not a transient network error: must surface as - // an auth-class error, same as the unsafe-redirect rejections. - var outErr *output.Error - require.ErrorAs(t, err, &outErr) - assert.Equal(t, output.CodeAuth, outErr.Code) - - assert.Zero(t, targetHits.Load(), "redirect target must never receive the body-less GET") - }) - } -} - -// TestRegisterBC3Client_StopsRedirectLoop guards against a looping endpoint that -// keeps issuing same-scheme (loopback http) redirects: each hop passes the -// secure-endpoint re-validation, so without a hop cap the DCR client would spin -// until the 30s timeout. The cap must fail fast after 10 redirects. -func TestRegisterBC3Client_StopsRedirectLoop(t *testing.T) { - var hops atomic.Int32 - mux := http.NewServeMux() - mux.HandleFunc("/register", func(w http.ResponseWriter, r *http.Request) { - hops.Add(1) - http.Redirect(w, r, "/register", http.StatusTemporaryRedirect) - }) - srv := httptest.NewServer(mux) - defer srv.Close() - - tmpDir := t.TempDir() - t.Setenv("XDG_CONFIG_HOME", tmpDir) - - m := &Manager{ - cfg: config.Default(), - httpClient: srv.Client(), - store: newTestStore(t, tmpDir), - } - opts := &LoginOptions{RedirectURI: "http://localhost:7777/cb"} - - _, err := m.registerBC3Client(context.Background(), srv.URL+"/register", opts) - require.Error(t, err, "redirect loop must fail rather than hang") - assert.Contains(t, err.Error(), "stopped after 10 redirects") - assert.LessOrEqual(t, hops.Load(), int32(11), "client must give up around the 10-redirect cap") - - // A redirect loop is a deterministic endpoint misconfiguration, not a - // transient network failure: it must surface as an auth-class error, not - // be masked as a retryable ErrNetwork. - var outErr *output.Error - require.ErrorAs(t, err, &outErr) - assert.Equal(t, output.CodeAuth, outErr.Code) -} - -func TestLoadClientCredentials_BC3_CustomRedirect_SkipsStoredClient(t *testing.T) { - // DCR server - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"client_id":"dcr-fresh","client_secret":"dcr-secret"}`) - })) - defer srv.Close() - - tmpDir := t.TempDir() - t.Setenv("XDG_CONFIG_HOME", tmpDir) - - m := &Manager{ - cfg: config.Default(), - httpClient: srv.Client(), - store: newTestStore(t, tmpDir), - } - - // Pre-populate client.json - storedCreds := &ClientCredentials{ClientID: "stored-id", ClientSecret: "stored-secret"} - require.NoError(t, m.saveBC3Client(storedCreds)) - - oauthCfg := &oauth.Config{RegistrationEndpoint: strPtr(srv.URL + "/register")} - - // Custom redirect: should skip stored client and do DCR - opts := &LoginOptions{RedirectURI: "http://localhost:7777/cb"} - creds, err := m.loadClientCredentials(context.Background(), oauthCfg, "bc3", opts) - require.NoError(t, err) - assert.Equal(t, "dcr-fresh", creds.ClientID, "should use DCR result, not stored client") -} - func TestParseCallbackURL(t *testing.T) { const state = "test-state-123" @@ -1236,22 +838,11 @@ func TestLoginRemoteAndLocalMutuallyExclusive(t *testing.T) { } func TestLoginRemoteMode(t *testing.T) { - // Set up httptest server that handles discovery + token exchange + // No protected-resource metadata (404) => Launchpad fallback, pointed + // at this server via BASECAMP_LAUNCHPAD_URL. srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { - case "/.well-known/oauth-authorization-server": - w.Header().Set("Content-Type", "application/json") - base := "http://" + r.Host - fmt.Fprintf(w, `{ - "issuer": "%s", - "authorization_endpoint": "%s/authorize", - "token_endpoint": "%s/token", - "registration_endpoint": "%s/register" - }`, base, base, base, base) - case "/register": - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"client_id":"test-client","client_secret":"test-secret"}`) - case "/token": + case "/authorization/token": w.Header().Set("Content-Type", "application/json") fmt.Fprint(w, `{"access_token":"remote-tok","token_type":"bearer","refresh_token":"remote-refresh"}`) default: @@ -1262,6 +853,7 @@ func TestLoginRemoteMode(t *testing.T) { tmpDir := t.TempDir() t.Setenv("XDG_CONFIG_HOME", tmpDir) + t.Setenv("BASECAMP_LAUNCHPAD_URL", srv.URL) cfg := &config.Config{BaseURL: srv.URL} m := NewManager(cfg, srv.Client()) @@ -1327,19 +919,7 @@ func TestLoginRemoteMode(t *testing.T) { func TestLoginRemoteMode_PromptWording(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { - case "/.well-known/oauth-authorization-server": - w.Header().Set("Content-Type", "application/json") - base := "http://" + r.Host - fmt.Fprintf(w, `{ - "issuer": "%s", - "authorization_endpoint": "%s/authorize", - "token_endpoint": "%s/token", - "registration_endpoint": "%s/register" - }`, base, base, base, base) - case "/register": - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"client_id":"test-client","client_secret":"test-secret"}`) - case "/token": + case "/authorization/token": w.Header().Set("Content-Type", "application/json") fmt.Fprint(w, `{"access_token":"tok","token_type":"bearer","refresh_token":"ref"}`) default: @@ -1350,6 +930,7 @@ func TestLoginRemoteMode_PromptWording(t *testing.T) { tmpDir := t.TempDir() t.Setenv("XDG_CONFIG_HOME", tmpDir) + t.Setenv("BASECAMP_LAUNCHPAD_URL", srv.URL) cfg := &config.Config{BaseURL: srv.URL} m := NewManager(cfg, srv.Client()) @@ -1403,27 +984,13 @@ func TestLoginRemoteMode_PromptWording(t *testing.T) { func TestLoginRemoteMode_StateMismatch(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/.well-known/oauth-authorization-server": - w.Header().Set("Content-Type", "application/json") - base := "http://" + r.Host - fmt.Fprintf(w, `{ - "issuer": "%s", - "authorization_endpoint": "%s/authorize", - "token_endpoint": "%s/token", - "registration_endpoint": "%s/register" - }`, base, base, base, base) - case "/register": - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"client_id":"test-client"}`) - default: - http.NotFound(w, r) - } + http.NotFound(w, r) })) defer srv.Close() tmpDir := t.TempDir() t.Setenv("XDG_CONFIG_HOME", tmpDir) + t.Setenv("BASECAMP_LAUNCHPAD_URL", srv.URL) cfg := &config.Config{BaseURL: srv.URL} m := NewManager(cfg, srv.Client()) @@ -1465,27 +1032,13 @@ func TestLoginRemoteMode_StateMismatch(t *testing.T) { func TestLoginRemoteMode_EmptyInput(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/.well-known/oauth-authorization-server": - w.Header().Set("Content-Type", "application/json") - base := "http://" + r.Host - fmt.Fprintf(w, `{ - "issuer": "%s", - "authorization_endpoint": "%s/authorize", - "token_endpoint": "%s/token", - "registration_endpoint": "%s/register" - }`, base, base, base, base) - case "/register": - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"client_id":"test-client"}`) - default: - http.NotFound(w, r) - } + http.NotFound(w, r) })) defer srv.Close() tmpDir := t.TempDir() t.Setenv("XDG_CONFIG_HOME", tmpDir) + t.Setenv("BASECAMP_LAUNCHPAD_URL", srv.URL) cfg := &config.Config{BaseURL: srv.URL} m := NewManager(cfg, srv.Client()) @@ -1526,19 +1079,7 @@ func TestLoginRemoteMode_EmptyInput(t *testing.T) { func TestLoginRemoteMode_CustomRedirectURI(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { - case "/.well-known/oauth-authorization-server": - w.Header().Set("Content-Type", "application/json") - base := "http://" + r.Host - fmt.Fprintf(w, `{ - "issuer": "%s", - "authorization_endpoint": "%s/authorize", - "token_endpoint": "%s/token", - "registration_endpoint": "%s/register" - }`, base, base, base, base) - case "/register": - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"client_id":"test-client"}`) - case "/token": + case "/authorization/token": w.Header().Set("Content-Type", "application/json") fmt.Fprint(w, `{"access_token":"tok","token_type":"bearer"}`) default: @@ -1549,6 +1090,7 @@ func TestLoginRemoteMode_CustomRedirectURI(t *testing.T) { tmpDir := t.TempDir() t.Setenv("XDG_CONFIG_HOME", tmpDir) + t.Setenv("BASECAMP_LAUNCHPAD_URL", srv.URL) cfg := &config.Config{BaseURL: srv.URL} m := NewManager(cfg, srv.Client()) @@ -1743,82 +1285,6 @@ func TestLoginLaunchpadClearsScope(t *testing.T) { assert.Equal(t, "", creds.Scope, "stored scope should be empty for Launchpad") } -func TestLoginBC3DefaultsToRead(t *testing.T) { - // BC3 mock server with successful discovery - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/.well-known/oauth-authorization-server": - w.Header().Set("Content-Type", "application/json") - base := "http://" + r.Host - fmt.Fprintf(w, `{ - "issuer": "%s", - "authorization_endpoint": "%s/authorize", - "token_endpoint": "%s/token", - "registration_endpoint": "%s/register" - }`, base, base, base, base) - case "/register": - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"client_id":"test-client"}`) - case "/token": - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"access_token":"bc3-tok","token_type":"bearer","refresh_token":"bc3-ref"}`) - default: - http.NotFound(w, r) - } - })) - defer srv.Close() - - tmpDir := t.TempDir() - t.Setenv("XDG_CONFIG_HOME", tmpDir) - - cfg := &config.Config{BaseURL: srv.URL} - m := NewManager(cfg, srv.Client()) - m.store = newTestStore(t, tmpDir) - - sl := newSyncLogger() - pr, pw := io.Pipe() - defer pr.Close() - - type loginOut struct { - result *LoginResult - err error - } - ch := make(chan loginOut, 1) - go func() { - result, err := m.Login(context.Background(), LoginOptions{ - // No scope specified — should default to "read" for BC3 - Remote: true, - Logger: sl.log, - InputReader: pr, - }) - ch <- loginOut{result, err} - }() - - var authURL string - select { - case authURL = <-sl.authReady: - case <-time.After(5 * time.Second): - t.Fatal("timed out waiting for auth URL") - } - - u, err := url.Parse(authURL) - require.NoError(t, err) - state := u.Query().Get("state") - - callbackURL := fmt.Sprintf("http://127.0.0.1:8976/callback?code=test-code&state=%s\n", state) - _, _ = pw.Write([]byte(callbackURL)) - pw.Close() - - out := <-ch - require.NoError(t, out.err) - assert.Equal(t, "read", out.result.Scope, "BC3 should default to read scope") - assert.Equal(t, "bc3", out.result.OAuthType) - - creds, err := m.store.Load(srv.URL) - require.NoError(t, err) - assert.Equal(t, "read", creds.Scope, "stored scope should be 'read' for BC3 default") -} - func TestRefreshLocked_LaunchpadSendsClientID(t *testing.T) { t.Setenv("BASECAMP_OAUTH_CLIENT_ID", "") t.Setenv("BASECAMP_OAUTH_CLIENT_SECRET", "") @@ -1936,81 +1402,6 @@ func TestRefreshLocked_GuardBlocksCredentialPOSTRedirect(t *testing.T) { } } -func TestRefreshLocked_BC3SendsClientID(t *testing.T) { - var mu sync.Mutex - var capturedBody string - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - body, _ := io.ReadAll(r.Body) - mu.Lock() - capturedBody = string(body) - mu.Unlock() - w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"access_token":"new-token","refresh_token":"new-refresh","expires_in":3600}`) - })) - defer srv.Close() - - tmpDir := t.TempDir() - t.Setenv("XDG_CONFIG_HOME", tmpDir) - - cfg := config.Default() - cfg.BaseURL = srv.URL - - m := &Manager{ - cfg: cfg, - httpClient: srv.Client(), - store: newTestStore(t, tmpDir), - } - - // Pre-populate client.json - require.NoError(t, m.saveBC3Client(&ClientCredentials{ - ClientID: "bc3-client-id", - ClientSecret: "bc3-client-secret", - })) - - creds := &Credentials{ - AccessToken: "old-token", - RefreshToken: "old-refresh", - OAuthType: "bc3", - TokenEndpoint: srv.URL + "/token", - ExpiresAt: time.Now().Add(-1 * time.Hour).Unix(), - } - require.NoError(t, m.store.Save(srv.URL, creds)) - - err := m.refreshLocked(context.Background(), srv.URL, creds) - require.NoError(t, err) - - mu.Lock() - body := capturedBody - mu.Unlock() - assert.Contains(t, body, "client_id=bc3-client-id") - assert.Contains(t, body, "client_secret=bc3-client-secret") -} - -func TestRefreshLocked_BC3WithoutClientJSON(t *testing.T) { - tmpDir := t.TempDir() - t.Setenv("XDG_CONFIG_HOME", tmpDir) - - cfg := config.Default() - m := &Manager{ - cfg: cfg, - httpClient: http.DefaultClient, - store: newTestStore(t, tmpDir), - } - - creds := &Credentials{ - AccessToken: "old-token", - RefreshToken: "old-refresh", - OAuthType: "bc3", - TokenEndpoint: "https://example.com/token", - ExpiresAt: time.Now().Add(-1 * time.Hour).Unix(), - } - - err := m.refreshLocked(context.Background(), "test", creds) - require.Error(t, err) - assert.Contains(t, err.Error(), "Cannot load BC3 client credentials") - assert.Contains(t, err.Error(), "custom-redirect") -} - func TestRefreshLocked_ClearsExpiresAtWhenServerOmits(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") @@ -2094,54 +1485,6 @@ func TestRefreshLocked_EmptyOAuthTypeDefaultsToLaunchpad(t *testing.T) { assert.Contains(t, body, "client_id="+launchpadClientID) } -// TestLoginDeviceOnlyServerFailsBeforeRegistration asserts the -// authorization-code capability check fires immediately after discovery: a -// device-only server (no authorization_endpoint) fails Login with an auth -// error before Dynamic Client Registration is attempted. -func TestLoginDeviceOnlyServerFailsBeforeRegistration(t *testing.T) { - var mu sync.Mutex - var requests []string - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - mu.Lock() - requests = append(requests, r.URL.Path) - mu.Unlock() - switch r.URL.Path { - case "/.well-known/oauth-authorization-server": - w.Header().Set("Content-Type", "application/json") - base := "http://" + r.Host - fmt.Fprintf(w, `{ - "issuer": "%s", - "token_endpoint": "%s/token", - "device_authorization_endpoint": "%s/device", - "registration_endpoint": "%s/register", - "grant_types_supported": ["urn:ietf:params:oauth:grant-type:device_code"] - }`, base, base, base, base) - default: - http.NotFound(w, r) - } - })) - defer srv.Close() - - tmpDir := t.TempDir() - t.Setenv("XDG_CONFIG_HOME", tmpDir) - - cfg := &config.Config{BaseURL: srv.URL} - m := NewManager(cfg, srv.Client()) - m.store = newTestStore(t, tmpDir) - - _, err := m.Login(context.Background(), LoginOptions{Logger: func(string) {}}) - require.Error(t, err) - assert.Contains(t, err.Error(), "does not advertise an authorization endpoint") - - var outErr *output.Error - require.ErrorAs(t, err, &outErr) - assert.Equal(t, output.CodeAuth, outErr.Code) - - mu.Lock() - defer mu.Unlock() - assert.NotContains(t, requests, "/register", "capability check must fire before Dynamic Client Registration") -} - func TestLoginRejectsInvalidScope(t *testing.T) { tmpDir := t.TempDir() cfg := &config.Config{BaseURL: "https://3.basecampapi.com"} diff --git a/internal/auth/device_test.go b/internal/auth/device_test.go new file mode 100644 index 000000000..93be99128 --- /dev/null +++ b/internal/auth/device_test.go @@ -0,0 +1,883 @@ +package auth + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "testing" + "time" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp/oauth" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/config" + "github.com/basecamp/basecamp-cli/internal/output" +) + +// deviceAS is a mock BC5 authorization server: RFC 8414 metadata advertising +// the device grant, a device-authorization endpoint, and a token endpoint. +// Response hooks are overridable per test; every form POST is recorded. +type deviceAS struct { + srv *httptest.Server + + mu sync.Mutex + deviceForms []url.Values + tokenForms []url.Values + + // metadata renders the AS metadata JSON. Defaults to a device-capable doc + // with issuer = the server's own origin. + metadata func() string + // deviceAuth renders the device-authorization response. + deviceAuth func() (status int, body string) + // token renders the nth (0-based) token poll response. + token func(call int) (status int, body string) +} + +func startDeviceAS(t *testing.T) *deviceAS { + t.Helper() + as := &deviceAS{} + + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/oauth-authorization-server", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, as.metadata()) + }) + mux.HandleFunc("/oauth/device", func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + as.mu.Lock() + as.deviceForms = append(as.deviceForms, r.PostForm) + as.mu.Unlock() + status, body := as.deviceAuth() + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + fmt.Fprint(w, body) + }) + mux.HandleFunc("/oauth/token", func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + as.mu.Lock() + call := len(as.tokenForms) + as.tokenForms = append(as.tokenForms, r.PostForm) + as.mu.Unlock() + status, body := as.token(call) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + fmt.Fprint(w, body) + }) + + as.srv = httptest.NewServer(mux) + t.Cleanup(as.srv.Close) + + as.metadata = func() string { + return fmt.Sprintf(`{ + "issuer": %q, + "token_endpoint": %q, + "device_authorization_endpoint": %q, + "grant_types_supported": ["urn:ietf:params:oauth:grant-type:device_code", "refresh_token"] + }`, as.srv.URL, as.srv.URL+"/oauth/token", as.srv.URL+"/oauth/device") + } + as.deviceAuth = func() (int, string) { + return http.StatusOK, fmt.Sprintf( + `{"device_code":"dev-code-1","user_code":"ABCD-EFGH","verification_uri":%q,"verification_uri_complete":%q,"expires_in":600,"interval":1}`, + as.srv.URL+"/verify", as.srv.URL+"/verify?user_code=ABCD-EFGH") + } + as.token = func(int) (int, string) { + return http.StatusOK, `{"access_token":"dev-tok","refresh_token":"dev-ref","token_type":"bearer","expires_in":3600,"scope":"read"}` + } + return as +} + +func (as *deviceAS) deviceCalls() []url.Values { + as.mu.Lock() + defer as.mu.Unlock() + return append([]url.Values(nil), as.deviceForms...) +} + +func (as *deviceAS) tokenCalls() []url.Values { + as.mu.Lock() + defer as.mu.Unlock() + return append([]url.Values(nil), as.tokenForms...) +} + +// startResourceServer starts a mock protected resource advertising the given +// authorization servers via RFC 9728 metadata. +func startResourceServer(t *testing.T, asURLs ...string) *httptest.Server { + t.Helper() + var srv *httptest.Server + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/oauth-protected-resource", func(w http.ResponseWriter, _ *http.Request) { + servers, err := json.Marshal(asURLs) + require.NoError(t, err) + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"resource": %q, "authorization_servers": %s}`, srv.URL, servers) + }) + srv = httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +// countingServer records how many requests it receives and 404s all of them. +func countingServer(t *testing.T) (*httptest.Server, *int) { + t.Helper() + var mu sync.Mutex + count := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + count++ + mu.Unlock() + http.NotFound(w, r) + })) + t.Cleanup(srv.Close) + return srv, &count +} + +func newDeviceTestManager(t *testing.T, baseURL string) *Manager { + t.Helper() + tmpDir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", tmpDir) + // Pin SSH auto-detection off so LoginOptions.defaults() is deterministic. + t.Setenv("SSH_CONNECTION", "") + t.Setenv("SSH_CLIENT", "") + t.Setenv("SSH_TTY", "") + cfg := &config.Config{BaseURL: baseURL} + m := NewManager(cfg, http.DefaultClient) + m.store = newTestStore(t, tmpDir) + return m +} + +// collectLogger is a mutex-guarded log sink for device-flow tests. +type collectLogger struct { + mu sync.Mutex + logs []string +} + +func (c *collectLogger) log(msg string) { + c.mu.Lock() + defer c.mu.Unlock() + c.logs = append(c.logs, msg) +} + +func (c *collectLogger) joined() string { + c.mu.Lock() + defer c.mu.Unlock() + return strings.Join(c.logs, "\n") +} + +// instantSleep makes the SDK polling loop tick without real delays. +func instantSleep() oauth.DeviceOption { + return oauth.WithDeviceSleep(func(context.Context, time.Duration) error { return nil }) +} + +func TestLoginDevice_HappyPath(t *testing.T) { + as := startDeviceAS(t) + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + cl := &collectLogger{} + var launched []string + result, err := m.Login(context.Background(), LoginOptions{ + Logger: cl.log, + BrowserLauncher: func(u string) error { launched = append(launched, u); return nil }, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.NoError(t, err) + + assert.Equal(t, &LoginResult{OAuthType: "bc5", Scope: "read"}, result) + + // The public client authenticates without a secret. + deviceCalls := as.deviceCalls() + require.Len(t, deviceCalls, 1) + assert.Equal(t, "basecamp-cli", deviceCalls[0].Get("client_id")) + _, hasSecret := deviceCalls[0]["client_secret"] + assert.False(t, hasSecret) + + tokenCalls := as.tokenCalls() + require.NotEmpty(t, tokenCalls) + assert.Equal(t, "urn:ietf:params:oauth:grant-type:device_code", tokenCalls[0].Get("grant_type")) + assert.Equal(t, "dev-code-1", tokenCalls[0].Get("device_code")) + assert.Equal(t, "basecamp-cli", tokenCalls[0].Get("client_id")) + + // Browser launched with the validated code-embedding URI. + require.Len(t, launched, 1) + assert.Equal(t, as.srv.URL+"/verify?user_code=ABCD-EFGH", launched[0]) + + // Code and URI shown to the user. + logs := cl.joined() + assert.Contains(t, logs, "ABCD-EFGH") + assert.Contains(t, logs, as.srv.URL+"/verify?user_code=ABCD-EFGH") + assert.Contains(t, logs, "Waiting for approval") + assert.Contains(t, logs, "(device flow)") + + // Stored credentials carry the bc5 identity. + creds, err := m.store.Load(config.NormalizeBaseURL(resource.URL)) + require.NoError(t, err) + assert.Equal(t, "dev-tok", creds.AccessToken) + assert.Equal(t, "dev-ref", creds.RefreshToken) + assert.Equal(t, "bc5", creds.OAuthType) + assert.Equal(t, as.srv.URL+"/oauth/token", creds.TokenEndpoint) + assert.Equal(t, "read", creds.Scope) + assert.Positive(t, creds.ExpiresAt) +} + +// TestLoginDevice_FlagMatrix covers the browser column of the flag matrix at +// the auth layer: Remote (what --remote/--device-code map to) and NoBrowser +// print the code and URI without launching; the default launches. +func TestLoginDevice_FlagMatrix(t *testing.T) { + for name, opts := range map[string]LoginOptions{ + "remote": {Remote: true}, + "no-browser": {NoBrowser: true}, + } { + t.Run(name+" does not launch", func(t *testing.T) { + as := startDeviceAS(t) + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + cl := &collectLogger{} + launched := 0 + opts.Logger = cl.log + opts.BrowserLauncher = func(string) error { launched++; return nil } + opts.deviceOptions = []oauth.DeviceOption{instantSleep()} + + _, err := m.Login(context.Background(), opts) + require.NoError(t, err) + + assert.Zero(t, launched, "headless mode must not launch a browser") + logs := cl.joined() + assert.Contains(t, logs, "ABCD-EFGH") + assert.Contains(t, logs, as.srv.URL+"/verify?user_code=ABCD-EFGH") + }) + } + + t.Run("default launches", func(t *testing.T) { + as := startDeviceAS(t) + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + launched := 0 + _, err := m.Login(context.Background(), LoginOptions{ + BrowserLauncher: func(string) error { launched++; return nil }, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.NoError(t, err) + assert.Equal(t, 1, launched) + }) +} + +func TestLoginDevice_InvalidURICompleteFallsBackToPlainURI(t *testing.T) { + for _, evil := range []string{"file:///etc/passwd", "https://user@evil.example/verify", "https://evil.example:70000/verify"} { + t.Run(evil, func(t *testing.T) { + as := startDeviceAS(t) + as.deviceAuth = func() (int, string) { + return http.StatusOK, fmt.Sprintf( + `{"device_code":"dc","user_code":"ABCD-EFGH","verification_uri":%q,"verification_uri_complete":%q,"expires_in":600,"interval":1}`, + as.srv.URL+"/verify", evil) + } + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + cl := &collectLogger{} + var launched []string + _, err := m.Login(context.Background(), LoginOptions{ + Logger: cl.log, + BrowserLauncher: func(u string) error { launched = append(launched, u); return nil }, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.NoError(t, err) + + require.Len(t, launched, 1, "must fall back to the plain verification URI") + assert.Equal(t, as.srv.URL+"/verify", launched[0]) + assert.NotContains(t, cl.joined(), evil, "invalid URI must never be shown as a target") + }) + } +} + +func TestLoginDevice_MalformedDisplayDataAbortsBeforePolling(t *testing.T) { + cases := map[string]string{ + "both URIs invalid": fmt.Sprintf( + `{"device_code":"dc","user_code":"ABCD-EFGH","verification_uri":%q,"verification_uri_complete":%q,"expires_in":600,"interval":1}`, + "file:///etc/passwd", "javascript:alert(1)"), + "user code sanitizes to empty": `{"device_code":"dc","user_code":"\u001b[31m","verification_uri":"URI","expires_in":600,"interval":1}`, + "user code is whitespace only": `{"device_code":"dc","user_code":" ","verification_uri":"URI","expires_in":600,"interval":1}`, + } + + for name, body := range cases { + t.Run(name, func(t *testing.T) { + as := startDeviceAS(t) + resolved := strings.Replace(body, `"URI"`, fmt.Sprintf("%q", as.srv.URL+"/verify"), 1) + as.deviceAuth = func() (int, string) { return http.StatusOK, resolved } + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + launched := 0 + _, err := m.Login(context.Background(), LoginOptions{ + Logger: func(string) {}, + BrowserLauncher: func(string) error { launched++; return nil }, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "malformed device authorization") + + var outErr *output.Error + require.ErrorAs(t, err, &outErr) + assert.Equal(t, output.CodeAPI, outErr.Code) + + assert.Zero(t, launched, "nothing may be launched on malformed display data") + assert.Empty(t, as.tokenCalls(), "the abort must land before any token poll") + }) + } +} + +func TestLoginDevice_SanitizesDisplayedCodeAndURI(t *testing.T) { + as := startDeviceAS(t) + // OSC hyperlink + CSI + CRLF smuggled into the user code. + as.deviceAuth = func() (int, string) { + return http.StatusOK, fmt.Sprintf( + `{"device_code":"dc","user_code":"AB\u001b]8;;https://evil.example\u0007CD\r\nEF\u001b[31m","verification_uri":%q,"expires_in":600,"interval":1}`, + as.srv.URL+"/verify") + } + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + cl := &collectLogger{} + var launched []string + _, err := m.Login(context.Background(), LoginOptions{ + Logger: cl.log, + BrowserLauncher: func(u string) error { launched = append(launched, u); return nil }, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.NoError(t, err) + + logs := cl.joined() + assert.Contains(t, logs, "ABCD EF", "controls stripped, CRLF collapsed to a space") + assert.NotContains(t, logs, "\x1b", "no escape sequences may reach the logger") + assert.NotContains(t, logs, "\r") + assert.NotContains(t, logs, "evil.example") + + // The raw validated URL still goes to the launcher (no URIComplete here, + // so the plain verification URI is the target). + require.Len(t, launched, 1) + assert.Equal(t, as.srv.URL+"/verify", launched[0]) +} + +func TestLoginDevice_BrowserLaunchFailureContinues(t *testing.T) { + as := startDeviceAS(t) + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + cl := &collectLogger{} + result, err := m.Login(context.Background(), LoginOptions{ + Logger: cl.log, + BrowserLauncher: func(string) error { return fmt.Errorf("no display") }, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.NoError(t, err, "launch failure must not abort the flow") + assert.Equal(t, "bc5", result.OAuthType) + assert.Contains(t, cl.joined(), "Couldn't open browser") +} + +func TestLoginDevice_ScopeWiring(t *testing.T) { + t.Run("explicit scope sent and used as fallback", func(t *testing.T) { + as := startDeviceAS(t) + as.token = func(int) (int, string) { + // No scope in the token response — opts.Scope is the fallback. + return http.StatusOK, `{"access_token":"tok","refresh_token":"ref","token_type":"bearer"}` + } + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + result, err := m.Login(context.Background(), LoginOptions{ + Scope: "full", + Remote: true, + Logger: func(string) {}, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.NoError(t, err) + + calls := as.deviceCalls() + require.Len(t, calls, 1) + assert.Equal(t, "full", calls[0].Get("scope")) + assert.Equal(t, "full", result.Scope) + }) + + t.Run("unset scope omitted and defaults to read", func(t *testing.T) { + as := startDeviceAS(t) + as.token = func(int) (int, string) { + return http.StatusOK, `{"access_token":"tok","refresh_token":"ref","token_type":"bearer"}` + } + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + result, err := m.Login(context.Background(), LoginOptions{ + Remote: true, + Logger: func(string) {}, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.NoError(t, err) + + calls := as.deviceCalls() + require.Len(t, calls, 1) + _, hasScope := calls[0]["scope"] + assert.False(t, hasScope, "no scope requested means no scope parameter") + assert.Equal(t, "read", result.Scope) + }) + + t.Run("server-granted scope wins", func(t *testing.T) { + as := startDeviceAS(t) + as.token = func(int) (int, string) { + return http.StatusOK, `{"access_token":"tok","refresh_token":"ref","token_type":"bearer","scope":"read"}` + } + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + result, err := m.Login(context.Background(), LoginOptions{ + Scope: "full", + Remote: true, + Logger: func(string) {}, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.NoError(t, err) + assert.Equal(t, "read", result.Scope, "the server's granted scope is authoritative") + + creds, err := m.store.Load(config.NormalizeBaseURL(resource.URL)) + require.NoError(t, err) + assert.Equal(t, "read", creds.Scope) + }) +} + +func TestDiscoverOAuth_ResourceDiscoveryFailedFallsBackWithWarning(t *testing.T) { + // No protected-resource metadata at all → resource_discovery_failed. + srv, _ := countingServer(t) + m := newDeviceTestManager(t, srv.URL) + + cl := &collectLogger{} + disc, err := m.discoverOAuth(context.Background(), cl.log) + require.NoError(t, err) + + assert.Equal(t, "launchpad", disc.oauthType) + require.NotNil(t, disc.config.AuthorizationEndpoint) + assert.Equal(t, "https://launchpad.37signals.com/authorization/new", *disc.config.AuthorizationEndpoint) + assert.Equal(t, "https://launchpad.37signals.com/authorization/token", disc.config.TokenEndpoint) + assert.Contains(t, cl.joined(), "warning: OAuth discovery failed") +} + +func TestDiscoverOAuth_NoASAdvertisedFallsBackQuietly(t *testing.T) { + // Valid resource metadata that advertises no non-Launchpad issuer. + resource := startResourceServer(t, "https://launchpad.37signals.com") + m := newDeviceTestManager(t, resource.URL) + + cl := &collectLogger{} + disc, err := m.discoverOAuth(context.Background(), cl.log) + require.NoError(t, err) + + assert.Equal(t, "launchpad", disc.oauthType) + assert.NotContains(t, cl.joined(), "warning:", "no_as_advertised is a quiet fallback") +} + +func TestDiscoverOAuth_HardErrorsNeverFallBack(t *testing.T) { + tests := map[string]struct { + setup func(t *testing.T) string // returns base URL + sentinel error + }{ + "ambiguous issuers": { + setup: func(t *testing.T) string { + return startResourceServer(t, "https://as1.example.com", "https://as2.example.com").URL + }, + sentinel: oauth.ErrAmbiguousIssuers, + }, + "AS metadata fetch fails": { + setup: func(t *testing.T) string { + as := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "boom", http.StatusInternalServerError) + })) + t.Cleanup(as.Close) + return startResourceServer(t, as.URL).URL + }, + sentinel: oauth.ErrASFetchFailed, + }, + "issuer mismatch": { + setup: func(t *testing.T) string { + as := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"issuer":"https://impostor.example.com","token_endpoint":"https://impostor.example.com/token"}`) + })) + t.Cleanup(as.Close) + return startResourceServer(t, as.URL).URL + }, + sentinel: oauth.ErrIssuerMismatch, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + baseURL := tt.setup(t) + + // A poisoned Launchpad recorder proves no fallback attempt is made. + launchpad, hits := countingServer(t) + t.Setenv("BASECAMP_LAUNCHPAD_URL", launchpad.URL) + + m := newDeviceTestManager(t, baseURL) + _, err := m.Login(context.Background(), LoginOptions{ + Remote: true, + Logger: func(string) {}, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.Error(t, err) + assert.ErrorIs(t, err, tt.sentinel) + assert.Zero(t, *hits, "a hard discovery error must never reach Launchpad") + }) + } +} + +func TestLoginDevice_CapabilityUnavailable(t *testing.T) { + as := startDeviceAS(t) + // Committed BC5 issuer without device capability: no device endpoint, + // no device grant. + as.metadata = func() string { + return fmt.Sprintf(`{ + "issuer": %q, + "token_endpoint": %q, + "grant_types_supported": ["refresh_token"] + }`, as.srv.URL, as.srv.URL+"/oauth/token") + } + resource := startResourceServer(t, as.srv.URL) + + launchpad, hits := countingServer(t) + t.Setenv("BASECAMP_LAUNCHPAD_URL", launchpad.URL) + + m := newDeviceTestManager(t, resource.URL) + _, err := m.Login(context.Background(), LoginOptions{ + Remote: true, + Logger: func(string) {}, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.Error(t, err) + + var dfe *oauth.DeviceFlowError + require.ErrorAs(t, err, &dfe) + assert.Equal(t, oauth.DeviceFlowUnavailable, dfe.Reason) + assert.Zero(t, *hits, "capability failure on a selected issuer must not fall back") + assert.Empty(t, as.deviceCalls(), "no device request may be made without the capability") +} + +func TestLoginDevice_AccessDenied(t *testing.T) { + as := startDeviceAS(t) + as.token = func(int) (int, string) { + return http.StatusBadRequest, `{"error":"access_denied"}` + } + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + _, err := m.Login(context.Background(), LoginOptions{ + Remote: true, + Logger: func(string) {}, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.Error(t, err) + + var dfe *oauth.DeviceFlowError + require.ErrorAs(t, err, &dfe) + assert.Equal(t, oauth.DeviceFlowAccessDenied, dfe.Reason) +} + +func TestLoginDevice_AuthorizationPendingThenSuccess(t *testing.T) { + as := startDeviceAS(t) + as.token = func(call int) (int, string) { + if call == 0 { + return http.StatusBadRequest, `{"error":"authorization_pending"}` + } + return http.StatusOK, `{"access_token":"tok","refresh_token":"ref","token_type":"bearer","scope":"read"}` + } + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + result, err := m.Login(context.Background(), LoginOptions{ + Remote: true, + Logger: func(string) {}, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.NoError(t, err) + assert.Equal(t, "bc5", result.OAuthType) + assert.Len(t, as.tokenCalls(), 2, "one pending poll, then success") +} + +func TestLoginDevice_CancellationSurvivesErrorChain(t *testing.T) { + as := startDeviceAS(t) + as.token = func(int) (int, string) { + return http.StatusBadRequest, `{"error":"authorization_pending"}` + } + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Cancel the parent context from the sleep seam after the first poll cycle. + polls := 0 + sleepThenCancel := oauth.WithDeviceSleep(func(context.Context, time.Duration) error { + polls++ + if polls > 1 { + cancel() + } + return nil + }) + + _, err := m.Login(ctx, LoginOptions{ + Remote: true, + Logger: func(string) {}, + deviceOptions: []oauth.DeviceOption{sleepThenCancel}, + }) + require.Error(t, err) + assert.ErrorIs(t, err, context.Canceled, "parent-context cancellation must survive the error chain") +} + +func TestLoginDevice_RejectsPoisonedEndpointsBeforePOST(t *testing.T) { + tests := map[string]struct { + metadata func(as *deviceAS) string + wantMsg string + }{ + "poisoned token endpoint": { + metadata: func(as *deviceAS) string { + return fmt.Sprintf(`{ + "issuer": %q, + "token_endpoint": "https://user@evil.example/token", + "device_authorization_endpoint": %q, + "grant_types_supported": ["urn:ietf:params:oauth:grant-type:device_code"] + }`, as.srv.URL, as.srv.URL+"/oauth/device") + }, + wantMsg: "invalid token endpoint", + }, + "poisoned device authorization endpoint": { + metadata: func(as *deviceAS) string { + return fmt.Sprintf(`{ + "issuer": %q, + "token_endpoint": %q, + "device_authorization_endpoint": "https://user@evil.example/device", + "grant_types_supported": ["urn:ietf:params:oauth:grant-type:device_code"] + }`, as.srv.URL, as.srv.URL+"/oauth/token") + }, + wantMsg: "invalid device authorization endpoint", + }, + "token endpoint with out-of-range port": { + metadata: func(as *deviceAS) string { + return fmt.Sprintf(`{ + "issuer": %q, + "token_endpoint": "https://evil.example:70000/token", + "device_authorization_endpoint": %q, + "grant_types_supported": ["urn:ietf:params:oauth:grant-type:device_code"] + }`, as.srv.URL, as.srv.URL+"/oauth/device") + }, + wantMsg: "invalid token endpoint", + }, + "device authorization endpoint with out-of-range port": { + metadata: func(as *deviceAS) string { + return fmt.Sprintf(`{ + "issuer": %q, + "token_endpoint": %q, + "device_authorization_endpoint": "https://evil.example:70000/device", + "grant_types_supported": ["urn:ietf:params:oauth:grant-type:device_code"] + }`, as.srv.URL, as.srv.URL+"/oauth/token") + }, + wantMsg: "invalid device authorization endpoint", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + as := startDeviceAS(t) + as.metadata = func() string { return tt.metadata(as) } + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + _, err := m.Login(context.Background(), LoginOptions{ + Remote: true, + Logger: func(string) {}, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantMsg) + assert.Empty(t, as.deviceCalls(), "hardening must reject before any POST") + assert.Empty(t, as.tokenCalls()) + }) + } +} + +func TestRefreshLocked_BC5PublicClient(t *testing.T) { + var mu sync.Mutex + var capturedForm url.Values + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + mu.Lock() + capturedForm = r.PostForm + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"access_token":"new-tok","refresh_token":"new-ref","expires_in":3600}`) + })) + defer srv.Close() + + m := &Manager{ + cfg: config.Default(), + httpClient: srv.Client(), + store: newTestStore(t, t.TempDir()), + } + + creds := &Credentials{ + AccessToken: "old-tok", + RefreshToken: "old-ref", + OAuthType: "bc5", + TokenEndpoint: srv.URL + "/oauth/token", + Scope: "read", + ExpiresAt: time.Now().Add(-1 * time.Hour).Unix(), + } + require.NoError(t, m.store.Save("test", creds)) + + require.NoError(t, m.refreshLocked(context.Background(), "test", creds)) + + mu.Lock() + form := capturedForm + mu.Unlock() + assert.Equal(t, "refresh_token", form.Get("grant_type"), "bc5 refresh uses the standard format") + assert.Equal(t, "basecamp-cli", form.Get("client_id")) + _, hasSecret := form["client_secret"] + assert.False(t, hasSecret, "the public client must not send a client_secret key") + + reloaded, err := m.store.Load("test") + require.NoError(t, err) + assert.Equal(t, "new-tok", reloaded.AccessToken) +} + +func TestRefreshLocked_LegacyBC3RequiresReauth(t *testing.T) { + transport := &recordingTransport{} + m := &Manager{ + cfg: config.Default(), + httpClient: &http.Client{Transport: transport}, + store: newTestStore(t, t.TempDir()), + } + + creds := &Credentials{ + AccessToken: "old-tok", + RefreshToken: "old-ref", + OAuthType: "bc3", + TokenEndpoint: "https://example.com/token", + } + + err := m.refreshLocked(context.Background(), "test", creds) + require.Error(t, err) + assert.Contains(t, err.Error(), "re-authenticate") + assert.False(t, transport.attempted.Load(), "legacy bc3 refresh must fail without any network request") +} + +func TestResourceOrigin(t *testing.T) { + valid := map[string]string{ + "https://3.basecampapi.com": "https://3.basecampapi.com", + "https://3.basecampapi.com/": "https://3.basecampapi.com", + "https://3.basecampapi.com/api/v1": "https://3.basecampapi.com", + "https://host.example.com:8443/deep/p": "https://host.example.com:8443", + "http://3.basecamp.localhost:3001": "http://3.basecamp.localhost:3001", + "http://127.0.0.1:3001/path": "http://127.0.0.1:3001", + // Canonicalization: the SDK binds the protected-resource identifier to + // this string code-point exact, so equivalent spellings must reduce to + // the canonical origin — else discovery silently soft-falls back to + // Launchpad. + "HTTPS://3.BasecampAPI.com": "https://3.basecampapi.com", + "https://3.basecampapi.com:443": "https://3.basecampapi.com", + "https://3.basecampapi.com:0443/x": "https://3.basecampapi.com", + "https://Host.Example.com:08443": "https://host.example.com:8443", + "http://LocalHost:80/path": "http://localhost", + "http://[::1]:3001/path": "http://[::1]:3001", + } + for input, want := range valid { + t.Run("strips "+input, func(t *testing.T) { + got, err := resourceOrigin(input) + require.NoError(t, err) + assert.Equal(t, want, got) + }) + } + + invalid := []string{ + "file:///etc/passwd", + "https://user@host.example.com", + "https://host.example.com?q=1", + "https://host.example.com?", // bare query: ForceQuery, empty RawQuery + "https://host.example.com/path?", // bare query after a path + "https://host.example.com/#frag", + "https:opaque", + "https://", + "http://evil.example.com", // http off localhost + "https://host.example.com:0", + "https://host.example.com:70000", + "://bad", + "", + } + for _, input := range invalid { + t.Run("rejects "+input, func(t *testing.T) { + _, err := resourceOrigin(input) + require.Error(t, err) + + var outErr *output.Error + require.ErrorAs(t, err, &outErr) + assert.Equal(t, output.CodeUsage, outErr.Code) + if input != "" { + assert.NotContains(t, err.Error(), input, + "validation failures must not echo the raw URL") + } + }) + } +} + +// TestLoginDevice_ResourceEchoEndToEnd proves the RFC 8707 multi-account +// contract end to end: a device login stores the token response's resource +// indicator, a later refresh echoes it as the resource form parameter, and the +// rotated credentials still carry it when the refresh response omits it — +// without any of which a BC5 device login dies at its first token expiry. +func TestLoginDevice_ResourceEchoEndToEnd(t *testing.T) { + as := startDeviceAS(t) + as.token = func(call int) (int, string) { + if call == 0 { + // The device-grant response binds the token to an account. + return http.StatusOK, `{"access_token":"dev-tok","refresh_token":"dev-ref","token_type":"bearer","expires_in":3600,"scope":"read","resource":"urn:bc:account:42"}` + } + // The refresh response rotates tokens but OMITS resource — the stored + // binding must survive the rotation. + return http.StatusOK, `{"access_token":"dev-tok-2","refresh_token":"dev-ref-2","token_type":"bearer","expires_in":3600}` + } + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + _, err := m.Login(context.Background(), LoginOptions{ + Logger: (&collectLogger{}).log, + NoBrowser: true, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + require.NoError(t, err) + + origin := config.NormalizeBaseURL(resource.URL) + creds, err := m.store.Load(origin) + require.NoError(t, err) + assert.Equal(t, "urn:bc:account:42", creds.Resource, "device login must persist the resource binding") + + // Simulate expiry so the next refresh is forced. + creds.ExpiresAt = 1 + require.NoError(t, m.store.Save(origin, creds)) + + require.NoError(t, m.Refresh(context.Background())) + + tokenCalls := as.tokenCalls() + require.Len(t, tokenCalls, 2) + refreshForm := tokenCalls[1] + assert.Equal(t, "refresh_token", refreshForm.Get("grant_type")) + assert.Equal(t, "urn:bc:account:42", refreshForm.Get("resource"), "refresh must echo the stored resource") + assert.Equal(t, "basecamp-cli", refreshForm.Get("client_id")) + _, hasSecret := refreshForm["client_secret"] + assert.False(t, hasSecret, "the public client sends no secret") + + rotated, err := m.store.Load(origin) + require.NoError(t, err) + assert.Equal(t, "dev-tok-2", rotated.AccessToken) + assert.Equal(t, "dev-ref-2", rotated.RefreshToken) + assert.Equal(t, "urn:bc:account:42", rotated.Resource, "an omitted resource must preserve the stored binding") +} diff --git a/internal/auth/keyring.go b/internal/auth/keyring.go index 208482e50..8deff4bd0 100644 --- a/internal/auth/keyring.go +++ b/internal/auth/keyring.go @@ -17,10 +17,17 @@ type Credentials struct { RefreshToken string `json:"refresh_token"` ExpiresAt int64 `json:"expires_at"` Scope string `json:"scope"` - OAuthType string `json:"oauth_type"` // "bc3" or "launchpad" + OAuthType string `json:"oauth_type"` // "bc5", "launchpad", or legacy "bc3" TokenEndpoint string `json:"token_endpoint"` UserID string `json:"user_id,omitempty"` UserEmail string `json:"user_email,omitempty"` + + // Resource is the RFC 8707 resource indicator the tokens are bound to + // (BC5: urn:bc:account:). BC5 device logins as the trusted + // basecamp-cli client mint MULTI-ACCOUNT refresh tokens, and the refresh + // grant rejects them without this echo — refresh sends it when set and + // preserves it when a refresh response omits it. + Resource string `json:"resource,omitempty"` } // Store wraps credstore.Store with typed Credentials marshaling. diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 8ddd54ee6..f2de24c6b 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -297,11 +297,11 @@ func buildLoginCmd(use string) *cobra.Command { }, } - cmd.Flags().StringVar(&scope, "scope", "", "OAuth scope: 'read' or 'full' (BC3 only)") + cmd.Flags().StringVar(&scope, "scope", "", "OAuth scope: 'read' or 'full' (ignored by Launchpad)") cmd.Flags().BoolVar(&noBrowser, "no-browser", false, "Don't open browser automatically") cmd.Flags().BoolVar(&remote, "remote", false, "Force remote/headless mode (paste callback URL instead of local listener)") cmd.Flags().BoolVar(&local, "local", false, "Force local mode (override SSH auto-detection)") - cmd.Flags().BoolVar(&deviceCode, "device-code", false, "Headless mode: display auth URL and paste callback (alias for --remote)") + cmd.Flags().BoolVar(&deviceCode, "device-code", false, "Headless authentication with manual browser instructions") cmd.MarkFlagsMutuallyExclusive("remote", "local") cmd.MarkFlagsMutuallyExclusive("device-code", "local") diff --git a/internal/commands/auth_login_test.go b/internal/commands/auth_login_test.go new file mode 100644 index 000000000..1c6fee136 --- /dev/null +++ b/internal/commands/auth_login_test.go @@ -0,0 +1,80 @@ +package commands + +import ( + "bytes" + "context" + "net/http" + "net/http/httptest" + "os" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/auth" + "github.com/basecamp/basecamp-cli/internal/config" +) + +// TestAuthLoginDeviceCodeForcesRemoteMode is the regression test for the +// --device-code → Remote flag mapping. Discovery falls back to Launchpad +// (pointed at a 404 test server), where remote mode is observable: it prints +// the paste-callback instructions instead of opening a browser and listening +// on the loopback callback port. +func TestAuthLoginDeviceCodeForcesRemoteMode(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + tmpDir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", tmpDir) + // Pin SSH auto-detection off so only the flag can select remote mode. + t.Setenv("SSH_CONNECTION", "") + t.Setenv("SSH_CLIENT", "") + t.Setenv("SSH_TTY", "") + + // No protected-resource metadata (404) → Launchpad fallback, pointed at + // this server. The token endpoint is never reached. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer srv.Close() + t.Setenv("BASECAMP_LAUNCHPAD_URL", srv.URL) + + // Remote mode reads the pasted callback URL from os.Stdin. Swap in an + // immediate EOF so the prompt fails fast instead of blocking. + devNull, err := os.Open(os.DevNull) + require.NoError(t, err) + defer devNull.Close() + origStdin := os.Stdin + os.Stdin = devNull + t.Cleanup(func() { os.Stdin = origStdin }) + + cfg := &config.Config{BaseURL: srv.URL} + authMgr := auth.NewManager(cfg, srv.Client()) + authMgr.SetStore(auth.NewStore(tmpDir)) + app := &appctx.App{Config: cfg, Auth: authMgr} + + // Safety net: if the mapping regresses to local mode, the loopback + // listener would otherwise wait out its full five-minute timeout. + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + cmd := NewAuthCmd() + cmd.SetArgs([]string{"login", "--device-code"}) + cmd.SetContext(appctx.WithApp(ctx, app)) + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + + err = cmd.Execute() + require.Error(t, err, "EOF on the paste prompt must abort the login") + assert.Contains(t, err.Error(), "no input received") + + output := out.String() + assert.Contains(t, output, "Remote Authentication", + "--device-code must select the remote paste-callback flow") + assert.Contains(t, output, "Paste the callback URL") + assert.NotContains(t, output, "Opening browser", + "remote mode must not attempt a browser launch") +} diff --git a/internal/commands/profile.go b/internal/commands/profile.go index 2959086f8..7e899b4e5 100644 --- a/internal/commands/profile.go +++ b/internal/commands/profile.go @@ -337,12 +337,12 @@ Examples: } cmd.Flags().StringVar(&baseURL, "base-url", "", "Basecamp API base URL (default: https://3.basecampapi.com)") - cmd.Flags().StringVar(&scope, "scope", "", "OAuth scope: 'read' or 'full' (BC3 only)") + cmd.Flags().StringVar(&scope, "scope", "", "OAuth scope: 'read' or 'full' (ignored by Launchpad)") cmd.Flags().StringVar(&accountID, "account", "", "Account ID") cmd.Flags().BoolVar(&noBrowser, "no-browser", false, "Don't open browser automatically") cmd.Flags().BoolVar(&remote, "remote", false, "Force remote/headless mode (paste callback URL instead of local listener)") cmd.Flags().BoolVar(&local, "local", false, "Force local mode (override SSH auto-detection)") - cmd.Flags().BoolVar(&deviceCode, "device-code", false, "Headless mode: display auth URL and paste callback (alias for --remote)") + cmd.Flags().BoolVar(&deviceCode, "device-code", false, "Headless authentication with manual browser instructions") cmd.MarkFlagsMutuallyExclusive("remote", "local") cmd.MarkFlagsMutuallyExclusive("device-code", "local") diff --git a/internal/commands/profile_test.go b/internal/commands/profile_test.go index 9b0fe6c6f..917e9929f 100644 --- a/internal/commands/profile_test.go +++ b/internal/commands/profile_test.go @@ -4,9 +4,12 @@ import ( "bytes" "context" "encoding/json" + "net/http" + "net/http/httptest" "os" "path/filepath" "testing" + "time" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -229,6 +232,61 @@ func TestProfileCreateDeviceCodeLocalExclusive(t *testing.T) { assert.Contains(t, err.Error(), "local") } +// TestProfileCreateDeviceCodeForcesRemoteMode is the regression test for the +// --device-code → Remote flag mapping duplicated in profile create (auth login +// has its own copy, covered by TestAuthLoginDeviceCodeForcesRemoteMode). +// Remote mode reads the pasted callback URL from stdin, so an immediate EOF +// there proves the remote paste-callback flow was selected — local mode would +// instead wait on the loopback callback listener. +func TestProfileCreateDeviceCodeForcesRemoteMode(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + tmpDir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", tmpDir) + // Pin SSH auto-detection off so only the flag can select remote mode. + t.Setenv("SSH_CONNECTION", "") + t.Setenv("SSH_CLIENT", "") + t.Setenv("SSH_TTY", "") + + // No protected-resource metadata (404) → Launchpad fallback, pointed at + // this server. The token endpoint is never reached. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer srv.Close() + t.Setenv("BASECAMP_LAUNCHPAD_URL", srv.URL) + + // Remote mode reads the pasted callback URL from os.Stdin. Swap in an + // immediate EOF so the prompt fails fast instead of blocking. + devNull, err := os.Open(os.DevNull) + require.NoError(t, err) + defer devNull.Close() + origStdin := os.Stdin + os.Stdin = devNull + t.Cleanup(func() { os.Stdin = origStdin }) + + cfg := &config.Config{BaseURL: srv.URL, CacheDir: t.TempDir(), Sources: make(map[string]string)} + authMgr := auth.NewManager(cfg, srv.Client()) + authMgr.SetStore(auth.NewStore(tmpDir)) + app := &appctx.App{Config: cfg, Auth: authMgr} + + // Safety net: if the mapping regresses to local mode, the loopback + // listener would otherwise wait out its full five-minute timeout. + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + root := &cobra.Command{Use: "basecamp"} + root.AddCommand(NewProfileCmd()) + root.SetArgs([]string{"profile", "create", "test-profile", "--base-url", srv.URL, "--device-code"}) + root.SetContext(appctx.WithApp(ctx, app)) + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + + err = root.Execute() + require.Error(t, err, "EOF on the paste prompt must abort the login") + assert.Contains(t, err.Error(), "no input received", + "--device-code must select the remote paste-callback flow") +} + func TestProfileCreateRejectsDuplicateName(t *testing.T) { cfg := &config.Config{ BaseURL: "https://3.basecampapi.com", diff --git a/internal/completion/refresh_test.go b/internal/completion/refresh_test.go index 10a485249..b29306ae9 100644 --- a/internal/completion/refresh_test.go +++ b/internal/completion/refresh_test.go @@ -31,6 +31,7 @@ func newTestAccountClient(t *testing.T) *basecamp.AccountClient { t.Helper() cfg := &basecamp.Config{ + BaseURL: "https://3.basecampapi.com", CacheEnabled: false, } client := basecamp.NewClient(cfg, &mockTokenProvider{}, diff --git a/internal/sdk/store.go b/internal/sdk/store.go index 27e20ee46..430038d85 100644 --- a/internal/sdk/store.go +++ b/internal/sdk/store.go @@ -6,9 +6,14 @@ type Credentials struct { RefreshToken string `json:"refresh_token,omitempty"` ExpiresAt int64 `json:"expires_at,omitempty"` TokenEndpoint string `json:"token_endpoint,omitempty"` - OAuthType string `json:"oauth_type,omitempty"` // "bc3" or "launchpad" + OAuthType string `json:"oauth_type,omitempty"` // "bc5", "launchpad", or legacy "bc3" Scope string `json:"scope,omitempty"` UserID string `json:"user_id,omitempty"` + + // Resource is the RFC 8707 resource indicator the tokens are bound to + // (BC5: urn:bc:account:), echoed on refresh — multi-account refresh + // tokens are rejected without it. Mirrors internal/auth.Credentials. + Resource string `json:"resource,omitempty"` } // CredentialStore provides persistent storage for OAuth credentials. diff --git a/internal/version/sdk-provenance.json b/internal/version/sdk-provenance.json index 2c47d43af..1ad07d9cf 100644 --- a/internal/version/sdk-provenance.json +++ b/internal/version/sdk-provenance.json @@ -1,9 +1,9 @@ { "sdk": { "module": "github.com/basecamp/basecamp-sdk/go", - "version": "v0.9.1-0.20260727173625-bb363c847b92", - "revision": "bb363c847b92", - "updated_at": "2026-07-27T17:36:25Z" + "version": "v0.9.1-0.20260728223259-0bdb14ac4a7b", + "revision": "0bdb14ac4a7b", + "updated_at": "2026-07-28T22:32:59Z" }, "api": { "repo": "basecamp/bc3", diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index d5c6d772e..2966ccceb 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -1033,7 +1033,7 @@ The CLI uses two directory namespaces: `basecamp` for your Basecamp identity and ``` ~/.config/basecamp/ # Basecamp identity (DO NOT read credentials) ├── credentials.json # OAuth tokens — NEVER read or log -├── client.json # DCR client registration +├── client.json # Obsolete (former dev-only client registration; safe to delete) └── config.json # Global preferences (account_id, base_url, format) ~/.cache/basecamp/ # Tool cache (ephemeral, auto-managed) @@ -1101,8 +1101,8 @@ leave the skill only and surface the per-agent `basecamp setup ` commands. ```bash basecamp auth status # Check auth basecamp auth login # Re-authenticate -basecamp auth login --scope full # Full access (BC3 OAuth only) -basecamp auth login --device-code # Headless: display URL, paste callback +basecamp auth login --scope full # Full access (ignored by Launchpad) +basecamp auth login --device-code # Headless authentication with manual browser instructions ``` **Network errors / localhost URLs:**