Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@

package com.google.cloud.storage;

import com.google.api.services.storage.model.Expr;
import com.google.api.services.storage.model.Policy.Bindings;
import com.google.cloud.Identity;
import com.google.cloud.Binding;
import com.google.cloud.Condition;
import com.google.cloud.Policy;
import com.google.cloud.Role;
import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;

/**
* Helper for converting between the Policy model provided by the API and the Policy model provided
Expand All @@ -35,29 +35,46 @@ static Policy convertFromApiPolicy(com.google.api.services.storage.model.Policy
Policy.Builder policyBuilder = Policy.newBuilder();
List<Bindings> bindings = apiPolicy.getBindings();
if (null != bindings && !bindings.isEmpty()) {
ImmutableList.Builder<Binding> coreBindings = ImmutableList.builder();
for (Bindings binding : bindings) {
for (String member : binding.getMembers()) {
policyBuilder.addIdentity(Role.of(binding.getRole()), Identity.valueOf(member));
Binding.Builder bindingBuilder = Binding.newBuilder();
bindingBuilder.setRole(binding.getRole());
bindingBuilder.setMembers(binding.getMembers());
if (binding.getCondition() != null) {
Condition.Builder conditionBuilder = Condition.newBuilder();
conditionBuilder.setTitle(binding.getCondition().getTitle());
conditionBuilder.setDescription(binding.getCondition().getDescription());
conditionBuilder.setExpression(binding.getCondition().getExpression());
bindingBuilder.setCondition(conditionBuilder.build());
}
coreBindings.add(bindingBuilder.build());
}
policyBuilder.setBindings(coreBindings.build());
} else {
throw new IllegalStateException("Missing required bindings.");
}
return policyBuilder.setEtag(apiPolicy.getEtag()).build();
return policyBuilder.setEtag(apiPolicy.getEtag()).setVersion(apiPolicy.getVersion()).build();
}

static com.google.api.services.storage.model.Policy convertToApiPolicy(Policy policy) {
List<Bindings> bindings = new ArrayList<>(policy.getBindings().size());
for (Map.Entry<Role, Set<Identity>> entry : policy.getBindings().entrySet()) {
List<String> members = new ArrayList<>(entry.getValue().size());
for (Identity identity : entry.getValue()) {
members.add(identity.strValue());
List<Bindings> bindings = new ArrayList<>(policy.getBindingsList().size());
for (Binding binding : policy.getBindingsList()) {
Bindings apiBinding = new Bindings();
apiBinding.setRole(binding.getRole());
apiBinding.setMembers(new ArrayList<>(binding.getMembers()));
if (binding.getCondition() != null) {
Expr expr = new Expr();
expr.setTitle(binding.getCondition().getTitle());
expr.setDescription(binding.getCondition().getDescription());
expr.setExpression(binding.getCondition().getExpression());
apiBinding.setCondition(expr);
}
bindings.add(new Bindings().setMembers(members).setRole(entry.getKey().getValue()));
bindings.add(apiBinding);
}
return new com.google.api.services.storage.model.Policy()
.setBindings(bindings)
.setEtag(policy.getEtag());
.setEtag(policy.getEtag())
.setVersion(policy.getVersion());
}

private PolicyHelper() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,10 @@ public static BucketSourceOption metagenerationNotMatch(long metageneration) {
public static BucketSourceOption userProject(String userProject) {
return new BucketSourceOption(StorageRpc.Option.USER_PROJECT, userProject);
}

public static BucketSourceOption requestedPolicyVersion(long version) {
return new BucketSourceOption(StorageRpc.Option.REQUESTED_POLICY_VERSION, version);
}
}

/** Class for specifying listHmacKeys options */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1376,11 +1376,16 @@ public Policy getIamPolicy(String bucket, Map<Option, ?> options) {
Span span = startSpan(HttpStorageRpcSpans.SPAN_NAME_GET_BUCKET_IAM_POLICY);
Scope scope = tracer.withSpan(span);
try {
return storage
.buckets()
.getIamPolicy(bucket)
.setUserProject(Option.USER_PROJECT.getString(options))
.execute();
Storage.Buckets.GetIamPolicy getIamPolicy =
storage
.buckets()
.getIamPolicy(bucket)
.setUserProject(Option.USER_PROJECT.getString(options));
if (null != Option.REQUESTED_POLICY_VERSION.getLong(options)) {
getIamPolicy.setOptionsRequestedPolicyVersion(
Option.REQUESTED_POLICY_VERSION.getLong(options).intValue());
}
return getIamPolicy.execute();
} catch (IOException ex) {
span.setStatus(Status.UNKNOWN.withDescription(ex.getMessage()));
throw translate(ex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ enum Option {
USER_PROJECT("userProject"),
KMS_KEY_NAME("kmsKeyName"),
SERVICE_ACCOUNT_EMAIL("serviceAccount"),
SHOW_DELETED_KEYS("showDeletedKeys");
SHOW_DELETED_KEYS("showDeletedKeys"),
REQUESTED_POLICY_VERSION("optionsRequestedPolicyVersion");

private final String value;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public void testEquivalence() {
Identity.user("test1@gmail.com"),
Identity.user("test2@gmail.com"))
.setEtag(ETAG)
.setVersion(1)
.build();
com.google.api.services.storage.model.Policy apiPolicy =
new com.google.api.services.storage.model.Policy()
Expand All @@ -53,7 +54,8 @@ public void testEquivalence() {
.setMembers(
ImmutableList.of("user:test1@gmail.com", "user:test2@gmail.com"))
.setRole("roles/storage.objectAdmin")))
.setEtag(ETAG);
.setEtag(ETAG)
.setVersion(1);

Policy actualLibPolicy = PolicyHelper.convertFromApiPolicy(apiPolicy);
com.google.api.services.storage.model.Policy actualApiPolicy =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ public class StorageImplTest {
Identity.user("test1@gmail.com"),
Identity.user("test2@gmail.com"))
.setEtag(POLICY_ETAG1)
.setVersion(1)
.build();

private static final ServiceAccount SERVICE_ACCOUNT = ServiceAccount.of("test@google.com");
Expand All @@ -302,7 +303,8 @@ public class StorageImplTest {
new Bindings()
.setMembers(ImmutableList.of("user:test1@gmail.com", "user:test2@gmail.com"))
.setRole("roles/storage.objectAdmin")))
.setEtag(POLICY_ETAG1);
.setEtag(POLICY_ETAG1)
.setVersion(1);

private static final String PRIVATE_KEY_STRING =
"MIICdwIBADANBgkqhkiG9w0BAQEFAASCAmEwggJdAgEAAoG"
Expand Down Expand Up @@ -2891,7 +2893,8 @@ public void testSetIamPolicy() {
new Bindings()
.setMembers(ImmutableList.of("group:test-group@gmail.com"))
.setRole("roles/storage.admin")))
.setEtag(POLICY_ETAG1);
.setEtag(POLICY_ETAG1)
.setVersion(1);
// postCommitApiPolicy is identical but for the etag, which has been updated.
com.google.api.services.storage.model.Policy postCommitApiPolicy =
new com.google.api.services.storage.model.Policy()
Expand All @@ -2907,7 +2910,8 @@ public void testSetIamPolicy() {
new Bindings()
.setMembers(ImmutableList.of("group:test-group@gmail.com"))
.setRole("roles/storage.admin")))
.setEtag(POLICY_ETAG2);
.setEtag(POLICY_ETAG2)
.setVersion(1);
Policy postCommitLibPolicy =
Policy.newBuilder()
.addIdentity(StorageRoles.objectViewer(), Identity.allUsers())
Expand All @@ -2917,6 +2921,7 @@ public void testSetIamPolicy() {
Identity.user("test2@gmail.com"))
.addIdentity(StorageRoles.admin(), Identity.group("test-group@gmail.com"))
.setEtag(POLICY_ETAG2)
.setVersion(1)
.build();

