Make recovery verify/reset endpoints factor-aware (phone recovery over HTTP)

Author: igorbenav

crudauth/email/router.py

@@ -1,13 +1,17 @@
-"""Builds the email-flow endpoints (verify / reset / change).
-
-The three *trigger* endpoints carry a per-IP rate-limit dependency (caller
-spray); the service additionally enforces a silent per-target-email cap.
+"""Builds the recovery-flow endpoints (verify / reset / change).
+
+The trigger endpoints carry a per-IP rate-limit dependency (caller spray); the
+service additionally enforces a silent per-target cap. The verify and reset
+request bodies are shaped to the contract's recovery factor (``email`` validated
+as an address, any other factor as a plain string), so a phone-recovery app can
+drive them over HTTP with a phone number. Change-email is email-specific and only
+mounts when the model actually has an email column.
 """
 
 from typing import Annotated, Any
 
 from fastapi import APIRouter, Depends
-from pydantic import BaseModel, EmailStr, Field
+from pydantic import BaseModel, EmailStr, Field, create_model
 
 from ..constants import MIN_PASSWORD_LENGTH
 from ..principal import Principal
@@ -17,10 +21,6 @@
 __all__ = ["build_email_router"]
 
 
-class _EmailIn(BaseModel):
-    email: EmailStr
-
-
 class _TokenIn(BaseModel):
     token: str
 
@@ -36,67 +36,88 @@ class _ChangeIn(BaseModel):
 
 
 def build_email_router(*, auth: Any, service: EmailFlowService) -> APIRouter:
-    """Build the email-flow router (verify / reset / change endpoints).
+    """Build the recovery-flow router (verify / reset, plus change-email when applicable).
+
+    The verify and reset request bodies are generated for the recovery factor: an
+    email-recovery app keeps ``{"email": ...}`` (validated as an address), a
+    phone-recovery app gets ``{"phone": ...}``. Change-email endpoints are added
+    only when the model has an ``email`` column, since they prove a real address.
 
     Args:
         auth: The owning [CRUDAuth][crudauth.crud_auth.CRUDAuth] (for ``session``,
             ``current_user``, and ``rate_limit`` dependencies).
         service: The [EmailFlowService][crudauth.email.service.EmailFlowService] that mints/verifies tokens.
 
     Returns:
-        An `APIRouter` with the six email endpoints.
+        An `APIRouter` with the recovery endpoints.
     """
     router = APIRouter(tags=["auth:email"])
     db_dep = auth.session
     user_dep = auth.current_user()
 
+    factor = service.repo.recovery
+    if factor is None:
+        raise RuntimeError("the recovery router requires a recovery factor (identity.recovery)")
+    field_type: Any = EmailStr if factor == "email" else str
+    fields: dict[str, Any] = {factor: (field_type, ...)}
+    RecoveryRequestModel = create_model("RecoveryRequestIn", **fields)
+    channel_noun = "email" if factor == "email" else "message"
+
     @router.post(
         "/email/verify-request",
         dependencies=[Depends(auth.rate_limit("email_verify_request", key=KeyBy.IP))],
     )
-    async def request_verification(body: _EmailIn, db: Annotated[Any, Depends(db_dep)]):
-        """Send an email-verification link. Always returns success (no enumeration)."""
-        await service.request_recovery_verification(db, body.email)
-        return {"detail": "If an account exists, a verification email has been sent."}
+    async def request_verification(
+        body: RecoveryRequestModel,  # type: ignore[valid-type]
+        db: Annotated[Any, Depends(db_dep)],
+    ):
+        """Send a verification link to the recovery factor. Always succeeds (no enumeration)."""
+        await service.request_recovery_verification(db, getattr(body, factor))
+        return {"detail": f"If an account exists, a verification {channel_noun} has been sent."}
 
     @router.post("/email/verify-confirm")
     async def confirm_verification(body: _TokenIn, db: Annotated[Any, Depends(db_dep)]):
-        """Confirm a verification token and mark the email verified."""
+        """Confirm a verification token and mark the recovery factor verified."""
         await service.confirm_recovery_verification(db, body.token)
-        return {"detail": "Email verified successfully."}
+        return {"detail": "Verified successfully."}
 
     @router.post(
         "/password/reset-request",
         dependencies=[Depends(auth.rate_limit("password_reset_request", key=KeyBy.IP))],
     )
