feat(0191): replace my key — revoke now, re-issue next quota period - #238
Conversation
…nd-trip "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.
A dashboard load fires /key and /usage in parallel, and on a local serve run against an SSO profile the two raced the credential chain's lazy first initialisation: the loser got 'profile file credentials provider initialization error already taken' and every call answered 502. One eager provide_credentials() at cold start serialises it; in the Lambda the chain reads the environment and this is a no-op.
… next period Reversed by Adam after seeing the swap live. "Replace my key" no longer issues a new key: it disables the current one immediately (UpdateApiKey enabled=false, session-only, no Discord round-trip) and a new key can be issued only from the start of the next quota period. The disabled key is the revocation record — its lastUpdatedDate is what the re-issue cap reads — so there is still no registry. The cap moved to the issue path: a revoked user's "Get my API key" passes eligibility and lands on ?issue=capped&next_eligible_at=…; once the period has rolled the same press deletes the record and creates the new key. The reveal answers 404 key_revoked with the date and never hands the dead value out again. The action=rework OAuth round-trip, its eight landings and the swap are removed; one IAM grant is added (apigateway:PATCH on /apikeys/*) and explained at the statement. The 2026-08-07 swap decision is struck through in the epic doc with the reversal dated, not deleted. 0192 (revoke) is absorbed. BREAKING CHANGE: POST /api-tokens/api/key/rework now revokes the key instead of pre-checking a swap; GET /key can answer 404 key_revoked. Tests: 19 HTTP in portal_rework.rs (rewritten), cap/naming units, 12 frontend; workspace 640 Rust / 88 portal, 0 failed.
adamkoot
changed the title
Two rounds on top of the revoke-now/re-issue-next-period slice: the 2026-08-21 four-lens audit, and the code review of that audit's own diff. They share files, so they land as one commit. From the audit: - One selector and one cap instant for four readers. The reveal capped on the earliest record's date while the issue capped on the latest, so two revocation records from different months made the page offer an issue the round-trip refused. Usage ignored `enabled` entirely. - The re-listing after a post-roll create excludes what it just deleted and anything disabled: `GetApiKeys` is eventually consistent, and a phantom earlier-created record would win, 404 on attach, and spend the retry. - A create starts only with 4s of the deadline left, and `CreateApiKey` is sent without SDK retries — no idempotency token, so a retried request whose first try landed is a duplicate. - The revoke requires the portal's own request marker and refuses cross-site `Sec-Fetch-Site`. `SameSite=Lax` is site-scoped; after the custom-domain cutover a sibling host's form POST would have cost the victim their key for a month. - IAM `/apikeys/*` is tag-scoped on GET/PATCH/DELETE. The per-key GET with `includeValue=true` was the account-wide exposure, not the listing. From the review of it: - An undated record no longer poisons the cap instant. `None` is capped, and a capped answer recomputes `next_eligible_at` from the current period on every read — so one undated duplicate rolled the date forward every month and locked its owner out for good. - The revoke decides `next_eligible_at` through `cap::decide` like the other four readers, so an idempotent press after the period rolled answers "now" instead of naming a month that is not owed. - `revoked_at` is nullable rather than an invented epoch, and the page renders a revocation without an instant instead of "1 January 1970", "just now", or the next-eligible phrase in the instant's place. - The propagation window is present-tense only while it is open: the revoked view renders on every page load, days after the fact. - `RECONCILE_FLOOR` below `CREATE_FLOOR` is recorded as deliberate, with a const assertion — an adoption still fits in the gap, a create does not. - README §3c drops "immediately", the claim the dialog had already lost. 648 Rust, 93 portal, 0 failed.
`a8693ec` on develop landed with `*emphasis*` where prettier writes `_emphasis_`, so `nx format:check --all` fails on every PR that merges develop. Four lines, no content change.
Step 0's DAY-period rollover measurement is dropped deliberately rather than attempted a third time, and the scratch stack it needed is gone. Two runs died silently: 2026-08-13 (three samples, dead two minutes in, while the archived note said "running" for a week) and 2026-08-21 (183 samples, dead ~9 h before the UTC midnight it existed to observe). Nothing was ever measured. The economics inverted. The proxy existed to avoid waiting for the real MONTH rollover, 19 days away when it was designed and 8 days away now, and a third run needs the script's defects fixed plus credentials that outlive a 26 h window -- to produce what the task itself calls evidence, not proof. What replaces it is better: the 1 September rollover read off production from the GetUsage reset warn in keys/gateway.rs. Nothing in the build waits on the answer; the cap is our rule, defined once in portal/period.rs, and a different AWS instant moves a dashboard label. The criterion this served -- stop presenting the boundary as AWS-documented -- was already met by the wording work on 2026-08-21, so the AC closes on that and says plainly that the measurement was dropped. Scratch key, usage plan and REST API deleted 07:45Z; the account greps clean of lore0180* and the production plan is untouched. The script's own teardown reported success while leaving the plan alive -- it deletes the plan before the REST API whose stage it references, and || true swallows the refusal. That defect and three others (a poll loop that dies silently under pipefail, a 26 h window outliving an SSO session, a log appended across runs that can make analyse span two of them) are now recorded at the top of the script, which is kept as a harness. Dead poll logs moved to .trash/. One AC left: the 1 September check.
Code review —
|
Seven findings from the PR #238 review, verified against the branch first — five held as written, two needed correcting, and two more turned up while checking them. The tag condition goes on `PATCH` alone, in its own statement; `GET` and `DELETE` go back to 0187's unconditioned grant, because narrowing those two changes shipped behaviour and 0187's comment forbade it in as many words. Narrowing them stays task 0194's, with the audit in hand. The post-roll cleanup logs and steps over a failed delete, like the loser sweep does — that branch runs on every press once the name holds nothing but revoked keys, so one undeletable record was `?issue=failed` forever. A revocation is now `Done`, `Partial` or an error. A partial one reaches the page as `partial: true` and renders "a duplicate may still work" rather than a plain "revoked"; the `502` copy no longer claims the key "is still active", which is a state the page cannot know. Every disable racing away is `NoKey`, not a dated record no issue press will honour. The revocation instant now comes off the `UpdateApiKey` response instead of this process's clock, so the answer and `cap::decide` cannot fall either side of a period boundary on the 1st. Also: `revocation_instant`'s rustdoc gets its own body back, and the dead `rework_round_trip` helper is deleted.
The revoked view still rendered "you do not have a working key" and the next-eligible date for a `Partial`. Both are false: the duplicate that refused to be disabled is a working key, and the issue path adopts it rather than refusing — `a_partial_revocation_is_reported_as_partial` asserts `?issue=ok` for exactly that state. The warning above them already carries the only instruction that applies. Also asserts the clean answer omits the `partial` flag rather than sending it as `false`, so a client that never learned about it reads the shape it always did.
The revoke model shipped in PR #238 (approved, CI green): deactivate every key under the caller's name, issue nothing, and refuse the re-issue until the next quota period. 0192 is already archived as superseded — its rule and measurements live here. The one criterion that could not be met is the MONTH rollover confirmation: the next real boundary is 1 September 2026, after this task closes. Spawned as 0221 rather than left as prose, and the criterion is marked deferred, not done. Nothing in the build waits on it — the cap is our rule, defined once in portal/period.rs.
2 participants
Summary
POST /key/reworkdisables the caller's key immediately (UpdateApiKey enabled=false, session-only, no Discord round-trip) and issues nothing. A new key can be issued only from the start of the next quota period — the 1st of next month, 00:00 UTC, our period rule (portal/period.rs, shared with the usage panel).lastUpdatedDatedecides), so "Get my API key" inside that period passes eligibility and lands on?issue=capped&next_eligible_at=…with nothing written; once the period rolls, the same press deletes the record and creates the new key. The reveal answers404 key_revokedwith the date and never hands the dead value out again. No registry needed (0190 stays cancelled); 0192 (revoke) is absorbed.delete-key, sends onePOST, disables on submit; the revoked state renders the date and defers the issue link until it has passed.apigateway:PATCHon/apikeys/*(PortalReadDisableAndDeleteOwnApiKeys), reasoning at the statement. NoUpdateUsagePlan/UpdateUsage. Also: AWS credentials resolved once atGatewayconstruction (a local-run race on the SSO profile provider answered every call with 502).DAY-proxy poller was re-started 2026-08-21 11:51Z and its verdict goes into the task's Step 0 table. The boundary is no longer presented as AWS-documented anywhere (0157, epic doc, runbooks); theMONTHconfirmation is dated 1 September 2026. The epic's 2026-08-07 swap decision is struck through with the reversal dated, not deleted.portal_rework.rs, cap/naming/period units, 12 frontend — workspace 640 Rust / 88 portal, 0 failed.Update — audit round + review of it (2026-08-21)
Two further rounds on this branch, one commit (they share files).
Four-lens audit. One selector and one cap instant for four readers — the reveal capped on the earliest record's date while the issue capped on the latest, so two revocation records from different months made the page offer an issue the round-trip refused, and
/usageignoredenabledentirely. The re-listing after a post-roll create excludes what it just deleted and anything disabled (GetApiKeysis eventually consistent; a phantom record would win, 404 on attach, and spend the retry). A create starts only with 4s of the deadline left, andCreateApiKeyis sent without SDK retries — no idempotency token, so a retried request whose first try landed is a duplicate. The revoke requires the portal's own request marker and refuses cross-siteSec-Fetch-Site:SameSite=Laxis site-scoped, and after the custom-domain cutover (0195) a sibling host's formPOSTwould have cost the victim their key for a month. IAM/apikeys/*is tag-scoped onGET/PATCH/DELETE— the per-keyGETwithincludeValue=truewas the account-wide exposure, not the listing.Review of that diff. An undated record no longer poisons the cap instant:
Noneis capped, and a capped answer recomputesnext_eligible_atfrom the current period on every read, so one undated duplicate rolled the date forward every month and locked its owner out permanently. The revoke decidesnext_eligible_atthroughcap::decidelike the other four readers, so an idempotent press after the period rolled answers "now" rather than naming a month that is not owed.revoked_atis nullable instead of an invented epoch, and the page renders a revocation without an instant rather than "1 January 1970", "just now", or the next-eligible phrase in the instant's place. The propagation window is present-tense only while it is open — the revoked view renders on every page load, days after the fact.RECONCILE_FLOORbelowCREATE_FLOORis recorded as deliberate with aconstassertion (an adoption still fits in the gap; a create does not). README §3c drops "immediately", the claim the dialog had already lost.Tests: 648 Rust / 93 portal, 0 failed.
cargo fmt --check,clippy --all-targets(0 warnings),cargo check --features lambda,nx run-many -t lint typecheck build test -p portal,nx format:check --allgreen.Still open on the task, deliberately: the item-7
DAY-proxy verdict, theMONTHconfirmation dated 1 September 2026, and the audit's B6/B9/B10/B11/C/D/E items (control-plane call budget, the 0205 deploy, CI allow-list, dialog focus/Escape, doc corrections).