[Storage] Bugfix - Smaller Scope Ensure Both Forms of Auth are Applied for Principal Bound User Delegation SAS

[ibrandes]

Bug

In the Java Storage SDK, when customers provided both a SAS (via sasToken/AzureSasCredential) and a TokenCredential to a client builder, the SDK behavior effectively overrode/dropped whichever credential was provided first, resulting in requests being authenticated only with one credential.

This meant the principal-bound identity SAS path (which requires both forms of auth) was not actually exercised. The issue was further masked by tests, which did not explicitly validate that the SAS was present on the outgoing request URL.

This issue applied to Blob, Datalake, File Share, and Queue.

PR Changes

1) Allow the supported “Token + SAS” dual-credential scenario

• Updated credential validation from “single credential only” to “not ambiguous” validation.
• New rule: up to two credentials may be present only when the combination is:
  • TokenCredential + (AzureSasCredential XOR sasToken)
• All other two-credential combos (and 3+ credentials) are rejected.

Relevant change:

• CredentialValidator.validateCredentialsNotAmbiguous(...) (new)
• Blob pipeline now calls this instead of validateSingleCredentialIsPresent(...).

2) Ensure both auth mechanisms are actually applied to the HTTP pipeline

• Refactored BuilderHelper.buildPipeline(...) to add credential policies independently rather than selecting a single “winner” policy.

This ensures that when a principal-bound user delegation SAS scenario is configured, both the Authorization header and SAS query parameters are used on requests, instead of SAS being overridden.

3) Builder behavior + diagnostics

• Updated BlobClientBuilder, BlobContainerClientBuilder, BlobServiceClientBuilder, and SpecializedBlobClientBuilder so setting TokenCredential no longer clears SAS (and setting SAS no longer clears TokenCredential), enabling the supported dual-auth scenario.
• Updated other package client builders in the same way.
• Added BuilderHelper.logCredentialChange(...) and used it to log when a caller changes credential types after one was already configured (helps diagnose subtle builder misconfiguration).

4) Tests now explicitly detect the regression

• Updated SAS user delegation tests to validate that outgoing requests include:
  • an Authorization header and
  • SAS parameters on the request URL (checks for sv=<SAS service version>)
• Introduced StorageCommonTestUtils.verifySasAndTokenInRequest(Response<T>) to enforce these assertions.
• Removed tests that previously expected failures for token + SAS combinations, since this combination is now intentionally supported for principal-bound identity SAS.

[Copilot]

Pull request overview

This pull request implements a bugfix to enable principal-bound user delegation SAS authentication by allowing both TokenCredential and SAS credentials to be used simultaneously in Azure Storage SDK clients.

Changes:

• Modified credential validation to allow TokenCredential + SAS combination
• Updated HTTP pipeline construction to add both credential policies when present
• Changed builder credential methods to not clear compatible credentials
• Enhanced tests to verify both authentication forms are applied

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/credentials/CredentialValidator.java	Added new validation method validateCredentialsNotAmbiguous to allow TokenCredential+SAS while rejecting other combinations
sdk/storage/azure-storage-common/src/test-shared/java/com/azure/storage/common/test/shared/StorageCommonTestUtils.java	Added verifySasAndTokenInRequest helper to verify both auth forms are present
sdk/storage/azure-storage-*/src/main/java/**/BuilderHelper.java	Changed pipeline construction to add credential policies independently, enabling multiple compatible policies
sdk/storage/azure-storage-*/src/main/java/**/*ClientBuilder.java	Updated credential setters to preserve compatible credentials (TokenCredential + SAS) and added logging for credential changes
sdk/storage/azure-storage-*/src/test/java/**/*SasClientTests.java	Updated tests to verify dual credential scenario works correctly and removed obsolete tests that prevented the valid combination
sdk/storage/azure-storage-blob/assets.json	Updated test recording tag to reflect new test behavior

[Copilot]

The new method validateCredentialsNotAmbiguous is missing JavaDoc documentation. The method should have documentation explaining its purpose, parameters, exceptions thrown, and the specific credential combinations that are allowed (TokenCredential + AzureSasCredential/sasToken).

Suggested change

	/**
	* Validates that the provided credentials do not form an ambiguous or unsupported combination.
	* <p>
	* This method is intended for scenarios where multiple credential types may be supplied and enforces
	* the following rules:
	* <ul>
	* <li>A maximum of two credentials may be provided at once. Supplying three or more credentials
	* results in an {@link IllegalStateException}.</li>
	* <li>When exactly two credentials are provided, the only supported combinations are:
	* <ul>
	* <li>{@link TokenCredential} together with {@link AzureSasCredential}</li>
	* <li>{@link TokenCredential} together with a SAS token string ({@code sasToken})</li>
	* </ul>
	* Any other two-credential combination (including {@link TokenCredential} with both SAS forms,
	* or combinations that do not include {@link TokenCredential}) is considered invalid and results
	* in an {@link IllegalStateException}.</li>
	* </ul>
	*
	* @param storageSharedKeyCredential {@link StorageSharedKeyCredential} if present; may be {@code null}.
	* @param tokenCredential {@link TokenCredential} if present; may be {@code null}.
	* @param azureSasCredential {@link AzureSasCredential} if present; may be {@code null}.
	* @param sasToken {@link String} representing a SAS token if present; may be {@code null}.
	* @param logger {@link ClientLogger} used to log and wrap thrown exceptions; must not be {@code null}.
	*
	* @throws IllegalStateException if more than two credentials are specified, or if two credentials are
	* provided in a combination other than {@link TokenCredential} plus either {@link AzureSasCredential}
	* or {@code sasToken}.
	*/

[Copilot]

Consider improving the error message clarity on line 57. The current message says "A maximum of two credentials is supported for specific scenarios" but doesn't indicate which scenarios are supported. A more helpful message might be: "Too many credentials specified. Only TokenCredential can be combined with a SAS credential (AzureSasCredential or sasToken). A maximum of two credentials is supported."

Suggested change

	"Too many credentials specified. A maximum of two credentials is supported for specific scenarios."));
	"Too many credentials specified. Only TokenCredential can be combined with a SAS credential "
	+ "(AzureSasCredential or sasToken). A maximum of two credentials is supported."));