Skip to content

Conversation

@tjdgus3488
Copy link
Contributor

@tjdgus3488 tjdgus3488 commented Jan 7, 2026

📄 작업 내용 요약

refreshToken의 RTR 방식을 기반으로 로그아웃 구현하였습니다.

기존 JWT 인증 과정에서 AT(accessToken)와 RT(refreshToken)의 불필요한 혼용을 제거하였습니다.
RT의 여부와 상관없이, AT 유효/만료/없음 만을 분리하여 필터를 동작하되, AT의 만료 시 RT의 refresh를 호출하도록 설계하였습니다.

로그아웃 시

  1. RefreshToken 쿠키가 존재하면 → Redis에서 해당 디바이스 토큰 무효화
  2. RefreshToken 쿠키는 항상 삭제(Set-Cookie, maxAge=0)
  3. SecurityContext 정리
    로직으로 RT를 무효화하는 방향으로 구현하였습니다.

또한, 핵심 수정 사항은 AT의 블랙리스트 방식을 사용하지 않았습니다.
로그아웃 시, 현재 AT를 redis의 블랙리스트에 등록하여 인증 과정에서 제외하는 방법을 많이 사용하는데, 관련 내용을 찾아보면서
redis의 블랙리스트를 사용함으로 인해 JWT의 stateless함을 해친다 + 추가 옵션임에도 불구하고 복잡도에서 비용을 늘인다 라는 관점의 글을 찾을 수 있었습니다.
따라서 AT의 expirationTime을 최소한(10~15)분으로 줄이고 RT를 RTR 방식으로 사용하여 관리하되,
로그아웃은 RT의 무효화로 AT를 재발급받지 못하도록 한다. 라는 관점으로 구현하였습니다.

보안적인 관점에서 블랙리스트 사용으로 즉시 차단하는 것이 마땅한 방식이라고는 생각하지만, 현재 구현 정도로 관리하는 것이 실무에서 사용될 수 있는 정도라면 참고할 만한 관점이라고 생각하여 우선 구현해두었습니다만, 어떻게 구현하는 것이 맞는 방향일지 혼자 고민하기에는 조금 모호한 부분이 있어 다른 분들의 의견이 궁금합니다.

📎 Issue 번호

Summary by CodeRabbit

  • New Features

    • POST /auth/logout 추가: 리프레시 토큰 쿠키를 제거하고 보안 컨텍스트를 초기화하여 로그아웃 처리(응답: "로그아웃 완료").
    • 회원 탈퇴 처리 추가: 계정 소프트 삭제와 삭제 시각 기록, 상태를 WITHDRAWN으로 전환.
  • Refactor

    • 인증 필터 간소화 및 일부 토큰 검증/블랙리스트 로직 제거로 인증 흐름 단순화.
    • 리프레시 토큰 쿠키 발급/삭제 경로 통일 및 쿠키 삭제 API 추가로 클라이언트 호환성 개선.

✏️ Tip: You can customize this high-level summary in your review settings.

@tjdgus3488 tjdgus3488 self-assigned this Jan 7, 2026
@tjdgus3488 tjdgus3488 added ✨ Feature 새로운 기능 개발 👨‍💻 성현 담당자: 성현 labels Jan 7, 2026
@tjdgus3488 tjdgus3488 linked an issue Jan 7, 2026 that may be closed by this pull request
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 7, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

로그아웃 및 회원 소프트 삭제 관련 변경이 도입되었습니다. POST /auth/logout 엔드포인트를 제공하는 LogoutController가 추가되어 요청에서 리프레시 토큰을 읽고 유효하면 memberId/deviceId로 RefreshTokenService에 무효화 요청을 시도한 뒤 리프레시 토큰 쿠키를 제거하고 SecurityContext를 클리어하며 NO_CONTENT 응답을 반환합니다. Member 엔티티에 isDeleted 및 deletedAt 필드와 withdraw() 메서드가 추가되고 MemberStatus에 WITHDRAWN 상수가 추가되었습니다. SecurityConfig에 /auth/logout이 공개 경로로 포함되었고, JwtAuthenticationFilter에서 RedisTemplate 및 블랙리스트 로직이 제거되어 인증/예외 흐름이 단순화되었습니다. JwtService에 clearRefreshTokenCookie(HttpServletResponse) 메서드가 추가되었고 JwtServiceImpl에 환경별 쿠키 삭제 구현 및 리프레시 쿠키 경로 변경("/")이 포함되었습니다.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR 제목은 로그아웃 구현이라는 주요 변경사항을 명확하게 반영하며, 구현된 LogoutController와 RefreshToken 무효화 로직을 정확히 요약합니다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b336eb and d849aaa.

