-
Notifications
You must be signed in to change notification settings - Fork 0
Refac/improve api auth #42
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
Changes from all commits
98b648c
a32e0cf
2544602
b4ed409
b43539c
1d975e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,128 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||
| from datetime import UTC, datetime, timedelta | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| import httpx | ||||||||||||||||||||||||||||||||||||||||||||||||
| import jwt | ||||||||||||||||||||||||||||||||||||||||||||||||
| from fastapi import Depends, HTTPException, Response, status | ||||||||||||||||||||||||||||||||||||||||||||||||
| from fastapi.responses import RedirectResponse | ||||||||||||||||||||||||||||||||||||||||||||||||
| from fastapi.security import APIKeyHeader | ||||||||||||||||||||||||||||||||||||||||||||||||
| from starlette.requests import Request | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| from framex.config import settings | ||||||||||||||||||||||||||||||||||||||||||||||||
| from framex.consts import DOCS_URL | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| api_key_header = APIKeyHeader(name="Authorization", auto_error=False) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| def create_jwt(payload: dict) -> str: | ||||||||||||||||||||||||||||||||||||||||||||||||
| if not settings.auth.oauth: | ||||||||||||||||||||||||||||||||||||||||||||||||
| raise RuntimeError("OAuth not configured") | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| now_utc = datetime.now(UTC) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| payload.update( | ||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||
| "iat": int(now_utc.timestamp()), | ||||||||||||||||||||||||||||||||||||||||||||||||
| "exp": int((now_utc + timedelta(hours=24)).timestamp()), | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||
| return jwt.encode(payload, settings.auth.oauth.jwt_secret, algorithm=settings.auth.oauth.jwt_algorithm) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| def auth_jwt(request: Request) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||
| if not settings.auth.oauth: | ||||||||||||||||||||||||||||||||||||||||||||||||
| return False | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| token = request.cookies.get("token") | ||||||||||||||||||||||||||||||||||||||||||||||||
| if not token: | ||||||||||||||||||||||||||||||||||||||||||||||||
| return False | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||
| jwt.decode( | ||||||||||||||||||||||||||||||||||||||||||||||||
| token, | ||||||||||||||||||||||||||||||||||||||||||||||||
| settings.auth.oauth.jwt_secret, | ||||||||||||||||||||||||||||||||||||||||||||||||
| algorithms=[settings.auth.oauth.jwt_algorithm], | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||
| return True | ||||||||||||||||||||||||||||||||||||||||||||||||
| except (jwt.InvalidTokenError, jwt.ExpiredSignatureError): | ||||||||||||||||||||||||||||||||||||||||||||||||
| return False | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| def authenticate(request: Request, api_key: str | None = Depends(api_key_header)) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||
| if settings.auth.oauth: | ||||||||||||||||||||||||||||||||||||||||||||||||
| if token := request.cookies.get("token"): | ||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||
| jwt.decode( | ||||||||||||||||||||||||||||||||||||||||||||||||
| token, | ||||||||||||||||||||||||||||||||||||||||||||||||
| settings.auth.oauth.jwt_secret, | ||||||||||||||||||||||||||||||||||||||||||||||||
| algorithms=[settings.auth.oauth.jwt_algorithm], | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||||||||||||||
| from framex.log import logger | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| logger.warning(f"JWT decode failed: {e}") | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+61
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace broad exception catch with specific JWT exceptions. Catching 🔎 Proposed fix- except Exception as e:
+ except (jwt.InvalidTokenError, jwt.ExpiredSignatureError) as e:
from framex.log import logger
logger.warning(f"JWT decode failed: {e}")📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.14.10)61-61: Do not catch blind exception: (BLE001) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| if api_key and api_key in (settings.auth.get_auth_keys(request.url.path) or []): | ||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| raise HTTPException( | ||||||||||||||||||||||||||||||||||||||||||||||||
| status_code=status.HTTP_302_FOUND, | ||||||||||||||||||||||||||||||||||||||||||||||||
| headers={ | ||||||||||||||||||||||||||||||||||||||||||||||||
| "Location": ( | ||||||||||||||||||||||||||||||||||||||||||||||||
| f"{settings.auth.oauth.authorization_url}" | ||||||||||||||||||||||||||||||||||||||||||||||||
| f"?client_id={settings.auth.oauth.client_id}" | ||||||||||||||||||||||||||||||||||||||||||||||||
| "&response_type=code" | ||||||||||||||||||||||||||||||||||||||||||||||||
| f"&redirect_uri={settings.auth.oauth.call_back_url}" | ||||||||||||||||||||||||||||||||||||||||||||||||
| "&scope=read_user" | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+50
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Search for settings.auth.oauth configuration and usage patterns
rg -n "settings\.auth\.oauth" --type py -A 3 -B 3Repository: touale/FrameX-kit Length of output: 9430 🏁 Script executed: #!/bin/bash
# Search for API key authentication and get_auth_keys usage
rg -n "get_auth_keys" --type py -A 3 -B 3Repository: touale/FrameX-kit Length of output: 4485 🏁 Script executed: #!/bin/bash
# Look for the settings definition and OAuth configuration
fd -e py | xargs grep -l "class.*Settings\|auth.*oauth" | head -20Repository: touale/FrameX-kit Length of output: 166 🏁 Script executed: #!/bin/bash
# Search for authenticate function calls and other auth functions
rg -n "def authenticate" --type py -A 10Repository: touale/FrameX-kit Length of output: 782 🏁 Script executed: #!/bin/bash
# Check the full authenticate function and surrounding context
sed -n '50,82p' src/framex/driver/auth.pyRepository: touale/FrameX-kit Length of output: 1181 🏁 Script executed: #!/bin/bash
# Look for how api_key_header dependency works
rg -n "api_key_header" --type py -B 5 -A 5Repository: touale/FrameX-kit Length of output: 3206 🏁 Script executed: #!/bin/bash
# Search for any tests or logic handling authenticate when oauth is None
rg -n "def test.*authenticate\|authenticate.*oauth.*None\|settings\.auth\s*=" --type py -A 3 -B 1Repository: touale/FrameX-kit Length of output: 43 🏁 Script executed: #!/bin/bash
# Look at how authenticate is used as a dependency in the application
rg -n "Depends(authenticate)" --type py -B 3 -A 3Repository: touale/FrameX-kit Length of output: 43 🏁 Script executed: #!/bin/bash
# Check the settings.auth configuration to understand defaults
cat src/framex/config.py | head -150Repository: touale/FrameX-kit Length of output: 4600 🏁 Script executed: #!/bin/bash
# Get the full settings.auth configuration definition
rg -n "class AuthConfig\|oauth\s*:" src/framex/config.py -A 20 | head -80Repository: touale/FrameX-kit Length of output: 43 API key validation is completely skipped when OAuth is not configured. The 🧰 Tools🪛 Ruff (0.14.10)59-59: Consider moving this statement to an (TRY300) 61-61: Do not catch blind exception: (BLE001) |
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| async def oauth_callback(code: str) -> Response: | ||||||||||||||||||||||||||||||||||||||||||||||||
| if not settings.auth.oauth: # pragma: no cover | ||||||||||||||||||||||||||||||||||||||||||||||||
| raise RuntimeError("OAuth not configured") | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| async with httpx.AsyncClient() as client: | ||||||||||||||||||||||||||||||||||||||||||||||||
| resp = await client.post( | ||||||||||||||||||||||||||||||||||||||||||||||||
| settings.auth.oauth.token_url, | ||||||||||||||||||||||||||||||||||||||||||||||||
| data={ | ||||||||||||||||||||||||||||||||||||||||||||||||
| "client_id": settings.auth.oauth.client_id, | ||||||||||||||||||||||||||||||||||||||||||||||||
| "client_secret": settings.auth.oauth.client_secret, | ||||||||||||||||||||||||||||||||||||||||||||||||
| "grant_type": "authorization_code", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "code": code, | ||||||||||||||||||||||||||||||||||||||||||||||||
| "redirect_uri": settings.auth.oauth.call_back_url, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+87
to
+97
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add timeout to token exchange POST request. The POST request to the token URL (line 88) does not have a timeout, while the subsequent GET request to user_info_url does (line 107). Without a timeout, the request can hang indefinitely, degrading reliability and potentially causing resource exhaustion. 🔎 Proposed fix resp = await client.post(
settings.auth.oauth.token_url,
data={
"client_id": settings.auth.oauth.client_id,
"client_secret": settings.auth.oauth.client_secret,
"grant_type": "authorization_code",
"code": code,
"redirect_uri": settings.auth.oauth.call_back_url,
},
+ timeout=10,
)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
| resp.raise_for_status() | ||||||||||||||||||||||||||||||||||||||||||||||||
| token_resp = resp.json() | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| if not (auth_token := token_resp.get("access_token")): # pragma: no cover | ||||||||||||||||||||||||||||||||||||||||||||||||
| raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="GitLab token exchange failed") | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| resp = await client.get( | ||||||||||||||||||||||||||||||||||||||||||||||||
| settings.auth.oauth.user_info_url, | ||||||||||||||||||||||||||||||||||||||||||||||||
| headers={"Authorization": f"Bearer {auth_token}"}, | ||||||||||||||||||||||||||||||||||||||||||||||||
| timeout=10, | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||
| resp.raise_for_status() | ||||||||||||||||||||||||||||||||||||||||||||||||
| user_resp = resp.json() | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| if not (username := user_resp.get("username")): # pragma: no cover | ||||||||||||||||||||||||||||||||||||||||||||||||
| raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Failed to get GitLab user") | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| user_info = { | ||||||||||||||||||||||||||||||||||||||||||||||||
| "message": f"Welcome {username}", | ||||||||||||||||||||||||||||||||||||||||||||||||
| "username": username, | ||||||||||||||||||||||||||||||||||||||||||||||||
| "email": user_resp.get("email"), | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| res = RedirectResponse(url=DOCS_URL, status_code=status.HTTP_302_FOUND) | ||||||||||||||||||||||||||||||||||||||||||||||||
| res.set_cookie( | ||||||||||||||||||||||||||||||||||||||||||||||||
| "token", | ||||||||||||||||||||||||||||||||||||||||||||||||
| create_jwt(user_info), | ||||||||||||||||||||||||||||||||||||||||||||||||
| httponly=True, | ||||||||||||||||||||||||||||||||||||||||||||||||
| samesite="lax", | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+122
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add The JWT cookie is set without the 🔎 Proposed fix res.set_cookie(
"token",
create_jwt(user_info),
httponly=True,
samesite="lax",
+ secure=True,
)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
| return res | ||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
PyJWT latest version PyPI 2.10.1💡 Result:
The latest PyJWT release on PyPI is 2.10.1 (uploaded Nov 28, 2024). [1][2]
Sources:
[1] PyPI project page for PyJWT.
[2] PyJWT changelog.
🌐 Web query:
PyJWT security vulnerabilities CVE 2024 2025💡 Result:
Summary — known CVEs for PyJWT (2024–2025)
CVE-2024-53861 — Issuer (iss) partial-comparison bug introduced in PyJWT 2.10.0 that allowed partial matches (e.g., "abc" matched "abc"). Patched in 2.10.1; upgrade recommended. [1][2]
CVE-2025-45768 — Reported July 31, 2025: weak/insufficient key length handling in JWS (CWE-311) affecting pyjwt 2.10.1 (disputed by the supplier). Scoring classified as High (CVSS ~7.0); vendors/trackers advise remediation or upgrading per vendor guidance. [3][4]
Recommended actions
Sources
If you want, I can: (a) fetch the PyJWT project advisory/GitHub commits for each CVE, (b) check whether a fixed PyPI release for CVE-2025-45768 exists, or (c) produce a remediation checklist for your codebase.
Address CVE-2025-45768 security vulnerability in PyJWT 2.10.1 before merging.
PyJWT 2.10.1 is the latest stable release, but it contains an active HIGH-severity vulnerability (CVE-2025-45768) related to weak/insufficient key length handling in JWS (CVSS ~7.0). Until a patched version is available, enforce strict minimum key length requirements and validate all JWS algorithm/key choices in the authentication implementation.
🤖 Prompt for AI Agents