From 61efb66abaaa79c00638c2420508d04ccb33385f Mon Sep 17 00:00:00 2001 From: Mario de Frutos Date: Fri, 26 May 2017 11:51:32 +0200 Subject: [PATCH] Add an extra check for google credentials If the user has a wrong base64 padded secret key the googlemaps python library is returning "TypeError: Incorrect padding" which is very hard to understand. So now we check if the secret key is a valid base64 string --- .../cartodb_services/google/exceptions.py | 2 ++ .../cartodb_services/google/geocoder.py | 17 +++++++++++++++-- .../cartodb_services/metrics/config.py | 4 ++-- server/lib/python/cartodb_services/setup.py | 2 +- .../test/test_googlegeocoder.py | 13 +++++++++---- 5 files changed, 29 insertions(+), 9 deletions(-) diff --git a/server/lib/python/cartodb_services/cartodb_services/google/exceptions.py b/server/lib/python/cartodb_services/cartodb_services/google/exceptions.py index f4a3015..4a0e15e 100644 --- a/server/lib/python/cartodb_services/cartodb_services/google/exceptions.py +++ b/server/lib/python/cartodb_services/cartodb_services/google/exceptions.py @@ -2,6 +2,8 @@ # -*- coding: utf-8 -*- import json +class InvalidGoogleCredentials(Exception): + pass class BadGeocodingParams(Exception): def __init__(self, value): diff --git a/server/lib/python/cartodb_services/cartodb_services/google/geocoder.py b/server/lib/python/cartodb_services/cartodb_services/google/geocoder.py index 2a836f3..a8458a4 100644 --- a/server/lib/python/cartodb_services/cartodb_services/google/geocoder.py +++ b/server/lib/python/cartodb_services/cartodb_services/google/geocoder.py @@ -1,15 +1,18 @@ #!/usr/local/bin/python # -*- coding: utf-8 -*- +import base64 import googlemaps -from exceptions import MalformedResult +from exceptions import MalformedResult, InvalidGoogleCredentials class GoogleMapsGeocoder: """A Google Maps Geocoder wrapper for python""" def __init__(self, client_id, client_secret, logger): + if not self._valid_credentials(client_secret): + raise InvalidGoogleCredentials('Invalid google secret key') self.client_id = self._clean_client_id(client_id) self.client_secret = client_secret self.geocoder = googlemaps.Client( @@ -36,7 +39,7 @@ class GoogleMapsGeocoder: return [longitude, latitude] def _build_optional_parameters(self, city=None, state=None, - country=None): + country=None): optional_params = {} if city: optional_params['locality'] = city @@ -49,3 +52,13 @@ class GoogleMapsGeocoder: def _clean_client_id(self, client_id): # Consistency with how the client_id is saved in metadata return client_id.replace('client=', '') + + def _valid_credentials(self, private_key): + try: + # Only fails if the string dont have a correct padding for b64 + # but this way we could provide a more clear error than + # TypeError: Incorrect padding + base64.b64decode(private_key) + return True + except TypeError: + return False diff --git a/server/lib/python/cartodb_services/cartodb_services/metrics/config.py b/server/lib/python/cartodb_services/cartodb_services/metrics/config.py index ee849c3..6ad260f 100644 --- a/server/lib/python/cartodb_services/cartodb_services/metrics/config.py +++ b/server/lib/python/cartodb_services/cartodb_services/metrics/config.py @@ -359,10 +359,10 @@ class GeocoderConfig(ServiceConfig): if self._geocoder_provider == self.NOKIA_GEOCODER: if not set(self.NOKIA_GEOCODER_REDIS_MANDATORY_KEYS).issubset(set(filtered_config.keys())) or \ not self.heremaps_app_id or not self.heremaps_app_code: - raise ConfigException("""Some mandatory parameter/s for Nokia geocoder are missing. Check it please""") + raise ConfigException("""Heremaps app id or app code is not set up""") elif self._geocoder_provider == self.GOOGLE_GEOCODER: if self.GOOGLE_GEOCODER_API_KEY not in filtered_config.keys(): - raise ConfigException("""Google geocoder need the mandatory parameter 'google_maps_private_key'""") + raise ConfigException("""Google geocoder private key is not set up""") elif self._geocoder_provider == self.MAPZEN_GEOCODER: if not self.mapzen_api_key: raise ConfigException("""Mapzen config is not set up""") diff --git a/server/lib/python/cartodb_services/setup.py b/server/lib/python/cartodb_services/setup.py index 681e064..1517adb 100644 --- a/server/lib/python/cartodb_services/setup.py +++ b/server/lib/python/cartodb_services/setup.py @@ -10,7 +10,7 @@ from setuptools import setup, find_packages setup( name='cartodb_services', - version='0.15.0', + version='0.15.1', description='CartoDB Services API Python Library', diff --git a/server/lib/python/cartodb_services/test/test_googlegeocoder.py b/server/lib/python/cartodb_services/test/test_googlegeocoder.py index 5593560..f700ec3 100644 --- a/server/lib/python/cartodb_services/test/test_googlegeocoder.py +++ b/server/lib/python/cartodb_services/test/test_googlegeocoder.py @@ -6,9 +6,7 @@ import requests_mock from mock import Mock from cartodb_services.google import GoogleMapsGeocoder -from cartodb_services.google.exceptions import BadGeocodingParams -from cartodb_services.google.exceptions import NoGeocodingParams -from cartodb_services.google.exceptions import MalformedResult +from cartodb_services.google.exceptions import MalformedResult, InvalidGoogleCredentials requests_mock.Mocker.TEST_PREFIX = 'test_' @@ -92,7 +90,8 @@ class GoogleGeocoderTestCase(unittest.TestCase): def setUp(self): logger = Mock() self.geocoder = GoogleMapsGeocoder('dummy_client_id', - 'MgxyOFxjZXIyOGO52jJlMzEzY1Oqy4hsO49E', logger) + 'MgxyOFxjZXIyOGO52jJlMzEzY1Oqy4hsO49E', + logger) def test_geocode_address_with_valid_params(self, req_mock): req_mock.register_uri('GET', self.GOOGLE_MAPS_GEOCODER_URL, @@ -119,3 +118,9 @@ class GoogleGeocoderTestCase(unittest.TestCase): searchtext='Calle Eloy Gonzalo 27', city='Madrid', country='España') + + def test_invalid_credentials(self, req_mock): + with self.assertRaises(InvalidGoogleCredentials): + GoogleMapsGeocoder('dummy_client_id', + 'lalala', + None)