Merge lp:~jpds/launchpad/fix_518232 into lp:launchpad

[Jonathan Davies]

Restricted access to distribution mirror attributes with lp.Admin and lp.Edit accessible interfaces. Landed by henninge.

[Jonathan Davies (jpds)]

= Summary =

Right now, anyone who uses the Launchpad API can access any attribute on IDistributionMirror, regardless if they don't have launchpad.Edit or launchpad.Admin powers.

This branch implements the necessary restrictions to allow people to access mirror data as they normally would be able to via the web UI.

[Henning Eggers (henninge)]

Thanks for making our webservice more secure. ;-)

Please, as discussed on IRC, make the test focus more on the attributes being restricted so that it's clearer what is beingn tested.

review: Approve (code)

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2010-02-04 21:18:32 +0000
+++ lib/lp/registry/configure.zcml	2010-02-10 10:40:30 +0000
@@ -1614,14 +1614,17 @@
 
     <!-- DistributionMirror -->
     <class class="lp.registry.model.distributionmirror.DistributionMirror">
-        <allow interface="lp.registry.interfaces.distributionmirror.IDistributionMirror" />
+        <allow
+            interface="lp.registry.interfaces.distributionmirror.IDistributionMirrorPublic" />
         <require
             permission="launchpad.Edit"
+            interface="lp.registry.interfaces.distributionmirror.IDistributionMirrorEditRestricted"
             set_attributes="name displayname description whiteboard
                             http_base_url ftp_base_url rsync_base_url enabled
                             speed country content official_candidate owner" />
         <require
             permission="launchpad.Admin"
+            interface="lp.registry.interfaces.distributionmirror.IDistributionMirrorAdminRestricted"
             set_attributes="status reviewer date_reviewed"
             attributes="destroySelf" />
     </class>

=== modified file 'lib/lp/registry/interfaces/distributionmirror.py'
--- lib/lp/registry/interfaces/distributionmirror.py	2010-02-04 23:34:10 +0000
+++ lib/lp/registry/interfaces/distributionmirror.py	2010-02-10 10:40:30 +0000
@@ -7,6 +7,9 @@
 
 __all__ = [
 'IDistributionMirror',
+'IDistributionMirrorAdminRestricted',
+'IDistributionMirrorEditRestricted',
+'IDistributionMirrorPublic',
 'IMirrorDistroArchSeries',
 'IMirrorDistroSeriesSource',
 'IMirrorProbeRecord',
@@ -280,20 +283,39 @@
     def getMirrorByURI(self, url):
         return getUtility(IDistributionMirrorSet).getByRsyncUrl(url)
 
-
-class IDistributionMirror(Interface):
-    """A mirror of a given distribution."""
-    export_as_webservice_entry()
+class IDistributionMirrorAdminRestricted(Interface):
+    """IDistributionMirror properties requiring launchpad.Admin permission."""
+
+    reviewer = exported(PublicPersonChoice(
+        title=_('Reviewer'), required=False, readonly=True,
+        vocabulary='ValidPersonOrTeam', description=_(
+            "The person who last reviewed this mirror.")))
+    date_reviewed = exported(Datetime(
+        title=_('Date reviewed'), required=False, readonly=True,
+        description=_(
+            "The date on which this mirror was last reviewed by a mirror admin.")))
+
+
+class IDistributionMirrorEditRestricted(Interface):
+    """IDistributionMirror properties requiring launchpad.Edit permission."""
+    
+    official_candidate = exported(Bool(
+        title=_('Apply to be an official mirror of this distribution'),
+        required=False, readonly=False, default=True))
+    whiteboard = exported(Whiteboard(
+        title=_('Whiteboard'), required=False, readonly=False,
+        description=_("Notes on the current status of the mirror (only "
+                      "visible to admins and the mirror's registrant).")))
+
+
+class IDistributionMirrorPublic(Interface):
+    """Public IDistributionMirror properties."""
 
     id = Int(title=_('The unique id'), required=True, readonly=True)
     owner = exported(PublicPersonChoice(
         title=_('Owner'), readonly=False, vocabulary='ValidOwner',
         required=True, description=_(
             "The person who is set as the current administrator of this mirror.")))
-    reviewer = exported(PublicPersonChoice(
-        title=_('Reviewer'), required=False, readonly=True,
-        vocabulary='ValidPersonOrTeam', description=_(
-            "The person who last reviewed this mirror.")))
     distribution = exported(
         Reference(
             Interface,
@@ -341,9 +363,6 @@
             'this distribution. Choose "Archive" if this is a '
             'mirror of packages for this distribution.'),
         vocabulary=MirrorContent))
