From cab5a79f6d070418215835ae320511bef8370f49 Mon Sep 17 00:00:00 2001 From: Sandro Date: Thu, 29 Oct 2026 15:05:05 +0100 Subject: [PATCH] refactor(storage): one definition of task list access (FLAW-20) accessibleTaskListsSQL in tasklists.go defines which task lists a user can see; GetTaskLists, GetTaskListByID and the task queries use it instead of three copies of the same subquery. Co-Authored-By: Claude Opus 5.5 --- Doku/flaws.md | 7 ++++--- internal/storage/tasklists.go | 37 +++++++++++++++++++---------------- internal/storage/thoughts.go | 8 +------- 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/Doku/flaws.md b/Doku/flaws.md index 817b2e4..c948352 100644 --- a/Doku/flaws.md +++ b/Doku/flaws.md @@ -28,8 +28,8 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** ( | FLAW-16 | Setup | `/auth/setup` ist bis zum ersten Konto offen; Demo-Modus legt `admin/admin` an | Mittel | | FLAW-17 | Workspace | ~~Bearbeiter-Freigaben für Notizen nicht wählbar~~ | **Behoben** | | FLAW-18 | Suche | ~~Nur eigene Gedanken und Dokumente, `LIKE` ohne Escaping, keine FTS~~ | **Behoben** | -| FLAW-19 | Qualität | Frontend-Tests nur für den Outliner, `lint` ist ein Platzhalter | Niedrig | -| FLAW-20 | Qualität | ~~Sehr große Dateien~~; duplizierte Zugriffslogik | Niedrig | +| FLAW-19 | Qualität | ~~Frontend-Tests nur für den Outliner, `lint` ist ein Platzhalter~~ | **Behoben** | +| FLAW-20 | Qualität | ~~Große Go-Dateien, duplizierte Zugriffslogik~~; große Frontend-Dateien | Niedrig | | FLAW-21 | Repo | ~~`LICENSE` fehlt, obwohl README MIT angibt~~ | **Behoben** | | FLAW-22 | Netzwerk | ~~Token in der URL (WS/SSE), CORS `*`, `CheckOrigin` immer `true`~~ | **Behoben** | | FLAW-23 | SQLite | ~~`SQLITE_BUSY` bei vielen gleichzeitigen Schreibern~~ | **Behoben** | @@ -162,12 +162,13 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** ( - **Fundstellen:** `internal/storage/search.go`, `internal/api/server.go`, `frontend/src/components/CommandPalette.tsx`. ### FLAW-19: Geringe Frontend-Testabdeckung +- **Status: Behoben.** `npm run lint` ist echtes ESLint (`eslint.config.js`: empfohlene JS-/TypeScript-Regeln und React-Hook-Regeln) und läuft im Image-Build; Fehler brechen ihn ab, 24 `exhaustive-deps`-Hinweise bleiben Warnungen. ESLint fand zwei echte Fehler: `TaskDetailModal` und `ManageColumnsModal` riefen Hooks nach `if (!isOpen) return null` auf. Neue Tests: Aufgaben-Dialog (Öffnen/Schließen, Datum/Uhrzeit, Lesemodus), Personen-Stapel geteilter Inhalte, Papierkorb, API-Key-Scopes, Passwortregel, Offline-Warteschlange, Fälligkeiten (insgesamt 90 Frontend-Tests). - **Stand:** Vitest mit jsdom und Testing Library ist eingerichtet (`npm run test:run`) und läuft beim Image-Build. Getestet sind bisher nur Outliner-Funktionen (Backspace-Zusammenführen, Cursor-Position beim Klicken). - **Problem:** Aufgaben, Listen, Freigabe-Dialoge und Berechtigungsanzeigen sind ungetestet. `"lint"` in [`frontend/package.json`](../frontend/package.json) ist weiterhin ein Platzhalter (`echo`). - **Maßnahme:** Tests für kritische Pfade ergänzen, ESLint als echtes `lint`-Skript. ### FLAW-20: Dateigröße und Duplikate -- **Teilweise behoben:** `sqlite.go` und `server.go` sind nach Bereichen in eigene Dateien aufgeteilt (reine Verschiebung, Verhalten unverändert; siehe [architecture.md](architecture.md#verzeichnisse)). Offen: große Frontend-Dateien und die doppelte Zugriffsabfrage. +- **Teilweise behoben:** `sqlite.go` und `server.go` sind nach Bereichen in eigene Dateien aufgeteilt (reine Verschiebung, Verhalten unverändert; siehe [architecture.md](architecture.md#verzeichnisse)). Der Lesezugriff auf Aufgabenlisten ist an einer Stelle definiert (`accessibleTaskListsSQL` in `tasklists.go`), die drei Kopien sind weg. Offen: `WorkspaceView.tsx` (≈ 2.200 Zeilen), `TasksView.tsx` (≈ 2.400) und `api.ts` (≈ 1.600) sind weiterhin groß. - `internal/storage/sqlite.go` (≈ 5.600 Zeilen), `internal/api/server.go` (≈ 3.000), `WorkspaceView.tsx` (≈ 2.300), `TasksView.tsx` (≈ 2.500), `api.ts` (≈ 1.500). - Die Zugriffsabfrage für Aufgabenlisten steht dreimal fast identisch im Code (`accessibleTaskListIDsQuery`, `GetTaskLists`, `GetTaskListByID`). Dokumentzugriff ist ähnlich verteilt. Änderungen an Berechtigungen müssen an mehreren Stellen erfolgen und werden leicht vergessen. - **Maßnahme:** In Teilmodule zerlegen (Auth, Dokumente, Aufgaben, Listen, Teams), Zugriffsprüfung zentralisieren. diff --git a/internal/storage/tasklists.go b/internal/storage/tasklists.go index aa15319..48b846a 100644 --- a/internal/storage/tasklists.go +++ b/internal/storage/tasklists.go @@ -214,6 +214,21 @@ func (s *Store) queryUserIDs(query string, args ...interface{}) []string { return ids } +// accessibleTaskListsSQL selects the IDs of every task list a user can see: own lists and lists shared +// with the user directly or through a group. It takes the user ID three times (accessibleTaskListArgs). +// This is the one definition of task list access for reading; GetTaskListPermission adds the role. +const accessibleTaskListsSQL = `SELECT id FROM task_lists + WHERE user_id = ? + OR id IN ( + SELECT task_list_id FROM task_list_shares + WHERE (target_type = 'user' AND target_id = ?) + OR (target_type = 'team' AND target_id IN (SELECT team_id FROM team_members WHERE user_id = ?)) + )` + +func accessibleTaskListArgs(userID string) []interface{} { + return []interface{}{userID, userID, userID} +} + // taskListAccessSQL selects every user ID with access to a task list: owner, direct shares and members of // shared groups. %[1]s is the list ID expression ("?" or a correlated column). const taskListAccessSQL = `SELECT user_id FROM task_lists WHERE id = %[1]s @@ -412,14 +427,9 @@ func (s *Store) GetTaskLists(userID string) ([]domain.TaskList, error) { rows, err := s.db.Query( `SELECT id, user_id, space_id, title, icon, color, COALESCE(columns, '[]'), created_at, updated_at FROM task_lists - WHERE user_id = ? - OR id IN ( - SELECT task_list_id FROM task_list_shares - WHERE (target_type = 'user' AND target_id = ?) - OR (target_type = 'team' AND target_id IN (SELECT team_id FROM team_members WHERE user_id = ?)) - ) + WHERE id IN (`+accessibleTaskListsSQL+`) ORDER BY updated_at DESC`, - userID, userID, userID, + accessibleTaskListArgs(userID)..., ) if err != nil { return []domain.TaskList{}, err @@ -470,16 +480,9 @@ func (s *Store) GetTaskListByID(userID, listID string) (*domain.TaskList, error) var colsJSON string err := s.db.QueryRow( `SELECT id, user_id, space_id, title, icon, color, COALESCE(columns, '[]'), created_at, updated_at - FROM task_lists - WHERE id = ? AND ( - user_id = ? - OR id IN ( - SELECT task_list_id FROM task_list_shares - WHERE (target_type = 'user' AND target_id = ?) - OR (target_type = 'team' AND target_id IN (SELECT team_id FROM team_members WHERE user_id = ?)) - ) - )`, - listID, userID, userID, userID, + FROM task_lists + WHERE id = ? AND id IN (`+accessibleTaskListsSQL+`)`, + append([]interface{}{listID}, accessibleTaskListArgs(userID)...)..., ).Scan(&tl.ID, &tl.UserID, &tl.SpaceID, &tl.Title, &tl.Icon, &tl.Color, &colsJSON, &tl.CreatedAt, &tl.UpdatedAt) if err != nil { return nil, err diff --git a/internal/storage/thoughts.go b/internal/storage/thoughts.go index 7c20779..184cb22 100644 --- a/internal/storage/thoughts.go +++ b/internal/storage/thoughts.go @@ -131,13 +131,7 @@ func scanThought(rows interface { } func (s *Store) accessibleTaskListIDsQuery() string { - return `SELECT id FROM task_lists - WHERE user_id = ? - OR id IN ( - SELECT task_list_id FROM task_list_shares - WHERE (target_type = 'user' AND target_id = ?) - OR (target_type = 'team' AND target_id IN (SELECT team_id FROM team_members WHERE user_id = ?)) - )` + return accessibleTaskListsSQL } func (s *Store) GetThoughtByID(userID, thoughtID string) (*domain.Thought, error) {