EasyMock.expect(storageRpcMock.getIamPolicy(BUCKET_NAME1, EMPTY_RPC_OPTIONS))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.auth.ServiceAccountSigner;
import com.google.auth.http.HttpTransportFactory;
import com.google.auth.oauth2.GoogleCredentials;
import com.google.cloud.Condition;
import com.google.cloud.Identity;
import com.google.cloud.Policy;
import com.google.cloud.ReadChannel;
Expand Down Expand Up @@ -103,6 +104,7 @@
import java.net.URLConnection;
import java.nio.ByteBuffer;
import java.security.Key;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -2344,6 +2346,8 @@ public void testReadCompressedBlob() throws IOException {
public void testBucketPolicy() {
testBucketPolicyRequesterPays(true);
testBucketPolicyRequesterPays(false);
testBucketPolicyV3RequesterPays(true);
testBucketPolicyV3RequesterPays(false);
}

private void testBucketPolicyRequesterPays(boolean requesterPays) {
Expand Down Expand Up @@ -2413,6 +2417,160 @@ private void testBucketPolicyRequesterPays(boolean requesterPays) {
bucketOptions));
}

private void testBucketPolicyV3RequesterPays(boolean requesterPays) {
if (requesterPays) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conditionals around asserts are a code smell. This should be two tests, one with and one without requesterPays.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or two helper methods

Bucket remoteBucket = storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID));
assertNull(remoteBucket.requesterPays());
remoteBucket = remoteBucket.toBuilder().setRequesterPays(true).build();
Bucket updatedBucket = storage.update(remoteBucket);
assertTrue(updatedBucket.requesterPays());
}
// Enable Uniform Bucket-Level Access
storage.update(
BucketInfo.newBuilder(BUCKET)
.setIamConfiguration(
BucketInfo.IamConfiguration.newBuilder()
.setIsUniformBucketLevelAccessEnabled(true)
.build())
.build());
String projectId = remoteStorageHelper.getOptions().getProjectId();

