Repository: openwisp/openwisp-utils (language: python)

Issue:
[bug] The menu can be shown also in non admin templates
If a user is logged in and is browsing any django template which is not part of the admin, the menu can be shown anyway.

We should find a way to avoid the menu being showed in templates that do not belong to the admin.


Discussion / hints:

Fix patch:
diff --git a/openwisp_utils/admin_theme/templates/admin/base.html b/openwisp_utils/admin_theme/templates/admin/base.html
index 967bc101..4b0359f2 100644
--- a/openwisp_utils/admin_theme/templates/admin/base.html
+++ b/openwisp_utils/admin_theme/templates/admin/base.html
@@ -23,25 +23,25 @@
   data-admin-utc-offset="{% now "Z" %}">
 
 <!-- Container -->
-<div id="container" class="toggle-menu{% if not user.is_authenticated %} no-auth{% endif %}" >
-  <div class="menu-backdrop"></div>
+<div id="container" class="toggle-menu {% if not user.is_authenticated or not user.is_staff %}no-auth{% endif %}" >
   {% if not is_popup %}
+    <div class="menu-backdrop"></div>
     <div id="menu">
       <div class="menu-bar">
         {% block menu-bar %}{% endblock %}
       </div>
-      {% if user.is_authenticated  %}
+      {% if user.is_authenticated and user.is_staff %}
       {% block menu %}{% endblock %}
       {% endif %}
     </div>
   {% endif %}
-  <div id="main-content" {% if not user.is_authenticated %}class="m-0"{% endif %}>
+  <div id="main-content" {% if not user.is_staff or not user.is_authenticated %}class="m-0"{% endif %}>
     {% if not is_popup %}
     <!-- Header -->
     {% block header %}
       <div id="header">
         {% block nav-global %}{% endblock %}
-        {% if user.is_authenticated %}
+        {% if user.is_authenticated and user.is_staff %}
           <div id="ow-user-tools">
           {% block user-tools %}{% endblock  %}
           </div>
diff --git a/openwisp_utils/admin_theme/templates/admin/base_site.html b/openwisp_utils/admin_theme/templates/admin/base_site.html
index 0ad04cda..a6a5bf0d 100644
--- a/openwisp_utils/admin_theme/templates/admin/base_site.html
+++ b/openwisp_utils/admin_theme/templates/admin/base_site.html
@@ -67,7 +67,7 @@ <h1 id="site-name"><a href="{% url 'admin:index' %}">{{ site_header }}</a></h1>
     if we put it in extrahead it will be
     loaded before jQuery is loaded
 {% endcomment %}
-{% if user.is_authenticated %}
+{% if user.is_authenticated and user.is_staff %}
     <script src="{% static "admin/js/menu.js" %}"></script>
 {% endif %}
 {% if OPENWISP_ADMIN_THEME_JS %}


Test patch:
diff --git a/tests/test_project/templates/test_project/menu_test.html b/tests/test_project/templates/test_project/menu_test.html
new file mode 100644
index 00000000..05eec332
--- /dev/null
+++ b/tests/test_project/templates/test_project/menu_test.html
@@ -0,0 +1,10 @@
+{% extends 'base.html' %}
+
+{% block content %}
+<h1> Menu Render Test View </h1>
+<p>
+  <strong>User:</strong> {{ request.user }} </br>
+  <strong>Is user authenticated?:</strong> {{ request.user.is_authenticated }} </br>
+  <strong>Does user has staff privileges?:</strong> {{ request.user.is_staff }} </br>
+</p>
+{% endblock %}
diff --git a/tests/test_project/tests/test_admin.py b/tests/test_project/tests/test_admin.py
index 96979928..71622446 100644
--- a/tests/test_project/tests/test_admin.py
+++ b/tests/test_project/tests/test_admin.py
@@ -156,6 +156,40 @@ def test_menu_items_visibility(self):
             response = self.client.get('/admin/login/')
             self.assertNotContains(response, 'id="nav"')
 
+    def test_menu_on_non_admin_page(self):
+        url = reverse('menu-test-view')
+        with self.subTest('Test menu visibility when user is a staff'):
+            user = User.objects.create(
+                username='tester',
+                password='pass',
+                email='email@email',
+                is_staff=True,
+                is_superuser=False,
+            )
+            self.client.force_login(user)
+            response = self.client.get(url)
+            self.assertContains(response, 'class="nav"')
+            self.assertContains(
+                response, '<strong>Does user has staff privileges?:</strong> True'
+            )
+            self.client.logout()
+
+        with self.subTest('Test menu visibility when user is not staff'):
+            # Test with non staff user
+            user = User.objects.create(
+                username='tester2',
+                password='pass',
+                email='email@email',
+                is_staff=False,
+                is_superuser=False,
+            )
+            self.client.force_login(user)
+            response = self.client.get(url)
+            self.assertNotContains(response, 'class="nav"')
+            self.assertContains(
+                response, '<strong>Does user has staff privileges?:</strong> False'
+            )
+
     def test_uuid_field_in_change(self):
         p = Project.objects.create(name='test-project')
         path = reverse('admin:test_project_project_change', args=[p.pk])
diff --git a/tests/test_project/tests/test_selenium.py b/tests/test_project/tests/test_selenium.py
index 095b8c56..b86937bd 100644
--- a/tests/test_project/tests/test_selenium.py
+++ b/tests/test_project/tests/test_selenium.py
@@ -55,7 +55,10 @@ def _test_menu_state(self, open, is_narrow=False):
                 menu_toggle.get_attribute('title'), self.config['maximize_menu']
             )
             container_class = container.get_attribute('class')
-            self.assertEqual(container_class, 'toggle-menu')
+            self.assertIn(
+                'toggle-menu',
+                container_class,
+            )
 
     def _test_login_and_logout_page(self, is_logged_in=True):
         if not is_logged_in:
diff --git a/tests/urls.py b/tests/urls.py
index 1f8c73cb..44d33ed6 100644
--- a/tests/urls.py
+++ b/tests/urls.py
@@ -1,8 +1,15 @@
 from django.contrib import admin
 from django.urls import include, path
+from django.views.generic import TemplateView
 
 urlpatterns = [
     path('admin/', admin.site.urls),
     path('api/v1/', include('openwisp_utils.api.urls')),
     path('api/v1/', include('test_project.api.urls')),
+    # Only used for tests
+    path(
+        'menu-test-view/',
+        TemplateView.as_view(template_name='test_project/menu_test.html'),
+        name='menu-test-view',
+    ),
 ]