This commit was merged in pull request #108.
This commit is contained in:
@@ -0,0 +1,122 @@
|
||||
# Security Checklist -- Mitgliederverwaltung
|
||||
|
||||
## Input Validation Audit
|
||||
|
||||
### Controllers Reviewed
|
||||
All controllers use parameterized queries via Nextcloud's QBMapper -- no raw SQL anywhere.
|
||||
|
||||
| Controller | Status | Notes |
|
||||
|----------------------|--------|-------------------------------------------------|
|
||||
| MemberController | OK | Validates required fields, dates, duplicates |
|
||||
| FamilyController | OK | Validates required name field |
|
||||
| FeeController | OK | Validates year, amounts, rule existence |
|
||||
| LagerController | OK | Validates required fields, date ordering |
|
||||
| InjuryController | OK | Validates required fields |
|
||||
| ReportController | OK | Type parameter validated against whitelist |
|
||||
| ExportController | OK | Password validation for encrypted exports |
|
||||
| PermissionController | OK | Validates level enum, NC user existence |
|
||||
| AuditController | OK | Read-only, pagination validated |
|
||||
| DsgvoController | OK | Admin-only, validates member existence |
|
||||
| MilestoneController | OK | Settings validated, read-only detection |
|
||||
| StufeController | OK | Validates required name and age range |
|
||||
|
||||
### SQL Injection Prevention
|
||||
- All database access uses `QBMapper` or `IQueryBuilder` with named parameters
|
||||
- No `$db->executeQuery($rawSql)` calls found
|
||||
- Field names in queries are hardcoded constants, never user input
|
||||
|
||||
### XSS Prevention
|
||||
- Frontend: Vue 3 auto-escapes all interpolated values (`{{ }}`)
|
||||
- Backend: `InputSanitizer` available for stripping HTML from inputs
|
||||
- Rich text fields (Beschreibung, Notizen) stored as-is but escaped on display
|
||||
|
||||
### CSRF Protection
|
||||
- All state-changing endpoints (POST/PUT/DELETE) require CSRF token
|
||||
- Only GET endpoints use `#[NoCSRFRequired]` attribute
|
||||
- Nextcloud framework enforces CSRF by default
|
||||
|
||||
## Rate Limiting
|
||||
|
||||
| Endpoint Group | Limit | Rationale |
|
||||
|----------------|----------------|-----------------------------------|
|
||||
| Search | 60 req/min | Prevent member data scraping |
|
||||
| Export | 10 req/min | Prevent bulk data extraction |
|
||||
| Reports | 10 req/min | CPU-intensive PDF generation |
|
||||
| Import | 5 req/min | Prevent upload flooding |
|
||||
| Default | 120 req/min | General API protection |
|
||||
|
||||
Implementation: `RateLimitMiddleware` using NC distributed cache.
|
||||
|
||||
## Permission Enforcement
|
||||
|
||||
| Resource | Read Access | Write Access | Delete Access |
|
||||
|-----------------------|--------------------|--------------------|--------------------|
|
||||
| Members | Viewer+ | Editor+ | Editor+ |
|
||||
| Member banking data | can_see_banking | Editor+ | Editor+ |
|
||||
| Families | Viewer+ | Editor+ | Editor+ |
|
||||
| Fee records | Viewer+ | Admin | Admin |
|
||||
| Fee rules | Viewer+ | Admin | Admin |
|
||||
| Permissions | Admin | Admin | Admin |
|
||||
| Audit log | Viewer+ | - | - |
|
||||
| Settings | Admin | Admin | - |
|
||||
| DSGVO (hard delete) | Admin | Admin | Admin |
|
||||
| Encrypted exports | Editor+ | - | - |
|
||||
| Import | Admin | Admin | - |
|
||||
|
||||
## Encrypted Data
|
||||
|
||||
| Field | Encryption Method | Visibility |
|
||||
|-----------------------|----------------------------|-----------------------------|
|
||||
| IBAN | NC ICrypto (AES-256-CTR) | can_see_banking users only |
|
||||
| Kontoinhaber | NC ICrypto (AES-256-CTR) | can_see_banking users only |
|
||||
| Allergien | NC ICrypto (AES-256-CTR) | Masked in audit log |
|
||||
| Export ZIPs | ZIP AES-256 encryption | Password required |
|
||||
|
||||
## Dependency Security
|
||||
|
||||
Run these commands regularly:
|
||||
|
||||
```bash
|
||||
# PHP dependencies
|
||||
composer audit
|
||||
|
||||
# JS dependencies
|
||||
npm audit
|
||||
```
|
||||
|
||||
Pin all dependency versions in `composer.json` and `package.json`.
|
||||
|
||||
## Penetration Testing Checklist
|
||||
|
||||
### Automated
|
||||
- [ ] Run OWASP ZAP automated scan against running instance
|
||||
- [ ] Run `composer audit` -- no critical/high vulnerabilities
|
||||
- [ ] Run `npm audit` -- no critical/high vulnerabilities
|
||||
|
||||
### Manual Testing
|
||||
- [ ] SQL injection on search endpoint with payloads: `' OR 1=1--`, `'; DROP TABLE--`
|
||||
- [ ] Permission bypass: Viewer accessing Admin endpoints
|
||||
- [ ] Permission bypass: Stufenzugriff user accessing other Stufe's members
|
||||
- [ ] Banking field visibility: user without `can_see_banking` sees no IBAN/Kontoinhaber
|
||||
- [ ] Encrypted export without password returns error
|
||||
- [ ] CSRF: POST/PUT/DELETE without token returns 401/403
|
||||
- [ ] Session fixation: verify session ID rotates on login
|
||||
- [ ] File path traversal: `../../etc/passwd` in file integration paths
|
||||
- [ ] XSS: `<script>alert(1)</script>` in member name, notes, beschreibung
|
||||
|
||||
### Audit Log Verification
|
||||
- [ ] All state-changing operations create audit entries
|
||||
- [ ] Encrypted fields show `[verschluesselt]` not plaintext
|
||||
- [ ] Deleted member sensitive data shows `[geloescht]`
|
||||
|
||||
## Deployment Hardening
|
||||
|
||||
1. Enable Nextcloud 2FA for all admin users
|
||||
2. Set `config.php` `loglevel` to 1 (Warn) in production
|
||||
3. Enable Nextcloud Brute Force Protection
|
||||
4. Restrict app to MySQL/MariaDB (SQLite not recommended for production)
|
||||
5. Enable HTTPS with valid TLS certificate
|
||||
6. Set `Strict-Transport-Security` header
|
||||
7. Configure CSP headers (Nextcloud handles this by default)
|
||||
8. Regular database backups with encryption at rest
|
||||
9. Restrict file permissions: web server user only needs read access to app directory
|
||||
@@ -4,6 +4,7 @@ declare(strict_types=1);
|
||||
|
||||
namespace OCA\Mitgliederverwaltung\AppInfo;
|
||||
|
||||
use OCA\Mitgliederverwaltung\Middleware\RateLimitMiddleware;
|
||||
use OCP\AppFramework\App;
|
||||
use OCP\AppFramework\Bootstrap\IBootContext;
|
||||
use OCP\AppFramework\Bootstrap\IBootstrap;
|
||||
@@ -17,6 +18,7 @@ class Application extends App implements IBootstrap {
|
||||
}
|
||||
|
||||
public function register(IRegistrationContext $context): void {
|
||||
$context->registerMiddleware(RateLimitMiddleware::class);
|
||||
}
|
||||
|
||||
public function boot(IBootContext $context): void {
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace OCA\Mitgliederverwaltung\Middleware;
|
||||
|
||||
/**
|
||||
* Input sanitization utilities for preventing XSS and injection attacks.
|
||||
*
|
||||
* All controller inputs should pass through these methods before use.
|
||||
* Vue handles output escaping on the frontend, but we sanitize inputs
|
||||
* as defense-in-depth.
|
||||
*
|
||||
* Part of Issue #64.
|
||||
*/
|
||||
class InputSanitizer {
|
||||
|
||||
/**
|
||||
* Strip HTML tags and trim whitespace from a string.
|
||||
*
|
||||
* @param string|null $input Raw input
|
||||
* @return string|null Sanitized string, or null if input was null
|
||||
*/
|
||||
public static function sanitizeString(?string $input): ?string {
|
||||
if ($input === null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Strip HTML/PHP tags
|
||||
$cleaned = strip_tags($input);
|
||||
|
||||
// Trim whitespace
|
||||
$cleaned = trim($cleaned);
|
||||
|
||||
return $cleaned;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize an integer input.
|
||||
*
|
||||
* @param mixed $input Raw input
|
||||
* @return int|null Integer value or null
|
||||
*/
|
||||
public static function sanitizeInt($input): ?int {
|
||||
if ($input === null || $input === '') {
|
||||
return null;
|
||||
}
|
||||
|
||||
$filtered = filter_var($input, FILTER_VALIDATE_INT);
|
||||
return $filtered !== false ? $filtered : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize a date string (YYYY-MM-DD format only).
|
||||
*
|
||||
* @param string|null $input Raw date input
|
||||
* @return string|null Valid date string or null
|
||||
*/
|
||||
public static function sanitizeDate(?string $input): ?string {
|
||||
if ($input === null || $input === '') {
|
||||
return null;
|
||||
}
|
||||
|
||||
$cleaned = trim($input);
|
||||
|
||||
// Only allow YYYY-MM-DD format
|
||||
if (!preg_match('/^\d{4}-\d{2}-\d{2}$/', $cleaned)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Validate it's a real date
|
||||
$parts = explode('-', $cleaned);
|
||||
if (!checkdate((int)$parts[1], (int)$parts[2], (int)$parts[0])) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return $cleaned;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize an email address.
|
||||
*
|
||||
* @param string|null $input Raw email input
|
||||
* @return string|null Valid email or null
|
||||
*/
|
||||
public static function sanitizeEmail(?string $input): ?string {
|
||||
if ($input === null || $input === '') {
|
||||
return null;
|
||||
}
|
||||
|
||||
$cleaned = trim($input);
|
||||
$filtered = filter_var($cleaned, FILTER_VALIDATE_EMAIL);
|
||||
return $filtered !== false ? $filtered : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize an array of data with known field types.
|
||||
*
|
||||
* @param array $data Raw input data
|
||||
* @param array $fieldTypes Map of field_name => type ('string'|'int'|'date'|'email'|'text')
|
||||
* @return array Sanitized data
|
||||
*/
|
||||
public static function sanitizeArray(array $data, array $fieldTypes): array {
|
||||
$sanitized = [];
|
||||
|
||||
foreach ($data as $key => $value) {
|
||||
$type = $fieldTypes[$key] ?? 'string';
|
||||
|
||||
$sanitized[$key] = match ($type) {
|
||||
'int' => self::sanitizeInt($value),
|
||||
'date' => self::sanitizeDate($value),
|
||||
'email' => self::sanitizeEmail($value),
|
||||
'text' => self::sanitizeString($value), // Same as string but could be extended
|
||||
'bool' => (bool)$value,
|
||||
'array' => is_array($value) ? $value : [],
|
||||
default => is_string($value) ? self::sanitizeString($value) : $value,
|
||||
};
|
||||
}
|
||||
|
||||
return $sanitized;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,131 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace OCA\Mitgliederverwaltung\Middleware;
|
||||
|
||||
use OCP\AppFramework\Http;
|
||||
use OCP\AppFramework\Http\JSONResponse;
|
||||
use OCP\AppFramework\Middleware;
|
||||
use OCP\ICache;
|
||||
use OCP\ICacheFactory;
|
||||
use OCP\IRequest;
|
||||
use OCP\IUserSession;
|
||||
use Psr\Log\LoggerInterface;
|
||||
|
||||
/**
|
||||
* Rate limiting middleware for sensitive API endpoints.
|
||||
*
|
||||
* Protects search, export, and report endpoints from abuse (data scraping).
|
||||
* Uses Nextcloud's distributed cache for rate tracking.
|
||||
*
|
||||
* Configurable limits per endpoint group:
|
||||
* - Search: 60 requests per minute
|
||||
* - Export: 10 requests per minute
|
||||
* - Report: 10 requests per minute
|
||||
* - Default: 120 requests per minute
|
||||
*
|
||||
* Part of Issue #64.
|
||||
*/
|
||||
class RateLimitMiddleware extends Middleware {
|
||||
|
||||
/** Rate limits: [url_pattern => [max_requests, window_seconds]] */
|
||||
private const LIMITS = [
|
||||
'/search' => [60, 60],
|
||||
'/export/' => [10, 60],
|
||||
'/report/' => [10, 60],
|
||||
'/import/' => [5, 60],
|
||||
];
|
||||
|
||||
private const DEFAULT_LIMIT = [120, 60];
|
||||
|
||||
private IRequest $request;
|
||||
private IUserSession $userSession;
|
||||
private ?ICache $cache;
|
||||
private LoggerInterface $logger;
|
||||
|
||||
public function __construct(
|
||||
IRequest $request,
|
||||
IUserSession $userSession,
|
||||
ICacheFactory $cacheFactory,
|
||||
LoggerInterface $logger
|
||||
) {
|
||||
$this->request = $request;
|
||||
$this->userSession = $userSession;
|
||||
$this->cache = $cacheFactory->isAvailable()
|
||||
? $cacheFactory->createDistributed('mv_ratelimit')
|
||||
: null;
|
||||
$this->logger = $logger;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check rate limit before controller action executes.
|
||||
*
|
||||
* @throws \OCP\AppFramework\Http\TooManyRequestsException
|
||||
*/
|
||||
public function beforeController($controller, $methodName): void {
|
||||
if ($this->cache === null) {
|
||||
// No cache backend available — cannot enforce rate limits
|
||||
return;
|
||||
}
|
||||
|
||||
$user = $this->userSession->getUser();
|
||||
if ($user === null) {
|
||||
return;
|
||||
}
|
||||
|
||||
$uri = $this->request->getRequestUri();
|
||||
[$maxRequests, $windowSeconds] = $this->getLimitForUri($uri);
|
||||
|
||||
$cacheKey = 'rl_' . $user->getUID() . '_' . md5($uri);
|
||||
$current = $this->cache->get($cacheKey);
|
||||
|
||||
if ($current === null) {
|
||||
$this->cache->set($cacheKey, 1, $windowSeconds);
|
||||
return;
|
||||
}
|
||||
|
||||
if ($current >= $maxRequests) {
|
||||
$this->logger->warning('Rate limit exceeded', [
|
||||
'user' => $user->getUID(),
|
||||
'uri' => $uri,
|
||||
'limit' => $maxRequests,
|
||||
'window' => $windowSeconds,
|
||||
'app' => 'mitgliederverwaltung',
|
||||
]);
|
||||
|
||||
// We'll handle this in afterException
|
||||
throw new \RuntimeException('Rate limit exceeded');
|
||||
}
|
||||
|
||||
$this->cache->set($cacheKey, $current + 1, $windowSeconds);
|
||||
}
|
||||
|
||||
/**
|
||||
* Convert rate limit exceptions to proper HTTP 429 responses.
|
||||
*/
|
||||
public function afterException($controller, $methodName, \Exception $exception): JSONResponse {
|
||||
if ($exception->getMessage() === 'Rate limit exceeded') {
|
||||
return new JSONResponse(
|
||||
['error' => 'Zu viele Anfragen. Bitte versuchen Sie es spaeter erneut.'],
|
||||
Http::STATUS_TOO_MANY_REQUESTS
|
||||
);
|
||||
}
|
||||
|
||||
throw $exception;
|
||||
}
|
||||
|
||||
/**
|
||||
* Determine rate limit for a given URI.
|
||||
*
|
||||
* @return int[] [max_requests, window_seconds]
|
||||
*/
|
||||
private function getLimitForUri(string $uri): array {
|
||||
foreach (self::LIMITS as $pattern => $limit) {
|
||||
if (str_contains($uri, $pattern)) {
|
||||
return $limit;
|
||||
}
|
||||
}
|
||||
return self::DEFAULT_LIMIT;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user