Canonical SSO provider

Merge lp:~canonical-isd-hackers/canonical-identity-provider/email-whitelist into lp:canonical-identity-provider/release

  1. email-whitelist
  2. Merge into trunk
Proposed by Michael Foord
Status: Merged
Approved by: Michael Nelson
Approved revision: no longer in the source branch.
Merged at revision: 194
Proposed branch: lp:~canonical-isd-hackers/canonical-identity-provider/email-whitelist
Merge into: lp:canonical-identity-provider/release
Diff against target: 498 lines (+170/-48)
12 files modified
identityprovider/api10/forms.py (+2/-2)
identityprovider/models/captcha.py (+14/-4)
identityprovider/schema.py (+3/-0)
identityprovider/tests/test_auth.py (+5/-6)
identityprovider/tests/test_captcha.py (+2/-2)
identityprovider/tests/test_command_cleanup.py (+2/-2)
identityprovider/tests/test_forms.py (+18/-0)
identityprovider/tests/test_models_captcha.py (+67/-26)
identityprovider/tests/test_views_ui.py (+53/-3)
identityprovider/views/ui.py (+2/-1)
requirements.txt (+1/-1)
setup.py (+1/-1)
To merge this branch: bzr merge lp:~canonical-isd-hackers/canonical-identity-provider/email-whitelist
Reviewer Review Type Date Requested Status
Michael Nelson (community) code Approve
Review via email: mp+69158@code.launchpad.net

Commit message

Email addresses can be whitelisted to bypass captcha verification. Whitelist emails by providing a list of regular expressions to match emails in the EMAIL_WHITELIST_REGEXP_LIST setting (captcha section).

Description of the change

Email addresses can be whitelisted to bypass captcha verification. Whitelist emails by providing a list of regular expressions to match emails in the EMAIL_WHITELIST_REGEXP_LIST setting (captcha section).

This mp adds an EMAIL_WHITELIST_REGEXP_LIST setting to the captcha section of the sso config.

captcha.verify now takes an email address and all patterns will be checked against the supplied email. If a match is found then captcha verification is bypassed.

For our test email(s) I suggest the following pattern:

    ^canonicaltest(?:\+.+)?@gmail\.com$

So in local.cfg add:

[captcha]
email_whitelist_regexp_list =
    ^canonicaltest(?:\+.+)?@gmail\.com$

All tests pass in this branch.

To post a comment you must log in.
Revision history for this message
Michael Nelson (michael.nelson) wrote :
Download full text (11.1 KiB)

Hi Michael. Excellent branch... the implementation and tests are all very clear.

The only potential danger I can see with this branch is the ability to add unsafe regexes, but I don't see anyway around that unless we restricted ourselves to gmail accounts (ie. usesd settings.GMAIL_IDENTIFIER_WHITELIST and automatically calculated the regex to allow identifier+whatever).

The only questions I've got below are for my own understanding or tiny style questions. I verified that all 611 tests pass.

> === modified file 'identityprovider/models/captcha.py'
> --- identityprovider/models/captcha.py 2011-06-30 04:55:13 +0000
> +++ identityprovider/models/captcha.py 2011-07-26 07:31:31 +0000
> @@ -190,3 +194,9 @@
> self._verified = False
> raise VerifyCaptchaError(self.response.traceback)
> return self._verified
> +
> + def check_whitelist(self, email):
> + for pattern in settings.EMAIL_WHITELIST_REGEXP_LIST:
> + if re.match(pattern, email):
> + return True
> + return False

So that's the crux of the branch right? Nice and simple :)

The only thought I had was whether it should be *outside* of captcha.verify(),
rather than being hidden within, but that's just a small point - nothing to
delay the branch landing over :)

>
> === modified file 'identityprovider/schema.py'
> --- identityprovider/schema.py 2011-07-12 09:41:08 +0000
> +++ identityprovider/schema.py 2011-07-26 07:31:31 +0000
> @@ -78,6 +78,9 @@
> captcha.captcha_public_key = StringConfigOption()
> captcha.captcha_private_key = StringConfigOption()
> captcha.disable_captcha_verification = BoolConfigOption()
> + captcha.email_whitelist_regexp_list = LinesConfigOption(
> + item=StringConfigOption(raw=True)

I wasn't sure at first why the raw=True was necessary, until I tried
repr('^canonicaltest(?:\+.+)?@gmail\.com$').

> + )
>
> # debug
> debug = ConfigSection()
>
> === modified file 'identityprovider/tests/test_auth.py'
> --- identityprovider/tests/test_auth.py 2011-07-15 15:17:19 +0000
> +++ identityprovider/tests/test_auth.py 2011-07-26 07:31:31 +0000

Great to fix up all the patch_object's :)

> === modified file 'identityprovider/tests/test_captcha.py'
> --- identityprovider/tests/test_captcha.py 2011-07-21 16:08:06 +0000
> +++ identityprovider/tests/test_captcha.py 2011-07-26 07:31:31 +0000
> === modified file 'identityprovider/tests/test_command_cleanup.py'
> --- identityprovider/tests/test_command_cleanup.py 2011-07-15 11:50:53 +0000
> +++ identityprovider/tests/test_command_cleanup.py 2011-07-26 07:31:31 +0000
> === modified file 'identityprovider/tests/test_forms.py'
> --- identityprovider/tests/test_forms.py 2011-07-15 11:50:53 +0000
> +++ identityprovider/tests/test_forms.py 2011-07-26 07:31:31 +0000
> @@ -18,6 +18,7 @@
> )
> from identityprovider.api10.forms import WebserviceCreateAccountForm
> from identityprovider.models.openidmodels import OpenIDRPConfig
> +from identityprovider.tests.utils import patch_settings
>
> from .utils import SSOBaseTestCase
>
> @@ -114,6 +115,23 @@
> self.assertTrue(form.is_valid())
> self.assertEqual(form.cleaned_da...

review: Approve (code)
Revision history for this message
Michael Foord (mfoord) wrote :

I agree with you about the danger of unsafe regexps. An accidental ".*" and captcha verification is always bypassed. :-o

> The only thought I had was whether it should be *outside* of captcha.verify(),
> rather than being hidden within, but that's just a small point - nothing to
> delay the branch landing over :)

Well, captcha.verify(...) is called from multiple places, so putting this into the verification seemed both "correct" (it is part of the verification) *and* simplest (only one place to change and update).

> Great to have the above, but I couldn't see why it was necessary given the
two tests below.

That was the test I wrote first. I think I was thinking that I could then just test .check_whitelist directly as I had already tested that verify used it. It didn't work out like that though.

I'm not entirely sure why we have dependencies in setup.py *and* in requirements.txt. I think it is because setup.py is used to build packages, but requirements.txt isn't.

> I don't see why you don't just use `with patch.object(Captcha, '_open')...`

Yeah, that would be better. I was just fixing some code that was a bit "odd". Just using patch.object would have been better...

Revision history for this message
Ricardo Kirkner (ricardokirkner) wrote :

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

>> === modified file 'setup.py'
>> --- setup.py 2011-07-06 17:36:56 +0000
>> +++ setup.py 2011-07-26 07:31:31 +0000
>> @@ -39,7 +39,7 @@
>> 'NoseDjango==0.8.1',
>> 'nosexcover',
>> 'coverage==3.4',
>> - 'mock==0.6.0',
>> + 'mock==0.7.2',
>
> Why is this even in the setup.py? I thought (probably wrongly) that mock or
> nose or wsgi-intercept needed to be in the requirements.txt for setting up the dev environment,
> but not in the setup.py, as there's no need for them to be installed? I assume
> that's wrong then (on my part)? Do python packages install their test
> dependencies so they can be tested after install? (is that possible with our
> packages?)

This I can explain. These dependencies are test dependencies, and marked
so (using tests_require).

Having them marked as test dependencies, does not require you to have
them installed for running/installing your product, but allows
setuptools to look for them/install them if necessary when running your
tests via "python setup.py test".
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk4uqS0ACgkQaHF+Qaymu6cPeQCdEjUK/n0pFqSLyYugqF93ucRD
xTUAnR+O1KaXze2NtCda/rZ3n2M+iQvq
=niKk
-----END PGP SIGNATURE-----

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'identityprovider/api10/forms.py'
--- identityprovider/api10/forms.py 2011-07-06 21:14:28 +0000
+++ identityprovider/api10/forms.py 2011-07-25 20:09:28 +0000
@@ -59,8 +59,8 @@
59 remote_ip = cleaned_data['remote_ip']59 remote_ip = cleaned_data['remote_ip']
6060
61 captcha = Captcha(captcha_id)61 captcha = Captcha(captcha_id)
6262 email = cleaned_data.get('email', '')
63 if captcha.verify(captcha_solution, remote_ip):63 if captcha.verify(captcha_solution, remote_ip, email):
64 return cleaned_data64 return cleaned_data
65 # not verified65 # not verified
66 raise forms.ValidationError(_("Wrong captcha solution."))66 raise forms.ValidationError(_("Wrong captcha solution."))
6767
=== modified file 'identityprovider/models/captcha.py'
--- identityprovider/models/captcha.py 2011-06-30 04:55:13 +0000
+++ identityprovider/models/captcha.py 2011-07-25 20:09:28 +0000
@@ -66,14 +66,15 @@
6666
67 As is verifying received solution:67 As is verifying received solution:
6868
69 >>> captcha = Captcha('captcha-id-received-from-client')69 >>> email = 'foo@email.com'
70 >>> captcha.verify("this-is-invalid-solution", ip_addr)70 >>> captcha = Captcha('captcha-id-received-from-client', email)
71 >>> captcha.verify("this-is-invalid-solution", ip_addr, email)
71 False72 False
7273
73 Once verified solution is cached, so calling again to .verify() method is74 Once verified solution is cached, so calling again to .verify() method is
74 very cheap (and returns same result):75 very cheap (and returns same result):
7576
76 >>> captcha.verify("this-is-invalid-solution", ip_addr)77 >>> captcha.verify("this-is-invalid-solution", ip_addr, email)
77 False78 False
7879
79 You can also get original response from reCaptcha:80 You can also get original response from reCaptcha:
@@ -159,7 +160,7 @@
159160
160 return CaptchaResponse(response.code, response, None)161 return CaptchaResponse(response.code, response, None)
161162
162 def verify(self, captcha_solution, remote_ip):163 def verify(self, captcha_solution, remote_ip, email):
163 if self._verified is not None:164 if self._verified is not None:
164 return self._verified165 return self._verified
165166
@@ -167,6 +168,9 @@
167 self.response = None168 self.response = None
168 return True169 return True
169170
171 if self.check_whitelist(email):
172 return True
173
170 if isinstance(captcha_solution, unicode):174 if isinstance(captcha_solution, unicode):
171 captcha_solution = captcha_solution.encode('utf-8')175 captcha_solution = captcha_solution.encode('utf-8')
172176
@@ -190,3 +194,9 @@
190 self._verified = False194 self._verified = False
191 raise VerifyCaptchaError(self.response.traceback)195 raise VerifyCaptchaError(self.response.traceback)
192 return self._verified196 return self._verified
197
198 def check_whitelist(self, email):
199 for pattern in settings.EMAIL_WHITELIST_REGEXP_LIST:
200 if re.match(pattern, email):
201 return True
202 return False
193203
=== modified file 'identityprovider/schema.py'
--- identityprovider/schema.py 2011-07-12 09:41:08 +0000
+++ identityprovider/schema.py 2011-07-25 20:09:28 +0000
@@ -78,6 +78,9 @@
78 captcha.captcha_public_key = StringConfigOption()78 captcha.captcha_public_key = StringConfigOption()
79 captcha.captcha_private_key = StringConfigOption()79 captcha.captcha_private_key = StringConfigOption()
80 captcha.disable_captcha_verification = BoolConfigOption()80 captcha.disable_captcha_verification = BoolConfigOption()
81 captcha.email_whitelist_regexp_list = LinesConfigOption(
82 item=StringConfigOption(raw=True)
83 )
8184
82 # debug85 # debug
83 debug = ConfigSection()86 debug = ConfigSection()
8487
=== modified file 'identityprovider/tests/test_auth.py'
--- identityprovider/tests/test_auth.py 2011-07-15 15:17:19 +0000
+++ identityprovider/tests/test_auth.py 2011-07-25 20:09:28 +0000
@@ -5,7 +5,6 @@
5from mock import (5from mock import (
6 Mock,6 Mock,
7 patch,7 patch,
8 patch_object,
9)8)
10from oauth_backend.models import Consumer9from oauth_backend.models import Consumer
11from piston.oauth import OAuthError as PistonOAuthError10from piston.oauth import OAuthError as PistonOAuthError
@@ -171,19 +170,19 @@
171170
172 def test_is_authenticated_invalid_request(self):171 def test_is_authenticated_invalid_request(self):
173 mock_is_valid_request = Mock(return_value=False)172 mock_is_valid_request = Mock(return_value=False)
174 with patch_object(self.auth, 'is_valid_request', mock_is_valid_request):173 with patch.object(self.auth, 'is_valid_request', mock_is_valid_request):
175 self.assertEqual(self.auth.is_authenticated(self.request), False)174 self.assertEqual(self.auth.is_authenticated(self.request), False)
176175
177 def test_is_authenticated_piston_oauth_error(self):176 def test_is_authenticated_piston_oauth_error(self):
178 mock_validate_token = Mock(side_effect=PistonOAuthError)177 mock_validate_token = Mock(side_effect=PistonOAuthError)
179 with patch_object(self.auth, 'validate_token', mock_validate_token):178 with patch.object(self.auth, 'validate_token', mock_validate_token):
180 self.assertEqual(self.auth.is_authenticated(self.request), False)179 self.assertEqual(self.auth.is_authenticated(self.request), False)
181180
182 def test_is_authenticated_no_consumer_token(self):181 def test_is_authenticated_no_consumer_token(self):
183 consumer, token, params = None, None, None182 consumer, token, params = None, None, None
184 mock_validate_token = Mock(return_value=(consumer, token, params))183 mock_validate_token = Mock(return_value=(consumer, token, params))
185 # authentication fails as there is no consumer and token in the request184 # authentication fails as there is no consumer and token in the request
186 with patch_object(self.auth, 'validate_token', mock_validate_token):185 with patch.object(self.auth, 'validate_token', mock_validate_token):
187 self.assertEqual(self.auth.is_authenticated(self.request), False)186 self.assertEqual(self.auth.is_authenticated(self.request), False)
188187
189 @patch('identityprovider.auth.Account.objects.get')188 @patch('identityprovider.auth.Account.objects.get')
@@ -194,7 +193,7 @@
194 consumer, token, params = Mock(), Mock(), None193 consumer, token, params = Mock(), Mock(), None
195 # make sure validate_token returns a consumer and a token194 # make sure validate_token returns a consumer and a token
196 mock_validate_token = Mock(return_value=(consumer, token, params))195 mock_validate_token = Mock(return_value=(consumer, token, params))
197 with patch_object(self.auth, 'validate_token', mock_validate_token):196 with patch.object(self.auth, 'validate_token', mock_validate_token):
198 self.assertEqual(self.auth.is_authenticated(self.request), False)197 self.assertEqual(self.auth.is_authenticated(self.request), False)
199198
200 @patch('identityprovider.auth.Account.objects.get')199 @patch('identityprovider.auth.Account.objects.get')
@@ -206,7 +205,7 @@
206 consumer, params = Mock(), None205 consumer, params = Mock(), None
207 # make sure validate_token returns a consumer and a token206 # make sure validate_token returns a consumer and a token
208 mock_validate_token = Mock(return_value=(consumer, mock_token, params))207 mock_validate_token = Mock(return_value=(consumer, mock_token, params))
209 with patch_object(self.auth, 'validate_token', mock_validate_token):208 with patch.object(self.auth, 'validate_token', mock_validate_token):
210 response = self.auth.is_authenticated(self.request)209 response = self.auth.is_authenticated(self.request)
211210
212 self.assertEqual(response, True)211 self.assertEqual(response, True)
213212
=== modified file 'identityprovider/tests/test_captcha.py'
--- identityprovider/tests/test_captcha.py 2011-07-21 16:08:06 +0000
+++ identityprovider/tests/test_captcha.py 2011-07-25 20:09:28 +0000
@@ -27,7 +27,7 @@
27 captcha = Captcha(None)27 captcha = Captcha(None)
28 captcha._open = Mock()28 captcha._open = Mock()
29 captcha._open.return_value.is_error = True29 captcha._open.return_value.is_error = True
30 captcha.verify(captcha_solution, 'foobar')30 captcha.verify(captcha_solution, 'foobar', '')
3131
32 captcha_response = mock_urlencode.call_args[0][0]['response']32 captcha_response = mock_urlencode.call_args[0][0]['response']
33 self.assertEqual(captcha_response, encoded_solution)33 self.assertEqual(captcha_response, encoded_solution)
@@ -59,7 +59,7 @@
59 def test_verify_is_short_circuited_when_disabled_in_settings(self):59 def test_verify_is_short_circuited_when_disabled_in_settings(self):
60 with patch_settings(DISABLE_CAPTCHA_VERIFICATION=True):60 with patch_settings(DISABLE_CAPTCHA_VERIFICATION=True):
61 captcha = Captcha('test')61 captcha = Captcha('test')
62 verify_result = captcha.verify('solution', '127.0.0.1')62 verify_result = captcha.verify('solution', '127.0.0.1', '')
6363
64 self.assertTrue(verify_result)64 self.assertTrue(verify_result)
65 self.assertTrue(captcha.response is None)65 self.assertTrue(captcha.response is None)
6666
=== modified file 'identityprovider/tests/test_command_cleanup.py'
--- identityprovider/tests/test_command_cleanup.py 2011-07-15 11:50:53 +0000
+++ identityprovider/tests/test_command_cleanup.py 2011-07-25 20:09:28 +0000
@@ -8,7 +8,7 @@
8from django.core.management import call_command8from django.core.management import call_command
9from django.db import connection9from django.db import connection
10from django.test import TestCase10from django.test import TestCase
11from mock import patch_object11from mock import patch
1212
13from identityprovider.models import OpenIDNonce13from identityprovider.models import OpenIDNonce
1414
@@ -34,7 +34,7 @@
34 self.populate()34 self.populate()
3535
36 mock_stdout = StringIO()36 mock_stdout = StringIO()
37 with patch_object(sys, 'stdout', mock_stdout):37 with patch.object(sys, 'stdout', mock_stdout):
38 call_command('cleanup', verbosity=0)38 call_command('cleanup', verbosity=0)
39 mock_stdout.seek(0)39 mock_stdout.seek(0)
40 self.assertTrue('No items selected to clean up.' in mock_stdout.read())40 self.assertTrue('No items selected to clean up.' in mock_stdout.read())
4141
=== modified file 'identityprovider/tests/test_forms.py'
--- identityprovider/tests/test_forms.py 2011-07-15 11:50:53 +0000
+++ identityprovider/tests/test_forms.py 2011-07-25 20:09:28 +0000
@@ -18,6 +18,7 @@
18)18)
19from identityprovider.api10.forms import WebserviceCreateAccountForm19from identityprovider.api10.forms import WebserviceCreateAccountForm
20from identityprovider.models.openidmodels import OpenIDRPConfig20from identityprovider.models.openidmodels import OpenIDRPConfig
21from identityprovider.tests.utils import patch_settings
2122
22from .utils import SSOBaseTestCase23from .utils import SSOBaseTestCase
2324
@@ -114,6 +115,23 @@
114 self.assertTrue(form.is_valid())115 self.assertTrue(form.is_valid())
115 self.assertEqual(form.cleaned_data['platform'], 'desktop')116 self.assertEqual(form.cleaned_data['platform'], 'desktop')
116117
118 def test_captcha_checked_for_whitelist(self):
119 data = {
120 'email': 'canonicaltest@gmail.com',
121 'password': 'password1A',
122 'captcha_id': '1',
123 'captcha_solution': '2',
124 'remote_ip': '127.0.0.1',
125 }
126 pattern = '^canonicaltest(?:\+.+)?@gmail\.com$'
127 overrides = dict(
128 DISABLE_CAPTCHA_VERIFICATION=False,
129 EMAIL_WHITELIST_REGEXP_LIST=[pattern],
130 )
131 with patch_settings(**overrides):
132 form = WebserviceCreateAccountForm(data)
133 self.assertTrue(form.is_valid())
134
117 def test_default_cleaned_validate_redirect_to(self):135 def test_default_cleaned_validate_redirect_to(self):
118 data = {136 data = {
119 'email': 'some@email.com',137 'email': 'some@email.com',
120138
=== modified file 'identityprovider/tests/test_models_captcha.py'
--- identityprovider/tests/test_models_captcha.py 2011-01-20 21:48:43 +0000
+++ identityprovider/tests/test_models_captcha.py 2011-07-25 20:09:28 +0000
@@ -4,7 +4,10 @@
4import unittest4import unittest
5import urllib25import urllib2
66
7from mock import patch_object7from mock import (
8 Mock,
9 patch,
10)
811
9from cStringIO import StringIO12from cStringIO import StringIO
10from django.conf import settings13from django.conf import settings
@@ -16,6 +19,8 @@
16 VerifyCaptchaError,19 VerifyCaptchaError,
17)20)
1821
22from identityprovider.tests.utils import patch_settings
23
1924
20class CaptchaResponseTestCase(unittest.TestCase):25class CaptchaResponseTestCase(unittest.TestCase):
2126
@@ -37,11 +42,11 @@
3742
38class CaptchaTestCase(unittest.TestCase):43class CaptchaTestCase(unittest.TestCase):
3944
40 @patch_object(Captcha, '_open')45 @patch.object(Captcha, '_open')
41 def test_new_captcha_error(self, mock_open):46 def test_new_captcha_error(self, mock_open):
42 self.assertRaises(NewCaptchaError, Captcha.new)47 self.assertRaises(NewCaptchaError, Captcha.new)
4348
44 @patch_object(Captcha, '_open')49 @patch.object(Captcha, '_open')
45 def test_new_captcha_no_challenge(self, mock_open):50 def test_new_captcha_no_challenge(self, mock_open):
46 try:51 try:
47 Captcha.new()52 Captcha.new()
@@ -70,23 +75,56 @@
70 def tearDown(self):75 def tearDown(self):
71 settings.DISABLE_CAPTCHA_VERIFICATION = self.old_DISABLE_CAPTCHA_VERIFICATION76 settings.DISABLE_CAPTCHA_VERIFICATION = self.old_DISABLE_CAPTCHA_VERIFICATION
7277
73 @patch_object(Captcha, '_open')78 @patch.object(Captcha, '_open')
79 def test_verify_calls_check_whitelist(self, mock_open):
80 captcha = Captcha(None)
81 captcha.check_whitelist = Mock(return_value=False)
82
83 result = captcha.verify(None, 'localhost', 'foo')
84 self.assertFalse(result)
85 self.assertTrue(mock_open.called)
86
87 captcha.check_whitelist.assert_called_with('foo')
88
89 @patch.object(Captcha, '_open')
90 def test_check_whitelist_match(self, mock_open):
91 captcha = Captcha(None)
92
93 regexps = ['not a match', '^foo$']
94 with patch_settings(
95 EMAIL_WHITELIST_REGEXP_LIST=regexps
96 ):
97 result = captcha.verify(None, 'localhost', 'foo')
98 self.assertTrue(result)
99 self.assertFalse(mock_open.called)
100
101 @patch.object(Captcha, '_open')
102 def test_check_whitelist_no_match(self, mock_open):
103 captcha = Captcha(None)
104
105 regexps = ['not a match', '^bar$']
106 with patch_settings(
107 EMAIL_WHITELIST_REGEXP_LIST=regexps
108 ):
109 result = captcha.verify(None, 'localhost', 'foo')
110 self.assertFalse(result)
111 self.assertTrue(mock_open.called)
112
113 @patch.object(Captcha, '_open')
74 def test_verify_already_verified(self, mock_open):114 def test_verify_already_verified(self, mock_open):
75 c = Captcha(None)115 c = Captcha(None)
76 c._verified = True116 c._verified = True
77 r = c.verify(None, None)117 r = c.verify(None, None, '')
78 self.assertTrue(r)118 self.assertTrue(r)
79119
80 @patch_object(Captcha, '_open')120 @patch.object(Captcha, '_open')
81 def test_verify_no_verification(self, mock_open):121 def test_verify_no_verification(self, mock_open):
82 settings.DISABLE_CAPTCHA_VERIFICATION = True122 with patch_settings(DISABLE_CAPTCHA_VERIFICATION=True):
83123 c = Captcha(None)
84 c = Captcha(None)124 r = c.verify(None, None, '')
85 r = c.verify(None, None)125 self.assertTrue(r)
86 self.assertTrue(r)126
87127 def test_verify_response_ok(self):
88 @patch_object(Captcha, '_open')
89 def test_verify_response_ok(self, mock_open):
90 @classmethod128 @classmethod
91 def mock_open(cls, request):129 def mock_open(cls, request):
92 r = CaptchaResponse(200, StringIO('true\nok'))130 r = CaptchaResponse(200, StringIO('true\nok'))
@@ -94,25 +132,28 @@
94 old_open = Captcha._open132 old_open = Captcha._open
95 Captcha._open = mock_open133 Captcha._open = mock_open
96134
97 c = Captcha.new()135 try:
98 r = c.verify(None, "localhost")136 c = Captcha.new()
99137 r = c.verify(None, "localhost", '')
100 self.assertTrue(r)138
101 self.assertEqual(c.message, 'ok')139 self.assertTrue(r)
102140 self.assertEqual(c.message, 'ok')
103 Captcha._open = old_open141 finally:
104142 Captcha._open = old_open
105 @patch_object(Captcha, '_open')143
144 @patch.object(Captcha, '_open')
106 def test_verify_no_captcha_id(self, mock_open):145 def test_verify_no_captcha_id(self, mock_open):
107 c = Captcha(None)146 c = Captcha(None)
108 r = c.verify(None, 'localhost')147 r = c.verify(None, 'localhost', '')
109148
110 self.assertFalse(r)149 self.assertFalse(r)
111 self.assertEqual(c.message, 'no-challenge')150 self.assertEqual(c.message, 'no-challenge')
112151
113 @patch_object(Captcha, '_open')152 @patch.object(Captcha, '_open')
114 def test_verify_error(self, mock_open):153 def test_verify_error(self, mock_open):
115 c = Captcha(None)154 c = Captcha(None)
116 c.captcha_id = 'challenge-id'155 c.captcha_id = 'challenge-id'
117156
118 self.assertRaises(VerifyCaptchaError, c.verify, None, 'localhost')157 self.assertRaises(
158 VerifyCaptchaError, c.verify, None, 'localhost', ''
159 )
119160
=== modified file 'identityprovider/tests/test_views_ui.py'
--- identityprovider/tests/test_views_ui.py 2011-07-19 14:21:42 +0000
+++ identityprovider/tests/test_views_ui.py 2011-07-25 20:09:28 +0000
@@ -7,7 +7,7 @@
7import urllib27import urllib2
8from urlparse import urlsplit8from urlparse import urlsplit
99
10from mock import patch_object, patch10from mock import patch
1111
12from django.conf import settings12from django.conf import settings
13from django.core import mail13from django.core import mail
@@ -30,6 +30,7 @@
30from identityprovider.tests.utils import (30from identityprovider.tests.utils import (
31 LPAccountTestCase,31 LPAccountTestCase,
32 MockHandler,32 MockHandler,
33 patch_settings,
33)34)
34from identityprovider.utils import is_django_1335from identityprovider.utils import is_django_13
3536
@@ -592,7 +593,7 @@
592 def __init__(self, *args):593 def __init__(self, *args):
593 pass594 pass
594595
595 def verify(self, solution, ip_addr):596 def verify(self, solution, ip_addr, email):
596 self.message = 'no-challenge'597 self.message = 'no-challenge'
597 return False598 return False
598599
@@ -635,7 +636,7 @@
635 self.assertEqual(len(mail.outbox), mails_sent)636 self.assertEqual(len(mail.outbox), mails_sent)
636 mail.outbox = []637 mail.outbox = []
637638
638 @patch_object(logging.Logger, 'debug')639 @patch.object(logging.Logger, 'debug')
639 def test_new_account_when_person_exists_and_account_not(self, mock_debug):640 def test_new_account_when_person_exists_and_account_not(self, mock_debug):
640 person = Person.objects.create(displayname='Person')641 person = Person.objects.create(displayname='Person')
641 EmailAddress.objects.create(email='person@example.com',642 EmailAddress.objects.create(email='person@example.com',
@@ -815,6 +816,55 @@
815 msg = 'It appears that our captcha service was unable to load'816 msg = 'It appears that our captcha service was unable to load'
816 self.assertContains(r, msg)817 self.assertContains(r, msg)
817818
819 def test_new_account_captcha_whitelist(self):
820 email = 'canonicaltest@gmail.com'
821 pattern = '^canonicaltest(?:\+.+)?@gmail\.com$'
822 overrides = dict(
823 DISABLE_CAPTCHA_VERIFICATION=False,
824 EMAIL_WHITELIST_REGEXP_LIST=[pattern],
825 )
826 with patch_settings(**overrides):
827 response = _post_new_account(
828 self.client,
829 email=email
830 )
831 # should redirect to '/+email-sent'
832 self.assertEqual(response.status_code, 302)
833 redirect = response['location']
834 self.assertTrue(redirect.endswith('/+email-sent'))
835
836 def test_new_account_captcha_whitelist_with_uuid(self):
837 email = 'canonicaltest+something@gmail.com'
838 pattern = '^canonicaltest(?:\+.+)?@gmail\.com$'
839 overrides = dict(
840 DISABLE_CAPTCHA_VERIFICATION=False,
841 EMAIL_WHITELIST_REGEXP_LIST=[pattern],
842 )
843 with patch_settings(**overrides):
844 response = _post_new_account(
845 self.client,
846 email=email
847 )
848 # should redirect to '/+email-sent'
849 self.assertEqual(response.status_code, 302)
850 redirect = response['location']
851 self.assertTrue(redirect.endswith('/+email-sent'))
852
853 def test_new_account_captcha_whitelist_fail(self):
854 email = 'notcanonicaltest@gmail.com'
855 pattern = '^canonicaltest(?:\+.+)?@gmail\.com$'
856 overrides = dict(
857 DISABLE_CAPTCHA_VERIFICATION=False,
858 EMAIL_WHITELIST_REGEXP_LIST=[pattern],
859 )
860 with patch_settings(**overrides):
861 response = _post_new_account(
862 self.client,
863 email=email
864 )
865 msg = 'It appears that our captcha service was unable to load'
866 self.assertContains(response, msg)
867
818868
819class LanguagesTestCase(SSOBaseTestCase):869class LanguagesTestCase(SSOBaseTestCase):
820870
821871
=== modified file 'identityprovider/views/ui.py'
--- identityprovider/views/ui.py 2011-07-21 16:08:06 +0000
+++ identityprovider/views/ui.py 2011-07-25 20:09:28 +0000
@@ -464,9 +464,10 @@
464def _verify_captcha_response(template, request, form):464def _verify_captcha_response(template, request, form):
465 captcha = Captcha(request.POST.get('recaptcha_challenge_field'))465 captcha = Captcha(request.POST.get('recaptcha_challenge_field'))
466 captcha_solution = request.POST.get('recaptcha_response_field')466 captcha_solution = request.POST.get('recaptcha_response_field')
467 email = request.POST.get('email', '')
467 ip_addr = request.environ["REMOTE_ADDR"]468 ip_addr = request.environ["REMOTE_ADDR"]
468 try:469 try:
469 verified = captcha.verify(captcha_solution, ip_addr)470 verified = captcha.verify(captcha_solution, ip_addr, email)
470 if verified:471 if verified:
471 return None472 return None
472 except VerifyCaptchaError, e:473 except VerifyCaptchaError, e:
473474
=== modified file 'requirements.txt'
--- requirements.txt 2011-07-12 14:38:02 +0000
+++ requirements.txt 2011-07-25 20:09:28 +0000
@@ -6,7 +6,7 @@
6NoseDjango==0.8.16NoseDjango==0.8.1
7nosexcover7nosexcover
8coverage==3.48coverage==3.4
9mock==0.6.09mock==0.7.2
10wsgiref==0.1.210wsgiref==0.1.2
11wsgi-intercept==0.411wsgi-intercept==0.4
12zope.testbrowser==3.5.112zope.testbrowser==3.5.1
1313
=== modified file 'setup.py'
--- setup.py 2011-07-06 17:36:56 +0000
+++ setup.py 2011-07-25 20:09:28 +0000
@@ -39,7 +39,7 @@
39 'NoseDjango==0.8.1',39 'NoseDjango==0.8.1',
40 'nosexcover',40 'nosexcover',
41 'coverage==3.4',41 'coverage==3.4',
42 'mock==0.6.0',42 'mock==0.7.2',
43 'wsgiref==0.1.2',43 'wsgiref==0.1.2',
44 'wsgi-intercept==0.4',44 'wsgi-intercept==0.4',
45 'zope.testbrowser==3.5.1',45 'zope.testbrowser==3.5.1',