diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index b754c1e..de225c6 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -38,6 +38,7 @@ jobs: bazel test //apollo/tests:test_api_updateinfo --test_output=all bazel test //apollo/tests:test_validation --test_output=all bazel test //apollo/tests:test_admin_routes_supported_products --test_output=all + bazel test //apollo/tests:test_admin_users --test_output=all bazel test //apollo/tests:test_api_osv --test_output=all bazel test //apollo/tests:test_database_service --test_output=all bazel test //apollo/tests:test_rh_matcher_activities --test_output=all diff --git a/.gitignore b/.gitignore index 2a814ab..d6adbc4 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,5 @@ node_modules container_data personal_work_dir *.log +CLAUDE.md +temp/ diff --git a/apollo/server/routes/admin_users.py b/apollo/server/routes/admin_users.py index c45c9d6..b30a336 100644 --- a/apollo/server/routes/admin_users.py +++ b/apollo/server/routes/admin_users.py @@ -211,7 +211,6 @@ async def admin_user_delete(request: Request, user_id: int): } ) - # Cannot delete yourself if user.id == request.state.user.id: return templates.TemplateResponse( "error.jinja", { @@ -220,15 +219,6 @@ async def admin_user_delete(request: Request, user_id: int): } ) - # Cannot delete admins - if user.role == "admin": - return templates.TemplateResponse( - "error.jinja", { - "request": request, - "message": "Cannot delete admin users", - } - ) - await user.delete() return RedirectResponse("/admin/users", status_code=302) diff --git a/apollo/server/templates/admin_user.jinja b/apollo/server/templates/admin_user.jinja index efc427e..b9f5ca2 100644 --- a/apollo/server/templates/admin_user.jinja +++ b/apollo/server/templates/admin_user.jinja @@ -42,25 +42,10 @@

Danger Zone

- - - - Delete user - - -

Are you sure you want to delete {{ user.name }}?

- - - Cancel - Delete - - - -
+ +
- - Delete user - diff --git a/apollo/tests/BUILD.bazel b/apollo/tests/BUILD.bazel index 806782f..f8a9725 100644 --- a/apollo/tests/BUILD.bazel +++ b/apollo/tests/BUILD.bazel @@ -71,6 +71,14 @@ py_test( ], ) +py_test( + name = "test_admin_users", + srcs = ["test_admin_users.py"], + deps = [ + "//apollo/server:server_lib", + ], +) + py_test( name = "test_api_osv", srcs = ["test_api_osv.py"], diff --git a/apollo/tests/test_admin_users.py b/apollo/tests/test_admin_users.py new file mode 100644 index 0000000..75070c7 --- /dev/null +++ b/apollo/tests/test_admin_users.py @@ -0,0 +1,104 @@ +""" +Tests for the admin_users delete route. +Mocks the database and template layers to test authorization behavior. +""" + +import asyncio +import sys +import os +import unittest +from unittest.mock import AsyncMock, MagicMock, patch + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "../..")) + +from apollo.server.routes.admin_users import admin_user_delete + + +def _make_request(current_user_id: int) -> MagicMock: + request = MagicMock() + request.state.user = MagicMock() + request.state.user.id = current_user_id + return request + + +def _make_user(user_id: int, role: str = "elevated") -> MagicMock: + user = MagicMock() + user.id = user_id + user.role = role + user.delete = AsyncMock() + return user + + +class TestAdminUserDelete(unittest.TestCase): + """Authorization and behavior tests for admin_user_delete.""" + + def test_delete_user_success_returns_redirect(self): + """Deleting another user redirects to the users list and calls delete().""" + target = _make_user(user_id=7, role="elevated") + request = _make_request(current_user_id=1) + + with patch( + "apollo.server.routes.admin_users.User.get_or_none", + new=AsyncMock(return_value=target), + ): + response = asyncio.run(admin_user_delete(request, user_id=7)) + + target.delete.assert_awaited_once() + self.assertEqual(response.status_code, 302) + self.assertEqual(response.headers["location"], "/admin/users") + + def test_delete_admin_user_is_allowed(self): + """Admin users can be deleted (regression: prior guard was removed).""" + target = _make_user(user_id=7, role="admin") + request = _make_request(current_user_id=1) + + with patch( + "apollo.server.routes.admin_users.User.get_or_none", + new=AsyncMock(return_value=target), + ): + response = asyncio.run(admin_user_delete(request, user_id=7)) + + target.delete.assert_awaited_once() + self.assertEqual(response.status_code, 302) + + def test_cannot_delete_yourself(self): + """Self-deletion is blocked and returns an error response without deleting.""" + target = _make_user(user_id=1, role="admin") + request = _make_request(current_user_id=1) + + with patch( + "apollo.server.routes.admin_users.User.get_or_none", + new=AsyncMock(return_value=target), + ), patch( + "apollo.server.routes.admin_users.templates.TemplateResponse" + ) as mock_template: + mock_template.return_value = MagicMock() + asyncio.run(admin_user_delete(request, user_id=1)) + + target.delete.assert_not_awaited() + mock_template.assert_called_once() + template_name, context = mock_template.call_args.args + self.assertEqual(template_name, "error.jinja") + self.assertEqual(context["message"], "Cannot delete yourself") + + def test_delete_nonexistent_user_returns_error(self): + """Deleting a missing user returns an error response, no delete() call.""" + request = _make_request(current_user_id=1) + + with patch( + "apollo.server.routes.admin_users.User.get_or_none", + new=AsyncMock(return_value=None), + ), patch( + "apollo.server.routes.admin_users.templates.TemplateResponse" + ) as mock_template: + mock_template.return_value = MagicMock() + asyncio.run(admin_user_delete(request, user_id=999)) + + mock_template.assert_called_once() + template_name, context = mock_template.call_args.args + self.assertEqual(template_name, "error.jinja") + self.assertIn("999", context["message"]) + + +if __name__ == "__main__": + unittest.main(verbosity=2)