Merge lp:~matiasb/canonical-identity-provider/validate-returns-is-verified into lp:canonical-identity-provider/release

Proposed by Matias Bordese
Status: Merged
Approved by: Ricardo Kirkner
Approved revision: no longer in the source branch.
Merged at revision: 1144
Proposed branch: lp:~matiasb/canonical-identity-provider/validate-returns-is-verified
Merge into: lp:canonical-identity-provider/release
Diff against target: 271 lines (+66/-29)
2 files modified
src/api/v20/handlers.py (+6/-5)
src/api/v20/tests/test_handlers.py (+60/-24)
To merge this branch: bzr merge lp:~matiasb/canonical-identity-provider/validate-returns-is-verified
Reviewer Review Type Date Requested Status
Ricardo Kirkner (community) Approve
Review via email: mp+226035@code.launchpad.net

Commit message

Added account_verified details to validate_request API endpoint.

To post a comment you must log in.
Revision history for this message
Ricardo Kirkner (ricardokirkner) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/api/v20/handlers.py'
2--- src/api/v20/handlers.py 2013-11-28 14:25:58 +0000
3+++ src/api/v20/handlers.py 2014-07-08 20:32:06 +0000
4@@ -458,7 +458,7 @@
5 if authorization is not None:
6 headers = {'Authorization': authorization}
7
8- result = {'is_valid': False}
9+ result = dict(is_valid=False, account_verified=False)
10
11 if not http_url or not http_method:
12 return result
13@@ -489,18 +489,19 @@
14 consumer, _, _ = oauth_server.verify_request(oauth_request)
15 except oauth.OAuthError:
16 # do nothing special, signature was invalid
17- valid = False
18+ pass
19 except Exception as e:
20 result['error'] = self.serialize_error(e)
21 logger.exception(
22 'RequestsHandler.create: could not verify request:')
23- valid = False
24 else:
25 # signature is valid, need to also check the account exists
26 # and is active
27- valid = Account.objects.active_by_openid(consumer.key) is not None
28+ account = Account.objects.active_by_openid(consumer.key)
29+ result['is_valid'] = account is not None
30+ result['account_verified'] = (
31+ account is not None and account.is_verified)
32
33- result['is_valid'] = valid
34 return result
35
36
37
38=== modified file 'src/api/v20/tests/test_handlers.py'
39--- src/api/v20/tests/test_handlers.py 2014-01-27 19:18:59 +0000
40+++ src/api/v20/tests/test_handlers.py 2014-07-08 20:32:06 +0000
41@@ -333,7 +333,8 @@
42 )
43 return data
44
45- def assert_request_response(self, response, expected, error=None):
46+ def assert_request_response(self, response, is_valid, account_verified,
47+ error=None):
48 self.assertEqual(response.status_code, 200)
49 try:
50 response = json.loads(response.content)
51@@ -341,7 +342,8 @@
52 self.fail('Could not JSON-decode response, content is %r.' %
53 response.content)
54 else:
55- expected = dict(is_valid=expected)
56+ expected = dict(is_valid=is_valid,
57+ account_verified=account_verified)
58 if error:
59 expected['error'] = error
60 self.assertEqual(response, expected)
61@@ -349,32 +351,37 @@
62 def test_invalid_if_not_method_given(self):
63 self.data.pop('http_method')
64 response = self.do_post()
65- self.assert_request_response(response, False)
66+ self.assert_request_response(
67+ response, is_valid=False, account_verified=False)
68 self.assertFalse(self.mock_logger.exception.called)
69
70 def test_invalid_if_method_none(self):
71 self.data['http_method'] = None
72 response = self.do_post()
73- self.assert_request_response(response, False)
74+ self.assert_request_response(
75+ response, is_valid=False, account_verified=False)
76 self.assertFalse(self.mock_logger.exception.called)
77
78 def test_invalid_if_not_url_given(self):
79 self.data.pop('http_url')
80 response = self.do_post()
81- self.assert_request_response(response, False)
82+ self.assert_request_response(
83+ response, is_valid=False, account_verified=False)
84 self.assertFalse(self.mock_logger.exception.called)
85
86 def test_invalid_if_url_none(self):
87 self.data['http_url'] = None
88 response = self.do_post()
89- self.assert_request_response(response, False)
90+ self.assert_request_response(
91+ response, is_valid=False, account_verified=False)
92 self.assertFalse(self.mock_logger.exception.called)
93
94 def test_url_encoded(self):
95 self.data = self.build_data(external_url=u'http://foo/ñoño ñandú/')
96 response = self.do_post()
97
98- self.assert_request_response(response, True)
99+ self.assert_request_response(
100+ response, is_valid=True, account_verified=True)
101 self.assertFalse(self.mock_logger.exception.called)
102
103 def test_error_in_response_is_robust_to_encodings(self):
104@@ -388,7 +395,8 @@
105
106 msg = ', '.join(repr(i) for i in exc.args)
107 self.assert_request_response(
108- response, False, error="TypeError: %s" % msg)
109+ response, is_valid=False, account_verified=False,
110+ error="TypeError: %s" % msg)
111
112 def test_invalid_if_from_request_fails(self):
113 self._apply_patch(
114@@ -397,7 +405,8 @@
115
116 response = self.do_post()
117 self.assert_request_response(
118- response, False, error="ValueError: 'foo \\xf1o\\xf1o'")
119+ response, is_valid=False, account_verified=False,
120+ error="ValueError: 'foo \\xf1o\\xf1o'")
121 self.mock_logger.exception.assert_called_once_with(
122 'RequestsHandler.create: could not build OAuthRequest with the '
123 'given parameters %r:', self.data)
124@@ -409,26 +418,30 @@
125
126 response = self.do_post()
127 self.assert_request_response(
128- response, False, error="KeyError: u'\\xf1'")
129+ response, is_valid=False, account_verified=False,
130+ error="KeyError: u'\\xf1'")
131 self.mock_logger.exception.assert_called_once_with(
132 'RequestsHandler.create: could not verify request:')
133
134 def test_invalid_if_header_none(self):
135 self.data['authorization'] = None
136 response = self.do_post()
137- self.assert_request_response(response, False)
138+ self.assert_request_response(
139+ response, is_valid=False, account_verified=False)
140 self.assertFalse(self.mock_logger.exception.called)
141
142 def test_invalid_if_not_header_given(self):
143 self.data.pop('authorization')
144 response = self.do_post()
145- self.assert_request_response(response, False)
146+ self.assert_request_response(
147+ response, is_valid=False, account_verified=False)
148 self.assertFalse(self.mock_logger.exception.called)
149
150 def test_invalid_if_no_header_or_query_string(self):
151 del self.data['authorization']
152 response = self.do_post()
153- self.assert_request_response(response, False)
154+ self.assert_request_response(
155+ response, is_valid=False, account_verified=False)
156 self.assertFalse(self.mock_logger.exception.called)
157
158 def test_invalid_if_bad_query_string(self):
159@@ -436,7 +449,8 @@
160 # make sure there is no authorization header
161 del self.data['authorization']
162 response = self.do_post()
163- self.assert_request_response(response, False)
164+ self.assert_request_response(
165+ response, is_valid=False, account_verified=False)
166 self.assertFalse(self.mock_logger.exception.called)
167
168 def test_invalid_if_query_string_invalid(self):
169@@ -451,7 +465,8 @@
170
171 # do a post with correct params
172 response = self.do_post()
173- self.assert_request_response(response, False)
174+ self.assert_request_response(
175+ response, is_valid=False, account_verified=False)
176 self.assertFalse(self.mock_logger.exception.called)
177
178 def test_url_mismatch(self):
179@@ -460,14 +475,16 @@
180 self.data['http_url'] = new_url
181 response = self.do_post()
182
183- self.assert_request_response(response, False)
184+ self.assert_request_response(
185+ response, is_valid=False, account_verified=False)
186 self.assertFalse(self.mock_logger.exception.called)
187
188 def test_method_mismatch(self):
189 self.data['http_method'] = self.default_http_method * 2
190 response = self.do_post()
191
192- self.assert_request_response(response, False)
193+ self.assert_request_response(
194+ response, is_valid=False, account_verified=False)
195 self.assertFalse(self.mock_logger.exception.called)
196
197 def test_valid_request(self):
198@@ -475,7 +492,20 @@
199 response = self.do_post()
200
201 self.assertEqual(response.status_code, 200)
202- self.assert_request_response(response, True)
203+ self.assert_request_response(
204+ response, is_valid=True, account_verified=True)
205+ self.assertFalse(self.mock_logger.exception.called)
206+
207+ def test_valid_request_unverified_user(self):
208+ self.account.verified_emails().delete()
209+ assert not self.account.is_verified
210+
211+ # do a post with correct params
212+ response = self.do_post()
213+
214+ self.assertEqual(response.status_code, 200)
215+ self.assert_request_response(
216+ response, is_valid=True, account_verified=False)
217 self.assertFalse(self.mock_logger.exception.called)
218
219 def test_valid_request_for_non_existent_account(self):
220@@ -483,7 +513,8 @@
221 # do a post with correct params for a non existent account
222 response = self.do_post()
223
224- self.assert_request_response(response, False)
225+ self.assert_request_response(
226+ response, is_valid=False, account_verified=False)
227
228 def test_valid_request_for_inactive_account(self):
229 for status, name in AccountStatus._get_choices():
230@@ -499,7 +530,8 @@
231 # do a post with correct params for an inactive account
232 response = self.do_post()
233
234- self.assert_request_response(response, False)
235+ self.assert_request_response(
236+ response, is_valid=False, account_verified=False)
237 self.assertFalse(self.mock_logger.exception.called)
238
239 def test_valid_request_with_only_query_string(self):
240@@ -511,7 +543,8 @@
241 response = self.do_post()
242
243 self.assertEqual(response.status_code, 200)
244- self.assert_request_response(response, True)
245+ self.assert_request_response(
246+ response, is_valid=True, account_verified=True)
247 self.assertFalse(self.mock_logger.exception.called)
248
249 def test_query_string_ignored_if_authorization_header_found(self):
250@@ -521,15 +554,18 @@
251 response = self.do_post()
252
253 self.assertEqual(response.status_code, 200)
254- self.assert_request_response(response, True)
255+ self.assert_request_response(
256+ response, is_valid=True, account_verified=True)
257 self.assertFalse(self.mock_logger.exception.called)
258
259 def test_replay_attack(self):
260 response = self.do_post()
261- self.assert_request_response(response, True)
262+ self.assert_request_response(
263+ response, is_valid=True, account_verified=True)
264
265 response = self.do_post()
266- self.assert_request_response(response, False)
267+ self.assert_request_response(
268+ response, is_valid=False, account_verified=False)
269
270
271 class RequestsWithHEADTestCase(RequestsTestCase):