diff --git a/api/src/appRequest/appRequest.database.ts b/api/src/appRequest/appRequest.database.ts index 7c996d84..71309ce8 100644 --- a/api/src/appRequest/appRequest.database.ts +++ b/api/src/appRequest/appRequest.database.ts @@ -121,8 +121,12 @@ function processFilters (filter?: AppRequestFilter) { joins.set('u', 'INNER JOIN accessUsers u ON u.id = ar.userId') joins.set('t', 'LEFT JOIN app_request_tags t ON t.appRequestId = ar.id') joins.set('tl', 'LEFT JOIN tag_labels tl ON tl.category = t.category AND tl.tag = t.tag') - where.push('(p.name LIKE ? OR u.login LIKE ? OR u.fullname LIKE ? OR t.tag LIKE ? OR tl.label LIKE ?)') binds.push(`${filter.search}%`, `${filter.search}%`, `%${filter.search}%`, `${filter.search}%`, `${filter.search}%`) + if (filter.searchNotes) { + joins.set('arn', 'LEFT JOIN app_request_notes arn ON arn.appRequestId = ar.id') + binds.push(filter.search) + } + where.push(`(p.name LIKE ? OR u.login LIKE ? OR u.fullname LIKE ? OR t.tag LIKE ? OR tl.label LIKE ?${filter.searchNotes ? ' OR MATCH(arn.content) AGAINST (? IN NATURAL LANGUAGE MODE)' : ''})`) } if (filter?.createdAfter) { where.push('ar.createdAt >= ?') diff --git a/api/src/appRequest/appRequest.model.ts b/api/src/appRequest/appRequest.model.ts index 605f2802..52c7dffb 100644 --- a/api/src/appRequest/appRequest.model.ts +++ b/api/src/appRequest/appRequest.model.ts @@ -225,6 +225,10 @@ export class AppRequestActions {} @InputType() export class AppRequestFilter { + constructor () { + this.searchNotes = false + } + @Field(type => [ID], { nullable: true }) ids?: string[] @@ -252,6 +256,8 @@ export class AppRequestFilter { @Field(type => String, { nullable: true, description: 'Search for appRequests that match this search term. This will do a prefix search across all fields that are indexed.' }) search?: string + searchNotes?: boolean + @Field(type => DateTime, { nullable: true, description: 'Only return appRequests that were created after this date.' }) createdAfter?: DateTime diff --git a/api/src/appRequest/appRequest.service.ts b/api/src/appRequest/appRequest.service.ts index 042ba5dc..2212b14d 100644 --- a/api/src/appRequest/appRequest.service.ts +++ b/api/src/appRequest/appRequest.service.ts @@ -108,8 +108,13 @@ export class AppRequestService extends AuthService { protected raw = this.svc(AppRequestServiceInternal) preprocessFilter (filter?: AppRequestFilter) { - filter ??= {} + filter ??= new AppRequestFilter() if (filter.own) filter.userInternalIds = [this.user?.internalId ?? -1] + // Notes are reviewer-only and never visible to applicants. + // non-reviewers should not be able to probe the existence/content of notes + // (even on their own requests) by observing which requests match. + // TODO: May need to add a mayViewNotes method. + if (this.mayViewReviewerInterface()) filter.searchNotes = true return filter } diff --git a/api/src/notes/notes.initialize.ts b/api/src/notes/notes.initialize.ts index d03a12b9..c10f60c8 100644 --- a/api/src/notes/notes.initialize.ts +++ b/api/src/notes/notes.initialize.ts @@ -23,5 +23,15 @@ export const noteMigrations: DatabaseMigration[] = [ const exists = await db.getval("SELECT 1 FROM information_schema.COLUMNS WHERE TABLE_NAME = 'app_request_notes' AND COLUMN_NAME = 'persistent'") if (!exists) await db.execute('ALTER TABLE app_request_notes ADD COLUMN persistent TINYINT(1) NOT NULL DEFAULT 0') } + }, + { + id: '20260617120000', + async execute (db: Queryable) { + // NOTE: Requires mysql version 5.7 which our systems support (may even be supported earlier) + // https://dev.mysql.com/doc/refman/5.7/en/fulltext-natural-language.html + // However if this is an issue we can revert to searching with LIKEs. + const exists = await db.getval("SELECT 1 FROM information_schema.STATISTICS WHERE TABLE_NAME = 'app_request_notes' AND INDEX_NAME = 'ft_content'") + if (!exists) await db.execute('ALTER TABLE app_request_notes ADD FULLTEXT INDEX ft_content (content)') + } } ] diff --git a/test/tests/default.notes.test.ts b/test/tests/default.notes.test.ts index 7d44ebdb..44b3b0bb 100644 --- a/test/tests/default.notes.test.ts +++ b/test/tests/default.notes.test.ts @@ -70,6 +70,17 @@ const NOTES_QUERY = ` } ` +const SEARCH_APP_REQUESTS_QUERY = ` + query SearchAppRequests($search: String!) { + appRequests(filter: { search: $search }) { + id + } + } +` + +// Search term only to be found in notes for reviewer-gated note-content search test. +const NOTE_SEARCH_MARKER = 'txstsearchmarker' + function warningMessages (messages: { message: string, type: string }[]) { return messages.filter(m => m.type === 'warning').map(m => m.message) } @@ -482,4 +493,26 @@ test.describe.serial('Notes - XSS sanitization', { tag: '@default' }, () => { expect(after.appRequests[0].notes.find(n => n.id === noteId)).toBeTruthy() }) + test('Reviewer - can find app request by searching note content', async ({ reviewerRequest }) => { + const content = `Reviewer-only note for internal purposes only ... ref ${NOTE_SEARCH_MARKER}. Applicants should not be able to search for this note.` + const { addNote } = await reviewerRequest.graphql(ADD_NOTE_MUTATION, { appRequestId, content }) + expect(addNote.success).toEqual(true) + + const { appRequests } = await reviewerRequest.graphql<{ appRequests: { id: string }[] }>(SEARCH_APP_REQUESTS_QUERY, { search: NOTE_SEARCH_MARKER }) + expect(appRequests.map(r => String(r.id))).toContain(String(appRequestId)) + }) + + test('Applicant - cannot discover notes via search, but can find their own data', async ({ applicantRequest }) => { + const { appRequests } = await applicantRequest.graphql<{ appRequests: { id: string }[] }>(SEARCH_APP_REQUESTS_QUERY, { search: NOTE_SEARCH_MARKER }) + expect(appRequests.map(r => String(r.id))).not.toContain(String(appRequestId)) + + const byLogin = await applicantRequest.graphql<{ appRequests: { id: string }[] }>(SEARCH_APP_REQUESTS_QUERY, { search: applicantLogin }) + expect(byLogin.appRequests.map(r => String(r.id))).toContain(String(appRequestId)) + }) + + test('Applicant2 - cannot discover other applicant notes via search', async ({ applicant2Request }) => { + const { appRequests } = await applicant2Request.graphql<{ appRequests: { id: string }[] }>(SEARCH_APP_REQUESTS_QUERY, { search: NOTE_SEARCH_MARKER }) + expect(appRequests.map(r => String(r.id))).not.toContain(String(appRequestId)) + }) + })