📒 Files selected for processing (1)
  • src/main/java/com/tavemakers/surf/global/jwt/JwtServiceImpl.java

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/java/com/tavemakers/surf/global/jwt/JwtServiceImpl.java (1)

144-156: 환경 분기 로직의 네이밍이 의도와 불일치합니다.

Lines 144-156과 204-213의 환경 분기 로직에서 isDev=true일 때 프로덕션 설정(secure=true, domain=.tavesurf.site, sameSite=None)을 적용하고, else 블록에서 로컬 개발 설정(secure=false, sameSite=Lax)을 적용하고 있습니다.

문제점:

  1. 변수명 isDev가 실제 동작과 반대 (dev=true → 배포 환경 설정 적용)
  2. Line 130-133의 TODO 주석도 혼란을 가중시킴
  3. 코드 유지보수 시 오해 가능성 높음

원인 추정:
spring.profiles.active가 "dev"로 설정된 환경이 실제로는 배포 서버이고, 로컬 환경은 다른 값(또는 미설정)을 사용하는 것으로 보입니다.

♻️ 명확한 네이밍으로 개선
-    boolean isDev = "dev".equalsIgnoreCase(activeProfile);
+    // 배포 환경(tavesurf.site)에서는 Secure + SameSite=None 필요
+    boolean isDeployedEnvironment = "dev".equalsIgnoreCase(activeProfile);
 
     ResponseCookie.ResponseCookieBuilder builder =
             ResponseCookie.from(REFRESH_COOKIE_NAME, refreshToken)
                     .httpOnly(true)
                     .path("/")
                     .maxAge(Duration.ofMillis(refreshTokenExpireMs));
 
