fix: 잘못된 비밀번호 검증 로직과 잘못된 비밀번호 테스트 수정#467
Merged
Zepelown merged 1 commit intodevelop/befrom Jun 4, 2025
Merged
Conversation
Zepelown
added
✅ Test
Contributor
Walkthrough비밀번호 유효성 검사 로직이 강화되어 공백 문자를 명시적으로 허용하지 않도록 정규식이 수정되었고, 숫자 매칭 표현이 변경되었습니다. 비밀번호 유효성 검사기의 null 또는 빈 문자열 처리 방식이 변경되었으며, 이에 맞춘 단위 테스트가 새로 추가되고 기존 테스트 케이스 일부가 조정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant 테스트 as PasswordValidatorTest
participant Validator as PasswordValidator
테스트->>Validator: isValid(비밀번호)
alt 비밀번호가 null 또는 빈 문자열
Validator-->>테스트: false 반환
else 정규식 불일치
Validator-->>테스트: false 반환
else 정규식 일치
Validator-->>테스트: true 반환
end
Possibly related PRs
Suggested labels
Suggested reviewers
✨ 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 (
|
Test Results77 tests 77 ✅ 2s ⏱️ Results for commit 69e2df0. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/src/test/java/moadong/unit/user/PasswordValidatorTest.java (1)
15-19: 유효한 비밀번호 테스트가 적절합니다.
UserFixture.collectPassword를 사용하여 유효한 비밀번호를 테스트하는 것이 좋습니다. 하지만 테스트 안정성을 위해 고정된 유효한 비밀번호 값도 추가로 테스트하는 것을 고려해보세요.추가 테스트 케이스 예시:
@Test void 유효한_비밀번호는_유효하다() { boolean isValid = passwordValidator.isValid(UserFixture.collectPassword, null); Assertions.assertThat(isValid).isTrue(); + + // 명시적인 유효한 비밀번호 테스트 + boolean isValidExplicit = passwordValidator.isValid("Abcd1234!", null); + Assertions.assertThat(isValidExplicit).isTrue(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/src/main/java/moadong/global/validator/PasswordValidator.java(1 hunks)backend/src/test/java/moadong/unit/user/PasswordValidatorTest.java(1 hunks)backend/src/test/java/moadong/unit/user/UserRegisterTest.java(0 hunks)
💤 Files with no reviewable changes (1)
- backend/src/test/java/moadong/unit/user/UserRegisterTest.java
🔇 Additional comments (3)
backend/src/main/java/moadong/global/validator/PasswordValidator.java (2)
10-11: 정규식 패턴이 크게 개선되었습니다!새로운 정규식의 주요 개선사항:
(?!.*\\s)추가로 공백 문자를 명시적으로 금지\d를[0-9]로 변경하여 더 명확한 숫자 매칭- 상수로 추출하여 코드 가독성과 유지보수성 향상
정규식 패턴이 보안 요구사항을 정확히 반영하고 있습니다.
14-16: null/empty 문자열 처리 로직이 올바르게 수정되었습니다.이전에 null/empty 문자열에 대해
true를 반환했던 것을false로 변경한 것은 보안상 올바른 결정입니다. 비밀번호 유효성 검사에서 null이나 빈 문자열은 유효하지 않은 입력으로 처리되어야 합니다.backend/src/test/java/moadong/unit/user/PasswordValidatorTest.java (1)
21-39: 무효한 비밀번호 테스트 케이스가 매우 포괄적입니다!테스트 케이스들이 정규식의 모든 검증 조건을 훌륭하게 다루고 있습니다:
- 길이 제한 (너무 짧거나 긴 경우)
- 문자 조합 요구사항 (영문, 숫자, 특수문자)
- 공백 문자 금지
- 허용되지 않은 특수문자 검증
테스트 커버리지가 우수하고 각 케이스에 명확한 주석이 포함되어 있어 이해하기 쉽습니다.
Comment on lines
+21
to
+39
| @ParameterizedTest | ||
| @ValueSource(strings = { | ||
| "short1!", // 7자 (길이 부족) | ||
| "longpassword1234567890!", // 21자 (길이 초과) | ||
| "abcdefgh", // 영문 소문자만 (숫자 없음) | ||
| "ABCDEFGH", // 영문 대문자만 (숫자 없음) | ||
| "12345678", // 숫자만 (영문 없음) | ||
| "Abcdefgh", // 영문만 (숫자 없음) | ||
| "abcd1234*", // 허용되지 않은 특수문자 `*` | ||
| "abc def123!", // 공백 포함 | ||
| "passWord!", // 특수문자 있음, 숫자 없음 | ||
| "1234!@#$", // 숫자 + 특수문자, 문자 없음 | ||
| "Abcdef12()", // 괄호 포함 (허용되지 않은 특수문자) | ||
| "ABCD1234~", // `~` 특수문자 허용 안됨 | ||
| }) | ||
| void 유효하지_않은_비밀번호는_실패한다(String password) { | ||
| boolean isValid = passwordValidator.isValid(password, null); | ||
| Assertions.assertThat(isValid).isFalse(); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
null과 빈 문자열에 대한 테스트 케이스를 추가하세요.
현재 테스트는 다양한 무효한 문자열 패턴을 다루고 있지만, 변경된 null/empty 처리 로직에 대한 명시적인 테스트가 누락되어 있습니다.
다음 테스트 케이스들을 추가하는 것을 권장합니다:
@ParameterizedTest
@ValueSource(strings = {
"short1!", // 7자 (길이 부족)
"longpassword1234567890!", // 21자 (길이 초과)
// ... 기존 케이스들
})
void 유효하지_않은_비밀번호는_실패한다(String password) {
boolean isValid = passwordValidator.isValid(password, null);
Assertions.assertThat(isValid).isFalse();
}
+@Test
+void null_또는_빈_비밀번호는_유효하지_않다() {
+ Assertions.assertThat(passwordValidator.isValid(null, null)).isFalse();
+ Assertions.assertThat(passwordValidator.isValid("", null)).isFalse();
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @ParameterizedTest | |
| @ValueSource(strings = { | |
| "short1!", // 7자 (길이 부족) | |
| "longpassword1234567890!", // 21자 (길이 초과) | |
| "abcdefgh", // 영문 소문자만 (숫자 없음) | |
| "ABCDEFGH", // 영문 대문자만 (숫자 없음) | |
| "12345678", // 숫자만 (영문 없음) | |
| "Abcdefgh", // 영문만 (숫자 없음) | |
| "abcd1234*", // 허용되지 않은 특수문자 `*` | |
| "abc def123!", // 공백 포함 | |
| "passWord!", // 특수문자 있음, 숫자 없음 | |
| "1234!@#$", // 숫자 + 특수문자, 문자 없음 | |
| "Abcdef12()", // 괄호 포함 (허용되지 않은 특수문자) | |
| "ABCD1234~", // `~` 특수문자 허용 안됨 | |
| }) | |
| void 유효하지_않은_비밀번호는_실패한다(String password) { | |
| boolean isValid = passwordValidator.isValid(password, null); | |
| Assertions.assertThat(isValid).isFalse(); | |
| } | |
| @ParameterizedTest | |
| @ValueSource(strings = { | |
| "short1!", // 7자 (길이 부족) | |
| "longpassword1234567890!", // 21자 (길이 초과) | |
| "abcdefgh", // 영문 소문자만 (숫자 없음) | |
| "ABCDEFGH", // 영문 대문자만 (숫자 없음) | |
| "12345678", // 숫자만 (영문 없음) | |
| "Abcdefgh", // 영문만 (숫자 없음) | |
| "abcd1234*", // 허용되지 않은 특수문자 `*` | |
| "abc def123!", // 공백 포함 | |
| "passWord!", // 특수문자 있음, 숫자 없음 | |
| "1234!@#$", // 숫자 + 특수문자, 문자 없음 | |
| "Abcdef12()", // 괄호 포함 (허용되지 않은 특수문자) | |
| "ABCD1234~", // `~` 특수문자 허용 안됨 | |
| }) | |
| void 유효하지_않은_비밀번호는_실패한다(String password) { | |
| boolean isValid = passwordValidator.isValid(password, null); | |
| Assertions.assertThat(isValid).isFalse(); | |
| } | |
| @Test | |
| void null_또는_빈_비밀번호는_유효하지_않다() { | |
| Assertions.assertThat(passwordValidator.isValid(null, null)).isFalse(); | |
| Assertions.assertThat(passwordValidator.isValid("", null)).isFalse(); | |
| } |
🤖 Prompt for AI Agents
In backend/src/test/java/moadong/unit/user/PasswordValidatorTest.java around
lines 21 to 39, the test cases lack coverage for null and empty string inputs,
which are important for validating the updated null/empty handling logic. Add
parameterized test cases for null and empty strings to the existing invalid
password test method or create a separate test method to explicitly assert that
these inputs are considered invalid by the passwordValidator.isValid method.
Due-IT
approved these changes
Jun 3, 2025
Collaborator
Due-IT
left a comment
Due-IT
left a comment
There was a problem hiding this comment.
정규식을 사용한 검증과 java 함수를 사용한 검증의 차이는 무엇일까요? 왜 정규식을 선택하셨는지도 궁금하네요
Member
Author
코드를 매우 간단히 만들고자 정규식을 사용하였습니다. |
This was referenced Jun 6, 2025
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
#️⃣연관된 이슈
#455
📝작업 내용
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
버그 수정
테스트