Greetings, * Michael Paquier (mich...@paquier.xyz) wrote: > On Tue, Dec 29, 2020 at 02:26:19PM -0600, Paul Martinez wrote: > > The checks for whether the current user can create a user with the > > SUPERUSER, > > REPLICATION, or BYPASSRLS attributes are chained together using if/else-if, > > before finally checking whether the user has CREATEROLE privileges in a > > final else. This construction is error-prone, since once one branch is > > visited, > > later ones are skipped, and it implicitly assumes that the permissions > > needed > > for each subsequent action are subsets of the permissions needed for the > > previous action. Since each branch requires SUPERUSER this is fine, but > > changing one of the checks could inadvertently allow users without the > > CREATEROLE permission to create users. > > Hmm. I agree with your position here. A careless change could easily > miss that all those if branches have to use a superuser check.
That really strikes me as pretty darn unlikely to happen, it's not like this code is really spread out across very many lines or that it's hard to see the pretty clear pattern. In thinking about these role attributes and the restrictions we have about what attributes don't require superuser to set and what do, I do feel like we're probably missing a bet regarding replication and bypassrls.. In other words, I suspect people would be happier if we provided a way for non-superusers a way to create replication roles and bypassrls roles. Exactly how we do that is a bit tricky to figure out but that's certainly a much more productive direction to be going in. Thanks, Stephen
signature.asc
Description: PGP signature