From cd356d04e56c4656e0c51a378ec2e7e06f178075 Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Mon, 14 Jun 2021 12:58:24 -0700 Subject: [PATCH 1/9] feat: adds CAB rules classes --- .../auth/oauth2/CredentialAccessBoundary.java | 267 ++++++++++++++++++ .../oauth2/CredentialAccessBoundaryTest.java | 225 +++++++++++++++ oauth2_http/pom.xml | 5 + 3 files changed, 497 insertions(+) create mode 100644 oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java create mode 100644 oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java diff --git a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java new file mode 100644 index 000000000..766878ae0 --- /dev/null +++ b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java @@ -0,0 +1,267 @@ +package com.google.auth.oauth2; + +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.api.client.json.GenericJson; +import java.util.ArrayList; +import java.util.List; +import javax.annotation.Nullable; + +/** Defines an upper bound of permissions available for a GCP credential. */ +final class CredentialAccessBoundary { + + private static final int RULES_SIZE_LIMIT = 10; + + private final List accessBoundaryRules; + + CredentialAccessBoundary(List accessBoundaryRules) { + this.accessBoundaryRules = checkNotNull(accessBoundaryRules); + if (accessBoundaryRules.isEmpty()) { + throw new IllegalArgumentException("At least one access boundary rule must be provided."); + } + if (accessBoundaryRules.size() > RULES_SIZE_LIMIT) { + throw new IllegalArgumentException( + "The provided list has more than 10 access boundary rules."); + } + } + + /** + * Internal method that returns the JSON string representation of the credential access boundary. + */ + String toJson() { + List rules = new ArrayList<>(); + for (AccessBoundaryRule rule : accessBoundaryRules) { + GenericJson ruleJson = new GenericJson(); + ruleJson.setFactory(OAuth2Utils.JSON_FACTORY); + + ruleJson.put("availableResource", rule.getAvailableResource()); + ruleJson.put("availablePermissions", rule.getAvailablePermissions()); + + AccessBoundaryRule.AvailabilityCondition availabilityCondition = + rule.getAvailabilityCondition(); + if (availabilityCondition != null) { + GenericJson availabilityConditionJson = new GenericJson(); + availabilityConditionJson.setFactory(OAuth2Utils.JSON_FACTORY); + + availabilityConditionJson.put("expression", availabilityCondition.getExpression()); + if (availabilityCondition.getTitle() != null) { + availabilityConditionJson.put("title", availabilityCondition.getTitle()); + } + if (availabilityCondition.getDescription() != null) { + availabilityConditionJson.put("description", availabilityCondition.getDescription()); + } + + ruleJson.put("availabilityCondition", availabilityConditionJson); + } + rules.add(ruleJson); + } + GenericJson accessBoundaryRulesJson = new GenericJson(); + accessBoundaryRulesJson.setFactory(OAuth2Utils.JSON_FACTORY); + accessBoundaryRulesJson.put("accessBoundaryRules", rules); + + GenericJson json = new GenericJson(); + json.setFactory(OAuth2Utils.JSON_FACTORY); + json.put("accessBoundary", accessBoundaryRulesJson); + return json.toString(); + } + + public List getAccessBoundaryRules() { + return accessBoundaryRules; + } + + public static Builder newBuilder() { + return new Builder(); + } + + public static class Builder { + private List accessBoundaryRules; + + private Builder() { + accessBoundaryRules = new ArrayList<>(); + } + + /** + * Sets the list of {@link AccessBoundaryRule}'s. + * + *

This list must not exceed 10 rules. + */ + public Builder setRules(List rule) { + accessBoundaryRules = new ArrayList<>(checkNotNull(rule)); + return this; + } + + public CredentialAccessBoundary.Builder addRule(AccessBoundaryRule rule) { + accessBoundaryRules.add(checkNotNull(rule)); + return this; + } + + public CredentialAccessBoundary build() { + return new CredentialAccessBoundary(accessBoundaryRules); + } + } + + /** Defines an upper bound of permissions on a particular resource. */ + public static final class AccessBoundaryRule { + + private final String availableResource; + private final List availablePermissions; + + @Nullable private final AvailabilityCondition availabilityCondition; + + AccessBoundaryRule( + String availableResource, + List availablePermissions, + @Nullable AvailabilityCondition availabilityCondition) { + this.availableResource = checkNotNull(availableResource); + this.availablePermissions = new ArrayList<>(checkNotNull(availablePermissions)); + + for (String permission : availablePermissions) { + if (permission == null || permission.isEmpty()) { + throw new IllegalArgumentException( + "One of the provided available permissions is either null or empty."); + } + } + + this.availabilityCondition = availabilityCondition; + } + + public String getAvailableResource() { + return availableResource; + } + + public List getAvailablePermissions() { + return availablePermissions; + } + + @Nullable + public AvailabilityCondition getAvailabilityCondition() { + return availabilityCondition; + } + + public static Builder newBuilder() { + return new Builder(); + } + + public static class Builder { + private String availableResource; + private List availablePermissions; + + @Nullable private AvailabilityCondition availabilityCondition; + + private Builder() {} + + /** + * Sets the available resource, which is the full resource name of the GCP resource to allow + * access to. + */ + public Builder setAvailableResource(String availableResource) { + this.availableResource = availableResource; + return this; + } + + /** + * Sets the list of permissions that can be used on the resource. This should be a list of IAM + * roles prefixed by inRole. + * + *

e.g. {"inRole:roles/storage.objectViewer"}. + */ + public Builder setAvailablePermissions(List availablePermissions) { + this.availablePermissions = new ArrayList<>(checkNotNull(availablePermissions)); + return this; + } + + /** + * Adds a permission that can be used on the resource. This should be an IAM role prefixed by + * inRole. + * + *

e.g. "inRole:roles/storage.objectViewer". + */ + public Builder addAvailablePermission(String availableResource) { + if (availablePermissions == null) { + availablePermissions = new ArrayList<>(); + } + availablePermissions.add(availableResource); + return this; + } + + /** + * Sets the availability condition which is an IAM condition that defines constraints to apply + * to the token expressed in CEL format. + */ + public Builder setAvailabilityCondition(AvailabilityCondition availabilityCondition) { + this.availabilityCondition = availabilityCondition; + return this; + } + + public AccessBoundaryRule build() { + return new AccessBoundaryRule( + availableResource, availablePermissions, availabilityCondition); + } + } + + /** + * An optional condition that can be used as part of a {@link CredentialAccessBoundary} to + * further restrict permissions. + */ + public static final class AvailabilityCondition { + private final String expression; + + @Nullable private final String title; + @Nullable private final String description; + + AvailabilityCondition( + String expression, @Nullable String title, @Nullable String description) { + this.expression = checkNotNull(expression); + this.title = title; + this.description = description; + } + + public String getExpression() { + return expression; + } + + @Nullable + public String getTitle() { + return title; + } + + @Nullable + public String getDescription() { + return description; + } + + public static Builder newBuilder() { + return new Builder(); + } + + public static final class Builder { + private String expression; + + @Nullable private String title; + @Nullable private String description; + + private Builder() {} + + /** */ + public Builder setExpression(String expression) { + this.expression = expression; + return this; + } + + public Builder setTitle(String title) { + this.title = title; + return this; + } + + public Builder setDescription(String description) { + this.description = description; + return this; + } + + public AvailabilityCondition build() { + return new AvailabilityCondition(expression, title, description); + } + } + } + } +} diff --git a/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java b/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java new file mode 100644 index 000000000..f442d6dc3 --- /dev/null +++ b/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java @@ -0,0 +1,225 @@ +package com.google.auth.oauth2; + +import static org.junit.Assert.*; + +import com.google.auth.oauth2.CredentialAccessBoundary.AccessBoundaryRule; +import com.google.auth.oauth2.CredentialAccessBoundary.AccessBoundaryRule.AvailabilityCondition; +import java.util.Arrays; +import java.util.Collections; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link CredentialAccessBoundary} and encompassing classes. */ +@RunWith(JUnit4.class) +public class CredentialAccessBoundaryTest { + + @Test + public void credentialAccessBoundary() { + AvailabilityCondition availabilityCondition = + AvailabilityCondition.newBuilder().setExpression("expression").build(); + + AccessBoundaryRule firstRule = + AccessBoundaryRule.newBuilder() + .setAvailableResource("firstResource") + .addAvailablePermission("firstPermission") + .setAvailabilityCondition(availabilityCondition) + .build(); + + AccessBoundaryRule secondRule = + AccessBoundaryRule.newBuilder() + .setAvailableResource("secondResource") + .addAvailablePermission("secondPermission") + .build(); + + CredentialAccessBoundary credentialAccessBoundary = + CredentialAccessBoundary.newBuilder() + .setRules(Arrays.asList(firstRule, secondRule)) + .build(); + + assertEquals(2, credentialAccessBoundary.getAccessBoundaryRules().size()); + assertEquals(firstRule, credentialAccessBoundary.getAccessBoundaryRules().get(0)); + assertEquals(secondRule, credentialAccessBoundary.getAccessBoundaryRules().get(1)); + } + + @Test + public void credentialAccessBoundary_withoutRules_throws() { + try { + CredentialAccessBoundary.newBuilder().build(); + fail("Should fail."); + } catch (IllegalArgumentException e) { + assertEquals("At least one access boundary rule must be provided.", e.getMessage()); + } + } + + @Test + public void credentialAccessBoundary_ruleCountExceeded_throws() { + AccessBoundaryRule rule = + AccessBoundaryRule.newBuilder() + .setAvailableResource("resource") + .addAvailablePermission("permission") + .build(); + + CredentialAccessBoundary.Builder builder = CredentialAccessBoundary.newBuilder(); + for (int i = 0; i <= 10; i++) { + builder.addRule(rule); + } + + try { + builder.build(); + fail("Should fail."); + } catch (IllegalArgumentException e) { + assertEquals("The provided list has more than 10 access boundary rules.", e.getMessage()); + } + } + + @Test + public void credentialAccessBoundary_toJson() { + AvailabilityCondition availabilityCondition = + AvailabilityCondition.newBuilder().setExpression("expression").build(); + + AccessBoundaryRule firstRule = + AccessBoundaryRule.newBuilder() + .setAvailableResource("firstResource") + .addAvailablePermission("firstPermission") + .setAvailabilityCondition(availabilityCondition) + .build(); + + AccessBoundaryRule secondRule = + AccessBoundaryRule.newBuilder() + .setAvailableResource("secondResource") + .setAvailablePermissions(Arrays.asList("firstPermission", "secondPermission")) + .build(); + + CredentialAccessBoundary credentialAccessBoundary = + CredentialAccessBoundary.newBuilder() + .setRules(Arrays.asList(firstRule, secondRule)) + .build(); + + String expectedJson = + "{\"accessBoundary\":{\"accessBoundaryRules\":" + + "[{\"availableResource\":\"firstResource\"," + + "\"availablePermissions\":[\"firstPermission\"]," + + "\"availabilityCondition\":{\"expression\":\"expression\"}}," + + "{\"availableResource\":\"secondResource\"," + + "\"availablePermissions\":[\"firstPermission\"," + + "\"secondPermission\"]}]}}"; + assertEquals(expectedJson, credentialAccessBoundary.toJson()); + } + + @Test + public void accessBoundaryRule_allFields() { + AvailabilityCondition availabilityCondition = + AvailabilityCondition.newBuilder().setExpression("expression").build(); + + AccessBoundaryRule rule = + AccessBoundaryRule.newBuilder() + .setAvailableResource("resource") + .addAvailablePermission("firstPermission") + .addAvailablePermission("secondPermission") + .setAvailabilityCondition(availabilityCondition) + .build(); + + assertEquals("resource", rule.getAvailableResource()); + assertEquals(2, rule.getAvailablePermissions().size()); + assertEquals("firstPermission", rule.getAvailablePermissions().get(0)); + assertEquals("secondPermission", rule.getAvailablePermissions().get(1)); + assertEquals(availabilityCondition, rule.getAvailabilityCondition()); + } + + @Test + public void accessBoundaryRule_requiredFields() { + AccessBoundaryRule rule = + AccessBoundaryRule.newBuilder() + .setAvailableResource("resource") + .setAvailablePermissions(Collections.singletonList("firstPermission")) + .build(); + + assertEquals("resource", rule.getAvailableResource()); + assertEquals(1, rule.getAvailablePermissions().size()); + assertEquals("firstPermission", rule.getAvailablePermissions().get(0)); + assertNull(rule.getAvailabilityCondition()); + } + + @Test + public void accessBoundaryRule_withoutAvailableResource_throws() { + try { + AccessBoundaryRule.newBuilder().addAvailablePermission("permission").build(); + fail("Should fail."); + } catch (NullPointerException e) { + // Expected. + } + } + + @Test + public void accessBoundaryRule_withoutAvailablePermissions_throws() { + try { + AccessBoundaryRule.newBuilder().setAvailableResource("resource").build(); + fail("Should fail."); + } catch (NullPointerException e) { + // Expected. + } + } + + @Test + public void accessBoundaryRule_withNullAvailablePermission_throws() { + try { + AccessBoundaryRule.newBuilder() + .setAvailableResource("resource") + .addAvailablePermission(null) + .build(); + fail("Should fail."); + } catch (IllegalArgumentException e) { + assertEquals( + "One of the provided available permissions is either null or empty.", e.getMessage()); + } + } + + @Test + public void accessBoundaryRule_withEmptyAvailablePermission_throws() { + try { + AccessBoundaryRule.newBuilder() + .setAvailableResource("resource") + .addAvailablePermission("") + .build(); + fail("Should fail."); + } catch (IllegalArgumentException e) { + assertEquals( + "One of the provided available permissions is either null or empty.", e.getMessage()); + } + } + + @Test + public void availabilityCondition_allFields() { + AvailabilityCondition availabilityCondition = + AvailabilityCondition.newBuilder() + .setExpression("expression") + .setTitle("title") + .setDescription("description") + .build(); + + assertEquals("expression", availabilityCondition.getExpression()); + assertEquals("title", availabilityCondition.getTitle()); + assertEquals("description", availabilityCondition.getDescription()); + } + + @Test + public void availabilityCondition_expressionOnly() { + AvailabilityCondition availabilityCondition = + AvailabilityCondition.newBuilder().setExpression("expression").build(); + + assertEquals("expression", availabilityCondition.getExpression()); + assertNull(availabilityCondition.getTitle()); + assertNull(availabilityCondition.getDescription()); + } + + @Test + public void availabilityCondition_nullExpression_throws() { + try { + AvailabilityCondition.newBuilder().setExpression(null).build(); + fail("Should fail."); + } catch (NullPointerException e) { + // Expected. + } + } +} diff --git a/oauth2_http/pom.xml b/oauth2_http/pom.xml index 4e42bbcc0..942b0c37a 100644 --- a/oauth2_http/pom.xml +++ b/oauth2_http/pom.xml @@ -147,5 +147,10 @@ 1.3 test + + com.google.auth + google-auth-library-credentials + 0.24.0 + From 7d5d7032a0f538d4d4543aa28d52a7dc3d6daf4a Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Mon, 14 Jun 2021 13:05:19 -0700 Subject: [PATCH 2/9] fix: copyright --- .../auth/oauth2/CredentialAccessBoundary.java | 31 +++++++++++++++++++ .../oauth2/CredentialAccessBoundaryTest.java | 31 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java index 766878ae0..af6c0ad14 100644 --- a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java +++ b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java @@ -1,3 +1,34 @@ +/* + * Copyright 2021 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + package com.google.auth.oauth2; import static com.google.common.base.Preconditions.checkNotNull; diff --git a/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java b/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java index f442d6dc3..72c748b59 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java @@ -1,3 +1,34 @@ +/* + * Copyright 2021 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + package com.google.auth.oauth2; import static org.junit.Assert.*; From 2a6f3d3ada0f43debc0393b08713fcb059316fcf Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Mon, 14 Jun 2021 13:20:02 -0700 Subject: [PATCH 3/9] fix: revert pom --- oauth2_http/pom.xml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/oauth2_http/pom.xml b/oauth2_http/pom.xml index 942b0c37a..4e42bbcc0 100644 --- a/oauth2_http/pom.xml +++ b/oauth2_http/pom.xml @@ -147,10 +147,5 @@ 1.3 test - - com.google.auth - google-auth-library-credentials - 0.24.0 - From f7292e9eb463ef0d5753590865334fdbedff928b Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Thu, 17 Jun 2021 13:43:36 -0700 Subject: [PATCH 4/9] fix: review --- .../auth/oauth2/CredentialAccessBoundary.java | 63 +++++++++++++++---- .../oauth2/CredentialAccessBoundaryTest.java | 63 ++++++++++++++++--- 2 files changed, 105 insertions(+), 21 deletions(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java index af6c0ad14..f03c1768b 100644 --- a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java +++ b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java @@ -31,14 +31,20 @@ package com.google.auth.oauth2; -import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.*; import com.google.api.client.json.GenericJson; import java.util.ArrayList; import java.util.List; import javax.annotation.Nullable; -/** Defines an upper bound of permissions available for a GCP credential. */ +/** + * Defines an upper bound of permissions available for a GCP credential via {@link + * AccessBoundaryRule}s. + * + *

See for more + * information. + */ final class CredentialAccessBoundary { private static final int RULES_SIZE_LIMIT = 10; @@ -52,7 +58,8 @@ final class CredentialAccessBoundary { } if (accessBoundaryRules.size() > RULES_SIZE_LIMIT) { throw new IllegalArgumentException( - "The provided list has more than 10 access boundary rules."); + String.format( + "The provided list has more than %s access boundary rules.", RULES_SIZE_LIMIT)); } } @@ -131,7 +138,19 @@ public CredentialAccessBoundary build() { } } - /** Defines an upper bound of permissions on a particular resource. */ + /** + * Defines an upper bound of permissions on a particular resource. + * + *

The following snippet shows an AccessBoundaryRule that applies to the GCS bucket bucket-one + * to set the upper bound of permissions to those defined by the roles/storage.objectViewer role. + * + *


+   * AccessBoundaryRule rule = AccessBoundaryRule.newBuilder()
+   *   .setAvailableResource("//storage.googleapis.com/projects/_/buckets/bucket-one")
+   *   .addAvailablePermission("inRole:roles/storage.objectViewer")
+   *   .build();
+   * 
+ */ public static final class AccessBoundaryRule { private final String availableResource; @@ -143,9 +162,12 @@ public static final class AccessBoundaryRule { String availableResource, List availablePermissions, @Nullable AvailabilityCondition availabilityCondition) { - this.availableResource = checkNotNull(availableResource); - this.availablePermissions = new ArrayList<>(checkNotNull(availablePermissions)); + checkArgument( + availableResource != null && !availableResource.isEmpty(), + "The provided availableResource is either null or empty."); + this.availableResource = availableResource; + this.availablePermissions = new ArrayList<>(checkNotNull(availablePermissions)); for (String permission : availablePermissions) { if (permission == null || permission.isEmpty()) { throw new IllegalArgumentException( @@ -184,6 +206,8 @@ private Builder() {} /** * Sets the available resource, which is the full resource name of the GCP resource to allow * access to. + * + *

For example: "//storage.googleapis.com/projects/_/buckets/example". */ public Builder setAvailableResource(String availableResource) { this.availableResource = availableResource; @@ -194,7 +218,7 @@ public Builder setAvailableResource(String availableResource) { * Sets the list of permissions that can be used on the resource. This should be a list of IAM * roles prefixed by inRole. * - *

e.g. {"inRole:roles/storage.objectViewer"}. + *

For example: {"inRole:roles/storage.objectViewer"}. */ public Builder setAvailablePermissions(List availablePermissions) { this.availablePermissions = new ArrayList<>(checkNotNull(availablePermissions)); @@ -205,7 +229,7 @@ public Builder setAvailablePermissions(List availablePermissions) { * Adds a permission that can be used on the resource. This should be an IAM role prefixed by * inRole. * - *

e.g. "inRole:roles/storage.objectViewer". + *

For example: "inRole:roles/storage.objectViewer". */ public Builder addAvailablePermission(String availableResource) { if (availablePermissions == null) { @@ -231,8 +255,21 @@ public AccessBoundaryRule build() { } /** - * An optional condition that can be used as part of a {@link CredentialAccessBoundary} to - * further restrict permissions. + * An optional condition that can be used as part of a {@link AccessBoundaryRule} to further + * restrict permissions. + * + *

For example, you can define an AvailabilityCondition that applies to a set of GCS objects + * whose names start with auth: + * + *


+     * AvailabilityCondition availabilityCondition = AvailabilityCondition.newBuilder()
+     *   .setExpression("resource.name.startsWith('projects/_/buckets/bucket-123/objects/auth')")
+     *   .build();
+     * 
+ * + * The expression is defined in Common Expression Language (CEL) format. See + * for more information. */ public static final class AvailabilityCondition { private final String expression; @@ -242,7 +279,10 @@ public static final class AvailabilityCondition { AvailabilityCondition( String expression, @Nullable String title, @Nullable String description) { - this.expression = checkNotNull(expression); + checkArgument( + expression != null && !expression.isEmpty(), + "The provided expression is null or empty."); + this.expression = expression; this.title = title; this.description = description; } @@ -273,7 +313,6 @@ public static final class Builder { private Builder() {} - /** */ public Builder setExpression(String expression) { this.expression = expression; return this; diff --git a/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java b/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java index 72c748b59..e8ec55941 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java @@ -31,7 +31,9 @@ package com.google.auth.oauth2; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; import com.google.auth.oauth2.CredentialAccessBoundary.AccessBoundaryRule; import com.google.auth.oauth2.CredentialAccessBoundary.AccessBoundaryRule.AvailabilityCondition; @@ -69,8 +71,23 @@ public void credentialAccessBoundary() { .build(); assertEquals(2, credentialAccessBoundary.getAccessBoundaryRules().size()); - assertEquals(firstRule, credentialAccessBoundary.getAccessBoundaryRules().get(0)); - assertEquals(secondRule, credentialAccessBoundary.getAccessBoundaryRules().get(1)); + + AccessBoundaryRule first = credentialAccessBoundary.getAccessBoundaryRules().get(0); + assertEquals(firstRule, first); + assertEquals("firstResource", first.getAvailableResource()); + assertEquals(1, first.getAvailablePermissions().size()); + assertEquals("firstPermission", first.getAvailablePermissions().get(0)); + assertEquals(availabilityCondition, first.getAvailabilityCondition()); + assertEquals("expression", first.getAvailabilityCondition().getExpression()); + assertNull(first.getAvailabilityCondition().getTitle()); + assertNull(first.getAvailabilityCondition().getDescription()); + + AccessBoundaryRule second = credentialAccessBoundary.getAccessBoundaryRules().get(1); + assertEquals(secondRule, second); + assertEquals("secondResource", second.getAvailableResource()); + assertEquals(1, second.getAvailablePermissions().size()); + assertEquals("secondPermission", second.getAvailablePermissions().get(0)); + assertNull(second.getAvailabilityCondition()); } @Test @@ -107,7 +124,11 @@ public void credentialAccessBoundary_ruleCountExceeded_throws() { @Test public void credentialAccessBoundary_toJson() { AvailabilityCondition availabilityCondition = - AvailabilityCondition.newBuilder().setExpression("expression").build(); + AvailabilityCondition.newBuilder() + .setExpression("expression") + .setTitle("title") + .setDescription("description") + .build(); AccessBoundaryRule firstRule = AccessBoundaryRule.newBuilder() @@ -131,7 +152,8 @@ public void credentialAccessBoundary_toJson() { "{\"accessBoundary\":{\"accessBoundaryRules\":" + "[{\"availableResource\":\"firstResource\"," + "\"availablePermissions\":[\"firstPermission\"]," - + "\"availabilityCondition\":{\"expression\":\"expression\"}}," + + "\"availabilityCondition\":{\"expression\":\"expression\"," + + "\"title\":\"title\",\"description\":\"description\"}}," + "{\"availableResource\":\"secondResource\"," + "\"availablePermissions\":[\"firstPermission\"," + "\"secondPermission\"]}]}}"; @@ -172,13 +194,26 @@ public void accessBoundaryRule_requiredFields() { assertNull(rule.getAvailabilityCondition()); } + @Test + public void accessBoundaryRule_withEmptyAvailableResource_throws() { + try { + AccessBoundaryRule.newBuilder() + .setAvailableResource("") + .addAvailablePermission("permission") + .build(); + fail("Should fail."); + } catch (IllegalArgumentException e) { + assertEquals("The provided availableResource is either null or empty.", e.getMessage()); + } + } + @Test public void accessBoundaryRule_withoutAvailableResource_throws() { try { AccessBoundaryRule.newBuilder().addAvailablePermission("permission").build(); fail("Should fail."); - } catch (NullPointerException e) { - // Expected. + } catch (IllegalArgumentException e) { + assertEquals("The provided availableResource is either null or empty.", e.getMessage()); } } @@ -249,8 +284,18 @@ public void availabilityCondition_nullExpression_throws() { try { AvailabilityCondition.newBuilder().setExpression(null).build(); fail("Should fail."); - } catch (NullPointerException e) { - // Expected. + } catch (IllegalArgumentException e) { + assertEquals("The provided expression is null or empty.", e.getMessage()); + } + } + + @Test + public void availabilityCondition_emptyExpression_throws() { + try { + AvailabilityCondition.newBuilder().setExpression("").build(); + fail("Should fail."); + } catch (IllegalArgumentException e) { + assertEquals("The provided expression is null or empty.", e.getMessage()); } } } From 9e953ed4bfadaa8ddfd043b392a4f82b9b71721a Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Thu, 17 Jun 2021 14:19:47 -0700 Subject: [PATCH 5/9] fix: bad link --- .../java/com/google/auth/oauth2/CredentialAccessBoundary.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java index f03c1768b..3fa6170d4 100644 --- a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java +++ b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java @@ -268,8 +268,7 @@ public AccessBoundaryRule build() { * * * The expression is defined in Common Expression Language (CEL) format. See - * for more information. + * href='https://cloud.google.com/iam/docs/conditions-overview#cel'>for more information. */ public static final class AvailabilityCondition { private final String expression; From 0620b38f484cffd3823cbaa7edd1d946bbe1b980 Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Fri, 18 Jun 2021 10:44:04 -0700 Subject: [PATCH 6/9] fix: more null and empty checks --- .../auth/oauth2/CredentialAccessBoundary.java | 9 ++++++--- .../oauth2/CredentialAccessBoundaryTest.java | 19 +++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java index 3fa6170d4..e825c636b 100644 --- a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java +++ b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java @@ -167,7 +167,10 @@ public static final class AccessBoundaryRule { "The provided availableResource is either null or empty."); this.availableResource = availableResource; - this.availablePermissions = new ArrayList<>(checkNotNull(availablePermissions)); + checkArgument( + availablePermissions != null && !availablePermissions.isEmpty(), + "The list of provided availablePermissions is either null or empty."); + this.availablePermissions = new ArrayList<>(availablePermissions); for (String permission : availablePermissions) { if (permission == null || permission.isEmpty()) { throw new IllegalArgumentException( @@ -231,11 +234,11 @@ public Builder setAvailablePermissions(List availablePermissions) { * *

For example: "inRole:roles/storage.objectViewer". */ - public Builder addAvailablePermission(String availableResource) { + public Builder addAvailablePermission(String availablePermission) { if (availablePermissions == null) { availablePermissions = new ArrayList<>(); } - availablePermissions.add(availableResource); + availablePermissions.add(availablePermission); return this; } diff --git a/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java b/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java index e8ec55941..8faa70a55 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java @@ -37,6 +37,7 @@ import com.google.auth.oauth2.CredentialAccessBoundary.AccessBoundaryRule; import com.google.auth.oauth2.CredentialAccessBoundary.AccessBoundaryRule.AvailabilityCondition; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import org.junit.Test; @@ -222,13 +223,27 @@ public void accessBoundaryRule_withoutAvailablePermissions_throws() { try { AccessBoundaryRule.newBuilder().setAvailableResource("resource").build(); fail("Should fail."); - } catch (NullPointerException e) { + } catch (IllegalArgumentException e) { // Expected. } } @Test - public void accessBoundaryRule_withNullAvailablePermission_throws() { + public void accessBoundaryRule_withEmptyAvailablePermissions_throws() { + try { + AccessBoundaryRule.newBuilder() + .setAvailableResource("resource") + .setAvailablePermissions(new ArrayList()) + .build(); + fail("Should fail."); + } catch (IllegalArgumentException e) { + assertEquals( + "The list of provided availablePermissions is either null or empty.", e.getMessage()); + } + } + + @Test + public void accessBoundaryRule_withNullAvailablePermissions_throws() { try { AccessBoundaryRule.newBuilder() .setAvailableResource("resource") From 92005da2952d7109d144acf74c69af2cc844c9b9 Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Fri, 18 Jun 2021 10:53:52 -0700 Subject: [PATCH 7/9] fix: expand javadoc --- .../auth/oauth2/CredentialAccessBoundary.java | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java index e825c636b..616dbf29e 100644 --- a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java +++ b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java @@ -141,8 +141,9 @@ public CredentialAccessBoundary build() { /** * Defines an upper bound of permissions on a particular resource. * - *

The following snippet shows an AccessBoundaryRule that applies to the GCS bucket bucket-one - * to set the upper bound of permissions to those defined by the roles/storage.objectViewer role. + *

The following snippet shows an AccessBoundaryRule that applies to the Cloud Storage bucket + * bucket-one to set the upper bound of permissions to those defined by the + * roles/storage.objectViewer role. * *


    * AccessBoundaryRule rule = AccessBoundaryRule.newBuilder()
@@ -261,17 +262,14 @@ public AccessBoundaryRule build() {
      * An optional condition that can be used as part of a {@link AccessBoundaryRule} to further
      * restrict permissions.
      *
-     * 

For example, you can define an AvailabilityCondition that applies to a set of GCS objects - * whose names start with auth: + *

For example, you can define an AvailabilityCondition that applies to a set of Cloud + * Storage objects whose names start with auth: * *


      * AvailabilityCondition availabilityCondition = AvailabilityCondition.newBuilder()
      *   .setExpression("resource.name.startsWith('projects/_/buckets/bucket-123/objects/auth')")
      *   .build();
      * 
- * - * The expression is defined in Common Expression Language (CEL) format. See for more information. */ public static final class AvailabilityCondition { private final String expression; @@ -315,16 +313,26 @@ public static final class Builder { private Builder() {} + /** + * Sets the required expression which must be defined in Common Expression Language (CEL) + * format. + * + *

This expression specifies the Cloud Storage object where permissions are available. + * See for more + * information. + */ public Builder setExpression(String expression) { this.expression = expression; return this; } + /** Sets the optional title that identifies the purpose of the condition. */ public Builder setTitle(String title) { this.title = title; return this; } + /** Sets the description that details the purpose of the condition. */ public Builder setDescription(String description) { this.description = description; return this; From 2b280336ab1f0512200a776035533cb584d455ec Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Fri, 18 Jun 2021 12:02:06 -0700 Subject: [PATCH 8/9] fix: split null/empty checks --- .../auth/oauth2/CredentialAccessBoundary.java | 54 +++++++++---------- .../oauth2/CredentialAccessBoundaryTest.java | 29 ++++++---- 2 files changed, 45 insertions(+), 38 deletions(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java index 616dbf29e..4b40b5a9c 100644 --- a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java +++ b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java @@ -45,22 +45,21 @@ *

See for more * information. */ -final class CredentialAccessBoundary { +public final class CredentialAccessBoundary { private static final int RULES_SIZE_LIMIT = 10; private final List accessBoundaryRules; CredentialAccessBoundary(List accessBoundaryRules) { - this.accessBoundaryRules = checkNotNull(accessBoundaryRules); - if (accessBoundaryRules.isEmpty()) { - throw new IllegalArgumentException("At least one access boundary rule must be provided."); - } - if (accessBoundaryRules.size() > RULES_SIZE_LIMIT) { - throw new IllegalArgumentException( - String.format( - "The provided list has more than %s access boundary rules.", RULES_SIZE_LIMIT)); - } + checkArgument(accessBoundaryRules != null, "The provided list of accessBoundaryRules is null."); + checkArgument( + !accessBoundaryRules.isEmpty(), "At least one access boundary rule must be provided."); + checkArgument( + accessBoundaryRules.size() < RULES_SIZE_LIMIT, + String.format( + "The provided list has more than %s access boundary rules.", RULES_SIZE_LIMIT)); + this.accessBoundaryRules = accessBoundaryRules; } /** @@ -114,9 +113,7 @@ public static Builder newBuilder() { public static class Builder { private List accessBoundaryRules; - private Builder() { - accessBoundaryRules = new ArrayList<>(); - } + private Builder() {} /** * Sets the list of {@link AccessBoundaryRule}'s. @@ -129,6 +126,9 @@ public Builder setRules(List rule) { } public CredentialAccessBoundary.Builder addRule(AccessBoundaryRule rule) { + if (accessBoundaryRules == null) { + accessBoundaryRules = new ArrayList<>(); + } accessBoundaryRules.add(checkNotNull(rule)); return this; } @@ -163,22 +163,22 @@ public static final class AccessBoundaryRule { String availableResource, List availablePermissions, @Nullable AvailabilityCondition availabilityCondition) { + checkArgument(availableResource != null, "The provided availableResource is null."); + checkArgument(!availableResource.isEmpty(), "The provided availableResource is empty."); checkArgument( - availableResource != null && !availableResource.isEmpty(), - "The provided availableResource is either null or empty."); - this.availableResource = availableResource; - + availablePermissions != null, "The list of provided availablePermissions is null."); checkArgument( - availablePermissions != null && !availablePermissions.isEmpty(), - "The list of provided availablePermissions is either null or empty."); - this.availablePermissions = new ArrayList<>(availablePermissions); + !availablePermissions.isEmpty(), "The list of provided availablePermissions is empty."); for (String permission : availablePermissions) { - if (permission == null || permission.isEmpty()) { - throw new IllegalArgumentException( - "One of the provided available permissions is either null or empty."); + if (permission == null) { + throw new IllegalArgumentException("One of the provided available permissions is null."); + } + if (permission.isEmpty()) { + throw new IllegalArgumentException("One of the provided available permissions is empty."); } } - + this.availableResource = availableResource; + this.availablePermissions = new ArrayList<>(availablePermissions); this.availabilityCondition = availabilityCondition; } @@ -279,12 +279,12 @@ public static final class AvailabilityCondition { AvailabilityCondition( String expression, @Nullable String title, @Nullable String description) { - checkArgument( - expression != null && !expression.isEmpty(), - "The provided expression is null or empty."); this.expression = expression; this.title = title; this.description = description; + + checkArgument(expression != null, "The provided expression is null."); + checkArgument(!expression.isEmpty(), "The provided expression is empty."); } public String getExpression() { diff --git a/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java b/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java index 8faa70a55..32e5a09ed 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java @@ -92,10 +92,20 @@ public void credentialAccessBoundary() { } @Test - public void credentialAccessBoundary_withoutRules_throws() { + public void credentialAccessBoundary_nullRules_throws() { try { CredentialAccessBoundary.newBuilder().build(); fail("Should fail."); + } catch (IllegalArgumentException e) { + assertEquals("The provided list of accessBoundaryRules is null.", e.getMessage()); + } + } + + @Test + public void credentialAccessBoundary_withoutRules_throws() { + try { + CredentialAccessBoundary.newBuilder().setRules(new ArrayList()).build(); + fail("Should fail."); } catch (IllegalArgumentException e) { assertEquals("At least one access boundary rule must be provided.", e.getMessage()); } @@ -204,7 +214,7 @@ public void accessBoundaryRule_withEmptyAvailableResource_throws() { .build(); fail("Should fail."); } catch (IllegalArgumentException e) { - assertEquals("The provided availableResource is either null or empty.", e.getMessage()); + assertEquals("The provided availableResource is empty.", e.getMessage()); } } @@ -214,7 +224,7 @@ public void accessBoundaryRule_withoutAvailableResource_throws() { AccessBoundaryRule.newBuilder().addAvailablePermission("permission").build(); fail("Should fail."); } catch (IllegalArgumentException e) { - assertEquals("The provided availableResource is either null or empty.", e.getMessage()); + assertEquals("The provided availableResource is null.", e.getMessage()); } } @@ -237,8 +247,7 @@ public void accessBoundaryRule_withEmptyAvailablePermissions_throws() { .build(); fail("Should fail."); } catch (IllegalArgumentException e) { - assertEquals( - "The list of provided availablePermissions is either null or empty.", e.getMessage()); + assertEquals("The list of provided availablePermissions is empty.", e.getMessage()); } } @@ -251,8 +260,7 @@ public void accessBoundaryRule_withNullAvailablePermissions_throws() { .build(); fail("Should fail."); } catch (IllegalArgumentException e) { - assertEquals( - "One of the provided available permissions is either null or empty.", e.getMessage()); + assertEquals("One of the provided available permissions is null.", e.getMessage()); } } @@ -265,8 +273,7 @@ public void accessBoundaryRule_withEmptyAvailablePermission_throws() { .build(); fail("Should fail."); } catch (IllegalArgumentException e) { - assertEquals( - "One of the provided available permissions is either null or empty.", e.getMessage()); + assertEquals("One of the provided available permissions is empty.", e.getMessage()); } } @@ -300,7 +307,7 @@ public void availabilityCondition_nullExpression_throws() { AvailabilityCondition.newBuilder().setExpression(null).build(); fail("Should fail."); } catch (IllegalArgumentException e) { - assertEquals("The provided expression is null or empty.", e.getMessage()); + assertEquals("The provided expression is null.", e.getMessage()); } } @@ -310,7 +317,7 @@ public void availabilityCondition_emptyExpression_throws() { AvailabilityCondition.newBuilder().setExpression("").build(); fail("Should fail."); } catch (IllegalArgumentException e) { - assertEquals("The provided expression is null or empty.", e.getMessage()); + assertEquals("The provided expression is empty.", e.getMessage()); } } } From 4b8f27645e4f9a78f72cb2a765ab5af9ca051ae9 Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Fri, 18 Jun 2021 14:13:02 -0700 Subject: [PATCH 9/9] fix: use checkNotNull --- .../auth/oauth2/CredentialAccessBoundary.java | 18 ++++++++---------- .../oauth2/CredentialAccessBoundaryTest.java | 14 +++++++------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java index 4b40b5a9c..92cacc258 100644 --- a/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java +++ b/oauth2_http/java/com/google/auth/oauth2/CredentialAccessBoundary.java @@ -31,7 +31,8 @@ package com.google.auth.oauth2; -import static com.google.common.base.Preconditions.*; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; import com.google.api.client.json.GenericJson; import java.util.ArrayList; @@ -52,7 +53,7 @@ public final class CredentialAccessBoundary { private final List accessBoundaryRules; CredentialAccessBoundary(List accessBoundaryRules) { - checkArgument(accessBoundaryRules != null, "The provided list of accessBoundaryRules is null."); + checkNotNull(accessBoundaryRules); checkArgument( !accessBoundaryRules.isEmpty(), "At least one access boundary rule must be provided."); checkArgument( @@ -163,10 +164,11 @@ public static final class AccessBoundaryRule { String availableResource, List availablePermissions, @Nullable AvailabilityCondition availabilityCondition) { - checkArgument(availableResource != null, "The provided availableResource is null."); + this.availableResource = checkNotNull(availableResource); + this.availablePermissions = new ArrayList<>(checkNotNull(availablePermissions)); + this.availabilityCondition = availabilityCondition; + checkArgument(!availableResource.isEmpty(), "The provided availableResource is empty."); - checkArgument( - availablePermissions != null, "The list of provided availablePermissions is null."); checkArgument( !availablePermissions.isEmpty(), "The list of provided availablePermissions is empty."); for (String permission : availablePermissions) { @@ -177,9 +179,6 @@ public static final class AccessBoundaryRule { throw new IllegalArgumentException("One of the provided available permissions is empty."); } } - this.availableResource = availableResource; - this.availablePermissions = new ArrayList<>(availablePermissions); - this.availabilityCondition = availabilityCondition; } public String getAvailableResource() { @@ -279,11 +278,10 @@ public static final class AvailabilityCondition { AvailabilityCondition( String expression, @Nullable String title, @Nullable String description) { - this.expression = expression; + this.expression = checkNotNull(expression); this.title = title; this.description = description; - checkArgument(expression != null, "The provided expression is null."); checkArgument(!expression.isEmpty(), "The provided expression is empty."); } diff --git a/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java b/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java index 32e5a09ed..ac042e065 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/CredentialAccessBoundaryTest.java @@ -96,8 +96,8 @@ public void credentialAccessBoundary_nullRules_throws() { try { CredentialAccessBoundary.newBuilder().build(); fail("Should fail."); - } catch (IllegalArgumentException e) { - assertEquals("The provided list of accessBoundaryRules is null.", e.getMessage()); + } catch (NullPointerException e) { + // Expected. } } @@ -223,8 +223,8 @@ public void accessBoundaryRule_withoutAvailableResource_throws() { try { AccessBoundaryRule.newBuilder().addAvailablePermission("permission").build(); fail("Should fail."); - } catch (IllegalArgumentException e) { - assertEquals("The provided availableResource is null.", e.getMessage()); + } catch (NullPointerException e) { + // Expected. } } @@ -233,7 +233,7 @@ public void accessBoundaryRule_withoutAvailablePermissions_throws() { try { AccessBoundaryRule.newBuilder().setAvailableResource("resource").build(); fail("Should fail."); - } catch (IllegalArgumentException e) { + } catch (NullPointerException e) { // Expected. } } @@ -306,8 +306,8 @@ public void availabilityCondition_nullExpression_throws() { try { AvailabilityCondition.newBuilder().setExpression(null).build(); fail("Should fail."); - } catch (IllegalArgumentException e) { - assertEquals("The provided expression is null.", e.getMessage()); + } catch (NullPointerException e) { + // Expected. } }