Harden host handling, GitHub email linking, and password reset

.env.example

@@ -17,6 +17,11 @@ GITHUB_CLIENT_SECRET="MOCK_GITHUB_CLIENT_SECRET"
 GITHUB_TOKEN="MOCK_GITHUB_TOKEN"
 GITHUB_REDIRECT_URI="https://example.com/auth/github/callback"
 
+# comma-separated hostnames your app is served from. The first one is the
+# canonical host used for links in emails when a request's Host header isn't in
+# this list. `${FLY_APP_NAME}.fly.dev` and localhost are always allowed.
+# ALLOWED_HOSTS="example.com,www.example.com"
+
 # set this to false to prevent search engines from indexing the website
 # default to allow indexing for seo safety
 ALLOW_INDEXING="true"

SECURITY.md

@@ -0,0 +1,20 @@
+# Security Policy
+
+## Reporting a Vulnerability
+
+Please do not report security vulnerabilities in public issues, discussions, or
+pull requests.
+
+Instead, report them privately using
+[GitHub private vulnerability reporting](https://github.com/epicweb-dev/epic-stack/security/advisories/new).
+Please include a description of the issue, the steps to reproduce it, and the
+affected files or versions if you know them.
+
+We'll respond as soon as we can and keep you updated while we work on a fix.
+
+## Supported Versions
+
+The Epic Stack is a template, so fixes are made on the `main` branch. Apps
+generated from the Epic Stack don't receive updates automatically. See
+[managing updates](./docs/managing-updates.md) for how to bring fixes into your
+app.

app/routes/_auth/auth.$provider/callback.test.ts

@@ -1,7 +1,7 @@
 import { invariant } from '@epic-web/invariant'
 import { faker } from '@faker-js/faker'
 import { SetCookie } from '@mjackson/headers'
-import { http } from 'msw'
+import { HttpResponse, http } from 'msw'
 import { type AppLoadContext } from 'react-router'
 import { afterEach, expect, test } from 'vitest'
 import { twoFAVerificationType } from '#app/routes/settings/profile/two-factor/_layout.tsx'
@@ -156,6 +156,38 @@ test('when a user exists with the same email, create connection and make session
 	await expect(response).toHaveSessionForUser(userId)
 })
 
