launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05460
[Merge] lp:~abentley/launchpad/enable-anonymous-jsoncache into lp:launchpad
Aaron Bentley has proposed merging lp:~abentley/launchpad/enable-anonymous-jsoncache into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~abentley/launchpad/enable-anonymous-jsoncache/+merge/81741
= Summary =
Enable the JSON request cache for anonymous users. This enables us to provide the new dynamic bug listings to anonymous users.
== Proposed fix ==
Obfuscate plain strings for anonymous users before serializing to JSON. (API 'Resource' object text fields are already obfuscated.)
== Pre-implementation notes ==
Discussed with Deryck.
== Implementation details ==
Also obfuscated the objects used when rendering bug listings, again so that we can provide dynamic bug listings to anonymous users.
== Tests ==
bin/test -t test_getCacheJSON_non_resource_context -t test_getCache_anonymous -t t test_mustache_rendering_obfuscation -t TestObfuscateStructure -t test_cache_javascript
== Demo and Q/A ==
Create a bug with an email address in its title. The email address should not appear in the initial listings. It should not appear when you hit the "First" link. It should not be present in the LP.cache
= Launchpad lint =
The issues in test_sharing_details are caused by very-long enum names.
Checking for conflicts and issues in changed files.
Linting changed files:
lib/canonical/launchpad/webapp/publisher.py
lib/canonical/launchpad/webapp/tests/test_publisher.py
lib/lp/services/tests/test_utils.py
lib/lp/translations/browser/tests/test_sharing_details.py
lib/lp/bugs/browser/tests/test_bugtask.py
lib/lp/bugs/browser/bugtask.py
lib/lp/services/utils.py
./lib/lp/translations/browser/tests/test_sharing_details.py
218: E251 no spaces around keyword / parameter equals
226: E251 no spaces around keyword / parameter equals
254: E251 no spaces around keyword / parameter equals
265: E251 no spaces around keyword / parameter equals
277: E251 no spaces around keyword / parameter equals
311: E251 no spaces around keyword / parameter equals
--
https://code.launchpad.net/~abentley/launchpad/enable-anonymous-jsoncache/+merge/81741
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/enable-anonymous-jsoncache into lp:launchpad.
=== modified file 'lib/canonical/launchpad/webapp/publisher.py'
--- lib/canonical/launchpad/webapp/publisher.py 2011-11-07 16:18:20 +0000
+++ lib/canonical/launchpad/webapp/publisher.py 2011-11-09 15:01:36 +0000
@@ -84,6 +84,7 @@
defaultFlagValue,
getFeatureFlag,
)
+from lp.services.utils import obfuscate_structure
# Monkeypatch NotFound to always avoid generating OOPS
# from NotFound in web service calls.
@@ -374,12 +375,11 @@
info_message = property(_getInfoMessage, _setInfoMessage)
def getCacheJSON(self):
- if self.user is not None:
- cache = dict(IJSONRequestCache(self.request).objects)
- else:
- cache = dict()
+ cache = dict(IJSONRequestCache(self.request).objects)
if WebLayerAPI(self.context).is_entry:
cache['context'] = self.context
+ if self.user is None:
+ cache = obfuscate_structure(cache)
return simplejson.dumps(
cache, cls=ResourceJSONEncoder,
media_type=EntryResource.JSON_TYPE)
=== modified file 'lib/canonical/launchpad/webapp/tests/test_publisher.py'
--- lib/canonical/launchpad/webapp/tests/test_publisher.py 2011-11-03 13:10:30 +0000
+++ lib/canonical/launchpad/webapp/tests/test_publisher.py 2011-11-09 15:01:36 +0000
@@ -49,7 +49,7 @@
def test_getCacheJSON_non_resource_context(self):
view = LaunchpadView(object(), LaunchpadTestRequest())
- self.assertEqual('{}', view.getCacheJSON())
+ self.assertEqual('{"beta_features": []}', view.getCacheJSON())
@staticmethod
def getCanada():
@@ -102,7 +102,7 @@
IJSONRequestCache(request).objects['my_bool'] = True
json_dict = simplejson.loads(view.getCacheJSON())
self.assertIsCanada(json_dict['context'])
- self.assertFalse('my_bool' in json_dict)
+ self.assertIn('my_bool', json_dict)
def test_getCache_anonymous_obfuscated(self):
request = LaunchpadTestRequest()
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2011-11-08 14:08:46 +0000
+++ lib/lp/bugs/browser/bugtask.py 2011-11-09 15:01:36 +0000
@@ -92,6 +92,7 @@
InputErrors,
)
from zope.app.form.utility import setUpWidget
+from zope.app.security.interfaces import IUnauthenticatedPrincipal
from zope.component import (
ComponentLookupError,
getAdapter,
@@ -278,6 +279,7 @@
cachedproperty,
get_property_cache,
)
+from lp.services.utils import obfuscate_structure
vocabulary_registry = getVocabularyRegistry()
@@ -2310,9 +2312,11 @@
@property
def mustache(self):
"""The rendered mustache template."""
- cache = IJSONRequestCache(self.request)
+ objects = IJSONRequestCache(self.request).objects
+ if IUnauthenticatedPrincipal.providedBy(self.request.principal):
+ objects = obfuscate_structure(objects)
return pystache.render(self.mustache_template,
- cache.objects['mustache_model'])
+ objects['mustache_model'])
@property
def model(self):
=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py 2011-11-08 14:08:46 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py 2011-11-09 15:01:36 +0000
@@ -1680,13 +1680,15 @@
field_visibility = cache.objects['field_visibility']
self.assertTrue(field_visibility['show_title'])
- def getBugtaskBrowser(self):
+ def getBugtaskBrowser(self, title=None, no_login=False):
"""Return a browser for a new bugtask."""
bugtask = self.factory.makeBugTask()
with person_logged_in(bugtask.target.owner):
bugtask.target.official_malone = True
+ if title is not None:
+ bugtask.bug.title = title
browser = self.getViewBrowser(
- bugtask.target, '+bugs', rootsite='bugs')
+ bugtask.target, '+bugs', rootsite='bugs', no_login=no_login)
return bugtask, browser
def assertHTML(self, browser, *tags, **kwargs):
@@ -1716,6 +1718,13 @@
bug_number = self.getBugNumberTag(bug_task)
self.assertHTML(browser, self.client_listing, bug_number)
+ def test_mustache_rendering_obfuscation(self):
+ """For anonymous users, email addresses are obfuscated."""
+ with self.dynamic_listings():
+ bug_task, browser = self.getBugtaskBrowser(title='a@xxxxxxxxxxx',
+ no_login=True)
+ self.assertNotIn('a@xxxxxxxxxxx', browser.contents)
+
def getNavigator(self):
request = LaunchpadTestRequest()
navigator = BugListingBatchNavigator([], request, [], 1)
=== modified file 'lib/lp/services/tests/test_utils.py'
--- lib/lp/services/tests/test_utils.py 2011-11-04 11:07:42 +0000
+++ lib/lp/services/tests/test_utils.py 2011-11-09 15:01:36 +0000
@@ -30,6 +30,7 @@
docstring_dedent,
file_exists,
iter_split,
+ obfuscate_structure,
load_bz2_pickle,
run_capturing_output,
save_bz2_pickle,
@@ -343,3 +344,43 @@
tempfile = os.path.join(tempdir, "dump")
save_bz2_pickle(data, tempfile)
self.assertEqual(data, load_bz2_pickle(tempfile))
+
+
+class TestObfuscateStructure(TestCase):
+
+ def test_obfuscate_string(self):
+ """Strings are obfuscated."""
+ obfuscated = obfuscate_structure('My address is a@xxxxxxxxxxx')
+ self.assertEqual(
+ 'My address is <email address hidden>', obfuscated)
+
+ def test_obfuscate_list(self):
+ """List elements are obfuscated."""
+ obfuscated = obfuscate_structure(['My address is a@xxxxxxxxxxx'])
+ self.assertEqual(
+ ['My address is <email address hidden>'], obfuscated)
+
+ def test_obfuscate_tuple(self):
+ """Tuple elements are obfuscated."""
+ obfuscated = obfuscate_structure(('My address is a@xxxxxxxxxxx',))
+ self.assertEqual(
+ ['My address is <email address hidden>'], obfuscated)
+
+ def test_obfuscate_dict_key(self):
+ """Dictionary keys are obfuscated."""
+ obfuscated = obfuscate_structure(
+ {'My address is a@xxxxxxxxxxx': 'foo'})
+ self.assertEqual(
+ {'My address is <email address hidden>': 'foo'}, obfuscated)
+
+ def test_obfuscate_dict_value(self):
+ """Dictionary values are obfuscated."""
+ obfuscated = obfuscate_structure(
+ {'foo': 'My address is a@xxxxxxxxxxx'})
+ self.assertEqual(
+ {'foo': 'My address is <email address hidden>'}, obfuscated)
+
+ def test_recursion(self):
+ """Values are obfuscated recursively."""
+ obfuscated = obfuscate_structure({'foo': (['a@xxxxxxxxxxx'],)})
+ self.assertEqual({'foo': [['<email address hidden>']]}, obfuscated)
=== modified file 'lib/lp/services/utils.py'
--- lib/lp/services/utils.py 2011-11-04 10:23:55 +0000
+++ lib/lp/services/utils.py 2011-11-09 15:01:36 +0000
@@ -20,6 +20,7 @@
'iter_split',
'load_bz2_pickle',
'obfuscate_email',
+ 'obfuscate_structure',
're_email_address',
'run_capturing_output',
'save_bz2_pickle',
@@ -355,3 +356,26 @@
return pickle.load(fin)
finally:
fin.close()
+
+
+def obfuscate_structure(o):
+ """Obfuscate the strings of a json-serializable structure.
+
+ Note: tuples are converted to lists because json encoders do not
+ distinguish between lists and tuples.
+
+ :param o: Any json-serializable object.
+ :return: a possibly-new structure in which all strings, list and tuple
+ elements, and dict keys and values have undergone obfuscate_email
+ recursively.
+ """
+ if isinstance(o, basestring):
+ return obfuscate_email(o)
+ elif isinstance(o, (list, tuple)):
+ return [obfuscate_structure(value) for value in o]
+ elif isinstance(o, (dict)):
+ return dict(
+ (obfuscate_structure(key), obfuscate_structure(value))
+ for key, value in o.iteritems())
+ else:
+ return o
=== modified file 'lib/lp/translations/browser/tests/test_sharing_details.py'
--- lib/lp/translations/browser/tests/test_sharing_details.py 2011-07-15 14:10:46 +0000
+++ lib/lp/translations/browser/tests/test_sharing_details.py 2011-11-09 15:01:36 +0000
@@ -937,12 +937,7 @@
def test_cache_javascript(self):
# Cache object entries propagate into the javascript.
sourcepackage = self.makeFullyConfiguredSharing()[0]
- anon_browser = self._getSharingDetailsViewBrowser(sourcepackage)
- # Anonymous users don't get cached objects due to bug #740208
- self.assertNotIn(
- 'productseries', extract_lp_cache(anon_browser.contents))
- browser = self._getSharingDetailsViewBrowser(
- sourcepackage, user=self.user)
+ browser = self._getSharingDetailsViewBrowser(sourcepackage)
self.assertIn(
'productseries', extract_lp_cache(browser.contents))
