Repository: firebase/firebase-admin-python

Issue:
Validate Project ID String Globally
We currently check if project_id is a string in each of the service modules. This can be done in one place -- namely when the project_id is first read in the `App.__init__()` method.

Discussion:

Patch:
diff --git a/firebase_admin/__init__.py b/firebase_admin/__init__.py
index 0bf08cd..03471a6 100644
--- a/firebase_admin/__init__.py
+++ b/firebase_admin/__init__.py
@@ -218,15 +218,38 @@ class App(object):
         self._options = _AppOptions(options)
         self._lock = threading.RLock()
         self._services = {}
-        pid = self._options.get('projectId')
+        self._project_id = App._lookup_project_id(self._credential, self._options)
+
+    @classmethod
+    def _lookup_project_id(cls, credential, options):
+        """Looks up the Firebase project ID associated with an App.
+
+        This method first inspects the app options for a ``projectId`` entry. Then it attempts to
+        get the project ID from the credential used to initialize the app. If that also fails,
+        attempts to look up the ``GCLOUD_PROJECT`` environment variable.
+
+        Args:
+            credential: A Firebase credential instance.
+            options: A Firebase AppOptions instance.
+
+        Returns:
+            str: A project ID string or None.
+
+        Raises:
+            ValueError: If a non-string project ID value is specified.
+        """
+        pid = options.get('projectId')
         if not pid:
             try:
-                pid = self._credential.project_id
+                pid = credential.project_id
             except AttributeError:
                 pass
         if not pid:
             pid = os.environ.get('GCLOUD_PROJECT')
-        self._project_id = pid
+        if pid is not None and not isinstance(pid, six.string_types):
+            raise ValueError(
+                'Invalid project ID: "{0}". project ID must be a string.'.format(pid))
+        return pid
 
     @property
     def name(self):
diff --git a/firebase_admin/firestore.py b/firebase_admin/firestore.py
index 0191c00..1c32368 100644
--- a/firebase_admin/firestore.py
+++ b/firebase_admin/firestore.py
@@ -28,8 +28,6 @@ except ImportError:
     raise ImportError('Failed to import the Cloud Firestore library for Python. Make sure '
                       'to install the "google-cloud-firestore" module.')
 
-import six
-
 from firebase_admin import _utils
 
 
@@ -75,7 +73,4 @@ class _FirestoreClient(object):
                 'Project ID is required to access Firestore. Either set the projectId option, '
                 'or use service account credentials. Alternatively, set the GCLOUD_PROJECT '
                 'environment variable.')
-        elif not isinstance(project, six.string_types):
-            raise ValueError(
-                'Invalid project ID: "{0}". project ID must be a string.'.format(project))
         return _FirestoreClient(credentials, project)
diff --git a/firebase_admin/instance_id.py b/firebase_admin/instance_id.py
index 5e4f5d4..70ace55 100644
--- a/firebase_admin/instance_id.py
+++ b/firebase_admin/instance_id.py
@@ -79,9 +79,6 @@ class _InstanceIdService(object):
                 'Project ID is required to access Instance ID service. Either set the projectId '
                 'option, or use service account credentials. Alternatively, set the '
                 'GCLOUD_PROJECT environment variable.')
-        elif not isinstance(project_id, six.string_types):
-            raise ValueError(
-                'Invalid project ID: "{0}". project ID must be a string.'.format(project_id))
         self._project_id = project_id
         self._client = _http_client.JsonHttpClient(
             credential=app.credential.get_credential(), base_url=_IID_SERVICE_URL)


Test patch:
diff --git a/tests/test_app.py b/tests/test_app.py
index e4450eb..aaa3f0a 100644
--- a/tests/test_app.py
+++ b/tests/test_app.py
@@ -317,6 +317,11 @@ class TestFirebaseApp(object):
             if project_id:
                 os.environ[GCLOUD_PROJECT] = project_id
 
+    def test_non_string_project_id(self):
+        options = {'projectId': {'key': 'not a string'}}
+        with pytest.raises(ValueError):
+            firebase_admin.initialize_app(CREDENTIAL, options=options)
+
     def test_app_get(self, init_app):
         assert init_app is firebase_admin.get_app(init_app.name)