Skip to content

Add login System + Complete system overhaul - #94

Merged
Zingzy merged 124 commits into
mainfrom
login-system
Nov 18, 2025
Merged

Add login System + Complete system overhaul#94
Zingzy merged 124 commits into
mainfrom
login-system

Conversation

@Zingzy

@Zingzy Zingzy commented Nov 10, 2025

Copy link
Copy Markdown
Member

🎉 Major Feature Release: Complete Authentication & URL Management System

📋 Overview

This PR introduces a comprehensive authentication system, complete URL management capabilities, and a modern dashboard UI to the spoo.me URL shortener. This represents a significant milestone in the project's evolution, transforming it from a simple URL shortener into a full-featured SaaS platform.


✨ Major Features

🔐 Authentication & User Management System

JWT-Based Authentication

  • Stateless JWT token authentication with access and refresh tokens
  • Secure token rotation on refresh for enhanced security
  • HTTP-only cookies for token storage (SameSite=Lax for OAuth compatibility)
  • Token expiration and automatic refresh mechanisms
  • Secure password hashing using industry-standard algorithms

Multi-Provider OAuth Integration

  • Google OAuth - Sign in with Google accounts
  • GitHub OAuth - Developer-friendly GitHub login
  • Discord OAuth - Community-focused Discord authentication
  • Account linking capability - connect multiple OAuth providers to one account
  • Email verification requirement for OAuth account linking (prevents account takeover)
  • Secure CSRF state parameter handling with user ID embedding
  • Environment-aware OAuth redirect URL generation

User Registration & Login

  • Email/password registration with comprehensive password validation
  • Password strength indicator with real-time feedback
  • Duplicate email detection with proper error handling
  • Profile management with customizable profile pictures
  • User session management across devices
  • Password setting for OAuth-only users

🎯 API v1 - Complete REST API

URL Management API (/api/v1/urls)

  • List URLs - Paginated listing with filtering and sorting
  • Update URLs - Modify any URL property (long URL, alias, password, expiration, etc.)
  • Delete URLs - Permanent URL deletion with cache invalidation
  • Status Management - Toggle URL active/inactive status
  • Search & Filter - Search URLs by alias, URL, or metadata
  • Ownership Validation - Secure access control for all operations

URL Shortening API (/api/v1/shorten)

  • Create shortened URLs with authentication
  • Support for custom aliases
  • Password protection
  • Max click limits
  • Time-based expiration
  • Bot blocking configuration
  • Private statistics settings

Statistics API (/api/v1/stats)

  • Flexible Filtering - Filter by browser, OS, device, country, city, referrer, short_code
  • Custom Aggregation - Group by multiple dimensions (time, browser, OS, country, city, etc.)
  • Time Bucketing - Automatic intelligent time bucketing (hourly, daily, weekly, monthly)
  • Timezone Support - Format output in user's local timezone (IANA timezone support)
  • Multiple Metrics - Track clicks, unique clicks, average redirection time
  • Scope-Based Access - all scope for authenticated users, anon for public stats
  • Advanced Queries - Combine multiple filters for detailed analytics

API Key Management (/api/v1/keys)

  • Create API Keys - Generate API keys with custom scopes
  • Scope-Based Permissions:
    • shorten:create - Create shortened URLs
    • urls:manage - Full URL management (update, delete)
    • urls:read - Read-only access to URLs
    • stats:read - Access statistics
    • admin:all - Full administrative access
  • Key Listing - View all API keys with metadata
  • Key Revocation - Soft delete (revoke) or hard delete
  • Expiration Support - Set expiration dates for keys
  • Rate Limiting - 5 keys per hour creation limit, max 20 active keys
  • Secure Storage - Keys hashed with SHA-256, only shown once at creation

📊 Advanced Statistics & Analytics

Enhanced Statistics Dashboard

  • Interactive Charts - Click-to-filter functionality on all charts
  • Real-time Updates - Auto-refresh with configurable intervals
  • Multiple Visualizations:
    • Time-series line charts
    • Browser/OS/Device distribution pie charts
    • Geographic distribution maps (country-level)
    • Referrer analysis
    • City-level tracking
  • Table View - Detailed tabular data view with sorting
  • Export Functionality - Export stats in multiple formats (JSON, CSV, Excel, XML)

Advanced Date Range Picker

  • Pre-defined ranges (Today, Yesterday, Last 7/30/90 days, etc.)
  • Custom date range selection with calendar UI
  • Relative time format support ("1 hour ago", "2 days ago", etc.)
  • Timezone-aware date handling
  • Mobile-responsive bottom sheet design

Filter System

  • Multi-select Dropdowns - Filter by multiple values simultaneously
  • Dimension Filters - browser, OS, country, city, referrer, short_code
  • Real-time Updates - Charts update instantly on filter changes
  • Filter Indicators - Visual badges showing active filters
  • Clear Filters - One-click filter reset

Time Bucketing Strategy

  • Automatic Bucketing - Intelligently selects granularity based on date range
    • < 2 days: Hourly buckets
    • 2-60 days: Daily buckets
    • 60-365 days: Weekly buckets
    • 365 days: Monthly buckets

  • Timezone-Aware - Respects user's timezone for bucketing
  • Performance Optimized - Aggregation pipelines for fast queries

🎨 Modern Dashboard UI

URL Management Modal

  • Create New URLs - Enhanced modal with all options
  • Edit Existing URLs - Update any property inline
  • Status Badges - Visual indicators for URL status (active, inactive, expired, blocked)
  • Delete URL - New confirmation dialog based delete URL

Settings & Profile

  • Account Information - View and manage account details
  • OAuth Account Linking - Connect/disconnect OAuth providers
  • Password Management - Set or change password

Keys Management

  • API Key Dashboard - View all keys with metadata
  • Revoke/Delete - Manage key lifecycle
  • API Key Management - Create and manage API keys with scope selection
  • Two-tier Permissions - Access level + detailed scopes UI

🏗️ Architecture Improvements

Builder Pattern Implementation

  • StatsQueryBuilder - Builds complex statistics queries with validation
  • ShortenRequestBuilder - Validates and constructs URL shortening requests
  • UpdateUrlRequestBuilder - Handles URL updates with ownership validation
  • UrlListQueryBuilder - Constructs paginated URL listing queries
  • BaseBuilder - Abstract base class for consistency

Cache System Enhancements

  • Dual-Layer Cache - In-memory + Redis for optimal performance
  • Cache Stampede Prevention - Stale-while-revalidate pattern
  • Automatic Cache Invalidation - Invalidates on URL updates, deletes, or status changes
  • v2 URL Cache Support - New cache schema for v2 URLs
  • TTL Management - Configurable TTL with stale TTL for graceful degradation

v2 URL Schema

  • New Collection (urls_v2) - Improved schema design
  • Backward Compatibility - Seamless support for v1 URLs and emoji URLs
  • Enhanced Fields:
    • owner_id - User ownership tracking
    • status - ACTIVE, INACTIVE, BLOCKED, EXPIRED states
    • max_clicks - Automatic expiration on click limit
    • expire_after - Time-based expiration
    • block_bots - Bot blocking flag (enabled by default)
    • private_stats - Privacy control for statistics
    • password_hash - Hashed password storage
    • created_at, updated_at - Timestamp tracking
  • Time-Series Click Data - Separate collection for analytics with metadata

🔧 Rate Limiting & Security

Enhanced Rate Limiting

  • Dynamic Limits - Different limits for authenticated vs. anonymous users
  • Per-Endpoint Limits - Granular control over API endpoints
  • User-Based Limiting - Limits per user ID for authenticated requests
  • IP-Based Limiting - Falls back to IP for anonymous requests
  • Header Support - Respects X-Forwarded-For and Cloudflare headers

Security Improvements

  • CSRF Protection - State parameter validation in OAuth flows
  • Password Hashing - Bcrypt with proper salt rounds
  • SQL Injection Prevention - Parameterized queries and input sanitization
  • XSS Prevention - HTML escaping in templates
  • Secure Cookies - HTTP-only, SameSite, Secure flags
  • Token Rotation - Refresh tokens rotated on each use

🐛 Bug Fixes

Issue Resolutions

#84 - Social Media Link Preview Not Working

  • Fixed - Bots now redirected without logging for proper meta tag crawling
  • ✅ Enhanced bot detection with crawlerdetect library
  • ✅ Improved robots.txt for better SEO
  • ✅ Added X-Robots-Tag headers for proper indexing control

#83 - Fix Search Engine Indexing

  • Fixed - Updated robots.txt to allow all pages
  • ✅ Added proper sitemap.xml serving via SEO blueprint
  • ✅ Improved bot handling for SEO crawlers
  • ✅ Added meta tags for better indexing

#73 - Unique Click Counter Not Working

  • Fixed - Migrated to time-series collection for accurate unique click tracking
  • ✅ IP-based unique visitor detection
  • ✅ Proper aggregation pipelines for unique click counting
  • ✅ Backward compatible with legacy v1 URLs

#30 - Refactor Flask App to Follow Best Practices

  • Implemented - Complete blueprint-based architecture
  • ✅ Separated concerns (auth, api, dashboard, redirector, oauth, seo, stats)
  • ✅ Builder pattern for complex operations
  • ✅ Utility modules for reusable logic
  • ✅ Proper error handling with custom exception classes

#17 - Enhancement: Revamp the Stats Page

  • Completely Redesigned - Modern, interactive statistics dashboard
  • ✅ Interactive charts with click-to-filter
  • ✅ Advanced filtering and grouping
  • ✅ Time-series visualizations
  • ✅ Geographic maps
  • ✅ Mobile-responsive design

#15 - Future Plans: Implement Login System

  • Fully Implemented - Complete authentication system
  • ✅ JWT-based auth with refresh tokens
  • ✅ Multi-provider OAuth (Google, GitHub, Discord)
  • ✅ User dashboard and profile management
  • ✅ API key system with scopes

#11 - Feature: Add Time Expiration for URLs

  • Implemented - Time-based expiration support
  • ✅ Custom expiration dates (ISO8601 or Unix timestamp)
  • ✅ Relative time formats ("1 hour", "2 days", etc.)
  • ✅ Automatic expiration on max clicks reached
  • ✅ Status tracking (ACTIVE, EXPIRED)

Additional Bug Fixes

  • ✅ Fixed table header showing when no keys exist
  • ✅ Fixed rate limiting behind Cloudflare proxy
  • ✅ Fixed IP address retrieval using X-Forwarded-For headers

🆕 Additional Improvements

Bot Detection & Blocking

  • Enhanced bot detection using crawlerdetect library
  • Updated bot user agents list (15+ new bots)
  • Bot blocking enabled by default for new URLs
  • Bot name tracking in analytics
  • SEO-friendly bot handling (redirect without logging)

URL Search API

  • Search URLs by alias, long URL, or metadata
  • Pagination support
  • Filtering by status, owner, date range
  • Sorting by creation date, clicks, etc.

Password Protection Enhancements

  • Password-protected URLs with bcrypt hashing (v2)
  • Backward compatible with plaintext passwords (v1)
  • Secure password validation

Performance Optimizations

  • MongoDB aggregation pipelines for fast queries
  • Redis caching with stale-while-revalidate
  • In-memory cache for frequently accessed URLs
  • Optimized database indexes
  • Reduced code redundancy through builder classes

Developer Experience

  • Comprehensive inline documentation
  • Type hints throughout codebase
  • Modular utility functions
  • Clean separation of concerns

Infrastructure

  • Docker Compose configuration for MongoDB and Redis
  • Updated Dockerfile with optimized builds
  • Environment variable management

🔄 Migration Guide

For Existing Users

  • Full Backward Compatibility - All existing v1 URLs continue to work
  • No Breaking Changes - Redirections work seamlessly
  • Legacy Support - v1 emoji URLs and standard URLs fully supported

For API Users

  • 🆕 New API v1 Endpoints - /api/v1/* for new features
  • Old API Still Works - /shorten and /stats endpoints unchanged for anonymous users
  • 🔑 API Keys Optional - Can still use without authentication (with limits)

Database Collections

  • users - User accounts (new)
  • api_keys - API key management (new)
  • urls_v2 - New URL schema (new)
  • click_data - Time-series click analytics (new)
  • urls - Legacy v1 URLs (existing)
  • emojies - Emoji URLs (existing)

This PR represents over 2 months of development work and is the largest update to the spoo.me platform since its inception. We're excited to bring these features to our users! 🎉

Summary by CodeRabbit

  • New Features

    • V1 API (shorten, manage, list, stats, export) and a full dashboard (Links, API Keys, Stats, Billing, URL management).
  • Security & Authentication

    • JWT-based sessions, full auth flows (login/refresh/logout), OAuth (Google/GitHub/Discord), email verification, API key creation/revocation, client password-strength UI.
  • Infrastructure

    • Sentry integration, Redis/Mongo support and improved caching, refined rate-limits, updated CI/workflows, refreshed Docker/dev compose.
  • UX & UI

    • New dashboard styles, auth modal, date-range picker, QR/recent-URLs, copy/share, accessibility improvements.

Zingzy added 30 commits August 15, 2025 15:52
- Introduced user authentication with login, registration, and logout functionalities.
- Implemented JWT-based access and refresh token management.
- Added user profile retrieval and dashboard view.
- Updated main application to include the new auth blueprint and ensure MongoDB indexes for users and refresh tokens.
- Enhanced frontend with a dashboard template and authentication modal for user interactions.
- Updated dependencies to include argon2-cffi for password hashing and pyjwt for JWT handling.
- Updated the logout process to ensure both access and refresh cookies are cleared.
- Modified the `revoke_refresh_token` function to support hard deletion of tokens.
- Improved client-side logout handling to update navigation state after logout.
- Converted multiple HTML files to utilize Jinja2 templating with blocks for title, meta, CSS, and scripts.
- Improved accessibility by adding alt attributes to images.
- Streamlined the structure of the templates for consistency and easier updates in the future.
- Added JWT configuration options to the .env.example file, including issuer, audience, and token expiration settings.
- Refactored authentication utility functions for better readability and consistency.
- Improved error handling in user retrieval functions and ensured proper cookie management for access and refresh tokens.
- Updated the auth blueprint to maintain consistent formatting and structure.
…nd organization

- Moved authentication modal styles to a dedicated CSS file for better separation of concerns.
- Updated the modal structure to enhance accessibility with ARIA attributes.
- Added email input type to the form and improved autocomplete attributes for better user experience.
- Changed the navigation link text from "Login / Register" to "Sign Up" for clarity.
- Registered a new API version blueprint for enhanced URL management.
- Introduced a new MongoDB collection for versioned URLs and created indexes for efficient querying.
- Added utility functions for inserting and retrieving URLs in the new versioned format.
- Implemented a new short code generator function for versioned URLs to support varying lengths.
- Updated the shorten API to limit custom alias length to 16 characters for better consistency.
- Refactored the index template to dynamically render recent URLs from localStorage instead of using cookies.
- Improved JavaScript to handle form submissions via the API and maintain recent URLs in localStorage.
- Simplified QR code generation and button interactions for better user experience.
- Introduced API key handling in the shorten API, allowing for token-based authentication.
- Added new utility functions for managing API keys in MongoDB, including insertion, retrieval, and revocation.
- Updated the dashboard to include a new route for displaying API keys.
- Enhanced the navbar to provide access to the API keys dashboard.
- Ensured proper validation of API key scopes during URL shortening requests.
- Updated the dashboard and API keys templates to enhance the layout with a new structure and styling.
- Introduced a gradient surface and page header for better visual hierarchy.
- Reorganized form elements for creating API keys into a grid layout for improved usability.
- Enhanced button styles for better consistency across the application.
- Removed inline styles and replaced them with external CSS for better maintainability.
…d usability

- Replaced the refresh token management with stateless JWTs for enhanced security.
- Updated authentication utilities to streamline token generation and verification processes.
- Improved error handling for token verification and added support for token rotation.
- Refactored the login and refresh functions to utilize the new JWT approach.
- Cleaned up unused refresh token functions and collections from the database.
- Enhanced cookie management for access and refresh tokens to ensure proper handling across routes.
…, and improve dashboard styling

- Changed the environment variable from SECRET_KEY to FLASK_SECRET_KEY for clarity.
- Integrated the FLASK_SECRET_KEY into the Flask app for improved security.
- Enhanced dashboard CSS for better layout and user experience, including updated margins, hover effects, and responsive design adjustments.
- Refactored HTML structure in the dashboard keys template for improved readability and styling consistency.
- Introduced a new modal for displaying the generated API key, ensuring users can easily copy it.
- Added scoped CSS styles for the key creation modal to improve layout and user interaction.
- Updated JavaScript functions to handle modal display and clipboard copying of the API key.
- Refactored the key creation process to enhance usability and security messaging for users.
- Added profile dropdown functionality to the desktop and mobile navbars for user account access.
- Updated CSS styles for the navbar and mobile header to accommodate new profile elements.
- Refactored JavaScript to manage profile dropdown toggling and user authentication state.
- Improved mobile navigation by integrating a profile button and dropdown for better user experience.
… readability and maintainability

- Introduced ShortenRequestBuilder class to encapsulate request handling and validation logic for URL shortening.
- Enhanced rate limiting by implementing dynamic limits based on user authentication status.
- Updated the shorten API endpoint to utilize the new builder for processing requests, improving code organization and clarity.
- Added validation for emoji aliases and improved error handling for various input scenarios.
- Registered the new 'urls' endpoint in the API v1 blueprint for better route management.
- Added 'urls:read' scope to the allowed scopes in the keys module to support new URL functionalities.
- Enhanced MongoDB index creation by adding a composite index on 'owner_id' and 'created_at' for optimized querying.
… for compatibility

- Updated the dashboard layout with a new sidebar and toolbar for improved navigation and filtering options.
- Enhanced CSS styles for better visual consistency and responsiveness across different screen sizes.
- Added a legacy URL shortening route for backwards compatibility, with a note to deprecate it in the future.
…onsistency

- Updated MongoDB connection messages to include a consistent prefix for better logging.
- Enhanced Redis connection success message for clarity.
- Refactored CSS styles in the dashboard for improved layout and responsiveness, including adjustments to padding and max-width properties.
- Improved JavaScript formatting and added options dropdown functionality for better user experience in the dashboard.
- Added segmented control UI elements for password and max clicks filters, replacing traditional select inputs for a more interactive experience.
- Enhanced JavaScript functionality to manage segmented control states and reset visual indicators on filter application.
- Updated CSS styles for segmented controls to improve layout and responsiveness, including adjustments to gaps and padding in the dashboard.
…ed rendering

- Added new routes for dashboard links, statistics, and settings, providing users with dedicated views for each section.
- Implemented a redirect to the links page as the default dashboard view for better user navigation.
- Updated rendering logic in the dashboard to utilize new templates for links, keys, and statistics, enhancing visual organization.
- Improved JavaScript functionality for better handling of empty states and pagination in the dashboard.
…reduce code duplication and increase modularity

- Added management module to the API v1 blueprint for better organization.
- Removed unused imports and classes from shorten.py and urls.py to streamline the codebase.
- Introduced UrlListQueryBuilder and ShortenRequestBuilder for enhanced request handling and validation.
- Updated rate limiting for the URLs endpoint to require authentication, improving security.
- Added a URL management modal for editing short URL settings, including alias, destination URL, password protection, and expiration settings.
- Introduced new status badges for active and inactive links, improving visual feedback on link status.
- Updated CSS for badge styling and modal layout, ensuring a cohesive design.
- Enhanced JavaScript functionality to manage badge visibility based on link status and to handle modal interactions.
…py button

- Updated CSS styles for link display and result sections, including margin adjustments and border radius changes.
- Added a QR code block in the success modal for easy access to generated links.
- Improved copy button functionality with clipboard support and visual feedback on copy action.
- Enhanced JavaScript to handle QR code generation and download functionality.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (3)
templates/dashboard/base.html (1)

64-77: Redirect loop risk in authFetch on 401 (previously flagged, still unresolved).

This issue was already identified in a previous review: Line 74 redirects to / on 401 responses, which can cause a redirect loop if the home page requires authentication or if the user navigates to a dashboard page from the login flow.

Consider adopting one of these approaches to prevent loops:

Option 1: Redirect to a dedicated login page (recommended)

if (response.status === 401) {
-    window.location.href = '/';
+    window.location.href = '/login';
}

Option 2: Check current location before redirecting

if (response.status === 401) {
-    window.location.href = '/';
+    if (!window.location.pathname.startsWith('/login')) {
+        window.location.href = '/login?redirect=' + encodeURIComponent(window.location.pathname);
+    }
}
main.py (2)

34-35: Lock down credentialed CORS to trusted origins.

This configuration still allows any origin to make credentialed requests, enabling any website to act on behalf of logged-in users. This was previously flagged as a critical security issue.

Please implement the suggested fix from the previous review to restrict CORS to a trusted origins allowlist.


106-111: Debug mode defaults to enabled when ENV is unset.

The condition debug=os.getenv("ENV") != "production" enables debug mode whenever the ENV variable is not set or set to any value other than "production". This is risky because:

  • If ENV is not configured, debug mode activates by default, exposing Flask's Werkzeug debugger which allows arbitrary code execution.
  • A previous security finding noted debug mode risks, but this logic still defaults to enabled.

Apply this diff to default debug mode to off and explicitly enable it only for development:

 if __name__ == "__main__":
+    env = os.getenv("ENV", "production")
     app.run(
         host="0.0.0.0",
         port=8000,
-        use_reloader=os.getenv("ENV") != "production",
-        debug=os.getenv("ENV") != "production",
+        use_reloader=env == "development",
+        debug=env == "development",
     )

Note: The host="0.0.0.0" binding is acceptable in containerized environments when behind a reverse proxy (as indicated by the Docker updates in this PR).

🧹 Nitpick comments (4)
templates/dashboard/base.html (3)

15-17: Inconsistent cache-busting version numbers on assets.

Assets use different version query strings: v=1 (dashboard-base.css, dashboard-base.js), v=3 (customNotification.js), and v=5 (customNotification.css). This is unclear—are these intentional version mismatches, or should they be synchronized to ensure all assets are invalidated together during deployments?

Clarify the versioning strategy: if each asset has an independent version, document why; if they should move together, align them to a single counter or use a build-time hash.

Also applies to: 59-60


31-31: Sentry loaded unconditionally; verify environment awareness.

The Sentry session replay script (line 31) is loaded for all environments. Confirm this is intentional or gate it behind an environment check (e.g., only in production):

-    <script src="https://js.sentry-cdn.com/846878c0ca155c98146ef71f12143e3e.min.js" crossorigin="anonymous"></script>
+    {% if env.ENVIRONMENT == 'production' %}
+    <script src="https://js.sentry-cdn.com/846878c0ca155c98146ef71f12143e3e.min.js" crossorigin="anonymous"></script>
+    {% endif %}

36-47: Mobile hamburger menu toggle has no keyboard trap or focus management.

The hamburger menu button (line 40) toggles a menu but there's no visible indication of focus state or mechanism to trap focus within the menu when open. If the sidebar is truly a modal-like overlay on mobile, implement focus trapping and ensure the menu can be closed via Escape key.

Verify the linked dashboard-base.js (line 60) handles these interactions; if not, this is an accessibility gap.

main.py (1)

51-51: Consider logging index creation failures.

The ensure_indexes() call at startup silently catches and ignores all exceptions. While some failures are expected (e.g., collections already existing), other issues like connection failures or permission errors could go unnoticed, potentially causing performance degradation or query failures later.

Consider adding selective logging to the ensure_indexes function to distinguish between expected and unexpected failures, helping surface configuration issues during deployment.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 97a5c39 and dffb3af.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (5)
  • main.py (4 hunks)
  • pyproject.toml (3 hunks)
  • requirements.txt (1 hunks)
  • templates/base.html (1 hunks)
  • templates/dashboard/base.html (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • templates/base.html
🧰 Additional context used
🧬 Code graph analysis (1)
main.py (2)
blueprints/oauth.py (1)
  • init_oauth_for_app (42-46)
utils/mongo_utils.py (1)
  • ensure_indexes (382-440)
🪛 ast-grep (0.39.9)
main.py

[warning] 105-110: Running flask app with host 0.0.0.0 could expose the server publicly.
Context: app.run(
host="0.0.0.0",
port=8000,
use_reloader=os.getenv("ENV") != "production",
debug=os.getenv("ENV") != "production",
)
Note: [CWE-668]: Exposure of Resource to Wrong Sphere [OWASP A01:2021]: Broken Access Control [REFERENCES]
https://owasp.org/Top10/A01_2021-Broken_Access_Control

(avoid_app_run_with_bad_host-python)

🔇 Additional comments (4)
templates/dashboard/base.html (1)

64-77: The review comment is incorrect. CSRF protection is already properly implemented via SameSite cookies.

The application uses a modern JWT-based authentication system with HttpOnly cookies configured with SameSite="Lax", which provides automatic CSRF protection. Explicit CSRF tokens (X-CSRF-Token headers) are not required—SameSite cookies handle this protection by restricting cookie transmission to same-site requests. The credentials: 'include' in fetch calls, combined with the SameSite policy, prevents cross-site request forgery attacks without needing a separate token mechanism.

Likely an incorrect or invalid review comment.

requirements.txt (2)

7-7: ✓ authlib updated to 1.6.5 (resolves known vulnerabilities).

Line 7 correctly pins authlib to 1.6.5, addressing the three HIGH severity vulnerabilities in 1.6.3 (JWT/JWS RFC bypass, DoS via oversized JOSE segments, JWE decompression bomb). This resolves the critical issue flagged in the prior review.


48-48: pyjwt 2.10.1 contains CVE-2025-45768 (weak encryption)—no patched version released yet.

CVE-2025-45768 affects pyjwt 2.10.1 with a reported weak encryption vulnerability (CVSS ~7.0, High severity). However, no PyJWT release currently addresses this issue; PyPI's latest published version remains 2.10.1.

Recommended actions while awaiting an upstream patch:

  • Enforce strong cryptographic key lengths and algorithms (RSA/ECDSA/EdDSA with recommended key sizes; avoid weak HMAC keys) at the application level.
  • Monitor the PyJWT GitHub repository for security advisories and patch releases.
  • If JWT functionality is critical, evaluate alternate maintained libraries pending upstream fixes.

This dependency requires manual verification and mitigation strategy before merging.

pyproject.toml (1)

21-21: CVE-2025-45768 is confirmed in pyjwt 2.10.1, but no patched version exists yet.

As of November 11, 2025, no published patched PyJWT release exists for CVE-2025-45768—both GitHub Advisory and NVD list the patched version as "Unknown". The vulnerability ("weak encryption") in pyjwt 2.10.1 is real, but you cannot resolve this by updating to a newer version because none has been released. Monitor the PyJWT repository and security advisories for a future patch, and coordinate with your security team on risk acceptance or alternative mitigations if needed.

Comment thread main.py
Comment thread main.py
Comment thread pyproject.toml Outdated
Comment thread requirements.txt
Comment thread templates/dashboard/base.html
- Added a centralized logging configuration with structured logging using structlog.
- Introduced a logger utility for consistent logging practices.
- Replaced print statements with appropriate logging calls in various modules.
- Enhanced error handling with detailed logging for exceptions.
- Integrated request logging middleware to capture request/response details.
- Updated dependencies to include structlog for structured logging support.
- Improved cache operations logging for better traceability.
- Added logging for OAuth provider initialization and user actions.
- Enhanced MongoDB connection logging and error handling.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
blueprints/redirector.py (1)

34-43: Move imports to the top of the file.

Module-level imports should appear before other statements. Lines 34-39 contain imports that come after blueprint setup (line 44), violating PEP 8 and causing E402 linting errors.

Move these imports to the top of the file (after line 28):

 from cache import cache_query as cq
 from cache.cache_url import UrlCacheData
 
-from .limiter import limiter
-
-log = get_logger(__name__)
-
 from ua_parser import parse
 from datetime import datetime, timezone
 from urllib.parse import unquote
 import re
 import tldextract
 from crawlerdetect import CrawlerDetect
 
+from .limiter import limiter
+
+log = get_logger(__name__)
+
 crawler_detect = CrawlerDetect()
 tld_no_cache_extract = tldextract.TLDExtract(cache_dir=None)
♻️ Duplicate comments (12)
blueprints/oauth.py (1)

59-66: [DUPLICATE] Critical: OAuth state parameter still lacks cryptographic integrity protection.

The state parameter generated by generate_oauth_state() remains unsigned (constructed as plain URL query parameters), despite past review flagging this. The TODO comment in utils/oauth_utils.py (line 147) confirms signing is still pending. An attacker can forge state/code pairs to mount CSRF attacks during OAuth login and account linking flows.

The fix requires either:

  1. HMAC/JWT-signing the entire state payload with a server-side secret, or
  2. Server-side nonce storage with callback validation
builders/base.py (1)

158-181: [DUPLICATE] Handle trailing "Z" in ISO 8601 timestamp parsing.

datetime.fromisoformat() at line 167 rejects ISO 8601 timestamps ending with "Z" (e.g., "2025-11-10T05:00:00Z"), causing valid RFC-compliant inputs to fail with a 400 error. Normalize the "Z" suffix to "+00:00" before parsing.

Apply this fix:

         try:
             if isinstance(expire_after, (int, float)):
                 self.expire_ts = int(expire_after)
             else:
-                dt = datetime.fromisoformat(str(expire_after))
+                raw = str(expire_after)
+                if raw.endswith("Z"):
+                    raw = raw[:-1] + "+00:00"
+                dt = datetime.fromisoformat(raw)
                 if dt.tzinfo is None:
                     dt = dt.replace(tzinfo=timezone.utc)
                 self.expire_ts = int(dt.timestamp())
builders/update.py (1)

60-69: Add None check to prevent crash when URL is missing.

If load_and_validate_ownership() fails (e.g., URL not found), self.existing_doc remains None, but validate_alias_custom() still executes due to the builder pattern. Line 65 attempts self.existing_doc.get("alias") without checking for None, raising AttributeError and turning the expected 404 into a 500.

Apply this fix:

 def validate_alias_custom(self) -> "UpdateUrlRequestBuilder":
     """Provides custom validation for alias updates"""
+    if self.error:
+        return self
+        
     if "alias" in self.payload:
         alias_value = self.payload.get("alias")
         # Treat same alias as no-op (idempotent update)
-        if alias_value == self.existing_doc.get("alias"):
+        if self.existing_doc and alias_value == self.existing_doc.get("alias"):
             return self

     # use parent validation logic for changed values
     return self.validate_alias()
main.py (3)

36-38: Require FLASK_SECRET_KEY before serving.

Right now the app runs with Flask’s default secret when FLASK_SECRET_KEY is missing, which breaks sessions across restarts and undermines CSRF protections. We need to fail fast instead of silently proceeding.

Apply this diff:

-flask_secret = os.getenv("FLASK_SECRET_KEY")
-if flask_secret:
-    app.secret_key = flask_secret
+flask_secret = os.getenv("FLASK_SECRET_KEY")
+if not flask_secret:
+    raise RuntimeError("FLASK_SECRET_KEY environment variable must be set")
+app.secret_key = flask_secret

41-41: Lock down credentialed CORS.

Enabling credentials without an explicit origin allowlist lets any site replay our users’ cookies—a serious CSRF/security risk. Please restrict credentialed access to trusted domains and keep everything else non-credentialed.

Apply this diff:

-# Enable credentials so refresh cookies can be sent cross-origin from frontend
-CORS(app, supports_credentials=True)
+allowed_origins = os.getenv("DASHBOARD_ALLOWED_ORIGINS", "")
+origin_list = [origin.strip() for origin in allowed_origins.split(",") if origin.strip()]
+if origin_list:
+    CORS(app, supports_credentials=True, origins=origin_list)
+else:
+    CORS(app, supports_credentials=False)

51-57: Make Sentry PII and sampling configurable.

Forcing send_default_pii=True and 100% sampling will leak user data and burn quota in production. Please gate these settings behind env vars with safe defaults.

Apply this diff:

 if os.getenv("SENTRY_DSN"):
     sentry_sdk.init(
         dsn=os.getenv("SENTRY_DSN"),
-        send_default_pii=True,
-        traces_sample_rate=1.0,
+        send_default_pii=os.getenv("SENTRY_SEND_PII", "false").lower() == "true",
+        traces_sample_rate=float(os.getenv("SENTRY_TRACES_SAMPLE_RATE", "0.1")),
         enable_logs=True,
-        profile_session_sample_rate=1.0,
+        profile_session_sample_rate=float(
+            os.getenv("SENTRY_PROFILE_SAMPLE_RATE", "0.1")
+        ),
         profile_lifecycle="trace",
     )
api/v1/keys.py (3)

31-42: Handle ISO 8601 Z timestamps in _parse_expires_at.

Clients commonly send timestamps ending with Z (e.g., 2025-01-01T00:00:00Z), but datetime.fromisoformat() rejects this suffix. Normalize Z to +00:00 before parsing to avoid silently dropping valid expiration dates.

Apply this diff:

 def _parse_expires_at(value: Optional[str | int | float]):
     if value is None:
         return None
     try:
         if isinstance(value, (int, float)):
             return datetime.fromtimestamp(int(value), tz=timezone.utc)
-        dt = datetime.fromisoformat(str(value))
+        raw = str(value)
+        if raw.endswith("Z"):
+            raw = raw[:-1] + "+00:00"
+        dt = datetime.fromisoformat(raw)
         if dt.tzinfo is None:
             dt = dt.replace(tzinfo=timezone.utc)
         return dt.astimezone(timezone.utc)
     except Exception:
         return None

Based on learnings


138-141: Don't call .strip() on untrusted non-strings.

If a client sends name or description as a non-string (e.g., 123), the (body.get(...) or "").strip() expression raises AttributeError, returning a 500 instead of the intended 400. Guard the type before stripping.

Apply this diff:

     body = request.get_json(silent=True) or {}
-    name = (body.get("name") or "").strip()
-    description = (body.get("description") or "").strip() or None
+    name_raw = body.get("name")
+    if not isinstance(name_raw, str):
+        return jsonify({"error": "name must be a string"}), 400
+    name = name_raw.strip()
+    
+    description_raw = body.get("description")
+    if description_raw is not None and not isinstance(description_raw, str):
+        return jsonify({"error": "description must be a string"}), 400
+    description = description_raw.strip() if isinstance(description_raw, str) else None
     scopes = body.get("scopes") or []
     expires_at_raw = body.get("expires_at")

Based on learnings


297-314: Normalize datetimes before calling .timestamp().

MongoDB may return naive UTC datetimes for created_at/expires_at. Calling .timestamp() on naive datetimes interprets them in the server's local timezone, shifting results. Attach UTC if tzinfo is missing before computing the epoch.

Apply this diff:

     keys = list_api_keys_by_user(g.user_id)
     result = []
     for k in keys:
+        created_at = k.get("created_at")
+        if created_at and created_at.tzinfo is None:
+            created_at = created_at.replace(tzinfo=timezone.utc)
+        
+        expires_at = k.get("expires_at")
+        if expires_at and expires_at.tzinfo is None:
+            expires_at = expires_at.replace(tzinfo=timezone.utc)
+        
         result.append(
             {
                 "id": str(k["_id"]),
                 "name": k.get("name"),
                 "description": k.get("description"),
                 "scopes": k.get("scopes", []),
-                "created_at": int(k.get("created_at").timestamp())
-                if k.get("created_at")
-                else None,
-                "expires_at": int(k.get("expires_at").timestamp())
-                if k.get("expires_at")
-                else None,
+                "created_at": int(created_at.timestamp()) if created_at else None,
+                "expires_at": int(expires_at.timestamp()) if expires_at else None,
                 "revoked": bool(k.get("revoked", False)),
                 "token_prefix": k.get("token_prefix"),
             }
         )

Based on learnings

builders/stats.py (2)

65-76: Support Z timestamps in _parse_datetime.

Timestamps like 2025-01-01T00:00:00Z return None because fromisoformat() rejects the Z suffix. Normalize Z to +00:00 before parsing so user-specified date ranges work correctly.

Apply this diff:

     def _parse_datetime(self, value: Any) -> Optional[datetime]:
         if value is None:
             return None
         try:
             if isinstance(value, (int, float)):
                 return datetime.fromtimestamp(int(value), tz=timezone.utc)
-            dt = datetime.fromisoformat(str(value))
+            raw = str(value)
+            if raw.endswith("Z"):
+                raw = raw[:-1] + "+00:00"
+            dt = datetime.fromisoformat(raw)
             if dt.tzinfo is None:
                 dt = dt.replace(tzinfo=timezone.utc)
             return dt.astimezone(timezone.utc)
         except Exception:
             return None

Based on learnings


328-355: Return error immediately instead of self when query construction fails.

The method signature declares -> Dict[str, Any] but line 355 returns self._fail(...) (which is self). The caller treats this as a dict, causing downstream errors. Set self.error and return an empty dict, or re-raise the exception.

Apply this diff:

         except Exception as e:
             log.error(
                 "stats_query_build_failed",
                 scope=self.scope,
                 error=str(e),
                 error_type=type(e).__name__,
             )
-            return self._fail({"error": "failed to build query"}, 500)
+            self._fail({"error": "failed to build query"}, 500)
+            return {}

And update build() to check for errors before using the query:

         try:
             # Build query
             query = self._build_click_query()
+            if self.error is not None:
+                return self.error
             if not query and self.scope == "anon":

Based on learnings

api/v1/management.py (1)

59-61: Update documentation to match validation logic.

The docstring states "Set to null or 0 to remove limit," but based on previous discussions, the validation rejects 0. Please update the documentation to reflect that only null removes the limit, not 0.

Apply this diff:

-    - **max_clicks** (integer | null): Update or remove click limit
-      - Must be positive integer to set
-      - Set to `null` or 0 to remove limit
+    - **max_clicks** (integer | null): Update or remove click limit
+      - Must be positive integer to set
+      - Set to `null` to remove limit

Based on learnings

🧹 Nitpick comments (5)
builders/create.py (3)

18-29: Consider adding a retry limit to the alias generation loop.

While collision probability is extremely low (1 in 62^7 ≈ 3.5 trillion), adding a bounded retry loop (e.g., max 10 attempts) would prevent indefinite hangs in case of bugs in check_if_v2_alias_exists or generate_short_code_v2.

Apply this diff:

     def validate_or_generate_alias(self) -> "ShortenRequestBuilder":
         # Try alias path if provided
         custom_alias = self.payload.get("alias")
         if custom_alias:
             return self.validate_alias()
         # Otherwise generate
-        while True:
+        max_attempts = 10
+        for _ in range(max_attempts):
             candidate = generate_short_code_v2(7)
             if not check_if_v2_alias_exists(candidate):
                 self.alias = candidate
-                break
+                return self
-        return self
+        # Extremely unlikely to reach here (1 in trillions per attempt)
+        return self._fail({"error": "failed to generate unique alias"}, 500)

48-48: Simplify redundant conditional expression.

The expression self.block_bots if self.block_bots is not None else None is equivalent to just self.block_bots.

Apply this diff:

-            "block_bots": self.block_bots if self.block_bots is not None else None,
+            "block_bots": self.block_bots,

57-81: Handle user-supplied alias collisions with proper HTTP semantics.

The generic exception handler returns 500 for all database errors, including duplicate key errors. When a user explicitly requests an alias that already exists, the response should be 409 Conflict, not 500 Internal Server Error. While the validate_or_generate_alias pre-check reduces this risk, a race condition window remains between validation and insertion.

Import and handle DuplicateKeyError explicitly:

 from flask import request, jsonify, Response
+from pymongo.errors import DuplicateKeyError

 from utils.url_utils import (
     generate_short_code_v2,

Then update the exception handling in build:

         try:
             collection.insert_one(doc)
 
             log.info(
                 "url_created",
                 alias=self.alias,
                 long_url=self.long_url,
                 owner_id=str(self.owner_id) if self.owner_id else None,
                 schema="v2",
                 has_password=bool(self.password_hash),
                 max_clicks=self.max_clicks,
                 block_bots=self.block_bots,
                 has_expiration=bool(self.expire_ts),
                 private_stats=self.private_stats,
             )
+        except DuplicateKeyError:
+            log.warning(
+                "url_creation_failed",
+                reason="duplicate_alias",
+                alias=self.alias,
+                schema="v2",
+            )
+            # User-supplied alias collision
+            if self.payload.get("alias"):
+                return jsonify({"error": "alias already exists"}), 409
+            # Auto-generated collision (extremely rare) - could retry here if desired
+            return jsonify({"error": "alias conflict, please retry"}), 409
         except Exception as e:
             log.error(
                 "url_creation_failed",
                 reason="database_error",
                 alias=self.alias,
                 schema="v2",
                 error=str(e),
                 error_type=type(e).__name__,
             )
             return jsonify({"error": "database error"}), 500
cache/cache_url.py (1)

13-18: Emit the deprecation warning on use, not import.

Calling warnings.warn in the class body fires immediately on import, so every module load emits a DeprecationWarning even if nobody instantiates UrlData. Please move the warning into __post_init__ so it only triggers when the legacy type is actually used.

Apply this diff:

 @dataclass
 class UrlData:
-    warnings.warn(
-        "[UrlCache] UrlData is deprecated, use UrlCacheData instead",
-        DeprecationWarning,
-        stacklevel=2,
-    )
     url: str
     short_code: str
     password: Optional[str]
     block_bots: bool
+
+    def __post_init__(self):
+        warnings.warn(
+            "[UrlCache] UrlData is deprecated, use UrlCacheData instead",
+            DeprecationWarning,
+            stacklevel=2,
+        )
blueprints/redirector.py (1)

567-567: Cache optimization opportunity.

The TODO comment indicates that URL data should be fetched from cache for better performance. This would reduce database load for password-protected URLs.

Would you like me to generate a patch to implement cache lookup similar to the redirect_url endpoint?

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dffb3af and 1b5f6bd.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (26)
  • .env.example (1 hunks)
  • LOGGING_IMPLEMENTATION.md (1 hunks)
  • api/v1/keys.py (1 hunks)
  • api/v1/management.py (1 hunks)
  • blueprints/auth.py (1 hunks)
  • blueprints/contact.py (3 hunks)
  • blueprints/dashboard.py (1 hunks)
  • blueprints/limiter.py (2 hunks)
  • blueprints/oauth.py (1 hunks)
  • blueprints/redirector.py (4 hunks)
  • blueprints/stats.py (11 hunks)
  • blueprints/url_shortener.py (8 hunks)
  • builders/base.py (1 hunks)
  • builders/create.py (1 hunks)
  • builders/exports.py (1 hunks)
  • builders/query.py (1 hunks)
  • builders/stats.py (1 hunks)
  • builders/update.py (1 hunks)
  • cache/__init__.py (1 hunks)
  • cache/base_cache.py (1 hunks)
  • cache/cache_url.py (2 hunks)
  • cache/dual_cache.py (2 hunks)
  • cache/redis_client.py (2 hunks)
  • main.py (5 hunks)
  • pyproject.toml (3 hunks)
  • requirements.txt (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • builders/query.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-10T06:17:02.860Z
Learnt from: Zingzy
Repo: spoo-me/url-shortener PR: 94
File: templates/base.html:7-7
Timestamp: 2025-11-10T06:17:02.860Z
Learning: In the spoo.me URL shortener project, hardcoded production URLs are preferred over Flask's `url_for()` helper for performance reasons, particularly in frequently-rendered templates like base.html. Avoid suggesting `url_for()` replacements for static asset URLs.

Applied to files:

  • blueprints/url_shortener.py
🧬 Code graph analysis (21)
blueprints/contact.py (1)
utils/logger.py (1)
  • get_logger (19-34)
cache/__init__.py (1)
cache/dual_cache.py (1)
  • DualCache (11-83)
builders/create.py (4)
utils/url_utils.py (3)
  • generate_short_code_v2 (129-131)
  • get_client_ip (48-66)
  • validate_alias (134-136)
utils/mongo_utils.py (1)
  • check_if_v2_alias_exists (188-193)
utils/logger.py (1)
  • get_logger (19-34)
builders/base.py (4)
  • BaseUrlRequestBuilder (22-204)
  • validate_alias (90-112)
  • _fail (41-43)
  • _ensure_owner_object_id (193-204)
builders/update.py (3)
utils/logger.py (1)
  • get_logger (19-34)
builders/base.py (5)
  • BaseUrlRequestBuilder (22-204)
  • _fail (41-43)
  • _ensure_owner_object_id (193-204)
  • validate_long_url (63-88)
  • validate_alias (90-112)
cache/cache_url.py (1)
  • invalidate_url_cache (85-103)
api/v1/management.py (5)
blueprints/limiter.py (2)
  • dynamic_limit_for_request (30-43)
  • rate_limit_key_for_request (46-56)
utils/logger.py (1)
  • get_logger (19-34)
builders/update.py (6)
  • UpdateUrlRequestBuilder (14-197)
  • load_and_validate_ownership (22-50)
  • validate_long_url_if_present (52-58)
  • validate_alias_custom (60-69)
  • build_update (82-197)
  • parse_status_change (71-80)
builders/base.py (6)
  • parse_auth_scope (45-61)
  • validate_password (114-129)
  • parse_max_clicks (139-156)
  • parse_expire_after (158-181)
  • parse_block_bots (131-137)
  • parse_private_stats (183-191)
cache/cache_url.py (1)
  • invalidate_url_cache (85-103)
blueprints/url_shortener.py (3)
utils/mongo_utils.py (3)
  • get_url_v2_by_alias (181-185)
  • load_emoji_url (75-80)
  • load_url (35-40)
utils/logger.py (1)
  • get_logger (19-34)
utils/url_utils.py (1)
  • validate_emoji_alias (143-150)
blueprints/stats.py (1)
utils/logger.py (1)
  • get_logger (19-34)
builders/base.py (4)
utils/url_utils.py (3)
  • validate_url (89-93)
  • validate_alias (134-136)
  • validate_password (69-86)
utils/mongo_utils.py (3)
  • check_if_v2_alias_exists (188-193)
  • check_if_slug_exists (66-72)
  • validate_blocked_url (114-122)
utils/auth_utils.py (2)
  • hash_password (51-52)
  • resolve_owner_id_from_request (276-361)
utils/logger.py (1)
  • get_logger (19-34)
blueprints/oauth.py (4)
utils/logger.py (1)
  • get_logger (19-34)
utils/auth_utils.py (5)
  • generate_access_jwt (63-76)
  • generate_refresh_jwt (88-103)
  • set_refresh_cookie (120-132)
  • set_access_cookie (149-161)
  • requires_auth (178-249)
utils/mongo_utils.py (2)
  • get_user_by_email (125-130)
  • get_user_by_id (148-153)
utils/oauth_utils.py (13)
  • init_oauth (23-111)
  • generate_oauth_state (114-152)
  • verify_oauth_state (155-189)
  • extract_user_info_from_google (192-209)
  • extract_user_info_from_github (212-251)
  • extract_user_info_from_discord (254-291)
  • find_user_by_provider (294-314)
  • create_oauth_user (317-374)
  • link_provider_to_user (377-436)
  • can_auto_link_accounts (439-469)
  • update_user_last_login (472-491)
  • get_oauth_redirect_url (494-522)
  • OAuthProviders (16-20)
main.py (6)
blueprints/oauth.py (1)
  • init_oauth_for_app (43-47)
utils/mongo_utils.py (1)
  • ensure_indexes (396-454)
utils/log_context.py (1)
  • setup_logging_middleware (95-179)
utils/logger.py (2)
  • get_logger (19-34)
  • hash_ip (72-95)
utils/url_utils.py (1)
  • get_client_ip (48-66)
utils/auth_utils.py (1)
  • resolve_owner_id_from_request (276-361)
cache/dual_cache.py (1)
utils/logger.py (1)
  • get_logger (19-34)
blueprints/limiter.py (3)
utils/url_utils.py (1)
  • get_client_ip (48-66)
utils/logger.py (2)
  • get_logger (19-34)
  • hash_ip (72-95)
utils/auth_utils.py (1)
  • resolve_owner_id_from_request (276-361)
cache/redis_client.py (1)
utils/logger.py (1)
  • get_logger (19-34)
blueprints/redirector.py (5)
utils/url_utils.py (4)
  • get_city (36-45)
  • get_client_ip (48-66)
  • get_city_cf (32-33)
  • get_country (20-29)
utils/mongo_utils.py (6)
  • update_url (59-63)
  • update_emoji_url (99-103)
  • get_url_by_length_and_type (255-289)
  • update_url_v2_clicks (196-210)
  • expire_url_if_max_clicks_reached (213-222)
  • insert_click_data (225-252)
utils/auth_utils.py (1)
  • verify_password (55-60)
utils/logger.py (3)
  • get_logger (19-34)
  • hash_ip (72-95)
  • should_sample (37-69)
cache/cache_url.py (4)
  • UrlCacheData (26-38)
  • get_url_cache_data (63-83)
  • set_url_cache_data (46-61)
  • invalidate_url_cache (85-103)
builders/stats.py (5)
utils/mongo_utils.py (1)
  • check_url_stats_privacy (322-347)
utils/aggregation_strategies.py (21)
  • AggregationStrategyFactory (459-485)
  • get (474-480)
  • build_pipeline (22-24)
  • build_pipeline (71-100)
  • build_pipeline (236-248)
  • build_pipeline (268-280)
  • build_pipeline (300-312)
  • build_pipeline (332-344)
  • build_pipeline (366-378)
  • build_pipeline (398-410)
  • build_pipeline (430-442)
  • format_results (27-29)
  • format_results (102-144)
  • format_results (250-258)
  • format_results (282-290)
  • format_results (314-322)
  • format_results (346-356)
  • format_results (380-388)
  • format_results (412-420)
  • format_results (444-452)
  • get_bucket_info (214-230)
utils/query_builder.py (6)
  • StatsQueryBuilderFactory (107-134)
  • StatsQueryBuilder (10-104)
  • for_user_stats (111-121)
  • for_anonymous_stats (124-134)
  • with_filters (46-52)
  • build (54-104)
utils/stats_utils.py (2)
  • format_stats_response_with_metadata (166-225)
  • validate_date_range (228-264)
utils/logger.py (2)
  • get_logger (19-34)
  • should_sample (37-69)
builders/exports.py (3)
builders/stats.py (3)
  • StatsQueryBuilder (19-566)
  • _fail (61-63)
  • build (512-566)
utils/query_builder.py (2)
  • StatsQueryBuilder (10-104)
  • build (54-104)
utils/logger.py (2)
  • get_logger (19-34)
  • should_sample (37-69)
cache/cache_url.py (2)
utils/logger.py (1)
  • get_logger (19-34)
cache/base_cache.py (3)
  • BaseCache (9-27)
  • get (19-20)
  • delete (25-27)
cache/base_cache.py (2)
utils/logger.py (1)
  • get_logger (19-34)
cache/redis_client.py (1)
  • get_redis (11-29)
api/v1/keys.py (4)
utils/auth_utils.py (1)
  • requires_auth (178-249)
utils/logger.py (1)
  • get_logger (19-34)
utils/mongo_utils.py (3)
  • insert_api_key (353-358)
  • list_api_keys_by_user (369-377)
  • revoke_api_key_by_id (380-393)
blueprints/limiter.py (1)
  • rate_limit_key_for_request (46-56)
blueprints/auth.py (6)
blueprints/limiter.py (1)
  • rate_limit_key_for_request (46-56)
utils/logger.py (1)
  • get_logger (19-34)
utils/auth_utils.py (11)
  • verify_password (55-60)
  • hash_password (51-52)
  • generate_access_jwt (63-76)
  • generate_refresh_jwt (88-103)
  • verify_refresh_jwt (106-117)
  • set_refresh_cookie (120-132)
  • set_access_cookie (149-161)
  • clear_refresh_cookie (135-146)
  • clear_access_cookie (164-175)
  • requires_auth (178-249)
  • get_user_profile (364-401)
utils/url_utils.py (2)
  • validate_password (69-86)
  • get_client_ip (48-66)
utils/password_utils.py (1)
  • validate_password (5-82)
utils/mongo_utils.py (2)
  • get_user_by_email (125-130)
  • get_user_by_id (148-153)
blueprints/dashboard.py (4)
utils/auth_utils.py (2)
  • requires_auth (178-249)
  • get_user_profile (364-401)
utils/mongo_utils.py (1)
  • get_user_by_id (148-153)
blueprints/limiter.py (1)
  • rate_limit_key_for_request (46-56)
utils/logger.py (1)
  • get_logger (19-34)
🪛 ast-grep (0.39.9)
main.py

[warning] 127-132: Running flask app with host 0.0.0.0 could expose the server publicly.
Context: app.run(
host="0.0.0.0",
port=8000,
use_reloader=os.getenv("ENV") != "production",
debug=os.getenv("ENV") != "production",
)
Note: [CWE-668]: Exposure of Resource to Wrong Sphere [OWASP A01:2021]: Broken Access Control [REFERENCES]
https://owasp.org/Top10/A01_2021-Broken_Access_Control

(avoid_app_run_with_bad_host-python)

🪛 dotenv-linter (4.0.0)
.env.example

[warning] 12-12: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 13-13: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 17-17: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 18-18: [UnorderedKey] The LOG_FORMAT key should go before the LOG_LEVEL key

(UnorderedKey)


[warning] 18-18: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 21-21: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 22-22: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 23-23: [UnorderedKey] The SAMPLE_RATE_CACHE key should go before the SAMPLE_RATE_REDIRECT key

(UnorderedKey)


[warning] 23-23: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 24-24: [UnorderedKey] The SAMPLE_RATE_EXPORT key should go before the SAMPLE_RATE_REDIRECT key

(UnorderedKey)


[warning] 24-24: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 27-27: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 28-28: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 29-29: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 29-29: [UnorderedKey] The HCAPTCHA_SECRET key should go before the URL_REPORT_WEBHOOK key

(UnorderedKey)


[warning] 33-33: [UnorderedKey] The JWT_AUDIENCE key should go before the JWT_ISSUER key

(UnorderedKey)


[warning] 34-34: [UnorderedKey] The ACCESS_TOKEN_TTL_SECONDS key should go before the JWT_AUDIENCE key

(UnorderedKey)


[warning] 36-36: [UnorderedKey] The COOKIE_SECURE key should go before the JWT_AUDIENCE key

(UnorderedKey)


[warning] 36-36: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 37-37: [UnorderedKey] The JWT_PRIVATE_KEY key should go before the REFRESH_TOKEN_TTL_SECONDS key

(UnorderedKey)


[warning] 38-38: [UnorderedKey] The JWT_PUBLIC_KEY key should go before the REFRESH_TOKEN_TTL_SECONDS key

(UnorderedKey)


[warning] 58-58: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 59-59: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 60-60: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 69-69: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 70-70: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 71-71: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 73-73: [ExtraBlankLine] Extra blank line detected

(ExtraBlankLine)


[warning] 81-81: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 82-82: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 83-83: [EndingBlankLine] No blank line at the end of the file

(EndingBlankLine)


[warning] 83-83: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)

🪛 GitHub Actions: Check Code Formatting
blueprints/limiter.py

[error] 2-2: F401 [*] flask_limiter.util.get_remote_address imported but unused. Remove unused import: flask_limiter.util.get_remote_address


[error] 5-5: F401 [*] utils.logger.hash_ip imported but unused. Remove unused import: utils.logger.hash_ip

blueprints/redirector.py

[error] 34-34: E402 Module level import not at top of file


[error] 35-35: E402 Module level import not at top of file


[error] 36-36: E402 Module level import not at top of file


[error] 37-37: E402 Module level import not at top of file


[error] 38-38: E402 Module level import not at top of file


[error] 39-39: E402 Module level import not at top of file

🪛 LanguageTool
LOGGING_IMPLEMENTATION.md

[uncategorized] ~303-~303: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... 100% --- ### Task 7: API Key & Rate Limiting Logging Files to update: `api/v1...

(EN_COMPOUND_ADJECTIVE_INTERNAL)


[grammar] ~514-~514: Ensure spelling is correct
Context: ...t Sampling** - Per-request overhead: ~2-4ms - CPU impact: ~1-2% - ❌ Not acceptable for...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🪛 markdownlint-cli2 (0.18.1)
LOGGING_IMPLEMENTATION.md

23-23: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


500-500: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


560-560: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


619-619: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


628-628: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (22)
pyproject.toml (1)

9-9: LGTM! Authlib vulnerability addressed.

The minimum version constraint has been correctly updated to 1.6.5, which addresses the HIGH-severity vulnerabilities that were present in 1.6.3.

cache/dual_cache.py (1)

6-8: LGTM! Structured logging improves observability.

The addition of structured logging with contextual fields (base_key, error, error_type) is a clear improvement over unstructured error handling.

Also applies to: 78-83

cache/base_cache.py (1)

4-6: LGTM! Structured logging improves error visibility.

The structured logging approach with error details is consistent with the broader logging improvements across the codebase.

Also applies to: 14-16

blueprints/contact.py (1)

11-15: LGTM! Logging adds valuable observability.

The structured logging events for webhook interactions include appropriate context (email domain, message length, short code, truncated reason) and properly differentiate between success and failure cases.

Also applies to: 64-75, 153-165

blueprints/limiter.py (1)

30-56: LGTM! Dynamic rate limiting enhances API protection.

The new helper functions correctly differentiate authenticated users (higher limits) from anonymous users and provide proper bucketing by user ID, API key prefix, or IP address. The implementation aligns well with the PR's authentication and rate-limiting objectives.

blueprints/stats.py (1)

26-32: LGTM! Structured logging improves legacy route observability.

The comprehensive logging additions across stats, analytics, and export routes provide valuable observability while maintaining existing functionality. The contextual fields (short_code, method, format, password_provided) are well-chosen for debugging and monitoring.

Also applies to: 61-178, 235-331

blueprints/url_shortener.py (2)

27-40: LGTM! Logging additions improve observability.

The structured logging events for URL creation (both success and failure) include appropriate context fields and support both v1 and v2 schemas.

Also applies to: 106-108, 172-181, 214-219, 278-287


307-323: LGTM! V2 lookup logic correctly handles schema detection.

The logic correctly initializes v2=False, attempts the V2 lookup first, and only sets v2=True when a V2 document is found. This prevents the KeyError issue flagged in the past review comment.

blueprints/dashboard.py (1)

182-206: Idempotent profile picture updates handled correctly.

The acknowledged / matched_count checks let repeat requests succeed quietly while still surfacing real failures—nice cleanup.

cache/__init__.py (1)

9-10: Cache TTLs fit the new refresh cadence.

The 10‑minute live window with a 1‑hour stale buffer aligns with the SWR strategy elsewhere—looks good.

cache/redis_client.py (1)

16-26: Structured Redis connection logging looks great.

Swapping print statements for bound logger calls keeps startup diagnostics consistent with the rest of the app.

api/v1/keys.py (2)

1-28: LGTM!

The imports and constants are well-organized. The ALLOWED_SCOPES set provides clear permission boundaries for API keys.


317-414: LGTM!

The delete/revoke logic is well-designed with clear separation between hard delete and soft delete (revoke) modes. The comprehensive docstring and error handling are excellent.

api/v1/management.py (2)

129-208: LGTM!

The filtered payload approach (line 199) ensures only the status field can be updated through this endpoint, preventing accidental modifications to other fields. Clean separation of concerns.


211-321: LGTM!

The cache invalidation (lines 299-309) is properly implemented with defensive error handling. Retrieving the short_code before deletion (line 284) ensures the cache can be invalidated even after the document is removed.

blueprints/redirector.py (4)

47-86: LGTM!

The error class hierarchy is well-designed with proper inheritance, status codes, and JSON response formatting. The _json_error_response method provides consistent error formatting across all error types.


88-292: LGTM!

The redirect flow correctly handles both v1 and v2 schemas with appropriate cache logic, password verification (hashed for v2, plain for v1), and status checks. The defensive type check for max_clicks (lines 118-122) guards against cache corruption.


316-446: LGTM!

The v2 click processing correctly implements time-series analytics with proper bot detection, referrer sanitization (lines 349-356), and max-clicks expiration logic. The User-Agent requirement (lines 321-332) is intentionally strict for bot prevention, as confirmed by the maintainer.


449-560: LGTM!

The legacy click processing maintains backward compatibility with the v1 schema. The exponential moving average for redirection time (lines 539-541) with alpha=0.1 is a good approach for smoothing fluctuations.

builders/stats.py (3)

112-326: LGTM!

The parsing methods are well-structured with comprehensive validation. The security check preventing short_code filter bypass (lines 247-262) is crucial for privacy protection. Timezone alias mapping (lines 302-315) handles deprecated names gracefully.


365-510: LGTM!

The aggregation execution handles per-dimension failures gracefully (lines 388-395), and the formatting methods include comprehensive metadata. The defensive error handling in _get_summary_stats (lines 501-502) ensures the API always returns valid data.


512-566: LGTM with dependency on query builder fix.

The build method has proper error checking (lines 513-514) and comprehensive logging with sampling. However, it depends on _build_click_query returning a dict (not self) in error cases, as noted in the previous comment.

Comment thread blueprints/limiter.py Outdated
Comment thread blueprints/limiter.py Outdated
Comment thread blueprints/redirector.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
blueprints/redirector.py (1)

586-588: Stop redirecting with the raw password in the query string. This still surfaces the secret in URLs, referrers, browser history, and logs—the same issue flagged in the earlier review remains unresolved. Please store the verification server-side (e.g., session flag or signed short-lived token) and redirect without ?password=….

🧹 Nitpick comments (1)
blueprints/redirector.py (1)

415-424: Handle Mongo helper failure explicitly before accessing attributes. update_url_v2_clicks/expire_url_if_max_clicks_reached return None when the driver raises, so dereferencing .acknowledged/.modified_count throws AttributeError and hides the original database error. Guard for None, log it, and raise a meaningful redirector error instead of masking the root cause.

-        update_result = update_url_v2_clicks(url_data["_id"], last_click_time=curr_time)
-        if not update_result.acknowledged:
+        update_result = update_url_v2_clicks(url_data["_id"], last_click_time=curr_time)
+        if update_result is None:
+            log.error(
+                "url_v2_click_update_failed",
+                short_code=short_code,
+                url_id=str(url_data["_id"]),
+                reason="mongo_exception",
+            )
+            raise InternalRedirectorError("Failed to update click analytics")
+        if not update_result.acknowledged:
             raise InternalRedirectorError("Failed to update click analytics")
@@
-        if url_data.get("max_clicks"):
-            expire_result = expire_url_if_max_clicks_reached(
-                url_data["_id"], url_data["max_clicks"]
-            )
-            if expire_result.modified_count > 0:
+        if url_data.get("max_clicks"):
+            expire_result = expire_url_if_max_clicks_reached(
+                url_data["_id"], url_data["max_clicks"]
+            )
+            if expire_result is None:
+                log.error(
+                    "url_v2_expire_check_failed",
+                    short_code=short_code,
+                    url_id=str(url_data["_id"]),
+                    reason="mongo_exception",
+                )
+            elif expire_result.modified_count > 0:
                 log.info(
                     "url_expired",
                     url_id=str(url_data["_id"]),
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b5f6bd and 56eea78.

📒 Files selected for processing (2)
  • blueprints/limiter.py (2 hunks)
  • blueprints/redirector.py (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
blueprints/limiter.py (3)
utils/logger.py (1)
  • get_logger (19-34)
utils/auth_utils.py (1)
  • resolve_owner_id_from_request (276-361)
utils/url_utils.py (1)
  • get_client_ip (48-66)
blueprints/redirector.py (5)
utils/url_utils.py (4)
  • get_city (36-45)
  • get_client_ip (48-66)
  • get_city_cf (32-33)
  • get_country (20-29)
utils/mongo_utils.py (6)
  • update_url (59-63)
  • update_emoji_url (99-103)
  • get_url_by_length_and_type (255-289)
  • update_url_v2_clicks (196-210)
  • expire_url_if_max_clicks_reached (213-222)
  • insert_click_data (225-252)
utils/auth_utils.py (1)
  • verify_password (55-60)
utils/logger.py (3)
  • get_logger (19-34)
  • hash_ip (72-95)
  • should_sample (37-69)
cache/cache_url.py (4)
  • UrlCacheData (26-38)
  • get_url_cache_data (63-83)
  • set_url_cache_data (46-61)
  • invalidate_url_cache (85-103)

Comment thread blueprints/limiter.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
blueprints/url_shortener.py (1)

369-372: Consider estimated_document_count() for better performance.

count_documents({}) performs a collection scan and can be slow on large collections. Since these are public metrics where exact precision may not be critical, consider using estimated_document_count() for faster performance.

Apply this diff:

-        v2_shortlinks = urls_v2_collection.count_documents({})
+        v2_shortlinks = urls_v2_collection.estimated_document_count()

         # Get document count from clicks time-series collection
-        total_clicks_from_ts = clicks_collection.count_documents({})
+        total_clicks_from_ts = clicks_collection.estimated_document_count()

Note: estimated_document_count() uses collection metadata and is much faster, but may be slightly less accurate if documents were recently added/deleted. For public metrics, this trade-off is typically acceptable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 56eea78 and 3273286.

📒 Files selected for processing (1)
  • blueprints/url_shortener.py (10 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-10T06:17:02.860Z
Learnt from: Zingzy
Repo: spoo-me/url-shortener PR: 94
File: templates/base.html:7-7
Timestamp: 2025-11-10T06:17:02.860Z
Learning: In the spoo.me URL shortener project, hardcoded production URLs are preferred over Flask's `url_for()` helper for performance reasons, particularly in frequently-rendered templates like base.html. Avoid suggesting `url_for()` replacements for static asset URLs.

Applied to files:

  • blueprints/url_shortener.py
🧬 Code graph analysis (1)
blueprints/url_shortener.py (3)
utils/mongo_utils.py (3)
  • get_url_v2_by_alias (181-185)
  • load_emoji_url (75-80)
  • load_url (35-40)
utils/general.py (1)
  • humanize_number (22-27)
utils/logger.py (1)
  • get_logger (19-34)
🔇 Additional comments (8)
blueprints/url_shortener.py (8)

27-29: LGTM!

The new imports for V2 schema support, structured logging, and metric timing are appropriate and used correctly throughout the file.

Also applies to: 32-32, 40-40, 43-43


52-52: LGTM!

Simplified template rendering by removing cookie-based history is a good architectural improvement, moving toward stateless operations.


55-56: LGTM!

Good documentation of technical debt. The TODO reminder helps track future deprecation work.


109-111: LGTM!

Excellent structured logging additions. The context provided (alias, schema, reason, has_password, max_clicks, block_bots) will be valuable for monitoring, debugging, and analytics.

Also applies to: 175-184


217-222: LGTM!

Logging additions mirror the standard shorten route pattern and provide consistent observability across both flows.

Also applies to: 281-290


310-320: LGTM!

The V2-first lookup pattern is now correctly implemented: v2 is set to True only after confirming get_url_v2_by_alias returns data, preventing the KeyError when falling back to legacy schema. The subsequent short_code derivation correctly uses the v2 flag to pick the appropriate key.

Also applies to: 323-326


346-354: LGTM!

Renaming to METRIC_PIPELINE_V1 clearly indicates this pipeline is for legacy V1 URLs and improves code clarity.


375-376: Review comment is incorrect — no double-counting exists in the metric calculation.

The code properly segregates click tracking by schema version:

  • V1 URLs (handle_legacy_click): Increment the "total-clicks" field directly in the urls_collection document. No time-series insertion occurs.
  • V2 URLs (handle_v2_click): Insert click events into the time-series clicks_collection via insert_click_data().

The metric at lines 375–376 combines these separate sources:

  • v1_clicks: aggregated sum of "total-clicks" fields from urls_collection
  • total_clicks_from_ts: count of all documents in clicks_collection

Since V1 clicks are never inserted into the time-series collection, there is no overlap or double-counting. Each click is tracked in exactly one location based on its schema version.

Comment thread blueprints/url_shortener.py
Comment thread blueprints/url_shortener.py Outdated
- Added email verification requirement for API key creation in keys.py.
- Updated password requirements in management.py.
- Enhanced auth.py to handle email verification during login and registration, including sending verification emails.
- Introduced email_service.py for sending transactional emails via ZeptoMail.
- Created verification_utils.py for managing OTP generation and verification.
- Added MongoDB collection and functions for handling verification tokens in mongo_utils.py.
- Updated auth_utils.py to include email verification status in JWT tokens.
- Modified base.py to check for email verification status when resolving owner ID.
… a verification page with enhanced user experience

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

♻️ Duplicate comments (8)
static/css/header.css (2)

44-48: Width fallback order is still incorrect; reverse to ensure proper CSS cascade.

This duplicates a previous review comment. The fallback value (width: 100%) must come first so that browsers supporting -webkit-fill-available override it. Currently, the order is reversed.

Apply this diff to fix the fallback order:

 .navbar .links {
-    padding: 0;
-    width: -webkit-fill-available;
     width: 100%;
+    width: -webkit-fill-available;
+    padding: 0;
 }

233-236: Add width fallback for profile dropdown hover state.

This duplicates a previous review comment. Line 236 uses width: -webkit-fill-available without the fallback width: 100%, which can break layout consistency in browsers that don't support webkit-specific values.

Apply this diff to add the fallback:

 .profile-dropdown a:hover,
-.profile-dropdown button:hover { background: rgba(255,255,255,0.08); text-shadow: none !important; width: -webkit-fill-available; }
+.profile-dropdown button:hover { background: rgba(255,255,255,0.08); text-shadow: none !important; width: 100%; width: -webkit-fill-available; }
api/v1/keys.py (4)

31-42: Support ISO 8601 timestamps with Z suffix in _parse_expires_at.

datetime.fromisoformat() rejects strings like "2025-01-01T00:00:00Z", which are common in APIs. Those values will currently fall into the except and be treated as invalid.

Normalize a trailing "Z"/"z" to "+00:00" before parsing:

 def _parse_expires_at(value: Optional[str | int | float]):
     if value is None:
         return None
     try:
         if isinstance(value, (int, float)):
             return datetime.fromtimestamp(int(value), tz=timezone.utc)
-        dt = datetime.fromisoformat(str(value))
+        raw = str(value)
+        if raw.endswith(("Z", "z")):
+            raw = raw[:-1] + "+00:00"
+        dt = datetime.fromisoformat(raw)
         if dt.tzinfo is None:
             dt = dt.replace(tzinfo=timezone.utc)
         return dt.astimezone(timezone.utc)
     except Exception:
         return None

Also consider explicitly rejecting booleans (True/False) instead of treating them as epoch seconds via the int path.


156-160: Harden request body parsing for name, description, and scopes.

(body.get("name") or "").strip() and the same pattern for description will raise AttributeError if a client passes a non-string (e.g., 123, false). Similarly, scopes currently only checks list-ness; elements can be non-strings, and unhashable values (e.g., objects) will raise when tested against ALLOWED_SCOPES.

Recommend:

  • Validate types before using string methods and before membership checks.
  • Return 400 on bad types instead of 500.

Example:

-    name = (body.get("name") or "").strip()
-    description = (body.get("description") or "").strip() or None
-    scopes = body.get("scopes") or []
+    raw_name = body.get("name")
+    if raw_name is None or not isinstance(raw_name, str):
+        return jsonify({"error": "name must be a non-empty string"}), 400
+    name = raw_name.strip()
+
+    raw_description = body.get("description")
+    if raw_description is not None and not isinstance(raw_description, str):
+        return jsonify({"error": "description must be a string if provided"}), 400
+    description = (raw_description or "").strip() or None
+
+    scopes = body.get("scopes") or []
+    if not isinstance(scopes, list):
+        return jsonify({"error": "scopes must be a non-empty array"}), 400
+    if not scopes or any(not isinstance(s, str) for s in scopes):
+        return jsonify({"error": "scopes must be a non-empty array of strings"}), 400

Then keep the ALLOWED_SCOPES membership check as-is.


183-235: Normalize datetimes before .timestamp() in the creation response.

doc["created_at"] is stored as an aware UTC datetime now, but older records or future migrations might introduce naive datetimes. Likewise, expires_at could conceivably be stored without tzinfo. Calling .timestamp() on naive values will interpret them in the server’s local timezone.

Normalize to UTC before converting:

-                "created_at": int(doc["created_at"].timestamp()),
-                "expires_at": int(expires_at.timestamp()) if expires_at else None,
+                "created_at": int(
+                    (doc["created_at"].replace(tzinfo=timezone.utc)
+                     if doc["created_at"].tzinfo is None
+                     else doc["created_at"].astimezone(timezone.utc)
+                    ).timestamp()
+                ),
+                "expires_at": (
+                    int(
+                        (expires_at.replace(tzinfo=timezone.utc)
+                         if expires_at.tzinfo is None
+                         else expires_at.astimezone(timezone.utc)
+                        ).timestamp()
+                    )
+                    if expires_at
+                    else None
+                ),

This mirrors the normalization logic used in _parse_expires_at and avoids subtle timezone bugs.


314-333: Normalize created_at / expires_at and handle DB errors explicitly in list_api_keys.

Two related concerns here:

  1. Datetime normalization (repeat of earlier concern).
    k.get("created_at").timestamp() and "expires_at" may be called on naive datetimes if older records or driver behavior change. Normalize to UTC before .timestamp():
-                "created_at": int(k.get("created_at").timestamp())
-                if k.get("created_at")
-                else None,
-                "expires_at": int(k.get("expires_at").timestamp())
-                if k.get("expires_at")
-                else None,
+                "created_at": (
+                    int(
+                        (
+                            k.get("created_at").replace(tzinfo=timezone.utc)
+                            if k.get("created_at") and k.get("created_at").tzinfo is None
+                            else k.get("created_at").astimezone(timezone.utc)
+                        ).timestamp()
+                    )
+                    if k.get("created_at")
+                    else None
+                ),
+                "expires_at": (
+                    int(
+                        (
+                            k.get("expires_at").replace(tzinfo=timezone.utc)
+                            if k.get("expires_at") and k.get("expires_at").tzinfo is None
+                            else k.get("expires_at").astimezone(timezone.utc)
+                        ).timestamp()
+                    )
+                    if k.get("expires_at")
+                    else None
+                ),
  1. DB error semantics.
    list_api_keys_by_user returns [] on exceptions (see utils/mongo_utils.py), so DB failures are indistinguishable from “no keys”. The docstring for this endpoint lists 500 for database error, but the current implementation always returns 200 with "keys": [] on failure.

If you want to surface DB errors:

  • Either change list_api_keys_by_user to propagate failures (or return (None, error)), or
  • Wrap the call here in a try/except and log/return 500 when something goes wrong.
blueprints/oauth.py (1)

61-61: CRITICAL: OAuth state parameter lacks cryptographic integrity protection.

The OAuth state parameter is still generated as an unsigned querystring (see utils/oauth_utils.py lines 113-151), making the flows vulnerable to CSRF and account takeover attacks. An attacker can forge state parameters or modify the user_id field in linking flows to hijack accounts.

The state must be cryptographically signed (HMAC or JWT) before use in production.

Recommended fix: Update generate_oauth_state in utils/oauth_utils.py to sign the state with a server secret:

import hmac
import hashlib
import json
import base64

def generate_oauth_state(provider: str, action: str = "login", user_id: Optional[str] = None) -> str:
    state_data = {
        "provider": provider,
        "action": action,
        "nonce": secrets.token_urlsafe(32),
        "timestamp": datetime.now(timezone.utc).isoformat(),
    }
    if user_id:
        state_data["user_id"] = user_id
    
    # Serialize and sign the state
    state_json = json.dumps(state_data)
    state_b64 = base64.urlsafe_b64encode(state_json.encode()).decode()
    
    # Sign with HMAC
    secret = os.getenv("FLASK_SECRET_KEY").encode()
    signature = hmac.new(secret, state_b64.encode(), hashlib.sha256).hexdigest()
    
    return f"{state_b64}.{signature}"

def verify_oauth_state(state: str, expected_provider: str) -> Tuple[bool, Dict[str, Any]]:
    try:
        # Split state and signature
        state_b64, signature = state.rsplit(".", 1)
        
        # Verify signature
        secret = os.getenv("FLASK_SECRET_KEY").encode()
        expected_sig = hmac.new(secret, state_b64.encode(), hashlib.sha256).hexdigest()
        if not hmac.compare_digest(signature, expected_sig):
            return False, {}
        
        # Decode state
        state_json = base64.urlsafe_b64decode(state_b64.encode()).decode()
        state_data = json.loads(state_json)
        
        # Validate provider and timestamp
        if state_data.get("provider") != expected_provider:
            return False, {}
        
        timestamp = datetime.fromisoformat(state_data.get("timestamp", ""))
        age = (datetime.now(timezone.utc) - timestamp).total_seconds()
        if age > 600:  # 10 minutes
            return False, {}
        
        return True, state_data
    except Exception:
        return False, {}

Also applies to: 316-316, 335-335, 591-591, 610-610, 864-864

builders/base.py (1)

163-185: Still need to handle ISO8601 timestamps ending with Z in parse_expire_after.

datetime.fromisoformat(str(expire_after)) does not accept UTC timestamps with a trailing "Z" (e.g., "2025-11-10T05:00:00Z"), so such RFC‑style inputs will hit the exception path and incorrectly return a 400.

You can normalize a trailing Z to +00:00 before parsing, e.g.:

-            else:
-                dt = datetime.fromisoformat(str(expire_after))
+            else:
+                raw = str(expire_after)
+                if raw.endswith("Z"):
+                    raw = raw[:-1] + "+00:00"
+                dt = datetime.fromisoformat(raw)

This preserves existing behavior while accepting common UTC ISO8601 representations.

🧹 Nitpick comments (9)
static/css/header.css (1)

129-145: Limit !important declarations to specificity conflicts only.

Lines 130, 134, 142, 143, 145, 184–187 include multiple !important flags. While some may be justified (e.g., GitHub badge button override), consider restructuring selectors to eliminate unnecessary !important usage and improve CSS maintainability.

For example, instead of:

.github-badge:hover {
    background: rgba(255, 255, 255, 0.12) !important;
    border-color: rgba(255, 255, 255, 0.25);
}

Use a more specific selector or class cascade to avoid the override flag.

static/css/mobile-header.css (2)

183-193: CSS duplication: .external-link is defined in both header.css and mobile-header.css.

The .external-link style (including hover transform behavior) is duplicated across header.css and mobile-header.css. Consider extracting this into a shared stylesheet to reduce duplication and improve maintainability.


225-237: Extract repeated profile avatar styling into a shared partial.

The .profile-initials-circle styling is duplicated between header.css (lines 206–218) and mobile-header.css (lines 225–237). Consider moving this shared component style to a separate partial or utility file (e.g., static/css/components.css or static/css/avatar.css) to reduce duplication.

static/js/dashboard/keys.js (1)

328-346: Consider removing or documenting the setTimeout delay.

The 100ms setTimeout on line 337 when showing detailed permissions appears arbitrary. If it's for animation coordination, consider using a CSS transitionend event listener or documenting why the delay is necessary.

If no animation depends on this timing, simplify:

-        setTimeout(() => {
-            detailedPermissions.classList.remove('hidden');
-        }, 100);
+        detailedPermissions.classList.remove('hidden');
api/v1/keys.py (2)

22-28: Restrict granting of admin:all scope to admin users only.

Right now any authenticated, email-verified user can request admin:all in scopes, since it’s included in ALLOWED_SCOPES and there’s no role/privilege check here. That risks privilege escalation if any endpoint authorizes solely based on this scope.

Consider:

  • Either removing admin:all from ALLOWED_SCOPES here, or
  • Adding a check (e.g., admin flag on the user) before allowing admin:all to be requested.

336-433: Differentiate DB errors from not-found in delete_api_key.

revoke_api_key_by_id returns False both when the key doesn’t match user_id and when a DB error occurs (it catches and suppresses exceptions). As a result, this endpoint always returns 404 for those cases, even though the docs mention a 500 for “database error”.

To align behavior with the docs and improve observability:

  • Have revoke_api_key_by_id distinguish failure modes (e.g., return an enum/str status, or raise on DB errors), and
  • Map DB failures here to a 500 with an appropriate log entry, while keeping 404 for not-found/ownership violations.

Example sketch:

status = revoke_api_key_by_id(g.user_id, key_id, hard_delete=not revoke_only)
if status == "db_error":
    log.error("api_key_deletion_failed", ..., reason="database_error")
    return jsonify({"error": "database error"}), 500
if status != "ok":
    ...
    return jsonify({"error": "key not found or access denied"}), 404
.env.example (1)

32-33: Consider adding placeholder values for JWT_ISSUER and JWT_AUDIENCE.

These fields are currently empty, which may cause confusion for users setting up the application. Consider adding example placeholder values like JWT_ISSUER="spoo.me" and JWT_AUDIENCE="spoo.me" to guide users.

-JWT_ISSUER=
-JWT_AUDIENCE=
+JWT_ISSUER="spoo.me"
+JWT_AUDIENCE="spoo.me"
api/v1/management.py (1)

211-321: Delete flow correctly enforces ownership and invalidates cache; consider minor builder chaining tweak.

The delete endpoint:

  • Validates url_id format upfront.
  • Reuses UpdateUrlRequestBuilder for auth scope and ownership checks before deletion.
  • Logs url_deleted with alias/owner context.
  • Invalidates the URL cache via cq.invalidate_url_cache(short_code=short_code) and tolerates cache failures.

If you want to be fully consistent with the fluent builder style and avoid any accidental work after a prior error, you could write:

-    builder = UpdateUrlRequestBuilder({}, url_id)
-    builder.parse_auth_scope(required_scopes={"urls:manage", "admin:all"})
-    builder.load_and_validate_ownership()
-
-    if builder.error:
-        return builder.error
+    builder = (
+        UpdateUrlRequestBuilder({}, url_id)
+        .parse_auth_scope(required_scopes={"urls:manage", "admin:all"})
+        .load_and_validate_ownership()
+    )
+    if builder.error:
+        return builder.error

Behavior is fine as-is; this is just for consistency and to ensure future changes to builder methods keep short‑circuiting cleanly.

builders/base.py (1)

25-36: Builder initialization enforces verified owners for all URL operations—confirm this matches product intent.

BaseUrlRequestBuilder.__init__ always calls resolve_owner_id_from_request(require_verified=True) and immediately stores g.verification_error as a 403 if present. That means any endpoint using this builder (create/update/delete) will hard‑fail for JWT users whose email isn’t verified, while API‑key callers bypass the verification check.

If you expect some URL operations to remain available to unverified JWT users, you may want to make require_verified configurable per endpoint/builder subclass instead of hard‑coding it here.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3273286 and 6dbee0a.

⛔ Files ignored due to path filters (4)
  • static/images/Lightning.png is excluded by !**/*.png
  • static/images/Redo.png is excluded by !**/*.png
  • static/images/Rocket.png is excluded by !**/*.png
  • static/images/Shield.png is excluded by !**/*.png
📒 Files selected for processing (10)
  • .env.example (1 hunks)
  • api/v1/keys.py (1 hunks)
  • api/v1/management.py (1 hunks)
  • blueprints/auth.py (1 hunks)
  • blueprints/oauth.py (1 hunks)
  • builders/base.py (1 hunks)
  • static/css/header.css (1 hunks)
  • static/css/mobile-header.css (1 hunks)
  • static/js/dashboard/keys.js (1 hunks)
  • static/js/header.js (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-10T06:00:33.952Z
Learnt from: Zingzy
Repo: spoo-me/url-shortener PR: 94
File: docker-compose.yml:7-13
Timestamp: 2025-11-10T06:00:33.952Z
Learning: In the spoo-me/url-shortener repository, Docker and docker-compose.yml are used only for local development, not in production. Hardcoded credentials in docker-compose.yml are acceptable.

Applied to files:

  • .env.example
🧬 Code graph analysis (6)
blueprints/oauth.py (5)
utils/logger.py (1)
  • get_logger (19-34)
utils/auth_utils.py (5)
  • generate_access_jwt (63-79)
  • generate_refresh_jwt (91-109)
  • set_refresh_cookie (126-138)
  • set_access_cookie (155-167)
  • requires_auth (184-270)
utils/mongo_utils.py (2)
  • get_user_by_email (126-131)
  • get_user_by_id (149-154)
utils/oauth_utils.py (13)
  • init_oauth (23-111)
  • generate_oauth_state (114-152)
  • verify_oauth_state (155-189)
  • extract_user_info_from_google (192-209)
  • extract_user_info_from_github (212-251)
  • extract_user_info_from_discord (254-291)
  • find_user_by_provider (294-314)
  • create_oauth_user (317-374)
  • link_provider_to_user (377-436)
  • can_auto_link_accounts (439-469)
  • update_user_last_login (472-491)
  • get_oauth_redirect_url (494-522)
  • OAuthProviders (16-20)
utils/email_service.py (1)
  • send_welcome_email (226-275)
api/v1/management.py (5)
blueprints/limiter.py (2)
  • dynamic_limit_for_request (29-42)
  • rate_limit_key_for_request (45-55)
utils/logger.py (1)
  • get_logger (19-34)
builders/update.py (6)
  • UpdateUrlRequestBuilder (14-197)
  • load_and_validate_ownership (22-50)
  • validate_long_url_if_present (52-58)
  • validate_alias_custom (60-69)
  • build_update (82-197)
  • parse_status_change (71-80)
builders/base.py (6)
  • parse_auth_scope (50-66)
  • validate_password (119-134)
  • parse_max_clicks (144-161)
  • parse_expire_after (163-186)
  • parse_block_bots (136-142)
  • parse_private_stats (188-196)
cache/cache_url.py (1)
  • invalidate_url_cache (85-103)
blueprints/auth.py (8)
blueprints/limiter.py (1)
  • rate_limit_key_for_request (45-55)
utils/logger.py (1)
  • get_logger (19-34)
utils/auth_utils.py (11)
  • verify_password (55-60)
  • hash_password (51-52)
  • generate_access_jwt (63-79)
  • generate_refresh_jwt (91-109)
  • verify_refresh_jwt (112-123)
  • set_refresh_cookie (126-138)
  • set_access_cookie (155-167)
  • clear_refresh_cookie (141-152)
  • clear_access_cookie (170-181)
  • requires_auth (184-270)
  • get_user_profile (408-445)
utils/url_utils.py (2)
  • validate_password (69-86)
  • get_client_ip (48-66)
utils/password_utils.py (1)
  • validate_password (5-82)
utils/mongo_utils.py (2)
  • get_user_by_email (126-131)
  • get_user_by_id (149-154)
utils/verification_utils.py (4)
  • create_email_verification_otp (75-146)
  • create_password_reset_otp (149-223)
  • verify_otp (226-326)
  • is_rate_limited (329-344)
utils/email_service.py (3)
  • send_verification_email (142-182)
  • send_welcome_email (226-275)
  • send_password_reset_email (184-224)
api/v1/keys.py (4)
utils/auth_utils.py (1)
  • requires_auth (184-270)
utils/logger.py (1)
  • get_logger (19-34)
utils/mongo_utils.py (3)
  • insert_api_key (354-359)
  • list_api_keys_by_user (370-378)
  • revoke_api_key_by_id (381-394)
blueprints/limiter.py (1)
  • rate_limit_key_for_request (45-55)
static/js/dashboard/keys.js (1)
static/js/dashboard.js (3)
  • res (160-160)
  • data (162-162)
  • node (87-87)
builders/base.py (4)
utils/url_utils.py (3)
  • validate_url (89-93)
  • validate_alias (134-136)
  • validate_password (69-86)
utils/mongo_utils.py (3)
  • check_if_v2_alias_exists (189-194)
  • check_if_slug_exists (67-73)
  • validate_blocked_url (115-123)
utils/auth_utils.py (2)
  • hash_password (51-52)
  • resolve_owner_id_from_request (297-405)
utils/logger.py (1)
  • get_logger (19-34)
🪛 Biome (2.1.2)
static/css/header.css

[error] 47-47: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.

width is already defined here.

Remove or rename the duplicate property to ensure consistent styling.

(lint/suspicious/noDuplicateProperties)

🪛 dotenv-linter (4.0.0)
.env.example

[warning] 12-12: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 13-13: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 17-17: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 18-18: [UnorderedKey] The LOG_FORMAT key should go before the LOG_LEVEL key

(UnorderedKey)


[warning] 18-18: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 21-21: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 22-22: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 23-23: [UnorderedKey] The SAMPLE_RATE_CACHE key should go before the SAMPLE_RATE_REDIRECT key

(UnorderedKey)


[warning] 23-23: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 24-24: [UnorderedKey] The SAMPLE_RATE_EXPORT key should go before the SAMPLE_RATE_REDIRECT key

(UnorderedKey)


[warning] 24-24: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 27-27: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 28-28: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 29-29: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 29-29: [UnorderedKey] The HCAPTCHA_SECRET key should go before the URL_REPORT_WEBHOOK key

(UnorderedKey)


[warning] 33-33: [UnorderedKey] The JWT_AUDIENCE key should go before the JWT_ISSUER key

(UnorderedKey)


[warning] 34-34: [UnorderedKey] The ACCESS_TOKEN_TTL_SECONDS key should go before the JWT_AUDIENCE key

(UnorderedKey)


[warning] 36-36: [UnorderedKey] The COOKIE_SECURE key should go before the JWT_AUDIENCE key

(UnorderedKey)


[warning] 36-36: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 37-37: [UnorderedKey] The JWT_PRIVATE_KEY key should go before the REFRESH_TOKEN_TTL_SECONDS key

(UnorderedKey)


[warning] 38-38: [UnorderedKey] The JWT_PUBLIC_KEY key should go before the REFRESH_TOKEN_TTL_SECONDS key

(UnorderedKey)


[warning] 58-58: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 59-59: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 60-60: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 69-69: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 70-70: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 71-71: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 73-73: [ExtraBlankLine] Extra blank line detected

(ExtraBlankLine)


[warning] 81-81: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 82-82: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 83-83: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 92-92: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 93-93: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 94-94: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 95-95: [EndingBlankLine] No blank line at the end of the file

(EndingBlankLine)


[warning] 95-95: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 95-95: [UnorderedKey] The APP_URL key should go before the ZEPTO_API_TOKEN key

(UnorderedKey)

🔇 Additional comments (14)
static/css/mobile-header.css (2)

94-128: Burger animation and aria-expanded pattern is well-implemented.

The burger icon morphing animation using aria-expanded="true" state is accessible and follows best practices for semantic HTML + CSS animation. The transforms (rotate, translate) on individual spans provide clear visual feedback for menu state changes.


131-144: Mobile menu animation with max-height transition is accessibility-friendly.

The collapsible menu uses max-height animation (0 → 600px) with cubic-bezier easing, which is a common pattern that preserves content accessibility for screen readers. The border-top on open state (line 143) provides clear visual separation.

static/js/dashboard/keys.js (6)

1-13: LGTM: Clean element registry pattern.

Centralizing DOM element references in a registry object is a good practice for maintainability and performance.


47-65: LGTM: Efficient DOM manipulation.

Using DocumentFragment (line 58) to batch DOM insertions is a performance best practice.


67-117: LGTM: Robust key row rendering.

The status calculation (line 102) correctly prioritizes the revoked state over expiration, and the conditional event listener attachment (line 113) prevents handler registration on disabled buttons.


119-139: LGTM: Proper error handling and user feedback.

The graceful JSON parsing fallback (line 129) and immediate list refresh (line 132) after revocation provide a good user experience.


213-291: LGTM: Comprehensive error handling and excellent UX.

The function demonstrates robust error handling with:

  • Specific error messages for rate limits and max keys (lines 270-274)
  • Modal closure before notifications for visibility (line 256)
  • Graceful handling of email verification requirements (lines 259-263)
  • Loading state management with proper cleanup in the finally block (lines 286-290)

348-394: LGTM: Comprehensive and accessible event handling.

The event listener setup demonstrates good practices:

  • Safe optional chaining (?.) for element references
  • Proper keyboard shortcuts (Escape to close, Enter to submit)
  • Click-to-copy UX for the token input (line 386)
  • Modal backdrop click handlers for intuitive closing
api/v1/keys.py (1)

169-182: Good validation of key quota and expires_at semantics.

Active-key limit, ISO/epoch parsing, and future-date enforcement look solid and align with the documented behavior. No changes needed here.

api/v1/management.py (2)

19-126: update_url_v1 builder chain and rate limiting look solid.

The endpoint wiring (payload parsing, dynamic rate limit, UpdateUrlRequestBuilder chain, and build_update() return) is cohesive and makes good use of the fluent builder pattern and centralized validation/ownership checks. No functional issues spotted beyond the doc vs behavior nits already mentioned.


129-208: Status-only endpoint is well-scoped and safe.

Limiting the payload to {"status": payload.get("status")} before building ensures other fields can’t be mutated via this route, and then reusing UpdateUrlRequestBuilder + parse_status_change() keeps ownership and validation consistent with the main update endpoint.

builders/base.py (3)

119-135: Password handling and validation are consistent with the documented rules.

validate_password() cleanly supports both “set new password” and “remove password” (via null/empty), and delegates complexity checks to utils.url_utils.validate_password with clear error messaging and logging. This matches the API docs in update_url_v1.


144-161: parse_max_clicks behavior matches the updated API docs.

The parser:

  • Treats None as “no limit” by setting self.max_clicks = None.
  • Accepts only integers > 0 and returns a structured 400 for invalid inputs, logging the raw value.

This is consistent with the documented “Must be positive integer to set; set to null to remove limit” semantics and avoids the earlier ambiguity around 0.


198-209: Owner ObjectId normalization helper is safe but may be redundant.

_ensure_owner_object_id() defensively converts self.owner_id to an ObjectId, but resolve_owner_id_from_request() already returns an ObjectId for both API key and JWT flows. Keeping this helper is harmless and can protect future call sites that might set owner_id differently, but today it’s largely a no‑op.

Comment thread api/v1/management.py
Comment thread blueprints/auth.py Outdated
Comment thread blueprints/oauth.py Outdated
Comment thread builders/base.py
Comment thread builders/base.py
Comment thread static/js/dashboard/keys.js
Comment thread static/js/dashboard/keys.js
Comment thread static/js/header.js
Comment thread static/js/header.js

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (11)
blueprints/url_shortener.py (2)

306-327: V2-first result lookup with safe legacy fallback looks correct

The updated result():

  • Tries get_url_v2_by_alias(short_code) and only flips v2 = True when a V2 document is actually found.
  • Falls back to legacy load_url(short_code) when no V2 match exists.
  • Derives short_code from url_data["alias"] for V2 and from url_data["_id"] for legacy/emoji docs.

This fixes the earlier legacy KeyError risk and keeps the rendered short URL consistent across schemas.


346-397: Add defensive error handling around metrics DB calls

The new metrics query correctly:

  • Aggregates v1 totals via METRIC_PIPELINE_V1,
  • Counts v2 URLs and time-series clicks,
  • Combines them into a single payload and logs timing.

However, the three Mongo operations (aggregate, count_documents on urls_v2_collection and clicks_collection) still run without any error handling. If any of these fail (transient network issue, collection missing, etc.), the exception will bubble up through dual_cache.get_or_set("metrics", query) and can both break the endpoint and potentially cache a failure.

Consider wrapping the DB calls in a try/except that:

  • Logs a structured error (including which operation failed), and
  • Returns a safe fallback result (e.g., zeros / humanized zeros) so /metric remains responsive.

This was called out in a previous review and is still worth addressing.

api/v1/management.py (1)

62-64: Docstring for expire_after still overstates validation

The update_url_v1 docs say:

expire_after – Unix epoch seconds (must be in future)

But BaseUrlRequestBuilder.parse_expire_after() currently only checks that the value is a valid ISO8601/epoch and never enforces > now. Clients rely on the docstring, so the behavior here is misleading.

Consider either:

  • Implementing a “must be in the future” check in parse_expire_after() (and returning 400 when it’s not), or
  • Relaxing the wording to just describe the accepted formats and leaving “future” semantics to a separate layer.
static/js/dashboard/statistics.js (2)

636-665: Fix XSS risk in filter option rendering by avoiding innerHTML.

createFilterOption injects option.value directly into data-value="..." via innerHTML. Because option.value originates from API data, a crafted value with quotes can break out of the attribute and run arbitrary JS. This is the same CodeQL issue previously flagged for this area.

Build the DOM with createElement so values are not interpreted as HTML:

-    const label = document.createElement('label');
-    label.className = 'option-item';
-    const isSelected = this.filterManager.isSelected(type, option.value);
-    label.innerHTML = `
-            <input type="checkbox" ${isSelected ? 'checked' : ''} data-value="${option.value}">
-            <span class="checkmark"></span>
-            <span class="option-text">${this.escapeHtml(option.label)}</span>
-            <span class="option-count">${this.formatNumber(option.count)}</span>
-        `;
-    const checkbox = label.querySelector('input[type="checkbox"]');
+    const label = document.createElement('label');
+    label.className = 'option-item';
+    const isSelected = this.filterManager.isSelected(type, option.value);
+
+    const checkbox = document.createElement('input');
+    checkbox.type = 'checkbox';
+    checkbox.checked = isSelected;
+    checkbox.dataset.value = String(option.value);
+    label.appendChild(checkbox);
+
+    const checkmark = document.createElement('span');
+    checkmark.className = 'checkmark';
+    label.appendChild(checkmark);
+
+    const textSpan = document.createElement('span');
+    textSpan.className = 'option-text';
+    textSpan.textContent = option.label ?? '';
+    label.appendChild(textSpan);
+
+    const countSpan = document.createElement('span');
+    countSpan.className = 'option-count';
+    countSpan.textContent = this.formatNumber(option.count);
+    label.appendChild(countSpan);

The rest of the change handler can stay the same, using checkbox.dataset.value.


809-843: Use ISO country codes for filters and map clicks so backend filtering works.

populateFilterOptions sets the option value to the country name while the backend stores and filters by ISO code. Likewise, addMapClickHandler toggles filters using countryName. This breaks both dropdown- and map-driven country filters because Mongo queries receive names instead of codes. This was previously reported and still appears here.

Align values with backend expectations:

-            this.availableOptions[type] = options.map(item => {
-                if (type === 'country') {
-                    // For countries, use country name as both value and label
-                    const countryName = this.getCountryName(item[type]);
-                    return {
-                        value: countryName,
-                        label: countryName,
-                        code: item[type], // Keep the code for reference
-                        count: item.clicks || item.total_clicks || 0
-                    };
+            this.availableOptions[type] = options.map(item => {
+                if (type === 'country') {
+                    const code = item[type];
+                    const countryName = this.getCountryName(code);
+                    return {
+                        value: code,       // ISO code used in filters/API
+                        label: countryName,
+                        code,
+                        count: item.clicks || item.total_clicks || 0
+                    };

And for the map handler:

-                if (countryCode) {
-                    const countryName = this.getCountryName(countryCode);
-                    if (countryName && countryName !== 'Unknown') {
-                        this.toggleChartFilter('country', countryName);
-                    }
+                if (countryCode) {
+                    if (countryCode !== 'XX') { // ignore "Unknown"
+                        this.toggleChartFilter('country', countryCode);
+                    }

This keeps UI labels as friendly names while ensuring filter payloads use ISO codes that match the backend schema.

Also applies to: 1350-1373

builders/stats.py (1)

65-77: Support ...Z ISO timestamps in _parse_datetime so frontend ranges work reliably.

The frontend calls toISOString(), producing values like 2025-01-01T00:00:00.000Z. datetime.fromisoformat does not accept a trailing Z in many Python versions, so _parse_datetime returns None, and parse_time_range silently falls back to the default last‑7‑days window instead of honoring the user’s range. This was previously called out and still appears in this implementation.

Normalize a trailing Z to +00:00 before parsing:

     def _parse_datetime(self, value: Any) -> Optional[datetime]:
         if value is None:
             return None
         try:
             if isinstance(value, (int, float)):
                 return datetime.fromtimestamp(int(value), tz=timezone.utc)
-            dt = datetime.fromisoformat(str(value))
+            raw = str(value)
+            if raw.endswith("Z"):
+                raw = raw[:-1] + "+00:00"
+            dt = datetime.fromisoformat(raw)
             if dt.tzinfo is None:
                 dt = dt.replace(tzinfo=timezone.utc)
             return dt.astimezone(timezone.utc)
         except Exception:
             return None
Does Python's `datetime.fromisoformat` in versions 3.10–3.12 accept ISO timestamps ending with a trailing `Z` (e.g. `2025-01-01T00:00:00Z`) without pre-normalization?
api/v1/keys.py (2)

31-42: Fix expires_at ISO8601 parsing to accept Z-suffixed timestamps (and align with docs).

_parse_expires_at currently passes strings directly to datetime.fromisoformat, which rejects common ISO 8601 values like "2025-12-31T23:59:59Z". For such inputs, the function returns None and create_api_key responds with a 400, even though the docs explicitly show a Z example.

Normalize a trailing Z/z to +00:00 before parsing so UTC timestamps behave as advertised, while keeping the existing epoch-seconds handling:

 def _parse_expires_at(value: Optional[str | int | float]):
     if value is None:
         return None
     try:
         if isinstance(value, (int, float)):
             return datetime.fromtimestamp(int(value), tz=timezone.utc)
-        dt = datetime.fromisoformat(str(value))
+        raw = str(value)
+        # Normalize common ISO 8601 'Z' suffix to UTC offset for fromisoformat
+        if raw.endswith("Z") or raw.endswith("z"):
+            raw = raw[:-1] + "+00:00"
+        dt = datetime.fromisoformat(raw)
         if dt.tzinfo is None:
             dt = dt.replace(tzinfo=timezone.utc)
         return dt.astimezone(timezone.utc)
     except Exception:
         return None

Also applies to: 79-82, 85-91


220-228: Normalize datetimes to UTC before calling .timestamp() (especially in list_api_keys).

create_api_key uses doc["created_at"] which is stored as datetime.now(timezone.utc), so int(doc["created_at"].timestamp()) is safe. However, list_api_keys reads created_at/expires_at from MongoDB and calls .timestamp() directly; older records or driver defaults may be naive datetimes. In that case, .timestamp() interprets them in the server’s local timezone, shifting API responses.

Normalize to UTC before converting, similar to _parse_expires_at:

-                "created_at": int(k.get("created_at").timestamp())
-                if k.get("created_at")
-                else None,
-                "expires_at": int(k.get("expires_at").timestamp())
-                if k.get("expires_at")
-                else None,
+                "created_at": (
+                    int(
+                        (k.get("created_at").replace(tzinfo=timezone.utc)
+                         if k.get("created_at") and k.get("created_at").tzinfo is None
+                         else k.get("created_at").astimezone(timezone.utc)
+                        ).timestamp()
+                    )
+                    if k.get("created_at")
+                    else None
+                ),
+                "expires_at": (
+                    int(
+                        (k.get("expires_at").replace(tzinfo=timezone.utc)
+                         if k.get("expires_at") and k.get("expires_at").tzinfo is None
+                         else k.get("expires_at").astimezone(timezone.utc)
+                        ).timestamp()
+                    )
+                    if k.get("expires_at")
+                    else None
+                ),

(You may want to factor this into a small helper to reduce duplication.)

Also applies to: 314-328

main.py (2)

36-38: Require a strong FLASK_SECRET_KEY in production instead of making it optional.

Right now the app only sets app.secret_key when FLASK_SECRET_KEY is present; if it’s missing, Flask falls back to an insecure default. Given this key underpins cookies, sessions, and CSRF/JWT-related features, production should fail fast when it’s not configured (and ideally enforce a minimum length/entropy).

Consider something like:

-flask_secret = os.getenv("FLASK_SECRET_KEY")
-if flask_secret:
-    app.secret_key = flask_secret
+flask_secret = os.getenv("FLASK_SECRET_KEY")
+env = os.getenv("ENV", "development")
+if env == "production":
+    if not flask_secret or len(flask_secret) < 32:
+        raise RuntimeError(
+            "FLASK_SECRET_KEY must be set to a strong secret in production"
+        )
+    app.secret_key = flask_secret
+else:
+    # Dev/test: generate a temporary secret if none provided, but log loudly
+    if not flask_secret:
+        log.warning(
+            "using_ephemeral_flask_secret_key", env=env
+        )
+        flask_secret = os.urandom(32).hex()
+    app.secret_key = flask_secret

40-41: Lock down credentialed CORS to an explicit allowlist of trusted origins.

CORS(app, supports_credentials=True) without an origins/resources allowlist mirrors any origin and still sends cookies/authorization headers. That effectively lets any website issue authenticated requests on behalf of logged-in users (classic cross-origin abuse).

Restrict credentialed CORS to a configured set of dashboard/API origins and disable credentials otherwise, e.g.:

-# Enable credentials so refresh cookies can be sent cross-origin from frontend
-CORS(app, supports_credentials=True)
+# Configure CORS: credentials only for explicit trusted origins
+allowed_origins = os.getenv("DASHBOARD_ALLOWED_ORIGINS", "").split(",")
+allowed_origins = [origin.strip() for origin in allowed_origins if origin.strip()]
+if allowed_origins:
+    CORS(app, supports_credentials=True, origins=allowed_origins)
+else:
+    # Safe default: no credentialed cross-origin requests
+    CORS(app, supports_credentials=False)
blueprints/oauth.py (1)

51-62: OAuth state still lacks integrity protection; sign or persist it server-side.

All login/link endpoints generate state via generate_oauth_state(...) and validate it with verify_oauth_state(...). From the referenced utils:

  • generate_oauth_state builds a querystring with provider/action/nonce/timestamp/user_id and returns it as-is.
  • There is no HMAC/JWT signature or server-side nonce store; verification only re-parses the string and checks provider + age.

That means a malicious site can still craft arbitrary state values that pass verify_oauth_state (e.g., with forged action/user_id), enabling login/link CSRF scenarios if combined with a stolen/abused OAuth code.

To harden this:

  • Either sign the entire state payload with a server-only secret (HMAC/JWT) and only accept tokens with a valid signature and reasonable expiry, or
  • Persist the nonce server-side (e.g., in Redis or session) at state generation, then on callback ensure the nonce exists and belongs to this session/user, and delete it on use.

Until one of these is implemented, the extra state checks here don’t materially prevent tampering beyond what the OAuth library’s own state handling might do.

Also applies to: 295-323, 325-336, 570-597, 600-611, 843-870

🧹 Nitpick comments (7)
blueprints/limiter.py (1)

30-57: Dynamic limits and keying look good; consider minor refinements

The auth-aware limits and keying strategy are sound:

  • Authenticated users get the higher authenticated limits; anonymous requests get the anonymous limits (e.g., 0 per minute when you want to force auth).
  • Rate-limiting buckets by user id when authenticated, otherwise by a SHA-256 hash of the API key token, then by IP. This avoids storing raw key prefixes while still being deterministic.

Two small improvements you might consider:

  1. Avoid resolving ownership twice per request
    Both dynamic_limit_for_request() and rate_limit_key_for_request() call resolve_owner_id_from_request(). If that function hits MongoDB (e.g., API key path), you’re doing redundant work. Caching the owner id in g or a similar request-local store inside resolve_owner_id_from_request() would let both call sites reuse it cheaply.

  2. Update the docstring to match behavior
    The rate_limit_key_for_request() docstring still says “API key prefix,” but you now use a hashed token. Adjusting the wording to “hashed API key” will prevent confusion when debugging limiter keys.

static/js/auth.js (2)

219-237: Avoid interpolating unescaped strings into innerHTML in showPasswordRequirements.

showPasswordRequirements builds HTML using template strings and injects each req directly into innerHTML. Today those messages are static, but if backend-provided messages ever include user-controlled content, this becomes an XSS sink. Prefer DOM construction or at least escaping:

-        const requirementsList = missingRequirements.map(req =>
-            `<li style="margin: 4px 0;"><span style="color: #ef4444; margin-right: 8px;">✗</span>${req}</li>`
-        ).join('');
-
-        errorEl.innerHTML = `
-            <div style="text-align: left;">
-                <div style="margin-bottom: 8px; font-weight: 500;">Password requirements not met:</div>
-                <ul style="margin: 0; padding-left: 0; list-style: none;">
-                    ${requirementsList}
-                </ul>
-            </div>
-        `;
+        errorEl.textContent = '';
+        const wrapper = document.createElement('div');
+        wrapper.style.textAlign = 'left';
+        const title = document.createElement('div');
+        title.style.marginBottom = '8px';
+        title.style.fontWeight = '500';
+        title.textContent = 'Password requirements not met:';
+        const list = document.createElement('ul');
+        list.style.margin = '0';
+        list.style.paddingLeft = '0';
+        list.style.listStyle = 'none';
+        for (const req of missingRequirements) {
+            const li = document.createElement('li');
+            li.style.margin = '4px 0';
+            const icon = document.createElement('span');
+            icon.style.color = '#ef4444';
+            icon.style.marginRight = '8px';
+            icon.textContent = '✗';
+            li.appendChild(icon);
+            li.appendChild(document.createTextNode(req));
+            list.appendChild(li);
+        }
+        wrapper.appendChild(title);
+        wrapper.appendChild(list);
+        errorEl.appendChild(wrapper);
         errorEl.style.display = 'block';

255-273: Guard DOM lookups in submitAuth to avoid runtime errors.

submitAuth assumes authEmail / authPassword always exist; if the function is ever reused on a page without those elements, it will throw before any error UI is shown. A small guard improves robustness:

 async function submitAuth() {
-    const email = document.getElementById('authEmail').value.trim();
-    const password = document.getElementById('authPassword').value;
+    const emailInput = document.getElementById('authEmail');
+    const passwordInput = document.getElementById('authPassword');
+    if (!emailInput || !passwordInput) {
+        showAuthError('Authentication form is not available on this page.');
+        return;
+    }
+    const email = emailInput.value.trim();
+    const password = passwordInput.value;
.env.example (2)

12-15: Quote Sentry sampling rates to satisfy dotenv linters.

dotenv-linter flags the unquoted float values for Sentry sample rates. Quoting them keeps tooling happy without changing semantics:

-SENTRY_DSN=""                          # Leave empty to disable Sentry
-SENTRY_SEND_PII="false"                # Send user emails/IPs (consider GDPR implications)
-SENTRY_TRACES_SAMPLE_RATE=0.1       # % of transactions to capture (1.0 in dev, 0.05-0.1 in prod to reduce costs)
-SENTRY_PROFILE_SAMPLE_RATE=0.05     # % of profiling sessions to capture (1.0 in dev, 0.01-0.05 in prod, very expensive)
+SENTRY_DSN=""                          # Leave empty to disable Sentry
+SENTRY_SEND_PII="false"                # Send user emails/IPs (consider GDPR implications)
+SENTRY_TRACES_SAMPLE_RATE="0.1"       # % of transactions to capture (1.0 in dev, 0.05-0.1 in prod to reduce costs)
+SENTRY_PROFILE_SAMPLE_RATE="0.05"     # % of profiling sessions to capture (1.0 in dev, 0.01-0.05 in prod, very expensive)

44-46: Adjust JWT key placeholders to avoid secret-scanner false positives.

Static analysis (Gitleaks) flags JWT_PRIVATE_KEY as a private key because the placeholder includes real PEM headers. To reduce noise and prevent accidental real-key commits, consider switching to explicit placeholders rather than PEM-like content:

-JWT_PRIVATE_KEY="-----BEGIN PRIVATE KEY-----\n.....\n-----END PRIVATE KEY-----"
-JWT_PUBLIC_KEY="-----BEGIN PUBLIC KEY-----\n.....\n-----END PUBLIC KEY-----"
+JWT_PRIVATE_KEY="YOUR_JWT_PRIVATE_KEY_PEM"
+JWT_PUBLIC_KEY="YOUR_JWT_PUBLIC_KEY_PEM"

Add comments above with the existing OpenSSL commands so users still know how to generate proper keys.

builders/update.py (1)

84-151: Defensively guard build_update against missing existing_doc.

build_update assumes load_and_validate_ownership() has run and self.existing_doc is a dict. If a future caller forgets that step, accesses like self.existing_doc.get("long_url") will raise and return a 500 instead of a structured error.

A small defensive check keeps the builder safer to use:

     def build_update(self) -> tuple[Response, int]:
         """Execute the update operation"""
         if self.error is not None:
             return self.error
+
+        if self.existing_doc is None:
+            log.error(
+                "url_update_missing_existing_doc",
+                url_id=self.url_id,
+                owner_id=str(self.owner_id) if self.owner_id else None,
+            )
+            return jsonify({"error": "URL not loaded for update"}), 500
main.py (1)

135-141: Avoid using app.run with host="0.0.0.0" as a production server.

Binding the Flask debug server to 0.0.0.0 is fine for containers/dev, but it’s not intended as a hardened production server. Static analysis already flagged this; consider either:

  • Using Gunicorn/uwsgi/etc. for production and keeping this block strictly for local/dev, or
  • Guarding the app.run call so it never executes when ENV == "production".

For example:

-if __name__ == "__main__":
-    app.run(
-        host="0.0.0.0",
-        port=8000,
-        use_reloader=os.getenv("ENV") != "production",
-        debug=os.getenv("ENV") != "production",
-    )
+if __name__ == "__main__" and os.getenv("ENV") != "production":
+    app.run(
+        host="0.0.0.0",
+        port=8000,
+        use_reloader=True,
+        debug=True,
+    )
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6dbee0a and 604873e.

📒 Files selected for processing (16)
  • .env.example (1 hunks)
  • .gitignore (2 hunks)
  • README.md (1 hunks)
  • api/v1/keys.py (1 hunks)
  • api/v1/management.py (1 hunks)
  • api/v1/shorten.py (1 hunks)
  • blueprints/auth.py (1 hunks)
  • blueprints/contact.py (5 hunks)
  • blueprints/limiter.py (2 hunks)
  • blueprints/oauth.py (1 hunks)
  • blueprints/url_shortener.py (10 hunks)
  • builders/stats.py (1 hunks)
  • builders/update.py (1 hunks)
  • main.py (5 hunks)
  • static/js/auth.js (1 hunks)
  • static/js/dashboard/statistics.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • .gitignore
  • api/v1/shorten.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-15T12:48:51.242Z
Learnt from: Zingzy
Repo: spoo-me/url-shortener PR: 94
File: api/v1/management.py:47-68
Timestamp: 2025-11-15T12:48:51.242Z
Learning: In the URL shortener API at api/v1/management.py, the default values for URL settings are: block_bots defaults to false (bot blocking disabled), and private_stats defaults to true for authenticated users (statistics are private). When null or missing values are provided for these fields, they are coerced to their respective defaults rather than being "removed."

Applied to files:

  • api/v1/management.py
📚 Learning: 2025-11-10T06:17:02.860Z
Learnt from: Zingzy
Repo: spoo-me/url-shortener PR: 94
File: templates/base.html:7-7
Timestamp: 2025-11-10T06:17:02.860Z
Learning: In the spoo.me URL shortener project, hardcoded production URLs are preferred over Flask's `url_for()` helper for performance reasons, particularly in frequently-rendered templates like base.html. Avoid suggesting `url_for()` replacements for static asset URLs.

Applied to files:

  • blueprints/url_shortener.py
🧬 Code graph analysis (11)
blueprints/oauth.py (5)
utils/logger.py (1)
  • get_logger (19-34)
utils/auth_utils.py (5)
  • generate_access_jwt (63-79)
  • generate_refresh_jwt (91-109)
  • set_refresh_cookie (126-138)
  • set_access_cookie (155-167)
  • requires_auth (184-270)
utils/mongo_utils.py (2)
  • get_user_by_email (126-131)
  • get_user_by_id (149-154)
utils/oauth_utils.py (13)
  • init_oauth (23-111)
  • generate_oauth_state (114-152)
  • verify_oauth_state (155-189)
  • extract_user_info_from_google (192-209)
  • extract_user_info_from_github (212-251)
  • extract_user_info_from_discord (254-291)
  • find_user_by_provider (294-314)
  • create_oauth_user (317-374)
  • link_provider_to_user (377-436)
  • can_auto_link_accounts (439-469)
  • update_user_last_login (472-491)
  • get_oauth_redirect_url (494-522)
  • OAuthProviders (16-20)
utils/email_service.py (1)
  • send_welcome_email (226-275)
builders/update.py (4)
utils/logger.py (1)
  • get_logger (19-34)
builders/base.py (5)
  • BaseUrlRequestBuilder (22-209)
  • _fail (46-48)
  • _ensure_owner_object_id (198-209)
  • validate_long_url (68-93)
  • validate_alias (95-117)
utils/url_utils.py (1)
  • validate_alias (134-136)
cache/cache_url.py (1)
  • invalidate_url_cache (85-103)
api/v1/management.py (4)
blueprints/limiter.py (2)
  • dynamic_limit_for_request (30-43)
  • rate_limit_key_for_request (46-57)
builders/update.py (6)
  • UpdateUrlRequestBuilder (14-199)
  • load_and_validate_ownership (22-50)
  • validate_long_url_if_present (52-58)
  • validate_alias_custom (60-71)
  • build_update (84-199)
  • parse_status_change (73-82)
builders/base.py (6)
  • parse_auth_scope (50-66)
  • validate_password (119-134)
  • parse_max_clicks (144-161)
  • parse_expire_after (163-186)
  • parse_block_bots (136-142)
  • parse_private_stats (188-196)
cache/cache_url.py (1)
  • invalidate_url_cache (85-103)
blueprints/limiter.py (3)
utils/logger.py (1)
  • get_logger (19-34)
utils/auth_utils.py (1)
  • resolve_owner_id_from_request (297-405)
utils/url_utils.py (1)
  • get_client_ip (48-66)
builders/stats.py (5)
utils/mongo_utils.py (1)
  • check_url_stats_privacy (323-348)
utils/aggregation_strategies.py (21)
  • AggregationStrategyFactory (459-485)
  • get (474-480)
  • build_pipeline (22-24)
  • build_pipeline (71-100)
  • build_pipeline (236-248)
  • build_pipeline (268-280)
  • build_pipeline (300-312)
  • build_pipeline (332-344)
  • build_pipeline (366-378)
  • build_pipeline (398-410)
  • build_pipeline (430-442)
  • format_results (27-29)
  • format_results (102-144)
  • format_results (250-258)
  • format_results (282-290)
  • format_results (314-322)
  • format_results (346-356)
  • format_results (380-388)
  • format_results (412-420)
  • format_results (444-452)
  • get_bucket_info (214-230)
utils/query_builder.py (6)
  • StatsQueryBuilderFactory (107-134)
  • StatsQueryBuilder (10-104)
  • for_user_stats (111-121)
  • for_anonymous_stats (124-134)
  • with_filters (46-52)
  • build (54-104)
utils/stats_utils.py (2)
  • format_stats_response_with_metadata (166-225)
  • validate_date_range (228-264)
utils/logger.py (2)
  • get_logger (19-34)
  • should_sample (37-69)
main.py (6)
blueprints/oauth.py (1)
  • init_oauth_for_app (44-48)
utils/mongo_utils.py (1)
  • ensure_indexes (397-464)
utils/log_context.py (1)
  • setup_logging_middleware (94-178)
utils/logger.py (2)
  • get_logger (19-34)
  • hash_ip (72-95)
utils/url_utils.py (1)
  • get_client_ip (48-66)
utils/auth_utils.py (1)
  • resolve_owner_id_from_request (297-405)
blueprints/contact.py (2)
utils/mongo_utils.py (2)
  • check_if_slug_exists (67-73)
  • check_if_v2_alias_exists (189-194)
utils/logger.py (1)
  • get_logger (19-34)
static/js/auth.js (1)
blueprints/auth.py (1)
  • logout (115-123)
api/v1/keys.py (5)
utils/auth_utils.py (1)
  • requires_auth (184-270)
utils/logger.py (1)
  • get_logger (19-34)
utils/mongo_utils.py (3)
  • insert_api_key (354-359)
  • list_api_keys_by_user (370-378)
  • revoke_api_key_by_id (381-394)
blueprints/limiter.py (1)
  • rate_limit_key_for_request (46-57)
static/js/dashboard/keys.js (4)
  • body (232-237)
  • name (214-214)
  • description (215-215)
  • scopes (219-219)
blueprints/auth.py (7)
blueprints/limiter.py (1)
  • rate_limit_key_for_request (46-57)
utils/auth_utils.py (11)
  • verify_password (55-60)
  • hash_password (51-52)
  • generate_access_jwt (63-79)
  • generate_refresh_jwt (91-109)
  • verify_refresh_jwt (112-123)
  • set_refresh_cookie (126-138)
  • set_access_cookie (155-167)
  • clear_refresh_cookie (141-152)
  • clear_access_cookie (170-181)
  • requires_auth (184-270)
  • get_user_profile (408-445)
utils/url_utils.py (2)
  • validate_password (69-86)
  • get_client_ip (48-66)
utils/password_utils.py (1)
  • validate_password (5-82)
utils/mongo_utils.py (2)
  • get_user_by_email (126-131)
  • get_user_by_id (149-154)
utils/verification_utils.py (4)
  • create_email_verification_otp (75-146)
  • create_password_reset_otp (149-223)
  • verify_otp (226-326)
  • is_rate_limited (329-344)
utils/email_service.py (3)
  • send_verification_email (142-182)
  • send_welcome_email (226-275)
  • send_password_reset_email (184-224)
blueprints/url_shortener.py (3)
utils/mongo_utils.py (3)
  • get_url_v2_by_alias (182-186)
  • load_emoji_url (76-81)
  • load_url (36-41)
utils/general.py (1)
  • humanize_number (22-27)
utils/logger.py (1)
  • get_logger (19-34)
🪛 ast-grep (0.39.9)
main.py

[warning] 135-140: Running flask app with host 0.0.0.0 could expose the server publicly.
Context: app.run(
host="0.0.0.0",
port=8000,
use_reloader=os.getenv("ENV") != "production",
debug=os.getenv("ENV") != "production",
)
Note: [CWE-668]: Exposure of Resource to Wrong Sphere [OWASP A01:2021]: Broken Access Control [REFERENCES]
https://owasp.org/Top10/A01_2021-Broken_Access_Control

(avoid_app_run_with_bad_host-python)

🪛 dotenv-linter (4.0.0)
.env.example

[warning] 12-12: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 14-14: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 15-15: [UnorderedKey] The SENTRY_PROFILE_SAMPLE_RATE key should go before the SENTRY_TRACES_SAMPLE_RATE key

(UnorderedKey)


[warning] 15-15: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)


[warning] 21-21: [UnorderedKey] The ENV key should go before the FLASK_SECRET_KEY key

(UnorderedKey)


[warning] 35-35: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 36-36: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 36-36: [UnorderedKey] The HCAPTCHA_SECRET key should go before the SHORTEN_API_RATE_LIMIT_PER_HOUR key

(UnorderedKey)


[warning] 40-40: [UnorderedKey] The JWT_AUDIENCE key should go before the JWT_ISSUER key

(UnorderedKey)


[warning] 41-41: [UnorderedKey] The ACCESS_TOKEN_TTL_SECONDS key should go before the JWT_AUDIENCE key

(UnorderedKey)


[warning] 43-43: [UnorderedKey] The COOKIE_SECURE key should go before the JWT_AUDIENCE key

(UnorderedKey)


[warning] 45-45: [UnorderedKey] The JWT_PUBLIC_KEY key should go before the REFRESH_TOKEN_TTL_SECONDS key

(UnorderedKey)


[warning] 46-46: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 46-46: [UnorderedKey] The JWT_SECRET key should go before the REFRESH_TOKEN_TTL_SECONDS key

(UnorderedKey)


[warning] 71-71: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 72-72: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 73-73: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 84-84: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 85-85: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 86-86: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 88-88: [ExtraBlankLine] Extra blank line detected

(ExtraBlankLine)


[warning] 97-97: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 98-98: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 99-99: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 108-108: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 109-109: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 111-111: [EndingBlankLine] No blank line at the end of the file

(EndingBlankLine)


[warning] 111-111: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 111-111: [UnorderedKey] The APP_URL key should go before the ZEPTO_API_TOKEN key

(UnorderedKey)

🪛 Gitleaks (8.29.0)
.env.example

[high] 44-45: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.

(private-key)

🔇 Additional comments (6)
README.md (1)

229-231: Footer year bump looks correct

The copyright footer now reflects 2025 and is consistent with this release cycle; no issues from a docs/API-contract perspective.

blueprints/limiter.py (1)

4-9: Limiter logger and IP whitelist are fine

Adding a module-level logger and documenting the whitelist behavior keeps behavior intact and improves observability. The whitelist check still correctly skips limits for configured IPs.

Also applies to: 20-27

blueprints/contact.py (1)

9-16: Contact/report logging and v2 checks look solid

  • Using a module-level logger and logging structured events for both contact and report flows improves observability without exposing sensitive content (only email domain and message length, plus short_code and truncated reason).
  • Switching /report to read from request.form and validating short_code against both legacy and v2 (check_if_slug_exists or check_if_v2_alias_exists) is a good hardening step and keeps the error message accurate when the code doesn’t exist.

No functional issues spotted here.

Also applies to: 64-75, 96-100, 137-172

api/v1/management.py (2)

19-27: Update URL v1 flow is wired correctly into the builder

The PATCH /urls/<url_id> endpoint:

  • Applies appropriate auth scopes (urls:manage or admin:all) and ownership validation via UpdateUrlRequestBuilder.
  • Runs the full validation chain (URL, alias, password, max_clicks, expire_after, bot/privacy flags) before applying updates.
  • Uses the dynamic limiter plus user/API-key–aware keying so authenticated users get higher quotas and anonymous calls are effectively disabled.

The overall structure and reuse of the builder look good.

Also applies to: 108-126


129-208: Status-only update and delete endpoints are cleanly implemented

  • update_url_status_v1 correctly constrains the payload to status and delegates all ownership and value checks to UpdateUrlRequestBuilder.parse_status_change().
  • delete_url_v1:
    • Validates URL ID format up front.
    • Reuses the builder for auth/ownership checks so semantics stay consistent with other v1 endpoints.
    • Logs a structured url_deleted event and invalidates the URL cache by alias, with cache errors logged but not allowed to break the 200 response.

This is a solid pattern for management endpoints.

Also applies to: 211-321

blueprints/url_shortener.py (1)

32-44: Creation logging for legacy routes is sensible

  • Adding log = get_logger(__name__) and emitting url_created events for both standard and emoji legacy routes gives you useful telemetry (alias, long_url, schema, password flag, click limit, bot-blocking) without logging secrets.
  • The url_creation_failed warnings on alias/emoji collisions should make debugging conflicts much easier.

No behavioral issues here.

Also applies to: 105-111, 175-184, 217-223, 281-290

Comment thread blueprints/auth.py
Comment thread blueprints/auth.py
Comment thread blueprints/oauth.py
Comment thread builders/stats.py
Comment thread static/js/dashboard/statistics.js
- Introduced a new V2 announcement modal to highlight new features.
- Added JavaScript functionality for modal navigation and auto-advancement.
- Included CSS for styling the announcement modal.
- Added multiple images for feature previews in the modal.
- Updated base and index templates to include the new modal and its assets.
- Removed the Product Hunt badge section from the index template.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
static/css/index.css (1)

50-51: The misleading "FireFox Fallback" comment remains unfixed.

As previously flagged, the comment on Line 50 incorrectly labels -webkit-fill-available (a WebKit property) as a Firefox fallback. The subsequent width: 100%; is the actual fallback for Firefox and non-WebKit browsers. Please update the comment for clarity:

 .alias-time-container .child {
-    width: -webkit-fill-available; /* FireFox Fallback */
+    width: -webkit-fill-available; /* Webkit browsers */
     width: 100%; /* Fallback for other browsers */
 }
🧹 Nitpick comments (3)
README.md (1)

130-135: Optional enhancement: Clarify the email provider.

The email configuration variables are provider-specific (ZOHO_MAIL_USERNAME/PASSWORD indicate Zoho mail service), but the description just says "Email Configuration". Consider adding a brief note such as "(Optional - Zoho mail)" or link to provider setup docs if other email providers are supported.

Also applies to: 207-211

static/css/v2-announcement.css (2)

39-39: Consolidate repeated font-family declarations with a CSS custom property or base rule.

The font-family 'Nata Sans', -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif is declared in three places (lines 39, 181, and implicitly inherited). Consider extracting this to a custom property or applying it to a common ancestor selector (e.g., .v2-modal-overlay) to reduce redundancy and improve maintainability.

Add a custom property to :root and reuse it:

 :root {
     --v2-bg: #050713;
     /* ... other variables ... */
+    --v2-font-family: 'Nata Sans', -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif;
 }

Then update the selectors:

 .v2-badge {
     /* ... other styles ... */
-    font-family: 'Nata Sans', -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif;
+    font-family: var(--v2-font-family);
 }

 .v2-modal {
     /* ... other styles ... */
-    font-family: 'Nata Sans', -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif;
+    font-family: var(--v2-font-family);
 }

Also applies to: 181-181


20-43: Consider extracting hardcoded positioning values into CSS variables.

Several hardcoded position and size values (e.g., -72px, -120px, 50px, 48px) are scattered throughout the stylesheet. Extracting these into CSS variables would improve consistency, aid responsive scaling, and simplify future adjustments.

Add spacing/size variables to :root and apply them:

 :root {
     --v2-bg: #050713;
     /* ... existing variables ... */
+    --v2-nav-arrow-offset: 72px;
+    --v2-nav-arrow-size: 50px;
+    --v2-modal-backdrop-inset: 120px;
 }

 .v2-prev-btn {
-    left: -72px;
+    left: calc(-1 * var(--v2-nav-arrow-offset));
 }

 .v2-next-btn.v2-nav-arrow {
-    right: -72px;
+    right: calc(-1 * var(--v2-nav-arrow-offset));
 }

 .v2-nav-arrow {
-    width: 50px;
-    height: 50px;
+    width: var(--v2-nav-arrow-size);
+    height: var(--v2-nav-arrow-size);
 }

 .v2-modal::before {
-    inset: -120px;
+    inset: calc(-1 * var(--v2-modal-backdrop-inset));
 }

Also applies to: 230-260, 597-618

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c473d0 and 6e9a4ce.

⛔ Files ignored due to path filters (21)
  • static/images/Links _ Dashboard _ spoo_me.jpeg is excluded by !**/*.jpeg
  • static/images/api_3D.png is excluded by !**/*.png
  • static/images/api_demo.jpeg is excluded by !**/*.jpeg
  • static/images/api_key_api_demo.jpeg is excluded by !**/*.jpeg
  • static/images/api_keys_demo.jpeg is excluded by !**/*.jpeg
  • static/images/api_permissions_demo.jpeg is excluded by !**/*.jpeg
  • static/images/dashboard_demo.jpeg is excluded by !**/*.jpeg
  • static/images/edit_url_demo.jpeg is excluded by !**/*.jpeg
  • static/images/geo_stats_demo.jpeg is excluded by !**/*.jpeg
  • static/images/key_3D.png is excluded by !**/*.png
  • static/images/link_3D.png is excluded by !**/*.png
  • static/images/login_3D.png is excluded by !**/*.png
  • static/images/max_clicks_demo.jpeg is excluded by !**/*.jpeg
  • static/images/party_popper_3D.png is excluded by !**/*.png
  • static/images/pause_demo.jpeg is excluded by !**/*.jpeg
  • static/images/rocket_3D.png is excluded by !**/*.png
  • static/images/signin_demo.jpeg is excluded by !**/*.jpeg
  • static/images/stats_3D.png is excluded by !**/*.png
  • static/images/stats_api_demo.jpeg is excluded by !**/*.jpeg
  • static/images/stats_demo.jpeg is excluded by !**/*.jpeg
  • static/images/time_filter_demo.jpeg is excluded by !**/*.jpeg
📒 Files selected for processing (3)
  • README.md (4 hunks)
  • static/css/index.css (2 hunks)
  • static/css/v2-announcement.css (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-10T06:17:02.860Z
Learnt from: Zingzy
Repo: spoo-me/url-shortener PR: 94
File: templates/base.html:7-7
Timestamp: 2025-11-10T06:17:02.860Z
Learning: In the spoo.me URL shortener project, hardcoded production URLs are preferred over Flask's `url_for()` helper for performance reasons, particularly in frequently-rendered templates like base.html. Avoid suggesting `url_for()` replacements for static asset URLs.

Applied to files:

  • README.md
🪛 Biome (2.1.2)
static/css/v2-announcement.css

[error] 69-69: Unexpected shorthand property mask after mask-composite

(lint/suspicious/noShorthandPropertyOverrides)

static/css/index.css

[error] 51-51: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.

width is already defined here.

Remove or rename the duplicate property to ensure consistent styling.

(lint/suspicious/noDuplicateProperties)

🔇 Additional comments (4)
static/css/index.css (2)

56-57: Good expansion of input grouping for consistency.

Adding input[type="number"] and input[type="email"] to the shared input selector ensures uniform styling across all text-like input types, which improves visual cohesion across the form. No concerns here.


491-492: Good cross-browser support for preventing text selection on SVGs.

Adding the -webkit-user-select: none; prefix alongside the standard user-select: none; ensures consistent text-selection prevention across all browsers, particularly for interactive SVG icons in the checkbox component. This improves the user experience by preventing accidental text selection during interaction.

README.md (2)

24-24: ✅ Features list accurately reflects PR additions.

The updated feature descriptions (URL Statistics with "advanced analytics", Dashboard, API Keys with rate limiting, and "No Registration Required" for public shortening) properly convey the new capabilities being introduced. The language is clear and aligns with the PR objectives.

Also applies to: 32-32, 36-37, 40-40


112-128: ✅ Environment variable documentation is consistent and complete.

Both Docker and Manual setup methods now include identical OAuth, JWT, and Email configuration sections. The OAuth credentials (Google, GitHub, Discord) correctly match the PR implementation, and the JWT secret generation guidance (openssl rand -hex 32) is helpful. The optional nature of these configs is properly communicated in the notes.

Also applies to: 189-205

Comment thread static/css/v2-announcement.css
Comment thread static/css/v2-announcement.css
Comment thread static/css/v2-announcement.css
@Zingzy
Zingzy merged commit 45e449b into main Nov 18, 2025
8 of 9 checks passed
@github-project-automation github-project-automation Bot moved this from 🏗️ In Progress to ✔️ Done in spoo.me Development Roadmap Nov 18, 2025
@Zingzy Zingzy linked an issue Nov 18, 2025 that may be closed by this pull request
6 tasks
@Zingzy
Zingzy deleted the login-system branch August 16, 2026 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Changes related to Backand/API database documentation Improvements or additions to documentation enhancement New feature or request frontend priority: high High priority tasks ✨ Refactor Pull requests that refactors the codebase

Projects

Status: ✔️ Done

3 participants