Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Server crashes on login if password is not provided in the request payload. #1209

Open
iConnell opened this issue Mar 1, 2025 · 1 comment
Labels

Comments

@iConnell
Copy link

iConnell commented Mar 1, 2025

Issue Overview

This PR fixes a bug where making a request to the login endpoint without providing the password payload causes an internal server error (500).
This happens because the existence of the password input is not checked before trying to perform actions on it.

🛑 Unhandled Missing Password Input

When a user submits a login request without a password, the server attempts to check if the password meets all the constraints without verifying if the password exists. This results in an TypeError: 'NoneType' object is not iterable.
Instead of returning a proper validation error (422 Unprocessable Entity), the server crashes with a 500 Internal Server Error.

🚨 Security & Usability Issue

Security Risk: A server crash may expose internal stack traces or system details.
User Experience: Instead of receiving a helpful validation message, users encounter an internal error.
API Consistency: Other required fields in the request model already trigger validation errors when missing, but password lacks this check.

Motivation & Context

🚀 Stability: Prevents unnecessary server crashes due to missing input.
🔍 Proper Validation: Ensures the API returns a 422 Unprocessable Entity error for missing passwords.
🔐 Security Fix: Prevents exposing stack traces or internal errors due to unhandled missing input.

Proposed Fix

Before (Bug - No Check for Missing Password):

def validate_password(cls, values: dict):
    """
    Validates passwords
    """
    if not isinstance(values, dict):
        return values
    password = values.get("password")
    email = values.get("email")
    totp_code = values.get("totp_code")

    # constraints for password
    if not any(c.islower() for c in password):
        raise ValueError("password must include at least one lowercase character")
    if not any(c.isupper() for c in password):
        raise ValueError("password must include at least one uppercase character")
    if not any(c.isdigit() for c in password):
        raise ValueError("password must include at least one digit")
    if not any(
        c in ["!", "@", "#", "$", "%", "&", "*", "?", "_", "-"] for c in password
    ):
        raise ValueError("password must include at least one special character")

After (Fix - Validate Missing Password Properly):

def validate_password(cls, values: dict):
    """
    Validates passwords
    """
    if not isinstance(values, dict):
        return values
    password = values.get("password")
    email = values.get("email")
    totp_code = values.get("totp_code")

    if password is None:
        return values

    # constraints for password
    if not any(c.islower() for c in password):
        raise ValueError("password must include at least one lowercase character")
    if not any(c.isupper() for c in password):
        raise ValueError("password must include at least one uppercase character")
    if not any(c.isdigit() for c in password):
        raise ValueError("password must include at least one digit")
    if not any(
        c in ["!", "@", "#", "$", "%", "&", "*", "?", "_", "-"] for c in password
    ):
        raise ValueError("password must include at least one special character")

🧪 How Has This Been Tested?

✅ Attempted login requests without a password and verified the request is correctly rejected with a 422 error.
✅ Tested login with a valid password to ensure normal functionality is unaffected.
✅ Ensured FastAPI’s validation properly prevents missing password crashes.
✅ Reviewed logs to confirm that no internal errors are exposed in the response.

🔄 Types of Changes

🐞 Bug Fix: Prevents login endpoint from crashing when password is missing.
🔐 Security Enhancement: Ensures proper validation before processing authentication.
🛠 Stability Improvement: Prevents unnecessary 500 Internal Server Error crashes.

✅ Checklist

  • My code follows the project’s coding style.
  • I have updated the documentation where necessary.
  • I have read the CONTRIBUTING guidelines.
  • All new and existing tests passed.

🔑 Key Notes for Reviewers

  • The login endpoint now properly handles missing passwords by returning a 422 error instead of crashing.
  • Consider reviewing other required fields in request models to ensure proper validation.

🎯 Expected Outcomes

✔ Prevents 500 Internal Server Error crashes on login.
✔ Ensures users receive a clear 422 validation error for missing passwords.
✔ Improves API reliability and security.

@iConnell
Copy link
Author

iConnell commented Mar 1, 2025

@joboy-dev @incredible-phoenix246 kindly check and approve 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants