Skip to content

Commit 32686b5

Browse files
Sparkyczwoodruffw
authored andcommitted
Add presenter/view of two-factor verification (login process)
Fixes: pypa#996
1 parent 83cdd95 commit 32686b5

File tree

10 files changed

+365
-52
lines changed

10 files changed

+365
-52
lines changed

dev/environment

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,6 @@ STATUSPAGE_URL=https://2p66nmmycsj3.statuspage.io
3535

3636
TOKEN_PASSWORD_SECRET="an insecure password reset secret key"
3737
TOKEN_EMAIL_SECRET="an insecure email verification secret key"
38+
TOKEN_TWO_FACTOR_SECRET="an insecure two-factor auth secret key"
3839

3940
WAREHOUSE_LEGACY_DOMAIN=pypi.python.org

tests/unit/accounts/test_forms.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,3 +553,29 @@ def test_password_breached(self):
553553
"This password has appeared in a breach or has otherwise been "
554554
"compromised and cannot be used."
555555
)
556+
557+
558+
class TestTwoFactorForm:
559+
def test_creation(self):
560+
user_service = pretend.stub()
561+
form = forms.TwoFactorForm(user_service=user_service)
562+
563+
assert form.user_service is user_service
564+
565+
def test_opt_secret_exists(self):
566+
form = forms.TwoFactorForm(
567+
data={
568+
"otp_secret": "",
569+
},
570+
user_service=pretend.stub()
571+
)
572+
assert not form.validate()
573+
assert form.otp_secret.errors.pop() == "This field is required."
574+
575+
form = forms.TwoFactorForm(
576+
data={
577+
"otp_secret": "otp_code",
578+
},
579+
user_service=pretend.stub()
580+
)
581+
assert form.validate()

tests/unit/accounts/test_views.py

Lines changed: 188 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ def test_post_validate_redirects(
157157
user_service = pretend.stub(
158158
find_userid=pretend.call_recorder(lambda username: user_id),
159159
update_user=pretend.call_recorder(lambda *a, **kw: None),
160+
has_two_factor=lambda userid: False,
160161
)
161162
breach_service = pretend.stub(check_password=lambda password, tags=None: False)
162163

@@ -220,6 +221,9 @@ def test_post_validate_redirects(
220221

221222
assert remember.calls == [pretend.call(pyramid_request, str(user_id))]
222223
assert pyramid_request.session.invalidate.calls == [pretend.call()]
224+
assert pyramid_request.find_service.calls == [
225+
pretend.call(IUserService, context=None),
226+
]
223227
assert pyramid_request.session.new_csrf_token.calls == [pretend.call()]
224228

225229
@pytest.mark.parametrize(
@@ -234,6 +238,7 @@ def test_post_validate_no_redirects(
234238
user_service = pretend.stub(
235239
find_userid=pretend.call_recorder(lambda username: 1),
236240
update_user=lambda *a, **k: None,
241+
has_two_factor=lambda userid: False,
237242
)
238243
breach_service = pretend.stub(check_password=lambda password, tags=None: False)
239244

@@ -264,6 +269,189 @@ def test_redirect_authenticated_user(self):
264269
assert isinstance(result, HTTPSeeOther)
265270
assert result.headers["Location"] == "/the-redirect"
266271

272+
@pytest.mark.parametrize("redirect_url", ["test_redirect_url", None])
273+
def test_two_factor_auth(self, pyramid_request, redirect_url, token_service):
274+
user_service = pretend.stub(
275+
find_userid=pretend.call_recorder(lambda username: 1),
276+
update_user=lambda *a, **k: None,
277+
has_two_factor=lambda userid: True,
278+
)
279+
280+
token_service = pretend.stub(
281+
dumps=pretend.call_recorder(lambda token_data: "test-secure-token")
282+
)
283+
pyramid_request.find_service = lambda interface, **kwargs: {
284+
IUserService: user_service,
285+
ITokenService: token_service,
286+
}[interface]
287+
288+
pyramid_request.method = "POST"
289+
if redirect_url:
290+
pyramid_request.POST["next"] = redirect_url
291+
292+
form_obj = pretend.stub(
293+
validate=pretend.call_recorder(lambda: True),
294+
username=pretend.stub(data="theuser"),
295+
)
296+
form_class = pretend.call_recorder(lambda d, user_service: form_obj)
297+
pyramid_request.route_path = pretend.call_recorder(
298+
lambda a: "/account/two-factor"
299+
)
300+
result = views.login(pyramid_request, _form_class=form_class)
301+
302+
token_expected_data = {"userid": 1}
303+
if redirect_url:
304+
token_expected_data['redirect_to'] = redirect_url
305+
assert token_service.dumps.calls == [pretend.call(token_expected_data)]
306+
307+
assert isinstance(result, HTTPSeeOther)
308+
assert result.headerlist == [
309+
('Content-Type', 'text/html; charset=UTF-8'),
310+
('Content-Length', '0'),
311+
('Location', '/account/two-factor'),
312+
('Set-Cookie', "{}={}; Path=/".
313+
format(views.TWO_FACTOR_COOKIE_KEY, "test-secure-token"))]
314+
315+
316+
class TestTwoFactor:
317+
@pytest.mark.parametrize("redirect_url", [None, "/foo/bar/", "/wat/"])
318+
def test_get_returns_form(self, pyramid_request, redirect_url):
319+
user_service = pretend.stub(
320+
find_userid=pretend.call_recorder(lambda username: 1),
321+
update_user=lambda *a, **k: None,
322+
send_otp_secret=pretend.call_recorder(lambda userid: None),
323+
)
324+
325+
token_data = {"userid": 1}
326+
if redirect_url:
327+
token_data['redirect_to'] = redirect_url
328+
329+
token_service = pretend.stub(
330+
loads=pretend.call_recorder(lambda token: token_data)
331+
)
332+
pyramid_request.find_service = lambda interface, **kwargs: {
333+
ITokenService: token_service,
334+
IUserService: user_service,
335+
}[interface]
336+
337+
form_obj = pretend.stub()
338+
form_class = pretend.call_recorder(lambda d, user_service: form_obj)
339+
340+
result = views.two_factor(pyramid_request, _form_class=form_class)
341+
342+
assert result == {
343+
"form": form_obj,
344+
}
345+
assert user_service.send_otp_secret.calls == [
346+
pretend.call(1)
347+
]
348+
assert form_class.calls == [
349+
pretend.call(pyramid_request.POST, user_service=user_service)
350+
]
351+
352+
@pytest.mark.parametrize("redirect_url", ["test_redirect_url", None])
353+
def test_two_factor_auth(self, monkeypatch, pyramid_request, redirect_url,
354+
token_service):
355+
remember = pretend.call_recorder(lambda request, user_id: [("foo", "bar")])
356+
monkeypatch.setattr(views, "remember", remember)
357+
user_service = pretend.stub(
358+
find_userid=pretend.call_recorder(lambda username: 1),
359+
update_user=lambda *a, **k: None,
360+
send_otp_secret=lambda userid: None,
361+
check_otp_secret=lambda userid, otp_token: True,
362+
)
363+
364+
new_session = {}
365+
366+
token_data = {"userid": str(1)}
367+
if redirect_url:
368+
token_data['redirect_to'] = redirect_url
369+
370+
token_service = pretend.stub(
371+
loads=pretend.call_recorder(lambda token: token_data)
372+
)
373+
pyramid_request.find_service = lambda interface, **kwargs: {
374+
IUserService: user_service,
375+
ITokenService: token_service,
376+
}[interface]
377+
378+
pyramid_request.method = "POST"
379+
pyramid_request.session = pretend.stub(
380+
items=lambda: [("a", "b"), ("foo", "bar")],
381+
update=new_session.update,
382+
invalidate=pretend.call_recorder(lambda: None),
383+
new_csrf_token=pretend.call_recorder(lambda: None),
384+
)
385+
386+
pyramid_request.set_property(
387+
lambda r: str(uuid.uuid4()),
388+
name="unauthenticated_userid",
389+
)
390+
391+
form_obj = pretend.stub(
392+
validate=pretend.call_recorder(lambda: True),
393+
otp_secret=pretend.stub(data="test-otp-secret"),
394+
)
395+
form_class = pretend.call_recorder(lambda d, user_service: form_obj)
396+
pyramid_request.route_path = pretend.call_recorder(
397+
lambda a: "/account/two-factor"
398+
)
399+
pyramid_request.cookies[views.TWO_FACTOR_COOKIE_KEY] = "test-secure-token"
400+
result = views.two_factor(pyramid_request, _form_class=form_class)
401+
402+
token_expected_data = {"userid": str(1)}
403+
if redirect_url:
404+
token_expected_data['redirect_to'] = redirect_url
405+
assert token_service.loads.calls == [pretend.call("test-secure-token")]
406+
407+
assert isinstance(result, HTTPSeeOther)
408+
409+
assert remember.calls == [pretend.call(pyramid_request, str(1))]
410+
assert pyramid_request.session.invalidate.calls == [pretend.call()]
411+
assert pyramid_request.session.new_csrf_token.calls == [pretend.call()]
412+
413+
@pytest.mark.parametrize("redirect_url", ["test_redirect_url", None])
414+
def test_two_factor_auth_invalid(self, pyramid_request, redirect_url,
415+
token_service):
416+
user_service = pretend.stub(
417+
find_userid=pretend.call_recorder(lambda username: 1),
418+
update_user=lambda *a, **k: None,
419+
send_otp_secret=lambda userid: None,
420+
check_otp_secret=lambda userid, otp_token: False,
421+
)
422+
423+
token_data = {"userid": str(1)}
424+
if redirect_url:
425+
token_data['redirect_to'] = redirect_url
426+
427+
token_service = pretend.stub(loads=pretend.call_recorder(
428+
lambda token: token_data)
429+
)
430+
pyramid_request.find_service = lambda interface, **kwargs: {
431+
IUserService: user_service,
432+
ITokenService: token_service,
433+
}[interface]
434+
435+
pyramid_request.method = "POST"
436+
437+
form_obj = pretend.stub(
438+
validate=pretend.call_recorder(lambda: True),
439+
otp_secret=pretend.stub(data="test-otp-secret"),
440+
)
441+
form_class = pretend.call_recorder(lambda d, user_service: form_obj)
442+
pyramid_request.route_path = pretend.call_recorder(
443+
lambda a: "/account/two-factor"
444+
)
445+
pyramid_request.cookies[views.TWO_FACTOR_COOKIE_KEY] = "test-secure-token"
446+
result = views.two_factor(pyramid_request, _form_class=form_class)
447+
448+
token_expected_data = {"userid": str(1)}
449+
if redirect_url:
450+
token_expected_data['redirect_to'] = redirect_url
451+
assert token_service.loads.calls == [pretend.call("test-secure-token")]
452+
453+
assert isinstance(result, HTTPSeeOther)
454+
267455

268456
class TestLogout:
269457
@pytest.mark.parametrize("next_url", [None, "/foo/bar/", "/wat/"])
@@ -729,7 +917,6 @@ def test_reset_password(self, db_request, user_service, token_service):
729917
pretend.call(IUserService, context=None),
730918
pretend.call(IPasswordBreachedService, context=None),
731919
pretend.call(ITokenService, name="password"),
732-
pretend.call(IUserService, context=None),
733920
]
734921

735922
@pytest.mark.parametrize(

warehouse/accounts/__init__.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ def includeme(config):
125125
config.register_service_factory(
126126
TokenServiceFactory(name="email"), ITokenService, name="email"
127127
)
128+
config.register_service_factory(
129+
TokenServiceFactory(name="two_factor"), ITokenService, name="two_factor"
130+
)
128131

129132
# Register our password breach detection service.
130133
breached_pw_class = config.maybe_dotted(

warehouse/accounts/forms.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ def validate_username(self, field):
3232
raise wtforms.validators.ValidationError("No user found with that username")
3333

3434

35-
class OtpCodeMixin:
35+
class OtpSecretMixin:
3636

37-
otp_code = wtforms.StringField(validators=[wtforms.validators.DataRequired()])
37+
otp_secret = wtforms.StringField(validators=[wtforms.validators.DataRequired()])
3838

3939

4040
class NewUsernameMixin:
@@ -224,7 +224,7 @@ def validate_password(self, field):
224224
)
225225

226226

227-
class TwoFactorForm(OtpCodeMixin, forms.Form):
227+
class TwoFactorForm(OtpSecretMixin, forms.Form):
228228
def __init__(self, *args, user_service, **kwargs):
229229
super().__init__(*args, **kwargs)
230230
self.user_service = user_service

warehouse/accounts/interfaces.py

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,19 @@ def __init__(self, *args, resets_in, **kwargs):
1919

2020
return super().__init__(*args, **kwargs)
2121

22+
class TokenException(Exception):
23+
pass
24+
2225

23-
class TokenExpired(Exception):
26+
class TokenExpired(TokenException):
2427
pass
2528

2629

27-
class TokenInvalid(Exception):
30+
class TokenInvalid(TokenException):
2831
pass
2932

3033

31-
class TokenMissing(Exception):
34+
class TokenMissing(TokenException):
3235
pass
3336

3437

@@ -97,6 +100,21 @@ def is_disabled(user_id):
97100
(IsDisabled: bool, Reason: Optional[DisableReason])
98101
"""
99102

103+
def has_two_factor(user_id):
104+
"""
105+
Returns True if the user has two factor authentication.
106+
"""
107+
108+
def send_otp_secret(user_id):
109+
"""
110+
Sends two factor authentication OTP code to user
111+
"""
112+
113+
def check_otp_secret(user_id, otp_secret):
114+
"""
115+
Returns True if the given OTP code is valid.
116+
"""
117+
100118

101119
class ITokenService(Interface):
102120
def dumps(data):

warehouse/accounts/services.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,24 @@ def is_disabled(self, user_id):
226226
else:
227227
return (True, user.disabled_for)
228228

229+
def has_two_factor(self, user_id):
230+
"""
231+
Returns True if the user has two factor authentication.
232+
"""
233+
return True
234+
235+
def send_otp_secret(self, user_id):
236+
"""
237+
Sends two factor authentication OTP code to user
238+
"""
239+
pass
240+
241+
def check_otp_secret(self, user_id, otp_secret):
242+
"""
243+
Returns True if the given OTP code is valid.
244+
"""
245+
return True
246+
229247

230248
@implementer(ITokenService)
231249
class TokenService:

0 commit comments

Comments
 (0)