-
Couldn't load subscription status.
- Fork 0
Merge Master #1
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
Merge Master #1
Changes from all commits
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,14 @@ | ||
| package com.ceos22.spring_boot.common.auth.dto; | ||
|
|
||
| import io.swagger.v3.oas.annotations.media.Schema; | ||
| import jakarta.validation.constraints.NotBlank; | ||
|
|
||
| public record LoginRequestDto( | ||
| @NotBlank | ||
| @Schema(description = "사용자 이름", example = "evan523") | ||
| String username, | ||
|
|
||
| @NotBlank | ||
| @Schema(description = "비밀번호", example = "password123") | ||
| String password | ||
| ) {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package com.ceos22.spring_boot.common.auth.dto; | ||
|
|
||
| import com.ceos22.spring_boot.domain.user.User; | ||
|
|
||
| import java.util.UUID; | ||
|
|
||
| public record LoginResponseDto( | ||
| String accessToken, | ||
| String tokenType, | ||
| UUID publicId, | ||
| String username, | ||
| String name, | ||
| String email, | ||
| long expiresIn | ||
| ) { | ||
| public static LoginResponseDto of(String token, User user, long expiresIn) { | ||
| return new LoginResponseDto( | ||
| token, | ||
| "Bearer", | ||
| user.getPublicId(), | ||
| user.getUsername(), | ||
| user.getName(), | ||
| user.getEmail(), | ||
| expiresIn | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,12 +11,14 @@ | |
| import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; | ||
| import org.springframework.security.core.Authentication; | ||
| import org.springframework.security.core.authority.SimpleGrantedAuthority; | ||
| import lombok.extern.slf4j.Slf4j; | ||
|
|
||
| import java.nio.charset.StandardCharsets; | ||
| import java.security.Key; | ||
| import java.util.*; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| @Slf4j | ||
| public class JwtTokenProvider { | ||
|
|
||
| private static final String AUTH_CLAIM = "auth"; | ||
|
|
@@ -26,7 +28,7 @@ public class JwtTokenProvider { | |
|
|
||
| public JwtTokenProvider(JwtProperties props, UserRepository users) { | ||
| this.key = Keys.hmacShaKeyFor(props.secret().getBytes(StandardCharsets.UTF_8)); | ||
| this.validityMillis = props.accessTokenValidityInSeconds() * 1000L; | ||
| this.validityMillis = props.accessTokenValidity().toMillis(); | ||
| this.users = users; | ||
| } | ||
|
|
||
|
|
@@ -37,8 +39,17 @@ public String issueToken(Authentication authentication) { | |
| .map(a -> a.getAuthority()) | ||
| .collect(Collectors.joining(",")); | ||
|
|
||
| Object principal = authentication.getPrincipal(); | ||
|
|
||
| String subject; | ||
| if (principal instanceof CustomUserPrincipal customPrincipal) { | ||
| subject = customPrincipal.getPublicId().toString(); | ||
| } else { | ||
| subject = authentication.getName(); | ||
| } | ||
|
|
||
| return Jwts.builder() | ||
| .setSubject(authentication.getName()) | ||
| .setSubject(subject) | ||
| .claim(AUTH_CLAIM, authorities) | ||
| .setIssuedAt(now) | ||
| .setExpiration(expiry) | ||
|
|
@@ -48,16 +59,25 @@ public String issueToken(Authentication authentication) { | |
|
|
||
| public Authentication getAuthentication(String token) { | ||
| Claims claims = parseClaims(token); | ||
| String username = claims.getSubject(); | ||
|
|
||
| String subject = claims.getSubject(); | ||
| UUID publicId; | ||
| try { | ||
| publicId = UUID.fromString(subject); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new GeneralException(ErrorStatus.INVALID_TOKEN, "잘못된 토큰 형식입니다: " + subject); | ||
| } | ||
|
|
||
| String authStr = claims.get(AUTH_CLAIM, String.class); | ||
| var authorities = Optional.ofNullable(authStr) | ||
| .stream() | ||
| .flatMap(s -> Arrays.stream(s.split(","))) | ||
| .filter(s -> !s.isBlank()) | ||
| .map(SimpleGrantedAuthority::new) | ||
| .collect(Collectors.toList()); | ||
| User u = users.findByUsername(username) | ||
| .orElseThrow(() -> new GeneralException(ErrorStatus._BAD_REQUEST, "사용자를 찾을 수 없습니다: " + username)); | ||
| User u = users.findByPublicId(publicId) | ||
| .orElseThrow(() -> new GeneralException(ErrorStatus._BAD_REQUEST, "사용자를 찾을 수 없습니다: " + publicId)); | ||
|
Comment on lines
+78
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. style: Database lookup on every request for token validation can impact performance. Consider caching user data or only fetching when necessary. Prompt To Fix With AIThis is a comment left during a code review.
Path: cgvclone/src/main/java/com/ceos22/spring_boot/common/auth/security/jwt/JwtTokenProvider.java
Line: 70:71
Comment:
**style:** Database lookup on every request for token validation can impact performance. Consider caching user data or only fetching when necessary.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
|
|
||
| var principal = new CustomUserPrincipal( | ||
| u.getUserId(), | ||
|
|
@@ -77,14 +97,23 @@ public boolean validate(String token) { | |
| parseClaims(token); | ||
| return true; | ||
| } catch (ExpiredJwtException e) { | ||
| return false; | ||
| log.info("JWT expired at {}", e.getClaims().getExpiration()); | ||
| throw new GeneralException(ErrorStatus.TOKEN_EXPIRED, "토큰이 만료되었습니다."); | ||
| } catch (JwtException | IllegalArgumentException e) { | ||
| return false; | ||
| throw new GeneralException(ErrorStatus.INVALID_TOKEN, "유효하지 않은 토큰입니다."); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| private Claims parseClaims(String token) { | ||
| return Jwts.parserBuilder().setSigningKey(key).build() | ||
| .parseClaimsJws(token).getBody(); | ||
| } | ||
|
|
||
| public long getExpiration(String token) { | ||
| Claims claims = parseClaims(token); | ||
| Date expiration = claims.getExpiration(); | ||
| return (expiration.getTime() - System.currentTimeMillis()) / 1000L; | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| package com.ceos22.spring_boot.common.enums; | ||
|
|
||
| public enum PaymentMethod { | ||
| 카드, 현금, 네이버페이, 카카오페이 | ||
| CARD, CASH, NAVER, KAKAO | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -37,11 +37,6 @@ public ResponseEntity<ApiResponse<Void>> handleNoResourceFoundException(NoResour | |||||||||||||||||||
| return ApiResponse.onFailure(ErrorStatus._NOT_FOUND); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @ExceptionHandler(Exception.class) | ||||||||||||||||||||
| public ResponseEntity<ApiResponse<Void>> handleException(Exception e) { | ||||||||||||||||||||
| return ApiResponse.onFailure(ErrorStatus._INTERNAL_SERVER_ERROR); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @ExceptionHandler(GeneralException.class) | ||||||||||||||||||||
| public ResponseEntity<ApiResponse<Void>> handleGeneralException(GeneralException e) { | ||||||||||||||||||||
| return ApiResponse.onFailure(e.getErrorStatus(), e.getMessage()); | ||||||||||||||||||||
|
|
@@ -58,5 +53,9 @@ public ResponseEntity<ApiResponse<Void>> handleAuthFailure(AuthFailureException | |||||||||||||||||||
| return ApiResponse.onFailure(e.getErrorStatus(), e.getMessage()); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @ExceptionHandler(Exception.class) | ||||||||||||||||||||
| public ResponseEntity<ApiResponse<Void>> handleException(Exception e) { | ||||||||||||||||||||
| return ApiResponse.onFailure(ErrorStatus._INTERNAL_SERVER_ERROR); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+56
to
+59
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. style: Log the exception details before returning generic error response to aid debugging of unexpected errors
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: cgvclone/src/main/java/com/ceos22/spring_boot/common/exception/GlobalExceptionHandler.java
Line: 56:59
Comment:
**style:** Log the exception details before returning generic error response to aid debugging of unexpected errors
```suggestion
@ExceptionHandler(Exception.class)
public ResponseEntity<ApiResponse<Void>> handleException(Exception e) {
log.error("Unhandled exception: ", e);
return ApiResponse.onFailure(ErrorStatus._INTERNAL_SERVER_ERROR);
}
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||
|
|
||||||||||||||||||||
| } | ||||||||||||||||||||
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.
style: fetching user again by username after authentication succeeds is redundant - the authenticated principal already contains user details. Can you extract user details from the Authentication object instead of querying the database again?
Prompt To Fix With AI