Skip to content

Better object ACL support #57

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mgriego wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Better object ACL support #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ed7d67a
Allow the default bucket ACL to be applied when visibility is not exp…
mgriego May 3, 2017
2bedf0c
Update tests for enhanced ACL capabilities, and fix bug discovered by…
mgriego May 3, 2017
4e4b991
Fix style issue
mgriego May 3, 2017
926d9b6
Fix remaining style issues not included in first report.
mgriego May 3, 2017
5c2ffaa
Refactor/cleanup.
mgriego May 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 45 additions & 10 deletions src/GoogleStorageAdapter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,6 @@ protected function getOptionsFromConfig(Config $config)

if ($visibility = $config->get('visibility')) {
$options['predefinedAcl'] = $this->getPredefinedAclForVisibility($visibility);
} else {
// if a file is created without an acl, it isn't accessible via the console
// we therefore default to private
$options['predefinedAcl'] = $this->getPredefinedAclForVisibility(AdapterInterface::VISIBILITY_PRIVATE);
}

if ($metadata = $config->get('metadata')) {
Expand Down Expand Up @@ -214,21 +210,60 @@ public function rename($path, $newpath)
return $this->delete($path);
}

/**
* Simplifies an array of ACL objects to an array of entity => role.
*
* @param array $acl
*
* @return array
*/
protected function simplifyAcl($acl)
{
return array_combine(array_column($acl, 'entity'), array_column($acl, 'role'));
}

/**
* Update the ACL of the target object to match the source object.
*
* @param $sourceObject
* @param $targetObject
*/
protected function copyAcl($sourceObject, $targetObject)
{
$sourceAcl = $this->simplifyAcl($sourceObject->acl()->get());

$targetAcl = $this->simplifyAcl($targetObject->acl()->get());

foreach (array_keys($targetAcl) as $entity) {
if (!isset($sourceAcl[$entity])) {
$targetObject->acl()->delete($entity);
}
}

foreach ($sourceAcl as $entity => $role) {
if (!isset($targetAcl[$entity])) {
$targetObject->acl()->add($entity, $role);
} elseif ($targetAcl[$entity] != $role) {
$targetObject->acl()->update($entity, $role);
}
}
}

/**
* {@inheritdoc}
*/
public function copy($path, $newpath)
{
$newpath = $this->applyPathPrefix($newpath);

// we want the new file to have the same visibility as the original file
$visibility = $this->getRawVisibility($path);
$sourceObject = $this->getObject($path);

$options = [
'name' => $newpath,
'predefinedAcl' => $this->getPredefinedAclForVisibility($visibility),
];
$this->getObject($path)->copy($this->bucket, $options);
$targetObject = $sourceObject->copy($this->bucket, $options);

$this->copyAcl($sourceObject, $targetObject);

return true;
}
Expand Down Expand Up @@ -410,11 +445,11 @@ protected function getRawVisibility($path)
{
try {
$acl = $this->getObject($path)->acl()->get(['entity' => 'allUsers']);
return $acl['role'] === Acl::ROLE_READER ?
return ($acl['role'] === Acl::ROLE_READER || $acl['role'] == Acl::ROLE_OWNER) ?
AdapterInterface::VISIBILITY_PUBLIC :
AdapterInterface::VISIBILITY_PRIVATE;
} catch (NotFoundException $e) {
// object may not have an acl entry, so handle that gracefully
// No ACL entry for allUsers means object is private.
return AdapterInterface::VISIBILITY_PRIVATE;
}
}
Expand Down
183 changes: 134 additions & 49 deletions tests/GoogleStorageAdapterTests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Tests;

use Google\Cloud\Exception\NotFoundException;
use Google\Cloud\Storage\Acl;
use Google\Cloud\Storage\Bucket;
use Google\Cloud\Storage\StorageClient;
Expand Down Expand Up @@ -53,7 +54,6 @@ public function testWrite()
'This is the file contents.',
[
'name' => 'prefix/file1.txt',
'predefinedAcl' => 'projectPrivate',
],
])
->once()
Expand Down Expand Up @@ -187,7 +187,6 @@ public function testWriteStream()
$stream,
[
'name' => 'prefix/file1.txt',
'predefinedAcl' => 'projectPrivate',
],
])
->once()
Expand Down Expand Up @@ -218,31 +217,79 @@ public function testRename()

