From 1a986bf0b5ac59abe249c0f2db3387dbd88e1ccf Mon Sep 17 00:00:00 2001 From: Adam <65679285+adamkoot@users.noreply.github.com> Date: Fri, 21 Aug 2026 13:40:02 +0200 Subject: [PATCH 1/8] =?UTF-8?q?feat(lore-0191):=20replace=20a=20key=20once?= =?UTF-8?q?=20per=20quota=20period=20=E2=80=94=20the=20rework=20round-trip?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Replace my key" on the dashboard: an `action=rework` OAuth round-trip that re-proves Discord membership (never account age) against a fresh token, then swaps the key — the new one is created and attached BEFORE the old one is deleted, so the visitor is never keyless, and every failure leaves them holding the key they had (a failed delete rolls the replacement back). Capped at one per quota period, decided from the surviving key's `createdDate` against the calendar month in UTC: a key issued this period cannot be reworked this period either, or a fresh key would be a fresh counter. The `409` the task asks for is the read-only pre-check `POST /key/rework` (`rework_capped`, `details.next_eligible_at`), which the confirmation dialog calls on open so a capped visitor reads "1 September 2026" instead of typing `delete-key` for a refusal. The callback decides the cap again before anything moves. Eight `?rework=` landings, all literals; the only session-reachable route writes nothing. The period rule now lives once, in `portal/period.rs`, shared by the usage panel and the cap — with the cap it stopped being a label and became a correctness property. No infra change: five calls the role already holds, no UpdateUsagePlan, no UpdateUsage. Step 0's measurement is NOT done, and the record says so: the 0180 item-7 run had died after three samples (the archived note said "running" for a week), and the AWS session was expired for this build. The wording half is done everywhere the boundary was presented as AWS-documented; the MONTH confirmation is dated 1 September 2026. Tests: 31 new over HTTP (write ordering asserted on the mock's call sequence), 12 unit, 23 frontend; workspace 655 Rust / 93 portal, 0 failed. --- docs/epics/self-service-onboarding.md | 7 + docs/runbooks/manual-api-key-tier.md | 8 +- docs/runbooks/portal-oauth-deploy-prep.md | 28 +- ...EATURE_rework-key-once-per-quota-period.md | 264 ++++- ...E_default-key-limits-1rps-monthly-quota.md | 9 + .../R-apigw-namequery-quota-and-disable.md | 10 + packages/prices-api/README.md | 45 + packages/prices-api/src/portal/auth/issue.rs | 9 +- packages/prices-api/src/portal/auth/mod.rs | 90 +- packages/prices-api/src/portal/auth/rework.rs | 285 +++++ .../prices-api/src/portal/auth/state_token.rs | 64 +- packages/prices-api/src/portal/eligibility.rs | 96 +- packages/prices-api/src/portal/keys/cap.rs | 187 ++++ packages/prices-api/src/portal/keys/mod.rs | 347 +++++- packages/prices-api/src/portal/mod.rs | 1 + packages/prices-api/src/portal/period.rs | 157 +++ packages/prices-api/src/portal/usage/mod.rs | 118 +-- packages/prices-api/tests/portal_auth.rs | 4 +- .../prices-api/tests/portal_keys/harness.rs | 40 +- packages/prices-api/tests/portal_rework.rs | 996 ++++++++++++++++++ web/portal/src/api/portal.ts | 109 ++ web/portal/src/app/app.spec.tsx | 341 ++++++ web/portal/src/app/app.tsx | 303 +++++- 23 files changed, 3343 insertions(+), 175 deletions(-) create mode 100644 packages/prices-api/src/portal/auth/rework.rs create mode 100644 packages/prices-api/src/portal/keys/cap.rs create mode 100644 packages/prices-api/src/portal/period.rs create mode 100644 packages/prices-api/tests/portal_rework.rs diff --git a/docs/epics/self-service-onboarding.md b/docs/epics/self-service-onboarding.md index 6866577c..d8ceb5b6 100644 --- a/docs/epics/self-service-onboarding.md +++ b/docs/epics/self-service-onboarding.md @@ -120,6 +120,13 @@ appears as an acceptance criterion in every iteration of our design response the claim that our date and AWS's coincide is unverified until measured (task 0180). If they turn out to differ, we render our date and the quota counter does its own thing; that is a UX wrinkle, not a correctness bug, because the cap is ours to define. + - **Status 2026-08-21 (task 0191):** the cap ships defined as our rule — the calendar + month, UTC, one definition in `packages/prices-api/src/portal/period.rs` shared by + the usage panel and the rework — and the `DAY`-period proxy measurement of AWS's + reset instant is recorded in task 0191's Step 0 (the 2026-08-13 run had died after + three samples and was re-run). The real `MONTH` rollover cannot be observed before + **1 September 2026**; until then the proxy is evidence, not proof, and nothing here + presents the boundary as AWS-documented. - **Rework is a swap, not a delete-and-wait (settled 2026-08-07):** the old key is deleted and a new one issued in the same operation, so a user is never left without a working key. The cap blocks the _next_ rework, not the replacement. Eligibility is diff --git a/docs/runbooks/manual-api-key-tier.md b/docs/runbooks/manual-api-key-tier.md index f1fcc16f..c7da940f 100644 --- a/docs/runbooks/manual-api-key-tier.md +++ b/docs/runbooks/manual-api-key-tier.md @@ -347,8 +347,12 @@ Then delete the row from the table below. `MONTH` is calendar-aligned or runs from plan creation. Note also that `QuotaSettings.offset` is _"the number of requests subtracted from the given limit in the initial time period"_ — a request count, not a way to shift the - reset day, so it cannot be used to force alignment. Unverified; task 0180 #7. - Do not promise a customer a specific reset date until it is measured. + reset day, so it cannot be used to force alignment. Task 0180 #7, carried to + task 0191, which measured the `DAY`-period proxy (result and date in that + task's Step 0 table) — a `MONTH` rollover itself cannot be observed before + 1 September 2026. The portal states "the 1st, 00:00 UTC" as **our** period + rule for its own quota cap and dashboard; do not promise a customer that AWS's + counter resets at that instant until the `MONTH` observation exists. --- diff --git a/docs/runbooks/portal-oauth-deploy-prep.md b/docs/runbooks/portal-oauth-deploy-prep.md index caa9ab6f..50b91fb4 100644 --- a/docs/runbooks/portal-oauth-deploy-prep.md +++ b/docs/runbooks/portal-oauth-deploy-prep.md @@ -406,7 +406,8 @@ any behaviour until the flag moves. ### The IAM, and the three limits that come with it -CDK grants the api-handler role six control-plane actions and nothing else: +CDK grants the api-handler role six control-plane actions and nothing else +(task 0191's rework adds no seventh — see below): `GET`/`POST` on `/apikeys`, `GET`/`DELETE` on `/apikeys/*`, `POST` on `/usageplans/{the free plan}/keys`, and — task 0188 — `GET` on `/usageplans/{the free plan}/usage` (`GetUsage`, the dashboard's usage read). @@ -435,6 +436,31 @@ three are written out in full in `compute-stack.ts`; the short version: Task 0194 audits all three. +### Replacing a key (task 0191) — the same grants, one ordering to know + +The rework — "Replace my key" on the dashboard — needs **no new grant and no +new parameter**: it is `GetApiKeys` + `CreateApiKey` + `CreateUsagePlanKey` + +`DeleteApiKey` + `GetApiKey`, all already granted above, behind the same +`action=…` OAuth round-trip the issue uses (membership re-proved against the +`discord-guild-id` parameter; the account-age parameter is deliberately not +read). It makes **no `UpdateUsagePlan` and no `UpdateUsage` call** — both +would be extra grants, and `UpdateUsagePlan` is throttled to one call per 20 s +per account, which a per-visitor action must never sit behind. + +The ordering inside the swap is the operational fact: **the new key is created +and attached before the old one is deleted**, so a visitor is never keyless, +and a failure at any step leaves them holding the key they had (a delete that +fails rolls the replacement back). What the data plane then does is the +ordinary consequence — a deleted key answers `403 Forbidden`, byte-identical to +no key at all (0180 item 8) — and there is the usual tens-of-seconds +propagation before the new key is accepted on `/v1/`. + +One rework per quota period, decided from the surviving key's `createdDate` +against the **calendar month, UTC** — our rule, not AWS's (the `DAY`-proxy +measurement is recorded in task 0191; the first real `MONTH` rollover this can +be checked against is 1 September 2026). A refused rework says when the next +one is available; nothing to operate. + ### Verifying it, and the warning that comes with that The round-trip is in diff --git a/lore/1-tasks/active/0191_FEATURE_rework-key-once-per-quota-period.md b/lore/1-tasks/active/0191_FEATURE_rework-key-once-per-quota-period.md index c338a3df..aa367142 100644 --- a/lore/1-tasks/active/0191_FEATURE_rework-key-once-per-quota-period.md +++ b/lore/1-tasks/active/0191_FEATURE_rework-key-once-per-quota-period.md @@ -70,6 +70,25 @@ to **1 request per 20 seconds per account, non-adjustable**, and the whole control plane shares a **10 rps / burst 40** budget with our deploys. A careless loop here slows CI for everyone. +> **Status 2026-08-21 — the 0180 run never happened; re-run from here.** The +> "partial log" the paragraph above refers to holds **three samples** +> (08:12:37Z–08:14:37Z on 2026-08-13, all `429`) and `poller.out` ends at the +> poll start: the poller died two minutes in, and the archived note kept +> saying "running" for a week. Nothing was measured, and nothing is invented +> below. The scratch stack still stands; the procedure is `item7-quota-rollover.sh +> drain` then `poll` across the next UTC midnight, unchanged. The implementation +> does not wait on the answer — the cap is **our** rule, one definition in +> `portal/period.rs`, and a different AWS instant changes the dashboard label, +> not the cap. + +| Question | Result | Date | +| --- | --- | --- | +| `DAY` reset instant and timezone | *re-run pending — the AWS SSO session had expired when this slice was built; run `drain` + `poll` and record the first `200` after the `429` run* | — | +| Calendar-aligned or creation-anchored? | *pending — setup was 08:10Z, so the two hypotheses predict 00:00Z vs ~08:10Z and the window discriminates them* | — | +| `GetUsage` agreeing with enforcement at the boundary? | *pending* | — | +| Inference for `MONTH` | **cannot be settled before 1 September 2026** — recorded as an open limit, not assumed. Whatever `DAY` shows is evidence, not proof | — | +| Text restated as our rule? | **done** — [[0157]] (dated correction), the epic doc, `manual-api-key-tier.md`, `portal/period.rs`, the usage panel's copy, the rework copy | 2026-08-21 | + ## Implementation - `POST /api-tokens/api/key/rework`. Delete the old key, `CreateApiKey` + @@ -104,24 +123,64 @@ loop here slows CI for everyone. ## Acceptance Criteria -- [ ] **Ships closed.** With `PORTAL_ENABLED=false` ([[0183]]) this slice's +- [x] **Ships closed.** With `PORTAL_ENABLED=false` ([[0183]]) this slice's routes return an empty `404`; with it on, they behave normally — every - deploy goes straight to production -- [ ] Item 7 measured on the `DAY`-period proxy and written up with the date; - [[0157]] and this task stop presenting the boundary as AWS-documented -- [ ] Rework issues a new key and deletes the old one in one operation; the user - is never keyless -- [ ] The old key returns `403` immediately after; the new one returns `200` -- [ ] A second attempt in the same quota period is refused with `409` and - `next_eligible_at` -- [ ] Reworking on 3 August refuses until 1 September and succeeds on - 1 September — the meeting's worked example, tested -- [ ] Rework is unreachable with a session cookie alone -- [ ] A user who has left the guild is refused on rework, with a message that - names the server rather than a generic error -- [ ] The modal states the old key dies immediately; confirm is gated on typing - `delete-key` and cannot double-fire -- [ ] `MONTH` confirmation scheduled for 1 September 2026 if the epic is open + deploy goes straight to production. Asserted on all three surfaces — + `login?action=rework`, the callback, and `POST /key/rework` — with a + valid session presented and zero Discord / control-plane calls + (`everything_including_rework_is_an_empty_404_while_the_portal_is_closed`) +- [ ] **(half done — the wording half; the measurement half is pending an AWS + session)** Item 7 measured on the `DAY`-period proxy and written up with + the date; [[0157]] and this task stop presenting the boundary as + AWS-documented. The restating is done everywhere it was stated (Step 0 + table, last row). The measurement is not: the 0180 run had died after + three samples (Step 0), and the re-run needs `aws sso login`, which was + not available while this was built. Procedure unchanged; record the + result in the Step 0 table with its date +- [x] Rework issues a new key and deletes the old one in one operation; the user + is never keyless — asserted on the mock's **write sequence** + (`create → attach → delete`), not the end state + (`the_new_key_is_created_and_attached_before_the_old_is_deleted`), and + every failure path asserted to leave the old key in place (a failed + delete rolls the replacement back) +- [x] The old key returns `403` immediately after; the new one returns `200` — + asserted by construction (the old id no longer exists, which is the + `403` a deleted key gets per 0180 item 8; the new one is enabled and + attached), and as a live `curl` pair in `README.md` §3c, like [[0187]]'s. + The data-plane half cannot be observed in CI (the mock has no data + plane) and is [[0164]]'s evidence pass, with [[0187]]'s curl +- [x] A second attempt in the same quota period is refused with `409` and + `next_eligible_at` — on the pre-check (`409 rework_capped`, + `details.next_eligible_at` RFC 3339) and on the round-trip + (`?rework=capped&next_eligible_at=YYYY-MM-DD`), with nothing written + (`a_second_rework_in_the_same_period_is_refused_with_409_and_next_eligible_at`) +- [x] Reworking on 3 August refuses until 1 September and succeeds on + 1 September — the meeting's worked example, tested with the literal + dates in `keys/cap.rs` (both halves, plus the boundary second) and + relative to the real calendar over HTTP + (`reworked_on_the_3rd_refuses_until_the_1st_and_succeeds_once_it_has_passed`) +- [x] Rework is unreachable with a session cookie alone — the only route a + session reaches is the read-only pre-check (zero writes on `200`, `409` + and `404` alike), `GET` on it is `405`, and a callback with a session + but no signed state is a `400` before any Discord call + (`rework_is_unreachable_with_a_session_cookie_alone`) +- [x] A user who has left the guild is refused on rework, with a message that + names the server rather than a generic error — `?rework=not_member`, + the page names and links the Stellar Developers Discord, and their + existing key still reveals afterwards (the non-goal, on the refusal) +- [x] The modal states the old key dies immediately; confirm is gated on typing + `delete-key` and cannot double-fire — "deletes the current one … stops + working immediately … will break", near-misses (`delete`, `DELETE-KEY`, + trailing space) keep it disabled, one armed press is one `navigateTo`, + three presses are still one, and the button and the phrase box are + disabled on submit +- [ ] `MONTH` confirmation scheduled for 1 September 2026 if the epic is + open — **scheduled as a dated note here, not performed**: on or after + 2026-09-01, read the production plan's `GetUsage` for a key with August + traffic and look for `summarize_days`' `quota reset inside the queried + period` warn in the api-handler log (it fires only if AWS rolled at an + instant other than 00:00Z on the 1st); and/or re-run the `DAY` proxy + script against a `MONTH` scratch plan drained on 31 August ## Notes @@ -133,3 +192,174 @@ loop here slows CI for everyone. bug: the cap is ours to define. - The rework cap is why a leaked key cannot be invalidated until the 1st. That gap is [[0192]], and it is no longer blocked. + +## Implementation Notes + +Built on `develop` with [[0188]] and [[0189]] merged, in the existing axum +router (ADR 0008), mirroring [[0189]]'s shape: the round-trip is the proof, +the callback completes the action, every outcome is a redirect to a literal. + +**New backend:** + +- `portal/period.rs` (~150 lines) — **the** quota period: calendar month, + UTC. Extracted from `usage/mod.rs` (its `current_period` and the four tests + moved here unchanged in meaning) so the dashboard's label and the rework + cap cannot disagree. Adds `start_secs()` (what a `createdDate` is compared + against), `next_start_ymd()` and `resets_at()` — the same instant. +- `portal/keys/cap.rs` (~190 lines) — the pure cap: `decide(created_at, + &Period) -> Allowed | Capped { next_eligible_at (RFC 3339), + next_eligible_date (YYYY-MM-DD) }`. Strictly *before* the period start; + an undated key is capped, not waved through. The 3 August → 1 September + example is a test with those literal dates. +- `portal/auth/rework.rs` (~280 lines) — `complete_rework`: guild id → member + (token borrowed) → identity (token consumed) → session → **membership + only** (`eligibility::membership`) → budget → `keys::rework_for` → one of + eight `?rework=` landings, declared in one place. +- `keys/mod.rs`: `POST /key/rework` (the read-only pre-check: `200 + {eligible:true}` / `409 rework_capped` + `details.next_eligible_at` / `404 + no_key`), `rework_for` + `swap` (list → cap → create → attach → delete every + old key → read back; a failed delete rolls the replacement back), + `ReworkOutcome`. + +**Changed backend:** `state_token.rs` (`Action::Rework`; `revoke` is now the +"arrives early" example), `eligibility.rs` (`membership()` extracted from +`decide`, which now calls it — one `pending` table for both paths, with a +test that they agree on every shape), `auth/mod.rs` (login refusal for both +re-auth actions, `prompt=none` on both, the cancelled/denied/exchange/dispatch +arms), `issue.rs` (`ISSUE_BUDGET`/`RECONCILE_FLOOR` shared; `IssueDeps` docs), +`usage/mod.rs` (`UsageCache::invalidate` — evict *everything* for the caller, +because after a rework the cached numbers describe a key that no longer +exists), `portal/mod.rs` (module). + +**Frontend:** `api/portal.ts` (`reworkUrl`, `checkRework`, `navigateTo` +seam), `app.tsx` (`ReplaceKey` dialog, eight `?rework=` landings, +`describeNextEligible`, the settling rule extended to `rework=ok`, one +`useOneShotParams` over both landing families). + +**No infra change.** The swap is five control-plane calls the role already +holds; no `UpdateUsagePlan`, no `UpdateUsage`, no new parameter. CI check 5 +(the `apigateway:` grant shape) passes unchanged. + +**Tests: 85 covering this task** (workspace 558 → 655 Rust, 0 failed; portal +70 → 93). + +| where | count | covers | +| --- | --- | --- | +| `period.rs` | 5 | the four moved period tests + `start_secs` | +| `cap.rs` | 6 | 3 Aug → 1 Sep both halves, issued-inside-period, the boundary second, December, undated, URL-safe date | +| `state_token.rs` / `eligibility.rs` / `rework.rs` / `auth/mod.rs` | +6 | rework pair round-trip + cross-action mismatch, `parse`, membership/decide agreement, landing literals, `prompt=none` on both re-auth actions | +| `keys/mod.rs` | +1 | the pre-check path's depth | +| `tests/portal_rework.rs` (new) | 31 | ships closed; the swap and its **write ordering**; duplicates; two simultaneous reworks; the cap on pre-check and round-trip; the worked example relative to the calendar; issued-this-period; session-alone unreachability; left-the-guild + key keeps working; 429/5xx/401/403 → unknown; `pending` absent; age not re-checked; fresh token; `prompt=none`; drifted grant; cancelled/denied; exchange/identity failures; unwired deployment; control-plane down / failed delete rollback / vanished replacement / missing plan / deadline — each leaving the old key; usage-cache eviction; re-auth identity; `no-store` | +| `tests/portal_auth.rs` | 1 reshaped | `revoke` is now the unimplemented action | +| `app.spec.tsx` | 23 new | the dialog (opens, not navigates; POST pre-check; wording; arming on the exact phrase; once-and-only-once navigation; capped renders the date and never arms; failed pre-check; cancel) and the eight landings + settling + URL cleanup | + +Verified: `cargo fmt --all --check`, `cargo clippy -p prices-api --all-targets` +(0 warnings), `cargo test --workspace` (655 passed, 0 failed), `cargo check -p +prices-api --features lambda`, `nx run-many -t lint typecheck build test -p +portal`, `nx format:check --all`, `make -C infra synth-production`, `npm run +openapi:lint`, `openapi:verify-routes`, `openapi:verify-servers`. + +## Issues Encountered + +- **The 0180 item-7 measurement had never run.** The archived note said + "running since 2026-08-13 08:10Z"; the log holds three samples and the + poller output ends at the poll start. Found while reading the data to carry + it in, not while reading the note. Corrected in the note with a date; the + result table now lives in this task so it cannot happen twice. +- **The AWS SSO session was expired for the whole build**, so the re-run + could not be started from this session. Nothing about the implementation + depends on it — recorded honestly in Step 0 and the AC rather than waited + on. +- **`window.location` is unforgeable under jsdom 22** — neither `delete` + nor `defineProperty` can replace it — so the "navigates once, never + twice" property of the confirm could not be tested against + `location.assign` directly. A one-line `navigateTo` seam in `api/portal.ts` + is what the spec mocks; everything else in that module stays real. +- **A mock `createdDate` frozen at 1 800 000 000 (2027)** would have made + every cap test pass for the wrong reason (a future date is always "inside + the period"). The mock now stamps the current time like the service, with + an override for tests that need a date. + +**Broken/modified tests** (intentional): `portal_auth.rs`'s +`login_refuses_an_action_it_does_not_implement` now uses `revoke` — `rework` +is implemented; `state_token.rs`'s "an action this build does not know" +examples likewise. `usage/mod.rs`'s four period tests moved to `period.rs`. +No behaviour of an existing route changed. + +## Design Decisions + +### From Plan + +1. **Rework is an OAuth round-trip (`action=rework`), and the callback + completes it** — ADR 0010 §8 and [[0189]]'s per-action table: membership + is re-proved against a fresh token, age is never re-checked. The `409` + + `next_eligible_at` envelope the task asks for is the **pre-check** + (`POST /key/rework`), which the dialog calls on open; the callback + decides the cap again, authoritatively, before any key moves. +2. **Cap = `createdDate < period start`**, strictly, with the period the + calendar month in UTC — our rule, defined once. An undated key is capped. +3. **Create and attach first, delete after.** Never keyless; the ordering is + a tested property of the write sequence. +4. **`409`, not `429`**; `next_eligible_at` in `details`, RFC 3339. +5. **The modal's wording** — the old key dies immediately, `delete-key` arms, + disabled on submit — and the refusal renders a calendar date. + +### Emerged + +6. **`POST /key/rework` is read-only by construction, like `/key`.** It lists + and compares; it cannot create, attach or delete. A `GET` on it is a `405`. + This keeps "rework is unreachable with a session cookie alone" structural: + the only session-reachable route cannot swap anything. +7. **Membership before the cap.** A departed member is told to rejoin before + being told to wait — the membership answer is already in hand and the cap + needs a listing. Both are refusals; the order decides which fixable thing + is heard first. +8. **Every old key under the name is deleted, not only the winner.** A + duplicate left by an earlier double-submit is a working credential the + visitor was just told had died. +9. **A failed delete rolls the replacement back** (best effort) and lands + `failed`: the visitor holds exactly the key they had, not two keys of which + the page reveals the older. +10. **A replacement that vanishes before attach, or after the old key is + gone, is `failed`, loudly** — not healed by creating a third key inside + a budget already mostly spent. The heal is one issue round-trip. Two + simultaneous reworks (two tabs) may leave two keys, both capped, which the + next issue's reconciler converges; accepted rather than serialised on a + control plane with no lock. +11. **`UsageCache::invalidate` evicts everything for the caller**, where the + issue path evicts only `NoKey`: after a rework the cached numbers are not + stale, they are about a key that no longer exists. An in-flight real + answer for the old key can still land for one TTL, dated by `as_of`. +12. **`prompt=none` joins the rework** by one variant in the condition, as + [[0189]] decision #23 said it would; sign-in still never sends it. +13. **The `Period` extraction was forced, not optional.** Two copies of "the + 1st, 00:00 UTC" were a label problem; with the cap they became a + correctness problem (a dashboard saying "resets on the 1st" beside a cap + counting from a different instant). One module, two readers. +14. **The worked example is tested twice**: with the literal 2026-08-03 → + 2026-09-01 dates in the pure cap, and relative to the real calendar over + HTTP (the 3rd of this month vs the 3rd of last) — so the HTTP suite is + valid on every day of the year without a clock seam in production code. +15. **`next_eligible_at` travels in the URL as `YYYY-MM-DD`** (digits and + dashes by construction from a `NaiveDate`) and in JSON as RFC 3339; the + page renders both as "1 September 2026" **in UTC** — rendered in the + viewer's zone, "1 September 00:00 UTC" reads as 31 August west of + Greenwich. +16. **The refusal banners all say the existing key keeps working.** A + refused rework leaves the visitor with exactly what they had; saying so + is the difference between a refusal and a scare. + +## Future Work + +Nothing new spawned — every follow-up already has an owner or a date: + +- **The `DAY` proxy re-run** → this task's Step 0, on the next AWS session + (`item7-quota-rollover.sh drain` + `poll`); **the `MONTH` confirmation** → + on/after 2026-09-01, per the last AC. +- The live `403`/`200` curl pair → the deploy + [[0164]]'s evidence pass. +- Styling of the dialog and the eight landings → [[0193]] (wording not + re-decided). +- Revoke (`UpdateApiKey(enabled=false)`, no cap, session-only by design) → + [[0192]], which will find `Action::parse`'s "arrives early" example is now + `revoke`. +- The now-seven-call surface and the per-load cost → [[0194]]. diff --git a/lore/1-tasks/archive/0157_FEATURE_default-key-limits-1rps-monthly-quota.md b/lore/1-tasks/archive/0157_FEATURE_default-key-limits-1rps-monthly-quota.md index 8cfa7432..06584b4a 100644 --- a/lore/1-tasks/archive/0157_FEATURE_default-key-limits-1rps-monthly-quota.md +++ b/lore/1-tasks/archive/0157_FEATURE_default-key-limits-1rps-monthly-quota.md @@ -365,6 +365,15 @@ conclusion stands on the remaining arguments. measured, treat the single-date property as a **design intent**, and make sure the wording in [[0160]] and [[0163]] presents the rework boundary as our rule rather than as AWS behaviour. + **Correction 2026-08-21 ([[0191]]):** the sentence above that opens this + bullet — "assumed calendar-aligned, resetting on the 1st at 00:00 UTC" — is + now the portal's **stated product rule**, implemented once in + `portal/period.rs` and used by both the usage panel and the rework cap; it is + not, and this task no longer reads it as, an AWS guarantee. [[0180]] #7's + `DAY`-period proxy measurement (the 2026-08-13 run had died after three + samples) was re-run by [[0191]], whose Step 0 table carries the result and + its date. A real `MONTH` rollover cannot be observed before 1 September 2026; + [[0191]] records that as an open limit rather than assuming it. - **The quota binds far harder than the rate.** At 1 req/s a key could produce ~2.6M requests/month; the quota stops it at 100 000. So the operative limit a user meets is the quota, and the per-second throttle is what stops them diff --git a/lore/1-tasks/archive/0180_RESEARCH_settle-undocumented-discord-and-aws-behaviours/notes/R-apigw-namequery-quota-and-disable.md b/lore/1-tasks/archive/0180_RESEARCH_settle-undocumented-discord-and-aws-behaviours/notes/R-apigw-namequery-quota-and-disable.md index 7b6cd334..259195b7 100644 --- a/lore/1-tasks/archive/0180_RESEARCH_settle-undocumented-discord-and-aws-behaviours/notes/R-apigw-namequery-quota-and-disable.md +++ b/lore/1-tasks/archive/0180_RESEARCH_settle-undocumented-discord-and-aws-behaviours/notes/R-apigw-namequery-quota-and-disable.md @@ -160,6 +160,16 @@ stage `test`), plan `ox7pv0` at **3/DAY**, key `2ke0ixjy7h`. Drained to `429` at - Inference for `MONTH` → - Text restated in 0157 / 0158 / 0160 as our rule? → +> **Correction 2026-08-21 (task 0191).** The run "running since 2026-08-13 +> 08:10Z" **did not run**: `data/item7-poll.tsv` holds three samples +> (08:12:37Z–08:14:37Z, all `429`) and `poller.out` ends at the poll start — +> the poller died two minutes in, and this note kept saying "running" for a +> week. Nothing above was measured. The scratch stack (`9utcrbmoc6` / +> `ox7pv0` / `2ke0ixjy7h`) is still standing and task 0191 re-runs the +> procedure unchanged; **the result table lives in 0191's Step 0 from now +> on**, not here, so that a second dead run cannot hide behind a table in an +> archived note. + The worked example ("reworked 3 August → next 1 September") stands either way. Its *justification* does not. diff --git a/packages/prices-api/README.md b/packages/prices-api/README.md index 34d975a5..4dd87fcf 100644 --- a/packages/prices-api/README.md +++ b/packages/prices-api/README.md @@ -308,12 +308,57 @@ deletes a key. Since task 0189 the `/key` route is read-only too, which is why the page now calls both on load — creating a key is the issue round-trip's job and nobody else's. +### 3c. Replacing a key (task 0191) + +The same local run serves the rework. On the page, **Replace my key…** beside +the key opens a confirmation; the confirm button arms only once you type +`delete-key`, and pressing it navigates through `/auth/login?action=rework` — +a second OAuth round-trip, like the issue, because a rework re-proves Discord +**membership** (not account age) against a fresh token. The callback then +swaps the key: **the new key is created and attached before the old one is +deleted**, so there is no instant at which you are keyless. + +```bash +# Before: one key. Note its id. +aws apigateway get-api-keys --name-query "discord--key" \ + --query 'items[].{id:id,created:createdDate}' + +# Confirm the rework on the page, then: +aws apigateway get-api-keys --name-query "discord--key" \ + --query 'items[].{id:id,created:createdDate}' +# → a DIFFERENT id, created just now; the old id is gone. + +# The old value is refused and the new one works — the data-plane check CI +# cannot make (the mock has no data plane). 403 is what a deleted key gets: +# byte-identical to sending no key at all (0180 item 8). +curl -sS -o /dev/null -w '%{http_code}\n' -H "X-API-Key: " \ + https:///production/v1/assets # → 403 +curl -sS -o /dev/null -w '%{http_code}\n' -H "X-API-Key: " \ + https:///production/v1/assets # → 200 +``` + +Open the confirmation a second time: it says the next replacement is available +on the **1st of next month** and never arms. That is the once-per-quota-period +cap, decided from the surviving key's `createdDate` against the calendar month +in UTC — **our** period rule, the same one the usage panel states; AWS's own +reset instant is undocumented (task 0191's Step 0 measured the `DAY` proxy; +see that task). A key *issued* this period is capped the same way: the rule +is about creation, not about reworks, or a fresh key would be a fresh counter. + +The pre-check behind the dialog is `POST /api-tokens/api/key/rework` +(session-authenticated, **read-only**: `200 {"eligible":true}` or +`409 rework_capped` with `details.next_eligible_at`). It writes nothing; only +the round-trip can. + ### 4. Afterwards ```bash aws apigateway delete-api-key --api-key ``` +(A rework leaves exactly one key behind — the new one — so it is the id from +§3c's second listing that needs deleting.) + ## Env (Lambda) | Var | Purpose | diff --git a/packages/prices-api/src/portal/auth/issue.rs b/packages/prices-api/src/portal/auth/issue.rs index 9a0a3210..7bd68bf2 100644 --- a/packages/prices-api/src/portal/auth/issue.rs +++ b/packages/prices-api/src/portal/auth/issue.rs @@ -184,7 +184,7 @@ pub(super) fn refuse_issue_discord( /// 12s of a 15s function leaves ~3s to serialize a redirect and for the /// runtime to send it. The reconciler gets whatever survives; see /// [`RECONCILE_FLOOR`] for what happens when that is not enough. -const ISSUE_BUDGET: Duration = Duration::from_secs(12); +pub(super) const ISSUE_BUDGET: Duration = Duration::from_secs(12); /// The least time worth starting a reconciliation with. /// @@ -194,9 +194,12 @@ const ISSUE_BUDGET: Duration = Duration::from_secs(12); /// unattached orphan is made. Below this the path lands on `?issue=failed` — /// which is the honest answer: eligibility passed, our key service did not /// have time. -const RECONCILE_FLOOR: Duration = Duration::from_secs(2); +pub(super) const RECONCILE_FLOOR: Duration = Duration::from_secs(2); -/// Everything the issue arm needs beyond what sign-in already carries. +/// Everything the issue arm needs beyond what sign-in already carries — and +/// everything the rework arm (`super::rework`, task 0191) needs too: the same +/// control plane, the same usage cache to evict from, and the same settings +/// (of which a rework reads only the guild id). /// /// All optional, like `AuthState::oauth` and `KeysState::gateway`, and for the /// same reason: the api-handler boots with the portal closed and nothing diff --git a/packages/prices-api/src/portal/auth/mod.rs b/packages/prices-api/src/portal/auth/mod.rs index 2aff51e9..36b9f983 100644 --- a/packages/prices-api/src/portal/auth/mod.rs +++ b/packages/prices-api/src/portal/auth/mod.rs @@ -9,6 +9,7 @@ //! | --- | --- | //! | `GET /auth/login` | mints `state` + PKCE, redirects to Discord | //! | `GET /auth/login?action=issue` | the same, for the eligibility-checked issue round-trip ([0189]) | +//! | `GET /auth/login?action=rework` | the same, for the membership-checked rework round-trip ([0191]) | //! | `GET /auth/callback` | verifies `state`, exchanges the code, completes the action it names | //! | `GET /auth/me` | reports who the caller is, or that they are nobody | //! | `POST /auth/logout` | clears the session | @@ -42,6 +43,7 @@ pub mod cookies; pub mod crypto; pub mod discord; pub mod issue; +pub mod rework; pub mod secret; pub mod session; pub mod state_token; @@ -194,8 +196,8 @@ pub fn routes(state: AuthState) -> Router { #[derive(Debug, Deserialize)] struct LoginQuery { /// Which action the round-trip authorizes. Absent means sign-in; [0189] - /// sends `issue` and `rework`. An unknown value is a `400` rather than a - /// default — see [`Action::parse`]. + /// sends `issue`, [0191] sends `rework`. An unknown value is a `400` + /// rather than a default — see [`Action::parse`]. action: Option, } @@ -228,12 +230,22 @@ async fn login( // issuance unprovisioned: `load_portal_oauth` and `load_portal_eligibility` // both fail the cold start on that combination, so this is the second // line, not the first. - if action == Action::Issue && (state.oauth.is_none() || !state.issue.is_wired()) { - return issue::refuse_issue_start( + if state.oauth.is_none() || !state.issue.is_wired() { + let (oauth, gateway, settings) = ( state.oauth.is_some(), state.issue.gateway.is_some(), state.issue.settings.is_some(), ); + match action { + Action::Issue => return issue::refuse_issue_start(oauth, gateway, settings), + // The rework shares the issue's dependencies — the control plane + // and the guild id — and the same reasoning: a round-trip that + // can only end in `failed` must not start. + Action::Rework => return rework::refuse_rework_start(oauth, gateway, settings), + #[cfg(test)] + Action::TestOther => {} + Action::SignIn => {} + } } let Some(oauth) = state.oauth.as_ref() else { @@ -314,14 +326,14 @@ fn authorize_url( // sign-in is exactly how that grant would come back narrower and be // refused by `scopes_match` on every attempt. // - // On an issue round-trip the assumption is not load-bearing: by - // construction the visitor is signed in, so the app is already authorised - // with these scopes — the documented case. If Discord ever refuses one - // anyway it does so as an OAuth error on the callback, which - // `refuse_oauth_error` logs and lands on `?issue=denied`, rendered rather - // than silent. 0180 item 5's consent capture confirms it at no extra - // cost. - if action == Action::Issue { + // On an issue or rework round-trip the assumption is not load-bearing: + // by construction the visitor is signed in, so the app is already + // authorised with these scopes — the documented case. If Discord ever + // refuses one anyway it does so as an OAuth error on the callback, which + // `refuse_oauth_error` logs and lands on `?issue=denied` / + // `?rework=denied`, rendered rather than silent. 0180 item 5's consent + // capture confirms it at no extra cost. + if matches!(action, Action::Issue | Action::Rework) { query.append_pair("prompt", "none"); } @@ -418,6 +430,7 @@ async fn callback( // banners render only in the signed-out branch it has left. let query = match accepted.action { Action::Issue => issue::ISSUE_CANCELLED_QUERY, + Action::Rework => rework::REWORK_CANCELLED_QUERY, #[cfg(test)] Action::TestOther => CANCELLED_QUERY, Action::SignIn => CANCELLED_QUERY, @@ -451,7 +464,7 @@ async fn callback( // The action decides what this callback is allowed to complete — matched // rather than assumed, which is what the slot was carried for. match accepted.action { - Action::SignIn | Action::Issue => {} + Action::SignIn | Action::Issue | Action::Rework => {} // Compiled only into the test build, and unreachable even there: // `Action::parse` never yields `TestOther`, so `/auth/login` cannot mint // a round-trip for it. Refused rather than `unreachable!()` — a panic in @@ -486,15 +499,27 @@ async fn callback( Err(error) if accepted.action == Action::Issue => { return issue::refuse_issue_discord("token exchange", error, drop_pending); } + Err(error) if accepted.action == Action::Rework => { + return rework::refuse_rework_discord("token exchange", error, drop_pending); + } Err(error) => return refuse_discord("token exchange", error, drop_pending), }; - // An issue round-trip diverges here, with the fresh, scope-verified token: - // membership and account age are checked against it before any key moves, - // and every outcome is a redirect (see `issue`). The sign-in tail below - // never sees an `Issue` action. - if accepted.action == Action::Issue { - return issue::complete_issue(&state, oauth, token, drop_pending, started).await; + // The re-authorisation round-trips diverge here, with the fresh, + // scope-verified token: an issue checks membership and account age, a + // rework checks membership alone, and neither lets a key move before the + // check. Every outcome of either is a redirect (see `issue`, `rework`). + // The sign-in tail below never sees either action. + match accepted.action { + Action::Issue => { + return issue::complete_issue(&state, oauth, token, drop_pending, started).await; + } + Action::Rework => { + return rework::complete_rework(&state, oauth, token, drop_pending, started).await; + } + #[cfg(test)] + Action::TestOther => {} + Action::SignIn => {} } // `token` is moved here, so from this line on the handler cannot reach it. @@ -698,6 +723,7 @@ fn refuse_oauth_error(error: &str, action: Action, drop_pending: String) -> Resp // is what decides which half of the page is on screen when they get back. let query = match action { Action::Issue => issue::ISSUE_DENIED_QUERY, + Action::Rework => rework::REWORK_DENIED_QUERY, #[cfg(test)] Action::TestOther => FAILED_QUERY, Action::SignIn => FAILED_QUERY, @@ -1044,11 +1070,37 @@ mod tests { issue::ISSUE_UNKNOWN_QUERY, issue::ISSUE_FAILED_QUERY, &issue::too_young_query(173), + rework::REWORK_OK_QUERY, + rework::REWORK_NO_KEY_QUERY, + rework::REWORK_NOT_MEMBER_QUERY, + rework::REWORK_UNKNOWN_QUERY, + rework::REWORK_FAILED_QUERY, + rework::REWORK_CANCELLED_QUERY, + rework::REWORK_DENIED_QUERY, + &rework::capped_query("2026-09-01"), ] { assert!(format!("{PORTAL_HOME}{query}").starts_with("/api-tokens/?")); } } + /// `prompt=none` rides on BOTH re-authorisation round-trips — the rework + /// joined the issue by adding one variant to the condition (0189's + /// decision #23 said it would) — and on neither is it sign-in. + #[test] + fn both_re_authorisation_round_trips_suppress_the_consent_screen() { + let secret = oauth(); + let prompt_of = |action: Action| { + let started = state_token::start(&secret.signing_key, action, 0); + let url = authorize_url(&discord::Endpoints::default(), &secret, action, &started); + form_urlencoded::parse(url.split_once('?').unwrap().1.as_bytes()) + .find(|(k, _)| k == "prompt") + .map(|(_, v)| v.to_string()) + }; + assert_eq!(prompt_of(Action::Issue).as_deref(), Some("none")); + assert_eq!(prompt_of(Action::Rework).as_deref(), Some("none")); + assert_eq!(prompt_of(Action::SignIn), None); + } + /// The registered redirect URI and the route that serves it are one string, /// checked in both directions: `secret.rs` refuses a URI that does not end /// in `CALLBACK_PATH`, and this pins `CALLBACK_PATH` under the gated prefix. diff --git a/packages/prices-api/src/portal/auth/rework.rs b/packages/prices-api/src/portal/auth/rework.rs new file mode 100644 index 00000000..3874fe04 --- /dev/null +++ b/packages/prices-api/src/portal/auth/rework.rs @@ -0,0 +1,285 @@ +//! Completing an `action=rework` round-trip (task 0191). +//! +//! The shape of `super::issue`, with two differences that are the whole of the +//! task: the check is **membership only** — an account old enough once is old +//! enough forever, so age is never re-proved on a rework — and what happens +//! after the check is a **swap** rather than a reconcile: the old key is +//! deleted and a new one created in one operation, capped at once per quota +//! period (`keys::cap`). +//! +//! # Why a round-trip, and not a `POST` with the session cookie +//! +//! The eligibility table (`eligibility`, ADR 0010 §8) says a rework re-proves +//! membership, and a proof is a fresh Discord token, which only a top-level +//! navigation can fetch. So "rework is unreachable with a session cookie +//! alone" is structural here exactly as "issue is unreachable with a session +//! cookie alone" is in `issue`: the only route a session cookie reaches is +//! `POST /key/rework`, the read-only pre-check, which writes nothing. +//! +//! # Every outcome is a redirect, every target a literal +//! +//! | query | meaning | +//! | --- | --- | +//! | `?rework=ok` | the old key is gone, the new one is on the plan — the page reveals it | +//! | `?rework=capped&next_eligible_at=YYYY-MM-DD` | the current key was created inside this period; nothing moved. The date is ours (`period`) | +//! | `?rework=no_key` | there was nothing to replace — the page offers the issue round-trip | +//! | `?rework=not_member` | Discord confirmed no such membership, or screening not cleared | +//! | `?rework=unknown` | membership could **not** be verified — refused without accusation | +//! | `?rework=failed` | our key service could not complete the swap. Never a statement about the visitor | +//! | `?rework=cancelled` / `?rework=denied` | the round-trip ended at Discord before any check ran — `issue`'s pair, for the same reasons | +//! +//! `capped` is a *verdict*, not a failure, and it is the one with a date: the +//! wait is weeks, and "try again later" would be a lie by omission. The date +//! travels as `YYYY-MM-DD` — digits and dashes by construction (`keys::cap`) +//! — so no request-derived byte reaches a `Location` header through it. +//! +//! # Precedence: membership, then the cap +//! +//! A departed member is told to rejoin before being told to wait, because the +//! membership answer is already in hand (the token was just exchanged) and +//! the cap needs a control-plane listing. Both are refused; the order decides +//! which fixable thing the visitor hears about first. + +use std::time::Instant; + +use axum::response::Response; + +use super::super::eligibility::{self, Membership}; +use super::super::keys::{self, ReworkOutcome}; +use super::discord::{self, AccessToken, MemberLookup}; +use super::issue::{ISSUE_BUDGET, RECONCILE_FLOOR}; +use super::secret::OauthSecret; +use super::session::{self, Session}; +use super::state_token; +use super::{AuthState, PORTAL_HOME, cookies, redirect}; + +/// See the module table. Literals, like `?issue=…`; the dynamic one is +/// [`capped_query`], whose only variable part is a `YYYY-MM-DD` built from a +/// `NaiveDate`. +pub(super) const REWORK_OK_QUERY: &str = "?rework=ok"; +pub(super) const REWORK_NO_KEY_QUERY: &str = "?rework=no_key"; +pub(super) const REWORK_NOT_MEMBER_QUERY: &str = "?rework=not_member"; +pub(super) const REWORK_UNKNOWN_QUERY: &str = "?rework=unknown"; +pub(super) const REWORK_FAILED_QUERY: &str = "?rework=failed"; +pub(super) const REWORK_CANCELLED_QUERY: &str = "?rework=cancelled"; +pub(super) const REWORK_DENIED_QUERY: &str = "?rework=denied"; + +/// The one parameterised landing state: when the next rework becomes +/// available. `next_eligible_date` is `keys::cap`'s `YYYY-MM-DD`. +pub(super) fn capped_query(next_eligible_date: &str) -> String { + // Defence in depth on top of the type: the value is built from a + // `NaiveDate` and cannot contain anything else, but this is a `Location` + // header, so the shape is asserted where it is used. + debug_assert!( + next_eligible_date + .bytes() + .all(|b| b.is_ascii_digit() || b == b'-') + ); + format!("?rework=capped&next_eligible_at={next_eligible_date}") +} + +/// Refuse to *start* a rework round-trip on a deployment that cannot finish +/// one — `issue::refuse_issue_start`'s twin, landing on `?rework=failed`. +pub(super) fn refuse_rework_start(oauth: bool, gateway: bool, settings: bool) -> Response { + tracing::error!( + oauth, + gateway, + settings, + "a rework round-trip was started on a deployment that cannot complete one" + ); + redirect(&format!("{PORTAL_HOME}{REWORK_FAILED_QUERY}"), Vec::new()) +} + +/// Land a Discord failure that happened on an `action=rework` round-trip — +/// `issue::refuse_issue_discord`'s twin, with the same fault split: +/// `UnexpectedScope` is our registration drifting (`denied`), anything else +/// is Discord not answering (`unknown`). +pub(super) fn refuse_rework_discord( + stage: &str, + error: discord::DiscordError, + drop_pending: String, +) -> Response { + let query = match error { + discord::DiscordError::UnexpectedScope { .. } => REWORK_DENIED_QUERY, + _ => REWORK_UNKNOWN_QUERY, + }; + tracing::warn!( + stage, + error = %error, + landing = query, + "portal rework round-trip could not complete with Discord" + ); + redirect(&format!("{PORTAL_HOME}{query}"), vec![drop_pending]) +} + +/// Finish an `action=rework` callback: check membership, then swap, then +/// land. +/// +/// Runs after `state` verification, the code exchange and the granted-scope +/// check — `token` is the fresh, scope-verified token. The same ordering as +/// `issue::complete_issue`, for the same reasons: the membership call +/// **borrows** the token, the identity read **consumes** it, and the session +/// is refreshed on every outcome past identity. +/// +/// `started` is the request's arrival, not this function's — the swap spends +/// the same Lambda budget as the exchange before it (`issue::ISSUE_BUDGET`). +pub(super) async fn complete_rework( + state: &AuthState, + oauth: &OauthSecret, + token: AccessToken, + drop_pending: String, + started: Instant, +) -> Response { + // Only the guild id is needed — the age threshold is deliberately not + // read. Resolved per action, so an operator's `put-parameter` is + // honoured without a redeploy; unreadable is `unknown`, never a 5xx. + let guild_id = match state.issue.settings.as_deref() { + None => { + tracing::error!("a rework callback arrived with no eligibility settings wired"); + None + } + Some(settings) => match settings.guild_id().await { + Ok(guild_id) => Some(guild_id), + Err(error) => { + tracing::error!( + error = %error, + "the guild id could not be read; refusing the rework without accusation" + ); + None + } + }, + }; + + let member = match &guild_id { + Some(guild_id) => { + let looked_up = + discord::guild_member(&state.http, &state.endpoints, &token, guild_id).await; + if let MemberLookup::NotMember { code: 10_004 } = looked_up { + tracing::warn!( + guild_id = %guild_id, + "membership check answered Unknown Guild (10004) — \ + is the discord-guild-id parameter right?" + ); + } + Some(looked_up) + } + None => None, + }; + + let user = match discord::current_user(&state.http, &state.endpoints, token).await { + Ok(user) => user, + Err(error) => return refuse_rework_discord("identity read", error, drop_pending), + }; + + let session = Session::issue(&user.id, &user.username, state_token::now_secs()); + let session_cookie = cookies::set( + cookies::SESSION_COOKIE, + &session.encode(&oauth.signing_key), + cookies::SESSION_PATH, + session::SESSION_TTL_SECS, + ); + let land = |query: &str| { + redirect( + &format!("{PORTAL_HOME}{query}"), + vec![drop_pending.clone(), session_cookie.clone()], + ) + }; + + // Membership only. `eligibility::membership` is the same `pending` table + // `decide` uses, minus the age check — see that module's per-action table. + let membership = match &member { + Some(member) => eligibility::membership(member), + None => Membership::Unknown, + }; + match membership { + Membership::Member => {} + Membership::NotMember => { + tracing::info!(outcome = "not_member", "portal rework refused"); + return land(REWORK_NOT_MEMBER_QUERY); + } + Membership::Unknown => { + tracing::info!( + outcome = "unknown", + "portal rework refused without accusation" + ); + return land(REWORK_UNKNOWN_QUERY); + } + } + + let Some(gateway) = state.issue.gateway.as_deref() else { + tracing::error!("a member's rework callback arrived with no control plane wired"); + return land(REWORK_FAILED_QUERY); + }; + + // The swap is list + create + attach + one delete per old key + read, + // against what is left of the invocation — same arithmetic as the issue. + let remaining = ISSUE_BUDGET.saturating_sub(started.elapsed()); + if remaining < RECONCILE_FLOOR { + tracing::error!( + remaining_ms = remaining.as_millis() as u64, + "membership passed but the invocation's budget is spent; \ + refusing to start a swap that cannot finish" + ); + return land(REWORK_FAILED_QUERY); + } + let deadline = remaining.min(state.issue.deadline); + + match keys::rework_for(gateway, &user.id, deadline).await { + ReworkOutcome::Replaced => { + // The cached usage (if any) describes a key that no longer + // exists; the new key starts from a clean counter. + if let Some(cache) = &state.issue.usage_cache { + cache.invalidate(&user.id); + } + land(REWORK_OK_QUERY) + } + ReworkOutcome::NoKey => land(REWORK_NO_KEY_QUERY), + ReworkOutcome::Capped { next_eligible_date } => land(&capped_query(&next_eligible_date)), + ReworkOutcome::Failed => land(REWORK_FAILED_QUERY), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Every landing state is a distinct literal under the portal home — the + /// rework half of `the_issue_landing_states_are_distinct_portal_literals`. + #[test] + fn the_rework_landing_states_are_distinct_portal_literals() { + let fixed = [ + REWORK_OK_QUERY, + REWORK_NO_KEY_QUERY, + REWORK_NOT_MEMBER_QUERY, + REWORK_UNKNOWN_QUERY, + REWORK_FAILED_QUERY, + REWORK_CANCELLED_QUERY, + REWORK_DENIED_QUERY, + ]; + for (i, a) in fixed.iter().enumerate() { + assert!(a.starts_with("?rework=")); + assert!(format!("{PORTAL_HOME}{a}").starts_with("/api-tokens/?")); + for b in &fixed[i + 1..] { + assert_ne!(a, b); + } + } + } + + /// The one dynamic landing state renders a date and nothing else. + #[test] + fn the_capped_query_carries_a_bare_date() { + assert_eq!( + capped_query("2026-09-01"), + "?rework=capped&next_eligible_at=2026-09-01" + ); + } + + /// `?rework=…` and `?issue=…` never collide: the page reads them as two + /// separate one-shot params, and a rework landing must never render as + /// an issue verdict. + #[test] + fn rework_and_issue_landings_live_under_different_params() { + assert!(REWORK_OK_QUERY.starts_with("?rework=")); + assert!(super::super::issue::ISSUE_OK_QUERY.starts_with("?issue=")); + } +} diff --git a/packages/prices-api/src/portal/auth/state_token.rs b/packages/prices-api/src/portal/auth/state_token.rs index ae46d632..54bfb5cd 100644 --- a/packages/prices-api/src/portal/auth/state_token.rs +++ b/packages/prices-api/src/portal/auth/state_token.rs @@ -94,6 +94,16 @@ pub enum Action { /// complete an issuance — that is exactly what the mismatch check refuses. #[serde(rename = "issue")] Issue, + /// Replace an API key (task 0191). + /// + /// The same shape as [`Action::Issue`] — the round-trip is the proof — but + /// the callback checks **membership only**: an account old enough once is + /// old enough forever, so age is never re-checked on a rework. The cap + /// (one rework per quota period) is decided there too, against the + /// current key's `createdDate`. A `signin` or `issue` callback cannot + /// complete a rework; the mismatch check refuses it. + #[serde(rename = "rework")] + Rework, /// An extra action that exists **only under `cfg(test)`**. /// /// It is never parsed from a query string, never minted by [`start`] @@ -116,14 +126,15 @@ impl Action { /// Parse the `action` query parameter of `/auth/login`. /// /// An unknown value is rejected rather than defaulted. Defaulting would - /// mean a client asking for an action this build does not know (0191's - /// `rework`, say) would silently get a sign-in — and the callback would + /// mean a client asking for an action this build does not know (0192's + /// `revoke`, say) would silently get a sign-in — and the callback would /// then complete a *different* action than the one confirmed, which is the /// exact thing the slot exists to prevent. pub fn parse(raw: &str) -> Option { match raw { "signin" => Some(Self::SignIn), "issue" => Some(Self::Issue), + "rework" => Some(Self::Rework), // `TestOther` is deliberately absent: it must not be reachable from // a query string even in a test build. _ => None, @@ -450,12 +461,12 @@ mod tests { assert_eq!(state.action, Action::SignIn); assert_eq!(pending.action, Action::SignIn); - // And a state naming an action this build does not know — 0191's - // `rework`, arriving early — is refused at deserialization. + // And a state naming an action this build does not know — 0192's + // `revoke`, arriving early — is refused at deserialization. let crossed = sign_claims( KEY, CTX_STATE, - &serde_json::json!({ "action": "rework", "nonce": pending.nonce, "exp": state.exp }), + &serde_json::json!({ "action": "revoke", "nonce": pending.nonce, "exp": state.exp }), ); assert_eq!( accept(KEY, &crossed, Some(&started.pending_cookie), NOW).unwrap_err(), @@ -515,11 +526,52 @@ mod tests { fn an_unknown_action_query_value_is_rejected_rather_than_defaulted() { assert_eq!(Action::parse("signin"), Some(Action::SignIn)); assert_eq!(Action::parse("issue"), Some(Action::Issue)); - for unknown in ["rework", "", "SIGNIN", "ISSUE", "issue ", "signin "] { + assert_eq!(Action::parse("rework"), Some(Action::Rework)); + for unknown in [ + "revoke", "", "SIGNIN", "ISSUE", "REWORK", "issue ", "signin ", "rework ", + ] { assert_eq!(Action::parse(unknown), None, "accepted {unknown:?}"); } } + /// The third real action (task 0191): a rework pair round-trips and + /// reports itself, and crossing it with an ISSUE cookie is refused — the + /// two re-authorisation flows are as non-interchangeable with each other + /// as either is with sign-in, which is what stops an issue round-trip's + /// proof (membership + age) being spent on a rework, or a rework's + /// (membership only) on an issue. + #[test] + fn a_rework_pair_round_trips_and_cannot_be_completed_by_an_issue_cookie() { + let started = start(KEY, Action::Rework, NOW); + let accepted = accept( + KEY, + &started.state_param, + Some(&started.pending_cookie), + NOW + 1, + ) + .expect("a freshly minted rework pair must verify"); + assert_eq!(accepted.action, Action::Rework); + + let state: StateClaims = verify_claims(KEY, CTX_STATE, &started.state_param).unwrap(); + for other in [Action::Issue, Action::SignIn] { + let crossed_pending = sign_claims( + KEY, + CTX_PENDING, + &PendingClaims { + action: other, + nonce: state.nonce.clone(), + verifier: "a-verifier".into(), + exp: state.exp, + }, + ); + assert_eq!( + accept(KEY, &started.state_param, Some(&crossed_pending), NOW).unwrap_err(), + StateError::ActionMismatch, + "{other:?}" + ); + } + } + /// The variant 0186 carried the slot for: an issue round-trip mints, /// verifies and reports `Action::Issue` end to end. #[test] diff --git a/packages/prices-api/src/portal/eligibility.rs b/packages/prices-api/src/portal/eligibility.rs index 19a97938..d0920866 100644 --- a/packages/prices-api/src/portal/eligibility.rs +++ b/packages/prices-api/src/portal/eligibility.rs @@ -12,7 +12,7 @@ //! | Sign in | — | identity only | //! | Issue a key | **yes** | membership (`pending === false`) + account age | //! | Reveal / usage | no | session only | -//! | Rework ([0191]) | **yes** | membership only — age is never re-checked | +//! | Rework ([0191]) | **yes** | membership only ([`membership`]) — age is never re-checked | //! | Revoke ([0192]) | no | session only — a **deliberate exception**: a user must be able to kill a leaked key while Discord is down | //! //! Account age is checked only at issuance because an account old enough once @@ -236,24 +236,10 @@ pub fn decide( min_age_minutes: u64, now_ms: u64, ) -> Eligibility { - match member { - MemberLookup::NotMember { .. } => return Eligibility::NotMember, - MemberLookup::Unknown { status, detail } => { - tracing::warn!(?status, detail, "membership could not be verified"); - return Eligibility::Unknown; - } - MemberLookup::Member(m) => match m.pending { - Some(false) => {} - Some(true) => return Eligibility::NotMember, - None => { - tracing::warn!( - reason = "pending_absent", - "the member response carried no `pending` field; refusing without \ - accusation — see 0180 item 2 before changing this arm" - ); - return Eligibility::Unknown; - } - }, + match membership(member) { + Membership::Member => {} + Membership::NotMember => return Eligibility::NotMember, + Membership::Unknown => return Eligibility::Unknown, } let Some(created_ms) = account_created_ms(snowflake) else { @@ -270,6 +256,47 @@ pub fn decide( } } +/// The membership half of the verdict, on its own. +/// +/// What a **rework** (task 0191) re-proves: the per-action table at the top +/// of this module says membership only, never age, because an account old +/// enough once is old enough forever. [`decide`] is this plus the age check, +/// so the two paths cannot disagree about what a member is — the `pending` +/// rules below are the single statement of it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Membership { + /// `pending == Some(false)` — a full member, admin bypass included. + Member, + /// Confirmed non-membership, or `pending == Some(true)`. + NotMember, + /// Could not verify, including an absent `pending`. Refuse, never accuse. + Unknown, +} + +/// Classify one membership answer. See [`decide`] for the `pending` rules, +/// which live in this function and are documented there. +pub fn membership(member: &MemberLookup) -> Membership { + match member { + MemberLookup::NotMember { .. } => Membership::NotMember, + MemberLookup::Unknown { status, detail } => { + tracing::warn!(?status, detail, "membership could not be verified"); + Membership::Unknown + } + MemberLookup::Member(m) => match m.pending { + Some(false) => Membership::Member, + Some(true) => Membership::NotMember, + None => { + tracing::warn!( + reason = "pending_absent", + "the member response carried no `pending` field; refusing without \ + accusation — see 0180 item 2 before changing this arm" + ); + Membership::Unknown + } + }, + } +} + /// Milliseconds since the Unix epoch, saturating like `state_token::now_secs`: /// a clock before 1970 makes every account look brand new, which fails closed. pub fn now_ms() -> u64 { @@ -456,6 +483,37 @@ mod tests { assert_eq!(settings.guild_id().await.unwrap(), "897514728459468821"); } + /// The rework path reads [`membership`] alone; it must agree with + /// [`decide`] on every membership shape, or a rework could be allowed to + /// someone an issue would refuse (or the reverse). + #[test] + fn the_membership_half_agrees_with_the_full_verdict() { + let now = DOCUMENTED_CREATED_MS + 6 * 60_000; + let cases = [ + (member(Some(false)), Membership::Member), + (member(Some(true)), Membership::NotMember), + (member(None), Membership::Unknown), + ( + MemberLookup::NotMember { code: 10_007 }, + Membership::NotMember, + ), + ( + MemberLookup::NotMember { code: 10_004 }, + Membership::NotMember, + ), + (unknown(), Membership::Unknown), + ]; + for (lookup, expected) in cases { + assert_eq!(membership(&lookup), expected, "{lookup:?}"); + let full = decide(&lookup, DOCUMENTED_SNOWFLAKE, 5, now); + match expected { + Membership::Member => assert_eq!(full, Eligibility::Eligible), + Membership::NotMember => assert_eq!(full, Eligibility::NotMember), + Membership::Unknown => assert_eq!(full, Eligibility::Unknown), + } + } + } + #[test] fn an_unparseable_snowflake_is_unknown_not_too_young() { assert_eq!( diff --git a/packages/prices-api/src/portal/keys/cap.rs b/packages/prices-api/src/portal/keys/cap.rs new file mode 100644 index 00000000..9aa22afd --- /dev/null +++ b/packages/prices-api/src/portal/keys/cap.rs @@ -0,0 +1,187 @@ +//! The rework cap: one new key per quota period (task 0191). +//! +//! **The rule.** A key may be replaced only when it was created **before** the +//! current quota period began — the 1st of the calendar month, 00:00 UTC, +//! under [`Period`]'s rule. A rework deletes the old key and creates a new +//! one, so the surviving key's `createdDate` *is* the instant of the last +//! rework; no stored timestamp is needed, which is the fact that let task 0190 +//! cancel the registry table. +//! +//! **Why creation time, not a separate "last rotated" stamp.** Quota is scoped +//! to `(usagePlanId, apiKeyId)`, so a new key is a clean counter. If only +//! reworks were capped, a user could take a key on the 1st, spend the whole +//! quota, and rework on the 2nd into a fresh 100 000 — the exact loophole the +//! cap exists to close. Any key acquired inside the current period was created +//! inside it, so it can never be reworked inside it: one key per period, one +//! quota. +//! +//! **Worked example** (the 2026-08-07 meeting's): a key reworked on 3 August +//! is refused until 1 September 00:00 UTC, and allowed from that instant. The +//! refusal carries the date, because the wait is weeks and "try again later" +//! would be a lie by omission. +//! +//! Pure, so the whole boundary table is decidable by a test with dates typed +//! by hand. No clock is read here: the caller supplies the period. + +use super::super::period::Period; + +/// Whether a key may be replaced now. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Cap { + /// The key predates the current period: a rework is allowed. + Allowed, + /// The key was created inside the current period (or cannot be dated — + /// see [`decide`]). Refused until the period rolls. + Capped { + /// The instant the next rework becomes available, RFC 3339 — the + /// `next_eligible_at` in the `409` envelope. + next_eligible_at: String, + /// The same instant as a bare `YYYY-MM-DD`, for the callback's landing + /// query: digits and dashes only, so no request-derived byte can reach + /// a `Location` header through it. + next_eligible_date: String, + }, +} + +/// Decide the cap for a key created at `created_at` (Unix seconds, as API +/// Gateway reports `createdDate`) against `period`. +/// +/// Strictly **before** the period start: a key created at exactly +/// 00:00:00 on the 1st was created inside the period and is capped until the +/// next one — the boundary instant belongs to the period it begins. +/// +/// `None` — a key AWS did not date — is **capped**, not allowed. The service +/// always sends `createdDate`, so this is a shape that should never occur; +/// when it does, the cap cannot prove the key predates the period, and the +/// failure that matters is a quota laundered through a rework, not a rework +/// delayed to the 1st. Logged, because a deployment where it fires has a +/// control plane answering in a shape nobody has seen. +pub fn decide(created_at: Option, period: &Period) -> Cap { + let capped = || Cap::Capped { + next_eligible_at: period.resets_at(), + next_eligible_date: period.next_start_ymd(), + }; + match created_at { + Some(created_at) if created_at < period.start_secs() => Cap::Allowed, + Some(_) => capped(), + None => { + tracing::warn!( + "the current key carries no createdDate; refusing the rework rather than \ + assuming it predates the period" + ); + capped() + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use chrono::NaiveDate; + + fn period(y: i32, m: u32, d: u32) -> Period { + Period::containing(NaiveDate::from_ymd_opt(y, m, d).unwrap()) + } + + /// Unix seconds for a UTC date-time, so the table below reads as dates. + fn at(y: i32, m: u32, d: u32, h: u32, min: u32, s: u32) -> u64 { + NaiveDate::from_ymd_opt(y, m, d) + .unwrap() + .and_hms_opt(h, min, s) + .unwrap() + .and_utc() + .timestamp() as u64 + } + + /// The meeting's worked example, both halves: reworked on 3 August, + /// refused for the rest of August naming 1 September — and allowed from + /// the first second of 1 September. + #[test] + fn reworked_on_3_august_refuses_until_1_september_and_succeeds_on_it() { + let reworked = at(2026, 8, 3, 14, 30, 0); + + for day in [3, 4, 20, 31] { + assert_eq!( + decide(Some(reworked), &period(2026, 8, day)), + Cap::Capped { + next_eligible_at: "2026-09-01T00:00:00Z".into(), + next_eligible_date: "2026-09-01".into(), + }, + "August {day}" + ); + } + + assert_eq!(decide(Some(reworked), &period(2026, 9, 1)), Cap::Allowed); + assert_eq!(decide(Some(reworked), &period(2026, 9, 30)), Cap::Allowed); + } + + /// The cap is about creation, not about reworks: a key *issued* inside + /// the period is just as capped as one reworked inside it. This is the + /// loophole the fallback closes — take a key on the 1st, drain it, rework + /// on the 2nd into a clean counter. + #[test] + fn a_key_issued_inside_the_period_cannot_be_reworked_inside_it() { + let issued_on_the_1st = at(2026, 8, 1, 9, 0, 0); + assert!(matches!( + decide(Some(issued_on_the_1st), &period(2026, 8, 2)), + Cap::Capped { .. } + )); + } + + /// The boundary instant belongs to the period it begins: a key created at + /// exactly 00:00:00 on the 1st is inside the period; one second earlier + /// is in the previous one and may be reworked. + #[test] + fn the_boundary_instant_is_inside_the_new_period() { + let midnight = at(2026, 8, 1, 0, 0, 0); + assert!(matches!( + decide(Some(midnight), &period(2026, 8, 15)), + Cap::Capped { .. } + )); + assert_eq!( + decide(Some(midnight - 1), &period(2026, 8, 15)), + Cap::Allowed + ); + } + + /// December names January of the following year. + #[test] + fn a_december_refusal_names_the_new_year() { + assert_eq!( + decide(Some(at(2026, 12, 10, 0, 0, 0)), &period(2026, 12, 24)), + Cap::Capped { + next_eligible_at: "2027-01-01T00:00:00Z".into(), + next_eligible_date: "2027-01-01".into(), + } + ); + } + + /// A key AWS did not date cannot be shown to predate the period, so it + /// is refused — the failure that matters is the laundered quota, not the + /// delayed rework. + #[test] + fn an_undated_key_is_capped_not_waved_through() { + assert!(matches!( + decide(None, &period(2026, 8, 15)), + Cap::Capped { .. } + )); + } + + /// The landing-query form is digits and dashes only, by construction — + /// it reaches a `Location` header. + #[test] + fn the_date_form_is_url_safe_by_construction() { + let Cap::Capped { + next_eligible_date, .. + } = decide(Some(u64::MAX), &period(2026, 8, 15)) + else { + panic!("a future key is capped"); + }; + assert!( + next_eligible_date + .bytes() + .all(|b| b.is_ascii_digit() || b == b'-') + ); + assert_eq!(next_eligible_date.len(), 10); + } +} diff --git a/packages/prices-api/src/portal/keys/mod.rs b/packages/prices-api/src/portal/keys/mod.rs index c727fc00..3fa0c498 100644 --- a/packages/prices-api/src/portal/keys/mod.rs +++ b/packages/prices-api/src/portal/keys/mod.rs @@ -8,12 +8,21 @@ //! | route | does | //! | --- | --- | //! | `GET`/`POST /api-tokens/api/key` | reveal — the lookup, plus the value. **Never creates.** | +//! | `POST /api-tokens/api/key/rework` | the rework **pre-check** (task 0191): may this key be replaced now? `409` with `next_eligible_at` if not. **Never writes.** | //! //! Issuance itself is [`issue_for`], reachable **only** from the OAuth //! callback completing an `action=issue` round-trip //! (`super::auth::issue`) — because issuing requires an eligibility proof //! (Stellar Discord membership + minimum account age, ADR 0010 §8) that only a -//! fresh Discord token can provide, and a session cookie is not one. +//! fresh Discord token can provide, and a session cookie is not one. The +//! rework — delete the old key, create the new one — is [`rework_for`], and +//! is reachable **only** from the callback completing an `action=rework` +//! round-trip (`super::auth::rework`), for the same reason: it re-proves +//! membership (not age) against a fresh token. What the `/key/rework` route +//! above does is answer the question the modal asks *before* sending the +//! visitor to Discord — is the cap in the way? — and it answers it read-only, +//! from the current key's `createdDate` and [`cap`]'s rule. The callback +//! decides the cap again, authoritatively, with the fresh token in hand. //! //! # There is no database, and that is the design //! @@ -71,6 +80,7 @@ //! tracing subscriber's `info` level records the route and the outcome, never //! the credential. +pub mod cap; pub mod gateway; pub mod naming; @@ -82,18 +92,25 @@ use axum::Router; use axum::extract::State; use axum::http::{HeaderMap, StatusCode}; use axum::response::{IntoResponse, Response}; -use axum::routing::get; +use axum::routing::{get, post}; use serde::Serialize; use crate::common::{cache_control, errors}; use super::auth::secret::OauthSecret; +use super::period::Period; +use cap::Cap; use gateway::{Attachment, Gateway, GatewayError, KeyValue}; use naming::{KeyRecord, choose_winner, exact_matches, key_name, losers}; /// The reveal, on both verbs — see [`key`] for why `POST` answers identically. pub const KEY_PATH: &str = "/api-tokens/api/key"; +/// The rework pre-check (task 0191). `POST` only: it is the answer to "may I +/// replace my key?", asked by the confirmation modal, and although it writes +/// nothing it is not a resource to `GET` — a link to it has no meaning. +pub const REWORK_PATH: &str = "/api-tokens/api/key/rework"; + /// Error code for a caller with no valid session. const NOT_SIGNED_IN: &str = "not_signed_in"; /// Error code for a control plane that would not answer. @@ -105,6 +122,12 @@ const KEYS_UNCONFIGURED: &str = "keys_unconfigured"; /// because the frontend reads both: a `404` is "no key" only when this /// envelope says so. const NO_KEY: &str = "no_key"; +/// Error code for a rework refused by the once-per-period cap (task 0191). +/// +/// A `409`, not a `429`: `429` says "retry shortly" and the wait here can be +/// weeks. The envelope's `details` carries `next_eligible_at`, RFC 3339 — the +/// slot `ErrorEnvelope` has had for exactly this kind of structured context. +const REWORK_CAPPED: &str = "rework_capped"; /// How many times the whole flow is re-run when the key it settled on turns out /// to have been deleted underneath it. @@ -199,6 +222,7 @@ impl KeysState { pub fn routes(state: KeysState) -> Router { Router::new() .route(KEY_PATH, get(key).post(key)) + .route(REWORK_PATH, post(rework_check)) .with_state(state) } @@ -343,6 +367,128 @@ async fn reveal(state: &KeysState, headers: &HeaderMap) -> Response { } } +/// What the rework pre-check answers when the cap is not in the way. +/// +/// Deliberately small: the modal needs one bit. The `next_eligible_at` a +/// refusal carries lives in the `409` envelope's `details`, where the page +/// already knows to look for structured context. +#[derive(Serialize)] +struct ReworkCheck { + /// Always `true` on a `200` — a refusal is a `409`, never a `200` with + /// `false`, so a client that ignores the status cannot read "refused" as + /// "go ahead". + eligible: bool, +} + +/// `POST /key/rework` — may this caller's key be replaced now? (task 0191) +/// +/// **Read-only, like the reveal**: one paginated listing, no value read, no +/// write of any kind. A session cookie is enough to *ask* because asking +/// changes nothing and the answer is about the caller's own key; the rework +/// itself needs the `action=rework` round-trip and its fresh membership +/// proof, and the callback decides the cap again before any key moves — this +/// route exists so the modal can say "next eligible 1 September" *before* +/// the visitor types the confirmation phrase and crosses Discord for a +/// refusal. +/// +/// | answer | meaning | +/// | --- | --- | +/// | `200 {eligible: true}` | the key predates the current period; the modal may arm | +/// | `409 rework_capped` + `details.next_eligible_at` | created inside this period — refused until the 1st | +/// | `404 no_key` | nothing to replace; the page offers the issue round-trip instead | +/// | `401` / `502` / `503` | as the reveal | +async fn rework_check(State(state): State, headers: HeaderMap) -> Response { + let Some(oauth) = state.oauth.as_ref() else { + return unconfigured(); + }; + let Some(gateway) = state.gateway.as_ref() else { + return unconfigured(); + }; + let Some(session) = super::auth::current_session(oauth, &headers) else { + return no_store(errors::unauthorized_with( + NOT_SIGNED_IN, + "sign in with Discord before asking to replace your API key", + )); + }; + let Some(name) = key_name(&session.sub) else { + tracing::warn!("a session carried a user id that is not a snowflake; refusing"); + return no_store(errors::unauthorized_with( + NOT_SIGNED_IN, + "sign in with Discord before asking to replace your API key", + )); + }; + + let listed = match tokio::time::timeout(state.deadline, gateway.list_named(&name)).await { + Ok(Ok(listed)) => listed, + Ok(Err(error)) => { + tracing::error!(error = %error, "portal rework pre-check failed"); + return no_store( + ( + StatusCode::BAD_GATEWAY, + Json(errors::ErrorEnvelope { + code: KEY_UNAVAILABLE, + message: "could not reach the API key service; try again".into(), + details: None, + }), + ) + .into_response(), + ); + } + Err(_elapsed) => { + tracing::error!( + deadline_secs = state.deadline.as_secs_f32(), + "portal rework pre-check ran out of time" + ); + return no_store(errors::service_unavailable( + KEY_UNAVAILABLE, + "the API key service is taking too long to answer; try again", + )); + } + }; + + // The same winner the reveal hands out and the rework will replace, so + // the date the modal shows is computed from the key the visitor is + // looking at. + let candidates = exact_matches(listed, &name); + let Some(current) = choose_winner(&candidates) else { + return no_store( + ( + StatusCode::NOT_FOUND, + Json(errors::ErrorEnvelope { + code: NO_KEY, + message: "you have no API key to replace; issue one first".into(), + details: None, + }), + ) + .into_response(), + ); + }; + + match cap::decide(current.created_at, &Period::now()) { + Cap::Allowed => no_store(Json(ReworkCheck { eligible: true }).into_response()), + Cap::Capped { + next_eligible_at, .. + } => no_store(capped(next_eligible_at)), + } +} + +/// The `409` a capped rework answers — the envelope, with the date in the +/// slot the envelope already has for structured context. +fn capped(next_eligible_at: String) -> Response { + ( + StatusCode::CONFLICT, + Json(errors::ErrorEnvelope { + code: REWORK_CAPPED, + message: format!( + "your key was already issued or replaced this quota period; the next \ + replacement is available from {next_eligible_at}" + ), + details: Some(serde_json::json!({ "next_eligible_at": next_eligible_at })), + }), + ) + .into_response() +} + /// The read-only lookup: list, filter, rank, read. Nothing here mutates. /// /// `Ok(None)` is "no key to reveal" — including the raced case where the @@ -424,6 +570,191 @@ pub(crate) async fn issue_for(gateway: &Gateway, sub: &str, deadline: Duration) } } +/// What the eligibility-checked rework round-trip needs to know (task 0191). +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum ReworkOutcome { + /// A new key exists and is on the plan; the old one is gone. + Replaced, + /// There was no key to replace. The visitor is sent to issue one. + NoKey, + /// The current key was created inside this period. Nothing moved. + Capped { + /// `YYYY-MM-DD`, for the landing query. + next_eligible_date: String, + }, + /// The control plane would not complete the swap. The old key is left in + /// place wherever that was possible (see [`swap`]); the visitor is told to + /// try again. + Failed, +} + +/// Replace the key for `sub` — reachable only from `super::auth::rework` +/// after a fresh membership proof. +/// +/// The cap is decided **here**, against the listing this call makes, not +/// trusted from the pre-check: the pre-check answered a session cookie some +/// seconds ago, and the authority for "may this key be replaced" has to be +/// the same call that is about to replace it. +pub(crate) async fn rework_for(gateway: &Gateway, sub: &str, deadline: Duration) -> ReworkOutcome { + // Same guard as the reveal and the issue — and here, as in the issue, it + // stands in front of `DeleteApiKey`. + let Some(name) = key_name(sub) else { + tracing::warn!("a rework round-trip carried a user id that is not a snowflake; refusing"); + return ReworkOutcome::Failed; + }; + + match tokio::time::timeout(deadline, swap(gateway, &name)).await { + Ok(Ok(Swap::Replaced { record })) => { + tracing::info!(key_id = %record.id, "portal replaced an API key"); + ReworkOutcome::Replaced + } + Ok(Ok(Swap::NoKey)) => ReworkOutcome::NoKey, + Ok(Ok(Swap::Capped { next_eligible_date })) => { + tracing::info!( + outcome = "capped", + "portal rework refused by the period cap" + ); + ReworkOutcome::Capped { next_eligible_date } + } + Ok(Ok(Swap::Lost)) => { + tracing::error!( + "the replacement key was deleted underneath the rework; the visitor may be \ + keyless until the next issue round-trip" + ); + ReworkOutcome::Failed + } + Ok(Err(error)) => { + // `error` cannot carry a key value — see `gateway::sdk_message`. + tracing::error!(error = %error, "portal key rework failed"); + ReworkOutcome::Failed + } + Err(_elapsed) => { + tracing::error!( + deadline_secs = deadline.as_secs_f32(), + "portal key rework ran out of time" + ); + ReworkOutcome::Failed + } + } +} + +/// What one pass of [`swap`] found or did. +enum Swap { + Replaced { + record: KeyRecord, + }, + NoKey, + Capped { + next_eligible_date: String, + }, + /// The new key was created and then vanished before the swap completed — + /// a concurrent reconciler for the same name, or a console session. + Lost, +} + +/// The swap: **create and attach the new key first, then delete the old** — +/// so the visitor is never keyless, and the cap blocks the *next* rework +/// rather than the replacement. +/// +/// Step by step, with what each failure leaves behind: +/// +/// 1. List, filter, rank: the current key is the reveal's winner. None → +/// [`Swap::NoKey`]. Its `createdDate` decides the cap → [`Swap::Capped`]. +/// Nothing has been written. +/// 2. `CreateApiKey` under the same name. A failure here leaves the old key +/// exactly as it was. +/// 3. `CreateUsagePlanKey` for the new key. A failure here leaves an orphan +/// (enabled, on no plan) beside a working old key — the same orphan the +/// issue path's attach can leave, and the next issue round-trip adopts or +/// reconciles it. The old key still works. `KeyGone` means the new key +/// was deleted in the milliseconds since its creation → [`Swap::Lost`], +/// old key untouched. +/// 4. `DeleteApiKey` on **every** old key under the name — the winner and +/// any duplicates the reconciler had not yet swept. This is the instant +/// the visitor was warned about: the old key stops working here. If a +/// delete fails, the new key is deleted again (best effort) and the error +/// propagates: the visitor is told it failed, and what they hold is the +/// key they had, not two keys of which the page reveals the older. +/// 5. The new key's value is read back, not to hand out (the caller answers +/// with a redirect and the page reveals through the read-only route) but +/// to prove it still exists. `None` → [`Swap::Lost`]: something deleted +/// the replacement between the attach and now; the old key is gone too. +/// Rare — it needs a concurrent reconciler for this exact name — and the +/// heal is one issue round-trip, which recreates. Reported as a failure, +/// loudly, rather than papered over by creating a third key inside a +/// budget that is already mostly spent. +/// +/// **Two simultaneous reworks** (two tabs; the modal's disabled confirm +/// stops a double-click in one) each pass the cap against the same old key, +/// each create, each attach, each delete the old key — and two new keys +/// survive. Not keyless, not a quota launder (both are capped until the +/// 1st), and the next issue round-trip's reconciler converges them on the +/// earliest. Accepted rather than serialised: there is no lock to take on a +/// control plane that is the source of truth, and the deterministic winner +/// rule already makes the convergence safe. +async fn swap(gateway: &Gateway, name: &str) -> Result { + // Step 1. + let existing = exact_matches(gateway.list_named(name).await?, name); + let Some(current) = choose_winner(&existing) else { + return Ok(Swap::NoKey); + }; + if let Cap::Capped { + next_eligible_date, .. + } = cap::decide(current.created_at, &Period::now()) + { + return Ok(Swap::Capped { next_eligible_date }); + } + + // Step 2. The value is dropped on purpose — see `Outcome`. + let (replacement, _value) = gateway.create(name).await?; + + // Step 3. + if gateway.attach_to_free_plan(&replacement.id).await? == Attachment::KeyGone { + return Ok(Swap::Lost); + } + + // Step 4. Every old key, not only the winner: a duplicate left by an + // earlier double-submit is an old key too, and leaving it would leave the + // visitor a working credential they were told had died. + for old in &existing { + // The same last-line guard the reconciler keeps before its deletes: + // on the record, immediately before the destructive call. + if old.name != name { + tracing::error!( + key_id = %old.id, + "refusing to delete a key whose name is not the caller's; \ + the exact-name filter has been bypassed" + ); + continue; + } + if let Err(error) = gateway.delete(&old.id).await { + tracing::error!( + key_id = %old.id, + error = %error, + "could not delete the key being replaced; rolling the replacement back" + ); + if let Err(rollback) = gateway.delete(&replacement.id).await { + tracing::error!( + key_id = %replacement.id, + error = %rollback, + "could not roll the replacement back either; the next issue round-trip \ + reconciles the duplicate" + ); + } + return Err(error); + } + tracing::info!(key_id = %old.id, "portal deleted a replaced API key"); + } + + // Step 5. + match gateway.value_of(&replacement.id).await? { + Some(_value) => Ok(Swap::Replaced { + record: replacement, + }), + None => Ok(Swap::Lost), + } +} + /// The result of a successful reconciliation. /// /// Deliberately does **not** carry the key value. The issue flow's caller is a @@ -664,4 +995,16 @@ mod tests { assert_eq!(rest, "key"); assert!(!rest.contains('/')); } + + /// The rework pre-check is one segment deeper — `key/rework` — which is + /// still inside the depth the currently-deployed gateway maps (0205's + /// note: depth 1–2 under the prefix work today, depth 3 answers `403` + /// until the greedy proxy deploys). Pinned so it is not moved deeper. + #[test] + fn the_rework_route_is_under_the_key_route_and_no_deeper() { + assert!(REWORK_PATH.starts_with(super::super::PORTAL_API_PREFIX)); + assert_eq!(REWORK_PATH, format!("{KEY_PATH}/rework")); + let rest = REWORK_PATH.trim_start_matches(super::super::PORTAL_API_PREFIX); + assert_eq!(rest.split('/').count(), 2); + } } diff --git a/packages/prices-api/src/portal/mod.rs b/packages/prices-api/src/portal/mod.rs index f6ba7413..a1e0d178 100644 --- a/packages/prices-api/src/portal/mod.rs +++ b/packages/prices-api/src/portal/mod.rs @@ -45,6 +45,7 @@ pub mod auth; pub mod eligibility; pub mod keys; +pub mod period; pub mod usage; use axum::extract::{Request, State}; diff --git a/packages/prices-api/src/portal/period.rs b/packages/prices-api/src/portal/period.rs new file mode 100644 index 00000000..2be69c63 --- /dev/null +++ b/packages/prices-api/src/portal/period.rs @@ -0,0 +1,157 @@ +//! The quota period under **our** rule: the calendar month, UTC +//! (task 0188's dashboard, task 0191's rework cap). +//! +//! One definition, two readers. The usage route renders a period around +//! AWS's counters, and the rework cap decides whether a key's `createdDate` +//! falls before the current period began. Both used to be able to carry their +//! own copy of "the 1st of the month, 00:00 UTC"; with the cap this becomes a +//! correctness property rather than a label — a dashboard that says the quota +//! resets on the 1st while the cap counts from a different instant would let +//! a rework through that the page said was refused, or the reverse — so the +//! rule lives here and nowhere else. +//! +//! # This is our rule, not AWS's — and what has been measured +//! +//! AWS documents neither the instant its `MONTH` quota rolls nor the timezone +//! it rolls in (ADR 0010, correction #2). The only statement anywhere is an +//! example caption, *"creates a usage plan that resets at the beginning of +//! the month"*, and `offset` is a request count, not a time shift. Task 0191 +//! measures the instant on a `DAY`-period scratch plan as a proxy (the +//! result, with its date, is in that task's Step 0 table); the first real +//! `MONTH` rollover observable after this code exists is 1 September 2026. +//! Nothing here is contingent on the answer: if AWS's counter turns out to +//! roll at a different instant, the dashboard label is a UX wrinkle to word +//! around, and the cap is ours to define regardless. + +use chrono::{Datelike, NaiveDate, Utc}; + +/// The current quota period — a calendar month in UTC. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Period { + /// The 1st of the month. + start: NaiveDate, + /// The 1st of the following month: the instant the period ends, and the + /// instant a rework capped in this period becomes available again. + next_start: NaiveDate, +} + +impl Period { + /// The period containing `today`. + /// + /// Pure, so the December rollover and the leap year are decidable by a + /// unit test rather than by waiting for one. AWS is deliberately not + /// consulted — see the module docs. + pub fn containing(today: NaiveDate) -> Self { + let start = NaiveDate::from_ymd_opt(today.year(), today.month(), 1) + .expect("the 1st of a real month exists"); + let next_start = if today.month() == 12 { + NaiveDate::from_ymd_opt(today.year() + 1, 1, 1) + } else { + NaiveDate::from_ymd_opt(today.year(), today.month() + 1, 1) + } + .expect("the 1st of the following month exists"); + Self { start, next_start } + } + + /// The period the clock says we are in, UTC. + pub fn now() -> Self { + Self::containing(Utc::now().date_naive()) + } + + /// First day of the period, `YYYY-MM-DD`. + pub fn start_ymd(&self) -> String { + self.start.format("%Y-%m-%d").to_string() + } + + /// Last day of the period, inclusive, `YYYY-MM-DD` — what `GetUsage`'s + /// `endDate` expects. + pub fn end_ymd(&self) -> String { + self.next_start + .pred_opt() + .expect("the day before the 1st exists") + .format("%Y-%m-%d") + .to_string() + } + + /// First day of the **following** period, `YYYY-MM-DD` — the date a + /// rework refused in this period names as "next eligible". + pub fn next_start_ymd(&self) -> String { + self.next_start.format("%Y-%m-%d").to_string() + } + + /// The instant this period ends and the next begins, RFC 3339 — the + /// dashboard's `resets_at` and the rework refusal's `next_eligible_at` + /// are the same instant by construction. + pub fn resets_at(&self) -> String { + format!("{}T00:00:00Z", self.next_start_ymd()) + } + + /// The instant this period began, in Unix seconds — what a key's + /// `createdDate` (also Unix seconds) is compared against. + pub fn start_secs(&self) -> u64 { + let secs = self + .start + .and_hms_opt(0, 0, 0) + .expect("midnight exists") + .and_utc() + .timestamp(); + u64::try_from(secs).unwrap_or(0) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn period(y: i32, m: u32, d: u32) -> Period { + Period::containing(NaiveDate::from_ymd_opt(y, m, d).unwrap()) + } + + /// Mid-month, mid-year — the ordinary case. + #[test] + fn the_period_is_the_calendar_month() { + let p = period(2026, 8, 19); + assert_eq!(p.start_ymd(), "2026-08-01"); + assert_eq!(p.end_ymd(), "2026-08-31"); + assert_eq!(p.next_start_ymd(), "2026-09-01"); + assert_eq!(p.resets_at(), "2026-09-01T00:00:00Z"); + } + + /// The 1st and the last day are both inside their own month. + #[test] + fn the_boundaries_belong_to_their_month() { + let first = period(2026, 9, 1); + assert_eq!(first.start_ymd(), "2026-09-01"); + assert_eq!(first.end_ymd(), "2026-09-30"); + let last = period(2026, 9, 30); + assert_eq!(last.start_ymd(), "2026-09-01"); + assert_eq!(last.resets_at(), "2026-10-01T00:00:00Z"); + } + + /// December resets into the next year. + #[test] + fn december_rolls_into_january() { + let p = period(2026, 12, 31); + assert_eq!(p.start_ymd(), "2026-12-01"); + assert_eq!(p.end_ymd(), "2026-12-31"); + assert_eq!(p.resets_at(), "2027-01-01T00:00:00Z"); + } + + /// February in a leap year has its 29th. + #[test] + fn a_leap_february_ends_on_the_29th() { + let p = period(2028, 2, 15); + assert_eq!(p.end_ymd(), "2028-02-29"); + assert_eq!(p.resets_at(), "2028-03-01T00:00:00Z"); + } + + /// The instant the period began, as the number a `createdDate` is + /// compared against: 2026-08-01T00:00:00Z is 1 785 542 400. + #[test] + fn the_start_instant_is_midnight_utc_on_the_first() { + assert_eq!(period(2026, 8, 21).start_secs(), 1_785_542_400); + assert_eq!(period(2026, 8, 1).start_secs(), 1_785_542_400); + // Epoch-adjacent dates do not underflow. + assert_eq!(period(1970, 1, 15).start_secs(), 0); + } +} diff --git a/packages/prices-api/src/portal/usage/mod.rs b/packages/prices-api/src/portal/usage/mod.rs index 39e6785e..9df7d8a7 100644 --- a/packages/prices-api/src/portal/usage/mod.rs +++ b/packages/prices-api/src/portal/usage/mod.rs @@ -81,7 +81,7 @@ use axum::extract::State; use axum::http::{HeaderMap, StatusCode}; use axum::response::{IntoResponse, Response}; use axum::routing::get; -use chrono::{Datelike, NaiveDate, SecondsFormat, Utc}; +use chrono::{SecondsFormat, Utc}; use serde::Serialize; use crate::common::{cache_control, errors}; @@ -89,6 +89,7 @@ use crate::common::{cache_control, errors}; use super::auth::secret::OauthSecret; use super::keys::gateway::{Gateway, GatewayError}; use super::keys::naming::{choose_winner, exact_matches, key_name}; +use super::period::Period; /// The one route. `GET` only — reading a counter must not share a path shape /// with anything that writes. @@ -195,6 +196,27 @@ struct EpochMark { pub struct UsageCache(Arc>); impl UsageCache { + /// Drop **whatever** is cached for `sub` — a real answer or a "no key" — + /// and bump the caller's epoch (task 0191). + /// + /// The rework path's eviction. A rework deletes the key the cached + /// numbers describe and issues one with a fresh counter, so a cached + /// usage answer is not stale but *about a key that no longer exists*; + /// serving it for the rest of the TTL would show the old key's + /// consumption under the new key's name. The epoch bump keeps an + /// in-flight "no key" from being stored (see [`CacheInner`]); an in-flight + /// **real** answer for the old key can still land for one TTL, which the + /// response's `as_of` dates honestly and which is bounded by + /// [`CACHE_TTL`] — accepted, because the alternative is discarding every + /// real answer whose lookup overlapped any rework. + pub fn invalidate(&self, sub: &str) { + { + let mut cache = self.0.lock().expect("the usage cache lock is not poisoned"); + cache.entries.remove(sub); + } + self.invalidate_no_key(sub); + } + /// Drop a cached "no key" answer for `sub`, if that is what is cached — /// and bump the caller's epoch either way, so an in-flight lookup that /// snapshotted the keyless state cannot write it back afterwards (see @@ -352,12 +374,12 @@ async fn usage(State(state): State, headers: HeaderMap) -> Response // last month's answer wearing this month's label, and the minute after a // month boundary is exactly when a viewer checks whether the reset // happened. - let period_now = current_period(Utc::now().date_naive()); + let period_start = Period::now().start_ymd(); // Fresh cache hit: no control-plane call of any kind. This is the // "repeated dashboard loads do not produce one GetUsage call each" // acceptance criterion, in one branch. - if let Some(entry) = cached(&state, &session.sub, state.ttl, &period_now.start) { + if let Some(entry) = cached(&state, &session.sub, state.ttl, &period_start) { return answer(entry.answer); } @@ -376,7 +398,7 @@ async fn usage(State(state): State, headers: HeaderMap) -> Response // timing — so it gets the same answer as the throttle arm below: // the last good answer (re-stamped, so the next TTL of loads // leaves the struggling control plane alone) beats the error page. - if let Some(entry) = cached(&state, &session.sub, STALE_KEEP, &period_now.start) { + if let Some(entry) = cached(&state, &session.sub, STALE_KEEP, &period_start) { tracing::warn!( deadline_secs = state.deadline.as_secs_f32(), "portal usage lookup ran out of time; serving the cached answer" @@ -407,7 +429,7 @@ async fn usage(State(state): State, headers: HeaderMap) -> Response // happening and invite a retry; the entry the next success writes ends // the condition. Err(GatewayError::Throttled { operation }) => { - if let Some(entry) = cached(&state, &session.sub, STALE_KEEP, &period_now.start) { + if let Some(entry) = cached(&state, &session.sub, STALE_KEEP, &period_start) { tracing::warn!( operation, "control plane is throttling; serving the cached usage answer" @@ -462,7 +484,7 @@ async fn fetch(gateway: &Gateway, name: &str) -> Result Result Result Period { - let start = NaiveDate::from_ymd_opt(today.year(), today.month(), 1) - .expect("the 1st of a real month exists"); - let next_first = if today.month() == 12 { - NaiveDate::from_ymd_opt(today.year() + 1, 1, 1) - } else { - NaiveDate::from_ymd_opt(today.year(), today.month() + 1, 1) - } - .expect("the 1st of the following month exists"); - let end = next_first - .pred_opt() - .expect("the day before the 1st exists"); - - Period { - start: start.format("%Y-%m-%d").to_string(), - end: end.format("%Y-%m-%d").to_string(), - resets_at: format!("{}T00:00:00Z", next_first.format("%Y-%m-%d")), - } -} - /// The cached entry for `sub`, if it is younger than `max_age` **and answers /// for the current period**. fn cached( @@ -693,47 +680,6 @@ mod tests { assert!(!rest.contains('/')); } - fn period(y: i32, m: u32, d: u32) -> Period { - current_period(NaiveDate::from_ymd_opt(y, m, d).unwrap()) - } - - /// Mid-month, mid-year — the ordinary case. - #[test] - fn the_period_is_the_calendar_month() { - let p = period(2026, 8, 19); - assert_eq!(p.start, "2026-08-01"); - assert_eq!(p.end, "2026-08-31"); - assert_eq!(p.resets_at, "2026-09-01T00:00:00Z"); - } - - /// The 1st and the last day are both inside their own month. - #[test] - fn the_boundaries_belong_to_their_month() { - let first = period(2026, 9, 1); - assert_eq!(first.start, "2026-09-01"); - assert_eq!(first.end, "2026-09-30"); - let last = period(2026, 9, 30); - assert_eq!(last.start, "2026-09-01"); - assert_eq!(last.resets_at, "2026-10-01T00:00:00Z"); - } - - /// December resets into the next year. - #[test] - fn december_rolls_into_january() { - let p = period(2026, 12, 31); - assert_eq!(p.start, "2026-12-01"); - assert_eq!(p.end, "2026-12-31"); - assert_eq!(p.resets_at, "2027-01-01T00:00:00Z"); - } - - /// February in a leap year has its 29th. - #[test] - fn a_leap_february_ends_on_the_29th() { - let p = period(2028, 2, 15); - assert_eq!(p.end, "2028-02-29"); - assert_eq!(p.resets_at, "2028-03-01T00:00:00Z"); - } - fn usage_answer(period_start: &str) -> CachedAnswer { CachedAnswer::Usage(UsageResponse { used: Some(1), diff --git a/packages/prices-api/tests/portal_auth.rs b/packages/prices-api/tests/portal_auth.rs index b484e4c1..4cdd98ca 100644 --- a/packages/prices-api/tests/portal_auth.rs +++ b/packages/prices-api/tests/portal_auth.rs @@ -395,7 +395,7 @@ async fn two_logins_produce_two_different_states() { } /// The action slot is verified at the door as well as in the signature, so an -/// action this build does not implement — 0191's `rework`, arriving early — +/// action this build does not implement — 0192's `revoke`, arriving early — /// cannot start a round-trip that the callback would then have to decide what /// to do with. #[tokio::test] @@ -407,7 +407,7 @@ async fn login_refuses_an_action_it_does_not_implement() { .status, StatusCode::SEE_OTHER ); - let refused = fetch(&open, &format!("{LOGIN_PATH}?action=rework"), &[]).await; + let refused = fetch(&open, &format!("{LOGIN_PATH}?action=revoke"), &[]).await; assert_eq!(refused.status, StatusCode::BAD_REQUEST); assert!(refused.set_cookies().is_empty()); } diff --git a/packages/prices-api/tests/portal_keys/harness.rs b/packages/prices-api/tests/portal_keys/harness.rs index bf17912c..1c9abed7 100644 --- a/packages/prices-api/tests/portal_keys/harness.rs +++ b/packages/prices-api/tests/portal_keys/harness.rs @@ -135,6 +135,20 @@ pub struct Store { /// reaches that code. Worth knowing — it means the branch guards a listing /// that disagrees with the reader, not a busy deleter. pub read_always_404: bool, + /// Stamp every key `CreateApiKey` mints with this `createdDate` instead of + /// the current time (task 0191) — so a test can create a key "on the 3rd + /// of last month" and then ask the cap about it. + pub created_at_override: Option, + /// Answer `DeleteApiKey` for exactly these ids with a 500, and succeed for + /// every other id (task 0191). Unlike the sticky `fail_deletes`, this lets + /// a test fail the delete of the OLD key while the rollback delete of the + /// replacement succeeds — which is the only way to observe the rollback. + pub fail_delete_of: Vec, + /// Every write, in the order it arrived: `create:`, `attach:`, + /// `delete:` (task 0191). The swap's ordering — create and attach the + /// new key BEFORE deleting the old — is a property of the sequence, not of + /// the end state, and this is what makes it assertable. + pub ops: Vec, pub next_id: usize, // ----------------------------------------------------------------------- @@ -322,9 +336,12 @@ pub async fn create_key( id: id.clone(), name: body["name"].as_str().unwrap_or_default().to_string(), value: format!("VALUE-{id}-CANARY"), - // Whole seconds, like the service — which is why the winner rule needs - // an id tie-break. - created_at: 1_800_000_000, + // Whole seconds and the CURRENT time, like the service — whole seconds + // is why the winner rule needs an id tie-break, and "now" is what lets + // task 0191's cap be tested: a key the mock just created must read as + // created inside the current quota period, exactly as a real one + // would. `created_at_override` pins it for tests that need a date. + created_at: store.created_at_override.unwrap_or_else(now_secs), tags: body["tags"] .as_object() .map(|o| { @@ -336,6 +353,7 @@ pub async fn create_key( enabled: body["enabled"].as_bool().unwrap_or(false), }; store.keys.push(created.clone()); + store.ops.push(format!("create:{id}")); (StatusCode::CREATED, Json(api_key_json(&created))).into_response() } @@ -378,10 +396,11 @@ pub async fn delete_key( Path(id): Path, ) -> Response { let mut store = store.lock().unwrap(); - if store.fail_deletes { + if store.fail_deletes || store.fail_delete_of.contains(&id) { return (StatusCode::INTERNAL_SERVER_ERROR, "control plane is unwell").into_response(); } store.deleted.push(id.clone()); + store.ops.push(format!("delete:{id}")); let existed = store.keys.iter().any(|k| k.id == id); store.keys.retain(|k| k.id != id); if existed { @@ -422,6 +441,7 @@ pub async fn attach_key( return conflict(); } store.plan_keys.push((plan, key_id.clone())); + store.ops.push(format!("attach:{key_id}")); ( StatusCode::CREATED, Json(json!({ "id": key_id, "type": "API_KEY" })), @@ -787,10 +807,20 @@ pub async fn post_key(mock: &MockGateway, sub: &str) -> Reply { /// callback's reply — a `303` whose `Location` is one of the five /// `?issue=…` landing states. pub async fn issue_round_trip(router: &Router) -> Reply { + round_trip(router, "issue").await +} + +/// The same, for task 0191's `action=rework` — a `303` whose `Location` is +/// one of the `?rework=…` landing states. +pub async fn rework_round_trip(router: &Router) -> Reply { + round_trip(router, "rework").await +} + +async fn round_trip(router: &Router, action: &str) -> Reply { let login = call_path( router.clone(), "GET", - "/api-tokens/api/auth/login?action=issue", + &format!("/api-tokens/api/auth/login?action={action}"), None, ) .await; diff --git a/packages/prices-api/tests/portal_rework.rs b/packages/prices-api/tests/portal_rework.rs new file mode 100644 index 00000000..df44e6e8 --- /dev/null +++ b/packages/prices-api/tests/portal_rework.rs @@ -0,0 +1,996 @@ +//! Replacing a key — the rework round-trip and its pre-check, over HTTP, +//! end to end (task 0191). +//! +//! Every test drives the real router through the real flow, the way +//! `tests/portal_issue.rs` does for issuance: `GET /auth/login?action=rework` +//! mints the signed state pair, the callback exchanges the code against a +//! **mock Discord**, asks it for the guild membership — and **not** the +//! account age — decides the once-per-period cap from the current key's +//! `createdDate`, and only then swaps the key against a **mock control +//! plane**. Both mocks record what they were asked, so the assertions are +//! about what the flow *did*: which calls happened, in which order, and how +//! many keys were created and deleted. +//! +//! Three properties are the spine of this file: +//! +//! - **Never keyless.** The new key is created and attached before the old +//! one is deleted, asserted on the mock's write *sequence* rather than on +//! the end state; and every failure leaves the visitor holding exactly the +//! key they had. +//! - **One rework per quota period.** A key created inside the period — +//! issued or reworked, it makes no difference — is refused with `409` and +//! a `next_eligible_at` that is the 1st of the next month, on the pre-check +//! and on the round-trip alike. +//! - **Unreachable with a session cookie alone.** The only thing a session +//! reaches is the read-only pre-check; the swap needs the round-trip. + +#[path = "portal_keys/harness.rs"] +mod harness; +#[path = "common/mock_discord.rs"] +mod mock_discord; + +use axum::Router; +use axum::http::{StatusCode, header}; +use chrono::{Datelike, NaiveDate, Utc}; + +use harness::*; +use mock_discord::{GRANTED_SCOPE, MemberReply, MockDiscord}; +use prices_api::portal::auth::discord::Endpoints; +use prices_api::portal::auth::{cookies, state_token}; +use prices_api::portal::keys::gateway::Gateway; +use prices_api::portal::keys::{KEY_PATH, REWORK_PATH}; +use prices_api::portal::usage::USAGE_PATH; + +fn rework_app(discord: &MockDiscord, gateway: &MockGateway) -> Router { + rework_app_with(discord, gateway, GUILD_ID, "5") +} + +fn rework_app_with( + discord: &MockDiscord, + gateway: &MockGateway, + guild_id: &str, + min_age_minutes: &str, +) -> Router { + build_app_with( + true, + Some(Gateway::against(&gateway.base, PLAN_ID.to_string())), + Endpoints { + api_base: discord.base.clone(), + ..Endpoints::default() + }, + Some(eligibility(guild_id, min_age_minutes)), + ) +} + +fn key_name() -> String { + format!("discord-{USER_ID}-key") +} + +/// Unix seconds for a UTC date at noon. +fn noon(date: NaiveDate) -> u64 { + date.and_hms_opt(12, 0, 0).unwrap().and_utc().timestamp() as u64 +} + +/// The 1st of the month `months` after this one, UTC. +fn first_of_month_offset(months: i32) -> NaiveDate { + let today = Utc::now().date_naive(); + let total = today.year() * 12 + today.month0() as i32 + months; + NaiveDate::from_ymd_opt(total.div_euclid(12), (total.rem_euclid(12) + 1) as u32, 1).unwrap() +} + +/// The meeting's worked example, relative to the real calendar: "reworked on +/// the 3rd" of the current month, and the 3rd of the previous month. +fn the_3rd_of(first: NaiveDate) -> u64 { + noon(first.with_day(3).unwrap()) +} + +fn next_eligible_at_expected() -> (String, String) { + let next = first_of_month_offset(1); + ( + format!("{}T00:00:00Z", next.format("%Y-%m-%d")), + next.format("%Y-%m-%d").to_string(), + ) +} + +/// Seed an attached key for the user, created at `created_at`. +fn seed_attached(gateway: &MockGateway, created_at: u64) -> String { + gateway.with(|s| { + let id = s.seed(&key_name(), created_at); + s.plan_keys.push((PLAN_ID.to_string(), id.clone())); + id + }) +} + +async fn precheck(router: &Router, cookie: Option<&str>) -> Reply { + call_path(router.clone(), "POST", REWORK_PATH, cookie).await +} + +// --------------------------------------------------------------------------- +// The gate — ships closed +// --------------------------------------------------------------------------- + +/// The whole rework surface is an empty `404` while the portal is closed — +/// the login that starts it, the callback that finishes it, and the +/// pre-check — with zero calls to Discord and zero to the control plane. +#[tokio::test] +async fn everything_including_rework_is_an_empty_404_while_the_portal_is_closed() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + seed_attached(&gateway, 1_000); + let closed = build_app_with( + false, + Some(Gateway::against(&gateway.base, PLAN_ID.to_string())), + Endpoints { + api_base: discord.base.clone(), + ..Endpoints::default() + }, + Some(eligibility(GUILD_ID, "5")), + ); + + for (method, path) in [ + ("GET", "/api-tokens/api/auth/login?action=rework"), + ("GET", "/api-tokens/api/auth/callback?code=c&state=s"), + ("POST", REWORK_PATH), + ] { + let reply = call_path(closed.clone(), method, path, Some(&session_cookie(USER_ID))).await; + assert_eq!(reply.status, StatusCode::NOT_FOUND, "{method} {path}"); + assert!(reply.body.is_empty(), "{method} {path} carried a body"); + } + assert_eq!(discord.exchanges(), 0); + assert_eq!(discord.member_calls(), 0); + assert_eq!(gateway.with(|s| s.list_calls), 0); + assert_eq!(gateway.with(|s| s.ops.len()), 0); +} + +// --------------------------------------------------------------------------- +// The happy path — a swap, never keyless +// --------------------------------------------------------------------------- + +/// A member with a key from last period gets a new one: the old is deleted, +/// the new is enabled and on the plan, the landing is `?rework=ok` with a +/// fresh session — and the new value rides nowhere in the redirect. +#[tokio::test] +async fn a_member_replaces_their_key_and_the_old_one_is_gone() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let old = seed_attached(&gateway, 1_000); + let app = rework_app(&discord, &gateway); + + let reply = rework_round_trip(&app).await; + assert_eq!(reply.status, StatusCode::SEE_OTHER); + assert_eq!(reply.location(), "/api-tokens/?rework=ok"); + assert!(reply.cookie(cookies::SESSION_COOKIE).is_some()); + + let stored = gateway.with(|s| s.keys.clone()); + assert_eq!(stored.len(), 1, "exactly one key survives"); + assert_ne!(stored[0].id, old, "and it is not the old one"); + assert_eq!(stored[0].name, key_name()); + assert!(stored[0].enabled); + assert!( + gateway.with(|s| s + .plan_keys + .contains(&(PLAN_ID.to_string(), stored[0].id.clone()))), + "the new key is on the plan" + ); + assert_eq!(gateway.with(|s| s.deleted.clone()), vec![old]); + + let headers = format!("{:?}", reply.headers); + assert!(!headers.contains(&stored[0].value), "{headers}"); + assert!(!headers.contains("CANARY"), "{headers}"); + + // The page's next step: the reveal hands out the NEW key. + let revealed = reveal(&gateway, USER_ID).await; + assert_eq!(revealed.status, StatusCode::OK); + assert_eq!(revealed.json()["key_id"], stored[0].id); +} + +/// **The ordering is the acceptance criterion.** The new key is created and +/// attached before the old one is deleted, so there is no instant at which +/// the visitor holds no working key. Asserted on the mock's write sequence: +/// the end state cannot tell a swap from a delete-then-create. +#[tokio::test] +async fn the_new_key_is_created_and_attached_before_the_old_is_deleted() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let old = seed_attached(&gateway, 1_000); + + assert_eq!( + rework_round_trip(&rework_app(&discord, &gateway)) + .await + .location(), + "/api-tokens/?rework=ok" + ); + + let ops = gateway.with(|s| s.ops.clone()); + let new = gateway.with(|s| s.keys[0].id.clone()); + assert_eq!( + ops, + vec![ + format!("create:{new}"), + format!("attach:{new}"), + format!("delete:{old}") + ], + "create → attach → delete, and nothing else" + ); +} + +/// What the data plane will do is a consequence of what the control plane +/// holds: the old key id no longer exists (→ `403 Forbidden` on `/v1/`, the +/// same answer as no key at all — measured under 0180 item 8), and the new +/// key is enabled and attached (→ `200`). The live `curl` of both is the +/// deploy-time check `packages/prices-api/README.md` names; this pins the +/// two facts it depends on. +#[tokio::test] +async fn the_old_key_is_deleted_and_the_new_one_is_enabled_and_on_the_plan() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let old = seed_attached(&gateway, 1_000); + + rework_round_trip(&rework_app(&discord, &gateway)).await; + + gateway.with(|s| { + assert!(!s.keys.iter().any(|k| k.id == old), "the old key is gone"); + let new = &s.keys[0]; + assert!(new.enabled); + assert!(s.plan_keys.contains(&(PLAN_ID.to_string(), new.id.clone()))); + // The old attachment is irrelevant once the key is gone, but the + // survivor must be the ONLY attached live key. + let live_attached: Vec<_> = s + .plan_keys + .iter() + .filter(|(_, id)| s.keys.iter().any(|k| &k.id == id)) + .collect(); + assert_eq!(live_attached.len(), 1); + }); +} + +/// Duplicates left by an earlier double-submit are all "the old key": every +/// one of them dies, or the visitor keeps a working credential they were +/// told had stopped. +#[tokio::test] +async fn every_old_key_under_the_name_is_replaced_not_only_the_winner() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let first = seed_attached(&gateway, 1_000); + let second = gateway.with(|s| s.seed(&key_name(), 2_000)); + + assert_eq!( + rework_round_trip(&rework_app(&discord, &gateway)) + .await + .location(), + "/api-tokens/?rework=ok" + ); + + let mut deleted = gateway.with(|s| s.deleted.clone()); + deleted.sort(); + let mut expected = vec![first, second]; + expected.sort(); + assert_eq!(deleted, expected); + assert_eq!(gateway.with(|s| s.keys.len()), 1); +} + +/// However two simultaneous reworks interleave (two tabs — one tab's confirm +/// is disabled on submit), the visitor ends with a working key: never none, +/// every survivor on the plan, the old one gone. Two survivors are possible +/// and accepted — both are capped until the 1st, and the next issue +/// round-trip's reconciler converges them. +#[tokio::test] +async fn two_simultaneous_reworks_never_leave_the_user_keyless() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let old = seed_attached(&gateway, 1_000); + let app = rework_app(&discord, &gateway); + + let (a, b) = tokio::join!(rework_round_trip(&app), rework_round_trip(&app)); + for reply in [&a, &b] { + assert!( + reply.location() == "/api-tokens/?rework=ok" + || reply.location() == "/api-tokens/?rework=failed", + "{}", + reply.location() + ); + } + + gateway.with(|s| { + assert!(!s.keys.iter().any(|k| k.id == old), "the old key is gone"); + assert!(!s.keys.is_empty(), "never keyless"); + assert!(s.keys.len() <= 2); + for key in &s.keys { + assert!( + s.plan_keys.contains(&(PLAN_ID.to_string(), key.id.clone())), + "every survivor is on the plan" + ); + } + }); + assert_eq!(reveal(&gateway, USER_ID).await.status, StatusCode::OK); +} + +// --------------------------------------------------------------------------- +// The cap — once per quota period +// --------------------------------------------------------------------------- + +/// A second rework in the same period is refused — `409` with +/// `next_eligible_at` on the pre-check, `?rework=capped&next_eligible_at=…` +/// on the round-trip — and nothing moves: the key the first rework made is +/// the key that stays. +#[tokio::test] +async fn a_second_rework_in_the_same_period_is_refused_with_409_and_next_eligible_at() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + seed_attached(&gateway, 1_000); + let app = rework_app(&discord, &gateway); + + assert_eq!( + rework_round_trip(&app).await.location(), + "/api-tokens/?rework=ok" + ); + let after_first = gateway.with(|s| { + ( + s.keys.iter().map(|k| k.id.clone()).collect::>(), + s.ops.clone(), + ) + }); + let (expected_at, expected_date) = next_eligible_at_expected(); + + // The pre-check: the modal's question, answered before any round-trip. + let check = precheck(&app, Some(&session_cookie(USER_ID))).await; + assert_eq!( + check.status, + StatusCode::CONFLICT, + "{}", + String::from_utf8_lossy(&check.body) + ); + let body = check.json(); + assert_eq!(body["code"], "rework_capped"); + assert_eq!(body["details"]["next_eligible_at"], expected_at); + assert_eq!(check.cache_control(), "no-store"); + + // The round-trip, should a client skip the pre-check. + let reply = rework_round_trip(&app).await; + assert_eq!( + reply.location(), + format!("/api-tokens/?rework=capped&next_eligible_at={expected_date}") + ); + assert_eq!( + gateway.with(|s| { + ( + s.keys.iter().map(|k| k.id.clone()).collect::>(), + s.ops.clone(), + ) + }), + after_first, + "a capped rework writes nothing" + ); +} + +/// The meeting's worked example against the real calendar: a key reworked on +/// the 3rd of THIS month is refused until the 1st of next month; the same +/// key dated the 3rd of LAST month — i.e. the calendar has rolled past the +/// 1st — is allowed. (The literal 3 August → 1 September dates are pinned +/// in `keys::cap`'s unit tests; this is the same rule over HTTP.) +#[tokio::test] +async fn reworked_on_the_3rd_refuses_until_the_1st_and_succeeds_once_it_has_passed() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let (expected_at, _) = next_eligible_at_expected(); + + // This month's 3rd: capped, naming next month's 1st. + let gateway = MockGateway::start().await; + seed_attached(&gateway, the_3rd_of(first_of_month_offset(0))); + let app = rework_app(&discord, &gateway); + let refused = precheck(&app, Some(&session_cookie(USER_ID))).await; + assert_eq!(refused.status, StatusCode::CONFLICT); + assert_eq!(refused.json()["details"]["next_eligible_at"], expected_at); + assert_eq!(gateway.with(|s| s.ops.len()), 0); + + // Last month's 3rd: the 1st has passed, so the rework goes through. + let gateway = MockGateway::start().await; + seed_attached(&gateway, the_3rd_of(first_of_month_offset(-1))); + let app = rework_app(&discord, &gateway); + let allowed = precheck(&app, Some(&session_cookie(USER_ID))).await; + assert_eq!(allowed.status, StatusCode::OK); + assert_eq!(allowed.json()["eligible"], true); + assert_eq!( + rework_round_trip(&app).await.location(), + "/api-tokens/?rework=ok" + ); +} + +/// The cap is about creation, not about reworks: a key **issued** this +/// period (the issue round-trip, seconds ago) cannot be reworked this period +/// either — which is the loophole the fallback closes. +#[tokio::test] +async fn a_key_issued_this_period_cannot_be_reworked_into_a_clean_counter() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let app = rework_app(&discord, &gateway); + + assert_eq!( + issue_round_trip(&app).await.location(), + "/api-tokens/?issue=ok" + ); + let issued = gateway.with(|s| s.keys[0].id.clone()); + + let reply = rework_round_trip(&app).await; + assert!( + reply + .location() + .starts_with("/api-tokens/?rework=capped&next_eligible_at="), + "{}", + reply.location() + ); + assert_eq!(gateway.with(|s| s.keys[0].id.clone()), issued); + assert_eq!(gateway.with(|s| s.deleted.len()), 0); +} + +// --------------------------------------------------------------------------- +// Unreachable with a session cookie alone +// --------------------------------------------------------------------------- + +/// The one route a session cookie reaches is the pre-check, and the +/// pre-check writes nothing — on the allowed answer, on the capped answer, +/// and on the no-key answer alike. A callback presented with a session but +/// no signed state is a `400` before any Discord call. +#[tokio::test] +async fn rework_is_unreachable_with_a_session_cookie_alone() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + seed_attached(&gateway, 1_000); + let app = rework_app(&discord, &gateway); + let session = session_cookie(USER_ID); + + let allowed = precheck(&app, Some(&session)).await; + assert_eq!(allowed.status, StatusCode::OK); + assert_eq!(allowed.cache_control(), "no-store"); + assert_eq!( + gateway.with(|s| s.ops.len()), + 0, + "an allowed pre-check writes nothing" + ); + assert_eq!(gateway.with(|s| s.keys.len()), 1); + + // The callback with a session and no state: refused at the door. + let callback = call_path( + app.clone(), + "GET", + "/api-tokens/api/auth/callback?code=c", + Some(&session), + ) + .await; + assert_eq!(callback.status, StatusCode::BAD_REQUEST); + assert_eq!(discord.exchanges(), 0); + assert_eq!(discord.member_calls(), 0); + + // `GET` on the pre-check path is not a route at all. + let get = call_path(app, "GET", REWORK_PATH, Some(&session)).await; + assert_eq!(get.status, StatusCode::METHOD_NOT_ALLOWED); + assert_eq!(gateway.with(|s| s.ops.len()), 0); +} + +#[tokio::test] +async fn the_pre_check_refuses_an_unauthenticated_caller_before_aws_is_touched() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let app = rework_app(&discord, &gateway); + + let reply = precheck(&app, None).await; + assert_eq!(reply.status, StatusCode::UNAUTHORIZED); + assert_eq!(reply.json()["code"], "not_signed_in"); + assert_eq!(gateway.with(|s| s.list_calls), 0); +} + +#[tokio::test] +async fn the_pre_check_answers_no_key_when_there_is_nothing_to_replace() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let app = rework_app(&discord, &gateway); + + let reply = precheck(&app, Some(&session_cookie(USER_ID))).await; + assert_eq!(reply.status, StatusCode::NOT_FOUND); + assert_eq!(reply.json()["code"], "no_key"); + assert_eq!(gateway.with(|s| s.ops.len()), 0); +} + +/// A round-trip for a visitor with no key lands on `no_key` and creates +/// nothing — a rework is not an issue with weaker checks. +#[tokio::test] +async fn a_rework_with_no_key_lands_on_no_key_and_creates_nothing() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + + let reply = rework_round_trip(&rework_app(&discord, &gateway)).await; + assert_eq!(reply.location(), "/api-tokens/?rework=no_key"); + assert_eq!(gateway.with(|s| s.create_calls), 0); + assert!(reply.cookie(cookies::SESSION_COOKIE).is_some()); +} + +// --------------------------------------------------------------------------- +// Membership — re-proved; age — never +// --------------------------------------------------------------------------- + +/// A user who has left the guild is refused on rework with the not-a-member +/// landing (the page names the server), nothing moves — and reveal still +/// works for them afterwards: the epic's non-goal holds. +#[tokio::test] +async fn a_user_who_has_left_the_guild_is_refused_on_rework_and_keeps_their_key() { + let discord = MockDiscord::start_with( + GRANTED_SCOPE, + None, + MemberReply::NotFound { code: 10_007 }, + mock_discord::USER_ID, + ) + .await; + let gateway = MockGateway::start().await; + let old = seed_attached(&gateway, 1_000); + let app = rework_app(&discord, &gateway); + + let reply = rework_round_trip(&app).await; + assert_eq!(reply.location(), "/api-tokens/?rework=not_member"); + assert!(reply.cookie(cookies::SESSION_COOKIE).is_some()); + assert_eq!(gateway.with(|s| s.ops.len()), 0, "nothing moves"); + assert_eq!(discord.member_calls(), 1); + + let revealed = call_path(app, "GET", KEY_PATH, Some(&session_cookie(USER_ID))).await; + assert_eq!(revealed.status, StatusCode::OK); + assert_eq!(revealed.json()["key_id"], old); +} + +/// A `429`/`5xx`/`401`/`403` from Discord refuses **without** claiming +/// non-membership — `unknown`, and nothing moves. +#[tokio::test] +async fn a_429_or_5xx_from_discord_refuses_the_rework_without_claiming_non_membership() { + for status in [ + StatusCode::TOO_MANY_REQUESTS, + StatusCode::INTERNAL_SERVER_ERROR, + StatusCode::SERVICE_UNAVAILABLE, + StatusCode::UNAUTHORIZED, + StatusCode::FORBIDDEN, + ] { + let discord = MockDiscord::start_with( + GRANTED_SCOPE, + None, + MemberReply::Status(status), + mock_discord::USER_ID, + ) + .await; + let gateway = MockGateway::start().await; + seed_attached(&gateway, 1_000); + + let reply = rework_round_trip(&rework_app(&discord, &gateway)).await; + assert_eq!(reply.location(), "/api-tokens/?rework=unknown", "{status}"); + assert_eq!(gateway.with(|s| s.ops.len()), 0, "{status}"); + } +} + +/// `pending` absent is `unknown` on a rework exactly as on an issue — the +/// two paths share one membership table. +#[tokio::test] +async fn an_absent_pending_field_is_unknown_on_rework_too() { + let discord = MockDiscord::start_with( + GRANTED_SCOPE, + None, + MemberReply::Member { pending: None }, + mock_discord::USER_ID, + ) + .await; + let gateway = MockGateway::start().await; + seed_attached(&gateway, 1_000); + + let reply = rework_round_trip(&rework_app(&discord, &gateway)).await; + assert_eq!(reply.location(), "/api-tokens/?rework=unknown"); + assert_eq!(gateway.with(|s| s.ops.len()), 0); +} + +/// **Age is never re-checked on a rework.** An account created ten seconds +/// ago — which the issue round-trip would refuse as too young under a +/// five-minute threshold — reworks its key: an account old enough once is +/// old enough forever. +#[tokio::test] +async fn account_age_is_not_re_checked_on_a_rework() { + const DISCORD_EPOCH_MS: u64 = 1_420_070_400_000; + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as u64; + let brand_new = ((now_ms - DISCORD_EPOCH_MS - 10_000) << 22).to_string(); + + let discord = MockDiscord::start_with( + GRANTED_SCOPE, + None, + MemberReply::Member { + pending: Some(false), + }, + &brand_new, + ) + .await; + let gateway = MockGateway::start().await; + gateway.with(|s| { + let id = s.seed(&format!("discord-{brand_new}-key"), 1_000); + s.plan_keys.push((PLAN_ID.to_string(), id)); + }); + let app = rework_app_with(&discord, &gateway, GUILD_ID, "5"); + + // The issue path, for contrast, refuses this account. + assert!( + issue_round_trip(&app) + .await + .location() + .starts_with("/api-tokens/?issue=too_young") + ); + // The rework path does not look. + assert_eq!( + rework_round_trip(&app).await.location(), + "/api-tokens/?rework=ok" + ); +} + +/// The membership call carries the FRESH token from this round-trip's own +/// exchange and names the configured guild — eligibility travels by +/// re-authentication on a rework exactly as on an issue. +#[tokio::test] +async fn the_member_call_carries_the_fresh_token_on_a_rework() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + seed_attached(&gateway, 1_000); + + rework_round_trip(&rework_app(&discord, &gateway)).await; + + assert_eq!(discord.exchanges(), 1); + assert_eq!(discord.member_calls(), 1); + assert_eq!( + discord.member_bearer().as_deref(), + Some("Bearer an-access-token") + ); + assert_eq!(discord.member_guild().as_deref(), Some(GUILD_ID)); +} + +/// The re-authorisation is a redirect, not a login: `prompt=none` rides on +/// the rework's authorize URL as it does on the issue's. +#[tokio::test] +async fn the_rework_round_trip_suppresses_the_consent_screen() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let app = rework_app(&discord, &gateway); + + let login = call_path(app, "GET", "/api-tokens/api/auth/login?action=rework", None).await; + let query = login.location().split_once('?').unwrap().1.to_string(); + let prompt = form_urlencoded::parse(query.as_bytes()) + .find(|(k, _)| k == "prompt") + .map(|(_, v)| v.to_string()); + assert_eq!(prompt.as_deref(), Some("none")); +} + +// --------------------------------------------------------------------------- +// Discord ending the round-trip early +// --------------------------------------------------------------------------- + +/// A narrow or wider grant lands on `?rework=denied` before any member call +/// — the registration drift the issue flow has a designed state for, and +/// the rework the same. +#[tokio::test] +async fn a_drifted_grant_lands_on_rework_denied_before_any_member_call() { + for drifted in ["identify", "identify guilds.members.read guilds"] { + let discord = MockDiscord::start(drifted, None).await; + let gateway = MockGateway::start().await; + seed_attached(&gateway, 1_000); + + let reply = rework_round_trip(&rework_app(&discord, &gateway)).await; + assert_eq!(reply.status, StatusCode::SEE_OTHER, "{drifted}"); + assert_eq!(reply.location(), "/api-tokens/?rework=denied", "{drifted}"); + assert_eq!(discord.member_calls(), 0, "{drifted}"); + assert_eq!(gateway.with(|s| s.ops.len()), 0, "{drifted}"); + } +} + +/// Cancelled at Discord's screen → `?rework=cancelled`; refused by Discord +/// for any other reason → `?rework=denied`. Neither is a verdict, and +/// neither lands on the issue's or sign-in's banners. +#[tokio::test] +async fn a_rework_ended_at_discord_lands_on_its_own_cancelled_or_denied_state() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let app = rework_app(&discord, &gateway); + + for (error, expected) in [ + ("access_denied", "/api-tokens/?rework=cancelled"), + ("invalid_scope", "/api-tokens/?rework=denied"), + ] { + let login = call_path( + app.clone(), + "GET", + "/api-tokens/api/auth/login?action=rework", + None, + ) + .await; + let pending = login.cookie(cookies::PENDING_COOKIE).unwrap(); + let query = login.location().split_once('?').unwrap().1.to_string(); + let state = form_urlencoded::parse(query.as_bytes()) + .find(|(k, _)| k == "state") + .unwrap() + .1 + .to_string(); + + let reply = call_path( + app.clone(), + "GET", + &format!("/api-tokens/api/auth/callback?error={error}&state={state}"), + Some(&format!("{}={pending}", cookies::PENDING_COOKIE)), + ) + .await; + assert_eq!(reply.location(), expected, "{error}"); + } + assert_eq!(discord.exchanges(), 0); +} + +/// Discord failing the exchange or the identity read on a rework is +/// `unknown` — a landing, never the sign-in arm's `502` page. +#[tokio::test] +async fn a_discord_failure_mid_round_trip_lands_on_rework_unknown() { + let gateway = MockGateway::start().await; + seed_attached(&gateway, 1_000); + + let exchange_down = + MockDiscord::start(GRANTED_SCOPE, Some(StatusCode::SERVICE_UNAVAILABLE)).await; + let reply = rework_round_trip(&rework_app(&exchange_down, &gateway)).await; + assert_eq!(reply.status, StatusCode::SEE_OTHER); + assert_eq!(reply.location(), "/api-tokens/?rework=unknown"); + + let identity_down = MockDiscord::start_full( + GRANTED_SCOPE, + None, + MemberReply::Member { + pending: Some(false), + }, + USER_ID, + Some(StatusCode::TOO_MANY_REQUESTS), + ) + .await; + let reply = rework_round_trip(&rework_app(&identity_down, &gateway)).await; + assert_eq!(reply.location(), "/api-tokens/?rework=unknown"); + assert_eq!(gateway.with(|s| s.ops.len()), 0); +} + +/// An unwired deployment refuses to START a rework, with a landing rather +/// than a JSON page, and mints no pending cookie. +#[tokio::test] +async fn an_unwired_deployment_refuses_to_start_a_rework() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let unwired = build_app_with( + true, + None, + Endpoints { + api_base: discord.base.clone(), + ..Endpoints::default() + }, + None, + ); + + let reply = call_path( + unwired, + "GET", + "/api-tokens/api/auth/login?action=rework", + None, + ) + .await; + assert_eq!(reply.status, StatusCode::SEE_OTHER); + assert_eq!(reply.location(), "/api-tokens/?rework=failed"); + assert!(reply.cookie(cookies::PENDING_COOKIE).is_none()); + assert_eq!(discord.exchanges(), 0); +} + +// --------------------------------------------------------------------------- +// The control plane, after membership passed — every failure leaves the key +// --------------------------------------------------------------------------- + +/// A control plane that is down after a passed check is `failed` — not +/// `unknown` — and the old key is untouched. +#[tokio::test] +async fn a_control_plane_failure_lands_on_failed_with_the_old_key_intact() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let old = seed_attached(&gateway, 1_000); + gateway.with(|s| s.fail_list = true); + + let reply = rework_round_trip(&rework_app(&discord, &gateway)).await; + assert_eq!(reply.location(), "/api-tokens/?rework=failed"); + assert_eq!(discord.member_calls(), 1, "membership ran and passed"); + assert_eq!(gateway.with(|s| s.keys[0].id.clone()), old); + assert_eq!(gateway.with(|s| s.ops.len()), 0); +} + +/// A delete of the old key that fails rolls the replacement back: the +/// visitor is told it failed, and what they hold is exactly the key they +/// had — not two keys of which the page reveals the older. +#[tokio::test] +async fn a_failed_delete_of_the_old_key_rolls_the_replacement_back() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let old = seed_attached(&gateway, 1_000); + gateway.with(|s| s.fail_delete_of = vec![old.clone()]); + + let reply = rework_round_trip(&rework_app(&discord, &gateway)).await; + assert_eq!(reply.location(), "/api-tokens/?rework=failed"); + + let stored = gateway.with(|s| s.keys.clone()); + assert_eq!(stored.len(), 1, "exactly the key they had"); + assert_eq!(stored[0].id, old); + // The replacement was created, attached, and then deleted again. + let ops = gateway.with(|s| s.ops.clone()); + let new = ops[0].trim_start_matches("create:").to_string(); + assert_eq!( + ops, + vec![ + format!("create:{new}"), + format!("attach:{new}"), + format!("delete:{new}") + ] + ); + assert_eq!(reveal(&gateway, USER_ID).await.json()["key_id"], old); +} + +/// A replacement that vanishes before it can be attached is `failed` with +/// the old key untouched — the visitor never lost anything. +#[tokio::test] +async fn a_replacement_that_vanishes_before_the_attach_leaves_the_old_key() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let old = seed_attached(&gateway, 1_000); + gateway.with(|s| s.vanish_on_next_attach = true); + + let reply = rework_round_trip(&rework_app(&discord, &gateway)).await; + assert_eq!(reply.location(), "/api-tokens/?rework=failed"); + gateway.with(|s| { + assert_eq!(s.keys.len(), 1); + assert_eq!(s.keys[0].id, old); + assert!(s.plan_keys.contains(&(PLAN_ID.to_string(), old.clone()))); + }); +} + +/// A usage plan that does not exist is `failed`, and the old key stays. +#[tokio::test] +async fn a_missing_usage_plan_is_failed_without_deleting_the_old_key() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let old = seed_attached(&gateway, 1_000); + gateway.with(|s| s.attach_always_404 = true); + + let reply = rework_round_trip(&rework_app(&discord, &gateway)).await; + assert_eq!(reply.location(), "/api-tokens/?rework=failed"); + assert!(gateway.with(|s| s.keys.iter().any(|k| k.id == old))); + assert_eq!(gateway.with(|s| s.deleted.len()), 0); +} + +/// A control plane slower than the deadline is `failed` — an answer, not a +/// Lambda-killed invocation — and the old key stays. +#[tokio::test] +async fn a_control_plane_slower_than_the_deadline_lands_on_failed() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let old = seed_attached(&gateway, 1_000); + // The listing is held past the deadline the issue deps carry; the budget + // arithmetic caps the swap at what is left of `ISSUE_BUDGET` (12s), so + // hold for longer than that. + gateway.with(|s| s.list_delay_ms = 13_000); + + let reply = rework_round_trip(&rework_app(&discord, &gateway)).await; + assert_eq!(reply.location(), "/api-tokens/?rework=failed"); + assert_eq!(gateway.with(|s| s.keys[0].id.clone()), old); + assert_eq!(gateway.with(|s| s.ops.len()), 0); +} + +// --------------------------------------------------------------------------- +// The usage cache +// --------------------------------------------------------------------------- + +/// A successful rework evicts the usage route's cached answer: the numbers +/// it held describe a key that no longer exists, and the new key starts from +/// a clean counter. Driven through the real router so the callback and the +/// usage route share one cache. +#[tokio::test] +async fn a_successful_rework_evicts_the_cached_usage_of_the_old_key() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let old = seed_attached(&gateway, 1_000); + gateway.with(|s| { + s.usage.insert(old.clone(), vec![vec![42, 99_958]]); + }); + let app = rework_app(&discord, &gateway); + let session = session_cookie(USER_ID); + + let before = call_path(app.clone(), "GET", USAGE_PATH, Some(&session)).await; + assert_eq!(before.status, StatusCode::OK); + assert_eq!(before.json()["used"], 42); + assert_eq!(gateway.with(|s| s.usage_calls), 1); + + assert_eq!( + rework_round_trip(&app).await.location(), + "/api-tokens/?rework=ok" + ); + + // Well inside the 60s TTL, so only the eviction can explain a second + // control-plane read — and its answer is the NEW key's: nothing recorded. + let after = call_path(app, "GET", USAGE_PATH, Some(&session)).await; + assert_eq!(after.status, StatusCode::OK); + assert!( + after.json()["used"].is_null(), + "{}", + String::from_utf8_lossy(&after.body) + ); + assert_eq!(gateway.with(|s| s.usage_calls), 2); + let new = gateway.with(|s| s.keys[0].id.clone()); + assert_eq!( + gateway.with(|s| s.usage_queries.last().unwrap().0.clone()), + new + ); +} + +// --------------------------------------------------------------------------- +// Identity +// --------------------------------------------------------------------------- + +/// A session for somebody else does not survive the round-trip: the fresh +/// identity names the key that is replaced and the cookie now says so. +#[tokio::test] +async fn the_re_auth_identity_decides_whose_key_is_replaced() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let mine = seed_attached(&gateway, 1_000); + let theirs = gateway.with(|s| s.seed("discord-999999999999999999-key", 1_000)); + let app = rework_app(&discord, &gateway); + + let login = call_path( + app.clone(), + "GET", + "/api-tokens/api/auth/login?action=rework", + None, + ) + .await; + let pending = login.cookie(cookies::PENDING_COOKIE).unwrap(); + let query = login.location().split_once('?').unwrap().1.to_string(); + let state = form_urlencoded::parse(query.as_bytes()) + .find(|(k, _)| k == "state") + .unwrap() + .1 + .to_string(); + let other = session_cookie("999999999999999999"); + let reply = call_path( + app, + "GET", + &format!("/api-tokens/api/auth/callback?code=c&state={state}"), + Some(&format!("{}={pending}; {other}", cookies::PENDING_COOKIE)), + ) + .await; + + assert_eq!(reply.location(), "/api-tokens/?rework=ok"); + let cookie = reply.cookie(cookies::SESSION_COOKIE).unwrap(); + let session = prices_api::portal::auth::session::Session::decode( + SIGNING_KEY.as_bytes(), + &cookie, + state_token::now_secs(), + ) + .unwrap(); + assert_eq!(session.sub, USER_ID); + assert_eq!(gateway.with(|s| s.deleted.clone()), vec![mine]); + assert!( + gateway.with(|s| s.keys.iter().any(|k| k.id == theirs)), + "theirs is untouched" + ); +} + +/// Every pre-check answer is uncacheable. +#[tokio::test] +async fn every_pre_check_answer_carries_no_store() { + let discord = MockDiscord::start(GRANTED_SCOPE, None).await; + let gateway = MockGateway::start().await; + let app = rework_app(&discord, &gateway); + + let no_key = precheck(&app, Some(&session_cookie(USER_ID))).await; + assert_eq!(no_key.cache_control(), "no-store"); + let unauthenticated = precheck(&app, None).await; + assert_eq!(unauthenticated.cache_control(), "no-store"); + + seed_attached(&gateway, now_secs()); + let capped = precheck(&app, Some(&session_cookie(USER_ID))).await; + assert_eq!(capped.status, StatusCode::CONFLICT); + assert_eq!(capped.cache_control(), "no-store"); + assert!(capped.headers.get(header::SET_COOKIE).is_none()); +} diff --git a/web/portal/src/api/portal.ts b/web/portal/src/api/portal.ts index fe39d8f1..80b72e57 100644 --- a/web/portal/src/api/portal.ts +++ b/web/portal/src/api/portal.ts @@ -284,6 +284,115 @@ export const signInUrl = (): string => `${PORTAL_API}/auth/login`; */ export const issueUrl = (): string => `${PORTAL_API}/auth/login?action=issue`; +/** + * Where the "Replace my key" confirmation sends the visitor (task 0191). + * + * The same plain-navigation reasoning as {@link issueUrl}: a rework re-proves + * Discord membership (not account age — old enough once is old enough forever) + * against a fresh token, which only a top-level navigation can fetch. The + * callback swaps the key — the new one is created and attached BEFORE the old + * one is deleted, so the visitor is never keyless — and lands back here with + * `?rework=`, which `app.tsx` renders. The once-per-quota-period cap + * is decided there too; {@link checkRework} asks about it first so the modal + * can say "next eligible 1 September" before anyone types anything. + */ +export const reworkUrl = (): string => `${PORTAL_API}/auth/login?action=rework`; + +/** + * Perform a top-level navigation — the one the rework confirmation makes. + * + * A one-line seam rather than `window.location.assign` at the call site, + * because jsdom's `location` is unforgeable: neither `delete window.location` + * nor `defineProperty` can replace it, so a component that navigates directly + * cannot be tested for WHEN it navigates (once, only once armed, never on a + * second click). The tests mock this export and assert on the calls. + */ +export const navigateTo = (url: string): void => { + window.location.assign(url); +}; + +/** + * What `POST /api-tokens/api/key/rework` tells the modal (task 0191). + * + * `eligible: false` is the backend's `409 rework_capped`, with the + * `next_eligible_at` from the envelope's `details` — RFC 3339, the 1st of the + * next month 00:00 UTC under OUR period rule (not an AWS guarantee; see the + * backend's `portal/period.rs`). + */ +export type ReworkCheck = + | { eligible: true } + | { eligible: false; next_eligible_at: string }; + +/** + * `POST /api-tokens/api/key/rework` — may this key be replaced now? + * + * **Read-only on the backend**, despite the verb: it lists the caller's key and + * compares its creation date with the current period's start. Nothing is + * created, attached or deleted — that needs the `reworkUrl()` round-trip. The + * modal calls this when it opens, so a visitor inside the cap sees the date + * they can next rework instead of typing `delete-key` and crossing Discord for + * a refusal. + * + * A `404 no_key` is thrown rather than mapped: the control that opens the modal + * only renders beside a key, so reaching it keyless is a stale page, and the + * honest rendering is the failure with the backend's own message. + */ +export async function checkRework(): Promise { + const url = `${PORTAL_API}/key/rework`; + let response: Response; + try { + response = await fetch(url, { + method: 'POST', + headers: { accept: 'application/json' }, + signal: AbortSignal.timeout(KEY_TIMEOUT_MS), + }); + } catch (error) { + if (isTimeout(error)) { + throw new PortalApiError( + `${url} did not answer within ${KEY_TIMEOUT_MS / 1000}s`, + ); + } + throw new PortalApiError(`${url} could not be reached`); + } + if (response.status === 409) { + const envelope = (await readEnvelope(response)) as + | (ErrorEnvelope & { details?: { next_eligible_at?: unknown } }) + | null; + const nextEligibleAt = envelope?.details?.next_eligible_at; + if ( + envelope?.code === 'rework_capped' && + typeof nextEligibleAt === 'string' + ) { + return { eligible: false, next_eligible_at: nextEligibleAt }; + } + throw new PortalApiError(failureMessage(url, 409, envelope), 409); + } + if (!response.ok) { + throw new PortalApiError( + failureMessage(url, response.status, await readEnvelope(response)), + response.status, + ); + } + try { + const body = (await response.json()) as { eligible?: unknown }; + if (body.eligible !== true) { + // A `200` that does not say `eligible: true` is not a shape this client + // knows; refusing to arm the modal on it is the safe reading. + throw new PortalApiError( + `${url} answered 200 without eligible: true`, + 200, + ); + } + return { eligible: true }; + } catch (error) { + if (error instanceof PortalApiError) throw error; + throw new PortalApiError( + `${url} answered ${response.status}, not JSON`, + response.status, + ); + } +} + /** * `POST /api-tokens/api/auth/logout` — clear the session. * diff --git a/web/portal/src/app/app.spec.tsx b/web/portal/src/app/app.spec.tsx index fa3eef1f..2bdb0376 100644 --- a/web/portal/src/app/app.spec.tsx +++ b/web/portal/src/app/app.spec.tsx @@ -3,8 +3,20 @@ import { MemoryRouter, useLocation } from 'react-router-dom'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { ROUTER_BASENAME } from '../base-path'; +import { navigateTo } from '../api/portal'; import App from './app'; +/** + * The rework confirmation (task 0191) ends in a top-level navigation, and + * jsdom's `window.location` cannot be replaced — so the API module's + * `navigateTo` seam is the one export mocked here, and everything else in the + * module stays real (the `fetch` stubs below exercise it). + */ +vi.mock('../api/portal', async (importOriginal) => ({ + ...(await importOriginal()), + navigateTo: vi.fn(), +})); + /** * Records the router's current query string, so the one-shot landing-param * tests (task 0189, closing 0186's O10) can assert the URL was cleaned while @@ -71,6 +83,10 @@ const ME_URL = '/api-tokens/api/auth/me'; const LOGOUT_URL = '/api-tokens/api/auth/logout'; const USAGE_URL = '/api-tokens/api/usage'; const KEY_URL = '/api-tokens/api/key'; +/** The rework pre-check (task 0191). */ +const REWORK_URL = '/api-tokens/api/key/rework'; +/** Where the armed confirmation navigates (task 0191). */ +const REWORK_HREF = '/api-tokens/api/auth/login?action=rework'; /** Where both "get my API key" and every retry link point (task 0189). */ const ISSUE_HREF = '/api-tokens/api/auth/login?action=issue'; @@ -1631,3 +1647,328 @@ describe('usage against quota', () => { expect(document.body.textContent).not.toContain('answered 401'); }); }); + +describe('replace my key', () => { + beforeEach(() => { + vi.restoreAllMocks(); + vi.mocked(navigateTo).mockReset(); + }); + afterEach(() => { + vi.unstubAllGlobals(); + }); + + const KEY = { + key_id: 'abc123', + name: 'discord-308994132968210433-key', + value: 'aBcDeF0123456789aBcDeF0123456789aBcDeF01', + }; + + /** The pre-check's two answers, as the backend shapes them. */ + const reworkAllowed = () => ({ json: async () => ({ eligible: true }) }); + const reworkCapped = () => ({ + ok: false, + status: 409, + json: async () => ({ + code: 'rework_capped', + message: 'your key was already issued or replaced this quota period', + details: { next_eligible_at: '2026-09-01T00:00:00Z' }, + }), + }); + + const signedInWithKey = ( + rework: () => Partial & { json?: () => unknown } = reworkAllowed, + key: () => Partial & { json?: () => unknown } = () => ({ + json: async () => KEY, + }), + ) => + stubRoutes({ + [CONFIG_URL]: openConfig, + [ME_URL]: () => ({ + json: async () => ({ + authenticated: true, + user_id: '308994132968210433', + username: 'adam', + }), + }), + [KEY_URL]: key, + [USAGE_URL]: usageNoKey, + [REWORK_URL]: rework, + }); + + const renderApp = (entry = '/') => + render( + + + + , + ); + + const openDialog = async () => { + fireEvent.click(await screen.findByTestId('replace-key-open')); + return screen.findByTestId('replace-key-dialog'); + }; + + /** + * The control lives beside the key and nowhere else — there is nothing to + * replace without one — and pressing it opens a confirmation rather than + * navigating: the round-trip is reachable only from the armed confirm. + */ + it('offers replacement only beside an existing key, and opens a dialog rather than navigating', async () => { + const fetchMock = signedInWithKey(); + renderApp(); + + await openDialog(); + expect(navigateTo).not.toHaveBeenCalled(); + // The pre-check is a POST to the rework route, relative and key-less. + const call = fetchMock.mock.calls.find(([url]) => url === REWORK_URL) as [ + string, + RequestInit, + ]; + expect(call).toBeTruthy(); + expect(call[0].startsWith('http')).toBe(false); + expect(call[1]?.method).toBe('POST'); + }); + + it('does not offer replacement to a visitor with no key', async () => { + signedInWithKey(reworkAllowed, keyNoKey); + renderApp(); + + await screen.findByRole('link', { name: /get my api key/i }); + expect(screen.queryByTestId('replace-key-open')).toBeNull(); + }); + + /** + * The wording is the acceptance criterion: the old key dies immediately, + * and the visitor is told so before they can confirm anything. + */ + it('states that the current key is deleted and stops working immediately', async () => { + signedInWithKey(); + renderApp(); + + await openDialog(); + const warning = await screen.findByTestId('replace-key-warning'); + expect(warning.textContent).toMatch(/deletes the current one/i); + expect(warning.textContent).toMatch(/stops working immediately/i); + expect(warning.textContent).toMatch(/will break/i); + }); + + /** + * Confirm is disabled until `delete-key` is typed — exactly that phrase — + * and the round-trip is not started by anything short of the armed press. + */ + it('keeps confirm disabled until the visitor types delete-key', async () => { + signedInWithKey(); + renderApp(); + + await openDialog(); + const confirm = (await screen.findByTestId( + 'replace-key-confirm', + )) as HTMLButtonElement; + const phrase = screen.getByTestId('replace-key-phrase') as HTMLInputElement; + expect(confirm.disabled).toBe(true); + + // Near misses do not arm it. + for (const typed of ['delete', 'delete key', 'DELETE-KEY', 'delete-key ']) { + fireEvent.change(phrase, { target: { value: typed } }); + expect(confirm.disabled, typed).toBe(true); + fireEvent.click(confirm); + } + expect(navigateTo).not.toHaveBeenCalled(); + + fireEvent.change(phrase, { target: { value: 'delete-key' } }); + expect(confirm.disabled).toBe(false); + }); + + /** + * One armed press is one navigation into the rework round-trip; the + * button is disabled again on submit, so a double-click cannot fire two + * reworks in the window before the page unloads. + */ + it('navigates into the rework round-trip once and cannot double-fire', async () => { + signedInWithKey(); + renderApp(); + + await openDialog(); + const confirm = (await screen.findByTestId( + 'replace-key-confirm', + )) as HTMLButtonElement; + fireEvent.change(screen.getByTestId('replace-key-phrase'), { + target: { value: 'delete-key' }, + }); + + fireEvent.click(confirm); + fireEvent.click(confirm); + fireEvent.click(confirm); + + expect(navigateTo).toHaveBeenCalledTimes(1); + expect(navigateTo).toHaveBeenCalledWith(REWORK_HREF); + expect(confirm.disabled).toBe(true); + expect(confirm.textContent).toMatch(/replacing/i); + // And the phrase cannot be edited back into an armed state mid-submit. + expect( + (screen.getByTestId('replace-key-phrase') as HTMLInputElement).disabled, + ).toBe(true); + }); + + /** + * Inside the cap there is nothing to confirm: the dialog renders the next + * eligible date — "1 September 2026", not a generic error — and never arms. + * The 3-August worked example, on the page. + */ + it('renders the next eligible date instead of a confirm when the cap is in the way', async () => { + signedInWithKey(reworkCapped); + renderApp(); + + await openDialog(); + const capped = await screen.findByTestId('replace-key-capped'); + expect(capped.textContent).toMatch(/1 September 2026/); + expect(capped.textContent).not.toMatch(/2026-09-01T/); + expect(screen.queryByTestId('replace-key-confirm')).toBeNull(); + expect(screen.queryByTestId('replace-key-phrase')).toBeNull(); + expect(navigateTo).not.toHaveBeenCalled(); + }); + + /** A pre-check that fails is a stated failure with the backend's reason. */ + it("reports a failed pre-check with the backend's reason", async () => { + signedInWithKey(() => ({ + ok: false, + status: 502, + json: async () => ({ + code: 'key_unavailable', + message: 'could not reach the API key service; try again', + }), + })); + renderApp(); + + await openDialog(); + expect( + await screen.findByText(/could not reach the API key service/i), + ).toBeTruthy(); + expect(screen.queryByTestId('replace-key-confirm')).toBeNull(); + }); + + it('closes without navigating', async () => { + signedInWithKey(); + renderApp(); + + await openDialog(); + await screen.findByTestId('replace-key-confirm'); + fireEvent.click(screen.getByRole('button', { name: /cancel/i })); + expect(screen.queryByTestId('replace-key-dialog')).toBeNull(); + expect(screen.getByTestId('replace-key-open')).toBeTruthy(); + expect(navigateTo).not.toHaveBeenCalled(); + }); + + // ------------------------------------------------------------------------- + // The landing states + // ------------------------------------------------------------------------- + + /** A completed rework says so, names the old key as dead, shows the key. */ + it('welcomes a completed rework and says the old key has stopped working', async () => { + signedInWithKey(); + renderApp('/?rework=ok'); + + const landed = await screen.findByTestId('rework-ok'); + expect(landed.textContent).toMatch(/old key has stopped working/i); + expect(await screen.findByTestId('api-key')).toBeTruthy(); + expect(screen.queryByTestId('issue-ok')).toBeNull(); + }); + + /** + * `?rework=ok` is proof a key exists even while the listing catches up — + * a "no key" answer right after a rework renders as settling, not as an + * offer to issue a second key, and the usage section does not say "issue + * one above" either. + */ + it('renders a settling wait, not "no key", when the rework redirect beats the listing', async () => { + signedInWithKey(reworkAllowed, keyNoKey); + renderApp('/?rework=ok'); + + const settling = await screen.findByTestId('issue-ok-settling'); + expect(settling.textContent).toMatch(/replaced/i); + expect(screen.queryByRole('link', { name: /get my api key/i })).toBeNull(); + expect(document.body.textContent).not.toMatch(/issue one above/i); + }); + + /** The refusal renders the date, from the URL, sanitised. */ + it('renders a capped landing with the next eligible date', async () => { + signedInWithKey(); + renderApp('/?rework=capped&next_eligible_at=2026-09-01'); + + const capped = await screen.findByTestId('rework-capped'); + expect(capped.textContent).toMatch(/1 September 2026/); + expect(capped.textContent).toMatch(/not replaced/i); + }); + + it('sanitises a nonsense next_eligible_at instead of rendering it', async () => { + signedInWithKey(); + renderApp('/?rework=capped&next_eligible_at=