Patrick Plate
338 lines
14 KiB
Markdown
338 lines
14 KiB
Markdown
# 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
|
||
<dependency>
|
||
<groupId>com.bucket4j</groupId>
|
||
<artifactId>bucket4j-core</artifactId>
|
||
<version>8.10.1</version>
|
||
</dependency>
|
||
```
|
||
|
||
Add Caffeine dependency to `cannamanage-api/pom.xml`:
|
||
```xml
|
||
<dependency>
|
||
<groupId>com.github.ben-manes.caffeine</groupId>
|
||
<artifactId>caffeine</artifactId>
|
||
<version>3.1.8</version>
|
||
</dependency>
|
||
```
|
||
|
||
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)
|