Skip to content

Remember device for 30 days option #14194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dev/environment
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ STATUSPAGE_URL=https://2p66nmmycsj3.statuspage.io
TOKEN_PASSWORD_SECRET="an insecure password reset secret key"
TOKEN_EMAIL_SECRET="an insecure email verification secret key"
TOKEN_TWO_FACTOR_SECRET="an insecure two-factor auth secret key"
TOKEN_REMEMBER_DEVICE_SECRET="an insecure remember device auth secret key"

WAREHOUSE_LEGACY_DOMAIN=pypi.python.org

Expand Down
5 changes: 5 additions & 0 deletions tests/unit/accounts/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,11 @@ def test_includeme(monkeypatch):
pretend.call(
TokenServiceFactory(name="two_factor"), ITokenService, name="two_factor"
),
pretend.call(
TokenServiceFactory(name="remember_device"),
ITokenService,
name="remember_device",
),
pretend.call(
HaveIBeenPwnedPasswordBreachedService.create_service,
IPasswordBreachedService,
Expand Down
140 changes: 127 additions & 13 deletions tests/unit/accounts/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import json
import uuid

from datetime import timedelta

import freezegun
import pretend
import pytest
Expand Down Expand Up @@ -46,7 +48,10 @@
TooManyFailedLogins,
TooManyPasswordResetRequests,
)
from warehouse.accounts.views import two_factor_and_totp_validate
from warehouse.accounts.views import (
REMEMBER_DEVICE_COOKIE,
two_factor_and_totp_validate,
)
from warehouse.admin.flags import AdminFlag, AdminFlagValue
from warehouse.events.tags import EventTag
from warehouse.metrics.interfaces import IMetricsService
Expand Down Expand Up @@ -435,9 +440,15 @@ def test_redirect_authenticated_user(self):
assert result.headers["Location"] == "/the-redirect"

@pytest.mark.parametrize("redirect_url", ["test_redirect_url", None])
def test_two_factor_auth(self, pyramid_request, redirect_url, token_service):
def test_two_factor_auth(
self, monkeypatch, pyramid_request, redirect_url, token_service
):
token_service.dumps = lambda d: "fake_token"

monkeypatch.setattr(
views, "_check_remember_device_token", lambda *a, **kw: False
)

user_service = pretend.stub(
find_userid=pretend.call_recorder(lambda username: 1),
update_user=lambda *a, **k: None,
Expand Down Expand Up @@ -590,7 +601,7 @@ def test_get_returns_totp_form(self, pyramid_request, redirect_url):
ITokenService: token_service,
IUserService: user_service,
}[interface]

pyramid_request.registry.settings = {"remember_device.days": 30}
pyramid_request.query_string = pretend.stub()

form_obj = pretend.stub()
Expand All @@ -603,7 +614,7 @@ def test_get_returns_totp_form(self, pyramid_request, redirect_url):
assert token_service.loads.calls == [
pretend.call(pyramid_request.query_string, return_timestamp=True)
]
assert result == {"totp_form": form_obj}
assert result == {"totp_form": form_obj, "remember_device_days": 30}
assert form_class.calls == [
pretend.call(
pyramid_request.POST,
Expand Down Expand Up @@ -643,16 +654,17 @@ def test_get_returns_webauthn(self, pyramid_request, redirect_url):
ITokenService: token_service,
IUserService: user_service,
}[interface]

pyramid_request.registry.settings = {"remember_device.days": 30}
pyramid_request.query_string = pretend.stub()

result = views.two_factor_and_totp_validate(
pyramid_request, _form_class=pretend.stub()
)

assert token_service.loads.calls == [
pretend.call(pyramid_request.query_string, return_timestamp=True)
]
assert result == {"has_webauthn": True}
assert result == {"has_webauthn": True, "remember_device_days": 30}

@pytest.mark.parametrize("redirect_url", [None, "/foo/bar/", "/wat/"])
def test_get_returns_recovery_code_status(self, pyramid_request, redirect_url):
Expand Down Expand Up @@ -683,7 +695,7 @@ def test_get_returns_recovery_code_status(self, pyramid_request, redirect_url):
ITokenService: token_service,
IUserService: user_service,
}[interface]

