Skip to content

Commit

Permalink
fix(azureblob): Return error if Azure returns no service principal to…
Browse files Browse the repository at this point in the history
…ken (#13195)

This adds an extra nil check to verify the Azure blob storage client has received a service principal token before trying to do anything with said token. This has led to crashes if `use_federated_token is enabled` and an error message is returned instead of a token.
  • Loading branch information
rgroothuijsen authored Dec 13, 2024
1 parent 3badbb3 commit e98a86b
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 0 deletions.
4 changes: 4 additions & 0 deletions pkg/storage/chunk/client/azure/blob_storage_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,10 @@ func (b *BlobStorage) getServicePrincipalToken(authFunctions authFunctions) (*ad

if b.cfg.UseFederatedToken {
token, err := b.servicePrincipalTokenFromFederatedToken(resource, authFunctions.NewOAuthConfigFunc, authFunctions.NewServicePrincipalTokenFromFederatedTokenFunc)
if err != nil {
return nil, err
}

var customRefreshFunc adal.TokenRefresh = func(_ context.Context, resource string) (*adal.Token, error) {
newToken, err := b.servicePrincipalTokenFromFederatedToken(resource, authFunctions.NewOAuthConfigFunc, authFunctions.NewServicePrincipalTokenFromFederatedTokenFunc)
if err != nil {
Expand Down
24 changes: 24 additions & 0 deletions pkg/storage/chunk/client/azure/blob_storage_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"testing"
"time"

"github.com/pkg/errors"

"github.com/Azure/go-autorest/autorest/adal"
"github.com/Azure/go-autorest/autorest/azure"
"github.com/grafana/dskit/flagext"
Expand Down Expand Up @@ -80,6 +82,28 @@ func (suite *FederatedTokenTestSuite) TestGetServicePrincipalToken() {
require.True(suite.T(), suite.mockedServicePrincipalToken == token, "should return the mocked object")
}

func (suite *FederatedTokenTestSuite) Test_HandleNoServicePrincipalToken() {
newOAuthConfigFunc := func(activeDirectoryEndpoint, tenantID string) (*adal.OAuthConfig, error) {
require.Equal(suite.T(), azure.PublicCloud.ActiveDirectoryEndpoint, activeDirectoryEndpoint)
require.Equal(suite.T(), "myTenantId", tenantID)

_, err := adal.NewOAuthConfig(activeDirectoryEndpoint, tenantID)
require.NoError(suite.T(), err)

return suite.mockOAuthConfig, nil
}

servicePrincipalTokenFromFederatedTokenFunc := func(_ adal.OAuthConfig, _ string, _ string, _ string, _ ...adal.TokenRefreshCallback) (*adal.ServicePrincipalToken, error) {
return nil, errors.New("No token")
}

token, err := suite.config.getServicePrincipalToken(authFunctions{newOAuthConfigFunc, servicePrincipalTokenFromFederatedTokenFunc})

require.Error(suite.T(), err)
require.EqualError(suite.T(), err, "No token")
require.True(suite.T(), token == nil, "should return error if no token was retrieved")
}

func Test_Hedging(t *testing.T) {
for _, tc := range []struct {
name string
Expand Down

0 comments on commit e98a86b

Please sign in to comment.