Repository navigation
Commit ac62cdf
authored
Import personal access tokens with auth login --with-token (#681)
* Import personal access tokens with auth login --with-token
A bot's Basecamp identity should never come from a browser sign-in on a
shared machine: whoever's session the browser holds is who the profile
quietly becomes, and nothing checks. Basecamp now mints personal access
tokens, so the CLI can take one from a secret store and refuse to keep it
unless it authenticates as the expected identity.
`basecamp auth login --with-token -P <profile> --account <id>` reads the
token from stdin (a terminal is refused, with the `op read … |` shape
shown), stores it under the profile as a non-expiring bc5 credential —
ExpiresAt 0 is the path AccessToken already never refreshes — and creates
the profile when --account names its account. The profile entry is written
only after /my/profile.json and the authorization endpoint have answered
for the token; a rejected token leaves the profile exactly as it was found,
previous credential included. Profile registration is factored out of
`profile create` so both paths write the same entry, and the root pre-run
lets a may-create command name a profile that does not exist yet.
`--expect-identity <id>` makes every login assertive — browser and device
flows included — discarding the new credential on a mismatch. `--json`
returns the envelope (profile, account, identity, person, oauth_type,
scope, expires_at null), and the interactive flows now refuse machine
output modes up front instead of printing prose and blocking (the
machine-mode half of #669).
`BASECAMP_OAUTH_ISSUER` pins the BC5 authorization server and skips
discovery, so a device login reaches a server that is piloting the client
while its metadata is still dark. It is a temporary escape hatch and is
documented as one. `--login-hint <email>` is wired through LoginOptions;
the pinned SDK cannot put it on the wire yet (basecamp/basecamp-sdk#841
adds the option), so this build tells the user the hint instead of sending
it, and Launchpad ignores it.
* Verify a login before storing it
Review of the token import found the rollback doing the wrong job: the
credential was written under the live profile key, checked, and then
restored or deleted — a window where another command could pick up an
unverified token, a restore that could itself fail, and a success line
printed before the check. Turn it around: the candidate token gets a client
of its own (App.SDKClientFor, same transport, hooks and user agent as the
real one), proves itself, and only then is stored. LoginOptions.Verify runs
that check inside the device and Launchpad flows before their store.Save,
so --expect-identity on a browser login stores nothing on a mismatch; the
snapshot/restore code is gone.
The check now covers the account, not only the identity. Production
serves /my/profile.json only under an account prefix (the unscoped path
404s — the old "Logged in as" line had been failing silently), so the
person lookup goes through the account-scoped People().Me, and that
account must be numeric, must be the effective one (--account or
BASECAMP_ACCOUNT_ID override the profile binding at runtime, so the
mismatch guard compares against that), must appear non-expired in the
authorization document, and for a new profile must have been given
explicitly rather than inherited from the operator's config. A reported
scope outside read/full is refused rather than stored.
Smaller findings from the same round: --scope and --expect-identity are
validated before stdin is consumed; --expect-identity compares parsed
integers; the envelope reports the identity's own email and marks an
existing default profile as default; server-supplied names, the login
hint and the pinned issuer are reduced to one line before reaching the
terminal; an unregistered profile name is no longer used as a cache path
component; the e2e cases assert exact error and code with BASECAMP_PROFILE
cleared; README and the doctor skill describe the token import, the
identity assertion, and that --login-hint is announced, not sent.
* Bind what the token was verified for, and keep ordinary logins best-effort
The import verifies a token for one account and base URL and stores it
under a profile; those must be the same place. An existing profile's base
URL must match the effective one (a BASECAMP_BASE_URL override would
verify against one host and store for another), an unbound profile is
bound to the explicitly given account alongside the token (other fields
preserved), and account comparisons are numeric like the rest of the
package. The profile entry is written before the credential, so a failure
between the two leaves a visibly unauthenticated profile rather than an
orphaned secret. Strict verification refuses an authorization document
with no identity id.
An ordinary browser or device login without --expect-identity is not the
assertive kind: an account the token cannot reach, or a person lookup
that fails, no longer discards the login — the identity line falls back
to the authorization document and no person id is fabricated (nothing is
written as user_id 0 either).
Stdin accepts exactly one trailing line ending, the shape a pipe delivers,
rather than trimming whatever whitespace surrounds the secret.
* Keep the server's expiry, refuse stray arguments and non-global bindings
The authorization document reports when the token expires; the import
now stores that instead of claiming the token never does, and the
envelope and status line report it. With no refresh token, AccessToken
refuses the token near expiry with the existing "No refresh token"
error, so the remedy — import again — is named rather than discovered
from failing requests. A document without an expiry still stores zero.
The login command takes no positional arguments, so a token pasted after
--with-token is refused instead of being ignored while it sits in shell
history. Binding an accountless profile rewrites the global config file,
so a profile that arrived from a system, repo or local config is refused
before stdin is read rather than left shadowing a binding it never sees.
* Judge a profile binding by the global file's own entry, refuse near-expiry tokens
Config layers merge profiles per name and record one aggregate source for
the whole map, so cfg.Sources["profiles"] cannot say where a particular
profile came from — a global unbound profile was refused whenever a repo
or local config contributed any other profile. Read the global file's
entry instead: binding proceeds when that entry exists and is unbound,
and is refused (before stdin) when it is missing or already bound, since
the accountless effective profile then came from another layer and would
keep shadowing whatever is written.
An imported token has nothing to refresh with, so one that the server
reports as expiring inside the refresh window would be refused by its
first command; the import now declines it up front. RefreshWindow is
exported from the auth package for that check instead of repeating the
five-minute literal.
* Refuse to rewrite a malformed config, and limit the expiry refusal to imports
Every writer of the global config file read it with parse errors ignored
and then wrote the map back, so a token import that registered a profile
could replace an operator's malformed-but-present config with a partial
decode. loadGlobalConfigFile now treats only a missing file as empty and
returns read and parse errors; register, unregister, bind and set-default
all go through it and refuse before writing.
The near-expiry refusal exists because an imported token has nothing to
refresh with. An asserted OAuth login (--expect-identity) reaches the same
verifier with a refresh token in hand, so the check is keyed to the
import (noRefresh) rather than to strict mode.
Imported credentials record source: "token", and auth status and profile
show report it, so a machine consumer can tell a non-refreshable personal
token from an OAuth credential.
* Pin an origin only, refuse a non-object profiles value, and name app.basecamp.com
BASECAMP_OAUTH_ISSUER derives its endpoints by appending /oauth/... to
the value, so a path would produce endpoints nothing serves; only an
origin is accepted now (no path, no opaque form). The global config
writers refuse a "profiles" value that is not an object for the same
reason they refuse a parse failure, and loadGlobalConfigFile creates the
config directory so every writer — set-default included — can run before
a global config exists. The README names app.basecamp.com for the token
page and the pinned issuer.
* Prove the config can take a profile before profile create logs in
profile create registers its entry after the OAuth login, and the global
config writers now refuse a malformed file, so a refusal there would have
left a live credential with no profile. The file is checked for
writability before the login runs; nothing is written by the check. The
README no longer calls an imported token non-expiring — its expiry is
whatever the server reports.
* Bump basecamp-sdk to main and send --login-hint on the wire
basecamp-sdk#841 (47e7ca38) adds oauth.WithDeviceLoginHint, so the device
flow now carries --login-hint as Basecamp's login_hint extension to the
device authorization request instead of telling the user the hint. The
option is appended only when a hint was given, so an unqualified login's
form is unchanged. Launchpad still logs that it ignores the flag.
make bump-sdk REF=main: v0.15.0 → v0.16.1-0.20260903193203-47e7ca381a49,
provenance updated; the catalog parity tests pass against the new SDK.
* Record the pinned SDK revision in the MCP model provenance
main now checks that the vendored MCP model was synced from the same
basecamp-sdk revision go.mod pins. This branch pins the post-v0.16.0
commit that carries the device-flow login hint, so the provenance moves
to that commit. scripts/sync-mcp-model.sh against that checkout leaves
behavior-model.json and openapi.json byte-identical to the v0.16.0
snapshot, and would describe the ref as go/v0.16.0-1-g47e7ca38; the ref
is written as the go.mod pseudo-version instead, which is what the
provenance test compares. A v0.16.1 tag would restore the tagged shape.
* Strip a trailing CR only as part of a CRLF pair
A token ending in a bare carriage return was accepted: the LF and CR
suffixes were trimmed independently, so "token\r" lost its CR and passed
the control-character check the contract says it must fail. The CR now
goes only when it precedes the final LF; on its own it is left for the
single-line check to refuse.
* Refuse a null config, and prove the file is writable before anything irreversible
The global config writers refuse a malformed file rather than rewrite
it, but a top-level null decoded into a nil map and the next assignment
panicked instead. It is refused like any other non-object value.
Two callers ran the refusal after a step that cannot be taken back: the
token import read stdin and verified the token against the server before
registering or binding the profile discovered the file was unusable, and
profile delete removed the credential first. Both now prove the file can
take the write up front, as profile create already did.
* Refuse a login whose override disagrees with the profile's binding, OAuth included
The token import refused an --account / BASECAMP_ACCOUNT_ID /
BASECAMP_BASE_URL override that disagreed with the named profile's
binding, since the credential would be verified for the override and
stored under a key that keeps the binding. The browser and device flows
had no such check: an asserted login under an override verified against
the override and then replaced the profile's working credential with one
proven for somewhere else. The check is one helper now, run by every
login that names an existing profile before any request is made.
* Let profile create assert the identity it signs in as
--expect-identity ran only on auth login, which refuses a profile that
does not exist yet; the browser or device login that creates one had no
way to refuse a wrong session. profile create takes the same flag and
runs the same verifier before the credential is stored: a mismatch
leaves no credential and no profile entry.
The verifier's account-scoped person lookup replaces the unscoped
/my/profile.json fetch that ran after registration, which production
answers with a 404. SDKClientFor now addresses the effective base URL
rather than the one the startup client was built with, since a profile
being created sets its own in memory before its login runs; for every
existing caller the two are the same value.
* Return only the error from writableGlobalProfiles, since no caller reads the map
* Bump google.golang.org/grpc to v1.83.2 for CVE-2026-84445
An indirect dependency main carries too; the Security workflow's Trivy
scan started failing on it once the advisory reached the database.
* Reject a bare query or fragment delimiter on the pinned issuer
url.Parse reports "https://as.example#" with an empty fragment, so the
origin check passed it, and the endpoints derived by concatenation
carried the delimiter: the token endpoint became https://as.example#/oauth/tokens,
posting to / with the path as a fragment. The delimiters are now refused
on the raw value.
* Say an imported token reported no expiry, not that it never expires
* Name the config preflight for what it checks
globalConfigTakesProfiles reads and validates the file's shape; it does
not probe permissions, and the old name claimed more than that.
* Recompute the Nix vendorHash for the SDK pin and grpc bump
Computed and verified with make update-nix-hash (Docker build succeeds
with the new hash).1 parent 6c88ef2 commit ac62cdf
23 files changed
Lines changed: 2640 additions & 139 deletions
File tree
- e2e
- internal
- appctx
- auth
- cli
- commands
- mcpserver/model
- stdinarg
- version
- nix
- skills
- basecamp-doctor
- basecamp
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1945 | 1945 | | |
1946 | 1946 | | |
1947 | 1947 | | |
| 1948 | + | |
1948 | 1949 | | |
1949 | 1950 | | |
1950 | 1951 | | |
1951 | 1952 | | |
1952 | 1953 | | |
1953 | 1954 | | |
1954 | 1955 | | |
| 1956 | + | |
1955 | 1957 | | |
1956 | 1958 | | |
1957 | 1959 | | |
| |||
1966 | 1968 | | |
1967 | 1969 | | |
1968 | 1970 | | |
| 1971 | + | |
1969 | 1972 | | |
1970 | 1973 | | |
1971 | 1974 | | |
| |||
10740 | 10743 | | |
10741 | 10744 | | |
10742 | 10745 | | |
| 10746 | + | |
10743 | 10747 | | |
10744 | 10748 | | |
10745 | 10749 | | |
10746 | 10750 | | |
10747 | 10751 | | |
10748 | 10752 | | |
10749 | 10753 | | |
| 10754 | + | |
10750 | 10755 | | |
10751 | 10756 | | |
10752 | 10757 | | |
| |||
10761 | 10766 | | |
10762 | 10767 | | |
10763 | 10768 | | |
| 10769 | + | |
10764 | 10770 | | |
10765 | 10771 | | |
10766 | 10772 | | |
| |||
11927 | 11933 | | |
11928 | 11934 | | |
11929 | 11935 | | |
| 11936 | + | |
11930 | 11937 | | |
11931 | 11938 | | |
11932 | 11939 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
194 | 194 | | |
195 | 195 | | |
196 | 196 | | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
197 | 225 | | |
198 | 226 | | |
199 | 227 | | |
| |||
218 | 246 | | |
219 | 247 | | |
220 | 248 | | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
221 | 255 | | |
222 | 256 | | |
223 | 257 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
90 | 90 | | |
91 | 91 | | |
92 | 92 | | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
93 | 122 | | |
94 | 123 | | |
95 | 124 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
| |||
141 | 141 | | |
142 | 142 | | |
143 | 143 | | |
144 | | - | |
| 144 | + | |
145 | 145 | | |
146 | 146 | | |
147 | 147 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
87 | 87 | | |
88 | 88 | | |
89 | 89 | | |
90 | | - | |
91 | | - | |
| 90 | + | |
| 91 | + | |
92 | 92 | | |
93 | 93 | | |
94 | 94 | | |
| |||
508 | 508 | | |
509 | 509 | | |
510 | 510 | | |
511 | | - | |
512 | | - | |
| 511 | + | |
| 512 | + | |
513 | 513 | | |
514 | 514 | | |
515 | 515 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
50 | 55 | | |
51 | 56 | | |
52 | 57 | | |
| |||
126 | 131 | | |
127 | 132 | | |
128 | 133 | | |
129 | | - | |
| 134 | + | |
130 | 135 | | |
131 | 136 | | |
132 | | - | |
133 | | - | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
134 | 140 | | |
135 | 141 | | |
136 | 142 | | |
| |||
147 | 153 | | |
148 | 154 | | |
149 | 155 | | |
150 | | - | |
151 | | - | |
152 | | - | |
153 | | - | |
154 | | - | |
155 | | - | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
156 | 163 | | |
157 | 164 | | |
158 | 165 | | |
159 | 166 | | |
160 | 167 | | |
161 | 168 | | |
162 | 169 | | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
163 | 181 | | |
164 | 182 | | |
165 | 183 | | |
| |||
0 commit comments