fix(ticketing): store oauth state in redis

Signed-off-by: zebbern <185730623+zebbern@users.noreply.github.com>

Author: zebbern

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');

backend/src/ticketing/__tests__/ticketing.service.spec.ts

@@ -63,6 +63,40 @@ function makeTicketLink(overrides: Record<string, unknown> = {}) {
   };
 }
 
+class MockRedis {
+  private readonly kv = new Map<string, string>();
+  private readonly ttls = new Map<string, number>();
+
+  async set(key: string, value: string, mode?: string, ttl?: number): Promise<string> {
+    this.kv.set(key, value);
+    if (mode === 'EX' && ttl) {
+      this.ttls.set(key, ttl);
+    }
+    return 'OK';
+  }
+
+  async get(key: string): Promise<string | null> {
+    return this.kv.get(key) ?? null;
+  }
+
+  async del(key: string): Promise<number> {
+    const existed = this.kv.has(key);
+    this.kv.delete(key);
+    this.ttls.delete(key);
+    return existed ? 1 : 0;
+  }
+
+  async quit(): Promise<void> {}
+
+  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<typeof createMocks>) {
+function createService(mocks: ReturnType<typeof createMocks>, 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,22 +239,26 @@ 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(
         /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/,
       );
     });
 
-    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();
+    });
   });
 
   // -----------------------------------------------------------------------

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<RedisConfig>('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,

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<string>('JIRA_CLIENT_ID', '');
     this.jiraClientSecret = configService.get<string>('JIRA_CLIENT_SECRET', '');
     this.jiraCallbackUrl = configService.get<string>('JIRA_CALLBACK_URL', '');
   }
 
+  async onModuleDestroy(): Promise<void> {
+    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<OAuthConnectResponse> {
     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<void> {
+    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<OAuthStateCacheEntry | null> {
+    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}`;
+  }
 }