test(e2e): PKCE-on-the-wire, per-user scoping, and desk clash coverage - #480
Merged
Conversation
Closes the last outstanding P0. login.spec.ts proves a login works and that the token is usable; this proves the handshake that produced it was safe, which is a different question. A client that leaked a secret or quietly dropped PKCE would still end up with a valid token and a working app, so nothing in the response tells you the exchange was sound. You have to look at what the browser sent. loginViaUI now records every /auth/* request it observes, and the new spec asserts S256 (never "plain"), a real base64url challenge, a code_verifier, the expected client_id, and no client_secret anywhere. Plus: the password never appears in a URL, and only ever reaches /auth/signin. Red-checked. These assertions previously lived in tasks/PPT-2536/, where nothing ran them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers WP-E2E-08 and AUTH-E2E-05. A second seeded user cannot see the booking in their listing, and cannot delete it; the owner still can. Includes a control asserting you can see your own booking, otherwise "nobody sees anything" would pass as success. This locks down something we learned the hard way: GET /bookings is scoped to the caller. An early leak check written as an admin reported zero bookings while the database plainly held one. It is both a privacy boundary and a trap for anyone writing tooling, and a regression would leak quietly rather than fail loudly. Red-checked: inverting the assertion shows the other user's listing really is empty while the booking exists. Also corrects two rows in the contract. Lockers and parking do NOT follow the desk metadata pattern as claimed - lockers come from banks then lockers within them, parking needs level zones tagged `parking` plus a separate spaces API. Both are more setup than desks, and the contract now says so rather than implying they are quick wins. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Maps to a shipped fix, "Fix rejecting overlapping bookings on desk assignment" (2607.1). Double-booking is the kind of regression that doesn't announce itself: nothing errors, nobody notices, and two people turn up to the same desk on Tuesday. Attempted as a SECOND user deliberately, because that's the real scenario and because a clash check that only consulted your own bookings would still pass a single-user version of this. Covers the identical slot and a partial overlap, which is the case a naive check misses. Two controls, so the test can't pass for the wrong reason: a genuinely non-overlapping slot must still be accepted (otherwise a backend that rejected everything would look correct), and the desk must free up once the booking is deleted (a cancelled booking that still blocks the desk is harder to diagnose than a plain double-booking). Red-checked: the API really does return 409. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rules out client disconnects as the trigger for the connection poisoning: aborting requests mid-flight strands nothing, while a burst with no aborts at all does. Kept next to the burst reproducer because a diagnostic that lives only in a ticket comment rots. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
test(e2e): PKCE-on-the-wire, per-user scoping, and desk clash coverage
|
Deployment failed with the following error: Learn More: https://vercel.com/placeos?upgradeToPro=build-rate-limit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-targets the work from #477 at
develop.#477 was stacked on
e2e/ci-verify, and when #476 merged that branch stopped flowing anywhere — so merging #477 landed its commits on a dead branch rather than ondevelop. Same commits, correct base. Nothing here is new relative to #477 beyonddevelopbeing merged in.What it adds
pkce.spec.tsplain), a real base64url challenge, acode_verifier, the expectedclient_id, noclient_secretanywhere, and that the password never appears in a URL.booking-scoping.spec.tsGET /bookingsis caller-scoped, so an early leak check written as an admin reported zero bookings while the database held one.desk-clash.spec.tsAlso carries the PPT-2642 client-abort diagnostic (
e2e/support/repro/reg09-client-abort.ts), which is what ruled out client disconnects as the cause of the connection-pool poisoning.All P0s in the coverage contract are done with this.
Verification
14/14 green on the self-hosted runner for this exact tree (advisory run on
e2e/coverage, head0d6694e), and 14/14 locally against a cold stack.🤖 Generated with Claude Code