Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Jan 8, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
@@ -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';

FelixMalfait marked this conversation as resolved.
Show resolved Hide resolved
import { Response } from 'express';

Expand All @@ -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) {}
Expand All @@ -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 {
Expand All @@ -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);
Expand All @@ -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,
);
}
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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,
);
}
}
}
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -20,45 +14,27 @@ 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;
}

const decodedPayload = await this.jwtWrapperService.decode(query['token'], {
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
},
);

Expand Down
8 changes: 8 additions & 0 deletions packages/twenty-ui/src/layout/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
Loading