fix: updates executable response spec for executable-sourced credenti…

…als (googleapis#955) * fix: expiration_time is only required for successful responses when an output file is specified in the credential configuration * fix: updates PluggableAuthCredentials java docs and missing spot in README * fix: doc fix * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
clundin25 · Aug 5, 2022 · 48ff83d · 48ff83d
1 parent dbbcfaf
commit 48ff83d
Show file tree

Hide file tree

Showing 6 changed files with 281 additions and 38 deletions.
diff --git a/README.md b/README.md
@@ -421,11 +421,15 @@ A sample executable error response:
 These are all required fields for an error response. The code and message
 fields will be used by the library as part of the thrown exception.
 
+For successful responses, the `expiration_time` field is only required
+when an output file is specified in the credential configuration. 
+
 Response format fields summary:
   * `version`: The version of the JSON output. Currently only version 1 is supported.
-  * `success`: The status of the response. When true, the response must contain the 3rd party token, 
-    token type, and expiration. The executable must also exit with exit code 0.
-    When false, the response must contain the error code and message fields and exit with a non-zero value.
+  * `success`: When true, the response must contain the 3rd party token and token type. The response must also contain
+    the expiration_time field if an output file was specified in the credential configuration. The executable must also 
+    exit with exit code 0. When false, the response must contain the error code and message fields and exit with a
+    non-zero value.
   * `token_type`: The 3rd party subject token type. Must be *urn:ietf:params:oauth:token-type:jwt*, 
      *urn:ietf:params:oauth:token-type:id_token*, or *urn:ietf:params:oauth:token-type:saml2*.
   * `id_token`: The 3rd party OIDC token.
@@ -435,8 +439,9 @@ Response format fields summary:
   * `message`: The error message.
 
 All response types must include both the `version` and `success` fields.
- * Successful responses must include the `token_type`, `expiration_time`, and one of
-   `id_token` or `saml_response`.
+ * Successful responses must include the `token_type` and one of
+   `id_token` or `saml_response`. The `expiration_time` field must also be present if an output file was specified in
+    the credential configuration. 
  * Error responses must include both the `code` and `message` fields.
 
 The library will populate the following environment variables when the executable is run:

diff --git a/oauth2_http/java/com/google/auth/oauth2/ExecutableResponse.java b/oauth2_http/java/com/google/auth/oauth2/ExecutableResponse.java
@@ -75,14 +75,11 @@ class ExecutableResponse {
             "The executable response is missing the `token_type` field.");
       }
 
-      if (!json.containsKey("expiration_time")) {
-        throw new PluggableAuthException(
-            "INVALID_EXECUTABLE_RESPONSE",
-            "The executable response is missing the `expiration_time` field.");
-      }
-
       this.tokenType = (String) json.get("token_type");
-      this.expirationTime = parseLongField(json.get("expiration_time"));
+
+      if (json.containsKey("expiration_time")) {
+        this.expirationTime = parseLongField(json.get("expiration_time"));
+      }
 
       if (SAML_SUBJECT_TOKEN_TYPE.equals(tokenType)) {
         this.subjectToken = (String) json.get("saml_response");
@@ -132,9 +129,9 @@ boolean isSuccessful() {
     return this.success;
   }
 
-  /** Returns true if the subject token is expired or not present, false otherwise. */
+  /** Returns true if the subject token is expired, false otherwise. */
   boolean isExpired() {
-    return this.expirationTime == null || this.expirationTime <= Instant.now().getEpochSecond();
+    return this.expirationTime != null && this.expirationTime <= Instant.now().getEpochSecond();
   }
 
   /** Returns whether the execution was successful and returned an unexpired token. */

diff --git a/oauth2_http/java/com/google/auth/oauth2/PluggableAuthCredentials.java b/oauth2_http/java/com/google/auth/oauth2/PluggableAuthCredentials.java
@@ -54,9 +54,9 @@
  * <p>Both OIDC and SAML are supported. The executable must adhere to a specific response format
  * defined below.
  *
- * <p>The executable should print out the 3rd party token to STDOUT in JSON format. This is not
- * required when an output_file is specified in the credential source, with the expectation being
- * that the output file will contain the JSON response instead.
+ * <p>The executable must print out the 3rd party token to STDOUT in JSON format. When an
+ * output_file is specified in the credential configuration, the executable must also handle writing
+ * the JSON response to this file.
  *
  * <pre>
  * OIDC response sample:
@@ -85,6 +85,9 @@
  *   "message": "Error message."
  * }
  *
+ * <p> The `expiration_time` field in the JSON response is only required for successful
+ *  responses when an output file was specified in the credential configuration.
+ *
  * The auth libraries will populate certain environment variables that will be accessible by the
  * executable, such as: GOOGLE_EXTERNAL_ACCOUNT_AUDIENCE, GOOGLE_EXTERNAL_ACCOUNT_TOKEN_TYPE,
  * GOOGLE_EXTERNAL_ACCOUNT_INTERACTIVE, GOOGLE_EXTERNAL_ACCOUNT_IMPERSONATED_EMAIL, and

