[WIP] implement oauth fix to not accept tokens from any issuer#53
Draft
martenson wants to merge 1 commit intoCESNET:mainfrom
Draft
[WIP] implement oauth fix to not accept tokens from any issuer#53martenson wants to merge 1 commit intoCESNET:mainfrom
martenson wants to merge 1 commit intoCESNET:mainfrom
Conversation
Member
Author
|
test is not running because ci needs docker credentials (!!) |
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.
LLM-based fix for token handling, to be reviewed.
OAuth2 Security Fix - Summary of Changes
Overview
Fixed a critical OAuth2 security vulnerability that allowed accepting tokens from any issuer, enabling attackers to create their own identity providers and gain unauthorized access.
Files Modified
1.
tesp_api/utils/token_validator.py(Complete Rewrite)Security Features Added:
exp,iat,sub,issclaims before proceedingPerformance Improvements:
New Helper Functions:
_get_allowed_issuers()- Retrieves and parses allowed issuers from config_get_required_audience()- Gets audience requirement from config_get_cached_jwks_client()- Caches JWKS clients with TTL2.
settings.toml(Configuration Updates)Added OAuth2 Configuration Options:
Security Hardening:
3.
.gitignore(Security)Added entries for secrets:
4.
README.md(Documentation)Added OAuth2 Configuration Documentation (lines 157-195):
.secrets.toml.exampleFiles Created
5.
.secrets.toml.example(Template)Security configuration template with examples for:
6.
requirements.txt(Dependencies)Created requirements.txt file with all project dependencies:
# Core dependencies aio_pika>=9.5.7,<10.0.0 fastapi>=0.75.1,<0.76.0 orjson>=3.6.8,<4.0.0 gunicorn>=20.1.0,<21.0.0 uvicorn>=0.17.6,<0.18.0 pydantic>=1.9.0,<2.0.0 dynaconf>=3.1.8,<4.0.0 motor>=3.0.0,<4.0.0 # Upgraded from 2.5.1 for Python 3.12 compatibility loguru>=0.6.0,<1.0.0 aiohttp>=3.13.3,<4.0.0 aioftp>=0.21.0,<1.0.0 PyMonad>=2.4.0,<3.0.0 aiobotocore>=2.4.2,<3.0.0 requests>=2.28.2,<3.0.0 pyjwt>=2.8.0,<3.0.0 cryptography>=41.0.5,<42.0.0 pytest>=7.1.1,<8.0.07.
tests/test_oauth2_security.py(Security Test Suite)Tests Created:
test_rejects_token_from_unauthorized_issuertest_rejects_token_with_none_algorithmtest_rejects_token_missing_required_claimstest_rejects_expired_tokenstest_rejects_malformed_tokentest_allows_only_secure_algorithms_by_defaulttest_empty_allowed_issuers_accepts_all_in_devtest_issuer_allow_list_formatKey Security Improvements
verify_aud=False)Test Results
8/8 OAuth2 security tests PASSED:
Production Setup Required
To enable OAuth2 in production, users must:
Critical:
oauth.allowed_issuersmust be configured whenoauth.enable=true, otherwise authentication has effectively no security boundaries.Backward Compatibility
oauth.enable = false)Verification
To verify the security fix works:
Test unauthorized issuer rejection:
iss: "https://evil.com"oauth.allowed_issuers = ["https://trusted.com"]in configOAuth2TokenErrorTest 'none' algorithm rejection:
alg: "none"and empty keyTest required claims:
suborissclaimRun the security tests:
Status
✅ COMPLETED - OAuth2 security vulnerability has been fixed and documented.