Launchpad itself

Merge lp:~sinzui/launchpad/spam-eggs-bug-495250 into lp:launchpad

  1. spam-eggs-bug-495250
  2. Merge into devel
Proposed by Curtis Hovey
Status: Merged
Merged at revision: not available
Proposed branch: lp:~sinzui/launchpad/spam-eggs-bug-495250
Merge into: lp:launchpad
Diff against target: 358 lines (+183/-15)
9 files modified
lib/lp/registry/browser/person.py (+42/-3)
lib/lp/registry/browser/tests/karmaaction-views.txt (+1/-1)
lib/lp/registry/browser/tests/person-views.txt (+121/-4)
lib/lp/registry/browser/tests/poll-views.txt (+1/-1)
lib/lp/registry/stories/person/xx-admin-person-review.txt (+4/-0)
lib/lp/registry/stories/person/xx-login.txt (+2/-0)
lib/lp/registry/stories/person/xx-reg-with-existing-email.txt (+5/-0)
lib/lp/registry/templates/person-index.pt (+3/-3)
lib/lp/testing/views.py (+4/-3)
To merge this branch: bzr merge lp:~sinzui/launchpad/spam-eggs-bug-495250
Reviewer Review Type Date Requested Status
Данило Шеган (community) release-critical Abstain
Henning Eggers (community) Approve
Review via email: mp+16174@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

This is my branch to discourage spammers.

    lp:~sinzui/launchpad/spam-eggs-bug-495250
    Diff size: 282
    Launchpad bug: https://bugs.launchpad.net/bugs/495250
    Test command: ./bin/test -vvt "person-views"
    Pre-implementation: flacoste, gary
    Target release: 3.1.12

= Discourage spammers =

Do not link the homepage_content of probationary users. Do not let search
engine index probationary user' pages.

== Rules ==

    * All users with karma == 0 are probationary.
    * Set the robots directive to noindex,nofollow
    * Do not linkify the homepage_content.
    * Set the HTTP status code to 410 on profile pages for suspended accounts.

== QA ==

    * Locate a suspended user (search answers)
      * Verify the request response headers are set the status code to 410.
    * Locate an account with zero karma.
      * Verify that the profile page meta robots tag is index,nofollow.
    * Locate an account with zero karma and a description (probably a spammer)
      * Verify the homepage text is not formated as HTML.

== Lint ==

Linting changed files:
  lib/lp/registry/browser/person.py
  lib/lp/registry/browser/tests/person-views.txt
  lib/lp/registry/templates/person-index.pt

== Test ==

    * lib/lp/registry/browser/tests/person-views.txt
      * Added tests to verify is_probationary_or_invalid_user and
        homepage_content properties on the PersonView.
      * Add a test to verify that the HTTP status code is set to 410
        for suspended accounts.

== Implementation ==

    * lib/lp/registry/browser/person.py
      * Added is_probationary_or_invalid_user and homepage_content properties
        to control how to display the profile page for probationary users.
      * Redefined the render() method to set the status code to 410 when the
        account is suspended.
    * lib/lp/registry/templates/person-index.pt
      * Updated the template to use the view instead of the context object
        to show content and set the meta data.
    * lib/lp/testing/views.py
      * Discovered that path_info was not used. I updated the helper to
        use it.

Revision history for this message
Henning Eggers (henninge) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 15.12.2009 00:00, Curtis Hovey schrieb:
> Curtis Hovey has proposed merging lp:~sinzui/launchpad/spam-eggs-bug-495250 into lp:launchpad/devel.

> This is my branch to discourage spammers.

Yeah! Cool stuff! Thanks for the quick and smart fix.

 review approve

All I found was a little glitch in a comment...

> === modified file 'lib/lp/registry/browser/person.py'

Straightforward implementation, clear comments, good code. ;)

> === modified file 'lib/lp/registry/browser/tests/person-views.txt'
> --- lib/lp/registry/browser/tests/person-views.txt 2009-11-20 18:44:57 +0000
> +++ lib/lp/registry/browser/tests/person-views.txt 2009-12-14 23:00:33 +0000
> @@ -4,7 +4,127 @@
> person's information.
>
>
> -== Email address disclosure ==
> +Probationary and invalid users
> +------------------------------
> +
> +The person +index view provides the is_probationary_or_invalid_user so that
> +page features can be disabled because the user may abuse them. Active
> +users with karma are not on probation. The user's homepage_content formatted
> +as HTML

Add an "is" and a ".", please. ;)

[...]

> === modified file 'lib/lp/registry/templates/person-index.pt'
As expected.

> === modified file 'lib/lp/testing/views.py'
Thanks for the drive-by fix.

Cheers,
Henning

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAksnW2cACgkQBT3oW1L17iigywCcDytaSGCqRg0vliB5hMTqAOT0
x7IAn2Pb+6N0J+kAeoBbYvvltNGPfxnV
=qBEM
-----END PGP SIGNATURE-----

review: Approve
Revision history for this message
Данило Шеган (danilo) wrote :

As agreed on IRC, let's have this QAd a bit first (perhaps by cowboying, or landing right after the rollout as an RC=danilo) and then include it in the re-roll or CP it if there's no re-roll.

review: Abstain (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2009-12-08 10:20:37 +0000
+++ lib/lp/registry/browser/person.py 2009-12-15 20:35:30 +0000
@@ -226,7 +226,8 @@
226 logoutPerson, allowUnauthenticatedSession)226 logoutPerson, allowUnauthenticatedSession)
227from canonical.launchpad.webapp.menu import get_current_view227from canonical.launchpad.webapp.menu import get_current_view
228from canonical.launchpad.webapp.publisher import LaunchpadView228from canonical.launchpad.webapp.publisher import LaunchpadView
229from canonical.launchpad.webapp.tales import DateTimeFormatterAPI229from canonical.launchpad.webapp.tales import (
230 DateTimeFormatterAPI, FormattersAPI)
230from lazr.uri import URI, InvalidURIError231from lazr.uri import URI, InvalidURIError
231232
232from canonical.launchpad import _233from canonical.launchpad import _
@@ -2460,7 +2461,7 @@
24602461
2461 @property2462 @property
2462 def next_url(self):2463 def next_url(self):
2463 """Redirect back to the +languages page if request originated there."""2464 """Redirect to the +languages page if request originated there."""
2464 redirection_url = self.request.get('redirection_url')2465 redirection_url = self.request.get('redirection_url')
2465 if redirection_url:2466 if redirection_url:
2466 return redirection_url2467 return redirection_url
@@ -2468,7 +2469,7 @@
24682469
2469 @property2470 @property
2470 def cancel_url(self):2471 def cancel_url(self):
2471 """Redirect back to the +languages page if request originated there."""2472 """Redirect to the +languages page if request originated there."""
2472 redirection_url = self.getRedirectionURL()2473 redirection_url = self.getRedirectionURL()
2473 if redirection_url:2474 if redirection_url:
2474 return redirection_url2475 return redirection_url
@@ -2663,6 +2664,38 @@
2663 check_permission('launchpad.Edit', self.context))2664 check_permission('launchpad.Edit', self.context))
26642665
2665 @cachedproperty2666 @cachedproperty
2667 def is_probationary_or_invalid_user(self):
2668 """True when the user is not active or does not have karma.
2669
2670 Some content should not be rendered when the context is not a an
2671 established user. For example, probationary and invalid user pages
2672 must not be indexed by search engines and their narrative linkified.
2673 """
2674 user = self.context
2675 if user.isTeam():
2676 # Teams are always valid and do not have probationary rules.
2677 return False
2678 else:
2679 return user.karma == 0 or not user.is_valid_person
2680
2681 @cachedproperty
2682 def homepage_content(self):
2683 """The user's HTML formatted homepage content.
2684
2685 The markup is simply escaped for probationary or invalid users.
2686 The homepage content is reformatted as HTML and linkified if the user
2687 is active.
2688 """
2689 content = self.context.homepage_content
2690 if content is None:
2691 return None
2692 elif self.is_probationary_or_invalid_user:
2693 return cgi.escape(content)
2694 else:
2695 formatter = FormattersAPI
2696 return formatter(content).text_to_html()
2697
2698 @cachedproperty
2666 def recently_approved_members(self):2699 def recently_approved_members(self):
2667 members = self.context.getMembersByStatus(2700 members = self.context.getMembersByStatus(
2668 TeamMembershipStatus.APPROVED,2701 TeamMembershipStatus.APPROVED,
@@ -3137,6 +3170,12 @@
3137 if self.request.method == "POST":3170 if self.request.method == "POST":
3138 self.processForm()3171 self.processForm()
31393172
3173 def render(self):
3174 """See `LaunchpadView`."""
3175 if self.context.account_status == AccountStatus.SUSPENDED:
3176 self.request.response.setStatus(410)
3177 return super(PersonIndexView, self).render()
3178
3140 @property3179 @property
3141 def page_title(self):3180 def page_title(self):
3142 context = self.context3181 context = self.context
31433182
=== modified file 'lib/lp/registry/browser/tests/karmaaction-views.txt'
--- lib/lp/registry/browser/tests/karmaaction-views.txt 2009-08-17 16:18:51 +0000
+++ lib/lp/registry/browser/tests/karmaaction-views.txt 2009-12-15 20:35:30 +0000
@@ -17,7 +17,7 @@
1717
18 >>> user = getUtility(ILaunchBag).user18 >>> user = getUtility(ILaunchBag).user
19 >>> view = create_view(karmaactionset, '+index', principal=user,19 >>> view = create_view(karmaactionset, '+index', principal=user,
20 ... PATH_INFO='/karmaaction')20 ... path_info='/karmaaction')
21 >>> print extract_text(find_tag_by_id(view(), 'karmas'))21 >>> print extract_text(find_tag_by_id(view(), 'karmas'))
22 Category Action Points22 Category Action Points
23 bugs Bug Accepted 523 bugs Bug Accepted 5
2424
=== modified file 'lib/lp/registry/browser/tests/person-views.txt'
--- lib/lp/registry/browser/tests/person-views.txt 2009-11-20 18:44:57 +0000
+++ lib/lp/registry/browser/tests/person-views.txt 2009-12-15 20:35:30 +0000
@@ -4,7 +4,127 @@
4person's information.4person's information.
55
66
7== Email address disclosure ==7Probationary and invalid users
8------------------------------
9
10The person +index view provides the is_probationary_or_invalid_user so that
11page features can be disabled because the user may abuse them. Active
12users with karma are not on probation; the user's homepage_content is
13formatted as HTML.
14
15 >>> from lp.registry.interfaces.person import IPersonSet
16
17 >>> homepage_content = "line one <script>\n\nhttp://aa.aa/"
18 >>> person_set = getUtility(IPersonSet)
19 >>> active_user = person_set.getByName('name12')
20 >>> login_person(active_user)
21 >>> active_user.homepage_content = homepage_content
22 >>> login(ANONYMOUS)
23 >>> view = create_initialized_view(active_user, '+index')
24 >>> view.is_probationary_or_invalid_user
25 False
26
27 >>> print view.homepage_content
28 <p>line one &lt;script&gt;</p>
29 <BLANKLINE>
30 <p><a rel="nofollow" href="http://aa.aa/">http://<wbr></wbr>aa.aa/</a></p>
31
32Teams are always valid and do not have probation rules; the homepage content
33is formatted HTML.
34
35 >>> team = factory.makeTeam()
36 >>> login_person(team.teamowner)
37 >>> team.homepage_content = homepage_content
38 >>> login(ANONYMOUS)
39 >>> view = create_initialized_view(team, '+index')
40 >>> view.is_probationary_or_invalid_user
41 False
42
43 >>> print view.homepage_content
44 <p>line one &lt;script&gt;</p>
45 <BLANKLINE>
46 <p><a rel="nofollow" href="http://aa.aa/">http://<wbr></wbr>aa.aa/</a></p>
47
48New users (those without karma) are on probation; the homepage content is
49escaped HTML.
50
51 >>> new_user = factory.makePerson()
52 >>> new_user.homepage_content = homepage_content
53 >>> new_user.karma
54 0
55
56 >>> view = create_initialized_view(new_user, '+index')
57 >>> view.is_probationary_or_invalid_user
58 True
59
60 >>> print view.homepage_content
61 line one &lt;script&gt;
62 <BLANKLINE>
63 http://aa.aa/
64
65Inactive and suspended users are invalid; the homepage content is escaped
66HTML.
67
68 >>> from canonical.launchpad.interfaces.account import AccountStatus
69 >>> from canonical.launchpad.interfaces import IMasterObject
70
71 # Only admins can change an account.
72 >>> admin_user = person_set.getByName('name16')
73 >>> login_person(admin_user)
74 >>> invalid_user = factory.makePerson(name="ugh")
75 >>> invalid_user.homepage_content = homepage_content
76 >>> IMasterObject(invalid_user.account).status = AccountStatus.NOACCOUNT
77 >>> view = create_initialized_view(invalid_user, '+index')
78 >>> view.is_probationary_or_invalid_user
79 True
80
81 >>> print view.homepage_content
82 line one &lt;script&gt;
83 <BLANKLINE>
84 http://aa.aa/
85
86 >>> login(ANONYMOUS)
87
88If the user has no homepage content, the view's value is None.
89
90 >>> new_user.homepage_content = None
91 >>> view = create_initialized_view(new_user, '+index')
92 >>> print view.homepage_content
93 None
94
95
96Account status and profile page status codes
97--------------------------------------------
98
99The PersonIndexView sets the HTTP status code is 410 when the profile is for
100a suspended account.
101
102 # Only admins can change an account.
103 >>> login_person(active_user)
104 >>> IMasterObject(invalid_user.account).status = AccountStatus.SUSPENDED
105 >>> login(ANONYMOUS)
106 >>> view = create_initialized_view(
107 ... invalid_user, '+index', principal=ANONYMOUS,
108 ... server_url='http://launchpad.dev',
109 ... path_info="/~%s" % invalid_user.name)
110
111 >>> content = view()
112 >>> view.request.response.getStatus()
113 410
114
115Otherwise it does nothing (which in Zope's case the response is 599).
116
117 >>> view = create_initialized_view(
118 ... active_user, '+index', principal=ANONYMOUS,
119 ... server_url='http://launchpad.dev',
120 ... path_info="/~%s" % active_user.name)
121 >>> content = view()
122 >>> view.request.response.getStatus()
123 599
124
125
126Email address disclosure
127------------------------
8128
9PersonView is the base for many views for Person objects, including the129PersonView is the base for many views for Person objects, including the
10default view. It provides several properties to help display email130default view. It provides several properties to help display email
@@ -19,9 +139,6 @@
19Mark has a registered email address, and he has chosen to disclose it to139Mark has a registered email address, and he has chosen to disclose it to
20the world.140the world.
21141
22 >>> from lp.registry.interfaces.person import IPersonSet
23
24 >>> person_set = getUtility(IPersonSet)
25 >>> mark = person_set.getByEmail('mark@example.com')142 >>> mark = person_set.getByEmail('mark@example.com')
26 >>> mark.preferredemail.email143 >>> mark.preferredemail.email
27 u'mark@example.com'144 u'mark@example.com'
28145
=== modified file 'lib/lp/registry/browser/tests/poll-views.txt'
--- lib/lp/registry/browser/tests/poll-views.txt 2009-11-20 14:06:04 +0000
+++ lib/lp/registry/browser/tests/poll-views.txt 2009-12-15 20:35:30 +0000
@@ -16,7 +16,7 @@
16 ... server_url = 'http://launchpad.dev'16 ... server_url = 'http://launchpad.dev'
17 ... view = create_view(17 ... view = create_view(
18 ... team, name=name, principal=principal,18 ... team, name=name, principal=principal,
19 ... server_url=server_url, PATH_INFO=path_info)19 ... server_url=server_url, path_info=path_info)
20 ... view.initialize()20 ... view.initialize()
21 ... return view21 ... return view
2222
2323
=== modified file 'lib/lp/registry/stories/person/xx-admin-person-review.txt'
--- lib/lp/registry/stories/person/xx-admin-person-review.txt 2009-09-18 15:24:30 +0000
+++ lib/lp/registry/stories/person/xx-admin-person-review.txt 2009-12-15 20:35:30 +0000
@@ -58,6 +58,10 @@
58 >>> admin_browser.getControl(58 >>> admin_browser.getControl(
59 ... name='field.status_comment').value = 'Bad boy.'59 ... name='field.status_comment').value = 'Bad boy.'
60 >>> admin_browser.getControl('Change').click()60 >>> admin_browser.getControl('Change').click()
61 Traceback (most recent call last):
62 ...
63 HTTPError: HTTP Error 410: Gone
64
61 >>> print admin_browser.title65 >>> print admin_browser.title
62 The one and only Salgado does not use Launchpad66 The one and only Salgado does not use Launchpad
6367
6468
=== modified file 'lib/lp/registry/stories/person/xx-login.txt'
--- lib/lp/registry/stories/person/xx-login.txt 2009-09-11 20:12:19 +0000
+++ lib/lp/registry/stories/person/xx-login.txt 2009-12-15 20:35:30 +0000
@@ -57,6 +57,8 @@
57when he visits his profile page.57when he visits his profile page.
5858
59 >>> browser = setupBrowser()59 >>> browser = setupBrowser()
60 >>> browser.handleErrors = True
61 >>> browser.raiseHttpErrors = False
60 >>> browser.open('http://launchpad.dev/~bad-user')62 >>> browser.open('http://launchpad.dev/~bad-user')
61 >>> print browser.title63 >>> print browser.title
62 Bad-user does not use Launchpad64 Bad-user does not use Launchpad
6365
=== modified file 'lib/lp/registry/stories/person/xx-reg-with-existing-email.txt'
--- lib/lp/registry/stories/person/xx-reg-with-existing-email.txt 2009-10-16 16:13:00 +0000
+++ lib/lp/registry/stories/person/xx-reg-with-existing-email.txt 2009-12-15 20:35:30 +0000
@@ -134,6 +134,8 @@
134when he visits his profile page.134when he visits his profile page.
135135
136 >>> browser = setupBrowser()136 >>> browser = setupBrowser()
137 >>> browser.handleErrors = True
138 >>> browser.raiseHttpErrors = False
137 >>> browser.open('http://launchpad.dev/~bad-user')139 >>> browser.open('http://launchpad.dev/~bad-user')
138 >>> browser.title140 >>> browser.title
139 'Bad-user does not use Launchpad'141 'Bad-user does not use Launchpad'
@@ -177,6 +179,9 @@
177 >>> browser.getControl(name='field.password').value = 'test'179 >>> browser.getControl(name='field.password').value = 'test'
178 >>> browser.getControl(name='field.password_dupe').value = 'test'180 >>> browser.getControl(name='field.password_dupe').value = 'test'
179 >>> browser.getControl('Continue').click()181 >>> browser.getControl('Continue').click()
182 Traceback (most recent call last):
183 ...
184 HTTPError: HTTP Error 410: Gone
180185
181 >>> for tag in find_tags_by_class(186 >>> for tag in find_tags_by_class(
182 ... browser.contents, 'warning'):187 ... browser.contents, 'warning'):
183188
=== modified file 'lib/lp/registry/templates/person-index.pt'
--- lib/lp/registry/templates/person-index.pt 2009-11-20 16:37:51 +0000
+++ lib/lp/registry/templates/person-index.pt 2009-12-15 20:35:30 +0000
@@ -16,7 +16,7 @@
16 title="FOAF" href="+rdf"16 title="FOAF" href="+rdf"
17 />17 />
18 </tal:valid_person_or_team>18 </tal:valid_person_or_team>
19 <meta tal:condition="not: context/is_valid_person_or_team"19 <meta tal:condition="view/is_probationary_or_invalid_user"
20 name="robots" content="noindex,nofollow" />20 name="robots" content="noindex,nofollow" />
21 <tal:openid_delegation condition="view/is_delegated_identity">21 <tal:openid_delegation condition="view/is_delegated_identity">
22 <link rel="openid.server"22 <link rel="openid.server"
@@ -84,8 +84,8 @@
8484
85 <div85 <div
86 class="description"86 class="description"
87 tal:condition="context/homepage_content"87 tal:condition="view/homepage_content"
88 tal:content="structure context/homepage_content/fmt:text-to-html"88 tal:content="structure view/homepage_content"
89 />89 />
90 <ul class="horizontal">90 <ul class="horizontal">
91 <tal:comment condition="nothing">91 <tal:comment condition="nothing">
9292
=== modified file 'lib/lp/testing/views.py'
--- lib/lp/testing/views.py 2009-10-05 20:31:28 +0000
+++ lib/lp/testing/views.py 2009-12-15 20:35:30 +0000
@@ -43,7 +43,7 @@
43 if request is None:43 if request is None:
44 request = LaunchpadTestRequest(44 request = LaunchpadTestRequest(
45 form=form, SERVER_URL=server_url, QUERY_STRING=query_string,45 form=form, SERVER_URL=server_url, QUERY_STRING=query_string,
46 HTTP_COOKIE=cookie, method=method, **kwargs)46 HTTP_COOKIE=cookie, method=method, PATH_INFO=path_info, **kwargs)
47 if principal is not None:47 if principal is not None:
48 request.setPrincipal(principal)48 request.setPrincipal(principal)
49 else:49 else:
@@ -59,7 +59,8 @@
5959
60def create_initialized_view(context, name, form=None, layer=None,60def create_initialized_view(context, name, form=None, layer=None,
61 server_url=None, method=None, principal=None,61 server_url=None, method=None, principal=None,
62 query_string=None, cookie=None, request=None):62 query_string=None, cookie=None, request=None,
63 path_info='/'):
63 """Return a view that has already been initialized."""64 """Return a view that has already been initialized."""
64 if method is None:65 if method is None:
65 if form is None:66 if form is None:
@@ -68,6 +69,6 @@
68 method = 'POST'69 method = 'POST'
69 view = create_view(70 view = create_view(
70 context, name, form, layer, server_url, method, principal,71 context, name, form, layer, server_url, method, principal,
71 query_string, cookie, request)72 query_string, cookie, request, path_info)
72 view.initialize()73 view.initialize()
73 return view74 return view