-    official_candidate = exported(Bool(
-        title=_('Apply to be an official mirror of this distribution'),
-        required=False, readonly=False, default=True))
     status = exported(Choice(
         title=_('Status'), required=True, readonly=False,
         vocabulary=MirrorStatus,
@@ -364,14 +383,6 @@
     date_created = exported(Datetime(
         title=_('Date Created'), required=True, readonly=True,
         description=_("The date on which this mirror was registered.")))
-    date_reviewed = exported(Datetime(
-        title=_('Date reviewed'), required=False, readonly=True,
-        description=_(
-            "The date on which this mirror was last reviewed by a mirror admin.")))
-    whiteboard = exported(Whiteboard(
-        title=_('Whiteboard'), required=False, readonly=False,
-        description=_("Notes on the current status of the mirror (only "
-                      "visible to admins and the mirror's registrant).")))
 
     @invariant
     def mirrorMustHaveHTTPOrFTPURL(mirror):
@@ -507,6 +518,11 @@
         Sources.gz file refer to and the path to the file itself.
         """
 
+class IDistributionMirror(IDistributionMirrorAdminRestricted,
+        IDistributionMirrorEditRestricted, IDistributionMirrorPublic):
+    """A mirror of a given distribution."""
+    export_as_webservice_entry()
+
 
 class UnableToFetchCDImageFileList(Exception):
     """Couldn't fetch the file list needed for probing cdimage mirrors."""

=== modified file 'lib/lp/registry/stories/webservice/xx-distribution-mirror.txt'
--- lib/lp/registry/stories/webservice/xx-distribution-mirror.txt	2010-02-05 13:48:10 +0000
+++ lib/lp/registry/stories/webservice/xx-distribution-mirror.txt	2010-02-10 10:40:30 +0000
@@ -70,16 +70,71 @@
     >>> login(ANONYMOUS)
     >>> karl_db = getUtility(IPersonSet).getByName('karl')
     >>> test_db = getUtility(IPersonSet).getByName('name12')
+    >>> no_priv_db = getUtility(IPersonSet).getByName('no-priv')
     >>> karl_webservice = webservice_for_person(karl_db,
     ...     permission=OAuthPermission.WRITE_PUBLIC)
     >>> test_webservice = webservice_for_person(test_db,
     ...     permission=OAuthPermission.WRITE_PUBLIC)
+    >>> no_priv_webservice = webservice_for_person(no_priv_db,
+    ...     permission=OAuthPermission.READ_PUBLIC)
     >>> logout()
     >>> karl = webservice.get("/~karl").jsonBody()
     >>> patch = {
     ...     u'owner_link': karl['self_link']
     ... }
 
+One must have special permissions to access certain attributes:
+
+    >>> archive_404_mirror = ubuntu_archive_mirrors['entries'][1]
+    >>> response = no_priv_webservice.get(
+    ...     archive_404_mirror['self_link']).jsonBody()
+    >>> pprint_entry(response)
+    content: ...
+    ...
+    date_reviewed: ...redacted...
+    ...
+    official_candidate: ...redacted...
+    ...
+    reviewer_link: ...redacted...
+    ...
+    whiteboard: ...redacted...
+
+Mirror registrars may see some:
+    
+    >>> response = test_webservice.get(
+    ...     archive_404_mirror['self_link']).jsonBody()
+    >>> pprint_entry(response)
+    content: ...
+    ...
+    date_reviewed: ...redacted...
+    ...
+    reviewer_link: ...redacted...
+
+Mirror listing admins may see all:
+
+    >>> response = karl_webservice.get(
+    ...     archive_404_mirror['self_link']).jsonBody()
+    >>> pprint_entry(response)
+    content: u'Archive'
+    date_created: u'2006-10-16T18:31:43.438573+00:00'
+    date_reviewed: None
+    description: None
+    displayname: None
+    distribution_link: u'http://.../ubuntu'
+    enabled: True
+    ftp_base_url: None
+    http_base_url: u'http://localhost:11375/archive-mirror/'
+    name: u'archive-404-mirror'
+    official_candidate: True
+    owner_link: u'http://.../~name12'
+    resource_type_link: u'http://.../#distribution_mirror'
+    reviewer_link: None
+    rsync_base_url: None
+    self_link: u'http://.../ubuntu/+mirror/archive-404-mirror'
+    speed: u'512 Kbps'
+    status: u'Official'
+    whiteboard: None
+
 Now trying to set the owner using Sample Person's webservice is not authorized.
 
     >>> response = test_webservice.patch(