Extend JWT format validation to verify and refresh endpoints#790
Merged
Conversation
This commit extends the JWT format validation helper (introduced in the password recovery feature) to the user verification and token refresh endpoints, providing consistent defense-in-depth security across all JWT-accepting endpoints. Changes: - Add JWT format validation to /verify endpoint (app/resources/auth.py) - Validates empty tokens, length (1024 char max), and JWT format - Rejects malformed tokens before cryptographic verification - Same validation strategy as reset password endpoint - Add JWT format validation to /refresh endpoint (app/managers/auth.py) - Import is_valid_jwt_format helper - Validates refresh token format before jwt.decode() - Prevents obviously invalid tokens from expensive crypto operations - Add comprehensive integration tests (tests/integration/test_auth_routes.py) - test_verify_malformed_jwt_tokens: 8 malformed token scenarios - test_refresh_malformed_jwt_tokens: 8 malformed token scenarios - Tests special chars, wrong part count, empty parts, length violations - Tests URL injection attempts (e.g., part1.part2&admin=true.part3) Security benefits: - Consistent validation across all JWT endpoints - Defense-in-depth: Multiple validation layers before token processing - Performance: Early rejection of invalid formats (fail fast) - Better error messages: "invalid format" detected before crypto errors - Attack surface reduction: Prevents malformed input from reaching jwt.decode() Coverage improvements: - auth.py manager: 36% → 55% coverage - All 25 auth integration tests pass - mypy and ruff checks pass
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Updates API reference documentation to document the new JWT format validation added to /verify and /refresh endpoints, providing consistency with the password reset endpoint documentation. Changes: - Add "Token Validation" section to /refresh endpoint - Documents 1024 char max length - Documents JWT format requirements (3 dot-separated base64url parts) - Documents empty token rejection - Documents malformed token rejection before crypto verification - Add "Security Features" section to /refresh endpoint - Format validation benefits (fast syntactic check) - Length validation (resource exhaustion prevention) - Defense-in-depth approach - Early rejection for better error messages - Cryptographic verification details - Token type checking (typ="refresh") - Add "Token Validation" section to /verify endpoint - Same validation details as refresh endpoint - Add "Security Features" section to /verify endpoint - Same security features as refresh endpoint - Additional note about URL parameter safety - Update error response documentation for /verify - Documents that malformed JWTs return 401 Unauthorized These documentation updates ensure consistency across all JWT-accepting endpoints and provide clear guidance on validation behavior and security features.
Adds unit tests for the JWT format validation logic introduced in the refresh endpoint, significantly improving test coverage and ensuring proper validation behavior at the unit level. Changes: - Add 6 new unit tests for AuthManager.refresh() validation: - test_refresh_malformed_jwt_special_chars: Tests special chars rejection - test_refresh_malformed_jwt_wrong_parts: Tests 2/4 part tokens - test_refresh_malformed_jwt_empty_parts: Tests tokens with empty parts - test_refresh_oversized_token: Tests 1024+ char length limit - test_refresh_url_injection_attempt: Tests URL injection patterns - test_refresh_valid_format_invalid_signature: Tests crypto verification Coverage improvements: - app/managers/auth.py: 36% → 86% (+50%) - Previously uncovered line 177 (InvalidTokenError handler) now covered - All 64 tests pass (39 unit + 25 integration) - mypy and ruff checks pass Test benefits: - Unit tests catch edge cases faster than integration tests - Better exception path coverage (especially InvalidTokenError) - Validates behavior at the manager level, not just endpoint level - Tests specific to JWT format validation requirements
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances security by extending JWT format validation to the user verification (/verify) and token refresh (/refresh) endpoints. The validation strategy, previously implemented for password reset, now provides consistent defense-in-depth protection across all JWT-accepting endpoints.
Key Changes:
- Added pre-validation checks (empty token, length limit, format verification) to
/verifyand/refreshendpoints before cryptographic operations - Implemented comprehensive test suites covering 8+ malformed token scenarios per endpoint, including URL injection attempts
- Updated API documentation with detailed security features and validation rules
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| app/resources/auth.py | Added JWT format validation to /verify endpoint with length and format checks before processing |
| app/managers/auth.py | Added JWT format validation to refresh() method with import of validation helper |
| tests/unit/test_auth_manager.py | Added 6 comprehensive unit tests for refresh endpoint validation covering special chars, wrong parts, empty parts, oversized tokens, URL injection, and invalid signatures |
| tests/integration/test_auth_routes.py | Added 2 integration tests covering 8 malformed token scenarios each for verify and refresh endpoints |
| docs/reference/api.md | Enhanced documentation with token validation rules and security features for both endpoints |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Eliminates magic number duplication by extracting 1024 into MAX_JWT_TOKEN_LENGTH constant in app/managers/helpers.py. Changes: - Add MAX_JWT_TOKEN_LENGTH constant - Update auth.py and resources/auth.py to use constant - Update all tests to use MAX_JWT_TOKEN_LENGTH + 1 - Replace manual character string with string.ascii_letters + string.digits Benefits: Single source of truth, better maintainability, cleaner code. All 89 tests pass, mypy and ruff checks pass. Suggested by GitHub Copilot review.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Moves JWT format validation from /verify route handler to AuthManager.verify() method to match the pattern used in AuthManager.refresh(). Changes: - Add validation to AuthManager.verify() (lines 183-191) - Remove validation from /verify route handler Benefits: Consistent architecture, manager owns validation, better unit testability, defense-in-depth regardless of caller. All 64 tests pass, coverage remains at 86% for auth manager. Addresses GitHub Copilot architectural consistency feedback.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Adds test_verify_valid_format_invalid_signature to cover the jwt.InvalidTokenError exception handler (line 242) that was previously uncovered. The test uses a JWT with valid format but invalid cryptographic signature, ensuring the exception path is exercised. Coverage: app/managers/auth.py now 100% on modified lines (86% overall). All 65 tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR extends the JWT format validation helper (introduced in the password recovery feature) to the user verification and token refresh endpoints, providing consistent defense-in-depth security across all JWT-accepting endpoints.
Changes:
Add JWT format validation to /verify endpoint (app/resources/auth.py)
Add JWT format validation to /refresh endpoint (app/managers/auth.py)
Add comprehensive integration tests (tests/integration/test_auth_routes.py)
Security benefits: