diff --git a/cannamanage-api/src/main/java/de/cannamanage/api/controller/DocumentController.java b/cannamanage-api/src/main/java/de/cannamanage/api/controller/DocumentController.java index d6748a2..453fc63 100644 --- a/cannamanage-api/src/main/java/de/cannamanage/api/controller/DocumentController.java +++ b/cannamanage-api/src/main/java/de/cannamanage/api/controller/DocumentController.java @@ -1,14 +1,17 @@ package de.cannamanage.api.controller; import de.cannamanage.domain.entity.Document; +import de.cannamanage.domain.entity.TenantContext; import de.cannamanage.domain.enums.DocumentAccessLevel; import de.cannamanage.domain.enums.DocumentCategory; import de.cannamanage.service.DocumentService; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.*; import org.springframework.web.multipart.MultipartFile; +import org.springframework.web.server.ResponseStatusException; import java.io.IOException; import java.security.Principal; @@ -26,6 +29,21 @@ public class DocumentController { this.documentService = documentService; } + /** + * Verify the requested document belongs to the caller's current tenant (club). + * Prevents IDOR: a user from club A must not be able to download/delete a document of club B + * just by guessing or enumerating the document UUID. + */ + private Document loadOwnedDocument(UUID documentId) { + Document doc = documentService.getDocument(documentId); + UUID currentTenantId = TenantContext.getCurrentTenant(); + if (currentTenantId == null || doc.getClubId() == null || !doc.getClubId().equals(currentTenantId)) { + // Use 403 (not 404) — caller is authenticated, just not authorized for this resource. + throw new ResponseStatusException(HttpStatus.FORBIDDEN, "Access denied to document"); + } + return doc; + } + @PostMapping("/documents/upload") public ResponseEntity uploadDocument( @RequestParam UUID clubId, @@ -51,7 +69,7 @@ public class DocumentController { @GetMapping("/documents/{id}/download") public ResponseEntity downloadDocument(@PathVariable UUID id) throws IOException { - Document doc = documentService.getDocument(id); + Document doc = loadOwnedDocument(id); byte[] content = documentService.downloadDocument(id); return ResponseEntity.ok() .header(HttpHeaders.CONTENT_DISPOSITION, "attachment; filename=\"" + doc.getFilename() + "\"") @@ -64,8 +82,15 @@ public class DocumentController { @PathVariable UUID id, @RequestParam UUID clubId, Principal principal) throws IOException { + // Verify the document belongs to the caller's tenant before honouring the delete. + // Also reject if the supplied clubId param disagrees with the authenticated tenant. + Document doc = loadOwnedDocument(id); + UUID currentTenantId = TenantContext.getCurrentTenant(); + if (!clubId.equals(currentTenantId)) { + throw new ResponseStatusException(HttpStatus.FORBIDDEN, "Tenant mismatch"); + } UUID userId = UUID.fromString(principal.getName()); - documentService.deleteDocument(id, userId, clubId); + documentService.deleteDocument(id, userId, doc.getClubId()); return ResponseEntity.noContent().build(); } diff --git a/cannamanage-api/src/main/java/de/cannamanage/api/security/JwtService.java b/cannamanage-api/src/main/java/de/cannamanage/api/security/JwtService.java index 7d6a127..11bbeb9 100644 --- a/cannamanage-api/src/main/java/de/cannamanage/api/security/JwtService.java +++ b/cannamanage-api/src/main/java/de/cannamanage/api/security/JwtService.java @@ -4,6 +4,7 @@ import io.jsonwebtoken.Claims; import io.jsonwebtoken.Jwts; import io.jsonwebtoken.io.Decoders; import io.jsonwebtoken.security.Keys; +import jakarta.annotation.PostConstruct; import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Service; @@ -29,6 +30,32 @@ public class JwtService { @Value("${cannamanage.security.jwt.refresh-token-expiry:2592000}") private long refreshTokenExpiry; // seconds (30 days) + /** + * Sentinel value used in the application.properties default. If the runtime JWT secret + * matches this string (or is missing/too short) the application must fail to start — + * we never want a deployment to silently fall back to a publicly-known dev secret. + */ + static final String UNCONFIGURED_SECRET_MARKER = "CHANGE_ME_IN_PRODUCTION_THIS_WILL_FAIL_ON_STARTUP"; + + /** + * Validate JWT secret on startup — fail fast if the deployment is missing a proper secret. + * Runs after Spring property binding (@Value) so we see the effective value. + */ + @PostConstruct + void validateSecret() { + if (secretKey == null + || secretKey.isBlank() + || secretKey.length() < 32 + || UNCONFIGURED_SECRET_MARKER.equals(secretKey)) { + throw new IllegalStateException( + "FATAL: JWT secret is not configured or uses the default dev placeholder. " + + "Set the CANNAMANAGE_SECURITY_JWT_SECRET environment variable " + + "(or cannamanage.security.jwt.secret property) to a base64-encoded " + + "256-bit (or larger) random key." + ); + } + } + /** * Generate access token for ADMIN/MEMBER roles (no permissions claim needed). */ diff --git a/cannamanage-api/src/main/java/de/cannamanage/api/security/SecurityConfig.java b/cannamanage-api/src/main/java/de/cannamanage/api/security/SecurityConfig.java index 9698ed6..653cf48 100644 --- a/cannamanage-api/src/main/java/de/cannamanage/api/security/SecurityConfig.java +++ b/cannamanage-api/src/main/java/de/cannamanage/api/security/SecurityConfig.java @@ -58,6 +58,10 @@ public class SecurityConfig { .requestMatchers("/api/v1/stock/**").hasAnyRole("ADMIN", "STAFF") .requestMatchers("/api/v1/compliance/**").hasAnyRole("ADMIN", "STAFF", "MEMBER") .requestMatchers("/api/v1/reports/**").hasRole("ADMIN") + // Documents endpoint — explicit listing for defense-in-depth so it can + // never accidentally end up in a permitAll() rule above. Per-document + // tenant ownership is additionally enforced in DocumentController. + .requestMatchers("/api/v1/documents/**").hasAnyRole("ADMIN", "STAFF", "MEMBER") .anyRequest().authenticated()) .addFilterBefore(jwtAuthFilter, UsernamePasswordAuthenticationFilter.class); diff --git a/cannamanage-api/src/main/resources/application.properties b/cannamanage-api/src/main/resources/application.properties index 860b09b..376a9cf 100644 --- a/cannamanage-api/src/main/resources/application.properties +++ b/cannamanage-api/src/main/resources/application.properties @@ -5,7 +5,12 @@ spring.jpa.properties.hibernate.packagesToScan=de.cannamanage.domain.entity spring.flyway.enabled=false # JWT Security -cannamanage.security.jwt.secret=Y2FubmFtYW5hZ2Utand0LXNlY3JldC1rZXktZm9yLWRldmVsb3BtZW50LW9ubHktMzI= +# DO NOT ship a usable default secret. JwtService.validateSecret() detects the marker below +# and refuses to start, forcing every deployment to provide a real base64-encoded 256-bit key +# via the CANNAMANAGE_SECURITY_JWT_SECRET environment variable (or override property). +# Test/integration profiles pin their own valid dev secret in application-test.properties / +# application-integration.properties. +cannamanage.security.jwt.secret=${CANNAMANAGE_SECURITY_JWT_SECRET:CHANGE_ME_IN_PRODUCTION_THIS_WILL_FAIL_ON_STARTUP} cannamanage.security.jwt.access-token-expiry=3600 cannamanage.security.jwt.refresh-token-expiry=2592000 diff --git a/cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java b/cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java index 74db283..27b8904 100644 --- a/cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java +++ b/cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java @@ -56,9 +56,12 @@ public class DocumentService { throw new IllegalArgumentException("File type not allowed. Allowed: PDF, DOCX, XLSX, PNG, JPG"); } - // Generate storage path + // Generate storage path — sanitize the original filename to prevent path traversal + // (e.g. "../../etc/passwd") and control-character / illegal-character injection into the + // on-disk path. We only keep the basename and strip everything that could break out of + // the per-club directory or confuse the OS filesystem. UUID documentId = UUID.randomUUID(); - String filename = file.getOriginalFilename() != null ? file.getOriginalFilename() : "document"; + String filename = sanitizeFilename(file.getOriginalFilename()); String storagePath = clubId + "/" + documentId + "_" + filename; Path fullPath = Paths.get(UPLOAD_BASE, storagePath); @@ -148,7 +151,10 @@ public class DocumentService { @Transactional public UUID archiveProtocol(UUID clubId, String title, byte[] pdfBytes, UUID uploadedBy) { UUID documentId = UUID.randomUUID(); - String filename = title.replaceAll("[^a-zA-Z0-9äöüÄÖÜß\\s\\-]", "") + ".pdf"; + // Same sanitization invariants as user-uploaded files: no path separators, no control + // chars, reasonable length, never empty. + String safeTitle = title == null ? "" : title.replaceAll("[^a-zA-Z0-9äöüÄÖÜß\\s\\-]", ""); + String filename = sanitizeFilename(safeTitle + ".pdf"); String storagePath = clubId + "/" + documentId + "_" + filename; Path fullPath = Paths.get(UPLOAD_BASE, storagePath); @@ -180,4 +186,36 @@ public class DocumentService { log.info("Protocol archived: {} for club {}", title, clubId); return documentId; } + + /** + * Sanitize a user-supplied filename for safe use in storage paths. + * - Strips any directory components (defeats "../../etc/passwd" style traversal) + * - Removes control characters and characters reserved on common filesystems + * - Limits length to keep the resulting path well under filesystem limits + * - Falls back to a UUID-based name when input is null/blank or sanitization empties it + */ + private String sanitizeFilename(String original) { + if (original == null || original.isBlank()) { + return UUID.randomUUID().toString(); + } + // Strip path components — keep only the basename + String name; + try { + name = Paths.get(original).getFileName().toString(); + } catch (RuntimeException e) { + // Invalid path on this platform — fall back to a random name + return UUID.randomUUID().toString(); + } + // Remove control characters and path-/shell-/Windows-reserved characters + name = name.replaceAll("[\\x00-\\x1F\\x7F/\\\\:*?\"<>|]", "_"); + // Limit length (filesystems usually cap individual segments at 255 bytes) + if (name.length() > 200) { + name = name.substring(0, 200); + } + // Ensure not empty after sanitization + if (name.isBlank() || ".".equals(name) || "..".equals(name)) { + return UUID.randomUUID().toString(); + } + return name; + } }