mirror of
https://github.com/Memo-2023/mana-monorepo.git
synced 2026-05-15 02:01:10 +02:00
fix(calendar,contacts,todo): second round of pre-launch audit fixes
Calendar: validate startTime < endTime on event create/update, verify share invitation recipient matches accepting user, add @MaxLength on search DTO, use ConfigService for FRONTEND_URL, fix Docker default port. Contacts: replace Error with NotFoundException in tag controller, verify contact ownership before tag operations, add @ArrayMaxSize(100) on batch DTOs, add unique constraint on contact tags (userId, name), add @MaxLength(10000) on note content, reorder photo upload for safety. Todo: add self-referencing FK on parentTaskId with cascade delete, validate parent task ownership on create, add @Min/@Max on query limit/offset, add @MaxLength(500) on search, add error handling to quick add in web app. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
83d0b64119
commit
135c636516
17 changed files with 133 additions and 44 deletions
|
|
@ -7,6 +7,7 @@ import {
|
|||
IsInt,
|
||||
Min,
|
||||
Max,
|
||||
MaxLength,
|
||||
} from 'class-validator';
|
||||
import { Transform, Type } from 'class-transformer';
|
||||
|
||||
|
|
@ -32,6 +33,7 @@ export class QueryEventsDto {
|
|||
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@MaxLength(500)
|
||||
search?: string;
|
||||
|
||||
@IsOptional()
|
||||
|
|
|
|||
|
|
@ -152,7 +152,7 @@ describe('EventService', () => {
|
|||
calendarId: calendar.id,
|
||||
title: 'New Event',
|
||||
startTime: new Date().toISOString(),
|
||||
endTime: new Date().toISOString(),
|
||||
endTime: new Date(Date.now() + 3600000).toISOString(),
|
||||
});
|
||||
|
||||
expect(result).toEqual(newEvent);
|
||||
|
|
@ -169,7 +169,7 @@ describe('EventService', () => {
|
|||
const result = await service.create(TEST_USER_ID, {
|
||||
title: 'New Event',
|
||||
startTime: new Date().toISOString(),
|
||||
endTime: new Date().toISOString(),
|
||||
endTime: new Date(Date.now() + 3600000).toISOString(),
|
||||
});
|
||||
|
||||
expect(result).toEqual(newEvent);
|
||||
|
|
@ -188,7 +188,7 @@ describe('EventService', () => {
|
|||
calendarId: calendar.id,
|
||||
title: 'New Event',
|
||||
startTime: new Date().toISOString(),
|
||||
endTime: new Date().toISOString(),
|
||||
endTime: new Date(Date.now() + 3600000).toISOString(),
|
||||
tagIds,
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,10 @@
|
|||
import { Injectable, Inject, NotFoundException, ForbiddenException } from '@nestjs/common';
|
||||
import {
|
||||
Injectable,
|
||||
Inject,
|
||||
NotFoundException,
|
||||
ForbiddenException,
|
||||
BadRequestException,
|
||||
} from '@nestjs/common';
|
||||
import { eq, and, gte, lte, inArray, or, ilike } from 'drizzle-orm';
|
||||
import { DATABASE_CONNECTION } from '../db/database.module';
|
||||
import { Database } from '../db/connection';
|
||||
|
|
@ -95,7 +101,15 @@ export class EventService {
|
|||
});
|
||||
}
|
||||
|
||||
private validateTimeRange(startTime: string, endTime: string): void {
|
||||
if (new Date(startTime) >= new Date(endTime)) {
|
||||
throw new BadRequestException('startTime must be before endTime');
|
||||
}
|
||||
}
|
||||
|
||||
async create(userId: string, dto: CreateEventDto): Promise<Event> {
|
||||
this.validateTimeRange(dto.startTime, dto.endTime);
|
||||
|
||||
let calendarId = dto.calendarId;
|
||||
let calendar;
|
||||
|
||||
|
|
@ -138,6 +152,13 @@ export class EventService {
|
|||
async update(id: string, userId: string, dto: UpdateEventDto): Promise<Event> {
|
||||
const existingEvent = await this.findByIdOrThrow(id, userId);
|
||||
|
||||
// Validate time range when either startTime or endTime is provided
|
||||
const effectiveStart = dto.startTime ?? existingEvent.startTime.toISOString();
|
||||
const effectiveEnd = dto.endTime ?? existingEvent.endTime.toISOString();
|
||||
if (dto.startTime || dto.endTime) {
|
||||
this.validateTimeRange(effectiveStart, effectiveEnd);
|
||||
}
|
||||
|
||||
// If changing calendar, verify user owns the new calendar
|
||||
if (dto.calendarId && dto.calendarId !== existingEvent.calendarId) {
|
||||
await this.calendarService.findByIdOrThrow(dto.calendarId, userId);
|
||||
|
|
|
|||
|
|
@ -57,7 +57,7 @@ export class ShareController {
|
|||
|
||||
@Post('shares/:shareId/accept')
|
||||
async acceptInvitation(@CurrentUser() user: CurrentUserData, @Param('shareId') shareId: string) {
|
||||
const share = await this.shareService.acceptInvitation(shareId, user.userId);
|
||||
const share = await this.shareService.acceptInvitation(shareId, user.userId, user.email);
|
||||
return { share };
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { Test, TestingModule } from '@nestjs/testing';
|
||||
import { NotFoundException, ForbiddenException } from '@nestjs/common';
|
||||
import { ConfigService } from '@nestjs/config';
|
||||
import { ShareService } from './share.service';
|
||||
import { CalendarService } from '../calendar/calendar.service';
|
||||
import { EmailService } from '../email/email.service';
|
||||
|
|
@ -53,6 +54,10 @@ describe('ShareService', () => {
|
|||
provide: EmailService,
|
||||
useValue: mockEmailService,
|
||||
},
|
||||
{
|
||||
provide: ConfigService,
|
||||
useValue: { get: jest.fn().mockReturnValue('http://localhost:5179') },
|
||||
},
|
||||
],
|
||||
}).compile();
|
||||
|
||||
|
|
@ -241,7 +246,7 @@ describe('ShareService', () => {
|
|||
mockDb.where.mockResolvedValueOnce([share]);
|
||||
mockDb.returning.mockResolvedValueOnce([acceptedShare]);
|
||||
|
||||
const result = await service.acceptInvitation(share.id, TEST_USER_ID);
|
||||
const result = await service.acceptInvitation(share.id, TEST_USER_ID, 'shared@example.com');
|
||||
|
||||
expect(result.status).toBe('accepted');
|
||||
expect(result.sharedWithUserId).toBe(TEST_USER_ID);
|
||||
|
|
@ -250,18 +255,18 @@ describe('ShareService', () => {
|
|||
it('should throw NotFoundException when invitation not found', async () => {
|
||||
mockDb.where.mockResolvedValueOnce([]);
|
||||
|
||||
await expect(service.acceptInvitation('non-existent-id', TEST_USER_ID)).rejects.toThrow(
|
||||
NotFoundException
|
||||
);
|
||||
await expect(
|
||||
service.acceptInvitation('non-existent-id', TEST_USER_ID, 'shared@example.com')
|
||||
).rejects.toThrow(NotFoundException);
|
||||
});
|
||||
|
||||
it('should throw ForbiddenException when invitation already processed', async () => {
|
||||
const share = createMockCalendarShare({ status: 'accepted' });
|
||||
mockDb.where.mockResolvedValueOnce([share]);
|
||||
|
||||
await expect(service.acceptInvitation(share.id, TEST_USER_ID)).rejects.toThrow(
|
||||
ForbiddenException
|
||||
);
|
||||
await expect(
|
||||
service.acceptInvitation(share.id, TEST_USER_ID, 'shared@example.com')
|
||||
).rejects.toThrow(ForbiddenException);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { Injectable, Inject, NotFoundException, ForbiddenException, Logger } from '@nestjs/common';
|
||||
import { ConfigService } from '@nestjs/config';
|
||||
import { eq, and, or } from 'drizzle-orm';
|
||||
import { randomBytes } from 'crypto';
|
||||
import { DATABASE_CONNECTION } from '../db/database.module';
|
||||
|
|
@ -19,7 +20,8 @@ export class ShareService {
|
|||
constructor(
|
||||
@Inject(DATABASE_CONNECTION) private db: Database,
|
||||
private calendarService: CalendarService,
|
||||
private emailService: EmailService
|
||||
private emailService: EmailService,
|
||||
private configService: ConfigService
|
||||
) {}
|
||||
|
||||
async findByCalendar(calendarId: string, userId: string): Promise<CalendarShare[]> {
|
||||
|
|
@ -76,7 +78,7 @@ export class ShareService {
|
|||
// Send invitation email if sharing with specific email
|
||||
if (dto.email && !dto.createLink) {
|
||||
const inviterName = inviterEmail.split('@')[0];
|
||||
const baseUrl = process.env.FRONTEND_URL || 'http://localhost:5179';
|
||||
const baseUrl = this.configService.get<string>('FRONTEND_URL', 'http://localhost:5179');
|
||||
const acceptUrl = `${baseUrl}/shares/${created.id}/accept`;
|
||||
|
||||
try {
|
||||
|
|
@ -131,7 +133,7 @@ export class ShareService {
|
|||
await this.db.delete(calendarShares).where(eq(calendarShares.id, id));
|
||||
}
|
||||
|
||||
async acceptInvitation(shareId: string, userId: string): Promise<CalendarShare> {
|
||||
async acceptInvitation(shareId: string, userId: string, email: string): Promise<CalendarShare> {
|
||||
const share = await this.findById(shareId);
|
||||
if (!share) {
|
||||
throw new NotFoundException(`Invitation not found`);
|
||||
|
|
@ -141,6 +143,15 @@ export class ShareService {
|
|||
throw new ForbiddenException('Invitation has already been processed');
|
||||
}
|
||||
|
||||
// Validate that the accepting user matches the invitation target
|
||||
const matchesUserId = share.sharedWithUserId && share.sharedWithUserId === userId;
|
||||
const matchesEmail = share.sharedWithEmail && share.sharedWithEmail === email;
|
||||
if (share.sharedWithUserId || share.sharedWithEmail) {
|
||||
if (!matchesUserId && !matchesEmail) {
|
||||
throw new ForbiddenException('You are not the intended recipient of this invitation');
|
||||
}
|
||||
}
|
||||
|
||||
const [updated] = await this.db
|
||||
.update(calendarShares)
|
||||
.set({
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
FROM node:20-alpine AS builder
|
||||
|
||||
# Build arguments for SvelteKit static env vars
|
||||
ARG PUBLIC_BACKEND_URL=http://calendar-backend:3016
|
||||
ARG PUBLIC_BACKEND_URL=http://calendar-backend:3014
|
||||
ARG PUBLIC_MANA_CORE_AUTH_URL=http://mana-core-auth:3001
|
||||
|
||||
# Set as environment variables for build
|
||||
|
|
|
|||
|
|
@ -1,12 +1,20 @@
|
|||
import { Controller, Post, Body, UseGuards } from '@nestjs/common';
|
||||
import { JwtAuthGuard, CurrentUser, CurrentUserData } from '@manacore/shared-nestjs-auth';
|
||||
import { BatchService } from './batch.service';
|
||||
import { IsArray, IsString, IsBoolean, IsOptional, ArrayMinSize } from 'class-validator';
|
||||
import {
|
||||
IsArray,
|
||||
IsString,
|
||||
IsBoolean,
|
||||
IsOptional,
|
||||
ArrayMinSize,
|
||||
ArrayMaxSize,
|
||||
} from 'class-validator';
|
||||
|
||||
class BatchContactIdsDto {
|
||||
@IsArray()
|
||||
@IsString({ each: true })
|
||||
@ArrayMinSize(1)
|
||||
@ArrayMaxSize(100)
|
||||
contactIds: string[];
|
||||
}
|
||||
|
||||
|
|
@ -26,6 +34,7 @@ class BatchTagsDto extends BatchContactIdsDto {
|
|||
@IsArray()
|
||||
@IsString({ each: true })
|
||||
@ArrayMinSize(1)
|
||||
@ArrayMaxSize(100)
|
||||
tagIds: string[];
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { pgTable, uuid, timestamp, varchar, primaryKey, index } from 'drizzle-orm/pg-core';
|
||||
import { pgTable, uuid, timestamp, varchar, primaryKey, index, unique } from 'drizzle-orm/pg-core';
|
||||
import { contacts } from './contacts.schema';
|
||||
|
||||
export const contactTags = pgTable(
|
||||
|
|
@ -12,6 +12,7 @@ export const contactTags = pgTable(
|
|||
},
|
||||
(table) => ({
|
||||
userIdx: index('contact_tags_user_idx').on(table.userId),
|
||||
userNameUnique: unique('contact_tags_user_name_unique').on(table.userId, table.name),
|
||||
})
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -11,10 +11,11 @@ import {
|
|||
} from '@nestjs/common';
|
||||
import { JwtAuthGuard, CurrentUser, CurrentUserData } from '@manacore/shared-nestjs-auth';
|
||||
import { NoteService } from './note.service';
|
||||
import { IsString, IsOptional, IsBoolean } from 'class-validator';
|
||||
import { IsString, IsOptional, IsBoolean, MaxLength } from 'class-validator';
|
||||
|
||||
class CreateNoteDto {
|
||||
@IsString()
|
||||
@MaxLength(10000)
|
||||
content!: string;
|
||||
|
||||
@IsBoolean()
|
||||
|
|
@ -25,6 +26,7 @@ class CreateNoteDto {
|
|||
class UpdateNoteDto {
|
||||
@IsString()
|
||||
@IsOptional()
|
||||
@MaxLength(10000)
|
||||
content?: string;
|
||||
|
||||
@IsBoolean()
|
||||
|
|
|
|||
|
|
@ -56,23 +56,11 @@ export class PhotoService {
|
|||
throw new BadRequestException('Contact not found');
|
||||
}
|
||||
|
||||
// Delete old photo if exists
|
||||
if (contact.photoUrl) {
|
||||
try {
|
||||
const oldKey = this.extractKeyFromUrl(contact.photoUrl);
|
||||
if (oldKey) {
|
||||
await this.storage.delete(oldKey);
|
||||
}
|
||||
} catch {
|
||||
// Ignore deletion errors
|
||||
}
|
||||
}
|
||||
|
||||
// Generate unique key for the new photo
|
||||
const filename = `${contactId}.${extension}`;
|
||||
const key = generateUserFileKey(userId, filename);
|
||||
|
||||
// Upload to S3
|
||||
// Upload new photo to S3 first (safe: if this fails, nothing is lost)
|
||||
const contentType = getContentType(filename);
|
||||
await this.storage.upload(key, file.buffer, {
|
||||
contentType,
|
||||
|
|
@ -89,6 +77,18 @@ export class PhotoService {
|
|||
.set({ photoUrl, updatedAt: new Date() })
|
||||
.where(eq(contacts.id, contactId));
|
||||
|
||||
// Delete old photo if exists (after successful upload + DB update)
|
||||
if (contact.photoUrl) {
|
||||
try {
|
||||
const oldKey = this.extractKeyFromUrl(contact.photoUrl);
|
||||
if (oldKey) {
|
||||
await this.storage.delete(oldKey);
|
||||
}
|
||||
} catch {
|
||||
// Ignore deletion errors - orphaned old file is harmless
|
||||
}
|
||||
}
|
||||
|
||||
return { photoUrl };
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -8,9 +8,11 @@ import {
|
|||
Param,
|
||||
UseGuards,
|
||||
ParseUUIDPipe,
|
||||
NotFoundException,
|
||||
} from '@nestjs/common';
|
||||
import { JwtAuthGuard, CurrentUser, CurrentUserData } from '@manacore/shared-nestjs-auth';
|
||||
import { TagService } from './tag.service';
|
||||
import { ContactService } from '../contact/contact.service';
|
||||
import { IsString, IsOptional, MaxLength } from 'class-validator';
|
||||
|
||||
class CreateTagDto {
|
||||
|
|
@ -39,7 +41,10 @@ class UpdateTagDto {
|
|||
@Controller('tags')
|
||||
@UseGuards(JwtAuthGuard)
|
||||
export class TagController {
|
||||
constructor(private readonly tagService: TagService) {}
|
||||
constructor(
|
||||
private readonly tagService: TagService,
|
||||
private readonly contactService: ContactService
|
||||
) {}
|
||||
|
||||
@Get()
|
||||
async findAll(@CurrentUser() user: CurrentUserData) {
|
||||
|
|
@ -81,7 +86,12 @@ export class TagController {
|
|||
// Verify tag belongs to user
|
||||
const tag = await this.tagService.findById(tagId, user.userId);
|
||||
if (!tag) {
|
||||
throw new Error('Tag not found');
|
||||
throw new NotFoundException('Tag not found');
|
||||
}
|
||||
// Verify contact belongs to user
|
||||
const contact = await this.contactService.findById(contactId, user.userId);
|
||||
if (!contact) {
|
||||
throw new NotFoundException('Contact not found');
|
||||
}
|
||||
await this.tagService.addTagToContact(contactId, tagId);
|
||||
return { success: true };
|
||||
|
|
@ -96,7 +106,12 @@ export class TagController {
|
|||
// Verify tag belongs to user
|
||||
const tag = await this.tagService.findById(tagId, user.userId);
|
||||
if (!tag) {
|
||||
throw new Error('Tag not found');
|
||||
throw new NotFoundException('Tag not found');
|
||||
}
|
||||
// Verify contact belongs to user
|
||||
const contact = await this.contactService.findById(contactId, user.userId);
|
||||
if (!contact) {
|
||||
throw new NotFoundException('Contact not found');
|
||||
}
|
||||
await this.tagService.removeTagFromContact(contactId, tagId);
|
||||
return { success: true };
|
||||
|
|
|
|||
|
|
@ -1,8 +1,10 @@
|
|||
import { Module } from '@nestjs/common';
|
||||
import { TagController } from './tag.controller';
|
||||
import { TagService } from './tag.service';
|
||||
import { ContactModule } from '../contact/contact.module';
|
||||
|
||||
@Module({
|
||||
imports: [ContactModule],
|
||||
controllers: [TagController],
|
||||
providers: [TagService],
|
||||
exports: [TagService],
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import {
|
|||
integer,
|
||||
jsonb,
|
||||
index,
|
||||
foreignKey,
|
||||
} from 'drizzle-orm/pg-core';
|
||||
import { projects } from './projects.schema';
|
||||
import { kanbanColumns } from './kanban-columns.schema';
|
||||
|
|
@ -110,6 +111,10 @@ export const tasks = pgTable(
|
|||
table.userId,
|
||||
table.scheduledDate
|
||||
),
|
||||
parentTaskFk: foreignKey({
|
||||
columns: [table.parentTaskId],
|
||||
foreignColumns: [table.id],
|
||||
}).onDelete('cascade'),
|
||||
})
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -6,6 +6,9 @@ import {
|
|||
IsString,
|
||||
IsNumber,
|
||||
IsDateString,
|
||||
Min,
|
||||
Max,
|
||||
MaxLength,
|
||||
} from 'class-validator';
|
||||
import { Transform } from 'class-transformer';
|
||||
import type { TaskPriority, TaskStatus } from '../../db/schema/tasks.schema';
|
||||
|
|
@ -46,6 +49,7 @@ export class QueryTasksDto {
|
|||
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@MaxLength(500)
|
||||
search?: string;
|
||||
|
||||
@IsOptional()
|
||||
|
|
@ -59,10 +63,13 @@ export class QueryTasksDto {
|
|||
@IsOptional()
|
||||
@Transform(({ value }) => parseInt(value, 10))
|
||||
@IsNumber()
|
||||
@Min(0)
|
||||
@Max(100)
|
||||
limit?: number;
|
||||
|
||||
@IsOptional()
|
||||
@Transform(({ value }) => parseInt(value, 10))
|
||||
@IsNumber()
|
||||
@Min(0)
|
||||
offset?: number;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -106,6 +106,11 @@ export class TaskService {
|
|||
await this.projectService.findByIdOrThrow(dto.projectId, userId);
|
||||
}
|
||||
|
||||
// Verify parent task belongs to user if provided
|
||||
if (dto.parentTaskId) {
|
||||
await this.findByIdOrThrow(dto.parentTaskId, userId);
|
||||
}
|
||||
|
||||
// Get the highest order value for the project
|
||||
const existingTasks = await this.findAll(userId, { projectId: dto.projectId ?? undefined });
|
||||
const maxOrder = existingTasks.reduce((max, t) => Math.max(max, t.order ?? 0), -1);
|
||||
|
|
|
|||
|
|
@ -89,16 +89,20 @@
|
|||
async function handleCreate(query: string): Promise<void> {
|
||||
if (!query.trim()) return;
|
||||
|
||||
const parsed = parseTaskInput(query);
|
||||
const resolved = resolveTaskIds(parsed, projectsStore.projects, labelsStore.labels);
|
||||
try {
|
||||
const parsed = parseTaskInput(query);
|
||||
const resolved = resolveTaskIds(parsed, projectsStore.projects, labelsStore.labels);
|
||||
|
||||
const result = await tasksStore.createTask({
|
||||
title: resolved.title,
|
||||
dueDate: resolved.dueDate,
|
||||
priority: resolved.priority,
|
||||
projectId: resolved.projectId,
|
||||
labelIds: resolved.labelIds,
|
||||
});
|
||||
await tasksStore.createTask({
|
||||
title: resolved.title,
|
||||
dueDate: resolved.dueDate,
|
||||
priority: resolved.priority,
|
||||
projectId: resolved.projectId,
|
||||
labelIds: resolved.labelIds,
|
||||
});
|
||||
} catch (error) {
|
||||
console.error('Failed to create task:', error);
|
||||
}
|
||||
}
|
||||
|
||||
// PillNav collapsed state (controlled by FAB)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue