Refactor : security path 경로 수정을 통한 문제 제출 이슈 해결 #172
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
minjee2758
requested review from
Kimminu7,
NCookies,
chat26666,
pokerbearkr and
thezz9
WalkthroughHTTP 보안 설정이 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Backend
participant DB
User->>Browser: /test/reset-password 접속
Browser->>Backend: GET /test/reset-password
Backend-->>Browser: test-reset-password.html 반환
User->>Browser: 새 비밀번호 입력 및 제출
Browser->>Backend: POST /api/auth/reset-password (token, 비밀번호)
Backend->>DB: 토큰 및 비밀번호 검증/저장
DB-->>Backend: 결과 반환
Backend-->>Browser: 성공/실패 응답
Browser-->>User: 성공 시 모달 표시 및 로그인 페이지로 리다이렉트
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/org/ezcode/codetest/presentation/view/ViewController.java (1)
23-26: 새 뷰 매핑은 좋지만 상수화·공통 prefix 사용을 고려하세요.
"/test/reset-password"경로가 하드코딩되어 있습니다. 다른 테스트용 뷰들과 동일하게/test/**패턴을 많이 사용하고 있지만, 추후 운영/테스트 분리가 필요할 때 유지‧변경 비용이 커질 수 있습니다.
public static final String TEST_PREFIX = "/test";같은 상수를 두고TEST_PREFIX + "/reset-password"형태로 지정하면 경로를 한곳에서 관리할 수 있습니다.- 혹은
@RequestMapping("/test")를 클래스 레벨에 선언해 중복을 제거하세요.src/main/resources/templates/test-reset-password.html (2)
144-149: 토글 함수가 실제 UI와 연결되지 않음
toggleVisibility()는 정의만 있고 호출이 없습니다.
아이콘(🔒) 우측에 눈 아이콘을 추가해 클릭 시 호출하도록 하거나, 사용하지 않을 예정이라면 코드를 제거해 주세요. 사용하지 않는 dead-code 는 유지보수 비용만 증가시킵니다.
172-180: 비밀번호 복잡도·길이 검증 없음클라이언트에서 최소 길이(예: 8자)·대소문자·숫자 조합 등을 1차로 확인해주면 불필요한 서버 round-trip을 줄일 수 있습니다.
if (newPassword.length < 8) { alert("비밀번호는 최소 8자 이상이어야 합니다."); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
.DS_Storeis excluded by!**/.DS_Storesrc/.DS_Storeis excluded by!**/.DS_Storesrc/main/.DS_Storeis excluded by!**/.DS_Storesrc/main/java/.DS_Storeis excluded by!**/.DS_Storesrc/main/java/org/.DS_Storeis excluded by!**/.DS_Storesrc/main/resources/static/images/lock-icon.pngis excluded by!**/*.png
📒 Files selected for processing (7)
src/main/java/org/ezcode/codetest/common/security/config/SecurityConfig.java(1 hunks)src/main/java/org/ezcode/codetest/common/security/util/SecurityPath.java(1 hunks)src/main/java/org/ezcode/codetest/presentation/view/ViewController.java(1 hunks)src/main/resources/static/css/test-login.css(1 hunks)src/main/resources/static/js/test-mypage.js(1 hunks)src/main/resources/templates/test-mypage.html(1 hunks)src/main/resources/templates/test-reset-password.html(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: NCookies
PR: ezcode-my/backend#17
File: src/main/java/org/ezcode/codetest/presentation/problemmanagement/ProblemAdminController.java:25-25
Timestamp: 2025-06-02T06:57:37.929Z
Learning: 이 프로젝트에서는 아직 Spring Security가 적용되지 않아서 @PreAuthorize 어노테이션이 주석 처리되어 있음. Spring Security 도입 전까지는 권한 검증 어노테이션들이 비활성화 상태임.
Learnt from: minjee2758
PR: ezcode-my/backend#9
File: src/main/java/org/ezcode/codetest/domain/user/service/UserDomainService.java:32-39
Timestamp: 2025-05-31T11:26:06.360Z
Learning: AuthService.signin() 메서드에서 userPasswordCheck 호출 시 signinRequest.getPassword() (원본 비밀번호)를 전달해야 하는데, loginUser.getPassword() (해시된 비밀번호)를 전달하는 버그가 있었음.
src/main/java/org/ezcode/codetest/common/security/config/SecurityConfig.java (1)
Learnt from: NCookies
PR: ezcode-my/backend#17
File: src/main/java/org/ezcode/codetest/presentation/problemmanagement/ProblemAdminController.java:25-25
Timestamp: 2025-06-02T06:57:37.929Z
Learning: 이 프로젝트에서는 아직 Spring Security가 적용되지 않아서 @PreAuthorize 어노테이션이 주석 처리되어 있음. Spring Security 도입 전까지는 권한 검증 어노테이션들이 비활성화 상태임.
src/main/resources/templates/test-reset-password.html (2)
Learnt from: thezz9
PR: ezcode-my/backend#73
File: src/main/resources/templates/submit-test.html:135-135
Timestamp: 2025-06-18T11:41:58.457Z
Learning: src/main/resources/templates/submit-test.html 파일은 테스트용 파일로, isCorrect 값의 하드코딩은 의도적이며 사용자가 UI 요소를 통해 정답/오답 상태를 선택할 수 있도록 개선 예정입니다.
Learnt from: minjee2758
PR: ezcode-my/backend#9
File: src/main/java/org/ezcode/codetest/domain/user/service/UserDomainService.java:32-39
Timestamp: 2025-05-31T11:26:06.360Z
Learning: AuthService.signin() 메서드에서 userPasswordCheck 호출 시 signinRequest.getPassword() (원본 비밀번호)를 전달해야 하는데, loginUser.getPassword() (해시된 비밀번호)를 전달하는 버그가 있었음.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
src/main/java/org/ezcode/codetest/common/security/util/SecurityPath.java (1)
20-20: 변경사항이 적절합니다.
/api/problems/**경로를 공개 경로에서 제거하고 SecurityConfig에서 HTTP 메서드별로 세분화하여 처리하는 것이 PR 목표와 일치합니다. 주석을 통해 변경 이유도 명확히 설명되어 있습니다.src/main/resources/static/css/test-login.css (1)
214-284: 모달 스타일이 잘 구현되었습니다.새로 추가된 모달 스타일이 기존 디자인 시스템과 일관성을 유지하고 있으며, 사용자 경험을 고려한 호버 효과와 적절한 색상 조합이 적용되었습니다.
src/main/java/org/ezcode/codetest/common/security/config/SecurityConfig.java (1)
79-81: HTTP 메서드별 접근 제어가 올바르게 구현되었습니다.GET 요청은 비인증 접근을 허용하고, 나머지 HTTP 메서드는 인증을 요구하도록 세분화된 설정이 PR 목표와 정확히 일치합니다. Spring Security 규칙의 순서도 올바르게 구성되어 있습니다.
src/main/resources/templates/test-mypage.html (1)
30-30: 프로필 이미지 기능 비활성화에 대한 확인이 필요합니다.img 태그에서
id="profileImage"속성이 제거되었고, 연관된 JavaScript에서도 동적 업데이트가 비활성화되었습니다. 이것이 임시적인 변경인지 영구적인 기능 제거인지 확인이 필요하며, 사용자의 프로필 이미지 관리 기능에 미치는 영향을 검토해야 합니다.src/main/resources/static/js/test-mypage.js (1)
59-59: 프로필 이미지 기능 주석 처리에 대한 명확화가 필요합니다.HTML 템플릿의 변경사항과 일관성 있게 프로필 이미지 업데이트 기능이 비활성화되었습니다. 하지만 주석 처리된 코드를 유지할 것인지, 완전히 제거할 것인지 결정이 필요하며, 사용자 데이터의
profileImageUrl필드 활용 계획도 명확히 해야 합니다.src/main/resources/templates/test-reset-password.html (1)
195-200:response.json()두 번 호출 가능성에러 처리에서 이미
await response.json()으로 스트림을 소비했으므로, 이후 성공 경로에서도await response.json()을 다시 호출하면 스트림이 비어 있어 예외가 날 수 있습니다. 한 번만 파싱하거나clone()을 사용하세요.- const result = await response.json(); + const result = await response.clone().json();또는 성공·실패 분기 전에 한 번만 파싱하도록 구조를 바꾸는 것이 명확합니다.
Comment on lines
+156
to
+164
| const modalHtml = ` | ||
| <div id="success-modal" class="modal" style="display: flex; align-items: center; justify-content: center;"> | ||
| <div class="modal-content"> | ||
| <h3>완료</h3> | ||
| <p>${message}</p> | ||
| </div> | ||
| </div> | ||
| `; | ||
| document.body.insertAdjacentHTML('beforeend', modalHtml); |
There was a problem hiding this comment.
XSS 취약점: 서버 메시지를 그대로 innerHTML 삽입
${message}가 escape 없이 모달에 삽입되어 있습니다. 악의적인 스크립트가 포함될 경우 클라이언트에서 실행될 수 있습니다.
- <p>${message}</p>
+ <p id="reset-msg"></p>그리고 JS 쪽에서:
- showSuccessModal(result.message || "비밀번호가 성공적으로 변경되었습니다.");
+ showSuccessModal(result.message ?? "비밀번호가 성공적으로 변경되었습니다.");
function showSuccessModal(message) {
const modalHtml = `
<div id="success-modal" class="modal" style="display:flex;align-items:center;justify-content:center;">
<div class="modal-content">
<h3>완료</h3>
<p id="reset-msg"></p>
</div>
</div>`;
document.body.insertAdjacentHTML('beforeend', modalHtml);
document.getElementById('reset-msg').textContent = message; // textContent 로 XSS 차단
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/resources/templates/test-reset-password.html around lines 156 to
164, the code inserts the server-provided message directly into the modal's
innerHTML without escaping, causing an XSS vulnerability. To fix this, sanitize
or escape the message content before inserting it into the HTML, or use
textContent assignment instead of innerHTML to safely display the message
without executing any embedded scripts.
thezz9
approved these changes
Jul 20, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
문제 상황
/api/problems&/api/problems/{problemId}경로 접근에 문제 발생현 상황
이 경로가 PUBLIC_PATH에 포함되어있음
문제 목록(GET)은 가능함
문제 발생
문제 제출(POST) 불가능
"로그인 후 이용 가능합니다" 예외 대신 "토큰이 필요합니다"라는 메시지가 반환
-> JWT 필터 그냥 넘어가지니까 (인증되지 않은 상태) 제출 로직에서 인증이 필요하여 발생
해결 방법
Spring Security의 authorizeHttpRequests 설정에서 경로별 HTTP 메서드를 구분
(요청마다 인증이 필요한거 없는거 나누기)
코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit
신규 기능
버그 수정
스타일
접근 제어