P0 security batch: admin-only user mgmt, unified role model, login rate limiting, webhook secret, security headers, pagination caps
- Remove POST /api/auth/register (404); no sign-up UI; users are admin-managed - Add admin-only POST/PATCH/DELETE /api/auth/users (forced canonical roles, self-lockout + reference guards) - Unify role model in app/core/roles.py; reject unknown roles at creation and at login/JWT validation; startup normalizes unambiguous legacy aliases - Login rate limiting ~5 fails/15 min per IP+email -> 429 (in-process, tunable) - WhatsApp webhook requires X-Webhook-Secret; fail-closed when env unset; GET handshake uses constant-time verify token (403 on mismatch) - GET /api/whatsapp/mock-log now requires auth - Security headers middleware: X-Frame-Options DENY, nosniff, CSP on HTML, HSTS behind TLS - Pagination: limit alias for page_size, hard cap enforced, both -> 422
This commit is contained in:
root
+139
-38
@@ -1,11 +1,16 @@
|
||||
"""Authentication service — register, login, refresh."""
|
||||
"""Authentication service — login, refresh, and admin user management.
|
||||
|
||||
Self-registration was removed (HARDENING/P0 batch): users are created and
|
||||
managed exclusively by admins through the admin user-management endpoints.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from fastapi import HTTPException, status
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy import func, select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.core.roles import is_known_role, normalize_role
|
||||
from app.core.security import (
|
||||
create_access_token,
|
||||
create_refresh_token,
|
||||
@@ -13,47 +18,31 @@ from app.core.security import (
|
||||
hash_password,
|
||||
verify_password,
|
||||
)
|
||||
from app.models.ticket import Escalation, Ticket, TicketTimeline
|
||||
from app.models.user import User
|
||||
from app.schemas.auth import RegisterRequest
|
||||
from app.schemas.auth import AdminCreateUserRequest, AdminUpdateUserRequest
|
||||
|
||||
# HARDENING.md P0.3 — least-privilege default for self-registered users.
|
||||
_SELF_REGISTER_ROLE = "CS Rep"
|
||||
|
||||
|
||||
async def register(db: AsyncSession, body: RegisterRequest) -> User:
|
||||
"""Create a new user. Raises 409 if email already exists.
|
||||
|
||||
HARDENING.md P0.3: unauthenticated self-registration must never mint a
|
||||
privileged role. The client-supplied ``role`` field is IGNORED — new
|
||||
self-registered users always land on the least-privilege role.
|
||||
Admins assign elevated roles directly (DB seed / admin endpoint).
|
||||
"""
|
||||
result = await db.execute(select(User).where(User.email == body.email))
|
||||
if result.scalar_one_or_none():
|
||||
raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Email already registered")
|
||||
|
||||
user = User(
|
||||
email=body.email,
|
||||
password_hash=hash_password(body.password),
|
||||
full_name=body.full_name,
|
||||
phone=body.phone,
|
||||
role=_SELF_REGISTER_ROLE,
|
||||
)
|
||||
db.add(user)
|
||||
await db.flush()
|
||||
await db.refresh(user)
|
||||
return user
|
||||
_UNAUTHORIZED = status.HTTP_401_UNAUTHORIZED
|
||||
|
||||
|
||||
# ── AuthN ────────────────────────────────────────────────────────────
|
||||
async def login(db: AsyncSession, email: str, password: str) -> tuple[str, str, User]:
|
||||
"""Authenticate and return (access_token, refresh_token, user)."""
|
||||
result = await db.execute(select(User).where(User.email == email))
|
||||
"""Authenticate and return (access_token, refresh_token, user).
|
||||
|
||||
Fails closed (401) for bad credentials, inactive accounts, and any user
|
||||
whose stored role is not part of the unified role model.
|
||||
"""
|
||||
result = await db.execute(select(User).where(User.email == email.strip().lower()))
|
||||
user = result.scalar_one_or_none()
|
||||
if user is None or not verify_password(password, user.password_hash):
|
||||
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid email or password")
|
||||
raise HTTPException(status_code=_UNAUTHORIZED, detail="Invalid email or password")
|
||||
if not user.active:
|
||||
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Account is inactive")
|
||||
|
||||
raise HTTPException(status_code=_UNAUTHORIZED, detail="Account is inactive")
|
||||
if not is_known_role(user.role):
|
||||
raise HTTPException(
|
||||
status_code=_UNAUTHORIZED,
|
||||
detail="Account role is not recognised; contact an administrator",
|
||||
)
|
||||
access_token = create_access_token({"sub": str(user.id)})
|
||||
refresh_token = create_refresh_token({"sub": str(user.id)})
|
||||
return access_token, refresh_token, user
|
||||
@@ -64,18 +53,130 @@ async def refresh_access_token(db: AsyncSession, token: str) -> tuple[str, str]:
|
||||
try:
|
||||
payload = decode_token(token)
|
||||
if payload.get("type") != "refresh":
|
||||
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token type")
|
||||
raise HTTPException(status_code=_UNAUTHORIZED, detail="Invalid token type")
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception:
|
||||
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid refresh token")
|
||||
raise HTTPException(status_code=_UNAUTHORIZED, detail="Invalid refresh token")
|
||||
|
||||
user_id: int = int(payload["sub"])
|
||||
result = await db.execute(select(User).where(User.id == user_id))
|
||||
user = result.scalar_one_or_none()
|
||||
if user is None or not user.active:
|
||||
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="User not found or inactive")
|
||||
raise HTTPException(status_code=_UNAUTHORIZED, detail="User not found or inactive")
|
||||
if not is_known_role(user.role):
|
||||
raise HTTPException(
|
||||
status_code=_UNAUTHORIZED,
|
||||
detail="Account role is not recognised; contact an administrator",
|
||||
)
|
||||
|
||||
new_access = create_access_token({"sub": str(user.id)})
|
||||
new_refresh = create_refresh_token({"sub": str(user.id)})
|
||||
return new_access, new_refresh
|
||||
|
||||
|
||||
# ── Admin user management ────────────────────────────────────────────
|
||||
async def _get_user_or_404(db: AsyncSession, user_id: int) -> User:
|
||||
result = await db.execute(select(User).where(User.id == user_id))
|
||||
user = result.scalar_one_or_none()
|
||||
if user is None:
|
||||
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="User not found")
|
||||
return user
|
||||
|
||||
|
||||
async def create_user(db: AsyncSession, body: AdminCreateUserRequest) -> User:
|
||||
"""Admin-created user with an explicit, canonical role. 409 on duplicate email."""
|
||||
result = await db.execute(select(User).where(User.email == body.email))
|
||||
if result.scalar_one_or_none():
|
||||
raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Email already registered")
|
||||
|
||||
# Defense in depth: schema already guarantees a canonical role.
|
||||
role = normalize_role(body.role)
|
||||
if role is None:
|
||||
raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Unknown role")
|
||||
|
||||
user = User(
|
||||
email=body.email,
|
||||
password_hash=hash_password(body.password),
|
||||
full_name=body.full_name.strip(),
|
||||
phone=body.phone,
|
||||
role=role,
|
||||
)
|
||||
db.add(user)
|
||||
await db.flush()
|
||||
await db.refresh(user)
|
||||
return user
|
||||
|
||||
|
||||
async def update_user(
|
||||
db: AsyncSession,
|
||||
actor: User,
|
||||
user_id: int,
|
||||
body: AdminUpdateUserRequest,
|
||||
) -> User:
|
||||
"""Admin role-change / deactivation for an existing user.
|
||||
|
||||
Guards:
|
||||
* an admin cannot modify their own account through the API (self-lockout);
|
||||
* role changes are limited to the canonical taxonomy.
|
||||
|
||||
The ≥1-active-admin invariant holds structurally: only admins can demote
|
||||
admins, and no admin can demote/deactivate themselves, so at least one
|
||||
canonical admin always remains.
|
||||
"""
|
||||
user = await _get_user_or_404(db, user_id)
|
||||
|
||||
if actor.id == user.id:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail="Admins cannot change their own role or active state through the API",
|
||||
)
|
||||
|
||||
new_role = normalize_role(body.role) if body.role is not None else None
|
||||
new_active = body.active
|
||||
|
||||
if new_role is not None:
|
||||
user.role = new_role
|
||||
if new_active is not None:
|
||||
user.active = new_active
|
||||
await db.flush()
|
||||
await db.refresh(user)
|
||||
return user
|
||||
|
||||
|
||||
async def delete_user(db: AsyncSession, actor: User, user_id: int) -> None:
|
||||
"""Admin deletes a user account (hard delete).
|
||||
|
||||
Guards:
|
||||
* an admin cannot delete their own account (self-guard also keeps the
|
||||
≥1-active-admin invariant: admins can never remove themselves);
|
||||
* users referenced by tickets / timeline / escalations are kept (409) so
|
||||
historical data never dangles — reassign or deactivate instead.
|
||||
"""
|
||||
user = await _get_user_or_404(db, user_id)
|
||||
|
||||
if actor.id == user.id:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail="Admins cannot delete their own account through the API",
|
||||
)
|
||||
|
||||
referenced = False
|
||||
for clause in (
|
||||
select(func.count(Ticket.id)).where(Ticket.assigned_to == user_id),
|
||||
select(func.count(TicketTimeline.id)).where(TicketTimeline.user_id == user_id),
|
||||
select(func.count(Escalation.id)).where(Escalation.escalated_to == user_id),
|
||||
):
|
||||
count = (await db.execute(clause)).scalar() or 0
|
||||
if count:
|
||||
referenced = True
|
||||
break
|
||||
if referenced:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
detail="User has related tickets, timeline entries, or escalations; "
|
||||
"reassign or deactivate instead of deleting",
|
||||
)
|
||||
|
||||
await db.delete(user)
|
||||
await db.flush()
|
||||
|
||||
@@ -13,6 +13,7 @@ from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.core.security import hash_password
|
||||
from app.core.roles import is_blocked_legacy_role, normalize_role
|
||||
from app.models.unit import Unit
|
||||
from app.models.user import User
|
||||
from app.models.category import Category
|
||||
@@ -41,6 +42,37 @@ SEED_USERS_DATA = [
|
||||
]
|
||||
|
||||
|
||||
async def normalize_legacy_user_roles(db: AsyncSession) -> int:
|
||||
"""Converge legacy role strings onto the unified canonical taxonomy.
|
||||
|
||||
Databases built before the P0 role-model batch can hold nickname roles
|
||||
(``technician``, ``cs``, ``fm``, ``ceo`` …) minted by the old open
|
||||
self-registration. Unambiguous aliases are rewritten to their canonical
|
||||
role so RBAC keeps working. Ambiguous/unknown roles (e.g. lowercase
|
||||
``admin``/``superadmin``) are NOT auto-mapped — they fail closed at login
|
||||
and JWT validation until an operator remediates the row.
|
||||
|
||||
Returns the number of rows rewritten. Idempotent.
|
||||
"""
|
||||
result = await db.execute(select(User))
|
||||
changed = 0
|
||||
for user in result.scalars().all():
|
||||
canonical = normalize_role(user.role)
|
||||
if canonical and canonical != user.role:
|
||||
logger.info("Normalizing legacy role %r → %r for %s", user.role, canonical, user.email)
|
||||
user.role = canonical
|
||||
changed += 1
|
||||
elif canonical is None and not is_blocked_legacy_role(user.role):
|
||||
logger.warning(
|
||||
"User %s has unrecognized role %r; login will be denied until fixed",
|
||||
user.email,
|
||||
user.role,
|
||||
)
|
||||
if changed:
|
||||
await db.flush()
|
||||
return changed
|
||||
|
||||
|
||||
async def seed_users(db: AsyncSession, default_password: str = "denya123") -> list[User]:
|
||||
"""Insert seed users if they don't already exist."""
|
||||
hashed = hash_password(default_password)
|
||||
|
||||
Reference in New Issue
Block a user