Skip to content

[FIX] admin 필터 범위 변경 - #343

Merged
hjh79gw merged 1 commit into
devfrom
fix/login-filter
Jun 3, 2026
Merged

hjh79gw merged 1 commit into
devfrom
fix/login-filter

Conversation

@hjh79gw

@hjh79gw hjh79gw commented Jun 3, 2026 •

Copy link
Copy Markdown
Contributor

📌 PR 설명


관리자 권한 필터 적용 범위 수정

✅ 완료한 기능 명세

  • 관리자 권한 필터 적용 범위를 /api/admin 전체에서 /api/admin/accounts로 변경
  • 수동 admin batch 경로가 관리자 권한 검사를 건너뛰는 테스트 추가

📸 스크린샷


💭 고민과 해결과정


🔗 관련 이슈

Closes #이슈번호


Summary by CodeRabbit

변경 사항

  • Bug Fixes

    • 관리자 계정 관련 엔드포인트에 인증이 필수로 설정되었습니다. 기타 관리자 API 엔드포인트의 접근 제어 규칙이 재정의되어 세밀한 권한 관리가 가능해졌습니다.
  • Tests

    • 경로별 접근 제어 동작을 검증하는 테스트 케이스가 추가되었습니다.

@hjh79gw hjh79gw added the FIX 버그 수정 label Jun 3, 2026
@coderabbitai

coderabbitai Bot commented Jun 3, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

이 PR은 관리자 인증 필터의 적용 범위를 /api/admin 전체에서 /api/admin/accounts 계정 관련 경로로 축소합니다. 보안 설정, 필터 로직, 테스트 케이스가 일관되게 업데이트됩니다.

Changes

Admin Authorization Scope Restriction

Layer / File(s) Summary
Security configuration path authorization
src/main/java/com/solv/wefin/global/config/SecurityConfig.java
HttpSecurity authorizeHttpRequests에서 /api/admin/accounts/** 경로는 authenticated()로 요구하고, 나머지 /api/admin 경로는 permitAll()로 변경하여 계정 엔드포인트와 일반 관리자 엔드포인트의 인증 요구를 분리합니다.
Filter constant and path detection logic
src/main/java/com/solv/wefin/global/config/security/AdminAuthorizationFilter.java
경로 판별 상수를 ADMIN_PATH_PREFIX에서 ADMIN_ACCOUNTS_PATH_PREFIX로 변경하고, isAdminPath 메서드가 /api/admin/accounts 경로와 정확히 일치하거나 하위 경로인 요청만 필터 로직을 적용하도록 수정합니다.
Test cases for scoped authorization
src/test/java/com/solv/wefin/global/config/security/AdminAuthorizationFilterTest.java
테스트 케이스의 @DisplayName을 /api/admin/accounts 경로 기준으로 구체화하고, /api/admin/batch 경로가 권한 검사를 건너뛰는 동작을 검증하는 새로운 테스트 메서드를 추가합니다.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 관리자 경로를 걸러내다,
/accounts만 지키는 필터,
나머지는 자유롭게,
권한 범위를 좁혀내며,
테스트도 함께 정렬된다! 🔐

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR 제목이 변경 내용을 명확하게 요약하고 있습니다. 'admin 필터 범위 변경'은 주요 변경사항인 관리자 권한 필터 적용 범위를 /api/admin에서 /api/admin/accounts로 변경한 것을 간결하게 표현하고 있습니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/login-filter

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/test/java/com/solv/wefin/global/config/security/AdminAuthorizationFilterTest.java (1)

65-142: ⚡ Quick win

경계 경로 회귀 테스트를 추가해 주세요.

현재 스코프 변경의 핵심은 경로 경계 처리라서, "/api/admin/accounts/123"은 권한 검사 수행, "/api/admin/accountsx"는 권한 검사 스킵 케이스를 각각 고정해두면 회귀 방지에 효과적입니다.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/test/java/com/solv/wefin/global/config/security/AdminAuthorizationFilterTest.java`
around lines 65 - 142, Add two regression tests in AdminAuthorizationFilterTest
to cover path-boundary behavior: (1) a test that sends a request to
"/api/admin/accounts/123" (use MockHttpServletRequest and
setAuthentication(UUID) to authenticate) and asserts the filter invokes
adminAuthorizationService.requireAdmin(userId) and returns 200 (or passes the
chain), and (2) a test that sends a request to "/api/admin/accountsx" and
asserts the filter skips authorization (verify(adminAuthorizationService,
never()).requireAdmin(...)) and returns 200; use the existing
filter.doFilter(...), MockFilterChain, and verify/Mockito patterns to mirror the
other tests so these boundary cases prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@src/test/java/com/solv/wefin/global/config/security/AdminAuthorizationFilterTest.java`:
- Around line 65-142: Add two regression tests in AdminAuthorizationFilterTest
to cover path-boundary behavior: (1) a test that sends a request to
"/api/admin/accounts/123" (use MockHttpServletRequest and
setAuthentication(UUID) to authenticate) and asserts the filter invokes
adminAuthorizationService.requireAdmin(userId) and returns 200 (or passes the
chain), and (2) a test that sends a request to "/api/admin/accountsx" and
asserts the filter skips authorization (verify(adminAuthorizationService,
never()).requireAdmin(...)) and returns 200; use the existing
filter.doFilter(...), MockFilterChain, and verify/Mockito patterns to mirror the
other tests so these boundary cases prevent regressions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ebaa14ed-58c7-4ff2-88a6-00313c95f50c

📥 Commits

Reviewing files that changed from the base of the PR and between f7d2e49 and 7bba5bc.

📒 Files selected for processing (3)
  • src/main/java/com/solv/wefin/global/config/SecurityConfig.java
  • src/main/java/com/solv/wefin/global/config/security/AdminAuthorizationFilter.java
  • src/test/java/com/solv/wefin/global/config/security/AdminAuthorizationFilterTest.java

@hjh79gw hjh79gw added the ready-for-review PR 리뷰 요청 label Jun 3, 2026

@cl-o-lc cl-o-lc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

신경쓰였던 부분 수정 감사합니다!! 🙏

@hjh79gw
hjh79gw merged commit 81a4499 into dev Jun 3, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

FIX 버그 수정 ready-for-review PR 리뷰 요청

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants