Skip to content

Commit 445ecea

Browse files
aspiersclaude
andcommitted
docs(design): spec full merge to one origin and one process
Add single-process-merge.md covering the maximal collapse: mount the auth-service Express app onto pds.app under /auth, delete the HMAC-signed /oauth/epds-callback and /_internal/* HTTP hops in favour of in-process calls, and retire the second process. Documents the real integration hazards (better-auth body-parser ordering, /static and favicon collisions, trust-proxy/CSRF/error-handler scoping), the now-live npm peer-dependency risk, and a reversible phased rollout gated on a peer audit and a callback-core extraction refactor. Cross-linked from single-domain-migration.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 800a3e0 commit 445ecea

2 files changed

Lines changed: 190 additions & 1 deletion

File tree

docs/design/single-domain-migration.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ This matters for both benefits and risks, so fix the terms up front:
3838
- **Merge the _processes_ (optional, later).** Fold auth-service into the
3939
pds-core Node process. Adds the "fewer privileged endpoints" benefit in full
4040
(calls become in-process) but is the only variant that can surface
41-
dependency conflicts. Treat as a separate, later decision.
41+
dependency conflicts. Treat as a separate, later decision — specced in full
42+
in [single-process-merge.md](single-process-merge.md).
4243

4344
## Positive benefits (beyond deleting costs)
4445

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
# Full Merge: One Origin, One Process
2+
3+
**Status:** Proposal (tracked in [#200](https://github.com/hypercerts-org/ePDS/issues/200))
4+
5+
This document specifies the **maximal** collapse of the two-service
6+
architecture: not just a single origin (covered in
7+
[single-domain-migration.md](single-domain-migration.md)) but a **single Node
8+
process** hosting both the PDS and the auth UI on one Express app.
9+
10+
Read [single-domain-migration.md](single-domain-migration.md) first — this doc
11+
assumes the single-origin work is done or done concurrently, and only adds the
12+
process-collapse layer on top. The "Two levels of merge" section there defines
13+
the origin-merge vs. process-merge distinction; this is the process-merge half,
14+
specced in full.
15+
16+
## Why go all the way to one process
17+
18+
Single-origin already dissolves every cross-origin cost. Merging the _processes_
19+
adds a distinct set of wins that single-origin alone cannot deliver:
20+
21+
- **The HMAC callback boundary disappears.** `/oauth/epds-callback` exists so
22+
auth-service can prove to pds-core that a redirect came from a legitimate auth
23+
flow, over the wire, signed with `EPDS_CALLBACK_SECRET`. In one process the
24+
auth flow can call the code-issuance path **directly** as a function — no
25+
HMAC, no secret to rotate, no signature-verification code, no replay window.
26+
- **The internal HTTP lookups disappear.** `/_internal/account-by-email`,
27+
`/_magic/check-email`, and the `/_internal/ping-request` keepalive become
28+
in-process function calls against the same objects (`pds.ctx.accountManager`,
29+
`provider.requestManager`) that pds-core already holds. No auth, no JSON
30+
round-trip, no drift between caller and callee.
31+
- **One deploy unit, one health check, one log stream, one restart.** True
32+
operational singularity, not just one origin.
33+
- **Shared in-memory state becomes trivial.** Today the PAR-keepalive
34+
(`ping-request`) and session-reuse logic reason about state across an HTTP
35+
boundary; in-process they share the live objects.
36+
37+
The cost of going this far — and the reason single-origin is the safer default —
38+
is the integration hazards in the next section and the npm-version risk below.
39+
40+
## Target architecture
41+
42+
Both packages already make this feasible:
43+
44+
- `packages/auth-service/src/index.ts` exposes `createAuthService(config)` which
45+
**returns a mountable `express.Express`** plus its context — it does not
46+
hardwire a server. The `app.listen()` is a thin wrapper below the factory.
47+
- `packages/pds-core/src/index.ts` mounts all its middleware onto **`pds.app`**,
48+
the upstream PDS's own Express instance.
49+
50+
So the merge is: build the auth app (or a router equivalent), **mount it onto
51+
`pds.app` under `/auth`**, and delete auth-service's own `listen()`. One process,
52+
one Express tree, one port.
53+
54+
```
55+
pds.app (single Express instance, single listen)
56+
├── (upstream PDS routes: xrpc, /oauth/authorize, /oauth/par, …)
57+
├── asMetadataOverride → authorization_endpoint = <host>/auth/oauth/authorize
58+
├── /auth/oauth/authorize → auth UI (was auth-service /oauth/authorize)
59+
├── /auth/api/auth/* → better-auth handler
60+
├── /auth/account/* → account settings
61+
├── /auth/complete, /auth/choose-handle, …
62+
└── (code issuance called in-process, no /oauth/epds-callback HTTP hop)
63+
```
64+
65+
## Integration hazards (the real work)
66+
67+
These are concrete Express-level collisions between the two apps. Each must be
68+
resolved deliberately; they are why this is a spec and not a one-line mount.
69+
70+
### 1. Body-parser ordering — the sharp edge
71+
72+
auth-service mounts better-auth **before** `express.json()`:
73+
74+
```ts
75+
// auth-service/src/index.ts
76+
app.all('/api/auth/*', toNodeHandler(betterAuthInstance)) // BEFORE json()
77+
app.use(express.urlencoded({ extended: true }))
78+
app.use(express.json())
79+
```
80+
81+
better-auth parses its own request bodies and breaks if a JSON parser consumes
82+
the stream first. pds-core's app already has its own body-parsing. **On the
83+
merged app, `/auth/api/auth/*` must be registered ahead of any global
84+
`express.json()`**, or scoped so the global parser skips that path. This is the
85+
single most likely source of a silent runtime break. Mount order is
86+
load-bearing.
87+
88+
### 2. Duplicate `/static` and favicon
89+
90+
Both apps do `app.use('/static', express.static(publicDir))` and serve a
91+
favicon. Merged, these collide. Namespace the auth assets under `/auth/static`
92+
(and `/auth/favicon`) so the two static roots don't shadow each other.
93+
94+
### 3. `trust proxy`, CSRF, rate-limit — set once, not twice
95+
96+
Both set `app.set('trust proxy', 1)` and install their own CSRF + rate-limit
97+
middleware. On one app: set `trust proxy` once; scope the auth CSRF/rate-limit
98+
middleware to the `/auth` mount (`app.use('/auth', csrfProtection(...))`) so
99+
they don't wrap PDS routes that have their own protections.
100+
101+
### 4. Error / not-found handlers
102+
103+
auth-service ends with `notFoundHandler` + `errorHandler` as terminal
104+
middleware. Terminal handlers mounted globally would swallow PDS routes. Scope
105+
them to the `/auth` router, not the merged app root.
106+
107+
### 5. CSP middleware convergence
108+
109+
Both set CSP per-route (auth via `security-headers.ts`, pds-core by rewriting
110+
upstream's CSP). These already coexist per-response, so no origin change is
111+
needed — but confirm the auth CSP middleware only fires on `/auth/*` once
112+
co-mounted, and does not leak `unsafe-inline` onto PDS routes.
113+
114+
### 6. Shared context / DB handles
115+
116+
auth-service builds `AuthServiceContext` (its own `db`, `emailSender`); pds-core
117+
holds `pds.ctx`. Merged, decide whether they share one SQLite handle or keep
118+
separate connections to the same files. The migration plan already notes
119+
`account.sqlite` is the single source of truth for email→DID; in-process, the
120+
direct-lookup replacements for `/_internal/account-by-email` read it through
121+
`pds.ctx.accountManager` rather than a second connection.
122+
123+
## Replacing the HMAC callback in-process
124+
125+
Today: auth flow → HMAC-signed 303 → `pds-core` `/oauth/epds-callback`
126+
`provider.requestManager.setAuthorized(...)` issues the code.
127+
128+
Merged: the auth-complete handler calls the same `setAuthorized` path
129+
**directly**. Concretely:
130+
131+
- Extract the body of the `/oauth/epds-callback` handler (the part that calls
132+
`requestManager.setAuthorized` / `createAccount`) into a plain function on a
133+
shared module, taking typed args instead of a signed request.
134+
- The auth `/auth/complete` handler calls that function directly.
135+
- `EPDS_CALLBACK_SECRET` and the signature-verification middleware are deleted.
136+
- **Keep the HTTP `/oauth/epds-callback` route only if** a transition period
137+
needs old auth-service instances to still call it; otherwise remove it.
138+
139+
This is the highest-value deletion in the whole merge and also the most
140+
security-sensitive change — the HMAC existed to stop an attacker forging a
141+
callback. In-process there is no forgeable wire boundary, but the review must
142+
confirm no other caller (including a partially-migrated deployment) can still
143+
reach the code-issuance path unauthenticated.
144+
145+
## npm version clash — now in play
146+
147+
Unlike single-origin, one process forces **one resolution** of every shared or
148+
peer dependency across `better-auth` and the `@atproto/*` stack.
149+
150+
- The repo is a pnpm workspace with non-flat `node_modules`, so today the two
151+
packages resolve independently. Merging their runtime removes that isolation.
152+
- Current overlap is small: the only shared runtime dep is `express ^4.18.2`
153+
(aligned). The realistic future conflict is `express` majors, or a transitive
154+
peer both pull (e.g. a differing `zod`/`@types/node` peer requirement between
155+
better-auth and an @atproto package).
156+
- **Mitigation:** before merging, run a dedup/peer audit (`pnpm why express`,
157+
`pnpm dedupe --check`) and pin the shared set. Treat any unresolvable peer
158+
conflict as a blocker that keeps the two processes separate (fall back to the
159+
origin-merge-only outcome).
160+
161+
## Phased rollout (extends the single-domain phases)
162+
163+
The single-origin phases (see [single-domain-migration.md](single-domain-migration.md))
164+
come first. This process-merge adds:
165+
166+
1. **Peer-dependency audit.** `pnpm why` / `pnpm dedupe --check` across the
167+
merged dependency set; pin shared deps. Gate: no unresolvable peer conflict.
168+
2. **Extract the callback core.** Refactor `/oauth/epds-callback`'s issuance
169+
logic into a directly-callable function; leave the HTTP route delegating to
170+
it (no behaviour change yet). Ship and test this alone.
171+
3. **Mount auth onto `pds.app`.** Convert `createAuthService` into a router
172+
mounted at `/auth` on the PDS app; resolve hazards 1–6 above. Keep
173+
auth-service's standalone `listen()` behind a flag for rollback.
174+
4. **Switch `/auth/complete` to the in-process call.** Route code issuance
175+
through the extracted function; stop signing/sending the HMAC callback.
176+
5. **Delete** `EPDS_CALLBACK_SECRET`, the callback signature middleware, the
177+
`/_internal/*` + `/_magic/*` HTTP endpoints, and auth-service's standalone
178+
server. Retire the second process.
179+
180+
Each step is independently shippable and reversible until step 5.
181+
182+
## Recommendation
183+
184+
Do the single-origin merge first and independently — it captures most of the
185+
value at a fraction of the risk. Treat the process merge as a **follow-on**,
186+
gated on the peer-dependency audit (step 1) and the callback-extraction refactor
187+
(step 2), both of which are worth doing on their own merits regardless of whether
188+
the final process collapse happens.

0 commit comments

Comments
 (0)