nullable password, prevent timing attack

this prevents timing attacks by always checking hash even if there is none
prevents using basic auth if 2fa is enabled

src/server/database/migrations/0005_quiet_sentinels.sql

@@ -1,2 +0,0 @@
-ALTER TABLE `users_table` ADD `oauth_provider` text;--> statement-breakpoint
-ALTER TABLE `users_table` ADD `oauth_id` text;

src/server/database/migrations/0005_supreme_groot.sql

@@ -0,0 +1,22 @@
+PRAGMA foreign_keys=OFF;--> statement-breakpoint
+CREATE TABLE `__new_users_table` (
+	`id` integer PRIMARY KEY AUTOINCREMENT NOT NULL,
+	`username` text NOT NULL,
+	`password` text,
+	`email` text,
+	`name` text NOT NULL,
+	`role` integer NOT NULL,
+	`totp_key` text,
+	`totp_verified` integer NOT NULL,
+	`enabled` integer NOT NULL,
+	`oauth_provider` text,
+	`oauth_id` text,
+	`created_at` text DEFAULT (CURRENT_TIMESTAMP) NOT NULL,
+	`updated_at` text DEFAULT (CURRENT_TIMESTAMP) NOT NULL
+);
+--> statement-breakpoint
+INSERT INTO `__new_users_table`("id", "username", "password", "email", "name", "role", "totp_key", "totp_verified", "enabled", "oauth_provider", "oauth_id", "created_at", "updated_at") SELECT "id", "username", "password", "email", "name", "role", "totp_key", "totp_verified", "enabled", "oauth_provider", "oauth_id", "created_at", "updated_at" FROM `users_table`;--> statement-breakpoint
+DROP TABLE `users_table`;--> statement-breakpoint
+ALTER TABLE `__new_users_table` RENAME TO `users_table`;--> statement-breakpoint
+PRAGMA foreign_keys=ON;--> statement-breakpoint
+CREATE UNIQUE INDEX `users_table_username_unique` ON `users_table` (`username`);

src/server/database/migrations/meta/0005_snapshot.json

@@ -1,7 +1,7 @@
 {
   "version": "6",
   "dialect": "sqlite",
-  "id": "1d6d806f-441e-4f18-b84b-3e9232e45359",
+  "id": "840b00a9-8e9d-4bc9-a0fa-6185ebb01a46",
   "prevId": "0f072f91-cd10-4702-ae7b-245255d69d1e",
   "tables": {
     "clients_table": {
@@ -749,7 +749,7 @@
           "name": "password",
           "type": "text",
           "primaryKey": false,
-          "notNull": true,
+          "notNull": false,
           "autoincrement": false
         },
         "email": {

src/server/database/migrations/meta/_journal.json

@@ -40,8 +40,8 @@
     {
       "idx": 5,
       "version": "6",
-      "when": 1779787680891,
-      "tag": "0005_quiet_sentinels",
+      "when": 1779862471761,
+      "tag": "0005_supreme_groot",
       "breakpoints": true
     }
   ]

src/server/database/repositories/user/schema.ts

@@ -6,7 +6,8 @@ import { client } from '../../schema';
 export const user = sqliteTable('users_table', {
   id: int().primaryKey({ autoIncrement: true }),
   username: text().notNull().unique(),
-  password: text().notNull(),
+  /** `password == null` means password login disabled */
+  password: text(),
   email: text(),
   name: text().notNull(),
   role: int().$type<Role>().notNull(),

src/server/database/repositories/user/service.ts

@@ -144,7 +144,7 @@ export class UserService {
     // Create new user
     await this.#db.insert(user).values({
       username,
-      password: '--- no password ---',
+      password: null,
       email,
       name,
       role: roles.ADMIN,
@@ -233,13 +233,11 @@ export class UserService {
         .findFirst({ where: eq(user.username, username) })
         .execute();
 
-      if (!txUser) {
-        return { success: false, error: 'INCORRECT_CREDENTIALS' };
-      }
+      // always check to avoid timing attack
+      const userHashPassword = txUser?.password ?? null;
+      const passwordValid = await isPasswordValid(password, userHashPassword);
 
-      const passwordValid = await isPasswordValid(password, txUser.password);
-
-      if (!passwordValid) {
+      if (!txUser || !passwordValid) {
         return { success: false, error: 'INCORRECT_CREDENTIALS' };
       }
 

src/server/utils/password.ts

@@ -3,13 +3,22 @@
 import argon2 from 'argon2';
 import { deserialize } from '@phc/format';
 
+const DUMMY_HASH =
+  '$argon2id$v=19$m=65536,t=3,p=4$jsh6z1/SbZHYAiO/Ww9HZw$ikzkoXWqc2b0Pc4O8ZNJjp1xKZSb7SNM/3dPMNUPk9Y';
+
 /**
- * Checks if `password` matches the hash.
+ * Checks if `password` matches the `hash`.
+ *
+ * Checks against `DUMMY_HASH` and returns false if `hash` is null
  */
-export function isPasswordValid(
+export async function isPasswordValid(
   password: string,
-  hash: string
+  hash: string | null
 ): Promise<boolean> {
+  if (hash === null) {
+    await argon2.verify(DUMMY_HASH, password);
+    return false;
+  }
   return argon2.verify(hash, password);
 }
 

src/server/utils/session.ts

@@ -73,21 +73,14 @@ export async function getCurrentUser(event: H3Event) {
       });
     }
 
-    // TODO: timing can be used to enumerate usernames
-
     const foundUser = await Database.users.getByUsername(username);
 
-    if (!foundUser) {
-      throw createError({
-        statusCode: 401,
-        statusMessage: 'Session failed',
-      });
-    }
-
-    const userHashPassword = foundUser.password;
+    // always check to avoid timing attack
+    const userHashPassword = foundUser?.password ?? null;
     const passwordValid = await isPasswordValid(password, userHashPassword);
 
-    if (!passwordValid) {
+    // can't login through basic auth if 2fa enabled
+    if (!foundUser || !passwordValid || foundUser.totpVerified) {
       throw createError({
         statusCode: 401,
         statusMessage: 'Session failed',