mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(oauth): purge tokens whose refresh token expired a month ago
Every refresh writes a new oauth_tokens row and nothing ever deleted the old ones, so an MCP client refreshing hourly grew the table, and every backup with it, without end. A nightly job now deletes tokens whose refresh token expired more than 30 days ago, leaves first, and keeps any row a live chain still points at for replay detection. Also types the backup's per-entry store flag.
This commit is contained in:
@@ -352,6 +352,35 @@ export class OauthTokensRepository extends TrekRepository<OauthTokens> {
|
||||
return rows.map((r) => r.id);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// Retention
|
||||
// ---------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Delete tokens whose refresh token expired before `cutoffIso`, oldest
|
||||
* generation first. A row is only deleted once no other token names it as
|
||||
* parent: `parent_token_id` has no ON DELETE action, and a token still in a
|
||||
* live chain is what replay detection walks. Each pass removes the current
|
||||
* leaves, so a whole expired chain goes over a few passes; `maxPasses`
|
||||
* bounds a chain that is still growing. Returns the number deleted.
|
||||
*/
|
||||
async deleteExpiredBefore(cutoffIso: string, maxPasses = 50): Promise<number> {
|
||||
let total = 0;
|
||||
for (let pass = 0; pass < maxPasses; pass++) {
|
||||
const result = await this.kysely<OauthTokensKyselyDB>()
|
||||
.deleteFrom('oauth_tokens')
|
||||
.where('refresh_token_expires_at', '<', cutoffIso)
|
||||
.where('id', 'not in', (eb) =>
|
||||
eb.selectFrom('oauth_tokens as child').select('child.parent_token_id').where('child.parent_token_id', 'is not', null),
|
||||
)
|
||||
.executeTakeFirst();
|
||||
const deleted = Number(result.numDeletedRows ?? 0);
|
||||
total += deleted;
|
||||
if (deleted === 0) break;
|
||||
}
|
||||
return total;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// OA19 — chain-wide revoke
|
||||
// ---------------------------------------------------------------------
|
||||
|
||||
@@ -301,7 +301,8 @@ export async function createBackup(storage: StorageService, prefix: 'backup' | '
|
||||
// Photos, videos and PDFs are compressed already; deflating them at level
|
||||
// 9 costs most of a backup's CPU time for a few bytes. They are stored.
|
||||
for (const entry of uploadEntries) {
|
||||
archive.file(entry.absPath, { name: entry.name, store: PRECOMPRESSED.test(entry.name) });
|
||||
const data: archiver.ZipEntryData = { name: entry.name, store: PRECOMPRESSED.test(entry.name) };
|
||||
archive.file(entry.absPath, data);
|
||||
}
|
||||
|
||||
// Plugin data — each plugin's own SQLite file and any blobs. This is the ONLY
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
import { Injectable, type OnApplicationBootstrap } from '@nestjs/common';
|
||||
import { InjectRepository } from '@mikro-orm/nestjs';
|
||||
import { OauthTokens } from '../../db/entities/OauthTokens.entity';
|
||||
import type { OauthTokensRepository } from '../../db/repositories/OauthTokens.repository';
|
||||
import { logError, logInfo } from '../audit/audit-log.logger';
|
||||
import { CronRegistrarService } from '../scheduling/cron-registrar.service';
|
||||
|
||||
/** How long a token is kept after its refresh token expired. */
|
||||
export const OAUTH_TOKEN_RETENTION_DAYS = 30;
|
||||
|
||||
/**
|
||||
* Nightly purge of OAuth tokens whose refresh token expired a month ago.
|
||||
* Every refresh writes a new row and nothing ever deleted the old ones, so
|
||||
* an MCP client refreshing hourly grew the table, and with it every backup,
|
||||
* without end. An expired refresh token is refused on its own, so a row this
|
||||
* old serves nothing; the repository keeps any row a live chain still points
|
||||
* at.
|
||||
*/
|
||||
@Injectable()
|
||||
export class OauthTokenRetentionJob implements OnApplicationBootstrap {
|
||||
constructor(
|
||||
@InjectRepository(OauthTokens) private readonly tokens: OauthTokensRepository,
|
||||
private readonly registrar: CronRegistrarService,
|
||||
) {}
|
||||
|
||||
onApplicationBootstrap(): void {
|
||||
if (!this.registrar.isEnabled()) return;
|
||||
this.registrar.register('oauth-token-retention', '30 3 * * *', () => this.tick());
|
||||
}
|
||||
|
||||
async tick(now: Date = new Date()): Promise<void> {
|
||||
try {
|
||||
const cutoff = new Date(now.getTime() - OAUTH_TOKEN_RETENTION_DAYS * 24 * 60 * 60 * 1000).toISOString();
|
||||
const removed = await this.tokens.deleteExpiredBefore(cutoff);
|
||||
if (removed > 0) logInfo(`OAuth token retention: removed ${removed} expired token(s)`);
|
||||
} catch (err: unknown) {
|
||||
logError(`OAuth token retention: ${err instanceof Error ? err.message : String(err)}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -7,6 +7,8 @@ import { clientRegistrationHandler } from '@modelcontextprotocol/sdk/server/auth
|
||||
import { OauthPublicController } from './oauth-public.controller';
|
||||
import { OauthApiController } from './oauth-api.controller';
|
||||
import { OauthService } from './oauth.service';
|
||||
import { OauthTokenRetentionJob } from './oauth-token-retention.job';
|
||||
import { SchedulingModule } from '../scheduling/scheduling.module';
|
||||
import { TrekClientsStore, TrekOAuthProvider } from './oauth-sdk.provider';
|
||||
import { AuditModule } from '../audit/audit.module';
|
||||
import { AddonsModule } from '../addons/addons.module';
|
||||
@@ -52,9 +54,9 @@ import { Users } from '../../db/entities/Users.entity';
|
||||
* `Users` call (Task 1's ruling: "do not add users methods" for this join).
|
||||
*/
|
||||
@Module({
|
||||
imports: [RateLimitModule, AuditModule, AddonsModule, MikroOrmModule.forFeature([OauthClients, OauthTokens, OauthConsents, Users])],
|
||||
imports: [RateLimitModule, AuditModule, AddonsModule, SchedulingModule, MikroOrmModule.forFeature([OauthClients, OauthTokens, OauthConsents, Users])],
|
||||
controllers: [OauthPublicController, OauthApiController],
|
||||
providers: [OauthService, TrekClientsStore, TrekOAuthProvider],
|
||||
providers: [OauthService, TrekClientsStore, TrekOAuthProvider, OauthTokenRetentionJob],
|
||||
exports: [OauthService],
|
||||
})
|
||||
export class OauthModule implements NestModule {
|
||||
|
||||
@@ -529,4 +529,34 @@ describe('OauthTokensRepository', () => {
|
||||
expect(row.scopes).toBe('["kept-scope"]'); // untouched by either read
|
||||
});
|
||||
});
|
||||
|
||||
describe('deleteExpiredBefore (retention)', () => {
|
||||
const longAgo = '2026-01-01T00:00:00.000Z';
|
||||
const cutoff = '2026-06-01T00:00:00.000Z';
|
||||
const ids = () => (testDb.prepare('SELECT id FROM oauth_tokens ORDER BY id').all() as { id: number }[]).map((r) => r.id);
|
||||
|
||||
it('OAUTHTOKREPO-040: deletes a whole expired chain, parents after their children', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
seedClient(user.id, 'ret-1');
|
||||
const root = seedToken({ clientId: 'ret-1', userId: user.id, refreshExpiresAt: longAgo, revokedAt: longAgo });
|
||||
const mid = seedToken({ clientId: 'ret-1', userId: user.id, refreshExpiresAt: longAgo, revokedAt: longAgo, parentId: root });
|
||||
seedToken({ clientId: 'ret-1', userId: user.id, refreshExpiresAt: longAgo, parentId: mid });
|
||||
|
||||
expect(await withRequestContext(t.orm, () => tokens.deleteExpiredBefore(cutoff))).toBe(3);
|
||||
expect(ids()).toEqual([]);
|
||||
});
|
||||
|
||||
it('OAUTHTOKREPO-041: keeps an expired parent while a live token still names it', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
seedClient(user.id, 'ret-2');
|
||||
const root = seedToken({ clientId: 'ret-2', userId: user.id, refreshExpiresAt: longAgo, revokedAt: longAgo });
|
||||
const live = seedToken({ clientId: 'ret-2', userId: user.id, parentId: root });
|
||||
const unrelated = seedToken({ clientId: 'ret-2', userId: user.id, refreshExpiresAt: longAgo });
|
||||
|
||||
expect(await withRequestContext(t.orm, () => tokens.deleteExpiredBefore(cutoff))).toBe(1);
|
||||
expect(ids()).toEqual([root, live].sort((a, b) => a - b));
|
||||
expect(ids()).not.toContain(unrelated);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
const logMock = vi.hoisted(() => ({ logInfo: vi.fn(), logError: vi.fn(), logWarn: vi.fn(), logDebug: vi.fn() }));
|
||||
vi.mock('../../../src/nest/audit/audit-log.logger', () => logMock);
|
||||
|
||||
import { OauthTokenRetentionJob, OAUTH_TOKEN_RETENTION_DAYS } from '../../../src/nest/oauth/oauth-token-retention.job';
|
||||
import type { OauthTokensRepository } from '../../../src/db/repositories/OauthTokens.repository';
|
||||
import type { CronRegistrarService } from '../../../src/nest/scheduling/cron-registrar.service';
|
||||
|
||||
function make(deleteExpiredBefore: (cutoff: string) => Promise<number>, enabled = true) {
|
||||
const registrar = { isEnabled: vi.fn(() => enabled), register: vi.fn() };
|
||||
const job = new OauthTokenRetentionJob({ deleteExpiredBefore } as unknown as OauthTokensRepository, registrar as unknown as CronRegistrarService);
|
||||
return { job, registrar };
|
||||
}
|
||||
|
||||
describe('OauthTokenRetentionJob', () => {
|
||||
it('OAUTHRET-001: registers a nightly job, and nothing under the test gate', () => {
|
||||
const { job, registrar } = make(vi.fn());
|
||||
job.onApplicationBootstrap();
|
||||
expect(registrar.register).toHaveBeenCalledWith('oauth-token-retention', '30 3 * * *', expect.any(Function));
|
||||
|
||||
const gated = make(vi.fn(), false);
|
||||
gated.job.onApplicationBootstrap();
|
||||
expect(gated.registrar.register).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('OAUTHRET-002: deletes what expired more than the retention period ago and logs the count', async () => {
|
||||
const del = vi.fn().mockResolvedValue(4);
|
||||
await make(del).job.tick(new Date('2026-10-07T00:00:00.000Z'));
|
||||
expect(OAUTH_TOKEN_RETENTION_DAYS).toBe(30);
|
||||
expect(del).toHaveBeenCalledWith('2026-09-07T00:00:00.000Z');
|
||||
expect(logMock.logInfo).toHaveBeenCalledWith('OAuth token retention: removed 4 expired token(s)');
|
||||
});
|
||||
|
||||
it('OAUTHRET-003: a failing purge is logged, not thrown', async () => {
|
||||
await make(vi.fn().mockRejectedValue(new Error('database is locked'))).job.tick();
|
||||
expect(logMock.logError).toHaveBeenCalledWith('OAuth token retention: database is locked');
|
||||
});
|
||||
});
|
||||
@@ -44,6 +44,7 @@ import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.se
|
||||
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
||||
import { BudgetService } from '../../../src/nest/budget/budget.service';
|
||||
import type { BudgetItemsRepository } from '../../../src/db/repositories/BudgetItems.repository';
|
||||
import type { BudgetSettlementsRepository } from '../../../src/db/repositories/BudgetSettlements.repository';
|
||||
import type { JourneyShareTokensRepository } from '../../../src/db/repositories/JourneyShareTokens.repository';
|
||||
import type { JourneysRepository } from '../../../src/db/repositories/Journeys.repository';
|
||||
import type { JourneyEntriesRepository } from '../../../src/db/repositories/JourneyEntries.repository';
|
||||
@@ -181,7 +182,7 @@ describe('erasePluginUserData', () => {
|
||||
// call — stubs are enough for those.
|
||||
const slimSvc = new UserCleanupService(
|
||||
(await sharedTestOrm(slim)).em, budget, await createTestUnitOfWork(slim), await createTestUsersRepo(slim),
|
||||
{} as unknown as TripMembersRepository, {} as unknown as BudgetItemsRepository,
|
||||
{} as unknown as TripMembersRepository, {} as unknown as BudgetItemsRepository, {} as unknown as BudgetSettlementsRepository,
|
||||
{} as unknown as JourneyShareTokensRepository, {} as unknown as JourneysRepository,
|
||||
{} as unknown as JourneyEntriesRepository, {} as unknown as JourneyContributorsRepository,
|
||||
// Plan 3h Task 6: a real repository (never a stub cast — `erasePluginUserData`
|
||||
|
||||
Reference in New Issue
Block a user