-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: Presentation Layer 구조 개선 #7
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.
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
Changes from all commits
fc3fd4e
40e6c68
386c45d
d49d338
e1da497
a3ca754
b4286f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| package com.stcom.smartmealtable.web.controller; | ||
|
|
||
| import com.stcom.smartmealtable.domain.member.Member; | ||
| import com.stcom.smartmealtable.exception.PasswordPolicyException; | ||
| import com.stcom.smartmealtable.infrastructure.dto.JwtTokenResponseDto; | ||
| import com.stcom.smartmealtable.security.JwtTokenService; | ||
| import com.stcom.smartmealtable.service.MemberService; | ||
| import com.stcom.smartmealtable.service.TermService; | ||
| import com.stcom.smartmealtable.service.dto.MemberDto; | ||
| import com.stcom.smartmealtable.service.dto.TermAgreementRequestDto; | ||
| import com.stcom.smartmealtable.web.argumentresolver.UserContext; | ||
| import com.stcom.smartmealtable.web.dto.ApiResponse; | ||
| import jakarta.validation.Valid; | ||
| import jakarta.validation.constraints.Email; | ||
| import java.util.List; | ||
| import lombok.AllArgsConstructor; | ||
| import lombok.Data; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.DeleteMapping; | ||
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.PostMapping; | ||
| import org.springframework.web.bind.annotation.RequestBody; | ||
| import org.springframework.web.bind.annotation.RequestMapping; | ||
| import org.springframework.web.bind.annotation.RequestParam; | ||
| import org.springframework.web.bind.annotation.ResponseStatus; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
|
|
||
| /** | ||
| * 인증(로그인, 회원가입) 관련 API. | ||
| */ | ||
| @RestController | ||
| @Slf4j | ||
| @RequiredArgsConstructor | ||
| @RequestMapping("/api/v1/auth") | ||
| public class AuthController { | ||
|
|
||
| private final MemberService memberService; | ||
| private final JwtTokenService jwtTokenService; | ||
| private final TermService termService; | ||
|
|
||
| @GetMapping("/email/check") | ||
| public ResponseEntity<ApiResponse<Void>> checkEmail(@Email @RequestParam String email) { | ||
| memberService.validateDuplicatedEmail(email); | ||
| return ResponseEntity.ok().body(ApiResponse.createSuccessWithNoContent()); | ||
| } | ||
|
|
||
| @ResponseStatus(HttpStatus.CREATED) | ||
| @PostMapping("/signup") | ||
| public ApiResponse<JwtTokenResponseDto> signUp(@Valid @RequestBody SignUpRequest request) | ||
| throws PasswordPolicyException { | ||
| memberService.validateDuplicatedEmail(request.getEmail()); | ||
| memberService.checkPasswordDoubly(request.getPassword(), request.getConfirmPassword()); | ||
|
|
||
| Member member = Member.builder() | ||
| .fullName(request.getFullName()) | ||
| .email(request.getEmail()) | ||
| .rawPassword(request.getPassword()) | ||
| .build(); | ||
|
|
||
| memberService.saveMember(member); | ||
| JwtTokenResponseDto tokenDto = jwtTokenService.createTokenDto(member.getId(), null); | ||
| tokenDto.setNewUser(true); | ||
| return ApiResponse.createSuccess(tokenDto); | ||
| } | ||
|
Comment on lines
+52
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 비밀번호 평문 저장 가능성
🤖 Prompt for AI Agents |
||
|
|
||
| @PostMapping("/signup/terms") | ||
| public ApiResponse<Void> agreeTerms(@UserContext MemberDto memberDto, | ||
| @RequestBody List<TermAgreementRequest> agreements) { | ||
| termService.agreeTerms( | ||
| memberDto.getMemberId(), | ||
| agreements.stream() | ||
| .map(dto -> new TermAgreementRequestDto(dto.getTermId(), dto.getIsAgreed())) | ||
| .toList() | ||
| ); | ||
| return ApiResponse.createSuccessWithNoContent(); | ||
| } | ||
|
Comment on lines
+69
to
+79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 약관 동의 로직의 입력 검증 부족
🤖 Prompt for AI Agents |
||
|
|
||
| @DeleteMapping("/signup") | ||
| public ApiResponse<Void> cancelSignUp(@UserContext MemberDto memberDto) { | ||
| memberService.deleteByMemberId(memberDto.getMemberId()); | ||
| return ApiResponse.createSuccessWithNoContent(); | ||
| } | ||
|
|
||
| @Data | ||
| @AllArgsConstructor | ||
| public static class SignUpRequest { | ||
| private String email; | ||
| private String password; | ||
| private String confirmPassword; | ||
| private String fullName; | ||
| } | ||
|
|
||
| @Data | ||
| @AllArgsConstructor | ||
| public static class TermAgreementRequest { | ||
| private Long termId; | ||
| private Boolean isAgreed; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,112 @@ | ||||||||||||||||||||||||||||||||||||
| package com.stcom.smartmealtable.web.controller; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| import com.stcom.smartmealtable.infrastructure.SocialAuthService; | ||||||||||||||||||||||||||||||||||||
| import com.stcom.smartmealtable.infrastructure.dto.JwtTokenResponseDto; | ||||||||||||||||||||||||||||||||||||
| import com.stcom.smartmealtable.infrastructure.dto.TokenDto; | ||||||||||||||||||||||||||||||||||||
| import com.stcom.smartmealtable.security.JwtBlacklistService; | ||||||||||||||||||||||||||||||||||||
| import com.stcom.smartmealtable.security.JwtTokenService; | ||||||||||||||||||||||||||||||||||||
| import com.stcom.smartmealtable.service.LoginService; | ||||||||||||||||||||||||||||||||||||
| import com.stcom.smartmealtable.service.dto.AuthResultDto; | ||||||||||||||||||||||||||||||||||||
| import com.stcom.smartmealtable.service.dto.MemberDto; | ||||||||||||||||||||||||||||||||||||
| import com.stcom.smartmealtable.web.argumentresolver.UserContext; | ||||||||||||||||||||||||||||||||||||
| import com.stcom.smartmealtable.web.dto.ApiResponse; | ||||||||||||||||||||||||||||||||||||
| import jakarta.servlet.http.HttpServletRequest; | ||||||||||||||||||||||||||||||||||||
| import jakarta.validation.constraints.Email; | ||||||||||||||||||||||||||||||||||||
| import jakarta.validation.constraints.NotEmpty; | ||||||||||||||||||||||||||||||||||||
| import java.util.Objects; | ||||||||||||||||||||||||||||||||||||
| import lombok.AllArgsConstructor; | ||||||||||||||||||||||||||||||||||||
| import lombok.Data; | ||||||||||||||||||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||||||||
| import lombok.extern.slf4j.Slf4j; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.PostMapping; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.RequestBody; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.RequestMapping; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.RestController; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * 인증 토큰 관련 API (로그인 / 로그아웃 / 토큰 리프레시 / 소셜 로그인). | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| @RestController | ||||||||||||||||||||||||||||||||||||
| @Slf4j | ||||||||||||||||||||||||||||||||||||
| @RequiredArgsConstructor | ||||||||||||||||||||||||||||||||||||
| @RequestMapping("/api/v1/auth") | ||||||||||||||||||||||||||||||||||||
| public class AuthTokenController { | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| private final LoginService loginService; | ||||||||||||||||||||||||||||||||||||
| private final JwtTokenService jwtTokenService; | ||||||||||||||||||||||||||||||||||||
| private final JwtBlacklistService jwtBlacklistService; | ||||||||||||||||||||||||||||||||||||
| private final SocialAuthService socialAuthService; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @PostMapping("/login") | ||||||||||||||||||||||||||||||||||||
| public ApiResponse<JwtTokenResponseDto> login(@RequestBody EmailLoginRequest request) { | ||||||||||||||||||||||||||||||||||||
| AuthResultDto authResultDto = loginService.loginWithEmail(request.getEmail(), request.getPassword()); | ||||||||||||||||||||||||||||||||||||
| JwtTokenResponseDto jwtDto = jwtTokenService.createTokenDto(authResultDto.getMemberId(), | ||||||||||||||||||||||||||||||||||||
| authResultDto.getProfileId()); | ||||||||||||||||||||||||||||||||||||
| if (authResultDto.isNewUser()) { | ||||||||||||||||||||||||||||||||||||
| jwtDto.setNewUser(true); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| return ApiResponse.createSuccess(jwtDto); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @PostMapping("/logout") | ||||||||||||||||||||||||||||||||||||
| public ApiResponse<Void> logout(HttpServletRequest request) { | ||||||||||||||||||||||||||||||||||||
| String jwt = request.getHeader("Authorization"); | ||||||||||||||||||||||||||||||||||||
| if (Objects.nonNull(jwt)) { | ||||||||||||||||||||||||||||||||||||
| jwtBlacklistService.addToBlacklist(jwt); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| return ApiResponse.createSuccessWithNoContent(); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+51
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bearer 토큰 파싱 오류 가능성
-String jwt = request.getHeader("Authorization");
-if (Objects.nonNull(jwt)) {
- jwtBlacklistService.addToBlacklist(jwt);
-}
+String bearer = request.getHeader("Authorization");
+if (StringUtils.hasText(bearer) && bearer.startsWith("Bearer ")) {
+ String jwt = bearer.substring(7);
+ jwtBlacklistService.addToBlacklist(jwt);
+}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @PostMapping("/oauth2/code") | ||||||||||||||||||||||||||||||||||||
| public ApiResponse<JwtTokenResponseDto> socialLogin(@RequestBody SocialLoginRequest request) { | ||||||||||||||||||||||||||||||||||||
| TokenDto token = socialAuthService.getTokenResponse(request.getProvider().toLowerCase(), | ||||||||||||||||||||||||||||||||||||
| request.getAuthorizationCode()); | ||||||||||||||||||||||||||||||||||||
| AuthResultDto authResultDto = loginService.socialLogin(token); | ||||||||||||||||||||||||||||||||||||
| JwtTokenResponseDto jwtDto = jwtTokenService.createTokenDto(authResultDto.getMemberId(), | ||||||||||||||||||||||||||||||||||||
| authResultDto.getProfileId()); | ||||||||||||||||||||||||||||||||||||
| if (authResultDto.isNewUser()) { | ||||||||||||||||||||||||||||||||||||
| jwtDto.setNewUser(true); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| return ApiResponse.createSuccess(jwtDto); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @PostMapping("/token/refresh") | ||||||||||||||||||||||||||||||||||||
| public ApiResponse<AccessTokenRefreshResponse> refreshAccessToken(@UserContext MemberDto memberDto, | ||||||||||||||||||||||||||||||||||||
| @RequestBody RefreshTokenRequest request) { | ||||||||||||||||||||||||||||||||||||
| String accessToken = jwtTokenService.createAccessToken(memberDto.getMemberId(), memberDto.getProfileId()); | ||||||||||||||||||||||||||||||||||||
| return ApiResponse.createSuccess(new AccessTokenRefreshResponse(accessToken, 3600, "Bearer")); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+73
to
+78
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 리프레시 토큰 검증이 전혀 이루어지지 않습니다
-@PostMapping("/token/refresh")
-public ApiResponse<AccessTokenRefreshResponse> refreshAccessToken(@UserContext MemberDto memberDto,
- @RequestBody RefreshTokenRequest request) {
- String accessToken = jwtTokenService.createAccessToken(memberDto.getMemberId(), memberDto.getProfileId());
+@PostMapping("/token/refresh")
+public ApiResponse<AccessTokenRefreshResponse> refreshAccessToken(@RequestBody RefreshTokenRequest request) {
+ jwtTokenService.validateRefreshToken(request.getRefreshToken()); // 서명·만료 검증
+ String accessToken = jwtTokenService.createAccessTokenFromRefresh(request.getRefreshToken());
return ApiResponse.createSuccess(new AccessTokenRefreshResponse(accessToken, 3600, "Bearer"));
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @Data | ||||||||||||||||||||||||||||||||||||
| @AllArgsConstructor | ||||||||||||||||||||||||||||||||||||
| public static class EmailLoginRequest { | ||||||||||||||||||||||||||||||||||||
| @NotEmpty | ||||||||||||||||||||||||||||||||||||
| private String email; | ||||||||||||||||||||||||||||||||||||
| @NotEmpty | ||||||||||||||||||||||||||||||||||||
| private String password; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @Data | ||||||||||||||||||||||||||||||||||||
| @AllArgsConstructor | ||||||||||||||||||||||||||||||||||||
| public static class SocialLoginRequest { | ||||||||||||||||||||||||||||||||||||
| @NotEmpty | ||||||||||||||||||||||||||||||||||||
| private String provider; | ||||||||||||||||||||||||||||||||||||
| @NotEmpty | ||||||||||||||||||||||||||||||||||||
| private String authorizationCode; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @Data | ||||||||||||||||||||||||||||||||||||
| public static class RefreshTokenRequest { | ||||||||||||||||||||||||||||||||||||
| @NotEmpty | ||||||||||||||||||||||||||||||||||||
| private String refreshToken; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @Data | ||||||||||||||||||||||||||||||||||||
| @AllArgsConstructor | ||||||||||||||||||||||||||||||||||||
| public static class AccessTokenRefreshResponse { | ||||||||||||||||||||||||||||||||||||
| private String accessToken; | ||||||||||||||||||||||||||||||||||||
| private int expiresIn; | ||||||||||||||||||||||||||||||||||||
| private String tokenType; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
member 파라미터에 대한 NPE 가능성 존재
@Builder로 생성될 때member인자가null이면linkMember(null)호출에서NullPointerException이 발생합니다.엔티티 생성 시점에 외부에서
member를 반드시 주입하지 않는 패스가 있다면 널 체크를 한 뒤 예외를 던지거나, 별도의 팩토리 메서드로 책임을 분리하는 편이 안전합니다.📝 Committable suggestion
🤖 Prompt for AI Agents