Storage.BucketSourceOption[] bucketOptions =
requesterPays
? new Storage.BucketSourceOption[] {
Storage.BucketSourceOption.requestedPolicyVersion(3),
Storage.BucketSourceOption.userProject(projectId)
}
: new Storage.BucketSourceOption[] {
Storage.BucketSourceOption.requestedPolicyVersion(3)
};
Identity projectOwner = Identity.projectOwner(projectId);
Identity projectEditor = Identity.projectEditor(projectId);
Identity projectViewer = Identity.projectViewer(projectId);
List<com.google.cloud.Binding> bindingsWithoutPublicRead =
ImmutableList.of(
com.google.cloud.Binding.newBuilder()
.setRole(StorageRoles.legacyBucketOwner().toString())
.setMembers(ImmutableList.of(projectEditor.strValue(), projectOwner.strValue()))
.build(),
com.google.cloud.Binding.newBuilder()
.setRole(StorageRoles.legacyBucketReader().toString())
.setMembers(ImmutableList.of(projectViewer.strValue()))
.build());
List<com.google.cloud.Binding> bindingsWithPublicRead =
ImmutableList.of(
com.google.cloud.Binding.newBuilder()
.setRole(StorageRoles.legacyBucketReader().toString())
.setMembers(ImmutableList.of(projectViewer.strValue()))
.build(),
com.google.cloud.Binding.newBuilder()
.setRole(StorageRoles.legacyBucketOwner().toString())
.setMembers(ImmutableList.of(projectEditor.strValue(), projectOwner.strValue()))
.build(),
com.google.cloud.Binding.newBuilder()
.setRole(StorageRoles.legacyObjectReader().toString())
.setMembers(ImmutableList.of("allUsers"))
.build());

List<com.google.cloud.Binding> bindingsWithConditionalPolicy =
ImmutableList.of(
com.google.cloud.Binding.newBuilder()
.setRole(StorageRoles.legacyBucketReader().toString())
.setMembers(ImmutableList.of(projectViewer.strValue()))
.build(),
com.google.cloud.Binding.newBuilder()
.setRole(StorageRoles.legacyBucketOwner().toString())
.setMembers(ImmutableList.of(projectEditor.strValue(), projectOwner.strValue()))
.build(),
com.google.cloud.Binding.newBuilder()
.setRole(StorageRoles.legacyObjectReader().toString())
.setMembers(
ImmutableList.of(
"serviceAccount:storage-python@spec-test-ruby-samples.iam.gserviceaccount.com"))
.setCondition(
Condition.newBuilder()
.setTitle("Title")
.setDescription("Description")
.setExpression(
"resource.name.startsWith(\"projects/_/buckets/bucket-name/objects/prefix-a-\")")
.build())
.build());

