diff --git a/README.md b/README.md index de7d5e8..7e5ec8c 100644 --- a/README.md +++ b/README.md @@ -428,7 +428,7 @@ For commercial licensing questions, hosted service permissions, or other use cas Do not publish security vulnerabilities in public GitHub issues. -Report security issues privately using the contact information provided in the repository or on the TaskView website. +Report security issues privately — see [SECURITY.md](SECURITY.md) for the reporting channels, response times, scope, and safe-harbor terms. When running TaskView in production: diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..cfe7de2 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,46 @@ +# Security Policy + +## Reporting a vulnerability + +Please do not report security vulnerabilities through public GitHub issues, discussions, or pull requests. + +Report them privately using one of these channels: + +- **GitHub private vulnerability reporting** (preferred): open the **Security** tab of this repository and click **Report a vulnerability**. +- **Email**: [support@taskview.tech](mailto:support@taskview.tech) with `[security]` in the subject. + +Please include as much of the following as you can: + +- A description of the issue and its impact +- Affected component (API, web app, MCP server, mobile app) and version +- Steps to reproduce, or a proof of concept +- Any suggested mitigation + +## What to expect + +- We will acknowledge your report within **5 business days**. +- We will keep you informed about progress and aim to release a fix for confirmed issues within **90 days** of the report, sooner for critical issues. +- Once a fix is released, we publish a GitHub Security Advisory for the affected versions and credit the reporter, unless they prefer to stay anonymous. +- We ask that you give us a reasonable time to fix the issue before disclosing it publicly. + +## Supported versions + +Security fixes are released for the latest minor version line only. Self-hosted installations should upgrade to the latest release to receive them. + +## Scope + +In scope: + +- The TaskView API server, web app, MCP server, and mobile app in this repository +- The hosted service at `app.taskview.tech` + +Out of scope: + +- Vulnerabilities in third-party dependencies that are not exploitable in TaskView (report them upstream) +- Findings that require a compromised admin account or physical access to the server +- Missing security headers, rate limiting, or best-practice recommendations without a demonstrated impact +- Denial-of-service testing against the hosted service + +## Safe harbor + +We will not pursue legal action against researchers who act in good faith: test only against their own self-hosted instance or their own accounts on the hosted service, avoid accessing or modifying other users' data, and report findings privately as described above. diff --git a/api/package.json b/api/package.json index a37493f..07e4ca3 100644 --- a/api/package.json +++ b/api/package.json @@ -1,6 +1,6 @@ { "name": "taskview-ce-api-server", - "version": "1.52.0", + "version": "1.53.0", "scripts": { "dev": "bun run --watch ./server.ts", "start": "NODE_ENV=production node ./dist/taskview-server.js", diff --git a/api/src/tv-modules/analytics/sections/SectionRegistry.ts b/api/src/tv-modules/analytics/sections/SectionRegistry.ts index b373da7..0068250 100644 --- a/api/src/tv-modules/analytics/sections/SectionRegistry.ts +++ b/api/src/tv-modules/analytics/sections/SectionRegistry.ts @@ -5,18 +5,12 @@ import { OverdueKpi } from './kpi/OverdueKpi' import { ThroughputSection } from './productivity/ThroughputSection' import { PriorityMixOverTimeSection } from './productivity/PriorityMixOverTimeSection' import { WorkloadByAssigneeSection } from './workload/WorkloadByAssigneeSection' -import { BlockedByDependenciesSection } from './workload/BlockedByDependenciesSection' -import { OverdueByAgeSection } from './quality/OverdueByAgeSection' import { StaleTasksSection } from './quality/StaleTasksSection' import { StatusDistributionSection } from './usage/StatusDistributionSection' -import { ActiveProjectsSection } from './usage/ActiveProjectsSection' import { IncomeExpenseMonthSection } from './financial/IncomeExpenseMonthSection' import { IncomeExpensePerProjectSection } from './financial/IncomeExpensePerProjectSection' import { IncomePerProjectMonthSection } from './financial/IncomePerProjectMonthSection' import { ExpensePerProjectMonthSection } from './financial/ExpensePerProjectMonthSection' -import { IncomePerTagMonthSection } from './financial/IncomePerTagMonthSection' -import { ExpensePerTagMonthSection } from './financial/ExpensePerTagMonthSection' -import { TopProjectsByAmountSection } from './financial/TopProjectsByAmountSection' import { AmountCoverageKpi } from './financial/AmountCoverageKpi' import { TotalIncomeKpi } from './financial/TotalIncomeKpi' import { TotalExpenseKpi } from './financial/TotalExpenseKpi' @@ -59,6 +53,12 @@ import { sectionLocales } from './locales' // - entered_at) for rows in that status. Without a transition log, this // metric cannot be computed correctly. // --------------------------------------------------------------------------- +// import { BlockedByDependenciesSection } from './workload/BlockedByDependenciesSection' +// import { ActiveProjectsSection } from './usage/ActiveProjectsSection' +// import { OverdueByAgeSection } from './quality/OverdueByAgeSection' +// import { TopProjectsByAmountSection } from './financial/TopProjectsByAmountSection' +// import { IncomePerTagMonthSection } from './financial/IncomePerTagMonthSection' +// import { ExpensePerTagMonthSection } from './financial/ExpensePerTagMonthSection' // import { AgingOpenTasksSection } from './workload/AgingOpenTasksSection' // import { TimeInKanbanStatusSection } from './workload/TimeInKanbanStatusSection' // import { CycleTimeKpi } from './kpi/CycleTimeKpi' @@ -82,25 +82,25 @@ const builders: SectionBuilder[] = [ new PriorityMixOverTimeSection(), // Workload new WorkloadByAssigneeSection(), - new BlockedByDependenciesSection(), + // new BlockedByDependenciesSection(), // disabled // new TimeInKanbanStatusSection(), // disabled — see top-of-file comment // new AgingOpenTasksSection(), // disabled — see top-of-file comment // Quality - new OverdueByAgeSection(), + // new OverdueByAgeSection(), // disabled // new CycleTimeHistogramSection(), // disabled — see top-of-file comment new StaleTasksSection(), // new CycleTimePerProjectSection(), // disabled — see top-of-file comment // Usage new StatusDistributionSection(), - new ActiveProjectsSection(), + // new ActiveProjectsSection(), // disabled // Financial new IncomeExpenseMonthSection(), new IncomeExpensePerProjectSection(), new IncomePerProjectMonthSection(), new ExpensePerProjectMonthSection(), - new IncomePerTagMonthSection(), - new ExpensePerTagMonthSection(), - new TopProjectsByAmountSection(), + // new IncomePerTagMonthSection(), // disabled + // new ExpensePerTagMonthSection(), // disabled + // new TopProjectsByAmountSection(), // disabled ] export class SectionRegistry { diff --git a/api/src/tv-modules/auth/AuthController.ts b/api/src/tv-modules/auth/AuthController.ts index 5979b24..ec67c35 100644 --- a/api/src/tv-modules/auth/AuthController.ts +++ b/api/src/tv-modules/auth/AuthController.ts @@ -367,6 +367,16 @@ export default class AuthController { // Invalidate code immediately to prevent replay attacks await req.appUser.authManager.repository.updateLoginCode(null, userData.email); + if (userData.block) { + if (!userData.confirm_email_code) { + return res.status(403).send({ message: 'account_blocked' }); + } + const confirmed = await req.appUser.authManager.repository.markEmailConfirmed(userData.email); + if (!confirmed) { + return res.status(500).end(); + } + } + const sessionId = await req.appUser.authManager.sessionStorage.createSession( userData.id, req.ip, diff --git a/api/src/tv-modules/auth/AuthModel.ts b/api/src/tv-modules/auth/AuthModel.ts index ae04b07..bbb3832 100644 --- a/api/src/tv-modules/auth/AuthModel.ts +++ b/api/src/tv-modules/auth/AuthModel.ts @@ -119,6 +119,21 @@ export default class AuthModel { } } + async markEmailConfirmed(email: string): Promise { + if (!email) return false; + + try { + const result = await this.db.dbDrizzle + .update(UsersSchema) + .set({ confirmEmailCode: null, block: 0 }) + .where(eq(UsersSchema.email, email)); + return (result.rowCount ?? 0) > 0; + } catch (error) { + $logger.error(error, `Error marking email confirmed for ${email}`); + return false; + } + } + async confirmEmail(login: string, code: string, block: number): Promise { const query = `UPDATE tv_auth.users SET confirm_email_code = NULL, block = $1 diff --git a/api/src/tv-modules/auth/__tests__/Auth.spec.ts b/api/src/tv-modules/auth/__tests__/Auth.spec.ts index 13e46cf..81be594 100644 --- a/api/src/tv-modules/auth/__tests__/Auth.spec.ts +++ b/api/src/tv-modules/auth/__tests__/Auth.spec.ts @@ -512,4 +512,65 @@ describe('Login API', () => { expect(te).toBe(0); }); + + it('loginByCode confirms and admits a blocked-unconfirmed account', async () => { + deleteTestUserEmail = `${Date.now()}test@mail.dest`; + const email = deleteTestUserEmail; + + await axios.post(`${url}/module/auth/registration`, { + email, + password: 'user1!#Q', + passwordRepeat: 'user1!#Q', + }); + + const userModel = new AuthModel(); + const before = await userModel.getUserByLogin(email, true); + expect(before).toBeTruthy(); + expect((before as any).block).toBe(1); + expect((before as any).confirm_email_code).toBeTruthy(); + + const code = '654321'; + await userModel.updateLoginCode(`${code}:${Date.now()}`, email); + + const response = await axios.post(`${url}/module/auth/login-by-code`, { email, code }); + + expect(response.status).toBe(200); + expect(response.data.access).toBeTruthy(); + expect(response.data.refresh).toBeTruthy(); + + const after = await userModel.getUserByLogin(email, true); + expect((after as any).block).toBe(0); + expect((after as any).confirm_email_code).toBeNull(); + }); + + it('loginByCode rejects a banned account (blocked, no confirm code)', async () => { + deleteTestUserEmail = `${Date.now()}test@mail.dest`; + const email = deleteTestUserEmail; + + await axios.post(`${url}/module/auth/registration`, { + email, + password: 'user1!#Q', + passwordRepeat: 'user1!#Q', + }); + + const db = Database.getInstance(); + await db.query('update tv_auth.users set block = 1, confirm_email_code = null where email = $1', [email]); + + const userModel = new AuthModel(); + const code = '112233'; + await userModel.updateLoginCode(`${code}:${Date.now()}`, email); + + let status = 0; + let message = ''; + await axios.post(`${url}/module/auth/login-by-code`, { email, code }).catch((err) => { + status = err.response.status; + message = err.response.data.message; + }); + + expect(status).toBe(403); + expect(message).toBe('account_blocked'); + + const after = await userModel.getUserByLogin(email, true); + expect((after as any).block).toBe(1); + }); }); diff --git a/api/src/tv-modules/collaboration-roles/middlewares/CanFetchRolesPermissionsCollaborationRoles.ts b/api/src/tv-modules/collaboration-roles/middlewares/CanFetchRolesPermissionsCollaborationRoles.ts index 4ddc288..46bdc99 100644 --- a/api/src/tv-modules/collaboration-roles/middlewares/CanFetchRolesPermissionsCollaborationRoles.ts +++ b/api/src/tv-modules/collaboration-roles/middlewares/CanFetchRolesPermissionsCollaborationRoles.ts @@ -5,7 +5,8 @@ import { GoalPermissions } from '../../../types/auth.types'; import { logError } from '../../../utils/api'; export const CanFetchRolesPermissionsCollaborationRoles = async (req: Request, res: Response, next: NextFunction) => { - const goalId = req.body.goalId ? req.body.goalId : req.params.goalId; + // the only route using this guard names the goal in the path + const goalId = req.params.goalId; if (!goalId) { return res.status(400).end(); diff --git a/api/src/tv-modules/collaboration/CollaborationRoutes.ts b/api/src/tv-modules/collaboration/CollaborationRoutes.ts index b0391c2..c34fe9d 100644 --- a/api/src/tv-modules/collaboration/CollaborationRoutes.ts +++ b/api/src/tv-modules/collaboration/CollaborationRoutes.ts @@ -5,7 +5,7 @@ import { IsOrgMemberIfProvided } from '../../middlewares/is-org-member'; import { CollaborationController } from './CollaborationController'; import { CanAddUserCollaboration } from './middlewares/CanAddUserCollaboration'; import { CanDeleteUserCollaboration } from './middlewares/CanDeleteUserCollaboration'; -// import { CanFetchUsersCollaboration } from './middlewares/CanFetchUsersCollaboration'; +import { CanFetchUsersCollaboration } from './middlewares/CanFetchUsersCollaboration'; import { CanToggleRolesCollaboration } from './middlewares/CanToggleRolesCollaboration'; export default class CollaborationRoutes implements Routable { @@ -56,6 +56,10 @@ export default class CollaborationRoutes implements Routable { /** * Fetch users for goal for collaboration */ - this.router.get('/:goalId', [IsLoggedIn], this.collaborationController.fetchUsersForGoalNew); + this.router.get( + '/:goalId', + [IsLoggedIn, CanFetchUsersCollaboration], + this.collaborationController.fetchUsersForGoalNew + ); } } diff --git a/api/src/tv-modules/collaboration/middlewares/CanFetchUsersCollaboration.ts b/api/src/tv-modules/collaboration/middlewares/CanFetchUsersCollaboration.ts index 9431823..7cd52c2 100644 --- a/api/src/tv-modules/collaboration/middlewares/CanFetchUsersCollaboration.ts +++ b/api/src/tv-modules/collaboration/middlewares/CanFetchUsersCollaboration.ts @@ -21,7 +21,8 @@ export const CanFetchUsersCollaboration = async (req: Request, res: Response, ne if ( permissions.hasPermissions(GoalPermissions.TASKS_CAN_ASSIGN_USERS) || - permissions.hasPermissions(GoalPermissions.GOAL_CAN_MANAGE_USERS) + permissions.hasPermissions(GoalPermissions.GOAL_CAN_MANAGE_USERS) || + permissions.hasPermissions(GoalPermissions.TASKS_CAN_WATCH_ASSIGNED_USERS) ) { return next(); } diff --git a/api/src/tv-modules/graph/GraphControler.ts b/api/src/tv-modules/graph/GraphControler.ts index 130a89b..ed7c5ed 100644 --- a/api/src/tv-modules/graph/GraphControler.ts +++ b/api/src/tv-modules/graph/GraphControler.ts @@ -33,6 +33,15 @@ export class GraphController { return res.tvJson(edges); }; + fetchTaskEdges = async (req: Request, res: Response) => { + const taskId = Number(req.params.taskId); + if (!Number.isFinite(taskId)) { + return res.status(400).send('Task ID is required'); + } + const edges = await req.appUser.graphManager.fetchEdgesForTask(taskId); + return res.tvJson(edges); + }; + deleteEdge = async (req: Request, res: Response) => { if (!req.params.id) { return res.status(400).send('Edge ID is required'); diff --git a/api/src/tv-modules/graph/GraphManager.ts b/api/src/tv-modules/graph/GraphManager.ts index fab0724..9e8530d 100644 --- a/api/src/tv-modules/graph/GraphManager.ts +++ b/api/src/tv-modules/graph/GraphManager.ts @@ -19,6 +19,10 @@ export class GraphManager { return await this.repository.fetchAllEdges(goalId); } + async fetchEdgesForTask(taskId: number) { + return await this.repository.fetchEdgesForTask(taskId); + } + async deleteEdge(id: number) { return await this.repository.deleteEdge(id); } diff --git a/api/src/tv-modules/graph/GraphRepository.ts b/api/src/tv-modules/graph/GraphRepository.ts index 6d99678..17e49ca 100644 --- a/api/src/tv-modules/graph/GraphRepository.ts +++ b/api/src/tv-modules/graph/GraphRepository.ts @@ -1,4 +1,4 @@ -import { eq } from 'drizzle-orm'; +import { eq, or } from 'drizzle-orm'; import { GraphRelationsSchema } from 'taskview-db-schemas'; import { Database } from '../../modules/db'; import { callWithCatch } from '../../utils/helpers'; @@ -32,6 +32,16 @@ export class GraphRepository { return result ?? []; } + public async fetchEdgesForTask(taskId: number): Promise { + const result = await callWithCatch(() => + this.db.dbDrizzle + .select() + .from(GraphRelationsSchema) + .where(or(eq(GraphRelationsSchema.fromTaskId, taskId), eq(GraphRelationsSchema.toTaskId, taskId))) + ); + return result ?? []; + } + public async deleteEdge(id: number): Promise { const result = await callWithCatch(() => this.db.dbDrizzle.delete(GraphRelationsSchema).where(eq(GraphRelationsSchema.id, id)) diff --git a/api/src/tv-modules/graph/GraphRoutes.ts b/api/src/tv-modules/graph/GraphRoutes.ts index c136517..501d045 100644 --- a/api/src/tv-modules/graph/GraphRoutes.ts +++ b/api/src/tv-modules/graph/GraphRoutes.ts @@ -21,6 +21,7 @@ export default class GraphRoutes implements Routable { initRoutes() { this.router.post('', [IsLoggedIn, CanManageGraph], this.graphController.addEdge); + this.router.get('/task/:taskId', [IsLoggedIn, CanViewGraph], this.graphController.fetchTaskEdges); this.router.get('/:goalId', [IsLoggedIn, CanViewGraph], this.graphController.fetchAllEdges); this.router.delete('/:id', [IsLoggedIn, CanManageGraph], this.graphController.deleteEdge); } diff --git a/api/src/tv-modules/graph/middlewares/resolveGoalId.ts b/api/src/tv-modules/graph/middlewares/resolveGoalId.ts index cc26308..40ad31f 100644 --- a/api/src/tv-modules/graph/middlewares/resolveGoalId.ts +++ b/api/src/tv-modules/graph/middlewares/resolveGoalId.ts @@ -3,28 +3,32 @@ import { GraphRepository } from '../GraphRepository'; import { TasksRepository } from '../../tasks/TasksRepository'; /** - * Resolves goalId from graph request. - * - GET /:goalId → params.goalId - * - POST (addEdge) → resolve via fromTaskId (body.source) - * - DELETE /:id → resolve via edge id + * Resolves the single goal a graph request belongs to. + * + * The source is chosen by what the route actually carries, not by probing every + * field in turn: a route parameter always wins, and only a request with no + * parameters at all (addEdge) is resolved from the body. Reading the body first + * would let a caller point the guard at a task they own while the handler acts + * on someone else's edge. + * + * A graph lives inside one project, so an edge whose endpoints sit in different + * goals is not a permission question — it is an impossible object. It resolves + * to null and the guards reject it before any permission is considered, the same + * invariant the tasks.check_task_graph_relation_goal trigger enforces in the DB. */ export async function resolveGoalId(req: Request): Promise { - // Direct goalId in params (fetchAllEdges) + // fetchAllEdges: GET /:goalId if (req.params.goalId) { - const id = Number(req.params.goalId); - return isNaN(id) ? null : id; + const goalId = Number(req.params.goalId); + return isNaN(goalId) ? null : goalId; } - // addEdge: resolve goalId from task - if (req.body?.source) { - const taskId = Number(req.body.source); - if (isNaN(taskId)) return null; - const tasksRepo = new TasksRepository(); - const task = await tasksRepo.fetchTaskByIdNew(taskId); - return task?.goalId ?? null; + // fetchTaskEdges: GET /task/:taskId + if (req.params.taskId) { + return goalIdForTask(req.params.taskId); } - // deleteEdge: resolve goalId from edge + // deleteEdge: DELETE /:id if (req.params.id) { const edgeId = Number(req.params.id); if (isNaN(edgeId)) return null; @@ -33,5 +37,25 @@ export async function resolveGoalId(req: Request): Promise { return edge?.goalId ?? null; } + // addEdge: POST with { source, target } — both endpoints must be in one goal + if (req.body?.source) { + const sourceGoalId = await goalIdForTask(req.body.source); + if (sourceGoalId === null) return null; + + const targetGoalId = await goalIdForTask(req.body.target); + if (targetGoalId !== sourceGoalId) return null; + + return sourceGoalId; + } + return null; } + +async function goalIdForTask(rawTaskId: unknown): Promise { + const taskId = Number(rawTaskId); + if (!taskId || isNaN(taskId)) return null; + + const tasksRepo = new TasksRepository(); + const task = await tasksRepo.fetchTaskByIdNew(taskId); + return task?.goalId ?? null; +} diff --git a/api/src/tv-modules/integrations/middlewares/resolveProjectId.ts b/api/src/tv-modules/integrations/middlewares/resolveProjectId.ts index 5c939fb..4cd6a70 100644 --- a/api/src/tv-modules/integrations/middlewares/resolveProjectId.ts +++ b/api/src/tv-modules/integrations/middlewares/resolveProjectId.ts @@ -2,22 +2,26 @@ import type { Request } from 'express'; import { IntegrationsRepository } from '../IntegrationsRepository'; /** - * Resolves projectId from request. - * Checks body (projectId, integrationId, id) and query (projectId, integrationId). + * Resolves the project to authorize the request against. + * + * When the request names an integration, the project is derived from that + * integration and a projectId supplied by the caller is ignored: every handler + * that takes an integration id acts on the integration, so authorizing a + * caller-supplied project would guard a different object than the one touched. + * + * Only create and fetch carry no integration id — there the project itself is + * the object being acted on, so it is read from the request. */ export async function resolveProjectId(req: Request): Promise { - // Direct projectId in body or query - const directId = req.body?.projectId ?? req.query?.projectId; - if (directId) { - const id = Number(directId); - return isNaN(id) ? null : id; + const integrationId = Number(req.body?.integrationId || req.query?.integrationId || req.body?.id); + if (integrationId && !isNaN(integrationId)) { + const repo = new IntegrationsRepository(); + const integration = await repo.fetchById(integrationId); + return integration?.projectId ?? null; } - // integrationId from body or query, or id from body - const integrationId = Number(req.body?.integrationId || req.query?.integrationId || req.body?.id); - if (!integrationId || isNaN(integrationId)) return null; + const projectId = Number(req.body?.projectId || req.query?.projectId); + if (!projectId || isNaN(projectId)) return null; - const repo = new IntegrationsRepository(); - const integration = await repo.fetchById(integrationId); - return integration?.projectId ?? null; + return projectId; } diff --git a/api/src/tv-modules/kanban/KanbanRoutes.ts b/api/src/tv-modules/kanban/KanbanRoutes.ts index da17150..97133d1 100644 --- a/api/src/tv-modules/kanban/KanbanRoutes.ts +++ b/api/src/tv-modules/kanban/KanbanRoutes.ts @@ -1,10 +1,11 @@ import { Router } from 'express'; import type { Routable } from '../../types/routable.type'; +import { GoalPermissions } from '../../types/auth.types'; import { IsLoggedIn } from '../auth/middlewares/is-logged-in'; import { KanbanController } from './KanbanController'; -import { CanManageKanban } from './middlewares/CanManageKanban'; -import { CanViewKanban } from './middlewares/CanViewKanban'; -import { CanFetchTasks } from './middlewares/CanFetchTasks'; +import { goalIdFromBody, goalIdFromParam, goalIdFromStatusBody } from './middlewares/goal-id-resolvers'; +import { requireKanbanPermission } from './middlewares/require-kanban-permission'; + export default class KanbanRoutes implements Routable { private readonly router: ReturnType; private readonly kanbanController: KanbanController; @@ -20,17 +21,89 @@ export default class KanbanRoutes implements Routable { } initRoutes() { - this.router.post('/fetch-statuses', [IsLoggedIn, CanViewKanban], this.kanbanController.fetchAllColumns); - this.router.post('/add-status', [IsLoggedIn, CanManageKanban], this.kanbanController.addStatus); - this.router.post('/delete-status', [IsLoggedIn, CanManageKanban], this.kanbanController.deleteStatus); - this.router.post('/update-status', [IsLoggedIn, CanManageKanban], this.kanbanController.updateStatus); + this.router.post( + '/fetch-statuses', + [ + IsLoggedIn, + requireKanbanPermission({ + anyOf: [GoalPermissions.KANBAN_CAN_VIEW], + resolveGoalId: goalIdFromBody, + }), + ], + this.kanbanController.fetchAllColumns + ); - // this.router.get('columns/:goalId', [IsLoggedIn], this.kanbanController.fetchAllColumns); - this.router.get('/tasks/:goalId/:columnId/:cursor', [IsLoggedIn, CanViewKanban, CanFetchTasks], this.kanbanController.fetchTasksForColumn); + this.router.post( + '/add-status', + [ + IsLoggedIn, requireKanbanPermission({ + anyOf: [GoalPermissions.KANBAN_CAN_MANAGE], + resolveGoalId: goalIdFromBody + }) + ], + this.kanbanController.addStatus + ); + + this.router.post( + '/delete-status', + [ + IsLoggedIn, requireKanbanPermission({ + anyOf: [GoalPermissions.KANBAN_CAN_MANAGE], + resolveGoalId: goalIdFromStatusBody + }) + ], + this.kanbanController.deleteStatus + ); + + this.router.post( + '/update-status', + [ + IsLoggedIn, requireKanbanPermission({ + anyOf: [GoalPermissions.KANBAN_CAN_MANAGE], + resolveGoalId: goalIdFromStatusBody + }) + ], + this.kanbanController.updateStatus + ); + + this.router.get( + '/tasks/:goalId/:columnId/:cursor', + [ + IsLoggedIn, + requireKanbanPermission({ + anyOf: [GoalPermissions.KANBAN_CAN_VIEW], + resolveGoalId: goalIdFromParam, + }), + requireKanbanPermission({ + anyOf: [GoalPermissions.COMPONENT_CAN_WATCH_CONTENT], + resolveGoalId: goalIdFromParam, + }), + ], + this.kanbanController.fetchTasksForColumn + ); //we do not use this route in the client (no logic for this route on the client side)!!! - this.router.get('/tasks-order/:goalId/:columnId/:cursor', [IsLoggedIn, CanManageKanban], this.kanbanController.getTasksOrderForColumnAndCursor); + this.router.get( + '/tasks-order/:goalId/:columnId/:cursor', + [ + IsLoggedIn, requireKanbanPermission({ + anyOf: [GoalPermissions.KANBAN_CAN_VIEW], + resolveGoalId: goalIdFromParam + }) + ], + this.kanbanController.getTasksOrderForColumnAndCursor + ); - this.router.patch('/update-tasks-order-and-column', [IsLoggedIn, CanManageKanban], this.kanbanController.updateTasksOrderAndColumn); + this.router.patch( + '/update-tasks-order-and-column', + [ + IsLoggedIn, + requireKanbanPermission({ + anyOf: [GoalPermissions.KANBAN_CAN_MANAGE], + resolveGoalId: goalIdFromBody + }) + ], + this.kanbanController.updateTasksOrderAndColumn + ); } } diff --git a/api/src/tv-modules/kanban/middlewares/CanFetchTasks.ts b/api/src/tv-modules/kanban/middlewares/CanFetchTasks.ts deleted file mode 100644 index f66d73a..0000000 --- a/api/src/tv-modules/kanban/middlewares/CanFetchTasks.ts +++ /dev/null @@ -1,33 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; -import { $logger } from '../../../modules/logget'; -import { GoalPermissions } from '../../../types/auth.types'; -import { logError } from '../../../utils/api'; -import { KanbanArkTypeCanManageKanban } from '../types'; -import { ArkErrors } from 'arktype'; - -export const CanFetchTasks = async (req: Request, res: Response, next: NextFunction) => { - const props = req.body.goalId ? req.body : req.params; - - const data = KanbanArkTypeCanManageKanban(props); - - if (data instanceof ArkErrors) { - return res.status(400).send(data.summary); - } - - const permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(data.goalId, GoalPermissionsFetcher.PERMISSION_TYPE_FOR_GOAL) - .catch(logError); - - - if (!permissions) { - $logger.error('Can not get permissions for CanAddTask middleware'); - return res.status(500).end(); - } - - if (permissions.hasPermissions(GoalPermissions.COMPONENT_CAN_WATCH_CONTENT)) { - return next(); - } - - return res.status(403).end(); -}; diff --git a/api/src/tv-modules/kanban/middlewares/CanManageKanban.ts b/api/src/tv-modules/kanban/middlewares/CanManageKanban.ts deleted file mode 100644 index 4b78c2b..0000000 --- a/api/src/tv-modules/kanban/middlewares/CanManageKanban.ts +++ /dev/null @@ -1,54 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; -import { $logger } from '../../../modules/logget'; -import { GoalPermissions } from '../../../types/auth.types'; -import { ALL_TASKS_LIST_ID, DEFAULT_ID } from '../../../types/tasks.types'; -import { logError } from '../../../utils/api'; -import { KanbanArkTypeCanManageKanban } from '../types'; -import { ArkErrors } from 'arktype'; - -export const CanManageKanban = async (req: Request, res: Response, next: NextFunction) => { - let props = req.body.goalId ? req.body : req.params; - - switch (req.url) { - case '/update-status': - const result = await req.appUser.kanbanManager.repository.fetchStatus(req.body.id); - props = { - goalId: result?.goal_id, - }; - break; - case '/delete-status': - const result2 = await req.appUser.kanbanManager.repository.fetchStatus(req.body.id); - props = { - goalId: result2?.goal_id, - }; - break; - default: - break; - } - - const data = KanbanArkTypeCanManageKanban(props); - - if (data instanceof ArkErrors) { - return res.status(400).send(data.summary); - } - - const permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(data.goalId, GoalPermissionsFetcher.PERMISSION_TYPE_FOR_GOAL) - .catch(logError); - - - if (!permissions) { - $logger.error('Can not get permissions for CanAddTask middleware'); - return res.status(500).end(); - } - - if ( - permissions.hasPermissions(GoalPermissions.COMPONENT_CAN_ADD_TASKS) || - permissions.hasPermissions(GoalPermissions.TASKS_CAN_ADD_SUBTASKS) - ) { - return next(); - } - - return res.status(403).end(); -}; diff --git a/api/src/tv-modules/kanban/middlewares/CanViewKanban.ts b/api/src/tv-modules/kanban/middlewares/CanViewKanban.ts deleted file mode 100644 index 4c93fef..0000000 --- a/api/src/tv-modules/kanban/middlewares/CanViewKanban.ts +++ /dev/null @@ -1,33 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; -import { $logger } from '../../../modules/logget'; -import { GoalPermissions } from '../../../types/auth.types'; -import { logError } from '../../../utils/api'; -import { KanbanArkTypeCanManageKanban } from '../types'; -import { ArkErrors } from 'arktype'; - -export const CanViewKanban = async (req: Request, res: Response, next: NextFunction) => { - const props = req.body.goalId ? req.body : req.params; - - const data = KanbanArkTypeCanManageKanban(props); - - if (data instanceof ArkErrors) { - return res.status(400).send(data.summary); - } - - const permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(data.goalId, GoalPermissionsFetcher.PERMISSION_TYPE_FOR_GOAL) - .catch(logError); - - - if (!permissions) { - $logger.error('Can not get permissions for CanAddTask middleware'); - return res.status(500).end(); - } - - if (permissions.hasPermissions(GoalPermissions.KANBAN_CAN_VIEW)) { - return next(); - } - - return res.status(403).end(); -}; diff --git a/api/src/tv-modules/kanban/middlewares/goal-id-resolvers.ts b/api/src/tv-modules/kanban/middlewares/goal-id-resolvers.ts new file mode 100644 index 0000000..f527936 --- /dev/null +++ b/api/src/tv-modules/kanban/middlewares/goal-id-resolvers.ts @@ -0,0 +1,19 @@ +import type { Request } from 'express'; + +export function goalIdFromParam(req: Request): number | null { + const goalId = Number(req.params.goalId); + return goalId && !isNaN(goalId) ? goalId : null; +} + +export function goalIdFromBody(req: Request): number | null { + const goalId = Number(req.body?.goalId); + return goalId && !isNaN(goalId) ? goalId : null; +} + +export async function goalIdFromStatusBody(req: Request): Promise { + const statusId = Number(req.body?.id); + if (!statusId || isNaN(statusId)) return null; + + const status = await req.appUser.kanbanManager.repository.fetchStatus(statusId); + return status?.goal_id ?? null; +} diff --git a/api/src/tv-modules/kanban/middlewares/require-kanban-permission.ts b/api/src/tv-modules/kanban/middlewares/require-kanban-permission.ts new file mode 100644 index 0000000..0476538 --- /dev/null +++ b/api/src/tv-modules/kanban/middlewares/require-kanban-permission.ts @@ -0,0 +1,24 @@ +import type { NextFunction, Request, Response } from 'express'; +import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; +import { $logger } from '../../../modules/logget'; +import { logError } from '../../../utils/api'; +import type { RequireKanbanPermissionArgs } from '../types'; + +export function requireKanbanPermission({ anyOf, resolveGoalId }: RequireKanbanPermissionArgs) { + return async (req: Request, res: Response, next: NextFunction) => { + const goalId = await resolveGoalId(req); + if (!goalId) return res.status(400).end(); + + const permissions = await req.appUser.permissionsFetcher + .getPermissionsForType(goalId, GoalPermissionsFetcher.PERMISSION_TYPE_FOR_GOAL) + .catch(logError); + + if (!permissions) { + $logger.error('Can not resolve kanban permissions'); + return res.status(500).end(); + } + + if (anyOf.some((permission) => permissions.hasPermissions(permission))) return next(); + return res.status(403).end(); + }; +} diff --git a/api/src/tv-modules/kanban/types.ts b/api/src/tv-modules/kanban/types.ts index e169f6a..cc9c324 100644 --- a/api/src/tv-modules/kanban/types.ts +++ b/api/src/tv-modules/kanban/types.ts @@ -1,6 +1,8 @@ import { type } from 'arktype'; +import type { Request } from 'express'; import { z } from 'zod'; import { StringToNumber } from '../../types/app.types'; +import type { GoalPermissionType } from '../../types/auth.types'; // ============ Arktype schemas ============ @@ -102,11 +104,13 @@ export const KanbanArkTypeUpdateTasksOrder = type({ export type KanbanArgUpdateTasksOrder = typeof KanbanArkTypeUpdateTasksOrder.infer; -export const KanbanArkTypeCanManageKanban = type({ - goalId: NumberFromString, -}); +export type KanbanGoalIdResolver = (req: Request) => Promise | number | null; -export type KanbanArgCanManageKanban = typeof KanbanArkTypeCanManageKanban.infer; +export type RequireKanbanPermissionArgs = { + /** the caller must hold at least ONE of these */ + anyOf: GoalPermissionType[]; + resolveGoalId: KanbanGoalIdResolver; +}; // ============ Deprecated Zod schemas ============ diff --git a/api/src/tv-modules/sso/SsoController.ts b/api/src/tv-modules/sso/SsoController.ts index c03cb38..473b799 100644 --- a/api/src/tv-modules/sso/SsoController.ts +++ b/api/src/tv-modules/sso/SsoController.ts @@ -184,6 +184,10 @@ export class SsoController { const userData = resolved.user + if (userData.block && !userData.confirm_email_code) { + return this.redirectSsoError(res, 'account_blocked') + } + await this.orgRepo.addMember(config.organizationId, userData.email, config.defaultOrgRole) await this.ssoRepo.upsertIdentity({ diff --git a/api/src/tv-modules/sso/__tests__/sso.utils.test.ts b/api/src/tv-modules/sso/__tests__/sso.utils.test.ts new file mode 100644 index 0000000..f4702fe --- /dev/null +++ b/api/src/tv-modules/sso/__tests__/sso.utils.test.ts @@ -0,0 +1,49 @@ +import { describe, it, expect } from 'vitest' +import { deriveSamlEmail } from '../sso.utils' + +const EMAIL_NAMEID_FORMAT = 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress' +const PERSISTENT_NAMEID_FORMAT = 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent' +const EMAIL_CLAIM = 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress' + +describe('deriveSamlEmail', () => { + it('takes the email attribute and lowercases it', () => { + expect(deriveSamlEmail({ email: 'User@Company.com', nameID: 'abc' })).toBe('user@company.com') + }) + + it('falls back to the xmlsoap emailaddress claim', () => { + expect(deriveSamlEmail({ [EMAIL_CLAIM]: 'a@b.com', nameID: 'abc' })).toBe('a@b.com') + }) + + it('uses nameID only when the NameID Format is emailAddress', () => { + expect(deriveSamlEmail({ + nameID: 'user@company.com', + nameIDFormat: EMAIL_NAMEID_FORMAT, + })).toBe('user@company.com') + }) + + it('does not use nameID for a non-email NameID Format', () => { + expect(deriveSamlEmail({ + nameID: 'user@company.com', + nameIDFormat: PERSISTENT_NAMEID_FORMAT, + })).toBeNull() + }) + + it('does not use nameID when no format is provided', () => { + expect(deriveSamlEmail({ nameID: 'user@company.com' })).toBeNull() + }) + + it('prefers the email attribute over an emailAddress-format nameID', () => { + expect(deriveSamlEmail({ + email: 'attr@company.com', + nameID: 'name@company.com', + nameIDFormat: EMAIL_NAMEID_FORMAT, + })).toBe('attr@company.com') + }) + + it('returns null for a blank or non-string email attribute', () => { + expect(deriveSamlEmail({ email: ' ', nameID: 'abc' })).toBeNull() + expect(deriveSamlEmail({ email: 123, nameID: 'abc' })).toBeNull() + expect(deriveSamlEmail({ nameID: 'abc' })).toBeNull() + expect(deriveSamlEmail({})).toBeNull() + }) +}) diff --git a/api/src/tv-modules/sso/providers/saml.provider.ts b/api/src/tv-modules/sso/providers/saml.provider.ts index f0f093e..80d2107 100644 --- a/api/src/tv-modules/sso/providers/saml.provider.ts +++ b/api/src/tv-modules/sso/providers/saml.provider.ts @@ -2,6 +2,7 @@ import { SAML, ValidateInResponseTo } from '@node-saml/node-saml' import type { Request, Response } from 'express' import type { SsoConfigsSchemaTypeForSelect } from 'taskview-db-schemas' import { PublicApiUrl } from '../../../modules/public-url' +import { deriveSamlEmail } from '../sso.utils' import type { SamlOptionsArgs } from '../types' import type { SsoProvider, SsoAuthResult } from './sso-provider.interface' import { SamlDbCacheProvider } from './saml-cache-provider' @@ -72,14 +73,13 @@ export class SamlProvider implements SsoProvider { throw new Error('SAML response missing nameID') } - const email = ( - profile.email - ?? profile['http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress'] - ?? profile.nameID - ) as string + const email = deriveSamlEmail(profile as Record) + if (!email) { + throw new Error('SAML response missing email attribute') + } return { - email: email.toLowerCase(), + email, externalId: profile.nameID, displayName: (profile.displayName ?? profile['http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name']) as string | undefined, diff --git a/api/src/tv-modules/sso/sso.utils.ts b/api/src/tv-modules/sso/sso.utils.ts index 92b1457..203a7a8 100644 --- a/api/src/tv-modules/sso/sso.utils.ts +++ b/api/src/tv-modules/sso/sso.utils.ts @@ -14,6 +14,21 @@ export function generateDomainVerifyToken(): string { return `tvdom_${randomBytes(32).toString('hex')}` } +const SAML_EMAIL_NAMEID_FORMAT = 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress' +const SAML_EMAIL_CLAIM = 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress' + +export function deriveSamlEmail(profile: Record): string | null { + const fromAttribute = profile.email ?? profile[SAML_EMAIL_CLAIM] + if (typeof fromAttribute === 'string' && fromAttribute.trim()) { + return fromAttribute.trim().toLowerCase() + } + if (profile.nameIDFormat === SAML_EMAIL_NAMEID_FORMAT + && typeof profile.nameID === 'string' && profile.nameID.trim()) { + return profile.nameID.trim().toLowerCase() + } + return null +} + export function trustedSsoDomains(): string[] { const raw = process.env.SSO_TRUSTED_DOMAINS if (!raw?.trim()) return [] @@ -80,9 +95,9 @@ export async function checkSsoDomainHttpFile(args: CheckSsoDomainProofArgs): Pro const urls = process.env.NODE_ENV === 'production' ? [`https://${args.domain}${SSO_DOMAIN_WELL_KNOWN_PATH}`] : [ - `https://${args.domain}${SSO_DOMAIN_WELL_KNOWN_PATH}`, - `http://${args.domain}${SSO_DOMAIN_WELL_KNOWN_PATH}`, - ] + `https://${args.domain}${SSO_DOMAIN_WELL_KNOWN_PATH}`, + `http://${args.domain}${SSO_DOMAIN_WELL_KNOWN_PATH}`, + ] for (const url of urls) { const urlError = validateMetadataUrl(url) diff --git a/api/src/tv-modules/sso/types.ts b/api/src/tv-modules/sso/types.ts index 23eda11..2e95f78 100644 --- a/api/src/tv-modules/sso/types.ts +++ b/api/src/tv-modules/sso/types.ts @@ -128,7 +128,7 @@ export type ApplySsoIdpEmailArgs = { email: string } -export type SsoCallbackError = 'authentication_failed' | 'email_in_use' +export type SsoCallbackError = 'authentication_failed' | 'email_in_use' | 'account_blocked' export type ResolveSsoUserResult = | { ok: true, user: UserDbRecord } diff --git a/api/src/tv-modules/tasks/TasksRoutes.ts b/api/src/tv-modules/tasks/TasksRoutes.ts index c4edd8f..0f3cc3e 100644 --- a/api/src/tv-modules/tasks/TasksRoutes.ts +++ b/api/src/tv-modules/tasks/TasksRoutes.ts @@ -2,25 +2,14 @@ import { Router } from 'express'; import type { Routable } from '../../types/routable.type'; import { IsLoggedIn } from '../auth/middlewares/is-logged-in'; import { CanAddTaskNew } from './middlewares/CanAddTaskNew'; -// import { CanAddTask } from './middlewares/CanAddTask'; -// import { CanUpdateTaskStatus } from './middlewares/CanUpdateTaskStatus'; import { CanDeleteTask } from './middlewares/CanDeleteTask'; -// import { CanUpdateTaskAssignee } from './middlewares/CanUpdateTaskAssignee'; import { CanFetchTask } from './middlewares/CanFetchTask'; -// import { CanUpdateTaskDescription } from './middlewares/CanUpdateTaskDescription'; -// import { CanUpdateTaskNote } from './middlewares/CanUpdateTaskNote'; -// import { CanUpdateTaskDeadline } from './middlewares/CanUpdateTaskDeadline'; -// import { CanFetchSubtasks } from './middlewares/CanFetchSubtasks'; -// import { CanUpdateTaskPriority } from './middlewares/CanUpdateTaskPriority'; -// import { CanMoveTask } from './middlewares/CanMoveTask'; -// import { CanSeeTaskAssignedUsers } from './middlewares/CanSeeTaskAssignedUsers'; import { CanFetchTaskHistory } from './middlewares/CanFetchTaskHistory'; import { CanFetchTasks } from './middlewares/CanFetchTasks'; import { CanRecoveryTaskHistory } from './middlewares/CanRecoveryTaskHistory'; import { CanUpdateTask } from './middlewares/CanUpdateTask'; import { CanUpdateTaskAssigneeNew } from './middlewares/CanUpdateTaskAssigneeNew'; import { TasksController } from './TasksController'; -// import { MainCanCreateTaskAction } from './middlewares/MainCanCreateTaskAction'; export default class TasksRoutes implements Routable { private readonly router: ReturnType; diff --git a/api/src/tv-modules/tasks/middlewares/CanAddTask.ts b/api/src/tv-modules/tasks/middlewares/CanAddTask.ts deleted file mode 100644 index 711d935..0000000 --- a/api/src/tv-modules/tasks/middlewares/CanAddTask.ts +++ /dev/null @@ -1,39 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; -import { $logger } from '../../../modules/logget'; -import { GoalPermissions } from '../../../types/auth.types'; -import { ALL_TASKS_LIST_ID, DEFAULT_ID } from '../../../types/tasks.types'; -import { logError } from '../../../utils/api'; - -export const CanAddTask = async (req: Request, res: Response, next: NextFunction) => { - const listId = req.body.componentId; - - if (!listId) { - return res.status(400).end(); - } - - let permissions; - if (Number(listId) === ALL_TASKS_LIST_ID && req.body.goalId && req.body.goalId !== DEFAULT_ID) { - permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(Number(req.body.goalId), GoalPermissionsFetcher.PERMISSION_TYPE_FOR_GOAL) - .catch(logError); - } else { - permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(Number(listId), GoalPermissionsFetcher.PERMISSION_TYPE_FOR_TASKLIST) - .catch(logError); - } - - if (!permissions) { - $logger.error('Can not get permissions for CanAddTask middleware'); - return res.status(500).end(); - } - - if ( - permissions.hasPermissions(GoalPermissions.COMPONENT_CAN_ADD_TASKS) || - permissions.hasPermissions(GoalPermissions.TASKS_CAN_ADD_SUBTASKS) - ) { - return next(); - } - - return res.status(403).end(); -}; diff --git a/api/src/tv-modules/tasks/middlewares/CanFetchSubtasks.ts b/api/src/tv-modules/tasks/middlewares/CanFetchSubtasks.ts deleted file mode 100644 index 5979819..0000000 --- a/api/src/tv-modules/tasks/middlewares/CanFetchSubtasks.ts +++ /dev/null @@ -1,28 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; -import { $logger } from '../../../modules/logget'; -import { GoalPermissions } from '../../../types/auth.types'; -import { logError } from '../../../utils/api'; - -export const CanFetchSubtasks = async (req: Request, res: Response, next: NextFunction) => { - const taskId = req.query.taskId; - - if (!taskId) { - return res.status(400).end(); - } - - const permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(Number(taskId), GoalPermissionsFetcher.PERMISSION_TYPE_FOR_TASK) - .catch(logError); - - if (!permissions) { - $logger.error('Can not get permissions for CanFetchSubtasks'); - return res.status(500).end(); - } - - if (permissions.hasPermissions(GoalPermissions.TASKS_CAN_WATCH_SUBTASKS)) { - return next(); - } - - return res.status(403).end(); -}; diff --git a/api/src/tv-modules/tasks/middlewares/CanFetchTask.ts b/api/src/tv-modules/tasks/middlewares/CanFetchTask.ts index 4e8a9ea..571f04e 100644 --- a/api/src/tv-modules/tasks/middlewares/CanFetchTask.ts +++ b/api/src/tv-modules/tasks/middlewares/CanFetchTask.ts @@ -5,7 +5,7 @@ import { GoalPermissions } from '../../../types/auth.types'; import { logError } from '../../../utils/api'; export const CanFetchTask = async (req: Request, res: Response, next: NextFunction) => { - const taskId = req.query.taskId || req.params.taskId; + const taskId = req.params.taskId; if (!taskId) { return res.status(400).end(); diff --git a/api/src/tv-modules/tasks/middlewares/CanMoveTask.ts b/api/src/tv-modules/tasks/middlewares/CanMoveTask.ts deleted file mode 100644 index cd9d685..0000000 --- a/api/src/tv-modules/tasks/middlewares/CanMoveTask.ts +++ /dev/null @@ -1,28 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; -import { $logger } from '../../../modules/logget'; -import { GoalPermissions } from '../../../types/auth.types'; -import { logError } from '../../../utils/api'; - -export const CanMoveTask = async (req: Request, res: Response, next: NextFunction) => { - const taskId = req.body.taskId; - - if (!taskId) { - return res.status(400).end(); - } - - const permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(Number(taskId), GoalPermissionsFetcher.PERMISSION_TYPE_FOR_TASK) - .catch(logError); - - if (!permissions) { - $logger.error('Can not get permissions for CanMoveTask'); - return res.status(500).end(); - } - - if (permissions.hasPermissions(GoalPermissions.TASKS_CAN_DELETE)) { - return next(); - } - - return res.status(403).end(); -}; diff --git a/api/src/tv-modules/tasks/middlewares/CanSeeTaskAssignedUsers.ts b/api/src/tv-modules/tasks/middlewares/CanSeeTaskAssignedUsers.ts deleted file mode 100644 index f8f5a33..0000000 --- a/api/src/tv-modules/tasks/middlewares/CanSeeTaskAssignedUsers.ts +++ /dev/null @@ -1,28 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; -import { $logger } from '../../../modules/logget'; -import { GoalPermissions } from '../../../types/auth.types'; -import { logError } from '../../../utils/api'; - -export const CanSeeTaskAssignedUsers = async (req: Request, res: Response, next: NextFunction) => { - const taskId = req.body.taskId; - - if (!taskId) { - return res.status(400).end(); - } - - const permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(Number(taskId), GoalPermissionsFetcher.PERMISSION_TYPE_FOR_TASK) - .catch(logError); - - if (!permissions) { - $logger.error('Can not get permissions for CanSeeTaskAssignedUsers'); - return res.status(500).end(); - } - - if (permissions.hasPermissions(GoalPermissions.TASKS_CAN_WATCH_ASSIGNED_USERS)) { - return next(); - } - - return res.status(403).end(); -}; diff --git a/api/src/tv-modules/tasks/middlewares/CanUpdateTaskAssignee.ts b/api/src/tv-modules/tasks/middlewares/CanUpdateTaskAssignee.ts deleted file mode 100644 index 91673a9..0000000 --- a/api/src/tv-modules/tasks/middlewares/CanUpdateTaskAssignee.ts +++ /dev/null @@ -1,29 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; -import { $logger } from '../../../modules/logget'; -import { GoalPermissions } from '../../../types/auth.types'; -import { logError } from '../../../utils/api'; - -/** @deprecated */ -export const CanUpdateTaskAssignee = async (req: Request, res: Response, next: NextFunction) => { - const taskId = req.body.taskId; - - if (!taskId) { - return res.status(400).end(); - } - - const permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(Number(taskId), GoalPermissionsFetcher.PERMISSION_TYPE_FOR_TASK) - .catch(logError); - - if (!permissions) { - $logger.error('Can not get permissions for CanUpdateTaskDescription'); - return res.status(500).end(); - } - - if (permissions.hasPermissions(GoalPermissions.TASKS_CAN_ASSIGN_USERS)) { - return next(); - } - - return res.status(403).end(); -}; diff --git a/api/src/tv-modules/tasks/middlewares/CanUpdateTaskDeadline.ts b/api/src/tv-modules/tasks/middlewares/CanUpdateTaskDeadline.ts deleted file mode 100644 index d1454f5..0000000 --- a/api/src/tv-modules/tasks/middlewares/CanUpdateTaskDeadline.ts +++ /dev/null @@ -1,28 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; -import { $logger } from '../../../modules/logget'; -import { GoalPermissions } from '../../../types/auth.types'; -import { logError } from '../../../utils/api'; - -export const CanUpdateTaskDeadline = async (req: Request, res: Response, next: NextFunction) => { - const taskId = req.body.taskId; - - if (!taskId) { - return res.status(400).end(); - } - - const permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(Number(taskId), GoalPermissionsFetcher.PERMISSION_TYPE_FOR_TASK) - .catch(logError); - - if (!permissions) { - $logger.error('Can not get permissions for CanUpdateTaskDeadline'); - return res.status(500).end(); - } - - if (permissions.hasPermissions(GoalPermissions.TASKS_CAN_EDIT_DEADLINE)) { - return next(); - } - - return res.status(403).end(); -}; diff --git a/api/src/tv-modules/tasks/middlewares/CanUpdateTaskDescription.ts b/api/src/tv-modules/tasks/middlewares/CanUpdateTaskDescription.ts deleted file mode 100644 index ae96174..0000000 --- a/api/src/tv-modules/tasks/middlewares/CanUpdateTaskDescription.ts +++ /dev/null @@ -1,28 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; -import { $logger } from '../../../modules/logget'; -import { GoalPermissions } from '../../../types/auth.types'; -import { logError } from '../../../utils/api'; - -export const CanUpdateTaskDescription = async (req: Request, res: Response, next: NextFunction) => { - const taskId = req.body.taskId; - - if (!taskId) { - return res.status(400).end(); - } - - const permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(Number(taskId), GoalPermissionsFetcher.PERMISSION_TYPE_FOR_TASK) - .catch(logError); - - if (!permissions) { - $logger.error('Can not get permissions for CanUpdateTaskDescription'); - return res.status(500).end(); - } - - if (permissions.hasPermissions(GoalPermissions.TASKS_CAN_EDIT_DESCRIPTION)) { - return next(); - } - - return res.status(403).end(); -}; diff --git a/api/src/tv-modules/tasks/middlewares/CanUpdateTaskNote.ts b/api/src/tv-modules/tasks/middlewares/CanUpdateTaskNote.ts deleted file mode 100644 index fb63e50..0000000 --- a/api/src/tv-modules/tasks/middlewares/CanUpdateTaskNote.ts +++ /dev/null @@ -1,28 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; -import { $logger } from '../../../modules/logget'; -import { GoalPermissions } from '../../../types/auth.types'; -import { logError } from '../../../utils/api'; - -export const CanUpdateTaskNote = async (req: Request, res: Response, next: NextFunction) => { - const taskId = req.body.taskId; - - if (!taskId) { - return res.status(400).end(); - } - - const permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(Number(taskId), GoalPermissionsFetcher.PERMISSION_TYPE_FOR_TASK) - .catch(logError); - - if (!permissions) { - $logger.error('Can not get permissions for CanUpdateTaskNote'); - return res.status(500).end(); - } - - if (permissions.hasPermissions(GoalPermissions.TASKS_CAN_EDIT_NOTE)) { - return next(); - } - - return res.status(403).end(); -}; diff --git a/api/src/tv-modules/tasks/middlewares/CanUpdateTaskPriority.ts b/api/src/tv-modules/tasks/middlewares/CanUpdateTaskPriority.ts deleted file mode 100644 index 15941d5..0000000 --- a/api/src/tv-modules/tasks/middlewares/CanUpdateTaskPriority.ts +++ /dev/null @@ -1,28 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; -import { $logger } from '../../../modules/logget'; -import { GoalPermissions } from '../../../types/auth.types'; -import { logError } from '../../../utils/api'; - -export const CanUpdateTaskPriority = async (req: Request, res: Response, next: NextFunction) => { - const taskId = req.body.taskId; - - if (!taskId) { - return res.status(400).end(); - } - - const permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(Number(taskId), GoalPermissionsFetcher.PERMISSION_TYPE_FOR_TASK) - .catch(logError); - - if (!permissions) { - $logger.error('Can not get permissions for CanUpdateTaskPriority'); - return res.status(500).end(); - } - - if (permissions.hasPermissions(GoalPermissions.TASKS_CAN_EDIT_PRIORITY)) { - return next(); - } - - return res.status(403).end(); -}; diff --git a/api/src/tv-modules/tasks/middlewares/CanUpdateTaskStatus.ts b/api/src/tv-modules/tasks/middlewares/CanUpdateTaskStatus.ts deleted file mode 100644 index 9d74d6a..0000000 --- a/api/src/tv-modules/tasks/middlewares/CanUpdateTaskStatus.ts +++ /dev/null @@ -1,28 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { GoalPermissionsFetcher } from '../../../core/GoalPermissionsFetcher'; -import { $logger } from '../../../modules/logget'; -import { GoalPermissions } from '../../../types/auth.types'; -import { logError } from '../../../utils/api'; - -export const CanUpdateTaskStatus = async (req: Request, res: Response, next: NextFunction) => { - const taskId = req.body.taskId; - - if (!taskId) { - return res.status(400).end(); - } - - const permissions = await req.appUser.permissionsFetcher - .getPermissionsForType(Number(taskId), GoalPermissionsFetcher.PERMISSION_TYPE_FOR_TASK) - .catch(logError); - - if (!permissions) { - $logger.error('Can not get permissions for CanAddTask middleware'); - return res.status(500).end(); - } - - if (permissions.hasPermissions(GoalPermissions.TASKS_CAN_EDIT_STATUS)) { - return next(); - } - - return res.status(403).end(); -}; diff --git a/package.json b/package.json index ca06928..7a317a2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "taskview-ce-monorepo", - "version": "1.52.0", + "version": "1.53.0", "private": true, "description": "TaskView CE monorepo containing web, API, and packages", "workspaces": [ diff --git a/taskview-packages/taskview-api/src/api/__tests__/collaboration-goal-access.test.ts b/taskview-packages/taskview-api/src/api/__tests__/collaboration-goal-access.test.ts new file mode 100644 index 0000000..b6d6201 --- /dev/null +++ b/taskview-packages/taskview-api/src/api/__tests__/collaboration-goal-access.test.ts @@ -0,0 +1,300 @@ +import { TvApi } from '@/tv' +import { TvPermissions } from '@/api/permissions' +import axios from 'axios' +import { describe, it, expect, beforeAll, afterAll } from 'vitest' +import { API_URL, initApi } from './init-api' + +/** + * GET /module/collaboration/:goalId must be an object-level protected route: + * only a member of the goal holding task_can_assign_users or goal_can_manage_users + * may read its collaborator list (emails, invitation dates, roles, goalOwner flag). + */ +describe('Collaboration goal member list access control', () => { + let ownerApi: TvApi + let outsiderApi: TvApi + let outsiderEmail: string + let deleteAllGoals: () => Promise + let manageUsersPermissionId: number + const permissionIdByName = new Map() + + beforeAll(async () => { + const init = await initApi() + ownerApi = init.$tvApi + outsiderApi = init.$tvApiForSecondUser + outsiderEmail = init.user2Email + deleteAllGoals = init.deleteAllGoals + + const allPermissions = await ownerApi.collaboration.fetchAllPermissions() + for (const permission of allPermissions) { + permissionIdByName.set(permission.name, permission.id) + } + const found = permissionIdByName.get(TvPermissions.GOAL_CAN_MANAGE_USERS) + if (!found) throw new Error('Permission "goal_can_manage_users" is not in DB') + manageUsersPermissionId = found + }) + + afterAll(async () => { + await deleteAllGoals() + }) + + async function expectHttpStatus(promise: Promise, status: number): Promise { + try { + await promise + throw new Error(`Expected HTTP ${status} but request succeeded`) + } catch (e: any) { + if (typeof e.message === 'string' && e.message.startsWith('Expected HTTP')) throw e + expect(e.response?.status ?? e.status, `Expected ${status}, got ${e.response?.status ?? e.status}`).toBe(status) + } + } + + // A goal owned by user1 that user2 is NOT a member of, holding a third-party email + async function createPrivateGoal(organizationId?: number) { + const goal = await ownerApi.goals.createGoal({ + name: `Private goal ${Date.now()}`, + ...(organizationId ? { organizationId } : {}), + }) + if (!goal) throw new Error('Failed to create goal') + + const invitedEmail = `outside-party-${Date.now()}@test.com` + const invited = await ownerApi.collaboration.inviteUserToGoal({ email: invitedEmail, goalId: goal.id }) + if (!invited) throw new Error('Failed to invite third-party email') + + return { goal, invitedEmail } + } + + // Invite user2 into user1's goal and grant a role carrying goal_can_manage_users + async function shareGoalWithOutsider() { + const goal = await ownerApi.goals.createGoal({ name: `Shared goal ${Date.now()}` }) + if (!goal) throw new Error('Failed to create goal') + + const collab = await ownerApi.collaboration.inviteUserToGoal({ email: outsiderEmail, goalId: goal.id }) + if (!collab) throw new Error('Failed to invite user2') + + const role = await ownerApi.collaboration.createRoleForGoal({ + goalId: goal.id, + roleName: `Manager ${Date.now()}`, + }) + if (!role) throw new Error('Failed to create role') + + const toggled = await ownerApi.collaboration.toggleRolePermission({ + roleId: role.id, + permissionId: manageUsersPermissionId, + }) + if (!toggled || toggled.add !== true) { + throw new Error(`Expected goal_can_manage_users to be added, got ${JSON.stringify(toggled)}`) + } + + await ownerApi.collaboration.toggleUserRoles({ + goalId: goal.id, + userId: collab.id, + roles: [role.id], + }) + + return { goal, role, collab } + } + + describe('JWT session of a non-member', () => { + it('cannot read the collaborator list of someone else goal', async () => { + const { goal } = await createPrivateGoal() + + await expectHttpStatus(outsiderApi.collaboration.fetchUsersForGoal(goal.id), 403) + }) + + it('cannot enumerate collaborator emails by walking goal ids', async () => { + const { goal, invitedEmail } = await createPrivateGoal() + + let leaked: Awaited> | null = null + try { + leaked = await outsiderApi.collaboration.fetchUsersForGoal(goal.id) + } catch { + return + } + + expect( + leaked ?? [], + `Leaked collaborator list of goal ${goal.id}: ${JSON.stringify(leaked)}`, + ).toEqual([]) + expect((leaked ?? []).some(u => u.email === invitedEmail)).toBe(false) + expect((leaked ?? []).some(u => u.goalOwner)).toBe(false) + }) + + it('gets the same rejection for a goal id that does not exist', async () => { + const nonExistentGoalId = 999999999 + + await expectHttpStatus(outsiderApi.collaboration.fetchUsersForGoal(nonExistentGoalId), 403) + }) + }) + + describe('Organization boundary', () => { + it('a member of the same organization who is not a member of the goal is still rejected', async () => { + const org = await ownerApi.organizations.create({ name: `Access org ${Date.now()}` }) + if (!org) throw new Error('Failed to create organization') + + const added = await ownerApi.organizations.addMember({ + organizationId: org.id, + email: outsiderEmail, + role: 'member', + }) + if (!added) throw new Error('Failed to add user2 to the organization') + + // The goal lives in the shared org, but user2 was never invited into the goal itself + const { goal } = await createPrivateGoal(org.id) + + await expectHttpStatus(outsiderApi.collaboration.fetchUsersForGoal(goal.id), 403) + }) + }) + + describe('Unauthenticated access', () => { + it('is rejected with 401 rather than served', async () => { + const { goal } = await createPrivateGoal() + + const response = await axios.get(`${API_URL}/module/collaboration/${goal.id}`, { + validateStatus: () => true, + }) + + expect( + response.status, + `Anonymous request returned ${response.status}: ${JSON.stringify(response.data)}`, + ).toBe(401) + }) + }) + + describe('API token of a non-member', () => { + it('cannot read the collaborator list of someone else goal', async () => { + const created = await outsiderApi.apiTokens.create({ name: `Access probe ${Date.now()}` }) + if (!created) throw new Error('Failed to create API token for user2') + + const tokenApi = new TvApi(axios.create({ + baseURL: API_URL, + headers: { Authorization: `Bearer ${created.token}` }, + })) + + const { goal } = await createPrivateGoal() + + try { + await expectHttpStatus(tokenApi.collaboration.fetchUsersForGoal(goal.id), 403) + } finally { + await outsiderApi.apiTokens.delete(created.item.id) + } + }) + + it('cannot read a goal that is outside the token allowedGoalIds scope', async () => { + const ownGoal = await outsiderApi.goals.createGoal({ name: `User2 goal ${Date.now()}` }) + if (!ownGoal) throw new Error('Failed to create user2 goal') + + // Token is explicitly scoped to user2's own goal only + const created = await outsiderApi.apiTokens.create({ + name: `Scoped probe ${Date.now()}`, + allowedGoalIds: [ownGoal.id], + }) + if (!created) throw new Error('Failed to create scoped API token for user2') + + const tokenApi = new TvApi(axios.create({ + baseURL: API_URL, + headers: { Authorization: `Bearer ${created.token}` }, + })) + + const { goal } = await createPrivateGoal() + + try { + await expectHttpStatus(tokenApi.collaboration.fetchUsersForGoal(goal.id), 403) + } finally { + await outsiderApi.apiTokens.delete(created.item.id) + } + }) + }) + + describe('Legitimate access is preserved', () => { + it('the goal owner can read the collaborator list', async () => { + const { goal, invitedEmail } = await createPrivateGoal() + + const users = await ownerApi.collaboration.fetchUsersForGoal(goal.id) + expect(users).toBeDefined() + expect(users?.some(u => u.email === invitedEmail)).toBe(true) + }) + + it('a member with goal_can_manage_users can read the collaborator list', async () => { + const { goal } = await shareGoalWithOutsider() + + const users = await outsiderApi.collaboration.fetchUsersForGoal(goal.id) + expect(users).toBeDefined() + expect(users?.some(u => u.email === outsiderEmail)).toBe(true) + }) + + /** + * A rank-and-file member must still see the project roster, otherwise the UI + * cannot render task assignees. Both default roles created by the goal trigger + * (editor and executor, migration 1.6.1/5.default-roles-for-project.sql) carry + * task_can_watch_assigned_users, so this is the common case, not an edge one. + */ + it('a member with only task_can_watch_assigned_users can read the collaborator list', async () => { + const goal = await ownerApi.goals.createGoal({ name: `Executor goal ${Date.now()}` }) + if (!goal) throw new Error('Failed to create goal') + + const collab = await ownerApi.collaboration.inviteUserToGoal({ email: outsiderEmail, goalId: goal.id }) + if (!collab) throw new Error('Failed to invite user2') + + const roles = await ownerApi.collaboration.fetchRolesForGoal(goal.id) + const executor = roles?.find(r => r.name === 'executor') + if (!executor) throw new Error('Default "executor" role is missing on a fresh goal') + + // the role grants the watch permission and neither of the two management ones, + // so a pass here can only come from task_can_watch_assigned_users + const matrix = await ownerApi.collaboration.fetchRoleToPermissionsForGoal(goal.id) + const executorPermissionIds = (matrix ?? []) + .filter(row => row.roleId === executor.id) + .map(row => row.permissionId) + expect(executorPermissionIds).toContain(permissionIdByName.get(TvPermissions.TASK_CAN_WATCH_ASSIGNED_USERS)) + expect(executorPermissionIds).not.toContain(permissionIdByName.get(TvPermissions.GOAL_CAN_MANAGE_USERS)) + expect(executorPermissionIds).not.toContain(permissionIdByName.get(TvPermissions.TASK_CAN_ASSIGN_USERS)) + + await ownerApi.collaboration.toggleUserRoles({ + goalId: goal.id, + userId: collab.id, + roles: [executor.id], + }) + + const users = await outsiderApi.collaboration.fetchUsersForGoal(goal.id) + expect(users).toBeDefined() + expect(users?.some(u => u.email === outsiderEmail)).toBe(true) + }) + + it('a member whose role carries none of the three permissions is rejected', async () => { + const goal = await ownerApi.goals.createGoal({ name: `Bare role goal ${Date.now()}` }) + if (!goal) throw new Error('Failed to create goal') + + const collab = await ownerApi.collaboration.inviteUserToGoal({ email: outsiderEmail, goalId: goal.id }) + if (!collab) throw new Error('Failed to invite user2') + + // a freshly created custom role carries no permissions at all + const bareRole = await ownerApi.collaboration.createRoleForGoal({ + goalId: goal.id, + roleName: `Bare ${Date.now()}`, + }) + if (!bareRole) throw new Error('Failed to create role') + + await ownerApi.collaboration.toggleUserRoles({ + goalId: goal.id, + userId: collab.id, + roles: [bareRole.id], + }) + + await expectHttpStatus(outsiderApi.collaboration.fetchUsersForGoal(goal.id), 403) + }) + }) + + describe('Revoked access', () => { + it('a removed collaborator loses access to the collaborator list', async () => { + const { goal, collab } = await shareGoalWithOutsider() + + // sanity: access is real before removal + const before = await outsiderApi.collaboration.fetchUsersForGoal(goal.id) + expect(before?.some(u => u.email === outsiderEmail)).toBe(true) + + const removed = await ownerApi.collaboration.deleteUserFromGoal({ goalId: goal.id, id: collab.id }) + expect(removed).toBeTruthy() + + await expectHttpStatus(outsiderApi.collaboration.fetchUsersForGoal(goal.id), 403) + }) + }) +}) diff --git a/taskview-packages/taskview-api/src/api/__tests__/graph-access.test.ts b/taskview-packages/taskview-api/src/api/__tests__/graph-access.test.ts new file mode 100644 index 0000000..a4f48c7 --- /dev/null +++ b/taskview-packages/taskview-api/src/api/__tests__/graph-access.test.ts @@ -0,0 +1,161 @@ +import { TvApi } from '@/tv' +import axios, { type AxiosInstance } from 'axios' +import { describe, it, expect, beforeAll, afterAll } from 'vitest' +import { API_URL, DEFAULT_PASSWORD, DEFAULT_USER_2, initApi } from './init-api' + +/** + * resolveGoalId() for the graph module inspects req.body.source before falling + * back to req.params.id, while deleteEdge acts on req.params.id. A caller must + * not be able to point the guard at a task they own while the handler operates + * on an edge belonging to someone else. + */ +describe('Graph object-level access control', () => { + let ownerApi: TvApi + let outsiderApi: TvApi + let deleteAllGoals: () => Promise + let attackerAxios: AxiosInstance + let victimGoalId: number + let attackerTaskId: number + let victimTaskId: number + + beforeAll(async () => { + const init = await initApi() + ownerApi = init.$tvApi + outsiderApi = init.$tvApiForSecondUser + deleteAllGoals = init.deleteAllGoals + + const auth = await axios.post(`${API_URL}/module/auth/login`, { + login: DEFAULT_USER_2, + password: DEFAULT_PASSWORD, + }) + attackerAxios = axios.create({ + baseURL: API_URL, + headers: { Authorization: `Bearer ${auth.data.access}` }, + validateStatus: () => true, + }) + + const victimGoal = await ownerApi.goals.createGoal({ name: `Victim graph ${Date.now()}` }) + if (!victimGoal) throw new Error('Failed to create victim goal') + victimGoalId = victimGoal.id + + const victimTask = await ownerApi.tasks.createTask({ + goalId: victimGoalId, + description: `victim-task-${Date.now()}`, + }) + if (!victimTask) throw new Error('Failed to create victim task') + victimTaskId = victimTask.id + + const attackerGoal = await outsiderApi.goals.createGoal({ name: `Attacker graph ${Date.now()}` }) + if (!attackerGoal) throw new Error('Failed to create attacker goal') + + const attackerTask = await outsiderApi.tasks.createTask({ + goalId: attackerGoal.id, + description: `attacker-task-${Date.now()}`, + }) + if (!attackerTask) throw new Error('Failed to create attacker task') + attackerTaskId = attackerTask.id + }) + + afterAll(async () => { + await deleteAllGoals() + }) + + async function expectHttpStatus(promise: Promise, status: number): Promise { + try { + await promise + throw new Error(`Expected HTTP ${status} but request succeeded`) + } catch (e: any) { + if (typeof e.message === 'string' && e.message.startsWith('Expected HTTP')) throw e + expect(e.response?.status ?? e.status, `Expected ${status}, got ${e.response?.status ?? e.status}`).toBe(status) + } + } + + async function createVictimEdge(): Promise { + const from = await ownerApi.tasks.createTask({ + goalId: victimGoalId, + description: `victim-edge-from-${Date.now()}`, + }) + const to = await ownerApi.tasks.createTask({ + goalId: victimGoalId, + description: `victim-edge-to-${Date.now()}`, + }) + if (!from || !to) throw new Error('Failed to create victim tasks') + + const edge = await ownerApi.graph.addEdge({ source: from.id, target: to.id }) + if (!edge) throw new Error('Failed to create victim edge') + return edge.id + } + + async function victimEdgeExists(edgeId: number): Promise { + const edges = await ownerApi.graph.fetchAllEdges(victimGoalId) + return (edges ?? []).some(e => e.id === edgeId) + } + + it('rejects deleting another user edge even when a self-owned source task is supplied', async () => { + const edgeId = await createVictimEdge() + + const response = await attackerAxios.delete(`/module/graph/${edgeId}`, { + data: { source: attackerTaskId }, + }) + + expect( + await victimEdgeExists(edgeId), + `victim edge ${edgeId} was destroyed by a non-member`, + ).toBe(true) + expect(response.status).toBe(403) + }) + + // A graph lives inside one project, so an edge across two of them is not a + // permission question but an impossible object: it is refused before any + // permission is looked at. Driven through the SDK on purpose — this needs no + // crafted request at all, an ordinary client using the public API reaches it. + it('rejects creating an edge whose endpoints live in different projects', async () => { + await expectHttpStatus( + outsiderApi.graph.addEdge({ source: attackerTaskId, target: victimTaskId }), + 400, + ) + }) + + // the mirror of the case above: a foreign source with an own target. This one + // fails closed even without the endpoint comparison (the goal would resolve to + // the victim project and the permission check would deny it), which is exactly + // why it needs pinning — a regression here would be silent + it('rejects creating an edge from a foreign task into a project the caller owns', async () => { + await expectHttpStatus( + outsiderApi.graph.addEdge({ source: victimTaskId, target: attackerTaskId }), + 400, + ) + }) + + // both endpoints inside the victim project: the goal resolves cleanly, so this + // is decided purely by the permission check on that goal + it('rejects creating an edge between two tasks of a project the caller is not a member of', async () => { + const second = await ownerApi.tasks.createTask({ + goalId: victimGoalId, + description: `victim-second-${Date.now()}`, + }) + if (!second) throw new Error('Failed to create second victim task') + + await expectHttpStatus( + outsiderApi.graph.addEdge({ source: victimTaskId, target: second.id }), + 403, + ) + }) + + it('control: without the injected source the guard already rejects the delete', async () => { + const edgeId = await createVictimEdge() + + const response = await attackerAxios.delete(`/module/graph/${edgeId}`) + + expect(response.status).toBe(403) + expect(await victimEdgeExists(edgeId)).toBe(true) + }) + + it('control: the owner can still delete their own edge', async () => { + const edgeId = await createVictimEdge() + + const deleted = await ownerApi.graph.deleteEdge(edgeId) + expect(deleted).toBeTruthy() + expect(await victimEdgeExists(edgeId)).toBe(false) + }) +}) diff --git a/taskview-packages/taskview-api/src/api/__tests__/guard-param-confusion.test.ts b/taskview-packages/taskview-api/src/api/__tests__/guard-param-confusion.test.ts new file mode 100644 index 0000000..5babd32 --- /dev/null +++ b/taskview-packages/taskview-api/src/api/__tests__/guard-param-confusion.test.ts @@ -0,0 +1,113 @@ +import { TvApi } from '@/tv' +import axios, { type AxiosInstance } from 'axios' +import { describe, it, expect, beforeAll, afterAll } from 'vitest' +import { API_URL, DEFAULT_PASSWORD, DEFAULT_USER_2, initApi } from './init-api' + +/** + * Several guards pick the goal to authorize with `req.body.goalId ? req.body : req.params`, + * while their handlers read `req.params`. Supplying a body that names a goal the caller owns + * must not authorize a request whose path points at someone else's goal. + */ +describe('Guard/handler parameter confusion', () => { + let ownerApi: TvApi + let deleteAllGoals: () => Promise + let attackerAxios: AxiosInstance + let victimGoalId: number + let attackerGoalId: number + let victimColumnId: number + + beforeAll(async () => { + const init = await initApi() + ownerApi = init.$tvApi + deleteAllGoals = init.deleteAllGoals + + const auth = await axios.post(`${API_URL}/module/auth/login`, { + login: DEFAULT_USER_2, + password: DEFAULT_PASSWORD, + }) + attackerAxios = axios.create({ + baseURL: API_URL, + headers: { Authorization: `Bearer ${auth.data.access}` }, + validateStatus: () => true, + }) + + const victimGoal = await ownerApi.goals.createGoal({ name: `Victim confusion ${Date.now()}` }) + if (!victimGoal) throw new Error('Failed to create victim goal') + victimGoalId = victimGoal.id + + const attackerGoal = await axios.post( + `${API_URL}/module/goals`, + { name: `Attacker confusion ${Date.now()}` }, + { headers: { Authorization: `Bearer ${auth.data.access}` } }, + ) + attackerGoalId = attackerGoal.data.response.id + + await ownerApi.tasks.createTask({ + goalId: victimGoalId, + description: `secret-task-${Date.now()}`, + }) + + const columns = await ownerApi.kanban.fetchAllColumns(victimGoalId) + if (!columns?.length) throw new Error('Victim goal has no kanban columns') + victimColumnId = columns[0].id + }) + + afterAll(async () => { + await deleteAllGoals() + }) + + it('rejects reading another goal kanban tasks when a self-owned goalId is put in the body', async () => { + const response = await attackerAxios.request({ + method: 'get', + url: `/module/kanban/tasks/${victimGoalId}/${victimColumnId}/0`, + data: { goalId: attackerGoalId, columnId: victimColumnId }, + }) + + expect( + response.status, + `Leaked kanban tasks of goal ${victimGoalId}: ${JSON.stringify(response.data)}`, + ).toBe(403) + }) + + it('rejects reading another goal task order when a self-owned goalId is put in the body', async () => { + const response = await attackerAxios.request({ + method: 'get', + url: `/module/kanban/tasks-order/${victimGoalId}/${victimColumnId}/0`, + data: { goalId: attackerGoalId, columnId: victimColumnId }, + }) + + expect(response.status).toBe(403) + }) + + it('rejects reading another goal role-to-permission matrix when a self-owned goalId is put in the body', async () => { + const response = await attackerAxios.request({ + method: 'get', + url: `/module/collaborationroles/role-to-permissions/${victimGoalId}`, + data: { goalId: attackerGoalId }, + }) + + expect( + response.status, + `Leaked role matrix of goal ${victimGoalId}: ${JSON.stringify(response.data)}`, + ).toBe(403) + }) + + it('control: the same requests without a body are already rejected', async () => { + const kanban = await attackerAxios.get(`/module/kanban/tasks/${victimGoalId}/${victimColumnId}/0`) + expect(kanban.status).toBe(403) + + const roles = await attackerAxios.get(`/module/collaborationroles/role-to-permissions/${victimGoalId}`) + expect(roles.status).toBe(403) + }) + + it('control: the owner still reads their own kanban tasks and role matrix', async () => { + const tasks = await ownerApi.kanban + .fetchTasksForColumn(victimGoalId, victimColumnId, 0) + .catch((e: any) => { throw new Error(`kanban read failed: ${e.response?.status}`) }) + expect(tasks).toBeDefined() + + const matrix = await ownerApi.collaboration.fetchRoleToPermissionsForGoal(victimGoalId) + .catch((e: any) => { throw new Error(`role matrix read failed: ${e.response?.status}`) }) + expect(matrix).toBeDefined() + }) +}) diff --git a/taskview-packages/taskview-api/src/api/__tests__/integrations-access.test.ts b/taskview-packages/taskview-api/src/api/__tests__/integrations-access.test.ts new file mode 100644 index 0000000..7faa003 --- /dev/null +++ b/taskview-packages/taskview-api/src/api/__tests__/integrations-access.test.ts @@ -0,0 +1,236 @@ +import { TvApi } from '@/tv' +import { TvPermissions } from '@/api/permissions' +import axios, { type AxiosInstance } from 'axios' +import { describe, it, expect, beforeAll, afterAll } from 'vitest' +import { API_URL, DEFAULT_PASSWORD, DEFAULT_USER_2, initApi } from './init-api' + +/** + * The integrations guards resolve the project to authorize against via + * resolveProjectId(), which prefers a projectId supplied by the caller over the + * one derived from integrationId. The handlers, however, act on integrationId. + * A caller must not be able to pass a project they own alongside someone else's + * integration id and have the guard authorize the wrong object. + */ +describe('Integrations object-level access control', () => { + let ownerApi: TvApi + let outsiderApi: TvApi + let deleteAllGoals: () => Promise + let outsiderEmail: string + let attackerAxios: AxiosInstance + let victimGoalId: number + let attackerGoalId: number + + beforeAll(async () => { + const init = await initApi() + ownerApi = init.$tvApi + outsiderApi = init.$tvApiForSecondUser + deleteAllGoals = init.deleteAllGoals + outsiderEmail = init.user2Email + + const auth = await axios.post(`${API_URL}/module/auth/login`, { + login: DEFAULT_USER_2, + password: DEFAULT_PASSWORD, + }) + attackerAxios = axios.create({ + baseURL: API_URL, + headers: { Authorization: `Bearer ${auth.data.access}` }, + validateStatus: () => true, + }) + + const victimGoal = await ownerApi.goals.createGoal({ name: `Victim project ${Date.now()}` }) + if (!victimGoal) throw new Error('Failed to create victim goal') + victimGoalId = victimGoal.id + + const attackerGoal = await outsiderApi.goals.createGoal({ name: `Attacker project ${Date.now()}` }) + if (!attackerGoal) throw new Error('Failed to create attacker goal') + attackerGoalId = attackerGoal.id + }) + + afterAll(async () => { + await deleteAllGoals() + }) + + async function createVictimIntegration(): Promise { + const created = await ownerApi.integrations.createIntegration({ + provider: 'github', + repoFullName: `victim-org/private-repo-${Date.now()}`, + projectId: victimGoalId, + }) + if (!created) throw new Error('Failed to create victim integration') + return created.id + } + + async function victimIntegrationExists(integrationId: number): Promise { + const list = await ownerApi.integrations.fetchIntegrations(victimGoalId) + return (list ?? []).some(i => i.id === integrationId) + } + + it('rejects deleting another user integration even when a self-owned projectId is supplied', async () => { + const integrationId = await createVictimIntegration() + + const response = await attackerAxios.delete('/module/integrations', { + data: { id: integrationId, projectId: attackerGoalId }, + }) + + // impact first, mechanism second — so a failure reports whether data was actually destroyed + expect( + await victimIntegrationExists(integrationId), + `victim integration ${integrationId} was destroyed by a non-member`, + ).toBe(true) + expect( + response.status, + `Guard authorized project ${attackerGoalId} while the handler acted on integration ${integrationId}`, + ).toBe(403) + }) + + it('rejects toggling another user integration even when a self-owned projectId is supplied', async () => { + const integrationId = await createVictimIntegration() + + const response = await attackerAxios.patch('/module/integrations/toggle', { + id: integrationId, + isActive: false, + projectId: attackerGoalId, + }) + + expect(response.status).toBe(403) + }) + + it('rejects reading another user integration repos even when a self-owned projectId is supplied', async () => { + const integrationId = await createVictimIntegration() + + const response = await attackerAxios.get('/module/integrations/repos', { + params: { integrationId, projectId: attackerGoalId }, + }) + + expect(response.status).toBe(403) + }) + + it('rejects syncing another user integration even when a self-owned projectId is supplied', async () => { + const integrationId = await createVictimIntegration() + + const response = await attackerAxios.post('/module/integrations/sync', { + integrationId, + projectId: attackerGoalId, + }) + + expect(response.status).toBe(403) + }) + + it('control: without the injected projectId the guard already rejects the same request', async () => { + const integrationId = await createVictimIntegration() + + const response = await attackerAxios.delete('/module/integrations', { + data: { id: integrationId }, + }) + + expect(response.status).toBe(403) + expect(await victimIntegrationExists(integrationId)).toBe(true) + }) + + // select-repo is the most consequential handler of the four: besides writing to + // the integration it kicks off syncIssues() and registerWebhook() against the repo + it('rejects selecting a repo on another user integration even when a self-owned projectId is supplied', async () => { + const integrationId = await createVictimIntegration() + + const response = await attackerAxios.patch('/module/integrations/select-repo', { + integrationId, + repoFullName: 'attacker-org/planted-repo', + repoExternalId: '424242', + projectId: attackerGoalId, + }) + + expect(response.status).toBe(403) + }) + + // resolveProjectId reads projectId from the query string too, so a fix that only + // hardens the body would still leave this door open + it('rejects the same bypass when projectId arrives via the query string', async () => { + const integrationId = await createVictimIntegration() + + const response = await attackerAxios.delete('/module/integrations', { + params: { projectId: attackerGoalId }, + data: { id: integrationId }, + }) + + expect( + await victimIntegrationExists(integrationId), + `victim integration ${integrationId} was destroyed via a query-string projectId`, + ).toBe(true) + expect(response.status).toBe(403) + }) + + it('rejects listing the integrations of a project the caller is not a member of', async () => { + await createVictimIntegration() + + const response = await attackerAxios.get('/module/integrations', { + params: { projectId: victimGoalId }, + }) + + expect( + response.status, + `Leaked integrations of project ${victimGoalId}: ${JSON.stringify(response.data)}`, + ).toBe(403) + }) + + it('rejects planting a new integration into a project the caller is not a member of', async () => { + const response = await attackerAxios.post('/module/integrations', { + provider: 'github', + repoFullName: 'attacker-org/planted-repo', + projectId: victimGoalId, + }) + + expect(response.status).toBe(403) + }) + + it('control: the owner can still manage their own integration', async () => { + const integrationId = await createVictimIntegration() + + const deleted = await ownerApi.integrations.deleteIntegration(integrationId) + expect(deleted).toBeTruthy() + expect(await victimIntegrationExists(integrationId)).toBe(false) + }) + + // guards against an over-strict fix: a project member holding integrations_can_manage + // must keep working, not just the goal owner + it('control: a project member with integrations_can_manage can delete the integration', async () => { + const goal = await ownerApi.goals.createGoal({ name: `Shared integrations ${Date.now()}` }) + if (!goal) throw new Error('Failed to create goal') + + const collab = await ownerApi.collaboration.inviteUserToGoal({ email: outsiderEmail, goalId: goal.id }) + if (!collab) throw new Error('Failed to invite user2') + + const allPermissions = await ownerApi.collaboration.fetchAllPermissions() + const managePermission = allPermissions.find(p => p.name === TvPermissions.INTEGRATIONS_CAN_MANAGE) + if (!managePermission) throw new Error('Permission "integrations_can_manage" is not in DB') + + const role = await ownerApi.collaboration.createRoleForGoal({ + goalId: goal.id, + roleName: `Integrator ${Date.now()}`, + }) + if (!role) throw new Error('Failed to create role') + + const toggled = await ownerApi.collaboration.toggleRolePermission({ + roleId: role.id, + permissionId: managePermission.id, + }) + if (!toggled || toggled.add !== true) { + throw new Error(`Expected integrations_can_manage to be added, got ${JSON.stringify(toggled)}`) + } + + await ownerApi.collaboration.toggleUserRoles({ + goalId: goal.id, + userId: collab.id, + roles: [role.id], + }) + + const created = await ownerApi.integrations.createIntegration({ + provider: 'github', + repoFullName: `shared-org/repo-${Date.now()}`, + projectId: goal.id, + }) + if (!created) throw new Error('Failed to create integration') + + const deleted = await outsiderApi.integrations.deleteIntegration(created.id) + expect(deleted).toBeTruthy() + }) +}) diff --git a/taskview-packages/taskview-api/src/api/__tests__/kanban.access.test.ts b/taskview-packages/taskview-api/src/api/__tests__/kanban.access.test.ts new file mode 100644 index 0000000..44e4847 --- /dev/null +++ b/taskview-packages/taskview-api/src/api/__tests__/kanban.access.test.ts @@ -0,0 +1,220 @@ +import { TvApi } from '@/tv' +import { TvPermissions } from '@/api/permissions' +import axios, { type AxiosInstance } from 'axios' +import { describe, it, expect, beforeAll, afterAll } from 'vitest' +import { API_URL, DEFAULT_PASSWORD, DEFAULT_USER_2, initApi } from './init-api' + +/** + * Board writes (columns and task placement) are gated by kanban_can_manage, the + * permission the UI has always used. They used to accept component_can_add_tasks + * or task_can_add_subtasks instead, which let a rank-and-file member delete other + * people's board columns. + */ +describe('Kanban permission boundaries', () => { + let ownerApi: TvApi + let outsiderApi: TvApi + let outsiderEmail: string + let deleteAllGoals: () => Promise + let attackerAxios: AxiosInstance + const permissionIdByName = new Map() + + beforeAll(async () => { + const init = await initApi() + ownerApi = init.$tvApi + outsiderApi = init.$tvApiForSecondUser + outsiderEmail = init.user2Email + deleteAllGoals = init.deleteAllGoals + + const auth = await axios.post(`${API_URL}/module/auth/login`, { + login: DEFAULT_USER_2, + password: DEFAULT_PASSWORD, + }) + attackerAxios = axios.create({ + baseURL: API_URL, + headers: { Authorization: `Bearer ${auth.data.access}` }, + validateStatus: () => true, + }) + + for (const permission of await ownerApi.collaboration.fetchAllPermissions()) { + permissionIdByName.set(permission.name, permission.id) + } + }) + + afterAll(async () => { + await deleteAllGoals() + }) + + async function expectHttpStatus(promise: Promise, status: number): Promise { + try { + await promise + throw new Error(`Expected HTTP ${status} but request succeeded`) + } catch (e: any) { + if (typeof e.message === 'string' && e.message.startsWith('Expected HTTP')) throw e + expect(e.response?.status ?? e.status, `Expected ${status}, got ${e.response?.status ?? e.status}`).toBe(status) + } + } + + /** A goal of user1 that user2 joins through a role carrying exactly `permissionNames`. */ + async function shareGoalWith(permissionNames: string[]) { + const goal = await ownerApi.goals.createGoal({ name: `Kanban access ${Date.now()}` }) + if (!goal) throw new Error('Failed to create goal') + + const collab = await ownerApi.collaboration.inviteUserToGoal({ email: outsiderEmail, goalId: goal.id }) + if (!collab) throw new Error('Failed to invite user2') + + const role = await ownerApi.collaboration.createRoleForGoal({ + goalId: goal.id, + roleName: `Role ${Date.now()}`, + }) + if (!role) throw new Error('Failed to create role') + + for (const name of permissionNames) { + const permissionId = permissionIdByName.get(name) + if (!permissionId) throw new Error(`Permission "${name}" is not in DB`) + + const toggled = await ownerApi.collaboration.toggleRolePermission({ roleId: role.id, permissionId }) + if (!toggled || toggled.add !== true) { + throw new Error(`Expected "${name}" to be added, got ${JSON.stringify(toggled)}`) + } + } + + await ownerApi.collaboration.toggleUserRoles({ goalId: goal.id, userId: collab.id, roles: [role.id] }) + + return goal + } + + async function addColumn(goalId: number, name: string) { + const column = await ownerApi.kanban.addColumn({ goalId, name }) + if (!column) throw new Error('Failed to create column') + return column + } + + describe('a member holding kanban_can_manage', () => { + it('can create, rename and delete a board column', async () => { + const goal = await shareGoalWith([TvPermissions.KANBAN_CAN_MANAGE]) + + const created = await outsiderApi.kanban.addColumn({ goalId: goal.id, name: 'Created by member' }) + expect(created?.id).toBeGreaterThan(0) + + const renamed = await outsiderApi.kanban.updateColumn({ id: created!.id, name: 'Renamed by member' }) + expect(renamed).toBeTruthy() + + const deleted = await outsiderApi.kanban.deleteColumn({ id: created!.id }) + expect(deleted).toBeTruthy() + }) + + it('can move a task into another column', async () => { + const goal = await shareGoalWith([TvPermissions.KANBAN_CAN_MANAGE]) + const from = await addColumn(goal.id, 'From') + const to = await addColumn(goal.id, 'To') + + const task = await ownerApi.tasks.createTask({ + goalId: goal.id, + description: `movable-${Date.now()}`, + statusId: from.id, + }) + if (!task) throw new Error('Failed to create task') + + const moved = await outsiderApi.kanban.updateTasksOrderAndColumn({ + goalId: goal.id, + columnId: to.id, + taskId: task.id, + prevTaskId: null, + nextTaskId: null, + }) + expect(moved).toBeDefined() + }) + }) + + describe('a member holding only task-level permissions', () => { + // exactly the pair the routes used to accept — the escalation that was closed + const TASK_LEVEL = [TvPermissions.COMPONENT_CAN_ADD_TASKS, TvPermissions.TASK_CAN_ADD_SUBTASKS] + + it('cannot create a board column', async () => { + const goal = await shareGoalWith(TASK_LEVEL) + + await expectHttpStatus(outsiderApi.kanban.addColumn({ goalId: goal.id, name: 'Nope' }), 403) + }) + + it('cannot rename or delete a board column', async () => { + const goal = await shareGoalWith(TASK_LEVEL) + const column = await addColumn(goal.id, 'Owned by user1') + + await expectHttpStatus(outsiderApi.kanban.updateColumn({ id: column.id, name: 'Nope' }), 403) + await expectHttpStatus(outsiderApi.kanban.deleteColumn({ id: column.id }), 403) + + const survivors = await ownerApi.kanban.fetchAllColumns(goal.id) + expect(survivors?.some(c => c.id === column.id), 'column was destroyed').toBe(true) + }) + + it('cannot move a task into another column', async () => { + const goal = await shareGoalWith(TASK_LEVEL) + const from = await addColumn(goal.id, 'From') + const to = await addColumn(goal.id, 'To') + + const task = await ownerApi.tasks.createTask({ + goalId: goal.id, + description: `pinned-${Date.now()}`, + statusId: from.id, + }) + if (!task) throw new Error('Failed to create task') + + await expectHttpStatus( + outsiderApi.kanban.updateTasksOrderAndColumn({ + goalId: goal.id, + columnId: to.id, + taskId: task.id, + prevTaskId: null, + nextTaskId: null, + }), + 403, + ) + }) + }) + + describe('a member holding only kanban_can_view', () => { + it('can read the task order of a column', async () => { + const goal = await shareGoalWith([TvPermissions.KANBAN_CAN_VIEW]) + const column = await addColumn(goal.id, 'Readable') + + const order = await outsiderApi.kanban.getTaskOrdersForColumnAndCursor(goal.id, column.id, null) + expect(order).toBeDefined() + }) + + it('cannot create a board column', async () => { + const goal = await shareGoalWith([TvPermissions.KANBAN_CAN_VIEW]) + + await expectHttpStatus(outsiderApi.kanban.addColumn({ goalId: goal.id, name: 'Nope' }), 403) + }) + }) + + describe('the goal a column belongs to is never taken from the request', () => { + it('rejects deleting or renaming a foreign column even when a self-owned goalId is supplied', async () => { + const victimGoal = await ownerApi.goals.createGoal({ name: `Victim board ${Date.now()}` }) + if (!victimGoal) throw new Error('Failed to create victim goal') + const victimColumn = await addColumn(victimGoal.id, 'Victim column') + + // a project user2 fully controls, offered to the guard as the authorization target + const ownGoal = await outsiderApi.goals.createGoal({ name: `Attacker board ${Date.now()}` }) + if (!ownGoal) throw new Error('Failed to create attacker goal') + + const deleteResponse = await attackerAxios.post('/module/kanban/delete-status', { + id: victimColumn.id, + goalId: ownGoal.id, + }) + const updateResponse = await attackerAxios.post('/module/kanban/update-status', { + id: victimColumn.id, + name: 'Renamed by an outsider', + goalId: ownGoal.id, + }) + + const survivors = await ownerApi.kanban.fetchAllColumns(victimGoal.id) + const survivor = (survivors ?? []).find(c => c.id === victimColumn.id) + expect(survivor, `victim column ${victimColumn.id} was destroyed`).toBeDefined() + expect(survivor?.name, 'victim column was renamed').toBe('Victim column') + + expect(deleteResponse.status).toBe(403) + expect(updateResponse.status).toBe(403) + }) + }) +}) diff --git a/taskview-packages/taskview-api/src/api/__tests__/tasks-access.test.ts b/taskview-packages/taskview-api/src/api/__tests__/tasks-access.test.ts new file mode 100644 index 0000000..7090884 --- /dev/null +++ b/taskview-packages/taskview-api/src/api/__tests__/tasks-access.test.ts @@ -0,0 +1,80 @@ +import { TvApi } from '@/tv' +import axios, { type AxiosInstance } from 'axios' +import { describe, it, expect, beforeAll, afterAll } from 'vitest' +import { API_URL, DEFAULT_PASSWORD, DEFAULT_USER_2, initApi } from './init-api' + +/** + * CanFetchTask authorizes `req.query.taskId || req.params.taskId`, while + * fetchTaskByIdNew reads `req.params`. A caller must not be able to name a task + * they own in the query string and have the guard authorize it while the handler + * returns someone else's task. + */ +describe('Task object-level access control', () => { + let ownerApi: TvApi + let outsiderApi: TvApi + let deleteAllGoals: () => Promise + let attackerAxios: AxiosInstance + let victimTaskId: number + let attackerTaskId: number + + beforeAll(async () => { + const init = await initApi() + ownerApi = init.$tvApi + outsiderApi = init.$tvApiForSecondUser + deleteAllGoals = init.deleteAllGoals + + const auth = await axios.post(`${API_URL}/module/auth/login`, { + login: DEFAULT_USER_2, + password: DEFAULT_PASSWORD, + }) + attackerAxios = axios.create({ + baseURL: API_URL, + headers: { Authorization: `Bearer ${auth.data.access}` }, + validateStatus: () => true, + }) + + const victimGoal = await ownerApi.goals.createGoal({ name: `Victim tasks ${Date.now()}` }) + if (!victimGoal) throw new Error('Failed to create victim goal') + const victimTask = await ownerApi.tasks.createTask({ + goalId: victimGoal.id, + description: `victim-secret-${Date.now()}`, + }) + if (!victimTask) throw new Error('Failed to create victim task') + victimTaskId = victimTask.id + + const attackerGoal = await outsiderApi.goals.createGoal({ name: `Attacker tasks ${Date.now()}` }) + if (!attackerGoal) throw new Error('Failed to create attacker goal') + const attackerTask = await outsiderApi.tasks.createTask({ + goalId: attackerGoal.id, + description: `attacker-own-${Date.now()}`, + }) + if (!attackerTask) throw new Error('Failed to create attacker task') + attackerTaskId = attackerTask.id + }) + + afterAll(async () => { + await deleteAllGoals() + }) + + it('rejects reading another user task when a self-owned taskId is put in the query string', async () => { + const response = await attackerAxios.get(`/module/tasks/${victimTaskId}`, { + params: { taskId: attackerTaskId }, + }) + + expect( + response.status, + `Leaked task ${victimTaskId}: ${JSON.stringify(response.data)}`, + ).toBe(403) + }) + + it('control: without the query parameter the guard already rejects the same request', async () => { + const response = await attackerAxios.get(`/module/tasks/${victimTaskId}`) + + expect(response.status).toBe(403) + }) + + it('control: the owner can still read their own task', async () => { + const task = await ownerApi.tasks.fetchTaskById(victimTaskId) + expect(task?.id).toBe(victimTaskId) + }) +}) diff --git a/taskview-packages/taskview-api/src/api/graph.ts b/taskview-packages/taskview-api/src/api/graph.ts index ac342fb..8964e12 100644 --- a/taskview-packages/taskview-api/src/api/graph.ts +++ b/taskview-packages/taskview-api/src/api/graph.ts @@ -13,6 +13,14 @@ export default class TvGraph extends TvApiBase { ); } + public async fetchTaskEdges(taskId: number) { + return this.request( + this.$axios.get>( + `${this.moduleUrl}/task/${taskId}` + ) + ); + } + public async fetchAllEdges(goalId: number) { return this.request( this.$axios.get>( diff --git a/taskview-packages/taskview-mcp/package.json b/taskview-packages/taskview-mcp/package.json index 4b63bf5..072f526 100644 --- a/taskview-packages/taskview-mcp/package.json +++ b/taskview-packages/taskview-mcp/package.json @@ -1,6 +1,6 @@ { "name": "taskview-mcp", - "version": "1.52.0", + "version": "1.53.0", "description": "MCP (Model Context Protocol) server for TaskView — lets AI assistants (Claude Code, Claude Desktop, etc.) manage projects and tasks via the TaskView API", "type": "module", "bin": { diff --git a/web/package.json b/web/package.json index 273df2e..d6958f5 100644 --- a/web/package.json +++ b/web/package.json @@ -2,7 +2,7 @@ "name": "web-nuxt-ui", "private": true, "type": "module", - "version": "1.52.0", + "version": "1.53.0", "scripts": { "dev": "vite", "build:packages": "pnpm --filter taskview-db-schemas build && pnpm --filter taskview-api build && pnpm --filter capacitor-widget-bridge build", diff --git a/web/src/components/features/auth/ServerSelector.vue b/web/src/components/features/auth/ServerSelector.vue index dffd53e..8f14d35 100644 --- a/web/src/components/features/auth/ServerSelector.vue +++ b/web/src/components/features/auth/ServerSelector.vue @@ -68,6 +68,10 @@ placeholder="https://api.example.com" class="w-full" autofocus + autocapitalize="none" + autocorrect="off" + spellcheck="false" + inputmode="url" @keyup.enter="handleAddServer" /> @@ -104,6 +108,7 @@ import { computed, onMounted, ref, watch } from 'vue' import { useI18n } from 'vue-i18n' import { useBreakpoints, breakpointsTailwind } from '@vueuse/core' import { useAdditionalServer } from '@/composables/useAdditionalServer' +import { normalizeServerUrl } from '@/helpers/serverUrl' const { t } = useI18n() const bp = useBreakpoints({ ...breakpointsTailwind, fullscreenModalMax: 1366 }) @@ -153,15 +158,7 @@ const serverOptions = computed(() => { return options }) -const isValidUrl = computed(() => { - if (!newServerUrl.value) return false - try { - const url = new URL(newServerUrl.value) - return url.protocol === 'http:' || url.protocol === 'https:' - } catch { - return false - } -}) +const isValidUrl = computed(() => normalizeServerUrl(newServerUrl.value) !== null) watch(selectedServer, (newServer) => { if (newServer && newServer !== mainServer.value) { @@ -171,9 +168,8 @@ watch(selectedServer, (newServer) => { }) function handleAddServer() { - if (!isValidUrl.value) return - - const url = newServerUrl.value.trim().replace(/\/$/, '') + const url = normalizeServerUrl(newServerUrl.value) + if (!url) return if (!allServers.value.includes(url) && url !== systemServer.value) { addServerFn.value(url) diff --git a/web/src/components/features/auth/auth.helper.ts b/web/src/components/features/auth/auth.helper.ts index 038b216..b8892c3 100644 --- a/web/src/components/features/auth/auth.helper.ts +++ b/web/src/components/features/auth/auth.helper.ts @@ -1,16 +1,9 @@ -import { ALL_TASKS_LIST_ID, type DefaultView } from 'taskview-api' import type { RouteLocationRaw, Router } from 'vue-router' import { $tvApi } from '@/plugins/axios' import { useUserStore } from '@/stores/user.store' import { useOrganizationStore } from '@/stores/organization.store' import { useUiPreferencesStore } from '@/stores/uiPreferences.store' - -const VIEW_ROUTES: Record = { - tasks: 'user', - kanban: 'kanban', - graph: 'graph', - sprints: 'sprints', -} +import { useProjectRoute } from '@/composables/useProjectRoute' export const redirectToUser = async (router: Router) => { const userStore = useUserStore() @@ -36,15 +29,12 @@ export const resolveDefaultRoute = async (): Promise => // The default project may live in any of the user's organizations; try the current one first const orgStore = useOrganizationStore() const orgs = [...orgStore.organizations].sort((a) => (a.slug === orgStore.currentOrgSlug ? -1 : 1)) + const { projectRoute } = useProjectRoute() for (const org of orgs) { const goals = await $tvApi.goals.fetchGoals(org.id) - if (!goals?.some((goal) => goal.id === projectId)) continue - - const view = uiPrefs.settings.defaultView ?? 'tasks' - const params: Record = { orgSlug: org.slug, projectId } - if (view === 'tasks') params.listId = ALL_TASKS_LIST_ID - return { name: VIEW_ROUTES[view], params } + const goal = goals?.find((g) => g.id === projectId) + if (goal) return projectRoute(goal, org.slug) } return null diff --git a/web/src/components/features/collaboration/CollaborationPanel.vue b/web/src/components/features/collaboration/CollaborationPanel.vue index 52d5bf8..cf1b1ec 100644 --- a/web/src/components/features/collaboration/CollaborationPanel.vue +++ b/web/src/components/features/collaboration/CollaborationPanel.vue @@ -18,6 +18,10 @@ class="w-full" :ui="{ list: 'rounded-2xl', trigger: 'rounded-xl', indicator: 'rounded-xl' }" > + + @@ -39,6 +39,7 @@ import { Handle, Position } from '@vue-flow/core' import { computed } from 'vue' import type { TaskItem } from '@/types/tasks.types' +import TaskItemCard from '@/components/features/tasks/parts/TaskItem.vue' import { useTasksStore } from '@/stores/tasks.store' import { Task } from 'taskview-api' diff --git a/web/src/components/features/graph/composables/useLayout.ts b/web/src/components/features/graph/composables/useLayout.ts index cc0063e..ab908f7 100644 --- a/web/src/components/features/graph/composables/useLayout.ts +++ b/web/src/components/features/graph/composables/useLayout.ts @@ -1,56 +1,106 @@ import dagre from '@dagrejs/dagre' -import { type Edge, type Node, Position, useVueFlow } from '@vue-flow/core' -import { ref } from 'vue' +import { type Edge, type Node, Position } from '@vue-flow/core' + +const NODE_WIDTH = 288 // w-72 wrapper in TaskNode +const NODE_MIN_HEIGHT = 74 // checkbox + priority column with paddings +const NODE_PADDING_Y = 28 // p-3.5 top + bottom +const TITLE_LINE_HEIGHT = 24 // text-base +// Conservative: word-wrapping rarely fills lines completely, better to +// overestimate height than to let ranks overlap +const TITLE_CHARS_PER_LINE = 22 +const BADGE_ROW_HEIGHT = 30 +const BADGE_ROW_GAP = 8 +const TITLE_BADGES_GAP = 4 +// Handles stick out ~8px beyond the card on both sides, and the height estimate +// can be off by a line — this safety margin keeps neighbors from touching +const NODE_SAFETY = 24 +const CONTENT_WIDTH = 230 // node width minus paddings and the checkbox column +const BADGE_CHROME_WIDTH = 34 // badge paddings + icon +const BADGE_CHAR_WIDTH = 6.5 +const BADGE_GAP = 8 + +const ISOLATED_GAP_X = 40 +const ISOLATED_GAP_Y = 32 +const ISOLATED_BLOCK_OFFSET = 120 +const ISOLATED_MIN_ROW_WIDTH = 1200 + +// Estimates the rendered TaskNode size from task data alone, so the layout can +// run before (and without) rendering every node — a prerequisite for +// only-render-visible-elements, where offscreen nodes are never measured. +function estimateNodeSize(node: Node): { width: number; height: number } { + const task = node.data?.task + if (!task) return { width: NODE_WIDTH, height: NODE_MIN_HEIGHT } + + const titleLines = Math.max(1, Math.ceil((task.description?.length ?? 0) / TITLE_CHARS_PER_LINE)) + + // Estimated pixel widths of the badges TaskItem renders, in render order + const badgeWidth = (labelLength: number) => + Math.min(CONTENT_WIDTH, BADGE_CHROME_WIDTH + labelLength * BADGE_CHAR_WIDTH) + const badgeWidths: number[] = [] + if (task.endDate) badgeWidths.push(badgeWidth(11)) // dd.Mon.yyyy + if (task.recurrenceRuleId) badgeWidths.push(BADGE_CHROME_WIDTH) // icon-only + if (task.goalListId) badgeWidths.push(badgeWidth(10)) // list name (unknown here) + if (task.amount) badgeWidths.push(badgeWidth(String(task.amount).length + 1)) + for (let i = 0; i < (task.assignedUsers?.length ?? 0); i++) badgeWidths.push(badgeWidth(20)) // email + for (let i = 0; i < (task.tags?.length ?? 0); i++) badgeWidths.push(badgeWidth(9)) // tag name (unknown here) + + // Greedy flex-wrap simulation: how many rows the badges take + let badgeRows = 0 + let rowRemaining = 0 + for (const width of badgeWidths) { + if (width + (badgeRows === 0 || rowRemaining === CONTENT_WIDTH ? 0 : BADGE_GAP) > rowRemaining) { + badgeRows += 1 + rowRemaining = CONTENT_WIDTH - width + } else { + rowRemaining -= width + BADGE_GAP + } + } + + const height = + NODE_PADDING_Y + + titleLines * TITLE_LINE_HEIGHT + + (badgeRows > 0 ? TITLE_BADGES_GAP + badgeRows * BADGE_ROW_HEIGHT + (badgeRows - 1) * BADGE_ROW_GAP : 0) + + return { width: NODE_WIDTH, height: Math.max(NODE_MIN_HEIGHT, height) + NODE_SAFETY } +} /** * Composable to run the layout algorithm on the graph. - * It uses the `dagre` library to calculate the layout of the nodes and edges. + * Connected nodes are laid out with `dagre`; isolated nodes (no edges) are + * arranged in a grid below the graph so they don't push linked nodes apart. + * Node sizes are estimated from data, so no prior render is required. */ export function useLayout() { - const { findNode } = useVueFlow() - - const graph = ref(new dagre.graphlib.Graph()) - - const previousDirection = ref('LR') - function layout(nodes: Node[], edges: Edge[], direction: 'LR' | 'TB') { - // we create a new graph instance, in case some nodes/edges were removed, otherwise dagre would act as if they were still there - const dagreGraph = new dagre.graphlib.Graph() - - graph.value = dagreGraph - - dagreGraph.setDefaultEdgeLabel(() => ({})) - const isHorizontal = direction === 'LR' + + // Isolated nodes would become extra dagre roots and push linked nodes apart — + // lay out only the connected subgraph, grid the rest separately + const connectedIds = new Set(edges.flatMap((edge) => [edge.source, edge.target])) + const connectedNodes = nodes.filter((node) => connectedIds.has(node.id)) + const isolatedNodes = nodes.filter((node) => !connectedIds.has(node.id)) + + const dagreGraph = new dagre.graphlib.Graph() + dagreGraph.setDefaultEdgeLabel(() => ({})) dagreGraph.setGraph({ rankdir: direction, - // align: 'UL', // Align to upper left nodesep: 50, // Minimum space between nodes ranksep: 100, // Minimum space between ranks marginx: 20, marginy: 20, }) - previousDirection.value = direction - - for (const node of nodes) { - // if you need width+height of nodes for your layout, you can use the dimensions property of the internal node (`GraphNode` type) - const graphNode = findNode(node.id) - - dagreGraph.setNode(node.id, { - width: graphNode?.dimensions.width || 150, - height: graphNode?.dimensions.height || 50, - }) + for (const node of connectedNodes) { + dagreGraph.setNode(node.id, estimateNodeSize(node)) } - for (const edge of edges) { dagreGraph.setEdge(edge.source, edge.target) } dagre.layout(dagreGraph) - // set nodes with updated positions - const layoutedNodes = nodes.map((node) => { + // dagre returns node centers — keep them as centers for the TB inversion below + const layoutedConnected = connectedNodes.map((node) => { const nodeWithPosition = dagreGraph.node(node.id) return { @@ -63,58 +113,60 @@ export function useLayout() { // For TB mode, invert Y coordinates to put root at top if (!isHorizontal) { - const maxY = Math.max(...layoutedNodes.map((node) => node.position.y)) + const maxY = Math.max(...layoutedConnected.map((node) => node.position.y)) - layoutedNodes.forEach((node) => { - const graphNode = findNode(node.id) - const nodeHeight = graphNode?.dimensions.height || 50 - - // Invert Y coordinate and adjust for node height to keep center aligned - node.position.y = maxY - node.position.y + nodeHeight + layoutedConnected.forEach((node) => { + node.position.y = maxY - node.position.y }) } - return layoutedNodes + // Convert centers to top-left corners (what vue-flow positions actually are) + layoutedConnected.forEach((node) => { + const { width, height } = estimateNodeSize(node) + node.position.x -= width / 2 + node.position.y -= height / 2 + }) + + // Grid for isolated nodes below the connected graph + const hasConnected = layoutedConnected.length > 0 + const boundsBottom = hasConnected + ? Math.max(...layoutedConnected.map((node) => node.position.y + estimateNodeSize(node).height)) + : 0 + const boundsLeft = hasConnected + ? Math.min(...layoutedConnected.map((node) => node.position.x)) + : 0 + const boundsWidth = hasConnected + ? Math.max(...layoutedConnected.map((node) => node.position.x + estimateNodeSize(node).width)) - boundsLeft + : 0 + const rowWidth = Math.max(boundsWidth, ISOLATED_MIN_ROW_WIDTH) + + let x = boundsLeft + let y = boundsBottom + (hasConnected ? ISOLATED_BLOCK_OFFSET : 0) + let rowHeight = 0 + + const layoutedIsolated = isolatedNodes.map((node) => { + const { width, height } = estimateNodeSize(node) + + if (x > boundsLeft && x + width > boundsLeft + rowWidth) { + x = boundsLeft + y += rowHeight + ISOLATED_GAP_Y + rowHeight = 0 + } + + const position = { x, y } + x += width + ISOLATED_GAP_X + rowHeight = Math.max(rowHeight, height) + + return { + ...node, + targetPosition: isHorizontal ? Position.Left : Position.Top, + sourcePosition: isHorizontal ? Position.Right : Position.Bottom, + position, + } + }) + + return [...layoutedConnected, ...layoutedIsolated] } - // function layout(nodes: Node[], edges: Edge[], direction: 'LR' | 'TB') { - // // we create a new graph instance, in case some nodes/edges were removed, otherwise dagre would act as if they were still there - // const dagreGraph = new dagre.graphlib.Graph() - - // graph.value = dagreGraph - - // dagreGraph.setDefaultEdgeLabel(() => ({})) - - // const isHorizontal = direction === 'LR' - // dagreGraph.setGraph({ rankdir: direction }) - - // previousDirection.value = direction - - // for (const node of nodes) { - // // if you need width+height of nodes for your layout, you can use the dimensions property of the internal node (`GraphNode` type) - // const graphNode = findNode(node.id) - - // dagreGraph.setNode(node.id, { width: graphNode.dimensions.width || 150, height: graphNode.dimensions.height || 50 }) - // } - - // for (const edge of edges) { - // dagreGraph.setEdge(edge.source, edge.target) - // } - - // dagre.layout(dagreGraph) - - // // set nodes with updated positions - // return nodes.map((node) => { - // const nodeWithPosition = dagreGraph.node(node.id) - - // return { - // ...node, - // targetPosition: isHorizontal ? Position.Left : Position.Top, - // sourcePosition: isHorizontal ? Position.Right : Position.Bottom, - // position: { x: nodeWithPosition.x, y: nodeWithPosition.y }, - // } - // }) - // } - - return { graph, layout, previousDirection } + return { layout } } diff --git a/web/src/components/features/kanban/KanbanBoard.vue b/web/src/components/features/kanban/KanbanBoard.vue index 5769fd3..acb5e33 100644 --- a/web/src/components/features/kanban/KanbanBoard.vue +++ b/web/src/components/features/kanban/KanbanBoard.vue @@ -11,6 +11,7 @@
@@ -46,6 +47,7 @@
{{ t('kanban.addColumn') }} diff --git a/web/src/components/features/kanban/parts/KanbanDeleteModal.vue b/web/src/components/features/kanban/parts/KanbanDeleteModal.vue index 3610c16..b72ecdd 100644 --- a/web/src/components/features/kanban/parts/KanbanDeleteModal.vue +++ b/web/src/components/features/kanban/parts/KanbanDeleteModal.vue @@ -32,6 +32,7 @@ {{ t('common.delete') }} diff --git a/web/src/components/features/kanban/parts/KanbanEditModal.vue b/web/src/components/features/kanban/parts/KanbanEditModal.vue index 1212675..255da83 100644 --- a/web/src/components/features/kanban/parts/KanbanEditModal.vue +++ b/web/src/components/features/kanban/parts/KanbanEditModal.vue @@ -20,6 +20,7 @@ @@ -35,6 +36,7 @@ {{ t('common.save') }} diff --git a/web/src/components/features/kanban/parts/KanbanTitleMenu.vue b/web/src/components/features/kanban/parts/KanbanTitleMenu.vue index 5b1670a..b99bf75 100644 --- a/web/src/components/features/kanban/parts/KanbanTitleMenu.vue +++ b/web/src/components/features/kanban/parts/KanbanTitleMenu.vue @@ -9,6 +9,7 @@ color="neutral" variant="ghost" size="sm" + data-testid="kanban-column-menu-trigger" /> @@ -48,6 +49,7 @@ const menuItems = computed(() => [ label: t('kanban.edit'), icon: 'i-lucide-pencil', ui, + 'data-testid': 'kanban-menu-edit', // size: 'lg', onSelect: () => { editOpen.value = true @@ -58,6 +60,7 @@ const menuItems = computed(() => [ icon: 'i-lucide-trash-2', color: 'error' as const, ui, + 'data-testid': 'kanban-menu-delete', onSelect: () => { deleteOpen.value = true }, diff --git a/web/src/components/features/main/screen-main/parts/SearchActivator.vue b/web/src/components/features/main/screen-main/parts/SearchActivator.vue index d4c3d17..5261f9c 100644 --- a/web/src/components/features/main/screen-main/parts/SearchActivator.vue +++ b/web/src/components/features/main/screen-main/parts/SearchActivator.vue @@ -9,11 +9,9 @@ diff --git a/web/src/components/features/organizations/parts/OrgSsoConfigCard.vue b/web/src/components/features/organizations/parts/OrgSsoConfigCard.vue index 5058d3a..307acf0 100644 --- a/web/src/components/features/organizations/parts/OrgSsoConfigCard.vue +++ b/web/src/components/features/organizations/parts/OrgSsoConfigCard.vue @@ -10,9 +10,17 @@

- + {{ config.enabled ? t('sso.enabled') : t('sso.disabled') }} - + + diff --git a/web/src/components/features/projects/parts/ProjectListBase.vue b/web/src/components/features/projects/parts/ProjectListBase.vue index 6d7c832..f64a3ae 100644 --- a/web/src/components/features/projects/parts/ProjectListBase.vue +++ b/web/src/components/features/projects/parts/ProjectListBase.vue @@ -26,7 +26,7 @@ v-for="project in projects" :key="project.id" variant="taskview" - :to="{ name: 'user', params: { projectId: project.id, listId: '-1401' } }" + :to="projectRoute(project)" :active="currentProjectId === project.id" >
() const { t } = useI18n() +const { projectRoute } = useProjectRoute() const isOpen = defineModel('open', { required: false, default: true }) diff --git a/web/src/components/features/settings/composables/useSettingsHub.ts b/web/src/components/features/settings/composables/useSettingsHub.ts index a41e5b3..2ef78b3 100644 --- a/web/src/components/features/settings/composables/useSettingsHub.ts +++ b/web/src/components/features/settings/composables/useSettingsHub.ts @@ -15,7 +15,7 @@ const LANGUAGE_OPTIONS = [ { label: 'Русский', value: 'ru' }, { label: 'Deutsch', value: 'de' }, { label: 'Español', value: 'es' }, - { label: 'Português do Brasil', value: 'ptBR' }, + { label: 'Português do Brasil', value: 'pt-BR' }, ] export function useSettingsHub() { diff --git a/web/src/components/features/tasks/TaskDetailPanel.vue b/web/src/components/features/tasks/TaskDetailPanel.vue index 4ce8f80..4a2ae04 100644 --- a/web/src/components/features/tasks/TaskDetailPanel.vue +++ b/web/src/components/features/tasks/TaskDetailPanel.vue @@ -103,6 +103,12 @@ :class="colClass(fieldId)" /> + +
+ + +
+ + + + + +
+
+ + + diff --git a/web/src/components/features/tasks/parts/TaskDependencyGroup.vue b/web/src/components/features/tasks/parts/TaskDependencyGroup.vue new file mode 100644 index 0000000..45c816c --- /dev/null +++ b/web/src/components/features/tasks/parts/TaskDependencyGroup.vue @@ -0,0 +1,84 @@ + + + diff --git a/web/src/components/features/tasks/parts/TaskDependencyPicker.vue b/web/src/components/features/tasks/parts/TaskDependencyPicker.vue new file mode 100644 index 0000000..a3ab269 --- /dev/null +++ b/web/src/components/features/tasks/parts/TaskDependencyPicker.vue @@ -0,0 +1,121 @@ + + + diff --git a/web/src/components/features/tasks/parts/TaskTimeTrackingForm.vue b/web/src/components/features/tasks/parts/TaskTimeTrackingForm.vue index cfce683..fbb275b 100644 --- a/web/src/components/features/tasks/parts/TaskTimeTrackingForm.vue +++ b/web/src/components/features/tasks/parts/TaskTimeTrackingForm.vue @@ -123,9 +123,11 @@ const emit = defineEmits<{ const { t } = useI18n() +// Keep seconds: a timer entry started and stopped within the same minute +// otherwise collapses to start == end and can never pass validation const fromDate = (d: Date): { date: CalendarDate; time: Time } => ({ date: new CalendarDate(d.getFullYear(), d.getMonth() + 1, d.getDate()), - time: new Time(d.getHours(), d.getMinutes()), + time: new Time(d.getHours(), d.getMinutes(), d.getSeconds()), }) const fromIso = (iso: string | undefined): { date?: CalendarDate; time?: Time } => { @@ -152,7 +154,7 @@ const endOpen = ref(false) const toJsDate = (date: CalendarDate | undefined, time: Time | undefined): Date | null => { if (!date) return null - return new Date(date.year, date.month - 1, date.day, time?.hour ?? 0, time?.minute ?? 0) + return new Date(date.year, date.month - 1, date.day, time?.hour ?? 0, time?.minute ?? 0, time?.second ?? 0) } const startJs = computed(() => toJsDate(startDate.value, startTime.value)) diff --git a/web/src/components/features/tasks/parts/TasksFilterDrawer.vue b/web/src/components/features/tasks/parts/TasksFilterDrawer.vue index 3351dfb..d62ddeb 100644 --- a/web/src/components/features/tasks/parts/TasksFilterDrawer.vue +++ b/web/src/components/features/tasks/parts/TasksFilterDrawer.vue @@ -11,7 +11,10 @@