-    if (isDev) {
-        // 배포 서버
+    if (isDeployedEnvironment) {
+        // 배포 서버 (tavesurf.site)
         builder
                 .secure(true)
                 .domain(".tavesurf.site")
                 .sameSite("None");
     } else {
-        // 로컬
+        // 로컬 개발 환경
         builder
                 .secure(false)
                 .sameSite("Lax");
-        // ⚠️ domain 설정하지 않음
     }

또는 프로파일명을 "prod" 또는 "deployed"로 변경하여 의도를 명확히 하는 것을 권장합니다.

Also applies to: 204-213

🤖 Fix all issues with AI agents
In @src/main/java/com/tavemakers/surf/domain/member/entity/Member.java:
- Around line 83-86: Add a Flyway migration that applies the new soft-delete
columns from the Member entity: create a new migration file (e.g.,
V{next_version}__add_soft_delete_to_member.sql) that ALTER TABLE member to ADD
COLUMN is_deleted BOOLEAN NOT NULL DEFAULT FALSE and ADD COLUMN deleted_at
TIMESTAMP NULL, and create an index on is_deleted (e.g., CREATE INDEX
idx_member_is_deleted ON member(is_deleted)); this will keep the database schema
in sync with the Member class fields isDeleted and deletedAt (and the @Where
soft-delete behavior).
- Line 34: The withdraw() method currently only marks isDeleted = true but fails
to anonymize personal data; update the Member.withdraw() implementation to set
isDeleted = true, set deletedAt (e.g., LocalDateTime.now()), set activityStatus
to false and status to MemberStatus.WITHDRAWN, and null out or replace sensitive
fields (email, phoneNumber, kakaoId, name -> fixed placeholder like "(탈퇴 회원)",
profileImageUrl, selfIntroduction) so no PII remains while preserving existing
post/comment associations; ensure you modify the Member class fields referenced
above and any related getters/setters to allow these changes and keep @Where
behavior unchanged for now.
- Around line 259-264: withdraw() currently only flips flags and leaves personal
data intact; update Member.withdraw() to also anonymize PII fields (email,
phoneNumber, kakaoId, realName, profileImageUrl, schoolInfo, bio, links) by
clearing or replacing them with non-identifying placeholders while preserving
member id and deletedAt, and ensure activityStatus and status remain set to
MemberStatus.WITHDRAWN; additionally, in the service handling withdrawal (where
memberRepository.save(...) is called) call the refresh token invalidation flow
(e.g., refreshTokenService.invalidateAll(memberId)) to revoke all refresh tokens
for that member and ensure related read models show "withdrawn user" for
posts/comments per data policy.

In @src/main/java/com/tavemakers/surf/global/jwt/JwtAuthenticationFilter.java:
- Around line 78-82: The unauthorized(HttpServletResponse res, String message)
method in JwtAuthenticationFilter builds JSON via string concatenation which can
break or allow XSS when message contains quotes or backslashes; fix it by
serializing a simple error DTO with a shared ObjectMapper (or new ObjectMapper)
instead of manual string construction — create a small ErrorResponse class
(e.g., private static class ErrorResponse { private String message; }) and
replace res.getWriter().write("{\"message\":\"" + message + "\"}"); with
res.getWriter().write(objectMapper.writeValueAsString(new
ErrorResponse(message))); ensuring objectMapper is available/injected or
instantiated and handling IOException as before.

In @src/main/java/com/tavemakers/surf/global/jwt/JwtServiceImpl.java:
- Line 141: Update JwtServiceImpl to handle cookie path migration and fix the
env flag logic: in the logout/refresh-cookie creation flow (e.g., methods that
build the refresh-token cookie such as the code that sets .path("/")), ensure
you clear the cookie at both "/" and "/auth/refresh" (send Set-Cookie deletions
for both paths) so existing cookies on the old path are removed; also correct
the inverted environment flag by renaming or flipping logic around isDev (or
change to isProd) so that when running locally secure=false and domain is unset,
and when in production secure=true and domain=".tavesurf.site" — update the
conditional(s) that set secure and domain accordingly.
🧹 Nitpick comments (4)
src/main/java/com/tavemakers/surf/domain/login/controller/LogoutController.java (1)

25-45: 로그아웃 로직이 견고하게 구현되었습니다.

RTR 방식에 맞춘 로그아웃 플로우가 잘 구현되어 있습니다:

  • 리프레시 토큰이 유효한 경우 Redis에서 무효화
  • 토큰 상태와 무관하게 쿠키는 항상 삭제 (멱등성 보장)
  • SecurityContext 정리

선택적 개선 제안:

  1. Optional 체이닝 가독성 개선 (Lines 28-36):
    현재 중첩된 ifPresentorElse(null) 체인을 flatMap으로 더 명확하게 표현할 수 있습니다.

  2. 보안 감사 로깅 추가:
    로그아웃 이벤트를 로깅하면 보안 감사 추적에 유용합니다.

♻️ 개선 제안
 @PostMapping("/auth/logout")
 public ApiResponse<Void> logout(HttpServletRequest request, HttpServletResponse response) {
 
-    jwtService.extractRefreshToken(request).ifPresent(refreshToken -> {
-        if (jwtService.isTokenValid(refreshToken)) {
-            Long memberId = jwtService.extractMemberId(refreshToken).orElse(null);
-            String deviceId = jwtService.extractDeviceId(refreshToken).orElse(null);
-            if (memberId != null && deviceId != null) {
-                refreshTokenService.invalidate(memberId, deviceId);
-            }
-        }
-    });
+    jwtService.extractRefreshToken(request)
+        .filter(jwtService::isTokenValid)
+        .ifPresent(refreshToken -> {
+            jwtService.extractMemberId(refreshToken)
+                .flatMap(memberId -> jwtService.extractDeviceId(refreshToken)
+                    .map(deviceId -> {
+                        refreshTokenService.invalidate(memberId, deviceId);
+                        log.info("User logged out: memberId={}, deviceId={}", memberId, deviceId);
+                        return true;
+                    }));
+        });
 
     // 쿠키 삭제는 항상 수행
     jwtService.clearRefreshTokenCookie(response);
src/main/java/com/tavemakers/surf/global/jwt/JwtServiceImpl.java (1)

194-217: 쿠키 삭제 로직의 구조는 견고합니다.

clearRefreshTokenCookie 구현이 발급 로직(sendRefreshToken)과 동일한 환경별 속성 및 경로를 사용하여 일관성을 유지하고 있습니다. maxAge(Duration.ZERO)로 즉시 만료 처리하는 방식도 정확합니다.

선택적 개선:
보안 감사를 위해 쿠키 삭제 작업을 로깅하는 것이 좋습니다.

@Override
public void clearRefreshTokenCookie(HttpServletResponse res) {
    // ... existing code ...
    
    ResponseCookie refreshCookie = builder.build();
    res.addHeader("Set-Cookie", refreshCookie.toString());
    log.debug("Refresh token cookie cleared");
}
src/main/java/com/tavemakers/surf/global/config/SecurityConfig.java (1)

36-36: 불필요한 RedisTemplate 의존성 제거를 권장합니다.

SecurityConfig 클래스에서 redisTemplate 필드(36줄)가 선언되어 있지만 전혀 사용되지 않고 있습니다. 특히 JwtAuthenticationFilter 생성(101줄)에서도 jwtServicememberRepository만 전달되므로, 이 필드를 제거해도 무방합니다.

@requiredargsconstructor를 사용 중이므로 필드를 제거하면 자동으로 생성자 파라미터도 정리됩니다. 이렇게 하면 불필요한 의존성 주입을 줄여 클래스의 책임을 명확히 할 수 있습니다.

src/main/java/com/tavemakers/surf/global/jwt/JwtAuthenticationFilter.java (1)

37-60: 세 단계 검증 로직이 간결하고 명확합니다.

토큰 없음 → 익명 허용, 유효하지 않음 → 401 반환, 유효함 → 인증 주입의 흐름이 직관적이며, 짧은 AT 만료시간과 RTR 방식으로 보안을 유지하는 전략이 합리적입니다.

탈퇴한 회원(isDeleted=true)의 경우 Member 엔티티의 @Where(clause = "is_deleted = false") 어노테이션에 의해 findById() 쿼리 자체에서 필터링되므로, AT 만료 전이라도 ifPresent 블록이 실행되지 않아 인증되지 않습니다. 따라서 명시적 검증 없이도 자동으로 안전하게 처리되고 있습니다.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 95c9bcb and a04c985.

📒 Files selected for processing (7)
  • src/main/java/com/tavemakers/surf/domain/login/controller/LogoutController.java
  • src/main/java/com/tavemakers/surf/domain/member/entity/Member.java
  • src/main/java/com/tavemakers/surf/domain/member/entity/enums/MemberStatus.java
  • src/main/java/com/tavemakers/surf/global/config/SecurityConfig.java
  • src/main/java/com/tavemakers/surf/global/jwt/JwtAuthenticationFilter.java
  • src/main/java/com/tavemakers/surf/global/jwt/JwtService.java
  • src/main/java/com/tavemakers/surf/global/jwt/JwtServiceImpl.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/tavemakers/surf/domain/login/controller/LogoutController.java (1)
src/main/java/com/tavemakers/surf/global/jwt/JwtAuthenticationFilter.java (1)
  • RequiredArgsConstructor (19-83)
🔇 Additional comments (5)
src/main/java/com/tavemakers/surf/global/jwt/JwtService.java (1)

23-24: 인터페이스 확장이 깔끔합니다.

로그아웃 시 리프레시 토큰 쿠키 제거를 위한 메서드가 적절하게 추가되었습니다. 메서드명도 의도를 명확하게 표현하고 있네요.

src/main/java/com/tavemakers/surf/global/config/SecurityConfig.java (1)

60-60: 로그아웃 엔드포인트 공개 접근 설정 정상.

인증 없이 로그아웃을 호출할 수 있도록 /auth/logout을 공개 URL로 지정한 것이 적절합니다. 로그아웃은 인증 상태와 무관하게 동작해야 하므로 올바른 구현입니다.

src/main/java/com/tavemakers/surf/domain/member/entity/enums/MemberStatus.java (1)

7-8: 탈퇴 상태 추가 완료.

WITHDRAWN 상태가 깔끔하게 추가되었습니다. 회원 탈퇴 기능을 위한 적절한 확장입니다.

src/main/java/com/tavemakers/surf/global/jwt/JwtAuthenticationFilter.java (2)

29-33: 엔드포인트 필터링 로직이 명확합니다.

로그아웃, 토큰 갱신, API 문서 경로를 인증 필터에서 제외하는 것은 적절한 접근입니다. 공개 엔드포인트 목록이 잘 정리되어 있습니다.


63-76: SecurityContext 설정이 Spring Security 모범 사례를 따릅니다.

CustomUserDetails 생성 → UsernamePasswordAuthenticationToken 구성 → WebAuthenticationDetailsSource로 요청 상세 정보 추가 → 새로운 SecurityContext에 설정하는 흐름이 정확합니다. Optional 체이닝으로 안전하게 처리한 점도 좋습니다.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a04c985 and f08e3e4.

📒 Files selected for processing (1)
  • src/main/java/com/tavemakers/surf/global/jwt/JwtAuthenticationFilter.java
🔇 Additional comments (3)
src/main/java/com/tavemakers/surf/global/jwt/JwtAuthenticationFilter.java (3)

27-27: ObjectMapper 추가로 XSS 취약점이 해결되었습니다.

이전 리뷰에서 지적된 JSON 문자열 연결 방식의 XSS 취약점이 ObjectMapper를 사용하여 안전하게 수정되었습니다. 👍

선택적 개선사항: Spring에서 설정된 ObjectMapper를 주입받으면 날짜 포맷 등 프로젝트 전역 설정과 일관성을 유지할 수 있습니다. 현재 방식도 동작에는 문제없습니다.


30-37: 필터 제외 경로 설정이 적절합니다.

/auth/logout, /auth/refresh, Swagger 엔드포인트가 필터 제외 목록에 추가되어 RTR 방식의 토큰 흐름과 일치합니다. SecurityConfig의 공개 경로 설정과도 일관성이 있습니다.


39-63: 인증 흐름이 RTR 방식에 맞게 단순화되었습니다.

AT가 없으면 익명 통과, 유효하지 않으면 401, 유효하면 인증 주입 — 세 가지 경로가 명확하게 분리되어 있습니다. Redis 블랙리스트 로직 제거로 복잡도가 감소했고, AT 만료 시 클라이언트가 /auth/refresh를 호출하도록 유도하는 설계가 일관성 있습니다.

Comment on lines 65 to 79
/** AccessToken → memberId → Member 로드 → SecurityContext 주입 */
private void authenticateUser(String accessToken, HttpServletRequest req) {
jwtService.extractMemberId(accessToken).flatMap(memberRepository::findById).ifPresent(member -> {
CustomUserDetails principal = new CustomUserDetails(member);
UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(
principal, null, principal.getAuthorities());
auth
.setDetails(new WebAuthenticationDetailsSource().buildDetails(req));

SecurityContext context = SecurityContextHolder.createEmptyContext();
context.setAuthentication(auth);
SecurityContextHolder.setContext(context);
jwtService.extractMemberId(accessToken)
.flatMap(memberRepository::findById)
.ifPresent(member -> {
CustomUserDetails principal = new CustomUserDetails(member);
UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(
principal, null, principal.getAuthorities());
auth.setDetails(new WebAuthenticationDetailsSource().buildDetails(req));

SecurityContext context = SecurityContextHolder.createEmptyContext();
context.setAuthentication(auth);
SecurityContextHolder.setContext(context);
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

회원 조회 실패 시 silent failure가 발생할 수 있습니다.

memberRepository.findById()가 빈 결과를 반환하면(탈퇴한 사용자 등) ifPresent로 인해 아무 동작 없이 넘어갑니다. 유효한 AT를 가진 요청이 익명으로 처리되어 의도치 않은 동작을 유발할 수 있습니다.

원인: @Where(clause = "is_deleted = false")가 적용된 Member 엔티티에서 soft-delete된 사용자는 조회되지 않습니다.

권장 개선안:

  1. 회원이 없을 때 401 응답 반환
  2. 최소한 디버깅을 위한 로그 추가
🛠️ 권장 수정 코드
     /** AccessToken → memberId → Member 로드 → SecurityContext 주입 */
-    private void authenticateUser(String accessToken, HttpServletRequest req) {
-        jwtService.extractMemberId(accessToken)
-                .flatMap(memberRepository::findById)
-                .ifPresent(member -> {
-                    CustomUserDetails principal = new CustomUserDetails(member);
-                    UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(
-                            principal, null, principal.getAuthorities());
-                    auth.setDetails(new WebAuthenticationDetailsSource().buildDetails(req));
-
-                    SecurityContext context = SecurityContextHolder.createEmptyContext();
-                    context.setAuthentication(auth);
-                    SecurityContextHolder.setContext(context);
-        });
-    }
+    private boolean authenticateUser(String accessToken, HttpServletRequest req) {
+        return jwtService.extractMemberId(accessToken)
+                .flatMap(memberRepository::findById)
+                .map(member -> {
+                    CustomUserDetails principal = new CustomUserDetails(member);
+                    UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(
+                            principal, null, principal.getAuthorities());
+                    auth.setDetails(new WebAuthenticationDetailsSource().buildDetails(req));
+
+                    SecurityContext context = SecurityContextHolder.createEmptyContext();
+                    context.setAuthentication(auth);
+                    SecurityContextHolder.setContext(context);
+                    return true;
+                })
+                .orElseGet(() -> {
+                    log.warn("Member not found for valid token");
+                    return false;
+                });
+    }

doFilterInternal에서 반환값 처리:

-        // 3) 유효하면 인증 주입 후 통과
-        authenticateUser(accessToken, request);
-        chain.doFilter(request, response);
+        // 3) 유효하면 인증 주입 후 통과
+        if (!authenticateUser(accessToken, request)) {
+            unauthorized(response, "Member not found");
+            return;
+        }
+        chain.doFilter(request, response);

Spring Security 공식 문서의 Authentication Architecture에서 인증 실패 처리 패턴을 참고하실 수 있습니다.

Committable suggestion skipped: line range outside the PR's diff.

@koreaioi koreaioi requested review from a team, GOOHAESEUNG, LAICME and koreaioi January 7, 2026 08:21
Copy link
Member

@koreaioi koreaioi left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다!
RefreshToken 관련 레퍼런스 공유해주시면 좋을 거 같아요!!

Copy link
Contributor

@LAICME LAICME left a comment

Choose a reason for hiding this comment

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

수고하셨습니다.

동아리 내부에서 쓸 서비스이기 때문에 지금 구현해두신 RT 무효화 방식이 더 적절하다고 생각합니다.

(처음에는 로그아웃 후에도 AT가 살아 있으면 위험하지 않나? 라고 생각이 들었습니다. 그렇지만 AT의 만료 시간을 짧게 가져가고 RT를 RTR 방식으로 관리할 경우에 악용 가능한 시간과 범위는 매우 제한적일 것 같습니다.)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/java/com/tavemakers/surf/global/jwt/JwtServiceImpl.java (1)

157-169: 운영 환경(prod) 프로필 처리 누락 및 주석 오류

두 가지 문제가 있습니다:

  1. 주석 오류: Line 158의 // 운영 주석이 isDev() 블록 안에 있어 혼란을 줍니다. isDev()는 개발 환경인데 주석은 운영이라고 표시되어 있습니다.

  2. Production 프로필 미처리: isDev()isTest() 외에 prod 프로필에 대한 처리가 없습니다. 운영 환경에서 쿠키가 secure, sameSite, domain 설정 없이 발급되어 보안상 취약해질 수 있습니다.

🔧 제안하는 수정 방안
-        if (isDev()) {
-            // 운영
+        if (isProd()) {
+            // 운영 환경
             builder
                     .secure(true)
                     .sameSite("None")
                     .domain(".tavesurf.site");
-
-        } else if (isTest()) {
-            // 테스트
+        } else {
+            // 개발/테스트 환경
             builder
                     .secure(false)      // HTTP라서 false
                     .sameSite("Lax");   // domain 설정 ❌
         }

그리고 isProd() 헬퍼 메서드 추가:

private boolean isProd() {
    return Arrays.asList(environment.getActiveProfiles()).contains("prod");
}
🧹 Nitpick comments (2)
src/main/java/com/tavemakers/surf/global/jwt/JwtServiceImpl.java (2)

207-228: clearRefreshTokenCookie 메서드 - 동일한 환경 처리 문제 및 중복 코드

sendRefreshToken과 동일하게 prod 프로필 처리가 누락되어 있습니다. 또한 쿠키 빌더 설정 로직이 두 메서드에서 중복됩니다.

중복 코드를 제거하고 환경별 설정을 일관되게 적용하려면 헬퍼 메서드로 추출하는 것을 권장합니다.

♻️ 중복 제거를 위한 리팩토링 제안
private void applyEnvironmentCookieSettings(ResponseCookie.ResponseCookieBuilder builder) {
    if (isProd()) {
        builder
            .secure(true)
            .sameSite("None")
            .domain(".tavesurf.site");
    } else {
        builder
            .secure(false)
            .sameSite("Lax");
    }
}

그 후 sendRefreshTokenclearRefreshTokenCookie 모두에서 이 헬퍼를 호출하세요:

 @Override
 public void clearRefreshTokenCookie(HttpServletResponse res) {
     ResponseCookie.ResponseCookieBuilder builder =
             ResponseCookie.from(REFRESH_COOKIE_NAME, "")
                     .httpOnly(true)
                     .path("/")
                     .maxAge(Duration.ZERO);

-    if (isDev()) {
-        builder
-                .secure(true)
-                .domain(".tavesurf.site")
-                .sameSite("None");
-    } else if (isTest()) {
-        builder
-                .secure(false)
-                .sameSite("Lax");
-    }
+    applyEnvironmentCookieSettings(builder);

     ResponseCookie refreshCookie = builder.build();
     res.addHeader("Set-Cookie", refreshCookie.toString());
 }

143-148: 주석 처리된 코드 정리 권장

TODO 주석과 주석 처리된 코드(lines 45-46, 143-148)가 남아 있습니다. 환경별 처리 로직이 이미 isDev(), isTest() 메서드로 구현되어 있으므로 불필요한 주석을 정리하면 코드 가독성이 향상됩니다.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 21d0c70 and 1b336eb.

📒 Files selected for processing (1)
  • src/main/java/com/tavemakers/surf/global/jwt/JwtServiceImpl.java
🔇 Additional comments (1)
src/main/java/com/tavemakers/surf/global/jwt/JwtServiceImpl.java (1)

154-154: 경로 변경 적절합니다.

쿠키 삭제 시 발급 경로와 동일해야 브라우저가 올바르게 쿠키를 제거합니다. clearRefreshTokenCookie와 경로가 일치하여 로그아웃 시 쿠키가 정상적으로 삭제됩니다.

@tjdgus3488
Copy link
Contributor Author

고생 많으셨습니다! RefreshToken 관련 레퍼런스 공유해주시면 좋을 거 같아요!!

https://medium.com/%40sarahnzeshi05/blacklisting-jwts-when-why-and-why-its-usually-a-bad-idea-4cc69a4988ce
https://mojoauth.com/ciam-qna/how-to-implement-logout-functionality-with-stateless-jwt-tokens

등등 이런저런 게시물들이 있었는데, 조금 더 찾아본 결과 블랙리스트의 도입이 JWT의 stateless함을 해친다는 단점은 있지만 현실적으로 완벽한 보안을 위해 희생한다(?)라는 관점이 지배적인 것 같습니다. 우선 머지하고 블랙리스트는 추가로 구현하도록 하겠습니다!

@tjdgus3488 tjdgus3488 merged commit 574dca4 into dev Jan 9, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ Feature 새로운 기능 개발 👨‍💻 성현 담당자: 성현

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEAT] 회원탈퇴 구현

4 participants