fix(security): hardening — rate limiting, CORS config, audit safety, CSP headers, validation
- Fix 1: Login rate limiting (5 attempts/min/IP) on POST /api/v1/auth/login - New LoginRateLimiter (ConcurrentHashMap + @Scheduled reset every 60s) - HTTP 429 with German message on exceed - Client IP via X-Forwarded-For with proxy fallback - @EnableScheduling on CannaManageApplication - Fix 2: CORS origins configurable via cannamanage.cors.allowed-origins env var - Defaults to localhost + docker frontend for dev - SecurityConfig reads with @Value, splits comma-separated list - Fix 3: Audit JSON safety — replaced manual string concat with Jackson ObjectMapper - New AuditService.toMetadataJson(Map) helper - RetentionService and AuthorityExportService refactored - Fix 4: Tomcat max-http-form-post-size=2MB prevents DoS via oversized payloads - Fix 5: @Valid added to @RequestBody on 17+ endpoints across ComplianceRecordsController, FinanceController, ConsentController, StaffController, ComplianceDeadlineController, SubscriptionController, ForumController (admin + portal) - Fix 6: Content-Security-Policy 'default-src \'self\'; frame-ancestors \'none\'' + frameOptions(deny) on both API + portal filter chains
This commit is contained in:
Patrick Plate
@@ -4,6 +4,7 @@ import org.springframework.boot.SpringApplication;
|
||||
import org.springframework.boot.autoconfigure.SpringBootApplication;
|
||||
import org.springframework.boot.persistence.autoconfigure.EntityScan;
|
||||
import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
|
||||
import org.springframework.scheduling.annotation.EnableScheduling;
|
||||
|
||||
/**
|
||||
* CannaManage Spring Boot application entry point.
|
||||
@@ -17,6 +18,7 @@ import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
|
||||
@SpringBootApplication(scanBasePackages = "de.cannamanage")
|
||||
@EnableJpaRepositories(basePackages = "de.cannamanage.service.repository")
|
||||
@EntityScan(basePackages = "de.cannamanage.domain.entity")
|
||||
@EnableScheduling
|
||||
public class CannaManageApplication {
|
||||
|
||||
public static void main(String[] args) {
|
||||
|
||||
@@ -4,11 +4,14 @@ import de.cannamanage.api.dto.auth.LoginRequest;
|
||||
import de.cannamanage.api.dto.auth.LoginResponse;
|
||||
import de.cannamanage.api.dto.auth.RefreshRequest;
|
||||
import de.cannamanage.api.dto.auth.SetPasswordRequest;
|
||||
import de.cannamanage.api.security.LoginRateLimiter;
|
||||
import de.cannamanage.api.service.AuthService;
|
||||
import io.swagger.v3.oas.annotations.Operation;
|
||||
import io.swagger.v3.oas.annotations.tags.Tag;
|
||||
import jakarta.servlet.http.HttpServletRequest;
|
||||
import jakarta.validation.Valid;
|
||||
import lombok.RequiredArgsConstructor;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.web.bind.annotation.PostMapping;
|
||||
import org.springframework.web.bind.annotation.RequestBody;
|
||||
@@ -24,10 +27,19 @@ import java.util.Map;
|
||||
public class AuthController {
|
||||
|
||||
private final AuthService authService;
|
||||
private final LoginRateLimiter loginRateLimiter;
|
||||
|
||||
@PostMapping("/login")
|
||||
@Operation(summary = "Login with email + password", description = "Returns JWT access + refresh tokens")
|
||||
public ResponseEntity<LoginResponse> login(@Valid @RequestBody LoginRequest request) {
|
||||
public ResponseEntity<?> login(@Valid @RequestBody LoginRequest request, HttpServletRequest httpRequest) {
|
||||
String ip = resolveClientIp(httpRequest);
|
||||
if (!loginRateLimiter.tryAcquire(ip)) {
|
||||
return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS)
|
||||
.body(Map.of(
|
||||
"error", "rate_limited",
|
||||
"message", "Zu viele Anmeldeversuche. Bitte warten Sie eine Minute."
|
||||
));
|
||||
}
|
||||
LoginResponse response = authService.login(request);
|
||||
return ResponseEntity.ok(response);
|
||||
}
|
||||
@@ -46,4 +58,17 @@ public class AuthController {
|
||||
authService.setPassword(request);
|
||||
return ResponseEntity.ok(Map.of("message", "Password set successfully. You can now log in."));
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the originating client IP, honouring X-Forwarded-For when present
|
||||
* (so reverse-proxy / load-balancer setups still get per-client rate limits).
|
||||
*/
|
||||
private String resolveClientIp(HttpServletRequest request) {
|
||||
String xff = request.getHeader("X-Forwarded-For");
|
||||
if (xff != null && !xff.isBlank()) {
|
||||
int comma = xff.indexOf(',');
|
||||
return (comma > 0 ? xff.substring(0, comma) : xff).trim();
|
||||
}
|
||||
return request.getRemoteAddr();
|
||||
}
|
||||
}
|
||||
|
||||
+3
-2
@@ -6,6 +6,7 @@ import de.cannamanage.domain.enums.ComplianceArea;
|
||||
import de.cannamanage.service.repository.ComplianceDeadlineRepository;
|
||||
import io.swagger.v3.oas.annotations.Operation;
|
||||
import io.swagger.v3.oas.annotations.tags.Tag;
|
||||
import jakarta.validation.Valid;
|
||||
import lombok.RequiredArgsConstructor;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.security.access.prepost.PreAuthorize;
|
||||
@@ -39,7 +40,7 @@ public class ComplianceDeadlineController {
|
||||
@PostMapping
|
||||
@Operation(summary = "Create a new compliance deadline")
|
||||
@PreAuthorize("hasRole('ADMIN') or @staffPermissions.has(authentication, T(de.cannamanage.domain.enums.StaffPermission).MANAGE_COMPLIANCE)")
|
||||
public ResponseEntity<ComplianceDeadline> createDeadline(@RequestBody CreateDeadlineRequest request) {
|
||||
public ResponseEntity<ComplianceDeadline> createDeadline(@Valid @RequestBody CreateDeadlineRequest request) {
|
||||
ComplianceDeadline deadline = new ComplianceDeadline();
|
||||
deadline.setClubId(request.clubId());
|
||||
deadline.setArea(request.area());
|
||||
@@ -57,7 +58,7 @@ public class ComplianceDeadlineController {
|
||||
@PreAuthorize("hasRole('ADMIN') or @staffPermissions.has(authentication, T(de.cannamanage.domain.enums.StaffPermission).MANAGE_COMPLIANCE)")
|
||||
public ResponseEntity<ComplianceDeadline> completeDeadline(
|
||||
@PathVariable UUID id,
|
||||
@RequestBody CompleteDeadlineRequest request) {
|
||||
@Valid @RequestBody CompleteDeadlineRequest request) {
|
||||
|
||||
ComplianceDeadline deadline = deadlineRepository.findById(id)
|
||||
.orElseThrow(() -> new IllegalArgumentException("Deadline not found: " + id));
|
||||
|
||||
+5
-4
@@ -6,6 +6,7 @@ import de.cannamanage.domain.enums.TransportStatus;
|
||||
import de.cannamanage.service.repository.*;
|
||||
import io.swagger.v3.oas.annotations.Operation;
|
||||
import io.swagger.v3.oas.annotations.tags.Tag;
|
||||
import jakarta.validation.Valid;
|
||||
import lombok.RequiredArgsConstructor;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.security.access.prepost.PreAuthorize;
|
||||
@@ -37,7 +38,7 @@ public class ComplianceRecordsController {
|
||||
@PostMapping("/destruction-records")
|
||||
@Operation(summary = "Record a cannabis destruction event")
|
||||
@PreAuthorize("hasRole('ADMIN') or @staffPermissions.has(authentication, T(de.cannamanage.domain.enums.StaffPermission).MANAGE_COMPLIANCE)")
|
||||
public ResponseEntity<DestructionRecord> recordDestruction(@RequestBody CreateDestructionRequest request) {
|
||||
public ResponseEntity<DestructionRecord> recordDestruction(@Valid @RequestBody CreateDestructionRequest request) {
|
||||
DestructionRecord record = new DestructionRecord();
|
||||
record.setClubId(request.clubId());
|
||||
record.setBatchId(request.batchId());
|
||||
@@ -65,7 +66,7 @@ public class ComplianceRecordsController {
|
||||
@PostMapping("/transport-records")
|
||||
@Operation(summary = "Record a cannabis transport event")
|
||||
@PreAuthorize("hasRole('ADMIN') or @staffPermissions.has(authentication, T(de.cannamanage.domain.enums.StaffPermission).MANAGE_COMPLIANCE)")
|
||||
public ResponseEntity<TransportRecord> recordTransport(@RequestBody CreateTransportRequest request) {
|
||||
public ResponseEntity<TransportRecord> recordTransport(@Valid @RequestBody CreateTransportRequest request) {
|
||||
TransportRecord record = new TransportRecord();
|
||||
record.setClubId(request.clubId());
|
||||
record.setDescription(request.description());
|
||||
@@ -94,7 +95,7 @@ public class ComplianceRecordsController {
|
||||
@PostMapping("/propagation-sources")
|
||||
@Operation(summary = "Record a propagation source (seed/cutting receipt)")
|
||||
@PreAuthorize("hasRole('ADMIN') or @staffPermissions.has(authentication, T(de.cannamanage.domain.enums.StaffPermission).MANAGE_COMPLIANCE)")
|
||||
public ResponseEntity<PropagationSource> recordPropagationSource(@RequestBody CreatePropagationSourceRequest request) {
|
||||
public ResponseEntity<PropagationSource> recordPropagationSource(@Valid @RequestBody CreatePropagationSourceRequest request) {
|
||||
PropagationSource record = new PropagationSource();
|
||||
record.setClubId(request.clubId());
|
||||
record.setSourceType(request.sourceType());
|
||||
@@ -121,7 +122,7 @@ public class ComplianceRecordsController {
|
||||
@PostMapping("/prevention-activities")
|
||||
@Operation(summary = "Record a prevention/education activity per KCanG §23")
|
||||
@PreAuthorize("hasRole('ADMIN') or @staffPermissions.has(authentication, T(de.cannamanage.domain.enums.StaffPermission).MANAGE_COMPLIANCE)")
|
||||
public ResponseEntity<PreventionActivity> recordPreventionActivity(@RequestBody CreatePreventionActivityRequest request) {
|
||||
public ResponseEntity<PreventionActivity> recordPreventionActivity(@Valid @RequestBody CreatePreventionActivityRequest request) {
|
||||
PreventionActivity record = new PreventionActivity();
|
||||
record.setClubId(request.clubId());
|
||||
record.setActivityDate(request.activityDate());
|
||||
|
||||
@@ -9,6 +9,7 @@ import de.cannamanage.service.repository.UserRepository;
|
||||
import io.swagger.v3.oas.annotations.Operation;
|
||||
import io.swagger.v3.oas.annotations.tags.Tag;
|
||||
import jakarta.servlet.http.HttpServletRequest;
|
||||
import jakarta.validation.Valid;
|
||||
import lombok.RequiredArgsConstructor;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.security.core.Authentication;
|
||||
@@ -43,7 +44,7 @@ public class ConsentController {
|
||||
@PostMapping
|
||||
@Operation(summary = "Grant consent")
|
||||
public ResponseEntity<ConsentResponse> grantConsent(
|
||||
@RequestBody GrantConsentRequest request,
|
||||
@Valid @RequestBody GrantConsentRequest request,
|
||||
Authentication auth,
|
||||
HttpServletRequest httpRequest) {
|
||||
UUID userId = resolveUserId(auth);
|
||||
|
||||
@@ -81,7 +81,7 @@ public class FinanceController {
|
||||
|
||||
@PutMapping("/finance/fee-schedules/{id}")
|
||||
public ResponseEntity<FeeSchedule> updateFeeSchedule(@PathVariable UUID id,
|
||||
@RequestBody UpdateFeeScheduleRequest request,
|
||||
@Valid @RequestBody UpdateFeeScheduleRequest request,
|
||||
@AuthenticationPrincipal UserDetails principal) {
|
||||
permissionChecker.requirePermission(principal, StaffPermission.MANAGE_FINANCES);
|
||||
FeeSchedule updated = financeService.updateFeeSchedule(
|
||||
|
||||
@@ -3,6 +3,7 @@ package de.cannamanage.api.controller;
|
||||
import de.cannamanage.domain.entity.*;
|
||||
import de.cannamanage.domain.enums.*;
|
||||
import de.cannamanage.service.ForumService;
|
||||
import jakarta.validation.Valid;
|
||||
import org.springframework.data.domain.Page;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.web.bind.annotation.*;
|
||||
@@ -26,7 +27,7 @@ public class ForumController {
|
||||
// ---- Admin Topic Endpoints ----
|
||||
|
||||
@PostMapping("/forum/topics")
|
||||
public ResponseEntity<ForumTopic> createTopic(@RequestBody CreateTopicRequest request,
|
||||
public ResponseEntity<ForumTopic> createTopic(@Valid @RequestBody CreateTopicRequest request,
|
||||
@RequestHeader("X-Club-Id") UUID clubId,
|
||||
@RequestHeader("X-User-Id") UUID userId) {
|
||||
ForumTopic topic = forumService.createTopic(clubId, request.title(), request.content(), userId);
|
||||
@@ -90,7 +91,7 @@ public class ForumController {
|
||||
|
||||
@PostMapping("/forum/topics/{topicId}/replies")
|
||||
public ResponseEntity<ForumReply> createReply(@PathVariable UUID topicId,
|
||||
@RequestBody CreateReplyRequest request,
|
||||
@Valid @RequestBody CreateReplyRequest request,
|
||||
@RequestHeader("X-User-Id") UUID userId) {
|
||||
ForumReply reply = forumService.createReply(topicId, request.content(), userId);
|
||||
return ResponseEntity.ok(reply);
|
||||
@@ -98,7 +99,7 @@ public class ForumController {
|
||||
|
||||
@PutMapping("/forum/replies/{id}")
|
||||
public ResponseEntity<ForumReply> editReply(@PathVariable UUID id,
|
||||
@RequestBody CreateReplyRequest request,
|
||||
@Valid @RequestBody CreateReplyRequest request,
|
||||
@RequestHeader("X-User-Id") UUID userId) {
|
||||
ForumReply reply = forumService.editReply(id, request.content(), userId);
|
||||
return ResponseEntity.ok(reply);
|
||||
@@ -114,7 +115,7 @@ public class ForumController {
|
||||
// ---- Reaction Endpoints ----
|
||||
|
||||
@PostMapping("/forum/reactions")
|
||||
public ResponseEntity<Map<String, Object>> toggleReaction(@RequestBody ReactionRequest request,
|
||||
public ResponseEntity<Map<String, Object>> toggleReaction(@Valid @RequestBody ReactionRequest request,
|
||||
@RequestHeader("X-User-Id") UUID userId) {
|
||||
var result = forumService.toggleReaction(
|
||||
request.targetType(), request.targetId(), userId, request.reactionType());
|
||||
@@ -125,7 +126,7 @@ public class ForumController {
|
||||
// ---- Report Endpoints ----
|
||||
|
||||
@PostMapping("/forum/reports")
|
||||
public ResponseEntity<Map<String, String>> reportContent(@RequestBody ReportRequest request,
|
||||
public ResponseEntity<Map<String, String>> reportContent(@Valid @RequestBody ReportRequest request,
|
||||
@RequestHeader("X-Club-Id") UUID clubId,
|
||||
@RequestHeader("X-User-Id") UUID userId) {
|
||||
forumService.reportContent(clubId, request.targetType(), request.targetId(), userId, request.reason());
|
||||
@@ -147,7 +148,7 @@ public class ForumController {
|
||||
|
||||
@PostMapping("/forum/reports/{id}/review")
|
||||
public ResponseEntity<ForumReport> reviewReport(@PathVariable UUID id,
|
||||
@RequestBody ReviewReportRequest request,
|
||||
@Valid @RequestBody ReviewReportRequest request,
|
||||
@RequestHeader("X-User-Id") UUID userId) {
|
||||
ForumReport report = forumService.reviewReport(id, userId, request.status());
|
||||
return ResponseEntity.ok(report);
|
||||
@@ -156,7 +157,7 @@ public class ForumController {
|
||||
// ---- Portal Endpoints (member-scoped, same logic) ----
|
||||
|
||||
@PostMapping("/portal/forum/topics")
|
||||
public ResponseEntity<ForumTopic> portalCreateTopic(@RequestBody CreateTopicRequest request,
|
||||
public ResponseEntity<ForumTopic> portalCreateTopic(@Valid @RequestBody CreateTopicRequest request,
|
||||
@RequestHeader("X-Club-Id") UUID clubId,
|
||||
@RequestHeader("X-User-Id") UUID userId) {
|
||||
return ResponseEntity.ok(forumService.createTopic(clubId, request.title(), request.content(), userId));
|
||||
@@ -185,20 +186,20 @@ public class ForumController {
|
||||
|
||||
@PostMapping("/portal/forum/topics/{topicId}/replies")
|
||||
public ResponseEntity<ForumReply> portalCreateReply(@PathVariable UUID topicId,
|
||||
@RequestBody CreateReplyRequest request,
|
||||
@Valid @RequestBody CreateReplyRequest request,
|
||||
@RequestHeader("X-User-Id") UUID userId) {
|
||||
return ResponseEntity.ok(forumService.createReply(topicId, request.content(), userId));
|
||||
}
|
||||
|
||||
@PutMapping("/portal/forum/replies/{id}")
|
||||
public ResponseEntity<ForumReply> portalEditReply(@PathVariable UUID id,
|
||||
@RequestBody CreateReplyRequest request,
|
||||
@Valid @RequestBody CreateReplyRequest request,
|
||||
@RequestHeader("X-User-Id") UUID userId) {
|
||||
return ResponseEntity.ok(forumService.editReply(id, request.content(), userId));
|
||||
}
|
||||
|
||||
@PostMapping("/portal/forum/reactions")
|
||||
public ResponseEntity<Map<String, Object>> portalToggleReaction(@RequestBody ReactionRequest request,
|
||||
public ResponseEntity<Map<String, Object>> portalToggleReaction(@Valid @RequestBody ReactionRequest request,
|
||||
@RequestHeader("X-User-Id") UUID userId) {
|
||||
var result = forumService.toggleReaction(
|
||||
request.targetType(), request.targetId(), userId, request.reactionType());
|
||||
@@ -206,7 +207,7 @@ public class ForumController {
|
||||
}
|
||||
|
||||
@PostMapping("/portal/forum/reports")
|
||||
public ResponseEntity<Map<String, String>> portalReportContent(@RequestBody ReportRequest request,
|
||||
public ResponseEntity<Map<String, String>> portalReportContent(@Valid @RequestBody ReportRequest request,
|
||||
@RequestHeader("X-Club-Id") UUID clubId,
|
||||
@RequestHeader("X-User-Id") UUID userId) {
|
||||
forumService.reportContent(clubId, request.targetType(), request.targetId(), userId, request.reason());
|
||||
|
||||
@@ -83,7 +83,7 @@ public class StaffController {
|
||||
@PreAuthorize("hasRole('ADMIN')")
|
||||
@Operation(summary = "Update staff permissions/profile (revokes tokens on permission change)")
|
||||
public ResponseEntity<StaffResponse> updateStaff(@PathVariable UUID id,
|
||||
@RequestBody UpdateStaffRequest request) {
|
||||
@Valid @RequestBody UpdateStaffRequest request) {
|
||||
UUID tenantId = TenantContext.getCurrentTenant();
|
||||
StaffAccount staff = staffService.updateStaff(
|
||||
tenantId, id,
|
||||
|
||||
+2
-1
@@ -10,6 +10,7 @@ import de.cannamanage.service.StripeService;
|
||||
import de.cannamanage.service.repository.ClubRepository;
|
||||
import io.swagger.v3.oas.annotations.Operation;
|
||||
import io.swagger.v3.oas.annotations.tags.Tag;
|
||||
import jakarta.validation.Valid;
|
||||
import lombok.RequiredArgsConstructor;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
@@ -46,7 +47,7 @@ public class SubscriptionController {
|
||||
@PostMapping("/checkout")
|
||||
@Operation(summary = "Create checkout session", description = "Creates a Stripe Checkout session for plan upgrade")
|
||||
@PreAuthorize("hasRole('ADMIN')")
|
||||
public ResponseEntity<Map<String, String>> createCheckout(@RequestBody CheckoutRequest request) throws StripeException {
|
||||
public ResponseEntity<Map<String, String>> createCheckout(@Valid @RequestBody CheckoutRequest request) throws StripeException {
|
||||
UUID tenantId = TenantContext.getCurrentTenant();
|
||||
UUID clubId = clubRepository.findByTenantId(tenantId)
|
||||
.orElseThrow(() -> new IllegalStateException("No club for tenant"))
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
package de.cannamanage.api.security;
|
||||
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.springframework.scheduling.annotation.Scheduled;
|
||||
import org.springframework.stereotype.Component;
|
||||
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
import java.util.concurrent.atomic.AtomicInteger;
|
||||
|
||||
/**
|
||||
* Simple in-memory brute-force protection for the login endpoint.
|
||||
*
|
||||
* <p>Tracks attempts per source IP and rejects further attempts once the
|
||||
* configured threshold ({@link #MAX_ATTEMPTS_PER_WINDOW}) is exceeded within
|
||||
* the current 60-second window. Counters are reset every minute by
|
||||
* {@link #resetCounters()}.
|
||||
*
|
||||
* <p>This deliberately stays in-memory rather than introducing Resilience4j /
|
||||
* Bucket4j for a single endpoint. For multi-instance deployments behind a
|
||||
* load balancer this should be revisited.
|
||||
*/
|
||||
@Slf4j
|
||||
@Component
|
||||
public class LoginRateLimiter {
|
||||
|
||||
/** Maximum failed/total login attempts allowed per IP per window. */
|
||||
public static final int MAX_ATTEMPTS_PER_WINDOW = 5;
|
||||
|
||||
private final ConcurrentHashMap<String, AtomicInteger> attemptsByIp = new ConcurrentHashMap<>();
|
||||
|
||||
/**
|
||||
* Records an attempt and returns {@code true} if the request is allowed
|
||||
* (still within the per-window quota), {@code false} if it must be
|
||||
* rejected with HTTP 429.
|
||||
*/
|
||||
public boolean tryAcquire(String ipAddress) {
|
||||
if (ipAddress == null || ipAddress.isBlank()) {
|
||||
ipAddress = "unknown";
|
||||
}
|
||||
AtomicInteger counter = attemptsByIp.computeIfAbsent(ipAddress, k -> new AtomicInteger(0));
|
||||
int current = counter.incrementAndGet();
|
||||
if (current > MAX_ATTEMPTS_PER_WINDOW) {
|
||||
log.warn("Login rate limit exceeded for IP {} ({} attempts in current window)", ipAddress, current);
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resets all counters every 60 seconds. Fixed-rate scheduler keeps the
|
||||
* implementation predictable and free of timestamp bookkeeping.
|
||||
*/
|
||||
@Scheduled(fixedRate = 60_000L)
|
||||
public void resetCounters() {
|
||||
if (!attemptsByIp.isEmpty()) {
|
||||
log.debug("Resetting login rate-limit counters for {} IPs", attemptsByIp.size());
|
||||
attemptsByIp.clear();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -19,6 +19,7 @@ import org.springframework.web.cors.CorsConfigurationSource;
|
||||
import org.springframework.web.cors.UrlBasedCorsConfigurationSource;
|
||||
|
||||
import jakarta.servlet.http.HttpServletResponse;
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
|
||||
/**
|
||||
@@ -34,6 +35,14 @@ public class SecurityConfig {
|
||||
private final JwtAuthFilter jwtAuthFilter;
|
||||
private final PortalUserDetailsService portalUserDetailsService;
|
||||
|
||||
/**
|
||||
* Comma-separated allowed CORS origins. Defaults to local dev origins; production
|
||||
* deployments override via the {@code CORS_ORIGINS} environment variable
|
||||
* (e.g. {@code https://cannamanage.plate-software.de}).
|
||||
*/
|
||||
@Value("${cannamanage.cors.allowed-origins:http://localhost:3000,http://frontend:3000}")
|
||||
private String allowedOrigins;
|
||||
|
||||
/**
|
||||
* API security — stateless JWT authentication.
|
||||
* URL-level role checks provide first layer; @PreAuthorize provides fine-grained.
|
||||
@@ -47,6 +56,10 @@ public class SecurityConfig {
|
||||
.csrf(csrf -> csrf.disable())
|
||||
.sessionManagement(session -> session
|
||||
.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
|
||||
.headers(headers -> headers
|
||||
.contentSecurityPolicy(csp -> csp.policyDirectives(
|
||||
"default-src 'self'; frame-ancestors 'none'"))
|
||||
.frameOptions(frame -> frame.deny()))
|
||||
.authorizeHttpRequests(auth -> auth
|
||||
.requestMatchers("/api/v1/auth/**").permitAll()
|
||||
.requestMatchers("/api/v1/webhooks/**").permitAll()
|
||||
@@ -82,6 +95,10 @@ public class SecurityConfig {
|
||||
.sessionManagement(session -> session
|
||||
.sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED)
|
||||
.maximumSessions(1))
|
||||
.headers(headers -> headers
|
||||
.contentSecurityPolicy(csp -> csp.policyDirectives(
|
||||
"default-src 'self'; frame-ancestors 'none'"))
|
||||
.frameOptions(frame -> frame.deny()))
|
||||
.userDetailsService(portalUserDetailsService)
|
||||
.formLogin(form -> form
|
||||
.loginProcessingUrl("/portal/login")
|
||||
@@ -132,10 +149,11 @@ public class SecurityConfig {
|
||||
@Bean
|
||||
public CorsConfigurationSource corsConfigurationSource() {
|
||||
CorsConfiguration config = new CorsConfiguration();
|
||||
config.setAllowedOrigins(List.of(
|
||||
"http://localhost:3000",
|
||||
"http://frontend:3000"
|
||||
));
|
||||
List<String> origins = Arrays.stream(allowedOrigins.split(","))
|
||||
.map(String::trim)
|
||||
.filter(s -> !s.isEmpty())
|
||||
.toList();
|
||||
config.setAllowedOrigins(origins);
|
||||
config.setAllowedMethods(List.of("GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS"));
|
||||
config.setAllowedHeaders(List.of("*"));
|
||||
config.setAllowCredentials(true);
|
||||
|
||||
@@ -53,3 +53,9 @@ server.servlet.session.cookie.secure=${SESSION_COOKIE_SECURE:false}
|
||||
spring.servlet.multipart.enabled=true
|
||||
spring.servlet.multipart.max-file-size=5MB
|
||||
spring.servlet.multipart.max-request-size=6MB
|
||||
|
||||
# Security hardening — limit non-multipart request body sizes to prevent DoS via oversized payloads
|
||||
server.tomcat.max-http-form-post-size=2MB
|
||||
|
||||
# CORS allowed origins (comma-separated). Override via CORS_ORIGINS env var in production.
|
||||
cannamanage.cors.allowed-origins=${CORS_ORIGINS:http://localhost:3000,http://frontend:3000}
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
package de.cannamanage.service;
|
||||
|
||||
import com.fasterxml.jackson.core.JsonProcessingException;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import de.cannamanage.domain.entity.AuditEvent;
|
||||
import de.cannamanage.domain.entity.TenantContext;
|
||||
import de.cannamanage.domain.enums.AuditEventType;
|
||||
@@ -13,6 +15,7 @@ import org.springframework.transaction.annotation.Transactional;
|
||||
|
||||
import java.time.Instant;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.UUID;
|
||||
|
||||
/**
|
||||
@@ -25,6 +28,9 @@ public class AuditService {
|
||||
|
||||
private static final Logger log = LoggerFactory.getLogger(AuditService.class);
|
||||
|
||||
/** Shared ObjectMapper for safe JSON serialization of audit metadata. */
|
||||
private static final ObjectMapper METADATA_MAPPER = new ObjectMapper();
|
||||
|
||||
private final AuditEventRepository auditEventRepository;
|
||||
private final PdfReportGenerator pdfReportGenerator;
|
||||
|
||||
@@ -33,6 +39,23 @@ public class AuditService {
|
||||
this.pdfReportGenerator = pdfReportGenerator;
|
||||
}
|
||||
|
||||
/**
|
||||
* Serializes the given metadata map to JSON safely (proper escaping of quotes,
|
||||
* newlines, unicode). Returns {@code null} for a null/empty input and falls
|
||||
* back to {@code "{}"} if serialization fails (never throws).
|
||||
*/
|
||||
public static String toMetadataJson(Map<String, ?> metadata) {
|
||||
if (metadata == null || metadata.isEmpty()) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
return METADATA_MAPPER.writeValueAsString(metadata);
|
||||
} catch (JsonProcessingException e) {
|
||||
log.warn("Failed to serialize audit metadata; storing empty object instead: {}", e.getMessage());
|
||||
return "{}";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Writes an immutable audit event. Once persisted, it cannot be modified or deleted.
|
||||
*/
|
||||
|
||||
@@ -170,7 +170,7 @@ public class RetentionService {
|
||||
"SYSTEM",
|
||||
"RETENTION_SERVICE",
|
||||
"KCanG §24: Member data anonymized after 5-year retention period",
|
||||
"{\"membershipNumber\":\"" + member.getMembershipNumber() + "\"}",
|
||||
AuditService.toMetadataJson(Map.of("membershipNumber", member.getMembershipNumber())),
|
||||
null
|
||||
);
|
||||
|
||||
|
||||
+2
-1
@@ -6,6 +6,7 @@ import de.cannamanage.domain.enums.AuditEventType;
|
||||
import de.cannamanage.domain.enums.ExportFormat;
|
||||
import de.cannamanage.domain.enums.ReportType;
|
||||
import de.cannamanage.service.AuditService;
|
||||
import java.util.Map;
|
||||
import de.cannamanage.service.repository.ClubRepository;
|
||||
import de.cannamanage.service.repository.MemberRepository;
|
||||
import org.slf4j.Logger;
|
||||
@@ -101,7 +102,7 @@ public class AuthorityExportService {
|
||||
"Club", clubId, userId,
|
||||
"System", "ADMIN",
|
||||
"Behörden-Export generiert. Grund: " + reason + ". Jahr: " + year,
|
||||
"{\"year\":" + year + ",\"reason\":\"" + escapeJson(reason) + "\"}",
|
||||
AuditService.toMetadataJson(Map.of("year", year, "reason", reason)),
|
||||
null
|
||||
);
|
||||
|
||||
|
||||
@@ -0,0 +1,291 @@
|
||||
# CannaManage — Final Security & Code Review
|
||||
|
||||
**Datum:** 2026-06-15
|
||||
**Reviewer:** Lumen (Code mode, SonarQube + manual deep-scan)
|
||||
**Scope:** Vollständiges Backend (Sprints 1–10) — ca. 29.000 LOC Java Production, 3.600 LOC Tests, 26.000 LOC Frontend TypeScript
|
||||
**Branch:** master
|
||||
**BigMind Session:** `43f1d5c3-4805-42a6-8408-145784f6603e`
|
||||
|
||||
---
|
||||
|
||||
## 🎯 Verdict: ⚠️ CONDITIONAL PASS
|
||||
|
||||
**Production-Deployment ist NICHT freigegeben**, bis die 4 BLOCKER unten behoben sind. Das Fundament ist solide — Architektur, Tenant-Isolation, Compliance-Layer, Spring-Boot-CVE-Hardening, XXE-Schutz, BCrypt+SHA-256, GoBD-Append-Only-Ledger sind allesamt korrekt umgesetzt. Aber 4 konkrete, vermeidbare Schwachstellen blockieren den Go-Live.
|
||||
|
||||
| Kategorie | Score | Status |
|
||||
|-----------|-------|--------|
|
||||
| Architektur & Patterns | 9/10 | ✅ Sehr gut |
|
||||
| Authentication (JWT/BCrypt) | 7/10 | ⚠️ Dev-Fallback noch da |
|
||||
| Authorization (Roles + Permissions) | 6/10 | ❌ DocumentController ungeschützt |
|
||||
| Tenant-Isolation | 8/10 | ⚠️ Lücke bei Download-by-UUID |
|
||||
| Input Validation | 7/10 | ⚠️ Path Traversal in DocumentService |
|
||||
| Data Protection (DSGVO/KCanG/GoBD) | 10/10 | ✅ Vollständig |
|
||||
| Dependency Hygiene | 9/10 | ✅ Snyk-Overrides 2026-06-12 |
|
||||
| Error Handling | 9/10 | ✅ RFC 9457, keine Info-Disclosure |
|
||||
| Test Coverage | 4/10 | ❌ 20 Tests für 29K LOC zu wenig |
|
||||
| Logging Quality | 10/10 | ✅ Keine Concat, alles parameterized |
|
||||
|
||||
---
|
||||
|
||||
## 1. SonarQube SAST — Findings Summary
|
||||
|
||||
7 Dateien gescannt mit `analyze_code_snippet`. Nur echte Findings (S1598 Pfad-Rauschen ignoriert):
|
||||
|
||||
| Datei | Rule | Schweregrad | Befund |
|
||||
|-------|------|-------------|--------|
|
||||
| [`DocumentService.uploadDocument()`](cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java:60) | S2184 | MAJOR | `file.getSize() > MAX_FILE_SIZE` — int overflow risk (cast `(long)` fehlt) |
|
||||
| [`DocumentService.uploadDocument()`](cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java:62) | S1075 | MAJOR | Hard-coded path delimiter `"/"` |
|
||||
| [`DocumentService`](cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java:36) | S1068 | MAJOR | Unbenutztes Feld `auditService` (Dead Code) |
|
||||
| [`Camt053Parser`](cannamanage-service/src/main/java/de/cannamanage/service/bankimport/Camt053Parser.java:102) | S112 | MAJOR | Generisches `throw new Exception(...)` |
|
||||
| [`Camt053Parser`](cannamanage-service/src/main/java/de/cannamanage/service/bankimport/Camt053Parser.java:308) | S3252 | MINOR | Statischer Zugriff via Instanz statt Class |
|
||||
| [`AuthService.sha256()`](cannamanage-api/src/main/java/de/cannamanage/api/service/AuthService.java:144) | S112 | MAJOR | Generisches `RuntimeException` |
|
||||
| [`AuthService`](cannamanage-api/src/main/java/de/cannamanage/api/service/AuthService.java:54) | S1192 | MINOR | String `"Invalid credentials"` 3× dupliziert |
|
||||
| [`SecurityConfig`](cannamanage-api/src/main/java/de/cannamanage/api/security/SecurityConfig.java:25) | S1068 | MAJOR | (Snippet-Artefakt; im Original wird `portalUserDetailsService` genutzt — kein echtes Finding) |
|
||||
|
||||
`JwtService` und `BankImportService` waren bis auf S1598-Rauschen sauber.
|
||||
|
||||
---
|
||||
|
||||
## 2. Manual 16-Item Security Checklist
|
||||
|
||||
| # | Regel | Prüfpunkt | Ergebnis | Begründung |
|
||||
|---|-------|-----------|----------|-----------|
|
||||
| 1 | SEC-001..004 | Keine hartkodierten Credentials in Production-Code | ⚠️ Teilweise | JWT-Dev-Secret in [`application.properties:8`](cannamanage-api/src/main/resources/application.properties:8) — Production-Profile nutzt Env-Var, aber Default-Profile hat Fallback |
|
||||
| 2 | SEC-005 | Credentials via `@Value`/env | ✅ | Production-Properties durchgängig `${...}`; Stripe-Keys, DB-Credentials, JWT-Secret korrekt |
|
||||
| 3 | SEC-011 | Keine SQL-Injection | ✅ | **Null** `nativeQuery=true` im gesamten Codebase, **null** String-Concat in `@Query`. JPA/HQL nur mit benannten Parametern |
|
||||
| 4 | SEC-012 | Kein Path Traversal | ❌ | [`DocumentService.uploadDocument()`](cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java:62) nutzt `file.getOriginalFilename()` unsaniert. BankImportService macht es richtig mit `FilenameUtils.getName()` — Inkonsistenz |
|
||||
| 5 | SEC-016 | Input-Validierung | ✅ | DTOs mit `@Valid`, `MethodArgumentNotValidException` zentral behandelt, File-Type-Whitelist auf DocumentService |
|
||||
| 6 | SEC-018 | Keine Info-Disclosure in Fehlern | ✅ | [`GlobalExceptionHandler`](cannamanage-api/src/main/java/de/cannamanage/api/exception/GlobalExceptionHandler.java:133) loggt intern, gibt nur "An unexpected error occurred" zurück. Production-Properties: `server.error.include-stacktrace=never` |
|
||||
| 7 | SEC-033 | PII-Verschlüsselung (at rest) | ✅ | PostgreSQL TDE-fähig; Passwords BCrypt, Refresh-Tokens SHA-256 hashed. Keine Klartext-PII-Spalten neu eingeführt |
|
||||
| 8 | SEC-035 | Kein PII in LLM-Verarbeitung | ✅ N/A | Keine LLM-Integration im Code |
|
||||
| 9 | SEC-040 | Keine sensiblen Daten in Logs | ✅ | **Null** Treffer für `log.x(".." + ...)` Concat-Antipattern. Alle Logs parameterized. Stichproben: keine Passwörter, JWT-Tokens, IBANs vollständig geloggt |
|
||||
| 10 | SEC-055 | Keine generierten Quellen geändert | ✅ N/A | Kein `src.gen/` in diesem Projekt |
|
||||
| 11 | — | Tenant-Isolation aktiv | ⚠️ | Hibernate `@Filter("tenantFilter")` auf [`AbstractTenantEntity`](cannamanage-domain/src/main/java/de/cannamanage/domain/entity/AbstractTenantEntity.java:22) + [`TenantFilterAspect`](cannamanage-api/src/main/java/de/cannamanage/api/security/TenantFilterAspect.java:30) — solide. ABER: bei Lookups per Raw-UUID ohne `clubId`-Parameter greift Filter nur, wenn Aspect vorher den Tenant gesetzt hat. Defense-in-depth fehlt bei DocumentService.download |
|
||||
| 12 | — | JWT-Implementierung sicher | ⚠️ | HS256 mit 32-Byte-Base64-Key ✓, jti claim ✓, Access 1h / Refresh 30d ✓ — aber siehe Punkt 1 (Dev-Fallback) |
|
||||
| 13 | — | CSRF / Session-Management | ✅ | Member-Portal: `CookieCsrfTokenRepository` + Session-Fixation-Schutz (`maximumSessions(1)`). API: stateless JWT, CSRF korrekt disabled |
|
||||
| 14 | — | CORS restriktiv | ❌ | [`SecurityConfig.corsConfigurationSource()`](cannamanage-api/src/main/java/de/cannamanage/api/security/SecurityConfig.java:131) hardcoded auf `localhost:3000` + `frontend:3000`. Production-Domains nicht konfigurierbar. `setAllowedHeaders("*")` zu permissiv |
|
||||
| 15 | — | Rate-Limiting auf kritischen Endpoints | ❌ | Nur [`AuthorityExportService`](cannamanage-service/src/main/java/de/cannamanage/service/report/AuthorityExportService.java:76) hat Rate-Limit (1/h pro Tenant). **Kein Rate-Limit auf `/api/v1/auth/login`** — Brute-Force-Vektor |
|
||||
| 16 | SEC-064 | Keine Secrets im Source (DataRake) | ⚠️ | Dev-Default-JWT in `application.properties` würde DataRake triggern (Base64 ≥ 6 Zeichen, in nicht-Test-Datei). Sonst sauber |
|
||||
|
||||
---
|
||||
|
||||
## 3. Vulnerability Summary — Sprint 9 Findings Status
|
||||
|
||||
| # | Sprint-9-Finding | Erwartet | Tatsächlicher Status |
|
||||
|---|-----------------|----------|---------------------|
|
||||
| 1 | Path Traversal in DocumentService (Medium) | FIXED | ❌ **NICHT FIXED** — Code unverändert, `file.getOriginalFilename()` unsaniert |
|
||||
| 2 | JWT Dev-Secret Fallback (Medium) | FIXED | ❌ **NICHT FIXED** — [`application.properties:8`](cannamanage-api/src/main/resources/application.properties:8) hat noch Default-Wert |
|
||||
| 3 | Spring Boot CVE (Medium) | FIXED | ✅ **FIXED** — [`pom.xml`](pom.xml) Override 2026-06-12: Spring 7.0.8, Tomcat 11.0.22, Postgres 42.7.11, "fixes 10 HIGH + 18 MEDIUM Snyk SCA findings" |
|
||||
| 4 | CORS Hardcoded Localhost (Low) | FIXED | ❌ **NICHT FIXED** — siehe Item 14 |
|
||||
| 5 | Login Rate-Limit (Low) | FIXED | ❌ **NICHT FIXED** — siehe Item 15 |
|
||||
|
||||
### 🚨 Neu entdeckte Findings
|
||||
|
||||
#### BLOCKER #1 — IDOR / Horizontal Privilege Escalation in [`DocumentController`](cannamanage-api/src/main/java/de/cannamanage/api/controller/DocumentController.java:52)
|
||||
|
||||
```java
|
||||
@GetMapping("/documents/{id}/download")
|
||||
public ResponseEntity<byte[]> downloadDocument(@PathVariable UUID id) throws IOException {
|
||||
Document doc = documentService.getDocument(id);
|
||||
byte[] content = documentService.downloadDocument(id);
|
||||
...
|
||||
}
|
||||
```
|
||||
|
||||
- Kein `clubId`-Parameter, keine `@PreAuthorize`, kein Permission-Check, keine Tenant-Verifikation in der Service-Methode
|
||||
- `documentService.downloadDocument(id)` lädt das Document direkt per Raw-UUID
|
||||
- `documentRepository.findById(documentId)` greift zwar Hibernate-Filter — aber **nur wenn `TenantContext.getCurrentTenant()` korrekt aus dem JWT gesetzt ist**. Da der Endpoint im SecurityConfig nicht in einem spezifischen role-Matcher gelistet ist, fällt er unter `.anyRequest().authenticated()` — **JEDER authentifizierte User (MEMBER, STAFF eines anderen Clubs)** kann mit einer geratenen/geleakten UUID Dokumente herunterladen
|
||||
- Gleiches Muster bei `@DeleteMapping("/documents/{id}")`: Auch hier kein Permission-Check
|
||||
|
||||
**Severity:** HIGH (CVSS ~7.5 — N/L/N/R/U/N/H/N — Confidentiality breach across tenants)
|
||||
|
||||
#### BLOCKER #2 — Path Traversal in [`DocumentService.uploadDocument()`](cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java:62)
|
||||
|
||||
```java
|
||||
String filename = file.getOriginalFilename() != null ? file.getOriginalFilename() : "document";
|
||||
String storagePath = clubId + "/" + documentId + "_" + filename;
|
||||
Path fullPath = Paths.get(UPLOAD_BASE, storagePath);
|
||||
```
|
||||
|
||||
- Filename wie `../../etc/passwd.pdf` schreibt außerhalb von `UPLOAD_BASE`
|
||||
- BankImportService verwendet bereits korrekt `FilenameUtils.getName()` — Fix-Muster vorhanden, nur nicht angewendet
|
||||
|
||||
**Severity:** HIGH (CVSS ~7.5 — Arbitrary file write)
|
||||
|
||||
#### BLOCKER #3 — JWT Dev-Secret Fallback in [`application.properties:8`](cannamanage-api/src/main/resources/application.properties:8)
|
||||
|
||||
```properties
|
||||
cannamanage.security.jwt.secret=Y2FubmFtYW5hZ2Utand0LXNlY3JldC1rZXktZm9yLWRldmVsb3BtZW50LW9ubHktMzI=
|
||||
```
|
||||
|
||||
- Wenn Production-Deployment vergisst, `SPRING_PROFILES_ACTIVE=production` zu setzen, wird der bekannte Dev-Secret verwendet
|
||||
- Angreifer kann beliebige JWTs signieren und sich als ADMIN ausgeben
|
||||
|
||||
**Severity:** HIGH (im Worst Case CRITICAL) — Misconfiguration-Risiko
|
||||
|
||||
#### BLOCKER #4 — Kein DocumentController-Matcher in SecurityConfig
|
||||
|
||||
Im [`SecurityConfig.apiSecurityFilterChain()`](cannamanage-api/src/main/java/de/cannamanage/api/security/SecurityConfig.java:50) fehlen Matcher für `/api/v1/documents/**`. Defense-in-depth-Schicht entfällt. Sollte mindestens `.hasAnyRole("ADMIN", "STAFF")` für POST/DELETE und `.hasAnyRole("ADMIN", "STAFF", "MEMBER")` für GET sein, kombiniert mit Service-seitiger Tenant-Prüfung.
|
||||
|
||||
### Weitere mittlere Findings
|
||||
|
||||
| # | Befund | Severity |
|
||||
|---|--------|----------|
|
||||
| M1 | `DocumentService.downloadDocument()` / `deleteDocument()` haben kein AuditService-Logging (Feld ist sogar als ungenutzt markiert) — DSGVO Art. 30 verlangt Verarbeitungsverzeichnis für Document-Access | MEDIUM |
|
||||
| M2 | CORS `setAllowedHeaders("*")` + `setAllowCredentials(true)` ist seit CVE-2017-* zwar kein direkter Bug, aber Best-Practice ist explizite Header-Liste | LOW |
|
||||
| M3 | `AuthService.sha256()` wirft generisches `RuntimeException` bei `NoSuchAlgorithmException` — unreachable, aber sollte spezifische Custom-Exception sein | LOW |
|
||||
| M4 | Refresh-Token-Hash in User-Entity: einzelner Slot pro User → kein Multi-Device-Support; Logout auf einem Device killt Session auf anderen. Aktuell akzeptabel, dokumentieren | INFO |
|
||||
|
||||
---
|
||||
|
||||
## 4. Code Quality Assessment
|
||||
|
||||
### ✅ Stärken
|
||||
|
||||
| Aspekt | Bewertung |
|
||||
|--------|-----------|
|
||||
| Pattern-Konsistenz | Sehr hoch — Service+Repository+Controller-Trennung sauber, `AbstractTenantEntity` als gemeinsame Basis durchgezogen |
|
||||
| Transaction Boundaries | Korrekt — `@Transactional` auf Service-Methoden, `readOnly=true` wo angebracht (RetentionService, DsgvoService) |
|
||||
| N+1 Query Risk | Niedrig — [`PaymentMatchingService.buildContexts()`](cannamanage-service/src/main/java/de/cannamanage/service/bankimport/PaymentMatchingService.java:196) preloadet bewusst in 3 Bulk-Queries |
|
||||
| Null Safety | Hoch — `Optional` wird konsistent verwendet, `.orElseThrow()` mit aussagekräftigen Messages |
|
||||
| Error Handling | RFC 9457 application/problem+json über [`GlobalExceptionHandler`](cannamanage-api/src/main/java/de/cannamanage/api/exception/GlobalExceptionHandler.java:28), 13 spezifische `@ExceptionHandler` |
|
||||
| Audit Logging | 63 Aufrufe von `auditService.log(...)` verteilt — sehr gute Abdeckung |
|
||||
| Compliance Domain | GoBD §147 (Append-Only Ledger, kompensierende Buchungen), KCanG §22/§24, DSGVO Art. 15/17/30 vollständig implementiert |
|
||||
| XML/XXE | [`Camt053Parser`](cannamanage-service/src/main/java/de/cannamanage/service/bankimport/Camt053Parser.java:69) defensiv konfiguriert (DTD off, External Entities off, Entity Refs off) — vorbildlich |
|
||||
| Streaming für große Exports | [`AuthorityExportService`](cannamanage-service/src/main/java/de/cannamanage/service/report/AuthorityExportService.java:92) `OutputStream`-basiert, kein Heap-Risiko |
|
||||
| Stripe Webhook | Signature-Validierung via `Webhook.constructEvent(...)` ✓ |
|
||||
|
||||
### ⚠️ Schwächen
|
||||
|
||||
| Aspekt | Bewertung |
|
||||
|--------|-----------|
|
||||
| **Test Coverage** | **Kritisch niedrig** — 20 Tests für 29.000 LOC Production. Verhältnis ~12% nach LOC. Domain-Modul (`cannamanage-domain`) hat **0 Tests** trotz `AbstractTenantEntity`-Logik und Entity-Validierungen |
|
||||
| Magic Strings | `"Invalid credentials"`, `"GELÖSCHT"`, `"DOCUMENT_UPLOADED"` — sollten Konstanten in `enum`/`class` sein |
|
||||
| Dead Code | `DocumentService.auditService` ungenutzt; Sprint-Cleanup steht aus |
|
||||
| Inkonsistente File-Sanitization | BankImport ✓, Document ✗ — gleiches Risiko, zwei verschiedene Lösungen |
|
||||
| Cookie-Härtung explizit | `Secure`/`HttpOnly`/`SameSite` nicht in application.properties gesetzt; Spring-Defaults greifen, sollte aber explizit dokumentiert sein |
|
||||
|
||||
### Code-Smells nach SonarQube
|
||||
|
||||
- 4× S1192 (Duplicated String Literals) — kosmetisch
|
||||
- 3× S112 (Generic Exception) — in Edge-Paths (sha256, XML-Parse)
|
||||
- 1× S2184 (long cast missing) — `file.getSize()` ist bereits `long`, Konstante `MAX_FILE_SIZE` als `int` deklariert → potenziell `int * 1024 * 1024` Overflow, falls jemand `MAX_FILE_SIZE` erhöht
|
||||
- 1× S1075 (hard-coded `"/"`) — gleichzeitig OS-Inkompatibilität
|
||||
|
||||
---
|
||||
|
||||
## 5. Test Coverage Analyse
|
||||
|
||||
```
|
||||
Total Production LOC: 28.977
|
||||
Total Test LOC: 3.609
|
||||
Test Files: 20 (domain: 0, service: 9, api: 11)
|
||||
Ratio: ~12% (LOC) / ~7 Tests pro 10K LOC
|
||||
```
|
||||
|
||||
**Branchenstandard für Finanz/Compliance-Software: 70–80% Coverage, mindestens 1 Test pro 100 LOC.**
|
||||
|
||||
Kritische Lücken:
|
||||
- `cannamanage-domain` — 0 Tests trotz `AbstractTenantEntity`, Enums mit Business-Logik, Entity-Validierung
|
||||
- `RetentionService.anonymizeExpiredMembers()` — KCanG §24-Compliance, sollte voll getestet sein
|
||||
- `DsgvoService.exportUserData()` — Art. 15 DSGVO, rechtlich kritisch
|
||||
- `PaymentMatchingService.scoreOne()` — komplexer Algorithmus mit Schwellwerten
|
||||
- `Camt053Parser` / `Mt940Parser` — externe Datenformate, perfekter Fall für umfangreiche Testdaten
|
||||
|
||||
---
|
||||
|
||||
## 6. Compliance Snapshot
|
||||
|
||||
| Verordnung | Erfüllt? | Beleg |
|
||||
|-----------|---------|-------|
|
||||
| **DSGVO Art. 15** (Auskunft) | ✅ | [`DsgvoService.exportUserData()`](cannamanage-service/src/main/java/de/cannamanage/service/DsgvoService.java:50) |
|
||||
| **DSGVO Art. 17** (Löschung) | ✅ | [`DsgvoService.deleteUserData()`](cannamanage-service/src/main/java/de/cannamanage/service/DsgvoService.java:123) — anonymisiert statt löscht (KCanG-konform) |
|
||||
| **DSGVO Art. 30** (Verarbeitungsverzeichnis) | ⚠️ | AuditService vorhanden, aber Document-Download nicht geloggt |
|
||||
| **DSGVO Art. 32** (Sicherheit der Verarbeitung) | ⚠️ | Blockiert durch IDOR-Findings |
|
||||
| **DSGVO Art. 33/34** (Breach Notification) | ✅ | Workflow dokumentiert, audit_event_log persistent |
|
||||
| **DSGVO Art. 35** (DPIA) | ✅ | Templates in Sprint-7-Plan |
|
||||
| **KCanG §22** (Mitgliederverzeichnis) | ✅ | Members-Modul vollständig |
|
||||
| **KCanG §24** (Aufbewahrung 5J) | ✅ | [`RetentionService`](cannamanage-service/src/main/java/de/cannamanage/service/RetentionService.java:44) Scheduled Job, anonymisiert nach 5 Jahren |
|
||||
| **AO §147** (GoBD 10J) | ✅ | Append-Only Ledger, voids = kompensierende Buchungen, immutable nach completion |
|
||||
| **PSD2 / IBAN-Handling** | ✅ | Normalisierung, kein Klartext-Logging |
|
||||
| **Stripe PCI-DSS** | ✅ | Stripe Hosted Checkout, kein Karten-Daten-Touch |
|
||||
|
||||
---
|
||||
|
||||
## 7. Recommendations — Priorisiert
|
||||
|
||||
### 🚨 BLOCKER (vor Production-Deploy beheben — geschätzt 4–6h)
|
||||
|
||||
1. **[`DocumentController`](cannamanage-api/src/main/java/de/cannamanage/api/controller/DocumentController.java:52)**: `@PreAuthorize` einbauen, `clubId` als Path/Param fordern, Service-seitig Tenant-Match prüfen:
|
||||
```java
|
||||
@GetMapping("/documents/{id}/download")
|
||||
@PreAuthorize("hasAnyRole('ADMIN','STAFF','MEMBER')")
|
||||
public ResponseEntity<byte[]> downloadDocument(@PathVariable UUID id, Principal principal) {
|
||||
byte[] content = documentService.downloadDocumentForTenant(id, getCurrentTenantFromPrincipal(principal));
|
||||
...
|
||||
}
|
||||
```
|
||||
Service muss `doc.getTenantId().equals(currentTenantId)` verifizieren.
|
||||
|
||||
2. **[`DocumentService.uploadDocument()`](cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java:60)**: Filename sanitization:
|
||||
```java
|
||||
String safeName = org.apache.commons.io.FilenameUtils.getName(file.getOriginalFilename());
|
||||
if (safeName == null || safeName.isBlank()) safeName = "document";
|
||||
```
|
||||
|
||||
3. **[`application.properties:8`](cannamanage-api/src/main/resources/application.properties:8)**: Default-JWT-Secret entfernen, durch Fail-Fast ersetzen:
|
||||
```properties
|
||||
cannamanage.security.jwt.secret=${CANNAMANAGE_SECURITY_JWT_SECRET}
|
||||
```
|
||||
Plus `JwtService` Postconstruct-Validation: Mindestlänge 32 Bytes, sonst Startup-Failure.
|
||||
|
||||
4. **[`SecurityConfig`](cannamanage-api/src/main/java/de/cannamanage/api/security/SecurityConfig.java:50)**: `/api/v1/documents/**`-Matcher hinzufügen (Defense-in-Depth):
|
||||
```java
|
||||
.requestMatchers(HttpMethod.GET, "/api/v1/documents/**").hasAnyRole("ADMIN","STAFF","MEMBER")
|
||||
.requestMatchers("/api/v1/documents/**").hasAnyRole("ADMIN","STAFF")
|
||||
```
|
||||
|
||||
### 🟡 SHOULD-FIX (innerhalb der nächsten 2 Wochen — geschätzt 8–12h)
|
||||
|
||||
5. CORS konfigurierbar machen über `app.cors.allowed-origins` Property
|
||||
6. Rate-Limiting auf `/api/v1/auth/login` (z.B. Bucket4j, 10 req/min pro IP)
|
||||
7. Audit-Logging für Document-Download/Delete-Operationen
|
||||
8. `MAX_FILE_SIZE` als `long` deklarieren
|
||||
9. Konstanten für duplizierte Strings (`Constants.AUTH_INVALID_CREDENTIALS`)
|
||||
10. Custom Exceptions statt generischer `RuntimeException`/`Exception`
|
||||
|
||||
### 🟢 NICE-TO-HAVE (technische Schuld, abarbeiten nach Go-Live)
|
||||
|
||||
11. **Test-Coverage auf 60% steigern** — primär `cannamanage-domain` + `RetentionService` + `DsgvoService` + Parser
|
||||
12. Dead-Code-Eliminierung (ungenutzte Felder)
|
||||
13. Cookie-Security explizit in `application-production.properties`
|
||||
14. Multi-Device-Refresh-Tokens (separates Table statt einzelner Hash)
|
||||
|
||||
---
|
||||
|
||||
## 8. Was wirklich gut ist (Anerkennung)
|
||||
|
||||
- **GoBD-Append-Only-Ledger in [`FinanceService`](cannamanage-service/src/main/java/de/cannamanage/service/FinanceService.java:201)** — Voids als kompensierende Buchungen, nicht als Updates. Lehrbuch-konform.
|
||||
- **XXE-Härtung in [`Camt053Parser`](cannamanage-service/src/main/java/de/cannamanage/service/bankimport/Camt053Parser.java:69)** — `SUPPORT_DTD=false` + `IS_SUPPORTING_EXTERNAL_ENTITIES=false` + `IS_REPLACING_ENTITY_REFERENCES=false`. Vorbildlich.
|
||||
- **Spring Boot 4.0.6 + Snyk Override 2026-06-12** — 10 HIGH + 18 MEDIUM Findings proaktiv eliminiert.
|
||||
- **PaymentMatching Weighted Scoring** — 35/30/20/15-Verteilung mit Schwellwerten AUTO=90/SUGGEST=60 und Doppelzahlungs-Schutz. Sauberes Algorithmus-Design.
|
||||
- **Tenant-Filter über Hibernate `@Filter` + AspectJ** — funktional korrekt, performant, idiomatic.
|
||||
- **RFC 9457 Problem Details** — moderner Standard im [`GlobalExceptionHandler`](cannamanage-api/src/main/java/de/cannamanage/api/exception/GlobalExceptionHandler.java:28).
|
||||
- **63 Audit-Log-Calls** — solides Activity-Trail-Niveau.
|
||||
- **KCanG-konforme Anonymisierung statt Löschung** — wichtiger Compliance-Nuance, korrekt umgesetzt.
|
||||
|
||||
---
|
||||
|
||||
## 9. Final Verdict
|
||||
|
||||
**⚠️ CONDITIONAL PASS — Production-Freigabe nach Behebung der 4 BLOCKER (geschätzt 4–6 Stunden Entwicklungszeit).**
|
||||
|
||||
Das Codebase ist insgesamt überraschend reif für 10 Sprints. Die Architektur ist solide, der Compliance-Layer ist außergewöhnlich gut, und die Dependency-Hygiene wurde aktiv gepflegt. Die 4 Blocker sind alle in einem Sprint-Tag fixbar — keine architektonischen Probleme, sondern konkrete vergessene Sicherheitsschichten beim `documents`-Modul + ein vergessener Cleanup-Schritt der Sprint-9-Findings.
|
||||
|
||||
**Nach Fix der Blocker und Re-Verification:** ✅ FULL PASS empfohlen.
|
||||
|
||||
Empfehlung für die nächste Iteration: **Test-Coverage von 12% auf ≥ 60%** ist die wichtigste mittelfristige Investition. Für eine Cannabis-Club-Management-Software mit Finanz- und Behörden-Reporting ist das aktuelle Niveau für einen produktiven Betrieb mit Echtkunden zu niedrig.
|
||||
|
||||
---
|
||||
|
||||
**Reviewer-Signatur:** Lumen (Claude-Opus-4.7), BigMind Session `43f1d5c3-4805-42a6-8408-145784f6603e`, 2026-06-15 18:46 CEST
|
||||
Reference in New Issue
Block a user