Skip to content

Commit b4c18e3

Browse files
authored
feat: sgID to MyInfo migration (#1167)
## Problem FormSG is doing a deprecation of sgID authentication and moving towards Singpass/MyInfo authentication. As a result the field `sgidUinFin` will be replaced by `uinFin`. This will break existing pipes that rely on that variable. ## Solution To maintain backwards compatibility and to ensure that current pipes do not break, we have to duplicate `uinFin` to `sgidUinFin`. And to hide one of `sgidUinFin` variable in the variable list and suggestions popper so users dont get confused. This PR introduces another metadata key called `isHiddenFromList`. If true, this variable will not appear in the variable list but will still render properly if the variable is already selected and in use. ### Screenshots 1. Hidden from variable list <img width="406" height="219" alt="image" src="https://github.com/user-attachments/assets/be0591a1-85ba-4b79-96fa-678f8c76b16f" /> 2. Both variables (`uinFin` and `sgidUinFin` still displays properly) <img width="956" height="416" alt="image" src="https://github.com/user-attachments/assets/598fef6d-0e12-46e5-aa3a-fec3addc4d1a" />
1 parent 43bdd16 commit b4c18e3

File tree

7 files changed

+40
-30
lines changed

7 files changed

+40
-30
lines changed

packages/backend/src/apps/formsg/__tests__/auth/decrypt-form-response.test.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,6 @@ describe('decrypt form response', () => {
279279
],
280280
verified: {
281281
uinFin: 'S1234567A',
282-
sgidUinFin: 'S2345678B',
283282
cpUid: 'U987654323PLUMBER',
284283
},
285284
})
@@ -313,7 +312,7 @@ describe('decrypt form response', () => {
313312
},
314313
verifiedSubmitterInfo: {
315314
uinFin: 'S1234567A',
316-
sgidUinFin: 'S2345678B',
315+
sgidUinFin: 'S1234567A',
317316
cpUid: 'U987654323PLUMBER',
318317
},
319318
}),
@@ -371,7 +370,7 @@ describe('decrypt form response', () => {
371370
},
372371
verifiedSubmitterInfo: {
373372
uinFin: 'Z1cImQNbDXdmOaeS2roacWNxH7MbJC75OiEeYOjSbRo=',
374-
sgidUinFin: 'UAM3XbFrbNVVuD9Phz3KV/roZj4aG/Ql3Ap5Y5dTtJ4=',
373+
sgidUinFin: 'Z1cImQNbDXdmOaeS2roacWNxH7MbJC75OiEeYOjSbRo=',
375374
cpUid: 'U987654323PLUMBER',
376375
},
377376
}),
@@ -407,7 +406,7 @@ describe('decrypt form response', () => {
407406
},
408407
verifiedSubmitterInfo: {
409408
uinFin: 'xxxxx567A',
410-
sgidUinFin: 'xxxxx678B',
409+
sgidUinFin: 'xxxxx567A',
411410
cpUid: 'U987654323PLUMBER',
412411
},
413412
}),
@@ -426,7 +425,6 @@ describe('decrypt form response', () => {
426425
},
427426
],
428427
verified: {
429-
sgidUinFin: 'S1234567A',
430428
uinFin: '12345678B',
431429
cpUid: 'U987654323PLUMBER',
432430
cpUen: '987654321Z',
@@ -438,7 +436,7 @@ describe('decrypt form response', () => {
438436
expect($.request.body).toEqual(
439437
expect.objectContaining({
440438
verifiedSubmitterInfo: {
441-
sgidUinFin: 'S1234567A',
439+
sgidUinFin: '12345678B',
442440
uinFin: '12345678B',
443441
cpUid: 'U987654323PLUMBER',
444442
cpUen: '987654321Z',

packages/backend/src/apps/formsg/auth/decrypt-form-response.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,10 @@ export async function decryptFormResponse(
189189
}
190190

191191
verifiedSubmitterInfo[key] = value
192+
// for backwards compatibility with old forms that were created with sgID authType
193+
if (key === 'uinFin') {
194+
verifiedSubmitterInfo['sgidUinFin'] = value
195+
}
192196
}
193197
}
194198

packages/backend/src/apps/formsg/triggers/new-submission/get-data-out-metadata.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,10 @@ function buildAnswerArrayMetadatum(
222222
function buildVerifiedSubmitterInfoMetadata(
223223
data: IJSONObject,
224224
): IDataOutMetadata | null {
225-
if (!data.verifiedSubmitterInfo) {
225+
if (
226+
!data.verifiedSubmitterInfo ||
227+
typeof data.verifiedSubmitterInfo !== 'object'
228+
) {
226229
return null
227230
}
228231

@@ -234,7 +237,10 @@ function buildVerifiedSubmitterInfoMetadata(
234237
metadata.uinFin = { label: 'NRIC/FIN (Verified)' }
235238
break
236239
case 'sgidUinFin':
237-
metadata.sgidUinFin = { label: 'NRIC/FIN (Verified)' }
240+
metadata.sgidUinFin = {
241+
label: 'NRIC/FIN (Verified)',
242+
isHiddenFromList: 'uinFin' in data.verifiedSubmitterInfo,
243+
}
238244
break
239245
case 'cpUid':
240246
metadata.cpUid = { label: 'CorpPass UID (Verified)' }

packages/backend/src/apps/formsg/triggers/new-submission/get-mock-data.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,16 @@ function generateVerifiedSubmitterInfoData(
4040
): Record<string, Record<string, string>> {
4141
const filteredNric = filterNric($, MOCK_NRIC)
4242
switch (authType) {
43-
case 'SGID':
44-
case 'SGID_MyInfo':
45-
return {
46-
verifiedSubmitterInfo: {
47-
sgidUinFin: filteredNric,
48-
},
49-
}
50-
case 'SP':
43+
case 'SGID': // deprecated
44+
case 'SGID_MyInfo': // deprecated
45+
case 'SP': // deprecated
5146
case 'MyInfo':
47+
// for backwards compatibility with old forms that were created with sgID authType,
48+
// we need to return both uinFin and sgidUinFin
5249
return {
5350
verifiedSubmitterInfo: {
5451
uinFin: filteredNric,
52+
sgidUinFin: filteredNric,
5553
},
5654
}
5755
case 'CP':

packages/frontend/src/components/VariablesList/index.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import TableVariableItem from './TableVariableItem'
2525
function VariableTag({
2626
type,
2727
}: {
28-
type: TDataOutMetadatumType | null
28+
type?: TDataOutMetadatumType
2929
}): JSX.Element | null {
3030
const { label, tooltip } = useMemo(() => {
3131
switch (type) {
@@ -156,6 +156,9 @@ export default function VariablesList(props: VariablesListProps) {
156156
const defaultVariables: Variable[] = []
157157
const collapsedVariables: Variable[] = []
158158
for (const variable of variables) {
159+
if (variable.isHiddenFromList) {
160+
continue
161+
}
159162
if (variable.isCollapsedByDefault) {
160163
collapsedVariables.push(variable)
161164
} else {
@@ -165,7 +168,7 @@ export default function VariablesList(props: VariablesListProps) {
165168
return { defaultVariables, collapsedVariables }
166169
}, [variables])
167170

168-
if (!variables || variables.length === 0) {
171+
if (!variables || defaultVariables.length + collapsedVariables.length === 0) {
169172
return <></>
170173
}
171174

packages/frontend/src/helpers/variables.ts

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,7 @@ export interface StepWithVariables {
2424
output: Variable[]
2525
addNew?: boolean
2626
}
27-
export interface Variable {
28-
label: string | null
29-
type: TDataOutMetadatumType | null
30-
order: number | null
31-
displayedValue: string | null
32-
isCollapsedByDefault: boolean
27+
export interface Variable extends IDataOutMetadatum {
3328
/**
3429
* CAVEAT: not _just_ a name; it contains the lodash.get path for dataOut. Do
3530
* not clobber unless you know what you're doing!
@@ -38,12 +33,6 @@ export interface Variable {
3833
value: unknown
3934
// NOTE: used in some variables as unique key
4035
id?: string
41-
/**
42-
* NOTE: used to hide columns in the variables list, specifically for 'table' object
43-
* we still need them in the dataOut to compare parameters against the last test execution
44-
* at the for-each step
45-
*/
46-
isHidden?: boolean
4736
}
4837

4938
function sortVariables(variables: Variable[]): void {
@@ -83,6 +72,7 @@ const process = (
8372
order = null,
8473
displayedValue = null,
8574
isCollapsedByDefault = false,
75+
isHiddenFromList = false,
8676
} = metadata
8777

8878
if (isHidden) {
@@ -99,6 +89,7 @@ const process = (
9989
type,
10090
order,
10191
isCollapsedByDefault,
92+
isHiddenFromList, // only applies to text type for now
10293
},
10394
]
10495
}

packages/types/index.d.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ export interface IDataOutMetadatum {
7272

7373
/**
7474
* Generally defaults to `false` in the front end if unspecified.
75+
* NOTE: used to hide columns in the variables list, specifically for 'table' object
76+
* we still need them in the dataOut to compare parameters against the last test execution
77+
* at the for-each step
7578
*/
7679
isHidden?: boolean
7780

@@ -105,6 +108,13 @@ export interface IDataOutMetadatum {
105108
* the "Show more" section in the variable list.
106109
*/
107110
isCollapsedByDefault?: boolean
111+
112+
/**
113+
* Is hidden from the variable list but variable is still displayed properly if
114+
* already in use. For e.g. this is used to preserve the old `sgidUinFin` variable
115+
* but we dont want to show to UIN/FIN variables in variable list
116+
*/
117+
isHiddenFromList?: boolean
108118
}
109119

110120
export interface IDataOutMetadata {

0 commit comments

Comments
 (0)