Skip to content

Commit

Permalink
fix: expiration_time is only required for successful responses when a…
Browse files Browse the repository at this point in the history 
…n output file is specified in the credential configuration
  • Loading branch information
@lsirac
lsirac committed Jul 28, 2022
1 parent ca1f7a6 commit 5582fe1
Show file tree
Hide file tree
Showing 5 changed files with 271 additions and 32 deletions.
8 changes: 6 additions & 2 deletions README.md
View file
Expand Up @@ -415,6 +415,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,
Expand All @@ -429,8 +432,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:
Expand Down
15 changes: 6 additions & 9 deletions oauth2_http/java/com/google/auth/oauth2/ExecutableResponse.java
View file
Expand Up @@ -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");
Expand Down Expand Up @@ -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. */
Expand Down
12 changes: 12 additions & 0 deletions oauth2_http/java/com/google/auth/oauth2/PluggableAuthHandler.java
View file
Expand Up @@ -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) {
Expand Down
56 changes: 35 additions & 21 deletions oauth2_http/javatests/com/google/auth/oauth2/ExecutableResponseTest.java
View file
Expand Up @@ -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
Expand All @@ -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());
}
Expand Down Expand Up @@ -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();
Expand Down
212 changes: 212 additions & 0 deletions oauth2_http/javatests/com/google/auth/oauth2/PluggableAuthHandlerTest.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 5582fe1

Please sign in to comment.