diff --git a/tests/unit/accounts/test_core.py b/tests/unit/accounts/test_core.py index 26278c420e5c..bbdc7c8d2849 100644 --- a/tests/unit/accounts/test_core.py +++ b/tests/unit/accounts/test_core.py @@ -40,7 +40,7 @@ def test_invalid_route(self, pyramid_request, pyramid_services): pretend.stub(), IPasswordBreachedService, None ) pyramid_request.matched_route = pretend.stub(name="route_name") - assert accounts._basic_auth_login("myuser", "mypass", pyramid_request) is None + assert accounts._basic_auth_check("myuser", "mypass", pyramid_request) is None assert service.find_userid.calls == [] def test_with_no_user(self, pyramid_request, pyramid_services): @@ -50,7 +50,7 @@ def test_with_no_user(self, pyramid_request, pyramid_services): pretend.stub(), IPasswordBreachedService, None ) pyramid_request.matched_route = pretend.stub(name="forklift.legacy.file_upload") - assert accounts._basic_auth_login("myuser", "mypass", pyramid_request) is None + assert accounts._basic_auth_check("myuser", "mypass", pyramid_request) is None assert service.find_userid.calls == [pretend.call("myuser")] def test_with_invalid_password(self, pyramid_request, pyramid_services): @@ -75,7 +75,7 @@ def test_with_invalid_password(self, pyramid_request, pyramid_services): with pytest.raises(BasicAuthFailedPassword) as excinfo: assert ( - accounts._basic_auth_login("myuser", "mypass", pyramid_request) is None + accounts._basic_auth_check("myuser", "mypass", pyramid_request) is None ) assert excinfo.value.status == ( @@ -122,7 +122,7 @@ def test_with_disabled_user_no_reason(self, pyramid_request, pyramid_services): with pytest.raises(BasicAuthFailedPassword) as excinfo: assert ( - accounts._basic_auth_login("myuser", "mypass", pyramid_request) is None + accounts._basic_auth_check("myuser", "mypass", pyramid_request) is None ) assert excinfo.value.status == ( @@ -169,7 +169,7 @@ def test_with_disabled_user_compromised_pw(self, pyramid_request, pyramid_servic with pytest.raises(BasicAuthBreachedPassword) as excinfo: assert ( - accounts._basic_auth_login("myuser", "mypass", pyramid_request) is None + accounts._basic_auth_check("myuser", "mypass", pyramid_request) is None ) assert excinfo.value.status == "401 Bad Password!" @@ -183,7 +183,7 @@ def test_with_valid_password(self, monkeypatch, pyramid_request, pyramid_service authenticate = pretend.call_recorder(lambda userid, request: principals) monkeypatch.setattr(accounts, "_authenticate", authenticate) - user = pretend.stub(id=2) + user = pretend.stub(id=2, has_two_factor=False) service = pretend.stub( get_user=pretend.call_recorder(lambda user_id: user), find_userid=pretend.call_recorder(lambda username: 2), @@ -208,7 +208,7 @@ def test_with_valid_password(self, monkeypatch, pyramid_request, pyramid_service with freezegun.freeze_time(now): assert ( - accounts._basic_auth_login("myuser", "mypass", pyramid_request) + accounts._basic_auth_check("myuser", "mypass", pyramid_request) is principals ) @@ -259,7 +259,7 @@ def test_via_basic_auth_compromised( pyramid_request.matched_route = pretend.stub(name="forklift.legacy.file_upload") with pytest.raises(BasicAuthBreachedPassword) as excinfo: - accounts._basic_auth_login("myuser", "mypass", pyramid_request) + accounts._basic_auth_check("myuser", "mypass", pyramid_request) assert excinfo.value.status == "401 Bad Password!" assert service.find_userid.calls == [pretend.call("myuser")] @@ -280,6 +280,64 @@ def test_via_basic_auth_compromised( ] assert send_email.calls == [pretend.call(pyramid_request, user)] + def test_via_basic_auth_2fa_enabled( + self, monkeypatch, pyramid_request, pyramid_services + ): + principals = pretend.stub() + authenticate = pretend.call_recorder(lambda userid, request: principals) + monkeypatch.setattr(accounts, "_authenticate", authenticate) + send_email = pretend.call_recorder(lambda *a, **kw: None) + monkeypatch.setattr( + accounts, "send_basic_auth_with_two_factor_email", send_email + ) + + user = pretend.stub(id=2, has_two_factor=True) + service = pretend.stub( + get_user=pretend.call_recorder(lambda user_id: user), + find_userid=pretend.call_recorder(lambda username: 2), + check_password=pretend.call_recorder( + lambda userid, password, tags=None: True + ), + is_disabled=pretend.call_recorder(lambda user_id: (False, None)), + disable_password=pretend.call_recorder(lambda user_id, reason=None: None), + update_user=pretend.call_recorder(lambda userid, last_login: None), + ) + breach_service = pretend.stub( + check_password=pretend.call_recorder(lambda pw, tags=None: False) + ) + + pyramid_services.register_service(service, IUserService, None) + pyramid_services.register_service( + breach_service, IPasswordBreachedService, None + ) + + pyramid_request.matched_route = pretend.stub(name="forklift.legacy.file_upload") + + now = datetime.datetime.utcnow() + + with freezegun.freeze_time(now): + assert ( + accounts._basic_auth_check("myuser", "mypass", pyramid_request) + is principals + ) + + assert service.find_userid.calls == [pretend.call("myuser")] + assert service.get_user.calls == [pretend.call(2)] + assert service.is_disabled.calls == [pretend.call(2)] + assert service.check_password.calls == [ + pretend.call( + 2, + "mypass", + tags=["mechanism:basic_auth", "method:auth", "auth_method:basic"], + ) + ] + assert breach_service.check_password.calls == [ + pretend.call("mypass", tags=["method:auth", "auth_method:basic"]) + ] + assert send_email.calls == [pretend.call(pyramid_request, user)] + assert service.update_user.calls == [pretend.call(2, last_login=now)] + assert authenticate.calls == [pretend.call(2, pyramid_request)] + class TestAuthenticate: @pytest.mark.parametrize( @@ -372,6 +430,17 @@ def test_route_matched_name_ok(self, monkeypatch): assert authenticate_obj.calls == [pretend.call(1, request)] +class TestMacaroonAuthenticate: + def test_macaroon_authenticate(self, monkeypatch): + authenticate_obj = pretend.call_recorder(lambda *a, **kw: True) + monkeypatch.setattr(accounts, "_authenticate", authenticate_obj) + request = pretend.stub( + matched_route=pretend.stub(name="includes.current-user-indicator") + ) + assert accounts._macaroon_authenticate(1, request) is True + assert authenticate_obj.calls == [pretend.call(1, request)] + + class TestUser: def test_with_user(self): user = pretend.stub() @@ -466,7 +535,7 @@ def test_includeme(monkeypatch): ] assert config.set_authentication_policy.calls == [pretend.call(authn_obj)] assert config.set_authorization_policy.calls == [pretend.call(authz_obj)] - assert basic_authn_cls.calls == [pretend.call(check=accounts._basic_auth_login)] + assert basic_authn_cls.calls == [pretend.call(check=accounts._basic_auth_check)] assert session_authn_cls.calls == [ pretend.call(callback=accounts._session_authenticate) ] diff --git a/tests/unit/email/test_init.py b/tests/unit/email/test_init.py index 5460fd0e7454..e484e79b9797 100644 --- a/tests/unit/email/test_init.py +++ b/tests/unit/email/test_init.py @@ -942,6 +942,77 @@ def test_password_compromised_email( ] +class TestBasicAuthWith2FAEmail: + @pytest.mark.parametrize("verified", [True, False]) + def test_basic_auth_with_2fa_email( + self, pyramid_request, pyramid_config, monkeypatch, verified + ): + stub_user = pretend.stub( + id="id", + username="username", + name="", + email="email@example.com", + primary_email=pretend.stub(email="email@example.com", verified=verified), + ) + subject_renderer = pyramid_config.testing_add_renderer( + "email/basic-auth-with-2fa/subject.txt" + ) + subject_renderer.string_response = "Email Subject" + body_renderer = pyramid_config.testing_add_renderer( + "email/basic-auth-with-2fa/body.txt" + ) + body_renderer.string_response = "Email Body" + html_renderer = pyramid_config.testing_add_renderer( + "email/basic-auth-with-2fa/body.html" + ) + html_renderer.string_response = "Email HTML Body" + + send_email = pretend.stub( + delay=pretend.call_recorder(lambda *args, **kwargs: None) + ) + pyramid_request.task = pretend.call_recorder(lambda *args, **kwargs: send_email) + monkeypatch.setattr(email, "send_email", send_email) + + pyramid_request.db = pretend.stub( + query=lambda a: pretend.stub( + filter=lambda *a: pretend.stub( + one=lambda: pretend.stub(user_id=stub_user.id) + ) + ), + ) + pyramid_request.user = stub_user + pyramid_request.registry.settings = {"mail.sender": "noreply@example.com"} + + result = email.send_basic_auth_with_two_factor_email(pyramid_request, stub_user) + + assert result == {} + assert pyramid_request.task.calls == [pretend.call(send_email)] + assert send_email.delay.calls == [ + pretend.call( + f"{stub_user.username} <{stub_user.email}>", + { + "subject": "Email Subject", + "body_text": "Email Body", + "body_html": ( + "\n\n" + "

Email HTML Body

\n\n" + ), + }, + { + "tag": "account:email:sent", + "user_id": stub_user.id, + "ip_address": pyramid_request.remote_addr, + "additional": { + "from_": "noreply@example.com", + "to": stub_user.email, + "subject": "Email Subject", + "redact_ip": False, + }, + }, + ) + ] + + class TestAccountDeletionEmail: def test_account_deletion_email(self, pyramid_request, pyramid_config, monkeypatch): diff --git a/warehouse/accounts/__init__.py b/warehouse/accounts/__init__.py index c945a1246e09..b28aae396ac1 100644 --- a/warehouse/accounts/__init__.py +++ b/warehouse/accounts/__init__.py @@ -32,7 +32,10 @@ TokenServiceFactory, database_login_factory, ) -from warehouse.email import send_password_compromised_email_hibp +from warehouse.email import ( + send_basic_auth_with_two_factor_email, + send_password_compromised_email_hibp, +) from warehouse.errors import BasicAuthBreachedPassword, BasicAuthFailedPassword from warehouse.macaroons.auth_policy import ( MacaroonAuthenticationPolicy, @@ -51,7 +54,32 @@ def _format_exc_status(exc, message): return exc -def _basic_auth_login(username, password, request): +def _authenticate(userid, request): + """Apply the necessary principals to the authenticated user""" + login_service = request.find_service(IUserService, context=None) + user = login_service.get_user(userid) + + if user is None: + return + + principals = [] + + if user.is_superuser: + principals.append("group:admins") + if user.is_moderator or user.is_superuser: + principals.append("group:moderators") + if user.is_psf_staff or user.is_superuser: + principals.append("group:psf_staff") + + # user must have base admin access if any admin permission + if principals: + principals.append("group:with_admin_dashboard_access") + + return principals + + +def _basic_auth_check(username, password, request): + # Basic authentication can only be used for uploading if request.matched_route.name not in ["forklift.legacy.file_upload"]: return @@ -89,11 +117,13 @@ def _basic_auth_login(username, password, request): raise _format_exc_status( BasicAuthBreachedPassword(), breach_service.failure_message_plain ) - else: - login_service.update_user( - user.id, last_login=datetime.datetime.utcnow() - ) - return _authenticate(user.id, request) + + if user.has_two_factor: + send_basic_auth_with_two_factor_email(request, user) + # Eventually, raise here to disable basic auth with 2FA enabled + + login_service.update_user(user.id, last_login=datetime.datetime.utcnow()) + return _authenticate(user.id, request) else: user.record_event( tag="account:login:failure", @@ -109,36 +139,18 @@ def _basic_auth_login(username, password, request): ) -def _authenticate(userid, request): - login_service = request.find_service(IUserService, context=None) - user = login_service.get_user(userid) - - if user is None: - return - - principals = [] - - if user.is_superuser: - principals.append("group:admins") - if user.is_moderator or user.is_superuser: - principals.append("group:moderators") - if user.is_psf_staff or user.is_superuser: - principals.append("group:psf_staff") - - # user must have base admin access if any admin permission - if principals: - principals.append("group:with_admin_dashboard_access") - - return principals - - def _session_authenticate(userid, request): + # Session authentication cannot be used for uploading if request.matched_route.name in ["forklift.legacy.file_upload"]: return return _authenticate(userid, request) +def _macaroon_authenticate(userid, request): + return _authenticate(userid, request) + + def _user(request): userid = request.authenticated_userid @@ -179,8 +191,8 @@ def includeme(config): MultiAuthenticationPolicy( [ SessionAuthenticationPolicy(callback=_session_authenticate), - BasicAuthAuthenticationPolicy(check=_basic_auth_login), - MacaroonAuthenticationPolicy(callback=_authenticate), + BasicAuthAuthenticationPolicy(check=_basic_auth_check), + MacaroonAuthenticationPolicy(callback=_macaroon_authenticate), ] ) ) diff --git a/warehouse/email/__init__.py b/warehouse/email/__init__.py index 8e38cbb77d64..8fb3fbb1245b 100644 --- a/warehouse/email/__init__.py +++ b/warehouse/email/__init__.py @@ -212,6 +212,11 @@ def send_token_compromised_email_leak(request, user, *, public_url, origin): return {"username": user.username, "public_url": public_url, "origin": origin} +@_email("basic-auth-with-2fa", allow_unverified=True) +def send_basic_auth_with_two_factor_email(request, user): + return {} + + @_email("account-deleted") def send_account_deletion_email(request, user): return {"username": user.username} diff --git a/warehouse/locale/messages.pot b/warehouse/locale/messages.pot index cb3b39a737c9..ef9b73aa5ca3 100644 --- a/warehouse/locale/messages.pot +++ b/warehouse/locale/messages.pot @@ -1235,6 +1235,43 @@ msgid "" "%(initiator_username)s to a project on %(site)s." msgstr "" +#: warehouse/templates/email/basic-auth-with-2fa/body.html:17 +#: warehouse/templates/email/password-compromised-hibp/body.html:18 +#: warehouse/templates/email/password-compromised/body.html:18 +msgid "What?" +msgstr "" + +#: warehouse/templates/email/basic-auth-with-2fa/body.html:19 +#, python-format +msgid "" +"During your recent upload or upload attempt to %(site)s, we noticed you " +"used basic authentication (username & password). However, your " +"account as two-factor authentication (2FA) enabled." +msgstr "" + +#: warehouse/templates/email/basic-auth-with-2fa/body.html:22 +#, python-format +msgid "" +"In the near future, %(site)s will begin prohibiting uploads using basic " +"authentication for accounts with two-factor authentication enabled. " +"Instead, we will require API tokens to be used instead." +msgstr "" + +#: warehouse/templates/email/basic-auth-with-2fa/body.html:25 +#: warehouse/templates/email/password-compromised-hibp/body.html:32 +#: warehouse/templates/email/password-compromised/body.html:31 +msgid "What should I do?" +msgstr "" + +#: warehouse/templates/email/basic-auth-with-2fa/body.html:27 +#, python-format +msgid "" +"First, generate an API token for your account or project at " +"%(new_token_url)s. Then, use this token when publishing instead of your " +"username and password. See %(token_help_url)s for help using API tokens " +"to publish." +msgstr "" + #: warehouse/templates/email/password-change/body.html:18 #, python-format msgid "" @@ -1242,11 +1279,6 @@ msgid "" "%(username)s." msgstr "" -#: warehouse/templates/email/password-compromised-hibp/body.html:18 -#: warehouse/templates/email/password-compromised/body.html:18 -msgid "What?" -msgstr "" - #: warehouse/templates/email/password-compromised/body.html:20 msgid "" "PyPI administrators have determined that your password is compromised. To" @@ -1264,11 +1296,6 @@ msgid "" " risk for PyPI and its users." msgstr "" -#: warehouse/templates/email/password-compromised-hibp/body.html:32 -#: warehouse/templates/email/password-compromised/body.html:31 -msgid "What should I do?" -msgstr "" - #: warehouse/templates/email/password-compromised/body.html:33 #, python-format msgid "" diff --git a/warehouse/templates/email/basic-auth-with-2fa/body.html b/warehouse/templates/email/basic-auth-with-2fa/body.html new file mode 100644 index 000000000000..aa7740c048fb --- /dev/null +++ b/warehouse/templates/email/basic-auth-with-2fa/body.html @@ -0,0 +1,29 @@ +{# + # Licensed under the Apache License, Version 2.0 (the "License"); + # you may not use this file except in compliance with the License. + # You may obtain a copy of the License at + # + # http://www.apache.org/licenses/LICENSE-2.0 + # + # Unless required by applicable law or agreed to in writing, software + # distributed under the License is distributed on an "AS IS" BASIS, + # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + # See the License for the specific language governing permissions and + # limitations under the License. +-#} +{% extends "email/_base/body.html" %} + +{% block content %} +

{% trans %}What?{% endtrans %}

+

+ {% trans site=request.registry.settings["site.name"] %}During your recent upload or upload attempt to {{ site }}, we noticed you used basic authentication (username & password). However, your account as two-factor authentication (2FA) enabled.{% endtrans %} +

+

+ {% trans site=request.registry.settings["site.name"] %}In the near future, {{ site }} will begin prohibiting uploads using basic authentication for accounts with two-factor authentication enabled. Instead, we will require API tokens to be used instead.{% endtrans %} +

+ +

{% trans %}What should I do?{% endtrans %}

+

+ {% trans trimmed new_token_url=request.route_url('manage.account.token', _host=request.registry.settings.get('warehouse.domain')), token_help_url=request.help_url(_anchor='apitoken') %}First, generate an API token for your account or project at {{ new_token_url }}. Then, use this token when publishing instead of your username and password. See {{ token_help_url }} for help using API tokens to publish.{% endtrans %} +

+{% endblock %} diff --git a/warehouse/templates/email/basic-auth-with-2fa/body.txt b/warehouse/templates/email/basic-auth-with-2fa/body.txt new file mode 100644 index 000000000000..c0ff3b66a6d3 --- /dev/null +++ b/warehouse/templates/email/basic-auth-with-2fa/body.txt @@ -0,0 +1,27 @@ +{# + # Licensed under the Apache License, Version 2.0 (the "License"); + # you may not use this file except in compliance with the License. + # You may obtain a copy of the License at + # + # http://www.apache.org/licenses/LICENSE-2.0 + # + # Unless required by applicable law or agreed to in writing, software + # distributed under the License is distributed on an "AS IS" BASIS, + # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + # See the License for the specific language governing permissions and + # limitations under the License. +-#} +{% extends "email/_base/body.txt" %} + +{% block content %} +# {% trans %}What?{% endtrans %} + +{% trans site=request.registry.settings["site.name"] %}During your recent upload or upload attempt to {{ site }}, we noticed you used basic authentication (username & password). However, your account as two-factor authentication (2FA) enabled.{% endtrans %} + +{% trans site=request.registry.settings["site.name"] %}In the near future, {{ site }} will begin prohibiting uploads using basic authentication for accounts with two-factor authentication enabled. Instead, we will require API tokens to be used instead.{% endtrans %} + +# {% trans %}What should I do?{% endtrans %} + +{% trans trimmed new_token_url=request.route_url('manage.account.token', _host=request.registry.settings.get('warehouse.domain')), token_help_url=request.help_url(_anchor='apitoken') %}First, generate an API token for your account or project at {{ new_token_url }}. Then, use this token when publishing instead of your username and password. See {{ token_help_url }} for help using API tokens to publish.{% endtrans %} + +{% endblock %} diff --git a/warehouse/templates/email/basic-auth-with-2fa/subject.txt b/warehouse/templates/email/basic-auth-with-2fa/subject.txt new file mode 100644 index 000000000000..0ccf94b52c19 --- /dev/null +++ b/warehouse/templates/email/basic-auth-with-2fa/subject.txt @@ -0,0 +1,17 @@ +{# + # Licensed under the Apache License, Version 2.0 (the "License"); + # you may not use this file except in compliance with the License. + # You may obtain a copy of the License at + # + # http://www.apache.org/licenses/LICENSE-2.0 + # + # Unless required by applicable law or agreed to in writing, software + # distributed under the License is distributed on an "AS IS" BASIS, + # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + # See the License for the specific language governing permissions and + # limitations under the License. +-#} + +{% extends "email/_base/subject.txt" %} + +{% block subject %}{% trans site=request.registry.settings["site.name"] %}Migrate to API tokens for uploading to {{ site }}{% endtrans %}{% endblock %}