grain.social/grain

Switch repository

Loading…

What changes

Web login. grain-serve is a confidential atproto OAuth client of each account's PDS, as knotless-serve's pages are. POST /oauth/login ({handle} or {create: true}, and back) pushes the request with PKCE and a DPoP key and answers the approval page; GET /oauth/callback redeems the code, keeps the session server-side and sets the cookie; POST /oauth/switch, POST /oauth/logout and GET /oauth/session cover a browser logged in to several accounts (up to ten, as hatk kept). Sign-up goes to GRAIN_SIGN_UP_PDS, https://selfhosted.social unless set, as the web's "Create one" does today.

The client is hatk's. Its ID is GRAIN_PUBLIC_URL/oauth-client-metadata.json, the redirect /oauth/callback, the key at /oauth/client-jwks.json under hatk's key ID pds-client-auth-key, and the metadata registers hatk's scope string word for word (login::REGISTERED, including the space, pool and group scopes zds and blacksky hold a request to). The login asks for grain::oauth::SCOPE, hatk's production grainScopes. A PDS binds each session to the client and its key ID, so this is what lets the next pull carry hatk's sessions over.

Storage. Index migration V7: oauth_key (private JWKs by key ID: the client key pds-client-auth-key, irreplaceable, as _oauth_keys is, and the cookie key web-cookie) and web_session (each account's grain::session::Session as JSON, and since, when its login was first kept). Each key is made on first start if the index has none.

The cookie. grain_session, a JWT naming the account, signed by web-cookie, a key of its own that is never published in the JWKS and never sent to a PDS (so a JWT the client key signs, which every PDS sees, is no cookie; tested), and that the import does not carry: HttpOnly; SameSite=Lax; Path=/; Max-Age 90 days, Secure when the public URL is https. It names no one once its session is gone, if it was signed before the session's since (so logging out ends every cookie for the account, in every browser), or while the account is taken down. A login is answered only in the browser holding the grain_login cookie naming its state, so a link to someone else's approval logs no one in.

CSRF. Login, logout, switch, and every procedure the cookie authenticates (viewer::writer, used by updateSeen now) require Origin to be the public URL; SameSite=Lax already keeps the cookie off cross-site POSTs.

Viewer. Queries take the cookie as the viewer when no Authorization is sent; a service token still wins, and the apps' path is unchanged.

Refresh. login::session(app, did) returns a session good for the call, refreshing one that expires within a minute under a per-account lock and re-reading the row after the wait, so concurrent requests never spend the same refresh token twice. A refresh the authorization server refuses (oauth::Refused, now a typed error in grain::oauth) ends the session; a network failure keeps it.

All outbound calls of the login go through Ctx::guarded, since the PDS and authorization server come from whatever a DID document says.

Why

The plan's web model: server-held session plus signed cookie, as knotless does it, so web writes (next pulls) go through grain-serve with that session. Sessions live in the index rather than knotless's per-account files so that the nightly backups of the database carry them, as they carry hatk's today.

How it was checked

crates/grain-serve/tests/login.rs runs the round trip against the reference PDS: grain-serve behind harness/tls-proxy.mjs at an https public URL the PDS fetches the metadata and JWKS from, harness/authorize.mjs approving (new --stop flag: stop at the redirect back and print it, so the test comes back to the callback with its own cookie jar). It covers metadata and JWKS, the origin check, an unknown handle, cookie flags, a callback from another browser and a replayed one, the cookie as viewer for a query and for updateSeen (refused without the site's Origin), two accounts in one browser, switching, logout (old cookies dead, even after logging in again), a refused approval, sign-up, a takedown, concurrent refresh (one refresh, both callers get it, the session works at the PDS), and a spent refresh token ending the session. Unit tests cover the store, JWK round trip, cookie parsing and the back-path check. fmt, clippy and the harness coverage gate are left to the checks.

Shortcomings

• Sessions do not survive cutover. Code in crates/ reads nothing in hatk's shape at runtime (the standing rule), so hatk's __hatk_session cookies are not read, and with no cookie a carried session is unreachable. Every web user logs in once more after cutover; the client ID, redirect, key ID and (once imported, in the next pull) the client key stay hatk's, so PDSes see the same client.
• hatk's OAuth issuer side (grain-native://app) is not ported, by decision: the apps move to their own PDS.
• A refresh that finishes after a logout keeps nothing (Index::refresh_web_session updates only a row that is there), per the review.
• Conditional space scopes (hatk adds spaceScopes and the pool/group scopes for a PDS that serves spaces) are not asked for yet; they come with private galleries and pools. They are registered, so asking for them later needs no metadata change.
• Read answers carry no Vary: Cookie / Cache-Control: private; nothing caches them now, but a cache in front would need it.
• A second login for an account already logged in (another browser) replaces the tokens without revoking the old grant; it expires on its own.
• Rotating web-cookie signs every browser out; rotating the client key voids every PDS session. Each can change without the other.

Review

The design and the security-sensitive parts: crates/grain-serve/src/login.rs (cookie validity in held, same_site, session's refresh), then viewer.rs, index/sessions.rs and the V7 migration.

🤖 Generated with Claude Code

This pull makes grain-serve log the web in as a confidential OAuth client with hatk's client ID, key ID and scope. It keeps sessions in the index (V7), sets a signed cookie, requires Origin on cookie-authenticated writes, and refreshes tokens under a per-account lock.

I could only read login.rs and index/sessions.rs in full. The diff is about 1,770 lines and I could not open the saved copy, so I did not read viewer.rs, the migration, the harness changes or the tests. I found nothing wrong in the parts I read.

• Cookie validity: held rejects bad signatures and expired cookies (verify_jwt checks exp). It also rejects cookies signed before the account's since, cookies whose session is gone, and takedowns.
• Login binding: the grain_login cookie ties a callback to the browser that started it.
• back_to: it accepts only same-site paths.
• Refresh: it re-reads the row after taking the lock and ends the session only on Refused.
• since: a repeat login keeps the old since, so logging in from a second browser does not kill the first browser's cookie.

Must fix: the check workflow failed. vp check reports a formatting issue in harness/authorize.mjs, which this pull changes (the new --stop flag). Run vp check --fix and push. The rust, harness and test checks had not finished when I looked, so I have not seen their results. Per AGENTS.md the harness coverage gate must also pass.

Minor, not blocking: in session, a refresh that is in flight when the user logs out finishes with keep_web_session. That re-inserts the row after end_web_session deleted it. Old cookies stay dead because the new since is later than their iat. The refreshed grant is still left stored for an account that logged out, and a fresh login would pick it up. Consider skipping the write, or ending the grant, if the row was deleted while the refresh ran.

This pull makes grain-serve log the web in as hatk's OAuth client. It keeps sessions in the index (V7), sets a signed cookie, requires Origin on cookie-authenticated writes, and refreshes tokens under a per-account lock.

Round 1 asked for the harness/authorize.mjs formatting fix. The check workflow now passes on d7d2ab1. The new commit also adds tests for failed logins and failed refreshes. rust, test and harness were still queued or running when I looked, so I have not seen their results. They need to pass before merging, including the harness coverage gate.

Not addressed, and not blocking: a refresh that is in flight when the user logs out still re-inserts the row. login.rs around line 575 calls keep_web_session after end_web_session deleted it, and the upsert recreates the row with a new since. Old cookies stay dead, but the refreshed grant remains stored and a later login would pick it up. Consider skipping the write when the row is gone after the refresh.

This pull makes grain-serve log the web in as hatk's OAuth client. It keeps sessions in the index (V7), sets a signed cookie, requires Origin on cookie-authenticated writes, and refreshes tokens under a per-account lock.

Round 2 left one thing open: a refresh in flight when the user logs out re-inserted the session row. That is fixed. session now calls the new Index::refresh_web_session, which updates only a row that still exists. If the row is gone, the call returns ended() and a logout stays a logout. The store test covers both cases: a refresh keeps since, and a refresh over a missing row writes nothing. The other changes in this commit only reword comments.

Before merging, check, rust, test and harness must all pass. All four were still queued when I looked, so I have not seen any result for ea052f5. The harness coverage gate is one of them.

Carried forward: the change this pull makes at 801cc65fbf716b46620e7200aa9a7ae17b19e7f6 is the one reviewed at ea052f5405c429fe38308d14edd42edbefe9b008, so that review stands. What is new comes from its base.

This pull makes grain-serve log the web in as hatk's OAuth client. It keeps sessions in the index (V7), sets a signed cookie, requires Origin on cookie-authenticated writes, and refreshes tokens under a per-account lock.

The only change since round 3 is that cookies are now signed with their own key, web-cookie. It is made on first start beside pds-client-auth-key, never put in the JWKS, and never sent to a PDS. Web::new loads both keys from oauth_key. The test checks that the two keys differ and that the cookie key's public part is not in the JWKS. This fits the design: a token the client key signs is no cookie, and either key can be rotated alone. Nothing I read is wrong.

On 8042129, check and test pass. rust and harness were still running when I looked, so I have not seen their results. Both must pass before merging, including the harness coverage gate.