Skip to content

Commit be29312

Browse files
fix: address critical security issues #326 #327 #328 #329 (#355)
- Issue #326: Changed getPoolAvailableBalance to throw error instead of returning Infinity on RPC failure This prevents bypassing liquidity validation when Stellar RPC is down - Issue #327: Migrated OperatorAuthGuard rate limiting from in-memory Map to Redis Rate limit failures now persist across server restarts, making brute-force protection more effective - Issue #328: Reduced JWT token expiry from 7 days to 1 hour Shorter token lifetime reduces the window of unauthorized access if a token is compromised - Issue #329: Updated startup validation to require at least one of ORACLE_OPERATOR_API_KEY or ADMIN_API_KEY Previously only ORACLE_OPERATOR_API_KEY was validated, but the guard falls back to ADMIN_API_KEY Additional changes: - Created RedisModule as a global module for Redis client injection - Updated OperatorAuthGuard.canActivate to be async to support Redis operations Co-authored-by: presidojay1 <305481097+boluwacodes@users.noreply.github.com>
1 parent 8f60094 commit be29312

6 files changed

Lines changed: 81 additions & 37 deletions

File tree

src/app.module.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { StellarModule } from './stellar/stellar.module';
1111
import { PrismaModule } from './prisma/prisma.module';
1212
import { AuthModule } from './auth/auth.module';
1313
import { HealthModule } from './health/health.module';
14+
import { RedisModule } from './redis/redis.module';
1415

1516
/**
1617
* Validate loaded environment configuration at startup.
@@ -73,6 +74,7 @@ function validateConfig(config: Record<string, unknown>) {
7374
storage: new ThrottlerStorageRedisService(config.get<string>('REDIS_URL') || 'redis://localhost:6379'),
7475
}),
7576
}),
77+
RedisModule,
7678
PrismaModule,
7779
StellarModule,
7880
AuthModule,

src/auth/jwt.service.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,16 @@ export interface JwtPayload {
1313
/**
1414
* JwtService — issues and verifies JWTs tied to Stellar wallet addresses.
1515
*
16-
* Tokens are signed with JWT_SECRET from the environment and expire in 7 days.
17-
* The token payload contains the wallet address, which is used to identify
18-
* the authenticated user on protected endpoints.
16+
* Tokens are signed with JWT_SECRET from the environment and expire in 1 hour.
17+
* For a financial application, short-lived tokens reduce the window of
18+
* unauthorized access if a token is compromised. Consider implementing
19+
* refresh tokens for longer sessions.
1920
*/
2021
@Injectable()
2122
export class JwtService {
2223
private readonly logger = new Logger(JwtService.name);
2324
private readonly secret: string;
24-
private readonly tokenExpiry = "7d";
25+
private readonly tokenExpiry = "1h";
2526

2627
constructor(private readonly config: ConfigService) {
2728
const secret = config.get<string>("JWT_SECRET");
@@ -38,13 +39,13 @@ export class JwtService {
3839

3940
/**
4041
* Sign a JWT for the given wallet address.
41-
* Token expires in 7 days.
42+
* Token expires in 1 hour.
4243
*/
4344
sign(walletAddress: string): string {
4445
const payload: JwtPayload = { walletAddress };
4546
const options: jwt.SignOptions = {
4647
algorithm: 'HS256',
47-
expiresIn: '7d',
48+
expiresIn: '1h',
4849
};
4950
const token = jwt.sign(payload, this.secret, options);
5051
this.logger.log(`JWT issued for wallet: ${walletAddress}`);
@@ -54,13 +55,13 @@ export class JwtService {
5455
/**
5556
* Sign a JWT for the given wallet address with explicit role and admin flag.
5657
* Useful for issuing tokens to privileged users (e.g. operators, admins).
57-
* Token expires in 7 days.
58+
* Token expires in 1 hour.
5859
*/
5960
signWithRole(walletAddress: string, role: string, admin = false): string {
6061
const payload: JwtPayload = { walletAddress, role, admin };
6162
const options: jwt.SignOptions = {
6263
algorithm: 'HS256',
63-
expiresIn: '7d',
64+
expiresIn: '1h',
6465
};
6566
const token = jwt.sign(payload, this.secret, options);
6667
this.logger.log(`JWT issued for wallet: ${walletAddress} (role=${role})`);

src/auth/operator-auth.guard.ts

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import { CanActivate, ExecutionContext, Injectable, InternalServerErrorException, UnauthorizedException, HttpException, HttpStatus } from '@nestjs/common';
1+
import { CanActivate, ExecutionContext, Injectable, InternalServerErrorException, UnauthorizedException, HttpException, HttpStatus, Inject } from '@nestjs/common';
22
import { ConfigService } from '@nestjs/config';
33
import { timingSafeEqual } from 'crypto';
44
import { JwtService } from './jwt.service';
55
import { AuthenticatedRequest } from './authenticated-request';
6+
import Redis from 'ioredis';
67

78
interface FailureRecord {
89
count: number;
@@ -11,56 +12,60 @@ interface FailureRecord {
1112

1213
const RATE_LIMIT_WINDOW_MS = 60_000; // 1 minute
1314
const RATE_LIMIT_MAX_FAILURES = 5;
15+
const REDIS_KEY_PREFIX = 'auth:operator:failures:';
1416

1517
@Injectable()
1618
export class OperatorAuthGuard implements CanActivate {
17-
private readonly failureMap = new Map<string, FailureRecord>();
18-
1919
constructor(
2020
private readonly config: ConfigService,
2121
private readonly jwtService: JwtService,
22+
@Inject('REDIS_CLIENT') private readonly redis: Redis,
2223
) {}
2324

24-
canActivate(context: ExecutionContext): boolean {
25+
async canActivate(context: ExecutionContext): Promise<boolean> {
2526
const request = context.switchToHttp().getRequest<AuthenticatedRequest>();
2627
const ip = this.getClientIp(request);
2728

28-
this.checkRateLimit(ip);
29+
await this.checkRateLimit(ip);
2930

3031
if (this.hasValidApiKey(request)) {
31-
this.resetFailures(ip);
32+
await this.resetFailures(ip);
3233
return true;
3334
}
3435

3536
const token = this.getOptionalBearerToken(request);
3637
if (!token) {
37-
this.recordFailure(ip);
38+
await this.recordFailure(ip);
3839
throw new UnauthorizedException('Operator API key or admin bearer token required');
3940
}
4041

4142
try {
4243
const payload = this.jwtService.verify(token);
4344
if (payload.admin !== true && payload.role !== 'admin') {
44-
this.recordFailure(ip);
45+
await this.recordFailure(ip);
4546
throw new UnauthorizedException('Admin bearer token required');
4647
}
47-
this.resetFailures(ip);
48+
await this.resetFailures(ip);
4849
request.wallet = payload.walletAddress;
4950
request.user = payload;
5051
return true;
5152
} catch (err) {
5253
if (err instanceof UnauthorizedException) throw err;
53-
this.recordFailure(ip);
54+
await this.recordFailure(ip);
5455
throw new UnauthorizedException('Invalid bearer token');
5556
}
5657
}
5758

58-
private checkRateLimit(ip: string): void {
59-
const record = this.failureMap.get(ip);
60-
if (!record) return;
59+
private async checkRateLimit(ip: string): Promise<void> {
60+
const key = `${REDIS_KEY_PREFIX}${ip}`;
61+
const recordStr = await this.redis.get(key);
62+
63+
if (!recordStr) return;
6164

65+
const record: FailureRecord = JSON.parse(recordStr);
66+
6267
if (Date.now() > record.resetAt) {
63-
this.failureMap.delete(ip);
68+
await this.redis.del(key);
6469
return;
6570
}
6671

@@ -72,19 +77,30 @@ export class OperatorAuthGuard implements CanActivate {
7277
}
7378
}
7479

75-
private recordFailure(ip: string): void {
80+
private async recordFailure(ip: string): Promise<void> {
81+
const key = `${REDIS_KEY_PREFIX}${ip}`;
7682
const now = Date.now();
77-
const existing = this.failureMap.get(ip);
83+
const recordStr = await this.redis.get(key);
7884

79-
if (!existing || now > existing.resetAt) {
80-
this.failureMap.set(ip, { count: 1, resetAt: now + RATE_LIMIT_WINDOW_MS });
85+
if (!recordStr) {
86+
const newRecord: FailureRecord = { count: 1, resetAt: now + RATE_LIMIT_WINDOW_MS };
87+
await this.redis.set(key, JSON.stringify(newRecord), 'PX', RATE_LIMIT_WINDOW_MS);
8188
} else {
82-
existing.count += 1;
89+
const record: FailureRecord = JSON.parse(recordStr);
90+
if (now > record.resetAt) {
91+
const newRecord: FailureRecord = { count: 1, resetAt: now + RATE_LIMIT_WINDOW_MS };
92+
await this.redis.set(key, JSON.stringify(newRecord), 'PX', RATE_LIMIT_WINDOW_MS);
93+
} else {
94+
record.count += 1;
95+
const ttl = record.resetAt - now;
96+
await this.redis.set(key, JSON.stringify(record), 'PX', Math.max(ttl, 1000));
97+
}
8398
}
8499
}
85100

86-
private resetFailures(ip: string): void {
87-
this.failureMap.delete(ip);
101+
private async resetFailures(ip: string): Promise<void> {
102+
const key = `${REDIS_KEY_PREFIX}${ip}`;
103+
await this.redis.del(key);
88104
}
89105

90106
private getClientIp(request: AuthenticatedRequest): string {

src/main.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,10 @@ async function bootstrap() {
5252
}
5353

5454
const operatorApiKey = configService.get<string>('ORACLE_OPERATOR_API_KEY');
55-
if (!operatorApiKey) {
56-
logger.error('Fatal Error: ORACLE_OPERATOR_API_KEY environment variable is required');
55+
const adminApiKey = configService.get<string>('ADMIN_API_KEY');
56+
57+
if (!operatorApiKey && !adminApiKey) {
58+
logger.error('Fatal Error: At least one of ORACLE_OPERATOR_API_KEY or ADMIN_API_KEY environment variables is required');
5759
process.exit(1);
5860
}
5961

src/policy/policy.service.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ export class PolicyService {
144144
* POLICY_ENGINE_CONTRACT account, which holds the pooled collateral.
145145
* Returns the balance as a number (in XLM-equivalent units, 7-decimal fixed point).
146146
* Returns Infinity when the contract is not configured so tests are unaffected.
147+
* Throws an error when RPC fails to prevent invalid liquidity validation.
147148
*/
148149
async getPoolAvailableBalance(): Promise<number> {
149150
const usdcContract = this.config.get<string>('USDC_CONTRACT');
@@ -159,22 +160,22 @@ export class PolicyService {
159160
const simResult = await this.stellar.simulateInvoke(usdcContract, 'balance', [engineAddress]);
160161

161162
if (StellarRpc.Api.isSimulationError(simResult)) {
162-
this.logger.warn(`Pool balance simulation error: ${simResult.error}`);
163-
return Infinity;
163+
this.logger.error(`Pool balance simulation error: ${simResult.error}`);
164+
throw new Error(`Failed to fetch pool balance: simulation error - ${simResult.error}`);
164165
}
165166

166167
const raw = (simResult as StellarRpc.Api.SimulateTransactionSuccessResponse).result?.retval;
167168
if (!raw) {
168-
this.logger.warn('Pool balance simulation returned no result');
169-
return Infinity;
169+
this.logger.error('Pool balance simulation returned no result');
170+
throw new Error('Failed to fetch pool balance: no result returned');
170171
}
171172

172173
const balance = Number(scValToNative(raw));
173174
this.logger.log(`Pool available balance: ${balance} (7-decimal fixed point)`);
174175
return balance;
175176
} catch (err) {
176-
this.logger.warn(`Failed to fetch pool balance: ${(err as Error).message}`);
177-
return Infinity;
177+
this.logger.error(`Failed to fetch pool balance: ${(err as Error).message}`);
178+
throw new Error(`Failed to fetch pool balance: ${(err as Error).message}`);
178179
}
179180
}
180181

src/redis/redis.module.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { Module, Global } from '@nestjs/common';
2+
import { ConfigService } from '@nestjs/config';
3+
import Redis from 'ioredis';
4+
5+
@Global()
6+
@Module({
7+
providers: [
8+
{
9+
provide: 'REDIS_CLIENT',
10+
useFactory: (configService: ConfigService) => {
11+
const redisUrl = configService.get<string>('REDIS_URL');
12+
if (!redisUrl) {
13+
throw new Error('REDIS_URL environment variable is required');
14+
}
15+
return new Redis(redisUrl);
16+
},
17+
inject: [ConfigService],
18+
},
19+
],
20+
exports: ['REDIS_CLIENT'],
21+
})
22+
export class RedisModule {}

0 commit comments

Comments
 (0)