Skip to content

Conversation

@jungdongha
Copy link
Collaborator

@jungdongha jungdongha commented Sep 30, 2025

close #40

@jungdongha jungdongha self-assigned this Sep 30, 2025
@jungdongha jungdongha added the enhancement New feature or request label Sep 30, 2025
@jungdongha jungdongha changed the title Feat/be/40 Security 추가 작업 Sep 30, 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/config/RedisConfig.kt

🟢 좋은점:

  • Kotlin 최적화 측면에서 null safety가 잘 준수됨 (Spring 주입 의존성인 connectionFactory가 nullable하지 않게 처리됨).
  • ktlint 규칙 준수: 코드 포맷팅이 깔끔하고, 네이밍 컨벤션이 일관적 (e.g., redisTemplate 메서드명, 클래스명). 불필요한 import나 중복 코드 없음.
  • 간단한 Config 클래스 구조가 명확하며, RedisTemplate의 기본 설정 (String Serializer)이 적절히 적용됨.

🟡 개선사항:

  • Kotlin 최적화: 현재 RedisTemplate<String, String>으로 고정되어 있지만, 프로젝트에서 다양한 타입 (e.g., Object나 JSON)을 사용할 가능성이 있다면 제네릭을 더 유연하게 확장하거나 별도의 Serializer (e.g., Jackson2JsonRedisSerializer) 추가를 고려.
  • 주석 "// Key와 Value의 Serializer를 String으로 설정"은 불필요하거나 영어로 통일하는 게 좋음 (e.g., "// Set String serializers for key and value"). ktlint에서 주석 스타일이 일관되지 않으면 경고 발생 가능.
  • RedisTemplate 초기화 후 template.afterPropertiesSet() 호출을 추가하면 Serializer 적용이 더 확실해짐 (Spring이 자동 처리하지만 명시적일수록 좋음).

🔴 문제점:

  • 없음. 글로벌 익셉션 처리나 ApiResponse는 Config 클래스와 무관하므로 해당되지 않음.

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/CustomOAuth2LoginSuccessHandler.kt

🟢 좋은점:

  • Kotlin 최적화 측면에서 Cookie 생성 시 apply 블록을 사용해 체이닝이 깔끔하고 가독성이 좋습니다. null safety는 대부분 준수되며, customUser.email처럼 타입 안전한 접근을 사용합니다.
  • ktlint 규칙 준수: 코드 포맷팅이 일관적이며, 네이밍 컨벤션(예: redisKey, refreshTokenExpirationDays)이 camelCase로 적절합니다. 불필요한 공백이나 들여쓰기 오류가 없습니다.
  • RedisTemplate을 사용해 refreshToken을 서버 측에 저장하는 방식은 보안상 우수하며, 쿠키 설정(HttpOnly=true, Secure=true)으로 클라이언트 측 취약점을 최소화합니다.
  • 패키지 구조 개선(securitycommon.security)으로 모듈화가 더 명확해졌습니다.

🟡 개선사항:

  • 글로벌 익셉션 처리: 이 핸들러에서 userRepository.findByEmail(email)이 실패하거나 Redis 저장 중 예외가 발생할 수 있으나, @ControllerAdvice가 컨트롤러 중심이므로 여기서는 try-catch나 별도 예외 핸들러(예: 커스텀 ExceptionHandler)를 추가해 리다이렉트 전에 표준 에러 응답(예: 에러 페이지 리다이렉트)을 고려하세요.
  • ApiResponse 사용: 이 코드는 JSON API 응답이 아닌 리다이렉트 기반이므로 직접 적용되지 않지만, 프론트엔드 콜백 URL에서 토큰을 처리할 때 ApiResponse를 활용한 후속 API(예: 토큰 검증 엔드포인트)를 연계하면 일관성 있게 응답을 감쌀 수 있습니다.
  • Kotlin 최적화: when 표현식을 사용해 user.role 처리(예: PENDING vs. ACTIVE)를 if-else 대신 분기 처리하면 더 idiomatic하게 만들 수 있습니다. 또한, 하드코딩된 URL("http://localhost:3000/...")을 @Value로 외부화(예: application.yml의 oauth.callback.url)하면 환경별(개발/프로덕션) 유연성이 높아집니다.
  • ktlint 규칙: `max

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/CustomOAuth2User.kt

🟢 좋은점:

  • Kotlin의 제네릭과 타입 추론을 활용해 getAttribute<Any>(nameAttributeKey)를 사용하며, is Map<*, *> 패턴 매칭으로 타입 체크를 수행하는 점이 Kotlin다운 최적화입니다. when 표현식 대신 if를 사용했지만, 간단한 로직에서 적절합니다.
  • 패키지 구조를 common.security로 재구성한 점이 모듈화 측면에서 좋으며, 네이밍 컨벤션(예: nameAttributeKey)이 ktlint 규칙에 부합합니다.
  • DefaultOAuth2User를 상속받아 OAuth2 사용자 정보를 커스터마이징하는 구조가 명확하고, val 프로퍼티로 불변성을 유지합니다.

🟡 개선사항:

  • data class로 변환할 수 없는 상속 구조이지만, 추가 프로퍼티(email, nameAttributeKey)를 고려해 equals/hashCode/toString을 명시적으로 오버라이드하거나, 필요 시 sealed class 패턴으로 확장성을 높일 수 있습니다.
  • getName() 메서드에서 nameAttribute.toString() 대신 Kotlin의 확장 함수나 safe call(?.toString() ?: "default")을 활용해 더 안전하고 읽기 쉽게 할 수 있습니다. null safety를 강화하면 (예: getAttribute의 null 가능성을 고려) 코드가 더 견고해집니다.
  • ktlint 관점에서, 긴 줄(예: if 블록)은 이미 적절하지만, trailing whitespace나 import 순서를 확인해 일관성을 유지하세요.

🔴 문제점:

  • null safety 위반: getAttribute<Any>(nameAttributeKey)는 null을 반환할 수 있지만, null 체크 없이 바로 is Map<*, *>로 검사합니다. 이는 NPE를 유발할 수 있으니, nameAttribute ?: return super.getName()처럼 안전하게 처리해야 합니다.
  • unsafe cast: nameAttribute["id"] as String은 키가 존재하지 않거나 null/잘못된 타입일 때 ClassCastException을 발생시킬 수 있습니다. as? String ?: throw IllegalStateException("Invalid id") 또는 Elvis 연산자로 대체하세요. 이는 Kotlin null safety 원칙을 위반합니다

@jungdongha jungdongha merged commit ec25aa3 into main Sep 30, 2025
1 check passed
@jungdongha jungdongha deleted the feat/be/40 branch September 30, 2025 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security 부분 추가 기능 구현

3 participants