Merge lp:~stevenk/launchpad/soft-oops-on-private-team-disclosure into lp:launchpad

Proposed by Steve Kowalik
Status: Merged
Approved by: Steve Kowalik
Approved revision: no longer in the source branch.
Merged at revision: 14148
Proposed branch: lp:~stevenk/launchpad/soft-oops-on-private-team-disclosure
Merge into: lp:launchpad
Diff against target: 231 lines (+98/-18)
3 files modified
lib/lp/app/browser/tales.py (+59/-18)
lib/lp/app/browser/tests/test_mixed_visibility.py (+34/-0)
lib/lp/services/features/flags.py (+5/-0)
To merge this branch: bzr merge lp:~stevenk/launchpad/soft-oops-on-private-team-disclosure
Reviewer Review Type Date Requested Status
Ian Booth (community) code Approve
Review via email: mp+79066@code.launchpad.net

Commit message

[r=wallyworld][bug=873161] Warn (via an informational OOPS) when mixed visibility WRT private teams is encountered.

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Ian Booth (wallyworld) wrote :

Thanks for making the suggested changes on irc - rename report() and improve unit test.
Thanks also for the drive-by lint fixes.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 05:41:57 +0000
@@ -1,14 +1,16 @@
1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the1# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4# pylint: disable-msg=C0103,W0613,R0911,F0401
5#
6"""Implementation of the lp: htmlform: fmt: namespaces in TALES."""4"""Implementation of the lp: htmlform: fmt: namespaces in TALES."""
75
8__metaclass__ = type6__metaclass__ = type
97
10import bisect8import bisect
11import cgi9import cgi
10from datetime import (
11 datetime,
12 timedelta,
13 )
12from email.Utils import formatdate14from email.Utils import formatdate
13import math15import math
14import os.path16import os.path
@@ -17,14 +19,22 @@
17from textwrap import dedent19from textwrap import dedent
18import urllib20import urllib
1921
20##import warnings
21
22from datetime import datetime, timedelta
23from lazr.enum import enumerated_type_registry22from lazr.enum import enumerated_type_registry
24from lazr.uri import URI23from lazr.uri import URI
2524import pytz
26from zope.interface import Interface, Attribute, implements25from z3c.ptcompat import ViewPageTemplateFile
27from zope.component import adapts, getUtility, queryAdapter, getMultiAdapter26from zope.error.interfaces import IErrorReportingUtility
27from zope.interface import (
28 Attribute,
29 Interface,
30 implements,
31 )
32from zope.component import (
33 adapts,
34 getUtility,
35 queryAdapter,
36 getMultiAdapter,
37 )
28from zope.app import zapi38from zope.app import zapi
29from zope.publisher.browser import BrowserView39from zope.publisher.browser import BrowserView
30from zope.traversing.interfaces import (40from zope.traversing.interfaces import (
@@ -36,23 +46,36 @@
36from zope.security.proxy import isinstance as zope_isinstance46from zope.security.proxy import isinstance as zope_isinstance
37from zope.schema import TextLine47from zope.schema import TextLine
3848
39import pytz
40from z3c.ptcompat import ViewPageTemplateFile
41
42from canonical.launchpad import _49from canonical.launchpad import _
43from canonical.launchpad.interfaces.launchpad import (50from canonical.launchpad.interfaces.launchpad import (
44 IHasIcon, IHasLogo, IHasMugshot, IPrivacy)51 IHasIcon,
52 IHasLogo,
53 IHasMugshot,
54 IPrivacy
55 )
45from canonical.launchpad.layers import LaunchpadLayer56from canonical.launchpad.layers import LaunchpadLayer
46import canonical.launchpad.pagetitles57import canonical.launchpad.pagetitles
47from canonical.launchpad.webapp import canonical_url, urlappend58from canonical.launchpad.webapp import canonical_url, urlappend
48from canonical.launchpad.webapp.authorization import check_permission59from canonical.launchpad.webapp.authorization import check_permission
49from canonical.launchpad.webapp.badge import IHasBadges60from canonical.launchpad.webapp.badge import IHasBadges
50from canonical.launchpad.webapp.interfaces import (61from canonical.launchpad.webapp.interfaces import (
51 IApplicationMenu, IContextMenu, IFacetMenu, ILaunchBag, INavigationMenu,62 IApplicationMenu,
52 IPrimaryContext, NoCanonicalUrl)63 IContextMenu,
53from canonical.launchpad.webapp.menu import get_current_view, get_facet64 IFacetMenu,
65 ILaunchBag,
66 INavigationMenu,
67 IPrimaryContext,
68 NoCanonicalUrl
69 )
70from canonical.launchpad.webapp.menu import (
71 get_current_view,
72 get_facet,
73 )
54from canonical.launchpad.webapp.publisher import (74from canonical.launchpad.webapp.publisher import (
55 get_current_browser_request, LaunchpadView, nearest)75 get_current_browser_request,
76 LaunchpadView,
77 nearest
78 )
56from canonical.launchpad.webapp.session import get_cookie_domain79from canonical.launchpad.webapp.session import get_cookie_domain
57from canonical.lazr.canonicalurl import nearest_adapter80from canonical.lazr.canonicalurl import nearest_adapter
58from lp.app.browser.stringformatter import escape, FormattersAPI81from lp.app.browser.stringformatter import escape, FormattersAPI
@@ -61,6 +84,7 @@
61from lp.bugs.interfaces.bug import IBug84from lp.bugs.interfaces.bug import IBug
62from lp.buildmaster.enums import BuildStatus85from lp.buildmaster.enums import BuildStatus
63from lp.code.interfaces.branch import IBranch86from lp.code.interfaces.branch import IBranch
87from lp.services.features import getFeatureFlag
64from lp.soyuz.enums import ArchivePurpose88from lp.soyuz.enums import ArchivePurpose
65from lp.soyuz.interfaces.archive import IPPA89from lp.soyuz.interfaces.archive import IPPA
66from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet90from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
@@ -1220,7 +1244,6 @@
1220 The link text uses both the display name and Launchpad id to clearly1244 The link text uses both the display name and Launchpad id to clearly
1221 indicate which user profile is linked.1245 indicate which user profile is linked.
1222 """1246 """
1223 from lp.services.features import getFeatureFlag
1224 if bool(getFeatureFlag('disclosure.picker_enhancements.enabled')):1247 if bool(getFeatureFlag('disclosure.picker_enhancements.enabled')):
1225 text = self.unique_displayname(None)1248 text = self.unique_displayname(None)
1226 # XXX sinzui 2011-05-31: Remove this next line when the feature1249 # XXX sinzui 2011-05-31: Remove this next line when the feature
@@ -1236,6 +1259,10 @@
1236 return self._makeLink(view_name, 'mainsite', text)1259 return self._makeLink(view_name, 'mainsite', text)
12371260
12381261
1262class MixedVisibilityError(Exception):
1263 """An informational error that visibility is being mixed."""
1264
1265
1239class TeamFormatterAPI(PersonFormatterAPI):1266class TeamFormatterAPI(PersonFormatterAPI):
1240 """Adapter for `ITeam` objects to a formatted string."""1267 """Adapter for `ITeam` objects to a formatted string."""
12411268
@@ -1249,6 +1276,7 @@
1249 """1276 """
1250 if not check_permission('launchpad.View', self._context):1277 if not check_permission('launchpad.View', self._context):
1251 # This person has no permission to view the team details.1278 # This person has no permission to view the team details.
1279 self._report_visibility_leak()
1252 return None1280 return None
1253 return super(TeamFormatterAPI, self).url(view_name, rootsite)1281 return super(TeamFormatterAPI, self).url(view_name, rootsite)
12541282
@@ -1256,6 +1284,7 @@
1256 """See `ObjectFormatterAPI`."""1284 """See `ObjectFormatterAPI`."""
1257 if not check_permission('launchpad.View', self._context):1285 if not check_permission('launchpad.View', self._context):
1258 # This person has no permission to view the team details.1286 # This person has no permission to view the team details.
1287 self._report_visibility_leak()
1259 return None1288 return None
1260 return super(TeamFormatterAPI, self).api_url(context)1289 return super(TeamFormatterAPI, self).api_url(context)
12611290
@@ -1268,6 +1297,7 @@
1268 person = self._context1297 person = self._context
1269 if not check_permission('launchpad.View', person):1298 if not check_permission('launchpad.View', person):
1270 # This person has no permission to view the team details.1299 # This person has no permission to view the team details.
1300 self._report_visibility_leak()
1271 return '<span class="sprite team">%s</span>' % cgi.escape(1301 return '<span class="sprite team">%s</span>' % cgi.escape(
1272 self.hidden)1302 self.hidden)
1273 return super(TeamFormatterAPI, self).link(view_name, rootsite)1303 return super(TeamFormatterAPI, self).link(view_name, rootsite)
@@ -1277,6 +1307,7 @@
1277 person = self._context1307 person = self._context
1278 if not check_permission('launchpad.View', person):1308 if not check_permission('launchpad.View', person):
1279 # This person has no permission to view the team details.1309 # This person has no permission to view the team details.
1310 self._report_visibility_leak()
1280 return self.hidden1311 return self.hidden
1281 return super(TeamFormatterAPI, self).displayname(view_name, rootsite)1312 return super(TeamFormatterAPI, self).displayname(view_name, rootsite)
12821313
@@ -1285,9 +1316,19 @@
1285 person = self._context1316 person = self._context
1286 if not check_permission('launchpad.View', person):1317 if not check_permission('launchpad.View', person):
1287 # This person has no permission to view the team details.1318 # This person has no permission to view the team details.
1319 self._report_visibility_leak()
1288 return self.hidden1320 return self.hidden
1289 return super(TeamFormatterAPI, self).unique_displayname(view_name)1321 return super(TeamFormatterAPI, self).unique_displayname(view_name)
12901322
1323 def _report_visibility_leak(self):
1324 if bool(getFeatureFlag('disclosure.log_private_team_leaks.enabled')):
1325 request = get_current_browser_request()
1326 try:
1327 raise MixedVisibilityError()
1328 except MixedVisibilityError:
1329 getUtility(IErrorReportingUtility).raising(
1330 sys.exc_info(), request)
1331
12911332
1292class CustomizableFormatter(ObjectFormatterAPI):1333class CustomizableFormatter(ObjectFormatterAPI):
1293 """A ObjectFormatterAPI that is easy to customize.1334 """A ObjectFormatterAPI that is easy to customize.
12941335
=== 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 05:41:57 +0000
@@ -0,0 +1,34 @@
1# Copyright 2011 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4__metaclass__ = type
5
6from canonical.testing.layers import DatabaseFunctionalLayer
7from lp.app.browser.tales import TeamFormatterAPI
8from lp.registry.interfaces.person import PersonVisibility
9from lp.services.features.testing import FeatureFixture
10from lp.testing import (
11 person_logged_in,
12 TestCaseWithFactory,
13 )
14
15
16MIXED_VISIBILITY_FLAG = {'disclosure.log_private_team_leaks.enabled': 'on'}
17
18
19class TestMixedVisibility(TestCaseWithFactory):
20 layer = DatabaseFunctionalLayer
21
22 def test_mixed_visibility(self):
23 # If a viewer attempts to (or code on their behalf) get information
24 # about a private team, with the feature flag enabled, an
25 # informational OOPS is logged.
26 team = self.factory.makeTeam(visibility=PersonVisibility.PRIVATE)
27 viewer = self.factory.makePerson()
28 with FeatureFixture(MIXED_VISIBILITY_FLAG):
29 with person_logged_in(viewer):
30 self.assertEqual(
31 u'<hidden>', TeamFormatterAPI(team).displayname(None))
32 self.assertEqual(1, len(self.oopses))
33 self.assertTrue(
34 'MixedVisibilityError' in self.oopses[0]['tb_text'])
035
=== 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 05:41:57 +0000
@@ -160,6 +160,11 @@
160 ('Enables the longpoll mechanism for merge proposals so that diffs, '160 ('Enables the longpoll mechanism for merge proposals so that diffs, '
161 'for example, are updated in-page when they are ready.'),161 'for example, are updated in-page when they are ready.'),
162 ''),162 ''),
163 ('disclosure.log_private_team_leaks.enabled',
164 'boolean',
165 ('Enables soft OOPSes for code that is mixing visibility rules, such '
166 'as disclosing private teams, so the data can be analyzed.'),
167 ''),
163 ])168 ])
164169
165# The set of all flag names that are documented.170# The set of all flag names that are documented.