launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26628
[Merge] ~pappacena/launchpad:ocirecipe-subscription into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:ocirecipe-subscription into launchpad:master with ~pappacena/launchpad:ocirecipe-private-reconcile-pillar as a prerequisite.
Commit message:
Adding OCI recipe subscription model
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399544
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:ocirecipe-subscription into launchpad:master.
diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index 2f4ffdf..5e5e000 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -61,6 +61,7 @@ public.milestone_sort_key(timestamp without time zone, text) = EXECUTE
public.min(debversion) = EXECUTE
public.name_blacklist_match(text, integer) = EXECUTE
public.null_count(anyarray) = EXECUTE
+public.ocirecipe_denorm_access(integer) = EXECUTE
public.person_sort_key(text, text) = EXECUTE
public.pgstatginindex(regclass) =
public.pgstathashindex(regclass) =
@@ -252,6 +253,7 @@ public.ocirecipearch = SELECT, INSERT, DELETE
public.ocirecipebuild = SELECT, INSERT, UPDATE, DELETE
public.ocirecipebuildjob = SELECT, INSERT, UPDATE, DELETE
public.ocirecipejob = SELECT, INSERT, UPDATE, DELETE
+public.ocirecipesubscription = SELECT, INSERT, UPDATE, DELETE
public.ociregistrycredentials = SELECT, INSERT, UPDATE, DELETE
public.officialbugtag = SELECT, INSERT, UPDATE, DELETE
public.openidconsumerassociation = SELECT, INSERT, UPDATE, DELETE
@@ -2333,6 +2335,7 @@ public.ociproject = SELECT, UPDATE
public.ociprojectseries = SELECT, UPDATE
public.ocirecipe = SELECT, UPDATE
public.ocirecipebuild = SELECT, UPDATE
+public.ocirecipesubscription = SELECT, UPDATE, DELETE
public.ociregistrycredentials = SELECT, UPDATE
public.officialbugtag = SELECT
public.openididentifier = SELECT, UPDATE
diff --git a/lib/lp/blueprints/model/specification.py b/lib/lp/blueprints/model/specification.py
index 9f3b00a..404481a 100644
--- a/lib/lp/blueprints/model/specification.py
+++ b/lib/lp/blueprints/model/specification.py
@@ -755,8 +755,9 @@ class Specification(SQLBase, BugLinkTargetMixin, InformationTypeMixin):
# Grant the subscriber access if they can't see the
# specification.
service = getUtility(IService, 'sharing')
- _, _, _, _, shared_specs = service.getVisibleArtifacts(
- person, specifications=[self], ignore_permissions=True)
+ shared_specs = service.getVisibleArtifacts(
+ person, specifications=[self],
+ ignore_permissions=True)["specifications"]
if not shared_specs:
service.ensureAccessGrants(
[person], subscribed_by, specifications=[self])
diff --git a/lib/lp/blueprints/tests/test_specification.py b/lib/lp/blueprints/tests/test_specification.py
index 86c4e4a..dcc17b6 100644
--- a/lib/lp/blueprints/tests/test_specification.py
+++ b/lib/lp/blueprints/tests/test_specification.py
@@ -492,8 +492,8 @@ class SpecificationTests(TestCaseWithFactory):
product=product, information_type=InformationType.PROPRIETARY)
spec.subscribe(user, subscribed_by=owner)
service = getUtility(IService, 'sharing')
- _, _, _, _, shared_specs = service.getVisibleArtifacts(
- user, specifications=[spec])
+ shared_specs = service.getVisibleArtifacts(
+ user, specifications=[spec])["specifications"]
self.assertEqual([spec], shared_specs)
# The spec is also returned by getSharedSpecifications(),
# which lists only specifications for which the use has
@@ -509,8 +509,8 @@ class SpecificationTests(TestCaseWithFactory):
service.sharePillarInformation(
product, user_2, owner, permissions)
spec.subscribe(user_2, subscribed_by=owner)
- _, _, _, _, shared_specs = service.getVisibleArtifacts(
- user_2, specifications=[spec])
+ shared_specs = service.getVisibleArtifacts(
+ user_2, specifications=[spec])["specifications"]
self.assertEqual([spec], shared_specs)
self.assertEqual(
[], service.getSharedSpecifications(product, user_2, owner))
@@ -529,8 +529,8 @@ class SpecificationTests(TestCaseWithFactory):
spec.subscribe(user, subscribed_by=owner)
spec.unsubscribe(user, unsubscribed_by=owner)
service = getUtility(IService, 'sharing')
- _, _, _, _, shared_specs = service.getVisibleArtifacts(
- user, specifications=[spec])
+ shared_specs = service.getVisibleArtifacts(
+ user, specifications=[spec])["specifications"]
self.assertEqual([], shared_specs)
def test_notificationRecipientAddresses_filters_based_on_sharing(self):
diff --git a/lib/lp/bugs/model/bug.py b/lib/lp/bugs/model/bug.py
index f2148e0..8f1480c 100644
--- a/lib/lp/bugs/model/bug.py
+++ b/lib/lp/bugs/model/bug.py
@@ -875,8 +875,8 @@ class Bug(SQLBase, InformationTypeMixin):
# there is at least one bugtask for which access can be checked.
if self.default_bugtask:
service = getUtility(IService, 'sharing')
- bugs, _, _, _, _ = service.getVisibleArtifacts(
- person, bugs=[self], ignore_permissions=True)
+ bugs = service.getVisibleArtifacts(
+ person, bugs=[self], ignore_permissions=True)["bugs"]
if not bugs:
service.ensureAccessGrants(
[person], subscribed_by, bugs=[self],
@@ -1819,8 +1819,8 @@ class Bug(SQLBase, InformationTypeMixin):
if information_type in PRIVATE_INFORMATION_TYPES:
service = getUtility(IService, 'sharing')
for person in (who, self.owner):
- bugs, _, _, _, _ = service.getVisibleArtifacts(
- person, bugs=[self], ignore_permissions=True)
+ bugs = service.getVisibleArtifacts(
+ person, bugs=[self], ignore_permissions=True)["bugs"]
if not bugs:
# subscribe() isn't sufficient if a subscription
# already exists, as it will do nothing even if
diff --git a/lib/lp/code/browser/branchsubscription.py b/lib/lp/code/browser/branchsubscription.py
index cee4504..7c3b043 100644
--- a/lib/lp/code/browser/branchsubscription.py
+++ b/lib/lp/code/browser/branchsubscription.py
@@ -276,8 +276,9 @@ class BranchSubscriptionEditView(LaunchpadEditFormView):
url = canonical_url(self.branch)
# If the subscriber can no longer see the branch, redirect them away.
service = getUtility(IService, 'sharing')
- _, branches, _, _, _ = service.getVisibleArtifacts(
- self.person, branches=[self.branch], ignore_permissions=True)
+ branches = service.getVisibleArtifacts(
+ self.person, branches=[self.branch],
+ ignore_permissions=True)["branches"]
if not branches:
url = canonical_url(self.branch.target)
return url
diff --git a/lib/lp/code/browser/gitsubscription.py b/lib/lp/code/browser/gitsubscription.py
index a18d593..a07df31 100644
--- a/lib/lp/code/browser/gitsubscription.py
+++ b/lib/lp/code/browser/gitsubscription.py
@@ -280,9 +280,9 @@ class GitSubscriptionEditView(LaunchpadEditFormView):
# If the subscriber can no longer see the repository, redirect them
# away.
service = getUtility(IService, "sharing")
- _, _, repositories, _, _ = service.getVisibleArtifacts(
+ repositories = service.getVisibleArtifacts(
self.person, gitrepositories=[self.repository],
- ignore_permissions=True)
+ ignore_permissions=True)["gitrepositories"]
if not repositories:
url = canonical_url(self.repository.target)
return url
diff --git a/lib/lp/code/model/branch.py b/lib/lp/code/model/branch.py
index 27746c7..f6de617 100644
--- a/lib/lp/code/model/branch.py
+++ b/lib/lp/code/model/branch.py
@@ -1041,8 +1041,9 @@ class Branch(SQLBase, WebhookTargetMixin, BzrIdentityMixin):
subscription.review_level = code_review_level
# Grant the subscriber access if they can't see the branch.
service = getUtility(IService, 'sharing')
- _, branches, _, _, _ = service.getVisibleArtifacts(
- person, branches=[self], ignore_permissions=True)
+ branches = service.getVisibleArtifacts(
+ person, branches=[self],
+ ignore_permissions=True)["branches"]
if not branches:
service.ensureAccessGrants(
[person], subscribed_by, branches=[self],
diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
index 3c896f6..286abcc 100644
--- a/lib/lp/code/model/gitrepository.py
+++ b/lib/lp/code/model/gitrepository.py
@@ -1002,8 +1002,9 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
subscription.review_level = code_review_level
# Grant the subscriber access if they can't see the repository.
service = getUtility(IService, "sharing")
- _, _, repositories, _, _ = service.getVisibleArtifacts(
- person, gitrepositories=[self], ignore_permissions=True)
+ repositories = service.getVisibleArtifacts(
+ person, gitrepositories=[self],
+ ignore_permissions=True)["gitrepositories"]
if not repositories:
service.ensureAccessGrants(
[person], subscribed_by, gitrepositories=[self],
diff --git a/lib/lp/code/model/tests/test_branchsubscription.py b/lib/lp/code/model/tests/test_branchsubscription.py
index b5b234e..9b13adf 100644
--- a/lib/lp/code/model/tests/test_branchsubscription.py
+++ b/lib/lp/code/model/tests/test_branchsubscription.py
@@ -134,8 +134,8 @@ class TestBranchSubscriptions(TestCaseWithFactory):
None, CodeReviewNotificationLevel.NOEMAIL, owner)
# The stacked on branch should be visible.
service = getUtility(IService, 'sharing')
- _, visible_branches, _, _, _ = service.getVisibleArtifacts(
- grantee, branches=[private_stacked_on_branch])
+ visible_branches = service.getVisibleArtifacts(
+ grantee, branches=[private_stacked_on_branch])["branches"]
self.assertContentEqual(
[private_stacked_on_branch], visible_branches)
self.assertIn(
@@ -162,8 +162,8 @@ class TestBranchSubscriptions(TestCaseWithFactory):
grantee, BranchSubscriptionNotificationLevel.NOEMAIL,
None, CodeReviewNotificationLevel.NOEMAIL, owner)
# The stacked on branch should not be visible.
- _, visible_branches, _, _, _ = service.getVisibleArtifacts(
- grantee, branches=[private_stacked_on_branch])
+ visible_branches = service.getVisibleArtifacts(
+ grantee, branches=[private_stacked_on_branch])["branches"]
self.assertContentEqual([], visible_branches)
self.assertIn(
grantee, branch.subscribers)
diff --git a/lib/lp/oci/configure.zcml b/lib/lp/oci/configure.zcml
index 1f0e284..82cf7ff 100644
--- a/lib/lp/oci/configure.zcml
+++ b/lib/lp/oci/configure.zcml
@@ -41,6 +41,17 @@
interface="lp.oci.interfaces.ocirecipe.IOCIRecipeSet"/>
</securedutility>
+ <!-- OCIRecipeSubscription -->
+
+ <class class="lp.oci.model.ocirecipesubscription.OCIRecipeSubscription">
+ <require
+ permission="launchpad.View"
+ interface="lp.oci.interfaces.ocirecipesubscription.IOCIRecipeSubscription"/>
+ <require
+ permission="launchpad.Edit"
+ set_schema="lp.oci.interfaces.ocirecipesubscription.IOCIRecipeSubscription"/>
+ </class>
+
<!-- OCIRecipeRequestBuildsJob related classes -->
<class class="lp.oci.model.ocirecipe.OCIRecipeBuildRequest">
<require
diff --git a/lib/lp/oci/interfaces/ocirecipe.py b/lib/lp/oci/interfaces/ocirecipe.py
index 26e48e0..b87e904 100644
--- a/lib/lp/oci/interfaces/ocirecipe.py
+++ b/lib/lp/oci/interfaces/ocirecipe.py
@@ -72,6 +72,7 @@ from lp.oci.enums import OCIRecipeBuildRequestStatus
from lp.registry.interfaces.distribution import IDistribution
from lp.registry.interfaces.distroseries import IDistroSeries
from lp.registry.interfaces.ociproject import IOCIProject
+from lp.registry.interfaces.person import IPerson
from lp.registry.interfaces.role import IHasOwner
from lp.services.database.constants import DEFAULT
from lp.services.fields import (
@@ -292,6 +293,10 @@ class IOCIRecipeView(Interface):
description=_("Use the credentials on a Distribution for "
"registry upload"))
+ subscribers = CollectionField(
+ title=_("Persons subscribed to this snap recipe."),
+ readonly=True, value_type=Reference(IPerson))
+
def requestBuild(requester, architecture):
"""Request that the OCI recipe is built.
@@ -327,6 +332,18 @@ class IOCIRecipeView(Interface):
"""Get an OCIRecipeBuildRequest object for the given job_id.
"""
+ def visibleByUser(user):
+ """Can the specified user see this snap recipe?"""
+
+ def getSubscription(person):
+ """Returns the OCIRecipeSubscription for the given user."""
+
+ def subscribe(person, subscribed_by):
+ """Subscribe a person to this snap recipe."""
+
+ def unsubscribe(person, unsubscribed_by):
+ """Unsubscribe a person to this snap recipe."""
+
class IOCIRecipeEdit(IWebhookTarget):
"""`IOCIRecipe` methods that require launchpad.Edit permission."""
@@ -506,6 +523,9 @@ class IOCIRecipeSet(Interface):
def exists(owner, oci_project, name):
"""Check to see if an existing OCI Recipe exists."""
+ def findByIds(ocirecipe_ids, visible_by_user=None):
+ """Returns the OCI recipes with the given IDs."""
+
def getByName(owner, oci_project, name):
"""Return the appropriate `OCIRecipe` for the given objects."""
diff --git a/lib/lp/oci/interfaces/ocirecipesubscription.py b/lib/lp/oci/interfaces/ocirecipesubscription.py
new file mode 100644
index 0000000..d0dc358
--- /dev/null
+++ b/lib/lp/oci/interfaces/ocirecipesubscription.py
@@ -0,0 +1,43 @@
+# Copyright 2020-2021 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""OCIRecipe subscription model."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+ 'IOCIRecipeSubscription'
+]
+
+from lazr.restful.fields import Reference
+from zope.interface import Interface
+from zope.schema import (
+ Datetime,
+ Int,
+ )
+
+from lp import _
+from lp.services.fields import PersonChoice
+from lp.oci.interfaces.ocirecipe import IOCIRecipe
+
+
+class IOCIRecipeSubscription(Interface):
+ """A person subscription to a specific OCIRecipe recipe."""
+
+ id = Int(title=_('ID'), readonly=True, required=True)
+ person = PersonChoice(
+ title=_('Person'), required=True, vocabulary='ValidPersonOrTeam',
+ readonly=True,
+ description=_("The person subscribed to the related OCI recipe."))
+ ocirecipe = Reference(
+ IOCIRecipe, title=_("OCI recipe"), required=True, readonly=True)
+ subscribed_by = PersonChoice(
+ title=_('Subscribed by'), required=True,
+ vocabulary='ValidPersonOrTeam', readonly=True,
+ description=_("The person who created this subscription."))
+ date_created = Datetime(
+ title=_('Date subscribed'), required=True, readonly=True)
+
+ def canBeUnsubscribedByUser(user):
+ """Can the user unsubscribe the subscriber from the OCI recipe?"""
diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index cabce57..17fcb7b 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -19,9 +19,14 @@ import six
from storm.databases.postgres import JSON
from storm.expr import (
And,
+ Coalesce,
Desc,
+ Exists,
+ Join,
Not,
+ Or,
Select,
+ SQL,
)
from storm.locals import (
Bool,
@@ -44,9 +49,17 @@ from zope.security.proxy import (
removeSecurityProxy,
)
-from lp.app.enums import InformationType
+from lp.app.enums import (
+ InformationType,
+ PUBLIC_INFORMATION_TYPES,
+ )
+from lp.app.errors import (
+ SubscriptionPrivacyViolation,
+ UserCannotUnsubscribePerson,
+ )
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.app.interfaces.security import IAuthorization
+from lp.app.interfaces.services import IService
from lp.app.validators.validation import validate_oci_branch_name
from lp.buildmaster.enums import BuildStatus
from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
@@ -83,14 +96,23 @@ from lp.oci.model.ocipushrule import (
)
from lp.oci.model.ocirecipebuild import OCIRecipeBuild
from lp.oci.model.ocirecipejob import OCIRecipeJob
+from lp.oci.model.ocirecipesubscription import OCIRecipeSubscription
+from lp.registry.interfaces.accesspolicy import (
+ IAccessArtifactGrantSource,
+ IAccessArtifactSource,
+ )
from lp.registry.interfaces.distribution import IDistributionSet
from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.role import IPersonRoles
-from lp.registry.model.accesspolicy import reconcile_access_for_artifacts
+from lp.registry.model.accesspolicy import (
+ AccessPolicyGrant,
+ reconcile_access_for_artifacts,
+ )
from lp.registry.model.distribution import Distribution
from lp.registry.model.distroseries import DistroSeries
from lp.registry.model.person import Person
from lp.registry.model.series import ACTIVE_STATUSES
+from lp.registry.model.teammembership import TeamParticipation
from lp.services.database.bulk import load_related
from lp.services.database.constants import (
DEFAULT,
@@ -103,6 +125,9 @@ from lp.services.database.interfaces import (
IStore,
)
from lp.services.database.stormexpr import (
+ Array,
+ ArrayAgg,
+ ArrayIntersects,
Greatest,
NullsLast,
)
@@ -219,6 +244,11 @@ class OCIRecipe(Storm, WebhookTargetMixin):
self._reconcileAccess()
@property
+ def private(self):
+ return (self.information_type is None
+ and self.information_type not in PUBLIC_INFORMATION_TYPES)
+
+ @property
def pillar(self):
return self.oci_project.pillar
@@ -246,7 +276,7 @@ class OCIRecipe(Storm, WebhookTargetMixin):
for k, v in (value or {}).items()}
def _reconcileAccess(self):
- """Reconcile the snap's sharing information.
+ """Reconcile the OCI recipe's sharing information.
Takes the privacy and pillar and makes the related AccessArtifact
and AccessPolicyArtifacts match.
@@ -254,11 +284,94 @@ class OCIRecipe(Storm, WebhookTargetMixin):
reconcile_access_for_artifacts([self], self.information_type,
[self.pillar])
+ def visibleByUser(self, user):
+ """See `IOCIRecipe`."""
+ if self.information_type in PUBLIC_INFORMATION_TYPES:
+ return True
+ if user is None:
+ return False
+ store = IStore(self)
+ return not store.find(
+ OCIRecipe,
+ OCIRecipe.id == self.id,
+ get_ocirecipe_privacy_filter(user)).is_empty()
+
+ def getSubscription(self, person):
+ """See `IOCIRecipe`."""
+ if person is None:
+ return None
+ return Store.of(self).find(
+ OCIRecipeSubscription,
+ OCIRecipeSubscription.person == person,
+ OCIRecipeSubscription.ocirecipe == self).one()
+
+ def userCanBeSubscribed(self, person):
+ """Checks if the given person can subscribe to this OCI recipe."""
+ return not (
+ self.private and
+ person.is_team and
+ person.anyone_can_join())
+
+ @property
+ def subscribers(self):
+ return Store.of(self).find(
+ Person,
+ OCIRecipeSubscription.person_id == Person.id,
+ OCIRecipeSubscription.ocirecipe == self)
+
+ def subscribe(self, person, subscribed_by, ignore_permissions=False):
+ """See `IOCIRecipe`."""
+ if not self.userCanBeSubscribed(person):
+ raise SubscriptionPrivacyViolation(
+ "Open and delegated teams cannot be subscribed to private "
+ "OCI recipes.")
+ subscription = self.getSubscription(person)
+ if subscription is None:
+ subscription = OCIRecipeSubscription(
+ person=person, ocirecipe=self, subscribed_by=subscribed_by)
+ Store.of(subscription).flush()
+ service = getUtility(IService, "sharing")
+ ocirecipes = service.getVisibleArtifacts(
+ person, ocirecipes=[self], ignore_permissions=True)["ocirecipes"]
+ if not ocirecipes:
+ service.ensureAccessGrants(
+ [person], subscribed_by, ocirecipes=[self],
+ ignore_permissions=ignore_permissions)
+
+ def unsubscribe(self, person, unsubscribed_by, ignore_permissions=False):
+ """See `IOCIRecipe`."""
+ subscription = self.getSubscription(person)
+ if subscription is None:
+ return
+ if (not ignore_permissions
+ and not subscription.canBeUnsubscribedByUser(unsubscribed_by)):
+ raise UserCannotUnsubscribePerson(
+ '%s does not have permission to unsubscribe %s.' % (
+ unsubscribed_by.displayname,
+ person.displayname))
+ artifact = getUtility(IAccessArtifactSource).find([self])
+ getUtility(IAccessArtifactGrantSource).revokeByArtifact(
+ artifact, [person])
+ store = Store.of(subscription)
+ store.remove(subscription)
+ IStore(self).flush()
+
+ def _deleteAccessGrants(self):
+ """Delete access grants for this snap recipe prior to deleting it."""
+ getUtility(IAccessArtifactSource).delete([self])
+
+ def _deleteOCIRecipeSubscriptions(self):
+ subscriptions = Store.of(self).find(
+ OCIRecipeSubscription, OCIRecipeSubscription.ocirecipe == self)
+ subscriptions.remove()
+
def destroySelf(self):
"""See `IOCIRecipe`."""
# XXX twom 2019-11-26 This needs to expand as more build artifacts
# are added
store = IStore(OCIRecipe)
+ self._deleteOCIRecipeSubscriptions()
+ self._deleteAccessGrants()
store.find(OCIRecipeArch, OCIRecipeArch.recipe == self).remove()
buildqueue_records = store.find(
BuildQueue,
@@ -673,6 +786,10 @@ class OCIRecipeSet:
store.add(oci_recipe)
oci_recipe._reconcileAccess()
+ # Automatically subscribe the owner to the OCI recipe.
+ oci_recipe.subscribe(oci_recipe.owner, registrant,
+ ignore_permissions=True)
+
if processors is None:
processors = [
p for p in oci_recipe.available_processors
@@ -692,6 +809,13 @@ class OCIRecipeSet:
"""See `IOCIRecipeSet`."""
return self._getByName(owner, oci_project, name) is not None
+ def findByIds(self, ocirecipe_ids, visible_by_user=None):
+ """See `IOCIRecipeSet`."""
+ clauses = [OCIRecipe.id.is_in(ocirecipe_ids)]
+ if visible_by_user is not None:
+ clauses.append(get_ocirecipe_privacy_filter(visible_by_user))
+ return IStore(OCIRecipe).find(OCIRecipe, *clauses)
+
def getByName(self, owner, oci_project, name):
"""See `IOCIRecipeSet`."""
oci_recipe = self._getByName(owner, oci_project, name)
@@ -808,3 +932,52 @@ class OCIRecipeBuildRequest:
def __hash__(self):
return hash((self.__class__, self.id))
+
+
+def get_ocirecipe_privacy_filter(user):
+ """Returns the filter for all OCI recipes that the given user has access
+ to, including private OCI recipes where the user has proper permission.
+
+ :param user: An IPerson, or a class attribute that references an IPerson
+ in the database.
+ :return: A storm condition.
+ """
+ # XXX pappacena 2021-03-11: Once we do the migration to back fill
+ # information_type, we should be able to change this.
+ private_recipe = SQL(
+ "COALESCE(information_type NOT IN ?, false)",
+ params=[tuple(i.value for i in PUBLIC_INFORMATION_TYPES)])
+ if user is None:
+ return private_recipe == False
+
+ artifact_grant_query = Coalesce(
+ ArrayIntersects(
+ SQL("%s.access_grants" % OCIRecipe.__storm_table__),
+ Select(
+ ArrayAgg(TeamParticipation.teamID),
+ tables=TeamParticipation,
+ where=(TeamParticipation.person == user)
+ )), False)
+
+ policy_grant_query = Coalesce(
+ ArrayIntersects(
+ Array(SQL("%s.access_policy" % OCIRecipe.__storm_table__)),
+ Select(
+ ArrayAgg(AccessPolicyGrant.policy_id),
+ tables=(AccessPolicyGrant,
+ Join(TeamParticipation,
+ TeamParticipation.teamID ==
+ AccessPolicyGrant.grantee_id)),
+ where=(TeamParticipation.person == user)
+ )), False)
+
+ admin_team_id = getUtility(ILaunchpadCelebrities).admin.id
+ user_is_admin = Exists(Select(
+ TeamParticipation.personID,
+ tables=[TeamParticipation],
+ where=And(
+ TeamParticipation.teamID == admin_team_id,
+ TeamParticipation.person == user)))
+ return Or(
+ private_recipe == False, artifact_grant_query, policy_grant_query,
+ user_is_admin)
diff --git a/lib/lp/oci/model/ocirecipesubscription.py b/lib/lp/oci/model/ocirecipesubscription.py
new file mode 100644
index 0000000..bec9c7a
--- /dev/null
+++ b/lib/lp/oci/model/ocirecipesubscription.py
@@ -0,0 +1,62 @@
+# Copyright 2020-2021 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""OCIRecipe subscription model."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+ 'OCIRecipeSubscription'
+]
+
+import pytz
+from storm.properties import (
+ DateTime,
+ Int,
+ )
+from storm.references import Reference
+from zope.interface import implementer
+
+from lp.registry.interfaces.person import validate_person
+from lp.registry.interfaces.role import IPersonRoles
+from lp.services.database.constants import UTC_NOW
+from lp.services.database.stormbase import StormBase
+from lp.oci.interfaces.ocirecipesubscription import IOCIRecipeSubscription
+
+
+@implementer(IOCIRecipeSubscription)
+class OCIRecipeSubscription(StormBase):
+ """A relationship between a person and an OCI recipe."""
+
+ __storm_table__ = 'OCIRecipeSubscription'
+
+ id = Int(primary=True)
+
+ person_id = Int(
+ "person", allow_none=False, validator=validate_person)
+ person = Reference(person_id, "Person.id")
+
+ ocirecipe_id = Int("ocirecipe", allow_none=False)
+ ocirecipe = Reference(ocirecipe_id, "OCIRecipe.id")
+
+ date_created = DateTime(allow_none=False, default=UTC_NOW, tzinfo=pytz.UTC)
+
+ subscribed_by_id = Int(
+ "subscribed_by", allow_none=False, validator=validate_person)
+ subscribed_by = Reference(subscribed_by_id, "Person.id")
+
+ def __init__(self, ocirecipe, person, subscribed_by):
+ super(OCIRecipeSubscription, self).__init__()
+ self.ocirecipe = ocirecipe
+ self.person = person
+ self.subscribed_by = subscribed_by
+
+ def canBeUnsubscribedByUser(self, user):
+ """See `IOCIRecipeSubscription`."""
+ if user is None:
+ return False
+ return (user.inTeam(self.ocirecipe.owner) or
+ user.inTeam(self.person) or
+ user.inTeam(self.subscribed_by) or
+ IPersonRoles(user).in_admin)
diff --git a/lib/lp/oci/tests/test_ocirecipe.py b/lib/lp/oci/tests/test_ocirecipe.py
index 6c08bb1..27f9d6a 100644
--- a/lib/lp/oci/tests/test_ocirecipe.py
+++ b/lib/lp/oci/tests/test_ocirecipe.py
@@ -5,6 +5,7 @@
from __future__ import absolute_import, print_function, unicode_literals
+from datetime import datetime
import json
from fixtures import FakeLogger
@@ -61,12 +62,17 @@ from lp.oci.tests.helpers import (
MatchesOCIRegistryCredentials,
OCIConfigHelperMixin,
)
+from lp.registry.enums import TeamMembershipPolicy
from lp.registry.interfaces.accesspolicy import (
IAccessArtifactSource,
IAccessPolicyArtifactSource,
IAccessPolicySource,
)
from lp.registry.interfaces.series import SeriesStatus
+from lp.registry.model.accesspolicy import (
+ AccessArtifact,
+ AccessArtifactGrant,
+ )
from lp.services.config import config
from lp.services.database.constants import (
ONE_DAY_AGO,
@@ -842,6 +848,124 @@ class TestOCIRecipeAccessControl(TestCaseWithFactory, OCIConfigHelperMixin):
self.assertEqual(
{i.policy.pillar for i in policy_artifacts}, {final_project})
+ def getGrants(self, ocirecipe, person=None):
+ conditions = [AccessArtifact.ocirecipe == ocirecipe]
+ if person is not None:
+ conditions.append(AccessArtifactGrant.grantee == person)
+ return IStore(AccessArtifactGrant).find(
+ AccessArtifactGrant,
+ AccessArtifactGrant.abstract_artifact_id == AccessArtifact.id,
+ *conditions)
+
+ def test_reconcile_set_public(self):
+ owner = self.factory.makePerson()
+ recipe = self.factory.makeOCIRecipe(
+ registrant=owner, owner=owner,
+ information_type=InformationType.USERDATA)
+ another_user = self.factory.makePerson()
+ with admin_logged_in():
+ recipe.subscribe(another_user, recipe.owner)
+ self.assertEqual(1, self.getGrants(recipe, another_user).count())
+ self.assertThat(
+ recipe.getSubscription(another_user),
+ MatchesStructure(
+ person=Equals(another_user),
+ ocirecipe=Equals(recipe),
+ subscribed_by=Equals(recipe.owner),
+ date_created=IsInstance(datetime)))
+
+ recipe.information_type = InformationType.PUBLIC
+ self.assertEqual(0, self.getGrants(recipe, another_user).count())
+ self.assertThat(
+ recipe.getSubscription(another_user),
+ MatchesStructure(
+ person=Equals(another_user),
+ ocirecipe=Equals(recipe),
+ subscribed_by=Equals(recipe.owner),
+ date_created=IsInstance(datetime)))
+
+ def test_owner_is_subscribed_automatically(self):
+ recipe = self.factory.makeOCIRecipe()
+ owner = recipe.owner
+ registrant = recipe.registrant
+ self.assertTrue(recipe.visibleByUser(owner))
+ self.assertIn(owner, recipe.subscribers)
+ with person_logged_in(owner):
+ self.assertThat(recipe.getSubscription(owner), MatchesStructure(
+ person=Equals(owner),
+ subscribed_by=Equals(registrant),
+ date_created=IsInstance(datetime)))
+
+ def test_only_owner_can_grant_access(self):
+ owner = self.factory.makePerson()
+ recipe = self.factory.makeOCIRecipe(
+ registrant=owner, owner=owner,
+ information_type=InformationType.USERDATA)
+ other_person = self.factory.makePerson()
+ with person_logged_in(owner):
+ recipe.subscribe(other_person, owner)
+ with person_logged_in(other_person):
+ self.assertRaises(
+ Unauthorized, recipe.subscribe, other_person, other_person)
+
+ def test_private_is_invisible_by_default(self):
+ owner = self.factory.makePerson()
+ person = self.factory.makePerson()
+ recipe = self.factory.makeOCIRecipe(
+ registrant=owner, owner=owner,
+ information_type=InformationType.USERDATA)
+ with person_logged_in(owner):
+ self.assertFalse(recipe.visibleByUser(person))
+
+ def test_private_is_visible_by_team_member(self):
+ person = self.factory.makePerson()
+ team = self.factory.makeTeam(
+ members=[person], membership_policy=TeamMembershipPolicy.MODERATED)
+ recipe = self.factory.makeOCIRecipe(
+ owner=team, registrant=person,
+ information_type=InformationType.USERDATA)
+ with person_logged_in(team):
+ self.assertTrue(recipe.visibleByUser(person))
+
+ def test_subscribing_changes_visibility(self):
+ person = self.factory.makePerson()
+ owner = self.factory.makePerson()
+ recipe = self.factory.makeOCIRecipe(
+ registrant=owner, owner=owner,
+ information_type=InformationType.USERDATA)
+
+ with person_logged_in(owner):
+ self.assertFalse(recipe.visibleByUser(person))
+ recipe.subscribe(person, recipe.owner)
+ self.assertThat(
+ recipe.getSubscription(person),
+ MatchesStructure(
+ person=Equals(person),
+ ocirecipe=Equals(recipe),
+ subscribed_by=Equals(recipe.owner),
+ date_created=IsInstance(datetime)))
+ # Calling again should be a no-op.
+ recipe.subscribe(person, recipe.owner)
+ self.assertTrue(recipe.visibleByUser(person))
+
+ recipe.unsubscribe(person, recipe.owner)
+ self.assertFalse(recipe.visibleByUser(person))
+ self.assertIsNone(recipe.getSubscription(person))
+
+ def test_owner_can_unsubscribe_anyone(self):
+ person = self.factory.makePerson()
+ owner = self.factory.makePerson()
+ admin = self.factory.makeAdministrator()
+ recipe = self.factory.makeOCIRecipe(
+ registrant=owner, owner=owner,
+ information_type=InformationType.USERDATA)
+ with person_logged_in(admin):
+ recipe.subscribe(person, admin)
+ self.assertTrue(recipe.visibleByUser(person))
+ with person_logged_in(owner):
+ recipe.unsubscribe(person, owner)
+ self.assertFalse(recipe.visibleByUser(person))
+
class TestOCIRecipeProcessors(TestCaseWithFactory):
diff --git a/lib/lp/registry/personmerge.py b/lib/lp/registry/personmerge.py
index a4a79e9..4cefd05 100644
--- a/lib/lp/registry/personmerge.py
+++ b/lib/lp/registry/personmerge.py
@@ -696,18 +696,36 @@ def _mergeOCIRecipe(cur, from_person, to_person):
existing_names = [
r.name for r in getUtility(IOCIRecipeSet).findByOwner(to_person)]
for recipe in oci_recipes:
- new_name = recipe.name
+ naked_recipe = removeSecurityProxy(recipe)
+ new_name = naked_recipe.name
count = 1
while new_name in existing_names:
- new_name = '%s-%s' % (recipe.name, count)
+ new_name = '%s-%s' % (naked_recipe.name, count)
count += 1
- naked_recipe = removeSecurityProxy(recipe)
naked_recipe.owner = to_person
naked_recipe.name = new_name
if not oci_recipes.is_empty():
IStore(oci_recipes[0]).flush()
+def _mergeOCIRecipeSubscription(cur, from_id, to_id):
+ # Update only the OCIRecipeSubscription that will not conflict.
+ cur.execute('''
+ UPDATE OCIRecipeSubscription
+ SET person=%(to_id)d
+ WHERE person=%(from_id)d AND ocirecipe NOT IN
+ (
+ SELECT ocirecipe
+ FROM OCIRecipeSubscription
+ WHERE person = %(to_id)d
+ )
+ ''' % vars())
+ # and delete those left over.
+ cur.execute('''
+ DELETE FROM OCIRecipeSubscription WHERE person=%(from_id)d
+ ''' % vars())
+
+
def _purgeUnmergableTeamArtifacts(from_team, to_team, reviewer):
"""Purge team artifacts that cannot be merged, but can be removed."""
# A team cannot have more than one mailing list.
@@ -941,8 +959,7 @@ def merge_people(from_person, to_person, reviewer, delete=False):
_mergeOCIRecipe(cur, from_person, to_person)
skip.append(('ocirecipe', 'owner'))
- # XXX pappacena 2021-03-05: We need to implement the proper handling for
- # this once we have OCIRecipeSubscription implemented.
+ _mergeOCIRecipeSubscription(cur, from_id, to_id)
skip.append(('ocirecipesubscription', 'person'))
# Sanity check. If we have a reference that participates in a
diff --git a/lib/lp/registry/services/sharingservice.py b/lib/lp/registry/services/sharingservice.py
index 5fc99c8..4b7be99 100644
--- a/lib/lp/registry/services/sharingservice.py
+++ b/lib/lp/registry/services/sharingservice.py
@@ -39,7 +39,10 @@ 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.oci.interfaces.ocirecipe import IOCIRecipe
+from lp.oci.interfaces.ocirecipe import (
+ IOCIRecipe,
+ IOCIRecipeSet,
+ )
from lp.oci.model.ocirecipe import OCIRecipe
from lp.registry.enums import (
BranchSharingPolicy,
@@ -360,6 +363,7 @@ class SharingService:
branch_ids = []
gitrepository_ids = []
snap_ids = []
+ ocirecipes_ids = []
for bug in bugs or []:
if (not ignore_permissions
and not check_permission('launchpad.View', bug)):
@@ -384,6 +388,11 @@ class SharingService:
if (not ignore_permissions
and not check_permission('launchpad.View', spec)):
raise Unauthorized
+ for ocirecipe in ocirecipes or []:
+ if (not ignore_permissions
+ and not check_permission('launchpad.View', ocirecipe)):
+ raise Unauthorized
+ ocirecipes_ids.append(ocirecipe.id)
# Load the bugs.
visible_bugs = []
@@ -424,9 +433,19 @@ class SharingService:
spec for spec in specifications
if spec.id in visible_private_spec_ids or not spec.private]
- return (
- visible_bugs, visible_branches, visible_gitrepositories,
- visible_snaps, visible_specs)
+ # Load the OCI recipes.
+ visible_ocirecipes = []
+ if ocirecipes:
+ visible_ocirecipes = list(getUtility(IOCIRecipeSet).findByIds(
+ snap_ids, visible_by_user=person))
+
+ return {
+ "bugs": visible_bugs,
+ "branches": visible_branches,
+ "gitrepositories": visible_gitrepositories,
+ "snaps": visible_snaps,
+ "specifications": visible_specs,
+ "ocirecipes": visible_ocirecipes}
def getInvisibleArtifacts(self, person, bugs=None, branches=None,
gitrepositories=None):
diff --git a/lib/lp/registry/services/tests/test_sharingservice.py b/lib/lp/registry/services/tests/test_sharingservice.py
index 6f52b89..5dbee9b 100644
--- a/lib/lp/registry/services/tests/test_sharingservice.py
+++ b/lib/lp/registry/services/tests/test_sharingservice.py
@@ -1104,12 +1104,14 @@ class TestSharingService(TestCaseWithFactory, OCIConfigHelperMixin):
# Check that grantees have expected access grants and subscriptions.
for person in [team_grantee, person_grantee]:
- (visible_bugs, visible_branches, visible_gitrepositories,
- visible_snaps, visible_specs) = (
- self.service.getVisibleArtifacts(
+ artifacts = self.service.getVisibleArtifacts(
person, bugs=bugs, branches=branches,
gitrepositories=gitrepositories,
- specifications=specifications))
+ specifications=specifications)
+ visible_bugs = artifacts["bugs"]
+ visible_branches = artifacts["branches"]
+ visible_specs = artifacts["specifications"]
+
self.assertContentEqual(bugs or [], visible_bugs)
self.assertContentEqual(branches or [], visible_branches)
# XXX cjwatson 2015-02-05: check Git repositories when
@@ -1136,11 +1138,14 @@ class TestSharingService(TestCaseWithFactory, OCIConfigHelperMixin):
for person in [team_grantee, person_grantee]:
for bug in bugs or []:
self.assertNotIn(person, bug.getDirectSubscribers())
- (visible_bugs, visible_branches, visible_gitrepositories,
- visible_snaps, visible_specs) = (
- self.service.getVisibleArtifacts(
+ artifacts = self.service.getVisibleArtifacts(
person, bugs=bugs, branches=branches,
- gitrepositories=gitrepositories))
+ gitrepositories=gitrepositories)
+ visible_bugs = artifacts["bugs"]
+ visible_branches = artifacts["branches"]
+ visible_gitrepositories = artifacts["gitrepositories"]
+ visible_specs = artifacts["specifications"]
+
self.assertContentEqual([], visible_bugs)
self.assertContentEqual([], visible_branches)
self.assertContentEqual([], visible_gitrepositories)
@@ -1847,11 +1852,14 @@ class TestSharingService(TestCaseWithFactory, OCIConfigHelperMixin):
grantee, ignore, bugs, branches, gitrepositories, specs = (
self._make_Artifacts())
# Check the results.
- (shared_bugs, shared_branches, shared_gitrepositories,
- shared_snaps, shared_specs) = (
- self.service.getVisibleArtifacts(
+ artifacts = self.service.getVisibleArtifacts(
grantee, bugs=bugs, branches=branches,
- gitrepositories=gitrepositories, specifications=specs))
+ gitrepositories=gitrepositories, specifications=specs)
+ shared_bugs = artifacts["bugs"]
+ shared_branches = artifacts["branches"]
+ shared_gitrepositories = artifacts["gitrepositories"]
+ shared_specs = artifacts["specifications"]
+
self.assertContentEqual(bugs[:5], shared_bugs)
self.assertContentEqual(branches[:5], shared_branches)
self.assertContentEqual(gitrepositories[:5], shared_gitrepositories)
@@ -1862,11 +1870,14 @@ class TestSharingService(TestCaseWithFactory, OCIConfigHelperMixin):
# user has a policy grant for the pillar of the specification.
_, owner, bugs, branches, gitrepositories, specs = (
self._make_Artifacts())
- (shared_bugs, shared_branches, shared_gitrepositories,
- shared_snaps, shared_specs) = (
- self.service.getVisibleArtifacts(
+ artifacts = self.service.getVisibleArtifacts(
owner, bugs=bugs, branches=branches,
- gitrepositories=gitrepositories, specifications=specs))
+ gitrepositories=gitrepositories, specifications=specs)
+ shared_bugs = artifacts["bugs"]
+ shared_branches = artifacts["branches"]
+ shared_gitrepositories = artifacts["gitrepositories"]
+ shared_specs = artifacts["specifications"]
+
self.assertContentEqual(bugs, shared_bugs)
self.assertContentEqual(branches, shared_branches)
self.assertContentEqual(gitrepositories, shared_gitrepositories)
@@ -1906,19 +1917,16 @@ class TestSharingService(TestCaseWithFactory, OCIConfigHelperMixin):
information_type=InformationType.USERDATA)
bugs.append(bug)
- (shared_bugs, shared_branches, shared_gitrepositories,
- visible_snaps, shared_specs) = (
- self.service.getVisibleArtifacts(grantee, bugs=bugs))
+ artifacts = self.service.getVisibleArtifacts(grantee, bugs=bugs)
+ shared_bugs = artifacts["bugs"]
self.assertContentEqual(bugs, shared_bugs)
# Change some bugs.
for x in range(0, 5):
change_callback(bugs[x], owner)
# Check the results.
- (shared_bugs, shared_branches, shared_gitrepositories,
- visible_snaps, shared_specs) = (
- self.service.getVisibleArtifacts(grantee, bugs=bugs))
- self.assertContentEqual(bugs[5:], shared_bugs)
+ artifacts = self.service.getVisibleArtifacts(grantee, bugs=bugs)
+ self.assertContentEqual(bugs[5:], artifacts["bugs"])
def test_getVisibleArtifacts_bug_policy_change(self):
# getVisibleArtifacts excludes bugs after change of information type.
diff --git a/lib/lp/security.py b/lib/lp/security.py
index 35c8b8c..4880698 100644
--- a/lib/lp/security.py
+++ b/lib/lp/security.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2021 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."""
@@ -107,6 +107,7 @@ from lp.oci.interfaces.ocirecipe import (
IOCIRecipeBuildRequest,
)
from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuild
+from lp.oci.interfaces.ocirecipesubscription import IOCIRecipeSubscription
from lp.oci.interfaces.ociregistrycredentials import IOCIRegistryCredentials
from lp.registry.enums import PersonVisibility
from lp.registry.interfaces.announcement import IAnnouncement
@@ -3441,6 +3442,12 @@ class ViewOCIRecipe(AnonymousAuthorization):
"""Anyone can view an `IOCIRecipe`."""
usedfor = IOCIRecipe
+ def checkUnauthenticated(self):
+ return self.obj.visibleByUser(None)
+
+ def checkAuthenticated(self, user):
+ return self.obj.visibleByUser(user.person)
+
class EditOCIRecipe(AuthorizationBase):
permission = 'launchpad.Edit'
@@ -3470,6 +3477,37 @@ class AdminOCIRecipe(AuthorizationBase):
and EditSnap(self.obj).checkAuthenticated(user))
+class OCIRecipeSubscriptionEdit(AuthorizationBase):
+ permission = 'launchpad.Edit'
+ usedfor = IOCIRecipeSubscription
+
+ def checkAuthenticated(self, user):
+ """Is the user able to edit an OCI recipe subscription?
+
+ Any team member can edit a OCI recipe subscription for their
+ team.
+ Launchpad Admins can also edit any OCI recipe subscription.
+ The owner of the subscribed OCI recipe can edit the subscription. If
+ the OCI recipe owner is a team, then members of the team can edit
+ the subscription.
+ """
+ return (user.inTeam(self.obj.ocirecipe.owner) or
+ user.inTeam(self.obj.person) or
+ user.inTeam(self.obj.subscribed_by) or
+ user.in_admin)
+
+
+class SnapSubscriptionView(AuthorizationBase):
+ permission = 'launchpad.View'
+ usedfor = IOCIRecipeSubscription
+
+ def checkUnauthenticated(self):
+ return self.obj.ocirecipe.visibleByUser(None)
+
+ def checkAuthenticated(self, user):
+ return self.obj.ocirecipe.visibleByUser(user.person)
+
+
class ViewOCIRecipeBuild(AnonymousAuthorization):
"""Anyone can view an `IOCIRecipe`."""
usedfor = IOCIRecipeBuild
diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
index be9fcce..be98ba4 100644
--- a/lib/lp/snappy/model/snap.py
+++ b/lib/lp/snappy/model/snap.py
@@ -1188,8 +1188,8 @@ class Snap(Storm, WebhookTargetMixin):
person=person, snap=self, subscribed_by=subscribed_by)
Store.of(subscription).flush()
service = getUtility(IService, "sharing")
- _, _, _, snaps, _ = service.getVisibleArtifacts(
- person, snaps=[self], ignore_permissions=True)
+ snaps = service.getVisibleArtifacts(
+ person, snaps=[self], ignore_permissions=True)["snaps"]
if not snaps:
service.ensureAccessGrants(
[person], subscribed_by, snaps=[self],
Follow ups
