From f9a87efb7a86558f6e20af9ba3b0bfdfd782c29b Mon Sep 17 00:00:00 2001 From: Patrick Plate Date: Thu, 18 Jun 2026 16:08:05 +0200 Subject: [PATCH] =?UTF-8?q?feat:=20Sprint=2013=20=E2=80=94=20Production=20?= =?UTF-8?q?Hardening=20(security=20fixes,=20CI=20gate,=20rate=20limiting,?= =?UTF-8?q?=20tests)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gitea/workflows/deploy.yml | 28 +- README.md | 138 ++--- cannamanage-api/pom.xml | 11 + .../api/controller/DocumentController.java | 10 +- .../api/security/LoginRateLimitFilter.java | 77 +++ .../api/security/SecurityConfig.java | 12 +- .../cannamanage/api/service/AuthService.java | 8 +- .../DocumentControllerSecurityTest.java | 164 +++++ .../security/LoginRateLimitFilterTest.java | 178 ++++++ .../api/security/SecurityConfigTest.java | 89 +++ .../api/service/AuthServiceTest.java | 194 ++++++ cannamanage-frontend/package.json | 7 +- .../cannamanage/service/DocumentService.java | 15 +- .../service/DocumentServiceTest.java | 77 +++ .../cannamanage-sprint13-analysis.md | 139 +++++ docs/sprint-13/cannamanage-sprint13-plan.md | 337 ++++++++++ .../cannamanage-sprint13-testplan.md | 585 ++++++++++++++++++ 17 files changed, 1962 insertions(+), 107 deletions(-) create mode 100644 cannamanage-api/src/main/java/de/cannamanage/api/security/LoginRateLimitFilter.java create mode 100644 cannamanage-api/src/test/java/de/cannamanage/api/controller/DocumentControllerSecurityTest.java create mode 100644 cannamanage-api/src/test/java/de/cannamanage/api/security/LoginRateLimitFilterTest.java create mode 100644 cannamanage-api/src/test/java/de/cannamanage/api/security/SecurityConfigTest.java create mode 100644 cannamanage-api/src/test/java/de/cannamanage/api/service/AuthServiceTest.java create mode 100644 docs/sprint-13/cannamanage-sprint13-analysis.md create mode 100644 docs/sprint-13/cannamanage-sprint13-plan.md create mode 100644 docs/sprint-13/cannamanage-sprint13-testplan.md diff --git a/.gitea/workflows/deploy.yml b/.gitea/workflows/deploy.yml index 194f1cc..fccf335 100644 --- a/.gitea/workflows/deploy.yml +++ b/.gitea/workflows/deploy.yml @@ -38,6 +38,19 @@ jobs: docker version --format 'docker {{.Server.Version}}' docker compose version + - name: Run backend tests + run: | + set -euo pipefail + mvn test --batch-mode -f pom.xml + + - name: Frontend type check + run: | + set -euo pipefail + cd cannamanage-frontend + corepack enable + pnpm install --frozen-lockfile + pnpm run lint + - name: Build images run: | set -euo pipefail @@ -66,11 +79,16 @@ jobs: - name: Verify frontend run: | - if wget -q -O /dev/null http://192.168.188.119:3000; then - echo "✅ Frontend responding on :3000" - else - echo "⚠️ Frontend not responding yet (may still be starting)" - fi + for i in $(seq 1 10); do + if wget -q -O /dev/null http://192.168.188.119:3000; then + echo "✅ Frontend responding on :3000" + exit 0 + fi + echo " attempt $i/10 — waiting 5s" + sleep 5 + done + echo "❌ Frontend did not respond" + exit 1 - name: Prune dangling images run: docker image prune -f || true diff --git a/README.md b/README.md index 28aca6b..a8d1e4c 100644 --- a/README.md +++ b/README.md @@ -1,111 +1,87 @@ # CannaManage -Multi-tenant cannabis club management platform for German **Anbauvereinigungen** (cultivation associations) under CanG §19. - -## Overview - -CannaManage handles member management, distribution tracking, and legal compliance for cannabis cultivation clubs in Germany. It enforces the strict quotas mandated by the Cannabis Act (CanG) — including monthly limits (50g adult / 30g under-21), daily limits (25g), and THC restrictions for minors. +Full-stack management platform for German cannabis cultivation associations (Anbauvereinigungen) under the CanG/KCanG regulatory framework. ## Tech Stack -| Component | Technology | -|-----------|-----------| -| Runtime | Java 21 (Temurin) | -| Framework | Spring Boot 4.0.6 | -| Security | Spring Security 7.0 + JWT (JJWT 0.12.6) | -| ORM | Hibernate 7 / JPA | -| Database | PostgreSQL (prod), H2 (test) | -| Migrations | Flyway 10 | -| API Docs | SpringDoc OpenAPI 2.8.6 | -| Build | Maven (multi-module) | -| Container | Docker Compose (Postgres + app) | +| Layer | Technology | +|-------|-----------| +| **Frontend** | Next.js 15, React 19, TypeScript, Tailwind CSS 4, shadcn/ui | +| **Backend** | Spring Boot 3.5, Java 17, Spring Security (JWT + session) | +| **Database** | PostgreSQL 16, Flyway migrations | +| **Infrastructure** | Docker Compose, Gitea Actions CI/CD, TrueNAS deployment | ## Project Structure ``` cannamanage/ -├── cannamanage-domain/ # JPA entities, enums, TenantContext -├── cannamanage-service/ # Business logic, repositories, ComplianceService -├── cannamanage-api/ # Spring Boot app, controllers, security, DTOs -├── docs/ -│ └── sprint-2/ # Sprint planning docs -└── docker-compose.yml # Local dev environment +├── cannamanage-api/ # Spring Boot REST API (entry point) +├── cannamanage-service/ # Business logic layer +├── cannamanage-domain/ # JPA entities, enums, value objects +├── cannamanage-frontend/ # Next.js frontend (pnpm) +├── deploy/ # Deployment scripts & nginx config +├── docker-compose.yml # Local development stack +└── .gitea/workflows/ # CI/CD pipeline ``` -## Modules +## Local Development -### cannamanage-domain -JPA entities with multi-tenant isolation via `@Filter("tenantFilter")`: -- `Member` — club members with age tracking -- `Distribution` — cannabis distribution records -- `MonthlyQuota` — per-member monthly usage tracking -- `Batch` / `Strain` / `StockMovement` — inventory management -- `Club` — association registration -- `User` — authentication accounts +### Prerequisites -### cannamanage-service -- `ComplianceService` — CanG §19 quota enforcement (25 unit tests) -- Repositories for all entities +- Java 17+ +- Maven 3.9+ +- Node.js 22+ with pnpm 10+ +- Docker & Docker Compose -### cannamanage-api -- **Auth** — JWT login + refresh token rotation (SHA-256 hashed) -- **Members** — CRUD for association members -- **Distributions** — compliance-gated distribution recording -- **Stock** — batch and inventory management -- **Compliance** — quota status API -- Multi-tenant isolation via `TenantFilterAspect` (Hibernate @Filter activation) - -## API Endpoints - -| Method | Path | Auth | Description | -|--------|------|------|-------------| -| POST | `/api/v1/auth/login` | Public | Login with email + password | -| POST | `/api/v1/auth/refresh` | Public | Refresh token rotation | -| GET | `/api/v1/compliance/quota/{memberId}` | ADMIN, MEMBER | Monthly quota status | -| GET/POST/PUT | `/api/v1/members/**` | ADMIN, MEMBER | Member CRUD | -| POST | `/api/v1/distributions/**` | ADMIN, MEMBER | Record distributions | -| GET/POST | `/api/v1/stock/**` | ADMIN | Stock management | - -Swagger UI: `http://localhost:8080/swagger-ui.html` - -## Running Locally +### Backend ```bash # Start PostgreSQL -docker compose up -d +docker compose up -d db -# Run the app -JAVA_HOME=/path/to/jdk-21 ./mvnw spring-boot:run -pl cannamanage-api - -# Run all tests (H2 in-memory) -JAVA_HOME=/path/to/jdk-21 ./mvnw clean verify +# Run Spring Boot +mvn spring-boot:run -f cannamanage-api/pom.xml -Dspring-boot.run.profiles=local ``` -## Testing +### Frontend -- **37 tests total** — all green -- 25 unit tests (`ComplianceServiceTest`) — quota enforcement logic -- 7 integration tests (`AuthControllerIntegrationTest`) — full HTTP auth flow -- 5 integration tests (`ComplianceControllerIntegrationTest`) — quota API with JWT +```bash +cd cannamanage-frontend +pnpm install +pnpm dev +``` -Integration tests use `@SpringBootTest(webEnvironment = RANDOM_PORT)` with H2 and Spring's `RestClient`. +The frontend runs on http://localhost:3000, backend on http://localhost:8080. -## Security Model +### Full Stack (Docker) -- **Stateless JWT** — no session, no UserDetailsService -- **Roles**: ADMIN (full access), MEMBER (self-service), STAFF (Sprint 3) -- **Multi-tenancy**: Hibernate `@Filter` activated per-request via AOP aspect -- **Refresh tokens**: SHA-256 hashed (Spring Security 7 enforces BCrypt 72-byte limit) -- Token rotation on refresh — old tokens invalidated +```bash +docker compose up --build +``` -## Sprint History +## Deployment -| Sprint | Focus | Status | -|--------|-------|--------| -| 1 | Domain entities, ComplianceService, 25 tests | ✅ Done | -| 2 | REST API, Spring Security, JWT, OpenAPI, integration tests | ✅ Done | -| 3 | Member portal, STAFF role, real-time notifications | 📋 Planned | +Push to `main` triggers the Gitea Actions CI pipeline which: +1. Runs backend tests (`mvn test`) +2. Runs frontend lint (`pnpm lint`) +3. Builds Docker images +4. Deploys to TrueNAS via Docker Compose +5. Verifies backend health + frontend availability + +Manual deploy: +```bash +cd deploy && ./deploy.sh +``` + +## Environment Variables + +| Variable | Purpose | Default | +|----------|---------|---------| +| `CANNAMANAGE_SECURITY_JWT_SECRET` | JWT signing key (base64, 256-bit) | — (required) | +| `CORS_ORIGINS` | Allowed CORS origins (comma-separated) | `http://localhost:3000` | +| `SMTP_HOST` / `SMTP_PORT` | Mail server for invites | `localhost:1025` | +| `SCHEDULERS_ENABLED` | Enable background jobs | `true` | ## License -Private — Patrick Plate +Proprietary — Patrick Plate diff --git a/cannamanage-api/pom.xml b/cannamanage-api/pom.xml index a8db88f..2cf2f84 100644 --- a/cannamanage-api/pom.xml +++ b/cannamanage-api/pom.xml @@ -140,6 +140,17 @@ org.springframework.boot spring-boot-starter-websocket + + + com.bucket4j + bucket4j-core + 8.10.1 + + + com.github.ben-manes.caffeine + caffeine + 3.1.8 + 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 453fc63..b9b9cf8 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 @@ -9,6 +9,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.web.bind.annotation.*; import org.springframework.web.multipart.MultipartFile; import org.springframework.web.server.ResponseStatusException; @@ -21,6 +22,7 @@ import java.util.UUID; @RestController @RequestMapping("/api/v1") +@PreAuthorize("hasAnyRole('ADMIN', 'STAFF', 'MEMBER')") public class DocumentController { private final DocumentService documentService; @@ -33,13 +35,14 @@ public class DocumentController { * 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. + * Returns 404 (not 403) to avoid revealing document existence to other tenants. */ 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 404 to prevent information leakage about document existence across tenants + throw new ResponseStatusException(HttpStatus.NOT_FOUND, "Document not found"); } return doc; } @@ -78,6 +81,7 @@ public class DocumentController { } @DeleteMapping("/documents/{id}") + @PreAuthorize("hasAnyRole('ADMIN', 'STAFF')") public ResponseEntity deleteDocument( @PathVariable UUID id, @RequestParam UUID clubId, @@ -87,7 +91,7 @@ public class DocumentController { Document doc = loadOwnedDocument(id); UUID currentTenantId = TenantContext.getCurrentTenant(); if (!clubId.equals(currentTenantId)) { - throw new ResponseStatusException(HttpStatus.FORBIDDEN, "Tenant mismatch"); + throw new ResponseStatusException(HttpStatus.NOT_FOUND, "Document not found"); } UUID userId = UUID.fromString(principal.getName()); documentService.deleteDocument(id, userId, doc.getClubId()); diff --git a/cannamanage-api/src/main/java/de/cannamanage/api/security/LoginRateLimitFilter.java b/cannamanage-api/src/main/java/de/cannamanage/api/security/LoginRateLimitFilter.java new file mode 100644 index 0000000..1fc9c97 --- /dev/null +++ b/cannamanage-api/src/main/java/de/cannamanage/api/security/LoginRateLimitFilter.java @@ -0,0 +1,77 @@ +package de.cannamanage.api.security; + +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import io.github.bucket4j.Bandwidth; +import io.github.bucket4j.Bucket; +import io.github.bucket4j.ConsumptionProbe; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.springframework.core.annotation.Order; +import org.springframework.stereotype.Component; +import org.springframework.web.filter.OncePerRequestFilter; + +import java.io.IOException; +import java.time.Duration; + +/** + * Rate-limits login attempts per client IP using Bucket4j + Caffeine cache. + * Allows 5 login attempts per minute per IP; returns 429 when exhausted. + */ +@Component +@Order(1) +public class LoginRateLimitFilter extends OncePerRequestFilter { + + private static final String LOGIN_PATH = "/api/v1/auth/login"; + private static final int CAPACITY = 5; + private static final Duration REFILL_PERIOD = Duration.ofMinutes(1); + + private final Cache buckets = Caffeine.newBuilder() + .maximumSize(10_000) + .expireAfterAccess(Duration.ofMinutes(10)) + .build(); + + @Override + protected void doFilterInternal(HttpServletRequest request, + HttpServletResponse response, + FilterChain filterChain) throws ServletException, IOException { + if (!"POST".equalsIgnoreCase(request.getMethod()) || !LOGIN_PATH.equals(request.getRequestURI())) { + filterChain.doFilter(request, response); + return; + } + + String clientIp = resolveClientIp(request); + Bucket bucket = buckets.get(clientIp, k -> createBucket()); + + ConsumptionProbe probe = bucket.tryConsumeAndReturnRemaining(1); + if (probe.isConsumed()) { + filterChain.doFilter(request, response); + } else { + long waitSeconds = probe.getNanosToWaitForRefill() / 1_000_000_000 + 1; + response.setStatus(429); + response.setHeader("Retry-After", String.valueOf(waitSeconds)); + response.setContentType("application/json"); + response.getWriter().write("{\"error\":\"Too many login attempts. Retry after " + waitSeconds + "s\"}"); + } + } + + private Bucket createBucket() { + return Bucket.builder() + .addLimit(Bandwidth.builder() + .capacity(CAPACITY) + .refillGreedy(CAPACITY, REFILL_PERIOD) + .build()) + .build(); + } + + private String resolveClientIp(HttpServletRequest request) { + String xff = request.getHeader("X-Forwarded-For"); + if (xff != null && !xff.isBlank()) { + // Take the first IP in the chain (original client) + return xff.split(",")[0].trim(); + } + return request.getRemoteAddr(); + } +} 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 6b02ec1..a5d338a 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 @@ -5,6 +5,7 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.annotation.Order; +import org.springframework.http.HttpMethod; import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; @@ -71,10 +72,13 @@ 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") + // Documents endpoint — method-specific matchers for defense-in-depth. + // POST (upload) and DELETE restricted to ADMIN/STAFF; GET allowed for all + // authenticated roles. Per-document tenant ownership is additionally + // enforced in DocumentController via TenantContext. + .requestMatchers(HttpMethod.GET, "/api/v1/documents/**").hasAnyRole("ADMIN", "STAFF", "MEMBER") + .requestMatchers(HttpMethod.POST, "/api/v1/documents/**").hasAnyRole("ADMIN", "STAFF") + .requestMatchers(HttpMethod.DELETE, "/api/v1/documents/**").hasAnyRole("ADMIN", "STAFF") .anyRequest().authenticated()) .addFilterBefore(jwtAuthFilter, UsernamePasswordAuthenticationFilter.class); diff --git a/cannamanage-api/src/main/java/de/cannamanage/api/service/AuthService.java b/cannamanage-api/src/main/java/de/cannamanage/api/service/AuthService.java index 428d352..995b931 100644 --- a/cannamanage-api/src/main/java/de/cannamanage/api/service/AuthService.java +++ b/cannamanage-api/src/main/java/de/cannamanage/api/service/AuthService.java @@ -34,6 +34,8 @@ import java.util.UUID; @RequiredArgsConstructor public class AuthService { + private static final String INVALID_CREDENTIALS = "Invalid credentials"; + private final UserRepository userRepository; private final JwtService jwtService; private final PasswordEncoder passwordEncoder; @@ -43,14 +45,14 @@ public class AuthService { @Transactional public LoginResponse login(LoginRequest request) { User user = userRepository.findByEmail(request.email()) - .orElseThrow(() -> new AuthenticationException("Invalid credentials")); + .orElseThrow(() -> new AuthenticationException(INVALID_CREDENTIALS)); if (!user.isActive()) { throw new AuthenticationException("Account not activated"); } if (!passwordEncoder.matches(request.password(), user.getPasswordHash())) { - throw new AuthenticationException("Invalid credentials"); + throw new AuthenticationException(INVALID_CREDENTIALS); } // Generate tokens @@ -147,7 +149,7 @@ public class AuthService { byte[] hash = digest.digest(input.getBytes(StandardCharsets.UTF_8)); return HexFormat.of().formatHex(hash); } catch (NoSuchAlgorithmException e) { - throw new RuntimeException("SHA-256 not available", e); + throw new IllegalStateException("SHA-256 not available", e); } } diff --git a/cannamanage-api/src/test/java/de/cannamanage/api/controller/DocumentControllerSecurityTest.java b/cannamanage-api/src/test/java/de/cannamanage/api/controller/DocumentControllerSecurityTest.java new file mode 100644 index 0000000..51c29f0 --- /dev/null +++ b/cannamanage-api/src/test/java/de/cannamanage/api/controller/DocumentControllerSecurityTest.java @@ -0,0 +1,164 @@ +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.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.web.server.ResponseStatusException; + +import java.io.IOException; +import java.security.Principal; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Security unit tests for {@link DocumentController}. + * Verifies tenant isolation (IDOR protection) at the controller layer. + */ +@ExtendWith(MockitoExtension.class) +class DocumentControllerSecurityTest { + + @Mock + private DocumentService documentService; + + @InjectMocks + private DocumentController documentController; + + private static final UUID CLUB_A = UUID.fromString("00000000-0000-0000-0000-00000000000a"); + private static final UUID CLUB_B = UUID.fromString("00000000-0000-0000-0000-00000000000b"); + private static final UUID DOC_ID = UUID.fromString("00000000-0000-0000-0000-000000000099"); + private static final UUID USER_ID = UUID.fromString("00000000-0000-0000-0000-000000000001"); + + @BeforeEach + void setUp() { + // Default tenant context: CLUB_A + TenantContext.setCurrentTenant(CLUB_A); + } + + @AfterEach + void tearDown() { + TenantContext.clear(); + } + + // --- T-09: Download wrong tenant → 404 --- + + @Test + @DisplayName("downloadDocument — wrong tenant throws 404 (IDOR protection)") + void testDownload_wrongTenant_returns404() { + // Document belongs to CLUB_B but user's tenant is CLUB_A + Document doc = new Document(); + doc.setId(DOC_ID); + doc.setClubId(CLUB_B); + doc.setFilename("secret.pdf"); + doc.setContentType("application/pdf"); + when(documentService.getDocument(DOC_ID)).thenReturn(doc); + + assertThatThrownBy(() -> documentController.downloadDocument(DOC_ID)) + .isInstanceOf(ResponseStatusException.class) + .satisfies(ex -> { + ResponseStatusException rse = (ResponseStatusException) ex; + assertThat(rse.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + }); + } + + // --- T-10: Download correct tenant → 200 --- + + @Test + @DisplayName("downloadDocument — correct tenant returns content") + void testDownload_correctTenant_succeeds() throws IOException { + Document doc = new Document(); + doc.setId(DOC_ID); + doc.setClubId(CLUB_A); + doc.setFilename("report.pdf"); + doc.setContentType("application/pdf"); + doc.setStoragePath(CLUB_A + "/" + DOC_ID + "_report.pdf"); + when(documentService.getDocument(DOC_ID)).thenReturn(doc); + when(documentService.downloadDocument(DOC_ID)).thenReturn("test content".getBytes()); + + ResponseEntity response = documentController.downloadDocument(DOC_ID); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(response.getBody()).isNotNull(); + } + + // --- T-11: Delete wrong tenant → 404 --- + + @Test + @DisplayName("deleteDocument — wrong tenant throws 404 (IDOR protection)") + void testDelete_wrongTenant_returns404() { + Document doc = new Document(); + doc.setId(DOC_ID); + doc.setClubId(CLUB_B); + doc.setTitle("Secret Doc"); + when(documentService.getDocument(DOC_ID)).thenReturn(doc); + + Principal principal = mock(Principal.class); + + assertThatThrownBy(() -> documentController.deleteDocument(DOC_ID, CLUB_A, principal)) + .isInstanceOf(ResponseStatusException.class) + .satisfies(ex -> { + ResponseStatusException rse = (ResponseStatusException) ex; + assertThat(rse.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + }); + } + + // --- T-12: Delete correct tenant → 204 --- + + @Test + @DisplayName("deleteDocument — correct tenant and matching clubId succeeds") + void testDelete_correctTenant_succeeds() throws IOException { + Document doc = new Document(); + doc.setId(DOC_ID); + doc.setClubId(CLUB_A); + doc.setTitle("My Doc"); + doc.setStoragePath(CLUB_A + "/" + DOC_ID + "_my.pdf"); + when(documentService.getDocument(DOC_ID)).thenReturn(doc); + + Principal principal = mock(Principal.class); + when(principal.getName()).thenReturn(USER_ID.toString()); + + ResponseEntity response = documentController.deleteDocument(DOC_ID, CLUB_A, principal); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NO_CONTENT); + } + + // --- T-13/T-14: Upload role restriction is handled by Spring Security @PreAuthorize, + // not testable in a pure unit test. Covered by SecurityConfigIntegrationTest. --- + + @Test + @DisplayName("deleteDocument — mismatched clubId param vs tenant throws 404") + void testDelete_mismatchedClubIdParam_returns404() { + // Document belongs to CLUB_A and tenant is CLUB_A, but clubId param is different + Document doc = new Document(); + doc.setId(DOC_ID); + doc.setClubId(CLUB_A); + doc.setTitle("Doc"); + when(documentService.getDocument(DOC_ID)).thenReturn(doc); + + Principal principal = mock(Principal.class); + + // Passing CLUB_B as the clubId param while tenant is CLUB_A + assertThatThrownBy(() -> documentController.deleteDocument(DOC_ID, CLUB_B, principal)) + .isInstanceOf(ResponseStatusException.class) + .satisfies(ex -> { + ResponseStatusException rse = (ResponseStatusException) ex; + assertThat(rse.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + }); + } +} diff --git a/cannamanage-api/src/test/java/de/cannamanage/api/security/LoginRateLimitFilterTest.java b/cannamanage-api/src/test/java/de/cannamanage/api/security/LoginRateLimitFilterTest.java new file mode 100644 index 0000000..7541f51 --- /dev/null +++ b/cannamanage-api/src/test/java/de/cannamanage/api/security/LoginRateLimitFilterTest.java @@ -0,0 +1,178 @@ +package de.cannamanage.api.security; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; + +import java.io.IOException; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.*; + +/** + * Unit tests for {@link LoginRateLimitFilter} covering rate limiting with Bucket4j + Caffeine. + * Tests per-IP bucket isolation, blocking after threshold, and Retry-After header. + */ +class LoginRateLimitFilterTest { + + private LoginRateLimitFilter filter; + private FilterChain filterChain; + + @BeforeEach + void setUp() { + filter = new LoginRateLimitFilter(); + filterChain = mock(FilterChain.class); + } + + // --- T-26: First 5 requests pass --- + + @Test + @DisplayName("Rate limit — first 5 requests from same IP are allowed") + void testRateLimit_allowsFirstFiveRequests() throws ServletException, IOException { + for (int i = 0; i < 5; i++) { + MockHttpServletRequest request = createLoginRequest("192.168.1.1"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + filter.doFilterInternal(request, response, filterChain); + + assertThat(response.getStatus()).isNotEqualTo(429); + } + + // FilterChain should have been invoked 5 times + verify(filterChain, times(5)).doFilter(any(), any()); + } + + // --- T-27: 6th request returns 429 --- + + @Test + @DisplayName("Rate limit — 6th request from same IP returns 429") + void testRateLimit_blocks6thRequest_returns429() throws ServletException, IOException { + String ip = "10.0.0.1"; + + // Exhaust the 5-request bucket + for (int i = 0; i < 5; i++) { + MockHttpServletRequest request = createLoginRequest(ip); + MockHttpServletResponse response = new MockHttpServletResponse(); + filter.doFilterInternal(request, response, filterChain); + } + + // 6th request should be rate-limited + MockHttpServletRequest request = createLoginRequest(ip); + MockHttpServletResponse response = new MockHttpServletResponse(); + filter.doFilterInternal(request, response, filterChain); + + assertThat(response.getStatus()).isEqualTo(429); + assertThat(response.getContentAsString()).contains("Too many login attempts"); + } + + // --- Retry-After header --- + + @Test + @DisplayName("Rate limit — 429 response includes Retry-After header") + void testRateLimit_includesRetryAfterHeader() throws ServletException, IOException { + String ip = "10.0.0.2"; + + // Exhaust the bucket + for (int i = 0; i < 5; i++) { + filter.doFilterInternal(createLoginRequest(ip), new MockHttpServletResponse(), filterChain); + } + + // 6th request — check headers + MockHttpServletResponse response = new MockHttpServletResponse(); + filter.doFilterInternal(createLoginRequest(ip), response, filterChain); + + assertThat(response.getStatus()).isEqualTo(429); + String retryAfter = response.getHeader("Retry-After"); + assertThat(retryAfter).isNotNull(); + assertThat(Integer.parseInt(retryAfter)).isGreaterThan(0); + } + + // --- T-28: Separate buckets per IP --- + + @Test + @DisplayName("Rate limit — different IPs have separate rate limit buckets") + void testRateLimit_separateBucketsPerIp() throws ServletException, IOException { + String ip1 = "192.168.1.100"; + String ip2 = "192.168.1.200"; + + // Exhaust quota for ip1 + for (int i = 0; i < 5; i++) { + filter.doFilterInternal(createLoginRequest(ip1), new MockHttpServletResponse(), filterChain); + } + + // ip1 should be blocked + MockHttpServletResponse responseIp1 = new MockHttpServletResponse(); + filter.doFilterInternal(createLoginRequest(ip1), responseIp1, filterChain); + assertThat(responseIp1.getStatus()).isEqualTo(429); + + // ip2 should still be allowed + MockHttpServletResponse responseIp2 = new MockHttpServletResponse(); + filter.doFilterInternal(createLoginRequest(ip2), responseIp2, filterChain); + assertThat(responseIp2.getStatus()).isNotEqualTo(429); + } + + // --- Non-login requests pass through --- + + @Test + @DisplayName("Non-login endpoint requests are not rate limited") + void testNonLoginEndpoint_notRateLimited() throws ServletException, IOException { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/v1/members"); + request.setRemoteAddr("10.0.0.5"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + filter.doFilterInternal(request, response, filterChain); + + verify(filterChain).doFilter(request, response); + assertThat(response.getStatus()).isNotEqualTo(429); + } + + @Test + @DisplayName("GET request to login path is not rate limited") + void testGetLoginPath_notRateLimited() throws ServletException, IOException { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/v1/auth/login"); + request.setRemoteAddr("10.0.0.6"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + filter.doFilterInternal(request, response, filterChain); + + verify(filterChain).doFilter(request, response); + assertThat(response.getStatus()).isNotEqualTo(429); + } + + // --- X-Forwarded-For header --- + + @Test + @DisplayName("Rate limit uses X-Forwarded-For header for client IP resolution") + void testRateLimit_usesXForwardedFor() throws ServletException, IOException { + String realIp = "203.0.113.50"; + + // Exhaust bucket via X-Forwarded-For IP + for (int i = 0; i < 5; i++) { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/api/v1/auth/login"); + request.setRemoteAddr("127.0.0.1"); // proxy IP + request.addHeader("X-Forwarded-For", realIp + ", 10.0.0.1"); + filter.doFilterInternal(request, new MockHttpServletResponse(), filterChain); + } + + // 6th request from same forwarded IP should be blocked + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/api/v1/auth/login"); + request.setRemoteAddr("127.0.0.1"); + request.addHeader("X-Forwarded-For", realIp + ", 10.0.0.1"); + MockHttpServletResponse response = new MockHttpServletResponse(); + filter.doFilterInternal(request, response, filterChain); + + assertThat(response.getStatus()).isEqualTo(429); + } + + // --- Helper --- + + private MockHttpServletRequest createLoginRequest(String ip) { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/api/v1/auth/login"); + request.setRemoteAddr(ip); + return request; + } +} diff --git a/cannamanage-api/src/test/java/de/cannamanage/api/security/SecurityConfigTest.java b/cannamanage-api/src/test/java/de/cannamanage/api/security/SecurityConfigTest.java new file mode 100644 index 0000000..2db515a --- /dev/null +++ b/cannamanage-api/src/test/java/de/cannamanage/api/security/SecurityConfigTest.java @@ -0,0 +1,89 @@ +package de.cannamanage.api.security; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.web.server.LocalServerPort; +import org.springframework.http.ResponseEntity; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.web.client.RestClient; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link SecurityConfig} — verifying that the security filter chain + * correctly requires authentication for protected endpoints and allows public endpoints. + * Uses RestClient against an actual HTTP server (same pattern as AuthControllerIntegrationTest). + * + * Note: The existing SecurityConfigIntegrationTest (Testcontainers) covers the same cases + * with a full database. This test uses the simpler "test" profile for faster execution. + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +@ActiveProfiles("test") +class SecurityConfigTest { + + @LocalServerPort + private int port; + + private RestClient restClient() { + return RestClient.builder() + .baseUrl("http://localhost:" + port) + .build(); + } + + // --- T-21: Document endpoints require authentication --- + + @Test + @DisplayName("GET /api/v1/documents — unauthenticated returns 401") + void testDocumentEndpoints_requireAuthentication() { + ResponseEntity response = restClient().get() + .uri("/api/v1/documents?clubId=00000000-0000-0000-0000-000000000001") + .retrieve() + .toEntity(String.class); + + assertThat(response.getStatusCode().value()).isEqualTo(401); + } + + @Test + @DisplayName("GET /api/v1/documents/{id}/download — unauthenticated returns 401") + void testDocumentDownload_requiresAuthentication() { + ResponseEntity response = restClient().get() + .uri("/api/v1/documents/00000000-0000-0000-0000-000000000099/download") + .retrieve() + .toEntity(String.class); + + assertThat(response.getStatusCode().value()).isEqualTo(401); + } + + // --- T-22: Auth endpoints are public --- + + @Test + @DisplayName("POST /api/v1/auth/login — accessible without authentication (not 401)") + void testAuthEndpoints_arePublic() { + ResponseEntity response = restClient().post() + .uri("/api/v1/auth/login") + .contentType(org.springframework.http.MediaType.APPLICATION_JSON) + .body("{\"email\":\"test@test.de\",\"password\":\"test\"}") + .retrieve() + .toEntity(String.class); + + // Auth endpoints are public — should NOT return 401/403 + // May return 400 or 500 (user not found), that's fine + assertThat(response.getStatusCode().value()).isNotEqualTo(401); + assertThat(response.getStatusCode().value()).isNotEqualTo(403); + } + + // --- T-23: Actuator health is public --- + + @Test + @DisplayName("GET /actuator/health — accessible without authentication") + void testActuatorHealth_isPublic() { + ResponseEntity response = restClient().get() + .uri("/actuator/health") + .retrieve() + .toEntity(String.class); + + assertThat(response.getStatusCode().value()).isEqualTo(200); + } +} diff --git a/cannamanage-api/src/test/java/de/cannamanage/api/service/AuthServiceTest.java b/cannamanage-api/src/test/java/de/cannamanage/api/service/AuthServiceTest.java new file mode 100644 index 0000000..5d850cc --- /dev/null +++ b/cannamanage-api/src/test/java/de/cannamanage/api/service/AuthServiceTest.java @@ -0,0 +1,194 @@ +package de.cannamanage.api.service; + +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.security.JwtService; +import de.cannamanage.domain.entity.User; +import de.cannamanage.domain.enums.UserRole; +import de.cannamanage.service.repository.InviteTokenRepository; +import de.cannamanage.service.repository.StaffAccountRepository; +import de.cannamanage.service.repository.UserRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.crypto.password.PasswordEncoder; + +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.util.HexFormat; +import java.util.Optional; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.when; + +/** + * Unit tests for {@link AuthService} covering login, token refresh, and SHA-256 hashing. + */ +@ExtendWith(MockitoExtension.class) +class AuthServiceTest { + + @Mock + private UserRepository userRepository; + + @Mock + private JwtService jwtService; + + @Mock + private PasswordEncoder passwordEncoder; + + @Mock + private InviteTokenRepository inviteTokenRepository; + + @Mock + private StaffAccountRepository staffAccountRepository; + + @InjectMocks + private AuthService authService; + + private User activeUser; + private static final UUID USER_ID = UUID.fromString("00000000-0000-0000-0000-000000000001"); + private static final UUID TENANT_ID = UUID.fromString("00000000-0000-0000-0000-000000000010"); + private static final String EMAIL = "admin@test.de"; + private static final String PASSWORD = "SecurePass123!"; + private static final String HASHED_PASSWORD = "$2a$10$hashedvalue"; + + @BeforeEach + void setUp() { + activeUser = new User(); + activeUser.setId(USER_ID); + activeUser.setEmail(EMAIL); + activeUser.setPasswordHash(HASHED_PASSWORD); + activeUser.setRole(UserRole.ROLE_ADMIN); + activeUser.setActive(true); + activeUser.setTenantId(TENANT_ID); + } + + // --- T-15: Login valid credentials → token pair --- + + @Test + @DisplayName("login — valid credentials returns token pair") + void testLogin_validCredentials_returnsTokenPair() { + when(userRepository.findByEmail(EMAIL)).thenReturn(Optional.of(activeUser)); + when(passwordEncoder.matches(PASSWORD, HASHED_PASSWORD)).thenReturn(true); + when(jwtService.generateAccessToken(any(), any(), anyString(), anyString())) + .thenReturn("access-token-123"); + when(jwtService.generateRefreshToken(any(), any())) + .thenReturn("refresh-token-456"); + when(userRepository.save(any(User.class))).thenReturn(activeUser); + + LoginResponse response = authService.login(new LoginRequest(EMAIL, PASSWORD)); + + assertThat(response).isNotNull(); + assertThat(response.accessToken()).isEqualTo("access-token-123"); + assertThat(response.refreshToken()).isEqualTo("refresh-token-456"); + assertThat(response.expiresIn()).isEqualTo(3600L); + assertThat(response.role()).isEqualTo("ADMIN"); + } + + // --- T-16: Login invalid password → 401 --- + + @Test + @DisplayName("login — invalid password throws AuthenticationException") + void testLogin_invalidPassword_throws401() { + when(userRepository.findByEmail(EMAIL)).thenReturn(Optional.of(activeUser)); + when(passwordEncoder.matches("wrong-password", HASHED_PASSWORD)).thenReturn(false); + + assertThatThrownBy(() -> authService.login(new LoginRequest(EMAIL, "wrong-password"))) + .isInstanceOf(AuthService.AuthenticationException.class) + .hasMessageContaining("Invalid credentials"); + } + + // --- T-17: Login non-existent user → 401 --- + + @Test + @DisplayName("login — non-existent user throws AuthenticationException") + void testLogin_nonExistentUser_throws401() { + when(userRepository.findByEmail("nobody@test.de")).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> authService.login(new LoginRequest("nobody@test.de", PASSWORD))) + .isInstanceOf(AuthService.AuthenticationException.class) + .hasMessageContaining("Invalid credentials"); + } + + // --- T-18: Refresh token valid → new access token --- + + @Test + @DisplayName("refresh — valid token returns new access token") + void testRefreshToken_validToken_returnsNewAccessToken() { + String oldRefreshToken = "valid-refresh-token"; + // Compute expected hash + String expectedHash = sha256(oldRefreshToken); + activeUser.setRefreshTokenHash(expectedHash); + + when(jwtService.isTokenValid(oldRefreshToken)).thenReturn(true); + when(jwtService.extractUserId(oldRefreshToken)).thenReturn(USER_ID); + when(userRepository.findById(USER_ID)).thenReturn(Optional.of(activeUser)); + when(jwtService.generateAccessToken(any(), any(), anyString(), anyString())) + .thenReturn("new-access-token"); + when(jwtService.generateRefreshToken(any(), any())) + .thenReturn("new-refresh-token"); + when(userRepository.save(any(User.class))).thenReturn(activeUser); + + LoginResponse response = authService.refresh(new RefreshRequest(oldRefreshToken)); + + assertThat(response).isNotNull(); + assertThat(response.accessToken()).isEqualTo("new-access-token"); + assertThat(response.refreshToken()).isEqualTo("new-refresh-token"); + } + + // --- T-19: Refresh token expired → 401 --- + + @Test + @DisplayName("refresh — expired/invalid token throws AuthenticationException") + void testRefreshToken_expired_throws401() { + String expiredToken = "expired-refresh-token"; + when(jwtService.isTokenValid(expiredToken)).thenReturn(false); + + assertThatThrownBy(() -> authService.refresh(new RefreshRequest(expiredToken))) + .isInstanceOf(AuthService.AuthenticationException.class) + .hasMessageContaining("Invalid or expired refresh token"); + } + + // --- T-20: SHA-256 hashing is deterministic --- + + @Test + @DisplayName("SHA-256 hashing is deterministic — same input always produces same hash") + void testSha256_deterministic() { + String input = "test-refresh-token-abc123"; + String hash1 = sha256(input); + String hash2 = sha256(input); + + assertThat(hash1).isEqualTo(hash2); + assertThat(hash1).hasSize(64); // SHA-256 produces 64 hex chars + assertThat(hash1).matches("[0-9a-f]{64}"); + } + + @Test + @DisplayName("SHA-256 hashing — different inputs produce different hashes") + void testSha256_differentInputs_differentHashes() { + String hash1 = sha256("token-one"); + String hash2 = sha256("token-two"); + + assertThat(hash1).isNotEqualTo(hash2); + } + + // Helper to replicate AuthService's sha256 logic for test verification + private String sha256(String input) { + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + byte[] hash = digest.digest(input.getBytes(StandardCharsets.UTF_8)); + return HexFormat.of().formatHex(hash); + } catch (Exception e) { + throw new IllegalStateException("SHA-256 not available", e); + } + } +} diff --git a/cannamanage-frontend/package.json b/cannamanage-frontend/package.json index 998ddbf..ce5a7e1 100644 --- a/cannamanage-frontend/package.json +++ b/cannamanage-frontend/package.json @@ -1,11 +1,11 @@ { - "name": "shadboard-nextjs-starter-kit", + "name": "cannamanage-frontend", "version": "1.0.0", "license": "MIT", "private": true, "author": { - "name": "Layth Alqadhi", - "url": "https://github.com/LaythAlqadhi" + "name": "Patrick Plate", + "url": "https://github.com/pplate" }, "scripts": { "dev": "next dev --turbopack", @@ -13,6 +13,7 @@ "start": "next start", "lint": "next lint", "lint:fix": "next lint --fix", + "type-check": "tsc --noEmit", "format": "prettier --ignore-path .gitignore --write .", "test": "vitest", "test:run": "vitest run", 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 27b8904..68faf42 100644 --- a/cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java +++ b/cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java @@ -5,6 +5,7 @@ import de.cannamanage.domain.enums.AuditEventType; import de.cannamanage.domain.enums.DocumentAccessLevel; import de.cannamanage.domain.enums.DocumentCategory; import de.cannamanage.service.repository.DocumentRepository; +import org.apache.commons.io.FilenameUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; @@ -198,13 +199,11 @@ public class DocumentService { 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(); + // Strip path components using commons-io — handles both Unix and Windows separators + // regardless of the current platform (unlike Paths.get which is platform-dependent) + String name = FilenameUtils.getName(original); + if (name == null || name.isBlank()) { + return "document"; } // Remove control characters and path-/shell-/Windows-reserved characters name = name.replaceAll("[\\x00-\\x1F\\x7F/\\\\:*?\"<>|]", "_"); @@ -214,7 +213,7 @@ public class DocumentService { } // Ensure not empty after sanitization if (name.isBlank() || ".".equals(name) || "..".equals(name)) { - return UUID.randomUUID().toString(); + return "document"; } return name; } diff --git a/cannamanage-service/src/test/java/de/cannamanage/service/DocumentServiceTest.java b/cannamanage-service/src/test/java/de/cannamanage/service/DocumentServiceTest.java index 203bb99..d5cc5dc 100644 --- a/cannamanage-service/src/test/java/de/cannamanage/service/DocumentServiceTest.java +++ b/cannamanage-service/src/test/java/de/cannamanage/service/DocumentServiceTest.java @@ -267,6 +267,83 @@ class DocumentServiceTest { } } + // --- Additional security tests for Sprint 13 --- + + @Test + void testUploadDocument_sanitizesPathTraversal_toBasename() throws IOException { + // Verify that "../../etc/passwd" is stripped to just "passwd" + MultipartFile file = mockValidFile("../../etc/passwd", "application/pdf", 512); + when(documentRepository.save(any(Document.class))).thenAnswer(inv -> inv.getArgument(0)); + + try (MockedStatic filesMock = mockStatic(Files.class)) { + filesMock.when(() -> Files.createDirectories(any(Path.class))).thenReturn(null); + filesMock.when(() -> Files.write(any(Path.class), any(byte[].class))).thenReturn(null); + + Document result = documentService.uploadDocument( + clubId, "Path Traversal Test", DocumentCategory.SONSTIGES, + DocumentAccessLevel.ALL_MEMBERS, null, file, uploadedBy); + + assertThat(result.getFilename()).isEqualTo("passwd"); + } + } + + @Test + void testUploadDocument_nullFilename_usesFallback() throws IOException { + MultipartFile file = mockValidFile(null, "application/pdf", 512); + when(documentRepository.save(any(Document.class))).thenAnswer(inv -> inv.getArgument(0)); + + try (MockedStatic filesMock = mockStatic(Files.class)) { + filesMock.when(() -> Files.createDirectories(any(Path.class))).thenReturn(null); + filesMock.when(() -> Files.write(any(Path.class), any(byte[].class))).thenReturn(null); + + Document result = documentService.uploadDocument( + clubId, "Null Filename", DocumentCategory.SONSTIGES, + DocumentAccessLevel.ALL_MEMBERS, null, file, uploadedBy); + + // Null filename should produce a non-blank fallback (UUID) + assertThat(result.getFilename()).isNotBlank(); + assertThat(result.getFilename()).doesNotContain(".."); + assertThat(result.getFilename()).doesNotContain("/"); + } + } + + @Test + void testUploadDocument_normalFilename_preserved() throws IOException { + MultipartFile file = mockValidFile("report.pdf", "application/pdf", 1024); + when(documentRepository.save(any(Document.class))).thenAnswer(inv -> inv.getArgument(0)); + + try (MockedStatic filesMock = mockStatic(Files.class)) { + filesMock.when(() -> Files.createDirectories(any(Path.class))).thenReturn(null); + filesMock.when(() -> Files.write(any(Path.class), any(byte[].class))).thenReturn(null); + + Document result = documentService.uploadDocument( + clubId, "Normal File", DocumentCategory.PROTOKOLL, + DocumentAccessLevel.ALL_MEMBERS, null, file, uploadedBy); + + assertThat(result.getFilename()).isEqualTo("report.pdf"); + } + } + + @Test + void testDownloadDocument_documentNotFound_throwsException() { + UUID nonExistentId = UUID.randomUUID(); + when(documentRepository.findById(nonExistentId)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> documentService.downloadDocument(nonExistentId)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("not found"); + } + + @Test + void testDeleteDocument_documentNotFound_throwsException() { + UUID nonExistentId = UUID.randomUUID(); + when(documentRepository.findById(nonExistentId)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> documentService.deleteDocument(nonExistentId, uploadedBy, clubId)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("not found"); + } + // --- Helpers --- private MultipartFile mockValidFile(String filename, String contentType, long size) { diff --git a/docs/sprint-13/cannamanage-sprint13-analysis.md b/docs/sprint-13/cannamanage-sprint13-analysis.md new file mode 100644 index 0000000..40718bf --- /dev/null +++ b/docs/sprint-13/cannamanage-sprint13-analysis.md @@ -0,0 +1,139 @@ +# Analysis: Sprint 13 — Production Hardening + +**Date:** 2026-06-18 +**Author:** Patrick Plate / Lumen (Planner) +**Status:** v1 +**Sprint Theme:** Production Hardening & Housekeeping + +--- + +## 1. Problem Analysis + +CannaManage has completed 12 sprints of feature development and is functionally complete for MVP. However, a comprehensive security review (2026-06-15) identified **4 production-blocking vulnerabilities** that prevent deployment. Additionally, backend test coverage sits at ~12% (20 tests for 29K LOC), the CI/CD pipeline deploys without running tests, and various repo hygiene issues remain unaddressed. + +Sprint 13 is a **hardening sprint** — no new features, purely focused on making the existing codebase production-ready. + +### Source Documents + +- [`docs/security-code-review-final.md`](docs/security-code-review-final.md) — Full security review with 4 BLOCKERs +- [`docs/sprint-12/SPRINT-12-SUMMARY.md`](docs/sprint-12/SPRINT-12-SUMMARY.md) — Sprint 12 outcome (test infra delivered) +- [`.gitea/workflows/deploy.yml`](.gitea/workflows/deploy.yml) — Current CI/CD pipeline (no tests) + +--- + +## 2. Affected Components + +| Component | Path | Issue | +|-----------|------|-------| +| DocumentController | `cannamanage-api/src/main/java/de/cannamanage/api/controller/DocumentController.java` | IDOR — no tenant check on download/delete | +| DocumentService | `cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java` | Path traversal via unsanitized filename | +| SecurityConfig | `cannamanage-api/src/main/java/de/cannamanage/api/security/SecurityConfig.java` | Missing `/api/v1/documents/**` matchers, CORS hardcoded | +| deploy.yml | `.gitea/workflows/deploy.yml` | Deploys without running tests | +| package.json | `cannamanage-frontend/package.json` | Wrong project name ("shadboard-nextjs-starter-kit") | +| cannamanage-domain | `cannamanage-domain/src/` | 0 unit tests | +| cannamanage-service | `cannamanage-service/src/` | Low test coverage on service layer | + +--- + +## 3. Current State (Ist-Zustand) + +### Security Posture + +The security review gave a **CONDITIONAL PASS** — architecture is solid (multi-tenant via `AbstractTenantEntity`, BCrypt+SHA-256, RFC 9457 errors, GoBD append-only audit) but 4 specific issues block go-live: + +| # | Blocker | Severity | Status Since | +|---|---------|----------|-------------| +| 1 | IDOR in DocumentController (download by raw UUID, no tenant verify) | HIGH | Sprint 9 (unfixed) | +| 2 | Path traversal in DocumentService (`file.getOriginalFilename()` unsanitized) | HIGH | Sprint 9 (unfixed) | +| 3 | JWT dev-secret fallback | HIGH | **FIXED** — `application.properties` now uses fail-on-startup marker | +| 4 | `/api/v1/documents/**` missing from SecurityConfig matchers | HIGH | Sprint 9 (unfixed) | + +**Note:** Blocker #3 is already resolved. The current `application.properties:13` reads: +``` +cannamanage.security.jwt.secret=${CANNAMANAGE_SECURITY_JWT_SECRET:CHANGE_ME_IN_PRODUCTION_THIS_WILL_FAIL_ON_STARTUP} +``` +The `JwtService.validateSecret()` detects this marker and refuses startup. This leaves **3 active blockers**. + +### CI/CD Pipeline + +The current [`.gitea/workflows/deploy.yml`](.gitea/workflows/deploy.yml:17) triggers on push to `main` and: +1. ✅ Checks out the commit +2. ✅ Builds Docker images +3. ✅ Deploys with `docker compose up -d` +4. ✅ Checks backend health (actuator) +5. ⚠️ Frontend check is non-blocking (doesn't fail the job) +6. ❌ **No tests run at all** — neither backend (Maven) nor frontend (Vitest/Playwright) + +### Test Coverage + +- **Backend:** ~20 tests across cannamanage-api (GlobalExceptionHandlerTest, some controller tests). cannamanage-domain and cannamanage-service have minimal or zero coverage. +- **Frontend:** Playwright integration specs (70+ tests) exist but are never run in CI. No Vitest unit tests in CI either. +- **Sprint 12** delivered the Docker Compose test infrastructure (`docker-compose.test.yml`) — it's ready to be wired into CI. + +### Repo Hygiene + +| Issue | Location | Impact | +|-------|----------|--------| +| Wrong project name | `cannamanage-frontend/package.json` → `"name": "shadboard-nextjs-starter-kit"` | Confusing, unprofessional | +| Dead `.github/` folder | `.github/modernize/` | GitHub-specific, project uses Gitea | +| No root README | `.` | No project documentation for new contributors | +| Leftover screenshot scripts | `cannamanage-frontend/*.mjs` | Dev clutter (gitignored PNGs, but scripts committed) | +| SonarQube findings | 7 MAJOR/MINOR issues | Dead fields, generic exceptions, string duplication | + +--- + +## 4. Risk Assessment + +| Risk | Probability | Impact | Mitigation | +|------|-------------|--------|------------| +| Document data leak via IDOR | High (any authenticated user) | Critical (DSGVO breach, multi-tenant violation) | Fix #1: Add tenant verification to DocumentController | +| Arbitrary file write via path traversal | Medium (requires auth + upload permission) | High (server compromise) | Fix #2: Sanitize with `FilenameUtils.getName()` | +| Broken deployment from untested code | Medium (no test gate) | High (production outage) | Add test step to deploy.yml | +| Low test confidence for future changes | Ongoing | Medium (regression risk) | Expand backend test suite | + +--- + +## 5. Solution Options + +### Option A: Minimal Security Fix Only (2-3 hours) +- Fix 3 remaining security blockers +- No CI/CD changes, no test expansion, no cleanup +- **Pro:** Fastest path to unblock deployment +- **Con:** Leaves test debt and CI risk intact; next feature sprint can introduce regressions + +### Option B: Security + CI Quality Gate (4-6 hours) +- Fix 3 security blockers +- Add Maven test + Playwright test steps to CI +- Basic repo cleanup (package.json, README) +- **Pro:** Production-safe deployment pipeline, reasonable effort +- **Con:** Test coverage still low for service/domain layer + +### Option C: Full Hardening Sprint (8-12 hours) ⬅️ RECOMMENDED +- Fix 3 security blockers +- Expand backend test suite (target: DocumentService, AuthService, key service methods) +- Wire tests into CI/CD pipeline (fail-fast on test failure) +- CORS configuration externalization +- Login rate limiting +- Complete repo cleanup (README, package.json, dead files, SonarQube fixes) +- **Pro:** Production-ready with confidence, clean repo, future-proof +- **Con:** Full sprint investment (no new features) + +--- + +## 6. Recommendation + +**Option C** — This is the right time for a full hardening sprint. The project is feature-complete for MVP, Sprint 12 already built the test infrastructure, and the security blockers have been unfixed since Sprint 9. A half-measure (Option A/B) would leave known technical debt that compounds with every future sprint. + +Priority ordering: +1. **Security fixes** (unblocks production) — ~2 hours +2. **CI test gate** (prevents future regressions) — ~2 hours +3. **Backend test expansion** (confidence for the security fixes themselves) — ~4 hours +4. **Repo cleanup + CORS + rate limiting** (polish) — ~2 hours + +--- + +## 7. Open Questions + +- [ ] Should CORS allowed origins come from `application.properties` or environment variables? +- [ ] Login rate limiting: Bucket4j (Spring-native) or custom filter with in-memory counter? +- [ ] Target test coverage % for this sprint? (Suggest: cover all security-critical paths = DocumentService, AuthService, TenantFilterAspect) diff --git a/docs/sprint-13/cannamanage-sprint13-plan.md b/docs/sprint-13/cannamanage-sprint13-plan.md new file mode 100644 index 0000000..97681da --- /dev/null +++ b/docs/sprint-13/cannamanage-sprint13-plan.md @@ -0,0 +1,337 @@ +# Plan: Sprint 13 — Production Hardening + +**Date:** 2026-06-18 +**Author:** Patrick Plate / Lumen (Planner) +**Status:** v2 (panel review incorporated) +**Basis:** cannamanage-sprint13-analysis.md + +--- + +## Background + +Sprint 13 addresses 3 remaining production-blocking security vulnerabilities (1 was already fixed), wires the existing test infrastructure into CI/CD as a quality gate, expands backend test coverage for security-critical paths, and performs repo cleanup. No new features — pure hardening. + +--- + +## Architecture + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ Security Hardening Layer │ +├─────────────────────────────────────────────────────────────────┤ +│ │ +│ SecurityConfig ──► Role matchers for /api/v1/documents/** │ +│ │ +│ DocumentController ──► @PreAuthorize + tenant verification │ +│ │ +│ DocumentService ──► FilenameUtils.getName() sanitization │ +│ ──► Explicit clubId check on download/delete │ +│ │ +├─────────────────────────────────────────────────────────────────┤ +│ CI/CD Quality Gate │ +├─────────────────────────────────────────────────────────────────┤ +│ │ +│ deploy.yml ──► [Checkout] → [Test Backend] → [Test Frontend] │ +│ ──► [Build Images] → [Deploy] → [Health Check] │ +│ │ +├─────────────────────────────────────────────────────────────────┤ +│ Operational Hardening │ +├─────────────────────────────────────────────────────────────────┤ +│ │ +│ CORS ──► Externalized via application.properties │ +│ Rate Limiting ──► Bucket4j on /api/v1/auth/login │ +│ │ +└─────────────────────────────────────────────────────────────────┘ +``` + +--- + +## Components + +| # | Component | Module | Action | +|---|-----------|--------|--------| +| 1 | DocumentController | cannamanage-api | Add `@PreAuthorize`, tenant verification | +| 2 | DocumentService | cannamanage-service | Sanitize filename, add clubId check | +| 3 | SecurityConfig | cannamanage-api | Add document endpoint matchers | +| 4 | deploy.yml | .gitea/workflows | Add test steps before deployment | +| 5 | DocumentServiceTest | cannamanage-service | New — comprehensive test class | +| 6 | DocumentControllerTest | cannamanage-api | New — security-focused integration tests | +| 7 | AuthServiceTest | cannamanage-api | New — auth flow tests | +| 8 | SecurityConfig | cannamanage-api | Externalize CORS origins | +| 9 | RateLimitFilter | cannamanage-api | New — login rate limiting | +| 10 | package.json | cannamanage-frontend | Fix project name | +| 11 | README.md | root | New — project documentation | + +--- + +## Implementation Steps + +### Phase 1: Security Fixes (Priority: CRITICAL) + +#### Step 1.1 — Fix IDOR in DocumentController + +**File:** `cannamanage-api/src/main/java/de/cannamanage/api/controller/DocumentController.java` + +- Add `@PreAuthorize("hasAnyRole('ADMIN', 'STAFF', 'MEMBER')")` on class level +- Inject `SecurityContextHolder` to extract current user's `clubId` +- On `downloadDocument(UUID id)`: after fetching the document, verify `document.getClubId().equals(currentUser.getClubId())` +- On `deleteDocument(UUID id)`: same tenant check + require `ADMIN` or `STAFF` role +- Return `404 Not Found` if tenant mismatch — prevents object enumeration. An attacker should not be able to determine whether a document UUID exists in another tenant. + +**Prerequisite:** Verify that portal JWT tokens include the `clubId` claim. If portal tokens lack `clubId`, portal document downloads will 403. Check `PortalAuthService` / `JwtService` token generation to confirm the claim is present before implementing tenant verification. + +#### Step 1.2 — Fix Path Traversal in DocumentService + +**File:** `cannamanage-service/src/main/java/de/cannamanage/service/DocumentService.java` + +- Replace `file.getOriginalFilename()` with `FilenameUtils.getName(file.getOriginalFilename())` +- Add null check: if result is blank, use `"document"` as fallback +- Add dependency on `commons-io` if not already present (it is — used by BankImportService) +- Pattern: consistent with existing [`BankImportService`](cannamanage-service/src/main/java/de/cannamanage/service/bankimport/BankImportService.java) which already does this correctly + +#### Step 1.3 — Add Document Endpoint Matchers to SecurityConfig + +**File:** `cannamanage-api/src/main/java/de/cannamanage/api/security/SecurityConfig.java` + +Add explicit matchers in `apiSecurityFilterChain()`: +```java +.requestMatchers(HttpMethod.GET, "/api/v1/documents/**").hasAnyRole("ADMIN", "STAFF", "MEMBER") +.requestMatchers(HttpMethod.POST, "/api/v1/documents/**").hasAnyRole("ADMIN", "STAFF") +.requestMatchers(HttpMethod.DELETE, "/api/v1/documents/**").hasAnyRole("ADMIN", "STAFF") +``` + +Place these BEFORE the `.anyRequest().authenticated()` catch-all. + +--- + +### Phase 2: CI/CD Quality Gate (Priority: HIGH) + +#### Step 2.1 — Add Backend Test Step to deploy.yml + +**File:** `.gitea/workflows/deploy.yml` + +Insert a new step after checkout, before Docker build: +```yaml +- name: Run backend tests + run: | + set -euo pipefail + cd cannamanage-api + mvn test --batch-mode -f pom.xml +``` + +**Note:** Maven is available in the runner image. The Maven Wrapper (`mvnw`) is not committed to this repo. + +This runs all backend tests. If any test fails, the deploy is aborted. + +#### Step 2.2 — Add Frontend Lint/Type-Check Step + +**Prerequisite:** Add `"type-check": "tsc --noEmit"` to `cannamanage-frontend/package.json` scripts. + +Insert after backend tests: +```yaml +- name: Frontend type check + run: | + set -euo pipefail + cd cannamanage-frontend + corepack enable + pnpm install --frozen-lockfile + pnpm run lint + pnpm run type-check +``` + +**Note:** Full Playwright integration tests are too heavy for every push (require Docker-in-Docker). Keep those for manual/nightly runs via `docker-compose.test.yml`. + +#### Step 2.3 — Make Frontend Health Check Blocking + +**File:** `.gitea/workflows/deploy.yml` + +Change the frontend verify step to `exit 1` on failure instead of just logging a warning. + +--- + +### Phase 3: Backend Test Expansion (Priority: HIGH) + +#### Step 3.1 — DocumentServiceTest (new) + +**File:** `cannamanage-service/src/test/java/de/cannamanage/service/DocumentServiceTest.java` + +Test cases: +- `testUploadDocument_sanitizesFilename()` — verify path traversal attempt is neutralized +- `testUploadDocument_nullFilename_usesFallback()` — null filename → "document" +- `testUploadDocument_validFilename_preserved()` — normal filename passes through +- `testDownloadDocument_wrongTenant_throwsForbidden()` — tenant isolation +- `testDownloadDocument_correctTenant_returnsContent()` — happy path +- `testDeleteDocument_wrongTenant_throwsForbidden()` — tenant isolation on delete +- `testDeleteDocument_adminRole_succeeds()` — admin can delete + +#### Step 3.2 — DocumentControllerSecurityTest (new) + +**File:** `cannamanage-api/src/test/java/de/cannamanage/api/controller/DocumentControllerSecurityTest.java` + +Integration tests using `@WebMvcTest` + `@WithMockUser`: +- `testDownload_unauthenticated_returns401()` +- `testDownload_wrongTenant_returns403()` +- `testDownload_correctTenant_returns200()` +- `testDelete_memberRole_returns403()` — MEMBER cannot delete +- `testDelete_staffRole_returns200()` — STAFF can delete +- `testUpload_memberRole_returns403()` — MEMBER cannot upload +- `testUpload_staffRole_returns200()` + +#### Step 3.3 — AuthServiceTest (new) + +**File:** `cannamanage-api/src/test/java/de/cannamanage/api/service/AuthServiceTest.java` + +- `testLogin_validCredentials_returnsTokenPair()` +- `testLogin_invalidPassword_throws401()` +- `testLogin_nonExistentUser_throws401()` +- `testRefreshToken_validToken_returnsNewAccess()` +- `testRefreshToken_expired_throws401()` +- `testSha256_consistent()` — hashing determinism + +#### Step 3.4 — SecurityConfigTest (new) + +**File:** `cannamanage-api/src/test/java/de/cannamanage/api/security/SecurityConfigTest.java` + +Verify URL pattern matching: +- `testDocumentEndpoints_requireAuthentication()` +- `testAuthEndpoints_arePublic()` +- `testActuatorHealth_isPublic()` + +--- + +### Phase 4: Operational Hardening (Priority: MEDIUM) + +#### Step 4.1 — Externalize CORS Configuration + +**File:** `cannamanage-api/src/main/resources/application.properties` + +Add: +```properties +cannamanage.cors.allowed-origins=${CORS_ALLOWED_ORIGINS:http://localhost:3000} +``` + +**File:** `cannamanage-api/src/main/java/de/cannamanage/api/security/SecurityConfig.java` + +Replace hardcoded origins with `@Value("${cannamanage.cors.allowed-origins}")` and split on comma. + +#### Step 4.2 — Login Rate Limiting + +Add Bucket4j dependency to `cannamanage-api/pom.xml`: +```xml + + com.bucket4j + bucket4j-core + 8.10.1 + +``` + +Add Caffeine dependency to `cannamanage-api/pom.xml`: +```xml + + com.github.ben-manes.caffeine + caffeine + 3.1.8 + +``` + +Create `cannamanage-api/src/main/java/de/cannamanage/api/security/LoginRateLimitFilter.java`: +- Use Caffeine cache (TTL-based eviction) instead of raw ConcurrentHashMap +- Key: IP address, Value: Bucket +- Max entries: 10,000 +- TTL: 10 minutes (auto-evicts stale entries, prevents memory leak under DDoS) +- Limit: 5 attempts per minute per IP +- Applies only to `POST /api/v1/auth/login` +- Returns `429 Too Many Requests` with `Retry-After` header when exceeded +- Register in SecurityConfig filter chain + +--- + +### Phase 5: Repo Cleanup (Priority: LOW) + +#### Step 5.1 — Fix package.json Project Name + +**File:** `cannamanage-frontend/package.json` + +Change `"name": "shadboard-nextjs-starter-kit"` → `"name": "cannamanage-frontend"` + +#### Step 5.2 — Remove Dead .github Directory + +```bash +rm -rf .github/ +``` + +Only if `.github/` contains GitHub-specific config (Actions, Dependabot) that doesn't apply to Gitea. + +#### Step 5.3 — Create Root README + +**File:** `README.md` + +Minimal project README: what it is, how to run locally, how to deploy, architecture overview. + +#### Step 5.4 — Clean Up Leftover Screenshot Scripts + +Remove one-shot `.mjs` scripts from `cannamanage-frontend/`: +- `upload-dialog-screenshot.mjs` +- `sprint12-final.mjs` +- `sprint12-v2.mjs` + +These were development utilities, not part of the test suite. + +#### Step 5.5 — Fix SonarQube Findings + +Address 7 MAJOR/MINOR findings from the security review: +- Remove unused `auditService` field in DocumentService (or wire it up for audit logging) +- Replace generic `throw new Exception()` in Camt053Parser with specific exception +- Replace generic `RuntimeException` in AuthService.sha256() with custom exception +- Extract duplicated `"Invalid credentials"` string to constant +- Fix static access via instance in Camt053Parser + +--- + +## Dependency Order + +``` +Step 1.1 ──┐ +Step 1.2 ──┼──► Step 3.1 (tests verify the fixes) +Step 1.3 ──┘ │ + ▼ + Step 3.2 (controller security tests) + │ + ▼ + Step 2.1 (CI runs these tests) + Step 2.2 + Step 2.3 + │ + ▼ + Step 4.1, 4.2 (operational hardening) + Step 5.1–5.5 (cleanup, parallel) +``` + +--- + +## Acceptance Criteria + +1. ✅ No authenticated user can download/delete documents from another tenant +2. ✅ Path traversal filenames are sanitized before storage +3. ✅ `/api/v1/documents/**` has explicit role matchers in SecurityConfig +4. ✅ CI pipeline runs backend tests before deployment (fails on test failure) +5. ✅ CI pipeline runs frontend lint + type-check before deployment +6. ✅ At least 15 new backend tests covering security-critical paths +7. ✅ CORS origins configurable via environment variable +8. ✅ Login endpoint rate-limited (5 attempts/min/IP) +9. ✅ `package.json` has correct project name +10. ✅ Root README exists + +--- + +## Estimated Effort + +| Phase | Effort | Cumulative | +|-------|--------|------------| +| Phase 1: Security Fixes | ~2h | 2h | +| Phase 2: CI Quality Gate | ~1.5h | 3.5h | +| Phase 3: Backend Tests | ~3h | 6.5h | +| Phase 4: Operational Hardening | ~2h | 8.5h | +| Phase 5: Repo Cleanup | ~1h | 9.5h | + +**Total: ~9.5 hours** (full sprint day) diff --git a/docs/sprint-13/cannamanage-sprint13-testplan.md b/docs/sprint-13/cannamanage-sprint13-testplan.md new file mode 100644 index 0000000..7e8aed3 --- /dev/null +++ b/docs/sprint-13/cannamanage-sprint13-testplan.md @@ -0,0 +1,585 @@ +# Testplan: Sprint 13 — Production Hardening + +**Date:** 2026-06-18 +**Author:** Patrick Plate / Lumen (Planner) +**Status:** v1 +**Basis:** cannamanage-sprint13-plan.md + +--- + +## Test Overview + +| ID | Description | Type | Class | Status | +|----|-------------|------|-------|--------| +| T-01 | Path traversal filename sanitization | Unit | `DocumentServiceTest` | ⬜ | +| T-02 | Null filename fallback | Unit | `DocumentServiceTest` | ⬜ | +| T-03 | Valid filename preserved | Unit | `DocumentServiceTest` | ⬜ | +| T-04 | Download wrong tenant — forbidden | Unit | `DocumentServiceTest` | ⬜ | +| T-05 | Download correct tenant — success | Unit | `DocumentServiceTest` | ⬜ | +| T-06 | Delete wrong tenant — forbidden | Unit | `DocumentServiceTest` | ⬜ | +| T-07 | Delete admin role — success | Unit | `DocumentServiceTest` | ⬜ | +| T-08 | Download unauthenticated — 401 | Integration | `DocumentControllerSecurityTest` | ⬜ | +| T-09 | Download wrong tenant — 403 | Integration | `DocumentControllerSecurityTest` | ⬜ | +| T-10 | Download correct tenant — 200 | Integration | `DocumentControllerSecurityTest` | ⬜ | +| T-11 | Delete as MEMBER — 403 | Integration | `DocumentControllerSecurityTest` | ⬜ | +| T-12 | Delete as STAFF — 200 | Integration | `DocumentControllerSecurityTest` | ⬜ | +| T-13 | Upload as MEMBER — 403 | Integration | `DocumentControllerSecurityTest` | ⬜ | +| T-14 | Upload as STAFF — 200 | Integration | `DocumentControllerSecurityTest` | ⬜ | +| T-15 | Login valid credentials — token pair | Unit | `AuthServiceTest` | ⬜ | +| T-16 | Login invalid password — 401 | Unit | `AuthServiceTest` | ⬜ | +| T-17 | Login non-existent user — 401 | Unit | `AuthServiceTest` | ⬜ | +| T-18 | Refresh token valid — new access token | Unit | `AuthServiceTest` | ⬜ | +| T-19 | Refresh token expired — 401 | Unit | `AuthServiceTest` | ⬜ | +| T-20 | SHA-256 hashing deterministic | Unit | `AuthServiceTest` | ⬜ | +| T-21 | Document endpoints require auth | Integration | `SecurityConfigTest` | ⬜ | +| T-22 | Auth endpoints are public | Integration | `SecurityConfigTest` | ⬜ | +| T-23 | Actuator health is public | Integration | `SecurityConfigTest` | ⬜ | +| T-24 | CORS allows configured origin | Integration | `SecurityConfigTest` | ⬜ | +| T-25 | CORS rejects unconfigured origin | Integration | `SecurityConfigTest` | ⬜ | +| T-26 | Rate limit — 5 requests pass | Integration | `LoginRateLimitFilterTest` | ⬜ | +| T-27 | Rate limit — 6th request returns 429 | Integration | `LoginRateLimitFilterTest` | ⬜ | +| T-28 | Rate limit — different IPs independent | Integration | `LoginRateLimitFilterTest` | ⬜ | +| T-29 | Rate limiter evicts stale entries (Caffeine TTL) | Unit | `LoginRateLimitFilterTest` | ⬜ | +| T-30 | CI backend tests run on push | Manual | CI/CD verification | ⬜ | +| T-31 | CI frontend lint runs on push | Manual | CI/CD verification | ⬜ | +| T-32 | CI blocks deploy on test failure | Manual | CI/CD verification | ⬜ | + +Status: ⬜ Open | ✅ Passed | ❌ Failed | ⏭️ Skipped + +--- + +## Test Cases + +### T-01: Path Traversal Filename Sanitization + +**Type:** Unit +**Class:** `cannamanage-service/src/test/java/de/cannamanage/service/DocumentServiceTest.java` +**Method:** `testUploadDocument_pathTraversalFilename_isSanitized()` + +**Preconditions:** +- DocumentService instantiated with mocked dependencies +- Mock file storage path configured + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | `"../../etc/passwd.pdf"` | Stored as `passwd.pdf` (path components stripped) | +| b | `"../../../tmp/evil.txt"` | Stored as `evil.txt` | +| c | `"..\\..\\windows\\system32\\bad.exe"` | Stored as `bad.exe` (backslash traversal) | +| d | `"normal-document.pdf"` | Stored as `normal-document.pdf` (unchanged) | + +**Postconditions:** +- File is stored under `{UPLOAD_BASE}/{clubId}/{docId}_{sanitizedFilename}` +- No path escapes the upload base directory + +--- + +### T-02: Null Filename Fallback + +**Type:** Unit +**Class:** `DocumentServiceTest` +**Method:** `testUploadDocument_nullFilename_usesFallback()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | `null` filename | Stored as `"document"` | +| b | Empty string `""` | Stored as `"document"` | +| c | Whitespace only `" "` | Stored as `"document"` | + +--- + +### T-03: Valid Filename Preserved + +**Type:** Unit +**Class:** `DocumentServiceTest` +**Method:** `testUploadDocument_validFilename_preserved()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | `"meeting-notes-2026.pdf"` | Stored as-is | +| b | `"Mitgliederversammlung Protokoll.docx"` | Stored as-is (spaces allowed) | +| c | `"report_v2.1_final.xlsx"` | Stored as-is (dots and underscores) | + +--- + +### T-04: Download Wrong Tenant — Forbidden + +**Type:** Unit +**Class:** `DocumentServiceTest` +**Method:** `testDownloadDocument_wrongTenant_throwsForbidden()` + +**Preconditions:** +- Document exists with `clubId = "club-A"` +- Current user's tenant context = `"club-B"` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | Request document by UUID belonging to different club | `AccessDeniedException` thrown | + +**Postconditions:** +- No file content is returned +- Audit log records the denied access attempt + +--- + +### T-05: Download Correct Tenant — Success + +**Type:** Unit +**Class:** `DocumentServiceTest` +**Method:** `testDownloadDocument_correctTenant_returnsContent()` + +**Preconditions:** +- Document exists with `clubId = "club-A"` +- Current user's tenant context = `"club-A"` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | Request document by valid UUID, same tenant | File bytes returned successfully | + +--- + +### T-06: Delete Wrong Tenant — Forbidden + +**Type:** Unit +**Class:** `DocumentServiceTest` +**Method:** `testDeleteDocument_wrongTenant_throwsForbidden()` + +**Preconditions:** +- Document exists with `clubId = "club-A"` +- Current user's tenant context = `"club-B"` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | Delete request for document in different club | `AccessDeniedException` thrown | + +**Postconditions:** +- Document is NOT deleted +- File remains on disk + +--- + +### T-07: Delete Admin Role — Success + +**Type:** Unit +**Class:** `DocumentServiceTest` +**Method:** `testDeleteDocument_adminRole_succeeds()` + +**Preconditions:** +- Document exists with `clubId = "club-A"` +- Current user: role `ADMIN`, tenant `"club-A"` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | Admin deletes own tenant's document | Document removed from DB + file system | + +--- + +### T-08: Download Unauthenticated — 401 + +**Type:** Integration +**Class:** `cannamanage-api/src/test/java/de/cannamanage/api/controller/DocumentControllerSecurityTest.java` +**Method:** `testDownload_unauthenticated_returns401()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | GET `/api/v1/documents/{id}/download` with no auth header | HTTP 401 | + +--- + +### T-09: Download Wrong Tenant — 403 + +**Type:** Integration +**Class:** `DocumentControllerSecurityTest` +**Method:** `testDownload_wrongTenant_returns403()` + +**Preconditions:** +- `@WithMockUser` configured for club-B +- Document belongs to club-A + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | Authenticated user requests document from different tenant | HTTP 403 | + +--- + +### T-10: Download Correct Tenant — 200 + +**Type:** Integration +**Class:** `DocumentControllerSecurityTest` +**Method:** `testDownload_correctTenant_returns200()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | Authenticated user downloads own tenant's document | HTTP 200 + file content | + +--- + +### T-11: Delete as MEMBER — 403 + +**Type:** Integration +**Class:** `DocumentControllerSecurityTest` +**Method:** `testDelete_memberRole_returns403()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | `@WithMockUser(roles="MEMBER")` → DELETE `/api/v1/documents/{id}` | HTTP 403 | + +--- + +### T-12: Delete as STAFF — 200 + +**Type:** Integration +**Class:** `DocumentControllerSecurityTest` +**Method:** `testDelete_staffRole_returns200()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | `@WithMockUser(roles="STAFF")` → DELETE `/api/v1/documents/{id}` | HTTP 200 or 204 | + +--- + +### T-13: Upload as MEMBER — 403 + +**Type:** Integration +**Class:** `DocumentControllerSecurityTest` +**Method:** `testUpload_memberRole_returns403()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | `@WithMockUser(roles="MEMBER")` → POST `/api/v1/documents` | HTTP 403 | + +--- + +### T-14: Upload as STAFF — 200 + +**Type:** Integration +**Class:** `DocumentControllerSecurityTest` +**Method:** `testUpload_staffRole_returns200()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | `@WithMockUser(roles="STAFF")` → POST `/api/v1/documents` with multipart file | HTTP 200 or 201 | + +--- + +### T-15: Login Valid Credentials — Token Pair + +**Type:** Unit +**Class:** `cannamanage-api/src/test/java/de/cannamanage/api/service/AuthServiceTest.java` +**Method:** `testLogin_validCredentials_returnsTokenPair()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | Valid email + matching BCrypt password | Response contains `accessToken` (non-null, non-empty) + `refreshToken` | + +--- + +### T-16: Login Invalid Password — 401 + +**Type:** Unit +**Class:** `AuthServiceTest` +**Method:** `testLogin_invalidPassword_throws401()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | Valid email + wrong password | Exception with 401 semantics | +| b | Valid email + empty password | Exception with 401 semantics | + +--- + +### T-17: Login Non-Existent User — 401 + +**Type:** Unit +**Class:** `AuthServiceTest` +**Method:** `testLogin_nonExistentUser_throws401()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | `unknown@example.com` + any password | Exception with 401 semantics | + +**Postconditions:** +- Timing is consistent with valid-user path (prevent user enumeration) + +--- + +### T-18: Refresh Token Valid — New Access Token + +**Type:** Unit +**Class:** `AuthServiceTest` +**Method:** `testRefreshToken_validToken_returnsNewAccess()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | Valid, non-expired refresh token | New access token returned, refresh token rotated | + +--- + +### T-19: Refresh Token Expired — 401 + +**Type:** Unit +**Class:** `AuthServiceTest` +**Method:** `testRefreshToken_expired_throws401()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | Refresh token past expiry date | Exception with 401 semantics | +| b | Token hash doesn't match stored hash | Exception with 401 semantics | + +--- + +### T-20: SHA-256 Hashing Deterministic + +**Type:** Unit +**Class:** `AuthServiceTest` +**Method:** `testSha256_sameInput_sameOutput()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | `"test-token-123"` hashed twice | Both results are identical | +| b | `"different-input"` | Different hash than input (a) | + +--- + +### T-21: Document Endpoints Require Auth + +**Type:** Integration +**Class:** `cannamanage-api/src/test/java/de/cannamanage/api/security/SecurityConfigTest.java` +**Method:** `testDocumentEndpoints_requireAuthentication()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | GET `/api/v1/documents` without auth | HTTP 401 | +| b | POST `/api/v1/documents` without auth | HTTP 401 | +| c | DELETE `/api/v1/documents/{id}` without auth | HTTP 401 | + +--- + +### T-22: Auth Endpoints Are Public + +**Type:** Integration +**Class:** `SecurityConfigTest` +**Method:** `testAuthEndpoints_arePublic()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | POST `/api/v1/auth/login` without auth | HTTP 200 or 400 (not 401) | +| b | POST `/api/v1/auth/register` without auth | HTTP 200 or 400 (not 401) | + +--- + +### T-23: Actuator Health Is Public + +**Type:** Integration +**Class:** `SecurityConfigTest` +**Method:** `testActuatorHealth_isPublic()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | GET `/actuator/health` without auth | HTTP 200 | + +--- + +### T-24: CORS Allows Configured Origin + +**Type:** Integration +**Class:** `SecurityConfigTest` +**Method:** `testCors_allowedOrigin_returns200()` + +**Preconditions:** +- `cannamanage.cors.allowed-origins=http://localhost:3000,https://app.cannamanage.de` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | `Origin: http://localhost:3000` | `Access-Control-Allow-Origin: http://localhost:3000` | +| b | `Origin: https://app.cannamanage.de` | `Access-Control-Allow-Origin: https://app.cannamanage.de` | + +--- + +### T-25: CORS Rejects Unconfigured Origin + +**Type:** Integration +**Class:** `SecurityConfigTest` +**Method:** `testCors_unconfiguredOrigin_noHeader()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | `Origin: https://evil.com` | No `Access-Control-Allow-Origin` header in response | + +--- + +### T-26: Rate Limit — 5 Requests Pass + +**Type:** Integration +**Class:** `cannamanage-api/src/test/java/de/cannamanage/api/security/LoginRateLimitFilterTest.java` +**Method:** `testRateLimit_5requests_allPass()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | 5 POST requests to `/api/v1/auth/login` from same IP within 1 minute | All return normal response (200 or 401) | + +--- + +### T-27: Rate Limit — 6th Request Returns 429 + +**Type:** Integration +**Class:** `LoginRateLimitFilterTest` +**Method:** `testRateLimit_6thRequest_returns429()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | 6th POST to `/api/v1/auth/login` from same IP within 1 minute | HTTP 429 + `Retry-After` header | + +--- + +### T-28: Rate Limit — Different IPs Independent + +**Type:** Integration +**Class:** `LoginRateLimitFilterTest` +**Method:** `testRateLimit_differentIPs_independent()` + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | 5 requests from IP-A, then 1 request from IP-B | IP-B request passes normally | + +--- + +### T-29: Rate Limiter Evicts Stale Entries (Caffeine TTL) + +**Type:** Unit +**Class:** `cannamanage-api/src/test/java/de/cannamanage/api/security/LoginRateLimitFilterTest.java` +**Method:** `testRateLimiter_evictsStaleEntries()` + +**Preconditions:** +- Caffeine cache configured with short TTL for testing (override via `@TestPropertySource` or direct instantiation) + +**Scenarios:** + +| # | Input | Expected Result | +|---|-------|-----------------| +| a | Exhaust 5 attempts from IP-A, wait for TTL to expire, then attempt again | Request passes (bucket evicted and recreated) | +| b | Verify cache size does not grow unbounded after many unique IPs | Cache respects `maximumSize(10_000)` — old entries evicted | + +**Postconditions:** +- No memory leak under simulated DDoS (many unique IPs) +- Stale rate limit buckets are automatically cleaned up + +--- + +### T-30: CI Backend Tests Run on Push + +**Type:** Manual +**Verification:** Push a commit to `main`, verify Gitea Actions log shows Maven test step executing. + +--- + +### T-31: CI Frontend Lint Runs on Push + +**Type:** Manual +**Verification:** Push a commit to `main`, verify Gitea Actions log shows pnpm lint + type-check step executing. + +--- + +### T-32: CI Blocks Deploy on Test Failure + +**Type:** Manual +**Verification:** Introduce a deliberately failing test, push to `main`, verify deployment does NOT proceed and the workflow fails at the test step. + +--- + +## Test Data + +### Documents Test Fixtures +- Club A: UUID `"11111111-1111-1111-1111-111111111111"`, document with known UUID +- Club B: UUID `"22222222-2222-2222-2222-222222222222"`, separate document + +### Auth Test Fixtures +- Valid user: `"test@example.com"`, BCrypt password hash of `"TestPass123!"` +- Non-existent user: `"ghost@example.com"` + +### Rate Limit Test Setup +- Use `MockHttpServletRequest` with different `remoteAddr` values to simulate multiple IPs + +--- + +## Test Coverage + +| Component | Unit | Integration | Manual | Total | +|-----------|------|-------------|--------|-------| +| DocumentService | 7 | 0 | 0 | 7 | +| DocumentController | 0 | 7 | 0 | 7 | +| AuthService | 6 | 0 | 0 | 6 | +| SecurityConfig | 0 | 5 | 0 | 5 | +| LoginRateLimitFilter | 1 | 3 | 0 | 4 | +| CI/CD Pipeline | 0 | 0 | 3 | 3 | +| **Total** | **14** | **15** | **3** | **32** | + +--- + +## Execution Order + +1. Run unit tests first (T-01 through T-07, T-15 through T-20, T-29) — fast, no Spring context +2. Run integration tests (T-08 through T-14, T-21 through T-28) — require `@WebMvcTest` / `@SpringBootTest` +3. Verify CI/CD manually (T-30 through T-32) — after all code is merged + +--- + +## Pass Criteria + +- **All 29 automated tests (T-01 through T-29) must pass** before merging +- **All 3 manual tests (T-30 through T-32) must pass** after CI changes are deployed +- **Zero tolerance** on security tests (T-01 through T-14) — any failure is a blocker