BC5 OAuth: resource-first discovery, device-flow login, RFC 8707 resource echo

[jeremy]

Adopts the SDK's BC5 OAuth stack end to end: resource-first discovery (RFC 9728 + 8414), RFC 8628 device-flow login as the pre-registered public basecamp-cli client, and the RFC 8707 multi-account refresh contract.

Why the resource echo matters: basecamp-cli is a trusted client in bc3's registry, so a device approval records an identity-wide grant and mints a multi-account refresh token. bc3's refresh grant hard-requires the RFC 8707 resource parameter for those (400 invalid_request without it) — without persisting and echoing it, every device login dies at its first token expiry (~1h). Credentials gain a resource field (auth + mirrored sdk store); device login persists token.Resource; refresh echoes it and preserves it when the refresh response omits it. An end-to-end test drives login → stored resource → forced refresh → form echo → binding surviving a resource-less rotation.

Gating: DRAFT until basecamp-sdk#478 (resource indicator + 429 poll contract, stacked on #376/#370) merges — the SDK pin tracks the #478 branch head (see go.mod + internal/version/sdk-provenance.json for the current pseudo-version — re-pinned on every go/ change); it moves to the merged SHA before this lands. No SDK release required first, per current pseudo-version practice.

Also restacked onto main (was 36 behind): reconciled the go.mod/go.sum/provenance conflict to main's newer pin, dropped stale go.sum entries the auto-merge left, and absorbed SDK API drift since the branch's old 7207f742 snapshot (VerificationURIComplete/RegistrationEndpoint → *string).

---

Summary by cubic

Adopts the BC5 OAuth stack end to end: resource‑first discovery, automatic RFC 8628 device‑flow login as public basecamp-cli, and RFC 8707 resource echo, with stricter endpoint validation and provider‑neutral flags. Updates the SDK to github.com/basecamp/basecamp-sdk/go@v0.10.1-0.20260729233651-9480aa4c84b8 (merged stack: device flow, resource indicators, hardened transport/parse/refresh).

• New Features

  • Resource‑first discovery (RFC 9728 → 8414) from a canonicalized resource origin; falls back to Launchpad only on “no AS advertised” or “resource discovery failed”.
  • Device‑flow login as basecamp-cli with validated verification URLs (HTTPS, localhost case‑insensitive, ports 1–65535), sanitized display, and early abort on bad data; --device-code forces headless; --scope help notes Launchpad ignores it.
  • Persist and echo the RFC 8707 resource across refreshes for multi‑account tokens; mirrored in the SDK store; legacy “bc3” refreshes surface a re‑authenticate error.
  • Fix the authorization‑info URL to use the canonical resource origin.

• Migration

  • Removed the BC3 dev‑only DCR/PKCE flow; client.json is obsolete and safe to delete.
  • Stored “bc3” credentials won’t refresh; re‑authenticate to migrate.

^{Written for commit 2252fc1. Summary will update on new commits.}

[Copilot]

Pull request overview

This PR updates Basecamp CLI authentication to adopt the SDK’s BC5 OAuth stack end-to-end: resource-first discovery (RFC 9728 → 8414), automatic device-flow login when available (RFC 8628), and persistence/echoing of the RFC 8707 resource indicator so multi-account refresh tokens can be refreshed reliably.

Changes:

• Implement provider discovery + BC5 device-flow login path, including endpoint validation and sanitized device-code display, and persist/echo resource on refresh.
• Update CLI help/docs to be provider-neutral and clarify --device-code semantics; document the now-obsolete client.json.
• Bump github.com/basecamp/basecamp-sdk/go to the PR’s required pseudo-version + update provenance; expand unit and e2e coverage around auth/flags/refresh behavior.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
skills/basecamp/SKILL.md	Document client.json as obsolete; clarify --scope and --device-code help text.
README.md	Update authentication docs to describe device-flow-first behavior and provider-neutral flags; remove client.json from config tree and note it’s safe to delete.
internal/version/sdk-provenance.json	Update pinned SDK revision metadata to match the new pseudo-version.
internal/sdk/store.go	Add persisted resource field to SDK-facing credentials struct.
internal/completion/refresh_test.go	Ensure test client config includes BaseURL for refreshed completion behavior.
internal/commands/profile.go	Update profile create flag help strings to match provider-neutral semantics.
internal/commands/profile_test.go	Add regression coverage ensuring --device-code forces remote/paste-callback mode.
internal/commands/auth.go	Update auth login flag help strings to match provider-neutral semantics.
internal/commands/auth_login_test.go	Add regression coverage ensuring --device-code forces remote/paste-callback mode in auth login.
internal/auth/keyring.go	Persist resource in stored credentials (keyring/file fallback).
internal/auth/device_test.go	Add comprehensive unit coverage for discovery, device flow, endpoint hardening, and RFC 8707 resource echo on refresh.
internal/auth/auth.go	Implement resource-first discovery, device-flow login path, refresh resource echo/preservation, and shared endpoint validation.
internal/auth/auth_test.go	Update existing auth tests to match the new discovery and Launchpad-only authorization-code behavior.
go.mod	Bump basecamp-sdk dependency to required pseudo-version.
go.sum	Update module sums for the SDK bump.
e2e/auth.bats	Add e2e assertions for updated help text phrasing (provider-neutral flags).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cubic-dev-ai]

1 issue found across 16 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/auth/auth.go">

<violation number="1" location="internal/auth/auth.go:762">
P1: Equivalent base-URL spellings with an explicit default port or mixed-case host will miss BC5 discovery and fall back to Launchpad. Canonicalize scheme, hostname, and default ports before returning the resource origin so it matches the protected-resource metadata identifier.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

internal/auth/device_test.go:563

• After making countingServer return a synchronized accessor, hits is a function; dereferencing it (*hits) won’t compile and also bypasses synchronization. Call the accessor instead.

	assert.Zero(t, *hits, "capability failure on a selected issuer must not fall back")

internal/auth/device_test.go:137

• countingServer returns a *int that is later read without taking the mutex, which will trigger a data race under -race when the handler increments count. Return a small accessor closure (or an atomic counter) so reads are synchronized too.

// 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

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.

[github-actions]

⚠️ Potential breaking changes detected:

• Modified description for the '--scope' flag to indicate that it is now ignored by Launchpad, which could affect user scripts relying on its previous functionality.
• Changed description for the '--device-code' flag, clarifying its functionality and removing the mention of being an alias for '--remote', which may impact user understanding or scripts that rely on this description.

Review carefully before merging. Consider a major version bump.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

internal/auth/auth.go:1053

• AuthorizationEndpoint() now treats stored oauthTypeBC5 the same as bc3 (returns <base_url>/authorization.json), but the AuthorizationEndpoint test suite doesn’t cover the new bc5 branch. Adding a targeted regression test would help prevent future changes from breaking bc5 behavior (and clarify how BASECAMP_TOKEN should be classified when using bc5 tokens).

	oauthType := m.GetOAuthType()
	switch oauthType {
	case "bc3", oauthTypeBC5:
		return config.NormalizeBaseURL(m.cfg.BaseURL) + "/authorization.json", nil
	case oauthTypeLaunchpad, "":
		// "launchpad" = stored credentials; "" = no stored credentials and

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.