602 fcmjson을 private하게 관리하는 로직 작성 #627
Conversation
WalkthroughPOST 요청에 대해 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as 클라이언트
participant Service as GoogleOauthService
participant S3 as AmazonS3
participant Google as GoogleCredentials
participant Redis as Redis(캐시)
Client->>Service: getToken() 호출
Service->>Redis: 캐시된 토큰 조회
alt 캐시 히트
Redis-->>Service: 토큰 반환
Service-->>Client: 토큰 반환
else 캐시 미스
Service->>S3: GetObject(bucketName, fcmConfigPath)
S3-->>Service: 자격증명 스트림 반환
Service->>Google: GoogleCredentials.fromStream(stream).createScoped(scope)
Google-->>Service: Credentials
Service->>Google: refreshIfExpired()/access token 요청
Google-->>Service: AccessToken
Service->>Redis: 토큰 저장(expire 3600)
Service-->>Client: 토큰 반환
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @src/main/kotlin/dsm/pick2024/global/security/jwt/path/SecurityPaths.kt:
- Around line 137-139: POST_AUTHENTICATED is defined with mismatched types
across the two SecurityPaths files (in this JWT file it's an Array via val
POST_AUTHENTICATED = arrayOf("/bug/**") while the non-JWT path/SecurityPaths.kt
defines const val POST_AUTHENTICATED = "/bug/**"), which can cause confusion and
runtime issues; unify the types across both files: either make both files export
an Array<String> (e.g., val POST_AUTHENTICATED = arrayOf("/bug/**") in both) and
update SecurityConfig to import the array version and use the spread operator
when passing into antMatchers, or make both files export a String constant
(const val POST_AUTHENTICATED = "/bug/**") and adjust SecurityConfig
accordingly—pick one approach and apply it consistently to the
POST_AUTHENTICATED symbol in both files.
🧹 Nitpick comments (2)
src/main/kotlin/dsm/pick2024/infrastructure/googleoauth/service/GoogleOauthService.kt (2)
30-42: S3 객체 리소스 관리 개선 권장
s3Object.objectContent에는.use{}를 사용하고 있지만,s3Object자체도Closeable을 구현합니다. 리소스 누수를 방지하기 위해 S3 객체도 명시적으로 닫는 것을 권장합니다.🔎 S3 객체 리소스 관리 개선안
- val s3Object = amazonS3.getObject(GetObjectRequest(bucketName, fcmConfigPath)) - - s3Object.objectContent.use { inputStream -> + amazonS3.getObject(GetObjectRequest(bucketName, fcmConfigPath)).use { s3Object -> + s3Object.objectContent.use { inputStream -> - val googleCredentials = GoogleCredentials - .fromStream(inputStream) - .createScoped(listOf(scope)) + val googleCredentials = GoogleCredentials + .fromStream(inputStream) + .createScoped(listOf(scope)) - googleCredentials.refreshIfExpired() - val token = googleCredentials.accessToken.tokenValue + googleCredentials.refreshIfExpired() + val token = googleCredentials.accessToken.tokenValue - redisUtil.setDataExpire(REDIS_KEY, token, 3600) - token + redisUtil.setDataExpire(REDIS_KEY, token, 3600) + token + } }
43-45: 예외 처리 시 로깅 추가 권장예외의 원인 정보가 손실되어 디버깅이 어려울 수 있습니다. 운영 환경에서 문제 발생 시 원인 파악을 위해 원본 예외를 로깅하는 것을 권장합니다.
🔎 예외 로깅 추가 제안
} catch (e: Exception) { + // 로거가 이미 정의되어 있다면 사용, 없다면 추가 필요 + // logger.error("Failed to retrieve FCM access token from S3", e) throw GoogleOauthFailedException }로거 추가가 필요한 경우:
import org.slf4j.LoggerFactory @Service class GoogleOauthService( // ... 기존 파라미터들 ) : GoogleOauthServicePort { private val logger = LoggerFactory.getLogger(javaClass) // ... 나머지 코드 }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (5)
src/main/kotlin/dsm/pick2024/global/config/security/SecurityConfig.ktsrc/main/kotlin/dsm/pick2024/global/security/jwt/path/SecurityPaths.ktsrc/main/kotlin/dsm/pick2024/global/security/path/SecurityPaths.ktsrc/main/kotlin/dsm/pick2024/infrastructure/googleoauth/service/GoogleOauthService.ktsrc/main/resources/application.yaml
🔇 Additional comments (6)
src/main/kotlin/dsm/pick2024/global/config/security/SecurityConfig.kt (1)
47-50: 변경 사항이 올바르게 구현되었습니다.
/bug/**엔드포인트에 대한 POST 요청에 인증을 요구하는 로직이 정확하게 추가되었습니다.SecurityPaths.POST_AUTHENTICATED가 String 타입으로 정의되어 있으므로 spread 연산자 없이 사용한 것이 올바릅니다.src/main/kotlin/dsm/pick2024/global/security/path/SecurityPaths.kt (1)
138-138: 이 파일의 정의는 올바르게 사용되고 있습니다.
POST_AUTHENTICATED가 String 상수로 정의되어 SecurityConfig에서 올바르게 사용되고 있습니다. 다만, JWT 버전의 SecurityPaths 파일과의 타입 불일치 문제는jwt/path/SecurityPaths.kt파일 리뷰에서 언급했습니다.src/main/kotlin/dsm/pick2024/infrastructure/googleoauth/service/GoogleOauthService.kt (3)
13-19: 생성자 리팩토링 승인FCM 설정을 S3에서 가져오도록 생성자가 적절하게 리팩토링되었습니다. 파라미터 이름도 더 명확해졌습니다 (
credentials→scope). 의존성 주입을 통한AmazonS3사용은 테스트 가능성과 유지보수성을 향상시킵니다.
21-23: 상수 정의 개선Redis 키를 companion object의 상수로 정의한 것은 좋은 관행입니다. 매직 스트링을 제거하고 코드 유지보수성을 향상시킵니다.
3-4: AWS SDK 버전 및 의존성은 정상적으로 구성되어 있습니다.프로젝트에서 AWS SDK v1(
com.amazonaws:aws-java-sdk-s3)을 의도적으로 사용하고 있으며,AmazonS3빈이AwsConfig.kt에서@Bean어노테이션으로 올바르게 구성되어 있습니다. 두 임포트 모두GoogleOauthService에서 활용되고 있습니다.src/main/resources/application.yaml (1)
93-93: FCM 설정 파일 경로 환경 변수 필수 설정
FCM_CONFIG_PATH환경 변수는 Google Firebase 인증에 필요한 서비스 계정 자격증명 JSON 파일이 저장된 S3 경로를 지정합니다.GoogleOauthService에서 이 경로를 통해 S3에서 자격증명 파일을 다운로드하므로, 배포 환경(개발, 스테이징, 프로덕션)에서 반드시 설정되어야 합니다.S3에 Google 서비스 계정 JSON 파일을 업로드한 후, 환경 변수를 해당 S3 객체 키(예:
config/fcm-service-account.json)로 설정해 주세요.
src/main/kotlin/dsm/pick2024/global/security/jwt/path/SecurityPaths.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/kotlin/dsm/pick2024/global/config/async/AsyncConfig.kt (1)
20-36: Executor 라이프사이클(종료 훅) 보장 + shadowing 정리 권장
ThreadPoolTaskExecutor()를getAsyncExecutor()내부에서 직접 생성하면, Spring bean 라이프사이클에 걸리지 않아setWaitForTasksToCompleteOnShutdown(true)/ termination 설정이 기대대로 적용되지 않을 여지가 있습니다(정상 종료 시 스레드 정리/대기). 또한setRejectedExecutionHandler { task, executor -> ... }에서 파라미터명이 바깥의executor변수와 겹쳐 가독성이 떨어집니다.제안 diff (@bean으로 등록해서 shutdown 보장 + 파라미터명 정리)
import org.springframework.context.annotation.Configuration +import org.springframework.context.annotation.Bean import org.springframework.retry.annotation.EnableRetry import org.springframework.scheduling.annotation.AsyncConfigurer import org.springframework.scheduling.annotation.EnableAsync import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor import org.springframework.security.task.DelegatingSecurityContextAsyncTaskExecutor // 추가 import java.util.concurrent.Executor import java.util.concurrent.RejectedExecutionException @Configuration @EnableAsync @EnableRetry class AsyncConfig : AsyncConfigurer { private val log = LoggerFactory.getLogger(this::class.java) + @Bean + fun asyncTaskExecutor(): ThreadPoolTaskExecutor = + ThreadPoolTaskExecutor().apply { + corePoolSize = 2 + maxPoolSize = 5 + queueCapacity = 10 + setThreadNamePrefix("async-") + setWaitForTasksToCompleteOnShutdown(true) + setAwaitTerminationSeconds(60) + setRejectedExecutionHandler { task, threadPoolExecutor -> + log.error("비동기 작업 거부됨: task={}, executor={}", task, threadPoolExecutor) + throw RejectedExecutionException("비동기 작업 큐가 가득 찼습니다") + } + initialize() + } + override fun getAsyncExecutor(): Executor { - val executor = ThreadPoolTaskExecutor().apply { - corePoolSize = 2 - maxPoolSize = 5 - queueCapacity = 10 - setThreadNamePrefix("async-") - setWaitForTasksToCompleteOnShutdown(true) - setAwaitTerminationSeconds(60) - setRejectedExecutionHandler { task, executor -> - log.error("비동기 작업 거부됨: task={}, executor={}", task, executor) - throw RejectedExecutionException("비동기 작업 큐가 가득 찼습니다") - } - initialize() - } - - return DelegatingSecurityContextAsyncTaskExecutor(executor) + return DelegatingSecurityContextAsyncTaskExecutor(asyncTaskExecutor()) } }(검증 포인트) 애플리케이션 종료 시
async-스레드가 정상적으로 내려가는지/종료 대기가 동작하는지 로그로 확인해 주세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/main/kotlin/dsm/pick2024/global/config/async/AsyncConfig.kt
🔇 Additional comments (1)
src/main/kotlin/dsm/pick2024/global/config/async/AsyncConfig.kt (1)
10-10: 검증 결과,DelegatingSecurityContextAsyncTaskExecutor는 현재 프로젝트의 Spring Boot 2.7.16에 포함된 Spring Security 5.7.x에서 정상적으로 제공되므로 의존성 관련 우려는 없습니다. 런타임에 NoClassDefFoundError가 발생할 위험이 없으며, 해당 클래스 import와 사용이 모두 올바릅니다.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/kotlin/dsm/pick2024/global/config/async/AsyncConfig.kt (2)
34-47: 현재 방식은 Executor lifecycle 관리가 빠져 shutdown 설정이 적용되지 않을 수 있습니다.
getAsyncExecutor()에서ThreadPoolTaskExecutor()를 직접 생성하면, 컨텍스트 종료 시점에DisposableBean/destroy 훅이 호출되지 않아setWaitForTasksToCompleteOnShutdown(true)/setAwaitTerminationSeconds(60)가 기대대로 동작하지 않을 가능성이 큽니다.ThreadPoolTaskExecutor를@Bean으로 등록해 Spring이 종료를 관리하게 하는 쪽을 권장합니다.제안 diff (Executor를 Bean으로 등록하여 lifecycle 관리)
@Configuration @EnableAsync @EnableRetry class AsyncConfig : AsyncConfigurer { private val log = LoggerFactory.getLogger(this::class.java) + @org.springframework.context.annotation.Bean(name = ["asyncTaskExecutor"]) + fun asyncTaskExecutor(): ThreadPoolTaskExecutor = + ThreadPoolTaskExecutor().apply { + corePoolSize = 2 + maxPoolSize = 5 + queueCapacity = 10 + setThreadNamePrefix("async-") + setWaitForTasksToCompleteOnShutdown(true) + setAwaitTerminationSeconds(60) + setRejectedExecutionHandler { task, threadPoolExecutor -> + log.error("비동기 작업 거부됨: task={}, executor={}", task, threadPoolExecutor) + throw RejectedExecutionException("비동기 작업 큐가 가득 찼습니다") + } + initialize() + } + override fun getAsyncExecutor(): Executor { - val executor = ThreadPoolTaskExecutor().apply { - corePoolSize = 2 - maxPoolSize = 5 - queueCapacity = 10 - setThreadNamePrefix("async-") - setWaitForTasksToCompleteOnShutdown(true) - setAwaitTerminationSeconds(60) - setRejectedExecutionHandler { task, executor -> - log.error("비동기 작업 거부됨: task={}, executor={}", task, executor) - throw RejectedExecutionException("비동기 작업 큐가 가득 찼습니다") - } - initialize() - } - - return DelegatingSecurityContextAsyncTaskExecutor(executor) + return DelegatingSecurityContextAsyncTaskExecutor(asyncTaskExecutor()) }
42-45: RejectedExecutionHandler에서 예외 throw 전략은 운영 관점에서 한 번 더 합의가 필요합니다.큐 포화 시
RejectedExecutionException을 throw 하면, 해당 작업을 제출한 호출자(예: 웹 요청 스레드)까지 예외가 전파되어 사용자 장애로 나타날 수 있습니다. 의도한 정책이 “즉시 실패”인지(좋음) vs “백프레셔/완화”인지(예: CallerRunsPolicy, 드롭+메트릭 등) 확인해 주세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/main/kotlin/dsm/pick2024/global/config/async/AsyncConfig.kt
🔇 Additional comments (1)
src/main/kotlin/dsm/pick2024/global/config/async/AsyncConfig.kt (1)
10-10: 버전 호환성 문제는 없습니다. 현재 구성이 적절합니다.프로젝트의 Spring Boot 2.7.16에 포함된 Spring Security 5.7.11에서
org.springframework.security.task.DelegatingSecurityContextAsyncTaskExecutor가 정상적으로 제공됩니다(Spring Security 3.2 이후 지원). 따라서 이 클래스의 존재 및 동작은 보장됩니다.코드도 적절하게 구현되었습니다:
ThreadPoolTaskExecutor에서initialize(),setWaitForTasksToCompleteOnShutdown(true),setAwaitTerminationSeconds(60)을 설정하여 Executor의 lifecycle이 Spring에 의해 관리됩니다.DelegatingSecurityContextAsyncTaskExecutor로 래핑하여@Async에서 SecurityContext를 적절하게 전파합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/main/kotlin/dsm/pick2024/global/security/path/SecurityPaths.kt:
- Line 136: The path string "weekend-meal/application" is missing the leading
slash; locate the constant or entry in SecurityPaths.kt that defines this route
(the one currently set to "weekend-meal/application") and change it to
"/weekend-meal/application" so it matches the other routes (e.g.,
"/weekend-meal/period", "/weekend-meal/all") and works correctly with Spring
Security path matching.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/main/kotlin/dsm/pick2024/global/security/path/SecurityPaths.kt
🔇 Additional comments (1)
src/main/kotlin/dsm/pick2024/global/security/path/SecurityPaths.kt (1)
138-139: 선언 패턴은 의도적이며 용도에 맞게 설계되어 있습니다.
POST_AUTHENTICATED는 단일 경로만 포함하므로const val로 정의하는 것이 적절합니다. 반면 다른 모든 엔드포인트 정의는 여러 경로를 포함하므로val arrayOf(...)를 사용합니다.
SecurityConfig에서 사용 방식을 보면 이 설계 차이가 명확합니다:
GET_AUTHENTICATED와STU_POST_ENDPOINTS등은*연산자로 배열을 펼쳐 사용POST_AUTHENTICATED는 단일 문자열로 직접 전달단일 경로에 불필요한 배열 래핑은 오버헤드만 추가하므로 현재 구현이 올바릅니다.
src/main/kotlin/dsm/pick2024/global/security/path/SecurityPaths.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/kotlin/dsm/pick2024/global/security/path/SecurityPaths.kt (1)
138-139: 일관성을 위해 배열 형태로 변경 권장
POST_AUTHENTICATED가 단일 문자열로 선언되어 있지만, 파일 내 다른 모든 엔드포인트 상수들(SCH_POST_ENDPOINTS,STU_POST_ENDPOINTS,GET_AUTHENTICATED등)은arrayOf()를 사용하고 있습니다.향후 인증이 필요한 POST 엔드포인트가 추가될 경우 리팩토링이 필요하며, 일관성 부족으로 혼란을 야기할 수 있습니다.
♻️ 배열 형태로 변경하는 제안
- const val POST_AUTHENTICATED = "/bug/**" + val POST_AUTHENTICATED = arrayOf( + "/bug/**" + )참고: 이 변경 시
SecurityConfig.kt에서 이 상수를 사용하는 부분도 함께 수정해야 합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/main/kotlin/dsm/pick2024/global/security/path/SecurityPaths.kt
🔇 Additional comments (1)
src/main/kotlin/dsm/pick2024/global/security/path/SecurityPaths.kt (1)
135-136: GET_AUTHENTICATED 엔드포인트 추가 확인 완료
/event와/weekend-meal/application엔드포인트가 GET_AUTHENTICATED에 올바르게 추가되었습니다. 다른 보안 규칙과의 충돌을 확인한 결과:
- PERMIT_ALL_ENDPOINTS와 중복 없음
- SCH_GET_ENDPOINTS, STU_GET_ENDPOINTS 등 역할별 엔드포인트와 중복 없음
/weekend-meal/application은/weekend-meal/application-list와 별도의 경로이므로 충돌 없음- SecurityConfig.kt에서
.authenticated()으로 올바르게 처리됨다만
POST_AUTHENTICATED는const val의 단일 문자열로 정의되어 있으나, 다른 모든 엔드포인트 상수는arrayOf()를 사용합니다. 기능상 문제는 없으나 일관성을 위해arrayOf("/bug/**")로 변경하는 것을 권장합니다.
기존에 S3에서 public하게 관리되던 fcm.json파일을 private하게 관리하도록 변경하였습니다.
버그제보, 알림 기능 정상작동 하는 것 확인했습니다.
Summary by CodeRabbit
보안
인프라 개선
기타
✏️ Tip: You can customize this high-level summary in your review settings.