feat: 학교 이메일 인증으로 HomeUniversity 자동 매핑#752
Open
sukangpunch wants to merge 11 commits into
Open
Conversation
- HomeUniversity에 emailDomain 컬럼 추가 (V50 마이그레이션) - HomeUniversityRepository에 findByEmailDomain 메서드 추가 - EmailService 구현 (JavaMailSender 기반 인증 코드 발송) - SchoolEmailService 구현 (인증 코드 발급/확인, Redis TTL 5분) - SiteUser.verifySchool() 도메인 메서드 추가 - MyPageController에 POST /my/school-email, POST /my/school-email/confirm 엔드포인트 추가 - 어드민 HomeUniversity DTO에 emailDomain 필드 반영 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
email 발송은 공통 인프라 관심사이므로 별도 email 패키지 대신 common/mail 패키지로 이동 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
클라이언트가 이미 알고 있는 이메일을 응답으로 돌려줄 필요가 없으므로 SchoolEmailResponse 제거 및 반환 타입을 void로 변경 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
RuntimeException 대신 SCHOOL_EMAIL_VERIFICATION_INFO_SAVE_FAILED ErrorCode를 사용하여 예외 처리를 명확하게 표현 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
데이터가 존재하지만 파싱 실패인 경우 REQUEST_NOT_FOUND 대신 SCHOOL_EMAIL_VERIFICATION_INFO_CORRUPTED로 명확하게 구분 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sukangpunch
requested review from
Gyuhyeok99,
Hexeong,
JAEHEE25,
lsy1307,
whqtker and
wibaek
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c52ee3c0a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE home_university | |||
| ADD COLUMN email_domain VARCHAR(100) NULL UNIQUE; | |||
There was a problem hiding this comment.
In the dev/prod profiles I inspected (application-db.yml), Flyway runs but data.sql is not initialized, so this migration leaves all existing home_university rows with email_domain = NULL. The new verification path then calls findByEmailDomain(...), so after deployment supported existing schools such as Inha/Incheon will be rejected as unsupported unless someone manually edits every row; include the domain UPDATEs in the Flyway migration rather than only in data.sql.
Useful? React with 👍 / 👎.
| if (atIndex == -1) { | ||
| throw new CustomException(SCHOOL_EMAIL_DOMAIN_NOT_SUPPORTED); | ||
| } | ||
| return email.substring(atIndex + 1); |
There was a problem hiding this comment.
When a user submits a valid school email with uppercase letters in the domain (for example student@INHA.EDU), validation still accepts it, but this exact substring is used for findByEmailDomain while the stored domains are lowercase (inha.edu, inu.ac.kr, etc.). Because email domains are case-insensitive, supported users can be incorrectly rejected as SCHOOL_EMAIL_DOMAIN_NOT_SUPPORTED; lower-case the extracted domain before the repository lookup.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
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/com/example/solidconnection/admin/university/service/AdminHomeUniversityService.java (1)
68-74:⚠️ Potential issue | 🟠 Major | ⚡ Quick winupdate에서도 emailDomain 중복 검증 필요
생성 시와 마찬가지로, 업데이트 시에도
emailDomain중복 검증이 없습니다.다른 대학의 도메인으로 변경할 때 충돌이 발생할 수 있습니다.
🛡️ 제안: update에 중복 검증 추가
public AdminHomeUniversityResponse updateHomeUniversity(Long id, AdminHomeUniversityUpdateRequest request) { HomeUniversity homeUniversity = homeUniversityRepository.findById(id) .orElseThrow(() -> new CustomException(HOME_UNIVERSITY_NOT_FOUND)); validateNameNotDuplicated(request.name(), id); + if (request.emailDomain() != null && !request.emailDomain().isBlank()) { + validateEmailDomainNotDuplicated(request.emailDomain(), id); + } homeUniversity.update(request.name(), request.emailDomain()); return AdminHomeUniversityResponse.from(homeUniversity); } +private void validateEmailDomainNotDuplicated(String emailDomain, Long excludeId) { + homeUniversityRepository.findByEmailDomain(emailDomain) + .ifPresent(existing -> { + if (!existing.getId().equals(excludeId)) { + throw new CustomException(HOME_UNIVERSITY_EMAIL_DOMAIN_ALREADY_EXISTS); + } + }); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/example/solidconnection/admin/university/service/AdminHomeUniversityService.java` around lines 68 - 74, The updateHomeUniversity method lacks duplicate emailDomain validation; add a check similar to name validation (e.g., call a validateEmailDomainNotDuplicated(emailDomain, id) or reuse an existing validator) before calling homeUniversity.update(request.name(), request.emailDomain()), and throw the same domain-duplicate CustomException (e.g., EMAIL_DOMAIN_DUPLICATED) if another HomeUniversity already uses request.emailDomain(); ensure the new validator accepts the current entity id to allow keeping the same domain on self-updates.
🧹 Nitpick comments (7)
src/main/java/com/example/solidconnection/common/mail/MailService.java (2)
14-20: ⚡ Quick win발신자(from) 주소를 명시적으로 설정하는 것을 권장합니다.
현재
setFrom()을 호출하지 않아 SMTP 서버의 기본 설정에 의존하고 있습니다. 일부 메일 서버는 발신자 주소가 명시되지 않으면 전송을 거부하거나 스팸으로 분류할 수 있습니다.♻️ 개선 제안
public void sendVerificationEmail(String to, String verificationCode) { SimpleMailMessage message = new SimpleMailMessage(); + message.setFrom("noreply@solidconnect.com"); // 또는 application.yml에서 주입 message.setTo(to); message.setSubject("[Solid Connect] 학교 이메일 인증");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/example/solidconnection/common/mail/MailService.java` around lines 14 - 20, The sendVerificationEmail method relies on the SMTP default sender and doesn't set an explicit from address; update sendVerificationEmail to call message.setFrom(...) with a configured sender (e.g., a property or constant) before javaMailSender.send(message), ensuring the from address is read from configuration (or a class-level constant) so message.setFrom is always set when sendVerificationEmail is invoked.
14-20: ⚡ Quick win이메일 발송 실패 시 로깅을 추가하는 것을 고려해보세요.
javaMailSender.send()에서 예외가 발생하면 호출자에게 전파되지만, 발송 성공/실패를 추적할 로그가 없어 운영 중 문제 진단이 어려울 수 있습니다. SchoolEmailService에서 이미 Redis에 인증 코드를 저장한 후 이메일 발송이 실패하면 사용자는 코드를 받지 못한 채 Redis에만 저장된 상태가 됩니다.♻️ 개선 제안
+import lombok.extern.slf4j.Slf4j; + +@Slf4j `@Service` `@RequiredArgsConstructor` public class MailService { private final JavaMailSender javaMailSender; public void sendVerificationEmail(String to, String verificationCode) { SimpleMailMessage message = new SimpleMailMessage(); message.setTo(to); message.setSubject("[Solid Connect] 학교 이메일 인증"); message.setText("인증 코드: " + verificationCode + "\n\n인증 코드는 5분간 유효합니다."); - javaMailSender.send(message); + try { + javaMailSender.send(message); + log.info("Verification email sent to: {}", to); + } catch (Exception e) { + log.error("Failed to send verification email to: {}", to, e); + throw e; // 재전송 로직이 없으므로 예외 전파 + } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/example/solidconnection/common/mail/MailService.java` around lines 14 - 20, Wrap the javaMailSender.send call in MailService.sendVerificationEmail with a try-catch to log both success and failures: catch MailException (or Exception), call the class logger to record an informational log on successful send and an error log with the exception details on failure, then rethrow or translate the exception as appropriate. Also update the caller flow in SchoolEmailService (the code path that stores the verification code to Redis) to handle send failures (e.g., delete the Redis entry or abort store) so Redis doesn't retain codes when email delivery failed; reference MailService.sendVerificationEmail, javaMailSender.send and SchoolEmailService in your changes.src/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityCreateRequest.java (1)
10-11: 💤 Low value이메일 도메인 형식 검증을 추가하는 것을 고려해보세요.
현재
emailDomain필드는 길이만 검증하고 있어 관리자가"user@inha.edu"또는"http://inha.edu"같은 잘못된 형식을 입력할 수 있습니다.SchoolEmailService.extractEmailDomain()은"@"기준으로 도메인을 추출하므로, 도메인만 저장되어야 정상 매칭됩니다.♻️ 개선 제안
+import jakarta.validation.constraints.Pattern; + public record AdminHomeUniversityCreateRequest( `@NotBlank`(message = "협정 대학명은 필수입니다") `@Size`(max = 100, message = "협정 대학명은 100자 이하여야 합니다") String name, `@Size`(max = 100, message = "이메일 도메인은 100자 이하여야 합니다") + `@Pattern`(regexp = "^[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$", message = "올바른 도메인 형식이 아닙니다 (예: inha.edu)") String emailDomain ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityCreateRequest.java` around lines 10 - 11, The emailDomain field in AdminHomeUniversityCreateRequest currently only enforces length; update validation to ensure it is a pure domain (no '@' or URL schemes) by adding a pattern-based constraint (e.g. a regex that allows hostnames like example.edu and disallows '@' and 'http(s)://') and a clear validation message; ensure the Pattern is applied to the field named emailDomain in class AdminHomeUniversityCreateRequest so SchoolEmailService.extractEmailDomain() will match stored values correctly.src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java (1)
162-164: ⚡ Quick win도메인 메서드에 불변식 검증 추가 고려
현재
verifySchool메서드는 검증 없이homeUniversityId를 설정합니다.잠재적 문제:
- 이미 인증된 사용자의
homeUniversityId를 덮어쓸 수 있음null값 설정 가능- 도메인 객체가 자신의 일관성을 보장하지 못함
서비스 레이어(
SchoolEmailService)에서 이미homeUniversityId != null체크를 하지만, 도메인 메서드가 직접 호출될 경우를 대비한 방어 로직이 있으면 더욱 안전합니다.🔒 제안: 도메인 불변식 추가
public void verifySchool(Long homeUniversityId) { + if (homeUniversityId == null) { + throw new IllegalArgumentException("homeUniversityId는 null일 수 없습니다"); + } + if (this.homeUniversityId != null) { + throw new IllegalStateException("이미 학교 인증이 완료된 사용자입니다"); + } this.homeUniversityId = homeUniversityId; }또는 CustomException을 사용하려면:
public void verifySchool(Long homeUniversityId) { + if (this.homeUniversityId != null) { + throw new CustomException(SCHOOL_EMAIL_ALREADY_VERIFIED); + } this.homeUniversityId = homeUniversityId; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java` around lines 162 - 164, The verifySchool method on SiteUser currently sets homeUniversityId without validation; update SiteUser.verifySchool(Long homeUniversityId) to defend its invariants by (1) throwing an exception (e.g., IllegalStateException or a domain-specific CustomDomainException) if this.homeUniversityId is already non-null to prevent silent overwrite, and (2) validating the incoming homeUniversityId is non-null (throw IllegalArgumentException or equivalent) before assignment; keep the exception types consistent with the domain conventions used elsewhere in SiteUser or the project.src/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityUpdateRequest.java (1)
10-11: 💤 Low valueemailDomain 형식 검증 추가 고려
현재
@Size만 적용되어 있어 길이만 제한됩니다. 이메일 도메인 형식이 아닌 값(예: 공백, 특수문자만 포함된 문자열 등)도 통과할 수 있습니다.도메인 형식을 검증하는 패턴 매칭을 추가하면 데이터 품질을 높일 수 있습니다.
🔍 제안: 도메인 형식 검증 추가
`@Size`(max = 100, message = "이메일 도메인은 100자 이하여야 합니다") +@Pattern(regexp = "^[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$", message = "올바른 도메인 형식이 아닙니다") String emailDomain🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityUpdateRequest.java` around lines 10 - 11, The emailDomain field in AdminHomeUniversityUpdateRequest currently only has `@Size` and should also enforce domain format; add a `@Pattern` constraint on the emailDomain field (or create a custom validator if preferred) to reject blanks/whitespace and invalid characters and to ensure a valid domain label + TLD structure (e.g., labels separated by dots, allowed chars a-z0-9- and proper lengths) so the field is both length- and format-validated.src/test/java/com/example/solidconnection/admin/university/service/AdminHomeUniversityServiceTest.java (1)
104-117: ⚡ Quick winemailDomain이 포함된 생성 케이스 테스트 추가 권장
현재 모든 테스트에서
emailDomain을null로 전달하고 있습니다. 학교 이메일 인증 기능이 정상적으로 동작하려면emailDomain이 설정되어야 하므로, 다음과 같은 추가 테스트 케이스를 고려해주세요:추가 테스트 시나리오:
emailDomain을 실제 값으로 설정하여 대학 생성 후, 응답에 해당 값이 포함되는지 확인emailDomain이 설정된 대학을 조회했을 때 올바른 값이 반환되는지 검증- 동일한
emailDomain으로 중복 생성 시도 시 예외 발생 여부 확인 (unique 제약 조건이 있다면)현재 테스트는 기존 기능의 호환성을 검증하기에 충분하지만, 새로운 필드의 동작을 명시적으로 확인하면 회귀 방지에 도움이 됩니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/example/solidconnection/admin/university/service/AdminHomeUniversityServiceTest.java` around lines 104 - 117, Add tests that exercise the emailDomain field: create a new test in AdminHomeUniversityServiceTest that constructs AdminHomeUniversityCreateRequest with a non-null emailDomain (e.g., "example.edu"), calls adminHomeUniversityService.createHomeUniversity(request), and asserts the returned AdminHomeUniversityResponse.emailDomain equals the value and the persisted HomeUniversity (fetched via homeUniversityRepository.findById(response.id())) has getEmailDomain() equal to the same value; also add a test that queries the service/repository for a university with emailDomain to verify retrieval returns the correct value, and a duplicate-creation test that attempts to create a second AdminHomeUniversityCreateRequest with the same emailDomain and asserts an exception is thrown (use assertThrows) to validate unique constraint behavior.src/main/java/com/example/solidconnection/siteuser/service/SchoolEmailService.java (1)
60-73: 예외로 인해 Redis 삭제가 누락된다는 우려 범위 수정
SiteUser.verifySchool(Long homeUniversityId)는this.homeUniversityId = homeUniversityId;단순 대입만 하므로, 여기서 예외가 발생해 72번redisTemplate.delete(...)가 스킵될 가능성은 낮습니다.redisTemplate.delete가 실행되지 않는 지점은verifySchool()호출 이전(예:code불일치,getVerificationInfo()에서 요청 정보 없음)에서 예외가 나는 경우입니다.
- 원 코멘트의 초점은
verifySchool보다는 “호출 이전 실패 케이스에서 TTL까지 Redis가 유지되는 정책”으로 옮겨주세요.- 보안 정책상 코드 불일치 시에도 즉시 정리(또는 재시도 허용 범위 조정)가 필요하다면 해당 케이스에 대해서만 삭제/try-finally 검토를 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/example/solidconnection/siteuser/service/SchoolEmailService.java` around lines 60 - 73, 리뷰 포인트를 verifySchool에서 발생할 예외로 좁히지 말고, 예외로 인해 Redis 삭제가 누락되는 주요 케이스는 confirmSchoolEmail 호출 이전(예: getVerificationInfo 실패 또는 코드 불일치)임을 명시하고, confirmSchoolEmail 내 getVerificationInfo, 코드 비교와 관련된 실패 시 Redis 키(KEY_PREFIX + siteUserId)의 보존/만료 정책을 검토하라는 지침을 남기세요; 보안상 코드 불일치에서 즉시 정리가 필요하면 confirmSchoolEmail 안의 코드 불일치 분기에서 redisTemplate.delete를 호출하거나(혹은 전체 흐름에서 안전히 정리하려면 try-finally로 감싸서 SiteUser.verifySchool 호출 전후의 예외와 상관없이 필요한 정리가 수행되도록 변경) 정책 결정에 따라 TTL 조정 권고(함수명: confirmSchoolEmail, getVerificationInfo, SiteUser.verifySchool, redisTemplate.delete, 상수: KEY_PREFIX 참조).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/example/solidconnection/admin/university/service/AdminHomeUniversityService.java`:
- Around line 49-53: Add duplicate-checking for emailDomain: implement
validateEmailDomainNotExists in AdminHomeUniversityService and call it at the
start of createHomeUniversity (alongside validateNameNotExists) to throw the new
HOME_UNIVERSITY_EMAIL_DOMAIN_ALREADY_EXISTS error when a matching emailDomain
already exists (use homeUniversityRepository.existsByEmailDomain or equivalent
repository query). Also add a DB-level UNIQUE constraint on the email_domain
column via a Flyway migration to enforce data integrity. Finally, add the new
ErrorCode enum value HOME_UNIVERSITY_EMAIL_DOMAIN_ALREADY_EXISTS(409, "이미 사용 중인
이메일 도메인입니다") so the service can return the proper error.
In
`@src/main/java/com/example/solidconnection/siteuser/service/SchoolEmailService.java`:
- Around line 101-107: The extractEmailDomain method currently only checks for
presence of '@' and can accept invalid emails like "`@domain`" or "local@"; update
extractEmailDomain to validate both sides of '@' (ensure atIndex > 0 && atIndex
< email.length() - 1) and throw a distinct format error (e.g.,
SCHOOL_EMAIL_INVALID_FORMAT) when that check fails instead of
SCHOOL_EMAIL_DOMAIN_NOT_SUPPORTED; optionally mention replacing or supplementing
this simple check with a proper regex or Jakarta `@Email` validation in the
calling flow for stricter validation.
- Around line 41-58: The method requestSchoolEmailVerification currently
performs Redis save (saveVerificationInfo) and external email send
(mailService.sendVerificationEmail) inside the `@Transactional` boundary which can
leave Redis state inconsistent if sending fails; refactor by moving only DB
operations into a transaction: extract the DB reads/validations
(siteUserRepository.findById, homeUniversityRepository.findByEmailDomain,
generateVerificationCode) into a new transactional helper (e.g.,
fetchAndValidateForEmailVerification) that returns the siteUserId, schoolEmail,
domain, homeUniversityId and code, then call saveVerificationInfo and
mailService.sendVerificationEmail after that helper returns (outside any
transaction) so Redis write and email send occur only after DB commit;
alternatively, if you prefer to keep them in the same method add a try/catch
around mailService.sendVerificationEmail to delete the Redis entry via
saveVerificationInfo removal on failure (compensating delete) — reference
requestSchoolEmailVerification, saveVerificationInfo,
mailService.sendVerificationEmail, and generateVerificationCode when making the
change.
---
Outside diff comments:
In
`@src/main/java/com/example/solidconnection/admin/university/service/AdminHomeUniversityService.java`:
- Around line 68-74: The updateHomeUniversity method lacks duplicate emailDomain
validation; add a check similar to name validation (e.g., call a
validateEmailDomainNotDuplicated(emailDomain, id) or reuse an existing
validator) before calling homeUniversity.update(request.name(),
request.emailDomain()), and throw the same domain-duplicate CustomException
(e.g., EMAIL_DOMAIN_DUPLICATED) if another HomeUniversity already uses
request.emailDomain(); ensure the new validator accepts the current entity id to
allow keeping the same domain on self-updates.
---
Nitpick comments:
In
`@src/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityCreateRequest.java`:
- Around line 10-11: The emailDomain field in AdminHomeUniversityCreateRequest
currently only enforces length; update validation to ensure it is a pure domain
(no '@' or URL schemes) by adding a pattern-based constraint (e.g. a regex that
allows hostnames like example.edu and disallows '@' and 'http(s)://') and a
clear validation message; ensure the Pattern is applied to the field named
emailDomain in class AdminHomeUniversityCreateRequest so
SchoolEmailService.extractEmailDomain() will match stored values correctly.
In
`@src/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityUpdateRequest.java`:
- Around line 10-11: The emailDomain field in AdminHomeUniversityUpdateRequest
currently only has `@Size` and should also enforce domain format; add a `@Pattern`
constraint on the emailDomain field (or create a custom validator if preferred)
to reject blanks/whitespace and invalid characters and to ensure a valid domain
label + TLD structure (e.g., labels separated by dots, allowed chars a-z0-9- and
proper lengths) so the field is both length- and format-validated.
In `@src/main/java/com/example/solidconnection/common/mail/MailService.java`:
- Around line 14-20: The sendVerificationEmail method relies on the SMTP default
sender and doesn't set an explicit from address; update sendVerificationEmail to
call message.setFrom(...) with a configured sender (e.g., a property or
constant) before javaMailSender.send(message), ensuring the from address is read
from configuration (or a class-level constant) so message.setFrom is always set
when sendVerificationEmail is invoked.
- Around line 14-20: Wrap the javaMailSender.send call in
MailService.sendVerificationEmail with a try-catch to log both success and
failures: catch MailException (or Exception), call the class logger to record an
informational log on successful send and an error log with the exception details
on failure, then rethrow or translate the exception as appropriate. Also update
the caller flow in SchoolEmailService (the code path that stores the
verification code to Redis) to handle send failures (e.g., delete the Redis
entry or abort store) so Redis doesn't retain codes when email delivery failed;
reference MailService.sendVerificationEmail, javaMailSender.send and
SchoolEmailService in your changes.
In `@src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java`:
- Around line 162-164: The verifySchool method on SiteUser currently sets
homeUniversityId without validation; update SiteUser.verifySchool(Long
homeUniversityId) to defend its invariants by (1) throwing an exception (e.g.,
IllegalStateException or a domain-specific CustomDomainException) if
this.homeUniversityId is already non-null to prevent silent overwrite, and (2)
validating the incoming homeUniversityId is non-null (throw
IllegalArgumentException or equivalent) before assignment; keep the exception
types consistent with the domain conventions used elsewhere in SiteUser or the
project.
In
`@src/main/java/com/example/solidconnection/siteuser/service/SchoolEmailService.java`:
- Around line 60-73: 리뷰 포인트를 verifySchool에서 발생할 예외로 좁히지 말고, 예외로 인해 Redis 삭제가
누락되는 주요 케이스는 confirmSchoolEmail 호출 이전(예: getVerificationInfo 실패 또는 코드 불일치)임을
명시하고, confirmSchoolEmail 내 getVerificationInfo, 코드 비교와 관련된 실패 시 Redis
키(KEY_PREFIX + siteUserId)의 보존/만료 정책을 검토하라는 지침을 남기세요; 보안상 코드 불일치에서 즉시 정리가 필요하면
confirmSchoolEmail 안의 코드 불일치 분기에서 redisTemplate.delete를 호출하거나(혹은 전체 흐름에서 안전히
정리하려면 try-finally로 감싸서 SiteUser.verifySchool 호출 전후의 예외와 상관없이 필요한 정리가 수행되도록 변경)
정책 결정에 따라 TTL 조정 권고(함수명: confirmSchoolEmail, getVerificationInfo,
SiteUser.verifySchool, redisTemplate.delete, 상수: KEY_PREFIX 참조).
In
`@src/test/java/com/example/solidconnection/admin/university/service/AdminHomeUniversityServiceTest.java`:
- Around line 104-117: Add tests that exercise the emailDomain field: create a
new test in AdminHomeUniversityServiceTest that constructs
AdminHomeUniversityCreateRequest with a non-null emailDomain (e.g.,
"example.edu"), calls adminHomeUniversityService.createHomeUniversity(request),
and asserts the returned AdminHomeUniversityResponse.emailDomain equals the
value and the persisted HomeUniversity (fetched via
homeUniversityRepository.findById(response.id())) has getEmailDomain() equal to
the same value; also add a test that queries the service/repository for a
university with emailDomain to verify retrieval returns the correct value, and a
duplicate-creation test that attempts to create a second
AdminHomeUniversityCreateRequest with the same emailDomain and asserts an
exception is thrown (use assertThrows) to validate unique constraint behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 62a9c522-8c61-4da0-a9a8-a35b944ae423
📒 Files selected for processing (22)
build.gradlesrc/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityCreateRequest.javasrc/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityResponse.javasrc/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityUpdateRequest.javasrc/main/java/com/example/solidconnection/admin/university/service/AdminHomeUniversityService.javasrc/main/java/com/example/solidconnection/common/exception/ErrorCode.javasrc/main/java/com/example/solidconnection/common/mail/MailService.javasrc/main/java/com/example/solidconnection/siteuser/controller/MyPageController.javasrc/main/java/com/example/solidconnection/siteuser/domain/SiteUser.javasrc/main/java/com/example/solidconnection/siteuser/dto/SchoolEmailConfirmRequest.javasrc/main/java/com/example/solidconnection/siteuser/dto/SchoolEmailRequest.javasrc/main/java/com/example/solidconnection/siteuser/dto/SchoolVerificationInfo.javasrc/main/java/com/example/solidconnection/siteuser/service/SchoolEmailService.javasrc/main/java/com/example/solidconnection/university/domain/HomeUniversity.javasrc/main/java/com/example/solidconnection/university/repository/HomeUniversityRepository.javasrc/main/resources/data.sqlsrc/main/resources/db/migration/V50__add_email_domain_to_home_university.sqlsrc/test/java/com/example/solidconnection/admin/university/service/AdminHomeUniversityServiceTest.javasrc/test/java/com/example/solidconnection/siteuser/service/SchoolEmailServiceTest.javasrc/test/java/com/example/solidconnection/university/fixture/HomeUniversityFixture.javasrc/test/java/com/example/solidconnection/university/fixture/HomeUniversityFixtureBuilder.javasrc/test/resources/application.yml
Comment on lines
+101
to
+107
| private String extractEmailDomain(String email) { | ||
| int atIndex = email.indexOf('@'); | ||
| if (atIndex == -1) { | ||
| throw new CustomException(SCHOOL_EMAIL_DOMAIN_NOT_SUPPORTED); | ||
| } | ||
| return email.substring(atIndex + 1); | ||
| } |
There was a problem hiding this comment.
이메일 형식 검증 강화 필요
현재 @ 문자 존재 여부만 확인하고 있어, @inha.edu(로컬 파트 없음)나 test@(도메인 없음) 같은 잘못된 형식이 통과될 수 있습니다. 또한 103-104번 라인에서 @가 없을 때 SCHOOL_EMAIL_DOMAIN_NOT_SUPPORTED 에러를 던지는데, 이는 형식 오류이지 도메인 미지원 문제가 아니므로 에러 코드가 오해를 불러일으킬 수 있습니다.
개선 방안:
@전후에 실제 텍스트가 있는지 확인 (atIndex > 0 && atIndex < email.length() - 1)- 필요시 정규식 또는 Jakarta Validation의
@Email어노테이션 활용 - 형식 오류용 별도 에러 코드 추가 고려 (예:
SCHOOL_EMAIL_INVALID_FORMAT)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/example/solidconnection/siteuser/service/SchoolEmailService.java`
around lines 101 - 107, The extractEmailDomain method currently only checks for
presence of '@' and can accept invalid emails like "`@domain`" or "local@"; update
extractEmailDomain to validate both sides of '@' (ensure atIndex > 0 && atIndex
< email.length() - 1) and throw a distinct format error (e.g.,
SCHOOL_EMAIL_INVALID_FORMAT) when that check fails instead of
SCHOOL_EMAIL_DOMAIN_NOT_SUPPORTED; optionally mention replacing or supplementing
this simple check with a proper regex or Jakarta `@Email` validation in the
calling flow for stricter validation.
- V50 마이그레이션에 기존 대학 email_domain 백필 UPDATE 추가 - AdminHomeUniversityService에 emailDomain 중복 검증 추가 - SchoolEmailService에서 이메일 발송 실패 시 Redis 보상 삭제 추가 - requestSchoolEmailVerification에서 불필요한 @transactional 제거 - extractEmailDomain에서 도메인 소문자 정규화 적용 - AdminHomeUniversity DTO의 emailDomain 검증을 @pattern으로 강화 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityCreateRequest.java (1)
11-14: ⚡ Quick win정규식 패턴 중복을 개선하면 유지보수성이 향상됩니다.
이 파일과
AdminHomeUniversityUpdateRequest에 동일한 정규식 패턴이 사용되고 있습니다. 향후 도메인 검증 규칙이 변경될 경우 두 곳을 모두 수정해야 하므로 일관성 유지가 어려워질 수 있습니다.다음과 같이 개선을 권장드립니다:
♻️ 공통 상수로 추출하는 개선 방안
별도의 상수 클래스를 생성합니다:
public final class ValidationPatterns { public static final String EMAIL_DOMAIN = "^[a-zA-Z0-9]([a-zA-Z0-9\\-]*[a-zA-Z0-9])?(\\.[a-zA-Z0-9]([a-zA-Z0-9\\-]*[a-zA-Z0-9])?)+$"; public static final String EMAIL_DOMAIN_MESSAGE = "올바른 이메일 도메인 형식이 아닙니다 (예: inha.edu, inu.ac.kr)"; private ValidationPatterns() { throw new AssertionError("Cannot instantiate utility class"); } }그런 다음 두 DTO에서 다음과 같이 참조합니다:
`@Pattern`(regexp = ValidationPatterns.EMAIL_DOMAIN, message = ValidationPatterns.EMAIL_DOMAIN_MESSAGE) String emailDomain🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityCreateRequest.java` around lines 11 - 14, Extract the duplicated email-domain regex and message into a shared constant holder (e.g., create a final utility class ValidationPatterns with public static final String EMAIL_DOMAIN and EMAIL_DOMAIN_MESSAGE and a private constructor), then update the `@Pattern` annotations in AdminHomeUniversityCreateRequest and AdminHomeUniversityUpdateRequest to use ValidationPatterns.EMAIL_DOMAIN and ValidationPatterns.EMAIL_DOMAIN_MESSAGE respectively so both DTOs reference the single source of truth for the regex and message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityCreateRequest.java`:
- Around line 11-14: Extract the duplicated email-domain regex and message into
a shared constant holder (e.g., create a final utility class ValidationPatterns
with public static final String EMAIL_DOMAIN and EMAIL_DOMAIN_MESSAGE and a
private constructor), then update the `@Pattern` annotations in
AdminHomeUniversityCreateRequest and AdminHomeUniversityUpdateRequest to use
ValidationPatterns.EMAIL_DOMAIN and ValidationPatterns.EMAIL_DOMAIN_MESSAGE
respectively so both DTOs reference the single source of truth for the regex and
message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7a35aa8d-54cf-46fc-a956-d239f28fb433
📒 Files selected for processing (5)
src/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityCreateRequest.javasrc/main/java/com/example/solidconnection/admin/university/dto/AdminHomeUniversityUpdateRequest.javasrc/main/java/com/example/solidconnection/admin/university/service/AdminHomeUniversityService.javasrc/main/java/com/example/solidconnection/common/exception/ErrorCode.javasrc/main/java/com/example/solidconnection/siteuser/service/SchoolEmailService.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/example/solidconnection/common/exception/ErrorCode.java
- src/main/java/com/example/solidconnection/admin/university/service/AdminHomeUniversityService.java
- src/main/java/com/example/solidconnection/siteuser/service/SchoolEmailService.java
whqtker
reviewed
Jun 10, 2026
| import java.util.concurrent.ThreadLocalRandom; | ||
| import java.util.concurrent.TimeUnit; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.beans.factory.annotation.Qualifier; |
| private final RedisTemplate<String, String> redisTemplate; | ||
| private final ObjectMapper objectMapper; | ||
|
|
||
| public void requestSchoolEmailVerification(long siteUserId, String schoolEmail) { |
| } | ||
| } | ||
|
|
||
| private String extractEmailDomain(String email) { |
Contributor
Author
There was a problem hiding this comment.
중복 검증이네요 바로 substring으로 잘라도 무방할것같습니다 수정하겠음돠
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 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.
관련 이슈
resolves: #751
작업 내용
1. 엔티티 구조 변경
HomeUniversity엔티티에emailDomain컬럼을 추가했습니다.SiteUser엔티티에 도메인 메서드verifySchool(Long homeUniversityId)를 추가했습니다. 인증 완료 시homeUniversityId를 설정하는 책임을 엔티티 안으로 넣었습니다.2. 학교 이메일 인증 서비스 추가
SchoolEmailService를 추가했습니다. 동작 흐름은 다음과 같습니다.POST /my/school-email): 이메일 도메인으로HomeUniversity를 조회하고, 6자리 인증 코드를 생성하여 Redis에 TTL 5분으로 저장 후 이메일 발송POST /my/school-email/confirm): Redis에서 인증 정보를 꺼내 코드가 일치하면SiteUser.homeUniversityId를 업데이트하고 Redis 키 삭제Redis에는
school-email:{siteUserId}키로SchoolVerificationInfo를 JSON 직렬화하여 저장합니다. 역직렬화 실패 시 손상된 데이터로 보고 키를 삭제하고SCHOOL_EMAIL_VERIFICATION_INFO_CORRUPTED예외를 던집니다.3. MailService 추가
common/mail/MailService를 추가했습니다.JavaMailSender를 래핑하며, 이메일 발송은 이 서비스를 통해서만 이루어집니다. 테스트에서는@MockitoBean으로 대체합니다.spring-boot-starter-mail의존성을 추가했으며, 메일 서버 설정은 AWS Parameter Store(/common/spring_mail_*)에서 주입받습니다.4. 어드민 DTO 변경
AdminHomeUniversityCreateRequest/AdminHomeUniversityUpdateRequest/AdminHomeUniversityResponse에emailDomain필드를 추가했습니다.5. 스키마 관련
V50__add_email_domain_to_home_university.sql을 추가했습니다.data.sql에 대학별 초기emailDomain데이터를 추가했습니다 (인하대, 경희대, 중앙대, 성신여대, 인천대).6. 테스트 코드 변경
SchoolEmailServiceTest를 추가했습니다 (6개 케이스,@TestContainerSpringBootTest기반).HomeUniversityFixture/HomeUniversityFixtureBuilder에emailDomain필드를 추가했고,AdminHomeUniversityServiceTest에서 깨진 생성자 호출을 수정했습니다.특이 사항
인증 코드 저장에
objectRedisTemplate대신RedisTemplate<String, String>+ObjectMapper를 사용했습니다.objectRedisTemplate의 기본 역직렬화기가LinkedHashMap으로 복원하여SchoolVerificationInfo로 캐스팅이 실패하는 문제가 있어 JSON 문자열로 저장하는 방식으로 처리했습니다.Parameter store에 /common 경로로 mail 관련 설정을 추가하였습니다.
리뷰 요구사항 (선택)