launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17941
[Merge] lp:~cjwatson/launchpad/git-finish-sharing into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/git-finish-sharing into lp:launchpad with lp:~cjwatson/launchpad/git-collection as a prerequisite.
Commit message:
Use Git repository collections to implement visibility and sharing.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1032731 in Launchpad itself: "Support for Launchpad-hosted Git repositories"
https://bugs.launchpad.net/launchpad/+bug/1032731
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-finish-sharing/+merge/250663
Use Git repository collections to implement visibility and sharing.
There are a few bits we can't do until there's some analogue of BranchSubscription, but otherwise this should be a reasonably complete privacy implementation now.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-finish-sharing into lp:launchpad.
=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py 2015-02-23 18:07:51 +0000
+++ lib/lp/code/model/gitrepository.py 2015-02-23 18:07:51 +0000
@@ -47,6 +47,7 @@
InvalidGitRepositoryException,
InvalidNamespace,
)
+from lp.code.interfaces.gitcollection import IAllGitRepositories
from lp.code.interfaces.gitlookup import IGitLookup
from lp.code.interfaces.gitnamespace import (
get_git_namespace,
@@ -303,9 +304,8 @@
elif user.id in self._known_viewers:
return True
else:
- # XXX cjwatson 2015-02-06: Fill this in once IGitCollection is
- # in place.
- return False
+ return not getUtility(IAllGitRepositories).withIds(
+ self.id).visibleByUser(user).is_empty()
def getAllowedInformationTypes(self, user):
"""See `IGitRepository`."""
=== modified file 'lib/lp/code/model/hasgitrepositories.py'
--- lib/lp/code/model/hasgitrepositories.py 2015-02-10 01:03:48 +0000
+++ lib/lp/code/model/hasgitrepositories.py 2015-02-23 18:07:51 +0000
@@ -8,6 +8,7 @@
from zope.component import getUtility
+from lp.code.interfaces.gitcollection import IGitCollection
from lp.code.interfaces.gitrepository import IGitRepositorySet
@@ -23,6 +24,5 @@
def getGitRepositories(self, visible_by_user=None, eager_load=False):
"""See `IHasGitRepositories`."""
- # XXX cjwatson 2015-02-06: Fill this in once IGitCollection is in
- # place.
- raise NotImplementedError
+ collection = IGitCollection(self).visibleByUser(visible_by_user)
+ return collection.getRepositories(eager_load=eager_load)
=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py 2015-02-23 18:07:51 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py 2015-02-23 18:07:51 +0000
@@ -33,11 +33,21 @@
IGitRepository,
IGitRepositorySet,
)
-from lp.registry.enums import BranchSharingPolicy
+from lp.registry.enums import (
+ BranchSharingPolicy,
+ PersonVisibility,
+ TeamMembershipPolicy,
+ )
+from lp.registry.interfaces.accesspolicy import (
+ IAccessArtifactSource,
+ IAccessPolicyArtifactSource,
+ IAccessPolicySource,
+ )
from lp.registry.interfaces.persondistributionsourcepackage import (
IPersonDistributionSourcePackageFactory,
)
from lp.registry.interfaces.personproduct import IPersonProductFactory
+from lp.registry.tests.test_accesspolicy import get_policies_for_artifact
from lp.services.database.constants import UTC_NOW
from lp.services.webapp.authorization import check_permission
from lp.testing import (
@@ -129,6 +139,15 @@
project.setDefaultGitRepository(repository)
self.assertGitIdentity(repository, project.name)
+ def test_git_identity_private_default_for_project(self):
+ # Private repositories also have a short lp: URL.
+ project = self.factory.makeProduct()
+ repository = self.factory.makeGitRepository(
+ target=project, information_type=InformationType.USERDATA)
+ with admin_logged_in():
+ project.setDefaultGitRepository(repository)
+ self.assertGitIdentity(repository, project.name)
+
def test_git_identity_default_for_package(self):
# If a repository is the default for a package, then its Git
# identity uses the path to that package.
@@ -293,6 +312,57 @@
self.assertEqual(namespace, repository.namespace)
+class TestGitRepositoryPrivacy(TestCaseWithFactory):
+ """Tests for Git repository privacy."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ # Use an admin user as we aren't checking edit permissions here.
+ super(TestGitRepositoryPrivacy, self).setUp("admin@xxxxxxxxxxxxx")
+
+ def test_personal_repositories_for_private_teams_are_private(self):
+ team = self.factory.makeTeam(
+ membership_policy=TeamMembershipPolicy.MODERATED,
+ visibility=PersonVisibility.PRIVATE)
+ repository = self.factory.makeGitRepository(owner=team, target=team)
+ self.assertTrue(repository.private)
+ self.assertEqual(
+ InformationType.PROPRIETARY, repository.information_type)
+
+ def test__reconcileAccess_for_project_repository(self):
+ # _reconcileAccess uses a project policy for a project repository.
+ repository = self.factory.makeGitRepository(
+ information_type=InformationType.USERDATA)
+ [artifact] = getUtility(IAccessArtifactSource).ensure([repository])
+ getUtility(IAccessPolicyArtifactSource).deleteByArtifact([artifact])
+ removeSecurityProxy(repository)._reconcileAccess()
+ self.assertContentEqual(
+ getUtility(IAccessPolicySource).find(
+ [(repository.target, InformationType.USERDATA)]),
+ get_policies_for_artifact(repository))
+
+ def test__reconcileAccess_for_package_repository(self):
+ # Git repository privacy isn't yet supported for distributions, so
+ # no AccessPolicyArtifact is created for a package repository.
+ repository = self.factory.makeGitRepository(
+ target=self.factory.makeDistributionSourcePackage(),
+ information_type=InformationType.USERDATA)
+ removeSecurityProxy(repository)._reconcileAccess()
+ self.assertEqual([], get_policies_for_artifact(repository))
+
+ def test__reconcileAccess_for_personal_repository(self):
+ # _reconcileAccess uses a person policy for a personal repository.
+ team_owner = self.factory.makeTeam()
+ repository = self.factory.makeGitRepository(
+ owner=team_owner, target=team_owner,
+ information_type=InformationType.USERDATA)
+ removeSecurityProxy(repository)._reconcileAccess()
+ self.assertContentEqual(
+ getUtility(IAccessPolicySource).findByTeam([team_owner]),
+ get_policies_for_artifact(repository))
+
+
class TestGitRepositoryGetAllowedInformationTypes(TestCaseWithFactory):
"""Test `IGitRepository.getAllowedInformationTypes`."""
@@ -345,6 +415,17 @@
self.assertFalse(
check_permission("launchpad.Moderate", repository))
+ def test_methods_smoketest(self):
+ # Users with launchpad.Moderate can call transitionToInformationType.
+ project = self.factory.makeProduct()
+ repository = self.factory.makeGitRepository(target=project)
+ with person_logged_in(project.owner):
+ project.setBranchSharingPolicy(BranchSharingPolicy.PUBLIC)
+ repository.transitionToInformationType(
+ InformationType.PRIVATESECURITY, project.owner)
+ self.assertEqual(
+ InformationType.PRIVATESECURITY, repository.information_type)
+
def test_attribute_smoketest(self):
# Users with launchpad.Moderate can set attributes.
project = self.factory.makeProduct()
@@ -477,6 +558,47 @@
repository.setTarget(target=owner, user=owner)
self.assertEqual(owner, repository.target)
+ def test_private_personal_forbidden_for_public_teams(self):
+ # Only private teams can have private personal repositories.
+ owner = self.factory.makeTeam()
+ repository = self.factory.makeGitRepository(
+ owner=owner, information_type=InformationType.USERDATA)
+ with admin_logged_in():
+ self.assertRaises(
+ GitTargetError, repository.setTarget, target=owner, user=owner)
+
+ def test_private_personal_allowed_for_private_teams(self):
+ # Only private teams can have private personal repositories.
+ owner = self.factory.makeTeam(visibility=PersonVisibility.PRIVATE)
+ with person_logged_in(owner):
+ repository = self.factory.makeGitRepository(
+ owner=owner, information_type=InformationType.USERDATA)
+ repository.setTarget(target=owner, user=owner)
+ self.assertEqual(owner, repository.target)
+
+ def test_reconciles_access(self):
+ # setTarget calls _reconcileAccess to make the sharing schema
+ # match the new target.
+ repository = self.factory.makeGitRepository(
+ information_type=InformationType.USERDATA)
+ new_project = self.factory.makeProduct()
+ with admin_logged_in():
+ repository.setTarget(target=new_project, user=repository.owner)
+ self.assertEqual(
+ new_project, get_policies_for_artifact(repository)[0].pillar)
+
+ def test_reconciles_access_personal(self):
+ # setTarget calls _reconcileAccess to make the sharing schema
+ # correct for a private personal repository.
+ owner = self.factory.makeTeam(visibility=PersonVisibility.PRIVATE)
+ with person_logged_in(owner):
+ repository = self.factory.makeGitRepository(
+ owner=owner, target=owner,
+ information_type=InformationType.USERDATA)
+ repository.setTarget(target=owner, user=owner)
+ self.assertEqual(
+ owner, get_policies_for_artifact(repository)[0].person)
+
def test_public_to_proprietary_only_project(self):
# A repository cannot be moved to a target where the sharing policy
# does not allow it.
@@ -513,3 +635,15 @@
person = self.factory.makePerson()
self.assertIsNone(
getUtility(IGitRepositorySet).getByPath(person, "nonexistent"))
+
+ def test_getByPath_inaccessible(self):
+ # If the given user cannot view the matched repository, then
+ # getByPath returns None.
+ owner = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(
+ owner=owner, information_type=InformationType.USERDATA)
+ grs = getUtility(IGitRepositorySet)
+ with person_logged_in(owner):
+ path = repository.shortened_path
+ self.assertEqual(repository, grs.getByPath(owner, path))
+ self.assertIsNone(grs.getByPath(self.factory.makePerson(), path))
=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py 2015-02-16 13:01:34 +0000
+++ lib/lp/registry/services/sharingservice.py 2015-02-23 18:07:51 +0000
@@ -36,6 +36,7 @@
from lp.bugs.interfaces.bugtask import IBugTaskSet
from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
from lp.code.interfaces.branchcollection import IAllBranches
+from lp.code.interfaces.gitcollection import IAllGitRepositories
from lp.registry.enums import (
BranchSharingPolicy,
BugSharingPolicy,
@@ -227,7 +228,11 @@
branches = list(wanted_branches.getBranches())
# Load the Git repositories.
gitrepositories = []
- # XXX cjwatson 2015-02-16: Fill in once IGitCollection is in place.
+ if gitrepository_ids:
+ all_gitrepositories = getUtility(IAllGitRepositories)
+ wanted_gitrepositories = all_gitrepositories.visibleByUser(
+ user).withIds(*gitrepository_ids)
+ gitrepositories = list(wanted_gitrepositories.getRepositories())
specifications = []
if specification_ids:
specifications = load(Specification, specification_ids)
@@ -360,7 +365,12 @@
# Load the Git repositories.
visible_gitrepositories = []
- # XXX cjwatson 2015-02-16: Fill in once IGitCollection is in place.
+ if gitrepositories_by_id:
+ all_gitrepositories = getUtility(IAllGitRepositories)
+ wanted_gitrepositories = all_gitrepositories.visibleByUser(
+ person).withIds(*gitrepositories_by_id.keys())
+ visible_gitrepositories = list(
+ wanted_gitrepositories.getRepositories())
# Load the specifications.
visible_specs = []
@@ -411,7 +421,17 @@
# Load the Git repositories.
invisible_gitrepositories = []
- # XXX cjwatson 2015-02-16: Fill in once IGitCollection is in place.
+ if gitrepositories_by_id:
+ all_gitrepositories = getUtility(IAllGitRepositories)
+ visible_gitrepository_ids = all_gitrepositories.visibleByUser(
+ person).withIds(
+ *gitrepositories_by_id.keys()).getRepositoryIds()
+ invisible_gitrepository_ids = (
+ set(gitrepositories_by_id.keys()).difference(
+ visible_gitrepository_ids))
+ invisible_gitrepositories = [
+ gitrepositories_by_id[gitrepository_id]
+ for gitrepository_id in invisible_gitrepository_ids]
return invisible_bugs, invisible_branches, invisible_gitrepositories
=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py 2015-02-16 13:01:34 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py 2015-02-23 18:07:51 +0000
@@ -23,6 +23,7 @@
CodeReviewNotificationLevel,
)
from lp.code.interfaces.branch import IBranch
+from lp.code.interfaces.gitrepository import IGitRepository
from lp.registry.enums import (
BranchSharingPolicy,
BugSharingPolicy,
@@ -928,12 +929,14 @@
[InformationType.USERDATA])
def _assert_revokeAccessGrants(self, pillar, bugs, branches,
- specifications):
+ gitrepositories, specifications):
artifacts = []
if bugs:
artifacts.extend(bugs)
if branches:
artifacts.extend(branches)
+ if gitrepositories:
+ artifacts.extend(gitrepositories)
if specifications:
artifacts.extend(specifications)
policy = self.factory.makeAccessPolicy(pillar=pillar,
@@ -961,6 +964,8 @@
branch.subscribe(person,
BranchSubscriptionNotificationLevel.NOEMAIL, None,
CodeReviewNotificationLevel.NOEMAIL, pillar.owner)
+ # XXX cjwatson 2015-02-05: subscribe to Git repositories when
+ # implemented
for spec in specifications or []:
spec.subscribe(person)
@@ -973,7 +978,7 @@
self.service.revokeAccessGrants(
pillar, grantee, pillar.owner, bugs=bugs, branches=branches,
- specifications=specifications)
+ gitrepositories=gitrepositories, specifications=specifications)
with block_on_job(self):
transaction.commit()
@@ -987,18 +992,22 @@
self.assertNotIn(grantee, bug.getDirectSubscribers())
for branch in branches or []:
self.assertNotIn(grantee, branch.subscribers)
+ # XXX cjwatson 2015-02-05: check revocation of subscription to Git
+ # repositories when implemented
for spec in specifications or []:
self.assertNotIn(grantee, spec.subscribers)
- # Someone else still has access to the bugs and branches.
+ # Someone else still has access to the artifacts.
grants = accessartifact_grant_source.findByArtifact(
access_artifacts, [someone])
self.assertEqual(1, grants.count())
- # Someone else still has subscriptions to the bugs and branches.
+ # Someone else still has subscriptions to the artifacts.
for bug in bugs or []:
self.assertIn(someone, bug.getDirectSubscribers())
for branch in branches or []:
self.assertIn(someone, branch.subscribers)
+ # XXX cjwatson 2015-02-05: check subscription to Git repositories
+ # when implemented
for spec in specifications or []:
self.assertIn(someone, spec.subscribers)
@@ -1010,7 +1019,7 @@
bug = self.factory.makeBug(
target=distro, owner=owner,
information_type=InformationType.USERDATA)
- self._assert_revokeAccessGrants(distro, [bug], None, None)
+ self._assert_revokeAccessGrants(distro, [bug], None, None, None)
def test_revokeAccessGrantsBranches(self):
owner = self.factory.makePerson()
@@ -1019,7 +1028,17 @@
branch = self.factory.makeBranch(
product=product, owner=owner,
information_type=InformationType.USERDATA)
- self._assert_revokeAccessGrants(product, None, [branch], None)
+ self._assert_revokeAccessGrants(product, None, [branch], None, None)
+
+ def test_revokeAccessGrantsGitRepositories(self):
+ owner = self.factory.makePerson()
+ product = self.factory.makeProduct(owner=owner)
+ login_person(owner)
+ gitrepository = self.factory.makeGitRepository(
+ target=product, owner=owner,
+ information_type=InformationType.USERDATA)
+ self._assert_revokeAccessGrants(
+ product, None, None, [gitrepository], None)
def test_revokeAccessGrantsSpecifications(self):
owner = self.factory.makePerson()
@@ -1030,15 +1049,18 @@
specification = self.factory.makeSpecification(
product=product, owner=owner,
information_type=InformationType.EMBARGOED)
- self._assert_revokeAccessGrants(product, None, None, [specification])
+ self._assert_revokeAccessGrants(
+ product, None, None, None, [specification])
def _assert_revokeTeamAccessGrants(self, pillar, bugs, branches,
- specifications):
+ gitrepositories, specifications):
artifacts = []
if bugs:
artifacts.extend(bugs)
if branches:
artifacts.extend(branches)
+ if gitrepositories:
+ artifacts.extend(gitrepositories)
if specifications:
artifacts.extend(specifications)
policy = self.factory.makeAccessPolicy(pillar=pillar,
@@ -1065,6 +1087,8 @@
branch.subscribe(
person, BranchSubscriptionNotificationLevel.NOEMAIL,
None, CodeReviewNotificationLevel.NOEMAIL, pillar.owner)
+ # XXX cjwatson 2015-02-05: subscribe to Git repositories when
+ # implemented
# Subscribing somebody to a specification does not yet imply
# granting access to this person.
if specifications:
@@ -1075,12 +1099,16 @@
# Check that grantees have expected access grants and subscriptions.
for person in [team_grantee, person_grantee]:
- visible_bugs, visible_branches, _, visible_specs = (
+ (visible_bugs, visible_branches, visible_gitrepositories,
+ visible_specs) = (
self.service.getVisibleArtifacts(
person, bugs=bugs, branches=branches,
+ gitrepositories=gitrepositories,
specifications=specifications))
self.assertContentEqual(bugs or [], visible_bugs)
self.assertContentEqual(branches or [], visible_branches)
+ # XXX cjwatson 2015-02-05: check Git repositories when
+ # subscription is implemented
self.assertContentEqual(specifications or [], visible_specs)
for person in [team_grantee, person_grantee]:
for bug in bugs or []:
@@ -1088,7 +1116,7 @@
self.service.revokeAccessGrants(
pillar, team_grantee, pillar.owner, bugs=bugs, branches=branches,
- specifications=specifications)
+ gitrepositories=gitrepositories, specifications=specifications)
with block_on_job(self):
transaction.commit()
@@ -1103,11 +1131,14 @@
for person in [team_grantee, person_grantee]:
for bug in bugs or []:
self.assertNotIn(person, bug.getDirectSubscribers())
- visible_bugs, visible_branches, _, visible_specs = (
+ (visible_bugs, visible_branches, visible_gitrepositories,
+ visible_specs) = (
self.service.getVisibleArtifacts(
- person, bugs=bugs, branches=branches))
+ person, bugs=bugs, branches=branches,
+ gitrepositories=gitrepositories))
self.assertContentEqual([], visible_bugs)
self.assertContentEqual([], visible_branches)
+ self.assertContentEqual([], visible_gitrepositories)
self.assertContentEqual([], visible_specs)
def test_revokeTeamAccessGrantsBugs(self):
@@ -1118,7 +1149,7 @@
bug = self.factory.makeBug(
target=distro, owner=owner,
information_type=InformationType.USERDATA)
- self._assert_revokeTeamAccessGrants(distro, [bug], None, None)
+ self._assert_revokeTeamAccessGrants(distro, [bug], None, None, None)
def test_revokeTeamAccessGrantsBranches(self):
# Users with launchpad.Edit can delete all access for a grantee.
@@ -1127,7 +1158,20 @@
login_person(owner)
branch = self.factory.makeBranch(
owner=owner, information_type=InformationType.USERDATA)
- self._assert_revokeTeamAccessGrants(product, None, [branch], None)
+ self._assert_revokeTeamAccessGrants(
+ product, None, [branch], None, None)
+
+ # XXX cjwatson 2015-02-05: Enable this once GitRepositorySubscription is
+ # implemented.
+ def disabled_test_revokeTeamAccessGrantsGitRepositories(self):
+ # Users with launchpad.Edit can delete all access for a grantee.
+ owner = self.factory.makePerson()
+ product = self.factory.makeProduct(owner=owner)
+ login_person(owner)
+ gitrepository = self.factory.makeGitRepository(
+ owner=owner, information_type=InformationType.USERDATA)
+ self._assert_revokeTeamAccessGrants(
+ product, None, None, [gitrepository], None)
def test_revokeTeamAccessGrantsSpecifications(self):
# Users with launchpad.Edit can delete all access for a grantee.
@@ -1140,7 +1184,7 @@
product=product, owner=owner,
information_type=InformationType.EMBARGOED)
self._assert_revokeTeamAccessGrants(
- product, None, None, [specification])
+ product, None, None, None, [specification])
def _assert_revokeAccessGrantsUnauthorized(self):
# revokeAccessGrants raises an Unauthorized exception if the user
@@ -1163,9 +1207,9 @@
login_person(self.factory.makePerson())
self._assert_revokeAccessGrantsUnauthorized()
- def test_revokeAccessGrants_without_bugs_or_branches(self):
+ def test_revokeAccessGrants_without_artifacts(self):
# The revokeAccessGrants method raises a ValueError if called without
- # specifying either bugs or branches.
+ # specifying any artifacts.
owner = self.factory.makePerson()
product = self.factory.makeProduct(owner=owner)
grantee = self.factory.makePerson()
@@ -1174,24 +1218,27 @@
ValueError, self.service.revokeAccessGrants,
product, grantee, product.owner)
- def _assert_ensureAccessGrants(self, user, bugs, branches, specifications,
- grantee=None):
+ def _assert_ensureAccessGrants(self, user, bugs, branches, gitrepositories,
+ specifications, grantee=None):
# Creating access grants works as expected.
if not grantee:
grantee = self.factory.makePerson()
self.service.ensureAccessGrants(
[grantee], user, bugs=bugs, branches=branches,
- specifications=specifications)
+ gitrepositories=gitrepositories, specifications=specifications)
# Check that grantee has expected access grants.
shared_bugs = []
shared_branches = []
+ shared_gitrepositories = []
shared_specifications = []
all_pillars = []
for bug in bugs or []:
all_pillars.extend(bug.affected_pillars)
for branch in branches or []:
all_pillars.append(branch.target.context)
+ for gitrepository in gitrepositories or []:
+ all_pillars.append(gitrepository.target)
for specification in specifications or []:
all_pillars.append(specification.target)
policies = getUtility(IAccessPolicySource).findByPillar(all_pillars)
@@ -1203,10 +1250,13 @@
shared_bugs.append(a.concrete_artifact)
elif IBranch.providedBy(a.concrete_artifact):
shared_branches.append(a.concrete_artifact)
+ elif IGitRepository.providedBy(a.concrete_artifact):
+ shared_gitrepositories.append(a.concrete_artifact)
elif ISpecification.providedBy(a.concrete_artifact):
shared_specifications.append(a.concrete_artifact)
self.assertContentEqual(bugs or [], shared_bugs)
self.assertContentEqual(branches or [], shared_branches)
+ self.assertContentEqual(gitrepositories or [], shared_gitrepositories)
self.assertContentEqual(specifications or [], shared_specifications)
def test_ensureAccessGrantsBugs(self):
@@ -1217,7 +1267,7 @@
bug = self.factory.makeBug(
target=distro, owner=owner,
information_type=InformationType.USERDATA)
- self._assert_ensureAccessGrants(owner, [bug], None, None)
+ self._assert_ensureAccessGrants(owner, [bug], None, None, None)
def test_ensureAccessGrantsBranches(self):
# Access grants can be created for branches.
@@ -1227,7 +1277,18 @@
branch = self.factory.makeBranch(
product=product, owner=owner,
information_type=InformationType.USERDATA)
- self._assert_ensureAccessGrants(owner, None, [branch], None)
+ self._assert_ensureAccessGrants(owner, None, [branch], None, None)
+
+ def test_ensureAccessGrantsGitRepositories(self):
+ # Access grants can be created for Git repositories.
+ owner = self.factory.makePerson()
+ product = self.factory.makeProduct(owner=owner)
+ login_person(owner)
+ gitrepository = self.factory.makeGitRepository(
+ target=product, owner=owner,
+ information_type=InformationType.USERDATA)
+ self._assert_ensureAccessGrants(
+ owner, None, None, [gitrepository], None)
def test_ensureAccessGrantsSpecifications(self):
# Access grants can be created for branches.
@@ -1244,7 +1305,8 @@
with person_logged_in(owner):
specification.transitionToInformationType(
InformationType.PROPRIETARY, owner)
- self._assert_ensureAccessGrants(owner, None, None, [specification])
+ self._assert_ensureAccessGrants(
+ owner, None, None, None, [specification])
def test_ensureAccessGrantsExisting(self):
# Any existing access grants are retained and new ones created.
@@ -1263,7 +1325,7 @@
# Test with a new bug as well as the one for which access is already
# granted.
self._assert_ensureAccessGrants(
- owner, [bug, bug2], None, None, grantee)
+ owner, [bug, bug2], None, None, None, grantee)
def _assert_ensureAccessGrantsUnauthorized(self, user):
# ensureAccessGrants raises an Unauthorized exception if the user
@@ -1331,7 +1393,8 @@
self._assert_updatePillarSharingPoliciesUnauthorized(anyone)
def create_shared_artifacts(self, product, grantee, user):
- # Create some shared bugs and branches.
+ # Create some shared bugs, branches, Git repositories, and
+ # specifications.
bugs = []
bug_tasks = []
for x in range(0, 10):
@@ -1346,6 +1409,12 @@
product=product, owner=product.owner,
information_type=InformationType.USERDATA)
branches.append(branch)
+ gitrepositories = []
+ for x in range(0, 10):
+ gitrepository = self.factory.makeGitRepository(
+ target=product, owner=product.owner,
+ information_type=InformationType.USERDATA)
+ gitrepositories.append(gitrepository)
specs = []
for x in range(0, 10):
spec = self.factory.makeSpecification(
@@ -1371,9 +1440,11 @@
grant_access(bug, i == 9)
for i, branch in enumerate(branches):
grant_access(branch, i == 9)
+ for i, gitrepository in enumerate(gitrepositories):
+ grant_access(gitrepository, i == 9)
getUtility(IService, 'sharing').ensureAccessGrants(
[grantee], product.owner, specifications=specs[:9])
- return bug_tasks, branches, specs
+ return bug_tasks, branches, gitrepositories, specs
def test_getSharedArtifacts(self):
# Test the getSharedArtifacts method.
@@ -1384,14 +1455,16 @@
login_person(owner)
grantee = self.factory.makePerson()
user = self.factory.makePerson()
- bug_tasks, branches, specs = self.create_shared_artifacts(
- product, grantee, user)
+ bug_tasks, branches, gitrepositories, specs = (
+ self.create_shared_artifacts(product, grantee, user))
# Check the results.
- shared_bugtasks, shared_branches, _, shared_specs = (
+ (shared_bugtasks, shared_branches, shared_gitrepositories,
+ shared_specs) = (
self.service.getSharedArtifacts(product, grantee, user))
self.assertContentEqual(bug_tasks[:9], shared_bugtasks)
self.assertContentEqual(branches[:9], shared_branches)
+ self.assertContentEqual(gitrepositories[:9], shared_gitrepositories)
self.assertContentEqual(specs[:9], shared_specs)
def _assert_getSharedProjects(self, product, who=None):
@@ -1531,7 +1604,7 @@
login_person(owner)
grantee = self.factory.makePerson()
user = self.factory.makePerson()
- bug_tasks, ignored, ignored = self.create_shared_artifacts(
+ bug_tasks, _, _, _ = self.create_shared_artifacts(
product, grantee, user)
# Check the results.
@@ -1547,7 +1620,7 @@
login_person(owner)
grantee = self.factory.makePerson()
user = self.factory.makePerson()
- ignored, branches, ignored = self.create_shared_artifacts(
+ _, branches, _, _ = self.create_shared_artifacts(
product, grantee, user)
# Check the results.
@@ -1555,6 +1628,23 @@
product, grantee, user)
self.assertContentEqual(branches[:9], shared_branches)
+ def test_getSharedGitRepositories(self):
+ # Test the getSharedGitRepositories method.
+ owner = self.factory.makePerson()
+ product = self.factory.makeProduct(
+ owner=owner, specification_sharing_policy=(
+ SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY))
+ login_person(owner)
+ grantee = self.factory.makePerson()
+ user = self.factory.makePerson()
+ _, _, gitrepositories, _ = self.create_shared_artifacts(
+ product, grantee, user)
+
+ # Check the results.
+ shared_gitrepositories = self.service.getSharedGitRepositories(
+ product, grantee, user)
+ self.assertContentEqual(gitrepositories[:9], shared_gitrepositories)
+
def test_getSharedSpecifications(self):
# Test the getSharedSpecifications method.
owner = self.factory.makePerson()
@@ -1564,7 +1654,7 @@
login_person(owner)
grantee = self.factory.makePerson()
user = self.factory.makePerson()
- ignored, ignored, specifications = self.create_shared_artifacts(
+ _, _, _, specifications = self.create_shared_artifacts(
product, grantee, user)
# Check the results.
@@ -1592,6 +1682,16 @@
login_person(owner)
self._assert_getPeopleWithoutAccess(product, branch)
+ def test_getPeopleWithAccessGitRepositories(self):
+ # Test the getPeopleWithoutAccess method with Git repositories.
+ owner = self.factory.makePerson()
+ product = self.factory.makeProduct(owner=owner)
+ gitrepository = self.factory.makeGitRepository(
+ target=product, owner=owner,
+ information_type=InformationType.USERDATA)
+ login_person(owner)
+ self._assert_getPeopleWithoutAccess(product, gitrepository)
+
def _assert_getPeopleWithoutAccess(self, product, artifact):
access_artifact = self.factory.makeAccessArtifact(concrete=artifact)
# Make some people to check. people[:5] will not have access.
@@ -1647,6 +1747,12 @@
product=product, owner=owner,
information_type=InformationType.USERDATA)
branches.append(branch)
+ gitrepositories = []
+ for x in range(0, 10):
+ gitrepository = self.factory.makeGitRepository(
+ target=product, owner=owner,
+ information_type=InformationType.USERDATA)
+ gitrepositories.append(gitrepository)
specifications = []
for x in range(0, 10):
@@ -1662,46 +1768,58 @@
artifact=access_artifact, grantee=grantee, grantor=owner)
return access_artifact
- # Grant access to some of the bugs and branches.
+ # Grant access to some of the artifacts.
for bug in bugs[:5]:
grant_access(bug)
for branch in branches[:5]:
grant_access(branch)
+ for gitrepository in gitrepositories[:5]:
+ grant_access(gitrepository)
for spec in specifications[:5]:
grant_access(spec)
- return grantee, owner, branches, bugs, specifications
+ return grantee, owner, bugs, branches, gitrepositories, specifications
def test_getVisibleArtifacts(self):
# Test the getVisibleArtifacts method.
- grantee, ignore, branches, bugs, specs = self._make_Artifacts()
+ grantee, ignore, bugs, branches, gitrepositories, specs = (
+ self._make_Artifacts())
# Check the results.
- shared_bugs, shared_branches, _, shared_specs = (
+ shared_bugs, shared_branches, shared_gitrepositories, shared_specs = (
self.service.getVisibleArtifacts(
- grantee, bugs=bugs, branches=branches, specifications=specs))
+ grantee, bugs=bugs, branches=branches,
+ gitrepositories=gitrepositories, specifications=specs))
self.assertContentEqual(bugs[:5], shared_bugs)
self.assertContentEqual(branches[:5], shared_branches)
+ self.assertContentEqual(gitrepositories[:5], shared_gitrepositories)
self.assertContentEqual(specs[:5], shared_specs)
def test_getVisibleArtifacts_grant_on_pillar(self):
# getVisibleArtifacts() returns private specifications if
# user has a policy grant for the pillar of the specification.
- ignore, owner, branches, bugs, specs = self._make_Artifacts()
- shared_bugs, shared_branches, _, shared_specs = (
+ _, owner, bugs, branches, gitrepositories, specs = (
+ self._make_Artifacts())
+ shared_bugs, shared_branches, shared_gitrepositories, shared_specs = (
self.service.getVisibleArtifacts(
- owner, bugs=bugs, branches=branches, specifications=specs))
+ owner, bugs=bugs, branches=branches,
+ gitrepositories=gitrepositories, specifications=specs))
self.assertContentEqual(bugs, shared_bugs)
self.assertContentEqual(branches, shared_branches)
+ self.assertContentEqual(gitrepositories, shared_gitrepositories)
self.assertContentEqual(specs, shared_specs)
def test_getInvisibleArtifacts(self):
# Test the getInvisibleArtifacts method.
- grantee, ignore, branches, bugs, specs = self._make_Artifacts()
+ grantee, ignore, bugs, branches, gitrepositories, specs = (
+ self._make_Artifacts())
# Check the results.
- not_shared_bugs, not_shared_branches, _ = (
+ not_shared_bugs, not_shared_branches, not_shared_gitrepositories = (
self.service.getInvisibleArtifacts(
- grantee, bugs=bugs, branches=branches))
+ grantee, bugs=bugs, branches=branches,
+ gitrepositories=gitrepositories))
self.assertContentEqual(bugs[5:], not_shared_bugs)
self.assertContentEqual(branches[5:], not_shared_branches)
+ self.assertContentEqual(
+ gitrepositories[5:], not_shared_gitrepositories)
def _assert_getVisibleArtifacts_bug_change(self, change_callback):
# Test the getVisibleArtifacts method excludes bugs after a change of
@@ -1723,7 +1841,7 @@
information_type=InformationType.USERDATA)
bugs.append(bug)
- shared_bugs, shared_branches, _, shared_specs = (
+ shared_bugs, shared_branches, shared_gitrepositories, shared_specs = (
self.service.getVisibleArtifacts(grantee, bugs=bugs))
self.assertContentEqual(bugs, shared_bugs)
@@ -1731,7 +1849,7 @@
for x in range(0, 5):
change_callback(bugs[x], owner)
# Check the results.
- shared_bugs, shared_branches, _, shared_specs = (
+ shared_bugs, shared_branches, shared_gitrepositories, shared_specs = (
self.service.getVisibleArtifacts(grantee, bugs=bugs))
self.assertContentEqual(bugs[5:], shared_bugs)
=== modified file 'lib/lp/registry/tests/test_accesspolicy.py'
--- lib/lp/registry/tests/test_accesspolicy.py 2013-06-20 05:50:00 +0000
+++ lib/lp/registry/tests/test_accesspolicy.py 2015-02-23 18:07:51 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -280,6 +280,13 @@
return self.factory.makeBranch()
+class TestAccessArtifactGitRepository(BaseAccessArtifactTests,
+ TestCaseWithFactory):
+
+ def getConcreteArtifact(self):
+ return self.factory.makeGitRepository()
+
+
class TestAccessArtifactBug(BaseAccessArtifactTests,
TestCaseWithFactory):
=== modified file 'lib/lp/registry/tests/test_sharingjob.py'
--- lib/lp/registry/tests/test_sharingjob.py 2014-06-19 06:38:53 +0000
+++ lib/lp/registry/tests/test_sharingjob.py 2015-02-23 18:07:51 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012 Canonical Ltd. This software is licensed under the
+# Copyright 2012-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for SharingJobs."""
@@ -118,6 +118,16 @@
'for branch_ids=[%d], requestor=%s>'
% (branch.id, requestor.name), repr(job))
+ def test_repr_gitrepositories(self):
+ requestor = self.factory.makePerson()
+ gitrepository = self.factory.makeGitRepository()
+ job = getUtility(IRemoveArtifactSubscriptionsJobSource).create(
+ requestor, artifacts=[gitrepository])
+ self.assertEqual(
+ '<REMOVE_ARTIFACT_SUBSCRIPTIONS job reconciling subscriptions '
+ 'for gitrepository_ids=[%d], requestor=%s>'
+ % (gitrepository.id, requestor.name), repr(job))
+
def test_repr_specifications(self):
requestor = self.factory.makePerson()
specification = self.factory.makeSpecification()
@@ -303,6 +313,9 @@
branch = self.factory.makeBranch(
owner=owner, product=product,
information_type=InformationType.USERDATA)
+ gitrepository = self.factory.makeGitRepository(
+ owner=owner, target=product,
+ information_type=InformationType.USERDATA)
specification = self.factory.makeSpecification(
owner=owner, product=product,
information_type=InformationType.PROPRIETARY)
@@ -317,6 +330,8 @@
branch.subscribe(artifact_indirect_grantee,
BranchSubscriptionNotificationLevel.NOEMAIL, None,
CodeReviewNotificationLevel.NOEMAIL, owner)
+ # XXX cjwatson 2015-02-05: Fill this in once we have
+ # GitRepositorySubscription.
# Subscribing somebody to a specification does not automatically
# create an artifact grant.
spec_artifact = self.factory.makeAccessArtifact(specification)
@@ -325,10 +340,10 @@
with person_logged_in(product.owner):
specification.subscribe(artifact_indirect_grantee, owner)
- # pick one of the concrete artifacts (bug, branch or spec)
- # and subscribe the teams and persons.
+ # pick one of the concrete artifacts (bug, branch, Git repository,
+ # or spec) and subscribe the teams and persons.
concrete_artifact, get_pillars, get_subscribers = configure_test(
- bug, branch, specification, policy_team_grantee,
+ bug, branch, gitrepository, specification, policy_team_grantee,
policy_indirect_grantee, artifact_team_grantee, owner)
# Subscribing policy_team_grantee has created an artifact grant so we
@@ -361,6 +376,8 @@
self.assertIn(artifact_team_grantee, subscribers)
self.assertIn(artifact_indirect_grantee, bug.getDirectSubscribers())
self.assertIn(artifact_indirect_grantee, branch.subscribers)
+ # XXX cjwatson 2015-02-05: Fill this in once we have
+ # GitRepositorySubscription.
self.assertIn(artifact_indirect_grantee,
removeSecurityProxy(specification).subscribers)
@@ -373,9 +390,9 @@
return removeSecurityProxy(
concrete_artifact).getDirectSubscribers()
- def configure_test(bug, branch, specification, policy_team_grantee,
- policy_indirect_grantee, artifact_team_grantee,
- owner):
+ def configure_test(bug, branch, gitrepository, specification,
+ policy_team_grantee, policy_indirect_grantee,
+ artifact_team_grantee, owner):
concrete_artifact = bug
bug.subscribe(policy_team_grantee, owner)
bug.subscribe(policy_indirect_grantee, owner)
@@ -393,9 +410,9 @@
def get_subscribers(concrete_artifact):
return concrete_artifact.subscribers
- def configure_test(bug, branch, specification, policy_team_grantee,
- policy_indirect_grantee, artifact_team_grantee,
- owner):
+ def configure_test(bug, branch, gitrepository, specification,
+ policy_team_grantee, policy_indirect_grantee,
+ artifact_team_grantee, owner):
concrete_artifact = branch
branch.subscribe(
policy_team_grantee,
@@ -414,6 +431,27 @@
self._assert_artifact_change_unsubscribes(
change_callback, configure_test)
+ def _assert_gitrepository_change_unsubscribes(self, change_callback):
+
+ def get_pillars(concrete_artifact):
+ return [concrete_artifact.product]
+
+ def get_subscribers(concrete_artifact):
+ return concrete_artifact.subscribers
+
+ def configure_test(bug, branch, gitrepository, specification,
+ policy_team_grantee, policy_indirect_grantee,
+ artifact_team_grantee, owner):
+ concrete_artifact = gitrepository
+ # XXX cjwatson 2015-02-05: Fill this in once we have
+ # GitRepositorySubscription.
+ return concrete_artifact, get_pillars, get_subscribers
+
+ # XXX cjwatson 2015-02-05: Uncomment once we have
+ # GitRepositorySubscription.
+ #self._assert_artifact_change_unsubscribes(
+ # change_callback, configure_test)
+
def _assert_specification_change_unsubscribes(self, change_callback):
def get_pillars(concrete_artifact):
@@ -422,9 +460,9 @@
def get_subscribers(concrete_artifact):
return concrete_artifact.subscribers
- def configure_test(bug, branch, specification, policy_team_grantee,
- policy_indirect_grantee, artifact_team_grantee,
- owner):
+ def configure_test(bug, branch, gitrepository, specification,
+ policy_team_grantee, policy_indirect_grantee,
+ artifact_team_grantee, owner):
naked_spec = removeSecurityProxy(specification)
naked_spec.subscribe(policy_team_grantee, owner)
naked_spec.subscribe(policy_indirect_grantee, owner)
@@ -444,6 +482,13 @@
self._assert_branch_change_unsubscribes(change_information_type)
+ def test_change_information_type_gitrepository(self):
+ def change_information_type(gitrepository):
+ removeSecurityProxy(gitrepository).information_type = (
+ InformationType.PRIVATESECURITY)
+
+ self._assert_gitrepository_change_unsubscribes(change_information_type)
+
def test_change_information_type_specification(self):
def change_information_type(specification):
removeSecurityProxy(specification).information_type = (
=== modified file 'lib/lp/security.py'
--- lib/lp/security.py 2015-02-23 03:22:37 +0000
+++ lib/lp/security.py 2015-02-23 18:07:51 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Security policies for using content objects."""
@@ -83,6 +83,7 @@
)
from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
from lp.code.interfaces.diff import IPreviewDiff
+from lp.code.interfaces.gitcollection import IGitCollection
from lp.code.interfaces.gitrepository import (
IGitRepository,
user_has_special_git_repository_access,
@@ -1020,6 +1021,12 @@
if not mp.is_empty():
return True
+ # Grant visibility to people who can see Git repositories owned
+ # by the private team.
+ team_repositories = IGitCollection(self.obj)
+ if not team_repositories.visibleByUser(user.person).is_empty():
+ return True
+
# Grant visibility to users in a team that has the private team as
# a member, so that they can see the team properly in member
# listings.
Follow ups
