-
Notifications
You must be signed in to change notification settings - Fork 1
[feat] JWT 기반 로그인, 회원가입 구현 #44
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
Conversation
Walkthrough이 변경 사항은 스프링 시큐리티 및 JWT 기반 인증 기능을 프로젝트에 도입합니다. 주요 내용은 보안 관련 의존성 추가, JWT 설정 클래스 및 필터 구현, 사용자 회원가입 및 로그인 엔드포인트 신설, 사용자 엔티티 및 서비스의 확장, DTO와 예외 케이스 추가 등입니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UserController
participant UserService
participant AuthService
participant UserRepository
participant PasswordEncoder
participant JwtTokenProvider
Client->>UserController: POST /api/v1/users/signup (UserRegisterRequestDto)
UserController->>UserService: signup(req)
UserService->>UserRepository: existsByEmail(email)
alt 이메일 중복
UserService-->>UserController: ApplicationException(USER_ALREADY_EXISTS)
else 신규 회원
UserService->>PasswordEncoder: encode(password)
UserService->>UserRepository: save(User)
UserService-->>UserController: void
end
UserController-->>Client: 회원가입 성공 응답
Client->>UserController: POST /api/v1/users/login (UserLoginRequestDto)
UserController->>AuthService: login(req)
AuthService->>UserRepository: findByEmail(email)
alt 사용자 없음
AuthService-->>UserController: ApplicationException(INVALID_CREDENTIALS)
else 사용자 있음
AuthService->>PasswordEncoder: matches(raw, encoded)
alt 불일치
AuthService-->>UserController: ApplicationException(INVALID_CREDENTIALS)
else 일치
AuthService->>JwtTokenProvider: generateToken(email, role)
AuthService-->>UserController: TokenResponseDto
end
end
UserController-->>Client: JWT 토큰 응답
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
src/main/java/com/wayble/server/user/entity/User.java (1)
36-37: TODO 주석 삭제: User 엔티티에서 비밀번호 암호화 주석 제거SecurityConfig에서 BCryptPasswordEncoder가 빈으로 등록되고, UserService에서
passwordEncoder.encode()를 통해 이미 비밀번호 암호화를 수행하므로 User 엔티티에 남아있는 TODO 주석은 불필요합니다. 아래 위치에서 해당 주석을 삭제해주세요.
- 파일: src/main/java/com/wayble/server/user/entity/User.java
- 라인: 36
제안하는 수정:
- // TODO: 비밀번호 암호화 필요
🧹 Nitpick comments (6)
src/main/java/com/wayble/server/user/entity/User.java (1)
68-88: 정적 팩토리 메서드 구현이 우수합니다
createUser메서드가 잘 구현되었습니다:
- 명확한 파라미터 목록
- 빌더 패턴 캡슐화
- 객체 생성 로직 중앙화
하지만 파라미터 검증 추가를 고려해보세요:
public static User createUser( String nickname, String username, String email, String password, LocalDate birthDate, Gender gender, LoginType loginType, UserType userType ) { + if (email == null || email.trim().isEmpty()) { + throw new IllegalArgumentException("Email cannot be null or empty"); + } + if (password == null || password.trim().isEmpty()) { + throw new IllegalArgumentException("Password cannot be null or empty"); + } + return User.builder() .nickname(nickname) .username(username) .email(email) .password(password) .birthDate(birthDate) .gender(gender) .loginType(loginType) .userType(userType) .build(); }src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthenticationFilter.java (1)
28-32: 예외 처리 추가를 고려해보세요.JWT 토큰 검증 과정에서 발생할 수 있는 예외(만료, 잘못된 형식 등)에 대한 처리가 없습니다. 토큰 파싱 오류가 발생할 경우 필터가 실패할 수 있습니다.
if (jwtProvider.validateToken(token)) { - String email = jwtProvider.getEmail(token); - var authentication = new JwtAuthentication(email); - SecurityContextHolder.getContext().setAuthentication(authentication); + try { + String email = jwtProvider.getEmail(token); + var authentication = new JwtAuthentication(email); + SecurityContextHolder.getContext().setAuthentication(authentication); + } catch (Exception e) { + SecurityContextHolder.clearContext(); + } }src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthentication.java (1)
10-14: 권한 관리 개선이 필요합니다.현재 모든 사용자에게 "ROLE_USER" 권한만 부여하고 있습니다. 향후 관리자나 다른 역할이 필요한 경우를 고려하여 사용자 타입에 따른 동적 권한 할당을 고려해보세요.
-public JwtAuthentication(String email) { - super(List.of(new SimpleGrantedAuthority("ROLE_USER"))); +public JwtAuthentication(String email, String userType) { + super(List.of(new SimpleGrantedAuthority("ROLE_" + userType))); this.email = email; setAuthenticated(true); }src/main/java/com/wayble/server/common/config/SecurityConfig.java (1)
30-47: CORS 설정 추가를 고려해보세요.프론트엔드와의 통신을 위해 CORS 설정이 필요할 수 있습니다.
http .csrf(csrf -> csrf.disable()) + .cors(cors -> cors.configurationSource(corsConfigurationSource())) .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS))src/main/java/com/wayble/server/user/service/auth/AuthService.java (1)
22-30: 로그인 시도 제한 기능 추가를 고려해보세요.브루트 포스 공격을 방지하기 위해 로그인 시도 제한 기능을 추가하는 것을 고려해보세요. Redis나 메모리 기반 캐시를 활용하여 IP별 또는 이메일별 로그인 시도 횟수를 제한할 수 있습니다.
src/main/java/com/wayble/server/user/dto/UserLoginRequestDto.java (1)
6-16: 로그인 DTO에서 불필요한 name 필드 제거를 고려해보세요.일반적으로 로그인 프로세스에서는 이메일과 비밀번호만 필요합니다. name 필드가 실제 로그인 로직에서 사용되지 않는다면 제거하는 것이 좋겠습니다.
public record UserLoginRequestDto( - @NotBlank(message = "이름 또는 닉네임은 필수입니다") - String name, - @NotBlank(message = "이메일은 필수입니다") @Email(message = "유효한 이메일 형식이 아닙니다") String email, @NotBlank(message = "비밀번호는 필수입니다") String password ) {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
build.gradle(1 hunks)src/main/java/com/wayble/server/common/config/SecurityConfig.java(1 hunks)src/main/java/com/wayble/server/common/config/security/JwtProperties.java(1 hunks)src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthentication.java(1 hunks)src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthenticationFilter.java(1 hunks)src/main/java/com/wayble/server/common/config/security/jwt/JwtTokenProvider.java(1 hunks)src/main/java/com/wayble/server/user/controller/UserController.java(1 hunks)src/main/java/com/wayble/server/user/dto/UserLoginRequestDto.java(1 hunks)src/main/java/com/wayble/server/user/dto/UserRegisterRequestDto.java(1 hunks)src/main/java/com/wayble/server/user/dto/token/TokenResponseDto.java(1 hunks)src/main/java/com/wayble/server/user/entity/User.java(1 hunks)src/main/java/com/wayble/server/user/exception/UserErrorCase.java(1 hunks)src/main/java/com/wayble/server/user/service/UserService.java(1 hunks)src/main/java/com/wayble/server/user/service/auth/AuthService.java(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
src/main/java/com/wayble/server/user/exception/UserErrorCase.java (1)
Learnt from: seung-in-Yoo
PR: Wayble-Project/wayble-spring#37
File: src/main/java/com/wayble/server/user/entity/User.java:61-62
Timestamp: 2025-07-13T15:08:59.288Z
Learning: Java에서 같은 패키지 내의 클래스들은 import 구문 없이 서로를 참조할 수 있다. com.wayble.server.user.entity 패키지 내의 클래스들은 명시적인 import 없이 사용 가능하다.
src/main/java/com/wayble/server/user/entity/User.java (1)
Learnt from: seung-in-Yoo
PR: Wayble-Project/wayble-spring#37
File: src/main/java/com/wayble/server/user/entity/User.java:61-62
Timestamp: 2025-07-13T15:08:59.288Z
Learning: Java에서 같은 패키지 내의 클래스들은 import 구문 없이 서로를 참조할 수 있다. com.wayble.server.user.entity 패키지 내의 클래스들은 명시적인 import 없이 사용 가능하다.
src/main/java/com/wayble/server/user/service/UserService.java (1)
Learnt from: seung-in-Yoo
PR: Wayble-Project/wayble-spring#37
File: src/main/java/com/wayble/server/user/entity/User.java:61-62
Timestamp: 2025-07-13T15:08:59.288Z
Learning: Java에서 같은 패키지 내의 클래스들은 import 구문 없이 서로를 참조할 수 있다. com.wayble.server.user.entity 패키지 내의 클래스들은 명시적인 import 없이 사용 가능하다.
src/main/java/com/wayble/server/user/dto/UserLoginRequestDto.java (1)
Learnt from: seung-in-Yoo
PR: Wayble-Project/wayble-spring#37
File: src/main/java/com/wayble/server/user/entity/User.java:61-62
Timestamp: 2025-07-13T15:08:59.288Z
Learning: Java에서 같은 패키지 내의 클래스들은 import 구문 없이 서로를 참조할 수 있다. com.wayble.server.user.entity 패키지 내의 클래스들은 명시적인 import 없이 사용 가능하다.
🧬 Code Graph Analysis (4)
src/main/java/com/wayble/server/common/config/SecurityConfig.java (1)
src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthenticationFilter.java (1)
RequiredArgsConstructor(13-37)
src/main/java/com/wayble/server/common/config/security/JwtProperties.java (1)
src/main/java/com/wayble/server/common/config/security/jwt/JwtTokenProvider.java (1)
Component(12-44)
src/main/java/com/wayble/server/user/service/auth/AuthService.java (1)
src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthenticationFilter.java (1)
RequiredArgsConstructor(13-37)
src/main/java/com/wayble/server/user/controller/UserController.java (1)
src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthenticationFilter.java (1)
RequiredArgsConstructor(13-37)
🔇 Additional comments (24)
src/main/java/com/wayble/server/user/dto/token/TokenResponseDto.java (1)
1-3: 구현이 적절합니다JWT 토큰 응답을 위한 간단하고 명확한 record 구현입니다. 불변성을 보장하고 용도에 맞게 설계되었습니다.
src/main/java/com/wayble/server/user/exception/UserErrorCase.java (1)
14-16: 에러 케이스 추가가 적절합니다인증 플로우에 필요한 에러 케이스들이 적절히 추가되었습니다:
USER_ALREADY_EXISTS: 회원가입 시 중복 이메일 처리INVALID_CREDENTIALS: 로그인 실패 처리HTTP 상태 코드와 에러 코드도 적절하게 설정되었습니다.
src/main/java/com/wayble/server/user/service/UserService.java (2)
21-23: 이메일 중복 확인 로직이 올바르게 구현되었습니다.
existsByEmail메소드를 사용하여 효율적으로 중복 확인을 수행하고, 적절한 예외 처리를 하고 있습니다.
24-34: 사용자 생성 로직이 안전하게 구현되었습니다.정적 팩토리 메소드를 사용하여 사용자 객체를 생성하고, 비밀번호를 안전하게 인코딩하여 저장하고 있습니다.
src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthenticationFilter.java (1)
25-26: 토큰 추출 로직이 올바르게 구현되었습니다."Bearer " 접두사를 정확히 확인하고 토큰을 추출하는 로직이 적절합니다.
src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthentication.java (1)
16-17: 인증 정보 접근 메소드가 올바르게 구현되었습니다.credentials와 principal 메소드가 적절히 구현되어 있습니다.
src/main/java/com/wayble/server/common/config/SecurityConfig.java (3)
32-33: 무상태 세션 관리가 올바르게 설정되었습니다.JWT 토큰 기반 인증에 적합한 무상태 세션 정책과 CSRF 비활성화가 적절하게 구성되어 있습니다.
34-43: 엔드포인트 접근 제어가 적절히 구성되었습니다.공개 엔드포인트(회원가입, 로그인, API 문서)에 대한 접근 허용과 나머지 엔드포인트에 대한 인증 요구가 올바르게 설정되어 있습니다.
44-44: JWT 필터 순서가 올바르게 설정되었습니다.JWT 인증 필터가 UsernamePasswordAuthenticationFilter 이전에 올바르게 배치되어 있습니다.
src/main/java/com/wayble/server/user/service/auth/AuthService.java (3)
22-30: 로그인 로직이 보안 모범 사례에 따라 구현되었습니다.사용자 존재 여부와 비밀번호 오류를 동일한 예외(
INVALID_CREDENTIALS)로 처리하여 타이밍 공격을 방지하고 있습니다. 이는 보안상 올바른 접근 방식입니다.
25-27: 비밀번호 검증 로직이 안전하게 구현되었습니다.
PasswordEncoder.matches()메소드를 사용하여 안전하게 비밀번호를 검증하고 있습니다.
28-29: JWT 토큰 생성이 적절히 구현되었습니다.사용자 이메일과 사용자 타입을 포함하여 JWT 토큰을 생성하고 적절한 응답 DTO로 래핑하고 있습니다.
src/main/java/com/wayble/server/user/dto/UserLoginRequestDto.java (2)
10-12: 검증 로직이 올바르게 구현되었습니다.이메일 필드의
@NotBlank와
14-15: 비밀번호 검증이 적절합니다.비밀번호 필드의
@NotBlank검증이 올바르게 구현되었습니다.src/main/java/com/wayble/server/user/dto/UserRegisterRequestDto.java (5)
11-13: 닉네임 검증 로직이 적절합니다.
@NotBlank와@Size(max = 8)어노테이션이 올바르게 적용되어 닉네임의 필수 입력과 길이 제한이 구현되었습니다.
18-20: 이메일 검증이 올바르게 구현되었습니다.
@NotBlank와
22-24: 비밀번호 보안 정책이 적절합니다.비밀번호 최소 8자 제한은 일반적인 보안 정책에 부합하며,
@NotBlank와@Size(min = 8)어노테이션이 올바르게 적용되었습니다.
26-28: 생년월일 검증이 올바릅니다.
@NotNull과@Past어노테이션을 통해 생년월일이 반드시 과거 날짜여야 한다는 제약 조건이 적절히 구현되었습니다.
30-37: Enum 타입 필드들의 검증이 적절합니다.Gender, LoginType, UserType 필드들에
@NotNull어노테이션이 올바르게 적용되어 필수 입력이 보장됩니다.src/main/java/com/wayble/server/user/controller/UserController.java (3)
19-22: 컨트롤러 설정이 적절합니다.
@RequestMapping을 통한 베이스 경로 설정과@Validated어노테이션이 올바르게 적용되었습니다.
28-42: 회원가입 엔드포인트가 올바르게 구현되었습니다.POST 메서드 사용,
@Valid어노테이션을 통한 요청 검증, 그리고 상세한 Swagger 문서화가 잘 구현되었습니다. API 응답 코드와 설명도 적절합니다.
44-58: 로그인 엔드포인트가 보안 요구사항에 맞게 구현되었습니다.JWT 토큰을 반환하는 로그인 엔드포인트가 올바르게 구현되었습니다.
@Valid검증과 적절한 응답 타입(TokenResponseDto) 사용이 좋습니다.src/main/java/com/wayble/server/common/config/security/jwt/JwtTokenProvider.java (2)
21-29: 토큰 생성 로직이 올바르게 구현되었습니다.subject에 이메일, claim에 role 설정, 그리고 적절한 만료 시간 설정이 잘 구현되었습니다. HS256 알고리즘 사용도 적절합니다.
31-38: 토큰 검증 로직이 안전하게 구현되었습니다.예외 처리를 통해 잘못된 토큰에 대해 false를 반환하는 로직이 적절합니다.
JwtException과IllegalArgumentException을 모두 처리한 것이 좋습니다.
| implementation 'io.jsonwebtoken:jjwt-api:0.11.5' | ||
| implementation 'jakarta.servlet:jakarta.servlet-api:6.0.0' | ||
| runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.11.5' | ||
| runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.11.5' |
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.
💡 Verification agent
🧩 Analysis chain
JWT 라이브러리 버전 확인 필요
JJWT 0.11.5 버전이 사용되고 있는데, 최신 버전과 보안 취약점 여부를 확인해야 합니다.
🌐 Web query:
What is the latest version of io.jsonwebtoken jjwt library and are there any security vulnerabilities in version 0.11.5?
💡 Result:
The latest version of the io.jsonwebtoken (JJWT) library is 0.12.6. (javadoc.io)
Regarding version 0.11.5, there are no direct vulnerabilities reported for this version in Snyk's vulnerability database. (security.snyk.io)
However, it's important to note that version 0.11.5 is outdated, and the latest version, 0.12.6, includes several updates and fixes. (github.com)
To ensure your application benefits from the latest security patches and features, it's recommended to upgrade to version 0.12.6.
Citations:
- 1: https://javadoc.io/doc/io.jsonwebtoken?utm_source=openai
- 2: https://security.snyk.io/package/maven/io.jsonwebtoken%3Ajjwt?utm_source=openai
- 3: https://github.com/jwtk/jjwt/blob/master/CHANGELOG.md?utm_source=openai
JWT 라이브러리 버전 업그레이드 권고
현재 io.jsonwebtoken:jjwt 0.11.5에는 알려진 취약점은 없으나, 최신 버전인 0.12.6 에 보안 패치와 기능 개선이 포함되어 있습니다. 안정성과 최신 기능 활용을 위해 의존성을 다음과 같이 업데이트하시기 바랍니다.
· 대상 파일: build.gradle (36–39줄)
· 변경 전:
implementation 'io.jsonwebtoken:jjwt-api:0.11.5'
runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.11.5'
runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.11.5'
· 제안하는 변경:
- implementation 'io.jsonwebtoken:jjwt-api:0.11.5'
- runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.11.5'
- runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.11.5'
+ implementation 'io.jsonwebtoken:jjwt-api:0.12.6'
+ runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.12.6'
+ runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.12.6'📝 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.
| implementation 'io.jsonwebtoken:jjwt-api:0.11.5' | |
| implementation 'jakarta.servlet:jakarta.servlet-api:6.0.0' | |
| runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.11.5' | |
| runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.11.5' | |
| implementation 'io.jsonwebtoken:jjwt-api:0.12.6' | |
| implementation 'jakarta.servlet:jakarta.servlet-api:6.0.0' | |
| runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.12.6' | |
| runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.12.6' |
🤖 Prompt for AI Agents
In build.gradle between lines 36 and 39, the JWT library dependencies are using
version 0.11.5, which is outdated. Update all three dependencies 'jjwt-api',
'jjwt-impl', and 'jjwt-jackson' to version 0.12.6 to include the latest security
patches and feature improvements. Replace the version numbers in the
implementation and runtimeOnly lines accordingly.
| @Component | ||
| @ConfigurationProperties(prefix = "jwt") | ||
| public class JwtProperties { | ||
|
|
||
| private String secret; | ||
| private long accessExp; | ||
|
|
||
|
|
||
| public String getSecret() { | ||
| return secret; | ||
| } | ||
|
|
||
| public long getAccessExp() { | ||
| return accessExp; | ||
| } | ||
|
|
||
| public void setSecret(String secret) { | ||
| this.secret = secret; | ||
| } | ||
|
|
||
| public void setAccessExp(long accessExp) { | ||
| this.accessExp = accessExp; | ||
| } | ||
| } |
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
JWT 설정 검증 및 보안 강화 필요
현재 구현에서 다음 사항들이 개선되어야 합니다:
- 비밀키 강도 검증 부족: JWT 비밀키의 최소 길이나 강도에 대한 검증이 없습니다.
- 기본값 부재: 설정이 누락될 경우를 대비한 기본값이 없습니다.
- 민감한 정보 노출 위험: 비밀키가 로그에 노출될 수 있습니다.
다음과 같이 개선하는 것을 권장합니다:
@Component
@ConfigurationProperties(prefix = "jwt")
+@Validated
public class JwtProperties {
+ @NotBlank(message = "JWT secret key cannot be blank")
+ @Size(min = 32, message = "JWT secret key must be at least 32 characters")
private String secret;
+
+ @Positive(message = "Access token expiration must be positive")
private long accessExp;
+ @PostConstruct
+ public void validateSecretKey() {
+ if (secret.getBytes().length < 32) {
+ throw new IllegalArgumentException("JWT secret key must be at least 256 bits (32 bytes)");
+ }
+ }
// getters and setters...
+
+ @Override
+ public String toString() {
+ return "JwtProperties{accessExp=" + accessExp + ", secret=[PROTECTED]}";
+ }
}📝 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.
| @Component | |
| @ConfigurationProperties(prefix = "jwt") | |
| public class JwtProperties { | |
| private String secret; | |
| private long accessExp; | |
| public String getSecret() { | |
| return secret; | |
| } | |
| public long getAccessExp() { | |
| return accessExp; | |
| } | |
| public void setSecret(String secret) { | |
| this.secret = secret; | |
| } | |
| public void setAccessExp(long accessExp) { | |
| this.accessExp = accessExp; | |
| } | |
| } | |
| @Component | |
| @ConfigurationProperties(prefix = "jwt") | |
| @Validated | |
| public class JwtProperties { | |
| @NotBlank(message = "JWT secret key cannot be blank") | |
| @Size(min = 32, message = "JWT secret key must be at least 32 characters") | |
| private String secret; | |
| @Positive(message = "Access token expiration must be positive") | |
| private long accessExp; | |
| @PostConstruct | |
| public void validateSecretKey() { | |
| if (secret.getBytes().length < 32) { | |
| throw new IllegalArgumentException("JWT secret key must be at least 256 bits (32 bytes)"); | |
| } | |
| } | |
| public String getSecret() { | |
| return secret; | |
| } | |
| public long getAccessExp() { | |
| return accessExp; | |
| } | |
| public void setSecret(String secret) { | |
| this.secret = secret; | |
| } | |
| public void setAccessExp(long accessExp) { | |
| this.accessExp = accessExp; | |
| } | |
| @Override | |
| public String toString() { | |
| return "JwtProperties{accessExp=" + accessExp + ", secret=[PROTECTED]}"; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/common/config/security/JwtProperties.java
lines 6 to 29, add validation logic to ensure the JWT secret key meets a minimum
length or strength requirement to enhance security. Provide sensible default
values for secret and accessExp fields to handle missing configuration
gracefully. Also, avoid exposing the secret key in logs or error messages by
removing or masking it wherever it might be logged.
| public void signup(UserRegisterRequestDto req) { | ||
| if (userRepository.existsByEmail(req.email())) { | ||
| throw new ApplicationException(UserErrorCase.USER_ALREADY_EXISTS); | ||
| } | ||
| User user = User.createUser( | ||
| req.nickname(), | ||
| req.username(), | ||
| req.email(), | ||
| passwordEncoder.encode(req.password()), | ||
| req.birthDate(), | ||
| req.gender(), | ||
| req.loginType(), | ||
| req.userType() | ||
| ); | ||
| userRepository.save(user); | ||
| } |
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
트랜잭션 관리 추가가 필요합니다.
회원가입 메소드에서 이메일 중복 확인과 사용자 저장 사이에 동시성 문제가 발생할 수 있습니다. 두 작업 사이에 다른 스레드에서 같은 이메일로 사용자를 생성할 수 있어 race condition이 발생할 가능성이 있습니다.
+import org.springframework.transaction.annotation.Transactional;
+@Transactional
public void signup(UserRegisterRequestDto req) {🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/user/service/UserService.java around lines 20
to 35, the signup method lacks transaction management, which can cause race
conditions between the email existence check and user saving. To fix this,
annotate the signup method with a transactional annotation (e.g.,
@Transactional) to ensure atomicity and prevent concurrent threads from creating
users with the same email simultaneously.
| String header = req.getHeader("Authorization"); | ||
|
|
||
| if (header != null && header.startsWith("Bearer ")) { | ||
| String token = header.substring(7); | ||
|
|
||
| if (jwtProvider.validateToken(token)) { | ||
| String email = jwtProvider.getEmail(token); | ||
| var authentication = new JwtAuthentication(email); | ||
| SecurityContextHolder.getContext().setAuthentication(authentication); | ||
| } | ||
| } |
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
토큰 무효화 시 SecurityContext 정리가 필요합니다.
토큰이 유효하지 않거나 검증에 실패한 경우, 기존 SecurityContext를 명시적으로 정리하는 것이 보안상 좋습니다.
if (header != null && header.startsWith("Bearer ")) {
String token = header.substring(7);
if (jwtProvider.validateToken(token)) {
String email = jwtProvider.getEmail(token);
var authentication = new JwtAuthentication(email);
SecurityContextHolder.getContext().setAuthentication(authentication);
+ } else {
+ SecurityContextHolder.clearContext();
}
+} else {
+ SecurityContextHolder.clearContext();
}📝 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.
| String header = req.getHeader("Authorization"); | |
| if (header != null && header.startsWith("Bearer ")) { | |
| String token = header.substring(7); | |
| if (jwtProvider.validateToken(token)) { | |
| String email = jwtProvider.getEmail(token); | |
| var authentication = new JwtAuthentication(email); | |
| SecurityContextHolder.getContext().setAuthentication(authentication); | |
| } | |
| } | |
| String header = req.getHeader("Authorization"); | |
| if (header != null && header.startsWith("Bearer ")) { | |
| String token = header.substring(7); | |
| if (jwtProvider.validateToken(token)) { | |
| String email = jwtProvider.getEmail(token); | |
| var authentication = new JwtAuthentication(email); | |
| SecurityContextHolder.getContext().setAuthentication(authentication); | |
| } else { | |
| SecurityContextHolder.clearContext(); | |
| } | |
| } else { | |
| SecurityContextHolder.clearContext(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthenticationFilter.java
around lines 23 to 33, the SecurityContext is not cleared when the JWT token is
invalid or fails validation. To fix this, explicitly clear the SecurityContext
by calling SecurityContextHolder.clearContext() in the else branch or after the
token validation check fails, ensuring no stale authentication remains.
| private Key getSigningKey() { | ||
| return Keys.hmacShaKeyFor(jwtProperties.getSecret().getBytes()); | ||
| } |
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
서명 키 생성 성능 개선이 필요합니다.
매번 새로운 키를 생성하는 것은 비효율적입니다. 키를 캐싱하여 성능을 향상시키세요.
@Component
@RequiredArgsConstructor
public class JwtTokenProvider {
private final JwtProperties jwtProperties;
+ private Key signingKey;
- private Key getSigningKey() {
- return Keys.hmacShaKeyFor(jwtProperties.getSecret().getBytes());
- }
+ private Key getSigningKey() {
+ if (signingKey == null) {
+ signingKey = Keys.hmacShaKeyFor(jwtProperties.getSecret().getBytes());
+ }
+ return signingKey;
+ }🤖 Prompt for AI Agents
In
src/main/java/com/wayble/server/common/config/security/jwt/JwtTokenProvider.java
around lines 17 to 19, the getSigningKey method creates a new signing key on
every call, which is inefficient. To improve performance, modify the class to
generate the signing key once, cache it in a private final field, and return the
cached key in getSigningKey instead of recreating it each time.
| public String getEmail(String token) { | ||
| return Jwts.parserBuilder().setSigningKey(getSigningKey()).build() | ||
| .parseClaimsJws(token).getBody().getSubject(); | ||
| } |
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.
보안 취약점: 토큰 검증 없이 파싱하고 있습니다.
getEmail() 메서드에서 토큰 검증 없이 바로 파싱하는 것은 보안상 위험합니다. 토큰 유효성을 먼저 검증해야 합니다.
public String getEmail(String token) {
+ if (!validateToken(token)) {
+ throw new IllegalArgumentException("Invalid token");
+ }
return Jwts.parserBuilder().setSigningKey(getSigningKey()).build()
.parseClaimsJws(token).getBody().getSubject();
}📝 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.
| public String getEmail(String token) { | |
| return Jwts.parserBuilder().setSigningKey(getSigningKey()).build() | |
| .parseClaimsJws(token).getBody().getSubject(); | |
| } | |
| public String getEmail(String token) { | |
| if (!validateToken(token)) { | |
| throw new IllegalArgumentException("Invalid token"); | |
| } | |
| return Jwts.parserBuilder().setSigningKey(getSigningKey()).build() | |
| .parseClaimsJws(token).getBody().getSubject(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/wayble/server/common/config/security/jwt/JwtTokenProvider.java
around lines 40 to 43, the getEmail() method parses the JWT token without
verifying its validity first, which is a security risk. Modify the method to
first validate the token's signature and expiration by calling a token
validation method before extracting the email. Ensure that if the token is
invalid, the method handles it appropriately, such as throwing an exception or
returning null.
KiSeungMin
left a comment
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.
엄청 빠르게 구현해주시다니...! 수고하셨습니다!!
|
|
||
| // 회원가입 | ||
| public void signup(UserRegisterRequestDto req) { | ||
| if (userRepository.existsByEmail(req.email())) { |
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.
하나의 이메일로 카카오, 구글, 애플 회원가입이 모두 가능해서, aaa@gmail.com이라는 이메일로 카톡, 구글, 애플로 모두 회원가입을 시도할 가능성이 있습니다!
그래서 이메일이 이미 존재하면 회원가입을 막는 로직이 나중에 문제가 발생할 수도 있어요...!
loginType 필드를 넣어놓은 이유도 같은 이유입니다! 같은 이메일이어도 로그인 플랫폼이 다르면 다른 계정으로 인식할 수 있어서..!
아마 나중에 수정이 필요할 것 같습니다..!
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.
이 부분에 대해서는 제가 제대로 로직을 파악하지 못한것 같네요,,, 알려주셔서 감사합니다! 다시 한번 수정할게요
| @@ -0,0 +1,3 @@ | |||
| package com.wayble.server.user.dto.token; | |||
|
|
|||
| public record TokenResponseDto(String accessToken) {} No newline at end of file | |||
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.
보통 유저가 회원가입 할 때 accessToken과 refreshToken을 모두 발급하는 것으로 알고 있습니다!
refreshToken은 아예 관련 로직이 없고 accessToken만 존재하는데 이유가 궁금합니다!
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.
refreshToken 저장 방법을 redis를 써서 저장해보고 싶었는데 해당 로직을 고민하고 있었습니다! 아직 토큰의 저장 방법 및 만료시 재발급, 갱신에 관한 흐름을 생각하고 있어서 우선 로그인,회원가입 로직을 구현하고 나서 추가적으로 구현할 생각이였습니다.
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.
넵 저도 refreshToken 저장을 Redis에 저장하는게 제일 좋은 방법으로 알고 있습니다!
다만 저희가 프리티어인데 Elastic Search까지 써야 해서 redis 쓰면 터질 확률이 높아보여요 흑흑 (저도 그래서 추천 기능에 Redis 쓰고 싶은데 못 쓰고 있어요 ㅠㅠ)
rdb로도 repository 파서 refreshToken을 저장하긴 해서 그렇게 짜야할 수도 있어요 ㅠㅠ
| if (header != null && header.startsWith("Bearer ")) { | ||
| String token = header.substring(7); | ||
|
|
||
| if (jwtProvider.validateToken(token)) { |
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.
토큰 인증에 실패하면 사용자의 refreshToken을 바탕으로 accessToken을 재발급하는 것으로 알고 있습니다! 그런데 현재 코드에는 refreshToken 관련 로직이나 refreshToken Repository가 없어서 accessToken 만료 시에 문제가 생길 것 같아요...!
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.
이것도 refreshToken 발급 관련 구현할때 로직 추가하겠습니다~!
#️⃣ 연관된 이슈
#41
📝 작업 내용
📸 스크린샷 (선택)
회원가입 성공
로그인 성공
💬 리뷰 요구사항 (선택)
Summary by CodeRabbit
신규 기능
버그 수정
문서화