Re: Open 7.3 items - Mailing list pgsql-hackers

OK, no one complained/commented on my idea of having global users have a
trailing '@', so here is a patch that implements that.  It has the
advantages of:

    no special install user (create global user before enabling feature)
    no /data/PG_INSTALLER file
    allows multiple global users to be easily added
    no namespace collisions because globals have a trailing @
    easy for postmaster to recognize global users
    no double-user lookups of pg_pwd changes
    very small patch footprint

The only downside is that it treats '@' as a special character when it
is enabled, but frankly, because we are appending @dbname anyway, having
'@' as a special character in that case makes sense.

Comments?

---------------------------------------------------------------------------

Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > I don't know where else to go with the patch at this point.  I think
> > > increasing the number of 'global' users is polluting the namespace too
> > > much,
> >
> > Why?  If the installation needs N global users, then it needs N global
> > users; who are you to make that value judgment for them?
> >
> > In practice I think an installation that's using this feature is going
> > to have a pretty small number of global users, and so the issue of
> > collisions with local usernames isn't really as big as it's been painted
> > in this thread.  We could ignore that issue (except for documenting it)
> > and have a perfectly serviceable feature.
>
> The original idea was that Marc wanted people who could create their own
> users for their own databases.  If we make the creation of global users
> too easy, all of a sudden people don't have control over their db
> usernames because they have to avoid all the global user names already
> defined.  By adding multiple global users, it is diluting the usefulness
> of the feature.
>
> I suppose a pg_global_users file would be a compromise because only the
> admin could actually add people to that file.  If it was more automatic,
> like writing pg_shadow, someone could create a user without an @ and
> block access for other users to other database, which is bad.
>
> I still don't like the fact that people think they have control over
> their db namespace, when they really don't, but no one else seems to see
> that as a problem.  The namespace conflicts just yell of poor design.
>
> OK, I have another idea.  What if we make global users end with an @, so
> dave@ is a global user.  We can easily check for that in the postmaster
> and not append the dbname.  I know it makes @ a special character, but
> considering the problem of namespace collision, it seems better than
> what we have now.  We could add the install user too if we wish, or just
> tell them to make sure they add a user@ before turning on the feature.
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 359-1001
>   +  If your life is a hard drive,     |  13 Roberts Road
>   +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/runtime.sgml
===================================================================
RCS file: /cvsroot/pgsql-server/doc/src/sgml/runtime.sgml,v
retrieving revision 1.125
diff -c -r1.125 runtime.sgml
*** doc/src/sgml/runtime.sgml    15 Aug 2002 14:26:15 -0000    1.125
--- doc/src/sgml/runtime.sgml    15 Aug 2002 15:32:29 -0000
***************
*** 1191,1196 ****
--- 1191,1211 ----
       </varlistentry>

       <varlistentry>
+       <term><varname>DB_USER_NAMESPACE</varname> (<type>boolean</type>)</term>
+       <listitem>
+        <para>
+         Appends <literal>@</> and the database name to the user name when
+         connecting to the database.  This allows per-database users.
+         User names ending with <literal>@</> are considered global and may
+         connect to any database.  It is recommended you create at least one
+         global user, e.g. <literal>postgres@</>, before enabling this feature.
+         Also, when creating user names containing <literal>@</>, you will need
+         to quote the user name.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
        <indexterm>
         <primary>deadlock</primary>
         <secondary>timeout</secondary>
Index: src/backend/libpq/auth.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/libpq/auth.c,v
retrieving revision 1.82
diff -c -r1.82 auth.c
*** src/backend/libpq/auth.c    20 Jun 2002 20:29:28 -0000    1.82
--- src/backend/libpq/auth.c    15 Aug 2002 15:32:30 -0000
***************
*** 117,123 ****
               version, PG_KRB4_VERSION);
          return STATUS_ERROR;
      }
!     if (strncmp(port->user, auth_data.pname, SM_USER) != 0)
      {
          elog(LOG, "pg_krb4_recvauth: name \"%s\" != \"%s\"",
               port->user, auth_data.pname);
--- 117,123 ----
               version, PG_KRB4_VERSION);
          return STATUS_ERROR;
      }
!     if (strncmp(port->user, auth_data.pname, SM_DATABASE_USER) != 0)
      {
          elog(LOG, "pg_krb4_recvauth: name \"%s\" != \"%s\"",
               port->user, auth_data.pname);
***************
*** 290,296 ****
      }

      kusername = pg_an_to_ln(kusername);
!     if (strncmp(port->user, kusername, SM_USER))
      {
          elog(LOG, "pg_krb5_recvauth: user name \"%s\" != krb5 name \"%s\"",
               port->user, kusername);
--- 290,296 ----
      }

      kusername = pg_an_to_ln(kusername);
!     if (strncmp(port->user, kusername, SM_DATABASE_USER))
      {
          elog(LOG, "pg_krb5_recvauth: user name \"%s\" != krb5 name \"%s\"",
               port->user, kusername);
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/postmaster/postmaster.c,v
retrieving revision 1.283
diff -c -r1.283 postmaster.c
*** src/backend/postmaster/postmaster.c    10 Aug 2002 20:29:18 -0000    1.283
--- src/backend/postmaster/postmaster.c    15 Aug 2002 15:32:34 -0000
***************
*** 116,122 ****
  sigset_t    UnBlockSig,
              BlockSig,
              AuthBlockSig;
-
  #else
  int            UnBlockSig,
              BlockSig,
--- 116,121 ----
***************
*** 191,196 ****
--- 190,197 ----
  bool        HostnameLookup;        /* for ps display */
  bool        ShowPortNumber;
  bool        Log_connections = false;
+ bool        Db_user_namespace = false;
+

  /* Startup/shutdown state */
  static pid_t StartupPID = 0,
***************
*** 1161,1166 ****
--- 1162,1177 ----
      if (port->user[0] == '\0')
          elog(FATAL, "no PostgreSQL user name specified in startup packet");

+     /* Append database name for per-db user namespace, exclude global users. */
+     if (Db_user_namespace && strlen(port->user) > 0 &&
+         port->user[strlen(port->user)-1] != '@')
+     {
+         char hold_user[SM_DATABASE_USER];
+         snprintf(hold_user, SM_DATABASE_USER, "%s@%s", port->user,
+                  port->database);
+         strcpy(port->user, hold_user);
+     }
+
      /*
       * If we're going to reject the connection due to database state, say
       * so now instead of wasting cycles on an authentication exchange.
***************
*** 2587,2597 ****
      if (FindExec(fullprogname, argv[0], "postmaster") < 0)
          return false;

!     filename = palloc(strlen(DataDir) + 20);
      sprintf(filename, "%s/postmaster.opts", DataDir);

!     fp = fopen(filename, "w");
!     if (fp == NULL)
      {
          postmaster_error("cannot create file %s: %s",
                           filename, strerror(errno));
--- 2598,2607 ----
      if (FindExec(fullprogname, argv[0], "postmaster") < 0)
          return false;

!     filename = palloc(strlen(DataDir) + 17);
      sprintf(filename, "%s/postmaster.opts", DataDir);

!     if ((fp = fopen(filename, "w")) == NULL)
      {
          postmaster_error("cannot create file %s: %s",
                           filename, strerror(errno));
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/utils/misc/guc.c,v
retrieving revision 1.82
diff -c -r1.82 guc.c
*** src/backend/utils/misc/guc.c    15 Aug 2002 02:51:26 -0000    1.82
--- src/backend/utils/misc/guc.c    15 Aug 2002 15:32:42 -0000
***************
*** 483,488 ****
--- 483,492 ----
          { "transform_null_equals", PGC_USERSET }, &Transform_null_equals,
          false, NULL, NULL
      },
+     {
+         { "db_user_namespace", PGC_SIGHUP }, &Db_user_namespace,
+         false, NULL, NULL
+     },

      {
          { NULL, 0 }, NULL, false, NULL, NULL
Index: src/backend/utils/misc/postgresql.conf.sample
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/utils/misc/postgresql.conf.sample,v
retrieving revision 1.44
diff -c -r1.44 postgresql.conf.sample
*** src/backend/utils/misc/postgresql.conf.sample    12 Aug 2002 00:36:12 -0000    1.44
--- src/backend/utils/misc/postgresql.conf.sample    15 Aug 2002 15:32:42 -0000
***************
*** 113,119 ****
  #
  #    Message display
  #
-
  #server_min_messages = notice    # Values, in order of decreasing detail:
                  #   debug5, debug4, debug3, debug2, debug1,
                  #   info, notice, warning, error, log, fatal,
--- 113,118 ----
***************
*** 201,203 ****
--- 200,203 ----
  #sql_inheritance = true
  #transform_null_equals = false
  #statement_timeout = 0                # 0 is disabled
+ #db_user_namespace = false
Index: src/include/libpq/libpq-be.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/libpq/libpq-be.h,v
retrieving revision 1.32
diff -c -r1.32 libpq-be.h
*** src/include/libpq/libpq-be.h    20 Jun 2002 20:29:49 -0000    1.32
--- src/include/libpq/libpq-be.h    15 Aug 2002 15:32:43 -0000
***************
*** 59,65 ****

      ProtocolVersion proto;
      char        database[SM_DATABASE + 1];
!     char        user[SM_USER + 1];
      char        options[SM_OPTIONS + 1];
      char        tty[SM_TTY + 1];
      char        auth_arg[MAX_AUTH_ARG];
--- 59,65 ----

      ProtocolVersion proto;
      char        database[SM_DATABASE + 1];
!     char        user[SM_DATABASE_USER + 1];
      char        options[SM_OPTIONS + 1];
      char        tty[SM_TTY + 1];
      char        auth_arg[MAX_AUTH_ARG];
***************
*** 72,78 ****
      SSL           *ssl;
      X509       *peer;
      char        peer_dn[128 + 1];
!     char        peer_cn[SM_USER + 1];
      unsigned long count;
  #endif
  } Port;
--- 72,78 ----
      SSL           *ssl;
      X509       *peer;
      char        peer_dn[128 + 1];
!     char        peer_cn[SM_DATABASE_USER + 1];
      unsigned long count;
  #endif
  } Port;
Index: src/include/libpq/pqcomm.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/libpq/pqcomm.h,v
retrieving revision 1.65
diff -c -r1.65 pqcomm.h
*** src/include/libpq/pqcomm.h    12 Aug 2002 14:35:26 -0000    1.65
--- src/include/libpq/pqcomm.h    15 Aug 2002 15:32:43 -0000
***************
*** 114,119 ****
--- 114,121 ----
  #define SM_DATABASE        64
  /* SM_USER should be the same size as the others.  bjm 2002-06-02 */
  #define SM_USER            32
+ /* We append database name if db_user_namespace true. */
+ #define SM_DATABASE_USER (SM_DATABASE+SM_USER)
  #define SM_OPTIONS        64
  #define SM_UNUSED        64
  #define SM_TTY            64
***************
*** 124,135 ****
--- 126,139 ----
  {
      ProtocolVersion protoVersion;        /* Protocol version */
      char        database[SM_DATABASE];    /* Database name */
+                 /* Db_user_namespace appends dbname */
      char        user[SM_USER];    /* User name */
      char        options[SM_OPTIONS];    /* Optional additional args */
      char        unused[SM_UNUSED];        /* Unused */
      char        tty[SM_TTY];    /* Tty for debug output */
  } StartupPacket;

+ extern bool Db_user_namespace;

  /* These are the authentication requests sent by the backend. */