[REVIEW] row level security (v3)

Lists: pgsql-hackers
From: Mike Blackwell <mike(dot)blackwell(at)rrd(dot)com>
To: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>, Atri Sharma <atri(dot)jiit(at)gmail(dot)com>
Subject: [REVIEW] row level security (v3)
Date: 2013-07-09 19:28:58
Message-ID: CANPAkgvCeArBy0xZgkMjsD8Cav5LapwbAmc5f1AjS+Hn24gNsA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

The most recent patch (v3) applies and builds cleanly and passes make
check. Documentation on the new SQL syntax and catalog changes is included
with the patch and looks good to me.

The regression tests look pretty complete. In addition to the included
tests, dropping and altering the data type on a column referenced in the
security clause work as expected, rejecting the change with a dependency
error. Renaming a column succeeds as expected.

pg_dump and restore properly was also successful.

I noticed that the security clause is visible to any user via psql \dt+, as
well as in the pg_rowsecurity view. Perhaps this should be mentioned in
the relevant section of user-manag.sgml so users realize any sensitive
information in the security clause isn't secure.

What I've checked looks good. I don't feel qualified to do a code review
so that's still outstanding. I believe Atri will be looking at that.

____________________________________________________________
______________________

*Mike Blackwell | Technical Analyst, Distribution Services/Rollout
Management | RR Donnelley*
1750 Wallace Ave | St Charles, IL 60174-3401
Office: 630.313.7818
Mike(dot)Blackwell(at)rrd(dot)com
http://www.rrdonnelley.com

<http://www.rrdonnelley.com/>
* <Mike(dot)Blackwell(at)rrd(dot)com>*


From: Atri Sharma <atri(dot)jiit(at)gmail(dot)com>
To: Mike Blackwell <mike(dot)blackwell(at)rrd(dot)com>
Cc: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [REVIEW] row level security (v3)
Date: 2013-07-10 05:26:05
Message-ID: CAOeZVidCw+ynVoTWXLt1Pta-g8yzmh4AsEsfp7JvKWuaMXCoeg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Jul 10, 2013 at 12:58 AM, Mike Blackwell <mike(dot)blackwell(at)rrd(dot)com> wrote:
> The most recent patch (v3) applies and builds cleanly and passes make check.
> Documentation on the new SQL syntax and catalog changes is included with the
> patch and looks good to me.
>
> The regression tests look pretty complete. In addition to the included
> tests, dropping and altering the data type on a column referenced in the
> security clause work as expected, rejecting the change with a dependency
> error. Renaming a column succeeds as expected.
>
> pg_dump and restore properly was also successful.
>
> I noticed that the security clause is visible to any user via psql \dt+, as
> well as in the pg_rowsecurity view. Perhaps this should be mentioned in the
> relevant section of user-manag.sgml so users realize any sensitive
> information in the security clause isn't secure.
>
> What I've checked looks good. I don't feel qualified to do a code review so
> that's still outstanding. I believe Atri will be looking at that.
>

Hi All,

I, in my effort to support Mike's excellent work, will be helping out
with Kaigai san's patch's review.

I have been thinking of some tests that could be edge cases for the
patch, and will discuss them with Mike this week and test.

Also, in the first look, the patch looks good. I will give it a more
thorough look this week and send over a review to the max of my
capabilities.

Regards,

Atri

--
Regards,

Atri
l'apprenant