Skip to content

Commit

Permalink
Storage: Re-enable HTTP endpoints for shared key credential (#22401)
Browse files Browse the repository at this point in the history
  • Loading branch information
souravgupta-msft authored Feb 21, 2024
1 parent d607da6 commit 775ba01
Show file tree
Hide file tree
Showing 12 changed files with 15 additions and 45 deletions.
2 changes: 2 additions & 0 deletions sdk/storage/azblob/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Bugs Fixed

* Re-enabled `SharedKeyCredential` authentication mode for non TLS protected endpoints.

### Other Changes

## 1.3.0 (2024-02-12)
Expand Down
2 changes: 1 addition & 1 deletion sdk/storage/azblob/assets.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
13 changes: 0 additions & 13 deletions sdk/storage/azblob/internal/exported/shared_key_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}
Expand All @@ -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
}
3 changes: 2 additions & 1 deletion sdk/storage/azblob/service/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1847,7 +1847,7 @@ func (s *ServiceUnrecordedTestsSuite) TestServiceBlobBatchErrors() {
_require.Error(err)
}

func (s *ServiceRecordedTestsSuite) TestServiceClientRejectHTTP() {
func (s *ServiceUnrecordedTestsSuite) TestServiceClientWithHTTP() {
_require := require.New(s.T())

cred, err := testcommon.GetGenericSharedKeyCredential(testcommon.TestAccountDefault)
Expand All @@ -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() {
Expand Down
2 changes: 2 additions & 0 deletions sdk/storage/azdatalake/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Bugs Fixed

* Re-enabled `SharedKeyCredential` authentication mode for non TLS protected endpoints.

### Other Changes

## 1.1.0 (2024-02-14)
Expand Down
2 changes: 1 addition & 1 deletion sdk/storage/azdatalake/assets.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
13 changes: 0 additions & 13 deletions sdk/storage/azdatalake/internal/exported/shared_key_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}
Expand All @@ -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
}
3 changes: 2 additions & 1 deletion sdk/storage/azdatalake/service/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ func (s *ServiceRecordedTestsSuite) TestAccountListFilesystemsEmptyPrefix() {
_require.GreaterOrEqual(count, 2)
}

func (s *ServiceRecordedTestsSuite) TestServiceClientRejectHTTP() {
func (s *ServiceUnrecordedTestsSuite) TestServiceClientWithHTTP() {
_require := require.New(s.T())
testName := s.T().Name()

Expand All @@ -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() {
Expand Down
2 changes: 2 additions & 0 deletions sdk/storage/azfile/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Bugs Fixed

* Re-enabled `SharedKeyCredential` authentication mode for non TLS protected endpoints.

### Other Changes

## 1.2.0 (2024-02-12)
Expand Down
2 changes: 1 addition & 1 deletion sdk/storage/azfile/assets.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
13 changes: 0 additions & 13 deletions sdk/storage/azfile/internal/exported/shared_key_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}
Expand All @@ -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
}
3 changes: 2 additions & 1 deletion sdk/storage/azfile/service/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ func (s *ServiceRecordedTestsSuite) TestPremiumAccountListShares() {
}
}

func (s *ServiceRecordedTestsSuite) TestServiceClientRejectHTTP() {
func (s *ServiceUnrecordedTestsSuite) TestServiceClientWithHTTP() {
_require := require.New(s.T())

cred, err := testcommon.GetGenericSharedKeyCredential(testcommon.TestAccountDefault)
Expand All @@ -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() {
Expand Down

0 comments on commit 775ba01

Please sign in to comment.