On Tue, Sep 27, 2022 at 03:38:49PM -0700, Jacob Champion wrote: > On 9/26/22 06:29, Drouvot, Bertrand wrote: > Since there are only internal clients to the API, I'd argue this makes > more sense as an Assert(authn_id != NULL), but I don't think it's a > dealbreaker.
Using an assert() looks like a good idea from here. If this is called with a NULL authn, this could reflect a problem in the authentication logic. >> 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. > > Unfortunately I don't have much to add here; I don't know enough about > the underlying problems. I have been looking at that, and after putting my hands on that this comes down to the facility introduced in 40c24bf. So, I think that we'd better use COERCE_SQL_SYNTAX so as there is no need to worry about the shortcuts this patch is trying to use with the collation setup. And there are a few tests for get_func_sql_syntax() in create_view.sql. Note that this makes the patch slightly shorter, and simpler. The docs still mentioned "name", and not "text". This brings in a second point. 40c24bf has refrained from removing SQLValueFunction, but based the experience on this thread I see a pretty good argument in doing the jump once and for all. This deserves a separate discussion, though. I'll do that and create a new thread. -- Michael
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 8b72f8a215..e89703a3bf 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/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/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 8cba888223..ee0985c7ed 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/parser/gram.y b/src/backend/parser/gram.y
index 0d8d292850..94d5142a4a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -743,7 +743,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,13 @@ func_expr_common_subexpr:
{
$$ = makeSQLValueFunction(SVFOP_SESSION_USER, -1, @1);
}
+ | SYSTEM_USER
+ {
+ $$ = (Node *) makeFuncCall(SystemFuncName("system_user"),
+ NIL,
+ COERCE_SQL_SYNTAX,
+ @1);
+ }
| USER
{
$$ = makeSQLValueFunction(SVFOP_USER, -1, @1);
@@ -17120,6 +17127,7 @@ reserved_keyword:
| SESSION_USER
| SOME
| SYMMETRIC
+ | SYSTEM_USER
| TABLE
| THEN
| TO
@@ -17500,6 +17508,7 @@ bare_label_keyword:
| SYMMETRIC
| SYSID
| SYSTEM_P
+ | SYSTEM_USER
| TABLE
| TABLES
| TABLESAMPLE
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 2b7b1b0c0f..c418403537 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -10317,6 +10317,10 @@ get_func_sql_syntax(FuncExpr *expr, deparse_context *context)
appendStringInfoChar(buf, ')');
return true;
+ case F_SYSTEM_USER:
+ appendStringInfoString(buf, "SYSTEM_USER");
+ return true;
+
case F_XMLEXISTS:
/* XMLEXISTS ... extra parens because args are c_expr */
appendStringInfoString(buf, "XMLEXISTS((");
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 683f616b1a..c9e55cdefd 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.
+ *
+ * This is built as auth_method:authn_id.
+ */
+void
+InitializeSystemUser(const char *authn_id, const char *auth_method)
+{
+ char *system_user;
+
+ /* call only once */
+ Assert(SystemUser == NULL);
+
+ /*
+ * InitializeSystemUser should be called only when authn_id is not NULL,
+ * meaning that auth_method is valid.
+ */
+ Assert(authn_id != NULL);
+
+ 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/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 47169a1d1e..a097e43afa 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
@@ -180,6 +180,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.
@@ -311,6 +318,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});
diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
index a828b1f6de..bf4ff30d86 100644
--- a/src/test/regress/expected/create_view.out
+++ b/src/test/regress/expected/create_view.out
@@ -1940,7 +1940,8 @@ select
trim(trailing ' foo ') as rt,
trim(E'\\000'::bytea from E'\\000Tom\\000'::bytea) as btb,
trim(leading E'\\000'::bytea from E'\\000Tom\\000'::bytea) as ltb,
- trim(trailing E'\\000'::bytea from E'\\000Tom\\000'::bytea) as rtb;
+ trim(trailing E'\\000'::bytea from E'\\000Tom\\000'::bytea) as rtb,
+ SYSTEM_USER as su;
select pg_get_viewdef('tt201v', true);
pg_get_viewdef
-----------------------------------------------------------------------------------------------
@@ -1961,7 +1962,8 @@ select pg_get_viewdef('tt201v', true);
TRIM(TRAILING FROM ' foo '::text) AS rt, +
TRIM(BOTH '\x00'::bytea FROM '\x00546f6d00'::bytea) AS btb, +
TRIM(LEADING '\x00'::bytea FROM '\x00546f6d00'::bytea) AS ltb, +
- TRIM(TRAILING '\x00'::bytea FROM '\x00546f6d00'::bytea) AS rtb;
+ TRIM(TRAILING '\x00'::bytea FROM '\x00546f6d00'::bytea) AS rtb, +
+ SYSTEM_USER AS su;
(1 row)
-- corner cases with empty join conditions
diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql
index 44a6775f90..913b4ee460 100644
--- a/src/test/regress/sql/create_view.sql
+++ b/src/test/regress/sql/create_view.sql
@@ -721,7 +721,8 @@ select
trim(trailing ' foo ') as rt,
trim(E'\\000'::bytea from E'\\000Tom\\000'::bytea) as btb,
trim(leading E'\\000'::bytea from E'\\000Tom\\000'::bytea) as ltb,
- trim(trailing E'\\000'::bytea from E'\\000Tom\\000'::bytea) as rtb;
+ trim(trailing E'\\000'::bytea from E'\\000Tom\\000'::bytea) as rtb,
+ SYSTEM_USER as su;
select pg_get_viewdef('tt201v', true);
-- corner cases with empty join conditions
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index d8718ed61e..409967f754 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>text</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>
signature.asc
Description: PGP signature
