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

On 9/12/16, Kevin Grittner <kgrittn(at)gmail(dot)com> wrote:
> On Sun, Sep 11, 2016 at 8:07 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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 roles
Role name | Attributes | Member of
-----------+-------------------------+-----------
fred | Create DB, Cannot login | {}

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

postgres=# \l db*
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 |
(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