Skip to content

Commit e98a86b

Browse files
fix(azureblob): Return error if Azure returns no service principal token (#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.
1 parent 3badbb3 commit e98a86b

File tree

2 files changed

+28
-0
lines changed

2 files changed

+28
-0
lines changed

‎pkg/storage/chunk/client/azure/blob_storage_client.go

+4
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,10 @@ func (b *BlobStorage) getServicePrincipalToken(authFunctions authFunctions) (*ad
474474

475475
if b.cfg.UseFederatedToken {
476476
token, err := b.servicePrincipalTokenFromFederatedToken(resource, authFunctions.NewOAuthConfigFunc, authFunctions.NewServicePrincipalTokenFromFederatedTokenFunc)
477+
if err != nil {
478+
return nil, err
479+
}
480+
477481
var customRefreshFunc adal.TokenRefresh = func(_ context.Context, resource string) (*adal.Token, error) {
478482
newToken, err := b.servicePrincipalTokenFromFederatedToken(resource, authFunctions.NewOAuthConfigFunc, authFunctions.NewServicePrincipalTokenFromFederatedTokenFunc)
479483
if err != nil {

‎pkg/storage/chunk/client/azure/blob_storage_client_test.go

+24
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"testing"
1111
"time"
1212

13+
"github.com/pkg/errors"
14+
1315
"github.com/Azure/go-autorest/autorest/adal"
1416
"github.com/Azure/go-autorest/autorest/azure"
1517
"github.com/grafana/dskit/flagext"
@@ -80,6 +82,28 @@ func (suite *FederatedTokenTestSuite) TestGetServicePrincipalToken() {
8082
require.True(suite.T(), suite.mockedServicePrincipalToken == token, "should return the mocked object")
8183
}
8284

85+
func (suite *FederatedTokenTestSuite) Test_HandleNoServicePrincipalToken() {
86+
newOAuthConfigFunc := func(activeDirectoryEndpoint, tenantID string) (*adal.OAuthConfig, error) {
87+
require.Equal(suite.T(), azure.PublicCloud.ActiveDirectoryEndpoint, activeDirectoryEndpoint)
88+
require.Equal(suite.T(), "myTenantId", tenantID)
89+
90+
_, err := adal.NewOAuthConfig(activeDirectoryEndpoint, tenantID)
91+
require.NoError(suite.T(), err)
92+
93+
return suite.mockOAuthConfig, nil
94+
}
95+
96+
servicePrincipalTokenFromFederatedTokenFunc := func(_ adal.OAuthConfig, _ string, _ string, _ string, _ ...adal.TokenRefreshCallback) (*adal.ServicePrincipalToken, error) {
97+
return nil, errors.New("No token")
98+
}
99+
100+
token, err := suite.config.getServicePrincipalToken(authFunctions{newOAuthConfigFunc, servicePrincipalTokenFromFederatedTokenFunc})
101+
102+
require.Error(suite.T(), err)
103+
require.EqualError(suite.T(), err, "No token")
104+
require.True(suite.T(), token == nil, "should return error if no token was retrieved")
105+
}
106+
83107
func Test_Hedging(t *testing.T) {
84108
for _, tc := range []struct {
85109
name string

0 commit comments

Comments
 (0)