Conversation
There was a problem hiding this comment.
Code Review
This pull request adds a JwtAuthGuard to the auth endpoint in the FilesController. However, feedback indicates that this addition is redundant because the class already enforces a global AdminGuard. To achieve the goal of allowing non-admin users to access specific endpoints, it is recommended to move the guard configuration from the class level to individual methods.
| } | ||
|
|
||
| @Get('auth') | ||
| @UseGuards(JwtAuthGuard) |
There was a problem hiding this comment.
현재 FilesController 클래스 레벨(43번 라인)에 @UseGuards(JwtAuthGuard, AdminGuard)가 이미 적용되어 있어, 모든 메서드에 관리자 권한이 필요합니다. authCheck에 JwtAuthGuard를 추가하는 것은 중복일 뿐만 아니라, 클래스 레벨의 AdminGuard를 우회하지 못하므로 일반 사용자의 접근을 허용할 수 없습니다.\n\n또한 PR 제목("jwt로 read 판단")의 의도가 조회 권한을 일반 사용자에게도 부여하는 것이라면, authCheck뿐만 아니라 readFiles(233번 라인) 등 다른 조회용 API의 가드 설정도 함께 검토되어야 합니다. 클래스 레벨에서 AdminGuard를 제거하고 권한이 필요한 메서드에만 개별 적용하는 방식을 권장합니다.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d01892fea7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| @Get('auth') | ||
| @UseGuards(JwtAuthGuard) |
There was a problem hiding this comment.
Remove duplicate JwtAuthGuard on auth endpoint
Because the controller already has @UseGuards(JwtAuthGuard, AdminGuard) at class scope, adding @UseGuards(JwtAuthGuard) on authCheck causes JwtAuthGuard to run twice for the same request. In this codebase, JwtAuthGuard.canActivate calls supabaseService.getClient().auth.getUser(token), so this change doubles external auth lookups (extra latency/load) and can introduce flaky 401s if the second lookup fails transiently after the first one succeeded.
Useful? React with 👍 / 👎.
No description provided.