Skip to content

Conversation

@jsoonworld
Copy link
Member

📄 Work Description

  • @AuthenticationPrincipal 적용: 모든 컨트롤러에서 하드코딩 되어있던 userId를 제거하고, Spring Security의 @AuthenticationPrincipal을 통해 동적으로 인증된 사용자 ID를 받아오도록 수정했습니다.
  • spring-security-test 의존성 추가: 컨트롤러 테스트에서 Spring Security 관련 기능을 사용하기 위해 build.gradle.kts에 의존성을 추가했습니다.
  • 테스트용 커스텀 어노테이션 @WithMockCustomUser 추가: 컨트롤러 테스트 시, @AuthenticationPrincipal로 주입되는 Long 타입의 사용자 ID를 간단하게 모킹(Mocking)하기 위한 커스텀 어노테이션과 Security Context Factory를 구현했습니다.
  • 컨트롤러 테스트 리팩토링: 새로운 인증 방식을 테스트하기 위해 모든 컨트롤러 테스트 코드에 @WithMockCustomUser를 적용하고 관련 로직을 수정했습니다.

💭 Thoughts

  • application 태스트를 제외하고 모두 통과하였는데요, 지난 회의에서 이야기를 나눴듯 auth 관련 이슈는 유빈님께서 해결해주시면 감사하겠습니다!
  • @WithMockCustomUser를 만든 이유:
    • 컨트롤러에서 @AuthenticationPrincipal Long userId 형태로 사용자 ID를 직접 받고 있습니다. 하지만 Spring Security Test의 기본 @WithMockUser는 principal을 String 타입의 username으로 설정하여 저희 로직과 맞지 않았습니다.
    • 이를 해결하기 위해 principal로 Long 타입의 userId를 직접 설정해주는 @WithMockCustomUser 어노테이션을 만들어 테스트의 편의성과 코드의 일관성을 높였습니다.
  • 유빈님, 이렇게 커스텀 어노테이션을 만들어 사용하면 앞으로 인증이 필요한 테스트를 작성할 때 더 편리할 것이라고 생각했는데, 어떻게 생각하시나요? 혹시 더 효율적이거나 표준적인 방법이 있다면 의견 공유해주시면 좋겠습니다!

✅ Testing Result

스크린샷 2025-06-26 오후 1 26 04

🗂 Related Issue

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors controller methods to inject the authenticated user ID via Spring Security’s @AuthenticationPrincipal and removes hard-coded user IDs and TODOs; it also introduces a custom @WithMockCustomUser annotation and TestSecurityConfig to mock authentication in controller tests.

  • Controllers now accept @AuthenticationPrincipal userId: Long and remove temporary hard-coded IDs.
  • Added WithMockCustomUser annotation and corresponding security context factory for tests.
  • Updated all controller tests to import TestSecurityConfig, use @WithMockCustomUser, and adjust CSRF handling and status assertions.

Reviewed Changes

Copilot reviewed 17 out of 19 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/test/kotlin/com/terning/server/kotlin/ui/api/SearchControllerTest.kt Imported TestSecurityConfig, removed @WithMockUser, added @WithMockCustomUser per test
src/test/kotlin/com/terning/server/kotlin/ui/api/ScrapControllerTest.kt Added @WithMockCustomUser, removed hard-coded auth setup
src/test/kotlin/com/terning/server/kotlin/ui/api/MyPageControllerTest.kt Added @WithMockCustomUser, imported CSRF for patch, changed status assertion
src/test/kotlin/com/terning/server/kotlin/ui/api/HomeControllerTest.kt Imported TestSecurityConfig, replaced @WithMockUser, added @WithMockCustomUser
src/test/kotlin/com/terning/server/kotlin/ui/api/FilterControllerTest.kt Added @WithMockCustomUser, imported CSRF for POST/PUT
src/test/kotlin/com/terning/server/kotlin/ui/api/CalendarControllerTest.kt Imported TestSecurityConfig, replaced @WithMockUser, added @WithMockCustomUser
src/test/kotlin/com/terning/server/kotlin/ui/api/AuthControllerTest.kt Added @WithMockCustomUser on logout test, updated display names
src/test/kotlin/com/terning/server/kotlin/ui/api/AnnouncementControllerTest.kt Imported TestSecurityConfig, replaced @WithMockUser, added @WithMockCustomUser
src/test/kotlin/com/terning/server/kotlin/support/WithMockCustomUserSecurityContextFactory.kt New security context factory for WithMockCustomUser annotation
src/test/kotlin/com/terning/server/kotlin/support/WithMockCustomUser.kt New annotation to mock a Long principal in tests
src/main/kotlin/com/terning/server/kotlin/ui/api/SearchController.kt Added @AuthenticationPrincipal userId, removed hard-coded IDs/TODOs
src/main/kotlin/com/terning/server/kotlin/ui/api/ScrapController.kt Added @AuthenticationPrincipal userId, removed hard-coded IDs/TODOs
src/main/kotlin/com/terning/server/kotlin/ui/api/MyPageController.kt Added @AuthenticationPrincipal userId, removed hard-coded IDs/TODOs
src/main/kotlin/com/terning/server/kotlin/ui/api/HomeController.kt Added @AuthenticationPrincipal userId, removed hard-coded IDs/TODOs
src/main/kotlin/com/terning/server/kotlin/ui/api/FilterController.kt Added @AuthenticationPrincipal userId, removed hard-coded IDs/TODOs
src/main/kotlin/com/terning/server/kotlin/ui/api/CalendarController.kt Added @AuthenticationPrincipal userId, removed hard-coded IDs/TODOs
src/main/kotlin/com/terning/server/kotlin/ui/api/AnnouncementController.kt Added @AuthenticationPrincipal userId, removed hard-coded IDs/TODOs
Comments suppressed due to low confidence (3)

src/test/kotlin/com/terning/server/kotlin/ui/api/ScrapControllerTest.kt:57

  • POST endpoint tests should include a CSRF token when CSRF protection is enabled. Add with(csrf()) inside the mockMvc.post block.
        // when & then

src/test/kotlin/com/terning/server/kotlin/ui/api/ScrapControllerTest.kt:69

  • PATCH endpoint tests should include a CSRF token. Add with(csrf()) inside the mockMvc.patch block.
    @DisplayName("스크랩 색상을 업데이트한다")

src/test/kotlin/com/terning/server/kotlin/ui/api/ScrapControllerTest.kt:81

  • DELETE endpoint tests should include a CSRF token. Add with(csrf()) inside the mockMvc.delete block.
        }.andExpect {

@WebMvcTest(SearchController::class)
@WithMockUser
@Import(TestSecurityConfig::class)
@ActiveProfiles("test")
Copy link

Copilot AI Jun 26, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider moving @WithMockCustomUser(userId = 1L) to the class level to avoid repeating it on every test method.

Suggested change
@ActiveProfiles("test")
@ActiveProfiles("test")
@WithMockCustomUser(userId = 1L)

Copilot uses AI. Check for mistakes.
Copy link
Member

@leeeyubin leeeyubin left a comment

Choose a reason for hiding this comment

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

변경된 사항 확인했습니다! TODO로 된 부분 많았을 텐데 수정해주셔서 감사해요!
장순님이 @WithMockCustomUser 만드신 거 좋은 것 같아요!
더 좋은 방법 찾으면 저도 공유해보도록 하겠습니다~ 수고하셨어요!

Comment on lines +6 to +8
concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
cancel-in-progress: true
Copy link
Member

Choose a reason for hiding this comment

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

오 여러 워크플로우에 대한 동시성 설정인가요?! 신기하군요

@jsoonworld jsoonworld merged commit 1409b5c into develop Jun 28, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[♻️ refactor] 컨트롤러에 @AuthenticationPrincipal을 사용하여 인증된 사용자 ID 연동

3 participants