20260202 #207 개발자용 관리자 페이지 구현#223
Hidden character warning
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Walkthrough관리자 UI(/admin)와 액추에이터(/actuator) 경로를 메인 JWT 보안 체인에서 제외하고, 관리자 전용 SecurityFilterChain을 추가해 인메모리 관리자 사용자·폼 로그인·기본 인증 등을 구성하며 관련 애플리케이션 설정을 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant MainFilter as JwtAuthenticationFilter
participant MainSecurity as Main SecurityFilterChain
participant AdminSecurity as Admin SecurityFilterChain
participant AuthProv as DaoAuthenticationProvider
participant UserSvc as UserDetailsService
Client->>MainFilter: 요청 (경로 검사)
alt 요청이 /admin 또는 /actuator*
MainFilter-->>Client: 필터 우회 -> 전달
Client->>AdminSecurity: /admin 요청 처리
AdminSecurity->>AuthProv: 인증 시도 (폼 또는 basic)
AuthProv->>UserSvc: 사용자 조회
UserSvc-->>AuthProv: 사용자정보
AuthProv-->>AdminSecurity: 인증 토큰 발급
AdminSecurity-->>Client: 응답 (인증/리다이렉트)
else 기타 요청
MainFilter->>MainSecurity: JWT 검증 흐름 (토큰 검사)
MainSecurity-->>Client: 응답
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
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🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@backend/build.gradle`:
- Line 70: The build file contains a duplicate dependency declaration for
org.springframework.boot:spring-boot-starter-security; remove the redundant
implementation 'org.springframework.boot:spring-boot-starter-security' entry
(the one shown in the diff) so only the single existing declaration remains (the
original declaration near the top), preventing confusion while keeping the
dependency.
In
`@backend/src/main/java/org/sejongisc/backend/common/config/security/AdminSecurityConfig.java`:
- Around line 37-93: The adminSecurityFilterChain currently configures formLogin
but omits logout handling; update adminSecurityFilterChain (HttpSecurity) to add
a logout configuration tied to adminContextPath (e.g., use
logout.logoutUrl(adminContextPath + "/logout") with
logoutSuccessUrl(adminContextPath + "/login?logout"),
invalidateHttpSession(true), clearAuthentication(true) and permitAll()) and
ensure the logout URL is allowed in the requestMatchers (or included in the
existing permitAll list) so admins can terminate sessions cleanly.
In `@backend/src/main/resources/application.yml`:
- Around line 23-34: Change the actuator health/details exposure to avoid
leaking internal secrets: update the
management.health.endpoint.health.show-details setting from "always" to
"when-authorized" and remove "heapdump" from the
management.endpoints.web.exposure.include list so JVM heap dumps are not
publicly exposed; locate the keys management.endpoint.health.show-details (or
management.health.endpoint.health.show-details) and
management.endpoints.web.exposure.include in the YAML and apply these two
changes so /actuator/health requires authorization for details and heapdump is
not included in the exposed endpoints.
🧹 Nitpick comments (2)
backend/src/main/java/org/sejongisc/backend/common/auth/filter/JwtAuthenticationFilter.java (1)
77-81: 방어적 코드로는 괜찮으나,SecurityConfig의securityMatcher와 중복됩니다.
SecurityConfig.securityFilterChain의securityMatcher가 이미/admin,/actuator경로를 메인 필터 체인에서 제외하므로, 이 필터의shouldNotFilter까지 도달하지 않습니다. 방어적 중복이라면 주석으로 의도를 남겨두면 좋겠습니다.backend/src/main/java/org/sejongisc/backend/common/config/security/SecurityConfig.java (1)
67-72: 람다 기반securityMatcher는 디버깅과 Spring Security 내부 최적화에 불리합니다.현재 람다를 사용하면
toString()이 의미 없는 값이 되어 디버그 로그에서 어떤 체인이 매칭되었는지 파악하기 어렵습니다.NegatedRequestMatcher또는 커스텀RequestMatcher구현을 사용하면 가독성과 디버깅이 개선됩니다.
backend/src/main/java/org/sejongisc/backend/common/config/security/AdminSecurityConfig.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@backend/src/main/java/org/sejongisc/backend/common/config/security/AdminSecurityConfig.java`:
- Line 79: In AdminSecurityConfig update the request matcher rules so the health
endpoint is public: replace the current .requestMatchers("/actuator/health",
"/actuator/info").hasRole("ADMIN") usage with two rules — allow
.requestMatchers("/actuator/health").permitAll() and keep sensitive endpoints
restricted (e.g. .requestMatchers("/actuator/info").hasRole("ADMIN") or other
admin-only actuator paths) so health checks from load balancers/K8s remain
unauthenticated while sensitive actuator endpoints still require ADMIN; modify
the security configuration where requestMatchers is used to reflect this
separation.
- Around line 75-76: The instance-registration endpoints are currently exposed
with .requestMatchers(adminContextPath + "/instances", adminContextPath +
"/instances/**").permitAll() (and CSRF disabled), which allows unauthenticated
registrations; change this to require HTTP Basic auth by replacing permitAll()
with an authentication requirement (e.g., .authenticated() or
.hasRole("SOME_ADMIN_ROLE")) for those requestMatchers and enable/configure
httpBasic() for the admin context, and ensure CSRF settings still allow the
intended client flow (or restrict CSRF disabling to only the necessary paths) so
registrations require credentials defined in application-prod.yml; update
references in AdminSecurityConfig to use adminContextPath + "/instances"
matchers, httpBasic(), and appropriate CSRF handling.
🧹 Nitpick comments (1)
backend/src/main/java/org/sejongisc/backend/common/config/security/AdminSecurityConfig.java (1)
40-44:UserDetails생성을 별도의@Bean으로 분리하는 것을 고려해 주세요.현재
adminSecurityFilterChain내부에서InMemoryUserDetailsManager와DaoAuthenticationProvider를 직접 생성하고 있습니다. 이는 동작에 문제는 없지만, 테스트 용이성과 관심사 분리를 위해 별도 빈으로 추출하면 더 깔끔합니다.
backend/src/main/java/org/sejongisc/backend/common/config/security/AdminSecurityConfig.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/common/config/security/AdminSecurityConfig.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@backend/src/main/java/org/sejongisc/backend/common/config/security/AdminSecurityConfig.java`:
- Around line 52-80: AdminSecurityConfig currently only matches adminContextPath
+ "/**" so /actuator/** bypasses all security; update the securityMatcher call
in AdminSecurityConfig to include "/actuator/**" as well (e.g.,
securityMatcher(adminContextPath + "/**", "/actuator/**")) and then keep or
adjust the authorizeHttpRequests block: retain the
requestMatchers("/actuator/health", "/actuator/info").permitAll() so health/info
remain public and let .anyRequest().authenticated() apply to other actuator
endpoints; verify the change in the securityMatcher invocation and the
authorizeHttpRequests() block (requestMatchers for actuator) to ensure actuator
endpoints are processed by this filter chain.
backend/src/main/java/org/sejongisc/backend/common/config/security/AdminSecurityConfig.java
Show resolved
Hide resolved
discipline24
left a comment
There was a problem hiding this comment.
고생하셨습니다~ 메인 풀을 안받아서 SecurityUrls 클래스가 없을수도 있는데, comment resolve시 참고해주세요~
#207
Summary by CodeRabbit
릴리스 노트
새로운 기능
보안