Skip to content

Commit d5c5876

Browse files
authored
Require email confirmation for TOTP-based logins (#18689)
* Add UserUniqueLogin model * Send an email on TOTP login and store a UniqueLogin * Tests * Basic admin interface * Update migration to match model * Update warehouse/templates/email/unrecognized-login/body.txt * Handle recovery code usage too * Add a composite index * Update error message * Update test * Add UserUniqueLogin.last_used * Fix HTML email template * Fall back to raw user agent string * Add geo data when available * Rebase migration * Rebase migration again * Tweak email text * Tweak view text * Update functional tests to avoid login confirmation flow * Add a functional test * Update translations * Linkify links * Linting
1 parent 292ca9e commit d5c5876

File tree

26 files changed

+2124
-326
lines changed

26 files changed

+2124
-326
lines changed

dev/environment

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ TOKEN_PASSWORD_SECRET="an insecure password reset secret key"
5959
TOKEN_EMAIL_SECRET="an insecure email verification secret key"
6060
TOKEN_TWO_FACTOR_SECRET="an insecure two-factor auth secret key"
6161
TOKEN_REMEMBER_DEVICE_SECRET="an insecure remember device auth secret key"
62+
TOKEN_CONFIRM_LOGIN_SECRET="an insecure confirm login auth secret key"
6263

6364
WAREHOUSE_LEGACY_DOMAIN=pypi.python.org
6465

tests/common/db/accounts.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@
1414
TermsOfServiceEngagement,
1515
User,
1616
UserTermsOfServiceEngagement,
17+
UserUniqueLogin,
1718
)
1819

20+
from ...common.constants import REMOTE_ADDR
1921
from .base import WarehouseFactory
2022

2123
fake = faker.Faker()
@@ -130,3 +132,11 @@ class Meta:
130132
# TODO: Replace when factory_boy supports `unique`.
131133
# See https://github.com/FactoryBoy/factory_boy/pull/997
132134
name = factory.Sequence(lambda _: fake.unique.user_name())
135+
136+
137+
class UserUniqueLoginFactory(WarehouseFactory):
138+
class Meta:
139+
model = UserUniqueLogin
140+
141+
user = factory.SubFactory(UserFactory)
142+
ip_address = REMOTE_ADDR

tests/conftest.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ def get_app_config(database, nondefaults=None):
313313
"warehouse.prevent_esi": True,
314314
"warehouse.token": "insecure token",
315315
"warehouse.ip_salt": "insecure salt",
316+
"token.confirm_login.secret": "insecure token",
316317
"camo.url": "http://localhost:9000/",
317318
"camo.key": "insecure key",
318319
"celery.broker_redis_url": "redis://localhost:0/",

tests/functional/manage/test_account_publishing.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
import pytest
88
import responses
99

10-
from tests.common.db.accounts import UserFactory
10+
from tests.common.constants import REMOTE_ADDR
11+
from tests.common.db.accounts import UserFactory, UserUniqueLoginFactory
12+
from warehouse.accounts.models import UniqueLoginStatus
1113
from warehouse.utils.otp import _get_totp
1214

1315

@@ -25,6 +27,9 @@ def test_add_pending_github_publisher_succeeds(self, webtest):
2527
with_terms_of_service_agreement=True,
2628
clear_pwd="password",
2729
)
30+
UserUniqueLoginFactory.create(
31+
user=user, ip_address=REMOTE_ADDR, status=UniqueLoginStatus.CONFIRMED
32+
)
2833
# Create a response from GitHub API for owner details
2934
# during form submission validation.
3035
responses.add(
@@ -101,6 +106,9 @@ def test_add_pending_gitlab_publisher_succeeds(self, webtest):
101106
with_terms_of_service_agreement=True,
102107
clear_pwd="password",
103108
)
109+
UserUniqueLoginFactory.create(
110+
user=user, ip_address=REMOTE_ADDR, status=UniqueLoginStatus.CONFIRMED
111+
)
104112

105113
# Act: Log in
106114
login_page = webtest.get("/account/login/", status=HTTPStatus.OK)

tests/functional/manage/test_organization_publishing.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@
77
import pytest
88
import responses
99

10-
from tests.common.db.accounts import UserFactory
10+
from tests.common.constants import REMOTE_ADDR
11+
from tests.common.db.accounts import UserFactory, UserUniqueLoginFactory
1112
from tests.common.db.organizations import OrganizationFactory, OrganizationRoleFactory
13+
from warehouse.accounts.models import UniqueLoginStatus
1214
from warehouse.organizations.models import OrganizationRoleType
1315
from warehouse.utils.otp import _get_totp
1416

@@ -27,6 +29,9 @@ def test_add_pending_github_publisher_to_organization(self, webtest):
2729
with_terms_of_service_agreement=True,
2830
clear_pwd="password",
2931
)
32+
UserUniqueLoginFactory.create(
33+
user=user, ip_address=REMOTE_ADDR, status=UniqueLoginStatus.CONFIRMED
34+
)
3035
organization = OrganizationFactory.create(name="test-organization")
3136
OrganizationRoleFactory.create(
3237
user=user,
@@ -109,6 +114,9 @@ def test_add_pending_gitlab_publisher_to_organization(self, webtest):
109114
with_terms_of_service_agreement=True,
110115
clear_pwd="password",
111116
)
117+
UserUniqueLoginFactory.create(
118+
user=user, ip_address=REMOTE_ADDR, status=UniqueLoginStatus.CONFIRMED
119+
)
112120
organization = OrganizationFactory.create(name="test-organization")
113121
OrganizationRoleFactory.create(
114122
user=user,
@@ -180,6 +188,9 @@ def test_add_pending_google_publisher_to_organization(self, webtest):
180188
with_terms_of_service_agreement=True,
181189
clear_pwd="password",
182190
)
191+
UserUniqueLoginFactory.create(
192+
user=user, ip_address=REMOTE_ADDR, status=UniqueLoginStatus.CONFIRMED
193+
)
183194
organization = OrganizationFactory.create(name="test-organization")
184195
OrganizationRoleFactory.create(
185196
user=user,
@@ -249,6 +260,9 @@ def test_add_pending_activestate_publisher_to_organization(self, webtest):
249260
with_terms_of_service_agreement=True,
250261
clear_pwd="password",
251262
)
263+
UserUniqueLoginFactory.create(
264+
user=user, ip_address=REMOTE_ADDR, status=UniqueLoginStatus.CONFIRMED
265+
)
252266
organization = OrganizationFactory.create(name="test-organization")
253267
OrganizationRoleFactory.create(
254268
user=user,

tests/functional/manage/test_project_publishing.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@
77
import pytest
88
import responses
99

10-
from tests.common.db.accounts import UserFactory
10+
from tests.common.constants import REMOTE_ADDR
11+
from tests.common.db.accounts import UserFactory, UserUniqueLoginFactory
1112
from tests.common.db.packaging import ProjectFactory, RoleFactory
13+
from warehouse.accounts.models import UniqueLoginStatus
1214
from warehouse.utils.otp import _get_totp
1315

1416

@@ -28,6 +30,9 @@ def test_add_github_publisher_to_existing_project(self, webtest):
2830
)
2931
project = ProjectFactory.create(name="existing-project")
3032
RoleFactory.create(user=user, project=project, role_name="Owner")
33+
UserUniqueLoginFactory.create(
34+
user=user, ip_address=REMOTE_ADDR, status=UniqueLoginStatus.CONFIRMED
35+
)
3136

3237
# Mock GitHub API for owner validation
3338
responses.add(
@@ -105,6 +110,9 @@ def test_add_gitlab_publisher_to_existing_project(self, webtest):
105110
)
106111
project = ProjectFactory.create(name="gitlab-project")
107112
RoleFactory.create(user=user, project=project, role_name="Owner")
113+
UserUniqueLoginFactory.create(
114+
user=user, ip_address=REMOTE_ADDR, status=UniqueLoginStatus.CONFIRMED
115+
)
108116

109117
# Act: Log in
110118
login_page = webtest.get("/account/login/", status=HTTPStatus.OK)

tests/functional/manage/test_views.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,16 @@
1010

1111
from webob.multidict import MultiDict
1212

13+
from tests.common.constants import REMOTE_ADDR
1314
from warehouse.accounts.interfaces import IPasswordBreachedService, IUserService
15+
from warehouse.accounts.models import UniqueLoginStatus
1416
from warehouse.manage import views
1517
from warehouse.manage.views import organizations as org_views
1618
from warehouse.organizations.interfaces import IOrganizationService
1719
from warehouse.organizations.models import OrganizationType
1820
from warehouse.utils.otp import _get_totp
1921

20-
from ...common.db.accounts import EmailFactory, UserFactory
22+
from ...common.db.accounts import EmailFactory, UserFactory, UserUniqueLoginFactory
2123

2224

2325
class TestManageAccount:
@@ -52,6 +54,9 @@ def test_changing_password_succeeds(self, webtest, socket_enabled):
5254
with_terms_of_service_agreement=True,
5355
clear_pwd="password",
5456
)
57+
UserUniqueLoginFactory.create(
58+
user=user, ip_address=REMOTE_ADDR, status=UniqueLoginStatus.CONFIRMED
59+
)
5560

5661
# visit login page
5762
login_page = webtest.get("/account/login/", status=HTTPStatus.OK)

