diff --git a/api_core/google/api_core/iam.py b/api_core/google/api_core/iam.py index a7a1c00e236d..f1309360304c 100644 --- a/api_core/google/api_core/iam.py +++ b/api_core/google/api_core/iam.py @@ -210,10 +210,10 @@ def bindings(self): policy.version = 3 policy.bindings = [ - { - "role": "roles/viewer", - "members": {USER, ADMIN_GROUP, SERVICE_ACCOUNT}, - "condition": CONDITION + { + "role": "roles/viewer", + "members": {USER, ADMIN_GROUP, SERVICE_ACCOUNT}, + "condition": CONDITION }, ... ] diff --git a/storage/google/cloud/storage/blob.py b/storage/google/cloud/storage/blob.py index bc97120e8790..68832b718e8f 100644 --- a/storage/google/cloud/storage/blob.py +++ b/storage/google/cloud/storage/blob.py @@ -1454,7 +1454,7 @@ def create_resumable_upload_session( except resumable_media.InvalidResponse as exc: _raise_from_invalid_response(exc) - def get_iam_policy(self, client=None): + def get_iam_policy(self, client=None, requested_policy_version=None): """Retrieve the IAM policy for the object. .. note: @@ -1473,6 +1473,18 @@ def get_iam_policy(self, client=None): :param client: Optional. The client to use. If not passed, falls back to the ``client`` stored on the current object's bucket. + :type requested_policy_version: int or ``NoneType`` + :param requested_policy_version: Optional. The version of IAM policies to request. + If a policy with a condition is requested without + setting this, the server will return an error. + This must be set to a value of 3 to retrieve IAM + policies containing conditions. This is to prevent + client code that isn't aware of IAM conditions from + interpreting and modifying policies incorrectly. + The service might return a policy with version lower + than the one that was requested, based on the + feature syntax in the policy fetched. + :rtype: :class:`google.api_core.iam.Policy` :returns: the policy instance, based on the resource returned from the ``getIamPolicy`` API request. @@ -1484,6 +1496,9 @@ def get_iam_policy(self, client=None): if self.user_project is not None: query_params["userProject"] = self.user_project + if requested_policy_version is not None: + query_params["optionsRequestedPolicyVersion"] = requested_policy_version + info = client._connection.api_request( method="GET", path="%s/iam" % (self.path,), diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index cb2adf106aef..ed275e88fcec 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -1865,7 +1865,7 @@ def disable_website(self): """ return self.configure_website(None, None) - def get_iam_policy(self, client=None): + def get_iam_policy(self, client=None, requested_policy_version=None): """Retrieve the IAM policy for the bucket. See @@ -1878,9 +1878,45 @@ def get_iam_policy(self, client=None): :param client: Optional. The client to use. If not passed, falls back to the ``client`` stored on the current bucket. + :type requested_policy_version: int or ``NoneType`` + :param requested_policy_version: Optional. The version of IAM policies to request. + If a policy with a condition is requested without + setting this, the server will return an error. + This must be set to a value of 3 to retrieve IAM + policies containing conditions. This is to prevent + client code that isn't aware of IAM conditions from + interpreting and modifying policies incorrectly. + The service might return a policy with version lower + than the one that was requested, based on the + feature syntax in the policy fetched. + :rtype: :class:`google.api_core.iam.Policy` :returns: the policy instance, based on the resource returned from the ``getIamPolicy`` API request. + + Example: + + .. code-block:: python + + from google.cloud.storage.iam import STORAGE_OBJECT_VIEWER_ROLE + + policy = bucket.get_iam_policy(requested_policy_version=3) + + policy.version = 3 + + # Add a binding to the policy via it's bindings property + policy.bindings.append({ + "role": STORAGE_OBJECT_VIEWER_ROLE, + "members": {"serviceAccount:account@project.iam.gserviceaccount.com", ...}, + # Optional: + "condition": { + "title": "prefix" + "description": "Objects matching prefix" + "expression": "resource.name.startsWith(\"projects/project-name/buckets/bucket-name/objects/prefix\")" + } + }) + + bucket.set_iam_policy(policy) """ client = self._require_client(client) query_params = {} @@ -1888,6 +1924,9 @@ def get_iam_policy(self, client=None): if self.user_project is not None: query_params["userProject"] = self.user_project + if requested_policy_version is not None: + query_params["optionsRequestedPolicyVersion"] = requested_policy_version + info = client._connection.api_request( method="GET", path="%s/iam" % (self.path,), diff --git a/storage/tests/system.py b/storage/tests/system.py index ea15990f7290..d689c2f2c6d2 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -262,6 +262,66 @@ def test_bucket_update_labels(self): bucket.update() self.assertEqual(bucket.labels, {}) + def test_get_set_iam_policy(self): + import pytest + from google.cloud.storage.iam import STORAGE_OBJECT_VIEWER_ROLE + from google.api_core.exceptions import BadRequest, PreconditionFailed + + bucket_name = "iam-policy" + unique_resource_id("-") + bucket = retry_429_503(Config.CLIENT.create_bucket)(bucket_name) + self.case_buckets_to_delete.append(bucket_name) + self.assertTrue(bucket.exists()) + + policy_no_version = bucket.get_iam_policy() + self.assertEqual(policy_no_version.version, 1) + + policy = bucket.get_iam_policy(requested_policy_version=3) + self.assertEqual(policy, policy_no_version) + + member = "serviceAccount:{}".format(Config.CLIENT.get_service_account_email()) + + BINDING_W_CONDITION = { + "role": STORAGE_OBJECT_VIEWER_ROLE, + "members": {member}, + "condition": { + "title": "always-true", + "description": "test condition always-true", + "expression": "true", + }, + } + policy.bindings.append(BINDING_W_CONDITION) + + with pytest.raises( + PreconditionFailed, match="enable uniform bucket-level access" + ): + bucket.set_iam_policy(policy) + + bucket.iam_configuration.uniform_bucket_level_access_enabled = True + bucket.patch() + + policy = bucket.get_iam_policy(requested_policy_version=3) + policy.bindings.append(BINDING_W_CONDITION) + + with pytest.raises(BadRequest, match="at least 3"): + bucket.set_iam_policy(policy) + + policy.version = 3 + returned_policy = bucket.set_iam_policy(policy) + self.assertEqual(returned_policy.version, 3) + self.assertEqual(returned_policy.bindings, policy.bindings) + + with pytest.raises( + BadRequest, match="cannot be less than the existing policy version" + ): + bucket.get_iam_policy() + with pytest.raises( + BadRequest, match="cannot be less than the existing policy version" + ): + bucket.get_iam_policy(requested_policy_version=2) + + fetched_policy = bucket.get_iam_policy(requested_policy_version=3) + self.assertEqual(fetched_policy.bindings, returned_policy.bindings) + @unittest.skipUnless(USER_PROJECT, "USER_PROJECT not set in environment.") def test_crud_bucket_with_requester_pays(self): new_bucket_name = "w-requester-pays" + unique_resource_id("-") diff --git a/storage/tests/unit/test_blob.py b/storage/tests/unit/test_blob.py index 2609d546ef49..aa3819545920 100644 --- a/storage/tests/unit/test_blob.py +++ b/storage/tests/unit/test_blob.py @@ -1928,7 +1928,7 @@ def test_get_iam_policy(self): BLOB_NAME = "blob-name" PATH = "/b/name/o/%s" % (BLOB_NAME,) ETAG = "DEADBEEF" - VERSION = 17 + VERSION = 1 OWNER1 = "user:phred@example.com" OWNER2 = "group:cloud-logs@google.com" EDITOR1 = "domain:google.com" @@ -1973,6 +1973,41 @@ def test_get_iam_policy(self): }, ) + def test_get_iam_policy_w_requested_policy_version(self): + from google.cloud.storage.iam import STORAGE_OWNER_ROLE + + BLOB_NAME = "blob-name" + PATH = "/b/name/o/%s" % (BLOB_NAME,) + ETAG = "DEADBEEF" + VERSION = 1 + OWNER1 = "user:phred@example.com" + OWNER2 = "group:cloud-logs@google.com" + RETURNED = { + "resourceId": PATH, + "etag": ETAG, + "version": VERSION, + "bindings": [{"role": STORAGE_OWNER_ROLE, "members": [OWNER1, OWNER2]}], + } + after = ({"status": http_client.OK}, RETURNED) + connection = _Connection(after) + client = _Client(connection) + bucket = _Bucket(client=client) + blob = self._make_one(BLOB_NAME, bucket=bucket) + + blob.get_iam_policy(requested_policy_version=3) + + kw = connection._requested + self.assertEqual(len(kw), 1) + self.assertEqual( + kw[0], + { + "method": "GET", + "path": "%s/iam" % (PATH,), + "query_params": {"optionsRequestedPolicyVersion": 3}, + "_target_object": None, + }, + ) + def test_get_iam_policy_w_user_project(self): from google.api_core.iam import Policy @@ -1980,7 +2015,7 @@ def test_get_iam_policy_w_user_project(self): USER_PROJECT = "user-project-123" PATH = "/b/name/o/%s" % (BLOB_NAME,) ETAG = "DEADBEEF" - VERSION = 17 + VERSION = 1 RETURNED = { "resourceId": PATH, "etag": ETAG, @@ -2023,7 +2058,7 @@ def test_set_iam_policy(self): BLOB_NAME = "blob-name" PATH = "/b/name/o/%s" % (BLOB_NAME,) ETAG = "DEADBEEF" - VERSION = 17 + VERSION = 1 OWNER1 = "user:phred@example.com" OWNER2 = "group:cloud-logs@google.com" EDITOR1 = "domain:google.com" @@ -2074,7 +2109,7 @@ def test_set_iam_policy_w_user_project(self): USER_PROJECT = "user-project-123" PATH = "/b/name/o/%s" % (BLOB_NAME,) ETAG = "DEADBEEF" - VERSION = 17 + VERSION = 1 BINDINGS = [] RETURNED = {"etag": ETAG, "version": VERSION, "bindings": BINDINGS} after = ({"status": http_client.OK}, RETURNED) diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 41918874b4b6..68399b3c8962 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -2023,7 +2023,7 @@ def test_get_iam_policy(self): NAME = "name" PATH = "/b/%s" % (NAME,) ETAG = "DEADBEEF" - VERSION = 17 + VERSION = 1 OWNER1 = "user:phred@example.com" OWNER2 = "group:cloud-logs@google.com" EDITOR1 = "domain:google.com" @@ -2067,7 +2067,7 @@ def test_get_iam_policy_w_user_project(self): USER_PROJECT = "user-project-123" PATH = "/b/%s" % (NAME,) ETAG = "DEADBEEF" - VERSION = 17 + VERSION = 1 RETURNED = { "resourceId": PATH, "etag": ETAG, @@ -2092,6 +2092,35 @@ def test_get_iam_policy_w_user_project(self): self.assertEqual(kw[0]["path"], "%s/iam" % (PATH,)) self.assertEqual(kw[0]["query_params"], {"userProject": USER_PROJECT}) + def test_get_iam_policy_w_requested_policy_version(self): + from google.cloud.storage.iam import STORAGE_OWNER_ROLE + + NAME = "name" + PATH = "/b/%s" % (NAME,) + ETAG = "DEADBEEF" + VERSION = 1 + OWNER1 = "user:phred@example.com" + OWNER2 = "group:cloud-logs@google.com" + RETURNED = { + "resourceId": PATH, + "etag": ETAG, + "version": VERSION, + "bindings": [{"role": STORAGE_OWNER_ROLE, "members": [OWNER1, OWNER2]}], + } + connection = _Connection(RETURNED) + client = _Client(connection, None) + bucket = self._make_one(client=client, name=NAME) + + policy = bucket.get_iam_policy(requested_policy_version=3) + + self.assertEqual(policy.version, VERSION) + + kw = connection._requested + self.assertEqual(len(kw), 1) + self.assertEqual(kw[0]["method"], "GET") + self.assertEqual(kw[0]["path"], "%s/iam" % (PATH,)) + self.assertEqual(kw[0]["query_params"], {"optionsRequestedPolicyVersion": 3}) + def test_set_iam_policy(self): import operator from google.cloud.storage.iam import STORAGE_OWNER_ROLE @@ -2102,7 +2131,7 @@ def test_set_iam_policy(self): NAME = "name" PATH = "/b/%s" % (NAME,) ETAG = "DEADBEEF" - VERSION = 17 + VERSION = 1 OWNER1 = "user:phred@example.com" OWNER2 = "group:cloud-logs@google.com" EDITOR1 = "domain:google.com" @@ -2155,7 +2184,7 @@ def test_set_iam_policy_w_user_project(self): USER_PROJECT = "user-project-123" PATH = "/b/%s" % (NAME,) ETAG = "DEADBEEF" - VERSION = 17 + VERSION = 1 OWNER1 = "user:phred@example.com" OWNER2 = "group:cloud-logs@google.com" EDITOR1 = "domain:google.com"