Skip to content

Commit

Permalink
Fixes: #17310 - Properly restrict GraphQL related object queries (#17312
Browse files Browse the repository at this point in the history
)

* Fixes: #17310 - Fix GraphQL restriction of related objects

* Fix some failing tests

* Fix test
  • Loading branch information
DanSheps authored Aug 30, 2024
1 parent 91ad3f2 commit 4fead1c
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 2 deletions.
4 changes: 4 additions & 0 deletions netbox/circuits/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class CircuitTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'status': 'planned',
}
user_permissions = ('circuits.view_provider', 'circuits.view_circuittype')

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -150,6 +151,7 @@ def setUpTestData(cls):
class CircuitTerminationTest(APIViewTestCases.APIViewTestCase):
model = CircuitTermination
brief_fields = ['_occupied', 'cable', 'circuit', 'description', 'display', 'id', 'term_side', 'url']
user_permissions = ('circuits.view_circuit', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -209,6 +211,7 @@ def setUpTestData(cls):
class ProviderAccountTest(APIViewTestCases.APIViewTestCase):
model = ProviderAccount
brief_fields = ['account', 'description', 'display', 'id', 'name', 'url']
user_permissions = ('circuits.view_provider', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -252,6 +255,7 @@ def setUpTestData(cls):
class ProviderNetworkTest(APIViewTestCases.APIViewTestCase):
model = ProviderNetwork
brief_fields = ['description', 'display', 'id', 'name', 'url']
user_permissions = ('circuits.view_provider', )

@classmethod
def setUpTestData(cls):
Expand Down
1 change: 1 addition & 0 deletions netbox/core/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class DataFileTest(
):
model = DataFile
brief_fields = ['display', 'id', 'path', 'url']
user_permissions = ('core.view_datasource', )

@classmethod
def setUpTestData(cls):
Expand Down
27 changes: 27 additions & 0 deletions netbox/dcim/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ class LocationTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'description': 'New description',
}
user_permissions = ('dcim.view_site', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -280,6 +281,7 @@ class RackTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'status': 'planned',
}
user_permissions = ('dcim.view_site', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -368,6 +370,7 @@ class RackReservationTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'description': 'New description',
}
user_permissions = ('dcim.view_rack', 'users.view_user')

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -447,6 +450,7 @@ class DeviceTypeTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'part_number': 'ABC123',
}
user_permissions = ('dcim.view_manufacturer', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -492,6 +496,7 @@ class ModuleTypeTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'part_number': 'ABC123',
}
user_permissions = ('dcim.view_manufacturer', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -663,6 +668,7 @@ class PowerOutletTemplateTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'description': 'New description',
}
user_permissions = ('dcim.view_devicetype', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -768,6 +774,7 @@ class FrontPortTemplateTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'description': 'New description',
}
user_permissions = ('dcim.view_rearporttemplate', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -905,6 +912,7 @@ class ModuleBayTemplateTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'description': 'New description',
}
user_permissions = ('dcim.view_devicetype', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -945,6 +953,7 @@ class DeviceBayTemplateTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'description': 'New description',
}
user_permissions = ('dcim.view_devicetype', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -985,6 +994,7 @@ class InventoryItemTemplateTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'description': 'New description',
}
user_permissions = ('dcim.view_devicetype', 'dcim.view_manufacturer',)

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -1103,6 +1113,10 @@ class DeviceTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'status': 'failed',
}
user_permissions = (
'dcim.view_site', 'dcim.view_rack', 'dcim.view_location', 'dcim.view_devicerole', 'dcim.view_devicetype',
'extras.view_configtemplate',
)

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -1293,6 +1307,7 @@ class ModuleTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'serial': '1234ABCD',
}
user_permissions = ('dcim.view_modulebay', 'dcim.view_moduletype', 'dcim.view_device')

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -1358,6 +1373,7 @@ class ConsolePortTest(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTestCa
'description': 'New description',
}
peer_termination_type = ConsoleServerPort
user_permissions = ('dcim.view_device', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -1400,6 +1416,7 @@ class ConsoleServerPortTest(Mixins.ComponentTraceMixin, APIViewTestCases.APIView
'description': 'New description',
}
peer_termination_type = ConsolePort
user_permissions = ('dcim.view_device', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -1442,6 +1459,7 @@ class PowerPortTest(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTestCase
'description': 'New description',
}
peer_termination_type = PowerOutlet
user_permissions = ('dcim.view_device', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -1481,6 +1499,7 @@ class PowerOutletTest(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTestCa
'description': 'New description',
}
peer_termination_type = PowerPort
user_permissions = ('dcim.view_device', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -1529,6 +1548,7 @@ class InterfaceTest(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTestCase
'description': 'New description',
}
peer_termination_type = Interface
user_permissions = ('dcim.view_device', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -1663,6 +1683,7 @@ class FrontPortTest(APIViewTestCases.APIViewTestCase):
'description': 'New description',
}
peer_termination_type = Interface
user_permissions = ('dcim.view_device', 'dcim.view_rearport')

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -1721,6 +1742,7 @@ class RearPortTest(APIViewTestCases.APIViewTestCase):
'description': 'New description',
}
peer_termination_type = Interface
user_permissions = ('dcim.view_device', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -1762,6 +1784,7 @@ class ModuleBayTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'description': 'New description',
}
user_permissions = ('dcim.view_device', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -1801,6 +1824,7 @@ class DeviceBayTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'description': 'New description',
}
user_permissions = ('dcim.view_device', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -1864,6 +1888,7 @@ class InventoryItemTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'description': 'New description',
}
user_permissions = ('dcim.view_device', 'dcim.view_manufacturer')

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -2160,6 +2185,7 @@ def setUpTestData(cls):
class PowerPanelTest(APIViewTestCases.APIViewTestCase):
model = PowerPanel
brief_fields = ['description', 'display', 'id', 'name', 'powerfeed_count', 'url']
user_permissions = ('dcim.view_site', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -2212,6 +2238,7 @@ class PowerFeedTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'status': 'planned',
}
user_permissions = ('dcim.view_powerpanel', )

@classmethod
def setUpTestData(cls):
Expand Down
1 change: 1 addition & 0 deletions netbox/ipam/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,7 @@ class FHRPGroupAssignmentTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'priority': 100,
}
user_permissions = ('ipam.view_fhrpgroup', )

@classmethod
def setUpTestData(cls):
Expand Down
2 changes: 1 addition & 1 deletion netbox/netbox/graphql/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ class Query(
query=Query,
config=StrawberryConfig(auto_camel_case=False),
extensions=[
DjangoOptimizerExtension,
DjangoOptimizerExtension(prefetch_custom_queryset=True),
]
)
13 changes: 12 additions & 1 deletion netbox/netbox/tests/test_graphql.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from core.models import ObjectType
from dcim.models import Site, Location
from ipam.models import ASN, RIR
from users.models import ObjectPermission
from utilities.testing import disable_warnings, APITestCase, TestCase

Expand Down Expand Up @@ -45,7 +46,6 @@ def test_graphiql_interface(self):
class GraphQLAPITestCase(APITestCase):

@override_settings(LOGIN_REQUIRED=True)
@override_settings(EXEMPT_VIEW_PERMISSIONS=['*', 'auth.user'])
def test_graphql_filter_objects(self):
"""
Test the operation of filters for GraphQL API requests.
Expand All @@ -66,6 +66,7 @@ def test_graphql_filter_objects(self):
obj_perm.save()
obj_perm.users.add(self.user)
obj_perm.object_types.add(ObjectType.objects.get_for_model(Location))
obj_perm.object_types.add(ObjectType.objects.get_for_model(Site))

# A valid request should return the filtered list
url = reverse('graphql')
Expand All @@ -75,10 +76,20 @@ def test_graphql_filter_objects(self):
data = json.loads(response.content)
self.assertNotIn('errors', data)
self.assertEqual(len(data['data']['location_list']), 1)
self.assertIsNotNone(data['data']['location_list'][0]['site'])

# An invalid request should return an empty list
query = '{location_list(filters: {site_id: "99999"}) {id site {id}}}' # Invalid site ID
response = self.client.post(url, data={'query': query}, format="json", **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
data = json.loads(response.content)
self.assertEqual(len(data['data']['location_list']), 0)

# Removing the permissions from location should result in an empty locations list
obj_perm.object_types.remove(ObjectType.objects.get_for_model(Location))
query = '{site(id: ' + str(sites[0].pk) + ') {id locations {id}}}'
response = self.client.post(url, data={'query': query}, format="json", **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
data = json.loads(response.content)
self.assertNotIn('errors', data)
self.assertEqual(len(data['data']['site']['locations']), 0)
1 change: 1 addition & 0 deletions netbox/tenancy/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ class ContactAssignmentTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'priority': ContactPriorityChoices.PRIORITY_INACTIVE,
}
user_permissions = ('tenancy.view_contact', )

@classmethod
def setUpTestData(cls):
Expand Down
2 changes: 2 additions & 0 deletions netbox/virtualization/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ class VMInterfaceTest(APIViewTestCases.APIViewTestCase):
'description': 'New description',
}
graphql_base_name = 'vm_interface'
user_permissions = ('virtualization.view_virtualmachine', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -342,6 +343,7 @@ class VirtualDiskTest(APIViewTestCases.APIViewTestCase):
'size': 888,
}
graphql_base_name = 'virtual_disk'
user_permissions = ('virtualization.view_virtualmachine', )

@classmethod
def setUpTestData(cls):
Expand Down
3 changes: 3 additions & 0 deletions netbox/vpn/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ class TunnelTerminationTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'role': TunnelTerminationRoleChoices.ROLE_PEER,
}
user_permissions = ('vpn.view_tunnel', )

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -430,6 +431,7 @@ def setUpTestData(cls):
class IPSecProfileTest(APIViewTestCases.APIViewTestCase):
model = IPSecProfile
brief_fields = ['description', 'display', 'id', 'name', 'url']
user_permissions = ('vpn.view_ikepolicy', 'vpn.view_ipsecpolicy')

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -558,6 +560,7 @@ def setUpTestData(cls):
class L2VPNTerminationTest(APIViewTestCases.APIViewTestCase):
model = L2VPNTermination
brief_fields = ['display', 'id', 'l2vpn', 'url']
user_permissions = ('dcim.view_location', 'vpn.view_l2vpn')

@classmethod
def setUpTestData(cls):
Expand Down
1 change: 1 addition & 0 deletions netbox/wireless/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ class WirelessLinkTest(APIViewTestCases.APIViewTestCase):
bulk_update_data = {
'status': 'planned',
}
user_permissions = ('dcim.view_interface', )

@classmethod
def setUpTestData(cls):
Expand Down

0 comments on commit 4fead1c

Please sign in to comment.