Keep Durable Functions system keys out of management URLs

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

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<String, String> 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<String, String> requiredHeaders) {
+        Map<String, String> headers = new LinkedHashMap<>();
+        if (requiredHeaders != null) {
+            requiredHeaders.forEach((name, value) -> headers.put(name.toLowerCase(Locale.ROOT), value));
+        }
+
+        List<String> 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<String, String> 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);
+        }
     }
 
     /**

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<Object> 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;
+    }
+}

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<String, String> 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());
+    }
 }