Add User.is_frozen (pypi#10992)

tests/unit/accounts/test_core.py

@@ -16,6 +16,8 @@
 import pretend
 import pytest
 
+from pyramid.httpexceptions import HTTPUnauthorized
+
 from warehouse import accounts
 from warehouse.accounts import AuthenticationMethod
 from warehouse.accounts.interfaces import (
@@ -121,32 +123,15 @@ def test_with_disabled_user_no_reason(self, pyramid_request, pyramid_services):
         pyramid_request.matched_route = pretend.stub(name="forklift.legacy.file_upload")
         pyramid_request.help_url = pretend.call_recorder(lambda **kw: "/the/help/url/")
 
-        with pytest.raises(BasicAuthFailedPassword) as excinfo:
+        with pytest.raises(HTTPUnauthorized) as excinfo:
             assert (
                 accounts._basic_auth_check("myuser", "mypass", pyramid_request) is None
             )
 
-        assert excinfo.value.status == (
-            "403 Invalid or non-existent authentication information. "
-            "See /the/help/url/ for more information."
-        )
+        assert excinfo.value.status == "401 Account is disabled."
         assert service.find_userid.calls == [pretend.call("myuser")]
         assert service.get_user.calls == [pretend.call(1)]
         assert service.is_disabled.calls == [pretend.call(1)]
-        assert service.check_password.calls == [
-            pretend.call(
-                1,
-                "mypass",
-                tags=["mechanism:basic_auth", "method:auth", "auth_method:basic"],
-            )
-        ]
-        assert user.record_event.calls == [
-            pretend.call(
-                tag="account:login:failure",
-                ip_address="1.2.3.4",
-                additional={"reason": "invalid_password", "auth_method": "basic"},
-            )
-        ]
 
     def test_with_disabled_user_compromised_pw(self, pyramid_request, pyramid_services):
         user = pretend.stub(id=1)
@@ -179,6 +164,39 @@ def test_with_disabled_user_compromised_pw(self, pyramid_request, pyramid_servic
         assert service.is_disabled.calls == [pretend.call(1)]
         assert service.check_password.calls == []
 
+    def test_with_disabled_user_frozen(self, pyramid_request, pyramid_services):
+        user = pretend.stub(
+            id=1,
+            record_event=pretend.call_recorder(lambda *a, **kw: None),
+            is_frozen=True,
+        )
+        service = pretend.stub(
+            get_user=pretend.call_recorder(lambda user_id: user),
+            find_userid=pretend.call_recorder(lambda username: 1),
+            check_password=pretend.call_recorder(
+                lambda userid, password, tags=None: False
+            ),
+            is_disabled=pretend.call_recorder(
+                lambda user_id: (True, DisableReason.AccountFrozen)
+            ),
+        )
+        pyramid_services.register_service(service, IUserService, None)
+        pyramid_services.register_service(
+            pretend.stub(), IPasswordBreachedService, None
+        )
+        pyramid_request.matched_route = pretend.stub(name="forklift.legacy.file_upload")
+        pyramid_request.help_url = pretend.call_recorder(lambda **kw: "/the/help/url/")
+
+        with pytest.raises(HTTPUnauthorized) as excinfo:
+            assert (
+                accounts._basic_auth_check("myuser", "mypass", pyramid_request) is None
+            )
+
+        assert excinfo.value.status == "401 Account is frozen."
+        assert service.find_userid.calls == [pretend.call("myuser")]
+        assert service.get_user.calls == [pretend.call(1)]
+        assert service.is_disabled.calls == [pretend.call(1)]
+
     def test_with_valid_password(self, monkeypatch, pyramid_request, pyramid_services):
         principals = pretend.stub()
         authenticate = pretend.call_recorder(lambda userid, request: principals)

tests/unit/accounts/test_services.py

@@ -430,6 +430,10 @@ def test_is_disabled(self, user_service, disabled, reason):
             user_service.disable_password(user.id, reason=reason)
         assert user_service.is_disabled(user.id) == (disabled, reason)
 
+    def test_is_disabled_user_frozen(self, user_service):
+        user = UserFactory.create(is_frozen=True)
+        assert user_service.is_disabled(user.id) == (True, DisableReason.AccountFrozen)
+
     def test_updating_password_undisables(self, user_service):
         user = UserFactory.create()
         user_service.disable_password(user.id, reason=DisableReason.CompromisedPassword)

warehouse/accounts/__init__.py

@@ -14,6 +14,7 @@
 import enum
 
 from pyramid.authorization import ACLAuthorizationPolicy
+from pyramid.httpexceptions import HTTPUnauthorized
 from pyramid_multiauth import MultiAuthenticationPolicy
 
 from warehouse.accounts.auth_policy import (
@@ -34,7 +35,11 @@
     database_login_factory,
 )
 from warehouse.email import send_password_compromised_email_hibp
-from warehouse.errors import BasicAuthBreachedPassword, BasicAuthFailedPassword
+from warehouse.errors import (
+    BasicAuthAccountFrozen,
+    BasicAuthBreachedPassword,
+    BasicAuthFailedPassword,
+)
 from warehouse.macaroons.auth_policy import (
     MacaroonAuthenticationPolicy,
     MacaroonAuthorizationPolicy,
@@ -101,7 +106,7 @@ def _basic_auth_check(username, password, request):
     if userid is not None:
         user = login_service.get_user(userid)
         is_disabled, disabled_for = login_service.is_disabled(user.id)
-        if is_disabled and disabled_for == DisableReason.CompromisedPassword:
+        if is_disabled:
             # This technically violates the contract a little bit, this function is
             # meant to return None if the user cannot log in. However we want to present
             # a different error message than is normal when we're denying the log in
@@ -110,9 +115,14 @@ def _basic_auth_check(username, password, request):
             # here because we've already successfully authenticated the credentials, so
             # it won't screw up the fall through to other authentication mechanisms
             # (since we wouldn't have fell through to them anyways).
-            raise _format_exc_status(
-                BasicAuthBreachedPassword(), breach_service.failure_message_plain
-            )
+            if disabled_for == DisableReason.CompromisedPassword:
+                raise _format_exc_status(
+                    BasicAuthBreachedPassword(), breach_service.failure_message_plain
+                )
+            elif disabled_for == DisableReason.AccountFrozen:
+                raise _format_exc_status(BasicAuthAccountFrozen(), "Account is frozen.")
+            else:
+                raise _format_exc_status(HTTPUnauthorized(), "Account is disabled.")
         elif login_service.check_password(
             user.id,
             password,

warehouse/accounts/models.py

@@ -54,6 +54,7 @@ def __getitem__(self, username):
 class DisableReason(enum.Enum):
 
     CompromisedPassword = "password compromised"
+    AccountFrozen = "account frozen"
 
 
 class User(SitemapMixin, db.Model):
@@ -74,6 +75,7 @@ class User(SitemapMixin, db.Model):
     password = Column(String(length=128), nullable=False)
     password_date = Column(TZDateTime, nullable=True, server_default=sql.func.now())
     is_active = Column(Boolean, nullable=False, server_default=sql.false())
+    is_frozen = Column(Boolean, nullable=False, server_default=sql.false())
     is_superuser = Column(Boolean, nullable=False, server_default=sql.false())
     is_moderator = Column(Boolean, nullable=False, server_default=sql.false())
     is_psf_staff = Column(Boolean, nullable=False, server_default=sql.false())

warehouse/accounts/services.py

@@ -43,7 +43,7 @@
     TooManyEmailsAdded,
     TooManyFailedLogins,
 )
-from warehouse.accounts.models import Email, RecoveryCode, User, WebAuthn
+from warehouse.accounts.models import DisableReason, Email, RecoveryCode, User, WebAuthn
 from warehouse.metrics import IMetricsService
 from warehouse.rate_limiting import DummyRateLimiter, IRateLimiter
 from warehouse.utils.crypto import BadData, SignatureExpired, URLSafeTimedSerializer
@@ -280,13 +280,16 @@ def disable_password(self, user_id, reason=None):
     def is_disabled(self, user_id):
         user = self.get_user(user_id)
 
-        # User is not disabled.
-        if self.hasher.is_enabled(user.password):
-            return (False, None)
-        # User is disabled.
-        else:
+        if user.is_frozen:
+            return (True, DisableReason.AccountFrozen)
+
+        # User is disabled due to password being disabled
+        if not self.hasher.is_enabled(user.password):
             return (True, user.disabled_for)
 
+        # User is not disabled.
+        return (False, None)
+
     def has_two_factor(self, user_id):
         """
         Returns True if the user has any form of two factor

warehouse/admin/templates/admin/users/detail.html

@@ -176,6 +176,7 @@ <h3 class="box-title">Permissions</h3>
 
               <div class="box-body">
                 {{ render_field("Active", form.is_active, "is-active") }}
+                {{ render_field("Frozen", form.is_frozen, "is-frozen") }}
                 {{ render_field("Superuser", form.is_superuser, "is-superuser")}}
                 {{ render_field("Moderator", form.is_moderator, "is-moderator")}}
                 {{ render_field("PSF Staff", form.is_psf_staff, "is-psf-staff")}}

warehouse/admin/views/users.py

@@ -86,6 +86,7 @@ class UserForm(forms.Form):
     )
 
     is_active = wtforms.fields.BooleanField()
+    is_frozen = wtforms.fields.BooleanField()
     is_superuser = wtforms.fields.BooleanField()
     is_moderator = wtforms.fields.BooleanField()
     is_psf_staff = wtforms.fields.BooleanField()

warehouse/errors.py

@@ -22,6 +22,10 @@ class BasicAuthBreachedPassword(HTTPUnauthorized):
     pass
 
 
+class BasicAuthAccountFrozen(HTTPUnauthorized):
+    pass
+
+
 class WarehouseDenied(Denied):
     def __new__(cls, s, *args, reason=None, **kwargs):
         inner = super().__new__(cls, s, *args, **kwargs)

warehouse/migrations/versions/fdf9e337538a_add_user_is_frozen.py

@@ -0,0 +1,38 @@
+# 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.
+"""
+Add User.is_frozen
+
+Revision ID: fdf9e337538a
+Revises: 19cf76d2d459
+Create Date: 2022-03-21 17:02:22.924858
+"""
+
+import sqlalchemy as sa
+
+from alembic import op
+
+revision = "fdf9e337538a"
+down_revision = "19cf76d2d459"
+
+
+def upgrade():
+    op.add_column(
+        "users",
+        sa.Column(
+            "is_frozen", sa.Boolean(), server_default=sa.text("false"), nullable=False
+        ),
+    )
+
+
+def downgrade():
+    op.drop_column("users", "is_frozen")