From 393258cfcce34fa3d49ff05f4fac966c4cec1d11 Mon Sep 17 00:00:00 2001 From: Alimedhat000 Date: Tue, 25 Aug 2026 23:11:49 +0300 Subject: [PATCH] fix(server,client): validate all document endpoints with Zod Document routes accepted unvalidated bodies and params: malformed ids surfaced Prisma errors as 500s, non-boolean flags reached the DB layer, and title/permission values were never length- or enum-checked. Auth schemas also lagged client-side bounds (server allowed 6-char passwords while the client requires 8). Add body/param/query schemas wired through the existing validate() middleware for every /document route, replace the manual share-link permission check with a query enum schema, and harden register/login/ addCollaborator bounds (email <=254, username <=50 + charset, password 8..128, fullName <=100). Mirror those bounds in the client (zod schemas + maxLength attributes) so users never hit a 400. Closes #52. Related: #37 (rate limiting), #83 (enumeration hardening). --- .../new-document-form-body.tsx | 1 + client/src/components/ui/Auth/login-form.tsx | 2 + .../src/components/ui/Auth/register-form.tsx | 4 + .../DocumentCardDropdown/rename-modal.tsx | 1 + .../collaborators-dropdown.tsx | 1 + client/src/lib/auth.ts | 28 ++++-- server/src/controllers/document.controller.ts | 18 +--- server/src/routers/document.router.ts | 49 ++++++---- .../src/validations/addCollaborator.schema.ts | 1 + .../src/validations/createDocument.schema.ts | 9 ++ .../src/validations/documentParams.schema.ts | 27 ++++++ server/src/validations/login.schema.ts | 3 +- server/src/validations/register.schema.ts | 11 ++- .../validations/updateDocSettings.schema.ts | 7 ++ .../src/validations/updateDocument.schema.ts | 9 ++ server/test/auth.test.ts | 20 ++++ server/test/document.test.ts | 93 +++++++++++++++++++ 17 files changed, 240 insertions(+), 44 deletions(-) create mode 100644 server/src/validations/createDocument.schema.ts create mode 100644 server/src/validations/documentParams.schema.ts create mode 100644 server/src/validations/updateDocSettings.schema.ts create mode 100644 server/src/validations/updateDocument.schema.ts diff --git a/client/src/components/common/NewDocumentFormBody/new-document-form-body.tsx b/client/src/components/common/NewDocumentFormBody/new-document-form-body.tsx index 795622e..b46a8a2 100644 --- a/client/src/components/common/NewDocumentFormBody/new-document-form-body.tsx +++ b/client/src/components/common/NewDocumentFormBody/new-document-form-body.tsx @@ -64,6 +64,7 @@ export default function NewDocumentFormBody({ type="text" label="Document Title" placeholder="New document title" + maxLength={200} error={errors.title} registration={register('title', { required: 'Title is required', diff --git a/client/src/components/ui/Auth/login-form.tsx b/client/src/components/ui/Auth/login-form.tsx index df77622..57dd17a 100644 --- a/client/src/components/ui/Auth/login-form.tsx +++ b/client/src/components/ui/Auth/login-form.tsx @@ -49,6 +49,7 @@ export default function LoginForm({ id="email" label="Email" placeholder="Enter your email" + maxLength={254} registration={register('email')} error={errors.email} autoComplete="email" @@ -59,6 +60,7 @@ export default function LoginForm({ id="password" label="Password" placeholder="Enter your password" + maxLength={128} registration={register('password')} error={errors.password} autoComplete="current-password" diff --git a/client/src/components/ui/Auth/register-form.tsx b/client/src/components/ui/Auth/register-form.tsx index ef4b595..8975fb3 100644 --- a/client/src/components/ui/Auth/register-form.tsx +++ b/client/src/components/ui/Auth/register-form.tsx @@ -46,6 +46,7 @@ export default function RegisterForm({ id="email" label="Email" placeholder="Enter your email" + maxLength={254} registration={register('email')} error={errors.email} autoComplete="email" @@ -56,6 +57,7 @@ export default function RegisterForm({ label="Username" id="username" placeholder="Enter your username" + maxLength={50} registration={register('username')} error={errors.username} autoComplete="username" @@ -66,6 +68,7 @@ export default function RegisterForm({ label="Full Name" id="fullname" placeholder="Enter your full name" + maxLength={100} registration={register('fullName')} error={errors.fullName} autoComplete="name" @@ -76,6 +79,7 @@ export default function RegisterForm({ id="password" label="Password" placeholder="Enter your password" + maxLength={128} registration={register('password')} error={errors.password} autoComplete="new-password" diff --git a/client/src/features/Dashboard/components/DocumentCardDropdown/rename-modal.tsx b/client/src/features/Dashboard/components/DocumentCardDropdown/rename-modal.tsx index f4f9414..89e5144 100644 --- a/client/src/features/Dashboard/components/DocumentCardDropdown/rename-modal.tsx +++ b/client/src/features/Dashboard/components/DocumentCardDropdown/rename-modal.tsx @@ -85,6 +85,7 @@ export function RenameDocumentModal({ type="text" value={newTitle} onChange={(e) => setNewTitle(e.target.value)} + maxLength={200} className="w-full px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-transparent" placeholder="Enter document title" onKeyDown={handleKeyDown} diff --git a/client/src/features/DocumentPage/components/DocumentHeader/CollaboratorsDropdown/collaborators-dropdown.tsx b/client/src/features/DocumentPage/components/DocumentHeader/CollaboratorsDropdown/collaborators-dropdown.tsx index 66f8b3f..ffd0da6 100644 --- a/client/src/features/DocumentPage/components/DocumentHeader/CollaboratorsDropdown/collaborators-dropdown.tsx +++ b/client/src/features/DocumentPage/components/DocumentHeader/CollaboratorsDropdown/collaborators-dropdown.tsx @@ -116,6 +116,7 @@ export const CollaboratorsDropdown = ({ placeholder="Email" value={email} onChange={(e) => setEmail(e.target.value)} + maxLength={254} aria-label="Collaborator email" className="h-8 min-w-0 flex-1 rounded border border-surface-border bg-transparent px-2 text-xs outline-none focus-visible:border-ring" /> diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts index b966778..b26d8d8 100644 --- a/client/src/lib/auth.ts +++ b/client/src/lib/auth.ts @@ -7,22 +7,32 @@ import { useAuth } from '@/context/auth'; import { api } from './api'; /** - * Zod schema validating registration input: valid email, username (min 3 - * characters), password (min 8 characters), and optional full name. + * Zod schema validating registration input: valid email (max 254 chars), + * username (3-50 chars, letters/digits/dots/dashes/underscores), password + * (8-128 characters), and optional full name (max 100 chars). Mirrors the + * server-side RegisterUserSchema bounds so users never hit a 400. */ export const RegisterSchema = z.object({ - email: z.string().email(), - username: z.string().min(3), - password: z.string().min(8), - fullName: z.string().optional(), + email: z.string().email().max(254), + username: z + .string() + .min(3) + .max(50) + .regex( + /^[a-zA-Z0-9_.-]+$/, + 'Only letters, digits, dots, dashes and underscores allowed', + ), + password: z.string().min(8).max(128), + fullName: z.string().max(100).optional(), }); /** - * zod schema validating login credentials (email format + required password). + * Zod schema validating login credentials: valid email (max 254 chars) and a + * password within the server-accepted length bounds. */ export const LoginSchema = z.object({ - email: z.string().email(), - password: z.string().min(8), + email: z.string().email().max(254), + password: z.string().min(8).max(128), }); export type RegisterSchemaType = z.infer; diff --git a/server/src/controllers/document.controller.ts b/server/src/controllers/document.controller.ts index e958b23..b8fdea8 100644 --- a/server/src/controllers/document.controller.ts +++ b/server/src/controllers/document.controller.ts @@ -544,7 +544,8 @@ export const getShareLink = asyncErrorWrapper(async (req: AuthenticatedRequest, const clientInfo = getClientInfo(req); const userId = req.user?.userId; const { id } = req.params; - const { permission = 'view' } = req.query; + // Validated + defaulted by ShareLinkQuerySchema on the route + const { permission } = req.query as { permission: 'view' | 'edit' }; logger.debug('Share link generation attempt', { action: 'GENERATE_SHARE_LINK_ATTEMPT', @@ -554,19 +555,6 @@ export const getShareLink = asyncErrorWrapper(async (req: AuthenticatedRequest, permission, }); - if (!['view', 'edit'].includes(permission as string)) { - logger.warn('Share link generation failed - invalid permission', { - action: 'GENERATE_SHARE_LINK_INVALID_PERMISSION', - ...clientInfo, - userId, - documentId: id, - permission, - }); - - res.status(StatusCodes.BAD_REQUEST).json({ error: 'Invalid permission' }); - return; - } - try { const doc = await prisma.document.findUnique({ where: { id } }); @@ -584,7 +572,7 @@ export const getShareLink = asyncErrorWrapper(async (req: AuthenticatedRequest, return; } - const token = generateShareToken(doc.shareId, permission as 'view' | 'edit'); + const token = generateShareToken(doc.shareId, permission); // Updated URL structure - token is now in the path const url = `${process.env.CLIENT_BASE}/app/doc/share/${token}`; diff --git a/server/src/routers/document.router.ts b/server/src/routers/document.router.ts index c7bdddd..8f9a32c 100644 --- a/server/src/routers/document.router.ts +++ b/server/src/routers/document.router.ts @@ -19,26 +19,43 @@ import { import { authenticate } from '@/middlewares/auth.middleware'; import { validate } from '@/middlewares/validation.middleware'; import { AddCollaboratorSchema } from '@/validations/addCollaborator.schema'; +import { CreateDocumentSchema } from '@/validations/createDocument.schema'; +import { + CollaboratorParamsSchema, + IdParamsSchema, + RequestIdParamsSchema, + ShareLinkQuerySchema, +} from '@/validations/documentParams.schema'; +import { UpdateDocSettingsSchema } from '@/validations/updateDocSettings.schema'; +import { UpdateDocumentSchema } from '@/validations/updateDocument.schema'; export const docRouter = express.Router(); docRouter.use(authenticate); docRouter.get('/share/:token', getDocByToken); -docRouter.post('/', createDoc); +docRouter.post('/', validate({ body: CreateDocumentSchema }), createDoc); docRouter.get('/', getDocs); -docRouter.get('/:id', getDoc); -docRouter.put('/:id', updateDoc); -docRouter.delete('/:id', deleteDoc); - -docRouter.patch('/:id/settings', updateDocSettings); // Used to toggle allowSelfJoin for the document // !Owner only access - -docRouter.get('/:id/share-link', getShareLink); // get the document share link with the share token - -docRouter.get('/:id/collaborators', getCollaborators); // returns list -docRouter.post('/:id/collaborators', validate({ body: AddCollaboratorSchema }), addCollaborator); // adds a new one by email //!Owner only access -docRouter.delete('/:id/collaborators/:userId', removeCollaborator); // optional - -docRouter.get('/:id/requests', getRequests); // !Owner only access -docRouter.post('/:id/requests/:requestId/approve', approveRequest); -docRouter.delete('/:id/requests/:requestId/reject', rejectRequest); +docRouter.get('/:id', validate({ params: IdParamsSchema }), getDoc); +docRouter.put('/:id', validate({ params: IdParamsSchema, body: UpdateDocumentSchema }), updateDoc); +docRouter.delete('/:id', validate({ params: IdParamsSchema }), deleteDoc); + +docRouter.patch( + '/:id/settings', + validate({ params: IdParamsSchema, body: UpdateDocSettingsSchema }), + updateDocSettings +); // Used to toggle allowSelfJoin for the document // !Owner only access + +docRouter.get('/:id/share-link', validate({ params: IdParamsSchema, query: ShareLinkQuerySchema }), getShareLink); // get the document share link with the share token + +docRouter.get('/:id/collaborators', validate({ params: IdParamsSchema }), getCollaborators); // returns list +docRouter.post( + '/:id/collaborators', + validate({ params: IdParamsSchema, body: AddCollaboratorSchema }), + addCollaborator +); // adds a new one by email //!Owner only access +docRouter.delete('/:id/collaborators/:userId', validate({ params: CollaboratorParamsSchema }), removeCollaborator); // optional + +docRouter.get('/:id/requests', validate({ params: IdParamsSchema }), getRequests); // !Owner only access +docRouter.post('/:id/requests/:requestId/approve', validate({ params: RequestIdParamsSchema }), approveRequest); +docRouter.delete('/:id/requests/:requestId/reject', validate({ params: RequestIdParamsSchema }), rejectRequest); diff --git a/server/src/validations/addCollaborator.schema.ts b/server/src/validations/addCollaborator.schema.ts index 28fdb76..4e0c7dc 100644 --- a/server/src/validations/addCollaborator.schema.ts +++ b/server/src/validations/addCollaborator.schema.ts @@ -4,6 +4,7 @@ export const AddCollaboratorSchema = z.object({ email: z .string() .email() + .max(254) .transform(email => email.trim().toLowerCase()), permission: z.enum(['edit', 'view']).default('edit'), }); diff --git a/server/src/validations/createDocument.schema.ts b/server/src/validations/createDocument.schema.ts new file mode 100644 index 0000000..00cfec3 --- /dev/null +++ b/server/src/validations/createDocument.schema.ts @@ -0,0 +1,9 @@ +import { z } from 'zod'; + +export const CreateDocumentSchema = z.object({ + title: z.string().trim().min(1).max(200), + content: z.string().max(1_000_000).optional(), + isPublic: z.boolean().default(false), +}); + +export type CreateDocumentSchema = z.infer; diff --git a/server/src/validations/documentParams.schema.ts b/server/src/validations/documentParams.schema.ts new file mode 100644 index 0000000..5e19448 --- /dev/null +++ b/server/src/validations/documentParams.schema.ts @@ -0,0 +1,27 @@ +import { z } from 'zod'; + +export const IdParamsSchema = z.object({ + id: z.string().uuid(), +}); + +export type IdParamsSchema = z.infer; + +export const RequestIdParamsSchema = z.object({ + id: z.string().uuid(), + requestId: z.string().uuid(), +}); + +export type RequestIdParamsSchema = z.infer; + +export const CollaboratorParamsSchema = z.object({ + id: z.string().uuid(), + userId: z.string().uuid(), +}); + +export type CollaboratorParamsSchema = z.infer; + +export const ShareLinkQuerySchema = z.object({ + permission: z.enum(['view', 'edit']).default('view'), +}); + +export type ShareLinkQuerySchema = z.infer; diff --git a/server/src/validations/login.schema.ts b/server/src/validations/login.schema.ts index a852fe5..f763a96 100644 --- a/server/src/validations/login.schema.ts +++ b/server/src/validations/login.schema.ts @@ -4,8 +4,9 @@ export const LoginUserSchema = z.object({ email: z .string() .email() + .max(254) .transform(email => email.trim().toLowerCase()), - password: z.string().min(6), + password: z.string().min(8).max(128), }); export type LoginUserSchema = z.infer; diff --git a/server/src/validations/register.schema.ts b/server/src/validations/register.schema.ts index 6895311..c97eac8 100644 --- a/server/src/validations/register.schema.ts +++ b/server/src/validations/register.schema.ts @@ -4,10 +4,15 @@ export const RegisterUserSchema = z.object({ email: z .string() .email() + .max(254) .transform(email => email.trim().toLowerCase()), - username: z.string().min(3), - password: z.string().min(6), - fullName: z.string().optional(), + username: z + .string() + .min(3) + .max(50) + .regex(/^[a-zA-Z0-9_.-]+$/, 'Username may only contain letters, digits, dots, dashes and underscores'), + password: z.string().min(8).max(128), + fullName: z.string().max(100).optional(), }); export type RegisterUserSchema = z.infer; diff --git a/server/src/validations/updateDocSettings.schema.ts b/server/src/validations/updateDocSettings.schema.ts new file mode 100644 index 0000000..9d877b0 --- /dev/null +++ b/server/src/validations/updateDocSettings.schema.ts @@ -0,0 +1,7 @@ +import { z } from 'zod'; + +export const UpdateDocSettingsSchema = z.object({ + allowSelfJoin: z.boolean(), +}); + +export type UpdateDocSettingsSchema = z.infer; diff --git a/server/src/validations/updateDocument.schema.ts b/server/src/validations/updateDocument.schema.ts new file mode 100644 index 0000000..72429de --- /dev/null +++ b/server/src/validations/updateDocument.schema.ts @@ -0,0 +1,9 @@ +import { z } from 'zod'; + +export const UpdateDocumentSchema = z.object({ + title: z.string().trim().min(1).max(200).optional(), + content: z.string().max(1_000_000).optional(), + isPublic: z.boolean().optional(), +}); + +export type UpdateDocumentSchema = z.infer; diff --git a/server/test/auth.test.ts b/server/test/auth.test.ts index 2b1847d..afde405 100644 --- a/server/test/auth.test.ts +++ b/server/test/auth.test.ts @@ -124,6 +124,26 @@ describe('Auth Routes', () => { expect(res.status).toBe(StatusCodes.BAD_REQUEST); }); + it('should reject registration with a 6-character password', async () => { + const res = await request(app).post('/api/auth/register').send({ + email: 'shortpass@test.dev', + username: 'shortpass', + password: 'abcdef', + }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should reject registration with a username containing spaces', async () => { + const res = await request(app).post('/api/auth/register').send({ + email: 'spaceuser@test.dev', + username: 'bad name', + password: 'secure123', + }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + it('should reject registration with missing fields', async () => { const res = await request(app).post('/api/auth/register').send({ email: 'newuser@test.dev', diff --git a/server/test/document.test.ts b/server/test/document.test.ts index 41e3927..af27ad0 100644 --- a/server/test/document.test.ts +++ b/server/test/document.test.ts @@ -550,6 +550,99 @@ describe('Document Routes', () => { }); }); +describe('Document request validation (#52)', () => { + it('should reject creating a document with a missing title', async () => { + const res = await request(app).post('/api/document').set('Authorization', `Bearer ${token}`).send({}); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should reject creating a document with a non-string title', async () => { + const res = await request(app).post('/api/document').set('Authorization', `Bearer ${token}`).send({ title: 123 }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should reject creating a document with an oversized title', async () => { + const res = await request(app) + .post('/api/document') + .set('Authorization', `Bearer ${token}`) + .send({ title: 'x'.repeat(201) }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should reject creating a document with oversized content', async () => { + const res = await request(app) + .post('/api/document') + .set('Authorization', `Bearer ${token}`) + .send({ title: 'Ok', content: 'x'.repeat(1_000_001) }); + + // A >1MB body trips express.json's 100kb payload limit first (413); + // CreateDocumentSchema's content cap remains as defense-in-depth. + expect([StatusCodes.BAD_REQUEST, StatusCodes.REQUEST_TOO_LONG]).toContain(res.status); + }); + + it('should reject updating a document with a non-boolean isPublic', async () => { + const created = await prisma.document.create({ + data: { title: 'Bool Check', authorId: userId, content: '' }, + }); + + const res = await request(app) + .put(`/api/document/${created.id}`) + .set('Authorization', `Bearer ${token}`) + .send({ title: 'Still Ok', isPublic: 'yes' }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should reject a settings update with a non-boolean allowSelfJoin', async () => { + const created = await prisma.document.create({ + data: { title: 'Settings Validation', authorId: userId, content: '' }, + }); + + const res = await request(app) + .patch(`/api/document/${created.id}/settings`) + .set('Authorization', `Bearer ${token}`) + .send({ allowSelfJoin: 'yes' }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should return 400 instead of 500 when getting a document with a non-uuid id', async () => { + const res = await request(app).get('/api/document/not-a-uuid').set('Authorization', `Bearer ${token}`); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should return 400 instead of 500 when updating a document with a non-uuid id', async () => { + const res = await request(app) + .put('/api/document/not-a-uuid') + .set('Authorization', `Bearer ${token}`) + .send({ title: 'Nope' }); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should return 400 instead of 500 when deleting a document with a non-uuid id', async () => { + const res = await request(app).delete('/api/document/not-a-uuid').set('Authorization', `Bearer ${token}`); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); + + it('should reject share-link generation with an invalid permission', async () => { + const created = await prisma.document.create({ + data: { title: 'Share Perm', authorId: userId, content: '' }, + }); + + const res = await request(app) + .get(`/api/document/${created.id}/share-link?permission=admin`) + .set('Authorization', `Bearer ${token}`); + + expect(res.status).toBe(StatusCodes.BAD_REQUEST); + }); +}); + describe('Collaboration request decision scoping (#46)', () => { async function createOwnedDocument(title: string) { return prisma.document.create({