Merge lp:~jtv/launchpad/bug-878115 into lp:launchpad

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: William Grant
Approved revision: no longer in the source branch.
Merged at revision: 14177
Proposed branch: lp:~jtv/launchpad/bug-878115
Merge into: lp:launchpad
Diff against target: 12 lines (+1/-1)
1 file modified
database/schema/security.cfg (+1/-1)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-878115
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+79919@code.launchpad.net

Commit message

Missing DELETE privilege for poimport on TranslationMessage.

Description of the change

= Summary =

See bug 878115 for a description of the problem.

== Proposed fix ==

Let's not complicate this. Add the missing privilege.

== Pre-implementation notes ==

Discussed with Stuart and with Julian: we really ought to get rid of these fine-grained database privileges for anything that's not in any way sensitive. Far more trouble than it's worth.

For keeping track of who accesses what, we should monitor accesses, not create an endless stream of critical bugs.

== Implementation details ==

I thought this had to go to db-devel in our current process, but William says it should go to devel.

== Tests ==

You got me there. I didn't add any.

Why? Well the _reason_ is that I'm at the same time lazy and eager to get this problem fixed. My _justifications_ are:
 * This is a policy change, not a mechanism change. No point in testing that each security.cfg entry does what it's supposed to.
 * The code path that fails is relatively obscure. I can change the unit-tests to run with "realistic" database privileges, at the cost of extra test time, but then the question becomes whether I missed any roles that might do the same thing.
 * The problem is unlikely to recur in its exact same form. How do you test for not having missed anything in testing, such as a different database role that also executes the same code?
 * Actually, if I test this under the poimport DB role, I _know_ I will be ignoring a realistic case: the appserver goes through the same code paths, but it already has the privileges it needs.

What if I got the diagnosis wrong? Then I fixed a different critical bug than what caused this one. Still worthwhile.

== Demo and Q/A ==

The problem should not recur on the launchpad-errors-reports list.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  database/schema/security.cfg

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/schema/security.cfg'
2--- database/schema/security.cfg 2011-10-03 18:34:08 +0000
3+++ database/schema/security.cfg 2011-10-20 07:10:28 +0000
4@@ -455,7 +455,7 @@
5 public.customlanguagecode = SELECT
6 public.translationgroup = SELECT
7 public.translationimportqueueentry = SELECT
8-public.translationmessage = SELECT, INSERT, UPDATE
9+public.translationmessage = SELECT, DELETE, INSERT, UPDATE
10 public.translationrelicensingagreement = SELECT
11 public.translator = SELECT
12 public.validpersoncache = SELECT