From 30f93bc673846107456c8fb3276ca74919ccb022 Mon Sep 17 00:00:00 2001 From: Guido Percu Date: Wed, 2 Apr 2025 14:19:10 -0400 Subject: [PATCH 01/11] Add CORS configuration to API Gateway and backend --- backend/src/iac/backend-stack.ts | 26 ++++++++++++++++++++++++++ backend/src/main.ts | 5 +---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/backend/src/iac/backend-stack.ts b/backend/src/iac/backend-stack.ts index d19f6a55..057d47f9 100644 --- a/backend/src/iac/backend-stack.ts +++ b/backend/src/iac/backend-stack.ts @@ -452,6 +452,32 @@ export class BackendStack extends cdk.Stack { reportStatusResource.addCorsPreflight(corsOptions); docsResource.addCorsPreflight(corsOptions); + // Configure Gateway Responses to add CORS headers to error responses + const gatewayResponseTypes = [ + apigateway.ResponseType.UNAUTHORIZED, + apigateway.ResponseType.ACCESS_DENIED, + apigateway.ResponseType.DEFAULT_4XX, + apigateway.ResponseType.DEFAULT_5XX, + apigateway.ResponseType.RESOURCE_NOT_FOUND, + apigateway.ResponseType.MISSING_AUTHENTICATION_TOKEN, + apigateway.ResponseType.INVALID_API_KEY, + apigateway.ResponseType.THROTTLED, + apigateway.ResponseType.INTEGRATION_FAILURE, + apigateway.ResponseType.INTEGRATION_TIMEOUT, + ]; + + gatewayResponseTypes.forEach((responseType) => { + new apigateway.CfnGatewayResponse(this, `${appName}GatewayResponse${responseType}-${props.environment}`, { + restApiId: api.restApiId, + responseType: responseType.toString(), + responseParameters: { + 'gatewayresponse.header.Access-Control-Allow-Origin': "'*'", + 'gatewayresponse.header.Access-Control-Allow-Headers': "'Content-Type,Authorization,X-Amz-Date,X-Api-Key'", + 'gatewayresponse.header.Access-Control-Allow-Methods': "'GET,POST,PUT,PATCH,DELETE,OPTIONS'" + }, + }); + }); + // Create API Gateway execution role with required permissions new iam.Role(this, `${appName}APIGatewayRole-${props.environment}`, { assumedBy: new iam.ServicePrincipal('apigateway.amazonaws.com'), diff --git a/backend/src/main.ts b/backend/src/main.ts index 2687ebcb..e4af91ad 100644 --- a/backend/src/main.ts +++ b/backend/src/main.ts @@ -13,10 +13,7 @@ async function bootstrap() { // Enable CORS app.enableCors({ origin: [ - 'http://localhost:5173', // Vite default dev server - 'http://localhost:3000', - 'http://localhost:4173', // Vite preview - ...(process.env.FRONTEND_URL ? [process.env.FRONTEND_URL] : []), + '*', // Vite default dev server ], methods: 'GET,HEAD,PUT,PATCH,POST,DELETE,OPTIONS', credentials: true, From feffd6d17db7ce4507503cf52c400c9b89257c37 Mon Sep 17 00:00:00 2001 From: Guido Percu Date: Wed, 2 Apr 2025 14:19:25 -0400 Subject: [PATCH 02/11] Add CORS configuration to API Gateway and backend --- backend/src/iac/backend-stack.ts | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/backend/src/iac/backend-stack.ts b/backend/src/iac/backend-stack.ts index 057d47f9..eb8edee8 100644 --- a/backend/src/iac/backend-stack.ts +++ b/backend/src/iac/backend-stack.ts @@ -466,16 +466,22 @@ export class BackendStack extends cdk.Stack { apigateway.ResponseType.INTEGRATION_TIMEOUT, ]; - gatewayResponseTypes.forEach((responseType) => { - new apigateway.CfnGatewayResponse(this, `${appName}GatewayResponse${responseType}-${props.environment}`, { - restApiId: api.restApiId, - responseType: responseType.toString(), - responseParameters: { - 'gatewayresponse.header.Access-Control-Allow-Origin': "'*'", - 'gatewayresponse.header.Access-Control-Allow-Headers': "'Content-Type,Authorization,X-Amz-Date,X-Api-Key'", - 'gatewayresponse.header.Access-Control-Allow-Methods': "'GET,POST,PUT,PATCH,DELETE,OPTIONS'" + gatewayResponseTypes.forEach(responseType => { + new apigateway.CfnGatewayResponse( + this, + `${appName}GatewayResponse${responseType}-${props.environment}`, + { + restApiId: api.restApiId, + responseType: responseType.toString(), + responseParameters: { + 'gatewayresponse.header.Access-Control-Allow-Origin': "'*'", + 'gatewayresponse.header.Access-Control-Allow-Headers': + "'Content-Type,Authorization,X-Amz-Date,X-Api-Key'", + 'gatewayresponse.header.Access-Control-Allow-Methods': + "'GET,POST,PUT,PATCH,DELETE,OPTIONS'", + }, }, - }); + ); }); // Create API Gateway execution role with required permissions From f5ebd1a625e1c8500ffdc56796478dca0358a6ee Mon Sep 17 00:00:00 2001 From: Guido Percu Date: Wed, 2 Apr 2025 15:05:36 -0400 Subject: [PATCH 03/11] Fix CORS --- backend/src/iac/backend-stack.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/src/iac/backend-stack.ts b/backend/src/iac/backend-stack.ts index eb8edee8..463e8524 100644 --- a/backend/src/iac/backend-stack.ts +++ b/backend/src/iac/backend-stack.ts @@ -466,13 +466,13 @@ export class BackendStack extends cdk.Stack { apigateway.ResponseType.INTEGRATION_TIMEOUT, ]; - gatewayResponseTypes.forEach(responseType => { + gatewayResponseTypes.forEach((responseType) => { new apigateway.CfnGatewayResponse( this, - `${appName}GatewayResponse${responseType}-${props.environment}`, + `${appName}GatewayResponse-${responseType.responseType.toString()}-${props.environment}`, { restApiId: api.restApiId, - responseType: responseType.toString(), + responseType: responseType.responseType.toString(), responseParameters: { 'gatewayresponse.header.Access-Control-Allow-Origin': "'*'", 'gatewayresponse.header.Access-Control-Allow-Headers': From cac2c7c9d64f94be30554442fabf2dbd9784338d Mon Sep 17 00:00:00 2001 From: Guido Percu Date: Wed, 2 Apr 2025 16:16:25 -0400 Subject: [PATCH 04/11] Add new CORS configuration related to preflight for android --- backend/src/iac/backend-stack.ts | 32 +++++++++++++++++++++++++------- backend/src/main.ts | 8 ++++++-- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/backend/src/iac/backend-stack.ts b/backend/src/iac/backend-stack.ts index 463e8524..3aea8706 100644 --- a/backend/src/iac/backend-stack.ts +++ b/backend/src/iac/backend-stack.ts @@ -445,12 +445,30 @@ export class BackendStack extends cdk.Stack { // Add CORS to all resources api.root.addCorsPreflight(corsOptions); - apiResource.addCorsPreflight(corsOptions); - reportsResource.addCorsPreflight(corsOptions); - latestResource.addCorsPreflight(corsOptions); - reportIdResource.addCorsPreflight(corsOptions); - reportStatusResource.addCorsPreflight(corsOptions); - docsResource.addCorsPreflight(corsOptions); + apiResource.addCorsPreflight({ + ...corsOptions, + allowCredentials: false // This is crucial - make sure OPTIONS requests don't require credentials + }); + reportsResource.addCorsPreflight({ + ...corsOptions, + allowCredentials: false + }); + latestResource.addCorsPreflight({ + ...corsOptions, + allowCredentials: false + }); + reportIdResource.addCorsPreflight({ + ...corsOptions, + allowCredentials: false + }); + reportStatusResource.addCorsPreflight({ + ...corsOptions, + allowCredentials: false + }); + docsResource.addCorsPreflight({ + ...corsOptions, + allowCredentials: false + }); // Configure Gateway Responses to add CORS headers to error responses const gatewayResponseTypes = [ @@ -466,7 +484,7 @@ export class BackendStack extends cdk.Stack { apigateway.ResponseType.INTEGRATION_TIMEOUT, ]; - gatewayResponseTypes.forEach((responseType) => { + gatewayResponseTypes.forEach(responseType => { new apigateway.CfnGatewayResponse( this, `${appName}GatewayResponse-${responseType.responseType.toString()}-${props.environment}`, diff --git a/backend/src/main.ts b/backend/src/main.ts index e4af91ad..058457c2 100644 --- a/backend/src/main.ts +++ b/backend/src/main.ts @@ -13,8 +13,12 @@ async function bootstrap() { // Enable CORS app.enableCors({ origin: [ - '*', // Vite default dev server - ], + 'http://localhost:5173', + 'http://localhost:3000', + 'http://localhost:4173', + 'https://localhost', // Add this for Capacitor + ...(process.env.FRONTEND_URL ? [process.env.FRONTEND_URL] : []), + ], methods: 'GET,HEAD,PUT,PATCH,POST,DELETE,OPTIONS', credentials: true, }); From a7c2f85d2a4622c996297c8161b98e5b0750a9ef Mon Sep 17 00:00:00 2001 From: Guido Percu Date: Wed, 2 Apr 2025 16:16:52 -0400 Subject: [PATCH 05/11] Add new CORS configuration related to preflight for android --- backend/src/iac/backend-stack.ts | 12 ++++++------ backend/src/main.ts | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/backend/src/iac/backend-stack.ts b/backend/src/iac/backend-stack.ts index 3aea8706..41ae0e17 100644 --- a/backend/src/iac/backend-stack.ts +++ b/backend/src/iac/backend-stack.ts @@ -447,27 +447,27 @@ export class BackendStack extends cdk.Stack { api.root.addCorsPreflight(corsOptions); apiResource.addCorsPreflight({ ...corsOptions, - allowCredentials: false // This is crucial - make sure OPTIONS requests don't require credentials + allowCredentials: false, // This is crucial - make sure OPTIONS requests don't require credentials }); reportsResource.addCorsPreflight({ ...corsOptions, - allowCredentials: false + allowCredentials: false, }); latestResource.addCorsPreflight({ ...corsOptions, - allowCredentials: false + allowCredentials: false, }); reportIdResource.addCorsPreflight({ ...corsOptions, - allowCredentials: false + allowCredentials: false, }); reportStatusResource.addCorsPreflight({ ...corsOptions, - allowCredentials: false + allowCredentials: false, }); docsResource.addCorsPreflight({ ...corsOptions, - allowCredentials: false + allowCredentials: false, }); // Configure Gateway Responses to add CORS headers to error responses diff --git a/backend/src/main.ts b/backend/src/main.ts index 058457c2..270af094 100644 --- a/backend/src/main.ts +++ b/backend/src/main.ts @@ -13,12 +13,12 @@ async function bootstrap() { // Enable CORS app.enableCors({ origin: [ - 'http://localhost:5173', - 'http://localhost:3000', - 'http://localhost:4173', - 'https://localhost', // Add this for Capacitor - ...(process.env.FRONTEND_URL ? [process.env.FRONTEND_URL] : []), - ], + 'http://localhost:5173', + 'http://localhost:3000', + 'http://localhost:4173', + 'https://localhost', // Add this for Capacitor + ...(process.env.FRONTEND_URL ? [process.env.FRONTEND_URL] : []), + ], methods: 'GET,HEAD,PUT,PATCH,POST,DELETE,OPTIONS', credentials: true, }); From a6d7b6fbc3323f6b004e9472c7388a4714a87832 Mon Sep 17 00:00:00 2001 From: Guido Percu Date: Wed, 2 Apr 2025 16:54:41 -0400 Subject: [PATCH 06/11] Update resource policy --- backend/src/iac/update-api-policy.js | 49 ++++++++++++++++------------ 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/backend/src/iac/update-api-policy.js b/backend/src/iac/update-api-policy.js index 35f59502..510d6ad8 100755 --- a/backend/src/iac/update-api-policy.js +++ b/backend/src/iac/update-api-policy.js @@ -62,29 +62,36 @@ async function main() { const policy = { Version: '2012-10-17', Statement: [ - // Allow authenticated Cognito users { - Effect: 'Allow', - Principal: '*', - Action: 'execute-api:Invoke', - Resource: `arn:aws:execute-api:${REGION}:*:${api.id}/*/*`, - Condition: { - StringEquals: { - 'cognito-identity.amazonaws.com:aud': cognitoUserPoolId + "Version": "2012-10-17", + "Statement": [ + // Allow OPTIONS requests + { + "Effect": "Allow", + "Principal": "*", + "Action": "execute-api:Invoke", + "Resource": "arn:aws:execute-api:us-east-1:*:xhvwo6wp66/*/OPTIONS/*" + }, + { + // Allow all other requests - authentication will be handled by Cognito + "Effect": "Allow", + "Principal": "*", + "Action": "execute-api:Invoke", + "Resource": "arn:aws:execute-api:us-east-1:*:xhvwo6wp66/*/*" + }, + { + // Deny non-HTTPS requests + "Effect": "Deny", + "Principal": "*", + "Action": "execute-api:Invoke", + "Resource": "arn:aws:execute-api:us-east-1:*:xhvwo6wp66/*/*", + "Condition": { + "Bool": { + "aws:SecureTransport": "false" + } + } } - } - }, - // Deny non-HTTPS requests - { - Effect: 'Deny', - Principal: '*', - Action: 'execute-api:Invoke', - Resource: `arn:aws:execute-api:${REGION}:*:${api.id}/*/*`, - Condition: { - Bool: { - 'aws:SecureTransport': 'false' - } - } + ] } ] }; From 2deaa43c8c169eb2ae9bcdcd3d9a74975b74f3ab Mon Sep 17 00:00:00 2001 From: Guido Percu Date: Thu, 3 Apr 2025 14:50:25 -0400 Subject: [PATCH 07/11] Improve error handling --- backend/src/reports/reports.service.ts | 194 +++++++++++++++++-------- 1 file changed, 137 insertions(+), 57 deletions(-) diff --git a/backend/src/reports/reports.service.ts b/backend/src/reports/reports.service.ts index 7f7ae201..9106df1e 100644 --- a/backend/src/reports/reports.service.ts +++ b/backend/src/reports/reports.service.ts @@ -1,10 +1,11 @@ -import { Injectable, NotFoundException } from '@nestjs/common'; +import { Injectable, NotFoundException, InternalServerErrorException, Logger } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { DynamoDBClient, ScanCommand, GetItemCommand, UpdateItemCommand, + DynamoDBServiceException, } from '@aws-sdk/client-dynamodb'; import { marshall, unmarshall } from '@aws-sdk/util-dynamodb'; import { Report } from './models/report.model'; @@ -15,27 +16,26 @@ import { UpdateReportStatusDto } from './dto/update-report-status.dto'; export class ReportsService { private readonly dynamoClient: DynamoDBClient; private readonly tableName: string; + private readonly logger = new Logger(ReportsService.name); constructor(private configService: ConfigService) { const region = this.configService.get('AWS_REGION', 'us-east-1'); + const accessKeyId = this.configService.get('AWS_ACCESS_KEY_ID'); + const secretAccessKey = this.configService.get('AWS_SECRET_ACCESS_KEY'); - try { - this.dynamoClient = new DynamoDBClient({ - region: this.configService.get('AWS_REGION', 'us-east-1'), - }); - } catch (error: unknown) { - console.error('DynamoDB Client Config:', JSON.stringify(error, null, 2)); - const accessKeyId = this.configService.get('AWS_ACCESS_KEY_ID'); - const secretAccessKey = this.configService.get('AWS_SECRET_ACCESS_KEY'); + // Prepare client configuration + const clientConfig: any = { region }; - const clientConfig: any = { region }; - - // Only add credentials if both values are present - if (accessKeyId && secretAccessKey) { - clientConfig.credentials = { accessKeyId, secretAccessKey }; - } + // Only add credentials if both values are present + if (accessKeyId && secretAccessKey) { + clientConfig.credentials = { accessKeyId, secretAccessKey }; + } + try { this.dynamoClient = new DynamoDBClient(clientConfig); + } catch (error: unknown) { + this.logger.error('Failed to initialize DynamoDB client:', error); + throw new InternalServerErrorException('Failed to initialize database connection'); } this.tableName = this.configService.get('DYNAMODB_REPORTS_TABLE', 'reports'); @@ -50,18 +50,27 @@ export class ReportsService { const response = await this.dynamoClient.send(command); return (response.Items || []).map(item => unmarshall(item) as Report); } catch (error: unknown) { - console.error('DynamoDB Error Details:', JSON.stringify(error, null, 2)); - if (error instanceof Error && error.name === 'UnrecognizedClientException') { - throw new Error( - 'Invalid AWS credentials. Please check your AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY.', - ); + this.logger.error(`Error fetching all reports: ${this.formatError(error)}`); + + if (error instanceof DynamoDBServiceException) { + if (error.name === 'UnrecognizedClientException') { + throw new InternalServerErrorException( + 'Invalid AWS credentials. Please check your AWS configuration.', + ); + } else if (error.name === 'ResourceNotFoundException') { + throw new InternalServerErrorException( + `Table "${this.tableName}" not found. Please check your database configuration.`, + ); + } } - throw error; + + throw new InternalServerErrorException('Failed to fetch reports from database'); } } async findLatest(queryDto: GetReportsQueryDto): Promise { - console.log('Running ReportsService.findLatest', queryDto); + this.logger.log(`Running findLatest with params: ${JSON.stringify(queryDto)}`); + // Convert limit to a number to avoid serialization errors const limit = typeof queryDto.limit === 'string' ? parseInt(queryDto.limit, 10) : queryDto.limit || 10; @@ -71,59 +80,130 @@ export class ReportsService { Limit: limit, }); - const response = await this.dynamoClient.send(command); - const reports = (response.Items || []).map(item => unmarshall(item) as Report); + try { + const response = await this.dynamoClient.send(command); + const reports = (response.Items || []).map(item => unmarshall(item) as Report); + + // Sort by createdAt in descending order + return reports + .sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()) + .slice(0, limit); + } catch (error: unknown) { + this.logger.error(`Error fetching latest reports: ${this.formatError(error)}`); + + if (error instanceof DynamoDBServiceException) { + if (error.name === 'ResourceNotFoundException') { + throw new InternalServerErrorException( + `Table "${this.tableName}" not found. Please check your database configuration.`, + ); + } + } - // Sort by createdAt in descending order - return reports - .sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()) - .slice(0, limit); + throw new InternalServerErrorException('Failed to fetch latest reports from database'); + } } async findOne(id: string): Promise { + if (!id) { + throw new NotFoundException('Report ID is required'); + } + const command = new GetItemCommand({ TableName: this.tableName, Key: marshall({ id }), }); - const response = await this.dynamoClient.send(command); + try { + const response = await this.dynamoClient.send(command); - if (!response.Item) { - throw new NotFoundException(`Report with ID ${id} not found`); - } + if (!response.Item) { + throw new NotFoundException(`Report with ID ${id} not found`); + } + + return unmarshall(response.Item) as Report; + } catch (error: unknown) { + if (error instanceof NotFoundException) { + throw error; + } + + this.logger.error(`Error fetching report with ID ${id}: ${this.formatError(error)}`); + + if (error instanceof DynamoDBServiceException) { + if (error.name === 'ResourceNotFoundException') { + throw new InternalServerErrorException( + `Table "${this.tableName}" not found. Please check your database configuration.`, + ); + } + } - return unmarshall(response.Item) as Report; + throw new InternalServerErrorException(`Failed to fetch report with ID ${id}`); + } } async updateStatus(id: string, updateDto: UpdateReportStatusDto): Promise { - // First check if the report exists - const existingReport = await this.findOne(id); + if (!id) { + throw new NotFoundException('Report ID is required'); + } - const command = new UpdateItemCommand({ - TableName: this.tableName, - Key: marshall({ id }), - UpdateExpression: 'SET #status = :status, updatedAt = :updatedAt', - ExpressionAttributeNames: { - '#status': 'status', - }, - ExpressionAttributeValues: marshall({ - ':status': updateDto.status, - ':updatedAt': new Date().toISOString(), - }), - ReturnValues: 'ALL_NEW', - }); + if (!updateDto || !updateDto.status) { + throw new InternalServerErrorException('Status is required for update'); + } + + try { + // First check if the report exists + const existingReport = await this.findOne(id); + + const command = new UpdateItemCommand({ + TableName: this.tableName, + Key: marshall({ id }), + UpdateExpression: 'SET #status = :status, updatedAt = :updatedAt', + ExpressionAttributeNames: { + '#status': 'status', + }, + ExpressionAttributeValues: marshall({ + ':status': updateDto.status, + ':updatedAt': new Date().toISOString(), + }), + ReturnValues: 'ALL_NEW', + }); + + const response = await this.dynamoClient.send(command); + + if (!response.Attributes) { + // If for some reason Attributes is undefined, return the existing report with updated status + return { + ...existingReport, + status: updateDto.status, + updatedAt: new Date().toISOString(), + }; + } + + return unmarshall(response.Attributes) as Report; + } catch (error: unknown) { + if (error instanceof NotFoundException) { + throw error; + } + + this.logger.error(`Error updating report status for ID ${id}: ${this.formatError(error)}`); - const response = await this.dynamoClient.send(command); + if (error instanceof DynamoDBServiceException) { + if (error.name === 'ConditionalCheckFailedException') { + throw new NotFoundException(`Report with ID ${id} not found`); + } else if (error.name === 'ResourceNotFoundException') { + throw new InternalServerErrorException( + `Table "${this.tableName}" not found. Please check your database configuration.`, + ); + } + } - if (!response.Attributes) { - // If for some reason Attributes is undefined, return the existing report with updated status - return { - ...existingReport, - status: updateDto.status, - updatedAt: new Date().toISOString(), - }; + throw new InternalServerErrorException(`Failed to update status for report with ID ${id}`); } + } - return unmarshall(response.Attributes) as Report; + private formatError(error: unknown): string { + if (error instanceof Error) { + return `${error.name}: ${error.message}`; + } + return JSON.stringify(error, null, 2); } } From c89bf1f27ecb67a0daaca11b6f22cb736d6b33bf Mon Sep 17 00:00:00 2001 From: Guido Percu Date: Thu, 3 Apr 2025 14:51:55 -0400 Subject: [PATCH 08/11] Improve error handling --- backend/src/reports/reports.service.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/backend/src/reports/reports.service.ts b/backend/src/reports/reports.service.ts index 9106df1e..e0183412 100644 --- a/backend/src/reports/reports.service.ts +++ b/backend/src/reports/reports.service.ts @@ -1,4 +1,9 @@ -import { Injectable, NotFoundException, InternalServerErrorException, Logger } from '@nestjs/common'; +import { + Injectable, + NotFoundException, + InternalServerErrorException, + Logger, +} from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { DynamoDBClient, From 3e2ebbd6fe9b09ce5099ca1a09ec4fa0bf353784 Mon Sep 17 00:00:00 2001 From: Guido Percu Date: Thu, 3 Apr 2025 16:47:32 -0400 Subject: [PATCH 09/11] Add user to filter the request --- backend/src/reports/reports.controller.ts | 39 ++++++--- backend/src/reports/reports.service.ts | 97 ++++++++++++++++------- 2 files changed, 98 insertions(+), 38 deletions(-) diff --git a/backend/src/reports/reports.controller.ts b/backend/src/reports/reports.controller.ts index e9f78ea4..f38f5a8d 100644 --- a/backend/src/reports/reports.controller.ts +++ b/backend/src/reports/reports.controller.ts @@ -1,4 +1,4 @@ -import { Controller, Get, Patch, Param, Body, Query, ValidationPipe } from '@nestjs/common'; +import { Controller, Get, Patch, Param, Body, Query, ValidationPipe, Req } from '@nestjs/common'; import { ApiTags, ApiOperation, @@ -11,6 +11,7 @@ import { ReportsService } from './reports.service'; import { Report } from './models/report.model'; import { GetReportsQueryDto } from './dto/get-reports.dto'; import { UpdateReportStatusDto } from './dto/update-report-status.dto'; +import { Request } from 'express'; @ApiTags('reports') @Controller('reports') @@ -25,8 +26,9 @@ export class ReportsController { type: [Report], }) @Get() - async findAll(): Promise { - return this.reportsService.findAll(); + async findAll(@Req() request: Request): Promise { + const userId = this.extractUserId(request); + return this.reportsService.findAll(userId); } @ApiOperation({ summary: 'Get latest reports' }) @@ -41,14 +43,18 @@ export class ReportsController { description: 'Maximum number of reports to return', }) @Get('latest') - async findLatest(@Query(ValidationPipe) queryDto: GetReportsQueryDto): Promise { - return this.reportsService.findLatest(queryDto); + async findLatest( + @Query(ValidationPipe) queryDto: GetReportsQueryDto, + @Req() request: Request, + ): Promise { + const userId = this.extractUserId(request); + return this.reportsService.findLatest(queryDto, userId); } @ApiOperation({ summary: 'GET report' }) @ApiResponse({ status: 200, - description: 'Report status updated successfully', + description: 'Report details', type: Report, }) @ApiResponse({ @@ -60,8 +66,9 @@ export class ReportsController { description: 'Report ID', }) @Get(':id') - async getReport(@Param('id') id: string): Promise { - return this.reportsService.findOne(id); + async getReport(@Param('id') id: string, @Req() request: Request): Promise { + const userId = this.extractUserId(request); + return this.reportsService.findOne(id, userId); } @ApiOperation({ summary: 'Update report status' }) @@ -82,7 +89,21 @@ export class ReportsController { async updateStatus( @Param('id') id: string, @Body(ValidationPipe) updateDto: UpdateReportStatusDto, + @Req() request: Request, ): Promise { - return this.reportsService.updateStatus(id, updateDto); + const userId = this.extractUserId(request); + return this.reportsService.updateStatus(id, updateDto, userId); + } + + private extractUserId(request: Request): string { + console.log(request); + // The user object is attached to the request by the AuthGuard + const user = request.user as any; + + if (!user || !user.sub) { + throw new Error('User ID not found in token'); + } + + return user.sub; } } diff --git a/backend/src/reports/reports.service.ts b/backend/src/reports/reports.service.ts index e0183412..30dfadb0 100644 --- a/backend/src/reports/reports.service.ts +++ b/backend/src/reports/reports.service.ts @@ -3,6 +3,7 @@ import { NotFoundException, InternalServerErrorException, Logger, + ForbiddenException, } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { @@ -46,16 +47,26 @@ export class ReportsService { this.tableName = this.configService.get('DYNAMODB_REPORTS_TABLE', 'reports'); } - async findAll(): Promise { - const command = new ScanCommand({ - TableName: this.tableName, - }); + async findAll(userId: string): Promise { + if (!userId) { + throw new ForbiddenException('User ID is required'); + } try { + // If the table has a GSI for userId, use QueryCommand instead + const command = new ScanCommand({ + TableName: this.tableName, + FilterExpression: 'userId = :userId', + ExpressionAttributeValues: marshall({ + ':userId': userId, + }), + }); + const response = await this.dynamoClient.send(command); return (response.Items || []).map(item => unmarshall(item) as Report); } catch (error: unknown) { - this.logger.error(`Error fetching all reports: ${this.formatError(error)}`); + this.logger.error(`Error fetching reports for user ${userId}:`); + this.logger.error(error); if (error instanceof DynamoDBServiceException) { if (error.name === 'UnrecognizedClientException') { @@ -73,19 +84,30 @@ export class ReportsService { } } - async findLatest(queryDto: GetReportsQueryDto): Promise { - this.logger.log(`Running findLatest with params: ${JSON.stringify(queryDto)}`); + async findLatest(queryDto: GetReportsQueryDto, userId: string): Promise { + this.logger.log( + `Running findLatest with params: ${JSON.stringify(queryDto)} for user ${userId}`, + ); + + if (!userId) { + throw new ForbiddenException('User ID is required'); + } // Convert limit to a number to avoid serialization errors const limit = typeof queryDto.limit === 'string' ? parseInt(queryDto.limit, 10) : queryDto.limit || 10; - const command = new ScanCommand({ - TableName: this.tableName, - Limit: limit, - }); - try { + // If the table has a GSI for userId, use QueryCommand instead + const command = new ScanCommand({ + TableName: this.tableName, + FilterExpression: 'userId = :userId', + ExpressionAttributeValues: marshall({ + ':userId': userId, + }), + Limit: limit * 5, // Fetch more items since we'll filter by userId + }); + const response = await this.dynamoClient.send(command); const reports = (response.Items || []).map(item => unmarshall(item) as Report); @@ -94,7 +116,8 @@ export class ReportsService { .sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()) .slice(0, limit); } catch (error: unknown) { - this.logger.error(`Error fetching latest reports: ${this.formatError(error)}`); + this.logger.error(`Error fetching latest reports for user ${userId}:`); + this.logger.error(error); if (error instanceof DynamoDBServiceException) { if (error.name === 'ResourceNotFoundException') { @@ -108,11 +131,15 @@ export class ReportsService { } } - async findOne(id: string): Promise { + async findOne(id: string, userId: string): Promise { if (!id) { throw new NotFoundException('Report ID is required'); } + if (!userId) { + throw new ForbiddenException('User ID is required'); + } + const command = new GetItemCommand({ TableName: this.tableName, Key: marshall({ id }), @@ -125,13 +152,21 @@ export class ReportsService { throw new NotFoundException(`Report with ID ${id} not found`); } - return unmarshall(response.Item) as Report; + const report = unmarshall(response.Item) as Report; + + // Verify the report belongs to the user + if (report.userId !== userId) { + throw new ForbiddenException('You do not have permission to access this report'); + } + + return report; } catch (error: unknown) { if (error instanceof NotFoundException) { throw error; } - this.logger.error(`Error fetching report with ID ${id}: ${this.formatError(error)}`); + this.logger.error(`Error fetching report with ID ${id}:`); + this.logger.error(error); if (error instanceof DynamoDBServiceException) { if (error.name === 'ResourceNotFoundException') { @@ -145,7 +180,11 @@ export class ReportsService { } } - async updateStatus(id: string, updateDto: UpdateReportStatusDto): Promise { + async updateStatus( + id: string, + updateDto: UpdateReportStatusDto, + userId: string, + ): Promise { if (!id) { throw new NotFoundException('Report ID is required'); } @@ -154,20 +193,26 @@ export class ReportsService { throw new InternalServerErrorException('Status is required for update'); } + if (!userId) { + throw new ForbiddenException('User ID is required'); + } + try { - // First check if the report exists - const existingReport = await this.findOne(id); + // First check if the report exists and belongs to the user + const existingReport = await this.findOne(id, userId); const command = new UpdateItemCommand({ TableName: this.tableName, Key: marshall({ id }), UpdateExpression: 'SET #status = :status, updatedAt = :updatedAt', + ConditionExpression: 'userId = :userId', // Ensure the report belongs to the user ExpressionAttributeNames: { '#status': 'status', }, ExpressionAttributeValues: marshall({ ':status': updateDto.status, ':updatedAt': new Date().toISOString(), + ':userId': userId, }), ReturnValues: 'ALL_NEW', }); @@ -189,11 +234,12 @@ export class ReportsService { throw error; } - this.logger.error(`Error updating report status for ID ${id}: ${this.formatError(error)}`); + this.logger.error(`Error updating report status for ID ${id}:`); + this.logger.error(error); if (error instanceof DynamoDBServiceException) { if (error.name === 'ConditionalCheckFailedException') { - throw new NotFoundException(`Report with ID ${id} not found`); + throw new ForbiddenException('You do not have permission to update this report'); } else if (error.name === 'ResourceNotFoundException') { throw new InternalServerErrorException( `Table "${this.tableName}" not found. Please check your database configuration.`, @@ -201,14 +247,7 @@ export class ReportsService { } } - throw new InternalServerErrorException(`Failed to update status for report with ID ${id}`); - } - } - - private formatError(error: unknown): string { - if (error instanceof Error) { - return `${error.name}: ${error.message}`; + throw new InternalServerErrorException(`Failed to update report status for ID ${id}`); } - return JSON.stringify(error, null, 2); } } From 23c8759090812b1863c301f5421934d8092b6ec2 Mon Sep 17 00:00:00 2001 From: Guido Percu Date: Thu, 3 Apr 2025 17:01:12 -0400 Subject: [PATCH 10/11] Get user from the request --- backend/src/app.module.ts | 11 +++--- backend/src/auth/auth.middleware.ts | 44 +++++++++++++++++++++++ backend/src/reports/reports.controller.ts | 35 +++++++++++------- 3 files changed, 73 insertions(+), 17 deletions(-) create mode 100644 backend/src/auth/auth.middleware.ts diff --git a/backend/src/app.module.ts b/backend/src/app.module.ts index 2cf9b087..30faa624 100644 --- a/backend/src/app.module.ts +++ b/backend/src/app.module.ts @@ -1,4 +1,4 @@ -import { Module, NestModule } from '@nestjs/common'; +import { Module, NestModule, MiddlewareConsumer } from '@nestjs/common'; import { ConfigModule } from '@nestjs/config'; import configuration from './config/configuration'; import { AppController } from './app.controller'; @@ -9,6 +9,8 @@ import { PerplexityController } from './controllers/perplexity/perplexity.contro import { UserController } from './user/user.controller'; import { ReportsModule } from './reports/reports.module'; import { HealthController } from './health/health.controller'; +import { AuthMiddleware } from './auth/auth.middleware'; + @Module({ imports: [ ConfigModule.forRoot({ @@ -21,8 +23,9 @@ import { HealthController } from './health/health.controller'; providers: [AppService, AwsSecretsService, PerplexityService], }) export class AppModule implements NestModule { - configure() { - // Add your middleware configuration here if needed - // If you don't need middleware, you can leave this empty + configure(consumer: MiddlewareConsumer) { + consumer + .apply(AuthMiddleware) + .forRoutes('*'); // Apply to all routes } } diff --git a/backend/src/auth/auth.middleware.ts b/backend/src/auth/auth.middleware.ts new file mode 100644 index 00000000..fac05827 --- /dev/null +++ b/backend/src/auth/auth.middleware.ts @@ -0,0 +1,44 @@ +import { Injectable, NestMiddleware } from '@nestjs/common'; +import { Request, Response, NextFunction } from 'express'; +import { ConfigService } from '@nestjs/config'; +import * as jwt from 'jsonwebtoken'; + +// Extend the Express Request interface to include the user property +export interface RequestWithUser extends Request { + user?: { + sub: string; + email?: string; + groups?: string[]; + [key: string]: any; + } | null; +} + +@Injectable() +export class AuthMiddleware implements NestMiddleware { + constructor(private configService: ConfigService) {} + + use(req: RequestWithUser, res: Response, next: NextFunction) { + const authHeader = req.headers.authorization; + + if (authHeader && authHeader.startsWith('Bearer ')) { + const token = authHeader.substring(7); + try { + // Verify the JWT token + const decoded = jwt.verify(token, this.configService.get('JWT_SECRET') || 'dev-secret'); + + // Attach the decoded user to the request + req.user = { + sub: decoded.sub as string, + }; + } catch (error) { + // If token verification fails, set user to null + req.user = null; + } + } else { + // No token provided + req.user = null; + } + + next(); + } +} diff --git a/backend/src/reports/reports.controller.ts b/backend/src/reports/reports.controller.ts index f38f5a8d..42317a81 100644 --- a/backend/src/reports/reports.controller.ts +++ b/backend/src/reports/reports.controller.ts @@ -1,4 +1,14 @@ -import { Controller, Get, Patch, Param, Body, Query, ValidationPipe, Req } from '@nestjs/common'; +import { + Controller, + Get, + Patch, + Param, + Body, + Query, + ValidationPipe, + Req, + UnauthorizedException, +} from '@nestjs/common'; import { ApiTags, ApiOperation, @@ -11,7 +21,7 @@ import { ReportsService } from './reports.service'; import { Report } from './models/report.model'; import { GetReportsQueryDto } from './dto/get-reports.dto'; import { UpdateReportStatusDto } from './dto/update-report-status.dto'; -import { Request } from 'express'; +import { RequestWithUser } from '../auth/auth.middleware'; @ApiTags('reports') @Controller('reports') @@ -22,11 +32,11 @@ export class ReportsController { @ApiOperation({ summary: 'Get all reports' }) @ApiResponse({ status: 200, - description: 'Returns all reports', + description: 'Returns all reports for the authenticated user', type: [Report], }) @Get() - async findAll(@Req() request: Request): Promise { + async findAll(@Req() request: RequestWithUser): Promise { const userId = this.extractUserId(request); return this.reportsService.findAll(userId); } @@ -34,7 +44,7 @@ export class ReportsController { @ApiOperation({ summary: 'Get latest reports' }) @ApiResponse({ status: 200, - description: 'Returns the latest reports', + description: 'Returns the latest reports for the authenticated user', type: [Report], }) @ApiQuery({ @@ -45,7 +55,7 @@ export class ReportsController { @Get('latest') async findLatest( @Query(ValidationPipe) queryDto: GetReportsQueryDto, - @Req() request: Request, + @Req() request: RequestWithUser, ): Promise { const userId = this.extractUserId(request); return this.reportsService.findLatest(queryDto, userId); @@ -66,7 +76,7 @@ export class ReportsController { description: 'Report ID', }) @Get(':id') - async getReport(@Param('id') id: string, @Req() request: Request): Promise { + async getReport(@Param('id') id: string, @Req() request: RequestWithUser): Promise { const userId = this.extractUserId(request); return this.reportsService.findOne(id, userId); } @@ -89,19 +99,18 @@ export class ReportsController { async updateStatus( @Param('id') id: string, @Body(ValidationPipe) updateDto: UpdateReportStatusDto, - @Req() request: Request, + @Req() request: RequestWithUser, ): Promise { const userId = this.extractUserId(request); return this.reportsService.updateStatus(id, updateDto, userId); } - private extractUserId(request: Request): string { - console.log(request); - // The user object is attached to the request by the AuthGuard - const user = request.user as any; + private extractUserId(request: RequestWithUser): string { + // The user object is attached to the request by our middleware + const user = request.user; if (!user || !user.sub) { - throw new Error('User ID not found in token'); + throw new UnauthorizedException('User ID not found in request'); } return user.sub; From c44f0f6c543317ee9a4108d8e339c36a63bbfbf1 Mon Sep 17 00:00:00 2001 From: Guido Percu Date: Thu, 3 Apr 2025 17:35:42 -0400 Subject: [PATCH 11/11] Fix JWT --- backend/src/app.module.ts | 4 +--- backend/src/auth/auth.middleware.ts | 22 ++++++++++++++++++---- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/backend/src/app.module.ts b/backend/src/app.module.ts index 30faa624..94a4c4c5 100644 --- a/backend/src/app.module.ts +++ b/backend/src/app.module.ts @@ -24,8 +24,6 @@ import { AuthMiddleware } from './auth/auth.middleware'; }) export class AppModule implements NestModule { configure(consumer: MiddlewareConsumer) { - consumer - .apply(AuthMiddleware) - .forRoutes('*'); // Apply to all routes + consumer.apply(AuthMiddleware).forRoutes('*'); // Apply to all routes } } diff --git a/backend/src/auth/auth.middleware.ts b/backend/src/auth/auth.middleware.ts index fac05827..f954afb0 100644 --- a/backend/src/auth/auth.middleware.ts +++ b/backend/src/auth/auth.middleware.ts @@ -13,25 +13,39 @@ export interface RequestWithUser extends Request { } | null; } +// Add this interface to define the token structure +interface DecodedToken { + payload: { + sub: string; + username?: string; + email?: string; + [key: string]: any; + }; + header: any; + signature: string; +} + @Injectable() export class AuthMiddleware implements NestMiddleware { constructor(private configService: ConfigService) {} use(req: RequestWithUser, res: Response, next: NextFunction) { const authHeader = req.headers.authorization; - if (authHeader && authHeader.startsWith('Bearer ')) { const token = authHeader.substring(7); try { // Verify the JWT token - const decoded = jwt.verify(token, this.configService.get('JWT_SECRET') || 'dev-secret'); + const decodedToken = jwt.decode(token, { complete: true }) as DecodedToken; - // Attach the decoded user to the request + // Access user info from the payload req.user = { - sub: decoded.sub as string, + sub: decodedToken?.payload.sub as string, + username: decodedToken?.payload.username as string, }; } catch (error) { // If token verification fails, set user to null + console.log('AuthMiddleware error'); + console.log(error); req.user = null; } } else {