From db1c3da5bcd16ba541ad7c6bd99503c2c713af2e Mon Sep 17 00:00:00 2001 From: browndav Date: Wed, 10 Jun 2026 16:55:39 -0400 Subject: [PATCH 1/5] fix bug in storage session credential with get content length 0 --- .../util/StorageSessionCredential.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/StorageSessionCredential.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/StorageSessionCredential.java index d74a4ebc5792..89006929ea8a 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/StorageSessionCredential.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/StorageSessionCredential.java @@ -56,26 +56,31 @@ void signRequest(HttpRequest request) { request.setHeader(HttpHeaderName.AUTHORIZATION, SESSION_PREFIX + sessionToken + ":" + signature); } - // Mirrors StorageSharedKeyCredential.buildStringToSign but does NOT replace "0" with "" for - // Content-Length. The Session protocol signs the literal value the wire carries. + // Mirrors StorageSharedKeyCredential.buildStringToSign. The server canonicalizes + // Content-Length: 0 to "" before computing its HMAC (matching the documented Shared Key + // canonicalization), so we must do the same here to produce a matching signature. // - // We inline this rather than delegate to StorageSharedKeyCredential because of a quirk in - // azure-core's RestProxyBase.configRequest (sdk/core/azure-core/src/main/java/com/azure/core/ - // implementation/http/rest/RestProxyBase.java, line 305): it unconditionally calls - // `request.setHeader(HttpHeaderName.CONTENT_LENGTH, "0")` for body-less requests including - // GETs (an RFC 7230 violation; .NET's transports skip it). SharedKey's canonicalization - // then normalizes "0" -> "" in the string-to-sign, but the server signs the literal "0" it - // sees on the wire, so delegating produces a signature mismatch. - // - // TODO: once RestProxyBase.java:305 is changed to skip Content-Length: 0 for GET/DELETE, - // delete this method and delegate to sharedKey.generateAuthorizationHeader(...). - // This matches what happens in dotnet: - // https://github.com/Azure/azure-sdk-for-net/blob/57598097b0ba056de7d90e5b1624d6c529cd3d60/sdk/core/Azure.Core/src/Pipeline/HttpWebRequestTransport.cs#L94-L99 + // TODO (azure-core, RFC hygiene only — does NOT affect Storage signing correctness): + // azure-core's RestProxyBase.configRequest (sdk/core/azure-core/.../RestProxyBase.java) + // unconditionally sets Content-Length: 0 on body-less requests, including GETs. Per + // RFC 7230 §3.3.2 a user agent SHOULD NOT send a Content-Length header when the request + // has no body and the method does not anticipate one (.NET's transports skip it). This + // does NOT cause a signing mismatch here — the server normalizes "0" -> "" and our local + // normalization above matches — so it is purely an RFC-hygiene issue. The Content-Length + // normalization in this method should remain in place even if azure-core is fixed: it + // reflects the documented Shared Key canonicalization rule, not a workaround for + // azure-core behavior. Track the azure-core fix separately if pursued. + private String buildStringToSign(HttpRequest request) { HttpHeaders headers = request.getHeaders(); Collator collator = Collator.getInstance(Locale.ROOT); String contentLength = getHeaderOrEmpty(headers, HttpHeaderName.CONTENT_LENGTH); + // Normalize "0" to "" to match the server's canonicalization (matches + // StorageSharedKeyCredential.buildStringToSign). + if ("0".equals(contentLength)) { + contentLength = ""; + } // If x-ms-date is present, the Date slot is empty. String dateHeader = headers.getValue(X_MS_DATE) != null ? "" : getHeaderOrEmpty(headers, HttpHeaderName.DATE); From 314c99c159d715ab66280c166c467badeda9e6e1 Mon Sep 17 00:00:00 2001 From: browndav Date: Wed, 10 Jun 2026 17:02:18 -0400 Subject: [PATCH 2/5] patch to enable session with environement variables --- .../azure/storage/blob/BlobClientBuilder.java | 10 + .../blob/BlobContainerClientBuilder.java | 10 + .../blob/BlobServiceClientBuilder.java | 12 +- .../implementation/util/BuilderHelper.java | 64 ++++++ .../storage/blob/BuilderHelperTests.java | 190 ++++++++++++++++++ .../azure/storage/blob/ContainerApiTests.java | 57 ++++++ .../storage/blob/ContainerAsyncApiTests.java | 58 ++++++ .../SessionTokenCredentialPolicyTest.java | 23 ++- .../util/StorageSessionCredentialTest.java | 13 +- 9 files changed, 412 insertions(+), 25 deletions(-) diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobClientBuilder.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobClientBuilder.java index a099f0aab5c5..6df5f792d0d4 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobClientBuilder.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobClientBuilder.java @@ -136,6 +136,11 @@ public BlobClient buildClient() { new IllegalArgumentException("Customer provided key and encryption " + "scope cannot both be set")); } + BuilderHelper.applyEnvironmentSessionDefaults(sessionOptions, configuration, LOGGER); + if (CoreUtils.isNullOrEmpty(containerName) && !CoreUtils.isNullOrEmpty(sessionOptions.getContainerName())) { + containerName = sessionOptions.getContainerName(); + } + BuilderHelper.validateSessionMode(sessionOptions, containerName, LOGGER); /* @@ -185,6 +190,11 @@ public BlobAsyncClient buildAsyncClient() { new IllegalArgumentException("Customer provided key and encryption " + "scope cannot both be set")); } + BuilderHelper.applyEnvironmentSessionDefaults(sessionOptions, configuration, LOGGER); + if (CoreUtils.isNullOrEmpty(containerName) && !CoreUtils.isNullOrEmpty(sessionOptions.getContainerName())) { + containerName = sessionOptions.getContainerName(); + } + /* Implicit and explicit root container access are functionally equivalent, but explicit references are easier to read and debug. diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobContainerClientBuilder.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobContainerClientBuilder.java index 1f0b003f01cc..e3f79b712bf6 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobContainerClientBuilder.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobContainerClientBuilder.java @@ -127,6 +127,11 @@ public BlobContainerClient buildClient() { new IllegalArgumentException("Customer provided key and encryption " + "scope cannot both be set")); } + BuilderHelper.applyEnvironmentSessionDefaults(sessionOptions, configuration, LOGGER); + if (CoreUtils.isNullOrEmpty(containerName) && !CoreUtils.isNullOrEmpty(sessionOptions.getContainerName())) { + containerName = sessionOptions.getContainerName(); + } + BuilderHelper.validateSessionMode(sessionOptions, containerName, LOGGER); /* @@ -170,6 +175,11 @@ public BlobContainerAsyncClient buildAsyncClient() { new IllegalArgumentException("Customer provided key and encryption " + "scope cannot both be set")); } + BuilderHelper.applyEnvironmentSessionDefaults(sessionOptions, configuration, LOGGER); + if (CoreUtils.isNullOrEmpty(containerName) && !CoreUtils.isNullOrEmpty(sessionOptions.getContainerName())) { + containerName = sessionOptions.getContainerName(); + } + BuilderHelper.validateSessionMode(sessionOptions, containerName, LOGGER); /* diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobServiceClientBuilder.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobServiceClientBuilder.java index 3cefa0395364..b367447de870 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobServiceClientBuilder.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobServiceClientBuilder.java @@ -154,11 +154,13 @@ public BlobServiceClient buildClient() { } private HttpPipeline constructPipeline() { - return (httpPipeline != null) - ? httpPipeline - : BuilderHelper.buildPipeline(storageSharedKeyCredential, tokenCredential, azureSasCredential, sasToken, - endpoint, retryOptions, coreRetryOptions, logOptions, clientOptions, httpClient, perCallPolicies, - perRetryPolicies, configuration, audience, LOGGER, sessionOptions, null); + if (httpPipeline != null) { + return httpPipeline; + } + BuilderHelper.applyEnvironmentSessionDefaults(sessionOptions, configuration, LOGGER); + return BuilderHelper.buildPipeline(storageSharedKeyCredential, tokenCredential, azureSasCredential, sasToken, + endpoint, retryOptions, coreRetryOptions, logOptions, clientOptions, httpClient, perCallPolicies, + perRetryPolicies, configuration, audience, LOGGER, sessionOptions, null); } /** diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BuilderHelper.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BuilderHelper.java index 914794f2bde1..a359a79536a9 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BuilderHelper.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BuilderHelper.java @@ -48,6 +48,7 @@ import java.net.MalformedURLException; import java.util.ArrayList; import java.util.List; +import java.util.Locale; import java.util.Map; import static com.azure.storage.common.Utility.STORAGE_TRACING_NAMESPACE_VALUE; @@ -61,6 +62,20 @@ public final class BuilderHelper { private static final String CLIENT_NAME; private static final String CLIENT_VERSION; + /** + * Environment variable / configuration key that, when set, selects the {@link SessionMode} + * to use on a builder that has not been explicitly configured (i.e. still using + * {@link SessionMode#AUTO}). Accepted values are the names of {@link SessionMode} + * (case-insensitive): {@code NONE}, {@code AUTO}, {@code SINGLE_SPECIFIED_CONTAINER}. + */ + public static final String PROPERTY_AZURE_STORAGE_SESSION_MODE = "AZURE_STORAGE_SESSION_MODE"; + + /** + * Environment variable / configuration key that, when set, supplies the container name to + * scope the session to on a builder where it has not been explicitly configured. + */ + public static final String PROPERTY_AZURE_STORAGE_SESSION_CONTAINER_NAME = "AZURE_STORAGE_SESSION_CONTAINER_NAME"; + static { Map properties = CoreUtils.getProperties("azure-storage-blob.properties"); CLIENT_NAME = properties.getOrDefault("name", "UnknownName"); @@ -300,4 +315,53 @@ public static void validateSessionMode(SessionOptions sessionOptions, String con "containerName must be set when using SessionMode." + sessionOptions.getSessionMode())); } } + + /** + * Applies environment / configuration based defaults to the supplied {@link SessionOptions}. + *

