[azblob][sas] Fix SignWithSharedKey if stored access policy is used @…

…stotz89 (#21230)

* [azblob][sas] Fix SignWithSharedKey if stored access policy is used

* Adding test and changelog

---------

Co-authored-by: Sourav Gupta <[email protected]>

sdk/storage/azblob/CHANGELOG.md

@@ -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://github.com/Azure/azure-sdk-for-go/issues/21229).
+
 ### Other Changes
 
 ## 1.1.0 (2023-07-13)
@@ -25,7 +27,6 @@
 
 * Fixed time formatting for the conditional request headers. Fixes [#20475](https://github.com/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://github.com/Azure/azure-sdk-for-go/issues/20735).
 
 ### Other Changes

sdk/storage/azblob/container/client_test.go

@@ -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())
@@ -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)
+}

sdk/storage/azblob/sas/service.go

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

sdk/storage/azblob/sas/service_test.go

@@ -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) {
@@ -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)
+	}
+}