From 811e936a2b0fb2a34cdf349f7bf07f4902bb08a0 Mon Sep 17 00:00:00 2001 From: Benjamin Bertrand <benjamin.bertrand@ess.eu> Date: Mon, 16 Mar 2020 16:47:27 +0100 Subject: [PATCH] Fix get_hosts for normal users The SQL query to filter hosts on sensitive networks was incorrect because of the join on Interfaces. There can be several interfaces per host. DISTINCT added to return only the proper number of hosts. JIRA INFRA-1888 #action In Progress --- app/api/network.py | 2 + tests/functional/test_api.py | 128 +++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+) diff --git a/app/api/network.py b/app/api/network.py index be24d54..503f575 100644 --- a/app/api/network.py +++ b/app/api/network.py @@ -326,6 +326,8 @@ def get_hosts(): query.join(models.Host.interfaces) .join(models.Interface.network) .filter(models.Network.sensitive.is_(False)) + .distinct(models.Host.id) + .from_self() ) query = query.order_by(models.Host.name) return utils.get_generic_model(model=models.Host, base_query=query) diff --git a/tests/functional/test_api.py b/tests/functional/test_api.py index 719e33e..c89617f 100644 --- a/tests/functional/test_api.py +++ b/tests/functional/test_api.py @@ -10,6 +10,7 @@ This module defines API tests. """ import datetime +import ipaddress import json import pytest from app import models @@ -146,6 +147,28 @@ def get_token(client, username, password): return response.get_json()["access_token"] +def create_hosts(number, host_factory, interface_factory, *args): + """Helper function to create a number of hosts + + :param number: number of hosts to create + :param host_factory: host_factory fixture + :param interface_factory: interface_factory fixture + :param *args: list of networks (host will have one interface per network) + """ + for i in range(number): + host = host_factory() + for n, network in enumerate(args): + ip = ipaddress.ip_address(network.first_ip) + i + if n == 0: + interface_factory( + name=host.name, host=host, network=network, ip=str(ip) + ) + else: + interface_factory( + name=f"{host.name}-{n}", host=host, network=network, ip=str(ip) + ) + + @pytest.fixture() def readonly_token(client): return get_token(client, "user_ro", "userro") @@ -2050,6 +2073,111 @@ def test_pagination(endpoint, client, host_factory, admin_token): ) +def test_get_hosts_normal_user( + client, + network_scope_factory, + network_factory, + interface_factory, + host_factory, + user_token, +): + # Create several networks + scope = network_scope_factory(name="FooNetworks", supernet="192.168.0.0/16") + admin_network = network_factory( + vlan_name="admin-network", + address="192.168.1.0/24", + first_ip="192.168.1.10", + last_ip="192.168.1.250", + admin_only=True, + scope=scope, + ) + sensitive_network = network_factory( + vlan_name="sensitive-network", + address="192.168.2.0/24", + first_ip="192.168.2.10", + last_ip="192.168.2.250", + admin_only=True, + sensitive=True, + scope=scope, + ) + user1_network = network_factory( + vlan_name="user1-network", + address="192.168.3.0/24", + first_ip="192.168.3.10", + last_ip="192.168.3.250", + scope=scope, + ) + user2_network = network_factory( + vlan_name="user2-network", + address="192.168.4.0/24", + first_ip="192.168.4.10", + last_ip="192.168.4.250", + scope=scope, + ) + user3_network = network_factory( + vlan_name="user3-network", + address="192.168.5.0/24", + first_ip="192.168.5.10", + last_ip="192.168.5.250", + scope=scope, + ) + # Create 15 hosts on admin network + create_hosts(15, host_factory, interface_factory, admin_network) + # Create 10 hosts on sensitive network (should be filtered out) + create_hosts(10, host_factory, interface_factory, sensitive_network) + # Create 20 hosts on user1 network + create_hosts(20, host_factory, interface_factory, user1_network) + # Create 10 hosts on user2 and user3 networks + # Hosts with several interfaces required to reproduce INFRA-1888 + create_hosts(10, host_factory, interface_factory, user2_network, user3_network) + # Create 8 more hosts without interfaces (should be filtered out) + create_hosts(8, host_factory, interface_factory) + url = f"{API_URL}/network/hosts" + # By default 20 hosts per page shall be returned + response = get(client, url, token=user_token) + assert response.status_code == 200 + assert len(response.get_json()) == 20 + assert response.headers["x-total-count"] == "45" + assert ( + f'{url}?per_page=20&page=2&recursive=False>; rel="next",' + in response.headers["link"] + ) + # Get second page + response = get( + client, f"{url}?per_page=20&page=2&recursive=False", token=user_token, + ) + assert response.status_code == 200 + assert len(response.get_json()) == 20 + assert ( + f'{url}?per_page=20&page=1&recursive=False>; rel="first",' + in response.headers["link"] + ) + assert ( + f'{url}?per_page=20&page=1&recursive=False>; rel="prev"' + in response.headers["link"] + ) + assert ( + f'{url}?per_page=20&page=3&recursive=False>; rel="next",' + in response.headers["link"] + ) + assert ( + f'{url}?per_page=20&page=3&recursive=False>; rel="last"' + in response.headers["link"] + ) + # Get third page + response = get( + client, f"{url}?per_page=20&page=3&recursive=False", token=user_token, + ) + assert response.status_code == 200 + assert len(response.get_json()) == 5 + assert ( + f'{url}?per_page=20&page=2&recursive=False>; rel="prev"' + in response.headers["link"] + ) + assert 'rel="next"' not in response.headers["link"] + assert 'rel="last"' not in response.headers["link"] + + def test_patch_host_no_data(client, host_factory, admin_token): host = host_factory() response = patch( -- GitLab