Re: [HACKERS] [WIP] ALTER ... OWNER TO ... CASCADE

[Vladimir Borodin]

> 16 февр. 2016 г., в 18:20, Alvaro Herrera  
> написал(а):
> 
> Vladimir Borodin wrote:
> 
>>> Moreover, the use case you've sketched (ie, change ownership of all
>>> objects inside a database) doesn't actually have anything to do with
>>> following dependencies.  It's a lot closer to REASSIGN OWNED ... in
>>> fact, it's not clear to me why REASSIGN OWNED doesn't solve that
>>> use-case already.
>> 
>> Sometimes I hit the following. You have created a database and schema
>> inside it from the superuser (i.e. postgres). Than you want to change
>> ownership of whole database to another user (i.e. alice), but only
>> this database, not all other objects in all other databases. It seems
>> that REASSIGN OWNED doesn’t solve this already.
> 
> So essentially you want to change all the objects in the database except
> those that were created together with the database itself (i.e. those
> that were copied from the template database).  

Yes. Without such syntax it is now done in a really awful way now, i.e. [0].

[0] 
https://github.com/saltstack/salt/blob/405d0aef1cf11bb56b5d2320b176f6992e6cdf3b/salt/modules/postgres.py#L1806-L1847

> That seems a reasonable
> use-case, but I'm not sure that this ALTER .. OWNER CASCADE is the right
> thing for that -- What object would you start with?  Each schema other
> than pg_catalog, pg_toast, information_schema?  As I recall, the problem
> is that REASSIGN OWNED refuses to work on pinned objects.  Maybe what
> you want is something like
>  REASSIGN OWNED BY xyz IN SCHEMA public TO xyzxxz
> i.e., an extension of the current REASSIGN OWNED BY command?

Well, I don’t know what syntax and implementation would be correct. I just want 
to give a specific user all rights to manage all objects in a specific database 
(which was created from postgres user earlier). It would be really useful.

> 
> -- 
> Álvaro Herrerahttp://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


--
Да пребудет с вами сила…
https://simply.name/ru

Re: [HACKERS] [WIP] ALTER ... OWNER TO ... CASCADE

[Alvaro Herrera]

Vladimir Borodin wrote:

> > Moreover, the use case you've sketched (ie, change ownership of all
> > objects inside a database) doesn't actually have anything to do with
> > following dependencies.  It's a lot closer to REASSIGN OWNED ... in
> > fact, it's not clear to me why REASSIGN OWNED doesn't solve that
> > use-case already.
> 
> Sometimes I hit the following. You have created a database and schema
> inside it from the superuser (i.e. postgres). Than you want to change
> ownership of whole database to another user (i.e. alice), but only
> this database, not all other objects in all other databases. It seems
> that REASSIGN OWNED doesn’t solve this already.

So essentially you want to change all the objects in the database except
those that were created together with the database itself (i.e. those
that were copied from the template database).  That seems a reasonable
use-case, but I'm not sure that this ALTER .. OWNER CASCADE is the right
thing for that -- What object would you start with?  Each schema other
than pg_catalog, pg_toast, information_schema?  As I recall, the problem
is that REASSIGN OWNED refuses to work on pinned objects.  Maybe what
you want is something like
  REASSIGN OWNED BY xyz IN SCHEMA public TO xyzxxz
i.e., an extension of the current REASSIGN OWNED BY command?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [WIP] ALTER ... OWNER TO ... CASCADE

[Dmitry Ivanov]

> Sometimes I hit the following. You have created a database and schema inside
> it from the superuser (i.e. postgres). Than you want to change ownership of
> whole database to another user (i.e. alice), but only this database, not
> all other objects in all other databases. 

Actually, it skips all files that belong to irrelevant databases:

/*
 * We only operate on shared objects and objects in the current
 * database
 */
if (sdepForm->dbid != MyDatabaseId &&
sdepForm->dbid != InvalidOid)
continue;

> It seems that REASSIGN OWNED doesn’t solve this already.

Yes, it doesn't solve this case. This is due to the fact that if the superuser 
that created the database is 'pinned' (e.g. postgres), it is impossible to 
track any object which depends on him, since such a dependency is not present 
in the pg_shdepend (pay attention to the comment below):

