From a00e956e73fa4a8a0bcbbf2db01c98a787f0c559 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Wed, 8 Apr 2026 22:49:15 +0200 Subject: [PATCH 1/3] fix(tasks): require JWT + org scope on stats endpoint The /api/tasks/stats endpoint was publicly accessible without authentication and returned a global count across all organizations. Align with other task endpoints by requiring JWT auth, resolving the organization context, and scoping the count query by organizationId. Closes #3430 --- MIGRATIONS.md | 19 ++++++++++++++++ modules/tasks/controllers/tasks.controller.js | 8 +++---- .../tasks/repositories/tasks.repository.js | 7 +++--- modules/tasks/routes/tasks.routes.js | 7 ++++-- modules/tasks/services/tasks.service.js | 8 +++++-- .../tasks/tests/tasks.integration.tests.js | 22 ++++++++++++++----- 6 files changed, 54 insertions(+), 17 deletions(-) diff --git a/MIGRATIONS.md b/MIGRATIONS.md index 386a31772..2020b59c1 100644 --- a/MIGRATIONS.md +++ b/MIGRATIONS.md @@ -20,6 +20,25 @@ Rate-limit middleware now keys authenticated requests by `user._id` (with `req.i --- +## Tasks stats endpoint requires JWT + org scope (2026-04-08) + +`GET /api/tasks/stats` now requires authentication and organization context, consistent with all other task endpoints. + +### What changed + +- `modules/tasks/routes/tasks.routes.js` — added JWT + `resolveOrganization` + `isAllowed` middleware +- `modules/tasks/controllers/tasks.controller.js` — passes `req.organization` to service, uses try/catch +- `modules/tasks/services/tasks.service.js` — `stats()` accepts organization and filters by `organizationId` +- `modules/tasks/repositories/tasks.repository.js` — `stats()` uses `countDocuments(filter)` instead of `estimatedDocumentCount()` + +### Action for downstream + +1. Any unauthenticated call to `/api/tasks/stats` will now return `401` +2. Authenticated calls return the count scoped to the user's current organization +3. Run `/update-stack` to pull the change + +--- + ## Remove dead scripts — ci/ssl, crons, db/dump (2026-04-07) Dead scripts and dev-local data removed from the stack. Downstream projects may have local copies or npm scripts referencing these. diff --git a/modules/tasks/controllers/tasks.controller.js b/modules/tasks/controllers/tasks.controller.js index a47850fd1..8c862ea73 100644 --- a/modules/tasks/controllers/tasks.controller.js +++ b/modules/tasks/controllers/tasks.controller.js @@ -90,11 +90,11 @@ const remove = async (req, res) => { * @throws Will throw an error if the task service fails to fetch the statistics */ const stats = async (req, res) => { - const data = await TasksService.stats(); - if (!data.err) { + try { + const data = await TasksService.stats(req.organization); responses.success(res, 'tasks stats')(data); - } else { - responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(data.err))(data.err); + } catch (err) { + responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err); } }; diff --git a/modules/tasks/repositories/tasks.repository.js b/modules/tasks/repositories/tasks.repository.js index da7c9ac81..86f458a4a 100644 --- a/modules/tasks/repositories/tasks.repository.js +++ b/modules/tasks/repositories/tasks.repository.js @@ -67,10 +67,11 @@ const deleteMany = (filter) => { /** * @function stats - * @description Data access operation to get the estimated count of documents in the tasks collection. - * @returns {Promise} estimated document count + * @description Data access operation to count tasks matching the given filter. + * @param {Object} [filter={}] - Optional filter (e.g. { organizationId }). + * @returns {Promise} document count */ -const stats = () => Task.estimatedDocumentCount().exec(); +const stats = (filter = {}) => Task.countDocuments(filter).exec(); /** * @function push diff --git a/modules/tasks/routes/tasks.routes.js b/modules/tasks/routes/tasks.routes.js index f29b37bd6..a6bc571fe 100644 --- a/modules/tasks/routes/tasks.routes.js +++ b/modules/tasks/routes/tasks.routes.js @@ -13,8 +13,11 @@ import tasksSchema from '../models/tasks.schema.js'; * Routes */ export default (app) => { - // stats — public aggregate endpoint, no auth required - app.route('/api/tasks/stats').get(tasks.stats); + // stats — org-scoped aggregate endpoint + app + .route('/api/tasks/stats') + .all(passport.authenticate('jwt', { session: false }), organization.resolveOrganization, policy.isAllowed) + .get(tasks.stats); // list & post app diff --git a/modules/tasks/services/tasks.service.js b/modules/tasks/services/tasks.service.js index 663ce096d..fddeca5d2 100644 --- a/modules/tasks/services/tasks.service.js +++ b/modules/tasks/services/tasks.service.js @@ -81,10 +81,14 @@ const remove = async (task) => { /** * @function stats * @description Service to fetch statistical data about tasks in the database. + * When an organization context is provided, only tasks belonging to that + * organization are counted. + * @param {Object} [organization] - Optional organization document whose _id is used to filter. * @returns {Promise} A promise resolving to the statistical data. */ -const stats = async () => { - const result = await TasksRepository.stats(); +const stats = async (organization) => { + const filter = organization ? { organizationId: organization._id } : {}; + const result = await TasksRepository.stats(filter); return result; }; diff --git a/modules/tasks/tests/tasks.integration.tests.js b/modules/tasks/tests/tasks.integration.tests.js index 3f9dfd8c4..ecb10c3e5 100644 --- a/modules/tasks/tests/tasks.integration.tests.js +++ b/modules/tasks/tests/tasks.integration.tests.js @@ -253,6 +253,18 @@ describe('Tasks integration tests:', () => { } }); + test('should be able to get tasks stats when authenticated', async () => { + try { + const result = await agent.get('/api/tasks/stats').expect(200); + expect(result.body.type).toBe('success'); + expect(result.body.message).toBe('tasks stats'); + expect(typeof result.body.data).toBe('number'); + } catch (err) { + console.log(err); + expect(err).toBeFalsy(); + } + }); + afterEach(async () => { // del task try { @@ -291,11 +303,9 @@ describe('Tasks integration tests:', () => { } }); - test('should be able to get a tasks stats', async () => { + test('should not be able to get tasks stats without auth', async () => { try { - const result = await agent.get('/api/tasks/stats').expect(200); - expect(result.body.type).toBe('success'); - expect(result.body.message).toBe('tasks stats'); + await agent.get('/api/tasks/stats').expect(401); } catch (err) { expect(err).toBeFalsy(); console.log(err); @@ -442,8 +452,8 @@ describe('Tasks integration tests:', () => { expect(result.body.description).toBe('DB error.'); }); - test('should return 422 when stats returns an error', async () => { - jest.spyOn(TasksService, 'stats').mockResolvedValueOnce({ err: new Error('DB error') }); + test('should return 422 when stats fails', async () => { + jest.spyOn(TasksService, 'stats').mockRejectedValueOnce(new Error('DB error')); const result = await agent.get('/api/tasks/stats').expect(422); expect(result.body.type).toBe('error'); expect(result.body.message).toBe('Unprocessable Entity'); From 3681804f08cbffe57e866676a4c3c983c77ade4d Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Wed, 8 Apr 2026 22:59:28 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix(auth):=20update=20authorization=20test?= =?UTF-8?q?=20=E2=80=94=20tasks/stats=20now=20requires=20JWT?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stats endpoint was changed from public to authenticated, so the guest-access test must expect 401 instead of 200. --- modules/auth/tests/auth.authorization.integration.tests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/auth/tests/auth.authorization.integration.tests.js b/modules/auth/tests/auth.authorization.integration.tests.js index c9758666b..307d36f3e 100644 --- a/modules/auth/tests/auth.authorization.integration.tests.js +++ b/modules/auth/tests/auth.authorization.integration.tests.js @@ -153,8 +153,8 @@ describe('Authorization integration tests:', () => { await publicAgent.get('/api/tasks').expect(401); }); - test('GET /api/tasks/stats should return 200 for guests', async () => { - await publicAgent.get('/api/tasks/stats').expect(200); + test('GET /api/tasks/stats should return 401 for guests (auth required)', async () => { + await publicAgent.get('/api/tasks/stats').expect(401); }); test('GET /api/users/stats should return 200 for guests', async () => { From f06dfc677538bb68bd98c4180a655b35858a486b Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Wed, 8 Apr 2026 23:08:06 +0200 Subject: [PATCH 3/3] =?UTF-8?q?fix(tasks):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20org-scoped=20stats=20assertion,=20fresh=20agent=20for=20logo?= =?UTF-8?q?ut,=20fix=20markdown?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- MIGRATIONS.md | 1 - .../tasks/tests/tasks.integration.tests.js | 19 +++++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/MIGRATIONS.md b/MIGRATIONS.md index 2020b59c1..ac6ed1256 100644 --- a/MIGRATIONS.md +++ b/MIGRATIONS.md @@ -32,7 +32,6 @@ Rate-limit middleware now keys authenticated requests by `user._id` (with `req.i - `modules/tasks/repositories/tasks.repository.js` — `stats()` uses `countDocuments(filter)` instead of `estimatedDocumentCount()` ### Action for downstream - 1. Any unauthenticated call to `/api/tasks/stats` will now return `401` 2. Authenticated calls return the count scoped to the user's current organization 3. Run `/update-stack` to pull the change diff --git a/modules/tasks/tests/tasks.integration.tests.js b/modules/tasks/tests/tasks.integration.tests.js index ecb10c3e5..5a267529d 100644 --- a/modules/tasks/tests/tasks.integration.tests.js +++ b/modules/tasks/tests/tasks.integration.tests.js @@ -17,6 +17,7 @@ describe('Tasks integration tests:', () => { const originalOrgEnabled = config.organizations.enabled; let TasksService; let TasksDataService; + let app; // Express app instance for fresh (unauthenticated) requests let agent; let user; let _user; @@ -33,7 +34,8 @@ describe('Tasks integration tests:', () => { UserService = (await import(path.resolve('./modules/users/services/users.service.js'))).default; TasksService = (await import(path.resolve('./modules/tasks/services/tasks.service.js'))).default; TasksDataService = (await import(path.resolve('./modules/tasks/services/tasks.data.service.js'))).default; - agent = request.agent(init.app); + app = init.app; + agent = request.agent(app); } catch (err) { console.log(err); expect(err).toBeFalsy(); @@ -253,12 +255,18 @@ describe('Tasks integration tests:', () => { } }); - test('should be able to get tasks stats when authenticated', async () => { + test('should be able to get tasks stats when authenticated (org-scoped)', async () => { try { + const tasksResult = await agent.get('/api/tasks').expect(200); + expect(tasksResult.body.type).toBe('success'); + expect(tasksResult.body.message).toBe('task list'); + expect(tasksResult.body.data).toBeInstanceOf(Array); + const result = await agent.get('/api/tasks/stats').expect(200); expect(result.body.type).toBe('success'); expect(result.body.message).toBe('tasks stats'); expect(typeof result.body.data).toBe('number'); + expect(result.body.data).toBe(tasksResult.body.data.length); } catch (err) { console.log(err); expect(err).toBeFalsy(); @@ -285,7 +293,7 @@ describe('Tasks integration tests:', () => { describe('Logout', () => { test('should not be able to save a task', async () => { try { - const result = await agent.post('/api/tasks').send(_tasks[0]).expect(401); + const result = await request(app).post('/api/tasks').send(_tasks[0]).expect(401); expect(result.error.text).toBe('Unauthorized'); } catch (err) { expect(err).toBeFalsy(); @@ -294,9 +302,8 @@ describe('Tasks integration tests:', () => { }); test('should not be able to get list of tasks without auth', async () => { - // task list now requires authentication try { - await agent.get('/api/tasks').expect(401); + await request(app).get('/api/tasks').expect(401); } catch (err) { console.log(err); expect(err).toBeFalsy(); @@ -305,7 +312,7 @@ describe('Tasks integration tests:', () => { test('should not be able to get tasks stats without auth', async () => { try { - await agent.get('/api/tasks/stats').expect(401); + await request(app).get('/api/tasks/stats').expect(401); } catch (err) { expect(err).toBeFalsy(); console.log(err);