Merge lp:~ralfjung-e/mailman/2.1 into lp:mailman/2.1

Proposed by Ralf Jung
Status: Merged
Merged at revision: 1815
Proposed branch: lp:~ralfjung-e/mailman/2.1
Merge into: lp:mailman/2.1
Diff against target: 167 lines (+76/-4)
7 files modified
Mailman/Cgi/listinfo.py (+17/-2)
Mailman/Cgi/subscribe.py (+8/-2)
Mailman/Defaults.py.in (+19/-0)
Mailman/Utils.py (+29/-0)
templates/de/listinfo.html (+1/-0)
templates/en/listinfo.html (+1/-0)
templates/fr/listinfo.html (+1/-0)
To merge this branch: bzr merge lp:~ralfjung-e/mailman/2.1
Reviewer Review Type Date Requested Status
Mark Sapiro Approve
Review via email: mp+368614@code.launchpad.net

Commit message

Implement a simple CAPTCHA scheme based on questions and answers configured by the site admin.

Fixes https://bugs.launchpad.net/mailman/+bug/1774826.

I have manually edited listinfo.html for a few languages for testing; is there a way to automate that or do I have to manually do that for all languages?

To post a comment you must log in.
Revision history for this message
Jim Popovitch (jimpop) wrote :

I like this solution, it's simple and easily customizable, doesn't rely on 3rd-party integrations, and the CAPTCHA questions can be easily updated. Nice work Ralf.

One thing that I would like to see in the future is storing the CAPTCHAs in pck or even sql, etc.

Revision history for this message
Mark Sapiro (msapiro) wrote :

Thanks for this. I am currently traveling and I will review it in detail as I have time.

Revision history for this message
Jim Popovitch (jimpop) wrote :

If line 20 (in the patch below) sets "captcha_idx = 0" python throws an error:

   TypeError: cannot concatenate 'str' and 'int' objects

I think a better solution would be to remove the captcha_idx from that section of code if it is None, (and set it to None at line 20).

lp:~ralfjung-e/mailman/2.1 updated
1816. By Ralf Jung <email address hidden>

fix computing the form hash when there is no CAPTCHA

Revision history for this message
Ralf Jung (ralfjung-e) wrote :

Ah good catch, I forgot to test this without CAPTCHAS set.

However, the variable is used later to compute the hash for the form secret. Removing it conditionally will mean we have to mirror the same logic on the other side in subscribe.py. So for now I changed it to use the empty string instead. I don't know if this will automatically get included in this MR?

lp:~ralfjung-e/mailman/2.1 updated
1817. By Ralf Jung <email address hidden>

Mention in the docs that 'en' is used as the default key

1818. By Ralf Jung <email address hidden>

Don't enable CAPTCHA if 'en' key is not set

Revision history for this message
Jim Popovitch (jimpop) wrote :

The recent change will be included in the MR, you can confirm this by looking at the diff down below these comments on webform for the MR.

Revision history for this message
Mark Sapiro (msapiro) wrote :

Thanks very much Ralf. I made a few minor style changes and added a NEWS item, but this is committed at https://bazaar.launchpad.net/~mailman-coders/mailman/2.1/revision/1815

review: Approve
Revision history for this message
Ralf Jung (ralfjung-e) wrote :

Great. :) I see you added <mm-captcha-ui> to all the templates, thanks!

Oh and could you please correct my name in NEWS? It's Ralf with an "f". :)

Revision history for this message
Mark Sapiro (msapiro) wrote :

