L-10: E-16 answers not_found for another user's device (D-156)
A member naming the other user's device now gets the same 404 not_found body as an unknown id, so E-16 no longer reveals whose a device id is. The admin keeps revoking any device except the current one (AC-3.2, §8.4). contracts/openapi.yaml drops the 403 on E-16 and states the rule; web/src/api/schema.d.ts regenerated. The forbidden test becomes a not_found test, plus one asserting the body equals the unknown-id body and that nothing was revoked. Also bumps ctutils 0.4.3 -> 0.4.4 (lockfile only) for RUSTSEC-2026-0385, published 2026-10-23, which failed cargo audit on main. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TQd3pEP9UeN2uifbHknQ7B
This commit is contained in:
@@ -626,7 +626,7 @@ paths:
|
|||||||
delete:
|
delete:
|
||||||
operationId: revokeDevice
|
operationId: revokeDevice
|
||||||
summary: E-16 Revoke a device
|
summary: E-16 Revoke a device
|
||||||
description: Revoking an already revoked device is 204. Admin any device except the current one; member own devices except the current one (SPEC §8.4).
|
description: Revoking an already revoked device is 204. Admin any device except the current one; member own devices except the current one, and a device of the other user is 404 `not_found`, the same answer as an unknown id (SPEC §8.4, D-156).
|
||||||
parameters:
|
parameters:
|
||||||
- $ref: '#/components/parameters/DeviceId'
|
- $ref: '#/components/parameters/DeviceId'
|
||||||
- $ref: '#/components/parameters/Protocol'
|
- $ref: '#/components/parameters/Protocol'
|
||||||
@@ -641,9 +641,6 @@ paths:
|
|||||||
$ref: '#/components/headers/ServerVersion'
|
$ref: '#/components/headers/ServerVersion'
|
||||||
'401':
|
'401':
|
||||||
$ref: '#/components/responses/Unauthorized'
|
$ref: '#/components/responses/Unauthorized'
|
||||||
'403':
|
|
||||||
$ref: '#/components/responses/Forbidden'
|
|
||||||
description: '`forbidden`.'
|
|
||||||
'404':
|
'404':
|
||||||
$ref: '#/components/responses/NotFound'
|
$ref: '#/components/responses/NotFound'
|
||||||
description: '`not_found`.'
|
description: '`not_found`.'
|
||||||
|
|||||||
Generated
+2
-2
@@ -496,9 +496,9 @@ dependencies = [
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "ctutils"
|
name = "ctutils"
|
||||||
version = "0.4.3"
|
version = "0.4.4"
|
||||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||||
checksum = "03bb0e1cc970d482d121d9a1744999169b69a07470b3d644a7894e53fcaf4574"
|
checksum = "12218662ccac37d9714f2191252553d67585b97df8c839cbe49e0bde278dc865"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"cmov",
|
"cmov",
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -11,7 +11,7 @@ use crate::api::{ApiJson, IdPath, JsonBody, rows};
|
|||||||
use crate::auth::AuthDevice;
|
use crate::auth::AuthDevice;
|
||||||
use crate::db;
|
use crate::db;
|
||||||
use crate::error::{AppError, ErrorCode};
|
use crate::error::{AppError, ErrorCode};
|
||||||
use crate::openapi::responses::{BadRequest, Conflict, Forbidden, NotFound};
|
use crate::openapi::responses::{BadRequest, Conflict, NotFound};
|
||||||
use crate::wire::{DeviceList, PushTokenRequest, Role, UuidSchema};
|
use crate::wire::{DeviceList, PushTokenRequest, Role, UuidSchema};
|
||||||
|
|
||||||
/// Longest `fcm_token` stored, in characters after trimming (D-114; a Firebase Installation ID has 22).
|
/// Longest `fcm_token` stored, in characters after trimming (D-114; a Firebase Installation ID has 22).
|
||||||
@@ -41,13 +41,12 @@ pub async fn list_devices(
|
|||||||
path = "/devices/{device_id}",
|
path = "/devices/{device_id}",
|
||||||
operation_id = "revokeDevice",
|
operation_id = "revokeDevice",
|
||||||
summary = "E-16 Revoke a device",
|
summary = "E-16 Revoke a device",
|
||||||
description = "Revoking an already revoked device is 204. Admin any device except the current one; member own devices except the current one (SPEC §8.4).",
|
description = "Revoking an already revoked device is 204. Admin any device except the current one; member own devices except the current one, and a device of the other user is 404 `not_found`, the same answer as an unknown id (SPEC §8.4, D-156).",
|
||||||
params(
|
params(
|
||||||
("device_id" = UuidSchema, Path, description = "Not a UUID is 404 `not_found`."),
|
("device_id" = UuidSchema, Path, description = "Not a UUID is 404 `not_found`."),
|
||||||
),
|
),
|
||||||
responses(
|
responses(
|
||||||
(status = 204, description = "Revoked."),
|
(status = 204, description = "Revoked."),
|
||||||
(status = 403, response = Forbidden),
|
|
||||||
(status = 404, response = NotFound),
|
(status = 404, response = NotFound),
|
||||||
(status = 409, response = Conflict),
|
(status = 409, response = Conflict),
|
||||||
)
|
)
|
||||||
@@ -58,12 +57,12 @@ pub async fn revoke_device(
|
|||||||
IdPath(device_id): IdPath,
|
IdPath(device_id): IdPath,
|
||||||
) -> Result<StatusCode, AppError> {
|
) -> Result<StatusCode, AppError> {
|
||||||
let mut tx = db::begin_immediate(&state.db).await?;
|
let mut tx = db::begin_immediate(&state.db).await?;
|
||||||
|
// D-156 (security review L-10): for a member, a device of the other user is the same `not_found` as an unknown
|
||||||
|
// id, so the answer does not tell whether the id belongs to someone.
|
||||||
let target = rows::device(&mut tx, device_id, device.device_id)
|
let target = rows::device(&mut tx, device_id, device.device_id)
|
||||||
.await?
|
.await?
|
||||||
|
.filter(|target| device.role == Role::Admin || target.user_id == device.user_id)
|
||||||
.ok_or_else(AppError::not_found)?;
|
.ok_or_else(AppError::not_found)?;
|
||||||
if device.role != Role::Admin && target.user_id != device.user_id {
|
|
||||||
return Err(AppError::forbidden());
|
|
||||||
}
|
|
||||||
if target.is_current {
|
if target.is_current {
|
||||||
return Err(AppError::new(
|
return Err(AppError::new(
|
||||||
ErrorCode::CannotRevokeCurrentDevice,
|
ErrorCode::CannotRevokeCurrentDevice,
|
||||||
|
|||||||
@@ -77,9 +77,9 @@ async fn e16_admin_revokes_another_device_which_then_gets_device_revoked() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn e16_forbidden_not_found_and_cannot_revoke_current_device() {
|
async fn e16_not_found_and_cannot_revoke_current_device() {
|
||||||
let app = TestApp::household().await;
|
let app = TestApp::household().await;
|
||||||
// A member may revoke only their own devices.
|
// A member may revoke only their own devices; another user's device is not_found (D-156).
|
||||||
let reply = app
|
let reply = app
|
||||||
.call(
|
.call(
|
||||||
Method::DELETE,
|
Method::DELETE,
|
||||||
@@ -88,7 +88,7 @@ async fn e16_forbidden_not_found_and_cannot_revoke_current_device() {
|
|||||||
None,
|
None,
|
||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
assert_error(&reply, 403, "forbidden");
|
assert_error(&reply, 404, "not_found");
|
||||||
let reply = app
|
let reply = app
|
||||||
.call(
|
.call(
|
||||||
Method::DELETE,
|
Method::DELETE,
|
||||||
@@ -109,6 +109,42 @@ async fn e16_forbidden_not_found_and_cannot_revoke_current_device() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn e16_member_naming_the_other_users_device_gets_the_unknown_id_answer() {
|
||||||
|
// D-156 (security review L-10): the answer does not tell whether the id belongs to a device.
|
||||||
|
let app = TestApp::household().await;
|
||||||
|
let unknown = app
|
||||||
|
.call(
|
||||||
|
Method::DELETE,
|
||||||
|
&format!("/devices/{UNKNOWN_ID}"),
|
||||||
|
&member_token(),
|
||||||
|
None,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
assert_error(&unknown, 404, "not_found");
|
||||||
|
for id in [ADMIN_DEVICE_ID, ADMIN_OLD_DEVICE_ID] {
|
||||||
|
let reply = app
|
||||||
|
.call(
|
||||||
|
Method::DELETE,
|
||||||
|
&format!("/devices/{id}"),
|
||||||
|
&member_token(),
|
||||||
|
None,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
assert_eq!(reply.status, unknown.status);
|
||||||
|
assert_eq!(reply.json, unknown.json);
|
||||||
|
}
|
||||||
|
// Nothing was revoked: the admin's current device still works.
|
||||||
|
let me = app.call(Method::GET, "/me", &admin_token(), None).await;
|
||||||
|
assert_eq!(me.status, 200);
|
||||||
|
let revoked: Option<String> = sqlx::query_scalar("SELECT revoked_at FROM devices WHERE id = ?")
|
||||||
|
.bind(ADMIN_DEVICE_ID)
|
||||||
|
.fetch_one(&app.state.db)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(revoked, None);
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn e16_member_revokes_own_other_device() {
|
async fn e16_member_revokes_own_other_device() {
|
||||||
let app = TestApp::household().await;
|
let app = TestApp::household().await;
|
||||||
|
|||||||
Vendored
+1
-3
@@ -239,7 +239,7 @@ export interface paths {
|
|||||||
post?: never;
|
post?: never;
|
||||||
/**
|
/**
|
||||||
* E-16 Revoke a device
|
* E-16 Revoke a device
|
||||||
* @description Revoking an already revoked device is 204. Admin any device except the current one; member own devices except the current one (SPEC §8.4).
|
* @description Revoking an already revoked device is 204. Admin any device except the current one; member own devices except the current one, and a device of the other user is 404 `not_found`, the same answer as an unknown id (SPEC §8.4, D-156).
|
||||||
*/
|
*/
|
||||||
delete: operations["revokeDevice"];
|
delete: operations["revokeDevice"];
|
||||||
options?: never;
|
options?: never;
|
||||||
@@ -1887,8 +1887,6 @@ export interface operations {
|
|||||||
content?: never;
|
content?: never;
|
||||||
};
|
};
|
||||||
401: components["responses"]["Unauthorized"];
|
401: components["responses"]["Unauthorized"];
|
||||||
/** @description `forbidden`. */
|
|
||||||
403: components["responses"]["Forbidden"];
|
|
||||||
/** @description `not_found`. */
|
/** @description `not_found`. */
|
||||||
404: components["responses"]["NotFound"];
|
404: components["responses"]["NotFound"];
|
||||||
/** @description `cannot_revoke_current_device`. */
|
/** @description `cannot_revoke_current_device`. */
|
||||||
|
|||||||
Reference in New Issue
Block a user