Use parameters when executing prepared statements rather than fmt.Sprintf

builtin/logical/mssql/backend_test.go

@@ -4,17 +4,17 @@ import (
 	"context"
 	"fmt"
 	"log"
-	"os"
 	"reflect"
 	"testing"
 
 	_ "github.com/denisenkom/go-mssqldb"
 	logicaltest "github.com/hashicorp/vault/helper/testhelpers/logical"
+	mssqlhelper "github.com/hashicorp/vault/helper/testhelpers/mssql"
 	"github.com/hashicorp/vault/sdk/logical"
 	"github.com/mitchellh/mapstructure"
 )
 
-func TestBackend_config_connection(t *testing.T) {
+func Backend_config_connection(t *testing.T) {
 	var resp *logical.Response
 	var err error
 	config := logical.TestBackendConfig()
@@ -55,15 +55,12 @@ func TestBackend_config_connection(t *testing.T) {
 }
 
 func TestBackend_basic(t *testing.T) {
-	if os.Getenv(logicaltest.TestEnvVar) == "" || os.Getenv("MSSQL_URL") == "" {
-		t.Skip(fmt.Sprintf("Acceptance tests skipped unless env '%s' set", logicaltest.TestEnvVar))
-	}
-	connURL := os.Getenv("MSSQL_URL")
-
 	b, _ := Factory(context.Background(), logical.TestBackendConfig())
 
+	cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t)
+	defer cleanup()
+
 	logicaltest.Test(t, logicaltest.TestCase{
-		AcceptanceTest: true,
 		PreCheck:       testAccPreCheckFunc(t, connURL),
 		LogicalBackend: b,
 		Steps: []logicaltest.TestStep{
@@ -75,15 +72,12 @@ func TestBackend_basic(t *testing.T) {
 }
 
 func TestBackend_roleCrud(t *testing.T) {
-	if os.Getenv(logicaltest.TestEnvVar) == "" || os.Getenv("MSSQL_URL") == "" {
-		t.Skip(fmt.Sprintf("Acceptance tests skipped unless env '%s' set", logicaltest.TestEnvVar))
-	}
-	connURL := os.Getenv("MSSQL_URL")
-
 	b := Backend()
 
+	cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t)
+	defer cleanup()
+
 	logicaltest.Test(t, logicaltest.TestCase{
-		AcceptanceTest: true,
 		PreCheck:       testAccPreCheckFunc(t, connURL),
 		LogicalBackend: b,
 		Steps: []logicaltest.TestStep{
@@ -97,15 +91,12 @@ func TestBackend_roleCrud(t *testing.T) {
 }
 
 func TestBackend_leaseWriteRead(t *testing.T) {
-	if os.Getenv(logicaltest.TestEnvVar) == "" || os.Getenv("MSSQL_URL") == "" {
-		t.Skip(fmt.Sprintf("Acceptance tests skipped unless env '%s' set", logicaltest.TestEnvVar))
-	}
-	connURL := os.Getenv("MSSQL_URL")
-
 	b := Backend()
 
+	cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t)
+	defer cleanup()
+
 	logicaltest.Test(t, logicaltest.TestCase{
-		AcceptanceTest: true,
 		PreCheck:       testAccPreCheckFunc(t, connURL),
 		LogicalBackend: b,
 		Steps: []logicaltest.TestStep{

builtin/logical/mssql/secret_creds.go

@@ -77,14 +77,13 @@ func (b *backend) secretCredsRevoke(ctx context.Context, req *logical.Request, d
 	// sessions.  There cannot be any active sessions before we drop the logins
 	// This isn't done in a transaction because even if we fail along the way,
 	// we want to remove as much access as possible
-	sessionStmt, err := db.Prepare(fmt.Sprintf(
-		"SELECT session_id FROM sys.dm_exec_sessions WHERE login_name = '%s';", username))
+	sessionStmt, err := db.Prepare("SELECT session_id FROM sys.dm_exec_sessions WHERE login_name = @p1;")
 	if err != nil {
 		return nil, err
 	}
 	defer sessionStmt.Close()
 
-	sessionRows, err := sessionStmt.Query()
+	sessionRows, err := sessionStmt.Query(username)
 	if err != nil {
 		return nil, err
 	}
@@ -105,13 +104,13 @@ func (b *backend) secretCredsRevoke(ctx context.Context, req *logical.Request, d
 	// we need to drop the database users before we can drop the login and the role
 	// This isn't done in a transaction because even if we fail along the way,
 	// we want to remove as much access as possible
-	stmt, err := db.Prepare(fmt.Sprintf("EXEC master.dbo.sp_msloginmappings '%s';", username))
+	stmt, err := db.Prepare("EXEC master.dbo.sp_msloginmappings @p1;")
 	if err != nil {
 		return nil, err
 	}
 	defer stmt.Close()
 
-	rows, err := stmt.Query()
+	rows, err := stmt.Query(username)
 	if err != nil {
 		return nil, err
 	}

go.mod

@@ -30,7 +30,7 @@ require (
 	github.com/client9/misspell v0.3.4
 	github.com/cockroachdb/cockroach-go v0.0.0-20181001143604-e0a95dfd547c
 	github.com/coreos/go-semver v0.2.0
-	github.com/denisenkom/go-mssqldb v0.0.0-20190412130859-3b1d194e553a
+	github.com/denisenkom/go-mssqldb v0.0.0-20200428022330-06a60b6afbbc
 	github.com/dnaeon/go-vcr v1.0.1 // indirect
 	github.com/dsnet/compress v0.0.1 // indirect
 	github.com/duosecurity/duo_api_golang v0.0.0-20190308151101-6c680f768e74

go.sum

@@ -202,6 +202,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/denisenkom/go-mssqldb v0.0.0-20190412130859-3b1d194e553a h1:yJ2kD1BvM28M4gt31MuDr0ROKsW+v6zBk9G0Bcr8qAY=
 github.com/denisenkom/go-mssqldb v0.0.0-20190412130859-3b1d194e553a/go.mod h1:zAg7JM8CkOJ43xKXIj7eRO9kmWm/TW578qo+oDO6tuM=
+github.com/denisenkom/go-mssqldb v0.0.0-20200428022330-06a60b6afbbc h1:VRRKCwnzqk8QCaRC4os14xoKDdbHqqlJtJA0oc1ZAjg=
+github.com/denisenkom/go-mssqldb v0.0.0-20200428022330-06a60b6afbbc/go.mod h1:xbL0rPBG9cCiLr28tMa8zpbdarY27NDyej4t/EjAShU=
 github.com/dgrijalva/jwt-go v3.2.0+incompatible h1:7qlOGliEKZXTDg6OTjfoBKDXWrumCAMpl/TFQ4/5kLM=
 github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
 github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no=
@@ -290,6 +292,8 @@ github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zV
 github.com/gogo/protobuf v1.3.1 h1:DqDEcV5aeaTmdFBePNpYsp3FlcVH/2ISVVM9Qf8PSls=
 github.com/gogo/protobuf v1.3.1/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o=
 github.com/goji/httpauth v0.0.0-20160601135302-2da839ab0f4d/go.mod h1:nnjvkQ9ptGaCkuDUx6wNykzzlUixGxvkme+H/lnzb+A=
+github.com/golang-sql/civil v0.0.0-20190719163853-cb61b32ac6fe h1:lXe2qZdvpiX5WZkZR4hgp4KJVfY3nMkvmwbVkpv1rVY=
+github.com/golang-sql/civil v0.0.0-20190719163853-cb61b32ac6fe/go.mod h1:8vg3r2VgvsThLBIFL93Qb5yWzgyZWhEmBwUJWevAkK0=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
 github.com/golang/groupcache v0.0.0-20160516000752-02826c3e7903/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
 github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=

helper/testhelpers/mssql/mssqlhelper.go

@@ -0,0 +1,50 @@
+package mssqlhelper
+
+import (
+	"database/sql"
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/hashicorp/vault/helper/testhelpers/docker"
+	"github.com/ory/dockertest"
+)
+
+func PrepareMSSQLTestContainer(t *testing.T) (cleanup func(), retURL string) {
+	if os.Getenv("MSSQL_URL") != "" {
+		return func() {}, os.Getenv("MSSQL_URL")
+	}
+
+	pool, err := dockertest.NewPool("")
+	if err != nil {
+		t.Fatalf("Failed to connect to docker: %s", err)
+	}
+
+	resource, err := pool.Run("mcr.microsoft.com/mssql/server", "2017-latest-ubuntu", []string{"ACCEPT_EULA=Y", "SA_PASSWORD=yourStrong(!)Password"})
+	if err != nil {
+		t.Fatalf("Could not start local MSSQL docker container: %s", err)
+	}
+
+	cleanup = func() {
+		docker.CleanupResource(t, pool, resource)
+	}
+
+	retURL = fmt.Sprintf("sqlserver://sa:yourStrong(!)[email protected]:%s", resource.GetPort("1433/tcp"))
+
+	// exponential backoff-retry
+	if err = pool.Retry(func() error {
+		var err error
+		var db *sql.DB
+		db, err = sql.Open("mssql", retURL)
+		if err != nil {
+			return err
+		}
+		defer db.Close()
+		return db.Ping()
+	}); err != nil {
+		cleanup()
+		t.Fatalf("Could not connect to MSSQL docker container: %s", err)
+	}
+
+	return
+}

plugins/database/mssql/mssql.go

@@ -215,14 +215,14 @@ func (m *MSSQL) revokeUserDefault(ctx context.Context, username string) error {
 	// sessions.  There cannot be any active sessions before we drop the logins
 	// This isn't done in a transaction because even if we fail along the way,
 	// we want to remove as much access as possible
-	sessionStmt, err := db.PrepareContext(ctx, fmt.Sprintf(
-		"SELECT session_id FROM sys.dm_exec_sessions WHERE login_name = '%s';", username))
+	sessionStmt, err := db.PrepareContext(ctx,
+		"SELECT session_id FROM sys.dm_exec_sessions WHERE login_name = @p1;")
 	if err != nil {
 		return err
 	}
 	defer sessionStmt.Close()
 
-	sessionRows, err := sessionStmt.QueryContext(ctx)
+	sessionRows, err := sessionStmt.QueryContext(ctx, username)
 	if err != nil {
 		return err
 	}
@@ -243,13 +243,13 @@ func (m *MSSQL) revokeUserDefault(ctx context.Context, username string) error {
 	// we need to drop the database users before we can drop the login and the role
 	// This isn't done in a transaction because even if we fail along the way,
 	// we want to remove as much access as possible
-	stmt, err := db.PrepareContext(ctx, fmt.Sprintf("EXEC master.dbo.sp_msloginmappings '%s';", username))
+	stmt, err := db.PrepareContext(ctx, "EXEC master.dbo.sp_msloginmappings @p1;")
 	if err != nil {
 		return err
 	}
 	defer stmt.Close()
 
-	rows, err := stmt.QueryContext(ctx)
+	rows, err := stmt.QueryContext(ctx, username)
 	if err != nil {
 		return err
 	}

plugins/database/mssql/mssql_test.go

@@ -4,19 +4,17 @@ import (
 	"context"
 	"database/sql"
 	"fmt"
-	"os"
 	"strings"
 	"testing"
 	"time"
 
+	mssqlhelper "github.com/hashicorp/vault/helper/testhelpers/mssql"
 	"github.com/hashicorp/vault/sdk/database/dbplugin"
 )
 
 func TestMSSQL_Initialize(t *testing.T) {
-	if os.Getenv("MSSQL_URL") == "" || os.Getenv("VAULT_ACC") != "1" {
-		t.SkipNow()
-	}
-	connURL := os.Getenv("MSSQL_URL")
+	cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t)
+	defer cleanup()
 
 	connectionDetails := map[string]interface{}{
 		"connection_url": connURL,
@@ -50,10 +48,8 @@ func TestMSSQL_Initialize(t *testing.T) {
 }
 
 func TestMSSQL_CreateUser(t *testing.T) {
-	if os.Getenv("MSSQL_URL") == "" || os.Getenv("VAULT_ACC") != "1" {
-		t.SkipNow()
-	}
-	connURL := os.Getenv("MSSQL_URL")
+	cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t)
+	defer cleanup()
 
 	connectionDetails := map[string]interface{}{
 		"connection_url": connURL,
@@ -91,10 +87,8 @@ func TestMSSQL_CreateUser(t *testing.T) {
 }
 
 func TestMSSQL_RotateRootCredentials(t *testing.T) {
-	if os.Getenv("MSSQL_URL") == "" || os.Getenv("VAULT_ACC") != "1" {
-		t.SkipNow()
-	}
-	connURL := os.Getenv("MSSQL_URL")
+	cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t)
+	defer cleanup()
 
 	connectionDetails := map[string]interface{}{
 		"connection_url": connURL,
@@ -130,10 +124,8 @@ func TestMSSQL_RotateRootCredentials(t *testing.T) {
 }
 
 func TestMSSQL_RevokeUser(t *testing.T) {
-	if os.Getenv("MSSQL_URL") == "" || os.Getenv("VAULT_ACC") != "1" {
-		t.SkipNow()
-	}
-	connURL := os.Getenv("MSSQL_URL")
+	cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t)
+	defer cleanup()
 
 	connectionDetails := map[string]interface{}{
 		"connection_url": connURL,