Auth Fix
CI / ci (push) Canceled after 0s

Fixed an issue where the user would be redirected to a localhost address because an auth url was not set in the env.
This commit is contained in:
Stefan Lazic committed 2026-09-06 21:01:54 +01:00
1 parent e9e28a243f
commit cbeccc6c1f
8 files changed
+420 -71

No files matched your search

+5
View File
@@ -38,6 +38,11 @@ DATABASE_PASSWORD="replace-me"
AUTH_SECRET="replace-me-with-a-32-byte-random-string"
# Auth.js needs to trust the host it's served from when not on localhost:3000.
AUTH_TRUST_HOST="true"
# Explicit, not just inferred from AUTH_TRUST_HOST + request headers —
# header-based detection (Host/X-Forwarded-Host) behaves inconsistently
# across reverse-proxy setups, and a wrong guess sends every post-sign-in
# redirect to the wrong origin. Must match DOMAIN above exactly.
AUTH_URL="https://example-business.example.com"
NEXT_PUBLIC_APP_URL="https://example-business.example.com"
# One-time setup token the installation wizard (/install) requires before
# it will create the first business/administrator — generate with:
@@ -35,3 +35,17 @@ describe("generateBusinessEnv — INSTALLATION_TOKEN", () => {
expect(envB).not.toContain("token-a");
});
});
describe("generateBusinessEnv — AUTH_URL", () => {
it("sets AUTH_URL explicitly to the business's own https domain — not left to header-based detection", () => {
const env = generateBusinessEnv(baseValues);
expect(env).toContain('AUTH_URL="https://acme.example.com"');
});
it("AUTH_URL always matches DOMAIN/NEXT_PUBLIC_APP_URL for a given business", () => {
const env = generateBusinessEnv({ ...baseValues, domain: "other-business.example.com" });
expect(env).toContain('DOMAIN="other-business.example.com"');
expect(env).toContain('AUTH_URL="https://other-business.example.com"');
expect(env).toContain('NEXT_PUBLIC_APP_URL="https://other-business.example.com"');
});
});
@@ -0,0 +1,102 @@
import { mkdtemp, mkdir, readFile, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import path from "node:path";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
const composeUpMock = vi.fn().mockResolvedValue(undefined);
const reloadCaddyMock = vi.fn().mockResolvedValue(undefined);
vi.mock("@/lib/docker", () => ({
composeUp: (...args: unknown[]) => composeUpMock(...args),
composeDown: vi.fn(),
composeStatus: vi.fn().mockResolvedValue("stopped"),
reloadCaddy: (...args: unknown[]) => reloadCaddyMock(...args),
runMigrations: vi.fn(),
}));
/**
* Regression coverage for a real production bug: a business behind a
* reverse proxy redirected every post-sign-in navigation to
* http://localhost:3000 instead of its own domain, because AUTH_URL was
* never set and Auth.js's header-based host detection didn't behave as
* expected in that deployment. The fix makes AUTH_URL explicit at
* provisioning time (see compose-template.test.ts) — this file covers
* the other place a business's domain gets set: editing it later, where
* AUTH_URL must keep tracking DOMAIN exactly like NEXT_PUBLIC_APP_URL
* already did, or a business that changes domains would silently regress
* back into the same bug.
*/
describe("updateBusiness — AUTH_URL tracks domain changes", () => {
let stacksDir: string;
const originalStacksDir = process.env.STACKS_DIR;
const originalMysqlHost = process.env.MYSQL_HOST;
beforeEach(async () => {
stacksDir = await mkdtemp(path.join(tmpdir(), "nc-manager-updatebiz-test-"));
process.env.STACKS_DIR = stacksDir;
// updateBusiness's own domain-availability check just scans other
// stacks' .env files via listBusinessStacks() — no real MySQL needed
// for this test since we never change the domain to something
// already in use.
composeUpMock.mockClear();
reloadCaddyMock.mockClear();
});
afterEach(async () => {
if (originalStacksDir === undefined) delete process.env.STACKS_DIR;
else process.env.STACKS_DIR = originalStacksDir;
if (originalMysqlHost === undefined) delete process.env.MYSQL_HOST;
else process.env.MYSQL_HOST = originalMysqlHost;
await rm(stacksDir, { recursive: true, force: true });
});
async function createStack(slug: string, envContent: string) {
const dir = path.join(stacksDir, slug);
await mkdir(dir, { recursive: true });
await writeFile(path.join(dir, "compose.yaml"), "name: test\n", "utf8");
await writeFile(path.join(dir, ".env"), envContent, "utf8");
return dir;
}
it("updates AUTH_URL to the new domain, alongside DOMAIN and NEXT_PUBLIC_APP_URL", async () => {
const { updateBusiness } = await import("../provisioning");
await createStack(
"acme",
[
'BUSINESS_NAME="Acme"',
'DOMAIN="old-domain.example.com"',
'AUTH_URL="https://old-domain.example.com"',
'NEXT_PUBLIC_APP_URL="https://old-domain.example.com"',
'AUTH_SECRET="unrelated-secret"',
].join("\n") + "\n",
);
const result = await updateBusiness({ slug: "acme", name: "Acme", domain: "new-domain.example.com" });
expect(result.restarted).toBe(true);
const envAfter = await readFile(path.join(stacksDir, "acme", ".env"), "utf8");
expect(envAfter).toContain('DOMAIN="new-domain.example.com"');
expect(envAfter).toContain('AUTH_URL="https://new-domain.example.com"');
expect(envAfter).toContain('NEXT_PUBLIC_APP_URL="https://new-domain.example.com"');
expect(envAfter).not.toContain("old-domain.example.com");
// Unrelated secrets untouched.
expect(envAfter).toContain('AUTH_SECRET="unrelated-secret"');
expect(composeUpMock).toHaveBeenCalledWith(path.join(stacksDir, "acme"), { forceRecreate: true });
});
it("leaves AUTH_URL alone (no restart) when only the business name changes, not the domain", async () => {
const { updateBusiness } = await import("../provisioning");
await createStack(
"acme",
['BUSINESS_NAME="Acme"', 'DOMAIN="acme.example.com"', 'AUTH_URL="https://acme.example.com"'].join("\n") + "\n",
);
const result = await updateBusiness({ slug: "acme", name: "Acme Barbers Ltd", domain: "acme.example.com" });
expect(result.restarted).toBe(false);
expect(composeUpMock).not.toHaveBeenCalled();
const envAfter = await readFile(path.join(stacksDir, "acme", ".env"), "utf8");
expect(envAfter).toContain('AUTH_URL="https://acme.example.com"');
expect(envAfter).toContain('BUSINESS_NAME="Acme Barbers Ltd"');
});
});
+8
View File
@@ -134,6 +134,14 @@ DATABASE_USER="${values.databaseUser}"
DATABASE_PASSWORD="${values.databasePassword}"
AUTH_SECRET="${values.authSecret}"
AUTH_TRUST_HOST="true"
# Explicit, not just inferred from AUTH_TRUST_HOST + request headers —
# header-based detection (Host/X-Forwarded-Host) is exactly the kind of
# thing that behaves inconsistently across reverse-proxy setups, and a
# wrong guess here sends every post-sign-in/post-install redirect to the
# wrong origin (seen for real: a business behind Caddy redirecting to
# http://localhost:3000 instead of its own domain). Auth.js's own docs
# recommend setting this explicitly for exactly this reason.
AUTH_URL="https://${values.domain}"
NEXT_PUBLIC_APP_URL="https://${values.domain}"
INSTALLATION_TOKEN="${values.installationToken}"
CRON_SECRET="${values.cronSecret}"
+6 -4
View File
@@ -254,14 +254,16 @@ export async function updateBusiness(input: UpdateBusinessInput): Promise<Update
const currentDomain = readEnvValue(currentEnv, "DOMAIN");
const domainChanged = currentDomain !== domain;
// Only these four keys are ever touched — everything else in the file
// Only these five keys are ever touched — everything else in the file
// (secrets, integrations, anything hand-edited) is left byte-for-byte
// alone. NEXT_PUBLIC_APP_URL/GOOGLE_CALENDAR_REDIRECT_URI are derived
// from the domain and must track it exactly, or the app would serve
// absolute URLs and OAuth callbacks pointed at the old domain.
// alone. AUTH_URL/NEXT_PUBLIC_APP_URL/GOOGLE_CALENDAR_REDIRECT_URI are
// derived from the domain and must track it exactly, or the app would
// serve absolute URLs, OAuth callbacks, and post-sign-in redirects
// pointed at the old domain.
await upsertEnvValues(envPath, {
BUSINESS_NAME: name,
DOMAIN: domain,
AUTH_URL: `https://${domain}`,
NEXT_PUBLIC_APP_URL: `https://${domain}`,
GOOGLE_CALENDAR_REDIRECT_URI: `https://${domain}/api/staff/calendar/google/callback`,
});
+10 -67
View File
@@ -3,44 +3,10 @@ import NextAuth from "next-auth";
import Credentials from "next-auth/providers/credentials";
import Google from "next-auth/providers/google";
import { authConfig } from "@/auth.config";
import { finalizeOtp, verifyOtp } from "@/lib/auth/otp.service";
import { authorizeCredentials } from "@/lib/auth/authorize-credentials";
import { prisma } from "@/lib/db/prisma";
import { checkRateLimit } from "@/lib/security/rate-limit";
import { secretsMatch } from "@/lib/security/secrets-match";
/**
* Two credential shapes, both ending in the same finalizeOtp() call and
* the same `{id, email, name, image}` return shape — see
* src/lib/auth/otp.service.ts's own header for the full verify/finalize
* design this builds on.
*
* `{ email, code }` — the normal path, used by sign-in, registration, and
* (since the "administrator invite verifies via OTP too" work) invitation
* acceptance as well: verifyOtp then finalizeOtp back to back in this one
* request (functionally "verify and consume in one shot" from the
* caller's point of view).
*
* `{ email, finalize: "true", token }` — used only by the installer's own
* immediate post-completion sign-in, where the administrator's code was
* already consumed by installApplication's own earlier verifyOtp call
* (inside its transaction) and can't be re-entered (one-time use). `token`
* must match the live INSTALLATION_TOKEN — required because
* /api/auth/callback/credentials is a public route regardless of this
* change (true of any Credentials provider), so this path must
* independently prove the caller already holds a secret an attacker can't
* have, not just trust that it's being called "from the right place".
* Without this, a race window (someone else's verifiedAt-but-not-yet-
* consumedAt row, mid-wizard) would be forgeable via a direct POST to that
* route with just a guessed email. (Invitation acceptance no longer uses
* this path — it has its own real OTP code to re-enter here instead, so
* there's nothing left to trust a bare token for there.)
*/
function verifyFinalizeToken(token: string): boolean {
if (!token) return false;
const installToken = process.env.INSTALLATION_TOKEN;
return !!installToken && secretsMatch(token, installToken);
}
export { authorizeCredentials } from "@/lib/auth/authorize-credentials";
export const { handlers, auth, signIn, signOut } = NextAuth({
...authConfig,
@@ -58,40 +24,17 @@ export const { handlers, auth, signIn, signOut } = NextAuth({
email: { label: "Email", type: "email" },
code: { label: "Code", type: "text" },
// Only ever set by the installer's own trusted server-side sign-in
// call — never rendered on any real form. See this file's own
// header for the full finalize/token design.
// call — never rendered on any real form. See
// src/lib/auth/authorize-credentials.ts's own header for the full
// finalize/token design.
finalize: { label: "Finalize", type: "text" },
token: { label: "Token", type: "text" },
},
authorize: async (raw) => {
const email = typeof raw?.email === "string" ? raw.email.trim().toLowerCase() : "";
if (!email) return null;
// Keyed by the attempted email, not IP — this is what actually
// stops credential stuffing against one account regardless of how
// many source addresses it's spread across. 30 attempts per 15
// minutes is well above anything a real forgetful user would ever
// hit, while still making online guessing impractical.
const { allowed } = checkRateLimit(`signin:${email}`, { maxAttempts: 30, windowMs: 15 * 60 * 1000 });
if (!allowed) return null;
if (raw?.finalize === "true") {
const token = typeof raw?.token === "string" ? raw.token : "";
if (!verifyFinalizeToken(token)) return null;
if (!(await finalizeOtp(prisma, email))) return null;
} else {
const code = typeof raw?.code === "string" ? raw.code : "";
if (!code) return null;
const verified = await verifyOtp(prisma, email, code);
if (!verified.ok) return null;
if (!(await finalizeOtp(prisma, email))) return null;
}
const user = await prisma.user.findUnique({ where: { email } });
if (!user) return null;
return { id: user.id, email: user.email, name: user.name, image: user.image };
},
// Kept in its own module (not inlined here) so it's importable from
// a plain Vitest test without dragging in this file's NextAuth(...)
// construction, which needs the Next.js runtime — see
// authorize-credentials.ts's own header.
authorize: authorizeCredentials,
}),
],
callbacks: {
@@ -0,0 +1,194 @@
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { prisma } from "@/services/scheduling/__tests__/fixtures";
import { authorizeCredentials } from "@/lib/auth/authorize-credentials";
import { requestOtp, verifyOtp } from "@/lib/auth/otp.service";
import { MockEmailProvider } from "@/services/notifications/providers/mock-email-provider";
import { installApplication } from "../installer.service";
import type { InstallInput } from "../schema";
/**
* Regression coverage for a real production bug: installApplication's own
* transaction consumes the administrator's OTP row via finalizeOtp (see
* installer.service.ts) — but src/auth.ts's Credentials provider used to
* call finalizeOtp a *second* time on its `{finalize:"true"}` sign-in
* path, which always fails (finalizeOtp is one-time-use) and turned every
* successful install into a CredentialsSignin error on the very last
* step. installer.service.integration.test.ts and
* otp.service.integration.test.ts each separately proved their own half
* correctly; neither exercised the interaction between them, which is
* exactly where this broke. This file exercises that interaction, end to
* end, against a real MySQL database.
*/
const INSTALLATION_TOKEN = "auth-finalize-test-install-token";
function buildInput(overrides: Partial<InstallInput> = {}): InstallInput {
return {
installationToken: INSTALLATION_TOKEN,
administrator: {
firstName: "Test",
lastName: "Owner",
email: `auth-finalize-test-${Date.now()}-${Math.random().toString(36).slice(2, 8)}@example.com`,
phone: "",
},
business: { name: "Auth Finalize Test Barbershop", shortName: "", description: "", email: "", phone: "", website: "" },
location: {
addressLine1: "1 Test Street",
addressLine2: "",
city: "Testville",
region: "",
postalCode: "T1 1TT",
country: "GB",
timezone: "Europe/London",
currency: "GBP",
locale: "en-GB",
},
hours: (["MONDAY", "TUESDAY", "WEDNESDAY", "THURSDAY", "FRIDAY", "SATURDAY", "SUNDAY"] as const).map((dayOfWeek) => ({
dayOfWeek,
isClosed: dayOfWeek === "SUNDAY",
opensAtMinutes: dayOfWeek === "SUNDAY" ? undefined : 540,
closesAtMinutes: dayOfWeek === "SUNDAY" ? undefined : 1080,
})),
bookingRules: {
allowGuestBooking: false,
minNoticeMinutes: 120,
maxAdvanceDays: 60,
cancellationWindowHours: 24,
rescheduleWindowHours: 24,
slotIntervalMinutes: 30,
bufferBeforeMinutes: 0,
bufferAfterMinutes: 5,
maxAppointmentsPerDay: null,
maxAppointmentsPerStaffPerDay: null,
customerCanChooseStaff: true,
requireApproval: false,
},
branding: { logoUrl: "", coverImageUrl: "", themeColor: "", backgroundColor: "" },
starterServices: [
{ categoryName: "Haircuts", name: "Classic Cut", description: "", price: "20.00", durationMinutes: 30, active: true, allowOnlineBooking: true },
],
...overrides,
};
}
async function verifyAdministratorEmail(email: string): Promise<void> {
const mockProvider = new MockEmailProvider();
await requestOtp(prisma, { email, purpose: "install", emailProvider: mockProvider });
const sent = mockProvider.sentEmails[0];
const code = /verification code is (\d{6})/.exec(sent?.text ?? "")?.[1];
if (!code) throw new Error("Test setup failed: couldn't extract the OTP code from the captured email.");
const result = await verifyOtp(prisma, email, code);
if (!result.ok) throw new Error(`Test setup failed: OTP verification failed (${result.reason}).`);
}
async function buildVerifiedInput(overrides: Partial<InstallInput> = {}): Promise<InstallInput> {
const input = buildInput(overrides);
await verifyAdministratorEmail(input.administrator.email);
return input;
}
async function resetInstallationSingleton(): Promise<void> {
await prisma.applicationInstallation.deleteMany({ where: { id: "singleton" } });
}
async function cleanupInstalledBusiness(businessId: string | undefined): Promise<void> {
if (!businessId) return;
const staff = await prisma.staffProfile.findMany({ where: { businessId }, select: { id: true, userId: true } });
const userIds = staff.map((s) => s.userId);
await prisma.staffService.deleteMany({ where: { staffId: { in: staff.map((s) => s.id) } } });
await prisma.staffProfile.deleteMany({ where: { businessId } });
await prisma.service.deleteMany({ where: { businessId } });
await prisma.serviceCategory.deleteMany({ where: { businessId } });
await prisma.rolePermission.deleteMany({ where: { role: { businessId } } });
await prisma.role.deleteMany({ where: { businessId } });
await prisma.auditLog.deleteMany({ where: { entityId: businessId } });
await prisma.loyaltyProgramme.deleteMany({ where: { businessId } });
await prisma.bookingRule.deleteMany({ where: { businessId } });
await prisma.businessSettings.deleteMany({ where: { businessId } });
await prisma.businessHours.deleteMany({ where: { businessId } });
await prisma.businessLocation.deleteMany({ where: { businessId } });
await prisma.business.deleteMany({ where: { id: businessId } });
await prisma.user.deleteMany({ where: { id: { in: userIds } } });
await resetInstallationSingleton();
}
describe("authorizeCredentials — post-install finalize sign-in (integration)", () => {
let createdBusinessId: string | undefined;
let originalToken: string | undefined;
beforeEach(async () => {
await resetInstallationSingleton();
createdBusinessId = undefined;
originalToken = process.env.INSTALLATION_TOKEN;
process.env.INSTALLATION_TOKEN = INSTALLATION_TOKEN;
});
afterEach(async () => {
await cleanupInstalledBusiness(createdBusinessId);
process.env.INSTALLATION_TOKEN = originalToken;
});
it("signs the administrator in immediately after a real installApplication() success — the exact sequence submitInstallationAction runs", async () => {
const input = await buildVerifiedInput();
const result = await installApplication(prisma, { input, installedVersion: "test" });
expect(result.success).toBe(true);
if (!result.success) return;
createdBusinessId = result.businessId;
// This is the reported bug: before the fix, this call always returned
// null (CredentialsSignin) because installApplication already
// consumed the OTP row and this path tried to finalize it again.
const user = await authorizeCredentials({
email: input.administrator.email,
finalize: "true",
token: INSTALLATION_TOKEN,
});
expect(user).not.toBeNull();
expect(user?.id).toBe(result.userId);
expect(user?.email).toBe(input.administrator.email);
});
it("rejects the finalize path with a wrong installation token, even right after a real install", async () => {
const input = await buildVerifiedInput();
const result = await installApplication(prisma, { input, installedVersion: "test" });
expect(result.success).toBe(true);
if (!result.success) return;
createdBusinessId = result.businessId;
const user = await authorizeCredentials({ email: input.administrator.email, finalize: "true", token: "wrong-token" });
expect(user).toBeNull();
});
it("rejects the finalize path entirely if no INSTALLATION_TOKEN is configured", async () => {
const input = await buildVerifiedInput();
const result = await installApplication(prisma, { input, installedVersion: "test" });
expect(result.success).toBe(true);
if (!result.success) return;
createdBusinessId = result.businessId;
delete process.env.INSTALLATION_TOKEN;
const user = await authorizeCredentials({ email: input.administrator.email, finalize: "true", token: INSTALLATION_TOKEN });
expect(user).toBeNull();
});
it("the ordinary {email, code} sign-in path is unaffected by this fix — still verifies and consumes its own fresh code", async () => {
const email = `auth-finalize-normal-${Date.now()}@example.com`;
const mockProvider = new MockEmailProvider();
await requestOtp(prisma, { email, purpose: "login", emailProvider: mockProvider });
const code = /verification code is (\d{6})/.exec(mockProvider.sentEmails[0]?.text ?? "")?.[1];
if (!code) throw new Error("Test setup failed: couldn't extract the OTP code.");
await prisma.user.upsert({
where: { email },
update: {},
create: { email, name: "Normal Login User" },
});
const user = await authorizeCredentials({ email, code });
expect(user).not.toBeNull();
expect(user?.email).toBe(email);
await prisma.user.deleteMany({ where: { email } });
});
});
+81
View File
@@ -0,0 +1,81 @@
import { finalizeOtp, verifyOtp } from "@/lib/auth/otp.service";
import { prisma } from "@/lib/db/prisma";
import { checkRateLimit } from "@/lib/security/rate-limit";
import { secretsMatch } from "@/lib/security/secrets-match";
/**
* Two credential shapes — see src/lib/auth/otp.service.ts's own header
* for the full verify/finalize design this builds on.
*
* `{ email, code }` — the normal path, used by sign-in, registration, and
* (since the "administrator invite verifies via OTP too" work) invitation
* acceptance as well: verifyOtp then finalizeOtp back to back in this one
* request (functionally "verify and consume in one shot" from the
* caller's point of view).
*
* `{ email, finalize: "true", token }` — used only by the installer's own
* immediate post-completion sign-in. Deliberately does NOT call
* finalizeOtp itself (unlike the branch above) — installApplication's own
* transaction already did, consuming the row as part of creating the
* administrator (see installer.service.ts). finalizeOtp is one-time-use
* (it sets consumedAt and refuses a second call on the same row), so
* calling it again here would always fail and turn every successful
* install into a CredentialsSignin error on this very last step — this
* was exactly that bug, caught because it reproduces on every install.
* `token` matching the live INSTALLATION_TOKEN is this branch's entire
* proof of legitimacy instead — required because
* /api/auth/callback/credentials is a public route regardless of this
* change (true of any Credentials provider), so this path must
* independently prove the caller already holds a secret an attacker
* can't have, not just trust that it's being called "from the right
* place". (Invitation acceptance doesn't use this path — it has its own
* real OTP code to re-enter here instead.)
*/
function verifyFinalizeToken(token: string): boolean {
if (!token) return false;
const installToken = process.env.INSTALLATION_TOKEN;
return !!installToken && secretsMatch(token, installToken);
}
/**
* The Credentials provider's `authorize` logic, pulled out of src/auth.ts
* into this module specifically so it's importable in a plain Vitest
* test without dragging in `next-auth`'s `NextAuth(...)` construction
* (which needs the Next.js runtime and fails to resolve under Vitest —
* see auth-finalize.integration.test.ts's own header for why this
* function's correctness matters enough to need that). src/auth.ts wires
* this in verbatim as its Credentials provider's `authorize` option.
*/
export async function authorizeCredentials(
raw: Partial<Record<"email" | "code" | "finalize" | "token", unknown>> | undefined,
): Promise<{ id: string; email: string | null; name: string | null; image: string | null } | null> {
const email = typeof raw?.email === "string" ? raw.email.trim().toLowerCase() : "";
if (!email) return null;
// Keyed by the attempted email, not IP — this is what actually
// stops credential stuffing against one account regardless of how
// many source addresses it's spread across. 30 attempts per 15
// minutes is well above anything a real forgetful user would ever
// hit, while still making online guessing impractical.
const { allowed } = checkRateLimit(`signin:${email}`, { maxAttempts: 30, windowMs: 15 * 60 * 1000 });
if (!allowed) return null;
if (raw?.finalize === "true") {
const token = typeof raw?.token === "string" ? raw.token : "";
if (!verifyFinalizeToken(token)) return null;
// No finalizeOtp call here — installApplication's own
// transaction already consumed it. See this file's own header.
} else {
const code = typeof raw?.code === "string" ? raw.code : "";
if (!code) return null;
const verified = await verifyOtp(prisma, email, code);
if (!verified.ok) return null;
if (!(await finalizeOtp(prisma, email))) return null;
}
const user = await prisma.user.findUnique({ where: { email } });
if (!user) return null;
return { id: user.id, email: user.email, name: user.name, image: user.image };
}