From 1b5c74e7bfeb96c86670a9bff9661038c6fabc8b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 17:37:28 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix:=20drop=20country=20tracking?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the `users.country` field, the `users.hasCountry` / `users.recordCountry` functions, the `POST /api/account/country` Worker route and the client hook that called it, restoring the pre-#33 code. The frontend went out without the matching Convex functions, and `useQuery(api.users.hasCountry)` sits in `ConvexClientProvider`, which wraps the whole app: a query for a function the deployment does not have throws during render, so every signed-in visit broke and production had to be rolled back. The feature is not wanted any more, so it is removed rather than hardened. Production never received the country schema, so this deploys cleanly. --- convex/_generated/api.d.ts | 2 - convex/lib/country.ts | 11 ---- convex/schema.ts | 22 ------- convex/users.test.ts | 70 --------------------- convex/users.ts | 42 +------------ src/__tests__/account-country-route.test.ts | 67 -------------------- src/app/api/account/country/route.ts | 56 ----------------- src/components/ConvexClientProvider.tsx | 21 +------ 8 files changed, 2 insertions(+), 289 deletions(-) delete mode 100644 convex/lib/country.ts delete mode 100644 convex/users.test.ts delete mode 100644 src/__tests__/account-country-route.test.ts delete mode 100644 src/app/api/account/country/route.ts diff --git a/convex/_generated/api.d.ts b/convex/_generated/api.d.ts index 6f3cc7f..e999458 100644 --- a/convex/_generated/api.d.ts +++ b/convex/_generated/api.d.ts @@ -11,7 +11,6 @@ import type * as auth from "../auth.js"; import type * as http from "../http.js"; import type * as lib_auth from "../lib/auth.js"; -import type * as lib_country from "../lib/country.js"; import type * as lib_hierarchy from "../lib/hierarchy.js"; import type * as lib_sync from "../lib/sync.js"; import type * as migrations from "../migrations.js"; @@ -29,7 +28,6 @@ declare const fullApi: ApiFromModules<{ auth: typeof auth; http: typeof http; "lib/auth": typeof lib_auth; - "lib/country": typeof lib_country; "lib/hierarchy": typeof lib_hierarchy; "lib/sync": typeof lib_sync; migrations: typeof migrations; diff --git a/convex/lib/country.ts b/convex/lib/country.ts deleted file mode 100644 index 4aa0b1b..0000000 --- a/convex/lib/country.ts +++ /dev/null @@ -1,11 +0,0 @@ -/** - * Cloudflare reports "XX" when it cannot place a client and "T1" for Tor exit - * nodes. Neither is a country, so neither is stored. - */ -const NOT_A_COUNTRY = new Set(["XX", "T1"]); - -/** A clean upper-case ISO 3166-1 alpha-2 code, or `null` when the value is not one. */ -export function normalizeCountry(value: string | null | undefined): string | null { - const code = value?.trim().toUpperCase() ?? ""; - return /^[A-Z]{2}$/.test(code) && !NOT_A_COUNTRY.has(code) ? code : null; -} diff --git a/convex/schema.ts b/convex/schema.ts index 01cf8eb..a129f27 100644 --- a/convex/schema.ts +++ b/convex/schema.ts @@ -42,28 +42,6 @@ const syncedFields = { export default defineSchema({ ...authTables, - // Convex Auth's own `users` table, restated so it can carry one extra field. - // Everything but `country` is copied from `authTables.users`; keep it in step - // with that definition when upgrading @convex-dev/auth. - users: defineTable({ - name: v.optional(v.string()), - image: v.optional(v.string()), - email: v.optional(v.string()), - emailVerificationTime: v.optional(v.number()), - phone: v.optional(v.string()), - phoneVerificationTime: v.optional(v.number()), - isAnonymous: v.optional(v.boolean()), - /** - * ISO 3166-1 alpha-2 country the account was first seen from (e.g. "ES"), - * as reported by Cloudflare. Set once, never overwritten, and only the - * country — the IP address is never stored. Absent for accounts that have - * not opened the app since this was introduced. - */ - country: v.optional(v.string()), - }) - .index("email", ["email"]) - .index("phone", ["phone"]), - folders: defineTable({ ...syncedFields, /** Ciphertext when `cryptoVersion` > 0, plaintext when 0. */ diff --git a/convex/users.test.ts b/convex/users.test.ts deleted file mode 100644 index c1d3b89..0000000 --- a/convex/users.test.ts +++ /dev/null @@ -1,70 +0,0 @@ -/// -// @vitest-environment edge-runtime -import { convexTest } from "convex-test"; -import { beforeEach, describe, expect, it } from "vitest"; - -import { api } from "./_generated/api"; -import type { Id } from "./_generated/dataModel"; -import { normalizeCountry } from "./lib/country"; -import schema from "./schema"; - -const modules = import.meta.glob("./**/*.ts"); - -let t: ReturnType; -let alice: Id<"users">; -let bob: Id<"users">; - -beforeEach(async () => { - t = convexTest(schema, modules); - alice = await t.run((ctx) => ctx.db.insert("users", {})); - bob = await t.run((ctx) => ctx.db.insert("users", {})); -}); - -const as = (userId: Id<"users">) => t.withIdentity({ subject: userId }); -const countryOf = (userId: Id<"users">) => t.run(async (ctx) => (await ctx.db.get(userId))?.country ?? null); - -describe("normalizeCountry", () => { - it("accepts two letters in any case and rejects everything else", () => { - expect(normalizeCountry("es")).toBe("ES"); - expect(normalizeCountry(" GB ")).toBe("GB"); - for (const bad of ["", "E", "ESP", "E1", "1E", null, undefined]) expect(normalizeCountry(bad)).toBeNull(); - }); - - it("does not treat Cloudflare's unknown and Tor markers as countries", () => { - expect(normalizeCountry("XX")).toBeNull(); - expect(normalizeCountry("T1")).toBeNull(); - }); -}); - -describe("recordCountry", () => { - it("requires a signed-in caller", async () => { - await expect(t.mutation(api.users.recordCountry, { country: "ES" })).rejects.toThrow("Not authenticated"); - }); - - it("stores the country once and never replaces it", async () => { - expect(await as(alice).mutation(api.users.recordCountry, { country: "es" })).toEqual({ stored: true }); - expect(await as(alice).mutation(api.users.recordCountry, { country: "FR" })).toEqual({ stored: false }); - expect(await countryOf(alice)).toBe("ES"); - }); - - it("ignores values that are not a country", async () => { - expect(await as(alice).mutation(api.users.recordCountry, { country: "XX" })).toEqual({ stored: false }); - expect(await as(alice).mutation(api.users.recordCountry, { country: "not-a-country" })).toEqual({ stored: false }); - expect(await countryOf(alice)).toBeNull(); - }); - - it("only ever writes the caller's own account", async () => { - await as(alice).mutation(api.users.recordCountry, { country: "ES" }); - expect(await countryOf(bob)).toBeNull(); - }); -}); - -describe("hasCountry", () => { - it("is null signed out, false until a country is recorded, then true", async () => { - expect(await t.query(api.users.hasCountry, {})).toBeNull(); - expect(await as(alice).query(api.users.hasCountry, {})).toBe(false); - await as(alice).mutation(api.users.recordCountry, { country: "US" }); - expect(await as(alice).query(api.users.hasCountry, {})).toBe(true); - expect(await as(bob).query(api.users.hasCountry, {})).toBe(false); - }); -}); diff --git a/convex/users.ts b/convex/users.ts index bff9795..34d4387 100644 --- a/convex/users.ts +++ b/convex/users.ts @@ -1,9 +1,5 @@ -import { v } from "convex/values"; - import type { Id } from "./_generated/dataModel"; -import { mutation, query } from "./_generated/server"; -import { requireUserId } from "./lib/auth"; -import { normalizeCountry } from "./lib/country"; +import { query } from "./_generated/server"; /** * The signed-in account, or `null` when there is no session. @@ -35,39 +31,3 @@ export const viewer = query({ }; }, }); - -/** - * Whether the caller's account already has a country on record, or `null` when - * signed out. The client asks this before reporting one, so an account that is - * already known costs no further requests. - */ -export const hasCountry = query({ - args: {}, - handler: async (ctx) => { - const identity = await ctx.auth.getUserIdentity(); - if (!identity) return null; - - const user = await ctx.db.get(identity.subject.split("|")[0] as Id<"users">); - return user ? user.country !== undefined : null; - }, -}); - -/** - * Records the country an account was first seen from. First write wins: a - * country already on file is never replaced, so travelling or a VPN cannot - * rewrite it, and an unusable value ("XX", "T1", garbage) stores nothing. - */ -export const recordCountry = mutation({ - args: { country: v.string() }, - handler: async (ctx, args) => { - const userId = await requireUserId(ctx); - const country = normalizeCountry(args.country); - if (!country) return { stored: false }; - - const user = await ctx.db.get(userId); - if (!user || user.country !== undefined) return { stored: false }; - - await ctx.db.patch(userId, { country }); - return { stored: true }; - }, -}); diff --git a/src/__tests__/account-country-route.test.ts b/src/__tests__/account-country-route.test.ts deleted file mode 100644 index 1a69211..0000000 --- a/src/__tests__/account-country-route.test.ts +++ /dev/null @@ -1,67 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; - -const mutation = vi.fn(); -let cfCountry: string | undefined; -let convexConfigured = true; - -vi.mock("@opennextjs/cloudflare", () => ({ - getCloudflareContext: () => ({ cf: cfCountry === undefined ? undefined : { country: cfCountry } }), -})); - -vi.mock("@/lib/convexServer", () => ({ - readBearerToken: (request: Request) => request.headers.get("authorization")?.replace(/^Bearer /, "") || null, - getConvexClientForToken: () => (convexConfigured ? { mutation } : null), -})); - -import { POST } from "@/app/api/account/country/route"; - -function post(headers: Record = {}) { - return POST(new Request("https://klipcode.test/api/account/country", { method: "POST", headers })); -} - -beforeEach(() => { - mutation.mockReset(); - mutation.mockResolvedValue({ stored: true }); - cfCountry = undefined; - convexConfigured = true; -}); - -describe("POST /api/account/country", () => { - it("rejects a request without a token", async () => { - expect((await post({ "cf-ipcountry": "ES" })).status).toBe(401); - expect(mutation).not.toHaveBeenCalled(); - }); - - it("reports the country from Cloudflare's header, normalised", async () => { - const res = await post({ authorization: "Bearer t", "cf-ipcountry": "es" }); - expect(await res.json()).toEqual({ stored: true }); - expect(mutation).toHaveBeenCalledWith(expect.anything(), { country: "ES" }); - }); - - it("falls back to request.cf.country when the header is missing", async () => { - cfCountry = "GB"; - await post({ authorization: "Bearer t" }); - expect(mutation).toHaveBeenCalledWith(expect.anything(), { country: "GB" }); - }); - - it.each(["XX", "T1", "", "ESP"])("stores nothing for %j", async (value) => { - const res = await post({ authorization: "Bearer t", "cf-ipcountry": value }); - expect(await res.json()).toEqual({ stored: false }); - expect(mutation).not.toHaveBeenCalled(); - }); - - it("stores nothing when Cloudflare gives no country at all", async () => { - expect(await (await post({ authorization: "Bearer t" })).json()).toEqual({ stored: false }); - expect(mutation).not.toHaveBeenCalled(); - }); - - it("does nothing without a Convex deployment", async () => { - convexConfigured = false; - expect(await (await post({ authorization: "Bearer t", "cf-ipcountry": "ES" })).json()).toEqual({ stored: false }); - }); - - it("answers 401 when Convex rejects the token", async () => { - mutation.mockRejectedValue(new Error("Not authenticated")); - expect((await post({ authorization: "Bearer t", "cf-ipcountry": "ES" })).status).toBe(401); - }); -}); diff --git a/src/app/api/account/country/route.ts b/src/app/api/account/country/route.ts deleted file mode 100644 index e4f5bdf..0000000 --- a/src/app/api/account/country/route.ts +++ /dev/null @@ -1,56 +0,0 @@ -import { getCloudflareContext } from "@opennextjs/cloudflare"; -import { api } from "@convex/_generated/api"; -import { normalizeCountry } from "@convex/lib/country"; -import { getConvexClientForToken, readBearerToken } from "@/lib/convexServer"; - -/** - * Tells Convex which country the signed-in user is connecting from. - * - * This has to be a Cloudflare route: the sign-in callback is served by Convex - * itself, which never sees Cloudflare's geolocation, whereas every request that - * reaches this Worker carries it. Only the two-letter country is read — the IP - * address is neither looked at nor stored. - * - * Like the other routes here it acts purely as the caller (their own token, no - * admin key), and `users.recordCountry` keeps the first value it is given. - */ - -function json(body: unknown, status = 200) { - return Response.json(body, { status, headers: { "cache-control": "no-store" } }); -} - -function requestCountry(request: Request): string | null { - const fromHeader = normalizeCountry(request.headers.get("cf-ipcountry")); - if (fromHeader) return fromHeader; - - try { - const cf = getCloudflareContext().cf as { country?: string } | undefined; - return normalizeCountry(cf?.country); - } catch { - // Outside the Workers runtime (plain `next dev`, tests): no geolocation. - return null; - } -} - -export async function POST(request: Request) { - const token = readBearerToken(request); - if (!token) { - return json({ error: "unauthorized" }, 401); - } - - const convex = getConvexClientForToken(token); - if (!convex) { - return json({ stored: false }); - } - - const country = requestCountry(request); - if (!country) { - return json({ stored: false }); - } - - try { - return json(await convex.mutation(api.users.recordCountry, { country })); - } catch { - return json({ error: "unauthorized" }, 401); - } -} diff --git a/src/components/ConvexClientProvider.tsx b/src/components/ConvexClientProvider.tsx index 405e684..84f7801 100644 --- a/src/components/ConvexClientProvider.tsx +++ b/src/components/ConvexClientProvider.tsx @@ -3,7 +3,7 @@ import { api } from "@convex/_generated/api"; import { ConvexAuthProvider, useAuthActions, useAuthToken } from "@convex-dev/auth/react"; import { useConvexAuth, useQuery } from "convex/react"; -import { useCallback, useEffect, useMemo, useRef } from "react"; +import { useCallback, useEffect, useMemo } from "react"; import { CloudSessionContext, type CloudSession } from "@/hooks/useCloudSession"; import { setAuthToken } from "@/lib/authToken"; @@ -28,25 +28,6 @@ function CloudSessionBridge({ children }: { children: React.ReactNode }) { setAuthToken(token ?? null); }, [token]); - // Accounts that predate country tracking, and new ones, report it once: the - // Worker reads it from Cloudflare and Convex keeps the first value. Failures - // are ignored — this is bookkeeping and the next visit simply asks again. - const hasCountry = useQuery(api.users.hasCountry, isAuthenticated ? {} : "skip"); - const countryAskedFor = useRef(null); - useEffect(() => { - const userId = viewer?.id; - if (!token || !userId || hasCountry !== false || countryAskedFor.current === userId) { - return; - } - countryAskedFor.current = userId; - void fetch("/api/account/country", { - method: "POST", - headers: { authorization: `Bearer ${token}` }, - }).catch(() => { - countryAskedFor.current = null; - }); - }, [hasCountry, token, viewer?.id]); - const handleSignIn = useCallback(async () => { await signIn("github", { redirectTo: window.location.href }); }, [signIn]);