Skip to content
16 changes: 16 additions & 0 deletions src/core/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,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
# =============================================================================
Expand Down
38 changes: 38 additions & 0 deletions src/database/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,41 @@ 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 count_uploaded_resources(*, user_id: int, expdb: AsyncConnection) -> int:
"""Count datasets, flows, runs, and studies uploaded or created by this user (expdb)."""
row = await expdb.execute(
text(
"""
SELECT (
(SELECT COUNT(*) FROM dataset WHERE uploader = :uid)
+ (SELECT COUNT(*) FROM implementation WHERE uploader = :uid)
+ (SELECT COUNT(*) FROM `run` WHERE uploader = :uid)
+ (SELECT COUNT(*) FROM study WHERE creator = :uid)
) AS total
""",
),
parameters={"uid": user_id},
)
return int(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(
Comment thread
igennova marked this conversation as resolved.
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},
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
2 changes: 2 additions & 0 deletions src/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,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
Expand Down Expand Up @@ -106,6 +107,7 @@ def create_api(configuration_file: Path | 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)
Expand Down
54 changes: 54 additions & 0 deletions src/routers/openml/users.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
"""User account HTTP endpoints."""

from typing import Annotated

from fastapi import APIRouter, Depends, Path, Response
from sqlalchemy.ext.asyncio import AsyncConnection

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

router = APIRouter(prefix="/users", tags=["users"])


@router.delete(
"/{user_id}",
responses={
204: {"description": "User account deleted."},
401: {"description": "Authentication failed or missing."},
403: {"description": "Not allowed to delete this account."},
404: {"description": "User id not found."},
409: {"description": "User still has datasets, flows, runs, or studies."},
Comment thread
igennova marked this conversation as resolved.
Outdated
},
)
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 a user account when they have no uploaded resources (Phase 1).

Callers may delete their own account. Administrators may delete any account
that satisfies the no-resources rule.
Comment thread
igennova marked this conversation as resolved.
Outdated
Comment thread
igennova marked this conversation as resolved.
Outdated
"""
Comment thread
igennova marked this conversation as resolved.
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)

resource_count = await database.users.count_uploaded_resources(user_id=user_id, expdb=expdb)
if resource_count > 0:
msg = (
"Cannot delete this account while datasets, flows, runs, or studies "
"are still associated with the user. Remove or transfer them first."
)
raise AccountHasResourcesError(msg)

Comment thread
igennova marked this conversation as resolved.
await database.users.delete_user_rows(user_id=user_id, userdb=userdb)
return Response(status_code=204)
Comment thread
igennova marked this conversation as resolved.
Outdated
191 changes: 191 additions & 0 deletions tests/routers/openml/users_delete_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
"""Tests for DELETE /users/{user_id} (Phase 1: no resources, self or admin)."""
Comment thread
igennova marked this conversation as resolved.
Outdated

import uuid
from http import HTTPStatus

import httpx
import pytest
from sqlalchemy import text
from sqlalchemy.ext.asyncio import AsyncConnection

from core.errors import AccountHasResourcesError, ForbiddenError, UserNotFoundError
from routers.openml.users import delete_user_account
from tests.users import ADMIN_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."


async def test_delete_user_not_found(py_api: httpx.AsyncClient) -> None:
response = await py_api.delete(
"/users/999999999",
params={"api_key": ApiKey.ADMIN},
)
assert response.status_code == HTTPStatus.NOT_FOUND
assert response.headers["content-type"] == "application/problem+json"
body = response.json()
assert body["type"] == UserNotFoundError.uri
assert body["detail"] == "User 999999999 not found."


async def test_delete_user_forbidden_non_admin_deletes_other(
py_api: httpx.AsyncClient,
) -> None:
response = await py_api.delete(
f"/users/{ADMIN_USER.user_id}",
params={"api_key": ApiKey.SOME_USER},
)
assert response.status_code == HTTPStatus.FORBIDDEN
body = response.json()
assert body["type"] == ForbiddenError.uri
assert body["detail"] == "You may only delete your own user account."


async def test_delete_user_conflict_when_user_has_resources(
py_api: httpx.AsyncClient,
) -> None:
"""User 16 owns dataset 130 in the test database."""
response = await py_api.delete(
"/users/16",
params={"api_key": ApiKey.ADMIN},
)
assert response.status_code == HTTPStatus.CONFLICT
assert response.headers["content-type"] == "application/problem+json"
body = response.json()
assert body["type"] == AccountHasResourcesError.uri
assert "datasets" in body["detail"]
Comment thread
igennova marked this conversation as resolved.
Outdated


@pytest.mark.mut
async def test_delete_user_api_success_self_delete(
py_api: httpx.AsyncClient,
user_test: AsyncConnection,
) -> None:
"""Disposable user deletes their own account using their API key (session_hash)."""
api_key = "fedcba9876543210fedcba9876543210"
Comment thread
igennova marked this conversation as resolved.
Outdated
suffix = uuid.uuid4().hex[:10]
username = f"tmp_self_{suffix}"
email = f"{suffix}@openml-self-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, 2)"),
parameters={"uid": new_id},
)

response = await py_api.delete(
f"/users/{new_id}",
params={"api_key": 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": new_id},
)
assert exists.one_or_none() is None


@pytest.mark.mut
async def test_delete_user_api_success_admin_deletes_disposable_user(
py_api: httpx.AsyncClient,
user_test: AsyncConnection,
) -> None:
suffix = uuid.uuid4().hex[:12]
username = f"tmp_del_{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
) VALUES (
'127.0.0.1', :username, 'x', :email, UNIX_TIMESTAMP(),
'', '', ''
)
""",
),
parameters={"username": username, "email": email},
)
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, 2)"),
parameters={"uid": new_id},
)

response = await py_api.delete(
f"/users/{new_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": new_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\."):
await delete_user_account(
user_id=888888888,
current_user=ADMIN_USER,
expdb=expdb_test,
userdb=user_test,
)


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\."):
await delete_user_account(
user_id=ADMIN_USER.user_id,
current_user=SOME_USER,
expdb=expdb_test,
userdb=user_test,
)


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"):
await delete_user_account(
user_id=16,
current_user=ADMIN_USER,
expdb=expdb_test,
userdb=user_test,
)
Loading