Stage M2
This commit is contained in:
@@ -0,0 +1,73 @@
|
||||
# ADR-0015: Frontends reach the API through their own origin
|
||||
|
||||
- **Status:** Accepted
|
||||
- **Date:** 2026-08-12
|
||||
|
||||
## Context
|
||||
|
||||
The refresh token is delivered as an httpOnly cookie (ADR-0008) — that is what
|
||||
stops an XSS from stealing the credential that mints new sessions.
|
||||
|
||||
Cookies are governed by `SameSite`. If the browser calls `api.example.com` from
|
||||
`admin.example.com`, that is a cross-site request, and the cookie only rides
|
||||
along with `SameSite=None`. `SameSite=None` requires `Secure`, which requires
|
||||
HTTPS. Local development runs on plain HTTP, so the cookie would simply never be
|
||||
set — presenting as a login that "succeeds" and then immediately forgets you.
|
||||
|
||||
The alternatives are all worse: put the refresh token in `localStorage` (readable
|
||||
by any script, which defeats the entire httpOnly design), run local development
|
||||
over self-signed HTTPS (friction on every machine and in CI), or accept that dev
|
||||
and production authenticate differently (the class of bug you only find in
|
||||
staging).
|
||||
|
||||
## Decision
|
||||
|
||||
**The browser always calls the API on the same origin as the page it is on.**
|
||||
|
||||
- Production: Nginx already routes `/api/` to the API container on the same host
|
||||
— this was in the topology from milestone 0.
|
||||
- Development: a Next.js `rewrites()` entry maps `/api/:path*` to
|
||||
`API_INTERNAL_URL`, reproducing that topology exactly.
|
||||
- `browserApi` is therefore created with `baseUrl: ''`, and `HttpClient`
|
||||
resolves a relative base against `window.location.origin`.
|
||||
|
||||
Server-side rendering is unaffected: it calls `API_INTERNAL_URL` directly over
|
||||
the internal network, because there is no cookie and no browser involved.
|
||||
|
||||
The cookie is then first-party, `SameSite=Lax`, `httpOnly`, `Secure` in
|
||||
production, and scoped to `path=/api/v1/auth` so it is attached to two endpoints
|
||||
rather than every API call.
|
||||
|
||||
## Consequences
|
||||
|
||||
Development and production share one authentication topology, so a cookie
|
||||
problem is reproducible locally instead of appearing first in staging.
|
||||
|
||||
CORS effectively disappears for browser traffic — the requests are same-origin.
|
||||
The API's CORS config remains for non-browser and tooling access.
|
||||
|
||||
Storefront and admin get separately named cookies (`sport_refresh`,
|
||||
`sport_admin_refresh`). Sharing a name would mean signing into the admin
|
||||
silently replaced a customer session in the same browser.
|
||||
|
||||
The costs, stated plainly:
|
||||
|
||||
- One extra network hop in development (browser → Next → API). Irrelevant
|
||||
locally, and absent in production where Nginx was already the front door.
|
||||
- The frontends now have a route namespace (`/api/*`) they must not use for
|
||||
their own route handlers. Worth noting in review; neither app has any.
|
||||
- `API_INTERNAL_URL` becomes required for the dev rewrite, not just for SSR.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**Refresh token in `localStorage`.** Removes the cookie problem entirely and
|
||||
removes the security property with it — any injected script can read it.
|
||||
Rejected outright.
|
||||
|
||||
**`SameSite=None; Secure` with HTTPS in development.** Correct, but forces every
|
||||
developer and CI job to trust a local certificate. Rejected as friction that
|
||||
buys nothing production does not already provide.
|
||||
|
||||
**A dedicated auth subdomain with a parent-domain cookie.** Works in production,
|
||||
but `localhost` has no usable parent domain, so development still diverges.
|
||||
Rejected for the same reason as above.
|
||||
@@ -23,6 +23,7 @@ An ADR is immutable once accepted. If a decision changes, add a new ADR that sup
|
||||
| [0012](./0012-postgresql-full-text-search-before-a-dedicated-search-engine.md) | PostgreSQL full-text search before a dedicated search engine | Accepted |
|
||||
| [0013](./0013-content-translations-in-typed-tables-ui-strings-in-message-catalogs.md) | Content translations in typed tables, UI strings in message catalogs | Accepted |
|
||||
| [0014](./0014-denormalised-price-projection-on-product.md) | A denormalised price/stock projection on Product | Accepted |
|
||||
| [0015](./0015-frontends-reach-the-api-through-their-own-origin.md) | Frontends reach the API through their own origin | Accepted |
|
||||
|
||||
## Decisions deliberately NOT recorded yet
|
||||
|
||||
|
||||
+122
-34
@@ -240,7 +240,65 @@ consumer transient too, which for `PrismaService` would mean a second connection
|
||||
|
||||
---
|
||||
|
||||
## 9. Internationalisation
|
||||
## 9. Authentication and sessions
|
||||
|
||||
See [ADR-0008](./adr/0008-short-access-tokens-rotating-refresh-tokens-separate-audiences.md) for
|
||||
the decision and [ADR-0015](./adr/0015-frontends-reach-the-api-through-their-own-origin.md) for
|
||||
the transport.
|
||||
|
||||
### The two tokens
|
||||
|
||||
| | Access token | Refresh token |
|
||||
| ------------ | ---------------------------------------------- | --------------------------------------- |
|
||||
| Form | JWT | Opaque random bytes |
|
||||
| Lifetime | 15 minutes | 30 days |
|
||||
| Stored where | Client memory only | httpOnly cookie + SHA-256 in `sessions` |
|
||||
| Revocable | No (stateless by design) | Yes |
|
||||
| Carries | userId, audience, type, permissions, sessionId | Nothing — it is a lookup key |
|
||||
|
||||
The refresh token is **not** a JWT. There is nothing for a client to read in it, and because it
|
||||
is checked against a row it can be revoked — which a stateless token fundamentally cannot be.
|
||||
Only its hash is stored, so a database leak yields no usable credential.
|
||||
|
||||
### Rotation and reuse detection
|
||||
|
||||
```
|
||||
login → session A (new family)
|
||||
refresh(A) → session B, A.replacedBy=B (same family)
|
||||
refresh(A) → REUSE: revoke whole family
|
||||
```
|
||||
|
||||
A rotated token that shows up again means either it leaked or the client is broken. Both justify
|
||||
killing the family, which signs out the thief _and_ the legitimate device — deliberately, because
|
||||
the alternative is letting a known-compromised session continue.
|
||||
|
||||
### Audience separation
|
||||
|
||||
Every token carries an audience (`storefront` / `admin`) and each has its own login endpoint and
|
||||
its own cookie name. It is enforced three times: at issuance (the user's type must match), on
|
||||
every request (`AccessTokenGuard`, before permissions are read), and per controller via
|
||||
`@RequireAudience`.
|
||||
|
||||
### Why login is deliberately uninformative
|
||||
|
||||
Unknown email, wrong password, suspended account and wrong audience all return the same message
|
||||
and status, and the unknown-email path burns equivalent CPU so it is not measurably faster.
|
||||
Anything else turns the login form into a user-enumeration oracle.
|
||||
|
||||
Failures are counted twice in Redis: per account (stops guessing one password from many IPs) and
|
||||
per IP (stops spraying one password across many accounts). Only failures count, so a busy
|
||||
legitimate user is never locked out.
|
||||
|
||||
### Known limitation
|
||||
|
||||
A permission change takes effect within one access-token lifetime, not instantly — that is the
|
||||
price of stateless guards with no database read on the hot path. `CACHE_KEYS.revokedSession`
|
||||
exists for an immediate denylist and is deliberately not wired up; it reintroduces exactly the
|
||||
lookup the design removes.
|
||||
|
||||
---
|
||||
|
||||
## 10. Internationalisation
|
||||
|
||||
Two languages, **two mechanisms**, split by who owns the string. Conflating them is the usual
|
||||
way an i18n project ends up half-finished. See
|
||||
@@ -284,7 +342,7 @@ locale-in-path would add a proxy hop and double the route tree for nothing.
|
||||
|
||||
---
|
||||
|
||||
## 10. Naming conventions
|
||||
## 11. Naming conventions
|
||||
|
||||
| Thing | Convention | Example |
|
||||
| --------------------- | ------------------------- | ------------------------------ |
|
||||
@@ -309,7 +367,7 @@ Booleans read as assertions: `isActive`, `hasVariants`, `canRefund`. Money field
|
||||
|
||||
---
|
||||
|
||||
## 11. Configuration and environment
|
||||
## 12. Configuration and environment
|
||||
|
||||
Four `.env` files, each with a committed `.env.example`:
|
||||
|
||||
@@ -336,7 +394,7 @@ Rules:
|
||||
|
||||
---
|
||||
|
||||
## 12. Where premature abstraction must be avoided
|
||||
## 13. Where premature abstraction must be avoided
|
||||
|
||||
Places where the instinct to generalise should be resisted until a second real case appears:
|
||||
|
||||
@@ -358,50 +416,80 @@ Places where the instinct to generalise should be resisted until a second real c
|
||||
|
||||
---
|
||||
|
||||
## 13. Architectural risks to prevent from day one
|
||||
## 14. Architectural risks to prevent from day one
|
||||
|
||||
| Risk | Why it is fatal later | Prevention in place |
|
||||
| ---------------------------------- | --------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------ |
|
||||
| Size/colour as product columns | Cannot express per-combination stock or price; requires re-modelling after orders exist | Variant model ([ADR-0003](./adr/0003-product-and-productvariant-as-separate-entities.md)) |
|
||||
| Float money | Silent discrepancies, unfixable retroactively | Integer minor units ([ADR-0011](./adr/0011-money-as-integer-minor-units.md)) |
|
||||
| Role checks scattered in code | Authorization becomes unauditable; new roles need deploys | RBAC + global guard ([ADR-0007](./adr/0007-rbac-permissions-instead-of-role-checks.md)) |
|
||||
| Admin querying the DB directly | A second write path where authorization is forgotten | No DB driver in admin ([ADR-0004](./adr/0004-the-admin-dashboard-has-no-database-access.md)) |
|
||||
| Module boundary erosion | The monolith becomes unsplittable and untestable | ESLint boundary rules + `public/` barrels |
|
||||
| Order lines joined to live catalog | Historical invoices change when prices do | Snapshot fields on order lines (milestone 5) |
|
||||
| Overselling under concurrency | Real money, real customers, real refunds | `reserved` column + transactional reservation |
|
||||
| Unversioned API | Cannot ship a breaking change once a mobile app exists | URI versioning from request one ([ADR-0005](./adr/0005-uri-based-api-versioning.md)) |
|
||||
| Binaries in PostgreSQL | Backups and replication degrade permanently | Object storage ([ADR-0009](./adr/0009-media-in-s3-compatible-storage-metadata-in-postgresql.md)) |
|
||||
| Single-warehouse inventory | Adding a location later means migrating live stock history | `(variant, location)` keys from the start |
|
||||
| Secrets in the repository | One leak compromises production | Validated env, generated dev secrets, `.env` gitignored |
|
||||
| No request correlation | Production incidents become guesswork | `x-request-id` end to end |
|
||||
| Risk | Why it is fatal later | Prevention in place |
|
||||
| ------------------------------------------ | --------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------ |
|
||||
| Size/colour as product columns | Cannot express per-combination stock or price; requires re-modelling after orders exist | Variant model ([ADR-0003](./adr/0003-product-and-productvariant-as-separate-entities.md)) |
|
||||
| Float money | Silent discrepancies, unfixable retroactively | Integer minor units ([ADR-0011](./adr/0011-money-as-integer-minor-units.md)) |
|
||||
| Role checks scattered in code | Authorization becomes unauditable; new roles need deploys | RBAC + global guard ([ADR-0007](./adr/0007-rbac-permissions-instead-of-role-checks.md)) |
|
||||
| Admin querying the DB directly | A second write path where authorization is forgotten | No DB driver in admin ([ADR-0004](./adr/0004-the-admin-dashboard-has-no-database-access.md)) |
|
||||
| Module boundary erosion | The monolith becomes unsplittable and untestable | ESLint boundary rules + `public/` barrels |
|
||||
| Order lines joined to live catalog | Historical invoices change when prices do | Snapshot fields on order lines (milestone 5) |
|
||||
| Overselling under concurrency | Real money, real customers, real refunds | `reserved` column + transactional reservation |
|
||||
| Unversioned API | Cannot ship a breaking change once a mobile app exists | URI versioning from request one ([ADR-0005](./adr/0005-uri-based-api-versioning.md)) |
|
||||
| Binaries in PostgreSQL | Backups and replication degrade permanently | Object storage ([ADR-0009](./adr/0009-media-in-s3-compatible-storage-metadata-in-postgresql.md)) |
|
||||
| Single-warehouse inventory | Adding a location later means migrating live stock history | `(variant, location)` keys from the start |
|
||||
| Secrets in the repository | One leak compromises production | Validated env, generated dev secrets, `.env` gitignored |
|
||||
| No request correlation | Production incidents become guesswork | `x-request-id` end to end |
|
||||
| Browser-only failures passing every check | Server rendering and curl both succeed while every click fails | Client paths must be exercised in a real browser before a milestone closes |
|
||||
| `onUnauthorized` retrying the refresh call | Unbounded refresh loop hammering the API from the browser | `skipAuthRetry` on all auth endpoints plus a single-flight refresh |
|
||||
| Concurrent 401s each rotating the token | The second rotation reads as token reuse and revokes the family | One shared in-flight refresh promise |
|
||||
|
||||
---
|
||||
|
||||
## 14. Deliberate limitations
|
||||
## 15. Testing that actually catches things
|
||||
|
||||
The catalog and auth milestones both passed lint, typecheck, unit tests, a full build and
|
||||
curl-based API checks — while the admin login button did nothing at all in a browser. Three
|
||||
defects hid in that gap, and all three shared one property: **they only exist in a browser.**
|
||||
|
||||
- `globalThis.fetch` stored on an object and called as a method throws `Illegal invocation`
|
||||
in a browser and works fine in Node. Server rendering and curl could not have caught it.
|
||||
- `onUnauthorized` refreshing through an endpoint that was itself retryable recursed without
|
||||
bound — visible only as a flood of requests in a real network panel.
|
||||
- Next 16 refuses upstream images on private IPs, and reports it with the _same_ message it
|
||||
uses for an unmatched remote pattern. Only the rendered page revealed it.
|
||||
|
||||
The rule this earns: **a milestone touching client behaviour is not done until its
|
||||
interactions have been clicked in a real browser**, with the console and network panel open.
|
||||
Regression tests now cover the first two (`packages/api-client/src/http-client.spec.ts`); the
|
||||
third belongs in an end-to-end test when one exists.
|
||||
|
||||
---
|
||||
|
||||
## 16. Deliberate limitations
|
||||
|
||||
Stated plainly so they are choices rather than oversights.
|
||||
|
||||
**Shipped (M0–M1)**
|
||||
**Shipped (M0–M2)**
|
||||
|
||||
- Catalog reads: products, variants, options, categories, collections, brands, navigation —
|
||||
localised, cached, filtered and faceted.
|
||||
- RBAC enforcement, response envelope, structured logging, media pipeline.
|
||||
- Storefront browsing and PDP in Vietnamese and English.
|
||||
- Storefront browsing and PDP in Vietnamese and English, with per-locale slugs and `hreflang`.
|
||||
- Auth: login, refresh rotation with reuse detection, audience separation, login throttling.
|
||||
- RBAC enforcement end to end, plus user administration and a role viewer in the admin.
|
||||
|
||||
**Not built yet, and why**
|
||||
|
||||
- **No token issuance.** Guards verify and enforce; login, refresh rotation and registration are
|
||||
M2. Every endpoint written from here is protected by default, before a credential exists.
|
||||
- **No cart, order or payment tables.** They arrive in M5, so the migration history stays
|
||||
reviewable and the variant model gets proven against real reads first.
|
||||
- **`best_selling` and `relevance` sorts fall back to newest.** There is no order data (M5) and
|
||||
no ranking (M6). Falling back is honest; a fake ranking would not be.
|
||||
- **No sport/gender facet counts.** Those dimensions are navigated by route, not refined within a
|
||||
page, so a count would render nowhere.
|
||||
- **Facet counts are computed per request.** Fine at this catalog size; the fix when it stops
|
||||
being fine is a search index (ADR-0012), not a bigger query.
|
||||
- **No customer-facing auth UI.** The storefront login/register endpoints exist and are tested;
|
||||
the screens land with the account milestone (M8) where they belong.
|
||||
- **The role editor is read-only.** Viewing which role grants what is the question operators
|
||||
actually ask; editing grants is destructive and wants a confirmation flow and an audit entry.
|
||||
- **A password reset does not revoke existing sessions.** Doing it properly means revoking every
|
||||
family for the user and belongs with the session-management screen, not bolted onto the
|
||||
endpoint. Noted in the code.
|
||||
- **No email.** Password reset, order confirmations and back-in-stock alerts all need it; Mailpit
|
||||
is already running locally for when it lands.
|
||||
- **No cart, order or payment tables.** M5, so the migration history stays reviewable.
|
||||
- **`best_selling` and `relevance` sorts fall back to newest.** No order data (M5), no ranking
|
||||
(M6). Falling back is honest; a fake ranking would not be.
|
||||
- **No sport/gender facet counts.** Navigated by route, not refined in-page, so a count would
|
||||
render nowhere.
|
||||
- **Facet counts are computed per request.** Fine at this catalog size; the fix when it is not is
|
||||
a search index (ADR-0012), not a bigger query.
|
||||
- **The event bus is in-process and lossy.** Anything that must not be lost stays in the same
|
||||
database transaction as its cause.
|
||||
transaction as its cause.
|
||||
- **No observability beyond logs.** Traces and metrics are worth adding once there is production
|
||||
traffic to explain.
|
||||
- **No CDN, TLS or WAF config.** That belongs to the deployment repository.
|
||||
|
||||
Reference in New Issue
Block a user