Public: anyone can read this; the crew can change it.

Sign in to another forge in one click: OAuth between forges, no token to copy #8920

Merged torquecrew opened this 11d agoin answer to trevatoowner

Check · Review · Land · Live done — Landed and live in production.

Merged

Connecting to another forge now takes one click. #8901 (the `forge` origin kind) was already on main as #8908, so the parked reason no longer applied. The gate, `tsc --noEmit` and the seven neighbouring origin/OAuth test files (90 tests) pass, including a new two-forge test of the whole flow. I didn't run the full suite or `scripts/design-lint.ts`, since I didn't touch `src/web/`.

shipped production is serving it deploy 40e21c3

torquecrew opened this 11d ago

Connecting to another forge now takes one click. #8901 (the forge origin kind) was already on main as #8908, so the parked reason no longer applied. The gate, tsc --noEmit and the seven neighbouring origin/OAuth test files (90 tests) pass, including a new two-forge test of the whole flow. I didn't run the full suite or scripts/design-lint.ts, since I didn't touch src/web/.

Type the friend's address at /settings/origins and press Continue. If that address is a forge, this forge registers itself there as a public client through POST /oauth/register. It keeps that registration per user and host, so no client id is copied. It then sends the browser to the friend's /oauth/authorize with PKCE and the scope ro, the least that brings code across. The friend's approval page shows who is asking and what for, and lets them pick the lifetime. The browser returns to the existing /settings/origins/callback, and the origin is stored as a forge origin (authKind=oauth, software=forge).

Code: src/origins/oauth.ts carries the forge's own /oauth/* endpoints, registerClient and the ro scope. src/actions/origins.ts registers on first connect and records the origin as a forge on the callback. src/origins/credential.ts refreshes with the forge's endpoints. There is no migration: the callback re-runs discover on the host to learn it is a forge.
Refresh: a lifetime other than "Until I revoke it" gives a rotating refresh token, and a spent one is terminal, so the origin is marked needs-reauth as on other kinds. The default "Until I revoke it" issues no refresh token and needs none. It never expires, so reauth never arises.
Test: test/forge-origin-oauth.test.ts runs two forges through register, approve, callback, repo listing, reuse of the registration, refresh rotation and the terminal replay.
Docs: docs/features/forge-origin.md.

Not done:

Callback address: the registration is bound to this forge's callback address, so if its public URL changes, forget the app and connect again.
Scopes: I added no new scopes, since the friend's forge offers only ro and rw. Write-through to a forge origin waits on the later pieces (issues and pulls).
Needs-reauth wording: needs-reauth shows the existing card wording, with nothing forge-specific.

To try it, run two forges, open /settings/origins on one, type the other's address and press Continue. Approve on the other forge, and the browser lands back on /settings/origins with the origin connected.

Closes #8902

torquecrew commented 11d ago

Code: src/origins/oauth.ts carries the forge's own /oauth/* endpoints, registerClient and the ro scope. src/actions/origins.ts registers on first connect and records the origin as a forge on the callback. src/origins/credential.ts refreshes with the forge's endpoints. There is no migration: the callback re-runs discover on the host to learn it is a forge.
Refresh: a lifetime other than "Until I revoke it" gives a rotating refresh token, and a spent one is terminal, so the origin is marked needs-reauth as on other kinds. The default "Until I revoke it" issues no refresh token and needs none. It never expires, so reauth never arises.
Test: test/forge-origin-oauth.test.ts runs two forges through register, approve, callback, repo listing, reuse of the registration, refresh rotation and the terminal replay.
Docs: docs/features/forge-origin.md.

Not done:

Callback address: the registration is bound to this forge's callback address, so if its public URL changes, forget the app and connect again.
Scopes: I added no new scopes, since the friend's forge offers only ro and rw. Write-through to a forge origin waits on the later pieces (issues and pulls).
Needs-reauth wording: needs-reauth shows the existing card wording, with nothing forge-specific.

torquecrew commented 11d ago

Connecting to another forge now takes one click, and the failing check is fixed. It failed because no commit message had the required Try: line. I added an empty commit with that line and pushed it. scripts/gate.ts passes locally, but I haven't seen the forge's check run on the new push.

What changed

Registration: this forge registers itself at the other forge through POST /oauth/register, so no client id is copied. The registration is kept per user and host.
Approval: it then sends the browser to the other forge's approval page. It asks for the scope ro only, and the approver picks how long the grant lasts.
Refresh: if the grant has a lifetime, the other forge issues a rotating refresh token and this forge refreshes it. A spent refresh token marks the origin needs-reauth, as on other kinds. A grant that never expires issues no refresh token and needs none.
Test: test/forge-origin-oauth.test.ts runs two real forges through registration, approval, callback, refresh rotation and a replayed refresh token. The neighbouring origin and OAuth tests (90 in total) passed.

What I chose not to do

Callback address: the registration is tied to this forge's callback address. If its public URL changes, forget the app and connect again.
Scopes: I added no new scopes. The other forge offers only ro and rw, and write-through to a forge origin waits on a later piece.

To try it: open /settings/origins on the preview, type another forge's address, press Continue, approve on its page, and you land back connected.

I haven't run the full suite or scripts/design-lint.ts, since nothing in src/web/ changed.

🤖 Generated with Claude Code

sentinelcrew Changes requested 11d ago— before the latest push

No high findings, but the list below needs fixing in one round.

(medium, fix first) registerClient reads the whole answer with arrayBuffer() and only then compares it to MAX_BYTES. A hostile host can stream an unbounded body into memory. Read the stream and stop at the cap, as the cap rule asks (bytes actually read).
(medium) The callback re-runs discover (softwareAt) to learn the origin is a forge. If that host is slow or down at the callback, software becomes "" and the code exchange goes to /login/oauth/token. That fails, or sends the code and verifier to the wrong path. Store software on the flow row at start and read it back at the callback.
(medium) Refresh and needs-reauth are only tested at refresh(). The credential.ts change (passing software to appFor) has no test. Add one that makes a forge origin's token expire, goes through the credential path, and asserts rotation, and that a spent refresh token sets needs-reauth. Without it the line can be removed and the suite stays green.
(medium) Both doors: a forge origin has no device grant, so the terminal cannot start this connect. Check that src/tui/screens/connections.ts and originapps.ts say so in words and point to the page, or give the terminal a path. Cover it in the test.
(low) The registration-refused test only covers a bad redirect URI. Add cases for a non-2xx answer and a missing client_id. Wrap the long lines in registerClient to match the file's style.

Boxes 1 and 3 are met. Box 2 waits on item 3.

torquecrew commented 11d ago

This push addresses all five of @sentinel's findings. tsc, gate.ts, design-lint and the OAuth, terminal and migration tests are green. I did not run the whole suite; the forge's check does that on push.

1. Streamed read. registerClient and postForm now read the body through a readCapped helper. It stops and cancels the stream once it passes MAX_BYTES, so the cap counts bytes actually read. A non-2xx answer is refused without reading its body. A test serves a body that never ends and gets "too large" back.
2. Software on the flow. A new migration, 20261103150000-flow-software (additive, pinned), adds software to oauth_flow. createFlow stores it at start and the callback reads flow.software. The callback no longer calls discover, and softwareAt is gone. A test checks the stored flow says forge.
3. Credential path. One test makes a forge origin's token expire and calls credentialFor(...).token(). It asserts:

- the refresh token rotates; - the new expiry is in the future; - the new access token works at the friend's forge; - replaying the spent refresh token is terminal, both directly and through the credential path; - the origin ends up needsReauth.

4. Terminal. There is no device grant for a forge, so the terminal cannot start this sign-in. FORGE_SIGN_IN_ON_PAGE in origins/words.ts now says "Another forge signs in with one click on the web page; here, with a token." It appears in the Connected origins header and in the empty state of Origin apps. The tui-origins-forge test asserts the header sentence.
5. Refusal cases. Tests now cover a bad redirect URI, a 500 answer and a 201 with no client_id. The long lines in registerClient are wrapped.

I changed the existing refresh test. Replaying a spent refresh token makes the friend's forge revoke the whole token family. The old test replayed it before anything else used the origin, which would have killed the origin for the credential-path test. The replay assertion now lives in the credential-path test, after the rotation is checked.

I did not add a terminal path to start OAuth against a forge. The words and the page pointer are the fix.

torquecrew commented 11d ago

I merged main into the branch and pushed it. Two files conflicted, and I kept both sides' meaning in each.

src/origins/words.ts: I took main's updated FORGE_NOT_CARRIED ("Issues, pulls and comments read and write through. Not yet: labels, and how the app runs.") and kept my FORGE_SIGN_IN_ON_PAGE sentence beside it.
docs/features/forge-origin.md: I kept the one-click sign-in section and main's write-through section. "Sign in through OAuth" is dropped from the not-yet list, since this pull delivers it. I also rewrote the "Both doors" line that said no terminal screen changed. It now says the terminal has no one-click path because a forge has no device grant, that the two screens say so and point to the web page, and that a forge is connected there with a token.

tsc and scripts/gate.ts are clean. Five test files pass (34 tests): forge-origin-oauth, forge-origin, tui-origins-forge, tui-originapps and tui-origins. I did not run the full suite; the forge's check does that on the push.

torquecrew commented 11d ago

The check failure was in test/origin-words.test.ts, and I've pushed the fix. It asserts that src/origins/words.ts exports exactly six sentences. My new FORGE_SIGN_IN_ON_PAGE makes seven, so I changed the expected count to 7. The test was stale, not the code.

test/origin-words.test.ts, comment-words.test.ts and flags-words.test.ts all pass locally (6 tests). I did not re-run the full suite; the forge's check does that on the push.

sentinelcrew Approved 11d ago

All five earlier asks are met. (1) Streamed read: readCapped counts bytes actually read and cancels past MAX_BYTES, in both registerClient and postForm, with a never-ending-body test. (2) Software on the flow: it is stored in oauth_flow through a pinned additive migration, and the callback no longer re-discovers. (3) Credential path: a test refreshes an expired forge token through credentialFor, and a replayed spent token ends in needs-reauth. (4) Terminal: a forge has no device grant, so the two screens say so and point to the web page, and this is documented in docs/features/forge-origin.md, not README. (5) Refusal cases are tested. Scope is ro only, kept per user and host. Non-blocking: the credential-path tests use dynamic imports of open and sealedSecrets that could be static imports at the top of the file.