fix: updates executable response spec for executable-sourced credentials

README.md

@@ -421,6 +421,9 @@ 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, 
@@ -435,8 +438,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:

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. */

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) {

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();

oauth2_http/javatests/com/google/auth/oauth2/PluggableAuthHandlerTest.java

@@ -51,7 +51,9 @@
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.time.Instant;
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import javax.annotation.Nullable;
@@ -218,6 +220,216 @@ void retrieveTokenFromExecutable_errorResponse_throws() throws InterruptedExcept
     assertEquals("Caller not authorized.", e.getErrorDescription());
   }
 
+  @Test
+  void retrieveTokenFromExecutable_successResponseWithoutExpirationTimeField()
+      throws InterruptedException, IOException {
+    TestEnvironmentProvider environmentProvider = new TestEnvironmentProvider();
+    environmentProvider.setEnv("GOOGLE_EXTERNAL_ACCOUNT_ALLOW_EXECUTABLES", "1");
+
+    // Expected environment mappings.
+    HashMap<String, String> expectedMap = new HashMap<>();
+    expectedMap.putAll(DEFAULT_OPTIONS.getEnvironmentMap());
+
+    Map<String, String> currentEnv = new HashMap<>();
+
+    // Mock executable handling.
+    Process mockProcess = Mockito.mock(Process.class);
+    when(mockProcess.waitFor(anyLong(), any(TimeUnit.class))).thenReturn(true);
+    when(mockProcess.exitValue()).thenReturn(EXIT_CODE_SUCCESS);
+
+    // Remove expiration_time from the executable responses.
+    GenericJson oidcResponse = buildOidcResponse();
+    oidcResponse.remove("expiration_time");
+
+    GenericJson samlResponse = buildSamlResponse();
+    samlResponse.remove("expiration_time");
+
+    List<GenericJson> responses = Arrays.asList(oidcResponse, samlResponse);
+    for (int i = 0; i < responses.size(); i++) {
+      when(mockProcess.getInputStream())
+          .thenReturn(
+              new ByteArrayInputStream(
+                  responses.get(i).toString().getBytes(StandardCharsets.UTF_8)));
+
+      InternalProcessBuilder processBuilder =
+          buildInternalProcessBuilder(
+              currentEnv, mockProcess, DEFAULT_OPTIONS.getExecutableCommand());
+
+      PluggableAuthHandler handler = new PluggableAuthHandler(environmentProvider, processBuilder);
+
+      // Call retrieveTokenFromExecutable().
+      String token = handler.retrieveTokenFromExecutable(DEFAULT_OPTIONS);
+
+      verify(mockProcess, times(i + 1)).destroy();
+      verify(mockProcess, times(i + 1))
+          .waitFor(
+              eq(Long.valueOf(DEFAULT_OPTIONS.getExecutableTimeoutMs())),
+              eq(TimeUnit.MILLISECONDS));
+
+      if (responses.get(i).equals(oidcResponse)) {
+        assertEquals(ID_TOKEN, token);
+      } else {
+        assertEquals(SAML_RESPONSE, token);
+      }
+
+      // Current env map should have the mappings from options.
+      assertEquals(2, currentEnv.size());
+      assertEquals(expectedMap, currentEnv);
+    }
+  }
+
+  @Test
+  void
+      retrieveTokenFromExecutable_successResponseWithoutExpirationTimeFieldWithOutputFileSpecified_throws()
+          throws InterruptedException, IOException {
+    TestEnvironmentProvider environmentProvider = new TestEnvironmentProvider();
+    environmentProvider.setEnv("GOOGLE_EXTERNAL_ACCOUNT_ALLOW_EXECUTABLES", "1");
+
+    // Options with output file specified.
+    ExecutableOptions options =
+        new ExecutableOptions() {
+          @Override
+          public String getExecutableCommand() {
+            return "/path/to/executable";
+          }
+
+          @Override
+          public Map<String, String> getEnvironmentMap() {
+            return ImmutableMap.of();
+          }
+
+          @Override
+          public int getExecutableTimeoutMs() {
+            return 30000;
+          }
+
+          @Override
+          public String getOutputFilePath() {
+            return "/path/to/output/file";
+          }
+        };
+
+    // Mock executable handling.
+    Process mockProcess = Mockito.mock(Process.class);
+    when(mockProcess.waitFor(anyLong(), any(TimeUnit.class))).thenReturn(true);
+    when(mockProcess.exitValue()).thenReturn(EXIT_CODE_SUCCESS);
+
+    // Remove expiration_time from the executable responses.
+    GenericJson oidcResponse = buildOidcResponse();
+    oidcResponse.remove("expiration_time");
+
+    GenericJson samlResponse = buildSamlResponse();
+    samlResponse.remove("expiration_time");
+
+    List<GenericJson> responses = Arrays.asList(oidcResponse, samlResponse);
+    for (int i = 0; i < responses.size(); i++) {
+      when(mockProcess.getInputStream())
+          .thenReturn(
+              new ByteArrayInputStream(
+                  responses.get(i).toString().getBytes(StandardCharsets.UTF_8)));
+
+      InternalProcessBuilder processBuilder =
+          buildInternalProcessBuilder(new HashMap<>(), mockProcess, options.getExecutableCommand());
+
+      PluggableAuthHandler handler = new PluggableAuthHandler(environmentProvider, processBuilder);
+
+      // Call retrieveTokenFromExecutable() should throw an exception as the STDOUT response
+      // is missing
+      // the `expiration_time` field and an output file was specified in the configuration.
+      PluggableAuthException exception =
+          assertThrows(
+              PluggableAuthException.class,
+              () -> handler.retrieveTokenFromExecutable(options),
+              "Exception should be thrown.");
+
+      assertEquals(
+          "Error code 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.",
+          exception.getMessage());
+
+      verify(mockProcess, times(i + 1)).destroy();
+      verify(mockProcess, times(i + 1))
+          .waitFor(eq(Long.valueOf(options.getExecutableTimeoutMs())), eq(TimeUnit.MILLISECONDS));
+    }
+  }
+
+  @Test
+  void retrieveTokenFromExecutable_successResponseInOutputFileMissingExpirationTimeField_throws()
+      throws InterruptedException, IOException {
+    TestEnvironmentProvider environmentProvider = new TestEnvironmentProvider();
+    environmentProvider.setEnv("GOOGLE_EXTERNAL_ACCOUNT_ALLOW_EXECUTABLES", "1");
+
+    // Build output_file.
+    File file = File.createTempFile("output_file", /* suffix= */ null, /* directory= */ null);
+    file.deleteOnExit();
+
+    // Options with output file specified.
+    ExecutableOptions options =
+        new ExecutableOptions() {
+          @Override
+          public String getExecutableCommand() {
+            return "/path/to/executable";
+          }
+
+          @Override
+          public Map<String, String> getEnvironmentMap() {
+            return ImmutableMap.of();
+          }
+
+          @Override
+          public int getExecutableTimeoutMs() {
+            return 30000;
+          }
+
+          @Override
+          public String getOutputFilePath() {
+            return file.getAbsolutePath();
+          }
+        };
+
+    // Mock executable handling that does nothing since we are using the output file.
+    Process mockProcess = Mockito.mock(Process.class);
+    InternalProcessBuilder processBuilder =
+        buildInternalProcessBuilder(new HashMap<>(), mockProcess, options.getExecutableCommand());
+
+    // Remove expiration_time from the executable responses.
+    GenericJson oidcResponse = buildOidcResponse();
+    oidcResponse.remove("expiration_time");
+
+    GenericJson samlResponse = buildSamlResponse();
+    samlResponse.remove("expiration_time");
+
+    List<GenericJson> responses = Arrays.asList(oidcResponse, samlResponse);
+    for (GenericJson json : responses) {
+      OAuth2Utils.writeInputStreamToFile(
+          new ByteArrayInputStream(json.toString().getBytes(StandardCharsets.UTF_8)),
+          file.getAbsolutePath());
+
+      PluggableAuthHandler handler = new PluggableAuthHandler(environmentProvider, processBuilder);
+
+      // Call retrieveTokenFromExecutable() which should throw an exception as the output file
+      // response is missing
+      // the `expiration_time` field.
+      PluggableAuthException exception =
+          assertThrows(
+              PluggableAuthException.class,
+              () -> handler.retrieveTokenFromExecutable(options),
+              "Exception should be thrown.");
+
+      assertEquals(
+          "Error code 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.",
+          exception.getMessage());
+
+      // Validate executable not invoked.
+      verify(mockProcess, times(0)).destroyForcibly();
+      verify(mockProcess, times(0))
+          .waitFor(eq(Long.valueOf(options.getExecutableTimeoutMs())), eq(TimeUnit.MILLISECONDS));
+    }
+  }
+
   @Test
   void retrieveTokenFromExecutable_withOutputFile_usesCachedResponse()
       throws IOException, InterruptedException {