towerops/docs/SECURITY_AUDIT_2026_03_14.md

Security Audit Report — TowerOps Web Application

Date: 2026-03-14
Scope: Authentication, authorization, and session security
Auditor: Automated code review (Skippy)

---

Summary

The TowerOps application demonstrates a well-structured security posture overall. Authentication uses Argon2 password hashing, TOTP-based MFA, session token management with expiry, and CSRF protection. Organization-scoped data access is consistently enforced through ScopedResource and AccessControl helpers.

The findings below are verified issues found in the source code.

---

Findings

1. API Device Create Allows organization_id Override

Severity: HIGH
File: lib/towerops_web/controllers/api/v1/devices_controller.ex, lines 245–249
Issue: The default_organization_id/2 function only sets organization_id when the param is nil or empty. If a client provides a different organization_id in the request body, it is passed through to Devices.create_device/2 unchanged. This allows an API token holder for Org A to create devices in Org B.

defp default_organization_id(device_params, organization_id) do
  case Map.get(device_params, "organization_id") do
    nil -> Map.put(device_params, "organization_id", organization_id)
    "" -> Map.put(device_params, "organization_id", organization_id)
    _provided_id -> device_params  # ← attacker-controlled org_id accepted
  end
end

Suggested Fix: Always force the authenticated organization_id:

defp default_organization_id(device_params, organization_id) do
  Map.put(device_params, "organization_id", organization_id)
end

---

2. Brute Force Protection Disabled in Production

Severity: HIGH
File: lib/towerops_web/plugs/brute_force_protection.ex, line 38
Issue: The plug's call/2 function unconditionally returns conn without any processing. The comment says "TEMPORARILY DISABLED." While Cloudflare provides edge-level protection, if Cloudflare is bypassed or misconfigured, the application has no server-side brute force protection beyond rate limiting.

def call(conn, _opts) do
  # TEMPORARILY DISABLED - brute force protection bypassed
  conn
end

Suggested Fix: Re-enable the brute force protection or remove the plug entirely to avoid a false sense of security. At minimum, document the Cloudflare dependency as a security requirement.

---

3. Admin API MIB/GeoIP Routes Use Application-Level Superuser Check, Not Pipeline

Severity: MEDIUM
File: lib/towerops_web/router.ex, lines 168–176
Files affected: lib/towerops_web/controllers/api/v1/mib_controller.ex, lib/towerops_web/controllers/api/v1/geoip_controller.ex
Issue: The admin API scope (/admin/api) uses only the :api_v1 pipeline (Bearer token auth) without the :require_superuser pipeline plug. Superuser checks are done inside each controller action via require_superuser/1. This is defense-in-depth weakness — if a developer adds a new action and forgets the check, it would be accessible to any API token holder.

# Router - no superuser pipeline plug
scope "/admin/api", V1 do
  pipe_through :api_v1
  # ...
end

The controllers do implement their own require_superuser check, which works, but the halt() return from within with :ok <- require_superuser(conn) is not propagated correctly because halt() is called inside the private function but with still proceeds. Let me verify...

Actually, reviewing more carefully: the require_superuser function calls halt() on the conn and returns the halted conn. The with :ok <- pattern means if it doesn't return :ok, the with block falls through. But the require_superuser/1 function calls json + halt and returns the conn, not :ok. So the with clause correctly handles this — if require_superuser returns the halted conn instead of :ok, the with doesn't match and the function returns without executing the body. However, the halted conn is not actually returned to the caller — it's discarded. The controller action returns normally without the halted conn.

Wait — re-reading the code: the with :ok <- require_superuser(conn) pattern means if require_superuser returns anything other than :ok, the with block uses its else clause or falls through. Since there's no explicit else, the non-matching return value (the halted conn) is returned as the function's return value. Phoenix will see that the conn is halted and stop processing. This is correct.

Revised assessment: The controller-level checks work correctly. The concern remains that this is a defense-in-depth gap — a pipeline-level check would be more robust.

