From 7140cf6f2787ecd60427ef4791b64cd0afd6a7db Mon Sep 17 00:00:00 2001 From: counterweight Date: Fri, 19 Dec 2025 10:30:23 +0100 Subject: [PATCH] first round of review --- backend/models.py | 2 +- backend/seed.py | 2 - backend/tests/test_profile.py | 1 - backend/tests/test_validation.py | 24 ++++------- backend/validation.py | 15 ++----- frontend/app/profile/page.tsx | 70 ++++++++++++++++++-------------- frontend/e2e/profile.spec.ts | 2 +- frontend/package-lock.json | 7 ++++ frontend/package.json | 1 + 9 files changed, 61 insertions(+), 63 deletions(-) diff --git a/backend/models.py b/backend/models.py index a08e755..e569adb 100644 --- a/backend/models.py +++ b/backend/models.py @@ -105,7 +105,7 @@ class User(Base): contact_email: Mapped[str | None] = mapped_column(String(255), nullable=True) telegram: Mapped[str | None] = mapped_column(String(64), nullable=True) signal: Mapped[str | None] = mapped_column(String(64), nullable=True) - nostr_npub: Mapped[str | None] = mapped_column(String(64), nullable=True) + nostr_npub: Mapped[str | None] = mapped_column(String(63), nullable=True) # Relationship to roles roles: Mapped[list[Role]] = relationship( diff --git a/backend/seed.py b/backend/seed.py index 45f7a09..185c2a6 100644 --- a/backend/seed.py +++ b/backend/seed.py @@ -67,8 +67,6 @@ async def upsert_user(db: AsyncSession, email: str, password: str, role_names: l async def seed() -> None: async with engine.begin() as conn: - # Drop all tables and recreate to ensure schema is up to date - await conn.run_sync(Base.metadata.drop_all) await conn.run_sync(Base.metadata.create_all) async with async_session() as db: diff --git a/backend/tests/test_profile.py b/backend/tests/test_profile.py index 5a45c56..7cae958 100644 --- a/backend/tests/test_profile.py +++ b/backend/tests/test_profile.py @@ -397,4 +397,3 @@ class TestProfilePrivacy: assert "telegram" not in data assert "signal" not in data assert "nostr_npub" not in data - diff --git a/backend/tests/test_validation.py b/backend/tests/test_validation.py index 3bcebaf..0f64ab1 100644 --- a/backend/tests/test_validation.py +++ b/backend/tests/test_validation.py @@ -61,8 +61,8 @@ class TestValidateTelegram: assert validate_telegram("@alice_bob") is None def test_valid_handle_min_length(self): - # 5 characters after @ - assert validate_telegram("@abcde") is None + # 1 character after @ + assert validate_telegram("@a") is None def test_valid_handle_max_length(self): # 32 characters after @ @@ -77,26 +77,20 @@ class TestValidateTelegram: result = validate_telegram("@") assert result is not None - def test_too_short(self): - # Less than 5 characters after @ - result = validate_telegram("@abcd") - assert result is not None - assert "5" in result - def test_too_long(self): # More than 32 characters after @ result = validate_telegram("@" + "a" * 33) assert result is not None assert "32" in result - def test_starts_with_number(self): - result = validate_telegram("@1alice") - assert result is not None - assert "letter" in result.lower() + def test_starts_with_number_is_valid(self): + # Now allowed - any character is valid + assert validate_telegram("@1alice") is None - def test_invalid_characters(self): - result = validate_telegram("@alice-bob") - assert result is not None + def test_special_characters_are_valid(self): + # Now allowed - any character is valid + assert validate_telegram("@alice-bob") is None + assert validate_telegram("@test.user") is None class TestValidateSignal: diff --git a/backend/validation.py b/backend/validation.py index e7c5d39..b697306 100644 --- a/backend/validation.py +++ b/backend/validation.py @@ -1,5 +1,4 @@ """Validation utilities for user profile fields.""" -import re from email_validator import validate_email, EmailNotValidError from bech32 import bech32_decode @@ -25,7 +24,7 @@ def validate_telegram(value: str | None) -> str | None: """ Validate Telegram handle. - Must start with @ if provided. + Must start with @ if provided, with 1-32 characters after @. Returns None if valid, error message if invalid. Empty/None values are valid (field is optional). """ @@ -35,21 +34,13 @@ def validate_telegram(value: str | None) -> str | None: if not value.startswith("@"): return "Telegram handle must start with @" - if len(value) < 2: - return "Telegram handle must have at least one character after @" - - # Telegram usernames: 5-32 characters, alphanumeric and underscores - # But we store with @, so check 6-33 total handle = value[1:] # Remove @ - if len(handle) < 5: - return "Telegram handle must be at least 5 characters (after @)" + if len(handle) < 1: + return "Telegram handle must have at least one character after @" if len(handle) > 32: return "Telegram handle must be at most 32 characters (after @)" - if not re.match(r'^[a-zA-Z][a-zA-Z0-9_]*$', handle): - return "Telegram handle must start with a letter and contain only letters, numbers, and underscores" - return None diff --git a/frontend/app/profile/page.tsx b/frontend/app/profile/page.tsx index 68584ce..9fcc393 100644 --- a/frontend/app/profile/page.tsx +++ b/frontend/app/profile/page.tsx @@ -2,6 +2,7 @@ import { useEffect, useState, useCallback } from "react"; import { useRouter } from "next/navigation"; +import { bech32 } from "bech32"; import { useAuth } from "../auth-context"; import { API_URL } from "../config"; import { sharedStyles } from "../styles/shared"; @@ -30,7 +31,9 @@ interface FieldErrors { // Client-side validation matching backend rules function validateEmail(value: string): string | undefined { if (!value) return undefined; - const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + // More comprehensive email regex that matches email-validator behavior + // Checks for: local part, @, domain with at least one dot, valid TLD + const emailRegex = /^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)+$/; if (!emailRegex.test(value)) { return "Please enter a valid email address"; } @@ -43,15 +46,12 @@ function validateTelegram(value: string): string | undefined { return "Telegram handle must start with @"; } const handle = value.slice(1); - if (handle.length < 5) { - return "Telegram handle must be at least 5 characters (after @)"; + if (handle.length < 1) { + return "Telegram handle must have at least one character after @"; } if (handle.length > 32) { return "Telegram handle must be at most 32 characters (after @)"; } - if (!/^[a-zA-Z][a-zA-Z0-9_]*$/.test(handle)) { - return "Telegram handle must start with a letter and contain only letters, numbers, and underscores"; - } return undefined; } @@ -71,15 +71,21 @@ function validateNostrNpub(value: string): string | undefined { if (!value.startsWith("npub1")) { return "Nostr npub must start with 'npub1'"; } - // Basic length check (valid npubs are 63 characters) - if (value.length !== 63) { - return "Invalid Nostr npub format"; + + try { + const decoded = bech32.decode(value); + if (decoded.prefix !== "npub") { + return "Nostr npub must have 'npub' prefix"; + } + // npub should decode to 32 bytes (256 bits) for a public key + // In bech32, each character encodes 5 bits, so 32 bytes = 52 characters of data + if (decoded.words.length !== 52) { + return "Invalid Nostr npub: incorrect length"; + } + return undefined; + } catch { + return "Invalid Nostr npub: bech32 checksum failed"; } - // Check for valid bech32 characters - if (!/^npub1[023456789acdefghjklmnpqrstuvwxyz]+$/.test(value)) { - return "Invalid Nostr npub: contains invalid characters"; - } - return undefined; } function validateForm(data: FormData): FieldErrors { @@ -138,21 +144,7 @@ export default function ProfilePage() { } }, [isLoading, user, router, isRegularUser]); - useEffect(() => { - if (user && isRegularUser) { - fetchProfile(); - } - }, [user, isRegularUser]); - - // Auto-dismiss toast after 3 seconds - useEffect(() => { - if (toast) { - const timer = setTimeout(() => setToast(null), 3000); - return () => clearTimeout(timer); - } - }, [toast]); - - const fetchProfile = async () => { + const fetchProfile = useCallback(async () => { try { const res = await fetch(`${API_URL}/api/profile`, { credentials: "include", @@ -167,13 +159,29 @@ export default function ProfilePage() { }; setFormData(formValues); setOriginalData(formValues); + } else { + setToast({ message: "Failed to load profile", type: "error" }); } } catch { - // Handle error silently for now + setToast({ message: "Network error. Please try again.", type: "error" }); } finally { setIsLoadingProfile(false); } - }; + }, []); + + useEffect(() => { + if (user && isRegularUser) { + fetchProfile(); + } + }, [user, isRegularUser, fetchProfile]); + + // Auto-dismiss toast after 3 seconds + useEffect(() => { + if (toast) { + const timer = setTimeout(() => setToast(null), 3000); + return () => clearTimeout(timer); + } + }, [toast]); const handleInputChange = (field: keyof FormData) => (e: React.ChangeEvent) => { const value = e.target.value; diff --git a/frontend/e2e/profile.spec.ts b/frontend/e2e/profile.spec.ts index 1f42908..35499ab 100644 --- a/frontend/e2e/profile.spec.ts +++ b/frontend/e2e/profile.spec.ts @@ -207,7 +207,7 @@ test.describe("Profile - Form Behavior", () => { await expect(page.getByText(/saved successfully/i)).toBeVisible(); // Wait for toast to disappear - await page.waitForTimeout(3500); + await expect(page.getByText(/saved successfully/i)).not.toBeVisible({ timeout: 5000 }); // Clear the field await page.fill("#telegram", ""); diff --git a/frontend/package-lock.json b/frontend/package-lock.json index a9930bd..c06727e 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -8,6 +8,7 @@ "name": "frontend", "version": "0.1.0", "dependencies": { + "bech32": "^2.0.0", "next": "15.1.2", "react": "19.0.0", "react-dom": "19.0.0" @@ -2092,6 +2093,12 @@ "baseline-browser-mapping": "dist/cli.js" } }, + "node_modules/bech32": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/bech32/-/bech32-2.0.0.tgz", + "integrity": "sha512-LcknSilhIGatDAsY1ak2I8VtGaHNhgMSYVxFrGLXv+xLHytaKZKcaUJJUE7qmBr7h33o5YQwP55pMI0xmkpJwg==", + "license": "MIT" + }, "node_modules/browserslist": { "version": "4.28.1", "resolved": "https://registry.npmjs.org/browserslist/-/browserslist-4.28.1.tgz", diff --git a/frontend/package.json b/frontend/package.json index daafff1..3dfd1a3 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -10,6 +10,7 @@ "test:e2e": "playwright test" }, "dependencies": { + "bech32": "^2.0.0", "next": "15.1.2", "react": "19.0.0", "react-dom": "19.0.0"