#98 URL을 통해 비밀번호가 걸려있는 방 접근을 막기 위한 jwt 토큰 로직 추가 #100
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's Guide전용 JWT 기반 방 입장 토큰 플로우를 추가하여, 비밀번호 입력 시 단기(Short‑lived) 토큰을 발급하고 이후 방 재입장 시에는 URL 직접 접근을 신뢰하지 않고 이 토큰을 검증하도록 변경했으며, 이에 맞춰 백엔드, 프론트엔드, CORS 및 에러 처리 로직을 수정합니다. JWT 기반 방 입장 플로우 시퀀스 다이어그램sequenceDiagram
actor User
participant Browser
participant FrontendApp as NodeJS_Frontend
participant ChatRoomController
participant ChatRoomService
participant JwtRoomProvider
participant JwtCoreProvider
participant RedisService
participant ExceptionController
rect rgb(230,230,250)
User->>Browser: Open room password popup
Browser->>FrontendApp: Submit roomId and roomPwd
FrontendApp->>ChatRoomController: POST /api/room/{roomId}/validatePwd
ChatRoomController->>ChatRoomService: validatePwd(email, roomId, roomPwd)
ChatRoomService->>RedisService: getRedisDataByDataType(roomId, CHATROOM)
RedisService-->>ChatRoomService: ChatRoom
ChatRoomService->>ChatRoomService: Check password and user count
alt Password and user count valid
ChatRoomService->>JwtRoomProvider: create(roomId, email)
JwtRoomProvider->>JwtCoreProvider: create(subject=email, claims, expireMs)
JwtCoreProvider-->>JwtRoomProvider: jwtToken
JwtRoomProvider-->>ChatRoomService: jwtToken
ChatRoomService-->>ChatRoomController: {isValidate=true, token}
ChatRoomController-->>FrontendApp: Response with token
FrontendApp->>Browser: Store token in sessionStorage.roomAccessToken
else Invalid password or room full
ChatRoomService-->>ChatRoomController: {isValidate=false}
ChatRoomController-->>FrontendApp: Response without token
FrontendApp->>User: Show error toast
end
end
rect rgb(220,245,220)
User->>Browser: Click join room (URL or button)
Browser->>FrontendApp: Request to join
FrontendApp->>Browser: Read sessionStorage.roomAccessToken
FrontendApp->>ChatRoomController: GET /api/room/{roomId}/join
Note over FrontendApp,ChatRoomController: Sends headers Authorization and X-Room-Token
ChatRoomController->>ChatRoomService: findRoomById(roomId)
ChatRoomService->>RedisService: getRedisDataByDataType(roomId, CHATROOM)
RedisService-->>ChatRoomService: ChatRoom
ChatRoomService-->>ChatRoomController: ChatRoom
alt Room is secretChk = true
ChatRoomController->>JwtRoomProvider: validate(roomToken, roomId)
JwtRoomProvider->>JwtCoreProvider: parse(token)
JwtCoreProvider-->>JwtRoomProvider: Claims
JwtRoomProvider->>JwtRoomProvider: Verify type ROOM_ACCESS and roomId
alt Token invalid
JwtRoomProvider-->>ChatRoomController: throw InvalidRoomAccessException
ChatRoomController->>ExceptionController: InvalidRoomAccessException
ExceptionController-->>FrontendApp: 400 {code:40061}
FrontendApp->>Browser: Redirect to roomlist.html
else Token valid
JwtRoomProvider-->>ChatRoomController: success
ChatRoomController->>FrontendApp: Join room response
FrontendApp->>User: Enter room
end
else Room not secretChk
ChatRoomController->>FrontendApp: Join room without token validation
end
end
잘못된 방 입장 토큰에 대한 에러 처리 시퀀스 다이어그램sequenceDiagram
participant FrontendApp as NodeJS_Frontend
participant ChatRoomController
participant JwtRoomProvider
participant JwtCoreProvider
participant ExceptionController
FrontendApp->>ChatRoomController: GET /api/room/{roomId}/join with invalid or missing X-Room-Token
ChatRoomController->>JwtRoomProvider: validate(token, roomId)
alt Token null or empty
JwtRoomProvider-->>ChatRoomController: throw InvalidRoomAccessException
else Token present but invalid
JwtRoomProvider->>JwtCoreProvider: parse(token)
JwtCoreProvider-->>JwtRoomProvider: Exception during parse or wrong claims
JwtRoomProvider-->>ChatRoomController: throw InvalidRoomAccessException
end
ChatRoomController->>ExceptionController: InvalidRoomAccessException
ExceptionController-->>FrontendApp: 400 {code:40061, message:Invalid or missing room access token}
FrontendApp->>FrontendApp: Detect code 40061
FrontendApp->>FrontendApp: Alert user and redirect to roomlist.html
JWT 방 입장 및 관련 서비스에 대한 업데이트된 클래스 다이어그램classDiagram
class ChatRoomService {
- RedisService redisService
- ChatKafkaProducer chatKafkaProducer
- JwtRoomProvider jwtRoomProvider
+ Map validatePwd(String email, String roomId, String roomPwd)
}
class ChatRoomController {
- ChatRoomService chatRoomService
- RoutingInstanceProvider instanceProvider
- RedisService redisService
- UserService userService
- JwtRoomProvider jwtRoomProvider
+ ResponseEntity createRoom(...)
+ ResponseEntity joinRoom(String roomId, String authorization, String roomToken, HttpServletRequest request, HttpServletResponse response)
+ ResponseEntity validatePwd(String roomId, String roomPwd)
}
class ExceptionController {
+ Map handleInvalidRoomAccess()
}
class InvalidRoomAccessException {
+ InvalidRoomAccessException(String message)
}
ExceptionController <|-- InvalidRoomAccessException
class JwtRoomProvider {
- long EXPIRE_MS
- JwtCoreProvider jwtCore
+ JwtRoomProvider(Key key)
+ String create(String roomId, String userId)
+ void validate(String token, String roomId)
}
class JwtCoreProvider {
- Key key
+ JwtCoreProvider(Key key)
+ String create(String subject, Map claims, long expireMs)
+ Claims parse(String token)
}
class JwtKeyConfig {
+ Key roomJwtKey(String secret)
}
class JwtTokenType {
<<enumeration>>
ROOM_ACCESS
}
class ChatRoom {
+ String roomId
+ String roomPwd
+ int userCount
+ int maxUserCnt
+ boolean secretChk
}
class RedisService {
+ ChatRoom getRedisDataByDataType(String key, DataType dataType, Class clazz)
}
class DataType {
<<enumeration>>
CHATROOM
}
class UserService
class RoutingInstanceProvider
class ChatKafkaProducer
ChatRoomService --> JwtRoomProvider : uses
ChatRoomService --> RedisService : reads ChatRoom
ChatRoomService --> ChatRoom : validates password
ChatRoomController --> ChatRoomService : service calls
ChatRoomController --> JwtRoomProvider : validate room token
ChatRoomController --> UserService : get oauth info
ChatRoomController --> RoutingInstanceProvider
JwtRoomProvider --> JwtCoreProvider : delegates
JwtRoomProvider --> JwtTokenType : uses ROOM_ACCESS
JwtCoreProvider --> JwtKeyConfig : gets Key bean
JwtKeyConfig --> JwtRoomProvider : provides roomJwtKey
ExceptionController <.. ChatRoomController : handles InvalidRoomAccessException
InvalidRoomAccessException --> ExceptionController : inner static
ChatRoomService --> ChatKafkaProducer
파일 단위 변경 사항
연결 가능성이 있는 이슈
Tips and commandsInteracting with Sourcery
Customizing Your Experiencedashboard에서 다음을 설정할 수 있습니다:
Getting Help
Original review guide in EnglishReviewer's GuideAdds a dedicated JWT-based room access token flow so that entering a password grants a short‑lived token, and subsequent room joins validate this token instead of trusting direct URL access, with corresponding backend, frontend, CORS, and error handling changes. Sequence diagram for JWT-based room access flowsequenceDiagram
actor User
participant Browser
participant FrontendApp as NodeJS_Frontend
participant ChatRoomController
participant ChatRoomService
participant JwtRoomProvider
participant JwtCoreProvider
participant RedisService
participant ExceptionController
rect rgb(230,230,250)
User->>Browser: Open room password popup
Browser->>FrontendApp: Submit roomId and roomPwd
FrontendApp->>ChatRoomController: POST /api/room/{roomId}/validatePwd
ChatRoomController->>ChatRoomService: validatePwd(email, roomId, roomPwd)
ChatRoomService->>RedisService: getRedisDataByDataType(roomId, CHATROOM)
RedisService-->>ChatRoomService: ChatRoom
ChatRoomService->>ChatRoomService: Check password and user count
alt Password and user count valid
ChatRoomService->>JwtRoomProvider: create(roomId, email)
JwtRoomProvider->>JwtCoreProvider: create(subject=email, claims, expireMs)
JwtCoreProvider-->>JwtRoomProvider: jwtToken
JwtRoomProvider-->>ChatRoomService: jwtToken
ChatRoomService-->>ChatRoomController: {isValidate=true, token}
ChatRoomController-->>FrontendApp: Response with token
FrontendApp->>Browser: Store token in sessionStorage.roomAccessToken
else Invalid password or room full
ChatRoomService-->>ChatRoomController: {isValidate=false}
ChatRoomController-->>FrontendApp: Response without token
FrontendApp->>User: Show error toast
end
end
rect rgb(220,245,220)
User->>Browser: Click join room (URL or button)
Browser->>FrontendApp: Request to join
FrontendApp->>Browser: Read sessionStorage.roomAccessToken
FrontendApp->>ChatRoomController: GET /api/room/{roomId}/join
Note over FrontendApp,ChatRoomController: Sends headers Authorization and X-Room-Token
ChatRoomController->>ChatRoomService: findRoomById(roomId)
ChatRoomService->>RedisService: getRedisDataByDataType(roomId, CHATROOM)
RedisService-->>ChatRoomService: ChatRoom
ChatRoomService-->>ChatRoomController: ChatRoom
alt Room is secretChk = true
ChatRoomController->>JwtRoomProvider: validate(roomToken, roomId)
JwtRoomProvider->>JwtCoreProvider: parse(token)
JwtCoreProvider-->>JwtRoomProvider: Claims
JwtRoomProvider->>JwtRoomProvider: Verify type ROOM_ACCESS and roomId
alt Token invalid
JwtRoomProvider-->>ChatRoomController: throw InvalidRoomAccessException
ChatRoomController->>ExceptionController: InvalidRoomAccessException
ExceptionController-->>FrontendApp: 400 {code:40061}
FrontendApp->>Browser: Redirect to roomlist.html
else Token valid
JwtRoomProvider-->>ChatRoomController: success
ChatRoomController->>FrontendApp: Join room response
FrontendApp->>User: Enter room
end
else Room not secretChk
ChatRoomController->>FrontendApp: Join room without token validation
end
end
Sequence diagram for error handling on invalid room access tokensequenceDiagram
participant FrontendApp as NodeJS_Frontend
participant ChatRoomController
participant JwtRoomProvider
participant JwtCoreProvider
participant ExceptionController
FrontendApp->>ChatRoomController: GET /api/room/{roomId}/join with invalid or missing X-Room-Token
ChatRoomController->>JwtRoomProvider: validate(token, roomId)
alt Token null or empty
JwtRoomProvider-->>ChatRoomController: throw InvalidRoomAccessException
else Token present but invalid
JwtRoomProvider->>JwtCoreProvider: parse(token)
JwtCoreProvider-->>JwtRoomProvider: Exception during parse or wrong claims
JwtRoomProvider-->>ChatRoomController: throw InvalidRoomAccessException
end
ChatRoomController->>ExceptionController: InvalidRoomAccessException
ExceptionController-->>FrontendApp: 400 {code:40061, message:Invalid or missing room access token}
FrontendApp->>FrontendApp: Detect code 40061
FrontendApp->>FrontendApp: Alert user and redirect to roomlist.html
Updated class diagram for JWT room access and related servicesclassDiagram
class ChatRoomService {
- RedisService redisService
- ChatKafkaProducer chatKafkaProducer
- JwtRoomProvider jwtRoomProvider
+ Map validatePwd(String email, String roomId, String roomPwd)
}
class ChatRoomController {
- ChatRoomService chatRoomService
- RoutingInstanceProvider instanceProvider
- RedisService redisService
- UserService userService
- JwtRoomProvider jwtRoomProvider
+ ResponseEntity createRoom(...)
+ ResponseEntity joinRoom(String roomId, String authorization, String roomToken, HttpServletRequest request, HttpServletResponse response)
+ ResponseEntity validatePwd(String roomId, String roomPwd)
}
class ExceptionController {
+ Map handleInvalidRoomAccess()
}
class InvalidRoomAccessException {
+ InvalidRoomAccessException(String message)
}
ExceptionController <|-- InvalidRoomAccessException
class JwtRoomProvider {
- long EXPIRE_MS
- JwtCoreProvider jwtCore
+ JwtRoomProvider(Key key)
+ String create(String roomId, String userId)
+ void validate(String token, String roomId)
}
class JwtCoreProvider {
- Key key
+ JwtCoreProvider(Key key)
+ String create(String subject, Map claims, long expireMs)
+ Claims parse(String token)
}
class JwtKeyConfig {
+ Key roomJwtKey(String secret)
}
class JwtTokenType {
<<enumeration>>
ROOM_ACCESS
}
class ChatRoom {
+ String roomId
+ String roomPwd
+ int userCount
+ int maxUserCnt
+ boolean secretChk
}
class RedisService {
+ ChatRoom getRedisDataByDataType(String key, DataType dataType, Class clazz)
}
class DataType {
<<enumeration>>
CHATROOM
}
class UserService
class RoutingInstanceProvider
class ChatKafkaProducer
ChatRoomService --> JwtRoomProvider : uses
ChatRoomService --> RedisService : reads ChatRoom
ChatRoomService --> ChatRoom : validates password
ChatRoomController --> ChatRoomService : service calls
ChatRoomController --> JwtRoomProvider : validate room token
ChatRoomController --> UserService : get oauth info
ChatRoomController --> RoutingInstanceProvider
JwtRoomProvider --> JwtCoreProvider : delegates
JwtRoomProvider --> JwtTokenType : uses ROOM_ACCESS
JwtCoreProvider --> JwtKeyConfig : gets Key bean
JwtKeyConfig --> JwtRoomProvider : provides roomJwtKey
ExceptionController <.. ChatRoomController : handles InvalidRoomAccessException
InvalidRoomAccessException --> ExceptionController : inner static
ChatRoomService --> ChatKafkaProducer
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 4개의 이슈를 발견했고, 몇 가지 상위 수준 피드백을 남깁니다:
- JwtCoreProvider는 @component로 애노테이션되어 있지만 JwtRoomProvider는 Spring을 거치지 않고
new로 직접 인스턴스화하고 있습니다. 이는 일관성이 없고 Spring 빈 와이어링과 충돌할 수 있습니다. JwtCoreProvider에서 @component를 제거하고 단순 유틸리티로 취급하거나, 적절한 key/qualifier를 사용해 공유 JwtCoreProvider 빈을 주입하는 방식 중 하나로 정리하는 것을 고려해 주세요. - ChatRoomService.validatePwd가 이제 원시 타입의
Map<String, Object>를 반환하는데, 이로 인해 백엔드와 프론트엔드 사이의 계약이 불투명하고 깨지기 쉬워졌습니다.isValidate,token필드를 가진 작은 DTO를 도입하면 API가 더 명확해지고 타입 안정성이 높아질 것입니다. - 프론트엔드에서
roomAccessToken이 sessionStorage에 전역으로 저장되고tokenAjax에서 모든 요청에 자동으로 첨부되고 있지만, 방을 떠날 때 이 값이 지워지지 않는 것으로 보입니다. 헤더 사용 범위를 방 입장 관련 호출로만 한정하고, 사용자가 방에서 나갈 때 토큰을 명시적으로 제거하는 것을 고려해 주세요.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- JwtCoreProvider is annotated as a @Component but JwtRoomProvider bypasses Spring and instantiates it with `new`, which is inconsistent and may conflict with Spring’s bean wiring; consider either removing @Component from JwtCoreProvider and treating it as a plain utility or injecting a shared JwtCoreProvider bean with the appropriate key/qualifier.
- ChatRoomService.validatePwd now returns a raw `Map<String, Object>` which makes the contract between backend and frontend opaque and brittle; introducing a small DTO (e.g., with `isValidate` and `token` fields) would make the API clearer and type-safe.
- On the frontend, `roomAccessToken` is stored globally in sessionStorage and automatically attached in `tokenAjax` for all requests, but it does not appear to be cleared when leaving a room; consider scoping the header usage only to room-join related calls and explicitly removing the token when the user exits a room.
## Individual Comments
### Comment 1
<location> `springboot-backend/src/main/java/webChat/security/jwt/core/JwtCoreProvider.java:31-35` </location>
<code_context>
+ .compact();
+ }
+
+ public Claims parse(String token) {
+ return Jwts.parser()
+ .setSigningKey(key)
+ .build()
+ .parseClaimsJws(token)
+ .getBody();
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** The JJWT parser usage (`Jwts.parser().build()`) looks inconsistent with current library APIs and may not compile.
Please confirm the jjwt version in use. For jjwt 0.11+ this should be updated to `Jwts.parserBuilder().setSigningKey(key).build().parseClaimsJws(token)` rather than relying on `parser()`, which no longer supports `.build()` and will not compile.
</issue_to_address>
### Comment 2
<location> `springboot-backend/src/main/java/webChat/security/jwt/core/JwtCoreProvider.java:12-17` </location>
<code_context>
+import java.util.HashMap;
+import java.util.Map;
+
+@Component
+public class JwtRoomProvider {
+ private static final long EXPIRE_MS = 1000L * 60 * 30;
</code_context>
<issue_to_address>
**suggestion:** JwtCoreProvider is annotated as a Spring component but is instantiated manually, which can be misleading.
Since JwtRoomProvider calls `new JwtCoreProvider(key)` directly, the `@Component` on JwtCoreProvider is effectively unused and suggests it’s managed by Spring when it isn’t. Please either remove `@Component` or refactor to inject JwtCoreProvider as a Spring bean with the key provided via configuration.
</issue_to_address>
### Comment 3
<location> `springboot-backend/src/main/java/webChat/security/jwt/JwtRoomProvider.java:38-55` </location>
<code_context>
+ return jwtCore.create(userId, claims, EXPIRE_MS);
+ }
+
+ public void validate(String token, String roomId) throws ExceptionController.InvalidRoomAccessException {
+ try {
+ if (StringUtil.isNullOrEmpty(token)) {
+ throw new ExceptionController.InvalidRoomAccessException("Invalid access");
+ }
+ Claims claims = jwtCore.parse(token);
+
+ if (!JwtTokenType.ROOM_ACCESS.name().equals(claims.get("type"))) {
+ throw new ExceptionController.InvalidRoomAccessException("Invalid room access info");
+ }
+
+ String tokenRoomId = claims.get("roomId", String.class);
+ if (!roomId.equals(tokenRoomId)) {
+ throw new ExceptionController.InvalidRoomAccessException("Invalid room access info");
+ }
+ } catch (Exception e) {
+ throw new ExceptionController.InvalidRoomAccessException("Invalid access");
+ }
</code_context>
<issue_to_address>
**suggestion:** The broad `catch (Exception)` rewraps `InvalidRoomAccessException`, losing the more specific failure reasons.
The inner `InvalidRoomAccessException` branches end up unused because the outer `catch (Exception e)` always wraps them as a generic "Invalid access". If you need the specific messages, either rethrow `InvalidRoomAccessException` unchanged or narrow the catch to only parsing/runtime exceptions so existing `InvalidRoomAccessException`s can bubble up.
```suggestion
public void validate(String token, String roomId) throws ExceptionController.InvalidRoomAccessException {
if (StringUtil.isNullOrEmpty(token)) {
throw new ExceptionController.InvalidRoomAccessException("Invalid access");
}
try {
Claims claims = jwtCore.parse(token);
if (!JwtTokenType.ROOM_ACCESS.name().equals(claims.get("type"))) {
throw new ExceptionController.InvalidRoomAccessException("Invalid room access info");
}
String tokenRoomId = claims.get("roomId", String.class);
if (!roomId.equals(tokenRoomId)) {
throw new ExceptionController.InvalidRoomAccessException("Invalid room access info");
}
} catch (RuntimeException e) {
// Token parsing or claim access failed: treat as generic invalid access
throw new ExceptionController.InvalidRoomAccessException("Invalid access");
}
```
</issue_to_address>
### Comment 4
<location> `nodejs-frontend/static/js/popup/room_popup.js:195-198` </location>
<code_context>
let successCallback = function(result) {
- if (result?.data && result?.result === 'success') {
+ if (result?.data?.isValidate === true) {
+ var roomToken = result.data.token;
+ sessionStorage.setItem('roomAccessToken', roomToken);
self.showToast('방에 정상적으로 입장했습니다!', 'success');
$('#enterRoomModal').modal('hide');
</code_context>
<issue_to_address>
**issue (bug_risk):** The password validation success handler no longer shows feedback when the password is wrong.
This change removed the `result?.result === 'success'` check and now only handles `result?.data?.isValidate === true`. When `isValidate` is `false` (wrong password/over capacity), this callback does nothing, so the user gets no error and the modal stays open. Add an `else` branch to show an error toast (e.g. as in `room_settings_popup.js`) or reuse existing invalid‑password error handling so failures are surfaced to the user.
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 4 issues, and left some high level feedback:
- JwtCoreProvider is annotated as a @component but JwtRoomProvider bypasses Spring and instantiates it with
new, which is inconsistent and may conflict with Spring’s bean wiring; consider either removing @component from JwtCoreProvider and treating it as a plain utility or injecting a shared JwtCoreProvider bean with the appropriate key/qualifier. - ChatRoomService.validatePwd now returns a raw
Map<String, Object>which makes the contract between backend and frontend opaque and brittle; introducing a small DTO (e.g., withisValidateandtokenfields) would make the API clearer and type-safe. - On the frontend,
roomAccessTokenis stored globally in sessionStorage and automatically attached intokenAjaxfor all requests, but it does not appear to be cleared when leaving a room; consider scoping the header usage only to room-join related calls and explicitly removing the token when the user exits a room.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- JwtCoreProvider is annotated as a @Component but JwtRoomProvider bypasses Spring and instantiates it with `new`, which is inconsistent and may conflict with Spring’s bean wiring; consider either removing @Component from JwtCoreProvider and treating it as a plain utility or injecting a shared JwtCoreProvider bean with the appropriate key/qualifier.
- ChatRoomService.validatePwd now returns a raw `Map<String, Object>` which makes the contract between backend and frontend opaque and brittle; introducing a small DTO (e.g., with `isValidate` and `token` fields) would make the API clearer and type-safe.
- On the frontend, `roomAccessToken` is stored globally in sessionStorage and automatically attached in `tokenAjax` for all requests, but it does not appear to be cleared when leaving a room; consider scoping the header usage only to room-join related calls and explicitly removing the token when the user exits a room.
## Individual Comments
### Comment 1
<location> `springboot-backend/src/main/java/webChat/security/jwt/core/JwtCoreProvider.java:31-35` </location>
<code_context>
+ .compact();
+ }
+
+ public Claims parse(String token) {
+ return Jwts.parser()
+ .setSigningKey(key)
+ .build()
+ .parseClaimsJws(token)
+ .getBody();
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** The JJWT parser usage (`Jwts.parser().build()`) looks inconsistent with current library APIs and may not compile.
Please confirm the jjwt version in use. For jjwt 0.11+ this should be updated to `Jwts.parserBuilder().setSigningKey(key).build().parseClaimsJws(token)` rather than relying on `parser()`, which no longer supports `.build()` and will not compile.
</issue_to_address>
### Comment 2
<location> `springboot-backend/src/main/java/webChat/security/jwt/core/JwtCoreProvider.java:12-17` </location>
<code_context>
+import java.util.HashMap;
+import java.util.Map;
+
+@Component
+public class JwtRoomProvider {
+ private static final long EXPIRE_MS = 1000L * 60 * 30;
</code_context>
<issue_to_address>
**suggestion:** JwtCoreProvider is annotated as a Spring component but is instantiated manually, which can be misleading.
Since JwtRoomProvider calls `new JwtCoreProvider(key)` directly, the `@Component` on JwtCoreProvider is effectively unused and suggests it’s managed by Spring when it isn’t. Please either remove `@Component` or refactor to inject JwtCoreProvider as a Spring bean with the key provided via configuration.
</issue_to_address>
### Comment 3
<location> `springboot-backend/src/main/java/webChat/security/jwt/JwtRoomProvider.java:38-55` </location>
<code_context>
+ return jwtCore.create(userId, claims, EXPIRE_MS);
+ }
+
+ public void validate(String token, String roomId) throws ExceptionController.InvalidRoomAccessException {
+ try {
+ if (StringUtil.isNullOrEmpty(token)) {
+ throw new ExceptionController.InvalidRoomAccessException("Invalid access");
+ }
+ Claims claims = jwtCore.parse(token);
+
+ if (!JwtTokenType.ROOM_ACCESS.name().equals(claims.get("type"))) {
+ throw new ExceptionController.InvalidRoomAccessException("Invalid room access info");
+ }
+
+ String tokenRoomId = claims.get("roomId", String.class);
+ if (!roomId.equals(tokenRoomId)) {
+ throw new ExceptionController.InvalidRoomAccessException("Invalid room access info");
+ }
+ } catch (Exception e) {
+ throw new ExceptionController.InvalidRoomAccessException("Invalid access");
+ }
</code_context>
<issue_to_address>
**suggestion:** The broad `catch (Exception)` rewraps `InvalidRoomAccessException`, losing the more specific failure reasons.
The inner `InvalidRoomAccessException` branches end up unused because the outer `catch (Exception e)` always wraps them as a generic "Invalid access". If you need the specific messages, either rethrow `InvalidRoomAccessException` unchanged or narrow the catch to only parsing/runtime exceptions so existing `InvalidRoomAccessException`s can bubble up.
```suggestion
public void validate(String token, String roomId) throws ExceptionController.InvalidRoomAccessException {
if (StringUtil.isNullOrEmpty(token)) {
throw new ExceptionController.InvalidRoomAccessException("Invalid access");
}
try {
Claims claims = jwtCore.parse(token);
if (!JwtTokenType.ROOM_ACCESS.name().equals(claims.get("type"))) {
throw new ExceptionController.InvalidRoomAccessException("Invalid room access info");
}
String tokenRoomId = claims.get("roomId", String.class);
if (!roomId.equals(tokenRoomId)) {
throw new ExceptionController.InvalidRoomAccessException("Invalid room access info");
}
} catch (RuntimeException e) {
// Token parsing or claim access failed: treat as generic invalid access
throw new ExceptionController.InvalidRoomAccessException("Invalid access");
}
```
</issue_to_address>
### Comment 4
<location> `nodejs-frontend/static/js/popup/room_popup.js:195-198` </location>
<code_context>
let successCallback = function(result) {
- if (result?.data && result?.result === 'success') {
+ if (result?.data?.isValidate === true) {
+ var roomToken = result.data.token;
+ sessionStorage.setItem('roomAccessToken', roomToken);
self.showToast('방에 정상적으로 입장했습니다!', 'success');
$('#enterRoomModal').modal('hide');
</code_context>
<issue_to_address>
**issue (bug_risk):** The password validation success handler no longer shows feedback when the password is wrong.
This change removed the `result?.result === 'success'` check and now only handles `result?.data?.isValidate === true`. When `isValidate` is `false` (wrong password/over capacity), this callback does nothing, so the user gets no error and the modal stays open. Add an `else` branch to show an error toast (e.g. as in `room_settings_popup.js`) or reuse existing invalid‑password error handling so failures are surfaced to the user.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+31
to
+35
| public Claims parse(String token) { | ||
| return Jwts.parser() | ||
| .setSigningKey(key) | ||
| .build() | ||
| .parseClaimsJws(token) |
Contributor
There was a problem hiding this comment.
issue (bug_risk): JJWT 파서 사용 방식(Jwts.parser().build())이 현재 라이브러리 API와 일치하지 않아 컴파일되지 않을 수 있습니다.
사용 중인 jjwt 버전을 확인해 주세요. jjwt 0.11+ 를 사용 중이라면, parser()에 의존하지 말고 Jwts.parserBuilder().setSigningKey(key).build().parseClaimsJws(token)로 업데이트해야 합니다. parser()는 더 이상 .build()를 지원하지 않으며, 이대로는 컴파일되지 않습니다.
Original comment in English
issue (bug_risk): The JJWT parser usage (Jwts.parser().build()) looks inconsistent with current library APIs and may not compile.
Please confirm the jjwt version in use. For jjwt 0.11+ this should be updated to Jwts.parserBuilder().setSigningKey(key).build().parseClaimsJws(token) rather than relying on parser(), which no longer supports .build() and will not compile.
Comment on lines
+195
to
198
| if (result?.data?.isValidate === true) { | ||
| var roomToken = result.data.token; | ||
| sessionStorage.setItem('roomAccessToken', roomToken); | ||
| self.showToast('방에 정상적으로 입장했습니다!', 'success'); |
Contributor
There was a problem hiding this comment.
issue (bug_risk): 비밀번호 검증 성공 핸들러가 이제 비밀번호가 틀렸을 때 피드백을 보여주지 않습니다.
이 변경으로 result?.result === 'success' 체크가 제거되고, result?.data?.isValidate === true 인 경우만 처리하게 되었습니다. isValidate가 false(비밀번호 오류/정원 초과 등)인 경우 이 콜백은 아무 것도 하지 않기 때문에, 사용자는 아무런 에러도 보지 못하고 모달은 열린 상태로 남습니다. room_settings_popup.js와 같이 에러 토스트를 보여주는 else 분기를 추가하거나, 기존의 비밀번호 오류 처리 로직을 재사용해서 실패 시에도 사용자에게 명확히 노출되도록 해 주세요.
Original comment in English
issue (bug_risk): The password validation success handler no longer shows feedback when the password is wrong.
This change removed the result?.result === 'success' check and now only handles result?.data?.isValidate === true. When isValidate is false (wrong password/over capacity), this callback does nothing, so the user gets no error and the modal stays open. Add an else branch to show an error toast (e.g. as in room_settings_popup.js) or reuse existing invalid‑password error handling so failures are surfaced to the user.
Owner
Owner
|
@kidonge 최근에 수정된 내용이 있어, 메인 브렌치에서 pull 받은 후 머지 부탁드립니다 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
비밀번호를 입력하여 정상적으로 방 입장 시에 JWT 토큰을 발급하여 관리하도록 하였습니다.
비정상적인 루트인 url를 통해 방 입장 시도 시 토큰을 확인하여 유효성 검사 로직을 추가하였습니다.
Summary by Sourcery
비밀번호로 보호되는 채팅방에 안전하게 입장할 수 있도록 JWT 기반의 방 액세스 토큰을 추가하고, 방 참가 요청 시 액세스를 검증합니다.
New Features:
X-Room-Token헤더를 통해 방별 액세스 토큰을 포함하고 이를 검증합니다.Enhancements:
40061을 사용합니다.X-Room-Token을 허용하도록 CORS 구성을 확장합니다.Original summary in English
Summary by Sourcery
Add JWT-based room access tokens to secure entry into password-protected chat rooms and validate access on room join requests.
New Features:
Enhancements: