mirror of
https://github.com/advplyr/audiobookshelf.git
synced 2026-07-24 10:11:44 +00:00
Make the refresh-token grace period configurable (default 10 min)
When /auth/refresh rotates the refresh token, the previous token is kept valid for a short grace period so a client that never received the rotation response (e.g. a dropped or suspended mobile request) can retry with the old token and get back the already-rotated current token instead of a 401. That window was hardcoded to 60 seconds, which is too short for real mobile conditions: iOS backgrounding, VPN/proxy timeouts, or network handoff can delay the retry past a minute, permanently locking the session out (#5281). Make it configurable via REFRESH_TOKEN_GRACE_PERIOD (seconds), following the existing REFRESH_TOKEN_EXPIRY / ACCESS_TOKEN_EXPIRY pattern, and raise the default to 10 minutes. Adds unit tests for the new config. Fixes #5281
This commit is contained in:
parent
59c7d0275c
commit
95af4812cd
2 changed files with 63 additions and 2 deletions
|
|
@ -16,6 +16,8 @@ class TokenManager {
|
|||
this.RefreshTokenExpiry = parseInt(process.env.REFRESH_TOKEN_EXPIRY) || 30 * 24 * 60 * 60 // 30 days
|
||||
/** @type {number} Access token expiry in seconds */
|
||||
this.AccessTokenExpiry = parseInt(process.env.ACCESS_TOKEN_EXPIRY) || 1 * 60 * 60 // 1 hour
|
||||
/** @type {number} Grace period in seconds during which a rotated (old) refresh token is still accepted */
|
||||
this.RefreshTokenGracePeriod = parseInt(process.env.REFRESH_TOKEN_GRACE_PERIOD) || 10 * 60 // 10 minutes
|
||||
|
||||
if (parseInt(process.env.REFRESH_TOKEN_EXPIRY) > 0) {
|
||||
Logger.info(`[TokenManager] Refresh token expiry set from ENV variable to ${this.RefreshTokenExpiry} seconds`)
|
||||
|
|
@ -23,6 +25,9 @@ class TokenManager {
|
|||
if (parseInt(process.env.ACCESS_TOKEN_EXPIRY) > 0) {
|
||||
Logger.info(`[TokenManager] Access token expiry set from ENV variable to ${this.AccessTokenExpiry} seconds`)
|
||||
}
|
||||
if (parseInt(process.env.REFRESH_TOKEN_GRACE_PERIOD) > 0) {
|
||||
Logger.info(`[TokenManager] Refresh token grace period set from ENV variable to ${this.RefreshTokenGracePeriod} seconds`)
|
||||
}
|
||||
}
|
||||
|
||||
get TokenSecret() {
|
||||
|
|
@ -199,9 +204,13 @@ class TokenManager {
|
|||
let lastRefreshTokenExpiresAt = null
|
||||
if (gracePeriod) {
|
||||
// Set grace period of old refresh token in case of race condition in token rotation.
|
||||
// This grace period may need to be longer if fetching the user data takes longer due to large progress objects
|
||||
// During this window a retry with the old refresh token returns the already-rotated
|
||||
// current token instead of failing, so a client that never received the rotation
|
||||
// response (e.g. dropped/suspended mobile request) can still recover the session.
|
||||
// Configurable via REFRESH_TOKEN_GRACE_PERIOD; may need to be longer if fetching the
|
||||
// user data takes longer due to large progress objects.
|
||||
lastRefreshToken = previousRefreshToken
|
||||
lastRefreshTokenExpiresAt = new Date(Date.now() + 60 * 1000) // 1 minute grace period
|
||||
lastRefreshTokenExpiresAt = new Date(Date.now() + this.RefreshTokenGracePeriod * 1000)
|
||||
}
|
||||
|
||||
// Only update if this session row still has the refresh token we read
|
||||
|
|
|
|||
52
test/server/auth/TokenManager.test.js
Normal file
52
test/server/auth/TokenManager.test.js
Normal file
|
|
@ -0,0 +1,52 @@
|
|||
const chai = require('chai')
|
||||
const sinon = require('sinon')
|
||||
const { expect } = chai
|
||||
|
||||
// Require Database before TokenManager to resolve the Database -> Auth -> TokenManager
|
||||
// require cycle (otherwise requiring TokenManager first yields an incomplete export).
|
||||
require('../../../server/Database')
|
||||
const Logger = require('../../../server/Logger')
|
||||
const TokenManager = require('../../../server/auth/TokenManager')
|
||||
|
||||
describe('TokenManager', () => {
|
||||
describe('refresh token grace period configuration', () => {
|
||||
let loggerInfoStub
|
||||
const originalEnv = process.env.REFRESH_TOKEN_GRACE_PERIOD
|
||||
|
||||
beforeEach(() => {
|
||||
loggerInfoStub = sinon.stub(Logger, 'info')
|
||||
delete process.env.REFRESH_TOKEN_GRACE_PERIOD
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
loggerInfoStub.restore()
|
||||
if (originalEnv === undefined) delete process.env.REFRESH_TOKEN_GRACE_PERIOD
|
||||
else process.env.REFRESH_TOKEN_GRACE_PERIOD = originalEnv
|
||||
})
|
||||
|
||||
it('defaults to 10 minutes (600 seconds) when the env var is not set', () => {
|
||||
const tokenManager = new TokenManager()
|
||||
expect(tokenManager.RefreshTokenGracePeriod).to.equal(600)
|
||||
})
|
||||
|
||||
it('reads a positive value from REFRESH_TOKEN_GRACE_PERIOD (in seconds)', () => {
|
||||
process.env.REFRESH_TOKEN_GRACE_PERIOD = '300'
|
||||
const tokenManager = new TokenManager()
|
||||
expect(tokenManager.RefreshTokenGracePeriod).to.equal(300)
|
||||
expect(loggerInfoStub.calledWithMatch(/grace period set from ENV/i)).to.equal(true)
|
||||
})
|
||||
|
||||
it('falls back to the default for a non-positive or invalid value', () => {
|
||||
process.env.REFRESH_TOKEN_GRACE_PERIOD = '0'
|
||||
expect(new TokenManager().RefreshTokenGracePeriod).to.equal(600)
|
||||
|
||||
process.env.REFRESH_TOKEN_GRACE_PERIOD = 'not-a-number'
|
||||
expect(new TokenManager().RefreshTokenGracePeriod).to.equal(600)
|
||||
})
|
||||
|
||||
it('does not log the ENV override message when the env var is unset', () => {
|
||||
new TokenManager()
|
||||
expect(loggerInfoStub.calledWithMatch(/grace period set from ENV/i)).to.equal(false)
|
||||
})
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue