From a6811ba962d6c7d79bb283385610847a3783973c Mon Sep 17 00:00:00 2001
From: Benjamin Bertrand <benjamin.bertrand@esss.se>
Date: Thu, 9 May 2019 22:28:20 +0200
Subject: [PATCH] Allow users to delete their host
- admin can delete any hosts
- normal users can delete hosts they created
Note that still only admin users can delete hosts via the API (more dangerous
operation as a script could delete many hosts).
JIRA INFRA-1018 #action In Progress
---
app/models.py | 12 ++++++++
app/network/views.py | 10 ++++++-
app/templates/network/view_host.html | 2 --
docs/network.rst | 2 +-
tests/functional/test_web.py | 45 ++++++++++++++++++++++++++++
5 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/app/models.py b/app/models.py
index 166074e..a9e75fd 100644
--- a/app/models.py
+++ b/app/models.py
@@ -324,6 +324,18 @@ class User(db.Model, UserMixin):
in current_app.config["ALLOWED_SET_BOOT_PROFILE_DOMAINS"]
)
+ def can_delete_host(self, host):
+ """Return True if the user can delete the host
+
+ - admin users can delete any host
+ - normal users must be creator of the host
+ - LOGIN_DISABLED can be set to True to turn off authentication check when testing.
+ In this case, this function always returns True.
+ """
+ if current_app.config.get("LOGIN_DISABLED") or self.is_admin:
+ return True
+ return self.id == host.user.id
+
def favorite_attributes(self):
"""Return all user's favorite attributes"""
favorites_list = [
diff --git a/app/network/views.py b/app/network/views.py
index 6b469f3..60860d4 100644
--- a/app/network/views.py
+++ b/app/network/views.py
@@ -127,15 +127,23 @@ def create_host():
@bp.route("/hosts/delete", methods=["POST"])
-@login_groups_accepted("admin")
+@login_required
def delete_host():
host = models.Host.query.get_or_404(request.form["host_id"])
+ if not current_user.can_delete_host(host):
+ abort(403)
# Deleting the host will also delete all
# associated interfaces due to the cascade delete option
# defined on the model
db.session.delete(host)
db.session.commit()
flash(f"Host {host.name} has been deleted", "success")
+ if host.device_type.name == "VirtualMachine":
+ flash(
+ "Note that the host was only removed from CSEntry. "
+ "To delete a running VM, please contact an administrator.",
+ "info",
+ )
return redirect(url_for("network.list_hosts"))
diff --git a/app/templates/network/view_host.html b/app/templates/network/view_host.html
index 97397c3..c46036c 100644
--- a/app/templates/network/view_host.html
+++ b/app/templates/network/view_host.html
@@ -27,7 +27,6 @@
<div class="col-sm-10">
{{ host.fqdn }}
</div>
- {% if current_user.is_admin %}
<div class="col-sm-2 text-right">
<form method="POST" action="/network/hosts/delete">
<input id="host_id" name="host_id" type="hidden" value="{{ host.id }}">
@@ -35,7 +34,6 @@
"Are you sure you want to delete the host %s?" | format(host.name)) }}
</form>
</div>
- {% endif %}
</div>
</dd>
<dt class="col-sm-3">Device Type</dt>
diff --git a/docs/network.rst b/docs/network.rst
index c3a39ef..cb1255a 100644
--- a/docs/network.rst
+++ b/docs/network.rst
@@ -105,7 +105,7 @@ If you need more than one interface, you should go to the *Add interface* pane f
Delete a host
-------------
-Note that this is restricted to admin users.
+Normal users can only delete hosts they created (admin can delete any hosts).
To delete a host, just click on the **trash** icon next to the hostname on the *View host* page.
.. image:: _static/network/view_host_delete.png
diff --git a/tests/functional/test_web.py b/tests/functional/test_web.py
index a5030fc..bf7f787 100644
--- a/tests/functional/test_web.py
+++ b/tests/functional/test_web.py
@@ -37,6 +37,13 @@ def logged_rw_client(client):
logout(client)
+@pytest.fixture
+def logged_admin_client(client):
+ login(client, "admin", "adminpasswd")
+ yield client
+ logout(client)
+
+
@pytest.fixture
def no_login_check_client(request, app):
app.config["LOGIN_DISABLED"] = True
@@ -497,3 +504,41 @@ def test_create_vm(
f"/network/hosts/view/{vioc_lab.name}", data=form, follow_redirects=True
)
check_vm_creation_response(response, success=False)
+
+
+def test_delete_host_as_admin(logged_admin_client, host_factory, user_factory):
+ # admin can delete any host
+ admin = models.User.query.filter_by(username="admin").first()
+ user1 = user_factory(username="user1")
+ host1 = host_factory(name="host1", user=admin)
+ host2 = host_factory(name="host2", user=user1)
+ assert len(models.Host.query.all()) == 2
+ response = logged_admin_client.post(
+ "/network/hosts/delete", data={"host_id": host1.id}
+ )
+ assert response.status_code == 302
+ response = logged_admin_client.post(
+ "/network/hosts/delete", data={"host_id": host2.id}
+ )
+ assert response.status_code == 302
+ assert len(models.Host.query.all()) == 0
+
+
+def test_delete_host_as_normal_user(logged_rw_client, host_factory, user_factory):
+ # a normal user can only delete its own hosts
+ user_rw = models.User.query.filter_by(username="user_rw").first()
+ user1 = user_factory(username="user1")
+ host1 = host_factory(name="host1", user=user_rw)
+ host2 = host_factory(name="host2", user=user1)
+ assert len(models.Host.query.all()) == 2
+ # user_rw can delete its host
+ response = logged_rw_client.post(
+ "/network/hosts/delete", data={"host_id": host1.id}
+ )
+ assert response.status_code == 302
+ # user_rw can't delete host owned by user1
+ response = logged_rw_client.post(
+ "/network/hosts/delete", data={"host_id": host2.id}
+ )
+ assert response.status_code == 403
+ assert len(models.Host.query.all()) == 1
--
GitLab