Skip to content

Conversation

@jungdongha
Copy link
Collaborator

Authentic extension 추가

close #78

@jungdongha jungdongha self-assigned this Oct 1, 2025
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/common/exception/GlobalExceptionHandler.kt

🟢 좋은점:

  • 글로벌 익셉션 처리(@ControllerAdvice)가 잘 유지되며, IllegalStateException을 추가로 명시적으로 핸들링하여 에러 응답의 세밀함을 높임. 표준 에러 응답(409 Conflict)으로 일관성 있음.
  • 모든 핸들러에서 ApiResponse (여기서는 Void)를 사용해 응답을 감싸는 규칙을 준수함.
  • Kotlin null safety를 적절히 활용 (ex.message ?: "기본 메시지")하여 안전한 코드 작성.
  • ktlint 규칙 준수: 코드 포맷팅이 깔끔하고, 네이밍 컨벤션 (e.g., handleIllegalState)이 일관적임.

🟡 개선사항:

  • Kotlin 최적화 측면에서, 여러 예외 핸들러를 when 표현식으로 통합 고려 (현재는 개별 @ExceptionHandler로 분리되어 있지만, 복잡도가 증가할 수 있음). 예: 단일 메서드에서 when으로 예외 타입 분기.
  • 로거 메시지("부적절한 상태: {}")가 한국어로 되어 있음; 영어로 통일하거나 i18n 지원을 고려하면 국제화에 유리할 수 있음.
  • 제네릭 Exception 핸들러에서 IllegalStateException이 이미 캐치되지 않도록 순서가 맞지만, 주석에 모든 예외 매핑을 명확히 업데이트 추천 (e.g., IllegalStateException 추가 반영).

🔴 문제점:

  • 없음. 변경사항이 규칙을 위반하지 않음.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/common/security/AuthenticationExtensions.kt.kt

🟢 좋은점:

  • Kotlin 확장 함수를 활용하여 Authentication 객체를 확장하는 방식으로, Kotlin 최적화 규칙(확장함수 사용)을 잘 준수하고 있습니다. 코드가 간결하고 재사용 가능합니다.

🟡 개선사항:

  • null safety를 강화하세요. principal이 null일 수 있는 경우(예: 인증되지 않은 상태)를 고려해 principal ?: throw IllegalStateException(...)처럼 null 체크를 추가하면 더 안전합니다.
  • 타입 체크를 when 표현식으로 리팩토링하면 Kotlin 최적화(when 표현식 사용)를 더 활용할 수 있습니다. 예:
    fun Authentication.getUserId(): Long {
        return when (val principal = this.principal) {
            is Long -> principal
            else -> throw IllegalStateException("인증된 사용자 ID를 찾을 수 없거나 타입이 올바르지 않습니다.")
        }
    }
  • 예외 메시지를 영어로 통일하거나, 로깅을 추가해 디버깅을 용이하게 하는 것을 고려하세요(현재 한국어로 되어 있어 프로젝트 언어 정책에 따라 다를 수 있음).

🔴 문제점:

  • 파일명이 AuthenticationExtensions.kt.kt로 중복 확장자(.kt.kt)가 되어 있습니다. AuthenticationExtensions.kt로 수정하세요. 이는 ktlint 규칙(포맷팅 및 네이밍 컨벤션)을 위반하며, 빌드 오류를 유발할 수 있습니다.
  • Spring Security의 표준 Authentication에서 principal은 보통 UserDetails 객체(예: User)이므로, 이를 Long으로 직접 가정하는 로직이 프로젝트의 인증 설정과 맞지 않을 수 있습니다. 만약 커스텀 principal을 사용한다면 문서화하거나, 실제 타입(예: CustomUserDetails)을 체크하도록 수정하세요. 그렇지 않으면 런타임 오류가 발생할 위험이 큽니다.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/domain/auth/controller/AuthController.kt

🟢 좋은점:

  • ApiResponse를 ResponseEntity로 감싸서 반환하는 구조를 유지하며, 성공 메시지를 명확히 포함하여 일관된 응답 형식을 준수함. 이는 필수 검토 항목 2번(ApiResponse 사용)을 잘 따름.
  • Kotlin 최적화 측면에서 Authentication 객체를 받아 확장함수(getUserId())를 활용하여 사용자 ID를 추출하는 방식이 깔끔하고, 불필요한 어노테이션 의존성을 줄임. 이는 Kotlin의 확장함수 활용으로 코드 가독성과 재사용성을 높이는 좋은 예시(필수 검토 항목 3번).
  • ktlint 규칙 준수: import 순서와 코드 포맷팅이 적절하며, 네이밍 컨벤션(예: 메서드명 updateUserRole, 변수명 userId)이 일관됨(필수 검토 항목 4번).

🟡 개선사항:

  • 글로벌 익셉션 처리(필수 검토 항목 1번): 이 메서드에서 authService.updateRoleAndLogin() 호출 시 발생할 수 있는 예외(예: InvalidRoleException 등)를 고려해 @ControllerAdvice에서 표준 에러 응답으로 처리되는지 확인 필요. 만약 서비스 레이어에서 커스텀 예외를 던진다면, 컨트롤러에서 try-catch로 로깅을 추가하는 것도 고려.
  • Kotlin null safety(필수 검토 항목 3번): getUserId() 확장함수가 null을 반환할 가능성이 있다면(예: 인증되지 않은 사용자), val userId = authentication?.getUserId() ?: throw UnauthorizedException()처럼 안전하게 처리하는 것이 좋음. 현재는 AuthenticationPrincipal을 제거했으므로, 인증되지 않은 요청 시 null 체크가 더 중요해짐.
  • ktlint 규칙(필수 검토 항목 4번): 코드가 짧아 큰 문제는 없으나, 긴 줄(예: ResponseEntity.ok() 호출)이 발생할 수 있으니 ktlint 포맷팅 도구로 전체 파일을 다시 검사 추천.

🔴 문제점:

  • 없음. 변경사항이 보안 및 기능적으로 안전하며, 기존 구조를 유지하면서 개선된 점이 보임. 다만, Authentication 파

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/common/exception/GlobalExceptionHandler.kt

🟢 좋은점:

  • 글로벌 익셉션 처리(@ControllerAdvice)가 잘 유지되며, IllegalStateException을 추가로 처리하여 표준 에러 응답(409 Conflict)을 제공합니다. 이는 예외 유형별 HTTP 상태 코드를 명확히 매핑하는 데 적합합니다.
  • 모든 예외 핸들러에서 ApiResponse를 일관되게 사용하며, 제네릭 타입(Void)을 명시적으로 지정하여 응답 형식을 표준화합니다.
  • Kotlin null safety를 활용하여 ex.message ?: "기본 메시지" 형태로 안전하게 처리합니다. 이는 Kotlin의 강점인 null 안전성을 잘 반영합니다.
  • ktlint 규칙을 준수하며, 코드 포맷팅(들여쓰기, 줄 바꿈)과 네이밍 컨벤션(함수명, 변수명)이 일관적입니다.

🟡 개선사항:

  • Kotlin 최적화 측면에서, 여러 예외 핸들러를 when 표현식으로 통합하여 코드 중복을 줄일 수 있습니다. 예를 들어, handleGenericException 내에서 when으로 IllegalStateException을 분기 처리하면 핸들러 메서드 수를 줄일 수 있지만, 현재 구조도 가독성이 좋습니다.
  • 로그 메시지("부적절한 상태: {}")가 한국어로 되어 있는데, 국제화(i18n)를 고려한다면 영어로 통일하거나 메시지 키를 사용하는 것이 더 나을 수 있습니다.

🔴 문제점:

  • 없음. 변경사항이 기존 구조와 잘 맞물리며, 모든 필수 규칙을 만족합니다.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/common/security/AuthenticationExtensions.kt

