Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
2 changes: 1 addition & 1 deletion crates/bymax-auth-axum/src/rate_limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ pub struct RateLimitConfig {
pub list_sessions: Option<RateLimit>,
/// `DELETE /auth/sessions/{id}` — 10 / 60s.
pub revoke_session: Option<RateLimit>,
/// `DELETE /auth/sessions/all` — 5 / 60s.
/// `POST /auth/sessions/revoke-all` — 5 / 60s.
pub revoke_all_sessions: Option<RateLimit>,
/// `GET /auth/oauth/{provider}` — 10 / 60s.
pub oauth_initiate: Option<RateLimit>,
Expand Down
17 changes: 12 additions & 5 deletions crates/bymax-auth-axum/src/routes/sessions.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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}",
Expand Down Expand Up @@ -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<AuthState>,
Expand Down
26 changes: 21 additions & 5 deletions crates/bymax-auth-axum/tests/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
14 changes: 9 additions & 5 deletions crates/bymax-auth-core/src/services/adapter_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 5 additions & 2 deletions crates/bymax-auth-types/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}";

Expand Down
11 changes: 7 additions & 4 deletions docs/technical_specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Comment thread
msalvatti marked this conversation as resolved.
| 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`)

Expand Down Expand Up @@ -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. |

Expand Down
2 changes: 1 addition & 1 deletion packages/rust-auth/src/shared/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading