fix(api): OAuth dynamic client registration fails for Claude Desktop - #8114
Conversation
Accept private-use scheme redirect URIs (RFC 8252 §7.1) in DCR, blocklisting browser-executable schemes. Fix the KeyError 500 when an invalid redirect URI is not first in the list, and stop leaking the ErrorDetail repr into error_description. Error formatting is extracted to mappers.map_drf_error_to_rfc7591_error_body. beep boop
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
📝 WalkthroughWalkthroughDynamic client registration now accepts custom-scheme redirect URIs through serializer input handling while applying expanded scheme, host, fragment, and format validation. Forbidden schemes are rejected, and HTTP is restricted to loopback hosts. DRF validation errors are mapped through a new RFC 7591 mapper with recursive message extraction, and registration tests cover the updated URI cases and error responses. Estimated code review effort: 3 (Moderate) | ~25 minutes 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 |
Docker builds report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 740120c7-5c71-44aa-b938-140fd1f19f1f
📒 Files selected for processing (6)
api/oauth2_metadata/mappers.pyapi/oauth2_metadata/serializers.pyapi/oauth2_metadata/services.pyapi/oauth2_metadata/views.pyapi/tests/unit/oauth2_metadata/test_dcr.pyapi/tests/unit/oauth2_metadata/test_services.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8114 +/- ##
=======================================
Coverage 98.69% 98.70%
=======================================
Files 1525 1526 +1
Lines 60607 60636 +29
=======================================
+ Hits 59819 59848 +29
Misses 788 788 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
✅ private-cloud · depot-ubuntu-latest-16 — run #18752 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18752 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #18752 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #18752 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18751 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ private-cloud · depot-ubuntu-latest-16 — run #18751 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Skipped testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss ✅ oss · depot-ubuntu-latest-arm-16 — run #18751 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #18751 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-16 — run #18750 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18750 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #18750 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #18750 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
DRF CharField coerces integers, so the child-field-error case never produced the index-keyed dict shape; a blank string does. Restructure _first_error_message as a loop so there is no unreachable fallback. beep boop
urlparse raises ValueError on inputs like https://[::1, which escaped as a 500. Catch it, and reject http(s) redirect URIs without a hostname (e.g. https:///callback). beep boop
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 85ddb5c5-dcd3-4c44-b044-2df11dedddc6
📒 Files selected for processing (2)
api/oauth2_metadata/services.pyapi/tests/unit/oauth2_metadata/test_services.py
urlparse defers port validation to attribute access, and django-oauth-toolkit reads .port on stored redirect URIs during authorisation — a URI like https://example.com:not-a-port/callback registered fine, then crashed the authorise flow. beep boop
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/oauth2_metadata/services.py (1)
53-58: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRecognise the full loopback address range.
127.0.0.2and other127.0.0.0/8addresses are valid loopback addresses, but this exact-match list rejects them. Useipaddress.ip_address(host).is_loopbackwhile retaining thelocalhostspecial case, and add a regression test for another127.x.x.xaddress.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8d7b847f-939c-43cd-8d97-bb356b0a5300
📒 Files selected for processing (2)
api/oauth2_metadata/services.pyapi/tests/unit/oauth2_metadata/test_services.py
4 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Closes #8113.
In this PR, we:
Allow custom protocols as part of OAuth redirect URLs. This enables MCP usage in e.g. Claude Desktop.
Map rest-framework serializer errors to errors conformant to RFC 7591.
How did you test this code?
Added unit tests reproducing the custom schema and error parsing issues.