diff --git a/package-lock.json b/package-lock.json index 187e0690..2d2de561 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "veritable-ui", - "version": "0.8.0", + "version": "0.8.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "veritable-ui", - "version": "0.8.0", + "version": "0.8.1", "license": "Apache-2.0", "dependencies": { "@digicatapult/tsoa-oauth-express": "^0.1.20", diff --git a/package.json b/package.json index f29a577a..e7817d94 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "veritable-ui", - "version": "0.8.0", + "version": "0.8.1", "description": "UI for Veritable", "main": "src/index.ts", "type": "module", diff --git a/src/controllers/connection/__tests__/fixtures.ts b/src/controllers/connection/__tests__/fixtures.ts index e1cf7699..e0abc411 100644 --- a/src/controllers/connection/__tests__/fixtures.ts +++ b/src/controllers/connection/__tests__/fixtures.ts @@ -61,6 +61,7 @@ export const validConnection: ConnectionRow = { status: 'pending', company_number: validCompanyNumber, company_name: 'must be a valid company name', + pin_attempt_count: 0, } const buildBase64Invite = (companyNumber: string) => diff --git a/src/controllers/connection/__tests__/newConnection.test.ts b/src/controllers/connection/__tests__/newConnection.test.ts index 5f8c3ebc..41890df8 100644 --- a/src/controllers/connection/__tests__/newConnection.test.ts +++ b/src/controllers/connection/__tests__/newConnection.test.ts @@ -318,6 +318,7 @@ describe('NewConnectionController', () => { company_number: '00000001', status: 'pending', agent_connection_id: null, + pin_attempt_count: 0, }, ]) }) @@ -329,6 +330,7 @@ describe('NewConnectionController', () => { connection_id: '42', oob_invite_id: 'id-NAME', expires_at: new Date(100 + 14 * 24 * 60 * 60 * 1000), + validity: 'valid', }) expect(typeof pin_hash).to.equal('string') }) @@ -508,6 +510,7 @@ describe('NewConnectionController', () => { company_number: '00000001', status: 'pending', agent_connection_id: 'oob-connection', + pin_attempt_count: 0, }, ]) }) @@ -519,6 +522,7 @@ describe('NewConnectionController', () => { connection_id: '42', oob_invite_id: 'oob-record', expires_at: new Date(100 + 14 * 24 * 60 * 60 * 1000), + validity: 'valid', }) expect(typeof pin_hash).to.equal('string') }) diff --git a/src/controllers/connection/newConnection.ts b/src/controllers/connection/newConnection.ts index 78e28b8d..0b5a31b1 100644 --- a/src/controllers/connection/newConnection.ts +++ b/src/controllers/connection/newConnection.ts @@ -331,6 +331,7 @@ export class NewConnectionController extends HTMLController { company_number: company.company_number, agent_connection_id: agentConnectionId, status: 'pending', + pin_attempt_count: 0, }) await db.insert('connection_invite', { @@ -338,6 +339,7 @@ export class NewConnectionController extends HTMLController { oob_invite_id: invitationId, pin_hash: pinHash, expires_at: new Date(new Date().getTime() + 14 * 24 * 60 * 60 * 1000), + validity: 'valid', }) connectionId = record.id }) diff --git a/src/env.ts b/src/env.ts index 190671fd..48ab00bb 100644 --- a/src/env.ts +++ b/src/env.ts @@ -84,6 +84,7 @@ export const envConfig = { CLOUDAGENT_ADMIN_ORIGIN: envalid.url({ devDefault: 'http://localhost:3100' }), CLOUDAGENT_ADMIN_WS_ORIGIN: envalid.url({ devDefault: 'ws://localhost:3100' }), INVITATION_PIN_SECRET: pinSecretValidator({ devDefault: Buffer.from('secret', 'utf8') }), + INVITATION_PIN_ATTEMPT_LIMIT: envalid.num({ default: 5 }), INVITATION_FROM_COMPANY_NUMBER: envalid.str({ devDefault: '07964699' }), ISSUANCE_DID_POLICY: issuanceRecordValidator({ devDefault: 'EXISTING_OR_NEW' }), ISSUANCE_SCHEMA_POLICY: issuanceRecordValidator({ devDefault: 'EXISTING_OR_NEW' }), diff --git a/src/models/db/index.ts b/src/models/db/index.ts index 808f2fff..64f27cba 100644 --- a/src/models/db/index.ts +++ b/src/models/db/index.ts @@ -3,7 +3,7 @@ import { container, singleton } from 'tsyringe' import { z } from 'zod' import { Env } from '../../env.js' -import Zod, { IDatabase, Models, Order, TABLE, Update, Where, tablesList } from './types.js' +import Zod, { ColumnsByType, IDatabase, Models, Order, TABLE, Update, Where, tablesList } from './types.js' import { reduceWhere } from './util.js' const env = container.resolve(Env) @@ -67,6 +67,18 @@ export default class Database { return z.array(Zod[model].get).parse(await query.returning('*')) } + increment = async ( + model: M, + column: ColumnsByType, + where?: Where, + amount: number = 1 + ): Promise => { + let query = this.db[model]() + query = reduceWhere(query, where) + query = query.increment(column, amount) + return z.array(Zod[model].get).parse(await query.returning('*')) + } + get = async ( model: M, where?: Where, diff --git a/src/models/db/migrations/20240605084834_company_name_trigram.ts b/src/models/db/migrations/20240605084834_company_name_trigram.ts index c33887fd..54cb4d59 100644 --- a/src/models/db/migrations/20240605084834_company_name_trigram.ts +++ b/src/models/db/migrations/20240605084834_company_name_trigram.ts @@ -8,6 +8,6 @@ export async function up(knex: Knex): Promise { } export async function down(knex: Knex): Promise { - await knex.raw('DROP INDEX company_name_trgm_idx on connection') + await knex.raw('DROP INDEX company_name_trgm_idx') await knex.raw('DROP EXTENSION "pg_trgm"') } diff --git a/src/models/db/migrations/20240710162655_invite-invalidation-cols.ts b/src/models/db/migrations/20240710162655_invite-invalidation-cols.ts new file mode 100644 index 00000000..01d65d32 --- /dev/null +++ b/src/models/db/migrations/20240710162655_invite-invalidation-cols.ts @@ -0,0 +1,28 @@ +import type { Knex } from 'knex' + +export async function up(knex: Knex): Promise { + await knex.schema.alterTable('connection', (def) => { + def.tinyint('pin_attempt_count').unsigned().notNullable().defaultTo(0) + }) + + await knex.schema.alterTable('connection_invite', (def) => { + def + .enum('validity', ['valid', 'expired', 'too_many_attempts', 'used'], { + useNative: true, + enumName: 'connection_invite_verification_status', + }) + .defaultTo('valid') + }) +} + +export async function down(knex: Knex): Promise { + await knex.schema.alterTable('connection', (def) => { + def.dropColumn('pin_attempt_count') + }) + + await knex.schema.alterTable('connection_invite', (def) => { + def.dropColumn('validity') + }) + + await knex.raw('DROP TYPE connection_invite_verification_status') +} diff --git a/src/models/db/types.ts b/src/models/db/types.ts index 6f3d8ed8..5932efe4 100644 --- a/src/models/db/types.ts +++ b/src/models/db/types.ts @@ -15,6 +15,7 @@ const insertConnection = z.object({ z.literal('disconnected'), ]), agent_connection_id: z.union([z.string(), z.null()]), + pin_attempt_count: z.number().int().gte(0).lte(255), }) const insertConnectionInvite = z.object({ @@ -22,6 +23,7 @@ const insertConnectionInvite = z.object({ oob_invite_id: z.string(), pin_hash: z.string(), expires_at: z.date(), + validity: z.union([z.literal('valid'), z.literal('expired'), z.literal('too_many_attempts'), z.literal('used')]), }) const insertQuery = z.object({ connection_id: z.string(), @@ -74,6 +76,10 @@ export type Models = { } } +export type ColumnsByType = { + [K in keyof Models[M]['get']]-?: Models[M]['get'][K] extends T ? K : never +}[keyof Models[M]['get']] + type WhereComparison = { [key in keyof Models[M]['get']]: [ Extract, @@ -88,6 +94,7 @@ export type WhereMatch = { export type Where = WhereMatch | (WhereMatch | WhereComparison[keyof Models[M]['get']])[] export type Order = [keyof Models[M]['get'], 'asc' | 'desc'][] export type Update = Partial + export type IDatabase = { [key in TABLE]: () => Knex.QueryBuilder } diff --git a/src/services/credentialEvents/__tests__/companyDetailsV1.test.ts b/src/services/credentialEvents/__tests__/companyDetailsV1.test.ts index 044b1e14..9a8a0307 100644 --- a/src/services/credentialEvents/__tests__/companyDetailsV1.test.ts +++ b/src/services/credentialEvents/__tests__/companyDetailsV1.test.ts @@ -51,7 +51,17 @@ describe('companyDetailsV1', function () { expect(dbMock.get.callCount).equal(2) expect(dbMock.get.firstCall.args).deep.equal(['connection', { agent_connection_id: 'agent-connection-id' }]) - expect(dbMock.get.secondCall.args).deep.equal(['connection_invite', { connection_id: 'connection-id' }]) + expect(dbMock.get.secondCall.args).deep.equal([ + 'connection_invite', + { connection_id: 'connection-id', validity: 'valid' }, + ]) + expect(dbMock.increment.callCount).to.equal(1) + expect(dbMock.increment.firstCall.args).to.deep.equal([ + 'connection', + 'pin_attempt_count', + { id: 'connection-id' }, + ]) + expect(dbMock.update.callCount).to.equal(0) const stub = cloudagentMock.acceptProposal expect(stub.callCount).to.equal(1) @@ -117,7 +127,17 @@ describe('companyDetailsV1', function () { expect(dbMock.get.callCount).equal(2) expect(dbMock.get.firstCall.args).deep.equal(['connection', { agent_connection_id: 'agent-connection-id' }]) - expect(dbMock.get.secondCall.args).deep.equal(['connection_invite', { connection_id: 'connection-id' }]) + expect(dbMock.get.secondCall.args).deep.equal([ + 'connection_invite', + { connection_id: 'connection-id', validity: 'valid' }, + ]) + expect(dbMock.increment.callCount).to.equal(1) + expect(dbMock.increment.firstCall.args).to.deep.equal([ + 'connection', + 'pin_attempt_count', + { id: 'connection-id' }, + ]) + expect(dbMock.update.callCount).to.equal(0) const stub = cloudagentMock.acceptProposal expect(stub.callCount).to.equal(1) @@ -402,6 +422,61 @@ describe('companyDetailsV1', function () { expect(stub.callCount).to.equal(0) }) + test(`pin attempt count exceeded`, async function () { + const { args, cloudagentMock, dbMock } = withCompanyDetailsDepsMock({ dbIncrement: [{ pin_attempt_count: 6 }] }) + const companyDetails = new CompanyDetailsV1Handler(...args) + + await companyDetails.handleProposalReceived( + { + id: 'credential-id', + connectionId: 'agent-connection-id', + protocolVersion: 'v2', + role: 'issuer', + state: 'proposal-received', + }, + { + proposalAttributes: [ + { + 'mime-type': 'text/plain', + name: 'company_name', + value: 'NAME', + }, + { + 'mime-type': 'text/plain', + name: 'company_number', + value: 'NUMBER', + }, + { + 'mime-type': 'text/plain', + name: 'pin', + value: '123456', + }, + ], + } + ) + + expect(dbMock.increment.callCount).to.equal(1) + expect(dbMock.increment.firstCall.args).to.deep.equal([ + 'connection', + 'pin_attempt_count', + { id: 'connection-id' }, + ]) + expect(dbMock.update.callCount).to.equal(2) + expect(dbMock.update.firstCall.args).to.deep.equal([ + 'connection_invite', + { connection_id: 'connection-id' }, + { validity: 'too_many_attempts' }, + ]) + expect(dbMock.update.secondCall.args).to.deep.equal([ + 'connection', + { id: 'connection-id' }, + { pin_attempt_count: 0 }, + ]) + + const stub = cloudagentMock.acceptProposal + expect(stub.callCount).to.equal(0) + }) + test(`invalid pin`, async function () { const { args, cloudagentMock } = withCompanyDetailsDepsMock({}) const companyDetails = new CompanyDetailsV1Handler(...args) @@ -440,9 +515,10 @@ describe('companyDetailsV1', function () { }) test(`invalid pin (expired)`, async function () { - const { args, cloudagentMock } = withCompanyDetailsDepsMock({ + const { args, cloudagentMock, dbMock } = withCompanyDetailsDepsMock({ dbGetConnectionInvites: [ { + id: 'invite-id', pin_hash: await argon2.hash('123456', { secret: invitePinSecret }), expires_at: new Date(0), }, @@ -479,6 +555,13 @@ describe('companyDetailsV1', function () { } ) + expect(dbMock.update.callCount).to.equal(1) + expect(dbMock.update.firstCall.args).to.deep.equal([ + 'connection_invite', + { id: 'invite-id' }, + { validity: 'expired' }, + ]) + const stub = cloudagentMock.acceptProposal expect(stub.callCount).to.equal(0) }) @@ -935,7 +1018,7 @@ describe('companyDetailsV1', function () { } describe('handleDone', async function () { - describe('happy path unverified as holder', async function () { + test('happy path unverified as holder', async function () { const { args, dbTransactionMock } = withCompanyDetailsDepsMock({}) const companyDetails = new CompanyDetailsV1Handler(...args) @@ -948,11 +1031,16 @@ describe('companyDetailsV1', function () { }) const stub = dbTransactionMock.update - expect(stub.callCount).to.equal(1) + expect(stub.callCount).to.equal(2) expect(stub.firstCall.args).to.deep.equal(['connection', { id: `connection-id` }, { status: 'verified_us' }]) + expect(stub.secondCall.args).to.deep.equal([ + 'connection_invite', + { connection_id: `connection-id` }, + { validity: 'used' }, + ]) }) - describe('happy path unverified as issuer', async function () { + test('happy path unverified as issuer', async function () { const { args, dbTransactionMock } = withCompanyDetailsDepsMock({}) const companyDetails = new CompanyDetailsV1Handler(...args) @@ -965,11 +1053,16 @@ describe('companyDetailsV1', function () { }) const stub = dbTransactionMock.update - expect(stub.callCount).to.equal(1) + expect(stub.callCount).to.equal(2) expect(stub.firstCall.args).to.deep.equal(['connection', { id: `connection-id` }, { status: 'verified_them' }]) + expect(stub.secondCall.args).to.deep.equal([ + 'connection_invite', + { connection_id: `connection-id` }, + { validity: 'used' }, + ]) }) - describe('happy path verified_them as holder', async function () { + test('happy path verified_them as holder', async function () { const { args, dbTransactionMock } = withCompanyDetailsDepsMock({ dbGetConnection: [ { status: 'verified_them', id: 'connection-id', company_name: 'NAME', company_number: 'NUMBER' }, @@ -986,11 +1079,16 @@ describe('companyDetailsV1', function () { }) const stub = dbTransactionMock.update - expect(stub.callCount).to.equal(1) + expect(stub.callCount).to.equal(2) expect(stub.firstCall.args).to.deep.equal(['connection', { id: `connection-id` }, { status: 'verified_both' }]) + expect(stub.secondCall.args).to.deep.equal([ + 'connection_invite', + { connection_id: `connection-id` }, + { validity: 'used' }, + ]) }) - describe('happy path verified_us as issuer', async function () { + test('happy path verified_us as issuer', async function () { const { args, dbTransactionMock } = withCompanyDetailsDepsMock({ dbGetConnection: [ { status: 'verified_us', id: 'connection-id', company_name: 'NAME', company_number: 'NUMBER' }, @@ -1007,11 +1105,16 @@ describe('companyDetailsV1', function () { }) const stub = dbTransactionMock.update - expect(stub.callCount).to.equal(1) + expect(stub.callCount).to.equal(2) expect(stub.firstCall.args).to.deep.equal(['connection', { id: `connection-id` }, { status: 'verified_both' }]) + expect(stub.secondCall.args).to.deep.equal([ + 'connection_invite', + { connection_id: `connection-id` }, + { validity: 'used' }, + ]) }) - describe('verified_both as holder', async function () { + test('verified_both as holder', async function () { const { args, dbTransactionMock } = withCompanyDetailsDepsMock({ dbGetConnection: [ { status: 'verified_both', id: 'connection-id', company_name: 'NAME', company_number: 'NUMBER' }, @@ -1031,7 +1134,7 @@ describe('companyDetailsV1', function () { expect(stub.callCount).to.equal(0) }) - describe('verified_both as issuer', async function () { + test('verified_both as issuer', async function () { const { args, dbTransactionMock } = withCompanyDetailsDepsMock({ dbGetConnection: [ { status: 'verified_both', id: 'connection-id', company_name: 'NAME', company_number: 'NUMBER' }, @@ -1051,7 +1154,7 @@ describe('companyDetailsV1', function () { expect(stub.callCount).to.equal(0) }) - describe('verified_us as holder', async function () { + test('verified_us as holder', async function () { const { args, dbTransactionMock } = withCompanyDetailsDepsMock({ dbGetConnection: [ { status: 'verified_us', id: 'connection-id', company_name: 'NAME', company_number: 'NUMBER' }, @@ -1071,7 +1174,7 @@ describe('companyDetailsV1', function () { expect(stub.callCount).to.equal(0) }) - describe('verified_them as issuer', async function () { + test('verified_them as issuer', async function () { const { args, dbTransactionMock } = withCompanyDetailsDepsMock({ dbGetConnection: [ { status: 'verified_them', id: 'connection-id', company_name: 'NAME', company_number: 'NUMBER' }, @@ -1091,7 +1194,7 @@ describe('companyDetailsV1', function () { expect(stub.callCount).to.equal(0) }) - describe('with unknown connection', async function () { + test('with unknown connection', async function () { const { args, dbTransactionMock } = withCompanyDetailsDepsMock({ dbGetConnection: [], }) diff --git a/src/services/credentialEvents/__tests__/helpers/mocks.ts b/src/services/credentialEvents/__tests__/helpers/mocks.ts index 73aa9d8c..0c1d3791 100644 --- a/src/services/credentialEvents/__tests__/helpers/mocks.ts +++ b/src/services/credentialEvents/__tests__/helpers/mocks.ts @@ -80,6 +80,11 @@ const defaultCompanyDetailsOptions = { expires_at: new Date(10), // needs to be in the future }, ] as unknown, + dbIncrement: [ + { + pin_attempt_count: 1, + }, + ] as unknown, } export const withCompanyDetailsDepsMock = (opts: Partial) => { @@ -89,7 +94,14 @@ export const withCompanyDetailsDepsMock = (opts: Partial { + switch (name) { + case 'INVITATION_PIN_SECRET': + return invitePinSecret + case 'INVITATION_PIN_ATTEMPT_LIMIT': + return 5 + } + }), } as unknown as Env const dbTransactionMock = { get: sinon.stub().resolves(options.dbGetConnection), @@ -101,6 +113,8 @@ export const withCompanyDetailsDepsMock = (opts: Partial Promise): Promise => { await handler(dbTransactionMock as unknown as Database) }), diff --git a/src/services/credentialEvents/companyDetailsV1.ts b/src/services/credentialEvents/companyDetailsV1.ts index c08694e2..489a6960 100644 --- a/src/services/credentialEvents/companyDetailsV1.ts +++ b/src/services/credentialEvents/companyDetailsV1.ts @@ -65,12 +65,31 @@ export default class CompanyDetailsV1Handler implements CredentialEventHandler<' return } + // before we check pin validity check we haven't had too many pin attempts already + const [{ pin_attempt_count: pinAttemptCount }] = await this.db.increment('connection', 'pin_attempt_count', { + id: connection.id, + }) + if (pinAttemptCount > this.env.get('INVITATION_PIN_ATTEMPT_LIMIT')) { + this.logger.warn( + { connectionId: connection.id }, + 'PIN verification attempt count exceeded for connection %s', + connection.id + ) + await this.db.update('connection_invite', { connection_id: connection.id }, { validity: 'too_many_attempts' }) + await this.db.update('connection', { id: connection.id }, { pin_attempt_count: 0 }) // reset so if a new pin is sent they can try again + return + } + // check the pin number provided is valid - const pinInvites = await this.db.get('connection_invite', { connection_id: connection.id }) + const pinInvites = await this.db.get('connection_invite', { + connection_id: connection.id, + validity: 'valid', + }) const isPinValid = ( await Promise.all( - pinInvites.map(async ({ pin_hash, expires_at }) => { + pinInvites.map(async ({ pin_hash, expires_at, id }) => { if (expires_at < new Date()) { + await this.db.update('connection_invite', { id }, { validity: 'expired' }) return false } return await argon2.verify(pin_hash, pinAttr.value, { secret: this.env.get('INVITATION_PIN_SECRET') }) @@ -179,6 +198,7 @@ export default class CompanyDetailsV1Handler implements CredentialEventHandler<' } await db.update('connection', { id: connection.id }, { status: newState }) + await db.update('connection_invite', { connection_id: connection.id }, { validity: 'used' }) }) } }