Sorry about the misspelling of your name (I did get it right in the commit message). Fixed now.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Mailman/Cgi/listinfo.py'
--- Mailman/Cgi/listinfo.py 2018-06-21 16:23:09 +0000
+++ Mailman/Cgi/listinfo.py 2019-06-10 20:07:09 +0000
@@ -216,10 +216,25 @@
216 # drop one : resulting in an invalid format, but it's only216 # drop one : resulting in an invalid format, but it's only
217 # for our hash so it doesn't matter.217 # for our hash so it doesn't matter.
218 remote = remote.rsplit(':', 1)[0]218 remote = remote.rsplit(':', 1)[0]
219 # render CAPTCHA, if configured
220 if isinstance(mm_cfg.CAPTCHAS, dict) and 'en' in mm_cfg.CAPTCHAS:
221 (captcha_question, captcha_box, captcha_idx) = \
222 Utils.captcha_display(mlist, lang, mm_cfg.CAPTCHAS)
223 pre_question = _(
224 '''Please answer the following question to prove that
225 you are not a bot:'''
226 )
227 replacements['<mm-captcha-ui>'] = (
228 """<tr><td BGCOLOR="#dddddd">%s<br>%s</td><td>%s</td></tr>"""
229 % (pre_question, captcha_question, captcha_box))
230 else:
231 captcha_idx = "" # just to have something to include in the hash below
232 # fill form
219 replacements['<mm-subscribe-form-start>'] += (233 replacements['<mm-subscribe-form-start>'] += (
220 '<input type="hidden" name="sub_form_token" value="%s:%s">\n'234 '<input type="hidden" name="sub_form_token" value="%s:%s:%s">\n'
221 % (now, Utils.sha_new(mm_cfg.SUBSCRIBE_FORM_SECRET + ":" +235 % (now, captcha_idx, Utils.sha_new(mm_cfg.SUBSCRIBE_FORM_SECRET + ":" +
222 now + ":" +236 now + ":" +
237 captcha_idx + ":" +
223 mlist.internal_name() + ":" +238 mlist.internal_name() + ":" +
224 remote239 remote
225 ).hexdigest()240 ).hexdigest()
226241
=== modified file 'Mailman/Cgi/subscribe.py'
--- Mailman/Cgi/subscribe.py 2018-06-17 23:47:34 +0000
+++ Mailman/Cgi/subscribe.py 2019-06-10 20:07:09 +0000
@@ -168,13 +168,14 @@
168 # for our hash so it doesn't matter.168 # for our hash so it doesn't matter.
169 remote1 = remote.rsplit(':', 1)[0]169 remote1 = remote.rsplit(':', 1)[0]
170 try:170 try:
171 ftime, fhash = cgidata.getfirst('sub_form_token', '').split(':')171 ftime, fcaptcha_idx, fhash = cgidata.getfirst('sub_form_token', '').split(':')
172 then = int(ftime)172 then = int(ftime)
173 except ValueError:173 except ValueError:
174 ftime = fhash = ''174 ftime = fcaptcha_idx = fhash = ''
175 then = 0175 then = 0
176 token = Utils.sha_new(mm_cfg.SUBSCRIBE_FORM_SECRET + ":" +176 token = Utils.sha_new(mm_cfg.SUBSCRIBE_FORM_SECRET + ":" +
177 ftime + ":" +177 ftime + ":" +
178 fcaptcha_idx + ":" +
178 mlist.internal_name() + ":" +179 mlist.internal_name() + ":" +
179 remote1).hexdigest()180 remote1).hexdigest()
180 if ftime and now - then > mm_cfg.FORM_LIFETIME:181 if ftime and now - then > mm_cfg.FORM_LIFETIME:
@@ -189,6 +190,11 @@
189 results.append(190 results.append(
190 _('There was no hidden token in your submission or it was corrupted.'))191 _('There was no hidden token in your submission or it was corrupted.'))
191 results.append(_('You must GET the form before submitting it.'))192 results.append(_('You must GET the form before submitting it.'))
193 # Check captcha
194 if isinstance(mm_cfg.CAPTCHAS, dict):
195 captcha_answer = cgidata.getvalue('captcha_answer', '')
196 if not Utils.captcha_verify(fcaptcha_idx, captcha_answer, mm_cfg.CAPTCHAS):
197 results.append(_('This was not the right answer to the CAPTCHA question.'))
192 # Was an attempt made to subscribe the list to itself?198 # Was an attempt made to subscribe the list to itself?
193 if email == mlist.GetListEmail():199 if email == mlist.GetListEmail():
194 syslog('mischief', 'Attempt to self subscribe %s: %s', email, remote)200 syslog('mischief', 'Attempt to self subscribe %s: %s', email, remote)
195201
=== modified file 'Mailman/Defaults.py.in'
--- Mailman/Defaults.py.in 2019-05-22 00:10:40 +0000
+++ Mailman/Defaults.py.in 2019-06-10 20:07:09 +0000
@@ -131,6 +131,25 @@
131# test.131# test.
132SUBSCRIBE_FORM_MIN_TIME = seconds(5)132SUBSCRIBE_FORM_MIN_TIME = seconds(5)
133133
134# Use a custom question-answer CAPTCHA to protect against subscription spam.
135# Has no effect unless SUBSCRIBE_FORM_SECRET is set.
136# Should be set to a dict mapping language keys to a list of pairs
137# of questions and regexes for the answers, e.g.
138# CAPTCHAS = {
139# 'en': [
140# ('What is two times six?', '(12|twelve)'),
141# ('What is this mailing list software called?', '[Mm]ailman'),
142# ],
143# 'de': [
144# ('Was ist 3 mal 6?', '(18|achtzehn)'),
145# ],
146# }
147# The regular expression must match the full string, i.e., it is implicitly
148# acting as if it had "^" in the beginning and "$" at the end.
149# An 'en' key must be present and is used as fall-back if there are no questions
150# for the currently set language.
151CAPTCHAS = None
152
134# Use Google reCAPTCHA to protect the subscription form from spam bots. The153# Use Google reCAPTCHA to protect the subscription form from spam bots. The
135# following must be set to a pair of keys issued by the reCAPTCHA service at154# following must be set to a pair of keys issued by the reCAPTCHA service at
136# https://www.google.com/recaptcha/admin155# https://www.google.com/recaptcha/admin
137156
=== modified file 'Mailman/Utils.py'
--- Mailman/Utils.py 2019-03-02 02:24:14 +0000
+++ Mailman/Utils.py 2019-06-10 20:07:09 +0000
@@ -1576,3 +1576,32 @@
1576 if not re.search(r'127\.0\.1\.255$', text, re.MULTILINE):1576 if not re.search(r'127\.0\.1\.255$', text, re.MULTILINE):
1577 return True1577 return True
1578 return False1578 return False
1579
1580
1581def captcha_display(mlist, lang, captchas):
1582 """Returns a CAPTCHA question, the HTML for the answer box, and
1583 the data to be put into the CSRF token"""
1584 if not lang in captchas:
1585 lang = 'en'
1586 captchas = captchas[lang]
1587 idx = random.randrange(len(captchas))
1588 question = captchas[idx][0]
1589 box_html = mlist.FormatBox('captcha_answer', size=30)
1590 # Remember to encode the language in the index so that we can get it out again!
1591 return (websafe(question), box_html, lang + "-" + str(idx))
1592
1593def captcha_verify(idx, given_answer, captchas):
1594 try:
1595 (lang, idx) = idx.split("-")
1596 idx = int(idx)
1597 except ValueError:
1598 return False
1599 if not lang in captchas:
1600 return False
1601 captchas = captchas[lang]
1602 if not idx in range(len(captchas)):
1603 return False
1604 # Check the given answer.
1605 # We append a `$` to emulate `re.fullmatch`.
1606 correct_answer_pattern = captchas[idx][1] + "$"
1607 return re.match(correct_answer_pattern, given_answer)
15791608
=== modified file 'templates/de/listinfo.html'
--- templates/de/listinfo.html 2018-01-29 12:58:42 +0000
+++ templates/de/listinfo.html 2019-06-10 20:07:09 +0000
@@ -115,6 +115,7 @@
115 </tr>115 </tr>
116 <mm-digest-question-end>116 <mm-digest-question-end>
117 <mm-recaptcha-ui>117 <mm-recaptcha-ui>
118 <mm-captcha-ui>
118 <tr>119 <tr>
119 <td colspan="3">120 <td colspan="3">
120 <center><MM-Subscribe-Button></center>121 <center><MM-Subscribe-Button></center>
121122
=== modified file 'templates/en/listinfo.html'
--- templates/en/listinfo.html 2018-01-29 12:58:42 +0000
+++ templates/en/listinfo.html 2019-06-10 20:07:09 +0000
@@ -116,6 +116,7 @@
116 </tr>116 </tr>
117 <mm-digest-question-end>117 <mm-digest-question-end>
118 <mm-recaptcha-ui>118 <mm-recaptcha-ui>
119 <mm-captcha-ui>
119 <tr>120 <tr>
120 <td colspan="3">121 <td colspan="3">
121 <center><MM-Subscribe-Button></center>122 <center><MM-Subscribe-Button></center>
122123
=== modified file 'templates/fr/listinfo.html'
--- templates/fr/listinfo.html 2018-01-29 12:58:42 +0000
+++ templates/fr/listinfo.html 2019-06-10 20:07:09 +0000
@@ -119,6 +119,7 @@
119 </tr>119 </tr>
120 <mm-digest-question-end>120 <mm-digest-question-end>
121 <mm-recaptcha-ui>121 <mm-recaptcha-ui>
122 <mm-captcha-ui>
122 <tr>123 <tr>
123 <td colspan="3"> 124 <td colspan="3">
124 <center><MM-Subscribe-Button></center>125 <center><MM-Subscribe-Button></center>