Merge pull request #497 from christianlouis/copilot/fix-user-sign-up-functionality

feat(auth): enable local user self-registration without SMTP + admin account creation
2026-03-07 21:44:24 +01:00
parent 79e76522e3 19c1ccb11c
commit 1883606a5e
10 changed files with 651 additions and 46 deletions
@@ -2,6 +2,8 @@

 Provides CRUD operations for user profiles and aggregate statistics so that
 administrators can inspect, configure, and manage users in multi-user mode.
+Also provides endpoints for admins to create and manage local (email/password)
+user accounts directly, without requiring email verification.
 """

 import logging
@@ -14,7 +16,8 @@ from sqlalchemy import func
 from sqlalchemy.orm import Session

 from app.database import get_db
-from app.models import FileRecord, UserProfile
+from app.models import FileRecord, LocalUser, UserProfile
+from app.utils.local_auth import hash_password

 logger = logging.getLogger(__name__)
 router = APIRouter(prefix="/admin/users", tags=["admin-users"])
@@ -97,6 +100,30 @@ class UserSummary(BaseModel):
    last_upload: str | None


+class LocalUserCreate(BaseModel):
+    """Body for admin-creating a local (email/password) user account."""
+
+    email: str = Field(..., max_length=255, description="Email address for the new user")
+    username: str = Field(..., min_length=3, max_length=64, pattern=r"^[a-zA-Z0-9_-]+$")
+    display_name: str | None = Field(default=None, max_length=255)
+    password: str = Field(..., min_length=8, max_length=128)
+    is_admin: bool = Field(default=False, description="Grant admin privileges")
+
+
+class LocalUserResponse(BaseModel):
+    """Summary of a local user account."""
+
+    id: int
+    email: str
+    username: str
+    display_name: str | None
+    is_active: bool
+    is_admin: bool
+    created_at: str | None
+
+    model_config = {"from_attributes": True}
+
+
 # ---------------------------------------------------------------------------
 # Helpers
 # ---------------------------------------------------------------------------
@@ -209,6 +236,110 @@ def list_users(
    }


+# ---------------------------------------------------------------------------
+# Local user management (admin-only)
+# ---------------------------------------------------------------------------
+# NOTE: These routes MUST be defined before /{user_id:path} to avoid being
+# swallowed by the catch-all path parameter.
+# ---------------------------------------------------------------------------
+
+
+@router.get("/local", summary="List all local (email/password) user accounts")
+def list_local_users(db: DbSession, _admin: AdminUser) -> list[dict[str, Any]]:
+    """Return every local user account with basic metadata."""
+    users = db.query(LocalUser).order_by(LocalUser.created_at.desc()).all()
+    return [
+        {
+            "id": u.id,
+            "email": u.email,
+            "username": u.username,
+            "display_name": u.display_name,
+            "is_active": u.is_active,
+            "is_admin": u.is_admin,
+            "created_at": u.created_at.isoformat() if u.created_at else None,
+        }
+        for u in users
+    ]
+
+
+@router.post("/local", status_code=status.HTTP_201_CREATED, summary="Create a local user account")
+def create_local_user(body: LocalUserCreate, db: DbSession, _admin: AdminUser) -> dict[str, Any]:
+    """Create a new local (email/password) user account.
+
+    The account is immediately active — no email verification is required when
+    created by an administrator.  A matching UserProfile row is also created.
+
+    Raises:
+        409: Email or username already registered.
+    """
+    if db.query(LocalUser).filter(LocalUser.email == body.email).first():
+        raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Email already registered.")
+    if db.query(LocalUser).filter(LocalUser.username == body.username).first():
+        raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Username already taken.")
+
+    user = LocalUser(
+        email=body.email,
+        username=body.username,
+        display_name=body.display_name,
+        hashed_password=hash_password(body.password),
+        is_active=True,
+        is_admin=body.is_admin,
+    )
+    db.add(user)
+
+    # Ensure a UserProfile exists for the new user
+    if not db.query(UserProfile).filter(UserProfile.user_id == body.email).first():
+        db.add(UserProfile(user_id=body.email, display_name=body.display_name or body.username))
+
+    try:
+        db.commit()
+        db.refresh(user)
+    except Exception:
+        db.rollback()
+        raise
+
+    logger.info("Admin created local user account: %s", body.email)
+    return {
+        "id": user.id,
+        "email": user.email,
+        "username": user.username,
+        "display_name": user.display_name,
+        "is_active": user.is_active,
+        "is_admin": user.is_admin,
+        "created_at": user.created_at.isoformat() if user.created_at else None,
+    }
+
+
+@router.delete(
+    "/local/{local_user_id}",
+    status_code=status.HTTP_204_NO_CONTENT,
+    summary="Delete a local user account",
+)
+def delete_local_user(local_user_id: int, db: DbSession, _admin: AdminUser) -> None:
+    """Delete a local user account by its numeric ID.
+
+    The associated UserProfile is also removed.  Documents owned by this user
+    are **not** deleted.
+    """
+    user = db.query(LocalUser).filter(LocalUser.id == local_user_id).first()
+    if not user:
+        raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Local user not found.")
+
+    # Remove associated profile if present
+    profile = db.query(UserProfile).filter(UserProfile.user_id == user.email).first()
+    if profile:
+        db.delete(profile)
+
+    try:
+        db.delete(user)
+        db.commit()
+    except Exception:
+        db.rollback()
+        raise
+
+    logger.info("Admin deleted local user account: %s", user.email)
+
+
@router.get("/{user_id:path}", summary="Get details for a single user")
 def get_user(user_id: str, db: DbSession, _admin: AdminUser) -> dict[str, Any]:
    """Return profile and document statistics for a specific user."""
@@ -128,15 +128,18 @@ async def reset_password_page(request: Request) -> Any:


@router.post("/api/auth/signup", status_code=status.HTTP_201_CREATED)
-async def signup(request: Request, body: SignupBody, db: DbSession) -> dict[str, str]:
-    """Create a new local user account and send a verification email.
+async def signup(request: Request, body: SignupBody, db: DbSession) -> dict[str, str | bool]:
+    """Create a new local user account.
+
+    When SMTP is configured the account is inactive until the user clicks the
+    verification link sent to their email.  When SMTP is **not** configured the
+    account is activated immediately so that deployments without email can still
+    use the self-registration flow.

-    The account is inactive until the user clicks the email link.
    Both ``MULTI_USER_ENABLED`` and ``ALLOW_LOCAL_SIGNUP`` must be ``True``.

    Raises:
        403: Multi-user mode or local signup is disabled.
-        503: SMTP is not configured.
        422: Passwords do not match.
        409: Email or username already registered.
    """
