diff --git a/packages/sync-server/src/app-gocardless/app-gocardless.js b/packages/sync-server/src/app-gocardless/app-gocardless.js index cab1faa2e3..cf5ed7c87b 100644 --- a/packages/sync-server/src/app-gocardless/app-gocardless.js +++ b/packages/sync-server/src/app-gocardless/app-gocardless.js @@ -75,7 +75,7 @@ app.post( accounts: await Promise.all( accounts.map(async account => account?.iban - ? { ...account, iban: await sha256String(account.iban) } + ? { ...account, iban: sha256String(account.iban) } : account, ), ), diff --git a/packages/sync-server/src/app-sync.test.ts b/packages/sync-server/src/app-sync.test.ts index 12e75451f9..648ffc433b 100644 --- a/packages/sync-server/src/app-sync.test.ts +++ b/packages/sync-server/src/app-sync.test.ts @@ -7,7 +7,7 @@ import request from 'supertest'; import { getAccountDb } from './account-db'; import { handlers as app } from './app-sync'; -import { getPathForUserFile } from './util/paths'; +import { getPathForUserFile, isValidFileId } from './util/paths'; const ADMIN_ROLE = 'ADMIN'; const OTHER_USER_ID = 'otherUser'; @@ -23,6 +23,14 @@ const deleteUser = (userId: string) => { getAccountDb().mutate('DELETE FROM users WHERE id = ?', [userId]); }; +function generateFileId() { + const id = crypto.randomBytes(16).toString('hex'); + if (!isValidFileId(id)) { + throw new Error('Generated fileId is invalid'); + } + return id; +} + describe('/user-get-key', () => { it('returns 401 if the user is not authenticated', async () => { const res = await request(app).post('/user-get-key'); @@ -380,7 +388,7 @@ describe('/upload-user-file', () => { }); it('uploads a new file successfully', async () => { - const fileId = crypto.randomBytes(16).toString('hex'); + const fileId = generateFileId(); const fileName = 'test-file.txt'; const fileContent = 'test file content'; const fileContentBuffer = Buffer.from(fileContent); @@ -431,7 +439,7 @@ describe('/upload-user-file', () => { }); it('uploads and updates an existing file successfully', async () => { - const fileId = crypto.randomBytes(16).toString('hex'); + const fileId = generateFileId(); const oldGroupId = null; //sync state was reset const oldFileName = 'old-test-file.txt'; const newFileName = 'new-test-file.txt'; @@ -559,7 +567,7 @@ describe('/upload-user-file', () => { }); it('returns 403 when non-owner overwrites another user file', async () => { - const fileId = crypto.randomBytes(16).toString('hex'); + const fileId = generateFileId(); const groupId = 'group-id'; const keyId = 'key-id'; const syncVersion = 2; @@ -598,7 +606,7 @@ describe('/upload-user-file', () => { }); it("allows an admin to overwrite another user's file", async () => { - const fileId = crypto.randomBytes(16).toString('hex'); + const fileId = generateFileId(); const groupId = 'admin-upload-group-id'; const keyId = 'key-id'; const syncVersion = 2; @@ -708,22 +716,23 @@ describe('/download-user-file', () => { }); it('returns an attachment file', async () => { + const fileId = generateFileId(); const fileContent = 'content'; - fs.writeFileSync(getPathForUserFile('file-id'), fileContent); + fs.writeFileSync(getPathForUserFile(fileId), fileContent); getAccountDb().mutate( 'INSERT INTO files (id, deleted) VALUES (?, FALSE)', - ['file-id'], + [fileId], ); const res = await request(app) .get('/download-user-file') .set('x-actual-token', 'valid-token') - .set('x-actual-file-id', 'file-id'); + .set('x-actual-file-id', fileId); expect(res.statusCode).toEqual(200); expect(res.headers).toEqual( expect.objectContaining({ - 'content-disposition': 'attachment;filename=file-id', + 'content-disposition': `attachment;filename=${fileId}`, 'content-type': 'application/octet-stream', }), ); @@ -734,7 +743,7 @@ describe('/download-user-file', () => { describe('access control', () => { it('returns 403 when non-owner downloads another user file', async () => { - const fileId = crypto.randomBytes(16).toString('hex'); + const fileId = generateFileId(); const fileContent = 'sensitive content'; fs.writeFileSync(getPathForUserFile(fileId), fileContent); getAccountDb().mutate( @@ -757,7 +766,7 @@ describe('/download-user-file', () => { }); it("allows an admin to download another user's file", async () => { - const fileId = crypto.randomBytes(16).toString('hex'); + const fileId = generateFileId(); const fileContent = 'admin-downloaded content'; fs.writeFileSync(getPathForUserFile(fileId), fileContent); getAccountDb().mutate( @@ -783,7 +792,7 @@ describe('/download-user-file', () => { it('allows non-owner with user_access to download via requireFileAccess (UserService.countUserAccess > 0)', async () => { // File owned by another user; access granted only via user_access row, not owner/admin. // This exercises the requireFileAccess branch that uses UserService.countUserAccess. - const fileId = crypto.randomBytes(16).toString('hex'); + const fileId = generateFileId(); const fileContent = 'shared-user content'; fs.writeFileSync(getPathForUserFile(fileId), fileContent); getAccountDb().mutate( diff --git a/packages/sync-server/src/app-sync.ts b/packages/sync-server/src/app-sync.ts index 38a8586f1c..c35b463191 100644 --- a/packages/sync-server/src/app-sync.ts +++ b/packages/sync-server/src/app-sync.ts @@ -26,7 +26,11 @@ import { requestLoggerMiddleware, validateSessionMiddleware, } from './util/middlewares'; -import { getPathForGroupFile, getPathForUserFile } from './util/paths'; +import { + getPathForGroupFile, + getPathForUserFile, + isValidFileId, +} from './util/paths'; const app = express(); app.use(validateSessionMiddleware); @@ -49,16 +53,11 @@ app.use(express.json({ limit: `${config.get('upload.fileSizeLimitMB')}mb` })); export { app as handlers }; const OK_RESPONSE = { status: 'ok' }; -const FILE_ID_PATTERN = /^[A-Za-z0-9_-]+$/; function boolToInt(deleted) { return deleted ? 1 : 0; } -function isValidFileId(fileId: unknown): fileId is string { - return typeof fileId === 'string' && FILE_ID_PATTERN.test(fileId); -} - const verifyFileExists = (fileId, filesService, res, errorObject) => { try { return filesService.get(fileId); diff --git a/packages/sync-server/src/util/hash.js b/packages/sync-server/src/util/hash.ts similarity index 69% rename from packages/sync-server/src/util/hash.js rename to packages/sync-server/src/util/hash.ts index e8f14f3e4e..2e29c64c21 100644 --- a/packages/sync-server/src/util/hash.js +++ b/packages/sync-server/src/util/hash.ts @@ -1,5 +1,5 @@ import crypto from 'crypto'; -export async function sha256String(str) { +export function sha256String(str: string) { return crypto.createHash('sha256').update(str).digest('base64'); } diff --git a/packages/sync-server/src/util/middlewares.js b/packages/sync-server/src/util/middlewares.ts similarity index 77% rename from packages/sync-server/src/util/middlewares.js rename to packages/sync-server/src/util/middlewares.ts index 9d1fe9cf69..4435b9e95c 100644 --- a/packages/sync-server/src/util/middlewares.js +++ b/packages/sync-server/src/util/middlewares.ts @@ -1,15 +1,15 @@ +import type { NextFunction, Request, Response } from 'express'; import * as expressWinston from 'express-winston'; import * as winston from 'winston'; import { validateSession } from './validate-user'; -/** - * @param {Error} err - * @param {import('express').Request} req - * @param {import('express').Response} res - * @param {import('express').NextFunction} next - */ -async function errorMiddleware(err, req, res, next) { +async function errorMiddleware( + err: Error, + req: Request, + res: Response, + next: NextFunction, +) { if (res.headersSent) { // If you call next() with an error after you have started writing the response // (for example, if you encounter an error while streaming the response @@ -30,12 +30,11 @@ async function errorMiddleware(err, req, res, next) { res.status(500).send({ status: 'error', reason: 'internal-error' }); } -/** - * @param {import('express').Request} req - * @param {import('express').Response} res - * @param {import('express').NextFunction} next - */ -const validateSessionMiddleware = async (req, res, next) => { +const validateSessionMiddleware = async ( + req: Request, + res: Response, + next: NextFunction, +) => { const session = await validateSession(req, res); if (!session) { return; @@ -54,7 +53,7 @@ const requestLoggerMiddleware = expressWinston.logger({ winston.format.timestamp(), winston.format.printf(args => { const { timestamp, level, meta } = args; - const { res, req } = meta; + const { res, req } = meta as { res: Response; req: Request }; return `${timestamp} ${level}: ${req.method} ${res.statusCode} ${req.url}`; }), diff --git a/packages/sync-server/src/util/paths.js b/packages/sync-server/src/util/paths.js deleted file mode 100644 index 842bb1848e..0000000000 --- a/packages/sync-server/src/util/paths.js +++ /dev/null @@ -1,13 +0,0 @@ -import { join, resolve } from 'node:path'; - -import { config } from '../load-config'; - -/** @param {string} fileId */ -export function getPathForUserFile(fileId) { - return join(resolve(config.get('userFiles')), `file-${fileId}.blob`); -} - -/** @param {string} groupId */ -export function getPathForGroupFile(groupId) { - return join(resolve(config.get('userFiles')), `group-${groupId}.sqlite`); -} diff --git a/packages/sync-server/src/util/paths.ts b/packages/sync-server/src/util/paths.ts new file mode 100644 index 0000000000..8eda2c0aac --- /dev/null +++ b/packages/sync-server/src/util/paths.ts @@ -0,0 +1,21 @@ +import { join, resolve } from 'node:path'; + +import { config } from '../load-config'; + +import type { BrandedId } from './types'; + +const ID_REGEX = /^[a-zA-Z0-9_-]+$/; + +type FileId = BrandedId<'file'>; + +export function isValidFileId(id: string): id is FileId { + return ID_REGEX.test(id); +} + +export function getPathForUserFile(fileId: FileId) { + return join(resolve(config.get('userFiles')), `file-${fileId}.blob`); +} + +export function getPathForGroupFile(groupId: FileId) { + return join(resolve(config.get('userFiles')), `group-${groupId}.sqlite`); +} diff --git a/packages/sync-server/src/util/payee-name.js b/packages/sync-server/src/util/payee-name.ts similarity index 74% rename from packages/sync-server/src/util/payee-name.js rename to packages/sync-server/src/util/payee-name.ts index e895eab5bf..3cbfab279e 100644 --- a/packages/sync-server/src/util/payee-name.js +++ b/packages/sync-server/src/util/payee-name.ts @@ -1,17 +1,19 @@ +import type { Transaction } from '../app-gocardless/gocardless-node.types'; + import { title } from './title/index'; -function formatPayeeIban(iban) { +function formatPayeeIban(iban: string) { return '(' + iban.slice(0, 4) + ' XXX ' + iban.slice(-4) + ')'; } -export const formatPayeeName = trans => { - const amount = trans.transactionAmount.amount; +export const formatPayeeName = (trans: Transaction) => { + const amount = Number(trans.transactionAmount.amount); const nameParts = []; // get the correct name and account fields for the transaction amount let name; let account; - if (amount > 0 || Object.is(Number(amount), 0)) { + if (amount > 0 || Object.is(amount, 0)) { name = trans.debtorName; account = trans.debtorAccount; } else { @@ -37,7 +39,7 @@ export const formatPayeeName = trans => { nameParts.push(title(name)); } - if (account && account.iban) { + if (typeof account === 'object' && account && account.iban) { nameParts.push(formatPayeeIban(account.iban)); } diff --git a/packages/sync-server/src/util/prompt.js b/packages/sync-server/src/util/prompt.ts similarity index 87% rename from packages/sync-server/src/util/prompt.js rename to packages/sync-server/src/util/prompt.ts index 8bb8d72b56..4cc373db2c 100644 --- a/packages/sync-server/src/util/prompt.js +++ b/packages/sync-server/src/util/prompt.ts @@ -1,6 +1,6 @@ import { createInterface, cursorTo } from 'node:readline'; -export async function prompt(message) { +export async function prompt(message: string) { const rl = createInterface({ input: process.stdin, output: process.stdout, @@ -38,15 +38,16 @@ export async function promptPassword() { return password; } -async function askForPassword(prompt) { - let dataListener, endListener; +async function askForPassword(prompt: string) { + let dataListener: (key: Buffer) => void = () => null; + let endListener: () => void = () => null; - const promise = new Promise(resolve => { + const promise = new Promise(resolve => { let result = ''; process.stdout.write(prompt); process.stdin.setRawMode(true); process.stdin.resume(); - dataListener = key => { + dataListener = (key: Buffer) => { switch (key[0]) { case 0x03: // ^C process.exit(); diff --git a/packages/sync-server/src/util/types.ts b/packages/sync-server/src/util/types.ts new file mode 100644 index 0000000000..fb6ddb8688 --- /dev/null +++ b/packages/sync-server/src/util/types.ts @@ -0,0 +1 @@ +export type BrandedId = string & { __brand: T }; diff --git a/upcoming-release-notes/7074.md b/upcoming-release-notes/7074.md new file mode 100644 index 0000000000..127ca52e54 --- /dev/null +++ b/upcoming-release-notes/7074.md @@ -0,0 +1,6 @@ +--- +category: Maintenance +authors: [jfdoming] +--- + +Move some sync-server utilities to TypeScript