Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.mycom.socket.auth.config;

import com.mycom.socket.auth.jwt.JWTFilter;
import com.mycom.socket.auth.jwt.JWTProperties;
import com.mycom.socket.auth.jwt.JWTUtil;
import com.mycom.socket.auth.service.MemberDetailsService;
import lombok.RequiredArgsConstructor;
Expand All @@ -20,6 +21,7 @@
public class SecurityConfig{

private final JWTUtil jwtUtil;
private final JWTProperties properties;
private final MemberDetailsService memberDetailsService;

@Bean
Expand All @@ -30,7 +32,10 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
.httpBasic(AbstractHttpConfigurer::disable)
.formLogin(AbstractHttpConfigurer::disable)

.addFilterBefore(new JWTFilter(jwtUtil, memberDetailsService), UsernamePasswordAuthenticationFilter.class)
.addFilterBefore(
new JWTFilter(properties, jwtUtil, memberDetailsService),
UsernamePasswordAuthenticationFilter.class
)

.sessionManagement(session ->
session.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
Expand Down
51 changes: 18 additions & 33 deletions src/main/java/com/mycom/socket/auth/controller/AuthController.java
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
package com.mycom.socket.auth.controller;

import com.mycom.socket.auth.dto.request.EmailRequestDto;
import com.mycom.socket.auth.dto.request.EmailVerificationRequestDto;
import com.mycom.socket.auth.dto.request.LoginRequestDto;
import com.mycom.socket.auth.dto.request.RegisterRequestDto;
import com.mycom.socket.auth.dto.response.EmailVerificationCheckResponseDto;
import com.mycom.socket.auth.dto.response.EmailVerificationResponseDto;
import com.mycom.socket.auth.dto.response.LoginResponseDto;
import com.mycom.socket.auth.dto.request.EmailRequest;
import com.mycom.socket.auth.dto.request.EmailVerificationRequest;
import com.mycom.socket.auth.dto.request.LoginRequest;
import com.mycom.socket.auth.dto.request.RegisterRequest;
import com.mycom.socket.auth.dto.response.EmailVerificationResponse;
import com.mycom.socket.auth.dto.response.LoginResponse;
import com.mycom.socket.auth.dto.response.RegisterResponse;
import com.mycom.socket.auth.service.AuthService;
import com.mycom.socket.auth.service.MailService;
import com.mycom.socket.auth.service.RateLimiter;
import com.mycom.socket.global.exception.BaseException;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
Expand All @@ -23,44 +21,31 @@ public class AuthController {

private final AuthService authService;
private final MailService mailService;
private final RateLimiter rateLimiter;

@PostMapping("/login")
public LoginResponseDto login(@Valid @RequestBody LoginRequestDto request,
HttpServletResponse response) {
public LoginResponse login(@Valid @RequestBody LoginRequest request,
HttpServletResponse response) {
return authService.login(request, response);
}

@PostMapping("/register")
public RegisterResponse register(@Valid @RequestBody RegisterRequest request) {
return authService.register(request);
}

@PostMapping("/logout")
public void logout(HttpServletResponse response) {
authService.logout(response);
}

@PostMapping("/register")
public Long register(@Valid @RequestBody RegisterRequestDto request) {
return authService.register(request);
}

@PostMapping("/verification")
public EmailVerificationResponseDto mailSend(@Valid @RequestBody EmailRequestDto emailRequestDto) {
try {
boolean isSuccess = mailService.sendMail(emailRequestDto.email());
return isSuccess ? EmailVerificationResponseDto.createSuccessResponse() : EmailVerificationResponseDto.createFailureResponse("이메일 전솑에 μ‹€νŒ¨ν–ˆμŠ΅λ‹ˆλ‹€.");
} catch (BaseException e) {
return EmailVerificationResponseDto.createFailureResponse(e.getMessage());
}
public EmailVerificationResponse sendVerificationEmail(@Valid @RequestBody EmailRequest request) {
return mailService.sendMail(request.email());
Comment on lines +42 to +43
Copy link

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

μ˜ˆμ™Έ 처리λ₯Ό μΆ”κ°€ν•˜μ—¬ μ•ˆμ •μ„±μ„ ν–₯μƒμ‹œν‚€μ‹­μ‹œμ˜€.

sendVerificationEmail λ©”μ„œλ“œμ—μ„œ 이메일 전솑 쀑 λ°œμƒν•  수 μžˆλŠ” μ˜ˆμ™Έμ— λŒ€ν•œ μ²˜λ¦¬κ°€ μ—†μŠ΅λ‹ˆλ‹€. 이메일 전솑 μ‹€νŒ¨ μ‹œ μ‚¬μš©μžμ—κ²Œ μ μ ˆν•œ ν”Όλ“œλ°±μ„ μ œκ³΅ν•  수 μžˆλ„λ‘ μ˜ˆμ™Έ 처리λ₯Ό μΆ”κ°€ν•˜λŠ” 것이 μ’‹μŠ΅λ‹ˆλ‹€.

}

@PostMapping("/email/verify")
public EmailVerificationCheckResponseDto mailCheck(@Valid @RequestBody EmailVerificationRequestDto emailRequestDto) {
try{
rateLimiter.checkRateLimit(emailRequestDto.email());// μ‹œλ„ 횟수 μ œν•œ
boolean isVerified = mailService.verifyCode(emailRequestDto.email(), emailRequestDto.code());
return isVerified ? EmailVerificationCheckResponseDto.createSuccessResponse() :
EmailVerificationCheckResponseDto.createFailureResponse("이메일 인증에 μ‹€νŒ¨ν–ˆμŠ΅λ‹ˆλ‹€.");
}catch (BaseException e){
return EmailVerificationCheckResponseDto.createFailureResponse(e.getMessage());
}
public EmailVerificationResponse verifyEmail(@Valid @RequestBody EmailVerificationRequest request) {
return mailService.verifyCode(request.email(), request.code());
Comment on lines +47 to +48
Copy link

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

μ˜ˆμ™Έ 처리λ₯Ό μΆ”κ°€ν•˜μ—¬ μ•ˆμ •μ„±μ„ ν–₯μƒμ‹œν‚€μ‹­μ‹œμ˜€.

verifyEmail λ©”μ„œλ“œμ—μ„œ 이메일 검증 쀑 λ°œμƒν•  수 μžˆλŠ” μ˜ˆμ™Έμ— λŒ€ν•œ μ²˜λ¦¬κ°€ μ—†μŠ΅λ‹ˆλ‹€. 인증 μ½”λ“œ 검증 μ‹€νŒ¨ μ‹œ μ‚¬μš©μžμ—κ²Œ μ μ ˆν•œ ν”Όλ“œλ°±μ„ μ œκ³΅ν•  수 μžˆλ„λ‘ μ˜ˆμ™Έ 처리λ₯Ό μΆ”κ°€ν•˜λŠ” 것이 μ’‹μŠ΅λ‹ˆλ‹€.

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import jakarta.validation.constraints.Email;
import jakarta.validation.constraints.NotEmpty;

public record EmailRequestDto(
public record EmailRequest(
@NotEmpty(message = "이메일 μ£Όμ†Œλ₯Ό μž…λ ₯ν•΄μ£Όμ„Έμš”.")
@Email(message = "μœ νš¨ν•˜μ§€ μ•Šμ€ 이메일 ν˜•μ‹μž…λ‹ˆλ‹€.")
String email
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import jakarta.validation.constraints.NotEmpty;
import jakarta.validation.constraints.Pattern;

public record EmailVerificationRequestDto(
public record EmailVerificationRequest(
@NotEmpty(message = "이메일 μ£Όμ†Œλ₯Ό μž…λ ₯ν•΄μ£Όμ„Έμš”.")
@Email(message = "μœ νš¨ν•˜μ§€ μ•Šμ€ 이메일 ν˜•μ‹μž…λ‹ˆλ‹€.")
String email,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import jakarta.validation.constraints.Email;
import jakarta.validation.constraints.NotBlank;

public record LoginRequestDto(
public record LoginRequest(
@NotBlank(message = "이메일은 ν•„μˆ˜μž…λ‹ˆλ‹€")
@Email(message = "μ˜¬λ°”λ₯Έ 이메일 ν˜•μ‹μ΄ μ•„λ‹™λ‹ˆλ‹€")
String email,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.Size;

public record RegisterRequestDto(
public record RegisterRequest(
@NotBlank(message = "이메일은 ν•„μˆ˜μž…λ‹ˆλ‹€")
@Email(message = "μ˜¬λ°”λ₯Έ 이메일 ν˜•μ‹μ΄ μ•„λ‹™λ‹ˆλ‹€")
String email,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.mycom.socket.auth.dto.response;

public record EmailVerificationResponse(
String message
) {
public static EmailVerificationResponse of(String message) {
return new EmailVerificationResponse(message);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.mycom.socket.auth.dto.response;

public record LoginResponse(
String email,
String nickname
) {
public static LoginResponse of(String email, String nickname) {
return new LoginResponse(email, nickname);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.mycom.socket.auth.dto.response;

public record RegisterResponse(
Long memberId,
String email,
String nickname,
String message
) {
public static RegisterResponse of(Long memberId, String email, String nickname) {
return new RegisterResponse(memberId, email, nickname, "νšŒμ›κ°€μž…μ΄ μ™„λ£Œλ˜μ—ˆμŠ΅λ‹ˆλ‹€.");
}
}
41 changes: 24 additions & 17 deletions src/main/java/com/mycom/socket/auth/jwt/JWTFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.core.userdetails.UserDetails;
Expand All @@ -15,33 +16,25 @@

import java.io.IOException;

@Slf4j
@RequiredArgsConstructor
public class JWTFilter extends OncePerRequestFilter {

private final JWTProperties jwtProperties;
private final JWTUtil jwtUtil;
private final MemberDetailsService memberDetailsService;

@Override
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
throws ServletException, IOException {

String token = resolveTokenFromCookie(request);

protected void doFilterInternal(HttpServletRequest request,
HttpServletResponse response,
FilterChain filterChain) throws ServletException, IOException {
try {
String token = resolveTokenFromCookie(request);
if (StringUtils.hasText(token) && jwtUtil.validateToken(token)) {
String email = jwtUtil.getEmail(token);
UserDetails userDetails = memberDetailsService.loadUserByUsername(email);

UsernamePasswordAuthenticationToken authentication =
new UsernamePasswordAuthenticationToken(
userDetails,
null,
userDetails.getAuthorities()
);

SecurityContextHolder.getContext().setAuthentication(authentication);
setAuthentication(token);
}
} catch (Exception e) {
log.warn("인증 처리 μ‹€νŒ¨", e);
SecurityContextHolder.clearContext();
}

Expand All @@ -52,11 +45,25 @@ private String resolveTokenFromCookie(HttpServletRequest request) {
Cookie[] cookies = request.getCookies();
if (cookies != null) {
for (Cookie cookie : cookies) {
if ("Authorization".equals(cookie.getName())) {
if (jwtProperties.getCookieName().equals(cookie.getName())) {
return cookie.getValue();
}
}
}
return null;
}

private void setAuthentication(String token) {
String email = jwtUtil.getEmail(token);
UserDetails userDetails = memberDetailsService.loadUserByUsername(email);

UsernamePasswordAuthenticationToken authentication =
new UsernamePasswordAuthenticationToken(
userDetails,
null,
userDetails.getAuthorities()
);

SecurityContextHolder.getContext().setAuthentication(authentication);
}
}
17 changes: 17 additions & 0 deletions src/main/java/com/mycom/socket/auth/jwt/JWTProperties.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package com.mycom.socket.auth.jwt;
import lombok.Getter;
import lombok.Setter;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.stereotype.Component;

@Getter
@Setter
@Component
@ConfigurationProperties(prefix = "jwt")
public class JWTProperties {
private String secret;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

JWT secret ν•„λ“œμ— λŒ€ν•œ μœ νš¨μ„± 검사 ν•„μš”

JWT secret은 ν•„μˆ˜ κ°’μ΄λ―€λ‘œ @NotEmpty μ–΄λ…Έν…Œμ΄μ…˜μ„ μΆ”κ°€ν•˜μ—¬ μœ νš¨μ„± 검사λ₯Ό μˆ˜ν–‰ν•΄μ•Ό ν•©λ‹ˆλ‹€.

-    private String secret;
+    @NotEmpty(message = "JWT secret은 ν•„μˆ˜ κ°’μž…λ‹ˆλ‹€.")
+    private String secret;
πŸ“ 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
private String secret;
@NotEmpty(message = "JWT secret은 ν•„μˆ˜ κ°’μž…λ‹ˆλ‹€.")
private String secret;

private long accessTokenValidityInSeconds = 1800;
Copy link

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

토큰 유효 기간에 λŒ€ν•œ μ΅œμ†Œκ°’ 검증 ν•„μš”

토큰 유효 기간이 λ„ˆλ¬΄ μ§§μ§€ μ•Šλ„λ‘ μ΅œμ†Œκ°’ 검증이 ν•„μš”ν•©λ‹ˆλ‹€.

-    private long accessTokenValidityInSeconds = 1800;
+    @Min(value = 300, message = "토큰 유효 기간은 μ΅œμ†Œ 5λΆ„ 이상이어야 ν•©λ‹ˆλ‹€.")
+    private long accessTokenValidityInSeconds = 1800;
πŸ“ 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
private long accessTokenValidityInSeconds = 1800;
@Min(value = 300, message = "토큰 유효 기간은 μ΅œμ†Œ 5λΆ„ 이상이어야 ν•©λ‹ˆλ‹€.")
private long accessTokenValidityInSeconds = 1800;

private String cookieName = "Authorization";
private String issuer = "go_socket";
private boolean secureCookie = false;
Copy link

Choose a reason for hiding this comment

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

πŸ’‘ Codebase verification

⚠️ Potential issue

λ³΄μ•ˆ μΏ ν‚€ μ„€μ • 문제 확인됨

ν˜„μž¬ κ΅¬μ„±μ—μ„œ λ‹€μŒκ³Ό 같은 μ‹¬κ°ν•œ λ³΄μ•ˆ λ¬Έμ œκ°€ ν™•μΈλ˜μ—ˆμŠ΅λ‹ˆλ‹€:

  • JWTProperties의 secureCookie 기본값이 false둜 섀정됨
  • ν”„λ‘œλ•μ…˜ ν™˜κ²½μ„ ν¬ν•¨ν•œ μ–΄λ–€ ν™˜κ²½ μ„€μ • νŒŒμΌμ—μ„œλ„ 이 값을 μž¬μ •μ˜ν•˜μ§€ μ•ŠμŒ
  • SecurityConfigμ—μ„œλ„ μΏ ν‚€ λ³΄μ•ˆ 섀정을 λ³„λ„λ‘œ κ΅¬μ„±ν•˜μ§€ μ•ŠμŒ

ν”„λ‘œλ•μ…˜ ν™˜κ²½μ˜ λ³΄μ•ˆμ„ μœ„ν•΄ λ°˜λ“œμ‹œ secureCookieλ₯Ό true둜 μ„€μ •ν•΄μ•Ό ν•©λ‹ˆλ‹€.

πŸ”— Analysis chain

λ³΄μ•ˆ μΏ ν‚€ μ„€μ • κ²€ν†  ν•„μš”

ν”„λ‘œλ•μ…˜ ν™˜κ²½μ—μ„œλŠ” λ³΄μ•ˆ μΏ ν‚€λ₯Ό ν™œμ„±ν™”ν•΄μ•Ό ν•©λ‹ˆλ‹€. ν˜„μž¬ 기본값이 false둜 μ„€μ •λ˜μ–΄ μžˆμ–΄ λ³΄μ•ˆμƒ μœ„ν—˜ν•  수 μžˆμŠ΅λ‹ˆλ‹€.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if secureCookie is properly configured in different environments

# Check application properties files for secure cookie configuration
echo "Checking application properties files for secure cookie configuration..."
rg -l "jwt\.secure-cookie" 

Length of output: 183


Script:

#!/bin/bash
# Search for various possible property naming patterns
echo "Checking different property naming patterns..."
rg -l "jwt\.(secure-?cookie|cookie\.secure|secureCookie)"

echo -e "\nChecking for application properties/yml files..."
fd -e properties -e yml

echo -e "\nChecking for security configuration classes..."
fd -e java -x grep -l "SecurityConfig\|WebSecurityConfig"

Length of output: 730


Script:

#!/bin/bash
echo "Checking application.yml content..."
cat src/main/resources/application.yml

echo -e "\nChecking application-dev.yml content..."
cat src/main/resources/yaml/application-dev.yml

echo -e "\nChecking SecurityConfig.java for cookie configurations..."
cat src/main/java/com/mycom/socket/auth/config/SecurityConfig.java

Length of output: 2900

}
37 changes: 27 additions & 10 deletions src/main/java/com/mycom/socket/auth/jwt/JWTUtil.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package com.mycom.socket.auth.jwt;

import io.jsonwebtoken.Claims;
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.security.Keys;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;
import org.springframework.util.StringUtils;

import javax.crypto.SecretKey;
import java.nio.charset.StandardCharsets;
Expand All @@ -16,42 +15,60 @@
public class JWTUtil {

private final SecretKey secretKey;
private final JWTProperties jwtProperties;

public JWTUtil(@Value("${jwt.secret}") String secret) {
this.secretKey = Keys.hmacShaKeyFor(secret.getBytes(StandardCharsets.UTF_8));
public JWTUtil(JWTProperties jwtProperties) {
this.jwtProperties = jwtProperties;
this.secretKey = Keys.hmacShaKeyFor(
jwtProperties.getSecret().getBytes(StandardCharsets.UTF_8)
);
}

/**
* JWT 토큰 생성
*/
public String createToken(String email) {
Claims claims = Jwts.claims().subject(email).build();
Date now = new Date();
// 30λΆ„
long accessTokenValidityInMilliseconds = 1000 * 60 * 30;
Date validity = new Date(now.getTime() + accessTokenValidityInMilliseconds);
Date validity = new Date(now.getTime() +
(jwtProperties.getAccessTokenValidityInSeconds() * 1000));

return Jwts.builder()
.claims(claims)
.issuer(jwtProperties.getIssuer())
.subject(email)
.issuedAt(now)
.expiration(validity)
.signWith(secretKey)
.compact();
}

/**
* 토큰 μœ νš¨μ„± 검증
*/
public boolean validateToken(String token) {
try {
if (!StringUtils.hasText(token)) {
return false;
}

Jwts.parser()
.verifyWith(secretKey)
.requireIssuer(jwtProperties.getIssuer())
.build()
.parseSignedClaims(token);
return true;
} catch (Exception e) {
log.warn("JWT 토큰 검증 쀑 μ—λŸ¬ λ°œμƒ: {}", e.getMessage());
log.warn("JWT 토큰 검증 μ‹€νŒ¨", e);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

μ˜ˆμ™Έ λ©”μ‹œμ§€μ— λ―Όκ°ν•œ 정보가 ν¬ν•¨λ˜μ§€ μ•Šλ„λ‘ 둜그λ₯Ό κ°œμ„ ν•˜μ‹­μ‹œμ˜€.

validateToken λ©”μ„œλ“œμ—μ„œ μ˜ˆμ™Έ λ°œμƒ μ‹œ 전체 μ˜ˆμ™Έλ₯Ό λ‘œκ·Έμ— κΈ°λ‘ν•˜κ³  μžˆμŠ΅λ‹ˆλ‹€. μ˜ˆμ™Έ λ©”μ‹œμ§€μ— λ―Όκ°ν•œ 정보가 포함될 수 μžˆμœΌλ―€λ‘œ, λ‘œκ·Έμ— μ˜ˆμ™Έ λ©”μ‹œμ§€λ₯Ό ν¬ν•¨ν•˜μ§€ μ•Šκ±°λ‚˜ μ΅œμ†Œν•œμ˜ μ •λ³΄λ§Œ κΈ°λ‘ν•˜λŠ” 것이 μ’‹μŠ΅λ‹ˆλ‹€.

return false;
}
}

/**
* ν† ν°μ—μ„œ 이메일 μΆ”μΆœ
*/
public String getEmail(String token) {
return Jwts.parser()
.verifyWith(secretKey)
.requireIssuer(jwtProperties.getIssuer())
.build()
.parseSignedClaims(token)
.getPayload()
Expand Down
Loading
Loading