feat: remove jwt library and implement jwt manually#21
feat: remove jwt library and implement jwt manually#21
Conversation
WalkthroughThis PR replaces the external PyJWT dependency with a custom JWT implementation using only standard library modules and updates example module import paths by removing numeric prefixes. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Auth as auth.authenticate()
participant JWTEnc as jwt_encode()
participant JWTDec as jwt_decode()
participant Secret as Secret Key
rect rgb(220, 240, 255)
note over Auth,Secret: Token Generation
Client->>Auth: credentials
Auth->>JWTEnc: payload, secret
JWTEnc->>Secret: HMAC-SHA256
JWTEnc-->>Auth: token (header.payload.sig)
Auth-->>Client: token
end
rect rgb(240, 220, 255)
note over Auth,Secret: Token Verification
Client->>Auth: token
Auth->>JWTDec: token, secret
JWTDec->>JWTDec: parse (header.payload.sig)
JWTDec->>Secret: verify HMAC-SHA256
JWTDec->>JWTDec: check expiration (exp vs time.time())
alt signature valid & not expired
JWTDec-->>Auth: payload
Auth-->>Client: authenticated
else invalid/expired
JWTDec-->>Auth: InvalidTokenError
Auth-->>Client: auth failed
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_custom_snippet.py (1)
35-37: Update token factory to use jwt_custom and integer exp.Align test token with new implementation.
Apply:
def get_token(): - payload = {"user": "test", "exp": datetime.now(timezone.utc) + timedelta(hours=1)} - return jwt.encode(payload, config.jwt_secret, algorithm="HS256") + payload = {"user": "test", "exp": int(time.time()) + 3600} + return jwt_encode(payload, config.jwt_secret, algorithm="HS256")
🧹 Nitpick comments (4)
lightapi/jwt_custom.py (1)
58-59: Consider using integer NumericDate for exp.JWT NumericDate is integer seconds. Recommend casting to int on encode and comparing with int(time.time()) on decode to avoid float drift.
Example:
- if "exp" in payload and payload["exp"] < time.time(): + now = int(time.time()) + if "exp" in payload and int(payload["exp"]) < now: raise ValueError("Token has expired")lightapi/auth.py (3)
110-113: Encode exp as integer seconds to align with JWT NumericDate.Prevents float drift and keeps parity with common JWT tooling.
Apply:
- "exp": time.time() + exp_seconds, + "exp": int(time.time()) + int(exp_seconds),
127-130: Preserve cause when wrapping decode errors.Use exception chaining for clearer traces and to satisfy Ruff B904.
Apply:
- except ValueError as e: - raise InvalidTokenError(str(e)) + except ValueError as e: + raise InvalidTokenError(str(e)) from e
35-46: 401 vs 403 for auth failures (optional).For authentication failures, 401 Unauthorized is conventional; 403 is typically for authorized-but-forbidden.
If middleware uses this, consider returning 401 with a WWW-Authenticate header.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
lightapi/auth.py(4 hunks)lightapi/jwt_custom.py(1 hunks)pyproject.toml(2 hunks)requirements.txt(1 hunks)tests/test_caching_example.py(8 hunks)tests/test_custom_snippet.py(1 hunks)tests/test_filtering_pagination_example.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/test_custom_snippet.py (1)
examples/middleware_cors_auth.py (3)
Company(24-47)CustomEndpoint(55-105)create_app(108-114)
lightapi/jwt_custom.py (1)
examples/authentication_jwt.py (3)
get(57-72)get(79-87)get(103-115)
lightapi/auth.py (1)
lightapi/jwt_custom.py (2)
jwt_encode(15-30)jwt_decode(32-61)
🪛 GitHub Actions: Development Tests
lightapi/jwt_custom.py
[error] 32-32: TypeError: 'type' object is not subscriptable. The annotation 'list[str] = ["HS256"]' uses PEP 585 syntax not supported in Python 3.8. Replace with 'List[str]' and import 'List' from typing, or enable from future import annotations.
🪛 Ruff (0.14.1)
lightapi/jwt_custom.py
26-26: Avoid specifying long messages outside the exception class
(TRY003)
32-32: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
36-36: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
36-36: Avoid specifying long messages outside the exception class
(TRY003)
42-42: Avoid specifying long messages outside the exception class
(TRY003)
49-49: Avoid specifying long messages outside the exception class
(TRY003)
54-54: Avoid specifying long messages outside the exception class
(TRY003)
59-59: Avoid specifying long messages outside the exception class
(TRY003)
lightapi/auth.py
130-130: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🔇 Additional comments (5)
tests/test_custom_snippet.py (1)
40-40: No unused PyJWT imports found; review comment is incorrect.The
import jwtat line 9 remains necessary because the activeget_token()helper function (lines 33–37) usesjwt.encode(). The removed functions are the test cases, not the helper. Similarly,tests/test_auth.pyactively uses jwt in its test methods. All jwt imports in test files are in use and require no changes.Likely an incorrect or invalid review comment.
tests/test_filtering_pagination_example.py (2)
460-460: Patch target correctly updated.The patch target aligns with the new module path.
9-14: Module path updates verified and correct.All references to the old numeric-prefixed module path have been successfully updated:
- Import path (lines 9-14):
examples.filtering_pagination✓- Patch target (line 461):
examples.filtering_pagination.create_engine✓No stale references to the old paths remain in the codebase.
tests/test_caching_example.py (2)
10-14: LGTM! Module path updated correctly.The import path has been updated to remove the numeric prefix, consistent with the module reorganization.
35-36: All patch targets updated consistently.All
@patchdecorators have been correctly updated to reference the new module path without the numeric prefix. The changes are consistent across all test methods.Also applies to: 67-68, 110-110, 140-140, 188-189, 218-219, 244-245
| import base64 | ||
| import json | ||
| import hmac | ||
| import hashlib | ||
| import time | ||
|
|
There was a problem hiding this comment.
Make typing Python 3.8‑compatible and remove mutable default.
list[str] breaks on 3.8 and default list violates B006; pipeline is failing. Use Optional[List[str]] and set default inside.
Apply:
import base64
import json
import hmac
import hashlib
import time
+from typing import List, Optional
@@
-def jwt_decode(token: str, secret: str, algorithms: list[str] = ["HS256"]) -> dict:
+def jwt_decode(token: str, secret: str, algorithms: Optional[List[str]] = None) -> dict:And inside jwt_decode before using algorithms:
- header_data = json.loads(b64_decode(encoded_header))
+ header_data = json.loads(b64_decode(encoded_header))
+ algorithms = algorithms or ["HS256"]Also applies to: 32-32
🤖 Prompt for AI Agents
In lightapi/jwt_custom.py around lines 2-7 and line 32, update any type
annotations using list[str] to use typing.Optional and typing.List (e.g.,
Optional[List[str]]), add the necessary imports from typing, change default
mutable parameters from [] to None, and inside jwt_decode set algorithms =
algorithms or [] (or algorithms = list(algorithms) if you need a copy) before
use so the default is created at call time rather than at function definition.
| def b64_decode(data: str) -> bytes: | ||
| padding = b"=" * (4 - (len(data) % 4)) | ||
| return base64.urlsafe_b64decode(data.encode("utf-8") + padding) |
There was a problem hiding this comment.
Fix Base64 padding (current code adds 4 "=" when none needed).
When len%4==0 you still append four "=" causing decode errors. Compute missing padding modulo 4.
Apply:
-def b64_decode(data: str) -> bytes:
- padding = b"=" * (4 - (len(data) % 4))
- return base64.urlsafe_b64decode(data.encode("utf-8") + padding)
+def b64_decode(data: str) -> bytes:
+ # Add only the required padding (0..3)
+ missing = (-len(data)) % 4
+ if missing:
+ data += "=" * missing
+ return base64.urlsafe_b64decode(data.encode("utf-8"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def b64_decode(data: str) -> bytes: | |
| padding = b"=" * (4 - (len(data) % 4)) | |
| return base64.urlsafe_b64decode(data.encode("utf-8") + padding) | |
| def b64_decode(data: str) -> bytes: | |
| # Add only the required padding (0..3) | |
| missing = (-len(data)) % 4 | |
| if missing: | |
| data += "=" * missing | |
| return base64.urlsafe_b64decode(data.encode("utf-8")) |
🤖 Prompt for AI Agents
In lightapi/jwt_custom.py around lines 11 to 13, the base64 padding logic always
appends four "=" when data length is already a multiple of 4 causing decode
errors; change the padding calculation to compute missing = (4 - (len(data) %
4)) % 4 (or equivalent negative-mod trick) and append b"=" * missing so you only
add the needed padding bytes before urlsafe_b64decode.
| try: | ||
| encoded_header, encoded_payload, encoded_signature = token.split(".") | ||
| except ValueError: | ||
| raise ValueError("Invalid token") | ||
|
|
||
| header_data = json.loads(b64_decode(encoded_header)) | ||
| alg = header_data.get("alg") | ||
|
|
||
| if not alg or alg not in algorithms: | ||
| raise ValueError("Invalid algorithm") | ||
|
|
||
| signing_input = f"{encoded_header}.{encoded_payload}".encode("utf-8") | ||
|
|
||
| if alg == "HS256": | ||
| expected_signature = hmac.new(secret.encode("utf-8"), signing_input, hashlib.sha256).digest() | ||
| else: | ||
| raise ValueError("Unsupported algorithm") | ||
|
|
||
| decoded_signature = b64_decode(encoded_signature) | ||
|
|
||
| if not hmac.compare_digest(decoded_signature, expected_signature): | ||
| raise ValueError("Invalid signature") | ||
|
|
||
| payload = json.loads(b64_decode(encoded_payload)) | ||
|
|
||
| if "exp" in payload and payload["exp"] < time.time(): | ||
| raise ValueError("Token has expired") | ||
|
|
||
| return payload |
There was a problem hiding this comment.
Normalize decode errors to ValueError to avoid 500s upstream.
Base64/JSON errors currently leak as non‑ValueError; auth.decode_token won’t catch them. Wrap and re‑raise ValueError.
Apply:
- try:
- encoded_header, encoded_payload, encoded_signature = token.split(".")
- except ValueError:
- raise ValueError("Invalid token")
+ try:
+ encoded_header, encoded_payload, encoded_signature = token.split(".")
+ except ValueError:
+ raise ValueError("Invalid token") from None
@@
- header_data = json.loads(b64_decode(encoded_header))
+ try:
+ header_data = json.loads(b64_decode(encoded_header))
+ except Exception:
+ raise ValueError("Invalid token") from None
@@
- decoded_signature = b64_decode(encoded_signature)
+ try:
+ decoded_signature = b64_decode(encoded_signature)
+ except Exception:
+ raise ValueError("Invalid token") from None
@@
- payload = json.loads(b64_decode(encoded_payload))
+ try:
+ payload = json.loads(b64_decode(encoded_payload))
+ except Exception:
+ raise ValueError("Invalid token") from None🧰 Tools
🪛 Ruff (0.14.1)
36-36: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
36-36: Avoid specifying long messages outside the exception class
(TRY003)
42-42: Avoid specifying long messages outside the exception class
(TRY003)
49-49: Avoid specifying long messages outside the exception class
(TRY003)
54-54: Avoid specifying long messages outside the exception class
(TRY003)
59-59: Avoid specifying long messages outside the exception class
(TRY003)
| PyJWT==2.9.0 | ||
| pytest==8.2.2 | ||
| PyYAML>=5.1 No newline at end of file | ||
| PyYAML>=5.1 |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM; confirm tests don’t rely on PyJWT.
PyJWT is removed project-wide; ensure no tests still import it. See fix proposed in tests/test_custom_snippet.py.
Run to find stragglers:
🏁 Script executed:
#!/bin/bash
rg -nP '^\s*import\s+jwt\b|jwt\.(encode|decode)\b|from\s+jwt\s+import' -g '!**/.venv/**' -SLength of output: 1747
Critical: Tests still import PyJWT despite removal from requirements.txt.
The verification found multiple test files that still import and use PyJWT:
tests/test_custom_snippet.py:9- import jwttests/test_auth.py:8, 22, 56, 86- import jwt with jwt.encode() and jwt.decode() calls
Removing PyJWT from requirements.txt will break these tests. Either update tests to not rely on PyJWT, or retain the dependency in requirements.txt.
🤖 Prompt for AI Agents
In requirements.txt at line 8, removing PyJWT breaks tests that still import and
use jwt (tests/test_custom_snippet.py and tests/test_auth.py); either restore
the dependency by re-adding a compatible PyJWT entry (e.g., PyJWT>=2.0,<3.0) to
requirements.txt so tests keep working, or update the tests to stop relying on
PyJWT by replacing jwt.encode()/jwt.decode() calls with a project-supported
alternative (mock the jwt module or use a different JWT library already in
requirements) and adjust imports accordingly; pick one approach and apply
consistently across the mentioned test files.
| @@ -9,7 +9,7 @@ | |||
| import jwt | |||
There was a problem hiding this comment.
Remove PyJWT usage in tests; use local jwt helper.
PyJWT is no longer a dependency; this import will fail.
Apply:
-import jwt
+import time
+from lightapi.jwt_custom import jwt_encode📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import jwt | |
| import time | |
| from lightapi.jwt_custom import jwt_encode |
🤖 Prompt for AI Agents
In tests/test_custom_snippet.py around line 9, remove the external "import jwt"
(PyJWT is no longer a dependency) and instead import the repository's local JWT
helper; replace the import with the local helper module/function used across the
test suite (for example import the encode helper such as from
tests.helpers.jwt_helper import jwt_encode or from tests.utils import
jwt_encode) and update any jwt.* calls in this test to use that local helper
(e.g., jwt_encode) so the test no longer depends on PyJWT.
1 participant
This PR removes the PyJWT dependency and replaces it with a manual implementation of JWT authentication.
Summary by CodeRabbit
Refactor
InvalidTokenErrorfor invalid tokens.Chores