Skip to content

Commit a233ebc

Browse files
authored
Merge pull request #818 from seapagan/fix/refresh-token-max-length
fix: add max_length constraint to refresh token schema
2 parents 9fc3bb2 + 6edae7d commit a233ebc

File tree

4 files changed

+25
-13
lines changed

4 files changed

+25
-13
lines changed

SECURITY-REVIEW.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -526,12 +526,17 @@
526526

527527
### 31. Missing Max Length on Token Refresh Schema
528528

529+
> [!NOTE]
530+
> **Done**: Added `max_length=1024` to refresh token field, matching
531+
> MAX_JWT_TOKEN_LENGTH. This provides defense-in-depth validation with
532+
> Pydantic catching oversized tokens at schema level. See PR #818.
533+
529534
**Location**: `app/schemas/request/auth.py:6-9`
530535

531-
- **Issue**: Refresh token field has no length validation in schema. While the
532-
endpoint validates format (auth.py:174-181), the schema itself has no
536+
- **Issue**: Refresh token field had no length validation in schema. While the
537+
endpoint validated format (auth.py:174-181), the schema itself had no
533538
constraints.
534-
- **Fix**: Add `max_length` and `min_length` to schema for defense in depth.
539+
- **Fix**: Add `max_length` to schema for defense in depth.
535540

536541
### 32. Bcrypt Work Factor Not Configurable
537542

app/schemas/request/auth.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,17 @@
22

33
from pydantic import BaseModel, EmailStr, Field
44

5+
from app.managers.helpers import MAX_JWT_TOKEN_LENGTH
6+
57

68
class TokenRefreshRequest(BaseModel):
79
"""Request schema for refreshing a JWT token."""
810

9-
refresh: str
11+
refresh: str = Field(
12+
...,
13+
max_length=MAX_JWT_TOKEN_LENGTH,
14+
description="JWT refresh token",
15+
)
1016

1117

1218
class ForgotPasswordRequest(BaseModel):

tests/integration/test_auth_routes.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
from app.database.helpers import hash_password, verify_password
1111
from app.managers.auth import AuthManager
12-
from app.managers.helpers import MAX_JWT_TOKEN_LENGTH
1312
from app.managers.user import ErrorMessages as UserErrorMessages
1413
from app.models.enums import RoleType
1514
from app.models.user import User
@@ -456,12 +455,13 @@ async def test_verify_malformed_jwt_tokens(
456455
await test_db.commit()
457456

458457
# Test various malformed JWT formats
458+
# Note: Oversized tokens are now rejected at schema validation (422)
459+
# rather than JWT format validation (401), so they're tested separately
459460
malformed_tokens = [
460461
"not.valid!.jwt", # Special character
461462
"only.two", # Only 2 parts
462463
"four.dot.separated.parts", # 4 parts
463464
"part1.part2&admin=true.part3", # URL injection attempt
464-
"a" * (MAX_JWT_TOKEN_LENGTH + 1) + ".b.c", # Exceeds max length
465465
".part2.part3", # Empty first part
466466
"part1..part3", # Empty middle part
467467
"part1.part2.", # Empty last part
@@ -482,12 +482,13 @@ async def test_refresh_malformed_jwt_tokens(
482482
await test_db.commit()
483483

484484
# Test various malformed JWT formats
485+
# Note: Oversized tokens are now rejected at schema validation (422)
486+
# rather than JWT format validation (401), so they're tested separately
485487
malformed_tokens = [
486488
"not.valid!.jwt", # Special character
487489
"only.two", # Only 2 parts
488490
"four.dot.separated.parts", # 4 parts
489491
"part1.part2&admin=true.part3", # URL injection attempt
490-
"a" * (MAX_JWT_TOKEN_LENGTH + 1) + ".b.c", # Exceeds max length
491492
".part2.part3", # Empty first part
492493
"part1..part3", # Empty middle part
493494
"part1.part2.", # Empty last part

tests/unit/test_auth_manager.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import jwt
66
import pytest
77
from fastapi import BackgroundTasks, HTTPException, status
8+
from pydantic import ValidationError
89

910
from app.config.settings import get_settings
1011
from app.database.helpers import verify_password
@@ -582,12 +583,11 @@ async def test_refresh_oversized_token(self, test_db) -> None:
582583
"""Test refresh rejects tokens exceeding max length."""
583584
# Create a token longer than MAX_JWT_TOKEN_LENGTH
584585
oversized_token = "a" * (MAX_JWT_TOKEN_LENGTH + 1) + ".b.c"
585-
with pytest.raises(HTTPException) as exc_info:
586-
await AuthManager.refresh(
587-
TokenRefreshRequest(refresh=oversized_token), test_db
588-
)
589-
assert exc_info.value.status_code == status.HTTP_401_UNAUTHORIZED
590-
assert exc_info.value.detail == ResponseMessages.INVALID_TOKEN
586+
# Schema validation now catches this before business logic
587+
with pytest.raises(ValidationError) as exc_info:
588+
TokenRefreshRequest(refresh=oversized_token)
589+
# Verify it's a string_too_long error
590+
assert "string_too_long" in str(exc_info.value).lower()
591591

592592
@pytest.mark.asyncio
593593
async def test_refresh_url_injection_attempt(self, test_db) -> None:

0 commit comments

Comments
 (0)