From fccb643d2224f867ecfa2541bd837b35ba4e8aa5 Mon Sep 17 00:00:00 2001 From: Sourav Gupta Date: Fri, 16 Feb 2024 13:41:44 +0530 Subject: [PATCH 1/2] re-enable http endpoints for shared key auth --- sdk/storage/azblob/CHANGELOG.md | 2 ++ sdk/storage/azblob/assets.json | 2 +- .../internal/exported/shared_key_credential.go | 13 ------------- sdk/storage/azblob/service/client_test.go | 3 ++- sdk/storage/azdatalake/CHANGELOG.md | 2 ++ sdk/storage/azdatalake/assets.json | 2 +- .../internal/exported/shared_key_credential.go | 13 ------------- sdk/storage/azdatalake/service/client_test.go | 3 ++- sdk/storage/azfile/CHANGELOG.md | 2 ++ sdk/storage/azfile/assets.json | 2 +- .../internal/exported/shared_key_credential.go | 13 ------------- sdk/storage/azfile/service/client_test.go | 3 ++- 12 files changed, 15 insertions(+), 45 deletions(-) diff --git a/sdk/storage/azblob/CHANGELOG.md b/sdk/storage/azblob/CHANGELOG.md index 3df8b5c52079..8d654251ebe6 100644 --- a/sdk/storage/azblob/CHANGELOG.md +++ b/sdk/storage/azblob/CHANGELOG.md @@ -8,6 +8,8 @@ ### Bugs Fixed +* Re-enabled `SharedKeyCredential` authentication mode for non TLS protected endpoints. + ### Other Changes ## 1.3.0 (2024-02-12) diff --git a/sdk/storage/azblob/assets.json b/sdk/storage/azblob/assets.json index df7d66f02108..d971ff1ec844 100644 --- a/sdk/storage/azblob/assets.json +++ b/sdk/storage/azblob/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "go", "TagPrefix": "go/storage/azblob", - "Tag": "go/storage/azblob_9f40a5a13d" + "Tag": "go/storage/azblob_71b0a04c12" } diff --git a/sdk/storage/azblob/internal/exported/shared_key_credential.go b/sdk/storage/azblob/internal/exported/shared_key_credential.go index e4b076601f4e..adf46b06816e 100644 --- a/sdk/storage/azblob/internal/exported/shared_key_credential.go +++ b/sdk/storage/azblob/internal/exported/shared_key_credential.go @@ -11,9 +11,7 @@ import ( "crypto/hmac" "crypto/sha256" "encoding/base64" - "errors" "fmt" - "github.com/Azure/azure-sdk-for-go/sdk/internal/errorinfo" "net/http" "net/url" "sort" @@ -204,10 +202,6 @@ func (s *SharedKeyCredPolicy) Do(req *policy.Request) (*http.Response, error) { return req.Next() } - if err := checkHTTPSForAuth(req); err != nil { - return nil, err - } - if d := getHeader(shared.HeaderXmsDate, req.Raw().Header); d == "" { req.Raw().Header.Set(shared.HeaderXmsDate, time.Now().UTC().Format(http.TimeFormat)) } @@ -229,10 +223,3 @@ func (s *SharedKeyCredPolicy) Do(req *policy.Request) (*http.Response, error) { } return response, err } - -func checkHTTPSForAuth(req *policy.Request) error { - if strings.ToLower(req.Raw().URL.Scheme) != "https" { - return errorinfo.NonRetriableError(errors.New("authenticated requests are not permitted for non TLS protected (https) endpoints")) - } - return nil -} diff --git a/sdk/storage/azblob/service/client_test.go b/sdk/storage/azblob/service/client_test.go index f99716b4b30f..7d656aa6a70b 100644 --- a/sdk/storage/azblob/service/client_test.go +++ b/sdk/storage/azblob/service/client_test.go @@ -1847,7 +1847,7 @@ func (s *ServiceUnrecordedTestsSuite) TestServiceBlobBatchErrors() { _require.Error(err) } -func (s *ServiceRecordedTestsSuite) TestServiceClientRejectHTTP() { +func (s *ServiceRecordedTestsSuite) TestServiceClientWithHTTP() { _require := require.New(s.T()) cred, err := testcommon.GetGenericSharedKeyCredential(testcommon.TestAccountDefault) @@ -1858,6 +1858,7 @@ func (s *ServiceRecordedTestsSuite) TestServiceClientRejectHTTP() { _, err = svcClient.GetProperties(context.Background(), nil) _require.Error(err) + testcommon.ValidateBlobErrorCode(_require, err, "AccountRequiresHttps") } func (s *ServiceRecordedTestsSuite) TestServiceClientWithNilSharedKey() { diff --git a/sdk/storage/azdatalake/CHANGELOG.md b/sdk/storage/azdatalake/CHANGELOG.md index 8f9a94857021..c3b5bd78d8df 100644 --- a/sdk/storage/azdatalake/CHANGELOG.md +++ b/sdk/storage/azdatalake/CHANGELOG.md @@ -8,6 +8,8 @@ ### Bugs Fixed +* Re-enabled `SharedKeyCredential` authentication mode for non TLS protected endpoints. + ### Other Changes ## 1.1.0 (2024-02-14) diff --git a/sdk/storage/azdatalake/assets.json b/sdk/storage/azdatalake/assets.json index a700b917992b..da2dfd857383 100644 --- a/sdk/storage/azdatalake/assets.json +++ b/sdk/storage/azdatalake/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "go", "TagPrefix": "go/storage/azdatalake", - "Tag": "go/storage/azdatalake_82ea48120e" + "Tag": "go/storage/azdatalake_6e4e5b7c87" } diff --git a/sdk/storage/azdatalake/internal/exported/shared_key_credential.go b/sdk/storage/azdatalake/internal/exported/shared_key_credential.go index bf0c728faf53..bf3f6c1879cc 100644 --- a/sdk/storage/azdatalake/internal/exported/shared_key_credential.go +++ b/sdk/storage/azdatalake/internal/exported/shared_key_credential.go @@ -13,7 +13,6 @@ import ( "encoding/base64" "errors" "fmt" - "github.com/Azure/azure-sdk-for-go/sdk/internal/errorinfo" "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob" "net/http" "net/url" @@ -217,10 +216,6 @@ func (s *SharedKeyCredPolicy) Do(req *policy.Request) (*http.Response, error) { return req.Next() } - if err := checkHTTPSForAuth(req); err != nil { - return nil, err - } - if d := getHeader(shared.HeaderXmsDate, req.Raw().Header); d == "" { req.Raw().Header.Set(shared.HeaderXmsDate, time.Now().UTC().Format(http.TimeFormat)) } @@ -242,11 +237,3 @@ func (s *SharedKeyCredPolicy) Do(req *policy.Request) (*http.Response, error) { } return response, err } - -// TODO: update the azblob dependency having checks for rejecting URLs that are not HTTPS -func checkHTTPSForAuth(req *policy.Request) error { - if strings.ToLower(req.Raw().URL.Scheme) != "https" { - return errorinfo.NonRetriableError(errors.New("authenticated requests are not permitted for non TLS protected (https) endpoints")) - } - return nil -} diff --git a/sdk/storage/azdatalake/service/client_test.go b/sdk/storage/azdatalake/service/client_test.go index 8720bba2980f..889ecd4bee15 100644 --- a/sdk/storage/azdatalake/service/client_test.go +++ b/sdk/storage/azdatalake/service/client_test.go @@ -801,7 +801,7 @@ func (s *ServiceRecordedTestsSuite) TestAccountListFilesystemsEmptyPrefix() { _require.GreaterOrEqual(count, 2) } -func (s *ServiceRecordedTestsSuite) TestServiceClientRejectHTTP() { +func (s *ServiceRecordedTestsSuite) TestServiceClientWithHTTP() { _require := require.New(s.T()) testName := s.T().Name() @@ -818,6 +818,7 @@ func (s *ServiceRecordedTestsSuite) TestServiceClientRejectHTTP() { _, err = fileClient.Create(context.Background(), nil) _require.Error(err) + testcommon.ValidateErrorCode(_require, err, "AccountRequiresHttps") } func (s *ServiceRecordedTestsSuite) TestServiceClientWithNilSharedKey() { diff --git a/sdk/storage/azfile/CHANGELOG.md b/sdk/storage/azfile/CHANGELOG.md index 4076a551fab6..08b7827e4db1 100644 --- a/sdk/storage/azfile/CHANGELOG.md +++ b/sdk/storage/azfile/CHANGELOG.md @@ -8,6 +8,8 @@ ### Bugs Fixed +* Re-enabled `SharedKeyCredential` authentication mode for non TLS protected endpoints. + ### Other Changes ## 1.2.0 (2024-02-12) diff --git a/sdk/storage/azfile/assets.json b/sdk/storage/azfile/assets.json index 0c84c78afe62..5ace4eab2c2b 100644 --- a/sdk/storage/azfile/assets.json +++ b/sdk/storage/azfile/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "go", "TagPrefix": "go/storage/azfile", - "Tag": "go/storage/azfile_8f8ed3dd66" + "Tag": "go/storage/azfile_e46f674336" } diff --git a/sdk/storage/azfile/internal/exported/shared_key_credential.go b/sdk/storage/azfile/internal/exported/shared_key_credential.go index b2545b2bbb72..fa74ec015f8e 100644 --- a/sdk/storage/azfile/internal/exported/shared_key_credential.go +++ b/sdk/storage/azfile/internal/exported/shared_key_credential.go @@ -11,9 +11,7 @@ import ( "crypto/hmac" "crypto/sha256" "encoding/base64" - "errors" "fmt" - "github.com/Azure/azure-sdk-for-go/sdk/internal/errorinfo" "net/http" "net/url" "sort" @@ -204,10 +202,6 @@ func (s *SharedKeyCredPolicy) Do(req *policy.Request) (*http.Response, error) { return req.Next() } - if err := checkHTTPSForAuth(req); err != nil { - return nil, err - } - if d := getHeader(shared.HeaderXmsDate, req.Raw().Header); d == "" { req.Raw().Header.Set(shared.HeaderXmsDate, time.Now().UTC().Format(http.TimeFormat)) } @@ -229,10 +223,3 @@ func (s *SharedKeyCredPolicy) Do(req *policy.Request) (*http.Response, error) { } return response, err } - -func checkHTTPSForAuth(req *policy.Request) error { - if strings.ToLower(req.Raw().URL.Scheme) != "https" { - return errorinfo.NonRetriableError(errors.New("authenticated requests are not permitted for non TLS protected (https) endpoints")) - } - return nil -} diff --git a/sdk/storage/azfile/service/client_test.go b/sdk/storage/azfile/service/client_test.go index da7b4ae7c334..216397a887dc 100644 --- a/sdk/storage/azfile/service/client_test.go +++ b/sdk/storage/azfile/service/client_test.go @@ -670,7 +670,7 @@ func (s *ServiceRecordedTestsSuite) TestPremiumAccountListShares() { } } -func (s *ServiceRecordedTestsSuite) TestServiceClientRejectHTTP() { +func (s *ServiceRecordedTestsSuite) TestServiceClientWithHTTP() { _require := require.New(s.T()) cred, err := testcommon.GetGenericSharedKeyCredential(testcommon.TestAccountDefault) @@ -681,6 +681,7 @@ func (s *ServiceRecordedTestsSuite) TestServiceClientRejectHTTP() { _, err = svcClient.GetProperties(context.Background(), nil) _require.Error(err) + testcommon.ValidateFileErrorCode(_require, err, "AccountRequiresHttps") } func (s *ServiceRecordedTestsSuite) TestServiceClientWithNilSharedKey() { From b49e0f320d5c0bdf6516258f2324510b27d4299b Mon Sep 17 00:00:00 2001 From: Sourav Gupta Date: Fri, 16 Feb 2024 15:27:13 +0530 Subject: [PATCH 2/2] test fix --- sdk/storage/azblob/service/client_test.go | 2 +- sdk/storage/azdatalake/service/client_test.go | 2 +- sdk/storage/azfile/service/client_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/storage/azblob/service/client_test.go b/sdk/storage/azblob/service/client_test.go index 7d656aa6a70b..3327e7a9565d 100644 --- a/sdk/storage/azblob/service/client_test.go +++ b/sdk/storage/azblob/service/client_test.go @@ -1847,7 +1847,7 @@ func (s *ServiceUnrecordedTestsSuite) TestServiceBlobBatchErrors() { _require.Error(err) } -func (s *ServiceRecordedTestsSuite) TestServiceClientWithHTTP() { +func (s *ServiceUnrecordedTestsSuite) TestServiceClientWithHTTP() { _require := require.New(s.T()) cred, err := testcommon.GetGenericSharedKeyCredential(testcommon.TestAccountDefault) diff --git a/sdk/storage/azdatalake/service/client_test.go b/sdk/storage/azdatalake/service/client_test.go index 889ecd4bee15..7ac500c0fc9c 100644 --- a/sdk/storage/azdatalake/service/client_test.go +++ b/sdk/storage/azdatalake/service/client_test.go @@ -801,7 +801,7 @@ func (s *ServiceRecordedTestsSuite) TestAccountListFilesystemsEmptyPrefix() { _require.GreaterOrEqual(count, 2) } -func (s *ServiceRecordedTestsSuite) TestServiceClientWithHTTP() { +func (s *ServiceUnrecordedTestsSuite) TestServiceClientWithHTTP() { _require := require.New(s.T()) testName := s.T().Name() diff --git a/sdk/storage/azfile/service/client_test.go b/sdk/storage/azfile/service/client_test.go index 216397a887dc..34fa5b9e3628 100644 --- a/sdk/storage/azfile/service/client_test.go +++ b/sdk/storage/azfile/service/client_test.go @@ -670,7 +670,7 @@ func (s *ServiceRecordedTestsSuite) TestPremiumAccountListShares() { } } -func (s *ServiceRecordedTestsSuite) TestServiceClientWithHTTP() { +func (s *ServiceUnrecordedTestsSuite) TestServiceClientWithHTTP() { _require := require.New(s.T()) cred, err := testcommon.GetGenericSharedKeyCredential(testcommon.TestAccountDefault)