Merge lp:~thumper/launchpad/vcs-imports-permission-review into lp:launchpad

[Tim Penhey]

Remove old weird permissions held by the vcs-imports team.

In the discussions around having trustworthy community people be members of vcs-imports, we kicked off a security review of the vcs-imports celebrity.

For historical reasons vcs-imports had launchpad.Admin on IBazaarApplication and IProductSeries. Both of these have been removed, as has an old unused menu for IBazaarApplication.

All deletions in this code.

Running the tests through ec2 now.

[Jelmer Vernooij (jelmer)]

It seems like EditCodeImportMachine can also be removed, it's unused as far as I can tell.

review: Approve (code*)

[Jelmer Vernooij (jelmer)]

> It seems like EditCodeImportMachine can also be removed, it's unused as far as
> I can tell.
If it is still used, do we want community members to be able to edit code import machines?

[Tim Penhey (thumper)]

On Wed, 12 May 2010 11:13:26 you wrote:
> > It seems like EditCodeImportMachine can also be removed, it's unused as
> > far as I can tell.
>
> If it is still used, do we want community members to be able to edit code
> import machines?

I was wondering about that. We could change it to be admins and bzr experts
very easily.

Tim

[Michael Nelson (michael.nelson)]

review: Approve

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2010-04-26 15:02:03 +0000
+++ lib/canonical/launchpad/security.py	2010-05-14 02:03:34 +0000
@@ -551,14 +551,6 @@
                 user.in_admin)
 
 
-class AdminSeriesByVCSImports(AuthorizationBase):
-    permission = 'launchpad.Admin'
-    usedfor = IProductSeries
-
-    def checkAuthenticated(self, user):
-        return user.in_vcs_imports
-
-
 class EditProjectMilestoneNever(AuthorizationBase):
     permission = 'launchpad.Edit'
     usedfor = IProjectGroupMilestone
@@ -1101,11 +1093,6 @@
         return user.in_admin or user.in_vcs_imports
 
 
-class AdminTheBazaar(OnlyVcsImportsAndAdmins):
-    permission = 'launchpad.Admin'
-    usedfor = IBazaarApplication
-
-
 class EditCodeImport(OnlyVcsImportsAndAdmins):
     """Control who can edit the object view of a CodeImport.
 
@@ -1136,7 +1123,7 @@
     usedfor = ICodeImportJobWorkflow
 
 
-class EditCodeImportMachine(OnlyVcsImportsAndAdmins):
+class EditCodeImportMachine(OnlyBazaarExpertsAndAdmins):
     """Control who can edit the object view of a CodeImportMachine.
 
     Access is restricted to members of ~vcs-imports and Launchpad admins.

=== modified file 'lib/lp/code/browser/bazaar.py'
--- lib/lp/code/browser/bazaar.py	2010-03-18 21:56:18 +0000
+++ lib/lp/code/browser/bazaar.py	2010-05-14 02:03:34 +0000
@@ -25,24 +25,8 @@
 from lp.code.interfaces.branch import IBranchCloud, IBranchSet
 from lp.code.interfaces.branchcollection import IAllBranches
 from lp.code.interfaces.codeimport import ICodeImportSet
-from canonical.launchpad.interfaces.launchpad import IBazaarApplication
 from lp.registry.interfaces.product import IProductSet
-from canonical.launchpad.webapp import (
-    ApplicationMenu, canonical_url, enabled_with_permission, LaunchpadView,
-    Link)
-
-
-class BazaarBranchesMenu(ApplicationMenu):
-    usedfor = IBazaarApplication
-    facet = 'branches'
-    links = ['importer']
-
-    @enabled_with_permission('launchpad.Admin')
-    def importer(self):
-        target = 'series/'
-        text = 'Branch importer'
-        summary = 'Manage CVS and SVN Trunk Imports'
-        return Link(target, text, summary, icon='branch')
+from canonical.launchpad.webapp import canonical_url, LaunchpadView
 
 
 class BazaarApplicationView(LaunchpadView):

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2010-05-12 17:23:07 +0000
+++ lib/lp/code/browser/configure.zcml	2010-05-14 02:03:34 +0000
@@ -141,10 +141,6 @@
             name="+index"
             template="../templates/bazaar-index.pt"/>
     </browser:pages>
-    <browser:menus
-        classes="
-            BazaarBranchesMenu"
-        module="lp.code.browser.bazaar"/>
     <browser:page
         for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"
         name="+hierarchy"

=== modified file 'lib/lp/code/doc/codeimport-machine.txt'
--- lib/lp/code/doc/codeimport-machine.txt	2009-06-12 16:36:02 +0000
+++ lib/lp/code/doc/codeimport-machine.txt	2010-05-14 02:03:34 +0000
@@ -140,19 +140,17 @@
 The setQuiescing method sets the machine's state to QUIESCING and
 records the corresponding event.  A user is passed into the method
 to be recorded in the event, and will in almost all cases be a
-member of vcs-imports.
+member of the bazaar experts or more likely a LOSA (administrator).
 
-    >>> login('[e-mail]')
+    >>> login('[e-mail]')
     >>> from canonical.launchpad.interfaces import ILaunchpadCelebrities
-    >>> ddaa = getUtility(ILaunchBag).user
-    >>> ddaa.inTeam(getUtility(ILaunchpadCelebrities).vcs_imports)
-    True
+    >>> admin = getUtility(ILaunchBag).user
 
     >>> new_machine.setOnline()
     >>> new_events = NewEvents()
-    >>> new_machine.setQuiescing(ddaa, "1.1.42 rollout")
+    >>> new_machine.setQuiescing(admin, "1.1.42 rollout")
     >>> print new_events.summary()
-    QUIESCE frobisher ddaa
+    QUIESCE frobisher name16
 
     >>> [new_event] = new_events
     >>> dict(new_event.items())[CodeImportEventDataType.MESSAGE]
@@ -210,7 +208,7 @@
 Attempting the transition from OFFLINE to QUIESCING is also logic error.
 
     >>> offline_machine = new_machine_with_state(OFFLINE)
-    >>> offline_machine.setQuiescing(ddaa, "No worky!")
+    >>> offline_machine.setQuiescing(admin, "No worky!")
     Traceback (most recent call last):
     ...
     AssertionError: State of machine ... was OFFLINE.
@@ -219,7 +217,7 @@
 must fail.
 
     >>> online_machine = new_machine_with_state(ONLINE)
-    >>> online_machine.setQuiescing(ddaa, "Because.")
+    >>> online_machine.setQuiescing(admin, "Because.")
     >>> print online_machine.state.name
     QUIESCING
 
@@ -243,7 +241,7 @@
     ONLINE
 
     >>> quiescing_machine = new_machine_with_state(QUIESCING)
-    >>> quiescing_machine.setQuiescing(ddaa, "No worky!")
+    >>> quiescing_machine.setQuiescing(admin, "No worky!")
     Traceback (most recent call last):
     ...
     AssertionError: State of machine ... was QUIESCING.

=== modified file 'lib/lp/code/model/tests/test_codeimportjob.py'
--- lib/lp/code/model/tests/test_codeimportjob.py	2010-03-18 17:49:21 +0000
+++ lib/lp/code/model/tests/test_codeimportjob.py	2010-05-14 02:03:34 +0000
@@ -651,14 +651,12 @@
     def test_wrongJobState(self):
         # Calling startJob with a job whose state is not PENDING is an error.
         code_import = self.factory.makeCodeImport()
-        machine = self.factory.makeCodeImportMachine()
+        machine = self.factory.makeCodeImportMachine(set_online=True)
         job = self.factory.makeCodeImportJob(code_import)
         # ICodeImportJob does not allow setting 'state', so we must
         # use removeSecurityProxy.
         RUNNING = CodeImportJobState.RUNNING
         removeSecurityProxy(job).state = RUNNING
-        # Machines are OFFLINE when they are created.
-        machine.setOnline()
         # Testing startJob failure.
         self.assertFailure(
             "The CodeImportJob associated with %s is "
@@ -709,8 +707,7 @@
     def setUp(self):
         super(TestCodeImportJobWorkflowFinishJob, self).setUp()
         login_for_code_imports()
-        self.machine = self.factory.makeCodeImportMachine()
-        self.machine.setOnline()
+        self.machine = self.factory.makeCodeImportMachine(set_online=True)
 
     def makeRunningJob(self, code_import=None):
         """Make and return a CodeImportJob object with state==RUNNING.

=== modified file 'lib/lp/code/model/tests/test_codeimportmachine.py'
--- lib/lp/code/model/tests/test_codeimportmachine.py	2010-02-19 03:15:04 +0000
+++ lib/lp/code/model/tests/test_codeimportmachine.py	2010-05-14 02:03:34 +0000
@@ -12,8 +12,6 @@
 from canonical.database.constants import UTC_NOW
 from lp.code.enums import (
     CodeImportMachineOfflineReason, CodeImportMachineState)
-from lp.code.model.tests.test_codeimportjob import (
-    login_for_code_imports)
 from lp.code.interfaces.codeimportjob import ICodeImportJobWorkflow
 from lp.testing import TestCaseWithFactory
 from canonical.testing import DatabaseFunctionalLayer
@@ -25,8 +23,8 @@
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
-        super(TestCodeImportMachineShouldLookForJob, self).setUp()
-        login_for_code_imports()
+        super(TestCodeImportMachineShouldLookForJob, self).setUp(
+            '[e-mail]')
         self.machine = self.factory.makeCodeImportMachine(set_online=True)
 
     def createJobRunningOnMachine(self, machine):

=== modified file 'lib/lp/code/stories/codeimport/xx-codeimport-machines.txt'
--- lib/lp/code/stories/codeimport/xx-codeimport-machines.txt	2009-10-06 17:44:23 +0000
+++ lib/lp/code/stories/codeimport/xx-codeimport-machines.txt	2010-05-14 02:03:34 +0000
@@ -25,7 +25,7 @@
 running jobs in the table.
 
     >>> from canonical.launchpad.ftests import login, logout
-    >>> login('[e-mail]')
+    >>> login('[e-mail]')
 
     >>> apollo, odin, saturn = (
     ...     factory.makeCodeImportMachine(set_online=True, hostname='apollo'),
@@ -105,22 +105,20 @@
     >>> print find_tag_by_id(browser.contents, 'update-status')
     None
 
-    >>> import_browser = setupBrowser(
-    ...     auth="Basic [e-mail]:test")
-    >>> import_browser.open(browser.url)
+    >>> admin_browser.open(browser.url)
 
-    >>> print find_tag_by_id(import_browser.contents,
+    >>> print find_tag_by_id(admin_browser.contents,
     ...     'update-status').h2.renderContents()
     Update machine status
 
-    >>> import_browser.getControl('Reason').value = (
+    >>> admin_browser.getControl('Reason').value = (
     ...     "Testing quiescing.")
-    >>> import_browser.getControl('Set Quiescing').click()
-    >>> print_heading(import_browser)
+    >>> admin_browser.getControl('Set Quiescing').click()
+    >>> print_heading(admin_browser)
     apollo: Quiescing
 
-    >>> print_events(import_browser)
+    >>> print_events(admin_browser)
     Related events
-    ...: David Allouche set Quiescing Requested
+    ...: ... set Quiescing Requested
       Message: Testing quiescing.
     ...