MOSU-352 refactor: 비밀번호 패턴 대문자 필수 제거#353
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedThe pull request is closed. WalkthroughRelaxed password regex: require at least one English letter of any case (instead of separate lower+upper); updated Swagger descriptions for password fields to match. Modified OAuth duplicate handling to unconditionally reject existing MOSU users with an OAuthException("DUPLICATE"); removed provider/pending branching for MOSU duplicates. No public API signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Validation
participant RegexEngine
Client->>Controller: POST /signup or /user/change-password (payload)
Controller->>Validation: Validate request fields (PasswordPattern)
Validation->>RegexEngine: Apply updated regex (?=.*[A-Za-z])(?=.*\d)...{8,20}
RegexEngine-->>Validation: pass / fail
Validation-->>Controller: validation result
Controller-->>Client: 200 OK or 4xx validation error
note right of RegexEngine #F0F7FF: Regex now accepts any letter case if other rules met
sequenceDiagram
participant OAuthClient
participant OAuthProcessor
participant UserRepo
OAuthClient->>OAuthProcessor: OAuth callback / token (with provider info)
OAuthProcessor->>UserRepo: findByEmail(...)
UserRepo-->>OAuthProcessor: existingUser (found)
alt existingUser.isMosuUser() == true
OAuthProcessor-->>OAuthClient: throw OAuthException("DUPLICATE")
note right of OAuthProcessor #FFF7F0: Unconditional MOSU duplicate rejection
else
OAuthProcessor->>UserRepo: update existingUser OR create new user
UserRepo-->>OAuthProcessor: persisted user
OAuthProcessor-->>OAuthClient: success
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Summary of Changes
Hello @chominju02, 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!
이 풀 리퀘스트는 비밀번호 유효성 검사 규칙을 간소화하기 위해 PasswordPattern 어노테이션에 정의된 정규 표현식을 수정합니다. 주요 변경 사항은 비밀번호에 대문자가 반드시 포함되어야 한다는 요구 사항을 제거하여 사용자가 더 유연하게 비밀번호를 설정할 수 있도록 하는 것입니다.
Highlights
- 비밀번호 패턴 변경: 기존 비밀번호 패턴 정규식에서 대문자 필수 포함 조건인
(?=.*[A-Z])이 제거되었습니다. - 업데이트된 유효성 검사 규칙: 새로운 비밀번호 패턴은 최소 하나의 영문자(대소문자 무관), 하나의 숫자, 하나의 특수문자를 포함하며 길이는 8자에서 20자 사이여야 합니다.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
src/main/java/life/mosu/mosuserver/global/annotation/PasswordPattern.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/life/mosu/mosuserver/global/annotation/PasswordPattern.java (2)
19-19: Update the default validation message to match the new rule.The text still says “영문 대/소문자 … 모두 포함” (both upper and lower), which no longer reflects the regex. Please align the copy.
Apply this diff:
- String message() default "비밀번호는 8~20자의 영문 대/소문자, 숫자, 특수문자를 모두 포함해야 합니다."; + String message() default "비밀번호는 8~20자이며 영문자, 숫자, 특수문자를 각각 최소 1자 이상 포함해야 합니다.";
1-24: Ensure Consistent Password Requirements in DTO DescriptionsThe repo sweep confirms that all occurrences of the new
PasswordPatternannotation and its Korean copy match the intended “영문 대/소문자, 숫자, 특수문자” requirement—except for one outdated schema description inLoginRequest.java. Please update it to align with the new pattern.• Files already correct (no change needed):
–ChangePasswordRequest.java(schema description uses “영문 대/소문자…”)
–SignUpAccountRequest.java(schema description uses “영문 대/소문자…”)
–PasswordPattern.java(default message uses “영문 대/소문자…”)• File requiring update:
–src/main/java/life/mosu/mosuserver/presentation/auth/dto/request/LoginRequest.javaDiff suggestion:
@Schema( - description = "비밀번호는 8~20자의 영문, 숫자, 특수문자(!@#$%^&*)만 사용 가능합니다.", + description = "비밀번호는 8~20자의 영문 대/소문자, 숫자, 특수문자(!@#$%^&*)만 사용 가능합니다.", example = "Password123!" ) @PasswordPattern String passwordNo other regex lookaheads or Korean copy referring to “대문자/소문자” were found outside these files. After applying this refactor, all DTOs and docs will consistently enforce and describe the same password policy.
🧹 Nitpick comments (3)
src/main/java/life/mosu/mosuserver/global/annotation/PasswordPattern.java (3)
3-6: Optional: Report a single violation with one consistent message.If you want a single, consistent error message instead of separate @notblank and @pattern messages, add @ReportAsSingleViolation and rely on the annotation’s default message (updated above). This also helps the frontend because the rule copy lives in one place.
Apply this diff:
import jakarta.validation.Constraint; +import jakarta.validation.ReportAsSingleViolation; import jakarta.validation.Payload; import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.Pattern; @@ -@Pattern(regexp = "^(?=.*[A-Za-z])(?=.*\\d)(?=.*[!@#$%^&*_/+=])[A-Za-z\\d!@#$%^&*_/+=]{8,20}$", message = "비밀번호 형식이 올바르지 않습니다.") +@Pattern(regexp = "^(?=.*[A-Za-z])(?=.*\\d)(?=.*[!@#$%^&*_/+=])[A-Za-z\\d!@#$%^&*_/+=]{8,20}$") @NotBlank +@ReportAsSingleViolation @Target({ElementType.FIELD, ElementType.PARAMETER})Note: remove the custom message on @pattern so only the annotation message is exposed.
Also applies to: 12-16
13-13: Optional: Localize the @notblank message for consistency.If you don’t adopt single-violation reporting, consider adding a localized NotBlank message so users don’t see provider defaults.
-@NotBlank +@NotBlank(message = "비밀번호는 비어 있을 수 없습니다.")
1-24: Test coverage nudge: add cases proving lowercase-only letters pass (with digit + special).Consider adding validator tests like:
- Pass:
abc123!@(length ≥ 8, has letter, digit, special)- Pass:
ABC123!@(still valid; uppercase allowed but not required)- Fail:
abcdefg!@(no digit)- Fail:
abcd1234(no special)- Fail:
a1!短(non-ASCII char should fail under current char class)- Boundary: length 8 and 20 exactly
I can draft a JUnit test using Jakarta Validator if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/main/java/life/mosu/mosuserver/global/annotation/PasswordPattern.java(1 hunks)
🔇 Additional comments (2)
src/main/java/life/mosu/mosuserver/global/annotation/PasswordPattern.java (2)
12-12: Regex update matches the PR intent (uppercase no longer required).The lookahead now checks for “at least one letter of any case” via
(?=.*[A-Za-z]), keeping the digit/special/length constraints intact. Change is precise and low risk.
12-12: Confirm the special character set is final.Allowed specials are
[!@#$%^&*_/+=]. Hyphen (-), period (.), parentheses, etc., are excluded. If that’s intentional, all good; if not, update both the lookahead and the allowed-character class in sync.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/life/mosu/mosuserver/presentation/user/dto/request/ChangePasswordRequest.java (1)
7-12: Sync PasswordPattern messages with the updated rule
In
src/main/java/life/mosu/mosuserver/global/annotation/PasswordPattern.java
• Change the default message() from
"비밀번호는 8~20자의 영문 대/소문자, 숫자, 특수문자를 모두 포함해야 합니다."
to
"비밀번호는 8~20자의 영문, 숫자, 특수문자를 모두 포함해야 합니다."
• (Optional) For consistency, you may also align the@Patternmeta-annotation’s message attribute to the same text, or use@OverridesAttributeto delegate the composed constraint’s message.Verified that both
SignUpAccountRequestandChangePasswordRequestuse Schema descriptions without “대문자” and already reflect the rule:
•"비밀번호는 8~20자의 영문, 숫자, 특수문자를 모두 포함해야 합니다."No Markdown or other in-repo documentation mentions password rules; if you maintain external docs or a wiki, update any password-rule text there and consider calling out the exact allowed special characters (
!@#$%^&*_/+=).Let me know if you’d like a small parameterized test suite for valid/invalid password samples.
🧹 Nitpick comments (2)
src/main/java/life/mosu/mosuserver/presentation/user/dto/request/ChangePasswordRequest.java (2)
8-10: Make the change explicit and example illustrative (uppercase not required).Minor copy tweak to preempt confusion from previous policy and to showcase a lowercase-only example.
Apply this diff:
- description = "새로운 비밀번호는 8~20자의 영문, 숫자, 특수문자를 모두 포함해야 합니다.", - example = "Mosu!1234" + description = "새로운 비밀번호는 8~20자의 영문(대소문자 구분 없음), 숫자, 특수문자를 모두 포함해야 합니다.", + example = "mosu!1234"
7-11: Surface length constraints in the OpenAPI schema.Adding minLength/maxLength helps client generators and Swagger UI show constraints without reading prose.
Apply this diff:
- example = "Mosu!1234" + example = "Mosu!1234", + minLength = 8, + maxLength = 20
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/life/mosu/mosuserver/presentation/auth/dto/request/SignUpAccountRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/user/dto/request/ChangePasswordRequest.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/life/mosu/mosuserver/presentation/auth/dto/request/SignUpAccountRequest.java
🔇 Additional comments (1)
src/main/java/life/mosu/mosuserver/presentation/user/dto/request/ChangePasswordRequest.java (1)
7-11: Docs now reflect “letter required” (not “uppercase required”) — LGTMThe updated Swagger description aligns with the new relaxed policy and keeps the example valid.
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit