mirror of
https://github.com/arsvendg/Stirling-PDF.git
synced 2026-07-15 19:10:47 +02:00
Add MCP server with OAuth/API-key auth (#6570)
Adds an optional MCP server (proprietary module) that exposes Stirling's PDF operations and AI capabilities to MCP clients. Off by default, zero footprint when disabled. ### What - New `/mcp` endpoint: streamable-HTTP + JSON-RPC 2.0; 8 tools (describe_operation, pages/convert/misc/security category tools, AI, upload, download). - Runs real operations over an internal loopback; results returned inline as base64 (small) or by fileId (large). ### Auth (two modes) - OAuth2 resource server: RFC 9728 protected-resource metadata, RFC 8707 audience binding, JWKS, `mcp.tools.read/write` scopes; binds each token to a provisioned Stirling account. - API-key mode: reuses Stirling per-user `X-API-KEY` (no IdP needed). ### Security - Per-user file ownership in FileStorage: async/queued writes scoped to the submitting user; legacy/owner-less files stay readable. - Admin allow/block list controls which operations are exposed. - Python engine gated behind a shared secret (`X-Engine-Auth`). - MCP filter chain is isolated and cannot weaken the main app's security. - Hardened: no upstream error-body leakage, log injection sanitized, fileId path/sidecar enumeration blocked. ### Config / footprint - Disabled by default (`mcp.enabled=false`); all beans `@ConditionalOnProperty`. --- ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [ ] I have performed a self-review of my own code - [ ] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have run `task check` to verify linters, typechecks, and tests pass - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing) for more details.
This commit is contained in:
@@ -11,23 +11,39 @@ public interface FileStore {
|
||||
/** Stored file record. */
|
||||
record Stored(String fileId, long size) {}
|
||||
|
||||
/** Store the given stream and return a generated file id and total bytes written. */
|
||||
Stored store(InputStream in, String originalName) throws IOException;
|
||||
/**
|
||||
* Store the given stream and return a generated file id and total bytes written. {@code owner}
|
||||
* may be null to indicate the file has no associated user (anonymous / desktop / async job with
|
||||
* no propagated security context); a non-null value is persisted alongside the data so {@link
|
||||
* #getOwner(String)} can return it later for authorization checks.
|
||||
*/
|
||||
Stored store(InputStream in, String originalName, String owner) throws IOException;
|
||||
|
||||
/** Store with no owner. Equivalent to {@link #store(InputStream, String, String)} with null. */
|
||||
default Stored store(InputStream in, String originalName) throws IOException {
|
||||
return store(in, originalName, null);
|
||||
}
|
||||
|
||||
/**
|
||||
* Store the file at {@code source} and return a generated file id and total bytes written.
|
||||
*
|
||||
* <p>Default implementation opens {@code source} as a stream and delegates to {@link
|
||||
* #store(InputStream, String)}. Local-disk implementations should override to use a direct
|
||||
* file-to-file copy ({@code Files.copy(source, dest)} can use {@code sendfile(2)} on Linux),
|
||||
* which avoids the two-memory-copy hit of streaming a disk-backed upload through the JVM heap.
|
||||
* #store(InputStream, String, String)}. Local-disk implementations should override to use a
|
||||
* direct file-to-file copy ({@code Files.copy(source, dest)} can use {@code sendfile(2)} on
|
||||
* Linux), which avoids the two-memory-copy hit of streaming a disk-backed upload through the
|
||||
* JVM heap.
|
||||
*/
|
||||
default Stored store(Path source, String originalName) throws IOException {
|
||||
default Stored store(Path source, String originalName, String owner) throws IOException {
|
||||
try (InputStream in = Files.newInputStream(source)) {
|
||||
return store(in, originalName);
|
||||
return store(in, originalName, owner);
|
||||
}
|
||||
}
|
||||
|
||||
/** Store with no owner. Equivalent to {@link #store(Path, String, String)} with null. */
|
||||
default Stored store(Path source, String originalName) throws IOException {
|
||||
return store(source, originalName, null);
|
||||
}
|
||||
|
||||
/** Open the stored file for streaming reads. Caller closes. */
|
||||
InputStream retrieve(String fileId) throws IOException;
|
||||
|
||||
@@ -42,4 +58,12 @@ public interface FileStore {
|
||||
|
||||
/** Whether the file id exists in the store. */
|
||||
boolean exists(String fileId);
|
||||
|
||||
/**
|
||||
* Returns the owner identifier recorded at store time, or {@code null} if the file does not
|
||||
* exist or was stored without an owner. Implementations must not throw when the file is missing
|
||||
* or when the owner record is absent; they should return null so callers can treat "no owner"
|
||||
* as a non-authoritative case.
|
||||
*/
|
||||
String getOwner(String fileId) throws IOException;
|
||||
}
|
||||
|
||||
+100
-23
@@ -3,9 +3,12 @@ package stirling.software.common.cluster.inprocess;
|
||||
import java.io.BufferedInputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.UUID;
|
||||
import java.util.concurrent.locks.ReentrantLock;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
|
||||
@@ -15,33 +18,47 @@ import stirling.software.common.cluster.FileStore;
|
||||
@Slf4j
|
||||
public class LocalDiskFileStore implements FileStore {
|
||||
|
||||
private static final String OWNER_SUFFIX = ".owner";
|
||||
|
||||
// File ids are generated as random UUIDs; reject anything else so a tainted id can never reach
|
||||
// Files.* APIs (defence in depth on top of the resolve() prefix check, and silences CodeQL's
|
||||
// path-injection finding on the resolveOwner sidecar lookup).
|
||||
private static final Pattern UUID_PATTERN =
|
||||
Pattern.compile(
|
||||
"^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$");
|
||||
|
||||
private final String baseDirPath;
|
||||
// Fixed-size lock stripes so concurrent store/delete on the same (or colliding) fileId
|
||||
// serialise the data-file + owner-sidecar pair as one critical section. Striped (not
|
||||
// per-id) so the map never has to be cleaned up; collisions across unrelated ids are
|
||||
// harmless contention.
|
||||
private static final int LOCK_STRIPES = 64;
|
||||
private final ReentrantLock[] stripes = new ReentrantLock[LOCK_STRIPES];
|
||||
|
||||
public LocalDiskFileStore(String baseDirPath) {
|
||||
this.baseDirPath = baseDirPath;
|
||||
for (int i = 0; i < LOCK_STRIPES; i++) {
|
||||
stripes[i] = new ReentrantLock();
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public Stored store(InputStream in, String originalName) throws IOException {
|
||||
public Stored store(InputStream in, String originalName, String owner) throws IOException {
|
||||
String fileId = UUID.randomUUID().toString();
|
||||
Path filePath = resolve(fileId);
|
||||
Files.createDirectories(filePath.getParent());
|
||||
ReentrantLock lock = acquire(fileId);
|
||||
boolean success = false;
|
||||
try {
|
||||
long size = Files.copy(in, filePath);
|
||||
writeOwner(fileId, owner);
|
||||
success = true;
|
||||
return new Stored(fileId, size);
|
||||
} finally {
|
||||
if (!success) {
|
||||
try {
|
||||
Files.deleteIfExists(filePath);
|
||||
} catch (IOException cleanupEx) {
|
||||
log.warn(
|
||||
"Failed to clean up partial file {} after store failure",
|
||||
filePath,
|
||||
cleanupEx);
|
||||
}
|
||||
cleanupAfterFailedStore(fileId, filePath);
|
||||
}
|
||||
release(fileId, lock);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -52,27 +69,44 @@ public class LocalDiskFileStore implements FileStore {
|
||||
* the source size before copying so the post-copy stat is unnecessary.
|
||||
*/
|
||||
@Override
|
||||
public Stored store(Path source, String originalName) throws IOException {
|
||||
public Stored store(Path source, String originalName, String owner) throws IOException {
|
||||
String fileId = UUID.randomUUID().toString();
|
||||
Path filePath = resolve(fileId);
|
||||
Files.createDirectories(filePath.getParent());
|
||||
long size = Files.size(source);
|
||||
ReentrantLock lock = acquire(fileId);
|
||||
boolean success = false;
|
||||
try {
|
||||
Files.copy(source, filePath);
|
||||
writeOwner(fileId, owner);
|
||||
success = true;
|
||||
return new Stored(fileId, size);
|
||||
} finally {
|
||||
if (!success) {
|
||||
try {
|
||||
Files.deleteIfExists(filePath);
|
||||
} catch (IOException cleanupEx) {
|
||||
log.warn(
|
||||
"Failed to clean up partial file {} after store failure",
|
||||
filePath,
|
||||
cleanupEx);
|
||||
}
|
||||
cleanupAfterFailedStore(fileId, filePath);
|
||||
}
|
||||
release(fileId, lock);
|
||||
}
|
||||
}
|
||||
|
||||
private void writeOwner(String fileId, String owner) throws IOException {
|
||||
if (owner == null || owner.isBlank()) {
|
||||
return;
|
||||
}
|
||||
Path ownerPath = resolveOwner(fileId);
|
||||
Files.write(ownerPath, owner.getBytes(StandardCharsets.UTF_8));
|
||||
}
|
||||
|
||||
private void cleanupAfterFailedStore(String fileId, Path filePath) {
|
||||
try {
|
||||
Files.deleteIfExists(filePath);
|
||||
} catch (IOException cleanupEx) {
|
||||
log.warn("Failed to clean up partial file {} after store failure", filePath, cleanupEx);
|
||||
}
|
||||
try {
|
||||
Files.deleteIfExists(resolveOwner(fileId));
|
||||
} catch (IOException cleanupEx) {
|
||||
log.warn("Failed to clean up owner sidecar for {} after store failure", fileId);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -101,11 +135,26 @@ public class LocalDiskFileStore implements FileStore {
|
||||
|
||||
@Override
|
||||
public boolean delete(String fileId) {
|
||||
ReentrantLock lock = acquire(fileId);
|
||||
try {
|
||||
return Files.deleteIfExists(resolve(fileId));
|
||||
} catch (IOException e) {
|
||||
log.error("Error deleting file with ID: {}", fileId, e);
|
||||
return false;
|
||||
// Data first, owner second: a concurrent retrieve that observes the transient
|
||||
// (data-gone, owner-still-present) window simply fails with IOException; the inverse
|
||||
// order would briefly look like an unowned file and could grant cross-user access.
|
||||
boolean removed;
|
||||
try {
|
||||
removed = Files.deleteIfExists(resolve(fileId));
|
||||
} catch (IOException e) {
|
||||
log.error("Error deleting file with ID: {}", fileId, e);
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
Files.deleteIfExists(resolveOwner(fileId));
|
||||
} catch (IOException e) {
|
||||
log.warn("Error deleting owner sidecar for file ID: {}", fileId, e);
|
||||
}
|
||||
return removed;
|
||||
} finally {
|
||||
release(fileId, lock);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -114,8 +163,21 @@ public class LocalDiskFileStore implements FileStore {
|
||||
return Files.exists(resolve(fileId));
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getOwner(String fileId) throws IOException {
|
||||
Path ownerPath = resolveOwner(fileId);
|
||||
if (!Files.exists(ownerPath)) {
|
||||
return null;
|
||||
}
|
||||
byte[] bytes = Files.readAllBytes(ownerPath);
|
||||
if (bytes.length == 0) {
|
||||
return null;
|
||||
}
|
||||
return new String(bytes, StandardCharsets.UTF_8);
|
||||
}
|
||||
|
||||
public Path resolve(String fileId) {
|
||||
if (fileId.contains("..") || fileId.contains("/") || fileId.contains("\\")) {
|
||||
if (fileId == null || !UUID_PATTERN.matcher(fileId).matches()) {
|
||||
throw new IllegalArgumentException("Invalid file ID");
|
||||
}
|
||||
Path basePath = Path.of(baseDirPath).normalize().toAbsolutePath();
|
||||
@@ -125,4 +187,19 @@ public class LocalDiskFileStore implements FileStore {
|
||||
}
|
||||
return resolvedPath;
|
||||
}
|
||||
|
||||
private Path resolveOwner(String fileId) {
|
||||
Path data = resolve(fileId);
|
||||
return data.resolveSibling(data.getFileName().toString() + OWNER_SUFFIX);
|
||||
}
|
||||
|
||||
private ReentrantLock acquire(String fileId) {
|
||||
ReentrantLock lock = stripes[(fileId.hashCode() & Integer.MAX_VALUE) % LOCK_STRIPES];
|
||||
lock.lock();
|
||||
return lock;
|
||||
}
|
||||
|
||||
private void release(String fileId, ReentrantLock lock) {
|
||||
lock.unlock();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -77,6 +77,7 @@ public class ApplicationProperties {
|
||||
private ProcessExecutor processExecutor = new ProcessExecutor();
|
||||
private PdfEditor pdfEditor = new PdfEditor();
|
||||
private AiEngine aiEngine = new AiEngine();
|
||||
private Mcp mcp = new Mcp();
|
||||
private InternalApi internalApi = new InternalApi();
|
||||
private Cluster cluster = new Cluster();
|
||||
|
||||
@@ -256,6 +257,94 @@ public class ApplicationProperties {
|
||||
private int longRunningTimeoutSeconds = 600;
|
||||
}
|
||||
|
||||
/**
|
||||
* Model Context Protocol (MCP) server configuration. All keys live under the top-level {@code
|
||||
* mcp.*} prefix. {@link #enabled} defaults to {@code false}: when off, no MCP beans are wired,
|
||||
* no /mcp endpoint exists, and no protected-resource metadata is published.
|
||||
*/
|
||||
@Data
|
||||
public static class Mcp {
|
||||
|
||||
/** Master switch. When {@code false} (default), no MCP beans are wired. */
|
||||
private boolean enabled = false;
|
||||
|
||||
/**
|
||||
* When {@code true} (default), invocations require an OAuth scope: {@code mcp.tools.read}
|
||||
* for read-style operations and {@code mcp.tools.write} for write/destructive ones. When
|
||||
* {@code false}, scope checks are skipped (use only if your IdP issues a single coarse
|
||||
* scope).
|
||||
*/
|
||||
private boolean scopesEnabled = true;
|
||||
|
||||
/** How often to refresh the AI capabilities manifest from the engine. */
|
||||
private int engineCapabilityRefreshMinutes = 5;
|
||||
|
||||
/**
|
||||
* Tool allow-list (operation ids, e.g. {@code compress-pdf}). When non-empty, ONLY these
|
||||
* operations are exposed over MCP; everything else is hidden, undescribable, and
|
||||
* uninvocable - on top of the global endpoint enable/disable config. Empty = allow all.
|
||||
*/
|
||||
private List<String> allowedOperations = new ArrayList<>();
|
||||
|
||||
/**
|
||||
* Tool deny-list (operation ids). Any operation listed here is removed from MCP even if it
|
||||
* would otherwise be allowed. Applied after {@link #allowedOperations}.
|
||||
*/
|
||||
private List<String> blockedOperations = new ArrayList<>();
|
||||
|
||||
/** Max MCP request body size in bytes; inline file uploads ride in the JSON-RPC body. */
|
||||
private long maxRequestBytes = 10L * 1024 * 1024;
|
||||
|
||||
/** Results up to this size return inline as base64; larger ones return a fileId only. */
|
||||
private long maxInlineResponseBytes = 10L * 1024 * 1024;
|
||||
|
||||
private Auth auth = new Auth();
|
||||
|
||||
@Data
|
||||
public static class Auth {
|
||||
/**
|
||||
* Authentication mode for the MCP endpoint. {@code oauth} (default) runs a full OAuth2
|
||||
* resource server (JWT, RFC 8707 audience, RFC 9728 metadata). {@code apikey} accepts a
|
||||
* Stirling per-user API key via the {@code X-API-KEY} header (or {@code Authorization:
|
||||
* Bearer <key>}) and binds the request to that user - the low-friction self-host path,
|
||||
* no external IdP required.
|
||||
*/
|
||||
private String mode = "oauth";
|
||||
|
||||
/** OAuth2 issuer URI, e.g. {@code http://localhost:9000}. Required when MCP is on. */
|
||||
private String issuerUri = "";
|
||||
|
||||
/**
|
||||
* JWKS URI. When blank, derived from the issuer's {@code
|
||||
* /.well-known/openid-configuration} document.
|
||||
*/
|
||||
private String jwksUri = "";
|
||||
|
||||
/**
|
||||
* RFC 8707 resource identifier of THIS MCP server, e.g. {@code
|
||||
* http://localhost:8080/mcp}. Tokens that do not list this id in their {@code aud}
|
||||
* claim are rejected with HTTP 401.
|
||||
*/
|
||||
private String resourceId = "";
|
||||
|
||||
/**
|
||||
* JWT claim whose value is matched against a provisioned Stirling username. Defaults to
|
||||
* {@code sub}; set to {@code email} or {@code preferred_username} to match how your IdP
|
||||
* maps users to Stirling accounts.
|
||||
*/
|
||||
private String usernameClaim = "sub";
|
||||
|
||||
/**
|
||||
* When {@code true} (default), a validated token is accepted only if its {@link
|
||||
* #usernameClaim} value resolves to an existing, enabled Stirling user account. Tokens
|
||||
* whose subject has no Stirling account (or a disabled one) are rejected with HTTP 403.
|
||||
* Set to {@code false} only if you intentionally want any IdP-valid token to use MCP
|
||||
* without a local account.
|
||||
*/
|
||||
private boolean requireExistingAccount = true;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Cluster backplane configuration. All keys live under the top-level {@code cluster.*} prefix
|
||||
* (e.g. env var {@code CLUSTER_ENABLED}). The master switch is {@link #enabled} and defaults to
|
||||
|
||||
@@ -5,6 +5,7 @@ import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.io.PipedInputStream;
|
||||
import java.io.PipedOutputStream;
|
||||
import java.util.Optional;
|
||||
import java.util.concurrent.Executors;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
|
||||
@@ -17,6 +18,7 @@ import lombok.RequiredArgsConstructor;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
|
||||
import stirling.software.common.cluster.FileStore;
|
||||
import stirling.software.common.util.JobContext;
|
||||
|
||||
/**
|
||||
* Service for storing and retrieving files with unique file IDs. Used by the AutoJobPostMapping
|
||||
@@ -32,8 +34,10 @@ public class FileStorage {
|
||||
|
||||
private final FileOrUploadService fileOrUploadService;
|
||||
private final FileStore fileStore;
|
||||
private final Optional<JobOwnershipService> jobOwnershipService;
|
||||
|
||||
public String storeFile(MultipartFile file) throws IOException {
|
||||
String owner = resolveOwner();
|
||||
// Fast path: when Spring buffered the multipart to disk (typical for large uploads), the
|
||||
// backing Resource exposes a real File. Hand the Path to the FileStore so it can do a
|
||||
// file-to-file copy (Linux sendfile, no copy through Java heap) rather than streaming
|
||||
@@ -48,7 +52,7 @@ public class FileStorage {
|
||||
if (res != null && res.isFile()) {
|
||||
try {
|
||||
FileStore.Stored stored =
|
||||
fileStore.store(res.getFile().toPath(), file.getOriginalFilename());
|
||||
fileStore.store(res.getFile().toPath(), file.getOriginalFilename(), owner);
|
||||
log.debug("Stored file with ID: {} (fast path)", stored.fileId());
|
||||
return stored.fileId();
|
||||
} catch (IOException ex) {
|
||||
@@ -57,40 +61,45 @@ public class FileStorage {
|
||||
}
|
||||
}
|
||||
try (InputStream in = file.getInputStream()) {
|
||||
FileStore.Stored stored = fileStore.store(in, file.getOriginalFilename());
|
||||
FileStore.Stored stored = fileStore.store(in, file.getOriginalFilename(), owner);
|
||||
log.debug("Stored file with ID: {}", stored.fileId());
|
||||
return stored.fileId();
|
||||
}
|
||||
}
|
||||
|
||||
public String storeBytes(byte[] bytes, String originalName) throws IOException {
|
||||
FileStore.Stored stored = fileStore.store(new ByteArrayInputStream(bytes), originalName);
|
||||
FileStore.Stored stored =
|
||||
fileStore.store(new ByteArrayInputStream(bytes), originalName, resolveOwner());
|
||||
log.debug("Stored byte array with ID: {}", stored.fileId());
|
||||
return stored.fileId();
|
||||
}
|
||||
|
||||
public MultipartFile retrieveFile(String fileId) throws IOException {
|
||||
enforceOwnership(fileId);
|
||||
byte[] fileData = fileStore.retrieveBytes(fileId);
|
||||
return fileOrUploadService.toMockMultipartFile(fileId, fileData);
|
||||
}
|
||||
|
||||
public byte[] retrieveBytes(String fileId) throws IOException {
|
||||
enforceOwnership(fileId);
|
||||
return fileStore.retrieveBytes(fileId);
|
||||
}
|
||||
|
||||
public InputStream retrieveInputStream(String fileId) throws IOException {
|
||||
enforceOwnership(fileId);
|
||||
return fileStore.retrieve(fileId);
|
||||
}
|
||||
|
||||
public StoredFile storeInputStream(InputStream inputStream, String originalName)
|
||||
throws IOException {
|
||||
FileStore.Stored stored = fileStore.store(inputStream, originalName);
|
||||
FileStore.Stored stored = fileStore.store(inputStream, originalName, resolveOwner());
|
||||
log.debug("Stored input stream with ID: {}", stored.fileId());
|
||||
return new StoredFile(stored.fileId(), stored.size());
|
||||
}
|
||||
|
||||
public String storeFromStreamingBody(StreamingResponseBody body, String originalName)
|
||||
throws IOException {
|
||||
String owner = resolveOwner();
|
||||
// Hold Throwable not IOException: an unchecked failure (NPE, IllegalState, OOM, etc.)
|
||||
// from the body writer would otherwise close the pipe with EOF and the consumer would
|
||||
// return a truncated file with no error surfaced to the caller.
|
||||
@@ -115,7 +124,7 @@ public class FileStorage {
|
||||
}
|
||||
}
|
||||
});
|
||||
FileStore.Stored stored = fileStore.store(in, originalName);
|
||||
FileStore.Stored stored = fileStore.store(in, originalName, owner);
|
||||
Throwable writerErr = bodyError.get();
|
||||
if (writerErr != null) {
|
||||
// Body failed mid-write: the FileStore persisted a truncated entry.
|
||||
@@ -159,21 +168,62 @@ public class FileStorage {
|
||||
|
||||
public String storeFromResource(Resource resource, String originalName) throws IOException {
|
||||
try (InputStream in = resource.getInputStream()) {
|
||||
FileStore.Stored stored = fileStore.store(in, originalName);
|
||||
FileStore.Stored stored = fileStore.store(in, originalName, resolveOwner());
|
||||
log.debug("Stored Resource with ID: {}", stored.fileId());
|
||||
return stored.fileId();
|
||||
}
|
||||
}
|
||||
|
||||
public boolean deleteFile(String fileId) {
|
||||
enforceOwnership(fileId);
|
||||
return fileStore.delete(fileId);
|
||||
}
|
||||
|
||||
public boolean fileExists(String fileId) {
|
||||
enforceOwnership(fileId);
|
||||
return fileStore.exists(fileId);
|
||||
}
|
||||
|
||||
public long getFileSize(String fileId) throws IOException {
|
||||
enforceOwnership(fileId);
|
||||
return fileStore.size(fileId);
|
||||
}
|
||||
|
||||
private String resolveOwner() {
|
||||
String propagated = JobContext.getOwner();
|
||||
if (propagated != null) {
|
||||
return propagated;
|
||||
}
|
||||
return jobOwnershipService.flatMap(JobOwnershipService::getCurrentUserId).orElse(null);
|
||||
}
|
||||
|
||||
private void enforceOwnership(String fileId) {
|
||||
if (jobOwnershipService.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
Optional<String> currentUser = jobOwnershipService.get().getCurrentUserId();
|
||||
if (currentUser.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
String owner;
|
||||
try {
|
||||
owner = fileStore.getOwner(fileId);
|
||||
} catch (IOException e) {
|
||||
log.warn("Failed to read owner for file {}: {}", fileId, e.getMessage());
|
||||
throw new SecurityException(
|
||||
"Access denied: could not verify ownership of the requested file");
|
||||
}
|
||||
if (owner == null) {
|
||||
return;
|
||||
}
|
||||
if (!owner.equals(currentUser.get())) {
|
||||
log.warn(
|
||||
"Access denied: user {} attempted to access file {} owned by {}",
|
||||
currentUser.get(),
|
||||
fileId,
|
||||
owner);
|
||||
throw new SecurityException(
|
||||
"Access denied: you do not have permission to access this file");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -89,6 +89,11 @@ public class JobExecutorService {
|
||||
|
||||
String jobId = scopedJobKey;
|
||||
|
||||
final String jobOwner =
|
||||
jobOwnershipService != null
|
||||
? jobOwnershipService.getCurrentUserId().orElse(null)
|
||||
: null;
|
||||
|
||||
long timeoutToUse = customTimeoutMs > 0 ? customTimeoutMs : effectiveTimeoutMs;
|
||||
|
||||
log.debug(
|
||||
@@ -119,6 +124,7 @@ public class JobExecutorService {
|
||||
try {
|
||||
stirling.software.common.util.JobContext.setJobId(
|
||||
capturedJobIdForQueue);
|
||||
stirling.software.common.util.JobContext.setOwner(jobOwner);
|
||||
Object result = work.get();
|
||||
processJobResult(capturedJobIdForQueue, result);
|
||||
return result;
|
||||
@@ -153,6 +159,7 @@ public class JobExecutorService {
|
||||
timeoutToUse);
|
||||
|
||||
stirling.software.common.util.JobContext.setJobId(capturedJobId);
|
||||
stirling.software.common.util.JobContext.setOwner(jobOwner);
|
||||
Object result = executeWithTimeout(() -> work.get(), timeoutToUse);
|
||||
processJobResult(capturedJobId, result);
|
||||
} catch (TimeoutException te) {
|
||||
|
||||
@@ -1,8 +1,9 @@
|
||||
package stirling.software.common.util;
|
||||
|
||||
/** Thread-local context for passing job ID across async boundaries */
|
||||
/** Thread-local context for passing job ID and owner across async boundaries */
|
||||
public class JobContext {
|
||||
private static final ThreadLocal<String> CURRENT_JOB_ID = new ThreadLocal<>();
|
||||
private static final ThreadLocal<String> CURRENT_OWNER = new ThreadLocal<>();
|
||||
|
||||
public static void setJobId(String jobId) {
|
||||
CURRENT_JOB_ID.set(jobId);
|
||||
@@ -12,7 +13,16 @@ public class JobContext {
|
||||
return CURRENT_JOB_ID.get();
|
||||
}
|
||||
|
||||
public static void setOwner(String owner) {
|
||||
CURRENT_OWNER.set(owner);
|
||||
}
|
||||
|
||||
public static String getOwner() {
|
||||
return CURRENT_OWNER.get();
|
||||
}
|
||||
|
||||
public static void clear() {
|
||||
CURRENT_JOB_ID.remove();
|
||||
CURRENT_OWNER.remove();
|
||||
}
|
||||
}
|
||||
|
||||
+44
@@ -3,11 +3,13 @@ package stirling.software.common.cluster.inprocess;
|
||||
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
import java.io.ByteArrayInputStream;
|
||||
import java.io.IOException;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
@@ -39,4 +41,46 @@ class LocalDiskFileStoreTest {
|
||||
assertThrows(IllegalArgumentException.class, () -> store.resolve("a/b"));
|
||||
assertThrows(IllegalArgumentException.class, () -> store.resolve("a\\b"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void ownerSidecarCannotBeReadAsFileId(@TempDir Path dir) throws IOException {
|
||||
LocalDiskFileStore store = new LocalDiskFileStore(dir.toString());
|
||||
FileStore.Stored stored =
|
||||
store.store(new ByteArrayInputStream("hi".getBytes()), "f.bin", "alice");
|
||||
String sidecarId = stored.fileId() + ".owner";
|
||||
assertThrows(IllegalArgumentException.class, () -> store.resolve(sidecarId));
|
||||
assertThrows(IllegalArgumentException.class, () -> store.retrieveBytes(sidecarId));
|
||||
}
|
||||
|
||||
@Test
|
||||
void ownerIsPersistedAndReturnedByGetOwner(@TempDir Path dir) throws IOException {
|
||||
LocalDiskFileStore store = new LocalDiskFileStore(dir.toString());
|
||||
FileStore.Stored stored =
|
||||
store.store(new ByteArrayInputStream("hi".getBytes()), "f.bin", "alice");
|
||||
assertEquals("alice", store.getOwner(stored.fileId()));
|
||||
}
|
||||
|
||||
@Test
|
||||
void getOwnerReturnsNullWhenNoOwnerWasRecorded(@TempDir Path dir) throws IOException {
|
||||
LocalDiskFileStore store = new LocalDiskFileStore(dir.toString());
|
||||
FileStore.Stored stored =
|
||||
store.store(new ByteArrayInputStream("hi".getBytes()), "f.bin", null);
|
||||
assertNull(store.getOwner(stored.fileId()));
|
||||
}
|
||||
|
||||
@Test
|
||||
void getOwnerReturnsNullForUnknownFileId(@TempDir Path dir) throws IOException {
|
||||
LocalDiskFileStore store = new LocalDiskFileStore(dir.toString());
|
||||
assertNull(store.getOwner("00000000-0000-0000-0000-000000000000"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void deleteRemovesOwnerSidecar(@TempDir Path dir) throws IOException {
|
||||
LocalDiskFileStore store = new LocalDiskFileStore(dir.toString());
|
||||
FileStore.Stored stored =
|
||||
store.store(new ByteArrayInputStream("hi".getBytes()), "f.bin", "alice");
|
||||
assertTrue(store.delete(stored.fileId()));
|
||||
assertFalse(Files.exists(dir.resolve(stored.fileId() + ".owner")));
|
||||
assertNull(store.getOwner(stored.fileId()));
|
||||
}
|
||||
}
|
||||
|
||||
+3
-1
@@ -5,6 +5,7 @@ import static org.mockito.Mockito.mock;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.nio.file.Path;
|
||||
import java.util.Optional;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.io.TempDir;
|
||||
@@ -19,7 +20,8 @@ class FileStorageDelegationTest {
|
||||
FileStorage fs =
|
||||
new FileStorage(
|
||||
mock(FileOrUploadService.class),
|
||||
new LocalDiskFileStore(tempDir.toString()));
|
||||
new LocalDiskFileStore(tempDir.toString()),
|
||||
Optional.empty());
|
||||
byte[] payload = "round-trip".getBytes();
|
||||
String id = fs.storeBytes(payload, "x.bin");
|
||||
assertArrayEquals(payload, fs.retrieveBytes(id));
|
||||
|
||||
+107
@@ -0,0 +1,107 @@
|
||||
package stirling.software.common.service;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.nio.file.Path;
|
||||
import java.util.Optional;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.io.TempDir;
|
||||
|
||||
import stirling.software.common.cluster.inprocess.LocalDiskFileStore;
|
||||
import stirling.software.common.util.JobContext;
|
||||
|
||||
class FileStorageOwnershipTest {
|
||||
|
||||
private FileStorage newStorageWithoutSecurity(Path tempDir) {
|
||||
return new FileStorage(
|
||||
mock(FileOrUploadService.class),
|
||||
new LocalDiskFileStore(tempDir.toString()),
|
||||
Optional.empty());
|
||||
}
|
||||
|
||||
private FileStorage newStorageWithCurrentUser(Path tempDir, AtomicReference<String> userRef) {
|
||||
JobOwnershipService svc = mock(JobOwnershipService.class);
|
||||
when(svc.getCurrentUserId()).thenAnswer(invocation -> Optional.ofNullable(userRef.get()));
|
||||
return new FileStorage(
|
||||
mock(FileOrUploadService.class),
|
||||
new LocalDiskFileStore(tempDir.toString()),
|
||||
Optional.of(svc));
|
||||
}
|
||||
|
||||
@Test
|
||||
void desktopMode_noOwnershipService_storesAndRetrievesWithoutChecks(@TempDir Path tempDir)
|
||||
throws IOException {
|
||||
FileStorage fs = newStorageWithoutSecurity(tempDir);
|
||||
byte[] payload = "desktop".getBytes();
|
||||
String id = fs.storeBytes(payload, "x.bin");
|
||||
assertArrayEquals(payload, fs.retrieveBytes(id));
|
||||
}
|
||||
|
||||
@Test
|
||||
void sameUserStoresAndRetrieves_allowed(@TempDir Path tempDir) throws IOException {
|
||||
AtomicReference<String> user = new AtomicReference<>("alice");
|
||||
FileStorage fs = newStorageWithCurrentUser(tempDir, user);
|
||||
byte[] payload = "alice's file".getBytes();
|
||||
String id = fs.storeBytes(payload, "x.bin");
|
||||
assertArrayEquals(payload, fs.retrieveBytes(id));
|
||||
}
|
||||
|
||||
@Test
|
||||
void differentUserRetrieves_throwsSecurityException(@TempDir Path tempDir) throws IOException {
|
||||
AtomicReference<String> user = new AtomicReference<>("alice");
|
||||
FileStorage fs = newStorageWithCurrentUser(tempDir, user);
|
||||
String id = fs.storeBytes("alice's file".getBytes(), "x.bin");
|
||||
user.set("bob");
|
||||
assertThrows(SecurityException.class, () -> fs.retrieveBytes(id));
|
||||
assertThrows(SecurityException.class, () -> fs.retrieveInputStream(id));
|
||||
assertThrows(SecurityException.class, () -> fs.getFileSize(id));
|
||||
assertThrows(SecurityException.class, () -> fs.fileExists(id));
|
||||
assertThrows(SecurityException.class, () -> fs.deleteFile(id));
|
||||
}
|
||||
|
||||
@Test
|
||||
void anonymousRetrieveOfOwnedFile_allowed_noCurrentUserMeansNoCompare(@TempDir Path tempDir)
|
||||
throws IOException {
|
||||
AtomicReference<String> user = new AtomicReference<>("alice");
|
||||
FileStorage fs = newStorageWithCurrentUser(tempDir, user);
|
||||
byte[] payload = "alice's file".getBytes();
|
||||
String id = fs.storeBytes(payload, "x.bin");
|
||||
user.set(null);
|
||||
assertArrayEquals(payload, fs.retrieveBytes(id));
|
||||
}
|
||||
|
||||
@Test
|
||||
void authedRetrieveOfAnonymousFile_allowed_noOwnerOnFile(@TempDir Path tempDir)
|
||||
throws IOException {
|
||||
AtomicReference<String> user = new AtomicReference<>(null);
|
||||
FileStorage fs = newStorageWithCurrentUser(tempDir, user);
|
||||
byte[] payload = "no-owner".getBytes();
|
||||
String id = fs.storeBytes(payload, "x.bin");
|
||||
user.set("alice");
|
||||
assertArrayEquals(payload, fs.retrieveBytes(id));
|
||||
}
|
||||
|
||||
@Test
|
||||
void propagatedOwner_scopesAsyncWriteWithNoLiveUser(@TempDir Path tempDir) throws IOException {
|
||||
AtomicReference<String> user = new AtomicReference<>(null);
|
||||
FileStorage fs = newStorageWithCurrentUser(tempDir, user);
|
||||
byte[] payload = "alice's async result".getBytes();
|
||||
String id;
|
||||
try {
|
||||
JobContext.setOwner("alice");
|
||||
id = fs.storeBytes(payload, "x.bin");
|
||||
} finally {
|
||||
JobContext.clear();
|
||||
}
|
||||
user.set("alice");
|
||||
assertArrayEquals(payload, fs.retrieveBytes(id));
|
||||
user.set("bob");
|
||||
assertThrows(SecurityException.class, () -> fs.retrieveBytes(id));
|
||||
}
|
||||
}
|
||||
@@ -9,6 +9,8 @@ import java.io.InputStream;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.Optional;
|
||||
import java.util.UUID;
|
||||
import java.util.stream.Stream;
|
||||
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
@@ -37,7 +39,10 @@ class FileStorageTest {
|
||||
void setUp() throws IOException {
|
||||
MockitoAnnotations.openMocks(this);
|
||||
fileStorage =
|
||||
new FileStorage(fileOrUploadService, new LocalDiskFileStore(tempDir.toString()));
|
||||
new FileStorage(
|
||||
fileOrUploadService,
|
||||
new LocalDiskFileStore(tempDir.toString()),
|
||||
Optional.empty());
|
||||
|
||||
// Create a mock MultipartFile
|
||||
mockFile = mock(MultipartFile.class);
|
||||
@@ -79,7 +84,7 @@ class FileStorageTest {
|
||||
void testRetrieveFile() throws IOException {
|
||||
// Arrange
|
||||
byte[] fileContent = "Test PDF content".getBytes();
|
||||
String fileId = "test-file-1";
|
||||
String fileId = UUID.randomUUID().toString();
|
||||
Path filePath = tempDir.resolve(fileId);
|
||||
Files.write(filePath, fileContent);
|
||||
|
||||
@@ -99,7 +104,7 @@ class FileStorageTest {
|
||||
void testRetrieveBytes() throws IOException {
|
||||
// Arrange
|
||||
byte[] fileContent = "Test PDF content".getBytes();
|
||||
String fileId = "test-file-2";
|
||||
String fileId = UUID.randomUUID().toString();
|
||||
Path filePath = tempDir.resolve(fileId);
|
||||
Files.write(filePath, fileContent);
|
||||
|
||||
@@ -113,7 +118,7 @@ class FileStorageTest {
|
||||
@Test
|
||||
void testRetrieveFile_FileNotFound() {
|
||||
// Arrange
|
||||
String nonExistentFileId = "non-existent-file";
|
||||
String nonExistentFileId = UUID.randomUUID().toString();
|
||||
|
||||
// Act & Assert
|
||||
assertThrows(IOException.class, () -> fileStorage.retrieveFile(nonExistentFileId));
|
||||
@@ -122,7 +127,7 @@ class FileStorageTest {
|
||||
@Test
|
||||
void testRetrieveBytes_FileNotFound() {
|
||||
// Arrange
|
||||
String nonExistentFileId = "non-existent-file";
|
||||
String nonExistentFileId = UUID.randomUUID().toString();
|
||||
|
||||
// Act & Assert
|
||||
assertThrows(IOException.class, () -> fileStorage.retrieveBytes(nonExistentFileId));
|
||||
@@ -132,7 +137,7 @@ class FileStorageTest {
|
||||
void testDeleteFile() throws IOException {
|
||||
// Arrange
|
||||
byte[] fileContent = "Test PDF content".getBytes();
|
||||
String fileId = "test-file-3";
|
||||
String fileId = UUID.randomUUID().toString();
|
||||
Path filePath = tempDir.resolve(fileId);
|
||||
Files.write(filePath, fileContent);
|
||||
|
||||
@@ -147,7 +152,7 @@ class FileStorageTest {
|
||||
@Test
|
||||
void testDeleteFile_FileNotFound() {
|
||||
// Arrange
|
||||
String nonExistentFileId = "non-existent-file";
|
||||
String nonExistentFileId = UUID.randomUUID().toString();
|
||||
|
||||
// Act
|
||||
boolean result = fileStorage.deleteFile(nonExistentFileId);
|
||||
@@ -160,7 +165,7 @@ class FileStorageTest {
|
||||
void testFileExists() throws IOException {
|
||||
// Arrange
|
||||
byte[] fileContent = "Test PDF content".getBytes();
|
||||
String fileId = "test-file-4";
|
||||
String fileId = UUID.randomUUID().toString();
|
||||
Path filePath = tempDir.resolve(fileId);
|
||||
Files.write(filePath, fileContent);
|
||||
|
||||
@@ -174,7 +179,7 @@ class FileStorageTest {
|
||||
@Test
|
||||
void testFileExists_FileNotFound() {
|
||||
// Arrange
|
||||
String nonExistentFileId = "non-existent-file";
|
||||
String nonExistentFileId = UUID.randomUUID().toString();
|
||||
|
||||
// Act
|
||||
boolean result = fileStorage.fileExists(nonExistentFileId);
|
||||
|
||||
Reference in New Issue
Block a user