Subject: Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf

From: Michael Paquier <[email-masked]>
Date: Wed, 11 Jan 2023 20:05:52 +0900

On Wed, Jan 11, 2023 at 09:04:56AM +0000, Jelte Fennema wrote:
> It's very different. I think easiest is to explain by example:
> 
> If there exist three users on the postgres server: admin, jelte and michael
> 
> Then this rule (your suggested rule):
> mapname /^(.*)$ \1
> 
> Is equivalent to:
> mapname admin admin
> mapname jelte jelte
> mapname michael michael
> 
> While with the "all" keyword you can create a rule like this:
> mapname admin all
> 
> which is equivalent to:
> mapname admin admin
> mapname admin jelte
> mapname admin michael

Thanks for the explanation, I was missing your point.  Hmm.  On top
of my mind, couldn't we also use a regexp for the pg-role rather than
just a hardcoded keyword here then, so as it would be possible to
allow a mapping to pass for a group of role names?  "all" is just a
pattern to allow everything, at the end.
--
Michael

---

From: Jelte Fennema <[email-masked]>
Date: Wed, 11 Jan 2023 15:22:35 +0100

> couldn't we also use a regexp for the pg-role rather than

That's a good point. I hadn't realised that you added support for
regexes in pg_hba.conf in 8fea868. Attached is a patchset
where I reuse the pg_hba.conf code path to add support to
pg_ident.conf for: all, +group and regexes.

The main uncertainty I have is if the case insensitivity check is
actually needed in check_role. It seems like a case insensitive
check against the database user shouldn't actually be necessary.
I only understand the need for the case insensitive check against
the system user. But I have too little experience with LDAP/kerberos
to say for certain. So for now I kept the existing behaviour to
not regress.

The patchset also contains 3 preparatory patches with two refactoring
passes and one small bugfix + test additions.

> - renaming "systemuser" to "system_user_token" to outline that this is
> not a simple string but an AuthToken with potentially a regexp?

I decided against this, since now both system user and database user
are tokens. Furthermore, compiler warnings should avoid any confusion
against using this as a normal string. If you feel strongly about this
though, I'm happy to change this.

---

From: Michael Paquier <[email-masked]>
Date: Thu, 12 Jan 2023 14:32:15 +0900

> to say for certain. So for now I kept the existing behaviour to

                if (!identLine->pg_user->quoted &&
+                       identLine->pg_user->string[0] != '+' &&
+                       !token_is_keyword(identLine->pg_user, "all") &&
+                       !token_has_regexp(identLine->pg_user) &&
If we finish by allowing a regexp for the PG user in an IdentLine, I
would choose to drop "all" entirely.  Simpler is better when it comes
to authentication, though we are working on getting things more..
Complicated.

+   Quoting a <replaceable>database-username</replaceable> containing
+   <literal>\1</literal> makes the <literal>\1</literal>
+   lose its special meaning.
0002 and 0003 need careful thinking.

+# Success as the regular expression matches and \1 is replaced
+reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$},
+       'test\1mapuser');
+test_role(
+       $node,
+       qq{testmapuser},
+       'peer',
+       0,
+       'with regular expression in user name map with \1',
+       log_like =>
+         [qr/connection authenticated: identity="$system_user" method=peer/]);
Relying on kerberos to check the substitution pattern is a bit
annoying..  I would be really tempted to extract and commit that
independently of the rest, actually, to provide some coverage of the
substitution case in the peer test.

> passes and one small bugfix + test additions.

Applied 0001, which looked fine and was an existing issue.  At the
end, I had no issues with the names you suggested.
--
Michael

---

From: Jelte Fennema <[email-masked]>
Date: Thu, 12 Jan 2023 10:10:02 +0100

> Simpler is better when it comes to authentication

I definitely agree with that, and if we didn't have existing
parsing logic for pg_hba.conf I would agree. But given the existing
logic for pg_hba.conf, I think the path of least surprises is to
support all of the same patterns that pg_hbac.conf supports.

It also makes the code simpler as we can simply reuse the
check_role function, since that. I removed the lines you quoted
since those are actually not strictly necessary. They only change
the detection logic a bit in case of a \1 existing in the string.
And I'm not sure what the desired behaviour is for those.

> I would be really tempted to extract and commit that

I split up that patch in two parts now and added the tests in a new 0001
patch.

> 0002 and 0003 need careful thinking.

0002 should change no behaviour, since it simply stores the token in
the IdentLine struct, but doesn't start using the quoted or the regex field
yet. 0003 is debatable indeed. To me it makes sense conceptually, but
having a literal \1 in a username seems like an unlikely scenario and
there might be pg_ident.conf files in existence where the \1 is quoted
that would break because of this change. I haven't removed 0003 from
the patch set yet, but I kinda feel that the advantage is probably not
worth the risk of breakage.

0004 adds some breakage too. But there I think the advantages far outweigh
the risk of breakage. Both because added functionality is a much bigger
advantage, and because we only risk breaking when there exist users that
are called "all", start with a literal + or start with a literal /.
Only "all" seems
like a somewhat reasonable username, but such a user existing seems
unlikely to me given all its special meaning in pg_hba.conf. (I added this
consideration to the commit message)

> > to say for certain. So for now I kept the existing behaviour to

You didn't write a response about this, but you did quote it. Did you intend
to respond to it?

> Applied 0001

Awesome :)

Finally, one separate thing I noticed is that regcomp_auth_token only
checks the / prefix, but doesn't check if the token was quoted or not.
So even if it's quoted it will be interpreted as a regex. Maybe we should
change that? At least for the regex parsing that is not released yet.

---

From: Michael Paquier <[email-masked]>
Date: Fri, 13 Jan 2023 11:09:11 +0900

> And I'm not sure what the desired behaviour is for those.

Hmm.  This is a very good point.  0004 gets really easy to follow
now.

> I split up that patch in two parts now and added the tests in a new 0001

Thanks, applied 0001.

> worth the risk of breakage.

0003 would allow folks to use \1 in a Postgres username if quoted.  My
choice would be to agree with you here.  Even if folks applying quotes
would not be able anymore to replace the pattern, the risk seems a bit
remote?  I would suspect that basically everybody does not rely on
'\1' being in the middle of pg-username string, using it only as a
strict replacement of the result coming from system-username to keep a
simpler mapping between the PG roles and the krb5/gss system roles.
Even if they use a more complex schema that depends on strstr(),
things would break if they began the pg-username with quotes.  Put it
simply, I'd agree with your 0003.

> consideration to the commit message)

I don't see how much that's different from the recent discussion with
regexps added for databases and users to pg_hba.conf.  And consistency
sounds pretty good to me here.

> change that? At least for the regex parsing that is not released yet.

regcomp_auth_token() should not decide to compile a regexp depending
on if an AuthToken is quoted or not.  Regexps can have commas, and
this would impact the case of database or role lists in HBA entries.
And that could be an issue with spaces as well?  See the docs for
patterns like:
db1,"/^db\d{2,4}$",db2

Point taken that we don't care about lists for pg_ident entries,
though.

> You didn't write a response about this, but you did quote it. Did you intend

Nah, I should have deleted it.  I had no useful opinion on this
particular point.
--
Michael

---

From: Jelte Fennema <[email-masked]>
Date: Fri, 13 Jan 2023 09:19:10 +0100

> Even if folks applying quotes
> remote?

Yeah I agree the risk is remote. To be clear, the main pattern I'm
worried about breaking is simply "\1". Where people had put
quotes around \1 for no reason. All in all, I'm fine if 0003 gets
merged, but I'd also be fine with it if it doesn't. Both the risk
and the advantage seem fairly small.

> sounds pretty good to me here.

It's not much different, except that here also all and + change their meaning
(for pg_hba.conf those special cases already existed). Mainly I called it out
because I realised this discussion was called out in that commit too.

> Regexps can have commas

That's a really good reason to allow quoted regexes indeed. Even for pg_ident
entries, commas in unquoted regexes would cause the AuthToken parsing to fail.

Is there anything you still want to see changed about any of the patches?

---

From: Michael Paquier <[email-masked]>
Date: Mon, 16 Jan 2023 14:22:27 +0900

> and the advantage seem fairly small.

Still, I am having a few second thoughts about 0003 after thinking
about it over the weekend.  Except if I am missing something, there
are no issues with 0004 if we keep the current behavior of always
replacing \1 even if pg-user is quoted?  I would certainly add a new
test case either way.

> Is there anything you still want to see changed about any of the patches?

+           /*
+            * Mark the token as quoted, so it will only be compared literally
+            * and not for special meanings like, such as "all" and membership
+            * checks using the + prefix.
+            */
+           expanded_pg_user_token = make_auth_token(expanded_pg_user, true);
It is critical to quote this AuthToken after the replacement, indeed.
Or we are in big trouble.

-           /* no substitution, so copy the match */
-           expanded_pg_user = pstrdup(identLine->pg_user->string);
+           expanded_pg_user_token = identLine->pg_user;
Perhaps it would be simpler to use copy_auth_token() in this code path
and always free the resulting token?

In the code path where system-user is a regexp, could it be better
to skip the replacement of \1 in the new AuthToken if pg-user is
itself a regexp?  The compiled regexp would be the same, but it could
be considered as a bit confusing, as it can be thought that the
compiled regexp of pg-user happened after the replacement?

No issues with 0002 after a second look, so applied to move on.
--
Michael

---

From: Jelte Fennema <[email-masked]>
Date: Mon, 16 Jan 2023 11:53:57 +0100

> replacing \1 even if pg-user is quoted?  I would certainly add a new

Yes, 0004 is not dependent on 003 at all. I attached a new version
of 0003 where only a test and some documentation is added.

> and always free the resulting token?

I initially tried that when working on the patch, but copy_auth_token
(surprisingly) doesn't copy the regex field into the new AuthToken.
So we'd have to regenerate it conditionally. Making the copy
conditional seemed just as simple code-wise, with the added
bonus that it's not doing a useless copy.

> compiled regexp of pg-user happened after the replacement?

I updated 0004 to prioritize membership checks and regexes over
substitution of \1. I also added tests for this. Prioritizing "all" over
substitution of \1 is not necessary, since by definition "all" does
not include \1.

---

From: Michael Paquier <[email-masked]>
Date: Tue, 17 Jan 2023 14:10:18 +0900

> bonus that it's not doing a useless copy.

Okay, I can live with that.

> substitution of \1 is not necessary, since by definition "all" does

Thanks, 0003 is OK, so applied now.

0004 looks fine as well, be it for the tests (I am hesitating to tweak
things a bit here actually for the role names), the code or the docs,
still I am planning a second lookup.
--
Michael

---

From: Jelte Fennema <[email-masked]>
Date: Wed, 18 Jan 2023 10:35:29 +0100

> things a bit here actually for the role names), the code or the docs,

Anything I can do to help with this? Or will you do that yourself?

---

From: Michael Paquier <[email-masked]>
Date: Thu, 19 Jan 2023 10:10:58 +0900

> Anything I can do to help with this? Or will you do that yourself?

Nope.  I just need some time to finish wrapping it, that's all.
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Thu, 19 Jan 2023 16:56:05 +0900

> Anything I can do to help with this? Or will you do that yourself?

So, I have done a second lookup, and tweaked a few things:
- Addition of a macro for pg_strcasecmp(), to match with
token_matches().
- Fixed a bit the documentation.
- Tweaked some comments and descriptions in the tests, I was rather
fine with the role and group names.

Jelte, do you like this version?
--
Michael
From 0e42a5e9c2a532355df49346e47cc5612da1889d Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Thu, 19 Jan 2023 16:54:59 +0900
Subject: [PATCH v6] 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.

There is some risk of breaking existing pg_ident.conf configs that
contained such special strings and which were treated as literal user
names. But the advantages brought by the features added in this change
far outweigh the risk of breakage. And we only risk breaking when there
exist roles that are called "all", start with a literal + or start with
a literal /. Only "all" seems like a somewhat reasonable role name, but
such a role existing seems unlikely to me given all its special meaning
in pg_hba.conf.

**This compatibility change should be mentioned in the release notes.**
---
 src/backend/libpq/hba.c               |  98 +++++++++++-----
 src/test/authentication/t/003_peer.pl | 160 ++++++++++++++++++++++++--
 doc/src/sgml/client-auth.sgml         |  27 ++++-
 3 files changed, 246 insertions(+), 39 deletions(-)

diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 029b8e4483..69f87667be 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -73,8 +73,10 @@ typedef struct
 } tokenize_error_callback_arg;
 
 #define token_has_regexp(t)	(t->regex != NULL)
+#define token_is_member_check(t)	(!t->quoted && t->string[0] == '+')
 #define token_is_keyword(t, k)	(!t->quoted && strcmp(t->string, k) == 0)
 #define token_matches(t, k)  (strcmp(t->string, k) == 0)
+#define token_matches_insensitive(t,k) (pg_strcasecmp(t->string, k) == 0)
 
 /*
  * Memory context holding the list of TokenizedAuthLines when parsing
@@ -995,10 +997,11 @@ is_member(Oid userid, const char *role)
  *
  * Each AuthToken listed is checked one-by-one.  Keywords are processed
  * first (these cannot have regular expressions), followed by regular
- * expressions (if any) and the exact match.
+ * expressions (if any), the case-insensitive match (if requested) 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;
@@ -1006,7 +1009,7 @@ check_role(const char *role, Oid roleid, List *tokens)
 	foreach(cell, tokens)
 	{
 		tok = lfirst(cell);
-		if (!tok->quoted && tok->string[0] == '+')
+		if (token_is_member_check(tok))
 		{
 			if (is_member(roleid, tok->string + 1))
 				return true;
@@ -1018,6 +1021,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 (token_matches_insensitive(tok, role))
+				return true;
+		}
 		else if (token_matches(tok, role))
 			return true;
 	}
@@ -2614,7 +2622,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 +2812,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 +2821,13 @@ 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 +2842,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 +2851,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 +2865,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,8 +2886,16 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
 			return;
 		}
 
-		if ((ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
+		/*
+		 * Replace \1 with the first captured group unless the field already
+		 * has some special meaning, like a group membership or a regexp-based
+		 * check.
+		 */
+		if (!token_is_member_check(identLine->pg_user) &&
+			!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 */
@@ -2891,46 +2920,53 @@ 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);
+
+			/*
+			 * Mark the token as quoted, so it will only be compared literally
+			 * and not for some special meaning, such as "all" or a group
+			 * membership checks.
+			 */
+			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);
+		/* check the Postgres 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 */
+		/*
+		 * Not a regular expression, so make a complete match.  If the system
+		 * user does not match, just leave.
+		 */
 		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 (!token_matches_insensitive(identLine->system_user,
+										   system_user))
+				return;
 		}
 		else
 		{
-			if (strcmp(identLine->pg_user->string, pg_user) == 0 &&
-				strcmp(identLine->system_user->string, system_user) == 0)
-				*found_p = true;
+			if (!token_matches(identLine->system_user, system_user))
+				return;
 		}
+
+		/* check the Postgres user */
+		*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 e6f5fdba16..a6be651ea7 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -98,8 +98,13 @@ 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 a 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");
+# Note the double quotes here.
+$node->safe_psql('postgres', 'CREATE ROLE "testmapgroupliteral\\1" LOGIN');
+$node->safe_psql('postgres', 'GRANT "testmapgroupliteral\\1" TO testmapuser');
 
 # Extract as well the system user for the user name map.
 my $system_user =
