From ddc58ee36deee7b7f71c2026c28f7632700b78e7 Mon Sep 17 00:00:00 2001 From: Jonathan Lui Date: Tue, 4 Feb 2020 18:38:43 -0800 Subject: [PATCH] feat: support iam conditions (#2416) * support IAM condition * update Storage JSON definition, rev,. 20190913 * throw InvalidOperationException if version is greater than 1 * nit: doc * do not cache Iam instance on bucket; make $version arg a key in $options array * revert version argument from IAM constructor * docs: document optionsRequestedPolicyVersion * revert stored $iam instance on bucket * docs: add example of a policy in PolicyBuilder * more docs * fix link * map requestedPolicyVersion arg to optionsRequestedPolicyVersion in Storage req opts * fix(docs): optionsRequestedPolicyVersion => requestedPolicyVersion * fix * test: validate policy version and conditions * test: assert requestedPolicyVersion arg is mapped to optionsRequestedPolicyVersion * merge Storage definition from master * fix: lint * lint * fix * docs: update inline sample to use prefix condition * add IAM get/set system tests * add conditional policy system test * fix docs Co-Authored-By: David Supplee * fix @see markdown links * fix ; * add @deprecated tag * use BadMethodCallException * fix style * fix style * add snippet coverage * update bucket->iam snippet tests * fix snippet parsing issue * Update Storage/tests/System/IamTest.php Co-Authored-By: David Supplee * Update Core/src/Iam/PolicyBuilder.php Co-Authored-By: David Supplee Co-authored-by: David Supplee --- .rnd | Bin 0 -> 1024 bytes Core/src/Iam/Iam.php | 4 + Core/src/Iam/PolicyBuilder.php | 88 +++++++++++- Core/tests/Unit/Iam/PolicyBuilderTest.php | 88 +++++++++++- Storage/src/Bucket.php | 11 +- Storage/src/Connection/IamBucket.php | 5 + Storage/tests/Snippet/BucketTest.php | 3 + Storage/tests/System/IamTest.php | 135 ++++++++++++++++++ .../tests/Unit/Connection/IamBucketTest.php | 12 ++ 9 files changed, 338 insertions(+), 8 deletions(-) create mode 100644 .rnd create mode 100644 Storage/tests/System/IamTest.php diff --git a/.rnd b/.rnd new file mode 100644 index 0000000000000000000000000000000000000000..94b5092fe6b89b19fbe36efeae533d6e6cbf4b50 GIT binary patch literal 1024 zcmV+b1poUzb`w&mBj+oiH$2jHAUWFyc?Zn!Te;W8;buo5!j6OPeWqA-Hk8AVv$KQoeS zt#PS=_6X7-ydXF-#>cV}>GnuS*aG-zW~z`mNu417U79gCGp0^`BekxW&Sf9gwXO&W=IsKWzaZo z?$%n+C^kxwm0)dhVkzDpDqB4kH?o6TRjFF4iLkXp;$$e+bmM41j)NE4)eGUZahR~$ z(~1^vPa~{PR3LEKhpo)F=>i~yGr*sWYel&(t$oGx*!RX_Zc>ss?GWgFSbh zT5Umhg(C@Kys6U!#>GjmTVyDn%HU%dW_f|G6IvkObgd`f@VRG+LUVQ*vn$&sq7bDukfGeWWDREJq*y=eFnO~?7JtD+LMyuZrZ?HE&1@-FD4 zgD3wvvy-fLqVQLuAn;|A)P&4%Q*?SrciQBocK%{JEmkxO}N0prCg5#atg`(Aq%v%`}J{u9Ss<#*+;u^W3E#~n6G0T z^Ff 'AgIc==', + * 'version' => 3, + * 'bindings' => [ + * [ + * 'role' => 'roles/admin', + * 'members' => [ + * 'user:admin@domain.com', + * 'user2:admin@domain.com' + * ], + * 'condition' => [ + * 'title' => 'match-prefix', + * 'description' => 'Applies to objects matching a prefix', + * 'expression' => + * 'resource.name.startsWith("projects/_/buckets/bucket-name/objects/prefix-a-")' + * ] + * ] + * ], + * ]; + * + * $builder = new PolicyBuilder($policy); + * ``` + * * @param array $policy A policy array * @throws InvalidArgumentException */ @@ -81,6 +114,10 @@ public function __construct(array $policy = []) * 'role' => 'roles/admin', * 'members' => [ * 'user:admin@domain.com' + * ], + * 'condition' => [ + * 'expression' => + * 'request.time < timestamp("2020-07-01T00:00:00.000Z")' * ] * ] * ]); @@ -92,17 +129,20 @@ public function __construct(array $policy = []) */ public function setBindings(array $bindings = []) { - $this->bindings = []; - foreach ($bindings as $binding) { - $this->addBinding($binding['role'], $binding['members']); - } - + $this->bindings = $bindings; return $this; } /** * Add a new binding to the policy. * + * This method will fail with an InvalidOpereationException if it is + * called on a Policy with a version greater than 1 as that indicates + * a more complicated policy than this method is prepared to handle. + * Changes to such policies must be made manually by the setBindings() + * method. + * + * * Example: * ``` * $builder->addBinding('roles/admin', [ 'user:admin@domain.com' ]); @@ -112,9 +152,13 @@ public function setBindings(array $bindings = []) * @param array $members An array of members to assign to the binding * @return PolicyBuilder * @throws InvalidArgumentException + * @throws BadMethodCallException if the policy's version is greater than 1. + * @deprecated */ public function addBinding($role, array $members) { + $this->validatePolicyVersion(); + $this->bindings[] = [ 'role' => $role, 'members' => $members @@ -126,6 +170,12 @@ public function addBinding($role, array $members) /** * Remove a binding from the policy. * + * This method will fail with a BadMethodCallException if it is + * called on a Policy with a version greater than 1 as that indicates + * a more complicated policy than this method is prepared to handle. + * Changes to such policies must be made manually by the setBindings() + * method. + * * Example: * ``` * $builder->setBindings([ @@ -144,9 +194,13 @@ public function addBinding($role, array $members) * @param array $members An array of members to remove from the role * @return PolicyBuilder * @throws InvalidArgumentException + * @throws BadMethodCallException if the policy's version is greater than 1. + * @deprecated */ public function removeBinding($role, array $members) { + $this->validatePolicyVersion(); + $bindings = $this->bindings; foreach ((array) $bindings as $i => $binding) { if ($binding['role'] == $role) { @@ -226,4 +280,28 @@ public function result() 'version' => $this->version ]); } + + private function validatePolicyVersion() + { + if (isset($this->version) && $this->version > 1) { + throw new BadMethodCallException("Helper methods cannot be " . + "invoked on policies with version {$this->version}."); + } + + $this->validateConditions(); + } + + private function validateConditions() + { + if (!$this->bindings) { + return; + } + + foreach ($this->bindings as $binding) { + if (isset($binding['condition'])) { + throw new BadMethodCallException("Helper methods cannot " . + "be invoked on policies containing conditions."); + } + } + } } diff --git a/Core/tests/Unit/Iam/PolicyBuilderTest.php b/Core/tests/Unit/Iam/PolicyBuilderTest.php index 88e1b1300bd3..d66f2adfa261 100644 --- a/Core/tests/Unit/Iam/PolicyBuilderTest.php +++ b/Core/tests/Unit/Iam/PolicyBuilderTest.php @@ -42,7 +42,7 @@ public function testBuilder() $builder = new PolicyBuilder; $builder->setEtag($etag); - $builder->setVersion(2); + $builder->setVersion(1); $builder->addBinding($role, $members); $result = $builder->result(); @@ -55,7 +55,7 @@ public function testBuilder() ] ], 'etag' => $etag, - 'version' => 2 + 'version' => 1 ]; $this->assertEquals($policy, $result); @@ -139,6 +139,43 @@ public function testConstructWithExistingPolicy() $this->assertEquals($policy, $result); } + /** + * @expectedException BadMethodCallException + * @expectedExceptionMessage Helper methods cannot be invoked on policies with version 3. + */ + public function testAddBindingVersionThrowsException() + { + $builder = new PolicyBuilder(); + $builder->setVersion(3); + + $builder->addBinding('test', ['user:test@test.com']); + } + + /** + * @expectedException BadMethodCallException + * @expectedExceptionMessage Helper methods cannot be invoked on policies containing conditions. + */ + public function testAddBindingWithConditionsThrowsException() + { + $policy = [ + 'bindings' => [ + [ + 'role' => 'test', + 'members' => [ + 'user:test@test.com', + ], + 'condition' => [ + 'expression' => 'true', + ] + ], + ], + ]; + $builder = new PolicyBuilder($policy); + $builder->setVersion(1); + + $builder->addBinding('test2', ['user:test@test.com']); + } + public function testRemoveBinding() { $policy = [ @@ -225,4 +262,51 @@ public function testRemoveBindingInvalidRoleThrowsException() $builder = new PolicyBuilder($policy); $builder->removeBinding('test2', ['user:test@test.com']); } + + /** + * @expectedException BadMethodCallException + * @expectedExceptionMessage Helper methods cannot be invoked on policies with version 3. + */ + public function testRemoveBindingVersionThrowsException() + { + $policy = [ + 'version' => 3, + 'bindings' => [ + [ + 'role' => 'test', + 'members' => [ + 'user:test@test.com', + ] + ], + ] + ]; + + $builder = new PolicyBuilder($policy); + $builder->removeBinding('test', ['user:test@test.com']); + } + + /** + * @expectedException BadMethodCallException + * @expectedExceptionMessage Helper methods cannot be invoked on policies containing conditions. + */ + public function testRemoveBindingWithConditionsThrowsException() + { + $policy = [ + 'bindings' => [ + [ + 'role' => 'test', + 'members' => [ + 'user:test@test.com', + ], + 'condition' => [ + 'expression' => 'true', + ] + ], + ], + ]; + + $builder = new PolicyBuilder($policy); + $builder->setVersion(1); + $builder->removeBinding('test', ['user:test@test.com']); + } } diff --git a/Storage/src/Bucket.php b/Storage/src/Bucket.php index 885f4702369c..0320461ea064 100644 --- a/Storage/src/Bucket.php +++ b/Storage/src/Bucket.php @@ -1170,11 +1170,19 @@ public function isWritable($file = null) /** * Manage the IAM policy for the current Bucket. * - * Please note that this method may not yet be available in your project. + * To request a policy with conditions, pass an array with + * '[requestedPolicyVersion => 3]' as argument to the policy() and + * reload() methods. * * Example: * ``` * $iam = $bucket->iam(); + * + * // Returns the stored policy, or fetches the policy if none exists. + * $policy = $iam->policy(['requestedPolicyVersion' => 3]); + * + * // Fetches a policy from the server. + * $policy = $iam->reload(['requestedPolicyVersion' => 3]); * ``` * * @codingStandardsIgnoreStart @@ -1182,6 +1190,7 @@ public function isWritable($file = null) * @see https://cloud.google.com/storage/docs/json_api/v1/buckets/getIamPolicy Get Bucket IAM Policy * @see https://cloud.google.com/storage/docs/json_api/v1/buckets/setIamPolicy Set Bucket IAM Policy * @see https://cloud.google.com/storage/docs/json_api/v1/buckets/testIamPermissions Test Bucket Permissions + * @see https://cloud.google.com/iam/docs/policies#versions policy versioning. * @codingStandardsIgnoreEnd * * @return Iam diff --git a/Storage/src/Connection/IamBucket.php b/Storage/src/Connection/IamBucket.php index b7b9b982931e..4a1d2511c23e 100644 --- a/Storage/src/Connection/IamBucket.php +++ b/Storage/src/Connection/IamBucket.php @@ -42,6 +42,11 @@ public function __construct(ConnectionInterface $connection) */ public function getPolicy(array $args) { + if (isset($args['requestedPolicyVersion'])) { + $args['optionsRequestedPolicyVersion'] = $args['requestedPolicyVersion']; + unset($args['requestedPolicyVersion']); + } + return $this->connection->getBucketIamPolicy($args); } diff --git a/Storage/tests/Snippet/BucketTest.php b/Storage/tests/Snippet/BucketTest.php index 2020ac8ac4a5..f75166ab1545 100644 --- a/Storage/tests/Snippet/BucketTest.php +++ b/Storage/tests/Snippet/BucketTest.php @@ -592,7 +592,10 @@ public function testIam() { $snippet = $this->snippetFromMethod(Bucket::class, 'iam'); $snippet->addLocal('bucket', $this->bucket); + $this->connection->getBucketIamPolicy(Argument::withEntry('optionsRequestedPolicyVersion', 3)) + ->shouldBeCalled(); + $this->bucket->___setProperty('connection', $this->connection->reveal()); $res = $snippet->invoke('iam'); $this->assertInstanceOf(Iam::class, $res->returnVal()); } diff --git a/Storage/tests/System/IamTest.php b/Storage/tests/System/IamTest.php new file mode 100644 index 000000000000..b8eda8227101 --- /dev/null +++ b/Storage/tests/System/IamTest.php @@ -0,0 +1,135 @@ + getenv('GOOGLE_CLOUD_PHP_TESTS_KEY_PATH'), + 'transport' => 'rest' + ]; + + $client = new StorageClient($config); + $this->$bucket = self::createBucket($client, uniqid(self::TESTING_PREFIX)); + $this->$bucket->update($this->bucketConfig()); + } + + public function testGetPolicy() + { + $keyfile = json_decode(file_get_contents(getenv('GOOGLE_CLOUD_PHP_TESTS_KEY_PATH')), true); + $projectId = $keyfile['project_id']; + + $iam = $this->$bucket->iam(); + $policy = $iam->policy(); + + $this->assertTrue(isset($policy['etag'])); + $this->assertEquals( + [ + [ + 'role' => 'roles/storage.legacyBucketOwner', + 'members' => [ + 'projectEditor:' . $projectId, + 'projectOwner:' . $projectId + ] + ], + [ + 'role' => 'roles/storage.legacyBucketReader', + 'members' => ['projectViewer:' . $projectId] + ] + + ], + $policy['bindings'] + ); + } + + public function testSetPolicy() + { + $keyfile = json_decode(file_get_contents(getenv('GOOGLE_CLOUD_PHP_TESTS_KEY_PATH')), true); + $projectId = $keyfile['project_id']; + + $iam = $this->$bucket->iam(); + $policy = $iam->policy(); + $newBinding = [ + 'role' => 'roles/storage.legacyBucketReader', + 'members' => ['allUsers'] + ]; + + $policy['bindings'][] = $newBinding; + + $iam->setPolicy($policy); + $policy = $iam->reload(); + $this->assertContains( + [ + 'role' => 'roles/storage.legacyBucketReader', + 'members' => ['allUsers', 'projectViewer:' . $projectId] + ], + $policy['bindings'] + ); + } + + public function testGetModifySetConditionalPolicy() + { + $keyfile = json_decode(file_get_contents(getenv('GOOGLE_CLOUD_PHP_TESTS_KEY_PATH')), true); + $email = $keyfile['client_email']; + + $iam = $this->$bucket->iam(); + $policy = $iam->policy(); + $policy['version'] = 3; + + $conditionalBinding = [ + 'role' => 'roles/storage.objectViewer', + 'members' => ['serviceAccount:' . $email], + 'condition' => [ + 'title' => 'always-true', + 'description' => 'this condition is always effective', + 'expression' => 'true', + ] + ]; + + $policy['bindings'][] = $conditionalBinding; + $iam->setPolicy($policy); + $policy = $iam->reload(['requestedPolicyVersion' => 3]); + $this->assertContains( + $conditionalBinding, + $policy['bindings'] + ); + } + + private function bucketConfig($enabled = true) + { + return [ + 'iamConfiguration' => [ + 'uniformBucketLevelAccess' => [ + 'enabled' => $enabled + ] + ] + ]; + } +} diff --git a/Storage/tests/Unit/Connection/IamBucketTest.php b/Storage/tests/Unit/Connection/IamBucketTest.php index b5808f1c8765..dc6887707236 100644 --- a/Storage/tests/Unit/Connection/IamBucketTest.php +++ b/Storage/tests/Unit/Connection/IamBucketTest.php @@ -41,6 +41,18 @@ public function testProxies($methodName, $proxyName, $args) $this->assertEquals($args, $iam->$methodName($args)); } + public function testRequestedPolicyVersion() + { + $connection = $this->prophesize(ConnectionInterface::class); + $connection->getBucketIamPolicy(['optionsRequestedPolicyVersion' => 3]) + ->willReturn(['version' => 3]) + ->shouldBeCalledTimes(1); + + $iam = new IamBucket($connection->reveal()); + $args = ['requestedPolicyVersion' => 3]; + $this->assertEquals(['version' => 3], $iam->getPolicy($args)); + } + public function methodProvider() { $args = ['foo' => 'bar'];