|
| 1 | +# OAuth TokenHandler Enhancement - Issue #1315 |
| 2 | + |
| 3 | +## Overview |
| 4 | + |
| 5 | +This enhancement addresses GitHub issue #1315, which requested that the `TokenHandler` should check the `Authorization` header for client credentials when they are missing from the request body. |
| 6 | + |
| 7 | +## Problem |
| 8 | + |
| 9 | +Previously, the `TokenHandler` only looked for client credentials (`client_id` and `client_secret`) in the request form data. However, according to OAuth 2.0 specifications, client credentials can also be provided in the `Authorization` header using Basic authentication. When credentials were only provided in the header, the handler would throw a `ValidationError` even though valid credentials were present. |
| 10 | + |
| 11 | +## Solution |
| 12 | + |
| 13 | +The `TokenHandler.handle()` method has been enhanced to: |
| 14 | + |
| 15 | +1. **Primary**: Continue using client credentials from form data when available |
| 16 | +2. **Fallback**: Check the `Authorization` header for Basic authentication when `client_id` is missing from form data |
| 17 | +3. **Graceful degradation**: Handle malformed or invalid Authorization headers without breaking the existing flow |
| 18 | + |
| 19 | +## Implementation Details |
| 20 | + |
| 21 | +### Code Changes |
| 22 | + |
| 23 | +The enhancement was implemented in `src/mcp/server/auth/handlers/token.py`: |
| 24 | + |
| 25 | +```python |
| 26 | +async def handle(self, request: Request): |
| 27 | + try: |
| 28 | + form_data = dict(await request.form()) |
| 29 | + |
| 30 | + # Try to get client credentials from header if missing in body |
| 31 | + if "client_id" not in form_data: |
| 32 | + auth_header = request.headers.get("Authorization") |
| 33 | + if auth_header and auth_header.startswith("Basic "): |
| 34 | + encoded = auth_header.split(" ")[1] |
| 35 | + decoded = base64.b64decode(encoded).decode("utf-8") |
| 36 | + client_id, _, client_secret = decoded.partition(":") |
| 37 | + client_secret = urllib.parse.unquote(client_secret) |
| 38 | + form_data.setdefault("client_id", client_id) |
| 39 | + form_data.setdefault("client_secret", client_secret) |
| 40 | + |
| 41 | + token_request = TokenRequest.model_validate(form_data).root |
| 42 | + # ... rest of the method |
| 43 | +``` |
| 44 | + |
| 45 | +### Key Features |
| 46 | + |
| 47 | +- **Base64 Decoding**: Properly decodes Basic authentication credentials |
| 48 | +- **URL Decoding**: Handles URL-encoded client secrets (e.g., `test%2Bsecret` → `test+secret`) |
| 49 | +- **Non-intrusive**: Only activates when credentials are missing from form data |
| 50 | +- **Backward Compatible**: Existing functionality remains unchanged |
| 51 | + |
| 52 | +## Testing |
| 53 | + |
| 54 | +Comprehensive tests have been added in `tests/server/auth/test_token_handler.py` covering: |
| 55 | + |
| 56 | +1. **Form Data Credentials**: Existing functionality continues to work |
| 57 | +2. **Authorization Header Fallback**: New functionality works correctly |
| 58 | +3. **URL-encoded Secrets**: Handles special characters in client secrets |
| 59 | +4. **Invalid Headers**: Gracefully handles malformed Authorization headers |
| 60 | +5. **Refresh Token Grants**: Works with both grant types |
| 61 | +6. **Error Cases**: Proper validation when no credentials are provided |
| 62 | + |
| 63 | +### Test Coverage |
| 64 | + |
| 65 | +- ✅ `test_handle_with_form_data_credentials` |
| 66 | +- ✅ `test_handle_with_authorization_header_credentials` |
| 67 | +- ✅ `test_handle_with_authorization_header_url_encoded_secret` |
| 68 | +- ✅ `test_handle_with_invalid_authorization_header` |
| 69 | +- ✅ `test_handle_with_malformed_basic_auth` |
| 70 | +- ✅ `test_handle_with_refresh_token_grant` |
| 71 | +- ✅ `test_handle_without_credentials_fails` |
| 72 | + |
| 73 | +## OAuth 2.0 Compliance |
| 74 | + |
| 75 | +This enhancement improves compliance with OAuth 2.0 specifications by supporting both authentication methods: |
| 76 | + |
| 77 | +- **client_secret_post** (form data) - RFC 6749 Section 2.3.1 |
| 78 | +- **client_secret_basic** (Authorization header) - RFC 6749 Section 2.3.1 |
| 79 | + |
| 80 | +## Impact |
| 81 | + |
| 82 | +- **Positive**: Improves OAuth 2.0 compliance and client compatibility |
| 83 | +- **Neutral**: No breaking changes to existing functionality |
| 84 | +- **Performance**: Minimal overhead (only processes header when needed) |
| 85 | + |
| 86 | +## Files Modified |
| 87 | + |
| 88 | +1. **`src/mcp/server/auth/handlers/token.py`** - Main implementation |
| 89 | +2. **`tests/server/auth/test_token_handler.py`** - New test suite |
| 90 | + |
| 91 | +## Verification |
| 92 | + |
| 93 | +- ✅ All new tests pass |
| 94 | +- ✅ All existing tests continue to pass |
| 95 | +- ✅ Code passes linting (ruff) |
| 96 | +- ✅ Code passes type checking (pyright) |
| 97 | +- ✅ No breaking changes to existing functionality |
| 98 | + |
| 99 | +## Usage Example |
| 100 | + |
| 101 | +Clients can now use either method: |
| 102 | + |
| 103 | +**Method 1: Form Data (existing)** |
| 104 | +```http |
| 105 | +POST /token |
| 106 | +Content-Type: application/x-www-form-urlencoded |
| 107 | +
|
| 108 | +grant_type=authorization_code&code=abc123&client_id=myapp&client_secret=secret |
| 109 | +``` |
| 110 | + |
| 111 | +**Method 2: Authorization Header (new)** |
| 112 | +```http |
| 113 | +POST /token |
| 114 | +Authorization: Basic bXlhcHA6c2VjcmV0 |
| 115 | +Content-Type: application/x-www-form-urlencoded |
| 116 | +
|
| 117 | +grant_type=authorization_code&code=abc123 |
| 118 | +``` |
| 119 | + |
| 120 | +Both methods will work seamlessly with the enhanced `TokenHandler`. |
0 commit comments