Skip to content

Commit

Permalink
Increase minimum password length to 8 characters
Browse files Browse the repository at this point in the history
This affects newly created passwords, for new or existing accounts, but not
existing logins.

Fixes hypothesis/product-backlog#1523
  • Loading branch information
robertknight committed Oct 26, 2023
1 parent 97f0ed5 commit b7efeca
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 16 deletions.
3 changes: 1 addition & 2 deletions h/accounts/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@
)
from h.schemas import validators
from h.schemas.base import CSRFSchema
from h.schemas.forms.accounts.util import PASSWORD_MIN_LENGTH

_ = i18n.TranslationString
log = logging.getLogger(__name__)

PASSWORD_MIN_LENGTH = 2 # FIXME: this is ridiculous


@lru_cache(maxsize=None)
def get_blacklist():
Expand Down
2 changes: 1 addition & 1 deletion h/schemas/forms/accounts/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

from h.schemas import validators

PASSWORD_MIN_LENGTH = 2 # FIXME: this is ridiculous
PASSWORD_MIN_LENGTH = 8


def new_password_node(**kwargs):
Expand Down
10 changes: 5 additions & 5 deletions tests/h/accounts/schemas_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def test_it_is_invalid_when_password_too_short(self, pyramid_request):

with pytest.raises(colander.Invalid) as exc:
schema.deserialize({"password": "a"})
assert exc.value.asdict()["password"] == ("Must be 2 characters or more.")
assert exc.value.asdict()["password"] == ("Must be 8 characters or more.")

def test_it_is_invalid_when_username_too_short(self, pyramid_request, user_model):
schema = schemas.RegisterSchema().bind(request=pyramid_request)
Expand Down Expand Up @@ -273,8 +273,8 @@ def test_it_is_invalid_if_passwords_dont_match(self, pyramid_csrf_request):
with pytest.raises(colander.Invalid) as exc:
schema.deserialize(
{
"new_password": "wibble",
"new_password_confirm": "wibble!",
"new_password": "foo-bar-baz",
"new_password_confirm": "foo-bar-buzz",
"password": "flibble",
}
)
Expand All @@ -292,8 +292,8 @@ def test_it_is_invalid_if_current_password_is_wrong(
with pytest.raises(colander.Invalid) as exc:
schema.deserialize(
{
"new_password": "wibble",
"new_password_confirm": "wibble",
"new_password": "foo-bar-baz",
"new_password_confirm": "foo-bar-baz",
"password": "flibble",
}
)
Expand Down
18 changes: 10 additions & 8 deletions tests/h/schemas/forms/accounts/reset_password_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
@pytest.mark.usefixtures("pyramid_config")
class TestResetPasswordSchemaDeserialize:
def test_it_is_valid_with_a_long_password(self, schema):
# Yeah... our minimum password length is 2 chars. See
# Yeah... our minimum password length is 8 chars. See
# `h.schema.forms.accounts.util`
schema.deserialize({"user": "*any*", "password": "aa"})
schema.deserialize({"user": "*any*", "password": "new-password"})

@pytest.mark.parametrize("password", ("", "a"))
def test_it_is_invalid_with_password_too_short(self, schema, password):
Expand All @@ -25,23 +25,23 @@ def test_it_is_invalid_with_password_too_short(self, schema, password):

def test_it_is_invalid_with_a_null_user(self, schema):
with pytest.raises(colander.Invalid) as exc:
schema.deserialize({"user": colander.null, "password": "*any*"})
schema.deserialize({"user": colander.null, "password": "new-password"})

assert "user" in exc.value.asdict()

def test_it_is_invalid_with_a_missing_user(self, schema, models):
models.User.get_by_username.return_value = None

with pytest.raises(colander.Invalid) as exc:
schema.deserialize({"user": "encoded_token", "password": "*any*"})
schema.deserialize({"user": "encoded_token", "password": "new-password"})

assert "user" in exc.value.asdict()

def test_it_is_invalid_with_invalid_user_token(self, schema, serializer):
serializer.loads.side_effect = BadData("Invalid token")

with pytest.raises(colander.Invalid) as exc:
schema.deserialize({"user": "INVALID_TOKEN", "password": "*any*"})
schema.deserialize({"user": "INVALID_TOKEN", "password": "new-password"})

assert "user" in exc.value.asdict()
assert "Wrong reset code." in exc.value.asdict()["user"]
Expand All @@ -50,7 +50,7 @@ def test_it_is_invalid_with_expired_token(self, schema, serializer):
serializer.loads.side_effect = SignatureExpired("Token has expired")

with pytest.raises(colander.Invalid) as exc:
schema.deserialize({"user": "encoded_token", "password": "*any*"})
schema.deserialize({"user": "encoded_token", "password": "new-password"})

serializer.loads.assert_called_once_with(
"encoded_token", max_age=72 * 3600, return_timestamp=True
Expand All @@ -74,7 +74,9 @@ def test_it_returns_user_when_valid(
):
user.password_updated = password_updated

appstruct = schema.deserialize({"user": "encoded_token", "password": "secret"})
appstruct = schema.deserialize(
{"user": "encoded_token", "password": "new-password"}
)

models.User.get_by_username.assert_called_once_with(
pyramid_csrf_request.db,
Expand All @@ -89,7 +91,7 @@ def test_it_is_invalid_if_user_has_already_reset_their_password(self, schema, us
user.password_updated = datetime.now() + timedelta(days=1)

with pytest.raises(colander.Invalid) as exc:
schema.deserialize({"user": "EXPIRED_TOKEN", "password": "*any*"})
schema.deserialize({"user": "EXPIRED_TOKEN", "password": "new-password"})

assert "user" in exc.value.asdict()
assert "This reset code has already been used." in exc.value.asdict()["user"]
Expand Down

0 comments on commit b7efeca

Please sign in to comment.