Fix OAuth2 userinfo request crash when endpoint is present but None - #10353
Fix OAuth2 userinfo request crash when endpoint is present but None#10353dpage wants to merge 1 commit into
Conversation
get_user_profile() tested key presence ('OAUTH2_USERINFO_ENDPOINT' not
in ...) rather than truthiness, so a config copied from the shipped
config.py template - which ships OAUTH2_USERINFO_ENDPOINT: None - would
pass the check and call client.get(None), raising
requests.exceptions.MissingSchema instead of skipping the call.
Closes pgadmin-org#10349
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. WalkthroughThe OAuth2 profile lookup now treats a missing or falsy ChangesOAuth2 user-info handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR prevents an invalid userinfo request when the endpoint is explicitly unset and adds coverage for that behavior; no actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Summary
OAuth2Authentication.get_user_profile()guarded the userinfo request with'OAUTH2_USERINFO_ENDPOINT' not in self.oauth2_config[...], which tests key presence, not truthiness.config.py's shippedOAUTH2_CONFIGtemplate entry ships'OAUTH2_USERINFO_ENDPOINT': None, so a config copied from that template (as documented) has the key present with aNonevalue — the check passes, and the code proceeds to callclient.get(None), raisingrequests.exceptions.MissingSchema: Invalid URL 'None'instead of skipping the call (for an OIDC provider that supplies claims via ID token / discovery) or logging the intended "not configured" message.if not self.oauth2_config[...].get('OAUTH2_USERINFO_ENDPOINT')), so an explicitNoneis treated the same as an absent key.Test plan
web/pgadmin/authenticate/tests/test_oauth2_userinfo_endpoint.py, exercisingget_user_profile()directly with a config carryingOAUTH2_USERINFO_ENDPOINT: None, asserting the userinfo client is never called.not incheck and passes with the fix.pycodestyleclean on the changed/added files.Closes #10349
Summary by CodeRabbit
Bug Fixes
Tests