launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/soft-oops-on-private-team-disclosure into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/soft-oops-on-private-team-disclosure/+merge/79066

Warn (via an informational OOPS) when mixed visibility is encountered. This allows the Teal Squad to investigate the scope of the issue at hand, and also allows us to see which teams and pillars are implicated.
-- 
https://code.launchpad.net/~stevenk/launchpad/soft-oops-on-private-team-disclosure/+merge/79066
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/soft-oops-on-private-team-disclosure into lp:launchpad.

=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py	2011-09-26 20:56:58 +0000
+++ lib/lp/app/browser/tales.py	2011-10-13 04:48:02 +0000
@@ -1,14 +1,16 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=C0103,W0613,R0911,F0401
-#
 """Implementation of the lp: htmlform: fmt: namespaces in TALES."""
 
 __metaclass__ = type
 
 import bisect
 import cgi
+from datetime import (
+    datetime,
+    timedelta,
+    )
 from email.Utils import formatdate
 import math
 import os.path
@@ -17,14 +19,22 @@
 from textwrap import dedent
 import urllib
 
-##import warnings
-
-from datetime import datetime, timedelta
 from lazr.enum import enumerated_type_registry
 from lazr.uri import URI
-
-from zope.interface import Interface, Attribute, implements
-from zope.component import adapts, getUtility, queryAdapter, getMultiAdapter
+import pytz
+from z3c.ptcompat import ViewPageTemplateFile
+from zope.error.interfaces import IErrorReportingUtility
+from zope.interface import (
+    Attribute,
+    Interface,
+    implements,
+    )
+from zope.component import (
+    adapts,
+    getUtility,
+    queryAdapter,
+    getMultiAdapter,
+    )
 from zope.app import zapi
 from zope.publisher.browser import BrowserView
 from zope.traversing.interfaces import (
@@ -36,23 +46,36 @@
 from zope.security.proxy import isinstance as zope_isinstance
 from zope.schema import TextLine
 
-import pytz
-from z3c.ptcompat import ViewPageTemplateFile
-
 from canonical.launchpad import _
 from canonical.launchpad.interfaces.launchpad import (
-    IHasIcon, IHasLogo, IHasMugshot, IPrivacy)
+    IHasIcon,
+    IHasLogo,
+    IHasMugshot,
+    IPrivacy
+    )
 from canonical.launchpad.layers import LaunchpadLayer
 import canonical.launchpad.pagetitles
 from canonical.launchpad.webapp import canonical_url, urlappend
 from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.badge import IHasBadges
 from canonical.launchpad.webapp.interfaces import (
-    IApplicationMenu, IContextMenu, IFacetMenu, ILaunchBag, INavigationMenu,
-    IPrimaryContext, NoCanonicalUrl)
-from canonical.launchpad.webapp.menu import get_current_view, get_facet
+    IApplicationMenu,
+    IContextMenu,
+    IFacetMenu,
+    ILaunchBag,
+    INavigationMenu,
+    IPrimaryContext,
+    NoCanonicalUrl
+    )
+from canonical.launchpad.webapp.menu import (
+    get_current_view,
+    get_facet,
+    )
 from canonical.launchpad.webapp.publisher import (
-    get_current_browser_request, LaunchpadView, nearest)
+    get_current_browser_request,
+    LaunchpadView,
+    nearest
+    )
 from canonical.launchpad.webapp.session import get_cookie_domain
 from canonical.lazr.canonicalurl import nearest_adapter
 from lp.app.browser.stringformatter import escape, FormattersAPI
@@ -61,6 +84,7 @@
 from lp.bugs.interfaces.bug import IBug
 from lp.buildmaster.enums import BuildStatus
 from lp.code.interfaces.branch import IBranch
+from lp.services.features import getFeatureFlag
 from lp.soyuz.enums import ArchivePurpose
 from lp.soyuz.interfaces.archive import IPPA
 from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