@@ -123,12 +128,56 @@ 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');
+
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	0,
+	'with keyword "all" as database user in user name map',
+	log_like =>
+	  [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+# Tests with the "all" keyword, but quoted (no effect here).
+reset_pg_ident($node, 'mypeermap', $system_user, '"all"');
+
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	2,
+	'with quoted keyword "all" as database user in user name map',
+	log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]);
+
+# Success as the regexp of the database user matches
+reset_pg_ident($node, 'mypeermap', $system_user, qq{/^testm.*\$});
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	0,
+	'with regexp of database user in user name map',
+	log_like =>
+	  [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+# Failure as the regexp of the database user does not match.
+reset_pg_ident($node, 'mypeermap', $system_user, qq{/^doesnotmatch.*\$});
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	2,
+	'with bad regexp of database user 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 +185,33 @@ test_role(
 	qq{testmapuser},
 	'peer',
 	0,
-	'with regular expression in user name map',
+	'with regexp of system user in user name map',
 	log_like =>
 	  [qr/connection authenticated: identity="$system_user" method=peer/]);
 
+# Success as both regular expressions match.
+reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$},
+	qq{/^testm.*\$});
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	0,
+	'with regexps for both system and database user 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 regexp of system user and keyword "all" in user name map',
+	log_like =>
+	  [qr/connection authenticated: identity="$system_user" method=peer/]);
 
 # Success as the regular expression matches and \1 is replaced in the given
 # subexpression.
@@ -179,17 +251,91 @@ test_role(
 	]);
 
 # 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 regexp of system user does not match.
+reset_pg_ident($node, 'mypeermap', qq{/^.*$bad_regex_test_string\$},
 	'testmapuser');
 test_role(
 	$node,
 	qq{testmapuser},
 	'peer',
 	2,
-	'with regular expression in user name map',
+	'with regexp of system user in user name map',
 	log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]);
 
+# Test using a group role match for the database 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/]);
+
+# Now apply quotes to the group match, nullifying its effect.
+reset_pg_ident($node, 'mypeermap', $system_user, '"+testmapgroup"');
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	2,
+	'plain user with quoted group name',
+	log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]);
+
+# Test using a regexp for the system user, with a group membership
+# check for the database user.
+reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$},
+	'+testmapgroup');
+
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	0,
+	'regexp of system user as group member',
+	log_like =>
+	  [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+test_role(
+	$node,
+	qq{testmapgroup},
+	'peer',
+	2,
+	'regexp of system user as non-member of group',
+	log_like => [qr/role "testmapgroup" is not permitted to log in/]);
+
+# Test that membership checks and regexes will use literal \1 instead of
+# replacing it, as subexpression replacement is not allowed in this case.
+reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string(.*)\$},
+	'+testmapgroupliteral\\1');
+
+test_role(
+	$node,
+	qq{testmapuser},
+	'peer',
+	0,
+	'membership check with literal \1',
+	log_like =>
+	  [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+# Do the same with a quoted regular expression for the database user this
+# time.  No replacement of \1 is done.
+reset_pg_ident(
+	$node, 'mypeermap',
+	qq{/^.*$regex_test_string(.*)\$},
+	'"/^testmapgroupliteral\\\\1$"');
+
+test_role(
+	$node,
+	'testmapgroupliteral\\\\1',
+	'peer',
+	0,
+	'regexp of database user with literal \1',
+	log_like =>
+	  [qr/connection authenticated: identity="$system_user" method=peer/]);
+
 done_testing();
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index e4959663c4..b9d73deced 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,16 @@ mymap   /^(.*)@otherdomain\.com$   guest
    <literal>\1</literal> <emphasis>does not</emphasis> make
    <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 is not possible to use <literal>\1</literal>
+   to use a capture from regular expression on
+   <replaceable>system-username</replaceable> for a regular expression
+   on <replaceable>database-username</replaceable>.
+  </para>
 
   <tip>
    <para>
-- 
2.39.0