Replace ip-api.com geolocation with Cloudflare headers
Use CF-IPLatitude/CF-IPLongitude headers instead of making HTTP calls to ip-api.com on every page load. Eliminates ~500ms latency, external API dependency, and flaky tests. Also disable test server to prevent port conflicts during development.
This commit is contained in:
parent
ba4f3ee392
commit
b5d7cf4a38
4 changed files with 86 additions and 263 deletions
|
|
@ -11,10 +11,6 @@
|
|||
# The filter_packets_by_bounds function has two clauses for map and list inputs
|
||||
~r/lib\/aprsme_web\/live\/map_live\/index\.ex.*The pattern.*can never match/,
|
||||
|
||||
# IP geolocation plug - these patterns are actually reachable but dialyzer can't prove it
|
||||
# The remote_ip can be nil or other values beyond just IPv4/IPv6 tuples
|
||||
~r/lib\/aprsme_web\/plugs\/ip_geolocation\.ex/,
|
||||
|
||||
# False positive: Aprs.parse/1 does return {:ok, _} but dialyzer can't see the vendored library types
|
||||
{"lib/aprsme/is/is.ex"},
|
||||
|
||||
|
|
|
|||
|
|
@ -35,7 +35,7 @@ config :aprsme, AprsmeWeb.Endpoint,
|
|||
adapter: Bandit.PhoenixAdapter,
|
||||
http: [ip: {127, 0, 0, 1}, port: 4002],
|
||||
secret_key_base: "IV9+ENaw9i8xjReRk4sULRvRgsmFVTGQwQGGrf4G+Q/SFMeHBCNWRlPXQ2YvT36R",
|
||||
server: true
|
||||
server: false
|
||||
|
||||
# Disable cleanup scheduler in test environment
|
||||
config :aprsme, :cleanup_scheduler, enabled: false
|
||||
|
|
|
|||
|
|
@ -1,14 +1,12 @@
|
|||
defmodule AprsmeWeb.Plugs.IPGeolocation do
|
||||
@moduledoc """
|
||||
Plug that performs IP-based geolocation on initial page load.
|
||||
Results are stored in the session to avoid repeated API calls.
|
||||
Plug that extracts geolocation from Cloudflare's CF-IPLatitude and CF-IPLongitude
|
||||
headers on initial page load. Results are stored in the session to avoid repeated parsing.
|
||||
"""
|
||||
@behaviour Plug
|
||||
|
||||
import Plug.Conn
|
||||
|
||||
require Logger
|
||||
|
||||
@impl true
|
||||
def init(opts), do: opts
|
||||
|
||||
|
|
@ -21,190 +19,18 @@ defmodule AprsmeWeb.Plugs.IPGeolocation do
|
|||
|
||||
def call(conn, _opts), do: conn
|
||||
|
||||
defp handle_geolocation(nil, conn), do: perform_geolocation(conn)
|
||||
defp handle_geolocation(nil, conn), do: extract_cf_headers(conn)
|
||||
defp handle_geolocation(_cached, conn), do: conn
|
||||
|
||||
defp perform_geolocation(conn) do
|
||||
ip = get_client_ip(conn)
|
||||
Logger.info("IP geolocation: Starting lookup for IP #{ip}")
|
||||
|
||||
if valid_ip_for_geolocation?(ip) do
|
||||
start_time = System.monotonic_time(:millisecond)
|
||||
|
||||
# Run geolocation in a task with a hard timeout
|
||||
task = Task.async(fn -> fetch_ip_location(ip) end)
|
||||
|
||||
case Task.yield(task, 600) || Task.shutdown(task, :brutal_kill) do
|
||||
{:ok, {:ok, location}} ->
|
||||
duration = System.monotonic_time(:millisecond) - start_time
|
||||
|
||||
Logger.info(
|
||||
"IP geolocation: Successfully located #{ip} at #{location["lat"]}, #{location["lng"]} in #{duration}ms"
|
||||
)
|
||||
|
||||
put_session(conn, :ip_geolocation, location)
|
||||
|
||||
{:ok, {:error, reason}} ->
|
||||
duration = System.monotonic_time(:millisecond) - start_time
|
||||
Logger.warning("IP geolocation: Failed for #{ip} after #{duration}ms - #{inspect(reason)}")
|
||||
conn
|
||||
|
||||
nil ->
|
||||
# Task timed out
|
||||
duration = System.monotonic_time(:millisecond) - start_time
|
||||
Logger.warning("IP geolocation: Task timeout for #{ip} after #{duration}ms")
|
||||
conn
|
||||
end
|
||||
defp extract_cf_headers(conn) do
|
||||
with [lat_str] <- get_req_header(conn, "cf-iplatitude"),
|
||||
[lng_str] <- get_req_header(conn, "cf-iplongitude"),
|
||||
{lat, ""} <- Float.parse(lat_str),
|
||||
{lng, ""} <- Float.parse(lng_str),
|
||||
true <- lat >= -90 and lat <= 90 and lng >= -180 and lng <= 180 do
|
||||
put_session(conn, :ip_geolocation, %{"lat" => lat, "lng" => lng})
|
||||
else
|
||||
Logger.info("IP geolocation: Skipping private/local IP #{ip}")
|
||||
conn
|
||||
_ -> conn
|
||||
end
|
||||
end
|
||||
|
||||
defp get_client_ip(conn) do
|
||||
# Check for Cloudflare header first (CF-Connecting-IP)
|
||||
cf_ip = get_req_header(conn, "cf-connecting-ip")
|
||||
|
||||
# Then check for standard forwarded headers
|
||||
forwarded_for = get_req_header(conn, "x-forwarded-for")
|
||||
real_ip = get_req_header(conn, "x-real-ip")
|
||||
|
||||
ip =
|
||||
case {cf_ip, forwarded_for, real_ip} do
|
||||
{[cf | _], _, _} ->
|
||||
# Cloudflare header takes precedence
|
||||
ip_from_cf = String.trim(cf)
|
||||
Logger.info("IP geolocation: Using Cloudflare CF-Connecting-IP: #{ip_from_cf}")
|
||||
ip_from_cf
|
||||
|
||||
{[], [forwarded | _], _} ->
|
||||
# Take the first IP from the X-Forwarded-For header
|
||||
ip_from_header =
|
||||
forwarded
|
||||
|> String.split(",")
|
||||
|> List.first()
|
||||
|> String.trim()
|
||||
|
||||
Logger.info("IP geolocation: Using forwarded IP from X-Forwarded-For header: #{ip_from_header}")
|
||||
ip_from_header
|
||||
|
||||
{[], [], [real | _]} ->
|
||||
# Use X-Real-IP header
|
||||
ip_from_real = String.trim(real)
|
||||
Logger.info("IP geolocation: Using X-Real-IP header: #{ip_from_real}")
|
||||
ip_from_real
|
||||
|
||||
{[], [], []} ->
|
||||
# Fall back to remote_ip
|
||||
ip_from_remote =
|
||||
case conn.remote_ip do
|
||||
{a, b, c, d} ->
|
||||
"#{a}.#{b}.#{c}.#{d}"
|
||||
|
||||
{a, b, c, d, e, f, g, h} ->
|
||||
# Convert IPv6 tuple to proper IPv6 format
|
||||
{a, b, c, d, e, f, g, h}
|
||||
|> :inet.ntoa()
|
||||
|> to_string()
|
||||
|
||||
_ ->
|
||||
nil
|
||||
end
|
||||
|
||||
if ip_from_remote do
|
||||
Logger.info("IP geolocation: Using remote_ip: #{ip_from_remote}")
|
||||
end
|
||||
|
||||
ip_from_remote
|
||||
end
|
||||
|
||||
ip
|
||||
end
|
||||
|
||||
defp valid_ip_for_geolocation?(ip) when is_binary(ip) do
|
||||
not private_ip?(ip)
|
||||
end
|
||||
|
||||
defp valid_ip_for_geolocation?(_), do: false
|
||||
|
||||
defp private_ip?("127." <> _), do: true
|
||||
defp private_ip?("::1" <> _), do: true
|
||||
defp private_ip?("10." <> _), do: true
|
||||
defp private_ip?("192.168." <> _), do: true
|
||||
defp private_ip?("172." <> _ = ip), do: in_172_private_range?(ip)
|
||||
defp private_ip?(_), do: false
|
||||
|
||||
# Check if IP is in 172.16.0.0/12 range (172.16.0.0 - 172.31.255.255)
|
||||
defp in_172_private_range?(ip) do
|
||||
case String.split(ip, ".") do
|
||||
["172", second_octet | _] ->
|
||||
case Integer.parse(second_octet) do
|
||||
{num, _} when num >= 16 and num <= 31 -> true
|
||||
_ -> false
|
||||
end
|
||||
|
||||
_ ->
|
||||
false
|
||||
end
|
||||
end
|
||||
|
||||
defp fetch_ip_location(ip) do
|
||||
url = "http://ip-api.com/json/#{ip}"
|
||||
Logger.info("IP geolocation: Making HTTP request to #{url}")
|
||||
|
||||
# Configure very short timeout to prevent blocking page load
|
||||
req_options = [
|
||||
# 500ms timeout - fail fast
|
||||
receive_timeout: 500,
|
||||
# No retries - we want fast failure
|
||||
retry: false,
|
||||
# Also limit connection pool wait time
|
||||
pool_timeout: 500
|
||||
]
|
||||
|
||||
case Req.get(url, req_options) do
|
||||
{:ok, %Req.Response{status: 200, body: body}} ->
|
||||
Logger.info("IP geolocation: Got 200 response with body: #{inspect(body)}")
|
||||
parse_ip_api_response(body)
|
||||
|
||||
{:ok, response} ->
|
||||
Logger.warning("IP geolocation: Got non-200 response: #{response.status}")
|
||||
{:error, :invalid_response}
|
||||
|
||||
{:error, reason} ->
|
||||
Logger.error("IP geolocation: HTTP request failed: #{inspect(reason)}")
|
||||
{:error, reason}
|
||||
end
|
||||
end
|
||||
|
||||
defp parse_ip_api_response(body) when is_map(body) do
|
||||
case body do
|
||||
%{"status" => "success", "lat" => lat, "lon" => lon}
|
||||
when is_number(lat) and is_number(lon) ->
|
||||
if lat >= -90 and lat <= 90 and lon >= -180 and lon <= 180 do
|
||||
Logger.info("IP geolocation: Parsed coordinates lat=#{lat}, lon=#{lon}")
|
||||
{:ok, %{"lat" => lat / 1.0, "lng" => lon / 1.0}}
|
||||
else
|
||||
Logger.warning("IP geolocation: Invalid coordinates lat=#{lat}, lon=#{lon}")
|
||||
{:error, :invalid_coordinates}
|
||||
end
|
||||
|
||||
%{"status" => "fail", "message" => message} ->
|
||||
Logger.warning("IP geolocation: API returned failure: #{message}")
|
||||
{:error, :api_failure}
|
||||
|
||||
data ->
|
||||
Logger.warning("IP geolocation: Unexpected response format: #{inspect(data)}")
|
||||
{:error, :api_failure}
|
||||
end
|
||||
end
|
||||
|
||||
defp parse_ip_api_response(body) when is_binary(body) do
|
||||
case Jason.decode(body) do
|
||||
{:ok, decoded} -> parse_ip_api_response(decoded)
|
||||
{:error, _} -> {:error, :json_decode_error}
|
||||
end
|
||||
end
|
||||
|
||||
defp parse_ip_api_response(_), do: {:error, :invalid_response}
|
||||
end
|
||||
|
|
|
|||
|
|
@ -3,11 +3,7 @@ defmodule AprsmeWeb.Plugs.IPGeolocationTest do
|
|||
|
||||
alias AprsmeWeb.Plugs.IPGeolocation
|
||||
|
||||
# Test IP address
|
||||
@test_ip {204, 110, 191, 254}
|
||||
|
||||
setup %{conn: conn} do
|
||||
# Initialize session for all tests
|
||||
conn =
|
||||
conn
|
||||
|> Plug.Session.call(Plug.Session.init(store: :cookie, key: "_aprs_key", signing_salt: "test"))
|
||||
|
|
@ -16,98 +12,48 @@ defmodule AprsmeWeb.Plugs.IPGeolocationTest do
|
|||
{:ok, conn: conn}
|
||||
end
|
||||
|
||||
describe "call/2" do
|
||||
@tag :external_api
|
||||
test "adds geolocation data to session when successful", %{conn: conn} do
|
||||
describe "call/2 with Cloudflare headers" do
|
||||
test "sets session from CF-IPLatitude and CF-IPLongitude headers on root GET", %{conn: conn} do
|
||||
conn =
|
||||
conn
|
||||
|> Map.put(:remote_ip, @test_ip)
|
||||
|> put_req_header("cf-iplatitude", "37.7749")
|
||||
|> put_req_header("cf-iplongitude", "-122.4194")
|
||||
|> IPGeolocation.call([])
|
||||
|
||||
geo = get_session(conn, :ip_geolocation)
|
||||
assert is_map(geo)
|
||||
assert is_number(geo["lat"])
|
||||
assert is_number(geo["lng"])
|
||||
# Test IP should geolocate to somewhere in the US
|
||||
assert geo["lat"] >= 24 and geo["lat"] <= 49
|
||||
assert geo["lng"] >= -125 and geo["lng"] <= -66
|
||||
assert geo == %{"lat" => 37.7749, "lng" => -122.4194}
|
||||
end
|
||||
|
||||
test "skips geolocation for local IP addresses", %{conn: conn} do
|
||||
test "skips when CF headers are missing", %{conn: conn} do
|
||||
conn = IPGeolocation.call(conn, [])
|
||||
|
||||
assert get_session(conn, :ip_geolocation) == nil
|
||||
end
|
||||
|
||||
test "skips when only latitude header is present", %{conn: conn} do
|
||||
conn =
|
||||
conn
|
||||
|> Map.put(:remote_ip, {127, 0, 0, 1})
|
||||
|> put_req_header("cf-iplatitude", "37.7749")
|
||||
|> IPGeolocation.call([])
|
||||
|
||||
assert get_session(conn, :ip_geolocation) == nil
|
||||
end
|
||||
|
||||
test "skips geolocation for private IP addresses", %{conn: _conn} do
|
||||
# Test various private IP ranges
|
||||
private_ips = [
|
||||
{10, 0, 0, 1},
|
||||
{172, 16, 0, 1},
|
||||
{192, 168, 1, 1}
|
||||
]
|
||||
|
||||
for ip <- private_ips do
|
||||
conn =
|
||||
build_conn()
|
||||
|> Plug.Session.call(Plug.Session.init(store: :cookie, key: "_aprs_key", signing_salt: "test"))
|
||||
|> fetch_session()
|
||||
|> Map.put(:remote_ip, ip)
|
||||
|> IPGeolocation.call([])
|
||||
|
||||
assert get_session(conn, :ip_geolocation) == nil
|
||||
end
|
||||
end
|
||||
|
||||
@tag :external_api
|
||||
test "handles IPv6 addresses", %{conn: conn} do
|
||||
test "skips when only longitude header is present", %{conn: conn} do
|
||||
conn =
|
||||
conn
|
||||
|> Map.put(:remote_ip, {0x2001, 0x4860, 0x4860, 0, 0, 0, 0, 0x8888})
|
||||
|> put_req_header("cf-iplongitude", "-122.4194")
|
||||
|> IPGeolocation.call([])
|
||||
|
||||
geo = get_session(conn, :ip_geolocation)
|
||||
assert is_map(geo)
|
||||
assert is_number(geo["lat"])
|
||||
assert is_number(geo["lng"])
|
||||
end
|
||||
|
||||
@tag :external_api
|
||||
test "uses forwarded IP when behind proxy", %{conn: conn} do
|
||||
conn =
|
||||
conn
|
||||
|> Map.put(:remote_ip, {10, 0, 0, 1})
|
||||
|> put_req_header("x-forwarded-for", "204.110.191.254, 10.0.0.1")
|
||||
|> IPGeolocation.call([])
|
||||
|
||||
geo = get_session(conn, :ip_geolocation)
|
||||
assert is_map(geo)
|
||||
assert is_number(geo["lat"])
|
||||
assert is_number(geo["lng"])
|
||||
end
|
||||
|
||||
test "caches geolocation in session", %{conn: conn} do
|
||||
# Mock the geolocation data by setting it in session first
|
||||
geo_data = %{"lat" => 37.4056, "lng" => -122.0775}
|
||||
|
||||
conn =
|
||||
conn
|
||||
|> put_session(:ip_geolocation, geo_data)
|
||||
|> Map.put(:remote_ip, @test_ip)
|
||||
|> IPGeolocation.call([])
|
||||
|
||||
# Should return the cached data without making an API call
|
||||
assert get_session(conn, :ip_geolocation) == geo_data
|
||||
assert get_session(conn, :ip_geolocation) == nil
|
||||
end
|
||||
|
||||
test "skips non-root paths", %{conn: conn} do
|
||||
conn =
|
||||
conn
|
||||
|> Map.put(:remote_ip, @test_ip)
|
||||
|> Map.put(:request_path, "/about")
|
||||
|> put_req_header("cf-iplatitude", "37.7749")
|
||||
|> put_req_header("cf-iplongitude", "-122.4194")
|
||||
|> IPGeolocation.call([])
|
||||
|
||||
assert get_session(conn, :ip_geolocation) == nil
|
||||
|
|
@ -116,11 +62,66 @@ defmodule AprsmeWeb.Plugs.IPGeolocationTest do
|
|||
test "skips non-GET requests", %{conn: conn} do
|
||||
conn =
|
||||
conn
|
||||
|> Map.put(:remote_ip, @test_ip)
|
||||
|> Map.put(:method, "POST")
|
||||
|> put_req_header("cf-iplatitude", "37.7749")
|
||||
|> put_req_header("cf-iplongitude", "-122.4194")
|
||||
|> IPGeolocation.call([])
|
||||
|
||||
assert get_session(conn, :ip_geolocation) == nil
|
||||
end
|
||||
|
||||
test "uses cached session data without re-reading headers", %{conn: conn} do
|
||||
geo_data = %{"lat" => 40.7128, "lng" => -74.006}
|
||||
|
||||
conn =
|
||||
conn
|
||||
|> put_session(:ip_geolocation, geo_data)
|
||||
|> put_req_header("cf-iplatitude", "37.7749")
|
||||
|> put_req_header("cf-iplongitude", "-122.4194")
|
||||
|> IPGeolocation.call([])
|
||||
|
||||
assert get_session(conn, :ip_geolocation) == geo_data
|
||||
end
|
||||
|
||||
test "rejects latitude out of range", %{conn: conn} do
|
||||
conn =
|
||||
conn
|
||||
|> put_req_header("cf-iplatitude", "91.0")
|
||||
|> put_req_header("cf-iplongitude", "-122.4194")
|
||||
|> IPGeolocation.call([])
|
||||
|
||||
assert get_session(conn, :ip_geolocation) == nil
|
||||
end
|
||||
|
||||
test "rejects longitude out of range", %{conn: conn} do
|
||||
conn =
|
||||
conn
|
||||
|> put_req_header("cf-iplatitude", "37.7749")
|
||||
|> put_req_header("cf-iplongitude", "181.0")
|
||||
|> IPGeolocation.call([])
|
||||
|
||||
assert get_session(conn, :ip_geolocation) == nil
|
||||
end
|
||||
|
||||
test "rejects non-numeric header values", %{conn: conn} do
|
||||
conn =
|
||||
conn
|
||||
|> put_req_header("cf-iplatitude", "not_a_number")
|
||||
|> put_req_header("cf-iplongitude", "-122.4194")
|
||||
|> IPGeolocation.call([])
|
||||
|
||||
assert get_session(conn, :ip_geolocation) == nil
|
||||
end
|
||||
|
||||
test "handles boundary coordinate values", %{conn: conn} do
|
||||
conn =
|
||||
conn
|
||||
|> put_req_header("cf-iplatitude", "-90.0")
|
||||
|> put_req_header("cf-iplongitude", "180.0")
|
||||
|> IPGeolocation.call([])
|
||||
|
||||
geo = get_session(conn, :ip_geolocation)
|
||||
assert geo == %{"lat" => -90.0, "lng" => 180.0}
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue