Repository navigation
feat(auth): oauth refresh tokens and persisted oauth tokens - #40
Conversation
|
@Showdown76py, rebased onto the rewritten main (3e02203): same three commits, byte-identical diff, CI green. The vm-create one is still standalone if you want to take it first. |
There was a problem hiding this comment.
Reviewed 633f254 and ran it as the real server, driven over HTTP with the full authorization_code flow (TOTP, ticket, finalize, PKCE S256) and real /mcp calls. Unit suite: 534 passed, ruff clean. The core is right: client_secret is checked before any grant, rotation and reuse detection behave as described, nothing raw lands in tokens.db (0600), and client_credentials still gets no refresh token.
What passed (38/38): refresh across a restart, rotation of both tokens, bad secret 401, another client's creds / unknown / missing token 400 without hurting the live family, replay revoking the current access and refresh tokens while leaving a second family alone, all four audit events, security_end_session through a real MCP client (refresh refused at once, bearer alive for the grace window, not resurrected after a restart), expired refresh with a 3 s TTL, DCR advertising and receiving refresh tokens, dashboard connector delete killing both tokens, and an upgrade from a main tokens.db (tables created, named token kept, old OAuth tokens gone as documented).
Two things before merging, both reproduced:
beaconmcp auth revokewhile the server is running no longer cuts access. The server loadsclients.jsononce, soclient_store.verify()keeps accepting a CLI-revoked client. In my run the revoked client did 5/5 successful refreshes after the revoke, and the new access token worked on/mcp. Before this PR the window was capped at 24 h by the bearer TTL; with refresh tokens it lasts until the next restart (whereprune_unknown_clientsdoes clean it up). The description says the startup prune "coversbeaconmcp auth revokerun while the server is up", but it only applies after a restart. Could you reloadclients.jsonwhen its mtime changes (or at least check it on the refresh grant)? Failing that, the docs should say a restart is required afterauth revoke.refresh_token_ttl: 0never purges rotated tokens. Rotated-out rows keepexpires_at = inf; after 25 refreshesrefresh_tokensheld 26 rows, 25 of themused. They grow by one row per refresh forever, in the table and in memory. Keepingusedrows for a bounded window (e.g. 30 days) would still catch replays.
Optional:
- Two concurrent refreshes with the same token (a client retrying after a network timeout) give one 200 and one 400, and the 400 counts as reuse, so the whole family is revoked, including the pair the winner just received. That is the strict RFC 9700 behaviour, but it forces a TOTP re-login on a plain retry. A short grace window before reuse revokes the family would avoid it.
Also: #38 conflicts with this PR in docs/configuration.md only; the unit suite passes on the merged result (537 passed).
OAuth access tokens only lived in memory and no refresh token was issued, so an authorization_code client like claude.ai fell back to "needs authentication" on every restart and at least once every 24 h, forcing a manual passkey/TOTP sign-in each time. - /oauth/token returns a refresh_token on the authorization_code grant (never on client_credentials, which keeps its TOTP-on-every-exchange rule) - new refresh_token grant: same client_secret_post check, rotates the refresh token and issues a new access token; replaying a rotated-out token revokes the whole family (OAuth 2.1 / RFC 9700 reuse detection) - OAuth access and refresh tokens are persisted in tokens.db as SHA-256 hashes (new oauth_tokens / refresh_tokens tables, created in place), reloaded at startup, expired rows purged; dashboard session bearers stay memory-only - server.refresh_token_ttl (default 30 days, 0 = never expires), env BEACONMCP_REFRESH_TOKEN_TTL, wizard field and config summary - refresh_token added to grant_types_supported (main and DCR metadata) and to the DCR registration response - deleting a connector drops its OAuth tokens; tokens of clients missing from clients.json are pruned at startup; security_end_session also drops the refresh family - audit: auth.token.refresh, auth.token.refresh.reuse, and auth.token.issue now also emitted for authorization_code - docs: configuration, security, clients, yaml example Tests: 498 passed.
Review follow-ups on the refresh-token PR. `beaconmcp auth revoke` run while the server was up no longer cut access: the server read clients.json once, so `client_store.verify()` kept accepting the revoked client and, with refresh tokens, it could mint bearers until the next restart. And with `refresh_token_ttl: 0` every rotated-out refresh token kept `expires_at = inf` and was never purged -- one row per refresh, forever, in the table and in memory. - ClientStore re-reads clients.json when its (mtime, size, inode) signature changes, on every public method. A broken or missing file keeps the current set rather than dropping every client. Our own `_save` does not count as a change, and it now holds a lock and happens after a reload, so a server-side DCR save no longer overwrites a CLI change made in the meantime. - `on_clients_removed` hook, wired in `_run_http` to `TokenStore.prune_unknown_clients`: the revoked client's OAuth bearers and refresh tokens die with it. The bearer middleware also checks that the token's client still exists, so a CLI revoke returns 401 on the very next /mcp call, not after the 24 h bearer TTL. - Rotated-out refresh tokens are kept 30 days at most (`USED_REFRESH_RETENTION`), whatever `refresh_token_ttl` says; rows written before the cap are bounded on load. - Retry grace: a rotated-out token presented again within 60 s (`REFRESH_REUSE_GRACE`) is read as the same client resending after a network timeout and gets the pair the first call minted, memory only. Past the window, or after a restart, reuse detection is as strict as before. - docs/security.md: the revoke, retention and grace behaviour. Tests: 552 passed (15 new), ruff clean. Reproduced the live scenario: client_credentials + /mcp 200, `auth revoke` from the CLI, /mcp 401 and /oauth/token 401 invalid_client at once, tokens.db emptied.
633f254 to
9249506
Compare
|
@Showdown76py, the revoke finding is the one that matters and the description was wrong about it: I wrote "covers 1. Revoke while running. 2. 3. Concurrent refresh. A rotated-out token presented again within 60 s ( Tests: 552 passed (15 new: 6 in |
There was a problem hiding this comment.
Thanks, 9249506 fixes all three points and the reload / retention / grace code reads right to me.
One change left, and it is a product decision on our side rather than a bug: a sign-in must not outlive 24 h. With a 30-day sliding refresh TTL (and 0 = never), a connector in regular use never goes back through TOTP, which is the opposite of what we want for a server that can run shell on the hypervisors.
What we would like:
- Absolute session lifetime of 24 h, counted from the
authorization_codeexchange: every refresh token of a family expires atfamily_created_at + 24 h, whateverrefresh_token_ttlsays, so a refresh can extend the access token up to that point but never past it. After that the client goes back through/oauth/authorize. - Drop the "never expires" option (
refresh_token_ttl: 0), and caprefresh_token_ttlat 24 h (reject or clamp larger values in config parsing and the wizard). - Keep the rest: persistence across restarts and self-updates, rotation, reuse detection, the 60 s retry grace and the CLI revoke handling are all still wanted.
- Docs (
configuration.md,security.md,clients.md, yaml example) should say clients re-authenticate once a day, and a test should pin that a refresh at 24 h + ε fails even if it was rotated a minute earlier.
tl;dr revert your modifications concerning the refresh_token, make sure it keeps expiring after 24h just like we did before, and update the documentation.
A refresh token can carry a connector up to 24 h after the authorization_code exchange, never past it: every access and refresh token of a family now shares a family_expires_at, and the pair minted by a refresh stops there however recent the rotation, so claude.ai, ChatGPT and Le Chat go back through TOTP once a day. refresh_token_ttl becomes an idle timeout inside that day: default and maximum 86400, 0 (never expires) rejected by the loader, the wizard and the BEACONMCP_REFRESH_TOKEN_TTL override. Rotated-out tokens die with their sign-in, which makes the 30-day retention cap pointless, so it goes. Existing tokens.db rows get the column and are bounded from each family's first token.
|
@Showdown76py, agreed on the product call, done in Absolute lifetime. Every refresh token row carries No more Kept, as listed: persistence across restarts (within the day), rotation, reuse detection, the 60 s retry grace, the Upgrade. Docs. Tests: 559 passed ( One reading of your tl;dr I did not take: I kept the refresh token rather than reverting to the pre-PR 24 h bearer with no refresh at all, since the bullet list says persistence, rotation and the grace are still wanted. If you'd rather have the plain revert, say so and I'll cut it down to that. |
Showdown76py
left a comment
There was a problem hiding this comment.
Both rounds addressed: CLI revoke now takes effect on the next request, TTL 0 is gone, concurrent refresh has its 60 s grace, and a sign-in is capped at 24 h including refreshes. Thanks, merging.
Generated by Claude Code
Fixes the reason an
authorization_codeconnector (claude.ai in particular) kept dropping to "needs authentication" at every restart, and lets it renew its bearer inside the day without a prompt, while keeping the rule that a sign-in never outlives 24 h:/oauth/tokencalledtoken_store.issue(client_id)without a name, so nothing was persisted and everysystemctl restart beaconmcp(including a self-update) turned the connector's bearer into a 401./oauth/tokennever returned arefresh_token. A refresh token now carries the connector up to 24 h after the authorization, never past it: the operator still types their TOTP once a day, which is the intended behaviour for a server that can run shell on the hypervisors.What changed
Refresh tokens (
authorization_codegrant only)/oauth/tokennow returnsrefresh_tokennext to the access token.grant_type=refresh_token: sameclient_id+client_secret_postcheck as the other grants, issues a new access token and a new refresh token, and rotates the old one out.family_expires_at(theauthorization_codeexchange +TokenStore.SESSION_LIFETIME), shared by the whole family and never pushed back. A refresh mints its pair withmin(now + ttl, family_expires_at), the access token is capped the same way (soexpires_inshrinks as the day runs out), and a refresh at 24 h + ε fails withexpiredand drops the family even if the token was rotated a minute earlier. The client then goes back through/oauth/authorize.REFRESH_REUSE_GRACE) is read as the same client resending after a network timeout and gets the pair the first call minted (memory only, never written totokens.db; strict again after a restart). Rotated-out tokens are kept for that detection until their sign-in ends, so the table is bounded by construction.client_credentialsstill gets no refresh token: it keeps its fresh-TOTP-on-every-exchange rule and its plain 24 h bearer.refresh_tokenis advertised ingrant_types_supported(main and DCR metadata) and in the DCR registration response.Persistence
tokens.db, in two new tables (oauth_tokens,refresh_tokens) created in place on first start. They are stored as SHA-256 hashes and kept in memory keyed by that hash, so the file alone does not hand out working credentials.Revocation
/app/connectorsdrops its OAuth tokens.beaconmcp auth revokewhile the server is running takes effect at the client's next request:ClientStorere-readsclients.jsonwhen its (mtime, size, inode) signature changes, hands the survivors toTokenStore.prune_unknown_clients, and the bearer middleware checks that the token's client still exists. A broken or missing file keeps the current client set rather than dropping everyone. Side effect worth knowing: a server-side save (DCR provisioning) now reloads first, so it no longer overwrites a CLIauth createmade in the meantime.clients.jsonare pruned too (revoke run while the server was down). Skipped if no client loaded at all, so an unreadableclients.jsoncannot wipe the db.security_end_sessionkeeps its grace window for the bearer, and also drops the refresh family right away, so the client can't quietly mint a replacement.Config
server.refresh_token_ttlis an idle timeout inside the day: default and maximum86400,0(never expires) and anything above 24 h rejected by the loader (MAX_REFRESH_TOKEN_TTL), by the wizard input and by theBEACONMCP_REFRESH_TOKEN_TTLoverride in_run_http(that path used to skip the loader).validate-configsummary updated.Audit
auth.token.refreshon success,auth.token.refresh.reusewhen a family gets revoked,auth.token.failwithreason=refresh_invalid|refresh_expiredotherwise.auth.token.issueis now also emitted forauthorization_code(it was only emitted forclient_credentials).Docs: configuration, security ("A sign-in never outlives 24 h"), clients (ChatGPT / Le Chat re-enter their TOTP once a day, no prompt in between), yaml example.
Notes
named_tokens(the dashboard revokes them by prefix). Hashing them too is doable but out of scope here.resourceis not tracked by the token endpoint today, so there is no audience to carry over on refresh./oauth/authorizeconfirmation screen shows "Access expires" at +24 h, which is exactly the sign-in lifetime again.auth.token.refresh; no dedicated audit event.Upgrade
No migration step: the new tables are created on first start and existing named tokens are kept. Access tokens issued by the previous version were never persisted, so clients connected before the upgrade have to sign in once after it; from then on restarts within the day are transparent. Rows written by the earlier builds of this branch (no
family_expires_at, families that lived as long as they refreshed) get the column on load: each family is dated from its first token and its access and refresh tokens are capped at that + 24 h, so a 20-day-old session is over on first start.Tests
tests/test_oauth_refresh.py(39 tests): refresh OK, rotation, replay revokes the family (also across a restart), other families untouched, wrong client refused, unknown/expired refresh refused, TTL bounds (0, negative, > 24 h rejected byTokenStore, the loader and the wizard input), sign-in capped at 24 h (refresh at 24 h + ε fails after a rotation a minute earlier; access tokenexpires_inshrinks to the family end;client_credentialsbearer unchanged; short TTL acts as an idle timeout only; family end survives a restart; legacy rows bounded on load), retry grace (same pair within the window, strict after it and after a restart, raw pair dropped once the window closes), persistence across a newTokenStoreon the same db, hashed storage, expired rows purged, in-place upgrade of an oldtokens.db, client revoke, end-session revoke, startup pruning, metadata, YAML parsing and wizard round-trip.tests/test_client_store_reload.py(9 tests): CLI revoke and CLI create seen by a running store,on_clients_removedfired with the survivors (once, not on own writes, not on additions), server-side save no longer clobbers a CLI change, broken and missing file keep the current set, file created after startup picked up.ruff checkclean.validate-configwithrefresh_token_ttl: 2592000fails withserver.refresh_token_ttl: must be between 1 and 86400 seconds (24 h),servewithBEACONMCP_REFRESH_TOKEN_TTL=0exits 1 with the matching message.beaconmcp serve:client_credentials200,/mcp200,beaconmcp auth revokefrom the CLI, then/mcp401invalid_tokenand/oauth/token401invalid_clienton the very next calls;oauth_tokensandrefresh_tokensemptied intokens.db.