diff --git a/CloudronManifest.json b/CloudronManifest.json index ec2c29b..d9187ca 100644 --- a/CloudronManifest.json +++ b/CloudronManifest.json @@ -4,7 +4,7 @@ "title": "Community Rule", "author": "MEDLab", "description": "Community governance and rule-building app", - "version": "0.1.12", + "version": "0.1.13", "httpPort": 3000, "healthCheckPath": "/api/health", "memoryLimit": 805306368, diff --git a/app/api/auth/magic-link/request/route.ts b/app/api/auth/magic-link/request/route.ts index b644c82..5db8b76 100644 --- a/app/api/auth/magic-link/request/route.ts +++ b/app/api/auth/magic-link/request/route.ts @@ -24,7 +24,7 @@ import { getPublicOrigin } from "../../../../../lib/server/publicOrigin"; import { magicLinkRequestBodySchema } from "../../../../../lib/server/validation/createFlowSchemas"; import { jsonFromZodError } from "../../../../../lib/server/validation/zodHttp"; -const MAGIC_LINK_TTL_MS = 15 * 60 * 1000; +const MAGIC_LINK_TTL_MS = 60 * 60 * 1000; const EMAIL_MIN_INTERVAL_MS = 60 * 1000; const IP_MIN_INTERVAL_MS = 20 * 1000; const SCOPE = "auth.magicLink.request"; diff --git a/app/api/auth/magic-link/verify/route.ts b/app/api/auth/magic-link/verify/route.ts index bcfdfe5..d5948bf 100644 --- a/app/api/auth/magic-link/verify/route.ts +++ b/app/api/auth/magic-link/verify/route.ts @@ -54,7 +54,15 @@ export async function GET(request: NextRequest) { where: { tokenHash }, }); - if (!row || row.expiresAt < new Date()) { + if (!row) { + return redirectWithRequestId( + request, + "/login?error=invalid_link", + requestId, + ); + } + + if (row.expiresAt < new Date()) { return redirectWithRequestId( request, "/login?error=expired_link", diff --git a/app/api/user/email-change/request/route.ts b/app/api/user/email-change/request/route.ts index d632d38..44f3d43 100644 --- a/app/api/user/email-change/request/route.ts +++ b/app/api/user/email-change/request/route.ts @@ -25,7 +25,7 @@ import { readLimitedJson } from "../../../../../lib/server/validation/requestBod import { emailChangeRequestBodySchema } from "../../../../../lib/server/validation/userEmailChangeSchemas"; import { jsonFromZodError } from "../../../../../lib/server/validation/zodHttp"; -const EMAIL_CHANGE_TTL_MS = 15 * 60 * 1000; +const EMAIL_CHANGE_TTL_MS = 60 * 60 * 1000; const EMAIL_MIN_INTERVAL_MS = 60 * 1000; const IP_MIN_INTERVAL_MS = 20 * 1000; const SCOPE = "user.emailChange.request"; diff --git a/lib/server/mail.ts b/lib/server/mail.ts index 1b904cf..11c9b07 100644 --- a/lib/server/mail.ts +++ b/lib/server/mail.ts @@ -18,17 +18,22 @@ export function resolveMailFrom(): string { ); } -/** Plaintext + HTML for one-time verify URLs. HTML `href` survives quoted-printable wrapping. */ +/** Avoid quoted-printable `token=3D` wrapping that some webmail clients do not decode. */ +export const MAIL_TEXT_ENCODING = "base64" as const; + +/** Plaintext + HTML for one-time verify URLs. */ export function buildVerifyLinkParts( verifyUrl: string, intro: string, outro: string, linkLabel: string, ): { text: string; html: string } { - const text = `${intro}\n\n${verifyUrl}\n\n${outro}`; + const href = escapeHtml(verifyUrl); + const text = `${intro}\n\n<${verifyUrl}>\n\n${outro}`; const html = `

${escapeHtml(intro).replace(/\n/g, "
")}

` + - `

${escapeHtml(linkLabel)}

` + + `

${escapeHtml(linkLabel)}

` + + `

${href}

` + `

${escapeHtml(outro)}

`; return { text, html }; } @@ -59,6 +64,7 @@ async function sendHtmlMail(opts: { subject: opts.subject, text: opts.text, html: opts.html, + textEncoding: MAIL_TEXT_ENCODING, replyTo: opts.replyTo, }); } @@ -69,7 +75,7 @@ export async function sendMagicLinkEmail( ): Promise { const { text, html } = buildVerifyLinkParts( verifyUrl, - "Open this link to sign in (it expires in 15 minutes):", + "Open this link to sign in (it expires in 60 minutes):", "If you did not request this, you can ignore this email.", "Sign in", ); @@ -90,7 +96,7 @@ export async function sendRuleStakeholderInviteEmail( ): Promise { const { text, html } = buildVerifyLinkParts( verifyUrl, - `You've been invited to view "${ruleTitle}" on Community Rule.\n\nOpen this link to create your account (or sign in) and open the rule. The link expires in 15 minutes and works once:`, + `You've been invited to view "${ruleTitle}" on Community Rule.\n\nOpen this link to create your account (or sign in) and open the rule. The link expires in 60 minutes and works once:`, "If you did not expect this, you can ignore this email.", "Open the rule", ); @@ -134,7 +140,7 @@ export async function sendEmailChangeEmail( ): Promise { const { text, html } = buildVerifyLinkParts( verifyUrl, - "You asked to change the email on your Community Rule account.\n\nOpen this link to confirm the new address (it expires in 15 minutes):", + "You asked to change the email on your Community Rule account.\n\nOpen this link to confirm the new address (it expires in 60 minutes):", "If you did not request this change, you can ignore this email. Your current login is unchanged until you confirm.", "Confirm email", ); diff --git a/lib/server/ruleStakeholders.ts b/lib/server/ruleStakeholders.ts index f1eee6b..07eff34 100644 --- a/lib/server/ruleStakeholders.ts +++ b/lib/server/ruleStakeholders.ts @@ -1,2 +1,2 @@ -/** Parity with magic-link request TTL (15 minutes). */ -export const STAKEHOLDER_INVITE_TTL_MS = 15 * 60 * 1000; +/** Parity with magic-link request TTL (60 minutes). */ +export const STAKEHOLDER_INVITE_TTL_MS = 60 * 60 * 1000; diff --git a/tests/components/LoginForm.test.tsx b/tests/components/LoginForm.test.tsx index aeeb6b6..8a74e5b 100644 --- a/tests/components/LoginForm.test.tsx +++ b/tests/components/LoginForm.test.tsx @@ -240,6 +240,14 @@ describe("LoginForm", () => { ).toBeInTheDocument(); }); + it("shows URL-driven error for invalid_link", () => { + navMock.searchParams = new URLSearchParams("error=invalid_link"); + renderLoginForm(); + expect( + screen.getByText(/that sign-in link is not valid/i), + ).toBeInTheDocument(); + }); + it("calls router.replace to clear error query when user types (full-page /login)", async () => { const user = userEvent.setup(); navMock.pathname = "/login"; diff --git a/tests/unit/authMagicLinkVerifyRoute.test.ts b/tests/unit/authMagicLinkVerifyRoute.test.ts new file mode 100644 index 0000000..b16f634 --- /dev/null +++ b/tests/unit/authMagicLinkVerifyRoute.test.ts @@ -0,0 +1,91 @@ +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const isDatabaseConfiguredMock = vi.fn(); +const getSessionPepperMock = vi.fn(); +const hashSessionTokenMock = vi.fn(); +const findUniqueMock = vi.fn(); +const deleteMock = vi.fn(); +const upsertMock = vi.fn(); +const createSessionForUserMock = vi.fn(); +const setSessionCookieMock = vi.fn(); + +vi.mock("../../lib/server/env", () => ({ + isDatabaseConfigured: () => isDatabaseConfiguredMock(), + getSessionPepper: () => getSessionPepperMock(), +})); + +vi.mock("../../lib/server/hash", () => ({ + hashSessionToken: (...args: unknown[]) => hashSessionTokenMock(...args), +})); + +vi.mock("../../lib/server/session", () => ({ + createSessionForUser: (...args: unknown[]) => + createSessionForUserMock(...args), + setSessionCookie: (...args: unknown[]) => setSessionCookieMock(...args), +})); + +vi.mock("../../lib/server/db", () => ({ + prisma: { + magicLinkToken: { + findUnique: (...args: unknown[]) => findUniqueMock(...args), + delete: (...args: unknown[]) => deleteMock(...args), + }, + user: { + upsert: (...args: unknown[]) => upsertMock(...args), + }, + }, +})); + +import { GET } from "../../app/api/auth/magic-link/verify/route"; + +beforeEach(() => { + isDatabaseConfiguredMock.mockReset(); + getSessionPepperMock.mockReset(); + hashSessionTokenMock.mockReset(); + findUniqueMock.mockReset(); + deleteMock.mockReset(); + upsertMock.mockReset(); + createSessionForUserMock.mockReset(); + setSessionCookieMock.mockReset(); + + isDatabaseConfiguredMock.mockReturnValue(true); + getSessionPepperMock.mockReturnValue("pepper"); + hashSessionTokenMock.mockReturnValue("token-hash"); +}); + +function getWithToken(token: string) { + return new NextRequest( + `https://x.test/api/auth/magic-link/verify?token=${encodeURIComponent(token)}`, + ); +} + +describe("GET /api/auth/magic-link/verify", () => { + it("redirects with invalid_link when the token is missing", async () => { + const res = await GET( + new NextRequest("https://x.test/api/auth/magic-link/verify"), + ); + expect(res.status).toBe(307); + expect(res.headers.get("location")).toContain("error=invalid_link"); + }); + + it("redirects with invalid_link when no row matches", async () => { + findUniqueMock.mockResolvedValue(null); + const res = await GET(getWithToken("a-token-value-long-enough")); + expect(res.status).toBe(307); + expect(res.headers.get("location")).toContain("error=invalid_link"); + }); + + it("redirects with expired_link when the row is past expiresAt", async () => { + findUniqueMock.mockResolvedValue({ + id: "row-1", + email: "a@b.c", + expiresAt: new Date(Date.now() - 1000), + nextPath: null, + draftPayload: null, + }); + const res = await GET(getWithToken("a-token-value-long-enough")); + expect(res.status).toBe(307); + expect(res.headers.get("location")).toContain("error=expired_link"); + }); +}); diff --git a/tests/unit/mail.test.ts b/tests/unit/mail.test.ts index 0d1dabb..aded45f 100644 --- a/tests/unit/mail.test.ts +++ b/tests/unit/mail.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it } from "vitest"; import nodemailer from "nodemailer"; import { + MAIL_TEXT_ENCODING, buildVerifyLinkParts, resolveMailFrom, } from "../../lib/server/mail"; @@ -8,12 +9,17 @@ import { const VERIFY_URL = "https://staging.communityrule.info/api/auth/magic-link/verify?token=5IdE_BHowaw-QJj7Rwue7CbB8wDXvYITvnxRb1FGqxA"; -function decodeQuotedPrintable(value: string): string { - return value - .replace(/=\r?\n/g, "") - .replace(/=([0-9A-Fa-f]{2})/g, (_, hex: string) => - String.fromCharCode(Number.parseInt(hex, 16)), - ); +function decodeBase64Parts(raw: string): string { + const blocks = [ + ...raw.matchAll( + /Content-Transfer-Encoding:\s*base64\s*\r?\n\r?\n([A-Za-z0-9+/=\s]+)/gi, + ), + ]; + return blocks + .map((match) => + Buffer.from(match[1].replace(/\s/g, ""), "base64").toString("utf8"), + ) + .join("\n"); } const MAIL_FROM_KEYS = ["SMTP_FROM", "CLOUDRON_MAIL_FROM"] as const; @@ -30,15 +36,16 @@ afterEach(() => { }); describe("buildVerifyLinkParts", () => { - it("puts the exact verify URL in both text and the HTML href", () => { + it("puts the exact verify URL in text, href, and visible HTML link", () => { const { text, html } = buildVerifyLinkParts( VERIFY_URL, - "Open this link to sign in (it expires in 15 minutes):", + "Open this link to sign in (it expires in 60 minutes):", "If you did not request this, you can ignore this email.", "Sign in", ); - expect(text).toContain(VERIFY_URL); + expect(text).toContain(`<${VERIFY_URL}>`); expect(html).toContain(`href="${VERIFY_URL}"`); + expect(html).toContain(`>${VERIFY_URL}`); expect(html).toContain(">Sign in"); }); @@ -57,10 +64,10 @@ describe("buildVerifyLinkParts", () => { }); describe("MIME encoding of verify-link mail", () => { - it("keeps a clickable href after quoted-printable encoding", async () => { + it("uses base64 so the raw MIME never contains token=3D", async () => { const { text, html } = buildVerifyLinkParts( VERIFY_URL, - "Open this link to sign in (it expires in 15 minutes):", + "Open this link to sign in (it expires in 60 minutes):", "If you did not request this, you can ignore this email.", "Sign in", ); @@ -75,14 +82,17 @@ describe("MIME encoding of verify-link mail", () => { subject: "Sign in to Community Rule", text, html, + textEncoding: MAIL_TEXT_ENCODING, }); const raw = Buffer.isBuffer(info.message) ? info.message.toString("utf8") : String(info.message); - const decoded = decodeQuotedPrintable(raw); + expect(raw).toMatch(/Content-Transfer-Encoding:\s*base64/i); + expect(raw).not.toContain("token=3D"); + expect(raw).not.toMatch(/quoted-printable/i); + const decoded = decodeBase64Parts(raw); expect(decoded).toContain(`href="${VERIFY_URL}"`); expect(decoded).toContain(VERIFY_URL); - expect(decoded).not.toContain("token=3D"); }); });