# Backend TODO

Findings from architectural review. Ordered roughly by severity.

---

## Bugs

### `@Value` annotation missing closing brace
**File:** `config/DataInitializer.java:33–37`
```java
// Current (broken — Spring may silently fail to resolve the property)
@Value("${app.admin.username:admin")
@Value("${app.admin.password:admin123")

// Fix
@Value("${app.admin.username:admin}")
@Value("${app.admin.password:admin123}")
```
Spring's `@Value` SpEL parser requires the closing `}`. Without it, the literal string `"${app.admin.username:admin"` is injected instead of the resolved value. The default credentials will appear to work but env-var overrides will silently be ignored.

---

### Default credentials logged in plaintext
**File:** `config/DataInitializer.java:64`
```java
log.info("Default Admin erstellt: User='admin', Pass='admin123'");
```
The password appears in application logs. Remove the password from the log statement entirely. Log only the username.

---

## Design Issues

### Test data runs in every environment
**File:** `config/DataInitializer.java:69–143`

`initData` seeds 500 fake documents and 4 random persons whenever the database is empty. This runs unconditionally — including in production — and the log message at line 141 claims "50 Personen" when only 4 are created (copy/paste error).

**Fix:** Guard the bean with a Spring profile so it only runs locally:
```java
@Bean
@Profile("dev")
public CommandLineRunner initData(...) { ... }
```
Set `spring.profiles.active=dev` in `application.properties` for local use, and ensure the production environment does not set that profile.

---

### Two redundant permission groups created on startup
**File:** `config/DataInitializer.java:49–53` and `101–105`

`initAdminUser` creates group `"Administrators"` with `{ADMIN, READ_ALL, WRITE_ALL}`.
`initData` creates group `"Admins"` with `{READ_ALL, WRITE_ALL, ADMIN}` — identical permissions, different name, different bean.

Both beans run when the DB is empty, resulting in two duplicate admin groups and the admin user only belonging to the first one. `initData` is also guarded by `personRepo.count() > 0` — so if persons already exist it won't create the second group, making startup behaviour inconsistent.

**Fix:** Consolidate into a single `CommandLineRunner` bean or extract group creation into a shared `@PostConstruct` method. Remove `initData`'s group creation entirely (it belongs in `initAdminUser`).

---

### CSRF protection disabled with no plan to re-enable it
**File:** `config/SecurityConfig.java:39`
```java
.csrf(csrf -> csrf.disable())
```
The comment says "for development". HTTP Basic Auth over a cookie-authenticated SvelteKit frontend is still vulnerable to CSRF for mutating endpoints (PUT, DELETE, POST).

**Fix:** Either:
- Re-enable CSRF with `CookieCsrfTokenRepository` and pass the token to the frontend, or
- Adopt a stateless JWT/session approach and verify `Origin`/`Referer` headers

At minimum, document this as a known gap that must be addressed before any external deployment.

---

### Spring Session tables are created but never used
**File:** `db/migration/V1__initial_schema.sql` (Spring Session tables), `config/SecurityConfig.java`

The schema includes `spring_session` and `spring_session_attributes` tables, but the auth model is stateless HTTP Basic — credentials are re-validated from the database on every request. Spring Session JDBC is wiring itself up but managing nothing meaningful.

**Fix:** Decide on one model:
- **Keep Basic Auth (stateless):** Remove the Spring Session JDBC dependency and tables. Each request re-authenticates — which is fine for a small internal app.
- **Switch to session-based auth:** Replace Basic Auth with form login, issue a server-side session ID, and let Spring Session manage it. This reduces per-request DB hits.

---

### `Permission` enum not used consistently — permissions are plain strings
**File:** `security/Permission.java`, `security/RequirePermission.java`, `security/PermissionAspect.java`

`@RequirePermission` takes a `Permission` enum value, but user group permissions are stored and compared as raw `String` values (e.g., `"ADMIN"`, `"READ_ALL"`). The comparison in `PermissionAspect` calls `permission.name()` to convert the enum back to a string for matching — so type-safety is only at the annotation call site, not end-to-end.

**Fix:** Make the entire stack type-safe: store permissions as the enum's name (which is already happening) but also parse them back into `Permission` enum values when loading `UserDetails` authorities. This way a typo in the DB is caught at load time rather than silently failing permission checks.

---

### `MassImportService` provides no status or error feedback
**File:** `service/MassImportService.java`, `controller/AdminController.java`

`/api/admin/trigger-import` returns immediately (async), but there is no way for the admin to know whether the import succeeded, failed, or is still running. Errors during async execution are silently swallowed.

**Fix options:**
- Store import job status in a DB table (`import_jobs`) with state (`RUNNING`, `DONE`, `FAILED`) and expose a `GET /api/admin/import-status` endpoint
- Alternatively, make the endpoint synchronous since it already blocks on file I/O — only use async if you need true non-blocking behaviour

---

## Missing Capabilities

### No test coverage
**File:** `src/test/java/org/raddatz/familienarchiv/FamilienarchivApplicationTests.java`

The only test is a Spring context load test. No unit or integration tests exist for any service, repository, or controller logic.

**Suggested starting points (highest value for effort):**
1. `DocumentSpecifications` — pure logic, easy to unit test with an in-memory H2 or Testcontainers PostgreSQL
2. `ExcelService` — parsing logic, test with fixture `.xlsx` files (one exists in `api_tests/`)
3. `PermissionAspect` — security logic should be tested; use `@WithMockUser` from Spring Security Test

---

### No API documentation
There is no OpenAPI/Swagger endpoint. The only documentation is the `.http` files in `backend/api_tests/`.

**Fix:** Add `springdoc-openapi-starter-webmvc-ui` to `pom.xml`. It generates `/swagger-ui.html` and `/v3/api-docs` automatically from existing controller annotations with zero additional code.

---

### `FileService` content-type detection is fragile
File extension is inferred from the S3 key string. Many document types (TIFF scans, HEIC photos, Word documents) are unhandled and fall back to `application/octet-stream`, forcing a download instead of inline display.

**Fix:** Use `java.nio.file.Files.probeContentType()` or Apache Tika for robust MIME detection. Alternatively, store the content-type at upload time in the `Document` entity and retrieve it on download.

---

### Generic error response is hardcoded in German
**File:** `controller/GlobalExceptionHandler.java:36`
```java
.body(new ErrorResponse("Ein Fehler ist aufgetreten"))
```
The fallback error message is hardcoded in German. It should use a locale-aware message source or at minimum be in English as the lingua franca of APIs.