public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jelte Fennema <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Jelte Fennema <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Nathan Bossart <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Subject: Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
Date: Thu, 12 Jan 2023 10:10:02 +0100
Message-ID: <CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <DBBPR83MB0507FEC2E8965012990A80D0F7FC9@DBBPR83MB0507.EURPRD83.prod.outlook.com>
<[email protected]>
<CAGECzQTkwELHUOAKhvdA+m3tWbUQySHHkExJV8GAZ1pwgbEgXg@mail.gmail.com>
<[email protected]>
> 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
> independently of the rest, actually, to provide some coverage of the
> substitution case in the peer test.
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)
> > 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.
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.
Attachments:
[application/octet-stream] v4-0003-Only-expand-1-in-pg_ident.conf-when-not-quoted.patch (2.6K, ../CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com/2-v4-0003-Only-expand-1-in-pg_ident.conf-when-not-quoted.patch)
download | inline diff:
From a789b47c401d263ff065cca1ca98b0e33deb0c7f Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Thu, 12 Jan 2023 09:23:45 +0100
Subject: [PATCH v4 3/4] Only expand \1 in pg_ident.conf when not quoted
While unlikely, it's possible for usernames to contain the literal
character sequence '\1'. This allows quoting the database-username in
a pg_ident.conf file such that it's possible to match against a literal
'\1' character sequence.
---
doc/src/sgml/client-auth.sgml | 3 +++
src/backend/libpq/hba.c | 3 ++-
src/test/authentication/t/003_peer.pl | 12 ++++++++++++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index cc8c59206c9..08a25a5e002 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -960,6 +960,9 @@ mymap /^(.*)@otherdomain\.com$ guest
will remove the domain part for users with system user names that end with
<literal>@mydomain.com</literal>, and allow any user whose system name ends with
<literal>@otherdomain.com</literal> to log in as <literal>guest</literal>.
+ Quoting a <replaceable>database-username</replaceable> containing
+ <literal>\1</literal> makes the <literal>\1</literal>
+ lose its special meaning.
</para>
<tip>
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 029b8e44838..0c00580dff0 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -2865,7 +2865,8 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
return;
}
- if ((ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
+ if (!identLine->pg_user->quoted &&
+ (ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
{
int offset;
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index c41f146b35e..9f397ee220a 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -153,6 +153,18 @@ test_role(
log_like =>
[qr/connection authenticated: identity="$system_user" method=peer/]);
+# Failure since the \1 is part of a quoted string
+reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$},
+ '"test\1mapuser"');
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 2,
+ 'with regular expression in user name map with a quoted \1',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
+
# Failure as the regular expression doesn't contain a group, but the database
# user contains \1
reset_pg_ident($node, 'mypeermap', qq{/^$system_user\$},
--
2.34.1
[application/octet-stream] v4-0001-Add-tests-for-1-in-pg_ident.conf-to-0003_peer.pl.patch (2.0K, ../CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com/3-v4-0001-Add-tests-for-1-in-pg_ident.conf-to-0003_peer.pl.patch)
download | inline diff:
From b8bd5bc0dfce59ddf32ba8510156350aed40ea31 Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Thu, 12 Jan 2023 09:18:19 +0100
Subject: [PATCH v4 1/4] Add tests for \1 in pg_ident.conf to 0003_peer.pl
There were no tests for \1 substitution in our main pg_ident.conf tests.
The reason was probably that it's not obvious how to get a consistent
capturing group given an arbitrary username. To get a consistent
capturing group value this patch always captures an empty string. While
it would be better to test a substitution where the group actually
captures some characters, the newly added test is better than having no
test at all.
---
src/test/authentication/t/003_peer.pl | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 24cefd14e0a..c41f146b35e 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -141,6 +141,30 @@ test_role(
[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(.*)\$},
+ '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/]);
+
+# Failure as the regular expression doesn't contain a group, but the database
+# user contains \1
+reset_pg_ident($node, 'mypeermap', qq{/^$system_user\$},
+ '\1testmapuser');
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 2,
+ 'with regular expression in user name map with \1 without a matching group',
+ 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;
--
2.34.1
[application/octet-stream] v4-0002-Store-pg_user-as-AuthToken-in-IdentLine.patch (4.7K, ../CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com/4-v4-0002-Store-pg_user-as-AuthToken-in-IdentLine.patch)
download | inline diff:
From 150fbec895d59b7bffc48ecf0d84ba003867321c Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Wed, 11 Jan 2023 11:51:27 +0100
Subject: [PATCH v4 2/4] Store pg_user as AuthToken in IdentLine
While system_user was stored as an AuthToken in IdentLine, pg_user was
stored as a plain string. This starts storing pg_user as an AuthToken
too in preparation of follow-up commits that start checking if pg_user
was quoted or not.
---
src/backend/libpq/hba.c | 20 +++++++++++---------
src/backend/utils/adt/hbafuncs.c | 2 +-
src/include/libpq/hba.h | 2 +-
3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 154b2857d2a..029b8e44838 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -2800,7 +2800,7 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
tokens = lfirst(field);
IDENT_MULTI_VALUE(tokens);
token = linitial(tokens);
- parsedline->pg_user = pstrdup(token->string);
+ parsedline->pg_user = copy_auth_token(token);
/*
* Now that the field validation is done, compile a regex from the user
@@ -2865,7 +2865,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
return;
}
- if ((ofs = strstr(identLine->pg_user, "\\1")) != NULL)
+ if ((ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
{
int offset;
@@ -2875,7 +2875,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
ereport(LOG,
(errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),
errmsg("regular expression \"%s\" has no subexpressions as requested by backreference in \"%s\"",
- identLine->system_user->string + 1, identLine->pg_user)));
+ identLine->system_user->string + 1, identLine->pg_user->string)));
*error_p = true;
return;
}
@@ -2884,9 +2884,9 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
* length: original length minus length of \1 plus length of match
* plus null terminator
*/
- expanded_pg_user = palloc0(strlen(identLine->pg_user) - 2 + (matches[1].rm_eo - matches[1].rm_so) + 1);
- offset = ofs - identLine->pg_user;
- memcpy(expanded_pg_user, identLine->pg_user, offset);
+ expanded_pg_user = palloc0(strlen(identLine->pg_user->string) - 2 + (matches[1].rm_eo - matches[1].rm_so) + 1);
+ offset = ofs - identLine->pg_user->string;
+ memcpy(expanded_pg_user, identLine->pg_user->string, offset);
memcpy(expanded_pg_user + offset,
system_user + matches[1].rm_so,
matches[1].rm_eo - matches[1].rm_so);
@@ -2895,7 +2895,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
else
{
/* no substitution, so copy the match */
- expanded_pg_user = pstrdup(identLine->pg_user);
+ expanded_pg_user = pstrdup(identLine->pg_user->string);
}
/*
@@ -2921,13 +2921,13 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
/* Not regular expression, so make complete match */
if (case_insensitive)
{
- if (pg_strcasecmp(identLine->pg_user, pg_user) == 0 &&
+ if (pg_strcasecmp(identLine->pg_user->string, pg_user) == 0 &&
pg_strcasecmp(identLine->system_user->string, system_user) == 0)
*found_p = true;
}
else
{
- if (strcmp(identLine->pg_user, pg_user) == 0 &&
+ if (strcmp(identLine->pg_user->string, pg_user) == 0 &&
strcmp(identLine->system_user->string, system_user) == 0)
*found_p = true;
}
@@ -3074,6 +3074,7 @@ load_ident(void)
{
newline = (IdentLine *) lfirst(parsed_line_cell);
free_auth_token(newline->system_user);
+ free_auth_token(newline->pg_user);
}
MemoryContextDelete(ident_context);
return false;
@@ -3086,6 +3087,7 @@ load_ident(void)
{
newline = (IdentLine *) lfirst(parsed_line_cell);
free_auth_token(newline->system_user);
+ free_auth_token(newline->pg_user);
}
}
if (parsed_ident_context != NULL)
diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c
index 8a552ef8e9d..73d3ad1dadc 100644
--- a/src/backend/utils/adt/hbafuncs.c
+++ b/src/backend/utils/adt/hbafuncs.c
@@ -493,7 +493,7 @@ fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
{
values[index++] = CStringGetTextDatum(ident->usermap);
values[index++] = CStringGetTextDatum(ident->system_user->string);
- values[index++] = CStringGetTextDatum(ident->pg_user);
+ values[index++] = CStringGetTextDatum(ident->pg_user->string);
}
else
{
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index ed4d5e7962c..189f6d0df24 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -143,7 +143,7 @@ typedef struct IdentLine
char *usermap;
AuthToken *system_user;
- char *pg_user;
+ AuthToken *pg_user;
} IdentLine;
/*
--
2.34.1
[application/octet-stream] v4-0004-Support-same-user-patterns-in-pg_ident.conf-as-in.patch (14.4K, ../CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com/5-v4-0004-Support-same-user-patterns-in-pg_ident.conf-as-in.patch)
download | inline diff:
From 91cce919109fadd3fc60c5f041eb97542f692c5f Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Wed, 11 Jan 2023 12:10:19 +0100
Subject: [PATCH v4 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.
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.**
---
doc/src/sgml/client-auth.sgml | 26 +++++-
src/backend/libpq/hba.c | 74 +++++++++------
src/test/authentication/t/003_peer.pl | 125 ++++++++++++++++++++++++--
3 files changed, 190 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..73b92506304 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)
@@ -2868,6 +2887,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
if (!identLine->pg_user->quoted &&
(ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
{
+ char *expanded_pg_user;
int offset;
/* substitution of the first argument requested */
@@ -2892,46 +2912,44 @@ 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 special meanings like, such as "all" and membership
+ * checks using the + prefix.
+ */
+ 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 9f397ee220a..64b16025433 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(.*)\$},
@@ -178,10 +240,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,
@@ -191,4 +253,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
view thread (13+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
In-Reply-To: <CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox