Skip to content

Commit f87d08c

Browse files
sliftyslifty
committed
Prevent json encoding of forbidden proposal_field_values
We never want to return `FORBIDDEN` ProposalFieldValues. The primary way to accomplish that is to ensure our queries appropriately condition against selecting those values in the first place. This commit is a "belt and suspenders" approach which is only taken on because of the level of sensitivity that this date represents. If a query attempts to select a forbidden ProposalFieldValue it will result in a database exception. This doesn't completely prevent access (e.g. if a query accesses field values without creating an entire row), but it is still a layer of defense. Issue #1621 Add the concept of "too sensitive to store data" to base fields
1 parent 0ae979b commit f87d08c

File tree

3 files changed

+134
-16
lines changed

3 files changed

+134
-16
lines changed

‎src/__tests__/proposalVersions.int.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
createChangemaker,
1717
createOrUpdateUserChangemakerPermission,
1818
createChangemakerProposal,
19+
createProposalFieldValue,
1920
} from '../database';
2021
import { getLogger } from '../logger';
2122
import {
@@ -457,6 +458,23 @@ describe('/proposalVersions', () => {
457458
label: 'Forbidden Field',
458459
},
459460
);
461+
await createProposal(db, testUserAuthContext, {
462+
externalId: `proposal-2525-01-04T00Z`,
463+
opportunityId: 1,
464+
});
465+
await createProposalVersion(db, testUserAuthContext, {
466+
proposalId: 1,
467+
applicationFormId: 1,
468+
sourceId: systemSource.id,
469+
});
470+
await createProposalFieldValue(db, null, {
471+
proposalVersionId: 1,
472+
applicationFormFieldId: forbiddenApplicationFormField.id,
473+
position: 1,
474+
value: 'Should not be returned',
475+
isValid: true,
476+
goodAsOf: null,
477+
});
460478
await createOrUpdateBaseField(db, null, {
461479
...forbiddenBaseField,
462480
sensitivityClassification: BaseFieldSensitivityClassification.FORBIDDEN,

‎src/database/initialization/__tests__/proposal_field_value_to_json.int.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import {
2+
createApplicationForm,
3+
createApplicationFormField,
4+
createOpportunity,
5+
createOrUpdateBaseField,
6+
createProposal,
7+
createProposalFieldValue,
8+
createProposalVersion,
9+
db,
10+
loadSystemFunder,
11+
loadSystemSource,
12+
} from '../..';
13+
import { getAuthContext, loadTestUser } from '../../../test/utils';
14+
import {
15+
BaseFieldDataType,
16+
BaseFieldScope,
17+
BaseFieldSensitivityClassification,
18+
} from '../../../types';
19+
20+
describe('/proposal_field_value_to_json', () => {
21+
it('returns a db error if attempting to load a forbidden proposal_field_value_to_json', async () => {
22+
const testUser = await loadTestUser();
23+
const testUserAuthContext = getAuthContext(testUser);
24+
const systemSource = await loadSystemSource(db, null);
25+
const systemFunder = await loadSystemFunder(db, null);
26+
await createOpportunity(db, null, {
27+
title: '🔥',
28+
funderShortCode: systemFunder.shortCode,
29+
});
30+
await createProposal(db, testUserAuthContext, {
31+
externalId: 'proposal-1',
32+
opportunityId: 1,
33+
});
34+
await createApplicationForm(db, null, {
35+
opportunityId: 1,
36+
});
37+
const forbiddenBaseField = await createOrUpdateBaseField(db, null, {
38+
label: 'Forbidden Field',
39+
description: 'This field should not be used in proposal versions',
40+
shortCode: 'forbiddenField',
41+
dataType: BaseFieldDataType.STRING,
42+
scope: BaseFieldScope.PROPOSAL,
43+
valueRelevanceHours: null,
44+
sensitivityClassification: BaseFieldSensitivityClassification.RESTRICTED,
45+
});
46+
const forbiddenApplicationFormField = await createApplicationFormField(
47+
db,
48+
null,
49+
{
50+
applicationFormId: 1,
51+
baseFieldShortCode: forbiddenBaseField.shortCode,
52+
position: 1,
53+
label: 'Forbidden Field',
54+
},
55+
);
56+
await createProposal(db, testUserAuthContext, {
57+
externalId: `proposal-2525-01-04T00Z`,
58+
opportunityId: 1,
59+
});
60+
await createProposalVersion(db, testUserAuthContext, {
61+
proposalId: 1,
62+
applicationFormId: 1,
63+
sourceId: systemSource.id,
64+
});
65+
await createProposalFieldValue(db, null, {
66+
proposalVersionId: 1,
67+
applicationFormFieldId: forbiddenApplicationFormField.id,
68+
position: 1,
69+
value: 'Should not be returned',
70+
isValid: true,
71+
goodAsOf: null,
72+
});
73+
await createOrUpdateBaseField(db, null, {
74+
...forbiddenBaseField,
75+
sensitivityClassification: BaseFieldSensitivityClassification.FORBIDDEN,
76+
});
77+
78+
await expect(
79+
db.query(
80+
'SELECT proposal_field_value_to_json(proposal_field_values.*) FROM proposal_field_values WHERE proposal_field_values.id = 1',
81+
),
82+
).rejects.toThrow(
83+
'Attempt to convert forbidden proposal_field_value to JSON (1)',
84+
);
85+
});
86+
});

‎src/database/initialization/proposal_field_value_to_json.sql

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,37 @@ CREATE FUNCTION proposal_field_value_to_json(
55
)
66
RETURNS jsonb AS $$
77
DECLARE
8-
application_form_field_json JSONB;
8+
is_forbidden BOOLEAN;
9+
application_form_field_json JSONB;
910
BEGIN
10-
SELECT application_form_field_to_json(application_form_fields.*)
11-
INTO application_form_field_json
12-
FROM application_form_fields
13-
WHERE application_form_fields.id = proposal_field_value.application_form_field_id;
11+
SELECT EXISTS (
12+
select 1
13+
FROM application_form_fields
14+
JOIN base_fields ON application_form_fields.base_field_short_code = base_fields.short_code
15+
WHERE application_form_fields.id = proposal_field_value.application_form_field_id
16+
AND base_fields.sensitivity_classification = 'forbidden'
17+
) INTO is_forbidden;
1418

15-
RETURN jsonb_build_object(
16-
'id', proposal_field_value.id,
17-
'proposalVersionId', proposal_field_value.proposal_version_id,
18-
'applicationFormFieldId', proposal_field_value.application_form_field_id,
19-
'applicationFormField', application_form_field_json,
20-
'position', proposal_field_value.position,
21-
'value', proposal_field_value.value,
22-
'goodAsOf', proposal_field_value.good_as_of,
23-
'isValid', proposal_field_value.is_valid,
24-
'createdAt', proposal_field_value.created_at
25-
);
19+
IF is_forbidden THEN
20+
RAISE EXCEPTION 'Attempt to convert forbidden proposal_field_value to JSON (%)', proposal_field_value.id
21+
USING ERRCODE = '22023'; -- invalid_parameter_value
22+
END IF;
23+
24+
SELECT application_form_field_to_json(application_form_fields.*)
25+
INTO application_form_field_json
26+
FROM application_form_fields
27+
WHERE application_form_fields.id = proposal_field_value.application_form_field_id;
28+
29+
RETURN jsonb_build_object(
30+
'id', proposal_field_value.id,
31+
'proposalVersionId', proposal_field_value.proposal_version_id,
32+
'applicationFormFieldId', proposal_field_value.application_form_field_id,
33+
'applicationFormField', application_form_field_json,
34+
'position', proposal_field_value.position,
35+
'value', proposal_field_value.value,
36+
'goodAsOf', proposal_field_value.good_as_of,
37+
'isValid', proposal_field_value.is_valid,
38+
'createdAt', proposal_field_value.created_at
39+
);
2640
END;
2741
$$ LANGUAGE plpgsql;

0 commit comments

Comments
 (0)