Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permission madness #420

Open
dreng opened this issue Oct 29, 2023 · 1 comment
Open

Permission madness #420

dreng opened this issue Oct 29, 2023 · 1 comment
Labels
status: under review Further discussion is needed to determine this issue's scope and/or implementation type: housekeeping Changes to the application which do not directly impact the end user

Comments

@dreng
Copy link
Collaborator

dreng commented Oct 29, 2023

NetBox version

v3.6.4

Topology Views version

v3.8.1

Steps to Reproduce

Check permissions in the following files:

  • navigation.py
  • views.py
  • api/views.py

Expected Behavior

Permission should make sense.

Observed Behavior

navigation.py:

The entry for plugins:netbox_topology_views:images wants these permissions:

  • dcim.view_site
  • dcim.view_devicerole

This entry is for making the menu item visible. Why should this item make use of Sites or Device Roles? The correct permission should be imho:

  • netbox_topology_views.view_roleimage

views.py

The class TopologyImagesView wants:

  • dcim.view_site
  • dcim.view_devicerole
  • dcim.add_devicerole
  • dcim.change_devicerole

The Images View does not need any Site information. I think this was just copy-pasted from the Topology View (where it is needed indeed). In addition, the view only displays something in the first step. Write permissions are only required when saving images. However, the permission to display the view should be controlled via the "Role Image" permission.
This seems to be reasonable and sufficient:

  • netbox_topology_views.view_roleimage
  • dcim.view_devicerole

api/views.py

This is the one I don't understand. The class SaveRoleImagesViewSet wants:

  • dcim.add_devicerole
  • dcim.change_devicerole

Here, too, the authorisation should be controlled via "Role Image" and being able to read the Device Role model should be sufficient. But why does it need the other permissions? Nothing will be written to Device Role model. Instead, Role Image model must be writable. When I tried to remove these permissions and set view permissions instead (in both, source code and admin panel), saving didn't work anymore.
In my opinion it should be:

  • netbox_topology_views.change_roleimage
  • dcim.view_devicerole

But as stated before, that didn't work for me. @mattieserver Hope you can contribute here.

@dreng dreng added status: under review Further discussion is needed to determine this issue's scope and/or implementation type: housekeeping Changes to the application which do not directly impact the end user labels Oct 29, 2023
@mattieserver
Copy link
Collaborator

A lot of those are from older code and can indeed be cleaned up.
I will create a PR to cleanup some of those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: under review Further discussion is needed to determine this issue's scope and/or implementation type: housekeeping Changes to the application which do not directly impact the end user
Projects
None yet
Development

No branches or pull requests

2 participants