diff --git a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/DurableClientContext.java b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/DurableClientContext.java index 34d7d549..1a1fa7c3 100644 --- a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/DurableClientContext.java +++ b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/DurableClientContext.java @@ -25,6 +25,7 @@ import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.util.Map; import java.util.concurrent.TimeoutException; /** @@ -35,6 +36,7 @@ public class DurableClientContext { private String rpcBaseUrl; private String taskHubName; private String requiredQueryStringParameters; + private Map requiredHeaders; private DurableTaskClient client; /** @@ -120,11 +122,12 @@ public HttpResponseMessage createCheckStatusResponse(HttpRequestMessage reque // More info: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded. // One potential workaround is to set ASPNETCORE_FORWARDEDHEADERS_ENABLED to true. - // Construct the response as an HTTP 202 with a JSON object payload + HttpManagementPayload payload = this.getClientResponseLinks(request, instanceId); return request.createResponseBuilder(HttpStatus.ACCEPTED) - .header("Location", this.getInstanceStatusURL(request, instanceId) + "?" + this.requiredQueryStringParameters) + .header("Location", payload.getStatusQueryGetUri()) .header("Content-Type", "application/json") - .body(this.getClientResponseLinks(request, instanceId)) + .header("Cache-Control", "no-store") + .body(payload) .build(); } @@ -215,7 +218,7 @@ public CleanEntityStorageResult cleanEntityStorage(CleanEntityStorageRequest req private HttpManagementPayload getClientResponseLinks(HttpRequestMessage request, String instanceId) { String instanceStatusURL = this.getInstanceStatusURL(request, instanceId); - return new HttpManagementPayload(instanceId, instanceStatusURL, this.requiredQueryStringParameters); + return new HttpManagementPayload(instanceId, instanceStatusURL, this.requiredQueryStringParameters, this.requiredHeaders); } private String getInstanceStatusURL(HttpRequestMessage request, String instanceId) { diff --git a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/HttpManagementPayload.java b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/HttpManagementPayload.java index 43e16ac9..46a80324 100644 --- a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/HttpManagementPayload.java +++ b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/HttpManagementPayload.java @@ -6,6 +6,16 @@ package com.microsoft.durabletask.azurefunctions; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; + /** * The class that holds the webhook URLs to manage orchestration instances. */ @@ -19,6 +29,7 @@ public class HttpManagementPayload { private final String terminatePostUri; private final String resumePostUri; private final String suspendPostUri; + private final Map requiredHeaders; /** * Creates a {@link HttpManagementPayload} to manage orchestration instances @@ -31,15 +42,81 @@ public HttpManagementPayload( String instanceId, String instanceStatusURL, String requiredQueryStringParameters) { + this(instanceId, instanceStatusURL, requiredQueryStringParameters, Collections.emptyMap()); + } + + /** + * Creates management URLs with authentication supplied separately from the query string. + * + * @param instanceId The ID of the orchestration instance + * @param instanceStatusURL The base webhook URL of the orchestration instance + * @param requiredQueryStringParameters The non-secret query parameters to include with requests + * @param requiredHeaders The headers callers must send when using the management URLs + */ + public HttpManagementPayload( + String instanceId, + String instanceStatusURL, + String requiredQueryStringParameters, + Map requiredHeaders) { + Map headers = new LinkedHashMap<>(); + if (requiredHeaders != null) { + requiredHeaders.forEach((name, value) -> headers.put(name.toLowerCase(Locale.ROOT), value)); + } + + List parameters = new ArrayList<>(); + String query = requiredQueryStringParameters == null ? "" : requiredQueryStringParameters; + if (query.startsWith("?")) { + query = query.substring(1); + } + + // Older hosts include the system key in the binding query string. + for (String parameter : query.split("&")) { + if (parameter.isEmpty()) { + continue; + } + + int separator = parameter.indexOf('='); + String name = separator < 0 ? parameter : parameter.substring(0, separator); + if (decode(name).equalsIgnoreCase("code")) { + if (separator >= 0) { + headers.putIfAbsent("x-functions-key", decode(parameter.substring(separator + 1))); + } + } else { + parameters.add(parameter); + } + } + + String queryString = String.join("&", parameters); + String querySuffix = queryString.isEmpty() ? "" : "?" + queryString; + String reasonSuffix = "?reason={text}" + (queryString.isEmpty() ? "" : "&" + queryString); this.id = instanceId; - this.purgeHistoryDeleteUri = instanceStatusURL + "?" + requiredQueryStringParameters; - this.restartPostUri = instanceStatusURL + "/restart?" + requiredQueryStringParameters; - this.rewindPostUri = instanceStatusURL + "/rewind?reason={text}&" + requiredQueryStringParameters; - this.sendEventPostUri = instanceStatusURL + "/raiseEvent/{eventName}?" + requiredQueryStringParameters; - this.statusQueryGetUri = instanceStatusURL + "?" + requiredQueryStringParameters; - this.terminatePostUri = instanceStatusURL + "/terminate?reason={text}&" + requiredQueryStringParameters; - this.resumePostUri = instanceStatusURL + "/resume?reason={text}&" + requiredQueryStringParameters; - this.suspendPostUri = instanceStatusURL + "/suspend?reason={text}&" + requiredQueryStringParameters; + this.requiredHeaders = Collections.unmodifiableMap(headers); + this.purgeHistoryDeleteUri = instanceStatusURL + querySuffix; + this.restartPostUri = instanceStatusURL + "/restart" + querySuffix; + this.rewindPostUri = instanceStatusURL + "/rewind" + reasonSuffix; + this.sendEventPostUri = instanceStatusURL + "/raiseEvent/{eventName}" + querySuffix; + this.statusQueryGetUri = instanceStatusURL + querySuffix; + this.terminatePostUri = instanceStatusURL + "/terminate" + reasonSuffix; + this.resumePostUri = instanceStatusURL + "/resume" + reasonSuffix; + this.suspendPostUri = instanceStatusURL + "/suspend" + reasonSuffix; + } + + /** + * Gets the headers callers must send when using the management URLs. + * Treat these values as secrets; do not log them or append them to URLs. + * + * @return An immutable map of required HTTP request headers. + */ + public Map getRequiredHeaders() { + return this.requiredHeaders; + } + + private static String decode(String value) { + try { + return URLDecoder.decode(value, StandardCharsets.UTF_8.name()); + } catch (UnsupportedEncodingException ex) { + throw new IllegalStateException("UTF-8 encoding is unavailable.", ex); + } } /** diff --git a/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/DurableClientContextTest.java b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/DurableClientContextTest.java new file mode 100644 index 00000000..363ab6f1 --- /dev/null +++ b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/DurableClientContextTest.java @@ -0,0 +1,79 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.durabletask.azurefunctions; + +import com.google.gson.Gson; +import com.google.gson.JsonObject; +import com.microsoft.azure.functions.HttpRequestMessage; +import com.microsoft.azure.functions.HttpResponseMessage; +import com.microsoft.azure.functions.HttpStatus; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +import java.net.URI; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +class DurableClientContextTest { + private final Gson gson = new Gson(); + + @Test + void bindingHeadersAreIncludedInPayloadAndLocationIsKeyFree() { + String binding = "{\"rpcBaseUrl\":\"http://127.0.0.1:12345\"," + + "\"requiredQueryStringParameters\":\"taskHub=hub&connection=storage\"," + + "\"requiredHeaders\":{\"x-functions-key\":\"synthetic-key\"}}"; + assertCheckStatusResponse(binding, "synthetic-key", + "?taskHub=hub&connection=storage"); + } + + @Test + void legacyBindingQueryKeyIsConvertedToHeaderMetadata() { + String binding = "{\"requiredQueryStringParameters\":\"CoDe=synthetic%2Bkey&site=example\"}"; + assertCheckStatusResponse(binding, "synthetic+key", "?site=example"); + } + + @Test + void bindingCredentialsRemainScopedToEachContext() { + DurableClientContext first = this.gson.fromJson( + "{\"requiredHeaders\":{\"x-functions-key\":\"first\"}}", DurableClientContext.class); + DurableClientContext second = this.gson.fromJson( + "{\"requiredHeaders\":{\"x-functions-key\":\"second\"}}", DurableClientContext.class); + HttpRequestMessage request = request(); + + assertEquals("first", first.createHttpManagementPayload(request, "instance") + .getRequiredHeaders().get("x-functions-key")); + assertEquals("second", second.createHttpManagementPayload(request, "instance") + .getRequiredHeaders().get("x-functions-key")); + } + + private void assertCheckStatusResponse(String binding, String key, String query) { + DurableClientContext context = this.gson.fromJson(binding, DurableClientContext.class); + HttpRequestMessage request = request(); + HttpResponseMessage.Builder builder = mock(HttpResponseMessage.Builder.class, RETURNS_SELF); + HttpResponseMessage response = mock(HttpResponseMessage.class); + when(request.createResponseBuilder(HttpStatus.ACCEPTED)).thenReturn(builder); + when(builder.build()).thenReturn(response); + + assertSame(response, context.createCheckStatusResponse(request, "instance")); + + ArgumentCaptor body = ArgumentCaptor.forClass(Object.class); + verify(builder).body(body.capture()); + HttpManagementPayload payload = (HttpManagementPayload) body.getValue(); + String expectedUrl = "https://example.com/runtime/webhooks/durabletask/instances/instance" + query; + assertEquals(expectedUrl, payload.getStatusQueryGetUri()); + verify(builder).header("Location", expectedUrl); + verify(builder).header("Cache-Control", "no-store"); + JsonObject json = this.gson.toJsonTree(payload).getAsJsonObject(); + assertEquals(key, json.getAsJsonObject("requiredHeaders").get("x-functions-key").getAsString()); + assertFalse(payload.getStatusQueryGetUri().contains("code=")); + assertFalse(payload.getStatusQueryGetUri().contains("synthetic")); + } + + private static HttpRequestMessage request() { + HttpRequestMessage request = mock(HttpRequestMessage.class); + when(request.getUri()).thenReturn(URI.create("https://example.com/api/start?code=starter-key")); + return request; + } +} diff --git a/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/HttpManagementPayloadTest.java b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/HttpManagementPayloadTest.java index 2798164b..e5ca980d 100644 --- a/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/HttpManagementPayloadTest.java +++ b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/HttpManagementPayloadTest.java @@ -2,6 +2,13 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import static org.junit.jupiter.api.Assertions.*; @@ -12,7 +19,7 @@ public class HttpManagementPayloadTest { private static final String INSTANCE_ID = "test-instance-id"; private static final String INSTANCE_STATUS_URL = "http://localhost:7071/runtime/webhooks/durabletask/instances/test-instance-id"; - private static final String QUERY_STRING = "code=abc123"; + private static final String QUERY_STRING = "taskHub=hub&connection=storage"; private HttpManagementPayload createPayload() { return new HttpManagementPayload(INSTANCE_ID, INSTANCE_STATUS_URL, QUERY_STRING); @@ -80,4 +87,49 @@ public void getRewindPostUri_ReturnsCorrectUrl() { HttpManagementPayload payload = createPayload(); assertEquals(INSTANCE_STATUS_URL + "/rewind?reason={text}&" + QUERY_STRING, payload.getRewindPostUri()); } + + @ParameterizedTest + @ValueSource(strings = { + "code=synthetic%2Bkey%2F%3D&site=value%2Bone", + "site=value%2Bone&CoDe=synthetic%2Bkey%2F%3D", + "%63ode=synthetic%2Bkey%2F%3D&site=value%2Bone&CODE=duplicate" + }) + void legacyQueryKeyIsMovedToRequiredHeaders(String query) { + HttpManagementPayload payload = new HttpManagementPayload(INSTANCE_ID, INSTANCE_STATUS_URL, query); + + assertEquals("synthetic+key/=", payload.getRequiredHeaders().get("x-functions-key")); + assertEquals(INSTANCE_STATUS_URL + "?site=value%2Bone", payload.getStatusQueryGetUri()); + for (String url : Arrays.asList( + payload.getStatusQueryGetUri(), payload.getSendEventPostUri(), payload.getTerminatePostUri(), + payload.getRewindPostUri(), payload.getPurgeHistoryDeleteUri(), payload.getRestartPostUri(), + payload.getSuspendPostUri(), payload.getResumePostUri())) { + assertTrue(url.contains("site=value%2Bone")); + assertFalse(url.toLowerCase().contains("code=")); + assertFalse(url.contains("synthetic")); + assertFalse(url.contains("duplicate")); + } + } + + @Test + void explicitHeadersOverrideLegacyKeyAndAreDefensivelyCopied() { + Map headers = new HashMap<>(); + headers.put("X-Functions-Key", "current-key"); + HttpManagementPayload payload = new HttpManagementPayload( + INSTANCE_ID, INSTANCE_STATUS_URL, "code=old-key&" + QUERY_STRING, headers); + headers.put("X-Functions-Key", "changed"); + + assertEquals(Collections.singletonMap("x-functions-key", "current-key"), payload.getRequiredHeaders()); + assertThrows(UnsupportedOperationException.class, () -> payload.getRequiredHeaders().put("extra", "value")); + assertEquals(INSTANCE_STATUS_URL + "?" + QUERY_STRING, payload.getStatusQueryGetUri()); + } + + @Test + void missingOrEmptyQueryDoesNotAddNullOrTrailingSeparators() { + for (String query : new String[] { null, "", "code=synthetic-key" }) { + HttpManagementPayload payload = new HttpManagementPayload(INSTANCE_ID, INSTANCE_STATUS_URL, query); + assertEquals(INSTANCE_STATUS_URL, payload.getStatusQueryGetUri()); + assertEquals(INSTANCE_STATUS_URL + "/terminate?reason={text}", payload.getTerminatePostUri()); + } + assertTrue(createPayload().getRequiredHeaders().isEmpty()); + } } diff --git a/endtoendtests/src/test/java/com/functions/EndToEndTests.java b/endtoendtests/src/test/java/com/functions/EndToEndTests.java index f596ed4e..7eb300e2 100644 --- a/endtoendtests/src/test/java/com/functions/EndToEndTests.java +++ b/endtoendtests/src/test/java/com/functions/EndToEndTests.java @@ -4,6 +4,7 @@ import io.restassured.http.ContentType; import io.restassured.path.json.JsonPath; import io.restassured.response.Response; +import io.restassured.specification.RequestSpecification; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Tag; @@ -11,21 +12,24 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import java.net.URI; import java.time.Duration; import java.time.Instant; import java.util.HashSet; +import java.util.Collections; +import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; -import static io.restassured.RestAssured.get; -import static io.restassured.RestAssured.post; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @Tag("e2e") public class EndToEndTests { + private Map requiredHeaders = Collections.emptyMap(); @BeforeAll public static void setup() { @@ -70,6 +74,26 @@ public void generalFunctions(String functionName) throws InterruptedException { assertTrue(pass); } + @Test + public void managementAuthenticationIsSeparateFromUrls() throws InterruptedException { + Response response = post("/api/StartOrchestration"); + assertEquals(202, response.statusCode()); + assertEquals("no-store", response.header("Cache-Control")); + JsonPath management = response.jsonPath(); + assertTrue(this.requiredHeaders.containsKey("x-functions-key")); + Map payload = management.getMap(""); + for (Map.Entry entry : payload.entrySet()) { + if (entry.getKey().endsWith("Uri")) { + String url = (String) entry.getValue(); + assertTrue(!url.toLowerCase(java.util.Locale.ROOT).contains("code=")); + assertTrue(!url.contains(this.requiredHeaders.get("x-functions-key"))); + } + } + String statusUrl = management.getString("statusQueryGetUri"); + assertEquals(statusUrl, response.header("Location")); + assertTrue(pollingCheck(statusUrl, "Completed", null, Duration.ofSeconds(30))); + } + @Test public void retryTest() throws InterruptedException { String startOrchestrationPath = "/api/RetriableOrchestration"; @@ -132,8 +156,7 @@ public void continueAsNewExternalEvent() throws InterruptedException { sendEventPostUri = sendEventPostUri.replace("{eventName}", "event"); // empty request body - RestAssured - .given() + requestFor(sendEventPostUri) .contentType(ContentType.JSON) // Set the request content type .body("{}") .post(sendEventPostUri) @@ -167,7 +190,9 @@ public void restart(boolean restartWithNewInstanceId) throws InterruptedExceptio Response statusResponse = get(statusQueryGetUri); String instanceId = statusResponse.jsonPath().get("instanceId"); - String restartPostUri = jsonPath.get("restartPostUri") + "&restartWithNewInstanceId=" + restartWithNewInstanceId; + String restartUrl = jsonPath.get("restartPostUri"); + String restartPostUri = restartUrl + (restartUrl.contains("?") ? "&" : "?") + + "restartWithNewInstanceId=" + restartWithNewInstanceId; Response restartResponse = post(restartPostUri); JsonPath restartJsonPath = restartResponse.jsonPath(); String restartStatusQueryGetUri = restartJsonPath.get("statusQueryGetUri"); @@ -216,8 +241,7 @@ public void suspendResume() throws InterruptedException { sendEventPostUri = sendEventPostUri.replace("{eventName}", "test"); String requestBody = "{\"value\":\"Test\"}"; - RestAssured - .given() + requestFor(sendEventPostUri) .contentType(ContentType.JSON) // Set the request content type .body(requestBody) // Set the request body .post(sendEventPostUri) @@ -311,8 +335,7 @@ public void externalEventDeserializeFail() throws InterruptedException { sendEventPostUri = sendEventPostUri.replace("{eventName}", "event"); String requestBody = "{\"value\":\"Test\"}"; - RestAssured - .given() + requestFor(sendEventPostUri) .contentType(ContentType.JSON) // Set the request content type .body(requestBody) // Set the request body .post(sendEventPostUri) @@ -590,6 +613,42 @@ public void callHttp() throws InterruptedException { assertEquals(200, output); } + private RequestSpecification requestFor(String url) { + RequestSpecification request = RestAssured.given().redirects().follow(false); + if (url.startsWith("http://") || url.startsWith("https://")) { + int queryStart = url.indexOf('?'); + URI target = URI.create(queryStart < 0 ? url : url.substring(0, queryStart)); + URI base = URI.create(RestAssured.baseURI); + assertEquals(base.getScheme(), target.getScheme(), "Unexpected management URL scheme"); + assertEquals(base.getHost(), target.getHost(), "Unexpected management URL host"); + int port = target.getPort() < 0 + ? ("https".equals(target.getScheme()) ? 443 : 80) : target.getPort(); + assertEquals(RestAssured.port, port, "Unexpected management URL port"); + assertTrue(target.getPath().startsWith("/runtime/webhooks/durabletask/")); + request.headers(this.requiredHeaders); + } + return request; + } + + private Response get(String url) { + return requestFor(url).get(url); + } + + private Response post(String url, Object... pathParameters) { + Response response = requestFor(url).post(url, pathParameters); + if (response.statusCode() == 202 && response.contentType() != null + && response.contentType().contains("application/json") + && !response.getBody().asString().isEmpty()) { + JsonPath payload = response.jsonPath(); + if (payload.get("statusQueryGetUri") != null) { + Map headers = payload.getMap("requiredHeaders"); + assertNotNull(headers, "Management responses must include requiredHeaders"); + this.requiredHeaders = headers; + } + } + return response; + } + private boolean pollingCheck(String statusQueryGetUri, String expectedState, Set continueStates,