include email in JWT token plus some other fixes#274
Conversation
Assisted-by: Cursor
WalkthroughThese changes add email claim support to JWT token generation. Both Makefile targets ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/Makefile (1)
296-300: LGTM —uv run --no-project --with "PyJWT[crypto]"is valid and correctly fixes the local dependency error.
--no-projectis a documented uv flag used exactly when a script doesn't depend on the project; it tells uv to skip project discovery and run the script in isolation. The--with "PyJWT[crypto]"addition correctly pulls in thecryptographyback-end required for RS256 signing.The empty-string placeholders (
"" "" "") forprivate_key_path,subject, andissuerare a workable but fragile coupling to the current positional-arg implementation ofgenerate_token.py. If the Python script is ever refactored to useargparse, these placeholders will need updating.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/Makefile` around lines 296 - 300, The make target generate-api-token uses fragile positional empty-string placeholders when invoking generate_token.py; update the Makefile target (generate-api-token) to pass explicit named CLI flags or environment variables instead of "" "" "" so the invocation is robust if generate_token.py switches to argparse—either (A) change the command to provide --private-key-path, --subject, and --issuer flags (pull values from make variables or defaults) or (B) set well-named environment variables read by generate_token.py; reference the target name generate-api-token, the script generate_token.py, and the EMAIL/UV make variables when implementing the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/Makefile`:
- Around line 296-300: The make target generate-api-token uses fragile
positional empty-string placeholders when invoking generate_token.py; update the
Makefile target (generate-api-token) to pass explicit named CLI flags or
environment variables instead of "" "" "" so the invocation is robust if
generate_token.py switches to argparse—either (A) change the command to provide
--private-key-path, --subject, and --issuer flags (pull values from make
variables or defaults) or (B) set well-named environment variables read by
generate_token.py; reference the target name generate-api-token, the script
generate_token.py, and the EMAIL/UV make variables when implementing the change.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #274 +/- ##
==========================================
- Coverage 83.79% 83.71% -0.09%
==========================================
Files 143 143
Lines 13543 13543
==========================================
- Hits 11349 11338 -11
- Misses 2194 2205 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0c52ae1
into
codeready-toolchain:master
The JWT token needs to have email claim defined because it's being enforced by oauth2-proxy.
Also, when trying running the makefile target locally, it failed with some weird python dependency error, so this PR contains vibe-coded fixes for that as well
Assisted-by: Cursor
Summary by CodeRabbit
New Features
Documentation