Add DELETE /users/{id} with no-resources rule (#317)

Fix : #194 
# Description

Adds a new `DELETE /users/{user_id}` endpoint (Phase 1) so users can
delete
their own OpenML account, and administrators can delete any account, as
long
as the account has no uploaded resources (datasets, flows, runs,
studies).

 ## Phase 1
- `204 No Content` — account deleted successfully
- `401 Unauthorized` — no / invalid API key
- `403 Forbidden` — non-admin tries to delete another user's account
- `404 Not Found` — `user_id` does not exist
- `409 Conflict` — user still has uploaded datasets / flows / runs /
studies
  (RFC 9457 `AccountHasResourcesError`)

---------

Co-authored-by: igennova <luckynegi025@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: 3 people

src/core/errors.py

@@ -231,6 +231,22 @@ class ForbiddenError(ProblemDetailError):
     _default_status_code = HTTPStatus.FORBIDDEN
 
 
+class UserNotFoundError(ProblemDetailError):
+    """Raised when a user id does not exist in the user database."""
+
+    uri = "https://openml.org/problems/user-not-found"
+    title = "User Not Found"
+    _default_status_code = HTTPStatus.NOT_FOUND
+
+
+class AccountHasResourcesError(ProblemDetailError):
+    """Raised when account deletion is blocked because the user still owns resources."""
+
+    uri = "https://openml.org/problems/account-has-resources"
+    title = "Account Has Active Resources"
+    _default_status_code = HTTPStatus.CONFLICT
+
+
 # =============================================================================
 # Tag Errors
 # =============================================================================

src/database/users.py

@@ -99,3 +99,52 @@ async def get_groups(self) -> list[UserGroup]:
 
     async def is_admin(self) -> bool:
         return UserGroup.ADMIN in await self.get_groups()
+
+
+async def exists_by_id(*, user_id: int, connection: AsyncConnection) -> bool:
+    row = await connection.execute(
+        text("SELECT 1 FROM users WHERE id = :user_id LIMIT 1"),
+        parameters={"user_id": user_id},
+    )
+    return row.one_or_none() is not None
+
+
+async def has_user_references(*, user_id: int, expdb: AsyncConnection) -> bool:
+    """Return ``True`` if any ``expdb`` row still references ``user_id``."""
+    row = await expdb.execute(
+        text(
+            """
+            SELECT EXISTS (
+                SELECT 1 FROM dataset              WHERE uploader = :uid
+                UNION ALL SELECT 1 FROM dataset_description WHERE uploader = :uid
+                UNION ALL SELECT 1 FROM dataset_status      WHERE user_id  = :uid
+                UNION ALL SELECT 1 FROM dataset_tag         WHERE uploader = :uid
+                UNION ALL SELECT 1 FROM dataset_topic       WHERE uploader = :uid
+                UNION ALL SELECT 1 FROM implementation      WHERE uploader = :uid
+                UNION ALL SELECT 1 FROM implementation_tag  WHERE uploader = :uid
+                UNION ALL SELECT 1 FROM `run`               WHERE uploader = :uid
+                UNION ALL SELECT 1 FROM run_study           WHERE uploader = :uid
+                UNION ALL SELECT 1 FROM run_tag             WHERE uploader = :uid
+                UNION ALL SELECT 1 FROM setup_tag           WHERE uploader = :uid
+                UNION ALL SELECT 1 FROM study               WHERE creator  = :uid
+                UNION ALL SELECT 1 FROM task                WHERE creator  = :uid
+                UNION ALL SELECT 1 FROM task_study          WHERE uploader = :uid
+                UNION ALL SELECT 1 FROM task_tag            WHERE uploader = :uid
+            ) AS has_refs
+            """,
+        ),
+        parameters={"uid": user_id},
+    )
+    return bool(row.scalar_one())
+
+
+async def delete_user_rows(*, user_id: int, userdb: AsyncConnection) -> None:
+    """Remove group memberships then the user row (openml user database)."""
+    await userdb.execute(
+        text("DELETE FROM users_groups WHERE user_id = :user_id"),
+        parameters={"user_id": user_id},
+    )
+    await userdb.execute(
+        text("DELETE FROM users WHERE id = :user_id"),
+        parameters={"user_id": user_id},
+    )

src/main.py

@@ -37,6 +37,7 @@
 from routers.openml.study import router as study_router
 from routers.openml.tasks import router as task_router
 from routers.openml.tasktype import router as ttype_router
+from routers.openml.users import router as users_router
 
 
 @asynccontextmanager
@@ -110,6 +111,7 @@ def create_api(configuration: Configuration | None = None) -> FastAPI:
     app.include_router(study_router)
     app.include_router(setup_router)
     app.include_router(run_router)
+    app.include_router(users_router)
 
     logger.info("App setup completed.")
     logger.remove(setup_sink)

src/routers/openml/users.py

@@ -0,0 +1,71 @@
+"""User account HTTP endpoints."""
+
+from http import HTTPStatus
+from typing import Annotated
+
+from fastapi import APIRouter, Depends, Path, Response
+from loguru import logger
+from sqlalchemy.exc import IntegrityError
+from sqlalchemy.ext.asyncio import AsyncConnection  # noqa: TC002 used at runtime by FastAPI Depends
+
+import database.users
+from core.errors import AccountHasResourcesError, ForbiddenError, UserNotFoundError
+from database.users import User
+from routers.dependencies import expdb_connection, fetch_user_or_raise, userdb_connection
+
+_ACCOUNT_HAS_RESOURCES_MSG = (
+    "Cannot delete this account while records still reference the user "
+    "(datasets, flows, runs, studies, tags, etc.). Remove or transfer them first."
+)
+
+router = APIRouter(prefix="/users", tags=["users"])
+
+
+@router.delete(
+    "/{user_id}",
+    responses={
+        HTTPStatus.NO_CONTENT: {"description": "User account deleted."},
+        HTTPStatus.UNAUTHORIZED: {"description": "Authentication failed or missing."},
+        HTTPStatus.FORBIDDEN: {"description": "Not allowed to delete this account."},
+        HTTPStatus.NOT_FOUND: {"description": "User id not found."},
+        HTTPStatus.CONFLICT: {
+            "description": "User still has datasets, flows, runs, or studies.",
+        },
+    },
+)
+async def delete_user_account(
+    user_id: Annotated[int, Path(description="Numeric user id to delete.", gt=0)],
+    current_user: Annotated[User, Depends(fetch_user_or_raise)],
+    expdb: Annotated[AsyncConnection, Depends(expdb_connection)],
+    userdb: Annotated[AsyncConnection, Depends(userdb_connection)],
+) -> Response:
+    """Delete the user account if they have no associated resources.
+
+    The account to be deleted must not have associated resources (such as
+    datasets, tasks, or tags). Users may only delete their own account.
+    Administrators may delete any account that satisfies the no-resources rule.
+    """
+    # How to handle users that do have associated resources is an ongoing discussion,
+    # see also: https://github.com/openml/server-api/issues/194
+    if current_user.user_id != user_id and not await current_user.is_admin():
+        msg = "You may only delete your own user account."
+        raise ForbiddenError(msg)
+
+    if not await database.users.exists_by_id(user_id=user_id, connection=userdb):
+        msg = f"User {user_id} not found."
+        raise UserNotFoundError(msg)
+
+    if await database.users.has_user_references(user_id=user_id, expdb=expdb):
+        raise AccountHasResourcesError(_ACCOUNT_HAS_RESOURCES_MSG)
+
+    try:
+        await database.users.delete_user_rows(user_id=user_id, userdb=userdb)
+    except IntegrityError as exc:
+        logger.error(
+            "Delete of user {user_id} failed with integrity error after pre-check.",
+            user_id=user_id,
+        )
+        raise AccountHasResourcesError(_ACCOUNT_HAS_RESOURCES_MSG) from exc
+
+    logger.info("User account {user_id} was removed.", user_id=user_id)
+    return Response(status_code=HTTPStatus.NO_CONTENT)

tests/routers/openml/users_delete_test.py

@@ -0,0 +1,202 @@
+"""Tests for DELETE /users/{user_id}."""
+
+import uuid
+from http import HTTPStatus
+from typing import NamedTuple
+
+import httpx  # noqa: TC002 used at runtime by pytest fixtures
+import pytest
+import pytest_mock  # noqa: TC002 used at runtime by pytest fixtures
+from sqlalchemy import text
+from sqlalchemy.exc import IntegrityError
+from sqlalchemy.ext.asyncio import AsyncConnection  # noqa: TC002 used at runtime by pytest fixtures
+
+from core.errors import AccountHasResourcesError, ForbiddenError, UserNotFoundError
+from database.users import UserGroup
+from routers.openml.users import delete_user_account
+from tests.users import ADMIN_USER, OWNER_USER, SOME_USER, ApiKey
+
+
+async def test_delete_user_missing_auth(py_api: httpx.AsyncClient) -> None:
+    response = await py_api.delete("/users/1")
+    assert response.status_code == HTTPStatus.UNAUTHORIZED
+    body = response.json()
+    assert body["code"] == "103"
+    assert body["detail"] == "No API key provided."
+
+
+class DisposableUser(NamedTuple):
+    user_id: int
+    api_key: str
+
+
+@pytest.fixture
+async def disposable_user(user_test: AsyncConnection) -> DisposableUser:
+    api_key = uuid.uuid4().hex
+    suffix = uuid.uuid4().hex[:10]
+    username = f"tmp_user_{suffix}"
+    email = f"{suffix}@openml-delete.test"
+
+    await user_test.execute(
+        text(
+            """
+            INSERT INTO users (
+                ip_address, username, password, email, created_on,
+                company, country, bio, session_hash
+            ) VALUES (
+                '127.0.0.1', :username, 'x', :email, UNIX_TIMESTAMP(),
+                '', '', '', :api_key
+            )
+            """,
+        ),
+        parameters={"username": username, "email": email, "api_key": api_key},
+    )
+    uid_row = await user_test.execute(text("SELECT LAST_INSERT_ID() AS id"))
+    (new_id,) = uid_row.one()
+    await user_test.execute(
+        text("INSERT INTO users_groups (user_id, group_id) VALUES (:uid, :gid)"),
+        parameters={"uid": new_id, "gid": UserGroup.READ_WRITE.value},
+    )
+    return DisposableUser(user_id=new_id, api_key=api_key)
+    # No explicit teardown: the ``user_test`` fixture rolls back at the end
+    # of the test, which removes the rows inserted above.
+
+
+@pytest.mark.mut
+async def test_delete_user_api_success_self_delete(
+    py_api: httpx.AsyncClient,
+    user_test: AsyncConnection,
+    disposable_user: DisposableUser,
+    mocker: pytest_mock.MockerFixture,
+) -> None:
+    log_info = mocker.patch("routers.openml.users.logger.info")
+
+    response = await py_api.delete(
+        f"/users/{disposable_user.user_id}",
+        params={"api_key": disposable_user.api_key},
+    )
+    assert response.status_code == HTTPStatus.NO_CONTENT
+    assert response.content == b""
+
+    exists = await user_test.execute(
+        text("SELECT 1 FROM users WHERE id = :id LIMIT 1"),
+        parameters={"id": disposable_user.user_id},
+    )
+    assert exists.one_or_none() is None
+
+    log_info.assert_any_call(
+        "User account {user_id} was removed.",
+        user_id=disposable_user.user_id,
+    )
+
+
+@pytest.mark.mut
+async def test_delete_user_api_success_admin_deletes_disposable_user(
+    py_api: httpx.AsyncClient,
+    user_test: AsyncConnection,
+    disposable_user: DisposableUser,
+) -> None:
+    response = await py_api.delete(
+        f"/users/{disposable_user.user_id}",
+        params={"api_key": ApiKey.ADMIN},
+    )
+    assert response.status_code == HTTPStatus.NO_CONTENT
+    assert response.content == b""
+
+    exists = await user_test.execute(
+        text("SELECT 1 FROM users WHERE id = :id LIMIT 1"),
+        parameters={"id": disposable_user.user_id},
+    )
+    assert exists.one_or_none() is None
+
+
+# ── Direct handler tests ──
+
+
+async def test_delete_user_direct_not_found(
+    user_test: AsyncConnection,
+    expdb_test: AsyncConnection,
+) -> None:
+    with pytest.raises(UserNotFoundError, match=r"User 888888888 not found\.") as exc_info:
+        await delete_user_account(
+            user_id=888888888,
+            current_user=ADMIN_USER,
+            expdb=expdb_test,
+            userdb=user_test,
+        )
+    assert exc_info.value.status_code == HTTPStatus.NOT_FOUND
+    assert exc_info.value.uri == UserNotFoundError.uri
+
+
+async def test_delete_user_direct_forbidden(
+    user_test: AsyncConnection,
+    expdb_test: AsyncConnection,
+) -> None:
+    with pytest.raises(
+        ForbiddenError, match=r"You may only delete your own user account\."
+    ) as exc_info:
+        await delete_user_account(
+            user_id=ADMIN_USER.user_id,
+            current_user=SOME_USER,
+            expdb=expdb_test,
+            userdb=user_test,
+        )
+    assert exc_info.value.status_code == HTTPStatus.FORBIDDEN
+    assert exc_info.value.uri == ForbiddenError.uri
+
+    admin_row = await user_test.execute(
+        text("SELECT 1 FROM users WHERE id = :id LIMIT 1"),
+        parameters={"id": ADMIN_USER.user_id},
+    )
+    assert admin_row.one_or_none() is not None
+
+
+async def test_delete_user_direct_conflict_has_resources(
+    user_test: AsyncConnection,
+    expdb_test: AsyncConnection,
+) -> None:
+    with pytest.raises(AccountHasResourcesError, match="Cannot delete this account") as exc_info:
+        await delete_user_account(
+            user_id=OWNER_USER.user_id,
+            current_user=ADMIN_USER,
+            expdb=expdb_test,
+            userdb=user_test,
+        )
+    assert exc_info.value.status_code == HTTPStatus.CONFLICT
+    assert exc_info.value.uri == AccountHasResourcesError.uri
+
+    owner_row = await user_test.execute(
+        text("SELECT 1 FROM users WHERE id = :id LIMIT 1"),
+        parameters={"id": OWNER_USER.user_id},
+    )
+    assert owner_row.one_or_none() is not None
+
+
+@pytest.mark.mut
+async def test_delete_user_integrity_error_logs_and_raises_conflict(
+    user_test: AsyncConnection,
+    expdb_test: AsyncConnection,
+    disposable_user: DisposableUser,
+    mocker: pytest_mock.MockerFixture,
+) -> None:
+    mocker.patch(
+        "database.users.delete_user_rows",
+        side_effect=IntegrityError(
+            "DELETE FROM users", {"user_id": disposable_user.user_id}, Exception("fk")
+        ),
+    )
+    log_error = mocker.patch("routers.openml.users.logger.error")
+
+    with pytest.raises(AccountHasResourcesError, match="Cannot delete this account") as exc_info:
+        await delete_user_account(
+            user_id=disposable_user.user_id,
+            current_user=ADMIN_USER,
+            expdb=expdb_test,
+            userdb=user_test,
+        )
+
+    assert exc_info.value.status_code == HTTPStatus.CONFLICT
+    log_error.assert_called_once_with(
+        "Delete of user {user_id} failed with integrity error after pre-check.",
+        user_id=disposable_user.user_id,
+    )