fix: add authentication to /tickets and /ai/log_correction (#786, #785)#791
Conversation
…itesh-1918#786, ritesh-1918#785) - GET /tickets now requires authentication via Depends(get_current_user) - Auto-resolves company_id from user profile when not explicitly provided - Prevents unauthenticated enumeration of all tickets across tenants - POST /ai/log_correction now requires authentication - Prevents fabricated correction data from polluting training dataset Fixes ritesh-1918#786 Fixes ritesh-1918#785
|
@rkhandrianto is attempting to deploy a commit to the ritesh Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
More reviews will be available in 2 minutes and 52 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughTwo API endpoints in ChangesEndpoint Authentication and Tenant Security
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/main.py`:
- Line 478: The current code references get_current_user in function signatures
for log_correction and get_tickets before get_current_user is defined, causing a
NameError on import; move the async def get_current_user definition above any
endpoints that Depend on it (or convert the Depends to a forward reference form)
so the symbol exists at module import. Also enforce tenant isolation in the
/tickets handler: when a caller supplies company_id, validate it matches the
authenticated user’s company (from get_current_user) and reject (401/403)
mismatches; when company_id is omitted, derive the company_id from the
authenticated user and if that lookup/profile fails, return an auth error
instead of running an unfiltered tickets query; update get_tickets and any
ticket-query helper to always use the resolved company_id from the authenticated
user.
- Around line 547-567: The current logic in the ticket listing path (the
company_id resolution and query-building around variables company_id, user, and
supabase.table("profiles")) allows unscoped reads; update it to first resolve
the authenticated user's tenant by fetching
profiles.select("company_id").eq("id", user_id).single(), and if that lookup
fails or returns no company_id return a 403; if the caller provided a company_id
ensure it matches the resolved tenant and return 403 on mismatch; only after
successful resolution/match build the tickets query with query.eq("company_id",
resolved_company_id) and never fall back to an unfiltered select("*") or accept
arbitrary provided company_id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
CodeRabbit identified that the get_tickets endpoint allowed cross-tenant
data reads in two cases:
1. Caller could pass any company_id parameter
2. Failed profile lookup fell through to unfiltered select('*') query
Fix:
- Always derive company_id from authenticated user profile
- Ignore client-provided company_id parameter
- Return 403 if user_id missing, profile lookup fails, or no company assigned
- Always filter by company_id in the query (never unfiltered)
|
Hi @zeroknowledge0x! Thanks for the contribution. I have triaged your PR and set it to merge into the
Welcome to the HELPDESK.AI developer family! 🚀💻 |
|
Hi @zeroknowledge0x! 🙌 Thank you so much for your excellent contribution: "fix: add authentication to /tickets and /ai/log_correction (#786, #785)"! We really appreciate the high-quality code and effort you have put into the platform. Just a quick, friendly heads-up as we prepare our manual merging and verification queues—please make sure to complete all the mandatory community steps listed below. Once those manual steps are verified, we'll get your PR officially merged into the Let's build something amazing together! 🚀🔥 🌟 Community Support & Network Steps (Take 10 Seconds!)As we prepare our manual verification and merging queues, please make sure you have taken a moment to complete these required steps to finalize your points:
Note: Having these steps completed manually is required before your PR points are officially cleared. |
Summary
Fixes #786 and #785 — Adds authentication to two unprotected endpoints that expose sensitive data.
Changes
GET /tickets (#786)
Depends(get_current_user)requirementcompany_idfrom user profile when not explicitly providedPOST /ai/log_correction (#785)
Depends(get_current_user)requirementSecurity Impact
GET /ticketsand submit fake corrections viaPOST /ai/log_correctionTesting
GET /ticketsnow return 401POST /ai/log_correctionnow return 401Fixes #786
Fixes #785
Summary by CodeRabbit