Thread: role support
Hello hackers, I createad a patch to add support for roles in saved server configurations. After a db connection it will issue a SET Role=<rolename> when a rolename is specified for the connected server. This allows using pgAdmin's gui to create/alter/delete objects in accordance with a security policy that forbids loggingin directly as superuser, but instead specifies that you should log into an admin account and then SET Role to the superuser. I also found something which seems to me a small escaping bug, a patch included for this issue also. Please review them for a possible upstream inclusion. Thank you: Benedek Laszlo
Attachment
Le 06/07/2010 16:36, Benedek László a écrit : > [...] > I createad a patch to add support for roles in saved server configurations. > After a db connection it will issue a SET Role=<rolename> when a rolename is specified for the connected server. > > This allows using pgAdmin's gui to create/alter/delete objects in accordance with a security policy that forbids loggingin directly > as superuser, but instead specifies that you should log into an admin account and then SET Role to the superuser. > It seems quite interesting. I still only read the patch (and it seems a good one), but I didn't test it actually. I'll do in the next few days. Anyway, we're still in beta for 1.12. If the patch is good enough, we'll have to wait the creation of the 1.12 branch before applying it. > I also found something which seems to me a small escaping bug, a patch included for this issue also. > Commited. I also created the ticket 208 for it. Thank you. -- Guillaume http://www.postgresql.fr http://dalibo.com
On Tue, 06 Jul 2010 23:33:56 +0200, Guillaume Lelarge <guillaume@lelarge.info> wrote: > Le 06/07/2010 16:36, Benedek László a écrit : >> [...] >> I createad a patch to add support for roles in saved server >> configurations. >> After a db connection it will issue a SET Role=<rolename> when a >> rolename is specified for the connected server. >> >> This allows using pgAdmin's gui to create/alter/delete objects in >> accordance with a security policy that forbids logging in directly >> as superuser, but instead specifies that you should log into an admin >> account and then SET Role to the superuser. >> > > It seems quite interesting. I still only read the patch (and it seems a > good one), but I didn't test it actually. I'll do in the next few days. > Anyway, we're still in beta for 1.12. If the patch is good enough, we'll > have to wait the creation of the 1.12 branch before applying it. > I tried to apply your patch, but there is a big issue. It doesn't apply cleanly at all. And when I say "at all", I really mean it. It seems the patch was written way before we apply the "application name" patch (that was 7 months ago). Can you update your patch and make sure it applies with current HEAD? (should I say the master branch for git? :) ) I will get a look at your updated patch. Thanks. -- Guillaume http://www.postgresql.fr http://dalibo.com
On 2010-07-20 11:05, guillaume wrote: > On Tue, 06 Jul 2010 23:33:56 +0200, Guillaume Lelarge > <guillaume@lelarge.info> wrote: > > I tried to apply your patch, but there is a big issue. It doesn't apply > cleanly at all. And when I say "at all", I really mean it. It seems the > patch was written way before we apply the "application name" patch (that > was 7 months ago). Can you update your patch and make sure it applies with > current HEAD? (should I say the master branch for git? :) ) I will get a > look at your updated patch. > > Thanks. > > Here is an updated one. The original was against some 1.10 version, sorry for that. Now I also included a backup form patch to specify the rolename there too. Comments are welcome. :) Thanks
Attachment
Here is a path to support the roles in global and server backups.
Attachment
Hi, Here is the latest and greatest :) This patch includes everything for a complete login role support (I hope). A rolename can be specified for each server configuration, which will then be applied after a successfull db connection. Backup and restore dialogs also contain a rolename field to be passed to pg_dump and pg_restore when their versions are >= 8.4. Comments are welcome. :) Thanks.
Attachment
Small fixes for the quick connection selector dialog: - The connection name includes the specified role's name - The last rolename saved Appliable onto the previous patch.
Attachment
Le 22/07/2010 16:19, Benedek László a écrit : > On 2010-07-20 11:05, guillaume wrote: >> On Tue, 06 Jul 2010 23:33:56 +0200, Guillaume Lelarge >> <guillaume@lelarge.info> wrote: >> >> I tried to apply your patch, but there is a big issue. It doesn't apply >> cleanly at all. And when I say "at all", I really mean it. It seems the >> patch was written way before we apply the "application name" patch (that >> was 7 months ago). Can you update your patch and make sure it applies with >> current HEAD? (should I say the master branch for git? :) ) I will get a >> look at your updated patch. >> >> Thanks. >> >> > > Here is an updated one. > The original was against some 1.10 version, sorry for that. > Works great on 9.0. Small issue, the rolename doesn't appear in the server's properties tab. Doesn't work on 7.4 and 8.0. These releases don't have role support (it was added on 8.1). Even if they'll be not maintain anymore, I would like that this patch supports them. It's a simple test to do before executing the "SET ROLE" statement. With that fix, it'll be ready to commit. > Now I also included a backup form patch to specify the rolename there too. > Checked that too. Works great. There's still something to do. You use a textbox on the server dialog and it is good because you have no way to know the available user on that server (because you're not connected to it when you can change the rolename value). You still use a textbox on the backup window. It would be better to use a combobox because you can have this window only when you're connected to the server... meaning you can easily get the list of users available. With this, this patch would be commited. > Comments are welcome. :) > Done :) Thanks for your patch. It seems a great addition to pgAdmin. -- Guillaume http://www.postgresql.fr http://dalibo.com
Le 22/07/2010 17:15, Benedek László a écrit : > Here is a path to support the roles in global and server backups. > Got a look a this one too. Same note than the backup patch (ie, you should use a combox instead of the textbox, for the rolename). Can you send us a complete patch with all the fixes? when I say complete, I don't mean a single one, but it seems the backupglobal patch doesn't apply on top of your other patches. I would prefer to get a 0001-setrole patch, a 0002-backuppatch, a 0003-backupglobalpatch. Not sure where the "Small fixes for the quick connection selector dialog" patch gets into (perhaps 0001... ?) Or you can also send me a single complete patch. Your choice :) BTW, any reason why there's no pg_restore patch? it should work the same, shouldn't it? Anyway, thanks again. I'm looking forward to commit this really soon :) -- Guillaume http://www.postgresql.fr http://dalibo.com
This is the next approximation, a single patch against current git master. Should cover all the features you asked. Please review it. Benedek Laszlo
Attachment
Le 26/07/2010 17:38, Benedek László a écrit : > This is the next approximation, a single patch against current git master. > > Should cover all the features you asked. Please review it. > Sure. Applies, compiles, and works great. Everything I asked was fixed or done. So, barring any objections, I'll apply it tomorrow morning. -- Guillaume http://www.postgresql.fr http://dalibo.com
Le 26/07/2010 18:59, Guillaume Lelarge a écrit : > Le 26/07/2010 17:38, Benedek László a écrit : >> This is the next approximation, a single patch against current git master. >> >> Should cover all the features you asked. Please review it. >> > > Sure. Applies, compiles, and works great. Everything I asked was fixed > or done. > > So, barring any objections, I'll apply it tomorrow morning. > Applied. Thanks for your patch. -- Guillaume http://www.postgresql.fr http://dalibo.com