From 6e730c6558a40d9a6653ff6b08ea32ac0f846edd Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Thu, 5 Feb 2026 12:42:35 -0600 Subject: [PATCH] performance and security improvements --- lib/towerops/devices.ex | 169 ++++++++++++++++++++++------------------ lib/towerops/sites.ex | 28 +++++-- 2 files changed, 117 insertions(+), 80 deletions(-) diff --git a/lib/towerops/devices.ex b/lib/towerops/devices.ex index cc06eaee..f2691755 100644 --- a/lib/towerops/devices.ex +++ b/lib/towerops/devices.ex @@ -335,7 +335,7 @@ defmodule Towerops.Devices do ssh_port: device.mikrotik_ssh_port || 22, use_ssl: if(device.mikrotik_use_ssl == nil, do: true, else: device.mikrotik_use_ssl), enabled: device.mikrotik_enabled || false, - source: String.to_atom(device.mikrotik_credential_source || "organization") + source: credential_source_atom(device.mikrotik_credential_source) } end @@ -347,24 +347,23 @@ defmodule Towerops.Devices do """ def propagate_site_mikrotik_change(site_id, attrs) do # Find all devices in this site that inherit credentials from site - devices_to_update = - Repo.all( - from(d in DeviceSchema, - where: d.site_id == ^site_id, - where: d.mikrotik_credential_source == "site" - ) + device_query = + from(d in DeviceSchema, + where: d.site_id == ^site_id, + where: d.mikrotik_credential_source == "site" ) - # Update each device's MikroTik credentials - Enum.each(devices_to_update, fn device -> - device - |> Ecto.Changeset.change(attrs) - |> Repo.update() + device_ids = Repo.all(from(d in device_query, select: d.id)) + now = DateTime.truncate(DateTime.utc_now(), :second) + updates = attrs |> Map.new() |> Map.put(:updated_at, now) |> Map.to_list() + Repo.update_all(device_query, set: updates) + + Enum.each(device_ids, fn device_id -> # Broadcast assignment change to trigger agent job list refresh Phoenix.PubSub.broadcast( Towerops.PubSub, - "device:#{device.id}:assignments", + "device:#{device_id}:assignments", {:assignments_changed, :mikrotik_updated} ) end) @@ -382,24 +381,23 @@ defmodule Towerops.Devices do def propagate_organization_mikrotik_change(organization_id, attrs) do # With denormalized credentials, update all devices that inherit from the organization # (devices with mikrotik_credential_source = "organization") - devices_to_update = - Repo.all( - from(d in DeviceSchema, - where: d.organization_id == ^organization_id, - where: d.mikrotik_credential_source == "organization" - ) + device_query = + from(d in DeviceSchema, + where: d.organization_id == ^organization_id, + where: d.mikrotik_credential_source == "organization" ) - # Update each device's MikroTik credentials - Enum.each(devices_to_update, fn device -> - device - |> Ecto.Changeset.change(attrs) - |> Repo.update() + device_ids = Repo.all(from(d in device_query, select: d.id)) + now = DateTime.truncate(DateTime.utc_now(), :second) + updates = attrs |> Map.new() |> Map.put(:updated_at, now) |> Map.to_list() + Repo.update_all(device_query, set: updates) + + Enum.each(device_ids, fn device_id -> # Broadcast assignment change to trigger agent job list refresh Phoenix.PubSub.broadcast( Towerops.PubSub, - "device:#{device.id}:assignments", + "device:#{device_id}:assignments", {:assignments_changed, :mikrotik_updated} ) end) @@ -443,7 +441,7 @@ defmodule Towerops.Devices do auth_password: device.snmpv3_auth_password, priv_protocol: device.snmpv3_priv_protocol || "AES", priv_password: device.snmpv3_priv_password, - source: String.to_atom(device.snmpv3_credential_source || "organization") + source: credential_source_atom(device.snmpv3_credential_source) } end @@ -452,22 +450,22 @@ defmodule Towerops.Devices do site that inherit credentials from the site (credential_source = "site"). """ def propagate_site_snmpv3_change(site_id, attrs) do - devices_to_update = - Repo.all( - from(d in DeviceSchema, - where: d.site_id == ^site_id, - where: d.snmpv3_credential_source == "site" - ) + device_query = + from(d in DeviceSchema, + where: d.site_id == ^site_id, + where: d.snmpv3_credential_source == "site" ) - Enum.each(devices_to_update, fn device -> - device - |> Ecto.Changeset.change(attrs) - |> Repo.update() + device_ids = Repo.all(from(d in device_query, select: d.id)) + now = DateTime.truncate(DateTime.utc_now(), :second) + updates = attrs |> Map.new() |> Map.put(:updated_at, now) |> Map.to_list() + Repo.update_all(device_query, set: updates) + + Enum.each(device_ids, fn device_id -> Phoenix.PubSub.broadcast( Towerops.PubSub, - "device:#{device.id}:assignments", + "device:#{device_id}:assignments", {:assignments_changed, :snmpv3_updated} ) end) @@ -482,22 +480,22 @@ defmodule Towerops.Devices do def propagate_organization_snmpv3_change(organization_id, attrs) do # With denormalized credentials, update all devices that inherit from the organization # (devices with snmpv3_credential_source = "organization") - devices_to_update = - Repo.all( - from(d in DeviceSchema, - where: d.organization_id == ^organization_id, - where: d.snmpv3_credential_source == "organization" - ) + device_query = + from(d in DeviceSchema, + where: d.organization_id == ^organization_id, + where: d.snmpv3_credential_source == "organization" ) - Enum.each(devices_to_update, fn device -> - device - |> Ecto.Changeset.change(attrs) - |> Repo.update() + device_ids = Repo.all(from(d in device_query, select: d.id)) + now = DateTime.truncate(DateTime.utc_now(), :second) + updates = attrs |> Map.new() |> Map.put(:updated_at, now) |> Map.to_list() + Repo.update_all(device_query, set: updates) + + Enum.each(device_ids, fn device_id -> Phoenix.PubSub.broadcast( Towerops.PubSub, - "device:#{device.id}:assignments", + "device:#{device_id}:assignments", {:assignments_changed, :snmpv3_updated} ) end) @@ -712,6 +710,15 @@ defmodule Towerops.Devices do defp present?(value) when is_binary(value), do: String.trim(value) != "" defp present?(_), do: false + defp credential_source_atom(value) do + case value do + "device" -> :device + "site" -> :site + "organization" -> :organization + _ -> :organization + end + end + @doc """ Propagates SNMP community string changes from a site to all devices that inherit from it (source = "site"). @@ -721,19 +728,19 @@ defmodule Towerops.Devices do """ def propagate_site_community_change(site_id, new_community) do # Find all devices in this site that inherit community from site - devices_to_update = - Repo.all(from(d in DeviceSchema, where: d.site_id == ^site_id, where: d.snmp_community_source == "site")) + device_query = + from(d in DeviceSchema, where: d.site_id == ^site_id, where: d.snmp_community_source == "site") - # Update each device's community string - Enum.each(devices_to_update, fn device -> - device - |> Ecto.Changeset.change(%{snmp_community: new_community}) - |> Repo.update() + device_ids = Repo.all(from(d in device_query, select: d.id)) + now = DateTime.truncate(DateTime.utc_now(), :second) + Repo.update_all(device_query, set: [snmp_community: new_community, updated_at: now]) + + Enum.each(device_ids, fn device_id -> # Broadcast assignment change to trigger agent job list refresh Phoenix.PubSub.broadcast( Towerops.PubSub, - "device:#{device.id}:assignments", + "device:#{device_id}:assignments", {:assignments_changed, :community_updated} ) end) @@ -749,24 +756,22 @@ defmodule Towerops.Devices do def propagate_organization_community_change(organization_id, new_community) do # With denormalized credentials, update all devices that inherit from the organization # (devices with snmp_community_source = "organization") - devices_to_update = - Repo.all( - from(d in DeviceSchema, - where: d.organization_id == ^organization_id, - where: d.snmp_community_source == "organization" - ) + device_query = + from(d in DeviceSchema, + where: d.organization_id == ^organization_id, + where: d.snmp_community_source == "organization" ) - # Update each device's community string - Enum.each(devices_to_update, fn device -> - device - |> Ecto.Changeset.change(%{snmp_community: new_community}) - |> Repo.update() + device_ids = Repo.all(from(d in device_query, select: d.id)) + now = DateTime.truncate(DateTime.utc_now(), :second) + Repo.update_all(device_query, set: [snmp_community: new_community, updated_at: now]) + + Enum.each(device_ids, fn device_id -> # Broadcast assignment change to trigger agent job list refresh Phoenix.PubSub.broadcast( Towerops.PubSub, - "device:#{device.id}:assignments", + "device:#{device_id}:assignments", {:assignments_changed, :community_updated} ) end) @@ -934,12 +939,28 @@ defmodule Towerops.Devices do # Insert at new position (1-based index, convert to 0-based for List.insert_at) new_order = List.insert_at(other_devices, new_position - 1, device) - # Update display_order for all devices - new_order - |> Enum.with_index(1) - |> Enum.each(fn {d, order} -> - Repo.update!(Ecto.Changeset.change(d, display_order: order)) - end) + {device_ids, orders} = + new_order + |> Enum.with_index(1) + |> Enum.map(fn {d, order} -> {d.id, order} end) + |> Enum.unzip() + + device_ids = Enum.map(device_ids, &Ecto.UUID.dump!/1) + now = DateTime.truncate(DateTime.utc_now(), :second) + + query = + from(d in DeviceSchema, + join: + data in fragment( + "SELECT * FROM unnest(?::uuid[], ?::int[]) AS data(id, ordering)", + ^device_ids, + ^orders + ), + on: d.id == data.id, + update: [set: [display_order: field(data, :ordering), updated_at: ^now]] + ) + + Repo.update_all(query, []) Repo.get!(DeviceSchema, device_id) end) diff --git a/lib/towerops/sites.ex b/lib/towerops/sites.ex index d57684ee..c06f753a 100644 --- a/lib/towerops/sites.ex +++ b/lib/towerops/sites.ex @@ -254,12 +254,28 @@ defmodule Towerops.Sites do # Insert at new position (1-based index, convert to 0-based for List.insert_at) new_order = List.insert_at(other_sites, new_position - 1, site) - # Update display_order for all sites - new_order - |> Enum.with_index(1) - |> Enum.each(fn {s, order} -> - Repo.update!(Ecto.Changeset.change(s, display_order: order)) - end) + {site_ids, orders} = + new_order + |> Enum.with_index(1) + |> Enum.map(fn {s, order} -> {s.id, order} end) + |> Enum.unzip() + + site_ids = Enum.map(site_ids, &Ecto.UUID.dump!/1) + now = DateTime.truncate(DateTime.utc_now(), :second) + + query = + from(s in Site, + join: + data in fragment( + "SELECT * FROM unnest(?::uuid[], ?::int[]) AS data(id, ordering)", + ^site_ids, + ^orders + ), + on: s.id == data.id, + update: [set: [display_order: field(data, :ordering), updated_at: ^now]] + ) + + Repo.update_all(query, []) Repo.get!(Site, site_id) end)