fix: critical security hardening — tenant isolation, CORS, error leaking, input validation
1. Tenant isolation bypass (CRITICAL): TenantResolver now verifies user has access to X-Tenant-ID via user_tenants lookup before setting context. Added VerifyAccess method to TenantLookup interface and TenantService. 2. Consolidated tenant resolution: Removed duplicate resolveTenant() from helpers.go and tenant resolution from auth middleware. TenantResolver is now the single source of truth. Deadlines and AI handlers use auth.TenantFromContext() instead of direct DB queries. 3. CalDAV credential masking: tenant settings responses now mask CalDAV passwords with "********" via maskSettingsPassword helper. Applied to GetTenant, ListTenants, and UpdateSettings responses. 4. CORS + security headers: New middleware/security.go with CORS (restricted to FRONTEND_ORIGIN) and security headers (X-Frame-Options, X-Content-Type-Options, HSTS, Referrer-Policy, X-XSS-Protection). 5. Internal error leaking: All writeError(w, 500, err.Error()) replaced with internalError() that logs via slog and returns generic "internal error" to client. Same for jsonError in tenant handler. 6. Input validation: Max length on title (500), description (10000), case_number (100), search (200). Pagination clamped to max 100. Content-Disposition filename sanitized against header injection. Regression test added for tenant access denial (403 on unauthorized X-Tenant-ID). All existing tests pass, go vet clean.
This commit is contained in:
@@ -2,12 +2,12 @@ package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"strings"
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/jmoiron/sqlx"
|
||||
|
||||
"mgit.msbls.de/m/KanzlAI-mGMT/internal/auth"
|
||||
)
|
||||
|
||||
func writeJSON(w http.ResponseWriter, status int, v any) {
|
||||
@@ -20,62 +20,9 @@ func writeError(w http.ResponseWriter, status int, msg string) {
|
||||
writeJSON(w, status, map[string]string{"error": msg})
|
||||
}
|
||||
|
||||
// resolveTenant gets the tenant ID for the authenticated user.
|
||||
// Checks X-Tenant-ID header first, then falls back to user's first tenant.
|
||||
func resolveTenant(r *http.Request, db *sqlx.DB) (uuid.UUID, error) {
|
||||
userID, ok := auth.UserFromContext(r.Context())
|
||||
if !ok {
|
||||
return uuid.Nil, errUnauthorized
|
||||
}
|
||||
|
||||
// Check header first
|
||||
if headerVal := r.Header.Get("X-Tenant-ID"); headerVal != "" {
|
||||
tenantID, err := uuid.Parse(headerVal)
|
||||
if err != nil {
|
||||
return uuid.Nil, errInvalidTenant
|
||||
}
|
||||
// Verify user has access to this tenant
|
||||
var count int
|
||||
err = db.Get(&count,
|
||||
`SELECT COUNT(*) FROM user_tenants WHERE user_id = $1 AND tenant_id = $2`,
|
||||
userID, tenantID)
|
||||
if err != nil || count == 0 {
|
||||
return uuid.Nil, errTenantAccess
|
||||
}
|
||||
return tenantID, nil
|
||||
}
|
||||
|
||||
// Fall back to user's first tenant
|
||||
var tenantID uuid.UUID
|
||||
err := db.Get(&tenantID,
|
||||
`SELECT tenant_id FROM user_tenants WHERE user_id = $1 ORDER BY created_at LIMIT 1`,
|
||||
userID)
|
||||
if err != nil {
|
||||
return uuid.Nil, errNoTenant
|
||||
}
|
||||
return tenantID, nil
|
||||
}
|
||||
|
||||
type apiError struct {
|
||||
msg string
|
||||
status int
|
||||
}
|
||||
|
||||
func (e *apiError) Error() string { return e.msg }
|
||||
|
||||
var (
|
||||
errUnauthorized = &apiError{msg: "unauthorized", status: http.StatusUnauthorized}
|
||||
errInvalidTenant = &apiError{msg: "invalid tenant ID", status: http.StatusBadRequest}
|
||||
errTenantAccess = &apiError{msg: "no access to tenant", status: http.StatusForbidden}
|
||||
errNoTenant = &apiError{msg: "no tenant found for user", status: http.StatusBadRequest}
|
||||
)
|
||||
|
||||
// handleTenantError writes the appropriate error response for tenant resolution errors
|
||||
func handleTenantError(w http.ResponseWriter, err error) {
|
||||
if ae, ok := err.(*apiError); ok {
|
||||
writeError(w, ae.status, ae.msg)
|
||||
return
|
||||
}
|
||||
// internalError logs the real error and returns a generic message to the client.
|
||||
func internalError(w http.ResponseWriter, msg string, err error) {
|
||||
slog.Error(msg, "error", err)
|
||||
writeError(w, http.StatusInternalServerError, "internal error")
|
||||
}
|
||||
|
||||
@@ -88,3 +35,74 @@ func parsePathUUID(r *http.Request, key string) (uuid.UUID, error) {
|
||||
func parseUUID(s string) (uuid.UUID, error) {
|
||||
return uuid.Parse(s)
|
||||
}
|
||||
|
||||
// --- Input validation helpers ---
|
||||
|
||||
const (
|
||||
maxTitleLen = 500
|
||||
maxDescriptionLen = 10000
|
||||
maxCaseNumberLen = 100
|
||||
maxSearchLen = 200
|
||||
maxPaginationLimit = 100
|
||||
)
|
||||
|
||||
// validateStringLength checks if a string exceeds the given max length.
|
||||
func validateStringLength(field, value string, maxLen int) string {
|
||||
if utf8.RuneCountInString(value) > maxLen {
|
||||
return field + " exceeds maximum length"
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// clampPagination enforces sane pagination defaults and limits.
|
||||
func clampPagination(limit, offset int) (int, int) {
|
||||
if limit <= 0 {
|
||||
limit = 20
|
||||
}
|
||||
if limit > maxPaginationLimit {
|
||||
limit = maxPaginationLimit
|
||||
}
|
||||
if offset < 0 {
|
||||
offset = 0
|
||||
}
|
||||
return limit, offset
|
||||
}
|
||||
|
||||
// sanitizeFilename removes characters unsafe for Content-Disposition headers.
|
||||
func sanitizeFilename(name string) string {
|
||||
// Remove control characters, quotes, and backslashes
|
||||
var b strings.Builder
|
||||
for _, r := range name {
|
||||
if r < 32 || r == '"' || r == '\\' || r == '/' {
|
||||
b.WriteRune('_')
|
||||
} else {
|
||||
b.WriteRune(r)
|
||||
}
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
|
||||
// maskSettingsPassword masks the CalDAV password in tenant settings JSON before returning to clients.
|
||||
func maskSettingsPassword(settings json.RawMessage) json.RawMessage {
|
||||
if len(settings) == 0 {
|
||||
return settings
|
||||
}
|
||||
var m map[string]json.RawMessage
|
||||
if err := json.Unmarshal(settings, &m); err != nil {
|
||||
return settings
|
||||
}
|
||||
caldavRaw, ok := m["caldav"]
|
||||
if !ok {
|
||||
return settings
|
||||
}
|
||||
var caldav map[string]json.RawMessage
|
||||
if err := json.Unmarshal(caldavRaw, &caldav); err != nil {
|
||||
return settings
|
||||
}
|
||||
if _, ok := caldav["password"]; ok {
|
||||
caldav["password"], _ = json.Marshal("********")
|
||||
}
|
||||
m["caldav"], _ = json.Marshal(caldav)
|
||||
result, _ := json.Marshal(m)
|
||||
return result
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user