Fixing totp mfa enable, disable, and recovery codes.

package.json

@@ -33,7 +33,7 @@
 		"@sveltejs/kit": "^2.5.25",
 		"@sveltejs/vite-plugin-svelte": "^3.1.2",
 		"@types/cookie": "^0.6.0",
-		"@types/node": "^20.16.2",
+		"@types/node": "^20.16.3",
 		"@types/pg": "^8.11.8",
 		"@typescript-eslint/eslint-plugin": "^7.18.0",
 		"@typescript-eslint/parser": "^7.18.0",
@@ -46,8 +46,8 @@
 		"just-debounce-it": "^3.2.0",
 		"lucia": "3.2.0",
 		"lucide-svelte": "^0.408.0",
-		"nodemailer": "^6.9.14",
-		"postcss": "^8.4.41",
+		"nodemailer": "^6.9.15",
+		"postcss": "^8.4.44",
 		"postcss-import": "^16.1.0",
 		"postcss-load-config": "^5.1.0",
 		"postcss-preset-env": "^9.6.0",
@@ -70,7 +70,7 @@
 		"tslib": "^2.7.0",
 		"tsx": "^4.19.0",
 		"typescript": "^5.5.4",
-		"vite": "^5.4.2",
+		"vite": "^5.4.3",
 		"vitest": "^1.6.0",
 		"zod": "^3.23.8"
 	},
@@ -85,7 +85,7 @@
 		"@internationalized/date": "^3.5.5",
 		"@lucia-auth/adapter-drizzle": "^1.1.0",
 		"@lukeed/uuid": "^2.0.1",
-		"@neondatabase/serverless": "^0.9.4",
+		"@neondatabase/serverless": "^0.9.5",
 		"@paralleldrive/cuid2": "^2.2.2",
 		"@resvg/resvg-js": "^2.6.2",
 		"@sveltejs/adapter-node": "^5.2.2",
@@ -95,7 +95,7 @@
 		"arctic": "^1.9.2",
 		"bits-ui": "^0.21.13",
 		"boardgamegeekclient": "^1.9.1",
-		"bullmq": "^5.12.12",
+		"bullmq": "^5.12.13",
 		"class-variance-authority": "^0.7.0",
 		"clsx": "^2.1.1",
 		"cookie": "^0.6.0",
@@ -106,7 +106,7 @@
 		"feather-icons": "^4.29.2",
 		"formsnap": "^1.0.1",
 		"handlebars": "^4.7.8",
-		"hono": "^4.5.9",
+		"hono": "^4.5.11",
 		"hono-rate-limiter": "^0.4.0",
 		"html-entities": "^2.5.2",
 		"iconify-icon": "^2.1.0",

src/lib/components/LeftNav.svelte

