launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13532
[Merge] lp:~sinzui/launchpad/trusted-series-branches into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/trusted-series-branches into lp:launchpad.
Commit message:
Do not permit linking series to branches owned by open teams.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/trusted-series-branches/+merge/130436
Project drivers can set a series branch to one owned by an open
team. This means anyone can compromise the project's code and
this compromise could cascade to PPAs via recipes.
This branch will not land until we have dealt with 163 problem series.
--------------------------------------------------------------------
RULES
Pre-implementation: wallyworld, wgrant
* Add a constraint that only lets users link series to branches
owned by exclusive teams.
* Add rule to accept the current branch value so that ~150
projects continue to work while we work to harden them.
* The existing BranchRestrictedOnProductVocabulary can be
update.
* Add a constraint to teams to ensure they cannot become
inclusive if they own branches that are linked to series.
ADDENDUM: flacoste
* Given the horror seen in https://pastebin.canonical.com/76876/
we will grant an exception to existing violators of this rule.
* We will fix all the series, teams, or branches we can ourselves.
* Send an email to the remaining project owners and teams asking
them to change the team or the branch owner.
* We will ask webops to unlink any branches after a deadline.
We will use SQL to set the series branch to NULL.
QA
* Push a branch to https://qastaging.launchpad.net/~flip-flop/gdp.
* Visit https://qastaging.launchpad.net/gdp/packaging/+edit
* Enter the URL to the inclusive team's branch.
* Verify that the form does not permit this.
* Change the team to exclusive
* Visit https://qastaging.launchpad.net/gdp/packaging/+edit
* Link the series to branch
* Visit https://qastaging.launchpad.net/~flip-flop/+edit
and try to change the team to Open.
* Verify the form does not permit the action.
LINT
lib/lp/code/interfaces/branchcollection.py
lib/lp/code/model/branchcollection.py
lib/lp/code/model/tests/test_branchcollection.py
lib/lp/code/vocabularies/branch.py
lib/lp/code/vocabularies/tests/test_branch_vocabularies.py
lib/lp/registry/model/person.py
lib/lp/registry/tests/test_team.py
TEST
./bin/test -vvc -t isExclusive lp.code.model.tests.test_branchcollection
./bin/test -vvc -t RestrictedBranchVocabulary lp.code.vocabularies.tests
./bin/test -vvc -t TestTeamMembershipPolicyCh lp.registry.tests.test_team
IMPLEMENTATION
I added infrastructure support to branch collection to get exclusive and
series branches.
lib/lp/code/interfaces/branchcollection.py
lib/lp/code/model/branchcollection.py
lib/lp/code/model/tests/test_branchcollection.py
I updated the existing vocabulary to further restrict product series branches
to branches owned by exclusive teams and users.
lib/lp/code/vocabularies/branch.py
lib/lp/code/vocabularies/tests/test_branch_vocabularies.py
I added a rule to prevent teams from becoming open if they own series branches.
lib/lp/registry/model/person.py
lib/lp/registry/tests/test_team.py
--
https://code.launchpad.net/~sinzui/launchpad/trusted-series-branches/+merge/130436
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/trusted-series-branches into lp:launchpad.
=== modified file 'lib/lp/code/interfaces/branchcollection.py'
--- lib/lp/code/interfaces/branchcollection.py 2012-07-17 21:58:06 +0000
+++ lib/lp/code/interfaces/branchcollection.py 2012-10-18 21:43:23 +0000
@@ -163,6 +163,12 @@
def isPrivate():
"""Restrict the collection to private branches."""
+ def isExclusive():
+ """Restrict the collection to branches owned by exclusive people."""
+
+ def isSeries():
+ """Restrict the collection to branches those linked to series."""
+
def ownedBy(person):
"""Restrict the collection to branches owned by 'person'."""
=== modified file 'lib/lp/code/model/branchcollection.py'
--- lib/lp/code/model/branchcollection.py 2012-09-28 06:25:44 +0000
+++ lib/lp/code/model/branchcollection.py 2012-10-18 21:43:23 +0000
@@ -63,6 +63,7 @@
from lp.code.model.codereviewvote import CodeReviewVoteReference
from lp.code.model.revision import Revision
from lp.code.model.seriessourcepackagebranch import SeriesSourcePackageBranch
+from lp.registry.enums import EXCLUSIVE_TEAM_POLICY
from lp.registry.model.distribution import Distribution
from lp.registry.model.distroseries import DistroSeries
from lp.registry.model.person import (
@@ -613,6 +614,22 @@
return self._filterBy([
Branch.information_type.is_in(PRIVATE_INFORMATION_TYPES)])
+ def isExclusive(self):
+ """See `IBranchCollection`."""
+ return self._filterBy(
+ [Person.membership_policy.is_in(EXCLUSIVE_TEAM_POLICY)],
+ table=Person,
+ join=Join(Person, Branch.ownerID == Person.id))
+
+ def isSeries(self):
+ """See `IBranchCollection`."""
+ # ProductSeries import's this module.
+ from lp.registry.model.productseries import ProductSeries
+ return self._filterBy(
+ [Branch.id == ProductSeries.branchID],
+ table=ProductSeries,
+ join=Join(ProductSeries, Branch.id == ProductSeries.branchID))
+
def ownedBy(self, person):
"""See `IBranchCollection`."""
return self._filterBy([Branch.owner == person], symmetric=False)
=== modified file 'lib/lp/code/model/tests/test_branchcollection.py'
--- lib/lp/code/model/tests/test_branchcollection.py 2012-09-28 06:25:44 +0000
+++ lib/lp/code/model/tests/test_branchcollection.py 2012-10-18 21:43:23 +0000
@@ -291,6 +291,65 @@
collection = self.all_branches.inProject(project)
self.assertEqual([branch], list(collection.getBranches()))
+ def test_isExclusive(self):
+ # 'isExclusive' is restricted to branches owned by exclusive
+ # teams and users.
+ user = self.factory.makePerson()
+ team = self.factory.makeTeam(
+ membership_policy=TeamMembershipPolicy.RESTRICTED)
+ other_team = self.factory.makeTeam(
+ membership_policy=TeamMembershipPolicy.OPEN)
+ team_branch = self.factory.makeAnyBranch(owner=team)
+ user_branch = self.factory.makeAnyBranch(owner=user)
+ self.factory.makeAnyBranch(owner=other_team)
+ collection = self.all_branches.isExclusive()
+ self.assertContentEqual(
+ [team_branch, user_branch], list(collection.getBranches()))
+
+ def test_inProduct_and_isExclusive(self):
+ # 'inProduct' and 'isExclusive' can combine to form a collection that
+ # is restricted to branches of a particular product owned exclusive
+ # teams and users.
+ team = self.factory.makeTeam(
+ membership_policy=TeamMembershipPolicy.RESTRICTED)
+ other_team = self.factory.makeTeam(
+ membership_policy=TeamMembershipPolicy.OPEN)
+ product = self.factory.makeProduct()
+ branch = self.factory.makeProductBranch(product=product, owner=team)
+ self.factory.makeAnyBranch(owner=team)
+ self.factory.makeProductBranch(product=product, owner=other_team)
+ collection = self.all_branches.inProduct(product).isExclusive()
+ self.assertEqual([branch], list(collection.getBranches()))
+ collection = self.all_branches.isExclusive().inProduct(product)
+ self.assertEqual([branch], list(collection.getBranches()))
+
+ def test_isSeries(self):
+ # 'isSeries' is restricted to branches linked to product series.
+ series = self.factory.makeProductSeries()
+ branch = self.factory.makeAnyBranch(product=series.product)
+ with person_logged_in(series.product.owner):
+ series.branch = branch
+ self.factory.makeAnyBranch(product=series.product)
+ collection = self.all_branches.isSeries()
+ self.assertContentEqual([branch], list(collection.getBranches()))
+
+ def test_ownedBy_and_isSeries(self):
+ # 'ownedBy' and 'inSeries' can combine to form a collection that is
+ # restricted to branches linked to product series owned by a particular
+ # person.
+ person = self.factory.makePerson()
+ series = self.factory.makeProductSeries()
+ branch = self.factory.makeProductBranch(
+ product=series.product, owner=person)
+ with person_logged_in(series.product.owner):
+ series.branch = branch
+ self.factory.makeAnyBranch(owner=person)
+ self.factory.makeProductBranch(product=series.product)
+ collection = self.all_branches.isSeries().ownedBy(person)
+ self.assertEqual([branch], list(collection.getBranches()))
+ collection = self.all_branches.ownedBy(person).isSeries()
+ self.assertEqual([branch], list(collection.getBranches()))
+
def test_ownedBy_and_inProduct(self):
# 'ownedBy' and 'inProduct' can combine to form a collection that is
# restricted to branches of a particular product owned by a particular
=== modified file 'lib/lp/code/vocabularies/branch.py'
--- lib/lp/code/vocabularies/branch.py 2012-01-01 02:58:52 +0000
+++ lib/lp/code/vocabularies/branch.py 2012-10-18 21:43:23 +0000
@@ -110,7 +110,7 @@
raise AssertionError('Unexpected context type')
def _getCollection(self):
- return getUtility(IAllBranches).inProduct(self.product)
+ return getUtility(IAllBranches).inProduct(self.product).isExclusive()
class HostedBranchRestrictedOnOwnerVocabulary(BranchVocabularyBase):
=== modified file 'lib/lp/code/vocabularies/tests/test_branch_vocabularies.py'
--- lib/lp/code/vocabularies/tests/test_branch_vocabularies.py 2012-01-01 02:58:52 +0000
+++ lib/lp/code/vocabularies/tests/test_branch_vocabularies.py 2012-10-18 21:43:23 +0000
@@ -14,6 +14,7 @@
BranchRestrictedOnProductVocabulary,
BranchVocabulary,
)
+from lp.registry.enums import TeamMembershipPolicy
from lp.registry.interfaces.product import IProductSet
from lp.testing import (
ANONYMOUS,
@@ -160,6 +161,7 @@
person = factory.makePerson(name='spotty')
factory.makeProductBranch(
owner=person, product=test_product, name='hill')
+ self.product = test_product
def test_mainBranches(self):
"""Look for widget's main branch.
@@ -203,6 +205,19 @@
self.vocab.getTermByToken,
'scotty')
+ def test_does_not_contain_inclusive_teams(self):
+ factory = LaunchpadObjectFactory()
+ open_team = factory.makeTeam(name='open-team',
+ membership_policy=TeamMembershipPolicy.OPEN)
+ delegated_team = factory.makeTeam(name='delegated-team',
+ membership_policy=TeamMembershipPolicy.DELEGATED)
+ for team in [open_team, delegated_team]:
+ factory.makeProductBranch(
+ owner=team, product=self.product, name='mountain')
+ results = self.vocab.searchForTerms('mountain')
+ branch_names = sorted([branch.token for branch in results])
+ self.assertEqual(['~scotty/widget/mountain'], branch_names)
+
class TestRestrictedBranchVocabularyOnBranch(
TestRestrictedBranchVocabularyOnProduct):
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2012-10-16 23:42:55 +0000
+++ lib/lp/registry/model/person.py 2012-10-18 21:43:23 +0000
@@ -1755,6 +1755,11 @@
"The team membership policy cannot be %s because one "
"or more if its super teams are not open." % policy)
+ # Does the team own a productseries.branch?
+ if getUtility(IAllBranches).ownedBy(self).isSeries().count() != 0:
+ raise TeamMembershipPolicyError(
+ "The team membership policy cannot be %s because it owns "
+ "or more branches linked to project series." % policy)
# Does this team subscribe or is assigned to any private bugs.
# Circular imports.
from lp.bugs.model.bug import Bug
=== modified file 'lib/lp/registry/tests/test_team.py'
--- lib/lp/registry/tests/test_team.py 2012-09-18 19:41:02 +0000
+++ lib/lp/registry/tests/test_team.py 2012-10-18 21:43:23 +0000
@@ -395,6 +395,32 @@
self.assertEqual(
None, self.field.validate(TeamMembershipPolicy.OPEN))
+ def test_closed_team_with_series_branch_cannot_become_open(self):
+ # The team cannot become open if it has a branch linked to
+ # a product series.
+ self.setUpTeams()
+ series = self.factory.makeProductSeries()
+ branch = self.factory.makeProductBranch(
+ product=series.product, owner=self.team)
+ with person_logged_in(series.product.owner):
+ series.branch = branch
+ self.assertFalse(
+ self.field.constraint(TeamMembershipPolicy.OPEN))
+ self.assertRaises(
+ TeamMembershipPolicyError, self.field.validate,
+ TeamMembershipPolicy.OPEN)
+
+ def test_closed_team_without_a_series_branch_can_become_open(self):
+ # The team can become open if does not have a branch linked to
+ # a product series.
+ self.setUpTeams()
+ series = self.factory.makeProductSeries()
+ self.factory.makeProductBranch(product=series.product, owner=self.team)
+ self.assertTrue(
+ self.field.constraint(TeamMembershipPolicy.OPEN))
+ self.assertEqual(
+ None, self.field.validate(TeamMembershipPolicy.OPEN))
+
def test_closed_team_with_private_bugs_cannot_become_open(self):
# The team cannot become open if it is subscribed to private bugs.
self.setUpTeams()
Follow ups