diff --git a/oauth2_http/java/com/google/auth/oauth2/PluggableAuthHandler.java b/oauth2_http/java/com/google/auth/oauth2/PluggableAuthHandler.java
@@ -112,6 +112,18 @@ public String retrieveTokenFromExecutable(ExecutableOptions options) throws IOEx
       executableResponse = getExecutableResponse(options);
     }
 
+    // If an output file is specified, successful responses must contain the `expiration_time`
+    // field.
+    if (options.getOutputFilePath() != null
+        && !options.getOutputFilePath().isEmpty()
+        && executableResponse.isSuccessful()
+        && executableResponse.getExpirationTime() == null) {
+      throw new PluggableAuthException(
+          "INVALID_EXECUTABLE_RESPONSE",
+          "The executable response must contain the `expiration_time` field for successful responses when an "
+              + "output_file has been specified in the configuration.");
+    }
+
     // The executable response includes a version. Validate that the version is compatible
     // with the library.
     if (executableResponse.getVersion() > EXECUTABLE_SUPPORTED_MAX_VERSION) {

diff --git a/oauth2_http/javatests/com/google/auth/oauth2/ExecutableResponseTest.java b/oauth2_http/javatests/com/google/auth/oauth2/ExecutableResponseTest.java
@@ -60,12 +60,27 @@ void constructor_successOidcResponse() throws IOException {
 
     assertTrue(response.isSuccessful());
     assertTrue(response.isValid());
-    assertEquals(1, response.getVersion());
+    assertEquals(EXECUTABLE_SUPPORTED_MAX_VERSION, response.getVersion());
     assertEquals(TOKEN_TYPE_OIDC, response.getTokenType());
     assertEquals(ID_TOKEN, response.getSubjectToken());
     assertEquals(
         Instant.now().getEpochSecond() + EXPIRATION_DURATION, response.getExpirationTime());
-    assertEquals(1, response.getVersion());
+  }
+
+  @Test
+  void constructor_successOidcResponseMissingExpirationTimeField_notExpired() throws IOException {
+    GenericJson jsonResponse = buildOidcResponse();
+    jsonResponse.remove("expiration_time");
+
+    ExecutableResponse response = new ExecutableResponse(jsonResponse);
+
+    assertTrue(response.isSuccessful());
+    assertTrue(response.isValid());
+    assertFalse(response.isExpired());
+    assertEquals(EXECUTABLE_SUPPORTED_MAX_VERSION, response.getVersion());
+    assertEquals(TOKEN_TYPE_OIDC, response.getTokenType());
+    assertEquals(ID_TOKEN, response.getSubjectToken());
+    assertNull(response.getExpirationTime());
   }
 
   @Test
@@ -81,17 +96,33 @@ void constructor_successSamlResponse() throws IOException {
         Instant.now().getEpochSecond() + EXPIRATION_DURATION, response.getExpirationTime());
   }
 
+  @Test
+  void constructor_successSamlResponseMissingExpirationTimeField_notExpired() throws IOException {
+    GenericJson jsonResponse = buildSamlResponse();
+    jsonResponse.remove("expiration_time");
+
+    ExecutableResponse response = new ExecutableResponse(jsonResponse);
+
+    assertTrue(response.isSuccessful());
+    assertTrue(response.isValid());
+    assertFalse(response.isExpired());
+    assertEquals(EXECUTABLE_SUPPORTED_MAX_VERSION, response.getVersion());
+    assertEquals(TOKEN_TYPE_SAML, response.getTokenType());
+    assertEquals(SAML_RESPONSE, response.getSubjectToken());
+    assertNull(response.getExpirationTime());
+  }
+
   @Test
   void constructor_validErrorResponse() throws IOException {
     ExecutableResponse response = new ExecutableResponse(buildErrorResponse());
 
     assertFalse(response.isSuccessful());
     assertFalse(response.isValid());
-    assertTrue(response.isExpired());
+    assertFalse(response.isExpired());
     assertNull(response.getSubjectToken());
     assertNull(response.getTokenType());
     assertNull(response.getExpirationTime());
-    assertEquals(1, response.getVersion());
+    assertEquals(EXECUTABLE_SUPPORTED_MAX_VERSION, response.getVersion());
     assertEquals("401", response.getErrorCode());
     assertEquals("Caller not authorized.", response.getErrorMessage());
   }
@@ -189,23 +220,6 @@ void constructor_successResponseMissingTokenTypeField_throws() {
         exception.getMessage());
   }
 
-  @Test
-  void constructor_successResponseMissingExpirationTimeField_throws() {
-    GenericJson jsonResponse = buildOidcResponse();
-    jsonResponse.remove("expiration_time");
-
-    PluggableAuthException exception =
-        assertThrows(
-            PluggableAuthException.class,
-            () -> new ExecutableResponse(jsonResponse),
-            "Exception should be thrown.");
-
-    assertEquals(
-        "Error code INVALID_EXECUTABLE_RESPONSE: The executable response is missing the "
-            + "`expiration_time` field.",
-        exception.getMessage());
-  }
-
   @Test
   void constructor_samlResponseMissingSubjectToken_throws() {
     GenericJson jsonResponse = buildSamlResponse();