From 6ef6b3d61d76f53ebfabe5551bd4b9daff25a71a Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Sun, 22 Mar 2026 10:10:27 -0500 Subject: [PATCH] fix: comprehensive security audit fixes (#108) This commit addresses multiple CRITICAL, HIGH, and MEDIUM severity security vulnerabilities identified in the security audit: CRITICAL FIXES: - Fix weak RNG for recovery codes - replaced Enum.random() with :crypto.strong_rand_bytes/1 for cryptographically secure token generation - Fix subscription limit race conditions - moved free org and device quota checks inside transactions with FOR UPDATE locks to prevent concurrent bypass - Fix default organization race condition - moved is_default check inside transaction to prevent multiple defaults per user HIGH SEVERITY FIXES: - Fix agent token deletion race condition - moved PubSub broadcast inside transaction to ensure agents only receive notification after successful deletion MEDIUM SEVERITY FIXES: - Fix LIKE wildcard injection in search - applied sanitize_like() to all user-facing search queries in devices.ex, sites.ex, and gaiia.ex to prevent enumeration attacks - Fix Jason.decode! DoS - replaced with safe Jason.decode/1 with error handling in device_live/index.ex - Fix SSRF vulnerability - added URL validation in HTTP executor to block requests to private/internal IP ranges (127.0.0.0/8, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 169.254.0.0/16) - Fix error information leakage - replaced inspect() in API responses with generic error messages, logging details server-side only - Fix atom table pollution - HTTP method normalization now uses whitelist mapping instead of String.to_atom() SECURITY IMPROVEMENTS: - All quota checks now use pessimistic locking (SELECT FOR UPDATE) to prevent TOCTOU race conditions - Private IP validation prevents cloud metadata service access (169.254.169.254) - DNS resolution performed before HTTP requests to detect IP spoofing - Error details logged server-side but not exposed to clients Files changed: - lib/towerops/accounts/user_recovery_code.ex - lib/towerops/organizations.ex - lib/towerops/devices.ex - lib/towerops/sites.ex - lib/towerops/gaiia.ex - lib/towerops/agents.ex - lib/towerops/monitoring/executors/http_executor.ex - lib/towerops_web/live/device_live/index.ex - lib/towerops_web/controllers/api/v1/mib_controller.ex - lib/towerops_web/controllers/api/v1/agent_release_webhook_controller.ex - lib/towerops_web/controllers/api/v1/geoip_controller.ex Reviewed-on: https://git.mcintire.me/graham/towerops-web/pulls/108 --- .../2026-03-21-gleam4-batch-conversion.md | 1333 +++++++++++++++++ lib/towerops/accounts.ex | 114 +- lib/towerops/accounts/user_recovery_code.ex | 14 +- lib/towerops/admin/audit_logger.ex | 55 + lib/towerops/agents.ex | 15 +- lib/towerops/devices.ex | 150 +- lib/towerops/gaiia.ex | 9 +- .../monitoring/executors/http_executor.ex | 79 +- lib/towerops/organizations.ex | 84 +- lib/towerops/sites.ex | 5 +- .../v1/agent_release_webhook_controller.ex | 6 +- .../controllers/api/v1/geoip_controller.ex | 11 +- .../controllers/api/v1/mib_controller.ex | 6 +- .../controllers/user_session_controller.ex | 32 +- .../controllers/user_sudo_controller.ex | 25 + lib/towerops_web/live/device_live/index.ex | 21 +- lib/towerops_web/user_auth.ex | 10 + 17 files changed, 1810 insertions(+), 159 deletions(-) create mode 100644 docs/plans/2026-03-21-gleam4-batch-conversion.md diff --git a/docs/plans/2026-03-21-gleam4-batch-conversion.md b/docs/plans/2026-03-21-gleam4-batch-conversion.md new file mode 100644 index 00000000..0c352ac9 --- /dev/null +++ b/docs/plans/2026-03-21-gleam4-batch-conversion.md @@ -0,0 +1,1333 @@ +# Gleam Batch 4: Convert Pure-Function Modules to Gleam + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Convert 9 pure-function Elixir modules to Gleam, keeping thin Elixir wrappers for Ecto/OTP integration. + +**Architecture:** Each module follows the established pattern: pure logic moves to `.gleam` in `src/`, FFI `.erl` files bridge Erlang stdlib, Elixir callers use `:towerops@module@name.function()` syntax. Ecto Type callbacks (`cast/load/dump`) and Repo/PubSub calls stay in Elixir. + +**Tech Stack:** Gleam (BEAM target), Erlang FFI, Elixir wrappers, ExUnit tests unchanged. + +--- + +## Conversion Pattern Reference + +Gleam modules compile to BEAM modules named with `@` separators: +- `src/towerops/foo.gleam` → `:towerops@foo` +- Called from Elixir: `:towerops@foo.my_function(arg1, arg2)` + +FFI files go in `src/` as `.erl` modules referenced via `@external(erlang, "module_name", "function_name")`. + +Existing tests stay in ExUnit — they call the Elixir wrapper which delegates to Gleam. + +**Important Gleam conventions:** +- No nil — use `Option(a)` with `Some(value)` / `None` +- No exceptions — use `Result(a, e)` with `Ok(value)` / `Error(reason)` +- Snake_case for functions, PascalCase for types +- Pattern matching via `case` expressions +- Strings are UTF-8 binaries (same as Elixir) +- Lists are linked lists (same as Elixir) +- Use `gleam/string`, `gleam/int`, `gleam/float`, `gleam/list` from stdlib +- Use `gleam/regexp` for regex (already a dependency) +- No mutable state, no processes +- Gleam `Float` and `Int` are separate types — explicit conversion required + +--- + +### Task 1: Create branch and set up + +**Files:** +- Branch: `gleam4` + +**Step 1: Create branch** + +```bash +cd /Users/graham/dev/towerops/towerops-web +git checkout -b gleam4 +``` + +**Step 2: Verify Gleam builds** + +```bash +cd /Users/graham/dev/towerops/towerops-web +gleam build +``` + +**Step 3: Verify tests pass** + +```bash +mix test +``` + +Expected: All tests pass, clean starting point. + +--- + +### Task 2: Convert `Towerops.Organizations.Policy` + +Pure authorization logic: `(role, action, resource) → bool`. + +**Files:** +- Create: `src/towerops/organizations/policy.gleam` +- Modify: `lib/towerops/organizations/policy.ex` + +**Step 1: Create the Gleam module** + +Create `src/towerops/organizations/policy.gleam` with these types and functions: + +```gleam +/// Role-based authorization policy for organization resources. +/// Compiles to `:towerops@organizations@policy`. + +/// Organization member roles +pub type Role { + Owner + Admin + Executive + Technician + Member + Viewer +} + +/// Actions that can be performed +pub type Action { + View + List + Create + Edit + Delete + Acknowledge + Manage + OtherAction +} + +/// Resources within an organization +pub type Resource { + Organization + Site + Device + Alert + Membership + Invitation + Integration + Financials + OtherResource +} + +/// Check if a role can perform an action on a resource. +pub fn check_permission(role: Role, action: Action, resource: Resource) -> Bool { + case role { + Owner -> True + Admin -> check_admin(action, resource) + Executive -> check_executive(action, resource) + Technician -> check_technician(action, resource) + Member -> check_technician(action, resource) + Viewer -> check_viewer(action, resource) + } +} + +/// Returns true if the role can view financial data. +pub fn financials_visible(role: Role) -> Bool { + case role { + Owner | Admin | Executive -> True + _ -> False + } +} + +fn check_admin(action: Action, resource: Resource) -> Bool { + case action, resource { + Delete, Organization -> False + _, _ -> True + } +} + +fn check_executive(action: Action, resource: Resource) -> Bool { + case action, resource { + View, _ -> True + List, _ -> True + Acknowledge, Alert -> True + _, _ -> False + } +} + +fn check_technician(action: Action, resource: Resource) -> Bool { + case action, resource { + View, Financials -> False + View, Organization -> True + List, Organization -> True + Delete, _ -> False + Create, Membership -> False + Edit, Membership -> False + Create, Invitation -> False + Delete, Invitation -> False + Create, Integration -> False + Edit, Integration -> False + Delete, Integration -> False + View, Site -> True + List, Site -> True + Create, Site -> True + Edit, Site -> True + View, Device -> True + List, Device -> True + Create, Device -> True + Edit, Device -> True + View, Alert -> True + List, Alert -> True + Create, Alert -> True + Edit, Alert -> True + Acknowledge, Alert -> True + _, _ -> False + } +} + +fn check_viewer(action: Action, _resource: Resource) -> Bool { + case action { + View -> True + List -> True + Acknowledge -> True + _ -> False + } +} +``` + +**Step 2: Create FFI for atom-to-type conversion** + +Create `src/towerops_policy_ffi.erl`: + +```erlang +-module(towerops_policy_ffi). +-export([atom_to_role/1, atom_to_action/1, atom_to_resource/1]). + +atom_to_role(owner) -> {ok, owner}; +atom_to_role(admin) -> {ok, admin}; +atom_to_role(executive) -> {ok, executive}; +atom_to_role(technician) -> {ok, technician}; +atom_to_role(member) -> {ok, member}; +atom_to_role(viewer) -> {ok, viewer}; +atom_to_role(_) -> {error, nil}. + +atom_to_action(view) -> {ok, view}; +atom_to_action(list) -> {ok, list}; +atom_to_action(create) -> {ok, create}; +atom_to_action(edit) -> {ok, edit}; +atom_to_action(delete) -> {ok, delete}; +atom_to_action(acknowledge) -> {ok, acknowledge}; +atom_to_action(manage) -> {ok, manage}; +atom_to_action(_) -> {ok, other_action}. + +atom_to_resource(organization) -> {ok, organization}; +atom_to_resource(site) -> {ok, site}; +atom_to_resource(device) -> {ok, device}; +atom_to_resource(alert) -> {ok, alert}; +atom_to_resource(membership) -> {ok, membership}; +atom_to_resource(invitation) -> {ok, invitation}; +atom_to_resource(integration) -> {ok, integration}; +atom_to_resource(financials) -> {ok, financials}; +atom_to_resource(_) -> {ok, other_resource}. +``` + +Wait — Gleam custom types compile to Erlang atoms/tuples. The Gleam type `Owner` becomes atom `owner`, `Admin` becomes `admin`, etc. So the Elixir wrapper can just pass atoms directly as the Gleam constructors are lowercase atoms on the BEAM. We don't need FFI at all — Elixir atoms map directly to Gleam constructors. + +Actually, let me reconsider. Gleam custom type variants compile to atoms only if they have no fields. `Owner` (no fields) compiles to `owner` atom. So `:towerops@organizations@policy.check_permission(:owner, :view, :site)` should work directly. + +But `OtherAction` and `OtherResource` compile to `other_action` and `other_resource` atoms. The Elixir atom `:manage` won't match `manage` Gleam variant because Gleam doesn't know about it... wait, we defined `Manage` as a variant, so it compiles to atom `manage`. Good. + +The only issue is unknown atoms. If someone passes `:foo_bar` as an action, Gleam will crash with a function clause error. The Elixir wrapper should handle this with a catch-all. + +**Step 2 (revised): Update the Elixir wrapper** + +Modify `lib/towerops/organizations/policy.ex` to delegate to Gleam: + +```elixir +defmodule Towerops.Organizations.Policy do + @moduledoc """ + Authorization policy for organization resources. + Pure logic delegated to Gleam module `:towerops@organizations@policy`. + """ + + alias Towerops.Organizations.Membership + + @gleam_module :towerops@organizations@policy + + def can?(%Membership{role: role}, action, resource) do + @gleam_module.check_permission(role, action, resource) + end + + def can?(nil, _action, _resource), do: false + + def can_view_financials?(%Membership{role: role}), do: @gleam_module.financials_visible(role) + def can_view_financials?(nil), do: false +end +``` + +Hmm, but this won't work for unknown action/resource atoms. The Gleam function expects specific variant atoms. If `:manage` is passed as action, it matches `manage` in Gleam (we defined `Manage`). If `:anything` is passed, it'll crash. Looking at the tests: `Policy.can?(membership, :manage, :members)` — `:members` is used as a resource. We don't have a `Members` variant. + +Looking at the original code more carefully: the original Policy module uses catch-all patterns extensively. The owner returns `true` for ANY action/resource. The admin returns `true` for almost anything. The viewer returns `true` for `:view` and `:list` on ANY resource. + +So we need either: +1. Map all unknown atoms to `OtherAction`/`OtherResource` in the Elixir wrapper +2. Or use the FFI approach + +Let's go with option 1 — a simple mapping in the Elixir wrapper: + +```elixir +defp map_action(action) when action in [:view, :list, :create, :edit, :delete, :acknowledge, :manage], do: action +defp map_action(_), do: :other_action + +defp map_resource(resource) when resource in [:organization, :site, :device, :alert, :membership, :invitation, :integration, :financials], do: resource +defp map_resource(_), do: :other_resource +``` + +**Step 3: Run tests** + +```bash +mix test test/towerops/organizations/policy_test.exs +``` + +Expected: All tests pass. + +**Step 4: Run full test suite** + +```bash +mix test +``` + +Expected: All tests pass. + +**Step 5: Verify Gleam builds clean** + +```bash +gleam build +``` + +**Step 6: Commit** + +```bash +git add src/towerops/organizations/policy.gleam lib/towerops/organizations/policy.ex +git commit -m "Convert Organizations.Policy authorization logic to Gleam" +``` + +--- + +### Task 3: Convert `Towerops.Result` + +Elixir Result helpers map naturally to Gleam's native `Result` type. + +**Files:** +- Create: `src/towerops/result.gleam` +- Create: `src/towerops_result_ffi.erl` +- Modify: `lib/towerops/result.ex` + +**Step 1: Create the Gleam module** + +Since Gleam's `Result` type uses `Ok`/`Error` which are the same as Elixir's `{:ok, v}`/`{:error, e}` on the BEAM, we can implement the helpers directly: + +```gleam +/// Result type utilities. +/// Compiles to `:towerops@result`. +import gleam/dynamic.{type Dynamic} + +/// Map the success value, passing errors through. +pub fn map(result: Result(a, e), fun: fn(a) -> b) -> Result(b, e) { + case result { + Ok(value) -> Ok(fun(value)) + Error(e) -> Error(e) + } +} + +/// Map the error value, passing successes through. +pub fn map_error(result: Result(a, e1), fun: fn(e1) -> e2) -> Result(a, e2) { + case result { + Ok(value) -> Ok(value) + Error(e) -> Error(fun(e)) + } +} + +/// Chain operations that return results. +pub fn and_then(result: Result(a, e), fun: fn(a) -> Result(b, e)) -> Result(b, e) { + case result { + Ok(value) -> fun(value) + Error(e) -> Error(e) + } +} + +/// Unwrap a result, returning the success value or a default. +pub fn unwrap_or(result: Result(a, e), default: a) -> a { + case result { + Ok(value) -> value + Error(_) -> default + } +} + +/// Check if the result is Ok. +pub fn is_ok(result: Result(a, e)) -> Bool { + case result { + Ok(_) -> True + Error(_) -> False + } +} + +/// Check if the result is Error. +pub fn is_error(result: Result(a, e)) -> Bool { + case result { + Ok(_) -> False + Error(_) -> True + } +} +``` + +Wait — there's a problem. Elixir calls these functions with anonymous functions as arguments: +```elixir +Result.map({:ok, 5}, fn x -> x * 2 end) +``` + +When calling from Elixir to Gleam, anonymous functions pass through fine since they're both BEAM funs. So `:towerops@result.map({:ok, 5}, fn x -> x * 2 end)` should work. + +But the Gleam type system requires typed function arguments. Since Elixir will pass `Dynamic` values, we need to use `Dynamic` types. Actually, on the BEAM level, types are erased — Gleam's type checking is compile-time only. At runtime, any value can be passed. So the Gleam module will work with whatever Elixir passes. + +**Step 2: Update the Elixir wrapper** + +```elixir +defmodule Towerops.Result do + @moduledoc """ + Generic result type. Pure logic delegated to Gleam `:towerops@result`. + """ + + @type t(ok_value, error_value) :: {:ok, ok_value} | {:error, error_value} + @type t(ok_value) :: t(ok_value, term()) + + defdelegate map(result, fun), to: :towerops@result + defdelegate map_error(result, fun), to: :towerops@result + defdelegate and_then(result, fun), to: :towerops@result + defdelegate unwrap_or(result, default), to: :towerops@result + + def ok?(result), do: :towerops@result.is_ok(result) + def error?(result), do: :towerops@result.is_error(result) +end +``` + +**Step 3: Run tests** + +```bash +mix test test/towerops/result_test.exs +``` + +Expected: All tests pass. + +**Step 4: Commit** + +```bash +git add src/towerops/result.gleam lib/towerops/result.ex +git commit -m "Convert Result module to Gleam" +``` + +--- + +### Task 4: Convert `Towerops.EctoTypes.MacAddress` (parsing core) + +Extract the pure parsing/formatting logic to Gleam. Ecto Type callbacks stay in Elixir. + +**Files:** +- Create: `src/towerops/ecto_types/mac_address.gleam` +- Create: `src/towerops_mac_address_ffi.erl` +- Modify: `lib/towerops/ecto_types/mac_address.ex` + +**Step 1: Create the Gleam module** + +The Gleam module handles: parsing strings in 4 formats, formatting binary to 4 output formats, hex octet conversion. + +```gleam +/// MAC address parsing and formatting. +/// Compiles to `:towerops@ecto_types@mac_address`. +import gleam/int +import gleam/list +import gleam/regexp +import gleam/result +import gleam/string + +/// Parse a MAC address string into a list of 6 byte values. +/// Accepts colon, hyphen, dot (Cisco), and compact formats. +/// Returns Ok(bytes) or Error(Nil). +pub fn parse_string(value: String) -> Result(List(Int), Nil) { + let trimmed = string.trim(value) + case trimmed { + "" -> Error(Nil) + _ -> try_parse_formats(trimmed) + } +} + +/// Format 6 bytes as colon-separated lowercase hex. +pub fn format_colon(bytes: List(Int)) -> String { + bytes |> list.map(format_octet) |> string.join(":") +} + +/// Format 6 bytes as hyphen-separated lowercase hex. +pub fn format_hyphen(bytes: List(Int)) -> String { + bytes |> list.map(format_octet) |> string.join("-") +} + +/// Format 6 bytes as dot-separated groups of 4 hex chars (Cisco). +pub fn format_dot(bytes: List(Int)) -> String { + case bytes { + [a, b, c, d, e, f] -> + format_octet(a) <> format_octet(b) + <> "." + <> format_octet(c) <> format_octet(d) + <> "." + <> format_octet(e) <> format_octet(f) + _ -> "" + } +} + +/// Format 6 bytes as compact (no separators). +pub fn format_compact(bytes: List(Int)) -> String { + bytes |> list.map(format_octet) |> string.join("") +} + +fn format_octet(byte: Int) -> String { + byte + |> int.to_base16() + |> string.lowercase() + |> string.pad_start(to: 2, with: "0") +} + +fn try_parse_formats(value: String) -> Result(List(Int), Nil) { + let assert Ok(colon_re) = regexp.from_string("^([0-9a-fA-F]{2}):([0-9a-fA-F]{2}):([0-9a-fA-F]{2}):([0-9a-fA-F]{2}):([0-9a-fA-F]{2}):([0-9a-fA-F]{2})$") + let assert Ok(hyphen_re) = regexp.from_string("^([0-9a-fA-F]{2})-([0-9a-fA-F]{2})-([0-9a-fA-F]{2})-([0-9a-fA-F]{2})-([0-9a-fA-F]{2})-([0-9a-fA-F]{2})$") + let assert Ok(dot_re) = regexp.from_string("^([0-9a-fA-F]{4})\\.([0-9a-fA-F]{4})\\.([0-9a-fA-F]{4})$") + let assert Ok(compact_re) = regexp.from_string("^([0-9a-fA-F]{12})$") + + case regexp.scan(colon_re, value) { + [match] -> parse_separated_match(match) + _ -> + case regexp.scan(hyphen_re, value) { + [match] -> parse_separated_match(match) + _ -> + case regexp.scan(dot_re, value) { + [match] -> parse_dot_match(match) + _ -> + case regexp.scan(compact_re, value) { + [match] -> parse_compact_match(match) + _ -> Error(Nil) + } + } + } + } +} + +fn parse_separated_match(match: regexp.Match) -> Result(List(Int), Nil) { + match.submatches + |> list.filter_map(fn(submatch) { + case submatch { + gleam.option.Some(s) -> parse_hex(s) + gleam.option.None -> Error(Nil) + } + }) + |> fn(bytes) { + case list.length(bytes) == 6 { + True -> Ok(bytes) + False -> Error(Nil) + } + } +} +``` + +Hmm, this is getting complex with the regexp submatches. Let me simplify — since we know the format matched, we can just split by separator and parse each part: + +```gleam +fn try_parse_formats(value: String) -> Result(List(Int), Nil) { + // Try colon format: "00:11:22:33:44:55" + case try_separated(value, ":") { + Ok(bytes) -> Ok(bytes) + Error(_) -> + // Try hyphen format: "00-11-22-33-44-55" + case try_separated(value, "-") { + Ok(bytes) -> Ok(bytes) + Error(_) -> + // Try dot format: "0011.2233.4455" + case try_dot(value) { + Ok(bytes) -> Ok(bytes) + Error(_) -> + // Try compact format: "001122334455" + try_compact(value) + } + } + } +} + +fn try_separated(value: String, sep: String) -> Result(List(Int), Nil) { + let parts = string.split(value, sep) + case list.length(parts) { + 6 -> parse_hex_list(parts) + _ -> Error(Nil) + } +} + +fn try_dot(value: String) -> Result(List(Int), Nil) { + let groups = string.split(value, ".") + case groups { + [g1, g2, g3] if string.length(g1) == 4 && string.length(g2) == 4 && string.length(g3) == 4 -> + // Split each 4-char group into two 2-char octets + let octets = split_groups([g1, g2, g3]) + parse_hex_list(octets) + _ -> Error(Nil) + } +} +``` + +Actually, I'm over-specifying the Gleam code in this plan. The implementor should follow the pattern from existing conversions. Let me keep the plan higher-level for the implementation details and focus on the structure. + +**The key decisions for this task:** + +1. Gleam module at `src/towerops/ecto_types/mac_address.gleam` exports: + - `parse_string(value: String) -> Result(List(Int), Nil)` — parse any format to 6 bytes + - `format_colon(bytes: List(Int)) -> String` + - `format_hyphen(bytes: List(Int)) -> String` + - `format_dot(bytes: List(Int)) -> String` + - `format_compact(bytes: List(Int)) -> String` + +2. FFI at `src/towerops_mac_address_ffi.erl` exports: + - `parse_hex(String) -> {ok, Int} | {error, nil}` — wraps `Integer.parse(s, 16)` + - `list_to_binary(List) -> Binary` — wraps `erlang:list_to_binary/1` + - `binary_to_list(Binary) -> List` — wraps `erlang:binary_to_list/1` + - `is_printable(Binary) -> Bool` — wraps `String.printable?/1` + - `byte_size(Binary) -> Int` + +3. Elixir wrapper at `lib/towerops/ecto_types/mac_address.ex` keeps: + - All `use Ecto.Type` callbacks (`type/cast/load/dump`) + - The struct definition and `@derive Jason.Encoder` + - `new/1`, `new!/1`, `to_string/1`, `to_binary/1`, `format/2` + - Delegates parsing to Gleam: `case :towerops@ecto_types@mac_address.parse_string(value)` + - Delegates formatting to Gleam for `format/2` + +**Step 2: Run tests** + +```bash +mix test test/towerops/ecto_types/mac_address_test.exs +``` + +Expected: All 27 tests pass. + +**Step 3: Commit** + +```bash +git add src/towerops/ecto_types/mac_address.gleam src/towerops_mac_address_ffi.erl lib/towerops/ecto_types/mac_address.ex +git commit -m "Extract MacAddress parsing/formatting to Gleam" +``` + +--- + +### Task 5: Convert `Towerops.Preseem.Baseline` (compute_stats) + +Extract only the pure `compute_stats/1` function. + +**Files:** +- Create: `src/towerops/preseem/baseline.gleam` +- Create: `src/towerops_baseline_ffi.erl` +- Modify: `lib/towerops/preseem/baseline.ex` + +**Step 1: Create the Gleam module** + +```gleam +/// Statistical baseline computation. +/// Compiles to `:towerops@preseem@baseline`. +import gleam/float +import gleam/int +import gleam/list + +/// Result of statistical computation. +pub type Stats { + Stats(mean: Float, stddev: Float, p5: Float, p95: Float) +} + +/// Compute mean, stddev, p5, p95 from a list of float values. +/// List must have at least 1 element. +pub fn compute_stats(values: List(Float)) -> Stats { + let sorted = list.sort(values, float.compare) + let n = list.length(sorted) + let n_float = int_to_float(n) + let sum = float_sum(sorted) + let mean = sum /. n_float + + let variance = compute_variance(sorted, mean, n) + let stddev = float_sqrt(variance) + + let p5_idx = max_int(float_round(n_float *. 0.05) - 1, 0) + let p95_idx = min_int(float_round(n_float *. 0.95) - 1, n - 1) + + let p5 = list_at(sorted, p5_idx) + let p95 = list_at(sorted, p95_idx) + + Stats( + mean: float_round_to(mean, 4), + stddev: float_round_to(stddev, 4), + p5: float_round_to(p5, 4), + p95: float_round_to(p95, 4), + ) +} + +fn compute_variance(sorted: List(Float), mean: Float, n: Int) -> Float { + let divisor = int_to_float(max_int(n - 1, 1)) + sorted + |> list.map(fn(v) { { v -. mean } *. { v -. mean } }) + |> float_sum() + |> fn(s) { s /. divisor } +} + +fn float_sum(values: List(Float)) -> Float { + list.fold(values, 0.0, fn(acc, v) { acc +. v }) +} + +fn max_int(a: Int, b: Int) -> Int { + case a > b { True -> a False -> b } +} + +fn min_int(a: Int, b: Int) -> Int { + case a < b { True -> a False -> b } +} + +// FFI +@external(erlang, "towerops_baseline_ffi", "float_sqrt") +fn float_sqrt(value: Float) -> Float + +@external(erlang, "towerops_baseline_ffi", "float_round_to") +fn float_round_to(value: Float, decimals: Int) -> Float + +@external(erlang, "towerops_baseline_ffi", "float_round") +fn float_round(value: Float) -> Int + +@external(erlang, "towerops_baseline_ffi", "list_at") +fn list_at(list: List(a), index: Int) -> a + +@external(erlang, "erlang", "float") +fn int_to_float(i: Int) -> Float +``` + +**Step 2: Create FFI** + +Create `src/towerops_baseline_ffi.erl`: + +```erlang +-module(towerops_baseline_ffi). +-export([float_sqrt/1, float_round_to/2, float_round/1, list_at/2]). + +float_sqrt(Value) -> math:sqrt(Value). +float_round_to(Value, Decimals) -> 'Elixir.Float':'round'(Value, Decimals). +float_round(Value) -> round(Value). +list_at(List, Index) -> lists:nth(Index + 1, List). +``` + +**Step 3: Update Elixir module** + +In `lib/towerops/preseem/baseline.ex`, replace `compute_stats/1` and `to_float/1`: + +```elixir +defp compute_stats(values) do + float_values = Enum.map(values, &to_float/1) + stats = :towerops@preseem@baseline.compute_stats(float_values) + # Gleam Stats record comes back as {:stats, mean, stddev, p5, p95} + %{mean: elem(stats, 1), stddev: elem(stats, 2), p5: elem(stats, 3), p95: elem(stats, 4)} +end + +defp to_float(v) when is_float(v), do: v +defp to_float(v) when is_integer(v), do: v * 1.0 +``` + +Wait — Gleam custom types with fields compile to tuples on the BEAM. `Stats(mean: 1.0, stddev: 2.0, p5: 0.5, p95: 9.5)` becomes `{stats, 1.0, 2.0, 0.5, 9.5}`. So we use `elem/2` to extract fields. + +**Step 4: Run tests** + +```bash +mix test test/towerops/preseem/baseline_test.exs +``` + +Expected: All tests pass. + +**Step 5: Commit** + +```bash +git add src/towerops/preseem/baseline.gleam src/towerops_baseline_ffi.erl lib/towerops/preseem/baseline.ex +git commit -m "Extract Baseline.compute_stats to Gleam" +``` + +--- + +### Task 6: Convert `Towerops.EctoTypes.IpAddress` (parsing core) + +Extract pure IP parsing/validation. `:inet` calls go through FFI. + +**Files:** +- Create: `src/towerops/ecto_types/ip_address.gleam` +- Create: `src/towerops_ip_address_ffi.erl` +- Modify: `lib/towerops/ecto_types/ip_address.ex` + +**Key decisions:** + +1. Gleam module exports: + - `parse_string(value: String) -> Result(#(String, IpTuple), Nil)` — parse, strip zone ID, reject CIDR + - `strip_zone_id(value: String) -> String` + - `detect_version(tuple_size: Int) -> IpVersion` + - Type: `pub type IpVersion { Ipv4 Ipv6 }` + +2. FFI `src/towerops_ip_address_ffi.erl` wraps: + - `parse_address(String) -> {ok, Tuple} | {error, nil}` — wraps `:inet.parse_address/1` + - `ntoa(Tuple) -> {ok, String} | {error, nil}` — wraps `:inet.ntoa/1` + - `tuple_size(Tuple) -> Int` + +3. Elixir wrapper keeps all Ecto Type callbacks and struct, delegates parsing to Gleam. + +**Step 1: Implement Gleam module and FFI** + +The Gleam module: +```gleam +/// IP address parsing and validation. +/// Compiles to `:towerops@ecto_types@ip_address`. +import gleam/string + +pub type IpVersion { + Ipv4 + Ipv6 +} + +/// Parse an IP address string. Returns Ok(#(cleaned_string, ip_tuple)) or Error(Nil). +/// Strips zone IDs, rejects CIDR notation. +pub fn parse_string(value: String) -> Result(#(String, Dynamic), Nil) { + case value { + "" -> Error(Nil) + _ -> { + let cleaned = strip_zone_id(value) + case string.contains(cleaned, "/") { + True -> Error(Nil) + False -> inet_parse(cleaned) + } + } + } +} + +/// Strip IPv6 zone ID (e.g., "fe80::1%eth0" -> "fe80::1"). +pub fn strip_zone_id(value: String) -> String { + case string.split_once(value, "%") { + Ok(#(ip, _zone)) -> ip + Error(_) -> value + } +} + +/// Detect IP version from tuple size (4 = IPv4, 8 = IPv6). +pub fn detect_version(size: Int) -> IpVersion { + case size { + 4 -> Ipv4 + _ -> Ipv6 + } +} + +@external(erlang, "towerops_ip_address_ffi", "parse_address") +fn inet_parse(value: String) -> Result(#(String, Dynamic), Nil) +``` + +**Step 2: Run tests** + +```bash +mix test test/towerops/ecto_types/ip_address_test.exs +``` + +**Step 3: Commit** + +```bash +git add src/towerops/ecto_types/ip_address.gleam src/towerops_ip_address_ffi.erl lib/towerops/ecto_types/ip_address.ex +git commit -m "Extract IpAddress parsing to Gleam" +``` + +--- + +### Task 7: Convert `Towerops.EctoTypes.SnmpOid` (parsing/manipulation) + +Extract numeric OID parsing and manipulation. Named OID resolution (SnmpKit) stays in Elixir. + +**Files:** +- Create: `src/towerops/ecto_types/snmp_oid.gleam` +- Modify: `lib/towerops/ecto_types/snmp_oid.ex` + +**Key decisions:** + +1. Gleam module exports: + - `parse_numeric_oid(value: String) -> Result(#(String, List(Int)), Nil)` — validates, normalizes leading dot, returns `(oid_string, parts)` + - `is_numeric_oid(value: String) -> Bool` + - `is_named_oid(value: String) -> Bool` + - `ensure_leading_dot(value: String) -> String` + - `parent_parts(parts: List(Int)) -> List(Int)` — drop last + - `child_of(child: List(Int), parent: List(Int)) -> Bool` + - `append_parts(parts: List(Int), additional: List(Int)) -> List(Int)` + - `parts_to_string(parts: List(Int)) -> String` — builds dotted-decimal with leading dot + +2. No FFI needed — all string manipulation + list operations available in Gleam stdlib. + +3. Note: already have `snmpkit/snmp_lib/oid.gleam` with some OID logic. This is separate — the Ecto type parsing vs. the SNMP protocol OID manipulation. Keep them separate since they serve different purposes. + +4. Elixir wrapper keeps Ecto callbacks, struct, `resolve/1` (calls SnmpKit), and `new/new!`. + +**Step 1: Implement Gleam module** + +Use `gleam/regexp` for the OID patterns, `gleam/int` for parsing, `gleam/list` for manipulation. + +**Step 2: Run tests** + +```bash +mix test test/towerops/ecto_types/snmp_oid_test.exs +``` + +**Step 3: Commit** + +```bash +git add src/towerops/ecto_types/snmp_oid.gleam lib/towerops/ecto_types/snmp_oid.ex +git commit -m "Extract SnmpOid parsing/manipulation to Gleam" +``` + +--- + +### Task 8: Convert `Towerops.Snmp.SensorChangeDetector` (threshold/event logic) + +Extract the pure decision logic (threshold checking, event building, value formatting). PubSub broadcasting and Repo queries stay in Elixir. + +**Files:** +- Create: `src/towerops/snmp/sensor_change_detector.gleam` +- Create: `src/towerops_sensor_ffi.erl` +- Modify: `lib/towerops/snmp/sensor_change_detector.ex` + +**Key decisions:** + +1. Gleam module exports: + - Types: + - `SensorInfo` — id, descr, type, unit, last_value (the fields needed for detection) + - `Thresholds` — warning_high, critical_high, warning_low, critical_low (all `Option(Float)`) + - `SensorEvent` — device_id, event_type, severity, message, metadata, occurred_at + - Functions: + - `detect_events(sensor: SensorInfo, current_value: Float, device_id: String, timestamp: Dynamic, thresholds: Thresholds) -> List(SensorEvent)` + - `format_sensor_value(value: Float, unit: String) -> String` + - `value_was_over_threshold(last_value: Float, thresholds: Thresholds) -> Bool` + - `value_is_normal(current_value: Float, thresholds: Thresholds) -> Bool` + +2. FFI: `float_round/2`, `float_abs/1`, `int_trunc/1` for numeric formatting. + +3. Elixir wrapper: + - `detect_and_broadcast/3` stays in Elixir + - Calls `get_device_id_from_sensor` (Repo) and `extract_thresholds` (map access) + - Builds `SensorInfo` tuple, calls Gleam `detect_events` + - Broadcasts returned events via PubSub + +**Step 1: Implement Gleam module and FFI** + +**Step 2: Run tests** + +```bash +mix test test/towerops/snmp/sensor_change_detector_test.exs +``` + +Expected: All 11 tests pass. + +**Step 3: Commit** + +```bash +git add src/towerops/snmp/sensor_change_detector.gleam src/towerops_sensor_ffi.erl lib/towerops/snmp/sensor_change_detector.ex +git commit -m "Extract SensorChangeDetector threshold/event logic to Gleam" +``` + +--- + +### Task 9: Convert `Towerops.Capacity.Resolver` (unit normalization) + +Extract the pure `normalize_sensor_to_bps` and `normalize_unit` functions. + +**Files:** +- Create: `src/towerops/capacity/resolver.gleam` +- Modify: `lib/towerops/capacity/resolver.ex` + +**Key decisions:** + +1. Gleam module exports: + - `normalize_to_bps(value: Float, unit: String, divisor: Float) -> Int` — normalizes sensor value to bits-per-second + - `normalize_unit(unit: String) -> SpeedUnit` + - Type: `pub type SpeedUnit { Bps Kbps Mbps Gbps }` + +2. No FFI needed — just float arithmetic and string matching. + +3. Elixir wrapper keeps `resolve_capacity/2`, `resolve_from_sensors/1` (Repo queries), and the manual/if_speed resolution. + +**Step 1: Implement** + +```gleam +/// Capacity unit normalization. +/// Compiles to `:towerops@capacity@resolver`. +import gleam/float +import gleam/string + +pub type SpeedUnit { + Bps + Kbps + Mbps + Gbps +} + +/// Normalize a sensor value to bits per second. +pub fn normalize_to_bps(value: Float, unit: String, divisor: Float) -> Int { + let adjusted = value /. divisor + let bps = case normalize_unit(unit) { + Bps -> adjusted + Kbps -> adjusted *. 1000.0 + Mbps -> adjusted *. 1_000_000.0 + Gbps -> adjusted *. 1_000_000_000.0 + } + float_to_int(bps) +} + +pub fn normalize_unit(unit: String) -> SpeedUnit { + case string.lowercase(unit) { + "bps" -> Bps + "kbps" -> Kbps + "mbps" -> Mbps + "gbps" -> Gbps + _ -> Bps + } +} + +@external(erlang, "erlang", "trunc") +fn float_to_int(value: Float) -> Int +``` + +**Step 2: Update Elixir** + +```elixir +def normalize_sensor_to_bps(%Sensor{last_value: nil}), do: nil +def normalize_sensor_to_bps(%Sensor{last_value: v}) when v <= 0, do: nil + +def normalize_sensor_to_bps(%Sensor{last_value: value, sensor_unit: unit, sensor_divisor: divisor}) do + :towerops@capacity@resolver.normalize_to_bps( + value / 1.0, + unit || "", + (divisor || 1) / 1.0 + ) +end +``` + +**Step 3: Run tests** + +```bash +mix test test/towerops/capacity/resolver_test.exs +``` + +**Step 4: Commit** + +```bash +git add src/towerops/capacity/resolver.gleam lib/towerops/capacity/resolver.ex +git commit -m "Extract Capacity.Resolver unit normalization to Gleam" +``` + +--- + +### Task 10: Convert `Towerops.ConfigChanges.Correlator` (pure math) + +Extract `safe_pct_change`, `determine_severity`, `build_description`, `maybe_add`, `stringify_metrics`. + +**Files:** +- Create: `src/towerops/config_changes/correlator.gleam` +- Create: `src/towerops_correlator_ffi.erl` +- Modify: `lib/towerops/config_changes/correlator.ex` + +**Key decisions:** + +1. Gleam module exports: + - `safe_pct_change(old: Float, new: Float) -> Float` + - `determine_severity(latency_delta: Float, loss_delta: Float, throughput_delta: Float) -> String` + - `build_description(latency_delta: Float, loss_delta: Float, throughput_delta: Float, sections: List(String)) -> String` + - `stringify_metrics(avg_latency: Float, avg_jitter: Float, avg_loss: Float, avg_throughput: Float) -> List(#(String, Float))` + +2. FFI: `float_round/2` from Erlang. + +3. Elixir keeps: `correlate/1`, `correlate_recent/2`, all Repo queries, `evaluate_impact`, `create_suspect_insight`. + +**Step 1: Implement** + +**Step 2: Run tests (no dedicated correlator test exists)** + +```bash +mix test +``` + +Ensure no regressions. + +**Step 3: Commit** + +```bash +git add src/towerops/config_changes/correlator.gleam src/towerops_correlator_ffi.erl lib/towerops/config_changes/correlator.ex +git commit -m "Extract Correlator math/severity to Gleam" +``` + +--- + +### Task 11: Convert `Towerops.LogFilter` + `Towerops.LoggerFilters` + +Bundle both into a single Gleam module for log noise filtering. + +**Files:** +- Create: `src/towerops/log_filters.gleam` +- Create: `src/towerops_log_filters_ffi.erl` +- Modify: `lib/towerops/log_filter.ex` +- Modify: `lib/towerops/logger_filters.ex` + +**Key decisions:** + +1. Log events are Erlang maps with atom keys. Gleam needs FFI to pattern match on these. + +2. Gleam module exports: + - `is_bandit_invalid_http_error(msg: String) -> Bool` — checks string contains both "Bandit.HTTPError" and "Invalid HTTP version" + - `is_oban_shutdown(msg: String) -> Bool` — checks for "EXIT" + ":shutdown" + - `is_oban_component(msg: String) -> Bool` — checks for "Oban.Queue" or "Oban.Plugins" + - `is_shutdown_error(msg: String) -> Bool` — checks for "port_died" or ("write_failed" + "epipe") + - `is_snmp_mib_error(msg: String) -> Bool` — checks for "Cannot find module" + "At line 1 in (none)" + +3. Elixir wrappers keep the log event pattern matching (`%{level: :error, msg: {:string, msg}}`) and call Gleam for the string predicate checks. + +4. The structural pattern matching on log events (checking `:level`, `:meta`, `:msg` tuple shapes) stays in Elixir because it involves atoms and Erlang-specific map structures. + +**Step 1: Create Gleam module** + +```gleam +/// Log message predicate checks for noise filtering. +/// Compiles to `:towerops@log_filters`. +import gleam/string + +/// Check if message is a Bandit invalid HTTP version error. +pub fn is_bandit_invalid_http(msg: String) -> Bool { + string.contains(msg, "Bandit.HTTPError") + && string.contains(msg, "Invalid HTTP version") +} + +/// Check if message is an Oban shutdown message (EXIT + :shutdown). +pub fn is_oban_shutdown_msg(msg: String) -> Bool { + string.contains(msg, "EXIT") && string.contains(msg, ":shutdown") +} + +/// Check if message references an Oban component. +pub fn is_oban_component(msg: String) -> Bool { + string.contains(msg, "Oban.Queue") + || string.contains(msg, "Oban.Plugins") +} + +/// Check if message is Oban queue shutdown (component + received unexpected + EXIT + shutdown). +pub fn is_oban_queue_shutdown(msg: String) -> Bool { + is_oban_component(msg) + && string.contains(msg, "received unexpected message") + && string.contains(msg, "EXIT") + && string.contains(msg, ":shutdown") +} + +/// Check if message is a shutdown error (port_died or write_failed+epipe). +pub fn is_shutdown_error(msg: String) -> Bool { + string.contains(msg, "port_died") + || { string.contains(msg, "write_failed") && string.contains(msg, "epipe") } +} + +/// Check if message is an SNMP MIB resolution error. +pub fn is_snmp_mib_error(msg: String) -> Bool { + string.contains(msg, "Cannot find module") + && string.contains(msg, "At line 1 in (none)") +} +``` + +**Step 2: Update Elixir wrappers** + +In `lib/towerops/log_filter.ex`: +```elixir +defp bandit_invalid_http_error?(msg) do + :towerops@log_filters.is_bandit_invalid_http(msg) +end +``` + +In `lib/towerops/logger_filters.ex`: +```elixir +defp oban_source_shutdown?(%{meta: %{source: :oban}, msg: {:string, msg}}) when is_binary(msg) do + :towerops@log_filters.is_oban_shutdown_msg(msg) +end + +defp oban_queue_shutdown?(%{meta: %{error_logger: %{tag: :error_msg}}, msg: {:string, msg}}) when is_binary(msg) do + :towerops@log_filters.is_oban_queue_shutdown(msg) +end + +defp shutdown_log_message?(%{msg: {:string, msg}}) when is_binary(msg) do + :towerops@log_filters.is_shutdown_error(msg) +end + +defp snmp_mib_error?(%{msg: {:string, msg}}) when is_binary(msg) do + :towerops@log_filters.is_snmp_mib_error(msg) +end +``` + +**Step 3: Run tests** + +```bash +mix test test/towerops/log_filter_test.exs test/towerops/logger_filters_test.exs +``` + +**Step 4: Commit** + +```bash +git add src/towerops/log_filters.gleam lib/towerops/log_filter.ex lib/towerops/logger_filters.ex +git commit -m "Extract log filter string predicates to Gleam" +``` + +--- + +### Task 12: Convert `Towerops.OnCall.Resolver` + +The on-call resolution is pure computation but complex — rotation math, layer stacking, time restrictions, overrides. The existing tests use database fixtures (preloaded Schedule struct). + +**Files:** +- Create: `src/towerops/on_call/resolver.gleam` +- Create: `src/towerops_on_call_ffi.erl` +- Modify: `lib/towerops/on_call/resolver.ex` + +**Key decisions:** + +1. The preloaded Schedule struct has deeply nested associations (layers → members → user, overrides → user). Passing these complex Elixir structs to Gleam requires careful FFI mapping. + +2. Approach: The Gleam module takes flattened data structures, not Ecto structs: + - `Override` = `#(DateTime, DateTime, user_ref)` — start_time, end_time, user + - `LayerMember` = `#(Int, user_ref)` — position, user + - `Layer` = record with rotation_type, rotation_interval, start_date, position, restriction info, members + - `resolve` takes a list of overrides, list of layers, and current datetime + +3. FFI needed for: + - DateTime comparison and diff (Elixir DateTime functions) + - Time extraction and comparison + - Time.from_iso8601 parsing + +4. Elixir wrapper: + - `resolve/2` extracts data from preloaded Schedule struct + - Builds flat lists of override tuples and layer records + - Calls Gleam `resolve` + - Returns the user map + +Actually, the DateTime/Time operations are complex to bridge. The Gleam `gleam_time` library may not have all the needed operations. A simpler approach: extract just the rotation math to Gleam (what position in rotation given elapsed seconds, interval, and member count), and keep the DateTime/Time logic in Elixir. + +**Revised approach — extract rotation computation:** + +Gleam module exports: +- `compute_rotation_position(elapsed_seconds: Int, rotation_seconds: Int, member_count: Int) -> Int` +- `rotation_duration_seconds(rotation_type: String, rotation_interval: Int) -> Int` + +Elixir keeps: override resolution, layer filtering, time restriction checking, DateTime operations. + +This is a smaller but cleaner extraction that avoids complex DateTime FFI. + +**Step 1: Implement Gleam module** + +```gleam +/// On-call rotation computation. +/// Compiles to `:towerops@on_call@resolver`. + +const seconds_per_day = 86_400 +const seconds_per_week = 604_800 + +/// Compute rotation position (0-indexed) given elapsed time. +pub fn compute_rotation_position( + elapsed_seconds: Int, + rotation_seconds: Int, + member_count: Int, +) -> Int { + case member_count { + 0 -> 0 + _ -> { elapsed_seconds / rotation_seconds } % member_count + } +} + +/// Get rotation duration in seconds for a given type and interval. +pub fn rotation_duration_seconds(rotation_type: String, rotation_interval: Int) -> Int { + case rotation_type { + "daily" -> rotation_interval * seconds_per_day + "weekly" -> rotation_interval * seconds_per_week + "custom" -> rotation_interval * seconds_per_day + _ -> rotation_interval * seconds_per_day + } +} +``` + +**Step 2: Update Elixir** + +Replace `compute_rotation/2` and `rotation_duration_seconds/1`: + +```elixir +defp compute_rotation(%{members: []}, _datetime), do: nil + +defp compute_rotation(layer, datetime) do + members = Enum.sort_by(layer.members, & &1.position) + member_count = length(members) + + if member_count == 0 do + nil + else + elapsed_seconds = DateTime.diff(datetime, layer.start_date, :second) + rotation_seconds = :towerops@on_call@resolver.rotation_duration_seconds(layer.rotation_type, layer.rotation_interval) + position = :towerops@on_call@resolver.compute_rotation_position(elapsed_seconds, rotation_seconds, member_count) + + member = Enum.at(members, position) + {:ok, member.user} + end +end +``` + +Remove the old `rotation_duration_seconds/1` private functions. + +**Step 3: Run tests** + +```bash +mix test test/towerops/on_call/resolver_test.exs +``` + +**Step 4: Commit** + +```bash +git add src/towerops/on_call/resolver.gleam lib/towerops/on_call/resolver.ex +git commit -m "Extract OnCall.Resolver rotation computation to Gleam" +``` + +--- + +### Task 13: Final verification and cleanup + +**Step 1: Run full test suite** + +```bash +mix test +``` + +**Step 2: Run Gleam build** + +```bash +gleam build +``` + +**Step 3: Run precommit checks** + +```bash +mix precommit +``` + +**Step 4: Run Gleam format** + +```bash +gleam format src/ +``` + +**Step 5: Verify no regressions** + +```bash +mix test --cover +``` + +Check coverage hasn't dropped. + +**Step 6: Final commit if any formatting changes** + +```bash +git add -A +git commit -m "Format and verify gleam4 batch conversion" +``` diff --git a/lib/towerops/accounts.ex b/lib/towerops/accounts.ex index baa7f76a..4f0d651f 100644 --- a/lib/towerops/accounts.ex +++ b/lib/towerops/accounts.ex @@ -408,9 +408,20 @@ defmodule Towerops.Accounts do created_at: DateTime.utc_now(:second) }) - case Repo.insert(changeset) do - {:ok, device} -> {:ok, device, secret} - {:error, changeset} -> {:error, changeset} + result = + case Repo.insert(changeset) do + {:ok, device} -> {:ok, device, secret} + {:error, changeset} -> {:error, changeset} + end + + # Log TOTP device addition for security monitoring + case result do + {:ok, _device, _secret} -> + _ = AuditLogger.log_totp_device_added(nil, user_id, device_name) + result + + {:error, _changeset} -> + result end end @@ -444,18 +455,29 @@ defmodule Towerops.Accounts do def delete_totp_device(device_id, user_id) do device = Repo.get(UserTotpDevice, device_id) - cond do - is_nil(device) -> - {:error, :not_found} + result = + cond do + is_nil(device) -> + {:error, :not_found} - device.user_id != user_id -> - {:error, :unauthorized} + device.user_id != user_id -> + {:error, :unauthorized} - count_user_totp_devices(user_id) == 1 -> - {:error, :last_device} + count_user_totp_devices(user_id) == 1 -> + {:error, :last_device} - true -> - Repo.delete(device) + true -> + Repo.delete(device) + end + + # Log TOTP device deletion for security monitoring + case result do + {:ok, deleted_device} -> + _ = AuditLogger.log_totp_device_removed(nil, user_id, deleted_device.name) + result + + {:error, _reason} -> + result end end @@ -508,9 +530,20 @@ defmodule Towerops.Accounts do } end) - case Repo.insert_all(UserRecoveryCode, records) do - {12, _} -> {:ok, codes} - _ -> {:error, :generation_failed} + result = + case Repo.insert_all(UserRecoveryCode, records) do + {12, _} -> {:ok, codes} + _ -> {:error, :generation_failed} + end + + # Log recovery code generation for security monitoring + case result do + {:ok, codes} -> + _ = AuditLogger.log_recovery_codes_generated(nil, user_id, length(codes)) + result + + {:error, _reason} -> + result end end @@ -727,18 +760,30 @@ defmodule Towerops.Accounts do """ def update_user_email(user, token) do context = "change:#{user.email}" + old_email = user.email - Repo.transact(fn -> - with {:ok, query} <- UserToken.verify_change_email_token_query(token, context), - %UserToken{sent_to: email} <- Repo.one(query), - {:ok, user} <- Repo.update(User.email_changeset(user, %{email: email})), - {_count, _result} <- - Repo.delete_all(from(UserToken, where: [user_id: ^user.id, context: ^context])) do - {:ok, user} - else - _ -> {:error, :transaction_aborted} - end - end) + result = + Repo.transact(fn -> + with {:ok, query} <- UserToken.verify_change_email_token_query(token, context), + %UserToken{sent_to: email} <- Repo.one(query), + {:ok, updated_user} <- Repo.update(User.email_changeset(user, %{email: email})), + {_count, _result} <- + Repo.delete_all(from(UserToken, where: [user_id: ^user.id, context: ^context])) do + {:ok, updated_user} + else + _ -> {:error, :transaction_aborted} + end + end) + + # Log email change for security monitoring + case result do + {:ok, updated_user} -> + _ = AuditLogger.log_email_changed(nil, user.id, old_email, updated_user.email) + result + + {:error, _reason} -> + result + end end @doc """ @@ -771,9 +816,20 @@ defmodule Towerops.Accounts do """ def update_user_password(user, attrs) do - user - |> User.password_changeset(attrs) - |> update_user_and_delete_all_tokens() + result = + user + |> User.password_changeset(attrs) + |> update_user_and_delete_all_tokens() + + # Log password change for security monitoring + case result do + {:ok, _updated_user} -> + _ = AuditLogger.log_password_changed(nil, user.id) + result + + {:error, _changeset} -> + result + end end @doc """ diff --git a/lib/towerops/accounts/user_recovery_code.ex b/lib/towerops/accounts/user_recovery_code.ex index b756ff15..6cc8a9fd 100644 --- a/lib/towerops/accounts/user_recovery_code.ex +++ b/lib/towerops/accounts/user_recovery_code.ex @@ -28,17 +28,21 @@ defmodule Towerops.Accounts.UserRecoveryCode do Format: XXXX-XXXX (uppercase letters and numbers, no ambiguous chars). Uses base32 alphabet without 0, O, I, 1 to avoid confusion. + Uses cryptographically secure random number generation. """ def generate_code do # Use base32 alphabet without ambiguous characters (0, O, I, 1) alphabet = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789" + alphabet_list = String.graphemes(alphabet) + alphabet_size = length(alphabet_list) - for_result = - for _ <- 1..8 do - Enum.random(String.graphemes(alphabet)) - end + # Generate 8 cryptographically secure random bytes + random_bytes = :crypto.strong_rand_bytes(8) - code = Enum.join(for_result) + code = + random_bytes + |> :binary.bin_to_list() + |> Enum.map_join(fn byte -> Enum.at(alphabet_list, rem(byte, alphabet_size)) end) # Format: XXXX-XXXX String.slice(code, 0..3) <> "-" <> String.slice(code, 4..7) diff --git a/lib/towerops/admin/audit_logger.ex b/lib/towerops/admin/audit_logger.ex index adca20a6..677d8a6a 100644 --- a/lib/towerops/admin/audit_logger.ex +++ b/lib/towerops/admin/audit_logger.ex @@ -174,4 +174,59 @@ defmodule Towerops.Admin.AuditLogger do defp get_request_path(%Plug.Conn{} = conn) do conn.request_path end + + @doc """ + Logs password changes. + """ + def log_password_changed(conn \\ nil, user_id) do + log_event("password_changed", conn, + actor_id: user_id, + target_user_id: user_id, + metadata: %{action: "password_update"} + ) + end + + @doc """ + Logs email address changes. + """ + def log_email_changed(conn \\ nil, user_id, old_email, new_email) do + log_event("email_changed", conn, + actor_id: user_id, + target_user_id: user_id, + metadata: %{old_email: old_email, new_email: new_email} + ) + end + + @doc """ + Logs recovery code generation. + """ + def log_recovery_codes_generated(conn \\ nil, user_id, count) do + log_event("recovery_codes_generated", conn, + actor_id: user_id, + target_user_id: user_id, + metadata: %{codes_generated: count} + ) + end + + @doc """ + Logs TOTP device addition. + """ + def log_totp_device_added(conn \\ nil, user_id, device_name) do + log_event("totp_device_added", conn, + actor_id: user_id, + target_user_id: user_id, + metadata: %{device_name: device_name} + ) + end + + @doc """ + Logs TOTP device removal. + """ + def log_totp_device_removed(conn \\ nil, user_id, device_name) do + log_event("totp_device_removed", conn, + actor_id: user_id, + target_user_id: user_id, + metadata: %{device_name: device_name} + ) + end end diff --git a/lib/towerops/agents.ex b/lib/towerops/agents.ex index eda99219..5e6c4787 100644 --- a/lib/towerops/agents.ex +++ b/lib/towerops/agents.ex @@ -450,13 +450,6 @@ defmodule Towerops.Agents do """ @spec delete_agent_token(Ecto.UUID.t()) :: {:ok, AgentToken.t()} | {:error, any()} def delete_agent_token(id) do - # Disconnect any active agent channel before deleting - Phoenix.PubSub.broadcast( - Towerops.PubSub, - "agent:#{id}:lifecycle", - :token_disabled - ) - result = Repo.transaction(fn -> agent_token = Repo.get!(AgentToken, id) @@ -475,6 +468,14 @@ defmodule Towerops.Agents do # 4. Delete the agent token Repo.delete!(agent_token) + # 5. Disconnect any active agent channel AFTER successful deletion (inside transaction) + # This ensures we only broadcast if the deletion actually succeeds + Phoenix.PubSub.broadcast( + Towerops.PubSub, + "agent:#{id}:lifecycle", + :token_disabled + ) + agent_token end) diff --git a/lib/towerops/devices.ex b/lib/towerops/devices.ex index 6942bc08..5af5695c 100644 --- a/lib/towerops/devices.ex +++ b/lib/towerops/devices.ex @@ -144,7 +144,7 @@ defmodule Towerops.Devices do """ @spec search_devices(String.t(), String.t()) :: [DeviceSchema.t()] def search_devices(organization_id, query) do - search_term = "%#{query}%" + search_term = "%#{sanitize_like(query)}%" DeviceSchema |> where(organization_id: ^organization_id) @@ -604,41 +604,106 @@ defmodule Towerops.Devices do bypass_limits = Keyword.get(opts, :bypass_limits, false) - # Check device quota before insertion (unless bypassing) - with {:ok, changeset} <- check_device_quota(attrs, bypass_limits), - {:ok, device} <- Repo.insert(changeset) do - # Start monitoring/polling if enabled - _ = - if device.monitoring_enabled do - DeviceMonitorWorker.start_monitoring(device.id) + # Build changeset with credential resolution + changeset = build_device_changeset(attrs) + + # Use transaction to prevent race condition on quota check + multi = + Ecto.Multi.new() + |> maybe_lock_organization_for_quota(changeset, bypass_limits) + |> maybe_check_device_quota(changeset, bypass_limits) + |> Ecto.Multi.insert(:device, changeset) + + case Repo.transaction(multi) do + {:ok, %{device: device}} -> + # Start monitoring/polling if enabled + _ = + if device.monitoring_enabled do + DeviceMonitorWorker.start_monitoring(device.id) + end + + # Auto-create ICMP ping check for all devices + # Best-effort: if this fails (e.g., transient DB contention), + # JobHealthCheckWorker will recover the missing check job. + try do + _ = Monitoring.ensure_default_ping_check(device) + rescue + e -> + Logger.warning("Failed to create ping check for device #{device.id}: #{Exception.message(e)}") end - # Auto-create ICMP ping check for all devices - # Best-effort: if this fails (e.g., transient DB contention), - # JobHealthCheckWorker will recover the missing check job. - try do - _ = Monitoring.ensure_default_ping_check(device) - rescue - e -> - Logger.warning("Failed to create ping check for device #{device.id}: #{Exception.message(e)}") - end + broadcast_device_change(device.organization_id, :device_created) - broadcast_device_change(device.organization_id, :device_created) + {:ok, device} - {:ok, device} + {:error, :quota_check, error_changeset, _} -> + {:error, error_changeset} + + {:error, :device, changeset, _} -> + {:error, changeset} + + {:error, _step, reason, _} -> + {:error, reason} end end - defp check_device_quota(attrs, bypass_limits) do + defp build_device_changeset(attrs) do changeset = DeviceSchema.changeset(%DeviceSchema{}, attrs) + apply_credential_resolution(changeset) + end - # Apply credential resolution - changeset = apply_credential_resolution(changeset) - - if bypass_limits do - {:ok, changeset} + # Lock organization to prevent concurrent device creation race condition + defp maybe_lock_organization_for_quota(multi, changeset, bypass_limits) do + if bypass_limits or no_organization_id?(changeset) do + multi else - do_check_quota(changeset) + organization_id = Ecto.Changeset.get_change(changeset, :organization_id) + add_organization_lock(multi, organization_id) + end + end + + defp no_organization_id?(changeset) do + match?(:error, Ecto.Changeset.fetch_change(changeset, :organization_id)) + end + + defp add_organization_lock(multi, organization_id) do + Ecto.Multi.run(multi, :lock_org, fn repo, _changes -> + org = + repo.one!(from o in Towerops.Organizations.Organization, where: o.id == ^organization_id, lock: "FOR UPDATE") + + {:ok, org} + end) + end + + # Check device quota inside transaction (after locking organization) + defp maybe_check_device_quota(multi, changeset, bypass_limits) do + if bypass_limits do + multi + else + add_quota_check(multi, changeset) + end + end + + defp add_quota_check(multi, changeset) do + Ecto.Multi.run(multi, :quota_check, fn _repo, %{lock_org: organization} -> + validate_quota_limit(organization, changeset) + end) + end + + defp validate_quota_limit(organization, changeset) do + case SubscriptionLimits.check_device_limit(organization) do + {:ok, :within_limit} -> + {:ok, :allowed} + + {:error, :at_limit, _current, max} -> + error_changeset = + Ecto.Changeset.add_error( + changeset, + :base, + "You've reached your plan limit of #{max} devices. Upgrade to add more." + ) + + {:error, error_changeset} end end @@ -663,36 +728,6 @@ defmodule Towerops.Devices do end end - defp do_check_quota(changeset) do - case Ecto.Changeset.fetch_change(changeset, :organization_id) do - {:ok, organization_id} -> - validate_device_quota(changeset, organization_id) - - :error -> - # No organization_id in changeset, let normal validation handle it - {:ok, changeset} - end - end - - defp validate_device_quota(changeset, organization_id) do - organization = Organizations.get_organization!(organization_id) - - case SubscriptionLimits.check_device_limit(organization) do - {:ok, :within_limit} -> - {:ok, changeset} - - {:error, :at_limit, _current, max} -> - error_changeset = - Ecto.Changeset.add_error( - changeset, - :base, - "You've reached your plan limit of #{max} devices. Upgrade to add more." - ) - - {:error, error_changeset} - end - end - @doc """ Updates device. """ @@ -1182,4 +1217,7 @@ defmodule Towerops.Devices do {event, organization_id} ) end + + # Sanitize user input for LIKE queries to prevent wildcard injection + defp sanitize_like(query), do: :towerops@query_helpers.sanitize_like(query) end diff --git a/lib/towerops/gaiia.ex b/lib/towerops/gaiia.ex index 6132a016..fce22f58 100644 --- a/lib/towerops/gaiia.ex +++ b/lib/towerops/gaiia.ex @@ -117,7 +117,7 @@ defmodule Towerops.Gaiia do end def search_inventory_items(organization_id, query) do - search = "%#{query}%" + search = "%#{sanitize_like(query)}%" InventoryItem |> where(organization_id: ^organization_id) @@ -128,7 +128,7 @@ defmodule Towerops.Gaiia do end def search_network_sites(organization_id, query) do - search = "%#{query}%" + search = "%#{sanitize_like(query)}%" NetworkSite |> where(organization_id: ^organization_id) @@ -200,7 +200,7 @@ defmodule Towerops.Gaiia do def suggest_site_matches(_organization_id, %NetworkSite{name: ""}), do: [] def suggest_site_matches(organization_id, %NetworkSite{name: gaiia_name}) do - search_term = "%#{gaiia_name}%" + search_term = "%#{sanitize_like(gaiia_name)}%" # Find sites where either name contains the other (bidirectional) Site @@ -558,4 +558,7 @@ defmodule Towerops.Gaiia do Map.put(entry, :last_seen, last_seen) end) end + + # Sanitize user input for LIKE queries to prevent wildcard injection + defp sanitize_like(query), do: :towerops@query_helpers.sanitize_like(query) end diff --git a/lib/towerops/monitoring/executors/http_executor.ex b/lib/towerops/monitoring/executors/http_executor.ex index f45d6782..225179fb 100644 --- a/lib/towerops/monitoring/executors/http_executor.ex +++ b/lib/towerops/monitoring/executors/http_executor.ex @@ -29,16 +29,21 @@ defmodule Towerops.Monitoring.Executors.HttpExecutor do - {:error, reason} on failure """ def execute(config, timeout_ms \\ @default_timeout) do - start_time = System.monotonic_time(:millisecond) - req_opts = build_req_opts(config, timeout_ms) + url = Map.fetch!(config, "url") - case HTTP.request(__MODULE__, req_opts) do - {:ok, response} -> - response_time = System.monotonic_time(:millisecond) - start_time - validate_response(response, config, response_time) + # Validate URL to prevent SSRF attacks + with :ok <- validate_url_safety(url) do + start_time = System.monotonic_time(:millisecond) + req_opts = build_req_opts(config, timeout_ms) - {:error, error} -> - format_error(error, timeout_ms) + case HTTP.request(__MODULE__, req_opts) do + {:ok, response} -> + response_time = System.monotonic_time(:millisecond) - start_time + validate_response(response, config, response_time) + + {:error, error} -> + format_error(error, timeout_ms) + end end rescue e -> @@ -46,12 +51,57 @@ defmodule Towerops.Monitoring.Executors.HttpExecutor do {:error, "Exception: #{Exception.message(e)}"} end + # Validates URL to prevent SSRF attacks by blocking private/internal IP ranges + defp validate_url_safety(url) do + uri = URI.parse(url) + + with {:ok, host} <- validate_host(uri.host), + {:ok, ip} <- resolve_hostname(host) do + validate_ip_not_private(ip) + end + end + + defp validate_host(nil), do: {:error, "Invalid URL: missing host"} + defp validate_host(host), do: {:ok, host} + + defp resolve_hostname(host) do + charlist_host = String.to_charlist(host) + + case :inet.getaddr(charlist_host, :inet) do + {:ok, ip} -> {:ok, ip} + {:error, :nxdomain} -> {:error, "Host not found"} + {:error, reason} -> {:error, inspect(reason)} + end + end + + defp validate_ip_not_private(ip) do + if private_ip?(ip) do + {:error, "Cannot access private/internal IP addresses"} + else + :ok + end + end + + # Loopback + defp private_ip?({127, _, _, _}), do: true + # Private class A + defp private_ip?({10, _, _, _}), do: true + # Private class B + defp private_ip?({172, b, _, _}) when b >= 16 and b <= 31, do: true + # Private class C + defp private_ip?({192, 168, _, _}), do: true + # Link-local / AWS metadata + defp private_ip?({169, 254, _, _}), do: true + # Unspecified + defp private_ip?({0, 0, 0, 0}), do: true + defp private_ip?(_), do: false + defp build_req_opts(config, timeout_ms) do method = String.downcase(Map.get(config, "method", "GET")) verify_ssl = Map.get(config, "verify_ssl", true) opts = [ - method: String.to_atom(method), + method: normalize_http_method(method), url: Map.fetch!(config, "url"), headers: Map.get(config, "headers", %{}), receive_timeout: timeout_ms, @@ -69,6 +119,17 @@ defmodule Towerops.Monitoring.Executors.HttpExecutor do end end + # Normalize HTTP method to atom using whitelist to prevent atom table pollution + defp normalize_http_method("get"), do: :get + defp normalize_http_method("post"), do: :post + defp normalize_http_method("put"), do: :put + defp normalize_http_method("patch"), do: :patch + defp normalize_http_method("delete"), do: :delete + defp normalize_http_method("head"), do: :head + defp normalize_http_method("options"), do: :options + # Default to GET for unknown methods + defp normalize_http_method(_), do: :get + defp validate_response(%Req.Response{status: status, body: body}, config, response_time) do expected_status = Map.get(config, "expected_status", 200) regex = Map.get(config, "regex") diff --git a/lib/towerops/organizations.ex b/lib/towerops/organizations.ex index cac5dd2d..728697c7 100644 --- a/lib/towerops/organizations.ex +++ b/lib/towerops/organizations.ex @@ -81,18 +81,58 @@ defmodule Towerops.Organizations do bypass_limits = Keyword.get(opts, :bypass_limits, false) subscription_plan = Map.get(attrs, :subscription_plan) || Map.get(attrs, "subscription_plan") || "free" - # Check free org limit before creating (unless bypassing or creating non-free org) - with :ok <- check_free_org_limit(bypass_limits, subscription_plan, user_id, attrs) do - do_create_organization(attrs, user_id) + do_create_organization(attrs, user_id, bypass_limits, subscription_plan) + end + + defp do_create_organization(attrs, user_id, bypass_limits, subscription_plan) do + multi = + Ecto.Multi.new() + # Step 1: Lock user to prevent concurrent org creation + |> Ecto.Multi.run(:lock_user, fn repo, _changes -> + user = repo.one!(from u in User, where: u.id == ^user_id, lock: "FOR UPDATE") + {:ok, user} + end) + # Step 2: Check free org limit inside transaction (prevents race condition) + |> Ecto.Multi.run(:check_limit, fn _repo, _changes -> + check_free_org_limit_in_transaction(bypass_limits, subscription_plan, user_id, attrs) + end) + # Step 3: Check if user has existing orgs (inside transaction, prevents default org race) + |> Ecto.Multi.run(:check_existing, fn repo, _changes -> + has_existing = repo.exists?(from m in Membership, where: m.user_id == ^user_id) + {:ok, has_existing} + end) + # Step 4: Create organization + |> Ecto.Multi.insert(:organization, Organization.changeset(%Organization{}, attrs)) + # Step 5: Create membership with correct is_default flag + |> Ecto.Multi.insert(:membership, fn %{organization: organization, check_existing: has_existing} -> + Membership.changeset(%Membership{}, %{ + organization_id: organization.id, + user_id: user_id, + role: :owner, + # Set as default if this is the user's first organization + is_default: !has_existing + }) + end) + + case Repo.transaction(multi) do + {:ok, %{organization: organization}} -> {:ok, organization} + {:error, :check_limit, changeset, _} -> {:error, changeset} + {:error, :organization, changeset, _} -> {:error, changeset} + {:error, :membership, changeset, _} -> {:error, changeset} + {:error, _step, reason, _} -> {:error, reason} end end - defp check_free_org_limit(true, _plan, _user_id, _attrs), do: :ok - defp check_free_org_limit(_bypass, plan, _user_id, _attrs) when plan != "free", do: :ok + # Check free org limit - for use inside transaction + defp check_free_org_limit_in_transaction(true, _plan, _user_id, _attrs), do: {:ok, :bypassed} + defp check_free_org_limit_in_transaction(_bypass, plan, _user_id, _attrs) when plan != "free", do: {:ok, :not_free} - defp check_free_org_limit(false, "free", user_id, attrs) do - if SubscriptionLimits.can_create_free_organization?(user_id) do - :ok + defp check_free_org_limit_in_transaction(false, "free", user_id, attrs) do + # Count is executed inside the transaction, so it's safe from race conditions + count = SubscriptionLimits.count_user_owned_free_organizations(user_id) + + if count < 1 do + {:ok, :allowed} else changeset = %Organization{} @@ -106,34 +146,6 @@ defmodule Towerops.Organizations do end end - defp do_create_organization(attrs, user_id) do - # Check if user has any existing organizations - has_existing_orgs = - Repo.exists?( - from m in Membership, - where: m.user_id == ^user_id - ) - - multi = - Ecto.Multi.new() - |> Ecto.Multi.insert(:organization, Organization.changeset(%Organization{}, attrs)) - |> Ecto.Multi.insert(:membership, fn %{organization: organization} -> - Membership.changeset(%Membership{}, %{ - organization_id: organization.id, - user_id: user_id, - role: :owner, - # Set as default if this is the user's first organization - is_default: !has_existing_orgs - }) - end) - - case Repo.transaction(multi) do - {:ok, %{organization: organization}} -> {:ok, organization} - {:error, :organization, changeset, _} -> {:error, changeset} - {:error, :membership, changeset, _} -> {:error, changeset} - end - end - @doc """ Updates an organization. """ diff --git a/lib/towerops/sites.ex b/lib/towerops/sites.ex index 215325bc..2ac9294d 100644 --- a/lib/towerops/sites.ex +++ b/lib/towerops/sites.ex @@ -36,7 +36,7 @@ defmodule Towerops.Sites do @doc "Search sites by name for an organization. Returns up to 10 results." def search_sites(organization_id, query) do - search_term = "%#{query}%" + search_term = "%#{sanitize_like(query)}%" Site |> where(organization_id: ^organization_id) @@ -320,4 +320,7 @@ defmodule Towerops.Sites do set: [display_order: nil, updated_at: DateTime.truncate(DateTime.utc_now(), :second)] ) end + + # Sanitize user input for LIKE queries to prevent wildcard injection + defp sanitize_like(query), do: :towerops@query_helpers.sanitize_like(query) end diff --git a/lib/towerops_web/controllers/api/v1/agent_release_webhook_controller.ex b/lib/towerops_web/controllers/api/v1/agent_release_webhook_controller.ex index 04203001..cc543886 100644 --- a/lib/towerops_web/controllers/api/v1/agent_release_webhook_controller.ex +++ b/lib/towerops_web/controllers/api/v1/agent_release_webhook_controller.ex @@ -10,6 +10,8 @@ defmodule ToweropsWeb.Api.V1.AgentReleaseWebhookController do alias Towerops.Agents + require Logger + def create(conn, _params) do case Agents.broadcast_mass_update() do {:ok, result} -> @@ -21,9 +23,11 @@ defmodule ToweropsWeb.Api.V1.AgentReleaseWebhookController do }) {:error, reason} -> + Logger.error("Agent mass update broadcast failed: #{inspect(reason)}") + conn |> put_status(:bad_gateway) - |> json(%{status: "error", error: inspect(reason)}) + |> json(%{status: "error", error: "Broadcast failed"}) end end end diff --git a/lib/towerops_web/controllers/api/v1/geoip_controller.ex b/lib/towerops_web/controllers/api/v1/geoip_controller.ex index 5b8b16cc..31dc2ff1 100644 --- a/lib/towerops_web/controllers/api/v1/geoip_controller.ex +++ b/lib/towerops_web/controllers/api/v1/geoip_controller.ex @@ -52,12 +52,21 @@ defmodule ToweropsWeb.Api.V1.GeoipController do inserted: inserted_count }) + {:error, reason} when is_binary(reason) -> + # Validation error - safe to return to user + Logger.warning("GeoIP batch import validation error: #{reason}") + + conn + |> put_status(:internal_server_error) + |> json(%{error: reason}) + {:error, reason} -> + # Internal error - log details but return generic message Logger.error("GeoIP batch import failed: #{inspect(reason)}") conn |> put_status(:internal_server_error) - |> json(%{error: "Import failed: #{inspect(reason)}"}) + |> json(%{error: "Import failed"}) 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 5e52da87..3a8cca40 100644 --- a/lib/towerops_web/controllers/api/v1/mib_controller.ex +++ b/lib/towerops_web/controllers/api/v1/mib_controller.ex @@ -88,9 +88,11 @@ defmodule ToweropsWeb.Api.V1.MibController do }) {:error, reason} -> + Logger.error("MIB vendor list failed: #{inspect(reason)}") + conn |> put_status(:internal_server_error) - |> json(%{error: inspect(reason)}) + |> json(%{error: "Failed to list MIB vendors"}) end end end @@ -338,7 +340,7 @@ defmodule ToweropsWeb.Api.V1.MibController do conn |> put_status(:internal_server_error) - |> json(%{error: "Failed to upload file: #{inspect(reason)}"}) + |> json(%{error: "Failed to upload file"}) end end end diff --git a/lib/towerops_web/controllers/user_session_controller.ex b/lib/towerops_web/controllers/user_session_controller.ex index 98beded4..b246188d 100644 --- a/lib/towerops_web/controllers/user_session_controller.ex +++ b/lib/towerops_web/controllers/user_session_controller.ex @@ -60,6 +60,17 @@ defmodule ToweropsWeb.UserSessionController do UserAuth.log_in_user(conn, user, user_params) end else + # Record failed login attempt for security monitoring + _ = + Accounts.record_login_attempt(%{ + email: email, + success: false, + failure_reason: "invalid_credentials", + method: "password", + ip_address: ToweropsWeb.RemoteIp.from_conn(conn), + user_agent: extract_user_agent(conn) + }) + form = Phoenix.Component.to_form(user_params, as: "user") # In order to prevent user enumeration attacks, don't disclose whether the email is registered. @@ -181,7 +192,19 @@ defmodule ToweropsWeb.UserSessionController do ) |> complete_totp_login(user) - {:error, _reason} -> + {:error, reason} -> + # Record failed TOTP verification for security monitoring + _ = + Accounts.record_login_attempt(%{ + email: user.email, + success: false, + failure_reason: "invalid_totp", + method: "totp", + ip_address: ToweropsWeb.RemoteIp.from_conn(conn), + user_agent: extract_user_agent(conn), + metadata: %{reason: inspect(reason)} + }) + render_totp_error(conn, code) end end @@ -209,4 +232,11 @@ defmodule ToweropsWeb.UserSessionController do |> put_flash(:error, t("Invalid authentication code. Please try again.")) |> render(:totp, form: form) end + + defp extract_user_agent(conn) do + case Plug.Conn.get_req_header(conn, "user-agent") do + [ua | _] -> ua + [] -> nil + end + end end diff --git a/lib/towerops_web/controllers/user_sudo_controller.ex b/lib/towerops_web/controllers/user_sudo_controller.ex index e2f9f119..b17d98a3 100644 --- a/lib/towerops_web/controllers/user_sudo_controller.ex +++ b/lib/towerops_web/controllers/user_sudo_controller.ex @@ -75,6 +75,9 @@ defmodule ToweropsWeb.UserSudoController do end {:error, :recovery_code_not_allowed} -> + # Log failed sudo attempt (recovery code used) + _ = log_failed_sudo_attempt(conn, user, "recovery_code_not_allowed") + form = Phoenix.Component.to_form(%{"totp_code" => totp_code}, as: "user") conn @@ -85,6 +88,9 @@ defmodule ToweropsWeb.UserSudoController do |> render(:verify, form: form) {:error, :invalid_code} -> + # Log failed sudo attempt (invalid code) + _ = log_failed_sudo_attempt(conn, user, "invalid_code") + form = Phoenix.Component.to_form(%{"totp_code" => totp_code}, as: "user") conn @@ -98,4 +104,23 @@ defmodule ToweropsWeb.UserSudoController do form = Phoenix.Component.to_form(%{}, as: "user") render(conn, :verify, form: form) end + + defp log_failed_sudo_attempt(conn, user, reason) do + Accounts.record_login_attempt(%{ + email: user.email, + success: false, + failure_reason: "sudo_verification_failed", + method: "sudo_totp", + ip_address: ToweropsWeb.RemoteIp.from_conn(conn), + user_agent: extract_user_agent(conn), + metadata: %{sudo_failure_reason: reason} + }) + end + + defp extract_user_agent(conn) do + case Plug.Conn.get_req_header(conn, "user-agent") do + [ua | _] -> ua + [] -> nil + end + end end diff --git a/lib/towerops_web/live/device_live/index.ex b/lib/towerops_web/live/device_live/index.ex index 0b10a27c..f3898b5a 100644 --- a/lib/towerops_web/live/device_live/index.ex +++ b/lib/towerops_web/live/device_live/index.ex @@ -193,16 +193,21 @@ defmodule ToweropsWeb.DeviceLive.Index do end def handle_event("add_discovered_device", params, socket) do - identifier = Jason.decode!(params["identifier"]) - discovered = Enum.find(socket.assigns.discovered_devices, &(&1.identifier == identifier)) + case Jason.decode(params["identifier"]) do + {:ok, identifier} -> + discovered = Enum.find(socket.assigns.discovered_devices, &(&1.identifier == identifier)) - prefill_params = %{ - "name" => discovered.hostname || "", - "ip_address" => List.first(discovered.ip_addresses) || "", - "snmp_enabled" => "true" - } + prefill_params = %{ + "name" => discovered.hostname || "", + "ip_address" => List.first(discovered.ip_addresses) || "", + "snmp_enabled" => "true" + } - {:noreply, push_navigate(socket, to: ~p"/devices/new?#{prefill_params}")} + {:noreply, push_navigate(socket, to: ~p"/devices/new?#{prefill_params}")} + + {:error, _reason} -> + {:noreply, put_flash(socket, :error, t_equipment("Invalid device identifier"))} + end end defp perform_site_reorder(socket, site_id, new_position, organization_id) do diff --git a/lib/towerops_web/user_auth.ex b/lib/towerops_web/user_auth.ex index dd5510dd..93b1d6bf 100644 --- a/lib/towerops_web/user_auth.ex +++ b/lib/towerops_web/user_auth.ex @@ -19,6 +19,7 @@ defmodule ToweropsWeb.UserAuth do alias Phoenix.LiveView alias Towerops.Accounts alias Towerops.Accounts.Scope + alias Towerops.Admin.AuditLogger alias ToweropsWeb.Helpers.StatusHelpers # Make the remember me cookie valid for 14 days. This should match @@ -443,6 +444,15 @@ defmodule ToweropsWeb.UserAuth do |> assign(:can_view_financials, Policy.can_view_financials?(membership)) |> put_session(:current_organization_id, organization.id) else + # Log failed authorization attempt + _ = + AuditLogger.log_failed_access_attempt( + conn, + scope.user.id, + "organization:#{organization.id}", + "not_member" + ) + conn |> put_flash(:error, t_auth("You don't have access to this organization.")) |> redirect(to: ~p"/orgs")