Skip to content

Don't allow users with unverified emails to upload at all #4292

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 1 commit into from
Jul 13, 2018
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
63 changes: 50 additions & 13 deletions tests/unit/forklift/test_legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,8 @@ def test_fails_invalid_version(self, pyramid_config, pyramid_request, version):
pyramid_request.POST["protocol_version"] = version
pyramid_request.flags = pretend.stub(enabled=lambda *a: False)

pyramid_request.user = pretend.stub(primary_email=pretend.stub(verified=True))

with pytest.raises(HTTPBadRequest) as excinfo:
legacy.file_upload(pyramid_request)

Expand Down Expand Up @@ -864,6 +866,9 @@ def test_fails_invalid_post_data(
self, pyramid_config, db_request, post_data, message
):
pyramid_config.testing_securitypolicy(userid=1)
user = UserFactory.create()
EmailFactory.create(user=user)
db_request.user = user
db_request.POST = MultiDict(post_data)

with pytest.raises(HTTPBadRequest) as excinfo:
Expand Down Expand Up @@ -1045,6 +1050,9 @@ def test_fails_with_admin_flag_set(self, pyramid_config, db_request):
)
admin_flag.enabled = True
pyramid_config.testing_securitypolicy(userid=1)
user = UserFactory.create()
EmailFactory.create(user=user)
db_request.user = user
name = "fails-with-admin-flag"
db_request.POST = MultiDict(
{
Expand Down Expand Up @@ -1078,6 +1086,9 @@ def test_fails_with_admin_flag_set(self, pyramid_config, db_request):

def test_upload_fails_without_file(self, pyramid_config, db_request):
pyramid_config.testing_securitypolicy(userid=1)
user = UserFactory.create()
EmailFactory.create(user=user)
db_request.user = user
db_request.POST = MultiDict(
{
"metadata_version": "1.2",
Expand All @@ -1098,8 +1109,10 @@ def test_upload_fails_without_file(self, pyramid_config, db_request):

@pytest.mark.parametrize("value", [("UNKNOWN"), ("UNKNOWN\n\n")])
def test_upload_cleans_unknown_values(self, pyramid_config, db_request, value):

pyramid_config.testing_securitypolicy(userid=1)
user = UserFactory.create()
EmailFactory.create(user=user)
db_request.user = user
db_request.POST = MultiDict(
{
"metadata_version": "1.2",
Expand All @@ -1117,6 +1130,9 @@ def test_upload_cleans_unknown_values(self, pyramid_config, db_request, value):

def test_upload_escapes_nul_characters(self, pyramid_config, db_request):
pyramid_config.testing_securitypolicy(userid=1)
user = UserFactory.create()
EmailFactory.create(user=user)
db_request.user = user
db_request.POST = MultiDict(
{
"metadata_version": "1.2",
Expand Down Expand Up @@ -1321,6 +1337,7 @@ def test_upload_fails_invlaid_content_type(
pyramid_config.testing_securitypolicy(userid=1)
user = UserFactory.create()
EmailFactory.create(user=user)
db_request.user = user
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
RoleFactory.create(user=user, project=project)
Expand Down Expand Up @@ -1359,6 +1376,7 @@ def test_upload_fails_with_legacy_type(self, pyramid_config, db_request):

user = UserFactory.create()
EmailFactory.create(user=user)
db_request.user = user
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
RoleFactory.create(user=user, project=project)
Expand Down Expand Up @@ -1394,6 +1412,7 @@ def test_upload_fails_with_legacy_ext(self, pyramid_config, db_request):

user = UserFactory.create()
EmailFactory.create(user=user)
db_request.user = user
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
RoleFactory.create(user=user, project=project)
Expand Down Expand Up @@ -1430,6 +1449,7 @@ def test_upload_fails_for_second_sdist(self, pyramid_config, db_request):
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -1470,6 +1490,7 @@ def test_upload_fails_with_invalid_signature(self, pyramid_config, db_request, s
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -1507,6 +1528,7 @@ def test_upload_fails_with_invalid_classifier(self, pyramid_config, db_request):
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -1546,6 +1568,7 @@ def test_upload_fails_with_deprecated_classifier(self, pyramid_config, db_reques
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -1622,6 +1645,7 @@ def test_upload_fails_with_invalid_digest(
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -1659,6 +1683,7 @@ def test_upload_fails_with_invalid_file(self, pyramid_config, db_request):
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -1691,6 +1716,7 @@ def test_upload_fails_with_too_large_file(self, pyramid_config, db_request):
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create(
name="foobar", upload_limit=(60 * 1024 * 1024) # 60 MB
Expand Down Expand Up @@ -1732,6 +1758,7 @@ def test_upload_fails_with_too_large_signature(self, pyramid_config, db_request)
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -1770,6 +1797,7 @@ def test_upload_fails_with_previously_used_filename(
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -1813,6 +1841,7 @@ def test_upload_noop_with_existing_filename_same_content(
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -1859,6 +1888,7 @@ def test_upload_fails_with_existing_filename_diff_content(
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -1910,6 +1940,8 @@ def test_upload_fails_with_diff_filename_same_blake2(
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
RoleFactory.create(user=user, project=project)
Expand Down Expand Up @@ -1961,6 +1993,7 @@ def test_upload_fails_with_wrong_filename(self, pyramid_config, db_request):
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -1999,6 +2032,7 @@ def test_upload_fails_with_invalid_extension(self, pyramid_config, db_request):
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -2039,6 +2073,7 @@ def test_upload_fails_with_unsafe_filename(
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -2430,6 +2465,7 @@ def test_upload_fails_with_unsupported_wheel_plat(
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
db_request.user = user
EmailFactory.create(user=user)
project = ProjectFactory.create()
release = ReleaseFactory.create(project=project, version="1.0")
Expand Down Expand Up @@ -2729,11 +2765,13 @@ def test_upload_succeeds_creates_project(self, pyramid_config, db_request):
@pytest.mark.parametrize(
("emails_verified", "expected_success"),
[
((True,), True),
((False,), False),
((True, True), True),
((True, False), True),
((False, False), False),
([], False),
([True], True),
([False], False),
([True, True], True),
([True, False], True),
([False, False], False),
([False, True], False),
],
)
def test_upload_requires_verified_email(
Expand All @@ -2742,8 +2780,8 @@ def test_upload_requires_verified_email(
pyramid_config.testing_securitypolicy(userid=1)

user = UserFactory.create()
for verified in emails_verified:
EmailFactory.create(user=user, verified=verified)
for i, verified in enumerate(emails_verified):
EmailFactory.create(user=user, verified=verified, primary=i == 0)

filename = "{}-{}.tar.gz".format("example", "1.0")

Expand Down Expand Up @@ -2782,11 +2820,10 @@ def test_upload_requires_verified_email(
assert resp.status_code == 400
assert resp.status == (
(
"400 User {!r} has no verified email "
"addresses, verify at least one "
"address before registering a new project "
"on PyPI. See /the/help/url/ "
"for more information."
"400 User {!r} does not have a verified primary email "
"address. Please add a verified primary email before "
"attempting to upload to PyPI. See /the/help/url/ for "
"more information.for more information."
).format(user.username)
)

Expand Down
38 changes: 20 additions & 18 deletions warehouse/forklift/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,26 @@ def file_upload(request):
HTTPForbidden, "Invalid or non-existent authentication information."
)

# Ensure that user has a verified, primary email address. This should both
# reduce the ease of spam account creation and activty, as well as act as
# a forcing function for https://github.com/pypa/warehouse/issues/3632.
# TODO: Once https://github.com/pypa/warehouse/issues/3632 has been solved,
# we might consider a different condition, possibly looking at
# User.is_active instead.
if not (request.user.primary_email and request.user.primary_email.verified):
raise _exc_with_message(
HTTPBadRequest,
(
"User {!r} does not have a verified primary email address. "
"Please add a verified primary email before attempting to "
"upload to PyPI. See {project_help} for more information."
"for more information."
).format(
request.user.username,
project_help=request.help_url(_anchor="verified-email"),
),
) from None

# Do some cleanup of the various form fields
for key in list(request.POST):
value = request.POST.get(key)
Expand Down Expand Up @@ -796,24 +816,6 @@ def file_upload(request):
).format(projecthelp=request.help_url(_anchor="admin-intervention")),
) from None

# Ensure that user has at least one verified email address. This should
# reduce the ease of spam account creation and activity.
# TODO: Once legacy is shutdown consider the condition here, perhaps
# move to user.is_active or some other boolean
if not any(email.verified for email in request.user.emails):
raise _exc_with_message(
HTTPBadRequest,
(
"User {!r} has no verified email addresses, "
"verify at least one address before registering "
"a new project on PyPI. See {projecthelp} "
"for more information."
).format(
request.user.username,
projecthelp=request.help_url(_anchor="verified-email"),
),
) from None

# Before we create the project, we're going to check our blacklist to
# see if this project is even allowed to be registered. If it is not,
# then we're going to deny the request to create this project.
Expand Down
3 changes: 2 additions & 1 deletion warehouse/templates/pages/help.html
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ <h3 id="verified-email">{{ verified_email() }}</h3>
Currently, PyPI requires a verified email address to perform the following operations:
</p>
<ul>
<li>Register a new project</li>
<li>Register a new project.</li>
<li>Upload a new version or file.</li>
</ul>
<p>
The list of activities that require a verified email address is likely to grow over time.
Expand Down