From 0ccdfa96a355ab22e961de3f403e1b63c3ba6b35 Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Fri, 9 Oct 2020 16:50:12 -0600 Subject: [PATCH 1/3] Migrate to Database v5 interface --- plugins/database/mssql/mssql.go | 209 +++++-------- plugins/database/mssql/mssql_test.go | 429 +++++++++++++-------------- 2 files changed, 286 insertions(+), 352 deletions(-) diff --git a/plugins/database/mssql/mssql.go b/plugins/database/mssql/mssql.go index 52023fcf2b51..d290242d3f47 100644 --- a/plugins/database/mssql/mssql.go +++ b/plugins/database/mssql/mssql.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "strings" - "time" _ "github.com/denisenkom/go-mssqldb" "github.com/hashicorp/errwrap" @@ -16,24 +15,25 @@ import ( "github.com/hashicorp/vault/sdk/database/helper/connutil" "github.com/hashicorp/vault/sdk/database/helper/credsutil" "github.com/hashicorp/vault/sdk/database/helper/dbutil" + "github.com/hashicorp/vault/sdk/database/newdbplugin" "github.com/hashicorp/vault/sdk/helper/dbtxn" "github.com/hashicorp/vault/sdk/helper/strutil" + "github.com/ryboe/q" ) const msSQLTypeName = "mssql" -var _ dbplugin.Database = &MSSQL{} +var _ newdbplugin.Database = &MSSQL{} // MSSQL is an implementation of Database interface type MSSQL struct { *connutil.SQLConnectionProducer - credsutil.CredentialsProducer } func New() (interface{}, error) { db := new() // Wrap the plugin with middleware to sanitize errors - dbType := dbplugin.NewDatabaseErrorSanitizerMiddleware(db, db.SecretValues) + dbType := newdbplugin.NewDatabaseErrorSanitizerMiddleware(db, db.secretValues) return dbType, nil } @@ -42,16 +42,8 @@ func new() *MSSQL { connProducer := &connutil.SQLConnectionProducer{} connProducer.Type = msSQLTypeName - credsProducer := &credsutil.SQLCredentialsProducer{ - DisplayNameLen: 20, - RoleNameLen: 20, - UsernameLen: 128, - Separator: "-", - } - return &MSSQL{ SQLConnectionProducer: connProducer, - CredentialsProducer: credsProducer, } } @@ -72,6 +64,12 @@ func (m *MSSQL) Type() (string, error) { return msSQLTypeName, nil } +func (m *MSSQL) secretValues() map[string]string { + return map[string]string{ + m.Password: "[password]", + } +} + func (m *MSSQL) getConnection(ctx context.Context) (*sql.DB, error) { db, err := m.Connection(ctx) if err != nil { @@ -81,49 +79,51 @@ func (m *MSSQL) getConnection(ctx context.Context) (*sql.DB, error) { return db.(*sql.DB), nil } -// CreateUser generates the username/password on the underlying MSSQL secret backend as instructed by -// the CreationStatement provided. -func (m *MSSQL) CreateUser(ctx context.Context, statements dbplugin.Statements, usernameConfig dbplugin.UsernameConfig, expiration time.Time) (username string, password string, err error) { - // Grab the lock +func (m *MSSQL) Initialize(ctx context.Context, req newdbplugin.InitializeRequest) (newdbplugin.InitializeResponse, error) { + newConf, err := m.SQLConnectionProducer.Init(ctx, req.Config, req.VerifyConnection) + if err != nil { + return newdbplugin.InitializeResponse{}, err + } + resp := newdbplugin.InitializeResponse{ + Config: newConf, + } + return resp, nil +} + +// NewUser generates the username/password on the underlying MSSQL secret backend as instructed by +// the statements provided. +func (m *MSSQL) NewUser(ctx context.Context, req newdbplugin.NewUserRequest) (newdbplugin.NewUserResponse, error) { m.Lock() defer m.Unlock() - statements = dbutil.StatementCompatibilityHelper(statements) - - // Get the connection db, err := m.getConnection(ctx) if err != nil { - return "", "", err + return newdbplugin.NewUserResponse{}, fmt.Errorf("unable to get connection: %w", err) } - if len(statements.Creation) == 0 { - return "", "", dbutil.ErrEmptyCreationStatement + if len(req.Statements.Commands) == 0 { + return newdbplugin.NewUserResponse{}, dbutil.ErrEmptyCreationStatement } - username, err = m.GenerateUsername(usernameConfig) + username, err := credsutil.GenerateUsername( + credsutil.DisplayName(req.UsernameConfig.DisplayName, 20), + credsutil.RoleName(req.UsernameConfig.RoleName, 20), + credsutil.MaxLength(128), + credsutil.Separator("-"), + ) if err != nil { - return "", "", err + return newdbplugin.NewUserResponse{}, err } - password, err = m.GeneratePassword() - if err != nil { - return "", "", err - } + expirationStr := req.Expiration.Format("2006-01-02 15:04:05-0700") - expirationStr, err := m.GenerateExpiration(expiration) - if err != nil { - return "", "", err - } - - // Start a transaction tx, err := db.BeginTx(ctx, nil) if err != nil { - return "", "", err + return newdbplugin.NewUserResponse{}, err } defer tx.Rollback() - // Execute each query - for _, stmt := range statements.Creation { + for _, stmt := range req.Statements.Commands { for _, query := range strutil.ParseArbitraryStringSlice(stmt, ";") { query = strings.TrimSpace(query) if len(query) == 0 { @@ -132,50 +132,45 @@ func (m *MSSQL) CreateUser(ctx context.Context, statements dbplugin.Statements, m := map[string]string{ "name": username, - "password": password, + "password": req.Password, "expiration": expirationStr, } if err := dbtxn.ExecuteTxQuery(ctx, tx, m, query); err != nil { - return "", "", err + return newdbplugin.NewUserResponse{}, err } } } - // Commit the transaction if err := tx.Commit(); err != nil { - return "", "", err + return newdbplugin.NewUserResponse{}, err } - return username, password, nil -} + resp := newdbplugin.NewUserResponse{ + Username: username, + } -// RenewUser is not supported on MSSQL, so this is a no-op. -func (m *MSSQL) RenewUser(ctx context.Context, statements dbplugin.Statements, username string, expiration time.Time) error { - // NOOP - return nil + return resp, nil } -// RevokeUser attempts to drop the specified user. It will first attempt to disable login, +// DeleteUser attempts to drop the specified user. It will first attempt to disable login, // then kill pending connections from that user, and finally drop the user and login from the // database instance. -func (m *MSSQL) RevokeUser(ctx context.Context, statements dbplugin.Statements, username string) error { - statements = dbutil.StatementCompatibilityHelper(statements) - - if len(statements.Revocation) == 0 { - return m.revokeUserDefault(ctx, username) +func (m *MSSQL) DeleteUser(ctx context.Context, req newdbplugin.DeleteUserRequest) (newdbplugin.DeleteUserResponse, error) { + if len(req.Statements.Commands) == 0 { + err := m.revokeUserDefault(ctx, req.Username) + return newdbplugin.DeleteUserResponse{}, err } - // Get connection db, err := m.getConnection(ctx) if err != nil { - return err + return newdbplugin.DeleteUserResponse{}, fmt.Errorf("unable to get connection: %w", err) } - var result error + merr := &multierror.Error{} // Execute each query - for _, stmt := range statements.Revocation { + for _, stmt := range req.Statements.Commands { for _, query := range strutil.ParseArbitraryStringSlice(stmt, ";") { query = strings.TrimSpace(query) if len(query) == 0 { @@ -183,15 +178,15 @@ func (m *MSSQL) RevokeUser(ctx context.Context, statements dbplugin.Statements, } m := map[string]string{ - "name": username, + "name": req.Username, } if err := dbtxn.ExecuteDBQuery(ctx, db, m, query); err != nil { - result = multierror.Append(result, err) + merr = multierror.Append(merr, err) } } } - return result + return newdbplugin.DeleteUserResponse{}, merr.ErrorOrNil() } func (m *MSSQL) revokeUserDefault(ctx context.Context, username string) error { @@ -297,84 +292,36 @@ func (m *MSSQL) revokeUserDefault(ctx context.Context, username string) error { return nil } -func (m *MSSQL) RotateRootCredentials(ctx context.Context, statements []string) (map[string]interface{}, error) { - m.Lock() - defer m.Unlock() - - if len(m.Username) == 0 || len(m.Password) == 0 { - return nil, errors.New("username and password are required to rotate") - } - - rotateStatents := statements - if len(rotateStatents) == 0 { - rotateStatents = []string{alterLoginSQL} - } - - db, err := m.getConnection(ctx) - if err != nil { - return nil, err - } - - tx, err := db.BeginTx(ctx, nil) - if err != nil { - return nil, err - } - defer func() { - tx.Rollback() - }() - - password, err := m.GeneratePassword() - if err != nil { - return nil, err - } - - for _, stmt := range rotateStatents { - for _, query := range strutil.ParseArbitraryStringSlice(stmt, ";") { - query = strings.TrimSpace(query) - if len(query) == 0 { - continue - } - - m := map[string]string{ - "username": m.Username, - "password": password, - } - if err := dbtxn.ExecuteTxQuery(ctx, tx, m, query); err != nil { - return nil, err - } - } - } - - if err := tx.Commit(); err != nil { - return nil, err - } - - if err := db.Close(); err != nil { - return nil, err +func (m *MSSQL) UpdateUser(ctx context.Context, req newdbplugin.UpdateUserRequest) (newdbplugin.UpdateUserResponse, error) { + if req.Password != nil { + err := m.updateUserPass(ctx, req.Username, req.Password) + return newdbplugin.UpdateUserResponse{}, err } - - m.RawConfig["password"] = password - return m.RawConfig, nil + // Expiration is a no-op + return newdbplugin.UpdateUserResponse{}, nil } -func (m *MSSQL) SetCredentials(ctx context.Context, statements dbplugin.Statements, staticUser dbplugin.StaticUserConfig) (username, password string, err error) { - if len(statements.Rotation) == 0 { - statements.Rotation = []string{alterLoginSQL} +func (m *MSSQL) updateUserPass(ctx context.Context, username string, changePass *newdbplugin.ChangePassword) error { + q.Q("Updating user", username, changePass) + stmts := changePass.Statements.Commands + if len(stmts) == 0 { + stmts = []string{alterLoginSQL} } - username = staticUser.Username - password = staticUser.Password + password := changePass.NewPassword if username == "" || password == "" { - return "", "", errors.New("must provide both username and password") + q.Q("username or password is empty") + return errors.New("must provide both username and password") } + q.Q("changing password now...") m.Lock() defer m.Unlock() db, err := m.getConnection(ctx) if err != nil { - return "", "", err + return err } var exists bool @@ -382,16 +329,14 @@ func (m *MSSQL) SetCredentials(ctx context.Context, statements dbplugin.Statemen err = db.QueryRowContext(ctx, "SELECT 1 FROM master.sys.server_principals where name = N'$1'", username).Scan(&exists) if err != nil && err != sql.ErrNoRows { - return "", "", err + return err } - stmts := statements.Rotation - - // Start a transaction tx, err := db.BeginTx(ctx, nil) if err != nil { - return "", "", err + return err } + defer func() { _ = tx.Rollback() }() @@ -409,16 +354,16 @@ func (m *MSSQL) SetCredentials(ctx context.Context, statements dbplugin.Statemen "password": password, } if err := dbtxn.ExecuteTxQuery(ctx, tx, m, query); err != nil { - return "", "", err + return fmt.Errorf("failed to execute query: %w", err) } } } if err := tx.Commit(); err != nil { - return "", "", err + return fmt.Errorf("failed to commit transaction: %w", err) } - return username, password, nil + return nil } const dropUserSQL = ` diff --git a/plugins/database/mssql/mssql_test.go b/plugins/database/mssql/mssql_test.go index e8522ac8b281..1bf108dc0f1b 100644 --- a/plugins/database/mssql/mssql_test.go +++ b/plugins/database/mssql/mssql_test.go @@ -4,196 +4,209 @@ import ( "context" "database/sql" "fmt" + "reflect" + "regexp" "strings" "testing" "time" - mssqlhelper "github.com/hashicorp/vault/helper/testhelpers/mssql" - "github.com/hashicorp/vault/sdk/database/dbplugin" + "github.com/hashicorp/vault/sdk/database/newdbplugin" + dbtesting "github.com/hashicorp/vault/sdk/database/newdbplugin/testing" "github.com/hashicorp/vault/sdk/helper/dbtxn" -) - -func TestMSSQL_Initialize(t *testing.T) { - cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t) - defer cleanup() - - connectionDetails := map[string]interface{}{ - "connection_url": connURL, - } - - db := new() - _, err := db.Init(context.Background(), connectionDetails, true) - if err != nil { - t.Fatalf("err: %s", err) - } - - if !db.Initialized { - t.Fatal("Database should be initialized") - } - - err = db.Close() - if err != nil { - t.Fatalf("err: %s", err) - } - - // Test decoding a string value for max_open_connections - connectionDetails = map[string]interface{}{ - "connection_url": connURL, - "max_open_connections": "5", - } - _, err = db.Init(context.Background(), connectionDetails, true) - if err != nil { - t.Fatalf("err: %s", err) - } -} + mssqlhelper "github.com/hashicorp/vault/helper/testhelpers/mssql" +) -func TestMSSQL_CreateUser(t *testing.T) { +func TestInitialize(t *testing.T) { cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t) defer cleanup() - connectionDetails := map[string]interface{}{ - "connection_url": connURL, - } - - db := new() - _, err := db.Init(context.Background(), connectionDetails, true) - if err != nil { - t.Fatalf("err: %s", err) - } - - usernameConfig := dbplugin.UsernameConfig{ - DisplayName: "test", - RoleName: "test", - } - - // Test with no configured Creation Statement - _, _, err = db.CreateUser(context.Background(), dbplugin.Statements{}, usernameConfig, time.Now().Add(time.Minute)) - if err == nil { - t.Fatal("Expected error when no creation statement is provided") + type testCase struct { + req newdbplugin.InitializeRequest } - statements := dbplugin.Statements{ - Creation: []string{testMSSQLRole}, + tests := map[string]testCase{ + "happy path": { + req: newdbplugin.InitializeRequest{ + Config: map[string]interface{}{ + "connection_url": connURL, + }, + VerifyConnection: true, + }, + }, + "max_open_connections set": { + newdbplugin.InitializeRequest{ + Config: map[string]interface{}{ + "connection_url": connURL, + "max_open_connections": "5", + }, + VerifyConnection: true, + }, + }, } - username, password, err := db.CreateUser(context.Background(), statements, usernameConfig, time.Now().Add(time.Minute)) - if err != nil { - t.Fatalf("err: %s", err) - } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + db := new() + dbtesting.AssertInitialize(t, db, test.req) + defer dbtesting.AssertClose(t, db) - if err = testCredsExist(t, connURL, username, password); err != nil { - t.Fatalf("Could not connect with new credentials: %s", err) + if !db.Initialized { + t.Fatal("Database should be initialized") + } + }) } } -func TestMSSQL_RotateRootCredentials(t *testing.T) { +func TestNewUser(t *testing.T) { cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t) defer cleanup() - connectionDetails := map[string]interface{}{ - "connection_url": connURL, - "username": "sa", - "password": "yourStrong(!)Password", - } - - db := new() - - connProducer := db.SQLConnectionProducer - - _, err := db.Init(context.Background(), connectionDetails, true) - if err != nil { - t.Fatalf("err: %s", err) - } - - if !connProducer.Initialized { - t.Fatal("Database should be initialized") - } - - newConf, err := db.RotateRootCredentials(context.Background(), nil) - if err != nil { - t.Fatalf("err: %v", err) - } - if newConf["password"] == "yourStrong(!)Password" { - t.Fatal("password was not updated") - } - - err = db.Close() - if err != nil { - t.Fatalf("err: %s", err) - } -} - -func TestMSSQL_SetCredentials_missingArgs(t *testing.T) { type testCase struct { - statements dbplugin.Statements - userConfig dbplugin.StaticUserConfig + req newdbplugin.NewUserRequest + usernameRegex string + expectErr bool + assertUser func(t testing.TB, connURL, username, password string) } tests := map[string]testCase{ - "empty rotation statements": { - statements: dbplugin.Statements{ - Rotation: nil, - }, - userConfig: dbplugin.StaticUserConfig{ - Username: "testuser", - Password: "password", - }, - }, - "empty username": { - statements: dbplugin.Statements{ - Rotation: []string{` - ALTER LOGIN [{{username}}] WITH PASSWORD = '{{password}}';`, + "no creation statements": { + req: newdbplugin.NewUserRequest{ + UsernameConfig: newdbplugin.UsernameMetadata{ + DisplayName: "test", + RoleName: "test", }, + Statements: newdbplugin.Statements{}, + Password: "AG4qagho-dsvZ", + Expiration: time.Now().Add(1 * time.Second), }, - userConfig: dbplugin.StaticUserConfig{ - Username: "", - Password: "password", - }, + usernameRegex: "^$", + expectErr: true, + assertUser: assertCredsDoNotExist, }, - "empty password": { - statements: dbplugin.Statements{ - Rotation: []string{` - ALTER LOGIN [{{username}}] WITH PASSWORD = '{{password}}';`, + "with creation statements": { + req: newdbplugin.NewUserRequest{ + UsernameConfig: newdbplugin.UsernameMetadata{ + DisplayName: "test", + RoleName: "test", }, + Statements: newdbplugin.Statements{ + Commands: []string{testMSSQLRole}, + }, + Password: "AG4qagho-dsvZ", + Expiration: time.Now().Add(1 * time.Second), }, - userConfig: dbplugin.StaticUserConfig{ - Username: "testuser", - Password: "", - }, + usernameRegex: "^v-test-test-[a-zA-Z0-9]{20}-[0-9]{10}$", + expectErr: false, + assertUser: assertCredsExist, }, } for name, test := range tests { t.Run(name, func(t *testing.T) { + usernameRe, err := regexp.Compile(test.usernameRegex) + if err != nil { + t.Fatalf("failed to compile username regex %q: %s", test.usernameRegex, err) + } + + initReq := newdbplugin.InitializeRequest{ + Config: map[string]interface{}{ + "connection_url": connURL, + }, + VerifyConnection: true, + } + db := new() + dbtesting.AssertInitialize(t, db, initReq) + defer dbtesting.AssertClose(t, db) + + createResp, err := db.NewUser(context.Background(), test.req) + if test.expectErr && err == nil { + t.Fatalf("err expected, got nil") + } + if !test.expectErr && err != nil { + t.Fatalf("no error expected, got: %s", err) + } - username, password, err := db.SetCredentials(context.Background(), test.statements, test.userConfig) - if err == nil { - t.Fatalf("expected err, got nil") + if !usernameRe.MatchString(createResp.Username) { + t.Fatalf("Generated username %q did not match regex %q", createResp.Username, test.usernameRegex) } - if username != "" { - t.Fatalf("expected empty username, got [%s]", username) + + // Protect against future fields that aren't specified + expectedResp := newdbplugin.NewUserResponse{ + Username: createResp.Username, } - if password != "" { - t.Fatalf("expected empty password, got [%s]", password) + if !reflect.DeepEqual(createResp, expectedResp) { + t.Fatalf("Fields missing from expected response: Actual: %#v", createResp) } + + test.assertUser(t, connURL, createResp.Username, test.req.Password) }) } } -func TestMSSQL_SetCredentials(t *testing.T) { +func TestUpdateUser_password(t *testing.T) { type testCase struct { - rotationStmts []string + req newdbplugin.UpdateUserRequest + expectErr bool + expectedPassword string } + dbUser := "vaultuser" + initPassword := "p4$sw0rd" + tests := map[string]testCase{ + "missing password": { + req: newdbplugin.UpdateUserRequest{ + Username: dbUser, + Password: &newdbplugin.ChangePassword{ + NewPassword: "", + Statements: newdbplugin.Statements{}, + }, + }, + expectErr: true, + expectedPassword: initPassword, + }, "empty rotation statements": { - rotationStmts: []string{}, - }, "username rotation": { - rotationStmts: []string{` - ALTER LOGIN [{{username}}] WITH PASSWORD = '{{password}}';`, + req: newdbplugin.UpdateUserRequest{ + Username: dbUser, + Password: &newdbplugin.ChangePassword{ + NewPassword: "N90gkKLy8$angf", + Statements: newdbplugin.Statements{}, + }, }, + expectErr: false, + expectedPassword: "N90gkKLy8$angf", + }, + "username rotation": { + req: newdbplugin.UpdateUserRequest{ + Username: dbUser, + Password: &newdbplugin.ChangePassword{ + NewPassword: "N90gkKLy8$angf", + Statements: newdbplugin.Statements{ + Commands: []string{ + "ALTER LOGIN [{{username}}] WITH PASSWORD = '{{password}}'", + }, + }, + }, + }, + expectErr: false, + expectedPassword: "N90gkKLy8$angf", + }, + "bad statements": { + req: newdbplugin.UpdateUserRequest{ + Username: dbUser, + Password: &newdbplugin.ChangePassword{ + NewPassword: "N90gkKLy8$angf", + Statements: newdbplugin.Statements{ + Commands: []string{ + "ahosh98asjdffs", + }, + }, + }, + }, + expectErr: true, + expectedPassword: initPassword, }, } @@ -202,124 +215,101 @@ func TestMSSQL_SetCredentials(t *testing.T) { cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t) defer cleanup() - connectionDetails := map[string]interface{}{ - "connection_url": connURL, + initReq := newdbplugin.InitializeRequest{ + Config: map[string]interface{}{ + "connection_url": connURL, + }, + VerifyConnection: true, } db := new() + dbtesting.AssertInitialize(t, db, initReq) + defer dbtesting.AssertClose(t, db) - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - _, err := db.Init(ctx, connectionDetails, true) - if err != nil { - t.Fatalf("err: %s", err) - } - - dbUser := "vaultstatictest" - initPassword := "p4$sw0rd" createTestMSSQLUser(t, connURL, dbUser, initPassword, testMSSQLLogin) - if err := testCredsExist(t, connURL, dbUser, initPassword); err != nil { - t.Fatalf("Could not connect with initial credentials: %s", err) - } - - statements := dbplugin.Statements{ - Rotation: test.rotationStmts, - } - - newPassword, err := db.GenerateCredentials(context.Background()) - if err != nil { - t.Fatal(err) - } + assertCredsExist(t, connURL, dbUser, initPassword) - usernameConfig := dbplugin.StaticUserConfig{ - Username: dbUser, - Password: newPassword, - } - - username, password, err := db.SetCredentials(ctx, statements, usernameConfig) - if err != nil { - t.Fatalf("err: %s", err) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + updateResp, err := db.UpdateUser(ctx, test.req) + if test.expectErr && err == nil { + t.Fatalf("err expected, got nil") } - - if err := testCredsExist(t, connURL, username, password); err != nil { - t.Fatalf("Could not connect with new credentials: %s", err) + if !test.expectErr && err != nil { + t.Fatalf("no error expected, got: %s", err) } - if err := testCredsExist(t, connURL, username, initPassword); err == nil { - t.Fatalf("Should not be able to connect with initial credentials") + // Protect against future fields that aren't specified + expectedResp := newdbplugin.UpdateUserResponse{} + if !reflect.DeepEqual(updateResp, expectedResp) { + t.Fatalf("Fields missing from expected response: Actual: %#v", updateResp) } + assertCredsExist(t, connURL, dbUser, test.expectedPassword) }) } - } -func TestMSSQL_RevokeUser(t *testing.T) { +func TestDeleteUser(t *testing.T) { cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t) defer cleanup() - connectionDetails := map[string]interface{}{ - "connection_url": connURL, - } + dbUser := "vaultuser" + initPassword := "p4$sw0rd" - db := new() - _, err := db.Init(context.Background(), connectionDetails, true) - if err != nil { - t.Fatalf("err: %s", err) + initReq := newdbplugin.InitializeRequest{ + Config: map[string]interface{}{ + "connection_url": connURL, + }, + VerifyConnection: true, } - statements := dbplugin.Statements{ - Creation: []string{testMSSQLRole}, - } + db := new() + dbtesting.AssertInitialize(t, db, initReq) + defer dbtesting.AssertClose(t, db) - usernameConfig := dbplugin.UsernameConfig{ - DisplayName: "test", - RoleName: "test", - } + createTestMSSQLUser(t, connURL, dbUser, initPassword, testMSSQLLogin) - username, password, err := db.CreateUser(context.Background(), statements, usernameConfig, time.Now().Add(2*time.Second)) - if err != nil { - t.Fatalf("err: %s", err) - } + assertCredsExist(t, connURL, dbUser, initPassword) - if err = testCredsExist(t, connURL, username, password); err != nil { - t.Fatalf("Could not connect with new credentials: %s", err) + deleteReq := newdbplugin.DeleteUserRequest{ + Username: dbUser, } - // Test default revoke statements - err = db.RevokeUser(context.Background(), statements, username) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + deleteResp, err := db.DeleteUser(ctx, deleteReq) if err != nil { - t.Fatalf("err: %s", err) + t.Fatalf("Failed to delete user: %s", err) } - if err := testCredsExist(t, connURL, username, password); err == nil { - t.Fatal("Credentials were not revoked") + // Protect against future fields that aren't specified + expectedResp := newdbplugin.DeleteUserResponse{} + if !reflect.DeepEqual(deleteResp, expectedResp) { + t.Fatalf("Fields missing from expected response: Actual: %#v", deleteResp) } - username, password, err = db.CreateUser(context.Background(), statements, usernameConfig, time.Now().Add(2*time.Second)) - if err != nil { - t.Fatalf("err: %s", err) - } - - if err = testCredsExist(t, connURL, username, password); err != nil { - t.Fatalf("Could not connect with new credentials: %s", err) - } + assertCredsDoNotExist(t, connURL, dbUser, initPassword) +} - // Test custom revoke statement - statements.Revocation = []string{testMSSQLDrop} - err = db.RevokeUser(context.Background(), statements, username) +func assertCredsExist(t testing.TB, connURL, username, password string) { + t.Helper() + err := testCredsExist(connURL, username, password) if err != nil { - t.Fatalf("err: %s", err) + t.Fatalf("Unable to log in as %q: %s", username, err) } +} - if err := testCredsExist(t, connURL, username, password); err == nil { - t.Fatal("Credentials were not revoked") +func assertCredsDoNotExist(t testing.TB, connURL, username, password string) { + t.Helper() + err := testCredsExist(connURL, username, password) + if err == nil { + t.Fatalf("Able to log in when it shouldn't") } } -func testCredsExist(t testing.TB, connURL, username, password string) error { +func testCredsExist(connURL, username, password string) error { // Log in with the new creds parts := strings.Split(connURL, "@") connURL = fmt.Sprintf("sqlserver://%s:%s@%s", username, password, parts[1]) @@ -332,7 +322,6 @@ func testCredsExist(t testing.TB, connURL, username, password string) error { } func createTestMSSQLUser(t *testing.T, connURL string, username, password, query string) { - db, err := sql.Open("mssql", connURL) defer db.Close() if err != nil { From 73c139f148cb2a42b652ff345dd6420e2095f73c Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Fri, 9 Oct 2020 16:52:06 -0600 Subject: [PATCH 2/3] Remove debug statements --- plugins/database/mssql/mssql.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/plugins/database/mssql/mssql.go b/plugins/database/mssql/mssql.go index d290242d3f47..ea4f58c9ef5a 100644 --- a/plugins/database/mssql/mssql.go +++ b/plugins/database/mssql/mssql.go @@ -18,7 +18,6 @@ import ( "github.com/hashicorp/vault/sdk/database/newdbplugin" "github.com/hashicorp/vault/sdk/helper/dbtxn" "github.com/hashicorp/vault/sdk/helper/strutil" - "github.com/ryboe/q" ) const msSQLTypeName = "mssql" @@ -302,7 +301,6 @@ func (m *MSSQL) UpdateUser(ctx context.Context, req newdbplugin.UpdateUserReques } func (m *MSSQL) updateUserPass(ctx context.Context, username string, changePass *newdbplugin.ChangePassword) error { - q.Q("Updating user", username, changePass) stmts := changePass.Statements.Commands if len(stmts) == 0 { stmts = []string{alterLoginSQL} @@ -311,10 +309,8 @@ func (m *MSSQL) updateUserPass(ctx context.Context, username string, changePass password := changePass.NewPassword if username == "" || password == "" { - q.Q("username or password is empty") return errors.New("must provide both username and password") } - q.Q("changing password now...") m.Lock() defer m.Unlock() From c71a247ddd38d64544a00b6367ff9c64e12078be Mon Sep 17 00:00:00 2001 From: Michael Golowka <72365+pcman312@users.noreply.github.com> Date: Tue, 13 Oct 2020 10:43:23 -0600 Subject: [PATCH 3/3] Remove legacy dbplugin package; check if changes are requested --- plugins/database/mssql/mssql.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/database/mssql/mssql.go b/plugins/database/mssql/mssql.go index ea4f58c9ef5a..4d0e466071d8 100644 --- a/plugins/database/mssql/mssql.go +++ b/plugins/database/mssql/mssql.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/errwrap" multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/vault/api" - "github.com/hashicorp/vault/sdk/database/dbplugin" "github.com/hashicorp/vault/sdk/database/helper/connutil" "github.com/hashicorp/vault/sdk/database/helper/credsutil" "github.com/hashicorp/vault/sdk/database/helper/dbutil" @@ -53,7 +52,7 @@ func Run(apiTLSConfig *api.TLSConfig) error { return err } - dbplugin.Serve(dbType.(dbplugin.Database), api.VaultPluginTLSProvider(apiTLSConfig)) + newdbplugin.Serve(dbType.(newdbplugin.Database), api.VaultPluginTLSProvider(apiTLSConfig)) return nil } @@ -292,6 +291,9 @@ func (m *MSSQL) revokeUserDefault(ctx context.Context, username string) error { } func (m *MSSQL) UpdateUser(ctx context.Context, req newdbplugin.UpdateUserRequest) (newdbplugin.UpdateUserResponse, error) { + if req.Password == nil && req.Expiration == nil { + return newdbplugin.UpdateUserResponse{}, fmt.Errorf("no changes requested") + } if req.Password != nil { err := m.updateUserPass(ctx, req.Username, req.Password) return newdbplugin.UpdateUserResponse{}, err