Skip to content

Commit

Permalink
Fixed bug where BlobContainer.SetAccessPolicy() and DataLakeFileClien…
Browse files Browse the repository at this point in the history
…t.SetAccessPolicy() were not putting permissions in the correct order (Azure#15727)
  • Loading branch information
seanmcc-msft authored and annelo-msft committed Feb 17, 2021
1 parent c6e8a51 commit cd9c8f1
Show file tree
Hide file tree
Showing 11 changed files with 837 additions and 19 deletions.
2 changes: 1 addition & 1 deletion sdk/storage/Azure.Storage.Blobs/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Release History

## 12.7.0-preview.2 (Unreleased)

- Fixed bug where BobContainerClient.SetAccessPolicy() would throw an exception if signed identifier permissions were not in the correct order.

## 12.7.0-preview.1 (2020-09-30)
- Added support for service version 2020-02-10.
Expand Down
18 changes: 17 additions & 1 deletion sdk/storage/Azure.Storage.Blobs/src/BlobContainerClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Azure.Storage.Blobs.Models;
using Azure.Storage.Blobs.Specialized;
using Azure.Storage.Cryptography;
using Azure.Storage.Sas;
using Metadata = System.Collections.Generic.IDictionary<string, string>;

namespace Azure.Storage.Blobs
Expand Down Expand Up @@ -2139,12 +2140,27 @@ private async Task<Response<BlobContainerInfo>> SetAccessPolicyInternal(
throw BlobErrors.BlobConditionsMustBeDefault(nameof(RequestConditions.IfMatch), nameof(RequestConditions.IfNoneMatch));
}

List<BlobSignedIdentifier> sanitizedPermissions = null;
if (permissions != null)
{
sanitizedPermissions = new List<BlobSignedIdentifier>();

foreach (BlobSignedIdentifier signedIdentifier in permissions)
{
signedIdentifier.AccessPolicy.Permissions = SasExtensions.ValidateAndSanitizeRawPermissions(
signedIdentifier.AccessPolicy.Permissions,
Constants.Sas.ValidPermissionsInOrder);

sanitizedPermissions.Add(signedIdentifier);
}
}

return await BlobRestClient.Container.SetAccessPolicyAsync(
ClientDiagnostics,
Pipeline,
Uri,
version: Version.ToVersionString(),
permissions: permissions,
permissions: sanitizedPermissions,
leaseId: conditions?.LeaseId,
access: accessType,
ifModifiedSince: conditions?.IfModifiedSince,
Expand Down
17 changes: 1 addition & 16 deletions sdk/storage/Azure.Storage.Blobs/src/Sas/BlobSasBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ public void SetPermissions(
{
rawPermissions = SasExtensions.ValidateAndSanitizeRawPermissions(
permissions: rawPermissions,
validPermissionsInOrder: s_validPermissionsInOrder);
validPermissionsInOrder: Constants.Sas.ValidPermissionsInOrder);
}

SetPermissions(rawPermissions);
Expand All @@ -237,21 +237,6 @@ public void SetPermissions(string rawPermissions)
Permissions = rawPermissions;
}

private static readonly List<char> s_validPermissionsInOrder = new List<char>
{
Constants.Sas.Permissions.Read,
Constants.Sas.Permissions.Add,
Constants.Sas.Permissions.Create,
Constants.Sas.Permissions.Write,
Constants.Sas.Permissions.Delete,
Constants.Sas.Permissions.DeleteBlobVersion,
Constants.Sas.Permissions.List,
Constants.Sas.Permissions.Tag,
Constants.Sas.Permissions.Update,
Constants.Sas.Permissions.Process,
Constants.Sas.Permissions.FilterByTags,
};

/// <summary>
/// Use an account's <see cref="StorageSharedKeyCredential"/> to sign this
/// shared access signature values to produce the proper SAS query
Expand Down
41 changes: 41 additions & 0 deletions sdk/storage/Azure.Storage.Blobs/tests/ContainerClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1210,6 +1210,47 @@ await TestHelper.AssertExpectedExceptionAsync<RequestFailedException>(
}
}

[Test]
public async Task SetAccessPolicyAsync_InvalidPermissionOrder()
{
await using DisposingContainer test = await GetTestContainerAsync();

// Arrange
PublicAccessType publicAccessType = PublicAccessType.BlobContainer;
BlobSignedIdentifier[] signedIdentifiers = new[]
{
new BlobSignedIdentifier
{
Id = GetNewString(),
AccessPolicy = new BlobAccessPolicy()
{
PolicyStartsOn = Recording.UtcNow.AddHours(-1),
PolicyExpiresOn = Recording.UtcNow.AddHours(1),
Permissions = "wrld"
}
}
};

// Act
await test.Container.SetAccessPolicyAsync(
accessType: publicAccessType,
permissions: signedIdentifiers
);

// Assert
Response<BlobContainerProperties> propertiesResponse = await test.Container.GetPropertiesAsync();
Assert.AreEqual(publicAccessType, propertiesResponse.Value.PublicAccess);

Response<BlobContainerAccessPolicy> response = await test.Container.GetAccessPolicyAsync();
Assert.AreEqual(1, response.Value.SignedIdentifiers.Count());

BlobSignedIdentifier acl = response.Value.SignedIdentifiers.First();
Assert.AreEqual(signedIdentifiers[0].Id, acl.Id);
Assert.AreEqual(signedIdentifiers[0].AccessPolicy.PolicyStartsOn, acl.AccessPolicy.PolicyStartsOn);
Assert.AreEqual(signedIdentifiers[0].AccessPolicy.PolicyExpiresOn, acl.AccessPolicy.PolicyExpiresOn);
Assert.AreEqual(signedIdentifiers[0].AccessPolicy.Permissions, acl.AccessPolicy.Permissions);
}

[Test]
public async Task AcquireLeaseAsync()
{
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit cd9c8f1

Please sign in to comment.