Skip to content

2FA basic auth notification email #13831

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.

Sign up for GitHub

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 3 commits into from
Jun 1, 2023
Merged

2FA basic auth notification email #13831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1374ad4
Update notification email
di Jun 1, 2023
cc8be43
Move basic auth check, send updated email
di Jun 1, 2023
65d90d7
Update translations
di Jun 1, 2023
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
56 changes: 1 addition & 55 deletions tests/unit/accounts/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,7 @@
TokenServiceFactory,
database_login_factory,
)
from warehouse.errors import (
BasicAuthBreachedPassword,
BasicAuthFailedPassword,
BasicAuthTwoFactorEnabled,
)
from warehouse.errors import BasicAuthBreachedPassword, BasicAuthFailedPassword
from warehouse.events.tags import EventTag
from warehouse.oidc.models import OIDCPublisher
from warehouse.rate_limiting import IRateLimiter, RateLimit
Expand Down Expand Up @@ -80,7 +76,6 @@ def test_with_invalid_password(self, pyramid_request, pyramid_services):
lambda userid, password, tags=None: False
),
is_disabled=pretend.call_recorder(lambda user_id: (False, None)),
has_two_factor=pretend.call_recorder(lambda uid: False),
)
pyramid_services.register_service(service, IUserService, None)
pyramid_services.register_service(
Expand Down Expand Up @@ -217,7 +212,6 @@ def test_with_valid_password(self, monkeypatch, pyramid_request, pyramid_service
),
update_user=pretend.call_recorder(lambda userid, last_login: None),
is_disabled=pretend.call_recorder(lambda user_id: (False, None)),
has_two_factor=pretend.call_recorder(lambda uid: False),
)
breach_service = pretend.stub(
check_password=pretend.call_recorder(lambda pw, tags=None: False)
Expand All @@ -238,7 +232,6 @@ def test_with_valid_password(self, monkeypatch, pyramid_request, pyramid_service
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.has_two_factor.calls == [pretend.call(2)]
assert service.check_password.calls == [
pretend.call(
2,
Expand All @@ -259,52 +252,6 @@ def test_with_valid_password(self, monkeypatch, pyramid_request, pyramid_service
)
]

def test_via_basic_auth_2fa_enforced(
self, monkeypatch, pyramid_request, pyramid_services
):
send_email = pretend.call_recorder(lambda *a, **kw: None)

user = pretend.stub(id=2, username="multiFactor")
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, request, reason=None: None
),
has_two_factor=pretend.call_recorder(lambda uid: True),
)
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")

with pytest.raises(BasicAuthTwoFactorEnabled) as excinfo:
_basic_auth_check("multiFactor", "mypass", pyramid_request)

assert excinfo.value.status == (
"401 User multiFactor has two factor auth enabled, "
"an API Token or Trusted Publisher must be used "
"to upload in place of password."
)
assert service.find_userid.calls == [pretend.call("multiFactor")]
assert service.get_user.calls == [pretend.call(2)]
assert service.is_disabled.calls == [pretend.call(2)]
assert service.has_two_factor.calls == [pretend.call(2)]
assert service.check_password.calls == []
assert breach_service.check_password.calls == []
assert service.disable_password.calls == []
assert send_email.calls == []

def test_via_basic_auth_compromised(
self, monkeypatch, pyramid_request, pyramid_services
):
Expand All @@ -324,7 +271,6 @@ def test_via_basic_auth_compromised(
disable_password=pretend.call_recorder(
lambda user_id, request, reason=None: None
),
has_two_factor=pretend.call_recorder(lambda uid: False),
)
breach_service = pretend.stub(
check_password=pretend.call_recorder(lambda pw, tags=None: True),
Expand Down
14 changes: 12 additions & 2 deletions tests/unit/forklift/test_legacy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

from warehouse.admin.flags import AdminFlag, AdminFlagValue
from warehouse.classifiers.models import Classifier
from warehouse.errors import BasicAuthTwoFactorEnabled
from warehouse.forklift import legacy
from warehouse.metrics import IMetricsService
from warehouse.packaging.interfaces import IFileStorage, IProjectService
Expand Down Expand Up @@ -2556,7 +2557,7 @@ def test_upload_fails_without_oidc_publisher_permission(
"See /the/help/url/ for more information."
).format(project.name)

def test_upload_succeeds_with_2fa_enabled(
def test_basic_auth_upload_fails_with_2fa_enabled(
self, pyramid_config, db_request, metrics, monkeypatch
):
user = UserFactory.create(totp_secret=b"secret")
Expand Down Expand Up @@ -2593,8 +2594,17 @@ def test_upload_succeeds_with_2fa_enabled(
IMetricsService: metrics,
}.get(svc)

legacy.file_upload(db_request)
with pytest.raises(BasicAuthTwoFactorEnabled) as excinfo:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably change name of this test now

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh? Is the change to test_basic_auth_upload_fails_with_2fa_enabled not sufficient?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

I guess weird folding in UI

legacy.file_upload(db_request)

resp = excinfo.value

assert resp.status_code == 401
assert resp.status == (
f"401 User { user.username } has two factor auth enabled, "
"an API Token or Trusted Publisher must be used to upload "
"in place of password."
)
assert send_email.calls == [
pretend.call(db_request, user, project_name=project.name)
]
Expand Down
10 changes: 0 additions & 10 deletions warehouse/accounts/security_policy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
BasicAuthAccountFrozen,
BasicAuthBreachedPassword,
BasicAuthFailedPassword,
BasicAuthTwoFactorEnabled,
WarehouseDenied,
)
from warehouse.events.tags import EventTag
Expand Down Expand Up @@ -76,15 +75,6 @@ def _basic_auth_check(username, password, request):
raise _format_exc_status(BasicAuthAccountFrozen(), "Account is frozen.")
else:
raise _format_exc_status(HTTPUnauthorized(), "Account is disabled.")
elif login_service.has_two_factor(user.id):
raise _format_exc_status(
BasicAuthTwoFactorEnabled(),
(
f"User {user.username} has two factor auth enabled, "
"an API Token or Trusted Publisher must be used to upload "
"in place of password."
),
)
elif login_service.check_password(
user.id,
password,
Expand Down
10 changes: 9 additions & 1 deletion warehouse/forklift/legacy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
send_basic_auth_with_two_factor_email,
send_gpg_signature_uploaded_email,
)
from warehouse.errors import BasicAuthTwoFactorEnabled
from warehouse.events.tags import EventTag
from warehouse.metrics import IMetricsService
from warehouse.packaging.interfaces import IFileStorage, IProjectService
Expand Down Expand Up @@ -1017,10 +1018,17 @@ def file_upload(request):
request.authentication_method == AuthenticationMethod.BASIC_AUTH
and request.user.has_two_factor
):
# Eventually, raise here to disable basic auth with 2FA enabled
send_basic_auth_with_two_factor_email(
request, request.user, project_name=project.name
)
raise _exc_with_message(
BasicAuthTwoFactorEnabled,
(
f"User { request.user.username } has two factor auth enabled, "
"an API Token or Trusted Publisher must be used to upload "
"in place of password."
),
)

# Update name if it differs but is still equivalent. We don't need to check if
# they are equivalent when normalized because that's already been done when we
Expand Down
21 changes: 13 additions & 8 deletions warehouse/locale/messages.pot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1744,17 +1744,17 @@ msgstr ""
#, python-format
msgid ""
"During your recent upload or upload attempt of %(project_name)s to "
"%(site)s, we noticed you used basic authentication (username & "
"password). However, your account has two-factor authentication (2FA) "
"enabled."
"%(site)s, we noticed you attempted to use basic authentication (username "
"& password). However, your account has 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."
"%(site)s now prohibits uploads using basic authentication for accounts "
"with two-factor authentication enabled. Instead, we require API tokens or"
" Trusted Publishers to be used instead."
msgstr ""

#: warehouse/templates/email/basic-auth-with-2fa/body.html:25
Expand All @@ -1767,12 +1767,17 @@ 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 "
"If using API tokens, 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/basic-auth-with-2fa/body.html:30
#, python-format
msgid "If using Trusted Publishers (preferred), see %(tp_url)s to get started."
msgstr ""

#: warehouse/templates/email/canceled-as-invited-organization-member/body.html:19
#, python-format
msgid ""
Expand Down
9 changes: 6 additions & 3 deletions warehouse/templates/email/basic-auth-with-2fa/body.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@
{% block content %}
<h3>{% trans %}What?{% endtrans %}</h3>
<p>
{% trans site=request.registry.settings["site.name"] %}During your recent upload or upload attempt of {{ project_name }} to {{ site }}, we noticed you used basic authentication (username &amp; password). However, your account has two-factor authentication (2FA) enabled.{% endtrans %}
{% trans site=request.registry.settings["site.name"] %}During your recent upload or upload attempt of {{ project_name }} to {{ site }}, we noticed you attempted to use basic authentication (username &amp; password). However, your account has two-factor authentication (2FA) enabled.{% endtrans %}
</p>
<p>
{% 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.{% endtrans %}
{% trans site=request.registry.settings["site.name"] %}{{ site }} now prohibits uploads using basic authentication for accounts with two-factor authentication enabled. Instead, we require API tokens or Trusted Publishers to be used instead.{% endtrans %}
</p>

<h3>{% trans %}What should I do?{% endtrans %}</h3>
<p>
{% trans 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 %}
{% trans new_token_url=request.route_url('manage.account.token', _host=request.registry.settings.get('warehouse.domain')), token_help_url=request.help_url(_anchor='apitoken') %}If using API tokens, 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 %}
</p>
<p>
{% trans tp_url="https://docs.pypi.org/trusted-publishers/" %}If using Trusted Publishers (preferred), see {{ tp_url }} to get started.{% endtrans %}
</p>
{% endblock %}
8 changes: 5 additions & 3 deletions warehouse/templates/email/basic-auth-with-2fa/body.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
{% block content %}
# {% trans %}What?{% endtrans %}

{% trans site=request.registry.settings["site.name"] %}During your recent upload or upload attempt of {{ project_name }} to {{ site }}, we noticed you used basic authentication (username & password). However, your account has two-factor authentication (2FA) enabled.{% endtrans %}
{% trans site=request.registry.settings["site.name"] %}During your recent upload or upload attempt of {{ project_name }} to {{ site }}, we noticed you attempted to use basic authentication (username & password). However, your account has 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.{% endtrans %}
{% trans site=request.registry.settings["site.name"] %}{{ site }} now prohibits uploads using basic authentication for accounts with two-factor authentication enabled. Instead, we require API tokens or Trusted Publishers to be used instead.{% endtrans %}

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

{% trans 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 %}
{% trans new_token_url=request.route_url('manage.account.token', _host=request.registry.settings.get('warehouse.domain')), token_help_url=request.help_url(_anchor='apitoken') %}If using API tokens, 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 %}

{% trans tp_url="https://docs.pypi.org/trusted-publishers/" %}If using Trusted Publishers (preferred), see {{ tp_url }} to get started.{% endtrans %}

{% endblock %}
2 changes: 1 addition & 1 deletion warehouse/templates/email/basic-auth-with-2fa/subject.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@

{% extends "email/_base/subject.txt" %}

{% block subject %}{% trans site=request.registry.settings["site.name"] %}Migrate to API tokens for uploading to {{ site }}{% endtrans %}{% endblock %}
{% block subject %}{% trans site=request.registry.settings["site.name"] %}Migrate authentication for uploading to {{ site }}{% endtrans %}{% endblock %}