Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough이 PR은 도커 컴포즈·CI 및 환경 변수들을 Keycloak 서비스 주소 및 런타임 환경변수로 전환하고 Keycloak 서비스에 헬스체크를 추가하며 userservice의 depends_on을 헬스 기반으로 변경합니다. 게이트웨이에 Reactive 메서드 보안 활성화 및 커스텀 JWT 권한 추출기를 도입하고 UserHeaderFilter의 인증/로깅/반응형 흐름을 재구성했습니다. user-service에는 JPA 감사 설정과 마스터 사용자 자동 초기화 로직을 추가하고 User 엔티티 생성자와 Feign 매핑을 일부 변경했습니다. Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Gateway as Gateway\n(SecurityConfig + UserHeaderFilter)
participant Keycloak as Keycloak
participant UserService as UserService
participant DB as Database
Client->>Gateway: 요청 (Bearer JWT)
Gateway->>Gateway: JWT 파싱 및 권한 추출\n(grantedAuthoritiesExtractor)
Gateway->>Keycloak: 토큰 검증 / 메타데이터 조회
Keycloak-->>Gateway: 검증 응답
Gateway->>Gateway: realm_access.roles 병합·ROLE_ 접두사 적용
Gateway->>UserService: 요청 전달\n(필요 시 X-User-Id / X-User-Role 주입)
UserService->>DB: 사용자 조회 / 마스터 사용자 존재 검사
DB-->>UserService: 사용자 데이터
UserService-->>Client: 응답 반환
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
gateway-server/src/main/java/com/shipflow/gatewayserver/config/UserHeaderFilter.java (1)
85-95:⚠️ Potential issue | 🟠 Major첫 번째 role만 전달하면 권한 판정이 다시 흔들립니다.
realm_access.roles는 여러 값을 가질 수 있고 순서가 권한 우선순위를 보장하지 않습니다. 지금처럼roles.get(0)을 헤더로 넘기면 downstream의 literal 비교가 다시 실패할 수 있으니, 애플리케이션 role만 필터링해서 우선순위대로 선택하거나Authentication#getAuthorities()에서 정규화된 값을 사용하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway-server/src/main/java/com/shipflow/gatewayserver/config/UserHeaderFilter.java` around lines 85 - 95, The extractRole(Jwt jwt) method currently returns roles.get(0) which is unstable; update extractRole to read realm_access.roles as a List, filter that list to only include recognised application roles (use a predefined ordered priority list or Enum of allowed roles), normalize values to your application's authority names (matching Authentication#getAuthorities()), then select the highest-priority match and return it; if no application role is found, continue throwing BusinessException(GateErrorCode.MISSING_ROLES). Ensure the Jwt claim handling and exception behavior (BusinessException/GateErrorCode) remain consistent.docker-compose.yml (1)
76-80:⚠️ Potential issue | 🟠 Majorgatewayserver도 Keycloak readiness를 기다려야 합니다.
Gateway가 Keycloak issuer 검증에 의존하지만 현재
depends_on에 keycloak이 없습니다. Userservice와 달리 gateway-server는 Keycloak이 준비되지 않은 상태에서 시작될 수 있어 JWT 디코더 초기화 중 실패할 수 있습니다.userservice에서 사용하는 것과 동일하게 healthcheck 기반depends_on을 추가하세요.수정 예시
gatewayserver: build: context: . dockerfile: Dockerfile args: MODULE: gateway-server ports: - "8000:8000" environment: <<: *service-env depends_on: <<: *service-depends-on + keycloak: + condition: service_healthy🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` around lines 76 - 80, gateway-server must wait for Keycloak readiness: update the gateway-server service in docker-compose.yml to add a healthcheck-based depends_on entry for the keycloak service (the same pattern used by userservice), so gatewayserver's depends_on references keycloak: condition: service_healthy (or the equivalent healthcheck key) to ensure Keycloak is fully ready before gateway-server starts and avoids JWT decoder init failures.
🧹 Nitpick comments (2)
user-service/src/main/java/com/shipflow/userservice/infrastructure/client/ShipmentFeignClient.java (1)
13-13: 메서드명과 HTTP 메서드를 일치시키세요.Line 13의
patchManager는@DeleteMapping어노테이션을 사용하여 DELETE 작업(삭제 대기 마킹)을 수행하지만, 메서드명은 PATCH 의미를 암시하고 있습니다. 이로 인해 호출부에서 의도를 오해할 수 있습니다.markPendingDeletionByUser등으로 이름을 변경하여 실제 동작을 명확하게 반영하는 것을 권장합니다.현재
patchManager는 UserService.java (line 105)와 UserServiceTest.java (line 342) 두 곳에서 호출됩니다.제안 diff
`@FeignClient`(name = "shipment-service") public interface ShipmentFeignClient { `@DeleteMapping`("/internal/shipment-managers/users/{userId}") - ClientApiResponse<Void> patchManager(`@PathVariable` UUID userId); + ClientApiResponse<Void> markPendingDeletionByUser(`@PathVariable` UUID userId); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user-service/src/main/java/com/shipflow/userservice/infrastructure/client/ShipmentFeignClient.java` at line 13, The method name patchManager in ShipmentFeignClient doesn't match its DELETE semantics; rename the interface method to a clear verb like markPendingDeletionByUser (or markPendingDeletion) and update all callers accordingly (UserService.java and UserServiceTest.java) so they invoke the new method name; ensure the `@DeleteMapping` and `@PathVariable` UUID userId signature remain unchanged and only the method identifier is updated to reflect the DELETE intent.user-service/src/main/java/com/shipflow/userservice/infrastructure/init/MasterUserInitializer.java (1)
36-36: 하드코딩된 UUID를 설정으로 외부화 고려master 사용자의 UUID가 하드코딩되어 있습니다. Keycloak의 master 사용자 ID와 동기화가 필요하다면, 환경 변수나 설정 파일로 외부화하면 관리가 용이합니다.
+@Value("${app.master-user.id}") +private String masterUserId; + `@Transactional` public void createMasterUserIfNotExists() { // ... - UUID masterId = UUID.fromString("0c6a758d-afe4-47a4-9f09-df82c6e99653"); + UUID masterId = UUID.fromString(masterUserId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user-service/src/main/java/com/shipflow/userservice/infrastructure/init/MasterUserInitializer.java` at line 36, MasterUserInitializer currently hardcodes the master UUID in the masterId variable; change this to read the UUID from configuration (e.g., application properties or an environment variable) and parse it (UUID.fromString) at initialization, validating the value and failing fast if missing/invalid; update any usages that reference masterId to use the injected/configured value and provide a sensible fallback or explicit error if the config key (e.g., "master.user.id" or env "MASTER_USER_ID") is not present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose.yml`:
- Around line 220-227: Remove the duplicated "environment:" key (line 223) so
there is only one environment mapping block and relocate all environment
variables under it; also delete or replace non-Keycloak variables ("user-realm",
"admin-client-id", "login-client-id") with official KC_* Keycloak variables
(e.g., KC_DB, KC_DB_URL, KC_DB_USERNAME, KC_DB_PASSWORD, KC_HTTP_ENABLED,
KC_HOSTNAME_STRICT) or remove them if unused so the environment block uses valid
Keycloak variable names and valid YAML structure.
In
`@gateway-server/src/main/java/com/shipflow/gatewayserver/config/UserHeaderFilter.java`:
- Around line 59-61: The code in UserHeaderFilter is logging sensitive JWT data
at INFO level; change the log calls that reference jwt, userId, role and
realm_access to avoid persisting sensitive claims: lower these logs to DEBUG
(e.g., replace log.info(...) with log.debug(...)) and remove the full claim dump
jwt.getClaimAsMap("realm_access") (if needed, log a minimal non-identifying
indicator or masked value instead); update the log lines in UserHeaderFilter
that currently call log.info for subject, role and realm_access accordingly.
In `@gateway-server/src/main/resources/application.yaml`:
- Line 52: The gateway currently hardcodes issuer-uri to
"http://keycloak:8080/realms/shipflow"; change it to read the realm/issuer from
the shared externalized property or environment variable used by docker-compose
and user-service (e.g., reference the common keycloak realm/issuer property or
an env var like KEYCLOAK_REALM/KEYCLOAK_ISSUER) so the gateway uses the same
configurable value instead of the fixed "shipflow" string.
In `@user-service/src/main/java/com/shipflow/userservice/domain/entity/User.java`:
- Around line 61-66: The User constructor is manually setting auditing fields
(createdAt, createdBy, updatedAt, updatedBy) which conflicts with JPA auditing
in BaseEntity; remove the manual assignments in User (the lines assigning
LocalDateTime now and setting those four fields) and instead provide an
AuditorAware<UUID> bean in this service (e.g., implement and register an
AuditorAware that returns the current user id in your JpaAuditingConfig or a
dedicated config class) so `@CreatedBy/`@LastModifiedBy on BaseEntity are
populated correctly; ensure `@EnableJpaAuditing` remains and that BaseEntity’s
`@CreatedDate/`@CreatedBy/@LastModifiedDate/@LastModifiedBy annotations are used
as the single source of truth.
In
`@user-service/src/main/java/com/shipflow/userservice/infrastructure/config/SecurityConfig.java`:
- Around line 32-35: The custom JwtDecoder bean (jwtDecoder()) in SecurityConfig
is overriding Spring Boot auto-configuration and hardcoding the issuer; either
remove this bean so Spring Boot will use
spring.security.oauth2.resourceserver.jwt.issuer-uri from application.yaml, or
change jwtDecoder() to read the issuer URI from configuration (inject
Environment or `@Value`("${spring.security.oauth2.resourceserver.jwt.issuer-uri}")
and call JwtDecoders.fromIssuerLocation(issuer)) so different environments can
set the issuer via properties.
In
`@user-service/src/main/java/com/shipflow/userservice/infrastructure/init/MasterUserInitializer.java`:
- Line 37: Remove the unused LocalDateTime variable declared as now in
MasterUserInitializer (it's a leftover from manual audit field setup) and delete
any unused imports at the top of the file related to that code; ensure no
remaining references to now exist and that the class compiles after removing the
declaration and cleaning up imports.
- Around line 22-28: The `@Transactional` on createMasterUserIfNotExists() is not
applied because initMasterUser() calls it via self-invocation inside
MasterUserInitializer; either move the master-user creation logic directly into
the ApplicationRunner lambda returned by initMasterUser() so the transactional
boundary is applied by the container, or extract createMasterUserIfNotExists()
into a separate `@Service` (e.g., MasterUserInitService) with `@Transactional` and
inject that service into MasterUserInitializer, then call
initService.createMasterUserIfNotExists() from the ApplicationRunner; update
references to initMasterUser() and createMasterUserIfNotExists() accordingly.
In `@user-service/src/main/resources/application.yaml`:
- Line 23: The issuer-uri is hardcoded to http://keycloak:8080/realms/shipflow
while keycloak.realm already uses ${KEYCLOAK_REALM}; change issuer-uri so it
derives from the same env/properties as keycloak.server-url and keycloak.realm
(e.g., compose issuer from the same KEYCLOAK_SERVER_URL/KEYCLOAK_HOST and
KEYCLOAK_REALM variables), and update the other occurrences mentioned (the block
around keycloak.server-url) to ensure issuer-uri, keycloak.server-url and
keycloak.realm all point to the same property/environment sources.
---
Outside diff comments:
In `@docker-compose.yml`:
- Around line 76-80: gateway-server must wait for Keycloak readiness: update the
gateway-server service in docker-compose.yml to add a healthcheck-based
depends_on entry for the keycloak service (the same pattern used by
userservice), so gatewayserver's depends_on references keycloak: condition:
service_healthy (or the equivalent healthcheck key) to ensure Keycloak is fully
ready before gateway-server starts and avoids JWT decoder init failures.
In
`@gateway-server/src/main/java/com/shipflow/gatewayserver/config/UserHeaderFilter.java`:
- Around line 85-95: The extractRole(Jwt jwt) method currently returns
roles.get(0) which is unstable; update extractRole to read realm_access.roles as
a List, filter that list to only include recognised application roles (use a
predefined ordered priority list or Enum of allowed roles), normalize values to
your application's authority names (matching Authentication#getAuthorities()),
then select the highest-priority match and return it; if no application role is
found, continue throwing BusinessException(GateErrorCode.MISSING_ROLES). Ensure
the Jwt claim handling and exception behavior (BusinessException/GateErrorCode)
remain consistent.
---
Nitpick comments:
In
`@user-service/src/main/java/com/shipflow/userservice/infrastructure/client/ShipmentFeignClient.java`:
- Line 13: The method name patchManager in ShipmentFeignClient doesn't match its
DELETE semantics; rename the interface method to a clear verb like
markPendingDeletionByUser (or markPendingDeletion) and update all callers
accordingly (UserService.java and UserServiceTest.java) so they invoke the new
method name; ensure the `@DeleteMapping` and `@PathVariable` UUID userId signature
remain unchanged and only the method identifier is updated to reflect the DELETE
intent.
In
`@user-service/src/main/java/com/shipflow/userservice/infrastructure/init/MasterUserInitializer.java`:
- Line 36: MasterUserInitializer currently hardcodes the master UUID in the
masterId variable; change this to read the UUID from configuration (e.g.,
application properties or an environment variable) and parse it
(UUID.fromString) at initialization, validating the value and failing fast if
missing/invalid; update any usages that reference masterId to use the
injected/configured value and provide a sensible fallback or explicit error if
the config key (e.g., "master.user.id" or env "MASTER_USER_ID") is not present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e5314f4c-7104-4aaa-a0eb-0b4828dbfc61
📒 Files selected for processing (11)
docker-compose.ymlgateway-server/src/main/java/com/shipflow/gatewayserver/config/SecurityConfig.javagateway-server/src/main/java/com/shipflow/gatewayserver/config/UserHeaderFilter.javagateway-server/src/main/resources/application.yamluser-service/src/main/java/com/shipflow/userservice/domain/entity/User.javauser-service/src/main/java/com/shipflow/userservice/infrastructure/client/ShipmentFeignClient.javauser-service/src/main/java/com/shipflow/userservice/infrastructure/config/JpaAuditingConfig.javauser-service/src/main/java/com/shipflow/userservice/infrastructure/config/SecurityConfig.javauser-service/src/main/java/com/shipflow/userservice/infrastructure/init/MasterUserInitializer.javauser-service/src/main/java/com/shipflow/userservice/presentation/controller/AuthController.javauser-service/src/main/resources/application.yaml
💤 Files with no reviewable changes (1)
- user-service/src/main/java/com/shipflow/userservice/presentation/controller/AuthController.java
Comment on lines
+61
to
+66
|
|
||
| LocalDateTime now = LocalDateTime.now(); | ||
| this.createdAt = now; | ||
| this.createdBy = id; | ||
| this.updatedAt = now; | ||
| this.updatedBy = id; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# AuditorAware 구현체 확인
echo "=== AuditorAware 구현 검색 ==="
rg -n "AuditorAware" --type java
# JpaAuditingConfig 상세 확인
echo "=== JpaAuditingConfig 내용 ==="
fd -t f "JpaAuditingConfig.java" --exec cat {}Repository: Ship-Flow/ship-flow
Length of output: 3471
🏁 Script executed:
#!/bin/bash
# Find BaseEntity in user-service
echo "=== Searching for BaseEntity in user-service ==="
fd -t f "BaseEntity.java" user-service --exec cat {}
# Look at User.java around lines 61-66 with more context
echo "=== User.java constructor context (lines 50-85) ==="
sed -n '50,85p' user-service/src/main/java/com/shipflow/userservice/domain/entity/User.javaRepository: Ship-Flow/ship-flow
Length of output: 1138
🏁 Script executed:
#!/bin/bash
# Search for BaseEntity across entire repository
echo "=== Searching for BaseEntity class ==="
fd -t f -i "baseentity.java" --exec echo "File: {}" \; --exec cat {} \;
# Read User.java class declaration and field definitions (lines 1-60)
echo "=== User.java class declaration and fields (lines 1-60) ==="
head -60 user-service/src/main/java/com/shipflow/userservice/domain/entity/User.javaRepository: Ship-Flow/ship-flow
Length of output: 4444
JPA 감사 자동화와 수동 설정의 충돌 문제
User 클래스는 common 모듈의 BaseEntity를 확장하며, BaseEntity에는 @EntityListeners(AuditingEntityListener.class)와 @CreatedDate, @CreatedBy, @LastModifiedDate, @LastModifiedBy 어노테이션이 설정되어 있습니다. 생성자에서 수동으로 설정한 createdAt, createdBy, updatedAt, updatedBy 값은 엔티티 퍼시스트 시점에 JPA 감사 리스너에 의해 덮어씌워집니다.
더욱이, user-service의 JpaAuditingConfig는 @EnableJpaAuditing만 활성화하고 AuditorAware 빈을 구현하지 않았습니다. 다른 서비스들(hub-service, product-service 등)은 AuditorAware<UUID> 빈을 제공하지만, user-service는 이를 구현하지 않아 createdBy와 updatedBy 필드가 올바르게 설정되지 않을 수 있습니다. 생성자에서의 수동 설정은 JPA 감사에 의해 덮어씌워지므로 실제 효과가 없습니다.
생성자에서의 수동 설정을 제거하고, user-service에 AuditorAware 구현을 추가하거나, 또는 현재 설계 의도를 명확히 하여 JPA 감사 설정을 조정해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@user-service/src/main/java/com/shipflow/userservice/domain/entity/User.java`
around lines 61 - 66, The User constructor is manually setting auditing fields
(createdAt, createdBy, updatedAt, updatedBy) which conflicts with JPA auditing
in BaseEntity; remove the manual assignments in User (the lines assigning
LocalDateTime now and setting those four fields) and instead provide an
AuditorAware<UUID> bean in this service (e.g., implement and register an
AuditorAware that returns the current user id in your JpaAuditingConfig or a
dedicated config class) so `@CreatedBy/`@LastModifiedBy on BaseEntity are
populated correctly; ensure `@EnableJpaAuditing` remains and that BaseEntity’s
`@CreatedDate/`@CreatedBy/@LastModifiedDate/@LastModifiedBy annotations are used
as the single source of truth.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gateway-server/src/main/java/com/shipflow/gatewayserver/config/UserHeaderFilter.java (1)
78-88:⚠️ Potential issue | 🟠 Major첫 번째 realm role을 그대로 헤더에 넣으면 권한 비교가 다시 깨질 수 있습니다.
roles.get(0)은 순서 의존적이라, role이 여러 개인 토큰에서는offline_access나default-roles-*같은 기술 role이X-User-Role로 들어갈 수 있습니다. 그러면 실제 애플리케이션 role과 다른 값이 전달돼 downstream 권한 체크가 다시 오동작합니다.gateway-server/src/main/java/com/shipflow/gatewayserver/config/SecurityConfig.javaLine 103-128에서 이미 role 정규화를 하고 있으니, 여기서도 동일한 화이트리스트/공용 로직으로 애플리케이션 role만 명시적으로 선택하는 편이 안전합니다. Based on learnings, downstream 서비스는 게이트웨이가 재주입한X-User-Role을 신뢰합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway-server/src/main/java/com/shipflow/gatewayserver/config/UserHeaderFilter.java` around lines 78 - 88, The extractRole(Jwt jwt) method currently returns roles.get(0) which is order-dependent; update extractRole to reuse the same whitelist/normalization logic from SecurityConfig (the role normalization code around lines 103-128) to filter the realm_access.roles list for application roles only (exclude technical roles like offline_access and default-roles-*, etc.), normalize to the canonical role name, and return that single application role; if no application role remains, throw GateErrorCode.MISSING_ROLES (or MISSING_REALM_ACCESS as appropriate) instead of returning the first element.
🧹 Nitpick comments (2)
user-service/src/main/java/com/shipflow/userservice/infrastructure/init/MasterUserInitializer.java (2)
21-33:@Transactional어노테이션이 의도한 대로 동작하지 않습니다
@Bean메서드에@Transactional을 선언하면 빈 생성 시점에만 트랜잭션이 적용되고, 반환된 람다(ApplicationRunner.run())가 실행될 때는 트랜잭션이 적용되지 않습니다.현재 코드는 간단한 조회/저장 로직이고 Spring Data JPA의
save()가 기본적으로 트랜잭션을 보장하므로 실제 동작에는 문제가 없지만, 어노테이션이 오해를 줄 수 있습니다.♻️ 권장 수정 방안
방안 1: 불필요한
@Transactional제거 (간단)`@Bean` -@Transactional public ApplicationRunner initMasterUser() {방안 2:
TransactionTemplate사용 (트랜잭션 명시적 필요 시)+import org.springframework.transaction.support.TransactionTemplate; + `@Configuration` `@RequiredArgsConstructor` public class MasterUserInitializer { private final UserRepository userRepository; + private final TransactionTemplate transactionTemplate; `@Bean` - `@Transactional` public ApplicationRunner initMasterUser() { - return args -> { + return args -> transactionTemplate.executeWithoutResult(status -> { String username = "master"; if (userRepository.findByUsername(username).isPresent()) { return; } UUID masterId = UUID.fromString("0c6a758d-afe4-47a4-9f09-df82c6e99653"); User master = new User(masterId, "master", "master", "master-admin", UserRole.MASTER, UserStatus.APPROVED); userRepository.save(master); - }; + }); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user-service/src/main/java/com/shipflow/userservice/infrastructure/init/MasterUserInitializer.java` around lines 21 - 33, The `@Transactional` on the initMasterUser bean is misplaced because it only affects bean creation, not the ApplicationRunner lambda; remove the `@Transactional` from the initMasterUser method in MasterUserInitializer and keep the current findByUsername/save logic (userRepository.findByUsername(...) and userRepository.save(master)); if you do need the runner to run inside a transaction instead, inject a TransactionTemplate and wrap the ApplicationRunner body with transactionTemplate.execute(...) so the lookup and save execute inside a real transaction.
35-48: 사용되지 않는 메서드 제거 필요
createMasterUserIfNotExists()메서드는 코드베이스에서 어디에서도 호출되지 않습니다. 이 메서드의 모든 로직이initMasterUser()Bean의 람다 표현식으로 이미 구현되어 있으므로, 불필요한 중복 코드입니다. 유지보수성을 위해 메서드를 삭제하세요.삭제 제안
} - - `@Transactional` - public void createMasterUserIfNotExists() { - String username = "master"; - - boolean exists = userRepository.findByUsername(username).isPresent(); - if (exists) { - return; - } - - UUID masterId = UUID.fromString("0c6a758d-afe4-47a4-9f09-df82c6e99653"); - - User master = new User(masterId, "master", "master", "master-admin", UserRole.MASTER, UserStatus.APPROVED); - userRepository.save(master); - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@user-service/src/main/java/com/shipflow/userservice/infrastructure/init/MasterUserInitializer.java` around lines 35 - 48, Remove the unused duplicate method createMasterUserIfNotExists() from MasterUserInitializer: delete the entire method (including its `@Transactional` annotation and body) since its logic is already implemented in the initMasterUser() Bean lambda; after removal, run a compile to ensure no references remain and clean up any now-unused imports or annotations in the class.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose.yml`:
- Around line 243-248: The healthcheck uses CMD-SHELL with the Bash-only
`/dev/tcp` idiom so it always fails on the Keycloak UBI 9 Micro image; replace
this with a portable check (e.g., use nc, curl, or wget against Keycloak’s HTTP
health endpoint like /health) or switch to a custom Keycloak image that includes
bash; update the docker-compose `healthcheck` block (replace the `test: [
"CMD-SHELL", "exec 3<>/dev/tcp/localhost/8080" ]` entry) to call a portable tool
or HTTP health endpoint so Keycloak reports healthy and `userservice`'s
condition: service_healthy dependency no longer blocks startup.
---
Outside diff comments:
In
`@gateway-server/src/main/java/com/shipflow/gatewayserver/config/UserHeaderFilter.java`:
- Around line 78-88: The extractRole(Jwt jwt) method currently returns
roles.get(0) which is order-dependent; update extractRole to reuse the same
whitelist/normalization logic from SecurityConfig (the role normalization code
around lines 103-128) to filter the realm_access.roles list for application
roles only (exclude technical roles like offline_access and default-roles-*,
etc.), normalize to the canonical role name, and return that single application
role; if no application role remains, throw GateErrorCode.MISSING_ROLES (or
MISSING_REALM_ACCESS as appropriate) instead of returning the first element.
---
Nitpick comments:
In
`@user-service/src/main/java/com/shipflow/userservice/infrastructure/init/MasterUserInitializer.java`:
- Around line 21-33: The `@Transactional` on the initMasterUser bean is misplaced
because it only affects bean creation, not the ApplicationRunner lambda; remove
the `@Transactional` from the initMasterUser method in MasterUserInitializer and
keep the current findByUsername/save logic (userRepository.findByUsername(...)
and userRepository.save(master)); if you do need the runner to run inside a
transaction instead, inject a TransactionTemplate and wrap the ApplicationRunner
body with transactionTemplate.execute(...) so the lookup and save execute inside
a real transaction.
- Around line 35-48: Remove the unused duplicate method
createMasterUserIfNotExists() from MasterUserInitializer: delete the entire
method (including its `@Transactional` annotation and body) since its logic is
already implemented in the initMasterUser() Bean lambda; after removal, run a
compile to ensure no references remain and clean up any now-unused imports or
annotations in the class.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 95665714-6ae8-4745-a482-985d7f890b52
📒 Files selected for processing (5)
.dockerignoredocker-compose.ymlgateway-server/src/main/java/com/shipflow/gatewayserver/config/UserHeaderFilter.javauser-service/src/main/java/com/shipflow/userservice/infrastructure/config/SecurityConfig.javauser-service/src/main/java/com/shipflow/userservice/infrastructure/init/MasterUserInitializer.java
💤 Files with no reviewable changes (1)
- user-service/src/main/java/com/shipflow/userservice/infrastructure/config/SecurityConfig.java
✅ Files skipped from review due to trivial changes (1)
- .dockerignore
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.env.example (1)
5-17:dotenv-linter경고를 없애려면 키 순서를 정렬해주세요.현재 순서로는
UnorderedKey경고가 반복 발생합니다. 린트 안정성을 위해 섹션별로 키를 정렬하는 편이 좋습니다.정렬 예시 diff
DB_HOST= -DB_PORT= DB_NAME= -DB_USER= DB_PASSWORD= +DB_PORT= +DB_USER= -KEYCLOAK_REALM= -KEYCLOAK_REALM_USER= -KEYCLOAK_ADMIN_USERNAME= +KEYCLOAK_ADMIN_CLIENT_ID= KEYCLOAK_ADMIN_PASSWORD= -KEYCLOAK_LOGIN_CLIENT_ID= -KEYCLOAK_ADMIN_CLIENT_ID= +KEYCLOAK_ADMIN_USERNAME= KEYCLOAK_DB_PASSWORD= +KEYCLOAK_LOGIN_CLIENT_ID= +KEYCLOAK_REALM= +KEYCLOAK_REALM_USER=🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 5 - 17, The .env keys are triggering dotenv-linter's UnorderedKey warning; reorder and group the keys so each section is alphabetized — e.g., group DB keys (DB_HOST, DB_NAME, DB_PASSWORD, DB_PORT, DB_USER) sorted alphabetically and group KEYCLOAK keys (KEYCLOAK_ADMIN_CLIENT_ID, KEYCLOAK_ADMIN_PASSWORD, KEYCLOAK_DB_PASSWORD, KEYCLOAK_LOGIN_CLIENT_ID, KEYCLOAK_REALM, KEYCLOAK_REALM_USER, KEYCLOAK_ADMIN_USERNAME) sorted alphabetically as well; update the key order in the file to match those sorted groups to eliminate the linter warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.env.example:
- Around line 5-17: The .env keys are triggering dotenv-linter's UnorderedKey
warning; reorder and group the keys so each section is alphabetized — e.g.,
group DB keys (DB_HOST, DB_NAME, DB_PASSWORD, DB_PORT, DB_USER) sorted
alphabetically and group KEYCLOAK keys (KEYCLOAK_ADMIN_CLIENT_ID,
KEYCLOAK_ADMIN_PASSWORD, KEYCLOAK_DB_PASSWORD, KEYCLOAK_LOGIN_CLIENT_ID,
KEYCLOAK_REALM, KEYCLOAK_REALM_USER, KEYCLOAK_ADMIN_USERNAME) sorted
alphabetically as well; update the key order in the file to match those sorted
groups to eliminate the linter warning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 319c74ea-20d6-455a-a800-d1c4eddc57fd
📒 Files selected for processing (1)
.env.example
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 19-28: The job-level env block currently exposes secrets
(KEYCLOAK_DB_PASSWORD, KEYCLOAK_ADMIN_USERNAME, KEYCLOAK_ADMIN_PASSWORD,
KEYCLOAK_REALM, KEYCLOAK_REALM_USER, KEYCLOAK_ADMIN_CLIENT_ID,
KEYCLOAK_LOGIN_CLIENT_ID, SLACK_BOT_TOKEN, GEMINI_API_KEY); move these variables
into the specific step-level env for the "Build and start services" step so only
that step (not the entire job or other actions) can access them, by removing
them from the job env and adding them under the env key inside the "Build and
start services" step.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5e882989-cc8f-4371-aa68-8359c7d4d6d5
📒 Files selected for processing (1)
.github/workflows/ci.yml
Comment on lines
+19
to
+28
| KEYCLOAK_DB_PASSWORD: ${{ secrets.KEYCLOAK_DB_PASSWORD }} | ||
| KEYCLOAK_ADMIN_USERNAME: ${{ secrets.KEYCLOAK_ADMIN_USERNAME }} | ||
| KEYCLOAK_ADMIN_PASSWORD: ${{ secrets.KEYCLOAK_ADMIN_PASSWORD }} | ||
| KEYCLOAK_REALM: ${{ secrets.KEYCLOAK_REALM }} | ||
| KEYCLOAK_REALM_USER: ${{ secrets.KEYCLOAK_REALM_USER }} | ||
| KEYCLOAK_ADMIN_CLIENT_ID: ${{ secrets.KEYCLOAK_ADMIN_CLIENT_ID }} | ||
| KEYCLOAK_LOGIN_CLIENT_ID: ${{ secrets.KEYCLOAK_LOGIN_CLIENT_ID }} | ||
|
|
||
| SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} | ||
| GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} |
Contributor
There was a problem hiding this comment.
시크릿을 job 전역 env에 두지 말고 필요한 step 범위로 축소하세요.
Line 19-28의 신규 시크릿이 job 전체(step 전부)에 노출됩니다. 외부 액션까지 포함해 노출면이 넓어져 최소 권한 원칙에 어긋납니다. Build and start services step의 env로 이동해 범위를 줄이는 게 안전합니다.
권장 수정안 (step 범위로 env 축소)
jobs:
docker-compose-test:
runs-on: ubuntu-latest
timeout-minutes: 30
env:
POSTGRES_DB: ${{ secrets.POSTGRES_DB }}
POSTGRES_USER: ${{ secrets.POSTGRES_USER }}
POSTGRES_PASSWORD: ${{ secrets.POSTGRES_PASSWORD }}
RABBITMQ_USERNAME: ${{ secrets.RABBITMQ_USERNAME }}
RABBITMQ_PASSWORD: ${{ secrets.RABBITMQ_PASSWORD }}
- KEYCLOAK_DB_PASSWORD: ${{ secrets.KEYCLOAK_DB_PASSWORD }}
- KEYCLOAK_ADMIN_USERNAME: ${{ secrets.KEYCLOAK_ADMIN_USERNAME }}
- KEYCLOAK_ADMIN_PASSWORD: ${{ secrets.KEYCLOAK_ADMIN_PASSWORD }}
- KEYCLOAK_REALM: ${{ secrets.KEYCLOAK_REALM }}
- KEYCLOAK_REALM_USER: ${{ secrets.KEYCLOAK_REALM_USER }}
- KEYCLOAK_ADMIN_CLIENT_ID: ${{ secrets.KEYCLOAK_ADMIN_CLIENT_ID }}
- KEYCLOAK_LOGIN_CLIENT_ID: ${{ secrets.KEYCLOAK_LOGIN_CLIENT_ID }}
- SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }}
- GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }}
steps:
- name: Checkout
uses: actions/checkout@v4
@@
- name: Build and start services
+ env:
+ KEYCLOAK_DB_PASSWORD: ${{ secrets.KEYCLOAK_DB_PASSWORD }}
+ KEYCLOAK_ADMIN_USERNAME: ${{ secrets.KEYCLOAK_ADMIN_USERNAME }}
+ KEYCLOAK_ADMIN_PASSWORD: ${{ secrets.KEYCLOAK_ADMIN_PASSWORD }}
+ KEYCLOAK_REALM: ${{ secrets.KEYCLOAK_REALM }}
+ KEYCLOAK_REALM_USER: ${{ secrets.KEYCLOAK_REALM_USER }}
+ KEYCLOAK_ADMIN_CLIENT_ID: ${{ secrets.KEYCLOAK_ADMIN_CLIENT_ID }}
+ KEYCLOAK_LOGIN_CLIENT_ID: ${{ secrets.KEYCLOAK_LOGIN_CLIENT_ID }}
+ SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }}
+ GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }}
run: docker compose up -d --build --wait --wait-timeout 500
timeout-minutes: 25🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 19 - 28, The job-level env block
currently exposes secrets (KEYCLOAK_DB_PASSWORD, KEYCLOAK_ADMIN_USERNAME,
KEYCLOAK_ADMIN_PASSWORD, KEYCLOAK_REALM, KEYCLOAK_REALM_USER,
KEYCLOAK_ADMIN_CLIENT_ID, KEYCLOAK_LOGIN_CLIENT_ID, SLACK_BOT_TOKEN,
GEMINI_API_KEY); move these variables into the specific step-level env for the
"Build and start services" step so only that step (not the entire job or other
actions) can access them, by removing them from the job env and adding them
under the env key inside the "Build and start services" step.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 PR 제목
[Refactor] 사용자 초기 주입 및 gateway 권한 role 비교문제 해결 (#51)
✨ 작업 내용
🔗 관련 이슈
Closes #51
리뷰 받고 싶은 포인트가 있으면 작성해주세요.
✅ 체크리스트
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
설정
인프라
기타