feat(mcp): rotate MCP OAuth refresh tokens with replay detection

forge/db/controllers/AccessToken.js

@@ -6,12 +6,18 @@ const DEFAULT_TOKEN_SESSION_EXPIRY = 1000 * 60 * 30 // 30 mins session - with re
 
 const DEFAULT_REFRESH_TOKEN_EXPIRY = 1000 * 60 * 60 * 24 * 30 // 30 days - sliding refresh token lifetime
 
-// Concurrent refreshes of the same refresh token reuse the cached access token
-// rather than each minting a new one and overwriting the row. Re-mint once the
-// cached token is within this window of expiry.
+// Concurrent refreshes of the same refresh token converge on one rotation result
+// via this shared cache (Valkey in production) rather than each minting its own
+// and racing on the row. The result carries the new refresh token so every caller
+// is handed the same one; a stale cached access token is re-minted once within
+// this window of expiry so a client is never handed a token about to die.
 const MCP_ACCESS_TOKEN_CACHE = 'mcp-oauth-access-token'
 const MCP_ACCESS_TOKEN_REMAINING_LIMIT = 1000 * 60 * 5 // 5 minutes
 
+// A rotated-out refresh token stays valid for this long so a concurrent or retried
+// refresh still succeeds; presenting it after the window is treated as a replay.
+const MCP_REFRESH_TOKEN_GRACE = 1000 * 60 // 60 seconds
+
 const DEFAULT_DEVICE_OTC_EXPIRY = 1000 * 60 * 60 * 24 // 24 hours
 
 /*
@@ -442,14 +448,12 @@ module.exports = {
     },
 
     refreshToken: async function (app, refreshToken) {
-        const existingToken = await app.db.models.AccessToken.byRefreshToken(refreshToken)
-        if (!existingToken) {
-            return null
-        }
         const [prefix] = refreshToken.split('_')
+        const existingToken = await app.db.models.AccessToken.byRefreshToken(refreshToken)
 
-        // Editor sessions have no refresh lifetime: rotate the refresh token each use.
-        if (!existingToken.refreshTokenExpiresAt) {
+        // Tokens without their own refresh lifetime (e.g. editor sessions) rotate
+        // the refresh token on each use and have no replay handling.
+        if (existingToken && !existingToken.refreshTokenExpiresAt) {
             const tokenUpdates = {
                 token: generateToken(32, prefix),
                 refreshToken: generateToken(32, prefix),
@@ -459,29 +463,73 @@ module.exports = {
             return tokenUpdates
         }
 
-        // Past its lifetime the refresh token is dead: remove the row.
-        if (existingToken.refreshTokenExpiresAt.getTime() < Date.now()) {
-            await existingToken.destroy()
-            return null
-        }
-
-        // Stable refresh token: concurrent refreshes reuse the cached access token
-        // instead of each minting one and overwriting the row.
         const cache = app.caches?.getCache?.(MCP_ACCESS_TOKEN_CACHE, { ttl: DEFAULT_TOKEN_SESSION_EXPIRY, max: 10000 })
         const cacheKey = sha256(refreshToken)
-        const cached = await cache?.get(cacheKey)
-        if (cached && cached.expiresAt - Date.now() > MCP_ACCESS_TOKEN_REMAINING_LIMIT) {
-            return { token: cached.token, expiresAt: cached.expiresAt, refreshToken }
+
+        // The presented token is the current one: rotate it.
+        if (existingToken) {
+            // Once the refresh lifetime has passed the grant is over: remove it
+            // rather than issuing a new access token.
+            if (existingToken.refreshTokenExpiresAt.getTime() < Date.now()) {
+                await existingToken.destroy()
+                return null
+            }
+            // A concurrent refresh may already have rotated this token; reuse its
+            // result so both callers receive the same new refresh token.
+            const cached = await cache?.get(cacheKey)
+            if (cached && cached.expiresAt - Date.now() > MCP_ACCESS_TOKEN_REMAINING_LIMIT) {
+                return { token: cached.token, expiresAt: cached.expiresAt, refreshToken: cached.refreshToken }
+            }
+            const token = generateToken(32, prefix)
+            const newRefreshToken = generateToken(32, prefix)
+            const expiresAt = Date.now() + DEFAULT_TOKEN_SESSION_EXPIRY
+            // Compare-and-swap on the current refresh token: the update only matches
+            // while this token is still current, so of two simultaneous refreshes
+            // exactly one rotates and the other sees zero rows affected.
+            const [rotatedCount] = await app.db.models.AccessToken.update(
+                {
+                    token,
+                    expiresAt,
+                    refreshToken: newRefreshToken,
+                    refreshTokenExpiresAt: Date.now() + DEFAULT_REFRESH_TOKEN_EXPIRY,
+                    previousRefreshToken: existingToken.refreshToken,
+                    previousRefreshTokenRotatedAt: new Date()
+                },
+                { where: { refreshToken: existingToken.refreshToken } }
+            )
+            if (rotatedCount === 0) {
+                // A concurrent refresh won the swap; return its result rather than a
+                // token that never made it onto the row.
+                const winner = await cache?.get(cacheKey)
+                if (winner) {
+                    return { token: winner.token, expiresAt: winner.expiresAt, refreshToken: winner.refreshToken }
+                }
+                return null
+            }
+            await cache?.set(cacheKey, { token, expiresAt, refreshToken: newRefreshToken })
+            return { token, expiresAt, refreshToken: newRefreshToken }
         }
 
-        const token = generateToken(32, prefix)
-        const expiresAt = Date.now() + DEFAULT_TOKEN_SESSION_EXPIRY
-        await app.db.models.AccessToken.update(
-            { token, expiresAt, refreshTokenExpiresAt: Date.now() + DEFAULT_REFRESH_TOKEN_EXPIRY },
-            { where: { refreshToken: existingToken.refreshToken } }
-        )
-        await cache?.set(cacheKey, { token, expiresAt })
-        return { token, expiresAt, refreshToken }
+        // Not the current token: it may be one just rotated out. byRefreshToken
+        // stores the sha256, so match the presented token's hash here too.
+        const rotated = await app.db.models.AccessToken.findOne({ where: { previousRefreshToken: cacheKey } })
+        if (!rotated) {
+            return null
+        }
+        const rotatedAt = rotated.previousRefreshTokenRotatedAt ? rotated.previousRefreshTokenRotatedAt.getTime() : 0
+        if (Date.now() - rotatedAt <= MCP_REFRESH_TOKEN_GRACE) {
+            // Within the grace window this is a retry or a lagging concurrent
+            // refresh: hand back the tokens the rotation produced.
+            const cached = await cache?.get(cacheKey)
+            if (cached) {
+                return { token: cached.token, expiresAt: cached.expiresAt, refreshToken: cached.refreshToken }
+            }
+            return null
+        }
+        // After the grace window the same token is a replay: revoke the grant so
+        // its access tokens stop working and the client must re-authorize.
+        await rotated.destroy()
+        return null
     },
 
     /**

forge/db/migrations/20260825-03-add-refresh-token-rotation-tracking.js

@@ -0,0 +1,36 @@
+/**
+ * Track the previously rotated-out refresh token so rotation can tell a
+ * legitimate concurrent or retried refresh from a replay.
+ *
+ * On each MCP refresh the refresh token is rotated: a new one is issued and the
+ * presented one is recorded here. Presenting the recorded token again within a
+ * short grace window is treated as a retry and returns the current tokens;
+ * presenting it after the window is a replay and revokes the grant.
+ *
+ *   previousRefreshToken          - sha256 of the last rotated-out refresh token.
+ *   previousRefreshTokenRotatedAt - when that rotation happened, used to bound
+ *                                   the grace window.
+ *
+ * Both are null for tokens that do not rotate (for example editor sessions).
+ */
+
+const { DataTypes } = require('sequelize')
+
+module.exports = {
+    up: async (context) => {
+        await context.addColumn('AccessTokens', 'previousRefreshToken', {
+            type: DataTypes.STRING,
+            allowNull: true,
+            defaultValue: null
+        })
+        await context.addColumn('AccessTokens', 'previousRefreshTokenRotatedAt', {
+            type: DataTypes.DATE,
+            allowNull: true,
+            defaultValue: null
+        })
+    },
+    down: async (context) => {
+        await context.removeColumn('AccessTokens', 'previousRefreshToken')
+        await context.removeColumn('AccessTokens', 'previousRefreshTokenRotatedAt')
+    }
+}

