Skip to content

Commit ea6a5e6

Browse files
authored
Implement users soft-deletion (#3326)
* Add test_deletes_user_with_resources * Return 400 if deleting non-existent users * Implement users soft-delete * Fix deleting users with long names * Fix deleting projects with long names * Delete members when soft-deleting users * Forbid deleting admin user via api * Fix server_default
1 parent 0255f94 commit ea6a5e6

File tree

7 files changed

+251
-35
lines changed

7 files changed

+251
-35
lines changed
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
"""Add UserModel.deleted and original_name
2+
3+
Revision ID: 06e977bc61c7
4+
Revises: 7d1ec2b920ac
5+
Create Date: 2025-11-26 11:43:34.825686
6+
7+
"""
8+
9+
import sqlalchemy as sa
10+
from alembic import op
11+
12+
# revision identifiers, used by Alembic.
13+
revision = "06e977bc61c7"
14+
down_revision = "7d1ec2b920ac"
15+
branch_labels = None
16+
depends_on = None
17+
18+
19+
def upgrade() -> None:
20+
# ### commands auto generated by Alembic - please adjust! ###
21+
with op.batch_alter_table("users", schema=None) as batch_op:
22+
batch_op.add_column(
23+
sa.Column("deleted", sa.Boolean(), server_default=sa.false(), nullable=False)
24+
)
25+
batch_op.add_column(sa.Column("original_name", sa.String(length=50), nullable=True))
26+
27+
with op.batch_alter_table("projects", schema=None) as batch_op:
28+
batch_op.add_column(sa.Column("original_name", sa.String(length=50), nullable=True))
29+
# ### end Alembic commands ###
30+
31+
32+
def downgrade() -> None:
33+
# ### commands auto generated by Alembic - please adjust! ###
34+
with op.batch_alter_table("users", schema=None) as batch_op:
35+
batch_op.drop_column("original_name")
36+
batch_op.drop_column("deleted")
37+
38+
with op.batch_alter_table("projects", schema=None) as batch_op:
39+
batch_op.drop_column("original_name")
40+
41+
# ### end Alembic commands ###

src/dstack/_internal/server/models.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,9 @@ class UserModel(BaseModel):
190190
global_role: Mapped[GlobalRole] = mapped_column(EnumAsString(GlobalRole, 100))
191191
# deactivated users cannot access API
192192
active: Mapped[bool] = mapped_column(Boolean, default=True)
193+
deleted: Mapped[bool] = mapped_column(Boolean, server_default=false())
194+
# `original_name` stores the name of a deleted user, while `name` is changed to a unique generated value.
195+
original_name: Mapped[Optional[str]] = mapped_column(String(50), nullable=True)
193196

194197
# SSH keys can be null for users created before 0.19.33.
195198
# Keys for those users are being gradually generated on /get_my_user calls.
@@ -212,8 +215,10 @@ class ProjectModel(BaseModel):
212215
)
213216
name: Mapped[str] = mapped_column(String(50), unique=True)
214217
created_at: Mapped[datetime] = mapped_column(NaiveDateTime, default=get_current_datetime)
215-
deleted: Mapped[bool] = mapped_column(Boolean, default=False)
216218
is_public: Mapped[bool] = mapped_column(Boolean, default=False)
219+
deleted: Mapped[bool] = mapped_column(Boolean, default=False)
220+
# `original_name` stores the name of a deleted project, while `name` is changed to a unique generated value.
221+
original_name: Mapped[Optional[str]] = mapped_column(String(50), nullable=True)
217222

218223
owner_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("users.id", ondelete="CASCADE"))
219224
owner: Mapped[UserModel] = relationship(lazy="joined")

src/dstack/_internal/server/services/projects.py

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import secrets
12
import uuid
23
from typing import Awaitable, Callable, List, Optional, Tuple
34

@@ -194,28 +195,36 @@ async def delete_projects(
194195
raise ServerClientError("Cannot delete the only project")
195196

196197
res = await session.execute(
197-
select(ProjectModel.id).where(ProjectModel.name.in_(projects_names))
198+
select(ProjectModel)
199+
.where(
200+
ProjectModel.name.in_(projects_names),
201+
ProjectModel.deleted == False,
202+
)
203+
.options(load_only(ProjectModel.id, ProjectModel.name))
198204
)
199-
project_ids = res.scalars().all()
200-
if len(project_ids) != len(projects_names):
205+
projects = res.scalars().all()
206+
if len(projects) != len(projects_names):
201207
raise ServerClientError("Failed to delete non-existent projects")
202208

203-
for project_id in project_ids:
209+
for p in projects:
204210
# FIXME: The checks are not under lock,
205211
# so there can be dangling active resources due to race conditions.
206-
await _check_project_has_active_resources(session=session, project_id=project_id)
212+
await _check_project_has_active_resources(session=session, project_id=p.id)
207213

208214
timestamp = str(int(get_current_datetime().timestamp()))
209-
new_project_name = "_deleted_" + timestamp + ProjectModel.name
210-
await session.execute(
211-
update(ProjectModel)
212-
.where(ProjectModel.name.in_(projects_names))
213-
.values(
214-
deleted=True,
215-
name=new_project_name,
215+
updates = []
216+
for p in projects:
217+
updates.append(
218+
{
219+
"id": p.id,
220+
"name": f"_deleted_{timestamp}_{secrets.token_hex(8)}",
221+
"original_name": p.name,
222+
"deleted": True,
223+
}
216224
)
217-
)
225+
await session.execute(update(ProjectModel), updates)
218226
await session.commit()
227+
logger.info("Deleted projects %s by user %s", projects_names, user.name)
219228

220229

221230
async def set_project_members(
@@ -244,12 +253,16 @@ async def set_project_members(
244253
}
245254
if new_admins_members != current_admins_members:
246255
raise ForbiddenError("Access denied: changing project admins")
256+
247257
# FIXME: potentially long write transaction
248258
# clear_project_members() issues DELETE without commit
249259
await clear_project_members(session=session, project=project)
250260
names = [m.username for m in members]
251261
res = await session.execute(
252-
select(UserModel).where((UserModel.name.in_(names)) | (UserModel.email.in_(names)))
262+
select(UserModel).where(
263+
(UserModel.name.in_(names)) | (UserModel.email.in_(names)),
264+
UserModel.deleted == False,
265+
)
253266
)
254267
users = res.scalars().all()
255268
username_to_user = {user.name: user for user in users}
@@ -311,7 +324,10 @@ async def add_project_members(
311324
raise ForbiddenError("Access denied: can only join public projects as user role")
312325

313326
res = await session.execute(
314-
select(UserModel).where((UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames)))
327+
select(UserModel).where(
328+
(UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames)),
329+
UserModel.deleted == False,
330+
)
315331
)
316332
users_found = res.scalars().all()
317333

@@ -700,7 +716,10 @@ async def remove_project_members(
700716
raise ForbiddenError("Access denied: insufficient permissions to remove members")
701717

702718
res = await session.execute(
703-
select(UserModel).where((UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames)))
719+
select(UserModel).where(
720+
(UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames)),
721+
UserModel.deleted == False,
722+
)
704723
)
705724
users_found = res.scalars().all()
706725

src/dstack/_internal/server/services/users.py

Lines changed: 62 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
import hashlib
22
import os
33
import re
4+
import secrets
45
import uuid
56
from typing import Awaitable, Callable, List, Optional, Tuple
67

78
from sqlalchemy import delete, select, update
89
from sqlalchemy import func as safunc
910
from sqlalchemy.ext.asyncio import AsyncSession
11+
from sqlalchemy.orm import load_only
1012

1113
from dstack._internal.core.errors import ResourceExistsError, ServerClientError
1214
from dstack._internal.core.models.users import (
@@ -17,11 +19,11 @@
1719
UserTokenCreds,
1820
UserWithCreds,
1921
)
20-
from dstack._internal.server.models import DecryptedString, UserModel
22+
from dstack._internal.server.models import DecryptedString, MemberModel, UserModel
2123
from dstack._internal.server.services.permissions import get_default_permissions
2224
from dstack._internal.server.utils.routers import error_forbidden
2325
from dstack._internal.utils import crypto
24-
from dstack._internal.utils.common import run_async
26+
from dstack._internal.utils.common import get_current_datetime, get_or_error, run_async
2527
from dstack._internal.utils.logging import get_logger
2628

2729
logger = get_logger(__name__)
@@ -53,8 +55,12 @@ async def list_users_for_user(
5355

5456
async def list_all_users(
5557
session: AsyncSession,
58+
include_deleted: bool = False,
5659
) -> List[User]:
57-
res = await session.execute(select(UserModel))
60+
filters = []
61+
if not include_deleted:
62+
filters.append(UserModel.deleted == False)
63+
res = await session.execute(select(UserModel).where(*filters))
5864
user_models = res.scalars().all()
5965
user_models = sorted(user_models, key=lambda u: u.created_at)
6066
return [user_model_to_user(u) for u in user_models]
@@ -116,7 +122,10 @@ async def update_user(
116122
) -> UserModel:
117123
await session.execute(
118124
update(UserModel)
119-
.where(UserModel.name == username)
125+
.where(
126+
UserModel.name == username,
127+
UserModel.deleted == False,
128+
)
120129
.values(
121130
global_role=global_role,
122131
email=email,
@@ -138,7 +147,10 @@ async def refresh_ssh_key(
138147
private_bytes, public_bytes = await run_async(crypto.generate_rsa_key_pair_bytes, username)
139148
await session.execute(
140149
update(UserModel)
141-
.where(UserModel.name == username)
150+
.where(
151+
UserModel.name == username,
152+
UserModel.deleted == False,
153+
)
142154
.values(
143155
ssh_private_key=private_bytes.decode(),
144156
ssh_public_key=public_bytes.decode(),
@@ -158,7 +170,10 @@ async def refresh_user_token(
158170
new_token = str(uuid.uuid4())
159171
await session.execute(
160172
update(UserModel)
161-
.where(UserModel.name == username)
173+
.where(
174+
UserModel.name == username,
175+
UserModel.deleted == False,
176+
)
162177
.values(
163178
token=DecryptedString(plaintext=new_token),
164179
token_hash=get_token_hash(new_token),
@@ -173,7 +188,37 @@ async def delete_users(
173188
user: UserModel,
174189
usernames: List[str],
175190
):
176-
await session.execute(delete(UserModel).where(UserModel.name.in_(usernames)))
191+
if _ADMIN_USERNAME in usernames:
192+
raise ServerClientError("User 'admin' cannot be deleted")
193+
194+
res = await session.execute(
195+
select(UserModel)
196+
.where(
197+
UserModel.name.in_(usernames),
198+
UserModel.deleted == False,
199+
)
200+
.options(load_only(UserModel.id, UserModel.name))
201+
)
202+
users = res.scalars().all()
203+
if len(users) != len(usernames):
204+
raise ServerClientError("Failed to delete non-existent users")
205+
206+
user_ids = [u.id for u in users]
207+
timestamp = str(int(get_current_datetime().timestamp()))
208+
updates = []
209+
for u in users:
210+
updates.append(
211+
{
212+
"id": u.id,
213+
"name": f"_deleted_{timestamp}_{secrets.token_hex(8)}",
214+
"original_name": u.name,
215+
"deleted": True,
216+
"active": False,
217+
}
218+
)
219+
await session.execute(update(UserModel), updates)
220+
await session.execute(delete(MemberModel).where(MemberModel.user_id.in_(user_ids)))
221+
# Projects are not deleted automatically if owners are deleted.
177222
await session.commit()
178223
logger.info("Deleted users %s by user %s", usernames, user.name)
179224

@@ -183,7 +228,7 @@ async def get_user_model_by_name(
183228
username: str,
184229
ignore_case: bool = False,
185230
) -> Optional[UserModel]:
186-
filters = []
231+
filters = [UserModel.deleted == False]
187232
if ignore_case:
188233
filters.append(safunc.lower(UserModel.name) == safunc.lower(username))
189234
else:
@@ -192,9 +237,14 @@ async def get_user_model_by_name(
192237
return res.scalar()
193238

194239

195-
async def get_user_model_by_name_or_error(session: AsyncSession, username: str) -> UserModel:
196-
res = await session.execute(select(UserModel).where(UserModel.name == username))
197-
return res.scalar_one()
240+
async def get_user_model_by_name_or_error(
241+
session: AsyncSession,
242+
username: str,
243+
ignore_case: bool = False,
244+
) -> UserModel:
245+
return get_or_error(
246+
await get_user_model_by_name(session=session, username=username, ignore_case=ignore_case)
247+
)
198248

199249

200250
async def log_in_with_token(session: AsyncSession, token: str) -> Optional[UserModel]:
@@ -203,6 +253,7 @@ async def log_in_with_token(session: AsyncSession, token: str) -> Optional[UserM
203253
select(UserModel).where(
204254
UserModel.token_hash == token_hash,
205255
UserModel.active == True,
256+
UserModel.deleted == False,
206257
)
207258
)
208259
user = res.scalar()

src/dstack/_internal/server/testing/common.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ async def create_user(
135135
ssh_public_key: Optional[str] = None,
136136
ssh_private_key: Optional[str] = None,
137137
active: bool = True,
138+
deleted: bool = False,
138139
) -> UserModel:
139140
if token is None:
140141
token = str(uuid.uuid4())
@@ -148,6 +149,7 @@ async def create_user(
148149
ssh_public_key=ssh_public_key,
149150
ssh_private_key=ssh_private_key,
150151
active=active,
152+
deleted=deleted,
151153
)
152154
session.add(user)
153155
await session.commit()

src/tests/_internal/server/routers/test_projects.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,9 +472,12 @@ async def test_cannot_delete_the_only_project(
472472

473473
@pytest.mark.asyncio
474474
@pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True)
475-
async def test_deletes_projects(self, test_db, session: AsyncSession, client: AsyncClient):
475+
@pytest.mark.parametrize("project_name", ["project1", "a" * 50])
476+
async def test_deletes_projects(
477+
self, test_db, session: AsyncSession, client: AsyncClient, project_name: str
478+
):
476479
user = await create_user(session=session, global_role=GlobalRole.USER)
477-
project1 = await create_project(session=session, owner=user, name="project1")
480+
project1 = await create_project(session=session, owner=user, name=project_name)
478481
await add_project_member(
479482
session=session, project=project1, user=user, project_role=ProjectRole.ADMIN
480483
)

0 commit comments

Comments
 (0)