From 7472d2405d6290b10089302c5062f6571679eedc Mon Sep 17 00:00:00 2001 From: Olu <142989683+olusegz07@users.noreply.github.com> Date: Wed, 11 Mar 2026 12:26:39 +0000 Subject: [PATCH 1/3] fix: case activity information disclosure issue --- app/routes/get-activities.js | 3 +- app/service/activity-service.js | 82 ++++++++++++++++++- charts/ccd-case-activity-api/values.yaml | 1 + config/custom-environment-variables.yaml | 2 + test/spec/app/route/get-activities.spec.js | 11 ++- .../spec/app/service/activity-service.spec.js | 58 ++++++++++--- 6 files changed, 138 insertions(+), 19 deletions(-) diff --git a/app/routes/get-activities.js b/app/routes/get-activities.js index c195c210..13893253 100644 --- a/app/routes/get-activities.js +++ b/app/routes/get-activities.js @@ -6,9 +6,10 @@ const { ifNotTimedOut } = utils; const getActivities = (activityService) => (req, res, next) => { const caseIds = req.params.caseids.split(','); const { user } = req.authentication; + const bearerToken = req.get('Authorization'); debug(`GET_ACTIVITIES request for caseIds: ${caseIds}`); - activityService.getActivities(caseIds, user) + activityService.getActivities(caseIds, user, bearerToken) .then((result) => ifNotTimedOut(req, () => { debug(`GET_ACTIVITIES response is ==> ${JSON.stringify(result)}`); res.status(200).json(result); diff --git a/app/service/activity-service.js b/app/service/activity-service.js index baa40b6d..76fc29a8 100644 --- a/app/service/activity-service.js +++ b/app/service/activity-service.js @@ -1,5 +1,7 @@ const moment = require('moment'); const debug = require('debug')('ccd-case-activity-api:activity-service'); +const fetch = require('../util/fetch'); +const jwtUtil = require('../util/jwt'); module.exports = (config, redis, ttlScoreGenerator) => { const redisActivityKeys = { @@ -7,6 +9,60 @@ module.exports = (config, redis, ttlScoreGenerator) => { edit: (caseId) => `case:${caseId}:editors`, }; + const ACCESS_PROCESS_ID = '[ACCESS_PROCESS]'; + const ACCESS_GRANTED_ID = '[ACCESS_GRANTED]'; + const BASIC_ACCESS = 'BASIC'; + const NON_STANDARD_ACCESS_TYPES = ['CHALLENGED', 'SPECIFIC']; + const CASE_VIEW_ACCEPT = 'application/vnd.uk.gov.hmcts.ccd-data-store-api.ui-case-view.v2+json'; + + const buildEmptyCaseStatus = (caseId) => ({ + caseId, + viewers: [], + unknownViewers: 0, + editors: [], + unknownEditors: 0, + }); + + const isStandardAccess = (caseView) => { + if (!caseView || !Array.isArray(caseView.metadataFields)) { + return true; + } + + const accessProcess = caseView.metadataFields + .find((metadataField) => metadataField.id === ACCESS_PROCESS_ID); + const accessGranted = caseView.metadataFields + .find((metadataField) => metadataField.id === ACCESS_GRANTED_ID); + + const accessGrantedValue = accessGranted ? accessGranted.value : null; + const accessGrantedBool = accessGrantedValue ? accessGrantedValue !== BASIC_ACCESS : false; + const accessProcessValue = accessProcess ? accessProcess.value : null; + + return accessGrantedBool || NON_STANDARD_ACCESS_TYPES.indexOf(accessProcessValue) === -1; + }; + + const getCaseView = (caseId, bearerToken) => { + const baseUrl = config.get('ccd.url'); + const url = `${baseUrl}/internal/cases/${caseId}`; + return fetch(url, { + headers: { + Authorization: jwtUtil.addBearer(bearerToken), + Accept: CASE_VIEW_ACCEPT, + }, + }).then((res) => res.json()); + }; + + const hasStandardAccess = (caseId, bearerToken) => { + if (!bearerToken) { + return Promise.resolve(false); + } + return getCaseView(caseId, bearerToken) + .then((caseView) => isStandardAccess(caseView)) + .catch((error) => { + debug(`Access check failed for caseId ${caseId}: ${error && error.message ? error.message : error}`); + return false; + }); + }; + const addActivity = (caseId, user, activity) => { const storeUserActivity = () => { const key = redisActivityKeys[activity](caseId); @@ -27,7 +83,7 @@ module.exports = (config, redis, ttlScoreGenerator) => { ]).exec(); }; - const getActivities = (caseIds, user) => { + const getActivitiesForCaseIds = (caseIds, user) => { const uniqueUserIds = []; let caseViewers = []; let caseEditors = []; @@ -83,5 +139,29 @@ module.exports = (config, redis, ttlScoreGenerator) => { return caseStatus; })); }; + + const getActivities = (caseIds, user, bearerToken) => { + if (!caseIds || caseIds.length === 0) { + return Promise.resolve([]); + } + + const accessChecks = caseIds.map((caseId) => hasStandardAccess(caseId, bearerToken) + .then((allowed) => ({ caseId, allowed }))); + + return Promise.all(accessChecks) + .then((accessResults) => { + const allowedCaseIds = accessResults + .filter((result) => result.allowed) + .map((result) => result.caseId); + if (!allowedCaseIds.length) { + return caseIds.map((caseId) => buildEmptyCaseStatus(caseId)); + } + return getActivitiesForCaseIds(allowedCaseIds, user) + .then((allowedResults) => { + const allowedMap = new Map(allowedResults.map((item) => [item.caseId, item])); + return caseIds.map((caseId) => allowedMap.get(caseId) || buildEmptyCaseStatus(caseId)); + }); + }); + }; return { addActivity, getActivities }; }; diff --git a/charts/ccd-case-activity-api/values.yaml b/charts/ccd-case-activity-api/values.yaml index c3cd72c1..44faad9e 100644 --- a/charts/ccd-case-activity-api/values.yaml +++ b/charts/ccd-case-activity-api/values.yaml @@ -26,6 +26,7 @@ nodejs: AUTH_WHITE_LIST: ^caseworker-.+ AUTH_BLACK_LIST: solicitor IDAM_BASE_URL: https://idam-api.{{ .Values.global.environment }}.platform.hmcts.net + SERVICES_CCD_COMPONENT_API: https://gateway-ccd.{{ .Values.global.environment }}.platform.hmcts.net REDIS_HOST: ccd-activity-service-{{ .Values.global.environment }}.redis.cache.windows.net REDIS_PORT: 6380 REDIS_SSL_ENABLED: true diff --git a/config/custom-environment-variables.yaml b/config/custom-environment-variables.yaml index cb5c8a00..8650b869 100644 --- a/config/custom-environment-variables.yaml +++ b/config/custom-environment-variables.yaml @@ -5,6 +5,8 @@ security: auth_blacklist: AUTH_BLACK_LIST idam: base_url: IDAM_BASE_URL +ccd: + url: SERVICES_CCD_COMPONENT_API redis: host: REDIS_HOST port: REDIS_PORT diff --git a/test/spec/app/route/get-activities.spec.js b/test/spec/app/route/get-activities.spec.js index c4a50c98..70ad5db5 100644 --- a/test/spec/app/route/get-activities.spec.js +++ b/test/spec/app/route/get-activities.spec.js @@ -26,7 +26,8 @@ describe("get activities route", () => { let req = { params: { caseids: '111, 121' }, authentication: { user: { id: 900 } }, - timedout: false + timedout: false, + get: (header) => (header === 'Authorization' ? 'Bearer token' : undefined) }; let next = () => { }; @@ -42,7 +43,7 @@ describe("get activities route", () => { getActivitesRoute(req, res, next) - expect(activityService.getActivities).to.have.been.calledWith(req.params.caseids.split(','), { id : 900 }) + expect(activityService.getActivities).to.have.been.calledWith(req.params.caseids.split(','), { id : 900 }, 'Bearer token') }) it("should not return a result when request is successful after it has timed out", (done) => { @@ -52,7 +53,8 @@ describe("get activities route", () => { caseids: '111,121' }, authentication: { user: { id: 900 } }, - timedout: true + timedout: true, + get: (header) => (header === 'Authorization' ? 'Bearer token' : undefined) }; let next = () => { @@ -79,7 +81,8 @@ describe("get activities route", () => { caseids: '111,121' }, authentication: { user: { id: 900 } }, - timedout: true + timedout: true, + get: (header) => (header === 'Authorization' ? 'Bearer token' : undefined) }; var res = buildResponse() diff --git a/test/spec/app/service/activity-service.spec.js b/test/spec/app/service/activity-service.spec.js index 6ef91266..de91346b 100644 --- a/test/spec/app/service/activity-service.spec.js +++ b/test/spec/app/service/activity-service.spec.js @@ -1,16 +1,19 @@ var redis = require('../../../../app/redis/redis-client'); var config = require('config'); var ttlScoreGenerator = require('../../../../app/service/ttl-score-generator'); -var activityService = require('../../../../app/service/activity-service')(config, redis, ttlScoreGenerator); var moment = require('moment'); var chai = require("chai"); var sinon = require("sinon"); var sinonChai = require("sinon-chai"); +var proxyquire = require('proxyquire'); chai.should(); var expect = chai.expect; chai.use(sinonChai); var sandbox = sinon.createSandbox(); +var activityService; +var fetchStub; + describe("activity service", () => { afterEach(function () { @@ -18,6 +21,19 @@ describe("activity service", () => { sandbox.restore(); }); + beforeEach(function () { + fetchStub = sandbox.stub().resolves({ + json: () => Promise.resolve({ + metadataFields: [{ id: '[ACCESS_PROCESS]', value: 'NONE' }] + }) + }); + + activityService = proxyquire('../../../../app/service/activity-service', { + '../util/fetch': fetchStub, + '../util/jwt': { addBearer: (jwt) => jwt } + })(config, redis, ttlScoreGenerator); + }); + const CASE_ID = 55; const USER_ID = '67'; const SCORE = 30; @@ -55,19 +71,23 @@ describe("activity service", () => { it("getActivities should create a redis pipeline with the correct redis commands for getViewers", (done) => { sandbox.stub(moment, 'now').returns(TIMESTAMP); - sandbox.stub(config, 'get').returns(USER_DETAILS_TTL); + sandbox.stub(config, 'get').callsFake((key) => { + if (key === 'redis.userDetailsTtlSec') return USER_DETAILS_TTL; + if (key === 'caseData.base_url') return 'http://case-data'; + return USER_DETAILS_TTL; + }); sandbox.stub(redis, "pipeline").callsFake(function (arguments) { argStr = JSON.stringify(arguments); if (argStr.includes('zrangebyscore')) { pipStub.exec = () => Promise.resolve([[null, [242]], [null, [12]]]); return pipStub; } else { - pipStub.exec = () => Promise.resolve([[null, "{\"forename\":\"nayab\",\"surname\":\"gul\"}"], [null, "{\"forename\":\"sam\",\"surname\":\"gamgee\"}"]]); + pipStub.exec = () => Promise.resolve([[null, '{"forename":"nayab","surname":"gul"}'], [null, '{"forename":"sam","surname":"gamgee"}']]); return pipStub; } }); - const result = activityService.getActivities(['767', '888'], { uid: '900' }); + const result = activityService.getActivities(['767', '888'], { uid: '900' }, 'Bearer token'); result.then((content) => { expect(redis.pipeline).to.have.been.calledWith([['zrangebyscore', 'case:767:viewers', TIMESTAMP, '+inf'], ['zrangebyscore', 'case:888:viewers', TIMESTAMP, '+inf']]); @@ -92,19 +112,23 @@ describe("activity service", () => { it("getActivities should return unknown users if users detail are missing", (done) => { sandbox.stub(moment, 'now').returns(TIMESTAMP); - sandbox.stub(config, 'get').returns(USER_DETAILS_TTL); + sandbox.stub(config, 'get').callsFake((key) => { + if (key === 'redis.userDetailsTtlSec') return USER_DETAILS_TTL; + if (key === 'caseData.base_url') return 'http://case-data'; + return USER_DETAILS_TTL; + }); sandbox.stub(redis, "pipeline").callsFake(function (arguments) { argStr = JSON.stringify(arguments); if (argStr.includes('zrangebyscore')) { pipStub.exec = () => Promise.resolve([[null, ['242']], [null, ['12']]]); return pipStub; } else { - pipStub.exec = () => Promise.resolve([[null, null], [null, "{\"forename\":\"sam\",\"surname\":\"gamgee\"}"]]); + pipStub.exec = () => Promise.resolve([[null, null], [null, '{"forename":"sam","surname":"gamgee"}']]); return pipStub; } }); - const result = activityService.getActivities(['767', '888'], { uid: '111' }); + const result = activityService.getActivities(['767', '888'], { uid: '111' }, 'Bearer token'); result.then((content) => { expect(content).deep.equal([{ @@ -126,19 +150,23 @@ describe("activity service", () => { it("getActivities should not return in the list of viewers the requesting user id", (done) => { sandbox.stub(moment, 'now').returns(TIMESTAMP); - sandbox.stub(config, 'get').returns(USER_DETAILS_TTL); + sandbox.stub(config, 'get').callsFake((key) => { + if (key === 'redis.userDetailsTtlSec') return USER_DETAILS_TTL; + if (key === 'caseData.base_url') return 'http://case-data'; + return USER_DETAILS_TTL; + }); sandbox.stub(redis, "pipeline").callsFake(function (arguments) { argStr = JSON.stringify(arguments); if (argStr.includes('zrangebyscore')) { pipStub.exec = () => Promise.resolve([[null, ['242']], [null, ['12']]]); return pipStub; } else { - pipStub.exec = () => Promise.resolve([[null, "{\"forename\":\"nayab\",\"surname\":\"gul\"}"], [null, "{\"forename\":\"sam\",\"surname\":\"gamgee\"}"]]); + pipStub.exec = () => Promise.resolve([[null, '{"forename":"nayab","surname":"gul"}'], [null, '{"forename":"sam","surname":"gamgee"}']]); return pipStub; } }); - const result = activityService.getActivities(['767', '888'], { uid: '242' }); + const result = activityService.getActivities(['767', '888'], { uid: '242' }, 'Bearer token'); result.then((content) => { expect(content).deep.equal([{ @@ -160,7 +188,11 @@ describe("activity service", () => { it("getActivities should not return the requesting user id in the list of unknown viewers", (done) => { sandbox.stub(moment, 'now').returns(TIMESTAMP); - sandbox.stub(config, 'get').returns(USER_DETAILS_TTL); + sandbox.stub(config, 'get').callsFake((key) => { + if (key === 'redis.userDetailsTtlSec') return USER_DETAILS_TTL; + if (key === 'caseData.base_url') return 'http://case-data'; + return USER_DETAILS_TTL; + }); sandbox.stub(redis, "pipeline").callsFake(function (arguments) { argStr = JSON.stringify(arguments); if (argStr.includes('zrangebyscore')) { @@ -169,12 +201,12 @@ describe("activity service", () => { return pipStub; } else { //return the following user info for users 242 (unkown) and 12 (sam gamgee) - pipStub.exec = () => Promise.resolve([[null, null], [null, "{\"forename\":\"sam\",\"surname\":\"gamgee\"}"]]); + pipStub.exec = () => Promise.resolve([[null, null], [null, '{"forename":"sam","surname":"gamgee"}']]); return pipStub; } }); - const result = activityService.getActivities(['767', '888'], { uid: '242' }); + const result = activityService.getActivities(['767', '888'], { uid: '242' }, 'Bearer token'); result.then((content) => { // don't expect unknown users since the unknown user is the requester From 81c3a96a28ff513c6c941cb4ca65a29eb6173123 Mon Sep 17 00:00:00 2001 From: hmcts-jenkins-a-to-c <62422075+hmcts-jenkins-a-to-c[bot]@users.noreply.github.com> Date: Wed, 11 Mar 2026 12:28:16 +0000 Subject: [PATCH 2/3] Bumping chart version/ fixing aliases --- charts/ccd-case-activity-api/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/ccd-case-activity-api/Chart.yaml b/charts/ccd-case-activity-api/Chart.yaml index 572d1df7..a1a10519 100644 --- a/charts/ccd-case-activity-api/Chart.yaml +++ b/charts/ccd-case-activity-api/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 description: Helm chart for the HMCTS CCD Case Activity name: ccd-case-activity-api home: https://github.com/hmcts/ccd-case-activity-api -version: 1.3.17 +version: 1.3.18 maintainers: - name: HMCTS CCD Dev Team email: ccd-devops@HMCTS.NET From 70e9a23817604562a25fc740633698d21d1fd819 Mon Sep 17 00:00:00 2001 From: Olu <142989683+olusegz07@users.noreply.github.com> Date: Wed, 11 Mar 2026 12:48:22 +0000 Subject: [PATCH 3/3] fix update test coverage --- .../spec/app/service/activity-service.spec.js | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/test/spec/app/service/activity-service.spec.js b/test/spec/app/service/activity-service.spec.js index de91346b..185851cf 100644 --- a/test/spec/app/service/activity-service.spec.js +++ b/test/spec/app/service/activity-service.spec.js @@ -226,4 +226,99 @@ describe("activity service", () => { done(); }).catch(err => console.log('error', done(err))); }) + + it("getActivities should return empty array when caseIds is empty", (done) => { + activityService.getActivities([], { uid: '123' }, 'token') + .then((content) => { + expect(content).deep.equal([]); + done(); + }).catch(err => console.log('error', done(err))); + }); + + it("getActivities should return empty case status when access is non-standard", (done) => { + fetchStub.resolves({ + json: () => Promise.resolve({ + metadataFields: [ + { id: '[ACCESS_PROCESS]', value: 'CHALLENGED' }, + { id: '[ACCESS_GRANTED]', value: 'BASIC' } + ] + }) + }); + sandbox.stub(config, 'get').callsFake((key) => { + if (key === 'ccd.url') return 'http://ccd'; + return USER_DETAILS_TTL; + }); + + activityService.getActivities(['1', '2'], { uid: 'user-1' }, 'token') + .then((content) => { + expect(content).deep.equal([{ + caseId: '1', + viewers: [], + unknownViewers: 0, + editors: [], + unknownEditors: 0 + }, { + caseId: '2', + viewers: [], + unknownViewers: 0, + editors: [], + unknownEditors: 0 + }]); + done(); + }).catch(err => console.log('error', done(err))); + }); + + it("getActivities should return empty case status when access check fails", (done) => { + fetchStub.rejects(new Error('boom')); + sandbox.stub(config, 'get').callsFake((key) => { + if (key === 'ccd.url') return 'http://ccd'; + return USER_DETAILS_TTL; + }); + + activityService.getActivities(['99'], { uid: 'user-1' }, 'token') + .then((content) => { + expect(content).deep.equal([{ + caseId: '99', + viewers: [], + unknownViewers: 0, + editors: [], + unknownEditors: 0 + }]); + done(); + }).catch(err => console.log('error', done(err))); + }); + + it("getActivities should call case view with bearer token when access allowed", (done) => { + fetchStub.resolves({ json: () => Promise.resolve({}) }); + sandbox.stub(moment, 'now').returns(TIMESTAMP); + sandbox.stub(config, 'get').callsFake((key) => { + if (key === 'ccd.url') return 'http://ccd'; + return USER_DETAILS_TTL; + }); + pipStub = sinon.stub(); + sandbox.stub(redis, "pipeline").callsFake(function (arguments) { + argStr = JSON.stringify(arguments); + if (argStr.includes('zrangebyscore')) { + pipStub.exec = () => Promise.resolve([[null, []]]); + return pipStub; + } + pipStub.exec = () => Promise.resolve([]); + return pipStub; + }); + + activityService.getActivities(['123'], { uid: 'user-1' }, 'token') + .then((content) => { + expect(fetchStub).to.have.been.calledWith('http://ccd/internal/cases/123', { + headers: { Authorization: 'token', Accept: 'application/vnd.uk.gov.hmcts.ccd-data-store-api.ui-case-view.v2+json' } + }); + expect(content).deep.equal([{ + caseId: '123', + viewers: [], + unknownViewers: 0, + editors: [], + unknownEditors: 0 + }]); + done(); + }).catch(err => console.log('error', done(err))); + }); });