diff --git a/backend/internal/web/handlers_files_test.go b/backend/internal/web/handlers_files_test.go index 78625ad..daa5980 100644 --- a/backend/internal/web/handlers_files_test.go +++ b/backend/internal/web/handlers_files_test.go @@ -13,6 +13,7 @@ package web import ( "bytes" "context" + "errors" "io" "mime/multipart" "net/http" @@ -28,9 +29,11 @@ import ( ) // stubbedFileStorer is a no-op FileStorer for test injection. -// Plan 02/03 will replace the Skip calls with real assertions that use this stub. +// deleteErr may be set to simulate S3 delete failures (TestFileDelete_S3Failure). type stubbedFileStorer struct { uploadedKey string + deletedKey string + deleteErr error } func (s *stubbedFileStorer) Upload(_ context.Context, key string, _ io.Reader) (string, int64, error) { @@ -38,12 +41,13 @@ func (s *stubbedFileStorer) Upload(_ context.Context, key string, _ io.Reader) ( return "application/octet-stream", 12, nil } -func (s *stubbedFileStorer) Delete(_ context.Context, _ string) error { - return nil +func (s *stubbedFileStorer) Delete(_ context.Context, key string) error { + s.deletedKey = key + return s.deleteErr } func (s *stubbedFileStorer) PresignDownload(_ context.Context, _ string) (string, error) { - return "https://example.com/presigned", nil + return "https://example.com/signed?key=foo", nil } // Compile-time assertion: stubbedFileStorer satisfies the files.FileStorer interface. @@ -343,21 +347,325 @@ func TestFilesTab(t *testing.T) { // TestFileDownload verifies that GET /tablos/{id}/files/{file_id}/download // returns a 302 redirect to a signed time-limited URL (FILE-04). func TestFileDownload(t *testing.T) { - t.Skip("FILE handler tests: not yet implemented — Plan 03") + pool, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + q := sqlc.New(pool) + store := auth.NewStore(q) + stub := &stubbedFileStorer{} + router := newFileTestRouter(q, store, stub) + + user := preInsertUser(t, ctx, q, "filedownload@example.com", "correct-horse-12") + tablo, err := q.InsertTablo(ctx, sqlc.InsertTabloParams{ + UserID: user.ID, + Title: "Download Test Tablo", + }) + if err != nil { + t.Fatalf("InsertTablo: %v", err) + } + dbFile, err := q.InsertTabloFile(ctx, sqlc.InsertTabloFileParams{ + TabloID: tablo.ID, + S3Key: "files/" + tablo.ID.String() + "/some-uuid", + Filename: "report.pdf", + ContentType: "application/pdf", + SizeBytes: 1024, + }) + if err != nil { + t.Fatalf("InsertTabloFile: %v", err) + } + + sessionVal, _, err := store.Create(ctx, user.ID) + if err != nil { + t.Fatalf("store.Create: %v", err) + } + sessionCookie := &http.Cookie{Name: auth.SessionCookieName, Value: sessionVal} + + url := "/tablos/" + tablo.ID.String() + "/files/" + dbFile.ID.String() + "/download" + req := httptest.NewRequest(http.MethodGet, url, nil) + req.AddCookie(sessionCookie) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusFound { + t.Fatalf("GET download status = %d; want 302\nbody: %s", rec.Code, rec.Body.String()) + } + loc := rec.Header().Get("Location") + if loc != "https://example.com/signed?key=foo" { + t.Errorf("Location = %q; want %q", loc, "https://example.com/signed?key=foo") + } +} + +// TestFileDownload_NonOwner verifies that a non-owner gets 404 on download (FILE-06). +func TestFileDownload_NonOwner(t *testing.T) { + pool, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + q := sqlc.New(pool) + store := auth.NewStore(q) + stub := &stubbedFileStorer{} + router := newFileTestRouter(q, store, stub) + + owner := preInsertUser(t, ctx, q, "dlowner@example.com", "correct-horse-12") + tablo, err := q.InsertTablo(ctx, sqlc.InsertTabloParams{ + UserID: owner.ID, + Title: "Owner Tablo", + }) + if err != nil { + t.Fatalf("InsertTablo: %v", err) + } + dbFile, err := q.InsertTabloFile(ctx, sqlc.InsertTabloFileParams{ + TabloID: tablo.ID, + S3Key: "files/" + tablo.ID.String() + "/x", + Filename: "secret.pdf", + ContentType: "application/pdf", + SizeBytes: 512, + }) + if err != nil { + t.Fatalf("InsertTabloFile: %v", err) + } + + nonOwner := preInsertUser(t, ctx, q, "dlnonowner@example.com", "correct-horse-12") + sessionVal, _, err := store.Create(ctx, nonOwner.ID) + if err != nil { + t.Fatalf("store.Create: %v", err) + } + sessionCookie := &http.Cookie{Name: auth.SessionCookieName, Value: sessionVal} + + url := "/tablos/" + tablo.ID.String() + "/files/" + dbFile.ID.String() + "/download" + req := httptest.NewRequest(http.MethodGet, url, nil) + req.AddCookie(sessionCookie) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("non-owner download status = %d; want 404", rec.Code) + } } // ---- TestFileDelete (FILE-05) ---- // TestFileDelete verifies that POST /tablos/{id}/files/{file_id}/delete -// removes the DB row (and invokes S3 delete via the stub) (FILE-05). +// removes the DB row, calls S3 delete, and returns the FileRowGone fragment for HTMX (FILE-05). func TestFileDelete(t *testing.T) { - t.Skip("FILE handler tests: not yet implemented — Plan 03") + pool, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + q := sqlc.New(pool) + store := auth.NewStore(q) + stub := &stubbedFileStorer{} + router := newFileTestRouter(q, store, stub) + + user := preInsertUser(t, ctx, q, "filedelete@example.com", "correct-horse-12") + tablo, err := q.InsertTablo(ctx, sqlc.InsertTabloParams{ + UserID: user.ID, + Title: "Delete Test Tablo", + }) + if err != nil { + t.Fatalf("InsertTablo: %v", err) + } + dbFile, err := q.InsertTabloFile(ctx, sqlc.InsertTabloFileParams{ + TabloID: tablo.ID, + S3Key: "files/" + tablo.ID.String() + "/del-uuid", + Filename: "todelete.txt", + ContentType: "text/plain", + SizeBytes: 100, + }) + if err != nil { + t.Fatalf("InsertTabloFile: %v", err) + } + + sessionVal, _, err := store.Create(ctx, user.ID) + if err != nil { + t.Fatalf("store.Create: %v", err) + } + sessionCookie := &http.Cookie{Name: auth.SessionCookieName, Value: sessionVal} + + deleteURL := "/tablos/" + tablo.ID.String() + "/files/" + dbFile.ID.String() + "/delete" + csrfToken, csrfCookies := getCSRFToken(t, router, "/tablos/"+tablo.ID.String()+"/files", []*http.Cookie{sessionCookie}) + + var body strings.Builder + body.WriteString("gorilla.csrf.Token=" + csrfToken) + + req := httptest.NewRequest(http.MethodPost, deleteURL, strings.NewReader(body.String())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("HX-Request", "true") + req.AddCookie(sessionCookie) + for _, c := range csrfCookies { + req.AddCookie(c) + } + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("HTMX delete status = %d; want 200\nbody: %s", rec.Code, rec.Body.String()) + } + + // Response should contain the gone zone for the file. + respBody := rec.Body.String() + if !strings.Contains(respBody, "file-"+dbFile.ID.String()) { + t.Errorf("response missing file-row-zone id; body: %.300s", respBody) + } + + // S3 delete should have been called. + if stub.deletedKey != dbFile.S3Key { + t.Errorf("stub.deletedKey = %q; want %q", stub.deletedKey, dbFile.S3Key) + } + + // DB row should no longer exist. + remaining, err := q.ListFilesByTablo(ctx, tablo.ID) + if err != nil { + t.Fatalf("ListFilesByTablo: %v", err) + } + if len(remaining) != 0 { + t.Errorf("expected 0 files after delete, got %d", len(remaining)) + } +} + +// TestFileDelete_S3Failure verifies that when S3 delete fails, the DB row is still +// deleted and the response is 200 (not 500) for the HTMX path (FILE-05 deviation pattern). +func TestFileDelete_S3Failure(t *testing.T) { + pool, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + q := sqlc.New(pool) + store := auth.NewStore(q) + stub := &stubbedFileStorer{deleteErr: errors.New("s3 unavailable")} + router := newFileTestRouter(q, store, stub) + + user := preInsertUser(t, ctx, q, "dels3fail@example.com", "correct-horse-12") + tablo, err := q.InsertTablo(ctx, sqlc.InsertTabloParams{ + UserID: user.ID, + Title: "S3 Fail Delete Tablo", + }) + if err != nil { + t.Fatalf("InsertTablo: %v", err) + } + dbFile, err := q.InsertTabloFile(ctx, sqlc.InsertTabloFileParams{ + TabloID: tablo.ID, + S3Key: "files/" + tablo.ID.String() + "/s3fail-uuid", + Filename: "s3fail.txt", + ContentType: "text/plain", + SizeBytes: 50, + }) + if err != nil { + t.Fatalf("InsertTabloFile: %v", err) + } + + sessionVal, _, err := store.Create(ctx, user.ID) + if err != nil { + t.Fatalf("store.Create: %v", err) + } + sessionCookie := &http.Cookie{Name: auth.SessionCookieName, Value: sessionVal} + + deleteURL := "/tablos/" + tablo.ID.String() + "/files/" + dbFile.ID.String() + "/delete" + csrfToken, csrfCookies := getCSRFToken(t, router, "/tablos/"+tablo.ID.String()+"/files", []*http.Cookie{sessionCookie}) + + var body strings.Builder + body.WriteString("gorilla.csrf.Token=" + csrfToken) + + req := httptest.NewRequest(http.MethodPost, deleteURL, strings.NewReader(body.String())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("HX-Request", "true") + req.AddCookie(sessionCookie) + for _, c := range csrfCookies { + req.AddCookie(c) + } + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + // Even when S3 fails, HTMX path returns 200 (not 500). + if rec.Code != http.StatusOK { + t.Fatalf("S3 failure delete status = %d; want 200\nbody: %s", rec.Code, rec.Body.String()) + } + + // DB row should still be deleted. + remaining, err := q.ListFilesByTablo(ctx, tablo.ID) + if err != nil { + t.Fatalf("ListFilesByTablo: %v", err) + } + if len(remaining) != 0 { + t.Errorf("expected 0 files after delete (S3 fail), got %d", len(remaining)) + } } // ---- TestFileOwnership (FILE-06) ---- -// TestFileOwnership verifies that a non-owner gets 404 on -// GET /tablos/{id}/files, POST /tablos/{id}/files, and all file sub-routes (FILE-06). +// TestFileOwnership verifies that a non-owner gets 404 on download, delete-confirm, +// and delete routes (FILE-06). func TestFileOwnership(t *testing.T) { - t.Skip("FILE handler tests: not yet implemented — Plan 03") + pool, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + q := sqlc.New(pool) + store := auth.NewStore(q) + stub := &stubbedFileStorer{} + router := newFileTestRouter(q, store, stub) + + owner := preInsertUser(t, ctx, q, "ownship@example.com", "correct-horse-12") + tablo, err := q.InsertTablo(ctx, sqlc.InsertTabloParams{ + UserID: owner.ID, + Title: "Ownership Tablo", + }) + if err != nil { + t.Fatalf("InsertTablo: %v", err) + } + dbFile, err := q.InsertTabloFile(ctx, sqlc.InsertTabloFileParams{ + TabloID: tablo.ID, + S3Key: "files/" + tablo.ID.String() + "/own-uuid", + Filename: "owned.txt", + ContentType: "text/plain", + SizeBytes: 10, + }) + if err != nil { + t.Fatalf("InsertTabloFile: %v", err) + } + + nonOwner := preInsertUser(t, ctx, q, "nonownship@example.com", "correct-horse-12") + sessionVal, _, err := store.Create(ctx, nonOwner.ID) + if err != nil { + t.Fatalf("store.Create: %v", err) + } + sessionCookie := &http.Cookie{Name: auth.SessionCookieName, Value: sessionVal} + + base := "/tablos/" + tablo.ID.String() + "/files/" + dbFile.ID.String() + + // Non-owner: GET download → 404 + reqDL := httptest.NewRequest(http.MethodGet, base+"/download", nil) + reqDL.AddCookie(sessionCookie) + recDL := httptest.NewRecorder() + router.ServeHTTP(recDL, reqDL) + if recDL.Code != http.StatusNotFound { + t.Errorf("non-owner download status = %d; want 404", recDL.Code) + } + + // Non-owner: GET delete-confirm → 404 + reqDC := httptest.NewRequest(http.MethodGet, base+"/delete-confirm", nil) + reqDC.AddCookie(sessionCookie) + recDC := httptest.NewRecorder() + router.ServeHTTP(recDC, reqDC) + if recDC.Code != http.StatusNotFound { + t.Errorf("non-owner delete-confirm status = %d; want 404", recDC.Code) + } + + // Non-owner: POST delete → 404 + // We need a CSRF token from non-owner's session to get past CSRF middleware. + csrfToken, csrfCookies := getCSRFToken(t, router, "/tablos/"+tablo.ID.String()+"/files", []*http.Cookie{sessionCookie}) + var delBody strings.Builder + delBody.WriteString("gorilla.csrf.Token=" + csrfToken) + reqDel := httptest.NewRequest(http.MethodPost, base+"/delete", strings.NewReader(delBody.String())) + reqDel.Header.Set("Content-Type", "application/x-www-form-urlencoded") + reqDel.AddCookie(sessionCookie) + for _, c := range csrfCookies { + reqDel.AddCookie(c) + } + recDel := httptest.NewRecorder() + router.ServeHTTP(recDel, reqDel) + if recDel.Code != http.StatusNotFound { + t.Errorf("non-owner delete status = %d; want 404", recDel.Code) + } }