From c852e76a5866b3af70aef98d74132da51e40d88a Mon Sep 17 00:00:00 2001
From: Jelte Fennema <jelte.fennema@microsoft.com>
Date: Wed, 11 Jan 2023 12:10:19 +0100
Subject: [PATCH v3 4/4] Support same user patterns in pg_ident.conf as in
 pg_hba.conf

While pg_hba.conf has support for non-literal username matches,
pg_ident.conf doesn't have this same functionality. This changes
permission checking in pg_ident.conf to handle all the special cases for
username checking:
1. The "all" keyword
2. Membership checks using the + prefix
3. Using a regex to match against multiple roles

This allows matching certain system users against many different
postgres users with a single line in pg_ident.conf. Without this you
you would need a line for each of the postgres users that a system user
can log in as.
---
 doc/src/sgml/client-auth.sgml         |  26 +++++-
 src/backend/libpq/hba.c               |  76 ++++++++++------
 src/test/authentication/t/003_peer.pl | 125 ++++++++++++++++++++++++--
 3 files changed, 192 insertions(+), 35 deletions(-)

diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 08a25a5e002..aefc9de0e45 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -941,7 +941,22 @@ local   db1,db2,@demodbs  all                                   md5
    implying that they are equivalent.  The connection will be allowed if
    there is any map entry that pairs the user name obtained from the
    external authentication system with the database user name that the
-   user has requested to connect as.
+   user has requested to connect as. The value <literal>all</literal>
+   can be used as the <replaceable>database-username</replaceable> to specify
+   that if the <replaceable>system-user</replaceable> matches, then this user
+   is allowed to log in as any of the existing database users. Quoting
+   <literal>all</literal> makes the keyword lose its special meaning.
+  </para>
+  <para>
+   If the <replaceable>database-username</replaceable> begins with a
+   <literal>+</literal> character, then the operating system user can login as
+   any user belonging to that role, similarly to how user names beginning with
+   <literal>+</literal> are treated in <literal>pg_hba.conf</literal>.
+   Thus, a <literal>+</literal> mark means <quote>match any of the roles that
+   are directly or indirectly members of this role</quote>, while a name
+   without a <literal>+</literal> mark matches only that specific role. Quoting
+   a username starting with a <literal>+</literal> makes the
+   <literal>+</literal> lose its special meaning.
   </para>
   <para>
    If the <replaceable>system-username</replaceable> field starts with a slash (<literal>/</literal>),
@@ -964,6 +979,15 @@ mymap   /^(.*)@otherdomain\.com$   guest
    <literal>\1</literal> makes the <literal>\1</literal>
    lose its special meaning.
   </para>
+  <para>
+   If the <replaceable>database-username</replaceable> field starts with a slash (<literal>/</literal>),
+   the remainder of the field is treated as a regular expression.
+   (See <xref linkend="posix-syntax-details"/> for details of
+   <productname>PostgreSQL</productname>'s regular expression syntax.) It's
+   currently not possible to use the <literal>\1</literal> to use a capture
+   from a <replaceable>system-username</replaceable> regular expression in a
+   regular expression for a <replaceable>database-username</replaceable>.
+  </para>
 
   <tip>
    <para>
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 0c00580dff0..e3c90a7a5bf 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -998,7 +998,7 @@ is_member(Oid userid, const char *role)
  * expressions (if any) and the exact match.
  */
 static bool
-check_role(const char *role, Oid roleid, List *tokens)
+check_role(const char *role, Oid roleid, List *tokens, bool case_insensitive)
 {
 	ListCell   *cell;
 	AuthToken  *tok;
@@ -1018,6 +1018,11 @@ check_role(const char *role, Oid roleid, List *tokens)
 			if (regexec_auth_token(role, tok, 0, NULL) == REG_OKAY)
 				return true;
 		}
+		else if (case_insensitive)
+		{
+			if (pg_strcasecmp(tok->string, role) == 0)
+				return true;
+		}
 		else if (token_matches(tok, role))
 			return true;
 	}
@@ -2614,7 +2619,7 @@ check_hba(hbaPort *port)
 					  hba->databases))
 			continue;
 
-		if (!check_role(port->user_name, roleid, hba->roles))
+		if (!check_role(port->user_name, roleid, hba->roles, false))
 			continue;
 
 		/* Found a record that matched! */
