refactor: 토큰을 Authorization 헤더가 아니라 쿼리스트링에서 가져오도록 #467
Conversation
- 인터셉터는 인증된 Principal 객체를 세션에 담는데 집중한다. - 토큰 필터가 인증 작업을 처리한다.
whqtker
requested review from
Gyuhyeok99,
lsy1307,
nayonsoso and
wibaek
as code owners
|
Caution Review failedThe pull request is closed. Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
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. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ 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: 1
🧹 Nitpick comments (4)
src/main/java/com/example/solidconnection/security/filter/TokenAuthenticationFilter.java (1)
45-45: 3) 경로 매칭의 startsWith는 컨텍스트 패스/프록시 환경에서 취약할 수 있어요.
애플리케이션 컨텍스트 패스가 존재하거나 게이트웨이에서 프리픽스가 붙는 경우, getRequestURI().startsWith("/connect")는 빗나갈 수 있습니다. getServletPath() 사용 또는 컨텍스트 패스를 고려한 비교를 권장합니다.가능한 변경 예시(아이디어):
- if (request.getRequestURI().startsWith("/connect")) { + final String path = request.getServletPath() != null ? request.getServletPath() : request.getRequestURI(); + if (path != null && path.startsWith("/connect")) { ... }또는 Spring Security 설정과 동일한 PathPattern/AntMatcher 기반 유틸리티로 일원화해도 좋아요.
src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java (3)
71-71: 5) 토큰을 URL에 붙일 때는 안전하게 인코딩해 주세요.
JWT는 대체로 base64url이라 문제 없지만, 일반화와 안전성을 위해 URI 빌더로 쿼리 파라미터를 조립하는 편이 좋습니다.아래처럼 교체를 제안해요.
- String tokenUrl = url + "?token=" + accessToken.token(); + String tokenUrl = org.springframework.web.util.UriComponentsBuilder + .fromUriString(url) + .queryParam("token", accessToken.token()) + .build() + .toUriString();추가로 필요 시 import:
- import org.springframework.web.util.UriComponentsBuilder;
85-85: 6) 타임유닛 표기를 통일하면 읽기성이 좋아져요.
위 테스트는 SECONDS와 TimeUnit.SECONDS를 혼용합니다. 하나로 맞추는 것을 권장합니다.아래처럼 정리 가능합니다.
- }).get(5, TimeUnit.SECONDS); + }).get(5, SECONDS);그리고 사용되지 않게 되는 import를 제거해 주세요.
- import java.util.concurrent.TimeUnit;
82-87: 8) 실패 케이스 보강 제안 — 빈/공백 토큰 등 경계값도 확인해요.
현재는 무토큰만 검증합니다. 빈 문자열(""), 공백(" "), 잘못된 서명 토큰 등 경계값도 401로 일관되게 떨어지는지 테스트를 추가하면 회귀를 줄일 수 있어요.원하시면 해당 케이스를 포함한 파라미터라이즈드 테스트 템플릿까지 만들어 드릴게요.
📜 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 by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/java/com/example/solidconnection/chat/config/WebSocketHandshakeInterceptor.java(1 hunks)src/main/java/com/example/solidconnection/security/filter/TokenAuthenticationFilter.java(2 hunks)src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-23T18:07:08.348Z
Learnt from: nayonsoso
PR: solid-connection/solid-connect-server#400
File: src/main/java/com/example/solidconnection/chat/config/StompWebSocketConfig.java:20-22
Timestamp: 2025-07-23T18:07:08.348Z
Learning: SockJS configuration should be added to STOMP endpoints using .withSockJS() for better browser compatibility and network environment support, even if frontend doesn't initially use SockJS client libraries.
Applied to files:
src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java
🧬 Code Graph Analysis (1)
src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java (1)
src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java (1)
TestContainerSpringBootTest(17-108)
⏰ 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: build
🔇 Additional comments (3)
src/main/java/com/example/solidconnection/chat/config/WebSocketHandshakeInterceptor.java (1)
24-24: 1) 핸드셰이크 무조건 허용으로 변경 — 보안 경계 동작 재확인이 필요해요.
이제 beforeHandshake가 항상 true를 반환하여 비인증 사용자도 핸드셰이크 단계는 통과합니다. 의도대로라면 Spring Security가 /connect 계층에서 인증 실패를 401로 처리해야 합니다. 테스트가 이를 검증하지만, 다른 엔드포인트나 프록시/필터 체인에서도 동일하게 동작하는지 한 번 더 점검해 주세요.
- 체크리스트:
- SecurityConfig에서 /connect/** 경로가 반드시 인증 요구인지 확인해요.
- SockJS의 /connect/info, /connect/xxx 서브 경로까지 일관되게 보호되는지 확인해요.
- 무효 토큰, 빈 토큰, 토큰 미전달의 세 케이스 모두 401이 나는지 확인해요.
src/main/java/com/example/solidconnection/security/filter/TokenAuthenticationFilter.java (1)
44-49: 쿼리스트링 토큰 노출 방지를 위한 운영 보호장치 적용 제안
쿼리스트링으로 전달되는 토큰은 접근 로그, 애플리케이션 로그, 브라우저 리퍼러, 프록시 로그 등을 통해 쉽게 노출될 수 있어요.
SockJS 제약으로 인해 쿼리스트링을 사용하는 것은 이해하지만, 운영 환경에서는 토큰 노출을 최소화하기 위해 다음 보호장치를 적용하는 것을 권장합니다.
- Access Log에서 쿼리스트링 미기록
• Tomcat 접근 로그 패턴에서%q제거- 애플리케이션 로그에서 queryString 마스킹
• CommonsRequestLoggingFilter 또는 Logback 마스커 필터 사용- 브라우저 Referrer-Policy 헤더 설정
•Referrer-Policy: no-referrer적용- 리버스 프록시(Nginx/ALB) 액세스 로그 쿼리 제거/마스킹
• 프록시 설정 파일에서$request_uri대신$uri등 사용코드베이스 내 application.yml/.properties 등에 위 설정이 아직 확인되지 않습니다. 수동으로 설정 파일을 검토해 주시고, 필요 시 다음 스크립트로 추가 검증 부탁드립니다.
rg -n -C2 -g '!**/build/**' -g '!**/target/**' \ 'server\.tomcat\.accesslog|Referrer-Policy|requestLoggingFilter|mask|redact|queryString' \ --type-add 'yml:*.yml' --type-add 'properties:*.properties'src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java (1)
74-78: 7) URL 파라미터 방식으로의 전환이 테스트에도 잘 반영되었습니다.
토큰을 URL로 전달하고 isConnected로 성공을 검증하는 흐름이 목적에 부합합니다. 간결하고 명확합니다.
src/main/java/com/example/solidconnection/security/filter/TokenAuthenticationFilter.java
Show resolved
Hide resolved
3 participants
관련 이슈
작업 내용
SockJS로 WebSocket 핸드셰이킹을 진행하는 경우 해당 HTTP 요청의 Authorization 헤더에 토큰을 포함시키는 기능을 지원하지 않습니다. 따라서 기존 방법을 사용하면 인증된 사용자 정보를 가져올 수 없게 됩니다.
토큰을 쿼리스트링을 통해 받도록 변경합니다.
[관련 논의]
특이 사항
리뷰 요구사항 (선택)