Suggested Fix: Add a require_superuser_api plug to the admin API pipeline:

scope "/admin/api", V1 do
  pipe_through [:api_v1, :require_superuser_api]
end

---

4. Honeybadger API Key Hardcoded in Source

Severity: MEDIUM
File: config/config.exs, line 29
Issue: The Honeybadger API key (hbp_xe5xMnpLZ2XJsXQJujoEkgCmiqCfwa0uYA3Y) is hardcoded in version-controlled source. While Honeybadger is an error-tracking service (not end-user-facing), a leaked API key could allow attackers to send fake error reports, pollute error data, or potentially access error data containing sensitive information (stack traces, request params).

config :honeybadger,
  api_key: "hbp_xe5xMnpLZ2XJsXQJujoEkgCmiqCfwa0uYA3Y",

Suggested Fix: Move to an environment variable:

config :honeybadger,
  api_key: System.get_env("HONEYBADGER_API_KEY"),

---

5. Org Settings LiveView Missing Role-Based Authorization on Sensitive Operations

Severity: MEDIUM
File: lib/towerops_web/live/org/settings_live.ex, lines 103–225
Issue: The handle_event callbacks for "save" (update org settings), "send_invitation", "remove_member", "change_role", "apply_snmp_to_all", and "apply_agent_to_all" do not check the current user's role/permissions. Any authenticated user who is a member of the organization can access the settings page (it's in the require_authenticated_user_and_organization live_session) and perform these operations, regardless of whether they are an owner, admin, or technician.

The Permissions module exists but is not imported or used in this LiveView. Only the backup delete event in DeviceLive.Show uses owner?().

Suggested Fix: Add role checks to sensitive operations:

def handle_event("save", %{"organization" => org_params}, socket) do
  import ToweropsWeb.Permissions
  unless admin?(socket), do: {:noreply, put_flash(socket, :error, "Permission denied")}
  # ... existing logic
end

def handle_event("send_invitation", params, socket) do
  import ToweropsWeb.Permissions
  unless admin?(socket), do: {:noreply, put_flash(socket, :error, "Permission denied")}
  # ... existing logic
end

---

6. Session Cookie Missing secure: true Flag

Severity: MEDIUM
File: lib/towerops_web/endpoint.ex, lines 9–14
Issue: The session cookie options don't set secure: true. While the comment in prod.exs says "SSL/TLS is handled by Cloudflared proxy," the cookie secure flag is independent of TLS termination — it tells the browser to only send the cookie over HTTPS connections. Without it, if a user somehow accesses the app over HTTP (e.g., direct IP access bypassing Cloudflare), the session cookie would be transmitted in cleartext.

@session_options [
  store: :cookie,
  key: "_towerops_key",
  signing_salt: "hrDZxLhd",
  encryption_salt: "vK3p8mNx",
  same_site: "Lax"
  # missing: secure: true
]

Suggested Fix: Add secure: true for production:

@session_options [
  store: :cookie,
  key: "_towerops_key",
  signing_salt: "hrDZxLhd",
  encryption_salt: "vK3p8mNx",
  same_site: "Lax",
  secure: Application.compile_env(:towerops, :env) == :prod
]

Or configure via config/prod.exs.

---

7. Remember-Me Cookie Missing http_only: true Flag

Severity: LOW
File: lib/towerops_web/user_auth.ex, lines 19–23
Issue: The @remember_me_options for the remember-me cookie don't explicitly set http_only: true. While Phoenix's put_resp_cookie defaults to http_only: true, and secure defaults to the conn's scheme, it's best practice to be explicit. The secure flag is also not set, same concern as finding #6.

@remember_me_options [
  sign: true,
  max_age: @max_cookie_age_in_days * 24 * 60 * 60,
  same_site: "Lax"
  # missing explicit: http_only: true, secure: true
]

Suggested Fix: Be explicit:

@remember_me_options [
  sign: true,
  max_age: @max_cookie_age_in_days * 24 * 60 * 60,
  same_site: "Lax",
  http_only: true,
  secure: true
]

---

8. No force_ssl Configured for Production

Severity: LOW
File: config/prod.exs, line 41
Issue: The comment acknowledges that SSL is handled by Cloudflared proxy, so force_ssl is not configured. However, without force_ssl, the application won't set HSTS headers. HSTS ensures browsers always use HTTPS even if a user types http://. This is a defense-in-depth measure.

Suggested Fix: Enable force_ssl with HSTS even behind Cloudflare:

config :towerops, ToweropsWeb.Endpoint,
  force_ssl: [hsts: true, rewrite_on: [:x_forwarded_proto]]

The rewrite_on: [:x_forwarded_proto] ensures it works correctly behind a proxy.

---

9. Alert Show Leaks Existence of Alerts Across Organizations via Error Type

Severity: LOW
File: lib/towerops_web/controllers/api/v1/alerts_controller.ex, lines 32–44
Issue: The show action uses Alerts.get_alert!(id) which raises Ecto.NoResultsError for non-existent alerts, but returns a 404 with "Alert not found" for alerts that exist in other organizations. An attacker can distinguish between "alert doesn't exist" and "alert exists but belongs to another org" by observing whether the error is caught by the rescue clause or the if-else branch. In practice, the response is the same 404 message, but the code path differs (potential timing difference).

This is a very minor information leak. The same pattern exists in acknowledge and resolve.

Suggested Fix: Use a non-raising get_alert/1 function that returns nil:

def show(conn, %{"id" => id}) do
  organization_id = conn.assigns.current_organization_id

  case Alerts.get_alert(id) do
    nil ->
      conn |> put_status(:not_found) |> json(%{error: "Alert not found"})
    alert ->
      if alert.device && alert.device.organization_id == organization_id do
        json(conn, %{data: format_alert(alert)})
      else
        conn |> put_status(:not_found) |> json(%{error: "Alert not found"})
      end
  end
end

---

Positive Observations

These are not findings — they are security strengths worth noting:

1. Password hashing: Uses Argon2 with timing-safe comparison and no_user_verify() to prevent user enumeration.
2. TOTP implementation: Properly separates TOTP from recovery codes for sudo mode. Recovery codes cannot bypass sudo verification.
3. Session management: Tokens expire after 14 days, sessions are reissued after 7 days, and there's a 30-minute inactivity timeout.
4. CSRF protection: protect_from_forgery plug in browser pipeline, session renewal on login to prevent fixation.
5. Organization scoping: ScopedResource pattern consistently enforces org-level isolation in API controllers.
6. LiveView auth: All live_sessions use on_mount hooks for authentication/authorization checks.
7. Impersonation audit trail: Impersonation start/stop creates audit log entries with IP addresses.
8. API token hashing: Tokens are SHA-256 hashed before storage; raw tokens shown only once.
9. Rate limiting: Auth endpoints limited to 10 req/min, API to 1000 req/min per IP.
10. GraphQL hardening: Introspection disabled in production, complexity and depth limits enforced.
11. Webhook signature verification: Uses Plug.Crypto.secure_compare for timing-safe comparison.
12. Content Security Policy: Properly configured with frame-ancestors 'none' to prevent clickjacking.
13. MIB upload path traversal protection: Vendor names and filenames validated; archive contents checked for path traversal.
14. Mobile API authorization: Each endpoint verifies the user has access to the requested organization.

---

Risk Summary

Severity	Count	Key Issues
CRITICAL	0	—
HIGH	2	API org_id override, brute force protection disabled
MEDIUM	3	Admin API pipeline gap, hardcoded API key, missing role checks in org settings
LOW	3	Cookie flags, no HSTS, minor info leak

Overall Assessment: No critical vulnerabilities found. The two HIGH findings should be addressed promptly — the API organization_id override is the most actionable. The brute force protection gap is mitigated by Cloudflare but represents a defense-in-depth weakness.