-    async def request_reset(body: _EmailIn, db: Annotated[Any, Depends(db_dep)]):
-        """Send a password-reset link. Always returns success (no enumeration)."""
-        await service.request_password_reset(db, body.email)
-        return {"detail": "If an account exists, a password reset email has been sent."}
+    async def request_reset(
+        body: RecoveryRequestModel,  # type: ignore[valid-type]
+        db: Annotated[Any, Depends(db_dep)],
+    ):
+        """Send a password-reset link to the recovery factor. Always succeeds (no enumeration)."""
+        await service.request_password_reset(db, getattr(body, factor))
+        return {"detail": f"If an account exists, a password reset {channel_noun} has been sent."}
 
     @router.post("/password/reset-confirm")
     async def reset(body: _ResetIn, db: Annotated[Any, Depends(db_dep)]):
         """Reset the password from a valid token and evict the user's other sessions."""
         await service.reset_password(db, body.token, body.new_password)
         return {"detail": "Password reset successfully."}
 
-    @router.post(
-        "/email/change-request",
-        dependencies=[Depends(auth.rate_limit("email_change_request", key=KeyBy.IP))],
-    )
-    async def change_request(
-        body: _ChangeIn,
-        db: Annotated[Any, Depends(db_dep)],
-        principal: Annotated[Principal, Depends(user_dep)],
-    ):
-        """Request an email change (authenticated; re-auth via current password)."""
-        await service.request_email_change(db, principal.user, body.new_email, body.password)
-        return {"detail": "If the address is available, a confirmation email has been sent."}
-
-    @router.post("/email/change-confirm")
-    async def change_confirm(body: _TokenIn, db: Annotated[Any, Depends(db_dep)]):
-        """Confirm an email-change token and apply the new address."""
-        await service.confirm_email_change(db, body.token)
-        return {"detail": "Email changed successfully."}
+    if service.repo.has("email"):
+
+        @router.post(
+            "/email/change-request",
+            dependencies=[Depends(auth.rate_limit("email_change_request", key=KeyBy.IP))],
+        )
+        async def change_request(
+            body: _ChangeIn,
+            db: Annotated[Any, Depends(db_dep)],
+            principal: Annotated[Principal, Depends(user_dep)],
+        ):
+            """Request an email change (authenticated; re-auth via current password)."""
+            await service.request_email_change(db, principal.user, body.new_email, body.password)
+            return {"detail": "If the address is available, a confirmation email has been sent."}
+
+        @router.post("/email/change-confirm")
+        async def change_confirm(body: _TokenIn, db: Annotated[Any, Depends(db_dep)]):
+            """Confirm an email-change token and apply the new address."""
+            await service.confirm_email_change(db, body.token)
+            return {"detail": "Email changed successfully."}
 
     return router

tests/test_recovery_verification.py

@@ -272,3 +272,38 @@ async def test_phone_verify_requires_the_delivered_token() -> None:
                     db, token
                 )  # replay rejected
             assert repo.recovery_verified(await repo.get_by_field(db, "phone", "777")) is True
+
+
+async def test_phone_verify_and_reset_work_over_http() -> None:
+    # the closed seam: the built-in request endpoints accept the recovery factor
+    # (phone) over HTTP, not an email, so phone recovery is end-to-end over HTTP.
+    async with _phone_app() as (auth, client, maker, channel):
+        async with maker() as db:
+            await auth.repo.create(
+                db, {"username": "neo", "phone": "555", "hashed_password": get_password_hash("pw")}
+            )
+
+        r = await client.post("/email/verify-request", json={"phone": "555"})
+        assert r.status_code == 200
+        assert channel.intents[-1].recipient == "555"
+        assert channel.intents[-1].kind == "verify_recovery"
+
+        r = await client.post("/password/reset-request", json={"phone": "555"})
+        assert r.status_code == 200
+        assert channel.intents[-1].recipient == "555"
+        assert channel.intents[-1].kind == "reset_password"
+
+
+async def test_phone_request_body_is_phone_shaped_not_email() -> None:
+    # the request body is shaped to the factor: a phone app's verify-request wants
+    # "phone", so posting "email" is a 422 (missing required field).
+    async with _phone_app() as (auth, client, _maker, _channel):
+        r = await client.post("/email/verify-request", json={"email": "a@x.com"})
+        assert r.status_code == 422
+
+
+async def test_email_less_phone_app_mounts_no_change_email() -> None:
+    # change-email proves a real address; an email-less phone app doesn't mount it.
+    async with _phone_app() as (auth, client, _maker, _channel):
+        r = await client.post("/email/change-request", json={"new_email": "a@x.com", "password": "pw"})
+        assert r.status_code == 404