Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(storage): Add cname support for V4 signature #72

Merged
merged 14 commits into from Mar 11, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion google/cloud/storage/_signing.py
Expand Up @@ -553,7 +553,7 @@ def generate_signed_url_v4(

header_names = [key.lower() for key in headers]
if "host" not in header_names:
headers["Host"] = "storage.googleapis.com"
headers["Host"] = six.moves.urllib.parse.urlparse(api_access_endpoint).netloc

if method.upper() == "RESUMABLE":
method = "POST"
Expand Down
25 changes: 24 additions & 1 deletion google/cloud/storage/blob.py
Expand Up @@ -362,6 +362,8 @@ def generate_signed_url(
service_account_email=None,
access_token=None,
virtual_hosted_style=False,
bucket_bound_host_name=None,
host_name_scheme="http",
):
"""Generates a signed URL for this blob.

Expand Down Expand Up @@ -460,6 +462,18 @@ def generate_signed_url(
(Optional) If true, then construct the URL relative the bucket's
virtual hostname, e.g., '<bucket-name>.storage.googleapis.com'.

:type bucket_bound_host_name: str
:param bucket_bound_host_name:
(Optional) If pass, then construct the URL relative to the bucket-bound hostname
as a CNAME. Value cane be a bare or with scheme, e.g., 'example.com ' or http://example.com.
See: https://cloud.google.com/storage/docs/request-endpoints#cname

:type host_name_scheme: str
:param host_name_scheme:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer scheme here.

(Optional) If ``bucket_bound_host_name`` is passed as a bare hostname, use
this value as the scheme. ``https`` will work only when using a CDN.
Defaults to ``"http"``.

:raises: :exc:`ValueError` when version is invalid.
:raises: :exc:`TypeError` when expiration is not a valid type.
:raises: :exc:`AttributeError` if credentials is not an instance
Expand All @@ -480,12 +494,21 @@ def generate_signed_url(
api_access_endpoint = "https://{bucket_name}.storage.googleapis.com".format(
bucket_name=self.bucket.name
)
resource = "/{quoted_name}".format(quoted_name=quoted_name)
elif bucket_bound_host_name:
if ":" in bucket_bound_host_name:
api_access_endpoint = bucket_bound_host_name
else:
api_access_endpoint = "{scheme}://{cname}".format(
scheme=host_name_scheme, cname=bucket_bound_host_name
)
else:
resource = "/{bucket_name}/{quoted_name}".format(
bucket_name=self.bucket.name, quoted_name=quoted_name
)

if virtual_hosted_style or bucket_bound_host_name:
resource = "/{quoted_name}".format(quoted_name=quoted_name)

if credentials is None:
client = self._require_client(client)
credentials = client._credentials
Expand Down
27 changes: 26 additions & 1 deletion google/cloud/storage/bucket.py
Expand Up @@ -2355,6 +2355,8 @@ def generate_signed_url(
credentials=None,
version=None,
virtual_hosted_style=False,
bucket_bound_host_name=None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use: bucket_bound_hostname

host_name_scheme="http",
):
"""Generates a signed URL for this bucket.

Expand All @@ -2373,6 +2375,8 @@ def generate_signed_url(
amount of time, you can use this method to generate a URL that
is only valid within a certain time period.

If ``cname`` is set as an argument of :attr:`api_access_endpoint`, ``https`` works only if using a ``CDN``.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cname no longer used in method signature should be updated.


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an inline example of using bucket_bound_hostname and scheme

This is particularly useful if you don't want publicly
accessible buckets, but don't want to require users to explicitly
log in.
Expand Down Expand Up @@ -2422,6 +2426,18 @@ def generate_signed_url(
(Optional) If true, then construct the URL relative the bucket's
virtual hostname, e.g., '<bucket-name>.storage.googleapis.com'.

:type bucket_bound_host_name: str
:param bucket_bound_host_name:
(Optional) If pass, then construct the URL relative to the bucket-bound hostname
as a CNAME. Value cane be a bare or with scheme, e.g., 'example.com ' or http://example.com.
See: https://cloud.google.com/storage/docs/request-endpoints#cname

:type host_name_scheme: str
:param host_name_scheme:
(Optional) If ``bucket_bound_host_name`` is passed as a bare hostname, use
this value as the scheme. ``https`` will work only when using a CDN.
Defaults to ``"http"``.

:raises: :exc:`ValueError` when version is invalid.
:raises: :exc:`TypeError` when expiration is not a valid type.
:raises: :exc:`AttributeError` if credentials is not an instance
Expand All @@ -2440,10 +2456,19 @@ def generate_signed_url(
api_access_endpoint = "https://{bucket_name}.storage.googleapis.com".format(
bucket_name=self.name
)
resource = "/"
elif bucket_bound_host_name:
if ":" in bucket_bound_host_name:
api_access_endpoint = bucket_bound_host_name
else:
api_access_endpoint = "{scheme}://{cname}".format(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use bucket_bound_hostname to not go back and forth between cname

scheme=host_name_scheme, cname=bucket_bound_host_name
)
else:
resource = "/{bucket_name}".format(bucket_name=self.name)

if virtual_hosted_style or bucket_bound_host_name:
resource = "/"

if credentials is None:
client = self._require_client(client)
credentials = client._credentials
Expand Down
22 changes: 21 additions & 1 deletion tests/unit/test_blob.py
Expand Up @@ -400,6 +400,8 @@ def _generate_signed_url_helper(
access_token=None,
service_account_email=None,
virtual_hosted_style=False,
bucket_bound_host_name=None,
host_name_scheme="http",
):
from six.moves.urllib import parse
from google.cloud._helpers import UTC
Expand Down Expand Up @@ -444,6 +446,7 @@ def _generate_signed_url_helper(
access_token=access_token,
service_account_email=service_account_email,
virtual_hosted_style=virtual_hosted_style,
bucket_bound_host_name=bucket_bound_host_name,
)

self.assertEqual(signed_uri, signer.return_value)
Expand All @@ -460,11 +463,20 @@ def _generate_signed_url_helper(
expected_api_access_endpoint = "https://{}.storage.googleapis.com".format(
bucket.name
)
expected_resource = "/{}".format(quoted_name)
elif bucket_bound_host_name:
if ":" in bucket_bound_host_name:
expected_api_access_endpoint = bucket_bound_host_name
else:
expected_api_access_endpoint = "{scheme}://{cname}".format(
scheme=host_name_scheme, cname=bucket_bound_host_name
)
else:
expected_api_access_endpoint = api_access_endpoint
expected_resource = "/{}/{}".format(bucket.name, quoted_name)

if virtual_hosted_style or bucket_bound_host_name:
expected_resource = "/{}".format(quoted_name)

if encryption_key is not None:
expected_headers = headers or {}
if effective_version == "v2":
Expand Down Expand Up @@ -619,6 +631,14 @@ def test_generate_signed_url_v4_w_csek_and_headers(self):
def test_generate_signed_url_v4_w_virtual_hostname(self):
self._generate_signed_url_v4_helper(virtual_hosted_style=True)

def test_generate_signed_url_v4_w_bucket_bound_hostname_w_scheme(self):
self._generate_signed_url_v4_helper(
bucket_bound_host_name="http://cdn.example.com"
)

def test_generate_signed_url_v4_w_bucket_bound_hostname_w_bare_hostname(self):
self._generate_signed_url_v4_helper(bucket_bound_host_name="cdn.example.com")

def test_generate_signed_url_v4_w_credentials(self):
credentials = object()
self._generate_signed_url_v4_helper(credentials=credentials)
Expand Down
22 changes: 21 additions & 1 deletion tests/unit/test_bucket.py
Expand Up @@ -2740,6 +2740,8 @@ def _generate_signed_url_helper(
credentials=None,
expiration=None,
virtual_hosted_style=False,
bucket_bound_host_name=None,
host_name_scheme="http",
):
from six.moves.urllib import parse
from google.cloud._helpers import UTC
Expand Down Expand Up @@ -2775,6 +2777,7 @@ def _generate_signed_url_helper(
query_parameters=query_parameters,
version=version,
virtual_hosted_style=virtual_hosted_style,
bucket_bound_host_name=bucket_bound_host_name,
)

self.assertEqual(signed_uri, signer.return_value)
Expand All @@ -2788,11 +2791,20 @@ def _generate_signed_url_helper(
expected_api_access_endpoint = "https://{}.storage.googleapis.com".format(
bucket_name
)
expected_resource = "/"
elif bucket_bound_host_name:
if ":" in bucket_bound_host_name:
expected_api_access_endpoint = bucket_bound_host_name
else:
expected_api_access_endpoint = "{scheme}://{cname}".format(
scheme=host_name_scheme, cname=bucket_bound_host_name
)
else:
expected_api_access_endpoint = api_access_endpoint
expected_resource = "/{}".format(parse.quote(bucket_name))

if virtual_hosted_style or bucket_bound_host_name:
expected_resource = "/"

expected_kwargs = {
"resource": expected_resource,
"expiration": expiration,
Expand Down Expand Up @@ -2928,6 +2940,14 @@ def test_generate_signed_url_v4_w_credentials(self):
def test_generate_signed_url_v4_w_virtual_hostname(self):
self._generate_signed_url_v4_helper(virtual_hosted_style=True)

def test_generate_signed_url_v4_w_bucket_bound_hostname_w_scheme(self):
self._generate_signed_url_v4_helper(
bucket_bound_host_name="http://cdn.example.com"
)

def test_generate_signed_url_v4_w_bucket_bound_hostname_w_bare_hostname(self):
self._generate_signed_url_v4_helper(bucket_bound_host_name="cdn.example.com")


class _Connection(object):
_delete_bucket = False
Expand Down