From 4b142619c0a2c18b40d1c9a4db1836515000c962 Mon Sep 17 00:00:00 2001 From: Nikolai Giman Date: Thu, 20 Aug 2026 16:13:23 +0200 Subject: [PATCH 01/15] fix: #6126 --- api/src/tv-modules/auth/AuthController.ts | 10 +++ api/src/tv-modules/auth/AuthModel.ts | 15 +++++ .../tv-modules/auth/__tests__/Auth.spec.ts | 61 +++++++++++++++++++ api/src/tv-modules/sso/SsoController.ts | 4 ++ api/src/tv-modules/sso/types.ts | 2 +- web/src/locales/de.ts | 1 + web/src/locales/en.ts | 1 + web/src/locales/es.ts | 1 + web/src/locales/ru.ts | 1 + web/src/pages/login.vue | 1 + 10 files changed, 96 insertions(+), 1 deletion(-) 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/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/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/web/src/locales/de.ts b/web/src/locales/de.ts index a5b374e..211cb29 100644 --- a/web/src/locales/de.ts +++ b/web/src/locales/de.ts @@ -69,6 +69,7 @@ export default { ssoError: 'SSO-Authentifizierung fehlgeschlagen. Bitte versuchen Sie es erneut.', ssoDomainUnverified: 'SSO ist nicht verfügbar, bis die Organisation diese E-Mail-Domain bestätigt.', ssoEmailInUse: 'Diese E-Mail wird bereits von einem anderen TaskView-Konto verwendet.', + ssoAccountBlocked: 'Dieses Konto ist gesperrt. Wenden Sie sich an Ihren Administrator.', invalidEmail: 'Ungültige E-Mail-Adresse', loginRequired: 'Anmeldung erforderlich', passwordRequired: 'Passwort erforderlich', diff --git a/web/src/locales/en.ts b/web/src/locales/en.ts index 14d1994..e99a129 100644 --- a/web/src/locales/en.ts +++ b/web/src/locales/en.ts @@ -81,6 +81,7 @@ export default { ssoError: 'SSO authentication failed. Please try again.', ssoDomainUnverified: 'SSO is not available until the organization verifies this email domain.', ssoEmailInUse: 'This email is already used by another TaskView account.', + ssoAccountBlocked: 'This account is blocked. Contact your administrator.', // Validation invalidEmail: 'Invalid email address', diff --git a/web/src/locales/es.ts b/web/src/locales/es.ts index fc7ec75..106ee83 100644 --- a/web/src/locales/es.ts +++ b/web/src/locales/es.ts @@ -69,6 +69,7 @@ export default { ssoError: 'Error de autenticación SSO. Inténtalo de nuevo.', ssoDomainUnverified: 'SSO no está disponible hasta que la organización verifique este dominio de correo.', ssoEmailInUse: 'Este correo ya está usado por otra cuenta de TaskView.', + ssoAccountBlocked: 'Esta cuenta está bloqueada. Contacta con tu administrador.', invalidEmail: 'Dirección de correo electrónico inválida', loginRequired: 'El usuario es obligatorio', passwordRequired: 'La contraseña es obligatoria', diff --git a/web/src/locales/ru.ts b/web/src/locales/ru.ts index f2bad01..3cc92d8 100644 --- a/web/src/locales/ru.ts +++ b/web/src/locales/ru.ts @@ -81,6 +81,7 @@ export default { ssoError: 'Ошибка SSO авторизации. Попробуйте снова.', ssoDomainUnverified: 'SSO недоступен, пока организация не подтвердит этот email-домен.', ssoEmailInUse: 'Эта почта уже занята другим аккаунтом TaskView.', + ssoAccountBlocked: 'Аккаунт заблокирован. Обратитесь к администратору.', // Validation invalidEmail: 'Неверный email адрес', diff --git a/web/src/pages/login.vue b/web/src/pages/login.vue index 76fb40b..f14b441 100644 --- a/web/src/pages/login.vue +++ b/web/src/pages/login.vue @@ -55,6 +55,7 @@ onMounted(async () => { 'registration-disabled': 'auth.registrationDisabled', domain_unverified: 'auth.ssoDomainUnverified', email_in_use: 'auth.ssoEmailInUse', + account_blocked: 'auth.ssoAccountBlocked', }[String(route.query.sso_error)] ?? 'auth.ssoError' toast.add({ From ebd25a94ea97c7864cb8cc1d5a1983de845fef2c Mon Sep 17 00:00:00 2001 From: Nikolai Giman Date: Thu, 20 Aug 2026 19:53:29 +0200 Subject: [PATCH 02/15] fix: sso email resolving and switcher for sso --- .../sso/__tests__/sso.utils.test.ts | 49 +++++++++++++++++++ .../tv-modules/sso/providers/saml.provider.ts | 12 ++--- api/src/tv-modules/sso/sso.utils.ts | 21 ++++++-- .../organizations/parts/OrgSsoConfigCard.vue | 36 ++++++++++++-- web/src/locales/de.ts | 2 + web/src/locales/en.ts | 2 + web/src/locales/es.ts | 2 + web/src/locales/ru.ts | 2 + 8 files changed, 113 insertions(+), 13 deletions(-) create mode 100644 api/src/tv-modules/sso/__tests__/sso.utils.test.ts 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/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/ui-customization/UiCustomizationOthers.vue b/web/src/components/features/ui-customization/UiCustomizationOthers.vue index c4cc179..2557dcc 100644 --- a/web/src/components/features/ui-customization/UiCustomizationOthers.vue +++ b/web/src/components/features/ui-customization/UiCustomizationOthers.vue @@ -38,7 +38,7 @@ v-model="defaultView" :items="viewItems" value-key="value" - :disabled="defaultProject === NONE" + data-testid="default-view-select" variant="soft" class="w-full lg:w-72" size="xl" @@ -70,7 +70,6 @@ const defaultProject = computed({ get: () => store.settings.defaultProjectId ?? NONE, set: (value) => { store.setSetting('defaultProjectId', value === NONE ? undefined : value) - if (value === NONE) store.setSetting('defaultView', undefined) }, }) diff --git a/web/src/components/sidebars/DashboardSidebarSecond.vue b/web/src/components/sidebars/DashboardSidebarSecond.vue index 565d1aa..1265d35 100644 --- a/web/src/components/sidebars/DashboardSidebarSecond.vue +++ b/web/src/components/sidebars/DashboardSidebarSecond.vue @@ -50,8 +50,8 @@ diff --git a/web/src/components/sidebars/dashboard-second/SidebarProjectSelect.vue b/web/src/components/sidebars/dashboard-second/SidebarProjectSelect.vue index 6503c5a..4f65b7e 100644 --- a/web/src/components/sidebars/dashboard-second/SidebarProjectSelect.vue +++ b/web/src/components/sidebars/dashboard-second/SidebarProjectSelect.vue @@ -150,9 +150,9 @@ import { computed, ref } from 'vue' import { useI18n } from 'vue-i18n' import { useRoute, useRouter } from 'vue-router' import { storeToRefs } from 'pinia' -import { ALL_TASKS_LIST_ID } from 'taskview-api' import { useGoalsStore } from '@/stores/goals.store' import { useGoalPermissionsFor } from '@/composables/useGoalPermissions' +import { useProjectRoute } from '@/composables/useProjectRoute' import type { Project, ProjectSaveData } from '@/components/features/projects/types' import ProjectEditModal from '@/components/features/projects/parts/ProjectEditModal.vue' import ProjectDeleteDialog from '@/components/features/projects/parts/ProjectDeleteDialog.vue' @@ -160,6 +160,7 @@ import ProjectDeleteDialog from '@/components/features/projects/parts/ProjectDel const { t } = useI18n() const route = useRoute() const router = useRouter() +const { projectRoute } = useProjectRoute() const goalsStore = useGoalsStore() const { goals } = storeToRefs(goalsStore) @@ -186,7 +187,7 @@ const { canEditGoal, canDeleteGoal } = useGoalPermissionsFor(currentProject) function selectProject(project: Project) { open.value = false - router.push({ name: 'user', params: { projectId: project.id, listId: ALL_TASKS_LIST_ID } }) + router.push(projectRoute(project)) } async function archive() { diff --git a/web/src/composables/useProjectRoute.ts b/web/src/composables/useProjectRoute.ts new file mode 100644 index 0000000..16ef649 --- /dev/null +++ b/web/src/composables/useProjectRoute.ts @@ -0,0 +1,37 @@ +import type { RouteLocationRaw } from 'vue-router' +import { ALL_TASKS_LIST_ID, type DefaultView, type GoalItem, type GoalPermissions } from 'taskview-api' +import { useUiPreferencesStore } from '@/stores/uiPreferences.store' +import { AllGoalPermissions } from '@/types/goals.types' + +export const VIEW_ROUTES: Record = { + tasks: 'user', + kanban: 'kanban', + graph: 'graph', + sprints: 'sprints', +} + +const VIEW_PERMISSIONS: Record, (keyof GoalPermissions)[]> = { + kanban: [AllGoalPermissions.KANBAN_CAN_VIEW, AllGoalPermissions.KANBAN_CAN_MANAGE], + graph: [AllGoalPermissions.GRAPH_CAN_VIEW, AllGoalPermissions.GRAPH_CAN_MANAGE], + sprints: [AllGoalPermissions.SPRINT_CAN_VIEW], +} + +function canOpenView(goal: GoalItem, view: DefaultView): boolean { + if (view === 'tasks') return true + return VIEW_PERMISSIONS[view].some((perm) => !!goal.permissions[perm]) +} + +export function useProjectRoute() { + const uiPrefs = useUiPreferencesStore() + + function projectRoute(goal: GoalItem, orgSlug?: string): RouteLocationRaw { + const preferred = uiPrefs.settings.defaultView ?? 'tasks' + const view: DefaultView = canOpenView(goal, preferred) ? preferred : 'tasks' + const params: Record = { projectId: goal.id } + if (orgSlug) params.orgSlug = orgSlug + if (view === 'tasks') params.listId = ALL_TASKS_LIST_ID + return { name: VIEW_ROUTES[view], params } + } + + return { projectRoute } +} diff --git a/web/src/locales/de.ts b/web/src/locales/de.ts index d28da1e..7deb0dc 100644 --- a/web/src/locales/de.ts +++ b/web/src/locales/de.ts @@ -810,7 +810,7 @@ export default { defaultProjectHint: 'Dieses Projekt direkt nach der Anmeldung öffnen.', defaultProjectNone: 'Startbildschirm (Standard)', defaultView: 'Standardansicht', - defaultViewHint: 'Welche Ansicht des Projekts geöffnet wird.', + defaultViewHint: 'Welche Ansicht beim Öffnen eines Projekts angezeigt wird.', viewTasks: 'Aufgaben', viewKanban: 'Kanban', viewGraph: 'Graph', diff --git a/web/src/locales/en.ts b/web/src/locales/en.ts index db3513a..6ab32bd 100644 --- a/web/src/locales/en.ts +++ b/web/src/locales/en.ts @@ -824,7 +824,7 @@ export default { defaultProjectHint: 'Open this project right after signing in.', defaultProjectNone: 'Home screen (default)', defaultView: 'Default view', - defaultViewHint: 'Which view of the default project to open.', + defaultViewHint: 'Which view to open when you enter a project.', viewTasks: 'Tasks', viewKanban: 'Kanban', viewGraph: 'Graph', diff --git a/web/src/locales/es.ts b/web/src/locales/es.ts index 8a90880..ddda0b6 100644 --- a/web/src/locales/es.ts +++ b/web/src/locales/es.ts @@ -810,7 +810,7 @@ export default { defaultProjectHint: 'Abrir este proyecto justo después de iniciar sesión.', defaultProjectNone: 'Pantalla de inicio (predeterminado)', defaultView: 'Vista predeterminada', - defaultViewHint: 'Qué vista del proyecto abrir.', + defaultViewHint: 'Qué vista abrir al entrar en un proyecto.', viewTasks: 'Tareas', viewKanban: 'Kanban', viewGraph: 'Grafo', diff --git a/web/src/locales/ru.ts b/web/src/locales/ru.ts index e45acda..d4c8049 100644 --- a/web/src/locales/ru.ts +++ b/web/src/locales/ru.ts @@ -797,7 +797,7 @@ export default { defaultProjectHint: 'Открывать этот проект сразу после входа.', defaultProjectNone: 'Главный экран (по умолчанию)', defaultView: 'Вид по умолчанию', - defaultViewHint: 'Какой вид проекта открывать.', + defaultViewHint: 'Какой вид открывать при переходе в проект.', viewTasks: 'Задачи', viewKanban: 'Канбан', viewGraph: 'Граф', From 5dc5b387de817cd4a4bb784ea4379641ea56c55b Mon Sep 17 00:00:00 2001 From: Nikolai Giman Date: Sat, 22 Aug 2026 16:43:48 +0200 Subject: [PATCH 07/15] fix: disable analytics --- api/src/tv-modules/analytics/sections/SectionRegistry.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/src/tv-modules/analytics/sections/SectionRegistry.ts b/api/src/tv-modules/analytics/sections/SectionRegistry.ts index d3b5fd4..0068250 100644 --- a/api/src/tv-modules/analytics/sections/SectionRegistry.ts +++ b/api/src/tv-modules/analytics/sections/SectionRegistry.ts @@ -11,8 +11,6 @@ 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 { AmountCoverageKpi } from './financial/AmountCoverageKpi' import { TotalIncomeKpi } from './financial/TotalIncomeKpi' import { TotalExpenseKpi } from './financial/TotalExpenseKpi' @@ -59,6 +57,8 @@ import { sectionLocales } from './locales' // 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' @@ -98,8 +98,8 @@ const builders: SectionBuilder[] = [ new IncomeExpensePerProjectSection(), new IncomePerProjectMonthSection(), new ExpensePerProjectMonthSection(), - new IncomePerTagMonthSection(), - new ExpensePerTagMonthSection(), + // new IncomePerTagMonthSection(), // disabled + // new ExpensePerTagMonthSection(), // disabled // new TopProjectsByAmountSection(), // disabled ] From 19e0212edf9434e58c3390ab035ea28fb5a8e3d6 Mon Sep 17 00:00:00 2001 From: Nikolai Giman Date: Sat, 22 Aug 2026 22:06:52 +0200 Subject: [PATCH 08/15] fix: add task on the main screen when no project is created --- web/package.json | 2 +- .../features/main/screen-main/parts/SearchActivator.vue | 6 ++---- web/src/composables/useTaskView.ts | 3 ++- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/web/package.json b/web/package.json index 6cd5d36..844430d 100644 --- a/web/package.json +++ b/web/package.json @@ -2,7 +2,7 @@ "name": "web-nuxt-ui", "private": true, "type": "module", - "version": "1.52.1", + "version": "1.52.2", "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/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/composables/useTaskView.ts b/web/src/composables/useTaskView.ts index 62f0dad..080eb45 100644 --- a/web/src/composables/useTaskView.ts +++ b/web/src/composables/useTaskView.ts @@ -13,7 +13,8 @@ export const useTaskView = () => { const isFullscreenModal = bp.smallerOrEqual('fullscreenModalMax') return { - hasActiveGoals: computed(() => goalsStore.goals.some(g => !g.isInbox)), + // Any non-archived project counts, including the Inbox — tasks can be added there too + hasActiveGoals: computed(() => goalsStore.goals.some(g => !g.archive)), isMobile, isDesktop, isFullscreenModal, From 9635120e663d72b07bc20f55c6a127515491bd62 Mon Sep 17 00:00:00 2001 From: Nikolai Giman Date: Sun, 23 Aug 2026 17:56:17 +0200 Subject: [PATCH 09/15] fix: add security md --- README.md | 2 +- SECURITY.md | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 SECURITY.md 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. From c1685e42b837490c45a1c1341ab131ff98d8b377 Mon Sep 17 00:00:00 2001 From: Nikolai Giman Date: Mon, 24 Aug 2026 14:21:46 +0200 Subject: [PATCH 10/15] feat: task dependencies in detailed panel --- api/src/tv-modules/graph/GraphControler.ts | 9 ++ api/src/tv-modules/graph/GraphManager.ts | 4 + api/src/tv-modules/graph/GraphRepository.ts | 12 +- api/src/tv-modules/graph/GraphRoutes.ts | 1 + .../graph/middlewares/resolveGoalId.ts | 8 ++ .../taskview-api/src/api/graph.ts | 8 ++ .../features/tasks/TaskDetailPanel.vue | 7 + .../tasks/parts/TaskDependencies.types.ts | 10 ++ .../features/tasks/parts/TaskDependencies.vue | 126 ++++++++++++++++++ .../tasks/parts/TaskDependencyGroup.vue | 84 ++++++++++++ .../tasks/parts/TaskDependencyPicker.vue | 121 +++++++++++++++++ web/src/composables/useTaskDetailPanel.ts | 12 +- web/src/locales/de.ts | 10 ++ web/src/locales/en.ts | 10 ++ web/src/locales/es.ts | 10 ++ web/src/locales/ru.ts | 10 ++ .../uiCustomization/sections/tasks.types.ts | 1 + 17 files changed, 438 insertions(+), 5 deletions(-) create mode 100644 web/src/components/features/tasks/parts/TaskDependencies.types.ts create mode 100644 web/src/components/features/tasks/parts/TaskDependencies.vue create mode 100644 web/src/components/features/tasks/parts/TaskDependencyGroup.vue create mode 100644 web/src/components/features/tasks/parts/TaskDependencyPicker.vue 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..e0f580b 100644 --- a/api/src/tv-modules/graph/middlewares/resolveGoalId.ts +++ b/api/src/tv-modules/graph/middlewares/resolveGoalId.ts @@ -15,6 +15,14 @@ export async function resolveGoalId(req: Request): Promise { return isNaN(id) ? null : id; } + if (req.params.taskId) { + const taskId = Number(req.params.taskId); + if (isNaN(taskId)) return null; + const tasksRepo = new TasksRepository(); + const task = await tasksRepo.fetchTaskByIdNew(taskId); + return task?.goalId ?? null; + } + // addEdge: resolve goalId from task if (req.body?.source) { const taskId = Number(req.body.source); 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/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/composables/useTaskDetailPanel.ts b/web/src/composables/useTaskDetailPanel.ts index e9b1666..b8ecdaf 100644 --- a/web/src/composables/useTaskDetailPanel.ts +++ b/web/src/composables/useTaskDetailPanel.ts @@ -46,10 +46,14 @@ const _useTaskDetailPanel = () => { const taskId = String(task.id) if (route.params.taskId !== taskId) { - router.push({ - name: 'user', - params: { projectId, listId, taskId }, - }) + // Switching from one open task to another (e.g. via dependencies) replaces + // the history entry, so back/close returns to the list, not the previous task + const target = { name: 'user', params: { projectId, listId, taskId } } + if (route.params.taskId) { + router.replace(target) + } else { + router.push(target) + } } } } diff --git a/web/src/locales/de.ts b/web/src/locales/de.ts index 7deb0dc..48e612d 100644 --- a/web/src/locales/de.ts +++ b/web/src/locales/de.ts @@ -535,6 +535,15 @@ export default { sprint: 'Sprint', estimate: 'Schätzung', }, + dependencies: { + title: 'Abhängigkeiten', + previous: 'Vorherige Aufgaben', + next: 'Nächste Aufgaben', + add: 'Hinzufügen', + searchPlaceholder: 'Aufgaben suchen…', + noResults: 'Nichts gefunden', + none: 'Keine', + }, status: 'Status', selectStatus: 'Status auswählen', searchStatuses: 'Status suchen...', @@ -827,6 +836,7 @@ export default { estimate: 'Schätzung', tags: 'Tags', deadline: 'Frist', + dependencies: 'Abhängigkeiten', amount: 'Betrag', timeTracking: 'Zeiterfassung', history: 'Verlauf', diff --git a/web/src/locales/en.ts b/web/src/locales/en.ts index 6ab32bd..38f948f 100644 --- a/web/src/locales/en.ts +++ b/web/src/locales/en.ts @@ -549,6 +549,15 @@ export default { sprint: 'Sprint', estimate: 'Estimate', }, + dependencies: { + title: 'Dependencies', + previous: 'Previous tasks', + next: 'Next tasks', + add: 'Add', + searchPlaceholder: 'Search tasks…', + noResults: 'Nothing found', + none: 'None', + }, status: 'Status', selectStatus: 'Select status', searchStatuses: 'Search statuses...', @@ -841,6 +850,7 @@ export default { estimate: 'Estimate', tags: 'Tags', deadline: 'Deadline', + dependencies: 'Dependencies', amount: 'Amount', timeTracking: 'Time tracking', history: 'History', diff --git a/web/src/locales/es.ts b/web/src/locales/es.ts index ddda0b6..83e0ea9 100644 --- a/web/src/locales/es.ts +++ b/web/src/locales/es.ts @@ -535,6 +535,15 @@ export default { sprint: 'Sprint', estimate: 'Estimación', }, + dependencies: { + title: 'Dependencias', + previous: 'Tareas anteriores', + next: 'Tareas siguientes', + add: 'Añadir', + searchPlaceholder: 'Buscar tareas…', + noResults: 'Sin resultados', + none: 'Ninguna', + }, status: 'Estado', selectStatus: 'Seleccionar estado', searchStatuses: 'Buscar estados...', @@ -827,6 +836,7 @@ export default { estimate: 'Estimación', tags: 'Etiquetas', deadline: 'Fecha límite', + dependencies: 'Dependencias', amount: 'Importe', timeTracking: 'Registro de tiempo', history: 'Historial', diff --git a/web/src/locales/ru.ts b/web/src/locales/ru.ts index d4c8049..0bd4d76 100644 --- a/web/src/locales/ru.ts +++ b/web/src/locales/ru.ts @@ -522,6 +522,15 @@ export default { sprint: 'Спринт', estimate: 'Оценка', }, + dependencies: { + title: 'Зависимости', + previous: 'Предыдущие задачи', + next: 'Следующие задачи', + add: 'Добавить', + searchPlaceholder: 'Поиск задач…', + noResults: 'Ничего не найдено', + none: 'Нет', + }, status: 'Статус', selectStatus: 'Выбрать статус', searchStatuses: 'Поиск статусов...', @@ -814,6 +823,7 @@ export default { estimate: 'Оценка', tags: 'Теги', deadline: 'Дедлайн', + dependencies: 'Зависимости', amount: 'Сумма', timeTracking: 'Учёт времени', history: 'История', diff --git a/web/src/uiCustomization/sections/tasks.types.ts b/web/src/uiCustomization/sections/tasks.types.ts index 6dd27ae..ab4c6a0 100644 --- a/web/src/uiCustomization/sections/tasks.types.ts +++ b/web/src/uiCustomization/sections/tasks.types.ts @@ -16,6 +16,7 @@ export const TASK_DETAIL_FIELDS = [ { id: 'estimate', width: 'narrow' }, { id: 'tags', width: 'narrow' }, { id: 'deadline', width: 'narrow' }, + { id: 'dependencies', width: 'wide' }, { id: 'amount', width: 'wide' }, { id: 'timeTracking', width: 'wide' }, { id: 'history', width: 'wide' }, From a596e023db3a287c218ecfadb4730819a3490384 Mon Sep 17 00:00:00 2001 From: Nikolai Giman Date: Mon, 24 Aug 2026 22:05:39 +0200 Subject: [PATCH 11/15] fix: performance issue in graph and update layout building --- web/package.json | 2 +- .../features/graph/ProjectGraph.vue | 24 +- .../components/features/graph/TaskNode.vue | 7 +- .../features/graph/composables/useLayout.ts | 208 +++++++++++------- web/src/pages/user/graph.vue | 4 +- web/src/stores/graph.store.ts | 2 +- 6 files changed, 154 insertions(+), 93 deletions(-) diff --git a/web/package.json b/web/package.json index 844430d..f5e008a 100644 --- a/web/package.json +++ b/web/package.json @@ -2,7 +2,7 @@ "name": "web-nuxt-ui", "private": true, "type": "module", - "version": "1.52.2", + "version": "1.52.7", "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/graph/ProjectGraph.vue b/web/src/components/features/graph/ProjectGraph.vue index 2d2910e..0013f5d 100644 --- a/web/src/components/features/graph/ProjectGraph.vue +++ b/web/src/components/features/graph/ProjectGraph.vue @@ -4,7 +4,7 @@ v-model:nodes="store.nodes" v-model:edges="store.edges" :min-zoom="-2" - fit-view-on-init + only-render-visible-elements elevate-edges-on-select elevate-nodes-on-select :pan-on-scroll-mode="PanOnScrollMode.Free" @@ -14,6 +14,7 @@ :nodes-connectable="canManageGraph" :nodes-draggable="canManageGraph" class="h-full w-full" + :class="{ 'opacity-0': !layoutReady }" @connect-start="onConnectStart" @connect-end="onConnectEnd" > @@ -118,7 +119,9 @@ const applyFilters = () => { const nodeIds = new Set(filtered.map((n) => n.id)) store.nodes = filtered store.edges = store.allEdges.filter((e) => nodeIds.has(e.source) && nodeIds.has(e.target)) - setTimeout(() => layoutGraph(layoutDirection.value), 50) + // Node sizes are estimated from data, so the layout can run right away — + // no need to wait for nodes to render and be measured + layoutGraph(layoutDirection.value) } watch(listIds, applyFilters, { deep: true }) @@ -132,8 +135,6 @@ const { onEdgesChange, onEdgeClick, onNodeDragStop, - getEdges, - updateEdgeData, removeEdges, screenToFlowCoordinate, } = useVueFlow() @@ -144,6 +145,8 @@ const store = useGraphStore() const { t } = useI18n() const { canManageGraph, canViewGraph } = useGoalPermissions() +const layoutReady = ref(false) + const addNewTaskToGraph = ref(false) const currentSession = ref(null) const successfulSession = ref(null) @@ -152,7 +155,7 @@ const nodePosition = ref<{ x: number; y: number } | undefined>(undefined) const defaultEdgeOptions: DefaultEdgeOptions = { type: 'smoothstep', - animated: true, + animated: false, style: { strokeWidth: 3, }, @@ -170,6 +173,7 @@ watch( projectId, (id) => { if (!id) return + layoutReady.value = false store.fetchAllTasksAndLists(id).then(() => { applyFilters() }) @@ -199,10 +203,15 @@ onConnect(async (params) => { addEdges([newEdge]) }) +function setAnimatedEdge(id: string | null) { + store.edges = store.edges.map((edge) => ({ ...edge, animated: edge.id === id })) +} + onEdgesChange((params) => { params.forEach((param) => { if (param.type === 'select' && !param.selected) { selectedEdge.value = null + setAnimatedEdge(null) } if (param.type === 'remove') { deleteSelectedEdge(+param.id) @@ -213,6 +222,7 @@ onEdgesChange((params) => { onEdgeClick((params) => { selectedEdge.value = params.edge + setAnimatedEdge(params.edge.id) }) const newToken = () => { @@ -285,9 +295,7 @@ const layoutGraph = async (direction: 'LR' | 'TB') => { store.nodes = layout(store.nodes, store.edges, direction) nextTick(() => { fitView() - getEdges.value.forEach((edge) => { - updateEdgeData(edge.id, edge) - }) + layoutReady.value = true }) } diff --git a/web/src/components/features/graph/TaskNode.vue b/web/src/components/features/graph/TaskNode.vue index 1591713..9cad17b 100644 --- a/web/src/components/features/graph/TaskNode.vue +++ b/web/src/components/features/graph/TaskNode.vue @@ -27,10 +27,10 @@ :style="sourceHandleStyle" /> -
@@ -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/pages/user/graph.vue b/web/src/pages/user/graph.vue index 6113eb4..8c68dbd 100644 --- a/web/src/pages/user/graph.vue +++ b/web/src/pages/user/graph.vue @@ -29,9 +29,9 @@
-
+
({ @@ -41,7 +42,6 @@ export const useGraphStore = defineStore('use-graph-store', { })) this.allNodes = nodes this.nodes = [...nodes] - await this.fetchAllEdges(goalId) }, async addNode(task: TaskItem) { From e11aba4d79779be3ed375f490bd374a30c21e619 Mon Sep 17 00:00:00 2001 From: Nikolai Giman Date: Wed, 26 Aug 2026 23:20:16 +0200 Subject: [PATCH 12/15] fix: time tracking edit --- .../features/tasks/parts/TaskTimeTrackingForm.vue | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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)) From a4c465469dd881c8e0e722758b5c7e587d73aefb Mon Sep 17 00:00:00 2001 From: Nikolai Giman Date: Wed, 26 Aug 2026 23:41:46 +0200 Subject: [PATCH 13/15] wip: updated locales --- .../features/settings/composables/useSettingsHub.ts | 2 +- web/src/locales/index.ts | 6 +++--- web/src/locales/{ptBR.ts => pt-br.ts} | 10 ++++++++++ 3 files changed, 14 insertions(+), 4 deletions(-) rename web/src/locales/{ptBR.ts => pt-br.ts} (99%) 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/locales/index.ts b/web/src/locales/index.ts index a647279..964c314 100644 --- a/web/src/locales/index.ts +++ b/web/src/locales/index.ts @@ -1,17 +1,17 @@ import de from './de' import en from './en' import es from './es' +import ptBr from './pt-br' import ru from './ru' -import ptBR from './ptBR' export const messages = { en, ru, de, es, - ptBR + 'pt-BR': ptBr, } export type Locale = keyof typeof messages -export const locales: Locale[] = ['en', 'ru', 'de', 'es', 'ptBR'] +export const locales: Locale[] = ['en', 'ru', 'de', 'es', 'pt-BR'] export const defaultLocale: Locale = 'en' diff --git a/web/src/locales/ptBR.ts b/web/src/locales/pt-br.ts similarity index 99% rename from web/src/locales/ptBR.ts rename to web/src/locales/pt-br.ts index b5fbccb..daf6438 100644 --- a/web/src/locales/ptBR.ts +++ b/web/src/locales/pt-br.ts @@ -548,6 +548,15 @@ export default { sprint: 'Sprint', estimate: 'Estimativa', }, + dependencies: { + title: 'Dependências', + previous: 'Tarefas anteriores', + next: 'Próximas tarefas', + add: 'Adicionar', + searchPlaceholder: 'Buscar tarefas…', + noResults: 'Nada encontrado', + none: 'Nenhuma', + }, status: 'Status', selectStatus: 'Selecionar status', searchStatuses: 'Pesquisar status...', @@ -840,6 +849,7 @@ export default { estimate: 'Estimativa', tags: 'Tags', deadline: 'Prazo', + dependencies: 'Dependências', amount: 'Valor', timeTracking: 'Controle de tempo', history: 'História', From f79614ebc5f0789bc991f0357479ad58aa32f095 Mon Sep 17 00:00:00 2001 From: Nikolai Giman Date: Thu, 27 Aug 2026 20:29:54 +0200 Subject: [PATCH 14/15] fix(security): enforce collaboration access permissions --- ...FetchRolesPermissionsCollaborationRoles.ts | 3 +- .../collaboration/CollaborationRoutes.ts | 8 +- .../middlewares/CanFetchUsersCollaboration.ts | 3 +- .../graph/middlewares/resolveGoalId.ts | 60 ++-- .../middlewares/resolveProjectId.ts | 30 +- api/src/tv-modules/kanban/KanbanRoutes.ts | 95 +++++- .../kanban/middlewares/CanFetchTasks.ts | 33 -- .../kanban/middlewares/CanManageKanban.ts | 54 ---- .../kanban/middlewares/CanViewKanban.ts | 33 -- .../kanban/middlewares/goal-id-resolvers.ts | 19 ++ .../middlewares/require-kanban-permission.ts | 24 ++ api/src/tv-modules/kanban/types.ts | 12 +- api/src/tv-modules/tasks/TasksRoutes.ts | 11 - .../tasks/middlewares/CanAddTask.ts | 39 --- .../tasks/middlewares/CanFetchSubtasks.ts | 28 -- .../tasks/middlewares/CanFetchTask.ts | 2 +- .../tasks/middlewares/CanMoveTask.ts | 28 -- .../middlewares/CanSeeTaskAssignedUsers.ts | 28 -- .../middlewares/CanUpdateTaskAssignee.ts | 29 -- .../middlewares/CanUpdateTaskDeadline.ts | 28 -- .../middlewares/CanUpdateTaskDescription.ts | 28 -- .../tasks/middlewares/CanUpdateTaskNote.ts | 28 -- .../middlewares/CanUpdateTaskPriority.ts | 28 -- .../tasks/middlewares/CanUpdateTaskStatus.ts | 28 -- .../collaboration-goal-access.test.ts | 300 ++++++++++++++++++ .../src/api/__tests__/graph-access.test.ts | 161 ++++++++++ .../__tests__/guard-param-confusion.test.ts | 113 +++++++ .../api/__tests__/integrations-access.test.ts | 236 ++++++++++++++ .../src/api/__tests__/kanban.access.test.ts | 220 +++++++++++++ .../src/api/__tests__/tasks-access.test.ts | 80 +++++ .../collaboration/CollaborationPanel.vue | 4 + .../parts/members/MemberAddInput.vue | 2 + .../parts/members/MemberEditModal.vue | 2 + .../parts/members/MemberItem.vue | 6 +- .../parts/members/MembersList.vue | 2 + .../parts/roles/PermissionsEditor.vue | 2 + .../parts/roles/RoleAddInput.vue | 2 + .../parts/roles/RoleDeleteDialog.vue | 1 + .../collaboration/parts/roles/RoleItem.vue | 6 +- .../collaboration/parts/roles/RolesList.vue | 2 + .../features/kanban/KanbanBoard.vue | 2 + .../features/kanban/parts/KanbanAddStatus.vue | 2 + .../kanban/parts/KanbanDeleteModal.vue | 1 + .../features/kanban/parts/KanbanEditModal.vue | 2 + .../features/kanban/parts/KanbanTitleMenu.vue | 3 + .../tasks/parts/TasksFilterDrawer.vue | 7 +- .../features/tasks/parts/TasksToolbar.vue | 1 + web/src/composables/useProjectDataLoader.ts | 2 +- 48 files changed, 1355 insertions(+), 483 deletions(-) delete mode 100644 api/src/tv-modules/kanban/middlewares/CanFetchTasks.ts delete mode 100644 api/src/tv-modules/kanban/middlewares/CanManageKanban.ts delete mode 100644 api/src/tv-modules/kanban/middlewares/CanViewKanban.ts create mode 100644 api/src/tv-modules/kanban/middlewares/goal-id-resolvers.ts create mode 100644 api/src/tv-modules/kanban/middlewares/require-kanban-permission.ts delete mode 100644 api/src/tv-modules/tasks/middlewares/CanAddTask.ts delete mode 100644 api/src/tv-modules/tasks/middlewares/CanFetchSubtasks.ts delete mode 100644 api/src/tv-modules/tasks/middlewares/CanMoveTask.ts delete mode 100644 api/src/tv-modules/tasks/middlewares/CanSeeTaskAssignedUsers.ts delete mode 100644 api/src/tv-modules/tasks/middlewares/CanUpdateTaskAssignee.ts delete mode 100644 api/src/tv-modules/tasks/middlewares/CanUpdateTaskDeadline.ts delete mode 100644 api/src/tv-modules/tasks/middlewares/CanUpdateTaskDescription.ts delete mode 100644 api/src/tv-modules/tasks/middlewares/CanUpdateTaskNote.ts delete mode 100644 api/src/tv-modules/tasks/middlewares/CanUpdateTaskPriority.ts delete mode 100644 api/src/tv-modules/tasks/middlewares/CanUpdateTaskStatus.ts create mode 100644 taskview-packages/taskview-api/src/api/__tests__/collaboration-goal-access.test.ts create mode 100644 taskview-packages/taskview-api/src/api/__tests__/graph-access.test.ts create mode 100644 taskview-packages/taskview-api/src/api/__tests__/guard-param-confusion.test.ts create mode 100644 taskview-packages/taskview-api/src/api/__tests__/integrations-access.test.ts create mode 100644 taskview-packages/taskview-api/src/api/__tests__/kanban.access.test.ts create mode 100644 taskview-packages/taskview-api/src/api/__tests__/tasks-access.test.ts 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/middlewares/resolveGoalId.ts b/api/src/tv-modules/graph/middlewares/resolveGoalId.ts index e0f580b..40ad31f 100644 --- a/api/src/tv-modules/graph/middlewares/resolveGoalId.ts +++ b/api/src/tv-modules/graph/middlewares/resolveGoalId.ts @@ -3,36 +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; } + // fetchTaskEdges: GET /task/:taskId if (req.params.taskId) { - const taskId = Number(req.params.taskId); - if (isNaN(taskId)) return null; - const tasksRepo = new TasksRepository(); - const task = await tasksRepo.fetchTaskByIdNew(taskId); - return task?.goalId ?? null; + return goalIdForTask(req.params.taskId); } - // 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; - } - - // deleteEdge: resolve goalId from edge + // deleteEdge: DELETE /:id if (req.params.id) { const edgeId = Number(req.params.id); if (isNaN(edgeId)) return null; @@ -41,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/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/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/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' }" > + +