Switch out AAD auth implementation to leverage go-mssqldb (#46)

* switch to go-mssqldb AAD auth

* reference correct file in test results

* test cleanup

* avoid creating readline instance

* update readme

* readme formatting

Author: shueybubbles

.pipelines/TestSql2017.yml

@@ -17,21 +17,21 @@ steps:
 
   - template: include-runtests-linux.yml
     parameters:
-      RunName: 'SQL 2017'
+      RunName: 'SQL2017'
       SQLCMDUSER: sa
       SQLPASSWORD: $(PASSWORD)
 
   - template: include-runtests-linux.yml
     parameters:
-      RunName: 'SQL DB'
+      RunName: 'SQLDB'
       # AZURESERVER must be defined as a variable in the pipeline
       SQLCMDSERVER: $(AZURESERVER)
       AZURECLIENTSECRET: $(AZURECLIENTSECRET)
 
   - task: Palmmedia.reportgenerator.reportgenerator-build-release-task.reportgenerator@4
     displayName: Merge coverage data
     inputs:
-      reports: '"SQL 2017.coverage.xml";"SQL DB.coverage.xml"' # REQUIRED # The coverage reports that should be parsed (separated by semicolon). Globbing is supported.
+      reports: '**/*.coverage.xml"' # REQUIRED # The coverage reports that should be parsed (separated by semicolon). Globbing is supported.
       targetdir: 'coverage' # REQUIRED # The directory where the generated report should be saved.
       reporttypes: 'HtmlInline_AzurePipelines;Cobertura' # The output formats and scope (separated by semicolon) Values: Badges, Clover, Cobertura, CsvSummary, Html, HtmlChart, HtmlInline, HtmlInline_AzurePipelines, HtmlInline_AzurePipelines_Dark, HtmlSummary, JsonSummary, Latex, LatexSummary, lcov, MarkdownSummary, MHtml, PngChart, SonarQube, TeamCitySummary, TextSummary, Xml, XmlSummary
       sourcedirs: '$(Build.SourcesDirectory)' # Optional directories which contain the corresponding source code (separated by semicolon). The source directories are used if coverage report contains classes without path information.

.pipelines/include-runtests-linux.yml

@@ -20,7 +20,7 @@ steps:
   - script: |
       ~/go/bin/gotestsum --junitfile "${{ parameters.RunName }}.testresults.xml" -- ./... -coverprofile="${{ parameters.RunName }}.coverage.txt" -covermode count
       ~/go/bin/gocov convert "${{ parameters.RunName }}.coverage.txt" > "${{ parameters.RunName }}.coverage.json"
-      ~/go/bin/gocov-xml < "${{ parameters.RunName }}.coverage.json" > "${{ parameters.RunName }}.coverage.xml"
+      ~/go/bin/gocov-xml < "${{ parameters.RunName }}.coverage.json" > ${{ parameters.RunName }}.coverage.xml
       mkdir -p coverage
     workingDirectory: '$(Build.SourcesDirectory)'
     displayName: 'run tests'
@@ -38,9 +38,9 @@ steps:
   - task: PublishTestResults@2
     displayName: "Publish junit-style results"
     inputs:
-      testResultsFiles: '"${{ parameters.RunName }}.coverage.xml"'
+      testResultsFiles: '${{ parameters.RunName }}.testresults.xml'
       testResultsFormat: JUnit
       searchFolder: '$(Build.SourcesDirectory)'
       testRunTitle: '${{ parameters.RunName }} - $(Build.SourceBranchName)'
+      failTaskOnFailedTests: true
     condition: always()
-    continueOnError: true

README.md

@@ -12,9 +12,9 @@ We will be implementing command line switches and behaviors over time. Several s
 
 - `-P` switch will be removed. Passwords for SQL authentication can only be provided through these mechanisms:
 
-    -The `SQLCMDPASSWORD` environment variable
-    -The `:CONNECT` command
-    -When prompted, the user can type the password to complete a connection
+    - The `SQLCMDPASSWORD` environment variable
+    - The `:CONNECT` command
+    - When prompted, the user can type the password to complete a connection (pending [#50](https://github.com/microsoft/go-sqlcmd/issues/50))
 
 - `-R` switch will be removed. The go runtime does not provide access to user locale information, and it's not readily available through syscall on all supported platforms.
 - `-I` switch will be removed. To disable quoted identifier behavior, add `SET QUOTED IDENTIFIER OFF` in your scripts.
@@ -28,7 +28,7 @@ We will be implementing command line switches and behaviors over time. Several s
 
 ### Azure Active Directory Authentication
 
-This version of sqlcmd supports a broader range of AAD authentication models, based on the [azidentity package](https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azidentity).
+This version of sqlcmd supports a broader range of AAD authentication models, based on the [azidentity package](https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azidentity). The implementation relies on an AAD Connector in the [driver](https://github.com/denisenkom/go-mssqldb).
 
 #### Command line
 
@@ -61,23 +61,20 @@ Set `AZURE_TENANT_ID` environment variable to the tenant id of the server if not
 `ActiveDirectoryInteractive`
 
 This method will launch a web browser to authenticate the user.
-Set `AZURE_TENANT_ID` environment variable to the tenant id of the server if not using the default.
 
 `ActiveDirectoryManagedIdentity`
 
 Use this method when running sqlcmd on an Azure VM that has either a system-assigned or user-assigned managed identity. If using a user-assigned managed identity, set the user name to the ID of the managed identity. If using a system-assigned identity, leave user name empty.
 
 `ActiveDirectoryServicePrincipal`
 
-This method authenticates the provided user name as a service principal id and the password as the client secret for the service principal. Set `AZURE_TENANT_ID` environment variable to the tenant id of the service principal.
+This method authenticates the provided user name as a service principal id and the password as the client secret for the service principal. Provide a user name in the form `<service principal id>@<tenant id>`. Set `SQLCMDPASSWORD` variable to the client secret. If using a certificate instead of a client secret, set `AZURE_CLIENT_CERTIFICATE_PATH` environment variable to the path of the certificate file.
 
 ### Environment variables for AAD auth
 
 Some settings for AAD auth do not have command line inputs, and some environment variables are consumed directly by the `azidentity` package used by `sqlcmd`.
 These environment variables can be set to configure some aspects of AAD auth and to bypass default behaviors. In addition to the variables listed above, the following are sqlcmd-specific and apply to multiple methods.
 
-`SQLCMDAZURERESOURCE` - defines the URL of the Azure SQL database resource in the Azure cloud where the database resides. By default, `sqlcmd` attempts to match the DNS suffix of the server name with one of the well known Azure cloud DNS suffixes. If no match is found it uses `https://database.windows.net`.
-
 `SQLCMDCLIENTID` - set this to the identifier of an application registered in your AAD which is authorized to authenticate to Azure SQL Database. Applies to `ActiveDirectoryInteractive` and `ActiveDirectoryPassword` methods.
 
 ### Packages

cmd/sqlcmd/main.go

@@ -8,6 +8,7 @@ import (
 	"os"
 
 	"github.com/alecthomas/kong"
+	"github.com/denisenkom/go-mssqldb/azuread"
 	"github.com/gohxs/readline"
 	"github.com/microsoft/go-sqlcmd/pkg/sqlcmd"
 )
@@ -79,11 +80,11 @@ func (a SQLCmdArguments) authenticationMethod(hasPassword bool) string {
 	if a.UseAad {
 		switch {
 		case a.UserName == "":
-			return sqlcmd.ActiveDirectoryIntegrated
+			return azuread.ActiveDirectoryIntegrated
 		case hasPassword:
-			return sqlcmd.ActiveDirectoryPassword
+			return azuread.ActiveDirectoryPassword
 		default:
-			return sqlcmd.ActiveDirectoryInteractive
+			return azuread.ActiveDirectoryInteractive
 		}
 	}
 	if a.AuthenticationMethod == "" {
@@ -117,9 +118,9 @@ func setVars(vars *sqlcmd.Variables, args *SQLCmdArguments) {
 				return "true"
 			}
 			switch a.AuthenticationMethod {
-			case sqlcmd.ActiveDirectoryIntegrated:
-			case sqlcmd.ActiveDirectoryInteractive:
-			case sqlcmd.ActiveDirectoryPassword:
+			case azuread.ActiveDirectoryIntegrated:
+			case azuread.ActiveDirectoryInteractive:
+			case azuread.ActiveDirectoryPassword:
 				return "true"
 			}
 			return ""
@@ -180,7 +181,7 @@ func run(vars *sqlcmd.Variables) (int, error) {
 		return 1, err
 	}
 
-	iactive := args.InputFile == nil
+	iactive := args.InputFile == nil && args.Query == ""
 	var line *readline.Instance
 	if iactive {
 		line, err = readline.New(">")
@@ -221,7 +222,7 @@ func run(vars *sqlcmd.Variables) (int, error) {
 	if err != nil {
 		return 1, err
 	}
-	if iactive {
+	if iactive || s.Query != "" {
 		err = s.Run(once, false)
 	} else {
 		for f := range args.InputFile {

cmd/sqlcmd/main_test.go

@@ -160,7 +160,7 @@ func TestQueryAndExit(t *testing.T) {
 func TestAzureAuth(t *testing.T) {
 
 	if !canTestAzureAuth() {
-		t.Skip("AZURE auth environment variables are not set or server name is not an Azure DB name")
+		t.Skip("Server name is not an Azure DB name")
 	}
 	o, err := os.CreateTemp("", "sqlcmdmain")
 	assert.NoError(t, err, "os.CreateTemp")
@@ -183,10 +183,9 @@ func TestAzureAuth(t *testing.T) {
 	}
 }
 
+// Assuming public Azure, use AAD when SQLCMDUSER environment variable is not set
 func canTestAzureAuth() bool {
-	tenant := os.Getenv("AZURE_TENANT_ID")
-	clientId := os.Getenv("AZURE_CLIENT_ID")
-	clientSecret := os.Getenv("AZURE_CLIENT_SECRET")
-	server := os.Getenv("SQLCMDSERVER")
-	return tenant != "" && clientId != "" && clientSecret != "" && strings.Contains(server, ".database.")
+	server := os.Getenv(sqlcmd.SQLCMDSERVER)
+	userName := os.Getenv(sqlcmd.SQLCMDUSER)
+	return strings.Contains(server, ".database.windows.net") && userName == ""
 }

pkg/sqlcmd/azure_auth.go

@@ -4,57 +4,19 @@
 package sqlcmd
 
 import (
-	"context"
 	"database/sql/driver"
+	"net/url"
 	"os"
-	"strings"
 
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
-	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
-	mssql "github.com/denisenkom/go-mssqldb"
+	"github.com/denisenkom/go-mssqldb/azuread"
 )
 
 const (
-	ActiveDirectoryDefault          = "ActiveDirectoryDefault"
-	ActiveDirectoryIntegrated       = "ActiveDirectoryIntegrated"
-	ActiveDirectoryPassword         = "ActiveDirectoryPassword"
-	ActiveDirectoryInteractive      = "ActiveDirectoryInteractive"
-	ActiveDirectoryManagedIdentity  = "ActiveDirectoryManagedIdentity"
-	ActiveDirectoryServicePrincipal = "ActiveDirectoryServicePrincipal"
-	SqlPassword                     = "SqlPassword"
-	NotSpecified                    = "NotSpecified"
-	sqlClientId                     = "a94f9c62-97fe-4d19-b06d-472bed8d2bcf"
+	NotSpecified = "NotSpecified"
+	SqlPassword  = "SqlPassword"
+	sqlClientId  = "a94f9c62-97fe-4d19-b06d-472bed8d2bcf"
 )
 
-func azureTenantId() string {
-	t := os.Getenv("AZURE_TENANT_ID")
-	if t == "" {
-		t = "common"
-	}
-	return t
-}
-
-var resourceMap = map[string]string{
-	".database.chinacloudapi.cn":  "https://database.chinacloudapi.cn/",
-	".database.cloudapi.de":       "https://database.cloudapi.de/",
-	".database.usgovcloudapi.net": "https://database.usgovcloudapi.net/",
-	".database.windows.net":       "https://database.windows.net/",
-}
-
-func (s *Sqlcmd) getResourceUrl() string {
-	resource := os.Getenv("SQLCMDAZURERESOURCE")
-	if resource == "" {
-		server, _, _, _ := s.vars.SQLCmdServer()
-		for k := range resourceMap {
-			if strings.HasSuffix(strings.ToLower(server), k) {
-				return resourceMap[k]
-			}
-		}
-	}
-	return "https://database.windows.net"
-}
-
 func getSqlClientId() string {
 	if clientId := os.Getenv("SQLCMDCLIENTID"); clientId != "" {
 		return clientId
@@ -63,38 +25,31 @@ func getSqlClientId() string {
 }
 
 func (s *Sqlcmd) GetTokenBasedConnection(connstr string, user string, password string) (driver.Connector, error) {
-	var cred azcore.TokenCredential
-	var err error
-	scope := ".default"
-	t := azureTenantId()
-	switch s.Connect.AuthenticationMethod {
-	case ActiveDirectoryDefault:
-		cred, err = azidentity.NewDefaultAzureCredential(nil)
-	case ActiveDirectoryInteractive:
-		cred, err = azidentity.NewInteractiveBrowserCredential(&azidentity.InteractiveBrowserCredentialOptions{TenantID: t, ClientID: getSqlClientId()})
-	case ActiveDirectoryPassword:
-		cred, err = azidentity.NewUsernamePasswordCredential(t, getSqlClientId(), user, password, nil)
-	case ActiveDirectoryManagedIdentity:
-		cred, err = azidentity.NewManagedIdentityCredential(user, nil)
-	case ActiveDirectoryServicePrincipal:
-		cred, err = azidentity.NewClientSecretCredential(t, user, password, nil)
-	default:
-		// no implementation of AAD Integrated yet
-		cred, err = azidentity.NewDefaultAzureCredential(nil)
-	}
 
+	connectionUrl, err := url.Parse(connstr)
 	if err != nil {
 		return nil, err
 	}
-	resourceUrl := s.getResourceUrl()
-	conn, err := mssql.NewAccessTokenConnector(connstr, func() (string, error) {
-		opts := policy.TokenRequestOptions{Scopes: []string{resourceUrl + scope}}
-		tk, err := cred.GetToken(context.Background(), opts)
-		if err != nil {
-			return "", err
+
+	if user != "" {
+		connectionUrl.User = url.UserPassword(user, password)
+	}
+
+	query := connectionUrl.Query()
+	query.Set("fedauth", s.Connect.authenticationMethod())
+	query.Set("applicationclientid", getSqlClientId())
+
+	switch s.Connect.AuthenticationMethod {
+	case azuread.ActiveDirectoryServicePrincipal:
+	case azuread.ActiveDirectoryApplication:
+		query.Set("clientcertpath", os.Getenv("AZURE_CLIENT_CERTIFICATE_PATH"))
+	case azuread.ActiveDirectoryInteractive:
+		// AAD interactive needs minutes at minimum
+		if s.Connect.LoginTimeoutSeconds < 120 {
+			query.Set("connection timeout", "120")
 		}
-		return tk.Token, err
-	})
+	}
 
-	return conn, err
+	connectionUrl.RawQuery = query.Encode()
+	return azuread.NewConnector(connectionUrl.String())
 }

pkg/sqlcmd/sqlcmd_test.go

@@ -13,6 +13,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/denisenkom/go-mssqldb/azuread"
+
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
 )
@@ -96,7 +98,7 @@ func TestSqlCmdConnectDb(t *testing.T) {
 	v := InitializeVariables(true)
 	s := &Sqlcmd{vars: v}
 	if canTestAzureAuth() {
-		s.Connect.AuthenticationMethod = ActiveDirectoryDefault
+		s.Connect.AuthenticationMethod = azuread.ActiveDirectoryDefault
 	} else {
 		s.Connect.Password = os.Getenv(SQLCMDPASSWORD)
 	}
@@ -116,7 +118,7 @@ func ConnectDb() (*sql.DB, error) {
 	v := InitializeVariables(true)
 	s := &Sqlcmd{vars: v}
 	if canTestAzureAuth() {
-		s.Connect.AuthenticationMethod = ActiveDirectoryDefault
+		s.Connect.AuthenticationMethod = azuread.ActiveDirectoryDefault
 	} else {
 		s.Connect.Password = os.Getenv(SQLCMDPASSWORD)
 	}
@@ -154,12 +156,14 @@ func TestIncludeFileNoExecutions(t *testing.T) {
 		}
 		file, err = os.CreateTemp("", "sqlcmdout")
 		assert.NoError(t, err, "os.CreateTemp")
+		defer os.Remove(file.Name())
 		s.SetOutput(file)
 		// The second file has a go so it will execute all statements before it
 		err = s.IncludeFile(dataPath+"twobatchnoendinggo.sql", false)
 		if assert.NoError(t, err, "IncludeFile twobatchnoendinggo.sql false") {
 			assert.Equal(t, "-", s.batch.State(), "s.batch.State() after IncludeFile twobatchnoendinggo.sql false")
 			assert.Equal(t, "select 'string' as title", s.batch.String(), "s.batch.String() after IncludeFile twobatchnoendinggo.sql false")
+			s.SetOutput(nil)
 			bytes, err := os.ReadFile(file.Name())
 			if assert.NoError(t, err, "os.ReadFile") {
 				assert.Equal(t, "100"+SqlcmdEol+SqlcmdEol+oneRowAffected+SqlcmdEol+"string"+SqlcmdEol+SqlcmdEol+oneRowAffected+SqlcmdEol+"100"+SqlcmdEol+SqlcmdEol+oneRowAffected+SqlcmdEol, string(bytes), "Incorrect output from Run")
@@ -312,11 +316,13 @@ func runSqlCmd(t testing.TB, s *Sqlcmd, lines []string) error {
 }
 
 func setupSqlCmdWithMemoryOutput(t testing.TB) (*Sqlcmd, *memoryBuffer) {
+	t.Helper()
 	v := InitializeVariables(true)
 	v.Set(SQLCMDMAXVARTYPEWIDTH, "0")
 	s := New(nil, "", v)
 	if canTestAzureAuth() {
-		s.Connect.AuthenticationMethod = ActiveDirectoryDefault
+		t.Log("Using ActiveDirectoryDefault")
+		s.Connect.AuthenticationMethod = azuread.ActiveDirectoryDefault
 	} else {
 		s.Connect.Password = os.Getenv(SQLCMDPASSWORD)
 	}
@@ -329,11 +335,13 @@ func setupSqlCmdWithMemoryOutput(t testing.TB) (*Sqlcmd, *memoryBuffer) {
 }
 
 func setupSqlcmdWithFileOutput(t testing.TB) (*Sqlcmd, *os.File) {
+	t.Helper()
 	v := InitializeVariables(true)
 	v.Set(SQLCMDMAXVARTYPEWIDTH, "0")
 	s := New(nil, "", v)
 	if canTestAzureAuth() {
-		s.Connect.AuthenticationMethod = ActiveDirectoryDefault
+		t.Log("Using ActiveDirectoryDefault")
+		s.Connect.AuthenticationMethod = azuread.ActiveDirectoryDefault
 	} else {
 		s.Connect.Password = os.Getenv(SQLCMDPASSWORD)
 	}
@@ -346,10 +354,9 @@ func setupSqlcmdWithFileOutput(t testing.TB) (*Sqlcmd, *os.File) {
 	return s, file
 }
 
+// Assuming public Azure, use AAD when SQLCMDUSER environment variable is not set
 func canTestAzureAuth() bool {
-	tenant := os.Getenv("AZURE_TENANT_ID")
-	clientId := os.Getenv("AZURE_CLIENT_ID")
-	clientSecret := os.Getenv("AZURE_CLIENT_SECRET")
-	server := os.Getenv("SQLCMDSERVER")
-	return tenant != "" && clientId != "" && clientSecret != "" && strings.Contains(server, ".database.")
+	server := os.Getenv(SQLCMDSERVER)
+	userName := os.Getenv(SQLCMDUSER)
+	return strings.Contains(server, ".database.windows.net") && userName == ""
 }