fix(storage): anonymous credentials for private bucket (#107)

* fix(storage): anonymous credentials for private bucket

* fix(storage): use API_BASE_URL for hostname

* fix(storage): nit

* fix(storage): fix system test

* fix(storage): revert changes

Co-authored-by: Frank Natividad <[email protected]>

Author: HemangChothani

google/cloud/storage/blob.py

@@ -54,7 +54,6 @@
 from google.cloud._helpers import _rfc3339_to_datetime
 from google.cloud._helpers import _to_bytes
 from google.cloud.exceptions import NotFound
-from google.cloud.storage._helpers import _get_storage_host
 from google.cloud.storage._helpers import _PropertyMixin
 from google.cloud.storage._helpers import _scalar_property
 from google.cloud.storage._helpers import _convert_to_timestamp
@@ -70,12 +69,11 @@
 from google.cloud.storage.constants import REGIONAL_LEGACY_STORAGE_CLASS
 from google.cloud.storage.constants import STANDARD_STORAGE_CLASS
 
-_STORAGE_HOST = _get_storage_host()
 
 _API_ACCESS_ENDPOINT = "https://storage.googleapis.com"
 _DEFAULT_CONTENT_TYPE = u"application/octet-stream"
-_DOWNLOAD_URL_TEMPLATE = _STORAGE_HOST + u"/download/storage/v1{path}?alt=media"
-_BASE_UPLOAD_TEMPLATE = _STORAGE_HOST + u"/upload/storage/v1{bucket_path}/o?uploadType="
+_DOWNLOAD_URL_TEMPLATE = u"{hostname}/download/storage/v1{path}?alt=media"
+_BASE_UPLOAD_TEMPLATE = u"{hostname}/upload/storage/v1{bucket_path}/o?uploadType="
 _MULTIPART_URL_TEMPLATE = _BASE_UPLOAD_TEMPLATE + u"multipart"
 _RESUMABLE_URL_TEMPLATE = _BASE_UPLOAD_TEMPLATE + u"resumable"
 # NOTE: "acl" is also writeable but we defer ACL management to
@@ -650,19 +648,24 @@ def _get_transport(self, client):
         client = self._require_client(client)
         return client._http
 
-    def _get_download_url(self):
+    def _get_download_url(self, client):
         """Get the download URL for the current blob.
 
         If the ``media_link`` has been loaded, it will be used, otherwise
         the URL will be constructed from the current blob's path (and possibly
         generation) to avoid a round trip.
 
+        :type client: :class:`~google.cloud.storage.client.Client`
+        :param client: The client to use.
+
         :rtype: str
         :returns: The download URL for the current blob.
         """
         name_value_pairs = []
         if self.media_link is None:
-            base_url = _DOWNLOAD_URL_TEMPLATE.format(path=self.path)
+            base_url = _DOWNLOAD_URL_TEMPLATE.format(
+                hostname=client._connection.API_BASE_URL, path=self.path
+            )
             if self.generation is not None:
                 name_value_pairs.append(("generation", "{:d}".format(self.generation)))
         else:
@@ -790,7 +793,8 @@ def download_to_file(
 
         :raises: :class:`google.cloud.exceptions.NotFound`
         """
-        download_url = self._get_download_url()
+        client = self._require_client(client)
+        download_url = self._get_download_url(client)
         headers = _get_encryption_headers(self._encryption_key)
         headers["accept-encoding"] = "gzip"
 
@@ -1055,7 +1059,9 @@ def _do_multipart_upload(
         info = self._get_upload_arguments(content_type)
         headers, object_metadata, content_type = info
 
-        base_url = _MULTIPART_URL_TEMPLATE.format(bucket_path=self.bucket.path)
+        base_url = _MULTIPART_URL_TEMPLATE.format(
+            hostname=self.client._connection.API_BASE_URL, bucket_path=self.bucket.path
+        )
         name_value_pairs = []
 
         if self.user_project is not None:
@@ -1190,7 +1196,9 @@ def _initiate_resumable_upload(
         if extra_headers is not None:
             headers.update(extra_headers)
 
-        base_url = _RESUMABLE_URL_TEMPLATE.format(bucket_path=self.bucket.path)
+        base_url = _RESUMABLE_URL_TEMPLATE.format(
+            hostname=self.client._connection.API_BASE_URL, bucket_path=self.bucket.path
+        )
         name_value_pairs = []
 
         if self.user_project is not None:

tests/unit/test_blob.py

@@ -751,7 +751,10 @@ def test__get_download_url_with_media_link(self):
         # Set the media link on the blob
         blob._properties["mediaLink"] = media_link
 
-        download_url = blob._get_download_url()
+        client = mock.Mock(_connection=_Connection)
+        client._connection.API_BASE_URL = "https://storage.googleapis.com"
+        download_url = blob._get_download_url(client)
+
         self.assertEqual(download_url, media_link)
 
     def test__get_download_url_with_media_link_w_user_project(self):
@@ -763,7 +766,10 @@ def test__get_download_url_with_media_link_w_user_project(self):
         # Set the media link on the blob
         blob._properties["mediaLink"] = media_link
 
-        download_url = blob._get_download_url()
+        client = mock.Mock(_connection=_Connection)
+        client._connection.API_BASE_URL = "https://storage.googleapis.com"
+        download_url = blob._get_download_url(client)
+
         self.assertEqual(
             download_url, "{}?userProject={}".format(media_link, user_project)
         )
@@ -774,7 +780,9 @@ def test__get_download_url_on_the_fly(self):
         blob = self._make_one(blob_name, bucket=bucket)
 
         self.assertIsNone(blob.media_link)
-        download_url = blob._get_download_url()
+        client = mock.Mock(_connection=_Connection)
+        client._connection.API_BASE_URL = "https://storage.googleapis.com"
+        download_url = blob._get_download_url(client)
         expected_url = (
             "https://storage.googleapis.com/download/storage/v1/b/"
             "buhkit/o/bzzz-fly.txt?alt=media"
@@ -790,7 +798,9 @@ def test__get_download_url_on_the_fly_with_generation(self):
         blob._properties["generation"] = str(generation)
 
         self.assertIsNone(blob.media_link)
-        download_url = blob._get_download_url()
+        client = mock.Mock(_connection=_Connection)
+        client._connection.API_BASE_URL = "https://storage.googleapis.com"
+        download_url = blob._get_download_url(client)
         expected_url = (
             "https://storage.googleapis.com/download/storage/v1/b/"
             "fictional/o/pretend.txt?alt=media&generation=1493058489532987"
@@ -804,7 +814,9 @@ def test__get_download_url_on_the_fly_with_user_project(self):
         blob = self._make_one(blob_name, bucket=bucket)
 
         self.assertIsNone(blob.media_link)
-        download_url = blob._get_download_url()
+        client = mock.Mock(_connection=_Connection)
+        client._connection.API_BASE_URL = "https://storage.googleapis.com"
+        download_url = blob._get_download_url(client)
         expected_url = (
             "https://storage.googleapis.com/download/storage/v1/b/"
             "fictional/o/pretend.txt?alt=media&userProject={}".format(user_project)
@@ -823,7 +835,9 @@ def test__get_download_url_on_the_fly_with_kms_key_name(self):
         blob = self._make_one(blob_name, bucket=bucket, kms_key_name=kms_resource)
 
         self.assertIsNone(blob.media_link)
-        download_url = blob._get_download_url()
+        client = mock.Mock(_connection=_Connection)
+        client._connection.API_BASE_URL = "https://storage.googleapis.com"
+        download_url = blob._get_download_url(client)
         expected_url = (
             "https://storage.googleapis.com/download/storage/v1/b/"
             "buhkit/o/bzzz-fly.txt?alt=media"
@@ -1003,7 +1017,8 @@ def test_download_to_file_with_failure(self):
 
     def test_download_to_file_wo_media_link(self):
         blob_name = "blob-name"
-        client = mock.Mock(spec=[u"_http"])
+        client = mock.Mock(_connection=_Connection, spec=[u"_http"])
+        client._connection.API_BASE_URL = "https://storage.googleapis.com"
         bucket = _Bucket(client)
         blob = self._make_one(blob_name, bucket=bucket)
         blob._do_download = mock.Mock()
@@ -1319,7 +1334,8 @@ def _do_multipart_success(
         transport = self._mock_transport(http_client.OK, {})
 
         # Create some mock arguments.
-        client = mock.Mock(_http=transport, spec=["_http"])
+        client = mock.Mock(_http=transport, _connection=_Connection, spec=["_http"])
+        client._connection.API_BASE_URL = "https://storage.googleapis.com"
         data = b"data here hear hier"
         stream = io.BytesIO(data)
         content_type = u"application/xml"
@@ -1485,7 +1501,8 @@ def _initiate_resumable_helper(
         transport = self._mock_transport(http_client.OK, response_headers)
 
         # Create some mock arguments and call the method under test.
-        client = mock.Mock(_http=transport, spec=[u"_http"])
+        client = mock.Mock(_http=transport, _connection=_Connection, spec=[u"_http"])
+        client._connection.API_BASE_URL = "https://storage.googleapis.com"
         data = b"hello hallo halo hi-low"
         stream = io.BytesIO(data)
         content_type = u"text/plain"
@@ -1772,7 +1789,8 @@ def _do_resumable_helper(
         )
 
         # Create some mock arguments and call the method under test.
-        client = mock.Mock(_http=transport, spec=["_http"])
+        client = mock.Mock(_http=transport, _connection=_Connection, spec=["_http"])
+        client._connection.API_BASE_URL = "https://storage.googleapis.com"
         stream = io.BytesIO(data)
         content_type = u"text/html"
         response = blob._do_resumable_upload(
@@ -2114,7 +2132,8 @@ def _create_resumable_upload_session_helper(self, origin=None, side_effect=None)
         # Create some mock arguments and call the method under test.
         content_type = u"text/plain"
         size = 10000
-        client = mock.Mock(_http=transport, spec=[u"_http"])
+        client = mock.Mock(_http=transport, _connection=_Connection, spec=[u"_http"])
+        client._connection.API_BASE_URL = "https://storage.googleapis.com"
         new_url = blob.create_resumable_upload_session(
             content_type=content_type, size=size, origin=origin, client=client
         )

tests/unit/test_client.py

@@ -137,18 +137,16 @@ def test_ctor_w_empty_client_options(self):
         )
 
     def test_ctor_w_client_options_dict(self):
-
         PROJECT = "PROJECT"
         credentials = _make_credentials()
-        client_options = {"api_endpoint": "https://www.foo-googleapis.com"}
+        api_endpoint = "https://www.foo-googleapis.com"
+        client_options = {"api_endpoint": api_endpoint}
 
         client = self._make_one(
             project=PROJECT, credentials=credentials, client_options=client_options
         )
 
-        self.assertEqual(
-            client._connection.API_BASE_URL, "https://www.foo-googleapis.com"
-        )
+        self.assertEqual(client._connection.API_BASE_URL, api_endpoint)
 
     def test_ctor_w_client_options_object(self):
         from google.api_core.client_options import ClientOptions