+test('does not sign in to an existing user when the GitHub primary email is unverified', async () => {
+	consoleError.mockImplementation(() => {})
+	const githubUser = await insertGitHubUser()
+	const email = githubUser.primaryEmail.toLowerCase()
+	const { userId } = await setupUser({ ...createUser(), email })
+	server.use(
+		http.get('https://api.github.com/user/emails', () =>
+			HttpResponse.json(
+				githubUser.emails.map((e) =>
+					e.primary ? { ...e, verified: false } : e,
+				),
+			),
+		),
+	)
+	const request = await setupRequest({ code: githubUser.code })
+	const response = await loader({
+		request,
+		...LOADER_ARGS_BASE,
+	}).catch((e) => e)
+
+	invariant(response instanceof Response, 'response should be a Response')
+	expect(response).toHaveRedirect('/login')
+	await expect(response).toSendToast(
+		expect.objectContaining({ title: 'Auth Failed', type: 'error' }),
+	)
+	expect(response.headers.get('set-cookie')).not.toContain('en_session=')
+	const connection = await prisma.connection.findFirst({
+		where: { userId, providerId: githubUser.profile.id.toString() },
+	})
+	expect(connection, 'a connection should not have been created').toBeNull()
+})
+
 test('gives an error if the account is already connected to another user', async () => {
 	const githubUser = await insertGitHubUser()
 	await prisma.user.create({

app/routes/_auth/forgot-password.test.ts

@@ -0,0 +1,127 @@
+import { invariant } from '@epic-web/invariant'
+import { type AppLoadContext } from 'react-router'
+import { afterEach, expect, test, vi } from 'vitest'
+import { prisma } from '#app/utils/db.server.ts'
+import { createUser } from '#tests/db-utils.ts'
+import { requireEmail } from '#tests/mocks/utils.ts'
+import { BASE_URL } from '#tests/utils.ts'
+import { type Route } from './+types/forgot-password.ts'
+import { action } from './forgot-password.tsx'
+
+const ROUTE_PATH = '/forgot-password'
+const ACTION_ARGS_BASE = {
+	params: {},
+	context: {} as AppLoadContext,
+	url: new URL(ROUTE_PATH, BASE_URL),
+	pattern: ROUTE_PATH,
+} satisfies Omit<Route.ActionArgs, 'request'>
+
+afterEach(() => {
+	vi.unstubAllEnvs()
+})
+
+async function setupUser() {
+	return prisma.user.create({
+		data: createUser(),
+		select: { email: true, username: true },
+	})
+}
+
+function createForgotPasswordRequest(
+	usernameOrEmail: string,
+	headers: Record<string, string> = {},
+) {
+	return new Request(new URL(ROUTE_PATH, BASE_URL), {
+		method: 'POST',
+		headers,
+		body: new URLSearchParams({ usernameOrEmail }),
+	})
+}
+
+async function getEmailedVerifyUrl(email: string) {
+	const sentEmail = await requireEmail(email)
+	const link = sentEmail.text.match(
+		/https?:\/\/[^\s\]]+\/verify\?[^\s\]]+/,
+	)?.[0]
+	invariant(link, 'verify link not found in email')
+	return new URL(link)
+}
+
+test('emailed reset link ignores an untrusted X-Forwarded-Host', async () => {
+	vi.stubEnv('ALLOWED_HOSTS', 'www.epicstack.dev')
+	const user = await setupUser()
+	const response = await action({
+		...ACTION_ARGS_BASE,
+		request: createForgotPasswordRequest(user.username, {
+			host: 'www.epicstack.dev',
+			'X-Forwarded-Host': 'attacker.example',
+			'X-Forwarded-Proto': 'https',
+		}),
+	})
+	invariant(response instanceof Response, 'response should be a Response')
+	expect(response.headers.get('location')).toMatch(
+		/^https:\/\/www\.epicstack\.dev\/verify\?/,
+	)
+	const verifyUrl = await getEmailedVerifyUrl(user.email)
+	expect(verifyUrl.origin).toBe('https://www.epicstack.dev')
+	expect(verifyUrl.searchParams.get('code')).toBeTruthy()
+})
+
+test('emailed reset link falls back to the canonical host for an untrusted Host header', async () => {
+	vi.stubEnv('ALLOWED_HOSTS', 'www.epicstack.dev,epicstack.dev')
+	const user = await setupUser()
+	await action({
+		...ACTION_ARGS_BASE,
+		request: createForgotPasswordRequest(user.email, {
+			host: 'attacker.example',
+			'X-Forwarded-Proto': 'https',
+		}),
+	})
+	const verifyUrl = await getEmailedVerifyUrl(user.email)
+	expect(verifyUrl.origin).toBe('https://www.epicstack.dev')
+})
+
+test('emailed reset link uses the request host when it is allowed', async () => {
+	vi.stubEnv('ALLOWED_HOSTS', 'www.epicstack.dev,epicstack.dev')
+	const user = await setupUser()
+	await action({
+		...ACTION_ARGS_BASE,
+		request: createForgotPasswordRequest(user.username, {
+			host: 'EpicStack.dev',
+			'X-Forwarded-Proto': 'https',
+		}),
+	})
+	const verifyUrl = await getEmailedVerifyUrl(user.email)
+	expect(verifyUrl.origin).toBe('https://epicstack.dev')
+})
+
+test('the Fly.io app hostname is allowed without configuration', async () => {
+	vi.stubEnv('ALLOWED_HOSTS', '')
+	vi.stubEnv('FLY_APP_NAME', 'epic-stack-test')
+	const user = await setupUser()
+	await action({
+		...ACTION_ARGS_BASE,
+		request: createForgotPasswordRequest(user.username, {
+			host: 'epic-stack-test.fly.dev',
+			'X-Forwarded-Proto': 'https',
+		}),
+	})
+	const verifyUrl = await getEmailedVerifyUrl(user.email)
+	expect(verifyUrl.origin).toBe('https://epic-stack-test.fly.dev')
+})
+
+test('falls back to the Fly.io app hostname for an untrusted host', async () => {
+	vi.stubEnv('ALLOWED_HOSTS', '')
+	vi.stubEnv('FLY_APP_NAME', 'epic-stack-test')
+	const user = await setupUser()
+	await action({
+		...ACTION_ARGS_BASE,
+		request: createForgotPasswordRequest(user.username, {
+			host: 'epic-stack-test.fly.dev',
+			'X-Forwarded-Host': 'attacker.example',
+			'X-Forwarded-Proto': 'https',
+		}),
+	})
+	const verifyUrl = await getEmailedVerifyUrl(user.email)
+	expect(verifyUrl.origin).toBe('https://epic-stack-test.fly.dev')
+})

app/routes/_auth/reset-password.test.ts

@@ -0,0 +1,60 @@
+import { faker } from '@faker-js/faker'
+import { type AppLoadContext } from 'react-router'
+import { expect, test } from 'vitest'
+import {
+	getSessionExpirationDate,
+	verifyUserPassword,
+} from '#app/utils/auth.server.ts'
+import { prisma } from '#app/utils/db.server.ts'
+import { verifySessionStorage } from '#app/utils/verification.server.ts'
+import { createPassword, createUser } from '#tests/db-utils.ts'
+import { BASE_URL, convertSetCookieToCookie } from '#tests/utils.ts'
+import { type Route } from './+types/reset-password.ts'
+import { action, resetPasswordUsernameSessionKey } from './reset-password.tsx'
+
+const ROUTE_PATH = '/reset-password'
+
+test('resetting a password signs out all existing sessions', async () => {
+	const user = await prisma.user.create({
+		data: {
+			...createUser(),
+			password: { create: createPassword() },
+			sessions: {
+				create: [
+					{ expirationDate: getSessionExpirationDate() },
+					{ expirationDate: getSessionExpirationDate() },
+				],
+			},
+		},
+		select: { id: true, username: true },
+	})
+
+	const verifySession = await verifySessionStorage.getSession()
+	verifySession.set(resetPasswordUsernameSessionKey, user.username)
+	const cookie = convertSetCookieToCookie(
+		await verifySessionStorage.commitSession(verifySession),
+	)
+	const newPassword = faker.internet.password()
+	const request = new Request(new URL(ROUTE_PATH, BASE_URL), {
+		method: 'POST',
+		headers: { cookie },
+		body: new URLSearchParams({
+			password: newPassword,
+			confirmPassword: newPassword,
+		}),
+	})
+
+	const response = await action({
+		request,
+		params: {},
+		context: {} as AppLoadContext,
+		url: new URL(ROUTE_PATH, BASE_URL),
+		pattern: ROUTE_PATH,
+	} satisfies Route.ActionArgs)
+
+	expect(response).toHaveRedirect('/login')
+	expect(await verifyUserPassword({ id: user.id }, newPassword)).toEqual({
+		id: user.id,
+	})
+	expect(await prisma.session.count({ where: { userId: user.id } })).toBe(0)
+})

app/routes/resources/theme-switch.test.ts

@@ -0,0 +1,31 @@
+import { type AppLoadContext } from 'react-router'
+import { expect, test } from 'vitest'
+import { BASE_URL } from '#tests/utils.ts'
+import { type Route } from './+types/theme-switch.ts'
+import { action } from './theme-switch.tsx'
+
+const ROUTE_PATH = '/resources/theme-switch'
+
+function submitTheme(redirectTo: string) {
+	const request = new Request(new URL(ROUTE_PATH, BASE_URL), {
+		method: 'POST',
+		body: new URLSearchParams({ theme: 'dark', redirectTo }),
+	})
+	return action({
+		request,
+		params: {},
+		context: {} as AppLoadContext,
+		url: new URL(ROUTE_PATH, BASE_URL),
+		pattern: ROUTE_PATH,
+	} satisfies Route.ActionArgs)
+}
+
+test('redirects back to the given path', async () => {
+	const response = await submitTheme('/users/kody?tab=notes')
+	expect(response).toHaveRedirect('/users/kody?tab=notes')
+})
+
+test('does not redirect to other origins', async () => {
+	expect(await submitTheme('https://attacker.example')).toHaveRedirect('/')
+	expect(await submitTheme('//attacker.example')).toHaveRedirect('/')
+})

app/routes/resources/theme-switch.tsx

@@ -2,6 +2,7 @@ import { useForm, getFormProps } from '@conform-to/react'
 import { parseWithZod } from '@conform-to/zod'
 import { invariantResponse } from '@epic-web/invariant'
 import { data, redirect, useFetcher, useFetchers } from 'react-router'
+import { safeRedirect } from 'remix-utils/safe-redirect'
 import { ServerOnly } from 'remix-utils/server-only'
 import { z } from 'zod'
 import { Icon } from '#app/components/ui/icon.tsx'
@@ -32,7 +33,7 @@ export async function action({ request }: Route.ActionArgs) {
 		headers: { 'set-cookie': setTheme(theme) },
 	}
 	if (redirectTo) {
-		return redirect(redirectTo, responseInit)
+		return redirect(safeRedirect(redirectTo), responseInit)
 	} else {
 		return data({ result: submission.reply() }, responseInit)
 	}

app/utils/auth.server.ts

@@ -108,6 +108,8 @@ export async function resetUserPassword({
 					hash: hashedPassword,
 				},
 			},
+			// a password reset should sign out anyone using the old password
+			sessions: { deleteMany: {} },
 		},
 	})
 }

