Fix OAuth2 userinfo request crash when endpoint is present but None

web/pgadmin/authenticate/oauth2.py

@@ -135,6 +135,9 @@ def oauth_authorize():
             session.pop('oauth2_current_client', None)
             return redirect(get_safe_post_login_redirect())
         session.pop('oauth2_current_client', None)
+        # get_user_profile() may already have stored the provider's logout
+        # URL before the login failed; don't leave it for a later logout.
+        session.pop('oauth2_logout_url', None)
         logout_user()
         flash(msg, MessageType.ERROR)
         return redirect(get_safe_post_login_redirect())
@@ -600,13 +603,10 @@ def login(self, form):
             current_app.logger.error(error_msg)
             return False, error_msg
 
-        additional_claims = None
-        if 'OAUTH2_ADDITIONAL_CLAIMS' in self.oauth2_config[
-                self.oauth2_current_client]:
-
-            additional_claims = self.oauth2_config[
-                self.oauth2_current_client
-            ]['OAUTH2_ADDITIONAL_CLAIMS']
+        # config.py ships the key with a value of None, so test whether it
+        # is set rather than whether it is present.
+        additional_claims = self.oauth2_config.get(
+            self.oauth2_current_client, {}).get('OAUTH2_ADDITIONAL_CLAIMS')
 
         # For OIDC providers, check ID token claims first, then userinfo
         # For non-OIDC providers, check userinfo only
@@ -689,10 +689,16 @@ def get_user_profile(self):
 
         session['pass_enc_key'] = session['oauth2_token']['access_token']
 
-        if 'OAUTH2_LOGOUT_URL' in self.oauth2_config[
-                self.oauth2_current_client]:
-            session['oauth2_logout_url'] = self.oauth2_config[
-                self.oauth2_current_client]['OAUTH2_LOGOUT_URL']
+        # As above, the shipped config.py sets this to None, so only stash
+        # it in the session when it actually holds a URL, and otherwise drop
+        # any left over from an earlier login so that logout cannot redirect
+        # to a different provider's URL.
+        logout_url = self.oauth2_config.get(
+            self.oauth2_current_client, {}).get('OAUTH2_LOGOUT_URL')
+        if logout_url:
+            session['oauth2_logout_url'] = logout_url
+        else:
+            session.pop('oauth2_logout_url', None)
 
         # For OIDC providers, parse the ID token JWT to extract claims.
         # We can skip the userinfo endpoint call if the ID token has
@@ -744,8 +750,9 @@ def get_user_profile(self):
 
         # For non-OIDC providers or when ID token is insufficient,
         # call the userinfo endpoint
-        if 'OAUTH2_USERINFO_ENDPOINT' not in self.oauth2_config[
-                self.oauth2_current_client]:
+        userinfo_endpoint = self.oauth2_config.get(
+            self.oauth2_current_client, {}).get('OAUTH2_USERINFO_ENDPOINT')
+        if not userinfo_endpoint:
             if self._is_oidc_provider():
                 # OIDC provider should have provided claims in ID token
                 current_app.logger.warning(
@@ -758,8 +765,7 @@ def get_user_profile(self):
             return {}
 
         resp = self.oauth2_clients[self.oauth2_current_client].get(
-            self.oauth2_config[
-                self.oauth2_current_client]['OAUTH2_USERINFO_ENDPOINT'],
+            userinfo_endpoint,
             token=session['oauth2_token']
         )
         resp.raise_for_status()

web/pgadmin/authenticate/tests/test_oauth2_userinfo_endpoint.py

@@ -0,0 +1,70 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2026, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+
+"""Verify that an OAUTH2_USERINFO_ENDPOINT of None (the shipped config
+template default) is treated the same as an absent key, rather than being
+passed to the HTTP client and crashing (issue #10349).
+"""
+
+import importlib
+from unittest.mock import MagicMock, patch
+
+from pgadmin.utils.route import BaseTestGenerator
+
+
+class OAuth2UserinfoEndpointNoneTestCase(BaseTestGenerator):
+    """Exercises get_user_profile() directly - no server connection needed."""
+
+    def setUp(self):
+        pass
+
+    def runTest(self):
+        # Resolved at call time, rather than imported at module load time:
+        # pgadmin.authenticate.oauth2 can be re-imported after this module
+        # is loaded, which would leave a module-level import here bound to a
+        # stale module object whose globals patch('...session', ...) below
+        # wouldn't reach. import_module returns the module that is already
+        # in sys.modules when there is one, and imports it otherwise, so
+        # this also works when nothing else has loaded it (in desktop mode
+        # the auth source registry doesn't).
+        oauth2_module = importlib.import_module('pgadmin.authenticate.oauth2')
+        OAuth2Authentication = oauth2_module.OAuth2Authentication
+
+        auth = OAuth2Authentication.__new__(OAuth2Authentication)
+        auth.oauth2_current_client = 'test_provider'
+        auth.oauth2_config = {
+            'test_provider': {
+                'OAUTH2_NAME': 'test_provider',
+                # Shipped config.py template default - key present, not set.
+                'OAUTH2_USERINFO_ENDPOINT': None,
+            }
+        }
+        mock_client = MagicMock()
+        auth.oauth2_clients = {'test_provider': mock_client}
+
+        # Left over from an earlier login with a provider that had one.
+        fake_session = {'oauth2_logout_url': 'https://idp.example.com/out'}
+
+        with self.app.app_context(), \
+                patch.object(auth, '_authorize_access_token',
+                             return_value={'access_token': 'tok'}), \
+                patch.object(auth, '_is_oidc_provider',
+                             return_value=False), \
+                patch('pgadmin.authenticate.oauth2.session', fake_session):
+            profile = auth.get_user_profile()
+
+        # Pre-fix, the None endpoint was handed straight to client.get(), so
+        # nothing raised: the MagicMock just returned a mock profile. It is
+        # these two assertions, rather than an exception, that prove the
+        # guard skips the call.
+        self.assertEqual(profile, {})
+        mock_client.get.assert_not_called()
+        # OAUTH2_LOGOUT_URL is absent here, so the stale URL must not
+        # survive to be used by the next logout.
+        self.assertNotIn('oauth2_logout_url', fake_session)

web/pgadmin/browser/tests/test_oauth2_with_mocking.py

@@ -139,6 +139,12 @@ class Oauth2LoginMockTestCase(BaseTestGenerator):
             profile={},
             id_token_claims=None,
         )),
+        ('OAuth2 Failed Callback Clears Stored Logout URL', dict(
+            oauth2_provider='github',
+            kind='callback_login_failure_clears_logout_url',
+            profile={},
+            id_token_claims=None,
+        )),
         ('OAuth2 openid Scope Without Metadata URL Fails Fast', dict(
             oauth2_provider='oidc-no-metadata',
             kind='openid_without_metadata_url',
@@ -327,6 +333,9 @@ def runTest(self):
             self._test_session_state_after_redirect(self.oauth2_provider)
         elif self.kind == 'callback_missing_provider_state':
             self._test_oauth2_callback_missing_provider_state()
+        elif self.kind == 'callback_login_failure_clears_logout_url':
+            self._test_oauth2_callback_failure_clears_logout_url(
+                self.oauth2_provider)
         elif self.kind == 'openid_without_metadata_url':
             self._test_openid_scope_without_metadata_url_fails_fast()
         else:
@@ -785,6 +794,26 @@ def _test_oauth2_callback_missing_provider_state(self):
         # an unhandled exception.
         self.assertLess(res.status_code, 500)
 
+    def _test_oauth2_callback_failure_clears_logout_url(self, provider):
+        """A failed OAuth2 callback must drop any logout URL already stored
+        in the session, since get_user_profile() stores it before the login
+        can fail, and a later logout would otherwise redirect to it.
+        """
+        with self.tester.session_transaction() as sess:
+            sess['oauth2_current_client'] = provider
+            sess['oauth2_logout_url'] = 'https://idp.example.com/logout'
+
+        with patch('pgadmin.authenticate.AuthSourceManager.login',
+                   return_value=(False, 'Login failed')):
+            res = self.tester.get('/oauth2/authorize',
+                                  follow_redirects=False)
+
+        self.assertEqual(res.status_code, 302)
+        with self.tester.session_transaction() as sess:
+            self.assertNotIn('oauth2_logout_url', sess)
+            self.assertNotIn('oauth2_current_client', sess)
+        self._assert_oauth2_session_not_logged_in()
+
     def _test_openid_scope_without_metadata_url_fails_fast(self):
         """'openid' in OAUTH2_SCOPE without OAUTH2_SERVER_METADATA_URL must
         fail fast with actionable guidance, before any network round-trip.