forge/db/models/AccessToken.js

@@ -46,6 +46,10 @@ module.exports = {
             }
         },
         refreshTokenExpiresAt: { type: DataTypes.DATE },
+        // Holds the sha256 of the last rotated-out refresh token (set directly,
+        // already hashed) so rotation can distinguish a retry from a replay.
+        previousRefreshToken: { type: DataTypes.STRING },
+        previousRefreshTokenRotatedAt: { type: DataTypes.DATE },
         name: { type: DataTypes.STRING },
         readOnly: { type: DataTypes.BOOLEAN, defaultValue: false, allowNull: false },
         adminOptIn: { type: DataTypes.BOOLEAN, defaultValue: false, allowNull: false }

forge/routes/auth/oauth.js

@@ -598,20 +598,25 @@ module.exports = async function (app) {
                 reply.send(response)
             }
         } else if (grant_type === 'refresh_token') {
-            const existingToken = await app.db.models.AccessToken.byRefreshToken(refresh_token)
-            if (!existingToken) {
-                badRequest(reply, 'invalid_request', 'Invalid refresh_token')
-                return
-            }
-            // Only project/device clients re-check resource ownership on refresh;
-            // ff-plugin and MCP clients are user-scoped.
+            // ff-plugin and MCP clients are user-scoped; only project/device
+            // clients need their resource ownership re-checked on refresh.
             let refreshAuthClient = null
             if (client_id !== 'ff-plugin') {
                 refreshAuthClient = await app.db.controllers.AuthClient.getAuthClient(client_id, client_secret)
                 if (!refreshAuthClient) {
                     return badRequest(reply, 'invalid_request', 'Invalid client_id')
                 }
             }
+            const isMcpClient = refreshAuthClient?.ownerType === 'mcp'
+            const existingToken = await app.db.models.AccessToken.byRefreshToken(refresh_token)
+            // A rotated-out MCP refresh token is no longer the row's current token, so
+            // byRefreshToken cannot find it. refreshToken() resolves the current-or-previous
+            // token, including the grace window and replay detection, so defer to it for
+            // MCP clients rather than rejecting an already-rotated token here.
+            if (!existingToken && !isMcpClient) {
+                badRequest(reply, 'invalid_request', 'Invalid refresh_token')
+                return
+            }
             if (refreshAuthClient && refreshAuthClient.ownerType !== 'mcp') {
                 // Check the owner of the existing session still has access to the project
                 // this client is owned by

test/unit/forge/db/controllers/AccessToken_spec.js

@@ -342,7 +342,7 @@ describe('AccessToken controller', function () {
 
         // Move the row's access and/or refresh token expiry, so the refresh
         // behaviour can be exercised without waiting for real time to pass.
-        async function setRowExpiry (refreshToken, { accessMs, refreshMs } = {}) {
+        async function setRowExpiry (refreshToken, { accessMs, refreshMs, rotatedMs } = {}) {
             const row = await app.db.models.AccessToken.byRefreshToken(refreshToken)
             const updates = {}
             if (accessMs !== undefined) {
@@ -351,6 +351,9 @@ describe('AccessToken controller', function () {
             if (refreshMs !== undefined) {
                 updates.refreshTokenExpiresAt = new Date(Date.now() + refreshMs)
             }
+            if (rotatedMs !== undefined) {
+                updates.previousRefreshTokenRotatedAt = new Date(Date.now() + rotatedMs)
+            }
             await app.db.models.AccessToken.update(updates, { where: { id: row.id } })
         }
 
@@ -388,7 +391,7 @@ describe('AccessToken controller', function () {
             ;(await app.db.models.AccessToken.count()).should.equal(0)
         })
 
-        it('keeps the refresh token stable and slides its expiry on refresh', async function () {
+        it('rotates the refresh token and slides its expiry on refresh', async function () {
             const original = await createToken()
             // Bring the refresh expiry close so the slide back out to the full
             // lifetime is observable rather than a same-millisecond tie.
@@ -397,20 +400,23 @@ describe('AccessToken controller', function () {
 
             const refreshed = await app.db.controllers.AccessToken.refreshToken(original.refreshToken)
             refreshed.token.should.not.equal(original.token)
-            // The refresh token is not rotated.
-            refreshed.refreshToken.should.equal(original.refreshToken)
+            // The refresh token is rotated, and the new access token authenticates.
+            refreshed.refreshToken.should.be.a.String().and.not.equal(original.refreshToken)
+            should.exist(await app.db.controllers.AccessToken.getOrExpire(refreshed.token))
 
-            const after = await app.db.models.AccessToken.byRefreshToken(original.refreshToken)
+            const after = await app.db.models.AccessToken.byRefreshToken(refreshed.refreshToken)
             after.refreshTokenExpiresAt.getTime().should.be.greaterThan(before.refreshTokenExpiresAt.getTime())
         })
 
-        it('reuses the same access token for a repeat refresh within its lifetime', async function () {
+        it('returns the same tokens for a retried refresh within the grace window', async function () {
             const original = await createToken()
             await setRowExpiry(original.refreshToken, { accessMs: -5000 })
 
             const first = await app.db.controllers.AccessToken.refreshToken(original.refreshToken)
-            const second = await app.db.controllers.AccessToken.refreshToken(original.refreshToken)
-            second.token.should.equal(first.token)
+            // The rotated-out token is presented again (a retry or lagging concurrent refresh).
+            const retry = await app.db.controllers.AccessToken.refreshToken(original.refreshToken)
+            retry.token.should.equal(first.token)
+            retry.refreshToken.should.equal(first.refreshToken)
             should.exist(await app.db.controllers.AccessToken.getOrExpire(first.token))
         })
 
@@ -421,6 +427,44 @@ describe('AccessToken controller', function () {
             should.not.exist(await app.db.controllers.AccessToken.refreshToken(original.refreshToken))
             ;(await app.db.models.AccessToken.count()).should.equal(0)
         })
+
+        it('revokes the grant when a rotated-out token is replayed after the grace window', async function () {
+            const original = await createToken()
+            await setRowExpiry(original.refreshToken, { accessMs: -5000 })
+            const first = await app.db.controllers.AccessToken.refreshToken(original.refreshToken)
+
+            // Age the rotation beyond the grace window, then present the old token again.
+            await setRowExpiry(first.refreshToken, { rotatedMs: -120000 })
+            should.not.exist(await app.db.controllers.AccessToken.refreshToken(original.refreshToken))
+            // The whole grant is revoked, so the current access token stops working too.
+            ;(await app.db.models.AccessToken.count()).should.equal(0)
+            should.not.exist(await app.db.controllers.AccessToken.getOrExpire(first.token))
+        })
+
+        it('returns null for an unknown refresh token', async function () {
+            should.not.exist(await app.db.controllers.AccessToken.refreshToken('ffpat_unknown'))
+        })
+
+        it('lets only one of two simultaneous refreshes rotate the token', async function () {
+            const original = await createToken()
+            await setRowExpiry(original.refreshToken, { accessMs: -5000 })
+
+            const [a, b] = await Promise.all([
+                app.db.controllers.AccessToken.refreshToken(original.refreshToken),
+                app.db.controllers.AccessToken.refreshToken(original.refreshToken)
+            ])
+
+            // The grant is neither duplicated nor revoked by the race.
+            ;(await app.db.models.AccessToken.count()).should.equal(1)
+            const results = [a, b].filter(Boolean)
+            results.length.should.be.aboveOrEqual(1)
+            // Every token handed back is the single winner's: it authenticates, and
+            // the refresh tokens returned never diverge (no caller gets a dead token).
+            for (const result of results) {
+                should.exist(await app.db.controllers.AccessToken.getOrExpire(result.token))
+            }
+            new Set(results.map(result => result.refreshToken)).size.should.equal(1)
+        })
     })
 
     describe('getOrExpire', function () {