Skip to content

Conversation

@kwon204
Copy link
Contributor

@kwon204 kwon204 commented May 3, 2025

#️⃣ 연관된 이슈>

📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)

  • 회원 탈퇴하는 기능입니다.
  • 참여하는 논의에서 탈퇴합니다. 주도자인 경우 논의를 삭제합니다.
  • 개인 일정을 삭제합니다.
  • 캘린더를 연동했다면
    • 캘린더를 삭제합니다. 추가로 구글 캘린더 푸쉬 알림 구독을 해제합니다.

🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

Summary by CodeRabbit

  • New Features

    • Added user account withdrawal functionality, allowing users to delete their accounts and associated data.
    • Implemented automatic cleanup of user-related data (discussions, personal events, calendars) upon account withdrawal.
    • Added support for unsubscribing from Google Calendar webhooks when a calendar is deleted.
  • Improvements

    • Enhanced configuration for frontend callback URLs to support multiple endpoints.
    • Improved event-driven handling for data deletion and external service unsubscriptions.
  • Bug Fixes

    • Removed unused imports for cleaner codebase.
  • Chores

    • Updated test configuration to reflect new callback URL structure.

@kwon204 kwon204 added the 🛠️ BE Backend label May 3, 2025
@kwon204 kwon204 added this to the 🍭11차 스프린트 milestone May 3, 2025
@kwon204 kwon204 self-assigned this May 3, 2025
@kwon204 kwon204 requested a review from efdao as a code owner May 3, 2025 20:03
@coderabbitai
Copy link

coderabbitai bot commented May 3, 2025

Walkthrough

This update introduces user withdrawal functionality to the backend. A new API endpoint allows users to withdraw, triggering a cascade of cleanup operations: removing the user from discussions, deleting personal events, and deleting calendar data. Supporting methods and event handlers are added across authentication, discussion, personal event, and calendar domains. The Google Calendar integration is enhanced to support unsubscribing from webhooks when a calendar is deleted. Configuration properties for frontend callback URLs are refactored to support multiple contexts. Repository and service layers are extended as needed to support bulk deletions and event-driven cleanup.

Changes

File(s) Change Summary
AuthController.java, AuthService.java, WithdrawUser.java, AuthEventHandler.java Added user withdrawal API endpoint, service logic, event record, and event handler for cascading user data cleanup.
CalendarController.java, application.yml Refactored frontend callback URL configuration to use specific keys for login and Google Calendar.
CalendarService.java, CalendarDeleteEvent.java Added method and event for deleting calendars by user ID, and publishing calendar deletion events.
DiscussionParticipantRepository.java, DiscussionParticipantService.java, DiscussionService.java Added bulk discussion participant deletion, service method, and extended discussion exit logic for first participant.
PersonalEventRepository.java, PersonalEventService.java Added methods for retrieving and deleting all personal events by user ID.
GoogleCalendarApi.java, GoogleCalendarService.java, UnsubscribeGoogleCalendar.java Added support for unsubscribing from Google Calendar webhooks and related DTO.
GoogleEventHandler.java Added event handler for calendar deletion events to trigger Google Calendar unsubscription.
DiscussionBitmapService.java Removed unused imports.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant AuthController
    participant AuthService
    participant ApplicationEventPublisher
    participant AuthEventHandler
    participant DiscussionService
    participant PersonalEventService
    participant CalendarService
    participant GoogleEventHandler
    participant GoogleCalendarService

    User->>AuthController: POST /api/v1/withdraw
    AuthController->>AuthService: withdraw(refreshToken)
    AuthService->>AuthService: logout(refreshToken)
    AuthService->>AuthService: getCurrentUser()
    AuthService->>ApplicationEventPublisher: publish WithdrawUser(user)
    AuthService->>AuthService: userRepository.delete(user)
    ApplicationEventPublisher-->>AuthEventHandler: WithdrawUser event
    AuthEventHandler->>DiscussionService: exitDiscussion for all user's discussions
    AuthEventHandler->>PersonalEventService: deletePersonalEventsByUserId(userId)
    AuthEventHandler->>CalendarService: deleteCalendarsByUserId(userId)
    CalendarService->>ApplicationEventPublisher: publish CalendarDeleteEvent(calendar)
    ApplicationEventPublisher-->>GoogleEventHandler: CalendarDeleteEvent
    GoogleEventHandler->>GoogleCalendarService: unsubscribeFromCalendar(calendar)
Loading

Suggested reviewers

  • efdao

Poem

A user departs, and with a hop,
The backend sweeps—no crumbs to mop!
Discussions, events, and calendars too,
All cleaned up as rabbits do.
With webhooks snipped and cookies gone,
The code hops forward, life moves on.
🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (7)
backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java (1)

336-342:

Details

❓ Verification inconclusive

Consider publishing events for deleted personal events.

Unlike the deletePersonalEventsByDiscussionId method (lines 324-334), this method doesn't publish DeletePersonalEvent events for each personal event being deleted. This could lead to inconsistent behavior if these events have external integrations (like Google Calendar) that need to be updated.

 public void deletePersonalEventsByUserId(Long userId) {
     List<PersonalEvent> personalEvents = personalEventRepository.findAllByUserId(userId);
     if (personalEvents.isEmpty()) {
         return;
     }
+    for (PersonalEvent event : personalEvents) {
+        if (event.getGoogleEventId() != null && event.getCalendarId() != null) {
+            eventPublisher.publishEvent(new DeletePersonalEvent(event));
+        }
+    }
     personalEventRepository.deleteAll(personalEvents);
 }

However, if this is intentional because the Google Calendar will be unsubscribed anyway during user withdrawal, please add a comment explaining this decision for future maintainers.


🏁 Script executed:

#!/bin/bash
# Check if findAllByUserId is properly implemented in the repository
rg -A 2 "findAllByUserId" --glob "*.java"

Length of output: 716


#!/bin/bash
# Inspect deletePersonalEventsByDiscussionId implementation for event publishing
rg -A 15 "deletePersonalEventsByDiscussionId" --glob "backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java"

🏁 Script executed:

#!/bin/bash
# Inspect deletePersonalEventsByDiscussionId implementation for event publishing
rg -A 20 "deletePersonalEventsByDiscussionId" --glob "backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java"

Length of output: 2686


Ensure deletion events are published in deletePersonalEventsByUserId

The deletePersonalEventsByDiscussionId method explicitly publishes a DeletePersonalEvent for each event before deletion, but deletePersonalEventsByUserId does not. This can lead to missed cleanup in external integrations (e.g., Google Calendar).

• File: backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java
Lines: ~336–342

Suggested patch:

 public void deletePersonalEventsByUserId(Long userId) {
     List<PersonalEvent> personalEvents = personalEventRepository.findAllByUserId(userId);
     if (personalEvents.isEmpty()) {
         return;
     }
+    for (PersonalEvent event : personalEvents) {
+        // mirror deletePersonalEventsByDiscussionId to notify external systems
+        if (event.getGoogleEventId() != null && event.getCalendarId() != null) {
+            eventPublisher.publishEvent(new DeletePersonalEvent(event));
+        }
+    }
     personalEventRepository.deleteAll(personalEvents);
 }

If omitting these notifications is intentional (for example, because calendar cleanup is handled elsewhere during user withdrawal), please add a clarifying comment to avoid confusion for future maintainers.

backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java (1)

178-199: Well-implemented unsubscription with proper checks

The method correctly checks for valid channel expiration before attempting to unsubscribe, and uses the retry executor pattern consistent with other methods in this class.

The log message uses [unsubscribeToCalendar] but the method is named unsubscribeFromCalendar. Consider making this consistent:

-                log.info("[unsubscribeToCalendar] success:");
+                log.info("[unsubscribeFromCalendar] success:");
backend/src/main/java/endolphin/backend/domain/auth/event/handler/AuthEventHandler.java (2)

28-31: Consider batch processing for better performance in discussion exits.

The current implementation iterates through each discussion individually to exit the user, which could be inefficient for users participating in many discussions. Consider implementing a batch operation if available.

-       List<Discussion> discussionList =
-           discussionParticipantService.getDiscussionsByUserId(user.getId());
-       discussionList.forEach(discussion ->
-           discussionService.exitDiscussion(discussion.getId(), user.getId()));
+       // If batch operation is available
+       discussionService.exitAllDiscussions(user.getId());
+       
+       // Or keep current approach but with potential for optimization
+       List<Discussion> discussionList =
+           discussionParticipantService.getDiscussionsByUserId(user.getId());
+       if (!discussionList.isEmpty()) {
+           discussionService.exitDiscussions(discussionList.stream()
+               .map(Discussion::getId)
+               .collect(Collectors.toList()), user.getId());
+       }

