fix(security): API keys only deliver data; one auth check everywhere
Until API keys get scopes they have exactly one right: POST /api/v1/inbox (thought or task via target) and the webhook ingest; every other endpoint answers 403 for a key. Middleware and webhook ingest share one token check (revocation list, token_version, active account), closing FLAW-08. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
62463bc6fd
commit
300fd6a7be
4 files changed
+99
-49
No files matched your search
+1
-1
@@ -1,6 +1,6 @@
|
|||||||
# REST-API
|
# REST-API
|
||||||
|
|
||||||
Basis: `/api/v1`. Antworten sind JSON; Fehler haben die Form `{"error": "Text"}`. Authentifizierung über `Authorization: Bearer <JWT>` oder `Bearer mindsift_sec_…` bzw. Header `X-API-Key`. Zeitstempel sind ISO-8601 (UTC). Quelle der Wahrheit sind die Routen in [server.go](../../internal/api/server.go).
|
Basis: `/api/v1`. Antworten sind JSON; Fehler haben die Form `{"error": "Text"}`. Authentifizierung über `Authorization: Bearer <JWT>`. **API-Keys** (`Bearer mindsift_sec_…` bzw. Header `X-API-Key`) haben vorerst genau ein Recht: Daten einliefern über `POST /inbox` (Gedanke oder per `target` direkt Aufgabe) und `POST /webhooks/ingest`; alle anderen Endpunkte antworten mit `403`. Feinere Berechtigungen (Scopes) folgen. Zeitstempel sind ISO-8601 (UTC). Quelle der Wahrheit sind die Routen in [server.go](../../internal/api/server.go).
|
||||||
|
|
||||||
## Öffentlich
|
## Öffentlich
|
||||||
|
|
||||||
|
|||||||
+4
-2
@@ -16,8 +16,8 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** (
|
|||||||
| FLAW-04 | Auth | ~~Fest eincodiertes Fallback-`JWT_SECRET`~~ | **Behoben** |
|
| FLAW-04 | Auth | ~~Fest eincodiertes Fallback-`JWT_SECRET`~~ | **Behoben** |
|
||||||
| FLAW-05 | Benachrichtigungen | ~~Discord-/Webhook-URLs ungeprüft (SSRF)~~ | **Behoben** |
|
| FLAW-05 | Benachrichtigungen | ~~Discord-/Webhook-URLs ungeprüft (SSRF)~~ | **Behoben** |
|
||||||
| FLAW-06 | Aufgaben | ~~Jeder Editor einer Aufgabenliste kann sie per `teamId` an weitere Gruppen freigeben~~ | **Behoben** |
|
| FLAW-06 | Aufgaben | ~~Jeder Editor einer Aufgabenliste kann sie per `teamId` an weitere Gruppen freigeben~~ | **Behoben** |
|
||||||
| FLAW-07 | Auth | API-Key-`scopes` werden nie ausgewertet; ein Key ist ein Vollzugriff | Mittel |
|
| FLAW-07 | Auth | ~~Ein API-Key ist ein Vollzugriff~~ – Keys dürfen nur noch einliefern; Scopes fehlen noch | Niedrig |
|
||||||
| FLAW-08 | Auth | Webhook-Ingest mit JWT prüft weder Sperrliste noch `token_version` noch `is_active` | Mittel |
|
| FLAW-08 | Auth | ~~Webhook-Ingest mit JWT prüft weder Sperrliste noch `token_version` noch `is_active`~~ | **Behoben** |
|
||||||
| FLAW-09 | Auth | Kein Rate-Limit, keine Sperre bei Fehlversuchen, Mindestpasswort 4 Zeichen | Mittel |
|
| FLAW-09 | Auth | Kein Rate-Limit, keine Sperre bei Fehlversuchen, Mindestpasswort 4 Zeichen | Mittel |
|
||||||
| FLAW-10 | Freigaben | ~~`sharing_mode` wird bei Freigaben nicht durchgesetzt; Benutzerverzeichnis leakt alle Konten~~ | **Behoben** |
|
| FLAW-10 | Freigaben | ~~`sharing_mode` wird bei Freigaben nicht durchgesetzt; Benutzerverzeichnis leakt alle Konten~~ | **Behoben** |
|
||||||
| FLAW-11 | Workspace | ~~Block-Upsert (`ON CONFLICT(id)`) prüft das Dokument nicht~~ | **Behoben** |
|
| FLAW-11 | Workspace | ~~Block-Upsert (`ON CONFLICT(id)`) prüft das Dokument nicht~~ | **Behoben** |
|
||||||
@@ -78,11 +78,13 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** (
|
|||||||
- **Maßnahme:** `teamId`-Änderungen nur für den Eigentümer; das Legacy-Feld `task_lists.team_id` zugunsten von `task_list_shares` abschaffen.
|
- **Maßnahme:** `teamId`-Änderungen nur für den Eigentümer; das Legacy-Feld `task_lists.team_id` zugunsten von `task_list_shares` abschaffen.
|
||||||
|
|
||||||
### FLAW-07: API-Key-Scopes ohne Wirkung
|
### FLAW-07: API-Key-Scopes ohne Wirkung
|
||||||
|
- **Teilweise behoben:** API-Keys dürfen nur noch `POST /inbox` und `POST /webhooks/ingest` nutzen (`apiKeyAllowed` in `server.go`), alles andere gibt `403`. Auswählbare Scopes (Triage, Aufgaben, Listen) stehen noch aus.
|
||||||
- **Fundstellen:** Tabelle `api_keys.scopes` (Standard `ingest,read`), `authMiddleware`.
|
- **Fundstellen:** Tabelle `api_keys.scopes` (Standard `ingest,read`), `authMiddleware`.
|
||||||
- **Problem:** Ein Key mit Präfix `mindsift_sec_` authentifiziert als der Benutzer, mit allen Rechten: alle `/api/v1`-Routen, bei Admins auch `/admin/*`, außerdem das Anlegen weiterer Keys. Für Webhooks (nur Aufgaben anlegen) ist das überdimensioniert.
|
- **Problem:** Ein Key mit Präfix `mindsift_sec_` authentifiziert als der Benutzer, mit allen Rechten: alle `/api/v1`-Routen, bei Admins auch `/admin/*`, außerdem das Anlegen weiterer Keys. Für Webhooks (nur Aufgaben anlegen) ist das überdimensioniert.
|
||||||
- **Maßnahme:** Scopes auswerten; Keys standardmäßig nur auf `/inbox` und `/webhooks/ingest` beschränken.
|
- **Maßnahme:** Scopes auswerten; Keys standardmäßig nur auf `/inbox` und `/webhooks/ingest` beschränken.
|
||||||
|
|
||||||
### FLAW-08: Webhook-Ingest umgeht Sitzungsprüfung
|
### FLAW-08: Webhook-Ingest umgeht Sitzungsprüfung
|
||||||
|
- **Status: Behoben.** Middleware und Webhook-Ingest nutzen dieselbe Prüfung (`authenticateToken`): Sperrliste, `token_version`, aktives Konto. Test: `TestAPIKeysOnlyDeliverData`.
|
||||||
- **Fundstelle:** `handleWebhookIngest`, JWT-Zweig.
|
- **Fundstelle:** `handleWebhookIngest`, JWT-Zweig.
|
||||||
- **Problem:** Das Token wird nur kryptografisch validiert. `IsTokenRevoked`, `token_version` und `is_active` werden nicht geprüft; ein ausgeloggtes oder deaktiviertes Konto kann weiter Aufgaben anlegen (bis Token-Ablauf nach 30 Tagen).
|
- **Problem:** Das Token wird nur kryptografisch validiert. `IsTokenRevoked`, `token_version` und `is_active` werden nicht geprüft; ein ausgeloggtes oder deaktiviertes Konto kann weiter Aufgaben anlegen (bis Token-Ablauf nach 30 Tagen).
|
||||||
- **Maßnahme:** Dieselbe Prüfkette wie in `authMiddleware` verwenden (gemeinsame Funktion statt Kopien; siehe auch `authenticateUserFromReq`).
|
- **Maßnahme:** Dieselbe Prüfkette wie in `authMiddleware` verwenden (gemeinsame Funktion statt Kopien; siehe auch `authenticateUserFromReq`).
|
||||||
|
|||||||
+51
-46
@@ -368,44 +368,58 @@ func (s *Server) apiLoggingMiddleware(next http.Handler) http.Handler {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// authenticateToken resolves a session JWT or an API key (mindsift_sec_…) to an active user. JWTs are
|
||||||
|
// checked against the revocation list and token_version; viaAPIKey tells the caller which it was.
|
||||||
|
func (s *Server) authenticateToken(token string) (user *domain.User, viaAPIKey bool) {
|
||||||
|
token = strings.TrimSpace(token)
|
||||||
|
if token == "" {
|
||||||
|
return nil, false
|
||||||
|
}
|
||||||
|
if strings.HasPrefix(token, "mindsift_sec_") {
|
||||||
|
u, err := s.store.ValidateAPIKeyAndGetUser(token)
|
||||||
|
if err != nil || u == nil || !u.IsActive {
|
||||||
|
return nil, true
|
||||||
|
}
|
||||||
|
return u, true
|
||||||
|
}
|
||||||
|
claims, err := auth.ValidateToken(s.cfg.JWTSecret, token)
|
||||||
|
if err != nil || claims == nil || s.store.IsTokenRevoked(token) {
|
||||||
|
return nil, false
|
||||||
|
}
|
||||||
|
u, err := s.store.GetUserByID(claims.UserID)
|
||||||
|
if err != nil || u == nil || !u.IsActive {
|
||||||
|
return nil, false
|
||||||
|
}
|
||||||
|
if claims.TokenVersion != 0 && claims.TokenVersion != u.TokenVersion {
|
||||||
|
return nil, false
|
||||||
|
}
|
||||||
|
return u, false
|
||||||
|
}
|
||||||
|
|
||||||
|
// apiKeyAllowed: until API keys get scopes, a key has exactly one right – delivering data into MindSift
|
||||||
|
// (POST /api/v1/inbox; /api/v1/webhooks/ingest authenticates on its own). Everything else needs a login.
|
||||||
|
func apiKeyAllowed(r *http.Request) bool {
|
||||||
|
return r.Method == http.MethodPost && r.URL.Path == "/api/v1/inbox"
|
||||||
|
}
|
||||||
|
|
||||||
func (s *Server) authMiddleware(next http.Handler) http.Handler {
|
func (s *Server) authMiddleware(next http.Handler) http.Handler {
|
||||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
// 1. Check Authorization Bearer Token
|
token := ""
|
||||||
authHeader := r.Header.Get("Authorization")
|
if authHeader := r.Header.Get("Authorization"); strings.HasPrefix(authHeader, "Bearer ") {
|
||||||
if strings.HasPrefix(authHeader, "Bearer ") {
|
token = strings.TrimPrefix(authHeader, "Bearer ")
|
||||||
tokenStr := strings.TrimPrefix(authHeader, "Bearer ")
|
} else {
|
||||||
if strings.HasPrefix(tokenStr, "mindsift_sec_") {
|
token = r.Header.Get("X-API-Key")
|
||||||
user, err := s.store.ValidateAPIKeyAndGetUser(tokenStr)
|
|
||||||
if err == nil && user != nil && user.IsActive {
|
|
||||||
next.ServeHTTP(w, r.WithContext(auth.WithUserID(r.Context(), user.ID)))
|
|
||||||
return
|
|
||||||
}
|
}
|
||||||
}
|
user, viaAPIKey := s.authenticateToken(token)
|
||||||
claims, err := auth.ValidateToken(s.cfg.JWTSecret, tokenStr)
|
if user == nil {
|
||||||
if err == nil && claims != nil {
|
|
||||||
if !s.store.IsTokenRevoked(tokenStr) {
|
|
||||||
user, userErr := s.store.GetUserByID(claims.UserID)
|
|
||||||
if userErr == nil && user != nil && user.IsActive {
|
|
||||||
if claims.TokenVersion == 0 || claims.TokenVersion == user.TokenVersion {
|
|
||||||
next.ServeHTTP(w, r.WithContext(auth.WithUserID(r.Context(), user.ID)))
|
|
||||||
return
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// 2. Check X-API-Key Header
|
|
||||||
apiKey := r.Header.Get("X-API-Key")
|
|
||||||
if apiKey != "" {
|
|
||||||
user, err := s.store.ValidateAPIKeyAndGetUser(apiKey)
|
|
||||||
if err == nil && user != nil && user.IsActive {
|
|
||||||
next.ServeHTTP(w, r.WithContext(auth.WithUserID(r.Context(), user.ID)))
|
|
||||||
return
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
writeError(w, http.StatusUnauthorized, "Nicht autorisiert – bitte anmelden")
|
writeError(w, http.StatusUnauthorized, "Nicht autorisiert – bitte anmelden")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if viaAPIKey && !apiKeyAllowed(r) {
|
||||||
|
writeError(w, http.StatusForbidden, "API-Keys dürfen nur Daten einliefern (POST /api/v1/inbox oder /api/v1/webhooks/ingest)")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
next.ServeHTTP(w, r.WithContext(auth.WithUserID(r.Context(), user.ID)))
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1983,18 +1997,9 @@ func (s *Server) handleWebhookIngest(w http.ResponseWriter, r *http.Request) {
|
|||||||
token = strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer ")
|
token = strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer ")
|
||||||
}
|
}
|
||||||
|
|
||||||
var user *domain.User
|
// Same checks as every other request (revoked/outdated sessions, inactive accounts)
|
||||||
var err error
|
user, _ := s.authenticateToken(token)
|
||||||
|
if user == nil {
|
||||||
if strings.HasPrefix(token, "mindsift_sec_") {
|
|
||||||
user, err = s.store.ValidateAPIKeyAndGetUser(token)
|
|
||||||
} else if token != "" {
|
|
||||||
if claims, jwtErr := auth.ValidateToken(s.cfg.JWTSecret, token); jwtErr == nil {
|
|
||||||
user, err = s.store.GetUserByID(claims.UserID)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if err != nil || user == nil {
|
|
||||||
writeError(w, http.StatusUnauthorized, "Ungültiger Webhook-Token oder API-Key")
|
writeError(w, http.StatusUnauthorized, "Ungültiger Webhook-Token oder API-Key")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1406,3 +1406,46 @@ func TestLegalTextsArePublic(t *testing.T) {
|
|||||||
t.Fatalf("privacy without text should be 404, got %d", rec.Code)
|
t.Fatalf("privacy without text should be 404, got %d", rec.Code)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// API keys have exactly one right until scopes exist: delivering data. Revoked sessions are refused
|
||||||
|
// everywhere, the webhook ingest included.
|
||||||
|
func TestAPIKeysOnlyDeliverData(t *testing.T) {
|
||||||
|
srv, store, token := setupTestServer(t)
|
||||||
|
router := srv.Router()
|
||||||
|
claims, _ := auth.ValidateToken(testJWTSecret, token)
|
||||||
|
key, err := store.CreateAPIKey(claims.UserID, "Home Assistant")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateAPIKey: %v", err)
|
||||||
|
}
|
||||||
|
do := func(method, path, auth, body string) int {
|
||||||
|
req := httptest.NewRequest(method, path, strings.NewReader(body))
|
||||||
|
req.Header.Set("Content-Type", "application/json")
|
||||||
|
if strings.HasPrefix(auth, "mindsift_sec_") {
|
||||||
|
req.Header.Set("X-API-Key", auth)
|
||||||
|
} else {
|
||||||
|
req.Header.Set("Authorization", "Bearer "+auth)
|
||||||
|
}
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
router.ServeHTTP(rec, req)
|
||||||
|
return rec.Code
|
||||||
|
}
|
||||||
|
if code := do(http.MethodPost, "/api/v1/inbox", key.RawKey, `{"content":"Waschmaschine fertig"}`); code >= 300 {
|
||||||
|
t.Fatalf("API key must be able to post into the inbox, got %d", code)
|
||||||
|
}
|
||||||
|
if code := do(http.MethodPost, "/api/v1/inbox", key.RawKey, `{"content":"Filter wechseln","target":"task"}`); code >= 300 {
|
||||||
|
t.Fatalf("API key must be able to create a task via the inbox, got %d", code)
|
||||||
|
}
|
||||||
|
if code := do(http.MethodGet, "/api/v1/documents", key.RawKey, ""); code != http.StatusForbidden {
|
||||||
|
t.Fatalf("API key must not read notes, got %d", code)
|
||||||
|
}
|
||||||
|
if code := do(http.MethodPost, "/api/v1/webhooks/ingest", key.RawKey, `{"content":"Rauchmelder"}`); code >= 300 {
|
||||||
|
t.Fatalf("API key must work on the webhook ingest, got %d", code)
|
||||||
|
}
|
||||||
|
// Logged-out session token: refused by the ingest as well (was FLAW-08)
|
||||||
|
if code := do(http.MethodPost, "/api/v1/auth/logout", token, ""); code >= 300 {
|
||||||
|
t.Fatalf("logout: %d", code)
|
||||||
|
}
|
||||||
|
if code := do(http.MethodPost, "/api/v1/webhooks/ingest", token, `{"content":"x"}`); code != http.StatusUnauthorized {
|
||||||
|
t.Fatalf("revoked token must be refused by the ingest, got %d", code)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user