From 0ff1b7ee84dc7ebd2bd805a678661a10d0ecbce1 Mon Sep 17 00:00:00 2001 From: dev-hari-prasad Date: Sat, 29 Aug 2026 17:26:08 +0530 Subject: [PATCH 1/7] Defer cloud SDK imports across AWS RDS, Azure, and Google Cloud --- web/pgadmin/misc/cloud/azure/__init__.py | 61 ++++++++++++++++------- web/pgadmin/misc/cloud/google/__init__.py | 58 +++++++++++++-------- web/pgadmin/misc/cloud/rds/__init__.py | 6 ++- 3 files changed, 83 insertions(+), 42 deletions(-) diff --git a/web/pgadmin/misc/cloud/azure/__init__.py b/web/pgadmin/misc/cloud/azure/__init__.py index 60d7c3cd942..845a69d240d 100644 --- a/web/pgadmin/misc/cloud/azure/__init__.py +++ b/web/pgadmin/misc/cloud/azure/__init__.py @@ -25,15 +25,6 @@ import os -from azure.mgmt.rdbms.postgresql_flexibleservers import \ - PostgreSQLManagementClient -from azure.identity import AzureCliCredential, DeviceCodeCredential,\ - AuthenticationRecord -from azure.mgmt.resource import ResourceManagementClient -from azure.mgmt.subscription import SubscriptionClient -from azure.mgmt.rdbms.postgresql_flexibleservers.models import \ - NameAvailabilityRequest - MODULE_NAME = 'azure' @@ -265,6 +256,34 @@ def clear_session(): return make_json_response(success=1) +def _azure_sdk(): + """Defer heavy Azure SDK imports until required by user actions. + Repeat calls are cheap via sys.modules caching. + """ + from types import SimpleNamespace + from azure.identity import ( + AzureCliCredential, DeviceCodeCredential, AuthenticationRecord + ) + from azure.mgmt.rdbms.postgresql_flexibleservers import ( + PostgreSQLManagementClient + ) + from azure.mgmt.rdbms.postgresql_flexibleservers.models import ( + NameAvailabilityRequest + ) + from azure.mgmt.resource import ResourceManagementClient + from azure.mgmt.subscription import SubscriptionClient + + return SimpleNamespace( + AzureCliCredential=AzureCliCredential, + DeviceCodeCredential=DeviceCodeCredential, + AuthenticationRecord=AuthenticationRecord, + PostgreSQLManagementClient=PostgreSQLManagementClient, + ResourceManagementClient=ResourceManagementClient, + SubscriptionClient=SubscriptionClient, + NameAvailabilityRequest=NameAvailabilityRequest, + ) + + class Azure: def __init__(self, interactive_browser_credential, tenant_id=None, session_token=None, region='eastus'): @@ -367,7 +386,8 @@ def _get_azure_credentials(self): def _azure_cli_auth(self): if self._cli_credentials is None: - self._cli_credentials = AzureCliCredential() + sdk = _azure_sdk() + self._cli_credentials = sdk.AzureCliCredential() self.list_subscriptions() return self._cli_credentials @@ -380,8 +400,9 @@ def _azure_interactive_auth_prompt_callback( session['azure']['azure_auth_code'] = azure_auth_code def _azure_interactive_auth(self): + sdk = _azure_sdk() if self.authentication_record_json is None: - _interactive_credential = DeviceCodeCredential( + _interactive_credential = sdk.DeviceCodeCredential( tenant_id=self._tenant_id, timeout=180, prompt_callback=self._azure_interactive_auth_prompt_callback, @@ -392,9 +413,9 @@ def _azure_interactive_auth(self): _auth_record = _interactive_credential.authenticate() self.authentication_record_json = _auth_record.serialize() else: - deserialized_auth_record = AuthenticationRecord.deserialize( + deserialized_auth_record = sdk.AuthenticationRecord.deserialize( self.authentication_record_json) - _interactive_credential = DeviceCodeCredential( + _interactive_credential = sdk.DeviceCodeCredential( tenant_id=self._tenant_id, timeout=180, prompt_callback=self._azure_interactive_auth_prompt_callback, @@ -411,15 +432,16 @@ def _get_azure_client(self, type): return self._clients[type] _, _credentials = self._get_azure_credentials() + sdk = _azure_sdk() if type == 'postgresql': - client = PostgreSQLManagementClient(_credentials, - self.subscription_id) + client = sdk.PostgreSQLManagementClient(_credentials, + self.subscription_id) elif type == 'resource': - client = ResourceManagementClient(_credentials, - self.subscription_id) + client = sdk.ResourceManagementClient(_credentials, + self.subscription_id) elif type == 'subscription': - client = SubscriptionClient(_credentials) + client = sdk.SubscriptionClient(_credentials) self._clients[type] = client return self._clients[type] @@ -430,8 +452,9 @@ def check_cluster_name_availability(self, cluster_name): :param cluster_name """ postgresql_client = self._get_azure_client('postgresql') + sdk = _azure_sdk() res = postgresql_client.check_name_availability.execute( - NameAvailabilityRequest( + sdk.NameAvailabilityRequest( name=cluster_name, type='Microsoft.DBforPostgreSQL/flexibleServers')) res = res.__dict__ diff --git a/web/pgadmin/misc/cloud/google/__init__.py b/web/pgadmin/misc/cloud/google/__init__.py index 20fb883593c..57aff041757 100644 --- a/web/pgadmin/misc/cloud/google/__init__.py +++ b/web/pgadmin/misc/cloud/google/__init__.py @@ -38,11 +38,6 @@ # fall back to google-auth cleanly. See issue #10110. sys.modules.setdefault('oauth2client', None) -from googleapiclient import discovery -from googleapiclient.errors import HttpError -from google_auth_oauthlib.flow import InstalledAppFlow -from google.auth.transport.requests import Request - MODULE_NAME = 'google' os.environ['OAUTHLIB_INSECURE_TRANSPORT'] = '1' # Required for Oauth2 @@ -374,6 +369,17 @@ def clear_google_session(): session.pop('google') +def _google_sdk(): + """Defer heavy Google API client imports until required by user actions. + Repeat calls are cheap via sys.modules caching. + """ + from types import SimpleNamespace + from googleapiclient import discovery + from googleapiclient.errors import HttpError + + return SimpleNamespace(discovery=discovery, HttpError=HttpError) + + class Google: def __init__(self, client_config=None): # Google cloud sql api versions @@ -468,6 +474,8 @@ def get_auth_url(self, host_url): self._verification_error = None try: self._redirect_url = host_url + 'google/callback' + # Defer InstalledAppFlow (heavy import, user action only, cached) + from google_auth_oauthlib.flow import InstalledAppFlow flow = InstalledAppFlow.from_client_config( client_config=self._client_config, scopes=self._scopes, redirect_uri=self._redirect_url) @@ -491,6 +499,8 @@ def callback(self, flask_request): if session['state'] != flask_request.args.get('state', None): self._verification_successful = False, self._verification_error = 'Invalid state parameter' + # Defer InstalledAppFlow (heavy import, user action only, cached) + from google_auth_oauthlib.flow import InstalledAppFlow flow = InstalledAppFlow.from_client_config( client_config=self._client_config, scopes=self._scopes, redirect_uri=self._redirect_url) @@ -532,6 +542,8 @@ def _get_credentials(self, scopes): if self._credentials and self._credentials.expired and \ self._credentials.refresh_token and \ self._credentials.has_scopes(scopes): + # Defer Request (heavy import, user action only, cached) + from google.auth.transport.requests import Request self._credentials.refresh(Request()) return self._credentials return self._credentials @@ -543,17 +555,18 @@ def get_projects(self): """ projects = [] error = None + sdk = _google_sdk() credentials = self._get_credentials(self._scopes) - service = discovery.build('cloudresourcemanager', - self._cloud_resource_manager_api_version, - credentials=credentials) + service = sdk.discovery.build('cloudresourcemanager', + self._cloud_resource_manager_api_version, + credentials=credentials) try: req = service.projects().list() res = req.execute() for project in res.get('projects', []): projects.append({'label': project['projectId'], 'value': project['projectId']}) - except HttpError as e: + except sdk.HttpError as e: error = e.reason except Exception as e: error = str(e) @@ -566,10 +579,11 @@ def get_regions(self, project): :return: """ self._project_id = project + sdk = _google_sdk() credentials = self._get_credentials(self._scopes) - service = discovery.build('compute', - self._compute_api_version, - credentials=credentials) + service = sdk.discovery.build('compute', + self._compute_api_version, + credentials=credentials) error = None try: req = service.regions().list(project=project) @@ -582,7 +596,7 @@ def get_regions(self, project): region_zones = list( map(lambda region: region.split('/')[-1], region_zones)) self._availability_zones[region_name] = region_zones - except HttpError as e: + except sdk.HttpError as e: error = e.reason except Exception as e: error = str(e) @@ -611,10 +625,11 @@ def get_instance_types(self, project, region): high_mem = [] instance_types = {} error = None + sdk = _google_sdk() credentials = self._get_credentials(self._scopes) - service = discovery.build('sqladmin', - self._sqladmin_api_version, - credentials=credentials) + service = sdk.discovery.build('sqladmin', + self._sqladmin_api_version, + credentials=credentials) try: req = service.tiers().list(project=project) res = req.execute() @@ -645,7 +660,7 @@ def get_instance_types(self, project, region): instance_types = {'standard': standard_instances, 'highmem': high_mem, 'shared': shared_instances} - except HttpError as e: + except sdk.HttpError as e: error = e.reason except Exception as e: error = str(e) @@ -659,10 +674,11 @@ def get_database_versions(self): pg_database_versions = [] database_versions = [] error = None + sdk = _google_sdk() credentials = self._get_credentials(self._scopes) - service = discovery.build('sqladmin', - self._sqladmin_api_version, - credentials=credentials) + service = sdk.discovery.build('sqladmin', + self._sqladmin_api_version, + credentials=credentials) try: req = service.flags().list() res = req.execute() @@ -673,7 +689,7 @@ def get_database_versions(self): label = (version.title().split('_')[0])[0:7] \ + 'SQL ' + version.split('_')[1] database_versions.append({'label': label, 'value': version}) - except HttpError as e: + except sdk.HttpError as e: error = e.reason except Exception as e: error = str(e) diff --git a/web/pgadmin/misc/cloud/rds/__init__.py b/web/pgadmin/misc/cloud/rds/__init__.py index 62c92add6c8..cd0ba507345 100644 --- a/web/pgadmin/misc/cloud/rds/__init__.py +++ b/web/pgadmin/misc/cloud/rds/__init__.py @@ -10,9 +10,7 @@ # AWS RDS Cloud Deployment Implementation import requests -import boto3 import json -from boto3.session import Session from flask_babel import gettext from flask import session, current_app, request from pgadmin.user_login_check import pga_login_required @@ -152,6 +150,8 @@ def get_regions(): """GET Regions for AWS.""" try: clear_aws_session() + # Defer boto3.session (heavy import, user action only, cached) + from boto3.session import Session _session = Session() res = _session.get_available_regions('rds') regions = [] @@ -192,6 +192,8 @@ def _get_aws_client(self, type): if type in self._clients: return self._clients[type] + # Defer boto3 (heavy import, user action only, cached) + import boto3 session = boto3.Session( aws_access_key_id=self._access_key, aws_secret_access_key=self._secret_key, From 6b0e6559384d28902bddd96303c958ac00d044a6 Mon Sep 17 00:00:00 2001 From: dev-hari-prasad Date: Tue, 1 Sep 2026 23:15:53 +0530 Subject: [PATCH 2/7] Handle ImportError cleanly in Google and Azure cloud modules - Guard _google_sdk() calls with try/except ImportError across Google methods, returning (data, error) tuples. - Guard _azure_sdk() calls and client creation in Azure methods to prevent unhandled 500 exceptions on missing/broken SDK installations. - Add explanatory comment in _google_sdk() referencing the oauth2client sentinel invariant. - Fix unpacking in google.instance_types route. - Add test coverage for missing SDK import error handling in both modules. --- web/pgadmin/misc/cloud/azure/__init__.py | 78 +++++++++++++------ .../azure/tests/test_azure_session_state.py | 35 +++++++++ web/pgadmin/misc/cloud/google/__init__.py | 28 +++++-- .../google/tests/test_google_session_state.py | 33 ++++++++ 4 files changed, 143 insertions(+), 31 deletions(-) diff --git a/web/pgadmin/misc/cloud/azure/__init__.py b/web/pgadmin/misc/cloud/azure/__init__.py index 845a69d240d..0db1295f521 100644 --- a/web/pgadmin/misc/cloud/azure/__init__.py +++ b/web/pgadmin/misc/cloud/azure/__init__.py @@ -431,8 +431,14 @@ def _get_azure_client(self, type): if type in self._clients: return self._clients[type] - _, _credentials = self._get_azure_credentials() - sdk = _azure_sdk() + status, _credentials = self._get_azure_credentials() + if not status: + return None + + try: + sdk = _azure_sdk() + except ImportError: + return None if type == 'postgresql': client = sdk.PostgreSQLManagementClient(_credentials, @@ -451,8 +457,13 @@ def check_cluster_name_availability(self, cluster_name): Checks whether given server name is available or not :param cluster_name """ + try: + sdk = _azure_sdk() + except ImportError as e: + return False, str(e) postgresql_client = self._get_azure_client('postgresql') - sdk = _azure_sdk() + if not postgresql_client: + return False, 'Failed to initialize Azure client.' res = postgresql_client.check_name_availability.execute( sdk.NameAvailabilityRequest( name=cluster_name, @@ -465,13 +476,18 @@ def list_subscriptions(self): List subscriptions :return: """ - subscription_client = self._get_azure_client('subscription') - sub_list = subscription_client.subscriptions.list() subscriptions_list = [] - for group in list(sub_list): - subscriptions_list.append( - {'subscription_id': group.subscription_id, - 'subscription_name': group.display_name}) + try: + subscription_client = self._get_azure_client('subscription') + if not subscription_client: + return subscriptions_list + sub_list = subscription_client.subscriptions.list() + for group in list(sub_list): + subscriptions_list.append( + {'subscription_id': group.subscription_id, + 'subscription_name': group.display_name}) + except ImportError: + return subscriptions_list return subscriptions_list def list_resource_groups(self, subscription_id): @@ -481,14 +497,19 @@ def list_resource_groups(self, subscription_id): :return: """ self.subscription_id = subscription_id - resource_client = self._get_azure_client('resource') - group_list = resource_client.resource_groups.list() resource_groups_list = [] - for group in list(group_list): - resource_groups_list.append( - {'label': group.name, - 'value': group.name, - 'region': group.location}) + try: + resource_client = self._get_azure_client('resource') + if not resource_client: + return resource_groups_list + group_list = resource_client.resource_groups.list() + for group in list(group_list): + resource_groups_list.append( + {'label': group.name, + 'value': group.name, + 'region': group.location}) + except ImportError: + return resource_groups_list return resource_groups_list def list_regions(self, subscription_id): @@ -498,13 +519,18 @@ def list_regions(self, subscription_id): :return: """ self.subscription_id = subscription_id - subscription_client = self._get_azure_client('subscription') - locations = subscription_client.subscriptions.list_locations( - subscription_id=self.subscription_id) locations_list = [] - for location in locations: - locations_list.append( - {'label': location.display_name, 'value': location.name}) + try: + subscription_client = self._get_azure_client('subscription') + if not subscription_client: + return locations_list + locations = subscription_client.subscriptions.list_locations( + subscription_id=self.subscription_id) + for location in locations: + locations_list.append( + {'label': location.display_name, 'value': location.name}) + except ImportError: + return locations_list return locations_list def is_zone_redundant_ha_supported(self, region): @@ -515,8 +541,10 @@ def is_zone_redundant_ha_supported(self, region): else: self._available_capabilities_list = \ self._get_available_capabilities_list(region) - return self._available_capabilities_list[0][ - 'zone_redundant_ha_supported'] + if self._available_capabilities_list: + return self._available_capabilities_list[0][ + 'zone_redundant_ha_supported'] + return False def list_azure_availability_zones(self, region): """ @@ -620,6 +648,8 @@ def _get_available_capabilities_object(self, region): :return: azure capabilities object """ postgresql_client = self._get_azure_client('postgresql') + if not postgresql_client: + return [] return postgresql_client.location_based_capabilities.execute( location_name=region) diff --git a/web/pgadmin/misc/cloud/azure/tests/test_azure_session_state.py b/web/pgadmin/misc/cloud/azure/tests/test_azure_session_state.py index 7fba7b18650..f692053c384 100644 --- a/web/pgadmin/misc/cloud/azure/tests/test_azure_session_state.py +++ b/web/pgadmin/misc/cloud/azure/tests/test_azure_session_state.py @@ -206,3 +206,38 @@ def runTest(self): "cloud.azure must not import the unsafe deserializer") self.assertNotIn(forbidden + '.dumps(', src) self.assertNotIn(forbidden + '.loads(', src) + + +class TestAzureImportErrorHandling( + _SkipServerSetUpMixin, BaseTestGenerator): + """Azure methods must handle ImportError cleanly without raising.""" + + scenarios = [('default', dict())] + + def runTest(self): + from unittest.mock import patch + from pgadmin.misc.cloud.azure import Azure + import pgadmin.misc.cloud.azure as azure_mod + + a = Azure.from_state({'tenant_id': 'tid'}) + with patch.object( + azure_mod, '_azure_sdk', + side_effect=ImportError('No module named azure.mgmt')): + client = a._get_azure_client('postgresql') + self.assertIsNone(client) + + avail, msg = a.check_cluster_name_availability('server') + self.assertFalse(avail) + self.assertIn('azure.mgmt', msg) + + subs = a.list_subscriptions() + self.assertEqual(subs, []) + + rgs = a.list_resource_groups('sub-1') + self.assertEqual(rgs, []) + + regs = a.list_regions('sub-1') + self.assertEqual(regs, []) + + ha = a.is_zone_redundant_ha_supported('eastus') + self.assertFalse(ha) diff --git a/web/pgadmin/misc/cloud/google/__init__.py b/web/pgadmin/misc/cloud/google/__init__.py index 57aff041757..7bd1097d045 100644 --- a/web/pgadmin/misc/cloud/google/__init__.py +++ b/web/pgadmin/misc/cloud/google/__init__.py @@ -259,12 +259,11 @@ def get_instance_types(project_id, region, instance_class): """ google_obj = _get_google_from_session() if google_obj is not None and project_id and region: - instance_types_dict = google_obj.get_instance_types( + instance_types_dict, error = google_obj.get_instance_types( project_id, region) - instance_types_list, error = ( - instance_types_dict.get(instance_class, [])) if error: return bad_request(errormsg=sanitize_external_text(error)) + instance_types_list = instance_types_dict.get(instance_class, []) return make_json_response(data=instance_types_list) else: return make_json_response(data=[]) @@ -372,6 +371,9 @@ def clear_google_session(): def _google_sdk(): """Defer heavy Google API client imports until required by user actions. Repeat calls are cheap via sys.modules caching. + + Note: The oauth2client sentinel installed at module import (line 39) + must precede this googleapiclient import. See issue #10110. """ from types import SimpleNamespace from googleapiclient import discovery @@ -555,7 +557,10 @@ def get_projects(self): """ projects = [] error = None - sdk = _google_sdk() + try: + sdk = _google_sdk() + except ImportError as e: + return projects, str(e) credentials = self._get_credentials(self._scopes) service = sdk.discovery.build('cloudresourcemanager', self._cloud_resource_manager_api_version, @@ -579,7 +584,10 @@ def get_regions(self, project): :return: """ self._project_id = project - sdk = _google_sdk() + try: + sdk = _google_sdk() + except ImportError as e: + return self._regions, str(e) credentials = self._get_credentials(self._scopes) service = sdk.discovery.build('compute', self._compute_api_version, @@ -625,7 +633,10 @@ def get_instance_types(self, project, region): high_mem = [] instance_types = {} error = None - sdk = _google_sdk() + try: + sdk = _google_sdk() + except ImportError as e: + return instance_types, str(e) credentials = self._get_credentials(self._scopes) service = sdk.discovery.build('sqladmin', self._sqladmin_api_version, @@ -674,7 +685,10 @@ def get_database_versions(self): pg_database_versions = [] database_versions = [] error = None - sdk = _google_sdk() + try: + sdk = _google_sdk() + except ImportError as e: + return database_versions, str(e) credentials = self._get_credentials(self._scopes) service = sdk.discovery.build('sqladmin', self._sqladmin_api_version, diff --git a/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py b/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py index 720bae27dae..f3b80f644e4 100644 --- a/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py +++ b/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py @@ -202,3 +202,36 @@ def runTest(self): self.assertNotIn( forbidden + '.loads(', src, "cloud.google must not call the unsafe deserialize call") + + +class TestGoogleImportErrorHandling( + _SkipServerSetUpMixin, BaseTestGenerator): + """Google methods must cleanly return error strings on ImportError.""" + + scenarios = [('default', dict())] + + def runTest(self): + from unittest.mock import patch + from pgadmin.misc.cloud.google import Google + import pgadmin.misc.cloud.google as google_mod + + g = Google() + with patch.object( + google_mod, '_google_sdk', + side_effect=ImportError('No module named googleapiclient')): + projects, err_p = g.get_projects() + self.assertEqual(projects, []) + self.assertIn('googleapiclient', err_p) + + regions, err_r = g.get_regions('test-project') + self.assertEqual(regions, []) + self.assertIn('googleapiclient', err_r) + + inst_types, err_it = g.get_instance_types( + 'test-project', 'us-central1') + self.assertEqual(inst_types, {}) + self.assertIn('googleapiclient', err_it) + + db_vers, err_dv = g.get_database_versions() + self.assertEqual(db_vers, []) + self.assertIn('googleapiclient', err_dv) From bfb7d65a623a4fcae7cf228a8bb3b996155716a7 Mon Sep 17 00:00:00 2001 From: dev-hari-prasad Date: Wed, 2 Sep 2026 17:24:05 +0530 Subject: [PATCH 3/7] Handle ImportError on deferred InstalledAppFlow in Google callback - Update Google.callback to catch ImportError alongside AccessDeniedError. - Set verification failure state and return error string instead of raising HTTP 500. - Add regression test TestGoogleCallbackImportErrorHandling exercising missing google_auth_oauthlib. --- web/pgadmin/misc/cloud/google/__init__.py | 4 ++ .../google/tests/test_google_session_state.py | 46 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/web/pgadmin/misc/cloud/google/__init__.py b/web/pgadmin/misc/cloud/google/__init__.py index 7bd1097d045..d0fbcca087f 100644 --- a/web/pgadmin/misc/cloud/google/__init__.py +++ b/web/pgadmin/misc/cloud/google/__init__.py @@ -519,6 +519,10 @@ def callback(self, flask_request): if self._verification_error == 'access_denied': self._verification_error = 'Access denied.' return self._verification_error + except ImportError as er: + self._verification_successful = False + self._verification_error = str(er) + return self._verification_error @staticmethod def _credentials_to_dict(credentials): diff --git a/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py b/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py index f3b80f644e4..4547b76a9c2 100644 --- a/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py +++ b/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py @@ -235,3 +235,49 @@ def runTest(self): db_vers, err_dv = g.get_database_versions() self.assertEqual(db_vers, []) self.assertIn('googleapiclient', err_dv) + + +class TestGoogleCallbackImportErrorHandling( + _SkipServerSetUpMixin, BaseTestGenerator): + """Google.callback must cleanly handle missing google_auth_oauthlib.""" + + scenarios = [('default', dict())] + + def runTest(self): + from flask import Flask, session + from unittest.mock import MagicMock, patch + from pgadmin.misc.cloud.google import Google + import pgadmin.misc.cloud.google as google_mod + + app = Flask(__name__) + app.secret_key = 'test' + with app.test_request_context('/google/callback?state=xyz'): + session['state'] = 'xyz' + g = Google() + req = MagicMock() + req.url = 'http://localhost/google/callback?state=xyz' + req.args = {'state': 'xyz'} + with patch.dict( + 'sys.modules', + {'google_auth_oauthlib': None, + 'google_auth_oauthlib.flow': None}): + res = g.callback(req) + + self.assertIn('google_auth_oauthlib', res) + self.assertFalse(g._verification_successful) + self.assertEqual(g._verification_error, res) + ack_status, ack_err = g.verification_ack() + self.assertFalse(ack_status) + self.assertEqual(ack_err, res) + + # Verify route handler returns 200 with error message instead of 500 + session['google'] = {'state': g.to_state()} + with patch.dict( + 'sys.modules', + {'google_auth_oauthlib': None, + 'google_auth_oauthlib.flow': None}): + route_response = google_mod.callback.__wrapped__() + self.assertEqual(route_response.status_code, 200) + self.assertIn('google_auth_oauthlib', + route_response.get_data(as_text=True)) + From a179dd213c90ef97096dd3894742fff0d500f304 Mon Sep 17 00:00:00 2001 From: dev-hari-prasad Date: Wed, 2 Sep 2026 17:27:29 +0530 Subject: [PATCH 4/7] Fix pycodestyle formatting issues in test_google_session_state.py --- .../misc/cloud/google/tests/test_google_session_state.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py b/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py index 4547b76a9c2..81ee679f1c6 100644 --- a/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py +++ b/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py @@ -270,7 +270,8 @@ def runTest(self): self.assertFalse(ack_status) self.assertEqual(ack_err, res) - # Verify route handler returns 200 with error message instead of 500 + # Verify route handler returns 200 with error message + # instead of 500 session['google'] = {'state': g.to_state()} with patch.dict( 'sys.modules', @@ -280,4 +281,3 @@ def runTest(self): self.assertEqual(route_response.status_code, 200) self.assertIn('google_auth_oauthlib', route_response.get_data(as_text=True)) - From 91a97c6929b9b2bffc15285732437618a15e30a7 Mon Sep 17 00:00:00 2001 From: dev-hari-prasad Date: Fri, 4 Sep 2026 16:09:56 +0530 Subject: [PATCH 5/7] Handle missing boto3 ImportError cleanly in AWS RDS module - Catch ImportError at import site in RDS._get_aws_client() and record error message. - Update RDS.validate_credentials to return clean error response when boto3 client creation fails. - Guard get_available_db_version and get_available_db_instance_class to return empty collections instead of raising. - Add regression tests in TestRdsImportErrorHandling verifying 200 responses for routes when boto3 is missing. --- web/pgadmin/misc/cloud/rds/__init__.py | 35 +++++--- .../cloud/rds/tests/test_rds_session_state.py | 82 +++++++++++++++++++ 2 files changed, 105 insertions(+), 12 deletions(-) diff --git a/web/pgadmin/misc/cloud/rds/__init__.py b/web/pgadmin/misc/cloud/rds/__init__.py index cd0ba507345..dda38f2e360 100644 --- a/web/pgadmin/misc/cloud/rds/__init__.py +++ b/web/pgadmin/misc/cloud/rds/__init__.py @@ -19,7 +19,7 @@ from pgadmin.utils.text_sanitize import sanitize_external_text from pgadmin.misc.cloud.utils import _create_server, CloudProcessDesc from pgadmin.misc.bgprocess.processes import BatchProcess -from pgadmin.utils.ajax import make_json_response,\ +from pgadmin.utils.ajax import make_json_response, \ internal_server_error, bad_request, success_return from .regions import AWS_REGIONS import json @@ -177,6 +177,7 @@ class RDS(): def __init__(self, access_key, secret_key, session_token=None, default_region='ap-south-1'): self._clients = {} + self._error = None self._access_key = access_key self._secret_key = secret_key @@ -193,25 +194,33 @@ def _get_aws_client(self, type): return self._clients[type] # Defer boto3 (heavy import, user action only, cached) - import boto3 - session = boto3.Session( - aws_access_key_id=self._access_key, - aws_secret_access_key=self._secret_key, - aws_session_token=self._session_token - ) + try: + import boto3 + session = boto3.Session( + aws_access_key_id=self._access_key, + aws_secret_access_key=self._secret_key, + aws_session_token=self._session_token + ) - self._clients[type] = session.client( - type, region_name=self._default_region) + self._clients[type] = session.client( + type, region_name=self._default_region) - return self._clients[type] + return self._clients[type] + except ImportError as e: + self._error = str(e) + return None def get_available_db_version(self, engine='postgres'): rds = self._get_aws_client('rds') + if not rds: + return {'DBEngineVersions': []} return rds.describe_db_engine_versions(Engine=engine) def get_available_db_instance_class(self, engine='postgres', engine_version='10'): rds = self._get_aws_client('rds') + if not rds: + return [] _instances = rds.describe_orderable_db_instance_options( Engine=engine, EngineVersion=engine_version) @@ -235,14 +244,16 @@ def get_db_instance(self, instance_name): DBInstanceIdentifier=instance_name) def validate_credentials(self): - client = self._get_aws_client('sts') try: + client = self._get_aws_client('sts') + if not client: + return False, self._error or 'Failed to initialize AWS client.' identity = client.get_caller_identity() return True, identity except Exception as e: return False, str(e) finally: - self._clients.pop('sts') + self._clients.pop('sts', None) def _get_rds_from_session(): diff --git a/web/pgadmin/misc/cloud/rds/tests/test_rds_session_state.py b/web/pgadmin/misc/cloud/rds/tests/test_rds_session_state.py index 7f7171bd6b2..ac7564f353f 100644 --- a/web/pgadmin/misc/cloud/rds/tests/test_rds_session_state.py +++ b/web/pgadmin/misc/cloud/rds/tests/test_rds_session_state.py @@ -161,3 +161,85 @@ def runTest(self): self.assertNotIn( forbidden + '.loads(', src, "cloud.rds must not call the unsafe deserialize call") + + +class TestRdsImportErrorHandling( + _SkipServerSetUpMixin, BaseTestGenerator): + """RDS methods must handle ImportError cleanly without raising.""" + + scenarios = [('default', dict())] + + def runTest(self): + import json + from flask import Flask, session + from unittest.mock import patch + from pgadmin.misc.cloud import rds as rds_mod + from pgadmin.misc.cloud.rds import RDS + + rds = RDS( + access_key='AKIA_TEST', + secret_key='SECRET_TEST', + session_token='STS_TOKEN', + default_region='us-east-1', + ) + with patch.dict('sys.modules', {'boto3': None, 'boto3.session': None}): + client = rds._get_aws_client('rds') + self.assertIsNone(client) + + status, msg = rds.validate_credentials() + self.assertFalse(status) + self.assertIn('boto3', msg) + + db_versions = rds.get_available_db_version() + self.assertEqual(db_versions, {'DBEngineVersions': []}) + + db_instances = rds.get_available_db_instance_class() + self.assertEqual(db_instances, []) + + # Verify route handlers return 200 responses instead of 500 + app = Flask(__name__) + app.secret_key = 'test' + with app.test_request_context( + '/rds/verify_credentials/', + method='POST', + data=json.dumps({ + 'cloud': 'aws', + 'secret': { + 'access_key': 'AKIA_TEST', + 'secret_access_key': 'SECRET_TEST', + 'region': 'us-east-1', + }, + }), + content_type='application/json'): + session['aws'] = {} + route_resp = rds_mod.verify_credentials.__wrapped__() + self.assertEqual(route_resp.status_code, 200) + body = json.loads(route_resp.data) + self.assertFalse(body.get('success')) + self.assertIn('boto3', body.get('info')) + + with app.test_request_context('/rds/db_versions/'): + session['aws'] = { + 'secret': { + 'access_key': 'AKIA_TEST', + 'secret_access_key': 'SECRET_TEST', + 'region': 'us-east-1', + } + } + route_resp = rds_mod.get_db_versions.__wrapped__() + self.assertEqual(route_resp.status_code, 200) + body = json.loads(route_resp.data) + self.assertEqual(body.get('data'), []) + + with app.test_request_context('/rds/db_instances/'): + session['aws'] = { + 'secret': { + 'access_key': 'AKIA_TEST', + 'secret_access_key': 'SECRET_TEST', + 'region': 'us-east-1', + } + } + route_resp = rds_mod.get_db_instances.__wrapped__() + self.assertEqual(route_resp.status_code, 200) + body = json.loads(route_resp.data) + self.assertEqual(body.get('data'), []) From fe818d363663153924196a0e45e7abad918c6bd1 Mon Sep 17 00:00:00 2001 From: dev-hari-prasad Date: Fri, 4 Sep 2026 16:10:02 +0530 Subject: [PATCH 6/7] Return (client, error) tuple and propagate errors in Azure module - Update Azure._get_azure_client() to return (client, error) tuple matching Google cloud pattern. - Propagate SDK ImportError and credential failures to callers instead of discarding them. - Surface error in check_cluster_name_availability and log errors in list_subscriptions, list_resource_groups, list_regions, and _get_available_capabilities_object. - Add regression tests in TestAzureCredentialFailureHandling and update TestAzureImportErrorHandling. --- web/pgadmin/misc/cloud/azure/__init__.py | 42 +++++++---- .../azure/tests/test_azure_session_state.py | 70 ++++++++++++++++++- 2 files changed, 97 insertions(+), 15 deletions(-) diff --git a/web/pgadmin/misc/cloud/azure/__init__.py b/web/pgadmin/misc/cloud/azure/__init__.py index 0db1295f521..79afe2080f9 100644 --- a/web/pgadmin/misc/cloud/azure/__init__.py +++ b/web/pgadmin/misc/cloud/azure/__init__.py @@ -429,16 +429,16 @@ def _azure_interactive_auth(self): def _get_azure_client(self, type): """ Create/cache/return an Azure client object """ if type in self._clients: - return self._clients[type] + return self._clients[type], None status, _credentials = self._get_azure_credentials() if not status: - return None + return None, _credentials try: sdk = _azure_sdk() - except ImportError: - return None + except ImportError as e: + return None, str(e) if type == 'postgresql': client = sdk.PostgreSQLManagementClient(_credentials, @@ -450,7 +450,7 @@ def _get_azure_client(self, type): client = sdk.SubscriptionClient(_credentials) self._clients[type] = client - return self._clients[type] + return self._clients[type], None def check_cluster_name_availability(self, cluster_name): """ @@ -461,9 +461,9 @@ def check_cluster_name_availability(self, cluster_name): sdk = _azure_sdk() except ImportError as e: return False, str(e) - postgresql_client = self._get_azure_client('postgresql') + postgresql_client, error = self._get_azure_client('postgresql') if not postgresql_client: - return False, 'Failed to initialize Azure client.' + return False, error res = postgresql_client.check_name_availability.execute( sdk.NameAvailabilityRequest( name=cluster_name, @@ -478,15 +478,19 @@ def list_subscriptions(self): """ subscriptions_list = [] try: - subscription_client = self._get_azure_client('subscription') + subscription_client, error = self._get_azure_client('subscription') if not subscription_client: + if current_app: + current_app.logger.error(error) return subscriptions_list sub_list = subscription_client.subscriptions.list() for group in list(sub_list): subscriptions_list.append( {'subscription_id': group.subscription_id, 'subscription_name': group.display_name}) - except ImportError: + except ImportError as e: + if current_app: + current_app.logger.error(str(e)) return subscriptions_list return subscriptions_list @@ -499,8 +503,10 @@ def list_resource_groups(self, subscription_id): self.subscription_id = subscription_id resource_groups_list = [] try: - resource_client = self._get_azure_client('resource') + resource_client, error = self._get_azure_client('resource') if not resource_client: + if current_app: + current_app.logger.error(error) return resource_groups_list group_list = resource_client.resource_groups.list() for group in list(group_list): @@ -508,7 +514,9 @@ def list_resource_groups(self, subscription_id): {'label': group.name, 'value': group.name, 'region': group.location}) - except ImportError: + except ImportError as e: + if current_app: + current_app.logger.error(str(e)) return resource_groups_list return resource_groups_list @@ -521,15 +529,19 @@ def list_regions(self, subscription_id): self.subscription_id = subscription_id locations_list = [] try: - subscription_client = self._get_azure_client('subscription') + subscription_client, error = self._get_azure_client('subscription') if not subscription_client: + if current_app: + current_app.logger.error(error) return locations_list locations = subscription_client.subscriptions.list_locations( subscription_id=self.subscription_id) for location in locations: locations_list.append( {'label': location.display_name, 'value': location.name}) - except ImportError: + except ImportError as e: + if current_app: + current_app.logger.error(str(e)) return locations_list return locations_list @@ -647,8 +659,10 @@ def _get_available_capabilities_object(self, region): :param region: :return: azure capabilities object """ - postgresql_client = self._get_azure_client('postgresql') + postgresql_client, error = self._get_azure_client('postgresql') if not postgresql_client: + if current_app: + current_app.logger.error(error) return [] return postgresql_client.location_based_capabilities.execute( location_name=region) diff --git a/web/pgadmin/misc/cloud/azure/tests/test_azure_session_state.py b/web/pgadmin/misc/cloud/azure/tests/test_azure_session_state.py index f692053c384..5a0318db5c2 100644 --- a/web/pgadmin/misc/cloud/azure/tests/test_azure_session_state.py +++ b/web/pgadmin/misc/cloud/azure/tests/test_azure_session_state.py @@ -223,8 +223,9 @@ def runTest(self): with patch.object( azure_mod, '_azure_sdk', side_effect=ImportError('No module named azure.mgmt')): - client = a._get_azure_client('postgresql') + client, err = a._get_azure_client('postgresql') self.assertIsNone(client) + self.assertIn('azure.mgmt', err) avail, msg = a.check_cluster_name_availability('server') self.assertFalse(avail) @@ -241,3 +242,70 @@ def runTest(self): ha = a.is_zone_redundant_ha_supported('eastus') self.assertFalse(ha) + + from flask import Flask, current_app + app = Flask(__name__) + app.secret_key = 'test' + with app.app_context(): + with patch.object(current_app.logger, 'error') as mock_log: + self.assertEqual(a.list_subscriptions(), []) + mock_log.assert_called() + self.assertIn('azure.mgmt', str(mock_log.call_args[0][0])) + + with patch.object(current_app.logger, 'error') as mock_log: + self.assertEqual(a.list_resource_groups('sub-1'), []) + mock_log.assert_called() + self.assertIn('azure.mgmt', str(mock_log.call_args[0][0])) + + with patch.object(current_app.logger, 'error') as mock_log: + self.assertEqual(a.list_regions('sub-1'), []) + mock_log.assert_called() + self.assertIn('azure.mgmt', str(mock_log.call_args[0][0])) + + +class TestAzureCredentialFailureHandling( + _SkipServerSetUpMixin, BaseTestGenerator): + """Azure methods must propagate or log credential errors cleanly.""" + + scenarios = [('default', dict())] + + def runTest(self): + from flask import Flask, current_app + from unittest.mock import patch + from pgadmin.misc.cloud.azure import Azure + + a = Azure.from_state({'tenant_id': 'tid'}) + cred_error = 'Azure authentication failed: az login expired' + with patch.object( + a, '_get_azure_credentials', + return_value=(False, cred_error)): + client, err = a._get_azure_client('postgresql') + self.assertIsNone(client) + self.assertEqual(err, cred_error) + + avail, msg = a.check_cluster_name_availability('server') + self.assertFalse(avail) + self.assertEqual(msg, cred_error) + + app = Flask(__name__) + app.secret_key = 'test' + with app.app_context(): + with patch.object(current_app.logger, 'error') as mock_log: + subs = a.list_subscriptions() + self.assertEqual(subs, []) + mock_log.assert_called_with(cred_error) + + with patch.object(current_app.logger, 'error') as mock_log: + rgs = a.list_resource_groups('sub-1') + self.assertEqual(rgs, []) + mock_log.assert_called_with(cred_error) + + with patch.object(current_app.logger, 'error') as mock_log: + regs = a.list_regions('sub-1') + self.assertEqual(regs, []) + mock_log.assert_called_with(cred_error) + + with patch.object(current_app.logger, 'error') as mock_log: + caps = a._get_available_capabilities_object('eastus') + self.assertEqual(caps, []) + mock_log.assert_called_with(cred_error) From 83402008928165d5f5a0f844195ec75756b1e8c7 Mon Sep 17 00:00:00 2001 From: dev-hari-prasad Date: Fri, 4 Sep 2026 18:45:01 +0530 Subject: [PATCH 7/7] Guard Google client construction and credential refresh inside lookup methods - Move _get_credentials() and sdk.discovery.build() inside the protected try blocks in get_projects(), get_regions(), get_instance_types(), and get_database_versions(). - Convert credential refresh failures and client construction errors into each method's existing (result, error) return contract instead of letting unhandled exceptions escape to routes. - Add regression tests in TestGoogleCredentialsAndDiscoveryErrorHandling in test_google_session_state.py. --- web/pgadmin/misc/cloud/google/__init__.py | 33 +++++----- .../google/tests/test_google_session_state.py | 60 +++++++++++++++++++ 2 files changed, 77 insertions(+), 16 deletions(-) diff --git a/web/pgadmin/misc/cloud/google/__init__.py b/web/pgadmin/misc/cloud/google/__init__.py index d0fbcca087f..171f62f9022 100644 --- a/web/pgadmin/misc/cloud/google/__init__.py +++ b/web/pgadmin/misc/cloud/google/__init__.py @@ -565,11 +565,12 @@ def get_projects(self): sdk = _google_sdk() except ImportError as e: return projects, str(e) - credentials = self._get_credentials(self._scopes) - service = sdk.discovery.build('cloudresourcemanager', - self._cloud_resource_manager_api_version, - credentials=credentials) try: + credentials = self._get_credentials(self._scopes) + service = sdk.discovery.build( + 'cloudresourcemanager', + self._cloud_resource_manager_api_version, + credentials=credentials) req = service.projects().list() res = req.execute() for project in res.get('projects', []): @@ -592,12 +593,12 @@ def get_regions(self, project): sdk = _google_sdk() except ImportError as e: return self._regions, str(e) - credentials = self._get_credentials(self._scopes) - service = sdk.discovery.build('compute', - self._compute_api_version, - credentials=credentials) error = None try: + credentials = self._get_credentials(self._scopes) + service = sdk.discovery.build('compute', + self._compute_api_version, + credentials=credentials) req = service.regions().list(project=project) res = req.execute() for item in res.get('items', []): @@ -641,11 +642,11 @@ def get_instance_types(self, project, region): sdk = _google_sdk() except ImportError as e: return instance_types, str(e) - credentials = self._get_credentials(self._scopes) - service = sdk.discovery.build('sqladmin', - self._sqladmin_api_version, - credentials=credentials) try: + credentials = self._get_credentials(self._scopes) + service = sdk.discovery.build('sqladmin', + self._sqladmin_api_version, + credentials=credentials) req = service.tiers().list(project=project) res = req.execute() for item in res.get('items', []): @@ -693,11 +694,11 @@ def get_database_versions(self): sdk = _google_sdk() except ImportError as e: return database_versions, str(e) - credentials = self._get_credentials(self._scopes) - service = sdk.discovery.build('sqladmin', - self._sqladmin_api_version, - credentials=credentials) try: + credentials = self._get_credentials(self._scopes) + service = sdk.discovery.build('sqladmin', + self._sqladmin_api_version, + credentials=credentials) req = service.flags().list() res = req.execute() for item in res.get('items', []): diff --git a/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py b/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py index 81ee679f1c6..ae1821065ad 100644 --- a/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py +++ b/web/pgadmin/misc/cloud/google/tests/test_google_session_state.py @@ -281,3 +281,63 @@ def runTest(self): self.assertEqual(route_response.status_code, 200) self.assertIn('google_auth_oauthlib', route_response.get_data(as_text=True)) + + +class TestGoogleCredentialsAndDiscoveryErrorHandling( + _SkipServerSetUpMixin, BaseTestGenerator): + """Google methods must cleanly return error strings when credential + refresh or discovery build fails.""" + + scenarios = [('default', dict())] + + def runTest(self): + from unittest.mock import MagicMock, patch + from pgadmin.misc.cloud.google import Google + + g = Google() + + # Test credential refresh failure + with patch.object( + g, '_get_credentials', + side_effect=Exception('Token refresh failed')): + projects, err_p = g.get_projects() + self.assertEqual(projects, []) + self.assertIn('Token refresh failed', err_p) + + regions, err_r = g.get_regions('test-project') + self.assertEqual(regions, []) + self.assertIn('Token refresh failed', err_r) + + inst_types, err_it = g.get_instance_types( + 'test-project', 'us-central1') + self.assertEqual(inst_types, {}) + self.assertIn('Token refresh failed', err_it) + + db_vers, err_dv = g.get_database_versions() + self.assertEqual(db_vers, []) + self.assertIn('Token refresh failed', err_dv) + + # Test discovery build failure + mock_sdk = MagicMock() + mock_sdk.HttpError = type('HttpError', (Exception,), {}) + mock_sdk.discovery.build.side_effect = Exception( + 'Discovery service unavailable') + with patch('pgadmin.misc.cloud.google._google_sdk', + return_value=mock_sdk): + with patch.object(g, '_get_credentials', return_value=MagicMock()): + projects, err_p = g.get_projects() + self.assertEqual(projects, []) + self.assertIn('Discovery service unavailable', err_p) + + regions, err_r = g.get_regions('test-project') + self.assertEqual(regions, []) + self.assertIn('Discovery service unavailable', err_r) + + inst_types, err_it = g.get_instance_types( + 'test-project', 'us-central1') + self.assertEqual(inst_types, {}) + self.assertIn('Discovery service unavailable', err_it) + + db_vers, err_dv = g.get_database_versions() + self.assertEqual(db_vers, []) + self.assertIn('Discovery service unavailable', err_dv)