@@ -2804,7 +2809,7 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
 
 	/*
 	 * Now that the field validation is done, compile a regex from the user
-	 * token, if necessary.
+	 * tokens, if necessary.
 	 */
 	if (regcomp_auth_token(parsedline->system_user, file_name, line_num,
 						   err_msg, elevel))
@@ -2813,6 +2818,14 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
 		return NULL;
 	}
 
+	if (regcomp_auth_token(parsedline->pg_user, file_name, line_num,
+						   err_msg, elevel))
+	{
+		/* err_msg includes the error to report */
+		return NULL;
+	}
+
+
 	return parsedline;
 }
 
@@ -2827,6 +2840,8 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
 					const char *pg_user, const char *system_user,
 					bool case_insensitive, bool *found_p, bool *error_p)
 {
+	Oid			roleid;
+
 	*found_p = false;
 	*error_p = false;
 
@@ -2834,6 +2849,9 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
 		/* Line does not match the map name we're looking for, so just abort */
 		return;
 
+	/* Get the target role's OID.  Note we do not error out for bad role. */
+	roleid = get_role_oid(pg_user, true);
+
 	/* Match? */
 	if (token_has_regexp(identLine->system_user))
 	{
@@ -2845,7 +2863,8 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
 		int			r;
 		regmatch_t	matches[2];
 		char	   *ofs;
-		char	   *expanded_pg_user;
+		AuthToken  *expanded_pg_user_token;
+		bool		created_temporary_token = false;
 
 		r = regexec_auth_token(system_user, identLine->system_user, 2, matches);
 		if (r)
@@ -2865,9 +2884,17 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
 			return;
 		}
 
+		/*
+		 * Create a temporary token with \1 substituted if the string is not
+		 * quoted and has no indicaters of other special meanings.
+		 */
 		if (!identLine->pg_user->quoted &&
+			identLine->pg_user->string[0] != '+' &&
+			!token_is_keyword(identLine->pg_user, "all") &&
+			!token_has_regexp(identLine->pg_user) &&
 			(ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
 		{
+			char	   *expanded_pg_user;
 			int			offset;
 
 			/* substitution of the first argument requested */
@@ -2892,46 +2919,39 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
 				   system_user + matches[1].rm_so,
 				   matches[1].rm_eo - matches[1].rm_so);
 			strcat(expanded_pg_user, ofs + 2);
+			expanded_pg_user_token = make_auth_token(expanded_pg_user, true);
+			created_temporary_token = true;
+			pfree(expanded_pg_user);
 		}
 		else
 		{
-			/* no substitution, so copy the match */
-			expanded_pg_user = pstrdup(identLine->pg_user->string);
+			expanded_pg_user_token = identLine->pg_user;
 		}
 
-		/*
-		 * now check if the username actually matched what the user is trying
-		 * to connect as
-		 */
-		if (case_insensitive)
-		{
-			if (pg_strcasecmp(expanded_pg_user, pg_user) == 0)
-				*found_p = true;
-		}
-		else
-		{
-			if (strcmp(expanded_pg_user, pg_user) == 0)
-				*found_p = true;
-		}
-		pfree(expanded_pg_user);
+		*found_p = check_role(pg_user, roleid, list_make1(expanded_pg_user_token), case_insensitive);
+
+		if (created_temporary_token)
+			free_auth_token(expanded_pg_user_token);
 
 		return;
 	}
 	else
 	{
-		/* Not regular expression, so make complete match */
+		/*
+		 * If the systemuser does not match, there's no match
+		 */
 		if (case_insensitive)
 		{
-			if (pg_strcasecmp(identLine->pg_user->string, pg_user) == 0 &&
-				pg_strcasecmp(identLine->system_user->string, system_user) == 0)
-				*found_p = true;
+			if (pg_strcasecmp(identLine->system_user->string, system_user) != 0)
+				return;
 		}
 		else
 		{
-			if (strcmp(identLine->pg_user->string, pg_user) == 0 &&
-				strcmp(identLine->system_user->string, system_user) == 0)
-				*found_p = true;
+			if (strcmp(identLine->system_user->string, system_user) != 0)
+				return;
 		}
+
+		*found_p = check_role(pg_user, roleid, list_make1(identLine->pg_user), case_insensitive);
 	}
 }
 
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 968f4a319d8..35a939bcc32 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -98,8 +98,10 @@ if (find_in_log(
 	plan skip_all => 'peer authentication is not supported on this platform';
 }
 
-# Add a database role, to use for the user name map.
+# Add a database role and group, to use for the user name map.
 $node->safe_psql('postgres', qq{CREATE ROLE testmapuser LOGIN});
+$node->safe_psql('postgres',"CREATE ROLE testmapgroup NOLOGIN");
+$node->safe_psql('postgres', "GRANT testmapgroup TO testmapuser");
 
 # Extract as well the system user for the user name map.
 my $system_user =
@@ -123,12 +125,48 @@ test_role($node, qq{testmapuser}, 'peer', 0, 'with user name map',
 	log_like =>
 	  [qr/connection authenticated: identity="$system_user" method=peer/]);
 
+# Tests with the "all" keyword
+reset_pg_ident($node, 'mypeermap', $system_user, 'all');
+
+# Success as the database role is the "all" keyword
+test_role($node, qq{testmapuser}, 'peer', 0, 'with all keyword in user name map',
+	log_like =>
+	  [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+# Tests with the the literal "all" user
+reset_pg_ident($node, 'mypeermap', $system_user, '"all"');
+
+# Failure as we're not logging is as the literal "all" user
+test_role($node, qq{testmapuser}, 'peer', 2, 'with literal all user in user name map',
+	log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]);
+
+# Success as the database user regular expression matches
+reset_pg_ident($node, 'mypeermap', $system_user, qq{/^testm.*\$});
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	0,
+	'with database user regular expression in user name map',
+	log_like =>
+	  [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+# Failure as the database user regular expression does not match.
+reset_pg_ident($node, 'mypeermap', $system_user, qq{/^doesnotmatch.*\$});
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	2,
+	'with bad database user regular expression in user name map',
+	log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]);
+
 # Test with regular expression in user name map.
 # Extract the last 3 characters from the system_user
 # or the entire system_user (if its length is <= -3).
 my $regex_test_string = substr($system_user, -3);
 
-# Success as the regular expression matches.
+# Success as the system user regular expression matches.
 reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$},
 	'testmapuser');
 test_role(
@@ -136,10 +174,34 @@ test_role(
 	qq{testmapuser},
 	'peer',
 	0,
-	'with regular expression in user name map',
+	'with system user regular expression in user name map',
 	log_like =>
 	  [qr/connection authenticated: identity="$system_user" method=peer/]);
 
+# Success as both regular expression match.
+reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$},
+	qq{/^testm.*\$});
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	0,
+	'with system and database user regular expressions in user name map',
+	log_like =>
+	  [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+# Success as the regular expression matches and database role is the "all"
+# keyword.
+reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$},
+	'all');
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	0,
+	'with system user regular expression and all keyword in user name map',
+	log_like =>
+	  [qr/connection authenticated: identity="$system_user" method=peer/]);
 
 # Success as the regular expression matches and \1 is replaced
 reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$},
@@ -179,10 +241,10 @@ test_role(
 	log_like => [qr/regular expression "\^$system_user\$" has no subexpressions as requested by backreference in "\\1testmapuser"/]);
 
 # Concatenate system_user to system_user.
-$regex_test_string = $system_user . $system_user;
+my $bad_regex_test_string = $system_user . $system_user;
 
-# Failure as the regular expression does not match.
-reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$},
+# Failure as the system user regular expression does not match.
+reset_pg_ident($node, 'mypeermap', qq{/^.*$bad_regex_test_string\$},
 	'testmapuser');
 test_role(
 	$node,
@@ -192,4 +254,55 @@ test_role(
 	'with regular expression in user name map',
 	log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]);
 
+# test using a group role match
+# first with a plain user ...
+reset_pg_ident($node, 'mypeermap', $system_user, '+testmapgroup');
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	0,
+	'plain user with group',
+	log_like =>
+	  [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+test_role(
+	$node,
+	qq{testmapgroup},
+	'peer',
+	2,
+	'group user with group',
+	log_like =>
+	  [qr/role "testmapgroup" is not permitted to log in/]);
+
+reset_pg_ident($node, 'mypeermap', $system_user, '"+testmapgroup"');
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	2,
+	'plain user with literal + in front of a wrong user',
+	log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]);
+
+# ... then with a regex user
+reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$}, '+testmapgroup');
+
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	0,
+	'regex user with group',
+	log_like =>
+	  [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+test_role(
+	$node,
+	qq{testmapgroup},
+	'peer',
+	2,
+	'regex group user with group',
+	log_like =>
+	  [qr/role "testmapgroup" is not permitted to log in/]);
+
 done_testing();
-- 
2.34.1

