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
Expand Up @@ -10,6 +10,7 @@ public enum SuccessMessage {
LOGIN_SUCCESS("로그인 성공"),
ACCESS_TOKEN_REISSUE_SUCCESS("액세스 토큰 재발급 성공"),
CHANGE_PASSWORD_SUCCESS("비밀번호 변경 성공"),
LOGOUT_SUCCESS("로그아웃 성공"),
;

private final String message;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
@RequiredArgsConstructor
public class SecurityConfig {

private final RedisService redisService;
private final JwtTokenProvider jwtTokenProvider;
private final UserRepository userRepository;
private final JwtAuthEntryPoint jwtAuthEntryPoint;
Expand All @@ -39,6 +40,7 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
.headers(h -> h.frameOptions(FrameOptionsConfig::sameOrigin)) // H2 콘솔 접근 시 iframe 사용 허용 (동일 출처만 허용하여 보안 유지)
.sessionManagement(sm -> sm.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
.authorizeHttpRequests(auth -> auth
.requestMatchers("/api/v1/auth/logout").authenticated()
.anyRequest().permitAll() // 추후 변경 예정
)
.exceptionHandling(e -> e
Expand Down Expand Up @@ -68,6 +70,6 @@ public CorsConfigurationSource corsConfigurationSource() {

@Bean
public TokenAuthenticationFilter tokenAuthenticationFilter() {
return new TokenAuthenticationFilter(jwtTokenProvider, userRepository);
return new TokenAuthenticationFilter(redisService, jwtTokenProvider, userRepository);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.almang.inventory.global.exception.BaseException;
import com.almang.inventory.global.exception.ErrorCode;
import com.almang.inventory.global.security.principal.CustomUserPrincipal;
import com.almang.inventory.user.auth.service.RedisService;
import com.almang.inventory.user.domain.User;
import com.almang.inventory.user.repository.UserRepository;
import jakarta.servlet.FilterChain;
Expand All @@ -25,6 +26,7 @@
@RequiredArgsConstructor
public class TokenAuthenticationFilter extends OncePerRequestFilter {

private final RedisService redisService;
private final JwtTokenProvider jwtTokenProvider;
private final UserRepository userRepository;
private static final String TOKEN_PREFIX = "Bearer ";
Expand Down Expand Up @@ -68,18 +70,24 @@ protected void doFilterInternal(
log.debug("[AUTH] path={}, authHeader={}", request.getServletPath(), request.getHeader(HttpHeaders.AUTHORIZATION));
log.debug("[AUTH] tokenStatus={}", status);
if (status == TokenStatus.VALID) {
Long userId = jwtTokenProvider.getUserIdFromToken(token);
User user = userRepository.findById(userId)
.orElseThrow(() -> new BaseException(ErrorCode.USER_NOT_FOUND));
if (redisService.isAccessTokenBlacklisted(token)) {
log.warn("블랙리스트에 등록된 액세스 토큰입니다.");
SecurityContextHolder.clearContext();
request.setAttribute("authErrorCode", ErrorCode.ACCESS_TOKEN_INVALID);
} else {
Long userId = jwtTokenProvider.getUserIdFromToken(token);
User user = userRepository.findById(userId)
.orElseThrow(() -> new BaseException(ErrorCode.USER_NOT_FOUND));

GrantedAuthority authority = new SimpleGrantedAuthority("ROLE_" + user.getRole().name());
List<GrantedAuthority> authorities = List.of(authority);
CustomUserPrincipal principal =
new CustomUserPrincipal(user.getId(), user.getUsername(), authorities);
GrantedAuthority authority = new SimpleGrantedAuthority("ROLE_" + user.getRole().name());
List<GrantedAuthority> authorities = List.of(authority);
CustomUserPrincipal principal =
new CustomUserPrincipal(user.getId(), user.getUsername(), authorities);

UsernamePasswordAuthenticationToken authenticationToken =
new UsernamePasswordAuthenticationToken(principal, null, authorities);
SecurityContextHolder.getContext().setAuthentication(authenticationToken);
UsernamePasswordAuthenticationToken authenticationToken =
new UsernamePasswordAuthenticationToken(principal, null, authorities);
SecurityContextHolder.getContext().setAuthentication(authenticationToken);
}
} else {
SecurityContextHolder.clearContext();
ErrorCode errorCode = (status == TokenStatus.EXPIRED)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.almang.inventory.user.auth.dto.response.AccessTokenResponse;
import com.almang.inventory.user.auth.dto.response.ChangePasswordResponse;
import com.almang.inventory.user.auth.dto.response.LoginResponse;
import com.almang.inventory.user.auth.dto.response.LogoutResponse;
import com.almang.inventory.user.auth.service.AuthService;
import com.almang.inventory.user.auth.service.TokenService;
import io.swagger.v3.oas.annotations.Operation;
Expand All @@ -20,6 +21,7 @@
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.ResponseEntity;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
Expand Down Expand Up @@ -76,4 +78,19 @@ public ResponseEntity<ApiResponse<ChangePasswordResponse>> changePassword(
ApiResponse.success(SuccessMessage.CHANGE_PASSWORD_SUCCESS.getMessage(), response)
);
}

@DeleteMapping("/logout")
@Operation(summary = "로그아웃", description = "로그아웃을 진행합니다.")
public ResponseEntity<ApiResponse<LogoutResponse>> logout(
@AuthenticationPrincipal CustomUserPrincipal userPrincipal,
HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse
) {
Long userId = userPrincipal.getId();
log.info("[AuthController] 로그아웃 요청 - userId={}", userId);
LogoutResponse response = authService.logout(userId, httpServletRequest, httpServletResponse);

return ResponseEntity.ok(
ApiResponse.success(SuccessMessage.LOGOUT_SUCCESS.getMessage(), response)
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.almang.inventory.user.auth.dto.response;

public record LogoutResponse(
boolean success
) {}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
import com.almang.inventory.user.auth.dto.request.LoginRequest;
import com.almang.inventory.user.auth.dto.response.ChangePasswordResponse;
import com.almang.inventory.user.auth.dto.response.LoginResponse;
import com.almang.inventory.user.auth.dto.response.LogoutResponse;
import com.almang.inventory.user.domain.User;
import com.almang.inventory.user.repository.UserRepository;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -48,6 +50,15 @@ public ChangePasswordResponse changePassword(ChangePasswordRequest request, Long
return new ChangePasswordResponse(true);
}

@Transactional
public LogoutResponse logout(Long userId, HttpServletRequest request, HttpServletResponse response) {
log.info("[AuthService] 로그아웃 요청 - userId: {}", userId);
tokenService.revokeTokens(request, response, userId);

log.info("[AuthService] 로그아웃 성공 - userId: {}", userId);
return new LogoutResponse(true);
}

private User findUserByUsername(String username) {
return userRepository.findByUsername(username)
.orElseThrow(() -> new BaseException(ErrorCode.USER_NOT_FOUND));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public class RedisService {

private static final String REFRESH_USER_PREFIX = "refresh:user:";
private static final String REFRESH_TOKEN_PREFIX = "refresh:token:";
private static final String ACCESS_BLACKLIST_PREFIX = "blacklist:access:";

private final RedisTemplate<String, String> redisTemplate;

Expand All @@ -26,6 +27,10 @@ private String tokenKey(String token) {
return REFRESH_TOKEN_PREFIX + token;
}

private String blacklistAccessTokenKey(String token) {
return ACCESS_BLACKLIST_PREFIX + token;
}

public void saveRefreshToken(String userId, String refreshToken) {
Duration ttl = Duration.ofDays(refreshTokenExpiration);

Expand Down Expand Up @@ -69,4 +74,14 @@ public void rotateRefreshToken(String userId, String oldToken, String newToken)
redisTemplate.delete(userKey(userId));
saveRefreshToken(userId, newToken);
}

public void addAccessTokenToBlacklist(String accessToken, long remainingMillis) {
if (remainingMillis <= 0) return;
redisTemplate.opsForValue().set(
blacklistAccessTokenKey(accessToken), "true", Duration.ofMillis(remainingMillis));
}

public boolean isAccessTokenBlacklisted(String accessToken) {
return redisTemplate.hasKey(blacklistAccessTokenKey(accessToken));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,36 @@ private String extractRefreshToken(HttpServletRequest request) {
.orElseThrow(() -> new BaseException(ErrorCode.REFRESH_TOKEN_NOT_FOUND));
}

public void revokeTokens(HttpServletRequest request, HttpServletResponse response, Long userId) {
String accessToken = resolveToken(request);

// 액세스 토큰 무효화
if (accessToken != null && !accessToken.isBlank()) {
long remainMillis = jwtTokenProvider.getRemainingMillis(accessToken);
if (remainMillis > 0) {
redisService.addAccessTokenToBlacklist(accessToken, remainMillis);
}
}

// 리프레시 토큰 삭제
redisService.deleteByUserId(userId.toString());

// 리프레시 쿠키 삭제
clearRefreshTokenCookie(response);
}
Comment on lines +85 to +101
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

토큰 폐기 로직이 전반적으로 잘 구현되었습니다.

다만, 액세스 토큰을 추출할 수 없는 경우(Line 86에서 accessToken이 null인 경우)에도 리프레시 토큰은 삭제됩니다. 이는 다음 시나리오를 고려해야 합니다:

잠재적 문제:

  • 사용자가 유효한 액세스 토큰을 보유하고 있지만 요청 헤더에 포함하지 않고 로그아웃을 시도하는 경우
  • 리프레시 토큰은 삭제되지만 액세스 토큰은 블랙리스트에 추가되지 않아 만료 시까지 사용 가능

개선 방안:

  1. 로그아웃 엔드포인트를 인증 필수로 만들어 항상 액세스 토큰이 존재하도록 보장
  2. 액세스 토큰이 없을 경우 예외를 발생시켜 명시적으로 실패 처리

현재 AuthController의 로그아웃 엔드포인트가 인증이 필요한지 확인이 필요합니다.

다음 스크립트로 로그아웃 엔드포인트의 인증 요구 사항을 확인하세요:


🏁 Script executed:

#!/bin/bash
# Description: Verify if logout endpoint requires authentication

# Check AuthController logout endpoint and SecurityConfig
echo "=== AuthController logout endpoint ==="
rg -nP -A10 'DELETE.*logout|@DeleteMapping.*logout' --type=java

echo -e "\n=== SecurityConfig authentication requirements ==="
rg -nP -A5 -B5 '/logout|permitAll|authenticated' --type=java -g '*SecurityConfig*'

Length of output: 4190


보안 설정 모순으로 인한 토큰 폐기 로직 실패 위험

검증 결과, AuthController의 logout 엔드포인트는 @AuthenticationPrincipal로 인증을 요구하려 하지만, SecurityConfig.anyRequest().permitAll() (43줄)이 이를 무효화합니다. 결과적으로 인증되지 않은 사용자가 logout을 호출하면 userPrincipal이 null이 되어 88줄에서 NullPointerException 발생하고, 더 심각하게는 해당 사용자의 토큰이 전혀 폐기되지 않습니다.

즉시 조치 필요:

  1. SecurityConfig.java.anyRequest().permitAll().requestMatchers("/logout").authenticated()로 변경하여 logout 엔드포인트를 인증 필수로 강제 (또는 Spring Security의 인증 필터가 401을 반환하도록 설정)
  2. 또는 명시적 에러 핸들링: userPrincipal이 null일 경우 BadCredentialsException 발생시켜 실패 처리

참고: 현재 SecurityConfig의 주석 "추후 변경 예정"과 함께 이 보안 설정은 개발/테스트 환경 전용입니다. 프로덕션 배포 전에 필수로 수정하세요.

🤖 Prompt for AI Agents
In src/main/java/com/almang/inventory/user/auth/service/TokenService.java around
lines 85–101, the revokeTokens flow assumes an authenticated principal and can
NPE if logout is invoked without authentication; fix by enforcing authentication
on the logout endpoint in SecurityConfig (replace .anyRequest().permitAll() with
a rule that requires .requestMatchers("/logout").authenticated() or otherwise
ensure the security filter returns 401 for unauthenticated requests), and/or add
an explicit null check in revokeTokens to throw a BadCredentialsException (or
similar authentication exception) when the user principal is null so the method
fails safely without proceeding to token revocation; implement one or both fixes
before production.


private void clearRefreshTokenCookie(HttpServletResponse response) {
ResponseCookie cookie = ResponseCookie.from(REFRESH_TOKEN_PREFIX, "")
.httpOnly(true)
.secure(true)
.sameSite("None")
.path("/")
.maxAge(0)
.build();

response.addHeader("Set-Cookie", cookie.toString());
}

private String resolveToken(HttpServletRequest request) {
String bearer = request.getHeader(HttpHeaders.AUTHORIZATION);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public SecurityFilterChain testSecurityFilterChain(HttpSecurity httpSecurity) th
httpSecurity
.csrf(AbstractHttpConfigurer::disable)
.authorizeHttpRequests(auth -> auth
.requestMatchers("/api/v1/auth/logout").authenticated()
.anyRequest().permitAll());
return httpSecurity.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.when;
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;

Expand All @@ -16,6 +17,7 @@
import com.almang.inventory.user.auth.dto.request.LoginRequest;
import com.almang.inventory.user.auth.dto.response.ChangePasswordResponse;
import com.almang.inventory.user.auth.dto.response.LoginResponse;
import com.almang.inventory.user.auth.dto.response.LogoutResponse;
import com.almang.inventory.user.auth.service.AuthService;
import com.almang.inventory.user.auth.service.TokenService;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand Down Expand Up @@ -214,4 +216,46 @@ public class AuthControllerTest {
.andExpect(jsonPath("$.message").value(ErrorCode.INVALID_INPUT_VALUE.getMessage()))
.andExpect(jsonPath("$.data").doesNotExist());
}

@Test
void 로그아웃에_성공한다() throws Exception {
// given
CustomUserPrincipal principal =
new CustomUserPrincipal(1L, "store_admin", List.of());
UsernamePasswordAuthenticationToken auth =
new UsernamePasswordAuthenticationToken(principal, null, principal.getAuthorities());

LogoutResponse response = new LogoutResponse(true);

when(authService.logout(anyLong(), any(HttpServletRequest.class), any(HttpServletResponse.class)))
.thenReturn(response);

// when & then
mockMvc.perform(delete("/api/v1/auth/logout")
.with(authentication(auth)))
.andExpect(status().isOk())
.andExpect(jsonPath("$.status").value(200))
.andExpect(jsonPath("$.message").value(SuccessMessage.LOGOUT_SUCCESS.getMessage()))
.andExpect(jsonPath("$.data.success").value(true));
}

@Test
void 로그아웃_중_예외가_발생하면_에러_응답을_반환한다() throws Exception {
// given
CustomUserPrincipal principal =
new CustomUserPrincipal(1L, "store_admin", List.of());
UsernamePasswordAuthenticationToken auth =
new UsernamePasswordAuthenticationToken(principal, null, principal.getAuthorities());

when(authService.logout(anyLong(), any(HttpServletRequest.class), any(HttpServletResponse.class)))
.thenThrow(new BaseException(ErrorCode.ACCESS_TOKEN_INVALID));

// when & then
mockMvc.perform(delete("/api/v1/auth/logout")
.with(authentication(auth)))
.andExpect(status().isUnauthorized())
.andExpect(jsonPath("$.status").value(ErrorCode.ACCESS_TOKEN_INVALID.getHttpStatus().value()))
.andExpect(jsonPath("$.message").value(ErrorCode.ACCESS_TOKEN_INVALID.getMessage()))
.andExpect(jsonPath("$.data").doesNotExist());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
import com.almang.inventory.user.auth.dto.request.LoginRequest;
import com.almang.inventory.user.auth.dto.response.ChangePasswordResponse;
import com.almang.inventory.user.auth.dto.response.LoginResponse;
import com.almang.inventory.user.auth.dto.response.LogoutResponse;
import com.almang.inventory.user.domain.User;
import com.almang.inventory.user.repository.UserRepository;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.util.Optional;
import org.junit.jupiter.api.Test;
Expand All @@ -28,6 +30,7 @@ public class AuthServiceTest {
@Mock private TokenService tokenService;
@Mock private UserRepository userRepository;
@Mock private PasswordEncoder passwordEncoder;
@Mock private HttpServletRequest httpServletRequest;
@Mock private HttpServletResponse httpServletResponse;

@InjectMocks private AuthService authService;
Expand Down Expand Up @@ -143,4 +146,17 @@ public class AuthServiceTest {
.isInstanceOf(BaseException.class)
.hasMessageContaining(ErrorCode.USER_NOT_FOUND.getMessage());
}

@Test
void 로그아웃에_성공하면_true를_반환하고_토큰을_폐기한다() {
// given
Long userId = 1L;

// when
LogoutResponse response = authService.logout(userId, httpServletRequest, httpServletResponse);

// then
assertThat(response.success()).isTrue();
verify(tokenService).revokeTokens(httpServletRequest, httpServletResponse, userId);
}
}
Loading