From 7bfea306ab823543772d86b522f89e07b0e78e3c Mon Sep 17 00:00:00 2001
From: Johan Lundberg <lundberg@sunet.se>
Date: Fri, 13 Mar 2026 20:40:05 +0100
Subject: [PATCH] refactor: use shared ProfileUpdate validation in admin routes

---
 src/porchlight/admin/routes.py        | 52 +++++++++++++++++++--------
 tests/test_admin/test_admin_routes.py | 20 +++++++++--
 2 files changed, 56 insertions(+), 16 deletions(-)

diff --git a/src/porchlight/admin/routes.py b/src/porchlight/admin/routes.py
index 44283d6..acf4d0f 100644
--- a/src/porchlight/admin/routes.py
+++ b/src/porchlight/admin/routes.py
@@ -1,11 +1,12 @@
 from base64 import urlsafe_b64decode
-from urllib.parse import urlparse
 
 from fastapi import APIRouter, Form, Request, Response
 from fastapi.responses import HTMLResponse, RedirectResponse
+from pydantic import ValidationError
 
 from porchlight.dependencies import get_session_user
 from porchlight.models import User
+from porchlight.validation import ProfileUpdate
 
 router = APIRouter(prefix="/admin", tags=["admin"])
 
@@ -143,12 +144,35 @@ async def update_user_profile(
         return HTMLResponse("Forbidden", status_code=403)
 
     # Validate
-    if email and "@" not in email:
-        return HTMLResponse('<div role="alert">Invalid email address</div>')
-    if picture:
-        parsed = urlparse(picture)
-        if parsed.scheme not in ("http", "https") or not parsed.netloc:
-            return HTMLResponse('<div role="alert">Picture URL must be a valid HTTP or HTTPS URL</div>')
+    try:
+        profile = ProfileUpdate(
+            given_name=given_name,
+            family_name=family_name,
+            preferred_username=preferred_username,
+            email=email,
+            phone_number=phone_number,
+            picture=picture,
+            locale=locale,
+        )
+    except ValidationError as exc:
+        error = exc.errors()[0]
+        field = error["loc"][-1] if error["loc"] else "input"
+        msg = error["msg"]
+        labels = {
+            "given_name": "Given name",
+            "family_name": "Family name",
+            "preferred_username": "Display name",
+            "email": "Email",
+            "phone_number": "Phone number",
+            "picture": "Picture URL",
+            "locale": "Locale",
+        }
+        label = labels.get(str(field), str(field))
+        if error["type"] == "value_error":
+            display_msg = msg.removeprefix("Value error, ")
+        else:
+            display_msg = f"{label}: {msg}"
+        return HTMLResponse(f'<div role="alert">{display_msg}</div>')
 
     user_repo = request.app.state.user_repo
     user = await user_repo.get_by_userid(userid)
@@ -157,13 +181,13 @@ async def update_user_profile(
 
     updated = user.model_copy(
         update={
-            "given_name": given_name or None,
-            "family_name": family_name or None,
-            "preferred_username": preferred_username or None,
-            "email": email or None,
-            "phone_number": phone_number or None,
-            "picture": picture or None,
-            "locale": locale or None,
+            "given_name": profile.given_name or None,
+            "family_name": profile.family_name or None,
+            "preferred_username": profile.preferred_username or None,
+            "email": profile.email,
+            "phone_number": profile.phone_number,
+            "picture": profile.picture,
+            "locale": profile.locale or None,
         }
     )
     await user_repo.update(updated)
diff --git a/tests/test_admin/test_admin_routes.py b/tests/test_admin/test_admin_routes.py
index c4e83ed..b579a44 100644
--- a/tests/test_admin/test_admin_routes.py
+++ b/tests/test_admin/test_admin_routes.py
@@ -153,7 +153,7 @@ async def test_update_profile(client: AsyncClient) -> None:
             "family_name": "Smith",
             "preferred_username": "bobby",
             "email": "bob@example.com",
-            "phone_number": "+1234567890",
+            "phone_number": "+46701234567",
             "picture": "https://example.com/bob.jpg",
             "locale": "en-US",
         },
@@ -183,7 +183,23 @@ async def test_update_profile_invalid_email(client: AsyncClient) -> None:
         headers={"X-CSRF-Token": token},
     )
     assert response.status_code == 200
-    assert "Invalid email" in response.text
+    assert "alert" in response.text
+    assert "email" in response.text.lower()
+
+
+@pytest.mark.asyncio
+async def test_update_profile_invalid_phone(client: AsyncClient) -> None:
+    await _login(client)
+    target = await _create_target_user(client, username="bob")
+    token = await get_csrf_token(client)
+    response = await client.post(
+        f"/admin/users/{target.userid}/profile",
+        data={"phone_number": "not-a-phone"},
+        headers={"X-CSRF-Token": token},
+    )
+    assert response.status_code == 200
+    assert "alert" in response.text
+    assert "phone" in response.text.lower()
 
 
 @pytest.mark.asyncio