From 6e875f4d4c5866c72bfed1572915618591a7afeb Mon Sep 17 00:00:00 2001 From: Victor Skvortsov Date: Tue, 25 Nov 2025 15:09:40 +0500 Subject: [PATCH 1/8] Add test_deletes_user_with_resources --- .../_internal/server/routers/test_users.py | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/src/tests/_internal/server/routers/test_users.py b/src/tests/_internal/server/routers/test_users.py index 54da63803..672405083 100644 --- a/src/tests/_internal/server/routers/test_users.py +++ b/src/tests/_internal/server/routers/test_users.py @@ -8,9 +8,18 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from dstack._internal.core.models.runs import JobStatus, RunStatus from dstack._internal.core.models.users import GlobalRole from dstack._internal.server.models import UserModel -from dstack._internal.server.testing.common import create_user, get_auth_headers +from dstack._internal.server.testing.common import ( + create_job, + create_probe, + create_project, + create_repo, + create_run, + create_user, + get_auth_headers, +) class TestListUsers: @@ -372,6 +381,34 @@ async def test_deletes_users(self, test_db, session: AsyncSession, client: Async res = await session.execute(select(UserModel).where(UserModel.name == user.name)) assert len(res.scalars().all()) == 0 + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + @pytest.mark.usefixtures("image_config_mock") + async def test_deletes_user_with_resources( + self, test_db, session: AsyncSession, client: AsyncClient + ): + admin = await create_user(name="admin", session=session) + user = await create_user(name="temp", session=session, global_role=GlobalRole.USER) + project = await create_project(session=session, owner=user) + repo = await create_repo(session=session, project_id=project.id) + run = await create_run( + session=session, + project=project, + repo=repo, + user=user, + status=RunStatus.RUNNING, + ) + job = await create_job(session=session, run=run, status=JobStatus.RUNNING) + await create_probe(session=session, job=job) + response = await client.post( + "/api/users/delete", + headers=get_auth_headers(admin.token), + json={"users": [user.name]}, + ) + assert response.status_code == 200 + res = await session.execute(select(UserModel).where(UserModel.name == user.name)) + assert res.scalar() is None + class TestRefreshToken: @pytest.mark.asyncio From c33fb589e53627dab12498ea5df06d41c6320883 Mon Sep 17 00:00:00 2001 From: Victor Skvortsov Date: Tue, 25 Nov 2025 15:15:35 +0500 Subject: [PATCH 2/8] Return 400 if deleting non-existent users --- src/dstack/_internal/server/services/users.py | 8 +++++++ .../_internal/server/routers/test_users.py | 21 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/dstack/_internal/server/services/users.py b/src/dstack/_internal/server/services/users.py index 9fdbe3b4e..4fcca1317 100644 --- a/src/dstack/_internal/server/services/users.py +++ b/src/dstack/_internal/server/services/users.py @@ -173,6 +173,14 @@ async def delete_users( user: UserModel, usernames: List[str], ): + res = await session.execute( + select(UserModel.id).where( + UserModel.name.in_(usernames), + ) + ) + user_ids = res.scalars().all() + if len(user_ids) != len(usernames): + raise ServerClientError("Failed to delete non-existent users") await session.execute(delete(UserModel).where(UserModel.name.in_(usernames))) await session.commit() logger.info("Deleted users %s by user %s", usernames, user.name) diff --git a/src/tests/_internal/server/routers/test_users.py b/src/tests/_internal/server/routers/test_users.py index 672405083..8ec71e3d5 100644 --- a/src/tests/_internal/server/routers/test_users.py +++ b/src/tests/_internal/server/routers/test_users.py @@ -381,6 +381,27 @@ async def test_deletes_users(self, test_db, session: AsyncSession, client: Async res = await session.execute(select(UserModel).where(UserModel.name == user.name)) assert len(res.scalars().all()) == 0 + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_returns_400_if_users_not_exist( + self, test_db, session: AsyncSession, client: AsyncClient + ): + admin = await create_user(name="admin", session=session) + user1 = await create_user(name="test1", session=session) + user2 = await create_user(name="test2", session=session) + response = await client.post( + "/api/users/delete", + headers=get_auth_headers(admin.token), + json={"users": [user1.name, "non_existing_user"]}, + ) + assert response.status_code == 400 + response = await client.post( + "/api/users/delete", + headers=get_auth_headers(admin.token), + json={"users": [user1.name, user2.name]}, + ) + assert response.status_code == 200 + @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) @pytest.mark.usefixtures("image_config_mock") From 72da7ec11921db8d72a19e5ecf77f335d27307ae Mon Sep 17 00:00:00 2001 From: Victor Skvortsov Date: Wed, 26 Nov 2025 10:33:21 +0500 Subject: [PATCH 3/8] Implement users soft-delete --- .../965760bacf93_add_usermodel_deleted.py | 34 ++++++++++++ src/dstack/_internal/server/models.py | 1 + .../_internal/server/services/projects.py | 25 ++++++--- src/dstack/_internal/server/services/users.py | 53 +++++++++++++++---- src/dstack/_internal/server/testing/common.py | 2 + .../_internal/server/routers/test_users.py | 11 +++- 6 files changed, 108 insertions(+), 18 deletions(-) create mode 100644 src/dstack/_internal/server/migrations/versions/965760bacf93_add_usermodel_deleted.py diff --git a/src/dstack/_internal/server/migrations/versions/965760bacf93_add_usermodel_deleted.py b/src/dstack/_internal/server/migrations/versions/965760bacf93_add_usermodel_deleted.py new file mode 100644 index 000000000..0fcf92c16 --- /dev/null +++ b/src/dstack/_internal/server/migrations/versions/965760bacf93_add_usermodel_deleted.py @@ -0,0 +1,34 @@ +"""Add UserModel.deleted + +Revision ID: 965760bacf93 +Revises: 7d1ec2b920ac +Create Date: 2025-11-25 15:17:05.513712 + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "965760bacf93" +down_revision = "7d1ec2b920ac" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table("users", schema=None) as batch_op: + batch_op.add_column( + sa.Column("deleted", sa.Boolean(), server_default=sa.text("0"), nullable=False) + ) + + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table("users", schema=None) as batch_op: + batch_op.drop_column("deleted") + + # ### end Alembic commands ### diff --git a/src/dstack/_internal/server/models.py b/src/dstack/_internal/server/models.py index e88f83d59..c053ce019 100644 --- a/src/dstack/_internal/server/models.py +++ b/src/dstack/_internal/server/models.py @@ -190,6 +190,7 @@ class UserModel(BaseModel): global_role: Mapped[GlobalRole] = mapped_column(EnumAsString(GlobalRole, 100)) # deactivated users cannot access API active: Mapped[bool] = mapped_column(Boolean, default=True) + deleted: Mapped[bool] = mapped_column(Boolean, server_default=false()) # SSH keys can be null for users created before 0.19.33. # Keys for those users are being gradually generated on /get_my_user calls. diff --git a/src/dstack/_internal/server/services/projects.py b/src/dstack/_internal/server/services/projects.py index 330fcceb4..e835691e1 100644 --- a/src/dstack/_internal/server/services/projects.py +++ b/src/dstack/_internal/server/services/projects.py @@ -194,7 +194,10 @@ async def delete_projects( raise ServerClientError("Cannot delete the only project") res = await session.execute( - select(ProjectModel.id).where(ProjectModel.name.in_(projects_names)) + select(ProjectModel.id).where( + ProjectModel.name.in_(projects_names), + ProjectModel.deleted == False, + ) ) project_ids = res.scalars().all() if len(project_ids) != len(projects_names): @@ -206,10 +209,10 @@ async def delete_projects( await _check_project_has_active_resources(session=session, project_id=project_id) timestamp = str(int(get_current_datetime().timestamp())) - new_project_name = "_deleted_" + timestamp + ProjectModel.name + new_project_name = f"_deleted_{timestamp}_" + ProjectModel.name await session.execute( update(ProjectModel) - .where(ProjectModel.name.in_(projects_names)) + .where(ProjectModel.id.in_(project_ids)) .values( deleted=True, name=new_project_name, @@ -244,12 +247,16 @@ async def set_project_members( } if new_admins_members != current_admins_members: raise ForbiddenError("Access denied: changing project admins") + # FIXME: potentially long write transaction # clear_project_members() issues DELETE without commit await clear_project_members(session=session, project=project) names = [m.username for m in members] res = await session.execute( - select(UserModel).where((UserModel.name.in_(names)) | (UserModel.email.in_(names))) + select(UserModel).where( + (UserModel.name.in_(names)) | (UserModel.email.in_(names)), + UserModel.deleted == False, + ) ) users = res.scalars().all() username_to_user = {user.name: user for user in users} @@ -311,7 +318,10 @@ async def add_project_members( raise ForbiddenError("Access denied: can only join public projects as user role") res = await session.execute( - select(UserModel).where((UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames))) + select(UserModel).where( + (UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames)), + UserModel.deleted == False, + ) ) users_found = res.scalars().all() @@ -700,7 +710,10 @@ async def remove_project_members( raise ForbiddenError("Access denied: insufficient permissions to remove members") res = await session.execute( - select(UserModel).where((UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames))) + select(UserModel).where( + (UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames)), + UserModel.deleted == False, + ) ) users_found = res.scalars().all() diff --git a/src/dstack/_internal/server/services/users.py b/src/dstack/_internal/server/services/users.py index 4fcca1317..59b5e67a5 100644 --- a/src/dstack/_internal/server/services/users.py +++ b/src/dstack/_internal/server/services/users.py @@ -4,8 +4,8 @@ import uuid from typing import Awaitable, Callable, List, Optional, Tuple -from sqlalchemy import delete, select, update from sqlalchemy import func as safunc +from sqlalchemy import select, update from sqlalchemy.ext.asyncio import AsyncSession from dstack._internal.core.errors import ResourceExistsError, ServerClientError @@ -21,7 +21,7 @@ from dstack._internal.server.services.permissions import get_default_permissions from dstack._internal.server.utils.routers import error_forbidden from dstack._internal.utils import crypto -from dstack._internal.utils.common import run_async +from dstack._internal.utils.common import get_current_datetime, get_or_error, run_async from dstack._internal.utils.logging import get_logger logger = get_logger(__name__) @@ -53,8 +53,12 @@ async def list_users_for_user( async def list_all_users( session: AsyncSession, + include_deleted: bool = False, ) -> List[User]: - res = await session.execute(select(UserModel)) + filters = [] + if not include_deleted: + filters.append(UserModel.deleted == False) + res = await session.execute(select(UserModel).where(*filters)) user_models = res.scalars().all() user_models = sorted(user_models, key=lambda u: u.created_at) return [user_model_to_user(u) for u in user_models] @@ -116,7 +120,10 @@ async def update_user( ) -> UserModel: await session.execute( update(UserModel) - .where(UserModel.name == username) + .where( + UserModel.name == username, + UserModel.deleted == False, + ) .values( global_role=global_role, email=email, @@ -138,7 +145,10 @@ async def refresh_ssh_key( private_bytes, public_bytes = await run_async(crypto.generate_rsa_key_pair_bytes, username) await session.execute( update(UserModel) - .where(UserModel.name == username) + .where( + UserModel.name == username, + UserModel.deleted == False, + ) .values( ssh_private_key=private_bytes.decode(), ssh_public_key=public_bytes.decode(), @@ -158,7 +168,10 @@ async def refresh_user_token( new_token = str(uuid.uuid4()) await session.execute( update(UserModel) - .where(UserModel.name == username) + .where( + UserModel.name == username, + UserModel.deleted == False, + ) .values( token=DecryptedString(plaintext=new_token), token_hash=get_token_hash(new_token), @@ -176,12 +189,24 @@ async def delete_users( res = await session.execute( select(UserModel.id).where( UserModel.name.in_(usernames), + UserModel.deleted == False, ) ) user_ids = res.scalars().all() if len(user_ids) != len(usernames): raise ServerClientError("Failed to delete non-existent users") - await session.execute(delete(UserModel).where(UserModel.name.in_(usernames))) + + timestamp = str(int(get_current_datetime().timestamp())) + new_username = f"_deleted_{timestamp}_" + UserModel.name + await session.execute( + update(UserModel) + .where(UserModel.id.in_(user_ids)) + .values( + deleted=True, + active=False, + name=new_username, + ) + ) await session.commit() logger.info("Deleted users %s by user %s", usernames, user.name) @@ -191,7 +216,7 @@ async def get_user_model_by_name( username: str, ignore_case: bool = False, ) -> Optional[UserModel]: - filters = [] + filters = [UserModel.deleted == False] if ignore_case: filters.append(safunc.lower(UserModel.name) == safunc.lower(username)) else: @@ -200,9 +225,14 @@ async def get_user_model_by_name( return res.scalar() -async def get_user_model_by_name_or_error(session: AsyncSession, username: str) -> UserModel: - res = await session.execute(select(UserModel).where(UserModel.name == username)) - return res.scalar_one() +async def get_user_model_by_name_or_error( + session: AsyncSession, + username: str, + ignore_case: bool = False, +) -> UserModel: + return get_or_error( + await get_user_model_by_name(session=session, username=username, ignore_case=ignore_case) + ) async def log_in_with_token(session: AsyncSession, token: str) -> Optional[UserModel]: @@ -211,6 +241,7 @@ async def log_in_with_token(session: AsyncSession, token: str) -> Optional[UserM select(UserModel).where( UserModel.token_hash == token_hash, UserModel.active == True, + UserModel.deleted == False, ) ) user = res.scalar() diff --git a/src/dstack/_internal/server/testing/common.py b/src/dstack/_internal/server/testing/common.py index 883ce1453..9c9585982 100644 --- a/src/dstack/_internal/server/testing/common.py +++ b/src/dstack/_internal/server/testing/common.py @@ -135,6 +135,7 @@ async def create_user( ssh_public_key: Optional[str] = None, ssh_private_key: Optional[str] = None, active: bool = True, + deleted: bool = False, ) -> UserModel: if token is None: token = str(uuid.uuid4()) @@ -148,6 +149,7 @@ async def create_user( ssh_public_key=ssh_public_key, ssh_private_key=ssh_private_key, active=active, + deleted=deleted, ) session.add(user) await session.commit() diff --git a/src/tests/_internal/server/routers/test_users.py b/src/tests/_internal/server/routers/test_users.py index 8ec71e3d5..aa827db1d 100644 --- a/src/tests/_internal/server/routers/test_users.py +++ b/src/tests/_internal/server/routers/test_users.py @@ -31,7 +31,9 @@ async def test_returns_40x_if_not_authenticated(self, test_db, client: AsyncClie @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_admins_see_all_users(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_admins_see_all_non_deleted_users( + self, test_db, session: AsyncSession, client: AsyncClient + ): admin = await create_user( session=session, name="admin", @@ -44,6 +46,13 @@ async def test_admins_see_all_users(self, test_db, session: AsyncSession, client created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), global_role=GlobalRole.USER, ) + await create_user( + session=session, + name="deleted_user", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + global_role=GlobalRole.USER, + deleted=True, + ) response = await client.post("/api/users/list", headers=get_auth_headers(admin.token)) assert response.status_code in [200] assert response.json() == [ From 8552647a0c3a077d2b2f7f337b47ff39e2b43720 Mon Sep 17 00:00:00 2001 From: Victor Skvortsov Date: Wed, 26 Nov 2025 11:19:06 +0500 Subject: [PATCH 4/8] Fix deleting users with long names --- ...316bfd_add_usermodel_original_name_and_.py | 38 +++++++++++++++++++ src/dstack/_internal/server/models.py | 6 ++- src/dstack/_internal/server/services/users.py | 30 +++++++++------ .../_internal/server/routers/test_users.py | 7 +++- 4 files changed, 66 insertions(+), 15 deletions(-) create mode 100644 src/dstack/_internal/server/migrations/versions/cc3912316bfd_add_usermodel_original_name_and_.py diff --git a/src/dstack/_internal/server/migrations/versions/cc3912316bfd_add_usermodel_original_name_and_.py b/src/dstack/_internal/server/migrations/versions/cc3912316bfd_add_usermodel_original_name_and_.py new file mode 100644 index 000000000..2130d21a0 --- /dev/null +++ b/src/dstack/_internal/server/migrations/versions/cc3912316bfd_add_usermodel_original_name_and_.py @@ -0,0 +1,38 @@ +"""Add UserModel.original_name and ProjectModel.original_name + +Revision ID: cc3912316bfd +Revises: 965760bacf93 +Create Date: 2025-11-26 11:04:18.383458 + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "cc3912316bfd" +down_revision = "965760bacf93" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table("projects", schema=None) as batch_op: + batch_op.add_column(sa.Column("original_name", sa.String(length=50), nullable=True)) + + with op.batch_alter_table("users", schema=None) as batch_op: + batch_op.add_column(sa.Column("original_name", sa.String(length=50), nullable=True)) + + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table("users", schema=None) as batch_op: + batch_op.drop_column("original_name") + + with op.batch_alter_table("projects", schema=None) as batch_op: + batch_op.drop_column("original_name") + + # ### end Alembic commands ### diff --git a/src/dstack/_internal/server/models.py b/src/dstack/_internal/server/models.py index c053ce019..04185762a 100644 --- a/src/dstack/_internal/server/models.py +++ b/src/dstack/_internal/server/models.py @@ -191,6 +191,8 @@ class UserModel(BaseModel): # deactivated users cannot access API active: Mapped[bool] = mapped_column(Boolean, default=True) deleted: Mapped[bool] = mapped_column(Boolean, server_default=false()) + # `original_name` stores the name of a deleted user, while `name` is changed to a unique generated value. + original_name: Mapped[Optional[str]] = mapped_column(String(50), nullable=True) # SSH keys can be null for users created before 0.19.33. # Keys for those users are being gradually generated on /get_my_user calls. @@ -213,8 +215,10 @@ class ProjectModel(BaseModel): ) name: Mapped[str] = mapped_column(String(50), unique=True) created_at: Mapped[datetime] = mapped_column(NaiveDateTime, default=get_current_datetime) - deleted: Mapped[bool] = mapped_column(Boolean, default=False) is_public: Mapped[bool] = mapped_column(Boolean, default=False) + deleted: Mapped[bool] = mapped_column(Boolean, default=False) + # `original_name` stores the name of a deleted project, while `name` is changed to a unique generated value. + original_name: Mapped[Optional[str]] = mapped_column(String(50), nullable=True) owner_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("users.id", ondelete="CASCADE")) owner: Mapped[UserModel] = relationship(lazy="joined") diff --git a/src/dstack/_internal/server/services/users.py b/src/dstack/_internal/server/services/users.py index 59b5e67a5..f7573364b 100644 --- a/src/dstack/_internal/server/services/users.py +++ b/src/dstack/_internal/server/services/users.py @@ -1,12 +1,14 @@ import hashlib import os import re +import secrets import uuid from typing import Awaitable, Callable, List, Optional, Tuple from sqlalchemy import func as safunc from sqlalchemy import select, update from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.orm import load_only from dstack._internal.core.errors import ResourceExistsError, ServerClientError from dstack._internal.core.models.users import ( @@ -187,26 +189,30 @@ async def delete_users( usernames: List[str], ): res = await session.execute( - select(UserModel.id).where( + select(UserModel) + .where( UserModel.name.in_(usernames), UserModel.deleted == False, ) + .options(load_only(UserModel.id, UserModel.name)) ) - user_ids = res.scalars().all() - if len(user_ids) != len(usernames): + users = res.scalars().all() + if len(users) != len(usernames): raise ServerClientError("Failed to delete non-existent users") timestamp = str(int(get_current_datetime().timestamp())) - new_username = f"_deleted_{timestamp}_" + UserModel.name - await session.execute( - update(UserModel) - .where(UserModel.id.in_(user_ids)) - .values( - deleted=True, - active=False, - name=new_username, + updates = [] + for u in users: + updates.append( + { + "id": u.id, + "name": f"_deleted_{timestamp}_{secrets.token_hex(8)}", + "original_name": u.name, + "deleted": True, + "active": False, + } ) - ) + await session.execute(update(UserModel), updates) await session.commit() logger.info("Deleted users %s by user %s", usernames, user.name) diff --git a/src/tests/_internal/server/routers/test_users.py b/src/tests/_internal/server/routers/test_users.py index aa827db1d..a5b3edac4 100644 --- a/src/tests/_internal/server/routers/test_users.py +++ b/src/tests/_internal/server/routers/test_users.py @@ -378,9 +378,12 @@ async def test_returns_40x_if_not_authenticated(self, test_db, client: AsyncClie @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_deletes_users(self, test_db, session: AsyncSession, client: AsyncClient): + @pytest.mark.parametrize("username", ["test", "a" * 50]) + async def test_deletes_users( + self, test_db, session: AsyncSession, client: AsyncClient, username: str + ): admin = await create_user(name="admin", session=session) - user = await create_user(name="test", session=session) + user = await create_user(name=username, session=session) response = await client.post( "/api/users/delete", headers=get_auth_headers(admin.token), From f0f2ed77e34620077686e6fcb71967d68337b9a7 Mon Sep 17 00:00:00 2001 From: Victor Skvortsov Date: Wed, 26 Nov 2025 11:24:53 +0500 Subject: [PATCH 5/8] Fix deleting projects with long names --- .../_internal/server/services/projects.py | 32 +++++++++++-------- .../_internal/server/routers/test_projects.py | 7 ++-- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/dstack/_internal/server/services/projects.py b/src/dstack/_internal/server/services/projects.py index e835691e1..cc6c37401 100644 --- a/src/dstack/_internal/server/services/projects.py +++ b/src/dstack/_internal/server/services/projects.py @@ -1,3 +1,4 @@ +import secrets import uuid from typing import Awaitable, Callable, List, Optional, Tuple @@ -194,31 +195,36 @@ async def delete_projects( raise ServerClientError("Cannot delete the only project") res = await session.execute( - select(ProjectModel.id).where( + select(ProjectModel) + .where( ProjectModel.name.in_(projects_names), ProjectModel.deleted == False, ) + .options(load_only(ProjectModel.id, ProjectModel.name)) ) - project_ids = res.scalars().all() - if len(project_ids) != len(projects_names): + projects = res.scalars().all() + if len(projects) != len(projects_names): raise ServerClientError("Failed to delete non-existent projects") - for project_id in project_ids: + for p in projects: # FIXME: The checks are not under lock, # so there can be dangling active resources due to race conditions. - await _check_project_has_active_resources(session=session, project_id=project_id) + await _check_project_has_active_resources(session=session, project_id=p.id) timestamp = str(int(get_current_datetime().timestamp())) - new_project_name = f"_deleted_{timestamp}_" + ProjectModel.name - await session.execute( - update(ProjectModel) - .where(ProjectModel.id.in_(project_ids)) - .values( - deleted=True, - name=new_project_name, + updates = [] + for p in projects: + updates.append( + { + "id": p.id, + "name": f"_deleted_{timestamp}_{secrets.token_hex(8)}", + "original_name": p.name, + "deleted": True, + } ) - ) + await session.execute(update(ProjectModel), updates) await session.commit() + logger.info("Deleted projects %s by user %s", projects_names, user.name) async def set_project_members( diff --git a/src/tests/_internal/server/routers/test_projects.py b/src/tests/_internal/server/routers/test_projects.py index d3b042696..8e21957f5 100644 --- a/src/tests/_internal/server/routers/test_projects.py +++ b/src/tests/_internal/server/routers/test_projects.py @@ -472,9 +472,12 @@ async def test_cannot_delete_the_only_project( @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_deletes_projects(self, test_db, session: AsyncSession, client: AsyncClient): + @pytest.mark.parametrize("project_name", ["project1", "a" * 50]) + async def test_deletes_projects( + self, test_db, session: AsyncSession, client: AsyncClient, project_name: str + ): user = await create_user(session=session, global_role=GlobalRole.USER) - project1 = await create_project(session=session, owner=user, name="project1") + project1 = await create_project(session=session, owner=user, name=project_name) await add_project_member( session=session, project=project1, user=user, project_role=ProjectRole.ADMIN ) From 1e5e9992746f29f793b9e203357919cc68744c09 Mon Sep 17 00:00:00 2001 From: Victor Skvortsov Date: Wed, 26 Nov 2025 11:40:55 +0500 Subject: [PATCH 6/8] Delete members when soft-deleting users --- src/dstack/_internal/server/services/users.py | 7 +++-- .../_internal/server/routers/test_users.py | 27 ++++++++++++++++++- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/dstack/_internal/server/services/users.py b/src/dstack/_internal/server/services/users.py index f7573364b..722160df0 100644 --- a/src/dstack/_internal/server/services/users.py +++ b/src/dstack/_internal/server/services/users.py @@ -5,8 +5,8 @@ import uuid from typing import Awaitable, Callable, List, Optional, Tuple +from sqlalchemy import delete, select, update from sqlalchemy import func as safunc -from sqlalchemy import select, update from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import load_only @@ -19,7 +19,7 @@ UserTokenCreds, UserWithCreds, ) -from dstack._internal.server.models import DecryptedString, UserModel +from dstack._internal.server.models import DecryptedString, MemberModel, UserModel from dstack._internal.server.services.permissions import get_default_permissions from dstack._internal.server.utils.routers import error_forbidden from dstack._internal.utils import crypto @@ -200,6 +200,7 @@ async def delete_users( if len(users) != len(usernames): raise ServerClientError("Failed to delete non-existent users") + user_ids = [u.id for u in users] timestamp = str(int(get_current_datetime().timestamp())) updates = [] for u in users: @@ -213,6 +214,8 @@ async def delete_users( } ) await session.execute(update(UserModel), updates) + await session.execute(delete(MemberModel).where(MemberModel.user_id.in_(user_ids))) + # Projects are not deleted automatically if owners are deleted. await session.commit() logger.info("Deleted users %s by user %s", usernames, user.name) diff --git a/src/tests/_internal/server/routers/test_users.py b/src/tests/_internal/server/routers/test_users.py index a5b3edac4..8b8c7ca2a 100644 --- a/src/tests/_internal/server/routers/test_users.py +++ b/src/tests/_internal/server/routers/test_users.py @@ -8,9 +8,11 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from dstack._internal.core.models.projects import ProjectRole from dstack._internal.core.models.runs import JobStatus, RunStatus from dstack._internal.core.models.users import GlobalRole -from dstack._internal.server.models import UserModel +from dstack._internal.server.models import MemberModel, UserModel +from dstack._internal.server.services.projects import add_project_member from dstack._internal.server.testing.common import ( create_job, create_probe, @@ -442,6 +444,29 @@ async def test_deletes_user_with_resources( res = await session.execute(select(UserModel).where(UserModel.name == user.name)) assert res.scalar() is None + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + @pytest.mark.usefixtures("image_config_mock") + async def test_deleting_users_deletes_members( + self, test_db, session: AsyncSession, client: AsyncClient + ): + admin = await create_user(name="admin", session=session) + user = await create_user(name="temp", session=session, global_role=GlobalRole.USER) + project = await create_project(session=session, owner=user) + await add_project_member( + session=session, project=project, user=user, project_role=ProjectRole.USER + ) + response = await client.post( + "/api/users/delete", + headers=get_auth_headers(admin.token), + json={"users": [user.name]}, + ) + assert response.status_code == 200 + res = await session.execute(select(UserModel).where(UserModel.name == user.name)) + assert res.scalar() is None + res = await session.execute(select(MemberModel).where(MemberModel.user_id == user.id)) + assert res.scalar() is None + class TestRefreshToken: @pytest.mark.asyncio From b8d72a262edb7dde98219b54e0013385333d45bc Mon Sep 17 00:00:00 2001 From: Victor Skvortsov Date: Wed, 26 Nov 2025 11:52:42 +0500 Subject: [PATCH 7/8] Forbid deleting admin user via api --- ...dd_usermodel_deleted_and_original_name.py} | 21 +++++++----- .../965760bacf93_add_usermodel_deleted.py | 34 ------------------- src/dstack/_internal/server/services/users.py | 3 ++ 3 files changed, 15 insertions(+), 43 deletions(-) rename src/dstack/_internal/server/migrations/versions/{cc3912316bfd_add_usermodel_original_name_and_.py => 06e977bc61c7_add_usermodel_deleted_and_original_name.py} (72%) delete mode 100644 src/dstack/_internal/server/migrations/versions/965760bacf93_add_usermodel_deleted.py diff --git a/src/dstack/_internal/server/migrations/versions/cc3912316bfd_add_usermodel_original_name_and_.py b/src/dstack/_internal/server/migrations/versions/06e977bc61c7_add_usermodel_deleted_and_original_name.py similarity index 72% rename from src/dstack/_internal/server/migrations/versions/cc3912316bfd_add_usermodel_original_name_and_.py rename to src/dstack/_internal/server/migrations/versions/06e977bc61c7_add_usermodel_deleted_and_original_name.py index 2130d21a0..0f5993531 100644 --- a/src/dstack/_internal/server/migrations/versions/cc3912316bfd_add_usermodel_original_name_and_.py +++ b/src/dstack/_internal/server/migrations/versions/06e977bc61c7_add_usermodel_deleted_and_original_name.py @@ -1,8 +1,8 @@ -"""Add UserModel.original_name and ProjectModel.original_name +"""Add UserModel.deleted and original_name -Revision ID: cc3912316bfd -Revises: 965760bacf93 -Create Date: 2025-11-26 11:04:18.383458 +Revision ID: 06e977bc61c7 +Revises: 7d1ec2b920ac +Create Date: 2025-11-26 11:43:34.825686 """ @@ -10,20 +10,22 @@ from alembic import op # revision identifiers, used by Alembic. -revision = "cc3912316bfd" -down_revision = "965760bacf93" +revision = "06e977bc61c7" +down_revision = "7d1ec2b920ac" branch_labels = None depends_on = None def upgrade() -> None: # ### commands auto generated by Alembic - please adjust! ### - with op.batch_alter_table("projects", schema=None) as batch_op: - batch_op.add_column(sa.Column("original_name", sa.String(length=50), nullable=True)) - with op.batch_alter_table("users", schema=None) as batch_op: + batch_op.add_column( + sa.Column("deleted", sa.Boolean(), server_default=sa.text("0"), nullable=False) + ) batch_op.add_column(sa.Column("original_name", sa.String(length=50), nullable=True)) + with op.batch_alter_table("projects", schema=None) as batch_op: + batch_op.add_column(sa.Column("original_name", sa.String(length=50), nullable=True)) # ### end Alembic commands ### @@ -31,6 +33,7 @@ def downgrade() -> None: # ### commands auto generated by Alembic - please adjust! ### with op.batch_alter_table("users", schema=None) as batch_op: batch_op.drop_column("original_name") + batch_op.drop_column("deleted") with op.batch_alter_table("projects", schema=None) as batch_op: batch_op.drop_column("original_name") diff --git a/src/dstack/_internal/server/migrations/versions/965760bacf93_add_usermodel_deleted.py b/src/dstack/_internal/server/migrations/versions/965760bacf93_add_usermodel_deleted.py deleted file mode 100644 index 0fcf92c16..000000000 --- a/src/dstack/_internal/server/migrations/versions/965760bacf93_add_usermodel_deleted.py +++ /dev/null @@ -1,34 +0,0 @@ -"""Add UserModel.deleted - -Revision ID: 965760bacf93 -Revises: 7d1ec2b920ac -Create Date: 2025-11-25 15:17:05.513712 - -""" - -import sqlalchemy as sa -from alembic import op - -# revision identifiers, used by Alembic. -revision = "965760bacf93" -down_revision = "7d1ec2b920ac" -branch_labels = None -depends_on = None - - -def upgrade() -> None: - # ### commands auto generated by Alembic - please adjust! ### - with op.batch_alter_table("users", schema=None) as batch_op: - batch_op.add_column( - sa.Column("deleted", sa.Boolean(), server_default=sa.text("0"), nullable=False) - ) - - # ### end Alembic commands ### - - -def downgrade() -> None: - # ### commands auto generated by Alembic - please adjust! ### - with op.batch_alter_table("users", schema=None) as batch_op: - batch_op.drop_column("deleted") - - # ### end Alembic commands ### diff --git a/src/dstack/_internal/server/services/users.py b/src/dstack/_internal/server/services/users.py index 722160df0..aed0a23de 100644 --- a/src/dstack/_internal/server/services/users.py +++ b/src/dstack/_internal/server/services/users.py @@ -188,6 +188,9 @@ async def delete_users( user: UserModel, usernames: List[str], ): + if _ADMIN_USERNAME in usernames: + raise ServerClientError("User 'admin' cannot be deleted") + res = await session.execute( select(UserModel) .where( From 62b4c56b4ac50f53ff0b2161613b780b65cdc9ad Mon Sep 17 00:00:00 2001 From: Victor Skvortsov Date: Wed, 26 Nov 2025 12:19:31 +0500 Subject: [PATCH 8/8] Fix server_default --- .../06e977bc61c7_add_usermodel_deleted_and_original_name.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dstack/_internal/server/migrations/versions/06e977bc61c7_add_usermodel_deleted_and_original_name.py b/src/dstack/_internal/server/migrations/versions/06e977bc61c7_add_usermodel_deleted_and_original_name.py index 0f5993531..434c0e286 100644 --- a/src/dstack/_internal/server/migrations/versions/06e977bc61c7_add_usermodel_deleted_and_original_name.py +++ b/src/dstack/_internal/server/migrations/versions/06e977bc61c7_add_usermodel_deleted_and_original_name.py @@ -20,7 +20,7 @@ def upgrade() -> None: # ### commands auto generated by Alembic - please adjust! ### with op.batch_alter_table("users", schema=None) as batch_op: batch_op.add_column( - sa.Column("deleted", sa.Boolean(), server_default=sa.text("0"), nullable=False) + sa.Column("deleted", sa.Boolean(), server_default=sa.false(), nullable=False) ) batch_op.add_column(sa.Column("original_name", sa.String(length=50), nullable=True))