🟢 좋은점:

  • Kotlin 확장 함수를 활용하여 Authentication 객체를 자연스럽게 확장하는 점이 우수합니다. 이는 Kotlin 최적화 규칙(확장 함수 사용)을 잘 따르고 있으며, 코드의 가독성과 재사용성을 높여줍니다.
  • 네이밍 컨벤션(ktlint 규칙)이 적절히 지켜지고 있으며, 함수명 getUserId()가 직관적입니다. 포맷팅도 간결하고 ktlint 규칙에 부합합니다.

🟡 개선사항:

  • principal의 null safety를 강화하세요. Spring Security에서 principal은 null일 수 있으므로, principal == null 체크를 추가하거나 principal as? Long을 사용해 안전하게 캐스팅하는 것이 좋습니다. 예:
    fun Authentication.getUserId(): Long {  
        return principal as? Long ?: throw IllegalStateException("인증된 사용자 ID를 찾을 수 없거나 타입이 올바르지 않습니다.")  
    }  
    이는 Kotlin null safety 규칙을 더 잘 활용합니다.
  • Kotlin 최적화 측면에서, 간단한 if 대신 when 표현식을 고려할 수 있습니다. (현재는 간단하지만, principal 타입이 다양해질 경우 확장성 좋음) 예:
    fun Authentication.getUserId(): Long = when (val p = principal) {  
        is Long -> p  
        else -> throw IllegalStateException("인증된 사용자 ID를 찾을 수 없거나 타입이 올바르지 않습니다.")  
    }  
  • 글로벌 익셉션 처리와 ApiResponse 규칙은 이 유틸리티 파일과 직접 관련이 없으나, 이 함수를 호출하는 컨트롤러/서비스에서 예외가 발생할 때 @ControllerAdvice로 처리되도록 보장하세요.

🔴 문제점:

  • 없음. 코드가 간단하고 규칙 위반이 명확하지 않습니다. 다만, Spring Security의 principal이 프로젝트에서 정말 Long 타입으로 설정되었는지 확인하세요. (일반적으로 UserDetails 객체이므로, 커스텀 구현이라면 문서화 추천)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/domain/auth/controller/AuthController.kt

🟢 좋은점:

  • ApiResponse를 사용하여 표준화된 응답 형식을 유지하고 있으며, 성공 메시지와 함께 적절히 반환됨. 이는 필수 검토 항목 2번을 잘 준수.
  • Kotlin 최적화 측면에서 getUserId() 확장함수를 활용하여 Authentication 객체에서 사용자 ID를 추출하는 방식이 깔끔하고, 코드 재사용성을 높임 (항목 3번).
  • ktlint 규칙 준수: 네이밍 컨벤션 (e.g., userId, authentication)과 포맷팅이 일관되며, 불필요한 공백이나 들여쓰기 문제 없음 (항목 4번).
  • 글로벌 익셉션 처리와 연계: 컨트롤러에서 직접 에러를 핸들링하지 않고 서비스 레이어로 위임하므로, @ControllerAdvice 기반의 표준 에러 응답이 적용될 수 있음 (항목 1번 준수 가능).

🟡 개선사항:

  • getUserId() 확장함수가 null을 반환할 가능성이 있다면 (e.g., 인증되지 않은 경우), null safety를 강화하기 위해 authentication.getUserId() ?: throw UnauthorizedException()처럼 명시적 예외 처리나 Elvis 연산자를 추가 고려. 현재는 non-null로 가정하지만, Kotlin의 null safety 원칙(항목 3번)을 더 강조할 수 있음.
  • @operation 어노테이션에 response 예시나 tags를 추가하면 Swagger 문서화가 더 명확해질 수 있음 (선택적 최적화).

🔴 문제점:

  • 없음. 변경사항이 보안 강화(AuthenticationPrincipal 대신 Authentication 사용)와 일관성을 유지하며, 모든 필수 항목을 위반하지 않음.

Copy link
Collaborator

@YangHJ2415 YangHJ2415 left a comment

Choose a reason for hiding this comment

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

넵 확인 했습니다.

@jungdongha jungdongha merged commit 3ab83e1 into main Oct 1, 2025
1 check passed
@jungdongha jungdongha deleted the feat/be/78 branch October 1, 2025 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security 리팩토링

3 participants