fix: harden security post-review (JWT exposure, rate limiter, token expiry, timing)
- Raw JWT never sent to browser: page.tsx uses peekLinkingToken (read-only), client sends opaque linkingId, API calls retrieveLinkingToken server-side - link-account rate limiter now uses isRateLimited/registerFailedAttempt/ resetAttempts correctly (count only failures, reset on success) - linking_tokens gains expiresAt column (indexed) + opportunistic expiry purge on insert to prevent unbounded table growth - secureTokenCompare fixed: pad+slice to expected length so timing is constant regardless of submitted token length (no length leak) - autoLinkOAuth uses config.oauth.allowAutoLinking (boolean) instead of process.env truthy check that mishandles OAUTH_ALLOW_AUTO_LINKING=false - Add Permissions-Policy header; restore X-Frame-Options for legacy UAs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -17,13 +17,13 @@ import { signIn } from "next-auth/react";
|
||||
interface LinkAccountClientProps {
|
||||
provider: string;
|
||||
email: string;
|
||||
linkingToken: string;
|
||||
linkingId: string;
|
||||
}
|
||||
|
||||
export default function LinkAccountClient({
|
||||
provider,
|
||||
email,
|
||||
linkingToken
|
||||
linkingId
|
||||
}: LinkAccountClientProps) {
|
||||
const router = useRouter();
|
||||
const [password, setPassword] = useState("");
|
||||
@@ -41,7 +41,7 @@ export default function LinkAccountClient({
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
linkingToken,
|
||||
linkingId,
|
||||
password
|
||||
})
|
||||
});
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { redirect } from "next/navigation";
|
||||
import { auth } from "@/src/lib/auth";
|
||||
import { retrieveLinkingToken, verifyLinkingToken } from "@/src/lib/services/account-linking";
|
||||
import { peekLinkingToken, verifyLinkingToken } from "@/src/lib/services/account-linking";
|
||||
import LinkAccountClient from "./LinkAccountClient";
|
||||
|
||||
interface LinkAccountPageProps {
|
||||
@@ -26,25 +26,27 @@ export default async function LinkAccountPage({ searchParams }: LinkAccountPageP
|
||||
|
||||
const linkingId = errorParam.replace("LINKING_REQUIRED:", "");
|
||||
|
||||
// Retrieve the raw JWT from the server-side store (one-time use)
|
||||
const rawToken = await retrieveLinkingToken(linkingId);
|
||||
// Peek at the raw JWT to decode display info (provider, email) without consuming it.
|
||||
// The API endpoint will consume (retrieve + delete) the token during password verification.
|
||||
const rawToken = await peekLinkingToken(linkingId);
|
||||
|
||||
if (!rawToken) {
|
||||
redirect("/login?error=Linking token expired or invalid");
|
||||
}
|
||||
|
||||
// Verify token and decode
|
||||
// Verify token and decode for display purposes only
|
||||
const tokenPayload = await verifyLinkingToken(rawToken);
|
||||
|
||||
if (!tokenPayload) {
|
||||
redirect("/login?error=Linking token expired or invalid");
|
||||
}
|
||||
|
||||
// Pass only the opaque linkingId to the client — the raw JWT never leaves the server
|
||||
return (
|
||||
<LinkAccountClient
|
||||
provider={tokenPayload.provider}
|
||||
email={tokenPayload.email}
|
||||
linkingToken={rawToken}
|
||||
linkingId={linkingId}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,22 +1,30 @@
|
||||
import { NextRequest, NextResponse } from "next/server";
|
||||
import { verifyLinkingToken, verifyAndLinkOAuth } from "@/src/lib/services/account-linking";
|
||||
import { retrieveLinkingToken, verifyLinkingToken, verifyAndLinkOAuth } from "@/src/lib/services/account-linking";
|
||||
import { createAuditEvent } from "@/src/lib/models/audit";
|
||||
import { registerFailedAttempt } from "@/src/lib/rate-limit";
|
||||
import { isRateLimited, registerFailedAttempt, resetAttempts } from "@/src/lib/rate-limit";
|
||||
|
||||
export async function POST(request: NextRequest) {
|
||||
try {
|
||||
const body = await request.json();
|
||||
const { linkingToken, password } = body;
|
||||
const { linkingId, password } = body;
|
||||
|
||||
if (!linkingToken || !password) {
|
||||
if (!linkingId || !password) {
|
||||
return NextResponse.json(
|
||||
{ error: "Missing required fields" },
|
||||
{ status: 400 }
|
||||
);
|
||||
}
|
||||
|
||||
// Verify linking token
|
||||
const tokenPayload = await verifyLinkingToken(linkingToken);
|
||||
// Retrieve and consume the linking token server-side — the raw JWT never reaches the browser
|
||||
const rawToken = await retrieveLinkingToken(linkingId);
|
||||
if (!rawToken) {
|
||||
return NextResponse.json(
|
||||
{ error: "Authentication failed" },
|
||||
{ status: 401 }
|
||||
);
|
||||
}
|
||||
|
||||
const tokenPayload = await verifyLinkingToken(rawToken);
|
||||
if (!tokenPayload) {
|
||||
return NextResponse.json(
|
||||
{ error: "Authentication failed" },
|
||||
@@ -24,11 +32,10 @@ export async function POST(request: NextRequest) {
|
||||
);
|
||||
}
|
||||
|
||||
// Rate limiting: prevent brute force password attacks during OAuth linking
|
||||
// Rate limiting: check before attempting password verification
|
||||
const rateLimitKey = `oauth-link-verify:${tokenPayload.userId}`;
|
||||
const rateLimitResult = registerFailedAttempt(rateLimitKey);
|
||||
if (rateLimitResult.blocked) {
|
||||
// Audit log for blocked attempt
|
||||
const rateLimitCheck = isRateLimited(rateLimitKey);
|
||||
if (rateLimitCheck.blocked) {
|
||||
await createAuditEvent({
|
||||
userId: tokenPayload.userId,
|
||||
action: "oauth_link_rate_limited",
|
||||
@@ -53,7 +60,9 @@ export async function POST(request: NextRequest) {
|
||||
);
|
||||
|
||||
if (!success) {
|
||||
// Audit log for failed password verification
|
||||
// Count this failure against the rate limit
|
||||
registerFailedAttempt(rateLimitKey);
|
||||
|
||||
await createAuditEvent({
|
||||
userId: tokenPayload.userId,
|
||||
action: "oauth_link_password_failed",
|
||||
@@ -69,7 +78,9 @@ export async function POST(request: NextRequest) {
|
||||
);
|
||||
}
|
||||
|
||||
// Audit log
|
||||
// Success — clear rate limit for this user
|
||||
resetAttempts(rateLimitKey);
|
||||
|
||||
await createAuditEvent({
|
||||
userId: tokenPayload.userId,
|
||||
action: "account_linked",
|
||||
|
||||
@@ -16,13 +16,12 @@ const SYNC_RATE_LIMITS = new Map<string, { count: number; windowStart: number }>
|
||||
* Timing-safe token comparison to prevent timing attacks
|
||||
*/
|
||||
function secureTokenCompare(a: string, b: string): boolean {
|
||||
if (a.length !== b.length) {
|
||||
// Compare against dummy to maintain constant time
|
||||
const dummy = Buffer.alloc(a.length, 0);
|
||||
timingSafeEqual(Buffer.from(a), dummy);
|
||||
return false;
|
||||
}
|
||||
return timingSafeEqual(Buffer.from(a), Buffer.from(b));
|
||||
// Always compare buffers of the expected length (b) to avoid leaking
|
||||
// the expected token length via early-return timing when a.length !== b.length
|
||||
const bufA = Buffer.from(a.padEnd(b.length, "\0").slice(0, b.length));
|
||||
const bufB = Buffer.from(b);
|
||||
const equal = timingSafeEqual(bufA, bufB);
|
||||
return equal && a.length === b.length;
|
||||
}
|
||||
|
||||
function getClientIp(request: NextRequest): string {
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
CREATE TABLE `linking_tokens` (
|
||||
`id` text PRIMARY KEY NOT NULL,
|
||||
`token` text NOT NULL,
|
||||
`created_at` text NOT NULL
|
||||
`created_at` text NOT NULL,
|
||||
`expires_at` text NOT NULL
|
||||
);
|
||||
CREATE INDEX `linking_tokens_expires_at_idx` ON `linking_tokens` (`expires_at`);
|
||||
|
||||
+3
-1
@@ -16,8 +16,10 @@ const nextConfig = {
|
||||
source: "/(.*)",
|
||||
headers: [
|
||||
{ key: "X-Content-Type-Options", value: "nosniff" },
|
||||
// X-Frame-Options omitted: frame-ancestors in CSP supersedes it in all modern browsers
|
||||
// X-Frame-Options kept for legacy browsers that don't support frame-ancestors CSP directive
|
||||
{ key: "X-Frame-Options", value: "DENY" },
|
||||
{ key: "Referrer-Policy", value: "strict-origin-when-cross-origin" },
|
||||
{ key: "Permissions-Policy", value: "camera=(), microphone=(), geolocation=(), interest-cohort=()" },
|
||||
{
|
||||
key: "Content-Security-Policy",
|
||||
value: [
|
||||
|
||||
@@ -182,5 +182,6 @@ export const auditEvents = sqliteTable("audit_events", {
|
||||
export const linkingTokens = sqliteTable("linking_tokens", {
|
||||
id: text("id").primaryKey(),
|
||||
token: text("token").notNull(),
|
||||
createdAt: text("created_at").notNull()
|
||||
createdAt: text("created_at").notNull(),
|
||||
expiresAt: text("expires_at").notNull()
|
||||
});
|
||||
|
||||
@@ -5,7 +5,7 @@ import { config } from "../config";
|
||||
import { findUserByEmail, findUserByProviderSubject, getUserById } from "../models/user";
|
||||
import db from "../db";
|
||||
import { users, linkingTokens } from "../db/schema";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { eq, lt } from "drizzle-orm";
|
||||
import { nowIso } from "../db";
|
||||
|
||||
const LINKING_TOKEN_EXPIRY = 5 * 60; // 5 minutes in seconds
|
||||
@@ -124,25 +124,53 @@ export async function verifyLinkingToken(token: string): Promise<LinkingTokenPay
|
||||
}
|
||||
|
||||
/**
|
||||
* Store a linking JWT in the DB and return an opaque 64-char hex ID
|
||||
* Store a linking JWT in the DB and return an opaque 64-char hex ID.
|
||||
* Expired rows are purged on each insert to prevent unbounded table growth.
|
||||
*/
|
||||
export async function storeLinkingToken(token: string): Promise<string> {
|
||||
const id = randomBytes(32).toString("hex");
|
||||
const now = nowIso();
|
||||
const expiresAt = new Date(Date.now() + LINKING_TOKEN_EXPIRY * 1000).toISOString();
|
||||
|
||||
// Purge expired tokens opportunistically
|
||||
await db.delete(linkingTokens).where(lt(linkingTokens.expiresAt, now));
|
||||
|
||||
await db.insert(linkingTokens).values({
|
||||
id,
|
||||
token,
|
||||
createdAt: nowIso()
|
||||
createdAt: now,
|
||||
expiresAt
|
||||
});
|
||||
return id;
|
||||
}
|
||||
|
||||
/**
|
||||
* Peek at a linking token by its opaque ID without consuming (deleting) it.
|
||||
* Used by the link-account page to decode display info (provider, email) while
|
||||
* keeping the token available for the subsequent API call.
|
||||
* Returns null if the ID is not found or the token is expired.
|
||||
*/
|
||||
export async function peekLinkingToken(id: string): Promise<string | null> {
|
||||
const now = nowIso();
|
||||
const rows = await db.select().from(linkingTokens)
|
||||
.where(eq(linkingTokens.id, id))
|
||||
.limit(1);
|
||||
if (rows.length === 0 || rows[0].expiresAt < now) {
|
||||
return null;
|
||||
}
|
||||
return rows[0].token;
|
||||
}
|
||||
|
||||
/**
|
||||
* Retrieve and delete a linking token by its opaque ID (one-time use).
|
||||
* Returns null if the ID is not found.
|
||||
* Returns null if the ID is not found or the token is expired.
|
||||
*/
|
||||
export async function retrieveLinkingToken(id: string): Promise<string | null> {
|
||||
const rows = await db.select().from(linkingTokens).where(eq(linkingTokens.id, id)).limit(1);
|
||||
if (rows.length === 0) {
|
||||
const now = nowIso();
|
||||
const rows = await db.select().from(linkingTokens)
|
||||
.where(eq(linkingTokens.id, id))
|
||||
.limit(1);
|
||||
if (rows.length === 0 || rows[0].expiresAt < now) {
|
||||
return null;
|
||||
}
|
||||
const { token } = rows[0];
|
||||
@@ -199,7 +227,7 @@ export async function autoLinkOAuth(
|
||||
|
||||
// Don't auto-link if user has a password (unless explicitly called for authenticated linking)
|
||||
// This check is bypassed when called from the authenticated linking flow
|
||||
if (user.password_hash && !process.env.OAUTH_ALLOW_AUTO_LINKING) {
|
||||
if (user.password_hash && !config.oauth.allowAutoLinking) {
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user