diff --git a/AGENTS.md b/AGENTS.md index ae0a7b55..baeaa007 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -82,15 +82,18 @@ Codex, Cursor, opencode, …) working in this repo. - Reference implementation: xtrmLLMBatchPython's pgcrypto-encrypted Postgres credential registry (`get_credential(name)`). Reuse that pattern (a DB-backed KV is fine) unless a dedicated KV is adopted. -- **This repo applies** — it is a Spring Boot service with real runtime secrets - (artifact-token HMAC secret, tenant-claims HMAC secret). **Known deviation to - migrate:** those secrets are currently injected straight from env via Spring - placeholders in `application-buyer-demo.yml` - (`clearfolio.artifact-token.secret: ${CLEARFOLIO_ARTIFACT_TOKEN_SECRET:}`, - `clearfolio.tenant-claims.hmac-secret: ${CLEARFOLIO_TENANT_CLAIMS_HMAC_SECRET:}`, - consumed by `ArtifactLinkService` / `TenantAccessService`). Move these to a - KV-backed lookup so env is only the bootstrap transport into the KV. New - secrets/credentials must go through the KV from the start, not new env reads. +- **This repo applies** — it is a Spring Boot service with real runtime secrets. + The tenant-claims HMAC secret is loaded as + `clearfolio.tenant-claims.hmac-secret` from the Spring config-tree credential + mount selected by the non-secret `CLEARFOLIO_SECRET_CONFIG_DIR` bootstrap + setting; do not restore direct runtime environment binding for that key. + **Known deviation to migrate:** the artifact-token HMAC secret is still + injected directly from an environment placeholder in + `application-buyer-demo.yml` + (`clearfolio.artifact-token.secret: ${CLEARFOLIO_ARTIFACT_TOKEN_SECRET:}`), + consumed by `ArtifactLinkService`. Move it to a KV-backed lookup so env is + only the bootstrap transport into the KV. New secrets/credentials must go + through the KV from the start, not new env reads. ### Code exploration diff --git a/CHANGELOG.md b/CHANGELOG.md index cc5ba27b..428a9a0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ ### Security - 정책 재정의 승인자의 원문 식별자를 감사 로그에서 제거하고, 전용 회전형 키와 도메인 분리를 사용하는 HMAC 기반 `approverFingerprint`로 대체했습니다. 전용 키가 없으면 원문이나 비키 해시로 폴백하지 않고 비상관 `unavailable` 표식을 기록합니다. - 감사 가명화 키의 소유권, 회전, 보존, 사고 대응 및 GDPR상 가명정보의 개인정보 지위를 문서화하고, 원문 승인자 식별자와 승인 토큰이 로그에 남지 않는 회귀 테스트를 추가했습니다. +- 관리자 API에 서명된 tenant claim 검증, `admin:read`/`admin:write` 최소 권한, tenant 소유권 검사를 적용했습니다. 누락 및 cross-tenant 객체는 동일한 not-found 응답으로 은폐합니다. +- 관리자 delete/retry가 검증된 `TenantContext`를 tenant-aware service mutation boundary에 전달하도록 변경해, controller 우회 호출에서도 소유권 검사가 적용되고 read-then-mutate 간격이 생기지 않도록 했습니다. +- 관리자 허용·거부·미존재·재시도 불가·실패 결정을 actor/tenant별 도메인 분리 HMAC 지문으로 기록하고, raw subject·tenant·claim signature·문서 메타데이터가 감사 로그와 retry provenance에 남지 않도록 했습니다. +- buyer-demo profile의 tenant-claims HMAC secret 환경변수 직접 바인딩을 제거하고 공통 Spring config-tree secret mount에서 읽도록 변경했습니다. # Changelog @@ -54,4 +58,4 @@ - 저장소 보안 정책, Maven/GitHub Actions Dependabot 설정, 기본 CodeQL/중앙 SAST 운영 지침, 다운로드 파일명 정규화 Jazzer fuzz target을 추가해 Scorecard 보안 거버넌스 신호를 보강했습니다. ### Fixed -- 뷰어 UI의 재시도 버튼 로딩 상태가 내부 DOM을 손상시키지 않고 안전하게 복원되도록 수정 \ No newline at end of file +- 뷰어 UI의 재시도 버튼 로딩 상태가 내부 DOM을 손상시키지 않고 안전하게 복원되도록 수정 diff --git a/docs/deployment/2026-07-02-buyer-deployment-integration-playbook.md b/docs/deployment/2026-07-02-buyer-deployment-integration-playbook.md index 8a32dc84..d21a057d 100644 --- a/docs/deployment/2026-07-02-buyer-deployment-integration-playbook.md +++ b/docs/deployment/2026-07-02-buyer-deployment-integration-playbook.md @@ -33,13 +33,23 @@ The deployment cannot yet prove: ## Runtime Profile -Use the `buyer-demo` Spring profile for a buyer sandbox: +Use the `buyer-demo` Spring profile for a buyer sandbox. Runtime key material is +loaded from a Spring Boot config-tree mount. `CLEARFOLIO_SECRET_CONFIG_DIR` +selects that mount and is not itself secret. The mounted +`clearfolio.tenant-claims.hmac-secret` file must contain at least 32 UTF-8 bytes +and must be provisioned through the deployment platform's secret manager in +shared environments. + +For a local sandbox, create an owner-readable config-tree file before startup: ```bash -mkdir -p .clearfolio/buyer-demo +umask 077 +mkdir -p .clearfolio/buyer-demo/secrets +openssl rand -base64 48 \ + > .clearfolio/buyer-demo/secrets/clearfolio.tenant-claims.hmac-secret export SPRING_PROFILES_ACTIVE=buyer-demo -export CLEARFOLIO_TENANT_CLAIMS_HMAC_SECRET="replace-with-gateway-shared-secret" +export CLEARFOLIO_SECRET_CONFIG_DIR="$PWD/.clearfolio/buyer-demo/secrets/" export CLEARFOLIO_ARTIFACT_TOKEN_SECRET="replace-with-artifact-token-secret" export CLEARFOLIO_ARTIFACT_LINK_LEDGER_PATH="$PWD/.clearfolio/buyer-demo/artifact-link-ledger.log" export CLEARFOLIO_ANALYTICS_SNAPSHOT_LEDGER_PATH="$PWD/.clearfolio/buyer-demo/kpi-snapshot-ledger.log" @@ -49,8 +59,12 @@ mvn spring-boot:run ``` The profile file is -`src/main/resources/application-buyer-demo.yml`. It uses environment variables -only; no secret value is committed. +`src/main/resources/application-buyer-demo.yml`. Non-secret operational settings +may use environment variables. Tenant-claims HMAC key material is not bound from +a runtime secret environment variable; it is read as +`clearfolio.tenant-claims.hmac-secret` from the shared config-tree import in +`application.yml`. `CLEARFOLIO_TENANT_CLAIMS_MAX_SKEW_SECONDS` remains a +non-secret runtime setting. For a Power Platform embedding test, replace `CLEARFOLIO_FRAME_ANCESTORS` with the exact buyer allowlist after the gateway hostname is known. Keep it narrow; @@ -58,8 +72,8 @@ do not use a wildcard until a security owner explicitly accepts that risk. ## Gateway Claim Contract -When `CLEARFOLIO_TENANT_CLAIMS_HMAC_SECRET` is set, every protected JSON API -call must include: +When the mounted `clearfolio.tenant-claims.hmac-secret` property is present, +every protected JSON API call must include: - `X-Clearfolio-Tenant-Id` - `X-Clearfolio-Subject-Id` @@ -96,6 +110,11 @@ gateway must send **and sign** already-canonical values: e.g. `viewer:read,job:read`. Sign what the verifier will re-derive, not the raw string. +The authenticated gateway must remove all untrusted inbound +`X-Clearfolio-*` claim headers before it maps the authenticated principal, +constructs canonical claims, signs them, and forwards the replacement header +set. Browsers and external API clients are not trusted claim issuers. + Buyer-demo permission set: ```text @@ -106,17 +125,20 @@ Production role mapping should later replace this scaffold with validated gateway or OIDC claims. Do not hand-roll JWT parsing in this service. For any environment that sets `SPRING_PROFILES_ACTIVE=production`, the service -fails startup unless `CLEARFOLIO_TENANT_CLAIMS_HMAC_SECRET` is present. The -buyer-demo profile can still run unsigned for local screenshots, but production -cannot accidentally inherit that unsigned mode. +fails startup unless the config-tree mount supplies a sufficiently strong +`clearfolio.tenant-claims.hmac-secret`. Setting only +`CLEARFOLIO_SECRET_CONFIG_DIR` without the required secret file does not enable +signed claims. The buyer-demo profile can still run unsigned for local +screenshots, but production cannot accidentally inherit that unsigned mode. ## Integration Flow 1. Buyer browser, Power Platform, or internal workflow authenticates at the buyer-controlled gateway. -2. Gateway maps the principal to Clearfolio tenant id, subject id, and - permissions. -3. Gateway signs the Clearfolio headers and forwards requests to +2. Gateway strips untrusted inbound Clearfolio claim headers, maps the principal + to Clearfolio tenant id, subject id, and permissions, and canonicalizes the + mapped values. +3. Gateway signs the canonical Clearfolio headers and forwards requests to `POST /api/v1/convert/jobs`, status, viewer bootstrap, retry, artifact-link, and analytics APIs. 4. Clearfolio verifies the signed headers, enforces permissions, and hides @@ -240,8 +262,9 @@ The buyer sandbox should not be promoted to production until these gates close: - buyer-release license-policy evidence remains green with `--require-no-review`, attribution drift check remains green, and final legal release review is obtained; -- `SPRING_PROFILES_ACTIVE=production` starts only with configured signed tenant - claims and later replaces the scaffold with validated OIDC/JWT claims; +- `SPRING_PROFILES_ACTIVE=production` starts only when the config-tree mount + contains a strong `clearfolio.tenant-claims.hmac-secret`, and later replaces + the scaffold with validated OIDC/JWT claims; - validated gateway or OIDC JWT issuer, audience, expiry, key rotation, and role mapping; - durable conversion job repository with persisted state transitions; diff --git a/docs/deployment/clearfolio-buyer-connector.openapi.yaml b/docs/deployment/clearfolio-buyer-connector.openapi.yaml index cb1e90f6..2e81683c 100644 --- a/docs/deployment/clearfolio-buyer-connector.openapi.yaml +++ b/docs/deployment/clearfolio-buyer-connector.openapi.yaml @@ -34,8 +34,11 @@ paths: operationId: submitConversionJob summary: Submit a document for asynchronous preview conversion. description: > - Requires `job:create`. The buyer gateway must add signed Clearfolio - tenant headers when `CLEARFOLIO_TENANT_CLAIMS_HMAC_SECRET` is enabled. + Requires `job:create`. The authenticated buyer gateway must strip any + untrusted inbound `X-Clearfolio-*` claim headers, then add canonical + signed Clearfolio tenant headers. Runtime verification reads + `clearfolio.tenant-claims.hmac-secret` from the config-tree mount + selected by `CLEARFOLIO_SECRET_CONFIG_DIR`. parameters: - $ref: "#/components/parameters/TenantId" - $ref: "#/components/parameters/SubjectId" diff --git a/docs/security/2026-08-05-administrative-authorization.md b/docs/security/2026-08-05-administrative-authorization.md new file mode 100644 index 00000000..57004e57 --- /dev/null +++ b/docs/security/2026-08-05-administrative-authorization.md @@ -0,0 +1,71 @@ +# Tenant-scoped administrative authorization + +## Decision + +Clearfolio's administrative job endpoints are not global superuser APIs. They are tenant-scoped operations that evaluate signed subject claims, an explicit administrative permission, the requested operation, and the target job's tenant ownership on every request. + +The implementation follows deny-by-default and least-privilege principles. Listing requires `admin:read`; deletion and dead-letter retry require `admin:write`. Possessing an opaque UUID is never sufficient authorization. Missing and cross-tenant jobs intentionally return the same not-found response so an object identifier cannot be used to enumerate another tenant's documents or operational state. + +Administrative endpoints never fall back to unsigned demo-header mode. If the tenant-claims HMAC verifier is absent or its configured key contains fewer than 32 UTF-8 bytes, the endpoints return `503 Service Unavailable` before repository access. This makes a missing or weak trust anchor an observable deployment failure rather than an authorization bypass. + +## Trust boundary + +The `X-Clearfolio-*` claim headers are an internal adapter contract between Clearfolio and an authenticated gateway or host such as naruon. They are not public client credentials. The upstream gateway must authenticate the caller, construct canonical tenant, subject, permission, and issue-time claims, and sign them with the tenant-claims HMAC key. + +Clearfolio verifies the signature and freshness before evaluating permissions. Deployments must strip untrusted inbound copies of these headers before adding verified claims. The service remains standalone because the claim verifier is an injectable component, but production must not expose the internal header adapter directly to arbitrary clients. + +The tenant-claims HMAC secret is read from the shared Spring config-tree secret mount as `clearfolio.tenant-claims.hmac-secret`. The buyer-demo profile no longer maps a secret-bearing environment variable directly into runtime configuration. Environment variables may select non-secret operational values or bootstrap a mounted credential store, but runtime authentication reads the mounted property. The mounted tenant-claims key must contain at least 32 UTF-8 bytes for privileged administrative endpoints. + +## Authorization sequence + +Every endpoint applies the same fail-closed sequence: + +1. Confirm that a strong signed-claim verifier is configured; otherwise return `503` before service access. +2. Parse the tenant, subject, permissions, issue time, and claim signature. +3. Verify signed claims and their freshness. +4. Require the action-specific permission. +5. Pass the verified `TenantContext` into the object-specific service mutation. +6. Select the target through a tenant-scoped repository lookup before evaluating or applying the mutation. +7. Return a non-enumerating not-found response for absent or cross-tenant objects. +8. Emit privacy-safe authorization evidence for the resulting outcome. + +List responses filter the complete repository result to the verified tenant before applying the optional dead-letter status filter. Delete and retry do not perform controller-level read-then-write authorization. Their service contracts receive the verified tenant context and enforce ownership at the mutation boundary, so non-HTTP callers cannot reach an unscoped administrative mutation by bypassing the controller. + +The legacy two-argument retry method remains only as a compatibility contract for non-administrative adapters. Administrative HTTP flows call the tenant-aware three-argument method. The durable implementation performs the tenant-scoped selection before invoking the shared state transition and therefore does not rely on a controller-side ownership check. + +## Audit evidence + +Administrative evidence contains only: + +- a controlled action code; +- a controlled outcome code; +- HTTP status; +- tenant and actor HMAC fingerprints in separate domains; +- an opaque job UUID when applicable; +- a numeric result count for list operations. + +It does not contain raw tenant identifiers, raw subject identifiers, claim signatures, permission headers, filenames, job messages, document text, or artifact bytes. The retry provenance stored with a job uses the actor-domain fingerprint rather than the source subject identifier. Pseudonymized values remain personal data and inherit the retention, access, rotation, and incident-response requirements in `2026-08-04-audit-pseudonymization.md`. + +## Verification requirements + +Automated tests must exercise the real signed-claim verifier and prove: + +- absent and weak verifier keys make privileged endpoints unavailable before service access; +- missing, malformed, expired, and incorrectly signed claims fail before service access; +- missing `admin:read` or `admin:write` permissions fail before service access; +- list results contain only tenant-owned jobs for all dead-letter filter states; +- missing and cross-tenant delete/retry targets produce indistinguishable not-found responses; +- delete and retry cross a tenant-aware service boundary without a separate controller lookup or unscoped administrative mutation call; +- the durable retry service rejects null, missing, and cross-tenant contexts without state transition or worker enqueue; +- accepted retry provenance is a domain-separated keyed fingerprint, never a raw or unkeyed subject value; +- not-found, not-eligible, repository failure, deletion failure, and retry failure paths return stable non-leaking responses; +- audit output contains no raw tenant, subject, signature, filename, or document data; +- JaCoCo reports 100% line and branch coverage for the `com.clearfolio.viewer.*` production package. + +## References + +Hu, V. C., Ferraiolo, D., Kuhn, D. R., Schnitzer, A., Sandlin, K., Miller, R., & Scarfone, K. (2014). *Guide to attribute based access control (ABAC) definition and considerations* (NIST Special Publication 800-162, updated August 2, 2019). National Institute of Standards and Technology. https://doi.org/10.6028/NIST.SP.800-162 + +OWASP Foundation. (2023). *API1:2023 broken object level authorization*. OWASP API Security Top 10. https://owasp.org/API-Security/editions/2023/en/0xa1-broken-object-level-authorization/ + +OWASP Foundation. (n.d.). *Authorization cheat sheet*. OWASP Cheat Sheet Series. Retrieved August 5, 2026, from https://cheatsheetseries.owasp.org/cheatsheets/Authorization_Cheat_Sheet.html diff --git a/src/main/java/com/clearfolio/viewer/audit/AdministrativeAuditLogger.java b/src/main/java/com/clearfolio/viewer/audit/AdministrativeAuditLogger.java new file mode 100644 index 00000000..b678279d --- /dev/null +++ b/src/main/java/com/clearfolio/viewer/audit/AdministrativeAuditLogger.java @@ -0,0 +1,167 @@ +package com.clearfolio.viewer.audit; + +import java.util.UUID; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatusCode; +import org.springframework.stereotype.Component; + +import com.clearfolio.viewer.auth.TenantContext; +import com.clearfolio.viewer.config.ConversionProperties; +import com.clearfolio.viewer.security.AuditPseudonymizer; + +/** + * Emits structured, privacy-safe administrative authorization evidence. + * + *

