From 124d68d28d604f208a3fe9a6e1ea857aac5b9c80 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Tue, 27 Jan 2026 09:12:22 -0600 Subject: [PATCH] credo fixes --- lib/snmpkit/snmp_lib/host_parser.ex | 53 +++++++++++-------- lib/snmpkit/snmp_lib/manager.ex | 15 ++---- .../controllers/api/v1/mib_controller.ex | 43 ++++++++------- lib/towerops_web/user_auth.ex | 48 +++++++++-------- mix.lock | 2 +- 5 files changed, 87 insertions(+), 74 deletions(-) diff --git a/lib/snmpkit/snmp_lib/host_parser.ex b/lib/snmpkit/snmp_lib/host_parser.ex index e4123adc..1e17ee7e 100644 --- a/lib/snmpkit/snmp_lib/host_parser.ex +++ b/lib/snmpkit/snmp_lib/host_parser.ex @@ -159,17 +159,7 @@ defmodule SnmpKit.SnmpLib.HostParser do # Keyword list input: [host: ..., port: ...] def parse([_ | _] = input, default_port) when is_list(input) do if Keyword.keyword?(input) do - host = Keyword.get(input, :host) - port = Keyword.get(input, :port, default_port) - - if host do - with {:ok, {ip_tuple, _}} <- parse(host, default_port), - :ok <- validate_port(port) do - {:ok, {ip_tuple, port}} - end - else - {:error, :missing_host} - end + parse_keyword(input, default_port) else # Try as charlist parse_charlist(input, default_port) @@ -186,6 +176,20 @@ defmodule SnmpKit.SnmpLib.HostParser do {:error, :unsupported_format} end + defp parse_keyword(input, default_port) do + host = Keyword.get(input, :host) + port = Keyword.get(input, :port, default_port) + + if host do + with {:ok, {ip_tuple, _}} <- parse(host, default_port), + :ok <- validate_port(port) do + {:ok, {ip_tuple, port}} + end + else + {:error, :missing_host} + end + end + # Private helper functions defp parse_string(input, default_port) do @@ -238,18 +242,7 @@ defmodule SnmpKit.SnmpLib.HostParser do length(parts) == 2 -> # Likely IPv4:port [host_part, port_part] = parts - - case Integer.parse(port_part) do - {port, ""} -> - with {:ok, ip_tuple} <- parse_ipv4_address(host_part), - :ok <- validate_port(port) do - {:ok, {ip_tuple, port}} - end - - _ -> - # Port part is not a number, might be IPv6 - parse_ip_without_port(input, default_port) - end + parse_host_with_port(host_part, port_part, input, default_port) length(parts) > 2 -> # Definitely IPv6 @@ -260,6 +253,20 @@ defmodule SnmpKit.SnmpLib.HostParser do end end + defp parse_host_with_port(host_part, port_part, input, default_port) do + case Integer.parse(port_part) do + {port, ""} -> + with {:ok, ip_tuple} <- parse_ipv4_address(host_part), + :ok <- validate_port(port) do + {:ok, {ip_tuple, port}} + end + + _ -> + # Port part is not a number, might be IPv6 + parse_ip_without_port(input, default_port) + end + end + defp parse_ip_without_port(input, default_port) do parser = cond do diff --git a/lib/snmpkit/snmp_lib/manager.ex b/lib/snmpkit/snmp_lib/manager.ex index a4cb3e54..f7ded5ff 100644 --- a/lib/snmpkit/snmp_lib/manager.ex +++ b/lib/snmpkit/snmp_lib/manager.ex @@ -337,12 +337,7 @@ defmodule SnmpKit.SnmpLib.Manager do with {:ok, socket} <- create_socket(opts) do results = get_multi_with_socket(socket, host, normalized_oids, opts) :ok = close_socket(socket) - - # Check if all operations failed due to network issues - case check_for_global_failure(results) do - {:global_failure, reason} -> {:error, reason} - :mixed_results -> {:ok, results} - end + process_multi_results(results) end end end @@ -948,7 +943,7 @@ defmodule SnmpKit.SnmpLib.Manager do end # Check if all results failed with the same network-related error - defp check_for_global_failure(results) do + defp process_multi_results(results) do errors = Enum.filter(results, fn {_oid, {:error, _}} -> true @@ -969,14 +964,14 @@ defmodule SnmpKit.SnmpLib.Manager do ^same -> # All errors are network errors, return the first one as global failure {_oid, {:error, reason}} = hd(errors) - {:global_failure, reason} + {:error, reason} _ -> - :mixed_results + {:ok, results} end _ -> - :mixed_results + {:ok, results} end end end diff --git a/lib/towerops_web/controllers/api/v1/mib_controller.ex b/lib/towerops_web/controllers/api/v1/mib_controller.ex index 040f5052..04dceaeb 100644 --- a/lib/towerops_web/controllers/api/v1/mib_controller.ex +++ b/lib/towerops_web/controllers/api/v1/mib_controller.ex @@ -101,29 +101,36 @@ defmodule ToweropsWeb.Api.V1.MibController do def delete(conn, %{"vendor" => vendor}) do with :ok <- require_superuser(conn) do vendor_dir = Path.join(@mib_dir, vendor) + delete_vendor_mibs(conn, vendor, vendor_dir) + end + end - if File.exists?(vendor_dir) do - case File.rm_rf(vendor_dir) do - {:ok, _files} -> - Logger.info("Deleted MIB files for vendor: #{vendor}") + defp delete_vendor_mibs(conn, vendor, vendor_dir) do + if File.exists?(vendor_dir) do + remove_vendor_directory(conn, vendor, vendor_dir) + else + conn + |> put_status(:not_found) + |> json(%{error: "Vendor not found: #{vendor}"}) + end + end - json(conn, %{ - status: "ok", - message: "Deleted MIB files for vendor: #{vendor}" - }) + defp remove_vendor_directory(conn, vendor, vendor_dir) do + case File.rm_rf(vendor_dir) do + {:ok, _files} -> + Logger.info("Deleted MIB files for vendor: #{vendor}") - {:error, reason, file} -> - Logger.error("Failed to delete MIB files for #{vendor}: #{file} - #{inspect(reason)}") + json(conn, %{ + status: "ok", + message: "Deleted MIB files for vendor: #{vendor}" + }) + + {:error, reason, file} -> + Logger.error("Failed to delete MIB files for #{vendor}: #{file} - #{inspect(reason)}") - conn - |> put_status(:internal_server_error) - |> json(%{error: "Failed to delete MIB files"}) - end - else conn - |> put_status(:not_found) - |> json(%{error: "Vendor not found: #{vendor}"}) - end + |> put_status(:internal_server_error) + |> json(%{error: "Failed to delete MIB files"}) end end diff --git a/lib/towerops_web/user_auth.ex b/lib/towerops_web/user_auth.ex index 4261e3a4..77c62045 100644 --- a/lib/towerops_web/user_auth.ex +++ b/lib/towerops_web/user_auth.ex @@ -86,28 +86,7 @@ defmodule ToweropsWeb.UserAuth do superuser_id = get_session(conn, :superuser_id) target_user_id = get_session(conn, :target_user_id) - # Validate we have both IDs before attempting to fetch users - if superuser_id && target_user_id do - with superuser when not is_nil(superuser) <- Accounts.get_user(superuser_id), - target_user when not is_nil(target_user) <- Accounts.get_user(target_user_id) do - assign(conn, :current_scope, Scope.for_impersonation(superuser, target_user)) - else - _ -> - # Impersonation invalid, clear it - conn - |> delete_session(:superuser_id) - |> delete_session(:target_user_id) - |> delete_session(:impersonating) - |> assign(:current_scope, Scope.for_user(nil)) - end - else - # Missing IDs, clear invalid impersonation state - conn - |> delete_session(:superuser_id) - |> delete_session(:target_user_id) - |> delete_session(:impersonating) - |> assign(:current_scope, Scope.for_user(nil)) - end + fetch_impersonation_scope(conn, superuser_id, target_user_id) else # Normal authentication flow with {token, conn} <- ensure_user_token(conn), @@ -121,6 +100,31 @@ defmodule ToweropsWeb.UserAuth do end end + defp fetch_impersonation_scope(conn, superuser_id, target_user_id) + when is_binary(superuser_id) and is_binary(target_user_id) do + with superuser when not is_nil(superuser) <- Accounts.get_user(superuser_id), + target_user when not is_nil(target_user) <- Accounts.get_user(target_user_id) do + assign(conn, :current_scope, Scope.for_impersonation(superuser, target_user)) + else + _ -> + # Impersonation invalid, clear it + clear_impersonation_session(conn) + end + end + + defp fetch_impersonation_scope(conn, _superuser_id, _target_user_id) do + # Missing IDs, clear invalid impersonation state + clear_impersonation_session(conn) + end + + defp clear_impersonation_session(conn) do + conn + |> delete_session(:superuser_id) + |> delete_session(:target_user_id) + |> delete_session(:impersonating) + |> assign(:current_scope, Scope.for_user(nil)) + end + defp ensure_user_token(conn) do if token = get_session(conn, :user_token) do {token, conn} diff --git a/mix.lock b/mix.lock index a6a13ff3..b4e95a51 100644 --- a/mix.lock +++ b/mix.lock @@ -5,7 +5,7 @@ "cbor": {:hex, :cbor, "1.0.1", "39511158e8ea5a57c1fcb9639aaa7efde67129678fee49ebbda780f6f24959b0", [:mix], [], "hexpm", "5431acbe7a7908f17f6a9cd43311002836a34a8ab01876918d8cfb709cd8b6a2"}, "cc_precompiler": {:hex, :cc_precompiler, "0.1.11", "8c844d0b9fb98a3edea067f94f616b3f6b29b959b6b3bf25fee94ffe34364768", [:mix], [{:elixir_make, "~> 0.7", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "3427232caf0835f94680e5bcf082408a70b48ad68a5f5c0b02a3bea9f3a075b9"}, "comeonin": {:hex, :comeonin, "5.5.1", "5113e5f3800799787de08a6e0db307133850e635d34e9fab23c70b6501669510", [:mix], [], "hexpm", "65aac8f19938145377cee73973f192c5645873dcf550a8a6b18187d17c13ccdb"}, - "credo": {:hex, :credo, "1.7.15", "283da72eeb2fd3ccf7248f4941a0527efb97afa224bcdef30b4b580bc8258e1c", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "291e8645ea3fea7481829f1e1eb0881b8395db212821338e577a90bf225c5607"}, + "credo": {:hex, :credo, "1.7.16", "a9f1389d13d19c631cb123c77a813dbf16449a2aebf602f590defa08953309d4", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "d0562af33756b21f248f066a9119e3890722031b6d199f22e3cf95550e4f1579"}, "db_connection": {:hex, :db_connection, "2.9.0", "a6a97c5c958a2d7091a58a9be40caf41ab496b0701d21e1d1abff3fa27a7f371", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "17d502eacaf61829db98facf6f20808ed33da6ccf495354a41e64fe42f9c509c"}, "decimal": {:hex, :decimal, "2.3.0", "3ad6255aa77b4a3c4f818171b12d237500e63525c2fd056699967a3e7ea20f62", [:mix], [], "hexpm", "a4d66355cb29cb47c3cf30e71329e58361cfcb37c34235ef3bf1d7bf3773aeac"}, "dialyxir": {:hex, :dialyxir, "1.4.7", "dda948fcee52962e4b6c5b4b16b2d8fa7d50d8645bbae8b8685c3f9ecb7f5f4d", [:mix], [{:erlex, ">= 0.2.8", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "b34527202e6eb8cee198efec110996c25c5898f43a4094df157f8d28f27d9efe"},