From 906c320e51361be38fbc487de18387f6330c3f36 Mon Sep 17 00:00:00 2001 From: Jage9 Date: Sat, 28 Feb 2026 20:06:43 -0500 Subject: [PATCH] Add admin delete-account flow with yes/no confirmation --- client/public/version.js | 2 +- client/src/main.ts | 72 +++++++++++++++++++- client/src/network/protocol.ts | 4 +- client/src/state/gameState.ts | 1 + docs/controls.md | 1 + docs/protocol-notes.md | 4 +- server/app/auth_service.py | 29 ++++++++ server/app/models.py | 9 ++- server/app/server.py | 33 +++++++++ server/tests/test_auth_service.py | 12 ++++ server/tests/test_models.py | 6 ++ server/tests/test_server_message_handling.py | 72 ++++++++++++++++++++ 12 files changed, 239 insertions(+), 6 deletions(-) diff --git a/client/public/version.js b/client/public/version.js index 5ab9d44..cacce6e 100644 --- a/client/public/version.js +++ b/client/public/version.js @@ -1,5 +1,5 @@ // Maintainer-controlled web client version. // Format: YYYY.MM.DD Rn (example: 2026.02.20 R2) -window.CHGRID_WEB_VERSION = "2026.02.28 R318"; +window.CHGRID_WEB_VERSION = "2026.02.28 R319"; // Optional display timezone for timestamps. Falls back to America/Detroit if unset/invalid. window.CHGRID_TIME_ZONE = "America/Detroit"; diff --git a/client/src/main.ts b/client/src/main.ts index 268c16d..78278af 100644 --- a/client/src/main.ts +++ b/client/src/main.ts @@ -210,7 +210,8 @@ type AdminUserSummary = { type AdminPendingUserMutation = | { action: 'set_role'; username: string; role: string } | { action: 'ban'; username: string } - | { action: 'unban'; username: string }; + | { action: 'unban'; username: string } + | { action: 'delete_account'; username: string }; type ItemManagementAction = 'delete' | 'transfer'; @@ -332,10 +333,11 @@ let adminRolePermissionIndex = 0; let adminRoleDeleteReplacementIndex = 0; let adminUsers: AdminUserSummary[] = []; let adminUserIndex = 0; -let adminPendingUserAction: 'set_role' | 'ban' | 'unban' | null = null; +let adminPendingUserAction: 'set_role' | 'ban' | 'unban' | 'delete_account' | null = null; let adminSelectedRoleName = ''; let adminSelectedUsername = ''; let adminPendingUserMutation: AdminPendingUserMutation | null = null; +let adminDeleteConfirmIndex = 0; let itemManagementSelectedItemId: string | null = null; let itemManagementOptions: ItemManagementOption[] = []; let itemManagementOptionIndex = 0; @@ -1830,6 +1832,16 @@ function handleAdminActionResult(message: Extract entry.username !== adminPendingUserMutation.username); + if (state.mode === 'adminUserList' && adminPendingUserAction === 'delete_account') { + if (adminUsers.length > 0) { + adminUserIndex = Math.max(0, Math.min(adminUserIndex, adminUsers.length - 1)); + } else { + state.mode = 'adminMenu'; + adminPendingUserAction = null; + } + } } adminPendingUserMutation = null; } @@ -3029,6 +3041,12 @@ function handleAdminMenuModeInput(code: string, key: string): void { adminPendingUserAction = 'unban'; signaling.send({ type: 'admin_users_list', action: 'unban' }); updateStatus('Loading users...'); + return; + } + if (selected.id === 'delete_account') { + adminPendingUserAction = 'delete_account'; + signaling.send({ type: 'admin_users_list', action: 'delete_account' }); + updateStatus('Loading users...'); } return; } @@ -3217,6 +3235,13 @@ function handleAdminUserListModeInput(code: string, key: string): void { adminPendingUserAction = 'unban'; return; } + if (adminPendingUserAction === 'delete_account') { + adminDeleteConfirmIndex = 0; + state.mode = 'adminUserDeleteConfirm'; + updateStatus(`Delete account ${selected.username}? ${YES_NO_OPTIONS[adminDeleteConfirmIndex].label}.`); + audio.sfxUiBlip(); + return; + } return; } if (control.type === 'cancel') { @@ -3254,6 +3279,48 @@ function handleAdminUserRoleSelectModeInput(code: string, key: string): void { } } +/** Handles yes/no confirmation for delete-account admin flow. */ +function handleAdminUserDeleteConfirmModeInput(code: string, key: string): void { + if (!adminSelectedUsername || adminPendingUserAction !== 'delete_account') { + state.mode = 'adminUserList'; + return; + } + const control = handleYesNoMenuInput(code, key, adminDeleteConfirmIndex); + if (control.type === 'move') { + adminDeleteConfirmIndex = control.index; + updateStatus(`Delete account ${adminSelectedUsername}? ${YES_NO_OPTIONS[adminDeleteConfirmIndex].label}.`); + audio.sfxUiBlip(); + return; + } + if (control.type === 'cancel') { + state.mode = 'adminUserList'; + const selected = adminUsers[adminUserIndex]; + if (selected) { + updateStatus(`${selected.username}, ${selected.role}, ${selected.status}.`); + } else { + updateStatus('Select user.'); + } + audio.sfxUiCancel(); + return; + } + if (control.type === 'select') { + const choice = YES_NO_OPTIONS[adminDeleteConfirmIndex]; + if (choice.id === 'no') { + state.mode = 'adminUserList'; + const selected = adminUsers[adminUserIndex]; + if (selected) { + updateStatus(`${selected.username}, ${selected.role}, ${selected.status}.`); + } else { + updateStatus('Select user.'); + } + audio.sfxUiCancel(); + return; + } + adminPendingUserMutation = { action: 'delete_account', username: adminSelectedUsername }; + signaling.send({ type: 'admin_user_delete', username: adminSelectedUsername }); + } +} + /** Handles text edit for new-role creation from admin role list. */ function handleAdminRoleNameEditModeInput(code: string, key: string, ctrlKey: boolean): void { const editAction = getEditSessionAction(code); @@ -3475,6 +3542,7 @@ function setupInputHandlers(): void { adminRoleDeleteReplacement: (currentCode, currentKey) => handleAdminRoleDeleteReplacementModeInput(currentCode, currentKey), adminUserList: (currentCode, currentKey) => handleAdminUserListModeInput(currentCode, currentKey), adminUserRoleSelect: (currentCode, currentKey) => handleAdminUserRoleSelectModeInput(currentCode, currentKey), + adminUserDeleteConfirm: (currentCode, currentKey) => handleAdminUserDeleteConfirmModeInput(currentCode, currentKey), adminRoleNameEdit: (currentCode, currentKey, currentCtrlKey) => handleAdminRoleNameEditModeInput(currentCode, currentKey, currentCtrlKey), itemProperties: (currentCode, currentKey) => itemPropertyEditor.handleItemPropertiesModeInput(currentCode, currentKey), diff --git a/client/src/network/protocol.ts b/client/src/network/protocol.ts index 34a7ffe..52573c1 100644 --- a/client/src/network/protocol.ts +++ b/client/src/network/protocol.ts @@ -315,6 +315,7 @@ export const adminActionResultSchema = z.object({ 'user_set_role', 'user_ban', 'user_unban', + 'user_delete', ]), message: z.string(), }); @@ -355,10 +356,11 @@ export type OutgoingMessage = | { type: 'admin_role_create'; name: string } | { type: 'admin_role_update_permissions'; role: string; permissions: string[] } | { type: 'admin_role_delete'; role: string; replacementRole: string } - | { type: 'admin_users_list'; action?: 'set_role' | 'ban' | 'unban' } + | { type: 'admin_users_list'; action?: 'set_role' | 'ban' | 'unban' | 'delete_account' } | { type: 'admin_user_set_role'; username: string; role: string } | { type: 'admin_user_ban'; username: string } | { type: 'admin_user_unban'; username: string } + | { type: 'admin_user_delete'; username: string } | { type: 'signal'; targetId: string; sdp?: RTCSessionDescriptionInit; ice?: RTCIceCandidateInit } | { type: 'update_position'; x: number; y: number } | { type: 'teleport_complete'; x: number; y: number } diff --git a/client/src/state/gameState.ts b/client/src/state/gameState.ts index 792128d..9d8474b 100644 --- a/client/src/state/gameState.ts +++ b/client/src/state/gameState.ts @@ -48,6 +48,7 @@ export type GameMode = | 'adminRoleDeleteReplacement' | 'adminUserList' | 'adminUserRoleSelect' + | 'adminUserDeleteConfirm' | 'adminRoleNameEdit' | 'pianoUse'; diff --git a/docs/controls.md b/docs/controls.md index d777a5b..28296b0 100644 --- a/docs/controls.md +++ b/docs/controls.md @@ -96,6 +96,7 @@ Applies to effect select, user/item list modes, item selection, item property li - change user role - ban user - unban user + - delete account - In admin role management: - role list includes role user-counts - `Enter` on role opens permission toggles diff --git a/docs/protocol-notes.md b/docs/protocol-notes.md index 9d88bc5..0b1c1d0 100644 --- a/docs/protocol-notes.md +++ b/docs/protocol-notes.md @@ -18,9 +18,10 @@ This is a behavior guide for packet semantics beyond raw schemas. - `admin_role_create`: create role. - `admin_role_update_permissions`: replace one role permission set. - `admin_role_delete`: delete role with replacement role reassignment. -- `admin_users_list`: request user list for admin actions (`action`: `set_role | ban | unban`). +- `admin_users_list`: request user list for admin actions (`action`: `set_role | ban | unban | delete_account`). - `admin_user_set_role`: set target user role. - `admin_user_ban` / `admin_user_unban`: disable/enable user account. +- `admin_user_delete`: permanently delete target account. - `update_position`: client movement intent; server enforces world bounds and movement rate policy. - `teleport_complete`: client signals teleport landing; server rebroadcasts spatial landing cue. - `update_nickname`: nickname change request (server enforces uniqueness). @@ -40,6 +41,7 @@ This is a behavior guide for packet semantics beyond raw schemas. - `admin_roles_list`: role list response payload. - `admin_users_list`: user list response payload. - `admin_action_result`: structured result for admin actions. + - admin mutations include `user_delete` for account deletion. - `welcome`: initial snapshot with users/items plus server UI/world metadata. - `signal`: forwarded WebRTC offer/answer/ICE. - `update_position`, `update_nickname`, `user_left`: presence updates. diff --git a/server/app/auth_service.py b/server/app/auth_service.py index 8e4dd15..8a0e932 100644 --- a/server/app/auth_service.py +++ b/server/app/auth_service.py @@ -438,6 +438,35 @@ class AuthService: self._db_commit() return normalized_username + def delete_user(self, target_username: str, *, actor_user_id: str | None = None) -> str: + """Delete one account and related session/state rows.""" + + normalized_username = self._normalize_username(target_username) + user_row = self._db_fetchone( + """ + SELECT u.id, u.status, r.name AS role_name + FROM users u + JOIN roles r ON r.id = u.role_id + WHERE u.username = ? + """, + (normalized_username,), + ) + if user_row is None: + raise AuthError("User not found.") + user_id = int(user_row["id"]) + current_status = str(user_row["status"]) + current_role = str(user_row["role_name"]) + if current_role == "admin" and current_status == "active" and self._active_admin_count() <= 1: + raise AuthError("Cannot delete the last active admin.") + if actor_user_id is not None and str(user_id) == str(actor_user_id): + if current_role == "admin" and current_status == "active" and self._active_admin_count() <= 1: + raise AuthError("Cannot delete your own account while you are the last active admin.") + self._db_execute("DELETE FROM sessions WHERE user_id = ?", (user_id,)) + self._db_execute("DELETE FROM user_state WHERE user_id = ?", (user_id,)) + self._db_execute("DELETE FROM users WHERE id = ?", (user_id,)) + self._db_commit() + return normalized_username + def get_user_by_id(self, user_id: str) -> AuthUser | None: """Return one user by id with current role and permissions.""" diff --git a/server/app/models.py b/server/app/models.py index e47dfd7..86e6c1b 100644 --- a/server/app/models.py +++ b/server/app/models.py @@ -86,7 +86,7 @@ class AdminRoleDeletePacket(BasePacket): class AdminUsersListPacket(BasePacket): type: Literal["admin_users_list"] - action: Literal["set_role", "ban", "unban"] | None = None + action: Literal["set_role", "ban", "unban", "delete_account"] | None = None class AdminUserSetRolePacket(BasePacket): @@ -105,6 +105,11 @@ class AdminUserUnbanPacket(BasePacket): username: str = Field(min_length=1, max_length=128) +class AdminUserDeletePacket(BasePacket): + type: Literal["admin_user_delete"] + username: str = Field(min_length=1, max_length=128) + + class PingPacket(BasePacket): type: Literal["ping"] clientSentAt: int @@ -187,6 +192,7 @@ ClientPacket = ( | AdminUserSetRolePacket | AdminUserBanPacket | AdminUserUnbanPacket + | AdminUserDeletePacket | PingPacket | ItemAddPacket | ItemPickupPacket @@ -449,5 +455,6 @@ class AdminActionResultPacket(BasePacket): "user_set_role", "user_ban", "user_unban", + "user_delete", ] message: str diff --git a/server/app/server.py b/server/app/server.py index d31b3ae..4add5e3 100644 --- a/server/app/server.py +++ b/server/app/server.py @@ -63,6 +63,7 @@ from .models import ( AdminRolesListPacket, AdminRolesListResultPacket, AdminUserBanPacket, + AdminUserDeletePacket, AdminUserSetRolePacket, AdminUserUnbanPacket, AdminUsersListPacket, @@ -133,6 +134,7 @@ ADMIN_MENU_ACTION_DEFINITIONS: tuple[dict[str, str], ...] = ( {"id": "change_user_role", "label": "Change user role", "permission": "user.change_role"}, {"id": "ban_user", "label": "Ban user", "permission": "user.ban_unban"}, {"id": "unban_user", "label": "Unban user", "permission": "user.ban_unban"}, + {"id": "delete_account", "label": "Delete account", "permission": "account.delete.any"}, ) @@ -1671,6 +1673,7 @@ class SignalingServer: "user_set_role", "user_ban", "user_unban", + "user_delete", ], message: str, ) -> None: @@ -1831,6 +1834,7 @@ class SignalingServer: AdminUserSetRolePacket, AdminUserBanPacket, AdminUserUnbanPacket, + AdminUserDeletePacket, ), ): return False @@ -1861,6 +1865,7 @@ class SignalingServer: if not ( self._client_has_permission(client, "user.change_role") or self._client_has_permission(client, "user.ban_unban") + or self._client_has_permission(client, "account.delete.any") ): await deny("user_set_role", "Not authorized.") return True @@ -2012,6 +2017,34 @@ class SignalingServer: ) return True + if isinstance(packet, AdminUserDeletePacket): + if not self._client_has_permission(client, "account.delete.any"): + await deny("user_delete", "Not authorized.") + return True + target_id = self.auth_service.get_user_id_by_username(packet.username) + try: + username = self.auth_service.delete_user(packet.username, actor_user_id=client.user_id) + except AuthError as exc: + await deny("user_delete", str(exc)) + return True + if target_id: + for active in list(self.clients.values()): + if active.user_id != target_id: + continue + await self._send( + active.websocket, + AuthResultPacket(type="auth_result", ok=False, message="Account deleted."), + ) + await active.websocket.close() + LOGGER.info("user deleted actor=%s target=%s", client.user_id, username) + await self._send_admin_action_result( + client, + ok=True, + action="user_delete", + message=f"Deleted account {username}.", + ) + return True + return True async def _handle_message(self, client: ClientConnection, raw_message: str) -> None: diff --git a/server/tests/test_auth_service.py b/server/tests/test_auth_service.py index ee840c4..b0a7635 100644 --- a/server/tests/test_auth_service.py +++ b/server/tests/test_auth_service.py @@ -87,3 +87,15 @@ def test_update_role_permissions_rejects_admin(tmp_path: Path) -> None: service.update_role_permissions("admin", ["chat.send"]) finally: service.close() + + +def test_delete_user_removes_account(tmp_path: Path) -> None: + service = make_auth_service(tmp_path) + try: + service.register("alpha", "password99") + deleted = service.delete_user("alpha") + assert deleted == "alpha" + with pytest.raises(AuthError): + service.login("alpha", "password99") + finally: + service.close() diff --git a/server/tests/test_models.py b/server/tests/test_models.py index fbe80f3..2879612 100644 --- a/server/tests/test_models.py +++ b/server/tests/test_models.py @@ -36,3 +36,9 @@ def test_item_transfer_packet_validates() -> None: adapter = TypeAdapter(ClientPacket) packet = adapter.validate_python({"type": "item_transfer", "itemId": "i1", "targetId": "u2"}) assert packet.type == "item_transfer" + + +def test_admin_user_delete_packet_validates() -> None: + adapter = TypeAdapter(ClientPacket) + packet = adapter.validate_python({"type": "admin_user_delete", "username": "alpha"}) + assert packet.type == "admin_user_delete" diff --git a/server/tests/test_server_message_handling.py b/server/tests/test_server_message_handling.py index 94efa64..527f114 100644 --- a/server/tests/test_server_message_handling.py +++ b/server/tests/test_server_message_handling.py @@ -435,6 +435,78 @@ async def test_item_transfer_rejects_when_not_authorized(monkeypatch: pytest.Mon assert "not authorized" in result.message.lower() +@pytest.mark.asyncio +async def test_admin_user_delete_requires_permission(monkeypatch: pytest.MonkeyPatch) -> None: + server = SignalingServer("127.0.0.1", 8765, None, None) + ws = _fake_ws() + client = ClientConnection( + websocket=ws, + id="u1", + nickname="Tester", + authenticated=True, + user_id="1", + username="tester", + permissions={"user.ban_unban"}, + ) + server.clients[ws] = client + + send_payloads: list[object] = [] + + async def fake_send(websocket: ServerConnection, packet: object) -> None: + send_payloads.append(packet) + + monkeypatch.setattr(server, "_send", fake_send) + + await server._handle_message(client, json.dumps({"type": "admin_user_delete", "username": "alpha"})) + + assert send_payloads + packet = send_payloads[-1] + assert getattr(packet, "type", "") == "admin_action_result" + assert packet.ok is False + assert packet.action == "user_delete" + assert "not authorized" in packet.message.lower() + + +@pytest.mark.asyncio +async def test_admin_user_delete_calls_auth_service(monkeypatch: pytest.MonkeyPatch) -> None: + server = SignalingServer("127.0.0.1", 8765, None, None) + ws = _fake_ws() + client = ClientConnection( + websocket=ws, + id="u1", + nickname="Tester", + authenticated=True, + user_id="1", + username="tester", + permissions={"account.delete.any"}, + ) + server.clients[ws] = client + + send_payloads: list[object] = [] + calls: list[tuple[str, str | None]] = [] + + async def fake_send(websocket: ServerConnection, packet: object) -> None: + send_payloads.append(packet) + + monkeypatch.setattr(server, "_send", fake_send) + monkeypatch.setattr(server.auth_service, "get_user_id_by_username", lambda _username: None) + + def fake_delete_user(username: str, *, actor_user_id: str | None = None) -> str: + calls.append((username, actor_user_id)) + return username + + monkeypatch.setattr(server.auth_service, "delete_user", fake_delete_user) + + await server._handle_message(client, json.dumps({"type": "admin_user_delete", "username": "alpha"})) + + assert calls == [("alpha", "1")] + assert send_payloads + packet = send_payloads[-1] + assert getattr(packet, "type", "") == "admin_action_result" + assert packet.ok is True + assert packet.action == "user_delete" + + @pytest.mark.asyncio async def test_broadcast_fanout_is_concurrent(monkeypatch: pytest.MonkeyPatch) -> None: server = SignalingServer("127.0.0.1", 8765, None, None)