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

Author: dstufft

tests/unit/forklift/test_legacy.py

@@ -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)
 
@@ -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:
@@ -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(
             {
@@ -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",
@@ -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",
@@ -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",
@@ -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)
@@ -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)
@@ -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)
@@ -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")
@@ -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")
@@ -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")
@@ -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")
@@ -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")
@@ -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")
@@ -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
@@ -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")
@@ -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")
@@ -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")
@@ -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")
@@ -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)
@@ -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")
@@ -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")
@@ -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")
@@ -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")
@@ -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(
@@ -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")
 
@@ -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)
             )
 

warehouse/forklift/legacy.py

@@ -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)
@@ -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.

warehouse/templates/pages/help.html

@@ -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.