domain: revoke_access_token returns Ok for a foreign/nonexistent token while the UI reports success #62

New issue

Closed

opened 2026-07-03 01:42:23 +00:00 by rosa · 1 comment

rosa commented

2026-07-03 01:42:23 +00:00

Owner

Found during a code review of the domain crate.

Location: crates/domain/src/services.rs:888 (revoke_access_token)
Severity: Correctness — inconsistent ownership guard

Problem

revoke_access_token returns Ok(()) when the token is absent or belongs to another user, unlike sibling owner-gated mutations (delete_post, update_post) which return NotFound/Unauthorized. The caller flashes a success message regardless, so a foreign or nonexistent id reports success while any such token stays live.

Failure scenario

A handler bug or forged id passes the wrong AccessTokenId; the user is told the token was revoked while it remains active, and there is no error variant to surface or assert against.

Suggested fix

Add a RevokeTokenError { NotFound, Unauthorized, Unknown } and return it, matching the other owner-gated mutations.

Found during a code review of the `domain` crate. **Location:** `crates/domain/src/services.rs:888` (`revoke_access_token`) **Severity:** Correctness — inconsistent ownership guard ## Problem `revoke_access_token` returns `Ok(())` when the token is absent or belongs to another user, unlike sibling owner-gated mutations (`delete_post`, `update_post`) which return NotFound/Unauthorized. The caller flashes a success message regardless, so a foreign or nonexistent id reports success while any such token stays live. ## Failure scenario A handler bug or forged id passes the wrong `AccessTokenId`; the user is told the token was revoked while it remains active, and there is no error variant to surface or assert against. ## Suggested fix Add a `RevokeTokenError { NotFound, Unauthorized, Unknown }` and return it, matching the other owner-gated mutations.

rosa added the

needs-triage

label

2026-07-03 01:43:57 +00:00

rosa referenced this issue

2026-07-03 20:55:18 +00:00

domain: authorization code redemption is non-atomic, concurrent redeems bypass single-use #49

rosa commented

2026-07-03 20:55:19 +00:00

Author

Owner

This was generated by AI during triage.

Verified during triage by code inspection: the service silently returns Ok(()) when the token id doesn't exist or belongs to another user, and the account-page handler flashes "Access token revoked." unconditionally. Sibling owner-gated mutations (post edit/delete) return dedicated NotFound/Unauthorized errors.

Agent Brief

Category: bug
Summary: revoke_access_token must report not-found / not-yours instead of silently succeeding, and the UI must stop claiming success when nothing was revoked

Current behavior:
revoke_access_token looks up the token, and only deletes it when it exists and belongs to the calling user — but returns Ok(()) in every case. The web handler then flashes a success message regardless. A nonexistent or foreign token id reports "Access token revoked." while any such token stays live. There is no error variant to surface or write assertions against.

Desired behavior:
Revocation follows the same owner-gated contract as the post mutations: a dedicated error enum distinguishing not-found, not-owned, and unknown failures. The web handler surfaces those as the app's usual not-found / forbidden responses instead of the success flash. Revoking one's own existing token behaves exactly as today (deleted + success flash).

Key interfaces:

A new error enum for revocation with NotFound, Unauthorized, and Unknown variants, following the naming and shape of the existing owner-gated mutation errors (e.g. the post delete error).
revoke_access_token returns it instead of anyhow::Error.
A From mapping into the web error type consistent with how the sibling errors map (not-found → 404-style, unauthorized → forbidden-style).

Acceptance criteria:

Revoking a nonexistent token id returns the not-found variant and the UI does not flash success.
Revoking another user's token returns the unauthorized variant, the token remains live, and the UI does not flash success.
Revoking one's own token still deletes it and flashes success.
Service-level tests assert all three outcomes against the mock repository.
The ci mise task passes.

Out of scope:

An RFC 7009 OAuth token-revocation endpoint — this is the logged-in account page's revoke action only.
Token listing UI changes.
Authorization-code redemption (issues #49/#61).

> *This was generated by AI during triage.* Verified during triage by code inspection: the service silently returns `Ok(())` when the token id doesn't exist or belongs to another user, and the account-page handler flashes "Access token revoked." unconditionally. Sibling owner-gated mutations (post edit/delete) return dedicated `NotFound`/`Unauthorized` errors. ## Agent Brief **Category:** bug **Summary:** `revoke_access_token` must report not-found / not-yours instead of silently succeeding, and the UI must stop claiming success when nothing was revoked **Current behavior:** `revoke_access_token` looks up the token, and only deletes it when it exists *and* belongs to the calling user — but returns `Ok(())` in every case. The web handler then flashes a success message regardless. A nonexistent or foreign token id reports "Access token revoked." while any such token stays live. There is no error variant to surface or write assertions against. **Desired behavior:** Revocation follows the same owner-gated contract as the post mutations: a dedicated error enum distinguishing not-found, not-owned, and unknown failures. The web handler surfaces those as the app's usual not-found / forbidden responses instead of the success flash. Revoking one's own existing token behaves exactly as today (deleted + success flash). **Key interfaces:** - A new error enum for revocation with `NotFound`, `Unauthorized`, and `Unknown` variants, following the naming and shape of the existing owner-gated mutation errors (e.g. the post delete error). - `revoke_access_token` returns it instead of `anyhow::Error`. - A `From` mapping into the web error type consistent with how the sibling errors map (not-found → 404-style, unauthorized → forbidden-style). **Acceptance criteria:** - [ ] Revoking a nonexistent token id returns the not-found variant and the UI does not flash success. - [ ] Revoking another user's token returns the unauthorized variant, the token remains live, and the UI does not flash success. - [ ] Revoking one's own token still deletes it and flashes success. - [ ] Service-level tests assert all three outcomes against the mock repository. - [ ] The `ci` mise task passes. **Out of scope:** - An RFC 7009 OAuth token-revocation endpoint — this is the logged-in account page's revoke action only. - Token listing UI changes. - Authorization-code redemption (issues #49/#61).

rosa added

bug

ready-for-agent

and removed

needs-triage

labels

2026-07-03 20:55:32 +00:00

rosa referenced this issue from a commit

2026-07-03 21:06:56 +00:00

fix: Report not-found and unauthorized from revoke_access_token

rosa referenced this issue from a pull request that will close it,

2026-07-03 21:07:22 +00:00