From b947222bf524510087337542d64c0e1e8261fee3 Mon Sep 17 00:00:00 2001
From: Guillaume Jacquet <guillaume.jacquet@gmail.com>
Date: Fri, 7 Jul 2023 05:17:35 -0400
Subject: [PATCH] Platform: Add support for Postgresql pgpass file (#61517)

lib/pq has built-in support to use pgpass file for authentication when
no password has been provided. However this requires that the connection
does not contain the password parameter at all.

Removing password parameter when postgresql password is empty in
SQL store.
---
 pkg/services/sqlstore/sqlstore.go      | 10 +++---
 pkg/services/sqlstore/sqlstore_test.go | 44 ++++++++++++++++++++------
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go
index 04b862f1619b..1106d18f30db 100644
--- a/pkg/services/sqlstore/sqlstore.go
+++ b/pkg/services/sqlstore/sqlstore.go
@@ -316,15 +316,15 @@ func (ss *SQLStore) buildConnectionString() (string, error) {
 			return "", fmt.Errorf("invalid host specifier '%s': %w", ss.dbCfg.Host, err)
 		}
 
-		if ss.dbCfg.Pwd == "" {
-			ss.dbCfg.Pwd = "''"
-		}
 		if ss.dbCfg.User == "" {
 			ss.dbCfg.User = "''"
 		}
-		cnnstr = fmt.Sprintf("user=%s password=%s host=%s port=%s dbname=%s sslmode=%s sslcert=%s sslkey=%s sslrootcert=%s",
-			ss.dbCfg.User, ss.dbCfg.Pwd, addr.Host, addr.Port, ss.dbCfg.Name, ss.dbCfg.SslMode, ss.dbCfg.ClientCertPath,
+		cnnstr = fmt.Sprintf("user=%s host=%s port=%s dbname=%s sslmode=%s sslcert=%s sslkey=%s sslrootcert=%s",
+			ss.dbCfg.User, addr.Host, addr.Port, ss.dbCfg.Name, ss.dbCfg.SslMode, ss.dbCfg.ClientCertPath,
 			ss.dbCfg.ClientKeyPath, ss.dbCfg.CaCertPath)
+		if ss.dbCfg.Pwd != "" {
+			cnnstr += fmt.Sprintf(" password=%s", ss.dbCfg.Pwd)
+		}
 
 		cnnstr += ss.buildExtraConnectionString(' ')
 	case migrator.SQLite:
diff --git a/pkg/services/sqlstore/sqlstore_test.go b/pkg/services/sqlstore/sqlstore_test.go
index b643216ea0a6..5032c9c9d29f 100644
--- a/pkg/services/sqlstore/sqlstore_test.go
+++ b/pkg/services/sqlstore/sqlstore_test.go
@@ -15,12 +15,15 @@ import (
 )
 
 type sqlStoreTest struct {
-	name          string
-	dbType        string
-	dbHost        string
-	dbURL         string
-	connStrValues []string
-	err           error
+	name                  string
+	dbType                string
+	dbHost                string
+	dbURL                 string
+	dbUser                string
+	dbPwd                 string
+	connStrValues         []string
+	connStrExcludedValues []string
+	err                   error
 }
 
 var sqlStoreTestCases = []sqlStoreTest{
@@ -42,6 +45,23 @@ var sqlStoreTestCases = []sqlStoreTest{
 		dbHost:        "1.2.3.4",
 		connStrValues: []string{"host=1.2.3.4", "port=5432"},
 	},
+	{
+		name:          "Postgres username and password",
+		dbType:        "postgres",
+		dbHost:        "1.2.3.4",
+		dbUser:        "grafana",
+		dbPwd:         "password",
+		connStrValues: []string{"host=1.2.3.4", "port=5432", "user=grafana", "password=password"},
+	},
+	{
+		name:                  "Postgres username no password",
+		dbType:                "postgres",
+		dbHost:                "1.2.3.4",
+		dbUser:                "grafana",
+		dbPwd:                 "",
+		connStrValues:         []string{"host=1.2.3.4", "port=5432", "user=grafana"},
+		connStrExcludedValues: []string{"password"},
+	},
 	{
 		name:          "MySQL IPv4 (Default Port)",
 		dbType:        "mysql",
@@ -92,13 +112,17 @@ func TestIntegrationSQLConnectionString(t *testing.T) {
 	for _, testCase := range sqlStoreTestCases {
 		t.Run(testCase.name, func(t *testing.T) {
 			sqlstore := &SQLStore{}
-			sqlstore.Cfg = makeSQLStoreTestConfig(t, testCase.dbType, testCase.dbHost, testCase.dbURL)
+			sqlstore.Cfg = makeSQLStoreTestConfig(t, testCase.dbType, testCase.dbHost, testCase.dbUser, testCase.dbPwd, testCase.dbURL)
 			connStr, err := sqlstore.buildConnectionString()
 			require.Equal(t, testCase.err, err)
 
 			for _, connSubStr := range testCase.connStrValues {
 				require.Contains(t, connStr, connSubStr)
 			}
+
+			for _, connExcludedSubStr := range testCase.connStrExcludedValues {
+				require.NotContains(t, connStr, connExcludedSubStr)
+			}
 		})
 	}
 }
@@ -151,7 +175,7 @@ func TestIntegrationIsUniqueConstraintViolation(t *testing.T) {
 	}
 }
 
-func makeSQLStoreTestConfig(t *testing.T, dbType, host, dbURL string) *setting.Cfg {
+func makeSQLStoreTestConfig(t *testing.T, dbType, host, user, password, dbURL string) *setting.Cfg {
 	t.Helper()
 
 	cfg := setting.NewCfg()
@@ -164,11 +188,11 @@ func makeSQLStoreTestConfig(t *testing.T, dbType, host, dbURL string) *setting.C
 	require.NoError(t, err)
 	_, err = sec.NewKey("url", dbURL)
 	require.NoError(t, err)
-	_, err = sec.NewKey("user", "user")
+	_, err = sec.NewKey("user", user)
 	require.NoError(t, err)
 	_, err = sec.NewKey("name", "test_db")
 	require.NoError(t, err)
-	_, err = sec.NewKey("password", "pass")
+	_, err = sec.NewKey("password", password)
 	require.NoError(t, err)
 
 	cfg.IsFeatureToggleEnabled = func(key string) bool { return true }