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

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"
 }

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

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

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

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<HttpPipelinePolicy> 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);
+    }
 }

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

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")

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<BlobItem> response = getUserDelegationInfo().flatMapMany(key -> {
+            Mono<Response<BlobProperties>> 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