From b901c6e7686b3716f1c9c1f47d7a4c3887835a7a Mon Sep 17 00:00:00 2001 From: Lauren Voswinkel Date: Tue, 13 Oct 2020 17:05:40 -0700 Subject: [PATCH 1/8] Update HanaDb to v5 dbplugin --- plugins/database/hana/hana.go | 157 +++++++++++++++-------------- plugins/database/hana/hana_test.go | 102 +++++++++++++------ 2 files changed, 152 insertions(+), 107 deletions(-) diff --git a/plugins/database/hana/hana.go b/plugins/database/hana/hana.go index a2af57526785..ab104cdcce4e 100644 --- a/plugins/database/hana/hana.go +++ b/plugins/database/hana/hana.go @@ -3,38 +3,38 @@ package hana import ( "context" "database/sql" - "errors" "fmt" "strings" - "time" - _ "github.com/SAP/go-hdb/driver" + "github.com/hashicorp/errwrap" "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" + "github.com/hashicorp/vault/sdk/database/newdbplugin" "github.com/hashicorp/vault/sdk/helper/dbtxn" "github.com/hashicorp/vault/sdk/helper/strutil" + + _ "github.com/SAP/go-hdb/driver" ) const ( - hanaTypeName = "hdb" + hanaTypeName = "hdb" + maxIdentifierLength = 127 ) // HANA is an implementation of Database interface type HANA struct { *connutil.SQLConnectionProducer - credsutil.CredentialsProducer } -var _ dbplugin.Database = &HANA{} +var _ newdbplugin.Database = &HANA{} // New implements builtinplugins.BuiltinFactory 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 } @@ -43,19 +43,28 @@ func new() *HANA { connProducer := &connutil.SQLConnectionProducer{} connProducer.Type = hanaTypeName - credsProducer := &credsutil.SQLCredentialsProducer{ - DisplayNameLen: 32, - RoleNameLen: 20, - UsernameLen: 128, - Separator: "_", - } - return &HANA{ SQLConnectionProducer: connProducer, - CredentialsProducer: credsProducer, } } +func (h *HANA) secretValues() map[string]string { + return map[string]string{ + h.Password: "[password]", + } +} + +func (h *HANA) Initialize(ctx context.Context, req newdbplugin.InitializeRequest) (newdbplugin.InitializeResponse, error) { + conf, err := h.Init(ctx, req.Config, req.VerifyConnection) + if err != nil { + return newdbplugin.InitializeResponse{}, errwrap.Wrapf("error initializing db: %s", err) + } + + return newdbplugin.InitializeResponse{ + Config: conf, + }, nil +} + // Run instantiates a HANA object, and runs the RPC server for the plugin func Run(apiTLSConfig *api.TLSConfig) error { dbType, err := New() @@ -63,7 +72,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 } @@ -84,59 +93,55 @@ func (h *HANA) getConnection(ctx context.Context) (*sql.DB, error) { // CreateUser generates the username/password on the underlying HANA secret backend // as instructed by the CreationStatement provided. -func (h *HANA) CreateUser(ctx context.Context, statements dbplugin.Statements, usernameConfig dbplugin.UsernameConfig, expiration time.Time) (username string, password string, err error) { +func (h *HANA) NewUser(ctx context.Context, req newdbplugin.NewUserRequest) (response newdbplugin.NewUserResponse, err error) { // Grab the lock h.Lock() defer h.Unlock() - statements = dbutil.StatementCompatibilityHelper(statements) - // Get the connection db, err := h.getConnection(ctx) if err != nil { - return "", "", err + return newdbplugin.NewUserResponse{}, err } - if len(statements.Creation) == 0 { - return "", "", dbutil.ErrEmptyCreationStatement + if len(req.Statements.Commands) == 0 { + return newdbplugin.NewUserResponse{}, dbutil.ErrEmptyCreationStatement } + dispName := credsutil.DisplayName(req.UsernameConfig.DisplayName, maxIdentifierLength) + roleName := credsutil.RoleName(req.UsernameConfig.RoleName, 20) + maxLen := credsutil.MaxLength(maxIdentifierLength) + separator := credsutil.Separator("_") + caps := credsutil.ToUpper() + // Generate username - username, err = h.GenerateUsername(usernameConfig) + username, err := credsutil.GenerateUsername(dispName, roleName, maxLen, separator, caps) if err != nil { - return "", "", err + return newdbplugin.NewUserResponse{}, err } // HANA does not allow hyphens in usernames, and highly prefers capital letters username = strings.Replace(username, "-", "_", -1) username = strings.ToUpper(username) - // Generate password - password, err = h.GeneratePassword() - if err != nil { - return "", "", err - } // Most HANA configurations have password constraints // Prefix with A1a to satisfy these constraints. User will be forced to change upon login + password := req.Password password = strings.Replace(password, "-", "_", -1) - password = "A1a" + password // If expiration is in the role SQL, HANA will deactivate the user when time is up, // regardless of whether vault is alive to revoke lease - expirationStr, err := h.GenerateExpiration(expiration) - if err != nil { - return "", "", err - } + expirationStr := req.Expiration.UTC().Format("2006-01-02 15:04:05") // 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 { @@ -148,86 +153,89 @@ func (h *HANA) CreateUser(ctx context.Context, statements dbplugin.Statements, u "password": 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, + } + + return resp, nil } // Renewing hana user just means altering user's valid until property -func (h *HANA) RenewUser(ctx context.Context, statements dbplugin.Statements, username string, expiration time.Time) error { - statements = dbutil.StatementCompatibilityHelper(statements) +func (h *HANA) UpdateUser(ctx context.Context, req newdbplugin.UpdateUserRequest) (newdbplugin.UpdateUserResponse, error) { // Get connection db, err := h.getConnection(ctx) if err != nil { - return err + return newdbplugin.UpdateUserResponse{}, err } // Start a transaction tx, err := db.BeginTx(ctx, nil) if err != nil { - return err + return newdbplugin.UpdateUserResponse{}, err } defer tx.Rollback() // If expiration is in the role SQL, HANA will deactivate the user when time is up, // regardless of whether vault is alive to revoke lease - expirationStr, err := h.GenerateExpiration(expiration) + expirationStr := req.Expiration.NewExpiration.String() if err != nil { - return err + return newdbplugin.UpdateUserResponse{}, err } // Renew user's valid until property field - stmt, err := tx.PrepareContext(ctx, "ALTER USER "+username+" VALID UNTIL "+"'"+expirationStr+"'") + stmt, err := tx.PrepareContext(ctx, "ALTER USER "+req.Username+" VALID UNTIL "+"'"+expirationStr+"'") if err != nil { - return err + return newdbplugin.UpdateUserResponse{}, err } defer stmt.Close() if _, err := stmt.ExecContext(ctx); err != nil { - return err + return newdbplugin.UpdateUserResponse{}, err } // Commit the transaction if err := tx.Commit(); err != nil { - return err + return newdbplugin.UpdateUserResponse{}, err } - return nil + return newdbplugin.UpdateUserResponse{}, nil } // Revoking hana user will deactivate user and try to perform a soft drop -func (h *HANA) RevokeUser(ctx context.Context, statements dbplugin.Statements, username string) error { - statements = dbutil.StatementCompatibilityHelper(statements) +func (h *HANA) DeleteUser(ctx context.Context, req newdbplugin.DeleteUserRequest) (newdbplugin.DeleteUserResponse, error) { // default revoke will be a soft drop on user - if len(statements.Revocation) == 0 { - return h.revokeUserDefault(ctx, username) + if len(req.Statements.Commands) == 0 { + return h.revokeUserDefault(ctx, req) } // Get connection db, err := h.getConnection(ctx) if err != nil { - return err + return newdbplugin.DeleteUserResponse{}, err } // Start a transaction tx, err := db.BeginTx(ctx, nil) if err != nil { - return err + return newdbplugin.DeleteUserResponse{}, err } defer tx.Rollback() // 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 { @@ -235,61 +243,56 @@ func (h *HANA) RevokeUser(ctx context.Context, statements dbplugin.Statements, u } m := map[string]string{ - "name": username, + "name": req.Username, } if err := dbtxn.ExecuteTxQuery(ctx, tx, m, query); err != nil { - return err + return newdbplugin.DeleteUserResponse{}, err } } } - return tx.Commit() + return newdbplugin.DeleteUserResponse{}, tx.Commit() } -func (h *HANA) revokeUserDefault(ctx context.Context, username string) error { +func (h *HANA) revokeUserDefault(ctx context.Context, req newdbplugin.DeleteUserRequest) (newdbplugin.DeleteUserResponse, error) { // Get connection db, err := h.getConnection(ctx) if err != nil { - return err + return newdbplugin.DeleteUserResponse{}, err } // Start a transaction tx, err := db.BeginTx(ctx, nil) if err != nil { - return err + return newdbplugin.DeleteUserResponse{}, err } defer tx.Rollback() // Disable server login for user - disableStmt, err := tx.PrepareContext(ctx, fmt.Sprintf("ALTER USER %s DEACTIVATE USER NOW", username)) + disableStmt, err := tx.PrepareContext(ctx, fmt.Sprintf("ALTER USER %s DEACTIVATE USER NOW", req.Username)) if err != nil { - return err + return newdbplugin.DeleteUserResponse{}, err } defer disableStmt.Close() if _, err := disableStmt.ExecContext(ctx); err != nil { - return err + return newdbplugin.DeleteUserResponse{}, err } // Invalidates current sessions and performs soft drop (drop if no dependencies) // if hard drop is desired, custom revoke statements should be written for role - dropStmt, err := tx.PrepareContext(ctx, fmt.Sprintf("DROP USER %s RESTRICT", username)) + dropStmt, err := tx.PrepareContext(ctx, fmt.Sprintf("DROP USER %s RESTRICT", req.Username)) if err != nil { - return err + return newdbplugin.DeleteUserResponse{}, err } defer dropStmt.Close() if _, err := dropStmt.ExecContext(ctx); err != nil { - return err + return newdbplugin.DeleteUserResponse{}, err } // Commit transaction if err := tx.Commit(); err != nil { - return err + return newdbplugin.DeleteUserResponse{}, err } - return nil -} - -// RotateRootCredentials is not currently supported on HANA -func (h *HANA) RotateRootCredentials(ctx context.Context, statements []string) (map[string]interface{}, error) { - return nil, errors.New("root credentaion rotation is not currently implemented in this database secrets engine") + return newdbplugin.DeleteUserResponse{}, nil } diff --git a/plugins/database/hana/hana_test.go b/plugins/database/hana/hana_test.go index bcb7a9e19950..42ba2af7b96e 100644 --- a/plugins/database/hana/hana_test.go +++ b/plugins/database/hana/hana_test.go @@ -9,7 +9,8 @@ import ( "testing" "time" - "github.com/hashicorp/vault/sdk/database/dbplugin" + "github.com/hashicorp/vault/sdk/database/helper/credsutil" + "github.com/hashicorp/vault/sdk/database/newdbplugin" ) func TestHANA_Initialize(t *testing.T) { @@ -22,8 +23,13 @@ func TestHANA_Initialize(t *testing.T) { "connection_url": connURL, } + initReq := newdbplugin.InitializeRequest{ + Config: connectionDetails, + VerifyConnection: true, + } + db := new() - _, err := db.Init(context.Background(), connectionDetails, true) + _, err := db.Initialize(context.Background(), initReq) if err != nil { t.Fatalf("err: %s", err) } @@ -39,7 +45,7 @@ func TestHANA_Initialize(t *testing.T) { } // this test will leave a lingering user on the system -func TestHANA_CreateUser(t *testing.T) { +func TestHANA_NewUser(t *testing.T) { if os.Getenv("HANA_URL") == "" || os.Getenv("VAULT_ACC") != "1" { t.SkipNow() } @@ -49,38 +55,55 @@ func TestHANA_CreateUser(t *testing.T) { "connection_url": connURL, } + initReq := newdbplugin.InitializeRequest{ + Config: connectionDetails, + VerifyConnection: true, + } + db := new() - _, err := db.Init(context.Background(), connectionDetails, true) + _, err := db.Initialize(context.Background(), initReq) if err != nil { t.Fatalf("err: %s", err) } - usernameConfig := dbplugin.UsernameConfig{ - DisplayName: "test-test", - RoleName: "test-test", + password, err := credsutil.RandomAlphaNumeric(32, true) + if err != nil { + t.Fatalf("failed to generate password: %s", err) + } + password = strings.Replace(password, "-", "_", -1) + + req := newdbplugin.NewUserRequest{ + UsernameConfig: newdbplugin.UsernameMetadata{ + DisplayName: "test-test", + RoleName: "test-test", + }, + Statements: newdbplugin.Statements{ + Commands: []string{}, + }, + Password: password, + Expiration: time.Now().Add(time.Hour), } // Test with no configured Creation Statement - _, _, err = db.CreateUser(context.Background(), dbplugin.Statements{}, usernameConfig, time.Now().Add(time.Hour)) + userResp, err := db.NewUser(context.Background(), req) if err == nil { t.Fatal("Expected error when no creation statement is provided") } - statements := dbplugin.Statements{ - Creation: []string{testHANARole}, - } + // Add a statement command + req.Statements.Commands = []string{testHANARole} - username, password, err := db.CreateUser(context.Background(), statements, usernameConfig, time.Now().Add(time.Hour)) + userResp, err = db.NewUser(context.Background(), req) if err != nil { t.Fatalf("err: %s", err) } - if err = testCredsExist(t, connURL, username, password); err != nil { + if err = testCredsExist(t, connURL, userResp.Username, password); err != nil { t.Fatalf("Could not connect with new credentials: %s", err) } } -func TestHANA_RevokeUser(t *testing.T) { +func TestHANA_DeleteUser(t *testing.T) { if os.Getenv("HANA_URL") == "" || os.Getenv("VAULT_ACC") != "1" { t.SkipNow() } @@ -90,53 +113,72 @@ func TestHANA_RevokeUser(t *testing.T) { "connection_url": connURL, } + initReq := newdbplugin.InitializeRequest{ + Config: connectionDetails, + VerifyConnection: true, + } + db := new() - _, err := db.Init(context.Background(), connectionDetails, true) + _, err := db.Initialize(context.Background(), initReq) if err != nil { t.Fatalf("err: %s", err) } - statements := dbplugin.Statements{ - Creation: []string{testHANARole}, + password, err := credsutil.RandomAlphaNumeric(32, true) + if err != nil { + t.Fatalf("failed to generate password: %s", err) } + password = strings.Replace(password, "-", "_", -1) - usernameConfig := dbplugin.UsernameConfig{ - DisplayName: "test-test", - RoleName: "test-test", + newReq := newdbplugin.NewUserRequest{ + UsernameConfig: newdbplugin.UsernameMetadata{ + DisplayName: "test-test", + RoleName: "test-test", + }, + Password: password, + Statements: newdbplugin.Statements{ + Commands: []string{testHANARole}, + }, + Expiration: time.Now().Add(time.Hour), } // Test default revoke statements - username, password, err := db.CreateUser(context.Background(), statements, usernameConfig, time.Now().Add(time.Hour)) + userResp, err := db.NewUser(context.Background(), newReq) if err != nil { t.Fatalf("err: %s", err) } - if err = testCredsExist(t, connURL, username, password); err != nil { + if err = testCredsExist(t, connURL, userResp.Username, password); err != nil { t.Fatalf("Could not connect with new credentials: %s", err) } - err = db.RevokeUser(context.Background(), statements, username) + delReq := newdbplugin.DeleteUserRequest{ + Username: userResp.Username, + } + + _, err = db.DeleteUser(context.Background(), delReq) if err != nil { t.Fatalf("err: %s", err) } - if err := testCredsExist(t, connURL, username, password); err == nil { + if err := testCredsExist(t, connURL, userResp.Username, password); err == nil { t.Fatal("Credentials were not revoked") } // Test custom revoke statement - username, password, err = db.CreateUser(context.Background(), statements, usernameConfig, time.Now().Add(time.Hour)) + userResp, err = db.NewUser(context.Background(), newReq) if err != nil { t.Fatalf("err: %s", err) } - if err = testCredsExist(t, connURL, username, password); err != nil { + if err = testCredsExist(t, connURL, userResp.Username, password); err != nil { t.Fatalf("Could not connect with new credentials: %s", err) } - statements.Revocation = []string{testHANADrop} - err = db.RevokeUser(context.Background(), statements, username) + delReq.Statements.Commands = []string{testHANADrop} + delReq.Username = userResp.Username + _, err = db.DeleteUser(context.Background(), delReq) if err != nil { t.Fatalf("err: %s", err) } - if err := testCredsExist(t, connURL, username, password); err == nil { + if err := testCredsExist(t, connURL, userResp.Username, password); err == nil { t.Fatal("Credentials were not revoked") } } @@ -154,7 +196,7 @@ func testCredsExist(t testing.TB, connURL, username, password string) error { } const testHANARole = ` -CREATE USER {{name}} PASSWORD {{password}} VALID UNTIL '{{expiration}}';` +CREATE USER {{name}} PASSWORD {{password}} NO FORCE_FIRST_PASSWORD_CHANGE VALID UNTIL '{{expiration}}';` const testHANADrop = ` DROP USER {{name}} CASCADE;` From 784cfe6f70c692aaa67f706244ea1b298746422a Mon Sep 17 00:00:00 2001 From: Lauren Voswinkel Date: Wed, 14 Oct 2020 14:52:51 -0700 Subject: [PATCH 2/8] Some small updates to fix nits --- plugins/database/hana/hana.go | 39 ++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/plugins/database/hana/hana.go b/plugins/database/hana/hana.go index ab104cdcce4e..2af3416c397f 100644 --- a/plugins/database/hana/hana.go +++ b/plugins/database/hana/hana.go @@ -6,7 +6,6 @@ import ( "fmt" "strings" - "github.com/hashicorp/errwrap" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/sdk/database/helper/connutil" "github.com/hashicorp/vault/sdk/database/helper/credsutil" @@ -57,7 +56,7 @@ func (h *HANA) secretValues() map[string]string { func (h *HANA) Initialize(ctx context.Context, req newdbplugin.InitializeRequest) (newdbplugin.InitializeResponse, error) { conf, err := h.Init(ctx, req.Config, req.VerifyConnection) if err != nil { - return newdbplugin.InitializeResponse{}, errwrap.Wrapf("error initializing db: %s", err) + return newdbplugin.InitializeResponse{}, fmt.Errorf("error initializing db: %s", err) } return newdbplugin.InitializeResponse{ @@ -108,7 +107,7 @@ func (h *HANA) NewUser(ctx context.Context, req newdbplugin.NewUserRequest) (res return newdbplugin.NewUserResponse{}, dbutil.ErrEmptyCreationStatement } - dispName := credsutil.DisplayName(req.UsernameConfig.DisplayName, maxIdentifierLength) + dispName := credsutil.DisplayName(req.UsernameConfig.DisplayName, 32) roleName := credsutil.RoleName(req.UsernameConfig.RoleName, 20) maxLen := credsutil.MaxLength(maxIdentifierLength) separator := credsutil.Separator("_") @@ -174,6 +173,10 @@ func (h *HANA) NewUser(ctx context.Context, req newdbplugin.NewUserRequest) (res // Renewing hana user just means altering user's valid until property func (h *HANA) UpdateUser(ctx context.Context, req newdbplugin.UpdateUserRequest) (newdbplugin.UpdateUserResponse, error) { + // No change requested + if req.Password == nil && req.Expiration == nil { + return newdbplugin.UpdateUserResponse{}, nil + } // Get connection db, err := h.getConnection(ctx) @@ -188,21 +191,23 @@ func (h *HANA) UpdateUser(ctx context.Context, req newdbplugin.UpdateUserRequest } defer tx.Rollback() - // If expiration is in the role SQL, HANA will deactivate the user when time is up, - // regardless of whether vault is alive to revoke lease - expirationStr := req.Expiration.NewExpiration.String() - if err != nil { - return newdbplugin.UpdateUserResponse{}, err - } + if req.Expiration != nil { + // If expiration is in the role SQL, HANA will deactivate the user when time is up, + // regardless of whether vault is alive to revoke lease + expirationStr := req.Expiration.NewExpiration.String() + if err != nil { + return newdbplugin.UpdateUserResponse{}, err + } - // Renew user's valid until property field - stmt, err := tx.PrepareContext(ctx, "ALTER USER "+req.Username+" VALID UNTIL "+"'"+expirationStr+"'") - if err != nil { - return newdbplugin.UpdateUserResponse{}, err - } - defer stmt.Close() - if _, err := stmt.ExecContext(ctx); err != nil { - return newdbplugin.UpdateUserResponse{}, err + // Renew user's valid until property field + stmt, err := tx.PrepareContext(ctx, "ALTER USER "+req.Username+" VALID UNTIL "+"'"+expirationStr+"'") + if err != nil { + return newdbplugin.UpdateUserResponse{}, err + } + defer stmt.Close() + if _, err := stmt.ExecContext(ctx); err != nil { + return newdbplugin.UpdateUserResponse{}, err + } } // Commit the transaction From d4883791a29f63be21262bb7d396d16249184cef Mon Sep 17 00:00:00 2001 From: Lauren Voswinkel Date: Wed, 14 Oct 2020 16:35:36 -0700 Subject: [PATCH 3/8] Add ability to update passwords for HANA db --- plugins/database/hana/hana.go | 90 +++++++++++++++++++++++++---- plugins/database/hana/hana_test.go | 93 ++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 10 deletions(-) diff --git a/plugins/database/hana/hana.go b/plugins/database/hana/hana.go index 2af3416c397f..3bbda5b97054 100644 --- a/plugins/database/hana/hana.go +++ b/plugins/database/hana/hana.go @@ -173,6 +173,9 @@ func (h *HANA) NewUser(ctx context.Context, req newdbplugin.NewUserRequest) (res // Renewing hana user just means altering user's valid until property func (h *HANA) UpdateUser(ctx context.Context, req newdbplugin.UpdateUserRequest) (newdbplugin.UpdateUserResponse, error) { + h.Lock() + defer h.Unlock() + // No change requested if req.Password == nil && req.Expiration == nil { return newdbplugin.UpdateUserResponse{}, nil @@ -191,23 +194,18 @@ func (h *HANA) UpdateUser(ctx context.Context, req newdbplugin.UpdateUserRequest } defer tx.Rollback() - if req.Expiration != nil { - // If expiration is in the role SQL, HANA will deactivate the user when time is up, - // regardless of whether vault is alive to revoke lease - expirationStr := req.Expiration.NewExpiration.String() + if req.Password != nil { + err = h.updateUserPassword(ctx, tx, req.Username, req.Password) if err != nil { return newdbplugin.UpdateUserResponse{}, err } + } - // Renew user's valid until property field - stmt, err := tx.PrepareContext(ctx, "ALTER USER "+req.Username+" VALID UNTIL "+"'"+expirationStr+"'") + if req.Expiration != nil { + err = h.updateUserExpiration(ctx, tx, req.Username, req.Expiration) if err != nil { return newdbplugin.UpdateUserResponse{}, err } - defer stmt.Close() - if _, err := stmt.ExecContext(ctx); err != nil { - return newdbplugin.UpdateUserResponse{}, err - } } // Commit the transaction @@ -218,8 +216,80 @@ func (h *HANA) UpdateUser(ctx context.Context, req newdbplugin.UpdateUserRequest return newdbplugin.UpdateUserResponse{}, nil } +func (h *HANA) updateUserPassword(ctx context.Context, tx *sql.Tx, username string, req *newdbplugin.ChangePassword) error { + password := req.NewPassword + + if username == "" || password == "" { + return fmt.Errorf("must provide both username and password") + } + + stmts := req.Statements.Commands + if len(stmts) == 0 { + stmts = []string{"ALTER USER {{username}} PASSWORD \"{{password}}\""} + } + + for _, stmt := range stmts { + for _, query := range strutil.ParseArbitraryStringSlice(stmt, ";") { + query = strings.TrimSpace(query) + if len(query) == 0 { + continue + } + + m := map[string]string{ + "name": username, + "username": username, + "password": password, + } + + if err := dbtxn.ExecuteTxQuery(ctx, tx, m, query); err != nil { + return fmt.Errorf("failed to execute query: %w", err) + } + } + } + + return nil +} + +func (h *HANA) updateUserExpiration(ctx context.Context, tx *sql.Tx, username string, req *newdbplugin.ChangeExpiration) error { + // If expiration is in the role SQL, HANA will deactivate the user when time is up, + // regardless of whether vault is alive to revoke lease + expirationStr := req.NewExpiration.String() + + if username == "" || expirationStr == "" { + return fmt.Errorf("must provide both username and expiration") + } + + stmts := req.Statements.Commands + if len(stmts) == 0 { + stmts = []string{"ALTER USER {{username}} VALID UNTIL '{{expiration}}'"} + } + + for _, stmt := range stmts { + for _, query := range strutil.ParseArbitraryStringSlice(stmt, ";") { + query = strings.TrimSpace(query) + if len(query) == 0 { + continue + } + + m := map[string]string{ + "name": username, + "username": username, + "expiration": expirationStr, + } + + if err := dbtxn.ExecuteTxQuery(ctx, tx, m, query); err != nil { + return fmt.Errorf("failed to execute query: %w", err) + } + } + } + + return nil +} + // Revoking hana user will deactivate user and try to perform a soft drop func (h *HANA) DeleteUser(ctx context.Context, req newdbplugin.DeleteUserRequest) (newdbplugin.DeleteUserResponse, error) { + h.Lock() + h.Unlock() // default revoke will be a soft drop on user if len(req.Statements.Commands) == 0 { diff --git a/plugins/database/hana/hana_test.go b/plugins/database/hana/hana_test.go index 42ba2af7b96e..418f7f84710e 100644 --- a/plugins/database/hana/hana_test.go +++ b/plugins/database/hana/hana_test.go @@ -103,6 +103,96 @@ func TestHANA_NewUser(t *testing.T) { } } +func TestHANA_UpdateUser(t *testing.T) { + if os.Getenv("HANA_URL") == "" || os.Getenv("VAULT_ACC") != "1" { + t.SkipNow() + } + connURL := os.Getenv("HANA_URL") + + connectionDetails := map[string]interface{}{ + "connection_url": connURL, + } + + initReq := newdbplugin.InitializeRequest{ + Config: connectionDetails, + VerifyConnection: true, + } + + db := new() + _, err := db.Initialize(context.Background(), initReq) + if err != nil { + t.Fatalf("err: %s", err) + } + + password, err := credsutil.RandomAlphaNumeric(32, true) + if err != nil { + t.Fatalf("failed to generate password: %s", err) + } + password = strings.Replace(password, "-", "_", -1) + + newReq := newdbplugin.NewUserRequest{ + UsernameConfig: newdbplugin.UsernameMetadata{ + DisplayName: "test-test", + RoleName: "test-test", + }, + Password: password, + Statements: newdbplugin.Statements{ + Commands: []string{testHANARole}, + }, + Expiration: time.Now().Add(time.Hour), + } + + // Test default revoke statements + userResp, err := db.NewUser(context.Background(), newReq) + if err != nil { + t.Fatalf("err: %s", err) + } + if err = testCredsExist(t, connURL, userResp.Username, password); err != nil { + t.Fatalf("Could not connect with new credentials: %s", err) + } + + newPassword, err := credsutil.RandomAlphaNumeric(32, true) + if err != nil { + t.Fatalf("failed to generate password: %s", err) + } + newPassword = strings.Replace(newPassword, "-", "_", -1) + + // Change Password + updateReq := newdbplugin.UpdateUserRequest{ + Username: userResp.Username, + Password: &newdbplugin.ChangePassword{ + NewPassword: newPassword, + }, + } + + _, err = db.UpdateUser(context.Background(), updateReq) + if err != nil { + t.Fatalf("err: %s", err) + } + if err := testCredsExist(t, connURL, userResp.Username, newPassword); err == nil { + t.Fatal("Credentials were not changed") + } + + // Test custom update statement + userResp, err = db.NewUser(context.Background(), newReq) + if err != nil { + t.Fatalf("err: %s", err) + } + if err = testCredsExist(t, connURL, userResp.Username, password); err != nil { + t.Fatalf("Could not connect with new credentials: %s", err) + } + + updateReq.Password.Statements.Commands = []string{testHANAUpdate} + updateReq.Username = userResp.Username + _, err = db.UpdateUser(context.Background(), updateReq) + if err != nil { + t.Fatalf("err: %s", err) + } + if err := testCredsExist(t, connURL, userResp.Username, newPassword); err == nil { + t.Fatal("Credentials were not revoked") + } +} + func TestHANA_DeleteUser(t *testing.T) { if os.Getenv("HANA_URL") == "" || os.Getenv("VAULT_ACC") != "1" { t.SkipNow() @@ -200,3 +290,6 @@ CREATE USER {{name}} PASSWORD {{password}} NO FORCE_FIRST_PASSWORD_CHANGE VALID const testHANADrop = ` DROP USER {{name}} CASCADE;` + +const testHANAUpdate = ` +ALTER USER {{name}} PASSWORD "{{password}}";` From 127b4acb4badd4f9aa7df8ef3855195d87487211 Mon Sep 17 00:00:00 2001 From: Lauren Voswinkel Date: Thu, 15 Oct 2020 13:17:59 -0700 Subject: [PATCH 4/8] Fixing up some more small issues --- plugins/database/hana/hana.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/database/hana/hana.go b/plugins/database/hana/hana.go index 3bbda5b97054..b35c7e5e02ad 100644 --- a/plugins/database/hana/hana.go +++ b/plugins/database/hana/hana.go @@ -56,7 +56,7 @@ func (h *HANA) secretValues() map[string]string { func (h *HANA) Initialize(ctx context.Context, req newdbplugin.InitializeRequest) (newdbplugin.InitializeResponse, error) { conf, err := h.Init(ctx, req.Config, req.VerifyConnection) if err != nil { - return newdbplugin.InitializeResponse{}, fmt.Errorf("error initializing db: %s", err) + return newdbplugin.InitializeResponse{}, fmt.Errorf("error initializing db: %w", err) } return newdbplugin.InitializeResponse{ @@ -289,7 +289,7 @@ func (h *HANA) updateUserExpiration(ctx context.Context, tx *sql.Tx, username st // Revoking hana user will deactivate user and try to perform a soft drop func (h *HANA) DeleteUser(ctx context.Context, req newdbplugin.DeleteUserRequest) (newdbplugin.DeleteUserResponse, error) { h.Lock() - h.Unlock() + defer h.Unlock() // default revoke will be a soft drop on user if len(req.Statements.Commands) == 0 { From 1868f091725652d66219d959cbda09e503f31f37 Mon Sep 17 00:00:00 2001 From: Lauren Voswinkel Date: Tue, 20 Oct 2020 11:58:23 -0700 Subject: [PATCH 5/8] More adjustments --- plugins/database/hana/hana.go | 78 +++--- plugins/database/hana/hana_test.go | 387 ++++++++++++++--------------- 2 files changed, 229 insertions(+), 236 deletions(-) diff --git a/plugins/database/hana/hana.go b/plugins/database/hana/hana.go index b35c7e5e02ad..b6caf8f75e41 100644 --- a/plugins/database/hana/hana.go +++ b/plugins/database/hana/hana.go @@ -7,10 +7,10 @@ import ( "strings" "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/sdk/database/dbplugin/v5" "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" @@ -27,13 +27,13 @@ type HANA struct { *connutil.SQLConnectionProducer } -var _ newdbplugin.Database = &HANA{} +var _ dbplugin.Database = &HANA{} // New implements builtinplugins.BuiltinFactory func New() (interface{}, error) { db := new() // Wrap the plugin with middleware to sanitize errors - dbType := newdbplugin.NewDatabaseErrorSanitizerMiddleware(db, db.secretValues) + dbType := dbplugin.NewDatabaseErrorSanitizerMiddleware(db, db.secretValues) return dbType, nil } @@ -53,13 +53,13 @@ func (h *HANA) secretValues() map[string]string { } } -func (h *HANA) Initialize(ctx context.Context, req newdbplugin.InitializeRequest) (newdbplugin.InitializeResponse, error) { +func (h *HANA) Initialize(ctx context.Context, req dbplugin.InitializeRequest) (dbplugin.InitializeResponse, error) { conf, err := h.Init(ctx, req.Config, req.VerifyConnection) if err != nil { - return newdbplugin.InitializeResponse{}, fmt.Errorf("error initializing db: %w", err) + return dbplugin.InitializeResponse{}, fmt.Errorf("error initializing db: %w", err) } - return newdbplugin.InitializeResponse{ + return dbplugin.InitializeResponse{ Config: conf, }, nil } @@ -71,7 +71,7 @@ func Run(apiTLSConfig *api.TLSConfig) error { return err } - newdbplugin.Serve(dbType.(newdbplugin.Database), api.VaultPluginTLSProvider(apiTLSConfig)) + dbplugin.Serve(dbType.(dbplugin.Database), api.VaultPluginTLSProvider(apiTLSConfig)) return nil } @@ -92,7 +92,7 @@ func (h *HANA) getConnection(ctx context.Context) (*sql.DB, error) { // CreateUser generates the username/password on the underlying HANA secret backend // as instructed by the CreationStatement provided. -func (h *HANA) NewUser(ctx context.Context, req newdbplugin.NewUserRequest) (response newdbplugin.NewUserResponse, err error) { +func (h *HANA) NewUser(ctx context.Context, req dbplugin.NewUserRequest) (response dbplugin.NewUserResponse, err error) { // Grab the lock h.Lock() defer h.Unlock() @@ -100,11 +100,11 @@ func (h *HANA) NewUser(ctx context.Context, req newdbplugin.NewUserRequest) (res // Get the connection db, err := h.getConnection(ctx) if err != nil { - return newdbplugin.NewUserResponse{}, err + return dbplugin.NewUserResponse{}, err } if len(req.Statements.Commands) == 0 { - return newdbplugin.NewUserResponse{}, dbutil.ErrEmptyCreationStatement + return dbplugin.NewUserResponse{}, dbutil.ErrEmptyCreationStatement } dispName := credsutil.DisplayName(req.UsernameConfig.DisplayName, 32) @@ -116,7 +116,7 @@ func (h *HANA) NewUser(ctx context.Context, req newdbplugin.NewUserRequest) (res // Generate username username, err := credsutil.GenerateUsername(dispName, roleName, maxLen, separator, caps) if err != nil { - return newdbplugin.NewUserResponse{}, err + return dbplugin.NewUserResponse{}, err } // HANA does not allow hyphens in usernames, and highly prefers capital letters @@ -135,7 +135,7 @@ func (h *HANA) NewUser(ctx context.Context, req newdbplugin.NewUserRequest) (res // Start a transaction tx, err := db.BeginTx(ctx, nil) if err != nil { - return newdbplugin.NewUserResponse{}, err + return dbplugin.NewUserResponse{}, err } defer tx.Rollback() @@ -154,17 +154,17 @@ func (h *HANA) NewUser(ctx context.Context, req newdbplugin.NewUserRequest) (res } if err := dbtxn.ExecuteTxQuery(ctx, tx, m, query); err != nil { - return newdbplugin.NewUserResponse{}, err + return dbplugin.NewUserResponse{}, err } } } // Commit the transaction if err := tx.Commit(); err != nil { - return newdbplugin.NewUserResponse{}, err + return dbplugin.NewUserResponse{}, err } - resp := newdbplugin.NewUserResponse{ + resp := dbplugin.NewUserResponse{ Username: username, } @@ -172,51 +172,51 @@ func (h *HANA) NewUser(ctx context.Context, req newdbplugin.NewUserRequest) (res } // Renewing hana user just means altering user's valid until property -func (h *HANA) UpdateUser(ctx context.Context, req newdbplugin.UpdateUserRequest) (newdbplugin.UpdateUserResponse, error) { +func (h *HANA) UpdateUser(ctx context.Context, req dbplugin.UpdateUserRequest) (dbplugin.UpdateUserResponse, error) { h.Lock() defer h.Unlock() // No change requested if req.Password == nil && req.Expiration == nil { - return newdbplugin.UpdateUserResponse{}, nil + return dbplugin.UpdateUserResponse{}, nil } // Get connection db, err := h.getConnection(ctx) if err != nil { - return newdbplugin.UpdateUserResponse{}, err + return dbplugin.UpdateUserResponse{}, err } // Start a transaction tx, err := db.BeginTx(ctx, nil) if err != nil { - return newdbplugin.UpdateUserResponse{}, err + return dbplugin.UpdateUserResponse{}, err } defer tx.Rollback() if req.Password != nil { err = h.updateUserPassword(ctx, tx, req.Username, req.Password) if err != nil { - return newdbplugin.UpdateUserResponse{}, err + return dbplugin.UpdateUserResponse{}, err } } if req.Expiration != nil { err = h.updateUserExpiration(ctx, tx, req.Username, req.Expiration) if err != nil { - return newdbplugin.UpdateUserResponse{}, err + return dbplugin.UpdateUserResponse{}, err } } // Commit the transaction if err := tx.Commit(); err != nil { - return newdbplugin.UpdateUserResponse{}, err + return dbplugin.UpdateUserResponse{}, err } - return newdbplugin.UpdateUserResponse{}, nil + return dbplugin.UpdateUserResponse{}, nil } -func (h *HANA) updateUserPassword(ctx context.Context, tx *sql.Tx, username string, req *newdbplugin.ChangePassword) error { +func (h *HANA) updateUserPassword(ctx context.Context, tx *sql.Tx, username string, req *dbplugin.ChangePassword) error { password := req.NewPassword if username == "" || password == "" { @@ -250,7 +250,7 @@ func (h *HANA) updateUserPassword(ctx context.Context, tx *sql.Tx, username stri return nil } -func (h *HANA) updateUserExpiration(ctx context.Context, tx *sql.Tx, username string, req *newdbplugin.ChangeExpiration) error { +func (h *HANA) updateUserExpiration(ctx context.Context, tx *sql.Tx, username string, req *dbplugin.ChangeExpiration) error { // If expiration is in the role SQL, HANA will deactivate the user when time is up, // regardless of whether vault is alive to revoke lease expirationStr := req.NewExpiration.String() @@ -287,7 +287,7 @@ func (h *HANA) updateUserExpiration(ctx context.Context, tx *sql.Tx, username st } // Revoking hana user will deactivate user and try to perform a soft drop -func (h *HANA) DeleteUser(ctx context.Context, req newdbplugin.DeleteUserRequest) (newdbplugin.DeleteUserResponse, error) { +func (h *HANA) DeleteUser(ctx context.Context, req dbplugin.DeleteUserRequest) (dbplugin.DeleteUserResponse, error) { h.Lock() defer h.Unlock() @@ -299,13 +299,13 @@ func (h *HANA) DeleteUser(ctx context.Context, req newdbplugin.DeleteUserRequest // Get connection db, err := h.getConnection(ctx) if err != nil { - return newdbplugin.DeleteUserResponse{}, err + return dbplugin.DeleteUserResponse{}, err } // Start a transaction tx, err := db.BeginTx(ctx, nil) if err != nil { - return newdbplugin.DeleteUserResponse{}, err + return dbplugin.DeleteUserResponse{}, err } defer tx.Rollback() @@ -321,53 +321,53 @@ func (h *HANA) DeleteUser(ctx context.Context, req newdbplugin.DeleteUserRequest "name": req.Username, } if err := dbtxn.ExecuteTxQuery(ctx, tx, m, query); err != nil { - return newdbplugin.DeleteUserResponse{}, err + return dbplugin.DeleteUserResponse{}, err } } } - return newdbplugin.DeleteUserResponse{}, tx.Commit() + return dbplugin.DeleteUserResponse{}, tx.Commit() } -func (h *HANA) revokeUserDefault(ctx context.Context, req newdbplugin.DeleteUserRequest) (newdbplugin.DeleteUserResponse, error) { +func (h *HANA) revokeUserDefault(ctx context.Context, req dbplugin.DeleteUserRequest) (dbplugin.DeleteUserResponse, error) { // Get connection db, err := h.getConnection(ctx) if err != nil { - return newdbplugin.DeleteUserResponse{}, err + return dbplugin.DeleteUserResponse{}, err } // Start a transaction tx, err := db.BeginTx(ctx, nil) if err != nil { - return newdbplugin.DeleteUserResponse{}, err + return dbplugin.DeleteUserResponse{}, err } defer tx.Rollback() // Disable server login for user disableStmt, err := tx.PrepareContext(ctx, fmt.Sprintf("ALTER USER %s DEACTIVATE USER NOW", req.Username)) if err != nil { - return newdbplugin.DeleteUserResponse{}, err + return dbplugin.DeleteUserResponse{}, err } defer disableStmt.Close() if _, err := disableStmt.ExecContext(ctx); err != nil { - return newdbplugin.DeleteUserResponse{}, err + return dbplugin.DeleteUserResponse{}, err } // Invalidates current sessions and performs soft drop (drop if no dependencies) // if hard drop is desired, custom revoke statements should be written for role dropStmt, err := tx.PrepareContext(ctx, fmt.Sprintf("DROP USER %s RESTRICT", req.Username)) if err != nil { - return newdbplugin.DeleteUserResponse{}, err + return dbplugin.DeleteUserResponse{}, err } defer dropStmt.Close() if _, err := dropStmt.ExecContext(ctx); err != nil { - return newdbplugin.DeleteUserResponse{}, err + return dbplugin.DeleteUserResponse{}, err } // Commit transaction if err := tx.Commit(); err != nil { - return newdbplugin.DeleteUserResponse{}, err + return dbplugin.DeleteUserResponse{}, err } - return newdbplugin.DeleteUserResponse{}, nil + return dbplugin.DeleteUserResponse{}, nil } diff --git a/plugins/database/hana/hana_test.go b/plugins/database/hana/hana_test.go index 418f7f84710e..30bfbb9c324f 100644 --- a/plugins/database/hana/hana_test.go +++ b/plugins/database/hana/hana_test.go @@ -4,13 +4,12 @@ import ( "context" "database/sql" "fmt" + "github.com/hashicorp/vault/sdk/database/dbplugin/v5" + dbtesting "github.com/hashicorp/vault/sdk/database/dbplugin/v5/testing" "os" "strings" "testing" "time" - - "github.com/hashicorp/vault/sdk/database/helper/credsutil" - "github.com/hashicorp/vault/sdk/database/newdbplugin" ) func TestHANA_Initialize(t *testing.T) { @@ -23,24 +22,15 @@ func TestHANA_Initialize(t *testing.T) { "connection_url": connURL, } - initReq := newdbplugin.InitializeRequest{ + initReq := dbplugin.InitializeRequest{ Config: connectionDetails, VerifyConnection: true, } db := new() - _, err := db.Initialize(context.Background(), initReq) - 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) + initResp := dbtesting.AssertInitialize(t, db, initReq) + if initResp.Config == nil { + t.Fatalf("config not set by initialization") } } @@ -49,57 +39,71 @@ func TestHANA_NewUser(t *testing.T) { if os.Getenv("HANA_URL") == "" || os.Getenv("VAULT_ACC") != "1" { t.SkipNow() } - connURL := os.Getenv("HANA_URL") - - connectionDetails := map[string]interface{}{ - "connection_url": connURL, - } - initReq := newdbplugin.InitializeRequest{ - Config: connectionDetails, - VerifyConnection: true, - } - - db := new() - _, err := db.Initialize(context.Background(), initReq) - if err != nil { - t.Fatalf("err: %s", err) - } - - password, err := credsutil.RandomAlphaNumeric(32, true) - if err != nil { - t.Fatalf("failed to generate password: %s", err) - } - password = strings.Replace(password, "-", "_", -1) + connURL := os.Getenv("HANA_URL") - req := newdbplugin.NewUserRequest{ - UsernameConfig: newdbplugin.UsernameMetadata{ - DisplayName: "test-test", - RoleName: "test-test", + type testCase struct { + req dbplugin.NewUserRequest + expectErr bool + assertUser func(t testing.TB, connURL, username, password string) + } + + tests := map[string]testCase{ + "no creation statements": { + req: dbplugin.NewUserRequest{ + UsernameConfig: dbplugin.UsernameMetadata{ + DisplayName: "test-test", + RoleName: "test-test", + }, + Statements: dbplugin.Statements{}, + Password: "AG4qagho_dsvZ", + Expiration: time.Now().Add(1 * time.Second), + }, + expectErr: true, + assertUser: assertCredsDoNotExist, }, - Statements: newdbplugin.Statements{ - Commands: []string{}, + "with creation statements": { + req: dbplugin.NewUserRequest{ + UsernameConfig: dbplugin.UsernameMetadata{ + DisplayName: "test-test", + RoleName: "test-test", + }, + Statements: dbplugin.Statements{ + Commands: []string{testHANARole}, + }, + Password: "AG4qagho_dsvZ", + Expiration: time.Now().Add(1 * time.Second), + }, + expectErr: false, + assertUser: assertCredsExist, }, - Password: password, - Expiration: time.Now().Add(time.Hour), } - // Test with no configured Creation Statement - userResp, err := db.NewUser(context.Background(), req) - if err == nil { - t.Fatal("Expected error when no creation statement is provided") - } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + connectionDetails := map[string]interface{}{ + "connection_url": connURL, + } - // Add a statement command - req.Statements.Commands = []string{testHANARole} + initReq := dbplugin.InitializeRequest{ + Config: connectionDetails, + VerifyConnection: true, + } - userResp, err = db.NewUser(context.Background(), req) - if err != nil { - t.Fatalf("err: %s", err) - } + 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, received nil") + } + if !test.expectErr && err != nil { + t.Fatalf("no error expected, got: %s", err) + } - if err = testCredsExist(t, connURL, userResp.Username, password); err != nil { - t.Fatalf("Could not connect with new credentials: %s", err) + test.assertUser(t, connURL, createResp.Username, test.req.Password) + }) } } @@ -109,87 +113,77 @@ func TestHANA_UpdateUser(t *testing.T) { } connURL := os.Getenv("HANA_URL") - connectionDetails := map[string]interface{}{ - "connection_url": connURL, - } - - initReq := newdbplugin.InitializeRequest{ - Config: connectionDetails, - VerifyConnection: true, - } - - db := new() - _, err := db.Initialize(context.Background(), initReq) - if err != nil { - t.Fatalf("err: %s", err) - } - - password, err := credsutil.RandomAlphaNumeric(32, true) - if err != nil { - t.Fatalf("failed to generate password: %s", err) - } - password = strings.Replace(password, "-", "_", -1) - - newReq := newdbplugin.NewUserRequest{ - UsernameConfig: newdbplugin.UsernameMetadata{ - DisplayName: "test-test", - RoleName: "test-test", + type testCase struct { + req dbplugin.UpdateUserRequest + startingPassword string + expectErrOnLogin bool + } + + tests := map[string]testCase{ + "no update statements": { + req: dbplugin.UpdateUserRequest{ + Password: &dbplugin.ChangePassword{ + NewPassword: "this_is_ALSO_Thirty_2_characters_", + }, + }, + startingPassword: "this_is_Thirty_2_characters_wow_", + expectErrOnLogin: true, }, - Password: password, - Statements: newdbplugin.Statements{ - Commands: []string{testHANARole}, + "with custom update statements": { + req: dbplugin.UpdateUserRequest{ + Password: &dbplugin.ChangePassword{ + NewPassword: "this_is_ALSO_Thirty_2_characters_", + Statements: dbplugin.Statements{ + Commands: []string{testHANAUpdate}, + }, + }, + }, + startingPassword: "this_is_Thirty_2_characters_wow_", + expectErrOnLogin: false, }, - Expiration: time.Now().Add(time.Hour), } - // Test default revoke statements - userResp, err := db.NewUser(context.Background(), newReq) - if err != nil { - t.Fatalf("err: %s", err) - } - if err = testCredsExist(t, connURL, userResp.Username, password); err != nil { - t.Fatalf("Could not connect with new credentials: %s", err) - } - - newPassword, err := credsutil.RandomAlphaNumeric(32, true) - if err != nil { - t.Fatalf("failed to generate password: %s", err) - } - newPassword = strings.Replace(newPassword, "-", "_", -1) - - // Change Password - updateReq := newdbplugin.UpdateUserRequest{ - Username: userResp.Username, - Password: &newdbplugin.ChangePassword{ - NewPassword: newPassword, - }, - } - - _, err = db.UpdateUser(context.Background(), updateReq) - if err != nil { - t.Fatalf("err: %s", err) - } - if err := testCredsExist(t, connURL, userResp.Username, newPassword); err == nil { - t.Fatal("Credentials were not changed") - } - - // Test custom update statement - userResp, err = db.NewUser(context.Background(), newReq) - if err != nil { - t.Fatalf("err: %s", err) - } - if err = testCredsExist(t, connURL, userResp.Username, password); err != nil { - t.Fatalf("Could not connect with new credentials: %s", err) - } - - updateReq.Password.Statements.Commands = []string{testHANAUpdate} - updateReq.Username = userResp.Username - _, err = db.UpdateUser(context.Background(), updateReq) - if err != nil { - t.Fatalf("err: %s", err) - } - if err := testCredsExist(t, connURL, userResp.Username, newPassword); err == nil { - t.Fatal("Credentials were not revoked") + for name, test := range tests { + t.Run(name, func(t *testing.T) { + connectionDetails := map[string]interface{}{ + "connection_url": connURL, + } + + initReq := dbplugin.InitializeRequest{ + Config: connectionDetails, + VerifyConnection: true, + } + + db := new() + dbtesting.AssertInitialize(t, db, initReq) + defer dbtesting.AssertClose(t, db) + + newReq := dbplugin.NewUserRequest{ + UsernameConfig: dbplugin.UsernameMetadata{ + DisplayName: "test-test", + RoleName: "test-test", + }, + Password: test.startingPassword, + Statements: dbplugin.Statements{ + Commands: []string{testHANARole}, + }, + Expiration: time.Now().Add(time.Hour), + } + + userResp := dbtesting.AssertNewUser(t, db, newReq) + assertCredsExist(t, connURL, userResp.Username, test.startingPassword) + + test.req.Username = userResp.Username + + dbtesting.AssertUpdateUser(t, db, test.req) + err := testCredsExist(t, connURL, userResp.Username, test.req.Password.NewPassword) + if test.expectErrOnLogin && err == nil { + t.Fatalf("Able to login with new creds when expecting an issue") + } + if !test.expectErrOnLogin && err != nil { + t.Fatalf("Unable to login: %s", err) + } + }) } } @@ -199,77 +193,60 @@ func TestHANA_DeleteUser(t *testing.T) { } connURL := os.Getenv("HANA_URL") - connectionDetails := map[string]interface{}{ - "connection_url": connURL, + type testCase struct { + req dbplugin.DeleteUserRequest } - initReq := newdbplugin.InitializeRequest{ - Config: connectionDetails, - VerifyConnection: true, + tests := map[string]testCase{ + "no update statements": { + req: dbplugin.DeleteUserRequest{}, + }, + "with custom update statements": { + req: dbplugin.DeleteUserRequest{ + Statements: dbplugin.Statements{ + Commands: []string{testHANADrop}, + }, + }, + }, } - db := new() - _, err := db.Initialize(context.Background(), initReq) - if err != nil { - t.Fatalf("err: %s", err) - } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + connectionDetails := map[string]interface{}{ + "connection_url": connURL, + } - password, err := credsutil.RandomAlphaNumeric(32, true) - if err != nil { - t.Fatalf("failed to generate password: %s", err) - } - password = strings.Replace(password, "-", "_", -1) + initReq := dbplugin.InitializeRequest{ + Config: connectionDetails, + VerifyConnection: true, + } - newReq := newdbplugin.NewUserRequest{ - UsernameConfig: newdbplugin.UsernameMetadata{ - DisplayName: "test-test", - RoleName: "test-test", - }, - Password: password, - Statements: newdbplugin.Statements{ - Commands: []string{testHANARole}, - }, - Expiration: time.Now().Add(time.Hour), - } + db := new() + dbtesting.AssertInitialize(t, db, initReq) + defer dbtesting.AssertClose(t, db) - // Test default revoke statements - userResp, err := db.NewUser(context.Background(), newReq) - if err != nil { - t.Fatalf("err: %s", err) - } - if err = testCredsExist(t, connURL, userResp.Username, password); err != nil { - t.Fatalf("Could not connect with new credentials: %s", err) - } + password := "this_is_Thirty_2_characters_wow_" - delReq := newdbplugin.DeleteUserRequest{ - Username: userResp.Username, - } + newReq := dbplugin.NewUserRequest{ + UsernameConfig: dbplugin.UsernameMetadata{ + DisplayName: "test-test", + RoleName: "test-test", + }, + Password: password, + Statements: dbplugin.Statements{ + Commands: []string{testHANARole}, + }, + Expiration: time.Now().Add(time.Hour), + } - _, err = db.DeleteUser(context.Background(), delReq) - if err != nil { - t.Fatalf("err: %s", err) - } - if err := testCredsExist(t, connURL, userResp.Username, password); err == nil { - t.Fatal("Credentials were not revoked") - } + userResp := dbtesting.AssertNewUser(t, db, newReq) + assertCredsExist(t, connURL, userResp.Username, password) - // Test custom revoke statement - userResp, err = db.NewUser(context.Background(), newReq) - if err != nil { - t.Fatalf("err: %s", err) - } - if err = testCredsExist(t, connURL, userResp.Username, password); err != nil { - t.Fatalf("Could not connect with new credentials: %s", err) - } + test.req.Username = userResp.Username - delReq.Statements.Commands = []string{testHANADrop} - delReq.Username = userResp.Username - _, err = db.DeleteUser(context.Background(), delReq) - if err != nil { - t.Fatalf("err: %s", err) - } - if err := testCredsExist(t, connURL, userResp.Username, password); err == nil { - t.Fatal("Credentials were not revoked") + dbtesting.AssertDeleteUser(t, db, test.req) + assertCredsDoNotExist(t, connURL, userResp.Username, password) + }) } } @@ -285,11 +262,27 @@ func testCredsExist(t testing.TB, connURL, username, password string) error { return db.Ping() } +func assertCredsExist(t testing.TB, connURL, username, password string) { + t.Helper() + err := testCredsExist(t, connURL, username, password) + if err != nil { + t.Fatalf("Unable to log in as %q: %s", username, err) + } +} + +func assertCredsDoNotExist(t testing.TB, connURL, username, password string) { + t.Helper() + err := testCredsExist(t, connURL, username, password) + if err == nil { + t.Fatalf("Able to log in when we should not be able to") + } +} + const testHANARole = ` -CREATE USER {{name}} PASSWORD {{password}} NO FORCE_FIRST_PASSWORD_CHANGE VALID UNTIL '{{expiration}}';` +CREATE USER {{name}} PASSWORD "{{password}}" NO FORCE_FIRST_PASSWORD_CHANGE VALID UNTIL '{{expiration}}';` const testHANADrop = ` DROP USER {{name}} CASCADE;` const testHANAUpdate = ` -ALTER USER {{name}} PASSWORD "{{password}}";` +ALTER USER {{name}} PASSWORD "{{password}}" NO FORCE_FIRST_PASSWORD_CHANGE;` From 2d3037d1533d26d6af13629b3ac27bbe95be4f4f Mon Sep 17 00:00:00 2001 From: Lauren Voswinkel Date: Tue, 20 Oct 2020 14:44:03 -0700 Subject: [PATCH 6/8] More fixes --- plugins/database/hana/hana.go | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/plugins/database/hana/hana.go b/plugins/database/hana/hana.go index b6caf8f75e41..e9d156a6d2f2 100644 --- a/plugins/database/hana/hana.go +++ b/plugins/database/hana/hana.go @@ -27,7 +27,7 @@ type HANA struct { *connutil.SQLConnectionProducer } -var _ dbplugin.Database = &HANA{} +var _ dbplugin.Database = (*HANA)(nil) // New implements builtinplugins.BuiltinFactory func New() (interface{}, error) { @@ -90,7 +90,7 @@ func (h *HANA) getConnection(ctx context.Context) (*sql.DB, error) { return db.(*sql.DB), nil } -// CreateUser generates the username/password on the underlying HANA secret backend +// NewUser generates the username/password on the underlying HANA secret backend // as instructed by the CreationStatement provided. func (h *HANA) NewUser(ctx context.Context, req dbplugin.NewUserRequest) (response dbplugin.NewUserResponse, err error) { // Grab the lock @@ -107,14 +107,15 @@ func (h *HANA) NewUser(ctx context.Context, req dbplugin.NewUserRequest) (respon return dbplugin.NewUserResponse{}, dbutil.ErrEmptyCreationStatement } - dispName := credsutil.DisplayName(req.UsernameConfig.DisplayName, 32) - roleName := credsutil.RoleName(req.UsernameConfig.RoleName, 20) - maxLen := credsutil.MaxLength(maxIdentifierLength) - separator := credsutil.Separator("_") - caps := credsutil.ToUpper() - // Generate username - username, err := credsutil.GenerateUsername(dispName, roleName, maxLen, separator, caps) + username, err := credsutil.GenerateUsername( + credsutil.DisplayName(req.UsernameConfig.DisplayName, 32), + credsutil.RoleName(req.UsernameConfig.RoleName, 20), + credsutil.MaxLength(maxIdentifierLength), + credsutil.Separator("_"), + credsutil.ToUpper(), + ) + if err != nil { return dbplugin.NewUserResponse{}, err } @@ -123,11 +124,6 @@ func (h *HANA) NewUser(ctx context.Context, req dbplugin.NewUserRequest) (respon username = strings.Replace(username, "-", "_", -1) username = strings.ToUpper(username) - // Most HANA configurations have password constraints - // Prefix with A1a to satisfy these constraints. User will be forced to change upon login - password := req.Password - password = strings.Replace(password, "-", "_", -1) - // If expiration is in the role SQL, HANA will deactivate the user when time is up, // regardless of whether vault is alive to revoke lease expirationStr := req.Expiration.UTC().Format("2006-01-02 15:04:05") @@ -149,7 +145,7 @@ func (h *HANA) NewUser(ctx context.Context, req dbplugin.NewUserRequest) (respon m := map[string]string{ "name": username, - "password": password, + "password": req.Password, "expiration": expirationStr, } @@ -171,7 +167,8 @@ func (h *HANA) NewUser(ctx context.Context, req dbplugin.NewUserRequest) (respon return resp, nil } -// Renewing hana user just means altering user's valid until property +// UpdateUser allows for updating the expiration or password of the user mentioned in +// the UpdateUserRequest func (h *HANA) UpdateUser(ctx context.Context, req dbplugin.UpdateUserRequest) (dbplugin.UpdateUserResponse, error) { h.Lock() defer h.Unlock() From 5639409dcbb865340d48e98b43c485a7632baf19 Mon Sep 17 00:00:00 2001 From: Lauren Voswinkel Date: Wed, 21 Oct 2020 09:59:12 -0700 Subject: [PATCH 7/8] Add error check support for HANA update tests --- plugins/database/hana/hana_test.go | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/plugins/database/hana/hana_test.go b/plugins/database/hana/hana_test.go index 30bfbb9c324f..46f4b8bca536 100644 --- a/plugins/database/hana/hana_test.go +++ b/plugins/database/hana/hana_test.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/vault/sdk/database/dbplugin/v5" dbtesting "github.com/hashicorp/vault/sdk/database/dbplugin/v5/testing" "os" + "reflect" "strings" "testing" "time" @@ -22,6 +23,8 @@ func TestHANA_Initialize(t *testing.T) { "connection_url": connURL, } + expectedConfig := copyConfig(connectionDetails) + initReq := dbplugin.InitializeRequest{ Config: connectionDetails, VerifyConnection: true, @@ -29,8 +32,9 @@ func TestHANA_Initialize(t *testing.T) { db := new() initResp := dbtesting.AssertInitialize(t, db, initReq) - if initResp.Config == nil { - t.Fatalf("config not set by initialization") + + if !reflect.DeepEqual(initResp.Config, expectedConfig) { + t.Fatalf("Actual config: %#v\nExpected config: %#v", initResp.Config, expectedConfig) } } @@ -117,6 +121,7 @@ func TestHANA_UpdateUser(t *testing.T) { req dbplugin.UpdateUserRequest startingPassword string expectErrOnLogin bool + expectedErrMsg string } tests := map[string]testCase{ @@ -128,6 +133,7 @@ func TestHANA_UpdateUser(t *testing.T) { }, startingPassword: "this_is_Thirty_2_characters_wow_", expectErrOnLogin: true, + expectedErrMsg: "user is forced to change password", }, "with custom update statements": { req: dbplugin.UpdateUserRequest{ @@ -177,8 +183,12 @@ func TestHANA_UpdateUser(t *testing.T) { dbtesting.AssertUpdateUser(t, db, test.req) err := testCredsExist(t, connURL, userResp.Username, test.req.Password.NewPassword) - if test.expectErrOnLogin && err == nil { - t.Fatalf("Able to login with new creds when expecting an issue") + if test.expectErrOnLogin { + if err == nil { + t.Fatalf("Able to login with new creds when expecting an issue") + } else if test.expectedErrMsg != "" && !strings.Contains(err.Error(), test.expectedErrMsg) { + t.Fatalf("Expected error message to contain \"%s\", received: %s", test.expectedErrMsg, err) + } } if !test.expectErrOnLogin && err != nil { t.Fatalf("Unable to login: %s", err) @@ -278,6 +288,14 @@ func assertCredsDoNotExist(t testing.TB, connURL, username, password string) { } } +func copyConfig(config map[string]interface{}) map[string]interface{} { + newConfig := map[string]interface{}{} + for k, v := range config { + newConfig[k] = v + } + return newConfig +} + const testHANARole = ` CREATE USER {{name}} PASSWORD "{{password}}" NO FORCE_FIRST_PASSWORD_CHANGE VALID UNTIL '{{expiration}}';` From c9df7a19e0f536a786f72fc37e7e48202e8bfed2 Mon Sep 17 00:00:00 2001 From: Lauren Voswinkel Date: Wed, 21 Oct 2020 11:10:09 -0700 Subject: [PATCH 8/8] Clean up tests --- plugins/database/hana/hana_test.go | 116 ++++++++++++++--------------- 1 file changed, 54 insertions(+), 62 deletions(-) diff --git a/plugins/database/hana/hana_test.go b/plugins/database/hana/hana_test.go index 46f4b8bca536..b38e93b459cb 100644 --- a/plugins/database/hana/hana_test.go +++ b/plugins/database/hana/hana_test.go @@ -4,13 +4,14 @@ import ( "context" "database/sql" "fmt" - "github.com/hashicorp/vault/sdk/database/dbplugin/v5" - dbtesting "github.com/hashicorp/vault/sdk/database/dbplugin/v5/testing" "os" "reflect" "strings" "testing" "time" + + "github.com/hashicorp/vault/sdk/database/dbplugin/v5" + dbtesting "github.com/hashicorp/vault/sdk/database/dbplugin/v5/testing" ) func TestHANA_Initialize(t *testing.T) { @@ -32,6 +33,7 @@ func TestHANA_Initialize(t *testing.T) { db := new() initResp := dbtesting.AssertInitialize(t, db, initReq) + defer dbtesting.AssertClose(t, db) if !reflect.DeepEqual(initResp.Config, expectedConfig) { t.Fatalf("Actual config: %#v\nExpected config: %#v", initResp.Config, expectedConfig) @@ -47,39 +49,21 @@ func TestHANA_NewUser(t *testing.T) { connURL := os.Getenv("HANA_URL") type testCase struct { - req dbplugin.NewUserRequest - expectErr bool - assertUser func(t testing.TB, connURL, username, password string) + commands []string + expectErr bool + assertUser func(t testing.TB, connURL, username, password string) } tests := map[string]testCase{ "no creation statements": { - req: dbplugin.NewUserRequest{ - UsernameConfig: dbplugin.UsernameMetadata{ - DisplayName: "test-test", - RoleName: "test-test", - }, - Statements: dbplugin.Statements{}, - Password: "AG4qagho_dsvZ", - Expiration: time.Now().Add(1 * time.Second), - }, - expectErr: true, - assertUser: assertCredsDoNotExist, + commands: []string{}, + expectErr: true, + assertUser: assertCredsDoNotExist, }, "with creation statements": { - req: dbplugin.NewUserRequest{ - UsernameConfig: dbplugin.UsernameMetadata{ - DisplayName: "test-test", - RoleName: "test-test", - }, - Statements: dbplugin.Statements{ - Commands: []string{testHANARole}, - }, - Password: "AG4qagho_dsvZ", - Expiration: time.Now().Add(1 * time.Second), - }, - expectErr: false, - assertUser: assertCredsExist, + commands: []string{testHANARole}, + expectErr: false, + assertUser: assertCredsExist, }, } @@ -98,7 +82,19 @@ func TestHANA_NewUser(t *testing.T) { dbtesting.AssertInitialize(t, db, initReq) defer dbtesting.AssertClose(t, db) - createResp, err := db.NewUser(context.Background(), test.req) + req := dbplugin.NewUserRequest{ + UsernameConfig: dbplugin.UsernameMetadata{ + DisplayName: "test-test", + RoleName: "test-test", + }, + Statements: dbplugin.Statements{ + Commands: test.commands, + }, + Password: "AG4qagho_dsvZ", + Expiration: time.Now().Add(1 * time.Second), + } + + createResp, err := db.NewUser(context.Background(), req) if test.expectErr && err == nil { t.Fatalf("err expected, received nil") } @@ -106,7 +102,7 @@ func TestHANA_NewUser(t *testing.T) { t.Fatalf("no error expected, got: %s", err) } - test.assertUser(t, connURL, createResp.Username, test.req.Password) + test.assertUser(t, connURL, createResp.Username, req.Password) }) } } @@ -118,33 +114,19 @@ func TestHANA_UpdateUser(t *testing.T) { connURL := os.Getenv("HANA_URL") type testCase struct { - req dbplugin.UpdateUserRequest - startingPassword string + commands []string expectErrOnLogin bool expectedErrMsg string } tests := map[string]testCase{ "no update statements": { - req: dbplugin.UpdateUserRequest{ - Password: &dbplugin.ChangePassword{ - NewPassword: "this_is_ALSO_Thirty_2_characters_", - }, - }, - startingPassword: "this_is_Thirty_2_characters_wow_", + commands: []string{}, expectErrOnLogin: true, expectedErrMsg: "user is forced to change password", }, "with custom update statements": { - req: dbplugin.UpdateUserRequest{ - Password: &dbplugin.ChangePassword{ - NewPassword: "this_is_ALSO_Thirty_2_characters_", - Statements: dbplugin.Statements{ - Commands: []string{testHANAUpdate}, - }, - }, - }, - startingPassword: "this_is_Thirty_2_characters_wow_", + commands: []string{testHANAUpdate}, expectErrOnLogin: false, }, } @@ -164,12 +146,13 @@ func TestHANA_UpdateUser(t *testing.T) { dbtesting.AssertInitialize(t, db, initReq) defer dbtesting.AssertClose(t, db) + password := "this_is_Thirty_2_characters_wow_" newReq := dbplugin.NewUserRequest{ UsernameConfig: dbplugin.UsernameMetadata{ DisplayName: "test-test", RoleName: "test-test", }, - Password: test.startingPassword, + Password: password, Statements: dbplugin.Statements{ Commands: []string{testHANARole}, }, @@ -177,12 +160,20 @@ func TestHANA_UpdateUser(t *testing.T) { } userResp := dbtesting.AssertNewUser(t, db, newReq) - assertCredsExist(t, connURL, userResp.Username, test.startingPassword) + assertCredsExist(t, connURL, userResp.Username, password) - test.req.Username = userResp.Username + req := dbplugin.UpdateUserRequest{ + Username: userResp.Username, + Password: &dbplugin.ChangePassword{ + NewPassword: "this_is_ALSO_Thirty_2_characters_", + Statements: dbplugin.Statements{ + Commands: test.commands, + }, + }, + } - dbtesting.AssertUpdateUser(t, db, test.req) - err := testCredsExist(t, connURL, userResp.Username, test.req.Password.NewPassword) + dbtesting.AssertUpdateUser(t, db, req) + err := testCredsExist(t, connURL, userResp.Username, req.Password.NewPassword) if test.expectErrOnLogin { if err == nil { t.Fatalf("Able to login with new creds when expecting an issue") @@ -204,19 +195,15 @@ func TestHANA_DeleteUser(t *testing.T) { connURL := os.Getenv("HANA_URL") type testCase struct { - req dbplugin.DeleteUserRequest + commands []string } tests := map[string]testCase{ "no update statements": { - req: dbplugin.DeleteUserRequest{}, + commands: []string{}, }, "with custom update statements": { - req: dbplugin.DeleteUserRequest{ - Statements: dbplugin.Statements{ - Commands: []string{testHANADrop}, - }, - }, + commands: []string{testHANADrop}, }, } @@ -252,9 +239,14 @@ func TestHANA_DeleteUser(t *testing.T) { userResp := dbtesting.AssertNewUser(t, db, newReq) assertCredsExist(t, connURL, userResp.Username, password) - test.req.Username = userResp.Username + req := dbplugin.DeleteUserRequest{ + Username: userResp.Username, + Statements: dbplugin.Statements{ + Commands: test.commands, + }, + } - dbtesting.AssertDeleteUser(t, db, test.req) + dbtesting.AssertDeleteUser(t, db, req) assertCredsDoNotExist(t, connURL, userResp.Username, password) }) }