Conversation
Walkthrough사용자 회원가입 흐름이 개편됨. 로그인 성공 시 세션에 firstUser(UserDto)를 저장하고 /register로 리다이렉트. GET /first-register가 세션 기반 DefaultRegisterUserDto를 반환. POST /register는 RegisterUserDto의 classCode와 세션 firstUser를 사용해 UserService.registerUser(UserDto, classCode) 호출 후 201 Created 반환. DTO/서비스 시그니처 정리. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant S as Spring Security<br/>CustomSuccessHandler
participant Sess as HttpSession
participant C as RegisterController
participant SV as UserService
U->>S: 로그인 성공
S->>S: 사용자 classCode 확인
alt classCode == -1 (미등록)
S->>Sess: setAttribute("firstUser", UserDto)
S-->>U: 302 Redirect to /register
else 등록됨
S-->>U: 토큰 발급 후 홈으로 리다이렉트
end
U->>C: GET /first-register
C->>Sess: getAttribute("firstUser")
C-->>U: 200 DefaultRegisterUserDto (email, username, role)
U->>C: POST /register { classCode }
C->>Sess: getAttribute("firstUser")
C->>Sess: removeAttribute("firstUser")
C->>SV: registerUser(UserDto, classCode)
SV-->>C: 완료
C-->>U: 201 Created
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks (3 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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. Comment |
![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/hello/cluebackend/global/config/CustomSuccessHandler.java (1)
70-76: 쿠키 보안 속성 누락: Secure + SameSite(None) 설정 필요refresh_token은 크로스 사이트(프론트: localhost:3000, 백엔드: 다른 포트/도메인) 전송이므로 기본 Lax에서는 전송되지 않거나 브라우저에 의해 차단됩니다. SameSite=None; Secure를 명시해야 합니다. HttpOnly만으로는 불충분합니다. 개발/운영 프로필별로 Secure 분기 처리하세요.
- private Cookie createCookie(String key, String value) { - Cookie cookie = new Cookie(key, value); - cookie.setMaxAge(7 * 24 * 60 * 60); - // cookie.setSecure(true); - cookie.setPath("/"); - cookie.setHttpOnly(true); - - return cookie; - } + private void addCookie(HttpServletResponse response, String key, String value) { + // Spring 6+ ResponseCookie를 사용하면 SameSite 설정 가능 + var cookie = org.springframework.http.ResponseCookie.from(key, value) + .httpOnly(true) + .secure(true) // dev 프로필에서는 환경변수로 false 전환 고려 + .sameSite("None") + .path("/") + .maxAge(7 * 24 * 60 * 60) + .build(); + response.addHeader(org.springframework.http.HttpHeaders.SET_COOKIE, cookie.toString()); + }그리고 호출부도 아래처럼 교체하세요.
- response.addCookie(createCookie("refresh_token", refresh)); + addCookie(response, "refresh_token", refresh);
🧹 Nitpick comments (4)
src/main/java/hello/cluebackend/global/config/CustomSuccessHandler.java (3)
43-49: 세션 키 하드코딩 제거 및 상수화 권장"firstUser" 문자열을 여러 곳에서 직접 사용하면 오타/변경 시 오류 위험이 큽니다. 공용 상수(예: SessionAttributes.FIRST_USER)로 추출해 컨트롤러/핸들러 간 일관성을 보장하세요.
- request.getSession().setAttribute("firstUser", userDTO); + request.getSession().setAttribute(SessionAttributes.FIRST_USER, userDTO);
51-58: 권한 컬렉션 비어있을 가능성 가드iterator.next()는 권한이 비어있으면 NoSuchElementException을 유발합니다. 방어 코드를 한 줄 추가하세요.
- Iterator<? extends GrantedAuthority> iterator = authorities.iterator(); - GrantedAuthority auth = iterator.next(); + Iterator<? extends GrantedAuthority> iterator = authorities.iterator(); + if (!iterator.hasNext()) throw new IllegalStateException("권한이 없습니다."); + GrantedAuthority auth = iterator.next();
59-61: 토큰/쿠키 TTL 매직 넘버 상수화가독성과 유지보수를 위해 액세스/리프레시 만료시간을 상수로 추출하세요.
src/main/java/hello/cluebackend/domain/user/service/UserService.java (1)
28-31: Optional: 반환값 제공 고려등록 결과(예: 생성된 userId)를 반환하면 호출부에서 후속 처리(로그, 리다이렉트 파라미터)가 편해집니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/hello/cluebackend/domain/user/presentation/RegisterController.java(1 hunks)src/main/java/hello/cluebackend/domain/user/presentation/dto/DefaultRegisterUserDto.java(1 hunks)src/main/java/hello/cluebackend/domain/user/presentation/dto/RegisterUserDto.java(0 hunks)src/main/java/hello/cluebackend/domain/user/service/UserService.java(1 hunks)src/main/java/hello/cluebackend/global/config/CustomSuccessHandler.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/hello/cluebackend/domain/user/presentation/dto/RegisterUserDto.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-08T23:38:29.680Z
Learnt from: fixgramwork
PR: EscapeFrame/clueBackend#23
File: src/main/java/hello/cluebackend/domain/user/service/RegisterUserService.java:17-23
Timestamp: 2025-07-08T23:38:29.680Z
Learning: This project uses OAuth2 authentication and does not require traditional password storage or validation. The RegisterUserService in src/main/java/hello/cluebackend/domain/user/service/RegisterUserService.java should not store passwords since OAuth2 handles authentication externally.
Applied to files:
src/main/java/hello/cluebackend/domain/user/service/UserService.javasrc/main/java/hello/cluebackend/domain/user/presentation/RegisterController.java
🧬 Code graph analysis (4)
src/main/java/hello/cluebackend/global/config/CustomSuccessHandler.java (2)
src/main/java/hello/cluebackend/global/config/SecurityConfig.java (2)
Configuration(22-140)Bean(36-139)src/main/java/hello/cluebackend/domain/user/service/CustomOAuth2UserService.java (1)
CustomOAuth2UserService(16-70)
src/main/java/hello/cluebackend/domain/user/service/UserService.java (2)
src/main/java/hello/cluebackend/domain/user/presentation/dto/UserDto.java (2)
Getter(10-41)UserDto(23-29)src/main/java/hello/cluebackend/domain/user/domain/UserEntity.java (1)
UserEntity(47-52)
src/main/java/hello/cluebackend/domain/user/presentation/dto/DefaultRegisterUserDto.java (3)
src/main/java/hello/cluebackend/domain/user/presentation/dto/RegisterUserDto.java (1)
RegisterUserDto(7-14)src/main/java/hello/cluebackend/domain/user/presentation/dto/UserDto.java (3)
Getter(10-41)UserDto(23-29)toEntity(31-40)src/main/java/hello/cluebackend/domain/user/domain/UserEntity.java (1)
ToString(11-64)
src/main/java/hello/cluebackend/domain/user/presentation/RegisterController.java (2)
src/main/java/hello/cluebackend/domain/user/presentation/dto/RegisterUserDto.java (1)
RegisterUserDto(7-14)src/main/java/hello/cluebackend/domain/user/service/CustomOAuth2UserService.java (1)
CustomOAuth2UserService(16-70)
🔇 Additional comments (2)
src/main/java/hello/cluebackend/domain/user/presentation/dto/DefaultRegisterUserDto.java (1)
8-15: 역할 분리 명확—구조 적절classCode를 분리하고 email/username/role만 노출하는 DTO로 재구성된 점 좋습니다. 빌더 추가도 적절합니다.
src/main/java/hello/cluebackend/domain/user/presentation/RegisterController.java (1)
23-42: 세션 쿠키 SameSite/Secure 설정 추가 필요
application*.yml/properties에서 server.servlet.session.cookie.same-site 및 server.servlet.session.cookie.secure 설정이 발견되지 않습니다.
- server.servlet.session.cookie.same-site=None
- server.servlet.session.cookie.secure=true (HTTPS)
를 프로필별로 추가해 FE(localhost:3000)→BE API 호출 시 JSESSIONID가 전송되도록 설정하세요.
대안으로 세션 대신 단회용 등록 토큰(JWT, 짧은 TTL)을 리다이렉트 시 쿼리/프래그먼트로 전달하는 방식을 고려할 수 있습니다.
Comment on lines
+23
to
32
| @GetMapping("/first-register") | ||
| public DefaultRegisterUserDto showRegistrationForm(HttpServletRequest request) { | ||
| HttpSession session = request.getSession(); | ||
| UserDto userDto = (UserDto) session.getAttribute("firstUser"); | ||
| return DefaultRegisterUserDto.builder() | ||
| .username(userDto.getUsername()) | ||
| .email(userDto.getEmail()) | ||
| .role(userDto.getRole()) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
NPE 및 세션 오용 가드: getSession(false)와 401/410 처리
세션이 없거나 firstUser가 없으면 현재 코드는 NPE가 납니다(getUsername 호출). 세션을 새로 만들지 말고(getSession(false)) 없으면 401(또는 410 Gone)로 응답하세요.
- public DefaultRegisterUserDto showRegistrationForm(HttpServletRequest request) {
- HttpSession session = request.getSession();
- UserDto userDto = (UserDto) session.getAttribute("firstUser");
+ public DefaultRegisterUserDto showRegistrationForm(HttpServletRequest request) {
+ HttpSession session = request.getSession(false);
+ if (session == null) {
+ throw new org.springframework.web.server.ResponseStatusException(HttpStatus.UNAUTHORIZED, "세션이 없습니다.");
+ }
+ UserDto userDto = (UserDto) session.getAttribute("firstUser");
+ if (userDto == null) {
+ throw new org.springframework.web.server.ResponseStatusException(HttpStatus.UNAUTHORIZED, "등록 세션이 만료되었습니다.");
+ }
return DefaultRegisterUserDto.builder()
.username(userDto.getUsername())
.email(userDto.getEmail())
.role(userDto.getRole())
.build();
}📝 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
| @GetMapping("/first-register") | |
| public DefaultRegisterUserDto showRegistrationForm(HttpServletRequest request) { | |
| HttpSession session = request.getSession(); | |
| UserDto userDto = (UserDto) session.getAttribute("firstUser"); | |
| return DefaultRegisterUserDto.builder() | |
| .username(userDto.getUsername()) | |
| .email(userDto.getEmail()) | |
| .role(userDto.getRole()) | |
| .build(); | |
| } | |
| @GetMapping("/first-register") | |
| public DefaultRegisterUserDto showRegistrationForm(HttpServletRequest request) { | |
| HttpSession session = request.getSession(false); | |
| if (session == null) { | |
| throw new org.springframework.web.server.ResponseStatusException( | |
| HttpStatus.UNAUTHORIZED, "세션이 없습니다." | |
| ); | |
| } | |
| UserDto userDto = (UserDto) session.getAttribute("firstUser"); | |
| if (userDto == null) { | |
| throw new org.springframework.web.server.ResponseStatusException( | |
| HttpStatus.UNAUTHORIZED, "등록 세션이 만료되었습니다." | |
| ); | |
| } | |
| return DefaultRegisterUserDto.builder() | |
| .username(userDto.getUsername()) | |
| .email(userDto.getEmail()) | |
| .role(userDto.getRole()) | |
| .build(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/hello/cluebackend/domain/user/presentation/RegisterController.java
around lines 23-32, the method currently calls request.getSession() and
dereferences a possibly null session/attribute causing NPE; change to
request.getSession(false), check if the session is null or
session.getAttribute("firstUser") is null (and assert the attribute type), and
if missing throw a ResponseStatusException with HttpStatus.UNAUTHORIZED (or
HttpStatus.GONE if you prefer 410) so no new session is created and a proper
401/410 is returned; otherwise safely cast the attribute to UserDto and
build/return the DefaultRegisterUserDto.
Comment on lines
+34
to
42
| @PostMapping("/register") | ||
| public ResponseEntity<?> processRegistration(HttpServletRequest request, | ||
| RegisterUserDto registerUserDto) { | ||
| HttpSession session = request.getSession(); | ||
| UserDto dto = (UserDto) session.getAttribute("firstUser"); | ||
| UserDto userDto = (UserDto) session.getAttribute("firstUser"); | ||
| session.removeAttribute("firstUser"); | ||
| return dto; | ||
| } | ||
|
|
||
| @PostMapping( | ||
| value = "/register", | ||
| consumes = MediaType.APPLICATION_JSON_VALUE | ||
| ) | ||
| public ResponseEntity<?> processRegistration(@RequestBody DefaultRegisterUserDto defaultRegisterUserDTO) { | ||
| log.info("ClassCode 1 : " + defaultRegisterUserDTO.getClassCode()); | ||
| userService.registerUser(defaultRegisterUserDTO); | ||
| log.info("ClassCode 1 : " + registerUserDto.getClassCode()); | ||
| userService.registerUser(userDto, registerUserDto.getClassCode()); | ||
| return new ResponseEntity<>(HttpStatus.CREATED); |
There was a problem hiding this comment.
JSON 바인딩 깨짐(@RequestBody 누락) + NPE/세션 제거 시점
- @RequestBody가 없어 프런트에서 JSON 전송 시 classCode가 바인딩되지 않습니다.
- firstUser가 없을 때 NPE 발생.
- removeAttribute를 저장 성공 후로 이동해야 재시도 가능.
- public ResponseEntity<?> processRegistration(HttpServletRequest request,
- RegisterUserDto registerUserDto) {
- HttpSession session = request.getSession();
- UserDto userDto = (UserDto) session.getAttribute("firstUser");
- session.removeAttribute("firstUser");
- log.info("ClassCode 1 : " + registerUserDto.getClassCode());
- userService.registerUser(userDto, registerUserDto.getClassCode());
+ public ResponseEntity<Void> processRegistration(
+ HttpServletRequest request,
+ @org.springframework.web.bind.annotation.RequestBody
+ @jakarta.validation.Valid RegisterUserDto registerUserDto) {
+ HttpSession session = request.getSession(false);
+ if (session == null) {
+ return new ResponseEntity<>(HttpStatus.UNAUTHORIZED);
+ }
+ UserDto userDto = (UserDto) session.getAttribute("firstUser");
+ if (userDto == null) {
+ return new ResponseEntity<>(HttpStatus.UNAUTHORIZED);
+ }
+ log.info("ClassCode 1 : {}", registerUserDto.getClassCode());
+ userService.registerUser(userDto, registerUserDto.getClassCode());
+ session.removeAttribute("firstUser");
- return new ResponseEntity<>(HttpStatus.CREATED);
+ return new ResponseEntity<>(HttpStatus.CREATED);
}📝 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
| @PostMapping("/register") | |
| public ResponseEntity<?> processRegistration(HttpServletRequest request, | |
| RegisterUserDto registerUserDto) { | |
| HttpSession session = request.getSession(); | |
| UserDto dto = (UserDto) session.getAttribute("firstUser"); | |
| UserDto userDto = (UserDto) session.getAttribute("firstUser"); | |
| session.removeAttribute("firstUser"); | |
| return dto; | |
| } | |
| @PostMapping( | |
| value = "/register", | |
| consumes = MediaType.APPLICATION_JSON_VALUE | |
| ) | |
| public ResponseEntity<?> processRegistration(@RequestBody DefaultRegisterUserDto defaultRegisterUserDTO) { | |
| log.info("ClassCode 1 : " + defaultRegisterUserDTO.getClassCode()); | |
| userService.registerUser(defaultRegisterUserDTO); | |
| log.info("ClassCode 1 : " + registerUserDto.getClassCode()); | |
| userService.registerUser(userDto, registerUserDto.getClassCode()); | |
| return new ResponseEntity<>(HttpStatus.CREATED); | |
| @PostMapping("/register") | |
| public ResponseEntity<Void> processRegistration( | |
| HttpServletRequest request, | |
| @org.springframework.web.bind.annotation.RequestBody | |
| @jakarta.validation.Valid RegisterUserDto registerUserDto) { | |
| HttpSession session = request.getSession(false); | |
| if (session == null) { | |
| return new ResponseEntity<>(HttpStatus.UNAUTHORIZED); | |
| } | |
| UserDto userDto = (UserDto) session.getAttribute("firstUser"); | |
| if (userDto == null) { | |
| return new ResponseEntity<>(HttpStatus.UNAUTHORIZED); | |
| } | |
| log.info("ClassCode 1 : {}", registerUserDto.getClassCode()); | |
| userService.registerUser(userDto, registerUserDto.getClassCode()); | |
| session.removeAttribute("firstUser"); | |
| return new ResponseEntity<>(HttpStatus.CREATED); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/hello/cluebackend/domain/user/presentation/RegisterController.java
around lines 34 to 42, the controller is missing @RequestBody on the
RegisterUserDto so JSON classCode won't bind, it dereferences session attribute
firstUser without null-check (NPE risk), and it removes the session attribute
before ensuring registration succeeded. Add @RequestBody to the RegisterUserDto
parameter, check session.getAttribute("firstUser") for null and return an
appropriate error response if absent, call userService.registerUser(...) only
after confirming firstUser is present, and move
session.removeAttribute("firstUser") to after a successful registration (or only
remove when registerUser returns/does not throw) so failed attempts can retry.
Comment on lines
+18
to
26
| public void registerUser(UserDto userDto, int classCode) { | ||
| UserEntity userEntity = new UserEntity( | ||
| userDTO.getClassCode(), | ||
| userDTO.getUsername(), | ||
| userDTO.getEmail(), | ||
| userDTO.getRole() | ||
| classCode, | ||
| userDto.getUsername(), | ||
| userDto.getEmail(), | ||
| userDto.getRole() | ||
| ); | ||
| userRepository.save(userEntity); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
트랜잭션과 중복/입력 검증 추가 필요
- 트랜잭션 경계가 없어 예외 시 일관성 보장이 약합니다.
- 이메일 중복(경합 포함)과 classCode 유효성 검증이 없습니다.
+import org.springframework.transaction.annotation.Transactional;
+
- public void registerUser(UserDto userDto, int classCode) {
+ @Transactional
+ public void registerUser(UserDto userDto, int classCode) {
+ if (userDto == null) throw new IllegalArgumentException("userDto is null");
+ if (classCode < 0) throw new IllegalArgumentException("invalid classCode");
+ userRepository.findByEmail(userDto.getEmail()).ifPresent(u -> {
+ throw new IllegalStateException("이미 등록된 이메일입니다.");
+ });
UserEntity userEntity = new UserEntity(
classCode,
userDto.getUsername(),
userDto.getEmail(),
userDto.getRole()
);
userRepository.save(userEntity);
}원하시면 예외를 도메인 예외로 치환해 드리겠습니다.
📝 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
| public void registerUser(UserDto userDto, int classCode) { | |
| UserEntity userEntity = new UserEntity( | |
| userDTO.getClassCode(), | |
| userDTO.getUsername(), | |
| userDTO.getEmail(), | |
| userDTO.getRole() | |
| classCode, | |
| userDto.getUsername(), | |
| userDto.getEmail(), | |
| userDto.getRole() | |
| ); | |
| userRepository.save(userEntity); | |
| } | |
| import org.springframework.transaction.annotation.Transactional; | |
| public class UserService { | |
| // … other fields and constructors … | |
| @Transactional | |
| public void registerUser(UserDto userDto, int classCode) { | |
| if (userDto == null) { | |
| throw new IllegalArgumentException("userDto is null"); | |
| } | |
| if (classCode < 0) { | |
| throw new IllegalArgumentException("invalid classCode"); | |
| } | |
| userRepository.findByEmail(userDto.getEmail()).ifPresent(u -> { | |
| throw new IllegalStateException("이미 등록된 이메일입니다."); | |
| }); | |
| UserEntity userEntity = new UserEntity( | |
| classCode, | |
| userDto.getUsername(), | |
| userDto.getEmail(), | |
| userDto.getRole() | |
| ); | |
| userRepository.save(userEntity); | |
| } | |
| // … other methods … | |
| } |
🤖 Prompt for AI Agents
In src/main/java/hello/cluebackend/domain/user/service/UserService.java around
lines 18 to 26, the registerUser method lacks a transaction boundary, does not
validate classCode, and does not guard against duplicate emails (including race
conditions). Make the method transactional (e.g., @Transactional at class or
method level), validate classCode against allowed values and throw a
domain-specific validation exception if invalid, check for existing email via
the repository (existsByEmail) and throw a domain duplicate-email exception
before saving, and additionally handle database-level unique constraint races by
catching DataIntegrityViolationException (or the persistence exception) and
translating it into the same domain duplicate-email exception so concurrent
inserts are handled consistently. Ensure all thrown exceptions are domain
exceptions defined in the user domain.
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
1 participant
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.
[#78] 회원가입 로직 수정
✅ 작업 내용
🔗 관련 이슈
Close #78
📌 체크리스트
Summary by CodeRabbit
신기능
변경 사항