Skip to content

refactor(auth): 일관되게 변경. if 절에서 throw 하도록 하여 가독성 향상#279

Merged
huhdy32 merged 2 commits intodevelopfrom
refactor/auth
Jan 9, 2026
Merged

refactor(auth): 일관되게 변경. if 절에서 throw 하도록 하여 가독성 향상#279
huhdy32 merged 2 commits intodevelopfrom
refactor/auth

Conversation

@huhdy32
Copy link
Collaborator

@huhdy32 huhdy32 commented Jan 9, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Corrected login authentication flow to reliably detect and record password mismatches, updating failure counts and account status to better prevent unauthorized access.
    • Improved successful-login handling to properly unlock accounts, generate session tokens, and log events for more reliable sign-ins.

✏️ Tip: You can customize this high-level summary in your review settings.

@huhdy32 huhdy32 self-assigned this Jan 9, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 9, 2026

📝 Walkthrough

Walkthrough

The password validation in the login service was reordered to check for password mismatch first (updates failure counts, persists member, throws exception); on match the member is unlocked and JWT login is generated. (Reordered control flow, no public API changes.)

Changes

Cohort / File(s) Summary
Login Authentication Logic Reordering
domain/mathrank-auth-domain/src/main/java/kr/co/mathrank/domain/auth/service/LoginService.java
Reordered password check: explicit mismatch handling first (increment failure count, save member, log warning, throw PasswordMismatchedException), then success path (unlock member, log info, call jwtLoginManager.login). +11 / -10 lines.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

"I hopped through code in morning light,
Found the check that wasn't quite right.
Mismatch first, then keys unfurl,
Tokens dance — a secure little whirl.
— Rabbit, guarding the auth tonight 🐇🔐"

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: refactoring auth logic for consistency and improved readability by using throw statements in if conditions.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c2aaab4 and a0d1275.

📒 Files selected for processing (1)
  • domain/mathrank-auth-domain/src/main/java/kr/co/mathrank/domain/auth/service/LoginService.java
⏰ 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: test
🔇 Additional comments (2)
domain/mathrank-auth-domain/src/main/java/kr/co/mathrank/domain/auth/service/LoginService.java (2)

48-55: LGTM! Guard clause pattern improves readability.

The refactor to check password mismatch first (early exit pattern) is a good improvement. The logic correctly:

  • Increments the failed login count
  • Persists the state change before throwing
  • Logs the failure appropriately
  • Throws the exception with remaining try count

The explicit save() before throwing is appropriate given the noRollbackFor annotation ensures the transaction commits.


57-59: No issue — unlock state is correctly persisted.

The apparent asymmetry between the explicit memberRepository.save(member) on Line 51 and the absence of it on Line 57 is intentional and correct.

On Line 51, the save is required because PasswordMismatchedException is thrown with noRollbackFor = true in the method's @Transactional annotation. This means the transaction commits despite the exception, so changes must be explicitly saved before throwing to ensure they persist.

On Line 57, no explicit save is needed because the method completes successfully and the transaction commits normally. Since member is a managed entity obtained from the repository within the transactional context, JPA automatically persists all changes (including the embedded LockInfo.unlock() modifications) through dirty checking on commit.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @huhdy32, 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!

이 풀 리퀘스트는 코드 가독성과 유지보수성을 높이기 위해 LoginService 내의 인증 로직을 리팩토링합니다. 비밀번호 일치 여부에 대한 조건부 검사를 역전시킴으로써, 시스템은 이제 가드 절 패턴을 활용하여 오류 조건이 예외와 함께 미리 처리되도록 보장하며, 성공적인 로그인 시도에 대한 흐름을 더욱 명확하게 만듭니다.

Highlights

  • 로그인 로직 리팩토링: LoginService.java 파일의 로그인 메서드 가독성을 향상시키기 위해 로직이 리팩토링되었습니다.
  • 가드 절 구현: 비밀번호 불일치 처리 로직이 역전되어, 비밀번호가 일치하지 않을 경우 즉시 PasswordMismatchedException을 던지는 가드 절 패턴을 사용합니다.
  • 일관된 오류 처리: 이 변경은 실패 조건을 명시적으로 확인하고 예외를 즉시 발생시키는 보다 일관된 오류 처리 패턴을 촉진합니다.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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

  1. 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.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

안녕하세요. login 메서드를 guard clause 패턴을 사용해 리팩터링하여 가독성을 높인 점이 좋습니다. 실패 케이스를 먼저 처리하고 함수를 종료함으로써 주된 로직의 흐름이 더 명확해졌습니다. 코드 변경 사항을 검토했으며, 한 가지 사소한 개선점을 제안합니다. if 블록 내에 불필요한 주석이 있어 제거하면 코드가 더 깔끔해질 것 같습니다. 전반적으로 좋은 변경입니다.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
domain/mathrank-auth-domain/src/main/java/kr/co/mathrank/domain/auth/service/LoginService.java (2)

48-50: Remove duplicate comment.

Lines 48 and 50 contain identical comments ("// 비밀번호 불일치"). The comment on line 48 is sufficient; the one on line 50 should be removed.

♻️ Proposed fix
 		// 비밀번호 불일치
 		if (!isMatch(command.password(), member.getPassword())) {
-			// 비밀번호 불일치
 			member.getLockInfo().addFailedCount(now);
 			memberRepository.save(member);

58-58: Consider explicit save for consistency.

On line 52, the member is explicitly saved after updating the failed count. Here, the unlock relies on JPA auto-flush at transaction commit. While functionally correct, consider explicitly saving the member after unlocking for consistency and clarity.

♻️ Proposed fix
 		member.getLockInfo().unlock();
+		memberRepository.save(member);
 		log.info("[LoginService.login] member login success: {}", member.getId());
 		return jwtLoginManager.login(member.getId(), member.getRole(), member.getName());
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2dac2ce and c2aaab4.

📒 Files selected for processing (1)
  • domain/mathrank-auth-domain/src/main/java/kr/co/mathrank/domain/auth/service/LoginService.java
⏰ 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: test

@huhdy32 huhdy32 enabled auto-merge (squash) January 9, 2026 07:16
@huhdy32 huhdy32 merged commit 239a084 into develop Jan 9, 2026
2 checks passed
@huhdy32 huhdy32 deleted the refactor/auth branch January 9, 2026 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant