From 19fe7e75d59792e9c6d6b702ab192fa2d5ccc936 Mon Sep 17 00:00:00 2001 From: Sergey Krylov Date: Tue, 1 Sep 2026 05:55:01 +0300 Subject: [PATCH] fix(tbank): isolate caches by connection generation --- .../brokerage-operations.service.spec.ts | 60 +++++++++ src/tbank/brokerage-operations.service.ts | 15 ++- src/tbank/portfolio.service.spec.ts | 114 +++++++++++++++--- src/tbank/portfolio.service.ts | 73 ++++++----- src/tbank/tbank-access.service.spec.ts | 32 +++++ src/tbank/tbank-access.service.ts | 45 +++++-- src/tbank/tbank-connection.service.spec.ts | 28 +++++ src/tbank/tbank-connection.service.ts | 5 +- src/tbank/tbank.crypto.spec.ts | 3 + src/tbank/upcoming-events.service.spec.ts | 1 + 10 files changed, 317 insertions(+), 59 deletions(-) diff --git a/src/tbank/brokerage-operations.service.spec.ts b/src/tbank/brokerage-operations.service.spec.ts index b22ff5a..6ef7415 100644 --- a/src/tbank/brokerage-operations.service.spec.ts +++ b/src/tbank/brokerage-operations.service.spec.ts @@ -1,5 +1,6 @@ import { BadRequestException, + ConflictException, NotFoundException, ServiceUnavailableException, } from '@nestjs/common'; @@ -25,12 +26,14 @@ describe('BrokerageOperationsService', () => { metadata as never, ); let operationsRequest: unknown; + let generation: string; beforeEach(() => { jest.clearAllMocks(); access.withAccount.mockReset(); tbank.getOperationsByCursor.mockReset(); metadata.resolve.mockReset(); + generation = 'generation-1'; access.withAccount.mockImplementation( ( _userId: string, @@ -39,6 +42,7 @@ describe('BrokerageOperationsService', () => { ) => handler({ token: 'token', + generation, account: { id: 'account', type: 'ACCOUNT_TYPE_TINKOFF', name: '' }, }), ); @@ -254,6 +258,54 @@ describe('BrokerageOperationsService', () => { expect(tbank.getOperationsByCursor).toHaveBeenCalledTimes(1); }); + it('does not reuse a cached ledger page across connection generations', async () => { + const query = { from: '2025-01-01', to: '2025-01-31' }; + + await service.get('user', 'account', query); + await service.get('user', 'account', query); + generation = 'generation-2'; + await service.get('user', 'account', query); + + expect(tbank.getOperationsByCursor).toHaveBeenCalledTimes(2); + }); + + it('does not reuse a cached ledger page after the connection is deleted', async () => { + const query = { from: '2025-01-01', to: '2025-01-31' }; + await service.get('user', 'account', query); + access.withAccount.mockRejectedValueOnce( + new ConflictException({ code: 'TBANK_NOT_CONNECTED' }), + ); + + await expect(service.get('user', 'account', query)).rejects.toEqual( + expect.any(ConflictException), + ); + expect(tbank.getOperationsByCursor).toHaveBeenCalledTimes(1); + }); + + it('keeps a late previous-generation page unavailable to ledger and income collection', async () => { + let finishPrevious: ((value: unknown) => void) | undefined; + const previous = new Promise((resolve) => { + finishPrevious = resolve; + }); + tbank.getOperationsByCursor + .mockReturnValueOnce(previous) + .mockResolvedValueOnce({ + items: [operation('current')], + }); + const range = { from: '2025-01-01', to: '2025-01-31' }; + + const previousRequest = service.collectRanges('user', 'account', [range]); + generation = 'generation-2'; + const current = await service.collectRanges('user', 'account', [range]); + finishPrevious?.({ items: [operation('previous')] }); + await previousRequest; + const reused = await service.collectRanges('user', 'account', [range]); + + expect(current.pages[0][0].items[0].id).toBe('current'); + expect(reused.pages[0][0].items[0].id).toBe('current'); + expect(tbank.getOperationsByCursor).toHaveBeenCalledTimes(2); + }); + it('collects every cursor page and rejects a malformed continuation cursor', async () => { tbank.getOperationsByCursor .mockResolvedValueOnce({ hasNext: true, nextCursor: 'next', items: [] }) @@ -311,3 +363,11 @@ describe('BrokerageOperationsService', () => { expect(tbank.getOperationsByCursor).toHaveBeenCalledTimes(102); }); }); + +function operation(id: string) { + return { + id, + date: { seconds: '1' }, + type: 'OPERATION_TYPE_COUPON', + }; +} diff --git a/src/tbank/brokerage-operations.service.ts b/src/tbank/brokerage-operations.service.ts index e132044..a4fdadc 100644 --- a/src/tbank/brokerage-operations.service.ts +++ b/src/tbank/brokerage-operations.service.ts @@ -40,8 +40,8 @@ export class BrokerageOperationsService { this.requestContext.set({ userId, brokerageAccountId: accountId }); const range = this.range(query); - return this.access.withAccount(userId, accountId, ({ token }) => - this.page(token, userId, accountId, query, range), + return this.access.withAccount(userId, accountId, ({ token, generation }) => + this.page(token, generation, userId, accountId, query, range), ); } @@ -59,7 +59,7 @@ export class BrokerageOperationsService { return this.access.withAccount( userId, accountId, - async ({ token, account }) => { + async ({ token, generation, account }) => { const accountOpenedAt = account.openedDate?.seconds ? new Date(Number(account.openedDate.seconds) * 1000) : undefined; @@ -69,7 +69,7 @@ export class BrokerageOperationsService { accountOpenedAt, pages: await Promise.all( selected.map((range) => - this.collectRange(token, userId, accountId, range), + this.collectRange(token, generation, userId, accountId, range), ), ), }; @@ -79,6 +79,7 @@ export class BrokerageOperationsService { private async collectRange( token: string, + generation: string, userId: string, accountId: string, range: DateOnlyRange, @@ -89,6 +90,7 @@ export class BrokerageOperationsService { do { const page = await this.page( token, + generation, userId, accountId, { @@ -111,12 +113,13 @@ export class BrokerageOperationsService { private async page( token: string, + generation: string, userId: string, accountId: string, query: BrokerageOperationsQueryDto, range: { from: Date; to: Date }, ): Promise { - const key = cacheKey(userId, accountId, query, range); + const key = cacheKey(generation, userId, accountId, query, range); const cached = this.pages.get(key); if (cached) return cached; if (range.from > range.to) { @@ -193,6 +196,7 @@ export class BrokerageOperationsService { } function cacheKey( + generation: string, userId: string, accountId: string, query: BrokerageOperationsQueryDto, @@ -203,6 +207,7 @@ function cacheKey( ? range.to.toISOString() : Math.floor(range.to.valueOf() / freshForMs); return JSON.stringify({ + generation, userId, accountId, from: range.from.toISOString(), diff --git a/src/tbank/portfolio.service.spec.ts b/src/tbank/portfolio.service.spec.ts index 834b49c..2cd56dc 100644 --- a/src/tbank/portfolio.service.spec.ts +++ b/src/tbank/portfolio.service.spec.ts @@ -1,5 +1,6 @@ import { BadRequestException, + ConflictException, ServiceUnavailableException, } from '@nestjs/common'; @@ -9,10 +10,16 @@ jest.mock('./tbank-access.service', () => ({ import { PortfolioService } from './portfolio.service'; import { InstrumentMetadataService } from './instrument-metadata.service'; -import type { TbankAccountContext } from './tbank-access.service'; +import type { + TbankAccountContext, + TbankConnectionContext, +} from './tbank-access.service'; describe('PortfolioService', () => { - const access = { withAccount: jest.fn() }; + const access = { + withConnection: jest.fn(), + withAccountForConnection: jest.fn(), + }; const tbank = { getPortfolio: jest.fn(), getPositions: jest.fn(), @@ -31,18 +38,26 @@ describe('PortfolioService', () => { requestContext as never, metadata, ); + let generation: string; beforeEach(() => { jest.clearAllMocks(); service.invalidate('user'); - access.withAccount.mockImplementation( + generation = 'generation-1'; + access.withConnection.mockImplementation( ( _userId: string, + handler: (context: TbankConnectionContext) => Promise, + ) => handler({ token: 'token', generation }), + ); + access.withAccountForConnection.mockImplementation( + ( + connection: TbankConnectionContext, _accountId: string, handler: (context: TbankAccountContext) => Promise, ) => handler({ - token: 'token', + ...connection, account: { id: 'account', type: 'ACCOUNT_TYPE_TINKOFF', name: '' }, }), ); @@ -52,14 +67,12 @@ describe('PortfolioService', () => { it('resolves account-scoped access before loading upstream portfolio', async () => { await service.get('user', 'account'); + await service.get('user', 'account'); - expect(access.withAccount).toHaveBeenCalledWith( - 'user', - 'account', - expect.any(Function), - ); - expect(tbank.getPortfolio).toHaveBeenCalledWith('token', 'account'); - expect(tbank.getPositions).toHaveBeenCalledWith('token', 'account'); + expect(access.withConnection).toHaveBeenCalledTimes(2); + expect(access.withAccountForConnection).toHaveBeenCalledTimes(1); + expect(tbank.getPortfolio).toHaveBeenCalledTimes(1); + expect(tbank.getPositions).toHaveBeenCalledTimes(1); }); it('normalizes upstream order, exact totals, metadata fallback, and cash', async () => { @@ -174,15 +187,17 @@ describe('PortfolioService', () => { it('uses fresh snapshots, falls back only for temporary errors, and clears them', async () => { await service.get('user', 'account'); await service.get('user', 'account'); - expect(access.withAccount).toHaveBeenCalledTimes(1); + expect(access.withConnection).toHaveBeenCalledTimes(2); + expect(access.withAccountForConnection).toHaveBeenCalledTimes(1); + expect(tbank.getPortfolio).toHaveBeenCalledTimes(1); service.invalidate('user'); await service.get('user', 'account'); - expect(access.withAccount).toHaveBeenCalledTimes(2); + expect(tbank.getPortfolio).toHaveBeenCalledTimes(2); const snapshots = ( service as never as { snapshots: Map } ).snapshots; snapshots.get('user:account')!.storedAt = Date.now() - 30_001; - access.withAccount.mockRejectedValueOnce( + tbank.getPortfolio.mockRejectedValueOnce( new ServiceUnavailableException({ code: 'TBANK_UNAVAILABLE' }), ); await expect(service.get('user', 'account')).resolves.toMatchObject({ @@ -196,11 +211,80 @@ describe('PortfolioService', () => { service as never as { snapshots: Map } ).snapshots; snapshots.get('user:account')!.storedAt = Date.now() - 30_001; - access.withAccount.mockRejectedValueOnce( + access.withAccountForConnection.mockRejectedValueOnce( new BadRequestException({ code: 'INVALID_TBANK_TOKEN' }), ); await expect(service.get('user', 'account')).rejects.toEqual( expect.any(BadRequestException), ); }); + + it('does not reuse a snapshot or fallback across connection generations', async () => { + await service.get('user', 'account'); + const snapshots = ( + service as never as { snapshots: Map } + ).snapshots; + snapshots.get('user:account')!.storedAt = Date.now() - 30_001; + generation = 'generation-2'; + tbank.getPortfolio.mockRejectedValueOnce( + new ServiceUnavailableException({ code: 'TBANK_UNAVAILABLE' }), + ); + + await expect(service.get('user', 'account')).rejects.toEqual( + expect.any(ServiceUnavailableException), + ); + }); + + it('keeps current-generation fallback when account resolution is temporarily unavailable', async () => { + await service.get('user', 'account'); + const snapshots = ( + service as never as { snapshots: Map } + ).snapshots; + snapshots.get('user:account')!.storedAt = Date.now() - 30_001; + access.withAccountForConnection.mockRejectedValueOnce( + new ServiceUnavailableException({ code: 'TBANK_UNAVAILABLE' }), + ); + + await expect(service.get('user', 'account')).resolves.toMatchObject({ + isStale: true, + }); + }); + + it('does not return a snapshot when the current connection is absent', async () => { + await service.get('user', 'account'); + access.withConnection.mockRejectedValueOnce( + new ConflictException({ code: 'TBANK_NOT_CONNECTED' }), + ); + + await expect(service.get('user', 'account')).rejects.toEqual( + expect.any(ConflictException), + ); + }); + + it('does not expose a late previous-generation portfolio snapshot', async () => { + let finishPrevious: ((value: { positions: never[] }) => void) | undefined; + const previous = new Promise<{ positions: never[] }>((resolve) => { + finishPrevious = resolve; + }); + tbank.getPortfolio + .mockReturnValueOnce(previous) + .mockResolvedValueOnce({ positions: [] }) + .mockResolvedValueOnce({ positions: [] }); + tbank.getPositions + .mockResolvedValueOnce({ money: [{ currency: 'RUB', units: 1 }] }) + .mockResolvedValueOnce({ money: [{ currency: 'USD', units: 2 }] }) + .mockResolvedValueOnce({ money: [{ currency: 'USD', units: 3 }] }); + + const previousRequest = service.get('user', 'account'); + generation = 'generation-2'; + const current = await service.get('user', 'account'); + finishPrevious?.({ positions: [] }); + await previousRequest; + const afterLateCompletion = await service.get('user', 'account'); + + expect(current.cashBalances).toEqual([{ currency: 'USD', value: '2' }]); + expect(afterLateCompletion.cashBalances).toEqual([ + { currency: 'USD', value: '3' }, + ]); + }); }); diff --git a/src/tbank/portfolio.service.ts b/src/tbank/portfolio.service.ts index 4223371..25c4d69 100644 --- a/src/tbank/portfolio.service.ts +++ b/src/tbank/portfolio.service.ts @@ -27,7 +27,11 @@ const types: Record = { dfa: 'DIGITAL_FINANCIAL_ASSET', }; -type CachedSnapshot = { snapshot: PortfolioDto; storedAt: number }; +type CachedSnapshot = { + generation: string; + snapshot: PortfolioDto; + storedAt: number; +}; @Injectable() export class PortfolioService { @@ -43,37 +47,46 @@ export class PortfolioService { async get(userId: string, accountId: string): Promise { this.requestContext.set({ userId, brokerageAccountId: accountId }); const key = `${userId}:${accountId}`; - const cached = this.snapshots.get(key); - if (cached && Date.now() - cached.storedAt <= freshForMs) - return cached.snapshot; - try { - const snapshot = await this.access.withAccount( - userId, - accountId, - async ({ token }) => { - const [portfolio, positions] = await Promise.all([ - this.tbank.getPortfolio(token, accountId), - this.tbank.getPositions(token, accountId), - ]); - return this.normalize( - token, - portfolio.positions ?? [], - positions.money ?? [], - ); - }, - ); - this.snapshots.set(key, { snapshot, storedAt: Date.now() }); - return snapshot; - } catch (error) { + return this.access.withConnection(userId, async ({ token, generation }) => { + const cached = this.snapshots.get(key); if ( - error instanceof ServiceUnavailableException && - cached && - Date.now() - cached.storedAt <= fallbackForMs - ) { - return { ...cached.snapshot, isStale: true }; + cached?.generation === generation && + Date.now() - cached.storedAt <= freshForMs + ) + return cached.snapshot; + try { + const snapshot = await this.access.withAccountForConnection( + { token, generation }, + accountId, + async () => { + const [portfolio, positions] = await Promise.all([ + this.tbank.getPortfolio(token, accountId), + this.tbank.getPositions(token, accountId), + ]); + return this.normalize( + token, + portfolio.positions ?? [], + positions.money ?? [], + ); + }, + ); + this.snapshots.set(key, { + generation, + snapshot, + storedAt: Date.now(), + }); + return snapshot; + } catch (error) { + if ( + error instanceof ServiceUnavailableException && + cached?.generation === generation && + Date.now() - cached.storedAt <= fallbackForMs + ) { + return { ...cached.snapshot, isStale: true }; + } + throw error; } - throw error; - } + }); } invalidate(userId: string): void { diff --git a/src/tbank/tbank-access.service.spec.ts b/src/tbank/tbank-access.service.spec.ts index 322a7a6..b8e7a15 100644 --- a/src/tbank/tbank-access.service.spec.ts +++ b/src/tbank/tbank-access.service.spec.ts @@ -73,6 +73,16 @@ describe('TbankAccessService', () => { expect(tbank.getOpenAccounts).not.toHaveBeenCalled(); }); + it('resolves the persisted connection generation without an upstream account call', async () => { + const context = await service.withConnection('user', (value) => value); + + expect(context).toEqual({ + token: 'plain-token', + generation: expect.any(String) as string, + }); + expect(tbank.getOpenAccounts).not.toHaveBeenCalled(); + }); + it('translates invalid tokens and preserves unavailable upstream errors', async () => { tbank.getOpenAccounts.mockRejectedValueOnce( new Error('INVALID_TBANK_TOKEN'), @@ -94,6 +104,7 @@ describe('TbankAccessService', () => { const handler = jest.fn((context: TbankAccountContext) => Promise.resolve({ resolvedToken: context.token, + generation: context.generation, accountId: context.account.id, }), ); @@ -102,10 +113,31 @@ describe('TbankAccessService', () => { service.withAccount('user', 'account', handler), ).resolves.toEqual({ resolvedToken: 'plain-token', + generation: expect.any(String) as string, accountId: 'account', }); }); + it('derives a stable opaque generation from each persisted encrypted payload', async () => { + const generation = () => + service.withAccount('user', 'account', ({ generation: value }) => + Promise.resolve(value), + ); + prisma.tbankConnection.findUnique + .mockResolvedValueOnce({ encryptedToken: 'encrypted:first-nonce' }) + .mockResolvedValueOnce({ encryptedToken: 'encrypted:first-nonce' }) + .mockResolvedValueOnce({ encryptedToken: 'encrypted:second-nonce' }); + + const first = await generation(); + const sameRecord = await generation(); + const replaced = await generation(); + + expect(sameRecord).toBe(first); + expect(replaced).not.toBe(first); + expect(first).not.toContain('plain-token'); + expect(first).not.toContain('encrypted:first-nonce'); + }); + it('rejects inaccessible accounts before running the handler', async () => { const handler = jest.fn(); diff --git a/src/tbank/tbank-access.service.ts b/src/tbank/tbank-access.service.ts index a432380..4fa72a3 100644 --- a/src/tbank/tbank-access.service.ts +++ b/src/tbank/tbank-access.service.ts @@ -3,6 +3,7 @@ import { Injectable, NotFoundException, } from '@nestjs/common'; +import { createHash } from 'node:crypto'; import { PrismaService } from '../prisma/prisma.service'; import { BrokerageAccountDto } from './brokerage-account.dto'; import { @@ -12,8 +13,12 @@ import { import { TbankClient, TbankAccount } from './tbank.client'; import { TbankTokenService } from './tbank-token.service'; -export type TbankAccountContext = { +export type TbankConnectionContext = { token: string; + generation: string; +}; + +export type TbankAccountContext = TbankConnectionContext & { account: TbankAccount; }; @@ -33,13 +38,14 @@ export class TbankAccessService { userId: string, handler: (context: { token: string; + generation: string; accounts: BrokerageAccountDto[]; }) => Promise | T, ): Promise { - const { token } = await this.connection(userId); - return this.tokens.translateInvalidToken(async () => + return this.withConnection(userId, async ({ token, generation }) => handler({ token, + generation, accounts: supportedTbankAccounts( await this.tbank.getOpenAccounts(token), ), @@ -47,28 +53,53 @@ export class TbankAccessService { ); } + async withConnection( + userId: string, + handler: (context: TbankConnectionContext) => Promise | T, + ): Promise { + const connection = await this.connection(userId); + return this.tokens.translateInvalidToken(async () => handler(connection)); + } + async withAccount( userId: string, accountId: string, handler: (context: TbankAccountContext) => Promise, ): Promise { - const { token } = await this.connection(userId); + return this.withConnection(userId, (connection) => + this.withAccountForConnection(connection, accountId, handler), + ); + } + + async withAccountForConnection( + connection: TbankConnectionContext, + accountId: string, + handler: (context: TbankAccountContext) => Promise, + ): Promise { + const { token, generation } = connection; return this.tokens.translateInvalidToken(async () => { const account = accessibleTbankAccount( await this.tbank.getOpenAccounts(token), accountId, ); if (!account) throw new NotFoundException(); - return handler({ token, account }); + return handler({ token, generation, account }); }); } - private async connection(userId: string): Promise<{ token: string }> { + private async connection( + userId: string, + ): Promise<{ token: string; generation: string }> { const connection = await this.prisma.tbankConnection.findUnique({ where: { userId }, }); if (!connection) throw new ConflictException({ code: 'TBANK_NOT_CONNECTED' }); - return { token: this.tokens.decrypt(connection.encryptedToken) }; + return { + token: this.tokens.decrypt(connection.encryptedToken), + generation: createHash('sha256') + .update(connection.encryptedToken) + .digest('base64url'), + }; } } diff --git a/src/tbank/tbank-connection.service.spec.ts b/src/tbank/tbank-connection.service.spec.ts index 74c25ad..5f0a0c9 100644 --- a/src/tbank/tbank-connection.service.spec.ts +++ b/src/tbank/tbank-connection.service.spec.ts @@ -83,6 +83,34 @@ describe('TbankConnectionService', () => { expect(portfolio.invalidate).toHaveBeenCalledWith('user-1'); }); + it('re-encrypts the same plaintext token for every successful replacement', async () => { + const connectedAt = new Date('2026-08-25T09:00:00.000Z'); + prisma.tbankConnection.upsert.mockResolvedValue({ connectedAt }); + tokens.encrypt + .mockReturnValueOnce('encrypted:first-nonce') + .mockReturnValueOnce('encrypted:second-nonce'); + + await service.put('user-1', 'same-token'); + await service.put('user-1', 'same-token'); + + expect(prisma.tbankConnection.upsert).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + update: expect.objectContaining({ + encryptedToken: 'encrypted:first-nonce', + }) as object, + }), + ); + expect(prisma.tbankConnection.upsert).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + update: expect.objectContaining({ + encryptedToken: 'encrypted:second-nonce', + }) as object, + }), + ); + }); + it('deletes a connection idempotently', async () => { await service.delete('user-1'); diff --git a/src/tbank/tbank-connection.service.ts b/src/tbank/tbank-connection.service.ts index 0598cc3..1210383 100644 --- a/src/tbank/tbank-connection.service.ts +++ b/src/tbank/tbank-connection.service.ts @@ -31,15 +31,16 @@ export class TbankConnectionService { await this.tokens.translateInvalidToken(() => this.tbank.validateToken(token), ); + const encryptedToken = this.tokens.encrypt(token); const connection = await this.prisma.tbankConnection.upsert({ where: { userId }, create: { userId, - encryptedToken: this.tokens.encrypt(token), + encryptedToken, connectedAt: new Date(), }, update: { - encryptedToken: this.tokens.encrypt(token), + encryptedToken, connectedAt: new Date(), }, }); diff --git a/src/tbank/tbank.crypto.spec.ts b/src/tbank/tbank.crypto.spec.ts index 59fdb35..111c1af 100644 --- a/src/tbank/tbank.crypto.spec.ts +++ b/src/tbank/tbank.crypto.spec.ts @@ -6,9 +6,12 @@ describe('TbankCrypto', () => { 'MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=', ); const encrypted = crypto.encrypt('secret-token'); + const replacement = crypto.encrypt('secret-token'); expect(encrypted).not.toContain('secret-token'); + expect(replacement).not.toBe(encrypted); expect(crypto.decrypt(encrypted)).toBe('secret-token'); + expect(crypto.decrypt(replacement)).toBe('secret-token'); expect(() => TbankCrypto.fromBase64('invalid')).toThrow( 'Invalid token encryption key', ); diff --git a/src/tbank/upcoming-events.service.spec.ts b/src/tbank/upcoming-events.service.spec.ts index 3a8e49b..bb7c37f 100644 --- a/src/tbank/upcoming-events.service.spec.ts +++ b/src/tbank/upcoming-events.service.spec.ts @@ -41,6 +41,7 @@ describe('UpcomingEventsService', () => { ) => handler({ token: 'token', + generation: 'generation-1', account: { id: 'account', type: 'ACCOUNT_TYPE_TINKOFF', name: '' }, }), );