From 1d2d7f96d98e7496d5c33b6848d63ab2825080a3 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 26 Feb 2019 14:45:23 -0500 Subject: [PATCH 01/37] Rename '_signing.generate_signed_url' -> 'generate_signed_url_v2'. In preparation for adding V4 versions. --- storage/google/cloud/storage/_signing.py | 4 ++-- storage/google/cloud/storage/blob.py | 4 ++-- storage/tests/unit/test__signing.py | 6 +++--- storage/tests/unit/test_blob.py | 16 ++++++++-------- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index 92703107421d..d908edc0fed6 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -109,7 +109,7 @@ def get_expiration_seconds(expiration): return expiration -def generate_signed_url( +def generate_signed_url_v2( credentials, resource, expiration, @@ -121,7 +121,7 @@ def generate_signed_url( response_disposition=None, generation=None, ): - """Generate signed URL to provide query-string auth'n to a resource. + """Generate a V2 signed URL to provide query-string auth'n to a resource. .. note:: diff --git a/storage/google/cloud/storage/blob.py b/storage/google/cloud/storage/blob.py index 2fec5c8a1ebc..852a04fe7b94 100644 --- a/storage/google/cloud/storage/blob.py +++ b/storage/google/cloud/storage/blob.py @@ -54,7 +54,7 @@ from google.api_core.iam import Policy from google.cloud.storage._helpers import _PropertyMixin from google.cloud.storage._helpers import _scalar_property -from google.cloud.storage._signing import generate_signed_url +from google.cloud.storage._signing import generate_signed_url_v2 from google.cloud.storage.acl import ACL from google.cloud.storage.acl import ObjectACL @@ -387,7 +387,7 @@ def generate_signed_url( client = self._require_client(client) credentials = client._credentials - return generate_signed_url( + return generate_signed_url_v2( credentials, resource=resource, api_access_endpoint=_API_ACCESS_ENDPOINT, diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index 01922ca97849..8ffd3d1483b4 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -126,12 +126,12 @@ def test_it(self): credentials.sign_bytes.assert_called_once_with(string_to_sign) -class Test_generate_signed_url(unittest.TestCase): +class Test_generate_signed_url_v2(unittest.TestCase): @staticmethod def _call_fut(*args, **kwargs): - from google.cloud.storage._signing import generate_signed_url + from google.cloud.storage._signing import generate_signed_url_v2 - return generate_signed_url(*args, **kwargs) + return generate_signed_url_v2(*args, **kwargs) def _generate_helper( self, response_type=None, response_disposition=None, generation=None diff --git a/storage/tests/unit/test_blob.py b/storage/tests/unit/test_blob.py index db4574cd7675..1a6e9148f27b 100644 --- a/storage/tests/unit/test_blob.py +++ b/storage/tests/unit/test_blob.py @@ -364,7 +364,7 @@ def _basic_generate_signed_url_helper(self, credentials=None): ) SIGNER = _Signer() - with mock.patch("google.cloud.storage.blob.generate_signed_url", new=SIGNER): + with mock.patch("google.cloud.storage.blob.generate_signed_url_v2", new=SIGNER): signed_uri = blob.generate_signed_url(EXPIRATION, credentials=credentials) self.assertEqual(signed_uri, URI) @@ -402,7 +402,7 @@ def test_generate_signed_url_w_content_type(self): SIGNER = _Signer() CONTENT_TYPE = "text/html" - with mock.patch("google.cloud.storage.blob.generate_signed_url", new=SIGNER): + with mock.patch("google.cloud.storage.blob.generate_signed_url_v2", new=SIGNER): signed_url = blob.generate_signed_url(EXPIRATION, content_type=CONTENT_TYPE) self.assertEqual(signed_url, URI) @@ -437,7 +437,7 @@ def test_generate_signed_url_lowercase_method(self): ) SIGNER = _Signer() - with mock.patch("google.cloud.storage.blob.generate_signed_url", new=SIGNER): + with mock.patch("google.cloud.storage.blob.generate_signed_url_v2", new=SIGNER): signed_url = blob.generate_signed_url(EXPIRATION, method="get") self.assertEqual(signed_url, URI) @@ -468,7 +468,7 @@ def test_generate_signed_url_non_ascii(self): ) SIGNER = _Signer() - with mock.patch("google.cloud.storage.blob.generate_signed_url", new=SIGNER): + with mock.patch("google.cloud.storage.blob.generate_signed_url_v2", new=SIGNER): signed_url = blob.generate_signed_url(EXPIRATION) self.assertEqual(signed_url, URI) @@ -498,7 +498,7 @@ def test_generate_signed_url_w_slash_in_name(self): ) SIGNER = _Signer() - with mock.patch("google.cloud.storage.blob.generate_signed_url", new=SIGNER): + with mock.patch("google.cloud.storage.blob.generate_signed_url_v2", new=SIGNER): signed_url = blob.generate_signed_url(EXPIRATION) self.assertEqual(signed_url, URI) @@ -528,7 +528,7 @@ def test_generate_signed_url_w_method_arg(self): ) SIGNER = _Signer() - with mock.patch("google.cloud.storage.blob.generate_signed_url", new=SIGNER): + with mock.patch("google.cloud.storage.blob.generate_signed_url_v2", new=SIGNER): signed_uri = blob.generate_signed_url(EXPIRATION, method="POST") self.assertEqual(signed_uri, URI) @@ -559,11 +559,11 @@ def test_generate_resumable_signed_url(self, mock_get_signed_query_params): Verify correct behavior of resumable upload URL generation """ from google.cloud.storage._signing import get_expiration_seconds - from google.cloud.storage._signing import generate_signed_url + from google.cloud.storage._signing import generate_signed_url_v2 expiry = get_expiration_seconds(datetime.timedelta(hours=1)) - signed_url = generate_signed_url( + signed_url = generate_signed_url_v2( _make_credentials(), "a-bucket", expiry, method="RESUMABLE" ) From cca63122fcd756010e660f4f3eedfbd2d58b259a Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 26 Feb 2019 17:22:36 -0500 Subject: [PATCH 02/37] Add '_signing.generate_signed_url_v4'. --- storage/google/cloud/storage/_signing.py | 171 +++++++++++++++++++++++ storage/tests/unit/test__signing.py | 102 +++++++++++++- 2 files changed, 267 insertions(+), 6 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index d908edc0fed6..b3ef6997af20 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -14,7 +14,9 @@ import base64 +import binascii import datetime +import hashlib import six @@ -231,3 +233,172 @@ def generate_signed_url_v2( resource=resource, querystring=six.moves.urllib.parse.urlencode(query_params), ) + + +SEVEN_DAYS = 7 * 24 * 60 * 60 # max expiration for V4 signed URLs. + + +def generate_signed_url_v4( + credentials, + resource, + expiration, + api_access_endpoint="", + method="GET", + content_md5=None, + content_type=None, + response_type=None, + response_disposition=None, + generation=None, + headers=None, +): + """Generate a V4 signed URL to provide query-string auth'n to a resource. + + .. note:: + + Assumes ``credentials`` implements the + :class:`google.auth.credentials.Signing` interface. Also assumes + ``credentials`` has a ``service_account_email`` property which + identifies the credentials. + + .. note:: + + If you are on Google Compute Engine, you can't generate a signed URL. + Follow `Issue 922`_ for updates on this. If you'd like to be able to + generate a signed URL from GCE, you can use a standard service account + from a JSON file rather than a GCE service account. + + See headers `reference`_ for more details on optional arguments. + + .. _Issue 922: https://github.com/GoogleCloudPlatform/\ + google-cloud-python/issues/922 + .. _reference: https://cloud.google.com/storage/docs/reference-headers + + :type credentials: :class:`google.auth.credentials.Signing` + :param credentials: Credentials object with an associated private key to + sign text. + + :type resource: str + :param resource: A pointer to a specific resource + (typically, ``/bucket-name/path/to/blob.txt``). + + :type expiration: :class:`int`, :class:`long`, :class:`datetime.datetime`, + :class:`datetime.timedelta` + :param expiration: When the signed URL should expire. Max value is + seven days. + + :type api_access_endpoint: str + :param api_access_endpoint: Optional URI base. Defaults to empty string. + + :type method: str + :param method: The HTTP verb that will be used when requesting the URL. + Defaults to ``'GET'``. If method is ``'RESUMABLE'`` then the + signature will additionally contain the `x-goog-resumable` + header, and the method changed to POST. See the signed URL + docs regarding this flow: + https://cloud.google.com/storage/docs/access-control/signed-urls + + + :type content_md5: str + :param content_md5: (Optional) The MD5 hash of the object referenced by + ``resource``. + + :type content_type: str + :param content_type: (Optional) The content type of the object referenced + by ``resource``. + + :type response_type: str + :param response_type: (Optional) Content type of responses to requests for + the signed URL. Used to over-ride the content type of + the underlying resource. + + :type response_disposition: str + :param response_disposition: (Optional) Content disposition of responses to + requests for the signed URL. + + :type generation: str + :param generation: (Optional) A value that indicates which generation of + the resource to fetch. + + :raises: :exc:`ValueError` when expiration is too large. + :raises: :exc:`TypeError` when expiration is not a valid type. + :raises: :exc:`AttributeError` if credentials is not an instance + of :class:`google.auth.credentials.Signing`. + + :rtype: str + :returns: A signed URL you can use to access the resource + until expiration. + """ + expiration = get_expiration_seconds(expiration) + + if expiration > SEVEN_DAYS: + raise ValueError( + "Max allowed expiration is seven days (%d seconds)".format(SEVEN_DAYS) + ) + + ensure_signed_credentials(credentials) + + now = NOW() + request_timestamp = now.strftime("%Y%m%dT%H%M%SZ") + datestamp = now.date().strftime("%Y%m%d") + + client_email = credentials.signer_email + credential_scope = "{}/auto/storage/goog4_request".format(datestamp) + credential = "{}/{}".format(client_email, credential_scope) + + if headers is None: + headers = {} + + ordered_headers = sorted(headers.items()) + canonical_headers = "\n".join( + ["{}:{}".format(key.lower(), val.strip()) for key, val in ordered_headers] + ) + signed_headers = ";".join([key.lower() for key, _ in ordered_headers]) + + query_params = { + "X-Goog-Algorithm": "GOOG4-RSA-SHA256", + "X-Goog-Credential": credential, + "X-Goog-Date": request_timestamp, + "X-Goog-Expires": expiration, + "X-Goog-SignedHeaders": signed_headers, + } + + if response_type is not None: + query_params["response-content-type"] = response_type + + if response_disposition is not None: + query_params["response-content-disposition"] = response_disposition + + if generation is not None: + query_params["generation"] = generation + + ordered_query_params = sorted(query_params.items()) + canonical_query_string = six.moves.urllib.parse.urlencode(ordered_query_params) + + canonical_elements = [ + method, + resource, + canonical_query_string, + canonical_headers, + signed_headers, + "UNSIGNED-PAYLOAD", + ] + canonical_request = "\n".join(canonical_elements) + + canonical_request_hash = hashlib.sha256( + canonical_request.encode("ascii") + ).hexdigest() + + string_elements = [ + "GOOG4-RSA-SHA256", + request_timestamp, + credential_scope, + canonical_request_hash, + ] + string_to_sign = "\n".join(string_elements) + + signature_bytes = credentials.sign_bytes(string_to_sign.encode("ascii")) + signature = binascii.hexlify(signature_bytes).decode("ascii") + + return "{}{}?{}&x-goog-signature={}".format( + api_access_endpoint, resource, canonical_query_string, signature + ) diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index 8ffd3d1483b4..598f6644a38e 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -13,6 +13,7 @@ # limitations under the License. import base64 +import binascii import calendar import datetime import time @@ -111,7 +112,7 @@ def _call_fut(credentials, expiration, string_to_sign): def test_it(self): sig_bytes = b"DEADBEEF" account_name = mock.sentinel.service_account_email - credentials = _make_credentials(signing=True, signer_email=account_name) + credentials = _make_credentials(signer_email=account_name) credentials.sign_bytes.return_value = sig_bytes expiration = 100 string_to_sign = "dummy_signature" @@ -138,9 +139,7 @@ def _generate_helper( ): endpoint = "http://api.example.com" resource = "/name/path" - credentials = _make_credentials( - signing=True, signer_email="service@example.com" - ) + credentials = _make_credentials(signer_email="service@example.com") credentials.sign_bytes.return_value = b"DEADBEEF" signed = base64.b64encode(credentials.sign_bytes.return_value) signed = signed.decode("ascii") @@ -207,10 +206,101 @@ def test_with_google_credentials(self): ) -def _make_credentials(signing=False, signer_email=None): +class Test_generate_signed_url_v4(unittest.TestCase): + @staticmethod + def _call_fut(*args, **kwargs): + from google.cloud.storage._signing import generate_signed_url_v4 + + return generate_signed_url_v4(*args, **kwargs) + + def _generate_helper( + self, + expiration=1000, + response_type=None, + response_disposition=None, + generation=None, + headers=None, + ): + now = datetime.datetime(2019, 2, 26, 19, 53, 27) + endpoint = "http://api.example.com" + resource = "/name/path" + signer_email = "service@example.com" + credentials = _make_credentials(signer_email=signer_email) + credentials.sign_bytes.return_value = b"DEADBEEF" + + with mock.patch("google.cloud.storage._signing.NOW", lambda: now): + url = self._call_fut( + credentials, + resource, + expiration, + api_access_endpoint=endpoint, + response_type=response_type, + response_disposition=response_disposition, + generation=generation, + headers=headers, + ) + + # Check the mock was called. + credentials.sign_bytes.assert_called_once() + + scheme, netloc, path, qs, frag = urllib_parse.urlsplit(url) + self.assertEqual(scheme, "http") + self.assertEqual(netloc, "api.example.com") + self.assertEqual(path, resource) + self.assertEqual(frag, "") + + # Check the URL parameters. + params = dict(urllib_parse.parse_qsl(qs)) + self.assertEqual(params["X-Goog-Algorithm"], "GOOG4-RSA-SHA256") + + now_date = now.date().strftime("%Y%m%d") + expected_cred = "{}/{}/auto/storage/goog4_request".format( + signer_email, now_date + ) + self.assertEqual(params["X-Goog-Credential"], expected_cred) + + now_stamp = now.strftime("%Y%m%dT%H%M%SZ") + self.assertEqual(params["X-Goog-Date"], now_stamp) + self.assertEqual(params["X-Goog-Expires"], str(expiration)) + + signed = binascii.hexlify(credentials.sign_bytes.return_value).decode("ascii") + self.assertEqual(params["x-goog-signature"], signed) + + if response_type is not None: + self.assertEqual(params["response-content-type"], response_type) + + if response_disposition is not None: + self.assertEqual( + params["response-content-disposition"], response_disposition + ) + + if generation is not None: + self.assertEqual(params["generation"], str(generation)) + + def test_w_expiration_too_long(self): + with self.assertRaises(ValueError): + self._generate_helper(expiration=datetime.timedelta(days=8)) + + def test_w_defaults(self): + self._generate_helper() + + def test_w_response_type(self): + self._generate_helper(response_type="application/octets") + + def test_w_response_disposition(self): + self._generate_helper(response_disposition="attachment") + + def test_w_generation(self): + self._generate_helper(generation=12345) + + def test_w_headers(self): + self._generate_helper(headers={"x-goog-foo": "bar"}) + + +def _make_credentials(signer_email=None): import google.auth.credentials - if signing: + if signer_email: credentials = mock.Mock(spec=google.auth.credentials.Signing) credentials.signer_email = signer_email return credentials From 6a6566b031ff9a241cfc6f4951b84282e6b87087 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 26 Feb 2019 17:47:38 -0500 Subject: [PATCH 03/37] Expose V4 signing via 'Blob.generate_signed_url'. --- storage/google/cloud/storage/blob.py | 45 ++++++-- storage/tests/unit/test_blob.py | 161 +++++++++++++-------------- 2 files changed, 110 insertions(+), 96 deletions(-) diff --git a/storage/google/cloud/storage/blob.py b/storage/google/cloud/storage/blob.py index 852a04fe7b94..455b4f902636 100644 --- a/storage/google/cloud/storage/blob.py +++ b/storage/google/cloud/storage/blob.py @@ -55,6 +55,7 @@ from google.cloud.storage._helpers import _PropertyMixin from google.cloud.storage._helpers import _scalar_property from google.cloud.storage._signing import generate_signed_url_v2 +from google.cloud.storage._signing import generate_signed_url_v4 from google.cloud.storage.acl import ACL from google.cloud.storage.acl import ObjectACL @@ -310,6 +311,7 @@ def generate_signed_url( response_type=None, client=None, credentials=None, + version="v2", ): """Generates a signed URL for this blob. @@ -371,6 +373,11 @@ def generate_signed_url( the URL. Defaults to the credentials stored on the client used. + :type version: str + :param version: (Optional) The version of signed credential to create. + Must be one of 'v2' | 'v4'. + + :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 of :class:`google.auth.credentials.Signing`. @@ -379,6 +386,9 @@ def generate_signed_url( :returns: A signed URL you can use to access the resource until expiration. """ + if version not in ("v2", "v4"): + raise ValueError("'version' must be either 'v2' or 'v4'") + resource = "/{bucket_name}/{quoted_name}".format( bucket_name=self.bucket.name, quoted_name=quote(self.name.encode("utf-8")) ) @@ -387,17 +397,30 @@ def generate_signed_url( client = self._require_client(client) credentials = client._credentials - return generate_signed_url_v2( - credentials, - resource=resource, - api_access_endpoint=_API_ACCESS_ENDPOINT, - expiration=expiration, - method=method.upper(), - content_type=content_type, - response_type=response_type, - response_disposition=response_disposition, - generation=generation, - ) + if version == "v2": + return generate_signed_url_v2( + credentials, + resource=resource, + api_access_endpoint=_API_ACCESS_ENDPOINT, + expiration=expiration, + method=method.upper(), + content_type=content_type, + response_type=response_type, + response_disposition=response_disposition, + generation=generation, + ) + else: + return generate_signed_url_v4( + credentials, + resource=resource, + api_access_endpoint=_API_ACCESS_ENDPOINT, + expiration=expiration, + method=method.upper(), + content_type=content_type, + response_type=response_type, + response_disposition=response_disposition, + generation=generation, + ) def exists(self, client=None): """Determines whether or not this blob exists. diff --git a/storage/tests/unit/test_blob.py b/storage/tests/unit/test_blob.py index 1a6e9148f27b..cd3a583d19f4 100644 --- a/storage/tests/unit/test_blob.py +++ b/storage/tests/unit/test_blob.py @@ -351,42 +351,48 @@ def test_public_url_with_non_ascii(self): expected_url = "https://storage.googleapis.com/name/winter%20%E2%98%83" self.assertEqual(blob.public_url, expected_url) - def _basic_generate_signed_url_helper(self, credentials=None): + def test_generate_signed_url_w_invalid_version(self): + BLOB_NAME = "blob-name" + EXPIRATION = "2014-10-16T20:34:37.000Z" + connection = _Connection() + client = _Client(connection) + bucket = _Bucket(client) + blob = self._make_one(BLOB_NAME, bucket=bucket) + with self.assertRaises(ValueError): + blob.generate_signed_url(EXPIRATION, version="nonesuch") + + def _basic_generate_signed_url_v2_helper(self, credentials=None): BLOB_NAME = "blob-name" EXPIRATION = "2014-10-16T20:34:37.000Z" connection = _Connection() client = _Client(connection) bucket = _Bucket(client) blob = self._make_one(BLOB_NAME, bucket=bucket) - URI = ( - "http://example.com/abucket/a-blob-name?Signature=DEADBEEF" - "&Expiration=2014-10-16T20:34:37.000Z" - ) - SIGNER = _Signer() - with mock.patch("google.cloud.storage.blob.generate_signed_url_v2", new=SIGNER): + with mock.patch("google.cloud.storage.blob.generate_signed_url_v2") as signer: signed_uri = blob.generate_signed_url(EXPIRATION, credentials=credentials) - self.assertEqual(signed_uri, URI) - PATH = "/name/%s" % (BLOB_NAME,) + self.assertEqual(signed_uri, signer.return_value) + if credentials is None: - EXPECTED_ARGS = (_Connection.credentials,) + expected_creds = _Connection.credentials else: - EXPECTED_ARGS = (credentials,) - EXPECTED_KWARGS = { + expected_creds = credentials + + expected_kwargs = { "api_access_endpoint": "https://storage.googleapis.com", "expiration": EXPIRATION, "method": "GET", - "resource": PATH, + "resource": "/name/{}".format(BLOB_NAME), "content_type": None, "response_type": None, "response_disposition": None, "generation": None, } - self.assertEqual(SIGNER._signed, [(EXPECTED_ARGS, EXPECTED_KWARGS)]) + signer.assert_called_once_with(expected_creds, **expected_kwargs) def test_generate_signed_url_w_default_method(self): - self._basic_generate_signed_url_helper() + self._basic_generate_signed_url_v2_helper() def test_generate_signed_url_w_content_type(self): BLOB_NAME = "blob-name" @@ -395,34 +401,28 @@ def test_generate_signed_url_w_content_type(self): client = _Client(connection) bucket = _Bucket(client) blob = self._make_one(BLOB_NAME, bucket=bucket) - URI = ( - "http://example.com/abucket/a-blob-name?Signature=DEADBEEF" - "&Expiration=2014-10-16T20:34:37.000Z" - ) - - SIGNER = _Signer() CONTENT_TYPE = "text/html" - with mock.patch("google.cloud.storage.blob.generate_signed_url_v2", new=SIGNER): + + with mock.patch("google.cloud.storage.blob.generate_signed_url_v2") as signer: signed_url = blob.generate_signed_url(EXPIRATION, content_type=CONTENT_TYPE) - self.assertEqual(signed_url, URI) - PATH = "/name/%s" % (BLOB_NAME,) - EXPECTED_ARGS = (_Connection.credentials,) - EXPECTED_KWARGS = { + self.assertEqual(signed_url, signer.return_value) + + expected_kwargs = { "api_access_endpoint": "https://storage.googleapis.com", "expiration": EXPIRATION, "method": "GET", - "resource": PATH, + "resource": "/name/{}".format(BLOB_NAME), "content_type": CONTENT_TYPE, "response_type": None, "response_disposition": None, "generation": None, } - self.assertEqual(SIGNER._signed, [(EXPECTED_ARGS, EXPECTED_KWARGS)]) + signer.assert_called_once_with(_Connection.credentials, **expected_kwargs) def test_generate_signed_url_w_credentials(self): credentials = object() - self._basic_generate_signed_url_helper(credentials=credentials) + self._basic_generate_signed_url_v2_helper(credentials=credentials) def test_generate_signed_url_lowercase_method(self): BLOB_NAME = "blob-name" @@ -431,29 +431,23 @@ def test_generate_signed_url_lowercase_method(self): client = _Client(connection) bucket = _Bucket(client) blob = self._make_one(BLOB_NAME, bucket=bucket) - URI = ( - u"http://example.com/abucket/a-blob-name?Signature=DEADBEEF" - u"&Expiration=2014-10-16T20:34:37.000Z" - ) - SIGNER = _Signer() - with mock.patch("google.cloud.storage.blob.generate_signed_url_v2", new=SIGNER): + with mock.patch("google.cloud.storage.blob.generate_signed_url_v2") as signer: signed_url = blob.generate_signed_url(EXPIRATION, method="get") - self.assertEqual(signed_url, URI) - PATH = "/name/%s" % (BLOB_NAME,) - EXPECTED_ARGS = (_Connection.credentials,) - EXPECTED_KWARGS = { + self.assertEqual(signed_url, signer.return_value) + + expected_kwargs = { "api_access_endpoint": "https://storage.googleapis.com", "expiration": EXPIRATION, "method": "GET", - "resource": PATH, + "resource": "/name/{}".format(BLOB_NAME), "content_type": None, "response_type": None, "response_disposition": None, "generation": None, } - self.assertEqual(SIGNER._signed, [(EXPECTED_ARGS, EXPECTED_KWARGS)]) + signer.assert_called_once_with(_Connection.credentials, **expected_kwargs) def test_generate_signed_url_non_ascii(self): BLOB_NAME = u"\u0410\u043a\u043a\u043e\u0440\u0434\u044b.txt" @@ -462,18 +456,13 @@ def test_generate_signed_url_non_ascii(self): client = _Client(connection) bucket = _Bucket(client) blob = self._make_one(BLOB_NAME, bucket=bucket) - URI = ( - u"http://example.com/abucket/a-blob-name?Signature=DEADBEEF" - u"&Expiration=2014-10-16T20:34:37.000Z" - ) - SIGNER = _Signer() - with mock.patch("google.cloud.storage.blob.generate_signed_url_v2", new=SIGNER): + with mock.patch("google.cloud.storage.blob.generate_signed_url_v2") as signer: signed_url = blob.generate_signed_url(EXPIRATION) - self.assertEqual(signed_url, URI) - EXPECTED_ARGS = (_Connection.credentials,) - EXPECTED_KWARGS = { + self.assertEqual(signed_url, signer.return_value) + + expected_kwargs = { "api_access_endpoint": "https://storage.googleapis.com", "expiration": EXPIRATION, "method": "GET", @@ -483,7 +472,7 @@ def test_generate_signed_url_non_ascii(self): "response_disposition": None, "generation": None, } - self.assertEqual(SIGNER._signed, [(EXPECTED_ARGS, EXPECTED_KWARGS)]) + signer.assert_called_once_with(_Connection.credentials, **expected_kwargs) def test_generate_signed_url_w_slash_in_name(self): BLOB_NAME = "parent/child" @@ -492,18 +481,13 @@ def test_generate_signed_url_w_slash_in_name(self): client = _Client(connection) bucket = _Bucket(client) blob = self._make_one(BLOB_NAME, bucket=bucket) - URI = ( - "http://example.com/abucket/a-blob-name?Signature=DEADBEEF" - "&Expiration=2014-10-16T20:34:37.000Z" - ) - SIGNER = _Signer() - with mock.patch("google.cloud.storage.blob.generate_signed_url_v2", new=SIGNER): + with mock.patch("google.cloud.storage.blob.generate_signed_url_v2") as signer: signed_url = blob.generate_signed_url(EXPIRATION) - self.assertEqual(signed_url, URI) - EXPECTED_ARGS = (_Connection.credentials,) - EXPECTED_KWARGS = { + self.assertEqual(signed_url, signer.return_value) + + expected_kwargs = { "api_access_endpoint": "https://storage.googleapis.com", "expiration": EXPIRATION, "method": "GET", @@ -513,7 +497,7 @@ def test_generate_signed_url_w_slash_in_name(self): "response_disposition": None, "generation": None, } - self.assertEqual(SIGNER._signed, [(EXPECTED_ARGS, EXPECTED_KWARGS)]) + signer.assert_called_once_with(_Connection.credentials, **expected_kwargs) def test_generate_signed_url_w_method_arg(self): BLOB_NAME = "blob-name" @@ -522,29 +506,48 @@ def test_generate_signed_url_w_method_arg(self): client = _Client(connection) bucket = _Bucket(client) blob = self._make_one(BLOB_NAME, bucket=bucket) - URI = ( - "http://example.com/abucket/a-blob-name?Signature=DEADBEEF" - "&Expiration=2014-10-16T20:34:37.000Z" - ) - SIGNER = _Signer() - with mock.patch("google.cloud.storage.blob.generate_signed_url_v2", new=SIGNER): + with mock.patch("google.cloud.storage.blob.generate_signed_url_v2") as signer: signed_uri = blob.generate_signed_url(EXPIRATION, method="POST") - self.assertEqual(signed_uri, URI) - PATH = "/name/%s" % (BLOB_NAME,) - EXPECTED_ARGS = (_Connection.credentials,) - EXPECTED_KWARGS = { + self.assertEqual(signed_uri, signer.return_value) + + expected_kwargs = { "api_access_endpoint": "https://storage.googleapis.com", "expiration": EXPIRATION, "method": "POST", - "resource": PATH, + "resource": "/name/{}".format(BLOB_NAME), "content_type": None, "response_type": None, "response_disposition": None, "generation": None, } - self.assertEqual(SIGNER._signed, [(EXPECTED_ARGS, EXPECTED_KWARGS)]) + signer.assert_called_once_with(_Connection.credentials, **expected_kwargs) + + def test_generate_signed_url_w_v4(self): + BLOB_NAME = "blob-name" + EXPIRATION = "2014-10-16T20:34:37.000Z" + connection = _Connection() + client = _Client(connection) + bucket = _Bucket(client) + blob = self._make_one(BLOB_NAME, bucket=bucket) + + with mock.patch("google.cloud.storage.blob.generate_signed_url_v4") as signer: + signed_uri = blob.generate_signed_url(EXPIRATION, version="v4") + + self.assertEqual(signed_uri, signer.return_value) + + expected_kwargs = { + "api_access_endpoint": "https://storage.googleapis.com", + "expiration": EXPIRATION, + "method": "GET", + "resource": "/name/{}".format(BLOB_NAME), + "content_type": None, + "response_type": None, + "response_disposition": None, + "generation": None, + } + signer.assert_called_once_with(_Connection.credentials, **expected_kwargs) @mock.patch( "google.cloud.storage._signing.get_signed_query_params", @@ -3304,18 +3307,6 @@ def delete_blob(self, blob_name, client=None, generation=None): self._deleted.append((blob_name, client, generation)) -class _Signer(object): - def __init__(self): - self._signed = [] - - def __call__(self, *args, **kwargs): - self._signed.append((args, kwargs)) - return ( - "http://example.com/abucket/a-blob-name?Signature=DEADBEEF" - "&Expiration=%s" % kwargs.get("expiration") - ) - - class _Client(object): def __init__(self, connection): self._base_connection = connection From 9a25e9a0e100e022cb0c05b548139c2745ee2b9c Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Mar 2019 13:10:33 -0400 Subject: [PATCH 04/37] Uppercase signature query param name. Addresses: https://github.com/googleapis/google-cloud-python/pull/7460/files#r262652253 --- storage/google/cloud/storage/_signing.py | 2 +- storage/tests/unit/test__signing.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index b3ef6997af20..dc05d8513279 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -399,6 +399,6 @@ def generate_signed_url_v4( signature_bytes = credentials.sign_bytes(string_to_sign.encode("ascii")) signature = binascii.hexlify(signature_bytes).decode("ascii") - return "{}{}?{}&x-goog-signature={}".format( + return "{}{}?{}&X-Goog-Signature={}".format( api_access_endpoint, resource, canonical_query_string, signature ) diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index 598f6644a38e..f156dbd98447 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -264,7 +264,7 @@ def _generate_helper( self.assertEqual(params["X-Goog-Expires"], str(expiration)) signed = binascii.hexlify(credentials.sign_bytes.return_value).decode("ascii") - self.assertEqual(params["x-goog-signature"], signed) + self.assertEqual(params["X-Goog-Signature"], signed) if response_type is not None: self.assertEqual(params["response-content-type"], response_type) From 7ea200a4764d601a562cabd52dc0447a5fcba552 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Mar 2019 15:01:40 -0400 Subject: [PATCH 05/37] Set headers from 'content_{type,md5}' params. Addresses: https://github.com/googleapis/google-cloud-python/pull/7460#discussion_r262653519 --- storage/google/cloud/storage/_signing.py | 6 ++++ storage/tests/unit/test__signing.py | 37 ++++++++++++++++++++---- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index dc05d8513279..5fc8f543f6d9 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -348,6 +348,12 @@ def generate_signed_url_v4( if headers is None: headers = {} + if content_type is not None: + headers["Content-Type"] = content_type + + if content_md5 is not None: + headers["Content-MD5"] = content_md5 + ordered_headers = sorted(headers.items()) canonical_headers = "\n".join( ["{}:{}".format(key.lower(), val.strip()) for key, val in ordered_headers] diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index f156dbd98447..80b0e161e587 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -216,13 +216,16 @@ def _call_fut(*args, **kwargs): def _generate_helper( self, expiration=1000, + api_access_endpoint="", + method="GET", + content_type=None, + content_md5=None, response_type=None, response_disposition=None, generation=None, headers=None, ): now = datetime.datetime(2019, 2, 26, 19, 53, 27) - endpoint = "http://api.example.com" resource = "/name/path" signer_email = "service@example.com" credentials = _make_credentials(signer_email=signer_email) @@ -233,7 +236,10 @@ def _generate_helper( credentials, resource, expiration, - api_access_endpoint=endpoint, + api_access_endpoint=api_access_endpoint, + method=method, + content_type=content_type, + content_md5=content_md5, response_type=response_type, response_disposition=response_disposition, generation=generation, @@ -244,8 +250,17 @@ def _generate_helper( credentials.sign_bytes.assert_called_once() scheme, netloc, path, qs, frag = urllib_parse.urlsplit(url) - self.assertEqual(scheme, "http") - self.assertEqual(netloc, "api.example.com") + + if api_access_endpoint is not None: + expected_scheme, expected_netloc, _, _, _ = urllib_parse.urlsplit( + api_access_endpoint + ) + self.assertEqual(scheme, expected_scheme) + self.assertEqual(netloc, expected_netloc) + else: + self.assertEqual(scheme, "") + self.assertEqual(netloc, "") + self.assertEqual(path, resource) self.assertEqual(frag, "") @@ -284,6 +299,18 @@ def test_w_expiration_too_long(self): def test_w_defaults(self): self._generate_helper() + def test_w_api_access_endpoint(self): + self._generate_helper(api_access_endpoint="http://api.example.com") + + def test_w_method(self): + self._generate_helper(method="PUT") + + def test_w_content_type(self): + self._generate_helper(content_type="text/plain") + + def test_w_content_md5(self): + self._generate_helper(content_md5="FACEDACE") + def test_w_response_type(self): self._generate_helper(response_type="application/octets") @@ -293,7 +320,7 @@ def test_w_response_disposition(self): def test_w_generation(self): self._generate_helper(generation=12345) - def test_w_headers(self): + def test_w_custom_headers(self): self._generate_helper(headers={"x-goog-foo": "bar"}) From 98b7e456c1c1c45176cecd4c9ed1b878ebfa351f Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Mar 2019 17:16:18 -0400 Subject: [PATCH 06/37] Pass user-supplied query parameters through for V4 signing. --- storage/google/cloud/storage/_signing.py | 40 +++++++++++++++++------- storage/tests/unit/test__signing.py | 30 +++++++++++------- 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index 5fc8f543f6d9..abb00cbe6698 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -250,6 +250,7 @@ def generate_signed_url_v4( response_disposition=None, generation=None, headers=None, + query_parameters=None, ): """Generate a V4 signed URL to provide query-string auth'n to a resource. @@ -319,6 +320,18 @@ def generate_signed_url_v4( :param generation: (Optional) A value that indicates which generation of the resource to fetch. + :type headers: dict + :param headers: + (Optional) Additional HTTP headers to be included as part of the + signed URLs. See: + https://cloud.google.com/storage/docs/xml-api/reference-headers + + :type query_parameters: dict + :param query_parameters: + (Optional) Additional query paramtersto be included as part of the + signed URLs. See: + https://cloud.google.com/storage/docs/xml-api/reference-headers#query + :raises: :exc:`ValueError` when expiration is too large. :raises: :exc:`TypeError` when expiration is not a valid type. :raises: :exc:`AttributeError` if credentials is not an instance @@ -360,25 +373,28 @@ def generate_signed_url_v4( ) signed_headers = ";".join([key.lower() for key, _ in ordered_headers]) - query_params = { - "X-Goog-Algorithm": "GOOG4-RSA-SHA256", - "X-Goog-Credential": credential, - "X-Goog-Date": request_timestamp, - "X-Goog-Expires": expiration, - "X-Goog-SignedHeaders": signed_headers, - } + if query_parameters is None: + query_parameters = {} + else: + query_parameters = {key: value or "" for key, value in query_parameters.items()} + + query_parameters["X-Goog-Algorithm"] = "GOOG4-RSA-SHA256" + query_parameters["X-Goog-Credential"] = credential + query_parameters["X-Goog-Date"] = request_timestamp + query_parameters["X-Goog-Expires"] = expiration + query_parameters["X-Goog-SignedHeaders"] = signed_headers if response_type is not None: - query_params["response-content-type"] = response_type + query_parameters["response-content-type"] = response_type if response_disposition is not None: - query_params["response-content-disposition"] = response_disposition + query_parameters["response-content-disposition"] = response_disposition if generation is not None: - query_params["generation"] = generation + query_parameters["generation"] = generation - ordered_query_params = sorted(query_params.items()) - canonical_query_string = six.moves.urllib.parse.urlencode(ordered_query_params) + ordered_query_parameters = sorted(query_parameters.items()) + canonical_query_string = six.moves.urllib.parse.urlencode(ordered_query_parameters) canonical_elements = [ method, diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index 80b0e161e587..6be537236669 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -224,6 +224,7 @@ def _generate_helper( response_disposition=None, generation=None, headers=None, + query_parameters=None, ): now = datetime.datetime(2019, 2, 26, 19, 53, 27) resource = "/name/path" @@ -244,6 +245,7 @@ def _generate_helper( response_disposition=response_disposition, generation=generation, headers=headers, + query_parameters=query_parameters, ) # Check the mock was called. @@ -251,21 +253,16 @@ def _generate_helper( scheme, netloc, path, qs, frag = urllib_parse.urlsplit(url) - if api_access_endpoint is not None: - expected_scheme, expected_netloc, _, _, _ = urllib_parse.urlsplit( - api_access_endpoint - ) - self.assertEqual(scheme, expected_scheme) - self.assertEqual(netloc, expected_netloc) - else: - self.assertEqual(scheme, "") - self.assertEqual(netloc, "") - + expected_scheme, expected_netloc, _, _, _ = urllib_parse.urlsplit( + api_access_endpoint + ) + self.assertEqual(scheme, expected_scheme) + self.assertEqual(netloc, expected_netloc) self.assertEqual(path, resource) self.assertEqual(frag, "") # Check the URL parameters. - params = dict(urllib_parse.parse_qsl(qs)) + params = dict(urllib_parse.parse_qsl(qs, keep_blank_values=True)) self.assertEqual(params["X-Goog-Algorithm"], "GOOG4-RSA-SHA256") now_date = now.date().strftime("%Y%m%d") @@ -281,6 +278,11 @@ def _generate_helper( signed = binascii.hexlify(credentials.sign_bytes.return_value).decode("ascii") self.assertEqual(params["X-Goog-Signature"], signed) + if query_parameters is not None: + for key, value in query_parameters.items(): + value = value.strip() if value else "" + self.assertEqual(params[key].lower(), value) + if response_type is not None: self.assertEqual(params["response-content-type"], response_type) @@ -323,6 +325,12 @@ def test_w_generation(self): def test_w_custom_headers(self): self._generate_helper(headers={"x-goog-foo": "bar"}) + def test_w_custom_query_parameters_w_string_value(self): + self._generate_helper(query_parameters={"delimiter": "/"}) + + def test_w_custom_query_parameters_w_none_value(self): + self._generate_helper(query_parameters={"acl": None}) + def _make_credentials(signer_email=None): import google.auth.credentials From 78d7de5211a21aec4d3574d3bab14181f10a1d78 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Mar 2019 19:47:54 -0400 Subject: [PATCH 07/37] Plumb 'headers'/'query_parameters' args through to 'Blob.generate_signed_url. Normalize remaining gaps between the V2 an V4 versions. --- storage/google/cloud/storage/_signing.py | 36 ++- storage/google/cloud/storage/blob.py | 72 +++--- storage/tests/unit/test__signing.py | 126 ++++++++--- storage/tests/unit/test_blob.py | 265 ++++++++++------------- 4 files changed, 296 insertions(+), 203 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index abb00cbe6698..49bca6dca8a0 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -122,6 +122,8 @@ def generate_signed_url_v2( response_type=None, response_disposition=None, generation=None, + headers=None, + query_parameters=None, ): """Generate a V2 signed URL to provide query-string auth'n to a resource. @@ -190,6 +192,18 @@ def generate_signed_url_v2( :param generation: (Optional) A value that indicates which generation of the resource to fetch. + :type headers: dict + :param headers: + (Optional) Additional HTTP headers to be included as part of the + signed URLs. See: + https://cloud.google.com/storage/docs/xml-api/reference-headers + + :type query_parameters: dict + :param query_parameters: + (Optional) Additional query paramtersto be included as part of the + signed URLs. See: + https://cloud.google.com/storage/docs/xml-api/reference-headers#query + :raises: :exc:`TypeError` when expiration is not a valid type. :raises: :exc:`AttributeError` if credentials is not an instance of :class:`google.auth.credentials.Signing`. @@ -206,6 +220,16 @@ def generate_signed_url_v2( else: canonicalized_resource = "{0}".format(resource) + if query_parameters is not None: + normalized_qp = list(sorted( + (key.lower(), value and value.strip() or "") + for key, value in query_parameters.items() + )) + encoded_qp = six.moves.urllib.parse.urlencode(normalized_qp) + canonicalized_resource = "{}?{}".format(canonicalized_resource, encoded_qp) + else: + normalized_qp = () + # Generate the string to sign. string_to_sign = "\n".join( [ @@ -218,20 +242,22 @@ def generate_signed_url_v2( ) # Set the right query parameters. - query_params = get_signed_query_params(credentials, expiration, string_to_sign) + signed_query_params = get_signed_query_params(credentials, expiration, string_to_sign) if response_type is not None: - query_params["response-content-type"] = response_type + signed_query_params["response-content-type"] = response_type if response_disposition is not None: - query_params["response-content-disposition"] = response_disposition + signed_query_params["response-content-disposition"] = response_disposition if generation is not None: - query_params["generation"] = generation + signed_query_params["generation"] = generation + + signed_query_params.update(normalized_qp) # Return the built URL. return "{endpoint}{resource}?{querystring}".format( endpoint=api_access_endpoint, resource=resource, - querystring=six.moves.urllib.parse.urlencode(query_params), + querystring=six.moves.urllib.parse.urlencode(signed_query_params), ) diff --git a/storage/google/cloud/storage/blob.py b/storage/google/cloud/storage/blob.py index 455b4f902636..6034f6e1c571 100644 --- a/storage/google/cloud/storage/blob.py +++ b/storage/google/cloud/storage/blob.py @@ -304,11 +304,15 @@ def public_url(self): def generate_signed_url( self, expiration, + api_access_endpoint=_API_ACCESS_ENDPOINT, method="GET", + content_md5=None, content_type=None, - generation=None, response_disposition=None, response_type=None, + generation=None, + headers=None, + query_parameters=None, client=None, credentials=None, version="v2", @@ -337,17 +341,20 @@ def generate_signed_url( :type expiration: int, long, datetime.datetime, datetime.timedelta :param expiration: When the signed URL should expire. + :type api_access_endpoint: str + :param api_access_endpoint: Optional URI base. Defaults to empty string. + :type method: str :param method: The HTTP verb that will be used when requesting the URL. + :type content_md5: str + :param content_md5: (Optional) The MD5 hash of the object referenced by + ``resource``. + :type content_type: str :param content_type: (Optional) The content type of the object referenced by ``resource``. - :type generation: str - :param generation: (Optional) A value that indicates which generation - of the resource to fetch. - :type response_disposition: str :param response_disposition: (Optional) Content disposition of responses to requests for the signed URL. @@ -361,6 +368,22 @@ def generate_signed_url( for the signed URL. Used to over-ride the content type of the underlying blob/object. + :type generation: str + :param generation: (Optional) A value that indicates which generation + of the resource to fetch. + + :type headers: dict + :param headers: + (Optional) Additional HTTP headers to be included as part of the + signed URLs. See: + https://cloud.google.com/storage/docs/xml-api/reference-headers + + :type query_parameters: dict + :param query_parameters: + (Optional) Additional query paramtersto be included as part of the + signed URLs. See: + https://cloud.google.com/storage/docs/xml-api/reference-headers#query + :type client: :class:`~google.cloud.storage.client.Client` or ``NoneType`` :param client: (Optional) The client to use. If not passed, falls back @@ -398,29 +421,24 @@ def generate_signed_url( credentials = client._credentials if version == "v2": - return generate_signed_url_v2( - credentials, - resource=resource, - api_access_endpoint=_API_ACCESS_ENDPOINT, - expiration=expiration, - method=method.upper(), - content_type=content_type, - response_type=response_type, - response_disposition=response_disposition, - generation=generation, - ) + helper = generate_signed_url_v2 else: - return generate_signed_url_v4( - credentials, - resource=resource, - api_access_endpoint=_API_ACCESS_ENDPOINT, - expiration=expiration, - method=method.upper(), - content_type=content_type, - response_type=response_type, - response_disposition=response_disposition, - generation=generation, - ) + helper = generate_signed_url_v4 + + return helper( + credentials, + resource=resource, + expiration=expiration, + api_access_endpoint=api_access_endpoint, + method=method.upper(), + content_md5=content_md5, + content_type=content_type, + response_type=response_type, + response_disposition=response_disposition, + generation=generation, + headers=headers, + query_parameters=query_parameters, + ) def exists(self, client=None): """Determines whether or not this blob exists. diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index 6be537236669..9d734fd666c4 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -135,8 +135,19 @@ def _call_fut(*args, **kwargs): return generate_signed_url_v2(*args, **kwargs) def _generate_helper( - self, response_type=None, response_disposition=None, generation=None + self, + api_access_endpoint="", + method="GET", + content_md5=None, + content_type=None, + response_type=None, + response_disposition=None, + generation=None, + headers=None, + query_parameters=None, ): + from six.moves.urllib.parse import urlencode + endpoint = "http://api.example.com" resource = "/name/path" credentials = _make_credentials(signer_email="service@example.com") @@ -149,49 +160,112 @@ def _generate_helper( credentials, resource, expiration, - api_access_endpoint=endpoint, + api_access_endpoint=api_access_endpoint, + method=method, + content_md5=content_md5, + content_type=content_type, response_type=response_type, response_disposition=response_disposition, generation=generation, + headers=headers, + query_parameters=query_parameters, ) # Check the mock was called. - string_to_sign = "\n".join(["GET", "", "", str(expiration), resource]) + method = method.upper() + + elements = [] + if method == "RESUMABLE": + elements.append("POST") + expected_resource = "x-goog-resumable:start\n{}".format(resource) + else: + elements.append(method) + expected_resource = resource + + if query_parameters is not None: + normalized_qp = { + key.lower(): value and value.strip() or "" + for key, value in query_parameters.items() + } + expected_qp = urlencode(sorted(normalized_qp.items())) + expected_resource = "{}?{}".format(resource, expected_qp) + + elements.append(content_md5 or "") + elements.append(content_type or "") + elements.append(str(expiration)) + elements.append(expected_resource) + + string_to_sign = "\n".join(elements) + credentials.sign_bytes.assert_called_once_with(string_to_sign) scheme, netloc, path, qs, frag = urllib_parse.urlsplit(url) - self.assertEqual(scheme, "http") - self.assertEqual(netloc, "api.example.com") + expected_scheme, expected_netloc, _, _, _ = urllib_parse.urlsplit( + api_access_endpoint + ) + self.assertEqual(scheme, expected_scheme) + self.assertEqual(netloc, expected_netloc) self.assertEqual(path, resource) self.assertEqual(frag, "") # Check the URL parameters. - params = urllib_parse.parse_qs(qs) - expected_params = { - "GoogleAccessId": [credentials.signer_email], - "Expires": [str(expiration)], - "Signature": [signed], - } + params = dict(urllib_parse.parse_qsl(qs, keep_blank_values=True)) + + self.assertEqual(params["GoogleAccessId"], credentials.signer_email) + self.assertEqual(params["Expires"], str(expiration)) + self.assertEqual(params["Signature"], signed) + if response_type is not None: - expected_params["response-content-type"] = [response_type] + self.assertEqual(params["response-content-type"], response_type) + if response_disposition is not None: - expected_params["response-content-disposition"] = [response_disposition] + self.assertEqual( + params["response-content-disposition"], response_disposition + ) + if generation is not None: - expected_params["generation"] = [generation] - self.assertEqual(params, expected_params) + self.assertEqual(params["generation"], str(generation)) + + if query_parameters is not None: + for key, value in query_parameters.items(): + value = value.strip() if value else "" + self.assertEqual(params[key].lower(), value) def test_w_expiration_int(self): self._generate_helper() - def test_w_custom_fields(self): + def test_w_endpoint(self): + api_access_endpoint = "https://api.example.com" + self._generate_helper(api_access_endpoint=api_access_endpoint) + + def test_w_method(self): + method = "POST" + self._generate_helper(method=method) + + def test_w_method_resumable(self): + method = "RESUMABLE" + self._generate_helper(method=method) + + def test_w_response_type(self): response_type = "text/plain" + self._generate_helper(response_type=response_type) + + def test_w_response_disposition(self): response_disposition = "attachment; filename=blob.png" + self._generate_helper(response_disposition=response_disposition) + + def test_w_generation(self): generation = "123" - self._generate_helper( - response_type=response_type, - response_disposition=response_disposition, - generation=generation, - ) + self._generate_helper(generation=generation) + + def test_w_custom_headers(self): + self._generate_helper(headers={"x-goog-foo": "bar"}) + + def test_w_custom_query_parameters_w_string_value(self): + self._generate_helper(query_parameters={"delimiter": "/"}) + + def test_w_custom_query_parameters_w_none_value(self): + self._generate_helper(query_parameters={"acl": None}) def test_with_google_credentials(self): resource = "/name/path" @@ -278,11 +352,6 @@ def _generate_helper( signed = binascii.hexlify(credentials.sign_bytes.return_value).decode("ascii") self.assertEqual(params["X-Goog-Signature"], signed) - if query_parameters is not None: - for key, value in query_parameters.items(): - value = value.strip() if value else "" - self.assertEqual(params[key].lower(), value) - if response_type is not None: self.assertEqual(params["response-content-type"], response_type) @@ -294,6 +363,11 @@ def _generate_helper( if generation is not None: self.assertEqual(params["generation"], str(generation)) + if query_parameters is not None: + for key, value in query_parameters.items(): + value = value.strip() if value else "" + self.assertEqual(params[key].lower(), value) + def test_w_expiration_too_long(self): with self.assertRaises(ValueError): self._generate_helper(expiration=datetime.timedelta(days=8)) diff --git a/storage/tests/unit/test_blob.py b/storage/tests/unit/test_blob.py index cd3a583d19f4..52586f5bad64 100644 --- a/storage/tests/unit/test_blob.py +++ b/storage/tests/unit/test_blob.py @@ -361,16 +361,47 @@ def test_generate_signed_url_w_invalid_version(self): with self.assertRaises(ValueError): blob.generate_signed_url(EXPIRATION, version="nonesuch") - def _basic_generate_signed_url_v2_helper(self, credentials=None): - BLOB_NAME = "blob-name" + def _generate_signed_url_helper( + self, + version, + blob_name="blob-name", + api_access_endpoint=None, + method="GET", + content_md5=None, + content_type=None, + response_type=None, + response_disposition=None, + generation=None, + headers=None, + query_parameters=None, + credentials=None, + ): + from six.moves.urllib import parse + from google.cloud.storage.blob import _API_ACCESS_ENDPOINT + + api_access_endpoint = api_access_endpoint or _API_ACCESS_ENDPOINT EXPIRATION = "2014-10-16T20:34:37.000Z" connection = _Connection() client = _Client(connection) bucket = _Bucket(client) - blob = self._make_one(BLOB_NAME, bucket=bucket) - - with mock.patch("google.cloud.storage.blob.generate_signed_url_v2") as signer: - signed_uri = blob.generate_signed_url(EXPIRATION, credentials=credentials) + blob = self._make_one(blob_name, bucket=bucket) + to_patch = "google.cloud.storage.blob.generate_signed_url_{}".format(version) + + with mock.patch(to_patch) as signer: + signed_uri = blob.generate_signed_url( + EXPIRATION, + api_access_endpoint=api_access_endpoint, + method=method, + credentials=credentials, + content_md5=content_md5, + content_type=content_type, + response_type=response_type, + response_disposition=response_disposition, + generation=generation, + headers=headers, + query_parameters=query_parameters, + version=version, + ) self.assertEqual(signed_uri, signer.return_value) @@ -379,175 +410,119 @@ def _basic_generate_signed_url_v2_helper(self, credentials=None): else: expected_creds = credentials + expected_resource = parse.quote("/name/{}".format(blob_name)) expected_kwargs = { - "api_access_endpoint": "https://storage.googleapis.com", + "resource": expected_resource, "expiration": EXPIRATION, - "method": "GET", - "resource": "/name/{}".format(BLOB_NAME), - "content_type": None, - "response_type": None, - "response_disposition": None, - "generation": None, + "api_access_endpoint": api_access_endpoint, + "method": method.upper(), + "content_md5": content_md5, + "content_type": content_type, + "response_type": response_type, + "response_disposition": response_disposition, + "generation": generation, + "headers": headers, + "query_parameters": query_parameters, } signer.assert_called_once_with(expected_creds, **expected_kwargs) - def test_generate_signed_url_w_default_method(self): - self._basic_generate_signed_url_v2_helper() - - def test_generate_signed_url_w_content_type(self): - BLOB_NAME = "blob-name" - EXPIRATION = "2014-10-16T20:34:37.000Z" - connection = _Connection() - client = _Client(connection) - bucket = _Bucket(client) - blob = self._make_one(BLOB_NAME, bucket=bucket) - CONTENT_TYPE = "text/html" + def _generate_signed_url_v2_helper(self, **kw): + version = "v2" + self._generate_signed_url_helper(version, **kw) - with mock.patch("google.cloud.storage.blob.generate_signed_url_v2") as signer: - signed_url = blob.generate_signed_url(EXPIRATION, content_type=CONTENT_TYPE) + def test_generate_signed_url_v2_w_defaults(self): + self._generate_signed_url_v2_helper() - self.assertEqual(signed_url, signer.return_value) + def test_generate_signed_url_v2_w_non_ascii_name(self): + BLOB_NAME = u"\u0410\u043a\u043a\u043e\u0440\u0434\u044b.txt" + self._generate_signed_url_v2_helper(blob_name=BLOB_NAME) - expected_kwargs = { - "api_access_endpoint": "https://storage.googleapis.com", - "expiration": EXPIRATION, - "method": "GET", - "resource": "/name/{}".format(BLOB_NAME), - "content_type": CONTENT_TYPE, - "response_type": None, - "response_disposition": None, - "generation": None, - } - signer.assert_called_once_with(_Connection.credentials, **expected_kwargs) + def test_generate_signed_url_v2_w_slash_in_name(self): + BLOB_NAME = "parent/child" + self._generate_signed_url_v2_helper(blob_name=BLOB_NAME) - def test_generate_signed_url_w_credentials(self): + def test_generate_signed_url_v2_w_endpoint(self): credentials = object() - self._basic_generate_signed_url_v2_helper(credentials=credentials) + self._generate_signed_url_v2_helper( + api_access_endpoint="https://api.example.com/v1" + ) - def test_generate_signed_url_lowercase_method(self): - BLOB_NAME = "blob-name" - EXPIRATION = "2014-10-16T20:34:37.000Z" - connection = _Connection() - client = _Client(connection) - bucket = _Bucket(client) - blob = self._make_one(BLOB_NAME, bucket=bucket) + def test_generate_signed_url_v2_w_method(self): + self._generate_signed_url_v2_helper(method="POST") - with mock.patch("google.cloud.storage.blob.generate_signed_url_v2") as signer: - signed_url = blob.generate_signed_url(EXPIRATION, method="get") + def test_generate_signed_url_v2_w_lowercase_method(self): + self._generate_signed_url_v2_helper(method="get") - self.assertEqual(signed_url, signer.return_value) + def test_generate_signed_url_v2_w_content_md5(self): + self._generate_signed_url_v2_helper(content_md5="FACEDACE") - expected_kwargs = { - "api_access_endpoint": "https://storage.googleapis.com", - "expiration": EXPIRATION, - "method": "GET", - "resource": "/name/{}".format(BLOB_NAME), - "content_type": None, - "response_type": None, - "response_disposition": None, - "generation": None, - } - signer.assert_called_once_with(_Connection.credentials, **expected_kwargs) + def test_generate_signed_url_v2_w_content_type(self): + self._generate_signed_url_v2_helper(content_type="text.html") - def test_generate_signed_url_non_ascii(self): - BLOB_NAME = u"\u0410\u043a\u043a\u043e\u0440\u0434\u044b.txt" - EXPIRATION = "2014-10-16T20:34:37.000Z" - connection = _Connection() - client = _Client(connection) - bucket = _Bucket(client) - blob = self._make_one(BLOB_NAME, bucket=bucket) + def test_generate_signed_url_v2_w_response_type(self): + self._generate_signed_url_v2_helper(response_type="text.html") - with mock.patch("google.cloud.storage.blob.generate_signed_url_v2") as signer: - signed_url = blob.generate_signed_url(EXPIRATION) + def test_generate_signed_url_v2_w_response_disposition(self): + self._generate_signed_url_v2_helper(response_disposition="inline") - self.assertEqual(signed_url, signer.return_value) + def test_generate_signed_url_v2_w_generation(self): + self._generate_signed_url_v2_helper(generation=12345) - expected_kwargs = { - "api_access_endpoint": "https://storage.googleapis.com", - "expiration": EXPIRATION, - "method": "GET", - "resource": "/name/%D0%90%D0%BA%D0%BA%D0%BE%D1%80%D0%B4%D1%8B.txt", - "content_type": None, - "response_type": None, - "response_disposition": None, - "generation": None, - } - signer.assert_called_once_with(_Connection.credentials, **expected_kwargs) + def test_generate_signed_url_v2_w_headers(self): + self._generate_signed_url_v2_helper(headers={"x-goog-foo": "bar"}) - def test_generate_signed_url_w_slash_in_name(self): - BLOB_NAME = "parent/child" - EXPIRATION = "2014-10-16T20:34:37.000Z" - connection = _Connection() - client = _Client(connection) - bucket = _Bucket(client) - blob = self._make_one(BLOB_NAME, bucket=bucket) + def test_generate_signed_url_v2_w_credentials(self): + credentials = object() + self._generate_signed_url_v2_helper(credentials=credentials) - with mock.patch("google.cloud.storage.blob.generate_signed_url_v2") as signer: - signed_url = blob.generate_signed_url(EXPIRATION) + def _generate_signed_url_v4_helper(self, **kw): + version = "v4" + self._generate_signed_url_helper(version, **kw) - self.assertEqual(signed_url, signer.return_value) + def test_generate_signed_url_v4_w_defaults(self): + self._generate_signed_url_v4_helper() - expected_kwargs = { - "api_access_endpoint": "https://storage.googleapis.com", - "expiration": EXPIRATION, - "method": "GET", - "resource": "/name/parent/child", - "content_type": None, - "response_type": None, - "response_disposition": None, - "generation": None, - } - signer.assert_called_once_with(_Connection.credentials, **expected_kwargs) + def test_generate_signed_url_v4_w_non_ascii_name(self): + BLOB_NAME = u"\u0410\u043a\u043a\u043e\u0440\u0434\u044b.txt" + self._generate_signed_url_v4_helper(blob_name=BLOB_NAME) - def test_generate_signed_url_w_method_arg(self): - BLOB_NAME = "blob-name" - EXPIRATION = "2014-10-16T20:34:37.000Z" - connection = _Connection() - client = _Client(connection) - bucket = _Bucket(client) - blob = self._make_one(BLOB_NAME, bucket=bucket) + def test_generate_signed_url_v4_w_slash_in_name(self): + BLOB_NAME = "parent/child" + self._generate_signed_url_v4_helper(blob_name=BLOB_NAME) + + def test_generate_signed_url_v4_w_endpoint(self): + credentials = object() + self._generate_signed_url_v4_helper( + api_access_endpoint="https://api.example.com/v1" + ) - with mock.patch("google.cloud.storage.blob.generate_signed_url_v2") as signer: - signed_uri = blob.generate_signed_url(EXPIRATION, method="POST") + def test_generate_signed_url_v4_w_method(self): + self._generate_signed_url_v4_helper(method="POST") - self.assertEqual(signed_uri, signer.return_value) + def test_generate_signed_url_v4_w_lowercase_method(self): + self._generate_signed_url_v4_helper(method="get") - expected_kwargs = { - "api_access_endpoint": "https://storage.googleapis.com", - "expiration": EXPIRATION, - "method": "POST", - "resource": "/name/{}".format(BLOB_NAME), - "content_type": None, - "response_type": None, - "response_disposition": None, - "generation": None, - } - signer.assert_called_once_with(_Connection.credentials, **expected_kwargs) + def test_generate_signed_url_v4_w_content_md5(self): + self._generate_signed_url_v4_helper(content_md5="FACEDACE") - def test_generate_signed_url_w_v4(self): - BLOB_NAME = "blob-name" - EXPIRATION = "2014-10-16T20:34:37.000Z" - connection = _Connection() - client = _Client(connection) - bucket = _Bucket(client) - blob = self._make_one(BLOB_NAME, bucket=bucket) + def test_generate_signed_url_v4_w_content_type(self): + self._generate_signed_url_v4_helper(content_type="text.html") - with mock.patch("google.cloud.storage.blob.generate_signed_url_v4") as signer: - signed_uri = blob.generate_signed_url(EXPIRATION, version="v4") + def test_generate_signed_url_v4_w_response_type(self): + self._generate_signed_url_v4_helper(response_type="text.html") - self.assertEqual(signed_uri, signer.return_value) + def test_generate_signed_url_v4_w_response_disposition(self): + self._generate_signed_url_v4_helper(response_disposition="inline") - expected_kwargs = { - "api_access_endpoint": "https://storage.googleapis.com", - "expiration": EXPIRATION, - "method": "GET", - "resource": "/name/{}".format(BLOB_NAME), - "content_type": None, - "response_type": None, - "response_disposition": None, - "generation": None, - } - signer.assert_called_once_with(_Connection.credentials, **expected_kwargs) + def test_generate_signed_url_v4_w_generation(self): + self._generate_signed_url_v4_helper(generation=12345) + + def test_generate_signed_url_v4_w_headers(self): + self._generate_signed_url_v4_helper(headers={"x-goog-foo": "bar"}) + + def test_generate_signed_url_v4_w_credentials(self): + credentials = object() + self._generate_signed_url_v4_helper(credentials=credentials) @mock.patch( "google.cloud.storage._signing.get_signed_query_params", From 188d0a3f50eeb56f4d756debe8ee92ccb2af7062 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Mar 2019 22:01:29 -0400 Subject: [PATCH 08/37] Fix non-ascii blob name tests under Python 2. --- storage/tests/unit/test_blob.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/storage/tests/unit/test_blob.py b/storage/tests/unit/test_blob.py index 52586f5bad64..bf60d8efc64c 100644 --- a/storage/tests/unit/test_blob.py +++ b/storage/tests/unit/test_blob.py @@ -410,7 +410,8 @@ def _generate_signed_url_helper( else: expected_creds = credentials - expected_resource = parse.quote("/name/{}".format(blob_name)) + encoded_name = blob_name.encode("utf-8") + expected_resource = "/name/{}".format(parse.quote(encoded_name)) expected_kwargs = { "resource": expected_resource, "expiration": EXPIRATION, From 31dade92b62d8b4d0feb79829af63823077f3b3b Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Mar 2019 22:02:57 -0400 Subject: [PATCH 09/37] Blacken. --- storage/google/cloud/storage/_signing.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index 49bca6dca8a0..eacb7c95a43d 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -221,10 +221,12 @@ def generate_signed_url_v2( canonicalized_resource = "{0}".format(resource) if query_parameters is not None: - normalized_qp = list(sorted( - (key.lower(), value and value.strip() or "") - for key, value in query_parameters.items() - )) + normalized_qp = list( + sorted( + (key.lower(), value and value.strip() or "") + for key, value in query_parameters.items() + ) + ) encoded_qp = six.moves.urllib.parse.urlencode(normalized_qp) canonicalized_resource = "{}?{}".format(canonicalized_resource, encoded_qp) else: @@ -242,7 +244,9 @@ def generate_signed_url_v2( ) # Set the right query parameters. - signed_query_params = get_signed_query_params(credentials, expiration, string_to_sign) + signed_query_params = get_signed_query_params( + credentials, expiration, string_to_sign + ) if response_type is not None: signed_query_params["response-content-type"] = response_type From 5c114d69782a951d76a1a3a83a5f2703c5100412 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 12 Mar 2019 11:17:02 -0400 Subject: [PATCH 10/37] Lint. --- storage/tests/unit/test__signing.py | 1 - storage/tests/unit/test_blob.py | 2 -- 2 files changed, 3 deletions(-) diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index 9d734fd666c4..3c570e5b4e63 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -148,7 +148,6 @@ def _generate_helper( ): from six.moves.urllib.parse import urlencode - endpoint = "http://api.example.com" resource = "/name/path" credentials = _make_credentials(signer_email="service@example.com") credentials.sign_bytes.return_value = b"DEADBEEF" diff --git a/storage/tests/unit/test_blob.py b/storage/tests/unit/test_blob.py index bf60d8efc64c..75bf83c0fe00 100644 --- a/storage/tests/unit/test_blob.py +++ b/storage/tests/unit/test_blob.py @@ -443,7 +443,6 @@ def test_generate_signed_url_v2_w_slash_in_name(self): self._generate_signed_url_v2_helper(blob_name=BLOB_NAME) def test_generate_signed_url_v2_w_endpoint(self): - credentials = object() self._generate_signed_url_v2_helper( api_access_endpoint="https://api.example.com/v1" ) @@ -492,7 +491,6 @@ def test_generate_signed_url_v4_w_slash_in_name(self): self._generate_signed_url_v4_helper(blob_name=BLOB_NAME) def test_generate_signed_url_v4_w_endpoint(self): - credentials = object() self._generate_signed_url_v4_helper( api_access_endpoint="https://api.example.com/v1" ) From 01e2cd96e015e0b4d31f4bee2deddcb181ded0ba Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 12 Mar 2019 16:17:30 -0400 Subject: [PATCH 11/37] Document semantics of 'headers' passed when generating signed URLs. --- storage/google/cloud/storage/_signing.py | 4 ++++ storage/google/cloud/storage/blob.py | 2 ++ 2 files changed, 6 insertions(+) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index eacb7c95a43d..5968006d220a 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -197,6 +197,8 @@ def generate_signed_url_v2( (Optional) Additional HTTP headers to be included as part of the signed URLs. See: https://cloud.google.com/storage/docs/xml-api/reference-headers + Requests using the signed URL *must* pass the specified header + (name and value) with each request for the URL. :type query_parameters: dict :param query_parameters: @@ -355,6 +357,8 @@ def generate_signed_url_v4( (Optional) Additional HTTP headers to be included as part of the signed URLs. See: https://cloud.google.com/storage/docs/xml-api/reference-headers + Requests using the signed URL *must* pass the specified header + (name and value) with each request for the URL. :type query_parameters: dict :param query_parameters: diff --git a/storage/google/cloud/storage/blob.py b/storage/google/cloud/storage/blob.py index 6034f6e1c571..5e0c8c358ecc 100644 --- a/storage/google/cloud/storage/blob.py +++ b/storage/google/cloud/storage/blob.py @@ -377,6 +377,8 @@ def generate_signed_url( (Optional) Additional HTTP headers to be included as part of the signed URLs. See: https://cloud.google.com/storage/docs/xml-api/reference-headers + Requests using the signed URL *must* pass the specified header + (name and value) with each request for the URL. :type query_parameters: dict :param query_parameters: From 9113cdd374af611d634c90a79c9920a8e32d94a9 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 12 Mar 2019 16:23:04 -0400 Subject: [PATCH 12/37] Don't use subresource names in query string tests. --- storage/tests/unit/test__signing.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index 3c570e5b4e63..f8f0a842d013 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -261,10 +261,10 @@ def test_w_custom_headers(self): self._generate_helper(headers={"x-goog-foo": "bar"}) def test_w_custom_query_parameters_w_string_value(self): - self._generate_helper(query_parameters={"delimiter": "/"}) + self._generate_helper(query_parameters={"bar": "/"}) def test_w_custom_query_parameters_w_none_value(self): - self._generate_helper(query_parameters={"acl": None}) + self._generate_helper(query_parameters={"qux": None}) def test_with_google_credentials(self): resource = "/name/path" @@ -399,10 +399,10 @@ def test_w_custom_headers(self): self._generate_helper(headers={"x-goog-foo": "bar"}) def test_w_custom_query_parameters_w_string_value(self): - self._generate_helper(query_parameters={"delimiter": "/"}) + self._generate_helper(query_parameters={"bar": "/"}) def test_w_custom_query_parameters_w_none_value(self): - self._generate_helper(query_parameters={"acl": None}) + self._generate_helper(query_parameters={"qux": None}) def _make_credentials(signer_email=None): From ba065a127f984152018ee2136558f87bbb64263d Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Thu, 14 Mar 2019 15:25:27 -0400 Subject: [PATCH 13/37] Fix header/qp handling for V2 signing. --- storage/google/cloud/storage/_signing.py | 129 ++++++++++++++++++----- storage/tests/unit/test__signing.py | 93 ++++++++++++++-- 2 files changed, 185 insertions(+), 37 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index 5968006d220a..926e966b20ed 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -15,6 +15,7 @@ import base64 import binascii +import collections import datetime import hashlib @@ -111,6 +112,99 @@ def get_expiration_seconds(expiration): return expiration +def get_canonical_headers(headers): + """Canonicalize headers for signing. + + See: + https://cloud.google.com/storage/docs/access-control/signed-urls#about-canonical-extension-headers + + :type headers: Union[dict|List(Tuple(str,str))] + :param headers: + (Optional) Additional HTTP headers to be included as part of the + signed URLs. See: + https://cloud.google.com/storage/docs/xml-api/reference-headers + Requests using the signed URL *must* pass the specified header + (name and value) with each request for the URL. + + :rtype: str + :returns: List of headers, normalized / sortted per the URL refernced above. + """ + if headers is None: + headers = [] + elif isinstance(headers, dict): + headers = list(headers.items()) + + if not headers: + return [] + + normalized = collections.defaultdict(list) + for key, value in headers: + key = key.lower().strip() + value = value.strip() + normalized[key].append(value) + + ordered_headers = sorted( + (key, ",".join(value)) for key, value in normalized.items() + ) + + return ["{}:{}".format(key, value) for key, value in ordered_headers] + + +_Canonical = collections.namedtuple( + "_Canonical", ["method", "resource", "query_parameters", "headers"] +) + + +def canonicalize(method, resource, query_parameters, headers): + """Canonicalize method, resource + + :type method: str + :param method: The HTTP verb that will be used when requesting the URL. + Defaults to ``'GET'``. If method is ``'RESUMABLE'`` then the + signature will additionally contain the `x-goog-resumable` + header, and the method changed to POST. See the signed URL + docs regarding this flow: + https://cloud.google.com/storage/docs/access-control/signed-urls + + :type resource: str + :param resource: A pointer to a specific resource + (typically, ``/bucket-name/path/to/blob.txt``). + + :type query_parameters: dict + :param query_parameters: + (Optional) Additional query paramtersto be included as part of the + signed URLs. See: + https://cloud.google.com/storage/docs/xml-api/reference-headers#query + + :type headers: Union[dict|List(Tuple(str,str))] + :param headers: + (Optional) Additional HTTP headers to be included as part of the + signed URLs. See: + https://cloud.google.com/storage/docs/xml-api/reference-headers + Requests using the signed URL *must* pass the specified header + (name and value) with each request for the URL. + + :rtype: :class:_Canonical + :returns: Canonical method, resource, query_parameters, and headers. + """ + headers = get_canonical_headers(headers) + + if method == "RESUMABLE": + method = "POST" + headers.append("x-goog-resumable:start") + + if query_parameters is None: + return _Canonical(method, resource, [], headers) + + normalized_qp = sorted( + (key.lower(), value and value.strip() or "") + for key, value in query_parameters.items() + ) + encoded_qp = six.moves.urllib.parse.urlencode(normalized_qp) + canonical_resource = "{}?{}".format(resource, encoded_qp) + return _Canonical(method, canonical_resource, normalized_qp, headers) + + def generate_signed_url_v2( credentials, resource, @@ -192,7 +286,7 @@ def generate_signed_url_v2( :param generation: (Optional) A value that indicates which generation of the resource to fetch. - :type headers: dict + :type headers: Union[dict|List(Tuple(str,str))] :param headers: (Optional) Additional HTTP headers to be included as part of the signed URLs. See: @@ -216,34 +310,15 @@ def generate_signed_url_v2( """ expiration = get_expiration_seconds(expiration) - if method == "RESUMABLE": - method = "POST" - canonicalized_resource = "x-goog-resumable:start\n{0}".format(resource) - else: - canonicalized_resource = "{0}".format(resource) - - if query_parameters is not None: - normalized_qp = list( - sorted( - (key.lower(), value and value.strip() or "") - for key, value in query_parameters.items() - ) - ) - encoded_qp = six.moves.urllib.parse.urlencode(normalized_qp) - canonicalized_resource = "{}?{}".format(canonicalized_resource, encoded_qp) - else: - normalized_qp = () + method, canonical_resource, normalized_qp, canonical_headers = canonicalize( + method, resource, query_parameters, headers + ) # Generate the string to sign. - string_to_sign = "\n".join( - [ - method, - content_md5 or "", - content_type or "", - str(expiration), - canonicalized_resource, - ] - ) + elements_to_sign = [method, content_md5 or "", content_type or "", str(expiration)] + elements_to_sign.extend(canonical_headers) + elements_to_sign.append(canonical_resource) + string_to_sign = "\n".join(elements_to_sign) # Set the right query parameters. signed_query_params = get_signed_query_params( diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index f8f0a842d013..5a79184429af 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -127,6 +127,75 @@ def test_it(self): credentials.sign_bytes.assert_called_once_with(string_to_sign) +class Test_get_canonical_headers(unittest.TestCase): + @staticmethod + def _call_fut(*args, **kwargs): + from google.cloud.storage._signing import get_canonical_headers + + return get_canonical_headers(*args, **kwargs) + + def test_w_none(self): + headers = None + expected = [] + self.assertEqual(self._call_fut(headers), expected) + + def test_w_dict(self): + headers = {"foo": "Foo 1.2.3", "Bar": " baz,bam,qux "} + expected = ["bar:baz,bam,qux", "foo:Foo 1.2.3"] + self.assertEqual(self._call_fut(headers), expected) + + def test_w_list_and_multiples(self): + headers = [ + ("foo", "Foo 1.2.3"), + ("Bar", " baz"), + ("Bar", "bam"), + ("Bar", "qux "), + ] + expected = ["bar:baz,bam,qux", "foo:Foo 1.2.3"] + self.assertEqual(self._call_fut(headers), expected) + + # TODO: handle folded line values? + + +class Test_canonicalize(unittest.TestCase): + @staticmethod + def _call_fut(*args, **kwargs): + from google.cloud.storage._signing import canonicalize + + return canonicalize(*args, **kwargs) + + def test_wo_headers_or_query_parameters(self): + method = "GET" + resource = "/bucket/blob" + canonical = self._call_fut(method, resource, None, None) + self.assertEqual(canonical.method, method) + self.assertEqual(canonical.resource, resource) + self.assertEqual(canonical.query_parameters, []) + self.assertEqual(canonical.headers, []) + + def test_w_headers_and_resumable(self): + method = "RESUMABLE" + resource = "/bucket/blob" + headers = [("x-goog-extension", "foobar")] + canonical = self._call_fut(method, resource, None, headers) + self.assertEqual(canonical.method, "POST") + self.assertEqual(canonical.resource, resource) + self.assertEqual(canonical.query_parameters, []) + self.assertEqual( + canonical.headers, ["x-goog-extension:foobar", "x-goog-resumable:start"] + ) + + def test_w_query_paramters(self): + method = "GET" + resource = "/bucket/blob" + query_parameters = {"foo": "bar", "baz": "qux"} + canonical = self._call_fut(method, resource, query_parameters, None) + self.assertEqual(canonical.method, method) + self.assertEqual(canonical.resource, "{}?baz=qux&foo=bar".format(resource)) + self.assertEqual(canonical.query_parameters, [("baz", "qux"), ("foo", "bar")]) + self.assertEqual(canonical.headers, []) + + class Test_generate_signed_url_v2(unittest.TestCase): @staticmethod def _call_fut(*args, **kwargs): @@ -173,13 +242,18 @@ def _generate_helper( # Check the mock was called. method = method.upper() + if headers is None: + headers = [] + elif isinstance(headers, dict): + headers = sorted(headers.items()) + elements = [] + expected_resource = resource if method == "RESUMABLE": elements.append("POST") - expected_resource = "x-goog-resumable:start\n{}".format(resource) + headers.append(("x-goog-resumable", "start")) else: elements.append(method) - expected_resource = resource if query_parameters is not None: normalized_qp = { @@ -192,6 +266,7 @@ def _generate_helper( elements.append(content_md5 or "") elements.append(content_type or "") elements.append(str(expiration)) + elements.extend(["{}:{}".format(*header) for header in headers]) elements.append(expected_resource) string_to_sign = "\n".join(elements) @@ -257,9 +332,12 @@ def test_w_generation(self): generation = "123" self._generate_helper(generation=generation) - def test_w_custom_headers(self): + def test_w_custom_headers_dict(self): self._generate_helper(headers={"x-goog-foo": "bar"}) + def test_w_custom_headers_list(self): + self._generate_helper(headers=[("x-goog-foo", "bar")]) + def test_w_custom_query_parameters_w_string_value(self): self._generate_helper(query_parameters={"bar": "/"}) @@ -270,13 +348,8 @@ def test_with_google_credentials(self): resource = "/name/path" credentials = _make_credentials() expiration = int(time.time() + 5) - self.assertRaises( - AttributeError, - self._call_fut, - credentials, - resource=resource, - expiration=expiration, - ) + with self.assertRaises(AttributeError): + self._call_fut(credentials, resource=resource, expiration=expiration) class Test_generate_signed_url_v4(unittest.TestCase): From c39ac665f03ebaf8b02fad679c930a213930c7e3 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Thu, 14 Mar 2019 15:38:57 -0400 Subject: [PATCH 14/37] Rename helper to indicate V2-only. --- storage/google/cloud/storage/_signing.py | 4 ++-- storage/tests/unit/test__signing.py | 6 +++--- storage/tests/unit/test_blob.py | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index 926e966b20ed..22b655211c55 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -52,7 +52,7 @@ def ensure_signed_credentials(credentials): ) -def get_signed_query_params(credentials, expiration, string_to_sign): +def get_signed_query_params_v2(credentials, expiration, string_to_sign): """Gets query parameters for creating a signed URL. :type credentials: :class:`google.auth.credentials.Signing` @@ -321,7 +321,7 @@ def generate_signed_url_v2( string_to_sign = "\n".join(elements_to_sign) # Set the right query parameters. - signed_query_params = get_signed_query_params( + signed_query_params = get_signed_query_params_v2( credentials, expiration, string_to_sign ) diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index 5a79184429af..327858c6b591 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -102,12 +102,12 @@ def test_w_timedelta_days(self): utcnow.assert_called_once_with() -class Test_get_signed_query_params(unittest.TestCase): +class Test_get_signed_query_params_v2(unittest.TestCase): @staticmethod def _call_fut(credentials, expiration, string_to_sign): - from google.cloud.storage._signing import get_signed_query_params + from google.cloud.storage._signing import get_signed_query_params_v2 - return get_signed_query_params(credentials, expiration, string_to_sign) + return get_signed_query_params_v2(credentials, expiration, string_to_sign) def test_it(self): sig_bytes = b"DEADBEEF" diff --git a/storage/tests/unit/test_blob.py b/storage/tests/unit/test_blob.py index 75bf83c0fe00..4ca8e3df36dd 100644 --- a/storage/tests/unit/test_blob.py +++ b/storage/tests/unit/test_blob.py @@ -524,14 +524,14 @@ def test_generate_signed_url_v4_w_credentials(self): self._generate_signed_url_v4_helper(credentials=credentials) @mock.patch( - "google.cloud.storage._signing.get_signed_query_params", + "google.cloud.storage._signing.get_signed_query_params_v2", return_value={ "GoogleAccessId": "service-account-name", "Expires": 12345, "Signature": "signed-data", }, ) - def test_generate_resumable_signed_url(self, mock_get_signed_query_params): + def test_generate_resumable_signed_url(self, mock_get_signed_query_params_v2): """ Verify correct behavior of resumable upload URL generation """ @@ -544,7 +544,7 @@ def test_generate_resumable_signed_url(self, mock_get_signed_query_params): _make_credentials(), "a-bucket", expiry, method="RESUMABLE" ) - self.assertTrue(mock_get_signed_query_params.called) + self.assertTrue(mock_get_signed_query_params_v2.called) self.assertGreater(len(signed_url), 0) self.assertIn("a-bucket", signed_url) self.assertIn("GoogleAccessId", signed_url) From 8b94be42803650db91597eb5021fc99647671621 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 29 Mar 2019 16:18:51 -0400 Subject: [PATCH 15/37] Fix typo causing signing tests to be skipped. --- storage/tests/system.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/tests/system.py b/storage/tests/system.py index 35d10d408f2a..da61b2f58f34 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -729,7 +729,7 @@ def setUp(self): if ( type(Config.CLIENT._credentials) - is not google.oauth2.service_account.credentials + is not google.oauth2.service_account.Credentials ): self.skipTest("Signing tests requires a service account credential") From 943e273d672fab96952bb7a375edb04422973d3b Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 29 Mar 2019 16:21:10 -0400 Subject: [PATCH 16/37] Prepare signed URL systests for testing both V2 and V4. --- storage/tests/system.py | 52 +++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/storage/tests/system.py b/storage/tests/system.py index da61b2f58f34..cfecc826e805 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -748,48 +748,41 @@ def tearDown(self): if blob.exists(): retry(blob.delete)() - def test_create_signed_read_url(self): - blob = self.bucket.blob("LogoToSign.jpg") + def _create_signed_read_url_helper( + self, blob_name="LogoToSign.jpg", method="GET", version="v2", payload=None + ): + blob = self.bucket.blob(blob_name) + if payload is not None: + blob.upload_from_string(payload) expiration = int(time.time() + 10) signed_url = blob.generate_signed_url( - expiration, method="GET", client=Config.CLIENT + expiration, method=method, client=Config.CLIENT, version=version ) response = requests.get(signed_url) self.assertEqual(response.status_code, 200) - self.assertEqual(response.content, self.LOCAL_FILE) - - def test_create_signed_read_url_lowercase_method(self): - blob = self.bucket.blob("LogoToSign.jpg") - expiration = int(time.time() + 10) - signed_url = blob.generate_signed_url( - expiration, method="get", client=Config.CLIENT - ) + if payload is not None: + self.assertEqual(response.content, payload) + else: + self.assertEqual(response.content, self.LOCAL_FILE) - response = requests.get(signed_url) - self.assertEqual(response.status_code, 200) - self.assertEqual(response.content, self.LOCAL_FILE) + def test_create_signed_read_url_v2(self): + self._create_signed_read_url_helper() - def test_create_signed_read_url_w_non_ascii_name(self): - blob = self.bucket.blob(u"Caf\xe9.txt") - payload = b"Test signed URL for blob w/ non-ASCII name" - blob.upload_from_string(payload) - self.case_blobs_to_delete.append(blob) + def test_create_signed_read_url_v2_lowercase_method(self): + self._create_signed_read_url_helper(method="get") - expiration = int(time.time() + 10) - signed_url = blob.generate_signed_url( - expiration, method="GET", client=Config.CLIENT + def test_create_signed_read_url_v2_w_non_ascii_name(self): + self._create_signed_read_url_helper( + blob_name=u"Caf\xe9.txt", + payload=b"Test signed URL for blob w/ non-ASCII name", ) - response = requests.get(signed_url) - self.assertEqual(response.status_code, 200) - self.assertEqual(response.content, payload) - - def test_create_signed_delete_url(self): + def _create_signed_delete_url_helper(self, version="v2"): blob = self.bucket.blob("LogoToSign.jpg") expiration = int(time.time() + 283473274) signed_delete_url = blob.generate_signed_url( - expiration, method="DELETE", client=Config.CLIENT + expiration, method="DELETE", client=Config.CLIENT, version=version ) response = requests.request("DELETE", signed_delete_url) @@ -799,6 +792,9 @@ def test_create_signed_delete_url(self): # Check that the blob has actually been deleted. self.assertFalse(blob.exists()) + def test_create_signed_delete_url_v2(self): + self._create_signed_delete_url_helper() + class TestStorageCompose(TestStorageFiles): From 0c9c812eca19798c113fdbbd9e43fa7025fe1b05 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sun, 31 Mar 2019 16:48:09 -0400 Subject: [PATCH 17/37] Add 'max_age' argument to 'Blob.generate_signed_url'. Exclusive with 'expiration', to permit clearer semantics for users. --- storage/google/cloud/storage/_signing.py | 146 ++++++++++++--- storage/google/cloud/storage/blob.py | 17 +- storage/tests/unit/test__signing.py | 216 +++++++++++++++++++---- storage/tests/unit/test_blob.py | 53 +++--- 4 files changed, 339 insertions(+), 93 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index 22b655211c55..ad2ec2adb4aa 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -83,17 +83,34 @@ def get_signed_query_params_v2(credentials, expiration, string_to_sign): } -def get_expiration_seconds(expiration): +def get_expiration_seconds_v2(expiration, max_age): """Convert 'expiration' to a number of seconds in the future. - :type expiration: int, long, datetime.datetime, datetime.timedelta - :param expiration: When the signed URL should expire. + :type expiration: Union[Integer, datetime.datetime, datetime.timedelta] + :param expiration: Point in time when the signed URL should expire. + Exclusive with :arg:`max_age`: exactly one of the + two must be passed. + + :type max_age: Integer + :param max_age: Max number of seconds until the signature expires. + Exclusive with :arg:`expiration`: exactly one of the + two must be passed. + :raises: :exc:`ValueError` when both :arg:`expiration` and + :arg:`max_age` are passed, or when neither is passed. :raises: :exc:`TypeError` when expiration is not a valid type. :rtype: int - :returns: a timestamp as an absolute number of seconds. + :returns: a timestamp as an absolute number of seconds since epoch. """ + if (expiration is None and max_age is None) or ( + expiration is not None and max_age is not None + ): + raise ValueError("Pass exactly one of 'expiration' or 'max_age'.") + + if max_age is not None: + expiration = datetime.timedelta(seconds=max_age) + # If it's a timedelta, add it to `now` in UTC. if isinstance(expiration, datetime.timedelta): now = NOW().replace(tzinfo=_helpers.UTC) @@ -112,6 +129,69 @@ def get_expiration_seconds(expiration): return expiration +_EXPIRATION_TYPES = six.integer_types + (datetime.datetime, datetime.timedelta) + + +def get_expiration_seconds_v4(expiration, max_age): + """Convert 'expiration' to a number of seconds offset from the current time. + + :type expiration: Union[Integer, datetime.datetime, datetime.timedelta] + :param expiration: Point in time when the signed URL should expire. + Exclusive with :arg:`max_age`: exactly one of the + two must be passed. + + :type max_age: Integer + :param max_age: Max number of seconds until the signature expires. + Exclusive with :arg:`expiration`: exactly one of the + two must be passed. + + :raises: :exc:`ValueError` when both :arg:`expiration` and + :arg:`max_age` are passed, or when neither is passed. + :raises: :exc:`TypeError` when expiration is not a valid type. + :raises: :exc:`ValueError` when expiration is too large. + :rtype: Integer + :returns: seconds in the future when the signed URL will expire + """ + if (expiration is None and max_age is None) or ( + expiration is not None and max_age is not None + ): + raise ValueError("Pass exactly one of 'expiration' or 'max_age'.") + + if max_age is not None: + return max_age + + if not isinstance(expiration, _EXPIRATION_TYPES): + raise TypeError( + "Expected an integer timestamp, datetime, or " + "timedelta. Got %s" % type(expiration) + ) + + now = NOW().replace(tzinfo=_helpers.UTC) + + if isinstance(expiration, six.integer_types): + now_seconds = _helpers._microseconds_from_datetime(now) // 10 ** 6 + seconds = expiration - now_seconds + + if isinstance(expiration, datetime.datetime): + + if expiration.tzinfo is None: + expiration = expiration.replace(tzinfo=_helpers.UTC) + + expiration = expiration - now + + if isinstance(expiration, datetime.timedelta): + seconds = int(expiration.total_seconds()) + + if seconds > SEVEN_DAYS: + raise ValueError( + "Max allowed expiration interval is seven days (%d seconds)".format( + SEVEN_DAYS + ) + ) + + return seconds + + def get_canonical_headers(headers): """Canonicalize headers for signing. @@ -208,7 +288,8 @@ def canonicalize(method, resource, query_parameters, headers): def generate_signed_url_v2( credentials, resource, - expiration, + expiration=None, + max_age=None, api_access_endpoint="", method="GET", content_md5=None, @@ -249,9 +330,15 @@ def generate_signed_url_v2( :param resource: A pointer to a specific resource (typically, ``/bucket-name/path/to/blob.txt``). - :type expiration: :class:`int`, :class:`long`, :class:`datetime.datetime`, - :class:`datetime.timedelta` - :param expiration: When the signed URL should expire. + :type expiration: Union[Integer, datetime.datetime, datetime.timedelta] + :param expiration: Point in time when the signed URL should expire. + Exclusive with :arg:`max_age`: exactly one of the + two must be passed. + + :type max_age: Integer + :param max_age: Max number of seconds until the signature expires. + Exclusive with :arg:`expiration`: exactly one of the + two must be passed. :type api_access_endpoint: str :param api_access_endpoint: Optional URI base. Defaults to empty string. @@ -301,6 +388,8 @@ def generate_signed_url_v2( https://cloud.google.com/storage/docs/xml-api/reference-headers#query :raises: :exc:`TypeError` when expiration is not a valid type. + :raises: :exc:`ValueError` when both :arg:`expiration` and + :arg:`max_age` are passed, or when neither is passed. :raises: :exc:`AttributeError` if credentials is not an instance of :class:`google.auth.credentials.Signing`. @@ -308,21 +397,26 @@ def generate_signed_url_v2( :returns: A signed URL you can use to access the resource until expiration. """ - expiration = get_expiration_seconds(expiration) + expiration_stamp = get_expiration_seconds_v2(expiration, max_age) method, canonical_resource, normalized_qp, canonical_headers = canonicalize( method, resource, query_parameters, headers ) # Generate the string to sign. - elements_to_sign = [method, content_md5 or "", content_type or "", str(expiration)] + elements_to_sign = [ + method, + content_md5 or "", + content_type or "", + str(expiration_stamp), + ] elements_to_sign.extend(canonical_headers) elements_to_sign.append(canonical_resource) string_to_sign = "\n".join(elements_to_sign) # Set the right query parameters. signed_query_params = get_signed_query_params_v2( - credentials, expiration, string_to_sign + credentials, expiration_stamp, string_to_sign ) if response_type is not None: @@ -342,13 +436,14 @@ def generate_signed_url_v2( ) -SEVEN_DAYS = 7 * 24 * 60 * 60 # max expiration for V4 signed URLs. +SEVEN_DAYS = 7 * 24 * 60 * 60 # max age for V4 signed URLs. def generate_signed_url_v4( credentials, resource, - expiration, + expiration=None, + max_age=None, api_access_endpoint="", method="GET", content_md5=None, @@ -389,10 +484,15 @@ def generate_signed_url_v4( :param resource: A pointer to a specific resource (typically, ``/bucket-name/path/to/blob.txt``). - :type expiration: :class:`int`, :class:`long`, :class:`datetime.datetime`, - :class:`datetime.timedelta` - :param expiration: When the signed URL should expire. Max value is - seven days. + :type expiration: Union[Integer, datetime.datetime, datetime.timedelta] + :param expiration: Point in time when the signed URL should expire. + Exclusive with :arg:`max_age`: exactly one of the + two must be passed. + + :type max_age: Integer + :param max_age: Max number of seconds until the signature expires. + Exclusive with :arg:`expiration`: exactly one of the + two must be passed. :type api_access_endpoint: str :param api_access_endpoint: Optional URI base. Defaults to empty string. @@ -441,8 +541,9 @@ def generate_signed_url_v4( signed URLs. See: https://cloud.google.com/storage/docs/xml-api/reference-headers#query - :raises: :exc:`ValueError` when expiration is too large. :raises: :exc:`TypeError` when expiration is not a valid type. + :raises: :exc:`ValueError` when both :arg:`expiration` and + :arg:`max_age` are passed, or when neither is passed. :raises: :exc:`AttributeError` if credentials is not an instance of :class:`google.auth.credentials.Signing`. @@ -450,12 +551,7 @@ def generate_signed_url_v4( :returns: A signed URL you can use to access the resource until expiration. """ - expiration = get_expiration_seconds(expiration) - - if expiration > SEVEN_DAYS: - raise ValueError( - "Max allowed expiration is seven days (%d seconds)".format(SEVEN_DAYS) - ) + expiration_seconds = get_expiration_seconds_v4(expiration, max_age) ensure_signed_credentials(credentials) @@ -490,7 +586,7 @@ def generate_signed_url_v4( query_parameters["X-Goog-Algorithm"] = "GOOG4-RSA-SHA256" query_parameters["X-Goog-Credential"] = credential query_parameters["X-Goog-Date"] = request_timestamp - query_parameters["X-Goog-Expires"] = expiration + query_parameters["X-Goog-Expires"] = expiration_seconds query_parameters["X-Goog-SignedHeaders"] = signed_headers if response_type is not None: diff --git a/storage/google/cloud/storage/blob.py b/storage/google/cloud/storage/blob.py index 5e0c8c358ecc..7e69fb9ad026 100644 --- a/storage/google/cloud/storage/blob.py +++ b/storage/google/cloud/storage/blob.py @@ -303,7 +303,8 @@ def public_url(self): def generate_signed_url( self, - expiration, + expiration=None, + max_age=None, api_access_endpoint=_API_ACCESS_ENDPOINT, method="GET", content_md5=None, @@ -338,8 +339,15 @@ def generate_signed_url( accessible blobs, but don't want to require users to explicitly log in. - :type expiration: int, long, datetime.datetime, datetime.timedelta - :param expiration: When the signed URL should expire. + :type expiration: Union[Integer, datetime.datetime, datetime.timedelta] + :param expiration: Point in time when the signed URL should expire. + Exclusive with :arg:`max_age`: exactly one of the + two must be passed. + + :type max_age: Integer + :param max_age: Max number of seconds until the signature expires. + Exclusive with :arg:`expiration`: exactly one of the + two must be passed. :type api_access_endpoint: str :param api_access_endpoint: Optional URI base. Defaults to empty string. @@ -403,6 +411,8 @@ def generate_signed_url( Must be one of 'v2' | 'v4'. :raises: :exc:`ValueError` when version is invalid. + :raises: :exc:`ValueError` when both :arg:`expiration` and + :arg:`max_age` are passed, or when neither is passed. :raises: :exc:`TypeError` when expiration is not a valid type. :raises: :exc:`AttributeError` if credentials is not an instance of :class:`google.auth.credentials.Signing`. @@ -431,6 +441,7 @@ def generate_signed_url( credentials, resource=resource, expiration=expiration, + max_age=max_age, api_access_endpoint=api_access_endpoint, method=method.upper(), content_md5=content_md5, diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index 327858c6b591..aadb1d10b120 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -24,43 +24,66 @@ from six.moves import urllib_parse -class Test_get_expiration_seconds(unittest.TestCase): - @staticmethod - def _call_fut(expiration): - from google.cloud.storage._signing import get_expiration_seconds +def _utc_seconds(when): + return int(calendar.timegm(when.timetuple())) - return get_expiration_seconds(expiration) +class Test_get_expiration_seconds_v2(unittest.TestCase): @staticmethod - def _utc_seconds(when): - return int(calendar.timegm(when.timetuple())) + def _call_fut(expiration, max_age): + from google.cloud.storage._signing import get_expiration_seconds_v2 + + return get_expiration_seconds_v2(expiration, max_age) + + def test_w_invalid_expiration_type(self): + with self.assertRaises(TypeError): + self._call_fut(object(), None) + + def test_w_neither_expiration_nor_max_age(self): + with self.assertRaises(ValueError): + self._call_fut(None, None) + + def test_w_both_expiration_and_max_age(self): + with self.assertRaises(ValueError): + self._call_fut(123, 123) + + def test_w_max_age_int(self): + dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) + + patch = mock.patch( + "google.cloud.storage._signing.NOW", return_value=dummy_utcnow + ) + + with patch as utcnow: + result = self._call_fut(None, 10) - def test_w_invalid(self): - self.assertRaises(TypeError, self._call_fut, object()) - self.assertRaises(TypeError, self._call_fut, None) + delta = datetime.timedelta(seconds=10) + when = dummy_utcnow + delta + expiration = _utc_seconds(when) + self.assertEqual(result, expiration) - def test_w_int(self): - self.assertEqual(self._call_fut(123), 123) + def test_w_expiration_int(self): + self.assertEqual(self._call_fut(123, None), 123) - def test_w_long(self): + def test_w_expiration_long(self): if six.PY3: raise unittest.SkipTest("No long on Python 3") - self.assertEqual(self._call_fut(long(123)), 123) # noqa: F821 + self.assertEqual(self._call_fut(long(123), None), 123) # noqa: F821 - def test_w_naive_datetime(self): + def test_w_expiration_naive_datetime(self): expiration_no_tz = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) - utc_seconds = self._utc_seconds(expiration_no_tz) - self.assertEqual(self._call_fut(expiration_no_tz), utc_seconds) + utc_seconds = _utc_seconds(expiration_no_tz) + self.assertEqual(self._call_fut(expiration_no_tz, None), utc_seconds) - def test_w_utc_datetime(self): + def test_w_expiration_utc_datetime(self): from google.cloud._helpers import UTC expiration_utc = datetime.datetime(2004, 8, 19, 0, 0, 0, 0, UTC) - utc_seconds = self._utc_seconds(expiration_utc) - self.assertEqual(self._call_fut(expiration_utc), utc_seconds) + utc_seconds = _utc_seconds(expiration_utc) + self.assertEqual(self._call_fut(expiration_utc, None), utc_seconds) - def test_w_other_zone_datetime(self): + def test_w_expiration_other_zone_datetime(self): from google.cloud._helpers import _UTC class CET(_UTC): @@ -69,39 +92,158 @@ class CET(_UTC): zone = CET() expiration_other = datetime.datetime(2004, 8, 19, 0, 0, 0, 0, zone) - utc_seconds = self._utc_seconds(expiration_other) + utc_seconds = _utc_seconds(expiration_other) cet_seconds = utc_seconds - (60 * 60) # CET one hour earlier than UTC - self.assertEqual(self._call_fut(expiration_other), cet_seconds) + self.assertEqual(self._call_fut(expiration_other, None), cet_seconds) - def test_w_timedelta_seconds(self): + def test_w_expiration_timedelta_seconds(self): dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) - utc_seconds = self._utc_seconds(dummy_utcnow) + utc_seconds = _utc_seconds(dummy_utcnow) expiration_as_delta = datetime.timedelta(seconds=10) patch = mock.patch( "google.cloud.storage._signing.NOW", return_value=dummy_utcnow ) with patch as utcnow: - result = self._call_fut(expiration_as_delta) + result = self._call_fut(expiration_as_delta, None) self.assertEqual(result, utc_seconds + 10) utcnow.assert_called_once_with() - def test_w_timedelta_days(self): + def test_w_expiration_timedelta_days(self): dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) - utc_seconds = self._utc_seconds(dummy_utcnow) + utc_seconds = _utc_seconds(dummy_utcnow) expiration_as_delta = datetime.timedelta(days=1) patch = mock.patch( "google.cloud.storage._signing.NOW", return_value=dummy_utcnow ) with patch as utcnow: - result = self._call_fut(expiration_as_delta) + result = self._call_fut(expiration_as_delta, None) self.assertEqual(result, utc_seconds + 86400) utcnow.assert_called_once_with() +class Test_get_expiration_seconds_v4(unittest.TestCase): + @staticmethod + def _call_fut(expiration, max_age): + from google.cloud.storage._signing import get_expiration_seconds_v4 + + return get_expiration_seconds_v4(expiration, max_age) + + def test_w_invalid_expiration_type(self): + with self.assertRaises(TypeError): + self._call_fut(object(), None) + + def test_w_neither_expiration_nor_max_age(self): + with self.assertRaises(ValueError): + self._call_fut(None, None) + + def test_w_both_expiration_and_max_age(self): + with self.assertRaises(ValueError): + self._call_fut(123, 123) + + def test_w_max_age_int(self): + self.assertEqual(self._call_fut(None, 10), 10) + + def test_w_expiration_int_gt_seven_days(self): + dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) + delta = datetime.timedelta(days=10) + expiration_utc = dummy_utcnow + delta + expiration_seconds = _utc_seconds(expiration_utc) + + patch = mock.patch( + "google.cloud.storage._signing.NOW", return_value=dummy_utcnow + ) + + with patch as utcnow: + with self.assertRaises(ValueError): + result = self._call_fut(expiration_seconds, None) + + def test_w_expiration_int(self): + dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) + delta = datetime.timedelta(seconds=10) + expiration_utc = dummy_utcnow + delta + expiration_seconds = _utc_seconds(expiration_utc) + + patch = mock.patch( + "google.cloud.storage._signing.NOW", return_value=dummy_utcnow + ) + + with patch as utcnow: + result = self._call_fut(expiration_seconds, None) + + self.assertEqual(result, delta.seconds) + utcnow.assert_called_once_with() + + def test_w_expiration_naive_datetime(self): + dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) + delta = datetime.timedelta(seconds=10) + expiration_no_tz = dummy_utcnow + delta + + patch = mock.patch( + "google.cloud.storage._signing.NOW", return_value=dummy_utcnow + ) + with patch as utcnow: + result = self._call_fut(expiration_no_tz, None) + + self.assertEqual(result, delta.seconds) + utcnow.assert_called_once_with() + + def test_w_expiration_utc_datetime(self): + from google.cloud._helpers import UTC + + dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0, UTC) + delta = datetime.timedelta(seconds=10) + expiration_utc = dummy_utcnow + delta + + patch = mock.patch( + "google.cloud.storage._signing.NOW", return_value=dummy_utcnow + ) + with patch as utcnow: + result = self._call_fut(expiration_utc, None) + + self.assertEqual(result, delta.seconds) + utcnow.assert_called_once_with() + + def test_w_expiration_other_zone_datetime(self): + from google.cloud._helpers import UTC + from google.cloud._helpers import _UTC + + class CET(_UTC): + _tzname = "CET" + _utcoffset = datetime.timedelta(hours=1) + + zone = CET() + dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0, UTC) + dummy_cetnow = dummy_utcnow.astimezone(zone) + delta = datetime.timedelta(seconds=10) + expiration_other = dummy_cetnow + delta + + patch = mock.patch( + "google.cloud.storage._signing.NOW", return_value=dummy_utcnow + ) + with patch as utcnow: + result = self._call_fut(expiration_other, None) + + self.assertEqual(result, delta.seconds) + utcnow.assert_called_once_with() + + def test_w_expiration_timedelta(self): + dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) + expiration_as_delta = datetime.timedelta(seconds=10) + + patch = mock.patch( + "google.cloud.storage._signing.NOW", return_value=dummy_utcnow + ) + with patch as utcnow: + result = self._call_fut(expiration_as_delta, None) + + self.assertEqual(result, expiration_as_delta.total_seconds()) + utcnow.assert_called_once_with() + + class Test_get_signed_query_params_v2(unittest.TestCase): @staticmethod def _call_fut(credentials, expiration, string_to_sign): @@ -224,10 +366,11 @@ def _generate_helper( signed = signed.decode("ascii") expiration = 1000 + url = self._call_fut( credentials, resource, - expiration, + expiration=expiration, api_access_endpoint=api_access_endpoint, method=method, content_md5=content_md5, @@ -361,7 +504,8 @@ def _call_fut(*args, **kwargs): def _generate_helper( self, - expiration=1000, + expiration=None, + max_age=1000, api_access_endpoint="", method="GET", content_type=None, @@ -382,7 +526,8 @@ def _generate_helper( url = self._call_fut( credentials, resource, - expiration, + expiration=expiration, + max_age=max_age, api_access_endpoint=api_access_endpoint, method=method, content_type=content_type, @@ -419,7 +564,7 @@ def _generate_helper( now_stamp = now.strftime("%Y%m%dT%H%M%SZ") self.assertEqual(params["X-Goog-Date"], now_stamp) - self.assertEqual(params["X-Goog-Expires"], str(expiration)) + self.assertEqual(params["X-Goog-Expires"], str(max_age)) signed = binascii.hexlify(credentials.sign_bytes.return_value).decode("ascii") self.assertEqual(params["X-Goog-Signature"], signed) @@ -440,7 +585,7 @@ def _generate_helper( value = value.strip() if value else "" self.assertEqual(params[key].lower(), value) - def test_w_expiration_too_long(self): + def test_w_max_age_too_long(self): with self.assertRaises(ValueError): self._generate_helper(expiration=datetime.timedelta(days=8)) @@ -453,6 +598,9 @@ def test_w_api_access_endpoint(self): def test_w_method(self): self._generate_helper(method="PUT") + def test_w_method_resumable(self): + self._generate_helper(method="RESUMABLE") + def test_w_content_type(self): self._generate_helper(content_type="text/plain") diff --git a/storage/tests/unit/test_blob.py b/storage/tests/unit/test_blob.py index 4ca8e3df36dd..239da142f651 100644 --- a/storage/tests/unit/test_blob.py +++ b/storage/tests/unit/test_blob.py @@ -375,12 +375,20 @@ def _generate_signed_url_helper( headers=None, query_parameters=None, credentials=None, + expiration=None, + max_age=None, ): from six.moves.urllib import parse + from google.cloud._helpers import UTC from google.cloud.storage.blob import _API_ACCESS_ENDPOINT api_access_endpoint = api_access_endpoint or _API_ACCESS_ENDPOINT - EXPIRATION = "2014-10-16T20:34:37.000Z" + + delta = datetime.timedelta(hours=1) + + if expiration is None and max_age is None: + expiration = datetime.datetime.utcnow().replace(tzinfo=UTC) + delta + connection = _Connection() client = _Client(connection) bucket = _Bucket(client) @@ -389,7 +397,8 @@ def _generate_signed_url_helper( with mock.patch(to_patch) as signer: signed_uri = blob.generate_signed_url( - EXPIRATION, + expiration=expiration, + max_age=max_age, api_access_endpoint=api_access_endpoint, method=method, credentials=credentials, @@ -414,7 +423,8 @@ def _generate_signed_url_helper( expected_resource = "/name/{}".format(parse.quote(encoded_name)) expected_kwargs = { "resource": expected_resource, - "expiration": EXPIRATION, + "expiration": expiration, + "max_age": max_age, "api_access_endpoint": api_access_endpoint, "method": method.upper(), "content_md5": content_md5, @@ -434,6 +444,15 @@ def _generate_signed_url_v2_helper(self, **kw): def test_generate_signed_url_v2_w_defaults(self): self._generate_signed_url_v2_helper() + def test_generate_signed_url_v2_w_expiration(self): + from google.cloud._helpers import UTC + + expiration = datetime.datetime.utcnow().replace(tzinfo=UTC) + self._generate_signed_url_v2_helper(expiration=expiration) + + def test_generate_signed_url_v2_w_max_age(self): + self._generate_signed_url_v2_helper(max_age=3600) + def test_generate_signed_url_v2_w_non_ascii_name(self): BLOB_NAME = u"\u0410\u043a\u043a\u043e\u0440\u0434\u044b.txt" self._generate_signed_url_v2_helper(blob_name=BLOB_NAME) @@ -523,34 +542,6 @@ def test_generate_signed_url_v4_w_credentials(self): credentials = object() self._generate_signed_url_v4_helper(credentials=credentials) - @mock.patch( - "google.cloud.storage._signing.get_signed_query_params_v2", - return_value={ - "GoogleAccessId": "service-account-name", - "Expires": 12345, - "Signature": "signed-data", - }, - ) - def test_generate_resumable_signed_url(self, mock_get_signed_query_params_v2): - """ - Verify correct behavior of resumable upload URL generation - """ - from google.cloud.storage._signing import get_expiration_seconds - from google.cloud.storage._signing import generate_signed_url_v2 - - expiry = get_expiration_seconds(datetime.timedelta(hours=1)) - - signed_url = generate_signed_url_v2( - _make_credentials(), "a-bucket", expiry, method="RESUMABLE" - ) - - self.assertTrue(mock_get_signed_query_params_v2.called) - self.assertGreater(len(signed_url), 0) - self.assertIn("a-bucket", signed_url) - self.assertIn("GoogleAccessId", signed_url) - self.assertIn("Expires", signed_url) - self.assertIn("Signature", signed_url) - def test_exists_miss(self): NONESUCH = "nonesuch" not_found_response = ({"status": http_client.NOT_FOUND}, b"") From a78605d30580b8bd53ef72b6ba24a0216b8204f3 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sun, 31 Mar 2019 17:04:28 -0400 Subject: [PATCH 18/37] Add 'Host' header to signed headers, if not passed. --- storage/google/cloud/storage/_signing.py | 4 ++++ storage/tests/unit/test__signing.py | 3 +++ 2 files changed, 7 insertions(+) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index ad2ec2adb4aa..4ef2b40c3afd 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -572,6 +572,10 @@ def generate_signed_url_v4( if content_md5 is not None: headers["Content-MD5"] = content_md5 + header_names = [key.lower() for key in headers] + if "host" not in header_names: + headers["Host"] = "storage.googleapis.com" + ordered_headers = sorted(headers.items()) canonical_headers = "\n".join( ["{}:{}".format(key.lower(), val.strip()) for key, val in ordered_headers] diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index aadb1d10b120..b9ba3893f581 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -616,6 +616,9 @@ def test_w_response_disposition(self): def test_w_generation(self): self._generate_helper(generation=12345) + def test_w_custom_host_header(self): + self._generate_helper(headers={"Host": "api.example.com"}) + def test_w_custom_headers(self): self._generate_helper(headers={"x-goog-foo": "bar"}) From 5a6d44cf2dbfc21d7316b53715e3313ac60dc064 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sun, 31 Mar 2019 17:07:13 -0400 Subject: [PATCH 19/37] Add support for system tests using 'expiration' and 'max_age'. --- storage/tests/system.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/storage/tests/system.py b/storage/tests/system.py index cfecc826e805..a531e37a301c 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -12,9 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. +import datetime import os -import tempfile import re +import tempfile import time import unittest @@ -749,14 +750,27 @@ def tearDown(self): retry(blob.delete)() def _create_signed_read_url_helper( - self, blob_name="LogoToSign.jpg", method="GET", version="v2", payload=None + self, + blob_name="LogoToSign.jpg", + method="GET", + version="v2", + payload=None, + expiration=None, + max_age=None, ): blob = self.bucket.blob(blob_name) if payload is not None: blob.upload_from_string(payload) - expiration = int(time.time() + 10) + + if expiration is None and max_age is None: + max_age = 10 + signed_url = blob.generate_signed_url( - expiration, method=method, client=Config.CLIENT, version=version + expiration=expiration, + max_age=max_age, + method=method, + client=Config.CLIENT, + version=version, ) response = requests.get(signed_url) @@ -769,6 +783,12 @@ def _create_signed_read_url_helper( def test_create_signed_read_url_v2(self): self._create_signed_read_url_helper() + def test_create_signed_read_url_v2_w_expiration(self): + now = datetime.datetime.utcnow() + delta = datetime.timedelta(seconds=10) + + self._create_signed_read_url_helper(expiration=now + delta) + def test_create_signed_read_url_v2_lowercase_method(self): self._create_signed_read_url_helper(method="get") From 9833d689df2deb10d6ddd65fc5e36f44142104a6 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sun, 31 Mar 2019 20:14:36 -0400 Subject: [PATCH 20/37] Add partial V4 conformance test suite and bash to fit. --- storage/google/cloud/storage/_signing.py | 26 ++++--- storage/tests/system.py | 3 + storage/tests/unit/test__signing.py | 68 ++++++++++++++++++ .../unit/url_signer_v4_test_account.json | 12 ++++ .../tests/unit/url_signer_v4_test_data.json | 70 +++++++++++++++++++ 5 files changed, 171 insertions(+), 8 deletions(-) create mode 100644 storage/tests/unit/url_signer_v4_test_account.json create mode 100644 storage/tests/unit/url_signer_v4_test_data.json diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index 4ef2b40c3afd..6f45263ebbb4 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -437,6 +437,7 @@ def generate_signed_url_v2( SEVEN_DAYS = 7 * 24 * 60 * 60 # max age for V4 signed URLs. +DEFAULT_ENDPOINT = "https://storage.googleapis.com" def generate_signed_url_v4( @@ -444,7 +445,7 @@ def generate_signed_url_v4( resource, expiration=None, max_age=None, - api_access_endpoint="", + api_access_endpoint=DEFAULT_ENDPOINT, method="GET", content_md5=None, content_type=None, @@ -453,6 +454,7 @@ def generate_signed_url_v4( generation=None, headers=None, query_parameters=None, + _request_timestamp=None, # for testing only ): """Generate a V4 signed URL to provide query-string auth'n to a resource. @@ -495,7 +497,8 @@ def generate_signed_url_v4( two must be passed. :type api_access_endpoint: str - :param api_access_endpoint: Optional URI base. Defaults to empty string. + :param api_access_endpoint: Optional URI base. Defaults to + "https://storage.googleapis.com/" :type method: str :param method: The HTTP verb that will be used when requesting the URL. @@ -555,9 +558,13 @@ def generate_signed_url_v4( ensure_signed_credentials(credentials) - now = NOW() - request_timestamp = now.strftime("%Y%m%dT%H%M%SZ") - datestamp = now.date().strftime("%Y%m%d") + if _request_timestamp is None: + now = NOW() + request_timestamp = now.strftime("%Y%m%dT%H%M%SZ") + datestamp = now.date().strftime("%Y%m%d") + else: + request_timestamp = _request_timestamp + datestamp = _request_timestamp[:8] client_email = credentials.signer_email credential_scope = "{}/auto/storage/goog4_request".format(datestamp) @@ -577,9 +584,12 @@ def generate_signed_url_v4( headers["Host"] = "storage.googleapis.com" ordered_headers = sorted(headers.items()) - canonical_headers = "\n".join( - ["{}:{}".format(key.lower(), val.strip()) for key, val in ordered_headers] - ) + canonical_headers = ( + "\n".join( + ["{}:{}".format(key.lower(), val.strip()) for key, val in ordered_headers] + ) + + "\n" + ) # Yes, Virginia, the extra newline is part of the spec. signed_headers = ";".join([key.lower() for key, _ in ordered_headers]) if query_parameters is None: diff --git a/storage/tests/system.py b/storage/tests/system.py index a531e37a301c..21f402f00959 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -783,6 +783,9 @@ def _create_signed_read_url_helper( def test_create_signed_read_url_v2(self): self._create_signed_read_url_helper() + def test_create_signed_read_url_v4(self): + self._create_signed_read_url_helper(version="v4") + def test_create_signed_read_url_v2_w_expiration(self): now = datetime.datetime.utcnow() delta = datetime.timedelta(seconds=10) diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index b9ba3893f581..c64fb7fd85bc 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -16,14 +16,36 @@ import binascii import calendar import datetime +import io +import json +import os import time import unittest import mock +import pytest import six from six.moves import urllib_parse +def _read_local_json(json_file): + here = os.path.dirname(__file__) + json_path = os.path.abspath(os.path.join(here, json_file)) + with io.open(json_path, "r", encoding="utf-8-sig") as fileobj: + return json.load(fileobj) + + +_SERVICE_ACCOUNT_JSON = _read_local_json("url_signer_v4_test_account.json") +_CONFORMANCE_TESTS = _read_local_json("url_signer_v4_test_data.json") +_CLIENT_TESTS = [test for test in _CONFORMANCE_TESTS if "bucket" not in test] +_BUCKET_TESTS = [ + test for test in _CONFORMANCE_TESTS if "bucket" in test and "object" not in test +] +_BLOB_TESTS = [ + test for test in _CONFORMANCE_TESTS if "bucket" in test and "object" in test +] + + def _utc_seconds(when): return int(calendar.timegm(when.timetuple())) @@ -629,6 +651,52 @@ def test_w_custom_query_parameters_w_none_value(self): self._generate_helper(query_parameters={"qux": None}) +_DUMMY_SERVICE_ACCOUNT = None + + +def dummy_service_account(): + global _DUMMY_SERVICE_ACCOUNT + + from google.oauth2.service_account import Credentials + + if _DUMMY_SERVICE_ACCOUNT is None: + _DUMMY_SERVICE_ACCOUNT = Credentials.from_service_account_info( + _SERVICE_ACCOUNT_JSON + ) + + return _DUMMY_SERVICE_ACCOUNT + + +@pytest.mark.parametrize("test_data", _CLIENT_TESTS) +@pytest.mark.skip(reason="Bucketless URLs not yet supported") +def test_conformance_client(test_data): + pass # pragma: NO COVER + + +@pytest.mark.parametrize("test_data", _BUCKET_TESTS) +@pytest.mark.skip(reason="Bucket-only URLs not yet supported") +def test_conformance_bucket(test_data): + pass # pragma: NO COVER + + +@pytest.mark.parametrize("test_data", _BLOB_TESTS) +def test_conformance_blob(test_data): + credentials = dummy_service_account() + + resource = "/{}/{}".format(test_data["bucket"], test_data["object"]) + + url = Test_generate_signed_url_v4._call_fut( + credentials, + resource, + max_age=test_data["expiration"], + method=test_data["method"], + _request_timestamp=test_data["timestamp"], + headers=test_data.get("headers"), + ) + + assert url == test_data["expectedUrl"] + + def _make_credentials(signer_email=None): import google.auth.credentials diff --git a/storage/tests/unit/url_signer_v4_test_account.json b/storage/tests/unit/url_signer_v4_test_account.json new file mode 100644 index 000000000000..5fdc01240ef8 --- /dev/null +++ b/storage/tests/unit/url_signer_v4_test_account.json @@ -0,0 +1,12 @@ +{ + "type": "service_account", + "project_id": "dummy-project-id", + "private_key_id": "ffffffffffffffffffffffffffffffffffffffff", + "private_key": "-----BEGIN PRIVATE KEY-----\nMIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQCsPzMirIottfQ2\nryjQmPWocSEeGo7f7Q4/tMQXHlXFzo93AGgU2t+clEj9L5loNhLVq+vk+qmnyDz5\nQ04y8jVWyMYzzGNNrGRW/yaYqnqlKZCy1O3bmnNjV7EDbC/jE1ZLBY0U3HaSHfn6\nS9ND8MXdgD0/ulRTWwq6vU8/w6i5tYsU7n2LLlQTl1fQ7/emO9nYcCFJezHZVa0H\nmeWsdHwWsok0skwQYQNIzP3JF9BpR5gJT2gNge6KopDesJeLoLzaX7cUnDn+CAnn\nLuLDwwSsIVKyVxhBFsFXPplgpaQRwmGzwEbf/Xpt9qo26w2UMgn30jsOaKlSeAX8\ncS6ViF+tAgMBAAECggEACKRuJCP8leEOhQziUx8Nmls8wmYqO4WJJLyk5xUMUC22\nSI4CauN1e0V8aQmxnIc0CDkFT7qc9xBmsMoF+yvobbeKrFApvlyzNyM7tEa/exh8\nDGD/IzjbZ8VfWhDcUTwn5QE9DCoon9m1sG+MBNlokB3OVOt8LieAAREdEBG43kJu\nyQTOkY9BGR2AY1FnAl2VZ/jhNDyrme3tp1sW1BJrawzR7Ujo8DzlVcS2geKA9at7\n55ua5GbHz3hfzFgjVXDfnkWzId6aHypUyqHrSn1SqGEbyXTaleKTc6Pgv0PgkJjG\nhZazWWdSuf1T5Xbs0OhAK9qraoAzT6cXXvMEvvPt6QKBgQDXcZKqJAOnGEU4b9+v\nOdoh+nssdrIOBNMu1m8mYbUVYS1aakc1iDGIIWNM3qAwbG+yNEIi2xi80a2RMw2T\n9RyCNB7yqCXXVKLBiwg9FbKMai6Vpk2bWIrzahM9on7AhCax/X2AeOp+UyYhFEy6\nUFG4aHb8THscL7b515ukSuKb5QKBgQDMq+9PuaB0eHsrmL6q4vHNi3MLgijGg/zu\nAXaPygSYAwYW8KglcuLZPvWrL6OG0+CrfmaWTLsyIZO4Uhdj7MLvX6yK7IMnagvk\nL3xjgxSklEHJAwi5wFeJ8ai/1MIuCn8p2re3CbwISKpvf7Sgs/W4196P4vKvTiAz\njcTiSYFIKQKBgCjMpkS4O0TakMlGTmsFnqyOneLmu4NyIHgfPb9cA4n/9DHKLKAT\noaWxBPgatOVWs7RgtyGYsk+XubHkpC6f3X0+15mGhFwJ+CSE6tN+l2iF9zp52vqP\nQwkjzm7+pdhZbmaIpcq9m1K+9lqPWJRz/3XXuqi+5xWIZ7NaxGvRjqaNAoGAdK2b\nutZ2y48XoI3uPFsuP+A8kJX+CtWZrlE1NtmS7tnicdd19AtfmTuUL6fz0FwfW4Su\nlQZfPT/5B339CaEiq/Xd1kDor+J7rvUHM2+5p+1A54gMRGCLRv92FQ4EON0RC1o9\nm2I4SHysdO3XmjmdXmfp4BsgAKJIJzutvtbqlakCgYB+Cb10z37NJJ+WgjDt+yT2\nyUNH17EAYgWXryfRgTyi2POHuJitd64Xzuy6oBVs3wVveYFM6PIKXlj8/DahYX5I\nR2WIzoCNLL3bEZ+nC6Jofpb4kspoAeRporj29SgesK6QBYWHWX2H645RkRGYGpDo\n51gjy9m/hSNqBbH2zmh04A==\n-----END PRIVATE KEY-----\n", + "client_email": "test-iam-credentials@dummy-project-id.iam.gserviceaccount.com", + "client_id": "000000000000000000000", + "auth_uri": "https://accounts.google.com/o/oauth2/auth", + "token_uri": "https://oauth2.googleapis.com/token", + "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs", + "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com" +} \ No newline at end of file diff --git a/storage/tests/unit/url_signer_v4_test_data.json b/storage/tests/unit/url_signer_v4_test_data.json new file mode 100644 index 000000000000..d29b5a4faea0 --- /dev/null +++ b/storage/tests/unit/url_signer_v4_test_data.json @@ -0,0 +1,70 @@ +[ + { + "description": "Simple GET", + "bucket": "test-bucket", + "object": "test-object", + "method": "GET", + "expiration": 10, + "timestamp": "20190201T090000Z", + "expectedUrl": "https://storage.googleapis.com/test-bucket/test-object?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190201T090000Z&X-Goog-Expires=10&X-Goog-SignedHeaders=host&X-Goog-Signature=95e6a13d43a1d1962e667f17397f2b80ac9bdd1669210d5e08e0135df9dff4e56113485dbe429ca2266487b9d1796ebdee2d7cf682a6ef3bb9fbb4c351686fba90d7b621cf1c4eb1fdf126460dd25fa0837dfdde0a9fd98662ce60844c458448fb2b352c203d9969cb74efa4bdb742287744a4f2308afa4af0e0773f55e32e92973619249214b97283b2daa14195244444e33f938138d1e5f561088ce8011f4986dda33a556412594db7c12fc40e1ff3f1bedeb7a42f5bcda0b9567f17f65855f65071fabb88ea12371877f3f77f10e1466fff6ff6973b74a933322ff0949ce357e20abe96c3dd5cfab42c9c83e740a4d32b9e11e146f0eb3404d2e975896f74" + }, + + { + "description": "Simple PUT", + "bucket": "test-bucket", + "object": "test-object", + "method": "PUT", + "expiration": 10, + "timestamp": "20190201T090000Z", + "expectedUrl": "https://storage.googleapis.com/test-bucket/test-object?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190201T090000Z&X-Goog-Expires=10&X-Goog-SignedHeaders=host&X-Goog-Signature=8adff1d4285739e31aa68e73767a46bc5511fde377497dbe08481bf5ceb34e29cc9a59921748d8ec3dd4085b7e9b7772a952afedfcdaecb3ae8352275b8b7c867f204e3db85076220a3127a8a9589302fc1181eae13b9b7fe41109ec8cdc93c1e8bac2d7a0cc32a109ca02d06957211326563ab3d3e678a0ba296e298b5fc5e14593c99d444c94724cc4be97015dbff1dca377b508fa0cb7169195de98d0e4ac96c42b918d28c8d92d33e1bd125ce0fb3cd7ad2c45dae65c22628378f6584971b8bf3945b26f2611eb651e9b6a8648970c1ecf386bb71327b082e7296c4e1ee2fc0bdd8983da80af375c817fb1ad491d0bc22c0f51dba0d66e2cffbc90803e47" + }, + + { + "description": "POST for resumable uploads", + "bucket": "test-bucket", + "object": "test-object", + "method": "POST", + "expiration": 10, + "headers": { + "x-goog-resumable": "start" + }, + "timestamp": "20190201T090000Z", + "expectedUrl": "https://storage.googleapis.com/test-bucket/test-object?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190201T090000Z&X-Goog-Expires=10&X-Goog-SignedHeaders=host%3Bx-goog-resumable&X-Goog-Signature=4a6d39b23343cedf4c30782aed4b384001828c79ffa3a080a481ea01a640dea0a0ceb58d67a12cef3b243c3f036bb3799c6ee88e8db3eaf7d0bdd4b70a228d0736e07eaa1ee076aff5c6ce09dff1f1f03a0d8ead0d2893408dd3604fdabff553aa6d7af2da67cdba6790006a70240f96717b98f1a6ccb24f00940749599be7ef72aaa5358db63ddd54b2de9e2d6d6a586eac4fe25f36d86fc6ab150418e9c6fa01b732cded226c6d62fc95b72473a4cc55a8257482583fe66d9ab6ede909eb41516a8690946c3e87b0f2052eb0e97e012a14b2f721c42e6e19b8a1cd5658ea36264f10b9b1ada66b8ed5bf7ed7d1708377ac6e5fe608ae361fb594d2e5b24c54" + }, + + { + "description": "Vary expiration and timestamp", + "bucket": "test-bucket", + "object": "test-object", + "method": "GET", + "expiration": 20, + "timestamp": "20190301T090000Z", + "expectedUrl": "https://storage.googleapis.com/test-bucket/test-object?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190301%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190301T090000Z&X-Goog-Expires=20&X-Goog-SignedHeaders=host&X-Goog-Signature=9669ed5b10664dc594c758296580662912cf4bcc5a4ba0b6bf055bcbf6f34eed7bdad664f534962174a924741a0c273a4f67bc1847cef20192a6beab44223bd9d4fbbd749c407b79997598c30f82ddc269ff47ec09fa3afe74e00616d438df0d96a7d8ad0adacfad1dc3286f864d924fe919fb0dce45d3d975c5afe8e13af2db9cc37ba77835f92f7669b61e94c6d562196c1274529e76cfff1564cc2cad7d5387dc8e12f7a5dfd925685fe92c30b43709eee29fa2f66067472cee5423d1a3a4182fe8cea75c9329d181dc6acad7c393cd04f8bf5bc0515127d8ebd65d80c08e19ad03316053ea60033fd1b1fd85a69c576415da3bf0a3718d9ea6d03e0d66f0" + }, + + { + "description": "Vary bucket and object", + "bucket": "test-bucket2", + "object": "test-object2", + "method": "GET", + "expiration": 10, + "timestamp": "20190201T090000Z", + "expectedUrl": "https://storage.googleapis.com/test-bucket2/test-object2?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190201T090000Z&X-Goog-Expires=10&X-Goog-SignedHeaders=host&X-Goog-Signature=36e3d58dfd3ec1d2dd2f24b5ee372a71e811ffaa2162a2b871d26728d0354270bc116face87127532969c4a3967ed05b7309af741e19c7202f3167aa8c2ac420b61417d6451442bb91d7c822cd17be8783f01e05372769c88913561d27e6660dd8259f0081a71f831be6c50283626cbf04494ac10c394b29bb3bce74ab91548f58a37118a452693cf0483d77561fc9cac8f1765d2c724994cca46a83517a10157ee0347a233a2aaeae6e6ab5e204ff8fc5f54f90a3efdb8301d9fff5475d58cd05b181affd657f48203f4fb133c3a3d355b8eefbd10d5a0a5fd70d06e9515460ad74e22334b2cba4b29cae4f6f285cdb92d8f3126d7a1479ca3bdb69c207d860" + }, + + { + "description": "Customer-supplied encryption key", + "bucket": "test-bucket", + "object": "test-object", + "headers": + { + "X-Goog-Encryption-Key": "key", + "X-Goog-Encryption-Key-Sha256": "key-hash", + "X-Goog-Encryption-Algorithm": "AES256" + }, + "method": "GET", + "expiration": 10, + "timestamp": "20190201T090000Z", + "expectedUrl": "https://storage.googleapis.com/test-bucket/test-object?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190201T090000Z&X-Goog-Expires=10&X-Goog-SignedHeaders=host%3Bx-goog-encryption-algorithm%3Bx-goog-encryption-key%3Bx-goog-encryption-key-sha256&X-Goog-Signature=278a1c5a3bad248637054a047014760353942433955871031ed08f515b54588654ad033e91f046ab202b68673030e117d1b786c325e870238b035ba75b3feed560a17aff9bab6bddebd4a31a52cb68b214e27d3b0bd886502c6b36b164306fe88b5a07c6063592afe746b2a5d205dbe90dd5386b94f0a78f75d9f53ee884e18f476e8fc2eb1dd910ce0b4ae1f5d7b09876ef9bf983f539c028429e14bad3c75dbd4ed1ae37856f6d6f8a1805eaf8b52a0d6fc993902e4c1ee8de477661f7b67c3663000474cb00e178189789b2a3ed6bd21b4ade684fca8108ac4dd106acb17f5954d045775f7aa5a98ebda5d3075e11a8ea49c64c6ad1481e463e8c9f11f704" + } +] From c860d51463e0e351a797bc31af9e3e1b76f61006 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sun, 31 Mar 2019 20:22:47 -0400 Subject: [PATCH 21/37] Add support for bucket-level conformance tests. --- storage/tests/unit/test__signing.py | 36 ++++++++++--------- .../tests/unit/url_signer_v4_test_data.json | 10 ++++++ 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index c64fb7fd85bc..a42cf69ea976 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -39,10 +39,10 @@ def _read_local_json(json_file): _CONFORMANCE_TESTS = _read_local_json("url_signer_v4_test_data.json") _CLIENT_TESTS = [test for test in _CONFORMANCE_TESTS if "bucket" not in test] _BUCKET_TESTS = [ - test for test in _CONFORMANCE_TESTS if "bucket" in test and "object" not in test + test for test in _CONFORMANCE_TESTS if "bucket" in test and not test.get("object") ] _BLOB_TESTS = [ - test for test in _CONFORMANCE_TESTS if "bucket" in test and "object" in test + test for test in _CONFORMANCE_TESTS if "bucket" in test and test.get("object") ] @@ -667,6 +667,20 @@ def dummy_service_account(): return _DUMMY_SERVICE_ACCOUNT +def _run_conformance_test(resource, test_data): + credentials = dummy_service_account() + + url = Test_generate_signed_url_v4._call_fut( + credentials, + resource, + max_age=test_data["expiration"], + method=test_data["method"], + _request_timestamp=test_data["timestamp"], + headers=test_data.get("headers"), + ) + + assert url == test_data["expectedUrl"] + @pytest.mark.parametrize("test_data", _CLIENT_TESTS) @pytest.mark.skip(reason="Bucketless URLs not yet supported") def test_conformance_client(test_data): @@ -674,27 +688,15 @@ def test_conformance_client(test_data): @pytest.mark.parametrize("test_data", _BUCKET_TESTS) -@pytest.mark.skip(reason="Bucket-only URLs not yet supported") def test_conformance_bucket(test_data): - pass # pragma: NO COVER + resource = "/{}".format(test_data["bucket"]) + _run_conformance_test(resource, test_data) @pytest.mark.parametrize("test_data", _BLOB_TESTS) def test_conformance_blob(test_data): - credentials = dummy_service_account() - resource = "/{}/{}".format(test_data["bucket"], test_data["object"]) - - url = Test_generate_signed_url_v4._call_fut( - credentials, - resource, - max_age=test_data["expiration"], - method=test_data["method"], - _request_timestamp=test_data["timestamp"], - headers=test_data.get("headers"), - ) - - assert url == test_data["expectedUrl"] + _run_conformance_test(resource, test_data) def _make_credentials(signer_email=None): diff --git a/storage/tests/unit/url_signer_v4_test_data.json b/storage/tests/unit/url_signer_v4_test_data.json index d29b5a4faea0..ad9edde878f9 100644 --- a/storage/tests/unit/url_signer_v4_test_data.json +++ b/storage/tests/unit/url_signer_v4_test_data.json @@ -66,5 +66,15 @@ "expiration": 10, "timestamp": "20190201T090000Z", "expectedUrl": "https://storage.googleapis.com/test-bucket/test-object?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190201T090000Z&X-Goog-Expires=10&X-Goog-SignedHeaders=host%3Bx-goog-encryption-algorithm%3Bx-goog-encryption-key%3Bx-goog-encryption-key-sha256&X-Goog-Signature=278a1c5a3bad248637054a047014760353942433955871031ed08f515b54588654ad033e91f046ab202b68673030e117d1b786c325e870238b035ba75b3feed560a17aff9bab6bddebd4a31a52cb68b214e27d3b0bd886502c6b36b164306fe88b5a07c6063592afe746b2a5d205dbe90dd5386b94f0a78f75d9f53ee884e18f476e8fc2eb1dd910ce0b4ae1f5d7b09876ef9bf983f539c028429e14bad3c75dbd4ed1ae37856f6d6f8a1805eaf8b52a0d6fc993902e4c1ee8de477661f7b67c3663000474cb00e178189789b2a3ed6bd21b4ade684fca8108ac4dd106acb17f5954d045775f7aa5a98ebda5d3075e11a8ea49c64c6ad1481e463e8c9f11f704" + }, + + { + "description": "List Objects", + "bucket": "test-bucket", + "object": "", + "method": "GET", + "expiration": 10, + "timestamp": "20190201T090000Z", + "expectedUrl": "https://storage.googleapis.com/test-bucket?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190201T090000Z&X-Goog-Expires=10&X-Goog-SignedHeaders=host&X-Goog-Signature=6dbe94f8e52b2b8a9a476b1c857efa474e09944e2b52b925800316e094a7169d8dbe0df9c0ac08dabb22ac7e827470ceccd65f5a3eadba2a4fb9beebfe37f0d9bb1e552b851fa31a25045bdf019e507f5feb44f061551ef1aeb18dcec0e38ba2e2f77d560a46eaace9c56ed9aa642281301a9d848b0eb30749e34bc7f73a3d596240533466ff9b5f289cd0d4c845c7d96b82a35a5abd0c3aff83e4440ee6873e796087f43545544dc8c01afe1d79c726696b6f555371e491980e7ec145cca0803cf562c38f3fa1d724242f5dea25aac91d74ec9ddd739ff65523627763eaef25cd1f95ad985aaf0079b7c74eb5bcb2870a9b137a7b2c8e41fbe838c95872f75b" } ] From 87aa6b673e14c1f7f0cba921ae7b7f22e98e8a77 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sun, 31 Mar 2019 20:39:49 -0400 Subject: [PATCH 22/37] Add 'Simple headers' conformance test and fix. Headers have to be sorted *after* lower-casing the keys. --- storage/google/cloud/storage/_signing.py | 11 +++++------ storage/tests/unit/test__signing.py | 1 + storage/tests/unit/url_signer_v4_test_data.json | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index 6f45263ebbb4..dd8c625f59b5 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -583,14 +583,13 @@ def generate_signed_url_v4( if "host" not in header_names: headers["Host"] = "storage.googleapis.com" - ordered_headers = sorted(headers.items()) + ordered_headers = sorted( + [(key.lower(), val.strip()) for key, val in headers.items()] + ) canonical_headers = ( - "\n".join( - ["{}:{}".format(key.lower(), val.strip()) for key, val in ordered_headers] - ) - + "\n" + "\n".join(["{}:{}".format(key, val) for key, val in ordered_headers]) + "\n" ) # Yes, Virginia, the extra newline is part of the spec. - signed_headers = ";".join([key.lower() for key, _ in ordered_headers]) + signed_headers = ";".join([key for key, _ in ordered_headers]) if query_parameters is None: query_parameters = {} diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index a42cf69ea976..5b75cc640104 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -681,6 +681,7 @@ def _run_conformance_test(resource, test_data): assert url == test_data["expectedUrl"] + @pytest.mark.parametrize("test_data", _CLIENT_TESTS) @pytest.mark.skip(reason="Bucketless URLs not yet supported") def test_conformance_client(test_data): diff --git a/storage/tests/unit/url_signer_v4_test_data.json b/storage/tests/unit/url_signer_v4_test_data.json index ad9edde878f9..9ba49b1b6ba9 100644 --- a/storage/tests/unit/url_signer_v4_test_data.json +++ b/storage/tests/unit/url_signer_v4_test_data.json @@ -52,6 +52,20 @@ "expectedUrl": "https://storage.googleapis.com/test-bucket2/test-object2?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190201T090000Z&X-Goog-Expires=10&X-Goog-SignedHeaders=host&X-Goog-Signature=36e3d58dfd3ec1d2dd2f24b5ee372a71e811ffaa2162a2b871d26728d0354270bc116face87127532969c4a3967ed05b7309af741e19c7202f3167aa8c2ac420b61417d6451442bb91d7c822cd17be8783f01e05372769c88913561d27e6660dd8259f0081a71f831be6c50283626cbf04494ac10c394b29bb3bce74ab91548f58a37118a452693cf0483d77561fc9cac8f1765d2c724994cca46a83517a10157ee0347a233a2aaeae6e6ab5e204ff8fc5f54f90a3efdb8301d9fff5475d58cd05b181affd657f48203f4fb133c3a3d355b8eefbd10d5a0a5fd70d06e9515460ad74e22334b2cba4b29cae4f6f285cdb92d8f3126d7a1479ca3bdb69c207d860" }, + { + "description": "Simple headers", + "bucket": "test-bucket", + "object": "test-object", + "headers": { + "foo": "foo-value", + "BAR": "BAR-value" + }, + "method": "GET", + "expiration": 10, + "timestamp": "20190201T090000Z", + "expectedUrl": "https://storage.googleapis.com/test-bucket/test-object?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190201T090000Z&X-Goog-Expires=10&X-Goog-SignedHeaders=bar%3Bfoo%3Bhost&X-Goog-Signature=68ecd3b008328ed30d91e2fe37444ed7b9b03f28ed4424555b5161980531ef87db1c3a5bc0265aad5640af30f96014c94fb2dba7479c41bfe1c020eb90c0c6d387d4dd09d4a5df8b60ea50eb6b01cdd786a1e37020f5f95eb8f9b6cd3f65a1f8a8a65c9fcb61ea662959efd9cd73b683f8d8804ef4d6d9b2852419b013368842731359d7f9e6d1139032ceca75d5e67cee5fd0192ea2125e5f2955d38d3d50cf116f3a52e6a62de77f6207f5b95aaa1d7d0f8a46de89ea72e7ea30f21286318d7eba0142232b0deb3a1dc9e1e812a981c66b5ffda3c6b01a8a9d113155792309fd53a3acfd054ca7776e8eec28c26480cd1e3c812f67f91d14217f39a606669d" + }, + { "description": "Customer-supplied encryption key", "bucket": "test-bucket", From e08ad304be386897633299ba61193ea0004bad23 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sun, 31 Mar 2019 21:13:02 -0400 Subject: [PATCH 23/37] Add conformance tests for collapsed header values and fix. In addition to stripping leading / trailing spaces, collapse multiple interior spaces into one. --- storage/google/cloud/storage/_signing.py | 8 +++++- .../tests/unit/url_signer_v4_test_data.json | 28 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index dd8c625f59b5..c92467d71289 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -18,6 +18,7 @@ import collections import datetime import hashlib +import re import six @@ -438,6 +439,8 @@ def generate_signed_url_v2( SEVEN_DAYS = 7 * 24 * 60 * 60 # max age for V4 signed URLs. DEFAULT_ENDPOINT = "https://storage.googleapis.com" +MULTIPLE_SPACES_RE = r"\s+" +MULTIPLE_SPACES = re.compile(MULTIPLE_SPACES_RE) def generate_signed_url_v4( @@ -584,7 +587,10 @@ def generate_signed_url_v4( headers["Host"] = "storage.googleapis.com" ordered_headers = sorted( - [(key.lower(), val.strip()) for key, val in headers.items()] + [ + (key.lower(), MULTIPLE_SPACES.sub(" ", val.strip())) + for key, val in headers.items() + ] ) canonical_headers = ( "\n".join(["{}:{}".format(key, val) for key, val in ordered_headers]) + "\n" diff --git a/storage/tests/unit/url_signer_v4_test_data.json b/storage/tests/unit/url_signer_v4_test_data.json index 9ba49b1b6ba9..807f6cf49a3e 100644 --- a/storage/tests/unit/url_signer_v4_test_data.json +++ b/storage/tests/unit/url_signer_v4_test_data.json @@ -66,6 +66,34 @@ "expectedUrl": "https://storage.googleapis.com/test-bucket/test-object?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190201T090000Z&X-Goog-Expires=10&X-Goog-SignedHeaders=bar%3Bfoo%3Bhost&X-Goog-Signature=68ecd3b008328ed30d91e2fe37444ed7b9b03f28ed4424555b5161980531ef87db1c3a5bc0265aad5640af30f96014c94fb2dba7479c41bfe1c020eb90c0c6d387d4dd09d4a5df8b60ea50eb6b01cdd786a1e37020f5f95eb8f9b6cd3f65a1f8a8a65c9fcb61ea662959efd9cd73b683f8d8804ef4d6d9b2852419b013368842731359d7f9e6d1139032ceca75d5e67cee5fd0192ea2125e5f2955d38d3d50cf116f3a52e6a62de77f6207f5b95aaa1d7d0f8a46de89ea72e7ea30f21286318d7eba0142232b0deb3a1dc9e1e812a981c66b5ffda3c6b01a8a9d113155792309fd53a3acfd054ca7776e8eec28c26480cd1e3c812f67f91d14217f39a606669d" }, + { + "description": "Headers should be trimmed", + "bucket": "test-bucket", + "object": "test-object", + "headers": { + "leading": " xyz", + "trailing": "abc ", + "collapsed": "abc def" + }, + "method": "GET", + "expiration": 10, + "timestamp": "20190201T090000Z", + "expectedUrl": "https://storage.googleapis.com/test-bucket/test-object?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190201T090000Z&X-Goog-Expires=10&X-Goog-SignedHeaders=collapsed%3Bhost%3Bleading%3Btrailing&X-Goog-Signature=1839511d6238d9ac2bbcbba8b23515b3757db35dfa7b8f9bc4b8b4aa270224df747c812526f1a3bcf294d67ed84cd14e074c36bc090e0a542782934a7c925af4a5ea68123e97533704ce8b08ccdf5fe6b412f89c9fc4de243e29abdb098382c5672188ee3f6fef7131413e252c78e7a35658825ad842a50609e9cc463731e17284ff7a14824c989f87cef22fb99dfec20cfeed69d8b3a08f00b43b8284eecd535e50e982b05cd74c5750cd5f986cfc21a2a05f7f3ab7fc31bd684ed1b823b64d29281e923fc6580c49005552ca19c253de087d9d2df881144e44eda40965cfdb4889bf3a35553c9809f4ed20b8355be481b92b9618952b6a04f3017b36053e15" + }, + + { + "description": "Header value with multiple inline values", + "bucket": "test-bucket", + "object": "test-object", + "headers": { + "multiple": " xyz , abc, def , xyz " + }, + "method": "GET", + "expiration": 10, + "timestamp": "20190201T090000Z", + "expectedUrl": "https://storage.googleapis.com/test-bucket/test-object?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190201T090000Z&X-Goog-Expires=10&X-Goog-SignedHeaders=host%3Bmultiple&X-Goog-Signature=5cc113735625341f59c7203f0c2c9febc95ba6af6b9c38814f8e523214712087dc0996e4960d273ae1889f248ac1e58d4d19cb3a69ad7670e9a8ca1b434e878f59339dc7006cf32dfd715337e9f593e0504371839174962a08294586e0c78160a7aa303397888c8350637c6af3b32ac310886cc4590bfda9ca561ee58fb5b8ec56bc606d2ada6e7df31f4276e9dcb96bcaea39dc2cd096f3fad774f9c4b30e317ad43736c05f76831437f44e8726c1e90d3f6c9827dc273f211f32fc85658dfc5d357eb606743a6b00a29e519eef1bebaf9db3e8f4b1f5f9afb648ad06e60bc42fa8b57025056697c874c9ea76f5a73201c9717ea43e54713ff3502ff3fc626b" + }, + { "description": "Customer-supplied encryption key", "bucket": "test-bucket", From db05933477463f208be7dd7f7aef9e093e1f7999 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sun, 31 Mar 2019 21:23:58 -0400 Subject: [PATCH 24/37] Lint. --- storage/tests/unit/test__signing.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index 5b75cc640104..17f93ec31060 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -83,6 +83,7 @@ def test_w_max_age_int(self): when = dummy_utcnow + delta expiration = _utc_seconds(when) self.assertEqual(result, expiration) + utcnow.assert_called_once_with() def test_w_expiration_int(self): self.assertEqual(self._call_fut(123, None), 123) @@ -181,7 +182,8 @@ def test_w_expiration_int_gt_seven_days(self): with patch as utcnow: with self.assertRaises(ValueError): - result = self._call_fut(expiration_seconds, None) + self._call_fut(expiration_seconds, None) + utcnow.assert_called_once_with() def test_w_expiration_int(self): dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) From dada493235882595ae27c87e4d2104bba30c4430 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sun, 31 Mar 2019 22:05:50 -0400 Subject: [PATCH 25/37] Add V4 systests paralleling existing V2 tests. --- storage/tests/system.py | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/storage/tests/system.py b/storage/tests/system.py index 21f402f00959..c4a402bb2df4 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -761,6 +761,7 @@ def _create_signed_read_url_helper( blob = self.bucket.blob(blob_name) if payload is not None: blob.upload_from_string(payload) + self.case_blobs_to_delete.append(blob) if expiration is None and max_age is None: max_age = 10 @@ -792,32 +793,59 @@ def test_create_signed_read_url_v2_w_expiration(self): self._create_signed_read_url_helper(expiration=now + delta) + def test_create_signed_read_url_v4_w_expiration(self): + now = datetime.datetime.utcnow() + delta = datetime.timedelta(seconds=10) + self._create_signed_read_url_helper(expiration=now + delta, version="v4") + def test_create_signed_read_url_v2_lowercase_method(self): self._create_signed_read_url_helper(method="get") + def test_create_signed_read_url_v4_lowercase_method(self): + self._create_signed_read_url_helper(method="get", version="v4") + def test_create_signed_read_url_v2_w_non_ascii_name(self): self._create_signed_read_url_helper( blob_name=u"Caf\xe9.txt", payload=b"Test signed URL for blob w/ non-ASCII name", ) - def _create_signed_delete_url_helper(self, version="v2"): + def test_create_signed_read_url_v4_w_non_ascii_name(self): + self._create_signed_read_url_helper( + blob_name=u"Caf\xe9.txt", + payload=b"Test signed URL for blob w/ non-ASCII name", + version="v4", + ) + + def _create_signed_delete_url_helper( + self, version="v2", expiration=None, max_age=None + ): + + if expiration is None and max_age is None: + max_age = 10 + blob = self.bucket.blob("LogoToSign.jpg") - expiration = int(time.time() + 283473274) + signed_delete_url = blob.generate_signed_url( - expiration, method="DELETE", client=Config.CLIENT, version=version + expiration=expiration, + max_age=max_age, + method="DELETE", + client=Config.CLIENT, + version=version, ) response = requests.request("DELETE", signed_delete_url) self.assertEqual(response.status_code, 204) self.assertEqual(response.content, b"") - # Check that the blob has actually been deleted. self.assertFalse(blob.exists()) def test_create_signed_delete_url_v2(self): self._create_signed_delete_url_helper() + def test_create_signed_delete_url_v4(self): + self._create_signed_delete_url_helper(version="v4") + class TestStorageCompose(TestStorageFiles): From de892c6bd535cef428110364d983af17a7064fab Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sun, 31 Mar 2019 22:34:50 -0400 Subject: [PATCH 26/37] Add 'Bucket.generate_signed_url' method. --- storage/google/cloud/storage/blob.py | 2 +- storage/google/cloud/storage/bucket.py | 118 +++++++++++++++++ storage/tests/unit/test_bucket.py | 171 ++++++++++++++++++++++++- 3 files changed, 289 insertions(+), 2 deletions(-) diff --git a/storage/google/cloud/storage/blob.py b/storage/google/cloud/storage/blob.py index 7e69fb9ad026..f8f53caf96fc 100644 --- a/storage/google/cloud/storage/blob.py +++ b/storage/google/cloud/storage/blob.py @@ -350,7 +350,7 @@ def generate_signed_url( two must be passed. :type api_access_endpoint: str - :param api_access_endpoint: Optional URI base. Defaults to empty string. + :param api_access_endpoint: Optional URI base. :type method: str :param method: The HTTP verb that will be used when requesting the URL. diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 2e14aad40d84..33afb219b5a6 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -33,6 +33,8 @@ from google.cloud.storage._helpers import _PropertyMixin from google.cloud.storage._helpers import _scalar_property from google.cloud.storage._helpers import _validate_name +from google.cloud.storage._signing import generate_signed_url_v2 +from google.cloud.storage._signing import generate_signed_url_v4 from google.cloud.storage.acl import BucketACL from google.cloud.storage.acl import DefaultObjectACL from google.cloud.storage.blob import Blob @@ -45,6 +47,7 @@ "valid before the bucket is created. Instead, pass the location " "to `Bucket.create`." ) +_API_ACCESS_ENDPOINT = "https://storage.googleapis.com" def _blobs_page_start(iterator, page, response): @@ -1969,3 +1972,118 @@ def lock_retention_policy(self, client=None): method="POST", path=path, query_params=query_params, _target_object=self ) self._set_properties(api_response) + + def generate_signed_url( + self, + expiration=None, + max_age=None, + api_access_endpoint=_API_ACCESS_ENDPOINT, + method="GET", + headers=None, + query_parameters=None, + client=None, + credentials=None, + version="v2", + ): + """Generates a signed URL for this bucket. + + .. note:: + + If you are on Google Compute Engine, you can't generate a signed + URL using GCE service account. Follow `Issue 50`_ for updates on + this. If you'd like to be able to generate a signed URL from GCE, + you can use a standard service account from a JSON file rather + than a GCE service account. + + .. _Issue 50: https://github.com/GoogleCloudPlatform/\ + google-auth-library-python/issues/50 + + If you have a bucket that you want to allow access to for a set + amount of time, you can use this method to generate a URL that + is only valid within a certain time period. + + This is particularly useful if you don't want publicly + accessible buckets, but don't want to require users to explicitly + log in. + + :type expiration: Union[Integer, datetime.datetime, datetime.timedelta] + :param expiration: Point in time when the signed URL should expire. + Exclusive with :arg:`max_age`: exactly one of the + two must be passed. + + :type max_age: Integer + :param max_age: Max number of seconds until the signature expires. + Exclusive with :arg:`expiration`: exactly one of the + two must be passed. + + :type api_access_endpoint: str + :param api_access_endpoint: Optional URI base. + + :type method: str + :param method: The HTTP verb that will be used when requesting the URL. + + :type headers: dict + :param headers: + (Optional) Additional HTTP headers to be included as part of the + signed URLs. See: + https://cloud.google.com/storage/docs/xml-api/reference-headers + Requests using the signed URL *must* pass the specified header + (name and value) with each request for the URL. + + :type query_parameters: dict + :param query_parameters: + (Optional) Additional query paramtersto be included as part of the + signed URLs. See: + https://cloud.google.com/storage/docs/xml-api/reference-headers#query + + :type client: :class:`~google.cloud.storage.client.Client` or + ``NoneType`` + :param client: (Optional) The client to use. If not passed, falls back + to the ``client`` stored on the blob's bucket. + + + :type credentials: :class:`oauth2client.client.OAuth2Credentials` or + :class:`NoneType` + :param credentials: (Optional) The OAuth2 credentials to use to sign + the URL. Defaults to the credentials stored on the + client used. + + :type version: str + :param version: (Optional) The version of signed credential to create. + Must be one of 'v2' | 'v4'. + + :raises: :exc:`ValueError` when version is invalid. + :raises: :exc:`ValueError` when both :arg:`expiration` and + :arg:`max_age` are passed, or when neither is passed. + :raises: :exc:`TypeError` when expiration is not a valid type. + :raises: :exc:`AttributeError` if credentials is not an instance + of :class:`google.auth.credentials.Signing`. + + :rtype: str + :returns: A signed URL you can use to access the resource + until expiration. + """ + if version not in ("v2", "v4"): + raise ValueError("'version' must be either 'v2' or 'v4'") + + resource = "/{bucket_name}".format(bucket_name=self.name) + + if credentials is None: + client = self._require_client(client) + credentials = client._credentials + + if version == "v2": + helper = generate_signed_url_v2 + else: + helper = generate_signed_url_v4 + + return helper( + credentials, + resource=resource, + expiration=expiration, + max_age=max_age, + api_access_endpoint=api_access_endpoint, + method=method.upper(), + headers=headers, + query_parameters=query_parameters, + ) diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 2c70911c005f..882d67ee03a4 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -2573,6 +2573,168 @@ def test_lock_retention_policy_w_user_project(self): {"ifMetagenerationMatch": 1234, "userProject": user_project}, ) + def test_generate_signed_url_w_invalid_version(self): + expiration = "2014-10-16T20:34:37.000Z" + connection = _Connection() + client = _Client(connection) + bucket = self._make_one(name="bucket_name", client=client) + with self.assertRaises(ValueError): + bucket.generate_signed_url(expiration, version="nonesuch") + + def _generate_signed_url_helper( + self, + version, + bucket_name="bucket-name", + api_access_endpoint=None, + method="GET", + content_md5=None, + content_type=None, + response_type=None, + response_disposition=None, + generation=None, + headers=None, + query_parameters=None, + credentials=None, + expiration=None, + max_age=None, + ): + from six.moves.urllib import parse + from google.cloud._helpers import UTC + from google.cloud.storage.blob import _API_ACCESS_ENDPOINT + + api_access_endpoint = api_access_endpoint or _API_ACCESS_ENDPOINT + + delta = datetime.timedelta(hours=1) + + if expiration is None and max_age is None: + expiration = datetime.datetime.utcnow().replace(tzinfo=UTC) + delta + + connection = _Connection() + client = _Client(connection) + bucket = self._make_one(name=bucket_name, client=client) + to_patch = "google.cloud.storage.bucket.generate_signed_url_{}".format(version) + + with mock.patch(to_patch) as signer: + signed_uri = bucket.generate_signed_url( + expiration=expiration, + max_age=max_age, + api_access_endpoint=api_access_endpoint, + method=method, + credentials=credentials, + headers=headers, + query_parameters=query_parameters, + version=version, + ) + + self.assertEqual(signed_uri, signer.return_value) + + if credentials is None: + expected_creds = client._credentials + else: + expected_creds = credentials + + encoded_name = bucket_name.encode("utf-8") + expected_resource = "/{}".format(parse.quote(encoded_name)) + expected_kwargs = { + "resource": expected_resource, + "expiration": expiration, + "max_age": max_age, + "api_access_endpoint": api_access_endpoint, + "method": method.upper(), + "headers": headers, + "query_parameters": query_parameters, + } + signer.assert_called_once_with(expected_creds, **expected_kwargs) + + def _generate_signed_url_v2_helper(self, **kw): + version = "v2" + self._generate_signed_url_helper(version, **kw) + + def test_generate_signed_url_v2_w_defaults(self): + self._generate_signed_url_v2_helper() + + def test_generate_signed_url_v2_w_expiration(self): + from google.cloud._helpers import UTC + + expiration = datetime.datetime.utcnow().replace(tzinfo=UTC) + self._generate_signed_url_v2_helper(expiration=expiration) + + def test_generate_signed_url_v2_w_max_age(self): + self._generate_signed_url_v2_helper(max_age=3600) + + def test_generate_signed_url_v2_w_endpoint(self): + self._generate_signed_url_v2_helper( + api_access_endpoint="https://api.example.com/v1" + ) + + def test_generate_signed_url_v2_w_method(self): + self._generate_signed_url_v2_helper(method="POST") + + def test_generate_signed_url_v2_w_lowercase_method(self): + self._generate_signed_url_v2_helper(method="get") + + def test_generate_signed_url_v2_w_content_md5(self): + self._generate_signed_url_v2_helper(content_md5="FACEDACE") + + def test_generate_signed_url_v2_w_content_type(self): + self._generate_signed_url_v2_helper(content_type="text.html") + + def test_generate_signed_url_v2_w_response_type(self): + self._generate_signed_url_v2_helper(response_type="text.html") + + def test_generate_signed_url_v2_w_response_disposition(self): + self._generate_signed_url_v2_helper(response_disposition="inline") + + def test_generate_signed_url_v2_w_generation(self): + self._generate_signed_url_v2_helper(generation=12345) + + def test_generate_signed_url_v2_w_headers(self): + self._generate_signed_url_v2_helper(headers={"x-goog-foo": "bar"}) + + def test_generate_signed_url_v2_w_credentials(self): + credentials = object() + self._generate_signed_url_v2_helper(credentials=credentials) + + def _generate_signed_url_v4_helper(self, **kw): + version = "v4" + self._generate_signed_url_helper(version, **kw) + + def test_generate_signed_url_v4_w_defaults(self): + self._generate_signed_url_v2_helper() + + def test_generate_signed_url_v4_w_endpoint(self): + self._generate_signed_url_v4_helper( + api_access_endpoint="https://api.example.com/v1" + ) + + def test_generate_signed_url_v4_w_method(self): + self._generate_signed_url_v4_helper(method="POST") + + def test_generate_signed_url_v4_w_lowercase_method(self): + self._generate_signed_url_v4_helper(method="get") + + def test_generate_signed_url_v4_w_content_md5(self): + self._generate_signed_url_v4_helper(content_md5="FACEDACE") + + def test_generate_signed_url_v4_w_content_type(self): + self._generate_signed_url_v4_helper(content_type="text.html") + + def test_generate_signed_url_v4_w_response_type(self): + self._generate_signed_url_v4_helper(response_type="text.html") + + def test_generate_signed_url_v4_w_response_disposition(self): + self._generate_signed_url_v4_helper(response_disposition="inline") + + def test_generate_signed_url_v4_w_generation(self): + self._generate_signed_url_v4_helper(generation=12345) + + def test_generate_signed_url_v4_w_headers(self): + self._generate_signed_url_v4_helper(headers={"x-goog-foo": "bar"}) + + def test_generate_signed_url_v4_w_credentials(self): + credentials = object() + self._generate_signed_url_v4_helper(credentials=credentials) + class _Connection(object): _delete_bucket = False @@ -2612,6 +2774,13 @@ def api_request(self, **kw): class _Client(object): def __init__(self, connection, project=None): - self._connection = connection self._base_connection = connection self.project = project + + @property + def _connection(self): + return self._base_connection + + @property + def _credentials(self): + return self._base_connection.credentials From eae12c56022269fff42774c52f68ce5e9ffba147 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sun, 31 Mar 2019 22:44:38 -0400 Subject: [PATCH 27/37] Add systests for listing bucket blobs (V2/V4). --- storage/tests/system.py | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/storage/tests/system.py b/storage/tests/system.py index c4a402bb2df4..c953adf7c8c2 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -19,6 +19,7 @@ import time import unittest +import pytest import requests import six @@ -749,6 +750,42 @@ def tearDown(self): if blob.exists(): retry(blob.delete)() + def _create_signed_list_blobs_url_helper( + self, version, expiration=None, max_age=None, method="GET" + ): + if expiration is None and max_age is None: + max_age = 10 + + signed_url = self.bucket.generate_signed_url( + expiration=expiration, + max_age=max_age, + method=method, + client=Config.CLIENT, + version=version, + ) + + response = requests.get(signed_url) + self.assertEqual(response.status_code, 200) + + def test_create_signed_list_blobs_url_v2(self): + self._create_signed_list_blobs_url_helper(version="v2") + + def test_create_signed_list_blobs_url_v2_w_expiration(self): + now = datetime.datetime.utcnow() + delta = datetime.timedelta(seconds=10) + + self._create_signed_list_blobs_url_helper(expiration=now + delta, version="v2") + + @pytest.mark.skip(reason="Back-end case-flattening bug: revisit 2019-04-03") + def test_create_signed_list_blobs_url_v4(self): + self._create_signed_list_blobs_url_helper(version="v4") + + @pytest.mark.skip(reason="Back-end case-flattening bug: revisit 2019-04-03") + def test_create_signed_list_blobs_url_v4_w_expiration(self): + now = datetime.datetime.utcnow() + delta = datetime.timedelta(seconds=10) + self._create_signed_list_blobs_url_helper(expiration=now + delta, version="v4") + def _create_signed_read_url_helper( self, blob_name="LogoToSign.jpg", From dc7bd126a29e80ad09d7f8d982bca6f9247e9334 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sun, 31 Mar 2019 23:55:59 -0400 Subject: [PATCH 28/37] Add warning for users signing URLs without passing an explicit version. --- storage/google/cloud/storage/blob.py | 13 ++++++- storage/google/cloud/storage/bucket.py | 8 +++- storage/tests/unit/test_blob.py | 52 +++++++++++++++++--------- storage/tests/unit/test_bucket.py | 42 +++++++++++++++------ 4 files changed, 82 insertions(+), 33 deletions(-) diff --git a/storage/google/cloud/storage/blob.py b/storage/google/cloud/storage/blob.py index f8f53caf96fc..7480ecced4ce 100644 --- a/storage/google/cloud/storage/blob.py +++ b/storage/google/cloud/storage/blob.py @@ -97,6 +97,12 @@ _READ_LESS_THAN_SIZE = ( "Size {:d} was specified but the file-like object only had " "{:d} bytes remaining." ) +_SIGNED_URL_V2_DEFAULT_MESSAGE = ( + "You have generated a signed URL using the default v2 signing " + "implementation. In the future, this will default to v4. " + "You may experience breaking changes if you use longer than 7 day " + "expiration times with v4. To opt-in to the behavior specify version='v2'." +) _DEFAULT_CHUNKSIZE = 104857600 # 1024 * 1024 B * 100 = 100 MB _MAX_MULTIPART_SIZE = 8388608 # 8 MB @@ -316,7 +322,7 @@ def generate_signed_url( query_parameters=None, client=None, credentials=None, - version="v2", + version=None, ): """Generates a signed URL for this blob. @@ -421,7 +427,10 @@ def generate_signed_url( :returns: A signed URL you can use to access the resource until expiration. """ - if version not in ("v2", "v4"): + if version is None: + version = "v2" + warnings.warn(DeprecationWarning(_SIGNED_URL_V2_DEFAULT_MESSAGE)) + elif version not in ("v2", "v4"): raise ValueError("'version' must be either 'v2' or 'v4'") resource = "/{bucket_name}/{quoted_name}".format( diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 33afb219b5a6..31bf1d732f38 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -37,6 +37,7 @@ from google.cloud.storage._signing import generate_signed_url_v4 from google.cloud.storage.acl import BucketACL from google.cloud.storage.acl import DefaultObjectACL +from google.cloud.storage.blob import _SIGNED_URL_V2_DEFAULT_MESSAGE from google.cloud.storage.blob import Blob from google.cloud.storage.notification import BucketNotification from google.cloud.storage.notification import NONE_PAYLOAD_FORMAT @@ -1983,7 +1984,7 @@ def generate_signed_url( query_parameters=None, client=None, credentials=None, - version="v2", + version=None, ): """Generates a signed URL for this bucket. @@ -2063,7 +2064,10 @@ def generate_signed_url( :returns: A signed URL you can use to access the resource until expiration. """ - if version not in ("v2", "v4"): + if version is None: + version = "v2" + warnings.warn(DeprecationWarning(_SIGNED_URL_V2_DEFAULT_MESSAGE)) + elif version not in ("v2", "v4"): raise ValueError("'version' must be either 'v2' or 'v4'") resource = "/{bucket_name}".format(bucket_name=self.name) diff --git a/storage/tests/unit/test_blob.py b/storage/tests/unit/test_blob.py index 239da142f651..db91c53931c3 100644 --- a/storage/tests/unit/test_blob.py +++ b/storage/tests/unit/test_blob.py @@ -20,6 +20,7 @@ import os import tempfile import unittest +import warnings import google.cloud.storage.blob import mock @@ -363,7 +364,7 @@ def test_generate_signed_url_w_invalid_version(self): def _generate_signed_url_helper( self, - version, + version=None, blob_name="blob-name", api_access_endpoint=None, method="GET", @@ -393,24 +394,38 @@ def _generate_signed_url_helper( client = _Client(connection) bucket = _Bucket(client) blob = self._make_one(blob_name, bucket=bucket) - to_patch = "google.cloud.storage.blob.generate_signed_url_{}".format(version) + + if version is None: + effective_version = "v2" + else: + effective_version = version + + to_patch = "google.cloud.storage.blob.generate_signed_url_{}".format( + effective_version) with mock.patch(to_patch) as signer: - signed_uri = blob.generate_signed_url( - expiration=expiration, - max_age=max_age, - api_access_endpoint=api_access_endpoint, - method=method, - credentials=credentials, - content_md5=content_md5, - content_type=content_type, - response_type=response_type, - response_disposition=response_disposition, - generation=generation, - headers=headers, - query_parameters=query_parameters, - version=version, - ) + with warnings.catch_warnings(record=True) as warned: + signed_uri = blob.generate_signed_url( + expiration=expiration, + max_age=max_age, + api_access_endpoint=api_access_endpoint, + method=method, + credentials=credentials, + content_md5=content_md5, + content_type=content_type, + response_type=response_type, + response_disposition=response_disposition, + generation=generation, + headers=headers, + query_parameters=query_parameters, + version=version, + ) + + if version is None: + self.assertEqual(len(warned), 1) + self.assertIs(warned[0].category, DeprecationWarning) + else: + self.assertEqual(len(warned), 0) self.assertEqual(signed_uri, signer.return_value) @@ -437,6 +452,9 @@ def _generate_signed_url_helper( } signer.assert_called_once_with(expected_creds, **expected_kwargs) + def test_generate_signed_url_no_version_passed_warning(self): + self._generate_signed_url_helper() + def _generate_signed_url_v2_helper(self, **kw): version = "v2" self._generate_signed_url_helper(version, **kw) diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 882d67ee03a4..6b7a035d8aa2 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -14,6 +14,7 @@ import datetime import unittest +import warnings import mock @@ -2583,7 +2584,7 @@ def test_generate_signed_url_w_invalid_version(self): def _generate_signed_url_helper( self, - version, + version=None, bucket_name="bucket-name", api_access_endpoint=None, method="GET", @@ -2612,19 +2613,33 @@ def _generate_signed_url_helper( connection = _Connection() client = _Client(connection) bucket = self._make_one(name=bucket_name, client=client) - to_patch = "google.cloud.storage.bucket.generate_signed_url_{}".format(version) + + if version is None: + effective_version = "v2" + else: + effective_version = version + + to_patch = "google.cloud.storage.bucket.generate_signed_url_{}".format( + effective_version) with mock.patch(to_patch) as signer: - signed_uri = bucket.generate_signed_url( - expiration=expiration, - max_age=max_age, - api_access_endpoint=api_access_endpoint, - method=method, - credentials=credentials, - headers=headers, - query_parameters=query_parameters, - version=version, - ) + with warnings.catch_warnings(record=True) as warned: + signed_uri = bucket.generate_signed_url( + expiration=expiration, + max_age=max_age, + api_access_endpoint=api_access_endpoint, + method=method, + credentials=credentials, + headers=headers, + query_parameters=query_parameters, + version=version, + ) + + if version is None: + self.assertEqual(len(warned), 1) + self.assertIs(warned[0].category, DeprecationWarning) + else: + self.assertEqual(len(warned), 0) self.assertEqual(signed_uri, signer.return_value) @@ -2646,6 +2661,9 @@ def _generate_signed_url_helper( } signer.assert_called_once_with(expected_creds, **expected_kwargs) + def test_generate_signed_url_no_version_passed_warning(self): + self._generate_signed_url_helper() + def _generate_signed_url_v2_helper(self, **kw): version = "v2" self._generate_signed_url_helper(version, **kw) From c52e2dbfa64a4d9ebf7c445622fb12f4fa851e20 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 1 Apr 2019 01:57:14 -0400 Subject: [PATCH 29/37] Clean up V2 signing using 'canonicalize' better. --- storage/google/cloud/storage/_signing.py | 21 ++++++++++----------- storage/tests/unit/test__signing.py | 5 ++++- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index c92467d71289..4705b29e09ee 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -27,6 +27,8 @@ NOW = datetime.datetime.utcnow # To be replaced by tests. +MULTIPLE_SPACES_RE = r"\s+" +MULTIPLE_SPACES = re.compile(MULTIPLE_SPACES_RE) def ensure_signed_credentials(credentials): @@ -221,7 +223,7 @@ def get_canonical_headers(headers): normalized = collections.defaultdict(list) for key, value in headers: key = key.lower().strip() - value = value.strip() + value = MULTIPLE_SPACES.sub(" ", value.strip()) normalized[key].append(value) ordered_headers = sorted( @@ -400,19 +402,17 @@ def generate_signed_url_v2( """ expiration_stamp = get_expiration_seconds_v2(expiration, max_age) - method, canonical_resource, normalized_qp, canonical_headers = canonicalize( - method, resource, query_parameters, headers - ) + canonical = canonicalize(method, resource, query_parameters, headers) # Generate the string to sign. elements_to_sign = [ - method, + canonical.method, content_md5 or "", content_type or "", str(expiration_stamp), ] - elements_to_sign.extend(canonical_headers) - elements_to_sign.append(canonical_resource) + elements_to_sign.extend(canonical.headers) + elements_to_sign.append(canonical.resource) string_to_sign = "\n".join(elements_to_sign) # Set the right query parameters. @@ -427,20 +427,19 @@ def generate_signed_url_v2( if generation is not None: signed_query_params["generation"] = generation - signed_query_params.update(normalized_qp) + signed_query_params.update(canonical.query_parameters) + sorted_signed_query_params = sorted(signed_query_params.items()) # Return the built URL. return "{endpoint}{resource}?{querystring}".format( endpoint=api_access_endpoint, resource=resource, - querystring=six.moves.urllib.parse.urlencode(signed_query_params), + querystring=six.moves.urllib.parse.urlencode(sorted_signed_query_params), ) SEVEN_DAYS = 7 * 24 * 60 * 60 # max age for V4 signed URLs. DEFAULT_ENDPOINT = "https://storage.googleapis.com" -MULTIPLE_SPACES_RE = r"\s+" -MULTIPLE_SPACES = re.compile(MULTIPLE_SPACES_RE) def generate_signed_url_v4( diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index 17f93ec31060..f29cd51b6f6b 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -320,7 +320,10 @@ def test_w_list_and_multiples(self): expected = ["bar:baz,bam,qux", "foo:Foo 1.2.3"] self.assertEqual(self._call_fut(headers), expected) - # TODO: handle folded line values? + def test_w_embedded_ws(self): + headers = {"foo": "Foo\n1.2.3", "Bar": " baz bam qux "} + expected = ["bar:baz bam qux", "foo:Foo 1.2.3"] + self.assertEqual(self._call_fut(headers), expected) class Test_canonicalize(unittest.TestCase): From 2c5759e09c69c9c3f8cbb2664803c88351bd806d Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 1 Apr 2019 02:31:26 -0400 Subject: [PATCH 30/37] Tidy up V4 signing using 'get_canonical_headers'. --- storage/google/cloud/storage/_signing.py | 31 ++++++++++-------------- storage/tests/unit/test__signing.py | 28 +++++++++++++++------ 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index 4705b29e09ee..41aa5ad07d09 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -218,19 +218,20 @@ def get_canonical_headers(headers): headers = list(headers.items()) if not headers: - return [] + return [], [] normalized = collections.defaultdict(list) - for key, value in headers: + for key, val in headers: key = key.lower().strip() - value = MULTIPLE_SPACES.sub(" ", value.strip()) - normalized[key].append(value) + val = MULTIPLE_SPACES.sub(" ", val.strip()) + normalized[key].append(val) ordered_headers = sorted( - (key, ",".join(value)) for key, value in normalized.items() + (key, ",".join(val)) for key, val in normalized.items() ) - return ["{}:{}".format(key, value) for key, value in ordered_headers] + canonical_headers = ["{}:{}".format(*item) for item in ordered_headers] + return canonical_headers, ordered_headers _Canonical = collections.namedtuple( @@ -270,7 +271,7 @@ def canonicalize(method, resource, query_parameters, headers): :rtype: :class:_Canonical :returns: Canonical method, resource, query_parameters, and headers. """ - headers = get_canonical_headers(headers) + headers, _ = get_canonical_headers(headers) if method == "RESUMABLE": method = "POST" @@ -556,9 +557,8 @@ def generate_signed_url_v4( :returns: A signed URL you can use to access the resource until expiration. """ - expiration_seconds = get_expiration_seconds_v4(expiration, max_age) - ensure_signed_credentials(credentials) + expiration_seconds = get_expiration_seconds_v4(expiration, max_age) if _request_timestamp is None: now = NOW() @@ -585,14 +585,9 @@ def generate_signed_url_v4( if "host" not in header_names: headers["Host"] = "storage.googleapis.com" - ordered_headers = sorted( - [ - (key.lower(), MULTIPLE_SPACES.sub(" ", val.strip())) - for key, val in headers.items() - ] - ) - canonical_headers = ( - "\n".join(["{}:{}".format(key, val) for key, val in ordered_headers]) + "\n" + canonical_headers, ordered_headers = get_canonical_headers(headers) + canonical_header_string = ( + "\n".join(canonical_headers) + "\n" ) # Yes, Virginia, the extra newline is part of the spec. signed_headers = ";".join([key for key, _ in ordered_headers]) @@ -623,7 +618,7 @@ def generate_signed_url_v4( method, resource, canonical_query_string, - canonical_headers, + canonical_header_string, signed_headers, "UNSIGNED-PAYLOAD", ] diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index f29cd51b6f6b..7fd6d0bd3a3b 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -302,13 +302,19 @@ def _call_fut(*args, **kwargs): def test_w_none(self): headers = None - expected = [] - self.assertEqual(self._call_fut(headers), expected) + expected_canonical = [] + expected_ordered = [] + canonical, ordered = self._call_fut(headers) + self.assertEqual(canonical, expected_canonical) + self.assertEqual(ordered, expected_ordered) def test_w_dict(self): headers = {"foo": "Foo 1.2.3", "Bar": " baz,bam,qux "} - expected = ["bar:baz,bam,qux", "foo:Foo 1.2.3"] - self.assertEqual(self._call_fut(headers), expected) + expected_canonical = ["bar:baz,bam,qux", "foo:Foo 1.2.3"] + expected_ordered = [tuple(item.split(":")) for item in expected_canonical] + canonical, ordered = self._call_fut(headers) + self.assertEqual(canonical, expected_canonical) + self.assertEqual(ordered, expected_ordered) def test_w_list_and_multiples(self): headers = [ @@ -317,13 +323,19 @@ def test_w_list_and_multiples(self): ("Bar", "bam"), ("Bar", "qux "), ] - expected = ["bar:baz,bam,qux", "foo:Foo 1.2.3"] - self.assertEqual(self._call_fut(headers), expected) + expected_canonical = ["bar:baz,bam,qux", "foo:Foo 1.2.3"] + expected_ordered = [tuple(item.split(":")) for item in expected_canonical] + canonical, ordered = self._call_fut(headers) + self.assertEqual(canonical, expected_canonical) + self.assertEqual(ordered, expected_ordered) def test_w_embedded_ws(self): headers = {"foo": "Foo\n1.2.3", "Bar": " baz bam qux "} - expected = ["bar:baz bam qux", "foo:Foo 1.2.3"] - self.assertEqual(self._call_fut(headers), expected) + expected_canonical = ["bar:baz bam qux", "foo:Foo 1.2.3"] + expected_ordered = [tuple(item.split(":")) for item in expected_canonical] + canonical, ordered = self._call_fut(headers) + self.assertEqual(canonical, expected_canonical) + self.assertEqual(ordered, expected_ordered) class Test_canonicalize(unittest.TestCase): From 7617b4f1485ff8c3311bfd2afc2f4c2e842b034f Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 1 Apr 2019 02:57:46 -0400 Subject: [PATCH 31/37] Handle 'RESUMABLE' method alias for V4 signing. --- storage/google/cloud/storage/_signing.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index 41aa5ad07d09..8f9552641843 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -585,6 +585,10 @@ def generate_signed_url_v4( if "host" not in header_names: headers["Host"] = "storage.googleapis.com" + if method.upper() == "RESUMABLE": + method = "POST" + headers["x-goog-resumable"] = "start" + canonical_headers, ordered_headers = get_canonical_headers(headers) canonical_header_string = ( "\n".join(canonical_headers) + "\n" From d33c68ee5270f1b5c6f93bec080d9762be065a63 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 1 Apr 2019 03:00:19 -0400 Subject: [PATCH 32/37] Add round-trip systest for signed resumable uploads. --- storage/tests/system.py | 105 +++++++++++++++++++++++++++++++--------- 1 file changed, 83 insertions(+), 22 deletions(-) diff --git a/storage/tests/system.py b/storage/tests/system.py index c953adf7c8c2..d9f456020aab 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -16,7 +16,6 @@ import os import re import tempfile -import time import unittest import pytest @@ -83,6 +82,7 @@ def setUpModule(): def tearDownModule(): + _empty_bucket(Config.TEST_BUCKET) errors = (exceptions.Conflict, exceptions.TooManyRequests) retry = RetryErrors(errors, max_tries=9) retry(_empty_bucket)(Config.TEST_BUCKET) @@ -725,30 +725,27 @@ def test_third_level(self): self.assertEqual(iterator.prefixes, set()) -class TestStorageSignURLs(TestStorageFiles): - def setUp(self): - super(TestStorageSignURLs, self).setUp() - +class TestStorageSignURLs(unittest.TestCase): + BLOB_CONTENT = b"This time for sure, Rocky!" + @classmethod + def setUpClass(cls): if ( type(Config.CLIENT._credentials) is not google.oauth2.service_account.Credentials ): - self.skipTest("Signing tests requires a service account credential") + cls.skipTest("Signing tests requires a service account credential") - logo_path = self.FILES["logo"]["path"] - with open(logo_path, "rb") as file_obj: - self.LOCAL_FILE = file_obj.read() + bucket_name = "gcp-signing" + unique_resource_id() + cls.bucket = Config.CLIENT.create_bucket(bucket_name) + cls.blob = cls.bucket.blob("README.txt") + cls.blob.upload_from_string(cls.BLOB_CONTENT) - blob = self.bucket.blob("LogoToSign.jpg") - blob.upload_from_string(self.LOCAL_FILE) - self.case_blobs_to_delete.append(blob) - - def tearDown(self): - errors = (exceptions.TooManyRequests, exceptions.ServiceUnavailable) + @classmethod + def tearDownClass(cls): + _empty_bucket(cls.bucket) + errors = (exceptions.Conflict, exceptions.TooManyRequests) retry = RetryErrors(errors, max_tries=6) - for blob in self.case_blobs_to_delete: - if blob.exists(): - retry(blob.delete)() + retry(cls.bucket.delete)(force=True) def _create_signed_list_blobs_url_helper( self, version, expiration=None, max_age=None, method="GET" @@ -795,10 +792,11 @@ def _create_signed_read_url_helper( expiration=None, max_age=None, ): - blob = self.bucket.blob(blob_name) if payload is not None: + blob = self.bucket.blob(blob_name) blob.upload_from_string(payload) - self.case_blobs_to_delete.append(blob) + else: + blob = self.blob if expiration is None and max_age is None: max_age = 10 @@ -816,7 +814,7 @@ def _create_signed_read_url_helper( if payload is not None: self.assertEqual(response.content, payload) else: - self.assertEqual(response.content, self.LOCAL_FILE) + self.assertEqual(response.content, self.BLOB_CONTENT) def test_create_signed_read_url_v2(self): self._create_signed_read_url_helper() @@ -861,7 +859,8 @@ def _create_signed_delete_url_helper( if expiration is None and max_age is None: max_age = 10 - blob = self.bucket.blob("LogoToSign.jpg") + blob = self.bucket.blob("DELETE_ME.txt") + blob.upload_from_string(b"DELETE ME!") signed_delete_url = blob.generate_signed_url( expiration=expiration, @@ -883,6 +882,68 @@ def test_create_signed_delete_url_v2(self): def test_create_signed_delete_url_v4(self): self._create_signed_delete_url_helper(version="v4") + def _signed_resumable_upload_url_helper( + self, + version="v2", + expiration=None, + max_age=None, + ): + blob = self.bucket.blob("cruddy.txt") + payload = b"DEADBEEF" + + if expiration is None and max_age is None: + max_age = 3600 + + # Initiate the upload using a signed URL. + signed_resumable_upload_url = blob.generate_signed_url( + expiration=expiration, + max_age=max_age, + method="RESUMABLE", + client=Config.CLIENT, + version=version, + ) + + post_headers = {"x-goog-resumable": "start"} + post_response = requests.post(signed_resumable_upload_url, headers=post_headers) + self.assertEqual(post_response.status_code, 201) + + # Finish uploading the body. + location = post_response.headers["Location"] + put_headers = {"content-length": str(len(payload))} + put_response = requests.put(location, headers=put_headers, data=payload) + self.assertEqual(put_response.status_code, 200) + + # Download using a signed URL and verify. + signed_download_url = blob.generate_signed_url( + expiration=expiration, + max_age=max_age, + method="GET", + client=Config.CLIENT, + version=version, + ) + + get_response = requests.get(signed_download_url) + self.assertEqual(get_response.status_code, 200) + self.assertEqual(get_response.content, payload) + + # Finally, delete the blob using a signed URL. + signed_delete_url = blob.generate_signed_url( + expiration=expiration, + max_age=max_age, + method="DELETE", + client=Config.CLIENT, + version=version, + ) + + delete_response = requests.delete(signed_delete_url) + self.assertEqual(delete_response.status_code, 204) + + def test_signed_resumable_upload_url_v2(self): + self._signed_resumable_upload_url_helper(version="v2") + + def test_signed_resumable_upload_url_v4(self): + self._signed_resumable_upload_url_helper(version="v4") + class TestStorageCompose(TestStorageFiles): From c37e77a7fc6ca21ee05bc47f1173e6c5fa0fddb0 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 17 Apr 2019 16:14:08 -0400 Subject: [PATCH 33/37] Remove 'max_age' argument for signing. --- storage/google/cloud/storage/_signing.py | 73 +++---------------- storage/google/cloud/storage/blob.py | 11 --- storage/google/cloud/storage/bucket.py | 11 --- storage/tests/system.py | 63 ++++++---------- storage/tests/unit/test__signing.py | 91 ++++++++---------------- storage/tests/unit/test_blob.py | 11 +-- storage/tests/unit/test_bucket.py | 11 +-- 7 files changed, 68 insertions(+), 203 deletions(-) diff --git a/storage/google/cloud/storage/_signing.py b/storage/google/cloud/storage/_signing.py index 8f9552641843..958b32ec12c5 100644 --- a/storage/google/cloud/storage/_signing.py +++ b/storage/google/cloud/storage/_signing.py @@ -86,34 +86,17 @@ def get_signed_query_params_v2(credentials, expiration, string_to_sign): } -def get_expiration_seconds_v2(expiration, max_age): +def get_expiration_seconds_v2(expiration): """Convert 'expiration' to a number of seconds in the future. :type expiration: Union[Integer, datetime.datetime, datetime.timedelta] :param expiration: Point in time when the signed URL should expire. - Exclusive with :arg:`max_age`: exactly one of the - two must be passed. - :type max_age: Integer - :param max_age: Max number of seconds until the signature expires. - Exclusive with :arg:`expiration`: exactly one of the - two must be passed. - - :raises: :exc:`ValueError` when both :arg:`expiration` and - :arg:`max_age` are passed, or when neither is passed. :raises: :exc:`TypeError` when expiration is not a valid type. :rtype: int :returns: a timestamp as an absolute number of seconds since epoch. """ - if (expiration is None and max_age is None) or ( - expiration is not None and max_age is not None - ): - raise ValueError("Pass exactly one of 'expiration' or 'max_age'.") - - if max_age is not None: - expiration = datetime.timedelta(seconds=max_age) - # If it's a timedelta, add it to `now` in UTC. if isinstance(expiration, datetime.timedelta): now = NOW().replace(tzinfo=_helpers.UTC) @@ -135,34 +118,17 @@ def get_expiration_seconds_v2(expiration, max_age): _EXPIRATION_TYPES = six.integer_types + (datetime.datetime, datetime.timedelta) -def get_expiration_seconds_v4(expiration, max_age): +def get_expiration_seconds_v4(expiration): """Convert 'expiration' to a number of seconds offset from the current time. :type expiration: Union[Integer, datetime.datetime, datetime.timedelta] :param expiration: Point in time when the signed URL should expire. - Exclusive with :arg:`max_age`: exactly one of the - two must be passed. - :type max_age: Integer - :param max_age: Max number of seconds until the signature expires. - Exclusive with :arg:`expiration`: exactly one of the - two must be passed. - - :raises: :exc:`ValueError` when both :arg:`expiration` and - :arg:`max_age` are passed, or when neither is passed. :raises: :exc:`TypeError` when expiration is not a valid type. :raises: :exc:`ValueError` when expiration is too large. :rtype: Integer :returns: seconds in the future when the signed URL will expire """ - if (expiration is None and max_age is None) or ( - expiration is not None and max_age is not None - ): - raise ValueError("Pass exactly one of 'expiration' or 'max_age'.") - - if max_age is not None: - return max_age - if not isinstance(expiration, _EXPIRATION_TYPES): raise TypeError( "Expected an integer timestamp, datetime, or " @@ -172,8 +138,7 @@ def get_expiration_seconds_v4(expiration, max_age): now = NOW().replace(tzinfo=_helpers.UTC) if isinstance(expiration, six.integer_types): - now_seconds = _helpers._microseconds_from_datetime(now) // 10 ** 6 - seconds = expiration - now_seconds + seconds = expiration if isinstance(expiration, datetime.datetime): @@ -226,9 +191,7 @@ def get_canonical_headers(headers): val = MULTIPLE_SPACES.sub(" ", val.strip()) normalized[key].append(val) - ordered_headers = sorted( - (key, ",".join(val)) for key, val in normalized.items() - ) + ordered_headers = sorted((key, ",".join(val)) for key, val in normalized.items()) canonical_headers = ["{}:{}".format(*item) for item in ordered_headers] return canonical_headers, ordered_headers @@ -292,8 +255,7 @@ def canonicalize(method, resource, query_parameters, headers): def generate_signed_url_v2( credentials, resource, - expiration=None, - max_age=None, + expiration, api_access_endpoint="", method="GET", content_md5=None, @@ -336,13 +298,6 @@ def generate_signed_url_v2( :type expiration: Union[Integer, datetime.datetime, datetime.timedelta] :param expiration: Point in time when the signed URL should expire. - Exclusive with :arg:`max_age`: exactly one of the - two must be passed. - - :type max_age: Integer - :param max_age: Max number of seconds until the signature expires. - Exclusive with :arg:`expiration`: exactly one of the - two must be passed. :type api_access_endpoint: str :param api_access_endpoint: Optional URI base. Defaults to empty string. @@ -392,8 +347,6 @@ def generate_signed_url_v2( https://cloud.google.com/storage/docs/xml-api/reference-headers#query :raises: :exc:`TypeError` when expiration is not a valid type. - :raises: :exc:`ValueError` when both :arg:`expiration` and - :arg:`max_age` are passed, or when neither is passed. :raises: :exc:`AttributeError` if credentials is not an instance of :class:`google.auth.credentials.Signing`. @@ -401,7 +354,7 @@ def generate_signed_url_v2( :returns: A signed URL you can use to access the resource until expiration. """ - expiration_stamp = get_expiration_seconds_v2(expiration, max_age) + expiration_stamp = get_expiration_seconds_v2(expiration) canonical = canonicalize(method, resource, query_parameters, headers) @@ -446,8 +399,7 @@ def generate_signed_url_v2( def generate_signed_url_v4( credentials, resource, - expiration=None, - max_age=None, + expiration, api_access_endpoint=DEFAULT_ENDPOINT, method="GET", content_md5=None, @@ -491,13 +443,6 @@ def generate_signed_url_v4( :type expiration: Union[Integer, datetime.datetime, datetime.timedelta] :param expiration: Point in time when the signed URL should expire. - Exclusive with :arg:`max_age`: exactly one of the - two must be passed. - - :type max_age: Integer - :param max_age: Max number of seconds until the signature expires. - Exclusive with :arg:`expiration`: exactly one of the - two must be passed. :type api_access_endpoint: str :param api_access_endpoint: Optional URI base. Defaults to @@ -548,8 +493,6 @@ def generate_signed_url_v4( https://cloud.google.com/storage/docs/xml-api/reference-headers#query :raises: :exc:`TypeError` when expiration is not a valid type. - :raises: :exc:`ValueError` when both :arg:`expiration` and - :arg:`max_age` are passed, or when neither is passed. :raises: :exc:`AttributeError` if credentials is not an instance of :class:`google.auth.credentials.Signing`. @@ -558,7 +501,7 @@ def generate_signed_url_v4( until expiration. """ ensure_signed_credentials(credentials) - expiration_seconds = get_expiration_seconds_v4(expiration, max_age) + expiration_seconds = get_expiration_seconds_v4(expiration) if _request_timestamp is None: now = NOW() diff --git a/storage/google/cloud/storage/blob.py b/storage/google/cloud/storage/blob.py index 7480ecced4ce..2e0afa6efb6b 100644 --- a/storage/google/cloud/storage/blob.py +++ b/storage/google/cloud/storage/blob.py @@ -310,7 +310,6 @@ def public_url(self): def generate_signed_url( self, expiration=None, - max_age=None, api_access_endpoint=_API_ACCESS_ENDPOINT, method="GET", content_md5=None, @@ -347,13 +346,6 @@ def generate_signed_url( :type expiration: Union[Integer, datetime.datetime, datetime.timedelta] :param expiration: Point in time when the signed URL should expire. - Exclusive with :arg:`max_age`: exactly one of the - two must be passed. - - :type max_age: Integer - :param max_age: Max number of seconds until the signature expires. - Exclusive with :arg:`expiration`: exactly one of the - two must be passed. :type api_access_endpoint: str :param api_access_endpoint: Optional URI base. @@ -417,8 +409,6 @@ def generate_signed_url( Must be one of 'v2' | 'v4'. :raises: :exc:`ValueError` when version is invalid. - :raises: :exc:`ValueError` when both :arg:`expiration` and - :arg:`max_age` are passed, or when neither is passed. :raises: :exc:`TypeError` when expiration is not a valid type. :raises: :exc:`AttributeError` if credentials is not an instance of :class:`google.auth.credentials.Signing`. @@ -450,7 +440,6 @@ def generate_signed_url( credentials, resource=resource, expiration=expiration, - max_age=max_age, api_access_endpoint=api_access_endpoint, method=method.upper(), content_md5=content_md5, diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 31bf1d732f38..5c6ae4346e69 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -1977,7 +1977,6 @@ def lock_retention_policy(self, client=None): def generate_signed_url( self, expiration=None, - max_age=None, api_access_endpoint=_API_ACCESS_ENDPOINT, method="GET", headers=None, @@ -2009,13 +2008,6 @@ def generate_signed_url( :type expiration: Union[Integer, datetime.datetime, datetime.timedelta] :param expiration: Point in time when the signed URL should expire. - Exclusive with :arg:`max_age`: exactly one of the - two must be passed. - - :type max_age: Integer - :param max_age: Max number of seconds until the signature expires. - Exclusive with :arg:`expiration`: exactly one of the - two must be passed. :type api_access_endpoint: str :param api_access_endpoint: Optional URI base. @@ -2054,8 +2046,6 @@ def generate_signed_url( Must be one of 'v2' | 'v4'. :raises: :exc:`ValueError` when version is invalid. - :raises: :exc:`ValueError` when both :arg:`expiration` and - :arg:`max_age` are passed, or when neither is passed. :raises: :exc:`TypeError` when expiration is not a valid type. :raises: :exc:`AttributeError` if credentials is not an instance of :class:`google.auth.credentials.Signing`. @@ -2085,7 +2075,6 @@ def generate_signed_url( credentials, resource=resource, expiration=expiration, - max_age=max_age, api_access_endpoint=api_access_endpoint, method=method.upper(), headers=headers, diff --git a/storage/tests/system.py b/storage/tests/system.py index d9f456020aab..d19b24615855 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -16,6 +16,7 @@ import os import re import tempfile +import time import unittest import pytest @@ -727,6 +728,7 @@ def test_third_level(self): class TestStorageSignURLs(unittest.TestCase): BLOB_CONTENT = b"This time for sure, Rocky!" + @classmethod def setUpClass(cls): if ( @@ -747,18 +749,23 @@ def tearDownClass(cls): retry = RetryErrors(errors, max_tries=6) retry(cls.bucket.delete)(force=True) + @staticmethod + def _morph_expiration(version, expiration): + if expiration is not None: + return expiration + + if version == "v2": + return int(time.time()) + 10 + + return 10 + def _create_signed_list_blobs_url_helper( - self, version, expiration=None, max_age=None, method="GET" + self, version, expiration=None, method="GET" ): - if expiration is None and max_age is None: - max_age = 10 + expiration = self._morph_expiration(version, expiration) signed_url = self.bucket.generate_signed_url( - expiration=expiration, - max_age=max_age, - method=method, - client=Config.CLIENT, - version=version, + expiration=expiration, method=method, client=Config.CLIENT, version=version ) response = requests.get(signed_url) @@ -790,23 +797,17 @@ def _create_signed_read_url_helper( version="v2", payload=None, expiration=None, - max_age=None, ): + expiration = self._morph_expiration(version, expiration) + if payload is not None: blob = self.bucket.blob(blob_name) blob.upload_from_string(payload) else: blob = self.blob - if expiration is None and max_age is None: - max_age = 10 - signed_url = blob.generate_signed_url( - expiration=expiration, - max_age=max_age, - method=method, - client=Config.CLIENT, - version=version, + expiration=expiration, method=method, client=Config.CLIENT, version=version ) response = requests.get(signed_url) @@ -852,19 +853,14 @@ def test_create_signed_read_url_v4_w_non_ascii_name(self): version="v4", ) - def _create_signed_delete_url_helper( - self, version="v2", expiration=None, max_age=None - ): - - if expiration is None and max_age is None: - max_age = 10 + def _create_signed_delete_url_helper(self, version="v2", expiration=None): + expiration = self._morph_expiration(version, expiration) blob = self.bucket.blob("DELETE_ME.txt") blob.upload_from_string(b"DELETE ME!") signed_delete_url = blob.generate_signed_url( expiration=expiration, - max_age=max_age, method="DELETE", client=Config.CLIENT, version=version, @@ -882,22 +878,14 @@ def test_create_signed_delete_url_v2(self): def test_create_signed_delete_url_v4(self): self._create_signed_delete_url_helper(version="v4") - def _signed_resumable_upload_url_helper( - self, - version="v2", - expiration=None, - max_age=None, - ): + def _signed_resumable_upload_url_helper(self, version="v2", expiration=None): + expiration = self._morph_expiration(version, expiration) blob = self.bucket.blob("cruddy.txt") payload = b"DEADBEEF" - if expiration is None and max_age is None: - max_age = 3600 - # Initiate the upload using a signed URL. signed_resumable_upload_url = blob.generate_signed_url( expiration=expiration, - max_age=max_age, method="RESUMABLE", client=Config.CLIENT, version=version, @@ -915,11 +903,7 @@ def _signed_resumable_upload_url_helper( # Download using a signed URL and verify. signed_download_url = blob.generate_signed_url( - expiration=expiration, - max_age=max_age, - method="GET", - client=Config.CLIENT, - version=version, + expiration=expiration, method="GET", client=Config.CLIENT, version=version ) get_response = requests.get(signed_download_url) @@ -929,7 +913,6 @@ def _signed_resumable_upload_url_helper( # Finally, delete the blob using a signed URL. signed_delete_url = blob.generate_signed_url( expiration=expiration, - max_age=max_age, method="DELETE", client=Config.CLIENT, version=version, diff --git a/storage/tests/unit/test__signing.py b/storage/tests/unit/test__signing.py index 7fd6d0bd3a3b..f2609d63e428 100644 --- a/storage/tests/unit/test__signing.py +++ b/storage/tests/unit/test__signing.py @@ -52,59 +52,39 @@ def _utc_seconds(when): class Test_get_expiration_seconds_v2(unittest.TestCase): @staticmethod - def _call_fut(expiration, max_age): + def _call_fut(expiration): from google.cloud.storage._signing import get_expiration_seconds_v2 - return get_expiration_seconds_v2(expiration, max_age) + return get_expiration_seconds_v2(expiration) def test_w_invalid_expiration_type(self): with self.assertRaises(TypeError): self._call_fut(object(), None) - def test_w_neither_expiration_nor_max_age(self): - with self.assertRaises(ValueError): - self._call_fut(None, None) - - def test_w_both_expiration_and_max_age(self): - with self.assertRaises(ValueError): - self._call_fut(123, 123) - - def test_w_max_age_int(self): - dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) - - patch = mock.patch( - "google.cloud.storage._signing.NOW", return_value=dummy_utcnow - ) - - with patch as utcnow: - result = self._call_fut(None, 10) - - delta = datetime.timedelta(seconds=10) - when = dummy_utcnow + delta - expiration = _utc_seconds(when) - self.assertEqual(result, expiration) - utcnow.assert_called_once_with() + def test_w_expiration_none(self): + with self.assertRaises(TypeError): + self._call_fut(None) def test_w_expiration_int(self): - self.assertEqual(self._call_fut(123, None), 123) + self.assertEqual(self._call_fut(123), 123) def test_w_expiration_long(self): if six.PY3: raise unittest.SkipTest("No long on Python 3") - self.assertEqual(self._call_fut(long(123), None), 123) # noqa: F821 + self.assertEqual(self._call_fut(long(123)), 123) # noqa: F821 def test_w_expiration_naive_datetime(self): expiration_no_tz = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) utc_seconds = _utc_seconds(expiration_no_tz) - self.assertEqual(self._call_fut(expiration_no_tz, None), utc_seconds) + self.assertEqual(self._call_fut(expiration_no_tz), utc_seconds) def test_w_expiration_utc_datetime(self): from google.cloud._helpers import UTC expiration_utc = datetime.datetime(2004, 8, 19, 0, 0, 0, 0, UTC) utc_seconds = _utc_seconds(expiration_utc) - self.assertEqual(self._call_fut(expiration_utc, None), utc_seconds) + self.assertEqual(self._call_fut(expiration_utc), utc_seconds) def test_w_expiration_other_zone_datetime(self): from google.cloud._helpers import _UTC @@ -117,7 +97,7 @@ class CET(_UTC): expiration_other = datetime.datetime(2004, 8, 19, 0, 0, 0, 0, zone) utc_seconds = _utc_seconds(expiration_other) cet_seconds = utc_seconds - (60 * 60) # CET one hour earlier than UTC - self.assertEqual(self._call_fut(expiration_other, None), cet_seconds) + self.assertEqual(self._call_fut(expiration_other), cet_seconds) def test_w_expiration_timedelta_seconds(self): dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) @@ -128,7 +108,7 @@ def test_w_expiration_timedelta_seconds(self): "google.cloud.storage._signing.NOW", return_value=dummy_utcnow ) with patch as utcnow: - result = self._call_fut(expiration_as_delta, None) + result = self._call_fut(expiration_as_delta) self.assertEqual(result, utc_seconds + 10) utcnow.assert_called_once_with() @@ -142,7 +122,7 @@ def test_w_expiration_timedelta_days(self): "google.cloud.storage._signing.NOW", return_value=dummy_utcnow ) with patch as utcnow: - result = self._call_fut(expiration_as_delta, None) + result = self._call_fut(expiration_as_delta) self.assertEqual(result, utc_seconds + 86400) utcnow.assert_called_once_with() @@ -150,25 +130,18 @@ def test_w_expiration_timedelta_days(self): class Test_get_expiration_seconds_v4(unittest.TestCase): @staticmethod - def _call_fut(expiration, max_age): + def _call_fut(expiration): from google.cloud.storage._signing import get_expiration_seconds_v4 - return get_expiration_seconds_v4(expiration, max_age) + return get_expiration_seconds_v4(expiration) def test_w_invalid_expiration_type(self): with self.assertRaises(TypeError): self._call_fut(object(), None) - def test_w_neither_expiration_nor_max_age(self): - with self.assertRaises(ValueError): - self._call_fut(None, None) - - def test_w_both_expiration_and_max_age(self): - with self.assertRaises(ValueError): - self._call_fut(123, 123) - - def test_w_max_age_int(self): - self.assertEqual(self._call_fut(None, 10), 10) + def test_w_expiration_none(self): + with self.assertRaises(TypeError): + self._call_fut(None) def test_w_expiration_int_gt_seven_days(self): dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) @@ -182,23 +155,21 @@ def test_w_expiration_int_gt_seven_days(self): with patch as utcnow: with self.assertRaises(ValueError): - self._call_fut(expiration_seconds, None) + self._call_fut(expiration_seconds) utcnow.assert_called_once_with() def test_w_expiration_int(self): dummy_utcnow = datetime.datetime(2004, 8, 19, 0, 0, 0, 0) - delta = datetime.timedelta(seconds=10) - expiration_utc = dummy_utcnow + delta - expiration_seconds = _utc_seconds(expiration_utc) + expiration_seconds = 10 patch = mock.patch( "google.cloud.storage._signing.NOW", return_value=dummy_utcnow ) with patch as utcnow: - result = self._call_fut(expiration_seconds, None) + result = self._call_fut(expiration_seconds) - self.assertEqual(result, delta.seconds) + self.assertEqual(result, expiration_seconds) utcnow.assert_called_once_with() def test_w_expiration_naive_datetime(self): @@ -210,7 +181,7 @@ def test_w_expiration_naive_datetime(self): "google.cloud.storage._signing.NOW", return_value=dummy_utcnow ) with patch as utcnow: - result = self._call_fut(expiration_no_tz, None) + result = self._call_fut(expiration_no_tz) self.assertEqual(result, delta.seconds) utcnow.assert_called_once_with() @@ -226,7 +197,7 @@ def test_w_expiration_utc_datetime(self): "google.cloud.storage._signing.NOW", return_value=dummy_utcnow ) with patch as utcnow: - result = self._call_fut(expiration_utc, None) + result = self._call_fut(expiration_utc) self.assertEqual(result, delta.seconds) utcnow.assert_called_once_with() @@ -249,7 +220,7 @@ class CET(_UTC): "google.cloud.storage._signing.NOW", return_value=dummy_utcnow ) with patch as utcnow: - result = self._call_fut(expiration_other, None) + result = self._call_fut(expiration_other) self.assertEqual(result, delta.seconds) utcnow.assert_called_once_with() @@ -262,7 +233,7 @@ def test_w_expiration_timedelta(self): "google.cloud.storage._signing.NOW", return_value=dummy_utcnow ) with patch as utcnow: - result = self._call_fut(expiration_as_delta, None) + result = self._call_fut(expiration_as_delta) self.assertEqual(result, expiration_as_delta.total_seconds()) utcnow.assert_called_once_with() @@ -535,6 +506,8 @@ def test_with_google_credentials(self): class Test_generate_signed_url_v4(unittest.TestCase): + DEFAULT_EXPIRATION = 1000 + @staticmethod def _call_fut(*args, **kwargs): from google.cloud.storage._signing import generate_signed_url_v4 @@ -543,8 +516,7 @@ def _call_fut(*args, **kwargs): def _generate_helper( self, - expiration=None, - max_age=1000, + expiration=DEFAULT_EXPIRATION, api_access_endpoint="", method="GET", content_type=None, @@ -566,7 +538,6 @@ def _generate_helper( credentials, resource, expiration=expiration, - max_age=max_age, api_access_endpoint=api_access_endpoint, method=method, content_type=content_type, @@ -603,7 +574,7 @@ def _generate_helper( now_stamp = now.strftime("%Y%m%dT%H%M%SZ") self.assertEqual(params["X-Goog-Date"], now_stamp) - self.assertEqual(params["X-Goog-Expires"], str(max_age)) + self.assertEqual(params["X-Goog-Expires"], str(self.DEFAULT_EXPIRATION)) signed = binascii.hexlify(credentials.sign_bytes.return_value).decode("ascii") self.assertEqual(params["X-Goog-Signature"], signed) @@ -624,7 +595,7 @@ def _generate_helper( value = value.strip() if value else "" self.assertEqual(params[key].lower(), value) - def test_w_max_age_too_long(self): + def test_w_expiration_too_long(self): with self.assertRaises(ValueError): self._generate_helper(expiration=datetime.timedelta(days=8)) @@ -690,7 +661,7 @@ def _run_conformance_test(resource, test_data): url = Test_generate_signed_url_v4._call_fut( credentials, resource, - max_age=test_data["expiration"], + expiration=test_data["expiration"], method=test_data["method"], _request_timestamp=test_data["timestamp"], headers=test_data.get("headers"), diff --git a/storage/tests/unit/test_blob.py b/storage/tests/unit/test_blob.py index db91c53931c3..6631863461f1 100644 --- a/storage/tests/unit/test_blob.py +++ b/storage/tests/unit/test_blob.py @@ -377,7 +377,6 @@ def _generate_signed_url_helper( query_parameters=None, credentials=None, expiration=None, - max_age=None, ): from six.moves.urllib import parse from google.cloud._helpers import UTC @@ -387,7 +386,7 @@ def _generate_signed_url_helper( delta = datetime.timedelta(hours=1) - if expiration is None and max_age is None: + if expiration is None: expiration = datetime.datetime.utcnow().replace(tzinfo=UTC) + delta connection = _Connection() @@ -401,13 +400,13 @@ def _generate_signed_url_helper( effective_version = version to_patch = "google.cloud.storage.blob.generate_signed_url_{}".format( - effective_version) + effective_version + ) with mock.patch(to_patch) as signer: with warnings.catch_warnings(record=True) as warned: signed_uri = blob.generate_signed_url( expiration=expiration, - max_age=max_age, api_access_endpoint=api_access_endpoint, method=method, credentials=credentials, @@ -439,7 +438,6 @@ def _generate_signed_url_helper( expected_kwargs = { "resource": expected_resource, "expiration": expiration, - "max_age": max_age, "api_access_endpoint": api_access_endpoint, "method": method.upper(), "content_md5": content_md5, @@ -468,9 +466,6 @@ def test_generate_signed_url_v2_w_expiration(self): expiration = datetime.datetime.utcnow().replace(tzinfo=UTC) self._generate_signed_url_v2_helper(expiration=expiration) - def test_generate_signed_url_v2_w_max_age(self): - self._generate_signed_url_v2_helper(max_age=3600) - def test_generate_signed_url_v2_w_non_ascii_name(self): BLOB_NAME = u"\u0410\u043a\u043a\u043e\u0440\u0434\u044b.txt" self._generate_signed_url_v2_helper(blob_name=BLOB_NAME) diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 6b7a035d8aa2..f00b79c8d6a2 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -2597,7 +2597,6 @@ def _generate_signed_url_helper( query_parameters=None, credentials=None, expiration=None, - max_age=None, ): from six.moves.urllib import parse from google.cloud._helpers import UTC @@ -2607,7 +2606,7 @@ def _generate_signed_url_helper( delta = datetime.timedelta(hours=1) - if expiration is None and max_age is None: + if expiration is None: expiration = datetime.datetime.utcnow().replace(tzinfo=UTC) + delta connection = _Connection() @@ -2620,13 +2619,13 @@ def _generate_signed_url_helper( effective_version = version to_patch = "google.cloud.storage.bucket.generate_signed_url_{}".format( - effective_version) + effective_version + ) with mock.patch(to_patch) as signer: with warnings.catch_warnings(record=True) as warned: signed_uri = bucket.generate_signed_url( expiration=expiration, - max_age=max_age, api_access_endpoint=api_access_endpoint, method=method, credentials=credentials, @@ -2653,7 +2652,6 @@ def _generate_signed_url_helper( expected_kwargs = { "resource": expected_resource, "expiration": expiration, - "max_age": max_age, "api_access_endpoint": api_access_endpoint, "method": method.upper(), "headers": headers, @@ -2677,9 +2675,6 @@ def test_generate_signed_url_v2_w_expiration(self): expiration = datetime.datetime.utcnow().replace(tzinfo=UTC) self._generate_signed_url_v2_helper(expiration=expiration) - def test_generate_signed_url_v2_w_max_age(self): - self._generate_signed_url_v2_helper(max_age=3600) - def test_generate_signed_url_v2_w_endpoint(self): self._generate_signed_url_v2_helper( api_access_endpoint="https://api.example.com/v1" From 2d04844ca8e75e81ed94d79480337c4c70763545 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 17 Apr 2019 16:38:07 -0400 Subject: [PATCH 34/37] Remove warning when no version is passed to 'generate_signed_url'. --- storage/google/cloud/storage/blob.py | 1 - storage/tests/unit/test_blob.py | 36 +++++++++++----------------- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/storage/google/cloud/storage/blob.py b/storage/google/cloud/storage/blob.py index 2e0afa6efb6b..0abe0cb3df31 100644 --- a/storage/google/cloud/storage/blob.py +++ b/storage/google/cloud/storage/blob.py @@ -419,7 +419,6 @@ def generate_signed_url( """ if version is None: version = "v2" - warnings.warn(DeprecationWarning(_SIGNED_URL_V2_DEFAULT_MESSAGE)) elif version not in ("v2", "v4"): raise ValueError("'version' must be either 'v2' or 'v4'") diff --git a/storage/tests/unit/test_blob.py b/storage/tests/unit/test_blob.py index 6631863461f1..aeb38b428f05 100644 --- a/storage/tests/unit/test_blob.py +++ b/storage/tests/unit/test_blob.py @@ -20,7 +20,6 @@ import os import tempfile import unittest -import warnings import google.cloud.storage.blob import mock @@ -404,27 +403,20 @@ def _generate_signed_url_helper( ) with mock.patch(to_patch) as signer: - with warnings.catch_warnings(record=True) as warned: - signed_uri = blob.generate_signed_url( - expiration=expiration, - api_access_endpoint=api_access_endpoint, - method=method, - credentials=credentials, - content_md5=content_md5, - content_type=content_type, - response_type=response_type, - response_disposition=response_disposition, - generation=generation, - headers=headers, - query_parameters=query_parameters, - version=version, - ) - - if version is None: - self.assertEqual(len(warned), 1) - self.assertIs(warned[0].category, DeprecationWarning) - else: - self.assertEqual(len(warned), 0) + signed_uri = blob.generate_signed_url( + expiration=expiration, + api_access_endpoint=api_access_endpoint, + method=method, + credentials=credentials, + content_md5=content_md5, + content_type=content_type, + response_type=response_type, + response_disposition=response_disposition, + generation=generation, + headers=headers, + query_parameters=query_parameters, + version=version, + ) self.assertEqual(signed_uri, signer.return_value) From ccccaed9a4e271e8f407751133483e7f224c1a71 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 17 Apr 2019 18:04:44 -0400 Subject: [PATCH 35/37] Remove warning message template. --- storage/google/cloud/storage/blob.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/storage/google/cloud/storage/blob.py b/storage/google/cloud/storage/blob.py index 0abe0cb3df31..5126cc16446b 100644 --- a/storage/google/cloud/storage/blob.py +++ b/storage/google/cloud/storage/blob.py @@ -97,12 +97,6 @@ _READ_LESS_THAN_SIZE = ( "Size {:d} was specified but the file-like object only had " "{:d} bytes remaining." ) -_SIGNED_URL_V2_DEFAULT_MESSAGE = ( - "You have generated a signed URL using the default v2 signing " - "implementation. In the future, this will default to v4. " - "You may experience breaking changes if you use longer than 7 day " - "expiration times with v4. To opt-in to the behavior specify version='v2'." -) _DEFAULT_CHUNKSIZE = 104857600 # 1024 * 1024 B * 100 = 100 MB _MAX_MULTIPART_SIZE = 8388608 # 8 MB From 3a583a4483522dab7af518517f2343d252e5f19b Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 17 Apr 2019 18:16:05 -0400 Subject: [PATCH 36/37] Remove no-version warning for 'Bucket.generate_signed_url', too. --- storage/google/cloud/storage/bucket.py | 2 -- storage/tests/unit/test_bucket.py | 26 +++++++++----------------- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 5c6ae4346e69..0eb6a754ff5e 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -37,7 +37,6 @@ from google.cloud.storage._signing import generate_signed_url_v4 from google.cloud.storage.acl import BucketACL from google.cloud.storage.acl import DefaultObjectACL -from google.cloud.storage.blob import _SIGNED_URL_V2_DEFAULT_MESSAGE from google.cloud.storage.blob import Blob from google.cloud.storage.notification import BucketNotification from google.cloud.storage.notification import NONE_PAYLOAD_FORMAT @@ -2056,7 +2055,6 @@ def generate_signed_url( """ if version is None: version = "v2" - warnings.warn(DeprecationWarning(_SIGNED_URL_V2_DEFAULT_MESSAGE)) elif version not in ("v2", "v4"): raise ValueError("'version' must be either 'v2' or 'v4'") diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index f00b79c8d6a2..31b7d293010e 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -14,7 +14,6 @@ import datetime import unittest -import warnings import mock @@ -2623,22 +2622,15 @@ def _generate_signed_url_helper( ) with mock.patch(to_patch) as signer: - with warnings.catch_warnings(record=True) as warned: - signed_uri = bucket.generate_signed_url( - expiration=expiration, - api_access_endpoint=api_access_endpoint, - method=method, - credentials=credentials, - headers=headers, - query_parameters=query_parameters, - version=version, - ) - - if version is None: - self.assertEqual(len(warned), 1) - self.assertIs(warned[0].category, DeprecationWarning) - else: - self.assertEqual(len(warned), 0) + signed_uri = bucket.generate_signed_url( + expiration=expiration, + api_access_endpoint=api_access_endpoint, + method=method, + credentials=credentials, + headers=headers, + query_parameters=query_parameters, + version=version, + ) self.assertEqual(signed_uri, signer.return_value) From 8f2e13f55320f48252f764616b65da3012414ae3 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 17 Apr 2019 18:19:19 -0400 Subject: [PATCH 37/37] Un-skip systests blocked on back-end case-flattening bug (now fixed). --- storage/tests/system.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/storage/tests/system.py b/storage/tests/system.py index d19b24615855..79a574f3029d 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -780,11 +780,9 @@ def test_create_signed_list_blobs_url_v2_w_expiration(self): self._create_signed_list_blobs_url_helper(expiration=now + delta, version="v2") - @pytest.mark.skip(reason="Back-end case-flattening bug: revisit 2019-04-03") def test_create_signed_list_blobs_url_v4(self): self._create_signed_list_blobs_url_helper(version="v4") - @pytest.mark.skip(reason="Back-end case-flattening bug: revisit 2019-04-03") def test_create_signed_list_blobs_url_v4_w_expiration(self): now = datetime.datetime.utcnow() delta = datetime.timedelta(seconds=10)