app/utils/env.server.ts

@@ -19,6 +19,11 @@ const schema = z.object({
 	GITHUB_TOKEN: z.string().optional(),
 
 	ALLOW_INDEXING: z.enum(['true', 'false']).optional(),
+	// Comma-separated hostnames the app is served from (like
+	// "example.com,www.example.com"). The first is the canonical host used for
+	// links in emails when a request's Host header isn't one of these.
+	// `${FLY_APP_NAME}.fly.dev` and localhost are always allowed.
+	ALLOWED_HOSTS: z.string().optional(),
 
 	// Tigris Object Storage Configuration
 	AWS_ACCESS_KEY_ID: z.string(),

app/utils/misc.tsx

@@ -61,12 +61,46 @@ export function cn(...inputs: ClassValue[]) {
 	return twMerge(clsx(inputs))
 }
 
+/**
+ * Hosts we trust to appear in the Host/X-Forwarded-Host headers. The first one
+ * is the canonical host used when the request's host is not trusted.
+ */
+function getAllowedHosts() {
+	const hosts = (process.env.ALLOWED_HOSTS ?? '')
+		.split(',')
+		.map((host) => host.trim().toLowerCase())
+		.filter(Boolean)
+	if (process.env.FLY_APP_NAME) {
+		hosts.push(`${process.env.FLY_APP_NAME}.fly.dev`.toLowerCase())
+	}
+	return hosts
+}
+
+function isLocalhost(host: string) {
+	const hostname = host.replace(/:\d+$/, '')
+	return ['localhost', '127.0.0.1', '[::1]'].includes(hostname)
+}
+
+/**
+ * Returns the origin of the app (like https://example.com). Host headers are
+ * client-controlled, so the request's host is only used if it is in
+ * ALLOWED_HOSTS, is the Fly.io app hostname, or is localhost. Otherwise we fall
+ * back to the canonical host. This is important because we use this to
+ * generate links we email to users (like password reset links).
+ */
 export function getDomainUrl(request: Request) {
-	const host =
+	const requestHost = (
 		request.headers.get('X-Forwarded-Host') ??
 		request.headers.get('host') ??
 		new URL(request.url).host
-	const protocol = request.headers.get('X-Forwarded-Proto') ?? 'http'
+	).toLowerCase()
+	const allowedHosts = getAllowedHosts()
+	const host =
+		allowedHosts.includes(requestHost) || isLocalhost(requestHost)
+			? requestHost
+			: (allowedHosts[0] ?? `localhost:${process.env.PORT || 3000}`)
+	const protocol =
+		request.headers.get('X-Forwarded-Proto') === 'https' ? 'https' : 'http'
 	return `${protocol}://${host}`
 }