Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## [Unreleased]
### Added
- **관리자 API 권한 검증 추가**
- `AdminController`의 전체 관리자 엔드포인트에 `TenantAccessService`를 통한 `ADMIN_READ` 및 `ADMIN_WRITE` 권한 검증 로직을 추가하여 인증 우회를 방지했습니다.

- **UI UX 개선**: 'Details' 버튼 클릭 시, 작업 상세 정보 로드 중에 사용자가 명시적인 로딩 상태를 확인할 수 있도록 'Loading...' 텍스트와 비활성화 상태를 표시하도록 추가했습니다.

# Changelog
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/com/clearfolio/viewer/auth/TenantPermissions.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ public final class TenantPermissions {
*/
public static final String ANALYTICS_READ = "analytics:read";

/**
* Permission required to list all conversion jobs across tenants.
*/
public static final String ADMIN_READ = "admin:read";

/**
* Permission required to modify or delete jobs globally.
*/
public static final String ADMIN_WRITE = "admin:write";

private TenantPermissions() {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,15 @@
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.http.HttpHeaders;
import org.springframework.web.bind.annotation.RequestHeader;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.server.ResponseStatusException;

import com.clearfolio.viewer.api.AdminJobListResponse;
import com.clearfolio.viewer.auth.TenantAccessService;
import com.clearfolio.viewer.auth.TenantPermissions;
import com.clearfolio.viewer.model.ConversionJob;
import com.clearfolio.viewer.service.DocumentConversionService;
import com.clearfolio.viewer.service.RetryDeadLetterResult;
Expand All @@ -26,24 +30,32 @@
public class AdminController {

private final DocumentConversionService conversionService;
private final TenantAccessService tenantAccessService;

/**
* Creates a controller for admin operations.
*
* @param conversionService conversion service
* @param tenantAccessService tenant access service
*/
public AdminController(DocumentConversionService conversionService) {
public AdminController(DocumentConversionService conversionService, TenantAccessService tenantAccessService) {
this.conversionService = conversionService;
this.tenantAccessService = tenantAccessService;
}

/**
* Retrieves all conversion jobs, optionally filtered by dead-letter status.
*
* @param deadLettered optional filter for dead-lettered jobs
* @param headers request headers carrying tenant claims
* @return list of conversion jobs
*/
@GetMapping("/api/v1/admin/convert/jobs")
public AdminJobListResponse getAllJobs(@RequestParam(required = false) Boolean deadLettered) {
public AdminJobListResponse getAllJobs(
@RequestParam(required = false) Boolean deadLettered,
@RequestHeader HttpHeaders headers) {
tenantAccessService.require(headers, TenantPermissions.ADMIN_READ);

Iterable<ConversionJob> allJobs = conversionService.getAllJobs();

if (deadLettered == null) {
Expand All @@ -63,10 +75,15 @@ public AdminJobListResponse getAllJobs(@RequestParam(required = false) Boolean d
* Deletes a conversion job.
*
* @param jobId conversion job identifier
* @param headers request headers carrying tenant claims
* @return no content on success
*/
@DeleteMapping("/api/v1/admin/convert/jobs/{jobId}")
public ResponseEntity<Void> deleteJob(@PathVariable UUID jobId) {
public ResponseEntity<Void> deleteJob(
@PathVariable UUID jobId,
@RequestHeader HttpHeaders headers) {
tenantAccessService.require(headers, TenantPermissions.ADMIN_WRITE);

conversionService.deleteJob(jobId);
return ResponseEntity.noContent().build();
}
Expand All @@ -75,10 +92,15 @@ public ResponseEntity<Void> deleteJob(@PathVariable UUID jobId) {
* Retries a dead-lettered conversion job.
*
* @param jobId conversion job identifier
* @param headers request headers carrying tenant claims
* @return accepted response on success
*/
@PostMapping("/api/v1/admin/convert/jobs/{jobId}/retry")
public ResponseEntity<Void> retryDeadLettered(@PathVariable UUID jobId) {
public ResponseEntity<Void> retryDeadLettered(
@PathVariable UUID jobId,
@RequestHeader HttpHeaders headers) {
tenantAccessService.require(headers, TenantPermissions.ADMIN_WRITE);

RetryDeadLetterResult result = conversionService.retryDeadLettered(jobId, "admin");
if (result == RetryDeadLetterResult.NOT_FOUND) {
throw new ResponseStatusException(HttpStatus.NOT_FOUND, "job not found");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,37 @@
import java.util.Arrays;
import java.util.UUID;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.test.web.reactive.server.WebTestClient;

import com.clearfolio.viewer.auth.TenantAccessService;
import com.clearfolio.viewer.auth.TenantContext;
import com.clearfolio.viewer.auth.TenantPermissions;
import com.clearfolio.viewer.model.ConversionJob;
import com.clearfolio.viewer.service.DocumentConversionService;
import com.clearfolio.viewer.service.RetryDeadLetterResult;

class AdminControllerTest {

private DocumentConversionService conversionService;
private TenantAccessService tenantAccessService;
private WebTestClient webTestClient;
private AdminController controller;

@BeforeEach
void setUp() {
conversionService = mock(DocumentConversionService.class);
controller = new AdminController(conversionService);
tenantAccessService = mock(TenantAccessService.class);

when(tenantAccessService.require(any(), any())).thenReturn(
new TenantContext("tenant-1", "user-1", java.util.Set.of())
);

controller = new AdminController(conversionService, tenantAccessService);
webTestClient = WebTestClient.bindToController(controller)
.controllerAdvice(new ApiExceptionHandler())
.build();
Expand All @@ -37,6 +50,7 @@ void getAllJobsReturnsAllJobsWhenNoFilterProvided() {

webTestClient.get()
.uri("/api/v1/admin/convert/jobs")
.header("X-Dummy", "dummy")
.exchange()
.expectStatus().isOk()
Comment on lines 51 to 55
.expectBody()
Expand All @@ -55,6 +69,7 @@ void getAllJobsFiltersByDeadLetteredTrue() {

webTestClient.get()
.uri("/api/v1/admin/convert/jobs?deadLettered=true")
.header("X-Dummy", "dummy")
.exchange()
.expectStatus().isOk()
.expectBody()
Expand All @@ -72,6 +87,7 @@ void getAllJobsFiltersByDeadLetteredFalse() {

webTestClient.get()
.uri("/api/v1/admin/convert/jobs?deadLettered=false")
.header("X-Dummy", "dummy")
.exchange()
.expectStatus().isOk()
.expectBody()
Expand All @@ -85,6 +101,7 @@ void deleteJobReturnsNoContent() {

webTestClient.delete()
.uri("/api/v1/admin/convert/jobs/" + jobId)
.header("X-Dummy", "dummy")
.exchange()
.expectStatus().isNoContent();
}
Comment on lines 102 to 107
Expand All @@ -96,6 +113,7 @@ void retryDeadLetteredReturnsAcceptedWhenAccepted() {

webTestClient.post()
.uri("/api/v1/admin/convert/jobs/" + jobId + "/retry")
.header("X-Dummy", "dummy")
.exchange()
.expectStatus().isAccepted();
}
Expand All @@ -107,6 +125,7 @@ void retryDeadLetteredReturnsNotFoundWhenNotFound() {

webTestClient.post()
.uri("/api/v1/admin/convert/jobs/" + jobId + "/retry")
.header("X-Dummy", "dummy")
.exchange()
.expectStatus().isNotFound();
}
Expand All @@ -118,6 +137,7 @@ void retryDeadLetteredReturnsConflictWhenNotEligible() {

webTestClient.post()
.uri("/api/v1/admin/convert/jobs/" + jobId + "/retry")
.header("X-Dummy", "dummy")
.exchange()
.expectStatus().isEqualTo(409); // isConflict() isn't always available depending on spring-test version, so using isEqualTo(409) is safer
}
Expand Down
Loading