feat(be): fix unauthorized access to course management pages#3576
feat(be): fix unauthorized access to course management pages#3576Choi-Jung-Hyeon wants to merge 6 commits into
Conversation
Choi-Jung-Hyeon
requested review from
EddieDawn,
RyuRaseul,
hjkim24,
nhjbest22,
westsunh and
zero1177
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the course detail layout into a server component to implement server-side authorization and data fetching, while also extracting the tab navigation into a new client component. In the backend, the GroupLeaderGuard was updated to throw a ForbiddenException for better error handling. Feedback highlights a critical security flaw in the isAdmin logic that could allow unauthenticated guests to bypass access controls, and suggests consolidating redundant API calls to improve performance.
There was a problem hiding this comment.
Pull request overview
Fixes an authorization hole where users who are not the target course’s GroupLeader could directly access /admin/course/:courseId/* pages via URL and only fail later during GraphQL fetch.
Changes:
- Converted
/admin/course/[courseId]/layout.tsxto a Server Component and added a server-side permission check (viaGET course/joined) with redirect before render. - Extracted tab/header UI into a new client component (
CourseDetailTabs) to remove client-hook dependencies from the server layout. - Updated backend
GroupLeaderGuardto throw aForbiddenExceptionwith a clearer message instead of returningfalse(still 403).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/frontend/app/admin/course/[courseId]/layout.tsx | Server-side pre-render authorization check + server-side course info fetch, rendering tabs via client component |
| apps/frontend/app/admin/course/[courseId]/_components/CourseDetailTabs.tsx | New client component encapsulating pathname-based active tab + header UI |
| apps/backend/libs/auth/src/roles/group-leader.guard.ts | Improves forbidden response messaging by throwing ForbiddenException explicitly |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Choi-Jung-Hyeon
changed the title
|
프론트한테 리뷰 부탁해야할듯? |
|
컨플릭 해결해주시고 |
zero1177
left a comment
zero1177
left a comment
There was a problem hiding this comment.
백엔드는 이상없고 프론트 리뷰만 받으면 될것 같습니다~
|
✅ Syncing Preview App Succeeded Application: |
|
❗ Syncing Preview App Failed Application: |
|
❗ Syncing Preview App Failed Application: |
|
❗ Syncing Preview App Failed Application: |
| } | ||
| } | ||
| } | ||
| ` |
There was a problem hiding this comment.
이거 get_course_query 는 지워주세요!
| Authorization: session.token.accessToken | ||
| }, | ||
| body: JSON.stringify({ | ||
| query: GET_COURSE_QUERY, |
There was a problem hiding this comment.
여기 그냥 GET_COURSE로 수정 부탁드립니다..!
Description
/admin/course/:courseId/*경로의 강의 관리 페이지에 권한 없는 유저가 URL 직접 접근할 수 있던 버그를 수정합니다.문제 상황:
Contest Manager 권한은 있지만 특정 강의의 GroupLeader가 아닌 유저가
/admin/course/2/user같은 URL에 직접 접근했을 때, 페이지가 그대로 렌더링되었습니다. 서버 사이드 권한 체크 없이 페이지가 먼저 뜨고, 클라이언트에서 데이터 fetch를 하는 단계에서야 "Forbidden resource"라는 맥락 없는 에러가 발생했습니다.원인:
기존
/admin/course/[courseId]/layout.tsx가'use client'Client Component로 선언되어 있어, 서버 사이드에서 사전 권한 확인 및 redirect 처리가 불가능한 구조였습니다.수정 내용:
layout.tsx를 Server Component로 전환했습니다.getCourse)를 서버 사이드에서 Fetch하여 성능을 최적화했습니다. 백엔드의GroupLeaderGuard가 검증을 수행하며, 인가되지 않은 유저일 경우 렌더링 전에 즉시/경로로 안전하게 redirect 처리합니다. (비로그인, 권한 없는 Manager 모두 차단됨)usePathname()등 클라이언트 훅 의존성은 새로운CourseDetailTabsClient Component로 분리했습니다.GroupLeaderGuard에서 권한 없을 때return false대신 명시적인 메시지로ForbiddenException을 throw하도록 수정했습니다. 기존에는 NestJS 내부 기본값인"Forbidden resource"가 노출되었으나, 이제 더 명확한 메시지를 반환합니다. HTTP 동작(403)은 기존과 동일합니다.Additional context
리뷰어분들이 아래 케이스를 확인해주시면 좋을 것 같습니다:
User→ 정상 접근 ✅User→/로 redirect ✅Admin/SuperAdmin→ Guard에 의해 체크 우회되어 정상 접근 ✅Before submitting the PR, please make sure you do the following
fixes #123).