Skip to content

Add IP banning: connection logging, IP bans, and transitive banning - #263

Open
decentraland-bot wants to merge 2 commits into
mainfrom
feature/ip-banning
Open

Add IP banning: connection logging, IP bans, and transitive banning#263
decentraland-bot wants to merge 2 commits into
mainfrom
feature/ip-banning

Conversation

@decentraland-bot

Copy link
Copy Markdown

Summary

This PR adds three related capabilities to the comms-gatekeeper moderation system:

1. Connection logging

  • Every successful POST /get-scene-adapter request now logs a (address, ip) pair to a new connection_logs table.
  • The client IP is extracted from x-forwarded-for (first entry) → x-real-ip → falls back to undefined (no-op) if neither header is present.
  • Note: a scheduled job to purge rows older than 90 days is a follow-up item; a comment in the migration notes this.

2. IP banning

  • New ip_bans table mirrors the shape of user_bans (same fields, keyed on banned_ip).
  • IP ban check runs on every token request before issuing credentials; banned IPs receive a 403.
  • New REST endpoints:
    • POST /ips/:ip/bans — ban an IP (mod auth required); body: { reason, duration?, customMessage?, banAllKnownAddresses? }
    • GET /ips/:ip/bans — get IP ban status (public)
    • DELETE /ips/:ip/bans — lift an IP ban (mod auth required)

3. Transitive banning

  • GET /users/:address/ips — list all IPs a wallet has connected from (mod read auth)
  • GET /ips/:ip/users — list all wallets seen from an IP (mod read auth)
  • POST /users/:address/bans now accepts banAllKnownIps?: boolean — if true, also bans all IPs the wallet has connected from.
  • POST /ips/:ip/bans accepts banAllKnownAddresses?: boolean — if true, also bans all wallets seen from that IP.
  • All transitive operations skip already-banned targets to avoid IpAlreadyBannedError / PlayerAlreadyBannedError.

New files

File Purpose
src/migrations/1772000000000_ip-banning.ts Adds connection_logs and ip_bans tables
src/adapters/ip-moderation-db.ts DB queries for IP bans and connection logs
src/logic/ip-moderation/types.ts Interfaces and types
src/logic/ip-moderation/errors.ts IpAlreadyBannedError, IpBanNotFoundError
src/logic/ip-moderation/component.ts Business logic (normalisation, transitive ops)
src/logic/ip-moderation/extract-ip.ts IP extraction utility from request headers
src/controllers/handlers/ip-moderation/ HTTP handlers (ban, status, lift, list)

Modified files

  • src/types.ts — adds ipModerationDb and ipModeration to BaseComponents
  • src/components.ts — instantiates and wires the new components
  • src/controllers/routes.ts — registers the new routes
  • src/controllers/handlers/comms-scene-handler.ts — adds IP ban check and connection logging
  • src/controllers/handlers/error-handler.ts — handles IpAlreadyBannedError and IpBanNotFoundError
  • src/controllers/handlers/user-moderation/ban-player-handler.ts — supports banAllKnownIps
  • src/controllers/handlers/user-moderation/schemas.ts — adds banAllKnownIps to BanPlayerSchema

Test plan

  • Unit tests added in test/unit/ip-moderation/ip-moderation.spec.ts covering all component methods
  • Integration tests added in test/integration/ip-moderation/ban-ip-handler.spec.ts covering all new endpoints
  • TypeScript compiles with zero errors (tsc --noEmit)
  • Existing comms-scene-handler integration tests unaffected (IP check is skipped when no IP header is present)

🤖 Generated with Claude Code

…anning

- New migration (1772000000000_ip-banning) adds connection_logs and ip_bans
  tables. connection_logs records wallet↔IP pairs on every successful token
  request; a 90-day TTL cleanup cron is a follow-up item.

- IP extraction (x-forwarded-for → x-real-ip) runs on every comms-scene
  request; the (address, ip) pair is logged after credentials are issued.

- IP ban check is performed before issuing a token; banned IPs receive 403.

- New DB adapter (ip-moderation-db) and logic component (ip-moderation)
  mirror the existing user-moderation layer: banIp, liftIpBan,
  getIpBanStatus, logConnection, getIpsByAddress, getAddressesByIp,
  banAllIpsForAddress, banAllAddressesForIp.

- New REST endpoints: POST/GET/DELETE /ips/:ip/bans, GET /users/:address/ips,
  GET /ips/:ip/users; all protected by the existing moderator middleware.

- POST /users/:address/bans gains an optional banAllKnownIps boolean for
  transitive banning; POST /ips/:ip/bans gains banAllKnownAddresses.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 26661766696

Coverage decreased (-0.5%) to 84.791%

Details

  • Coverage decreased (-0.5%) from the base build.
  • Patch coverage: 34 uncovered changes across 10 files (162 of 196 lines covered, 82.65%).
  • 2 coverage regressions across 1 file.

Uncovered Changes

File Changed Covered %
src/controllers/handlers/comms-scene-handler.ts 14 4 28.57%
src/adapters/ip-moderation-db.ts 31 27 87.1%
src/controllers/handlers/ip-moderation/ban-ip-handler.ts 15 12 80.0%
src/controllers/handlers/ip-moderation/get-addresses-by-ip-handler.ts 9 6 66.67%
src/controllers/handlers/ip-moderation/get-ips-by-address-handler.ts 9 6 66.67%
src/controllers/handlers/ip-moderation/ip-ban-status-handler.ts 9 6 66.67%
src/controllers/handlers/ip-moderation/lift-ip-ban-handler.ts 12 9 75.0%
src/logic/ip-moderation/extract-ip.ts 8 6 75.0%
src/migrations/1772000000000_ip-banning.ts 11 9 81.82%
src/controllers/handlers/user-moderation/ban-player-handler.ts 2 1 50.0%
Total (17 files) 196 162 82.65%

Coverage Regressions

2 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
src/adapters/scene-stream-access-manager.ts 2 20.93%

Coverage Stats

Coverage Status
Relevant Lines: 3590
Covered Lines: 3163
Line Coverage: 88.11%
Relevant Branches: 1440
Covered Branches: 1102
Branch Coverage: 76.53%
Branches in Coverage %: Yes
Coverage Strength: 73.87 hits per line

💛 - Coveralls

Implement GDPR data subject rights (Art. 15, 17, 20) for the IP banning
feature since IP addresses are PII under GDPR:

- GET /users/:address/personal-data — exports all PII (connection logs,
  bans, warnings, IP bans) for a wallet address (moderator read access)
- DELETE /users/:address/personal-data — purges connection logs for a
  wallet address while retaining moderation records under legitimate
  interest basis (moderator write access)
- Automated daily retention cleanup purging connection logs older than
  configurable CONNECTION_LOG_RETENTION_DAYS (default 90 days)
- Migration adding index on connection_logs.connected_at for efficient
  TTL purge queries

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants