From f698394cf131c0ced532d683b37208b02fabc3ce Mon Sep 17 00:00:00 2001 From: Guillaume Blaquiere Date: Mon, 4 Jan 2021 22:56:29 +0100 Subject: [PATCH 1/5] Fix: signed_url_v4 to accept credentials without private key. Preserve checks for efficiency in case of neither access_token nor service_account_email are provided. Fix: tests v4 with token to take into account not Signing credential class. --- google/cloud/storage/_signing.py | 7 +++++-- tests/unit/test__signing.py | 7 +++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/google/cloud/storage/_signing.py b/google/cloud/storage/_signing.py index 1382ebc77..2f5fb1805 100644 --- a/google/cloud/storage/_signing.py +++ b/google/cloud/storage/_signing.py @@ -533,7 +533,6 @@ def generate_signed_url_v4( :returns: A signed URL you can use to access the resource until expiration. """ - ensure_signed_credentials(credentials) expiration_seconds = get_expiration_seconds_v4(expiration) if _request_timestamp is None: @@ -542,7 +541,11 @@ def generate_signed_url_v4( request_timestamp = _request_timestamp datestamp = _request_timestamp[:8] - client_email = credentials.signer_email + client_email = service_account_email + if not access_token or not service_account_email: + ensure_signed_credentials(credentials) + client_email = credentials.signer_email + credential_scope = "{}/auto/storage/goog4_request".format(datestamp) credential = "{}/{}".format(client_email, credential_scope) diff --git a/tests/unit/test__signing.py b/tests/unit/test__signing.py index 1a3c383de..5d89eaf50 100644 --- a/tests/unit/test__signing.py +++ b/tests/unit/test__signing.py @@ -655,8 +655,8 @@ def test_w_custom_query_parameters_w_none_value(self): def test_with_access_token(self): resource = "/name/path" - signer_email = "service@example.com" - credentials = _make_credentials(signer_email=signer_email) + credentials = _make_credentials() + email = mock.sentinel.service_account_email with mock.patch( "google.cloud.storage._signing._sign_message", return_value=b"DEADBEEF" ): @@ -664,11 +664,10 @@ def test_with_access_token(self): credentials, resource=resource, expiration=datetime.timedelta(days=5), - service_account_email=signer_email, + service_account_email=email, access_token="token", ) - class Test_sign_message(unittest.TestCase): @staticmethod def _call_fut(*args, **kwargs): From a3a3b544d6e5491c2d225a09e36bca7a122e3f35 Mon Sep 17 00:00:00 2001 From: Guillaume Blaquiere Date: Mon, 4 Jan 2021 22:58:26 +0100 Subject: [PATCH 2/5] fix typo: 2 new lines before new test class --- tests/unit/test__signing.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test__signing.py b/tests/unit/test__signing.py index 5d89eaf50..bcfddfa45 100644 --- a/tests/unit/test__signing.py +++ b/tests/unit/test__signing.py @@ -668,6 +668,7 @@ def test_with_access_token(self): access_token="token", ) + class Test_sign_message(unittest.TestCase): @staticmethod def _call_fut(*args, **kwargs): From 127787d20db9ed44c48e5872be1889c277ddc129 Mon Sep 17 00:00:00 2001 From: Guillaume Blaquiere Date: Thu, 28 Jan 2021 23:29:42 +0100 Subject: [PATCH 3/5] fix doc: Improve docstring to explain the use of the access_token AND the service_account_email, or the signer email. --- google/cloud/storage/_signing.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/google/cloud/storage/_signing.py b/google/cloud/storage/_signing.py index 2f5fb1805..4a5748a01 100644 --- a/google/cloud/storage/_signing.py +++ b/google/cloud/storage/_signing.py @@ -457,9 +457,12 @@ def generate_signed_url_v4( 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. + sign text. That credentials must provide signer_email + only if service_account_email and access_token are not + passed. :type resource: str :param resource: A pointer to a specific resource From 53aa9afb4d249d8e56d7e1e6c0bf2846a9ae734e Mon Sep 17 00:00:00 2001 From: Guillaume Blaquiere Date: Thu, 28 Jan 2021 23:31:31 +0100 Subject: [PATCH 4/5] fix: test coverage with the new IF branch --- tests/unit/test__signing.py | 53 ++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/tests/unit/test__signing.py b/tests/unit/test__signing.py index bcfddfa45..a2eb1db7c 100644 --- a/tests/unit/test__signing.py +++ b/tests/unit/test__signing.py @@ -653,7 +653,7 @@ def test_w_custom_query_parameters_w_string_value(self): def test_w_custom_query_parameters_w_none_value(self): self._generate_helper(query_parameters={"qux": None}) - def test_with_access_token(self): + def test_with_access_token_and_service_account_email(self): resource = "/name/path" credentials = _make_credentials() email = mock.sentinel.service_account_email @@ -668,6 +668,57 @@ def test_with_access_token(self): access_token="token", ) + def test_with_access_token_and_service_account_email_and_signer_email(self): + resource = "/name/path" + signer_email = "service@example.com" + credentials = _make_credentials(signer_email=signer_email) + with mock.patch( + "google.cloud.storage._signing._sign_message", return_value=b"DEADBEEF" + ): + self._call_fut( + credentials, + resource=resource, + expiration=datetime.timedelta(days=5), + service_account_email=signer_email, + access_token="token", + ) + + def test_with_signer_email(self): + resource = "/name/path" + signer_email = "service@example.com" + credentials = _make_credentials(signer_email=signer_email) + credentials.sign_bytes.return_value = b"DEADBEEF" + self._call_fut( + credentials, + resource=resource, + expiration=datetime.timedelta(days=5), + ) + + def test_with_service_account_email_and_signer_email(self): + resource = "/name/path" + signer_email = "service@example.com" + credentials = _make_credentials(signer_email=signer_email) + credentials.sign_bytes.return_value = b"DEADBEEF" + self._call_fut( + credentials, + resource=resource, + expiration=datetime.timedelta(days=5), + service_account_email=signer_email, + ) + + def test_with_token_and_signer_email(self): + resource = "/name/path" + signer_email = "service@example.com" + credentials = _make_credentials(signer_email=signer_email) + credentials.sign_bytes.return_value = b"DEADBEEF" + self._call_fut( + credentials, + resource=resource, + expiration=datetime.timedelta(days=5), + access_token="token", + ) + + class Test_sign_message(unittest.TestCase): @staticmethod From 28f2b1067b379eab7135501a774551d50c86e169 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Thu, 18 Feb 2021 13:20:37 -0800 Subject: [PATCH 5/5] lint --- tests/unit/test__signing.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/unit/test__signing.py b/tests/unit/test__signing.py index b21cda108..3eac70cc1 100644 --- a/tests/unit/test__signing.py +++ b/tests/unit/test__signing.py @@ -673,7 +673,7 @@ def test_with_access_token_and_service_account_email_and_signer_email(self): signer_email = "service@example.com" credentials = _make_credentials(signer_email=signer_email) with mock.patch( - "google.cloud.storage._signing._sign_message", return_value=b"DEADBEEF" + "google.cloud.storage._signing._sign_message", return_value=b"DEADBEEF" ): self._call_fut( credentials, @@ -689,9 +689,7 @@ def test_with_signer_email(self): credentials = _make_credentials(signer_email=signer_email) credentials.sign_bytes.return_value = b"DEADBEEF" self._call_fut( - credentials, - resource=resource, - expiration=datetime.timedelta(days=5), + credentials, resource=resource, expiration=datetime.timedelta(days=5), ) def test_with_service_account_email_and_signer_email(self): @@ -719,7 +717,6 @@ def test_with_token_and_signer_email(self): ) - class Test_sign_message(unittest.TestCase): @staticmethod def _call_fut(*args, **kwargs):