+ * This is a fallback that only fills in values the caller has not explicitly configured on the + * builder, so explicit programmatic configuration always wins: + *

    + *
  • {@link #PROPERTY_AZURE_STORAGE_SESSION_MODE} is consulted only when + * {@link SessionOptions#getSessionMode()} is still {@link SessionMode#AUTO} (the default). + * The env var value is matched case-insensitively against the names of {@link SessionMode}.
    • + *
  • {@link #PROPERTY_AZURE_STORAGE_SESSION_CONTAINER_NAME} is consulted only when + * {@link SessionOptions#getContainerName()} is {@code null} or empty.
    • + *
+ * Mutates {@code sessionOptions} in place. + * + * @param sessionOptions the options instance to populate; must not be {@code null}. + * @param configuration the configuration store to read from; if {@code null}, the global + * configuration is used. + * @param logger {@link ClientLogger} used to log any exception. + * @throws IllegalArgumentException if {@link #PROPERTY_AZURE_STORAGE_SESSION_MODE} is set to a + * value that does not name a known {@link SessionMode}. + */ + public static void applyEnvironmentSessionDefaults(SessionOptions sessionOptions, Configuration configuration, + ClientLogger logger) { + Configuration effectiveConfiguration + = (configuration == null) ? Configuration.getGlobalConfiguration() : configuration; + + if (sessionOptions.getSessionMode() == SessionMode.AUTO) { + String envMode = effectiveConfiguration.get(PROPERTY_AZURE_STORAGE_SESSION_MODE); + if (!CoreUtils.isNullOrEmpty(envMode)) { + SessionMode parsed; + try { + parsed = SessionMode.valueOf(envMode.trim().toUpperCase(Locale.ROOT)); + } catch (IllegalArgumentException ex) { + throw logger.logExceptionAsError(new IllegalArgumentException("Invalid value '" + envMode + + "' for environment variable " + PROPERTY_AZURE_STORAGE_SESSION_MODE + + ". Allowed values are: NONE, AUTO, SINGLE_SPECIFIED_CONTAINER.", ex)); + } + sessionOptions.setSessionMode(parsed); + } + } + + if (CoreUtils.isNullOrEmpty(sessionOptions.getContainerName())) { + String envContainer = effectiveConfiguration.get(PROPERTY_AZURE_STORAGE_SESSION_CONTAINER_NAME); + if (!CoreUtils.isNullOrEmpty(envContainer)) { + sessionOptions.setContainerName(envContainer.trim()); + } + } + } } diff --git a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/BuilderHelperTests.java b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/BuilderHelperTests.java index 370b33e88424..c060adfb642f 100644 --- a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/BuilderHelperTests.java +++ b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/BuilderHelperTests.java @@ -16,7 +16,10 @@ import com.azure.core.test.http.MockHttpResponse; import com.azure.core.test.http.NoOpHttpClient; import com.azure.core.test.utils.MockTokenCredential; +import com.azure.core.test.utils.TestConfigurationSource; import com.azure.core.util.ClientOptions; +import com.azure.core.util.Configuration; +import com.azure.core.util.ConfigurationBuilder; import com.azure.core.util.CoreUtils; import com.azure.core.util.DateTimeRfc1123; import com.azure.core.util.Header; @@ -31,6 +34,7 @@ import com.azure.storage.common.StorageSharedKeyCredential; import com.azure.storage.common.policy.RequestRetryOptions; import com.azure.storage.common.policy.RetryPolicyType; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -834,4 +838,190 @@ public void containerBuilderWithNoSessionOptionsSucceeds() { } // endregion + + // region environment variable session activation tests + + private static Configuration envConfiguration(String mode, String container) { + TestConfigurationSource envSource = new TestConfigurationSource(); + if (mode != null) { + envSource.put(BuilderHelper.PROPERTY_AZURE_STORAGE_SESSION_MODE, mode); + } + if (container != null) { + envSource.put(BuilderHelper.PROPERTY_AZURE_STORAGE_SESSION_CONTAINER_NAME, container); + } + return new ConfigurationBuilder(new TestConfigurationSource(), new TestConfigurationSource(), envSource) + .build(); + } + + @Test + public void containerBuilderActivatesSessionFromEnvWhenNothingExplicit() { + Configuration config = envConfiguration("SINGLE_SPECIFIED_CONTAINER", "envcontainer"); + + assertDoesNotThrow(() -> new BlobContainerClientBuilder().endpoint(ENDPOINT) + .credential(new MockTokenCredential()) + .httpClient(new NoOpHttpClient()) + .configuration(config) + .buildClient()); + } + + @Test + public void containerBuilderEnvModeWithoutContainerNameStillThrows() { + Configuration config = envConfiguration("SINGLE_SPECIFIED_CONTAINER", null); + + assertThrows(IllegalArgumentException.class, + () -> new BlobContainerClientBuilder().endpoint(ENDPOINT) + .credential(new MockTokenCredential()) + .httpClient(new NoOpHttpClient()) + .configuration(config) + .buildClient()); + } + + @Test + public void containerBuilderEnvModeIsCaseInsensitive() { + Configuration config = envConfiguration("single_specified_container", "envcontainer"); + + assertDoesNotThrow(() -> new BlobContainerClientBuilder().endpoint(ENDPOINT) + .credential(new MockTokenCredential()) + .httpClient(new NoOpHttpClient()) + .configuration(config) + .buildClient()); + } + + @Test + public void containerBuilderInvalidEnvModeThrows() { + Configuration config = envConfiguration("NOT_A_REAL_MODE", "envcontainer"); + + assertThrows(IllegalArgumentException.class, + () -> new BlobContainerClientBuilder().endpoint(ENDPOINT) + .credential(new MockTokenCredential()) + .httpClient(new NoOpHttpClient()) + .configuration(config) + .buildClient()); + } + + @Test + public void containerBuilderExplicitSessionModeOverridesEnv() { + Configuration config = envConfiguration("SINGLE_SPECIFIED_CONTAINER", "envcontainer"); + SessionOptions explicitNone = new SessionOptions().setSessionMode(SessionMode.NONE); + + // Explicit NONE must not be upgraded by env vars and no container is required. + assertDoesNotThrow(() -> new BlobContainerClientBuilder().endpoint(ENDPOINT) + .credential(new MockTokenCredential()) + .httpClient(new NoOpHttpClient()) + .configuration(config) + .sessionOptions(explicitNone) + .buildClient()); + } + + @Test + public void containerBuilderExplicitContainerNameWinsOverEnv() { + Configuration config = envConfiguration("SINGLE_SPECIFIED_CONTAINER", "envcontainer"); + SessionOptions options = new SessionOptions().setContainerName("explicitcontainer"); + + assertDoesNotThrow(() -> new BlobContainerClientBuilder().endpoint(ENDPOINT) + .containerName("explicitcontainer") + .credential(new MockTokenCredential()) + .httpClient(new NoOpHttpClient()) + .configuration(config) + .sessionOptions(options) + .buildClient()); + + assertEquals("explicitcontainer", options.getContainerName()); + } + + @Test + public void blobBuilderActivatesSessionFromEnvWhenNothingExplicit() { + Configuration config = envConfiguration("SINGLE_SPECIFIED_CONTAINER", "envcontainer"); + + assertDoesNotThrow(() -> new BlobClientBuilder().endpoint(ENDPOINT) + .blobName("myblob") + .credential(new MockTokenCredential()) + .httpClient(new NoOpHttpClient()) + .configuration(config) + .buildClient()); + } + + @Test + public void serviceBuilderActivatesSessionFromEnvWhenNothingExplicit() { + Configuration config = envConfiguration("SINGLE_SPECIFIED_CONTAINER", "envcontainer"); + SessionOptions options = new SessionOptions(); + + assertDoesNotThrow(() -> new BlobServiceClientBuilder().endpoint(ENDPOINT) + .credential(new MockTokenCredential()) + .httpClient(new NoOpHttpClient()) + .configuration(config) + .sessionOptions(options) + .buildClient()); + + // Env vars must have flowed through to the SessionOptions instance. + assertEquals(SessionMode.SINGLE_SPECIFIED_CONTAINER, options.getSessionMode()); + assertEquals("envcontainer", options.getContainerName()); + } + + @Test + public void applyEnvironmentSessionDefaultsLeavesExplicitValuesIntact() { + Configuration config = envConfiguration("SINGLE_SPECIFIED_CONTAINER", "envcontainer"); + SessionOptions options = new SessionOptions().setSessionMode(SessionMode.NONE).setContainerName("explicit"); + + BuilderHelper.applyEnvironmentSessionDefaults(options, config, new ClientLogger(BuilderHelperTests.class)); + + assertEquals(SessionMode.NONE, options.getSessionMode()); + assertEquals("explicit", options.getContainerName()); + } + + @Test + public void applyEnvironmentSessionDefaultsAppliesOnlyContainerNameWhenModeExplicit() { + Configuration config = envConfiguration(null, "envcontainer"); + SessionOptions options = new SessionOptions().setSessionMode(SessionMode.SINGLE_SPECIFIED_CONTAINER); + + BuilderHelper.applyEnvironmentSessionDefaults(options, config, new ClientLogger(BuilderHelperTests.class)); + + assertEquals(SessionMode.SINGLE_SPECIFIED_CONTAINER, options.getSessionMode()); + assertEquals("envcontainer", options.getContainerName()); + } + + // endregion + + // region environment-variable end-to-end test + // + // This single test verifies that a customer can activate the session feature with NO code + // change at all -- just by exporting environment variables before starting the JVM: + // + // set AZURE_STORAGE_SESSION_MODE=SINGLE_SPECIFIED_CONTAINER + // set AZURE_STORAGE_SESSION_CONTAINER_NAME=mycontainer + // + // It is @Disabled by default because: + // 1. CI doesn't (and shouldn't) set these process-level env vars. + // 2. EnvironmentConfiguration in azure-core caches reads from the global Configuration + // for the lifetime of the JVM, so it cannot be reliably reset between tests inside + // the same Surefire fork. + // + // To run it manually after setting the env vars above: + // + // mvn -pl sdk/storage/azure-storage-blob test ^ + // "-Dtest=BuilderHelperTests#environmentVariablesActivateSession" + // + // The 10 injection-based tests above already cover all of the helper's branching logic + // by injecting a Configuration directly; this test exists only to prove the real + // System.getenv lookup path also works. + + @Test + @Disabled("Run manually after exporting AZURE_STORAGE_SESSION_MODE=SINGLE_SPECIFIED_CONTAINER and " + + "AZURE_STORAGE_SESSION_CONTAINER_NAME=. See the comment above for details.") + public void environmentVariablesActivateSession() { + SessionOptions options = new SessionOptions(); + assertDoesNotThrow(() -> new BlobContainerClientBuilder().endpoint(ENDPOINT) + .credential(new MockTokenCredential()) + .httpClient(new NoOpHttpClient()) + .sessionOptions(options) + .buildClient()); + + String expectedContainer = System.getenv(BuilderHelper.PROPERTY_AZURE_STORAGE_SESSION_CONTAINER_NAME); + assertEquals(SessionMode.SINGLE_SPECIFIED_CONTAINER, options.getSessionMode(), + "Expected env var AZURE_STORAGE_SESSION_MODE=SINGLE_SPECIFIED_CONTAINER to populate sessionMode"); + assertEquals(expectedContainer, options.getContainerName(), + "Expected env var AZURE_STORAGE_SESSION_CONTAINER_NAME to populate containerName"); + } + + // endregion } diff --git a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java index 76195bb37083..8b83c216b69f 100644 --- a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java +++ b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java @@ -2291,4 +2291,61 @@ private BlobContainerClient sessionEnabledContainerClient(HttpPipelinePolicy... .setAccountName(cc.getAccountName()); return getOAuthServiceClient(sessionOptions, policies).getBlobContainerClient(cc.getBlobContainerName()); } + + @Test + @LiveOnly + @Disabled("This test is disabled since it requires specific environment vars to be set that are not normally set") + @ResourceLock("BlobSessionAuth") + // Verifies the env-var session-activation feature end-to-end. With NO explicit SessionOptions + // and NO .sessionOptions(...) call, a customer who only exports the AZURE_STORAGE_SESSION_MODE + // and AZURE_STORAGE_SESSION_CONTAINER_NAME environment variables should get blob downloads + // automatically signed with the "Session" auth scheme instead of "Bearer". + public void downloadBlobUsingEnvVarSessionAuth() { + + String myContainerName = "session-test-container"; + String endpoint = ENVIRONMENT.getPrimaryAccount().getBlobEndpoint(); + + // Setup: provision the container and upload a blob the customer will later download. + primaryBlobServiceClient.createBlobContainer(myContainerName); + String blobName = generateBlobName(); + primaryBlobServiceClient.getBlobContainerClient(myContainerName) + .getBlobClient(blobName) + .getBlockBlobClient() + .upload(DATA.getDefaultInputStream(), DATA.getDefaultDataSize()); + + // A real customer would deploy with these environment variables set on the process: + // AZURE_STORAGE_SESSION_MODE=SINGLE_SPECIFIED_CONTAINER + // AZURE_STORAGE_SESSION_CONTAINER_NAME= + // A JVM cannot mutate its own env, so we set the equivalent system properties. + // azure-core's EnvironmentConfiguration reads system properties as a fallback for env + // vars, so this faithfully simulates the deployed scenario from inside a test. + + List downloadAuthSchemes = Collections.synchronizedList(new ArrayList<>()); + RequestInspectionPolicy inspect = new RequestInspectionPolicy(req -> { + String auth = req.getHeaders().getValue(HttpHeaderName.AUTHORIZATION); + String path = req.getUrl().getPath(); + String trimmed = path != null && path.startsWith("/") ? path.substring(1) : path; + if (auth != null && trimmed != null && trimmed.contains("/")) { + downloadAuthSchemes.add(auth.startsWith("Session ") ? "Session" : "Bearer"); + } + }); + + try { + // Customer code: no .sessionOptions(...), no .configuration(...). The env vars + // set above are the only thing turning on session-based auth. + BlobContainerClient sessionCc = instrument(new BlobContainerClientBuilder().endpoint(endpoint) + .containerName(myContainerName) + .credential(new DefaultAzureCredentialBuilder().build()) + .addPolicy(inspect)).buildClient(); + + BinaryData downloaded = sessionCc.getBlobClient(blobName).downloadContent(); + assertEquals(DATA.getDefaultText(), downloaded.toString()); + + assertFalse(downloadAuthSchemes.isEmpty(), "Expected to observe at least one blob download request"); + assertTrue(downloadAuthSchemes.stream().allMatch("Session"::equals), + "Expected env-var-configured client to use Session auth on blob downloads; saw " + downloadAuthSchemes); + } finally { + primaryBlobServiceClient.deleteBlobContainer(myContainerName); + } + } } diff --git a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerAsyncApiTests.java b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerAsyncApiTests.java index ee3579271283..a08e31e4c5cf 100644 --- a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerAsyncApiTests.java +++ b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerAsyncApiTests.java @@ -2311,4 +2311,62 @@ private BlobContainerAsyncClient sessionEnabledContainerAsyncClient(HttpPipeline .getBlobContainerAsyncClient(ccAsync.getBlobContainerName()); } + @Test + @LiveOnly + @Disabled("This test is disabled since it requires specific environment vars to be set that are not normally set") + @ResourceLock("BlobSessionAuth") + // Async twin of ContainerApiTests#downloadBlobUsingEnvVarSessionAuth. Verifies the env-var + // session-activation feature end-to-end. With NO explicit SessionOptions and NO + // .sessionOptions(...) call, a customer who only exports AZURE_STORAGE_SESSION_MODE and + // AZURE_STORAGE_SESSION_CONTAINER_NAME should get blob downloads automatically signed with + // the "Session" auth scheme instead of "Bearer". + public void downloadBlobUsingEnvVarSessionAuth() { + + String myContainerName = "session-test-container"; + String endpoint = ENVIRONMENT.getPrimaryAccount().getBlobEndpoint(); + + // Setup: provision the container and upload a blob the customer will later download. + primaryBlobServiceAsyncClient.createBlobContainer(myContainerName).block(); + String blobName = generateBlobName(); + primaryBlobServiceAsyncClient.getBlobContainerAsyncClient(myContainerName) + .getBlobAsyncClient(blobName) + .getBlockBlobAsyncClient() + .upload(DATA.getDefaultFlux(), DATA.getDefaultDataSize()) + .block(); + + // A real customer would deploy with these environment variables set on the process: + // AZURE_STORAGE_SESSION_MODE=SINGLE_SPECIFIED_CONTAINER + // AZURE_STORAGE_SESSION_CONTAINER_NAME= + // This test relies on those env vars being set on the host running it. + + List downloadAuthSchemes = Collections.synchronizedList(new ArrayList<>()); + RequestInspectionPolicy inspect = new RequestInspectionPolicy(req -> { + String auth = req.getHeaders().getValue(HttpHeaderName.AUTHORIZATION); + String path = req.getUrl().getPath(); + String trimmed = path != null && path.startsWith("/") ? path.substring(1) : path; + if (auth != null && trimmed != null && trimmed.contains("/")) { + downloadAuthSchemes.add(auth.startsWith("Session ") ? "Session" : "Bearer"); + } + }); + + try { + // Customer code: no .sessionOptions(...), no .configuration(...). The env vars + // set on the host are the only thing turning on session-based auth. + BlobContainerAsyncClient sessionCcAsync = instrument(new BlobContainerClientBuilder().endpoint(endpoint) + .containerName(myContainerName) + .credential(new DefaultAzureCredentialBuilder().build()) + .addPolicy(inspect)).buildAsyncClient(); + + StepVerifier.create(sessionCcAsync.getBlobAsyncClient(blobName).downloadContent()) + .assertNext(downloaded -> assertEquals(DATA.getDefaultText(), downloaded.toString())) + .verifyComplete(); + + assertFalse(downloadAuthSchemes.isEmpty(), "Expected to observe at least one blob download request"); + assertTrue(downloadAuthSchemes.stream().allMatch("Session"::equals), + "Expected env-var-configured client to use Session auth on blob downloads; saw " + downloadAuthSchemes); + } finally { + primaryBlobServiceAsyncClient.deleteBlobContainer(myContainerName).block(); + } + } + } diff --git a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/implementation/util/SessionTokenCredentialPolicyTest.java b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/implementation/util/SessionTokenCredentialPolicyTest.java index 020284e56f78..3d9c28e41947 100644 --- a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/implementation/util/SessionTokenCredentialPolicyTest.java +++ b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/implementation/util/SessionTokenCredentialPolicyTest.java @@ -626,15 +626,18 @@ public void getBlobRequestProducesWellFormedSessionAuthHeader() { } /** - * Guards the workaround in {@link StorageSessionCredential#buildStringToSign}: the Session - * protocol signs the literal {@code Content-Length} value rather than normalizing - * {@code "0" -> ""} like SharedKey does. This is required today because azure-core's - * {@code RestProxyBase} unconditionally adds {@code Content-Length: 0} to body-less GET - * requests. Once that is fixed in azure-core, the buildStringToSign workaround can be removed - * and this test should be updated (or deleted) to reflect the new behavior. + * Regression guard: the Session protocol must normalize {@code Content-Length: "0"} to + * {@code ""} in the string-to-sign, matching the server's canonicalization (which is the + * same as documented Shared Key canonicalization). Signing with {@code Content-Length: 0} + * must therefore produce the same HMAC as signing without a Content-Length header at all. + *

+ * Originally we expected the opposite (signing the literal "0") based on a misread of the + * service behavior; that caused 401 InvalidAuthenticationInfo errors on real blob GETs + * because azure-core's {@code RestProxyBase} unconditionally puts {@code Content-Length: 0} + * on body-less GETs while the server canonicalizes that to "" before computing its HMAC. */ @Test - public void contentLengthZeroIsIncludedInSessionSignature() { + public void contentLengthZeroProducesSameSignatureAsMissingContentLength() { String pinnedDate = "Wed, 22 Apr 2026 20:00:00 GMT"; HttpRequest withCl0 @@ -658,9 +661,9 @@ public void contentLengthZeroIsIncludedInSessionSignature() { credentialWithToken(FIRST_TOKEN).signRequest(withoutCl); String sigWithoutCl = extractSignature(withoutCl.getHeaders().getValue(authHeaderName)); - assertTrue(!sigWithCl0.equals(sigWithoutCl), - "Session signature must include literal Content-Length value: signing with " - + "Content-Length: 0 must differ from signing without Content-Length"); + assertEquals(sigWithCl0, sigWithoutCl, + "Session signature must normalize Content-Length: 0 to empty: signing with " + + "Content-Length: 0 must match signing without Content-Length"); } private static String extractSignature(String authHeader) { diff --git a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/implementation/util/StorageSessionCredentialTest.java b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/implementation/util/StorageSessionCredentialTest.java index 2215af27c1a0..dc70b97398d4 100644 --- a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/implementation/util/StorageSessionCredentialTest.java +++ b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/implementation/util/StorageSessionCredentialTest.java @@ -58,16 +58,9 @@ public void signRequestSetsXmsDateHeader() throws MalformedURLException { // encoded query string (e.g. snapshot=...%3A...). // // Scope is intentionally narrow. Session and SharedKey legitimately diverge on: - // - missing Content-Length (SharedKey emits literal "null" via String.join; Session emits "") - // - Content-Length "0" on GETs (SharedKey normalizes to ""; Session preserves "0" to match - // what azure-core's RestProxyBase puts on the wire — see the comment on - // StorageSessionCredential.buildStringToSign). - // Content-Length is pinned to a realistic non-zero value to bypass both quirks. - // - // DELETE this test once azure-core stops setting Content-Length: 0 on GETs and - // StorageSessionCredential.buildStringToSign is removed in favor of delegating to - // sharedKey.generateAuthorizationHeader(...). At that point this assertion becomes - // tautological (SharedKey vs. SharedKey). + // - missing Content-Length (SharedKey emits literal "null" via String.join; Session emits ""). + // Content-Length is pinned to a realistic non-zero value to bypass that quirk. Equivalence for + // Content-Length: 0 (which the server normalizes to "") is covered separately. @Test public void canonicalizationMatchesSharedKeyForEncodedQuery() throws MalformedURLException { StorageSessionCredential sessionCred = SessionTestHelper.createValidCredential(); From 44a9ee3125e1d9e3b0b4f65d0d3041998ba4bc07 Mon Sep 17 00:00:00 2001 From: Isabelle <141270045+ibrandes@users.noreply.github.com> Date: Thu, 11 Jun 2026 10:10:55 -0700 Subject: [PATCH 3/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../java/com/azure/storage/blob/ContainerApiTests.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java index 8b83c216b69f..b7daa22469ce 100644 --- a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java +++ b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java @@ -2314,11 +2314,9 @@ public void downloadBlobUsingEnvVarSessionAuth() { .upload(DATA.getDefaultInputStream(), DATA.getDefaultDataSize()); // A real customer would deploy with these environment variables set on the process: - // AZURE_STORAGE_SESSION_MODE=SINGLE_SPECIFIED_CONTAINER - // AZURE_STORAGE_SESSION_CONTAINER_NAME= - // A JVM cannot mutate its own env, so we set the equivalent system properties. - // azure-core's EnvironmentConfiguration reads system properties as a fallback for env - // vars, so this faithfully simulates the deployed scenario from inside a test. +// AZURE_STORAGE_SESSION_MODE=SINGLE_SPECIFIED_CONTAINER +// AZURE_STORAGE_SESSION_CONTAINER_NAME=session-test-container +// This test relies on those env vars being set on the host running it. List downloadAuthSchemes = Collections.synchronizedList(new ArrayList<>()); RequestInspectionPolicy inspect = new RequestInspectionPolicy(req -> { From 45b7a4bbcd3b1982ed944e6304f8902b439025df Mon Sep 17 00:00:00 2001 From: browndav Date: Thu, 11 Jun 2026 13:30:27 -0400 Subject: [PATCH 4/5] add code to ensure container name only accepted when session enabled --- .../src/main/java/com/azure/storage/blob/BlobClientBuilder.java | 1 + .../azure/storage/blob/implementation/util/BuilderHelper.java | 2 +- .../java/com/azure/storage/blob/ContainerAsyncApiTests.java | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobClientBuilder.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobClientBuilder.java index 6df5f792d0d4..0a2fc03574d1 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobClientBuilder.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobClientBuilder.java @@ -194,6 +194,7 @@ public BlobAsyncClient buildAsyncClient() { if (CoreUtils.isNullOrEmpty(containerName) && !CoreUtils.isNullOrEmpty(sessionOptions.getContainerName())) { containerName = sessionOptions.getContainerName(); } + BuilderHelper.validateSessionMode(sessionOptions, containerName, LOGGER); /* Implicit and explicit root container access are functionally equivalent, but explicit references are easier diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BuilderHelper.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BuilderHelper.java index a359a79536a9..4ce72b4164d9 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BuilderHelper.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BuilderHelper.java @@ -357,7 +357,7 @@ public static void applyEnvironmentSessionDefaults(SessionOptions sessionOptions } } - if (CoreUtils.isNullOrEmpty(sessionOptions.getContainerName())) { + if (sessionOptions.getSessionMode().resolve() != SessionMode.NONE && CoreUtils.isNullOrEmpty(sessionOptions.getContainerName())) { String envContainer = effectiveConfiguration.get(PROPERTY_AZURE_STORAGE_SESSION_CONTAINER_NAME); if (!CoreUtils.isNullOrEmpty(envContainer)) { sessionOptions.setContainerName(envContainer.trim()); diff --git a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerAsyncApiTests.java b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerAsyncApiTests.java index a08e31e4c5cf..b8555db0cbee 100644 --- a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerAsyncApiTests.java +++ b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerAsyncApiTests.java @@ -2336,7 +2336,7 @@ public void downloadBlobUsingEnvVarSessionAuth() { // A real customer would deploy with these environment variables set on the process: // AZURE_STORAGE_SESSION_MODE=SINGLE_SPECIFIED_CONTAINER - // AZURE_STORAGE_SESSION_CONTAINER_NAME= + // AZURE_STORAGE_SESSION_CONTAINER_NAME=session-test-container // This test relies on those env vars being set on the host running it. List downloadAuthSchemes = Collections.synchronizedList(new ArrayList<>()); From 06aa7efc3714ae7dd06e065e18569db263298d9c Mon Sep 17 00:00:00 2001 From: browndav Date: Thu, 11 Jun 2026 14:11:09 -0400 Subject: [PATCH 5/5] fix formatting and javadocs alignment --- .../implementation/util/BuilderHelper.java | 47 ++++++++++--------- .../azure/storage/blob/ContainerApiTests.java | 6 +-- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BuilderHelper.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BuilderHelper.java index 4ce72b4164d9..79c6b6558c19 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BuilderHelper.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BuilderHelper.java @@ -87,22 +87,22 @@ public final class BuilderHelper { * authentication support. * * @param storageSharedKeyCredential {@link StorageSharedKeyCredential} if present. - * @param tokenCredential {@link TokenCredential} if present. - * @param azureSasCredential {@link AzureSasCredential} if present. - * @param sasToken SAS token if present. - * @param endpoint The endpoint for the client. - * @param retryOptions Storage's retry options to set in the retry policy. - * @param coreRetryOptions Core's retry options to set in the retry policy. - * @param logOptions Logging options to set in the logging policy. - * @param clientOptions Client options. - * @param httpClient HttpClient to use in the builder. - * @param perCallPolicies Additional {@link HttpPipelinePolicy policies} to set in the pipeline per call. - * @param perRetryPolicies Additional {@link HttpPipelinePolicy policies} to set in the pipeline per retry. - * @param configuration Configuration store contain environment settings. - * @param logger {@link ClientLogger} used to log any exception. - * @param audience {@link BlobAudience} used to determine the audience of the blob. - * @param sessionOptions {@link SessionOptions} containing the session mode, container name, and account name for session-based authentication. - * @param serviceVersion The service version for session creation. Required when session is active. + * @param tokenCredential {@link TokenCredential} if present. + * @param azureSasCredential {@link AzureSasCredential} if present. + * @param sasToken SAS token if present. + * @param endpoint The endpoint for the client. + * @param retryOptions Storage's retry options to set in the retry policy. + * @param coreRetryOptions Core's retry options to set in the retry policy. + * @param logOptions Logging options to set in the logging policy. + * @param clientOptions Client options. + * @param httpClient HttpClient to use in the builder. + * @param perCallPolicies Additional {@link HttpPipelinePolicy policies} to set in the pipeline per call. + * @param perRetryPolicies Additional {@link HttpPipelinePolicy policies} to set in the pipeline per retry. + * @param configuration Configuration store contain environment settings. + * @param logger {@link ClientLogger} used to log any exception. + * @param audience {@link BlobAudience} used to determine the audience of the blob. + * @param sessionOptions {@link SessionOptions} containing the session mode, container name, and account name for session-based authentication. + * @param serviceVersion The service version for session creation. Required when session is active. * @return A new {@link HttpPipeline} from the passed values. */ public static HttpPipeline buildPipeline(StorageSharedKeyCredential storageSharedKeyCredential, @@ -256,8 +256,8 @@ public static String getEndpoint(BlobUrlParts parts) throws MalformedURLExceptio * Validates that the client is properly configured to use https. * * @param objectToCheck The object to check for. - * @param objectName The name of the object. - * @param endpoint The endpoint for the client. + * @param objectName The name of the object. + * @param endpoint The endpoint for the client. */ public static void httpsValidation(Object objectToCheck, String objectName, String endpoint, ClientLogger logger) { if (objectToCheck != null && !BlobUrlParts.parse(endpoint).getScheme().equals(Constants.HTTPS)) { @@ -302,7 +302,7 @@ public static Tracer createTracer(ClientOptions clientOptions) { /** * Logs information about credential changes in builders. * - * @param logger The logger to use. + * @param logger The logger to use. * @param newCredentialType The credential type being set. */ public static void logCredentialChange(ClientLogger logger, String newCredentialType) { @@ -331,9 +331,9 @@ public static void validateSessionMode(SessionOptions sessionOptions, String con * Mutates {@code sessionOptions} in place. * * @param sessionOptions the options instance to populate; must not be {@code null}. - * @param configuration the configuration store to read from; if {@code null}, the global - * configuration is used. - * @param logger {@link ClientLogger} used to log any exception. + * @param configuration the configuration store to read from; if {@code null}, the global + * configuration is used. + * @param logger {@link ClientLogger} used to log any exception. * @throws IllegalArgumentException if {@link #PROPERTY_AZURE_STORAGE_SESSION_MODE} is set to a * value that does not name a known {@link SessionMode}. */ @@ -357,7 +357,8 @@ public static void applyEnvironmentSessionDefaults(SessionOptions sessionOptions } } - if (sessionOptions.getSessionMode().resolve() != SessionMode.NONE && CoreUtils.isNullOrEmpty(sessionOptions.getContainerName())) { + if (sessionOptions.getSessionMode().resolve() != SessionMode.NONE + && CoreUtils.isNullOrEmpty(sessionOptions.getContainerName())) { String envContainer = effectiveConfiguration.get(PROPERTY_AZURE_STORAGE_SESSION_CONTAINER_NAME); if (!CoreUtils.isNullOrEmpty(envContainer)) { sessionOptions.setContainerName(envContainer.trim()); diff --git a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java index b7daa22469ce..cff568e88547 100644 --- a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java +++ b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/ContainerApiTests.java @@ -2314,9 +2314,9 @@ public void downloadBlobUsingEnvVarSessionAuth() { .upload(DATA.getDefaultInputStream(), DATA.getDefaultDataSize()); // A real customer would deploy with these environment variables set on the process: -// AZURE_STORAGE_SESSION_MODE=SINGLE_SPECIFIED_CONTAINER -// AZURE_STORAGE_SESSION_CONTAINER_NAME=session-test-container -// This test relies on those env vars being set on the host running it. + // AZURE_STORAGE_SESSION_MODE=SINGLE_SPECIFIED_CONTAINER + // AZURE_STORAGE_SESSION_CONTAINER_NAME=session-test-container + // This test relies on those env vars being set on the host running it. List downloadAuthSchemes = Collections.synchronizedList(new ArrayList<>()); RequestInspectionPolicy inspect = new RequestInspectionPolicy(req -> {