Merge lp:~abentley/launchpad/recipe-interfaces into lp:launchpad

[Aaron Bentley]

Update security on SourcePackageRecipe to use multiple-interface style.

= Summary =
Update the security configuration to use multiple interfaces.

== Proposed fix ==
Update the security configuration to use multiple interfaces.

== Pre-implementation notes ==
Discussed with rockstar

== Implementation details == For reasons I don't understand, the builder_recipe
and date_last_modified attributes weren't accessible (Forbidden, not
Unauthorized), despite being on the ISourcePackageRecipeEditableAttributes
interface. I had to provide them manually.

== Tests ==
bin/test -v test_sourcepackagerecipe

== Demo and Q/A ==
None

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/configure.zcml
  lib/lp/code/model/tests/test_sourcepackagerecipe.py
  lib/lp/code/interfaces/sourcepackagerecipe.py

./lib/lp/code/model/tests/test_sourcepackagerecipe.py
     280: E231 missing whitespace after ','
     304: E231 missing whitespace after ','
     312: E231 missing whitespace after ','
     319: E231 missing whitespace after ','
     327: E231 missing whitespace after ','
     365: E231 missing whitespace after ','
     425: E231 missing whitespace after ','
     506: E301 expected 1 blank line, found 0
     727: Line exceeds 78 characters.
./lib/lp/code/interfaces/sourcepackagerecipe.py
      77: E231 missing whitespace after ','
     121: E202 whitespace before ')'
     121: E231 missing whitespace after ','
     150: E302 expected 2 blank lines, found 1

[Paul Hummer (rockstar)]

review: Approve

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2010-08-27 02:11:36 +0000
+++ lib/lp/code/configure.zcml	2010-09-02 17:47:47 +0000
@@ -960,28 +960,23 @@
   <!-- SourcePackageRecipeData -->
   <class
      class="lp.code.model.sourcepackagerecipedata.SourcePackageRecipeData">
-    <allow
+    <require permission="launchpad.View"
     interface="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipeData"/>
   </class>
   <!-- SourcePackageRecipe -->
   <class
      class="lp.code.model.sourcepackagerecipe.SourcePackageRecipe">
     <require permission="launchpad.View"
-      interface="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipe"
+      interface="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipeView
+                 lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipeEditableAttributes
+                 lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipeData"
       />
     <require
        permission="launchpad.Edit"
-       set_attributes="
-           build_daily
-           builder_recipe
-           daily_build_archive
-           date_last_modified
-           description
-           distroseries
-           name
-           owner
-           sourcepackagename
-           is_stale"/>
+       interface="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipeEdit"
+       set_schema="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipeEditableAttributes"
+       set_attributes="builder_recipe date_last_modified"
+        />
   </class>
   <class
      class="bzrlib.plugins.builder.recipe.BaseRecipeBranch">

=== modified file 'lib/lp/code/interfaces/sourcepackagerecipe.py'
--- lib/lp/code/interfaces/sourcepackagerecipe.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/interfaces/sourcepackagerecipe.py	2010-09-02 17:47:47 +0000
@@ -80,21 +80,12 @@
         """An iterator of the branches referenced by this recipe."""
 
 
-class ISourcePackageRecipe(IHasOwner, ISourcePackageRecipeData):
-    """An ISourcePackageRecipe describes how to build a source package.
-
-    More precisely, it describes how to combine a number of branches into a
-    debianized source tree.
-    """
-    export_as_webservice_entry()
+class ISourcePackageRecipeView(Interface):
+    """IBranch attributes that require launchpad.View permission."""
 
     id = Int()
 
-    daily_build_archive = exported(Reference(
-        IArchive, title=_("The archive to use for daily builds.")))
-
     date_created = Datetime(required=True, readonly=True)
-    date_last_modified = Datetime(required=True, readonly=True)
 
     registrant = exported(
         PublicPersonChoice(
@@ -102,42 +93,8 @@
             required=True, readonly=True,
             vocabulary='ValidPersonOrTeam'))
 
-    owner = exported(
-        PersonChoice(
-            title=_('Owner'),
-            required=True, readonly=False,
-            vocabulary='UserTeamsParticipationPlusSelf',
-            description=_("The person or team who can edit this recipe.")))
-
-    distroseries = CollectionField(
-        Reference(IDistroSeries), title=_("The distroseries this recipe will"
-            " build a source package for"),
-        readonly=False)
-    build_daily = exported(Bool(
-        title=_("Build daily")))
     is_stale = Bool(title=_('Recipe is stale.'))
 