// Validate getting policy.
Policy currentPolicy = storage.getIamPolicy(BUCKET, bucketOptions);
assertEquals(bindingsWithoutPublicRead, currentPolicy.getBindingsList());

// Validate updating policy.
List<com.google.cloud.Binding> currentBindings = new ArrayList(currentPolicy.getBindingsList());
currentBindings.add(
com.google.cloud.Binding.newBuilder()
.setRole(StorageRoles.legacyObjectReader().getValue())
.addMembers(Identity.allUsers().strValue())
.build());
Policy updatedPolicy =
storage.setIamPolicy(
BUCKET, currentPolicy.toBuilder().setBindings(currentBindings).build(), bucketOptions);
assertTrue(
bindingsWithPublicRead.size() == updatedPolicy.getBindingsList().size()
&& bindingsWithPublicRead.containsAll(updatedPolicy.getBindingsList()));

// Remove a member
List<com.google.cloud.Binding> updatedBindings = new ArrayList(updatedPolicy.getBindingsList());
for (int i = 0; i < updatedBindings.size(); i++) {
com.google.cloud.Binding binding = updatedBindings.get(i);
if (binding.getRole().equals(StorageRoles.legacyObjectReader().toString())) {
List<String> members = new ArrayList(binding.getMembers());
members.remove(Identity.allUsers().strValue());
updatedBindings.set(i, binding.toBuilder().setMembers(members).build());
break;
}
}

updatedPolicy.toBuilder().setBindings(updatedBindings);
Policy revertedPolicy =
storage.setIamPolicy(
BUCKET, updatedPolicy.toBuilder().setBindings(updatedBindings).build(), bucketOptions);

assertEquals(bindingsWithoutPublicRead, revertedPolicy.getBindingsList());
assertTrue(
bindingsWithoutPublicRead.size() == revertedPolicy.getBindingsList().size()
&& bindingsWithoutPublicRead.containsAll(revertedPolicy.getBindingsList()));

// Add Conditional Policy
List<com.google.cloud.Binding> conditionalBindings =
new ArrayList(revertedPolicy.getBindingsList());
conditionalBindings.add(
com.google.cloud.Binding.newBuilder()
.setRole(StorageRoles.legacyObjectReader().toString())
.addMembers(
"serviceAccount:storage-python@spec-test-ruby-samples.iam.gserviceaccount.com")
.setCondition(
Condition.newBuilder()
.setTitle("Title")
.setDescription("Description")
.setExpression(
"resource.name.startsWith(\"projects/_/buckets/bucket-name/objects/prefix-a-\")")
.build())
.build());
Policy conditionalPolicy =
storage.setIamPolicy(
BUCKET,
currentPolicy.toBuilder().setBindings(conditionalBindings).setVersion(3).build(),
bucketOptions);
assertTrue(
bindingsWithConditionalPolicy.size() == conditionalPolicy.getBindingsList().size()
&& bindingsWithConditionalPolicy.containsAll(conditionalPolicy.getBindingsList()));

// Validate testing permissions.
List<Boolean> expectedPermissions = ImmutableList.of(true, true);
assertEquals(
expectedPermissions,
storage.testIamPermissions(
BUCKET,
ImmutableList.of("storage.buckets.getIamPolicy", "storage.buckets.setIamPolicy"),
bucketOptions));
}

@Test
public void testUpdateBucketLabel() {
Bucket remoteBucket = storage.get(BUCKET, Storage.BucketGetOption.fields(BucketField.ID));
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
<github.global.server>github</github.global.server>
<site.installationModule>google-cloud-storage-parent</site.installationModule>
<google.core.version>1.92.5</google.core.version>
<google.core.version>1.92.6-SNAPSHOT</google.core.version>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hard veto on this; we must not depend on snapshots. If this means we have to wait for a release of google core so be it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm doing this only for development*

<google.api-common.version>1.8.1</google.api-common.version>
<junit.version>4.13</junit.version>
<threeten.version>1.4.1</threeten.version>
Expand Down