[FEAT] Toss Payments Webhook 연동 및 상태 동기화 로직 구현#95
Conversation
📝 WalkthroughWalkthroughAdds Toss Payments webhook support: a new REST controller receives raw webhook POSTs, verifies signatures via TossPaymentService, deserializes and validates payloads into PaymentWebhookDTO, and delegates processing to PaymentService.processWebhook which maps external statuses and updates Payment entities (with optimistic locking). Changes
Sequence Diagram(s)sequenceDiagram
participant Client as External Payment System
participant Controller as PaymentWebhookController
participant Verifier as TossPaymentService
participant Service as PaymentService
participant DB as Database
Client->>Controller: POST /api/v1/payments/webhook\n(raw JSON + headers)
activate Controller
Controller->>Verifier: verifyWebhookSignature(json, signature, timestamp)
alt signature valid
Controller->>Controller: ObjectMapper.parse(json) -> PaymentWebhookDTO\nValidator.validate(dto)
alt validation ok
Controller->>Service: processWebhook(dto)
else validation fail
Controller-->>Client: HTTP 400 (validation error)
end
else invalid signature
Controller-->>Client: HTTP 401 (invalid signature)
end
deactivate Controller
activate Service
Service->>Service: map external status -> internal PaymentStatus
alt target status equals current
Service-->>DB: no-op (idempotent)
else needs transition
alt COMPLETED
Service->>Service: completePayment(...)
else REFUNDED
Service->>Service: cancelPayment(...)
end
Service->>DB: update payment record (optimistic locking via version)
end
deactivate Service
Service-->>Controller: processing result
Controller-->>Client: HTTP 200 (ack / ignored / error text)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This pull request implements Toss Payments webhook integration to synchronize payment status even when clients disconnect or encounter network errors during the payment confirmation process.
Changes:
- Added webhook endpoint to receive payment status updates from Toss Payments
- Implemented webhook payload DTO with payment key, order ID, and status information
- Created service method to process webhooks and update payment status accordingly
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| PaymentWebhookDTO.java | New DTO to receive webhook payload from Toss Payments with payment key, order ID, status, and event type |
| PaymentWebhookController.java | New REST controller to handle POST requests from Toss Payments webhook |
| PaymentService.java | Added processWebhook method to synchronize payment status based on webhook data |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| public record PaymentWebhookDTO( | ||
| String paymentKey, | ||
| String orderId, | ||
| String status, |
There was a problem hiding this comment.
Missing input validation constraints on webhook payload fields. All critical fields (paymentKey, orderId, status) should have @NotNull or @notblank annotations to ensure they are present in the webhook payload. This is consistent with other DTOs in the codebase (e.g., PaymentConfirmDTO at src/main/java/com/eatsfine/eatsfine/domain/payment/dto/request/PaymentConfirmDTO.java:9-12 uses @NotNull). Without validation, the service method will fail with NullPointerException if these fields are missing.
| @JsonIgnoreProperties(ignoreUnknown = true) | |
| public record PaymentWebhookDTO( | |
| String paymentKey, | |
| String orderId, | |
| String status, | |
| import javax.validation.constraints.NotNull; | |
| @JsonIgnoreProperties(ignoreUnknown = true) | |
| public record PaymentWebhookDTO( | |
| @NotNull String paymentKey, | |
| @NotNull String orderId, | |
| @NotNull String status, |
| public ResponseEntity<String> handleWebhook(@RequestBody PaymentWebhookDTO dto) { | ||
| log.info("Webhook received: orderId={}, status={}", dto.orderId(), dto.status()); | ||
| paymentService.processWebhook(dto); | ||
| return ResponseEntity.ok("Received"); | ||
| } |
There was a problem hiding this comment.
Missing webhook signature verification creates a critical security vulnerability. Toss Payments webhooks should be authenticated to prevent malicious actors from spoofing payment status updates. Without signature verification, an attacker could send fake webhook requests to mark payments as completed or refunded. Toss Payments provides webhook signatures that should be validated before processing. This is a critical security requirement for production webhook endpoints.
| payment.completePayment( | ||
| LocalDateTime.now(), | ||
| PaymentMethod.SIMPLE_PAYMENT, |
There was a problem hiding this comment.
Hardcoded PaymentMethod.SIMPLE_PAYMENT may not match the actual payment method used. In the confirmPayment method (lines 110-116), the payment method is set based on the Toss API response, but here it's hardcoded. The webhook payload may contain method information that should be parsed similarly to how it's done in confirmPayment to ensure consistency. This could lead to data inconsistencies where the payment method stored differs depending on whether it was set via the confirm API or the webhook.
| payment.completePayment( | |
| LocalDateTime.now(), | |
| PaymentMethod.SIMPLE_PAYMENT, | |
| // Determine payment method from existing payment or webhook payload, instead of hardcoding. | |
| PaymentMethod paymentMethod = payment.getPaymentMethod(); | |
| try { | |
| // If the webhook provides a method value, try to map it to the PaymentMethod enum. | |
| // If dto.method() is null or invalid, we keep the existing payment method. | |
| if (dto.method() != null) { | |
| paymentMethod = PaymentMethod.valueOf(dto.method()); | |
| } | |
| } catch (IllegalArgumentException e) { | |
| log.warn("Unknown payment method '{}' in webhook for order {}. Keeping existing method {}.", | |
| dto.method(), dto.orderId(), paymentMethod, e); | |
| } | |
| payment.completePayment( | |
| LocalDateTime.now(), | |
| paymentMethod, |
| if (targetStatus == PaymentStatus.COMPLETED) { | ||
| payment.completePayment( | ||
| LocalDateTime.now(), | ||
| PaymentMethod.SIMPLE_PAYMENT, | ||
| dto.paymentKey(), | ||
| null, | ||
| null); | ||
| log.info("Webhook processed: Payment {} status updated to COMPLETED", dto.orderId()); |
There was a problem hiding this comment.
Missing payment amount validation creates a security risk. The confirmPayment method validates that the payment amount matches the expected amount (line 80), but the webhook processing does not perform this check. This could allow an attacker (if webhook signature verification is added) to complete a payment with a different amount than initially requested. The webhook should validate that the amount in the Toss webhook matches the stored payment amount before updating the status.
| public void processWebhook(PaymentWebhookDTO dto) { | ||
| Payment payment = paymentRepository.findByOrderId(dto.orderId()) | ||
| .orElseThrow(() -> new PaymentException(PaymentErrorStatus._PAYMENT_NOT_FOUND)); | ||
|
|
||
| PaymentStatus targetStatus = null; | ||
| if ("DONE".equals(dto.status())) { | ||
| targetStatus = PaymentStatus.COMPLETED; | ||
| } else if ("CANCELED".equals(dto.status())) { | ||
| targetStatus = PaymentStatus.REFUNDED; | ||
| } | ||
|
|
||
| if (targetStatus == null) { | ||
| log.info("Webhook skipped: Unknown or unhandled status {}", dto.status()); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The eventType field in PaymentWebhookDTO is not being used or validated. According to Toss Payments webhook documentation, the eventType field indicates the type of event (e.g., "PAYMENT_STATUS_CHANGED"). This field should be validated to ensure only expected event types are processed. Currently, the code only checks the status field, but different eventTypes may require different handling logic.
|
|
||
| @Operation(summary = "Toss Payments 웹훅 수신", description = "Toss Payments 서버로부터 결제/취소 결과(PaymentKey, Status 등)를 수신하여 서버 상태를 동기화합니다.") | ||
| @PostMapping | ||
| public ResponseEntity<String> handleWebhook(@RequestBody PaymentWebhookDTO dto) { |
There was a problem hiding this comment.
Missing @Valid annotation on @RequestBody parameter. The PaymentWebhookDTO should be validated using @Valid to trigger Jakarta Bean Validation on the DTO fields. This is consistent with other controllers in the codebase (e.g., PaymentController at src/main/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentController.java:30, 37, 45 all use @Valid). Without @Valid, even if validation annotations are added to PaymentWebhookDTO, they won't be enforced.
| if (payment.getPaymentStatus() == targetStatus) { | ||
| log.info("Webhook skipped: Payment {} already in status {}", dto.orderId(), targetStatus); | ||
| return; | ||
| } | ||
|
|
||
| if (targetStatus == PaymentStatus.COMPLETED) { | ||
| payment.completePayment( | ||
| LocalDateTime.now(), | ||
| PaymentMethod.SIMPLE_PAYMENT, | ||
| dto.paymentKey(), | ||
| null, | ||
| null); | ||
| log.info("Webhook processed: Payment {} status updated to COMPLETED", dto.orderId()); | ||
| } else if (targetStatus == PaymentStatus.REFUNDED) { | ||
| payment.cancelPayment(); | ||
| log.info("Webhook processed: Payment {} status updated to REFUNDED", dto.orderId()); | ||
| } |
There was a problem hiding this comment.
Missing validation of current payment status before state transition. The webhook only checks if the payment is already in the target status (line 242), but doesn't validate if the transition from the current status is valid. For example, a payment that is FAILED or REFUNDED should not be transitioned to COMPLETED. Consider adding state transition validation similar to a state machine pattern to ensure only valid status transitions are allowed (e.g., PENDING → COMPLETED, COMPLETED → REFUNDED, but not FAILED → COMPLETED).
| @PostMapping | ||
| public ResponseEntity<String> handleWebhook(@RequestBody PaymentWebhookDTO dto) { | ||
| log.info("Webhook received: orderId={}, status={}", dto.orderId(), dto.status()); | ||
| paymentService.processWebhook(dto); |
There was a problem hiding this comment.
Missing error handling in webhook endpoint. If processWebhook throws an exception (e.g., PaymentException when payment is not found), it will propagate to the client with a 500 status code. Webhook endpoints should handle errors gracefully and return appropriate HTTP status codes. Consider wrapping the service call in a try-catch block and returning 200 OK even for known errors (to prevent webhook retries for invalid data) while logging the error, or return appropriate 4xx codes for validation errors vs 5xx for actual server errors.
| paymentService.processWebhook(dto); | |
| try { | |
| paymentService.processWebhook(dto); | |
| } catch (Exception e) { | |
| log.error("Error processing payment webhook: orderId={}, status={}", dto.orderId(), dto.status(), e); | |
| // For webhook endpoints, returning 2xx even on known errors prevents unnecessary retries. | |
| return ResponseEntity.ok("Received"); | |
| } |
| @Transactional | ||
| public void processWebhook(PaymentWebhookDTO dto) { | ||
| Payment payment = paymentRepository.findByOrderId(dto.orderId()) | ||
| .orElseThrow(() -> new PaymentException(PaymentErrorStatus._PAYMENT_NOT_FOUND)); | ||
|
|
||
| PaymentStatus targetStatus = null; | ||
| if ("DONE".equals(dto.status())) { | ||
| targetStatus = PaymentStatus.COMPLETED; | ||
| } else if ("CANCELED".equals(dto.status())) { | ||
| targetStatus = PaymentStatus.REFUNDED; | ||
| } | ||
|
|
||
| if (targetStatus == null) { | ||
| log.info("Webhook skipped: Unknown or unhandled status {}", dto.status()); | ||
| return; | ||
| } | ||
|
|
||
| if (payment.getPaymentStatus() == targetStatus) { | ||
| log.info("Webhook skipped: Payment {} already in status {}", dto.orderId(), targetStatus); | ||
| return; | ||
| } | ||
|
|
||
| if (targetStatus == PaymentStatus.COMPLETED) { | ||
| payment.completePayment( | ||
| LocalDateTime.now(), | ||
| PaymentMethod.SIMPLE_PAYMENT, | ||
| dto.paymentKey(), | ||
| null, | ||
| null); | ||
| log.info("Webhook processed: Payment {} status updated to COMPLETED", dto.orderId()); | ||
| } else if (targetStatus == PaymentStatus.REFUNDED) { | ||
| payment.cancelPayment(); | ||
| log.info("Webhook processed: Payment {} status updated to REFUNDED", dto.orderId()); | ||
| } |
There was a problem hiding this comment.
Potential race condition between webhook and confirmPayment API calls. If both the webhook and the client's confirmPayment API call execute simultaneously (which is possible in edge cases), they could both try to update the payment status concurrently. While the current idempotency check (line 242) helps, it doesn't prevent both transactions from reading the same initial status and attempting updates. Consider adding optimistic locking (e.g., @Version in Payment entity) or using database-level locking to prevent concurrent modifications.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentWebhookController.java`:
- Around line 24-29: The controller currently processes webhooks without
verifying the Toss signature; update PaymentWebhookController.handleWebhook to
first verify the tosspayments-webhook-signature (and use
tosspayments-webhook-transmission-time) before calling
paymentService.processWebhook: read the raw request body (so the HMAC is
computed over the exact payload), construct the message
"{payload}:{transmissionTime}", compute HMAC-SHA256 with the configured Toss
secret, Base64-decode the signature after stripping the "v1:" prefix, and
perform a constant-time comparison; if verification fails return 401 and do not
call paymentService.processWebhook, otherwise deserialize payload into
PaymentWebhookDTO and proceed. Ensure configurable secret access (e.g., from
application properties) and clear reference to handleWebhook,
PaymentWebhookController, paymentService.processWebhook, and the header names
tosspayments-webhook-signature and tosspayments-webhook-transmission-time.
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/dto/request/PaymentWebhookDTO.java`:
- Around line 5-10: Add bean-validation annotations to the PaymentWebhookDTO
record to enforce required non-null/non-blank fields (e.g., annotate paymentKey,
orderId, status, eventType with `@NotBlank` or `@NotNull/`@NotBlank as appropriate)
so invalid/missing webhook payloads produce 400s; then update the
PaymentWebhookController.handleWebhook(...) method signature to annotate the
request parameter with `@Valid` (and keep `@RequestBody`) so validation is
triggered. Ensure javax.validation (or jakarta.validation) annotations are
imported and the controller method parameter uses `@Valid` on the
PaymentWebhookDTO type.
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java`:
- Around line 242-258: The current webhook handling applies transitions blindly
and can regress statuses (e.g., REFUNDED -> COMPLETED); add explicit
allowed-transition guards before calling payment.completePayment or
payment.cancelPayment by checking payment.getPaymentStatus() against permitted
source states for the targetStatus (e.g., only allow COMPLETED when current
status is PENDING/PROCESSING, only allow REFUNDED when current status is
COMPLETED), and if not allowed log a skipped transition and return; implement
this as a small helper or map of allowedTransitions referenced from this block
to keep logic centralized and testable (use the existing symbols
payment.getPaymentStatus(), targetStatus, PaymentStatus.COMPLETED,
PaymentStatus.REFUNDED, payment.completePayment, payment.cancelPayment).
- Around line 226-233: In processWebhook(PaymentWebhookDTO dto) add a guard that
verifies if the fetched Payment already has a paymentKey and that it matches
dto.paymentKey(); if payment.getPaymentKey() != null and
!payment.getPaymentKey().equals(dto.paymentKey()) then throw a PaymentException
with an appropriate error status (create or reuse a PaymentErrorStatus like
_PAYMENT_KEY_MISMATCH) instead of proceeding to update the payment; place this
check after retrieving the Payment from paymentRepository and before
assigning/updating the payment key or status.
src/main/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentWebhookController.java
Show resolved
Hide resolved
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| public record PaymentWebhookDTO( | ||
| String paymentKey, | ||
| String orderId, | ||
| String status, | ||
| String eventType) { |
There was a problem hiding this comment.
Add bean validation for required fields (and wire @Valid in the controller).
This prevents null/blank payloads from causing 500s during webhook handling and returns a clean 400 instead.
🔧 Proposed update
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import jakarta.validation.constraints.NotBlank;
`@JsonIgnoreProperties`(ignoreUnknown = true)
public record PaymentWebhookDTO(
- String paymentKey,
- String orderId,
- String status,
- String eventType) {
+ `@NotBlank` String paymentKey,
+ `@NotBlank` String orderId,
+ `@NotBlank` String status,
+ `@NotBlank` String eventType) {
}Also add @Valid to PaymentWebhookController.handleWebhook(...) to activate validation.
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/dto/request/PaymentWebhookDTO.java`
around lines 5 - 10, Add bean-validation annotations to the PaymentWebhookDTO
record to enforce required non-null/non-blank fields (e.g., annotate paymentKey,
orderId, status, eventType with `@NotBlank` or `@NotNull/`@NotBlank as appropriate)
so invalid/missing webhook payloads produce 400s; then update the
PaymentWebhookController.handleWebhook(...) method signature to annotate the
request parameter with `@Valid` (and keep `@RequestBody`) so validation is
triggered. Ensure javax.validation (or jakarta.validation) annotations are
imported and the controller method parameter uses `@Valid` on the
PaymentWebhookDTO type.
| public void processWebhook(PaymentWebhookDTO dto) { | ||
| Payment payment = paymentRepository.findByOrderId(dto.orderId()) | ||
| .orElseThrow(() -> new PaymentException(PaymentErrorStatus._PAYMENT_NOT_FOUND)); | ||
|
|
||
| PaymentStatus targetStatus = null; | ||
| if ("DONE".equals(dto.status())) { | ||
| targetStatus = PaymentStatus.COMPLETED; | ||
| } else if ("CANCELED".equals(dto.status())) { |
There was a problem hiding this comment.
Guard against mismatched paymentKey before updating the payment.
If a payment already has a key, accepting a different key from the webhook can corrupt state or bind the wrong transaction.
🛡️ Suggested guard
public void processWebhook(PaymentWebhookDTO dto) {
Payment payment = paymentRepository.findByOrderId(dto.orderId())
.orElseThrow(() -> new PaymentException(PaymentErrorStatus._PAYMENT_NOT_FOUND));
+
+ if (payment.getPaymentKey() != null && !payment.getPaymentKey().equals(dto.paymentKey())) {
+ log.warn("Webhook skipped: paymentKey mismatch for orderId {}", dto.orderId());
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void processWebhook(PaymentWebhookDTO dto) { | |
| Payment payment = paymentRepository.findByOrderId(dto.orderId()) | |
| .orElseThrow(() -> new PaymentException(PaymentErrorStatus._PAYMENT_NOT_FOUND)); | |
| PaymentStatus targetStatus = null; | |
| if ("DONE".equals(dto.status())) { | |
| targetStatus = PaymentStatus.COMPLETED; | |
| } else if ("CANCELED".equals(dto.status())) { | |
| public void processWebhook(PaymentWebhookDTO dto) { | |
| Payment payment = paymentRepository.findByOrderId(dto.orderId()) | |
| .orElseThrow(() -> new PaymentException(PaymentErrorStatus._PAYMENT_NOT_FOUND)); | |
| if (payment.getPaymentKey() != null && !payment.getPaymentKey().equals(dto.paymentKey())) { | |
| log.warn("Webhook skipped: paymentKey mismatch for orderId {}", dto.orderId()); | |
| return; | |
| } | |
| PaymentStatus targetStatus = null; | |
| if ("DONE".equals(dto.status())) { | |
| targetStatus = PaymentStatus.COMPLETED; | |
| } else if ("CANCELED".equals(dto.status())) { |
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java`
around lines 226 - 233, In processWebhook(PaymentWebhookDTO dto) add a guard
that verifies if the fetched Payment already has a paymentKey and that it
matches dto.paymentKey(); if payment.getPaymentKey() != null and
!payment.getPaymentKey().equals(dto.paymentKey()) then throw a PaymentException
with an appropriate error status (create or reuse a PaymentErrorStatus like
_PAYMENT_KEY_MISMATCH) instead of proceeding to update the payment; place this
check after retrieving the Payment from paymentRepository and before
assigning/updating the payment key or status.
src/main/java/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/service/TossPaymentService.java`:
- Around line 53-59: In verifyWebhookSignature, the payload is built incorrectly
and the signature check is vulnerable to timing attacks: build payload as
jsonBody + ":" + timestamp (body first, colon separator), split the incoming
signature header by commas to handle multiple "v1:<sig>" entries, for each entry
strip the "v1:" prefix and compute hmacSha256(payload, widgetSecretKey) once,
then compare the expected signature to each provided signature using a
constant-time comparison (MessageDigest.isEqual) and accept if any match; update
references to widgetSecretKey, hmacSha256, and verifyWebhookSignature
accordingly.
🧹 Nitpick comments (3)
src/main/java/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java (1)
277-286: Consider extracting provider parsing to a shared helper.The provider parsing logic is duplicated between
confirmPayment(lines 99-108) andprocessWebhook. A small helper method would reduce duplication.♻️ Example helper extraction
private PaymentProvider parseEasyPayProvider(String providerCode) { if ("토스페이".equals(providerCode)) { return PaymentProvider.TOSS; } else if ("카카오페이".equals(providerCode)) { return PaymentProvider.KAKAOPAY; } return null; }src/main/java/com/eatsfine/eatsfine/domain/payment/service/TossPaymentService.java (1)
62-67: Consider adding proper imports for crypto classes.Using fully qualified names inline works but reduces readability. Adding imports would be cleaner.
♻️ Suggested imports
import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; import java.nio.charset.StandardCharsets; import java.util.Base64;src/main/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentWebhookController.java (1)
29-32: Use import statement instead of fully qualified name.Line 30 uses a fully qualified class name for
TossPaymentServicewhile other dependencies use imports.♻️ Suggested fix
Add to imports section:
import com.eatsfine.eatsfine.domain.payment.service.TossPaymentService;Then change line 30:
- private final com.eatsfine.eatsfine.domain.payment.service.TossPaymentService tossPaymentService; + private final TossPaymentService tossPaymentService;
| public void verifyWebhookSignature(String jsonBody, String signature, String timestamp) throws Exception { | ||
| String payload = timestamp + "." + jsonBody; | ||
| String calculatedSignature = hmacSha256(payload, widgetSecretKey); | ||
|
|
||
| if (!signature.contains("v1:" + calculatedSignature)) { | ||
| throw new SecurityException("Signature verification failed"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Toss Payments webhook signature verification payload format HMAC
💡 Result:
For Toss Payments (토스페이먼츠) webhooks that include a signature (payout.changed, seller.changed), the HMAC input (“signed payload”) format is:
- message to sign:
{WEBHOOK_PAYLOAD}:{tosspayments-webhook-transmission-time} - HMAC algorithm:
HMAC-SHA256 - HMAC key: your Toss Payments 보안 키 (security key) (the webhook signing secret) (docs.tosspayments.com)
Signature header format
- Header:
tosspayments-webhook-signature - It contains two v1 signatures, e.g.:
v1:<sigA>,v1:<sigB> - Verification:
- Base64-decode both
<sigA>and<sigB> - Compute
HMAC-SHA256(key=securityKey, msg="{payload}:{transmissionTime}") - If your computed HMAC matches either decoded value, the webhook is valid (docs.tosspayments.com)
- Base64-decode both
Notes:
tosspayments-webhook-transmission-timeis the timestamp header you must append after:in the signed payload (docs.tosspayments.com)
Citations:
- 1: https://docs.tosspayments.com/reference/using-api/webhook-events
- 2: https://docs.tosspayments.com/reference/using-api/webhook-events
- 3: https://docs.tosspayments.com/reference/using-api/webhook-events
🌐 Web query:
Toss Payments tosspayments-webhook-transmission-time signature format documentation
💡 Result:
Toss Payments 웹훅에서 tosspayments-webhook-transmission-time / signature 형식은 아래처럼 문서에 정의돼 있습니다.
tosspayments-webhook-transmission-time: “웹훅이 발송된 시간” 헤더이며, 예시처럼 ISO 8601 타임스탬프 문자열로 옵니다. (docs.tosspayments.com)tosspayments-webhook-signature포함 범위: 모든 웹훅에 항상 오는 값이 아니라, 문서 기준으로payout.changed,seller.changed웹훅에만 포함됩니다. (docs.tosspayments.com)- signature 헤더 문자열 형식:
v1:<sig1>,v1:<sig2>처럼v1:프리픽스가 붙은 값 2개가 콤마로 구분되어 전달됩니다. (toss.tech) - 검증(verification) 데이터 포맷: 아래 문자열을 만들고 보안 키로 HMAC-SHA256을 계산합니다. (docs.tosspayments.com)
{WEBHOOK_PAYLOAD}:{tosspayments-webhook-transmission-time}
- 비교 방식: 헤더의 두
v1:값 각각에서v1:뒤를 떼어 base64 디코딩한 바이트 배열과, HMAC 결과를 비교해서 둘 중 하나라도 일치하면 유효합니다. (docs.tosspayments.com)
추가로, 개발자 커뮤니티 사례에서도 tosspayments-webhook-transmission-time, -id, -retried-count는 보이는데 signature가 없는 경우가 언급되며(즉 “항상 있는 헤더가 아님” 상황), 위 “payout/seller에만 포함” 규정과 맥락이 맞습니다. (techchat.tosspayments.com)
Citations:
- 1: https://docs.tosspayments.com/reference/using-api/webhook-events?utm_source=openai
- 2: https://docs.tosspayments.com/reference/using-api/webhook-events?utm_source=openai
- 3: https://toss.tech/article/payments-legacy-4?utm_source=openai
- 4: https://docs.tosspayments.com/reference/using-api/webhook-events?utm_source=openai
- 5: https://docs.tosspayments.com/reference/using-api/webhook-events?utm_source=openai
- 6: https://techchat.tosspayments.com/m/1400303384015405158?utm_source=openai
🏁 Script executed:
# Check the codebase for any Toss documentation references or comments
rg -i "toss|webhook|signature" src/main/java/com/eatsfine/eatsfine/domain/payment/service/TossPaymentService.java -B 2 -A 2Repository: Eatsfine/BE
Length of output: 2160
🏁 Script executed:
# Look for any test files that might clarify the expected signature format
fd -e java -path "*test*" -o -path "*spec*" | xargs rg -l "TossPayment|webhook.*signature" 2>/dev/null | head -20Repository: Eatsfine/BE
Length of output: 656
🏁 Script executed:
# Check the webhook controller to see how signature is extracted
cat src/main/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentWebhookController.javaRepository: Eatsfine/BE
Length of output: 3675
🏁 Script executed:
# Look for any test files related to webhook signature verification
fd -type f -name "*.java" | xargs grep -l "verifyWebhookSignature\|webhook.*signature" 2>/dev/nullRepository: Eatsfine/BE
Length of output: 227
🏁 Script executed:
# Search for test files with webhook or signature tests
find . -name "*.java" -type f 2>/dev/null | xargs grep -l "webhook.*signature\|verifyWebhook" 2>/dev/nullRepository: Eatsfine/BE
Length of output: 233
🏁 Script executed:
# Check if hmacSha256 method is using String or byte comparison
cat src/main/java/com/eatsfine/eatsfine/domain/payment/service/TossPaymentService.java | tail -20Repository: Eatsfine/BE
Length of output: 1042
Fix payload format and use constant-time comparison for signature verification.
The signature verification has two critical issues:
-
Incorrect payload format: Current code uses
timestamp + "." + jsonBody, but Toss Payments documentation specifies{WEBHOOK_PAYLOAD}:{tosspayments-webhook-transmission-time}(body first, colon separator). This will cause verification to fail with real Toss webhooks. -
Timing attack vulnerability:
String.contains()is not a constant-time comparison and is unsafe for cryptographic operations. UseMessageDigest.isEqual()instead.
Additionally, per Toss documentation, the signature header contains two v1-prefixed values (e.g., v1:<sigA>,v1:<sigB>), and verification should succeed if either matches.
🔐 Suggested fix
public void verifyWebhookSignature(String jsonBody, String signature, String timestamp) throws Exception {
- String payload = timestamp + "." + jsonBody;
+ String payload = jsonBody + ":" + timestamp;
String calculatedSignature = hmacSha256(payload, widgetSecretKey);
- if (!signature.contains("v1:" + calculatedSignature)) {
+ boolean valid = false;
+ for (String part : signature.split(",")) {
+ String sigValue = part.trim().replace("v1:", "");
+ if (java.security.MessageDigest.isEqual(
+ calculatedSignature.getBytes(java.nio.charset.StandardCharsets.UTF_8),
+ sigValue.getBytes(java.nio.charset.StandardCharsets.UTF_8))) {
+ valid = true;
+ break;
+ }
+ }
+ if (!valid) {
throw new SecurityException("Signature verification failed");
}
}🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/service/TossPaymentService.java`
around lines 53 - 59, In verifyWebhookSignature, the payload is built
incorrectly and the signature check is vulnerable to timing attacks: build
payload as jsonBody + ":" + timestamp (body first, colon separator), split the
incoming signature header by commas to handle multiple "v1:<sig>" entries, for
each entry strip the "v1:" prefix and compute hmacSha256(payload,
widgetSecretKey) once, then compare the expected signature to each provided
signature using a constant-time comparison (MessageDigest.isEqual) and accept if
any match; update references to widgetSecretKey, hmacSha256, and
verifyWebhookSignature accordingly.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import org.springframework.beans.factory.annotation.Qualifier; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.web.client.RestClient; | ||
| import org.springframework.beans.factory.annotation.Value; |
There was a problem hiding this comment.
Import statements are not properly ordered. The @value import from org.springframework.beans.factory.annotation should appear before @qualifier import to maintain alphabetical ordering of Spring Framework imports.
| import org.springframework.beans.factory.annotation.Qualifier; | |
| import org.springframework.stereotype.Service; | |
| import org.springframework.web.client.RestClient; | |
| import org.springframework.beans.factory.annotation.Value; | |
| import org.springframework.beans.factory.annotation.Value; | |
| import org.springframework.beans.factory.annotation.Qualifier; | |
| import org.springframework.stereotype.Service; | |
| import org.springframework.web.client.RestClient; |
| @Transactional | ||
| public void processWebhook(PaymentWebhookDTO dto) { |
There was a problem hiding this comment.
The @transactional annotation on processWebhook doesn't handle OptimisticLockException (or its Spring equivalent ObjectOptimisticLockingFailureException) that can be thrown when the version field conflicts during concurrent webhook deliveries. Toss Payments may deliver the same webhook multiple times, and without retry logic or proper exception handling, legitimate updates could be lost. Consider adding retry logic with @retryable or catching and handling the optimistic lock exception appropriately.
| if (data.totalAmount() == null || payment.getAmount().compareTo(data.totalAmount()) != 0) { | ||
| log.error("Webhook amount verification failed for OrderID: {}. Expected: {}, Received: {}", | ||
| data.orderId(), payment.getAmount(), data.totalAmount()); | ||
| payment.failPayment(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
When amount verification fails and failPayment() is called, the method returns immediately without throwing an exception or providing proper feedback to Toss Payments. This could cause webhook retry storms because Toss will keep retrying when it doesn't receive a 200 OK response for a successfully processed (even if invalid) webhook. Consider whether this should throw an exception or return a specific response code to prevent unnecessary retries.
| String providerCode = data.easyPay().provider(); | ||
| if ("토스페이".equals(providerCode)) { | ||
| provider = PaymentProvider.TOSS; | ||
| } else if ("카카오페이".equals(providerCode)) { | ||
| provider = PaymentProvider.KAKAOPAY; | ||
| } |
There was a problem hiding this comment.
Potential NullPointerException when accessing data.easyPay().provider(). The code checks if data.easyPay() is not null but then immediately calls .provider() on it without null-checking the provider field itself. If the easyPay object exists but provider is null, the string comparison will fail. Consider checking if providerCode is not null before performing string comparisons.
| @Transactional | ||
| public void processWebhook(PaymentWebhookDTO dto) { | ||
| // 이벤트 타입 검증 | ||
| if (!"PAYMENT_STATUS_CHANGED".equals(dto.eventType())) { | ||
| log.info("Webhook skipped: Unhandled event type {}", dto.eventType()); | ||
| return; | ||
| } | ||
|
|
||
| PaymentWebhookDTO.PaymentData data = dto.data(); | ||
|
|
||
| Payment payment = paymentRepository.findByOrderId(data.orderId()) | ||
| .orElseThrow(() -> new PaymentException(PaymentErrorStatus._PAYMENT_NOT_FOUND)); | ||
|
|
||
| PaymentStatus targetStatus = null; | ||
| if ("DONE".equals(data.status())) { | ||
| targetStatus = PaymentStatus.COMPLETED; | ||
| } else if ("CANCELED".equals(data.status())) { | ||
| targetStatus = PaymentStatus.REFUNDED; | ||
| } | ||
|
|
||
| if (targetStatus == null) { | ||
| log.info("Webhook skipped: Unknown or unhandled status {}", data.status()); | ||
| return; | ||
| } | ||
|
|
||
| if (payment.getPaymentStatus() == targetStatus) { | ||
| log.info("Webhook skipped: Payment {} already in status {}", data.orderId(), targetStatus); | ||
| return; | ||
| } | ||
|
|
||
| // 상태 전환 유효성 검사 | ||
| // COMPLETED 완료 처리는 오직 PENDING 상태에서만 가능 | ||
| if (targetStatus == PaymentStatus.COMPLETED && payment.getPaymentStatus() != PaymentStatus.PENDING) { | ||
| log.warn("Webhook skipped: Invalid state transition from {} to {} for OrderID {}", | ||
| payment.getPaymentStatus(), targetStatus, data.orderId()); | ||
| return; | ||
| } | ||
| if (targetStatus == PaymentStatus.REFUNDED && payment.getPaymentStatus() != PaymentStatus.COMPLETED) { | ||
| log.warn("Webhook skipped: Invalid state transition from {} to {} for OrderID {}", | ||
| payment.getPaymentStatus(), targetStatus, data.orderId()); | ||
| return; | ||
| } | ||
|
|
||
| if (targetStatus == PaymentStatus.COMPLETED) { | ||
| // 금액 검증 | ||
| if (data.totalAmount() == null || payment.getAmount().compareTo(data.totalAmount()) != 0) { | ||
| log.error("Webhook amount verification failed for OrderID: {}. Expected: {}, Received: {}", | ||
| data.orderId(), payment.getAmount(), data.totalAmount()); | ||
| payment.failPayment(); | ||
| return; | ||
| } | ||
|
|
||
| // Provider 파싱 | ||
| PaymentProvider provider = null; | ||
| if (data.easyPay() != null) { | ||
| String providerCode = data.easyPay().provider(); | ||
| if ("토스페이".equals(providerCode)) { | ||
| provider = PaymentProvider.TOSS; | ||
| } else if ("카카오페이".equals(providerCode)) { | ||
| provider = PaymentProvider.KAKAOPAY; | ||
| } | ||
| } | ||
|
|
||
| payment.completePayment( | ||
| LocalDateTime.now(), | ||
| PaymentMethod.SIMPLE_PAYMENT, | ||
| data.paymentKey(), | ||
| provider, | ||
| null); | ||
| log.info("Webhook processed: Payment {} status updated to COMPLETED", data.orderId()); | ||
| } else if (targetStatus == PaymentStatus.REFUNDED) { | ||
| payment.cancelPayment(); | ||
| log.info("Webhook processed: Payment {} status updated to REFUNDED", data.orderId()); | ||
| } | ||
| } |
There was a problem hiding this comment.
The processWebhook method lacks test coverage for critical business logic including state transition validation, amount verification, and provider parsing. The codebase demonstrates testing practices (e.g., HealthControllerTest). Consider adding unit tests for: event type filtering, status mapping (DONE→COMPLETED, CANCELED→REFUNDED), state transition rules (PENDING→COMPLETED, COMPLETED→REFUNDED), amount mismatch handling, provider mapping edge cases, and optimistic lock conflict scenarios.
| if ("토스페이".equals(providerCode)) { | ||
| provider = PaymentProvider.TOSS; | ||
| } else if ("카카오페이".equals(providerCode)) { | ||
| provider = PaymentProvider.KAKAOPAY; |
There was a problem hiding this comment.
The provider mapping is incomplete and may silently fail for other payment providers. According to Toss Payments documentation, there are additional providers like 네이버페이, 페이코, 삼성페이, etc. Currently, if a user pays with an unsupported provider, the payment will complete but the provider field will be null, causing data loss. Consider either adding comprehensive provider mapping or logging a warning when an unknown provider is encountered.
| provider = PaymentProvider.KAKAOPAY; | |
| provider = PaymentProvider.KAKAOPAY; | |
| } else if (providerCode != null) { | |
| // 지원하지 않는 간편결제 제공자 코드 처리 | |
| log.warn("Unknown easyPay provider '{}' for orderId {}", providerCode, data.orderId()); |
| @RequestHeader("tosspayments-webhook-signature") String signature, | ||
| @RequestHeader("tosspayments-webhook-transmission-time") String timestamp) throws JsonProcessingException { |
There was a problem hiding this comment.
The webhook endpoint accepts String parameters for signature and timestamp headers without any validation (null checks, format validation, etc.) before using them. If these headers are missing or null, the verifyWebhookSignature method will fail with a NullPointerException instead of returning a clear error. Consider adding @RequestHeader(required = true) or manual null checks with appropriate error responses.
| public ResponseEntity<String> handleWebhook( | ||
| @RequestBody String jsonBody, | ||
| @RequestHeader("tosspayments-webhook-signature") String signature, | ||
| @RequestHeader("tosspayments-webhook-transmission-time") String timestamp) throws JsonProcessingException { |
There was a problem hiding this comment.
The JsonProcessingException from objectMapper.readValue is thrown in the method signature but will never actually be thrown because it's caught by the general Exception handler below. This makes the 'throws JsonProcessingException' declaration misleading and unnecessary. Consider either removing it from the method signature or handling it explicitly before the general catch block.
| } catch (PaymentException e) { | ||
| log.error("Webhook processing failed (Business Logic): {}", e.getMessage()); | ||
| return ResponseEntity.ok("Ignored: " + e.getMessage()); |
There was a problem hiding this comment.
The exception handling returns 200 OK with "Ignored: " message for PaymentException. According to webhook best practices and Toss Payments documentation, returning 200 OK tells Toss that the webhook was successfully processed and prevents retries. However, for genuine business logic errors (like PAYMENT_NOT_FOUND), you might want the webhook to be retried. Consider returning 4xx status codes for unrecoverable errors and 5xx for temporary errors that should be retried.
| @PostMapping | ||
| public ResponseEntity<String> handleWebhook( | ||
| @RequestBody String jsonBody, | ||
| @RequestHeader("tosspayments-webhook-signature") String signature, | ||
| @RequestHeader("tosspayments-webhook-transmission-time") String timestamp) throws JsonProcessingException { | ||
|
|
||
| try { | ||
| tossPaymentService.verifyWebhookSignature(jsonBody, signature, timestamp); | ||
| } catch (Exception e) { | ||
| log.error("Webhook signature verification failed", e); | ||
| return ResponseEntity.status(401).body("Invalid Signature"); | ||
| } | ||
|
|
||
| PaymentWebhookDTO dto = objectMapper.readValue(jsonBody, PaymentWebhookDTO.class); | ||
|
|
||
| if (hasValidationErrors(dto)) { | ||
| return ResponseEntity.badRequest().body("Validation failed"); | ||
| } | ||
|
|
||
| log.info("Webhook received: orderId={}, status={}", dto.data().orderId(), dto.data().status()); | ||
|
|
||
| try { | ||
| paymentService.processWebhook(dto); | ||
| } catch (PaymentException e) { | ||
| log.error("Webhook processing failed (Business Logic): {}", e.getMessage()); | ||
| return ResponseEntity.ok("Ignored: " + e.getMessage()); | ||
| } catch (Exception e) { | ||
| log.error("Webhook processing failed (System Error)", e); | ||
| return ResponseEntity.internalServerError().body("Internal Server Error"); | ||
| } | ||
|
|
||
| return ResponseEntity.ok("Received"); | ||
| } |
There was a problem hiding this comment.
The webhook functionality lacks test coverage. Given that this is a critical payment integration feature with security implications (signature verification), state transitions, and concurrency handling (optimistic locking), comprehensive tests are essential. The codebase shows evidence of testing practices (e.g., HealthControllerTest at src/test/java/com/eatsfine/eatsfine/controller/HealthControllerTest.java). Consider adding tests for: webhook signature verification, amount validation, state transition logic, concurrent webhook handling, and edge cases like missing headers or malformed payloads.
3 participants
💡 작업 개요
✅ 작업 내용
🧪 테스트 내용
./gradlew clean build성공📝 기타 참고 사항
https://eatsfine.co.kr/api/v1/payments/webhookURL을 등록해야 기능이 활성화됩니다.SecurityConfig) 변경 없이 접근 가능함을 확인했습니다.