Should we remove db_user_namespace?

Started by Nathan Bossartover 2 years ago17 messages

nathandbossart@gmail.com

over 2 years ago

1 attachment(s)

I think this is the second decennial thread [0]/messages/by-id/CAA-aLv6wnwp6Qr5fZo+7K=VSr51qFMuLsCeYvTkSF3E5qEPvqA@mail.gmail.com for removing this GUC.
This topic came up at PGCon, so I thought I'd start the discussion on the
lists.

I'm personally not aware of anyone using this parameter. A couple of my
colleagues claimed to have used it in the aughts, but they also noted that
users were confused by the current implementation, and they seemed
generally in favor of removing it. AFAICT the strongest reason for keeping
it is that there is presently no viable replacement. Does this opinion
still stand? If so, perhaps we can look into adding a viable replacement
for v17.

The attached patch simply removes the GUC.

[0]: /messages/by-id/CAA-aLv6wnwp6Qr5fZo+7K=VSr51qFMuLsCeYvTkSF3E5qEPvqA@mail.gmail.com

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Attachments:

v1-0001-remove-db_user_namespace.patchtext/x-diff; charset=us-asciiDownload

From 6677f4b98fd0b1bd68e07d773b04caf45cf27715 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Fri, 30 Jun 2023 12:46:08 -0700
Subject: [PATCH v1 1/1] remove db_user_namespace

---
 doc/src/sgml/config.sgml                      | 52 -------------------
 src/backend/libpq/auth.c                      |  5 --
 src/backend/libpq/hba.c                       | 12 -----
 src/backend/postmaster/postmaster.c           | 19 -------
 src/backend/utils/misc/guc_tables.c           |  9 ----
 src/backend/utils/misc/postgresql.conf.sample |  1 -
 src/include/libpq/pqcomm.h                    |  2 -
 7 files changed, 100 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6262cb7bb2..e6cea8ddfc 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1188,58 +1188,6 @@ include_dir 'conf.d'
        </para>
       </listitem>
      </varlistentry>
-
-     <varlistentry id="guc-db-user-namespace" xreflabel="db_user_namespace">
-      <term><varname>db_user_namespace</varname> (<type>boolean</type>)
-      <indexterm>
-       <primary><varname>db_user_namespace</varname> configuration parameter</primary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        This parameter enables per-database user names.  It is off by default.
-        This parameter can only be set in the <filename>postgresql.conf</filename>
-        file or on the server command line.
-       </para>
-
-       <para>
-        If this is on, you should create users as <replaceable>username@dbname</replaceable>.
-        When <replaceable>username</replaceable> is passed by a connecting client,
-        <literal>@</literal> and the database name are appended to the user
-        name and that database-specific user name is looked up by the
-        server. Note that when you create users with names containing
-        <literal>@</literal> within the SQL environment, you will need to
-        quote the user name.
-       </para>
-
-       <para>
-        With this parameter enabled, you can still create ordinary global
-        users.  Simply append <literal>@</literal> when specifying the user
-        name in the client, e.g., <literal>joe@</literal>.  The <literal>@</literal>
-        will be stripped off before the user name is looked up by the
-        server.
-       </para>
-
-       <para>
-        <varname>db_user_namespace</varname> causes the client's and
-        server's user name representation to differ.
-        Authentication checks are always done with the server's user name
-        so authentication methods must be configured for the
-        server's user name, not the client's.  Because
-        <literal>md5</literal> uses the user name as salt on both the
-        client and server, <literal>md5</literal> cannot be used with
-        <varname>db_user_namespace</varname>.
-       </para>
-
-       <note>
-        <para>
-         This feature is intended as a temporary measure until a
-         complete solution is found.  At that time, this option will
-         be removed.
-        </para>
-       </note>
-      </listitem>
-     </varlistentry>
      </variablelist>
      </sect2>
 
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index a98b934a8e..65d452f099 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -873,11 +873,6 @@ CheckMD5Auth(Port *port, char *shadow_pass, const char **logdetail)
 	char	   *passwd;
 	int			result;
 
-	if (Db_user_namespace)
-		ereport(FATAL,
-				(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
-				 errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled")));
-
 	/* include the salt to use for computing the response */
 	if (!pg_strong_random(md5Salt, 4))
 	{
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index f89f138f3c..5d4ddbb04d 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -1741,19 +1741,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel)
 	else if (strcmp(token->string, "reject") == 0)
 		parsedline->auth_method = uaReject;
 	else if (strcmp(token->string, "md5") == 0)
-	{
-		if (Db_user_namespace)
-		{
-			ereport(elevel,
-					(errcode(ERRCODE_CONFIG_FILE_ERROR),
-					 errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"),
-					 errcontext("line %d of configuration file \"%s\"",
-								line_num, file_name)));
-			*err_msg = "MD5 authentication is not supported when \"db_user_namespace\" is enabled";
-			return NULL;
-		}
 		parsedline->auth_method = uaMD5;
-	}
 	else if (strcmp(token->string, "scram-sha-256") == 0)
 		parsedline->auth_method = uaSCRAM;
 	else if (strcmp(token->string, "pam") == 0)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 4c49393fc5..33a13fdf32 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -236,7 +236,6 @@ int			AuthenticationTimeout = 60;
 
 bool		log_hostname;		/* for ps display and logging */
 bool		Log_connections = false;
-bool		Db_user_namespace = false;
 
 bool		enable_bonjour = false;
 char	   *bonjour_name;
@@ -2272,24 +2271,6 @@ retry1:
 	if (port->database_name == NULL || port->database_name[0] == '\0')
 		port->database_name = pstrdup(port->user_name);
 
-	if (Db_user_namespace)
-	{
-		/*
-		 * If user@, it is a global user, remove '@'. We only want to do this
-		 * if there is an '@' at the end and no earlier in the user string or
-		 * they may fake as a local user of another database attaching to this
-		 * database.
-		 */
-		if (strchr(port->user_name, '@') ==
-			port->user_name + strlen(port->user_name) - 1)
-			*strchr(port->user_name, '@') = '\0';
-		else
-		{
-			/* Append '@' and dbname */
-			port->user_name = psprintf("%s@%s", port->user_name, port->database_name);
-		}
-	}
-
 	/*
 	 * Truncate given database and user names to length of a Postgres name.
 	 * This avoids lookup failures when overlength names are given.
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 71e27f8eb0..25d9008bb6 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1534,15 +1534,6 @@ struct config_bool ConfigureNamesBool[] =
 		false,
 		NULL, NULL, NULL
 	},
-	{
-		{"db_user_namespace", PGC_SIGHUP, CONN_AUTH_AUTH,
-			gettext_noop("Enables per-database user names."),
-			NULL
-		},
-		&Db_user_namespace,
-		false,
-		NULL, NULL, NULL
-	},
 	{
 		{"default_transaction_read_only", PGC_USERSET, CLIENT_CONN_STATEMENT,
 			gettext_noop("Sets the default read-only status of new transactions."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index e4c0269fa3..c768af9a73 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -96,7 +96,6 @@
 #authentication_timeout = 1min		# 1s-600s
 #password_encryption = scram-sha-256	# scram-sha-256 or md5
 #scram_iterations = 4096
-#db_user_namespace = off
 
 # GSSAPI using Kerberos
 #krb_server_keyfile = 'FILE:${sysconfdir}/krb5.keytab'
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index c85090259d..3da00f7983 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -103,8 +103,6 @@ typedef ProtocolVersion MsgType;
 
 typedef uint32 PacketLen;
 
-extern PGDLLIMPORT bool Db_user_namespace;
-
 /*
  * In protocol 3.0 and later, the startup packet length is not fixed, but
  * we set an arbitrary limit on it anyway.  This is just to prevent simple
-- 
2.25.1

Nathan Bossart