Обсуждение: TODO item question [pg_hba.conf]
Hi, As advised, I spend a moment reading the code regarding the GRANT and REVOKE In order to add a new privilege to the ACL, I have created a mini patch. Could this be checked to see if I am on the right track? http://www.xs4all.nl/~gevik/patch/alpha.patch Thank you.
Gevik Babakhani wrote: > As advised, I spend a moment reading the code regarding the GRANT and REVOKE > In order to add a new privilege to the ACL, I have created a mini patch. > Could this be checked to see if I am on the right track? > > http://www.xs4all.nl/~gevik/patch/alpha.patch You are missing an ACL_*_CHR symbol and updating the ACL_ALL_RIGHTS_STR symbol. Also, you should know that changing this requires a change in CATALOG_VERSION_NO in catversion.h as well. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Thank you :)
> You are missing an ACL_*_CHR symbol and updating the ACL_ALL_RIGHTS_STR
> symbol.
That is why I could not see the new permission in pg_database.
I was actually looking for that for sometime :)
I have added the ACL_*_CHR 'D' Is this okay?
> Also, you should know that changing this requires a change in
> CATALOG_VERSION_NO in catversion.h as well.
Why is this needed? Is this a functional requirement?
I have changed it to
#define CATALOG_VERSION_NO 200604211
Is this okay?
Regards,
Gevik.
gevik=# create role user1;
CREATE ROLE
gevik=# grant connection on database db2 to user1;
GRANT
gevik=# select datname,datacl from pg_catalog.pg_database; datname | datacl
-----------+------------------------------------------postgres |db1 | {=T/gevik,gevik=CTD/gevik}template1 |
{gevik=CTD/gevik}template0| {gevik=CTD/gevik}gevik | {=T/gevik,gevik=CTD/gevik}db2 |
{=T/gevik,gevik=CTD/gevik,user1=D/gevik}
(6 rows)
Gevik Babakhani wrote: > Thank you :) > > > You are missing an ACL_*_CHR symbol and updating the ACL_ALL_RIGHTS_STR > > symbol. > > That is why I could not see the new permission in pg_database. > I was actually looking for that for sometime :) > > I have added the ACL_*_CHR 'D' Is this okay? Hum, you literally added a symbol ACL_*_CHR? I was actually thinking in ACL_CONNECT_CHR or something like that ... While at it, why D? Isn't 'c' more natural? (And conveniently unused.) > > Also, you should know that changing this requires a change in > > CATALOG_VERSION_NO in catversion.h as well. > > Why is this needed? Is this a functional requirement? To force an initdb, because you are causing a system catalog change. Now that I think about it, maybe it's not needed, because the default state of the system should be the same as if no privilege has changed. OTOH you need to speficy the interpretation of the initial state of the ACL for a database. I think it should mean that PUBLIC has the CONNECT privilege. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
"Gevik Babakhani" <pgdev@xs4all.nl> writes:
> I have added the ACL_*_CHR 'D' Is this okay?
That seems an excessively random choice of character for CONNECT
privilege. I see that 'C' is already taken, but we could use 'c'.
>> Also, you should know that changing this requires a change in
>> CATALOG_VERSION_NO in catversion.h as well.
> Why is this needed? Is this a functional requirement?
It's just something we do to avoid bogus bug reports from people
who haven't initdb'd after something that requires a catalog change.
In this case, since the system would stop working (or at least stop
letting you connect) if you hadn't initdb'd, I think a catversion
bump is indicated.
regards, tom lane
Hi, I have created a new patch. Please check to see if I am on the right track. 1) The GRANT and REVOKE statements look like: GRANT CONNECTION ON DATABASE db1 TO user1 (,user2,user3) REVOKE CONNECTION ON DATABASE db1 TO user1 (,user2,user3) 2) The file parsenodes.h is updated to support #define ACL_DATABASE_CONNECT 3) The file acl.h is updated to support #define ACL_DATABASE_CONNECT_CHR 'c' 4) Functions "string_to_privilege" and "privilege_to_string" in aclchk.c are updated to support ACL_DATABASE_CONNECT 5) Function "aclparse" in acl.c is updated to support ACL_DATABASE_CONNECT 6) Catalog version number is updated to CATALOG_VERSION_NO 200604211 7) File postinit.c method "ReverifyMyDatabase" is updated by following: First we check to make sure we are not in bootstrap processing mode. If not, we check to see if the connected user has ACL_DATABASE_CONNECT. If not, ereport(FATAL,.....) (Perhaps we should change the error message later) 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); This would mean, every time a new database gets created the owner of the database gets the ACL_OBJECT_DATABASE privilege and can login. Other users not having the privilege to that database get an error message. Because the catalog version is changed a pg_dump is necessarily, means all the new roles created from that point will get the ACL_OBJECT_DATABASE and everything should be "backward-compatible" 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 :) http://www.xs4all.nl/~gevik/patch/patch-0.1.diff Did I forget something? Please advice.