Logging Out Didn’t Log You Out, Twice
Issue
A user logs out. The endpoint returns 200 {"message": "Logout processed"}. The API key keeps working.
Root Cause
Bug one: an enum member that does not exist.
The handler set the key’s status to APIKeyStatus.REVOKED. There is no REVOKED member — the enum has PENDING, ACTIVE, EXPIRED and DECLINED. So the line raised AttributeError at runtime, inside a try whose except returned a friendly "Logout processed; key state unchanged." with HTTP 200.
Two ordinary mistakes producing a security control that fails open and fails silent: an enum typo invisible to any linter that does not resolve members, laundered into a success response by an over-broad exception handler.
The fix was to stamp revoked_at, matching how the existing revoke path already worked and matching the revoked_at IS NULL filter every lookup already applied.
Bug two, thirteen minutes later: the right invalidation, aimed at the wrong namespace.
The database write now worked, and the key still kept working for the length of a cache TTL. Authentication is cached in tiers. The logout path invalidated api_key_auth:{hash_of_key}. The hot path in the validator reads a different tier: api_key_validation:{token_id}.
The invalidation call was real, it succeeded, and it cleared a cache nobody was reading on that path. This is the worst shape a cache bug can take — the fix looks present in the diff and does nothing.
The fix was to call the same helper the existing revoke flow used, which clears the validation, daily-limit and auth tiers together.
A sibling of the same bug lived nearby: naturally expiring keys kept working for about fifteen minutes, because the cached authentication payload did not carry expires_at at all. On a cache hit there was nothing to re-check.
Solution
- Write
revoked_at, and delete the code path that could have written a non-existent status. - Narrow the exception handler so a failure to revoke is a 500, not a 200. A security endpoint must not have a success-shaped fallback.
- Route every invalidation through one helper that clears all tiers, and forbid open-coding cache keys at call sites.
- Include the fields the hot path needs to re-check — expiry among them — in the cached payload, so a hit can still say no.
💡 Takeaway
- A
try/exceptthat returns a success message on failure is worse than a crash. It makes the control fail open and removes the evidence in one move. - Cache invalidation is a correctness API, not a performance one. When tiers are keyed differently — by key hash here, by token id there — invalidation belongs behind a single helper that every write path calls.
- A cached authorisation decision must carry everything needed to re-evaluate it. Otherwise a cache hit is a decision nobody is allowed to revisit.
- Verify security fixes from the outside. Both bugs presented identically to the user: a 200 and a key that still worked. Only an end-to-end “revoke, then call the API” check distinguishes them.
