launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #28789
[Merge] ~lgp171188/launchpad:split-security.py-final-bits into launchpad:master
Guruprasad has proposed merging ~lgp171188/launchpad:split-security.py-final-bits into launchpad:master.
Commit message:
Split and move all the remaining package-specific security adapters in lp.security
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/426704
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:split-security.py-final-bits into launchpad:master.
diff --git a/doc/how-to/security.rst b/doc/how-to/security.rst
index 19ff373..35bdb89 100644
--- a/doc/how-to/security.rst
+++ b/doc/how-to/security.rst
@@ -40,7 +40,8 @@ interface.
In Launchpad, an authorization policy is expressed through a security adapter.
To define a security adapter for a given permission on an interface:
-1. Define the adapter in ``lib/lp/security.py``. Here's a simple example of
+1. Define the adapter in the corresponding package's `security.py`. Create
+ the file, if it doesn't exist already. Here's a simple example of
an adapter that authorizes only an object owner for the
``launchpad.Edit`` permission on objects that implement the ``IHasOwner``
interface:
@@ -111,3 +112,11 @@ Note that "accessed" means ``getattr()``, while "modified" means
``setattr()``. The process of calling a method starts by using ``getattr()``
to fetch the method from the object, so methods should be declared in
``interface`` or ``attributes`` even if they modify the object.
+
+3. Ensure that there is an ``<authorizations />`` directive in the package's
+ top-level ``configure.zcml` file that specifies the package's security
+ module. If it isn't there already, add one like:
+
+.. code-block:: xml
+
+ <authorizations module=".security" />
diff --git a/lib/lp/answers/security.py b/lib/lp/answers/security.py
index 8332000..c10cc67 100644
--- a/lib/lp/answers/security.py
+++ b/lib/lp/answers/security.py
@@ -15,7 +15,7 @@ from lp.app.security import AnonymousAuthorization, AuthorizationBase
from lp.registry.interfaces.distributionsourcepackage import (
IDistributionSourcePackage,
)
-from lp.security import EditByOwnersOrAdmins
+from lp.registry.security import EditByOwnersOrAdmins
class AdminQuestion(AuthorizationBase):
diff --git a/lib/lp/archivepublisher/configure.zcml b/lib/lp/archivepublisher/configure.zcml
index f52d4ff..2ea8a71 100644
--- a/lib/lp/archivepublisher/configure.zcml
+++ b/lib/lp/archivepublisher/configure.zcml
@@ -10,6 +10,7 @@
xmlns:xmlrpc="http://namespaces.zope.org/xmlrpc";
i18n_domain="launchpad">
+ <authorizations module=".security" />
<!-- ArchiveGPGSigningKey -->
<class class="lp.archivepublisher.archivegpgsigningkey.ArchiveGPGSigningKey">
<allow interface="lp.archivepublisher.interfaces.archivegpgsigningkey.IArchiveGPGSigningKey"/>
diff --git a/lib/lp/archivepublisher/security.py b/lib/lp/archivepublisher/security.py
new file mode 100644
index 0000000..b5be6c1
--- /dev/null
+++ b/lib/lp/archivepublisher/security.py
@@ -0,0 +1,13 @@
+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Security adapters for the archivepublisher package."""
+
+__all__ = []
+
+from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfig
+from lp.security import AdminByAdminsTeam
+
+
+class ViewPublisherConfig(AdminByAdminsTeam):
+ usedfor = IPublisherConfig
diff --git a/lib/lp/blueprints/security.py b/lib/lp/blueprints/security.py
index b4d8da2..c090dbf 100644
--- a/lib/lp/blueprints/security.py
+++ b/lib/lp/blueprints/security.py
@@ -17,7 +17,8 @@ from lp.blueprints.interfaces.specificationsubscription import (
)
from lp.blueprints.interfaces.sprint import ISprint
from lp.blueprints.interfaces.sprintspecification import ISprintSpecification
-from lp.security import EditByOwnersOrAdmins, ModerateByRegistryExpertsOrAdmins
+from lp.registry.security import EditByOwnersOrAdmins
+from lp.security import ModerateByRegistryExpertsOrAdmins
class EditSpecificationBranch(AuthorizationBase):
diff --git a/lib/lp/bugs/security.py b/lib/lp/bugs/security.py
index 0d60eaf..06675de 100644
--- a/lib/lp/bugs/security.py
+++ b/lib/lp/bugs/security.py
@@ -18,6 +18,7 @@ from lp.bugs.interfaces.bugnomination import IBugNomination
from lp.bugs.interfaces.bugsubscription import IBugSubscription
from lp.bugs.interfaces.bugsubscriptionfilter import IBugSubscriptionFilter
from lp.bugs.interfaces.bugsupervisor import IHasBugSupervisor
+from lp.bugs.interfaces.bugtarget import IOfficialBugTagTargetRestricted
from lp.bugs.interfaces.bugtask import IBugTaskDelete
from lp.bugs.interfaces.bugtracker import IBugTracker
from lp.bugs.interfaces.bugwatch import IBugWatch
@@ -383,14 +384,25 @@ class AdminBugWatch(AuthorizationBase):
class EditStructuralSubscription(AuthorizationBase):
- """Edit permissions for `IStructuralSubscription`."""
-
permission = "launchpad.Edit"
usedfor = IStructuralSubscription
def checkAuthenticated(self, user):
- """Subscribers can edit their own structural subscriptions."""
- return user.inTeam(self.obj.subscriber)
+ """Who can edit StructuralSubscriptions."""
+
+ assert self.obj.target
+
+ # Removal of a target cascades removals to StructuralSubscriptions,
+ # so we need to allow editing to those who can edit the target itself.
+ can_edit_target = self.forwardCheckAuthenticated(user, self.obj.target)
+
+ # Who is actually allowed to edit a subscription is determined by
+ # a helper method on the model.
+ can_edit_subscription = self.obj.target.userCanAlterSubscription(
+ self.obj.subscriber, user.person
+ )
+
+ return can_edit_target or can_edit_subscription
class EditBugSubscriptionFilter(AuthorizationBase):
@@ -412,3 +424,17 @@ class EditVulnerability(DelegatedAuthorization):
def __init__(self, obj):
super().__init__(obj, obj.distribution, "launchpad.SecurityAdmin")
+
+
+class BugTargetOwnerOrBugSupervisorOrAdmins(AuthorizationBase):
+ """Product's owner and bug supervisor can set official bug tags."""
+
+ permission = "launchpad.BugSupervisor"
+ usedfor = IOfficialBugTagTargetRestricted
+
+ def checkAuthenticated(self, user):
+ return (
+ user.inTeam(self.obj.bug_supervisor)
+ or user.inTeam(self.obj.owner)
+ or user.in_admin
+ )
diff --git a/lib/lp/buildmaster/security.py b/lib/lp/buildmaster/security.py
index 2c90386..50813e1 100644
--- a/lib/lp/buildmaster/security.py
+++ b/lib/lp/buildmaster/security.py
@@ -4,13 +4,17 @@
"""Security adapters for the buildmaster package."""
__all__ = [
+ "EditPackageBuild",
"ViewBuilder",
"ViewProcessor",
]
from lp.app.security import AnonymousAuthorization
-from lp.buildmaster.interfaces.builder import IBuilder
+from lp.buildmaster.interfaces.builder import IBuilder, IBuilderSet
+from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJob
+from lp.buildmaster.interfaces.packagebuild import IPackageBuild
from lp.buildmaster.interfaces.processor import IProcessor
+from lp.security import AdminByBuilddAdmin
class ViewBuilder(AnonymousAuthorization):
@@ -23,3 +27,47 @@ class ViewProcessor(AnonymousAuthorization):
"""Anyone can view an `IProcessor`."""
usedfor = IProcessor
+
+
+class AdminBuilderSet(AdminByBuilddAdmin):
+ usedfor = IBuilderSet
+
+
+class AdminBuilder(AdminByBuilddAdmin):
+ usedfor = IBuilder
+
+
+class EditBuilder(AdminByBuilddAdmin):
+ permission = "launchpad.Edit"
+ usedfor = IBuilder
+
+
+class ModerateBuilder(EditBuilder):
+ permission = "launchpad.Moderate"
+ usedfor = IBuilder
+
+ def checkAuthenticated(self, user):
+ return user.in_registry_experts or super().checkAuthenticated(user)
+
+
+class AdminBuildRecord(AdminByBuilddAdmin):
+ usedfor = IBuildFarmJob
+
+
+class EditBuildFarmJob(AdminByBuilddAdmin):
+ permission = "launchpad.Edit"
+ usedfor = IBuildFarmJob
+
+
+class EditPackageBuild(EditBuildFarmJob):
+ usedfor = IPackageBuild
+
+ def checkAuthenticated(self, user):
+ """Check if the user has access to edit the archive."""
+ if EditBuildFarmJob.checkAuthenticated(self, user):
+ return True
+
+ # If the user is in the owning team for the archive,
+ # then they have access to edit the builds.
+ # If it's a PPA or a copy archive only allow its owner.
+ return self.obj.archive.owner and user.inTeam(self.obj.archive.owner)
diff --git a/lib/lp/charms/security.py b/lib/lp/charms/security.py
index 9d8dad2..afe78f1 100644
--- a/lib/lp/charms/security.py
+++ b/lib/lp/charms/security.py
@@ -16,7 +16,8 @@ from lp.charms.interfaces.charmrecipe import (
ICharmRecipeBuildRequest,
)
from lp.charms.interfaces.charmrecipebuild import ICharmRecipeBuild
-from lp.security import AdminByBuilddAdmin, EditByRegistryExpertsOrAdmins
+from lp.security import AdminByBuilddAdmin
+from lp.services.webapp.security import EditByRegistryExpertsOrAdmins
class ViewCharmRecipe(AuthorizationBase):
diff --git a/lib/lp/code/configure.zcml b/lib/lp/code/configure.zcml
index 99a5bf0..2f6cb44 100644
--- a/lib/lp/code/configure.zcml
+++ b/lib/lp/code/configure.zcml
@@ -12,7 +12,7 @@
i18n_domain="launchpad">
<include package=".browser"/>
<include package=".vocabularies"/>
- <authorizations module="lp.code.security" />
+ <authorizations module=".security" />
<publisher
name="code"
diff --git a/lib/lp/registry/security.py b/lib/lp/registry/security.py
index e80fa3b..cf6e715 100644
--- a/lib/lp/registry/security.py
+++ b/lib/lp/registry/security.py
@@ -87,7 +87,10 @@ from lp.registry.interfaces.projectgroup import (
IProjectGroup,
IProjectGroupSet,
)
-from lp.registry.interfaces.role import IHasDrivers
+from lp.registry.interfaces.role import (
+ IHasDrivers,
+ IHasOwner,
+ )
from lp.registry.interfaces.sourcepackage import ISourcePackage
from lp.registry.interfaces.ssh import ISSHKey
from lp.registry.interfaces.teammembership import (
@@ -99,14 +102,12 @@ from lp.registry.model.person import Person
from lp.security import (
AdminByAdminsTeam,
AdminByCommercialTeamOrAdmins,
- EditByOwnersOrAdmins,
- EditByRegistryExpertsOrAdmins,
- is_commercial_case,
ModerateByRegistryExpertsOrAdmins,
OnlyRosettaExpertsAndAdmins,
)
from lp.services.database.interfaces import IStore
from lp.services.identity.interfaces.account import IAccount
+from lp.services.webapp.security import EditByRegistryExpertsOrAdmins
from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
@@ -118,6 +119,14 @@ def can_edit_team(team, user):
return team in user.person.getAdministratedTeams()
+class EditByOwnersOrAdmins(AuthorizationBase):
+ permission = 'launchpad.Edit'
+ usedfor = IHasOwner
+
+ def checkAuthenticated(self, user):
+ return user.isOwner(self.obj) or user.in_admin
+
+
class ModerateDistroSeries(ModerateByRegistryExpertsOrAdmins):
usedfor = IDistroSeries
@@ -1256,3 +1265,8 @@ class EditOCIProjectSeries(DelegatedAuthorization):
def __init__(self, obj):
super().__init__(obj, obj.oci_project)
+
+
+def is_commercial_case(obj, user):
+ """Is this a commercial project and the user is a commercial admin?"""
+ return obj.has_current_commercial_subscription and user.in_commercial_admin
diff --git a/lib/lp/security.py b/lib/lp/security.py
index 71c71e8..b827f61 100644
--- a/lib/lp/security.py
+++ b/lib/lp/security.py
@@ -7,11 +7,6 @@ __all__ = [
'AdminByAdminsTeam',
'AdminByBuilddAdmin',
'AdminByCommercialTeamOrAdmins',
- 'BugTargetOwnerOrBugSupervisorOrAdmins',
- 'EditByOwnersOrAdmins',
- 'EditByRegistryExpertsOrAdmins',
- 'EditPackageBuild',
- 'is_commercial_case',
'ModerateByRegistryExpertsOrAdmins',
'OnlyBazaarExpertsAndAdmins',
'OnlyRosettaExpertsAndAdmins',
@@ -27,23 +22,7 @@ import pytz
from zope.interface import Interface
from lp.app.security import AuthorizationBase
-from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfig
-from lp.bugs.interfaces.bugtarget import IOfficialBugTagTargetRestricted
-from lp.bugs.interfaces.structuralsubscription import IStructuralSubscription
-from lp.buildmaster.interfaces.builder import (
- IBuilder,
- IBuilderSet,
- )
-from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJob
-from lp.buildmaster.interfaces.packagebuild import IPackageBuild
-from lp.registry.interfaces.role import IHasOwner
from lp.services.config import config
-from lp.services.webapp.interfaces import ILaunchpadRoot
-
-
-def is_commercial_case(obj, user):
- """Is this a commercial project and the user is a commercial admin?"""
- return obj.has_current_commercial_subscription and user.in_commercial_admin
class ViewByLoggedInUser(AuthorizationBase):
@@ -138,14 +117,6 @@ class AdminByCommercialTeamOrAdmins(AuthorizationBase):
return user.in_commercial_admin or user.in_admin
-class EditByRegistryExpertsOrAdmins(AuthorizationBase):
- permission = 'launchpad.Edit'
- usedfor = ILaunchpadRoot
-
- def checkAuthenticated(self, user):
- return user.in_admin or user.in_registry_experts
-
-
class ModerateByRegistryExpertsOrAdmins(AuthorizationBase):
permission = 'launchpad.Moderate'
usedfor = None
@@ -154,14 +125,6 @@ class ModerateByRegistryExpertsOrAdmins(AuthorizationBase):
return user.in_admin or user.in_registry_experts
-class EditByOwnersOrAdmins(AuthorizationBase):
- permission = 'launchpad.Edit'
- usedfor = IHasOwner
-
- def checkAuthenticated(self, user):
- return user.isOwner(self.obj) or user.in_admin
-
-
class OnlyRosettaExpertsAndAdmins(AuthorizationBase):
"""Base class that allow access to Rosetta experts and Launchpad admins.
"""
@@ -171,40 +134,6 @@ class OnlyRosettaExpertsAndAdmins(AuthorizationBase):
return user.in_admin or user.in_rosetta_experts
-class BugTargetOwnerOrBugSupervisorOrAdmins(AuthorizationBase):
- """Product's owner and bug supervisor can set official bug tags."""
-
- permission = 'launchpad.BugSupervisor'
- usedfor = IOfficialBugTagTargetRestricted
-
- def checkAuthenticated(self, user):
- return (user.inTeam(self.obj.bug_supervisor) or
- user.inTeam(self.obj.owner) or
- user.in_admin)
-
-
-class EditStructuralSubscription(AuthorizationBase):
- permission = 'launchpad.Edit'
- usedfor = IStructuralSubscription
-
- def checkAuthenticated(self, user):
- """Who can edit StructuralSubscriptions."""
-
- assert self.obj.target
-
- # Removal of a target cascades removals to StructuralSubscriptions,
- # so we need to allow editing to those who can edit the target itself.
- can_edit_target = self.forwardCheckAuthenticated(
- user, self.obj.target)
-
- # Who is actually allowed to edit a subscription is determined by
- # a helper method on the model.
- can_edit_subscription = self.obj.target.userCanAlterSubscription(
- self.obj.subscriber, user.person)
-
- return (can_edit_target or can_edit_subscription)
-
-
class OnlyBazaarExpertsAndAdmins(AuthorizationBase):
"""Base class that allows only the Launchpad admins and Bazaar
experts."""
@@ -227,53 +156,3 @@ class AdminByBuilddAdmin(AuthorizationBase):
def checkAuthenticated(self, user):
"""Allow admins and buildd_admins."""
return user.in_buildd_admin or user.in_admin
-
-
-class AdminBuilderSet(AdminByBuilddAdmin):
- usedfor = IBuilderSet
-
-
-class AdminBuilder(AdminByBuilddAdmin):
- usedfor = IBuilder
-
-
-class EditBuilder(AdminByBuilddAdmin):
- permission = 'launchpad.Edit'
- usedfor = IBuilder
-
-
-class ModerateBuilder(EditBuilder):
- permission = 'launchpad.Moderate'
- usedfor = IBuilder
-
- def checkAuthenticated(self, user):
- return (user.in_registry_experts or
- super().checkAuthenticated(user))
-
-
-class AdminBuildRecord(AdminByBuilddAdmin):
- usedfor = IBuildFarmJob
-
-
-class EditBuildFarmJob(AdminByBuilddAdmin):
- permission = 'launchpad.Edit'
- usedfor = IBuildFarmJob
-
-
-class EditPackageBuild(EditBuildFarmJob):
- usedfor = IPackageBuild
-
- def checkAuthenticated(self, user):
- """Check if the user has access to edit the archive."""
- if EditBuildFarmJob.checkAuthenticated(self, user):
- return True
-
- # If the user is in the owning team for the archive,
- # then they have access to edit the builds.
- # If it's a PPA or a copy archive only allow its owner.
- return (self.obj.archive.owner and
- user.inTeam(self.obj.archive.owner))
-
-
-class ViewPublisherConfig(AdminByAdminsTeam):
- usedfor = IPublisherConfig
diff --git a/lib/lp/services/identity/security.py b/lib/lp/services/identity/security.py
index 03caf92..05a1dc1 100644
--- a/lib/lp/services/identity/security.py
+++ b/lib/lp/services/identity/security.py
@@ -6,7 +6,7 @@
__all__ = []
from lp.app.security import AuthorizationBase
-from lp.security import EditByOwnersOrAdmins
+from lp.registry.security import EditByOwnersOrAdmins
from lp.services.identity.interfaces.emailaddress import IEmailAddress
diff --git a/lib/lp/services/webapp/metazcml.py b/lib/lp/services/webapp/metazcml.py
index dcdc374..5824b4e 100644
--- a/lib/lp/services/webapp/metazcml.py
+++ b/lib/lp/services/webapp/metazcml.py
@@ -67,8 +67,13 @@ class IAuthorizationsDirective(Interface):
module = GlobalObject(title='module', required=True)
-def _isAuthorization(module_member):
+def _isAuthorization(module, module_member):
return (type(module_member) is type and
+ # The check below ensures that only the security
+ # adapters belonging to the current module are
+ # selected and the security adapters imported from
+ # elsewhere are ignored.
+ module.__name__ == module_member.__module__ and
IAuthorization.implementedBy(module_member))
@@ -77,8 +82,10 @@ def authorizations(_context, module):
raise TypeError("module attribute must be a module: %s, %s" %
module, type(module))
provides = IAuthorization
- for nameinmodule, authorization in inspect.getmembers(module,
- _isAuthorization):
+ for nameinmodule, authorization in inspect.getmembers(
+ module,
+ lambda member: _isAuthorization(module, member)
+ ):
if (authorization.permission is not None and
authorization.usedfor is not None):
name = authorization.permission
diff --git a/lib/lp/services/webapp/security.py b/lib/lp/services/webapp/security.py
index 9c61116..c494321 100644
--- a/lib/lp/services/webapp/security.py
+++ b/lib/lp/services/webapp/security.py
@@ -3,4 +3,17 @@
"""Security adapters for the lp.services.webapp package."""
-__all__ = []
+__all__ = [
+ "EditByRegistryExpertsOrAdmins",
+]
+
+from lp.app.security import AuthorizationBase
+from lp.services.webapp.interfaces import ILaunchpadRoot
+
+
+class EditByRegistryExpertsOrAdmins(AuthorizationBase):
+ permission = 'launchpad.Edit'
+ usedfor = ILaunchpadRoot
+
+ def checkAuthenticated(self, user):
+ return user.in_admin or user.in_registry_experts
diff --git a/lib/lp/snappy/security.py b/lib/lp/snappy/security.py
index 1c24258..3aa3eaa 100644
--- a/lib/lp/snappy/security.py
+++ b/lib/lp/snappy/security.py
@@ -10,10 +10,8 @@ from lp.app.security import (
AuthorizationBase,
DelegatedAuthorization,
)
-from lp.security import (
- AdminByBuilddAdmin,
- EditByRegistryExpertsOrAdmins,
- )
+from lp.security import AdminByBuilddAdmin
+from lp.services.webapp.security import EditByRegistryExpertsOrAdmins
from lp.snappy.interfaces.snap import (
ISnap,
ISnapBuildRequest,
diff --git a/lib/lp/soyuz/security.py b/lib/lp/soyuz/security.py
index 1cabec3..deb8b35 100644
--- a/lib/lp/soyuz/security.py
+++ b/lib/lp/soyuz/security.py
@@ -19,11 +19,11 @@ from lp.app.security import (
AuthorizationBase,
DelegatedAuthorization,
)
+from lp.buildmaster.security import EditPackageBuild
from lp.security import (
AdminByAdminsTeam,
AdminByBuilddAdmin,
AdminByCommercialTeamOrAdmins,
- EditPackageBuild,
)
from lp.services.database.interfaces import IStore
from lp.soyuz.interfaces.archive import (
