[refactor] Auth 토큰/쿠키 정책 정리 및 토큰 도메인 정리

[jiyoon-00]

#️⃣ 연관된 이슈

#352

📝 작업 내용

auth 쿠키 정책을 프로퍼티로 분리하여 하드코딩 제거
refreshToken을 바디로 전달하지 않고 쿠키 기반으로 정리
인증 쿠키 생성/세팅/삭제 로직을 공통화하여 중복 제거
PasswordResetToken 엔티티 위치 이동하여 정리

🖼️ 스크린샷 (선택)

💬 리뷰 요구사항 (선택)

리뷰어가 특히 봐주었으면 하는 부분이 있다면 작성해주세요

[github-actions]

🤖 AI Code Review

View results

🧠 요약

• 쿠키 설정 로직 통합: 쿠키 생성 및 삭제 로직을 통합하여 중복을 줄였으나, 쿠키의 보안 설정이 제대로 적용되고 있는지 확인이 필요하다.
• PasswordResetToken 위치 변경: 도메인 구조 변경으로 인해 코드의 가독성이 향상되었으나, 기존 코드와의 호환성 문제를 검토해야 한다.
• 테스트 케이스 부족: 쿠키 기반 인증 로직 변경으로 인해 기존 테스트 케이스가 충분하지 않을 수 있으며, 새로운 테스트 케이스 추가가 필요하다.

⚠️ 세부 코드 리뷰

• [src/main/java/com/server/auth/controller/AuthController.java:30] 의존성 주입 → AuthCookieProperties를 통해 쿠키 설정을 외부화한 것은 좋은 접근이다. 하지만, 이 설정이 실제로 잘 적용되고 있는지에 대한 검증이 필요하다. 예를 들어, secure, sameSite 등의 값이 올바르게 설정되었는지 확인하는 로직이 추가되어야 한다.

• [src/main/java/com/server/auth/controller/AuthController.java:62] 중복 제거 → 쿠키 설정 로직을 setAuthCookies 메서드로 통합한 것은 코드의 가독성을 높이고 유지보수를 용이하게 한다. 그러나, 이 메서드가 여러 곳에서 호출되므로, 예외 처리나 로그를 추가하여 문제가 발생했을 때 쉽게 디버깅할 수 있도록 개선할 필요가 있다.

• [src/main/java/com/server/auth/controller/AuthController.java:116] 쿠키 삭제 통일 → clearAuthCookies 메서드를 통해 쿠키 삭제 로직을 통일한 것은 좋으나, 이 메서드가 호출되는 모든 경로에서 쿠키가 제대로 삭제되는지 확인해야 한다. 특히, 로그아웃 시에만 호출되는 것이 아니라 다른 상황에서도 호출될 수 있으므로, 이 부분에 대한 테스트가 필요하다.

• [src/main/java/com/server/auth/domain/PasswordResetToken.java:1] 도메인 구조 변경 → PasswordResetToken의 패키지를 변경한 것은 도메인 간의 관계를 명확히 하고, 코드의 가독성을 높이는 데 기여한다. 그러나, 이 변경으로 인해 기존에 이 엔티티를 참조하던 코드에서 문제가 발생할 수 있으므로, 전체 코드베이스에서 이 엔티티를 사용하는 부분을 점검해야 한다.

🧪 테스트 제안

• 쿠키 설정 및 삭제 테스트: AuthController의 login, refresh, logout 메서드에 대해 쿠키가 올바르게 설정되고 삭제되는지 검증하는 테스트 케이스를 추가해야 한다. 특히, 쿠키의 속성(예: httpOnly, secure, sameSite)이 올바르게 설정되었는지 확인하는 것이 중요하다.

• PasswordResetToken 도메인 테스트: PasswordResetToken의 위치 변경으로 인해 기존에 작성된 테스트가 실패할 수 있으므로, 이 도메인에 대한 테스트 케이스를 점검하고 필요 시 수정해야 한다.

• 예외 처리 테스트: 인증 과정에서 발생할 수 있는 다양한 예외 상황(예: 잘못된 비밀번호, 만료된 토큰 등)에 대한 테스트 케이스를 추가하여, 시스템이 예상대로 동작하는지 확인해야 한다.

[github-actions]

🤖 AI Code Review

View results

🧠 요약

• 쿠키 정책 변경: 인증 쿠키의 생성 및 삭제 로직을 공통화하여 중복을 제거하고, 하드코딩된 값을 프로퍼티로 분리하여 관리성을 높였습니다. 하지만, 쿠키의 보안 설정이 제대로 적용되지 않을 경우 보안 리스크가 발생할 수 있습니다.
• 리팩토링으로 인한 버그 가능성: 기존의 로그인 및 토큰 재발급 로직이 변경되면서, 새로운 로직에서 발생할 수 있는 예외 처리 및 유효성 검사가 누락될 경우, 인증 관련 버그가 발생할 수 있습니다.
• 테스트 케이스의 보완 필요: 쿠키 기반 인증 로직에 대한 테스트가 충분히 검증되지 않을 경우, 실제 운영 환경에서 문제가 발생할 수 있습니다.

⚠️ 세부 코드 리뷰

• [src/main/java/com/server/auth/controller/AuthController.java:30] 의존성 주입 → AuthCookieProperties를 통해 쿠키 설정을 관리하게 되어 코드의 가독성과 유지보수성이 향상되었습니다. 하지만, 이 클래스의 프로퍼티가 잘못 설정될 경우, 쿠키의 보안 설정이 취약해질 수 있습니다. 따라서, 프로퍼티의 유효성 검사를 추가하는 것이 좋습니다.

• [src/main/java/com/server/auth/controller/AuthController.java:62] 중복 제거 → 쿠키 세팅 로직을 setAuthCookies 메서드로 분리하여 중복을 제거했습니다. 이는 코드의 재사용성을 높이고, 유지보수를 용이하게 합니다. 그러나, 이 메서드가 호출되는 모든 경로에서 authCookieProperties가 유효한지 확인해야 합니다.

• [src/main/java/com/server/auth/controller/AuthController.java:116] 쿠키 삭제 로직 통일 → clearAuthCookies 메서드를 통해 쿠키 삭제 로직을 통일했습니다. 이는 코드의 일관성을 높이지만, 쿠키 삭제 시 예외가 발생할 경우 적절한 예외 처리가 필요합니다.

• [src/main/java/com/server/auth/service/PasswordResetService.java:6] 도메인 이동 → PasswordResetToken 엔티티의 위치를 변경하여 도메인 구조를 정리했습니다. 이는 도메인 간의 의존성을 줄이고, 코드의 명확성을 높입니다. 하지만, 이로 인해 기존의 서비스나 레포지토리에서 이 엔티티를 참조하는 부분이 있다면, 해당 부분의 수정이 필요합니다.

• [src/test/java/com/server/auth/controller/AuthControllerTest.java:47] 테스트 케이스 보완 → 쿠키 관련 테스트 케이스가 추가되었습니다. 이는 새로운 쿠키 기반 인증 로직의 동작을 검증하는 데 도움이 됩니다. 그러나, 다양한 시나리오(예: 쿠키가 없거나 만료된 경우)에 대한 테스트가 추가되어야 합니다.

🧪 테스트 제안

• 쿠키 유효성 검사 테스트: 쿠키가 설정되지 않았거나 만료된 경우, 적절한 예외가 발생하는지 확인하는 테스트 케이스를 추가해야 합니다.
• 보안 설정 테스트: AuthCookieProperties의 설정 값이 잘못된 경우, 쿠키가 올바르게 설정되지 않는지 확인하는 테스트를 추가해야 합니다.
• 동시성 테스트: 여러 사용자가 동시에 로그인할 때 쿠키가 올바르게 설정되고, 서로의 세션에 영향을 미치지 않는지 검증하는 테스트가 필요합니다.