Actor and tenant identifiers are pseudonymized in separate HMAC domains. + * The logger never emits raw claim headers, subject identifiers, tenant + * identifiers, tokens, filenames, job messages, or document content.

+ */ +@Component +public final class AdministrativeAuditLogger { + + private static final Logger LOGGER = LoggerFactory.getLogger(AdministrativeAuditLogger.class); + private static final String NO_JOB_ID = "none"; + private static final int NO_RESULT_COUNT = -1; + + private final AuditPseudonymizer actorPseudonymizer; + private final AuditPseudonymizer tenantPseudonymizer; + + /** + * Administrative actions represented in authorization evidence. + */ + public enum Action { + /** Lists tenant-owned conversion jobs. */ + LIST_JOBS, + /** Deletes one tenant-owned conversion job. */ + DELETE_JOB, + /** Retries one tenant-owned dead-lettered conversion job. */ + RETRY_JOB + } + + /** + * Stable outcomes represented in authorization evidence. + */ + public enum Outcome { + /** Authorization and the requested operation succeeded. */ + ALLOWED, + /** Authentication or permission evaluation denied the request. */ + DENIED, + /** The resource was absent or intentionally concealed. */ + NOT_FOUND, + /** The resource existed but its state rejected the operation. */ + NOT_ELIGIBLE, + /** Authorization succeeded but the operation failed unexpectedly. */ + FAILED + } + + /** + * Creates the logger from the dedicated audit pseudonym configuration. + * + * @param properties conversion and audit configuration + */ + public AdministrativeAuditLogger(ConversionProperties properties) { + this.actorPseudonymizer = AuditPseudonymizer.forAdministrativeActor( + properties.getAuditPseudonymSecret(), + properties.getAuditPseudonymKeyVersion() + ); + this.tenantPseudonymizer = AuditPseudonymizer.forAdministrativeTenant( + properties.getAuditPseudonymSecret(), + properties.getAuditPseudonymKeyVersion() + ); + } + + /** + * Returns a privacy-safe actor identifier suitable for retry provenance. + * + * @param context authenticated tenant context, or null when unavailable + * @return administrative actor fingerprint + */ + public String actorFingerprint(TenantContext context) { + return actorPseudonymizer.fingerprint(context == null ? null : context.subjectId()); + } + + /** + * Records an authorization decision using authenticated context values. + * + * @param context authenticated tenant context, or null when unavailable + * @param action administrative action + * @param outcome decision outcome + * @param status response status + * @param jobId optional opaque job identifier + * @param resultCount optional list result count + */ + public void record( + TenantContext context, + Action action, + Outcome outcome, + HttpStatusCode status, + UUID jobId, + Integer resultCount + ) { + recordIdentifiers( + context == null ? null : context.tenantId(), + context == null ? null : context.subjectId(), + action, + outcome, + status, + jobId, + resultCount + ); + } + + /** + * Records a denied request using untrusted headers only as HMAC inputs. + * + * @param headers request headers, or null when unavailable + * @param action administrative action + * @param outcome decision outcome + * @param status response status + * @param jobId optional opaque job identifier + */ + public void recordHeaders( + HttpHeaders headers, + Action action, + Outcome outcome, + HttpStatusCode status, + UUID jobId + ) { + recordIdentifiers( + firstHeader(headers, TenantContext.TENANT_ID_HEADER), + firstHeader(headers, TenantContext.SUBJECT_ID_HEADER), + action, + outcome, + status, + jobId, + null + ); + } + + private void recordIdentifiers( + String tenantId, + String subjectId, + Action action, + Outcome outcome, + HttpStatusCode status, + UUID jobId, + Integer resultCount + ) { + LOGGER.info( + "Administrative access decision action={} outcome={} status={} " + + "tenantFingerprint={} actorFingerprint={} jobId={} resultCount={}", + action, + outcome, + status.value(), + tenantPseudonymizer.fingerprint(tenantId), + actorPseudonymizer.fingerprint(subjectId), + jobId == null ? NO_JOB_ID : jobId, + resultCount == null ? NO_RESULT_COUNT : resultCount + ); + } + + private String firstHeader(HttpHeaders headers, String name) { + return headers == null ? null : headers.getFirst(name); + } +} diff --git a/src/main/java/com/clearfolio/viewer/auth/TenantAccessService.java b/src/main/java/com/clearfolio/viewer/auth/TenantAccessService.java index ab02e96a..ef5a32d8 100644 --- a/src/main/java/com/clearfolio/viewer/auth/TenantAccessService.java +++ b/src/main/java/com/clearfolio/viewer/auth/TenantAccessService.java @@ -26,6 +26,7 @@ public class TenantAccessService { private static final String HMAC_SHA_256 = "HmacSHA256"; + private static final int MIN_SIGNED_CLAIMS_SECRET_BYTES = 32; private static final Base64.Encoder URL_ENCODER = Base64.getUrlEncoder().withoutPadding(); private final String claimsHmacSecret; @@ -61,6 +62,10 @@ public TenantAccessService( /** * Resolves tenant claims and verifies the required permission. * + *

This method preserves the repository's explicit unsigned demo mode for + * non-privileged local flows. Privileged endpoints must use + * {@link #requireSigned(HttpHeaders, String)} instead.

+ * * @param headers request headers * @param permission required permission * @return verified tenant context @@ -81,6 +86,27 @@ public TenantContext require(HttpHeaders headers, String permission) { return context; } + /** + * Resolves claims for a privileged endpoint and requires a strong verifier. + * + *

The endpoint is unavailable rather than falling back to unsigned + * client-supplied headers when the signed-claim HMAC secret is absent or + * contains fewer than 32 UTF-8 bytes.

+ * + * @param headers request headers + * @param permission required permission + * @return verified tenant context + */ + public TenantContext requireSigned(HttpHeaders headers, String permission) { + if (!hasStrongSignedClaimsVerifier()) { + throw new ResponseStatusException( + HttpStatus.SERVICE_UNAVAILABLE, + "signed auth verifier unavailable" + ); + } + return require(headers, permission); + } + /** * Hides resources that do not belong to the request tenant. * @@ -93,6 +119,12 @@ public void requireSameTenant(TenantContext context, ConversionJob job) { } } + private boolean hasStrongSignedClaimsVerifier() { + return claimsHmacSecret != null + && claimsHmacSecret.getBytes(StandardCharsets.UTF_8).length + >= MIN_SIGNED_CLAIMS_SECRET_BYTES; + } + private void requireSignedClaimsWhenConfigured(HttpHeaders headers, TenantContext context) { if (claimsHmacSecret == null) { return; diff --git a/src/main/java/com/clearfolio/viewer/auth/TenantPermissions.java b/src/main/java/com/clearfolio/viewer/auth/TenantPermissions.java index ced5e6a3..5d537241 100644 --- a/src/main/java/com/clearfolio/viewer/auth/TenantPermissions.java +++ b/src/main/java/com/clearfolio/viewer/auth/TenantPermissions.java @@ -50,6 +50,16 @@ public final class TenantPermissions { */ public static final String ANALYTICS_READ = "analytics:read"; + /** + * Permission required to list tenant-scoped administrative job state. + */ + public static final String ADMIN_READ = "admin:read"; + + /** + * Permission required to mutate tenant-scoped administrative job state. + */ + public static final String ADMIN_WRITE = "admin:write"; + private TenantPermissions() { } } diff --git a/src/main/java/com/clearfolio/viewer/controller/AdminController.java b/src/main/java/com/clearfolio/viewer/controller/AdminController.java index 412d4eb8..872f0a66 100644 --- a/src/main/java/com/clearfolio/viewer/controller/AdminController.java +++ b/src/main/java/com/clearfolio/viewer/controller/AdminController.java @@ -4,88 +4,269 @@ import java.util.List; import java.util.UUID; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; +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.audit.AdministrativeAuditLogger; +import com.clearfolio.viewer.audit.AdministrativeAuditLogger.Action; +import com.clearfolio.viewer.audit.AdministrativeAuditLogger.Outcome; +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; /** - * Controller for admin-specific endpoints. + * Exposes tenant-scoped administrative conversion-job operations. + * + *

Every endpoint requires strongly configured signed gateway claims, + * evaluates a least-privilege permission, and delegates object-level tenant + * authorization to the same service boundary that performs each mutation. + * Missing and cross-tenant objects intentionally share the same not-found + * response.

*/ @RestController public class AdminController { private final DocumentConversionService conversionService; + private final TenantAccessService tenantAccessService; + private final AdministrativeAuditLogger auditLogger; /** - * Creates a controller for admin operations. + * Creates a controller for authenticated tenant-administrator operations. * * @param conversionService conversion service + * @param tenantAccessService signed-claim authorization service + * @param auditLogger privacy-safe administrative evidence logger */ - public AdminController(DocumentConversionService conversionService) { + public AdminController( + DocumentConversionService conversionService, + TenantAccessService tenantAccessService, + AdministrativeAuditLogger auditLogger + ) { this.conversionService = conversionService; + this.tenantAccessService = tenantAccessService; + this.auditLogger = auditLogger; } /** - * Retrieves all conversion jobs, optionally filtered by dead-letter status. + * Retrieves conversion jobs owned by the authenticated tenant. * - * @param deadLettered optional filter for dead-lettered jobs - * @return list of conversion jobs + * @param deadLettered optional dead-letter status filter + * @param headers signed gateway claim headers + * @return tenant-scoped list of conversion jobs */ @GetMapping("/api/v1/admin/convert/jobs") - public AdminJobListResponse getAllJobs(@RequestParam(required = false) Boolean deadLettered) { - Iterable allJobs = conversionService.getAllJobs(); + public AdminJobListResponse getAllJobs( + @RequestParam(required = false) Boolean deadLettered, + @RequestHeader HttpHeaders headers + ) { + TenantContext context = authorize( + headers, + TenantPermissions.ADMIN_READ, + Action.LIST_JOBS, + null + ); - if (deadLettered == null) { - return AdminJobListResponse.from(allJobs); - } - - List filtered = new ArrayList<>(); - for (ConversionJob job : allJobs) { - if (job.isDeadLettered() == deadLettered) { - filtered.add(job); + try { + List filtered = new ArrayList<>(); + for (ConversionJob job : conversionService.getAllJobs()) { + boolean tenantOwned = job.belongsToTenant(context.tenantId()); + boolean deadLetterMatches = deadLettered == null + || job.isDeadLettered() == deadLettered; + if (tenantOwned && deadLetterMatches) { + filtered.add(job); + } } + auditLogger.record( + context, + Action.LIST_JOBS, + Outcome.ALLOWED, + HttpStatus.OK, + null, + filtered.size() + ); + return AdminJobListResponse.from(filtered); + } catch (RuntimeException ex) { + auditLogger.record( + context, + Action.LIST_JOBS, + Outcome.FAILED, + HttpStatus.INTERNAL_SERVER_ERROR, + null, + null + ); + throw ex; } - return AdminJobListResponse.from(filtered); } /** - * Deletes a conversion job. + * Deletes one conversion job owned by the authenticated tenant. * * @param jobId conversion job identifier + * @param headers signed gateway claim headers * @return no content on success */ @DeleteMapping("/api/v1/admin/convert/jobs/{jobId}") - public ResponseEntity deleteJob(@PathVariable UUID jobId) { - conversionService.deleteJob(jobId); + public ResponseEntity deleteJob( + @PathVariable UUID jobId, + @RequestHeader HttpHeaders headers + ) { + TenantContext context = authorize( + headers, + TenantPermissions.ADMIN_WRITE, + Action.DELETE_JOB, + jobId + ); + + boolean deleted; + try { + deleted = conversionService.deleteJob(jobId, context); + } catch (RuntimeException ex) { + auditLogger.record( + context, + Action.DELETE_JOB, + Outcome.FAILED, + HttpStatus.INTERNAL_SERVER_ERROR, + jobId, + null + ); + throw ex; + } + + if (!deleted) { + auditLogger.record( + context, + Action.DELETE_JOB, + Outcome.NOT_FOUND, + HttpStatus.NOT_FOUND, + jobId, + null + ); + throw notFound(); + } + + auditLogger.record( + context, + Action.DELETE_JOB, + Outcome.ALLOWED, + HttpStatus.NO_CONTENT, + jobId, + null + ); return ResponseEntity.noContent().build(); } /** - * Retries a dead-lettered conversion job. + * Retries one dead-lettered conversion job owned by the authenticated tenant. * * @param jobId conversion job identifier + * @param headers signed gateway claim headers * @return accepted response on success */ @PostMapping("/api/v1/admin/convert/jobs/{jobId}/retry") - public ResponseEntity retryDeadLettered(@PathVariable UUID jobId) { - RetryDeadLetterResult result = conversionService.retryDeadLettered(jobId, "admin"); - if (result == RetryDeadLetterResult.NOT_FOUND) { - throw new ResponseStatusException(HttpStatus.NOT_FOUND, "job not found"); + public ResponseEntity retryDeadLettered( + @PathVariable UUID jobId, + @RequestHeader HttpHeaders headers + ) { + TenantContext context = authorize( + headers, + TenantPermissions.ADMIN_WRITE, + Action.RETRY_JOB, + jobId + ); + + RetryDeadLetterResult result; + try { + result = conversionService.retryDeadLettered( + jobId, + context, + auditLogger.actorFingerprint(context) + ); + } catch (RuntimeException ex) { + auditLogger.record( + context, + Action.RETRY_JOB, + Outcome.FAILED, + HttpStatus.INTERNAL_SERVER_ERROR, + jobId, + null + ); + throw ex; } - if (result == RetryDeadLetterResult.NOT_ELIGIBLE) { - throw new ResponseStatusException(HttpStatus.CONFLICT, "job is not eligible for retry"); + + return switch (result) { + case ACCEPTED -> { + auditLogger.record( + context, + Action.RETRY_JOB, + Outcome.ALLOWED, + HttpStatus.ACCEPTED, + jobId, + null + ); + yield ResponseEntity.accepted().build(); + } + case NOT_FOUND -> { + auditLogger.record( + context, + Action.RETRY_JOB, + Outcome.NOT_FOUND, + HttpStatus.NOT_FOUND, + jobId, + null + ); + throw notFound(); + } + case NOT_ELIGIBLE -> { + auditLogger.record( + context, + Action.RETRY_JOB, + Outcome.NOT_ELIGIBLE, + HttpStatus.CONFLICT, + jobId, + null + ); + throw new ResponseStatusException( + HttpStatus.CONFLICT, + "job is not eligible for retry" + ); + } + }; + } + + private TenantContext authorize( + HttpHeaders headers, + String permission, + Action action, + UUID jobId + ) { + try { + return tenantAccessService.requireSigned(headers, permission); + } catch (ResponseStatusException ex) { + auditLogger.recordHeaders( + headers, + action, + Outcome.DENIED, + ex.getStatusCode(), + jobId + ); + throw ex; } - return ResponseEntity.accepted().build(); + } + + private ResponseStatusException notFound() { + return new ResponseStatusException(HttpStatus.NOT_FOUND, "job not found"); } } diff --git a/src/main/java/com/clearfolio/viewer/security/AuditPseudonymizer.java b/src/main/java/com/clearfolio/viewer/security/AuditPseudonymizer.java index ed3bd31d..961ae02c 100644 --- a/src/main/java/com/clearfolio/viewer/security/AuditPseudonymizer.java +++ b/src/main/java/com/clearfolio/viewer/security/AuditPseudonymizer.java @@ -21,6 +21,8 @@ public final class AuditPseudonymizer { private static final String HMAC_SHA_256 = "HmacSHA256"; private static final String DEFAULT_KEY_VERSION = "v1"; private static final String APPROVER_DOMAIN = "clearfolio:audit-approver:v1"; + private static final String ADMIN_ACTOR_DOMAIN = "clearfolio:audit-admin-actor:v1"; + private static final String ADMIN_TENANT_DOMAIN = "clearfolio:audit-admin-tenant:v1"; private static final int FINGERPRINT_BYTES = 16; private static final int MIN_SECRET_BYTES = 32; private static final int MAX_KEY_VERSION_LENGTH = 32; @@ -41,6 +43,28 @@ public AuditPseudonymizer(String secret, String keyVersion) { this(secret, keyVersion, APPROVER_DOMAIN); } + /** + * Creates a pseudonymizer for administrative actor identifiers. + * + * @param secret dedicated audit pseudonym secret + * @param keyVersion non-sensitive key-rotation identifier + * @return actor-domain pseudonymizer + */ + public static AuditPseudonymizer forAdministrativeActor(String secret, String keyVersion) { + return new AuditPseudonymizer(secret, keyVersion, ADMIN_ACTOR_DOMAIN); + } + + /** + * Creates a pseudonymizer for administrative tenant identifiers. + * + * @param secret dedicated audit pseudonym secret + * @param keyVersion non-sensitive key-rotation identifier + * @return tenant-domain pseudonymizer + */ + public static AuditPseudonymizer forAdministrativeTenant(String secret, String keyVersion) { + return new AuditPseudonymizer(secret, keyVersion, ADMIN_TENANT_DOMAIN); + } + /** * Creates a pseudonymizer with an explicit domain for isolated internal use * and domain-separation verification. diff --git a/src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java b/src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java index ec1d22cc..df3dd13e 100644 --- a/src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java +++ b/src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java @@ -102,6 +102,18 @@ public DefaultDocumentConversionService( ); } + /** + * Creates the conversion service with repository-backed lifecycle state and + * an isolated in-memory artifact store. + * + *

This convenience constructor is intended for tests and legacy wiring + * that do not provide lifecycle and artifact-store collaborators directly.

+ * + * @param repository conversion job repository + * @param validationService document validation service + * @param conversionWorker conversion worker + * @param conversionProperties conversion configuration values + */ public DefaultDocumentConversionService( ConversionJobRepository repository, DocumentValidationService validationService, @@ -111,11 +123,21 @@ public DefaultDocumentConversionService( repository, validationService, conversionWorker, - new com.clearfolio.viewer.artifact.InMemoryArtifactStore(), + new InMemoryArtifactStore(), conversionProperties ); } + /** + * Creates the conversion service with repository-backed lifecycle state and + * the supplied artifact store. + * + * @param repository conversion job repository + * @param validationService document validation service + * @param conversionWorker conversion worker + * @param artifactStore generated artifact store used for PDF passthrough seeding + * @param conversionProperties conversion configuration values + */ public DefaultDocumentConversionService( ConversionJobRepository repository, DocumentValidationService validationService, @@ -225,12 +247,46 @@ public void deleteJob(UUID jobId) { repository.deleteById(jobId); } + /** + * {@inheritDoc} + */ + @Override + public RetryDeadLetterResult retryDeadLettered( + UUID jobId, + TenantContext tenantContext, + String operatorId + ) { + if (tenantContext == null) { + return RetryDeadLetterResult.NOT_FOUND; + } + + Optional existing = repository.findByTenantAndId( + tenantContext.tenantId(), + jobId + ); + return retryExistingJob(existing, operatorId); + } + /** * {@inheritDoc} */ @Override public RetryDeadLetterResult retryDeadLettered(UUID jobId, String operatorId) { - Optional existing = repository.findById(jobId); + return retryExistingJob(repository.findById(jobId), operatorId); + } + + /** + * Applies the retry transition to a job that has already been selected by + * the caller's required authorization scope. + * + * @param existing selected conversion job, or empty when no authorized job exists + * @param operatorId privacy-safe operator fingerprint recorded by the state store + * @return accepted, not-found, or not-eligible retry result + */ + private RetryDeadLetterResult retryExistingJob( + Optional existing, + String operatorId + ) { if (existing.isEmpty()) { return RetryDeadLetterResult.NOT_FOUND; } diff --git a/src/main/java/com/clearfolio/viewer/service/DocumentConversionService.java b/src/main/java/com/clearfolio/viewer/service/DocumentConversionService.java index 6676f561..6ff0aad4 100644 --- a/src/main/java/com/clearfolio/viewer/service/DocumentConversionService.java +++ b/src/main/java/com/clearfolio/viewer/service/DocumentConversionService.java @@ -60,6 +60,36 @@ default UUID submit(MultipartFile file, PolicyOverrideRequest overrideRequest, T */ RetryDeadLetterResult retryDeadLettered(UUID jobId, String operatorId); + /** + * Retries a dead-lettered conversion job owned by the supplied tenant. + * + *

The default implementation preserves compatibility for adapters that + * have not yet implemented an atomic tenant-aware transition. Durable + * implementations should override this method so ownership and transition + * are enforced within one persistence boundary.

+ * + * @param jobId conversion job identifier + * @param tenantContext tenant and subject claims for the retry request + * @param operatorId privacy-safe operator fingerprint that triggered retry + * @return accepted, not-found, or not-eligible retry outcome + */ + default RetryDeadLetterResult retryDeadLettered( + UUID jobId, + TenantContext tenantContext, + String operatorId + ) { + if (tenantContext == null) { + return RetryDeadLetterResult.NOT_FOUND; + } + + Optional job = getJob(jobId); + if (job.isEmpty() || !job.get().belongsToTenant(tenantContext.tenantId())) { + return RetryDeadLetterResult.NOT_FOUND; + } + + return retryDeadLettered(jobId, operatorId); + } + /** * Deletes a conversion job owned by the supplied tenant context. * diff --git a/src/main/resources/application-buyer-demo.yml b/src/main/resources/application-buyer-demo.yml index 4e4df409..bdddfe5a 100644 --- a/src/main/resources/application-buyer-demo.yml +++ b/src/main/resources/application-buyer-demo.yml @@ -25,7 +25,9 @@ clearfolio: artifact-token: secret: ${CLEARFOLIO_ARTIFACT_TOKEN_SECRET:} tenant-claims: - hmac-secret: ${CLEARFOLIO_TENANT_CLAIMS_HMAC_SECRET:} + # `hmac-secret` is loaded from the config-tree secret mount configured in + # application.yml. Runtime authentication must not read secret material + # directly from an environment placeholder. max-skew-seconds: ${CLEARFOLIO_TENANT_CLAIMS_MAX_SKEW_SECONDS:300} artifact-link-ledger: path: ${CLEARFOLIO_ARTIFACT_LINK_LEDGER_PATH:} diff --git a/src/test/java/com/clearfolio/viewer/audit/AdministrativeAuditLoggerTest.java b/src/test/java/com/clearfolio/viewer/audit/AdministrativeAuditLoggerTest.java new file mode 100644 index 00000000..2e1fbd1e --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/audit/AdministrativeAuditLoggerTest.java @@ -0,0 +1,202 @@ +package com.clearfolio.viewer.audit; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.UUID; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.Logger; +import org.apache.logging.log4j.core.appender.AbstractAppender; +import org.apache.logging.log4j.core.layout.PatternLayout; +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; + +import com.clearfolio.viewer.audit.AdministrativeAuditLogger.Action; +import com.clearfolio.viewer.audit.AdministrativeAuditLogger.Outcome; +import com.clearfolio.viewer.auth.TenantContext; +import com.clearfolio.viewer.config.ConversionProperties; + +class AdministrativeAuditLoggerTest { + + private static final String AUDIT_SECRET = "0123456789abcdef0123456789abcdef"; + + @Test + void recordsAuthenticatedContextWithoutRawIdentifiers() { + AdministrativeAuditLogger auditLogger = configuredLogger(AUDIT_SECRET, "rotation-8"); + TenantContext context = new TenantContext( + "sensitive-tenant", + "employee-007@example.com", + Set.of("admin:read") + ); + CapturingAppender appender = attachAppender(); + + try { + auditLogger.record( + context, + Action.LIST_JOBS, + Outcome.ALLOWED, + HttpStatus.OK, + null, + 2 + ); + } finally { + appender.closeAndDetach(); + } + + String message = appender.singleMessage(); + assertTrue(message.contains("action=LIST_JOBS")); + assertTrue(message.contains("outcome=ALLOWED")); + assertTrue(message.contains("status=200")); + assertTrue(message.contains("jobId=none")); + assertTrue(message.contains("resultCount=2")); + assertTrue(message.matches(".*tenantFingerprint=rotation-8:[0-9a-f]{32}.*")); + assertTrue(message.matches(".*actorFingerprint=rotation-8:[0-9a-f]{32}.*")); + assertFalse(message.contains("sensitive-tenant")); + assertFalse(message.contains("employee-007@example.com")); + assertEquals("absent:rotation-8", auditLogger.actorFingerprint(null)); + assertNotEquals( + auditLogger.actorFingerprint(context), + message.substring( + message.indexOf("tenantFingerprint=") + "tenantFingerprint=".length(), + message.indexOf(" actorFingerprint=") + ) + ); + } + + @Test + void recordsUntrustedHeadersOnlyAsPseudonyms() { + AdministrativeAuditLogger auditLogger = configuredLogger(AUDIT_SECRET, "v2"); + HttpHeaders headers = new HttpHeaders(); + headers.set(TenantContext.TENANT_ID_HEADER, "tenant-from-untrusted-header"); + headers.set(TenantContext.SUBJECT_ID_HEADER, "subject-from-untrusted-header"); + UUID jobId = UUID.randomUUID(); + CapturingAppender appender = attachAppender(); + + try { + auditLogger.recordHeaders( + headers, + Action.DELETE_JOB, + Outcome.DENIED, + HttpStatus.FORBIDDEN, + jobId + ); + } finally { + appender.closeAndDetach(); + } + + String message = appender.singleMessage(); + assertTrue(message.contains("action=DELETE_JOB")); + assertTrue(message.contains("outcome=DENIED")); + assertTrue(message.contains("status=403")); + assertTrue(message.contains("jobId=" + jobId)); + assertTrue(message.contains("resultCount=-1")); + assertFalse(message.contains("tenant-from-untrusted-header")); + assertFalse(message.contains("subject-from-untrusted-header")); + } + + @Test + void usesExplicitAbsentAndUnavailableMarkers() { + AdministrativeAuditLogger unavailableLogger = configuredLogger("", "v9"); + HttpHeaders presentHeaders = new HttpHeaders(); + presentHeaders.set(TenantContext.TENANT_ID_HEADER, "tenant"); + presentHeaders.set(TenantContext.SUBJECT_ID_HEADER, "subject"); + CapturingAppender appender = attachAppender(); + + try { + unavailableLogger.recordHeaders( + presentHeaders, + Action.RETRY_JOB, + Outcome.DENIED, + HttpStatus.UNAUTHORIZED, + null + ); + unavailableLogger.recordHeaders( + null, + Action.RETRY_JOB, + Outcome.DENIED, + HttpStatus.UNAUTHORIZED, + null + ); + unavailableLogger.record( + null, + Action.RETRY_JOB, + Outcome.FAILED, + HttpStatus.INTERNAL_SERVER_ERROR, + null, + null + ); + } finally { + appender.closeAndDetach(); + } + + List messages = appender.messages(); + assertEquals(3, messages.size()); + assertTrue(messages.get(0).contains("tenantFingerprint=unavailable:v9")); + assertTrue(messages.get(0).contains("actorFingerprint=unavailable:v9")); + assertTrue(messages.get(1).contains("tenantFingerprint=absent:v9")); + assertTrue(messages.get(1).contains("actorFingerprint=absent:v9")); + assertTrue(messages.get(2).contains("tenantFingerprint=absent:v9")); + assertTrue(messages.get(2).contains("actorFingerprint=absent:v9")); + } + + private static AdministrativeAuditLogger configuredLogger(String secret, String version) { + ConversionProperties properties = new ConversionProperties(); + properties.setAuditPseudonymSecret(secret); + properties.setAuditPseudonymKeyVersion(version); + return new AdministrativeAuditLogger(properties); + } + + private static CapturingAppender attachAppender() { + Logger logger = (Logger) LogManager.getLogger(AdministrativeAuditLogger.class); + CapturingAppender appender = new CapturingAppender(logger); + appender.start(); + logger.addAppender(appender); + logger.setLevel(Level.INFO); + return appender; + } + + private static final class CapturingAppender extends AbstractAppender { + + private final Logger logger; + private final List messages = new ArrayList<>(); + + private CapturingAppender(Logger logger) { + super( + "administrative-audit-test-appender", + null, + PatternLayout.newBuilder().withPattern("%m").build(), + false, + null + ); + this.logger = logger; + } + + @Override + public void append(LogEvent event) { + messages.add(event.getMessage().getFormattedMessage()); + } + + private String singleMessage() { + assertEquals(1, messages.size()); + return messages.getFirst(); + } + + private List messages() { + return List.copyOf(messages); + } + + private void closeAndDetach() { + logger.removeAppender(this); + stop(); + } + } +} diff --git a/src/test/java/com/clearfolio/viewer/auth/TenantAccessServiceStrictClaimsTest.java b/src/test/java/com/clearfolio/viewer/auth/TenantAccessServiceStrictClaimsTest.java new file mode 100644 index 00000000..67ac3b0a --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/auth/TenantAccessServiceStrictClaimsTest.java @@ -0,0 +1,113 @@ +package com.clearfolio.viewer.auth; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.time.Clock; +import java.time.Instant; +import java.time.ZoneOffset; +import java.util.Set; + +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.web.server.ResponseStatusException; + +/** + * Defines the fail-closed signed-claim contract for privileged endpoints. + */ +class TenantAccessServiceStrictClaimsTest { + + private static final Instant NOW = Instant.parse("2026-08-05T00:00:00Z"); + private static final String STRONG_SECRET = "0123456789abcdef0123456789abcdef"; + + @Test + void requireSignedRejectsMissingVerifierSecret() { + TenantAccessService blankSecret = new TenantAccessService( + " ", + 300L, + Clock.fixed(NOW, ZoneOffset.UTC) + ); + TenantAccessService nullSecret = new TenantAccessService( + null, + 300L, + Clock.fixed(NOW, ZoneOffset.UTC) + ); + + assertEquals(HttpStatus.SERVICE_UNAVAILABLE, assertThrows( + ResponseStatusException.class, + () -> blankSecret.requireSigned( + unsignedHeaders(TenantPermissions.ADMIN_READ), + TenantPermissions.ADMIN_READ + ) + ).getStatusCode()); + assertEquals(HttpStatus.SERVICE_UNAVAILABLE, assertThrows( + ResponseStatusException.class, + () -> nullSecret.requireSigned( + unsignedHeaders(TenantPermissions.ADMIN_READ), + TenantPermissions.ADMIN_READ + ) + ).getStatusCode()); + } + + @Test + void requireSignedRejectsWeakConfiguredVerifierSecret() { + TenantAccessService weakSecret = new TenantAccessService( + "short-secret", + 300L, + Clock.fixed(NOW, ZoneOffset.UTC) + ); + + ResponseStatusException exception = assertThrows( + ResponseStatusException.class, + () -> weakSecret.requireSigned( + unsignedHeaders(TenantPermissions.ADMIN_READ), + TenantPermissions.ADMIN_READ + ) + ); + + assertEquals(HttpStatus.SERVICE_UNAVAILABLE, exception.getStatusCode()); + } + + @Test + void requireSignedAcceptsStrongFreshSignedClaims() { + TenantAccessService service = new TenantAccessService( + STRONG_SECRET, + 300L, + Clock.fixed(NOW, ZoneOffset.UTC) + ); + HttpHeaders headers = signedHeaders(TenantPermissions.ADMIN_READ); + + TenantContext context = assertDoesNotThrow( + () -> service.requireSigned(headers, TenantPermissions.ADMIN_READ) + ); + + assertEquals(TenantContext.DEMO_TENANT_ID, context.tenantId()); + assertEquals(TenantContext.DEMO_SUBJECT_ID, context.subjectId()); + } + + private static HttpHeaders unsignedHeaders(String permission) { + HttpHeaders headers = new HttpHeaders(); + headers.set(TenantContext.TENANT_ID_HEADER, TenantContext.DEMO_TENANT_ID); + headers.set(TenantContext.SUBJECT_ID_HEADER, TenantContext.DEMO_SUBJECT_ID); + headers.set(TenantContext.PERMISSIONS_HEADER, permission); + return headers; + } + + private static HttpHeaders signedHeaders(String permission) { + HttpHeaders headers = unsignedHeaders(permission); + String issuedAt = Long.toString(NOW.getEpochSecond()); + TenantContext context = new TenantContext( + TenantContext.DEMO_TENANT_ID, + TenantContext.DEMO_SUBJECT_ID, + Set.of(permission) + ); + headers.set(TenantContext.CLAIMS_ISSUED_AT_HEADER, issuedAt); + headers.set( + TenantContext.CLAIMS_SIGNATURE_HEADER, + TenantAccessService.signClaims(context, issuedAt, STRONG_SECRET) + ); + return headers; + } +} diff --git a/src/test/java/com/clearfolio/viewer/config/BuyerDemoSecretConfigurationTest.java b/src/test/java/com/clearfolio/viewer/config/BuyerDemoSecretConfigurationTest.java new file mode 100644 index 00000000..9af6ec44 --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/config/BuyerDemoSecretConfigurationTest.java @@ -0,0 +1,30 @@ +package com.clearfolio.viewer.config; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +import org.junit.jupiter.api.Test; +import org.springframework.core.io.ClassPathResource; + +class BuyerDemoSecretConfigurationTest { + + @Test + void tenantClaimsSecretComesFromTheSharedConfigTree() throws IOException { + String baseConfiguration = readResource("application.yml"); + String buyerDemoConfiguration = readResource("application-buyer-demo.yml"); + + assertTrue(baseConfiguration.contains( + "optional:configtree:${CLEARFOLIO_SECRET_CONFIG_DIR:/run/secrets/clearfolio/}" + )); + assertFalse(buyerDemoConfiguration.contains("CLEARFOLIO_TENANT_CLAIMS_HMAC_SECRET")); + assertFalse(buyerDemoConfiguration.contains("hmac-secret: ${")); + } + + private static String readResource(String name) throws IOException { + return new ClassPathResource(name) + .getContentAsString(StandardCharsets.UTF_8); + } +} diff --git a/src/test/java/com/clearfolio/viewer/controller/AdminControllerSignedClaimsRequirementTest.java b/src/test/java/com/clearfolio/viewer/controller/AdminControllerSignedClaimsRequirementTest.java new file mode 100644 index 00000000..61854084 --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/controller/AdminControllerSignedClaimsRequirementTest.java @@ -0,0 +1,53 @@ +package com.clearfolio.viewer.controller; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; + +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpHeaders; +import org.springframework.test.web.reactive.server.WebTestClient; + +import com.clearfolio.viewer.audit.AdministrativeAuditLogger; +import com.clearfolio.viewer.auth.TenantAccessService; +import com.clearfolio.viewer.auth.TenantContext; +import com.clearfolio.viewer.auth.TenantPermissions; +import com.clearfolio.viewer.config.ConversionProperties; +import com.clearfolio.viewer.service.DocumentConversionService; + +/** + * Proves privileged endpoints never fall back to unsigned demo-header mode. + */ +class AdminControllerSignedClaimsRequirementTest { + + private static final String AUDIT_SECRET = "0123456789abcdef0123456789abcdef"; + + @Test + void administrativeListIsUnavailableWithoutAConfiguredSignedClaimVerifier() { + DocumentConversionService conversionService = mock(DocumentConversionService.class); + ConversionProperties properties = new ConversionProperties(); + properties.setAuditPseudonymSecret(AUDIT_SECRET); + properties.setAuditPseudonymKeyVersion("admin-v1"); + AdminController controller = new AdminController( + conversionService, + new TenantAccessService("", 300L), + new AdministrativeAuditLogger(properties) + ); + WebTestClient client = WebTestClient.bindToController(controller) + .controllerAdvice(new ApiExceptionHandler()) + .build(); + HttpHeaders unsignedHeaders = new HttpHeaders(); + unsignedHeaders.set(TenantContext.TENANT_ID_HEADER, "tenant-north"); + unsignedHeaders.set(TenantContext.SUBJECT_ID_HEADER, "administrator@example.com"); + unsignedHeaders.set(TenantContext.PERMISSIONS_HEADER, TenantPermissions.ADMIN_READ); + + client.get() + .uri("/api/v1/admin/convert/jobs") + .headers(target -> target.addAll(unsignedHeaders)) + .exchange() + .expectStatus().isEqualTo(503) + .expectBody() + .jsonPath("$.errorCode").isEqualTo("SERVICE_UNAVAILABLE"); + + verifyNoInteractions(conversionService); + } +} diff --git a/src/test/java/com/clearfolio/viewer/controller/AdminControllerTenantMutationBoundaryTest.java b/src/test/java/com/clearfolio/viewer/controller/AdminControllerTenantMutationBoundaryTest.java new file mode 100644 index 00000000..bbd47537 --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/controller/AdminControllerTenantMutationBoundaryTest.java @@ -0,0 +1,127 @@ +package com.clearfolio.viewer.controller; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.time.Instant; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.UUID; + +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpHeaders; +import org.springframework.test.web.reactive.server.WebTestClient; + +import com.clearfolio.viewer.audit.AdministrativeAuditLogger; +import com.clearfolio.viewer.auth.TenantAccessService; +import com.clearfolio.viewer.auth.TenantContext; +import com.clearfolio.viewer.auth.TenantPermissions; +import com.clearfolio.viewer.config.ConversionProperties; +import com.clearfolio.viewer.security.AuditPseudonymizer; +import com.clearfolio.viewer.service.DocumentConversionService; +import com.clearfolio.viewer.service.RetryDeadLetterResult; + +/** + * Verifies that administrator mutations cross a tenant-scoped service boundary. + * + *

The controller must not authorize by reading an object and then invoke an + * unscoped mutation in a separate step. Tenant ownership is part of the service + * mutation contract so non-HTTP callers and future persistence adapters cannot + * bypass or race the controller-level check.

+ */ +class AdminControllerTenantMutationBoundaryTest { + + private static final String CLAIM_SECRET = "tenant-claims-" + "integration-secret"; + private static final String AUDIT_SECRET = "0123456789abcdef".repeat(2); + private static final String TENANT_ID = "tenant-north"; + private static final String SUBJECT_ID = "administrator@example.com"; + + @Test + void deleteUsesTheTenantScopedServiceMutationWithoutASeparateLookup() { + DocumentConversionService conversionService = mock(DocumentConversionService.class); + UUID jobId = UUID.randomUUID(); + when(conversionService.deleteJob(any(UUID.class), any(TenantContext.class))) + .thenReturn(true); + + WebTestClient client = client(conversionService); + + client.delete() + .uri("/api/v1/admin/convert/jobs/{jobId}", jobId) + .headers(target -> target.addAll(signedHeaders(TenantPermissions.ADMIN_WRITE))) + .exchange() + .expectStatus().isNoContent(); + + verify(conversionService).deleteJob( + eq(jobId), + argThat(context -> TENANT_ID.equals(context.tenantId())) + ); + verify(conversionService, never()).getJob(jobId); + verify(conversionService, never()).deleteJob(jobId); + } + + @Test + void retryUsesTheTenantScopedServiceMutationWithoutASeparateLookup() { + DocumentConversionService conversionService = mock(DocumentConversionService.class); + UUID jobId = UUID.randomUUID(); + String actorFingerprint = AuditPseudonymizer.forAdministrativeActor( + AUDIT_SECRET, + "admin-v1" + ).fingerprint(SUBJECT_ID); + when(conversionService.retryDeadLettered( + any(UUID.class), + any(TenantContext.class), + any(String.class) + )).thenReturn(RetryDeadLetterResult.ACCEPTED); + + WebTestClient client = client(conversionService); + + client.post() + .uri("/api/v1/admin/convert/jobs/{jobId}/retry", jobId) + .headers(target -> target.addAll(signedHeaders(TenantPermissions.ADMIN_WRITE))) + .exchange() + .expectStatus().isAccepted(); + + verify(conversionService).retryDeadLettered( + eq(jobId), + argThat(context -> TENANT_ID.equals(context.tenantId())), + eq(actorFingerprint) + ); + verify(conversionService, never()).getJob(jobId); + verify(conversionService, never()).retryDeadLettered(any(UUID.class), any(String.class)); + } + + private static WebTestClient client(DocumentConversionService conversionService) { + ConversionProperties properties = new ConversionProperties(); + properties.setAuditPseudonymSecret(AUDIT_SECRET); + properties.setAuditPseudonymKeyVersion("admin-v1"); + AdminController controller = new AdminController( + conversionService, + new TenantAccessService(CLAIM_SECRET, 300L), + new AdministrativeAuditLogger(properties) + ); + return WebTestClient.bindToController(controller) + .controllerAdvice(new ApiExceptionHandler()) + .build(); + } + + private static HttpHeaders signedHeaders(String... permissions) { + LinkedHashSet permissionSet = new LinkedHashSet<>(List.of(permissions)); + TenantContext context = new TenantContext(TENANT_ID, SUBJECT_ID, permissionSet); + String issuedAt = Long.toString(Instant.now().getEpochSecond()); + HttpHeaders headers = new HttpHeaders(); + headers.set(TenantContext.TENANT_ID_HEADER, TENANT_ID); + headers.set(TenantContext.SUBJECT_ID_HEADER, SUBJECT_ID); + headers.set(TenantContext.PERMISSIONS_HEADER, String.join(",", permissions)); + headers.set(TenantContext.CLAIMS_ISSUED_AT_HEADER, issuedAt); + headers.set( + TenantContext.CLAIMS_SIGNATURE_HEADER, + TenantAccessService.signClaims(context, issuedAt, CLAIM_SECRET) + ); + return headers; + } +} diff --git a/src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java b/src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java index ad63a801..5cd95f2b 100644 --- a/src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java +++ b/src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java @@ -1,124 +1,409 @@ package com.clearfolio.viewer.controller; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; -import java.util.Arrays; +import java.time.Instant; +import java.util.LinkedHashSet; +import java.util.List; import java.util.UUID; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.http.HttpHeaders; import org.springframework.test.web.reactive.server.WebTestClient; +import com.clearfolio.viewer.audit.AdministrativeAuditLogger; +import com.clearfolio.viewer.auth.TenantAccessService; +import com.clearfolio.viewer.auth.TenantContext; +import com.clearfolio.viewer.auth.TenantPermissions; +import com.clearfolio.viewer.config.ConversionProperties; import com.clearfolio.viewer.model.ConversionJob; +import com.clearfolio.viewer.security.AuditPseudonymizer; import com.clearfolio.viewer.service.DocumentConversionService; import com.clearfolio.viewer.service.RetryDeadLetterResult; class AdminControllerTest { + private static final String CLAIM_SECRET = "tenant-claims-" + "integration-secret"; + private static final String AUDIT_SECRET = "0123456789abcdef".repeat(2); + private static final String TENANT_ID = "tenant-north"; + private static final String SUBJECT_ID = "administrator@example.com"; + private DocumentConversionService conversionService; private WebTestClient webTestClient; - private AdminController controller; @BeforeEach void setUp() { conversionService = mock(DocumentConversionService.class); - controller = new AdminController(conversionService); + ConversionProperties properties = new ConversionProperties(); + properties.setAuditPseudonymSecret(AUDIT_SECRET); + properties.setAuditPseudonymKeyVersion("admin-v1"); + AdminController controller = new AdminController( + conversionService, + new TenantAccessService(CLAIM_SECRET, 300L), + new AdministrativeAuditLogger(properties) + ); webTestClient = WebTestClient.bindToController(controller) .controllerAdvice(new ApiExceptionHandler()) .build(); } @Test - void getAllJobsReturnsAllJobsWhenNoFilterProvided() { - ConversionJob job1 = new ConversionJob(UUID.randomUUID(), "a.pdf", "application/pdf", "hash-a", 100L); - ConversionJob job2 = new ConversionJob(UUID.randomUUID(), "b.pdf", "application/pdf", "hash-b", 100L); - when(conversionService.getAllJobs()).thenReturn(Arrays.asList(job1, job2)); - + void missingClaimsAreDeniedBeforeServiceAccess() { webTestClient.get() .uri("/api/v1/admin/convert/jobs") .exchange() - .expectStatus().isOk() - .expectBody() - .jsonPath("$.jobs.length()").isEqualTo(2) - .jsonPath("$.jobs[0].fileName").isEqualTo("a.pdf") - .jsonPath("$.jobs[1].fileName").isEqualTo("b.pdf"); + .expectStatus().isUnauthorized(); + + verifyNoInteractions(conversionService); } @Test - void getAllJobsFiltersByDeadLetteredTrue() { - ConversionJob job1 = new ConversionJob(UUID.randomUUID(), "a.pdf", "application/pdf", "hash-a", 100L); - job1.markDeadLettered("failed"); - ConversionJob job2 = new ConversionJob(UUID.randomUUID(), "b.pdf", "application/pdf", "hash-b", 100L); + void malformedExpiredAndInvalidSignedClaimsAreDenied() { + HttpHeaders malformed = unsignedClaimHeaders( + TENANT_ID, + SUBJECT_ID, + TenantPermissions.ADMIN_READ + ); + malformed.set(TenantContext.CLAIMS_ISSUED_AT_HEADER, "not-an-epoch"); + malformed.set(TenantContext.CLAIMS_SIGNATURE_HEADER, "ignored"); - when(conversionService.getAllJobs()).thenReturn(Arrays.asList(job1, job2)); + requestJobs(malformed).expectStatus().isUnauthorized(); + requestJobs(signedHeadersAt( + TENANT_ID, + SUBJECT_ID, + Instant.now().minusSeconds(1_000L).getEpochSecond(), + TenantPermissions.ADMIN_READ + )).expectStatus().isUnauthorized(); - webTestClient.get() - .uri("/api/v1/admin/convert/jobs?deadLettered=true") - .exchange() - .expectStatus().isOk() - .expectBody() - .jsonPath("$.jobs.length()").isEqualTo(1) - .jsonPath("$.jobs[0].fileName").isEqualTo("a.pdf"); + HttpHeaders invalidSignature = signedHeaders( + TENANT_ID, + SUBJECT_ID, + TenantPermissions.ADMIN_READ + ); + invalidSignature.set(TenantContext.CLAIMS_SIGNATURE_HEADER, "invalid-signature"); + requestJobs(invalidSignature).expectStatus().isUnauthorized(); + + verifyNoInteractions(conversionService); } @Test - void getAllJobsFiltersByDeadLetteredFalse() { - ConversionJob job1 = new ConversionJob(UUID.randomUUID(), "a.pdf", "application/pdf", "hash-a", 100L); - job1.markDeadLettered("failed"); - ConversionJob job2 = new ConversionJob(UUID.randomUUID(), "b.pdf", "application/pdf", "hash-b", 100L); + void missingReadPermissionIsDeniedBeforeServiceAccess() { + requestJobs(signedHeaders(TENANT_ID, SUBJECT_ID, TenantPermissions.JOB_READ)) + .expectStatus().isForbidden(); - when(conversionService.getAllJobs()).thenReturn(Arrays.asList(job1, job2)); + verifyNoInteractions(conversionService); + } - webTestClient.get() - .uri("/api/v1/admin/convert/jobs?deadLettered=false") - .exchange() + @Test + void listReturnsOnlyTenantOwnedJobsAndAppliesDeadLetterFilter() { + ConversionJob deadLettered = job(TENANT_ID, "dead.pdf", true); + ConversionJob ready = job(TENANT_ID, "ready.pdf", false); + ConversionJob otherTenant = job("tenant-south", "secret.pdf", false); + when(conversionService.getAllJobs()).thenReturn(List.of(deadLettered, ready, otherTenant)); + HttpHeaders headers = signedHeaders(TENANT_ID, SUBJECT_ID, TenantPermissions.ADMIN_READ); + + requestJobs(headers) + .expectStatus().isOk() + .expectBody() + .jsonPath("$.jobs.length()").isEqualTo(2) + .jsonPath("$.jobs[0].fileName").isEqualTo("dead.pdf") + .jsonPath("$.jobs[1].fileName").isEqualTo("ready.pdf"); + + requestJobs(headers, "?deadLettered=true") .expectStatus().isOk() .expectBody() .jsonPath("$.jobs.length()").isEqualTo(1) - .jsonPath("$.jobs[0].fileName").isEqualTo("b.pdf"); + .jsonPath("$.jobs[0].fileName").isEqualTo("dead.pdf"); + + requestJobs(headers, "?deadLettered=false") + .expectStatus().isOk() + .expectBody() + .jsonPath("$.jobs.length()").isEqualTo(1) + .jsonPath("$.jobs[0].fileName").isEqualTo("ready.pdf"); + + verify(conversionService, times(3)).getAllJobs(); + } + + @Test + void listServiceFailureReturnsGenericInternalError() { + when(conversionService.getAllJobs()).thenThrow(new IllegalStateException("repository unavailable")); + + requestJobs(signedHeaders(TENANT_ID, SUBJECT_ID, TenantPermissions.ADMIN_READ)) + .expectStatus().is5xxServerError() + .expectBody() + .jsonPath("$.errorCode").isEqualTo("INTERNAL_ERROR") + .jsonPath("$.message").isEqualTo("Unexpected error"); } @Test - void deleteJobReturnsNoContent() { + void readOnlyAdministratorCannotDelete() { UUID jobId = UUID.randomUUID(); webTestClient.delete() .uri("/api/v1/admin/convert/jobs/" + jobId) + .headers(target -> target.addAll(signedHeaders( + TENANT_ID, + SUBJECT_ID, + TenantPermissions.ADMIN_READ + ))) .exchange() - .expectStatus().isNoContent(); + .expectStatus().isForbidden(); + + verifyNoInteractions(conversionService); + } + + @Test + void deleteConcealsMissingAndCrossTenantServiceOutcomes() { + UUID missingId = UUID.randomUUID(); + UUID crossTenantId = UUID.randomUUID(); + when(conversionService.deleteJob(eq(missingId), any(TenantContext.class))) + .thenReturn(false); + when(conversionService.deleteJob(eq(crossTenantId), any(TenantContext.class))) + .thenReturn(false); + HttpHeaders headers = signedHeaders(TENANT_ID, SUBJECT_ID, TenantPermissions.ADMIN_WRITE); + + deleteJob(missingId, headers).expectStatus().isNotFound(); + deleteJob(crossTenantId, headers).expectStatus().isNotFound(); + + verify(conversionService).deleteJob( + eq(missingId), + argThat(context -> TENANT_ID.equals(context.tenantId())) + ); + verify(conversionService).deleteJob( + eq(crossTenantId), + argThat(context -> TENANT_ID.equals(context.tenantId())) + ); + verify(conversionService, never()).getJob(any(UUID.class)); + verify(conversionService, never()).deleteJob(any(UUID.class)); } @Test - void retryDeadLetteredReturnsAcceptedWhenAccepted() { + void deleteUsesTenantScopedServiceAndReportsFailuresGenerically() { + UUID successId = UUID.randomUUID(); + UUID lookupFailureId = UUID.randomUUID(); + UUID deleteFailureId = UUID.randomUUID(); + when(conversionService.deleteJob(eq(successId), any(TenantContext.class))) + .thenReturn(true); + when(conversionService.deleteJob(eq(lookupFailureId), any(TenantContext.class))) + .thenThrow(new IllegalStateException("lookup failed")); + when(conversionService.deleteJob(eq(deleteFailureId), any(TenantContext.class))) + .thenThrow(new IllegalStateException("delete failed")); + HttpHeaders headers = signedHeaders(TENANT_ID, SUBJECT_ID, TenantPermissions.ADMIN_WRITE); + + deleteJob(successId, headers).expectStatus().isNoContent(); + deleteJob(lookupFailureId, headers).expectStatus().is5xxServerError(); + deleteJob(deleteFailureId, headers).expectStatus().is5xxServerError(); + + verify(conversionService).deleteJob( + eq(successId), + argThat(context -> TENANT_ID.equals(context.tenantId())) + ); + verify(conversionService).deleteJob( + eq(lookupFailureId), + argThat(context -> TENANT_ID.equals(context.tenantId())) + ); + verify(conversionService).deleteJob( + eq(deleteFailureId), + argThat(context -> TENANT_ID.equals(context.tenantId())) + ); + verify(conversionService, never()).getJob(any(UUID.class)); + verify(conversionService, never()).deleteJob(any(UUID.class)); + } + + @Test + void retryAcceptedUsesDomainSeparatedActorFingerprint() { UUID jobId = UUID.randomUUID(); - when(conversionService.retryDeadLettered(jobId, "admin")).thenReturn(RetryDeadLetterResult.ACCEPTED); + String expectedActor = AuditPseudonymizer.forAdministrativeActor( + AUDIT_SECRET, + "admin-v1" + ).fingerprint(SUBJECT_ID); + when(conversionService.retryDeadLettered( + eq(jobId), + any(TenantContext.class), + eq(expectedActor) + )).thenReturn(RetryDeadLetterResult.ACCEPTED); - webTestClient.post() - .uri("/api/v1/admin/convert/jobs/" + jobId + "/retry") - .exchange() + retryJob(jobId, signedHeaders(TENANT_ID, SUBJECT_ID, TenantPermissions.ADMIN_WRITE)) .expectStatus().isAccepted(); + + verify(conversionService).retryDeadLettered( + eq(jobId), + argThat(context -> TENANT_ID.equals(context.tenantId())), + eq(expectedActor) + ); + verify(conversionService, never()).getJob(any(UUID.class)); + verify(conversionService, never()).retryDeadLettered(any(UUID.class), any(String.class)); + verify(conversionService, never()).retryDeadLettered(jobId, SUBJECT_ID); } @Test - void retryDeadLetteredReturnsNotFoundWhenNotFound() { - UUID jobId = UUID.randomUUID(); - when(conversionService.retryDeadLettered(jobId, "admin")).thenReturn(RetryDeadLetterResult.NOT_FOUND); + void retryConcealsMissingAndCrossTenantServiceOutcomes() { + UUID missingId = UUID.randomUUID(); + UUID crossTenantId = UUID.randomUUID(); + when(conversionService.retryDeadLettered( + eq(missingId), + any(TenantContext.class), + any(String.class) + )).thenReturn(RetryDeadLetterResult.NOT_FOUND); + when(conversionService.retryDeadLettered( + eq(crossTenantId), + any(TenantContext.class), + any(String.class) + )).thenReturn(RetryDeadLetterResult.NOT_FOUND); + HttpHeaders headers = signedHeaders(TENANT_ID, SUBJECT_ID, TenantPermissions.ADMIN_WRITE); - webTestClient.post() - .uri("/api/v1/admin/convert/jobs/" + jobId + "/retry") - .exchange() - .expectStatus().isNotFound(); + retryJob(missingId, headers).expectStatus().isNotFound(); + retryJob(crossTenantId, headers).expectStatus().isNotFound(); + + verify(conversionService).retryDeadLettered( + eq(missingId), + argThat(context -> TENANT_ID.equals(context.tenantId())), + any(String.class) + ); + verify(conversionService).retryDeadLettered( + eq(crossTenantId), + argThat(context -> TENANT_ID.equals(context.tenantId())), + any(String.class) + ); + verify(conversionService, never()).getJob(any(UUID.class)); + verify(conversionService, never()).retryDeadLettered(any(UUID.class), any(String.class)); } @Test - void retryDeadLetteredReturnsConflictWhenNotEligible() { + void retryMapsTenantScopedServiceOutcomesWithoutLeakingJobState() { + UUID disappearedId = UUID.randomUUID(); + UUID ineligibleId = UUID.randomUUID(); + when(conversionService.retryDeadLettered( + eq(disappearedId), + any(TenantContext.class), + any(String.class) + )).thenReturn(RetryDeadLetterResult.NOT_FOUND); + when(conversionService.retryDeadLettered( + eq(ineligibleId), + any(TenantContext.class), + any(String.class) + )).thenReturn(RetryDeadLetterResult.NOT_ELIGIBLE); + HttpHeaders headers = signedHeaders(TENANT_ID, SUBJECT_ID, TenantPermissions.ADMIN_WRITE); + + retryJob(disappearedId, headers).expectStatus().isNotFound(); + retryJob(ineligibleId, headers).expectStatus().isEqualTo(409); + } + + @Test + void retryServiceFailureReturnsGenericInternalError() { UUID jobId = UUID.randomUUID(); - when(conversionService.retryDeadLettered(jobId, "admin")).thenReturn(RetryDeadLetterResult.NOT_ELIGIBLE); + when(conversionService.retryDeadLettered( + eq(jobId), + any(TenantContext.class), + any(String.class) + )).thenThrow(new IllegalStateException("queue unavailable")); + + retryJob(jobId, signedHeaders(TENANT_ID, SUBJECT_ID, TenantPermissions.ADMIN_WRITE)) + .expectStatus().is5xxServerError() + .expectBody() + .jsonPath("$.errorCode").isEqualTo("INTERNAL_ERROR"); + } - webTestClient.post() + private WebTestClient.ResponseSpec requestJobs(HttpHeaders headers) { + return requestJobs(headers, ""); + } + + private WebTestClient.ResponseSpec requestJobs(HttpHeaders headers, String query) { + return webTestClient.get() + .uri("/api/v1/admin/convert/jobs" + query) + .headers(target -> target.addAll(headers)) + .exchange(); + } + + private WebTestClient.ResponseSpec deleteJob(UUID jobId, HttpHeaders headers) { + return webTestClient.delete() + .uri("/api/v1/admin/convert/jobs/" + jobId) + .headers(target -> target.addAll(headers)) + .exchange(); + } + + private WebTestClient.ResponseSpec retryJob(UUID jobId, HttpHeaders headers) { + return webTestClient.post() .uri("/api/v1/admin/convert/jobs/" + jobId + "/retry") - .exchange() - .expectStatus().isEqualTo(409); // isConflict() isn't always available depending on spring-test version, so using isEqualTo(409) is safer + .headers(target -> target.addAll(headers)) + .exchange(); + } + + private static HttpHeaders signedHeaders( + String tenantId, + String subjectId, + String... permissions + ) { + return signedHeadersAt( + tenantId, + subjectId, + Instant.now().getEpochSecond(), + permissions + ); + } + + private static HttpHeaders signedHeadersAt( + String tenantId, + String subjectId, + long issuedAtEpoch, + String... permissions + ) { + LinkedHashSet permissionSet = new LinkedHashSet<>(List.of(permissions)); + TenantContext context = new TenantContext(tenantId, subjectId, permissionSet); + String issuedAt = Long.toString(issuedAtEpoch); + HttpHeaders headers = unsignedClaimHeaders(tenantId, subjectId, permissions); + headers.set(TenantContext.CLAIMS_ISSUED_AT_HEADER, issuedAt); + headers.set( + TenantContext.CLAIMS_SIGNATURE_HEADER, + TenantAccessService.signClaims(context, issuedAt, CLAIM_SECRET) + ); + return headers; + } + + private static HttpHeaders unsignedClaimHeaders( + String tenantId, + String subjectId, + String... permissions + ) { + HttpHeaders headers = new HttpHeaders(); + headers.set(TenantContext.TENANT_ID_HEADER, tenantId); + headers.set(TenantContext.SUBJECT_ID_HEADER, subjectId); + headers.set(TenantContext.PERMISSIONS_HEADER, String.join(",", permissions)); + return headers; + } + + private static ConversionJob job(String tenantId, String fileName, boolean deadLettered) { + return job(tenantId, fileName, deadLettered, UUID.randomUUID()); + } + + private static ConversionJob job( + String tenantId, + String fileName, + boolean deadLettered, + UUID jobId + ) { + ConversionJob job = new ConversionJob( + jobId, + tenantId, + "owner", + fileName, + "application/pdf", + "hash", + 100L, + 3 + ); + if (deadLettered) { + job.markDeadLettered("failed"); + } + return job; } } diff --git a/src/test/java/com/clearfolio/viewer/security/AuditPseudonymizerTest.java b/src/test/java/com/clearfolio/viewer/security/AuditPseudonymizerTest.java index ce9b01b7..b50d05a9 100644 --- a/src/test/java/com/clearfolio/viewer/security/AuditPseudonymizerTest.java +++ b/src/test/java/com/clearfolio/viewer/security/AuditPseudonymizerTest.java @@ -113,6 +113,10 @@ void defaultsOnlyMissingKeyVersionAndRejectsInvalidExplicitValues() { IllegalArgumentException.class, () -> new AuditPseudonymizer(AUDIT_KEY_ONE, "v1 ") ); + assertThrows( + IllegalArgumentException.class, + () -> new AuditPseudonymizer(AUDIT_KEY_ONE, " v1 ") + ); assertThrows( IllegalArgumentException.class, () -> new AuditPseudonymizer(AUDIT_KEY_ONE, "bad/version") diff --git a/src/test/java/com/clearfolio/viewer/service/TenantScopedRetryContractTest.java b/src/test/java/com/clearfolio/viewer/service/TenantScopedRetryContractTest.java new file mode 100644 index 00000000..f6f165d4 --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/service/TenantScopedRetryContractTest.java @@ -0,0 +1,197 @@ +package com.clearfolio.viewer.service; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Optional; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.jupiter.api.Test; +import org.springframework.web.multipart.MultipartFile; + +import com.clearfolio.viewer.artifact.InMemoryArtifactStore; +import com.clearfolio.viewer.auth.TenantContext; +import com.clearfolio.viewer.config.ConversionProperties; +import com.clearfolio.viewer.model.ConversionJob; +import com.clearfolio.viewer.model.ConversionJobStatus; +import com.clearfolio.viewer.repository.InMemoryConversionJobRepository; + +/** + * Verifies the compatibility and durable-service contracts for tenant-scoped + * dead-letter retry operations. + */ +class TenantScopedRetryContractTest { + + @Test + void interfaceDefaultRejectsUnownedJobsBeforeLegacyMutation() { + UUID jobId = UUID.randomUUID(); + ConversionJob job = job(jobId, "tenant-a", "default-contract"); + AtomicReference retriedJobId = new AtomicReference<>(); + AtomicReference retriedOperatorId = new AtomicReference<>(); + DocumentConversionService service = new DocumentConversionService() { + @Override + public UUID submit(MultipartFile file) { + return UUID.randomUUID(); + } + + @Override + public Optional getJob(UUID requestedJobId) { + return jobId.equals(requestedJobId) ? Optional.of(job) : Optional.empty(); + } + + @Override + public RetryDeadLetterResult retryDeadLettered(UUID requestedJobId, String operatorId) { + retriedJobId.set(requestedJobId); + retriedOperatorId.set(operatorId); + return RetryDeadLetterResult.ACCEPTED; + } + + @Override + public void deleteJob(UUID requestedJobId) { + } + + @Override + public Iterable getAllJobs() { + return java.util.List.of(job); + } + }; + TenantContext tenantA = new TenantContext("tenant-a", "subject-a", Set.of()); + TenantContext tenantB = new TenantContext("tenant-b", "subject-b", Set.of()); + + assertEquals( + RetryDeadLetterResult.NOT_FOUND, + service.retryDeadLettered(jobId, null, "operator-null") + ); + assertEquals( + RetryDeadLetterResult.NOT_FOUND, + service.retryDeadLettered(UUID.randomUUID(), tenantA, "operator-missing") + ); + assertEquals( + RetryDeadLetterResult.NOT_FOUND, + service.retryDeadLettered(jobId, tenantB, "operator-cross-tenant") + ); + assertEquals( + RetryDeadLetterResult.ACCEPTED, + service.retryDeadLettered(jobId, tenantA, "operator-owned") + ); + assertEquals(jobId, retriedJobId.get()); + assertEquals("operator-owned", retriedOperatorId.get()); + } + + @Test + void durableServiceRejectsNullMissingAndCrossTenantJobsWithoutMutation() { + InMemoryConversionJobRepository repository = new InMemoryConversionJobRepository(); + RecordingConversionWorker worker = new RecordingConversionWorker(); + DocumentConversionService service = service(repository, worker); + ConversionJob job = deadLetteredJob(UUID.randomUUID(), "tenant-a", "durable-reject"); + repository.save(job); + + assertEquals( + RetryDeadLetterResult.NOT_FOUND, + service.retryDeadLettered(job.getJobId(), null, "operator-null") + ); + assertEquals( + RetryDeadLetterResult.NOT_FOUND, + service.retryDeadLettered( + UUID.randomUUID(), + new TenantContext("tenant-a", "subject-a", Set.of()), + "operator-missing" + ) + ); + assertEquals( + RetryDeadLetterResult.NOT_FOUND, + service.retryDeadLettered( + job.getJobId(), + new TenantContext("tenant-b", "subject-b", Set.of()), + "operator-cross-tenant" + ) + ); + assertEquals(ConversionJobStatus.FAILED, job.getStatus()); + assertTrue(job.isDeadLettered()); + assertEquals(0, worker.enqueuedCount()); + } + + @Test + void durableServiceMapsOwnedEligibilityAndAcceptsOwnedDeadLetteredJob() { + InMemoryConversionJobRepository repository = new InMemoryConversionJobRepository(); + RecordingConversionWorker worker = new RecordingConversionWorker(); + DocumentConversionService service = service(repository, worker); + TenantContext tenant = new TenantContext("tenant-a", "subject-a", Set.of()); + ConversionJob active = job(UUID.randomUUID(), "tenant-a", "durable-active"); + ConversionJob deadLettered = deadLetteredJob( + UUID.randomUUID(), + "tenant-a", + "durable-accepted" + ); + repository.save(active); + repository.save(deadLettered); + + assertEquals( + RetryDeadLetterResult.NOT_ELIGIBLE, + service.retryDeadLettered(active.getJobId(), tenant, "operator-active") + ); + assertEquals( + RetryDeadLetterResult.ACCEPTED, + service.retryDeadLettered(deadLettered.getJobId(), tenant, "operator-owned") + ); + assertEquals(ConversionJobStatus.SUBMITTED, deadLettered.getStatus()); + assertTrue(deadLettered.getStatusMessage().contains("operator-owned")); + assertEquals(1, worker.enqueuedCount()); + assertEquals(deadLettered.getJobId(), worker.lastEnqueuedJobId()); + } + + private static DocumentConversionService service( + InMemoryConversionJobRepository repository, + RecordingConversionWorker worker + ) { + return new DefaultDocumentConversionService( + repository, + new DefaultDocumentValidationService(new ConversionProperties()), + worker, + new InMemoryArtifactStore(), + new ConversionProperties() + ); + } + + private static ConversionJob deadLetteredJob(UUID jobId, String tenantId, String hash) { + ConversionJob job = job(jobId, tenantId, hash); + assertTrue(job.markProcessing("first attempt")); + job.markDeadLettered("retries exhausted"); + return job; + } + + private static ConversionJob job(UUID jobId, String tenantId, String hash) { + return new ConversionJob( + jobId, + tenantId, + "subject-a", + "contract.docx", + "application/octet-stream", + hash, + 1L, + 3 + ); + } + + private static final class RecordingConversionWorker implements ConversionWorker { + private final AtomicInteger count = new AtomicInteger(); + private final AtomicReference lastJobId = new AtomicReference<>(); + + @Override + public void enqueue(UUID jobId) { + lastJobId.set(jobId); + count.incrementAndGet(); + } + + int enqueuedCount() { + return count.get(); + } + + UUID lastEnqueuedJobId() { + return lastJobId.get(); + } + } +}