Fix multi-tenant bearer authorization (Azure#446)

* Fix multi-tenant bearer authorization Type MultiTenantServicePrincipalToken didn't fully implement the RefresherWithContext interface, as a result the type assertion would always fail in WithAuthorization() and tokens were not retrieved. * update test to ensure token value matches hard-coded value
tombuildsstuff · Aug 9, 2019 · 880eb0e · 880eb0e
1 parent 84062b4
commit 880eb0e
Show file tree

Hide file tree

Showing 4 changed files with 181 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # CHANGELOG
 
+## v12.4.3
+
+### Bug Fixes
+
+- `autorest.MultiTenantServicePrincipalTokenAuthorizer` will now properly add its auxiliary bearer tokens.
+
 ## v12.4.2
 
 ### Bug Fixes

diff --git a/autorest/adal/token.go b/autorest/adal/token.go
@@ -1024,6 +1024,32 @@ func (mt *MultiTenantServicePrincipalToken) EnsureFreshWithContext(ctx context.C
 	return nil
 }
 
+// RefreshWithContext obtains a fresh token for the Service Principal.
+func (mt *MultiTenantServicePrincipalToken) RefreshWithContext(ctx context.Context) error {
+	if err := mt.PrimaryToken.RefreshWithContext(ctx); err != nil {
+		return fmt.Errorf("failed to refresh primary token: %v", err)
+	}
+	for _, aux := range mt.AuxiliaryTokens {
+		if err := aux.RefreshWithContext(ctx); err != nil {
+			return fmt.Errorf("failed to refresh auxiliary token: %v", err)
+		}
+	}
+	return nil
+}
+
+// RefreshExchangeWithContext refreshes the token, but for a different resource.
+func (mt *MultiTenantServicePrincipalToken) RefreshExchangeWithContext(ctx context.Context, resource string) error {
+	if err := mt.PrimaryToken.RefreshExchangeWithContext(ctx, resource); err != nil {
+		return fmt.Errorf("failed to refresh primary token: %v", err)
+	}
+	for _, aux := range mt.AuxiliaryTokens {
+		if err := aux.RefreshExchangeWithContext(ctx, resource); err != nil {
+			return fmt.Errorf("failed to refresh auxiliary token: %v", err)
+		}
+	}
+	return nil
+}
+
 // NewMultiTenantServicePrincipalToken creates a new MultiTenantServicePrincipalToken with the specified credentials and resource.
 func NewMultiTenantServicePrincipalToken(multiTenantCfg MultiTenantOAuthConfig, clientID string, secret string, resource string) (*MultiTenantServicePrincipalToken, error) {
 	if err := validateStringParam(clientID, "clientID"); err != nil {

diff --git a/autorest/authorization_test.go b/autorest/authorization_test.go
@@ -18,6 +18,7 @@ import (
 	"fmt"
 	"net/http"
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/Azure/go-autorest/autorest/adal"
@@ -26,6 +27,9 @@ import (
 
 const (
 	TestTenantID                = "TestTenantID"
+	TestAuxTenent1              = "aux1"
+	TestAuxTenent2              = "aux2"
+	TestAuxTenent3              = "aux3"
 	TestActiveDirectoryEndpoint = "https://login/test.com/"
 )
 
@@ -267,7 +271,7 @@ func (m mockMTSPTProvider) AuxiliaryOAuthTokens() []string {
 func TestMultitenantAuthorizationOne(t *testing.T) {
 	mtSPTProvider := mockMTSPTProvider{
 		p: "primary",
-		a: []string{"aux1"},
+		a: []string{TestAuxTenent1},
 	}
 	mt := NewMultiTenantServicePrincipalTokenAuthorizer(mtSPTProvider)
 	req, err := Prepare(mocks.NewRequest(), mt.WithAuthorization())
@@ -285,7 +289,7 @@ func TestMultitenantAuthorizationOne(t *testing.T) {
 func TestMultitenantAuthorizationThree(t *testing.T) {
 	mtSPTProvider := mockMTSPTProvider{
 		p: "primary",
-		a: []string{"aux1", "aux2", "aux3"},
+		a: []string{TestAuxTenent1, TestAuxTenent2, TestAuxTenent3},
 	}
 	mt := NewMultiTenantServicePrincipalTokenAuthorizer(mtSPTProvider)
 	req, err := Prepare(mocks.NewRequest(), mt.WithAuthorization())
@@ -299,3 +303,145 @@ func TestMultitenantAuthorizationThree(t *testing.T) {
 		t.Fatalf("bad auxiliary authorization header %s", aux)
 	}
 }
+
+func TestMultiTenantServicePrincipalTokenWithAuthorizationRefresh(t *testing.T) {
+	multiTenantCfg, err := adal.NewMultiTenantOAuthConfig(TestActiveDirectoryEndpoint, TestTenantID, []string{TestAuxTenent1, TestAuxTenent2, TestAuxTenent3}, adal.OAuthOptions{})
+	if err != nil {
+		t.Fatalf("azure: adal#NewMultiTenantOAuthConfig returned an error (%v)", err)
+	}
+	mtSpt, err := adal.NewMultiTenantServicePrincipalToken(multiTenantCfg, "id", "secret", "resource")
+	if err != nil {
+		t.Fatalf("azure: adal#NewMultiTenantServicePrincipalToken returned an error (%v)", err)
+	}
+
+	primaryToken := `{
+		"access_token" : "primary token refreshed",
+		"expires_in"   : "3600",
+		"expires_on"   : "test",
+		"not_before"   : "test",
+		"resource"     : "test",
+		"token_type"   : "Bearer"
+	}`
+
+	auxToken1 := `{
+		"access_token" : "aux token 1 refreshed",
+		"expires_in"   : "3600",
+		"expires_on"   : "test",
+		"not_before"   : "test",
+		"resource"     : "test",
+		"token_type"   : "Bearer"
+	}`
+
+	auxToken2 := `{
+		"access_token" : "aux token 2 refreshed",
+		"expires_in"   : "3600",
+		"expires_on"   : "test",
+		"not_before"   : "test",
+		"resource"     : "test",
+		"token_type"   : "Bearer"
+	}`
+
+	auxToken3 := `{
+		"access_token" : "aux token 3 refreshed",
+		"expires_in"   : "3600",
+		"expires_on"   : "test",
+		"not_before"   : "test",
+		"resource"     : "test",
+		"token_type"   : "Bearer"
+	}`
+
+	s := mocks.NewSender()
+	s.AppendResponse(mocks.NewResponseWithBodyAndStatus(mocks.NewBody(primaryToken), http.StatusOK, "OK"))
+	s.AppendResponse(mocks.NewResponseWithBodyAndStatus(mocks.NewBody(auxToken1), http.StatusOK, "OK"))
+	s.AppendResponse(mocks.NewResponseWithBodyAndStatus(mocks.NewBody(auxToken2), http.StatusOK, "OK"))
+	s.AppendResponse(mocks.NewResponseWithBodyAndStatus(mocks.NewBody(auxToken3), http.StatusOK, "OK"))
+
+	mtSpt.PrimaryToken.SetSender(s)
+	for _, aux := range mtSpt.AuxiliaryTokens {
+		aux.SetSender(s)
+	}
+
+	mta := NewMultiTenantServicePrincipalTokenAuthorizer(mtSpt)
+	req, err := Prepare(mocks.NewRequest(), mta.WithAuthorization())
+	if err != nil {
+		t.Fatalf("azure: multiTenantSPTAuthorizer#WithAuthorization returned an error (%v)", err)
+	}
+	if ah := req.Header.Get(http.CanonicalHeaderKey("Authorization")); ah != fmt.Sprintf("Bearer %s", mtSpt.PrimaryOAuthToken()) {
+		t.Fatal("azure: multiTenantSPTAuthorizer#WithAuthorization failed to set Authorization header for primary token")
+	} else if ah != "Bearer primary token refreshed" {
+		t.Fatal("azure: multiTenantSPTAuthorizer#WithAuthorization primary token value doesn't match")
+	}
+	auxTokens := mtSpt.AuxiliaryOAuthTokens()
+	for i := range auxTokens {
+		auxTokens[i] = fmt.Sprintf("Bearer %s", auxTokens[i])
+	}
+	auxHeader := req.Header.Get(http.CanonicalHeaderKey(headerAuxAuthorization))
+	if auxHeader != strings.Join(auxTokens, "; ") {
+		t.Fatal("azure: multiTenantSPTAuthorizer#WithAuthorization failed to set Authorization header for auxiliary tokens")
+	}
+	for i := range auxTokens {
+		if auxTokens[i] != fmt.Sprintf("Bearer aux token %d refreshed", i+1) {
+			t.Fatal("azure: multiTenantSPTAuthorizer#WithAuthorization auxiliary token value doesn't match")
+		}
+	}
+}
+
+func TestMultiTenantServicePrincipalTokenWithAuthorizationRefreshFail1(t *testing.T) {
+	multiTenantCfg, err := adal.NewMultiTenantOAuthConfig(TestActiveDirectoryEndpoint, TestTenantID, []string{TestAuxTenent1, TestAuxTenent2, TestAuxTenent3}, adal.OAuthOptions{})
+	if err != nil {
+		t.Fatalf("azure: adal#NewMultiTenantOAuthConfig returned an error (%v)", err)
+	}
+	mtSpt, err := adal.NewMultiTenantServicePrincipalToken(multiTenantCfg, "id", "secret", "resource")
+	if err != nil {
+		t.Fatalf("azure: adal#NewMultiTenantServicePrincipalToken returned an error (%v)", err)
+	}
+
+	s := mocks.NewSender()
+	s.AppendResponse(mocks.NewResponseWithStatus("access denied", http.StatusForbidden))
+
+	mtSpt.PrimaryToken.SetSender(s)
+	for _, aux := range mtSpt.AuxiliaryTokens {
+		aux.SetSender(s)
+	}
+
+	mta := NewMultiTenantServicePrincipalTokenAuthorizer(mtSpt)
+	_, err = Prepare(mocks.NewRequest(), mta.WithAuthorization())
+	if err == nil {
+		t.Fatalf("azure: multiTenantSPTAuthorizer#WithAuthorization unexpected nil error")
+	}
+}
+
+func TestMultiTenantServicePrincipalTokenWithAuthorizationRefreshFail2(t *testing.T) {
+	multiTenantCfg, err := adal.NewMultiTenantOAuthConfig(TestActiveDirectoryEndpoint, TestTenantID, []string{TestAuxTenent1, TestAuxTenent2, TestAuxTenent3}, adal.OAuthOptions{})
+	if err != nil {
+		t.Fatalf("azure: adal#NewMultiTenantOAuthConfig returned an error (%v)", err)
+	}
+	mtSpt, err := adal.NewMultiTenantServicePrincipalToken(multiTenantCfg, "id", "secret", "resource")
+	if err != nil {
+		t.Fatalf("azure: adal#NewMultiTenantServicePrincipalToken returned an error (%v)", err)
+	}
+
+	primaryToken := `{
+		"access_token" : "primary token refreshed",
+		"expires_in"   : "3600",
+		"expires_on"   : "test",
+		"not_before"   : "test",
+		"resource"     : "test",
+		"token_type"   : "Bearer"
+	}`
+
+	s := mocks.NewSender()
+	s.AppendResponse(mocks.NewResponseWithBodyAndStatus(mocks.NewBody(primaryToken), http.StatusOK, "OK"))
+	s.AppendResponse(mocks.NewResponseWithStatus("access denied", http.StatusForbidden))
+
+	mtSpt.PrimaryToken.SetSender(s)
+	for _, aux := range mtSpt.AuxiliaryTokens {
+		aux.SetSender(s)
+	}
+
+	mta := NewMultiTenantServicePrincipalTokenAuthorizer(mtSpt)
+	_, err = Prepare(mocks.NewRequest(), mta.WithAuthorization())
+	if err == nil {
+		t.Fatalf("azure: multiTenantSPTAuthorizer#WithAuthorization unexpected nil error")
+	}
+}
diff --git a/autorest/version.go b/autorest/version.go
@@ -19,7 +19,7 @@ import (
 	"runtime"
 )
 
-const number = "v12.4.2"
+const number = "v12.4.3"
 
 var (
 	userAgent = fmt.Sprintf("Go/%s (%s-%s) go-autorest/%s",