fix(sessions)!: move bulk revocation to POST /auth/sessions/revoke-all - #153
Open
msalvatti wants to merge 2 commits into
Open
fix(sessions)!: move bulk revocation to POST /auth/sessions/revoke-all#153msalvatti wants to merge 2 commits into
msalvatti wants to merge 2 commits into
Conversation
- `DELETE /auth/sessions/all` becomes `POST /auth/sessions/revoke-all`.
Guards, rate limit (5/60), `204` and error codes are unchanged; only the
method and the last path segment moved.
- The verb was the defect. The handler needs the refresh token naming the
caller's own session — the one it must NOT revoke — and a bearer-mode
deployment carries that token in the request body. RFC 7231 gives a
payload on `DELETE` no defined semantics, so an OpenAPI generator drops
it: the generated client sends no body, the handler cannot identify the
current session, and every call answers `auth.session_not_found`.
- Paired with nest-auth's `POST {prefix}/sessions/revoke-all`, so one
generated client drives both servers rather than needing a
per-implementation branch.
- `DELETE /auth/platform/sessions` is deliberately not moved: it revokes
every platform session including the caller's, reads no body at all, and
carries none of the defect.
- The old shape is gone, not deprecated, and a test pins it: `DELETE
/auth/sessions/all` falls through to the `{id}` capture, where `all` is
not a session hash, so it answers `404 auth.session_not_found` rather
than reporting success having revoked nothing.
- `SESSIONS_REVOKE_ALL` moves in `bymax-auth-types`, which regenerates the
npm `AUTH_ROUTES` entry; the route tables in the README and the
specification follow, and the specification's §8.2.4 row now names the
optional `RefreshDto` body it has always accepted in bearer mode.
There was a problem hiding this comment.
Pull request overview
Moves bulk session revocation to a POST endpoint so bearer clients can reliably send refresh tokens.
Changes:
- Registers
POST /auth/sessions/revoke-alland removes the old route. - Updates route constants, tests, rate limits, and documentation.
- Corrects revocation error documentation.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
README.md |
Updates the route table. |
CHANGELOG.md |
Documents the breaking change. |
docs/technical_specification.md |
Updates endpoint and rate-limit tables. |
packages/rust-auth/src/shared/routes.ts |
Regenerates the npm route constant. |
crates/bymax-auth-types/src/constants.rs |
Changes the canonical route constant. |
crates/bymax-auth-core/src/services/adapter_api.rs |
Corrects revocation rustdoc. |
crates/bymax-auth-axum/src/routes/sessions.rs |
Registers the new POST route. |
crates/bymax-auth-axum/src/rate_limit.rs |
Updates rate-limit documentation. |
crates/bymax-auth-axum/tests/adapter.rs |
Tests new and removed route shapes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- The specification's routing note under §8.2.4 still described a static
`all` segment winning over the `{id}` capture. That segment no longer
exists: the retired `DELETE /auth/sessions/all` reaches the capture as
`id = "all"`, which is not a session hash, so it answers
`404 auth.session_not_found`. The route table above it moved without the
prose two lines below.
- The README row and the `SESSIONS_REVOKE_ALL` rustdoc both said the
endpoint revokes every session. It revokes every session **except the
caller's own** — which is the reason the request carries the caller's
refresh token at all, since that token names the session to keep. A
consumer reading either one would expect a full sign-out and ship a
"sign out everywhere" button that leaves the current device signed in.
- The engine-side docs already said "except the current one"
(`revoke_other_user_sessions`, `revoke_all_except_current`, the route
handler); these two were the places where the qualifier had been dropped.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (1)
CHANGELOG.md:18
- The same
Unreleasedsection still contains a later bullet at lines 481–486 claiming thatDELETE /auth/sessions/allaccepts a refresh-token body. The final unreleased API no longer provides that endpoint, so the changelog now advertises incompatible contracts. Remove that stale bullet or rewrite it without naming the retired route.
- **BREAKING: bulk session revocation moved from `DELETE /auth/sessions/all` to
`POST /auth/sessions/revoke-all`.** Guards, rate limit (5/60), `204` and error codes are
unchanged; only the method and the last path segment moved. `AUTH_ROUTES.SESSIONS_REVOKE_ALL`
in the npm client carries the new path.
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
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.
Summary
DELETE /auth/sessions/allbecomesPOST /auth/sessions/revoke-all. Guards, rate limit (5/60),204and error codes are unchanged — only the method and the last path segment move.The verb was the defect. The handler needs the refresh token naming the caller's own session, the one session it must not revoke, and a bearer-mode deployment carries that token in the request body. RFC 7231 gives a payload on
DELETEno defined semantics, so an OpenAPI generator drops it: the generated client sends no body, the handler cannot identify the current session, and every call answersauth.session_not_found. ADELETEthat needs a body is aDELETEa generated client cannot call.This repository ships no OpenAPI document, so the broken client cannot originate here — the breakage reaches a consumer through a shared client, or one generated from nest-auth's document. nest-auth made the same move (
POST {prefix}/sessions/revoke-all); matching it means one generated client drives both servers instead of needing a per-implementation branch.Changes
bymax-auth-types—routes::SESSIONS_REVOKE_ALLmoves to/auth/sessions/revoke-all. This is the source of truth: the npmAUTH_ROUTESentry is regenerated from it, so the client constant follows automatically rather than being edited in parallel.bymax-auth-axum— the route is registered asposton the new path. The module doc records why the verb is load-bearing, so the next person to "clean up" a POST that looks like a DELETE finds the reason before the fix regresses.bymax-auth-core— the rustdoc onrevoke_other_user_sessionssaid the credential-less case "is a no-op". It is not: the function returnsAuthError::SessionNotFound, and the inline comment eight lines below says so explicitly. Corrected, and# Errorsnow lists that error.POST. A new assertion pins that the old shape is gone rather than tolerated:DELETE /auth/sessions/allfalls through to the{id}capture, whereallis not a session hash, so it answers404 auth.session_not_foundinstead of reporting success having revoked nothing.RefreshDtobody it has always accepted in bearer mode, matching how/auth/refreshis already documented; listing it as—was imprecise before and misleading once the body became the documented channel.How to verify
Against a bearer-mode deployment with
sessionsenabled:POST /auth/sessions/revoke-allwith{"refreshToken": "…"}→204, every other session revoked, epoch bumped.404 auth.session_not_found, unchanged: without the token there is no way to tell which session to keep, and answering204having revoked nothing is the failure this route exists to avoid.DELETE /auth/sessions/all→404 auth.session_not_found.Notes
Breaking. A caller on
DELETE /auth/sessions/allmust change the method toPOSTand the last path segment torevoke-all. There is no alias and no deprecation window — the library is unreleased, so nothing is in the field to migrate.DELETE /auth/platform/sessionsis deliberately not moved. It revokes every platform session including the caller's, takes no body at all, and therefore carries none of the defect. Moving it would be churn justified by a symmetry that does not exist.Independent of #152. That PR changes tenant refusal; this one changes a route. They share no code and can merge in either order.