From b94ae566317dc41761e5e2522a4a664e08f47e50 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Fri, 26 Jul 2019 14:36:40 -0700 Subject: [PATCH 1/5] Add support for uniform bucket-level access. --- storage/google/cloud/storage/bucket.py | 47 +++++++++++++++++------- storage/tests/system.py | 13 +++---- storage/tests/unit/test_bucket.py | 50 +++++++++++++------------- 3 files changed, 67 insertions(+), 43 deletions(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 3c3afbe97af7..8549e6cac30c 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -280,6 +280,12 @@ class IAMConfiguration(dict): :type bucket: :class:`Bucket` :params bucket: Bucket for which this instance is the policy. + :type uniform_bucket_level_access_enabled: bool + :params bucket_policy_only_enabled: (optional) whether the IAM-only policy is enabled for the bucket. + + :type uniform_bucket_level_locked_time: :class:`datetime.datetime` + :params uniform_bucket_level_locked_time: (optional) When the bucket's IAM-only policy was ehabled. This value should normally only be set by the back-end API. + :type bucket_policy_only_enabled: bool :params bucket_policy_only_enabled: (optional) whether the IAM-only policy is enabled for the bucket. @@ -292,10 +298,13 @@ def __init__( bucket, bucket_policy_only_enabled=False, bucket_policy_only_locked_time=None, + uniform_bucket_level_access_enabled=False, + uniform_bucket_level_access_locked_time=None, ): - data = {"bucketPolicyOnly": {"enabled": bucket_policy_only_enabled}} - if bucket_policy_only_locked_time is not None: - data["bucketPolicyOnly"]["lockedTime"] = _datetime_to_rfc3339( + # Which one wins??? + data = {"uniformBucketLevelAccess": {"enabled": bucket_policy_only_enabled}} + if uniform_bucket_level_access_locked_time is not None: + data["uniformBucketLevelAccess"]["lockedTime"] = _datetime_to_rfc3339( bucket_policy_only_locked_time ) super(IAMConfiguration, self).__init__(data) @@ -329,35 +338,49 @@ def bucket(self): @property def bucket_policy_only_enabled(self): + """Alias for uniform_bucket_level_access_enabled""" + self.uniform_bucket_level_access_enabled() + + @property + def uniform_bucket_level_access_enabled(self): """If set, access checks only use bucket-level IAM policies or above. :rtype: bool :returns: whether the bucket is configured to allow only IAM. """ - bpo = self.get("bucketPolicyOnly", {}) + bpo = self.get("uniformBucketLevelAccess", {}) return bpo.get("enabled", False) + @uniform_bucket_level_access_enabled.setter + def uniform_bucket_level_access_enabled(self, value): + ubla = self.setdefault("uniformBucketLevelAccess", {}) + ubla["enabled"] = bool(value) + self.bucket._patch_property("iamConfiguration", self) + @bucket_policy_only_enabled.setter def bucket_policy_only_enabled(self, value): - bpo = self.setdefault("bucketPolicyOnly", {}) - bpo["enabled"] = bool(value) - self.bucket._patch_property("iamConfiguration", self) + self.uniform_bucket_level_access_enabled(value) @property def bucket_policy_only_locked_time(self): - """Deadline for changing :attr:`bucket_policy_only_enabled` from true to false. + """Alias for uniform_bucket_level_access.""" + return self.uniform_bucket_level_access_locked_time() + + @property + def uniform_bucket_level_access_locked_time(self): + """Deadline for changing :attr:`uniform_bucket_level_access_enabled` from true to false. - If the bucket's :attr:`bucket_policy_only_enabled` is true, this property + If the bucket's :attr:`uniform_bucket_level_access_enabled` is true, this property is time time after which that setting becomes immutable. - If the bucket's :attr:`bucket_policy_only_enabled` is false, this property + If the bucket's :attr:`uniform_bucket_level_access_enabled` is false, this property is ``None``. :rtype: Union[:class:`datetime.datetime`, None] - :returns: (readonly) Time after which :attr:`bucket_policy_only_enabled` will + :returns: (readonly) Time after which :attr:`uniform_bucket_level_access_enabled` will be frozen as true. """ - bpo = self.get("bucketPolicyOnly", {}) + ubla = self.get("uniformBucketLevelAccess", {}) stamp = bpo.get("lockedTime") if stamp is not None: stamp = _rfc3339_to_datetime(stamp) diff --git a/storage/tests/system.py b/storage/tests/system.py index e2784fd5155a..6485fef3a6bb 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -1647,13 +1647,13 @@ def tearDown(self): bucket = Config.CLIENT.bucket(bucket_name) retry_429_harder(bucket.delete)(force=True) - def test_new_bucket_w_bpo(self): - new_bucket_name = "new-w-bpo" + unique_resource_id("-") + def test_new_bucket_w_ubla(self): + new_bucket_name = "new-w-ubla" + unique_resource_id("-") self.assertRaises( exceptions.NotFound, Config.CLIENT.get_bucket, new_bucket_name ) bucket = Config.CLIENT.bucket(new_bucket_name) - bucket.iam_configuration.bucket_policy_only_enabled = True + bucket.iam_configuration.uniform_bucket_level_access_enabled = True retry_429(bucket.create)() self.case_buckets_to_delete.append(new_bucket_name) @@ -1702,17 +1702,17 @@ def test_bpo_set_unset_preserves_acls(self): blob_acl_before = list(bucket.acl) # Set BPO - bucket.iam_configuration.bucket_policy_only_enabled = True + bucket.iam_configuration.uniform_bucket_level_access_enabled = True bucket.patch() - self.assertTrue(bucket.iam_configuration.bucket_policy_only_enabled) + self.assertTrue(bucket.iam_configuration.uniform_bucket_level_access_enabled) # While BPO is set, cannot get / set ACLs with self.assertRaises(exceptions.BadRequest): bucket.acl.reload() # Clear BPO - bucket.iam_configuration.bucket_policy_only_enabled = False + bucket.iam_configuration.uniform_bucket_level_access_enabled = False bucket.patch() # Query ACLs after clearing BPO @@ -1723,3 +1723,4 @@ def test_bpo_set_unset_preserves_acls(self): self.assertEqual(bucket_acl_before, bucket_acl_after) self.assertEqual(blob_acl_before, blob_acl_after) + diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 9ac4995525cf..1ecd3fde776c 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -197,8 +197,8 @@ def test_ctor_defaults(self): config = self._make_one(bucket) self.assertIs(config.bucket, bucket) - self.assertFalse(config.bucket_policy_only_enabled) - self.assertIsNone(config.bucket_policy_only_locked_time) + self.assertFalse(config.uniform_bucket_level_access_enabled) + self.assertIsNone(config.uniform_bucket_level_access_locked_time) def test_ctor_explicit(self): import datetime @@ -208,12 +208,12 @@ def test_ctor_explicit(self): now = datetime.datetime.utcnow().replace(tzinfo=pytz.UTC) config = self._make_one( - bucket, bucket_policy_only_enabled=True, bucket_policy_only_locked_time=now + bucket, uniform_bucket_level_access_enabled=True, uniform_bucket_level_access_locked_time=now ) self.assertIs(config.bucket, bucket) - self.assertTrue(config.bucket_policy_only_enabled) - self.assertEqual(config.bucket_policy_only_locked_time, now) + self.assertTrue(config.uniform_bucket_level_access_enabled) + self.assertEqual(config.uniform_bucket_level_access_locked_time, now) def test_from_api_repr_w_empty_resource(self): klass = self._get_target_class() @@ -223,30 +223,30 @@ def test_from_api_repr_w_empty_resource(self): config = klass.from_api_repr(resource, bucket) self.assertIs(config.bucket, bucket) - self.assertFalse(config.bucket_policy_only_enabled) - self.assertIsNone(config.bucket_policy_only_locked_time) + self.assertFalse(config.uniform_bucket_level_access_enabled) + self.assertIsNone(config.uniform_bucket_level_access_locked_time) - def test_from_api_repr_w_empty_bpo(self): + def test_from_api_repr_w_empty_ubla(self): klass = self._get_target_class() bucket = self._make_bucket() - resource = {"bucketPolicyOnly": {}} + resource = {"uniformBucketLevelAccess": {}} config = klass.from_api_repr(resource, bucket) self.assertIs(config.bucket, bucket) - self.assertFalse(config.bucket_policy_only_enabled) - self.assertIsNone(config.bucket_policy_only_locked_time) + self.assertFalse(config.uniform_bucket_level_access_enabled) + self.assertIsNone(config.uniform_bucket_level_access_locked_time) def test_from_api_repr_w_disabled(self): klass = self._get_target_class() bucket = self._make_bucket() - resource = {"bucketPolicyOnly": {"enabled": False}} + resource = {"uniformBucketLevelAccess": {"enabled": False}} config = klass.from_api_repr(resource, bucket) self.assertIs(config.bucket, bucket) - self.assertFalse(config.bucket_policy_only_enabled) - self.assertIsNone(config.bucket_policy_only_locked_time) + self.assertFalse(config.uniform_bucket_level_access_enabled) + self.assertIsNone(config.uniform_bucket_level_access_locked_time) def test_from_api_repr_w_enabled(self): import datetime @@ -257,7 +257,7 @@ def test_from_api_repr_w_enabled(self): bucket = self._make_bucket() now = datetime.datetime.utcnow().replace(tzinfo=pytz.UTC) resource = { - "bucketPolicyOnly": { + "uniformBucketLevelAccess": { "enabled": True, "lockedTime": _datetime_to_rfc3339(now), } @@ -266,16 +266,16 @@ def test_from_api_repr_w_enabled(self): config = klass.from_api_repr(resource, bucket) self.assertIs(config.bucket, bucket) - self.assertTrue(config.bucket_policy_only_enabled) - self.assertEqual(config.bucket_policy_only_locked_time, now) + self.assertTrue(config.uniform_bucket_level_access_enabled) + self.assertEqual(config.uniform_bucket_level_access_locked_time, now) - def test_bucket_policy_only_enabled_setter(self): + def test_uniform_bucket_level_access_enabled_setter(self): bucket = self._make_bucket() config = self._make_one(bucket) - config.bucket_policy_only_enabled = True + config.uniform_bucket_level_access_enabled = True - self.assertTrue(config["bucketPolicyOnly"]["enabled"]) + self.assertTrue(config["uniformBucketLevelAccess"]["enabled"]) bucket._patch_property.assert_called_once_with("iamConfiguration", config) @@ -1249,8 +1249,8 @@ def test_iam_configuration_policy_missing(self): self.assertIsInstance(config, IAMConfiguration) self.assertIs(config.bucket, bucket) - self.assertFalse(config.bucket_policy_only_enabled) - self.assertIsNone(config.bucket_policy_only_locked_time) + self.assertFalse(config.uniform_bucket_level_access_enabled) + self.assertIsNone(config.uniform_bucket_level_access_locked_time) def test_iam_configuration_policy_w_entry(self): import datetime @@ -1262,7 +1262,7 @@ def test_iam_configuration_policy_w_entry(self): NAME = "name" properties = { "iamConfiguration": { - "bucketPolicyOnly": { + "uniformBucketLevelAccess": { "enabled": True, "lockedTime": _datetime_to_rfc3339(now), } @@ -1274,8 +1274,8 @@ def test_iam_configuration_policy_w_entry(self): self.assertIsInstance(config, IAMConfiguration) self.assertIs(config.bucket, bucket) - self.assertTrue(config.bucket_policy_only_enabled) - self.assertEqual(config.bucket_policy_only_locked_time, now) + self.assertTrue(config.uniform_bucket_level_access_enabled) + self.assertEqual(config.uniform_bucket_level_access_locked_time, now) def test_lifecycle_rules_getter_unknown_action_type(self): NAME = "name" From 03723d0f572a8cb1489a61acc80292b2fadcaddf Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Sat, 27 Jul 2019 00:49:59 -0700 Subject: [PATCH 2/5] Add workaround --- storage/google/cloud/storage/bucket.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 8549e6cac30c..ad61af4bb1b8 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -348,14 +348,18 @@ def uniform_bucket_level_access_enabled(self): :rtype: bool :returns: whether the bucket is configured to allow only IAM. """ - bpo = self.get("uniformBucketLevelAccess", {}) - return bpo.get("enabled", False) + ubla = self.get("uniformBucketLevelAccess", {}) + return ubla.get("enabled", False) @uniform_bucket_level_access_enabled.setter def uniform_bucket_level_access_enabled(self, value): ubla = self.setdefault("uniformBucketLevelAccess", {}) ubla["enabled"] = bool(value) + #### THIS IS A WORKAROUND #### + bpo = self.setdefault("bucketPolicyOnly", {}) + bpo["enabled"] = bool(value) self.bucket._patch_property("iamConfiguration", self) + #### THIS IS A WORKAROUND #### @bucket_policy_only_enabled.setter def bucket_policy_only_enabled(self, value): @@ -381,7 +385,7 @@ def uniform_bucket_level_access_locked_time(self): be frozen as true. """ ubla = self.get("uniformBucketLevelAccess", {}) - stamp = bpo.get("lockedTime") + stamp = ubla.get("lockedTime") if stamp is not None: stamp = _rfc3339_to_datetime(stamp) return stamp From 6e9e0bbdf031d38f3410eba709fc853d2a171994 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Sat, 27 Jul 2019 00:51:09 -0700 Subject: [PATCH 3/5] Unskip test --- storage/tests/system.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/storage/tests/system.py b/storage/tests/system.py index 6485fef3a6bb..96b76cceaf7d 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -1683,9 +1683,8 @@ def test_new_bucket_w_ubla(self): with self.assertRaises(exceptions.BadRequest): blob_acl.save() - @unittest.skipUnless(False, "Back-end fix for BPO/UBLA expected 2019-07-12") def test_bpo_set_unset_preserves_acls(self): - new_bucket_name = "bpo-acls" + unique_resource_id("-") + new_bucket_name = "ubla-acls" + unique_resource_id("-") self.assertRaises( exceptions.NotFound, Config.CLIENT.get_bucket, new_bucket_name ) @@ -1697,25 +1696,25 @@ def test_bpo_set_unset_preserves_acls(self): payload = b"DEADBEEF" blob.upload_from_string(payload) - # Preserve ACLs before setting BPO + # Preserve ACLs before setting UBLA bucket_acl_before = list(bucket.acl) blob_acl_before = list(bucket.acl) - # Set BPO + # Set UBLA bucket.iam_configuration.uniform_bucket_level_access_enabled = True bucket.patch() self.assertTrue(bucket.iam_configuration.uniform_bucket_level_access_enabled) - # While BPO is set, cannot get / set ACLs + # While UBLA is set, cannot get / set ACLs with self.assertRaises(exceptions.BadRequest): bucket.acl.reload() - # Clear BPO + # Clear UBLA bucket.iam_configuration.uniform_bucket_level_access_enabled = False bucket.patch() - # Query ACLs after clearing BPO + # Query ACLs after clearing UBLA bucket.acl.reload() bucket_acl_after = list(bucket.acl) blob.acl.reload() From ee3db0d7e1e5961d980fff196ceffaa10444beed Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Sat, 27 Jul 2019 02:49:38 -0700 Subject: [PATCH 4/5] Support both constructor parameters --- storage/google/cloud/storage/bucket.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index ad61af4bb1b8..ee3bdb52e0bb 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -302,11 +302,19 @@ def __init__( uniform_bucket_level_access_locked_time=None, ): # Which one wins??? - data = {"uniformBucketLevelAccess": {"enabled": bucket_policy_only_enabled}} + if bucket_policy_only_enabled: + data = {"bucketPolicyOnly": {"enabled": bucket_policy_only_enabled}} + if bucket_policy_only_locked_time: + data["bucketPolicyOnly"]["lockedTime"] = _datetime_to_rfc3339( + bucket_policy_only_locked_time + ) + if uniform_bucket_level_access_enabled: + data = {"uniformBucketLevelAccess": {"enabled": uniform_bucket_level_access_enabled}} if uniform_bucket_level_access_locked_time is not None: data["uniformBucketLevelAccess"]["lockedTime"] = _datetime_to_rfc3339( - bucket_policy_only_locked_time + uniform_bucket_level_access_locked_time ) + super(IAMConfiguration, self).__init__(data) self._bucket = bucket From eb36359d588cf326fac5c18b20af8090d810b754 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Mon, 12 Aug 2019 17:43:44 -0700 Subject: [PATCH 5/5] Select expected state --- storage/google/cloud/storage/bucket.py | 32 +++++++++++++++++--------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index ee3bdb52e0bb..6ffb6a41dc05 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -301,19 +301,28 @@ def __init__( uniform_bucket_level_access_enabled=False, uniform_bucket_level_access_locked_time=None, ): - # Which one wins??? + data = {"bucketPolicyOnly": {}, "uniformBucketLevelAccess": {}} if bucket_policy_only_enabled: - data = {"bucketPolicyOnly": {"enabled": bucket_policy_only_enabled}} - if bucket_policy_only_locked_time: - data["bucketPolicyOnly"]["lockedTime"] = _datetime_to_rfc3339( - bucket_policy_only_locked_time - ) + data["bucketPolicyOnly"]["enabled"] = bucket_policy_only_enabled + data["uniformBucketLevelAccess"]["enabled"] = bucket_policy_only_enabled + if bucket_policy_only_locked_time is not None: + data["bucketPolicyOnly"]["lockedTime"] = _datetime_to_rfc3339( + bucket_policy_only_locked_time + ) + data["uniformBucketLevelAccess"]["lockedTime"] = _datetime_to_rfc3339( + bucket_policy_only_locked_time + ) + if uniform_bucket_level_access_enabled: - data = {"uniformBucketLevelAccess": {"enabled": uniform_bucket_level_access_enabled}} + data["bucketPolicyOnly"]["enabled"] = uniform_bucket_level_access_enabled + data["uniformBucketLevelAccess"]["enabled"] = uniform_bucket_level_access_enabled if uniform_bucket_level_access_locked_time is not None: - data["uniformBucketLevelAccess"]["lockedTime"] = _datetime_to_rfc3339( - uniform_bucket_level_access_locked_time - ) + data["bucketPolicyOnly"]["lockedTime"] = _datetime_to_rfc3339( + uniform_bucket_level_access_locked_time + ) + data["uniformBucketLevelAccess"]["lockedTime"] = _datetime_to_rfc3339( + uniform_bucket_level_access_locked_time + ) super(IAMConfiguration, self).__init__(data) self._bucket = bucket @@ -331,6 +340,7 @@ def from_api_repr(cls, resource, bucket): :rtype: :class:`IAMConfiguration` :returns: Instance created from resource. """ + instance = cls(bucket) instance.update(resource) return instance @@ -366,8 +376,8 @@ def uniform_bucket_level_access_enabled(self, value): #### THIS IS A WORKAROUND #### bpo = self.setdefault("bucketPolicyOnly", {}) bpo["enabled"] = bool(value) - self.bucket._patch_property("iamConfiguration", self) #### THIS IS A WORKAROUND #### + self.bucket._patch_property("iamConfiguration", self) @bucket_policy_only_enabled.setter def bucket_policy_only_enabled(self, value):