diff --git a/CHANGELOG.md b/CHANGELOG.md index dbe4974..43923dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,30 @@ version bump. ## [Unreleased] +### Changed + +- **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. + + **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 `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`. A `DELETE` that needs a + body is a `DELETE` a generated client cannot call. + + Paired with nest-auth's `POST {prefix}/sessions/revoke-all`, so one generated client drives + both servers instead of 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 so carries none of the defect. + + **Apply:** change the method to `POST` and the path's last segment to `revoke-all`. A caller + still on `DELETE /auth/sessions/all` now falls through to the `{id}` capture and gets + `404 auth.session_not_found`, the same answer a malformed session hash has always produced. + ### Fixed - **The error-catalog parity test now reads the shared contract's status table instead of one diff --git a/README.md b/README.md index d6f8453..5dbdc3a 100644 --- a/README.md +++ b/README.md @@ -776,7 +776,7 @@ Route groups mount only when their feature **and** runtime toggle are enabled, s | POST | `/auth/mfa/disable` | `AuthUser` | Disable MFA | | POST | `/auth/mfa/recovery-codes` | `AuthUser` | Regenerate recovery codes (TOTP-gated) | | GET | `/auth/sessions` | `AuthUser`, `UserStatus` | List active sessions | -| DELETE | `/auth/sessions/all` | `AuthUser`, `UserStatus` | Revoke all sessions | +| POST | `/auth/sessions/revoke-all` | `AuthUser`, `UserStatus` | Revoke every session except the caller's own | | DELETE | `/auth/sessions/:id` | `AuthUser`, `UserStatus` | Revoke a specific session (ownership-checked) | | POST | `/auth/invitations` | `AuthUser` | Create a tenant invitation | | POST | `/auth/invitations/accept` | Public | Accept an invitation and create the user | diff --git a/crates/bymax-auth-axum/src/rate_limit.rs b/crates/bymax-auth-axum/src/rate_limit.rs index d0043d7..be4542c 100644 --- a/crates/bymax-auth-axum/src/rate_limit.rs +++ b/crates/bymax-auth-axum/src/rate_limit.rs @@ -208,7 +208,7 @@ pub struct RateLimitConfig { pub list_sessions: Option, /// `DELETE /auth/sessions/{id}` — 10 / 60s. pub revoke_session: Option, - /// `DELETE /auth/sessions/all` — 5 / 60s. + /// `POST /auth/sessions/revoke-all` — 5 / 60s. pub revoke_all_sessions: Option, /// `GET /auth/oauth/{provider}` — 10 / 60s. pub oauth_initiate: Option, diff --git a/crates/bymax-auth-axum/src/routes/sessions.rs b/crates/bymax-auth-axum/src/routes/sessions.rs index cf8dd40..e107917 100644 --- a/crates/bymax-auth-axum/src/routes/sessions.rs +++ b/crates/bymax-auth-axum/src/routes/sessions.rs @@ -1,13 +1,20 @@ //! The `sessions` route group (§8.2.4), gated behind the `sessions` feature: list / revoke-all //! / revoke-by-id. All three require [`AuthUser`] + [`UserStatus`]. Axum 0.8 uses brace path -//! syntax (`/{id}`); the static `all` segment wins over the `{id}` capture (static beats +//! syntax (`/{id}`); the static `revoke-all` segment wins over the `{id}` capture (static beats //! capture in axum 0.8, regardless of declaration order). +//! +//! Bulk revocation is a `POST`, not a `DELETE`, and the verb is load-bearing. 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 +//! `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 +//! `session_not_found`. Held identical to nest-auth's `POST {prefix}/sessions/revoke-all`. use axum::Json; use axum::Router; use axum::extract::{Path, State}; use axum::response::{IntoResponse, Response}; -use axum::routing::{delete, get}; +use axum::routing::{delete, get, post}; use bymax_auth_core::services::session::SessionInfo; use http::StatusCode; use serde_json::{Value, json}; @@ -29,8 +36,8 @@ pub(crate) fn routes(config: &AxumAuthConfig, ip_source: ClientIpSource) -> Rout crate::router::throttled(get(list), limits.list_sessions, ip_source), ) .route( - "/sessions/all", - crate::router::throttled(delete(revoke_all), limits.revoke_all_sessions, ip_source), + "/sessions/revoke-all", + crate::router::throttled(post(revoke_all), limits.revoke_all_sessions, ip_source), ) .route( "/sessions/{id}", @@ -60,7 +67,7 @@ async fn list( } } -/// `DELETE /auth/sessions/all` (204). Requires [`AuthUser`] + [`UserStatus`]. Revokes every +/// `POST /auth/sessions/revoke-all` (204). Requires [`AuthUser`] + [`UserStatus`]. Revokes every /// session except the caller's current one. async fn revoke_all( State(state): State, diff --git a/crates/bymax-auth-axum/tests/adapter.rs b/crates/bymax-auth-axum/tests/adapter.rs index a3a0d67..5d6b9bd 100644 --- a/crates/bymax-auth-axum/tests/adapter.rs +++ b/crates/bymax-auth-axum/tests/adapter.rs @@ -250,15 +250,28 @@ async fn revoke_all_sessions_works_in_bearer_mode_and_never_reports_a_silent_suc ); // No body-supplied token: the route cannot tell which session to keep, and says so. - let blind = Req::delete("/auth/sessions/all") + let blind = Req::post("/auth/sessions/revoke-all") .bearer(&access) .send(&app) .await; assert_eq!(blind.status, StatusCode::NOT_FOUND); assert_eq!(blind.json()["error"]["code"], "auth.session_not_found"); + // The old verb and path are GONE, not merely discouraged. `DELETE /auth/sessions/all` now + // falls through to the `{id}` capture, where `all` is not a session hash — so a caller that + // was never updated is refused rather than quietly revoking nothing under a `204`. Asserted + // because the removal is the breaking half of the change: a route that answered the old + // shape would make the move invisible to exactly the callers it is meant to reach. + let old_shape = Req::delete("/auth/sessions/all") + .bearer(&access) + .json(serde_json::json!({ "refreshToken": refresh })) + .send(&app) + .await; + assert_eq!(old_shape.status, StatusCode::NOT_FOUND); + assert_eq!(old_shape.json()["error"]["code"], "auth.session_not_found"); + // With the token in the body — the channel a bearer client actually has — it works. - let revoked = Req::delete("/auth/sessions/all") + let revoked = Req::post("/auth/sessions/revoke-all") .bearer(&access) .json(serde_json::json!({ "refreshToken": refresh })) .send(&app) @@ -961,8 +974,11 @@ async fn sessions_list_revoke_one_and_revoke_all() { assert_eq!(sessions[0]["isCurrent"], true); let hash = sessions[0]["sessionHash"].as_str().unwrap_or("").to_owned(); - // The static `all` segment wins over the `{id}` capture. - let revoke_all = Req::delete("/auth/sessions/all") + // Cookie mode: the jar carries the refresh token, so the caller's own session is + // identifiable without a body. (Path precedence against the `{id}` capture is not what this + // exercises — `{id}` is registered for DELETE only, so a POST could never reach it; the + // bearer-mode test covers that by sending the old DELETE shape.) + let revoke_all = Req::post("/auth/sessions/revoke-all") .cookie("access_token", &access) .cookie("refresh_token", &refresh) .send(&app) @@ -3002,7 +3018,7 @@ async fn sessions_list_and_revoke_all_store_failure_arms() { assert_eq!(list.status, StatusCode::INTERNAL_SERVER_ERROR); // `revoke_all` fails in the store → the revoke-all handler error arm renders a 500. - let revoke = Req::delete("/auth/sessions/all") + let revoke = Req::post("/auth/sessions/revoke-all") .cookie("access_token", &access) .cookie("refresh_token", &refresh) .send(&app) diff --git a/crates/bymax-auth-core/src/services/adapter_api.rs b/crates/bymax-auth-core/src/services/adapter_api.rs index 13964e3..b74ed20 100644 --- a/crates/bymax-auth-core/src/services/adapter_api.rs +++ b/crates/bymax-auth-core/src/services/adapter_api.rs @@ -208,14 +208,18 @@ impl AuthEngine { Ok(()) } - /// Revoke every session for the caller except the current one (`DELETE /auth/sessions/all`). - /// The current session is identified by the request's raw refresh token; when none is - /// present the caller's session cannot be excluded, so this is a no-op rather than wiping - /// the live session out from under the request. + /// Revoke every session for the caller except the current one + /// (`POST /auth/sessions/revoke-all`). + /// + /// The current session is identified by the request's raw refresh token. Without it there is + /// no way to tell which session to keep, and the call is **refused** rather than treated as + /// a no-op: answering success having done nothing is the failure mode this route exists to + /// avoid, since the caller reaching for it believes a device is compromised right now. /// /// # Errors /// - /// Returns a store [`AuthError`] on an infrastructure failure. + /// Returns [`AuthError::SessionNotFound`] when no usable refresh token names the caller's + /// current session, or a store [`AuthError`] on an infrastructure failure. pub async fn revoke_other_user_sessions( &self, user_id: &str, diff --git a/crates/bymax-auth-types/src/constants.rs b/crates/bymax-auth-types/src/constants.rs index 70128b7..4975861 100644 --- a/crates/bymax-auth-types/src/constants.rs +++ b/crates/bymax-auth-types/src/constants.rs @@ -97,8 +97,11 @@ pub mod routes { // SessionController — `sessions` feature /// `GET` — list the caller's sessions. pub const SESSIONS_LIST: &str = "/auth/sessions"; - /// `DELETE` — revoke every session. - pub const SESSIONS_REVOKE_ALL: &str = "/auth/sessions/all"; + /// `POST` — revoke every session **except the caller's own**, which is why the request + /// carries the caller's refresh token: it names the one session to keep. The constant is + /// named for the path, not for a full sign-out — for that, the account's own sessions are + /// swept by the flows that change a credential (password reset, MFA disable). + pub const SESSIONS_REVOKE_ALL: &str = "/auth/sessions/revoke-all"; /// `DELETE` — revoke one session by its hash (`/auth/sessions/{id}`). pub const SESSIONS_REVOKE_ONE: &str = "/auth/sessions/{id}"; diff --git a/docs/technical_specification.md b/docs/technical_specification.md index 13f2898..ab81a57 100644 --- a/docs/technical_specification.md +++ b/docs/technical_specification.md @@ -2631,13 +2631,16 @@ their ordering reproduces the NestJS guard pipeline. | Method | Path | Handler | Extractors / guards | Success | Body DTO | Feature | | ------ | ----------------------- | -------------------- | ------------------------------ | ------- | -------- | -------- | | GET | `/auth/sessions` | `list_sessions` | `AuthUser`, `UserStatus` | 200 | — | sessions | -| DELETE | `/auth/sessions/all` | `revoke_all_sessions`| `AuthUser`, `UserStatus` | 204 | — | sessions | +| POST | `/auth/sessions/revoke-all` | `revoke_all_sessions`| `AuthUser`, `UserStatus` | 204 | `RefreshDto` (bearer mode)| sessions | | DELETE | `/auth/sessions/{id}` | `revoke_session` | `AuthUser`, `UserStatus` | 204 | — | sessions | > `{id}` is the full 64-char SHA-256 session hash from `GET /auth/sessions`. > Axum 0.8 path syntax uses braces (`/{id}`), not the colon form. The static -> `all` segment is registered and matched ahead of the `{id}` capture; in Axum -> 0.8 static segments win over captures, so declaration order is irrelevant. +> `revoke-all` segment is registered and matched ahead of the `{id}` capture; +> in Axum 0.8 static segments win over captures, so declaration order is +> irrelevant. There is **no** static `all` segment: the retired +> `DELETE /auth/sessions/all` reaches the `{id}` capture as `id = "all"`, +> which is not a session hash, so it answers `404 auth.session_not_found`. #### 8.2.5 PlatformAuthController — group `platform` (feature `platform`) @@ -5073,7 +5076,7 @@ brute-force headroom per IP. | `POST /auth/email/change/confirm` | `email_change_confirm` | 5 | 60 | Bounds guessing at the address-change token. | | `GET /auth/sessions` | `list_sessions` | 30 | 60 | Generous read limit. | | `DELETE /auth/sessions/{id}` | `revoke_session` | 10 | 60 | Bound single-session revocation. | -| `DELETE /auth/sessions/all` | `revoke_all_sessions` | 5 | 60 | Bound bulk revocation. | +| `POST /auth/sessions/revoke-all` | `revoke_all_sessions` | 5 | 60 | Bound bulk revocation. | | `GET /auth/oauth/{provider}` | `oauth_initiate` | 10 | 60 | Initiate + callback = 2 reqs/login; cap of 10 keeps effective logins ≤5. | | `GET /auth/oauth/{provider}/callback` | `oauth_callback` | 10 | 60 | Matches `oauth_initiate`; prevents callback-only flooding. | diff --git a/packages/rust-auth/src/shared/routes.ts b/packages/rust-auth/src/shared/routes.ts index e2eefcd..0393651 100644 --- a/packages/rust-auth/src/shared/routes.ts +++ b/packages/rust-auth/src/shared/routes.ts @@ -21,7 +21,7 @@ export const AUTH_ROUTES = { PASSWORD_VERIFY_OTP: "/auth/password/verify-otp", PASSWORD_RESEND_OTP: "/auth/password/resend-otp", SESSIONS_LIST: "/auth/sessions", - SESSIONS_REVOKE_ALL: "/auth/sessions/all", + SESSIONS_REVOKE_ALL: "/auth/sessions/revoke-all", SESSIONS_REVOKE_ONE: "/auth/sessions/{id}", PLATFORM_LOGIN: "/auth/platform/login", PLATFORM_MFA_CHALLENGE: "/auth/platform/mfa/challenge",