launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #09912
[Merge] lp:~wgrant/launchpad/branch-type-policy-model into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/branch-type-policy-model into lp:launchpad with lp:~wgrant/launchpad/branch-type-policy-db as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #750871 in Launchpad itself: "only commercial admins can setup branch privacy policies"
https://bugs.launchpad.net/launchpad/+bug/750871
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/branch-type-policy-model/+merge/114805
This branch adds functional branch sharing policies, and basic admin-only UI to control them, using the column added in lp:~wgrant/launchpad/branch-type-policy-db.
Branch sharing policies replace BranchVisibilityPolicy. A project owner will be able to select one to determine the default and allowed information types for their branches, from the following options (names are terrible so feel free to rewrite them, but at least it works for now):
‣ Public: Branches are public unless they contain sensitive security information.
‣ Public, can be proprietary: New branches are public, but can be made proprietary later.
‣ Proprietary, can be public: New branches are proprietary, but can be made public later. Only people who can see the project's proprietary information can create new branches.
‣ Proprietary: Branches are always proprietary. Only people who can see the project's proprietary information can create new branches.
Note that while the text says Proprietary, the implementation still says User Data. This will be changed in a later branch once Proprietary is more widely supported. Branch privacy still falls back to BranchVisibilityPolicy unless Product.branch_sharing_policy is set to a non-null value, which can only currently be achieved by ~registry using Product:+admin, and only when the disclosure.branch_sharing_policy.show_to_admin feature flag is enabled. It'll later be exposed somewhere for project owners, probably on +sharing.
You can see this in action on launchpad.dev:
- Log in as admin@xxxxxxxxxxxxx.
- Add ~name12 to ~registry at <https://launchpad.dev/~registry/+addmember>.
- At <https://launchpad.dev/+feature-rules>, add 'disclosure.branch_sharing_policy.show_to_admin default 0 true'.
- Log in as test@xxxxxxxxxxxxx and head over to <https://code.launchpad.dev/landscape>.
- Set the development focus to '~name12/landscape/feature-x'.
- Observe the privacy portlet stating default branch privacy.
- Change the branch sharing policy at <https://launchpad.dev/landscape/+admin>, and the User Data permissions at <https://launchpad.dev/landscape/+sharing>, and observe the change in the project privacy portlet and the available information types on <https://code.launchpad.dev/~name12/landscape/feature-x/+edit>.
I also added an enum and model/interface definition for bug_sharing_policy, but they're as-yet unused.
--
https://code.launchpad.net/~wgrant/launchpad/branch-type-policy-model/+merge/114805
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/branch-type-policy-model into lp:launchpad.
=== modified file 'lib/lp/code/model/branchnamespace.py'
--- lib/lp/code/model/branchnamespace.py 2012-07-12 04:36:41 +0000
+++ lib/lp/code/model/branchnamespace.py 2012-07-13 08:51:30 +0000
@@ -19,6 +19,7 @@
from zope.interface import implements
from zope.security.proxy import removeSecurityProxy
+from lp.app.interfaces.services import IService
from lp.code.enums import (
BranchLifecycleStatus,
BranchSubscriptionDiffSize,
@@ -45,6 +46,7 @@
from lp.code.interfaces.branchtarget import IBranchTarget
from lp.code.model.branch import Branch
from lp.registry.enums import (
+ BranchSharingPolicy,
InformationType,
PRIVATE_INFORMATION_TYPES,
PUBLIC_INFORMATION_TYPES,
@@ -79,6 +81,29 @@
MAIN_STORE,
)
+POLICY_ALLOWED_TYPES = {
+ BranchSharingPolicy.PUBLIC: PUBLIC_INFORMATION_TYPES,
+ BranchSharingPolicy.PUBLIC_OR_PROPRIETARY:
+ PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES,
+ BranchSharingPolicy.PROPRIETARY_OR_PUBLIC:
+ PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES,
+ BranchSharingPolicy.PROPRIETARY: PRIVATE_INFORMATION_TYPES,
+ }
+
+POLICY_DEFAULT_TYPES = {
+ BranchSharingPolicy.PUBLIC: InformationType.PUBLIC,
+ BranchSharingPolicy.PUBLIC_OR_PROPRIETARY: InformationType.PUBLIC,
+ BranchSharingPolicy.PROPRIETARY_OR_PUBLIC: InformationType.USERDATA,
+ BranchSharingPolicy.PROPRIETARY: InformationType.USERDATA,
+ }
+
+POLICY_REQUIRED_GRANTS = {
+ BranchSharingPolicy.PUBLIC: None,
+ BranchSharingPolicy.PUBLIC_OR_PROPRIETARY: None,
+ BranchSharingPolicy.PROPRIETARY_OR_PUBLIC: InformationType.USERDATA,
+ BranchSharingPolicy.PROPRIETARY: InformationType.USERDATA,
+ }
+
class _BaseNamespace:
"""Common code for branch namespaces."""
@@ -340,6 +365,10 @@
"""See `IBranchNamespace`."""
return IBranchTarget(self.product)
+ @property
+ def _using_branchvisibilitypolicy(self):
+ return self.product.branch_sharing_policy is None
+
def _getRelatedPolicies(self):
"""Return the privacy policies relating to the owner."""
policies = self.product.getBranchVisibilityTeamPolicies()
@@ -355,6 +384,12 @@
def getPrivacySubscriber(self):
"""See `IBranchNamespace`."""
+ # New branch_sharing_policy-based privacy doesn't
+ # require a privacy subscriber, as branches are shared through
+ # AccessPolicyGrants.
+ if not self._using_branchvisibilitypolicy:
+ return None
+
# If there is a rule defined for the owner, then there is no privacy
# subscriber.
rule = self.product.getBranchVisibilityRuleForTeam(self.owner)
@@ -371,6 +406,24 @@
def getAllowedInformationTypes(self):
"""See `IBranchNamespace`."""
+ if not self._using_branchvisibilitypolicy:
+ # The project uses the new simplified branch_sharing_policy
+ # rules, so check them.
+
+ # Some policies require that the owner have full access to
+ # an information type. If it's required and the owner
+ # doesn't hold it, no information types are legal.
+ required_grant = POLICY_REQUIRED_GRANTS[
+ self.product.branch_sharing_policy]
+ if (required_grant is not None
+ and not getUtility(IService, 'sharing').checkPillarAccess(
+ self.product, required_grant, self.owner)):
+ return []
+
+ return POLICY_ALLOWED_TYPES[
+ self.product.branch_sharing_policy]
+
+ # The project still uses BranchVisibilityPolicy, so check that.
private_rules = (
BranchVisibilityRule.PRIVATE,
BranchVisibilityRule.PRIVATE_ONLY)
@@ -403,6 +456,17 @@
types.extend(PRIVATE_INFORMATION_TYPES)
return types
+ def getDefaultInformationType(self):
+ """See `IBranchNamespace`."""
+ if not self._using_branchvisibilitypolicy:
+ default_type = POLICY_DEFAULT_TYPES[
+ self.product.branch_sharing_policy]
+ if default_type not in self.getAllowedInformationTypes():
+ return None
+ return default_type
+
+ return super(ProductNamespace, self).getDefaultInformationType()
+
class PackageNamespace(_BaseNamespace):
"""A namespace for source package branches.
=== modified file 'lib/lp/code/model/tests/test_branchnamespace.py'
--- lib/lp/code/model/tests/test_branchnamespace.py 2012-07-11 22:16:47 +0000
+++ lib/lp/code/model/tests/test_branchnamespace.py 2012-07-13 08:51:30 +0000
@@ -8,6 +8,7 @@
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
+from lp.app.interfaces.services import IService
from lp.app.validators import LaunchpadValidationError
from lp.code.enums import (
BranchLifecycleStatus,
@@ -36,9 +37,11 @@
ProductNamespace,
)
from lp.registry.enums import (
+ BranchSharingPolicy,
InformationType,
PRIVATE_INFORMATION_TYPES,
PUBLIC_INFORMATION_TYPES,
+ SharingPermission,
)
from lp.registry.errors import (
NoSuchDistroSeries,
@@ -52,7 +55,11 @@
)
from lp.registry.interfaces.product import NoSuchProduct
from lp.registry.model.sourcepackage import SourcePackage
-from lp.testing import TestCaseWithFactory
+from lp.services.features.testing import FeatureFixture
+from lp.testing import (
+ admin_logged_in,
+ TestCaseWithFactory,
+ )
from lp.testing.layers import DatabaseFunctionalLayer
@@ -356,8 +363,12 @@
self.assertEqual(IBranchTarget(product), namespace.target)
-class TestProductNamespacePrivacy(TestCaseWithFactory):
- """Tests for the privacy aspects of `ProductNamespace`."""
+class TestProductNamespacePrivacyWithBranchVisibility(TestCaseWithFactory):
+ """Tests for the privacy aspects of `ProductNamespace`.
+
+ This tests the behaviour for a product using the old
+ BranchVisibilityPolicy rules.
+ """
layer = DatabaseFunctionalLayer
@@ -436,6 +447,84 @@
self.assertEqual(team, namespace.getPrivacySubscriber())
+class TestProductNamespacePrivacyWithInformationType(TestCaseWithFactory):
+ """Tests for the privacy aspects of `ProductNamespace`.
+
+ This tests the behaviour for a product using the new
+ branch_sharing_policy rules.
+ """
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestProductNamespacePrivacyWithInformationType, self).setUp()
+ self.useFixture(FeatureFixture(
+ {'disclosure.enhanced_sharing.writable': 'true'}))
+
+ def makeProductNamespace(self, sharing_policy, person=None):
+ if person is None:
+ person = self.factory.makePerson()
+ product = self.factory.makeProduct()
+ removeSecurityProxy(product).branch_sharing_policy = (
+ sharing_policy)
+ namespace = ProductNamespace(person, product)
+ return namespace
+
+ def test_public_anyone(self):
+ namespace = self.makeProductNamespace(
+ BranchSharingPolicy.PUBLIC)
+ self.assertContentEqual(
+ PUBLIC_INFORMATION_TYPES, namespace.getAllowedInformationTypes())
+ self.assertEqual(
+ InformationType.PUBLIC, namespace.getDefaultInformationType())
+
+ def test_public_or_proprietary_anyone(self):
+ namespace = self.makeProductNamespace(
+ BranchSharingPolicy.PUBLIC_OR_PROPRIETARY)
+ self.assertContentEqual(
+ PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES,
+ namespace.getAllowedInformationTypes())
+ self.assertEqual(
+ InformationType.PUBLIC, namespace.getDefaultInformationType())
+
+ def test_proprietary_or_public_anyone(self):
+ namespace = self.makeProductNamespace(
+ BranchSharingPolicy.PROPRIETARY_OR_PUBLIC)
+ self.assertContentEqual([], namespace.getAllowedInformationTypes())
+ self.assertIs(None, namespace.getDefaultInformationType())
+
+ def test_proprietary_or_public_grantor(self):
+ namespace = self.makeProductNamespace(
+ BranchSharingPolicy.PROPRIETARY_OR_PUBLIC)
+ with admin_logged_in():
+ getUtility(IService, 'sharing').sharePillarInformation(
+ namespace.product, namespace.owner, namespace.product.owner,
+ {InformationType.USERDATA: SharingPermission.ALL})
+ self.assertContentEqual(
+ PUBLIC_INFORMATION_TYPES + PRIVATE_INFORMATION_TYPES,
+ namespace.getAllowedInformationTypes())
+ self.assertEqual(
+ InformationType.USERDATA, namespace.getDefaultInformationType())
+
+ def test_proprietary_anyone(self):
+ namespace = self.makeProductNamespace(
+ BranchSharingPolicy.PROPRIETARY)
+ self.assertContentEqual([], namespace.getAllowedInformationTypes())
+ self.assertIs(None, namespace.getDefaultInformationType())
+
+ def test_proprietary_grantor(self):
+ namespace = self.makeProductNamespace(
+ BranchSharingPolicy.PROPRIETARY)
+ with admin_logged_in():
+ getUtility(IService, 'sharing').sharePillarInformation(
+ namespace.product, namespace.owner, namespace.product.owner,
+ {InformationType.USERDATA: SharingPermission.ALL})
+ self.assertContentEqual(
+ PRIVATE_INFORMATION_TYPES, namespace.getAllowedInformationTypes())
+ self.assertEqual(
+ InformationType.USERDATA, namespace.getDefaultInformationType())
+
+
class TestPackageNamespace(TestCaseWithFactory, NamespaceMixin):
"""Tests for `PackageNamespace`."""
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2012-07-06 19:00:25 +0000
+++ lib/lp/registry/browser/product.py 2012-07-13 08:51:30 +0000
@@ -114,6 +114,7 @@
from lp.app.widgets.itemswidgets import (
CheckBoxMatrixWidget,
LaunchpadRadioWidget,
+ LaunchpadRadioWidgetWithDescription,
)
from lp.app.widgets.popup import PersonPickerWidget
from lp.app.widgets.product import (
@@ -1573,6 +1574,9 @@
"private_bugs",
]
+ custom_widget(
+ 'branch_sharing_policy', LaunchpadRadioWidgetWithDescription)
+
@property
def page_title(self):
"""The HTML page title."""
@@ -1590,6 +1594,8 @@
if not admin:
self.field_names.remove('owner')
self.field_names.remove('autoupdate')
+ if getFeatureFlag('disclosure.branch_sharing_policy.show_to_admin'):
+ self.field_names.append('branch_sharing_policy')
super(ProductAdminView, self).setUpFields()
self.form_fields = self._createAliasesField() + self.form_fields
if admin:
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2012-06-29 02:15:05 +0000
+++ lib/lp/registry/configure.zcml 2012-07-13 08:51:30 +0000
@@ -1324,7 +1324,7 @@
project_reviewed
license_approved"
set_schema="lp.registry.interfaces.product.IProductModerateRestricted"
- set_attributes="active"/>
+ set_attributes="active branch_sharing_policy"/>
<!-- IHasAliases -->
=== modified file 'lib/lp/registry/enums.py'
--- lib/lp/registry/enums.py 2012-05-16 02:09:30 +0000
+++ lib/lp/registry/enums.py 2012-07-13 08:51:30 +0000
@@ -5,6 +5,8 @@
__metaclass__ = type
__all__ = [
+ 'BranchSharingPolicy',
+ 'BugSharingPolicy',
'DistroSeriesDifferenceStatus',
'DistroSeriesDifferenceType',
'InformationType',
@@ -102,6 +104,65 @@
""")
+class BranchSharingPolicy(DBEnumeratedType):
+
+ PUBLIC = DBItem(1, """
+ Public
+
+ Branches are public unless they contain sensitive security
+ information.
+ """)
+
+ PUBLIC_OR_PROPRIETARY = DBItem(2, """
+ Public, can be proprietary
+
+ New branches are public, but can be made proprietary later.
+ """)
+
+ PROPRIETARY_OR_PUBLIC = DBItem(3, """
+ Proprietary, can be public
+
+ New branches are proprietary, but can be made public later. Only
+ people who can see the project's proprietary information can create
+ new branches.
+ """)
+
+ PROPRIETARY = DBItem(4, """
+ Proprietary
+
+ Branches are always proprietary. Only people who can see the
+ project's proprietary information can create new branches.
+ """)
+
+
+class BugSharingPolicy(DBEnumeratedType):
+
+ PUBLIC = DBItem(1, """
+ Public
+
+ Bugs are public unless they contain sensitive security
+ information.
+ """)
+
+ PUBLIC_OR_PROPRIETARY = DBItem(2, """
+ Public, can be proprietary
+
+ New bugs are public, but can be made proprietary later.
+ """)
+
+ PROPRIETARY_OR_PUBLIC = DBItem(3, """
+ Proprietary, can be public
+
+ New bugs are proprietary, but can be made public later.
+ """)
+
+ PROPRIETARY = DBItem(4, """
+ Proprietary
+
+ Bugs are always proprietary.
+ """)
+
+
class DistroSeriesDifferenceStatus(DBEnumeratedType):
"""Distribution series difference status.
=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py 2012-05-25 21:18:48 +0000
+++ lib/lp/registry/interfaces/product.py 2012-07-13 08:51:30 +0000
@@ -108,6 +108,10 @@
IHasMergeProposals,
)
from lp.code.interfaces.hasrecipes import IHasRecipes
+from lp.registry.enums import (
+ BranchSharingPolicy,
+ BugSharingPolicy,
+ )
from lp.registry.interfaces.announcement import IMakesAnnouncements
from lp.registry.interfaces.commercialsubscription import (
ICommercialSubscription,
@@ -634,6 +638,17 @@
description=_(
"Whether or not bugs reported into this project "
"are private by default.")))
+ branch_sharing_policy = exported(Choice(
+ title=_('Branch sharing policy'),
+ description=_("Sharing policy for this project's branches."),
+ required=False, readonly=False, vocabulary=BranchSharingPolicy),
+ as_of='devel')
+ bug_sharing_policy = exported(Choice(
+ title=_('Bug sharing policy'),
+ description=_("Sharing policy for this project's bugs."),
+ required=False, readonly=True, vocabulary=BugSharingPolicy),
+ as_of='devel')
+
licenses = exported(
Set(title=_('Licences'),
value_type=Choice(vocabulary=License)))
=== modified file 'lib/lp/registry/interfaces/sharingservice.py'
--- lib/lp/registry/interfaces/sharingservice.py 2012-06-18 03:23:18 +0000
+++ lib/lp/registry/interfaces/sharingservice.py 2012-07-13 08:51:30 +0000
@@ -45,6 +45,13 @@
# version 'devel'
export_as_webservice_entry(publish_web_link=False, as_of='beta')
+ def checkPillarAccess(pillar, information_type, person):
+ """Check the person's access to the given pillar and information type.
+
+ :return: True if the user has access to all the pillar's information
+ of that type, False otherwise
+ """
+
def getSharedArtifacts(pillar, person, user):
"""Return the artifacts shared between the pillar and person.
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2012-07-09 04:14:09 +0000
+++ lib/lp/registry/model/product.py 2012-07-13 08:51:30 +0000
@@ -116,7 +116,11 @@
)
from lp.code.model.sourcepackagerecipe import SourcePackageRecipe
from lp.code.model.sourcepackagerecipedata import SourcePackageRecipeData
-from lp.registry.enums import InformationType
+from lp.registry.enums import (
+ BranchSharingPolicy,
+ BugSharingPolicy,
+ InformationType,
+ )
from lp.registry.errors import CommercialSubscribersOnly
from lp.registry.interfaces.accesspolicy import (
IAccessPolicySource,
@@ -469,6 +473,10 @@
reviewer_whiteboard = StringCol(notNull=False, default=None)
private_bugs = BoolCol(
dbName='private_bugs', notNull=True, default=False)
+ bug_sharing_policy = EnumCol(
+ enum=BugSharingPolicy, notNull=False, default=None)
+ branch_sharing_policy = EnumCol(
+ enum=BranchSharingPolicy, notNull=False, default=None)
autoupdate = BoolCol(dbName='autoupdate', notNull=True, default=False)
freshmeatproject = StringCol(notNull=False, default=None)
sourceforgeproject = StringCol(notNull=False, default=None)
=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py 2012-07-02 23:40:56 +0000
+++ lib/lp/registry/services/sharingservice.py 2012-07-13 08:51:30 +0000
@@ -84,6 +84,25 @@
bool(getFeatureFlag(
'disclosure.enhanced_sharing.writable')))
+ def checkPillarAccess(self, pillar, information_type, person):
+ """See `ISharingService`."""
+ policy = getUtility(IAccessPolicySource).find(
+ [(pillar, information_type)]).one()
+ if policy is None:
+ return False
+ store = IStore(AccessPolicyGrant)
+ tables = [
+ AccessPolicyGrant,
+ Join(
+ TeamParticipation,
+ TeamParticipation.teamID == AccessPolicyGrant.grantee_id),
+ ]
+ result = store.using(*tables).find(
+ AccessPolicyGrant,
+ AccessPolicyGrant.policy_id == policy.id,
+ TeamParticipation.personID == person.id)
+ return not result.is_empty()
+
def getSharedArtifacts(self, pillar, person, user):
"""See `ISharingService`."""
policies = getUtility(IAccessPolicySource).findByPillar([pillar])
=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py 2012-07-11 01:21:56 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py 2012-07-13 08:51:30 +0000
@@ -39,6 +39,7 @@
from lp.services.webapp.interfaces import ILaunchpadRoot
from lp.services.webapp.publisher import canonical_url
from lp.testing import (
+ admin_logged_in,
login,
login_person,
StormStatementRecorder,
@@ -493,7 +494,7 @@
[InformationType.USERDATA,
InformationType.EMBARGOEDSECURITY]),
]
- else:
+ else:
expected_sharee_grants = [
(sharee,
{es_policy: SharingPermission.ALL,
@@ -1253,6 +1254,38 @@
self._assert_getVisibleArtifacts_bug_change(retarget_bugtask)
+ def test_checkPillarAccess(self):
+ # checkPillarAccess checks whether the user has full access to
+ # an information type.
+ product = self.factory.makeProduct()
+ right_person = self.factory.makePerson()
+ right_team = self.factory.makeTeam(members=[right_person])
+ wrong_person = self.factory.makePerson()
+ with FeatureFixture(WRITE_FLAG):
+ with admin_logged_in():
+ self.service.sharePillarInformation(
+ product, right_team, product.owner,
+ {InformationType.USERDATA: SharingPermission.ALL})
+ self.service.sharePillarInformation(
+ product, wrong_person, product.owner,
+ {InformationType.EMBARGOEDSECURITY: SharingPermission.ALL})
+ self.assertEqual(
+ False,
+ self.service.checkPillarAccess(
+ product, InformationType.USERDATA, wrong_person))
+ self.assertEqual(
+ True,
+ self.service.checkPillarAccess(
+ product, InformationType.USERDATA, right_person))
+
+ def test_checkPillarAccess_no_policy(self):
+ # checkPillarAccess returns False if there's no policy.
+ self.assertEqual(
+ False,
+ self.service.checkPillarAccess(
+ self.factory.makeProduct(), InformationType.PUBLIC,
+ self.factory.makePerson()))
+
class ApiTestMixin:
"""Common tests for launchpadlib and webservice."""
=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py 2012-07-11 01:24:42 +0000
+++ lib/lp/services/features/flags.py 2012-07-13 08:51:30 +0000
@@ -318,6 +318,13 @@
'',
'',
''),
+ ('disclosure.branch_sharing_policy.show_to_admin',
+ 'boolean',
+ ('If true, the branch sharing policy field is shown on '
+ 'Product:+admin, letting BranchVisibilityPolicy be overridden.'),
+ '',
+ '',
+ ''),
])
# The set of all flag names that are documented.
Follow ups