@@ -144,11 +147,6 @@ async def signup(request: Request, body: SignupBody, db: DbSession) -> dict[str,
        raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Multi-user mode is not enabled.")
    if not settings.allow_local_signup:
        raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Registration is not enabled.")
-    if not settings.email_host:
-        raise HTTPException(
-            status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
-            detail="Email (SMTP) must be configured before local signup can be enabled.",
-        )
    if body.password != body.password_confirm:
        raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail="Passwords do not match.")

@@ -157,16 +155,30 @@ async def signup(request: Request, body: SignupBody, db: DbSession) -> dict[str,
    if db.query(LocalUser).filter(LocalUser.username == body.username).first():
        raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Username already taken.")

-    token = generate_token()
-    user = LocalUser(
-        email=body.email,
-        username=body.username,
-        display_name=body.display_name,
-        hashed_password=hash_password(body.password),
-        is_active=False,
-        email_verification_token=token,
-        email_verification_sent_at=datetime.now(tz=timezone.utc),
-    )
+    smtp_configured = bool(settings.email_host)
+
+    if smtp_configured:
+        token = generate_token()
+        user = LocalUser(
+            email=body.email,
+            username=body.username,
+            display_name=body.display_name,
+            hashed_password=hash_password(body.password),
+            is_active=False,
+            email_verification_token=token,
+            email_verification_sent_at=datetime.now(tz=timezone.utc),
+        )
+    else:
+        # No SMTP configured — activate the account immediately.
+        token = None
+        user = LocalUser(
+            email=body.email,
+            username=body.username,
+            display_name=body.display_name,
+            hashed_password=hash_password(body.password),
+            is_active=True,
+        )
+
    db.add(user)

    profile = UserProfile(
@@ -185,22 +197,28 @@ async def signup(request: Request, body: SignupBody, db: DbSession) -> dict[str,
        db.rollback()
        raise

-    base_url = str(request.base_url).rstrip("/")
-    try:
-        send_verification_email(body.email, body.username, token, base_url)
-    except Exception as exc:
-        # Email failed — roll back so no unverifiable user row persists.
-        # The user can simply try registering again once SMTP is fixed.
-        db.rollback()
-        logger.warning("Signup email failed for %s: %s", body.email, exc)
-        raise HTTPException(
-            status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
-            detail=("Failed to send verification email. Please check that SMTP is correctly configured and try again."),
-        ) from exc
+    if smtp_configured and token:
+        base_url = str(request.base_url).rstrip("/")
+        try:
+            send_verification_email(body.email, body.username, token, base_url)
+        except Exception as exc:
+            # Email failed — roll back so no unverifiable user row persists.
+            # The user can simply try registering again once SMTP is fixed.
+            db.rollback()
+            logger.warning("Signup email failed for %s: %s", body.email, exc)
+            raise HTTPException(
+                status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
+                detail=(
+                    "Failed to send verification email. Please check that SMTP is correctly configured and try again."
+                ),
+            ) from exc

    db.commit()
    logger.info("New local user registered: %s", body.email)
-    return {"message": "Verification email sent. Please check your inbox."}
+
+    if smtp_configured:
+        return {"message": "Verification email sent. Please check your inbox.", "email_verification_required": True}
+    return {"message": "Account created successfully. You can now log in.", "email_verification_required": False}


@router.get("/verify-email", include_in_schema=False)
@@ -32,6 +32,10 @@ def _inject_global_context(ctx: dict) -> None:
    ctx.setdefault("ui_default_color_scheme", getattr(settings, "ui_default_color_scheme", "system"))
    ctx.setdefault("multi_user_enabled", getattr(settings, "multi_user_enabled", False))
    ctx.setdefault("auth_enabled", getattr(settings, "auth_enabled", True))
+    ctx.setdefault(
+        "allow_signup",
+        getattr(settings, "multi_user_enabled", False) and getattr(settings, "allow_local_signup", False),
+    )

    req = ctx.get("request")
    if req is not None: