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 |
Related bugs: |
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:/
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?
Description of the change
Jim Popovitch (jimpop) wrote : | # |
Mark Sapiro (msapiro) wrote : | # |
Thanks for this. I am currently traveling and I will review it in detail as I have time.
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).
- 1816. By Ralf Jung <email address hidden>
-
fix computing the form hash when there is no CAPTCHA
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?
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.
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:/
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". :)
Mark Sapiro (msapiro) wrote : | # |
Sorry about the misspelling of your name (I did get it right in the commit message). Fixed now.
Preview Diff
1 | === modified file 'Mailman/Cgi/listinfo.py' | |||
2 | --- Mailman/Cgi/listinfo.py 2018-06-21 16:23:09 +0000 | |||
3 | +++ Mailman/Cgi/listinfo.py 2019-06-10 20:07:09 +0000 | |||
4 | @@ -216,10 +216,25 @@ | |||
5 | 216 | # drop one : resulting in an invalid format, but it's only | 216 | # drop one : resulting in an invalid format, but it's only |
6 | 217 | # for our hash so it doesn't matter. | 217 | # for our hash so it doesn't matter. |
7 | 218 | remote = remote.rsplit(':', 1)[0] | 218 | remote = remote.rsplit(':', 1)[0] |
8 | 219 | # render CAPTCHA, if configured | ||
9 | 220 | if isinstance(mm_cfg.CAPTCHAS, dict) and 'en' in mm_cfg.CAPTCHAS: | ||
10 | 221 | (captcha_question, captcha_box, captcha_idx) = \ | ||
11 | 222 | Utils.captcha_display(mlist, lang, mm_cfg.CAPTCHAS) | ||
12 | 223 | pre_question = _( | ||
13 | 224 | '''Please answer the following question to prove that | ||
14 | 225 | you are not a bot:''' | ||
15 | 226 | ) | ||
16 | 227 | replacements['<mm-captcha-ui>'] = ( | ||
17 | 228 | """<tr><td BGCOLOR="#dddddd">%s<br>%s</td><td>%s</td></tr>""" | ||
18 | 229 | % (pre_question, captcha_question, captcha_box)) | ||
19 | 230 | else: | ||
20 | 231 | captcha_idx = "" # just to have something to include in the hash below | ||
21 | 232 | # fill form | ||
22 | 219 | replacements['<mm-subscribe-form-start>'] += ( | 233 | replacements['<mm-subscribe-form-start>'] += ( |
25 | 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' |
26 | 221 | % (now, Utils.sha_new(mm_cfg.SUBSCRIBE_FORM_SECRET + ":" + | 235 | % (now, captcha_idx, Utils.sha_new(mm_cfg.SUBSCRIBE_FORM_SECRET + ":" + |
27 | 222 | now + ":" + | 236 | now + ":" + |
28 | 237 | captcha_idx + ":" + | ||
29 | 223 | mlist.internal_name() + ":" + | 238 | mlist.internal_name() + ":" + |
30 | 224 | remote | 239 | remote |
31 | 225 | ).hexdigest() | 240 | ).hexdigest() |
32 | 226 | 241 | ||
33 | === modified file 'Mailman/Cgi/subscribe.py' | |||
34 | --- Mailman/Cgi/subscribe.py 2018-06-17 23:47:34 +0000 | |||
35 | +++ Mailman/Cgi/subscribe.py 2019-06-10 20:07:09 +0000 | |||
36 | @@ -168,13 +168,14 @@ | |||
37 | 168 | # for our hash so it doesn't matter. | 168 | # for our hash so it doesn't matter. |
38 | 169 | remote1 = remote.rsplit(':', 1)[0] | 169 | remote1 = remote.rsplit(':', 1)[0] |
39 | 170 | try: | 170 | try: |
41 | 171 | ftime, fhash = cgidata.getfirst('sub_form_token', '').split(':') | 171 | ftime, fcaptcha_idx, fhash = cgidata.getfirst('sub_form_token', '').split(':') |
42 | 172 | then = int(ftime) | 172 | then = int(ftime) |
43 | 173 | except ValueError: | 173 | except ValueError: |
45 | 174 | ftime = fhash = '' | 174 | ftime = fcaptcha_idx = fhash = '' |
46 | 175 | then = 0 | 175 | then = 0 |
47 | 176 | token = Utils.sha_new(mm_cfg.SUBSCRIBE_FORM_SECRET + ":" + | 176 | token = Utils.sha_new(mm_cfg.SUBSCRIBE_FORM_SECRET + ":" + |
48 | 177 | ftime + ":" + | 177 | ftime + ":" + |
49 | 178 | fcaptcha_idx + ":" + | ||
50 | 178 | mlist.internal_name() + ":" + | 179 | mlist.internal_name() + ":" + |
51 | 179 | remote1).hexdigest() | 180 | remote1).hexdigest() |
52 | 180 | if ftime and now - then > mm_cfg.FORM_LIFETIME: | 181 | if ftime and now - then > mm_cfg.FORM_LIFETIME: |
53 | @@ -189,6 +190,11 @@ | |||
54 | 189 | results.append( | 190 | results.append( |
55 | 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.')) |
56 | 191 | results.append(_('You must GET the form before submitting it.')) | 192 | results.append(_('You must GET the form before submitting it.')) |
57 | 193 | # Check captcha | ||
58 | 194 | if isinstance(mm_cfg.CAPTCHAS, dict): | ||
59 | 195 | captcha_answer = cgidata.getvalue('captcha_answer', '') | ||
60 | 196 | if not Utils.captcha_verify(fcaptcha_idx, captcha_answer, mm_cfg.CAPTCHAS): | ||
61 | 197 | results.append(_('This was not the right answer to the CAPTCHA question.')) | ||
62 | 192 | # Was an attempt made to subscribe the list to itself? | 198 | # Was an attempt made to subscribe the list to itself? |
63 | 193 | if email == mlist.GetListEmail(): | 199 | if email == mlist.GetListEmail(): |
64 | 194 | syslog('mischief', 'Attempt to self subscribe %s: %s', email, remote) | 200 | syslog('mischief', 'Attempt to self subscribe %s: %s', email, remote) |
65 | 195 | 201 | ||
66 | === modified file 'Mailman/Defaults.py.in' | |||
67 | --- Mailman/Defaults.py.in 2019-05-22 00:10:40 +0000 | |||
68 | +++ Mailman/Defaults.py.in 2019-06-10 20:07:09 +0000 | |||
69 | @@ -131,6 +131,25 @@ | |||
70 | 131 | # test. | 131 | # test. |
71 | 132 | SUBSCRIBE_FORM_MIN_TIME = seconds(5) | 132 | SUBSCRIBE_FORM_MIN_TIME = seconds(5) |
72 | 133 | 133 | ||
73 | 134 | # Use a custom question-answer CAPTCHA to protect against subscription spam. | ||
74 | 135 | # Has no effect unless SUBSCRIBE_FORM_SECRET is set. | ||
75 | 136 | # Should be set to a dict mapping language keys to a list of pairs | ||
76 | 137 | # of questions and regexes for the answers, e.g. | ||
77 | 138 | # CAPTCHAS = { | ||
78 | 139 | # 'en': [ | ||
79 | 140 | # ('What is two times six?', '(12|twelve)'), | ||
80 | 141 | # ('What is this mailing list software called?', '[Mm]ailman'), | ||
81 | 142 | # ], | ||
82 | 143 | # 'de': [ | ||
83 | 144 | # ('Was ist 3 mal 6?', '(18|achtzehn)'), | ||
84 | 145 | # ], | ||
85 | 146 | # } | ||
86 | 147 | # The regular expression must match the full string, i.e., it is implicitly | ||
87 | 148 | # acting as if it had "^" in the beginning and "$" at the end. | ||
88 | 149 | # An 'en' key must be present and is used as fall-back if there are no questions | ||
89 | 150 | # for the currently set language. | ||
90 | 151 | CAPTCHAS = None | ||
91 | 152 | |||
92 | 134 | # Use Google reCAPTCHA to protect the subscription form from spam bots. The | 153 | # Use Google reCAPTCHA to protect the subscription form from spam bots. The |
93 | 135 | # following must be set to a pair of keys issued by the reCAPTCHA service at | 154 | # following must be set to a pair of keys issued by the reCAPTCHA service at |
94 | 136 | # https://www.google.com/recaptcha/admin | 155 | # https://www.google.com/recaptcha/admin |
95 | 137 | 156 | ||
96 | === modified file 'Mailman/Utils.py' | |||
97 | --- Mailman/Utils.py 2019-03-02 02:24:14 +0000 | |||
98 | +++ Mailman/Utils.py 2019-06-10 20:07:09 +0000 | |||
99 | @@ -1576,3 +1576,32 @@ | |||
100 | 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): |
101 | 1577 | return True | 1577 | return True |
102 | 1578 | return False | 1578 | return False |
103 | 1579 | |||
104 | 1580 | |||
105 | 1581 | def captcha_display(mlist, lang, captchas): | ||
106 | 1582 | """Returns a CAPTCHA question, the HTML for the answer box, and | ||
107 | 1583 | the data to be put into the CSRF token""" | ||
108 | 1584 | if not lang in captchas: | ||
109 | 1585 | lang = 'en' | ||
110 | 1586 | captchas = captchas[lang] | ||
111 | 1587 | idx = random.randrange(len(captchas)) | ||
112 | 1588 | question = captchas[idx][0] | ||
113 | 1589 | box_html = mlist.FormatBox('captcha_answer', size=30) | ||
114 | 1590 | # Remember to encode the language in the index so that we can get it out again! | ||
115 | 1591 | return (websafe(question), box_html, lang + "-" + str(idx)) | ||
116 | 1592 | |||
117 | 1593 | def captcha_verify(idx, given_answer, captchas): | ||
118 | 1594 | try: | ||
119 | 1595 | (lang, idx) = idx.split("-") | ||
120 | 1596 | idx = int(idx) | ||
121 | 1597 | except ValueError: | ||
122 | 1598 | return False | ||
123 | 1599 | if not lang in captchas: | ||
124 | 1600 | return False | ||
125 | 1601 | captchas = captchas[lang] | ||
126 | 1602 | if not idx in range(len(captchas)): | ||
127 | 1603 | return False | ||
128 | 1604 | # Check the given answer. | ||
129 | 1605 | # We append a `$` to emulate `re.fullmatch`. | ||
130 | 1606 | correct_answer_pattern = captchas[idx][1] + "$" | ||
131 | 1607 | return re.match(correct_answer_pattern, given_answer) | ||
132 | 1579 | 1608 | ||
133 | === modified file 'templates/de/listinfo.html' | |||
134 | --- templates/de/listinfo.html 2018-01-29 12:58:42 +0000 | |||
135 | +++ templates/de/listinfo.html 2019-06-10 20:07:09 +0000 | |||
136 | @@ -115,6 +115,7 @@ | |||
137 | 115 | </tr> | 115 | </tr> |
138 | 116 | <mm-digest-question-end> | 116 | <mm-digest-question-end> |
139 | 117 | <mm-recaptcha-ui> | 117 | <mm-recaptcha-ui> |
140 | 118 | <mm-captcha-ui> | ||
141 | 118 | <tr> | 119 | <tr> |
142 | 119 | <td colspan="3"> | 120 | <td colspan="3"> |
143 | 120 | <center><MM-Subscribe-Button></center> | 121 | <center><MM-Subscribe-Button></center> |
144 | 121 | 122 | ||
145 | === modified file 'templates/en/listinfo.html' | |||
146 | --- templates/en/listinfo.html 2018-01-29 12:58:42 +0000 | |||
147 | +++ templates/en/listinfo.html 2019-06-10 20:07:09 +0000 | |||
148 | @@ -116,6 +116,7 @@ | |||
149 | 116 | </tr> | 116 | </tr> |
150 | 117 | <mm-digest-question-end> | 117 | <mm-digest-question-end> |
151 | 118 | <mm-recaptcha-ui> | 118 | <mm-recaptcha-ui> |
152 | 119 | <mm-captcha-ui> | ||
153 | 119 | <tr> | 120 | <tr> |
154 | 120 | <td colspan="3"> | 121 | <td colspan="3"> |
155 | 121 | <center><MM-Subscribe-Button></center> | 122 | <center><MM-Subscribe-Button></center> |
156 | 122 | 123 | ||
157 | === modified file 'templates/fr/listinfo.html' | |||
158 | --- templates/fr/listinfo.html 2018-01-29 12:58:42 +0000 | |||
159 | +++ templates/fr/listinfo.html 2019-06-10 20:07:09 +0000 | |||
160 | @@ -119,6 +119,7 @@ | |||
161 | 119 | </tr> | 119 | </tr> |
162 | 120 | <mm-digest-question-end> | 120 | <mm-digest-question-end> |
163 | 121 | <mm-recaptcha-ui> | 121 | <mm-recaptcha-ui> |
164 | 122 | <mm-captcha-ui> | ||
165 | 122 | <tr> | 123 | <tr> |
166 | 123 | <td colspan="3"> | 124 | <td colspan="3"> |
167 | 124 | <center><MM-Subscribe-Button></center> | 125 | <center><MM-Subscribe-Button></center> |
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.