Conversation
There was a problem hiding this comment.
Code Review
This pull request increments the version of the server-rest package and introduces a new GET /auth endpoint in the FilesController to support Nginx auth_request verification. The review feedback identifies critical performance risks associated with external API calls in the authentication guard, potential access issues for non-admin users, and concerns over high-importance logging for static assets. It is also recommended to simplify the endpoint implementation by leveraging NestJS's default response handling instead of manually injecting the response object.
| @Get('auth') | ||
| @ApiOperation({ | ||
| summary: 'nginx auth_request용 인증 확인', | ||
| description: | ||
| 'JWT가 유효하면 200, 아니면 401/403을 반환한다. nginx auth_request 디렉티브(/private/ 경로)에서 사용한다.', | ||
| }) | ||
| @ApiOkResponse({ description: 'Authenticated' }) | ||
| authCheck(@Res() res: Response): Response { | ||
| return res.status(HttpStatus.OK).send(); | ||
| } |
There was a problem hiding this comment.
nginx auth_request 용도로 사용되는 이 엔드포인트는 다음과 같은 성능 및 운영상의 문제를 야기할 수 있습니다:
- 성능 저하: 현재 적용된
JwtAuthGuard는 매 요청마다 Supabase API(getUser)를 호출하는 네트워크 요청을 발생시킵니다. 정적 자원(이미지, JS 등) 요청 시마다 외부 API 호출이 유발되어 응답 속도가 느려지고 Supabase API 레이트 리밋에 걸릴 위험이 큽니다.auth_request용도로는JwtService를 이용해 로컬에서 토큰 서명을 검증하는 가벼운 Guard를 별도로 사용하는 것을 권장합니다. - 권한 범위: 컨트롤러 레벨의
AdminGuard로 인해, 일반 사용자가 접근해야 하는 정적 자원까지 접근이 차단(403 Forbidden)될 수 있습니다. 의도된 것이 아니라면 권한 설정을 조정해야 합니다. - 로그 폭증:
@LogMetadata({ importance: 'high' })설정으로 인해 모든 정적 자원 요청에 대해 고수준 로그가 생성되어 시스템 부하 및 저장 비용이 증가할 수 있습니다.
| authCheck(@Res() res: Response): Response { | ||
| return res.status(HttpStatus.OK).send(); | ||
| } |
There was a problem hiding this comment.
NestJS에서는 단순한 200 OK 응답을 위해 @Res()를 사용하여 직접 응답 객체를 제어하기보다, NestJS의 기본 응답 처리 방식을 사용하는 것이 더 간결하고 테스트하기 쉽습니다. GET 요청은 기본적으로 200 OK를 반환하므로 메서드 본문을 비워두어도 무방합니다.
authCheck(): void {}
No description provided.