feat(shortener): real GeoIP analytics + redirect rate-limit/bot-filter/unique clicks#70
feat(shortener): real GeoIP analytics + redirect rate-limit/bot-filter/unique clicks#70PenguinzTech wants to merge 11 commits into
Conversation
…startup Fixes two critical findings: - C1: JWT_SECRET_KEY no longer falls back to the dev SECRET_KEY default in production. ProductionConfig now requires it via env var and validate() rejects the dev default, closing an admin-token forgery vector. - C2: create_app() now calls ProductionConfig.validate() at startup (guarded by hasattr so Dev/Testing configs are unaffected), so the dead secret-guard actually runs and the app boot-fails on default/missing secrets. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tokens off URL Fixes account-takeover and token-leak findings in the OIDC/SSO consumer: - OC1: callback now validates the id_token (JWKS signature, iss/aud/exp/nonce) instead of blindly trusting the userinfo response; identity derives from validated claims. Adds PKCE (S256) + nonce to authorize/callback. - OC2: auto-link/auto-provision gated on email_verified (+ OIDC_AUTO_LINK_ VERIFIED_EMAIL flag, default off); no more silent email-based account linking. - OH4: access/refresh tokens returned as httpOnly Secure SameSite cookies, not in the redirect URL query string. - OH5: identity linking only via authenticated OIDC round-trip; removed body-supplied external_sub/external_email linking. Adds nullable columns oidc_providers.issuer, user_oidc_links.email_verified. _encrypt_secret fail-open and update_provider gating deferred to A5. Full backend suite: 403 passed, 1 skipped, 90.16% coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…H3/H1) Closes IDOR: a global viewer holds global tenants:read/teams:read, so the scope gate passed for everyone and routes then loaded any object by id with no membership check — any user could read any org's tenants/teams + member emails across tenants. - Adds membership helpers in rbac.py; each object-scoped tenant/team route now requires the caller to be a global admin OR a member of that specific tenant/team (admin ops require tenant/team admin scope, resolved with the id so tenant/team-scoped roles are actually consulted). - create_team validates the body tenant_id against caller membership (no more cross-tenant writes). - Regression tests: non-member viewer 403, member 200, global admin 200, foreign-tenant create 403. Also gitignore /worktrees/ (repo-local worktree convention). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…follow-up) The initial isolation fix guarded only read routes; write/admin routes still had just a global scope gate, so a user with global teams:write/tenants:write (e.g. a global maintainer) could modify/delete any team/tenant and grant roles across orgs — privilege escalation. (Also restores object-scoped authz that had regressed when team_id_param was dropped from the decorators.) - Adds _is_team_admin / _is_tenant_admin helpers (global admin scope OR team/tenant- scoped admin role for THAT object). - update/delete/add-member/remove-member on both teams.py and tenants.py now require the caller to be an admin of that specific team/tenant (or global admin). - add_team_member/add_tenant_member especially: they grant roles incl. *_admin, so non-admins of the object are now blocked. - Regression tests: global maintainer (no admin role) → 403 on every write route; team/tenant admin → 200/201. Full suite: 423 passed, 90.10% coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uth introspect/revoke (C3/H3/M4/OM1) - C3: access tokens now carry a jti; a revoked_access_tokens blocklist is checked in auth_required, so /logout and /oauth/revoke actually invalidate the current access token (previously only refresh tokens were revoked). Blocklist self-prunes on expiry. - H3/M4: real rate limiter (app/rate_limiter.py) — Redis-backed when REDIS_ENABLED else in-memory, respects RATE_LIMIT_ENABLED. Applied to /auth/login, /auth/register, /oauth/token; returns 429 on limit. - OM1: /oauth/introspect and /oauth/revoke now require authentication; introspect returns active:false for revoked tokens or deactivated (is_active=false) users. Tests: revocation, rate-limit 429, oauth-security + comprehensive rate_limiter tests. Suite: 441 passed, 1 skipped, 90.82% coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…time, logged errors, safe admin seed (OM4/M1/M2/M3/H2) - OM4: refresh grant (both /refresh and /oauth/token) now resolves tenant_id + scopes like the password grant, so refreshed access tokens keep their tenant/scope claims. - M1: define rbac + oidc tables (and the former inline create_user table) once at startup in init_db, eliminating the request-time check-then-define race on the shared DAL across executor threads. - M2: datetime.utcnow()/utcfromtimestamp() -> timezone-aware datetime.now(timezone.utc) in token exp/iat and expiry comparisons. - M3: replace silent except: pass around tenant/scope resolution and revoke storage with logged warnings (no PII/secrets), so degraded-permission failures are visible. - H2: in production, refuse to seed the default admin with the built-in weak password; require DEFAULT_ADMIN_PASSWORD to be set or skip seeding. Dev keeps the convenience default with a prominent warning. Suite: 467 passed, 90.44% coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…des, SSRF-safe URL validation (B1-B2) Foundation of the open-source URL shortener: - Schema (models.py + DDL for postgres/mysql/sqlite, defined at startup): urls (short_code UNIQUE, tenant/team/collection FKs, expiry, click_count), collections (folders via parent_id, unique per tenant+parent), link_clicks (ip_hash only, for the redirect task). - app/urls.py + app/collections.py blueprints under /api/v1, all @auth_required + @require_scope + tenant-scoped (tenant derived from token, never body; no cross-tenant access; creator-or-tenant-admin for edits). - Short-code generation via secrets (not random); DB UNIQUE + bounded retry. - app/urlvalidation.py: scheme allowlist (blocks javascript:/data:/file:), rejects literal + resolved private/loopback/link-local/reserved/multicast/metadata IPs (IPv4 incl. decimal/hex/octal, IPv6); unresolvable public hostnames allowed (the server 301-redirects, never fetches). Reserved-path list for aliases. - rbac.py: collections:read/write/delete scopes added to role bundles. - Redirect, click-tracking, analytics, QR, and tier-gating are follow-up tasks (TODO hooks left). Tests: 595 passed, 90.92% coverage — incl. SSRF bypass payloads rejected, alias collision, tenant isolation, collection nesting/cycle prevention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ics (B3-B4) - app/redirect.py: public GET /<short_code> → 302 to long_url (404 for unknown/ inactive/expired), destination re-validated before redirect. Click tracking is non-blocking (asyncio.create_task after the response); click_count incremented atomically. link_clicks stores ip_hash (salted SHA256, never raw IP) + UA-parsed device/browser/os + referer. - QR: GET /api/v1/urls/<id>/qr (auth + urls:read + tenant-scoped) returns on-demand PNG of the short URL (qrcode+Pillow), not stored. TODO(tiering) branded/color QR. - app/analytics.py: /analytics/urls/<id> (per-link: total, clicks-by-day, top referers) and /analytics/summary (tenant-wide totals, top links, time series), analytics:read + tenant-scoped. Basic tiers implemented; geo/device/browser breakdowns marked TODO(tiering) for the Enterprise advanced-analytics gate. - deps: qrcode, Pillow, user-agents (pinned+hashed via uv). Tests: 619 passed, 90.66% coverage — redirect/404/expiry, async click + ip_hash, QR png, analytics date-filter/pagination/tenant-isolation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…license (B5) Two-layer gating (PostHog flag defaulted OFF + license tier), per house tier matrix: - app/features.py: feature_enabled() (graceful degradation, cached last-known, new flags OFF, never crash), get_license_tier() (free/professional/enterprise; dev & bypass-domain → enterprise; production fails CLOSED to free on error), require_tier/require_professional/require_enterprise decorators → 402 when below. - Free 5-collection cap enforced server-side in collections.py (402 on 6th; Pro/Ent unlimited) — flag current.collections. - Branded QR (color/logo params) gated to Professional in urls.py QR endpoint (plain QR stays Free) — flag current.branded-qr. - Advanced analytics (geo/device/browser/OS breakdowns) at /analytics/urls/<id>/advanced gated to Enterprise; basic analytics stays all-tiers — flag current.advanced-analytics. - config: POSTHOG_KEY/POSTHOG_HOST (default https://license.penguintech.io). Tests: 649 passed, 90% coverage; features.py 100% (tier resolution, flag degradation, each gate 402/allow, mocked posthog+license clients, no network). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…verage >90%) Adds comprehensive test coverage for: - Advanced analytics endpoint with Enterprise tier gating and feature flags - Date filtering (from/to) in advanced analytics - Advanced breakdown fields (geo, devices, browsers, os) - Redirect error paths: invalid destination URL validation, user-agent parsing - Click recording error handling and database exceptions - Edge cases: no clicks, no user-agent header, database errors with rollback Coverage improvements: - analytics.py: 72% → 92% (advanced endpoint now fully covered) - redirect.py: 23% → 90% (error paths and edge cases now covered) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r/unique clicks - Add MaxMind GeoLite2 integration (geoip2 library) for real country/region/city geo data with graceful degradation if DB absent - Implement bot detection via user-agent heuristics; flag is_bot on clicks, exclude from click_count + analytics totals - Apply rate limiting to public redirect endpoint (1000 req/IP/min configurable) with fail-open on Redis backend error - Track unique visitors (distinct by ip_hash) in analytics, returned alongside totals - Replace hardcoded geo/device/browser/os placeholders in advanced analytics with real GROUP BY aggregations over link_clicks; exclude bots from all breakdowns - Add is_bot boolean column to link_clicks table (migration needed separately) - Implement PII-safe IP hashing (hash-only, never store raw IP) - All changes behind PostHog feature flags with graceful degradation Files added: - app/geoip.py: GeoIP client wrapper - app/bot_detection.py: User-agent-based bot detection Files modified: - app/redirect.py: Apply rate limiting, bot detection, GeoIP lookup, exclude bots - app/analytics.py: Real aggregations for geo/device/browser/os, unique visitor counts - app/models.py: Add is_bot column to link_clicks (both SQLite/MySQL and PostgreSQL) - app/config.py: Add GEOIP_DB_PATH and RATE_LIMIT_REDIRECT config - app/__init__.py: Initialize GeoIP client at startup - requirements.in/txt: Add geoip2 dependency - tests/test_redirect_analytics.py: Comprehensive coverage for bot filtering, rate limiting, geo aggregation, unique visitors, graceful degradation Test coverage: 46/46 pass. All redis-dependent paths have fallback logic. Production deployment: Provide GEOIP_DB_PATH to enable geo lookups; if absent, gracefully continues with country/region/city = None. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @PenguinzTech, your pull request is larger than the review limit of 150000 diff characters
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
1 participant
Summary
Hardens the shortener redirect + analytics path by replacing hardcoded placeholder data with real geography lookups, implementing bot detection, and adding rate limiting. Two blocking issues fixed:
All changes gracefully degrade if dependencies unavailable (GeoIP DB, Redis). Bots detected via user-agent heuristics; clicks still recorded but flagged
is_bot=trueand excluded from totals/analytics.Implementation
New modules:
app/geoip.py: MaxMind GeoLite2 client with graceful degradation (if DB absent, returns None for geo fields)app/bot_detection.py: User-agent pattern matching for common bots (Googlebot, curl, wget, etc.)Modified:
app/redirect.py: Apply rate limit check + bot detection before redirect; pass geo/is_bot to async click recorderapp/analytics.py: Real aggregations for geo/device/browser/os; track unique_visitors (distinct ip_hash); exclude bots from all counts + breakdownsapp/models.py: Addis_bot booleancolumn to link_clicks (platform-neutral SQL for PostgreSQL/SQLite/MySQL)app/config.py: Add GEOIP_DB_PATH, RATE_LIMIT_REDIRECT env varsapp/__init__.py: Initialize GeoIP + rate limiter at app startuprequirements.in/txt: Add geoip2==4.7.0 dependencyTests:
46/46 tests pass covering:
Columns Added (for migration author)
Add to Alembic migration for link_clicks table:
is_bot BOOLEAN DEFAULT FALSE— Flag for bot-detected clicksProduction Deployment
Required setup:
GEOIP_DB_PATH=/path/to/GeoLite2-City.mmdbGraceful degradation: If GEOIP_DB_PATH absent or file not found, geo fields remain NULL and a single warning is logged (never crashes).
Rate limiting: Uses existing
RATE_LIMIT_REDIRECTconfig (default 1000 req/IP/min). Falls open if Redis unavailable.Stacking
This PR stacks logically on #66 (shortener tiering). No conflicts; ready to merge to
release/v0.1.x.🤖 Generated with Claude Code