Skip to content

Commit f72fe43

Browse files
Fix devDependency misplacement, missing REDIS_URL warning, dropped claim fields, and redundant wallet param (#361)
* fix(#342): move @types/jsonwebtoken to devDependencies Type definitions are only needed at compile time; listing them as a production dependency bloats the deployed node_modules for no runtime benefit. * fix(#343): warn at startup when REDIS_URL is unset validateConfig checked JWT_SECRET, DATABASE_URL, STELLAR_RPC_URL, and KEEPER_SECRET_KEY but said nothing about REDIS_URL, so the ThrottlerModule's fallback to redis://localhost:6379 was silent and easy to miss in a real deployment that meant to point at a shared Redis instance. Not made a hard requirement since the localhost fallback is legitimate for local dev -- just logged. * fix(#344): include txHash and createdAt in claim API responses Both columns exist on the Claim model, but ClaimSummary (the actual mapped shape returned by getClaim/getClaimsByWallet) dropped both, so an already-issued payout's on-chain transaction hash was unrecoverable from the API despite being stored in the DB. Added both fields to ClaimSummary, the two service mapping functions, and ClaimResponseDto (createdAt was missing there too). * fix(#345): remove redundant wallet query param from GET /policies/me The wallet query parameter was only ever compared against the JWT's wallet address for authorization -- the JWT wallet was what actually got used to fetch policies. Removed the parameter entirely; the endpoint now always operates on the authenticated wallet, with a BadRequestException if the JWT carries none. Updated the controller spec's call sites and dropped the now-meaningless "another wallet" mismatch test. --------- Co-authored-by: presidoclintonbased-alt <presidoclintonbased-alt@users.noreply.github.com>
1 parent e3574ab commit f72fe43

7 files changed

Lines changed: 49 additions & 40 deletions

File tree

package-lock.json

Lines changed: 5 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
"ioredis": "^5.4.0",
2929
"@prisma/client": "^6.0.0",
3030
"@stellar/stellar-sdk": "^13.0.0",
31-
"@types/jsonwebtoken": "^9.0.10",
3231
"axios": "^1.7.0",
3332
"class-transformer": "^0.5.1",
3433
"class-validator": "^0.14.0",
@@ -42,6 +41,7 @@
4241
"@nestjs/testing": "^11.0.0",
4342
"@types/express": "^5.0.0",
4443
"@types/jest": "^29.5.12",
44+
"@types/jsonwebtoken": "^9.0.10",
4545
"@types/node": "^22.0.0",
4646
"jest": "^29.7.0",
4747
"prisma": "^6.0.0",

src/app.module.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Module } from '@nestjs/common';
1+
import { Logger, Module } from '@nestjs/common';
22
import { ConfigModule, ConfigService } from '@nestjs/config';
33
import { ScheduleModule } from '@nestjs/schedule';
44
import { ThrottlerModule, ThrottlerGuard } from '@nestjs/throttler';
@@ -19,6 +19,15 @@ import { RedisModule } from './redis/redis.module';
1919
*/
2020
function validateConfig(config: Record<string, unknown>) {
2121
const errors: string[] = [];
22+
const logger = new Logger('ConfigValidation');
23+
24+
// #343 — not a hard requirement (a localhost fallback is fine for local
25+
// dev), but the ThrottlerModule storage below falls back to it silently,
26+
// which is easy to miss in a real deployment that meant to point at a
27+
// shared Redis instance.
28+
if (!config['REDIS_URL']) {
29+
logger.warn('REDIS_URL is not set — falling back to redis://localhost:6379');
30+
}
2231

2332
if (!config['JWT_SECRET']) {
2433
errors.push('JWT_SECRET is required');

src/claims/claims.service.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ export interface ClaimSummary {
2020
status: string;
2121
submittedAt: number;
2222
processedAt: number | null;
23+
// #344 — both columns exist on the Claim model but were dropped when
24+
// mapping to this summary shape, so the API response silently lost them.
25+
txHash: string | null;
26+
createdAt: number;
2327
}
2428

2529
/**
@@ -385,6 +389,8 @@ export class ClaimsService {
385389
processedAt: claim.processedAt
386390
? Math.floor(claim.processedAt.getTime() / 1000)
387391
: null,
392+
txHash: claim.txHash,
393+
createdAt: Math.floor(claim.createdAt.getTime() / 1000),
388394
}));
389395

390396
return {
@@ -412,6 +418,8 @@ export class ClaimsService {
412418
processedAt: claim.processedAt
413419
? Math.floor(claim.processedAt.getTime() / 1000)
414420
: null,
421+
txHash: claim.txHash,
422+
createdAt: Math.floor(claim.createdAt.getTime() / 1000),
415423
};
416424
}
417425
}

src/claims/dto/submit-claim.dto.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,7 @@ export class ClaimResponseDto {
5050

5151
@ApiProperty({ description: 'Stellar transaction hash for payout', nullable: true })
5252
txHash: string | null;
53+
54+
@ApiProperty({ description: 'Claim record creation timestamp (Unix seconds)' })
55+
createdAt: number;
5356
}

src/policy/policy.controller.spec.ts

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ describe("PolicyController", () => {
5656
it("should accept valid page and limit parameters", async () => {
5757
mockPolicyService.getUserPolicies.mockResolvedValue(mockPoliciesResponse);
5858

59-
const result = await controller.getMyPolicies(wallet, "2", "50", mockReq);
59+
const result = await controller.getMyPolicies("2", "50", mockReq);
6060

6161
expect(mockPolicyService.getUserPolicies).toHaveBeenCalledWith(
6262
wallet,
@@ -72,7 +72,7 @@ describe("PolicyController", () => {
7272
mockPoliciesResponse,
7373
);
7474

75-
await controller.getMyPolicies(wallet, "0", "20", mockReq);
75+
await controller.getMyPolicies("0", "20", mockReq);
7676

7777
expect(mockPolicyService.getUserPolicies).toHaveBeenCalledWith(
7878
wallet,
@@ -86,7 +86,7 @@ describe("PolicyController", () => {
8686
mockPoliciesResponse,
8787
);
8888

89-
await controller.getMyPolicies(wallet, "-5", "20", mockReq);
89+
await controller.getMyPolicies("-5", "20", mockReq);
9090

9191
expect(mockPolicyService.getUserPolicies).toHaveBeenCalledWith(
9292
wallet,
@@ -100,7 +100,7 @@ describe("PolicyController", () => {
100100
mockPoliciesResponse,
101101
);
102102

103-
await controller.getMyPolicies(wallet, "abc", "20", mockReq);
103+
await controller.getMyPolicies("abc", "20", mockReq);
104104

105105
expect(mockPolicyService.getUserPolicies).toHaveBeenCalledWith(
106106
wallet,
@@ -114,7 +114,7 @@ describe("PolicyController", () => {
114114
mockPoliciesResponse,
115115
);
116116

117-
await controller.getMyPolicies(wallet, "2.7", "20", mockReq);
117+
await controller.getMyPolicies("2.7", "20", mockReq);
118118

119119
expect(mockPolicyService.getUserPolicies).toHaveBeenCalledWith(
120120
wallet,
@@ -131,7 +131,7 @@ describe("PolicyController", () => {
131131
limit: 1,
132132
});
133133

134-
await controller.getMyPolicies(wallet, "1", "-5", mockReq);
134+
await controller.getMyPolicies("1", "-5", mockReq);
135135

136136
expect(mockPolicyService.getUserPolicies).toHaveBeenCalledWith(
137137
wallet,
@@ -145,7 +145,7 @@ describe("PolicyController", () => {
145145
mockPoliciesResponse,
146146
);
147147

148-
await controller.getMyPolicies(wallet, "1", "0", mockReq);
148+
await controller.getMyPolicies("1", "0", mockReq);
149149

150150
expect(mockPolicyService.getUserPolicies).toHaveBeenCalledWith(
151151
wallet,
@@ -160,7 +160,7 @@ describe("PolicyController", () => {
160160
limit: 100,
161161
});
162162

163-
await controller.getMyPolicies(wallet, "1", "999999", mockReq);
163+
await controller.getMyPolicies("1", "999999", mockReq);
164164

165165
expect(mockPolicyService.getUserPolicies).toHaveBeenCalledWith(
166166
wallet,
@@ -175,7 +175,7 @@ describe("PolicyController", () => {
175175
limit: 100,
176176
});
177177

178-
await controller.getMyPolicies(wallet, "1", "500", mockReq);
178+
await controller.getMyPolicies("1", "500", mockReq);
179179

180180
expect(mockPolicyService.getUserPolicies).toHaveBeenCalledWith(
181181
wallet,
@@ -189,7 +189,7 @@ describe("PolicyController", () => {
189189
mockPoliciesResponse,
190190
);
191191

192-
await controller.getMyPolicies(wallet, "1", "xyz", mockReq);
192+
await controller.getMyPolicies("1", "xyz", mockReq);
193193

194194
expect(mockPolicyService.getUserPolicies).toHaveBeenCalledWith(
195195
wallet,
@@ -203,7 +203,7 @@ describe("PolicyController", () => {
203203
mockPoliciesResponse,
204204
);
205205

206-
await controller.getMyPolicies(wallet, "1", "25.9", mockReq);
206+
await controller.getMyPolicies("1", "25.9", mockReq);
207207

208208
expect(mockPolicyService.getUserPolicies).toHaveBeenCalledWith(
209209
wallet,
@@ -220,7 +220,6 @@ describe("PolicyController", () => {
220220
);
221221

222222
await controller.getMyPolicies(
223-
wallet,
224223
undefined as any,
225224
undefined as any,
226225
mockReq,
@@ -238,7 +237,7 @@ describe("PolicyController", () => {
238237
mockPoliciesResponse,
239238
);
240239

241-
await controller.getMyPolicies(wallet, "", "", mockReq);
240+
await controller.getMyPolicies("", "", mockReq);
242241

243242
expect(mockPolicyService.getUserPolicies).toHaveBeenCalledWith(
244243
wallet,
@@ -249,23 +248,14 @@ describe("PolicyController", () => {
249248
});
250249

251250
describe("wallet authorization", () => {
252-
it("should throw ForbiddenException when trying to access another wallet policies", async () => {
253-
const otherWallet =
254-
"GBACDEFGHIJKLMNOPQRSTUVWXYZ1234567890ABCDEFGHIJKLMNOPQRSTUV";
255-
256-
await expect(
257-
controller.getMyPolicies(otherWallet, "1", "20", mockReq),
258-
).rejects.toThrow(ForbiddenException);
259-
});
260-
261251
it("should throw BadRequestException when no wallet is available in request", async () => {
262252
const reqNoWallet = {
263253
user: { walletAddress: null },
264254
wallet: undefined,
265255
} as AuthenticatedRequest;
266256

267257
await expect(
268-
controller.getMyPolicies(wallet, "1", "20", reqNoWallet),
258+
controller.getMyPolicies("1", "20", reqNoWallet),
269259
).rejects.toThrow(BadRequestException);
270260
});
271261
});
@@ -294,7 +284,6 @@ describe("PolicyController", () => {
294284
mockPolicyService.getUserPolicies.mockResolvedValue(response);
295285

296286
const result = await controller.getMyPolicies(
297-
wallet,
298287
"1",
299288
"20",
300289
mockReq,
@@ -315,7 +304,7 @@ describe("PolicyController", () => {
315304
page: 999999999,
316305
});
317306

318-
await controller.getMyPolicies(wallet, "999999999", "1", mockReq);
307+
await controller.getMyPolicies("999999999", "1", mockReq);
319308

320309
// Service should still receive the page number and handle it safely
321310
// (the service would return skip=(page-1)*limit which is safe)
@@ -332,7 +321,7 @@ describe("PolicyController", () => {
332321
limit: 100,
333322
});
334323

335-
await controller.getMyPolicies(wallet, "1", "99999999999999", mockReq);
324+
await controller.getMyPolicies("1", "99999999999999", mockReq);
336325

337326
// Should cap at 100
338327
expect(mockPolicyService.getUserPolicies).toHaveBeenCalledWith(
@@ -350,7 +339,6 @@ describe("PolicyController", () => {
350339
});
351340

352341
await controller.getMyPolicies(
353-
wallet,
354342
"-999999",
355343
"999999999999999",
356344
mockReq,

src/policy/policy.controller.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,11 @@ export class PolicyController {
6060
return { success: true, data: products };
6161
}
6262

63-
/** GET /api/v1/policies/me?wallet=<address>&page=&limit= — get paginated policies for a wallet */
63+
/** GET /api/v1/policies/me?page=&limit= — get paginated policies for the authenticated wallet */
6464
@Get('policies/me')
6565
@UseGuards(JwtAuthGuard)
6666
@ApiBearerAuth()
67-
@ApiOperation({ summary: 'Get paginated policies for a wallet address' })
68-
@ApiQuery({ name: 'wallet', required: true, description: 'Stellar wallet address' })
67+
@ApiOperation({ summary: 'Get paginated policies for the authenticated wallet' })
6968
@ApiQuery({ name: 'page', required: false, description: 'Page number (default 1)', example: 1 })
7069
@ApiQuery({ name: 'limit', required: false, description: 'Items per page, max 100 (default 20)', example: 20 })
7170
@ApiResponse({
@@ -83,22 +82,20 @@ export class PolicyController {
8382
},
8483
})
8584
async getMyPolicies(
86-
@Query('wallet') wallet: string,
8785
@Query('page') page: string = '1',
8886
@Query('limit') limit: string = '20',
8987
@Req() req: AuthenticatedRequest,
9088
) {
89+
// #345 — wallet used to come from a client-supplied query param, checked
90+
// only for authorization against the JWT wallet; the JWT wallet was
91+
// always the one actually used, making the param redundant and confusing.
9192
const authedWallet = req.user?.walletAddress || req.wallet;
9293
if (!authedWallet) {
93-
throw new BadRequestException('wallet query param required');
94-
}
95-
const targetWallet = wallet || authedWallet;
96-
if (targetWallet !== authedWallet) {
97-
throw new ForbiddenException('Cannot fetch policies for another wallet');
94+
throw new BadRequestException('Not authenticated');
9895
}
9996
const pageNum = Math.max(1, parseInt(page, 10) || 1);
10097
const limitNum = Math.min(100, Math.max(1, parseInt(limit, 10) || 20));
101-
const result = await this.policy.getUserPolicies(targetWallet, pageNum, limitNum);
98+
const result = await this.policy.getUserPolicies(authedWallet, pageNum, limitNum);
10299
return { success: true, ...result };
103100
}
104101

0 commit comments

Comments
 (0)