@@ -1220,7 +1244,6 @@
         The link text uses both the display name and Launchpad id to clearly
         indicate which user profile is linked.
         """
-        from lp.services.features import getFeatureFlag
         if bool(getFeatureFlag('disclosure.picker_enhancements.enabled')):
             text = self.unique_displayname(None)
             # XXX sinzui 2011-05-31: Remove this next line when the feature
@@ -1236,6 +1259,10 @@
         return self._makeLink(view_name, 'mainsite', text)
 
 
+class MixedVisibilityError(Exception):
+    """An informational error that visibility is being mixed."""
+
+
 class TeamFormatterAPI(PersonFormatterAPI):
     """Adapter for `ITeam` objects to a formatted string."""
 
@@ -1249,6 +1276,7 @@
         """
         if not check_permission('launchpad.View', self._context):
             # This person has no permission to view the team details.
+            self._report()
             return None
         return super(TeamFormatterAPI, self).url(view_name, rootsite)
 
@@ -1256,6 +1284,7 @@
         """See `ObjectFormatterAPI`."""
         if not check_permission('launchpad.View', self._context):
             # This person has no permission to view the team details.
+            self._report()
             return None
         return super(TeamFormatterAPI, self).api_url(context)
 
@@ -1268,6 +1297,7 @@
         person = self._context
         if not check_permission('launchpad.View', person):
             # This person has no permission to view the team details.
+            self._report()
             return '<span class="sprite team">%s</span>' % cgi.escape(
                 self.hidden)
         return super(TeamFormatterAPI, self).link(view_name, rootsite)
@@ -1277,6 +1307,7 @@
         person = self._context
         if not check_permission('launchpad.View', person):
             # This person has no permission to view the team details.
+            self._report()
             return self.hidden
         return super(TeamFormatterAPI, self).displayname(view_name, rootsite)
 
@@ -1285,9 +1316,19 @@
         person = self._context
         if not check_permission('launchpad.View', person):
             # This person has no permission to view the team details.
+            self._report()
             return self.hidden
         return super(TeamFormatterAPI, self).unique_displayname(view_name)
 
+    def _report(self):
+        if bool(getFeatureFlag('disclosure.log_private_team_leaks.enabled')):
+            request = get_current_browser_request()
+            try:
+                raise MixedVisibilityError()
+            except MixedVisibilityError:
+                getUtility(IErrorReportingUtility).raising(
+                    sys.exc_info(), request)
+
 
 class CustomizableFormatter(ObjectFormatterAPI):
     """A ObjectFormatterAPI that is easy to customize.

=== added file 'lib/lp/app/browser/tests/test_mixed_visibility.py'
--- lib/lp/app/browser/tests/test_mixed_visibility.py	1970-01-01 00:00:00 +0000
+++ lib/lp/app/browser/tests/test_mixed_visibility.py	2011-10-13 04:48:02 +0000
@@ -0,0 +1,34 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.app.browser.tales import TeamFormatterAPI
+from lp.registry.interfaces.person import PersonVisibility
+from lp.services.features.testing import FeatureFixture
+from lp.testing import (
+    person_logged_in,
+    TestCaseWithFactory,
+    )
+
+
+MIXED_VISIBILITY_FLAG = {'disclosure.log_private_team_leaks.enabled': 'on'}
+
+
+class TestMixedVisibility(TestCaseWithFactory):
+    layer = DatabaseFunctionalLayer
+
+    def test_mixed_visibility(self):
+        # If a viewer attempts to (or code on their behalf) get information
+        # about a private team, with the feature flag enabled, an
+        # informational OOPS is logged.
+        team = self.factory.makeTeam(visibility=PersonVisibility.PRIVATE)
+        viewer = self.factory.makePerson() 
+        with FeatureFixture(MIXED_VISIBILITY_FLAG):
+            with person_logged_in(viewer):
+                self.assertEqual(
+                    u'<hidden>', TeamFormatterAPI(team).displayname(None))
+            self.assertEqual(1, len(self.oopses))
+            self.assertTrue(
+                self.oopses[0]['tb_text'].startswith('Traceback'))

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2011-10-12 10:33:08 +0000
+++ lib/lp/services/features/flags.py	2011-10-13 04:48:02 +0000
@@ -160,6 +160,11 @@
      ('Enables the longpoll mechanism for merge proposals so that diffs, '
       'for example, are updated in-page when they are ready.'),
      ''),
+    ('disclosure.log_private_team_leaks.enabled',
+     'boolean',
+     ('Enables soft OOPSes for code that is mixing visibility rules, such '
+      'as disclosing private teams, so the data can be analyzed.'),
+     ''),
     ])
 
 # The set of all flag names that are documented.