Re: [REVIEW] Tab Completion for CREATE DATABASE ... TEMPLATE ...

On 9/12/16, Kevin Grittner <kgrittn@gmail.com> wrote:
> On Sun, Sep 11, 2016 at 8:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> I wasn't aware that this patch was doing anything nontrivial ...
>
> Well, it is not doing anything other than showing candidate
> templates for tab completion beyond those flagged as template
> databases.
>
>> After looking at it I think it's basically uninformed about how to test
>> for ownership.  An explicit join against pg_roles is almost never the
>> right way for SQL queries to do that.  Lose the join and write it more
>> like this:
>>
>> +"SELECT pg_catalog.quote_ident(d.datname) "\
>> +"  FROM pg_catalog.pg_database d "\
>> +" WHERE substring(pg_catalog.quote_ident(d.datname),1,%d)='%s' "\
>> +"   AND (d.datistemplate OR pg_catalog.pg_has_role(d.datdba, 'USAGE'))"
>
> But that gives incorrect results for the case I asked about earlier
> on the thread, while the query I pushed gives correct results:
>
> test=# \du fred
>                   List of roles
>  Role name |       Attributes        | Member of
> -----------+-------------------------+-----------
>  fred      | Create DB, Cannot login | {}
>
> test=# \du bob
>            List of roles
>  Role name | Attributes | Member of
> -----------+------------+-----------
>  bob       |            | {fred}
>
> test=# \l
>                                  List of databases
>     Name    |  Owner  | Encoding |   Collate   |    Ctype    |  Access
> privileges
> ------------+---------+----------+-------------+-------------+---------------------
>  db1        | fred    | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
>  db2        | bob     | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
>  postgres   | fred    | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
>  regression | kgrittn | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
> =Tc/kgrittn        +
>             |         |          |             |             |
> kgrittn=CTc/kgrittn
>  template0  | kgrittn | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
> =c/kgrittn         +
>             |         |          |             |             |
> kgrittn=CTc/kgrittn
>  template1  | kgrittn | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
> =c/kgrittn         +
>             |         |          |             |             |
> kgrittn=CTc/kgrittn
>  test       | kgrittn | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
> (7 rows)
>
> test=# set role bob;
> SET
> test=> SELECT pg_catalog.quote_ident(d.datname)
>   FROM pg_catalog.pg_database d
>  WHERE (d.datistemplate OR pg_catalog.pg_has_role(d.datdba, 'USAGE'));
>  quote_ident
> -------------
>  template1
>  template0
>  postgres
>  db1
>  db2
> (5 rows)
>
> test=> SELECT pg_catalog.quote_ident(datname)
>   FROM pg_catalog.pg_database d
>   JOIN pg_catalog.pg_roles r ON rolname = CURRENT_USER
>  WHERE (datistemplate OR rolsuper OR datdba = r.oid);
>  quote_ident
> -------------
>  template1
>  template0
>  db2
> (3 rows)
>
> There is absolutely no question that the query you suggest is
> wrong; the only case I had to think about very hard after
> establishing that CREATEDB was not inherited was when bob owned a
> database but did not have CREATEDB permission.  It seemed to me
> least astonishing to show such a database, because that seemed
> parallel to, for example, tab completion for table names on CREATE
> TABLE AS.  We show a database object in the tab completion when it
> would be available except for absence of a permission on the user.
> That seems sane to me, because the attempt to actually execute with
> that object gives a potentially useful error message.  Anyway, I
> tend to like symmetry in these things -- it could also be
> considered sane not to show t2 on tab completion fro bob above, but
> then we should probably add more security tests to other tab
> completion queries, like CREATE TABLE AS ... SELECT ... FROM.
>
> --
> Kevin Grittner
> EDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company

While I was checking Tom's version, I thought that way, but finally I
realized there is no matter what tab completion shows because user
without the CREATEDB privileges can create no database at all:

postgres=# \du fred                 List of rolesRole name |       Attributes        | Member of
-----------+-------------------------+-----------fred      | Create DB, Cannot login | {}

postgres=# \du bob          List of rolesRole name | Attributes | Member of
-----------+------------+-----------bob       |            | {fred}

postgres=# \l db*                           List of databasesName | Owner | Encoding |   Collate   |    Ctype    |
Accessprivileges
 
------+-------+----------+-------------+-------------+-------------------db1  | fred  | UTF8     | en_US.UTF-8 |
en_US.UTF-8|db2  | bob   | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
 
(2 rows)

postgres=# set role bob;
SET
postgres=> CREATE DATABASE ss TEMPLATE db  -- shows both
db1  db2
postgres=> CREATE DATABASE ss TEMPLATE db2;
ERROR:  permission denied to create database
postgres=>

So a check for the CREATEDB privilege should be done at the point
whether to show CREATE DATABASE or not.
But if a user has privileges, Tom's version works fine.

-- 
Best regards,
Vitaly Burovoy