Fix : #8825 If attachment token expires, it throws a 500 error instead of Unauthenticated (#9043)

Fixes #8825

FilePathGuard implements token verification via verifyWorkspaceToken
function which throws AuthException error ,
since CanActivate expects a boolean value , we add a try catch while
verifying the token

if token is invalid/expired 
<img width="1470" alt="Screenshot 2024-12-12 at 9 44 58 PM"
src="https://github.com/user-attachments/assets/106a85dd-f894-46ea-80c3-f29b4ea5b4d3"
/>
else 
<img width="917" alt="Screenshot 2024-12-12 at 9 47 10 PM"
src="https://github.com/user-attachments/assets/d82168f4-d140-48dc-94a4-56773a93db83"
/>

---------

Co-authored-by: Félix Malfait <felix.malfait@gmail.com>
Co-authored-by: Félix Malfait <felix@twenty.com>
Co-authored-by: Charles Bochet <charles@twenty.com>
This commit is contained in:
munch-lax
2025-01-08 23:39:33 +05:30
committed by GitHub
parent d61409a5df
commit d324cac742
7 changed files with 108 additions and 50 deletions

View File

@ -14,9 +14,9 @@ import {
AuthExceptionCode,
} from 'src/engine/core-modules/auth/auth.exception';
import {
PASSWORD_REGEX,
compareHash,
hashPassword,
PASSWORD_REGEX,
} from 'src/engine/core-modules/auth/auth.util';
import { DomainManagerService } from 'src/engine/core-modules/domain-manager/service/domain-manager.service';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';

View File

@ -1,4 +1,12 @@
import { Controller, Get, Param, Req, Res, UseGuards } from '@nestjs/common';
import {
Controller,
Get,
Param,
Req,
Res,
UseFilters,
UseGuards,
} from '@nestjs/common';
import { Response } from 'express';
@ -7,15 +15,20 @@ import {
FileStorageExceptionCode,
} from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception';
import {
FileException,
FileExceptionCode,
} from 'src/engine/core-modules/file/file.exception';
import {
checkFilePath,
checkFilename,
} from 'src/engine/core-modules/file/file.utils';
import { FileApiExceptionFilter } from 'src/engine/core-modules/file/filters/file-api-exception.filter';
import { FilePathGuard } from 'src/engine/core-modules/file/guards/file-path-guard';
import { FileService } from 'src/engine/core-modules/file/services/file.service';
// TODO: Add cookie authentication
@Controller('files')
@UseFilters(FileApiExceptionFilter)
@UseGuards(FilePathGuard)
export class FileController {
constructor(private readonly fileService: FileService) {}
@ -27,15 +40,15 @@ export class FileController {
@Req() req: Request,
) {
const folderPath = checkFilePath(params[0]);
const filename = checkFilename(params['filename']);
const workspaceId = (req as any)?.workspaceId;
if (!workspaceId) {
return res
.status(401)
.send({ error: 'Unauthorized, missing workspaceId' });
throw new FileException(
'Unauthorized: missing workspaceId',
FileExceptionCode.UNAUTHENTICATED,
);
}
try {
@ -46,7 +59,10 @@ export class FileController {
);
fileStream.on('error', () => {
res.status(500).send({ error: 'Internal server error' });
throw new FileException(
'Error streaming file from storage',
FileExceptionCode.INTERNAL_SERVER_ERROR,
);
});
fileStream.pipe(res);
@ -55,10 +71,16 @@ export class FileController {
error instanceof FileStorageException &&
error.code === FileStorageExceptionCode.FILE_NOT_FOUND
) {
return res.status(404).send({ error: 'File not found' });
throw new FileException(
'File not found',
FileExceptionCode.FILE_NOT_FOUND,
);
}
return res.status(500).send({ error: 'Internal server error' });
throw new FileException(
`Error retrieving file: ${error.message}`,
FileExceptionCode.INTERNAL_SERVER_ERROR,
);
}
}
}

View File

