diff --git a/api/aggregators/itemAggregator.js b/api/aggregators/itemAggregator.js index 4b7c709e..0c558499 100644 --- a/api/aggregators/itemAggregator.js +++ b/api/aggregators/itemAggregator.js @@ -28,7 +28,10 @@ exports.resolveParentGate = async (schemaName, roles) => { } if (schemaName === constants.COMMENT) { - return { unreadablePeriodIds: await parentRead.unreadablePeriodIds(roles) }; + // The public sees a comment only under a period it can read; staff also see one whose period is gone. + return updateRules.isPublicCaller(roles) + ? { readablePeriodIds: await parentRead.readablePeriodIds(roles) } + : { unreadablePeriodIds: await parentRead.unreadablePeriodIds(roles) }; } return {}; @@ -57,7 +60,8 @@ exports.createItemAggr = (itemId, schemaName, roles, gate) => { aggregation.push( { - '$match': { _id: new mongoose.Types.ObjectId(itemId) } + // Every model shares the epic collection: without the schema name, any allowed name reads any record. + '$match': { _id: new mongoose.Types.ObjectId(itemId), _schemaName: schemaName } }, { $redact: { @@ -95,8 +99,10 @@ exports.createItemAggr = (itemId, schemaName, roles, gate) => { aggregation.push(...parentRead.parentReadMatch(gate.unreadableParentIds)); } else if (schemaName === constants.COMMENT) { // A comment hangs off a comment period, which hangs off a project. Both levels gate it, and - // unreadablePeriodIds already folds the project level into the period set. - aggregation.push(...parentRead.parentReadMatch(gate.unreadablePeriodIds, 'period')); + // both period sets already fold the project level in. + aggregation.push(...(gate.readablePeriodIds + ? parentRead.readablePeriodMatch(gate.readablePeriodIds) + : parentRead.parentReadMatch(gate.unreadablePeriodIds, 'period'))); } if (schemaName === constants.INSPECTION) { diff --git a/api/controllers/comment.js b/api/controllers/comment.js index 7dadd238..86dd31f5 100644 --- a/api/controllers/comment.js +++ b/api/controllers/comment.js @@ -3,35 +3,16 @@ var mongoose = require('mongoose'); var Actions = require('../helpers/actions'); var Utils = require('../helpers/utils'); const demiPush = require('../helpers/demiPush'); +const parentRead = require('../helpers/parentRead'); const { DateTime } = require('luxon'); -const { COMMENT_STAFF_FIELDS } = require('../helpers/constants'); - -const ALLOWED_FIELDS = [ - 'author', - 'comment', - 'commentId', - 'dateAdded', - 'datePosted', - 'dateUpdated', - 'documents', - 'eaoNotes', - 'eaoStatus', - 'submittedCAC', - 'isAnonymous', - 'location', - 'period', - 'proponentNotes', - 'proponentStatus', - 'publishedNotes', - 'rejectedNotes', - 'rejectedReason', - 'valuedComponents', - 'read', - 'write', - 'delete' -]; -// Staff review fields stay off the public routes. -const PUBLIC_FIELDS = ALLOWED_FIELDS.filter(f => !COMMENT_STAFF_FIELDS.includes(f)); +const { COMMENT_FIELDS: ALLOWED_FIELDS, COMMENT_PUBLIC_FIELDS: PUBLIC_FIELDS, PUBLIC_ROLES } = require('../helpers/constants'); + +// A comment's own read[] stays Published when staff unpublish its period or project, so the +// public routes keep only comments under a period the public can read, as dataset=Item does. +const publicPeriodGate = async () => parentRead.readablePeriodMatch(await parentRead.readablePeriodIds(PUBLIC_ROLES)); + +// Sorting on a field the reply hides would leak its order, so the public sorts on these only. +const PUBLIC_SORT_FIELDS = ['commentId', 'dateAdded']; // Hiding an anonymous author needs isAnonymous: read it with author, drop it when the caller did not ask. const publicCommentQuery = fields => { @@ -110,7 +91,8 @@ exports.publicHead = async function (args, res) { null, // sort null, // skip null, // limit - true); // count + true, // count + await publicPeriodGate()); // pre-query pipeline steps Utils.recordAction('Head', 'CommentPeriod', 'public'); // /api/comment/ route, return 200 OK with 0 items if necessary if (!(args.swagger.params.period && args.swagger.params.period.value) || (data && data.length > 0)) { @@ -133,13 +115,15 @@ exports.publicGet = async function (args, res) { if (args.swagger.params.period && args.swagger.params.period.value) { query = Utils.buildQuery('period', args.swagger.params.period.value, query); } - // Sort if (args.swagger.params.sortBy && args.swagger.params.sortBy.value) { - args.swagger.params.sortBy.value.forEach(function (value) { - var order_by = value.charAt(0) == '-' ? -1 : 1; - var sort_by = value.slice(1); - sort[sort_by] = order_by; - }, this); + args.swagger.params.sortBy.value.forEach(value => { + const sortBy = value.slice(1); + if (PUBLIC_SORT_FIELDS.includes(sortBy)) { + sort[sortBy] = value.charAt(0) === '-' ? -1 : 1; + } else { + defaultLog.debug('publicGet: ignoring sort on %s', sortBy); + } + }); } var processedParameters = Utils.getSkipLimitParameters(args.swagger.params.pageSize, args.swagger.params.pageNum); @@ -159,7 +143,8 @@ exports.publicGet = async function (args, res) { sort, // sort skip, // skip limit, // limit - true); // count + true, // count + await publicPeriodGate()); // pre-query pipeline steps const results = data[0] ? data[0].results : []; results.forEach(commentQuery.redact); @@ -299,7 +284,7 @@ exports.protectedGet = async function (args, res) { limit, // limit count); // count Utils.recordAction('Get', 'Comment', args.swagger.params.auth_payload.preferred_username, args.swagger.params.commentId && args.swagger.params.commentId.value ? args.swagger.params.commentId.value : null); - defaultLog.info('Got comment(s):', data); + defaultLog.info('Got %d comment(s)', count ? (data[0] ? data[0].total_items : 0) : data.length); // This is to get the next pending comment information. if (args.swagger.params.populateNextComment && args.swagger.params.populateNextComment.value) { @@ -332,7 +317,7 @@ exports.protectedGet = async function (args, res) { // Create a new Comment exports.protectedPost = async function (args, res) { var obj = args.swagger.params.comment.value; - defaultLog.info('Incoming new comment:', obj); + defaultLog.info('Incoming new comment for period: %s', obj.period); var Comment = mongoose.model('Comment'); @@ -383,7 +368,7 @@ exports.protectedPost = async function (args, res) { try { var c = await comment.save(); Utils.recordAction('Post', 'Comment', args.swagger.params.auth_payload.preferred_username, c._id); - defaultLog.info('Saved new comment object:', c); + defaultLog.info('Saved new comment object: %s', c._id); return Actions.sendMirrored(res, 200, c, demiPush.comment(c)); } catch (e) { defaultLog.error(`Error: ${e.message}`); @@ -401,7 +386,7 @@ async function getNextCommentIdCount(period) { exports.unProtectedPost = async function (args, res) { try { const obj = args.swagger.params.comment.value; - defaultLog.info('Incoming new object:', obj); + defaultLog.info('Incoming new comment for period: %s', obj.period); if (args.swagger.params.period && args.swagger.params.period.value && !mongoose.Types.ObjectId.isValid(args.swagger.params.period.value)) { return Actions.sendResponse(res, 400, { }); } @@ -411,7 +396,7 @@ exports.unProtectedPost = async function (args, res) { const commentPeriodModel = mongoose.model('CommentPeriod'); const period = await commentPeriodModel.findOne({ _id: new mongoose.Types.ObjectId(obj.period) }); - defaultLog.info('Period:', period); + defaultLog.info('Period: %s', period && period._id); if (DateTime.now() > DateTime.fromJSDate(new Date(period.dateCompleted))) { // Past the specified time @@ -489,7 +474,7 @@ exports.protectedPut = async function (args, res) { }; comment = setPermissionsFromEaoStatus(obj.eaoStatus, comment); - defaultLog.info('Incoming updated object:', comment); + defaultLog.info('Updating comment %s to status %s', objId, comment.eaoStatus); try { var c = await Comment.updateOne({ _id: objId }, { $set: comment }); diff --git a/api/controllers/search.js b/api/controllers/search.js index 1f83a59d..7eb9c8e0 100644 --- a/api/controllers/search.js +++ b/api/controllers/search.js @@ -6,7 +6,7 @@ const Actions = require('../helpers/actions'); const Utils = require('../helpers/utils'); const analytics = require('../helpers/analytics'); const constants = require('../helpers/constants').schemaTypes; -const { COMMENT_STAFF_FIELDS } = require('../helpers/constants'); +const { COMMENT_PUBLIC_FIELDS } = require('../helpers/constants'); const documentAggregator = require('../aggregators/documentAggregator'); const projectAggregator = require('../aggregators/projectAggregator'); @@ -34,6 +34,19 @@ const PROJECT_JOINED_FIELDS = ['proponent', 'currentPhaseName', 'eacDecision', ' // Project fields still on the root before the page is cut; unwindProjectData copies them into default after it. const PROJECT_ROOT_SORT_FIELDS = ['_id', ...aggregateHelper.PROJECT_ROOT_FIELDS]; +const PUBLIC_COMMENT_ITEM_FIELDS = ['_id', '_schemaName', ...COMMENT_PUBLIC_FIELDS]; + +// A public caller gets only the public comment fields; the author stays hidden unless the commenter chose to show it. +const commentItemView = (item, isAuthenticated) => { + const view = isAuthenticated + ? { ...item } + : Object.fromEntries(PUBLIC_COMMENT_ITEM_FIELDS.filter(f => f in item).map(f => [f, item[f]])); + if (item.isAnonymous !== false) { + delete view.author; + } + return view; +}; + const searchCollection = async function (roles, keywords, schemaName, pageNum, pageSize, project, projectLegislation, sortField = undefined, sortDirection = undefined, caseSensitive, populate = false, and, or, sortingValue, categorized, fuzzy, signedIn) { defaultLog.debug('collation:', aggregateHelper.AGGREGATE_COLLATION); defaultLog.debug('populate:', populate); @@ -355,16 +368,6 @@ const executeQuery = async function (args, res) { ); } - // TODO: this should be moved into the aggregation. - if (dataset === constants.COMMENT) { - // Filter - collectionData[0].searchResults.forEach(function (item) { - if (item.isAnonymous !== false) { - delete item.author; - } - }); - } - return Actions.sendResponse(res, 200, collectionData); } else if (dataset === constants.ITEM) { @@ -379,26 +382,18 @@ const executeQuery = async function (args, res) { const collectionObj = mongoose.model(schemaNameVal); const gate = await itemAggregator.resolveParentGate(schemaNameVal, roles); const aggregation = itemAggregator.createItemAggr(args.swagger.params._id.value, schemaNameVal, roles, gate); - let data = await collectionObj.aggregate(aggregation).allowDiskUse(true); + const data = await collectionObj.aggregate(aggregation).allowDiskUse(true); - if (schemaNameVal === constants.COMMENT) { - // Filter - data.forEach(function (item) { - if (item.isAnonymous !== false) { - delete item.author; - } - if (!isAuthenticated) { - COMMENT_STAFF_FIELDS.forEach(field => delete item[field]); - } - }); - } + // Keyed on each record's own schema as well as the requested one, in case a record of another + // schema ever gets past the $match. + const isSchema = (item, schemaName) => schemaNameVal === schemaName || item._schemaName === schemaName; - if (schemaNameVal === constants.PROJECT) { - // If we are a project, and we are not authed, we need to sanitize some fields. - data = Utils.filterData(schemaNameVal, data, roles); - } + // filterData strips fields in place. + Utils.filterData(constants.PROJECT, data.filter(item => isSchema(item, constants.PROJECT)), roles); + + const results = data.map(item => (isSchema(item, constants.COMMENT) ? commentItemView(item, isAuthenticated) : item)); - return Actions.sendResponse(res, 200, data); + return Actions.sendResponse(res, 200, results); } else { defaultLog.warn('search.js executeQuery: bad request — missing schemaName'); return Actions.sendResponse(res, 400, {}); diff --git a/api/helpers/constants.js b/api/helpers/constants.js index a9dbc3f0..85b60475 100644 --- a/api/helpers/constants.js +++ b/api/helpers/constants.js @@ -20,8 +20,40 @@ exports.MAX_FEATURE_DOCS = 5; exports.PUBLIC_ROLES = ['public']; exports.SECURE_ROLES = ['sysadmin', 'staff']; +// Comment fields the comment routes may return. +const COMMENT_FIELDS = Object.freeze([ + 'author', + 'comment', + 'commentId', + 'dateAdded', + 'datePosted', + 'dateUpdated', + 'documents', + 'eaoNotes', + 'eaoStatus', + 'submittedCAC', + 'isAnonymous', + 'location', + 'period', + 'proponentNotes', + 'proponentStatus', + 'publishedNotes', + 'rejectedNotes', + 'rejectedReason', + 'valuedComponents', + 'read', + 'write', + 'delete' +]); +exports.COMMENT_FIELDS = COMMENT_FIELDS; + // Comment fields only signed-in staff may read. -exports.COMMENT_STAFF_FIELDS = Object.freeze(['eaoNotes', 'eaoStatus', 'proponentNotes', 'rejectedNotes', 'rejectedReason']); +const COMMENT_STAFF_FIELDS = Object.freeze(['eaoNotes', 'eaoStatus', 'proponentNotes', 'rejectedNotes', 'rejectedReason']); +exports.COMMENT_STAFF_FIELDS = COMMENT_STAFF_FIELDS; + +// Comment fields the public routes may return. write and delete name the staff roles that may edit it. +const COMMENT_HIDDEN_FROM_PUBLIC = [...COMMENT_STAFF_FIELDS, 'write', 'delete']; +exports.COMMENT_PUBLIC_FIELDS = Object.freeze(COMMENT_FIELDS.filter(f => !COMMENT_HIDDEN_FROM_PUBLIC.includes(f))); // Legislation registry, keyed by year. A project keeps one sub-document per year under `legislation_`. // Adding an Act: add its entry here and its year to the project POST and publish prose in swagger.yaml (a test checks). diff --git a/api/helpers/parentRead.js b/api/helpers/parentRead.js index 23293fe9..12639a8e 100644 --- a/api/helpers/parentRead.js +++ b/api/helpers/parentRead.js @@ -49,21 +49,33 @@ exports.unreadableParentIds = async (roles) => { return ids; }; +// A period hides its comments on its own read[] or because its project is unreadable. +const hidesComments = (roles, unreadableProjects) => [unreadableBy(roles), { project: { $in: unreadableProjects } }]; + /** * Ids of the comment periods that hide their comments, either on their own `read[]` or because * their project hides them. Folding both levels in here keeps the comment gate a single $match. */ exports.unreadablePeriodIds = async (roles) => { const parents = await exports.unreadableParentIds(roles); - const ids = await idsMatching({ - _schemaName: 'CommentPeriod', - $or: [unreadableBy(roles), { project: { $in: parents } }] - }); + const ids = await idsMatching({ _schemaName: 'CommentPeriod', $or: hidesComments(roles, parents) }); defaultLog.debug('parent read gate: %d unreadable comment periods', ids.length); return ids; }; +/** + * Ids of the comment periods whose comments the caller may see: every period unreadablePeriodIds + * leaves out. Matching a comment against this set also hides one whose period is unset or gone. + */ +exports.readablePeriodIds = async (roles) => { + const parents = await exports.unreadableParentIds(roles); + const ids = await idsMatching({ _schemaName: 'CommentPeriod', $nor: hidesComments(roles, parents) }); + defaultLog.debug('parent read gate: %d readable comment periods', ids.length); + + return ids; +}; + /** * The gate. $nin does not match a row whose parent field is absent or points at a parent that is * gone, so those keep only their own `read[]`, which the caller applies. Empty when the caller can @@ -81,3 +93,14 @@ exports.parentReadMatch = (unreadableIds, parentField = 'project') => { return [{ $match: { [parentField]: { $nin: unreadableIds } } }]; }; + +/** + * The strict comment gate: keeps only comments whose period is in readablePeriodIds. + */ +exports.readablePeriodMatch = (readableIds) => { + if (!Array.isArray(readableIds)) { + throw new TypeError('comment period gate needs an array of readable ids from helpers/parentRead'); + } + + return [{ $match: { period: { $in: readableIds } } }]; +}; diff --git a/api/swagger/swagger.yaml b/api/swagger/swagger.yaml index 43080af1..98aedf47 100644 --- a/api/swagger/swagger.yaml +++ b/api/swagger/swagger.yaml @@ -491,8 +491,6 @@ definitions: - 'publishedNotes' - 'valuedComponents' - 'read' - - 'write' - - 'delete' CommentSortByFields: type: string description: "Optional fields to sort by (prefixed with + or - for order)" @@ -502,6 +500,14 @@ definitions: - -author - +dateAdded - -dateAdded + PublicCommentSortByFields: + type: string + description: "Fields the public comment routes sort by. Other fields are ignored." + enum: &publicCommentSortByFields + - +commentId + - -commentId + - +dateAdded + - -dateAdded ### Comment Period Definitions CommentStatus: @@ -5387,12 +5393,12 @@ paths: description: "Number of results per page (if 'pageNum' is specified)." - in: query name: sortBy - description: "Comment fields to sort by." + description: "Comment fields to sort by: commentId or dateAdded. Other fields are ignored." required: false type: array items: type: string - enum: *commentSortByFields + enum: *publicCommentSortByFields responses: "200": description: "Success" diff --git a/test/aggregators/aggregators.test.js b/test/aggregators/aggregators.test.js index 5afc1915..07012332 100644 --- a/test/aggregators/aggregators.test.js +++ b/test/aggregators/aggregators.test.js @@ -700,6 +700,14 @@ describe('Item Aggregator', () => { expect(gateIndex).to.be.lessThan(lookupIndex); }); + it('should keep a comment only under the readable periods it is given', () => { + const period = new mongoose.Types.ObjectId(); + const result = itemAggregator.createItemAggr(new mongoose.Types.ObjectId().toString(), 'Comment', ['public'], { readablePeriodIds: [period] }); + + const gateStage = result.find(stage => stage.$match && stage.$match.period); + expect(gateStage.$match.period).to.deep.equal({ $in: [period] }); + }); + it('should refuse to build a pipeline with no gate object', () => { expect(() => itemAggregator.createItemAggr(new mongoose.Types.ObjectId().toString(), 'Item', ['public'])) .to.throw(TypeError, /resolveParentGate/); diff --git a/test/controllers/analytics-callsites.test.js b/test/controllers/analytics-callsites.test.js index d36328f3..4913ca2f 100644 --- a/test/controllers/analytics-callsites.test.js +++ b/test/controllers/analytics-callsites.test.js @@ -122,6 +122,7 @@ describe('Analytics call sites', () => { // is covered against a real MongoDB by test/db/*ParentRead.test.js. sinon.stub(parentRead, 'unreadableParentIds').resolves([]); sinon.stub(parentRead, 'unreadablePeriodIds').resolves([]); + sinon.stub(parentRead, 'readablePeriodIds').resolves([]); sinon.stub(analytics, 'auditEvent'); sinon.stub(analytics, 'trackEvent'); diff --git a/test/controllers/comment.test.js b/test/controllers/comment.test.js index e1677fc8..f306ba85 100644 --- a/test/controllers/comment.test.js +++ b/test/controllers/comment.test.js @@ -9,8 +9,10 @@ const { expect } = require('chai'); const sinon = require('sinon'); const mongoose = require('mongoose'); +const util = require('util'); const { DateTime } = require('luxon'); const Utils = require('../../api/helpers/utils'); +const parentRead = require('../../api/helpers/parentRead'); const Actions = require('../../api/helpers/actions'); const defaultLog = require('winston').loggers.get('default'); @@ -120,6 +122,9 @@ describe('Comment Controller', () => { }); sinon.stub(Utils, 'runDataQuery'); + // The period gate reads the epic collection; what it returns is covered against a real MongoDB + // by test/db/commentParentRead.test.js. + sinon.stub(parentRead, 'readablePeriodIds').resolves([]); sinon.stub(Utils, 'recordAction').resolves(); sinon.stub(Utils, 'buildQuery').callsFake((prop, val, q) => { q[prop] = val; @@ -259,6 +264,145 @@ describe('Comment Controller', () => { }); }); + describe('public routes keep only comments under a period the public can read', () => { + const SHOWN_PERIOD = new mongoose.Types.ObjectId('507f1f77bcf86cd799439055'); + // runDataQuery's tenth argument: stages put ahead of its $match. + const preQuerySteps = () => Utils.runDataQuery.firstCall.args[9]; + + beforeEach(() => { + parentRead.readablePeriodIds.resolves([SHOWN_PERIOD]); + Utils.runDataQuery.resolves([{ total_items: 0, results: [] }]); + }); + + const routes = [ + ['publicHead on a period', () => commentController.publicHead(makeArgs(), res)], + ['publicGet on a period', () => { + const args = makeArgs(); + delete args.swagger.params.commentId; + return commentController.publicGet(args, res); + }], + ['publicGet by id', () => commentController.publicGet({ + swagger: { params: { commentId: { value: VALID_COMM_ID }, fields: { value: ['comment'] } } } + }, res)] + ]; + + routes.forEach(([label, call]) => { + it(`on ${label}`, async () => { + await call(); + + expect(parentRead.readablePeriodIds.calledOnceWithExactly(['public'])).to.be.true; + expect(preQuerySteps()).to.deep.equal([{ $match: { period: { $in: [SHOWN_PERIOD] } } }]); + }); + }); + + it('matches nothing when the public can read no period', async () => { + parentRead.readablePeriodIds.resolves([]); + + await commentController.publicHead(makeArgs(), res); + + expect(preQuerySteps()).to.deep.equal([{ $match: { period: { $in: [] } } }]); + }); + }); + + describe('public sort and fields', () => { + const listArgs = (overrides) => { + const args = makeArgs(overrides); + delete args.swagger.params.commentId; + return args; + }; + + beforeEach(() => Utils.runDataQuery.resolves([{ total_items: 0, results: [] }])); + + it('sorts only on commentId and dateAdded, never on a hidden field such as author', async () => { + await commentController.publicGet(listArgs({ sortBy: { value: ['+author', '-dateAdded', '+eaoStatus', '-commentId'] } }), res); + + expect(Utils.runDataQuery.firstCall.args[5]).to.deep.equal({ dateAdded: -1, commentId: -1 }); + }); + + it('never reads write or delete for a public caller', async () => { + await commentController.publicGet(listArgs({ fields: { value: ['comment', 'write', 'delete'] } }), res); + + expect(Utils.runDataQuery.firstCall.args[3]).to.deep.equal(['comment']); + }); + }); + + describe('logs', () => { + const BODY = { + author: 'Jane Private', + comment: 'My private words', + location: 'Smalltown', + period: VALID_CP_ID, + isAnonymous: true, + eaoStatus: 'Pending', + eaoNotes: 'Staff note text', + rejectedNotes: 'Rejected note text', + valuedComponents: [], + documents: [] + }; + const SECRETS = ['Jane Private', 'My private words', 'Smalltown', 'Staff note text', 'Rejected note text']; + const logged = () => [defaultLog.info, defaultLog.debug, defaultLog.error] + .flatMap(spy => spy.args) + .map(args => util.format(...args)) + .join('\n'); + const expectNoBody = () => SECRETS.forEach(secret => expect(logged()).to.not.include(secret)); + const withComment = () => makeArgs({ comment: { value: { ...BODY } } }); + + function useMockComment() { + function MockComment(obj) { + Object.assign(this, obj); + this.save = sinon.stub().resolves({ _id: VALID_COMM_ID, ...obj }); + } + mongoose.model.restore(); + sinon.stub(mongoose, 'model').callsFake(name => { + if (name === 'Comment') return MockComment; + if (name === 'CommentPeriod') return cpModel; + return {}; + }); + } + + beforeEach(() => { + sinon.stub(defaultLog, 'debug'); + cpModel.findOne.resolves({ _id: VALID_CP_ID, dateCompleted: DateTime.now().plus({ days: 2 }).toJSDate() }); + cpModel.findOneAndUpdate.resolves({ commentIdCount: 15 }); + }); + + it('logs a public submission without its author, location or text', async () => { + useMockComment(); + + await commentController.unProtectedPost(withComment(), res); + + expect(res.status.calledWith(200)).to.be.true; + expectNoBody(); + }); + + it('logs a staff-entered comment without its body', async () => { + useMockComment(); + + await commentController.protectedPost(withComment(), res); + + expect(res.status.calledWith(200)).to.be.true; + expectNoBody(); + }); + + it('logs a staff read by count, not by the comments read', async () => { + Utils.runDataQuery.resolves([{ _id: VALID_COMM_ID, ...BODY }]); + + await commentController.protectedGet(makeArgs(), res); + + expect(res.status.calledWith(200)).to.be.true; + expectNoBody(); + }); + + it('logs a staff update without the notes it writes', async () => { + commentModel.updateOne.resolves({ modifiedCount: 1 }); + + await commentController.protectedPut(withComment(), res); + + expect(res.status.calledWith(200)).to.be.true; + expectNoBody(); + }); + }); + describe('protectedGet', () => { it('allows filtering by eaoStatus', async () => { const args = makeArgs({ pending: { value: true }, published: { value: true } }); diff --git a/test/controllers/search.test.js b/test/controllers/search.test.js index 259e3c10..c8177322 100644 --- a/test/controllers/search.test.js +++ b/test/controllers/search.test.js @@ -81,6 +81,7 @@ describe('Search Controller', () => { // is covered against a real MongoDB by test/db/*ParentRead.test.js. sinon.stub(parentRead, 'unreadableParentIds').resolves([]); sinon.stub(parentRead, 'unreadablePeriodIds').resolves([]); + sinon.stub(parentRead, 'readablePeriodIds').resolves([]); sinon.stub(Actions, 'sendResponse').callsFake((r, code, data) => { r.status(code).json(data); @@ -263,6 +264,67 @@ describe('Search Controller', () => { expect(res.status.firstCall.args[0]).to.equal(200); expect(res.json.firstCall.args[0][0]).to.include(staffFields); }); + + const HIDDEN_FIELDS = { + ...staffFields, + demiPushPending: true, + demiPushFailedAt: new Date(), + demiPushError: 'push error', + write: ['staff'], + delete: ['staff'], + _addedBy: 'staff-user', + _updatedBy: 'staff-user', + _deletedBy: 'staff-user' + }; + const SHOWN_FIELDS = { _id: 'a', _schemaName: 'Comment', comment: 'Hi', commentId: 7, period: 'p1', read: ['public'], isAnonymous: false, author: 'Lee' }; + const publicCaller = { auth_payload: { realm_access: { roles: ['public'] }, preferred_username: 'public' } }; + + it('returns only the public comment fields to a public caller', async () => { + modelStub.aggregate.returns({ allowDiskUse: () => Promise.resolve([{ ...SHOWN_FIELDS, ...HIDDEN_FIELDS }]) }); + + await searchController.publicGet(commentItem(publicCaller), res); + + expect(res.json.firstCall.args[0]).to.deep.equal([SHOWN_FIELDS]); + }); + + it('cleans a stored comment even when another schema name was asked for', async () => { + modelStub.aggregate.returns({ allowDiskUse: () => Promise.resolve([{ ...SHOWN_FIELDS, isAnonymous: true, ...HIDDEN_FIELDS }]) }); + + await searchController.publicGet(commentItem({ ...publicCaller, _schemaName: { value: 'Document' } }), res); + + const expected = { ...SHOWN_FIELDS, isAnonymous: true }; + delete expected.author; + expect(res.json.firstCall.args[0]).to.deep.equal([expected]); + }); + }); + + describe('Item schema name', () => { + const itemArgs = (schemaName, overrides = {}) => makeArgs({ + dataset: { value: 'Item' }, + _schemaName: { value: schemaName }, + _id: { value: '507f1f77bcf86cd799439011' }, + auth_payload: { realm_access: { roles: ['public'] }, preferred_username: 'public' }, + ...overrides + }); + + it('matches on the requested schema name as well as the id', async () => { + aggregateStub.allowDiskUse.resolves([]); + + await searchController.publicGet(itemArgs('Document'), res); + + const firstStage = modelStub.aggregate.firstCall.args[0][0]; + expect(String(firstStage.$match._id)).to.equal('507f1f77bcf86cd799439011'); + expect(firstStage.$match._schemaName).to.equal('Document'); + }); + + it('strips the staff project fields from a stored project asked for under another name', async () => { + Utils.filterData.restore(); + aggregateStub.allowDiskUse.resolves([{ _id: 'p', _schemaName: 'Project', name: 'Mine', review180Start: 'x', reviewExtensions: ['y'] }]); + + await searchController.publicGet(itemArgs('Document'), res); + + expect(res.json.firstCall.args[0]).to.deep.equal([{ _id: 'p', _schemaName: 'Project', name: 'Mine' }]); + }); }); describe('Document search signed-in flag', () => { @@ -297,7 +359,7 @@ describe('Search Controller', () => { _id: { value: '507f1f77bcf86cd799439011' } }); - aggregateStub.exec.resolves([{ _id: '123' }]); + aggregateStub.allowDiskUse.resolves([{ _id: '123' }]); await searchController.publicGet(args, res); expect(res.status.calledWith(200)).to.be.true; @@ -448,7 +510,7 @@ describe('Search Controller', () => { _schemaName: { value: schemaName }, _id: { value: '507f1f77bcf86cd799439011' } }); - aggregateStub.exec.resolves([]); + aggregateStub.allowDiskUse.resolves([]); await searchController.publicGet(args, res); diff --git a/test/db/commentParentRead.test.js b/test/db/commentParentRead.test.js new file mode 100644 index 00000000..01272cfc --- /dev/null +++ b/test/db/commentParentRead.test.js @@ -0,0 +1,194 @@ +/** + * Public comment routes against a real MongoDB. + * + * A comment keeps its own Published read[] when staff unpublish its comment period or project, so + * /api/public/comment must gate on the period the way dataset=Item does. + * + * Needs a MongoDB the aggregation framework actually runs on, so it is not part of `npm test`. + * + * npm run db:up + * npm run test:db + */ + +'use strict'; + +const { expect } = require('chai'); +const sinon = require('sinon'); +const mongoose = require('mongoose'); + +require('../../app_helper'); + +const Utils = require('../../api/helpers/utils'); +const commentController = require('../../api/controllers/comment'); + +const { + TEST_URI, + id, + STAFF_ONLY, + PUBLIC_READ, + PUBLIC_PROJECT, + PRIVATE_PROJECT, + PARENT_FIXTURES +} = require('./parentReadFixtures'); + +const CP_UNDER_PUBLIC_PROJECT = id('58990017d334ee001d60ab01'); +const CP_UNDER_PRIVATE_PROJECT = id('58990017d334ee001d60ab02'); +const STAFF_CP_UNDER_PUBLIC_PROJECT = id('58990017d334ee001d60ab03'); +const AUTHORS_CP = id('58990017d334ee001d60ab04'); +const DELETED_PERIOD = id('58990017d334ee001d60abff'); + +const COMMENT_UNDER_PUBLIC_PERIOD = id('58990017d334ee001d60ac01'); +const COMMENT_UNDER_PRIVATE_PROJECT = id('58990017d334ee001d60ac02'); +const COMMENT_UNDER_STAFF_PERIOD = id('58990017d334ee001d60ac03'); +const COMMENT_WITH_NO_PERIOD = id('58990017d334ee001d60ac04'); +const COMMENT_UNDER_DELETED_PERIOD = id('58990017d334ee001d60ac05'); +const ANONYMOUS_COMMENT = id('58990017d334ee001d60ac06'); +const NAMED_COMMENT = id('58990017d334ee001d60ac07'); + +const period = (_id, project, read) => ({ _id, _schemaName: 'CommentPeriod', project, read }); +// Every comment is Published in its own right; only its period or project differs. +const comment = (_id, periodId) => ({ + _id, + _schemaName: 'Comment', + period: periodId, + comment: 'Published words', + read: PUBLIC_READ, + write: STAFF_ONLY, + delete: STAFF_ONLY +}); + +const FIXTURES = [ + ...PARENT_FIXTURES, + period(CP_UNDER_PUBLIC_PROJECT, PUBLIC_PROJECT, PUBLIC_READ), + period(CP_UNDER_PRIVATE_PROJECT, PRIVATE_PROJECT, PUBLIC_READ), + period(STAFF_CP_UNDER_PUBLIC_PROJECT, PUBLIC_PROJECT, STAFF_ONLY), + comment(COMMENT_UNDER_PUBLIC_PERIOD, CP_UNDER_PUBLIC_PROJECT), + comment(COMMENT_UNDER_PRIVATE_PROJECT, CP_UNDER_PRIVATE_PROJECT), + comment(COMMENT_UNDER_STAFF_PERIOD, STAFF_CP_UNDER_PUBLIC_PROJECT), + comment(COMMENT_WITH_NO_PERIOD, null), + comment(COMMENT_UNDER_DELETED_PERIOD, DELETED_PERIOD), + period(AUTHORS_CP, PUBLIC_PROJECT, PUBLIC_READ), + { ...comment(ANONYMOUS_COMMENT, AUTHORS_CP), author: 'Anon Person', isAnonymous: true }, + { ...comment(NAMED_COMMENT, AUTHORS_CP), author: 'Named Person', isAnonymous: false } +]; + +function capture() { + const body = { headers: {} }; + const res = { + status(code) { body.code = code; return this; }, + json(data) { body.data = data; return this; }, + send(data) { body.data = data; return this; }, + setHeader(name, value) { body.headers[name] = value; } + }; + return { res, body }; +} + +const periodArgs = (periodId, fields = ['comment']) => ({ + swagger: { + params: { + period: { value: periodId ? String(periodId) : undefined }, + fields: { value: fields }, + sortBy: { value: [] }, + pageSize: { value: 100 }, + pageNum: { value: 0 }, + count: { value: true } + }, + operation: { 'x-security-scopes': ['public'] } + } +}); + +const byIdArgs = (commentId, fields = ['comment']) => ({ + swagger: { params: { commentId: { value: String(commentId) }, fields: { value: fields } } } +}); + +async function call(handler, args) { + const { res, body } = capture(); + await handler(args, res); + return body; +} + +const ids = rows => (rows || []).map(row => String(row._id)); + +describe('public comment parent visibility (requires MongoDB)', function () { + this.timeout(20000); + + before(async () => { + await mongoose.connect(TEST_URI); + await mongoose.connection.collection('epic').deleteMany({}); + await mongoose.connection.collection('epic').insertMany(FIXTURES); + }); + + after(async () => { + await mongoose.connection.collection('epic').deleteMany({}); + await mongoose.disconnect(); + }); + + beforeEach(() => { + // recordAction writes an audit row; irrelevant here and it would pollute the collection. + sinon.stub(Utils, 'recordAction').resolves(); + }); + + afterEach(() => sinon.restore()); + + [ + ['an unpublished project', CP_UNDER_PRIVATE_PROJECT, COMMENT_UNDER_PRIVATE_PROJECT], + ['an unpublished period', STAFF_CP_UNDER_PUBLIC_PROJECT, COMMENT_UNDER_STAFF_PERIOD], + ['a deleted period', DELETED_PERIOD, COMMENT_UNDER_DELETED_PERIOD] + ].forEach(([label, periodId, commentId]) => { + it(`lists no comments for a period under ${label}`, async () => { + const body = await call(commentController.publicGet, periodArgs(periodId)); + + expect(body.code).to.equal(200); + expect(body.headers['x-total-count']).to.equal(0); + expect(body.data).to.deep.equal([]); + }); + + it(`counts no comments for a period under ${label}`, async () => { + const body = await call(commentController.publicHead, periodArgs(periodId)); + + expect(body.headers['x-total-count']).to.equal(0); + }); + + it(`returns nothing for a comment by id under ${label}`, async () => { + const body = await call(commentController.publicGet, byIdArgs(commentId)); + + expect(body.code).to.equal(200); + expect(body.data).to.deep.equal([]); + }); + }); + + it('still lists, counts and returns a comment under a public period and project', async () => { + const list = await call(commentController.publicGet, periodArgs(CP_UNDER_PUBLIC_PROJECT)); + const head = await call(commentController.publicHead, periodArgs(CP_UNDER_PUBLIC_PROJECT)); + const one = await call(commentController.publicGet, byIdArgs(COMMENT_UNDER_PUBLIC_PERIOD)); + + expect(ids(list.data)).to.deep.equal([String(COMMENT_UNDER_PUBLIC_PERIOD)]); + expect(head.headers['x-total-count']).to.equal(1); + expect(ids(one.data)).to.deep.equal([String(COMMENT_UNDER_PUBLIC_PERIOD)]); + }); + + it('returns nothing for a comment by id that has no period', async () => { + const body = await call(commentController.publicGet, byIdArgs(COMMENT_WITH_NO_PERIOD)); + + expect(body.data).to.deep.equal([]); + }); + + it('lists and counts, across all periods, only comments under a period the public can read', async () => { + const list = await call(commentController.publicGet, periodArgs(null)); + const head = await call(commentController.publicHead, periodArgs(null)); + + expect(ids(list.data)).to.have.members([String(COMMENT_UNDER_PUBLIC_PERIOD), String(ANONYMOUS_COMMENT), String(NAMED_COMMENT)]); + expect(head.headers['x-total-count']).to.equal(3); + }); + + it('shows the author only where the commenter chose to be named, in a list and by id', async () => { + const list = await call(commentController.publicGet, periodArgs(AUTHORS_CP, ['author', 'comment'])); + const anonymous = await call(commentController.publicGet, byIdArgs(ANONYMOUS_COMMENT, ['author'])); + const named = await call(commentController.publicGet, byIdArgs(NAMED_COMMENT, ['author'])); + + const authors = Object.fromEntries(list.data.map(row => [String(row._id), row.author])); + expect(authors).to.deep.equal({ [String(ANONYMOUS_COMMENT)]: undefined, [String(NAMED_COMMENT)]: 'Named Person' }); + expect(anonymous.data[0]).to.not.have.property('author'); + expect(named.data[0].author).to.equal('Named Person'); + }); +}); diff --git a/test/db/itemParentRead.test.js b/test/db/itemParentRead.test.js index 8c1f801a..1c54bdfe 100644 --- a/test/db/itemParentRead.test.js +++ b/test/db/itemParentRead.test.js @@ -50,6 +50,31 @@ const COMMENT_UNDER_PUBLIC_PROJECT = id('58990017d334ee001d609c01'); const COMMENT_UNDER_PRIVATE_PROJECT = id('58990017d334ee001d609c02'); const COMMENT_UNDER_STAFF_PERIOD = id('58990017d334ee001d609c03'); const COMMENT_WITH_NO_PERIOD = id('58990017d334ee001d609c04'); +const COMMENT_UNDER_DELETED_PERIOD = id('58990017d334ee001d609c06'); +const DELETED_PERIOD = id('58990017d334ee001d609bff'); + +// Published, under a public period and project, carrying every field a public caller must not see. +const COMMENT_WITH_STAFF_FIELDS = id('58990017d334ee001d609c05'); +const COMMENT_HIDDEN_FIELDS = { + author: 'Jane Private', + isAnonymous: true, + eaoNotes: 'staff note', + eaoStatus: 'Published', + proponentNotes: 'proponent note', + rejectedNotes: 'rejected note', + rejectedReason: 'rejected reason', + demiPushPending: true, + demiPushFailedAt: new Date('2026-10-01T00:00:00Z'), + demiPushError: 'push error', + _addedBy: 'staff-user', + _updatedBy: 'staff-user', + _deletedBy: 'staff-user' +}; + +// Records eagle-admin opens through dataset=Item, staff-only so only a staff caller reads them. +const ADMIN_USER = id('58990017d334ee001d609e01'); +const ADMIN_ORGANIZATION = id('58990017d334ee001d609e02'); +const ADMIN_UPDATE = id('58990017d334ee001d609e03'); const VC_UNDER_PUBLIC_PROJECT = id('58990017d334ee001d609d01'); const VC_UNDER_PRIVATE_PROJECT = id('58990017d334ee001d609d02'); @@ -77,8 +102,13 @@ const FIXTURES = [ child(COMMENT_UNDER_PRIVATE_PROJECT, 'Comment', 'period', CP_UNDER_PRIVATE_PROJECT), child(COMMENT_UNDER_STAFF_PERIOD, 'Comment', 'period', STAFF_CP_UNDER_PUBLIC_PROJECT), child(COMMENT_WITH_NO_PERIOD, 'Comment', 'period', null), + child(COMMENT_UNDER_DELETED_PERIOD, 'Comment', 'period', DELETED_PERIOD), + { ...child(COMMENT_WITH_STAFF_FIELDS, 'Comment', 'period', CP_UNDER_PUBLIC_PROJECT), comment: 'Hello', ...COMMENT_HIDDEN_FIELDS }, child(VC_UNDER_PUBLIC_PROJECT, 'Vc', 'project', PUBLIC_PROJECT), - child(VC_UNDER_PRIVATE_PROJECT, 'Vc', 'project', PRIVATE_PROJECT) + child(VC_UNDER_PRIVATE_PROJECT, 'Vc', 'project', PRIVATE_PROJECT), + { _id: ADMIN_USER, _schemaName: 'User', read: STAFF_ONLY, firstName: 'Ada' }, + { _id: ADMIN_ORGANIZATION, _schemaName: 'Organization', read: STAFF_ONLY, name: 'Org' }, + { ...child(ADMIN_UPDATE, 'RecentActivity', 'project', PUBLIC_PROJECT, STAFF_ONLY), status: 'draft' } ]; function itemArgs(roles, schemaName, itemId) { @@ -222,10 +252,14 @@ describe('item parent visibility (requires MongoDB)', function () { expect(idsIn(body.data)).to.deep.equal([String(COMMENT_UNDER_PUBLIC_PROJECT)]); }); - it('keeps a comment whose period reference resolves to nothing', async () => { - const body = await anonymousGet('Comment', COMMENT_WITH_NO_PERIOD); + [['no period', COMMENT_WITH_NO_PERIOD], ['a deleted period', COMMENT_UNDER_DELETED_PERIOD]].forEach(([label, commentId]) => { + it(`hides a comment with ${label} from an anonymous caller, and still returns it to staff`, async () => { + const anonymous = await anonymousGet('Comment', commentId); + const staff = await staffGet('Comment', commentId); - expect(idsIn(body.data)).to.deep.equal([String(COMMENT_WITH_NO_PERIOD)]); + expect(idsIn(anonymous.data)).to.be.empty; + expect(idsIn(staff.data)).to.deep.equal([String(commentId)]); + }); }); }); @@ -250,6 +284,54 @@ describe('item parent visibility (requires MongoDB)', function () { }); }); + // Every model shares the epic collection, so the requested name must match the stored one. + describe('a _schemaName that is not the stored record\'s', () => { + ['List', 'Organization', 'Topic', 'Document'].forEach(schemaName => { + it(`returns nothing when a comment is asked for as ${schemaName}`, async () => { + const body = await anonymousGet(schemaName, COMMENT_WITH_STAFF_FIELDS); + + expect(body.code).to.equal(200); + expect(body.data).to.deep.equal([]); + }); + }); + + it('returns nothing for a comment under an unpublished project asked for as List', async () => { + const body = await anonymousGet('List', COMMENT_UNDER_PRIVATE_PROJECT); + + expect(body.data).to.deep.equal([]); + }); + + it('returns nothing when a project is asked for as List', async () => { + const body = await anonymousGet('List', PUBLIC_PROJECT); + + expect(body.data).to.deep.equal([]); + }); + + it('still returns the comment asked for under its own name, without the hidden fields', async () => { + const body = await anonymousGet('Comment', COMMENT_WITH_STAFF_FIELDS); + + expect(idsIn(body.data)).to.deep.equal([String(COMMENT_WITH_STAFF_FIELDS)]); + expect(body.data[0].comment).to.equal('Hello'); + expect(body.data[0]).to.not.have.any.keys('write', 'delete', ...Object.keys(COMMENT_HIDDEN_FIELDS).filter(f => f !== 'isAnonymous')); + }); + }); + + describe('staff reads of the schemas eagle-admin opens by id', () => { + [ + ['User', ADMIN_USER], + ['Organization', ADMIN_ORGANIZATION], + ['RecentActivity', ADMIN_UPDATE], + ['ProjectNotification', PRIVATE_NOTIFICATION] + ].forEach(([schemaName, itemId]) => { + it(`returns a ${schemaName} asked for under its own name`, async () => { + const body = await staffGet(schemaName, itemId); + + expect(body.code).to.equal(200); + expect(idsIn(body.data)).to.deep.equal([String(itemId)]); + }); + }); + }); + // A project has no parent, so the gate must not touch it. describe('_schemaName=Project', () => { it('returns a published project to an anonymous caller', async () => {