[github-actions]

🤖 AI Code Review

View results

🧠 요약

• 쿠키 정책 변경: 인증 쿠키의 설정을 프로퍼티로 분리하여 하드코딩을 제거한 점은 유지보수성과 유연성을 높이는 긍정적인 변화입니다. 그러나, 쿠키 설정이 잘못될 경우 보안 취약점이 발생할 수 있으므로 주의가 필요합니다.
• 중복 코드 제거: 쿠키 생성 및 삭제 로직을 공통화하여 중복을 제거한 것은 코드의 가독성과 유지보수성을 향상시킵니다. 그러나, 이 과정에서 예외 처리나 로깅이 누락될 경우 디버깅이 어려워질 수 있습니다.
• PasswordResetToken 도메인 이동: 도메인 구조를 정리한 것은 좋지만, 이로 인해 기존 코드와의 호환성 문제가 발생할 수 있으므로, 관련된 모든 코드에서의 참조를 확인해야 합니다.

⚠️ 세부 코드 리뷰

• [src/main/java/com/server/auth/config/AuthCookieProperties.java:1] 새 파일 추가 → 쿠키 설정을 프로퍼티로 분리한 것은 좋은 접근입니다. 그러나, secure, sameSite 등의 값이 잘못 설정될 경우 보안 문제가 발생할 수 있습니다. 이를 위해 기본값을 설정하거나 유효성 검사를 추가하는 것이 좋습니다.

• [src/main/java/com/server/auth/controller/AuthController.java:58] 로그인 메서드 변경 → 기존의 하드코딩된 쿠키 설정을 setAuthCookies 메서드로 분리한 것은 중복을 줄이고 가독성을 높였습니다. 그러나, 쿠키 설정 시 예외가 발생할 경우 적절한 예외 처리가 필요합니다. 예를 들어, 쿠키 설정 실패 시 사용자에게 적절한 오류 메시지를 반환해야 합니다.

• [src/main/java/com/server/auth/controller/AuthController.java:120] 로그아웃 메서드 변경 → 쿠키 삭제 로직을 clearAuthCookies 메서드로 통일한 점은 좋습니다. 하지만, 로그아웃 처리 후 사용자에게 반환되는 메시지가 단순하므로, 로그아웃 성공 여부를 클라이언트에서 확인할 수 있도록 추가적인 정보를 제공하는 것이 좋습니다.

• [src/main/java/com/server/auth/domain/PasswordResetToken.java:1] 도메인 이동 → 도메인 구조를 정리한 것은 좋지만, 기존 코드에서 이 도메인을 참조하는 부분이 많을 수 있으므로, 이로 인해 발생할 수 있는 호환성 문제를 사전에 점검해야 합니다.

🧪 테스트 제안

• 쿠키 설정 및 삭제 테스트: setAuthCookies 및 clearAuthCookies 메서드에 대한 단위 테스트를 추가하여, 쿠키가 올바르게 설정되고 삭제되는지 검증하는 테스트 케이스를 작성해야 합니다.
• 예외 처리 테스트: 쿠키 설정 시 예외가 발생하는 경우에 대한 테스트 케이스를 추가하여, 시스템이 적절한 오류 메시지를 반환하는지 확인해야 합니다.
• PasswordResetToken 도메인 이동에 따른 통합 테스트: 도메인 이동으로 인해 발생할 수 있는 모든 참조를 점검하는 통합 테스트를 작성하여, 기존 기능이 정상적으로 작동하는지 확인해야 합니다.

[github-actions]

🤖 AI Code Review

View results

🧠 요약

