Skip to content

Fix#78 #79

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Hgyeol merged 2 commits into from
Sep 9, 2025
Merged

Fix#78 #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c0b93ff
fix(#78) : 회원가입 로직 수정
Hgyeol Sep 9, 2025
cf9c5c8
fix(#78) : /first-register get으로 변경
Hgyeol Sep 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 15 additions & 19 deletions src/main/java/hello/cluebackend/domain/user/presentation/RegisterController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package hello.cluebackend.domain.user.presentation;

import com.nimbusds.oauth2.sdk.TokenResponse;
import hello.cluebackend.domain.user.domain.UserEntity;
import hello.cluebackend.domain.user.presentation.dto.DefaultRegisterUserDto;
import hello.cluebackend.domain.user.presentation.dto.RegisterUserDto;
import hello.cluebackend.domain.user.presentation.dto.UserDto;
Expand All @@ -10,7 +8,6 @@
import jakarta.servlet.http.HttpSession;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;

Expand All @@ -23,26 +20,25 @@ public RegisterController(UserService userService) {
this.userService = userService;
}

@PostMapping
public String processRegistration(RegisterUserDto registerUserDTO) {
return "redirect:/";
@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();
}
Comment on lines +23 to 32
Copy link

@coderabbitai coderabbitai bot Sep 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.


@PostMapping("/first-register")
public UserDto showRegistrationForm(HttpServletRequest request) {
@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);
Comment on lines +34 to 42
Copy link

@coderabbitai coderabbitai bot Sep 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.

}
}
7 changes: 4 additions & 3 deletions src/main/java/hello/cluebackend/domain/user/presentation/dto/DefaultRegisterUserDto.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
package hello.cluebackend.domain.user.presentation.dto;

import hello.cluebackend.domain.user.domain.Role;
import lombok.Getter;
import lombok.Setter;
import lombok.*;

@Getter
@Setter
@Builder
@NoArgsConstructor
@AllArgsConstructor
public class DefaultRegisterUserDto {
private String email;
private String username;
private int classCode;
private Role role;
}
3 changes: 0 additions & 3 deletions src/main/java/hello/cluebackend/domain/user/presentation/dto/RegisterUserDto.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,5 @@
@Getter
@Setter
public class RegisterUserDto {
private String email;
private String username;
private int classCode;
private Role role;
}
10 changes: 5 additions & 5 deletions src/main/java/hello/cluebackend/domain/user/service/UserService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ public UserService(UserRepository userRepository) {
this.userRepository = userRepository;
}

public void registerUser(DefaultRegisterUserDto userDTO) {
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);
}
Comment on lines +18 to 26
Copy link

@coderabbitai coderabbitai bot Sep 9, 2025

Choose a reason for hiding this comment

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

🛠️ 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.

Expand Down
1 change: 1 addition & 0 deletions src/main/java/hello/cluebackend/global/config/CustomSuccessHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public void onAuthenticationSuccess(HttpServletRequest request, HttpServletRespo

int classCode = userDTO.getClassCode();
if (classCode == -1) {
request.getSession().setAttribute("firstUser", userDTO);
getRedirectStrategy().sendRedirect(
request,
response,
Expand Down
Loading