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 @@ -3,6 +3,7 @@
import com.permitseoul.permitserver.domain.admin.base.api.AdminProperties;
import com.permitseoul.permitserver.domain.admin.base.api.dto.res.UserAuthorityGetResponse;
import com.permitseoul.permitserver.domain.admin.base.api.exception.AdminAuthorizationException;
import com.permitseoul.permitserver.domain.auth.core.jwt.RefreshTokenManager;
import com.permitseoul.permitserver.domain.user.core.component.UserRetriever;
import com.permitseoul.permitserver.domain.user.core.component.UserUpdater;
import com.permitseoul.permitserver.domain.user.core.domain.User;
Expand All @@ -20,6 +21,7 @@ public class AdminService {
private final AdminProperties adminProperties;
private final UserRetriever userRetriever;
private final UserUpdater userUpdater;
private final RefreshTokenManager refreshTokenManager;

public void validateAdminCode(final String adminCode) {
if(!(adminProperties.accessCode().equals(adminCode))){
Expand All @@ -44,6 +46,7 @@ public void updateUserAuthority(final long userId, final UserRole userRole) {
try {
userEntity = userRetriever.findUserEntityById(userId);
userUpdater.updateUserRole(userEntity, userRole);
refreshTokenManager.deleteRefreshToken(userEntity.getUserId());
} catch (UserNotFoundException e) {
throw new AdminAuthorizationException(ErrorCode.NOT_FOUND_USER);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,17 @@ public TokenDto login(final SocialType socialType, final String authorizationCod
public TokenDto reissue(final String refreshToken) {
try {
final long userId = jwtProvider.extractUserIdFromToken(refreshToken);
checkIsSameRefreshToken(userId, refreshToken);
final UserRole userRole = UserRole.valueOf(jwtProvider.extractUserRoleFromToken(refreshToken));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the AuthService.java file around line 101
head -120 src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java | tail -30

Repository: PERMIT-SEOUL/permit-server

Length of output: 1403


🏁 Script executed:

# Find the method containing line 101 to understand the context and exception handling
rg -n -B20 -A10 'UserRole\.valueOf.*extractUserRoleFromToken' --type java src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java

Repository: PERMIT-SEOUL/permit-server

Length of output: 1753


🏁 Script executed:

# Check the implementation of extractUserRoleFromToken
rg -n -A5 'extractUserRoleFromToken' --type java

Repository: PERMIT-SEOUL/permit-server

Length of output: 2511


🏁 Script executed:

# Check how issueToken sets the role claim
rg -n -A10 'issueToken.*|setUserRole' --type java

Repository: PERMIT-SEOUL/permit-server

Length of output: 11749


🏁 Script executed:

# Check existing exception handling for UNAUTHORIZED_WRONG_RT
rg -n 'UNAUTHORIZED_WRONG_RT' --type java

Repository: PERMIT-SEOUL/permit-server

Length of output: 569


UserRole.valueOf(...) 호출 시 발생 가능한 예외가 인증 오류로 매핑되지 않습니다.

Line 101에서 extractUserRoleFromToken()이 반환하는 문자열이 null이거나 enum과 일치하지 않으면 IllegalArgumentException 또는 NullPointerException이 발생합니다. 현재 catch 블록에서 이를 명시적으로 처리하지 않아 예외가 그대로 전파됩니다. 다음과 같이 처리해주세요:

🔧 제안 수정안
-            final UserRole userRole = UserRole.valueOf(jwtProvider.extractUserRoleFromToken(refreshToken));
+            final UserRole userRole;
+            try {
+                userRole = UserRole.valueOf(jwtProvider.extractUserRoleFromToken(refreshToken));
+            } catch (IllegalArgumentException | NullPointerException e) {
+                throw new AuthUnAuthorizedException(ErrorCode.UNAUTHORIZED_WRONG_RT);
+            }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java`
at line 101, The use of
UserRole.valueOf(jwtProvider.extractUserRoleFromToken(refreshToken)) can throw
IllegalArgumentException or NullPointerException when the token yields null or
an unrecognized role; modify the AuthService refresh token flow to catch these
specific exceptions (in addition to existing catches) around the
UserRole.valueOf call and rethrow or map them to the same authentication-related
exception used elsewhere (e.g., InvalidTokenException/AuthenticationException)
so invalid or missing roles are handled consistently; locate the code around
AuthService where jwtProvider.extractUserRoleFromToken(...) and
UserRole.valueOf(...) are invoked and update the try/catch to handle
IllegalArgumentException and NullPointerException and produce the unified auth
error.

checkIsSameRefreshToken(userId, refreshToken);

final User user = userRetriever.findUserById(userId);
final Token newToken = getLoginOrReissueJwtToken(user.getUserId(), user.getUserRole());
final Token newToken = getLoginOrReissueJwtToken(userId, userRole);

saveRefreshTokenInRedis(user.getUserId(), newToken.getRefreshToken());
saveRefreshTokenInRedis(userId, newToken.getRefreshToken());
Comment on lines 100 to +106
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

재발급 시 role을 토큰 claim만 신뢰하면 권한/계정 상태 변경 반영이 늦어질 수 있습니다.

Line 101~Line 104에서 사용자 현재 상태를 서버 측 소스로 재확인하지 않고 RT의 role claim으로 바로 재발급합니다. 이 경로는 역할 강등/계정 비활성화(또는 삭제) 이후에도 기존 RT가 유효한 동안 이전 권한으로 AT 재발급이 가능해질 수 있습니다. 재발급 시점에 사용자 상태(존재/활성/권한)를 다시 검증하는 경로를 유지하는 것이 안전합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java`
around lines 100 - 106, In reissue flow replace blind trust of the RT role
claim: after extracting userId from refreshToken
(jwtProvider.extractUserIdFromToken) load the current user record (e.g., via
UserRepository or UserService) and verify existence, active status, and current
role, then call checkIsSameRefreshToken(userId, refreshToken); use the
server-side role from the loaded user when calling getLoginOrReissueJwtToken (do
not use jwtProvider.extractUserRoleFromToken), and if the user is
missing/inactive or role mismatches, throw an appropriate auth exception before
saveRefreshTokenInRedis.


return TokenDto.of(newToken.getAccessToken(), newToken.getRefreshToken());
} catch (AuthWrongJwtException | AuthRTNotFoundException e) {
} catch (AuthWrongJwtException e) {
throw new AuthUnAuthorizedException(ErrorCode.UNAUTHORIZED_WRONG_RT);
} catch (AuthExpiredJwtException e) {
} catch (AuthExpiredJwtException |AuthRTNotFoundException e) {
throw new AuthUnAuthorizedException(ErrorCode.UNAUTHORIZED_RT_EXPIRED);
} catch (AuthRTException e) {
throw new AuthUnAuthorizedException(ErrorCode.INTERNAL_RT_REDIS_ERROR);
Expand All @@ -125,7 +125,6 @@ public void logout(final long userId, final String refreshTokenFromCookie) {
} catch (DataAccessException e) {
throw new AuthRedisException(ErrorCode.INTERNAL_RT_REDIS_ERROR);
}

}

private void checkIsSameRefreshToken(final long userId, final String refreshToken) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,11 @@
@RedisHash("refreshToken")
public class RefreshToken {

/** 사용자 식별자(키). 사용자당 1개 세션만 허용 모델 */
@Id
private Long userId;

/** 토큰 해시(SHA-256 등). 평문 저장 금지 */
private String refreshToken;

/** 만료 시간(초 단위). @RedisHash 클래스 단위 TTL 대신 인스턴스별 TTL 추천 */
@TimeToLive
private long ttlSeconds;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ public long extractUserIdFromToken(final String token) {

public String extractUserRoleFromToken(final String token) {
final Jws<Claims> claims = parseToken(token);
return claims.getBody().get(Constants.USER_ROLE, String.class);
final String role = claims.getBody().get(Constants.USER_ROLE, String.class);
if(role == null) {
throw new AuthWrongJwtException();
}
return role;
Comment on lines +51 to +55
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

USER_ROLE는 null 체크만 하지 말고 허용 역할값까지 검증해 주세요.

현재는 null만 차단하고 임의 문자열은 통과됩니다. reissue가 DB 조회 없이 토큰 클레임에 의존하므로, 여기서 UserRole로 파싱/검증 후 실패 시 AuthWrongJwtException으로 매핑하는 편이 안전합니다.

제안 수정안
-    public String extractUserRoleFromToken(final String token) {
+    public UserRole extractUserRoleFromToken(final String token) {
         final Jws<Claims> claims = parseToken(token);
         final String role = claims.getBody().get(Constants.USER_ROLE, String.class);
-        if(role == null) {
+        if (role == null) {
             throw new AuthWrongJwtException();
         }
-        return role;
+        try {
+            return UserRole.valueOf(role);
+        } catch (IllegalArgumentException e) {
+            throw new AuthWrongJwtException();
+        }
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/JwtProvider.java`
around lines 51 - 55, The code in JwtProvider currently only checks that
Constants.USER_ROLE is non-null but allows arbitrary strings; update the logic
to parse and validate the claim as a known UserRole enum/value and throw
AuthWrongJwtException on invalid/unknown values. Specifically, retrieve the
claim via claims.getBody().get(Constants.USER_ROLE, String.class), attempt to
convert it to the UserRole (e.g., UserRole.valueOf or a safe parser on the
UserRole type) and if parsing fails or the value is not one of the allowed
roles, throw AuthWrongJwtException instead of returning the raw string.

}

//토큰 파싱
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,30 @@

@Component
@Slf4j
@Profile("!local")
//@Profile("!local")
@Order(Ordered.HIGHEST_PRECEDENCE)
class MDCLoggingFilter extends OncePerRequestFilter {
class RequestObservabilityFilter extends OncePerRequestFilter {
private static final String NGINX_REQUEST_ID = "X-Request-ID";
private static final String TRACE_ID = "trace_id";

private static final String URI = "uri";
private static final String METHOD = "method";
private static final String STATUS = "status";

private static final long SLOW_REQUEST_THRESHOLD_MS = 1000L;

@Override
protected void doFilterInternal(@NonNull final HttpServletRequest request,
@NonNull final HttpServletResponse response,
@NonNull final FilterChain filterChain) throws ServletException, IOException {
@NonNull final HttpServletResponse response,
@NonNull final FilterChain filterChain) throws ServletException, IOException {
final String uri = request.getRequestURI();
// 헬스체크는 패스
if (uri != null && uri.contains(Constants.HEALTH_CHECK_URL)) {
filterChain.doFilter(request, response);
return;
}

final long start = System.currentTimeMillis();
try {
String traceId = request.getHeader(NGINX_REQUEST_ID);
if (traceId == null || traceId.isBlank()) {
Expand All @@ -48,9 +52,17 @@ protected void doFilterInternal(@NonNull final HttpServletRequest request,
MDC.put(METHOD, request.getMethod());

filterChain.doFilter(request, response);

MDC.put(STATUS, String.valueOf(response.getStatus()));
} finally {
final long duration = System.currentTimeMillis() - start;
final int status = response.getStatus();
final String method = request.getMethod();

if (duration >= SLOW_REQUEST_THRESHOLD_MS) {
log.warn("[SLOW] {} {} → {} ({}ms)", method, uri, status, duration);
} else {
log.info("{} {} → {} ({}ms)", method, uri, status, duration);
}

MDC.remove(STATUS);
MDC.remove(METHOD);
MDC.remove(URI);
Expand Down