@@ -0,0 +1,77 @@
+<script lang="ts">
+import { page } from '$app/stores'
+
+type Route = {
+	href: string
+	label: string
+}
+
+let { children, routes }: { children: unknown; routes: Route[] } = $props()
+</script>
+
+<div class="security-nav">
+	<nav>
+		<ul>
+			{#each routes as { href, label }}
+				<li>
+					<a href={href} class:active={$page.url.pathname === href}>
+						{label}
+					</a>
+				</li>
+			{/each}
+		</ul>
+	</nav>
+	<div class="security-nav-content">
+		{@render children()}
+	</div>
+</div>
+
+<style lang="postcss">
+	.security-nav {
+		display: flex;
+
+		nav {
+			width: 16rem;
+			position: sticky;
+			top: 0;
+			left: 0;
+			background-color: #fff;
+			padding: 1rem;
+			border-right: 1px solid #ddd;
+			height: 100vh;
+
+			ul {
+				list-style-type: none;
+				padding: 0;
+				margin: 0;
+			}
+
+			li {
+				margin-bottom: 0.5rem;
+			}
+
+			a {
+				text-decoration: none;
+				color: #337ab7;
+				display: block;
+				padding: 0.5rem;
+				border-radius: 0.25rem;
+
+				&:hover {
+					background-color: #f8f9fa;
+				}
+
+				&.active {
+					color: #23527c;
+					font-weight: bold;
+					background-color: #e9ecef;
+				}
+			}
+		}
+	}
+
+	.security-nav-content {
+		flex: 1;
+		padding: 1rem;
+	}
+</style>

src/lib/server/api/controllers/mfa.controller.ts

@@ -2,13 +2,15 @@ import 'reflect-metadata'
 import { StatusCodes } from '$lib/constants/status-codes'
 import type { Controller } from '$lib/server/api/common/interfaces/controller.interface'
 import { verifyTotpDto } from '$lib/server/api/dtos/verify-totp.dto'
+import { db } from '$lib/server/api/packages/drizzle'
+import { RecoveryCodesService } from '$lib/server/api/services/recovery-codes.service'
 import { TotpService } from '$lib/server/api/services/totp.service'
+import { UsersService } from '$lib/server/api/services/users.service'
 import { zValidator } from '@hono/zod-validator'
 import { Hono } from 'hono'
 import { inject, injectable } from 'tsyringe'
 import { CredentialsType } from '../databases/tables'
 import { requireAuth } from '../middleware/auth.middleware'
-import { UsersService } from '../services/users.service'
 import type { HonoTypes } from '../types'
 
 @injectable()
@@ -16,6 +18,7 @@ export class MfaController implements Controller {
 	controller = new Hono<HonoTypes>()
 
 	constructor(
+		@inject(RecoveryCodesService) private readonly recoveryCodesService: RecoveryCodesService,
 		@inject(TotpService) private readonly totpService: TotpService,
 		@inject(UsersService) private readonly usersService: UsersService,
 	) {}
@@ -36,6 +39,8 @@ export class MfaController implements Controller {
 				const user = c.var.user
 				try {
 					await this.totpService.deleteOneByUserIdAndType(user.id, CredentialsType.TOTP)
+					await this.recoveryCodesService.deleteAllRecoveryCodesByUserId(user.id)
+					await this.usersService.updateUser(user.id, { mfa_enabled: false })
 					console.log('TOTP deleted')
 					return c.body(null, StatusCodes.NO_CONTENT)
 				} catch (e) {
@@ -43,16 +48,26 @@ export class MfaController implements Controller {
 					return c.status(StatusCodes.INTERNAL_SERVER_ERROR)
 				}
 			})
+			.get('/totp/recoveryCodes', requireAuth, async (c) => {
+				const user = c.var.user
+				// You can only view recovery codes once and that is on creation
+				const existingCodes = await this.recoveryCodesService.findAllRecoveryCodesByUserId(user.id)
+				if (existingCodes) {
+					return c.body('You have already generated recovery codes', StatusCodes.BAD_REQUEST)
+				}
+				const recoveryCodes = await this.recoveryCodesService.createRecoveryCodes(user.id)
+				return c.json({ recoveryCodes })
+			})
 			.post('/totp/verify', requireAuth, zValidator('json', verifyTotpDto), async (c) => {
 				try {
 					const user = c.var.user
 					const { code } = c.req.valid('json')
 					const verified = await this.totpService.verify(user.id, code)
 					if (verified) {
-						this.usersService.updateUser(user.id, { mfa_enabled: true })
+						await this.usersService.updateUser(user.id, { mfa_enabled: true })
 						return c.json({}, StatusCodes.OK)
 					}
-					return c.json({}, StatusCodes.BAD_REQUEST)
+					return c.json('Invalid code', StatusCodes.BAD_REQUEST)
 				} catch (e) {
 					console.error(e)
 					return c.status(StatusCodes.INTERNAL_SERVER_ERROR)

src/lib/server/api/repositories/credentials.repository.ts

@@ -1,3 +1,4 @@
+import 'reflect-metadata'
 import type { Repository } from '$lib/server/api/common/interfaces/repository.interface'
 import { CredentialsType, credentialsTable } from '$lib/server/api/databases/tables/credentials.table'
 import { DatabaseProvider } from '$lib/server/api/providers/database.provider'

src/lib/server/api/repositories/recovery-codes.repository.ts

@@ -0,0 +1,27 @@
+import 'reflect-metadata'
+import type { Repository } from '$lib/server/api/common/interfaces/repository.interface'
+import { DatabaseProvider } from '$lib/server/api/providers/database.provider'
+import { type InferInsertModel, eq } from 'drizzle-orm'
+import { inject, injectable } from 'tsyringe'
+import { recoveryCodesTable } from '../databases/tables'
+
+export type CreateRecoveryCodes = InferInsertModel<typeof recoveryCodesTable>
+
+@injectable()
+export class RecoveryCodesRepository implements Repository {
+	constructor(@inject(DatabaseProvider) private readonly db: DatabaseProvider) {}
+
+	async findAllByUserId(userId: string) {
+		return this.db.query.recoveryCodesTable.findFirst({
+			where: eq(recoveryCodesTable.userId, userId),
+		})
+	}
+
+	async deleteAllByUserId(userId: string) {
+		return this.db.delete(recoveryCodesTable).where(eq(recoveryCodesTable.userId, userId))
+	}
+
+	trxHost(trx: DatabaseProvider) {
+		return new RecoveryCodesRepository(trx)
+	}
+}

src/lib/server/api/services/recovery-codes.service.ts

@@ -0,0 +1,38 @@
+import 'reflect-metadata'
+import { recoveryCodesTable } from '$lib/server/api/databases/tables'
+import { db } from '$lib/server/api/packages/drizzle'
+import { RecoveryCodesRepository } from '$lib/server/api/repositories/recovery-codes.repository'
+import { alphabet, generateRandomString } from 'oslo/crypto'
+import { Argon2id } from 'oslo/password'
+import { inject, injectable } from 'tsyringe'
+
+@injectable()
+export class RecoveryCodesService {
+	constructor(@inject(RecoveryCodesRepository) private readonly recoveryCodesRepository: RecoveryCodesRepository) {}
+
+	async findAllRecoveryCodesByUserId(userId: string) {
+		return this.recoveryCodesRepository.findAllByUserId(userId)
+	}
+
+	async createRecoveryCodes(userId: string) {
+		const createdRecoveryCodes = Array.from({ length: 5 }, () => generateRandomString(10, alphabet('A-Z', '0-9')))
+		if (createdRecoveryCodes && userId) {
+			for (const code of createdRecoveryCodes) {
+				const hashedCode = await new Argon2id().hash(code)
+				console.log('Inserting recovery code', code, hashedCode)
+				await db.insert(recoveryCodesTable).values({
+					userId,
+					code: hashedCode,
+				})
+			}
+
+			return createdRecoveryCodes
+		}
+
+		return []
+	}
+
+	async deleteAllRecoveryCodesByUserId(userId: string) {
+		return this.recoveryCodesRepository.deleteAllByUserId(userId)
+	}
+}

src/routes/(app)/(protected)/profile/security/+layout.server.ts

@@ -0,0 +1,3 @@
+export const load = async () => {
+	return {}
+}

src/routes/(app)/(protected)/profile/security/+layout.svelte

@@ -0,0 +1,15 @@
+<script lang="ts">
+import LeftNav from '$components/LeftNav.svelte'
+
+let { children } = $props()
+
+const routes = [
+	{ href: '/profile/security', label: 'Security' },
+	{ href: '/profile/security/mfa', label: 'MFA' },
+	{ href: '/profile/security/password/change', label: 'Change Password' },
+]
+</script>
+
+<LeftNav {routes}>
+	{@render children()}
+</LeftNav>

src/routes/(app)/(protected)/profile/security/mfa/+page.server.ts

@@ -1,22 +1,15 @@
-import { StatusCodes } from '$lib/constants/status-codes'
 import { notSignedInMessage } from '$lib/flashMessages'
-import { db } from '$lib/server/api/packages/drizzle'
-import { userNotAuthenticated } from '$lib/server/auth-utils'
 import { addTwoFactorSchema, removeTwoFactorSchema } from '$lib/validations/account'
 import env from '$src/env'
-import { type Actions, error, fail } from '@sveltejs/kit'
-import { eq } from 'drizzle-orm'
+import { type Actions, fail } from '@sveltejs/kit'
 import kebabCase from 'just-kebab-case'
-import { HMAC } from 'oslo/crypto'
-import { decodeHex, encodeHex } from 'oslo/encoding'
-import { TOTPController, createTOTPKeyURI } from 'oslo/otp'
-import { Argon2id } from 'oslo/password'
+import { base32, decodeHex } from 'oslo/encoding'
+import { createTOTPKeyURI } from 'oslo/otp'
 import QRCode from 'qrcode'
-import { redirect, setFlash } from 'sveltekit-flash-message/server'
+import { redirect } from 'sveltekit-flash-message/server'
 import { zod } from 'sveltekit-superforms/adapters'
 import { setError, superValidate } from 'sveltekit-superforms/server'
 import type { PageServerLoad } from '../../$types'
-import { type Credentials, credentialsTable, recoveryCodesTable, usersTable } from '../../../../../../lib/server/api/databases/tables'
 
 export const load: PageServerLoad = async (event) => {
 	const { locals } = event
@@ -69,7 +62,11 @@ export const load: PageServerLoad = async (event) => {
 			addTwoFactorForm,
 		})
 	}
-	const totpUri = createTOTPKeyURI(issuer, accountName, decodeHex(createdTotpCredentials.secret_data))
+	const decodedHexSecret = decodeHex(createdTotpCredentials.secret_data)
+	const secret = base32.encode(new Uint8Array(decodedHexSecret), {
+		includePadding: false,
+	})
+	const totpUri = createTOTPKeyURI(issuer, accountName, decodedHexSecret)
 
 	addTwoFactorForm.data = {
 		current_password: '',
@@ -82,6 +79,7 @@ export const load: PageServerLoad = async (event) => {
 		recoveryCodes: [],
 		totpUri,
 		qrCode: await QRCode.toDataURL(totpUri),
+		secret,
 	}
 }
 
@@ -118,21 +116,25 @@ export const actions: Actions = {
 		}
 
 		const twoFactorCode = addTwoFactorForm.data.two_factor_code
-		const { error: verifyTotpError } = locals.api.mfa.totp.verify
+		const { error: verifyTotpError } = await locals.api.mfa.totp.verify
 			.$post({
 				json: { code: twoFactorCode },
 			})
 			.then(locals.parseApiResponse)
-
 		if (verifyTotpError) {
 			return setError(addTwoFactorForm, 'two_factor_code', 'Invalid code')
 		}
 
-		redirect(302, '/profile/security/two-factor/recovery-codes')
+		redirect(302, '/profile/security/mfa/recovery-codes')
 	},
 	disableTotp: async (event) => {
 		const { locals } = event
-		const { user, session } = locals
+
+		const authedUser = await locals.getAuthedUser()
+		if (!authedUser) {
+			throw redirect(302, '/login', notSignedInMessage, event)
+		}
+
 		const removeTwoFactorForm = await superValidate(event, zod(removeTwoFactorSchema))
 
 		if (!removeTwoFactorForm.valid) {
@@ -140,51 +142,27 @@ export const actions: Actions = {
 				removeTwoFactorForm,
 			})
 		}
-
-		if (!user || !session) {
-			return fail(401, {
-				removeTwoFactorForm,
+		const { error: verifyPasswordError } = await locals.api.me.verify.password
+			.$post({
+				json: { password: removeTwoFactorForm.data.current_password },
 			})
-		}
+			.then(locals.parseApiResponse)
 
-		const dbUser = await db.query.usersTable.findFirst({
-			where: eq(usersTable.id, user.id),
-		})
-
-		// if (!dbUser?.hashed_password) {
-		// 	removeTwoFactorForm.data.current_password = '';
-		// 	return setError(
-		// 		removeTwoFactorForm,
-		// 		'Error occurred. Please try again or contact support if you need further help.',
-		// 	);
-		// }
-
-		const currentPasswordVerified = await new Argon2id().verify(
-			// dbUser.hashed_password,
-			removeTwoFactorForm.data.current_password,
-		)
-
-		if (!currentPasswordVerified) {
+		if (verifyPasswordError) {
+			console.log(verifyPasswordError)
 			return setError(removeTwoFactorForm, 'current_password', 'Your password is incorrect')
 		}
 
-		const twoFactorDetails = await db.query.twoFactor.findFirst({
-			where: eq(twoFactor.userId, dbUser.id),
-		})
-
-		if (!twoFactorDetails) {
+		const { error: deleteTotpError } = await locals.api.mfa.totp.$delete().then(locals.parseApiResponse)
+		if (deleteTotpError) {
 			return fail(500, {
 				removeTwoFactorForm,
 			})
 		}
 
-		await db.update(twoFactor).set({ enabled: false }).where(eq(twoFactor.userId, user.id))
-		await db.delete(recoveryCodes).where(eq(recoveryCodes.userId, user.id))
-
-		// setFlash({ type: 'success', message: 'Two-Factor Authentication has been disabled.' }, cookies);
 		redirect(
 			302,
-			'/profile/security/two-factor',
+			'/profile/security/mfa',
 			{
 				type: 'success',
 				message: 'Two-Factor Authentication has been disabled.',

src/routes/(app)/(protected)/profile/security/mfa/+page.svelte

@@ -1,37 +1,38 @@
 <script lang="ts">
-	import { zodClient } from 'sveltekit-superforms/adapters';
-	import { superForm } from 'sveltekit-superforms/client';
-	import { AlertTriangle } from 'lucide-svelte';
-	import * as Alert from '$components/ui/alert';
-	import * as Form from '$components/ui/form';
-	import { Input } from '$components/ui/input';
-	import { addTwoFactorSchema, removeTwoFactorSchema } from '$lib/validations/account';
-	import PinInput from '$components/pin-input.svelte';
+import PinInput from '$components/pin-input.svelte'
+import * as Alert from '$components/ui/alert'
+import * as Form from '$components/ui/form'
+import { Input } from '$components/ui/input'
+import { addTwoFactorSchema, removeTwoFactorSchema } from '$lib/validations/account'
+import { AlertTriangle } from 'lucide-svelte'
+import { zodClient } from 'sveltekit-superforms/adapters'
+import { superForm } from 'sveltekit-superforms/client'
 
-	export let data;
+export let data
 
-	const { qrCode, twoFactorEnabled, recoveryCodes } = data;
+const { qrCode, secret, twoFactorEnabled, recoveryCodes } = data
 
 const addTwoFactorForm = superForm(data.addTwoFactorForm, {
 	taintedMessage: null,
 	validators: zodClient(addTwoFactorSchema),
 	delayMs: 500,
 	multipleSubmits: 'prevent',
-	});
+})
 
 const removeTwoFactorForm = superForm(data.removeTwoFactorForm, {
 	taintedMessage: null,
 	validators: zodClient(removeTwoFactorSchema),
 	delayMs: 500,
 	multipleSubmits: 'prevent',
-	});
+})
 
-	console.log('Two Factor: ', twoFactorEnabled, recoveryCodes);
+console.log('Two Factor: ', twoFactorEnabled, recoveryCodes)
 
-	const { form: addTwoFactorFormData, enhance: addTwoFactorEnhance } = addTwoFactorForm;
-	const { form: removeTwoFactorFormData, enhance: removeTwoFactorEnhance } = removeTwoFactorForm;
+const { form: addTwoFactorFormData, enhance: addTwoFactorEnhance } = addTwoFactorForm
+const { form: removeTwoFactorFormData, enhance: removeTwoFactorEnhance } = removeTwoFactorForm
 </script>
 
+<section class="two-factor">
 <h1>Two-Factor Authentication</h1>
 
 {#if twoFactorEnabled}
@@ -70,4 +71,13 @@
 		</Form.Field>
 		<Form.Button>Submit</Form.Button>
 	</form>
+	<span>Secret: {secret}</span>
 {/if}
+</section>
+
+<style lang="postcss">
+	section {
+		max-width: 20rem;
+		line-break: anywhere;
+	}
+</style>

src/routes/(app)/(protected)/profile/security/mfa/recovery-codes/+page.server.ts

@@ -1,11 +1,6 @@
 import { notSignedInMessage } from '$lib/flashMessages'
-import { db } from '$lib/server/api/packages/drizzle'
-import { eq } from 'drizzle-orm'
-import { alphabet, generateRandomString } from 'oslo/crypto'
-import { Argon2id } from 'oslo/password'
 import { redirect } from 'sveltekit-flash-message/server'
 import type { PageServerLoad } from '../../../$types'
-import { recoveryCodesTable } from '../../../../../../../lib/server/api/databases/tables'
 
 export const load: PageServerLoad = async (event) => {
 	const { locals } = event
@@ -16,30 +11,18 @@ export const load: PageServerLoad = async (event) => {
 	}
 
 	if (authedUser.mfa_enabled) {
-		const dbRecoveryCodes = await db.query.recoveryCodesTable.findMany({
-			where: eq(recoveryCodesTable.userId, authedUser.id),
-		})
-
-		if (dbRecoveryCodes.length === 0) {
-			const createdRecoveryCodes = Array.from({ length: 5 }, () => generateRandomString(10, alphabet('A-Z', '0-9')))
-			if (createdRecoveryCodes) {
-				for (const code of createdRecoveryCodes) {
-					const hashedCode = await new Argon2id().hash(code)
-					console.log('Inserting recovery code', code, hashedCode)
-					await db.insert(recoveryCodesTable).values({
-						userId: authedUser.id,
-						code: hashedCode,
-					})
-				}
-			}
-			return {
-				recoveryCodes: createdRecoveryCodes,
-			}
-		}
+		const { data: recoveryCodesData, error: recoveryCodesError } = await locals.api.mfa.totp.recoveryCodes.$get().then(locals.parseApiResponse)
+		console.log('recoveryCodesData', recoveryCodesData)
+		console.log('recoveryCodesError', recoveryCodesError)
+		if (recoveryCodesError || !recoveryCodesData || !recoveryCodesData.recoveryCodes) {
 			return {
 				recoveryCodes: [],
 			}
 		}
-	console.error('2FA not enabled')
+		return {
+			recoveryCodes: recoveryCodesData.recoveryCodes,
+		}
+	}
+
 	redirect(302, '/profile', { message: 'Two-Factor Authentication is not enabled', type: 'error' }, event)
 }