-    name = exported(TextLine(
-            title=_("Name"), required=True,
-            constraint=name_validator,
-            description=_("The name of this recipe.")))
-
-    description = Text(
-        title=_('Description'), required=True,
-        description=_('A short description of the recipe.'))
-
-    builder_recipe = Attribute(
-        _("The bzr-builder data structure for the recipe."))
-
-    base_branch = Reference(
-        IBranch, title=_("The base branch used by this recipe."),
-        required=True, readonly=True)
-
-    @operation_parameters(recipe_text=Text())
-    @export_write_operation()
-    def setRecipeText(recipe_text):
-        """Set the text of the recipe."""
-
     recipe_text = exported(Text())
 
     def isOverQuota(requester, distroseries):
@@ -147,6 +104,16 @@
         :param distroseries: The distroseries to build for.
         """
 
+    def getBuilds(pending=False):
+        """Return a ResultSet of all the builds in the given state.
+
+        :param pending: If True, select all builds that are pending.  If
+            False, select all builds that are not pending.
+        """
+
+    def getLastBuild():
+        """Return the the most recent build of this recipe."""
+
     @call_with(requester=REQUEST_USER)
     @operation_parameters(
         archive=Reference(schema=IArchive),
@@ -164,15 +131,14 @@
             able to upload to the archive.
         """
 
-    def getBuilds(pending=False):
-        """Return a ResultSet of all the builds in the given state.
-
-        :param pending: If True, select all builds that are pending.  If
-            False, select all builds that are not pending.
-        """
-
-    def getLastBuild():
-        """Return the the most recent build of this recipe."""
+
+class ISourcePackageRecipeEdit(Interface):
+    """ISourcePackageRecipe methods that require launchpad.Edit permission."""
+
+    @operation_parameters(recipe_text=Text())
+    @export_write_operation()
+    def setRecipeText(recipe_text):
+        """Set the text of the recipe."""
 
     def destroySelf():
         """Remove this SourcePackageRecipe from the database.
@@ -181,6 +147,56 @@
         references to this object.
         """
 
+class ISourcePackageRecipeEditableAttributes(IHasOwner):
+    """ISourcePackageRecipe attributes that can be edited.
+
+    These attributes need launchpad.View to see, and launchpad.Edit to change.
+    """
+    daily_build_archive = exported(Reference(
+        IArchive, title=_("The archive to use for daily builds.")))
+
+    builder_recipe = Attribute(
+        _("The bzr-builder data structure for the recipe."))
+
+    owner = exported(
+        PersonChoice(
+            title=_('Owner'),
+            required=True, readonly=False,
+            vocabulary='UserTeamsParticipationPlusSelf',
+            description=_("The person or team who can edit this recipe.")))
+
+    distroseries = CollectionField(
+        Reference(IDistroSeries), title=_("The distroseries this recipe will"
+            " build a source package for"),
+        readonly=False)
+    build_daily = exported(Bool(
+        title=_("Build daily")))
+
+    name = exported(TextLine(
+            title=_("Name"), required=True,
+            constraint=name_validator,
+            description=_("The name of this recipe.")))
+
+    description = Text(
+        title=_('Description'), required=True,
+        description=_('A short description of the recipe.'))
+
+    date_last_modified = Datetime(required=True, readonly=True)
+
+
+class ISourcePackageRecipe(ISourcePackageRecipeData,
+    ISourcePackageRecipeEdit, ISourcePackageRecipeEditableAttributes,
+    ISourcePackageRecipeView):
+    """An ISourcePackageRecipe describes how to build a source package.
+
+    More precisely, it describes how to combine a number of branches into a
+    debianized source tree.
+    """
+    export_as_webservice_entry()
+    base_branch = Reference(
+        IBranch, title=_("The base branch used by this recipe."),
+        required=True, readonly=True)
+
 
 class ISourcePackageRecipeSource(Interface):
     """A utility of this interface can be used to create and access recipes.

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py	2010-09-01 03:25:36 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py	2010-09-02 17:47:47 +0000
@@ -177,7 +177,8 @@
         recipe = self.makeSourcePackageRecipeFromBuilderRecipe(
             self.factory.makeRecipe())
         transaction.commit()
-        self.assertProvides(recipe, ISourcePackageRecipe)
+        with person_logged_in(recipe.owner):
+            self.assertProvides(recipe, ISourcePackageRecipe)
 
     def test_base_branch(self):
         # When a recipe is created, we can access its base branch.
@@ -484,7 +485,8 @@
         self.factory.makeSourcePackageRecipeBuildJob(
             recipe_build=past_build)
         removeSecurityProxy(past_build).datebuilt = datetime.now(UTC)
-        recipe.destroySelf()
+        with person_logged_in(recipe.owner):
+            recipe.destroySelf()
         # Show no database constraints were violated
         Store.of(recipe).flush()