pyramid_request.registry.settings = {"remember_device.days": 30}
pyramid_request.query_string = pretend.stub()
result = views.two_factor_and_totp_validate(
pyramid_request, _form_class=pretend.stub()
Expand All @@ -692,16 +704,25 @@ def test_get_returns_recovery_code_status(self, pyramid_request, redirect_url):
assert token_service.loads.calls == [
pretend.call(pyramid_request.query_string, return_timestamp=True)
]
assert result == {"has_recovery_codes": True}
assert result == {"has_recovery_codes": True, "remember_device_days": 30}

@pytest.mark.parametrize("redirect_url", ["test_redirect_url", None])
@pytest.mark.parametrize("has_recovery_codes", [True, False])
@pytest.mark.parametrize("remember_device", [True, False])
def test_totp_auth(
self, monkeypatch, pyramid_request, redirect_url, has_recovery_codes
self,
monkeypatch,
pyramid_request,
redirect_url,
has_recovery_codes,
remember_device,
):
remember = pretend.call_recorder(lambda request, user_id: [("foo", "bar")])
monkeypatch.setattr(views, "remember", remember)

_remember_device = pretend.call_recorder(lambda *a, **kw: None)
monkeypatch.setattr(views, "_remember_device", _remember_device)

query_params = {"userid": str(1)}
if redirect_url:
query_params["redirect_to"] = redirect_url
Expand Down Expand Up @@ -751,10 +772,12 @@ def test_totp_auth(
lambda *args: None
)
pyramid_request.session.record_password_timestamp = lambda timestamp: None
pyramid_request.registry.settings = {"remember_device.days": 30}

form_obj = pretend.stub(
validate=pretend.call_recorder(lambda: True),
totp_value=pretend.stub(data="test-otp-secret"),
remember_device=pretend.stub(data=remember_device),
)
form_class = pretend.call_recorder(lambda d, user_service, **kw: form_obj)
pyramid_request.route_path = pretend.call_recorder(
Expand Down Expand Up @@ -793,6 +816,12 @@ def test_totp_auth(
[] if has_recovery_codes else [pretend.call(pyramid_request, user)]
)

assert _remember_device.calls == (
[]
if not remember_device
else [pretend.call(pyramid_request, result, str(1), "totp")]
)

def test_totp_auth_already_authed(self):
request = pretend.stub(
authenticated_userid="not_none",
Expand Down Expand Up @@ -836,6 +865,7 @@ def test_totp_form_invalid(self):
IUserService: user_service,
}[interface],
query_string=pretend.stub(),
registry=pretend.stub(settings={"remember_device.days": 30}),
)

form_obj = pretend.stub(
Expand All @@ -849,7 +879,7 @@ def test_totp_form_invalid(self):
assert token_service.loads.calls == [
pretend.call(request.query_string, return_timestamp=True)
]
assert result == {"totp_form": form_obj}
assert result == {"totp_form": form_obj, "remember_device_days": 30}

def test_two_factor_token_missing_userid(self, pyramid_request):
token_service = pretend.stub(
Expand Down Expand Up @@ -1003,7 +1033,10 @@ def test_webauthn_validate_invalid_form(self, monkeypatch):
assert result == {"fail": {"errors": ["Fake validation failure"]}}

@pytest.mark.parametrize("has_recovery_codes", [True, False])
def test_webauthn_validate(self, monkeypatch, pyramid_request, has_recovery_codes):
@pytest.mark.parametrize("remember_device", [True, False])
def test_webauthn_validate(
self, monkeypatch, pyramid_request, has_recovery_codes, remember_device
):
_get_two_factor_data = pretend.call_recorder(
lambda r: {"redirect_to": "foobar", "userid": 1}
)
Expand All @@ -1012,6 +1045,9 @@ def test_webauthn_validate(self, monkeypatch, pyramid_request, has_recovery_code
_login_user = pretend.call_recorder(lambda *a, **kw: pretend.stub())
monkeypatch.setattr(views, "_login_user", _login_user)

_remember_device = pretend.call_recorder(lambda *a, **kw: None)
monkeypatch.setattr(views, "_remember_device", _remember_device)

user = pretend.stub(
webauthn=pretend.stub(sign_count=pretend.stub()),
has_recovery_codes=has_recovery_codes,
Expand Down Expand Up @@ -1039,6 +1075,7 @@ def test_webauthn_validate(self, monkeypatch, pyramid_request, has_recovery_code
credential_device_type="single_device",
credential_backed_up=False,
),
remember_device=pretend.stub(data=remember_device),
)
form_class = pretend.call_recorder(lambda *a, **kw: form_obj)
monkeypatch.setattr(views, "WebAuthnAuthenticationForm", form_class)
Expand All @@ -1053,7 +1090,7 @@ def test_webauthn_validate(self, monkeypatch, pyramid_request, has_recovery_code
pretend.call(
pyramid_request,
1,
two_factor_method="webauthn",
"webauthn",
two_factor_label="webauthn_label",
)
]
Expand All @@ -1065,12 +1102,89 @@ def test_webauthn_validate(self, monkeypatch, pyramid_request, has_recovery_code
[] if has_recovery_codes else [pretend.call(pyramid_request, user)]
)

assert _remember_device.calls == (
[]
if not remember_device
else [
pretend.call(pyramid_request, pyramid_request.response, 1, "webauthn")
]
)

assert result == {
"success": "Successful WebAuthn assertion",
"redirect_to": "foobar",
}


class TestRememberDevice:
def test_check_remember_device_token_valid(self):
token_service = pretend.stub(loads=lambda *a: {"user_id": str(1)})
request = pretend.stub(
cookies=pretend.stub(get=lambda *a, **kw: "token"),
find_service=lambda interface, **kwargs: {
ITokenService: token_service,
}[interface],
)
assert views._check_remember_device_token(request, 1)

def test_check_remember_device_token_invalid_no_cookie(self):
request = pretend.stub(
cookies=pretend.stub(get=lambda *a, **kw: ""),
)
assert not views._check_remember_device_token(request, 1)

def test_check_remember_device_token_invalid_bad_token(self):
token_service = pretend.stub(loads=pretend.raiser(TokenException))
request = pretend.stub(
cookies=pretend.stub(get=lambda *a, **kw: "token"),
find_service=lambda interface, **kwargs: {
ITokenService: token_service,
}[interface],
)
assert not views._check_remember_device_token(request, 1)

def test_check_remember_device_token_invalid_wrong_user(self):
token_service = pretend.stub(loads=lambda *a: {"user_id": str(999)})
request = pretend.stub(
cookies=pretend.stub(get=lambda *a, **kw: "token"),
find_service=lambda interface, **kwargs: {
ITokenService: token_service,
}[interface],
)
assert not views._check_remember_device_token(request, 1)

def test_remember_device(self):
token_service = pretend.stub(dumps=lambda *a: "token_data")
pyramid_request = pretend.stub(
find_service=lambda interface, **kwargs: {
ITokenService: token_service,
}[interface],
scheme="https",
route_path=lambda *a, **kw: "/accounts/login",
user=pretend.stub(
record_event=pretend.call_recorder(lambda *a, **kw: None)
),
registry=pretend.stub(
settings={"remember_device.seconds": timedelta(days=30).total_seconds()}
),
)
response = pretend.stub(set_cookie=pretend.call_recorder(lambda *a, **kw: None))

views._remember_device(pyramid_request, response, 1, "webauthn")

assert response.set_cookie.calls == [
pretend.call(
REMEMBER_DEVICE_COOKIE,
"token_data",
max_age=timedelta(days=30).total_seconds(),
httponly=True,
secure=True,
samesite=b"strict",
path="/accounts/login",
)
]


class TestRecoveryCode:
def test_already_authenticated(self):
request = pretend.stub(
Expand Down Expand Up @@ -1129,7 +1243,6 @@ def test_get_returns_form(self, pyramid_request):
ITokenService: token_service,
IUserService: user_service,
}[interface]

pyramid_request.query_string = pretend.stub()

form_obj = pretend.stub()
Expand Down Expand Up @@ -1276,6 +1389,7 @@ def test_recovery_code_form_invalid(self):
IUserService: user_service,
}[interface],
query_string=pretend.stub(),
# registry=pretend.stub(settings={"remember_device.days": 30}),
)

form_obj = pretend.stub(
Expand Down
4 changes: 4 additions & 0 deletions tests/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import os

from datetime import timedelta
from unittest import mock

import orjson
Expand Down Expand Up @@ -246,6 +247,9 @@ def __init__(self):
"warehouse.commit": "null",
"site.name": "Warehouse",
"token.two_factor.max_age": 300,
"remember_device.days": 30,
"remember_device.seconds": timedelta(days=30).total_seconds(),
"token.remember_device.max_age": timedelta(days=30).total_seconds(),
"token.default.max_age": 21600,
"pythondotorg.host": "https://www.python.org",
"warehouse.xmlrpc.client.ratelimit_string": "3600 per hour",
Expand Down
5 changes: 5 additions & 0 deletions warehouse/accounts/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ def includeme(config):
config.register_service_factory(
TokenServiceFactory(name="two_factor"), ITokenService, name="two_factor"
)
config.register_service_factory(
TokenServiceFactory(name="remember_device"),
ITokenService,
name="remember_device",
)

# Register our password breach detection service.
breached_pw_class = config.maybe_dotted(
Expand Down
2 changes: 2 additions & 0 deletions warehouse/accounts/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,8 @@ def __init__(self, *args, request, user_id, user_service, **kwargs):
self.user_id = user_id
self.user_service = user_service

remember_device = wtforms.BooleanField(default=False)


class TOTPAuthenticationForm(TOTPValueMixin, _TwoFactorAuthenticationForm):
def validate_totp_value(self, field):
Expand Down
Loading