fix: allow unset/null privateKeyId for JwtCredentials (#336)

chingor13 · web-flow · commit d28a6ed08a52 · 2019-08-22T10:32:54.000-07:00
* fix: add failing test for unset/null privateKeyId

* fix: relax preconditions to allow null privateKeyId

* chore: fix lint

* fix: remove invalid test
diff --git a/oauth2_http/java/com/google/auth/oauth2/JwtCredentials.java b/oauth2_http/java/com/google/auth/oauth2/JwtCredentials.java
@@ -86,7 +86,7 @@ public class JwtCredentials extends Credentials implements JwtProvider {
 
   private JwtCredentials(Builder builder) {
     this.privateKey = Preconditions.checkNotNull(builder.getPrivateKey());
-    this.privateKeyId = Preconditions.checkNotNull(builder.getPrivateKeyId());
+    this.privateKeyId = builder.getPrivateKeyId();
     this.jwtClaims = Preconditions.checkNotNull(builder.getJwtClaims());
     Preconditions.checkState(jwtClaims.isComplete(), JWT_INCOMPLETE_ERROR_MESSAGE);
     this.lifeSpanSeconds = Preconditions.checkNotNull(builder.getLifeSpanSeconds());
@@ -220,7 +220,7 @@ public PrivateKey getPrivateKey() {
     }
 
     public Builder setPrivateKeyId(String privateKeyId) {
-      this.privateKeyId = Preconditions.checkNotNull(privateKeyId);
+      this.privateKeyId = privateKeyId;
       return this;
     }
 
diff --git a/oauth2_http/javatests/com/google/auth/oauth2/JwtCredentialsTest.java b/oauth2_http/javatests/com/google/auth/oauth2/JwtCredentialsTest.java
@@ -114,22 +114,6 @@ public void builder_requiresPrivateKey() {
     }
   }
 
-  @Test
-  public void builder_requiresPrivateKeyId() {
-    try {
-      JwtClaims claims =
-          JwtClaims.newBuilder()
-              .setAudience("some-audience")
-              .setIssuer("some-issuer")
-              .setSubject("some-subject")
-              .build();
-      JwtCredentials.newBuilder().setJwtClaims(claims).setPrivateKey(getPrivateKey()).build();
-      fail("Should throw exception");
-    } catch (NullPointerException ex) {
-      // expected
-    }
-  }
-
   @Test
   public void builder_requiresClaims() {
     try {
@@ -248,6 +232,40 @@ public void getRequestMetadata_withAdditionalClaims_hasJwtAccess() throws IOExce
         Collections.singletonMap("foo", "bar"));
   }
 
+  @Test
+  public void privateKeyIdNull() throws IOException {
+    JwtClaims claims =
+        JwtClaims.newBuilder()
+            .setAudience("some-audience")
+            .setIssuer("some-issuer")
+            .setSubject("some-subject")
+            .build();
+    JwtCredentials credentials =
+        JwtCredentials.newBuilder()
+            .setJwtClaims(claims)
+            .setPrivateKey(getPrivateKey())
+            .setPrivateKeyId(null)
+            .build();
+
+    Map<String, List<String>> metadata = credentials.getRequestMetadata();
+    verifyJwtAccess(metadata, "some-audience", "some-issuer", "some-subject", null);
+  }
+
+  @Test
+  public void privateKeyIdNotSpecified() throws IOException {
+    JwtClaims claims =
+        JwtClaims.newBuilder()
+            .setAudience("some-audience")
+            .setIssuer("some-issuer")
+            .setSubject("some-subject")
+            .build();
+    JwtCredentials credentials =
+        JwtCredentials.newBuilder().setJwtClaims(claims).setPrivateKey(getPrivateKey()).build();
+
+    Map<String, List<String>> metadata = credentials.getRequestMetadata();
+    verifyJwtAccess(metadata, "some-audience", "some-issuer", "some-subject", null);
+  }
+
   private void verifyJwtAccess(
       Map<String, List<String>> metadata,
       String expectedAudience,

Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,7 @@ public class JwtCredentials extends Credentials implements JwtProvider {`
`86`	`86`
`87`	`87`	`private JwtCredentials(Builder builder) {`
`88`	`88`	`this.privateKey = Preconditions.checkNotNull(builder.getPrivateKey());`
`89`		`- this.privateKeyId = Preconditions.checkNotNull(builder.getPrivateKeyId());`
	`89`	`+ this.privateKeyId = builder.getPrivateKeyId();`
`90`	`90`	`this.jwtClaims = Preconditions.checkNotNull(builder.getJwtClaims());`
`91`	`91`	`Preconditions.checkState(jwtClaims.isComplete(), JWT_INCOMPLETE_ERROR_MESSAGE);`
`92`	`92`	`this.lifeSpanSeconds = Preconditions.checkNotNull(builder.getLifeSpanSeconds());`
`@@ -220,7 +220,7 @@ public PrivateKey getPrivateKey() {`
`220`	`220`	`}`
`221`	`221`
`222`	`222`	`public Builder setPrivateKeyId(String privateKeyId) {`
`223`		`- this.privateKeyId = Preconditions.checkNotNull(privateKeyId);`
	`223`	`+ this.privateKeyId = privateKeyId;`
`224`	`224`	`return this;`
`225`	`225`	`}`
`226`	`226`