From 38b373e6df969a21f532b76a315bfb5071b20c67 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Wed, 7 Aug 2019 14:23:50 +0900
Subject: [PATCH] "node check": check role membership when trying to read
 pg_settings

From PostgreSQL 10, a member of the default roles "pg_monitor" and/or
"pg_read_all_settings" can read pg_settings without requiring superuser
privileges.

Previously, a hint was being emitted about making the repmgr user a
member of one of those groups, but no check for membership was being
made, meaning the check could only be run by a superuser.
---
 HISTORY              |  3 +++
 dbutils.c            | 51 ++++++++++++++++++++++++++++++++++++++++----
 dbutils.h            |  1 +
 repmgr-action-node.c |  4 ++--
 4 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/HISTORY b/HISTORY
index 9dc01de3..3b65f352 100644
--- a/HISTORY
+++ b/HISTORY
@@ -1,6 +1,9 @@
 4.5     2019-??-??
         general: parse configuration file using flex (Ian)
 
+4.4.1   2019-??-??
+        repmgr: improve data directory check (Ian)
+
 4.4     2019-06-27
         repmgr: improve "daemon status" output (Ian)
         repmgr: add "--siblings-follow" option to "standby promote" (Ian)
diff --git a/dbutils.c b/dbutils.c
index 9fa5be43..a4f5a13f 100644
--- a/dbutils.c
+++ b/dbutils.c
@@ -336,12 +336,10 @@ establish_db_connection_by_params(t_conninfo_param_list *param_list,
 bool
 is_superuser_connection(PGconn *conn, t_connection_user *userinfo)
 {
-	char	   *current_user = NULL;
-	const char *superuser_status = NULL;
 	bool		is_superuser = false;
+	const char *current_user = PQuser(conn);
+	const char *superuser_status = PQparameterStatus(conn, "is_superuser");
 
-	current_user = PQuser(conn);
-	superuser_status = PQparameterStatus(conn, "is_superuser");
 	is_superuser = (strcmp(superuser_status, "on") == 0) ? true : false;
 
 	if (userinfo != NULL)
@@ -356,6 +354,51 @@ is_superuser_connection(PGconn *conn, t_connection_user *userinfo)
 }
 
 
+bool
+connection_has_pg_settings(PGconn *conn)
+{
+	bool		has_pg_settings = false;
+
+	/* superusers can always read pg_settings */
+	if (is_superuser_connection(conn, NULL) == true)
+	{
+		has_pg_settings = true;
+	}
+	/* from PostgreSQL 10, a non-superuser may have been granted access */
+	else if(PQserverVersion(conn) >= 100000)
+	{
+		PQExpBufferData query;
+		PGresult   *res;
+
+		initPQExpBuffer(&query);
+		appendPQExpBufferStr(&query,
+							 "  SELECT CASE "
+							 "           WHEN pg_catalog.pg_has_role(CURRENT_USER, 'pg_monitor','MEMBER') "
+							 "             THEN TRUE "
+							 "           WHEN pg_catalog.pg_has_role(CURRENT_USER, 'pg_read_all_settings','MEMBER') "
+							 "             THEN TRUE "
+							 "           ELSE FALSE "
+							 "         END AS has_pg_settings");
+
+		res = PQexec(conn, query.data);
+
+		if (PQresultStatus(res) != PGRES_TUPLES_OK)
+		{
+			log_db_error(conn, query.data,
+						 _("connection_has_pg_settings(): unable to query user roles"));
+		}
+		else
+		{
+			has_pg_settings = atobool(PQgetvalue(res, 0, 0));
+		}
+		termPQExpBuffer(&query);
+		PQclear(res);
+	}
+
+	return has_pg_settings;
+}
+
+
 void
 close_connection(PGconn **conn)
 {
diff --git a/dbutils.h b/dbutils.h
index e518998d..19e30ced 100644
--- a/dbutils.h
+++ b/dbutils.h
@@ -438,6 +438,7 @@ PGconn	   *get_primary_connection(PGconn *standby_conn, int *primary_id, char *p
 PGconn	   *get_primary_connection_quiet(PGconn *standby_conn, int *primary_id, char *primary_conninfo_out);
 
 bool		is_superuser_connection(PGconn *conn, t_connection_user *userinfo);
+bool		connection_has_pg_settings(PGconn *conn);
 void		close_connection(PGconn **conn);
 
 /* conninfo manipulation functions */
diff --git a/repmgr-action-node.c b/repmgr-action-node.c
index 77255d4a..7c521e5a 100644
--- a/repmgr-action-node.c
+++ b/repmgr-action-node.c
@@ -1833,7 +1833,7 @@ do_node_check_data_directory(PGconn *conn, OutputMode mode, t_node_info *node_in
 	 * a superuser connection
 	 */
 
-	if (is_superuser_connection(conn, NULL) == true)
+	if (connection_has_pg_settings(conn) == true)
 	{
 		/* we expect to have a database connection */
 		if (get_pg_setting(conn, "data_directory", actual_data_directory) == false)
@@ -1878,7 +1878,7 @@ do_node_check_data_directory(PGconn *conn, OutputMode mode, t_node_info *node_in
 			/* XXX add -S/--superuser option */
 			if (PQserverVersion(conn) >= 100000)
 			{
-				log_hint(_("add the \"%s\" user to group \"pg_read_all_settings\""),
+				log_hint(_("add the \"%s\" user to group \"pg_read_all_settings\" or \"pg_monitor\""),
 						   PQuser(conn));
 			}
 		}