Fix: Security (#1355)

* separate route for onboarding

* zse zod for validation

* use argon2id

* add build tools

Dockerfile

@@ -5,6 +5,9 @@ WORKDIR /app
 # Install pnpm
 RUN corepack enable pnpm
 
+# add build tools for argon2
+RUN apk add --no-cache make gcc g++ python3
+
 # Copy Web UI
 COPY src ./
 RUN pnpm install

src/app/stores/auth.ts

@@ -5,7 +5,7 @@ export const useAuthStore = defineStore('Auth', () => {
    * @throws if unsuccessful
    */
   async function signup(username: string, password: string) {
-    const response = await api.createAccount({ username, password });
+    const response = await api.setupAccount({ username, password });
     return response.success;
   }
 

src/app/utils/api.ts

@@ -128,14 +128,14 @@ class API {
     });
   }
 
-  async createAccount({
+  async setupAccount({
     username,
     password,
   }: {
     username: string;
     password: string;
   }) {
-    return $fetch('/api/account/new', {
+    return $fetch('/api/account/setup', {
       method: 'post',
       body: { username, password },
     });

src/i18n.config.ts

@@ -48,11 +48,6 @@ export default defineI18nConfig(() => ({
       Permanent: 'Permanent',
       OneTimeLink: 'Generate short one time link',
       errorInit: 'Initialization failed.',
-      errorDatabaseConn: 'Failed to connect to the database.',
-      errorPasswordReq:
-        'Password does not meet the strength requirements. It must be at least 12 characters long, with at least one uppercase letter, one lowercase letter, one number, and one special character.',
-      errorUsernameReq: 'Username must be longer than 8 characters.',
-      errorUserExist: 'User already exists.',
     },
     ua: {
       name: 'Ім`я',
@@ -280,12 +275,6 @@ export default defineI18nConfig(() => ({
       Permanent: 'Permanent',
       OneTimeLink: 'Générer un lien court à usage unique',
       errorInit: "Échec de l'initialisation.",
-      errorDatabaseConn: 'Échec de la connexion à la base de données.',
-      errorPasswordReq:
-        'Le mot de passe ne répond pas aux exigences de sécurité. Il doit comporter au moins 12 caractères, dont au moins une lettre majuscule, une lettre minuscule, un chiffre et un caractère spécial.',
-      errorUsernameReq:
-        "Le nom d'utilisateur doit comporter plus de 8 caractères.",
-      errorUserExist: "L'utilisateur existe déjà.",
     },
     de: {
       // github.com/florian-asche

src/package.json

@@ -25,8 +25,8 @@
     "@pinia/nuxt": "^0.5.4",
     "@tailwindcss/forms": "^0.5.8",
     "apexcharts": "^3.53.0",
+    "argon2": "^0.41.1",
     "basic-auth": "^2.0.1",
-    "bcryptjs": "^2.4.3",
     "cidr-tools": "^11.0.2",
     "crc-32": "^1.2.2",
     "debug": "^4.3.7",
@@ -45,7 +45,6 @@
   },
   "devDependencies": {
     "@nuxt/eslint-config": "^0.5.5",
-    "@types/bcryptjs": "^2.4.6",
     "@types/debug": "^4.1.12",
     "@types/qrcode": "^1.5.5",
     "eslint": "^9.9.1",

src/pnpm-lock.yaml

@@ -26,12 +26,12 @@ importers:
       apexcharts:
         specifier: ^3.53.0
         version: 3.53.0
+      argon2:
+        specifier: ^0.41.1
+        version: 0.41.1
       basic-auth:
         specifier: ^2.0.1
         version: 2.0.1
-      bcryptjs:
-        specifier: ^2.4.3
-        version: 2.4.3
       cidr-tools:
         specifier: ^11.0.2
         version: 11.0.2
@@ -81,9 +81,6 @@ importers:
       '@nuxt/eslint-config':
         specifier: ^0.5.5
         version: 0.5.5([email protected]([email protected]))([email protected])
-      '@types/bcryptjs':
-        specifier: ^2.4.6
-        version: 2.4.6
       '@types/debug':
         specifier: ^4.1.12
         version: 4.1.12
@@ -1013,6 +1010,10 @@ packages:
     resolution: {integrity: sha512-HNjmfLQEVRZmHRET336f20H/8kOozUGwk7yajvsonjNxbj2wBTK1WsQuHkD5yYh9RxFGL2EyDHryOihOwUoKDA==}
     engines: {node: '>= 10.0.0'}
 
+  '@phc/[email protected]':
+    resolution: {integrity: sha512-m7X9U6BG2+J+R1lSOdCiITLLrxm+cWlNI3HUFA92oLO77ObGNzaKdh8pMLqdZcshtkKuV84olNNXDfMc4FezBQ==}
+    engines: {node: '>=10'}
+
   '@pinia/[email protected]':
     resolution: {integrity: sha512-nNEs2pq6+Ji5qIyRwmeD9LUdctL8aJ8QMVLTYxUc16cXEOcIIN+MSA8Xudsd0lVETYgEAROT5HiBHnOYRDY3yQ==}
 
@@ -1211,9 +1212,6 @@ packages:
     resolution: {integrity: sha512-L7z9BgrNEcYyUYtF+HaEfiS5ebkh9jXqbszz7pC0hRBPaatV0XjSD3+eHrpqFemQfgwiFF0QPIarnIihIDn7OA==}
     engines: {node: '>=10.13.0'}
 
-  '@types/[email protected]':
-    resolution: {integrity: sha512-9xlo6R2qDs5uixm0bcIqCeMCE6HiQsIyel9KQySStiyqNl2tnj2mP3DX1Nf56MD6KMenNNlBBsy3LJ7gUEQPXQ==}
-
   '@types/[email protected]':
     resolution: {integrity: sha512-vIChWdVG3LG1SMxEvI/AK+FWJthlrqlTu7fbrlywTkkaONwk/UAGaULXRlf8vkzFBLVm0zkMdCquhL5aOjhXPQ==}
 
@@ -1520,6 +1518,10 @@ packages:
   [email protected]:
     resolution: {integrity: sha512-PYjyFOLKQ9y57JvQ6QLo8dAgNqswh8M1RMJYdQduT6xbWSgK36P/Z/v+p888pM69jMMfS8Xd8F6I1kQ/I9HUGg==}
 
+  [email protected]:
+    resolution: {integrity: sha512-dqCW8kJXke8Ik+McUcMDltrbuAWETPyU6iq+4AhxqKphWi7pChB/Zgd/Tp/o8xRLbg8ksMj46F/vph9wnxpTzQ==}
+    engines: {node: '>=16.17.0'}
+
   [email protected]:
     resolution: {integrity: sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==}
 
@@ -1567,9 +1569,6 @@ packages:
     resolution: {integrity: sha512-NF+epuEdnUYVlGuhaxbbq+dvJttwLnGY+YixlXlME5KpQ5W3CnXA5cVTneY3SPbPDRkcjMbifrwmFYcClgOZeg==}
     engines: {node: '>= 0.8'}
 
-  [email protected]:
-    resolution: {integrity: sha512-V/Hy/X9Vt7f3BbPJEi8BdVFMByHi+jNXrYkW3huaybV/kQ0KJg0Y6PkEMbn+zeT+i+SiKZ/HMqJGIIt4LZDqNQ==}
-
   [email protected]:
     resolution: {integrity: sha512-Ceh+7ox5qe7LJuLHoY0feh3pHuUDHAcRUeyL2VYghZwfpkNIy/+8Ocg0a3UuSoYzavmylwuLWQOf3hl0jjMMIw==}
     engines: {node: '>=8'}
@@ -3031,6 +3030,10 @@ packages:
   [email protected]:
     resolution: {integrity: sha512-5m3bsyrjFWE1xf7nz7YXdN4udnVtXK6/Yfgn5qnahL6bCkf2yKt4k3nuTKAtT4r3IG8JNR2ncsIMdZuAzJjHQQ==}
 
+  [email protected]:
+    resolution: {integrity: sha512-yBY+qqWSv3dWKGODD6OGE6GnTX7Q2r+4+DfpqxHSHh8x0B4EKP9+wVGLS6U/AM1vxSNNmUEuIV5EGhYwPpfOwQ==}
+    engines: {node: ^18 || ^20 || >= 21}
+
   [email protected]:
     resolution: {integrity: sha512-IhOigYzAKHd244OC0JIMIUrjzctirCmPkaIfhDeGcEETWof5zKYUW7e7MYvChGWh/4CJeXEgsRyGzuF334rOOQ==}
 
@@ -5484,6 +5487,8 @@ snapshots:
       '@parcel/watcher-win32-ia32': 2.4.1
       '@parcel/watcher-win32-x64': 2.4.1
 
+  '@phc/[email protected]': {}
+
   '@pinia/[email protected]([email protected])([email protected])([email protected])([email protected]([email protected]))':
     dependencies:
       '@nuxt/kit': 3.13.0([email protected])([email protected])
@@ -5651,8 +5656,6 @@ snapshots:
 
   '@trysound/[email protected]': {}
 
-  '@types/[email protected]': {}
-
   '@types/[email protected]':
     dependencies:
       '@types/ms': 0.7.34
@@ -6082,6 +6085,12 @@ snapshots:
 
   [email protected]: {}
 
+  [email protected]:
+    dependencies:
+      '@phc/format': 1.0.0
+      node-addon-api: 8.1.0
+      node-gyp-build: 4.8.2
+
   [email protected]: {}
 
   [email protected]:
@@ -6127,8 +6136,6 @@ snapshots:
     dependencies:
       safe-buffer: 5.1.2
 
-  [email protected]: {}
-
   [email protected]: {}
 
   [email protected]:
@@ -7727,6 +7734,8 @@ snapshots:
 
   [email protected]: {}
 
+  [email protected]: {}
+
   [email protected]: {}
 
   [email protected]:

src/server/api/account/create.post.ts

@@ -0,0 +1,8 @@
+export default defineEventHandler(async (event) => {
+  const { username, password } = await readValidatedBody(
+    event,
+    validateZod(passwordType)
+  );
+  await Database.createUser(username, password);
+  return { success: true };
+});

src/server/api/account/new.post.ts

@@ -1,24 +0,0 @@
-import { DatabaseError } from '~~/services/database/repositories/database';
-
-export default defineEventHandler(async (event) => {
-  setHeader(event, 'Content-Type', 'application/json');
-  try {
-    const { username, password } = await readValidatedBody(
-      event,
-      validateZod(passwordType)
-    );
-    await Database.newUserWithPassword(username, password);
-    return { success: true };
-  } catch (error) {
-    if (error instanceof DatabaseError) {
-      const t = await useTranslation(event);
-      throw createError({
-        statusCode: 400,
-        statusMessage: t(error.message),
-        message: error.message,
-      });
-    } else {
-      throw createError('Something happened !');
-    }
-  }
-});

src/server/api/account/setup.post.ts

@@ -0,0 +1,15 @@
+export default defineEventHandler(async (event) => {
+  const { username, password } = await readValidatedBody(
+    event,
+    validateZod(passwordType)
+  );
+  const users = await Database.getUsers();
+  if (users.length !== 0) {
+    throw createError({
+      statusCode: 400,
+      statusMessage: 'Invalid state',
+    });
+  }
+  await Database.createUser(username, password);
+  return { success: true };
+});

src/server/api/session.post.ts

@@ -15,7 +15,8 @@ export default defineEventHandler(async (event) => {
     });
 
   const userHashPassword = user.password;
-  if (!isPasswordValid(password, userHashPassword)) {
+  const passwordValid = await isPasswordValid(password, userHashPassword);
+  if (!passwordValid) {
     throw createError({
       statusCode: 401,
       statusMessage: 'Incorrect credentials',

src/server/middleware/session.ts

@@ -2,8 +2,7 @@ export default defineEventHandler(async (event) => {
   const url = getRequestURL(event);
   if (
     !url.pathname.startsWith('/api/') ||
-    // TODO: only allowed on onboarding!
-    url.pathname === '/api/account/new' ||
+    url.pathname === '/api/account/setup' ||
     url.pathname === '/api/session' ||
     url.pathname === '/api/lang' ||
     url.pathname === '/api/release' ||
@@ -34,7 +33,11 @@ export default defineEventHandler(async (event) => {
       });
 
     const userHashPassword = user.password;
-    if (isPasswordValid(authorization, userHashPassword)) {
+    const passwordValid = await isPasswordValid(
+      authorization,
+      userHashPassword
+    );
+    if (passwordValid) {
       return;
     }
     throw createError({

src/server/middleware/setup.ts

@@ -3,16 +3,21 @@ export default defineEventHandler(async (event) => {
   const url = getRequestURL(event);
 
   if (
-    url.pathname.startsWith('/setup') ||
-    url.pathname === '/api/account/new' ||
+    url.pathname === '/setup' ||
+    url.pathname === '/api/account/setup' ||
     url.pathname === '/api/features'
   ) {
     return;
   }
 
   const users = await Database.getUsers();
-  // TODO: better error messages for api requests
   if (users.length === 0) {
+    if (url.pathname.startsWith('/api/')) {
+      throw createError({
+        statusCode: 400,
+        statusMessage: 'Invalid State',
+      });
+    }
     return sendRedirect(event, '/setup', 302);
   }
 });

src/server/utils/password.ts

@@ -1,47 +1,18 @@
-import bcrypt from 'bcryptjs';
+import argon2 from 'argon2';
 
 /**
- * Checks if `password` matches the user password.
- *
- * @param {string} password string to test
- * @returns {boolean} `true` if matching user password, otherwise `false`
+ * Checks if `password` matches the hash.
  */
-export function isPasswordValid(password: string, hash: string): boolean {
-  return bcrypt.compareSync(password, hash);
-}
-
-/**
- * Checks if a password is strong based on following criteria :
- *
- * - minimum length of 12 characters
- * - contains at least one uppercase letter
- * - contains at least one lowercase letter
- * - contains at least one number
- * - contains at least one special character (e.g., !@#$%^&*(),.?":{}|<>).
- *
- * @param {string} password - The password to validate
- * @returns {boolean} `true` if the password is strong, otherwise `false`
- */
-
-export function isPasswordStrong(password: string): boolean {
-  if (password.length < 12) {
-    return false;
-  }
-
-  const hasUpperCase = /[A-Z]/.test(password);
-  const hasLowerCase = /[a-z]/.test(password);
-  const hasNumber = /\d/.test(password);
-  const hasSpecialChar = /[!@#$%^&*(),.?":{}|<>]/.test(password);
-
-  return hasUpperCase && hasLowerCase && hasNumber && hasSpecialChar;
+export function isPasswordValid(
+  password: string,
+  hash: string
+): Promise<boolean> {
+  return argon2.verify(hash, password);
 }
 
 /**
  * Hashes a password.
- *
- * @param {string} password - The plaintext password to hash
- * @returns {string} The hash of the password
  */
-export function hashPassword(password: string): string {
-  return bcrypt.hashSync(password, 12);
+export async function hashPassword(password: string): Promise<string> {
+  return argon2.hash(password);
 }

src/server/utils/types.ts

@@ -1,6 +1,8 @@
 import type { ZodSchema } from 'zod';
 import { z, ZodError } from 'zod';
 
+// TODO: use i18n for messages
+
 const safeStringRefine = z
   .string()
   .refine(
@@ -28,10 +30,19 @@ const file = z
 
 const username = z
   .string({ message: 'Username must be a valid string' })
+  .min(8, 'Username must be at least 8 Characters')
   .pipe(safeStringRefine);
 
 const password = z
   .string({ message: 'Password must be a valid string' })
+  .min(12, 'Password must be at least 12 Characters')
+  .regex(/[A-Z]/, 'Password must have at least 1 uppercase letter')
+  .regex(/[a-z]/, 'Password must have at least 1 lowercase letter')
+  .regex(/\d/, 'Password must have at least 1 number')
+  .regex(
+    /[!@#$%^&*(),.?":{}|<>]/,
+    'Password must have at least 1 special character'
+  )
   .pipe(safeStringRefine);
 
 const remember = z.boolean({ message: 'Remember must be a valid boolean' });

src/services/database/lowdb.ts

@@ -40,7 +40,7 @@ export default class LowDB extends DatabaseProvider {
       DEBUG('Migrations ran successfully');
     } catch (e) {
       DEBUG(e);
-      throw new DatabaseError(DatabaseError.ERROR_INIT);
+      throw new Error('Failed to initialize Database');
     }
     this.#connected = true;
     DEBUG('Connected successfully');
@@ -75,32 +75,27 @@ export default class LowDB extends DatabaseProvider {
     return this.#db.data.users.find((user) => user.id === id);
   }
 
-  async newUserWithPassword(username: string, password: string) {
-    DEBUG('New User');
-
-    // TODO: should be handled by zod. completely remove database error
-    if (username.length < 8) {
-      throw new DatabaseError(DatabaseError.ERROR_USERNAME_REQ);
-    }
-
-    if (!isPasswordStrong(password)) {
-      throw new DatabaseError(DatabaseError.ERROR_PASSWORD_REQ);
-    }
+  async createUser(username: string, password: string) {
+    DEBUG('Create User');
 
-    // TODO: multiple names are no problem
     const isUserExist = this.#db.data.users.find(
       (user) => user.username === username
     );
     if (isUserExist) {
-      throw new DatabaseError(DatabaseError.ERROR_USER_EXIST);
+      throw createError({
+        statusCode: 409,
+        statusMessage: 'Username already taken',
+      });
     }
 
     const now = new Date().toISOString();
     const isUserEmpty = this.#db.data.users.length === 0;
 
+    const hash = await hashPassword(password);
+
     const newUser: User = {
       id: crypto.randomUUID(),
-      password: hashPassword(password),
+      password: hash,
       username,
       name: 'Administrator',
       role: isUserEmpty ? 'ADMIN' : 'CLIENT',

src/services/database/repositories/database.ts

@@ -46,10 +46,7 @@ export abstract class DatabaseProvider
 
   abstract getUsers(): Promise<User[]>;
   abstract getUser(id: string): Promise<User | undefined>;
-  abstract newUserWithPassword(
-    username: string,
-    password: string
-  ): Promise<void>;
+  abstract createUser(username: string, password: string): Promise<void>;
   abstract updateUser(user: User): Promise<void>;
   abstract deleteUser(id: string): Promise<void>;
 
@@ -82,7 +79,7 @@ export abstract class DatabaseProvider
  *
  * Example:
  * ```typescript
- * throw new DatabaseError(DatabaseError.ERROR_PASSWORD_REQ);
+ * throw new DatabaseError(DatabaseError.ERROR_INIT);
  * ...
  * // event handler routes
  * if (error instanceof DatabaseError) {
@@ -101,10 +98,6 @@ export abstract class DatabaseProvider
  */
 export class DatabaseError extends Error {
   static readonly ERROR_INIT = 'errorInit';
-  static readonly ERROR_PASSWORD_REQ = 'errorPasswordReq';
-  static readonly ERROR_USER_EXIST = 'errorUserExist';
-  static readonly ERROR_DATABASE_CONNECTION = 'errorDatabaseConn';
-  static readonly ERROR_USERNAME_REQ = 'errorUsernameReq';
 
   constructor(message: string) {
     super(message);

src/services/database/repositories/user.ts

@@ -39,7 +39,7 @@ export interface UserRepository {
    */
   getUser(id: string): Promise<User | undefined>;
 
-  newUserWithPassword(username: string, password: string): Promise<void>;
+  createUser(username: string, password: string): Promise<void>;
 
   /**
    * Updates a user in the database.