Thread: Proposed Patchs

Proposed Patchs

From
"Thomas Sondag"
Date:
Hi,

With PostgreSQL 8.1 and new ROLE object remplacing traditional
USER/GROUP, I was a bit confuse using the dlgProperty and
dlgSecurityProperty dialog because I can only select USER (ROLE with
LOGIN privilege) for owner and GROUP (ROLE without LOGIN privilege)
for privileges .
And I not sure this comportment can match all PostgreSQL 8.1 usages
scenarios (like one of my case).

This proposed patch :
 - change owner and privilege list to get the full ROLE list.
 - select by default currently connected ROLE in the owner list
(replacing the blank filed) for new object creation
 - remove pg_global in the available tablespace list
 - select current user default tablespace in tablespace list
(replacing the blank filed, yes I don't like blank field) for new
object creation

It's not fully finish yet, I have one or two bug in mind, but it's
usable if you want to test it, I post it to know your opinion about
it, know if go to the wrong way.

Does it make sense to replace user and group tree by a role tree if
your are connected to a PostgreSQL server version 8.1 ?

Comments are welcomes.

    Thomas

Attachment

Re: Proposed Patchs

From
"Dave Page"
Date:

> -----Original Message-----
> From: pgadmin-support-owner@postgresql.org
> [mailto:pgadmin-support-owner@postgresql.org] On Behalf Of
> Thomas Sondag
> Sent: 24 May 2006 17:28
> To: pgadmin-support@postgresql.org
> Subject: [pgadmin-support] Proposed Patchs
>
> Hi,
>
> With PostgreSQL 8.1 and new ROLE object remplacing traditional
> USER/GROUP, I was a bit confuse using the dlgProperty and
> dlgSecurityProperty dialog because I can only select USER (ROLE with
> LOGIN privilege) for owner and GROUP (ROLE without LOGIN privilege)
> for privileges .
> And I not sure this comportment can match all PostgreSQL 8.1 usages
> scenarios (like one of my case).
>
> This proposed patch :
>  - change owner and privilege list to get the full ROLE list.

How is this different from the current behaviour if the Show Users for
Privileges option is turned on? The whole point there is to promote the
use of group based permissions rather than user based for both
simplicity (because the list only shows the groups), and for cleanliness
of design (users come and go, groups tend to be more permanent). In 8.1+
of course, we simply replace users and groups with roles with or without
the login flag.

>  - select by default currently connected ROLE in the owner list
> (replacing the blank filed) for new object creation

OK.

>  - remove pg_global in the available tablespace list

Probably a good idea, yes.

>  - select current user default tablespace in tablespace list
> (replacing the blank filed, yes I don't like blank field) for new
> object creation

OK.

Regards, Dave.


Re: Proposed Patchs

From
"Thomas Sondag"
Date:
2006/5/24, Dave Page <dpage@vale-housing.co.uk>:
>
>
> > -----Original Message-----
> > From: pgadmin-support-owner@postgresql.org
> > [mailto:pgadmin-support-owner@postgresql.org] On Behalf Of
> > Thomas Sondag
> > Sent: 24 May 2006 17:28
> > To: pgadmin-support@postgresql.org
> > Subject: [pgadmin-support] Proposed Patchs
> >
> > Hi,
> >
> > With PostgreSQL 8.1 and new ROLE object remplacing traditional
> > USER/GROUP, I was a bit confuse using the dlgProperty and
> > dlgSecurityProperty dialog because I can only select USER (ROLE with
> > LOGIN privilege) for owner and GROUP (ROLE without LOGIN privilege)
> > for privileges .
> > And I not sure this comportment can match all PostgreSQL 8.1 usages
> > scenarios (like one of my case).
> >
> > This proposed patch :
> >  - change owner and privilege list to get the full ROLE list.
>
> How is this different from the current behaviour if the Show Users for
> Privileges option is turned on? The whole point there is to promote the
> use of group based permissions rather than user based for both
> simplicity (because the list only shows the groups), and for cleanliness
> of design (users come and go, groups tend to be more permanent). In 8.1+
> of course, we simply replace users and groups with roles with or without
> the login flag.
>
Hum, I miss this option ... sorry, but the main difference with the
current behaviour is for object owning. The main idea was to set
object owner to a group like that :
database foo -> group foo schema bar -> group bar schema bar read user -> user toto

I don't know if that's a good policy, but this case may exist, we may
add an option like "Show Group for object owning" ?

This is not the appropriate list to talk about that, but I'm realy
interested in a good practice guide for privilege and owning
management for PostgreSQL, like create an admin account without
superuser right, use samerole in pg_hba.conf and so on ...

> >  - select by default currently connected ROLE in the owner list
> > (replacing the blank filed) for new object creation
>
> OK.
>
The last bug I have is for database creation, I don't know how to get
the current login.

> >  - remove pg_global in the available tablespace list
>
> Probably a good idea, yes.
>
> >  - select current user default tablespace in tablespace list
> > (replacing the blank filed, yes I don't like blank field) for new
> > object creation
>
> OK.
>
> Regards, Dave.
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
>                http://www.postgresql.org/docs/faq
> Thomas

Re: Proposed Patchs

From
"Dave Page"
Date:

> -----Original Message-----
> From: Thomas Sondag [mailto:thomas.sondag@gmail.com]
> Sent: 29 May 2006 10:02
> To: Dave Page
> Cc: pgadmin-support@postgresql.org
> Subject: Re: [pgadmin-support] Proposed Patchs
>
> Hum, I miss this option ... sorry, but the main difference with the
> current behaviour is for object owning. The main idea was to set
> object owner to a group like that :
> database foo -> group foo
>   schema bar -> group bar
>   schema bar read user -> user toto
>
> I don't know if that's a good policy, but this case may exist, we may
> add an option like "Show Group for object owning" ?

Yes, that seems like a much better option - though I wonder if it should
default to just groups and then use the existing setting as per the
permissions. That would seem more consistent.

> The last bug I have is for database creation, I don't know how to get
> the current login.

Well dlgDatabase will automatically use the parent node's database (ie.
The maintenance DB) when appropriate so you should just be able to get
the username from that.

From dlgDatabase.cpp

dlgProperty *pgDatabaseFactory::CreateDialog(frmMain *frame, pgObject
*node, pgObject *parent)
{   dlgDatabase *dlg=new dlgDatabase(this, frame, (pgDatabase*)node);   if (dlg && !node)   {       // use the server's
connectionto avoid "template1 in use"       dlg->connection=parent->GetConnection();   }   return dlg; 
}

Regards, Dave.


Re: Proposed Patchs

From
"Thomas Sondag"
Date:
2006/5/29, Dave Page <dpage@vale-housing.co.uk>:
>
>
> > -----Original Message-----
> > From: Thomas Sondag [mailto:thomas.sondag@gmail.com]
> > Sent: 29 May 2006 10:02
> > To: Dave Page
> > Cc: pgadmin-support@postgresql.org
> > Subject: Re: [pgadmin-support] Proposed Patchs
> >
> > Hum, I miss this option ... sorry, but the main difference with the
> > current behaviour is for object owning. The main idea was to set
> > object owner to a group like that :
> > database foo -> group foo
> >   schema bar -> group bar
> >   schema bar read user -> user toto
> >
> > I don't know if that's a good policy, but this case may exist, we may
> > add an option like "Show Group for object owning" ?
>
> Yes, that seems like a much better option - though I wonder if it should
> default to just groups and then use the existing setting as per the
> permissions. That would seem more consistent.
>

I'm not sure this is the better way,  because most of the time objects
are owned by the current user, so showing only group by default can be
a bit confusing.

If you want  to see the result with the "Show group ..." option try
the folowing patch . the main idea for AddRoles function was to remove
one sql query.

I test this patch only with postgresql 8.1 and it stop segfaulting now :)

 Thomas


Re: Proposed Patchs

From
"Dave Page"
Date:

> -----Original Message-----
> From: Thomas Sondag [mailto:thomas.sondag@gmail.com]
> Sent: 30 May 2006 13:42
> To: Dave Page
> Cc: pgadmin-support@postgresql.org
> Subject: Re: [pgadmin-support] Proposed Patchs
>
> I'm not sure this is the better way,  because most of the time objects
> are owned by the current user, so showing only group by default can be
> a bit confusing.

There's no reason we cannot use groups + current user. I do think it
would be more confusing to have the behaviour work in precisely the
opposite way to the privileges.

> If you want  to see the result with the "Show group ..." option try
> the folowing patch . the main idea for AddRoles function was to remove
> one sql query.
>
> I test this patch only with postgresql 8.1 and it stop
> segfaulting now :)

Think you forgot the patch :-(

/D


Re: Proposed Patchs

From
"Thomas Sondag"
Date:
2006/5/30, Dave Page <dpage@vale-housing.co.uk>:
>
>
> > -----Original Message-----
> > From: Thomas Sondag [mailto:thomas.sondag@gmail.com]
> > Sent: 30 May 2006 13:42
> > To: Dave Page
> > Cc: pgadmin-support@postgresql.org
> > Subject: Re: [pgadmin-support] Proposed Patchs
> >
> > I'm not sure this is the better way,  because most of the time objects
> > are owned by the current user, so showing only group by default can be
> > a bit confusing.
>
> There's no reason we cannot use groups + current user. I do think it
> would be more confusing to have the behaviour work in precisely the
> opposite way to the privileges.
>
I'm agree, I wasn't see things that way, but that's different to the
current behaviour.

> > If you want  to see the result with the "Show group ..." option try
> > the folowing patch . the main idea for AddRoles function was to remove
> > one sql query.
> >
> > I test this patch only with postgresql 8.1 and it stop
> > segfaulting now :)
>
> Think you forgot the patch :-(
>
I hope you'll get it this time.

Thomas

Attachment