Re: CREATE ROLE IF NOT EXISTS

Greetings,

* David Christensen (david.christensen@crunchydata.com) wrote:
> On Mon, Nov 8, 2021 at 1:22 PM Mark Dilger <mark.dilger@enterprisedb.com>
> wrote:
>
> > > On Nov 8, 2021, at 10:38 AM, Stephen Frost <sfrost@snowman.net> wrote:
> >
> > > I don't quite follow this.  The entire point of Alice writing a script
> > > that uses IF NOT EXISTS is to have that command not fail if, indeed,
> > > that role already exists, but for the rest of the script to be run.
> > > That there's some potential attacker with CREATEROLE running around
> > > creating roles that they think someone *else* might create is really
> > > stretching things to a very questionable level- especially with
> > > CREATEROLE where Charlie could just CREATE a new role which is a member
> > > of Bob anyway after the fact and then GRANT that role to themselves.
> >
> > I don't see why this is "stretching things to a very questionable level".
> > It might help this discussion if you could provide pseudo-code or similar
> > for adding roles which is well-written and secure, and which benefits from
> > this syntax.  I would expect the amount of locking and checking for
> > pre-existing roles that such logic would require would make the IF NOT
> > EXIST option useless.  Perhaps I'm wrong?
> >
>
> The main motivator for me writing this was trying to increase idempotency
> for things like scripting, where you want to be able to minimize the effort
> required to get things into a particular state.  I agree with Stephen that
> whether or not this is a best practices approach, this is something that is
> being done in the wild via DO blocks or similar, so providing a tool to
> handle this better seems useful on its own.

Agreed.

> This originally came from me looking into the failures to load certain
> `pg_dump` or `pg_dumpall` output when generated with the `--clean` flag,
> which necessarily cannot work, as it fails with the error `current user
> cannot be dropped`.  Not that I am promoting the use of `pg_dumpall
> --clean`, as there are clearly better solutions here, but something which
> generates unusable output does not seem that useful.  Instead, you could
> generate `CREATE ROLE IF NOT EXISTS username` statements and emit `ALTER
> ROLE ...`, which is what it is already doing (modulo `IF NOT EXISTS`).

The other very common case that I've seen is where the role ends up
owning objects and therefore can't be dropped without also dropping
those objects- possibly just GRANTs but may also be tables or other
things.  In other words, a script like this:

DROP ROLE IF EXISTS r1;
CREATE ROLE r1;
CREATE SCHEMA IF NOT EXISTS r1 AUTHORIZATION r1;

isn't able to be re-run, while this is able to be:

CREATE ROLE IF NOT EXISTS r1;
CREATE SCHEMA IF NOT EXISTS r1 AUTHORIZATION r1;

> This seems to introduce no further security vectors compared to field work
> and increases utility in some cases, so seems generally useful to me.

Yeah.

> If CINE semantics are at issue, what about the CREATE OR REPLACE semantics
> with some sort of merge into the existing role?  I don't care strongly
> about which approach is taken, just think the overall "make this role exist
> in this form" without an error is useful in my own work, and CINE was
> easier to implement as a first pass.

I don't really see how we could do CREATE OR REPLACE here, at least for
the cases that I'm thinking about.  How would that work with existing
GRANTs, for example?  Perhaps it'd be alright if we limited it to just
what can be specified in the CREATE ROLE and then left anything else in
place.  I do generally like the idea of being able to explicitly say how
the role should look in one shot.

Thanks,

Stephen