feat: 에러 시 http 응답 코드 변경 #118
Conversation
WalkthroughError handling and HTTP status mappings were tightened across auth components. parseRefreshToken now attaches exception messages to CoreException; revokeUserTokens rethrows CoreException and logs other exceptions; JwtUtil lets ExpiredJwtException propagate; JwtFilter distinguishes expired vs invalid access tokens; three ErrorType entries now map to 401; two error messages were clarified; CoreException constructor now accepts a message. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant TC as TokenController
participant TS as TokenService
participant JT as JwtUtil
participant JF as JwtFilter
participant ET as CoreException/ErrorType
rect rgb(240,248,255)
Note over C,TC: Refresh token reissue flow
C->>TC: POST /auth/reissue (refreshToken)
TC->>TS: parseRefreshToken(refreshToken)
TS->>JT: parse/validate(refreshToken)
JT-->>TS: Claims or throws
alt ExpiredJwtException
TS->>ET: throw CoreException(REFRESH_TOKEN_EXPIRED, e.message)
TC-->>C: 401 + ApiResponse.error(REFRESH_TOKEN_EXPIRED)
else Signature/Malformed/IllegalArg
TS->>ET: throw CoreException(INVALID_REFRESH_TOKEN, e.message)
TC-->>C: Error response (status from ErrorType)
else Success
TS-->>TC: Claims
TC-->>C: New tokens (200)
end
end
rect rgb(245,255,250)
Note over C,JF: Incoming request with access token
C->>JF: Request with access token
JF->>JT: isValid(token)
JT-->>JF: true or throws
alt Valid
JF->>JF: continue filter chain
else ExpiredJwtException
JF->>ET: rejectInvalidToken(ACCESS_TOKEN_EXPIRED)
JF-->>C: 401 (ACCESS_TOKEN_EXPIRED)
else Signature/Malformed/IllegalArg
JF->>ET: rejectInvalidToken(INVALID_ACCESS_TOKEN)
JF-->>C: 401/400 per mapping
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorType.java (1)
28-29: 401 for expired tokens: alignment with client behavior and RFC 6750 looks goodChanging ACCESS_TOKEN_EXPIRED and REFRESH_TOKEN_EXPIRED to UNAUTHORIZED (401) matches the PR goal and is semantically correct for Bearer token failures. Consider also returning a WWW-Authenticate header from the exception handler for better client interoperability.
If you want, I can propose a small addition in your global exception handler to set:
WWW-Authenticate: Bearer error="invalid_token", error_description="token expired"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
capturecat-core/src/main/java/com/capturecat/core/service/auth/TokenService.java(2 hunks)capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorType.java(1 hunks)
| throw new CoreException(ErrorType.REFRESH_TOKEN_EXPIRED, e.getMessage()); | ||
| } catch (SignatureException | MalformedJwtException | IllegalArgumentException e) { | ||
| throw new CoreException(ErrorType.INVALID_REFRESH_TOKEN); | ||
| throw new CoreException(ErrorType.INVALID_REFRESH_TOKEN, e.getMessage()); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Constructor consistency and message exposure: avoid leaking low-level JWT messages; update throws or restore convenience constructor
You now pass e.getMessage() into CoreException for JWT parsing errors. Two concerns:
- If CoreException was changed to only accept (ErrorType, String), earlier calls in this class still use the single-arg constructor (Lines 71, 79, 85). That will not compile unless an overload remains.
- Propagating library exception messages to clients can leak internal details. Prefer a sanitized, product-defined message (or null) and keep the detailed cause only in logs.
Action options:
- Option A (recommended): keep a 1-arg convenience constructor in CoreException and use sanitized messages only when truly helpful to the client.
- Option B: update all throw sites in this class to provide a sanitized message string.
Proposed minimal change here (sanitized messages), keeping your behavior:
- } catch (ExpiredJwtException e) {
- throw new CoreException(ErrorType.REFRESH_TOKEN_EXPIRED, e.getMessage());
- } catch (SignatureException | MalformedJwtException | IllegalArgumentException e) {
- throw new CoreException(ErrorType.INVALID_REFRESH_TOKEN, e.getMessage());
+ } catch (ExpiredJwtException e) {
+ throw new CoreException(ErrorType.REFRESH_TOKEN_EXPIRED, "Refresh token expired");
+ } catch (SignatureException | MalformedJwtException | IllegalArgumentException e) {
+ throw new CoreException(ErrorType.INVALID_REFRESH_TOKEN, "Invalid refresh token");Follow-up edits outside the changed lines (apply if the 1-arg constructor was removed):
// Lines 71, 79, 85
throw new CoreException(ErrorType.INVALID_REFRESH_TOKEN, "Invalid refresh token");Verification script to detect remaining single-arg usages:
Additionally, since this change starts passing messages through exceptions, ensure your global exception handler does not echo raw library texts to API responses. Keep details in logs, return standardized messages to clients.
🏁 Script executed:
#!/bin/bash
# Find single-arg CoreException(ErrorType ...) calls
rg -nP --glob '!**/build/**' 'new\s+CoreException\s*\(\s*ErrorType\.[A-Z0-9_]+\s*\)'Length of output: 10013
TokenService: Sanitize JWT error messages
I’ve confirmed that CoreException still provides the single-arg constructor (there are numerous calls like new CoreException(ErrorType.XYZ) that compile successfully), so you don’t need to restore or re-add that overload.
However, you should avoid passing raw JWT exception messages back to clients. Please replace the e.getMessage() parameters in your catch blocks with clear, user-friendly text, and keep the underlying exception logged rather than exposed. For example in TokenService.java lines 88–91:
- } catch (ExpiredJwtException e) {
- throw new CoreException(ErrorType.REFRESH_TOKEN_EXPIRED, e.getMessage());
- } catch (SignatureException | MalformedJwtException | IllegalArgumentException e) {
- throw new CoreException(ErrorType.INVALID_REFRESH_TOKEN, e.getMessage());
+ } catch (ExpiredJwtException e) {
+ log.warn("Refresh token expired", e);
+ throw new CoreException(ErrorType.REFRESH_TOKEN_EXPIRED, "Refresh token has expired");
+ } catch (SignatureException | MalformedJwtException | IllegalArgumentException e) {
+ log.warn("Invalid refresh token", e);
+ throw new CoreException(ErrorType.INVALID_REFRESH_TOKEN, "Refresh token is invalid");• Apply similar sanitization wherever you currently use e.getMessage() for security-sensitive errors.
• Ensure your global exception handler returns only standardized error bodies to API clients while logging full details internally.
📝 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.
| throw new CoreException(ErrorType.REFRESH_TOKEN_EXPIRED, e.getMessage()); | |
| } catch (SignatureException | MalformedJwtException | IllegalArgumentException e) { | |
| throw new CoreException(ErrorType.INVALID_REFRESH_TOKEN); | |
| throw new CoreException(ErrorType.INVALID_REFRESH_TOKEN, e.getMessage()); | |
| } | |
| } catch (ExpiredJwtException e) { | |
| log.warn("Refresh token expired", e); | |
| throw new CoreException(ErrorType.REFRESH_TOKEN_EXPIRED, "Refresh token has expired"); | |
| } catch (SignatureException | MalformedJwtException | IllegalArgumentException e) { | |
| log.warn("Invalid refresh token", e); | |
| throw new CoreException(ErrorType.INVALID_REFRESH_TOKEN, "Refresh token is invalid"); | |
| } |
🤖 Prompt for AI Agents
In
capturecat-core/src/main/java/com/capturecat/core/service/auth/TokenService.java
around lines 88-91, replace the use of e.getMessage() in the catch blocks with
concise, user-friendly error messages (e.g., "Refresh token expired" and
"Invalid refresh token") and stop returning raw JWT exception text to clients;
instead log the full exception (stacktrace) internally using the logger and
construct CoreException using the ErrorType plus the sanitized message, and then
apply the same sanitization pattern wherever e.getMessage() is used for
security-sensitive exceptions and ensure the global exception handler only
returns standardized error bodies to API clients while preserving full details
in internal logs.
| } catch (CoreException e) { | ||
| throw e; | ||
| } catch (Exception e) { | ||
| log.error("[revokeUserTokens] Unexpected error", e); | ||
| throw new CoreException(ErrorType.INTERNAL_SERVER_ERROR); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Unexpected-error mapping: include a safe message and consider preserving the cause; also redact token logs elsewhere
Two points:
- If CoreException no longer has a 1-arg constructor, this throw will not compile. Even if it does, add a safe message to aid observability.
- You’re already logging the full exception; avoid duplicating sensitive details in the client response.
Suggested tweak:
- } catch (Exception e) {
- log.error("[revokeUserTokens] Unexpected error", e);
- throw new CoreException(ErrorType.INTERNAL_SERVER_ERROR);
+ } catch (Exception e) {
+ log.error("[revokeUserTokens] Unexpected error", e);
+ throw new CoreException(ErrorType.INTERNAL_SERVER_ERROR, "Unexpected error while revoking tokens");Security hygiene (outside these lines): you log full tokens at info level in parseRefreshToken (Line 75) and blacklistAccessToken (Line 155). Logging raw tokens is risky. Redact and downshift to debug:
// helper inside TokenService
private static String redact(String token) {
if (token == null) return "null";
int n = Math.min(token.length(), 10);
return token.substring(0, n) + "...";
}
// then replace
log.debug("Refresh Token: {}", redact(refreshToken));
log.debug("Blacklist Token: {}", redact(accessToken));You can also consider hashing tokens (e.g., SHA-256) when composing Redis keys to avoid storing raw tokens as key names.
To check for raw-token logging occurrences:
🏁 Script executed:
#!/bin/bash
rg -n -C1 'Refresh Token:|Blacklist Token:' Length of output: 743
🏁 Script executed:
#!/bin/bash
# Search for CoreException definition in the capturecat-core module
rg -n -C3 'class CoreException' capturecat-core/src/main/javaLength of output: 1699
🏁 Script executed:
#!/bin/bash
# Search for constructors in CoreException.java
rg -nP 'public CoreException\\(' capturecat-core/src/main/java/com/capturecat/core/support/error/CoreException.javaLength of output: 190
#!/bin/bash
# Correctly search for constructors in CoreException.java
rg -nP 'public\s+CoreException\s*\(' capturecat-core/src/main/java/com/capturecat/core/support/error/CoreException.java#!/bin/bash
# List constructor declarations in CoreException.java
rg -n 'public CoreException\(' capturecat-core/src/main/java/com/capturecat/core/support/error/CoreException.java#!/bin/bash
# Display the contents of CoreException.java to inspect available constructors
sed -n '1,200p' capturecat-core/src/main/java/com/capturecat/core/support/error/CoreException.java🏁 Script executed:
#!/bin/bash
# Display CoreException.java to inspect constructors
sed -n '1,200p' capturecat-core/src/main/java/com/capturecat/core/support/error/CoreException.javaLength of output: 672
Secure token logging & improve exception messaging
Two updates in TokenService.java:
-
Enhance exception mapping (lines 115–119)
Use the existing two-arg CoreException constructor to include a safe, descriptive message without exposing stack details to clients:} catch (Exception e) { log.error("[revokeUserTokens] Unexpected error", e); - throw new CoreException(ErrorType.INTERNAL_SERVER_ERROR); + throw new CoreException( + ErrorType.INTERNAL_SERVER_ERROR, + "Unexpected error while revoking tokens" + ); }• CoreException already defines
CoreException(ErrorType, String), so this compiles as-is.
• If you need to preserve the originalThrowablein the exception itself, consider adding a new constructor in CoreException that accepts a cause (e.g.,public CoreException(ErrorType, String, Throwable)chaining tosuper(message, cause)). -
Prevent raw-token leakage (lines 75 & 155)
Logging full tokens at info level risks leaking sensitive data. Define a helper to redact or truncate tokens, and shift these logs to debug:// helper inside TokenService private static String redact(String token) { if (token == null) return "null"; int n = Math.min(token.length(), 10); return token.substring(0, n) + "..."; } // around line 75 - log.info("Refresh Token: {}", refreshToken); + log.debug("Refresh Token: {}", redact(refreshToken)); // around line 155 - log.info("Blacklist Token: {}", accessToken); + log.debug("Blacklist Token: {}", redact(accessToken));
These changes improve observability with a safe client message and block sensitive token data from higher-level logs.
🤖 Prompt for AI Agents
In
capturecat-core/src/main/java/com/capturecat/core/service/auth/TokenService.java
around lines 115-119 and also addressing logs at ~75 and ~155: replace the
generic rethrow on unexpected Exception with the two-arg
CoreException(ErrorType, String) to provide a safe client-facing message (e.g.,
"Unexpected error revoking tokens") and optionally add a new CoreException
constructor that accepts a cause if you want to preserve the original throwable;
additionally, stop logging raw tokens at info level by adding a private helper
that redacts/truncates tokens (return "null" for null, first N chars + "..."
otherwise), change existing log.info(...) calls that print tokens at lines ~75
and ~155 to log.debug(...) and use the redact helper to avoid leaking sensitive
token data.
| LOGIN_FAIL(HttpStatus.BAD_REQUEST, ErrorCode.INVALID_REQUEST, LogLevel.WARN), | ||
| VALIDATION_FAIL(HttpStatus.BAD_REQUEST, ErrorCode.BEAN_VALIDATION_FAIL, LogLevel.WARN), | ||
| INVALID_JWT(HttpStatus.BAD_REQUEST, ErrorCode.BEAN_VALIDATION_FAIL, LogLevel.WARN), | ||
| INVALID_JWT(HttpStatus.UNAUTHORIZED, ErrorCode.BEAN_VALIDATION_FAIL, LogLevel.WARN), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potentially incorrect ErrorCode for INVALID_JWT (mapped to BEAN_VALIDATION_FAIL)
INVALID_JWT currently uses ErrorCode.BEAN_VALIDATION_FAIL. This seems semantically wrong and could mislead clients that rely on ErrorCode for UX or analytics.
Preferred fix (if the enum exists):
- INVALID_JWT(HttpStatus.UNAUTHORIZED, ErrorCode.BEAN_VALIDATION_FAIL, LogLevel.WARN),
+ INVALID_JWT(HttpStatus.UNAUTHORIZED, ErrorCode.INVALID_JWT, LogLevel.WARN),Fallback (if INVALID_JWT doesn’t exist in ErrorCode):
- INVALID_JWT(HttpStatus.UNAUTHORIZED, ErrorCode.BEAN_VALIDATION_FAIL, LogLevel.WARN),
+ INVALID_JWT(HttpStatus.UNAUTHORIZED, ErrorCode.INVALID_AUTH_TOKEN, LogLevel.WARN),Run this to verify available ErrorCode constants and current usages:
🏁 Script executed:
#!/bin/bash
rg -nP 'enum\s+ErrorCode|INVALID_.*JWT|BEAN_VALIDATION_FAIL' -C2Length of output: 3399
🏁 Script executed:
#!/bin/bash
# Print the first 200 lines of ErrorType enum for context
sed -n '1,200p' capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorType.java
echo "----"
# Print the first 200 lines of ErrorCode enum for context
sed -n '1,200p' capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorCode.java
echo "----"
# Search for occurrences of INVALID_JWT in the codebase
rg -n 'INVALID_JWT' .
echo "----"
# Search for occurrences of INVALID_AUTH_TOKEN in the codebase
rg -n 'INVALID_AUTH_TOKEN' .
echo "----"
# Search for occurrences of INVALID_TOKEN in the codebase
rg -n 'INVALID_TOKEN' .Length of output: 7110
Refactor INVALID_JWT to use an appropriate ErrorCode
Mapping INVALID_JWT to ErrorCode.BEAN_VALIDATION_FAIL is misleading—clients receiving this error will think there’s a validation issue with their input, not an expired or invalid JSON Web Token. Please update the enum to use a more semantically correct constant.
• In capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorType.java (around line 25):
- INVALID_JWT(HttpStatus.UNAUTHORIZED, ErrorCode.BEAN_VALIDATION_FAIL, LogLevel.WARN),
+ INVALID_JWT(HttpStatus.UNAUTHORIZED, ErrorCode.INVALID_TOKEN, LogLevel.WARN),• If you’d prefer a JWT-specific code, add to ErrorCode.java:
INVALID_JWT("유효하지 않은 JWT입니다."),and then map accordingly:
- INVALID_JWT(HttpStatus.UNAUTHORIZED, ErrorCode.BEAN_VALIDATION_FAIL, LogLevel.WARN),
+ INVALID_JWT(HttpStatus.UNAUTHORIZED, ErrorCode.INVALID_JWT, LogLevel.WARN),• Don’t forget to update any tests or error-handling logic that expect the old BEAN_VALIDATION_FAIL for this case.
🤖 Prompt for AI Agents
In
capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorType.java
around line 25, INVALID_JWT is incorrectly mapped to
ErrorCode.BEAN_VALIDATION_FAIL; change the mapping to a semantically correct
error code (e.g., ErrorCode.INVALID_JWT). If ErrorCode.INVALID_JWT does not
exist, add it to
capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorCode.java
(e.g., INVALID_JWT("유효하지 않은 JWT입니다.")) and then update the enum entry in
ErrorType to use that constant; finally, run and update any tests or
error-handling logic that assumed the previous BEAN_VALIDATION_FAIL mapping.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
capturecat-core/src/main/java/com/capturecat/core/api/auth/TokenController.java (1)
37-38: 401 Mapping Verified; Confirm Exception Detail Handling
- The HTTP status mapping for token-related errors in
ErrorType.java(lines 28–31, 36–38) all useHttpStatus.UNAUTHORIZEDas intended; the earlier verification script confirms consistent 401 responses.- In
capturecat-core/src/main/java/com/capturecat/core/support/response/ApiResponse.java(lines 19–21), theerror(ErrorType)overload constructs anApiResponsewith anullbody and wraps only theErrorTypeinErrorMessage, omittinge.getMessage().If you intend to surface sanitized exception details in your API responses:
- You can leverage the existing overload
error(ErrorType, Object... args)to pass formatted parameters into yourErrorMessage.- Or introduce a dedicated overload, e.g.
error(ErrorType, String detail), to include a safe message string alongside the error type.capturecat-core/src/main/java/com/capturecat/core/config/jwt/JwtUtil.java (1)
73-82: Propagating ExpiredJwtException is correct; broaden invalid-token handling and clarify API semantics.
- Keeping expiry as an exception and mapping it at the caller matches the new 401 behavior. Good.
- Consider also treating UnsupportedJwtException as invalid (returns false), which covers unsupported/changed algs.
- Minor nit: ExpiredJwtException import is unused in this class; safe to remove.
- Optional hardening: isAccessToken/isRefreshToken can NPE if "type" claim is missing. Invert equals to avoid NPE.
Apply these diffs:
@@ - } catch (SignatureException | MalformedJwtException | IllegalArgumentException e) { + } catch (SignatureException | MalformedJwtException | IllegalArgumentException | UnsupportedJwtException e) { return false; }Additional edits outside the selected lines:
@@ -import io.jsonwebtoken.ExpiredJwtException; +// import io.jsonwebtoken.ExpiredJwtException; // not used directly here; remove if not referenced elsewhere +import io.jsonwebtoken.UnsupportedJwtException;And to avoid possible NPEs when type claim is absent:
@@ - public boolean isAccessToken(String accessToken) { - return getTokenType(accessToken).equals(TokenType.ACCESS.name()); - } + public boolean isAccessToken(String accessToken) { + return TokenType.ACCESS.name().equals(getTokenType(accessToken)); + } @@ - public boolean isRefreshToken(String refreshToken) { - return getTokenType(refreshToken).equals(TokenType.REFRESH.name()); - } + public boolean isRefreshToken(String refreshToken) { + return TokenType.REFRESH.name().equals(getTokenType(refreshToken)); + }capturecat-core/src/main/java/com/capturecat/core/config/jwt/JwtFilter.java (2)
51-58: Unify error handling in-filter; also catch CoreException from resolveToken.Right now, a CoreException from resolveToken(authHeader) (Line 48) bypasses this try/catch and relies on global handlers, while invalid/expired tokens here are handled in-filter. For a single, predictable 401 path and fewer surprises in filter chains, include resolveToken and catch CoreException locally.
Also, the comment on Line 56 mentions “만료 시 …” but this branch handles generic invalid tokens; consider moving that comment to the ExpiredJwtException catch.
Proposed refactor:
@@ - String accessToken = jwtUtil.resolveToken(authHeader); // "Bearer " 이후 토큰 - - // 토큰 유효성 검사 - try { - if (!jwtUtil.isAccessToken(accessToken) - || !jwtUtil.isValid(accessToken) - || tokenService.isBlacklisted(accessToken)) { - //만료 시 client에 즉시 응답. client는 재발급 요청 수행. - rejectInvalidToken(response, ErrorType.INVALID_ACCESS_TOKEN); - return; - } - } catch (ExpiredJwtException e) { + try { + String accessToken = jwtUtil.resolveToken(authHeader); // "Bearer " 이후 토큰 + // 토큰 유효성 검사 + if (!jwtUtil.isAccessToken(accessToken) + || !jwtUtil.isValid(accessToken) + || tokenService.isBlacklisted(accessToken)) { + rejectInvalidToken(response, ErrorType.INVALID_ACCESS_TOKEN); + return; + } + // downstream needs username/role; consider parsing once and reusing claims if perf becomes a concern + request.setAttribute("ACCESS_TOKEN", accessToken); + } catch (ExpiredJwtException e) { + // 만료 시 client에 즉시 응답. client는 재발급 요청 수행. rejectInvalidToken(response, ErrorType.ACCESS_TOKEN_EXPIRED); return; + } catch (com.capturecat.core.support.error.CoreException e) { + rejectInvalidToken(response, e.getErrorType()); + return; }Optionally parse claims once to avoid repeated parsing (getUsername/getRole/isAccessToken/isValid each parse). If desired, we can draft that refactor next.
75-79: Set UTF-8 charset (Korean messages) and consider WWW-Authenticate for Bearer.Error bodies contain Korean; set explicit UTF-8 to avoid client mis-decoding. Adding WWW-Authenticate improves interoperability for Bearer flows.
private void rejectInvalidToken(HttpServletResponse response, ErrorType errorType) throws IOException { response.setStatus(HttpStatus.UNAUTHORIZED.value()); - response.setContentType(MediaType.APPLICATION_JSON_VALUE); + response.setContentType(MediaType.APPLICATION_JSON_VALUE); + response.setCharacterEncoding(java.nio.charset.StandardCharsets.UTF_8.name()); + // Optional: advertise Bearer auth error context per RFC 6750 + response.setHeader(HttpHeaders.WWW_AUTHENTICATE, "Bearer error=\"invalid_token\", error_description=\"" + errorType.name() + "\""); objectMapper.writeValue(response.getWriter(), ApiResponse.error(errorType)); }If you prefer to keep headers minimal, feel free to skip the WWW-Authenticate line.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
capturecat-core/src/main/java/com/capturecat/core/api/auth/TokenController.java(1 hunks)capturecat-core/src/main/java/com/capturecat/core/config/jwt/JwtFilter.java(2 hunks)capturecat-core/src/main/java/com/capturecat/core/config/jwt/JwtUtil.java(1 hunks)capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorCode.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-dev
🔇 Additional comments (1)
capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorCode.java (1)
23-24: LGTM: clearer client guidance on expired tokens.The updated messages explicitly direct the client to reissue or re-login, matching the new 401 semantics. Ensure clients don’t parse these human messages for logic; they should key off ErrorType or HTTP status.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
capturecat-core/src/test/java/com/capturecat/core/api/auth/TokenApiTest.java (2)
91-94: Rename test to reflect 401 (consistency/readability).The method name still says returnsBadRequest while we now expect 401. Rename to avoid confusion for future readers.
- @DisplayName("Refresh token 재발급 실패 - 잘못된 Refresh token으로 요청할 경우 에러 응답") - void reissue_withInvalidRefreshToken_returnsBadRequest() throws Exception { + @DisplayName("Refresh token 재발급 실패 - 잘못된 Refresh token 요청 시 401 Unauthorized 반환") + void reissue_withInvalidRefreshToken_returnsUnauthorized() throws Exception {
93-98: Strengthen negative assertions: ensure no auth headers are leaked on error.On failure responses, asserting that neither Authorization nor Refresh headers are present helps prevent accidental token issuance on error paths.
.andExpect(status().isUnauthorized()) + .andExpect(header().doesNotExist(HttpHeaders.AUTHORIZATION)) + .andExpect(header().doesNotExist(JwtUtil.REFRESH_TOKEN_HEADER)) .andExpect(jsonPath("$.result").value("ERROR")) .andExpect(jsonPath("$.error").exists());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
capturecat-core/src/test/java/com/capturecat/core/api/auth/TokenApiTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-dev
🔇 Additional comments (1)
capturecat-core/src/test/java/com/capturecat/core/api/auth/TokenApiTest.java (1)
95-95: Expectation updated to 401 — matches PR objective.Good change. Asserting Unauthorized here aligns the test with the new ErrorType-to-HTTP-status mapping and the client’s expectation of 401s for token problems.
📌 관련 이슈 (필수)
📝 작업 내용 (필수)
💬 리뷰 참고 사항 (선택)
Summary by CodeRabbit
Bug Fixes
Documentation