Merge lp:~abentley/launchpad/info-type-adds-policy into lp:launchpad

Proposed by Aaron Bentley
Status: Merged
Approved by: Aaron Bentley
Approved revision: no longer in the source branch.
Merged at revision: 16234
Proposed branch: lp:~abentley/launchpad/info-type-adds-policy
Merge into: lp:launchpad
Diff against target: 58 lines (+19/-3)
2 files modified
lib/lp/registry/model/product.py (+1/-0)
lib/lp/registry/tests/test_product.py (+18/-3)
To merge this branch: bzr merge lp:~abentley/launchpad/info-type-adds-policy
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email: mp+132749@code.launchpad.net

Commit message

Product information_type access policy is retained.

Description of the change

= Summary =
Fix bug #1074139: Setting policies different from project information_type locks the owner out

== Proposed fix ==
Update Product._pruneUnusedPolicies to consider Product.information_type when determining which policies can be deleted.

== Pre-implementation notes ==
None

== LOC Rationale ==
Part of private products.

== Implementation details ==
Cleaned up some lint as a driveby.

== Tests ==
bin/test -t test_information_type_prevents_pruning product

== Demo and Q/A ==
Create a Proprietary product. Change all the sharing policies to Public. You should still be able to see the product, and you should be listed under Proprietary.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/model/product.py
  lib/lp/registry/tests/test_product.py

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

I am surprised this fixes the issue. cr3 had a Public project, then made it Proprietary using +edit; he was locked out. I looked at the project and discovered 2 things wrong.

1. The sharing policies where not updated to reflect the transitive state of the project...the sharing policies were not changed to Proprietary.
2. Proprietary was not shared with the Maintainer.

I change a policy to proprietary, which automatically shared shared all proprietary with the maintainer, which restored cr3's access to the project. I then set the other sharing policies to Proprietary only.

LATER, cr3 made some sharing polices Public, but he was not locked out since Proprietary was still shared. I changed the Polices back to Proprietary because though Bugs and Blueprints could claimed to be public, they cannot be because privacy is transitive; staking a Public branch on a Proprietary branch is possible, but the branch will become Proprietary. A bug cannot really be public because the project/bug is not shared with the 1 million Lp users, and you cannot share any public information type.

Revision history for this message
Aaron Bentley (abentley) wrote :

Curtis, I started off with the assumption that making a public project proprietary could cause this bug, but I was unable to write a test case demonstrating this, because Product._set_information_type ensures that the appropriate access policy exists for the product's information type. That's why I wrote "It appears that the case where information_type is changed is already covered" in the bug. I'd be very interested if you can demonstrate otherwise.

I attempted to get more information from cr3, but he did not respond on IRC. So I proceeded with the assumption that the cause of the bug was different from what we originally supposed-- that changing the access policies, not changing the information type, had locked the user out.

I know you feel that privacy should be transitive, but that's orthagonal to this bug, because this bug can be exhibited even when no elements are public. As I noted in the bug: "However, there is another case: the project is Embargoed and the sharing policies are all proprietary. This situation would not be forbidden by Curtis's rule."

So let's keep this bug being about the fact that the maintainer can be locked out of their own product. Please file a separate bug for the lack of transitivity between Product and its policies. Deryck and I do not agree with you on that matter, but AIUI Deryck plans to discuss it with you.

Revision history for this message
Benji York (benji) wrote :

Thanks for the branch. Everything looks good to me.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/model/product.py'
2--- lib/lp/registry/model/product.py 2012-10-26 17:58:33 +0000
3+++ lib/lp/registry/model/product.py 2012-11-02 20:18:23 +0000
4@@ -704,6 +704,7 @@
5 )
6 allowed_types = allowed_bug_types.union(allowed_branch_types)
7 allowed_types = allowed_types.union(allowed_specification_types)
8+ allowed_types.add(self.information_type)
9 # Fetch all APs, and after filtering out ones that are forbidden
10 # by the bug, branch, and specification policies, the APs that have no
11 # APAs are unused and can be deleted.
12
13=== modified file 'lib/lp/registry/tests/test_product.py'
14--- lib/lp/registry/tests/test_product.py 2012-10-26 17:58:33 +0000
15+++ lib/lp/registry/tests/test_product.py 2012-11-02 20:18:23 +0000
16@@ -909,6 +909,21 @@
17 self.assertTrue(check_permission('launchpad.View', product))
18 self.assertFalse(check_permission('launchpad.View', product))
19
20+ def test_information_type_prevents_pruning(self):
21+ # Access policies for Product.information_type are not pruned.
22+ owner = self.factory.makePerson()
23+ for info_type in [
24+ InformationType.PROPRIETARY, InformationType.EMBARGOED]:
25+ product = self.factory.makeProduct(
26+ information_type=info_type, owner=owner)
27+ with person_logged_in(owner):
28+ product.setBugSharingPolicy(BugSharingPolicy.PUBLIC)
29+ product.setSpecificationSharingPolicy(
30+ SpecificationSharingPolicy.PUBLIC)
31+ product.setBranchSharingPolicy(BranchSharingPolicy.PUBLIC)
32+ self.assertIsNot(None, getUtility(IAccessPolicySource).find(
33+ [(product, info_type)]).one())
34+
35
36 class TestProductBugInformationTypes(TestCaseWithFactory):
37
38@@ -1877,7 +1892,7 @@
39 def test_users_private_products(self):
40 # Ignore any public products the user may own.
41 owner = self.factory.makePerson()
42- public = self.factory.makeProduct(
43+ self.factory.makeProduct(
44 information_type=InformationType.PUBLIC,
45 owner=owner)
46 proprietary = self.factory.makeProduct(
47@@ -1990,9 +2005,9 @@
48 product = self.factory.makeProduct(
49 owner=owner, translations_usage=ServiceUsage.LAUNCHPAD)
50 series = self.factory.makeProductSeries(product)
51- po_template = self.factory.makePOTemplate(productseries=series)
52+ self.factory.makePOTemplate(productseries=series)
53 with person_logged_in(owner):
54- product.information_type=InformationType.PROPRIETARY
55+ product.information_type = InformationType.PROPRIETARY
56 # Anonymous users do not see private products.
57 with person_logged_in(ANONYMOUS):
58 translatables = getUtility(IProductSet).getTranslatables()