From 885399d89ff795f221150d12290e6faff2f16a43 Mon Sep 17 00:00:00 2001 From: igennova Date: Mon, 20 Apr 2026 03:21:05 +0530 Subject: [PATCH 1/2] Add DELETE /users/{id} with no-resources rule --- src/core/errors.py | 16 ++ src/database/users.py | 38 +++++ src/main.py | 2 + src/routers/openml/users.py | 54 ++++++ tests/routers/openml/users_delete_test.py | 191 ++++++++++++++++++++++ 5 files changed, 301 insertions(+) create mode 100644 src/routers/openml/users.py create mode 100644 tests/routers/openml/users_delete_test.py diff --git a/src/core/errors.py b/src/core/errors.py index 69d7e0c..fe111c5 100644 --- a/src/core/errors.py +++ b/src/core/errors.py @@ -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 # ============================================================================= diff --git a/src/database/users.py b/src/database/users.py index 0b09fb0..2507abf 100644 --- a/src/database/users.py +++ b/src/database/users.py @@ -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( + 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}, + ) diff --git a/src/main.py b/src/main.py index 46cd79c..0219db8 100644 --- a/src/main.py +++ b/src/main.py @@ -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 @@ -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) diff --git a/src/routers/openml/users.py b/src/routers/openml/users.py new file mode 100644 index 0000000..838f870 --- /dev/null +++ b/src/routers/openml/users.py @@ -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."}, + }, +) +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. + """ + 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) + + await database.users.delete_user_rows(user_id=user_id, userdb=userdb) + return Response(status_code=204) diff --git a/tests/routers/openml/users_delete_test.py b/tests/routers/openml/users_delete_test.py new file mode 100644 index 0000000..7903c69 --- /dev/null +++ b/tests/routers/openml/users_delete_test.py @@ -0,0 +1,191 @@ +"""Tests for DELETE /users/{user_id} (Phase 1: no resources, self or admin).""" + +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"] + + +@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" + 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, + ) From e3b016dae79d918e8549aa6963229ded61905090 Mon Sep 17 00:00:00 2001 From: igennova Date: Mon, 20 Apr 2026 03:42:08 +0530 Subject: [PATCH 2/2] query update --- src/database/users.py | 29 ++++++++++++++++------- src/routers/openml/users.py | 23 +++++++++++------- tests/routers/openml/users_delete_test.py | 9 +++---- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/database/users.py b/src/database/users.py index 2507abf..4f53968 100644 --- a/src/database/users.py +++ b/src/database/users.py @@ -86,22 +86,33 @@ async def exists_by_id(*, user_id: int, connection: AsyncConnection) -> bool: 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).""" +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 ( - (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 + 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 int(row.scalar_one()) + return bool(row.scalar_one()) async def delete_user_rows(*, user_id: int, userdb: AsyncConnection) -> None: diff --git a/src/routers/openml/users.py b/src/routers/openml/users.py index 838f870..40a0baa 100644 --- a/src/routers/openml/users.py +++ b/src/routers/openml/users.py @@ -3,6 +3,7 @@ from typing import Annotated from fastapi import APIRouter, Depends, Path, Response +from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncConnection import database.users @@ -10,6 +11,11 @@ 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"]) @@ -42,13 +48,14 @@ async def delete_user_account( 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) + 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: + # Safety net: a user-reference table was added without updating + # ``has_user_references``. Surface a clean 409 instead of a 500. + raise AccountHasResourcesError(_ACCOUNT_HAS_RESOURCES_MSG) from exc - await database.users.delete_user_rows(user_id=user_id, userdb=userdb) return Response(status_code=204) diff --git a/tests/routers/openml/users_delete_test.py b/tests/routers/openml/users_delete_test.py index 7903c69..407a427 100644 --- a/tests/routers/openml/users_delete_test.py +++ b/tests/routers/openml/users_delete_test.py @@ -9,6 +9,7 @@ from sqlalchemy.ext.asyncio import AsyncConnection 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, SOME_USER, ApiKey @@ -88,8 +89,8 @@ async def test_delete_user_api_success_self_delete( 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}, + text("INSERT INTO users_groups (user_id, group_id) VALUES (:uid, :gid)"), + parameters={"uid": new_id, "gid": UserGroup.READ_WRITE.value}, ) response = await py_api.delete( @@ -131,8 +132,8 @@ async def test_delete_user_api_success_admin_deletes_disposable_user( 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}, + text("INSERT INTO users_groups (user_id, group_id) VALUES (:uid, :gid)"), + parameters={"uid": new_id, "gid": UserGroup.READ_WRITE.value}, ) response = await py_api.delete(