From 90b6e47772d95e8e888c93e07ddebbb42ad8b227 Mon Sep 17 00:00:00 2001 From: GRACENOBLE Date: Tue, 23 Jun 2026 07:58:15 +0300 Subject: [PATCH 1/2] feat(geo): add IP geolocation via ipapi.co MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integrates ipapi.co to resolve requester IPs to geographic metadata. The client is always initialised (free tier works without a key); Redis caching (24 h TTL) is used when REDIS_URL is set. - domain/geolocation.go: GeoLocation entity (IP, CountryCode, CountryName, Region, City, Timezone, Currency, IsEU) - usecase/geolocation.go: GeoLocator interface - infrastructure/ipgeo/ipapi_client.go: full rewrite — New/NewWithBaseURL constructors, private-IP short-circuit, cache-aside lookup, optional API key (?key= param), User-Agent: template-backend/1.0 - transport/middleware/geo.go: GeoFromRequest best-effort middleware + exported RealIP helper (XFF → X-Real-IP → RemoteAddr precedence) - Bootstrap: GeoLocator always wired in Run(); IPAPIKey added to Config - Handler: geoLocator field (10th param); applied to /api/v1 group - .env.example: IPAPI_KEY= (optional, free tier comment) - Docs: geo.md created; environment.md, bootstrap.md, middleware.md updated Closes #23 --- backend/.env.example | 5 +- backend/docs/_index.md | 1 + backend/docs/bootstrap.md | 7 +- backend/docs/environment.md | 1 + backend/docs/geo.md | 179 +++++++++++++++++ backend/docs/middleware.md | 61 +++++- backend/internal/bootstrap/bootstrap.go | 10 + backend/internal/domain/geolocation.go | 13 ++ .../infrastructure/ipgeo/ipapi_client.go | 135 ++++++++++--- .../infrastructure/ipgeo/ipapi_client_test.go | 180 ++++++++++++++---- backend/internal/server/server.go | 2 +- .../internal/transport/handlers/handler.go | 3 + .../transport/handlers/health_handler_test.go | 4 +- backend/internal/transport/handlers/routes.go | 3 + backend/internal/transport/middleware/geo.go | 45 +++++ .../internal/transport/middleware/geo_test.go | 162 ++++++++++++++++ backend/internal/usecase/geolocation.go | 12 ++ 17 files changed, 753 insertions(+), 70 deletions(-) create mode 100644 backend/docs/geo.md create mode 100644 backend/internal/domain/geolocation.go create mode 100644 backend/internal/transport/middleware/geo.go create mode 100644 backend/internal/transport/middleware/geo_test.go create mode 100644 backend/internal/usecase/geolocation.go diff --git a/backend/.env.example b/backend/.env.example index d8c9b96..94f16e5 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -39,4 +39,7 @@ R2_SECRET_KEY=your_r2_secret_key_here # The name of your R2 bucket R2_BUCKET=your_bucket_name_here # Public URL for the bucket (set up a custom domain or use r2.dev subdomain) -R2_PUBLIC_URL=https://pub-xxxxxxxxxxxx.r2.dev \ No newline at end of file +R2_PUBLIC_URL=https://pub-xxxxxxxxxxxx.r2.dev +# IP geolocation via ipapi.co (optional; free tier works without a key) +# Get a key from: https://ipapi.co/api/#introduction +IPAPI_KEY= \ No newline at end of file diff --git a/backend/docs/_index.md b/backend/docs/_index.md index 276fe0f..0862d73 100644 --- a/backend/docs/_index.md +++ b/backend/docs/_index.md @@ -21,3 +21,4 @@ The `docs` agent reads this index first to locate the right file before diving i | Firebase Cloud Messaging — token storage, send API, FCM endpoints | [fcm.md](fcm.md) | `internal/domain/fcm_token.go`, `internal/usecase/notification.go`, `internal/infrastructure/database/postgres/fcm_token_repository.go`, `internal/transport/handlers/fcm_handler.go`, `pkg/firebase/app.go`, `pkg/firebase/messaging.go` | | Transactional email (Mailjet) — EmailSender interface, MailjetSender, sandbox mode, templates | [email.md](email.md) | `internal/usecase/email.go`, `internal/infrastructure/email/mailjet.go`, `internal/infrastructure/email/templates/welcome.html`, `internal/bootstrap/bootstrap.go`, `internal/server/server.go`, `internal/transport/handlers/handler.go` | | Object storage (Cloudflare R2) — StorageService interface, R2 implementation, presign/delete endpoints | [storage.md](storage.md) | `internal/usecase/storage.go`, `internal/infrastructure/storage/r2/storage.go`, `internal/transport/handlers/storage_handler.go`, `internal/transport/handlers/routes.go`, `internal/bootstrap/bootstrap.go` | +| IP geolocation (ipapi.co) — GeoLocation entity, GeoLocator interface, ipgeo.Client, GeoFromRequest middleware, RealIP helper | [geo.md](geo.md) | `internal/domain/geolocation.go`, `internal/usecase/geolocation.go`, `internal/infrastructure/ipgeo/ipapi_client.go`, `internal/transport/middleware/geo.go`, `internal/bootstrap/bootstrap.go` | diff --git a/backend/docs/bootstrap.md b/backend/docs/bootstrap.md index 83972cd..b9faa71 100644 --- a/backend/docs/bootstrap.md +++ b/backend/docs/bootstrap.md @@ -22,11 +22,12 @@ type App struct { FCMSender usecase.NotificationSender // nil when FIREBASE_PROJECT_ID is not set EmailSender usecase.EmailSender // nil when MAILJET_API_KEY is not set StorageService usecase.StorageService // nil when R2_ACCOUNT_ID is not set + GeoLocator usecase.GeoLocator // always initialised; free tier works without a key Config Config Log *slog.Logger } ``` -`App` is constructed once by `Run` and passed to `server.NewServer`. Nothing re-initialises dependencies after this point. Optional fields (`Cache`, `Enqueuer`, `Firebase`, `FCMSender`, `EmailSender`, `StorageService`) are nil when their corresponding env vars are absent. +`App` is constructed once by `Run` and passed to `server.NewServer`. Nothing re-initialises dependencies after this point. Optional fields (`Cache`, `Enqueuer`, `Firebase`, `FCMSender`, `EmailSender`, `StorageService`) are nil when their corresponding env vars are absent. `GeoLocator` is always non-nil after a successful `Run()`. ## Config struct ```go @@ -49,6 +50,7 @@ type Config struct { R2SecretKey string R2Bucket string R2PublicURL string + IPAPIKey string } ``` `loadConfig()` reads all values from environment variables. `PORT` defaults to `8080`; `BLUEPRINT_DB_SCHEMA` defaults to `public`; `BLUEPRINT_DB_SSLMODE` defaults to `disable`. `RateLimitBurst` is derived as `int(RPS)*5` when omitted and RPS is set. Optional fields (`RedisURL`, `FirebaseProjectID`, `FirebaseServiceAccountJSON`) default to empty string — their respective services are skipped when empty. @@ -65,7 +67,8 @@ type Config struct { 7. Init Firebase app via `firebase.NewApp(ctx, ...)`, then init Auth client (`firebase.NewAuthClient`) and FCM messaging client (`firebase.NewMessagingClient`) from the same app instance — all skipped when `FIREBASE_PROJECT_ID` is empty 8. Init Mailjet email sender via `email.NewMailjetSender(...)` — skipped when `MAILJET_API_KEY` or `MAILJET_SECRET_KEY` is empty; startup fails if only a partial Mailjet config is provided 9. Init R2 storage client via `r2.New(...)` — skipped when `R2_ACCOUNT_ID` is empty; startup fails if `R2_ACCOUNT_ID` is set but any other R2 var is missing -10. Return `*App` on success; return a non-nil error on any failure +10. Init `ipgeo.Client` via `ipgeo.New(cache, cfg.IPAPIKey)` — always runs, never conditional; `cache` is nil when Redis is unavailable, which disables caching for geolocation +11. Return `*App` on success; return a non-nil error on any failure ```go ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) diff --git a/backend/docs/environment.md b/backend/docs/environment.md index 11dc4f2..0d79c34 100644 --- a/backend/docs/environment.md +++ b/backend/docs/environment.md @@ -46,6 +46,7 @@ This runs on package init before any env var is read — no explicit `godotenv.L | `R2_SECRET_KEY` | `bootstrap.go` | — | R2 API token secret key. Required when `R2_ACCOUNT_ID` is set; startup fails if omitted. | | `R2_BUCKET` | `bootstrap.go` | — | R2 bucket name. Required when `R2_ACCOUNT_ID` is set; startup fails if omitted. | | `R2_PUBLIC_URL` | `bootstrap.go` | — | Public base URL for the R2 bucket (custom domain or `r2.dev` subdomain). Required when `R2_ACCOUNT_ID` is set; startup fails if omitted. | +| `IPAPI_KEY` | `bootstrap.go` | — | ipapi.co API key (optional). Free tier works without a key; supplying one enables higher rate limits. | Variables marked **required** are validated by `bootstrap.validateConfig` at startup — the process exits before attempting a DB connection if any are missing. diff --git a/backend/docs/geo.md b/backend/docs/geo.md new file mode 100644 index 0000000..31a6e4f --- /dev/null +++ b/backend/docs/geo.md @@ -0,0 +1,179 @@ +--- +topic: geo +last_verified: 2026-06-23 +sources: + - backend/internal/domain/geolocation.go + - backend/internal/usecase/geolocation.go + - backend/internal/infrastructure/ipgeo/ipapi_client.go + - backend/internal/transport/middleware/geo.go + - backend/internal/bootstrap/bootstrap.go +--- + +# IP Geolocation + +## What it does +The geolocation feature resolves a client IP address to geographic metadata (country, region, city, timezone, currency, EU membership) using the [ipapi.co](https://ipapi.co) JSON API. Results are cached in Redis under the key `geo:` for 24 hours when a `CacheService` is available. + +The feature is always initialised — ipapi.co's free tier requires no API key. Supplying `IPAPI_KEY` enables the paid tier with higher rate limits. + +Private and loopback IPs (`ErrPrivateIP`) are rejected immediately without any HTTP call or cache lookup. + +## domain.GeoLocation + +```go +// backend/internal/domain/geolocation.go +type GeoLocation struct { + IP string + CountryCode string + CountryName string + Region string + City string + Timezone string + Currency string + IsEU bool +} +``` + +## usecase.GeoLocator interface + +```go +// backend/internal/usecase/geolocation.go +type GeoLocator interface { + Lookup(ctx context.Context, ip string) (*domain.GeoLocation, error) +} +``` + +## ipgeo.Client constructors + +```go +// Production — cache may be nil (disables caching), apiKey may be empty (free tier). +func New(cache usecase.CacheService, apiKey string) *Client + +// Tests — points the HTTP client at a custom base URL (e.g. an httptest.Server). +func NewWithBaseURL(cache usecase.CacheService, apiKey, baseURL string) *Client +``` + +`Client` satisfies `usecase.GeoLocator` via a compile-time assertion: +```go +var _ usecase.GeoLocator = (*Client)(nil) +``` + +### Lookup behaviour + +1. Returns `ErrPrivateIP` immediately for loopback and RFC-1918/RFC-4193 addresses. +2. Checks the cache (`geo:`) — returns the deserialised value on hit. +3. On cache miss, calls `GET {baseURL}/{ip}/json/` (appends `?key=` when a key is set). +4. On success, writes the serialised result to the cache (TTL 24 h, best-effort — write errors are silently ignored). +5. Returns the populated `*domain.GeoLocation`. + +Sentinel error: `ipgeo.ErrPrivateIP` + +## GeoFromRequest middleware + +```go +// backend/internal/transport/middleware/geo.go +const GeoLocationKey = "geo_location" + +func GeoFromRequest(locator usecase.GeoLocator) gin.HandlerFunc +``` + +Best-effort: if `locator.Lookup` returns any error (private IP, rate-limit, network error), the middleware calls `c.Next()` without storing anything. Handlers must nil-check before reading the key. + +## RealIP helper + +```go +func RealIP(r *http.Request) string +``` + +Precedence order: +1. `X-Forwarded-For` header — takes the first (leftmost) comma-separated address. +2. `X-Real-IP` header. +3. `r.RemoteAddr` — `net.SplitHostPort` strips the port; falls back to the raw string on parse error. + +`RealIP` is exported so tests and other packages can call it directly. + +## Reading geo data in a handler + +```go +val, exists := c.Get(middleware.GeoLocationKey) +if !exists { + // geo unavailable — private IP, rate-limited, or middleware not registered + return +} +geo, ok := val.(*domain.GeoLocation) +if !ok || geo == nil { + return +} +// use geo.CountryCode, geo.City, etc. +``` + +## Bootstrap wiring + +`GeoLocator` is always initialised inside `bootstrap.Run()`, unconditionally: + +```go +// bootstrap.go — always runs, not guarded by an env var check +geoLocator := ipgeo.New(cache, cfg.IPAPIKey) +log.Info("bootstrap: ipapi geolocation client initialised", "cached", cache != nil) +``` + +`cache` is the same `usecase.CacheService` used elsewhere — it is `nil` when `REDIS_URL` is not set, which disables caching for geolocation as well. + +`App.GeoLocator` is always non-nil after a successful `Run()`. + +## Environment variable + +| Variable | Required | Default | Description | +|---|---|---|---| +| `IPAPI_KEY` | No | — | ipapi.co API key. Omit or leave empty for the free tier. | + +## Testing patterns + +### Unit-testing ipgeo.Client +Use `NewWithBaseURL` pointing at an `httptest.NewServer`: + +```go +srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{ + "ip": "1.2.3.4", "country_code": "US", ... + }) +})) +defer srv.Close() + +client := ipgeo.NewWithBaseURL(nil, "", srv.URL) +geo, err := client.Lookup(context.Background(), "1.2.3.4") +``` + +Inject a cache with an inline mock struct: + +```go +type mockCacheService struct { + data map[string]string +} + +func (m *mockCacheService) Get(_ context.Context, key string) (string, bool, error) { + v, ok := m.data[key] + return v, ok, nil +} +func (m *mockCacheService) Set(_ context.Context, key, value string, _ time.Duration) error { + m.data[key] = value + return nil +} +// implement remaining usecase.CacheService methods as no-ops +``` + +### Unit-testing GeoFromRequest middleware +Use an inline `mockGeoLocator`: + +```go +type mockGeoLocator struct { + geo *domain.GeoLocation + err error +} + +func (m *mockGeoLocator) Lookup(_ context.Context, _ string) (*domain.GeoLocation, error) { + return m.geo, m.err +} +``` + +Record the handler under test with `httptest.NewRecorder` and assert `c.Get(middleware.GeoLocationKey)`. diff --git a/backend/docs/middleware.md b/backend/docs/middleware.md index 22320d7..f268ee2 100644 --- a/backend/docs/middleware.md +++ b/backend/docs/middleware.md @@ -1,10 +1,13 @@ --- topic: middleware -last_verified: 2026-06-15 +last_verified: 2026-06-23 sources: - internal/transport/middleware/logger.go - internal/transport/middleware/ratelimit.go - internal/transport/middleware/auth.go + - internal/transport/middleware/metrics.go + - internal/transport/middleware/local_network.go + - internal/transport/middleware/geo.go - internal/transport/handlers/routes.go --- @@ -77,6 +80,62 @@ token, ok := val.(*usecase.FirebaseToken) Pass `nil` as the `verifier` to `NewHandler` to skip Firebase auth entirely (development without credentials). `RegisterRoutes` reads `h.verifier` from the struct — it is not a parameter of `RegisterRoutes`. +## PrometheusMiddleware + +`PrometheusMiddleware() gin.HandlerFunc` records two metrics for every request except `/metrics` itself: + +- `http_requests_total` — counter vector with labels `method`, `path`, `status`. +- `http_request_duration_seconds` — histogram vector with labels `method`, `path`. + +Unmatched routes (404s with no Gin `FullPath()`) are recorded under the path label `"unmatched"`. + +## LocalNetworkOnly + +`LocalNetworkOnly() gin.HandlerFunc` aborts with `403 Forbidden` when the client IP is neither a loopback address nor an RFC 1918 private address. In release mode, `RegisterRoutes` applies it as a per-route middleware on `/metrics` so the Prometheus scrape endpoint is reachable from the internal network but not from external clients. + +```go +// release mode only: +r.GET("/metrics", middleware.LocalNetworkOnly(), gin.WrapH(promhttp.Handler())) +``` + +## GeoFromRequest + +`GeoFromRequest(locator usecase.GeoLocator) gin.HandlerFunc` resolves the request's originating IP to geographic metadata and stores the result in the Gin context. It is best-effort: any error from `locator.Lookup` (private IP, rate-limit, network failure) is silently dropped and the request continues without geo data. + +```go +const GeoLocationKey = "geo_location" + +func GeoFromRequest(locator usecase.GeoLocator) gin.HandlerFunc +``` + +Context key: `middleware.GeoLocationKey` (`"geo_location"`). Value type: `*domain.GeoLocation`. + +Reading geo data in a handler: +```go +val, exists := c.Get(middleware.GeoLocationKey) +if !exists { + // geo unavailable + return +} +geo, ok := val.(*domain.GeoLocation) +if !ok || geo == nil { + return +} +``` + +### RealIP helper + +```go +func RealIP(r *http.Request) string +``` + +IP extraction precedence: +1. First address in `X-Forwarded-For` (proxy/Railway deploys). +2. `X-Real-IP` header. +3. `r.RemoteAddr` with port stripped via `net.SplitHostPort`. + +`RealIP` is exported for direct use in tests and other packages. + ## Adding new middleware 1. Create `internal/transport/middleware/.go` with a function returning `gin.HandlerFunc`. diff --git a/backend/internal/bootstrap/bootstrap.go b/backend/internal/bootstrap/bootstrap.go index 91419bf..35c603d 100644 --- a/backend/internal/bootstrap/bootstrap.go +++ b/backend/internal/bootstrap/bootstrap.go @@ -18,6 +18,7 @@ import ( "backend/internal/infrastructure/cache/redis" "backend/internal/infrastructure/database/postgres" "backend/internal/infrastructure/email" + "backend/internal/infrastructure/ipgeo" "backend/internal/infrastructure/queue" "backend/internal/infrastructure/storage/r2" "backend/internal/usecase" @@ -43,6 +44,7 @@ type App struct { FCMSender usecase.NotificationSender // nil when FIREBASE_PROJECT_ID is not set EmailSender usecase.EmailSender // nil when MAILJET_API_KEY is not set StorageService usecase.StorageService // nil when R2_ACCOUNT_ID is not set + GeoLocator usecase.GeoLocator // always initialised; free tier works without a key Config Config Log *slog.Logger } @@ -67,6 +69,7 @@ type Config struct { R2SecretKey string R2Bucket string R2PublicURL string + IPAPIKey string } // ConfigError is returned when required configuration is absent or invalid. @@ -172,6 +175,11 @@ func Run(ctx context.Context) (*App, error) { log.Info("bootstrap: R2 storage client initialised", "bucket", cfg.R2Bucket) } + // Geolocation is always available — ipapi.co has a free tier. + // Cache is used when Redis is available; nil cache means no caching. + geoLocator := ipgeo.New(cache, cfg.IPAPIKey) + log.Info("bootstrap: ipapi geolocation client initialised", "cached", cache != nil) + log.Info("bootstrap: all checks passed — ready to serve") return &App{ @@ -182,6 +190,7 @@ func Run(ctx context.Context) (*App, error) { FCMSender: fcmSender, EmailSender: emailSender, StorageService: storageService, + GeoLocator: geoLocator, Config: cfg, Log: log, }, nil @@ -227,6 +236,7 @@ func loadConfig() Config { R2SecretKey: os.Getenv("R2_SECRET_KEY"), R2Bucket: os.Getenv("R2_BUCKET"), R2PublicURL: os.Getenv("R2_PUBLIC_URL"), + IPAPIKey: os.Getenv("IPAPI_KEY"), DB: postgres.DBConfig{ Host: os.Getenv("BLUEPRINT_DB_HOST"), Port: os.Getenv("BLUEPRINT_DB_PORT"), diff --git a/backend/internal/domain/geolocation.go b/backend/internal/domain/geolocation.go new file mode 100644 index 0000000..8ddca24 --- /dev/null +++ b/backend/internal/domain/geolocation.go @@ -0,0 +1,13 @@ +package domain + +// GeoLocation holds IP-derived geographic metadata. +type GeoLocation struct { + IP string + CountryCode string + CountryName string + Region string + City string + Timezone string + Currency string + IsEU bool +} diff --git a/backend/internal/infrastructure/ipgeo/ipapi_client.go b/backend/internal/infrastructure/ipgeo/ipapi_client.go index a3e84d5..ffde1dc 100644 --- a/backend/internal/infrastructure/ipgeo/ipapi_client.go +++ b/backend/internal/infrastructure/ipgeo/ipapi_client.go @@ -8,78 +8,151 @@ import ( "net" "net/http" "time" + + "backend/internal/domain" + "backend/internal/usecase" ) // ErrPrivateIP is returned when the caller passes a loopback or RFC-1918 // address. Callers should silently skip the location update in this case. var ErrPrivateIP = errors.New("ipgeo: private or loopback IP address") -// IPAPIClient implements domain/services.IPGeolocationService by querying -// https://ipapi.co/{ip}/json/. -type IPAPIClient struct { +const defaultBaseURL = "https://ipapi.co" + +// Client implements usecase.GeoLocator by querying https://ipapi.co/{ip}/json/. +type Client struct { httpClient *http.Client - // baseURL is overridable in tests; production code leaves it empty and the - // default "https://ipapi.co" is used. - baseURL string + cache usecase.CacheService // nil = no caching + apiKey string // empty = free tier + baseURL string // empty = defaultBaseURL +} + +// New returns a Client configured for production use. +// cache may be nil (disables caching). apiKey may be empty (free tier). +func New(cache usecase.CacheService, apiKey string) *Client { + return &Client{ + httpClient: &http.Client{Timeout: 5 * time.Second}, + cache: cache, + apiKey: apiKey, + } } -// NewIPAPIClient returns an IPAPIClient with a 5-second HTTP client timeout. -func NewIPAPIClient() *IPAPIClient { - return &IPAPIClient{httpClient: &http.Client{Timeout: 5 * time.Second}} +// NewWithBaseURL returns a Client with a custom base URL, intended for tests +// pointing at an httptest server. +func NewWithBaseURL(cache usecase.CacheService, apiKey, baseURL string) *Client { + return &Client{ + httpClient: &http.Client{Timeout: 5 * time.Second}, + cache: cache, + apiKey: apiKey, + baseURL: baseURL, + } } +// Compile-time check: Client satisfies usecase.GeoLocator. +var _ usecase.GeoLocator = (*Client)(nil) + // ipapiResponse matches the JSON shape returned by ipapi.co. type ipapiResponse struct { - Latitude float64 `json:"latitude"` - Longitude float64 `json:"longitude"` - Error bool `json:"error"` - Reason string `json:"reason"` + IP string `json:"ip"` + CountryCode string `json:"country_code"` + CountryName string `json:"country_name"` + Region string `json:"region"` + City string `json:"city"` + Timezone string `json:"timezone"` + Currency string `json:"currency"` + InEU bool `json:"in_eu"` + Error bool `json:"error"` + Reason string `json:"reason"` +} + +// cacheKey returns the Redis key for a given IP. +func cacheKey(ip string) string { + return "geo:" + ip } -// Locate resolves ip to approximate lat/lon coordinates. +// Lookup resolves ip to geographic metadata. // Returns ErrPrivateIP for loopback and RFC-1918 addresses without making -// an outbound HTTP call. -func (c *IPAPIClient) Locate(ctx context.Context, ip string) (lat, lon float64, err error) { +// any outbound HTTP call or cache lookup. +func (c *Client) Lookup(ctx context.Context, ip string) (*domain.GeoLocation, error) { if isPrivateIP(ip) { - return 0, 0, ErrPrivateIP + return nil, ErrPrivateIP + } + + // Check cache first. + if c.cache != nil { + if val, ok, err := c.cache.Get(ctx, cacheKey(ip)); err == nil && ok { + var geo domain.GeoLocation + if jsonErr := json.Unmarshal([]byte(val), &geo); jsonErr == nil { + return &geo, nil + } + } } + geo, err := c.fetch(ctx, ip) + if err != nil { + return nil, err + } + + // Populate cache. + if c.cache != nil { + if data, jsonErr := json.Marshal(geo); jsonErr == nil { + // Best-effort; ignore cache write errors. + _ = c.cache.Set(ctx, cacheKey(ip), string(data), 24*time.Hour) + } + } + + return geo, nil +} + +// fetch performs the HTTP GET against ipapi.co and returns a GeoLocation. +func (c *Client) fetch(ctx context.Context, ip string) (*domain.GeoLocation, error) { base := c.baseURL if base == "" { - base = "https://ipapi.co" + base = defaultBaseURL + } + + var rawURL string + if c.apiKey != "" { + rawURL = fmt.Sprintf("%s/%s/json/?key=%s", base, ip, c.apiKey) + } else { + rawURL = fmt.Sprintf("%s/%s/json/", base, ip) } - url := fmt.Sprintf("%s/%s/json/", base, ip) - return c.fetch(ctx, url) -} -// fetch performs the HTTP GET and parses the response. -func (c *IPAPIClient) fetch(ctx context.Context, url string) (lat, lon float64, err error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil) if err != nil { - return 0, 0, fmt.Errorf("ipgeo: build request: %w", err) + return nil, fmt.Errorf("ipgeo: build request: %w", err) } - req.Header.Set("User-Agent", "gigz-backend/1.0") + req.Header.Set("User-Agent", "template-backend/1.0") resp, err := c.httpClient.Do(req) if err != nil { - return 0, 0, fmt.Errorf("ipgeo: http request: %w", err) + return nil, fmt.Errorf("ipgeo: http request: %w", err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return 0, 0, fmt.Errorf("ipgeo: unexpected status %d", resp.StatusCode) + return nil, fmt.Errorf("ipgeo: unexpected status %d", resp.StatusCode) } var body ipapiResponse if decErr := json.NewDecoder(resp.Body).Decode(&body); decErr != nil { - return 0, 0, fmt.Errorf("ipgeo: decode response: %w", decErr) + return nil, fmt.Errorf("ipgeo: decode response: %w", decErr) } if body.Error { - return 0, 0, fmt.Errorf("ipgeo: api error: %s", body.Reason) + return nil, fmt.Errorf("ipgeo: api error: %s", body.Reason) } - return body.Latitude, body.Longitude, nil + return &domain.GeoLocation{ + IP: body.IP, + CountryCode: body.CountryCode, + CountryName: body.CountryName, + Region: body.Region, + City: body.City, + Timezone: body.Timezone, + Currency: body.Currency, + IsEU: body.InEU, + }, nil } // isPrivateIP reports whether the given IP string is a loopback or diff --git a/backend/internal/infrastructure/ipgeo/ipapi_client_test.go b/backend/internal/infrastructure/ipgeo/ipapi_client_test.go index b348998..389bc8c 100644 --- a/backend/internal/infrastructure/ipgeo/ipapi_client_test.go +++ b/backend/internal/infrastructure/ipgeo/ipapi_client_test.go @@ -6,47 +6,96 @@ import ( "errors" "net/http" "net/http/httptest" + "strings" "testing" + "time" + + "backend/internal/usecase" ) -// newTestIPAPIClient builds an IPAPIClient that points at the given test -// server base URL instead of the live ipapi.co endpoint. -func newTestIPAPIClient(baseURL string) *IPAPIClient { - return &IPAPIClient{ - httpClient: &http.Client{}, - baseURL: baseURL, - } +// mockCacheService is an in-memory implementation of usecase.CacheService for tests. +type mockCacheService struct { + store map[string]string +} + +func newMockCache() *mockCacheService { + return &mockCacheService{store: make(map[string]string)} +} + +func (m *mockCacheService) Get(_ context.Context, key string) (string, bool, error) { + v, ok := m.store[key] + return v, ok, nil +} + +func (m *mockCacheService) Set(_ context.Context, key string, value string, _ time.Duration) error { + m.store[key] = value + return nil } -func TestIPAPIClient_Locate_ValidIP(t *testing.T) { +func (m *mockCacheService) PingContext(_ context.Context) error { return nil } +func (m *mockCacheService) Delete(_ context.Context, _ string) error { + return nil +} +func (m *mockCacheService) Exists(_ context.Context, _ string) (bool, error) { return false, nil } +func (m *mockCacheService) SetNX(_ context.Context, _ string, _ string, _ time.Duration) (bool, error) { + return false, nil +} +func (m *mockCacheService) Close() error { return nil } + +// Compile-time check: mockCacheService satisfies usecase.CacheService. +var _ usecase.CacheService = (*mockCacheService)(nil) + +func TestClient_Lookup_ValidIP(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") if err := json.NewEncoder(w).Encode(ipapiResponse{ - Latitude: 0.3476, - Longitude: 32.5825, - Error: false, + IP: "8.8.8.8", + CountryCode: "US", + CountryName: "United States", + Region: "California", + City: "Mountain View", + Timezone: "America/Los_Angeles", + Currency: "USD", + InEU: false, + Error: false, }); err != nil { t.Errorf("encode response: %v", err) } })) defer srv.Close() - client := newTestIPAPIClient(srv.URL) - - lat, lon, err := client.Locate(context.Background(), "8.8.8.8") + client := NewWithBaseURL(nil, "", srv.URL) + geo, err := client.Lookup(context.Background(), "8.8.8.8") if err != nil { t.Fatalf("unexpected error: %v", err) } - if lat != 0.3476 { - t.Errorf("lat: got %v, want 0.3476", lat) + if geo.IP != "8.8.8.8" { + t.Errorf("IP: got %q, want %q", geo.IP, "8.8.8.8") + } + if geo.CountryCode != "US" { + t.Errorf("CountryCode: got %q, want %q", geo.CountryCode, "US") + } + if geo.CountryName != "United States" { + t.Errorf("CountryName: got %q, want %q", geo.CountryName, "United States") } - if lon != 32.5825 { - t.Errorf("lon: got %v, want 32.5825", lon) + if geo.Region != "California" { + t.Errorf("Region: got %q, want %q", geo.Region, "California") + } + if geo.City != "Mountain View" { + t.Errorf("City: got %q, want %q", geo.City, "Mountain View") + } + if geo.Timezone != "America/Los_Angeles" { + t.Errorf("Timezone: got %q, want %q", geo.Timezone, "America/Los_Angeles") + } + if geo.Currency != "USD" { + t.Errorf("Currency: got %q, want %q", geo.Currency, "USD") + } + if geo.IsEU { + t.Errorf("IsEU: got true, want false") } } -func TestIPAPIClient_Locate_PrivateIP_NoHTTPCall(t *testing.T) { - // This server should never be called; if it is, the test fails immediately. +func TestClient_Lookup_PrivateIP(t *testing.T) { called := false srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { called = true @@ -54,14 +103,12 @@ func TestIPAPIClient_Locate_PrivateIP_NoHTTPCall(t *testing.T) { })) defer srv.Close() - // Use a real NewIPAPIClient (not the test one) — private-IP check fires - // before any HTTP call regardless of baseURL. - client := NewIPAPIClient() + client := NewWithBaseURL(nil, "", srv.URL) privateIPs := []string{"127.0.0.1", "10.0.0.1", "192.168.1.1", "::1"} for _, ip := range privateIPs { t.Run(ip, func(t *testing.T) { - _, _, err := client.Locate(context.Background(), ip) + _, err := client.Lookup(context.Background(), ip) if !errors.Is(err, ErrPrivateIP) { t.Errorf("ip %s: got error %v, want ErrPrivateIP", ip, err) } @@ -73,7 +120,7 @@ func TestIPAPIClient_Locate_PrivateIP_NoHTTPCall(t *testing.T) { } } -func TestIPAPIClient_Locate_APIErrorInBody(t *testing.T) { +func TestClient_Lookup_APIErrorInBody(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") if err := json.NewEncoder(w).Encode(ipapiResponse{ @@ -85,24 +132,93 @@ func TestIPAPIClient_Locate_APIErrorInBody(t *testing.T) { })) defer srv.Close() - client := newTestIPAPIClient(srv.URL) - - _, _, err := client.Locate(context.Background(), "1.2.3.4") + client := NewWithBaseURL(nil, "", srv.URL) + _, err := client.Lookup(context.Background(), "1.2.3.4") if err == nil { t.Fatal("expected error when api body has error:true, got nil") } } -func TestIPAPIClient_Locate_Non200Status(t *testing.T) { +func TestClient_Lookup_Non200Status(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusTooManyRequests) })) defer srv.Close() - client := newTestIPAPIClient(srv.URL) - - _, _, err := client.Locate(context.Background(), "1.2.3.4") + client := NewWithBaseURL(nil, "", srv.URL) + _, err := client.Lookup(context.Background(), "1.2.3.4") if err == nil { t.Fatal("expected error for non-200 status, got nil") } } + +func TestClient_Lookup_CacheHit(t *testing.T) { + callCount := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(ipapiResponse{ + IP: "8.8.8.8", + CountryCode: "US", + CountryName: "United States", + Region: "California", + City: "Mountain View", + Timezone: "America/Los_Angeles", + Currency: "USD", + InEU: false, + }); err != nil { + t.Errorf("encode response: %v", err) + } + })) + defer srv.Close() + + cache := newMockCache() + client := NewWithBaseURL(cache, "", srv.URL) + + // First call — should hit HTTP. + geo1, err := client.Lookup(context.Background(), "8.8.8.8") + if err != nil { + t.Fatalf("first lookup error: %v", err) + } + if callCount != 1 { + t.Errorf("expected 1 HTTP call after first lookup, got %d", callCount) + } + + // Second call — should be served from cache, no new HTTP call. + geo2, err := client.Lookup(context.Background(), "8.8.8.8") + if err != nil { + t.Fatalf("second lookup error: %v", err) + } + if callCount != 1 { + t.Errorf("expected still 1 HTTP call after cache hit, got %d", callCount) + } + + if geo1.CountryCode != geo2.CountryCode { + t.Errorf("cache returned different CountryCode: first=%q second=%q", geo1.CountryCode, geo2.CountryCode) + } +} + +func TestClient_Lookup_APIKey(t *testing.T) { + var capturedURL string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedURL = r.URL.String() + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(ipapiResponse{ + IP: "1.2.3.4", + CountryCode: "DE", + }); err != nil { + t.Errorf("encode response: %v", err) + } + })) + defer srv.Close() + + client := NewWithBaseURL(nil, "my-secret-key", srv.URL) + _, err := client.Lookup(context.Background(), "1.2.3.4") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !strings.Contains(capturedURL, "key=my-secret-key") { + t.Errorf("expected ?key=my-secret-key in URL, got %q", capturedURL) + } +} diff --git a/backend/internal/server/server.go b/backend/internal/server/server.go index be403fd..c20b282 100644 --- a/backend/internal/server/server.go +++ b/backend/internal/server/server.go @@ -56,7 +56,7 @@ func NewServer(app *bootstrap.App, hub *ws.Hub) (*http.Server, error) { } } - h := handlers.NewHandler(healthUC, app.Firebase, hub, app.Enqueuer, queueUI, app.FCMSender, fcmTokenRepo, app.EmailSender, app.StorageService) + h := handlers.NewHandler(healthUC, app.Firebase, hub, app.Enqueuer, queueUI, app.FCMSender, fcmTokenRepo, app.EmailSender, app.StorageService, app.GeoLocator) // Register DB pool metrics collector. // AlreadyRegisteredError is silenced — only the first registration wins diff --git a/backend/internal/transport/handlers/handler.go b/backend/internal/transport/handlers/handler.go index a00fde5..7bde88a 100644 --- a/backend/internal/transport/handlers/handler.go +++ b/backend/internal/transport/handlers/handler.go @@ -18,6 +18,7 @@ type Handler struct { fcmTokenRepo usecase.FCMTokenRepository // nil when Firebase is not configured emailSender usecase.EmailSender // nil when MAILJET_API_KEY is not set storageService usecase.StorageService // nil when R2_ACCOUNT_ID is not set + geoLocator usecase.GeoLocator // nil when geo client is not configured } // NewHandler constructs a Handler with all required use cases. @@ -31,6 +32,7 @@ func NewHandler( fcmTokenRepo usecase.FCMTokenRepository, emailSender usecase.EmailSender, storageService usecase.StorageService, + geoLocator usecase.GeoLocator, ) *Handler { return &Handler{ healthUC: healthUC, @@ -42,5 +44,6 @@ func NewHandler( fcmTokenRepo: fcmTokenRepo, emailSender: emailSender, storageService: storageService, + geoLocator: geoLocator, } } diff --git a/backend/internal/transport/handlers/health_handler_test.go b/backend/internal/transport/handlers/health_handler_test.go index cc90ec7..ce9a6fa 100644 --- a/backend/internal/transport/handlers/health_handler_test.go +++ b/backend/internal/transport/handlers/health_handler_test.go @@ -31,7 +31,7 @@ func TestHealthHandler_Success(t *testing.T) { Status: "up", Message: "It's healthy", } - h := NewHandler(&mockHealthUC{stats: want}, nil, nil, nil, nil, nil, nil, nil, nil) + h := NewHandler(&mockHealthUC{stats: want}, nil, nil, nil, nil, nil, nil, nil, nil, nil) r := gin.New() r.GET("/health", h.HealthHandler) @@ -50,7 +50,7 @@ func TestHealthHandler_Success(t *testing.T) { } func TestHealthHandler_ServiceUnavailable(t *testing.T) { - h := NewHandler(&mockHealthUC{err: errors.New("connection refused")}, nil, nil, nil, nil, nil, nil, nil, nil) + h := NewHandler(&mockHealthUC{err: errors.New("connection refused")}, nil, nil, nil, nil, nil, nil, nil, nil, nil) r := gin.New() r.GET("/health", h.HealthHandler) diff --git a/backend/internal/transport/handlers/routes.go b/backend/internal/transport/handlers/routes.go index 4da0a2b..177e46e 100644 --- a/backend/internal/transport/handlers/routes.go +++ b/backend/internal/transport/handlers/routes.go @@ -63,6 +63,9 @@ func (h *Handler) RegisterRoutes(rps float64, burst int, sentryDSN string) http. if h.verifier != nil { api.Use(middleware.FirebaseAuth(h.verifier)) } + if h.geoLocator != nil { + api.Use(middleware.GeoFromRequest(h.geoLocator)) + } api.GET("/me", h.MeHandler) if h.fcmTokenRepo != nil { diff --git a/backend/internal/transport/middleware/geo.go b/backend/internal/transport/middleware/geo.go new file mode 100644 index 0000000..7095233 --- /dev/null +++ b/backend/internal/transport/middleware/geo.go @@ -0,0 +1,45 @@ +package middleware + +import ( + "net" + "net/http" + "strings" + + "github.com/gin-gonic/gin" + + "backend/internal/usecase" +) + +// GeoLocationKey is the Gin context key under which *domain.GeoLocation is stored. +const GeoLocationKey = "geo_location" + +// GeoFromRequest is a best-effort middleware that resolves the request IP to +// geographic metadata and stores it in the Gin context under GeoLocationKey. +// If geolocation fails (private IP, rate-limit, network error), the request +// continues without geo data — handlers must nil-check before reading the key. +func GeoFromRequest(locator usecase.GeoLocator) gin.HandlerFunc { + return func(c *gin.Context) { + ip := RealIP(c.Request) + if geo, err := locator.Lookup(c.Request.Context(), ip); err == nil { + c.Set(GeoLocationKey, geo) + } + c.Next() + } +} + +// RealIP extracts the originating IP from the request, respecting +// X-Forwarded-For (Railway/proxy) and X-Real-IP headers. +// Exported so tests and other packages can call it directly. +func RealIP(r *http.Request) string { + if xff := r.Header.Get("X-Forwarded-For"); xff != "" { + return strings.TrimSpace(strings.SplitN(xff, ",", 2)[0]) + } + if xri := r.Header.Get("X-Real-IP"); xri != "" { + return strings.TrimSpace(xri) + } + host, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + return r.RemoteAddr + } + return host +} diff --git a/backend/internal/transport/middleware/geo_test.go b/backend/internal/transport/middleware/geo_test.go new file mode 100644 index 0000000..b8da443 --- /dev/null +++ b/backend/internal/transport/middleware/geo_test.go @@ -0,0 +1,162 @@ +package middleware_test + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + + "backend/internal/domain" + "backend/internal/infrastructure/ipgeo" + "backend/internal/transport/middleware" + "backend/internal/usecase" +) + +// mockGeoLocator is a test double implementing usecase.GeoLocator. +type mockGeoLocator struct { + geo *domain.GeoLocation + err error +} + +func (m *mockGeoLocator) Lookup(_ context.Context, _ string) (*domain.GeoLocation, error) { + return m.geo, m.err +} + +// Compile-time check. +var _ usecase.GeoLocator = (*mockGeoLocator)(nil) + +func TestGeoFromRequest_AttachesGeoOnSuccess(t *testing.T) { + gin.SetMode(gin.TestMode) + + want := &domain.GeoLocation{ + IP: "1.2.3.4", + CountryCode: "US", + CountryName: "United States", + City: "Mountain View", + } + locator := &mockGeoLocator{geo: want} + + var captured *domain.GeoLocation + r := gin.New() + r.Use(middleware.GeoFromRequest(locator)) + r.GET("/", func(c *gin.Context) { + val, exists := c.Get(middleware.GeoLocationKey) + if !exists { + t.Error("geo_location key not set in context") + c.Status(http.StatusInternalServerError) + return + } + geo, ok := val.(*domain.GeoLocation) + if !ok { + t.Errorf("geo_location wrong type: %T", val) + c.Status(http.StatusInternalServerError) + return + } + captured = geo + c.Status(http.StatusOK) + }) + + req := httptest.NewRequest("GET", "/", nil) + req.RemoteAddr = "1.2.3.4:1234" + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } + if captured == nil || captured.CountryCode != want.CountryCode { + t.Errorf("captured geo mismatch: got %+v, want %+v", captured, want) + } +} + +func TestGeoFromRequest_SkipsOnError(t *testing.T) { + gin.SetMode(gin.TestMode) + + locator := &mockGeoLocator{err: errors.New("rate limited")} + + reached := false + r := gin.New() + r.Use(middleware.GeoFromRequest(locator)) + r.GET("/", func(c *gin.Context) { + reached = true + _, exists := c.Get(middleware.GeoLocationKey) + if exists { + t.Error("geo_location key should not be set on error") + } + c.Status(http.StatusOK) + }) + + req := httptest.NewRequest("GET", "/", nil) + req.RemoteAddr = "1.2.3.4:1234" + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if !reached { + t.Error("handler was not reached — middleware aborted the request") + } + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } +} + +func TestGeoFromRequest_PrivateIPSkipped(t *testing.T) { + gin.SetMode(gin.TestMode) + + locator := &mockGeoLocator{err: ipgeo.ErrPrivateIP} + + reached := false + r := gin.New() + r.Use(middleware.GeoFromRequest(locator)) + r.GET("/", func(c *gin.Context) { + reached = true + _, exists := c.Get(middleware.GeoLocationKey) + if exists { + t.Error("geo_location key should not be set for private IP") + } + c.Status(http.StatusOK) + }) + + req := httptest.NewRequest("GET", "/", nil) + req.RemoteAddr = "127.0.0.1:5678" + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if !reached { + t.Error("handler was not reached — middleware aborted the request") + } +} + +func TestRealIP_XForwardedFor(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("X-Forwarded-For", "1.2.3.4, 10.0.0.1") + req.RemoteAddr = "10.0.0.1:9999" + + got := middleware.RealIP(req) + if got != "1.2.3.4" { + t.Errorf("RealIP: got %q, want %q", got, "1.2.3.4") + } +} + +func TestRealIP_XRealIP(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("X-Real-IP", "5.6.7.8") + req.RemoteAddr = "10.0.0.1:9999" + + got := middleware.RealIP(req) + if got != "5.6.7.8" { + t.Errorf("RealIP: got %q, want %q", got, "5.6.7.8") + } +} + +func TestRealIP_RemoteAddr(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + req.RemoteAddr = "9.10.11.12:4567" + + got := middleware.RealIP(req) + if got != "9.10.11.12" { + t.Errorf("RealIP: got %q, want %q", got, "9.10.11.12") + } +} diff --git a/backend/internal/usecase/geolocation.go b/backend/internal/usecase/geolocation.go new file mode 100644 index 0000000..c286794 --- /dev/null +++ b/backend/internal/usecase/geolocation.go @@ -0,0 +1,12 @@ +package usecase + +import ( + "context" + + "backend/internal/domain" +) + +// GeoLocator resolves an IP address to geographic metadata. +type GeoLocator interface { + Lookup(ctx context.Context, ip string) (*domain.GeoLocation, error) +} From 11094ca91905210c96a83f0d9daf786f38978e92 Mon Sep 17 00:00:00 2001 From: GRACENOBLE Date: Tue, 23 Jun 2026 08:13:04 +0300 Subject: [PATCH 2/2] fix(geo): address CodeRabbit review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Reject malformed IP strings before isPrivateIP/fetch — net.ParseIP returning nil now returns an error immediately, preventing unnecessary outbound calls from untrusted header input - Log cache read/write errors via slog.WarnContext instead of silently swallowing them; errors are non-fatal (cache miss falls through to HTTP, cache write failure still returns valid geo data) - RealIP now only trusts X-Forwarded-For / X-Real-IP when RemoteAddr is a private or loopback address (i.e. the connection came through a trusted proxy); direct clients with public RemoteAddr cannot spoof the originating IP via forwarding headers - Add TestClient_Lookup_InvalidIP and TestRealIP_XForwardedFor_IgnoredFromPublicAddr --- .../infrastructure/ipgeo/ipapi_client.go | 22 ++++++++----- .../infrastructure/ipgeo/ipapi_client_test.go | 9 ++++++ backend/internal/transport/middleware/geo.go | 31 ++++++++++++------- .../internal/transport/middleware/geo_test.go | 13 ++++++++ 4 files changed, 57 insertions(+), 18 deletions(-) diff --git a/backend/internal/infrastructure/ipgeo/ipapi_client.go b/backend/internal/infrastructure/ipgeo/ipapi_client.go index ffde1dc..5e6e774 100644 --- a/backend/internal/infrastructure/ipgeo/ipapi_client.go +++ b/backend/internal/infrastructure/ipgeo/ipapi_client.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "log/slog" "net" "net/http" "time" @@ -71,16 +72,22 @@ func cacheKey(ip string) string { } // Lookup resolves ip to geographic metadata. -// Returns ErrPrivateIP for loopback and RFC-1918 addresses without making -// any outbound HTTP call or cache lookup. +// Returns an error for malformed or private/loopback IPs without making any +// outbound HTTP call or cache lookup. func (c *Client) Lookup(ctx context.Context, ip string) (*domain.GeoLocation, error) { + if net.ParseIP(ip) == nil { + return nil, fmt.Errorf("ipgeo: invalid IP address: %q", ip) + } if isPrivateIP(ip) { return nil, ErrPrivateIP } - // Check cache first. + // Check cache first; log but don't abort on Redis errors. if c.cache != nil { - if val, ok, err := c.cache.Get(ctx, cacheKey(ip)); err == nil && ok { + val, ok, err := c.cache.Get(ctx, cacheKey(ip)) + if err != nil { + slog.WarnContext(ctx, "ipgeo: cache get failed", "ip", ip, "error", err) + } else if ok { var geo domain.GeoLocation if jsonErr := json.Unmarshal([]byte(val), &geo); jsonErr == nil { return &geo, nil @@ -93,11 +100,12 @@ func (c *Client) Lookup(ctx context.Context, ip string) (*domain.GeoLocation, er return nil, err } - // Populate cache. + // Populate cache; log but don't fail the lookup on Redis errors. if c.cache != nil { if data, jsonErr := json.Marshal(geo); jsonErr == nil { - // Best-effort; ignore cache write errors. - _ = c.cache.Set(ctx, cacheKey(ip), string(data), 24*time.Hour) + if setErr := c.cache.Set(ctx, cacheKey(ip), string(data), 24*time.Hour); setErr != nil { + slog.WarnContext(ctx, "ipgeo: cache set failed", "ip", ip, "error", setErr) + } } } diff --git a/backend/internal/infrastructure/ipgeo/ipapi_client_test.go b/backend/internal/infrastructure/ipgeo/ipapi_client_test.go index 389bc8c..f7e2246 100644 --- a/backend/internal/infrastructure/ipgeo/ipapi_client_test.go +++ b/backend/internal/infrastructure/ipgeo/ipapi_client_test.go @@ -45,6 +45,15 @@ func (m *mockCacheService) Close() error { return nil } // Compile-time check: mockCacheService satisfies usecase.CacheService. var _ usecase.CacheService = (*mockCacheService)(nil) +func TestClient_Lookup_InvalidIP(t *testing.T) { + client := NewWithBaseURL(nil, "", "http://should-not-be-called") + + _, err := client.Lookup(context.Background(), "not-an-ip") + if err == nil { + t.Fatal("expected error for malformed IP, got nil") + } +} + func TestClient_Lookup_ValidIP(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") diff --git a/backend/internal/transport/middleware/geo.go b/backend/internal/transport/middleware/geo.go index 7095233..eb83adc 100644 --- a/backend/internal/transport/middleware/geo.go +++ b/backend/internal/transport/middleware/geo.go @@ -27,19 +27,28 @@ func GeoFromRequest(locator usecase.GeoLocator) gin.HandlerFunc { } } -// RealIP extracts the originating IP from the request, respecting -// X-Forwarded-For (Railway/proxy) and X-Real-IP headers. +// RealIP extracts the originating IP from the request. +// Forwarding headers (X-Forwarded-For, X-Real-IP) are only trusted when +// RemoteAddr is a private or loopback address — i.e. the connection actually +// came through a trusted proxy (Railway, nginx). Direct clients cannot spoof +// the originating IP this way. // Exported so tests and other packages can call it directly. func RealIP(r *http.Request) string { - if xff := r.Header.Get("X-Forwarded-For"); xff != "" { - return strings.TrimSpace(strings.SplitN(xff, ",", 2)[0]) - } - if xri := r.Header.Get("X-Real-IP"); xri != "" { - return strings.TrimSpace(xri) - } - host, _, err := net.SplitHostPort(r.RemoteAddr) + remoteHost, _, err := net.SplitHostPort(r.RemoteAddr) if err != nil { - return r.RemoteAddr + remoteHost = r.RemoteAddr } - return host + + // Only honour forwarding headers from a trusted proxy (private/loopback). + remoteIP := net.ParseIP(remoteHost) + if remoteIP != nil && (remoteIP.IsLoopback() || remoteIP.IsPrivate()) { + if xff := r.Header.Get("X-Forwarded-For"); xff != "" { + return strings.TrimSpace(strings.SplitN(xff, ",", 2)[0]) + } + if xri := r.Header.Get("X-Real-IP"); xri != "" { + return strings.TrimSpace(xri) + } + } + + return remoteHost } diff --git a/backend/internal/transport/middleware/geo_test.go b/backend/internal/transport/middleware/geo_test.go index b8da443..af76be4 100644 --- a/backend/internal/transport/middleware/geo_test.go +++ b/backend/internal/transport/middleware/geo_test.go @@ -160,3 +160,16 @@ func TestRealIP_RemoteAddr(t *testing.T) { t.Errorf("RealIP: got %q, want %q", got, "9.10.11.12") } } + +func TestRealIP_XForwardedFor_IgnoredFromPublicAddr(t *testing.T) { + // A direct client with a public RemoteAddr must not be able to spoof + // the originating IP via X-Forwarded-For. + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("X-Forwarded-For", "evil.spoofed.ip, 1.2.3.4") + req.RemoteAddr = "5.6.7.8:1234" // public IP — not a trusted proxy + + got := middleware.RealIP(req) + if got != "5.6.7.8" { + t.Errorf("RealIP: got %q, want RemoteAddr %q (XFF should be ignored)", got, "5.6.7.8") + } +}