secret/database: Guard against panic with InfluxDB plugin (#8282)

* database/influx: fix panic when trying to revoke user

Guard against other nil responses

* return an error if response is nil, which is unlikely but best safe than sorry

* refactor a deeply nested statement into a function

plugins/database/influxdb/connection_producer.go

@@ -249,6 +249,9 @@ func isUserAdmin(cli influx.Client, user string) (bool, error) {
 	if err != nil {
 		return false, err
 	}
+	if response == nil {
+		return false, fmt.Errorf("empty response")
+	}
 	if response.Error() != nil {
 		return false, response.Error()
 	}

plugins/database/influxdb/influxdb.go

@@ -129,23 +129,9 @@ func (i *Influxdb) CreateUser(ctx context.Context, statements dbplugin.Statement
 				"password": password,
 			}), "", "")
 			response, err := cli.Query(q)
-			if err != nil && response.Error() != nil {
-				for _, stmt := range rollbackIFQL {
-					for _, query := range strutil.ParseArbitraryStringSlice(stmt, ";") {
-						query = strings.TrimSpace(query)
-
-						if len(query) == 0 {
-							continue
-						}
-						q := influx.NewQuery(dbutil.QueryHelper(query, map[string]string{
-							"username": username,
-						}), "", "")
-
-						response, err := cli.Query(q)
-						if err != nil && response.Error() != nil {
-							return "", "", err
-						}
-					}
+			if err != nil {
+				if response != nil && response.Error() != nil {
+					attemptRollback(cli, username, rollbackIFQL)
 				}
 				return "", "", err
 			}
@@ -154,6 +140,31 @@ func (i *Influxdb) CreateUser(ctx context.Context, statements dbplugin.Statement
 	return username, password, nil
 }
 
+// attemptRollback will attempt to roll back user creation if an error occurs in
+// CreateUser
+func attemptRollback(cli influx.Client, username string, rollbackStatements []string) error {
+	for _, stmt := range rollbackStatements {
+		for _, query := range strutil.ParseArbitraryStringSlice(stmt, ";") {
+			query = strings.TrimSpace(query)
+
+			if len(query) == 0 {
+				continue
+			}
+			q := influx.NewQuery(dbutil.QueryHelper(query, map[string]string{
+				"username": username,
+			}), "", "")
+
+			response, err := cli.Query(q)
+			if err != nil {
+				if response != nil && response.Error() != nil {
+					return err
+				}
+			}
+		}
+	}
+	return nil
+}
+
 // RenewUser is not supported on Influxdb, so this is a no-op.
 func (i *Influxdb) RenewUser(ctx context.Context, statements dbplugin.Statements, username string, expiration time.Time) error {
 	// NOOP
@@ -190,7 +201,9 @@ func (i *Influxdb) RevokeUser(ctx context.Context, statements dbplugin.Statement
 			}), "", "")
 			response, err := cli.Query(q)
 			result = multierror.Append(result, err)
-			result = multierror.Append(result, response.Error())
+			if response != nil {
+				result = multierror.Append(result, response.Error())
+			}
 		}
 	}
 	return result.ErrorOrNil()
@@ -230,7 +243,9 @@ func (i *Influxdb) RotateRootCredentials(ctx context.Context, statements []strin
 			}), "", "")
 			response, err := cli.Query(q)
 			result = multierror.Append(result, err)
-			result = multierror.Append(result, response.Error())
+			if response != nil {
+				result = multierror.Append(result, response.Error())
+			}
 		}
 	}
 

plugins/database/influxdb/influxdb_test.go

@@ -193,6 +193,56 @@ func TestMyInfluxdb_RenewUser(t *testing.T) {
 	}
 }
 
+// TestInfluxdb_RevokeDeletedUser tests attempting to revoke a user that was
+// deleted externally. Guards against a panic, see
+// https://github.com/hashicorp/vault/issues/6734
+func TestInfluxdb_RevokeDeletedUser(t *testing.T) {
+	if os.Getenv("VAULT_ACC") == "" {
+		t.SkipNow()
+	}
+	cleanup, address, port := prepareInfluxdbTestContainer(t)
+
+	connectionDetails := map[string]interface{}{
+		"host":     address,
+		"port":     port,
+		"username": "influx-root",
+		"password": "influx-root",
+	}
+
+	db := new()
+	_, err := db.Init(context.Background(), connectionDetails, true)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	statements := dbplugin.Statements{
+		Creation: []string{testInfluxRole},
+	}
+
+	usernameConfig := dbplugin.UsernameConfig{
+		DisplayName: "test",
+		RoleName:    "test",
+	}
+
+	username, password, err := db.CreateUser(context.Background(), statements, usernameConfig, time.Now().Add(time.Minute))
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	if err = testCredsExist(t, address, port, username, password); err != nil {
+		t.Fatalf("Could not connect with new credentials: %s", err)
+	}
+
+	// call cleanup to remove database
+	cleanup()
+
+	// attempt to revoke the user after database is gone
+	err = db.RevokeUser(context.Background(), statements, username)
+	if err == nil {
+		t.Fatalf("Expected err, got nil")
+	}
+}
+
 func TestInfluxdb_RevokeUser(t *testing.T) {
 	if os.Getenv("VAULT_ACC") == "" {
 		t.SkipNow()
@@ -301,7 +351,7 @@ func testCredsExist(t testing.TB, address string, port int, username, password s
 	if err != nil {
 		return errwrap.Wrapf("error querying influxdb server: {{err}}", err)
 	}
-	if response.Error() != nil {
+	if response != nil && response.Error() != nil {
 		return errwrap.Wrapf("error using the correct influx database: {{err}}", response.Error())
 	}
 	return nil