Thread: Re: [COMMITTERS] pgsql: Add pg_audit, an auditing extension

Re: [COMMITTERS] pgsql: Add pg_audit, an auditing extension

From
Alvaro Herrera
Date:
Tom Lane wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > I've pushed a change which should clean it up by simply loading the
> > module after each reconnects is done, more-or-less simulating having it
> > be in shared_preload_libraries.  It also wasn't using the correct
> > database for reconnecting.
> 
> > I'll keep an eye on it.
> 
> Another thing that looks not amazingly well-thought-out about that
> regression test is that it creates a superuser role with a known name
> (and no password, not that adding a password would make it better).

We create a lot of roles in other tests too; the foreign_data test is
the only one that create a superuser role.  While working on the tests
for the DDL deparse thing, I had to create a script with a list of roles
that all the tests use, and it's pretty amazing.  I remember thinking at
the time that it'd be better to initialize a number of standard roles in
an initial step, and have them be used consistently in the tests that
require them, rather than having create/drop everywhere.

-- create roles used throughout the tests
create role clstr_user;
create role "current_user";
create role foreign_data_user;
create role "Public";
create role regressgroup1;
create role regressgroup2;
create role regression_bob;
create role regression_group;
create role regression_user1;
create role regression_user2;
create role regression_user3;
create role regression_user;
create role regresslo;
create role regress_rol_lock1;
create role regress_test_indirect;
create role regress_test_role;
create role regress_test_role2;
create role regress_test_role_super superuser;
create role regressuser1;
create role regressuser2;
create role regressuser3;
create role regressuser4;
create role regressuser5;
create role regtest_unpriv_user;
create role regtest_addr_user;
create role regtest_alter_user1;
create role regtest_alter_user2;
create role regtest_alter_user3;
create role rls_regress_group1;
create role rls_regress_group2;
create role rls_regress_user0;
create role rls_regress_user1;
create role rls_regress_user2;
create role rls_regress_exempt_user;
create role schemauser2;
create role seclabel_user1;
create role seclabel_user2;
create role selinto_user;
create role testrol1;
create role testrol2;
create role testrolx;
create role unprivileged_role;
create role "user";
create role view_user2;

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



Re: [COMMITTERS] pgsql: Add pg_audit, an auditing extension

From
Tom Lane
Date:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Tom Lane wrote:
>> Another thing that looks not amazingly well-thought-out about that
>> regression test is that it creates a superuser role with a known name
>> (and no password, not that adding a password would make it better).

> We create a lot of roles in other tests too; the foreign_data test is
> the only one that create a superuser role.  While working on the tests
> for the DDL deparse thing, I had to create a script with a list of roles
> that all the tests use, and it's pretty amazing.  I remember thinking at
> the time that it'd be better to initialize a number of standard roles in
> an initial step, and have them be used consistently in the tests that
> require them, rather than having create/drop everywhere.

It would definitely be better if the names were less randomly chosen and
hence less likely to conflict with existing role names in an installation.
I'm not sure why we don't insist that they should all start with "regress"
or similar, for instance.

But what I'm on about at the moment is that I think creating new
superusers is a bad idea from a security standpoint.  It seems quite
unlikely that we *have* to do that for testing purposes.

Also, I notice that the pg_audit test fails to drop the roles it
created, even if it reaches the end successfully.  That's just bad.
        regards, tom lane