• 쿠키 정책 변경: 인증 쿠키를 프로퍼티로 분리하여 하드코딩을 제거한 것은 유지보수성을 높이는 긍정적인 변화입니다. 그러나 잘못된 설정으로 인해 보안 문제가 발생할 수 있습니다.
• 중복 코드 제거: 쿠키 세팅 및 삭제 로직을 공통화하여 중복을 줄인 점은 코드의 가독성과 유지보수성을 향상시킵니다. 하지만 이 과정에서 예외 처리나 로깅이 누락될 경우 디버깅이 어려워질 수 있습니다.
• 테스트 케이스 보완 필요: 쿠키 기반 인증 로직에 대한 테스트가 추가되었으나, 다양한 시나리오에 대한 테스트가 부족할 수 있습니다.

⚠️ 세부 코드 리뷰

• [src/main/java/com/server/auth/controller/AuthController.java:30] 의존성 주입 → AuthCookieProperties를 통해 쿠키 설정을 관리하는 것은 좋은 접근입니다. 그러나 이 클래스의 프로퍼티가 잘못 설정되었을 경우, 애플리케이션이 예상치 못한 동작을 할 수 있습니다. 이를 방지하기 위해 @ConfigurationProperties에 대한 유효성 검사를 추가하는 것이 좋습니다.

• [src/main/java/com/server/auth/controller/AuthController.java:62] 중복 제거 → 쿠키 세팅 로직을 setAuthCookies 메서드로 분리한 것은 중복을 줄이고 가독성을 높였습니다. 그러나 이 메서드에서 예외가 발생할 경우에 대한 처리가 필요합니다. 예를 들어, 쿠키 설정 중 오류가 발생할 경우 적절한 예외를 던지거나 로그를 남기는 로직이 필요합니다.

• [src/main/java/com/server/auth/controller/AuthController.java:116] 쿠키 삭제 로직 통일 → clearAuthCookies 메서드를 통해 쿠키 삭제 로직을 통일한 점은 좋습니다. 그러나 이 메서드에서 쿠키 삭제가 실패할 경우에 대한 처리가 필요합니다. 예를 들어, 로그를 남기거나 사용자에게 알림을 주는 방법이 있습니다.

• [src/main/java/com/server/auth/service/PasswordResetService.java:6] 도메인 이동 → PasswordResetToken 엔티티의 패키지 이동은 도메인 구조를 명확히 하여 유지보수성을 높이는 긍정적인 변화입니다. 그러나 이 변경으로 인해 기존의 의존성이 깨질 수 있으므로, 관련된 모든 코드에서 이 엔티티를 사용하는 부분을 점검해야 합니다.

🧪 테스트 제안

• 쿠키 설정 및 삭제 테스트: setAuthCookies와 clearAuthCookies 메서드에 대한 단위 테스트를 추가하여, 쿠키가 올바르게 설정되고 삭제되는지 확인하는 테스트 케이스를 작성해야 합니다.

• 유효성 검사 테스트: AuthCookieProperties의 프로퍼티에 대한 유효성 검사를 추가한 후, 잘못된 설정이 들어왔을 때 애플리케이션이 어떻게 반응하는지를 테스트하는 케이스를 작성해야 합니다.

• 예외 처리 테스트: 쿠키 세팅이나 삭제 중 예외가 발생했을 때의 동작을 검증하는 테스트를 추가하여, 시스템의 안정성을 높이는 것이 필요합니다.

[seung-in-Yoo]

PR 확인 했습니다!! 개인적으로 CSRF 방어 로직도 추가되면 더 탄탄하게 구현이 가능할 것 같습니다 ㅎㅎ 수고하셨어요!

[seung-in-Yoo]

요 부분 refreshToken이 null 이여도 호출되지 않나요?? 만약 그런거라면 컨트롤러 혹은 서비스 로직에서 관련 예외처리가 필요할 것 같아요

[jiyoon-00]

감사합니다 수정해보겠습니다~

[dbjoung]

오 지윤님 fe 단에서 fetch 로직 통일하면서 refresh 토큰 로컬 스토리지에 저장했었는데 이 패치 머지해주시면 다시 바꾸겠습니다 ㅎㅎ
(혹시 fe쪽 pr 보고 혼란 느끼실까봐 코멘트 남깁니닷)