Hi,
On 9/8/22 3:17 AM, Michael Paquier wrote:
On Wed, Sep 07, 2022 at 08:48:43AM -0700, Jacob Champion wrote:
We could pass the bare auth_method index, or update the documentation
for auth_method to state that it's guaranteed to be zero if authn_id is
NULL (and then enforce that).
case SVFOP_CURRENT_USER:
case SVFOP_USER:
case SVFOP_SESSION_USER:
+ case SVFOP_SYSTEM_USER:
case SVFOP_CURRENT_CATALOG:
case SVFOP_CURRENT_SCHEMA:
svf->type = NAMEOID;
Should this be moved to use TEXTOID instead?
Yeah, it should. There is actually a second and much deeper issue
here, in the shape of a collation problem. See the assertion failure
in exprSetCollation(), because we expect SQLValueFunction nodes to
return a name or a non-collatable type. However, for this case, we'd
require a text to get rid of the 63-character limit, and that's
a collatable type. This reminds me of the recent thread to work on
getting rid of this limit for the name type..
Please find attached V4 taking care of Jacob's previous comments.
As far the assertion failure mentioned by Michael when moving the
SVFOP_SYSTEM_USER from NAMEOID to TEXTOID: V4 is assuming that it is
safe to force the collation to C_COLLATION_OID for SQLValueFunction
having a TEXT type, but I would be happy to also hear your thoughts
about it.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index e1fe4fec57..fe99e65dd5 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -22623,6 +22623,25 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS
t(ls,n);
</para></entry>
</row>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>system_user</primary>
+ </indexterm>
+ <function>system_user</function>
+ <returnvalue>name</returnvalue>
+ </para>
+ <para>
+ Returns the authentication method and the identity (if any) that the
+ user presented during the authentication cycle before they were
+ assigned a database role. It is represented as
+ <literal>auth_method:identity</literal> or
+ <literal>NULL</literal> if the user has not been authenticated (for
+ example if <link linkend="auth-trust">Trust authentication</link> has
+ been used).
+ </para></entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
diff --git a/src/backend/access/transam/parallel.c
b/src/backend/access/transam/parallel.c
index bc93101ff7..c2a08e9414 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -1496,6 +1496,14 @@ ParallelWorkerMain(Datum main_arg)
false);
RestoreClientConnectionInfo(clientconninfospace);
+ /*
+ * Initialize SystemUser now that MyClientConnectionInfo is restored.
+ * Also ensure that auth_method is actually valid, aka authn_id is not
NULL.
+ */
+ if (MyClientConnectionInfo.authn_id)
+ InitializeSystemUser(MyClientConnectionInfo.authn_id,
+
hba_authname(MyClientConnectionInfo.auth_method));
+
/* Attach to the leader's serializable transaction, if SERIALIZABLE. */
AttachSerializableXact(fps->serializable_xact_handle);
diff --git a/src/backend/executor/execExprInterp.c
b/src/backend/executor/execExprInterp.c
index 9b9bbf00a9..c51578c0b9 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -2537,6 +2537,11 @@ ExecEvalSQLValueFunction(ExprState *state, ExprEvalStep
*op)
*op->resvalue = session_user(fcinfo);
*op->resnull = fcinfo->isnull;
break;
+ case SVFOP_SYSTEM_USER:
+ InitFunctionCallInfoData(*fcinfo, NULL, 0, InvalidOid,
NULL, NULL);
+ *op->resvalue = system_user(fcinfo);
+ *op->resnull = fcinfo->isnull;
+ break;
case SVFOP_CURRENT_CATALOG:
InitFunctionCallInfoData(*fcinfo, NULL, 0, InvalidOid,
NULL, NULL);
*op->resvalue = current_database(fcinfo);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 3bac350bf5..453ba84494 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -1142,7 +1142,8 @@ exprSetCollation(Node *expr, Oid collation)
((MinMaxExpr *) expr)->minmaxcollid = collation;
break;
case T_SQLValueFunction:
- Assert((((SQLValueFunction *) expr)->type == NAMEOID) ?
+ Assert((((SQLValueFunction *) expr)->type == NAMEOID ||
+ ((SQLValueFunction *) expr)->type ==
TEXTOID) ?
(collation == C_COLLATION_OID) :
(collation == InvalidOid));
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 82f03fc9c9..480e6d8c4b 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -744,7 +744,7 @@ static Node *makeRecursiveViewSelect(char *relname, List
*aliases, Node *query);
SERIALIZABLE SERVER SESSION SESSION_USER SET SETS SETOF SHARE SHOW
SIMILAR SIMPLE SKIP SMALLINT SNAPSHOT SOME SQL_P STABLE STANDALONE_P
START STATEMENT STATISTICS STDIN STDOUT STORAGE STORED STRICT_P STRIP_P
- SUBSCRIPTION SUBSTRING SUPPORT SYMMETRIC SYSID SYSTEM_P
+ SUBSCRIPTION SUBSTRING SUPPORT SYMMETRIC SYSID SYSTEM_P SYSTEM_USER
TABLE TABLES TABLESAMPLE TABLESPACE TEMP TEMPLATE TEMPORARY TEXT_P THEN
TIES TIME TIMESTAMP TO TRAILING TRANSACTION TRANSFORM
@@ -15239,6 +15239,10 @@ func_expr_common_subexpr:
{
$$ =
makeSQLValueFunction(SVFOP_SESSION_USER, -1, @1);
}
+ | SYSTEM_USER
+ {
+ $$ =
makeSQLValueFunction(SVFOP_SYSTEM_USER, -1, @1);
+ }
| USER
{
$$ = makeSQLValueFunction(SVFOP_USER,
-1, @1);
@@ -17120,6 +17124,7 @@ reserved_keyword:
| SESSION_USER
| SOME
| SYMMETRIC
+ | SYSTEM_USER
| TABLE
| THEN
| TO
@@ -17500,6 +17505,7 @@ bare_label_keyword:
| SYMMETRIC
| SYSID
| SYSTEM_P
+ | SYSTEM_USER
| TABLE
| TABLES
| TABLESAMPLE
diff --git a/src/backend/parser/parse_collate.c
b/src/backend/parser/parse_collate.c
index 7582faabb3..95d21dab67 100644
--- a/src/backend/parser/parse_collate.c
+++ b/src/backend/parser/parse_collate.c
@@ -707,6 +707,19 @@ assign_collations_walker(Node *node,
assign_collations_context *context)
* Now figure out what collation to assign to
this node.
*/
typcollation = get_typcollation(exprType(node));
+
+ /*
+ * If this is a SQLValueFunction with a TEXT
type, then
+ * assign its collation to the standard C
collation.
+ */
+ if (IsA(node, SQLValueFunction))
+ {
+ if (((SQLValueFunction *) node)->type
== TEXTOID)
+ {
+ typcollation = C_COLLATION_OID;
+ }
+ }
+
if (OidIsValid(typcollation))
{
/* Node's result is collatable; what
about its input? */
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 7aaf1c673f..a9c5c48102 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -2239,6 +2239,9 @@ transformSQLValueFunction(ParseState *pstate,
SQLValueFunction *svf)
case SVFOP_CURRENT_SCHEMA:
svf->type = NAMEOID;
break;
+ case SVFOP_SYSTEM_USER:
+ svf->type = TEXTOID;
+ break;
}
return (Node *) svf;
diff --git a/src/backend/parser/parse_target.c
b/src/backend/parser/parse_target.c
index bd8057bc3e..1cd08a65e8 100644
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -1907,6 +1907,9 @@ FigureColnameInternal(Node *node, char **name)
case SVFOP_SESSION_USER:
*name = "session_user";
return 2;
+ case SVFOP_SYSTEM_USER:
+ *name = "system_user";
+ return 2;
case SVFOP_CURRENT_CATALOG:
*name = "current_catalog";
return 2;
diff --git a/src/backend/utils/adt/ruleutils.c
b/src/backend/utils/adt/ruleutils.c
index 2b7b1b0c0f..c621b4328e 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -9210,6 +9210,9 @@ get_rule_expr(Node *node, deparse_context *context,
case SVFOP_SESSION_USER:
appendStringInfoString(buf,
"SESSION_USER");
break;
+ case SVFOP_SYSTEM_USER:
+ appendStringInfoString(buf,
"SYSTEM_USER");
+ break;
case SVFOP_CURRENT_CATALOG:
appendStringInfoString(buf,
"CURRENT_CATALOG");
break;
diff --git a/src/backend/utils/init/miscinit.c
b/src/backend/utils/init/miscinit.c
index 683f616b1a..fa38a76c37 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -477,6 +477,7 @@ static Oid AuthenticatedUserId = InvalidOid;
static Oid SessionUserId = InvalidOid;
static Oid OuterUserId = InvalidOid;
static Oid CurrentUserId = InvalidOid;
+static const char *SystemUser = NULL;
/* We also have to remember the superuser state of some of these levels */
static bool AuthenticatedUserIsSuperuser = false;
@@ -548,6 +549,16 @@ SetSessionUserId(Oid userid, bool is_superuser)
CurrentUserId = userid;
}
+/*
+ * Return the system user representing the authenticated identity.
+ * It is defined in InitializeSystemUser() as auth_method:authn_id.
+ */
+const char *
+GetSystemUser(void)
+{
+ return SystemUser;
+}
+
/*
* GetAuthenticatedUserId - get the authenticated user ID
*/
@@ -818,6 +829,45 @@ InitializeSessionUserIdStandalone(void)
SetSessionUserId(BOOTSTRAP_SUPERUSERID, true);
}
+/*
+ * Initialize the system user.
+ */
+void
+InitializeSystemUser(const char *authn_id, const char *auth_method)
+{
+ /* call only once */
+ Assert(SystemUser == NULL);
+
+ /*
+ * InitializeSystemUser should already be called once we are sure that
+ * authn_id is not NULL (means auth_method is actually valid).
+ * But keep the test here also for safety.
+ */
+ if (authn_id)
+ {
+ /* Build system user as auth_method:authn_id */
+ char *system_user;
+
+ system_user = psprintf("%s:%s", auth_method, authn_id);
+ /* Store SystemUser in long-lived storage */
+ SystemUser = MemoryContextStrdup(TopMemoryContext, system_user);
+ pfree(system_user);
+ }
+}
+
+/*
+ * SQL-function SYSTEM_USER
+ */
+Datum
+system_user(PG_FUNCTION_ARGS)
+{
+ const char *sysuser = GetSystemUser();
+
+ if (sysuser)
+ PG_RETURN_DATUM(CStringGetTextDatum(sysuser));
+ else
+ PG_RETURN_NULL();
+}
/*
* Change session auth ID while running
diff --git a/src/backend/utils/init/postinit.c
b/src/backend/utils/init/postinit.c
index 4a207a7391..31b7e1de5d 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -904,6 +904,10 @@ InitPostgres(const char *in_dbname, Oid dboid,
Assert(MyProcPort != NULL);
PerformAuthentication(MyProcPort);
InitializeSessionUserId(username, useroid);
+ /* ensure that auth_method is actually valid, aka authn_id is
not NULL */
+ if (MyClientConnectionInfo.authn_id)
+ InitializeSystemUser(MyClientConnectionInfo.authn_id,
+
hba_authname(MyClientConnectionInfo.auth_method));
am_superuser = superuser();
}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index a07e737a33..68bb032d3e 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -1508,6 +1508,9 @@
{ oid => '746', descr => 'session user name',
proname => 'session_user', provolatile => 's', prorettype => 'name',
proargtypes => '', prosrc => 'session_user' },
+{ oid => '9977', descr => 'system user name',
+ proname => 'system_user', provolatile => 's', prorettype => 'text',
+ proargtypes => '', prosrc => 'system_user' },
{ oid => '744',
proname => 'array_eq', prorettype => 'bool',
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index ee48e392ed..e7ebea4ff4 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -357,6 +357,9 @@ extern void InitializeSessionUserIdStandalone(void);
extern void SetSessionAuthorization(Oid userid, bool is_superuser);
extern Oid GetCurrentRoleId(void);
extern void SetCurrentRoleId(Oid roleid, bool is_superuser);
+extern void InitializeSystemUser(const char *authn_id,
+ const char
*auth_method);
+extern const char *GetSystemUser(void);
/* in utils/misc/superuser.c */
extern bool superuser(void); /* current user is superuser */
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 40661334bb..74fbc6a4af 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1318,6 +1318,7 @@ typedef enum SQLValueFunctionOp
SVFOP_CURRENT_USER,
SVFOP_USER,
SVFOP_SESSION_USER,
+ SVFOP_SYSTEM_USER,
SVFOP_CURRENT_CATALOG,
SVFOP_CURRENT_SCHEMA
} SQLValueFunctionOp;
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index 9a7cc0c6bd..ccc927851c 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -409,6 +409,7 @@ PG_KEYWORD("support", SUPPORT, UNRESERVED_KEYWORD,
BARE_LABEL)
PG_KEYWORD("symmetric", SYMMETRIC, RESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("sysid", SYSID, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("system", SYSTEM_P, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("system_user", SYSTEM_USER, RESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("table", TABLE, RESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("tables", TABLES, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("tablesample", TABLESAMPLE, TYPE_FUNC_NAME_KEYWORD, BARE_LABEL)
diff --git a/src/test/authentication/t/001_password.pl
b/src/test/authentication/t/001_password.pl
index 3e3079c824..b0b7aac4c0 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -72,6 +72,11 @@ $node->safe_psql('postgres',
$node->safe_psql('postgres',
"SET password_encryption='md5'; CREATE ROLE md5_role LOGIN PASSWORD
'pass';"
);
+# Set up a table for tests of SYSTEM_USER.
+$node->safe_psql(
+ 'postgres',
+ "CREATE TABLE sysuser_data (n) AS SELECT NULL FROM generate_series(1,
10);
+ GRANT ALL ON sysuser_data TO md5_role;");
$ENV{"PGPASSWORD"} = 'pass';
# For "trust" method, all users should be able to connect. These users are not
@@ -82,6 +87,25 @@ test_role($node, 'scram_role', 'trust', 0,
test_role($node, 'md5_role', 'trust', 0,
log_unlike => [qr/connection authenticated:/]);
+# SYSTEM_USER is null when not authenticated.
+my $res = $node->safe_psql('postgres', "SELECT SYSTEM_USER IS NULL;");
+is($res, 't', "users with trust authentication use SYSTEM_USER = NULL");
+
+# Test SYSTEM_USER with parallel workers when not authenticated.
+$res = $node->safe_psql(
+ 'postgres', "
+ SET min_parallel_table_scan_size TO 0;
+ SET parallel_setup_cost TO 0;
+ SET parallel_tuple_cost TO 0;
+ SET max_parallel_workers_per_gather TO 2;
+
+ SELECT bool_and(SYSTEM_USER IS NOT DISTINCT FROM n) FROM sysuser_data;
+ ",
+ connstr => "user=md5_role");
+is($res, 't',
+ "users with trust authentication use SYSTEM_USER = NULL in parallel
workers"
+);
+
# For plain "password" method, all users should also be able to connect.
reset_pg_hba($node, 'password');
test_role($node, 'scram_role', 'password', 0,
@@ -120,6 +144,26 @@ test_role($node, 'md5_role', 'md5', 0,
log_like =>
[qr/connection authenticated: identity="md5_role" method=md5/]);
+# Test SYSTEM_USER <> NULL with parallel workers.
+$node->safe_psql(
+ 'postgres',
+ "TRUNCATE sysuser_data;
+INSERT INTO sysuser_data SELECT 'md5:md5_role' FROM generate_series(1, 10);",
+ connstr => "user=md5_role");
+$res = $node->safe_psql(
+ 'postgres', "
+ SET min_parallel_table_scan_size TO 0;
+ SET parallel_setup_cost TO 0;
+ SET parallel_tuple_cost TO 0;
+ SET max_parallel_workers_per_gather TO 2;
+
+ SELECT bool_and(SYSTEM_USER IS NOT DISTINCT FROM n) FROM sysuser_data;
+ ",
+ connstr => "user=md5_role");
+is($res, 't',
+ "users with md5 authentication use SYSTEM_USER = md5:role in parallel
workers"
+);
+
# Tests for channel binding without SSL.
# Using the password authentication method; channel binding can't work
reset_pg_hba($node, 'password');
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index 62e0542639..f7d8228b24 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -4,8 +4,8 @@
# Sets up a KDC and then runs a variety of tests to make sure that the
# GSSAPI/Kerberos authentication and encryption are working properly,
# that the options in pg_hba.conf and pg_ident.conf are handled correctly,
-# and that the server-side pg_stat_gssapi view reports what we expect to
-# see for each test.
+# that the server-side pg_stat_gssapi view reports what we expect to
+# see for each test and that SYSTEM_USER returns what we expect to see.
#
# Since this requires setting up a full KDC, it doesn't make much sense
# to have multiple test scripts (since they'd have to also create their
@@ -176,6 +176,13 @@ $node->start;
$node->safe_psql('postgres', 'CREATE USER test1;');
+# Set up a table for SYSTEM_USER parallel worker testing.
+$node->safe_psql('postgres',
+ "CREATE TABLE ids (id) AS SELECT 'gss:test1\@$realm' FROM
generate_series(1, 10);"
+);
+
+$node->safe_psql('postgres', 'GRANT SELECT ON ids TO public;');
+
note "running tests";
# Test connection success or failure, and if success, that query returns true.
@@ -307,6 +314,23 @@ test_query(
'gssencmode=require',
'sending 100K lines works');
+# Test that SYSTEM_USER works.
+test_query($node, 'test1', 'SELECT SYSTEM_USER;',
+ qr/^gss:test1\@$realm$/s, 'gssencmode=require', 'testing system_user');
+
+# Test that SYSTEM_USER works with parallel workers.
+test_query(
+ $node,
+ 'test1',
+ "SET min_parallel_table_scan_size TO 0;\n"
+ . "SET parallel_setup_cost TO 0;\n"
+ . "SET parallel_tuple_cost TO 0;\n"
+ . "SET max_parallel_workers_per_gather TO 2;\n"
+ . "SELECT bool_and(SYSTEM_USER = id) FROM ids;",
+ qr/^t$/s,
+ 'gssencmode=require',
+ 'testing system_user with parallel workers');
+
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
qq{hostgssenc all all $hostaddr/32 gss map=mymap});
