launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26252
[Merge] ~pappacena/launchpad:snap-pillar-accesspolicy into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:snap-pillar-accesspolicy into launchpad:master with ~pappacena/launchpad:snap-pillar-ui as a prerequisite.
Commit message:
Adding Snap as an artifact for sharing services
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/397692
Depends on https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/397459
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:snap-pillar-accesspolicy into launchpad:master.
diff --git a/lib/lp/registry/browser/pillar.py b/lib/lp/registry/browser/pillar.py
index dd83648..52b1554 100644
--- a/lib/lp/registry/browser/pillar.py
+++ b/lib/lp/registry/browser/pillar.py
@@ -439,7 +439,7 @@ class PillarPersonSharingView(LaunchpadView):
def _loadSharedArtifacts(self):
# As a concrete can by linked via more than one policy, we use sets to
# filter out dupes.
- (self.bugtasks, self.branches, self.gitrepositories,
+ (self.bugtasks, self.branches, self.gitrepositories, self.snaps,
self.specifications) = (
self.sharing_service.getSharedArtifacts(
self.pillar, self.person, self.user))
diff --git a/lib/lp/registry/interfaces/accesspolicy.py b/lib/lp/registry/interfaces/accesspolicy.py
index 2c454a7..0e2c8c8 100644
--- a/lib/lp/registry/interfaces/accesspolicy.py
+++ b/lib/lp/registry/interfaces/accesspolicy.py
@@ -36,6 +36,7 @@ class IAccessArtifact(Interface):
bug_id = Attribute("bug_id")
branch_id = Attribute("branch_id")
gitrepository_id = Attribute("gitrepository_id")
+ snap_id = Attribute("snap_id")
specification_id = Attribute("specification_id")
diff --git a/lib/lp/registry/interfaces/sharingservice.py b/lib/lp/registry/interfaces/sharingservice.py
index 386fef4..f6c39fb 100644
--- a/lib/lp/registry/interfaces/sharingservice.py
+++ b/lib/lp/registry/interfaces/sharingservice.py
@@ -1,4 +1,4 @@
-# Copyright 2012-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2012-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Interfaces for sharing service."""
@@ -45,7 +45,7 @@ from lp.registry.interfaces.distribution import IDistribution
from lp.registry.interfaces.person import IPerson
from lp.registry.interfaces.pillar import IPillar
from lp.registry.interfaces.product import IProduct
-
+from lp.snappy.interfaces.snap import ISnap
# XXX 2012-02-24 wallyworld bug 939910
# Need to export for version 'beta' even though we only want to use it in
@@ -177,7 +177,8 @@ class ISharingService(IService):
"""
def getVisibleArtifacts(person, bugs=None, branches=None,
- gitrepositories=None, specifications=None):
+ gitrepositories=None, snaps=None,
+ specifications=None):
"""Return the artifacts shared with person.
Given lists of artifacts, return those a person has access to either
@@ -188,6 +189,7 @@ class ISharingService(IService):
:param branches: the branches to check for which a person has access.
:param gitrepositories: the Git repositories to check for which a
person has access.
+ :param snaps: the snap recipes to check for which a person has access.
:param specifications: the specifications to check for which a
person has access.
:return: a collection of artifacts the person can see.
@@ -328,12 +330,16 @@ class ISharingService(IService):
gitrepositories=List(
Reference(schema=IGitRepository),
title=_('Git repositories'), required=False),
+ snaps=List(
+ Reference(schema=ISnap),
+ title=_('Snap recipes'), required=False),
specifications=List(
Reference(schema=ISpecification), title=_('Specifications'),
required=False))
@operation_for_version('devel')
def revokeAccessGrants(pillar, grantee, user, bugs=None, branches=None,
- gitrepositories=None, specifications=None):
+ gitrepositories=None, snaps=None,
+ specifications=None):
"""Remove a grantee's access to the specified artifacts.
:param pillar: the pillar from which to remove access
@@ -342,6 +348,7 @@ class ISharingService(IService):
:param bugs: the bugs for which to revoke access
:param branches: the branches for which to revoke access
:param gitrepositories: the Git repositories for which to revoke access
+ :param snaps: The snap recipes for which to revoke access
:param specifications: the specifications for which to revoke access
"""
@@ -357,10 +364,15 @@ class ISharingService(IService):
Reference(schema=IBranch), title=_('Branches'), required=False),
gitrepositories=List(
Reference(schema=IGitRepository),
- title=_('Git repositories'), required=False))
+ title=_('Git repositories'), required=False),
+ snaps=List(
+ Reference(schema=ISnap),
+ title=_('Snap recipes'), required=False)
+ )
@operation_for_version('devel')
def ensureAccessGrants(grantees, user, bugs=None, branches=None,
- gitrepositories=None, specifications=None):
+ gitrepositories=None, snaps=None,
+ specifications=None):
"""Ensure a grantee has an access grant to the specified artifacts.
:param grantees: the people or teams for whom to grant access
@@ -368,6 +380,7 @@ class ISharingService(IService):
:param bugs: the bugs for which to grant access
:param branches: the branches for which to grant access
:param gitrepositories: the Git repositories for which to grant access
+ :param snaps: the snap recipes for which to grant access
:param specifications: the specifications for which to grant access
"""
diff --git a/lib/lp/registry/model/accesspolicy.py b/lib/lp/registry/model/accesspolicy.py
index 2e8fddf..472509f 100644
--- a/lib/lp/registry/model/accesspolicy.py
+++ b/lib/lp/registry/model/accesspolicy.py
@@ -1,4 +1,4 @@
-# Copyright 2011-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Model classes for pillar and artifact access policies."""
@@ -98,6 +98,8 @@ class AccessArtifact(StormBase):
branch = Reference(branch_id, 'Branch.id')
gitrepository_id = Int(name='gitrepository')
gitrepository = Reference(gitrepository_id, 'GitRepository.id')
+ snap_id = Int(name="snap")
+ snap = Reference(snap_id, 'Snap.id')
specification_id = Int(name='specification')
specification = Reference(specification_id, 'Specification.id')
@@ -114,12 +116,15 @@ class AccessArtifact(StormBase):
from lp.bugs.interfaces.bug import IBug
from lp.code.interfaces.branch import IBranch
from lp.code.interfaces.gitrepository import IGitRepository
+ from lp.snappy.interfaces.snap import ISnap
if IBug.providedBy(concrete_artifact):
col = cls.bug
elif IBranch.providedBy(concrete_artifact):
col = cls.branch
elif IGitRepository.providedBy(concrete_artifact):
col = cls.gitrepository
+ elif ISnap.providedBy(concrete_artifact):
+ col = cls.snap
elif ISpecification.providedBy(concrete_artifact):
col = cls.specification
else:
@@ -143,6 +148,7 @@ class AccessArtifact(StormBase):
from lp.bugs.interfaces.bug import IBug
from lp.code.interfaces.branch import IBranch
from lp.code.interfaces.gitrepository import IGitRepository
+ from lp.snappy.interfaces.snap import ISnap
existing = list(cls.find(concrete_artifacts))
if len(existing) == len(concrete_artifacts):
@@ -156,18 +162,20 @@ class AccessArtifact(StormBase):
insert_values = []
for concrete in needed:
if IBug.providedBy(concrete):
- insert_values.append((concrete, None, None, None))
+ insert_values.append((concrete, None, None, None, None))
elif IBranch.providedBy(concrete):
- insert_values.append((None, concrete, None, None))
+ insert_values.append((None, concrete, None, None, None))
elif IGitRepository.providedBy(concrete):
- insert_values.append((None, None, concrete, None))
+ insert_values.append((None, None, concrete, None, None))
elif ISpecification.providedBy(concrete):
- insert_values.append((None, None, None, concrete))
+ insert_values.append((None, None, None, concrete, None))
+ elif ISnap.providedBy(concrete):
+ insert_values.append((None, None, None, None, concrete))
else:
raise ValueError("%r is not a supported artifact" % concrete)
- new = create(
- (cls.bug, cls.branch, cls.gitrepository, cls.specification),
- insert_values, get_objects=True)
+ columns = (cls.bug, cls.branch, cls.gitrepository, cls.specification,
+ cls.snap)
+ new = create(columns, insert_values, get_objects=True)
return list(existing) + new
@classmethod
diff --git a/lib/lp/registry/services/sharingservice.py b/lib/lp/registry/services/sharingservice.py
index 09744ee..01e90da 100644
--- a/lib/lp/registry/services/sharingservice.py
+++ b/lib/lp/registry/services/sharingservice.py
@@ -1,4 +1,4 @@
-# Copyright 2012-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2012-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Classes for pillar and artifact sharing service."""
@@ -81,6 +81,10 @@ from lp.services.webapp.authorization import (
available_with_permission,
check_permission,
)
+from lp.snappy.interfaces.snap import (
+ ISnap,
+ ISnapSet,
+ )
@implementer(ISharingService)
@@ -197,11 +201,12 @@ class SharingService:
@available_with_permission('launchpad.Driver', 'pillar')
def getSharedArtifacts(self, pillar, person, user, include_bugs=True,
include_branches=True, include_gitrepositories=True,
- include_specifications=True):
+ include_snaps=True, include_specifications=True):
"""See `ISharingService`."""
bug_ids = set()
branch_ids = set()
gitrepository_ids = set()
+ snap_ids = set()
specification_ids = set()
for artifact in self.getArtifactGrantsForPersonOnPillar(
pillar, person):
@@ -211,6 +216,8 @@ class SharingService:
branch_ids.add(artifact.branch_id)
elif artifact.gitrepository_id and include_gitrepositories:
gitrepository_ids.add(artifact.gitrepository_id)
+ elif artifact.snap_id and include_snaps:
+ snap_ids.add(artifact.snap_id)
elif artifact.specification_id and include_specifications:
specification_ids.add(artifact.specification_id)
@@ -234,16 +241,20 @@ class SharingService:
wanted_gitrepositories = all_gitrepositories.visibleByUser(
user).withIds(*gitrepository_ids)
gitrepositories = list(wanted_gitrepositories.getRepositories())
+ snaps = []
+ if snap_ids:
+ all_snaps = getUtility(ISnapSet)
+ snaps = all_snaps.findByIds(snap_ids)
specifications = []
if specification_ids:
specifications = load(Specification, specification_ids)
- return bugtasks, branches, gitrepositories, specifications
+ return bugtasks, branches, gitrepositories, snaps, specifications
@available_with_permission('launchpad.Driver', 'pillar')
def getSharedBugs(self, pillar, person, user):
"""See `ISharingService`."""
- bugtasks, _, _, _ = self.getSharedArtifacts(
+ bugtasks, _, _, _, _ = self.getSharedArtifacts(
pillar, person, user, include_branches=False,
include_gitrepositories=False, include_specifications=False)
return bugtasks
@@ -251,7 +262,7 @@ class SharingService:
@available_with_permission('launchpad.Driver', 'pillar')
def getSharedBranches(self, pillar, person, user):
"""See `ISharingService`."""
- _, branches, _, _ = self.getSharedArtifacts(
+ _, branches, _, _, _ = self.getSharedArtifacts(
pillar, person, user, include_bugs=False,
include_gitrepositories=False, include_specifications=False)
return branches
@@ -259,15 +270,23 @@ class SharingService:
@available_with_permission('launchpad.Driver', 'pillar')
def getSharedGitRepositories(self, pillar, person, user):
"""See `ISharingService`."""
- _, _, gitrepositories, _ = self.getSharedArtifacts(
+ _, _, gitrepositories, _, _ = self.getSharedArtifacts(
pillar, person, user, include_bugs=False, include_branches=False,
include_specifications=False)
return gitrepositories
@available_with_permission('launchpad.Driver', 'pillar')
+ def getSharedSnaps(self, pillar, person, user):
+ """See `ISharingService`."""
+ _, _, _, snaps, _ = self.getSharedArtifacts(
+ pillar, person, user, include_bugs=False, include_branches=False,
+ include_gitrepositories=False)
+ return snaps
+
+ @available_with_permission('launchpad.Driver', 'pillar')
def getSharedSpecifications(self, pillar, person, user):
"""See `ISharingService`."""
- _, _, _, specifications = self.getSharedArtifacts(
+ _, _, _, _, specifications = self.getSharedArtifacts(
pillar, person, user, include_bugs=False, include_branches=False,
include_gitrepositories=False)
return specifications
@@ -307,8 +326,8 @@ class SharingService:
In(Specification.id, spec_ids)))
def getVisibleArtifacts(self, person, bugs=None, branches=None,
- gitrepositories=None, specifications=None,
- ignore_permissions=False):
+ gitrepositories=None, snaps=None,
+ specifications=None, ignore_permissions=False):
"""See `ISharingService`."""
bug_ids = []
branch_ids = []
@@ -752,11 +771,11 @@ class SharingService:
@available_with_permission('launchpad.Edit', 'pillar')
def revokeAccessGrants(self, pillar, grantee, user, bugs=None,
- branches=None, gitrepositories=None,
+ branches=None, gitrepositories=None, snaps=None,
specifications=None):
"""See `ISharingService`."""
- if (not bugs and not branches and not gitrepositories and
+ if (not bugs and not branches and not gitrepositories and not snaps and
not specifications):
raise ValueError(
"Either bugs, branches, gitrepositories, or specifications "
@@ -769,6 +788,8 @@ class SharingService:
artifacts.extend(branches)
if gitrepositories:
artifacts.extend(gitrepositories)
+ if snaps:
+ artifacts.extend(snaps)
if specifications:
artifacts.extend(specifications)
# Find the access artifacts associated with the bugs, branches, Git
@@ -779,14 +800,20 @@ class SharingService:
getUtility(IAccessArtifactGrantSource).revokeByArtifact(
artifacts_to_delete, [grantee])
+ # XXX: Pappacena 2021-02-05: snaps should not trigger this job,
+ # since we do not have a "SnapSubscription" yet.
+ artifacts = [i for i in artifacts if not ISnap.providedBy(i)]
+ if not artifacts:
+ return
+
# Create a job to remove subscriptions for artifacts the grantee can no
# longer see.
return getUtility(IRemoveArtifactSubscriptionsJobSource).create(
user, artifacts, grantee=grantee, pillar=pillar)
def ensureAccessGrants(self, grantees, user, bugs=None, branches=None,
- gitrepositories=None, specifications=None,
- ignore_permissions=False):
+ gitrepositories=None, snaps=None,
+ specifications=None, ignore_permissions=False):
"""See `ISharingService`."""
artifacts = []
@@ -796,6 +823,8 @@ class SharingService:
artifacts.extend(branches)
if gitrepositories:
artifacts.extend(gitrepositories)
+ if snaps:
+ artifacts.extend(snaps)
if specifications:
artifacts.extend(specifications)
if not ignore_permissions:
diff --git a/lib/lp/registry/services/tests/test_sharingservice.py b/lib/lp/registry/services/tests/test_sharingservice.py
index 50ce8c6..9d0e07c 100644
--- a/lib/lp/registry/services/tests/test_sharingservice.py
+++ b/lib/lp/registry/services/tests/test_sharingservice.py
@@ -1459,7 +1459,7 @@ class TestSharingService(TestCaseWithFactory):
# Check the results.
(shared_bugtasks, shared_branches, shared_gitrepositories,
- shared_specs) = (
+ shared_snaps, shared_specs) = (
self.service.getSharedArtifacts(product, grantee, user))
self.assertContentEqual(bug_tasks[:9], shared_bugtasks)
self.assertContentEqual(branches[:9], shared_branches)
diff --git a/lib/lp/snappy/interfaces/snap.py b/lib/lp/snappy/interfaces/snap.py
index 8aa065c..890b6f3 100644
--- a/lib/lp/snappy/interfaces/snap.py
+++ b/lib/lp/snappy/interfaces/snap.py
@@ -859,6 +859,12 @@ class ISnapAdminAttributes(Interface):
"Allow access to external network resources via a proxy. "
"Resources hosted on Launchpad itself are always allowed.")))
+ def subscribe(person, subscribed_by):
+ """Subscribe a person to this snap recipe."""
+
+ def unsubscribe(person, unsubscribed_by):
+ """Unsubscribe a person to this snap recipe."""
+
# XXX cjwatson 2015-07-17 bug=760849: "beta" is a lie to get WADL
# generation working. Individual attributes must set their version to
@@ -902,6 +908,9 @@ class ISnapSet(Interface):
def isValidPrivacy(private, owner, branch=None, git_ref=None):
"""Whether or not the privacy context is valid."""
+ def findByIds(snap_ids):
+ """Return all snap packages with the given ids."""
+
@operation_parameters(
owner=Reference(IPerson, title=_("Owner"), required=True),
name=TextLine(title=_("Snap name"), required=True))
diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
index fea5f29..a23016e 100644
--- a/lib/lp/snappy/model/snap.py
+++ b/lib/lp/snappy/model/snap.py
@@ -24,11 +24,14 @@ import six
from six.moves.urllib.parse import urlsplit
from storm.expr import (
And,
+ Coalesce,
Desc,
+ Join,
LeftJoin,
Not,
Or,
Select,
+ SQL,
)
from storm.locals import (
Bool,
@@ -59,6 +62,7 @@ from lp.app.browser.tales import (
)
from lp.app.errors import IncompatibleArguments
from lp.app.interfaces.security import IAuthorization
+from lp.app.interfaces.services import IService
from lp.buildmaster.enums import BuildStatus
from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
from lp.buildmaster.model.builder import Builder
@@ -105,6 +109,7 @@ from lp.registry.interfaces.role import (
IHasOwner,
IPersonRoles,
)
+from lp.registry.model.accesspolicy import AccessPolicyGrant
from lp.registry.model.distroseries import DistroSeries
from lp.registry.model.series import ACTIVE_STATUSES
from lp.registry.model.teammembership import TeamParticipation
@@ -123,6 +128,9 @@ from lp.services.database.interfaces import (
IStore,
)
from lp.services.database.stormexpr import (
+ Array,
+ ArrayAgg,
+ ArrayIntersects,
Greatest,
IsDistinctFrom,
NullsLast,
@@ -1052,6 +1060,19 @@ class Snap(Storm, WebhookTargetMixin):
order_by = Desc(SnapBuild.id)
return self._getBuilds(filter_term, order_by)
+ def subscribe(self, person, subscribed_by):
+ """See `ISnap`."""
+ # XXX pappacena 2021-02-05: We may need a "SnapSubscription" here.
+ service = getUtility(IService, "sharing")
+ service.ensureAccessGrants([person], subscribed_by, snaps=[self])
+
+ def unsubscribe(self, person, unsubscribed_by):
+ """See `ISnap`."""
+ service = getUtility(IService, "sharing")
+ service.revokeAccessGrants(
+ self.pillar, person, unsubscribed_by, snaps=[self])
+ IStore(self).flush()
+
def destroySelf(self):
"""See `ISnap`."""
store = IStore(Snap)
@@ -1227,6 +1248,10 @@ class SnapSet:
expressions.append(Snap.owner == owner)
return IStore(Snap).find(Snap, *expressions)
+ def findByIds(self, snap_ids):
+ """See `ISnapSet`."""
+ return IStore(ISnap).find(Snap, Snap.id.is_in(snap_ids))
+
def findByOwner(self, owner):
"""See `ISnapSet`."""
return IStore(Snap).find(Snap, Snap.owner == owner)
@@ -1311,7 +1336,8 @@ class SnapSet:
Snap.private == False,
Snap.owner_id.is_in(Select(
TeamParticipation.teamID,
- TeamParticipation.person == visible_by_user)))
+ TeamParticipation.person == visible_by_user)),
+ *get_private_snap_subscriber_filter(visible_by_user))
def findByURL(self, url, owner=None, visible_by_user=None):
"""See `ISnapSet`."""
@@ -1508,3 +1534,31 @@ class SnapStoreSecretsEncryptedContainer(NaClEncryptedContainerBase):
config.snappy.store_secrets_private_key.encode("UTF-8"))
else:
return None
+
+
+def get_private_snap_subscriber_filter(user):
+ """Returns the filter for all private Snaps that the given user is
+ subscribed to (that is, has access without being directly an owner).
+ """
+ artifact_grant_query = Coalesce(
+ ArrayIntersects(
+ SQL("%s.access_grants" % Snap.__storm_table__),
+ Select(
+ ArrayAgg(TeamParticipation.teamID),
+ tables=TeamParticipation,
+ where=(TeamParticipation.person == user)
+ )), False)
+
+ policy_grant_query = Coalesce(
+ ArrayIntersects(
+ Array(SQL("%s.access_policy" % Snap.__storm_table__)),
+ Select(
+ ArrayAgg(AccessPolicyGrant.policy_id),
+ tables=(AccessPolicyGrant,
+ Join(TeamParticipation,
+ TeamParticipation.teamID ==
+ AccessPolicyGrant.grantee_id)),
+ where=(TeamParticipation.person == user)
+ )), False)
+
+ return [Or(artifact_grant_query, policy_grant_query)]
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index bbc8be1..e21efa1 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -1519,6 +1519,42 @@ class TestSnapSet(TestCaseWithFactory):
self.assertContentEqual(snaps[:3], snap_set.findByPerson(owners[0]))
self.assertContentEqual(snaps[3:], snap_set.findByPerson(owners[1]))
+ def test_findSnapVisibilityClause_includes_grants(self):
+ grantee, creator = [self.factory.makePerson() for i in range(2)]
+ # All snaps are owned by "creator", and "grantee" will later have
+ # access granted using sharing service.
+ snap_data = dict(registrant=creator, owner=creator, private=True)
+ private_snaps = [self.factory.makeSnap(**snap_data) for _ in range(2)]
+ shared_snaps = [self.factory.makeSnap(**snap_data) for _ in range(2)]
+ snap_data["private"] = False
+ public_snaps = [self.factory.makeSnap(**snap_data) for _ in range(3)]
+
+ with admin_logged_in():
+ for snap in shared_snaps:
+ snap.subscribe(grantee, creator)
+
+ snap_set = getUtility(ISnapSet)
+
+ def all_snaps_visible_by(person):
+ snaps = removeSecurityProxy(snap_set)
+ return IStore(Snap).find(
+ Snap, snaps._findSnapVisibilityClause(person))
+
+ # Creator should get all snaps.
+ self.assertContentEqual(
+ public_snaps + private_snaps + shared_snaps,
+ all_snaps_visible_by(creator))
+
+ # Grantee should get public and shared snaps.
+ self.assertContentEqual(
+ public_snaps + shared_snaps, all_snaps_visible_by(grantee))
+
+ with admin_logged_in():
+ # After revoking, Grantee should have no access to the shared ones.
+ for snap in shared_snaps:
+ snap.unsubscribe(grantee, creator)
+ self.assertContentEqual(public_snaps, all_snaps_visible_by(grantee))
+
def test_findByProject(self):
# ISnapSet.findByProject returns all Snaps based on branches or
# repositories for the given project.
Follow ups
