diff --git a/app/models.py b/app/models.py index 166074e8eb3a077e838daf99bc368687f48c9524..a9e75fdd29aac0b85ace6e31fcfbbd99576c9bf0 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 6b469f37684f686cd83a45b4600641239e9c6caf..60860d444c04417cbcd6f4154472b457fc65e874 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 97397c3c87fcb11fad73d19c16cf572f5bb643a2..c46036cd43aa6fa813ba2eaeda5ad97380707f08 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 c3a39eff926ff695d83e206f3ba830505fd109f5..cb1255a31a5610a0557e2cb5662844d1215b14d1 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 a5030fc89badec875302d15a8077ae70edae04f9..bf7f787e6cf09cff6fb2a824142e11705503cdb4 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