Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 8483ab9e84 | |||
| 2e16cf9d51 |
@@ -0,0 +1,65 @@
|
|||||||
|
# Project Review & Refactoring TODOs
|
||||||
|
|
||||||
|
This document contains the prioritized list of refactoring tasks, architectural improvements, and testing strategies for the Aktiteil project.
|
||||||
|
|
||||||
|
## 🚨 Must do (Security & Critical Best Practices)
|
||||||
|
|
||||||
|
- [ ] **Fix Critical XSS Vulnerability (`{@html}` without sanitization)**
|
||||||
|
- **Where:** `src/routes/akti/[aktiId]/+page.svelte`
|
||||||
|
- **Why:** Rendering user input via `{@html data.akti.body}` without sanitization allows malicious scripts to be injected.
|
||||||
|
- **Fix:** Use the already installed `sanitize-html` library on the server to sanitize `changeRequest.body` before updating/inserting into the database.
|
||||||
|
|
||||||
|
- [ ] **Move Server-Only Code to `$lib/server`**
|
||||||
|
- **Where:** `src/lib/auth.ts`
|
||||||
|
- **Why:** It imports from `./server/db`. Keeping server-side dependencies in the general `$lib` folder risks accidental imports by client components, breaking the Vite build and potentially leaking server logic.
|
||||||
|
- **Fix:** Move and rename it to `src/lib/server/session.ts` (or `authUtils.ts`) and update imports in `.server.ts` files.
|
||||||
|
|
||||||
|
- [ ] **Fix Action Validation Error Handling**
|
||||||
|
- **Where:** `src/routes/akti/[aktiId]/+page.server.ts` and `src/routes/akti/[aktiId]/comment/+page.server.ts`
|
||||||
|
- **Why:** Currently returning `error(400)` on validation failure, which wipes form data and shows a generic error page.
|
||||||
|
- **Fix:** Use SvelteKit's `fail(400, { message: 'Invalid data' })` to keep the user on the page and preserve their input.
|
||||||
|
|
||||||
|
- [ ] **Fix Hacky Fallback in Auth Query**
|
||||||
|
- **Where:** `src/lib/auth.ts` -> `getSession()`
|
||||||
|
- **Why:** Querying the DB with a fallback UUID (`eaf930...`) when email is missing is an anti-pattern.
|
||||||
|
- **Fix:** Implement an early return (`if (!session?.user?.email) return null;`) before hitting the database.
|
||||||
|
|
||||||
|
## 🛠️ Should do (Performance & Architecture)
|
||||||
|
|
||||||
|
- [ ] **Parallelize Database Queries**
|
||||||
|
- **Where:** `src/routes/akti/[aktiId]/+page.server.ts` (load function)
|
||||||
|
- **Why:** Queries are running sequentially.
|
||||||
|
- **Fix:** Use `Promise.all([ db.query.aktis.findFirst(...), db.query.ratings.findMany(...) ])` to run concurrently.
|
||||||
|
|
||||||
|
- [ ] **Implement Pagination / Limit for the Dashboard**
|
||||||
|
- **Where:** `src/routes/+page.server.ts`
|
||||||
|
- **Why:** Querying all records joined with ratings will scale poorly.
|
||||||
|
- **Fix:** Add a `.limit()` clause and consider basic pagination or infinite scrolling.
|
||||||
|
|
||||||
|
- [ ] **Extend Auth.js Types Globally**
|
||||||
|
- **Where:** `src/app.d.ts`
|
||||||
|
- **Why:** TypeScript doesn't inherently know `session.user.id` exists, leading to hacky workarounds.
|
||||||
|
- **Fix:** Override `@auth/sveltekit` Session types in `app.d.ts` to include `id` and `email` strictly.
|
||||||
|
|
||||||
|
- [ ] **Consider Adopting a Form Library**
|
||||||
|
- **Where:** `src/lib/extractFormData.ts`
|
||||||
|
- **Why:** Custom form extractors lack instant client-side validation and seamless server-side error mapping.
|
||||||
|
- **Fix:** Consider switching to `sveltekit-superforms` which integrates well with Valibot.
|
||||||
|
|
||||||
|
## ✨ Nice to have (UX & Polish)
|
||||||
|
|
||||||
|
- [ ] **Clarify File Naming (`auth.ts` vs `auth.ts`)**
|
||||||
|
- Rename `src/lib/auth.ts` to `session.ts` or similar to distinguish from `src/auth.ts` (Auth.js setup).
|
||||||
|
|
||||||
|
- [ ] **Abstract Heavy Database Queries**
|
||||||
|
- Move complex aggregations (like computing averages in `src/routes/+page.server.ts`) into a dedicated `src/lib/server/db/queries.ts` file to keep routes clean.
|
||||||
|
|
||||||
|
- [ ] **Clean up Redundant Imports**
|
||||||
|
- In `src/routes/+layout.server.ts`, change `import { getSession as getSession }` to `import { getSession }`.
|
||||||
|
|
||||||
|
## 🧪 Testing Plan
|
||||||
|
|
||||||
|
- [ ] **Add Playwright (End-to-End Testing)**
|
||||||
|
- Install Playwright to test SvelteKit server actions, DB integration, and Flowbite forms holistically.
|
||||||
|
- [ ] **Add Vitest + Svelte Testing Library (Unit/Component Testing)**
|
||||||
|
- Set up Vitest to test UI components (`AktiCard`, `AktiEditor`) and utility functions (`extractFormData`) in isolation.
|
||||||
Vendored
+11
@@ -1,3 +1,5 @@
|
|||||||
|
import { DefaultSession } from '@auth/sveltekit';
|
||||||
|
|
||||||
// See https://svelte.dev/docs/kit/types#app.d.ts
|
// See https://svelte.dev/docs/kit/types#app.d.ts
|
||||||
// for information about these interfaces
|
// for information about these interfaces
|
||||||
declare global {
|
declare global {
|
||||||
@@ -10,4 +12,13 @@ declare global {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
declare module '@auth/sveltekit' {
|
||||||
|
interface Session {
|
||||||
|
user: {
|
||||||
|
id: string;
|
||||||
|
email: string;
|
||||||
|
} & DefaultSession['user'];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export {};
|
export {};
|
||||||
|
|||||||
+4
-8
@@ -1,4 +1,4 @@
|
|||||||
import type { Session, User } from '@auth/sveltekit';
|
import type { Session } from '@auth/sveltekit';
|
||||||
import { error } from '@sveltejs/kit';
|
import { error } from '@sveltejs/kit';
|
||||||
import { db } from './server/db';
|
import { db } from './server/db';
|
||||||
import { users } from './server/db/schema';
|
import { users } from './server/db/schema';
|
||||||
@@ -8,21 +8,17 @@ interface Event {
|
|||||||
auth(): Promise<Session | null>;
|
auth(): Promise<Session | null>;
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
interface UserWithId extends User {
|
|
||||||
id: string;
|
|
||||||
email: string;
|
|
||||||
}
|
|
||||||
|
|
||||||
export async function ensureAuth(event: Event): Promise<UserWithId> {
|
export async function ensureAuth(event: Event): Promise<Session['user']> {
|
||||||
const session = await getSession(event);
|
const session = await getSession(event);
|
||||||
if (!session) error(401, { message: 'Du muesch di zersch iiloge' });
|
if (!session) error(401, { message: 'Du muesch di zersch iiloge' });
|
||||||
|
|
||||||
const user = session?.user;
|
const user = session.user;
|
||||||
if (!user || !user.email || !user.id) {
|
if (!user || !user.email || !user.id) {
|
||||||
error(401, { message: 'Du muesch di zersch iiloge' });
|
error(401, { message: 'Du muesch di zersch iiloge' });
|
||||||
}
|
}
|
||||||
|
|
||||||
return { ...user, id: user.id, email: user.email }; // weird thingamajig so that ts compiler is happy
|
return user;
|
||||||
}
|
}
|
||||||
export async function getSession(event: Event) {
|
export async function getSession(event: Event) {
|
||||||
const session = await event.locals.auth();
|
const session = await event.locals.auth();
|
||||||
|
|||||||
Reference in New Issue
Block a user