starred/audiobookshelf
1
0
Fork 0
mirror of https://github.com/advplyr/audiobookshelf.git synced 2026-05-30 23:40:40 +02:00
Code Issues 1.0k Packages Projects Releases 10 Wiki Activity

[PR #5004] Access token refresh grace period #4394

New Issue
Open
opened 2026-04-25 00:19:35 +02:00 by adam · 0 comments
adam commented 2026-04-25 00:19:35 +02:00
Owner
Copy Link
Copy Source

📋 Pull Request Information

Original PR: https://github.com/advplyr/audiobookshelf/pull/5004
Author: @nichwall
Created: 1/25/2026
Status: 🔄 Open

Base: master ← Head: token_refresh_race_condition

📝 Commits (6)

  • da0a64d Add: 10 second grace period to access token cycle
  • 7aa2f84 Revert default token expiry
  • e1ae4f2 Fix: race condition in rotation
  • b8a2d11 Allow rotation without grace period for invalidating all user sessions
  • 077b523 Fix JS Doc deletion
  • cfeb6bd Fix: grace period enable statement

📊 Changes

3 files changed (+166 additions, -15 deletions)

View changed files

📝 server/auth/TokenManager.js (+70 -15)
➕ server/migrations/v2.33.0-add-last-refresh-token.js (+84 -0)
📝 server/models/Session.js (+12 -0)

📄 Description

Brief summary

This pull request adds a brief 1 minute grace period when generating new access tokens to address a race condition when rotating tokens.

Which issue is fixed?

Attempted fix https://github.com/advplyr/audiobookshelf/issues/4630

In-depth Description

A race condition can occur in the token refresh logic when the Access Token has expired, because the rotateTokensForSession saves the new session regardless of whether it has already been updated. This allows a client to send two refresh requests to the server, where each generates a new refresh token. The first refresh token will be returned to the client, but the second one overwrites the first in the database, causing the user to be unable to generate a new access token an hour later when it expires.

To fix this, I added two new columns to the session table, lastRefreshToken and lastRefreshTokenExpiresAt, to give a short grace period to the second request. When checking if the access token is valid, we check if the current refresh token matches and has not expired, or if the last refresh token has not expired during its 1 minute grace period (we can make this shorter, but the race condition is more likely to occur with larger databases so erred on the side of caution).

Then, when rotating the tokens, we only store the new tokens if the refreshToken matches what is now the lastRefreshToken to prevent the second request from overwriting the first request. If this write to the database does not occur, the rotation function instead loads the refresh token from the database and returns this token instead of generating a new one.

How have you tested this?

I was unable to recreate the original issue with a short lived access token (3 seconds), but tested by navigating between pages on a web browser (desktop) and Android app at the same time, with brief pauses to verify each token was refreshing correctly.

Screenshots

No front end changes.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/advplyr/audiobookshelf/pull/5004 **Author:** [@nichwall](https://github.com/nichwall) **Created:** 1/25/2026 **Status:** 🔄 Open **Base:** `master` ← **Head:** `token_refresh_race_condition` --- ### 📝 Commits (6) - [`da0a64d`](https://github.com/advplyr/audiobookshelf/commit/da0a64daed44697e020ff734c53cee6e8b7fab43) Add: 10 second grace period to access token cycle - [`7aa2f84`](https://github.com/advplyr/audiobookshelf/commit/7aa2f84daad13fc9f4b7646797385e25cf2f9872) Revert default token expiry - [`e1ae4f2`](https://github.com/advplyr/audiobookshelf/commit/e1ae4f2d315d153e206edebb8857883af896a5c7) Fix: race condition in rotation - [`b8a2d11`](https://github.com/advplyr/audiobookshelf/commit/b8a2d113f05b180623f6f944f69ea746c9c03a26) Allow rotation without grace period for invalidating all user sessions - [`077b523`](https://github.com/advplyr/audiobookshelf/commit/077b523bd66fce70639aa8f88d92e2c442115fca) Fix JS Doc deletion - [`cfeb6bd`](https://github.com/advplyr/audiobookshelf/commit/cfeb6bd502a0f69483db35c37217b8f738c5d029) Fix: grace period enable statement ### 📊 Changes **3 files changed** (+166 additions, -15 deletions) <details> <summary>View changed files</summary> 📝 `server/auth/TokenManager.js` (+70 -15) ➕ `server/migrations/v2.33.0-add-last-refresh-token.js` (+84 -0) 📝 `server/models/Session.js` (+12 -0) </details> ### 📄 Description <!-- For Work In Progress Pull Requests, please use the Draft PR feature, see https://github.blog/2019-02-14-introducing-draft-pull-requests/ for further details. If you do not follow this template, the PR may be closed without review. Please ensure all checks pass. If you are a new contributor, the workflows will need to be manually approved before they run. --> ## Brief summary This pull request adds a brief 1 minute grace period when generating new access tokens to address a race condition when rotating tokens. ## Which issue is fixed? Attempted fix https://github.com/advplyr/audiobookshelf/issues/4630 ## In-depth Description A race condition can occur in the token refresh logic when the Access Token has expired, because the `rotateTokensForSession` saves the new session regardless of whether it has already been updated. This allows a client to send two refresh requests to the server, where each generates a new refresh token. The first refresh token will be returned to the client, but the second one overwrites the first in the database, causing the user to be unable to generate a new access token an hour later when it expires. To fix this, I added two new columns to the session table, `lastRefreshToken` and `lastRefreshTokenExpiresAt`, to give a short grace period to the second request. When checking if the access token is valid, we check if the current refresh token matches and has not expired, or if the last refresh token has not expired during its 1 minute grace period (we can make this shorter, but the race condition is more likely to occur with larger databases so erred on the side of caution). Then, when rotating the tokens, we only store the new tokens if the `refreshToken` matches what is now the `lastRefreshToken` to prevent the second request from overwriting the first request. If this write to the database does not occur, the rotation function instead loads the refresh token from the database and returns this token instead of generating a new one. ## How have you tested this? I was unable to recreate the original issue with a short lived access token (3 seconds), but tested by navigating between pages on a web browser (desktop) and Android app at the same time, with brief pauses to verify each token was refreshing correctly. ## Screenshots No front end changes. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2026-04-25 00:19:35 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
authentication
backlog
bug
chapter editor
config-issue
ebooks
encoding/embedding
enhancement
help wanted
listening sessions & progress
planned
possible plugin
progress sync
pull-request
Mirrored from GitHub Pull Request
 sorting/filtering/searching
unable to reproduce
upload
users & permissions
waiting
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam (Adam Melkus)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/audiobookshelf#4394
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?