diff --git a/server/Auth.js b/server/Auth.js index 65532897d..f63e84460 100644 --- a/server/Auth.js +++ b/server/Auth.js @@ -81,7 +81,7 @@ class Auth { * @param {import('./models/User')} user * @param {Request} req * @param {Response} res - * @returns {Promise<{ accessToken:string, refreshToken:string }|null>} new tokens for the current session if kept alive + * @returns {Promise} accessToken only if user is current user and refresh token is valid */ async invalidateJwtSessionsForUser(user, req, res) { return this.tokenManager.invalidateJwtSessionsForUser(user, req, res) diff --git a/server/auth/TokenManager.js b/server/auth/TokenManager.js index 5ebc6530b..f91adc8b9 100644 --- a/server/auth/TokenManager.js +++ b/server/auth/TokenManager.js @@ -464,19 +464,14 @@ class TokenManager { * @param {import('../models/User')} user * @param {import('express').Request} req * @param {import('express').Response} res - * @returns {Promise<{ accessToken:string, refreshToken:string }|null>} new tokens for the current session if kept alive + * @returns {Promise} accessToken only if user is current user and refresh token is valid */ async invalidateJwtSessionsForUser(user, req, res) { - const currentRefreshToken = req.cookies.refresh_token || req.headers['x-refresh-token'] + const currentRefreshToken = req.cookies.refresh_token if (req.user.id === user.id && currentRefreshToken) { // Current user is the same as the user to invalidate sessions for // So rotate token for current session - const currentSession = await Database.sessionModel.findOne({ - where: { - userId: user.id, - [Op.or]: [{ refreshToken: currentRefreshToken }, { lastRefreshToken: currentRefreshToken }] - } - }) + const currentSession = await Database.sessionModel.findOne({ where: { refreshToken: currentRefreshToken } }) if (currentSession) { const newTokens = await this.rotateTokensForSession(currentSession, user, req, res, false) @@ -490,10 +485,7 @@ class TokenManager { } }) - return { - accessToken: newTokens.accessToken, - refreshToken: newTokens.refreshToken - } + return newTokens.accessToken } else { Logger.error(`[TokenManager] No session found to rotate tokens`) } diff --git a/server/controllers/MeController.js b/server/controllers/MeController.js index 101dc6f14..ec2198799 100644 --- a/server/controllers/MeController.js +++ b/server/controllers/MeController.js @@ -351,8 +351,6 @@ class MeController { * User change password. Requires current password. * Guest users cannot change password. * - * Invalidates all other JWT sessions for the user. If using x-refresh-token, returns new tokens for the current session. - * * @this import('../routers/ApiRouter') * * @param {RequestWithUser} req @@ -375,24 +373,6 @@ class MeController { return res.status(400).send(result.error) } - const shouldReturnTokens = !!req.headers['x-refresh-token'] - const newTokens = await this.auth.invalidateJwtSessionsForUser(req.user, req, res) - - if (newTokens?.accessToken) { - Logger.info(`[MeController] Invalidated other JWT sessions for user ${req.user.username} after password change`) - if (shouldReturnTokens) { - return res.json({ - success: true, - user: { - accessToken: newTokens.accessToken, - refreshToken: newTokens.refreshToken - } - }) - } - } else { - Logger.info(`[MeController] Invalidated all JWT sessions for user ${req.user.username} after password change`) - } - res.sendStatus(200) } diff --git a/server/controllers/UserController.js b/server/controllers/UserController.js index 0a476a9d6..4a4da0366 100644 --- a/server/controllers/UserController.js +++ b/server/controllers/UserController.js @@ -253,7 +253,6 @@ class UserController { // Updating password if (updatePayload.password) { user.pash = await this.auth.localAuthStrategy.hashPassword(updatePayload.password) - shouldInvalidateJwtSessions = true hasUpdates = true } @@ -332,11 +331,14 @@ class UserController { Logger.info(`[UserController] User ${user.username} has generated a new api token`) } - // Handle JWT session invalidation for username/password changes + // Handle JWT session invalidation for username changes if (shouldInvalidateJwtSessions) { - const newTokens = await this.auth.invalidateJwtSessionsForUser(user, req, res) - if (newTokens) { - // Note: for admin users changing their own password they should use MeController.updatePassword instead. This endpoint does not return tokens + const newAccessToken = await this.auth.invalidateJwtSessionsForUser(user, req, res) + if (newAccessToken) { + user.accessToken = newAccessToken + // Refresh tokens are only returned for mobile clients + // Mobile apps currently do not use this API endpoint so always set to null + user.refreshToken = null Logger.info(`[UserController] Invalidated JWT sessions for user ${user.username} and rotated tokens for current session`) } else { Logger.info(`[UserController] Invalidated JWT sessions for user ${user.username}`) diff --git a/test/server/auth/TokenManager.test.js b/test/server/auth/TokenManager.test.js index ad583455c..38c739d09 100644 --- a/test/server/auth/TokenManager.test.js +++ b/test/server/auth/TokenManager.test.js @@ -1,6 +1,5 @@ const { expect } = require('chai') const sinon = require('sinon') -const { Op } = require('sequelize') const Database = require('../../../server/Database') const jwt = require('../../../server/libs/jsonwebtoken') @@ -66,77 +65,4 @@ describe('TokenManager', () => { expect(done.calledWith(null, user)).to.be.true }) }) - - describe('invalidateJwtSessionsForUser', () => { - const targetUser = { id: userId, username: 'testuser' } - const currentSession = { - id: 'session-current', - userId, - refreshToken: 'refresh-current' - } - - /** Minimal req/res for session invalidation (ApiRouter provides auth on real requests). */ - function makeReq({ requestUserId = userId, refreshToken = null, cookieRefreshToken = null } = {}) { - return { - user: { id: requestUserId }, - cookies: cookieRefreshToken ? { refresh_token: cookieRefreshToken } : {}, - headers: refreshToken ? { 'x-refresh-token': refreshToken } : {} - } - } - - let sessionFindOne - let sessionDestroy - let rotateStub - - beforeEach(() => { - sessionFindOne = sinon.stub().resolves(currentSession) - sessionDestroy = sinon.stub().resolves(1) - sinon.stub(Database, 'sessionModel').get(() => ({ - findOne: sessionFindOne, - destroy: sessionDestroy - })) - rotateStub = sinon.stub(tokenManager, 'rotateTokensForSession').resolves({ - accessToken: 'access-new', - refreshToken: 'refresh-new' - }) - }) - - it('self password change: keeps current session, deletes others', async () => { - const req = makeReq({ refreshToken: 'refresh-current' }) - const res = { cookie: sinon.spy() } - - const result = await tokenManager.invalidateJwtSessionsForUser(targetUser, req, res) - - // Found this device's session using the x-refresh-token header - expect(sessionFindOne.calledOnce).to.be.true - const findWhere = sessionFindOne.firstCall.args[0].where - expect(findWhere.userId).to.equal(userId) - expect(findWhere[Op.or]).to.deep.equal([{ refreshToken: 'refresh-current' }, { lastRefreshToken: 'refresh-current' }]) - - // Rotated in place (no grace period) so the caller keeps a valid session - expect(rotateStub.calledOnceWith(currentSession, targetUser, req, res, false)).to.be.true - - // Deleted all other sessions, but not this one - expect(sessionDestroy.calledOnce).to.be.true - const destroyWhere = sessionDestroy.firstCall.args[0].where - expect(destroyWhere.userId).to.equal(userId) - expect(destroyWhere.id[Op.ne]).to.equal(currentSession.id) - - expect(result).to.deep.equal({ accessToken: 'access-new', refreshToken: 'refresh-new' }) - }) - - it('admin password reset: deletes all target sessions', async () => { - const req = makeReq({ requestUserId: 'admin-id', refreshToken: 'refresh-current' }) - const res = { cookie: sinon.spy() } - - const result = await tokenManager.invalidateJwtSessionsForUser(targetUser, req, res) - - // Token rotation did not happen because target is a different user - expect(sessionFindOne.called).to.be.false - expect(rotateStub.called).to.be.false - - expect(sessionDestroy.calledOnceWith(sinon.match({ where: { userId } }))).to.be.true - expect(result).to.equal(null) - }) - }) })