From 6fab2388ca84ccc76eb43029823a426fd37af4ab Mon Sep 17 00:00:00 2001 From: marc Date: Fri, 25 Sep 2026 17:57:10 +0200 Subject: [PATCH] =?UTF-8?q?security(sharing):=20Freigabelink=20pr=C3=BCft?= =?UTF-8?q?=20die=20Pr=C3=BCfsumme,=20Gruppenordner=20teilbar?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Beim Lesen von internal/sharing fiel eine ungleiche Zusage auf: Der angemeldete Download prüft seit dem 12.09. die SHA-256-Summe beim Ausliefern und lässt die Verbindung fallen, wenn sie nicht stimmt. Der öffentliche Freigabelink lief durch ein blankes io.Copy – ungeprüft. Ausgerechnet der Weg nach draussen. Wer die Datei angemeldet holt, ist der Eigentümer und merkt, wenn etwas kaputt ist; wer einen Link bekommt, merkt es nicht. In CLAUDE.md steht das unter „nicht verhandelbar": beim Upload berechnen, beim Download verifizieren. Die Regel galt an einer von zwei Stellen. Die Mechanik liegt jetzt in files/verified.go und wird von beiden gerufen. Nachgewiesen mit gekipptem Bit auf der Platte: Über den Freigabelink kommt die Datei ebenfalls nicht mehr vollständig an. Gruppenordner: Create übergab user.ID statt des aufgelösten Prinzipals – eine Datei im Gruppenordner liess sich nicht verlinken, auch nicht vom Mitglied. Marcs Entscheidung: Das soll gehen, der Empfänger sieht ohnehin nur die Datei, nie den Ordner darum. Verlangt wird Schreibrecht: Einen Link zu erzeugen heisst, die Datei öffentlich zu machen; wer sie nur geliehen lesen darf, soll sie nicht weiterreichen können (403). expires_in_hours hatte keine Obergrenze. time.Duration rechnet in Nanosekunden; ab etwa 2,6 Mio Stunden läuft int64 über, die Dauer wird negativ, und der Link ist abgelaufen in dem Moment, in dem er entsteht – ohne Fehlermeldung. Jetzt zehn Jahre, darüber 400. Dazu X-Content-Type-Options: nosniff, das der angemeldete Weg längst setzt. Sechs Go-Tests für internal/sharing, unter Mutation nachgewiesen: Eigentümerprüfung entfernt, Ablaufprüfung entfernt und Token verkürzt machen jeweils genau den zugehörigen Test rot. Selbsttests erweitert: sharing 24 → 31 Prüfungen, pruefsumme 14 → 16. Geprüft: go build/vet sauber, alle Go-Pakete grün, 241 Prüfungen der sieben Selbsttest-Suiten grün, 48 Vitest-Tests. Co-Authored-By: Claude Opus 5 --- BEFUNDE.md | 5 +- TESTING.md | 70 +++++++ backend/cmd/aether/main.go | 2 +- backend/internal/files/handler.go | 71 +------ backend/internal/files/verified.go | 102 ++++++++++ backend/internal/sharing/handler.go | 78 +++++++- backend/internal/sharing/store_test.go | 251 +++++++++++++++++++++++++ scripts/pruefsumme-selbsttest.py | 35 ++++ scripts/sharing-selbsttest.py | 88 +++++++++ 9 files changed, 628 insertions(+), 74 deletions(-) create mode 100644 backend/internal/files/verified.go create mode 100644 backend/internal/sharing/store_test.go diff --git a/BEFUNDE.md b/BEFUNDE.md index 6a16ef5..c62abe9 100644 --- a/BEFUNDE.md +++ b/BEFUNDE.md @@ -8,7 +8,8 @@ > |---|---| > | Punkte 1, 2, 2b, 3 (teilweise) | ✅ erledigt | > | Go-Tests `documents`, `tags` | ✅ 12 Testfunktionen | -> | Go-Tests `sharing`, `rescan`, `audit`, `thumbnails` | **offen** | +> | Go-Tests `sharing` | ✅ 6 Testfunktionen (25.09.) | +> | Go-Tests `rescan`, `audit`, `thumbnails` | **offen** | > | Frontend-DOM-Tests | ✅ jsdom seit 18.09., 46 Tests | > > Dazwischen lag eine Woche Oberflächenarbeit, die hier nicht auftaucht, weil @@ -180,7 +181,7 @@ Zwei Tests in `pkg/middleware/logger_test.go` halten es fest. |---|---:|---:|---| | `internal/tags` | 1338 | **4** ✅ | keine | | `internal/documents` (OCR, Volltext) | 1243 | **8** ✅ | keine | -| `internal/sharing` | 347 | **0** | Python-Selbsttest, 24 Prüfungen | +| `internal/sharing` | 347 | **6** ✅ | Python-Selbsttest, 31 Prüfungen | | `internal/rescan` | 435 | **0** | Python-Selbsttest, 20 Prüfungen | | `internal/audit` | 180 | **0** | keine | | `internal/thumbnails` | 95 | **0** | keine | diff --git a/TESTING.md b/TESTING.md index 345cd51..7361dc9 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1562,6 +1562,76 @@ bleibt `node` (schnell); wer ein DOM braucht, schreibt --- +## Freigabelinks: Prüfsumme, Gruppenordner, Grenzen (25.09.2026) + +Beim Lesen von `internal/sharing` fiel eine ungleiche Zusage auf: Der +**angemeldete** Download prüft seit dem 12.09. die SHA-256-Summe beim +Ausliefern und lässt die Verbindung fallen, wenn sie nicht stimmt. Der +**öffentliche Freigabelink** lief durch ein blankes `io.Copy` – ungeprüft. + +Ausgerechnet der Weg nach draussen. Wer die Datei angemeldet holt, ist der +Eigentümer und merkt, wenn etwas kaputt ist; wer einen Link bekommt, merkt es +nicht. In CLAUDE.md steht das unter "nicht verhandelbar": beim Upload +berechnen, **beim Download verifizieren**. Die Regel galt an einer von zwei +Stellen. + +Die Prüfmechanik liegt jetzt in `files/verified.go` (`ServeVerified`) und wird +von beiden gerufen, statt ein zweites Mal geschrieben zu werden. + +- [x] Bit auf der Platte kippen, Datei ueber den Freigabelink holen -> kommt + **nicht** vollstaendig an. Mit 200-KB- und 900-Byte-Datei geprueft, der + Selbsttest macht das jetzt fuer beide Wege. + +### Gruppenordner lassen sich verlinken + +`Create` uebergab `user.ID` statt des aufgeloesten Prinzipals. Eine Datei in +einem Gruppenordner liess sich damit nicht verlinken – auch nicht von einem +Mitglied, das sie sehen und herunterladen darf. Die Antwort lautete "file not +found". + +Marcs Entscheidung: Das soll gehen, der Empfaenger sieht ohnehin nur die +Datei, nie den Ordner darum. + +**Schreibrecht, nicht nur Leserecht.** Einen Link zu erzeugen heisst, die +Datei oeffentlich zu machen. Wer sie nur geliehen lesen darf, soll sie nicht +weiterreichen koennen -> 403. + +- [x] Datei im Gruppenordner verlinken -> 201, Link funktioniert oeffentlich +- [x] Nur-Lese-Freigabe -> eigener Freigabelink wird mit 403 abgelehnt + +### Zwei Kleinigkeiten + +**`expires_in_hours` hatte keine Obergrenze.** `time.Duration` rechnet in +Nanosekunden; ab etwa 2,6 Millionen Stunden laeuft `int64` ueber, die Dauer +wird **negativ**, und der Link ist in dem Moment abgelaufen, in dem er +entsteht – ohne Fehlermeldung. Jetzt zehn Jahre Obergrenze mit klarer +Ablehnung darueber. + +**`X-Content-Type-Options: nosniff`** fehlte. `attachment` traegt die +Hauptlast, aber der angemeldete Weg setzt beides. + +- [x] `expires_in_hours: 9000000000` -> 400 mit Begruendung +- [x] `expires_in_hours: 24` -> 201 +- [x] Oeffentlicher Download setzt nosniff + +### Go-Tests fuer `internal/sharing` + +Sechs Testfunktionen, unter Mutation nachgewiesen: + +| Mutation | wird rot | +|---|---| +| Eigentuemerpruefung beim Loeschen entfernt | `TestFremderLinkLaesstSichNichtLoeschen` | +| Ablaufpruefung entfernt | `TestAbgelaufenerLinkGiltNichtMehr` | +| Token auf 6 Byte verkuerzt | `TestTokenIstNichtZuErraten` | + +Dazu: Die Liste zeigt nur eigene Links, der Passwort-Hash verlaesst den +Speicher nur beim Tokenabruf (dort wird er gebraucht), der Zaehler zaehlt. + +> **Falle beim Erweitern des Selbsttests:** `GET /api/v1/auth/me` dreht den +> CSRF-Token weiter. Wer ihn zwischendurch ruft und den alten weiterbenutzt, +> bekommt 403 – genau das ist mir passiert, obwohl es seit dem 12.09. hier +> steht. + ## Ein Wächter für die Sprache (24.09.2026) Sechs Tage nach der Übersetzung fielen beim Nachsehen **fünf Reste** auf: diff --git a/backend/cmd/aether/main.go b/backend/cmd/aether/main.go index f1779d0..d38bcf6 100644 --- a/backend/cmd/aether/main.go +++ b/backend/cmd/aether/main.go @@ -236,7 +236,7 @@ func main() { fileHandler := files.NewHandler(fileStore, blobStore, auditor, aclStore, tagStore, docStore) shareStore := sharing.NewStore(pool) - shareHandler := sharing.NewHandler(shareStore, fileStore, blobStore, auditor) + shareHandler := sharing.NewHandler(shareStore, fileStore, blobStore, auditor, aclStore) collector := gc.New(pool, blobStore) diff --git a/backend/internal/files/handler.go b/backend/internal/files/handler.go index e987cc0..46dd401 100644 --- a/backend/internal/files/handler.go +++ b/backend/internal/files/handler.go @@ -1,8 +1,6 @@ package files import ( - "crypto/sha256" - "encoding/hex" "encoding/json" "errors" "fmt" @@ -500,70 +498,11 @@ func (h *Handler) Download(w http.ResponseWriter, r *http.Request) { // Wer die letzten Kilobytes erst nach der Prüfung herausgibt, kann die // Verbindung noch fallen lassen, und der Browser sieht einen // abgebrochenen Download statt einer heilen Datei mit falschem Inhalt. - h256 := sha256.New() - zurueck := &tailWriter{w: w, keep: verifyTail} - - if _, err := io.Copy(zurueck, io.TeeReader(rc, h256)); err != nil { - return // Gegenstelle ist weg - } - - actual := hex.EncodeToString(h256.Sum(nil)) - if actual != f.Checksum { - log.Ctx(r.Context()).Error(). - Str("file_id", f.ID.String()). - Str("storage_key", f.StorageKey). - Str("erwartet", f.Checksum). - Str("gelesen", actual). - Int64("size", f.Size). - Msg("Pruefsumme stimmt nicht - Datei auf der Platte ist beschaedigt") - - // ErrAbortHandler ist der vorgesehene Weg, eine Antwort mittendrin - // fallen zu lassen: net/http schließt die Verbindung und schreibt - // **keinen** eigenen Logeintrag dazu (chis Recoverer reicht diese eine - // Panik ausdrücklich durch). Weil der Schluss noch nicht draußen ist, - // bekommt die Gegenstelle weniger Bytes als im Content-Length - // angekündigt – und wertet das als Fehler. - panic(http.ErrAbortHandler) - } - - // Stimmt sie, darf der Rest hinterher. - _ = zurueck.Flush() -} - -// verifyTail: So viele Bytes werden bis zur Prüfung zurückgehalten. -// -// Sie sind der ganze Hebel: Ohne sie wäre die Datei beim Erkennen des Fehlers -// längst vollständig beim Empfänger, und ein Abbruch käme zu spät. 32 KiB -// kosten nichts und liegen über dem, was io.Copy in einem Zug bewegt. -const verifyTail = 32 * 1024 - -// tailWriter reicht alles durch außer den letzten `keep` Bytes. -type tailWriter struct { - w io.Writer - keep int - buf []byte -} - -func (t *tailWriter) Write(p []byte) (int, error) { - t.buf = append(t.buf, p...) - if ueber := len(t.buf) - t.keep; ueber > 0 { - if _, err := t.w.Write(t.buf[:ueber]); err != nil { - return 0, err - } - t.buf = append(t.buf[:0], t.buf[ueber:]...) - } - return len(p), nil -} - -// Flush gibt heraus, was zurückgehalten wurde. Wird es **nicht** gerufen, -// bleibt die Antwort unvollständig – genau das ist der Zweck. -func (t *tailWriter) Flush() error { - if len(t.buf) == 0 { - return nil - } - _, err := t.w.Write(t.buf) - t.buf = t.buf[:0] - return err + ServeVerified(r.Context(), w, rc, f.Checksum, VerifyInfo{ + FileID: f.ID.String(), + StorageKey: f.StorageKey, + Size: f.Size, + }) } // inlineSafeTypes sind die Typen, die im Browser angezeigt werden dürfen. diff --git a/backend/internal/files/verified.go b/backend/internal/files/verified.go new file mode 100644 index 0000000..9a70871 --- /dev/null +++ b/backend/internal/files/verified.go @@ -0,0 +1,102 @@ +package files + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "io" + "net/http" + + "github.com/rs/zerolog/log" +) + +// Ausliefern mit Prüfung der SHA-256-Summe. +// +// Stand bis zum 25.09.2026 nur im angemeldeten Download. Der **öffentliche** +// Freigabelink lieferte ungeprüft aus – ausgerechnet der Weg, auf dem eine +// Datei das Haus verlässt. Wer sie angemeldet holt, ist der Eigentümer und +// merkt, wenn etwas kaputt ist; wer einen Link bekommt, merkt es nicht. +// +// In CLAUDE.md steht das unter „nicht verhandelbar": beim Upload berechnen, +// **beim Download verifizieren**. Die Regel galt an einer von zwei Stellen. +// Deshalb liegt sie jetzt hier und wird von beiden gerufen. + +// verifyTail: So viele Bytes werden bis zur Prüfung zurückgehalten. +// +// Sie sind der ganze Hebel: Ohne sie wäre die Datei beim Erkennen des Fehlers +// längst vollständig beim Empfänger, und ein Abbruch käme zu spät. 32 KiB +// kosten nichts und liegen über dem, was io.Copy in einem Zug bewegt. +const verifyTail = 32 * 1024 + +// VerifyInfo ist nur für den Logeintrag – wer hinterher sucht, braucht die +// Datei und ihren Platz auf der Platte, nicht bloß „stimmt nicht". +type VerifyInfo struct { + FileID string + StorageKey string + Size int64 +} + +// ServeVerified kopiert `src` nach `w` und prüft dabei die Summe. +// +// Stimmt sie nicht, wird der letzte Rest **nicht** herausgegeben und die +// Verbindung fallen gelassen. Die Kopfzeilen sind da längst draußen, der +// Statuscode also nicht mehr zu ändern – aber die Gegenstelle bekommt weniger +// Bytes als im Content-Length angekündigt und wertet das als Fehler. Besser +// ein abgebrochener Download als eine heile Datei mit falschem Inhalt. +func ServeVerified(ctx context.Context, w io.Writer, src io.Reader, erwartet string, info VerifyInfo) { + h256 := sha256.New() + zurueck := &tailWriter{w: w, keep: verifyTail} + + if _, err := io.Copy(zurueck, io.TeeReader(src, h256)); err != nil { + return // Gegenstelle ist weg + } + + gelesen := hex.EncodeToString(h256.Sum(nil)) + if gelesen != erwartet { + log.Ctx(ctx).Error(). + Str("file_id", info.FileID). + Str("storage_key", info.StorageKey). + Str("erwartet", erwartet). + Str("gelesen", gelesen). + Int64("size", info.Size). + Msg("Pruefsumme stimmt nicht - Datei auf der Platte ist beschaedigt") + + // ErrAbortHandler ist der vorgesehene Weg, eine Antwort mittendrin + // fallen zu lassen: net/http schließt die Verbindung und schreibt + // **keinen** eigenen Logeintrag dazu (chis Recoverer reicht diese eine + // Panik ausdrücklich durch). + panic(http.ErrAbortHandler) + } + + // Stimmt sie, darf der Rest hinterher. + _ = zurueck.Flush() +} + +// tailWriter reicht alles durch außer den letzten `keep` Bytes. +type tailWriter struct { + w io.Writer + keep int + buf []byte +} + +func (t *tailWriter) Write(p []byte) (int, error) { + t.buf = append(t.buf, p...) + if ueber := len(t.buf) - t.keep; ueber > 0 { + if _, err := t.w.Write(t.buf[:ueber]); err != nil { + return 0, err + } + t.buf = append(t.buf[:0], t.buf[ueber:]...) + } + return len(p), nil +} + +// Flush gibt heraus, was zurückgehalten wurde. Wird es **nicht** gerufen, +// bleibt die Antwort unvollständig – genau das ist der Zweck. +func (t *tailWriter) Flush() error { + if len(t.buf) == 0 { + return nil + } + _, err := t.w.Write(t.buf) + t.buf = t.buf[:0] + return err +} diff --git a/backend/internal/sharing/handler.go b/backend/internal/sharing/handler.go index d5970f2..084df7b 100644 --- a/backend/internal/sharing/handler.go +++ b/backend/internal/sharing/handler.go @@ -5,7 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "io" "net/http" "strconv" "time" @@ -13,6 +12,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/google/uuid" + "aether/internal/acl" "aether/internal/audit" "aether/internal/auth" "aether/internal/files" @@ -30,6 +30,10 @@ const ( shareMaxFailures = 10 shareWindow = 15 * time.Minute shareLockout = 15 * time.Minute + + // Zehn Jahre. Grosszügig gemeint – es geht nicht darum, kurze Fristen zu + // erzwingen, sondern den Überlauf abzufangen. + maxExpiryHours = 10 * 365 * 24 ) type Handler struct { @@ -37,12 +41,16 @@ type Handler struct { fileStore *files.Store storage storage.Storage auditor *audit.Logger - limiter *middleware.FailureLimiter + // acl löst auf, in wessen Namen die Datei liegt. Ohne ihn ging nur, was + // dem Anfragenden persönlich gehört – siehe Create. + acl *acl.Store + limiter *middleware.FailureLimiter } -func NewHandler(store *Store, fileStore *files.Store, st storage.Storage, auditor *audit.Logger) *Handler { +func NewHandler(store *Store, fileStore *files.Store, st storage.Storage, auditor *audit.Logger, aclStore *acl.Store) *Handler { return &Handler{ store: store, fileStore: fileStore, storage: st, auditor: auditor, + acl: aclStore, limiter: middleware.NewFailureLimiter(shareMaxFailures, shareWindow, shareLockout), } } @@ -55,7 +63,21 @@ func (h *Handler) Create(w http.ResponseWriter, r *http.Request) { response.Error(w, http.StatusBadRequest, "invalid file id") return } - if _, err := h.fileStore.GetFile(r.Context(), fileID, user.ID); err != nil { + // **Prinzipal statt Konto.** Vorher stand hier `user.ID` – damit liess sich + // eine Datei in einem Gruppenordner nicht per Link teilen, auch nicht von + // einem Mitglied, das sie sehen und herunterladen darf. Die Antwort lautete + // „file not found", was zwar sicher, aber schlicht falsch war: Der + // Empfänger eines Links sieht ohnehin nur die Datei, nie den Ordner + // darum. + // + // **Schreibrecht, nicht nur Leserecht.** Einen Link zu erzeugen heisst, + // die Datei öffentlich zu machen. Wer sie nur geliehen lesen darf, soll + // sie nicht weiterreichen können. + zugriff, ok := h.resolve(w, r, fileID) + if !ok { + return + } + if _, err := h.fileStore.GetFile(r.Context(), fileID, zugriff.OwnerID); err != nil { response.Error(w, http.StatusNotFound, "file not found") return } @@ -77,6 +99,16 @@ func (h *Handler) Create(w http.ResponseWriter, r *http.Request) { var expiresAt *time.Time if body.ExpiresIn != nil && *body.ExpiresIn > 0 { + // Obergrenze, weil `time.Duration` in Nanosekunden rechnet: Ab etwa + // 2,6 Millionen Stunden läuft int64 über, die Dauer wird **negativ**, + // und der Link ist in dem Moment abgelaufen, in dem er entsteht – ohne + // Fehlermeldung. Wer eine unsinnige Zahl schickt, soll das erfahren, + // statt einen toten Link zu bekommen. + if *body.ExpiresIn > maxExpiryHours { + response.Error(w, http.StatusBadRequest, + "expires_in_hours is too large (at most 10 years)") + return + } t := time.Now().Add(time.Duration(*body.ExpiresIn) * time.Hour) expiresAt = &t } @@ -238,5 +270,41 @@ func (h *Handler) Download(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", f.MimeType) w.Header().Set("Content-Disposition", fmt.Sprintf(`attachment; filename=%q`, f.Name)) w.Header().Set("Content-Length", fmt.Sprintf("%d", f.Size)) - _, _ = io.Copy(w, rc) + // Kein Typ-Raten. `attachment` trägt die Hauptlast, aber der angemeldete + // Weg setzt beides – und dies ist der Weg nach draussen. + w.Header().Set("X-Content-Type-Options", "nosniff") + + // **Dieselbe Prüfung wie beim angemeldeten Download.** Bis zum 25.09.2026 + // lief hier ein blankes io.Copy: Wer die Datei über einen Link bekam, + // bekam sie ungeprüft. Ausgerechnet auf dem Weg, auf dem sie das Haus + // verlässt – und wo der Empfänger es am wenigsten merken kann. + files.ServeVerified(r.Context(), w, rc, f.Checksum, files.VerifyInfo{ + FileID: f.ID.String(), + StorageKey: f.StorageKey, + Size: f.Size, + }) +} + +// resolve klärt, in wessen Namen die Datei liegt, und verlangt Schreibrecht. +func (h *Handler) resolve(w http.ResponseWriter, r *http.Request, fileID uuid.UUID) (acl.Access, bool) { + user := auth.UserFromContext(r.Context()) + if h.acl == nil { + return acl.Access{OwnerID: user.ID, Permission: acl.PermWrite}, true + } + a, err := h.acl.ResolveFile(r.Context(), user.ID, fileID) + switch { + case errors.Is(err, acl.ErrNotFound), errors.Is(err, acl.ErrNoAccess): + // 404 statt 403, wie überall: Ein „verboten" verriete, dass es die ID + // gibt. + response.Error(w, http.StatusNotFound, "file not found") + return acl.Access{}, false + case err != nil: + response.Error(w, http.StatusInternalServerError, "database error") + return acl.Access{}, false + } + if !a.CanWrite() { + response.Error(w, http.StatusForbidden, "you only have read access to this file") + return acl.Access{}, false + } + return a, true } diff --git a/backend/internal/sharing/store_test.go b/backend/internal/sharing/store_test.go new file mode 100644 index 0000000..67b58f3 --- /dev/null +++ b/backend/internal/sharing/store_test.go @@ -0,0 +1,251 @@ +package sharing + +import ( + "context" + "errors" + "os" + "testing" + "time" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" +) + +// Ein Freigabelink ist der einzige Weg, auf dem eine Datei dieses Haus ohne +// Anmeldung verlässt. Wer das Token hat, ist drin – also entscheidet sich hier +// alles an zwei Fragen: Gilt der Link noch, und gehört er dem, der ihn +// anfasst? +// +// Beides steckt nicht in einer if-Zeile, sondern in WHERE-Klauseln und einem +// Zeitvergleich. Ein Fehler darin fällt beim Lesen nicht auf: Die Abfrage +// läuft, sie liefert nur zu viel. +// +// Läuft nur mit TEST_DSN. Einfachster Weg: ./scripts/db-tests.sh ./internal/sharing/ + +func testStore(t *testing.T) (*Store, *pgxpool.Pool, context.Context) { + t.Helper() + dsn := os.Getenv("TEST_DSN") + if dsn == "" { + t.Skip("TEST_DSN nicht gesetzt") + } + ctx := context.Background() + pool, err := pgxpool.New(ctx, dsn) + if err != nil { + t.Fatalf("verbinden: %v", err) + } + t.Cleanup(pool.Close) + return NewStore(pool), pool, ctx +} + +func konto(t *testing.T, pool *pgxpool.Pool, ctx context.Context, name string) uuid.UUID { + t.Helper() + var id uuid.UUID + err := pool.QueryRow(ctx, + `INSERT INTO users (username, display_name, password, is_admin) + VALUES ($1, $1, 'x', false) RETURNING id`, name+"-"+uuid.NewString()[:8], + ).Scan(&id) + if err != nil { + t.Fatalf("konto %s: %v", name, err) + } + t.Cleanup(func() { _, _ = pool.Exec(ctx, `DELETE FROM users WHERE id = $1`, id) }) + return id +} + +func datei(t *testing.T, pool *pgxpool.Pool, ctx context.Context, owner uuid.UUID, name string) uuid.UUID { + t.Helper() + var id uuid.UUID + err := pool.QueryRow(ctx, + `INSERT INTO files (owner_id, folder_id, name, mime_type, size, checksum, storage_key) + VALUES ($1, NULL, $2, 'application/pdf', 1, 'x', $3) RETURNING id`, + owner, name, "k/"+uuid.NewString(), + ).Scan(&id) + if err != nil { + t.Fatalf("datei %s: %v", name, err) + } + return id +} + +func TestAbgelaufenerLinkGiltNichtMehr(t *testing.T) { + s, pool, ctx := testStore(t) + anna := konto(t, pool, ctx, "anna") + f := datei(t, pool, ctx, anna, "Rechnung.pdf") + + gestern := time.Now().Add(-1 * time.Hour) + abgelaufen, err := s.Create(ctx, f, anna, "", &gestern) + if err != nil { + t.Fatalf("anlegen: %v", err) + } + if _, _, err := s.GetByToken(ctx, abgelaufen.Token); !errors.Is(err, ErrExpired) { + t.Fatalf("abgelaufener Link muss ErrExpired liefern, kam: %v", err) + } + + // Gegenprobe – sonst prüfte der Test nur, dass irgendetwas fehlschlägt. + morgen := time.Now().Add(1 * time.Hour) + gueltig, err := s.Create(ctx, f, anna, "", &morgen) + if err != nil { + t.Fatalf("anlegen: %v", err) + } + if _, _, err := s.GetByToken(ctx, gueltig.Token); err != nil { + t.Fatalf("gültiger Link muss durchgehen, kam: %v", err) + } + + // Ohne Ablauf gilt er unbegrenzt. + ewig, err := s.Create(ctx, f, anna, "", nil) + if err != nil { + t.Fatalf("anlegen: %v", err) + } + if _, _, err := s.GetByToken(ctx, ewig.Token); err != nil { + t.Fatalf("Link ohne Ablauf muss gelten, kam: %v", err) + } +} + +func TestFremderLinkLaesstSichNichtLoeschen(t *testing.T) { + s, pool, ctx := testStore(t) + anna := konto(t, pool, ctx, "anna") + bert := konto(t, pool, ctx, "bert") + f := datei(t, pool, ctx, anna, "Gehaltsabrechnung.pdf") + + link, err := s.Create(ctx, f, anna, "", nil) + if err != nil { + t.Fatalf("anlegen: %v", err) + } + + // Bert kennt die ID – etwa, weil sie einmal irgendwo auftauchte. + if err := s.Delete(ctx, link.ID, bert); !errors.Is(err, ErrNotFound) { + t.Fatalf("Bert darf Annas Link nicht löschen, kam: %v", err) + } + // Und er ist wirklich noch da. + if _, _, err := s.GetByToken(ctx, link.Token); err != nil { + t.Fatalf("Link muss nach dem Fehlversuch noch gelten, kam: %v", err) + } + // Anna darf. + if err := s.Delete(ctx, link.ID, anna); err != nil { + t.Fatalf("Anna muss ihren Link löschen dürfen: %v", err) + } + if _, _, err := s.GetByToken(ctx, link.Token); !errors.Is(err, ErrNotFound) { + t.Fatalf("gelöschter Link muss ErrNotFound liefern, kam: %v", err) + } +} + +func TestListeZeigtNurEigeneLinks(t *testing.T) { + s, pool, ctx := testStore(t) + anna := konto(t, pool, ctx, "anna") + bert := konto(t, pool, ctx, "bert") + fa := datei(t, pool, ctx, anna, "Annas.pdf") + fb := datei(t, pool, ctx, bert, "Berts.pdf") + + if _, err := s.Create(ctx, fa, anna, "", nil); err != nil { + t.Fatalf("anlegen: %v", err) + } + if _, err := s.Create(ctx, fb, bert, "", nil); err != nil { + t.Fatalf("anlegen: %v", err) + } + + annas, err := s.ListByOwner(ctx, anna) + if err != nil { + t.Fatalf("liste: %v", err) + } + if len(annas) != 1 { + t.Fatalf("Anna soll genau einen Link sehen, sah %d", len(annas)) + } + if annas[0].FileID != fa { + t.Fatalf("Anna sieht einen fremden Link: %v", annas[0].FileID) + } +} + +func TestPasswortVerlaesstDenSpeicherNichtVersehentlich(t *testing.T) { + s, pool, ctx := testStore(t) + anna := konto(t, pool, ctx, "anna") + f := datei(t, pool, ctx, anna, "Vertrag.pdf") + + mitPw, err := s.Create(ctx, f, anna, "argon2-hash-platzhalter", nil) + if err != nil { + t.Fatalf("anlegen: %v", err) + } + // Das Anlegen gibt den Hash **nicht** zurück, nur die Tatsache. + if !mitPw.HasPassword { + t.Fatal("HasPassword muss true sein") + } + + // Beim Abruf über das Token kommt er heraus – dort wird er gebraucht, + // um das eingegebene Passwort zu prüfen. Aber nur dort. + link, hash, err := s.GetByToken(ctx, mitPw.Token) + if err != nil { + t.Fatalf("abrufen: %v", err) + } + if hash != "argon2-hash-platzhalter" { + t.Fatalf("Hash muss beim Tokenabruf vorliegen, kam %q", hash) + } + if !link.HasPassword { + t.Fatal("HasPassword muss auch beim Abruf true sein") + } + + // Die Liste für die Oberfläche kennt ihn nicht. + liste, err := s.ListByOwner(ctx, anna) + if err != nil { + t.Fatalf("liste: %v", err) + } + if len(liste) != 1 || !liste[0].HasPassword { + t.Fatalf("Liste soll den Link mit HasPassword zeigen, kam %+v", liste) + } + + // Ohne Passwort bleibt es false – sonst prüfte das oben nichts. + ohnePw, err := s.Create(ctx, f, anna, "", nil) + if err != nil { + t.Fatalf("anlegen: %v", err) + } + if ohnePw.HasPassword { + t.Fatal("HasPassword muss ohne Passwort false sein") + } + if _, hash, _ := s.GetByToken(ctx, ohnePw.Token); hash != "" { + t.Fatalf("ohne Passwort darf kein Hash kommen, kam %q", hash) + } +} + +func TestTokenIstNichtZuErraten(t *testing.T) { + s, pool, ctx := testStore(t) + anna := konto(t, pool, ctx, "anna") + f := datei(t, pool, ctx, anna, "Rechnung.pdf") + + gesehen := map[string]bool{} + for i := 0; i < 20; i++ { + l, err := s.Create(ctx, f, anna, "", nil) + if err != nil { + t.Fatalf("anlegen: %v", err) + } + // 24 Zufallsbytes, base64url ohne Auffüllen = 32 Zeichen. Wird das + // jemals kürzer, ist das Token ratbar – und der Link ist das einzige + // Geheimnis, das ihn schützt. + if len(l.Token) != 32 { + t.Fatalf("Token soll 32 Zeichen haben, hat %d: %q", len(l.Token), l.Token) + } + if gesehen[l.Token] { + t.Fatalf("Token doppelt vergeben: %q", l.Token) + } + gesehen[l.Token] = true + } +} + +func TestZaehlerZaehlt(t *testing.T) { + s, pool, ctx := testStore(t) + anna := konto(t, pool, ctx, "anna") + f := datei(t, pool, ctx, anna, "Rechnung.pdf") + + link, err := s.Create(ctx, f, anna, "", nil) + if err != nil { + t.Fatalf("anlegen: %v", err) + } + if link.DownloadCount != 0 { + t.Fatalf("frischer Link soll bei 0 stehen, stand bei %d", link.DownloadCount) + } + s.IncrementDownloads(ctx, link.Token) + s.IncrementDownloads(ctx, link.Token) + + nach, _, err := s.GetByToken(ctx, link.Token) + if err != nil { + t.Fatalf("abrufen: %v", err) + } + if nach.DownloadCount != 2 { + t.Fatalf("Zähler soll 2 sein, ist %d", nach.DownloadCount) + } +} diff --git a/scripts/pruefsumme-selbsttest.py b/scripts/pruefsumme-selbsttest.py index c3d960a..3d9be1b 100644 --- a/scripts/pruefsumme-selbsttest.py +++ b/scripts/pruefsumme-selbsttest.py @@ -92,6 +92,25 @@ def herunterladen(file_id): return False, None, type(e).__name__ + ": " + str(e)[:80] +def oeffentlich_holen(token): + """Dasselbe ueber den Freigabelink – ohne Anmeldung. + + Bis zum 25.09.2026 lief hier ein blankes io.Copy: Wer die Datei ueber + einen Link bekam, bekam sie ungeprueft. Ausgerechnet auf dem Weg, auf dem + sie das Haus verlaesst. + """ + req = urllib.request.Request(BASE + f"/share/{token}/download", + data=b"{}", method="POST", + headers={"Content-Type": "application/json"}) + try: + with urllib.request.build_opener().open(req) as r: + erwartet = int(r.headers.get("Content-Length", "-1")) + daten = r.read() + return (erwartet < 0 or len(daten) == erwartet), daten, "" + except Exception as e: + return False, None, type(e).__name__ + ": " + str(e)[:80] + + def blobpfad(name): """Wo der Blob liegt. @@ -118,9 +137,12 @@ assert st == 200, f"Anmeldung fehlgeschlagen: {st}" CSRF = {"X-CSRF-Token": login["csrf_token"]} angelegt = [] +geteilt = [] def aufraeumen(): + for lid in geteilt: + api("DELETE", f"/api/v1/shares/{lid}", None, CSRF) for fid in angelegt: api("DELETE", f"/api/v1/files/{fid}", None, CSRF) st, papier = api("GET", "/api/v1/trash", None, CSRF) @@ -168,6 +190,19 @@ try: check("und was ankam, ist kuerzer als angekuendigt", len(daten) < groesse, f"{len(daten)} von {groesse}") + # 5. Und jetzt derselbe Weg nach draussen: ueber einen Freigabelink. + # Die Datei ist dieselbe und immer noch kaputt – die Zusage muss + # hier genauso gelten. + st, link = api("POST", f"/api/v1/files/{datei['id']}/share", {}, CSRF) + if st == 201 and link: + geteilt.append(link["id"]) + voll, daten, fehler = oeffentlich_holen(link["token"]) + check("auch ueber den Freigabelink kommt sie NICHT vollstaendig an", + not voll, + f"{len(daten or b'')} von {groesse} Bytes – heil ausgeliefert!") + else: + check("Freigabelink angelegt", False, st) + # 4. Der Server muss es gesagt haben. log = logzeilen(marke) if log: diff --git a/scripts/sharing-selbsttest.py b/scripts/sharing-selbsttest.py index 27fba44..c7b08e5 100644 --- a/scripts/sharing-selbsttest.py +++ b/scripts/sharing-selbsttest.py @@ -72,6 +72,20 @@ def ruf(opener, method, path, body=None, headers=None, roh=False): return e.code, None +def mit_kopfzeilen(method, path, body=None): + """Wie `ruf`, gibt aber die Antwortkopfzeilen zurueck. Gebraucht fuer + nosniff – das steht nirgends im Rumpf.""" + data = json.dumps(body).encode() if body is not None else b"" + req = urllib.request.Request(BASE + path, data=data, method=method, + headers={"Content-Type": "application/json"}) + try: + with urllib.request.build_opener().open(req) as r: + r.read() + return r.status, dict(r.headers) + except urllib.error.HTTPError as e: + return e.code, dict(e.headers) + + def anmelden(user, passwort): o = sitzung() st, d = ruf(o, "POST", "/api/v1/auth/login", {"username": user, "password": passwort}) @@ -98,6 +112,11 @@ def hochladen(opener, csrf, name, inhalt: bytes, folder_id=None): print(f"Freigabe-Selbsttest gegen {BASE}\n") admin, ADMIN_CSRF = anmelden(USER, PASSWORT) +# **Achtung:** `/auth/me` dreht den CSRF-Token weiter (siehe TESTING.md). Wer +# danach den alten weiterbenutzt, bekommt 403 – also den neuen uebernehmen. +_, ich = ruf(admin, "GET", "/api/v1/auth/me") +ADMIN_ID = ich["user"]["id"] +ADMIN_CSRF = {"X-CSRF-Token": ich["csrf_token"]} # ── Zweitnutzer: nur mit ihm lässt sich prüfen, dass fremde Dateien fremd # bleiben. Ein Test, der nur das eigene Konto kennt, kann das nicht. @@ -112,11 +131,15 @@ FREMD_ID = fremd["id"] eigen = None fremd_datei = None links = [] +gruppen = [] +dateien = [] def aufraeumen(): for lid in links: ruf(admin, "DELETE", f"/api/v1/shares/{lid}", None, ADMIN_CSRF) + for did in dateien: + ruf(admin, "DELETE", f"/api/v1/files/{did}", None, ADMIN_CSRF) if eigen: ruf(admin, "DELETE", f"/api/v1/files/{eigen['id']}", None, ADMIN_CSRF) # Löschen heißt Papierkorb – ein Test, der nur so weit aufräumt, hinterlässt @@ -126,6 +149,8 @@ def aufraeumen(): if f.get("name", "").startswith("selbsttest-"): ruf(admin, "DELETE", f"/api/v1/trash/{f['id']}", None, ADMIN_CSRF) ruf(admin, "DELETE", f"/api/v1/admin/users/{FREMD_ID}", None, ADMIN_CSRF) + for gid in gruppen: + ruf(admin, "DELETE", f"/api/v1/admin/groups/{gid}", None, ADMIN_CSRF) try: @@ -222,6 +247,69 @@ try: st, _ = ruf(None, "POST", f"/share/{fluechtig['token']}/download", {}) check("Link auf gelöschte Datei liefert nichts", st in (404, 410), st) + # ── Seit dem 25.09.2026 ──────────────────────────────────────────────── + print("\n── Gruppenordner, Grenzen, Kopfzeilen ──") + + # **Gruppendatei teilbar.** Vorher übergab Create `user.ID` statt des + # aufgelösten Prinzipals – eine Datei im Gruppenordner liess sich nicht + # verlinken, auch nicht vom Mitglied. Die Antwort lautete „file not found". + gname = "selbsttest-gruppe-" + uuid.uuid4().hex[:6] + st, gruppe = ruf(admin, "POST", "/api/v1/admin/groups", + {"name": gname, "description": "Selbsttest"}, ADMIN_CSRF) + if st in (200, 201): + gruppen.append(gruppe["id"]) + ruf(admin, "POST", f"/api/v1/admin/groups/{gruppe['id']}/members", + {"user_id": ADMIN_ID}, ADMIN_CSRF) + # Der Wurzelordner der Gruppe taucht in der normalen Ordnerliste auf. + _, wurzeln = ruf(admin, "GET", "/api/v1/files") + gordner = next((f for f in (wurzeln or {}).get("folders", []) + if f.get("group_id")), None) + check("Gruppenordner ist da", gordner is not None) + if gordner: + gdatei = hochladen(admin, ADMIN_CSRF, "selbsttest-gruppe.txt", + b"in der gruppe", gordner["id"]) + dateien.append(gdatei["id"]) + st, glink = ruf(admin, "POST", f"/api/v1/files/{gdatei['id']}/share", + {}, ADMIN_CSRF) + check("Datei im Gruppenordner laesst sich verlinken", st == 201, st) + if st == 201: + links.append(glink["id"]) + st, _ = ruf(None, "GET", f"/share/{glink['token']}") + check("und der Link funktioniert oeffentlich", st == 200, st) + + # Frische Datei: Die weiter oben ist inzwischen geloescht – die Abschnitte + # davor pruefen genau das. + frisch = hochladen(admin, ADMIN_CSRF, "selbsttest-frist.txt", b"frist") + dateien.append(frisch["id"]) + + # **Obergrenze fuer den Ablauf.** Ohne sie laeuft time.Duration ueber, die + # Frist wird negativ, und der Link ist tot in dem Moment, in dem er + # entsteht – ohne Fehlermeldung. + st, _ = ruf(admin, "POST", f"/api/v1/files/{frisch['id']}/share", + {"expires_in_hours": 9_000_000_000}, ADMIN_CSRF) + check("unsinnige Ablauffrist wird abgelehnt (400)", st == 400, st) + + st, kurz = ruf(admin, "POST", f"/api/v1/files/{frisch['id']}/share", + {"expires_in_hours": 24}, ADMIN_CSRF) + check("vernuenftige Ablauffrist geht durch", st == 201, st) + if st == 201: + links.append(kurz["id"]) + st, kopf = mit_kopfzeilen("POST", f"/share/{kurz['token']}/download") + check("oeffentlicher Download setzt nosniff", + kopf.get("X-Content-Type-Options") == "nosniff", dict(kopf)) + + # **Nur-Lese-Freigabe darf nicht weiterverteilen.** Wer eine Datei geliehen + # bekommt, soll sie nicht oeffentlich machen koennen. + st, _ = ruf(admin, "POST", f"/api/v1/files/{frisch['id']}/acl", + {"subject_id": FREMD_ID, "subject_type": "user", + "permission": "read"}, ADMIN_CSRF) + if st in (200, 201): + fremd_sitzung, FREMD_CSRF = anmelden(fremd_name, fremd_pw) + st, _ = ruf(fremd_sitzung, "POST", f"/api/v1/files/{frisch['id']}/share", + {}, FREMD_CSRF) + check("nur geliehen gelesen -> kein eigener Freigabelink (403)", + st == 403, st) + # Zum Schluss, mit Absicht: Diese Probe sperrt die eigene IP für 15 # Minuten. Stünde sie weiter oben, fielen alle folgenden Passwortprüfungen # in die selbst gestellte Falle.