[locations] Refine location matching checks

2 files changed, 28 insertions, 21 deletions

diff --git a/modules-available/locations/inc/location.inc.php b/modules-available/locations/inc/location.inc.php
index e35670cc..ac866cbc 100644
--- a/modules-available/locations/inc/location.inc.php
+++ b/modules-available/locations/inc/location.inc.php
@@ -329,7 +329,7 @@ class Location
 			if ($uuid !== false) {
 				// Machine ip maps to a location, and we have a client supplied uuid (which might not be known if the client boots for the first time)
 				$uuidLoc = self::getFromMachineUuid($uuid);
-				if (self::isUuidLocationValid($uuidLoc, $ipLoc)) {
+				if (self::isFixedLocationValid($uuidLoc, $ipLoc)) {
 					$locationId = $uuidLoc;
 				}
 			}
@@ -337,20 +337,22 @@ class Location
 		return $locationId;
 	}
 
-	public static function isUuidLocationValid($uuidLoc, $ipLoc)
+	public static function isFixedLocationValid($uuidLoc, $ipLoc)
 	{
-		if ($ipLoc !== false && $uuidLoc !== false && $uuidLoc !== $ipLoc) {
-			// Validate that the location the IP maps to is in the chain we get using the
-			// location determined by the uuid
-			$uuidLocations = self::getLocationRootChain($uuidLoc);
-			$ipLocations = self::getLocationRootChain($ipLoc);
-			if (count($ipLocations) + 2 >= count($uuidLocations) && in_array($ipLoc, $uuidLocations)) {
-				// UUID is max one level deeper than IP loc, accept
-				return true;
-			}
-			// UUID and IP disagree too much, play safe and ignore the UUID
-		}
-		return false;
+		$uuidLoc = (int)$uuidLoc;
+		$ipLoc = (int)$ipLoc;
+		if ($uuidLoc === $ipLoc || $uuidLoc === 0)
+			return true;
+		if ($ipLoc === 0)
+			return false; // roomplanner assignment, but no subnet - deny
+		// Validate that the location the IP maps to is in the chain we get using the
+		// location determined by roomplanner
+		$uuidLocations = self::getLocationRootChain($uuidLoc);
+		if (!empty(self::$assocLocationCache[$uuidLoc]['children']))
+			return false; // roomplanner location isn't actually leaf node, this is not valid
+		$idx = array_search($ipLoc, $uuidLocations);
+		// If roomplanner loc is max two levels deeper than IP loc, accept
+		return $idx !== false && $idx <= 2;
 	}
 
 	/**
diff --git a/modules-available/locations/inc/locationutil.inc.php b/modules-available/locations/inc/locationutil.inc.php
index 6b18d864..708cc8a2 100644
--- a/modules-available/locations/inc/locationutil.inc.php
+++ b/modules-available/locations/inc/locationutil.inc.php
@@ -125,9 +125,7 @@ class LocationUtil
 		$return = [];
 		$locs = false;
 		while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
-			if ($row['subnetlocationid'] === $row['fixedlocationid'])
-				continue;
-			if (Location::isUuidLocationValid((int)$row['fixedlocationid'], (int)$row['subnetlocationid']))
+			if (Location::isFixedLocationValid($row['fixedlocationid'], $row['subnetlocationid']))
 				continue;
 			$lid = (int)$row['fixedlocationid'];
 			if (!isset($return[$lid])) {
@@ -145,14 +143,21 @@ class LocationUtil
 				$return[$lid]['count']++;
 			} else {
 				$slid = (int)$row['subnetlocationid'];
-				$return[$lid]['clients'][] = [
+				$data = [
 					'machineuuid' => $row['machineuuid'],
 					'hostname' => $row['hostname'],
 					'clientip' => $row['clientip'],
-					'iplocationid' => $slid,
-					'iplocationname' => $locs[$slid]['locationname'],
-					'canmove' => !$checkPerms || $ipLocs === true || in_array($slid, $ipLocs), // Can machine be moved to subnet's locationid?
 				];
+				if ($slid !== 0) {
+					$data += [
+						'iplocationid' => $slid,
+						'iplocationname' => $locs[$slid]['locationname'],
+						'ipisleaf' => empty($locs[$slid]['children']),
+						'canmove' => empty($locs[$slid]['children'])
+							&& (!$checkPerms || $ipLocs === true || in_array($slid, $ipLocs)), // Can machine be moved to subnet's locationid?
+					];
+				}
+				$return[$lid]['clients'][] = $data;
 			}
 		}
 		if (empty($return))