1-14: Consider adding a domain exception handler.

For a robust implementation, consider adding specialized exception handling for domain-specific errors that might occur during the withdrawal process, such as constraints that prevent a user from leaving a discussion.

package endolphin.backend.domain.auth.exception;

public class WithdrawalProcessException extends RuntimeException {
    public WithdrawalProcessException(String message) {
        super(message);
    }
    
    public WithdrawalProcessException(String message, Throwable cause) {
        super(message, cause);
    }
}
backend/src/main/java/endolphin/backend/domain/discussion/DiscussionParticipantService.java (2)

43-43: The new dependency on DiscussionBitmapService is defined but not used in the new method.

The DiscussionBitmapService is added as a dependency but isn't used in the newly added deleteDiscussion method. This might indicate a missing implementation or that it's being used elsewhere.


327-329: Consider adding transaction management for bulk deletion operation.

The new method correctly implements bulk deletion of discussion participants. Consider whether explicit transaction management is needed for this operation, especially if it's executed as part of a larger withdrawal process.

You might want to add a comment explaining the purpose of this method in the context of user withdrawal.

backend/src/main/java/endolphin/backend/domain/discussion/DiscussionService.java (1)

414-420: Consider adding more logging for discussion deletion.

The deleteDiscussion method performs critical cleanup operations but lacks any logging. Consider adding log statements to track when discussions are deleted, especially for debugging purposes in a production environment.

private void deleteDiscussion(Discussion discussion) {
+   log.info("Deleting discussion: {}", discussion.getId());
    discussionBitmapService.deleteDiscussionBitmaps(discussion.getId());
    discussionParticipantService.deleteDiscussion(discussion.getId());
    if (discussion.getDiscussionStatus() == DiscussionStatus.ONGOING)
        return;
    sharedEventService.deleteSharedEvent(discussion.getId());
+   log.info("Discussion deleted successfully: {}", discussion.getId());
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 094822f and 92afaaa.

📒 Files selected for processing (18)
  • backend/src/main/java/endolphin/backend/domain/auth/AuthController.java (2 hunks)
  • backend/src/main/java/endolphin/backend/domain/auth/AuthService.java (4 hunks)
  • backend/src/main/java/endolphin/backend/domain/auth/event/WithdrawUser.java (1 hunks)
  • backend/src/main/java/endolphin/backend/domain/auth/event/handler/AuthEventHandler.java (1 hunks)
  • backend/src/main/java/endolphin/backend/domain/calendar/CalendarController.java (1 hunks)
  • backend/src/main/java/endolphin/backend/domain/calendar/CalendarService.java (2 hunks)
  • backend/src/main/java/endolphin/backend/domain/calendar/event/CalendarDeleteEvent.java (1 hunks)
  • backend/src/main/java/endolphin/backend/domain/discussion/DiscussionParticipantRepository.java (1 hunks)
  • backend/src/main/java/endolphin/backend/domain/discussion/DiscussionParticipantService.java (2 hunks)
  • backend/src/main/java/endolphin/backend/domain/discussion/DiscussionService.java (1 hunks)
  • backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventRepository.java (1 hunks)
  • backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java (1 hunks)
  • backend/src/main/java/endolphin/backend/global/google/GoogleCalendarApi.java (1 hunks)
  • backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java (2 hunks)
  • backend/src/main/java/endolphin/backend/global/google/dto/UnsubscribeGoogleCalendar.java (1 hunks)
  • backend/src/main/java/endolphin/backend/global/google/event/handler/GoogleEventHandler.java (2 hunks)
  • backend/src/main/java/endolphin/backend/global/redis/DiscussionBitmapService.java (0 hunks)
  • backend/src/test/resources/application.yml (1 hunks)
💤 Files with no reviewable changes (1)
  • backend/src/main/java/endolphin/backend/global/redis/DiscussionBitmapService.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java (1)
backend/src/main/java/endolphin/backend/global/util/TimeUtil.java (1)
  • TimeUtil (11-119)
🪛 ast-grep (0.31.1)
backend/src/main/java/endolphin/backend/domain/auth/AuthController.java

[warning] 99-99: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(cookieUtil.deleteRefreshTokenCookie());
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly

(cookie-missing-httponly-java)


[warning] 99-99: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(cookieUtil.deleteRefreshTokenCookie());
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application

(cookie-missing-samesite-java)

🔇 Additional comments (16)
backend/src/test/resources/application.yml (1)

85-87: Good configuration refactoring to support multiple callback contexts

The change from a single callback URL to a structured map with specific callback URLs for different purposes (login and Google Calendar) properly supports the user withdrawal functionality implementation. This approach provides better separation of concerns and makes the configuration more maintainable as the application evolves.

backend/src/main/java/endolphin/backend/domain/calendar/CalendarController.java (1)

28-29: Correctly updated dependency injection to match configuration changes

This change correctly updates the Value annotation to use the new specific google-calendar callback URL path from the restructured configuration. This ensures the controller redirects to the appropriate frontend page after Google Calendar operations.

backend/src/main/java/endolphin/backend/global/google/dto/UnsubscribeGoogleCalendar.java (1)

1-8: Well-designed DTO for Google Calendar unsubscription

This Java record is a clean, immutable data carrier for the Google Calendar unsubscription request. Using a record is appropriate here as it automatically provides equals(), hashCode(), and toString() methods while maintaining immutability. The design directly supports the PR objective of unsubscribing from Google Calendar push notifications during user withdrawal.

backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventRepository.java (1)

68-71: Properly implemented repository method for user data cleanup

This new query method follows the repository's established patterns and will efficiently retrieve all personal events for a specific user. It directly supports the PR objective of deleting user schedules during account withdrawal, providing a clean way to identify all events that need to be removed.

backend/src/main/java/endolphin/backend/domain/calendar/event/CalendarDeleteEvent.java (1)

1-9: Looks good!

This new record class is a clean and appropriate implementation for an event object that will carry calendar deletion information through the system.

backend/src/main/java/endolphin/backend/domain/auth/event/WithdrawUser.java (1)

1-9: Looks good!

This record class provides a clean way to transport user information during the withdrawal process throughout the event-driven architecture.

backend/src/main/java/endolphin/backend/global/google/event/handler/GoogleEventHandler.java (2)

4-4: Appropriate import added.

The import for the new CalendarDeleteEvent class is correctly added.


59-63: Clean event handler implementation.

The new async event handler follows the established pattern in the class and correctly unsubscribes from the Google Calendar webhook when a calendar is deleted.

backend/src/main/java/endolphin/backend/domain/discussion/DiscussionParticipantRepository.java (1)

139-146: Well-implemented bulk deletion method.

The new deleteByDiscussionId method properly provides bulk deletion capability for all participants of a discussion, which is essential for the user withdrawal functionality. The method correctly uses the @Modifying annotation to indicate it modifies data.

backend/src/main/java/endolphin/backend/domain/calendar/CalendarService.java (2)

4-4: LGTM!

Properly added import for the new CalendarDeleteEvent class.


124-133: Clean implementation of calendar deletion with event publishing.

The method follows a good pattern:

  1. Early return if no calendar exists
  2. Retrieve the calendar before deletion
  3. Publish an event for downstream handlers
  4. Delete the calendar entity

This approach properly separates concerns by delegating Google Calendar unsubscription to event handlers.

backend/src/main/java/endolphin/backend/domain/auth/AuthController.java (1)

38-38: LGTM!

Property injection updated to reflect the restructured configuration for multiple callback URLs.

backend/src/main/java/endolphin/backend/domain/auth/AuthService.java (1)

5-6: LGTM!

Dependencies and imports correctly added to support the withdrawal functionality.

Also applies to: 17-18, 20-20, 31-32

backend/src/main/java/endolphin/backend/domain/auth/event/handler/AuthEventHandler.java (1)

24-25: Verify order of operations and consider potential dependencies.

The current order of operations (exit discussions → delete personal events → delete calendars) seems logical, but it's important to verify there are no dependencies that would require a different order. For example, if calendar deletions have foreign key constraints with personal events.

backend/src/main/java/endolphin/backend/global/google/GoogleCalendarApi.java (1)

156-173: ⚠️ Potential issue

Fix return type mismatch in the unsubscribe method.

The unsubscribe method is declared as void but the implementation attempts to return a WatchResponse object in the exchange handler. Since the return value isn't used, this creates an inconsistency.

Apply this change to align the implementation with the method signature:

public void unsubscribe(String url, Object body, String accessToken) {
    restClient.post()
        .uri(url)
        .header(HttpHeaders.AUTHORIZATION, BEARER + accessToken)
        .contentType(MediaType.APPLICATION_JSON)
        .body(body)
        .exchange((request, response) -> {
            if (response.getStatusCode().is2xxSuccessful()) {
-                return response.bodyTo(WatchResponse.class);
+                return Optional.empty();
            }
            log.error("Invalid request: {}", response.bodyTo(String.class));
            if (response.getStatusCode().isSameCodeAs(HttpStatus.UNAUTHORIZED)) {
                throw new OAuthException(ErrorCode.OAUTH_UNAUTHORIZED_ERROR);
            }
            throw new CalendarException((HttpStatus) response.getStatusCode(),
                response.bodyTo(String.class));
        });
+    log.info("[unsubscribeFromGoogleCalendar] success");
}

Likely an incorrect or invalid review comment.

backend/src/main/java/endolphin/backend/domain/discussion/DiscussionService.java (1)

405-408: LGTM! Clear handling of host exit case.

The implementation correctly handles the case where the exiting user is the discussion host (offset 0), triggering deletion of the entire discussion. This matches the PR requirement that when a user withdraws, if they are the discussion leader, the entire discussion is deleted.

Comment on lines +96 to +102
@Operation(summary = "회원 탈퇴", description = "회원 탈퇴를 합니다.")
@PostMapping("/api/v1/withdraw")
public ResponseEntity<Void> withdraw(HttpServletRequest request, HttpServletResponse response) {
authService.withdraw(cookieUtil.getRefreshToken(request.getCookies()));
response.addCookie(cookieUtil.deleteRefreshTokenCookie());
return ResponseEntity.noContent().build();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify cookie security for this sensitive operation.

The withdrawal endpoint follows the established pattern in the application, correctly retrieving the refresh token and clearing it after withdrawal. However, ensure that the cookies used for authentication are properly secured.


🏁 Script executed:

#!/bin/bash
# Check if the CookieUtil class properly sets security attributes for authentication cookies

# Find and examine the CookieUtil implementation
echo "Searching for CookieUtil class implementation..."
cat backend/src/main/java/endolphin/backend/global/util/CookieUtil.java

# Look specifically for the deleteRefreshTokenCookie method
echo "\nChecking deleteRefreshTokenCookie method implementation..."
rg -A 10 "deleteRefreshTokenCookie" backend/src/main/java/endolphin/backend/global/util/

Length of output: 3074


🏁 Script executed:

#!/bin/bash
# Inspect RefreshTokenCookieProperties for SameSite, path, domain settings
echo "Displaying RefreshTokenCookieProperties..."
cat backend/src/main/java/endolphin/backend/global/config/RefreshTokenCookieProperties.java

Length of output: 505


Cookie security attributes verified – add SameSite for CSRF protection

I confirmed that CookieUtil uses the configured httpOnly, secure, path and maxAge flags when creating and deleting the refresh-token cookie. To further harden this sensitive endpoint, please extend the implementation to include a SameSite attribute (e.g. Lax or Strict) and, if your deployment spans subdomains, explicitly set the domain.

Points to address:

  • backend/src/main/java/endolphin/backend/global/util/CookieUtil.java
    · After creating the Cookie, append the SameSite attribute (since jakarta.servlet.http.Cookie doesn’t expose it, you may need to set a response header or switch to Spring’s ResponseCookie).
  • backend/src/main/java/endolphin/backend/global/config/RefreshTokenCookieProperties.java
    · Add a sameSite (and optionally domain) property so the attribute can be configured.

No other issues found for this withdrawal endpoint.

🧰 Tools
🪛 ast-grep (0.31.1)

[warning] 99-99: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(cookieUtil.deleteRefreshTokenCookie());
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly

(cookie-missing-httponly-java)


[warning] 99-99: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(cookieUtil.deleteRefreshTokenCookie());
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application

(cookie-missing-samesite-java)

Comment on lines +64 to +72
public void withdraw(String refreshToken) {
logout(refreshToken);

User user = userService.getCurrentUser();

applicationEventPublisher.publishEvent(new WithdrawUser(user));

userRepository.delete(user);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add transaction and error handling for the critical withdrawal operation.

The withdrawal implementation logically follows these steps:

  1. Invalidate the refresh token
  2. Get the current user
  3. Publish the withdrawal event for downstream processing
  4. Delete the user entity

Consider these improvements for this critical operation:

-    public void withdraw(String refreshToken) {
-        logout(refreshToken);
-
-        User user = userService.getCurrentUser();
-
-        applicationEventPublisher.publishEvent(new WithdrawUser(user));
-
-        userRepository.delete(user);
-    }
+    @Transactional
+    public void withdraw(String refreshToken) {
+        try {
+            logout(refreshToken);
+
+            User user = userService.getCurrentUser();
+            if (user == null) {
+                throw new IllegalStateException("User not found during withdrawal process");
+            }
+
+            applicationEventPublisher.publishEvent(new WithdrawUser(user));
+
+            userRepository.delete(user);
+        } catch (Exception e) {
+            // Log the error with appropriate context
+            log.error("Error during user withdrawal process", e);
+            throw e;
+        }
+    }

Comment on lines +24 to +35
@EventListener(classes = {WithdrawUser.class})
public void withdrawUser(WithdrawUser event) {
User user = event.user();

List<Discussion> discussionList =
discussionParticipantService.getDiscussionsByUserId(user.getId());
discussionList.forEach(discussion ->
discussionService.exitDiscussion(discussion.getId(), user.getId()));

personalEventService.deletePersonalEventsByUserId(user.getId());
calendarService.deleteCalendarsByUserId(user.getId());
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add transaction management to ensure atomic operations.

The withdrawUser method performs multiple service calls that modify data, but lacks transaction management. If an operation fails midway, it could leave the system in an inconsistent state with partially deleted user data.

@Component
@RequiredArgsConstructor
public class AuthEventHandler {

    private final PersonalEventService personalEventService;
    private final CalendarService calendarService;
    private final DiscussionParticipantService discussionParticipantService;
    private final DiscussionService discussionService;

    @EventListener(classes = {WithdrawUser.class})
+   @Transactional
    public void withdrawUser(WithdrawUser event) {
        User user = event.user();

        List<Discussion> discussionList =
            discussionParticipantService.getDiscussionsByUserId(user.getId());
        discussionList.forEach(discussion ->
            discussionService.exitDiscussion(discussion.getId(), user.getId()));

        personalEventService.deletePersonalEventsByUserId(user.getId());
        calendarService.deleteCalendarsByUserId(user.getId());
    }
}

Don't forget to add the import:

import org.springframework.transaction.annotation.Transactional;

🛠️ Refactor suggestion

Add error handling and logging for this critical operation.

User withdrawal is a significant operation that should include proper error handling and logging for auditing and debugging purposes. Currently, if any service call fails, subsequent operations won't execute.

@Component
@RequiredArgsConstructor
public class AuthEventHandler {

    private final PersonalEventService personalEventService;
    private final CalendarService calendarService;
    private final DiscussionParticipantService discussionParticipantService;
    private final DiscussionService discussionService;
+   private final Logger log = LoggerFactory.getLogger(AuthEventHandler.class);

    @EventListener(classes = {WithdrawUser.class})
    public void withdrawUser(WithdrawUser event) {
        User user = event.user();
+       log.info("Processing withdrawal for user: {}", user.getId());

        try {
            List<Discussion> discussionList =
                discussionParticipantService.getDiscussionsByUserId(user.getId());
+           log.debug("Found {} discussions for user {}", discussionList.size(), user.getId());
            discussionList.forEach(discussion -> {
                try {
                    discussionService.exitDiscussion(discussion.getId(), user.getId());
+                   log.debug("User {} exited discussion {}", user.getId(), discussion.getId());
                } catch (Exception e) {
+                   log.error("Failed to exit discussion {} for user {}: {}", discussion.getId(), user.getId(), e.getMessage(), e);
+                   throw e;
                }
            });

            personalEventService.deletePersonalEventsByUserId(user.getId());
+           log.debug("Deleted personal events for user {}", user.getId());
            
            calendarService.deleteCalendarsByUserId(user.getId());
+           log.debug("Deleted calendars for user {}", user.getId());
            
+           log.info("Successfully completed withdrawal process for user {}", user.getId());
+       } catch (Exception e) {
+           log.error("Failed to process withdrawal for user {}: {}", user.getId(), e.getMessage(), e);
+           throw e;
+       }
    }
}

Don't forget to add the imports:

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@kwon204 kwon204 marked this pull request as draft May 4, 2025 01:07
@efdao
Copy link
Collaborator

efdao commented May 27, 2025

얘는 드래프트라 리뷰 안하고있었는데 필요하면 말씀해주세요

@kwon204 kwon204 closed this Jul 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🛠️ BE Backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants