Skip to content

Conversation

@ndigvijay
Copy link
Contributor

@ndigvijay ndigvijay commented Jul 1, 2025

🔧 Implement Pydantic-based Input Validation

Summary

Replaces manual assertion-based validation with robust Pydantic models for the /authenticate endpoint, providing better type safety, clearer error messages, and improved developer experience.

Changes Made

🆕 New Features

  • Created ValidateInputModel Pydantic class (models/validate_input_model.py)
    • Strict type validation with ConfigDict(strict=True)
    • Custom field validators for username, password, and fields
    • Automatic trimming of username whitespace
    • Validation against predefined allowed profile fields

🔄 Updated Components

  • Route Handler (app/app.py):

    • Replaced manual assertions with Pydantic model validation
    • Enhanced error handling with structured validation messages
    • Maintained backward compatibility for API responses
  • API Documentation:

    • Updated Swagger annotations to reference Pydantic validation
    • Enhanced parameter descriptions with validation details
    • Improved error response documentation
  • Dependencies (pyproject.toml):

    • Added pydantic>=2.6.0 to project dependencies

🧪 Test Updates

  • Validation Tests (tests/unit/test_validation.py):
    • Updated to expect ValidationError instead of AssertionError
    • Fixed error message assertions to match Pydantic's output
    • All 41 tests passing with new validation system

📚 Documentation

  • Developer Documentation (README.md):
    • Added comprehensive "Input Validation with Pydantic" section
    • Documented validation rules and allowed field values
    • Provided validation error examples and developer usage guide

Benefits

  • 🛡️ Enhanced Security: Strict type checking prevents type confusion attacks
  • 🎯 Better Error Messages: Clear, descriptive validation errors for API consumers
  • 🔒 Type Safety: Compile-time and runtime type validation
  • 🚀 Developer Experience: IDE support with autocomplete and type hints
  • 🧹 Code Quality: Eliminated repetitive manual validation logic

Validation Rules

Field Type Validation
username str Cannot be empty; automatically trimmed
password str Cannot be empty
profile bool Strict boolean (no string coercion)
fields Optional[list[str]] Must be from predefined allowed fields or None

Testing

  • ✅ All existing tests pass (41/41)
  • ✅ Pre-commit hooks pass (formatting, linting, type checking)
  • ✅ Validation error handling verified

@ndigvijay ndigvijay requested a review from aditeyabaral July 1, 2025 14:41
@aditeyabaral
Copy link
Member

I have not reviewed everything. Please work on these while I get back.

@achyu-dev
Copy link
Contributor

Check the new PR @aditeyabaral . All changes have been made

@aditeyabaral
Copy link
Member

aditeyabaral commented Jul 8, 2025

Changes (from my end)

  • Deprecated support for Python 3.10: Python 3.13 is now more or less stable given its has been out for 9+ months now.
  • Added models for the response as ResponseModel and profile field as ProfileModel, renamed the old model to RequestModel.
  • Updated tests to reflect changes.

aditeyabaral
aditeyabaral previously approved these changes Jul 8, 2025
@aditeyabaral
Copy link
Member

The rest of the changes look good, and this can be merged. Please note the following:

  1. We will need to update the model classes in the next PR to add the following attributes: example, default. This is not just for code readability, but OpenAPI (and thus FastAPI) directly loads these into the documentation generated. I will leave this to @silicoflare since he has already started working on it.
  2. We should implement some custom exception classes. I will raise an issue about this soon, but for reference, please check pesuacademy-py, which implements some custom classes like CSRFTokenException.
  3. The README might require updates. But this is of lower priority at the moment, since PESUAuth v2.0: Migrate API Server from Flask + Flasgger to FastAPI #45 is needed asap.

@silicoflare
Copy link
Contributor

@aditeyabaral Should I merge this PR and get started on FastAPI??

@aditeyabaral
Copy link
Member

I will merge it in an hour or so. Since we migrated to a new org, I just want to ensure that the CI/CD pipeline isn't broken first.

@ndigvijay ndigvijay merged commit a355915 into pesu-dev:main Jul 8, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants