Add e2e tests for me.* procedures and fix code review issues

- Add comprehensive e2e tests for me.get, me.authStatus, me.setupProfile,
  me.updateProfile, me.setPassword, and me.delete (21 tests)
- Make createDb require explicit connection string (no default env lookup)
- Add database name validation to prevent SQL injection in CREATE DATABASE
- Fix getTestDatabaseUrl to throw instead of returning empty string
- Replace brittle relative path with findRepoRoot() function
- Extract magic numbers (SESSION_EXPIRY_MS, API_TOKEN_EXPIRY_MS, ONE_DAY_MS)
- Consolidate duplicate createAPIContext functions
- Add hasPassword field to meAuthStatus and toUserResponse

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
RevIQ
2026-01-10 15:24:42 +08:00
parent cc77211969
commit 2c2556a5ea
8 changed files with 743 additions and 48 deletions

View File

@@ -0,0 +1,667 @@
/**
* End-to-end tests for Me procedures (user profile and account management)
*
* These tests use a real PostgreSQL database to test:
* - me.get - get user profile
* - me.authStatus - get authentication status
* - me.setupProfile - initial profile setup
* - me.updateProfile - update profile fields
* - me.setPassword - set/change password
* - me.delete - delete account
*/
import type { Database } from "@reviq/db-schema";
import type { Kysely } from "kysely";
import type { APIContext } from "../../context.js";
import {
afterAll,
beforeAll,
beforeEach,
describe,
expect,
test,
} from "bun:test";
import { call } from "@orpc/server";
import { router } from "../../router.js";
import { hashPassword } from "../../utils/password.js";
import { hashToken } from "../../utils/crypto.js";
import { COOKIE_NAMES } from "../../utils/cookies.js";
import { TEST_RP } from "../helpers/test-constants.js";
import {
createTestDb,
createTestUser,
destroyTestDb,
runMigrations,
truncateAllTables,
} from "../helpers/test-db.js";
/** Session expiry duration: 24 hours in milliseconds */
const SESSION_EXPIRY_MS = 24 * 60 * 60 * 1000;
/** API token expiry duration: 1 year in milliseconds */
const API_TOKEN_EXPIRY_MS = 365 * 24 * 60 * 60 * 1000;
/** API token expiry duration in cascade test: 1 day in milliseconds */
const ONE_DAY_MS = 86400000;
let db: Kysely<Database> | undefined;
function getDb(): Kysely<Database> {
if (!db) {
throw new Error("Database not initialized");
}
return db;
}
/**
* Create an API context with optional authentication
*/
function createAPIContext(options?: {
sessionToken?: string;
apiKey?: string;
}): APIContext {
const reqHeaders = new Headers();
if (options?.sessionToken) {
reqHeaders.set(
"cookie",
`${COOKIE_NAMES.SESSION_TOKEN}=${options.sessionToken}`,
);
}
if (options?.apiKey) {
reqHeaders.set("x-api-key", options.apiKey);
}
return {
db: getDb(),
origin: TEST_RP.origin,
allowedOrigins: [...TEST_RP.allowedOrigins],
rpName: TEST_RP.rpName,
reqHeaders,
resHeaders: new Headers(),
};
}
/**
* Create a real session in the database and return the token
*/
async function createSession(userId: number): Promise<string> {
const token = "test-session-" + String(Date.now()) + String(Math.random());
const tokenHashValue = await hashToken(token);
const expiresAt = new Date(Date.now() + SESSION_EXPIRY_MS);
await getDb()
.insertInto("sessions")
.values({
user_id: userId,
token_hash: tokenHashValue,
ip_address: "127.0.0.1",
user_agent: "test-agent",
expires_at: expiresAt,
trusted_mode: false,
})
.execute();
return token;
}
/**
* Create an API token in the database and return the token
*/
async function createApiToken(
userId: number,
): Promise<{ token: string; name: string }> {
const token =
"test-api-token-" + String(Date.now()) + String(Math.random());
const tokenHashValue = await hashToken(token);
const expiresAt = new Date(Date.now() + API_TOKEN_EXPIRY_MS);
await getDb()
.insertInto("api_tokens")
.values({
user_id: userId,
token_hash: tokenHashValue,
name: "Test API Token",
expires_at: expiresAt,
})
.execute();
return { token, name: "Test API Token" };
}
beforeAll(async () => {
await runMigrations();
db = createTestDb();
});
afterAll(async () => {
if (db) {
await destroyTestDb(db);
}
});
beforeEach(async () => {
await truncateAllTables(getDb());
});
describe("me.get", () => {
test("returns user profile with all fields", async () => {
const user = await createTestUser(getDb(), {
email: "test@example.com",
displayName: "Test User",
fullName: "Test Full Name",
emailVerifiedAt: new Date(),
});
// Update with phone number
await getDb()
.updateTable("users")
.set({ phone_number: "+1234567890" })
.where("id", "=", user.id)
.execute();
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
const result = await call(router.me.get, undefined, { context });
expect(result.id).toBe(user.id);
expect(result.email).toBe("test@example.com");
expect(result.displayName).toBe("Test User");
expect(result.fullName).toBe("Test Full Name");
expect(result.phoneNumber).toBe("+1234567890");
expect(result.emailVerified).toBe(true);
expect(result.needsSetup).toBe(false);
expect(result.isSuperuser).toBe(false);
expect(result.hasPassword).toBe(false);
});
test("returns needsSetup=true when displayName is null", async () => {
const user = await createTestUser(getDb(), {
email: "newuser@example.com",
displayName: undefined,
});
// Set display_name to null explicitly
await getDb()
.updateTable("users")
.set({ display_name: null })
.where("id", "=", user.id)
.execute();
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
const result = await call(router.me.get, undefined, { context });
expect(result.needsSetup).toBe(true);
expect(result.displayName).toBeNull();
});
test("returns hasPassword=true when user has password", async () => {
const passwordHash = await hashPassword("securePassword123!");
const user = await createTestUser(getDb(), {
email: "withpassword@example.com",
passwordHash,
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
const result = await call(router.me.get, undefined, { context });
expect(result.hasPassword).toBe(true);
});
test("returns isSuperuser=true for superuser", async () => {
const user = await createTestUser(getDb(), {
email: "admin@example.com",
isSuperuser: true,
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
const result = await call(router.me.get, undefined, { context });
expect(result.isSuperuser).toBe(true);
});
});
describe("me.authStatus", () => {
test("returns session auth info", async () => {
const user = await createTestUser(getDb(), {
email: "session@example.com",
displayName: "Session User",
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
const result = await call(router.me.authStatus, undefined, { context });
expect(result.user.email).toBe("session@example.com");
expect(result.user.displayName).toBe("Session User");
expect(result.auth.method).toBe("session");
if (result.auth.method === "session") {
expect(result.auth.expiresAt).toBeInstanceOf(Date);
}
});
test("returns api_token auth info", async () => {
const user = await createTestUser(getDb(), {
email: "apitoken@example.com",
});
const { token } = await createApiToken(user.id);
const context = createAPIContext({ apiKey: token });
const result = await call(router.me.authStatus, undefined, { context });
expect(result.user.email).toBe("apitoken@example.com");
expect(result.auth.method).toBe("api_token");
if (result.auth.method === "api_token") {
expect(result.auth.tokenName).toBe("Test API Token");
expect(result.auth.expiresAt).toBeInstanceOf(Date);
}
});
});
describe("me.setupProfile", () => {
test("sets up profile with required fields", async () => {
const user = await createTestUser(getDb(), {
email: "setup@example.com",
displayName: undefined,
});
// Clear display_name
await getDb()
.updateTable("users")
.set({ display_name: null })
.where("id", "=", user.id)
.execute();
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
await call(
router.me.setupProfile,
{
displayName: "New Display Name",
fullName: "John Doe",
phoneNumber: "+12025551234",
},
{ context },
);
// Verify changes
const updated = await getDb()
.selectFrom("users")
.select(["display_name", "full_name", "phone_number"])
.where("id", "=", user.id)
.executeTakeFirstOrThrow();
expect(updated.display_name).toBe("New Display Name");
expect(updated.full_name).toBe("John Doe");
expect(updated.phone_number).toBe("+12025551234");
});
test("sets up profile with only required displayName", async () => {
const user = await createTestUser(getDb(), {
email: "minimal@example.com",
});
await getDb()
.updateTable("users")
.set({ display_name: null })
.where("id", "=", user.id)
.execute();
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
await call(
router.me.setupProfile,
{
displayName: "Minimal User",
},
{ context },
);
const updated = await getDb()
.selectFrom("users")
.select(["display_name", "full_name", "phone_number"])
.where("id", "=", user.id)
.executeTakeFirstOrThrow();
expect(updated.display_name).toBe("Minimal User");
expect(updated.full_name).toBeNull();
expect(updated.phone_number).toBeNull();
});
});
describe("me.updateProfile", () => {
test("updates displayName only", async () => {
const user = await createTestUser(getDb(), {
email: "update@example.com",
displayName: "Original Name",
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
await call(
router.me.updateProfile,
{
displayName: "Updated Name",
},
{ context },
);
const updated = await getDb()
.selectFrom("users")
.select(["display_name"])
.where("id", "=", user.id)
.executeTakeFirstOrThrow();
expect(updated.display_name).toBe("Updated Name");
});
test("updates multiple fields at once", async () => {
const user = await createTestUser(getDb(), {
email: "multi@example.com",
displayName: "Original",
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
await call(
router.me.updateProfile,
{
displayName: "New Display",
fullName: "Full Name Here",
phoneNumber: "+12025551234",
},
{ context },
);
const updated = await getDb()
.selectFrom("users")
.select(["display_name", "full_name", "phone_number"])
.where("id", "=", user.id)
.executeTakeFirstOrThrow();
expect(updated.display_name).toBe("New Display");
expect(updated.full_name).toBe("Full Name Here");
expect(updated.phone_number).toBe("+12025551234");
});
test("empty strings in optional fields are treated as no-op", async () => {
// Empty strings in optionalString fields are transformed to undefined,
// which means no update happens - fields keep their existing values
const user = await createTestUser(getDb(), {
email: "clear@example.com",
displayName: "Keep Me",
fullName: "Keep This Too",
});
await getDb()
.updateTable("users")
.set({ phone_number: "+12025551234" })
.where("id", "=", user.id)
.execute();
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
await call(
router.me.updateProfile,
{
fullName: "",
phoneNumber: "",
},
{ context },
);
const updated = await getDb()
.selectFrom("users")
.select(["display_name", "full_name", "phone_number"])
.where("id", "=", user.id)
.executeTakeFirstOrThrow();
// Empty strings are transformed to undefined by optionalString,
// so no update happens - fields keep their existing values
expect(updated.display_name).toBe("Keep Me");
expect(updated.full_name).toBe("Keep This Too");
expect(updated.phone_number).toBe("+12025551234");
});
test("does nothing when no fields provided", async () => {
const user = await createTestUser(getDb(), {
email: "noop@example.com",
displayName: "Stay Same",
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
await call(router.me.updateProfile, {}, { context });
const updated = await getDb()
.selectFrom("users")
.select(["display_name"])
.where("id", "=", user.id)
.executeTakeFirstOrThrow();
expect(updated.display_name).toBe("Stay Same");
});
});
describe("me.setPassword", () => {
test("sets password for user without password", async () => {
const user = await createTestUser(getDb(), {
email: "nopass@example.com",
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
// Use a strong password
await call(
router.me.setPassword,
{
newPassword: "SuperSecure123!@#$%",
},
{ context },
);
const updated = await getDb()
.selectFrom("users")
.select(["password_hash"])
.where("id", "=", user.id)
.executeTakeFirstOrThrow();
expect(updated.password_hash).not.toBeNull();
});
test("changes password with correct current password", async () => {
const oldPassword = "OldPassword123!@#";
const oldHash = await hashPassword(oldPassword);
const user = await createTestUser(getDb(), {
email: "changepass@example.com",
passwordHash: oldHash,
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
await call(
router.me.setPassword,
{
currentPassword: oldPassword,
newPassword: "NewSecurePassword456!@#",
},
{ context },
);
const updated = await getDb()
.selectFrom("users")
.select(["password_hash"])
.where("id", "=", user.id)
.executeTakeFirstOrThrow();
expect(updated.password_hash).not.toBe(oldHash);
});
test("fails without current password when user has password", async () => {
const oldHash = await hashPassword("ExistingPass123!");
const user = await createTestUser(getDb(), {
email: "haspass@example.com",
passwordHash: oldHash,
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
await expect(
call(
router.me.setPassword,
{
newPassword: "NewPassword123!@#",
},
{ context },
),
).rejects.toThrow("Current password required");
});
test("fails with incorrect current password", async () => {
const oldHash = await hashPassword("CorrectPassword123!");
const user = await createTestUser(getDb(), {
email: "wrongpass@example.com",
passwordHash: oldHash,
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
await expect(
call(
router.me.setPassword,
{
currentPassword: "WrongPassword123!",
newPassword: "NewPassword456!@#",
},
{ context },
),
).rejects.toThrow("Current password is incorrect");
});
test("fails with weak password", async () => {
const user = await createTestUser(getDb(), {
email: "weak@example.com",
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
// Password must be at least 8 chars to pass schema validation
// "password" passes length check but fails zxcvbn strength check
// zxcvbn provides feedback like "This is a top-10 common password"
await expect(
call(
router.me.setPassword,
{
newPassword: "password", // 8 chars but extremely common
},
{ context },
),
).rejects.toThrow(/common|top|weak|guess/i);
});
});
describe("me.delete", () => {
test("deletes account with correct password", async () => {
const password = "DeleteMe123!@#";
const passwordHash = await hashPassword(password);
const user = await createTestUser(getDb(), {
email: "delete@example.com",
passwordHash,
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
await call(router.me.delete, { password }, { context });
// Verify user is deleted
const deleted = await getDb()
.selectFrom("users")
.where("id", "=", user.id)
.selectAll()
.executeTakeFirst();
expect(deleted).toBeUndefined();
});
test("fails without password set", async () => {
const user = await createTestUser(getDb(), {
email: "nopassdelete@example.com",
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
await expect(
call(router.me.delete, { password: "anything" }, { context }),
).rejects.toThrow("Cannot delete account without a password");
});
test("fails with incorrect password", async () => {
const passwordHash = await hashPassword("CorrectPassword123!");
const user = await createTestUser(getDb(), {
email: "wrongdelete@example.com",
passwordHash,
});
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
await expect(
call(router.me.delete, { password: "WrongPassword123!" }, { context }),
).rejects.toThrow("Incorrect password");
});
test("cascades deletion to related records", async () => {
const password = "CascadeDelete123!@#";
const passwordHash = await hashPassword(password);
const user = await createTestUser(getDb(), {
email: "cascade@example.com",
passwordHash,
});
// Create related records
await getDb()
.insertInto("api_tokens")
.values({
user_id: user.id,
token_hash: "test-hash",
name: "Test Token",
expires_at: new Date(Date.now() + ONE_DAY_MS),
})
.execute();
const sessionToken = await createSession(user.id);
const context = createAPIContext({ sessionToken });
await call(router.me.delete, { password }, { context });
// Verify cascaded deletion
const tokens = await getDb()
.selectFrom("api_tokens")
.where("user_id", "=", user.id)
.selectAll()
.execute();
expect(tokens).toHaveLength(0);
});
});

View File

@@ -4,10 +4,12 @@
import type { Database } from "@reviq/db-schema";
import { join } from "node:path";
import { Kysely, PostgresDialect, sql } from "kysely";
import { createDb } from "@reviq/db";
import type { Kysely } from "kysely";
import { sql } from "kysely";
import pg from "pg";
const { Pool, Client } = pg;
const { Client } = pg;
/** Tables to truncate between tests (in order that respects foreign keys) */
const TABLES_TO_TRUNCATE = [
@@ -28,41 +30,28 @@ const TABLES_TO_TRUNCATE = [
/**
* Creates a test database connection.
* Uses TEST_DATABASE_URL env var, falls back to DATABASE_URL with _test suffix.
* Requires TEST_DATABASE_URL env var to be set.
*
* @throws Error if TEST_DATABASE_URL is not set
*/
export function createTestDb(): Kysely<Database> {
const connectionString = getTestDatabaseUrl();
if (!connectionString) {
throw new Error(
"Test database URL not configured. Set TEST_DATABASE_URL environment variable.",
);
}
const dialect = new PostgresDialect({
pool: new Pool({ connectionString }),
});
return new Kysely<Database>({ dialect });
return createDb(getTestDatabaseUrl());
}
/**
* Gets the test database URL from environment.
* Requires TEST_DATABASE_URL to be set.
* Adds sslmode=disable for local development.
*
* @throws Error if TEST_DATABASE_URL is not set
*/
export function getTestDatabaseUrl(): string {
let url: string;
const url = Bun.env.TEST_DATABASE_URL;
// Prefer explicit TEST_DATABASE_URL
if (Bun.env.TEST_DATABASE_URL) {
url = Bun.env.TEST_DATABASE_URL;
} else if (Bun.env.DATABASE_URL) {
// Fall back to DATABASE_URL with _test suffix
const parsed = new URL(Bun.env.DATABASE_URL);
parsed.pathname = `${parsed.pathname}_test`;
url = parsed.toString();
} else {
return "";
if (!url) {
throw new Error(
"Test database URL not configured. Set TEST_DATABASE_URL environment variable.",
);
}
// Add sslmode=disable for local postgres
@@ -93,17 +82,23 @@ function parsePostgresUrl(url: string): {
};
}
/** Valid database name pattern: alphanumeric, underscores, hyphens only */
const VALID_DB_NAME_PATTERN = /^[a-zA-Z0-9_-]+$/;
/**
* Creates the test database if it doesn't exist.
*/
async function ensureTestDatabaseExists(): Promise<void> {
const testDbUrl = getTestDatabaseUrl();
if (!testDbUrl) {
throw new Error("Test database URL not configured");
}
const { host, port, user, password, database } = parsePostgresUrl(testDbUrl);
// Validate database name to prevent SQL injection
if (!VALID_DB_NAME_PATTERN.test(database)) {
throw new Error(
`Invalid database name: "${database}". Only alphanumeric characters, underscores, and hyphens are allowed.`,
);
}
// Connect to 'postgres' database to create the test database
const client = new Client({
host,
@@ -124,7 +119,7 @@ async function ensureTestDatabaseExists(): Promise<void> {
if (result.rows.length === 0) {
// Create the database
// Note: database names can't be parameterized, but we control this value
// Database name is validated above, safe to use in query
await client.query(`CREATE DATABASE "${database}"`);
console.log(`Created test database: ${database}`);
}
@@ -133,6 +128,34 @@ async function ensureTestDatabaseExists(): Promise<void> {
}
}
/**
* Finds the repository root by looking for db/migrations directory.
* Walks up from the current directory until found.
*
* @throws Error if repo root cannot be found
*/
function findRepoRoot(): string {
const { existsSync } = require("node:fs");
let current = import.meta.dir;
// Walk up to 10 levels to find the repo root
for (let i = 0; i < 10; i++) {
const migrationsPath = join(current, "db", "migrations");
if (existsSync(migrationsPath)) {
return current;
}
const parent = join(current, "..");
if (parent === current) {
break; // Reached filesystem root
}
current = parent;
}
throw new Error(
"Could not find repository root (looking for db/migrations directory)",
);
}
/**
* Runs database migrations using dbmate CLI.
* Creates the database if it doesn't exist.
@@ -140,16 +163,11 @@ async function ensureTestDatabaseExists(): Promise<void> {
*/
export async function runMigrations(): Promise<void> {
const testDbUrl = getTestDatabaseUrl();
if (!testDbUrl) {
throw new Error("Test database URL not configured");
}
// Ensure the database exists first
await ensureTestDatabaseExists();
// Find the repo root (where db/migrations lives)
// From apps/api-server/src/__tests__/helpers/ -> repo root is 5 levels up
const repoRoot = join(import.meta.dir, "../../../../..");
const repoRoot = findRepoRoot();
const proc = Bun.spawn(["dbmate", "up"], {
env: { ...process.env, DATABASE_URL: testDbUrl },

View File

@@ -19,7 +19,11 @@ const logger = pino({
},
});
const db = createDb();
const databaseUrl = Bun.env.DATABASE_URL;
if (!databaseUrl) {
throw new Error("DATABASE_URL environment variable is required");
}
const db = createDb(databaseUrl);
const handler = new RPCHandler(router, {
plugins: [
new LoggingHandlerPlugin({

View File

@@ -25,6 +25,7 @@ export const toUserResponse = (user: Selectable<Users>) => ({
emailVerified: user.email_verified_at !== null,
needsSetup: user.display_name === null,
isSuperuser: user.is_superuser,
hasPassword: user.password_hash !== null,
});
/** Transform site record to API response format */

View File

@@ -201,6 +201,7 @@ const meAuthStatus = os.me.authStatus
"avatar_url",
"email_verified_at",
"is_superuser",
"password_hash",
])
.where("id", "=", context.user.id)
.executeTakeFirstOrThrow();
@@ -216,6 +217,7 @@ const meAuthStatus = os.me.authStatus
emailVerified: user.email_verified_at !== null,
needsSetup: user.display_name === null,
isSuperuser: user.is_superuser,
hasPassword: user.password_hash !== null,
},
auth: context.auth,
};

View File

@@ -15,7 +15,13 @@ async function bootstrap(
console.log("RevIQ Bootstrap - Create Superuser");
console.log("===================================\n");
const db = createDb();
const databaseUrl = Bun.env.DATABASE_URL;
if (!databaseUrl) {
console.error("Error: DATABASE_URL environment variable is required");
this.process.exit(1);
}
const db = createDb(databaseUrl);
try {
// Execute the bootstrap operation