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

permissions are not respected with /dcim/connected-device api #7051

Closed
sthiriet opened this issue Aug 27, 2021 · 3 comments
Closed

permissions are not respected with /dcim/connected-device api #7051

sthiriet opened this issue Aug 27, 2021 · 3 comments
Assignees
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application

Comments

@sthiriet
Copy link

sthiriet commented Aug 27, 2021

NetBox version

v2.11.12

Python version

3.8

Steps to Reproduce

  1. Enable permissions
  2. As admin, create objects required to have data returned by API /dcim/connected-device (devices, interfaces, cables,...)
  3. As admin, create a user with no permissions or only on virtualization->cluster for example
  4. Use this user to call GET ​/dcim​/connected-device​/ API with correct parameters
curl -X GET "https://demo.netbox.dev/api/dcim/connected-device/?peer_device=dmi01-akron-sw01&peer_interface=GigabitEthernet1%2F0%2F1" -H  "accept: application/json" -H "Authorization: ....."

Expected Behavior

As user has no permission on dcim, nothing should be returned, but hard to say what should be the necessary permissions.

Observed Behavior

response body contains data that user should probably not be able to access:

{
  "id": 1,
  "url": "https://demo.netbox.dev/api/dcim/devices/1/",
  "display": "dmi01-akron-rtr01",
  "name": "dmi01-akron-rtr01",
  "display_name": "dmi01-akron-rtr01",
  "device_type": {
    "id": 6,
    "url": "https://demo.netbox.dev/api/dcim/device-types/6/",
    "display": "ISR 1111-8P",
    "manufacturer": {
      "id": 3,
      "url": "https://demo.netbox.dev/api/dcim/manufacturers/3/",
      "display": "Cisco",
      "name": "Cisco",
      "slug": "cisco"
    },
    "model": "ISR 1111-8P",
    "slug": "isr1111",
    "display_name": "Cisco ISR 1111-8P"
  },
  "device_role": {
    "id": 1,
    "url": "https://demo.netbox.dev/api/dcim/device-roles/1/",
    "display": "Router",
    "name": "Router",
    "slug": "router"
  },
  "tenant": {
    "id": 5,
    "url": "https://demo.netbox.dev/api/tenancy/tenants/5/",
    "display": "Dunder-Mifflin, Inc.",
    "name": "Dunder-Mifflin, Inc.",
    "slug": "dunder-mifflin"
  },
  "platform": {
    "id": 1,
    "url": "https://demo.netbox.dev/api/dcim/platforms/1/",
    "display": "Cisco IOS",
    "name": "Cisco IOS",
    "slug": "cisco-ios"
  },
  "serial": "",
  "asset_tag": null,
  "site": {
    "id": 2,
    "url": "https://demo.netbox.dev/api/dcim/sites/2/",
    "display": "DM-Akron",
    "name": "DM-Akron",
    "slug": "dm-akron"
  },
  "location": null,
  "rack": {
    "id": 1,
    "url": "https://demo.netbox.dev/api/dcim/racks/1/",
    "display": "Comms closet",
    "name": "Comms closet",
    "display_name": "Comms closet"
  },
  "position": 4,
  "face": {
    "value": "front",
    "label": "Front"
  },
  "parent_device": null,
  "status": {
    "value": "active",
    "label": "Active"
  },
  "primary_ip": null,
  "primary_ip4": null,
  "primary_ip6": null,
  "cluster": null,
  "virtual_chassis": null,
  "vc_position": null,
  "vc_priority": null,
  "comments": "",
  "local_context_data": null,
  "tags": [],
  "custom_fields": {},
  "created": "2020-12-20",
  "last_updated": "2020-12-20T02:51:03.257000Z"
}
@sthiriet sthiriet added the type: bug A confirmed report of unexpected behavior in the application label Aug 27, 2021
@DanSheps DanSheps added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation type: feature Introduction of new functionality to the application and removed type: bug A confirmed report of unexpected behavior in the application labels Sep 3, 2021
@xraurp
Copy link

xraurp commented Oct 4, 2021

I wonder why this is not a bug. Other endpoints like /api/dcim/devices/ requires user permissions even if LOGIN_REQUIRED is false. Why this one should be left unsecured?

I guess the correct permissions should be dcim.view_device and dcim.view_interface because device and interface are passed in as query parameters. Other endpoints requires permissions based on parameters. For example the /api/dcim/devices/ takes device id as parameter and requires dcim.view_device.
Endpoint should respect permissions at least when LOGIN_REQUIRED is True.
Correct me if I'm mistaken.

@xraurp
Copy link

xraurp commented Oct 4, 2021

I have fix for this, assign me and I'll create pull request.

@DanSheps DanSheps added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Oct 5, 2021
@jeremystretch jeremystretch self-assigned this Oct 7, 2021
@jeremystretch
Copy link
Member

After some digging, it's clear there's a bit more work needed on this endpoint. I'm going to merge a fix to both improve permissions handling and to avoid raising an exception when the connected object is not a device. Thanks @xraurp!

jeremystretch added a commit that referenced this issue Oct 7, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants