보안: 관리자 API 인증 및 인가 기능 추가 - #198
Conversation
관리자 엔드포인트(/api/v1/admin/convert/jobs 등)에 인증 로직이 누락되어 있던 보안 취약점을 수정했습니다. TenantAccessService를 주입하고 ADMIN_READ, ADMIN_WRITE 권한 검사를 추가하여 접근을 제어하도록 변경했습니다. 관련 단위 테스트도 올바르게 인증 헤더를 전달하도록 수정했습니다.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
This PR addresses a critical security gap by enforcing authentication/authorization on previously unprotected admin endpoints in AdminController, aligning admin operations with the existing tenant-claims permission model used elsewhere in the service.
Changes:
- Added admin-specific permission constants (
ADMIN_READ,ADMIN_WRITE) inTenantPermissions. - Injected
TenantAccessServiceintoAdminControllerand enforced permission checks per endpoint. - Updated tests/docs to reflect the new security enforcement (plus sentinel/changelog notes).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/clearfolio/viewer/controller/AdminController.java | Enforces tenant-claims auth and admin permissions for admin job endpoints. |
| src/main/java/com/clearfolio/viewer/auth/TenantPermissions.java | Adds ADMIN_READ / ADMIN_WRITE permission constants. |
| src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java | Updates controller construction for new dependency; adjusts requests (currently with dummy header). |
| CHANGELOG.md | Documents the admin endpoint auth/authorization change. |
| .jules/sentinel.md | Records the admin endpoint auth enforcement learning/prevention note. |
Comments suppressed due to low confidence (2)
src/main/java/com/clearfolio/viewer/controller/AdminController.java:78
- The method signature now includes
@RequestHeader HttpHeaders headers, but the JavaDoc doesn’t document the newheadersparameter. Please keep controller JavaDoc in sync with the endpoint contract.
* Deletes a conversion job.
*
* @param jobId conversion job identifier
* @return no content on success
*/
src/main/java/com/clearfolio/viewer/controller/AdminController.java:94
- The method signature now includes
@RequestHeader HttpHeaders headers, but the JavaDoc doesn’t document the newheadersparameter. Please keep controller JavaDoc in sync with the endpoint contract.
* Retries a dead-lettered conversion job.
*
* @param jobId conversion job identifier
* @return accepted response on success
*/
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void setUp() { | ||
| conversionService = mock(DocumentConversionService.class); | ||
| controller = new AdminController(conversionService); | ||
| tenantAccessService = mock(TenantAccessService.class); | ||
| controller = new AdminController(conversionService, tenantAccessService); | ||
| webTestClient = WebTestClient.bindToController(controller) |
| tenantAccessService.require(headers, TenantPermissions.ADMIN_WRITE); | ||
|
|
||
| RetryDeadLetterResult result = conversionService.retryDeadLettered(jobId, "admin"); |
Superseded by #172, which provides the same admin read/write permission model with successful exact-head CI, security, SAST, and fuzz validation. Closing the duplicate avoids conflicting patches.