Feat/#214 mypage#220
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 사용자가 자신의 계정 정보를 관리하고 활동 내역을 확인할 수 있는 포괄적인 마이페이지 기능을 도입합니다. 프로필 정보 조회 및 수정, 작성한 게시글, 댓글, 스크랩 목록 확인과 같은 핵심 기능들이 추가되었으며, 멀티 프로필 환경에서 프로필 전환을 위한 로그인 정보 반환 로직도 개선되었습니다. 이는 사용자 경험을 향상시키고 개인화된 서비스 제공을 위한 중요한 단계입니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![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
이 PR은 프로필 조회/수정, 사용자의 게시글/댓글/스크랩 목록 조회 등 마이페이지 기능을 도입합니다. 코드는 새로운 서비스, 컨트롤러, DTO로 잘 구조화되어 있습니다. 하지만 다른 사용자의 데이터에 접근하고 수정할 수 있는 심각한 보안 취약점(IDOR)을 발견했습니다. 또한 닉네임 업데이트 로직의 버그, API 설계의 비일관성, 불필요한 null 체크, 그리고 사용되지 않는 의존성 등 몇 가지 개선점을 찾았습니다. 제 의견을 검토해 주시기 바랍니다.
|
|
||
| // 내 정보 조회(닉네임, 조직(동아리라면 대학), 이메일) | ||
| @GetMapping("/profiles/{memberOrgId}") | ||
| public ResponseEntity<ApiResponse<MyPageInfoResp>> getMyPageInfo(@PathVariable Long memberOrgId) { |
There was a problem hiding this comment.
심각한 보안 취약점을 해결하려면, 요청된 memberOrgId에 접근할 권한이 사용자에게 있는지 확인해야 합니다. 여기서 인증된 사용자의 principal을 가져와서 서비스 계층에 ID를 전달하여 소유권을 확인하십시오. 이 변경 사항은 memberOrgId를 사용하는 이 컨트롤러의 모든 엔드포인트(getMyPageInfo, updateProfile, getMyPosts, getMyComments, getMyScraps)에 적용되어야 합니다. 또한 @AuthenticationPrincipal과 CustomUserDetails에 대한 import 문을 추가해야 합니다.
| public ResponseEntity<ApiResponse<MyPageInfoResp>> getMyPageInfo(@PathVariable Long memberOrgId) { | |
| public ResponseEntity<ApiResponse<MyPageInfoResp>> getMyPageInfo(@PathVariable Long memberOrgId, @org.springframework.security.core.annotation.AuthenticationPrincipal org.example.tackit.domain.auth.login.security.CustomUserDetails user) { |
References
- In this closed, organization-based community application, it is acceptable to retrieve the organization directly from
CustomUserDetailsas it is a fundamental piece of user information. This comment aligns with usingCustomUserDetailsfor authorization checks.
There was a problem hiding this comment.
소유자 권한 검증 메서드 MemberOrgValidator에 추가하여 해결
| public MyPageInfoResp getMypageInfo(Long memberOrgId) { | ||
| MemberOrg memberOrg = memberOrgValidator.validateActiveMembership(memberOrgId); | ||
|
|
||
| return MyPageInfoResp.from(memberOrg); |
There was a problem hiding this comment.
여기에 심각한 IDOR(Insecure Direct Object Reference) 취약점이 있습니다. 이 메서드는 현재 인증된 사용자가 해당 프로필의 소유자인지 확인하지 않고 memberOrgId를 사용하여 데이터를 가져옵니다. 이로 인해 모든 사용자가 다른 사용자의 정보를 볼 수 있습니다. 소유권 확인을 추가해야 합니다. 이 문제는 이 서비스의 모든 public 메서드(getMypageInfo, updateProfile, getMyPosts, getMyComments, getMyScraps)에 해당됩니다.
| public MyPageInfoResp getMypageInfo(Long memberOrgId) { | |
| MemberOrg memberOrg = memberOrgValidator.validateActiveMembership(memberOrgId); | |
| return MyPageInfoResp.from(memberOrg); | |
| public MyPageInfoResp getMypageInfo(Long memberOrgId, Long memberId) { | |
| MemberOrg memberOrg = memberOrgValidator.validateActiveMembership(memberOrgId); | |
| if (!memberOrg.getMember().getId().equals(memberId)) { | |
| throw new BusinessException(ErrorCode.ACCESS_DENIED_EDIT); // 이 권한 실패에 대해 더 구체적인 오류 코드를 고려해보세요. | |
| } | |
| return MyPageInfoResp.from(memberOrg); |
There was a problem hiding this comment.
소유자 권한 검증 메서드 MemberOrgValidator에 추가하여 해결
| public void updateProfile(Long memberOrgId, UpdateProfileReq request, String email) { | ||
|
|
||
| MemberOrg memberOrg = memberOrgValidator.validateActiveMembership(memberOrgId); | ||
| memberOrgValidator.validateOwner(memberOrg, email); |
There was a problem hiding this comment.
저희 소유자 검증하는 거는 마이페이지 뿐만 아니라 다른 모든 API에도 적용되어야 해서, ProfileContextResolver 에서 수정하는 게 맞지 않나 싶습니다! 일단 현재로서는 마이페이지 API 배포가 필요해서 그냥 진행하는데, 리팩토링하는게 좋을 것 같아요
| @GetMapping("/scraps/{memberOrgId}") | ||
| public ResponseEntity<ApiResponse<List<MyScrapListResp>>> getMyScraps( | ||
| @PathVariable Long memberOrgId, | ||
| @AuthenticationPrincipal CustomUserDetails userDetails |
There was a problem hiding this comment.
이번에 구현한 마이페이지의 경우 특정 모임의 프로필의 마이페이지다보니 @ActiveProfile 을 사용해서 진행하는 게 맞지 않나 싶습니다! 나중에 이것도 리팩토링 해주세요~
2 participants
이슈 번호
작업 내용