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
1=== modified file 'lib/lp/app/browser/tales.py'
2--- lib/lp/app/browser/tales.py 2011-09-26 20:56:58 +0000
3+++ lib/lp/app/browser/tales.py 2011-10-13 05:41:57 +0000
4@@ -1,14 +1,16 @@
5-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
6+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9-# pylint: disable-msg=C0103,W0613,R0911,F0401
10-#
11 """Implementation of the lp: htmlform: fmt: namespaces in TALES."""
12
13 __metaclass__ = type
14
15 import bisect
16 import cgi
17+from datetime import (
18+ datetime,
19+ timedelta,
20+ )
21 from email.Utils import formatdate
22 import math
23 import os.path
24@@ -17,14 +19,22 @@
25 from textwrap import dedent
26 import urllib
27
28-##import warnings
29-
30-from datetime import datetime, timedelta
31 from lazr.enum import enumerated_type_registry
32 from lazr.uri import URI
33-
34-from zope.interface import Interface, Attribute, implements
35-from zope.component import adapts, getUtility, queryAdapter, getMultiAdapter
36+import pytz
37+from z3c.ptcompat import ViewPageTemplateFile
38+from zope.error.interfaces import IErrorReportingUtility
39+from zope.interface import (
40+ Attribute,
41+ Interface,
42+ implements,
43+ )
44+from zope.component import (
45+ adapts,
46+ getUtility,
47+ queryAdapter,
48+ getMultiAdapter,
49+ )
50 from zope.app import zapi
51 from zope.publisher.browser import BrowserView
52 from zope.traversing.interfaces import (
53@@ -36,23 +46,36 @@
54 from zope.security.proxy import isinstance as zope_isinstance
55 from zope.schema import TextLine
56
57-import pytz
58-from z3c.ptcompat import ViewPageTemplateFile
59-
60 from canonical.launchpad import _
61 from canonical.launchpad.interfaces.launchpad import (
62- IHasIcon, IHasLogo, IHasMugshot, IPrivacy)
63+ IHasIcon,
64+ IHasLogo,
65+ IHasMugshot,
66+ IPrivacy
67+ )
68 from canonical.launchpad.layers import LaunchpadLayer
69 import canonical.launchpad.pagetitles
70 from canonical.launchpad.webapp import canonical_url, urlappend
71 from canonical.launchpad.webapp.authorization import check_permission
72 from canonical.launchpad.webapp.badge import IHasBadges
73 from canonical.launchpad.webapp.interfaces import (
74- IApplicationMenu, IContextMenu, IFacetMenu, ILaunchBag, INavigationMenu,
75- IPrimaryContext, NoCanonicalUrl)
76-from canonical.launchpad.webapp.menu import get_current_view, get_facet
77+ IApplicationMenu,
78+ IContextMenu,
79+ IFacetMenu,
80+ ILaunchBag,
81+ INavigationMenu,
82+ IPrimaryContext,
83+ NoCanonicalUrl
84+ )
85+from canonical.launchpad.webapp.menu import (
86+ get_current_view,
87+ get_facet,
88+ )
89 from canonical.launchpad.webapp.publisher import (
90- get_current_browser_request, LaunchpadView, nearest)
91+ get_current_browser_request,
92+ LaunchpadView,
93+ nearest
94+ )
95 from canonical.launchpad.webapp.session import get_cookie_domain
96 from canonical.lazr.canonicalurl import nearest_adapter
97 from lp.app.browser.stringformatter import escape, FormattersAPI
98@@ -61,6 +84,7 @@
99 from lp.bugs.interfaces.bug import IBug
100 from lp.buildmaster.enums import BuildStatus
101 from lp.code.interfaces.branch import IBranch
102+from lp.services.features import getFeatureFlag
103 from lp.soyuz.enums import ArchivePurpose
104 from lp.soyuz.interfaces.archive import IPPA
105 from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
106@@ -1220,7 +1244,6 @@
107 The link text uses both the display name and Launchpad id to clearly
108 indicate which user profile is linked.
109 """
110- from lp.services.features import getFeatureFlag
111 if bool(getFeatureFlag('disclosure.picker_enhancements.enabled')):
112 text = self.unique_displayname(None)
113 # XXX sinzui 2011-05-31: Remove this next line when the feature
114@@ -1236,6 +1259,10 @@
115 return self._makeLink(view_name, 'mainsite', text)
116
117
118+class MixedVisibilityError(Exception):
119+ """An informational error that visibility is being mixed."""
120+
121+
122 class TeamFormatterAPI(PersonFormatterAPI):
123 """Adapter for `ITeam` objects to a formatted string."""
124
125@@ -1249,6 +1276,7 @@
126 """
127 if not check_permission('launchpad.View', self._context):
128 # This person has no permission to view the team details.
129+ self._report_visibility_leak()
130 return None
131 return super(TeamFormatterAPI, self).url(view_name, rootsite)
132
133@@ -1256,6 +1284,7 @@
134 """See `ObjectFormatterAPI`."""
135 if not check_permission('launchpad.View', self._context):
136 # This person has no permission to view the team details.
137+ self._report_visibility_leak()
138 return None
139 return super(TeamFormatterAPI, self).api_url(context)
140
141@@ -1268,6 +1297,7 @@
142 person = self._context
143 if not check_permission('launchpad.View', person):
144 # This person has no permission to view the team details.
145+ self._report_visibility_leak()
146 return '<span class="sprite team">%s</span>' % cgi.escape(
147 self.hidden)
148 return super(TeamFormatterAPI, self).link(view_name, rootsite)
149@@ -1277,6 +1307,7 @@
150 person = self._context
151 if not check_permission('launchpad.View', person):
152 # This person has no permission to view the team details.
153+ self._report_visibility_leak()
154 return self.hidden
155 return super(TeamFormatterAPI, self).displayname(view_name, rootsite)
156
157@@ -1285,9 +1316,19 @@
158 person = self._context
159 if not check_permission('launchpad.View', person):
160 # This person has no permission to view the team details.
161+ self._report_visibility_leak()
162 return self.hidden
163 return super(TeamFormatterAPI, self).unique_displayname(view_name)
164
165+ def _report_visibility_leak(self):
166+ if bool(getFeatureFlag('disclosure.log_private_team_leaks.enabled')):
167+ request = get_current_browser_request()
168+ try:
169+ raise MixedVisibilityError()
170+ except MixedVisibilityError:
171+ getUtility(IErrorReportingUtility).raising(
172+ sys.exc_info(), request)
173+
174
175 class CustomizableFormatter(ObjectFormatterAPI):
176 """A ObjectFormatterAPI that is easy to customize.
177
178=== added file 'lib/lp/app/browser/tests/test_mixed_visibility.py'
179--- lib/lp/app/browser/tests/test_mixed_visibility.py 1970-01-01 00:00:00 +0000
180+++ lib/lp/app/browser/tests/test_mixed_visibility.py 2011-10-13 05:41:57 +0000
181@@ -0,0 +1,34 @@
182+# Copyright 2011 Canonical Ltd. This software is licensed under the
183+# GNU Affero General Public License version 3 (see the file LICENSE).
184+
185+__metaclass__ = type
186+
187+from canonical.testing.layers import DatabaseFunctionalLayer
188+from lp.app.browser.tales import TeamFormatterAPI
189+from lp.registry.interfaces.person import PersonVisibility
190+from lp.services.features.testing import FeatureFixture
191+from lp.testing import (
192+ person_logged_in,
193+ TestCaseWithFactory,
194+ )
195+
196+
197+MIXED_VISIBILITY_FLAG = {'disclosure.log_private_team_leaks.enabled': 'on'}
198+
199+
200+class TestMixedVisibility(TestCaseWithFactory):
201+ layer = DatabaseFunctionalLayer
202+
203+ def test_mixed_visibility(self):
204+ # If a viewer attempts to (or code on their behalf) get information
205+ # about a private team, with the feature flag enabled, an
206+ # informational OOPS is logged.
207+ team = self.factory.makeTeam(visibility=PersonVisibility.PRIVATE)
208+ viewer = self.factory.makePerson()
209+ with FeatureFixture(MIXED_VISIBILITY_FLAG):
210+ with person_logged_in(viewer):
211+ self.assertEqual(
212+ u'<hidden>', TeamFormatterAPI(team).displayname(None))
213+ self.assertEqual(1, len(self.oopses))
214+ self.assertTrue(
215+ 'MixedVisibilityError' in self.oopses[0]['tb_text'])
216
217=== modified file 'lib/lp/services/features/flags.py'
218--- lib/lp/services/features/flags.py 2011-10-12 10:33:08 +0000
219+++ lib/lp/services/features/flags.py 2011-10-13 05:41:57 +0000
220@@ -160,6 +160,11 @@
221 ('Enables the longpoll mechanism for merge proposals so that diffs, '
222 'for example, are updated in-page when they are ready.'),
223 ''),
224+ ('disclosure.log_private_team_leaks.enabled',
225+ 'boolean',
226+ ('Enables soft OOPSes for code that is mixing visibility rules, such '
227+ 'as disclosing private teams, so the data can be analyzed.'),
228+ ''),
229 ])
230
231 # The set of all flag names that are documented.