fix: zoom header debugging #132
Conversation
WalkthroughZoom 웹훅 처리가 리팩토링되어 레거시 OAuth/클라이언트/핸들러가 제거되고, 컨트롤러→서비스→AOP 인증 흐름과 토큰 암호화, 신규 DTO/예외/타입, 핸들러 시그니처 변경이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Zoom as Zoom
participant Controller as ZoomWebhookController
participant Aspect as ZoomWebhookAspect
participant Service as ZoomWebhookService
participant TokenProv as ZoomTokenProvider
participant Attendance as ZoomAttendanceService
participant DB as DB
Zoom->>Controller: POST /api/v3/zoom/webhook/events (ZoomWebhookEvent)
Controller->>Aspect: method(`@ZoomAuth`) 진입
Aspect->>Aspect: 이벤트가 URL_VALIDATION인지 판별
alt URL_VALIDATION
Aspect-->>Controller: 인증 생략
Controller->>Service: handleUrlValidationRequest(event)
Service->>TokenProv: generateEncryptedToken(plainToken)
TokenProv-->>Service: encryptedToken
Service-->>Zoom: 200 (ZoomWebhookValidationResponse)
else 일반 이벤트
Aspect->>Aspect: Authorization 헤더 검증
Aspect-->>Controller: 인증 통과
Controller->>Service: handleWebhookEvents(event)
Service->>Attendance: handleParticipantJoined/Left(WebhookParticipant)
Attendance->>DB: findActiveSessionByParticipantUuid / 저장/수정
DB-->>Attendance: 결과
Attendance-->>Service: 완료
Service-->>Zoom: 200 (ZoomWebhookValidationResponse nulls)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java (2)
32-37: 불필요한 try-catch 블록을 제거하세요.로깅 작업 자체는 일반적으로 예외를 발생시키지 않으며, 발생하더라도 조용히 무시하는 것은 문제를 숨길 수 있습니다. 이 try-catch는 불필요한 방어 코드입니다.
다음 diff를 적용하여 try-catch를 제거하세요:
- // 디버깅용 전체 헤더 & 바디 로깅 - try { - log.info("Zoom Webhook headers: {}", headers); - log.info("Zoom Webhook raw body: {}", rawBody); - } catch (Exception e) { - log.warn("Zoom Webhook logging failed: {}", e.getMessage()); - } + // 디버깅용 전체 헤더 & 바디 로깅 + log.info("Zoom Webhook headers: {}", headers); + log.info("Zoom Webhook raw body: {}", rawBody);
28-29: 디버깅 완료 후 원래 메서드 시그니처로 복원을 고려하세요.PR 제목과 코드 주석을 보면 이 변경사항은 헤더/바디 디버깅을 위한 임시 조치로 보입니다. 디버깅이 완료되면 원래의
@RequestBody ZoomWebhookEvent event바인딩 방식으로 되돌리는 것을 고려하세요. 현재 방식은 Spring의 자동 바인딩 이점을 포기하고 수동으로 파싱합니다.디버깅이 완료되면 다음과 같이 원래 방식으로 복원하세요:
@PostMapping("/api/v3/zoom/webhook/events") public ResponseEntity<String> handleWebhookEvent( @RequestHeader("Authorization") String authHeader, @RequestBody ZoomWebhookEvent event) { // 기존 검증 및 처리 로직 ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java(2 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java(2 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: build
techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java
Outdated
Show resolved
Hide resolved
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java (1)
40-46: 민감한 정보 노출 문제가 이전 리뷰에서 이미 지적되었습니다.전체 헤더와 바디를 INFO 레벨로 로깅하면 인증 토큰, 이메일 주소 등 민감한 정보가 프로덕션 로그에 기록됩니다. 이전 리뷰의 권장 사항을 적용해 주세요.
🧹 Nitpick comments (2)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java (2)
48-50: LGTM!HTTP 헤더의 대소문자 차이를 고려한 처리가 적절합니다. 다만 Spring의
HttpHeaders를 사용하면 대소문자 구분 없이 헤더를 가져올 수 있어 더 간단해질 수 있습니다.
52-62: 파싱 오류 처리가 적절합니다.JSON 파싱 실패 시 400 응답과 함께 오류를 로깅하는 것은 올바른 접근입니다. 단, 주석(Lines 54-57)이 다소 장황하므로 정리를 권장합니다.
try { - // 기존 도메인 로직을 최대한 건드리지 않기 위해, - // 일단 기존 DTO 매핑이 동작하던 방식 그대로 유지하려면 - // @RequestBody ZoomWebhookEvent 를 쓰는 원래 메서드를 래핑하는 편이 가장 좋지만, - // 지금은 빠른 헤더 확인을 위해 최소한의 파싱만 시도하지 않고, 토큰 검증/이벤트 타입까지만 본다. + // rawBody를 수동 파싱하여 기존 DTO 구조 유지 event = ZoomWebhookEvent.fromJson(rawBody); } catch (Exception e) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
techeerzip/src/main/java/backend/techeerzip/domain/todayCs/scheduler/TodayCsScheduler.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java(0 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java(2 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java(2 hunks)techeerzip/src/test/java/backend/techeerzip/domain/projectMember/service/ProjectMemberServiceTest.java(2 hunks)
💤 Files with no reviewable changes (1)
- techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java
✅ Files skipped from review due to trivial changes (1)
- techeerzip/src/test/java/backend/techeerzip/domain/projectMember/service/ProjectMemberServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java
⏰ 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 (2)
techeerzip/src/main/java/backend/techeerzip/domain/todayCs/scheduler/TodayCsScheduler.java (1)
37-37: 포맷팅 변경만 있어 동작에는 영향 없습니다
eventPublisher.publishEvent(new SlackEvent.Channel(errorMessage, SlackChannelType.EA));로 한 줄 정리한 변경은 기존과 동등 동작이며, 가독성도 충분히 괜찮습니다. 추가 수정 없이 그대로 유지해도 될 것 같습니다.techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java (1)
3-6: LGTM!새로운 기능에 필요한 import와 의존성이 적절하게 추가되었습니다.
Also applies to: 13-13, 25-25
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java (2)
37-42: 헬스체크 엔드포인트의 기존 지적사항이 아직 반영되지 않았습니다.이전 리뷰에서 지적된 문제들이 여전히 남아있습니다:
- 연결 실패 시에도 항상 200 OK를 반환
- 메서드 이름
handleWebhookEvent()가 실제 기능(health check)과 불일치- 연결 상태를 응답 코드에 반영해야 함
이전 리뷰의 제안사항을 참고하세요.
85-109: 민감한 정보 로깅에 대한 기존 지적사항이 아직 반영되지 않았습니다.이전 리뷰에서 지적된 문제가 여전히 남아있습니다:
- 전체 헤더 로깅으로 인한 Authorization 토큰 노출
- 전체 바디 로깅으로 인한 참가자 이메일, user_id 등 개인정보 노출
- 프로덕션 환경에서 GDPR/개인정보보호 위험
디버깅이 완료되면 민감한 필드를 마스킹하거나 로그를 제거/DEBUG 레벨로 변경하세요.
🧹 Nitpick comments (1)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java (1)
174-206: Hex 변환 로직을 간소화할 수 있습니다.HMAC-SHA256 구현은 올바르지만, 16진수 변환 로직(lines 191-198)을 Apache Commons Codec의
Hex.encodeHexString()또는 Java 17+의HexFormat을 사용하여 간소화할 수 있습니다.Option 1: Apache Commons Codec 사용 (이미 의존성이 있는 경우)
+import org.apache.commons.codec.binary.Hex; + private String generateEncryptedToken(String plainToken) { String secretToken = zoomApiConfig.getWebhookSecretToken(); if (secretToken == null || secretToken.isEmpty()) { log.error("Webhook secret token이 설정되지 않았습니다"); return null; } try { Mac mac = Mac.getInstance("HmacSHA256"); SecretKeySpec secretKeySpec = new SecretKeySpec( secretToken.getBytes(StandardCharsets.UTF_8), "HmacSHA256"); mac.init(secretKeySpec); byte[] encryptedBytes = mac.doFinal(plainToken.getBytes(StandardCharsets.UTF_8)); - - // 16진수 문자열로 변환 - StringBuilder hexString = new StringBuilder(); - for (byte b : encryptedBytes) { - String hex = Integer.toHexString(0xff & b); - if (hex.length() == 1) { - hexString.append('0'); - } - hexString.append(hex); - } - - return hexString.toString(); + return Hex.encodeHexString(encryptedBytes); } catch (Exception e) { log.error("토큰 암호화 중 오류: {}", e.getMessage(), e); return null; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java(3 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: build
🔇 Additional comments (2)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java (2)
3-22: 새로운 의존성이 적절하게 추가되었습니다.추가된 import들은 웹훅 URL 검증(
javax.crypto,ObjectMapper), 원본 헤더/바디 처리(Map,RequestParam), 그리고 헬스체크(ZoomApiClient) 기능을 지원하기 위해 필요합니다.
34-34: 헬스체크를 위한 의존성 추가가 적절합니다.
ZoomApiClient가 헬스체크 엔드포인트에서 연결 상태를 확인하는 데 사용됩니다.
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java (2)
38-43: 헬스 체크 엔드포인트가testConnection()결과를 반영하지 않습니다.현재 구현은
zoomApiClient.testConnection()결과와 관계없이 항상200 OK를 반환합니다. 이 상태로는 실제 Zoom API 연결이 끊겨도 헬스 체크/모니터링에서 감지하기 어렵고, 메서드명도 POST 웹훅 핸들러(handleWebhookEvent)와 같아 혼동을 줄 수 있습니다. (이전에 지적되었던 부분과 동일합니다.)예를 들어 아래처럼 전용 헬스 엔드포인트로 분리하고 상태 코드를 분기하는 걸 고려해 주세요:
- @GetMapping("/api/v3/zoom") - public ResponseEntity<Void> handleWebhookEvent() { - boolean isConnected = zoomApiClient.testConnection(); - log.debug("Zoom testConnection: {}", isConnected); - return ResponseEntity.ok().build(); - } + @GetMapping("/api/v3/zoom/health") + public ResponseEntity<Void> checkZoomHealth() { + boolean isConnected = zoomApiClient.testConnection(); + log.debug("Zoom testConnection: {}", isConnected); + if (!isConnected) { + log.warn("Zoom health check failed"); + return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE).build(); + } + return ResponseEntity.ok().build(); + }(
HttpStatusimport 추가 필요)
88-94: 헤더·바디 전체를 INFO 레벨로 로깅하면 토큰·이메일 등 민감정보가 노출됩니다.현재 구현은 다음과 같은 정보를 그대로 로그에 남길 수 있습니다.
Authorization헤더의 토큰- 웹훅 바디 내 참가자 이메일, user_id, participant_uuid 등 식별자
프로덕션 환경에서 이런 로그는 개인정보·보안 리스크가 커서, 이전 리뷰에서 이미 지적됐던 부분과 동일합니다.
아래와 같이 최소한 Authorization 을 제외하고, 바디는 길이 정도만 DEBUG 레벨로 남기는 식으로 조정하는 걸 권장합니다.
- try { - log.info("Zoom Webhook headers: {}", headers); - log.info("Zoom Webhook raw body: {}", rawBody); - } catch (Exception e) { - log.warn("Zoom Webhook logging failed: {}", e.getMessage()); - } + try { + Map<String, String> sanitizedHeaders = headers.entrySet().stream() + .filter(e -> !"authorization".equalsIgnoreCase(e.getKey())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + log.debug("Zoom Webhook headers (sanitized): {}", sanitizedHeaders); + log.debug("Zoom Webhook body length: {}", rawBody != null ? rawBody.length() : 0); + } catch (Exception e) { + log.warn("Zoom Webhook logging failed: {}", e.getMessage()); + }(위 예시 사용 시
java.util.stream.Collectorsimport 추가 필요)운영 환경에서는 가능하면 바디 전문 로그는 완전히 제거하는 것도 고려해 주세요.
🧹 Nitpick comments (2)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java (2)
45-81: URL 검증 GET 핸들러는 동작은 맞지만, JSON 처리 부분은 단순화할 수 있습니다.동작 자체(plainToken → encryptedToken 생성 후 JSON 응답)는 문제 없어 보입니다. 실제 Zoom
endpoint.url_validation플로우는 아래 POST 핸들러에서 처리되고, 이 GET은 디버깅/수동 호출 용도로 보입니다.다만 유지보수·성능 측면에서 아래 정도의 리팩터링을 고려해 볼 수 있습니다.
ObjectMapper mapper = new ObjectMapper();를 매번 새로 만들기보다는,ObjectMapper를 필드로 주입해 재사용 (private final ObjectMapper objectMapper;)mapper.writeValueAsString(response)로 문자열을 직접 만들어ResponseEntity<String>을 반환하기보다는,ResponseEntity.ok(response)로ObjectNode자체를 반환해 Spring의 메시지 컨버터에 JSON 직렬화를 맡기기필수 변경 사항은 아니지만, 전반적인 JSON 처리 일관성을 맞추는 데 도움이 될 것 같습니다.
150-183: URL 검증 POST 처리와 HMAC-SHA256 토큰 생성 구현은 타당해 보입니다.
handleUrlValidationRequest에서payload.plainToken을 추출해generateEncryptedToken으로 HMAC-SHA256(plainToken, webhookSecretToken) 을 만든 뒤{ plainToken, encryptedToken }JSON 으로 응답하는 흐름은 Zoom 문서와 일치하고, secret 미설정 시 500으로 fail-fast 하는 것도 합리적입니다.추가로 굳이 수정하진 않아도 되는 수준의 제안만 드리면:
- 여기서도
ObjectMapper를 새로 만들기보다, 클래스 필드로 주입해 재사용하면 전체 웹훅 핸들러 전반의 JSON 처리 비용과 코드 중복을 줄일 수 있습니다.기능 측면에서는 현재 구현으로 충분히 동작할 것으로 보입니다.
Also applies to: 210-243
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-12T07:36:39.412Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/infra/github/GitHubApiService.java:99-103
Timestamp: 2025-09-12T07:36:39.412Z
Learning: GitHub URL 검증은 서비스 레이어가 아닌 request DTO 단에서 Bean Validation 어노테이션을 사용하여 수행해야 한다.
Applied to files:
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java
⏰ 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
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java (1)
86-140: Fix the execution order inZoomLeaveReason.isCompleteExit()method.The method has a critical logic flaw: it checks if the input contains any enum value and returns
trueimmediately (lines 53-57), making the exclusion case checks unreachable (lines 60-74). If Zoom sends a reason containing both a matching enum value and an exclusion keyword (e.g., "left the meeting after joining breakout room"), the exclusion check will never execute and an incorrect result will be returned.Move the exclusion checks before the enum value loop to ensure exclusions are evaluated first, or redesign the logic to check exclusions while iterating through enum values.
🧹 Nitpick comments (7)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/type/ZoomWebhookEventType.java (1)
17-27: 정적 메서드의 null 처리가 안전하게 구현되었습니다.
getEventName().equals(eventName)패턴을 사용하여 null 안전성이 확보되어 있습니다. eventName이 null인 경우equals()는 false를 반환합니다.선택적으로 null 처리를 명시적으로 문서화하면 더 명확할 수 있습니다:
/** + * 주어진 이벤트 이름이 URL 검증 이벤트인지 확인 + * + * @param eventName 확인할 이벤트 이름 (null 가능) + * @return URL 검증 이벤트면 true, 그 외(null 포함)는 false + */ public static boolean isUrlValidation(String eventName) { return URL_VALIDATION.getEventName().equals(eventName); }techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomTokenProvider.java (1)
23-55: 토큰 암호화 로직이 올바르게 구현되었습니다.HMAC-SHA256를 사용한 토큰 암호화가 적절하게 구현되어 있으며, 설정 검증과 예외 처리도 잘 되어 있습니다.
선택적으로 16진수 변환 부분을
HexFormat(Java 17+) 또는 라이브러리를 사용하여 간소화할 수 있습니다:+import java.util.HexFormat; public String generateEncryptedToken(String plainToken) { // ... (기존 로직) byte[] encryptedBytes = mac.doFinal(plainToken.getBytes(StandardCharsets.UTF_8)); - - // 16진수 문자열로 변환 - StringBuilder hexString = new StringBuilder(); - for (byte b : encryptedBytes) { - String hex = Integer.toHexString(0xff & b); - if (hex.length() == 1) { - hexString.append('0'); - } - hexString.append(hex); - } - - return hexString.toString(); + return HexFormat.of().formatHex(encryptedBytes); } catch (Exception e) { // ... }techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookController.java (1)
22-33: 요청 데이터 검증 추가를 권장합니다.컨트롤러에서
@RequestBody에 대한 검증이 없습니다.ZoomWebhookEvent의 필수 필드(예:@NonNull필드들)에 대한 검증을 추가하면 더 안전합니다.다음과 같이
@Valid어노테이션을 추가하세요:+import jakarta.validation.Valid; @PostMapping("/api/v3/zoom/webhook/events") public ResponseEntity<ZoomWebhookValidationResponse> handleWebhookEvent( - @RequestBody ZoomWebhookEvent event) { + @RequestBody @Valid ZoomWebhookEvent event) {이를 위해
ZoomWebhookEvent클래스에 적절한 검증 어노테이션(@NotNull,@NotBlank등)이 정의되어 있는지 확인하세요.techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookFilterConfig.java (1)
25-25: URL 패턴의 범위를 확인하세요.현재 URL 패턴
/api/v3/zoom/webhook/*는 단일 경로 세그먼트만 매칭합니다. 향후/api/v3/zoom/webhook/events/subpath와 같은 하위 경로가 추가될 경우 매칭되지 않습니다.만약 하위 경로를 포함한 모든 경로를 매칭하려면 다음과 같이 수정하세요:
-registrationBean.addUrlPatterns("/api/v3/zoom/webhook/*"); +registrationBean.addUrlPatterns("/api/v3/zoom/webhook/**");현재
/api/v3/zoom/webhook/events엔드포인트만 있다면 현재 패턴으로도 충분합니다.techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java (2)
71-74: 중복된 Authorization 헤더 조회 로직입니다.
HttpServletRequest.getHeader()는 Servlet 스펙에 따라 대소문자를 구분하지 않습니다. 따라서"authorization"과"Authorization"두 가지를 확인할 필요가 없습니다.- String authHeader = - request.getHeader("authorization") != null - ? request.getHeader("authorization") - : request.getHeader("Authorization"); + String authHeader = request.getHeader("Authorization");
87-92: 광범위한 예외 처리로 인해 디버깅이 어려울 수 있습니다.모든 예외를 동일하게 처리하면 인증 실패와 서버 오류를 구분하기 어렵습니다. JSON 파싱 오류 등은 400 Bad Request로 처리하는 것이 더 적절할 수 있습니다.
- } catch (Exception e) { - log.error("웹훅 인증 필터 처리 중 오류: {}", e.getMessage(), e); - response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - response.setContentType(MediaType.TEXT_PLAIN_VALUE); - response.getWriter().write("Internal Server Error"); - } + } catch (com.fasterxml.jackson.core.JsonProcessingException e) { + log.warn("웹훅 요청 body 파싱 실패: {}", e.getMessage()); + response.setStatus(HttpServletResponse.SC_BAD_REQUEST); + response.setContentType(MediaType.TEXT_PLAIN_VALUE); + response.getWriter().write("Bad Request"); + } catch (Exception e) { + log.error("웹훅 인증 필터 처리 중 오류: {}", e.getMessage(), e); + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + response.setContentType(MediaType.TEXT_PLAIN_VALUE); + response.getWriter().write("Internal Server Error"); + }techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookService.java (1)
71-71: 의미론적으로 부적절한 반환값입니다.
handleWebhookEvents는 URL 검증이 아닌 이벤트 처리 메서드인데ZoomWebhookValidationResponse(null, null)을 반환합니다. 반환 타입을void로 변경하거나 별도의 응답 DTO를 사용하는 것이 더 명확합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java(3 hunks)techeerzip/src/main/java/backend/techeerzip/global/exception/ErrorCode.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/client/ZoomApiClient.java(0 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomApiConfig.java(0 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomTokenProvider.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookFilterConfig.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomOAuthTokenResponse.java(0 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java(3 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookValidationResponse.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomException.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/type/ZoomLeaveReason.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/type/ZoomWebhookEventType.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookController.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java(0 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookService.java(1 hunks)
💤 Files with no reviewable changes (4)
- techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomOAuthTokenResponse.java
- techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomApiConfig.java
- techeerzip/src/main/java/backend/techeerzip/infra/zoom/client/ZoomApiClient.java
- techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookHandler.java
🧰 Additional context used
🧬 Code graph analysis (4)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/type/ZoomWebhookEventType.java (2)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java (4)
Getter(13-115)Getter(30-44)Getter(46-77)Getter(79-114)techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookValidationResponse.java (1)
Getter(10-20)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookValidationResponse.java (1)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java (4)
Getter(13-115)Getter(30-44)Getter(46-77)Getter(79-114)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/type/ZoomLeaveReason.java (1)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java (4)
Getter(13-115)Getter(30-44)Getter(46-77)Getter(79-114)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomTokenProvider.java (3)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomException.java (1)
ZoomException(7-16)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java (1)
Slf4j(24-136)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookService.java (1)
Slf4j(13-73)
⏰ 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 (5)
techeerzip/src/main/java/backend/techeerzip/global/exception/ErrorCode.java (1)
230-236: Zoom 관련 에러 코드가 적절하게 추가되었습니다.에러 코드, 메시지, HTTP 상태 코드가 모두 적절하게 정의되었습니다. 인프라 설정 오류에 대해 INTERNAL_SERVER_ERROR를 사용하는 것이 적합합니다.
techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java (1)
27-83: 참가자 입장 처리 로직이 적절하게 구현되었습니다.활성 세션 존재 여부를 확인하여 소회의실 복귀를 무시하는 로직이 명확하게 구현되어 있습니다. participant_uuid의 동작에 대한 주석도 이해를 돕습니다.
techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomException.java (1)
1-16: 예외 클래스가 적절하게 구현되었습니다.InfraException을 확장하여 Zoom 관련 예외를 명확하게 구분하고, 표준 예외 패턴을 따르고 있습니다.
techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookValidationResponse.java (1)
9-20: LGTM!Zoom 웹훅 URL 검증 응답을 위한 깔끔한 DTO 구조입니다. Lombok 어노테이션 사용이 적절합니다.
techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java (1)
108-113: 새로운 필드 추가가 적절합니다.
publicIp와participantUserId필드 추가가 Zoom 웹훅 스펙에 맞게 구현되었습니다.@JsonIgnoreProperties(ignoreUnknown = true)와 함께 사용되어 향후 API 변경에도 유연하게 대응할 수 있습니다.
...rzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java
Outdated
Show resolved
Hide resolved
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookFilterConfig.java
Outdated
Show resolved
Hide resolved
techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java
Show resolved
Hide resolved
techeerzip/src/main/java/backend/techeerzip/infra/zoom/type/ZoomLeaveReason.java
Show resolved
Hide resolved
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookService.java
Outdated
Show resolved
Hide resolved
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookInvalidAuthenticationException.java (1)
14-16: 예외 체이닝을 위한 생성자 추가를 고려해 주세요.
ZoomWebhookProcessingException과 달리Throwable cause를 받는 생성자가 없습니다.AuthenticationException은(String msg, Throwable cause)생성자를 제공하므로, 디버깅 시 근본 원인 추적을 위해 추가 생성자를 제공하는 것이 좋습니다.public ZoomWebhookInvalidAuthenticationException() { super(ERROR_CODE.getMessage()); } + + public ZoomWebhookInvalidAuthenticationException(Throwable cause) { + super(ERROR_CODE.getMessage(), cause); + }techeerzip/src/main/java/backend/techeerzip/global/config/SecurityConfig.java (2)
22-56: Zoom 전용 SecurityFilterChain 구조는 적절하지만, 의도를 코드로 더 명확히 드러내는 것이 좋습니다.
@Order(0) + securityMatcher("/api/v3/zoom/webhook/**")로 Zoom 웹훅을 별도 체인으로 분리한 것은 좋은 패턴입니다.ZoomWebhookAuthenticationFilter를UsernamePasswordAuthenticationFilter앞에 두는 것도 위치 선정이 적절해 보입니다.- 다만 이 체인에서
authorizeHttpRequests설정이 전혀 없어 기본 동작에 의존하고 있기 때문에, 의도를 명시적으로 드러내는 편이 좋습니다. 예를 들어 아래처럼 인증 요구를 분명히 적어두면, 나중에 Spring Security 버전/기본값이 바뀌더라도 의미가 명확합니다.public SecurityFilterChain zoomSecurityFilterChain(HttpSecurity http) throws Exception { http .securityMatcher("/api/v3/zoom/webhook/**") .csrf(AbstractHttpConfigurer::disable) .sessionManagement(s -> s.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) + .authorizeHttpRequests(auth -> auth.anyRequest().authenticated()) .addFilterBefore( zoomWebhookAuthenticationFilter, UsernamePasswordAuthenticationFilter.class) .exceptionHandling( ex -> ex.authenticationEntryPoint(customAuthenticationEntryPoint)); return http.build(); }
- Zoom 측이 인증 실패 시 기대하는 응답 형식/HTTP 상태 코드가
CustomAuthenticationEntryPoint의 동작과 잘 맞는지도 한 번만 더 점검해 두면 좋겠습니다(예: JSON 바디 vs 단순 텍스트, 401/403 코드 등).
119-139:/api/v3/zoom/webhook/**에 대한 설정이 두 체인에 중복 정의되어 있어 혼동 여지가 있습니다.
zoomSecurityFilterChain이@Order(0)에서securityMatcher("/api/v3/zoom/webhook/**")를 사용하므로, 이 경로에 대한 요청은 이 체인에서만 처리되고apiSecurityFilterChain에는 도달하지 않습니다.- 따라서
apiSecurityFilterChain의.requestMatchers( "/api/v3/zoom/webhook/**", "/actuator/prometheus") .permitAll()중 Zoom 관련 부분은 실질적으로는 사용되지 않는(dead config) 상태입니다.
- 나중에 설정을 읽는 사람이 “웹훅이 permitAll 인가?”라고 혼동할 수 있으니,
/api/v3/zoom/webhook/**는 여기에서 제거하고 Zoom 전용 체인에만 유지하는 것을 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
techeerzip/src/main/java/backend/techeerzip/global/config/SecurityConfig.java(5 hunks)techeerzip/src/main/java/backend/techeerzip/global/config/WebConfig.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/global/exception/ErrorCode.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/global/exception/GlobalExceptionHandler.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookFilterConfig.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookInvalidAuthenticationException.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookProcessingException.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- techeerzip/src/main/java/backend/techeerzip/global/exception/GlobalExceptionHandler.java
🚧 Files skipped from review as they are similar to previous changes (2)
- techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookFilterConfig.java
- techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java
⏰ 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 (4)
techeerzip/src/main/java/backend/techeerzip/global/config/WebConfig.java (1)
18-18: LGTM! final 키워드 추가가 적절합니다.생성자를 통해 주입되는 필드를 final로 선언하는 것은 Java 모범 사례입니다. 이는 불변성을 보장하고 의도를 명확히 전달합니다.
techeerzip/src/main/java/backend/techeerzip/global/exception/ErrorCode.java (1)
230-240: LGTM! Zoom 관련 에러 코드가 적절히 추가되었습니다.에러 코드 네이밍 컨벤션(ZOOM001-ZOOM005)과 HttpStatus 선택이 기존 패턴과 일관성 있게 작성되었습니다. 설정 오류와 처리 오류에는
INTERNAL_SERVER_ERROR, 인증 실패에는UNAUTHORIZED를 사용한 것이 적절합니다.techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookProcessingException.java (1)
1-11: LGTM! 예외 클래스가 적절히 구현되었습니다.
Throwable cause를 통한 예외 체이닝이 올바르게 구현되어 있어 디버깅 시 근본 원인 추적이 가능합니다.techeerzip/src/main/java/backend/techeerzip/global/config/SecurityConfig.java (1)
10-11:csrf(AbstractHttpConfigurer::disable)로 통일한 변경은 읽기 쉽고 최신 스타일입니다.
- Swagger 체인과 API 체인 모두에서 CSRF 비활성화를
AbstractHttpConfigurer::disable로 통일한 것은 Spring Security 6 스타일에 잘 맞고 가독성도 좋아졌습니다.- JWT 기반/웹훅 전용의
SessionCreationPolicy.STATELESS구성과도 일관성이 있습니다.Also applies to: 65-67, 139-145
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/type/ZoomLeaveReason.java (1)
45-78: 이전 치명적 버그는 해결되었고, isCompleteExit 로직도 안전하게 작성되었습니다.
null/공백 처리 → 소회의실/대기실/호스트 미참여 제외 → enum 기반 완전 퇴장 여부 판별 순서라, breakout 관련 퇴장을 잘 거르고 있습니다.- 매칭 실패 시 기본적으로
false를 반환하는 것도 안전한 선택입니다.선택 사항이지만, 추후 제외 키워드나 완전 퇴장 사유가 늘어날 가능성을 생각하면,
"breakout room","join breakout","leave breakout"같은 문자열과 enumreasonText들을 각각 상수 리스트/셋으로 추출해containsAny(...)유틸로 돌리면 유지보수가 조금 더 쉬워질 것 같습니다.techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java (2)
49-50: 패턴 매칭 변수명을 개선하세요.패턴 매칭에서 생성된 변수
contentcachingrequestwrapper는 모두 소문자로 되어 있어 가독성이 떨어집니다.다음 diff를 적용하여 더 읽기 쉬운 변수명으로 변경하세요:
- if (request instanceof ContentCachingRequestWrapper contentcachingrequestwrapper) { - wrappedRequest = contentcachingrequestwrapper; + if (request instanceof ContentCachingRequestWrapper wrapped) { + wrappedRequest = wrapped;
69-72: Remove the redundant header check.The Servlet API's
HttpServletRequest.getHeader()method is case-insensitive; the javadoc explicitly documents this, and HTTP specifications (RFC 2616 / RFC 9110) specify that header field names are case-insensitive. The duplicate check for "authorization" and "Authorization" is unnecessary—a single call with either case will retrieve the header.techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java (3)
17-26: Lombok@NonNull필드는 현재 구조에서는 실질적인 검증을 하지 않습니다.이 클래스는
@NoArgsConstructor+@Getter만 사용해서 Lombok이 생성자/세터를 생성하지 않으므로,eventName·payload의@NonNull에 따른 NPE 체크 코드가 실제로는 만들어지지 않습니다.
요청 바디 유효성 검사 목적이라면lombok.NonNull대신jakarta.validation.constraints.NotNull(+ 컨트롤러 파라미터에@Valid)로 Bean Validation을 쓰거나, 검증을 서비스 레이어에서만 한다면@NonNull을 제거해서 혼동을 줄이는 편이 명확해 보입니다.
39-40:plainToken필드 추가와 서비스 레이어 검증 플로우가 잘 맞습니다.URL 검증 이벤트에서만 사용되는
plainToken을 payload 쪽으로 옮기고,ZoomWebhookService.handleUrlValidationRequest에서 null/blank 체크로 예외를 던지는 구조라 이벤트 타입별 필드 존재 여부 문제(이전@NotBlank이슈)를 잘 정리하신 것 같습니다.
추가로, 운영 중에 URL 검증 이외 이벤트에plainToken이 예기치 않게 채워지는 케이스가 없는지 디버깅 로그 한 번만 확인해 두면 향후 분석에 도움이 될 수 있습니다.
105-109: 참가자 추가 필드(publicIp,participantUserId)는 괜찮지만, 사용/로깅 정책을 정해두는 것이 좋습니다.Zoom에서 내려주는
public_ip,participant_user_id를 DTO에 매핑해 두신 것은 확장성 면에서 문제 없습니다. 다만 현재 사용처가 없다면:
- 향후 통계·진단 등 실제 사용 계획이 없다면 과도한 필드 노출이 될 수 있고,
- 특히 IP/내부 ID는 민감 정보가 될 수 있으니, 로그에 그대로 출력하지 않도록 주의(마스킹 또는 미로그) 정책을 정해 두면 좋겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
techeerzip/src/main/java/backend/techeerzip/global/exception/ErrorCode.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java(3 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookPlainTokenException.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookProcessingException.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/type/ZoomLeaveReason.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookService.java
- techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookProcessingException.java
🧰 Additional context used
🧬 Code graph analysis (3)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/type/ZoomLeaveReason.java (1)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java (4)
Getter(11-111)Getter(28-41)Getter(43-74)Getter(76-110)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java (5)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookInvalidAuthenticationException.java (1)
ZoomWebhookInvalidAuthenticationException(10-21)techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookProcessingException.java (1)
ZoomWebhookProcessingException(7-12)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookService.java (1)
Slf4j(16-81)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomTokenProvider.java (1)
Slf4j(15-56)techeerzip/src/main/java/backend/techeerzip/global/exception/GlobalExceptionHandler.java (1)
Slf4j(18-171)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java (6)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookValidationResponse.java (1)
Getter(10-20)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookService.java (1)
Slf4j(16-81)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomTokenProvider.java (1)
Slf4j(15-56)techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java (1)
Slf4j(18-155)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookController.java (1)
Slf4j(14-34)techeerzip/src/main/java/backend/techeerzip/domain/statistic/service/StatisticService.java (1)
Slf4j(37-219)
⏰ 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 (11)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/type/ZoomLeaveReason.java (1)
6-37: ZoomLeaveReason enum 설계는 명확하고 사용 의도가 잘 드러납니다.도메인에 맞는 상수 이름과
reasonText필드, Lombok 애너테이션 구성 모두 자연스럽습니다.
다만reasonText문자열이 실제 Zoomleave_reason값과 정확히 매칭된다는 전제에 강하게 의존하므로, 공식 문서나 실제 웹훅 샘플 payload 기준으로 한 번 더 값과 철자를 검증해 두면 안정성이 높아질 것 같습니다.techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookPlainTokenException.java (1)
1-12: LGTM! 예외 클래스 구현이 올바릅니다.Zoom 웹훅 처리 시 plainToken 누락에 대한 전용 예외 클래스가 적절하게 구현되었습니다. 코드베이스의 다른 예외 클래스와 일관된 패턴을 따르고 있으며, Javadoc 주석도 포함되어 있습니다.
techeerzip/src/main/java/backend/techeerzip/global/exception/ErrorCode.java (2)
228-229: LGTM! enum 항목 추가를 위한 올바른 포맷 변경입니다.GITHUB_SERVER_ERROR의 세미콜론을 콤마로 변경하여 추가 enum 항목을 허용하도록 했습니다.
230-242: LGTM! Zoom 관련 에러코드가 잘 정의되었습니다.6개의 Zoom 웹훅 관련 에러코드가 적절하게 추가되었습니다:
- 에러코드 번호가 순차적(ZOOM001-ZOOM006)이고 일관성 있게 명명되었습니다
- HTTP 상태코드가 각 에러 유형에 적합합니다 (설정/내부 오류는 500, 인증 실패는 401, 필수 데이터 누락은 400)
- 한글 에러 메시지가 명확하고 적절한 수준의 정보를 제공합니다
현재 5개의 에러코드(ZOOM_WEBHOOK_SECRET_TOKEN_NOT_CONFIGURED, ZOOM_TOKEN_ENCRYPTION_FAILED, ZOOM_WEBHOOK_AUTHENTICATION_FAILED, ZOOM_WEBHOOK_PROCESSING_FAILED, ZOOM_WEBHOOK_PLAIN_TOKEN_MISSING)는 대응하는 예외 클래스에서 활발히 사용 중이며, ZOOM_OAUTH_TOKEN_GENERATION_FAILED는 향후 OAuth 토큰 생성 기능 구현을 위한 선제적 정의로 보입니다.
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java (6)
1-22: 임포트 구성이 적절합니다.필요한 모든 의존성이 올바르게 임포트되어 있습니다.
24-32: 클래스 구조가 올바릅니다.
OncePerRequestFilter확장과 의존성 주입 구성이 적절합니다.
55-55: Body 캐싱이 올바르게 구현되었습니다.이전 리뷰의 피드백을 반영하여
readAllBytes()를 호출하여 body를 캐싱하도록 수정되었습니다.
82-88: 필터에서 발생한 예외 처리를 확인하세요.
ZoomWebhookProcessingException(InfraException 상속)이 필터에서 발생하면@RestControllerAdvice가 아닌 서블릿 컨테이너의 에러 핸들링으로 처리됩니다. Spring Security 필터 체인에서 발생한 예외는 컨트롤러에 도달하기 전에 발생하므로GlobalExceptionHandler에서 처리되지 않을 수 있습니다.필터 예외 처리가 의도대로 작동하는지 테스트를 통해 확인하거나, 필터 체인에 별도의 예외 처리 로직을 추가하는 것을 고려하세요. Spring Security의
AuthenticationEntryPoint나 별도의 에러 처리 필터 사용을 검토해보세요.
91-106: 이벤트 타입 추출 로직이 올바릅니다.캐시된 body에서 이벤트 타입을 안전하게 추출하며, 에러 처리도 적절합니다.
108-131: 웹훅 인증 검증 로직이 올바릅니다.Zoom이 Bearer 접두사 없이 토큰을 전송한다는 점을 적절히 처리하고 있으며, 검증 로직이 정확합니다.
techeerzip/src/main/java/backend/techeerzip/infra/zoom/dto/ZoomWebhookEvent.java (1)
14-15:@JsonIgnoreProperties(ignoreUnknown = true)적용으로 Zoom 스키마 변경에 대한 내성이 좋아졌습니다.루트/내부 DTO 모두에
ignoreUnknown = true를 둔 덕분에 Zoom 쪽에서 필드를 추가해도 파싱 실패 위험이 줄어들어 운영 안정성 측면에서 좋은 선택 같습니다.Also applies to: 30-31, 45-46, 78-79
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookService.java (1)
42-45: participantUuid 길이가 8자 미만일 때substring(0, 8)에서 예외 발생 가능
participantUuid가 null은 아니지만 길이가 8자보다 짧은 경우 현재 구현은StringIndexOutOfBoundsException을 발생시킬 수 있습니다. 로그용 필드라서 예외로 전체 Webhook 처리가 실패하는 것은 리스크가 크니, 길이 체크를 포함한 방어 코드를 넣는 것이 좋겠습니다.아래와 같이 수정하면 안전하게 동작합니다:
- String participantUuidLogMsg = - participantUuid != null ? participantUuid.substring(0, 8) + "..." : "없음"; + String participantUuidLogMsg = + participantUuid != null + ? participantUuid.substring(0, Math.min(8, participantUuid.length())) + "..." + : "없음";이전 리뷰에서도 동일한 지적이 있었는데, 현재 커밋에는 아직 반영되지 않은 것 같아 다시 한 번 남깁니다.
🧹 Nitpick comments (5)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java (3)
62-62: 패턴 매칭 변수명을 camelCase로 변경하세요.변수명
contentcachingrequestwrapper가 Java 명명 규칙을 위반합니다.다음 diff를 적용하여 수정하세요:
- if (request instanceof ContentCachingRequestWrapper contentcachingrequestwrapper) { - wrappedRequest = contentcachingrequestwrapper; + if (request instanceof ContentCachingRequestWrapper cachedRequest) { + wrappedRequest = cachedRequest;
82-85: 헤더 조회 방식을 개선할 수 있습니다.현재 방식도 작동하지만, HTTP 헤더는 명세상 대소문자를 구분하지 않으므로 더 간결한 방법을 사용할 수 있습니다.
다음 diff를 적용하여 개선하세요:
- // 일반 웹훅 이벤트는 인증 검증 - String authHeader = request.getHeader("authorization"); - if (authHeader == null) { - authHeader = request.getHeader("Authorization"); - } + // 일반 웹훅 이벤트는 인증 검증 (헤더는 대소문자 구분 없음) + String authHeader = request.getHeader("Authorization");참고: Servlet API의
getHeader()는 이미 대소문자를 구분하지 않으므로 한 번만 호출하면 됩니다.
121-144: 웹훅 인증 검증 로직이 올바르게 구현되었습니다.토큰 설정 확인, null 체크, 토큰 비교 등 모든 보안 검증 단계가 적절하게 포함되어 있습니다.
선택적 보안 강화: Timing attack을 방지하려면
MessageDigest.isEqual()을 사용할 수 있습니다:// Zoom은 Bearer 접두사 없이 토큰만 보냄 String token = authHeader.trim(); - boolean isValid = zoomApiConfig.getWebhookVerificationToken().equals(token); + boolean isValid = MessageDigest.isEqual( + zoomApiConfig.getWebhookVerificationToken().getBytes(StandardCharsets.UTF_8), + token.getBytes(StandardCharsets.UTF_8));필요한 import 추가:
import java.nio.charset.StandardCharsets; import java.security.MessageDigest;참고: Webhook verification token은 일반적으로 timing attack에 덜 취약하므로 이는 선택적 강화입니다.
techeerzip/src/main/java/backend/techeerzip/global/config/SecurityConfig.java (2)
42-66: Zoom Webhook 전용 체인 구성은 좋지만 기본 접근 정책을 명시하면 더 명확합니다.Zoom Webhook 을 별도 SecurityFilterChain 으로 분리하고, STATELESS + CSRF 비활성화 + 전용 필터를 둔 방향은 적절해 보입니다. 다만 현재 체인은
authorizeHttpRequests/formLogin/httpBasic설정을 사용하지 않아, Spring Security 기본값에 의존한다는 점이 읽는 사람에게는 다소 불명확합니다.Webhook 용 체인임을 더 분명히 하기 위해, 아래처럼 정책을 명시하는 것을 권장합니다(실제 의도에 따라
authenticated()vspermitAll()중 선택):http .securityMatcher(zoomWebhookPath + "/**") .csrf(AbstractHttpConfigurer::disable) .sessionManagement(s -> s.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) .authorizeHttpRequests(auth -> auth.anyRequest().authenticated()) // 또는 permitAll() .addFilterBefore(zoomWebhookFilter, UsernamePasswordAuthenticationFilter.class) .exceptionHandling(ex -> ex.authenticationEntryPoint(customAuthenticationEntryPoint)) .formLogin(AbstractHttpConfigurer::disable) .httpBasic(AbstractHttpConfigurer::disable);이렇게 하면 기본 로그인/Basic 인증 헤더가 Zoom Webhook 응답에 섞이지 않고, 체인의 의도도 더 분명해집니다.
32-41:zoomWebhookPath활용을 일원화하면 설정 실수를 줄일 수 있습니다.
zoom.webhook.path프로퍼티를zoomWebhookPath로 받아서.securityMatcher(zoomWebhookPath + "/**")에 사용하면서,
apiSecurityFilterChain쪽에서는 여전히"/api/v3/zoom/webhook/**"를 하드코딩해permitAll()로 두고 있습니다. 지금은 기본값이 같아서 문제는 없지만,
- 프로퍼티를 다른 경로로 변경할 때 두 곳을 모두 수정해야 하고,
- 실수로 프로퍼티에
"/api/v3/zoom/webhook/**"같이/**를 포함해 넣으면securityMatcher(zoomWebhookPath + "/**")가 깨질 수 있습니다.아래처럼
requestMatchers쪽에서도zoomWebhookPath를 재사용하면 유지보수성이 좋아집니다..requestMatchers( "/api/v3/auth/email", // ... zoomWebhookPath + "/**", "/actuator/prometheus") .permitAll();또한
zoom.webhook.path프로퍼티에는 와일드카드(/**)를 포함하지 않는다는 전제를 주석이나 yml 예시로 명시해 두면, 설정 단계에서의 혼선을 줄일 수 있을 것 같습니다.Also applies to: 129-146
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
techeerzip/src/main/java/backend/techeerzip/global/config/SecurityConfig.java(3 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookService.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java (7)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookInvalidAuthenticationException.java (1)
ZoomWebhookInvalidAuthenticationException(10-21)techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookProcessingException.java (1)
ZoomWebhookProcessingException(7-12)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookService.java (1)
Slf4j(14-79)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomTokenProvider.java (1)
Slf4j(15-56)techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java (1)
Slf4j(18-155)techeerzip/src/main/java/backend/techeerzip/global/exception/GlobalExceptionHandler.java (1)
Slf4j(18-171)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookController.java (1)
Slf4j(14-34)
⏰ 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 (7)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookService.java (1)
21-36: URL 검증 로직과 예외 처리 구성이 명확합니다.
plainToken에 대해 null/blank 체크 후 도메인 예외를 던지고, 암호화까지 이어지는 플로우가 깔끔합니다. 현재 구현은 그대로 사용해도 무방해 보입니다.techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java (5)
23-38: 클래스 구조가 올바르게 설계되었습니다.
OncePerRequestFilter상속과 불변 필드 사용으로 스레드 안전성을 확보했으며, 생성자를 통한 의존성 주입이 적절하게 구현되었습니다.
40-45: 경로 필터링 로직이 올바릅니다.웹훅 경로에만 필터가 적용되도록 정확하게 구현되었습니다.
60-68: 이전 리뷰의 critical issue가 올바르게 수정되었습니다.Line 68에서
readAllBytes()를 호출하여 body를 강제로 캐싱하도록 구현했습니다. 이제getContentAsByteArray()가 올바르게 데이터를 반환합니다.
104-119: 이벤트 타입 추출 로직이 안전하게 구현되었습니다.빈 body와 파싱 실패에 대한 적절한 예외 처리가 포함되어 있으며, null 반환으로 안전하게 fallback 됩니다.
95-97: No changes needed — AuthenticationException handling is correctly configured.The code properly handles
ZoomWebhookInvalidAuthenticationExceptionthrough Spring Security. The custom exception correctly extendsAuthenticationException, andSecurityConfigregisterscustomAuthenticationEntryPointin thezoomSecurityFilterChainexception handling (line 62–63). Re-throwing the exception allowsExceptionTranslationFilterto process it, which is the correct pattern.techeerzip/src/main/java/backend/techeerzip/global/config/SecurityConfig.java (1)
10-10: CSRF 비활성화 방식을AbstractHttpConfigurer::disable로 통일한 변경은 적절합니다.Swagger 체인과 API 체인 모두에서 CSRF 를 비활성화하는 방식을
AbstractHttpConfigurer::disable로 정리한 것은 Spring Security 최신 스타일과도 잘 맞고, 기존csrf(csrf -> csrf.disable())와 동작 차이도 없어 보입니다. 이 부분은 그대로 유지하셔도 무방해 보입니다.Also applies to: 75-75, 149-149
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java (3)
60-66: 패턴 변수 네이밍 컨벤션 개선 권장Pattern matching의 변수명
contentcachingrequestwrapper가 Java 네이밍 컨벤션(camelCase)을 따르지 않습니다.다음 diff를 적용하여 가독성을 개선하세요:
- if (request instanceof ContentCachingRequestWrapper contentcachingrequestwrapper) { - wrappedRequest = contentcachingrequestwrapper; + if (request instanceof ContentCachingRequestWrapper cached) { + wrappedRequest = cached; } else { wrappedRequest = new ContentCachingRequestWrapper(request); }
86-89: 불필요한 대소문자 처리 코드 제거 권장Servlet 스펙에 따르면
HttpServletRequest.getHeader()는 이미 대소문자를 구분하지 않습니다. Lines 86-89의 조건부 헤더 조회는 불필요합니다.다음 diff를 적용하여 코드를 단순화하세요:
- // 일반 웹훅 이벤트는 인증 검증 - String authHeader = request.getHeader("authorization"); - if (authHeader == null) { - authHeader = request.getHeader("Authorization"); - } + // 일반 웹훅 이벤트는 인증 검증 + String authHeader = request.getHeader("Authorization");
68-72: 요청 body 크기 제한 고려 권장Line 72에서
readAllBytes()를 호출할 때 요청 body 크기에 대한 제한이 없습니다. 대용량 페이로드로 인한 메모리 이슈를 방지하기 위해 application 레벨이나 웹 서버 레벨에서 요청 크기 제한을 설정하는 것을 권장합니다.예:
application.yml에서spring: servlet: multipart: max-request-size: 10MB server: tomcat: max-http-form-post-size: 10MB
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java (2)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookInvalidAuthenticationException.java (1)
ZoomWebhookInvalidAuthenticationException(10-21)techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookProcessingException.java (1)
ZoomWebhookProcessingException(7-12)
⏰ 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 (4)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java (4)
23-38: LGTM! 클린한 필터 구조OncePerRequestFilter를 상속받아 웹훅 필터를 구현한 구조가 적절하며, 생성자를 통한 의존성 주입으로 테스트 가능성도 확보되어 있습니다.
40-45: 경로 기반 필터링 적절함webhookPath를 기준으로 필터 적용 여부를 결정하는 로직이 명확하고 정확합니다.
68-76: 과거 리뷰 이슈 수정 확인됨이전 리뷰에서 지적된 ContentCachingRequestWrapper 캐싱 문제가 올바르게 수정되었습니다. Line 72에서
readAllBytes()를 호출하여 body를 먼저 캐싱한 후 Line 75-76에서 캐시된 내용을 사용하고 있습니다.
108-148: 헬퍼 메서드 구현 양호
extractEventType과isValidWebhook모두 적절한 방어 코딩과 에러 처리를 포함하고 있으며, 로깅도 잘 구현되어 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java (1)
28-38: 의존성 주입 패턴을 일관되게 유지하세요.
@RequiredArgsConstructor로 생성자 주입을 사용하면서@Value로 필드 주입을 혼용하고 있습니다. 테스트 용이성과 일관성을 위해webhookPath도 생성자 주입으로 변경하는 것을 권장합니다.다음 diff를 적용하여 생성자 주입으로 통일할 수 있습니다:
@Slf4j @Component -@RequiredArgsConstructor public class ZoomWebhookAuthenticationFilter extends OncePerRequestFilter { private static final String URL_VALIDATION_EVENT = ZoomWebhookEventType.URL_VALIDATION.getEventName(); private final ZoomApiConfig zoomApiConfig; private final ObjectMapper objectMapper; + private final String webhookPath; - @Value("${zoom.webhook.path:/api/v3/zoom/webhook}") - private String webhookPath; + public ZoomWebhookAuthenticationFilter( + ZoomApiConfig zoomApiConfig, + ObjectMapper objectMapper, + @Value("${zoom.webhook.path:/api/v3/zoom/webhook}") String webhookPath) { + this.zoomApiConfig = zoomApiConfig; + this.objectMapper = objectMapper; + this.webhookPath = webhookPath; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
techeerzip/src/main/java/backend/techeerzip/global/config/SecurityConfig.java(4 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- techeerzip/src/main/java/backend/techeerzip/global/config/SecurityConfig.java
🔇 Additional comments (5)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAuthenticationFilter.java (5)
40-45: LGTM!웹훅 경로에 대해서만 필터를 적용하는 로직이 명확하고 정확합니다.
54-67: Body 캐싱 구현이 정확합니다.
readAllBytes()를 호출하여ContentCachingRequestWrapper에 body를 캐싱하는 로직이 올바르게 구현되었습니다. 이전 리뷰에서 지적된 문제가 해결되었습니다.더 명확한 의도 표현을 위해 다음과 같이 변수에 할당하는 것도 고려해볼 수 있습니다:
// 스트림을 읽어서 ContentCachingRequestWrapper에 캐시되도록 함 // 이렇게 하면 이후 getContentAsByteArray()로 캐시된 내용을 읽을 수 있고, // 컨트롤러에서도 캐시된 내용을 읽을 수 있음 - wrappedRequest.getInputStream().readAllBytes(); + byte[] cachedBody = wrappedRequest.getInputStream().readAllBytes();
69-77: LGTM!URL 검증 이벤트를 인증 없이 통과시키는 로직이 Zoom 웹훅 스펙에 맞게 올바르게 구현되었습니다.
79-91: 인증 검증 로직이 올바르게 구현되었습니다.웹훅 토큰 검증 및 인증 실패 시 예외 처리가 정확합니다. wrappedRequest를 전달하여 downstream에서 body를 다시 읽을 수 있도록 한 것도 올바릅니다.
참고: HTTP 헤더는 case-insensitive이므로 servlet API가 자동으로 처리하지만, 방어적 코딩으로 양쪽을 체크하는 것은 문제없습니다.
102-142: LGTM!이벤트 타입 추출 및 웹훅 토큰 검증 로직이 모두 정확하게 구현되었습니다.
extractEventType: 캐시된 body에서 JSON 파싱 후 event 필드를 안전하게 추출합니다.isValidWebhook: verification token의 존재 여부와 값을 case-sensitive하게 비교하여 보안을 유지합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAspect.java (2)
29-29: 메서드 이름을 동사 형태로 변경하는 것을 권장합니다.Java 메서드 명명 규칙에 따라
validationZoomToken보다는validateZoomToken이 더 적절합니다.- public void validationZoomToken(JoinPoint joinPoint) { + public void validateZoomToken(JoinPoint joinPoint) {
47-50: Authorization 헤더 조회 로직을 단순화할 수 있습니다.HTTP 헤더는 대소문자를 구분하지 않으며, Servlet API 명세에 따르면
HttpServletRequest.getHeader()는 명시적으로 대소문자 구분 없이 동작합니다(RFC 2616 §4.2, RFC 7230 준수). 현재의 이중 체크는 불필요합니다.단일 호출로 단순화하는 것을 권장합니다:
- String authHeader = - request.getHeader("authorization") != null - ? request.getHeader("authorization") - : request.getHeader("Authorization"); + String authHeader = request.getHeader("Authorization");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
techeerzip/src/main/java/backend/techeerzip/global/config/SecurityConfig.java(3 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomAuth.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAspect.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookController.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- techeerzip/src/main/java/backend/techeerzip/global/config/SecurityConfig.java
- techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookController.java
🧰 Additional context used
🧬 Code graph analysis (1)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAspect.java (2)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/exception/ZoomWebhookInvalidAuthenticationException.java (1)
ZoomWebhookInvalidAuthenticationException(10-21)techeerzip/src/main/java/backend/techeerzip/infra/zoom/webhook/ZoomWebhookController.java (1)
Slf4j(15-36)
⏰ 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 (6)
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomAuth.java (1)
1-10: LGTM! 잘 정의된 마커 어노테이션입니다.Zoom webhook 인증을 위한 마커 어노테이션이 올바르게 정의되었습니다.
METHOD타겟과RUNTIME보존 정책이 AOP aspect에서의 사용에 적합합니다.techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAspect.java (5)
20-26: LGTM! Aspect 구성이 올바릅니다.Spring AOP aspect가 적절한 어노테이션으로 구성되었습니다. Lombok을 통한 의존성 주입도 표준 패턴을 따릅니다.
41-44: LGTM! URL 검증 요청 처리가 올바릅니다.URL 검증 이벤트에 대해 인증을 건너뛰는 로직이 Zoom webhook 명세에 맞게 구현되었습니다.
52-55: LGTM! 인증 검증 및 예외 처리가 적절합니다.유효하지 않은 webhook 인증에 대해 적절한 예외를 발생시키고 있으며, 로깅도 적절합니다.
58-81: LGTM! Webhook 검증 로직이 잘 구현되었습니다.검증 토큰 확인, 헤더 존재 여부, 토큰 일치 여부를 단계별로 검증하고 있으며, 각 실패 케이스에 대한 로깅도 적절합니다. Zoom이 Bearer 접두사 없이 토큰을 전송한다는 주석도 유용합니다.
30-32: RequestContextHolder usage is safe in current context but consider request context assumptions.
RequestContextHolder.currentRequestAttributes()will throwIllegalStateExceptionif called outside a servlet request context. Currently,@ZoomAuthis used exclusively on theZoomWebhookController.handleWebhookEvent()method, which runs in a servlet context where a request is always available. However, if@ZoomAuthis applied to non-servlet endpoints (e.g., async, message handlers) in the future, this will fail. Consider adding error handling or documentation to prevent misuse of this annotation outside servlet contexts.
techeerzip/src/main/java/backend/techeerzip/infra/zoom/config/ZoomWebhookAspect.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
techeerzip/src/main/java/backend/techeerzip/domain/zoom/repository/ZoomAttendanceRepository.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java(3 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: build
🔇 Additional comments (3)
techeerzip/src/main/java/backend/techeerzip/domain/zoom/repository/ZoomAttendanceRepository.java (1)
40-44: 날짜 경계 문제 해결을 위한 메서드 추가 승인날짜 제약 없이
participantUuid로만 활성 세션을 조회하는 메서드가 추가되어, 자정을 넘어가는 세션 처리가 가능해졌습니다. JPQL 쿼리 구문과 메서드 시그니처 모두 올바르게 작성되었습니다.techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java (2)
86-137: 퇴장 이벤트 처리 로직 승인
handleParticipantLeft메서드가 올바르게 구현되었습니다:
- 날짜 제약 없는
findActiveSessionByParticipantUuid를 사용하여 자정 경계 문제 해결ZoomLeaveReason.isCompleteExit로 완전 퇴장만 필터링하여 소회의실 이동 등 무시- null 체크와 예외 처리가 적절하게 구현됨
140-151: 시간 파싱 실패 시 fallback 동작 검토 권장현재
parseZoomDateTime은 파싱 실패 시LocalDateTime.now()를 반환합니다. Zoom에서 잘못된 형식의 타임스탬프를 전송하는 경우, 실제 입/퇴장 시간이 아닌 현재 시각으로 기록이 생성되어 데이터 정합성 문제가 발생할 수 있습니다.프로덕션 환경에서 이러한 케이스가 발생하는지 모니터링하고, 필요 시 예외를 상위로 전파하여 웹훅 처리를 실패시키는 방안을 고려하세요.
techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
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)
techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java (1)
139-150: 파싱 실패 시 잘못된 데이터 생성 위험
parseZoomDateTime이 파싱에 실패하면LocalDateTime.now()로 폴백하여 잘못된 타임스탬프로 출석 기록을 생성합니다. 이는 출석 데이터 정확성에 심각한 영향을 미칩니다.폴백 대신 예외를 던져서 상위 호출자의
catch블록에서 전체 이벤트 처리를 중단하도록 하는 것이 좋습니다.다음 수정을 적용하세요:
private LocalDateTime parseZoomDateTime(String dateTimeStr) { - try { - // Zoom 형식: "2025-01-20T13:30:15Z" - if (dateTimeStr.endsWith("Z")) { - dateTimeStr = dateTimeStr.substring(0, dateTimeStr.length() - 1); - } - return LocalDateTime.parse(dateTimeStr, DateTimeFormatter.ISO_LOCAL_DATE_TIME); - } catch (Exception e) { - log.error("시간 파싱 오류: {}", dateTimeStr, e); - return LocalDateTime.now(); // fallback - } + // Zoom 형식: "2025-01-20T13:30:15Z" + if (dateTimeStr.endsWith("Z")) { + dateTimeStr = dateTimeStr.substring(0, dateTimeStr.length() - 1); + } + return LocalDateTime.parse(dateTimeStr, DateTimeFormatter.ISO_LOCAL_DATE_TIME); }
🧹 Nitpick comments (2)
techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java (2)
76-76: UUID 로깅 시 방어 코드 추가 권장
participantUuid.substring(0, 8)는 UUID가 8자보다 짧으면StringIndexOutOfBoundsException을 발생시킬 수 있습니다. 실제 UUID는 항상 36자이므로 발생 가능성은 낮지만, 방어적 코딩을 위해 길이 체크를 추가하는 것이 좋습니다.동일한 패턴이 line 129에도 있습니다.
다음과 같이 개선할 수 있습니다:
- participantUuid.substring(0, 8) + "...", + participantUuid.length() > 8 ? participantUuid.substring(0, 8) + "..." : participantUuid,
56-59: 주석과 구현 불일치주석은 "같은 날짜에 다른 participant_uuid로 기록이 있을 수 있음"을 언급하며 완료된 세션 확인을 설명하지만, 실제로는 이러한 체크가 구현되어 있지 않습니다. 코드는 단순히 새로운 출석 기록을 생성합니다.
주석을 제거하거나 실제 동작을 반영하도록 수정하세요.
다음과 같이 주석을 명확히 할 수 있습니다:
-// 완전히 나갔다 다시 들어온 경우 (새로운 participant_uuid) 또는 첫 입장 -// 기존에 완료된 세션이 있는지 확인 (같은 날짜에 다른 participant_uuid로 기록이 있을 수 있음) -// 하지만 participant_uuid가 다르면 새로운 세션이므로 별도 엔티티로 저장 - +// 새로운 participant_uuid이므로 별도의 출석 기록 생성 +// (완전히 나갔다 다시 들어온 경우 또는 첫 입장)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java(3 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: build
🔇 Additional comments (3)
techeerzip/src/main/java/backend/techeerzip/domain/zoom/service/ZoomAttendanceService.java (3)
45-46: 이전 리뷰 이슈 해결 확인이전 리뷰에서 지적된 날짜 경계 문제가 해결되었습니다. 이제
handleParticipantJoined와handleParticipantLeft모두 날짜 제약 없는findActiveSessionByParticipantUuid를 사용하여 자정을 넘어가는 세션을 올바르게 처리합니다.
33-36: 타임스탬프 null 체크 적절함필수 타임스탬프 정보가 누락된 경우 경고 로그를 남기고 조기 반환하는 처리가 적절합니다. 타임스탬프 없이는 출석 기록을 생성할 수 없으므로 이러한 방어 로직이 합리적입니다.
Also applies to: 91-94
96-103: 퇴장 사유 필터링 로직 우수
ZoomLeaveReason.isCompleteExit를 사용하여 완전한 퇴장만 처리하고 소회의실 이동이나 대기실 관련 이벤트를 제외하는 로직이 우수합니다. 디버그 로깅도 적절하게 추가되어 있습니다.
|
요약
작업 내용
참고 사항
관련 이슈
BACKEND-
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.