Re: Please advice TODO Item pg_hba.conf

Hi,

On Sun, 2006-04-23 at 17:06 -0400, Alvaro Herrera wrote:
> Gevik Babakhani wrote:
>
> Hi,
>
> > 2) The file parsenodes.h is updated to support
> > #define ACL_DATABASE_CONNECT
>
> I wouldn't call it ACL_DATABASE_CONNECT, just ACL_CONNECT. Currently we
> don't have any objects other than databases that need connecting to, but
> you never know.
>

OK :)

> Please make sure you use the established style for writing the ereport()
> call. The newlines you placed are not consistent with the rest of the
> code. Also the message text does not follow the guidelines (no initial
> capital letter, no ending period for the primary message; the other way
> around for secondary messages). I think it should be
>
> ereport(FATAL,
> (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> errmsg("couldn't connect to database %s", database_name),
> errdetail("User %s doesn't have the CONNECT privilege for database %s.",
> user_name, database_name)));
>

OK. I will put the above as the error message. We can always change this
later if we wanted to.

> > 8) File dbcommands.c method "createdb" is updated by following:
> > When a new database is being created we add a default ACL by
> > calling acldefault(ACL_OBJECT_DATABASE,.... and adding the default ACL
> > by new_record[Anum_pg_database_datacl - 1] =
> > PointerGetDatum(defaultAcl);
>
> If I'm not mistaken, the general principle for creating objects is leave
> their ACLs as NULLs. Later, when a privilege is going to be checked, a
> NULL is treated as if it contained whatever default privilege the object
> class has. So you should leave this code alone, and have the checking
> code replace the default ACL when it gets a NULL (this way, it's even
> more backwards compatible).

Personally I think this would create an conflict only in case of the
CONNECT privilege. If the ACL is NULL and we treat NULL as default and
the CONNECT privilege is part of default privileges then how do we
distinguish between someone NOT HAVING THE CONNECT PRIVILEGE to connect
to a certain database. This was the reason I thought when someone
connects to a database NULL ACL will not do because you cannot know the
user connecting does or does not have the CONNECT privilege. The problem
is I think when you revoke a privilege from ACL, the current code
regarding this actually removes/deletes the privilege from the ACL bits.
>
> > At this moment the owner of the database CAN REVOKE himself form the
> > ACL_OBJECT_DATABASE. If the implementation above is acceptable then I
> > can work on this one :)
>
> Hmm, what do you want to do about it? ISTM the owner should be able to
> revoke the privilege from himself ... (Maybe we could raise a WARNING
> whenever anyone revokes the last CONNECT privilege to a database, so
> that he can GRANT it to somebody before disconnecting.)

Personally I think it would be better for the database owner not have
the option to REVOKE himself from the CONNECTION privilege of his own
database.

> Also I'm not sure if we have discussed what's the default (initial)
> privilege state. Do we want PUBLIC to have CONNECT privilege?

If I where a DBA I would rather explicitly give people connect
permission to my database rather than having to explicitly block them
from connecting to the database. It is like all the doors are closed to
the public unless the doorman opens the door for the person he knows.
But then again this is my personal opinion. We can always discuss this
of course.