Fix timeout middleware panic on proxy/WebSocket routes and worker healthcheck
The TimeoutMiddleware wraps the response writer in *http.timeoutWriter which doesn't implement http.Flusher. When the admin reverse proxy or WebSocket upgrader tries to flush, it panics and crashes the container (502 Bad Gateway). Skip timeout for /admin, /_next, and /ws routes. Also fix the Dockerfile HEALTHCHECK to detect the worker process — the worker has no HTTP server so the curl-based check always failed, marking it unhealthy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Trey t
281
docs/SECURE_MEDIA_ACCESS.md
Normal file
281
docs/SECURE_MEDIA_ACCESS.md
Normal file
@@ -0,0 +1,281 @@
|
||||
# Secure Media Access Control Plan
|
||||
|
||||
## Problem Statement
|
||||
|
||||
Users upload private images and documents (task completion photos, warranties, documents) that may contain sensitive information. Currently, these files are **publicly accessible** via predictable URLs (`/uploads/{category}/{timestamp}_{uuid}.{ext}`). Anyone who knows or guesses a URL can access the file without authentication.
|
||||
|
||||
**Goal**: Ensure only users with access to a residence can view media files associated with that residence.
|
||||
|
||||
## User Decisions
|
||||
|
||||
| Decision | Choice |
|
||||
|----------|--------|
|
||||
| **Security Method** | Token-based proxy endpoint |
|
||||
| **Protected Media** | All uploaded media (completions, documents, warranties) |
|
||||
|
||||
---
|
||||
|
||||
## Current Architecture (Problem)
|
||||
|
||||
```
|
||||
Mobile App Go API Disk
|
||||
│ │ │
|
||||
├─→ POST /api/documents/ ─────────→│ Save to /uploads/docs/ │
|
||||
│ (authenticated) │────────────────────────────→│
|
||||
│ │ │
|
||||
│←─ { file_url: "/uploads/..." } ←─│ │
|
||||
│ │ │
|
||||
├─→ GET /uploads/docs/file.jpg ───→│ Static middleware │
|
||||
│ (NO AUTH CHECK!) │←───────────────────────────→│
|
||||
│←─ file bytes ←──────────────────←│ │
|
||||
```
|
||||
|
||||
**Issues**:
|
||||
1. `r.Static("/uploads", cfg.Storage.UploadDir)` serves files without authentication
|
||||
2. File URLs are predictable (timestamp + short UUID)
|
||||
3. URLs can be shared/leaked, granting permanent access
|
||||
4. No audit trail for file access
|
||||
|
||||
---
|
||||
|
||||
## Recommended Solution: Authenticated Media Proxy
|
||||
|
||||
### Architecture
|
||||
|
||||
```
|
||||
Mobile App Go API Disk
|
||||
│ │ │
|
||||
├─→ POST /api/documents/ ─────────→│ Save to /uploads/docs/ │
|
||||
│ (authenticated) │────────────────────────────→│
|
||||
│ │ │
|
||||
│←─ { file_url: "/api/media/..." }←│ Return proxy URL │
|
||||
│ │ │
|
||||
├─→ GET /api/media/{type}/{id}────→│ Media Handler: │
|
||||
│ Authorization: Token xxx │ 1. Verify token │
|
||||
│ │ 2. Get file record │
|
||||
│ │ 3. Check residence access │
|
||||
│ │ 4. Stream file │
|
||||
│←─ file bytes ←──────────────────←│←───────────────────────────→│
|
||||
```
|
||||
|
||||
### Key Changes
|
||||
|
||||
1. **Remove public static file serving** - No more `r.Static("/uploads", ...)`
|
||||
2. **Create authenticated media endpoint** - `GET /api/media/{type}/{id}`
|
||||
3. **Update stored URLs** - Store internal paths, return proxy URLs in API responses
|
||||
4. **Add access control** - Verify user has residence access before serving
|
||||
|
||||
---
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
### Phase 1: Create Media Handler
|
||||
|
||||
**New File: `/internal/handlers/media_handler.go`**
|
||||
|
||||
```go
|
||||
type MediaHandler struct {
|
||||
documentRepo repositories.DocumentRepository
|
||||
taskRepo repositories.TaskRepository
|
||||
residenceRepo repositories.ResidenceRepository
|
||||
storageSvc *services.StorageService
|
||||
}
|
||||
|
||||
// GET /api/media/document/{id}
|
||||
// GET /api/media/document-image/{id}
|
||||
// GET /api/media/completion-image/{id}
|
||||
func (h *MediaHandler) ServeMedia(c *gin.Context) {
|
||||
// 1. Extract user from auth context
|
||||
user := c.MustGet(middleware.AuthUserKey).(*models.User)
|
||||
|
||||
// 2. Get media type and ID from path
|
||||
mediaType := c.Param("type") // "document", "document-image", "completion-image"
|
||||
mediaID := c.Param("id")
|
||||
|
||||
// 3. Look up the file record and get residence ID
|
||||
residenceID, filePath, err := h.getFileInfo(mediaType, mediaID)
|
||||
|
||||
// 4. Check residence access
|
||||
hasAccess, _ := h.residenceRepo.HasAccess(residenceID, user.ID)
|
||||
if !hasAccess {
|
||||
c.JSON(403, gin.H{"error": "Access denied"})
|
||||
return
|
||||
}
|
||||
|
||||
// 5. Stream the file
|
||||
c.File(filePath)
|
||||
}
|
||||
```
|
||||
|
||||
### Phase 2: Update Models & Responses
|
||||
|
||||
**Change how URLs are stored and returned:**
|
||||
|
||||
| Model | Current Storage | New Storage | API Response |
|
||||
|-------|-----------------|-------------|--------------|
|
||||
| `Document.FileURL` | `/uploads/docs/file.pdf` | `docs/file.pdf` (relative) | `/api/media/document/{id}` |
|
||||
| `DocumentImage.ImageURL` | `/uploads/images/img.jpg` | `images/img.jpg` (relative) | `/api/media/document-image/{id}` |
|
||||
| `TaskCompletionImage.ImageURL` | `/uploads/completions/img.jpg` | `completions/img.jpg` (relative) | `/api/media/completion-image/{id}` |
|
||||
|
||||
**Update Response DTOs:**
|
||||
|
||||
```go
|
||||
// /internal/dto/responses/document_response.go
|
||||
type DocumentResponse struct {
|
||||
ID uint `json:"id"`
|
||||
// Don't expose FileURL directly
|
||||
// FileURL string `json:"file_url"` // REMOVE
|
||||
MediaURL string `json:"media_url"` // NEW: "/api/media/document/123"
|
||||
// ...
|
||||
}
|
||||
|
||||
type DocumentImageResponse struct {
|
||||
ID uint `json:"id"`
|
||||
MediaURL string `json:"media_url"` // "/api/media/document-image/456"
|
||||
Caption string `json:"caption"`
|
||||
}
|
||||
```
|
||||
|
||||
### Phase 3: Update Router
|
||||
|
||||
**File: `/internal/router/router.go`**
|
||||
|
||||
```go
|
||||
// REMOVE this line:
|
||||
// r.Static("/uploads", cfg.Storage.UploadDir)
|
||||
|
||||
// ADD authenticated media routes:
|
||||
media := api.Group("/media")
|
||||
media.Use(middleware.AuthRequired(cfg, userService))
|
||||
{
|
||||
media.GET("/document/:id", mediaHandler.ServeDocument)
|
||||
media.GET("/document-image/:id", mediaHandler.ServeDocumentImage)
|
||||
media.GET("/completion-image/:id", mediaHandler.ServeCompletionImage)
|
||||
}
|
||||
```
|
||||
|
||||
### Phase 4: Update Mobile Clients
|
||||
|
||||
**iOS - Update image loading to include auth headers:**
|
||||
|
||||
```swift
|
||||
// Before: Direct URL access
|
||||
AsyncImage(url: URL(string: document.fileUrl))
|
||||
|
||||
// After: Use authenticated request
|
||||
AuthenticatedImage(url: document.mediaUrl)
|
||||
|
||||
// New component that adds auth header
|
||||
struct AuthenticatedImage: View {
|
||||
let url: String
|
||||
|
||||
var body: some View {
|
||||
// Use URLSession with auth header, or
|
||||
// use a library like Kingfisher with request modifier
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**KMM - Update Ktor client for image requests:**
|
||||
|
||||
```kotlin
|
||||
// Add auth header to image requests
|
||||
suspend fun fetchMedia(mediaUrl: String): ByteArray {
|
||||
val token = TokenStorage.getToken()
|
||||
return httpClient.get(mediaUrl) {
|
||||
header("Authorization", "Token $token")
|
||||
}.body()
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Files to Modify
|
||||
|
||||
### Go API (myCribAPI-go)
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `/internal/handlers/media_handler.go` | **NEW** - Authenticated media serving |
|
||||
| `/internal/router/router.go` | Remove static serving, add media routes |
|
||||
| `/internal/dto/responses/document_response.go` | Add `MediaURL` field, remove direct file URL |
|
||||
| `/internal/dto/responses/task_response.go` | Add `MediaURL` to completion images |
|
||||
| `/internal/services/document_service.go` | Update to generate proxy URLs |
|
||||
| `/internal/services/task_service.go` | Update to generate proxy URLs for completions |
|
||||
|
||||
### iOS (MyCribKMM/iosApp)
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `iosApp/Components/AuthenticatedImage.swift` | **NEW** - Image view with auth headers |
|
||||
| `iosApp/Task/TaskCompletionDetailView.swift` | Use AuthenticatedImage |
|
||||
| `iosApp/Documents/DocumentDetailView.swift` | Use AuthenticatedImage |
|
||||
|
||||
### KMM/Android
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `network/MediaApi.kt` | **NEW** - Authenticated media fetching |
|
||||
| `ui/components/AuthenticatedImage.kt` | **NEW** - Compose image with auth |
|
||||
|
||||
---
|
||||
|
||||
## Migration Strategy
|
||||
|
||||
### For Existing Data
|
||||
|
||||
Option A: **Update URLs in database** (Recommended)
|
||||
```sql
|
||||
-- Strip /uploads prefix from existing URLs
|
||||
UPDATE documents SET file_url = REPLACE(file_url, '/uploads/', '') WHERE file_url LIKE '/uploads/%';
|
||||
UPDATE document_images SET image_url = REPLACE(image_url, '/uploads/', '') WHERE image_url LIKE '/uploads/%';
|
||||
UPDATE task_completion_images SET image_url = REPLACE(image_url, '/uploads/', '') WHERE image_url LIKE '/uploads/%';
|
||||
```
|
||||
|
||||
Option B: **Handle both formats in code** (Temporary)
|
||||
```go
|
||||
func (h *MediaHandler) getFilePath(storedURL string) string {
|
||||
// Handle legacy /uploads/... URLs
|
||||
if strings.HasPrefix(storedURL, "/uploads/") {
|
||||
return filepath.Join(h.uploadDir, strings.TrimPrefix(storedURL, "/uploads/"))
|
||||
}
|
||||
// Handle new relative paths
|
||||
return filepath.Join(h.uploadDir, storedURL)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Security Considerations
|
||||
|
||||
### Pros of This Approach
|
||||
- ✓ All media access requires valid auth token
|
||||
- ✓ Access control tied to residence membership
|
||||
- ✓ No URL guessing possible (URLs are opaque IDs)
|
||||
- ✓ Can add audit logging easily
|
||||
- ✓ Can add rate limiting per user
|
||||
|
||||
### Cons / Trade-offs
|
||||
- ✗ Every image request hits the API (increased server load)
|
||||
- ✗ Can't use CDN caching for images
|
||||
- ✗ Mobile apps need to handle auth for image loading
|
||||
|
||||
### Mitigations
|
||||
1. **Add caching headers** - `Cache-Control: private, max-age=3600`
|
||||
2. **Consider short-lived signed URLs** later if performance becomes an issue
|
||||
3. **Add response compression** for images if not already enabled
|
||||
|
||||
---
|
||||
|
||||
## Implementation Order
|
||||
|
||||
1. **Create MediaHandler** with access control logic
|
||||
2. **Update router** - add media routes, keep static for now
|
||||
3. **Update response DTOs** - add MediaURL fields
|
||||
4. **Update services** - generate proxy URLs
|
||||
5. **Test with Postman** - verify access control works
|
||||
6. **Update iOS** - AuthenticatedImage component
|
||||
7. **Update Android/KMM** - AuthenticatedImage component
|
||||
8. **Run migration** - update existing URLs in database
|
||||
9. **Remove static serving** - final step after clients updated
|
||||
10. **Add audit logging** (optional)
|
||||
Reference in New Issue
Block a user