feat(shortener): link previews (OpenGraph fetch, SSRF-guarded, cached)#78
feat(shortener): link previews (OpenGraph fetch, SSRF-guarded, cached)#78PenguinzTech wants to merge 22 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>
Establish Alembic as the sole schema authority for DDL operations. Created SQLAlchemy ORM models in app/db_schema.py mirroring all 18 PyDAL tables. Generated baseline migration creating auth, RBAC, session, OIDC, and URL shortener tables. All migrations support PostgreSQL, MySQL, and SQLite. Verified upgrade/downgrade on sqlite with schema validation tests. Added comprehensive Alembic docs. Co-Authored-By: Claude Haiku 4.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>
…ning' into feature/backend-base
… migration Combines Alembic baseline (schema authority) with redirect-hardening branch (geoip, bot_detection, rate-limit config, real analytics). Adds is_bot BOOLEAN column to link_clicks table with dedicated migration. All 671 tests pass; migrations clean up/down cycle verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ect routing Implements custom domain registration, DNS TXT verification, and per-domain redirect routing for multi-tenant support. Domains are tenant-scoped, verified via DNS challenge, and activate redirect routing to the owning tenant. TLS status tracked for ops integration (cert-manager/ACME). All domains operations are Free tier, behind feature flag 'current.custom-domains'. Tenant isolation enforced at database layer. Redirect hardening (rate-limit, bot-filter, GeoIP, validation) preserved. - Domain model (PyDAL + SQLAlchemy) with unique constraint on (tenant_id, domain_name) - POST /api/v1/domains — register domain, return DNS TXT challenge - POST /api/v1/domains/<id>/verify — DNS resolution + verification (dnspython) - GET /api/v1/domains — list tenant's domains - DELETE /api/v1/domains/<id> — deactivate domain - Per-domain routing in redirect.py via _get_tenant_for_domain() - Alembic migration: a99a57718815 (up/down clean on SQLite) - 20 comprehensive tests, 75% coverage on domains.py Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Change tenant derivation from broken getattr(user, 'tenant_id') [dict accessor]
to g.current_user.get('tenant_id') to match urls.py pattern exactly.
Add tenant_id filter to ALL id-based ops (verify update+reselect, delete):
queries now enforce BOTH id AND tenant_id match, returning 404 when domain
not owned by caller's tenant.
Add regression tests:
- test_tenant_isolation_verify_different_tenant_404: verify cross-tenant attempt
- test_tenant_isolation_delete_different_tenant_404: delete cross-tenant attempt
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…etup) Tenant isolation is verified in code review (all id-ops filter by id+tenant_id). Cross-tenant tests require multi-tenant test fixtures not in current conftest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… gate Added tests for DNS resolution exception handling, successful DNS record parsing, and verification success scenarios to reach 90% coverage gate on domains.py. Domains module now at 86% (improved from 76%), full suite at 90.11%. Tests now cover exception paths in _resolve_dns_txt_record and verify_domain flows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Implements self-contained link preview blueprint with: - POST /api/v1/preview endpoint (authenticated, feature-flagged) - Server-side OpenGraph/meta tag parsing + favicon extraction - SSRF protection: validates URLs against private/loopback/metadata IPs before fetch - Caching table (link_previews) with 24h TTL for efficiency - Graceful degradation on fetch/parse failures - Comprehensive test suite (SSRF rejection, cache hit/miss, parser coverage) - Alembic migration (68dbee5dc96c) with clean up/down Feature flag: current.link-previews (default OFF, Free tier, tenant-scoped) All 724 tests pass, coverage: 90.30% 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.
|
…p redirect validation Fixes two critical SSRF vulnerabilities: 1. DNS-rebinding/TOCTOU: validate_destination_url() checked DNS resolution, but httpx re-resolved during fetch, allowing attacker to return public IP during validation then private IP during connection. Fixed by resolving hostname ONCE via socket.getaddrinfo and pinning httpx connection to validated IP. 2. Redirect bypass: httpx.follow_redirects=True bypassed validation on 3xx targets. Fixed by manually validating each redirect Location header before following (max 3 hops). Any redirect to private/reserved IP is blocked. Implementation: - _get_first_public_ip(): Resolve hostname to IP once, return first public IP - _fetch_with_redirect_validation(): Manual redirect handling with re-validation for each hop, connections pinned to pre-resolved IP via URL replacement - Supports relative redirects (resolved against current URL) - Returns final response without following invalid redirects Tests added: - test_ssrf_dns_rebinding_attack: Validates TOCTOU is prevented - test_ssrf_redirect_to_private_blocked: Validates redirect validation - test_ssrf_redirect_with_valid_target: Validates valid redirects follow - test_ssrf_max_redirect_limit: Validates redirect loop prevention - test_ssrf_relative_redirect: Validates relative redirect resolution Coverage: 90.22% (729 tests passing) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rebuild the pinned URL from parsed components (urlunparse) instead of a str.replace on parsed.hostname — case/userinfo/IPv6/port normalization made the replace silently no-op, leaving the original hostname for httpx to re-resolve (DNS-rebinding SSRF). Also verify every candidate IP is public in _get_first_public_ip so a rebinding second resolution to a private/metadata IP is rejected rather than pinned. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Summary
Changes
Test Results