diff --git a/AGENTS.md b/AGENTS.md index ce9c404..1ae8e7e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,7 +4,7 @@ `netbox-floorplan-plugin` is a NetBox plugin that adds spatial floorplans to sites and locations. A **floorplan** is a canvas on which racks and unracked devices are placed, so that the drawing reflects where equipment physically sits within a room. Each placed object references the NetBox record it represents, and is reconciled against NetBox every time the floorplan is viewed. -The plugin has real models and migrations, UI views, a REST API, and a substantial browser-side editor built on Fabric.js. It has **no configuration parameters** — nothing goes in `PLUGINS_CONFIG` — and no GraphQL API. The supported NetBox range is in `COMPATIBILITY.md`. +The plugin has real models and migrations, UI views, a REST API, and a substantial browser-side editor built on Fabric.js. It has no GraphQL API. It declares one optional `PLUGINS_CONFIG` setting, `top_level_menu` (see `netbox_floorplan/navigation.py`) — resist adding others for things that belong on the model. The supported NetBox range is in `COMPATIBILITY.md`. ## Tech Stack @@ -30,15 +30,18 @@ The plugin declares **no** `install_requires`. Defer version pins to `setup.py` ```text . ├── netbox_floorplan/ -│ ├── __init__.py — FloorplanConfig (PluginConfig): NetBox min/max version. -│ │ No default_settings; the plugin has no settings. +│ ├── __init__.py — FloorplanConfig (PluginConfig): NetBox min/max version, +│ │ default_settings (top_level_menu). │ ├── models.py — Floorplan and FloorplanImage. Holds resync_canvas(). │ ├── views.py — Generic views for FloorplanImage, plus bare Views for the │ │ canvas editor and the add-by-query-param flow, plus the │ │ Site/Location tab views. │ ├── forms.py, tables.py, filtersets.py │ ├── urls.py — Explicit paths plus get_model_urls() for FloorplanImage. -│ ├── navigation.py — Plugin menu (Floorplan Images only). +│ ├── navigation.py — Plugin menu (Floorplan Images only). Reads the +│ │ top_level_menu setting to choose between a dedicated +│ │ PluginMenu (`menu`) and NetBox's shared Plugins menu +│ │ (`menu_items`). │ ├── utils.py — file_upload() path helper. │ ├── templatetags/ │ │ └── template_utils.py — denormalize_measurement, js_str, rack_outer_js. @@ -206,7 +209,7 @@ Remember that the canvas document is persisted, so a change to the structure the - **Changelog.** User-visible changes get an entry in the root `CHANGELOG.md`. Do not edit `docs/changelog.md` — it is a one-line `pymdownx.snippets` include (`--8<-- "CHANGELOG.md"`) so there is a single source of truth. - **Never read deprecated NetBox fields directly.** See Architecture. - **Never interpolate values into JavaScript without `js_str`.** See Architecture. -- **The plugin has no settings.** Resist adding `PLUGINS_CONFIG` options for things that belong on the model. +- **The plugin has one setting, `top_level_menu`.** Resist adding further `PLUGINS_CONFIG` options for things that belong on the model. - **`display` and `brief_fields`** belong on every serializer. A brief representation with no human-readable label is not useful. ## Troubleshooting diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d12302..1614f0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,27 @@ for earlier releases. declared no `brief_fields`. It now returns `id`, `url` and `display`. Clients relying on `?brief=1` returning full objects must drop the parameter. +### Features + +* **Added an optional `top_level_menu` setting** ([#104](https://github.com/netbox-community/netbox-floorplan-plugin/issues/104)). + Setting `PLUGINS_CONFIG['netbox_floorplan']['top_level_menu'] = True` registers a + dedicated top-level "Floorplan" menu instead of nesting "Floorplan Images" under + NetBox's shared "Plugins" menu. Defaults to `False`, preserving current behaviour. + +### Security + +* **A user with only view permission on Floorplan could open the canvas editor** + ([#94](https://github.com/netbox-community/netbox-floorplan-plugin/issues/94)). + `FloorplanMapEditView` only required login, not `change_floorplan` — its + `permission_required` attribute was never consulted because the view inherited + `LoginRequiredMixin` rather than `PermissionRequiredMixin`, and even so referenced a + permission codename, `edit_floorplan`, that does not exist. The editor now requires + `netbox_floorplan.change_floorplan`, and the Add/Edit/Delete Floorplan buttons on the + site/location tab are hidden unless the viewer holds the corresponding permission. + Saving and deleting were already enforced correctly server-side, via the REST API and + NetBox's generic delete view respectively; this closes the gap that let an + unauthorized user reach the editor UI at all. + ### Bug Fixes * **The floorplan image detail page no longer renders a broken link.** Its first row printed diff --git a/docs/installation.md b/docs/installation.md index 938e04d..3323e9a 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -1,6 +1,6 @@ # Installation -Unlike many NetBox plugins, this one adds models, so installing it requires running migrations and collecting static files. It has no configuration parameters. +Unlike many NetBox plugins, this one adds models, so installing it requires running migrations and collecting static files. It has one optional configuration parameter; see [Configuration](#configuration) below. !!! note Check the [compatibility matrix](https://github.com/netbox-community/netbox-floorplan-plugin/blob/main/COMPATIBILITY.md) before installing, and choose a plugin release which supports your NetBox version. @@ -40,7 +40,23 @@ PLUGINS = [ !!! note If there are no plugins already installed, you might need to create this parameter. If so, be sure to define `PLUGINS` as a list _containing_ the plugin name as above, rather than just the name. -There is nothing to add to `PLUGINS_CONFIG` — the plugin has no settings. +There is nothing required in `PLUGINS_CONFIG`. See [Configuration](#configuration) below for the one optional setting. + +## Configuration + +`netbox_floorplan` supports one optional setting, under its own key in `PLUGINS_CONFIG`: + +| Setting | Default | Description | +|---|---|---| +| `top_level_menu` | `False` | Register a dedicated top-level "Floorplan" menu instead of nesting "Floorplan Images" under NetBox's shared "Plugins" menu. | + +```python +PLUGINS_CONFIG = { + "netbox_floorplan": { + "top_level_menu": True, + }, +} +``` ## 4. Run Migrations @@ -81,7 +97,7 @@ Restart the NetBox services to load the plugin: sudo systemctl restart netbox netbox-rq ``` -A **Floor Plan** tab should now appear on site and location detail views, and a **Netbox Floorplan** section should appear in the Plugins menu. +A **Floor Plan** tab should now appear on site and location detail views, and a **Netbox Floorplan** section should appear in the Plugins menu — or as its own top-level **Floorplan** menu, if `top_level_menu` is enabled. ## Upgrading diff --git a/docs/models/floorplan.md b/docs/models/floorplan.md index e7827eb..30d6ea2 100644 --- a/docs/models/floorplan.md +++ b/docs/models/floorplan.md @@ -108,3 +108,5 @@ The floorplan is saved only if something actually changed. Standard NetBox object permissions apply: `netbox_floorplan.view_floorplan`, `add_floorplan`, `change_floorplan`, and `delete_floorplan`. Viewing the **Floor Plan** tab on a site or location additionally requires the relevant `dcim.view_site` or `dcim.view_location` permission. + +The canvas editor requires `change_floorplan`, regardless of who created the floorplan. The Add, Edit and Delete Floorplan buttons on the tab are hidden unless the viewer holds the corresponding permission. diff --git a/netbox_floorplan/__init__.py b/netbox_floorplan/__init__.py index dd643e1..d7851ed 100644 --- a/netbox_floorplan/__init__.py +++ b/netbox_floorplan/__init__.py @@ -11,6 +11,11 @@ class FloorplanConfig(PluginConfig): base_url = "floorplan" min_version = "4.7.0" max_version = "4.7.99" + default_settings = { + # Register a dedicated top-level menu instead of nesting under NetBox's shared + # "Plugins" menu. See netbox_floorplan/navigation.py. + 'top_level_menu': False, + } config = FloorplanConfig diff --git a/netbox_floorplan/navigation.py b/netbox_floorplan/navigation.py index 753c68d..2da0ca4 100644 --- a/netbox_floorplan/navigation.py +++ b/netbox_floorplan/navigation.py @@ -2,7 +2,9 @@ Define the plugin menu buttons & the plugin navigation bar enteries. """ -from netbox.plugins import PluginMenuItem, PluginMenuButton +from netbox.plugins import PluginMenu, PluginMenuButton, PluginMenuItem, get_plugin_config + +PLUGIN_NAME = 'netbox_floorplan' # @@ -24,4 +26,19 @@ ) -menu_items = menu_buttons +# By default the plugin nests its links under NetBox's shared "Plugins" menu, via +# menu_items. Setting PLUGINS_CONFIG['netbox_floorplan']['top_level_menu'] = True instead +# registers a dedicated top-level menu, via menu. NetBox's PluginConfig.ready() registers +# whichever of the two is non-empty, so only one is ever active. +if get_plugin_config(PLUGIN_NAME, 'top_level_menu'): + menu = PluginMenu( + label='Floorplan', + groups=( + ('Floorplan', menu_buttons), + ), + icon_class='mdi mdi-floor-plan', + ) + menu_items = () +else: + menu = None + menu_items = menu_buttons diff --git a/netbox_floorplan/templates/netbox_floorplan/inc/floorplan_canvas.html b/netbox_floorplan/templates/netbox_floorplan/inc/floorplan_canvas.html index 3e377e1..0aecb92 100644 --- a/netbox_floorplan/templates/netbox_floorplan/inc/floorplan_canvas.html +++ b/netbox_floorplan/templates/netbox_floorplan/inc/floorplan_canvas.html @@ -11,28 +11,34 @@
{% if floorplan is None %} - - - {% trans "Add Floorplan" %} - + {% if perms.netbox_floorplan.add_floorplan %} + + + {% trans "Add Floorplan" %} + + {% endif %} {% else %} {% trans "Export SVG" %} - - - {% trans "Edit Floorplan" %} - - - - {% trans "Delete Floorplan" %} - + {% if perms.netbox_floorplan.change_floorplan %} + + + {% trans "Edit Floorplan" %} + + {% endif %} + {% if perms.netbox_floorplan.delete_floorplan %} + + + {% trans "Delete Floorplan" %} + + {% endif %} {% endif %}
diff --git a/netbox_floorplan/tests/test_navigation.py b/netbox_floorplan/tests/test_navigation.py new file mode 100644 index 0000000..9426e6a --- /dev/null +++ b/netbox_floorplan/tests/test_navigation.py @@ -0,0 +1,38 @@ +from importlib import reload + +from django.test import TestCase, override_settings + +from netbox.plugins import PluginMenu, PluginMenuItem + +from netbox_floorplan import navigation + + +class NavigationTestCase(TestCase): + """ + navigation.py reads PLUGINS_CONFIG['netbox_floorplan']['top_level_menu'] at import + time, so exercising both branches means reloading the module under each setting. + """ + + def tearDown(self): + # Restore the module to its real, non-overridden state for subsequent tests. + reload(navigation) + + @override_settings(PLUGINS_CONFIG={'netbox_floorplan': {'top_level_menu': False}}) + def test_default_nests_under_plugins_menu(self): + reload(navigation) + self.assertIsNone(navigation.menu) + self.assertEqual(len(navigation.menu_items), 1) + self.assertIsInstance(navigation.menu_items[0], PluginMenuItem) + self.assertEqual(navigation.menu_items[0].link_text, 'Floorplan Images') + + @override_settings(PLUGINS_CONFIG={'netbox_floorplan': {'top_level_menu': True}}) + def test_top_level_menu_registers_a_dedicated_menu(self): + reload(navigation) + self.assertEqual(navigation.menu_items, ()) + self.assertIsInstance(navigation.menu, PluginMenu) + self.assertEqual(navigation.menu.label, 'Floorplan') + self.assertEqual(len(navigation.menu.groups), 1) + group = navigation.menu.groups[0] + self.assertEqual(group.label, 'Floorplan') + self.assertEqual(len(group.items), 1) + self.assertEqual(group.items[0].link_text, 'Floorplan Images') diff --git a/netbox_floorplan/tests/test_views.py b/netbox_floorplan/tests/test_views.py index 79eb332..d352e70 100644 --- a/netbox_floorplan/tests/test_views.py +++ b/netbox_floorplan/tests/test_views.py @@ -128,11 +128,26 @@ def setUpTestData(cls): cls.floorplan = Floorplan.objects.create(site=cls.site) def test_editor_renders(self): + self.add_permissions('netbox_floorplan.change_floorplan') response = self.client.get( reverse('plugins:netbox_floorplan:floorplan_edit', args=[self.floorplan.pk]) ) self.assertHttpStatus(response, 200) + def test_editor_requires_change_permission(self): + response = self.client.get( + reverse('plugins:netbox_floorplan:floorplan_edit', args=[self.floorplan.pk]) + ) + self.assertEqual(response.status_code, 403) + + def test_view_permission_alone_is_not_sufficient(self): + # A user who can only view floorplans must not be able to open the editor. + self.add_permissions('netbox_floorplan.view_floorplan') + response = self.client.get( + reverse('plugins:netbox_floorplan:floorplan_edit', args=[self.floorplan.pk]) + ) + self.assertEqual(response.status_code, 403) + class FloorplanTabsTestCase(TestCase): """The plugin registers a Floor Plan tab on both Site and Location.""" @@ -154,6 +169,27 @@ def test_location_tab_renders_without_a_floorplan(self): response = self.client.get(reverse('dcim:location_floorplans', args=[self.location.pk])) self.assertHttpStatus(response, 200) + def test_view_only_user_sees_no_edit_or_delete_controls(self): + self.add_permissions('dcim.view_site', 'netbox_floorplan.view_floorplan') + response = self.client.get(reverse('dcim:site_floorplans', args=[self.site.pk])) + content = response.content.decode() + self.assertNotIn('Edit Floorplan', content) + self.assertNotIn('Delete Floorplan', content) + + def test_user_with_change_permission_sees_edit_control(self): + self.add_permissions( + 'dcim.view_site', 'netbox_floorplan.view_floorplan', 'netbox_floorplan.change_floorplan' + ) + response = self.client.get(reverse('dcim:site_floorplans', args=[self.site.pk])) + self.assertIn('Edit Floorplan', response.content.decode()) + + def test_user_with_delete_permission_sees_delete_control(self): + self.add_permissions( + 'dcim.view_site', 'netbox_floorplan.view_floorplan', 'netbox_floorplan.delete_floorplan' + ) + response = self.client.get(reverse('dcim:site_floorplans', args=[self.site.pk])) + self.assertIn('Delete Floorplan', response.content.decode()) + class FloorplanObjectListViewsTestCase(TestCase): """ @@ -194,6 +230,7 @@ def setUpTestData(cls): def test_editor_publishes_media_url(self): from django.conf import settings + self.add_permissions('netbox_floorplan.change_floorplan') response = self.client.get( reverse('plugins:netbox_floorplan:floorplan_edit', args=[self.floorplan.pk]) ) @@ -204,6 +241,7 @@ def test_editor_publishes_media_url(self): def test_media_url_is_published_before_the_module_script(self): # The module reads the global at import time, so ordering matters. + self.add_permissions('netbox_floorplan.change_floorplan') response = self.client.get( reverse('plugins:netbox_floorplan:floorplan_edit', args=[self.floorplan.pk]) ) @@ -302,6 +340,7 @@ def test_vendored_fabric_filename_matches_the_version_inside(self): self.assertIn(version, self.FABRIC) def test_editor_references_the_vendored_fabric(self): + self.add_permissions('netbox_floorplan.change_floorplan') response = self.client.get( reverse('plugins:netbox_floorplan:floorplan_edit', args=[self.floorplan.pk]) ) diff --git a/netbox_floorplan/views.py b/netbox_floorplan/views.py index e1cd316..4339bac 100644 --- a/netbox_floorplan/views.py +++ b/netbox_floorplan/views.py @@ -2,7 +2,7 @@ from . import forms, models, tables from .ui import panels from dcim.models import Site, Rack, Device, Location -from django.contrib.auth.mixins import LoginRequiredMixin, PermissionRequiredMixin +from django.contrib.auth.mixins import PermissionRequiredMixin from django.views import View from django.shortcuts import render, redirect from django.db.models import Q @@ -121,8 +121,8 @@ class FloorplanDeleteView(generic.ObjectDeleteView): queryset = models.Floorplan.objects.all() -class FloorplanMapEditView(LoginRequiredMixin, View): - permission_required = "netbox_floorplan.edit_floorplan" +class FloorplanMapEditView(PermissionRequiredMixin, View): + permission_required = "netbox_floorplan.change_floorplan" def get(self, request, pk): fp = models.Floorplan.objects.get(pk=pk)