From e0fbfdd571eb15efaae2ff2b4843119abc7de205 Mon Sep 17 00:00:00 2001 From: dmekala-va Date: Thu, 10 Oct 2024 10:26:15 -0500 Subject: [PATCH 1/5] initial changes --- dev-config.base.json | 3 ++- src/MpiUserClient.ts | 14 ++++++++++++-- src/cli/index.js | 7 +++++++ src/routes/acsHandlers.ts | 16 +++++++++++++++- 4 files changed, 36 insertions(+), 4 deletions(-) diff --git a/dev-config.base.json b/dev-config.base.json index 60692467..cad8c4f4 100644 --- a/dev-config.base.json +++ b/dev-config.base.json @@ -25,11 +25,12 @@ "sessionSecret": "fake-session-secret", "cacheEnabled": true, "redisPort": "6379", + "fraudBlockEnabled": true, "redisHost": "127.0.0.1", "spIdpSsoBinding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect", "idpSamlLoginsEnabled": true, "logStyleElementsEnabled": true, - "idpSamlLogins": + "idpSamlLogins": [ { "category": "example2SamlIdp", diff --git a/src/MpiUserClient.ts b/src/MpiUserClient.ts index 4a42e5a4..8da15f4a 100644 --- a/src/MpiUserClient.ts +++ b/src/MpiUserClient.ts @@ -15,7 +15,12 @@ export class MpiUserClient { public async getMpiTraitsForLoa3User( user: SAMLUser - ): Promise<{ icn: string; first_name: string; last_name: string }> { + ): Promise<{ + icn: string; + first_name: string; + last_name: string; + idTheftIndicator: boolean; + }> { const body: Record = { idp_uuid: user.uuid, dslogon_edipi: user.edipi || null, @@ -38,7 +43,12 @@ export class MpiUserClient { }) .then((response) => { const data = response.data.data; - return data.attributes; + return { + icn: data.attributes.icn, + first_name: data.attributes.first_name, + last_name: data.attributes.last_name, + idTheftIndicator: data.id_theft_indicator || false, + }; }) .catch(() => { throw { diff --git a/src/cli/index.js b/src/cli/index.js index d6f3c573..146076ac 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -41,6 +41,13 @@ export function processArgs() { KEY_CERT_HELP_TEXT ), }, + fraudBlockEnabled: { + description: + "Enable or disable blocking logins based on the fraud identity indicator", + required: false, + boolean: true, + default: true, + }, idpKey: { description: "IdP Signature PrivateKey Certificate", required: true, diff --git a/src/routes/acsHandlers.ts b/src/routes/acsHandlers.ts index 5542cecc..fc398d96 100644 --- a/src/routes/acsHandlers.ts +++ b/src/routes/acsHandlers.ts @@ -22,6 +22,8 @@ import { } from "../metrics"; import rTracer from "cls-rtracer"; import { selectPassportStrategyKey } from "./passport"; +import { processArgs } from "../cli"; +const argv = processArgs(); const unknownUsersErrorTemplate = (error: any) => { // `error` comes from: @@ -123,7 +125,12 @@ export const loadICN = async ( const action = "loadICN"; try { - const { icn, first_name, last_name } = await requestWithMetrics( + const { + icn, + first_name, + last_name, + idTheftIndicator, + } = await requestWithMetrics( MVIRequestMetrics, (): Promise => { return req.mpiUserClient.getMpiTraitsForLoa3User(req.user.claims); @@ -135,6 +142,13 @@ export const loadICN = async ( action, result: "success", }); + + if (argv.fraudBlockEnabled && idTheftIndicator) { + logger.warn("Fradulent identity detected, blocking login."); + return res.render("sensitive_error", { + request_id: rTracer.id(), + }); + } req.user.claims.icn = icn; if (first_name) { req.user.claims.firstName = first_name; From c06cc362c68c8537981a4b8ff2851f3b9bc924ff Mon Sep 17 00:00:00 2001 From: dmekala-va Date: Thu, 10 Oct 2024 14:33:35 -0500 Subject: [PATCH 2/5] working id theft change --- dev-config.base.json | 3 +-- src/MpiUserClient.ts | 9 ++++++++- src/MpiUserClientConfig.js | 1 + src/app.js | 3 ++- src/routes/acsHandlers.ts | 7 +++---- test/testServer.js | 1 + 6 files changed, 16 insertions(+), 8 deletions(-) diff --git a/dev-config.base.json b/dev-config.base.json index cad8c4f4..60692467 100644 --- a/dev-config.base.json +++ b/dev-config.base.json @@ -25,12 +25,11 @@ "sessionSecret": "fake-session-secret", "cacheEnabled": true, "redisPort": "6379", - "fraudBlockEnabled": true, "redisHost": "127.0.0.1", "spIdpSsoBinding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect", "idpSamlLoginsEnabled": true, "logStyleElementsEnabled": true, - "idpSamlLogins": + "idpSamlLogins": [ { "category": "example2SamlIdp", diff --git a/src/MpiUserClient.ts b/src/MpiUserClient.ts index 8da15f4a..3eff32b0 100644 --- a/src/MpiUserClient.ts +++ b/src/MpiUserClient.ts @@ -4,13 +4,20 @@ import axios from "axios"; export class MpiUserClient { mpiUserEndpoint: string; headers: object; + fraudIdTheft: boolean; - constructor(apiKey: string, mpiUserEndpoint: string, accessKey: string) { + constructor( + apiKey: string, + mpiUserEndpoint: string, + accessKey: string, + fraudIdTheft: boolean + ) { this.mpiUserEndpoint = mpiUserEndpoint; this.headers = { apiKey: apiKey, accesskey: accessKey, }; + this.fraudIdTheft = fraudIdTheft; } public async getMpiTraitsForLoa3User( diff --git a/src/MpiUserClientConfig.js b/src/MpiUserClientConfig.js index 5226b43c..3d099293 100644 --- a/src/MpiUserClientConfig.js +++ b/src/MpiUserClientConfig.js @@ -3,5 +3,6 @@ export default class MpiUserClientConfig { this.mpiUserEndpoint = argv.mpiUserEndpoint; this.accessKey = argv.accessKey; this.apiKey = argv.vetsAPIToken; + this.fraudIdTheft = argv.fraudBlockEnabled || false; } } diff --git a/src/app.js b/src/app.js index 7b22ab80..dfe022ca 100644 --- a/src/app.js +++ b/src/app.js @@ -101,7 +101,8 @@ function runServer(argv) { const mpiUserClient = new MpiUserClient( mpiUserClientConfig.apiKey, mpiUserClientConfig.mpiUserEndpoint, - mpiUserClientConfig.accessKey + mpiUserClientConfig.accessKey, + mpiUserClientConfig.fraudIdTheft ); const vsoClient = new VsoClient( vsoConfig.token, diff --git a/src/routes/acsHandlers.ts b/src/routes/acsHandlers.ts index fc398d96..f0d70194 100644 --- a/src/routes/acsHandlers.ts +++ b/src/routes/acsHandlers.ts @@ -22,8 +22,6 @@ import { } from "../metrics"; import rTracer from "cls-rtracer"; import { selectPassportStrategyKey } from "./passport"; -import { processArgs } from "../cli"; -const argv = processArgs(); const unknownUsersErrorTemplate = (error: any) => { // `error` comes from: @@ -143,9 +141,10 @@ export const loadICN = async ( result: "success", }); - if (argv.fraudBlockEnabled && idTheftIndicator) { + if (req.mpiUserClient.fraudIdTheft && idTheftIndicator) { logger.warn("Fradulent identity detected, blocking login."); - return res.render("sensitive_error", { + return res.render("layout", { + body: "sensitive_error", request_id: rTracer.id(), }); } diff --git a/test/testServer.js b/test/testServer.js index 3fbe943f..ffba133a 100644 --- a/test/testServer.js +++ b/test/testServer.js @@ -36,6 +36,7 @@ const defaultTestingConfig = { spValidateNameIDFormat: true, spNameIDFormat: "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", spRequestAuthnContext: true, + fraudBlockEnabled: true, }; export const idpConfig = new IDPConfig(defaultTestingConfig); From aae852d5e7f326674bd2cc218ccbd8455527ffef Mon Sep 17 00:00:00 2001 From: dmekala-va Date: Fri, 11 Oct 2024 08:56:22 -0500 Subject: [PATCH 3/5] adding unit tests --- src/MpiUserClient.test.ts | 18 +++++++---- src/routes/acsHandlers.test.ts | 59 +++++++++++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/MpiUserClient.test.ts b/src/MpiUserClient.test.ts index e9d5cd0d..cc397912 100644 --- a/src/MpiUserClient.test.ts +++ b/src/MpiUserClient.test.ts @@ -50,6 +50,7 @@ beforeEach(() => { icn: "fakeICN", first_name: "Edward", last_name: "Paget", + id_theft_indicator: true, }, }, }, @@ -62,7 +63,8 @@ describe("getMVITraitsForLoa3User", () => { const client = new MpiUserClient( "faketoken", "https://example.gov/mvi-user", - "faketoken" + "faketoken", + true ); await client.getMpiTraitsForLoa3User(samlTraitsEDIPI); expect(axios.post).toHaveBeenCalledWith( @@ -89,7 +91,8 @@ describe("getMVITraitsForLoa3User", () => { const client = new MpiUserClient( "faketoken", "https://example.gov/mpi-user", - "faketoken" + "faketoken", + true ); await client.getMpiTraitsForLoa3User(samlTraitsICN); expect(axios.post).toHaveBeenCalledWith( @@ -111,7 +114,8 @@ describe("getMVITraitsForLoa3User", () => { const client = new MpiUserClient( "faketoken", "https://example.gov/mpi-user", - "faketoken" + "faketoken", + true ); await client.getMpiTraitsForLoa3User(samlTraits); expect(axios.post).toHaveBeenCalledWith( @@ -138,9 +142,11 @@ describe("getMVITraitsForLoa3User", () => { const client = new MpiUserClient( "faketoken", "https://example.gov", - "faketoken" + "faketoken", + false ); - const { icn } = await client.getMpiTraitsForLoa3User(samlTraits); - expect(icn).toEqual("fakeICN"); + const mpiTraits = await client.getMpiTraitsForLoa3User(samlTraits); + expect(mpiTraits.icn).toEqual("fakeICN"); + expect(mpiTraits.idTheftIndicator).toEqual(false); }); }); diff --git a/src/routes/acsHandlers.test.ts b/src/routes/acsHandlers.test.ts index bf6bff94..d3bcb122 100644 --- a/src/routes/acsHandlers.test.ts +++ b/src/routes/acsHandlers.test.ts @@ -25,7 +25,8 @@ const vsoClient = new VsoClient("fakeToken", "https://example.gov"); const mpiUserClient = new MpiUserClient( "fakeToken", "http://example.com/mpiuser", - "fakekey" + "fakekey", + true ); // Technically Doesn't TypeCheck, but typechecking is off for test files @@ -211,6 +212,8 @@ describe("scrubUserClaims", () => { }); }); + + describe("loadICN", () => { beforeEach(() => { // @ts-ignore @@ -219,6 +222,60 @@ describe("loadICN", () => { vsoClient.getVSOSearch.mockReset(); }); + it("should block login when fraudIdTheft is true and idTheftIndicator is true", async () => { + const nextFn = jest.fn(); + const renderMock = jest.fn(); + const req: any = { + mpiUserClient: { ...mpiUserClient, fraudIdTheft: true }, + vsoClient: vsoClient, + user: { + claims: { ...claimsWithICN }, + }, + }; + + req.mpiUserClient.getMpiTraitsForLoa3User.mockResolvedValueOnce({ + icn: "anICN", + first_name: "Edward", + last_name: "Paget", + idTheftIndicator: true, + }); + + const response: any = { render: renderMock }; + await handlers.loadICN(req, response, nextFn); + + expect(req.mpiUserClient.getMpiTraitsForLoa3User).toHaveBeenCalled(); + expect(renderMock).toHaveBeenCalledWith("layout", { + body: "sensitive_error", + }); + expect(nextFn).not.toHaveBeenCalled(); + }); + + it("should not block login when fraudIdTheft is true and idTheftIndicator is false", async () => { + const nextFn = jest.fn(); + const renderMock = jest.fn(); + const req: any = { + mpiUserClient: { ...mpiUserClient, fraudIdTheft: true }, + vsoClient: vsoClient, + user: { + claims: { ...claimsWithICN }, + }, + }; + + req.mpiUserClient.getMpiTraitsForLoa3User.mockResolvedValueOnce({ + icn: "anICN", + first_name: "Edward", + last_name: "Paget", + idTheftIndicator: false, + }); + + const response: any = { render: renderMock }; + await handlers.loadICN(req, response, nextFn); + + expect(renderMock).not.toHaveBeenCalled(); + expect(nextFn).toHaveBeenCalled(); + expect(req.user.claims.icn).toEqual("anICN"); + }); + it("should call getMVITraits... calls when ICN Exists", async () => { const nextFn = jest.fn(); const req: any = { From d4a0e034ee7de63f7a39d4e3c7018b8c3534dd8d Mon Sep 17 00:00:00 2001 From: dmekala-va Date: Fri, 11 Oct 2024 12:44:57 -0500 Subject: [PATCH 4/5] addressing comments --- dev-config.base.json | 3 ++- src/MpiUserClient.ts | 6 +++--- src/MpiUserClientConfig.js | 2 +- src/app.js | 2 +- src/cli/index.js | 2 +- src/routes/acsHandlers.test.ts | 36 ++++++++++++++++++++++++++++------ src/routes/acsHandlers.ts | 2 +- src/rsa.json | 0 8 files changed, 39 insertions(+), 14 deletions(-) create mode 100644 src/rsa.json diff --git a/dev-config.base.json b/dev-config.base.json index 60692467..4c2b80aa 100644 --- a/dev-config.base.json +++ b/dev-config.base.json @@ -29,7 +29,8 @@ "spIdpSsoBinding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect", "idpSamlLoginsEnabled": true, "logStyleElementsEnabled": true, - "idpSamlLogins": + "fraudBlockEnabled": true, + "idpSamlLogins": [ { "category": "example2SamlIdp", diff --git a/src/MpiUserClient.ts b/src/MpiUserClient.ts index 3eff32b0..c96be8c6 100644 --- a/src/MpiUserClient.ts +++ b/src/MpiUserClient.ts @@ -4,20 +4,20 @@ import axios from "axios"; export class MpiUserClient { mpiUserEndpoint: string; headers: object; - fraudIdTheft: boolean; + fraudBlockEnabled: boolean; constructor( apiKey: string, mpiUserEndpoint: string, accessKey: string, - fraudIdTheft: boolean + fraudBlockEnabled: boolean ) { this.mpiUserEndpoint = mpiUserEndpoint; this.headers = { apiKey: apiKey, accesskey: accessKey, }; - this.fraudIdTheft = fraudIdTheft; + this.fraudBlockEnabled = fraudBlockEnabled; } public async getMpiTraitsForLoa3User( diff --git a/src/MpiUserClientConfig.js b/src/MpiUserClientConfig.js index 3d099293..50c57198 100644 --- a/src/MpiUserClientConfig.js +++ b/src/MpiUserClientConfig.js @@ -3,6 +3,6 @@ export default class MpiUserClientConfig { this.mpiUserEndpoint = argv.mpiUserEndpoint; this.accessKey = argv.accessKey; this.apiKey = argv.vetsAPIToken; - this.fraudIdTheft = argv.fraudBlockEnabled || false; + this.fraudBlockEnabled = argv.fraudBlockEnabled || false; } } diff --git a/src/app.js b/src/app.js index dfe022ca..fe0223f0 100644 --- a/src/app.js +++ b/src/app.js @@ -102,7 +102,7 @@ function runServer(argv) { mpiUserClientConfig.apiKey, mpiUserClientConfig.mpiUserEndpoint, mpiUserClientConfig.accessKey, - mpiUserClientConfig.fraudIdTheft + mpiUserClientConfig.fraudBlockEnabled ); const vsoClient = new VsoClient( vsoConfig.token, diff --git a/src/cli/index.js b/src/cli/index.js index 146076ac..a18ba0ca 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -46,7 +46,7 @@ export function processArgs() { "Enable or disable blocking logins based on the fraud identity indicator", required: false, boolean: true, - default: true, + default: false, }, idpKey: { description: "IdP Signature PrivateKey Certificate", diff --git a/src/routes/acsHandlers.test.ts b/src/routes/acsHandlers.test.ts index d3bcb122..097be238 100644 --- a/src/routes/acsHandlers.test.ts +++ b/src/routes/acsHandlers.test.ts @@ -212,8 +212,6 @@ describe("scrubUserClaims", () => { }); }); - - describe("loadICN", () => { beforeEach(() => { // @ts-ignore @@ -222,11 +220,11 @@ describe("loadICN", () => { vsoClient.getVSOSearch.mockReset(); }); - it("should block login when fraudIdTheft is true and idTheftIndicator is true", async () => { + it("should block login when fraudBlockEnabled is true and idTheftIndicator is true", async () => { const nextFn = jest.fn(); const renderMock = jest.fn(); const req: any = { - mpiUserClient: { ...mpiUserClient, fraudIdTheft: true }, + mpiUserClient: { ...mpiUserClient, fraudBlockEnabled: true }, vsoClient: vsoClient, user: { claims: { ...claimsWithICN }, @@ -250,11 +248,11 @@ describe("loadICN", () => { expect(nextFn).not.toHaveBeenCalled(); }); - it("should not block login when fraudIdTheft is true and idTheftIndicator is false", async () => { + it("should not block login when fraudBlockEnabled is true and idTheftIndicator is false", async () => { const nextFn = jest.fn(); const renderMock = jest.fn(); const req: any = { - mpiUserClient: { ...mpiUserClient, fraudIdTheft: true }, + mpiUserClient: { ...mpiUserClient, fraudBlockEnabled: true }, vsoClient: vsoClient, user: { claims: { ...claimsWithICN }, @@ -276,6 +274,32 @@ describe("loadICN", () => { expect(req.user.claims.icn).toEqual("anICN"); }); + it("should not block login when fraudBlockEnabled is false and idTheftIndicator is true", async () => { + const nextFn = jest.fn(); + const renderMock = jest.fn(); + const req: any = { + mpiUserClient: { ...mpiUserClient, fraudBlockEnabled: false }, + vsoClient: vsoClient, + user: { + claims: { ...claimsWithICN }, + }, + }; + + req.mpiUserClient.getMpiTraitsForLoa3User.mockResolvedValueOnce({ + icn: "anICN", + first_name: "Edward", + last_name: "Paget", + idTheftIndicator: true, + }); + + const response: any = { render: renderMock }; + await handlers.loadICN(req, response, nextFn); + + expect(renderMock).not.toHaveBeenCalled(); + expect(nextFn).toHaveBeenCalled(); + expect(req.user.claims.icn).toEqual("anICN"); + }); + it("should call getMVITraits... calls when ICN Exists", async () => { const nextFn = jest.fn(); const req: any = { diff --git a/src/routes/acsHandlers.ts b/src/routes/acsHandlers.ts index f0d70194..fd046a18 100644 --- a/src/routes/acsHandlers.ts +++ b/src/routes/acsHandlers.ts @@ -141,7 +141,7 @@ export const loadICN = async ( result: "success", }); - if (req.mpiUserClient.fraudIdTheft && idTheftIndicator) { + if (req.mpiUserClient.fraudBlockEnabled && idTheftIndicator) { logger.warn("Fradulent identity detected, blocking login."); return res.render("layout", { body: "sensitive_error", diff --git a/src/rsa.json b/src/rsa.json new file mode 100644 index 00000000..e69de29b From 2bb98c45ac5e90a3edb522e7f0a9f0c1f1a917f2 Mon Sep 17 00:00:00 2001 From: dmekala-va Date: Fri, 11 Oct 2024 13:37:36 -0500 Subject: [PATCH 5/5] remove json file --- src/rsa.json | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 src/rsa.json diff --git a/src/rsa.json b/src/rsa.json deleted file mode 100644 index e69de29b..00000000