A disabled account's token still works: the check is at login only #143

New issue

Closed

opened 2026-08-16 08:41:50 +02:00 by Claude · 1 comment

Claude commented

2026-08-16 08:41:50 +02:00

Collaborator

Upstream refuses a disabled account on every request, not only at login.
AuthService.Authenticate throws SecurityException("User account has been disabled.") for any request carrying the token
(Emby.Server.Implementations/HttpServer/Security/AuthService.cs), so the
refusal survives the token the way the remote-access and parental-schedule
checks now do.

This port checks IsDisabled at login only
(crates/jellyfin-app/src/session/auth/mod.rs). #134 closed the other two
because they live in DefaultAuthorizationHandler, which
crates/jellyfin-http/src/auth.rs now models; this one lives one layer up, in
the authentication service, and so was not picked up by that work.

Mostly masked, and that is the reason it is p3 rather than p2.
session/users.rs::set_policy revokes an account's tokens when it sets
is_disabled, so the dashboard path closes the window itself. What is left is a
token issued before the row was disabled by any other route — a restore, a
direct write, or a future route that disables without revoking.

Where to start

crates/jellyfin-http/src/auth.rs, in Authenticated::of rather than in
admitted: it is authentication and not a policy, so it applies to every type
in that module's table — including Anonymous, where a token that arrives is
still resolved, and including the websocket, which authenticates by hand.

The answer shape is a SecurityException, so it is ApiError::Forbidden — the
403 text/plain one — and not the empty 403 a policy writes. That distinction
was measured while closing #142 and the two variants are already there.

Note the cost: it is one more column off a row Authenticated::of does not read
today. The permissions are already read there, and IsDisabled is one of them,
so this may be free.

How to know it is done

A token minted for an account, the account then disabled without the revoke, and
the token replayed: refused with the status and body upstream gives, on a route
under each of the policy types. A route test is enough — the fixture has no
disabled account and creating one is a write.

House rules

Tests run under cargo nextest; upstream's source is the checkout in upstream/
at the pin; comments say what the code does, not how it got there. The shared
reference on :8096 is read-only, and disabling an account is a write.

Upstream refuses a disabled account on **every** request, not only at login. `AuthService.Authenticate` throws `SecurityException("User account has been disabled.")` for any request carrying the token (`Emby.Server.Implementations/HttpServer/Security/AuthService.cs`), so the refusal survives the token the way the remote-access and parental-schedule checks now do. This port checks `IsDisabled` at login only (`crates/jellyfin-app/src/session/auth/mod.rs`). #134 closed the other two because they live in `DefaultAuthorizationHandler`, which `crates/jellyfin-http/src/auth.rs` now models; this one lives one layer up, in the *authentication* service, and so was not picked up by that work. **Mostly masked, and that is the reason it is p3 rather than p2.** `session/users.rs::set_policy` revokes an account's tokens when it sets `is_disabled`, so the dashboard path closes the window itself. What is left is a token issued before the row was disabled by any other route — a restore, a direct write, or a future route that disables without revoking. ## Where to start `crates/jellyfin-http/src/auth.rs`, in `Authenticated::of` rather than in `admitted`: it is authentication and not a policy, so it applies to every type in that module's table — including `Anonymous`, where a token that arrives is still resolved, and including the websocket, which authenticates by hand. The answer shape is a `SecurityException`, so it is `ApiError::Forbidden` — the `403 text/plain` one — and not the empty `403` a policy writes. That distinction was measured while closing #142 and the two variants are already there. Note the cost: it is one more column off a row `Authenticated::of` does not read today. The permissions are already read there, and `IsDisabled` is one of them, so this may be free. ## How to know it is done A token minted for an account, the account then disabled without the revoke, and the token replayed: refused with the status and body upstream gives, on a route under each of the policy types. A route test is enough — the fixture has no disabled account and creating one is a write. ### House rules Tests run under `cargo nextest`; upstream's source is the checkout in `upstream/` **at the pin**; comments say what the code does, not how it got there. The shared reference on `:8096` is read-only, and disabling an account is a write.

Claude referenced this issue

2026-08-16 08:42:21 +02:00

A token whose account row is gone is admitted, where upstream refuses it #144

Claude added the

http

security

labels

2026-08-16 08:58:02 +02:00

alex referenced this issue from a commit

2026-08-16 09:28:23 +02:00

auth: a check made at login has to survive the token

alex closed this issue

2026-08-16 09:28:23 +02:00

Claude commented

2026-08-16 09:29:28 +02:00

Author

Collaborator

Closed by 9775776.

The issue had the answer wrong, and so did the review that raised it: it is
401, not 403. Upstream refuses a disabled account in AuthService, which
is authentication, and CustomAuthenticationHandler turns the
SecurityException into AuthenticateResult.Fail — a challenge, not a forbid.

There is a trap in measuring it. Disabling through POST /Users/{id}/Policy
revokes the account's tokens, and a revoked token also answers 401 — so the
obvious measurement proves nothing. Isolated by writing IsDisabled under a
live token on a private instance over a cp -a copy of the fixture:

route	with the flag written under the token
`/Users/Me`, `/Items`, `/System/Info`, `/Localization/Cultures`, `/ScheduledTasks`, `/Environment/DefaultDirectoryBrowser`	401, empty
`/System/Info/Public`	200
`/Branding/Splashscreen`	404 — anonymous, and reported missing

That last row is the consequence of it being authentication rather than a
policy: a route that wanted no caller anyway goes on without one instead of
refusing. Authenticated::of raises it, so Option<Authenticated> and
Anonymous see None — which is what an absent principal does upstream.

Replayed as a pair against that instance afterwards: 8 of 8 routes agree.

Closed by `9775776`. **The issue had the answer wrong, and so did the review that raised it: it is `401`, not `403`.** Upstream refuses a disabled account in `AuthService`, which is *authentication*, and `CustomAuthenticationHandler` turns the `SecurityException` into `AuthenticateResult.Fail` — a challenge, not a forbid. There is a trap in measuring it. Disabling through `POST /Users/{id}/Policy` revokes the account's tokens, and a revoked token also answers `401` — so the obvious measurement proves nothing. Isolated by writing `IsDisabled` under a live token on a private instance over a `cp -a` copy of the fixture: | route | with the flag written under the token | |---|---| | `/Users/Me`, `/Items`, `/System/Info`, `/Localization/Cultures`, `/ScheduledTasks`, `/Environment/DefaultDirectoryBrowser` | **401**, empty | | `/System/Info/Public` | 200 | | `/Branding/Splashscreen` | 404 — anonymous, and reported missing | That last row is the consequence of it being authentication rather than a policy: a route that wanted no caller anyway goes on without one instead of refusing. `Authenticated::of` raises it, so `Option<Authenticated>` and `Anonymous` see `None` — which is what an absent principal does upstream. Replayed as a pair against that instance afterwards: 8 of 8 routes agree.