Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion api/src/appRequest/appRequest.database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 >= ?')
Expand Down
6 changes: 6 additions & 0 deletions api/src/appRequest/appRequest.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,10 @@ export class AppRequestActions {}

@InputType()
export class AppRequestFilter {
constructor () {
this.searchNotes = false
}

@Field(type => [ID], { nullable: true })
ids?: string[]

Expand Down Expand Up @@ -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

Expand Down
7 changes: 6 additions & 1 deletion api/src/appRequest/appRequest.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,13 @@ export class AppRequestService extends AuthService<AppRequest> {
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
}

Expand Down
10 changes: 10 additions & 0 deletions api/src/notes/notes.initialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)')
}
}
]
33 changes: 33 additions & 0 deletions test/tests/default.notes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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<AddNoteResponse>(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))
})

})