Migrate sync server general utils to typescript (#7074)

* Migrate sync server general utils to typescript

* Add release notes

* Rabbit

* Fix

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,
             ),
           ),

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(

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);

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');
 }

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}`;
     }),

packages/sync-server/src/util/paths.js

@@ -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`);
-}

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`);
+}

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));
   }
 

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<string>(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();

packages/sync-server/src/util/types.ts

@@ -0,0 +1 @@
+export type BrandedId<T extends string> = string & { __brand: T };

upcoming-release-notes/7074.md

@@ -0,0 +1,6 @@
+---
+category: Maintenance
+authors: [jfdoming]
+---
+
+Move some sync-server utilities to TypeScript