if (isSharedObjectPinned(AuthIdRelationId, roleid, sdepRel))
{
.
ereport(ERROR,
(errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
 errmsg("cannot reassign ownership

/*
 * There's no need to tell the whole truth, which is that we
 * didn't track these dependencies at all ...
 */
}

This prevents you from doing something like:

test=# reassign owned by postgres to test;
ERROR:  cannot reassign ownership of objects owned by role postgres because 
they are required by the database system

I think that my solution might fit better.

-- 
Dmitry Ivanov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [WIP] ALTER ... OWNER TO ... CASCADE

[Oleg Bartunov]

On Mon, Feb 15, 2016 at 7:25 PM, Tom Lane  wrote:

> Teodor Sigaev  writes:
> >> So basically, a generic CASCADE facility sounds like a lot of work to
> >> produce something that would seldom be anything but a foot-gun.
>
> > DELETE FROM  or TRUNCATE could be a foot-gun too, but it's not a reason
> to
> > remove tham. I faced with problem when I tried to change owner of
> datadase with
> > all objects inside. Think, this feature could be useful although it
> should
> > restricted to superuser obly.
>
> That's a pretty weak argument, and I do not think you have thought through
> all the consequences.  It is not hard at all to imagine cases where using
> this sort of thing could be a security vulnerability.  Are you familiar
> with the reasons why Unix systems don't typically allow users to "give
> away" ownership of files?  The same problem exists here.
>

yes, I remember AT and BSD :)



>
> To be concrete about it:
>
> 1. Alice does, say, "CREATE EXTENSION cube".
>
> 2. Bob creates a security-definer function owned by himself, using a
>"cube"-type parameter so that it's dependent on the extension.
>(It needn't actually do anything with that parameter.)
>
> 3. Alice does ALTER EXTENSION cube OWNER TO charlie CASCADE.
>
> 4. Bob now has a security-definer function owned by (and therefore
>executing as) Charlie, whose contents were determined by Bob.
>Game over for Charlie ... and for everyone else too, if Charlie is
>a superuser, which is not unlikely for an extension owner.
>
> The only way Alice can be sure that the ALTER EXTENSION is safe is if
> she manually inspects every dependent object, in which case she might
> as well not use CASCADE.
>
> Moreover, the use case you've sketched (ie, change ownership of all
> objects inside a database) doesn't actually have anything to do with
> following dependencies.  It's a lot closer to REASSIGN OWNED ... in
> fact, it's not clear to me why REASSIGN OWNED doesn't solve that
> use-case already.
>
> I remain of the opinion that this is a terrible idea.
>

+1, I also suggest to check REASSIGN OWNED.


>
>
regards, tom lane
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

Re: [HACKERS] [WIP] ALTER ... OWNER TO ... CASCADE

[Vladimir Borodin]

> 15 февр. 2016 г., в 19:25, Tom Lane  написал(а):
> 
> Teodor Sigaev  writes:
>>> So basically, a generic CASCADE facility sounds like a lot of work to
>>> produce something that would seldom be anything but a foot-gun.
> 
>> DELETE FROM  or TRUNCATE could be a foot-gun too, but it's not a reason to 
>> remove tham. I faced with problem when I tried to change owner of datadase 
>> with 
>> all objects inside. Think, this feature could be useful although it should 
>> restricted to superuser obly.
> 
> That's a pretty weak argument, and I do not think you have thought through
> all the consequences.  It is not hard at all to imagine cases where using
> this sort of thing could be a security vulnerability.  Are you familiar
> with the reasons why Unix systems don't typically allow users to "give
> away" ownership of files?  The same problem exists here.
> 
> To be concrete about it:
> 
> 1. Alice does, say, "CREATE EXTENSION cube".
> 
> 2. Bob creates a security-definer function owned by himself, using a
>   "cube"-type parameter so that it's dependent on the extension.
>   (It needn't actually do anything with that parameter.)
> 
> 3. Alice does ALTER EXTENSION cube OWNER TO charlie CASCADE.
> 
> 4. Bob now has a security-definer function owned by (and therefore
>   executing as) Charlie, whose contents were determined by Bob.
>   Game over for Charlie ... and for everyone else too, if Charlie is
>   a superuser, which is not unlikely for an extension owner.
> 
> The only way Alice can be sure that the ALTER EXTENSION is safe is if
> she manually inspects every dependent object, in which case she might
> as well not use CASCADE.

Seems to be a problem that should be addressed.

> 
> Moreover, the use case you've sketched (ie, change ownership of all
> objects inside a database) doesn't actually have anything to do with
> following dependencies.  It's a lot closer to REASSIGN OWNED ... in
> fact, it's not clear to me why REASSIGN OWNED doesn't solve that
> use-case already.

Sometimes I hit the following. You have created a database and schema inside it 
from the superuser (i.e. postgres). Than you want to change ownership of whole 
database to another user (i.e. alice), but only this database, not all other 
objects in all other databases. It seems that REASSIGN OWNED doesn’t solve this 
already.

> 
> I remain of the opinion that this is a terrible idea.
> 
>   regards, tom lane
> 
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers


--
May the force be with you…
https://simply.name

Re: [HACKERS] [WIP] ALTER ... OWNER TO ... CASCADE

[Tom Lane]

Teodor Sigaev  writes:
>> So basically, a generic CASCADE facility sounds like a lot of work to
>> produce something that would seldom be anything but a foot-gun.

> DELETE FROM  or TRUNCATE could be a foot-gun too, but it's not a reason to 
> remove tham. I faced with problem when I tried to change owner of datadase 
> with 
> all objects inside. Think, this feature could be useful although it should 
> restricted to superuser obly.

That's a pretty weak argument, and I do not think you have thought through
all the consequences.  It is not hard at all to imagine cases where using
this sort of thing could be a security vulnerability.  Are you familiar
with the reasons why Unix systems don't typically allow users to "give
away" ownership of files?  The same problem exists here.

To be concrete about it:

1. Alice does, say, "CREATE EXTENSION cube".

2. Bob creates a security-definer function owned by himself, using a
   "cube"-type parameter so that it's dependent on the extension.
   (It needn't actually do anything with that parameter.)

3. Alice does ALTER EXTENSION cube OWNER TO charlie CASCADE.

4. Bob now has a security-definer function owned by (and therefore
   executing as) Charlie, whose contents were determined by Bob.
   Game over for Charlie ... and for everyone else too, if Charlie is
   a superuser, which is not unlikely for an extension owner.

The only way Alice can be sure that the ALTER EXTENSION is safe is if
she manually inspects every dependent object, in which case she might
as well not use CASCADE.

Moreover, the use case you've sketched (ie, change ownership of all
objects inside a database) doesn't actually have anything to do with
following dependencies.  It's a lot closer to REASSIGN OWNED ... in
fact, it's not clear to me why REASSIGN OWNED doesn't solve that
use-case already.

I remain of the opinion that this is a terrible idea.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [WIP] ALTER ... OWNER TO ... CASCADE

[Teodor Sigaev]

TBH, this sounds like a completely terrible idea.  There are far too many
sorts of dependencies across which one would not expect ownership to
propagate; for example, use of a function in a view, or even just a
foreign key dependency between two tables.

I'm not even clear that there are *any* cases where this behavior is
wanted, other than perhaps ALTER OWNER on an extension --- and even there,
what you would want is altering the ownership of the member objects, but
not everything that depends on the member objects.

So basically, a generic CASCADE facility sounds like a lot of work to
produce something that would seldom be anything but a foot-gun.


DELETE FROM  or TRUNCATE could be a foot-gun too, but it's not a reason to 
remove tham. I faced with problem when I tried to change owner of datadase with 
all objects inside. Think, this feature could be useful although it should 
restricted to superuser obly.


--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [WIP] ALTER ... OWNER TO ... CASCADE

[Dmitry Ivanov]

> Dmitry Ivanov  writes:
> > As of now there's no way to transfer the ownership of an object and all
> > its
> > dependent objects in one step. One has to manually alter the owner of each
> > object, be it a table, a schema or something else.
> 
> TBH, this sounds like a completely terrible idea.  There are far too many
> sorts of dependencies across which one would not expect ownership to
> propagate; for example, use of a function in a view, or even just a
> foreign key dependency between two tables.

Well, actually this is a statement of the fact, and I don't propose enabling 
this option for every dependency possible. This patch includes an experimental 
feature that anyone can try and discuss, nothing more. Besides, it acts a lot 
like 'drop ... cascade' (the findDependentObjects() function is used under the 
hood), so the behavior is totally predictable. It also writes down all objects 
that have been touched, so no change goes unnoticed.

> I'm not even clear that there are *any* cases where this behavior is
> wanted, other than perhaps ALTER OWNER on an extension

At very least this might be useful in order to change owner of all tables 
which inherit some table.

-- 
Dmitry Ivanov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [WIP] ALTER ... OWNER TO ... CASCADE

[Tom Lane]

Dmitry Ivanov  writes:
> As of now there's no way to transfer the ownership of an object and all its 
> dependent objects in one step. One has to manually alter the owner of each 
> object, be it a table, a schema or something else.

TBH, this sounds like a completely terrible idea.  There are far too many
sorts of dependencies across which one would not expect ownership to
propagate; for example, use of a function in a view, or even just a
foreign key dependency between two tables.

I'm not even clear that there are *any* cases where this behavior is
wanted, other than perhaps ALTER OWNER on an extension --- and even there,
what you would want is altering the ownership of the member objects, but
not everything that depends on the member objects.

So basically, a generic CASCADE facility sounds like a lot of work to
produce something that would seldom be anything but a foot-gun.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] [WIP] ALTER ... OWNER TO ... CASCADE

[Dmitry Ivanov]

Hi hackers,

Recently I've been working on a CASCADE option for ALTER ... OWNER TO 
statement. Although it's still a working prototype, I think it's time to share 
my work.


Introduction


As of now there's no way to transfer the ownership of an object and all its 
dependent objects in one step. One has to manually alter the owner of each 
object, be it a table, a schema or something else. This patch adds the 
'CASCADE' option to every 'ALTER X OWNER TO' statement, including the 'ALTER 
DATABASE db OWNER TO user CASCADE' which turns out to be a delicate matter.


Implementation
==

There are two functions that process 'ALTER ... OWNER' statement: 
ExecAlterOwnerStmt() and ATExecCmd(). The latter function deals with the tasks 
that refer to all kinds of relations, while the first one handles the remaining 
object types. Basically, all I had to do is to add 'cascade' flag to the 
corresponding parsenodes and to make these functions call the dependency tree 
walker function (which would change the ownership of the dependent objects if 
needed). Of course, there are various corner cases for each kind of objects 
that require special treatment, but the code speaks for itself.

The aforementioned 'ALTER DATABASE db ...' is handled in a special way. Since 
objects that don't belong to the 'current database' are hidden, it is 
impossible to change their owner directly, so we have to do the job in the 
background worker that is connected to the 'db'. I'm not sure if this is the 
best solution available, but anyway.


What works
==

Actually, it seems to work in simple cases like 'a table with its inheritors' 
or 'a schema full of tables', but of course there might be things I've 
overlooked. There are some regression tests, though, and I'll continue to 
write some more.


What's dubious
==

It is unclear what kinds of objects should be transferred in case of database 
ownership change, since there's no way to get the full list of objects that 
depend on a given database. Currently the code changes ownership of all 
schemas (excluding the 'information_schema' and some others) and their 
contents, but this is a temporary limitation.

Feedback is welcome!


-- 
Dmitry Ivanov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Companydiff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index c48e37b..54be671 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -21,6 +21,7 @@
 #include "catalog/index.h"
 #include "catalog/objectaccess.h"
 #include "catalog/pg_amop.h"
+#include "catalog/pg_aggregate.h"
 #include "catalog/pg_amproc.h"
 #include "catalog/pg_attrdef.h"
 #include "catalog/pg_authid.h"
@@ -40,6 +41,7 @@
 #include "catalog/pg_foreign_server.h"
 #include "catalog/pg_language.h"
 #include "catalog/pg_largeobject.h"
+#include "catalog/pg_largeobject_metadata.h"
 #include "catalog/pg_namespace.h"
 #include "catalog/pg_opclass.h"
 #include "catalog/pg_operator.h"
@@ -56,6 +58,7 @@
 #include "catalog/pg_ts_template.h"
 #include "catalog/pg_type.h"
 #include "catalog/pg_user_mapping.h"
+#include "commands/alter.h"
 #include "commands/comment.h"
 #include "commands/defrem.h"
 #include "commands/event_trigger.h"
@@ -66,6 +69,7 @@
 #include "commands/seclabel.h"
 #include "commands/trigger.h"
 #include "commands/typecmds.h"
+#include "commands/tablecmds.h"
 #include "nodes/nodeFuncs.h"
 #include "parser/parsetree.h"
 #include "rewrite/rewriteRemove.h"
@@ -77,17 +81,6 @@
 #include "utils/tqual.h"
 
 
-/*
- * Deletion processing requires additional state for each ObjectAddress that
- * it's planning to delete.  For simplicity and code-sharing we make the
- * ObjectAddresses code support arrays with or without this extra state.
- */
-typedef struct
-{
-	int			flags;			/* bitmask, see bit definitions below */
-	ObjectAddress dependee;		/* object whose deletion forced this one */
-} ObjectAddressExtra;
-
 /* ObjectAddressExtra flag bits */
 #define DEPFLAG_ORIGINAL	0x0001		/* an original deletion target */
 #define DEPFLAG_NORMAL		0x0002		/* reached via normal dependency */
@@ -97,17 +90,6 @@ typedef struct
 #define DEPFLAG_REVERSE		0x0020		/* reverse internal/extension link */
 
 
-/* expansible list of ObjectAddresses */
-struct ObjectAddresses
-{
-	ObjectAddress *refs;		/* => palloc'd array */
-	ObjectAddressExtra *extras; /* => palloc'd array, or NULL if not used */
-	int			numrefs;		/* current number of references */
-	int			maxrefs;		/* current size of palloc'd array(s) */
-};
-
-/* typedef ObjectAddresses appears in dependency.h */
-
 /* threaded list of ObjectAddresses, for recursion detection */
 typedef struct ObjectAddressStack
 {
@@ -164,12 +146,21 @@ static const Oid object_classes[] = {
 };
 
 
+/*
+ * We limit the number of dependencies reported to the client to
+ * MAX_REPORTED_DEPS, since client software may not deal well with
+ * enormous error strings.  The server log always gets a