@ -0,0 +1,14 @@
import { CustomException } from 'src/utils/custom-exception';
export enum FileExceptionCode {
UNAUTHENTICATED = 'UNAUTHENTICATED',
INTERNAL_SERVER_ERROR = 'INTERNAL_SERVER_ERROR',
FILE_NOT_FOUND = 'FILE_NOT_FOUND',
}
export class FileException extends CustomException {
code: FileExceptionCode;
constructor(message: string, code: FileExceptionCode) {
super(message, code);
}
}

View File

@ -0,0 +1,43 @@
import { ArgumentsHost, Catch, ExceptionFilter } from '@nestjs/common';
import { Response } from 'express';
import { HttpExceptionHandlerService } from 'src/engine/core-modules/exception-handler/http-exception-handler.service';
import {
FileException,
FileExceptionCode,
} from 'src/engine/core-modules/file/file.exception';
@Catch(FileException)
export class FileApiExceptionFilter implements ExceptionFilter {
constructor(
private readonly httpExceptionHandlerService: HttpExceptionHandlerService,
) {}
catch(exception: FileException, host: ArgumentsHost) {
const ctx = host.switchToHttp();
const response = ctx.getResponse<Response>();
switch (exception.code) {
case FileExceptionCode.UNAUTHENTICATED:
return this.httpExceptionHandlerService.handleError(
exception,
response,
403,
);
case FileExceptionCode.FILE_NOT_FOUND:
return this.httpExceptionHandlerService.handleError(
exception,
response,
404,
);
case FileExceptionCode.INTERNAL_SERVER_ERROR:
default:
return this.httpExceptionHandlerService.handleError(
exception,
response,
500,
);
}
}
}

View File

@ -1,10 +1,4 @@
import {
CanActivate,
ExecutionContext,
HttpException,
HttpStatus,
Injectable,
} from '@nestjs/common';
import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';
@ -20,12 +14,16 @@ export class FilePathGuard implements CanActivate {
return false;
}
const payload = await this.jwtWrapperService.verifyWorkspaceToken(
query['token'],
'FILE',
);
try {
const payload = await this.jwtWrapperService.verifyWorkspaceToken(
query['token'],
'FILE',
);
if (!payload.workspaceId) {
if (!payload.workspaceId) {
return false;
}
} catch (error) {
return false;
}
@ -33,32 +31,10 @@ export class FilePathGuard implements CanActivate {
json: true,
});
const expirationDate = decodedPayload?.['expirationDate'];
const workspaceId = decodedPayload?.['workspaceId'];
const isExpired = await this.isExpired(expirationDate);
if (isExpired) {
return false;
}
request.workspaceId = workspaceId;
return true;
}
private async isExpired(expirationDate: string): Promise<boolean> {
if (!expirationDate) {
return true;
}
if (new Date(expirationDate) < new Date()) {
throw new HttpException(
'This url has expired. Please reload twenty page and open file again.',
HttpStatus.FORBIDDEN,
);
}
return false;
}
}

View File

@ -2,9 +2,6 @@ import { Injectable } from '@nestjs/common';
import { Stream } from 'stream';
import { addMilliseconds } from 'date-fns';
import ms from 'ms';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service';
import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';
@ -39,15 +36,13 @@ export class FileService {
payloadToEncode.workspaceId,
);
const expirationDate = addMilliseconds(new Date(), ms(fileTokenExpiresIn));
const signedPayload = this.jwtWrapperService.sign(
{
expirationDate: expirationDate,
...payloadToEncode,
},
{
secret,
expiresIn: fileTokenExpiresIn,
},
);

View File

@ -1,4 +1,12 @@
export * from './animated-expandable-container/components/AnimatedExpandableContainer';
export * from './animated-expandable-container/types/AnimationDimension';
export * from './animated-expandable-container/types/AnimationDurationObject';
export * from './animated-expandable-container/types/AnimationDurations';
export * from './animated-expandable-container/types/AnimationMode';
export * from './animated-expandable-container/types/AnimationSize';
export * from './animated-expandable-container/utils/getCommonStyles';
export * from './animated-expandable-container/utils/getExpandableAnimationConfig';
export * from './animated-expandable-container/utils/getTransitionValues';
export * from './animated-placeholder/components/AnimatedPlaceholder';
export * from './animated-placeholder/components/EmptyPlaceholderStyled';
export * from './animated-placeholder/components/ErrorPlaceholderStyled';