tests/functional/test_login.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
# SPDX-License-Identifier: Apache-2.0
2+
3+
import time
4+
5+
from http import HTTPStatus
6+
7+
from tests.common.db.accounts import UserFactory
8+
from warehouse.accounts.models import UniqueLoginStatus, UserUniqueLogin
9+
from warehouse.utils.otp import _get_totp
10+
11+
12+
def test_unrecognized_login_with_totp(webtest):
13+
"""
14+
Tests that a user with TOTP logging in from an unrecognized IP address
15+
is required to confirm their email address.
16+
"""
17+
18+
# Arrange: Create a user with a verified email and TOTP enabled
19+
user = UserFactory.create(
20+
with_verified_primary_email=True,
21+
clear_pwd="password",
22+
)
23+
24+
# Act 1: Go to the login page
25+
login_page = webtest.get("/account/login/")
26+
assert login_page.status_code == HTTPStatus.OK
27+
28+
# Fill out the login form and submit it
29+
login_form = login_page.forms["login-form"]
30+
login_form["username"] = user.username
31+
login_form["password"] = "password"
32+
resp = login_form.submit()
33+
34+
# We should be redirected to the two-factor authentication page
35+
assert resp.status_code == HTTPStatus.SEE_OTHER
36+
assert resp.headers["Location"].startswith("http://localhost/account/two-factor/")
37+
two_factor_page = resp.follow()
38+
assert "Two-factor authentication" in two_factor_page
39+
40+
# This is the first time we're logging in from this IP, so we should
41+
# be redirected to the "unrecognized login" page after submitting the
42+
# TOTP code.
43+
two_factor_form = two_factor_page.forms["totp-auth-form"]
44+
two_factor_form["totp_value"] = (
45+
_get_totp(user.totp_secret).generate(time.time()).decode()
46+
)
47+
resp = two_factor_form.submit()
48+
49+
assert resp.status_code == HTTPStatus.SEE_OTHER
50+
assert resp.headers["Location"].endswith("/account/confirm-login/")
51+
unrecognized_page = resp.follow()
52+
assert "Unrecognized device" in unrecognized_page
53+
54+
# This is a hack because the functional test doesn't have another way to
55+
# determine the magic link that was sent in the email. Instead, find the
56+
# UserUniqueLogin that was created for this user and manually confirm it
57+
db_session = webtest.extra_environ["warehouse.db_session"]
58+
unique_login = (
59+
db_session.query(UserUniqueLogin).filter(UserUniqueLogin.user == user).one()
60+
)
61+
assert unique_login.status == UniqueLoginStatus.PENDING
62+
unique_login.status = UniqueLoginStatus.CONFIRMED
63+
db_session.commit()
64+
65+
# Act 2: Try to log in again
66+
login_page = webtest.get("/account/login/")
67+
login_form = login_page.forms["login-form"]
68+
login_form["username"] = user.username
69+
login_form["password"] = "password"
70+
resp = login_form.submit()
71+
72+
# We should be redirected to the two-factor authentication page
73+
assert resp.status_code == HTTPStatus.SEE_OTHER
74+
assert resp.headers["Location"].startswith("http://localhost/account/two-factor/")
75+
two_factor_page = resp.follow()
76+
assert "Two-factor authentication" in two_factor_page
77+
78+
# Fill out the TOTP form and submit it
79+
two_factor_form = two_factor_page.forms["totp-auth-form"]
80+
two_factor_form["totp_value"] = (
81+
_get_totp(user.totp_secret).generate(time.time()).decode()
82+
)
83+
84+
# We should be able to successfully log in
85+
logged_in = two_factor_form.submit().follow(status=HTTPStatus.OK)
86+
assert logged_in.html.find("title", string="Warehouse · The Python Package Index")

tests/unit/accounts/test_core.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,11 @@ def test_includeme(monkeypatch):
167167
pretend.call(
168168
TokenServiceFactory(name="two_factor"), ITokenService, name="two_factor"
169169
),
170+
pretend.call(
171+
TokenServiceFactory(name="confirm_login"),
172+
ITokenService,
173+
name="confirm_login",
174+
),
170175
pretend.call(
171176
TokenServiceFactory(name="remember_device"),
172177
ITokenService,

tests/unit/accounts/test_models.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,21 @@
77

88
from pyramid.authorization import Authenticated
99

10-
from warehouse.accounts.models import Email, RecoveryCode, User, UserFactory, WebAuthn
10+
from warehouse.accounts.models import (
11+
Email,
12+
RecoveryCode,
13+
User,
14+
UserFactory,
15+
WebAuthn,
16+
)
1117
from warehouse.authnz import Permissions
1218
from warehouse.utils.security_policy import principals_for
1319

1420
from ...common.db.accounts import (
1521
EmailFactory as DBEmailFactory,
1622
UserEventFactory as DBUserEventFactory,
1723
UserFactory as DBUserFactory,
24+
UserUniqueLoginFactory,
1825
)
1926
from ...common.db.packaging import (
2027
ProjectFactory as DBProjectFactory,
@@ -309,3 +316,14 @@ def test_user_projects_is_ordered_by_name(self, db_session):
309316
DBRoleFactory.create(project=project3, user=user)
310317

311318
assert user.projects == [project2, project3, project1]
319+
320+
321+
class TestUserUniqueLogin:
322+
def test_repr(self, db_session):
323+
unique_login = UserUniqueLoginFactory.create()
324+
assert (
325+
repr(unique_login)
326+
== f"<UserUniqueLogin(user={unique_login.user.username!r}, "
327+
f"ip_address={unique_login.ip_address!r}, "
328+
f"status={unique_login.status!r})>"
329+
)

0 commit comments

Comments
 (0)