From 58b1aa50c297cb4ea6e96a75d1f12550ba1fdb37 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Wed, 28 Jan 2026 16:55:41 -0600 Subject: [PATCH] security fixes --- lib/towerops/alerts.ex | 10 +++ lib/towerops/sites.ex | 10 +++ lib/towerops_web/live/alert_live/index.ex | 32 +++++--- lib/towerops_web/live/device_live/form.ex | 25 ++++++- lib/towerops_web/live/device_live/index.ex | 86 +++++++++++++++------- priv/static/changelog.txt | 1 + 6 files changed, 124 insertions(+), 40 deletions(-) diff --git a/lib/towerops/alerts.ex b/lib/towerops/alerts.ex index 04a95f3e..a0a64261 100644 --- a/lib/towerops/alerts.ex +++ b/lib/towerops/alerts.ex @@ -147,6 +147,16 @@ defmodule Towerops.Alerts do |> Repo.preload([:device, :acknowledged_by]) end + @doc """ + Gets a single alert. Returns nil if not found. + """ + def get_alert(id) do + case Repo.get(Alert, id) do + nil -> nil + alert -> Repo.preload(alert, [:device, :acknowledged_by]) + end + end + @doc """ Acknowledges an alert. """ diff --git a/lib/towerops/sites.ex b/lib/towerops/sites.ex index d9cdf90d..980451d0 100644 --- a/lib/towerops/sites.ex +++ b/lib/towerops/sites.ex @@ -70,6 +70,16 @@ defmodule Towerops.Sites do |> Repo.preload([:parent_site, :child_sites, :device]) end + @doc """ + Gets a single site. Returns nil if not found. + """ + def get_site(id) do + case Repo.get(Site, id) do + nil -> nil + site -> Repo.preload(site, [:parent_site, :child_sites, :device]) + end + end + @doc """ Gets a single site belonging to an organization. """ diff --git a/lib/towerops_web/live/alert_live/index.ex b/lib/towerops_web/live/alert_live/index.ex index 230968fb..b233ce80 100644 --- a/lib/towerops_web/live/alert_live/index.ex +++ b/lib/towerops_web/live/alert_live/index.ex @@ -3,6 +3,7 @@ defmodule ToweropsWeb.AlertLive.Index do use ToweropsWeb, :live_view alias Towerops.Alerts + alias Towerops.Repo @impl true def mount(_params, _session, socket) do @@ -34,18 +35,31 @@ defmodule ToweropsWeb.AlertLive.Index do @impl true def handle_event("acknowledge", %{"id" => id}, socket) do - alert = Alerts.get_alert!(id) + organization = socket.assigns.current_organization user_id = socket.assigns.current_scope.user.id - case Alerts.acknowledge_alert(alert, user_id) do - {:ok, _alert} -> - {:noreply, - socket - |> put_flash(:info, "Alert acknowledged") - |> load_alerts(socket.assigns.current_organization.id)} + case Alerts.get_alert(id) do + nil -> + {:noreply, put_flash(socket, :error, "Alert not found")} - {:error, _changeset} -> - {:noreply, put_flash(socket, :error, "Unable to acknowledge alert")} + alert -> + # Preload device and site to check organization access + alert = Repo.preload(alert, device: [site: :organization]) + + if alert.device.site.organization_id == organization.id do + case Alerts.acknowledge_alert(alert, user_id) do + {:ok, _alert} -> + {:noreply, + socket + |> put_flash(:info, "Alert acknowledged") + |> load_alerts(organization.id)} + + {:error, _changeset} -> + {:noreply, put_flash(socket, :error, "Unable to acknowledge alert")} + end + else + {:noreply, put_flash(socket, :error, "You don't have access to this alert")} + end end end diff --git a/lib/towerops_web/live/device_live/form.ex b/lib/towerops_web/live/device_live/form.ex index 1708f6a7..d1d40e31 100644 --- a/lib/towerops_web/live/device_live/form.ex +++ b/lib/towerops_web/live/device_live/form.ex @@ -101,11 +101,30 @@ defmodule ToweropsWeb.DeviceLive.Form do end defp apply_action(socket, :edit, %{"id" => id}) do - device = Devices.get_device!(id) + organization = socket.assigns.current_organization - # Preload necessary associations for agent resolution - device = Towerops.Repo.preload(device, site: [organization: :default_agent_token]) + case Devices.get_device(id) do + nil -> + socket + |> put_flash(:error, "Device not found") + |> push_navigate(to: ~p"/devices") + device -> + # Preload necessary associations for agent resolution + device = Towerops.Repo.preload(device, site: [organization: :default_agent_token]) + + # Verify device belongs to current organization + if device.site.organization_id == organization.id do + apply_edit_action(socket, device) + else + socket + |> put_flash(:error, "You don't have access to this device") + |> push_navigate(to: ~p"/devices") + end + end + end + + defp apply_edit_action(socket, device) do # Get current agent assignment if any current_assignment = Agents.get_device_assignment(device.id) diff --git a/lib/towerops_web/live/device_live/index.ex b/lib/towerops_web/live/device_live/index.ex index 11c61fea..d0a7bf58 100644 --- a/lib/towerops_web/live/device_live/index.ex +++ b/lib/towerops_web/live/device_live/index.ex @@ -4,6 +4,7 @@ defmodule ToweropsWeb.DeviceLive.Index do alias Towerops.Devices alias Towerops.Organizations.SubscriptionLimits + alias Towerops.Repo alias Towerops.Sites alias Towerops.Snmp alias Towerops.Workers.DiscoveryWorker @@ -72,45 +73,74 @@ defmodule ToweropsWeb.DeviceLive.Index do @impl true def handle_event("reorder_site", %{"site_id" => site_id, "new_position" => new_position}, socket) do - # Handle both integer and string input - position = if is_integer(new_position), do: new_position, else: String.to_integer(new_position) + organization = socket.assigns.current_organization - case Sites.reorder_site(site_id, position) do - {:ok, _site} -> - # Reload devices with updated order - devices = Devices.list_organization_devices(socket.assigns.current_organization.id) - grouped_devices = group_devices_by_site(devices) + # Verify site belongs to current organization + case Sites.get_site(site_id) do + nil -> + {:noreply, put_flash(socket, :error, "Site not found")} - {:noreply, - socket - |> assign(:device, devices) - |> assign(:grouped_devices, grouped_devices) - |> put_flash(:info, "Site order updated")} + site -> + if site.organization_id == organization.id do + # Handle both integer and string input + position = if is_integer(new_position), do: new_position, else: String.to_integer(new_position) - {:error, _changeset} -> - {:noreply, put_flash(socket, :error, "Failed to reorder site")} + case Sites.reorder_site(site_id, position) do + {:ok, _site} -> + # Reload devices with updated order + devices = Devices.list_organization_devices(organization.id) + grouped_devices = group_devices_by_site(devices) + + {:noreply, + socket + |> assign(:device, devices) + |> assign(:grouped_devices, grouped_devices) + |> put_flash(:info, "Site order updated")} + + {:error, _changeset} -> + {:noreply, put_flash(socket, :error, "Failed to reorder site")} + end + else + {:noreply, put_flash(socket, :error, "You don't have access to this site")} + end end end @impl true def handle_event("reorder_device", %{"device_id" => device_id, "new_position" => new_position}, socket) do - # Handle both integer and string input - position = if is_integer(new_position), do: new_position, else: String.to_integer(new_position) + organization = socket.assigns.current_organization - case Devices.reorder_device(device_id, position) do - {:ok, _device} -> - # Reload devices with updated order - devices = Devices.list_organization_devices(socket.assigns.current_organization.id) - grouped_devices = group_devices_by_site(devices) + # Verify device belongs to current organization + case Devices.get_device(device_id) do + nil -> + {:noreply, put_flash(socket, :error, "Device not found")} - {:noreply, - socket - |> assign(:device, devices) - |> assign(:grouped_devices, grouped_devices) - |> put_flash(:info, "Device order updated")} + device -> + # Preload site to check organization access + device = Repo.preload(device, site: :organization) - {:error, _changeset} -> - {:noreply, put_flash(socket, :error, "Failed to reorder device")} + if device.site.organization_id == organization.id do + # Handle both integer and string input + position = if is_integer(new_position), do: new_position, else: String.to_integer(new_position) + + case Devices.reorder_device(device_id, position) do + {:ok, _device} -> + # Reload devices with updated order + devices = Devices.list_organization_devices(organization.id) + grouped_devices = group_devices_by_site(devices) + + {:noreply, + socket + |> assign(:device, devices) + |> assign(:grouped_devices, grouped_devices) + |> put_flash(:info, "Device order updated")} + + {:error, _changeset} -> + {:noreply, put_flash(socket, :error, "Failed to reorder device")} + end + else + {:noreply, put_flash(socket, :error, "You don't have access to this device")} + end end end diff --git a/priv/static/changelog.txt b/priv/static/changelog.txt index 3a23a6f0..844b1503 100644 --- a/priv/static/changelog.txt +++ b/priv/static/changelog.txt @@ -10,6 +10,7 @@ Devices Working * GDPR: My Data links in user settings and navigation * Bug fix: SNMP Community when using remote agent * Feature: Add mandatory TOTP MFA +* Security improvements 2025-01-27 * Infrastructure improvements