Skip to content

Narrow SessionToken saves to mutated fields to preserve concurrent revocations - #34

Merged
lnagel merged 1 commit into
mainfrom
fix/unrevoke-race-update-fields
Jun 12, 2026
Merged

Narrow SessionToken saves to mutated fields to preserve concurrent revocations#34
lnagel merged 1 commit into
mainfrom
fix/unrevoke-race-update-fields

Conversation

@lnagel

@lnagel lnagel commented Jun 12, 2026

Copy link
Copy Markdown
Member

Fixes #33

Problem

utils.authenticate_payload loaded the token via SessionToken.objects.active().get(...), stamped last_used_at (and ip_address/user_agent/version via update_attributes), then called a bare session_token.save(). The in-memory instance still carried revoked_at=None, so a revocation landing in the DB between the SELECT and the save was silently overwritten — the token was un-revoked.

The review found the same pattern at the two other save() sites in the library: ObtainSessionTokenView.post and ObtainAuthorizationTokenView.post, where the resurrected session additionally received a freshly issued JWT.

Fix

Save with update_fields limited to the fields each call site actually mutates:

  • authenticate_payload: last_used_at, plus ip_address/user_agent/version only when a request was passed
  • ObtainSessionTokenView.post: ip_address, user_agent, version, last_issued_at (full insert for new tokens, detected via _state.adding since the UUID pk is pre-assigned)
  • ObtainAuthorizationTokenView.post: ip_address, user_agent, version

Side benefit: a concurrently deleted row now raises ("did not affect any rows") instead of Django's update-then-insert silently resurrecting it.

Tests

  • Race regression tests for all three sites: wrap SessionToken.save to inject a queryset-level revocation between the SELECT and the save, assert revoked_at survives in the DB. All three fail against the unfixed code.
  • test_authenticate_persists_request_attributes_and_last_used_at guards the update_fields list completeness against future additions to update_attributes.

🤖 Generated with Claude Code

…vocations

A full save() after loading a token via .active() wrote the stale
in-memory revoked_at=None back to the database, silently un-revoking
a token revoked between the SELECT and the save. Use update_fields
at all three save sites (authenticate_payload, ObtainSessionTokenView,
ObtainAuthorizationTokenView) so only the fields actually mutated are
written.

Fixes #33

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lnagel
lnagel merged commit 0987683 into main Jun 12, 2026
12 checks passed
@lnagel
lnagel deleted the fix/unrevoke-race-update-fields branch June 12, 2026 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

authenticate_payload: full save() can un-revoke a concurrently revoked SessionToken

1 participant