diff --git a/backend/src/common/redis/redis.tokens.ts b/backend/src/common/redis/redis.tokens.ts index b390fd06..400d3f06 100644 --- a/backend/src/common/redis/redis.tokens.ts +++ b/backend/src/common/redis/redis.tokens.ts @@ -1,2 +1,3 @@ export const PROVISIONING_REDIS = Symbol('PROVISIONING_REDIS'); export const INSTANCE_HEARTBEAT_REDIS = Symbol('INSTANCE_HEARTBEAT_REDIS'); +export const TICKETING_OAUTH_REDIS = Symbol('TICKETING_OAUTH_REDIS'); diff --git a/backend/src/ticketing/__tests__/ticketing.service.spec.ts b/backend/src/ticketing/__tests__/ticketing.service.spec.ts index f51273fa..4f0362be 100644 --- a/backend/src/ticketing/__tests__/ticketing.service.spec.ts +++ b/backend/src/ticketing/__tests__/ticketing.service.spec.ts @@ -63,6 +63,40 @@ function makeTicketLink(overrides: Record = {}) { }; } +class MockRedis { + private readonly kv = new Map(); + private readonly ttls = new Map(); + + async set(key: string, value: string, mode?: string, ttl?: number): Promise { + this.kv.set(key, value); + if (mode === 'EX' && ttl) { + this.ttls.set(key, ttl); + } + return 'OK'; + } + + async get(key: string): Promise { + return this.kv.get(key) ?? null; + } + + async del(key: string): Promise { + const existed = this.kv.has(key); + this.kv.delete(key); + this.ttls.delete(key); + return existed ? 1 : 0; + } + + async quit(): Promise {} + + getTtl(key: string): number | undefined { + return this.ttls.get(key); + } + + has(key: string): boolean { + return this.kv.has(key); + } +} + // --------------------------------------------------------------------------- // Mocks // --------------------------------------------------------------------------- @@ -135,12 +169,13 @@ function createMocks() { return { repoMock, adapterMock, encryptionMock, configMock }; } -function createService(mocks: ReturnType) { +function createService(mocks: ReturnType, redis: MockRedis | null = null) { return new TicketingService( mocks.repoMock as unknown as TicketingRepository, mocks.adapterMock as unknown as JiraAdapter, mocks.encryptionMock as unknown as TokenEncryptionService, mocks.configMock as any, + redis as any, ); } @@ -189,8 +224,12 @@ describe('TicketingService', () => { // ----------------------------------------------------------------------- describe('startOAuthFlow', () => { - it('returns authorization URL with correct scopes', () => { - const result = service.startOAuthFlow(ORG_ID, USER_ID, 'https://app.example.com/callback'); + it('returns authorization URL with correct scopes', async () => { + const result = await service.startOAuthFlow( + ORG_ID, + USER_ID, + 'https://app.example.com/callback', + ); expect(result.authorizationUrl).toContain('https://auth.atlassian.com/authorize'); expect(result.authorizationUrl).toContain('read%3Ajira-work'); @@ -200,8 +239,12 @@ describe('TicketingService', () => { expect(typeof result.state).toBe('string'); }); - it('returns a UUID state parameter', () => { - const result = service.startOAuthFlow(ORG_ID, USER_ID, 'https://app.example.com/callback'); + it('returns a UUID state parameter', async () => { + const result = await service.startOAuthFlow( + ORG_ID, + USER_ID, + 'https://app.example.com/callback', + ); // UUID v4 format check expect(result.state).toMatch( @@ -209,13 +252,13 @@ describe('TicketingService', () => { ); }); - it('throws when Jira OAuth is not configured', () => { + it('throws when Jira OAuth is not configured', async () => { mocks.configMock.get.mockReturnValue(''); const svc = createService(mocks); - expect(() => svc.startOAuthFlow(ORG_ID, USER_ID, 'https://app.example.com/callback')).toThrow( - BadRequestException, - ); + await expect( + svc.startOAuthFlow(ORG_ID, USER_ID, 'https://app.example.com/callback'), + ).rejects.toThrow(BadRequestException); }); }); @@ -232,7 +275,11 @@ describe('TicketingService', () => { it('throws when no accessible Jira cloud resources found', async () => { // Set up a valid state first - const { state } = service.startOAuthFlow(ORG_ID, USER_ID, 'https://app.example.com/callback'); + const { state } = await service.startOAuthFlow( + ORG_ID, + USER_ID, + 'https://app.example.com/callback', + ); // Mock the code exchange const fetchSpy = spyOn(globalThis, 'fetch').mockResolvedValueOnce( @@ -250,6 +297,36 @@ describe('TicketingService', () => { fetchSpy.mockRestore(); }); + + it('consumes OAuth state across service instances through Redis', async () => { + const redis = new MockRedis(); + const startService = createService(mocks, redis); + const callbackService = createService(mocks, redis); + + const { state } = await startService.startOAuthFlow( + ORG_ID, + USER_ID, + 'https://app.example.com/callback', + ); + const redisKey = `sentris:ticketing:oauth-state:${state}`; + + expect(redis.getTtl(redisKey)).toBe(300); + + const fetchSpy = spyOn(globalThis, 'fetch').mockResolvedValueOnce( + new Response( + JSON.stringify({ access_token: 'at', refresh_token: 'rt', expires_in: 3600 }), + { status: 200 }, + ), + ); + + await expect(callbackService.handleOAuthCallback('code-123', state)).resolves.toEqual({ + success: true, + }); + + expect(redis.has(redisKey)).toBe(false); + + fetchSpy.mockRestore(); + }); }); // ----------------------------------------------------------------------- diff --git a/backend/src/ticketing/ticketing.module.ts b/backend/src/ticketing/ticketing.module.ts index bfc4a25c..1100585c 100644 --- a/backend/src/ticketing/ticketing.module.ts +++ b/backend/src/ticketing/ticketing.module.ts @@ -1,5 +1,9 @@ -import { forwardRef, Module } from '@nestjs/common'; +import { forwardRef, Logger, Module } from '@nestjs/common'; +import { ConfigModule, ConfigService } from '@nestjs/config'; +import Redis from 'ioredis'; +import { TICKETING_OAUTH_REDIS } from '../common/redis/redis.tokens'; +import type { RedisConfig } from '../config'; import { DatabaseModule } from '../database/database.module'; import { FindingTriageModule } from '../findings/finding-triage.module'; import { IntegrationsModule } from '../integrations/integrations.module'; @@ -12,9 +16,31 @@ import { JiraWebhookController } from './jira/jira-webhook.controller'; import { JiraWebhookService } from './jira/jira-webhook.service'; @Module({ - imports: [DatabaseModule, IntegrationsModule, forwardRef(() => FindingTriageModule)], + imports: [ + ConfigModule, + DatabaseModule, + IntegrationsModule, + forwardRef(() => FindingTriageModule), + ], controllers: [TicketingController, JiraWebhookController], providers: [ + { + provide: TICKETING_OAUTH_REDIS, + useFactory: (configService: ConfigService) => { + const redis = configService.get('redis'); + const url = redis?.url ?? redis?.terminalUrl; + if (!url) { + new Logger('TicketingModule').warn('Redis URL not set; ticketing OAuth state disabled'); + return null; + } + const client = new Redis(url); + client.on('error', (err) => + new Logger('TicketingModule').warn(`TICKETING_OAUTH_REDIS error: ${err.message}`), + ); + return client; + }, + inject: [ConfigService], + }, TicketingService, TicketingRepository, TicketingListenerService, diff --git a/backend/src/ticketing/ticketing.service.ts b/backend/src/ticketing/ticketing.service.ts index 5e731e35..dd3246a3 100644 --- a/backend/src/ticketing/ticketing.service.ts +++ b/backend/src/ticketing/ticketing.service.ts @@ -1,9 +1,18 @@ -import { BadRequestException, Injectable, Logger, NotFoundException } from '@nestjs/common'; +import { + BadRequestException, + Inject, + Injectable, + Logger, + NotFoundException, + OnModuleDestroy, +} from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { randomUUID } from 'crypto'; import type { SecretEncryptionMaterial } from '@sentris/shared'; import type { TicketingConnectionConfig } from '@sentris/shared'; +import type Redis from 'ioredis'; +import { TICKETING_OAUTH_REDIS } from '../common/redis/redis.tokens'; import { TokenEncryptionService } from '../integrations/token.encryption'; import { TicketingRepository } from './ticketing.repository'; import { JiraAdapter, JiraApiError } from './jira/jira.adapter'; @@ -29,10 +38,7 @@ export interface ConnectionStatus { } // --------------------------------------------------------------------------- -// OAuth state in-memory cache (TTL 5 minutes) -// TODO(S2-4): Migrate OAuth state to Redis for multi-instance support. -// Use SET with EX (5min TTL) for storage, GET+DEL for consumption. -// See PROVISIONING_REDIS token in common/redis/redis.tokens.ts for pattern. +// OAuth state cache (Redis-backed with local fallback, TTL 5 minutes) // --------------------------------------------------------------------------- interface OAuthStateCacheEntry { @@ -42,14 +48,16 @@ interface OAuthStateCacheEntry { expiresAt: number; } +const OAUTH_STATE_TTL_SECONDS = 5 * 60; const OAUTH_STATE_TTL_MS = 5 * 60 * 1000; +const OAUTH_STATE_KEY_PREFIX = 'sentris:ticketing:oauth-state:'; // --------------------------------------------------------------------------- // Service // --------------------------------------------------------------------------- @Injectable() -export class TicketingService { +export class TicketingService implements OnModuleDestroy { private readonly logger = new Logger(TicketingService.name); private readonly jiraClientId: string; private readonly jiraClientSecret: string; @@ -62,12 +70,21 @@ export class TicketingService { private readonly jiraAdapter: JiraAdapter, private readonly encryption: TokenEncryptionService, configService: ConfigService, + @Inject(TICKETING_OAUTH_REDIS) private readonly oauthStateRedis: Redis | null, ) { this.jiraClientId = configService.get('JIRA_CLIENT_ID', ''); this.jiraClientSecret = configService.get('JIRA_CLIENT_SECRET', ''); this.jiraCallbackUrl = configService.get('JIRA_CALLBACK_URL', ''); } + async onModuleDestroy(): Promise { + try { + await this.oauthStateRedis?.quit(); + } catch { + // ignore + } + } + // --------------------------------------------------------------------------- // Connection status // --------------------------------------------------------------------------- @@ -98,20 +115,19 @@ export class TicketingService { // OAuth 2.0 (3LO) // --------------------------------------------------------------------------- - startOAuthFlow( + async startOAuthFlow( organizationId: string, userId: string, redirectUri: string, - ): OAuthConnectResponse { + ): Promise { this.requireJiraConfig(); const state = randomUUID(); - this.oauthStateCache.set(state, { + await this.storeOAuthState(state, { organizationId, userId, redirectUri, expiresAt: Date.now() + OAUTH_STATE_TTL_MS, }); - this.cleanExpiredStates(); const url = new URL('https://auth.atlassian.com/authorize'); url.searchParams.set('audience', 'api.atlassian.com'); @@ -130,12 +146,10 @@ export class TicketingService { async handleOAuthCallback(code: string, state: string): Promise<{ success: boolean }> { this.requireJiraConfig(); - const cached = this.oauthStateCache.get(state); - if (!cached || cached.expiresAt < Date.now()) { - this.oauthStateCache.delete(state ?? ''); + const cached = await this.consumeOAuthState(state); + if (!cached) { throw new BadRequestException('Invalid or expired OAuth state'); } - this.oauthStateCache.delete(state); const { organizationId, userId } = cached; const tokenResponse = await this.exchangeCodeForTokens(code); @@ -483,4 +497,52 @@ export class TicketingService { if (entry.expiresAt < now) this.oauthStateCache.delete(key); } } + + private async storeOAuthState(state: string, entry: OAuthStateCacheEntry): Promise { + if (this.oauthStateRedis) { + try { + await this.oauthStateRedis.set( + this.oauthStateKey(state), + JSON.stringify(entry), + 'EX', + OAUTH_STATE_TTL_SECONDS, + ); + return; + } catch (error) { + this.logger.warn(`Failed to store Jira OAuth state in Redis: ${error}`); + } + } + + this.oauthStateCache.set(state, entry); + this.cleanExpiredStates(); + } + + private async consumeOAuthState(state: string): Promise { + if (this.oauthStateRedis) { + try { + const key = this.oauthStateKey(state); + const raw = await this.oauthStateRedis.get(key); + if (raw) { + await this.oauthStateRedis.del(key); + const parsed = JSON.parse(raw) as OAuthStateCacheEntry; + if (parsed.expiresAt < Date.now()) return null; + return parsed; + } + } catch (error) { + this.logger.warn(`Failed to consume Jira OAuth state from Redis: ${error}`); + } + } + + const cached = this.oauthStateCache.get(state); + if (!cached || cached.expiresAt < Date.now()) { + this.oauthStateCache.delete(state); + return null; + } + this.oauthStateCache.delete(state); + return cached; + } + + private oauthStateKey(state: string): string { + return `${OAUTH_STATE_KEY_PREFIX}${state}`; + } }