From 46f130150a4777d4ecaf0895ddac8d61f4d1ebf4 Mon Sep 17 00:00:00 2001 From: Shihaam Abdul Rahman Date: Sat, 1 Aug 2026 14:02:44 +0500 Subject: [PATCH] softdelete users and migration script --- backend/app/models.py | 13 +++- backend/app/routers/client.py | 77 +++++++++++++------ backend/migrate.py | 135 ++++++++++++++++++++++++++++++++++ backend/radadmin_schema.sql | 32 +++++++- 4 files changed, 233 insertions(+), 24 deletions(-) create mode 100644 backend/migrate.py diff --git a/backend/app/models.py b/backend/app/models.py index e9dba99..0c2ec8a 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -84,13 +84,24 @@ class RadadminClient(Base): __tablename__ = "radadmin_clients" id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True) - mac_address: Mapped[str] = mapped_column(String(17), nullable=False, unique=True) + # Not DB-unique on its own — uniqueness is enforced only among live rows by the + # generated `active_mac` column (see radadmin_schema.sql). Soft-deleted rows + # (deleted_at set) may share a mac_address with the live row and each other. + mac_address: Mapped[str] = mapped_column(String(17), nullable=False) name: Mapped[str | None] = mapped_column(String(128), nullable=True) phone: Mapped[str | None] = mapped_column(String(32), nullable=True) alias: Mapped[str | None] = mapped_column(String(64), nullable=True) + # Mirror of the client's group (radusergroup) and billing status (customers): + # set on add, synced on edit, and kept frozen on the row after soft-delete so + # the deleted client's group/status survive their hard-deleted source rows. + groupname: Mapped[str | None] = mapped_column(String(64), nullable=True, default=None) + status: Mapped[str | None] = mapped_column(String(10), nullable=True, default=None) created_at: Mapped[datetime | None] = mapped_column( DateTime, nullable=True, server_default=func.current_timestamp() ) + # Soft delete: NULL = live, a timestamp = deleted. Deleted rows are never + # returned by the API and are kept only for restore/reference. + deleted_at: Mapped[datetime | None] = mapped_column(DateTime, nullable=True, default=None) class RadadminDevice(Base): diff --git a/backend/app/routers/client.py b/backend/app/routers/client.py index 6b8ea46..e2a3d0e 100644 --- a/backend/app/routers/client.py +++ b/backend/app/routers/client.py @@ -11,7 +11,7 @@ This router hides that fan-out behind mac_address + group + status. """ from fastapi import APIRouter, Depends from pydantic import ValidationError -from sqlalchemy import delete, func, or_, select, update +from sqlalchemy import and_, delete, func, or_, select, update from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session @@ -48,10 +48,16 @@ def _stage_client(db: Session, cli: ClientCreate) -> None: Does not commit — the caller controls the transaction boundary. """ mac = cli.mac_address + status = "paid" db.add(RadCheck(username=mac, attribute="Cleartext-Password", op=":=", value=mac)) db.add(RadUserGroup(username=mac, groupname=cli.group, priority=1)) - db.add(Customer(username=mac, mac_address=mac, status="paid")) - db.add(RadadminClient(mac_address=mac, name=cli.name, phone=cli.phone, alias=cli.alias)) + db.add(Customer(username=mac, mac_address=mac, status=status)) + # radadmin_clients mirrors the client's group/status too, so the row is a full + # record that survives (frozen) after the RADIUS/billing rows are hard-deleted. + db.add(RadadminClient( + mac_address=mac, name=cli.name, phone=cli.phone, alias=cli.alias, + groupname=cli.group, status=status, + )) @router.get("/", response_model=Page[ClientOut]) @@ -73,13 +79,19 @@ def list_clients( RadadminClient.name, RadadminClient.phone, RadadminClient.alias, ) .outerjoin(RadUserGroup, RadUserGroup.username == Customer.username) - .outerjoin(RadadminClient, RadadminClient.mac_address == Customer.username) + .outerjoin( + RadadminClient, + and_(RadadminClient.mac_address == Customer.username, RadadminClient.deleted_at.is_(None)), + ) ) # Count over the same joins so filtered totals drive pagination correctly. count_stmt = ( select(func.count()) .select_from(Customer) - .outerjoin(RadadminClient, RadadminClient.mac_address == Customer.username) + .outerjoin( + RadadminClient, + and_(RadadminClient.mac_address == Customer.username, RadadminClient.deleted_at.is_(None)), + ) ) if search: @@ -114,7 +126,10 @@ def get_client(mac_address: str, db: Session = Depends(get_db)): RadadminClient.name, RadadminClient.phone, RadadminClient.alias, ) .outerjoin(RadUserGroup, RadUserGroup.username == Customer.username) - .outerjoin(RadadminClient, RadadminClient.mac_address == Customer.username) + .outerjoin( + RadadminClient, + and_(RadadminClient.mac_address == Customer.username, RadadminClient.deleted_at.is_(None)), + ) .where(Customer.username == mac) ) row = db.execute(stmt).first() @@ -216,29 +231,35 @@ def edit_client(payload: ClientEdit, db: Session = Depends(get_db)): if customer is None: raise APIError(status_code=404, detail=f"Client '{mac}' not found") + # The live metadata row mirrors group/status/name/phone/alias; created at add, + # but create it here too in case a legacy client predates the mirror. + meta = db.execute( + select(RadadminClient).where( + RadadminClient.mac_address == mac, RadadminClient.deleted_at.is_(None) + ) + ).scalar_one_or_none() + if meta is None: + meta = RadadminClient(mac_address=mac, groupname=None, status=customer.status) + db.add(meta) + if payload.group is not None: if not _group_exists(db, payload.group): raise APIError(status_code=400, detail=f"Group '{payload.group}' not found") db.execute( update(RadUserGroup).where(RadUserGroup.username == mac).values(groupname=payload.group) ) + meta.groupname = payload.group if payload.status is not None: customer.status = payload.status + meta.status = payload.status - if payload.name is not None or payload.phone is not None or payload.alias is not None: - meta = db.execute( - select(RadadminClient).where(RadadminClient.mac_address == mac) - ).scalar_one_or_none() - if meta is None: - meta = RadadminClient(mac_address=mac) - db.add(meta) - if payload.name is not None: - meta.name = payload.name - if payload.phone is not None: - meta.phone = payload.phone - if payload.alias is not None: - meta.alias = payload.alias + if payload.name is not None: + meta.name = payload.name + if payload.phone is not None: + meta.phone = payload.phone + if payload.alias is not None: + meta.alias = payload.alias db.commit() @@ -246,7 +267,9 @@ def edit_client(payload: ClientEdit, db: Session = Depends(get_db)): select(RadUserGroup.groupname).where(RadUserGroup.username == mac).limit(1) ).scalar_one_or_none() meta = db.execute( - select(RadadminClient).where(RadadminClient.mac_address == mac) + select(RadadminClient).where( + RadadminClient.mac_address == mac, RadadminClient.deleted_at.is_(None) + ) ).scalar_one_or_none() return ClientOut( mac_address=mac, group=group, status=customer.status, @@ -258,13 +281,23 @@ def edit_client(payload: ClientEdit, db: Session = Depends(get_db)): @router.delete("/{mac_address}", status_code=204) def delete_client(mac_address: str, db: Session = Depends(get_db)): - """Remove a client from radcheck, radusergroup and customers.""" + """Delete a client: hard-remove its radcheck, radusergroup and customers rows, + but only *soft* delete its radadmin_clients metadata (name/phone/alias) by + stamping deleted_at, so it can be restored/referenced later. Soft-deleted rows + are never returned by the API, and the client's MAC may be freely re-added.""" mac = mac_address.strip().upper().replace(":", "-") if not _client_exists(db, mac): raise APIError(status_code=404, detail=f"Client '{mac}' not found") + # radadmin_clients already mirrors group/status (kept in sync on add/edit), so + # deleting just hard-removes the RADIUS/billing rows and soft-deletes metadata; + # the mirrored group/status stay frozen on the row for reference. db.execute(delete(RadCheck).where(RadCheck.username == mac)) db.execute(delete(RadUserGroup).where(RadUserGroup.username == mac)) db.execute(delete(Customer).where(Customer.username == mac)) - db.execute(delete(RadadminClient).where(RadadminClient.mac_address == mac)) + db.execute( + update(RadadminClient) + .where(RadadminClient.mac_address == mac, RadadminClient.deleted_at.is_(None)) + .values(deleted_at=func.now()) + ) db.commit() diff --git a/backend/migrate.py b/backend/migrate.py new file mode 100644 index 0000000..6749ed5 --- /dev/null +++ b/backend/migrate.py @@ -0,0 +1,135 @@ +#!/usr/bin/env python3 +"""Idempotent schema migrations for the radadmin API. + +Applies incremental DDL that ``radadmin_schema.sql`` can't (it only CREATEs +tables IF NOT EXISTS, so it never alters an existing one). Each step checks +information_schema first, so this is safe to re-run and works on both MySQL and +MariaDB. Uses the same DB connection settings as the app (.env / config.py). + +Run from the backend/ directory: + + python migrate.py # apply pending migrations + python migrate.py --dry-run # show what would run, change nothing +""" +from __future__ import annotations + +import argparse +import sys + +from sqlalchemy import text + +from app.database import engine + +TABLE = "radadmin_clients" + + +def _column_exists(conn, table: str, column: str) -> bool: + row = conn.execute( + text( + "SELECT COUNT(*) FROM information_schema.columns " + "WHERE table_schema = DATABASE() AND table_name = :t AND column_name = :c" + ), + {"t": table, "c": column}, + ).scalar() + return bool(row) + + +def _index_exists(conn, table: str, index: str) -> bool: + row = conn.execute( + text( + "SELECT COUNT(*) FROM information_schema.statistics " + "WHERE table_schema = DATABASE() AND table_name = :t AND index_name = :i" + ), + {"t": table, "i": index}, + ).scalar() + return bool(row) + + +def _table_exists(conn, table: str) -> bool: + row = conn.execute( + text( + "SELECT COUNT(*) FROM information_schema.tables " + "WHERE table_schema = DATABASE() AND table_name = :t" + ), + {"t": table}, + ).scalar() + return bool(row) + + +# Each migration: (description, predicate that returns True when it still needs +# to run, SQL to run). Ordered — later steps may depend on earlier ones. +MIGRATIONS = [ + ( + "add radadmin_clients.groupname (mirror of radusergroup)", + lambda c: not _column_exists(c, TABLE, "groupname"), + f"ALTER TABLE {TABLE} ADD COLUMN groupname VARCHAR(64) NULL AFTER alias", + ), + ( + "add radadmin_clients.status (mirror of customers.status)", + lambda c: not _column_exists(c, TABLE, "status"), + f"ALTER TABLE {TABLE} ADD COLUMN status VARCHAR(10) NULL AFTER groupname", + ), + ( + "add radadmin_clients.deleted_at (soft-delete state)", + lambda c: not _column_exists(c, TABLE, "deleted_at"), + f"ALTER TABLE {TABLE} ADD COLUMN deleted_at DATETIME NULL DEFAULT NULL AFTER created_at", + ), + ( + "add radadmin_clients.active_mac (generated; NULL when soft-deleted)", + lambda c: not _column_exists(c, TABLE, "active_mac"), + f"ALTER TABLE {TABLE} ADD COLUMN active_mac VARCHAR(17) " + "AS (IF(deleted_at IS NULL, mac_address, NULL)) STORED", + ), + ( + "drop old unique index uq_radadmin_clients_mac", + lambda c: _index_exists(c, TABLE, "uq_radadmin_clients_mac"), + f"ALTER TABLE {TABLE} DROP INDEX uq_radadmin_clients_mac", + ), + ( + "add partial-unique index uq_radadmin_clients_active_mac", + lambda c: not _index_exists(c, TABLE, "uq_radadmin_clients_active_mac"), + f"ALTER TABLE {TABLE} ADD UNIQUE KEY uq_radadmin_clients_active_mac (active_mac)", + ), +] + + +def main() -> int: + parser = argparse.ArgumentParser(description="Apply radadmin schema migrations.") + parser.add_argument( + "--dry-run", action="store_true", help="show pending migrations without applying them" + ) + args = parser.parse_args() + + with engine.connect() as conn: + if not _table_exists(conn, TABLE): + print( + f"error: table '{TABLE}' not found — import radadmin_schema.sql first.", + file=sys.stderr, + ) + return 1 + + applied = 0 + for desc, needs_run, sql in MIGRATIONS: + if not needs_run(conn): + print(f" skip {desc} (already applied)") + continue + if args.dry_run: + print(f" PEND {desc}\n {sql}") + applied += 1 + continue + print(f" apply {desc}") + conn.execute(text(sql)) + conn.commit() # DDL auto-commits, but be explicit for the connection + applied += 1 + + if args.dry_run: + print(f"\n{applied} migration(s) pending. Re-run without --dry-run to apply.") + elif applied: + print(f"\nDone — {applied} migration(s) applied.") + else: + print("\nDatabase already up to date.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/backend/radadmin_schema.sql b/backend/radadmin_schema.sql index 7d88b1c..f53f81f 100644 --- a/backend/radadmin_schema.sql +++ b/backend/radadmin_schema.sql @@ -12,17 +12,47 @@ CREATE TABLE IF NOT EXISTS customers ( PRIMARY KEY (id) ); +-- Soft delete: deleting a client hard-removes its radcheck/radusergroup/customers +-- rows but only SETS deleted_at here, so the human metadata survives and can be +-- restored/referenced. deleted_at IS NULL means "live". Uniqueness on mac_address +-- is enforced only among live rows via the generated active_mac column (NULL for +-- soft-deleted rows — MySQL/MariaDB allows many NULLs in a UNIQUE index), so a +-- re-added client coexists with any number of old soft-deleted copies. +-- groupname/status mirror the client's current group (radusergroup) and billing +-- status (customers): populated on add and kept in sync on edit. They stay on the +-- row when the client is soft-deleted, so a deleted client's group/status aren't +-- lost along with its hard-deleted RADIUS/billing rows. CREATE TABLE IF NOT EXISTS radadmin_clients ( id INT UNSIGNED NOT NULL AUTO_INCREMENT, mac_address VARCHAR(17) NOT NULL, name VARCHAR(128) NULL, phone VARCHAR(32) NULL, alias VARCHAR(64) NULL, + groupname VARCHAR(64) NULL, + status VARCHAR(10) NULL, created_at DATETIME NULL DEFAULT CURRENT_TIMESTAMP, + deleted_at DATETIME NULL DEFAULT NULL, + active_mac VARCHAR(17) AS (IF(deleted_at IS NULL, mac_address, NULL)) STORED, PRIMARY KEY (id), - UNIQUE KEY uq_radadmin_clients_mac (mac_address) + UNIQUE KEY uq_radadmin_clients_active_mac (active_mac) ); +-- Soft-delete migration for radadmin_clients tables created before this change +-- (MariaDB syntax; idempotent, and a no-op on the fresh schema created above). +ALTER TABLE radadmin_clients + ADD COLUMN IF NOT EXISTS groupname VARCHAR(64) NULL AFTER alias; +ALTER TABLE radadmin_clients + ADD COLUMN IF NOT EXISTS status VARCHAR(10) NULL AFTER groupname; +ALTER TABLE radadmin_clients + ADD COLUMN IF NOT EXISTS deleted_at DATETIME NULL DEFAULT NULL AFTER created_at; +ALTER TABLE radadmin_clients + ADD COLUMN IF NOT EXISTS active_mac VARCHAR(17) + AS (IF(deleted_at IS NULL, mac_address, NULL)) STORED; +ALTER TABLE radadmin_clients + DROP INDEX IF EXISTS uq_radadmin_clients_mac; +ALTER TABLE radadmin_clients + ADD UNIQUE KEY IF NOT EXISTS uq_radadmin_clients_active_mac (active_mac); + CREATE TABLE IF NOT EXISTS radadmin_devices ( id INT UNSIGNED NOT NULL AUTO_INCREMENT, mac_address VARCHAR(17) NOT NULL,