Make sure authority hosts are https (#12728)

* Check authority hosts are https * update to have scheme check as a func * Update tests for interactive browser * Update tests to use variadic ctor * Updating test with new ServerOptions * Adding updated go mod and sum * Re-integrating scheme check into setDefaultValues * Update AuthorizationCodeCredential tests * Updating InteractiveBrowserCredential tests to solve race condition * go mod tidy * Remove server TLS config from transport in tests
Azure · Oct 13, 2020 · 69b131d · 69b131d
1 parent ef01731
commit 69b131d
Show file tree

Hide file tree

Showing 11 changed files with 63 additions and 31 deletions.
diff --git a/sdk/azidentity/authorization_code_credential_test.go b/sdk/azidentity/authorization_code_credential_test.go
@@ -62,7 +62,7 @@ func TestAuthorizationCodeCredential_CreateAuthRequestSuccess(t *testing.T) {
 }
 
 func TestAuthorizationCodeCredential_GetTokenSuccess(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))
 	options := DefaultAuthorizationCodeCredentialOptions()
@@ -80,7 +80,7 @@ func TestAuthorizationCodeCredential_GetTokenSuccess(t *testing.T) {
 }
 
 func TestAuthorizationCodeCredential_GetTokenInvalidCredentials(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.SetResponse(mock.WithBody([]byte(accessTokenRespError)), mock.WithStatusCode(http.StatusUnauthorized))
 	options := DefaultAuthorizationCodeCredentialOptions()
@@ -129,7 +129,7 @@ func TestAuthorizationCodeCredential_GetTokenInvalidCredentials(t *testing.T) {
 }
 
 func TestAuthorizationCodeCredential_GetTokenUnexpectedJSON(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespMalformed)))
 	options := DefaultAuthorizationCodeCredentialOptions()

diff --git a/sdk/azidentity/azidentity.go b/sdk/azidentity/azidentity.go
@@ -4,7 +4,9 @@
 package azidentity
 
 import (
+	"errors"
 	"net/http"
+	"net/url"
 	"os"
 	"time"
 
@@ -141,6 +143,14 @@ func (c *TokenCredentialOptions) setDefaultValues() (*TokenCredentialOptions, er
 		c.AuthorityHost = authorityHost
 	}
 
+	s, err := url.Parse(c.AuthorityHost)
+	if err != nil {
+		return nil, err
+	}
+
+	if s.Scheme != "https" {
+		return nil, errors.New("cannot use an authority host without https")
+	}
 	return c, nil
 }
 

diff --git a/sdk/azidentity/azidentity_test.go b/sdk/azidentity/azidentity_test.go
@@ -115,3 +115,11 @@ func Test_AzureGovernmentAuthorityHost(t *testing.T) {
 		t.Fatalf("Did not retrieve expected authority host string")
 	}
 }
+
+func Test_NonHTTPSAuthorityHost(t *testing.T) {
+	opts := &TokenCredentialOptions{AuthorityHost: "http://foo.com"}
+	opts, err := opts.setDefaultValues()
+	if err == nil {
+		t.Fatal("Expected an error but did not receive one.")
+	}
+}
diff --git a/sdk/azidentity/bearer_token_policy_test.go b/sdk/azidentity/bearer_token_policy_test.go
@@ -128,7 +128,7 @@ func TestRetryPolicy_NonRetriable(t *testing.T) {
 }
 
 func TestRetryPolicy_HTTPRequest(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithStatusCode(http.StatusUnauthorized))
 	cred, err := NewClientSecretCredential(tenantID, clientID, wrongSecret, &ClientSecretCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})

diff --git a/sdk/azidentity/chained_token_credential_test.go b/sdk/azidentity/chained_token_credential_test.go
@@ -64,7 +64,7 @@ func TestChainedTokenCredential_GetTokenSuccess(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Could not set environment variables for testing: %v", err)
 	}
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))
 	secCred, err := NewClientSecretCredential(tenantID, clientID, secret, &ClientSecretCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})
@@ -92,7 +92,7 @@ func TestChainedTokenCredential_GetTokenSuccess(t *testing.T) {
 }
 
 func TestChainedTokenCredential_GetTokenFail(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithStatusCode(http.StatusUnauthorized))
 	secCred, err := NewClientSecretCredential(tenantID, clientID, wrongSecret, &ClientSecretCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})
@@ -117,7 +117,7 @@ func TestChainedTokenCredential_GetTokenFail(t *testing.T) {
 }
 
 func TestChainedTokenCredential_GetTokenWithUnavailableCredentialInChain(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendError(&CredentialUnavailableError{CredentialType: "MockCredential", Message: "Mocking a credential unavailable error"})
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))

diff --git a/sdk/azidentity/client_certificate_credential_test.go b/sdk/azidentity/client_certificate_credential_test.go
@@ -65,7 +65,7 @@ func TestClientCertificateCredential_CreateAuthRequestSuccess(t *testing.T) {
 }
 
 func TestClientCertificateCredential_GetTokenSuccess(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))
 	cred, err := NewClientCertificateCredential(tenantID, clientID, certificatePath, &ClientCertificateCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})
@@ -79,7 +79,7 @@ func TestClientCertificateCredential_GetTokenSuccess(t *testing.T) {
 }
 
 func TestClientCertificateCredential_GetTokenInvalidCredentials(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.SetResponse(mock.WithStatusCode(http.StatusUnauthorized))
 	cred, err := NewClientCertificateCredential(tenantID, clientID, certificatePath, &ClientCertificateCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})
@@ -97,7 +97,7 @@ func TestClientCertificateCredential_GetTokenInvalidCredentials(t *testing.T) {
 }
 
 func TestClientCertificateCredential_WrongCertificatePath(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.SetResponse(mock.WithStatusCode(http.StatusUnauthorized))
 	_, err := NewClientCertificateCredential(tenantID, clientID, wrongCertificatePath, &ClientCertificateCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})
@@ -107,7 +107,7 @@ func TestClientCertificateCredential_WrongCertificatePath(t *testing.T) {
 }
 
 func TestClientCertificateCredential_GetTokenCheckPrivateKeyBlocks(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))
 	cred, err := NewClientCertificateCredential(tenantID, clientID, "testdata/certificate_formatB.pem", &ClientCertificateCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})
@@ -121,7 +121,7 @@ func TestClientCertificateCredential_GetTokenCheckPrivateKeyBlocks(t *testing.T)
 }
 
 func TestClientCertificateCredential_GetTokenCheckCertificateBlocks(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))
 	cred, err := NewClientCertificateCredential(tenantID, clientID, "testdata/certificate_formatA.pem", &ClientCertificateCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})
@@ -135,7 +135,7 @@ func TestClientCertificateCredential_GetTokenCheckCertificateBlocks(t *testing.T
 }
 
 func TestClientCertificateCredential_GetTokenEmptyCertificate(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))
 	_, err := NewClientCertificateCredential(tenantID, clientID, "testdata/certificate_empty.pem", &ClientCertificateCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})
@@ -145,7 +145,7 @@ func TestClientCertificateCredential_GetTokenEmptyCertificate(t *testing.T) {
 }
 
 func TestClientCertificateCredential_GetTokenNoPrivateKey(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))
 	_, err := NewClientCertificateCredential(tenantID, clientID, "testdata/certificate_nokey.pem", &ClientCertificateCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})

diff --git a/sdk/azidentity/client_secret_credential_test.go b/sdk/azidentity/client_secret_credential_test.go
@@ -64,7 +64,7 @@ func TestClientSecretCredential_CreateAuthRequestSuccess(t *testing.T) {
 }
 
 func TestClientSecretCredential_GetTokenSuccess(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))
 	cred, err := NewClientSecretCredential(tenantID, clientID, secret, &ClientSecretCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})
@@ -78,7 +78,7 @@ func TestClientSecretCredential_GetTokenSuccess(t *testing.T) {
 }
 
 func TestClientSecretCredential_GetTokenInvalidCredentials(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.SetResponse(mock.WithBody([]byte(accessTokenRespError)), mock.WithStatusCode(http.StatusUnauthorized))
 	cred, err := NewClientSecretCredential(tenantID, clientID, wrongSecret, &ClientSecretCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})
@@ -123,7 +123,7 @@ func TestClientSecretCredential_GetTokenInvalidCredentials(t *testing.T) {
 }
 
 func TestClientSecretCredential_GetTokenUnexpectedJSON(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespMalformed)))
 	cred, err := NewClientSecretCredential(tenantID, clientID, secret, &ClientSecretCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})

diff --git a/sdk/azidentity/device_code_credential_test.go b/sdk/azidentity/device_code_credential_test.go
@@ -149,7 +149,7 @@ func TestDeviceCodeCredential_RequestNewDeviceCodeCustomTenantIDClientID(t *test
 }
 
 func TestDeviceCodeCredential_GetTokenSuccess(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(deviceCodeResponse)))
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))
@@ -170,7 +170,7 @@ func TestDeviceCodeCredential_GetTokenSuccess(t *testing.T) {
 }
 
 func TestDeviceCodeCredential_GetTokenInvalidCredentials(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.SetResponse(mock.WithStatusCode(http.StatusUnauthorized))
 	cred, err := NewDeviceCodeCredential(&DeviceCodeCredentialOptions{TenantID: tenantID, ClientID: clientID, Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})
@@ -184,7 +184,7 @@ func TestDeviceCodeCredential_GetTokenInvalidCredentials(t *testing.T) {
 }
 
 func TestDeviceCodeCredential_GetTokenAuthorizationPending(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(deviceCodeResponse)))
 	srv.AppendResponse(mock.WithBody([]byte(authorizationPendingResponse)), mock.WithStatusCode(http.StatusUnauthorized))
@@ -202,7 +202,7 @@ func TestDeviceCodeCredential_GetTokenAuthorizationPending(t *testing.T) {
 }
 
 func TestDeviceCodeCredential_GetTokenExpiredToken(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(deviceCodeResponse)))
 	srv.AppendResponse(mock.WithBody([]byte(authorizationPendingResponse)), mock.WithStatusCode(http.StatusUnauthorized))
@@ -219,7 +219,7 @@ func TestDeviceCodeCredential_GetTokenExpiredToken(t *testing.T) {
 }
 
 func TestDeviceCodeCredential_GetTokenWithRefreshTokenFailure(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespError)), mock.WithStatusCode(http.StatusUnauthorized))
 	cred, err := NewDeviceCodeCredential(&DeviceCodeCredentialOptions{TenantID: tenantID, ClientID: clientID, Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})
@@ -238,7 +238,7 @@ func TestDeviceCodeCredential_GetTokenWithRefreshTokenFailure(t *testing.T) {
 }
 
 func TestDeviceCodeCredential_GetTokenWithRefreshTokenSuccess(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))
 	handler := func(DeviceCodeMessage) {}

diff --git a/sdk/azidentity/go.mod b/sdk/azidentity/go.mod
@@ -8,4 +8,5 @@ require (
 	github.com/Azure/azure-sdk-for-go/sdk/to v0.1.1
 	github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4
 	golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0
+	golang.org/x/net v0.0.0-20201010224723-4f7140c49acb
 )
diff --git a/sdk/azidentity/interactive_browser_credential_test.go b/sdk/azidentity/interactive_browser_credential_test.go
@@ -1,5 +1,5 @@
-// // Copyright (c) Microsoft Corporation. All rights reserved.
-// // Licensed under the MIT License.
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
 
 package azidentity
 
@@ -12,6 +12,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/azure-sdk-for-go/sdk/internal/mock"
 	"github.com/Azure/azure-sdk-for-go/sdk/to"
+	"golang.org/x/net/http2"
 )
 
 func TestInteractiveBrowserCredential_CreateWithNilOptions(t *testing.T) {
@@ -31,11 +32,17 @@ func TestInteractiveBrowserCredential_CreateWithNilOptions(t *testing.T) {
 }
 
 func TestInteractiveBrowserCredential_GetTokenSuccess(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer(mock.WithHTTP2Enabled(true))
 	defer close()
+	tr := &http.Transport{}
+	if err := http2.ConfigureTransport(tr); err != nil {
+		t.Fatalf("Failed to configure http2 transport: %v", err)
+	}
+	tr.TLSClientConfig.InsecureSkipVerify = true
+	client := &http.Client{Transport: tr}
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))
 	options := DefaultInteractiveBrowserCredentialOptions()
-	options.Options = &TokenCredentialOptions{AuthorityHost: srv.URL()}
+	options.Options = &TokenCredentialOptions{AuthorityHost: srv.URL(), HTTPClient: client}
 	cred, err := NewInteractiveBrowserCredential(&options)
 	if err != nil {
 		t.Fatalf("Unable to create credential. Received: %v", err)
@@ -56,12 +63,18 @@ func TestInteractiveBrowserCredential_GetTokenSuccess(t *testing.T) {
 }
 
 func TestInteractiveBrowserCredential_GetTokenInvalidCredentials(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer(mock.WithHTTP2Enabled(true))
 	defer close()
+	tr := &http.Transport{}
+	if err := http2.ConfigureTransport(tr); err != nil {
+		t.Fatalf("Failed to configure http2 transport: %v", err)
+	}
+	tr.TLSClientConfig.InsecureSkipVerify = true
+	client := &http.Client{Transport: tr}
 	srv.SetResponse(mock.WithBody([]byte(accessTokenRespError)), mock.WithStatusCode(http.StatusUnauthorized))
 	options := DefaultInteractiveBrowserCredentialOptions()
 	options.ClientSecret = to.StringPtr(wrongSecret)
-	options.Options = &TokenCredentialOptions{AuthorityHost: srv.URL()}
+	options.Options = &TokenCredentialOptions{AuthorityHost: srv.URL(), HTTPClient: client}
 	cred, err := NewInteractiveBrowserCredential(&options)
 	if err != nil {
 		t.Fatalf("Unable to create credential. Received: %v", err)

diff --git a/sdk/azidentity/username_password_credential_test.go b/sdk/azidentity/username_password_credential_test.go
@@ -62,7 +62,7 @@ func TestUsernamePasswordCredential_CreateAuthRequestSuccess(t *testing.T) {
 }
 
 func TestUsernamePasswordCredential_GetTokenSuccess(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))
 	cred, err := NewUsernamePasswordCredential(tenantID, clientID, "username", "password", &UsernamePasswordCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})
@@ -76,7 +76,7 @@ func TestUsernamePasswordCredential_GetTokenSuccess(t *testing.T) {
 }
 
 func TestUsernamePasswordCredential_GetTokenInvalidCredentials(t *testing.T) {
-	srv, close := mock.NewServer()
+	srv, close := mock.NewTLSServer()
 	defer close()
 	srv.SetResponse(mock.WithStatusCode(http.StatusUnauthorized))
 	cred, err := NewUsernamePasswordCredential(tenantID, clientID, "username", "wrong_password", &UsernamePasswordCredentialOptions{Options: &TokenCredentialOptions{HTTPClient: srv, AuthorityHost: srv.URL()}})