diff --git a/sdk/storage/azure-storage-blob/assets.json b/sdk/storage/azure-storage-blob/assets.json index a2bc9bdaa663..c8bfbc0fb874 100644 --- a/sdk/storage/azure-storage-blob/assets.json +++ b/sdk/storage/azure-storage-blob/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "java", "TagPrefix": "java/storage/azure-storage-blob", - "Tag": "java/storage/azure-storage-blob_f26563826e" + "Tag": "java/storage/azure-storage-blob_8c8c6e5dfd" } 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 d5ca70babfb9..1d0ac36d4ce8 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 @@ -246,6 +246,10 @@ public BlobClientBuilder encryptionScope(String encryptionScope) { */ public BlobClientBuilder credential(StorageSharedKeyCredential credential) { this.storageSharedKeyCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.tokenCredential != null || this.sasToken != null) { + BuilderHelper.logCredentialChange(LOGGER, "StorageSharedKeyCredential"); + } this.tokenCredential = null; this.sasToken = null; return this; @@ -276,8 +280,11 @@ public BlobClientBuilder credential(AzureNamedKeyCredential credential) { @Override public BlobClientBuilder credential(TokenCredential credential) { this.tokenCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "TokenCredential"); + } this.storageSharedKeyCredential = null; - this.sasToken = null; return this; } @@ -291,8 +298,11 @@ public BlobClientBuilder credential(TokenCredential credential) { */ public BlobClientBuilder sasToken(String sasToken) { this.sasToken = Objects.requireNonNull(sasToken, "'sasToken' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "sasToken"); + } this.storageSharedKeyCredential = null; - this.tokenCredential = null; return this; } 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 d8413a535783..5ca6281bb1fb 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 @@ -274,6 +274,10 @@ public BlobContainerClientBuilder encryptionScope(String encryptionScope) { */ public BlobContainerClientBuilder credential(StorageSharedKeyCredential credential) { this.storageSharedKeyCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.tokenCredential != null || this.sasToken != null) { + BuilderHelper.logCredentialChange(LOGGER, "StorageSharedKeyCredential"); + } this.tokenCredential = null; this.sasToken = null; return this; @@ -304,8 +308,11 @@ public BlobContainerClientBuilder credential(AzureNamedKeyCredential credential) @Override public BlobContainerClientBuilder credential(TokenCredential credential) { this.tokenCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "TokenCredential"); + } this.storageSharedKeyCredential = null; - this.sasToken = null; return this; } @@ -319,8 +326,11 @@ public BlobContainerClientBuilder credential(TokenCredential credential) { */ public BlobContainerClientBuilder sasToken(String sasToken) { this.sasToken = Objects.requireNonNull(sasToken, "'sasToken' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "sasToken"); + } this.storageSharedKeyCredential = null; - this.tokenCredential = null; return this; } 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 66731774d9e0..5fb46965824f 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 @@ -281,6 +281,10 @@ public BlobServiceClientBuilder encryptionScope(String encryptionScope) { */ public BlobServiceClientBuilder credential(StorageSharedKeyCredential credential) { this.storageSharedKeyCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.tokenCredential != null || this.sasToken != null) { + BuilderHelper.logCredentialChange(LOGGER, "StorageSharedKeyCredential"); + } this.tokenCredential = null; this.sasToken = null; return this; @@ -311,8 +315,11 @@ public BlobServiceClientBuilder credential(AzureNamedKeyCredential credential) { @Override public BlobServiceClientBuilder credential(TokenCredential credential) { this.tokenCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "TokenCredential"); + } this.storageSharedKeyCredential = null; - this.sasToken = null; return this; } @@ -326,8 +333,11 @@ public BlobServiceClientBuilder credential(TokenCredential credential) { */ public BlobServiceClientBuilder sasToken(String sasToken) { this.sasToken = Objects.requireNonNull(sasToken, "'sasToken' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "sasToken"); + } this.storageSharedKeyCredential = null; - this.tokenCredential = null; return this; } 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 7c56941c7014..0866d310981c 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 @@ -90,7 +90,7 @@ public static HttpPipeline buildPipeline(StorageSharedKeyCredential storageShare List perRetryPolicies, Configuration configuration, BlobAudience audience, ClientLogger logger) { - CredentialValidator.validateSingleCredentialIsPresent(storageSharedKeyCredential, tokenCredential, + CredentialValidator.validateCredentialsNotAmbiguous(storageSharedKeyCredential, tokenCredential, azureSasCredential, sasToken, logger); // Closest to API goes first, closest to wire goes last. @@ -115,25 +115,22 @@ public static HttpPipeline buildPipeline(StorageSharedKeyCredential storageShare } policies.add(new MetadataValidationPolicy()); - HttpPipelinePolicy credentialPolicy; if (storageSharedKeyCredential != null) { - credentialPolicy = new StorageSharedKeyCredentialPolicy(storageSharedKeyCredential); - } else if (tokenCredential != null) { + policies.add(new StorageSharedKeyCredentialPolicy(storageSharedKeyCredential)); + } + + if (tokenCredential != null) { httpsValidation(tokenCredential, "bearer token", endpoint, logger); String scope = audience != null ? ((audience.toString().endsWith("/") ? audience + ".default" : audience + "/.default")) : Constants.STORAGE_SCOPE; - credentialPolicy = new StorageBearerTokenChallengeAuthorizationPolicy(tokenCredential, scope); - } else if (azureSasCredential != null) { - credentialPolicy = new AzureSasCredentialPolicy(azureSasCredential, false); - } else if (sasToken != null) { - credentialPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(sasToken), false); - } else { - credentialPolicy = null; + policies.add(new StorageBearerTokenChallengeAuthorizationPolicy(tokenCredential, scope)); } - if (credentialPolicy != null) { - policies.add(credentialPolicy); + if (azureSasCredential != null) { + policies.add(new AzureSasCredentialPolicy(azureSasCredential, false)); + } else if (sasToken != null) { + policies.add(new AzureSasCredentialPolicy(new AzureSasCredential(sasToken), false)); } policies.addAll(perRetryPolicies); @@ -225,4 +222,14 @@ public static Tracer createTracer(ClientOptions clientOptions) { return TracerProvider.getDefaultProvider() .createTracer(CLIENT_NAME, CLIENT_VERSION, STORAGE_TRACING_NAMESPACE_VALUE, tracingOptions); } + + /** + * Logs information about credential changes in builders. + * + * @param logger The logger to use. + * @param newCredentialType The credential type being set. + */ + public static void logCredentialChange(ClientLogger logger, String newCredentialType) { + logger.info("Credential set to '{}' when it was previously configured.", newCredentialType); + } } diff --git a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/SpecializedBlobClientBuilder.java b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/SpecializedBlobClientBuilder.java index 50fea2ae1bd3..54fd3682e72c 100644 --- a/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/SpecializedBlobClientBuilder.java +++ b/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/specialized/SpecializedBlobClientBuilder.java @@ -403,6 +403,10 @@ public SpecializedBlobClientBuilder encryptionScope(String encryptionScope) { */ public SpecializedBlobClientBuilder credential(StorageSharedKeyCredential credential) { this.storageSharedKeyCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.tokenCredential != null || this.sasToken != null) { + BuilderHelper.logCredentialChange(LOGGER, "StorageSharedKeyCredential"); + } this.tokenCredential = null; this.sasToken = null; return this; @@ -433,8 +437,11 @@ public SpecializedBlobClientBuilder credential(AzureNamedKeyCredential credentia @Override public SpecializedBlobClientBuilder credential(TokenCredential credential) { this.tokenCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "TokenCredential"); + } this.storageSharedKeyCredential = null; - this.sasToken = null; return this; } @@ -448,8 +455,11 @@ public SpecializedBlobClientBuilder credential(TokenCredential credential) { */ public SpecializedBlobClientBuilder sasToken(String sasToken) { this.sasToken = Objects.requireNonNull(sasToken, "'sasToken' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "sasToken"); + } this.storageSharedKeyCredential = null; - this.tokenCredential = null; return this; } 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 fcda9579f5c8..6ae92f47bac5 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 @@ -455,13 +455,6 @@ public void throwsOnAmbiguousCredentialsWithAzureSasCredential() { .credential(new AzureSasCredential("foo")) .buildClient()); - assertThrows(IllegalStateException.class, - () -> new BlobClientBuilder().endpoint(ENDPOINT) - .blobName("foo") - .credential(new MockTokenCredential()) - .credential(new AzureSasCredential("foo")) - .buildClient()); - assertThrows(IllegalStateException.class, () -> new BlobClientBuilder().endpoint(ENDPOINT) .blobName("foo") @@ -482,13 +475,6 @@ public void throwsOnAmbiguousCredentialsWithAzureSasCredential() { .credential(new AzureSasCredential("foo")) .buildBlockBlobClient()); - assertThrows(IllegalStateException.class, - () -> new SpecializedBlobClientBuilder().endpoint(ENDPOINT) - .blobName("foo") - .credential(new MockTokenCredential()) - .credential(new AzureSasCredential("foo")) - .buildBlockBlobClient()); - assertThrows(IllegalStateException.class, () -> new SpecializedBlobClientBuilder().endpoint(ENDPOINT) .blobName("foo") @@ -508,12 +494,6 @@ public void throwsOnAmbiguousCredentialsWithAzureSasCredential() { .credential(new AzureSasCredential("foo")) .buildClient()); - assertThrows(IllegalStateException.class, - () -> new BlobContainerClientBuilder().endpoint(ENDPOINT) - .credential(new MockTokenCredential()) - .credential(new AzureSasCredential("foo")) - .buildClient()); - assertThrows(IllegalStateException.class, () -> new BlobContainerClientBuilder().endpoint(ENDPOINT) .sasToken("foo") @@ -531,12 +511,6 @@ public void throwsOnAmbiguousCredentialsWithAzureSasCredential() { .credential(new AzureSasCredential("foo")) .buildClient()); - assertThrows(IllegalStateException.class, - () -> new BlobServiceClientBuilder().endpoint(ENDPOINT) - .credential(new MockTokenCredential()) - .credential(new AzureSasCredential("foo")) - .buildClient()); - assertThrows(IllegalStateException.class, () -> new BlobServiceClientBuilder().endpoint(ENDPOINT) .sasToken("foo") diff --git a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/SasAsyncClientTests.java b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/SasAsyncClientTests.java index 86b9c91304fc..888b3c67607f 100644 --- a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/SasAsyncClientTests.java +++ b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/SasAsyncClientTests.java @@ -48,8 +48,6 @@ import reactor.util.function.Tuple2; import reactor.util.function.Tuples; -import java.io.File; -import java.io.IOException; import java.nio.ByteBuffer; import java.time.LocalDateTime; import java.time.OffsetDateTime; @@ -211,9 +209,8 @@ public void blobSasUserDelegationDelegatedObjectId() { BlobSasPermission permissions = new BlobSasPermission().setReadPermission(true); OffsetDateTime expiryTime = testResourceNamer.now().plusHours(1); - TokenCredential tokenCredential = StorageCommonTestUtils.getTokenCredential(interceptorManager); - // We need to get the object ID from the token credential used to authenticate the request + TokenCredential tokenCredential = StorageCommonTestUtils.getTokenCredential(interceptorManager); String oid = getOidFromToken(tokenCredential); BlobServiceSasSignatureValues sasValues = new BlobServiceSasSignatureValues(expiryTime, permissions).setDelegatedUserObjectId(oid); @@ -231,7 +228,9 @@ public void blobSasUserDelegationDelegatedObjectId() { return client.getPropertiesWithResponse(null); }); - StepVerifier.create(response).assertNext(r -> assertResponseStatusCode(r, 200)).verifyComplete(); + StepVerifier.create(response) + .assertNext(StorageCommonTestUtils::verifySasAndTokenInRequest) + .verifyComplete(); }); } @@ -398,7 +397,7 @@ public void containerSasUserDelegationDelegatedObjectId() { BlobServiceSasSignatureValues sasValues = new BlobServiceSasSignatureValues(expiryTime, permissions).setDelegatedUserObjectId(oid); - Flux response = getUserDelegationInfo().flatMapMany(key -> { + Mono> response = getUserDelegationInfo().flatMap(key -> { String sas = ccAsync.generateUserDelegationSas(sasValues, key); // When a delegated user object ID is set, the client must be authenticated with both the SAS and the @@ -408,10 +407,12 @@ public void containerSasUserDelegationDelegatedObjectId() { .sasToken(sas) .credential(tokenCredential)).buildAsyncClient(); - return client.listBlobs(); + return client.getBlobAsyncClient(blobName).getBlockBlobAsyncClient().getPropertiesWithResponse(null); }); - StepVerifier.create(response).expectNextCount(1).verifyComplete(); + StepVerifier.create(response) + .assertNext(StorageCommonTestUtils::verifySasAndTokenInRequest) + .verifyComplete(); }); } @@ -697,7 +698,7 @@ public void blobSasEncryptionScope(boolean userDelegation) { } // Generate a sasClient that does not have an encryptionScope - sasClient = builder.sasToken(sas) + sasClient = getContainerClientBuilder(cc.getBlobContainerUrl()).sasToken(sas) .encryptionScope(null) .buildAsyncClient() .getBlobAsyncClient(sharedKeyClient.getBlobName()) @@ -893,7 +894,7 @@ public void accountSasCreateContainerSucceeds() { } @Test - public void accountSasOnEndpoint() throws IOException { + public void accountSasOnEndpoint() { AccountSasService service = new AccountSasService().setBlobAccess(true); AccountSasResourceType resourceType = new AccountSasResourceType().setContainer(true).setService(true).setObject(true); @@ -912,11 +913,11 @@ public void accountSasOnEndpoint() throws IOException { StepVerifier.create(response).expectNextCount(1).verifyComplete(); - BlobAsyncClient bc = getBlobAsyncClient(ENVIRONMENT.getPrimaryAccount().getCredential(), - primaryBlobServiceAsyncClient.getAccountUrl() + "/" + containerName + "/" + blobName + "?" + sas); - File file = getRandomFile(256); - file.deleteOnExit(); - StepVerifier.create(bc.uploadFromFile(file.toPath().toString(), true)).verifyComplete(); + BlobAsyncClient bc = instrument(new BlobClientBuilder() + .endpoint(primaryBlobServiceAsyncClient.getAccountUrl() + "/" + containerName + "/" + blobName + "?" + sas)) + .buildAsyncClient(); + + StepVerifier.create(bc.getProperties()).expectNextCount(1).verifyComplete(); } @Test diff --git a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/SasClientTests.java b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/SasClientTests.java index 6e6a0f25d112..7c9b6b53550e 100644 --- a/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/SasClientTests.java +++ b/sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/SasClientTests.java @@ -40,8 +40,6 @@ import org.junit.jupiter.params.provider.ValueSource; import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.IOException; import java.time.LocalDateTime; import java.time.OffsetDateTime; import java.time.ZoneOffset; @@ -50,6 +48,7 @@ import java.util.stream.Stream; import static com.azure.storage.common.test.shared.StorageCommonTestUtils.getOidFromToken; +import static com.azure.storage.common.test.shared.StorageCommonTestUtils.verifySasAndTokenInRequest; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -192,10 +191,10 @@ public void blobSasUserDelegationDelegatedObjectId() { BlobSasPermission permissions = new BlobSasPermission().setReadPermission(true); OffsetDateTime expiryTime = testResourceNamer.now().plusHours(1); - TokenCredential tokenCredential = StorageCommonTestUtils.getTokenCredential(interceptorManager); - // We need to get the object ID from the token credential used to authenticate the request + TokenCredential tokenCredential = StorageCommonTestUtils.getTokenCredential(interceptorManager); String oid = getOidFromToken(tokenCredential); + BlobServiceSasSignatureValues sasValues = new BlobServiceSasSignatureValues(expiryTime, permissions).setDelegatedUserObjectId(oid); String sas = sasClient.generateUserDelegationSas(sasValues, getUserDelegationInfo()); @@ -208,7 +207,7 @@ public void blobSasUserDelegationDelegatedObjectId() { .getBlockBlobClient(); Response response = client.getPropertiesWithResponse(null, null, Context.NONE); - assertResponseStatusCode(response, 200); + verifySasAndTokenInRequest(response); }); } @@ -350,7 +349,11 @@ public void containerSasUserDelegationDelegatedObjectId() { .sasToken(sas) .credential(tokenCredential)).buildClient(); - assertDoesNotThrow(() -> client.listBlobs().iterator().hasNext()); + Response response = client.getBlobClient(blobName) + .getBlockBlobClient() + .getPropertiesWithResponse(null, null, Context.NONE); + + verifySasAndTokenInRequest(response); }); } @@ -621,7 +624,7 @@ public void blobSasEncryptionScope(boolean userDelegation) { } // Generate a sasClient that does not have an encryptionScope - sasClient = builder.sasToken(sas) + sasClient = getContainerClientBuilder(cc.getBlobContainerUrl()).sasToken(sas) .encryptionScope(null) .buildClient() .getBlobClient(sharedKeyClient.getBlobName()) @@ -805,7 +808,7 @@ public void accountSasCreateContainerSucceeds() { } @Test - public void accountSasOnEndpoint() throws IOException { + public void accountSasOnEndpoint() { AccountSasService service = new AccountSasService().setBlobAccess(true); AccountSasResourceType resourceType = new AccountSasResourceType().setContainer(true).setService(true).setObject(true); @@ -816,18 +819,18 @@ public void accountSasOnEndpoint() throws IOException { String sas = primaryBlobServiceClient.generateAccountSas(sasValues); BlobServiceClient sc = getServiceClient(primaryBlobServiceClient.getAccountUrl() + "?" + sas); - sc.createBlobContainer(generateContainerName()); + assertDoesNotThrow(() -> sc.createBlobContainer(generateContainerName())); BlobContainerClient cc = getContainerClientBuilder(primaryBlobServiceClient.getAccountUrl() + "/" + containerName + "?" + sas) .buildClient(); assertDoesNotThrow(cc::getProperties); - BlobClient bc = getBlobClient(ENVIRONMENT.getPrimaryAccount().getCredential(), - primaryBlobServiceClient.getAccountUrl() + "/" + containerName + "/" + blobName + "?" + sas); - File file = getRandomFile(256); - file.deleteOnExit(); - assertDoesNotThrow(() -> bc.uploadFromFile(file.toPath().toString(), true)); + BlobClient bc = instrument(new BlobClientBuilder() + .endpoint(primaryBlobServiceClient.getAccountUrl() + "/" + containerName + "/" + blobName + "?" + sas)) + .buildClient(); + + assertDoesNotThrow(bc::getProperties); } @Test diff --git a/sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/credentials/CredentialValidator.java b/sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/credentials/CredentialValidator.java index bb43c7bb14a6..46decc0bda7f 100644 --- a/sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/credentials/CredentialValidator.java +++ b/sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/credentials/CredentialValidator.java @@ -15,7 +15,7 @@ /** * This class provides helper methods for validation of credentials used in storage. - * + *
* RESERVED FOR INTERNAL USE. */ public class CredentialValidator { @@ -41,4 +41,29 @@ public static void validateSingleCredentialIsPresent(StorageSharedKeyCredential .collect(Collectors.joining(",")))); } } + + public static void validateCredentialsNotAmbiguous(StorageSharedKeyCredential storageSharedKeyCredential, + TokenCredential tokenCredential, AzureSasCredential azureSasCredential, String sasToken, ClientLogger logger) { + boolean hasSharedKey = storageSharedKeyCredential != null; + boolean hasToken = tokenCredential != null; + boolean hasAzureSas = azureSasCredential != null; + boolean hasTokenSas = sasToken != null; + + int credentialCount + = (hasSharedKey ? 1 : 0) + (hasToken ? 1 : 0) + (hasAzureSas ? 1 : 0) + (hasTokenSas ? 1 : 0); + + if (credentialCount >= 3) { + throw logger.logExceptionAsError(new IllegalStateException( + "Too many credentials specified. A maximum of two credentials is supported for specific scenarios.")); + } + + if (credentialCount == 2) { + // Only allow: tokenCredential + (azureSasCredential or sasToken) + boolean validCombo = hasToken && (hasAzureSas ^ hasTokenSas); + if (!validCombo) { + throw logger.logExceptionAsError(new IllegalStateException( + "Invalid combination of credentials. Only TokenCredential with either AzureSasCredential or sasToken is supported when two credentials are provided.")); + } + } + } } diff --git a/sdk/storage/azure-storage-common/src/test-shared/java/com/azure/storage/common/test/shared/StorageCommonTestUtils.java b/sdk/storage/azure-storage-common/src/test-shared/java/com/azure/storage/common/test/shared/StorageCommonTestUtils.java index f6790e20d37d..5b840a073823 100644 --- a/sdk/storage/azure-storage-common/src/test-shared/java/com/azure/storage/common/test/shared/StorageCommonTestUtils.java +++ b/sdk/storage/azure-storage-common/src/test-shared/java/com/azure/storage/common/test/shared/StorageCommonTestUtils.java @@ -10,6 +10,7 @@ import com.azure.core.http.netty.NettyAsyncHttpClientProvider; import com.azure.core.http.okhttp.OkHttpAsyncClientProvider; import com.azure.core.http.policy.HttpLogOptions; +import com.azure.core.http.rest.Response; import com.azure.core.test.InterceptorManager; import com.azure.core.test.TestMode; import com.azure.core.test.utils.MockTokenCredential; @@ -51,6 +52,7 @@ import static java.util.Base64.getUrlDecoder; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * This class contains utility methods for Storage tests. @@ -422,4 +424,16 @@ public static String getOidFromToken(TokenCredential credential) { } throw new RuntimeException("Could not find oid in token"); } + + public static void verifySasAndTokenInRequest(Response response) { + assertResponseStatusCode(response, 200); + //assert sas token exists in URL + auth header exists + assertTrue(response.getRequest().getHeaders().stream().anyMatch(h -> h.getName().equals("Authorization"))); + assertTrue(response.getRequest().getUrl().toString().contains("sv=" + Constants.SAS_SERVICE_VERSION)); + } + + public static Response assertResponseStatusCode(Response response, int expectedStatusCode) { + assertEquals(expectedStatusCode, response.getStatusCode()); + return response; + } } diff --git a/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeFileSystemClientBuilder.java b/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeFileSystemClientBuilder.java index 146bff7b68f4..e1cd034825fe 100644 --- a/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeFileSystemClientBuilder.java +++ b/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeFileSystemClientBuilder.java @@ -216,6 +216,10 @@ public DataLakeFileSystemClientBuilder endpoint(String endpoint) { public DataLakeFileSystemClientBuilder credential(StorageSharedKeyCredential credential) { blobContainerClientBuilder.credential(credential); this.storageSharedKeyCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.tokenCredential != null || this.azureSasCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "StorageSharedKeyCredential"); + } this.tokenCredential = null; this.azureSasCredential = null; return this; @@ -247,8 +251,11 @@ public DataLakeFileSystemClientBuilder credential(AzureNamedKeyCredential creden public DataLakeFileSystemClientBuilder credential(TokenCredential credential) { blobContainerClientBuilder.credential(credential); this.tokenCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "TokenCredential"); + } this.storageSharedKeyCredential = null; - this.azureSasCredential = null; return this; } @@ -264,8 +271,11 @@ public DataLakeFileSystemClientBuilder sasToken(String sasToken) { blobContainerClientBuilder.sasToken(sasToken); this.azureSasCredential = new AzureSasCredential(Objects.requireNonNull(sasToken, "'sasToken' cannot be null.")); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "sasToken"); + } this.storageSharedKeyCredential = null; - this.tokenCredential = null; return this; } diff --git a/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathClientBuilder.java b/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathClientBuilder.java index c012f854269d..667da59d3de9 100644 --- a/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathClientBuilder.java +++ b/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathClientBuilder.java @@ -276,6 +276,10 @@ private void validateConstruction() { public DataLakePathClientBuilder credential(StorageSharedKeyCredential credential) { blobClientBuilder.credential(credential); this.storageSharedKeyCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.tokenCredential != null || this.azureSasCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "StorageSharedKeyCredential"); + } this.tokenCredential = null; this.azureSasCredential = null; return this; @@ -307,8 +311,11 @@ public DataLakePathClientBuilder credential(AzureNamedKeyCredential credential) public DataLakePathClientBuilder credential(TokenCredential credential) { blobClientBuilder.credential(credential); this.tokenCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "TokenCredential"); + } this.storageSharedKeyCredential = null; - this.azureSasCredential = null; return this; } @@ -324,8 +331,11 @@ public DataLakePathClientBuilder sasToken(String sasToken) { blobClientBuilder.sasToken(sasToken); this.azureSasCredential = new AzureSasCredential(Objects.requireNonNull(sasToken, "'sasToken' cannot be null.")); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "sasToken"); + } this.storageSharedKeyCredential = null; - this.tokenCredential = null; return this; } diff --git a/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeServiceClientBuilder.java b/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeServiceClientBuilder.java index b4222426087f..088b981fc19e 100644 --- a/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeServiceClientBuilder.java +++ b/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeServiceClientBuilder.java @@ -189,6 +189,10 @@ public DataLakeServiceClientBuilder endpoint(String endpoint) { public DataLakeServiceClientBuilder credential(StorageSharedKeyCredential credential) { blobServiceClientBuilder.credential(credential); this.storageSharedKeyCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.tokenCredential != null || this.azureSasCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "StorageSharedKeyCredential"); + } this.tokenCredential = null; this.azureSasCredential = null; return this; @@ -220,8 +224,11 @@ public DataLakeServiceClientBuilder credential(AzureNamedKeyCredential credentia public DataLakeServiceClientBuilder credential(TokenCredential credential) { blobServiceClientBuilder.credential(credential); this.tokenCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "TokenCredential"); + } this.storageSharedKeyCredential = null; - this.azureSasCredential = null; return this; } @@ -237,8 +244,11 @@ public DataLakeServiceClientBuilder sasToken(String sasToken) { blobServiceClientBuilder.sasToken(sasToken); this.azureSasCredential = new AzureSasCredential(Objects.requireNonNull(sasToken, "'sasToken' cannot be null.")); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "sasToken"); + } this.storageSharedKeyCredential = null; - this.tokenCredential = null; return this; } diff --git a/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/implementation/util/BuilderHelper.java b/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/implementation/util/BuilderHelper.java index 9b8b7974f9bd..367c239c2d02 100644 --- a/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/implementation/util/BuilderHelper.java +++ b/sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/implementation/util/BuilderHelper.java @@ -94,7 +94,7 @@ public static HttpPipeline buildPipeline(StorageSharedKeyCredential storageShare List perRetryPolicies, Configuration configuration, DataLakeAudience audience, ClientLogger logger) { - CredentialValidator.validateSingleCredentialIsPresent(storageSharedKeyCredential, tokenCredential, + CredentialValidator.validateCredentialsNotAmbiguous(storageSharedKeyCredential, tokenCredential, azureSasCredential, null, logger); // Closest to API goes first, closest to wire goes last. @@ -119,23 +119,20 @@ public static HttpPipeline buildPipeline(StorageSharedKeyCredential storageShare } policies.add(new MetadataValidationPolicy()); - HttpPipelinePolicy credentialPolicy; if (storageSharedKeyCredential != null) { - credentialPolicy = new StorageSharedKeyCredentialPolicy(storageSharedKeyCredential); - } else if (tokenCredential != null) { + policies.add(new StorageSharedKeyCredentialPolicy(storageSharedKeyCredential)); + } + + if (tokenCredential != null) { // The endpoint scope for the BearerToken is the blob endpoint not dfs String scope = audience != null ? ((audience.toString().endsWith("/") ? audience + ".default" : audience + "/.default")) : Constants.STORAGE_SCOPE; - credentialPolicy = new StorageBearerTokenChallengeAuthorizationPolicy(tokenCredential, scope); - } else if (azureSasCredential != null) { - credentialPolicy = new AzureSasCredentialPolicy(azureSasCredential, false); - } else { - credentialPolicy = null; + policies.add(new StorageBearerTokenChallengeAuthorizationPolicy(tokenCredential, scope)); } - if (credentialPolicy != null) { - policies.add(credentialPolicy); + if (azureSasCredential != null) { + policies.add(new AzureSasCredentialPolicy(azureSasCredential, false)); } policies.addAll(perRetryPolicies); @@ -264,4 +261,14 @@ public static Context skipResponseValidationForEncryptionKey(Context context) { } } + + /** + * Logs information about credential changes in builders. + * + * @param logger The logger to use. + * @param newCredentialType The credential type being set. + */ + public static void logCredentialChange(ClientLogger logger, String newCredentialType) { + logger.info("Credential set to '{}' when it was previously configured.", newCredentialType); + } } diff --git a/sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/BuilderHelperTests.java b/sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/BuilderHelperTests.java index d963a1f08088..55e0a528a480 100644 --- a/sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/BuilderHelperTests.java +++ b/sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/BuilderHelperTests.java @@ -262,12 +262,6 @@ public void throwsOnAmbiguousCredentialsWithAzureSasCredential() { .credential(new AzureSasCredential("foo")) .buildClient()); - assertThrows(IllegalStateException.class, - () -> new DataLakeFileSystemClientBuilder().endpoint(ENDPOINT) - .credential(new MockTokenCredential()) - .credential(new AzureSasCredential("foo")) - .buildClient()); - assertThrows(IllegalStateException.class, () -> new DataLakeFileSystemClientBuilder().endpoint(ENDPOINT) .sasToken("foo") @@ -286,13 +280,6 @@ public void throwsOnAmbiguousCredentialsWithAzureSasCredential() { .credential(new AzureSasCredential("foo")) .buildDirectoryClient()); - assertThrows(IllegalStateException.class, - () -> new DataLakePathClientBuilder().endpoint(ENDPOINT) - .pathName("foo") - .credential(new MockTokenCredential()) - .credential(new AzureSasCredential("foo")) - .buildDirectoryClient()); - assertThrows(IllegalStateException.class, () -> new DataLakePathClientBuilder().endpoint(ENDPOINT) .pathName("foo") @@ -312,12 +299,6 @@ public void throwsOnAmbiguousCredentialsWithAzureSasCredential() { .credential(new AzureSasCredential("foo")) .buildClient()); - assertThrows(IllegalStateException.class, - () -> new DataLakeServiceClientBuilder().endpoint(ENDPOINT) - .credential(new MockTokenCredential()) - .credential(new AzureSasCredential("foo")) - .buildClient()); - assertThrows(IllegalStateException.class, () -> new DataLakeServiceClientBuilder().endpoint(ENDPOINT) .sasToken("foo") diff --git a/sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/SasAsyncTests.java b/sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/SasAsyncTests.java index 198c18abdc87..71281779ca9c 100644 --- a/sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/SasAsyncTests.java +++ b/sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/SasAsyncTests.java @@ -134,7 +134,7 @@ public void fileDatalakeSasUserDelegationDelegatedObjectId() { .credential(tokenCredential)).buildFileAsyncClient(); StepVerifier.create(client.getPropertiesWithResponse(null)) - .assertNext(r -> assertEquals(200, r.getStatusCode())) + .assertNext(StorageCommonTestUtils::verifySasAndTokenInRequest) .verifyComplete(); }); } @@ -248,7 +248,7 @@ public void directorySasUserDelegationDelegatedObjectId() { .credential(tokenCredential)).buildDirectoryAsyncClient(); StepVerifier.create(client.getPropertiesWithResponse(null)) - .assertNext(r -> assertEquals(200, r.getStatusCode())) + .assertNext(StorageCommonTestUtils::verifySasAndTokenInRequest) .verifyComplete(); }); } @@ -615,7 +615,9 @@ public void fileSystemSasUserDelegationDelegatedObjectId() { .sasToken(sas) .credential(tokenCredential)).buildAsyncClient(); - StepVerifier.create(client.listPaths().collectList()).expectNextCount(1).verifyComplete(); + StepVerifier.create(client.getFileAsyncClient(pathName).getPropertiesWithResponse(null)) + .assertNext(StorageCommonTestUtils::verifySasAndTokenInRequest) + .verifyComplete(); }); } diff --git a/sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/SasTests.java b/sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/SasTests.java index 9e6979070c7c..5299216d813a 100644 --- a/sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/SasTests.java +++ b/sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/SasTests.java @@ -4,6 +4,7 @@ import com.azure.core.credential.AzureSasCredential; import com.azure.core.credential.TokenCredential; +import com.azure.core.http.rest.Response; import com.azure.core.util.Context; import com.azure.storage.blob.specialized.BlockBlobClient; import com.azure.storage.common.implementation.Constants; @@ -46,6 +47,7 @@ import java.util.stream.Stream; import static com.azure.storage.common.test.shared.StorageCommonTestUtils.getOidFromToken; +import static com.azure.storage.common.test.shared.StorageCommonTestUtils.verifySasAndTokenInRequest; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -145,7 +147,8 @@ public void fileDatalakeSasUserDelegationDelegatedObjectId() { .sasToken(sas) .credential(tokenCredential)).buildFileClient(); - assertDoesNotThrow(() -> client.getPropertiesWithResponse(null, null, Context.NONE)); + Response response = client.getPropertiesWithResponse(null, null, Context.NONE); + verifySasAndTokenInRequest(response); }); } @@ -247,7 +250,8 @@ public void directorySasUserDelegationDelegatedObjectId() { .sasToken(sas) .credential(tokenCredential)).buildDirectoryClient(); - assertDoesNotThrow(() -> client.getPropertiesWithResponse(null, null, Context.NONE)); + Response response = client.getPropertiesWithResponse(null, null, Context.NONE); + verifySasAndTokenInRequest(response); }); } @@ -601,7 +605,10 @@ public void fileSystemSasUserDelegationDelegatedObjectId() { .sasToken(sas) .credential(tokenCredential)).buildClient(); - assertDoesNotThrow(() -> client.listPaths().iterator().hasNext()); + Response response + = client.getFileClient(pathName).getPropertiesWithResponse(null, null, Context.NONE); + + verifySasAndTokenInRequest(response); }); } diff --git a/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/ShareClientBuilder.java b/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/ShareClientBuilder.java index 27033bf41b82..60330e2073e4 100644 --- a/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/ShareClientBuilder.java +++ b/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/ShareClientBuilder.java @@ -312,6 +312,10 @@ public ShareClientBuilder snapshot(String snapshot) { */ public ShareClientBuilder credential(StorageSharedKeyCredential credential) { this.storageSharedKeyCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.tokenCredential != null || this.sasToken != null) { + BuilderHelper.logCredentialChange(LOGGER, "StorageSharedKeyCredential"); + } this.tokenCredential = null; this.sasToken = null; return this; @@ -345,8 +349,11 @@ public ShareClientBuilder credential(AzureNamedKeyCredential credential) { @Override public ShareClientBuilder credential(TokenCredential tokenCredential) { this.tokenCredential = Objects.requireNonNull(tokenCredential, "'credential' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "TokenCredential"); + } this.storageSharedKeyCredential = null; - this.sasToken = null; return this; } @@ -360,8 +367,11 @@ public ShareClientBuilder credential(TokenCredential tokenCredential) { */ public ShareClientBuilder sasToken(String sasToken) { this.sasToken = Objects.requireNonNull(sasToken, "'sasToken' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "sasToken"); + } this.storageSharedKeyCredential = null; - this.tokenCredential = null; return this; } diff --git a/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/ShareFileClientBuilder.java b/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/ShareFileClientBuilder.java index 6bb9abce758c..fe1fbc329ade 100644 --- a/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/ShareFileClientBuilder.java +++ b/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/ShareFileClientBuilder.java @@ -401,6 +401,10 @@ public ShareFileClientBuilder resourcePath(String resourcePath) { */ public ShareFileClientBuilder credential(StorageSharedKeyCredential credential) { this.storageSharedKeyCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.tokenCredential != null || this.sasToken != null) { + BuilderHelper.logCredentialChange(LOGGER, "StorageSharedKeyCredential"); + } this.tokenCredential = null; this.sasToken = null; return this; @@ -431,8 +435,11 @@ public ShareFileClientBuilder credential(AzureNamedKeyCredential credential) { @Override public ShareFileClientBuilder credential(TokenCredential tokenCredential) { this.tokenCredential = Objects.requireNonNull(tokenCredential, "'credential' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "TokenCredential"); + } this.storageSharedKeyCredential = null; - this.sasToken = null; return this; } @@ -446,8 +453,11 @@ public ShareFileClientBuilder credential(TokenCredential tokenCredential) { */ public ShareFileClientBuilder sasToken(String sasToken) { this.sasToken = Objects.requireNonNull(sasToken, "'sasToken' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "sasToken"); + } this.storageSharedKeyCredential = null; - this.tokenCredential = null; return this; } diff --git a/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/ShareServiceClientBuilder.java b/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/ShareServiceClientBuilder.java index 265c5c3008ee..dddee6529049 100644 --- a/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/ShareServiceClientBuilder.java +++ b/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/ShareServiceClientBuilder.java @@ -256,6 +256,10 @@ public ShareServiceClientBuilder endpoint(String endpoint) { */ public ShareServiceClientBuilder credential(StorageSharedKeyCredential credential) { this.storageSharedKeyCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.tokenCredential != null || this.sasToken != null) { + BuilderHelper.logCredentialChange(LOGGER, "StorageSharedKeyCredential"); + } this.tokenCredential = null; this.sasToken = null; return this; @@ -289,8 +293,11 @@ public ShareServiceClientBuilder credential(AzureNamedKeyCredential credential) @Override public ShareServiceClientBuilder credential(TokenCredential tokenCredential) { this.tokenCredential = Objects.requireNonNull(tokenCredential, "'credential' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "TokenCredential"); + } this.storageSharedKeyCredential = null; - this.sasToken = null; return this; } @@ -304,6 +311,10 @@ public ShareServiceClientBuilder credential(TokenCredential tokenCredential) { */ public ShareServiceClientBuilder sasToken(String sasToken) { this.sasToken = Objects.requireNonNull(sasToken, "'sasToken' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "sasToken"); + } this.storageSharedKeyCredential = null; return this; } diff --git a/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/implementation/util/BuilderHelper.java b/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/implementation/util/BuilderHelper.java index a15d3fa8db47..c58fbdddd2a8 100644 --- a/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/implementation/util/BuilderHelper.java +++ b/sdk/storage/azure-storage-file-share/src/main/java/com/azure/storage/file/share/implementation/util/BuilderHelper.java @@ -93,7 +93,7 @@ public static HttpPipeline buildPipeline(StorageSharedKeyCredential storageShare List perRetryPolicies, Configuration configuration, ShareAudience audience, ClientLogger logger) { - CredentialValidator.validateSingleCredentialIsPresent(storageSharedKeyCredential, tokenCredential, + CredentialValidator.validateCredentialsNotAmbiguous(storageSharedKeyCredential, tokenCredential, azureSasCredential, sasToken, logger); // Closest to API goes first, closest to wire goes last. @@ -116,25 +116,22 @@ public static HttpPipeline buildPipeline(StorageSharedKeyCredential storageShare } policies.add(new MetadataValidationPolicy()); - HttpPipelinePolicy credentialPolicy; if (storageSharedKeyCredential != null) { - credentialPolicy = new StorageSharedKeyCredentialPolicy(storageSharedKeyCredential); - } else if (tokenCredential != null) { + policies.add(new StorageSharedKeyCredentialPolicy(storageSharedKeyCredential)); + } + + if (tokenCredential != null) { httpsValidation(tokenCredential, "bearer token", endpoint, logger); String scope = audience != null ? ((audience.toString().endsWith("/") ? audience + ".default" : audience + "/.default")) : Constants.STORAGE_SCOPE; - credentialPolicy = new StorageBearerTokenChallengeAuthorizationPolicy(tokenCredential, scope); - } else if (azureSasCredential != null) { - credentialPolicy = new AzureSasCredentialPolicy(azureSasCredential, false); - } else if (sasToken != null) { - credentialPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(sasToken), false); - } else { - credentialPolicy = null; + policies.add(new StorageBearerTokenChallengeAuthorizationPolicy(tokenCredential, scope)); } - if (credentialPolicy != null) { - policies.add(credentialPolicy); + if (azureSasCredential != null) { + policies.add(new AzureSasCredentialPolicy(azureSasCredential, false)); + } else if (sasToken != null) { + policies.add(new AzureSasCredentialPolicy(new AzureSasCredential(sasToken), false)); } policies.addAll(perRetryPolicies); @@ -433,4 +430,14 @@ private static Tracer createTracer(ClientOptions clientOptions) { return TracerProvider.getDefaultProvider() .createTracer(CLIENT_NAME, CLIENT_VERSION, STORAGE_TRACING_NAMESPACE_VALUE, tracingOptions); } + + /** + * Logs information about credential changes in builders. + * + * @param logger The logger to use. + * @param newCredentialType The credential type being set. + */ + public static void logCredentialChange(ClientLogger logger, String newCredentialType) { + logger.info("Credential set to '{}' when it was previously configured.", newCredentialType); + } } diff --git a/sdk/storage/azure-storage-file-share/src/test/java/com/azure/storage/file/share/FileSasClientTests.java b/sdk/storage/azure-storage-file-share/src/test/java/com/azure/storage/file/share/FileSasClientTests.java index 6d72eb6e6091..6a5dbde579a2 100644 --- a/sdk/storage/azure-storage-file-share/src/test/java/com/azure/storage/file/share/FileSasClientTests.java +++ b/sdk/storage/azure-storage-file-share/src/test/java/com/azure/storage/file/share/FileSasClientTests.java @@ -42,6 +42,7 @@ import java.util.Arrays; import static com.azure.storage.common.test.shared.StorageCommonTestUtils.getOidFromToken; +import static com.azure.storage.common.test.shared.StorageCommonTestUtils.verifySasAndTokenInRequest; import static com.azure.storage.file.share.FileShareTestHelper.assertExceptionStatusCodeAndMessage; import static com.azure.storage.file.share.FileShareTestHelper.assertResponseStatusCode; import static org.junit.jupiter.api.Assertions.assertArrayEquals; @@ -228,7 +229,7 @@ public void fileSasUserDelegationDelegatedObjectId() { .credential(tokenCredential)).buildFileClient(); Response response = client.getPropertiesWithResponse(null, Context.NONE); - FileShareTestHelper.assertResponseStatusCode(response, 200); + verifySasAndTokenInRequest(response); }); } @@ -262,7 +263,9 @@ public void fileSasUserDelegationDelegatedObjectIdAsync() { return client.getPropertiesWithResponse(); }); - StepVerifier.create(response).assertNext(r -> assertResponseStatusCode(r, 200)).verifyComplete(); + StepVerifier.create(response) + .assertNext(StorageCommonTestUtils::verifySasAndTokenInRequest) + .verifyComplete(); }); } @@ -354,8 +357,9 @@ public void shareSasUserDelegationDelegatedObjectId() { .shareTokenIntent(ShareTokenIntent.BACKUP) .credential(tokenCredential)).buildClient(); - Response response = client.getPropertiesWithResponse(null, Context.NONE); - FileShareTestHelper.assertResponseStatusCode(response, 200); + Response response + = client.getFileClient(filePath).getPropertiesWithResponse(null, Context.NONE); + verifySasAndTokenInRequest(response); }); } @@ -376,7 +380,7 @@ public void shareSasUserDelegationDelegatedObjectIdAsync() { ShareServiceSasSignatureValues sasValues = new ShareServiceSasSignatureValues(expiryTime, permissions).setDelegatedUserObjectId(oid); - Flux> response = getUserDelegationInfoAsync().flatMapMany(key -> { + Mono> response = getUserDelegationInfoAsync().flatMap(key -> { String sas = primaryShareAsyncClient.generateUserDelegationSas(sasValues, key); // When a delegated user object ID is set, the client must be authenticated with both the SAS and the // token credential. @@ -386,10 +390,12 @@ public void shareSasUserDelegationDelegatedObjectIdAsync() { .shareTokenIntent(ShareTokenIntent.BACKUP) .credential(tokenCredential)).buildAsyncClient(); - return client.getPropertiesWithResponse(); + return client.getFileClient(filePath).getPropertiesWithResponse(); }); - StepVerifier.create(response).assertNext(r -> assertResponseStatusCode(r, 200)).verifyComplete(); + StepVerifier.create(response) + .assertNext(StorageCommonTestUtils::verifySasAndTokenInRequest) + .verifyComplete(); }); } diff --git a/sdk/storage/azure-storage-queue/src/main/java/com/azure/storage/queue/QueueClientBuilder.java b/sdk/storage/azure-storage-queue/src/main/java/com/azure/storage/queue/QueueClientBuilder.java index d1fa92d082c1..f7e6d6f724ed 100644 --- a/sdk/storage/azure-storage-queue/src/main/java/com/azure/storage/queue/QueueClientBuilder.java +++ b/sdk/storage/azure-storage-queue/src/main/java/com/azure/storage/queue/QueueClientBuilder.java @@ -292,6 +292,10 @@ public QueueClientBuilder queueName(String queueName) { */ public QueueClientBuilder credential(StorageSharedKeyCredential credential) { this.storageSharedKeyCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.tokenCredential != null || this.sasToken != null) { + BuilderHelper.logCredentialChange(LOGGER, "StorageSharedKeyCredential"); + } this.tokenCredential = null; this.sasToken = null; return this; @@ -322,8 +326,11 @@ public QueueClientBuilder credential(AzureNamedKeyCredential credential) { @Override public QueueClientBuilder credential(TokenCredential credential) { this.tokenCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "TokenCredential"); + } this.storageSharedKeyCredential = null; - this.sasToken = null; return this; } @@ -337,8 +344,11 @@ public QueueClientBuilder credential(TokenCredential credential) { */ public QueueClientBuilder sasToken(String sasToken) { this.sasToken = Objects.requireNonNull(sasToken, "'sasToken' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "sasToken"); + } this.storageSharedKeyCredential = null; - this.tokenCredential = null; return this; } diff --git a/sdk/storage/azure-storage-queue/src/main/java/com/azure/storage/queue/QueueServiceClientBuilder.java b/sdk/storage/azure-storage-queue/src/main/java/com/azure/storage/queue/QueueServiceClientBuilder.java index 02b89b6cb145..917b4e424250 100644 --- a/sdk/storage/azure-storage-queue/src/main/java/com/azure/storage/queue/QueueServiceClientBuilder.java +++ b/sdk/storage/azure-storage-queue/src/main/java/com/azure/storage/queue/QueueServiceClientBuilder.java @@ -262,6 +262,10 @@ public QueueServiceClientBuilder endpoint(String endpoint) { */ public QueueServiceClientBuilder credential(StorageSharedKeyCredential credential) { this.storageSharedKeyCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.tokenCredential != null || this.sasToken != null) { + BuilderHelper.logCredentialChange(LOGGER, "StorageSharedKeyCredential"); + } this.tokenCredential = null; this.sasToken = null; return this; @@ -292,8 +296,11 @@ public QueueServiceClientBuilder credential(AzureNamedKeyCredential credential) @Override public QueueServiceClientBuilder credential(TokenCredential credential) { this.tokenCredential = Objects.requireNonNull(credential, "'credential' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "TokenCredential"); + } this.storageSharedKeyCredential = null; - this.sasToken = null; return this; } @@ -307,8 +314,11 @@ public QueueServiceClientBuilder credential(TokenCredential credential) { */ public QueueServiceClientBuilder sasToken(String sasToken) { this.sasToken = Objects.requireNonNull(sasToken, "'sasToken' cannot be null."); + + if (this.storageSharedKeyCredential != null) { + BuilderHelper.logCredentialChange(LOGGER, "sasToken"); + } this.storageSharedKeyCredential = null; - this.tokenCredential = null; return this; } diff --git a/sdk/storage/azure-storage-queue/src/main/java/com/azure/storage/queue/implementation/util/BuilderHelper.java b/sdk/storage/azure-storage-queue/src/main/java/com/azure/storage/queue/implementation/util/BuilderHelper.java index 3276b785e957..c19628576232 100644 --- a/sdk/storage/azure-storage-queue/src/main/java/com/azure/storage/queue/implementation/util/BuilderHelper.java +++ b/sdk/storage/azure-storage-queue/src/main/java/com/azure/storage/queue/implementation/util/BuilderHelper.java @@ -169,7 +169,7 @@ public static HttpPipeline buildPipeline(StorageSharedKeyCredential storageShare List perRetryPolicies, Configuration configuration, QueueAudience audience, ClientLogger logger) { - CredentialValidator.validateSingleCredentialIsPresent(storageSharedKeyCredential, tokenCredential, + CredentialValidator.validateCredentialsNotAmbiguous(storageSharedKeyCredential, tokenCredential, azureSasCredential, sasToken, logger); // Closest to API goes first, closest to wire goes last. @@ -192,25 +192,20 @@ public static HttpPipeline buildPipeline(StorageSharedKeyCredential storageShare } policies.add(new MetadataValidationPolicy()); - HttpPipelinePolicy credentialPolicy; if (storageSharedKeyCredential != null) { - credentialPolicy = new StorageSharedKeyCredentialPolicy(storageSharedKeyCredential); - } else if (tokenCredential != null) { + policies.add(new StorageSharedKeyCredentialPolicy(storageSharedKeyCredential)); + } + if (tokenCredential != null) { httpsValidation(tokenCredential, "bearer token", endpoint, logger); String scope = audience != null ? ((audience.toString().endsWith("/") ? audience + ".default" : audience + "/.default")) : Constants.STORAGE_SCOPE; - credentialPolicy = new StorageBearerTokenChallengeAuthorizationPolicy(tokenCredential, scope); - } else if (azureSasCredential != null) { - credentialPolicy = new AzureSasCredentialPolicy(azureSasCredential, false); - } else if (sasToken != null) { - credentialPolicy = new AzureSasCredentialPolicy(new AzureSasCredential(sasToken), false); - } else { - credentialPolicy = null; + policies.add(new StorageBearerTokenChallengeAuthorizationPolicy(tokenCredential, scope)); } - - if (credentialPolicy != null) { - policies.add(credentialPolicy); + if (azureSasCredential != null) { + policies.add(new AzureSasCredentialPolicy(azureSasCredential, false)); + } else if (sasToken != null) { + policies.add(new AzureSasCredentialPolicy(new AzureSasCredential(sasToken), false)); } policies.addAll(perRetryPolicies); @@ -340,4 +335,14 @@ public QueueUrlParts setSasToken(String sasToken) { return this; } } + + /** + * Logs information about credential changes in builders. + * + * @param logger The logger to use. + * @param newCredentialType The credential type being set. + */ + public static void logCredentialChange(ClientLogger logger, String newCredentialType) { + logger.info("Credential set to '{}' when it was previously configured.", newCredentialType); + } } diff --git a/sdk/storage/azure-storage-queue/src/test/java/com/azure/storage/queue/BuildHelperTests.java b/sdk/storage/azure-storage-queue/src/test/java/com/azure/storage/queue/BuildHelperTests.java index fd71d2cfdd5b..c9fc01347057 100644 --- a/sdk/storage/azure-storage-queue/src/test/java/com/azure/storage/queue/BuildHelperTests.java +++ b/sdk/storage/azure-storage-queue/src/test/java/com/azure/storage/queue/BuildHelperTests.java @@ -253,13 +253,6 @@ public void throwsOnAmbiguousCredentialsWithAzureSasCredential() { .credential(new AzureSasCredential("foo")) .buildClient()); - assertThrows(IllegalStateException.class, - () -> new QueueClientBuilder().endpoint(ENDPOINT) - .queueName("foo") - .credential(MOCK_CREDENTIAL) - .credential(new AzureSasCredential("foo")) - .buildClient()); - assertThrows(IllegalStateException.class, () -> new QueueClientBuilder().endpoint(ENDPOINT) .queueName("foo") @@ -279,12 +272,6 @@ public void throwsOnAmbiguousCredentialsWithAzureSasCredential() { .credential(new AzureSasCredential("foo")) .buildClient()); - assertThrows(IllegalStateException.class, - () -> new QueueServiceClientBuilder().endpoint(ENDPOINT) - .credential(MOCK_CREDENTIAL) - .credential(new AzureSasCredential("foo")) - .buildClient()); - assertThrows(IllegalStateException.class, () -> new QueueServiceClientBuilder().endpoint(ENDPOINT) .sasToken("foo") diff --git a/sdk/storage/azure-storage-queue/src/test/java/com/azure/storage/queue/QueueSasAsyncClientTests.java b/sdk/storage/azure-storage-queue/src/test/java/com/azure/storage/queue/QueueSasAsyncClientTests.java index b042584fd886..f4c9dd52d4b8 100644 --- a/sdk/storage/azure-storage-queue/src/test/java/com/azure/storage/queue/QueueSasAsyncClientTests.java +++ b/sdk/storage/azure-storage-queue/src/test/java/com/azure/storage/queue/QueueSasAsyncClientTests.java @@ -37,7 +37,6 @@ import static com.azure.storage.common.test.shared.StorageCommonTestUtils.getOidFromToken; import static com.azure.storage.queue.QueueTestHelper.assertExceptionStatusCodeAndMessage; -import static com.azure.storage.queue.QueueTestHelper.assertResponseStatusCode; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -204,7 +203,9 @@ public void queueSasUserDelegationDelegatedObjectId() { return client.getPropertiesWithResponse(); }); - StepVerifier.create(response).assertNext(r -> assertResponseStatusCode(r, 200)).verifyComplete(); + StepVerifier.create(response) + .assertNext(StorageCommonTestUtils::verifySasAndTokenInRequest) + .verifyComplete(); }); } diff --git a/sdk/storage/azure-storage-queue/src/test/java/com/azure/storage/queue/QueueSasClientTests.java b/sdk/storage/azure-storage-queue/src/test/java/com/azure/storage/queue/QueueSasClientTests.java index 0a6c9df1466d..da9bd68121cf 100644 --- a/sdk/storage/azure-storage-queue/src/test/java/com/azure/storage/queue/QueueSasClientTests.java +++ b/sdk/storage/azure-storage-queue/src/test/java/com/azure/storage/queue/QueueSasClientTests.java @@ -34,8 +34,8 @@ import java.util.Iterator; import static com.azure.storage.common.test.shared.StorageCommonTestUtils.getOidFromToken; +import static com.azure.storage.common.test.shared.StorageCommonTestUtils.verifySasAndTokenInRequest; import static com.azure.storage.queue.QueueTestHelper.assertExceptionStatusCodeAndMessage; -import static com.azure.storage.queue.QueueTestHelper.assertResponseStatusCode; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -211,7 +211,7 @@ public void queueSasUserDelegationDelegatedObjectId() { .buildClient(); Response response = client.getPropertiesWithResponse(null, Context.NONE); - assertResponseStatusCode(response, 200); + verifySasAndTokenInRequest(response); }); }