$oldStorageObjectAcl = Mockery::mock(Acl::class);
$oldStorageObjectAcl->shouldReceive('get')
->with(['entity' => 'allUsers'])
->withNoArgs()
->once()
->andReturn([
'role' => Acl::ROLE_OWNER,
[
'entity' => 'should-not-change',
'role' => Acl::ROLE_OWNER,
],
[
'entity' => 'should-be-added',
'role' => Acl::ROLE_OWNER,
],
[
'entity' => 'should-be-updated',
'role' => Acl::ROLE_OWNER,
],
]);

$newStorageObjectAcl = Mockery::mock(Acl::class);
$newStorageObjectAcl->shouldReceive('get')
->withNoArgs()
->once()
->andReturn([
[
'entity' => 'should-not-change',
'role' => Acl::ROLE_OWNER,
],
[
'entity' => 'should-be-deleted',
'role' => Acl::ROLE_OWNER,
],
[
'entity' => 'should-be-updated',
'role' => Acl::ROLE_READER,
],
]);
$newStorageObjectAcl->shouldReceive('delete')
->with('should-be-deleted')
->once();
$newStorageObjectAcl->shouldReceive('add')
->with('should-be-added', Acl::ROLE_OWNER)
->once();
$newStorageObjectAcl->shouldReceive('update')
->with('should-be-updated', Acl::ROLE_OWNER)
->once();

$newStorageObject = Mockery::mock(StorageObject::class);
$newStorageObject->shouldReceive('acl')
->withNoArgs()
->times(4)
->andReturn($newStorageObjectAcl);

$oldStorageObject = Mockery::mock(StorageObject::class);
$oldStorageObject->shouldReceive('acl')
->withNoArgs()
->once()
->andReturn($oldStorageObjectAcl);

$oldStorageObject->shouldReceive('copy')
->withArgs([
$bucket,
[
'name' => 'prefix/new_file.txt',
'predefinedAcl' => 'projectPrivate',
],
])
->once();
->once()
->andReturn($newStorageObject);

$oldStorageObject->shouldReceive('delete')
->once();

$bucket->shouldReceive('object')
->with('prefix/old_file.txt')
->times(3)
->twice()
->andReturn($oldStorageObject);

$storageClient = Mockery::mock(StorageClient::class);
Expand All @@ -258,70 +305,80 @@ public function testCopy()

$oldStorageObjectAcl = Mockery::mock(Acl::class);
$oldStorageObjectAcl->shouldReceive('get')
->with(['entity' => 'allUsers'])
->withNoArgs()
->once()
->andReturn([
'role' => Acl::ROLE_OWNER,
[
'entity' => 'should-not-change',
'role' => Acl::ROLE_OWNER,
],
[
'entity' => 'should-be-added',
'role' => Acl::ROLE_OWNER,
],
[
'entity' => 'should-be-updated',
'role' => Acl::ROLE_OWNER,
],
]);

$oldStorageObject = Mockery::mock(StorageObject::class);
$oldStorageObject->shouldReceive('acl')
$newStorageObjectAcl = Mockery::mock(Acl::class);
$newStorageObjectAcl->shouldReceive('get')
->withNoArgs()
->once()
->andReturn($oldStorageObjectAcl);
$oldStorageObject->shouldReceive('copy')
->withArgs([
$bucket,
->andReturn([
[
'name' => 'prefix/new_file.txt',
'predefinedAcl' => 'projectPrivate',
'entity' => 'should-not-change',
'role' => Acl::ROLE_OWNER,
],
])
[
'entity' => 'should-be-deleted',
'role' => Acl::ROLE_OWNER,
],
[
'entity' => 'should-be-updated',
'role' => Acl::ROLE_READER,
],
]);
$newStorageObjectAcl->shouldReceive('delete')
->with('should-be-deleted')
->once();
$newStorageObjectAcl->shouldReceive('add')
->with('should-be-added', Acl::ROLE_OWNER)
->once();
$newStorageObjectAcl->shouldReceive('update')
->with('should-be-updated', Acl::ROLE_OWNER)
->once();

