Re: [PATCHES] Users/Groups -> Roles

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> Also, I've been looking through the diff between my tree and what you
> committed to CVS and had a couple comments

> First, sorry about the gratuitous name changes, it helped me find
> every place I needed to look at the code and think about if it needed
> to be changed in some way (ie: Int32GetDatum -> ObjectIdGetDatum,
> etc). I had planned on changing some of them back to minimize the
> patch but kind of ran out of time.

No problem, I figured that was why you'd done it, but changing them back
helped me to understand the patch also ;-)

> Second, looks like I missed fixing an owner check in pg_proc.c

Got it. I was wondering if there were more --- might be worth checking
all the superuser() calls.

> Third, I feel it's incorrect to only allow superuser() to change
> ownership of objects under a role-based system.

I took that out because it struck me as a likely security hole; we don't
allow non-superuser users to give away objects now, and we shouldn't
allow non-superuser roles to do so either. Moreover the tests you had
were inconsistent (not same test everyplace).

> Users must be able to
> create objects owned by a role they're in (as opposed to owned only
> by themselves).

This is what SET SESSION AUTHORIZATION/SET ROLE is for, no? You set the
auth to a role you are allowed to be in, then create the object. I do
notice that we don't have this yet, but it's surely a required piece of
the puzzle.

> Fourth, not that I use it, but, it looks like my changes to
> src/interfaces/ecpg/preproc/preproc.y were lost. Not sure if that was
> intentional or not

Yeah, it was. I leave it to Michael Meskes to sync ecpg with the main
parser; on the occasions where I've tried to do it for him, things
didn't work out well.

> I do wish ecpg could just
> be the differences necessary for ecpg and be based off the main parser
> somehow,

Me too, but I haven't seen a way yet.

> src/tools/pgindent/pgindent also appears to not have gotten the
> changes that I made.

That's an automatically generated list; there's no need to edit it.

regards, tom lane