Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[azblob][sas] Fix SignWithSharedKey if stored access policy is used @stotz89 #21230

Merged
merged 5 commits into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion sdk/storage/azblob/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Bugs Fixed

* Fixed service SAS creation where expiry time or permissions can be omitted when stored access policy is used. Fixes [#21229](https:/Azure/azure-sdk-for-go/issues/21229).

### Other Changes

## 1.1.0 (2023-07-13)
Expand All @@ -25,7 +27,6 @@

* Fixed time formatting for the conditional request headers. Fixes [#20475](https:/Azure/azure-sdk-for-go/issues/20475).
* Fixed an issue where passing a blob tags map of length 0 would result in the x-ms-tags header to be sent to the service with an empty string as value.

* Fixed block size and number of blocks calculation in `UploadBuffer` and `UploadFile`. Fixes [#20735](https:/Azure/azure-sdk-for-go/issues/20735).

### Other Changes
Expand Down
59 changes: 58 additions & 1 deletion sdk/storage/azblob/container/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1548,7 +1548,7 @@ func (s *ContainerRecordedTestsSuite) TestContainerGetPermissionsPublicAccessNot
_require.Equal(*resp.BlobPublicAccess, container.PublicAccessTypeBlob)
}

func (s *ContainerRecordedTestsSuite) TestContainerSetPermissionsPublicAccessNone() {
func (s *ContainerUnrecordedTestsSuite) TestContainerSetPermissionsPublicAccessNone() {
// Test the basic one by making an anonymous request to ensure it's actually doing it and also with GetPermissions
// For all the others, can just use GetPermissions since we've validated that it at least registers on the server correctly
_require := require.New(s.T())
Expand Down Expand Up @@ -3027,3 +3027,60 @@ func (s *ContainerUnrecordedTestsSuite) TestContainerBlobBatchErrors() {
_, err = cntClient.SubmitBatch(context.Background(), nil, nil)
_require.Error(err)
}

func (s *ContainerUnrecordedTestsSuite) TestContainerSASUsingAccessPolicy() {
_require := require.New(s.T())
testName := s.T().Name()

cred, err := testcommon.GetGenericSharedKeyCredential(testcommon.TestAccountDefault)
_require.NoError(err)

svcClient, err := testcommon.GetServiceClient(s.T(), testcommon.TestAccountDefault, nil)
_require.NoError(err)

// Creating container client
containerName := testcommon.GenerateContainerName(testName)
cntClient := testcommon.CreateNewContainer(context.Background(), _require, containerName, svcClient)
defer testcommon.DeleteContainer(context.Background(), _require, cntClient)

id := "testAccessPolicy"
signedIdentifiers := make([]*container.SignedIdentifier, 0)
signedIdentifiers = append(signedIdentifiers, &container.SignedIdentifier{
AccessPolicy: &container.AccessPolicy{
Expiry: to.Ptr(time.Now().Add(1 * time.Hour)),
Start: to.Ptr(time.Now()),
Permission: to.Ptr("racwd"),
},
ID: &id,
})

_, err = cntClient.SetAccessPolicy(context.Background(), &container.SetAccessPolicyOptions{
ContainerACL: signedIdentifiers,
})
_require.NoError(err)

gResp, err := cntClient.GetAccessPolicy(context.Background(), nil)
_require.NoError(err)
_require.Len(gResp.SignedIdentifiers, 1)

time.Sleep(30 * time.Second)

sasQueryParams, err := sas.BlobSignatureValues{
Protocol: sas.ProtocolHTTPS,
Identifier: id,
ContainerName: containerName,
}.SignWithSharedKey(cred)
_require.NoError(err)

cntSAS := cntClient.URL() + "?" + sasQueryParams.Encode()
cntClientSAS, err := container.NewClientWithNoCredential(cntSAS, nil)
_require.NoError(err)

bbClient := testcommon.CreateNewBlockBlob(context.Background(), _require, testcommon.GenerateBlobName(testName), cntClientSAS)

_, err = bbClient.GetProperties(context.Background(), nil)
_require.NoError(err)

_, err = bbClient.Delete(context.Background(), nil)
_require.NoError(err)
}
2 changes: 1 addition & 1 deletion sdk/storage/azblob/sas/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func getDirectoryDepth(path string) string {

// SignWithSharedKey uses an account's SharedKeyCredential to sign this signature values to produce the proper SAS query parameters.
func (v BlobSignatureValues) SignWithSharedKey(sharedKeyCredential *SharedKeyCredential) (QueryParameters, error) {
if v.ExpiryTime.IsZero() || v.Permissions == "" {
if v.Identifier == "" && (v.ExpiryTime.IsZero() || v.Permissions == "") {
return QueryParameters{}, errors.New("service SAS is missing at least one of these: ExpiryTime or Permissions")
}

Expand Down
45 changes: 45 additions & 0 deletions sdk/storage/azblob/sas/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
package sas

import (
"errors"
"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/internal/exported"
"github.com/stretchr/testify/require"
"testing"
"time"
)

func TestContainerPermissions_String(t *testing.T) {
Expand Down Expand Up @@ -257,3 +260,45 @@ func TestGetDirectoryDepth(t *testing.T) {
require.Equal(t, c.expected, getDirectoryDepth(c.input))
}
}

func TestBlobSignatureValues_SignWithSharedKey(t *testing.T) {
cred, err := exported.NewSharedKeyCredential("fakeaccountname", "AKIAIOSFODNN7EXAMPLE")
require.Nil(t, err, "error creating valid shared key credentials.")

expiryDate, err := time.Parse("2006-01-02", "2023-07-20")
require.Nil(t, err, "error creating valid expiry date.")

testdata := []struct {
object BlobSignatureValues
expected QueryParameters
expectedError error
}{
{
object: BlobSignatureValues{ContainerName: "fakestoragecontainer", Permissions: "a", ExpiryTime: expiryDate},
expected: QueryParameters{version: Version, permissions: "a", expiryTime: expiryDate, resource: "c"},
expectedError: nil,
},
{
object: BlobSignatureValues{ContainerName: "fakestoragecontainer", Permissions: "", ExpiryTime: expiryDate},
expected: QueryParameters{},
expectedError: errors.New("service SAS is missing at least one of these: ExpiryTime or Permissions"),
},
{
object: BlobSignatureValues{ContainerName: "fakestoragecontainer", Permissions: "a", ExpiryTime: *new(time.Time)},
expected: QueryParameters{},
expectedError: errors.New("service SAS is missing at least one of these: ExpiryTime or Permissions"),
},
{
object: BlobSignatureValues{ContainerName: "fakestoragecontainer", Permissions: "", ExpiryTime: *new(time.Time), Identifier: "fakepolicyname"},
expected: QueryParameters{version: Version, resource: "c", identifier: "fakepolicyname"},
expectedError: nil,
},
}
for _, c := range testdata {
act, err := c.object.SignWithSharedKey(cred)
// ignore signature value
act.signature = ""
require.Equal(t, c.expected, act)
require.Equal(t, c.expectedError, err)
}
}