launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03698
[Merge] lp:~henninge/launchpad/bug-740208-leak-email-bug-title into lp:launchpad
Henning Eggers has proposed merging lp:~henninge/launchpad/bug-740208-leak-email-bug-title into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~henninge/launchpad/bug-740208-leak-email-bug-title/+merge/61784
= Summary =
Email addresses were not obfuscated in Bug and Question titles on the
web ui. This has not caused any problem yet but for consistency with
obfuscating the description, this should be done as well.
== Proposed fix ==
Apply the "obfuscate_email" formatter at the right places. The formatter
checks for a logged-in user and only obfuscates for anonymous users.
== Pre-implementation notes ==
This was originally part of bug 740208 and the real problem here is
the webservice. But this was easy enough to fix so I split it off.
== Implementation details ==
There are some differences between Questions and Bugs pages. The
Questions page does not mention the title in the browser title (i.e.
the <title> tag) while Bugs does. Also, the page title for a Bug is
editable in-place, so the formatter had to be applied in lazrjs.py.
== Tests ==
I added new view tests for both affected views that test for this
specific problem. I had to XXX because bug 740208 is not yet fixed and
it still leaks the email address into the page. Also, I don't know why
the LP.cache on the Bugs page does not contain the description while
the one on Answers does. This is the reason for the different counts
in the tests.
bin/test -vvcm lp.bugs.browser.tests.test_bug_views \
-t TestEmailObfuscated
bin/test -vvcm lp.answers.browser.tests.test_views \
-t TestEmailObfuscated
== Demo and Q/A ==
Create a bug and an answer with an email address in the title and then
log out from the server. You should see <email address hidden> in the
title.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/browser/tests/test_bug_views.py
lib/lp/app/browser/lazrjs.py
lib/lp/answers/stories/webservice.txt
lib/lp/testing/factory.py
lib/lp/app/templates/base-layout-macros.pt
lib/lp/answers/browser/tests/test_views.py
lib/lp/bugs/browser/bugtask.py
lib/lp/answers/browser/question.py
--
https://code.launchpad.net/~henninge/launchpad/bug-740208-leak-email-bug-title/+merge/61784
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~henninge/launchpad/bug-740208-leak-email-bug-title into lp:launchpad.
=== modified file 'lib/lp/answers/browser/question.py'
--- lib/lp/answers/browser/question.py 2011-05-16 02:23:17 +0000
+++ lib/lp/answers/browser/question.py 2011-05-20 15:26:16 +0000
@@ -106,6 +106,7 @@
LaunchpadFormView,
safe_action,
)
+from lp.app.browser.stringformatter import FormattersAPI
from lp.app.errors import (
NotFoundError,
UnexpectedFormData,
@@ -831,7 +832,7 @@
@property
def label(self):
- return self.context.title
+ return FormattersAPI(self.context.title).obfuscate_email()
@property
def page_title(self):
=== modified file 'lib/lp/answers/browser/tests/test_views.py'
--- lib/lp/answers/browser/tests/test_views.py 2010-10-04 19:50:45 +0000
+++ lib/lp/answers/browser/tests/test_views.py 2011-05-20 15:26:16 +0000
@@ -20,10 +20,42 @@
DatabaseFunctionalLayer,
LaunchpadFunctionalLayer,
)
+from lp.testing import BrowserTestCase
+
+
+class TestEmailObfuscated(BrowserTestCase):
+ """Test for obfuscated emails on answers pages."""
+
+ layer = DatabaseFunctionalLayer
+
+ def getBrowserForQuestionWithEmail(self, email_address, no_login):
+ question = self.factory.makeQuestion(
+ title="Title with %s contained" % email_address,
+ description="Description with %s contained." % email_address)
+ return self.getViewBrowser(
+ question, rootsite="answers", no_login=no_login)
+
+ def test_user_sees_email_address(self):
+ """A logged-in user can see the email address on the page."""
+ email_address = "mark@xxxxxxxxxxx"
+ browser = self.getBrowserForQuestionWithEmail(
+ email_address, no_login=False)
+ self.assertEqual(4, browser.contents.count(email_address))
+
+ def test_anonymous_sees_not_email_address(self):
+ """The anonymous user cannot see the email address on the page."""
+ email_address = "mark@xxxxxxxxxxx"
+ browser = self.getBrowserForQuestionWithEmail(
+ email_address, no_login=True)
+ # XXX: henninge 20110520 bug=740208: One occurrence is still found
+ # in the LP.cache['context'] on the page.
+ self.assertEqual(2, browser.contents.count(email_address))
def test_suite():
suite = unittest.TestSuite()
+ loader = unittest.TestLoader()
+ suite.addTest(loader.loadTestsFromTestCase(TestEmailObfuscated))
suite.addTest(LayeredDocFileSuite('question-subscribe_me.txt',
setUp=setUp, tearDown=tearDown,
layer=DatabaseFunctionalLayer))
@@ -34,6 +66,3 @@
setUp=setUp, tearDown=tearDown,
layer=LaunchpadFunctionalLayer))
return suite
-
-if __name__ == '__main__':
- unittest.main()
=== modified file 'lib/lp/answers/stories/webservice.txt'
--- lib/lp/answers/stories/webservice.txt 2011-05-18 04:46:17 +0000
+++ lib/lp/answers/stories/webservice.txt 2011-05-20 15:26:16 +0000
@@ -202,7 +202,7 @@
date_last_query: u'20...+00:00'
date_last_response: u'20...+00:00'
date_solved: None
- description: u'description'
+ description: u'description...'
id: ...
language_link: u'http://api.launchpad.dev/devel/+languages/en'
messages_collection_link:
=== modified file 'lib/lp/app/browser/lazrjs.py'
--- lib/lp/app/browser/lazrjs.py 2011-03-18 07:40:01 +0000
+++ lib/lp/app/browser/lazrjs.py 2011-05-20 15:26:16 +0000
@@ -176,8 +176,9 @@
def value(self):
text = getattr(self.context, self.attribute_name, self.default_text)
if text is None:
- text = self.default_text
- return text
+ return self.default_text
+ else:
+ return FormattersAPI(text).obfuscate_email()
class TextAreaEditorWidget(TextWidgetBase):
@@ -246,8 +247,8 @@
:param exported_field: The attribute being edited. This should be
a field from an interface of the form ISomeInterface['fieldname']
:param default_html: Default display of attribute.
- :param content_box_id: The HTML id to use for this widget. Automatically
- generated if this is not provided.
+ :param content_box_id: The HTML id to use for this widget.
+ Automatically generated if this is not provided.
:param header: The large text at the top of the picker.
:param step_title: Smaller line of text below the header.
:param remove_button_text: Override default button text: "Remove"
@@ -493,8 +494,8 @@
:param edit_url: The URL to use for editing when the user isn't logged
in and when JS is off. Defaults to the edit_view on the context.
:param edit_title: Used to set the title attribute of the anchor.
- :param content_box_id: The HTML id to use for this widget. Automatically
- generated if this is not provided.
+ :param content_box_id: The HTML id to use for this widget.
+ Automatically generated if this is not provided.
:param header: The large text at the top of the choice popup.
"""
super(BooleanChoiceWidget, self).__init__(
@@ -546,8 +547,8 @@
:param exported_field: The attribute being edited. This should be
a field from an interface of the form ISomeInterface['fieldname']
:param header: The large text at the top of the picker.
- :param content_box_id: The HTML id to use for this widget. Automatically
- generated if this is not provided.
+ :param content_box_id: The HTML id to use for this widget.
+ Automatically generated if this is not provided.
:param enum: The enumerated type used to generate the widget items.
:param edit_view: The view name to use to generate the edit_url if
one is not specified.
=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt 2011-05-16 21:45:17 +0000
+++ lib/lp/app/templates/base-layout-macros.pt 2011-05-20 15:26:16 +0000
@@ -177,7 +177,7 @@
<script tal:condition="context/webservice:is_entry"
tal:content="string:LP.cache['context'] =
- ${context/webservice:json/fmt:obfuscate-email};">
+ ${context/webservice:json};">
</script>
</metal:lp-client-cache>
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2011-05-17 00:43:05 +0000
+++ lib/lp/bugs/browser/bugtask.py 2011-05-20 15:26:16 +0000
@@ -178,6 +178,7 @@
TextLineEditorWidget,
vocabulary_to_choice_edit_items,
)
+from lp.app.browser.stringformatter import FormattersAPI
from lp.app.browser.tales import (
ObjectImageDisplayAPI,
PersonFormatterAPI,
@@ -650,7 +651,8 @@
def page_title(self):
heading = 'Bug #%s in %s' % (
self.context.bug.id, self.context.bugtargetdisplayname)
- return smartquote('%s: "%s"') % (heading, self.context.bug.title)
+ title = FormattersAPI(self.context.bug.title).obfuscate_email()
+ return smartquote('%s: "%s"') % (heading, title)
@property
def next_url(self):
=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py 2011-05-17 14:54:56 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py 2011-05-20 15:26:16 +0000
@@ -48,6 +48,34 @@
self.assertTrue('href' not in dupe_warning)
+class TestEmailObfuscated(BrowserTestCase):
+ """Test for obfuscated emails on bug pages."""
+
+ layer = DatabaseFunctionalLayer
+
+ def getBrowserForBugWithEmail(self, email_address, no_login):
+ bug = self.factory.makeBug(
+ title="Title with %s contained" % email_address,
+ description="Description with %s contained." % email_address)
+ return self.getViewBrowser(bug, rootsite="bugs", no_login=no_login)
+
+ def test_user_sees_email_address(self):
+ """A logged-in user can see the email address on the page."""
+ email_address = "mark@xxxxxxxxxxx"
+ browser = self.getBrowserForBugWithEmail(
+ email_address, no_login=False)
+ self.assertEqual(6, browser.contents.count(email_address))
+
+ def test_anonymous_sees_not_email_address(self):
+ """The anonymous user cannot see the email address on the page."""
+ email_address = "mark@xxxxxxxxxxx"
+ browser = self.getBrowserForBugWithEmail(
+ email_address, no_login=True)
+ # XXX: henninge 20110520 bug=740208: One occurrence is still found
+ # in the LP.cache['context'] on the page.
+ self.assertEqual(1, browser.contents.count(email_address))
+
+
class TestBugPortletSubscribers(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
@@ -173,7 +201,6 @@
self.assertTrue('mute_subscription' in html)
# The template uses user_should_see_mute_link to decide
# whether or not to display the mute link.
- soup = BeautifulSoup(html)
self.assertTrue(
self._hasCSSClass(html, 'mute-link-container', 'hidden'),
'No "hidden" CSS class in mute-link-container.')
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-05-18 18:30:37 +0000
+++ lib/lp/testing/factory.py 2011-05-20 15:26:16 +0000
@@ -419,7 +419,7 @@
The string returned will always be a valid name that can be used in
Launchpad URLs.
- :param prefix: Used as a prefix for the unique string. If
+ :param prefix: Used as a prefix for the unique string. If
unspecified, generates a name starting with 'unique' and
mentioning the calling source location.
"""
@@ -2043,7 +2043,8 @@
makeBlueprint = makeSpecification
- def makeQuestion(self, target=None, title=None, owner=None):
+ def makeQuestion(self, target=None, title=None,
+ owner=None, description=None):
"""Create and return a new, arbitrary Question.
:param target: The IQuestionTarget to make the question on. If one is
@@ -2059,9 +2060,11 @@
title = self.getUniqueString('title')
if owner is None:
owner = target.owner
+ if description is None:
+ description = self.getUniqueString('description')
with person_logged_in(target.owner):
question = target.newQuestion(
- owner=owner, title=title, description='description')
+ owner=owner, title=title, description=description)
return question
def makeFAQ(self, target=None, title=None):
Follow ups