$bucket->shouldReceive('object')
->with('prefix/old_file.txt')
->times(2)
->andReturn($oldStorageObject);

$storageClient = Mockery::mock(StorageClient::class);

$adapter = new GoogleStorageAdapter($storageClient, $bucket, 'prefix');

$adapter->copy('old_file.txt', 'new_file.txt');
}

public function testCopyWhenOriginalFileIsPublic()
{
$storageClient = Mockery::mock(StorageClient::class);
$bucket = Mockery::mock(Bucket::class);

$oldStorageObjectAcl = Mockery::mock(Acl::class);
$oldStorageObjectAcl->shouldReceive('get')
->with(['entity' => 'allUsers'])
->once()
->andReturn([
'role' => Acl::ROLE_READER,
]);
$newStorageObject = Mockery::mock(StorageObject::class);
$newStorageObject->shouldReceive('acl')
->withNoArgs()
->times(4)
->andReturn($newStorageObjectAcl);

$oldStorageObject = Mockery::mock(StorageObject::class);
$oldStorageObject->shouldReceive('acl')
->withNoArgs()
->once()
->andReturn($oldStorageObjectAcl);

$oldStorageObject->shouldReceive('copy')
->withArgs([
$bucket,
[
'name' => 'prefix/new_file.txt',
'predefinedAcl' => 'publicRead',
],
])
->once();
->once()
->andReturn($newStorageObject);

$bucket->shouldReceive('object')
->with('prefix/old_file.txt')
->times(2)
->once()
->andReturn($oldStorageObject);

$storageClient = Mockery::mock(StorageClient::class);

$adapter = new GoogleStorageAdapter($storageClient, $bucket, 'prefix');

$adapter->copy('old_file.txt', 'new_file.txt');
Expand Down Expand Up @@ -835,9 +892,7 @@ public function testGetVisibilityWhenVisibilityIsPrivate()
$storageObjectAcl->shouldReceive('get')
->with(['entity' => 'allUsers'])
->once()
->andReturn([
'role' => Acl::ROLE_OWNER,
]);
->andThrow(NotFoundException::class);

$storageObject = Mockery::mock(StorageObject::class);
$storageObject->shouldReceive('acl')
Expand All @@ -857,7 +912,7 @@ public function testGetVisibilityWhenVisibilityIsPrivate()
$this->assertEquals(['visibility' => AdapterInterface::VISIBILITY_PRIVATE], $visibility);
}

public function testGetVisibilityWhenVisibilityIsPublic()
public function testGetVisibilityIsPublicWhenAllUsersIsReader()
{
$bucket = Mockery::mock(Bucket::class);

Expand Down Expand Up @@ -887,6 +942,36 @@ public function testGetVisibilityWhenVisibilityIsPublic()
$this->assertEquals(['visibility' => AdapterInterface::VISIBILITY_PUBLIC], $visibility);
}

public function testGetVisibilityIsPublicWhenAllUsersIsOwner()
{
$bucket = Mockery::mock(Bucket::class);

$storageObjectAcl = Mockery::mock(Acl::class);
$storageObjectAcl->shouldReceive('get')
->with(['entity' => 'allUsers'])
->once()
->andReturn([
'role' => Acl::ROLE_OWNER,
]);

$storageObject = Mockery::mock(StorageObject::class);
$storageObject->shouldReceive('acl')
->once()
->andReturn($storageObjectAcl);

$bucket->shouldReceive('object')
->with('prefix/file.txt')
->once()
->andReturn($storageObject);

$storageClient = Mockery::mock(StorageClient::class);

$adapter = new GoogleStorageAdapter($storageClient, $bucket, 'prefix');

$visibility = $adapter->getVisibility('file.txt');
$this->assertEquals(['visibility' => AdapterInterface::VISIBILITY_PUBLIC], $visibility);
}

public function testSetGetStorageApiUri()
{
$storageClient = Mockery::mock(StorageClient::class);
Expand Down