first round of review

This commit is contained in:
counterweight 2025-12-19 10:30:23 +01:00
parent 5908660e56
commit 7140cf6f27
Signed by: counterweight
GPG key ID: 883EDBAA726BD96C
9 changed files with 61 additions and 63 deletions

View file

@ -105,7 +105,7 @@ class User(Base):
contact_email: Mapped[str | None] = mapped_column(String(255), nullable=True) contact_email: Mapped[str | None] = mapped_column(String(255), nullable=True)
telegram: Mapped[str | None] = mapped_column(String(64), nullable=True) telegram: Mapped[str | None] = mapped_column(String(64), nullable=True)
signal: 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 # Relationship to roles
roles: Mapped[list[Role]] = relationship( roles: Mapped[list[Role]] = relationship(

View file

@ -67,8 +67,6 @@ async def upsert_user(db: AsyncSession, email: str, password: str, role_names: l
async def seed() -> None: async def seed() -> None:
async with engine.begin() as conn: 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) await conn.run_sync(Base.metadata.create_all)
async with async_session() as db: async with async_session() as db:

View file

@ -397,4 +397,3 @@ class TestProfilePrivacy:
assert "telegram" not in data assert "telegram" not in data
assert "signal" not in data assert "signal" not in data
assert "nostr_npub" not in data assert "nostr_npub" not in data

View file

@ -61,8 +61,8 @@ class TestValidateTelegram:
assert validate_telegram("@alice_bob") is None assert validate_telegram("@alice_bob") is None
def test_valid_handle_min_length(self): def test_valid_handle_min_length(self):
# 5 characters after @ # 1 character after @
assert validate_telegram("@abcde") is None assert validate_telegram("@a") is None
def test_valid_handle_max_length(self): def test_valid_handle_max_length(self):
# 32 characters after @ # 32 characters after @
@ -77,26 +77,20 @@ class TestValidateTelegram:
result = validate_telegram("@") result = validate_telegram("@")
assert result is not None 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): def test_too_long(self):
# More than 32 characters after @ # More than 32 characters after @
result = validate_telegram("@" + "a" * 33) result = validate_telegram("@" + "a" * 33)
assert result is not None assert result is not None
assert "32" in result assert "32" in result
def test_starts_with_number(self): def test_starts_with_number_is_valid(self):
result = validate_telegram("@1alice") # Now allowed - any character is valid
assert result is not None assert validate_telegram("@1alice") is None
assert "letter" in result.lower()
def test_invalid_characters(self): def test_special_characters_are_valid(self):
result = validate_telegram("@alice-bob") # Now allowed - any character is valid
assert result is not None assert validate_telegram("@alice-bob") is None
assert validate_telegram("@test.user") is None
class TestValidateSignal: class TestValidateSignal:

View file

@ -1,5 +1,4 @@
"""Validation utilities for user profile fields.""" """Validation utilities for user profile fields."""
import re
from email_validator import validate_email, EmailNotValidError from email_validator import validate_email, EmailNotValidError
from bech32 import bech32_decode from bech32 import bech32_decode
@ -25,7 +24,7 @@ def validate_telegram(value: str | None) -> str | None:
""" """
Validate Telegram handle. 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. Returns None if valid, error message if invalid.
Empty/None values are valid (field is optional). Empty/None values are valid (field is optional).
""" """
@ -35,21 +34,13 @@ def validate_telegram(value: str | None) -> str | None:
if not value.startswith("@"): if not value.startswith("@"):
return "Telegram handle must start with @" 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 @ handle = value[1:] # Remove @
if len(handle) < 5: if len(handle) < 1:
return "Telegram handle must be at least 5 characters (after @)" return "Telegram handle must have at least one character after @"
if len(handle) > 32: if len(handle) > 32:
return "Telegram handle must be at most 32 characters (after @)" 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 return None

View file

@ -2,6 +2,7 @@
import { useEffect, useState, useCallback } from "react"; import { useEffect, useState, useCallback } from "react";
import { useRouter } from "next/navigation"; import { useRouter } from "next/navigation";
import { bech32 } from "bech32";
import { useAuth } from "../auth-context"; import { useAuth } from "../auth-context";
import { API_URL } from "../config"; import { API_URL } from "../config";
import { sharedStyles } from "../styles/shared"; import { sharedStyles } from "../styles/shared";
@ -30,7 +31,9 @@ interface FieldErrors {
// Client-side validation matching backend rules // Client-side validation matching backend rules
function validateEmail(value: string): string | undefined { function validateEmail(value: string): string | undefined {
if (!value) return 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)) { if (!emailRegex.test(value)) {
return "Please enter a valid email address"; return "Please enter a valid email address";
} }
@ -43,15 +46,12 @@ function validateTelegram(value: string): string | undefined {
return "Telegram handle must start with @"; return "Telegram handle must start with @";
} }
const handle = value.slice(1); const handle = value.slice(1);
if (handle.length < 5) { if (handle.length < 1) {
return "Telegram handle must be at least 5 characters (after @)"; return "Telegram handle must have at least one character after @";
} }
if (handle.length > 32) { if (handle.length > 32) {
return "Telegram handle must be at most 32 characters (after @)"; 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; return undefined;
} }
@ -71,15 +71,21 @@ function validateNostrNpub(value: string): string | undefined {
if (!value.startsWith("npub1")) { if (!value.startsWith("npub1")) {
return "Nostr npub must start with 'npub1'"; return "Nostr npub must start with 'npub1'";
} }
// Basic length check (valid npubs are 63 characters)
if (value.length !== 63) { try {
return "Invalid Nostr npub format"; 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 { function validateForm(data: FormData): FieldErrors {
@ -138,21 +144,7 @@ export default function ProfilePage() {
} }
}, [isLoading, user, router, isRegularUser]); }, [isLoading, user, router, isRegularUser]);
useEffect(() => { const fetchProfile = useCallback(async () => {
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 () => {
try { try {
const res = await fetch(`${API_URL}/api/profile`, { const res = await fetch(`${API_URL}/api/profile`, {
credentials: "include", credentials: "include",
@ -167,13 +159,29 @@ export default function ProfilePage() {
}; };
setFormData(formValues); setFormData(formValues);
setOriginalData(formValues); setOriginalData(formValues);
} else {
setToast({ message: "Failed to load profile", type: "error" });
} }
} catch { } catch {
// Handle error silently for now setToast({ message: "Network error. Please try again.", type: "error" });
} finally { } finally {
setIsLoadingProfile(false); 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<HTMLInputElement>) => { const handleInputChange = (field: keyof FormData) => (e: React.ChangeEvent<HTMLInputElement>) => {
const value = e.target.value; const value = e.target.value;

View file

@ -207,7 +207,7 @@ test.describe("Profile - Form Behavior", () => {
await expect(page.getByText(/saved successfully/i)).toBeVisible(); await expect(page.getByText(/saved successfully/i)).toBeVisible();
// Wait for toast to disappear // Wait for toast to disappear
await page.waitForTimeout(3500); await expect(page.getByText(/saved successfully/i)).not.toBeVisible({ timeout: 5000 });
// Clear the field // Clear the field
await page.fill("#telegram", ""); await page.fill("#telegram", "");

View file

@ -8,6 +8,7 @@
"name": "frontend", "name": "frontend",
"version": "0.1.0", "version": "0.1.0",
"dependencies": { "dependencies": {
"bech32": "^2.0.0",
"next": "15.1.2", "next": "15.1.2",
"react": "19.0.0", "react": "19.0.0",
"react-dom": "19.0.0" "react-dom": "19.0.0"
@ -2092,6 +2093,12 @@
"baseline-browser-mapping": "dist/cli.js" "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": { "node_modules/browserslist": {
"version": "4.28.1", "version": "4.28.1",
"resolved": "https://registry.npmjs.org/browserslist/-/browserslist-4.28.1.tgz", "resolved": "https://registry.npmjs.org/browserslist/-/browserslist-4.28.1.tgz",

View file

@ -10,6 +10,7 @@
"test:e2e": "playwright test" "test:e2e": "playwright test"
}, },
"dependencies": { "dependencies": {
"bech32": "^2.0.0",
"next": "15.1.2", "next": "15.1.2",
"react": "19.0.0", "react": "19.0.0",
"react-dom": "19.0.0" "react-dom": "19.0.0"