From b07959dad2bdc3ad8fb68faea729121a8a01f454 Mon Sep 17 00:00:00 2001 From: TemoSulava Date: Sat, 22 Aug 2026 19:01:31 +0400 Subject: [PATCH 1/5] fix(api): redirect instead of 500 on an invalid password reset link ResetPasswordSpaceEndpoint looked the user up inside a try block that only caught DjangoUnicodeDecodeError, so a reset link whose uidb64 decodes to an unknown UUID raised User.DoesNotExist, one that decodes to a non-UUID string raised ValidationError, and one that is not decodable base64 raised ValueError - all three surfaced as unhandled 500s instead of the invalid-link page. Handle those cases on both reset endpoints and redirect to the reset-password page with INVALID_PASSWORD_TOKEN, keeping EXPIRED_PASSWORD_TOKEN for an undecodable uidb64. Because DjangoUnicodeDecodeError subclasses ValueError, that clause has to come first - in the app endpoint it sat on an outer try below `except (ValueError, ...)`, so it was already unreachable and an undecodable uidb64 answered 5125 there and 5130 on the space endpoint. Both endpoints now answer identical input identically. Adds contract tests covering every branch of both endpoints; the five that target the crash paths fail against the unpatched views. Fixes #9172 --- .../views/app/password_management.py | 124 +++++----- .../views/space/password_management.py | 82 ++++--- .../tests/contract/app/test_password_reset.py | 211 ++++++++++++++++++ 3 files changed, 318 insertions(+), 99 deletions(-) create mode 100644 apps/api/plane/tests/contract/app/test_password_reset.py diff --git a/apps/api/plane/authentication/views/app/password_management.py b/apps/api/plane/authentication/views/app/password_management.py index 48b54dcccb4..b61158dcc6e 100644 --- a/apps/api/plane/authentication/views/app/password_management.py +++ b/apps/api/plane/authentication/views/app/password_management.py @@ -100,77 +100,77 @@ class ResetPasswordEndpoint(View): def post(self, request, uidb64, token): try: # Decode the id from the uidb64 - try: - id = smart_str(urlsafe_base64_decode(uidb64)) - user = User.objects.get(id=id) - except (ValueError, User.DoesNotExist): - exc = AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD_TOKEN"], - error_message="INVALID_PASSWORD_TOKEN", - ) - params = exc.get_error_dict() - url = urljoin( - base_host(request=request, is_app=True), - "accounts/reset-password?" + urlencode(params), - ) - return HttpResponseRedirect(url) - - # check if the token is valid for the user - if not PasswordResetTokenGenerator().check_token(user, token): - exc = AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD_TOKEN"], - error_message="INVALID_PASSWORD_TOKEN", - ) - params = exc.get_error_dict() - url = urljoin( - base_host(request=request, is_app=True), - "accounts/reset-password?" + urlencode(params), - ) - return HttpResponseRedirect(url) - - password = request.POST.get("password", False) - - if not password: - exc = AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD"], - error_message="INVALID_PASSWORD", - ) - url = urljoin( - base_host(request=request, is_app=True), - "accounts/reset-password?" + urlencode(exc.get_error_dict()), - ) - return HttpResponseRedirect(url) - - # Check the password complexity - results = zxcvbn(password) - if results["score"] < 3: - exc = AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES["PASSWORD_TOO_WEAK"], - error_message="PASSWORD_TOO_WEAK", - ) - url = urljoin( - base_host(request=request, is_app=True), - "accounts/reset-password?" + urlencode(exc.get_error_dict()), - ) - return HttpResponseRedirect(url) - - # set_password also hashes the password that the user will get - user.set_password(password) - user.is_password_autoset = False - user.save() + id = smart_str(urlsafe_base64_decode(uidb64)) + user = User.objects.get(id=id) + except DjangoUnicodeDecodeError: + exc = AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["EXPIRED_PASSWORD_TOKEN"], + error_message="EXPIRED_PASSWORD_TOKEN", + ) + url = urljoin( + base_host(request=request, is_app=True), + "accounts/reset-password?" + urlencode(exc.get_error_dict()), + ) + return HttpResponseRedirect(url) + except (ValueError, ValidationError, User.DoesNotExist): + # Malformed base64, a non-UUID id or an id that matches no user + exc = AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD_TOKEN"], + error_message="INVALID_PASSWORD_TOKEN", + ) + params = exc.get_error_dict() + url = urljoin( + base_host(request=request, is_app=True), + "accounts/reset-password?" + urlencode(params), + ) + return HttpResponseRedirect(url) + + # check if the token is valid for the user + if not PasswordResetTokenGenerator().check_token(user, token): + exc = AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD_TOKEN"], + error_message="INVALID_PASSWORD_TOKEN", + ) + params = exc.get_error_dict() + url = urljoin( + base_host(request=request, is_app=True), + "accounts/reset-password?" + urlencode(params), + ) + return HttpResponseRedirect(url) + + password = request.POST.get("password", False) + if not password: + exc = AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD"], + error_message="INVALID_PASSWORD", + ) url = urljoin( base_host(request=request, is_app=True), - "sign-in?" + urlencode({"success": True}), + "accounts/reset-password?" + urlencode(exc.get_error_dict()), ) return HttpResponseRedirect(url) - except DjangoUnicodeDecodeError: + + # Check the password complexity + results = zxcvbn(password) + if results["score"] < 3: exc = AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES["EXPIRED_PASSWORD_TOKEN"], - error_message="EXPIRED_PASSWORD_TOKEN", + error_code=AUTHENTICATION_ERROR_CODES["PASSWORD_TOO_WEAK"], + error_message="PASSWORD_TOO_WEAK", ) url = urljoin( base_host(request=request, is_app=True), "accounts/reset-password?" + urlencode(exc.get_error_dict()), ) return HttpResponseRedirect(url) + + # set_password also hashes the password that the user will get + user.set_password(password) + user.is_password_autoset = False + user.save() + + url = urljoin( + base_host(request=request, is_app=True), + "sign-in?" + urlencode({"success": True}), + ) + return HttpResponseRedirect(url) diff --git a/apps/api/plane/authentication/views/space/password_management.py b/apps/api/plane/authentication/views/space/password_management.py index ed6682d74ae..47d111323aa 100644 --- a/apps/api/plane/authentication/views/space/password_management.py +++ b/apps/api/plane/authentication/views/space/password_management.py @@ -114,43 +114,6 @@ def post(self, request, uidb64, token): # Decode the id from the uidb64 id = smart_str(urlsafe_base64_decode(uidb64)) user = User.objects.get(id=id) - - # check if the token is valid for the user - if not PasswordResetTokenGenerator().check_token(user, token): - exc = AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD_TOKEN"], - error_message="INVALID_PASSWORD_TOKEN", - ) - params = exc.get_error_dict() - url = f"{base_host(request=request, is_space=True)}/accounts/reset-password/?{urlencode(params)}" - return HttpResponseRedirect(url) - - password = request.POST.get("password", False) - - if not password: - exc = AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD"], - error_message="INVALID_PASSWORD", - ) - url = f"{base_host(request=request, is_space=True)}/accounts/reset-password/?{urlencode(exc.get_error_dict())}" # noqa: E501 - return HttpResponseRedirect(url) - - # Check the password complexity - results = zxcvbn(password) - if results["score"] < 3: - exc = AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES["PASSWORD_TOO_WEAK"], - error_message="PASSWORD_TOO_WEAK", - ) - url = f"{base_host(request=request, is_space=True)}/accounts/reset-password/?{urlencode(exc.get_error_dict())}" # noqa: E501 - return HttpResponseRedirect(url) - - # set_password also hashes the password that the user will get - user.set_password(password) - user.is_password_autoset = False - user.save() - - return HttpResponseRedirect(base_host(request=request, is_space=True)) except DjangoUnicodeDecodeError: exc = AuthenticationException( error_code=AUTHENTICATION_ERROR_CODES["EXPIRED_PASSWORD_TOKEN"], @@ -158,3 +121,48 @@ def post(self, request, uidb64, token): ) url = f"{base_host(request=request, is_space=True)}/accounts/reset-password/?{urlencode(exc.get_error_dict())}" # noqa: E501 return HttpResponseRedirect(url) + except (ValueError, ValidationError, User.DoesNotExist): + # Malformed base64, a non-UUID id or an id that matches no user + exc = AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD_TOKEN"], + error_message="INVALID_PASSWORD_TOKEN", + ) + url = f"{base_host(request=request, is_space=True)}/accounts/reset-password/?{urlencode(exc.get_error_dict())}" # noqa: E501 + return HttpResponseRedirect(url) + + # check if the token is valid for the user + if not PasswordResetTokenGenerator().check_token(user, token): + exc = AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD_TOKEN"], + error_message="INVALID_PASSWORD_TOKEN", + ) + params = exc.get_error_dict() + url = f"{base_host(request=request, is_space=True)}/accounts/reset-password/?{urlencode(params)}" + return HttpResponseRedirect(url) + + password = request.POST.get("password", False) + + if not password: + exc = AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD"], + error_message="INVALID_PASSWORD", + ) + url = f"{base_host(request=request, is_space=True)}/accounts/reset-password/?{urlencode(exc.get_error_dict())}" # noqa: E501 + return HttpResponseRedirect(url) + + # Check the password complexity + results = zxcvbn(password) + if results["score"] < 3: + exc = AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["PASSWORD_TOO_WEAK"], + error_message="PASSWORD_TOO_WEAK", + ) + url = f"{base_host(request=request, is_space=True)}/accounts/reset-password/?{urlencode(exc.get_error_dict())}" # noqa: E501 + return HttpResponseRedirect(url) + + # set_password also hashes the password that the user will get + user.set_password(password) + user.is_password_autoset = False + user.save() + + return HttpResponseRedirect(base_host(request=request, is_space=True)) diff --git a/apps/api/plane/tests/contract/app/test_password_reset.py b/apps/api/plane/tests/contract/app/test_password_reset.py new file mode 100644 index 00000000000..cd7db399026 --- /dev/null +++ b/apps/api/plane/tests/contract/app/test_password_reset.py @@ -0,0 +1,211 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +import uuid + +import pytest +from django.contrib.auth.tokens import PasswordResetTokenGenerator +from django.test import Client +from django.utils.encoding import smart_bytes +from django.utils.http import urlsafe_base64_encode + +from plane.authentication.adapter.error import AUTHENTICATION_ERROR_CODES +from plane.db.models import User + +STRONG_PASSWORD = "correct-horse-battery-staple-9x" + + +@pytest.fixture(autouse=True) +def _pin_web_url(settings): + """Pin the redirect host so base_host() does not depend on the ambient environment""" + settings.WEB_URL = "http://testserver" + settings.SPACE_BASE_URL = None + settings.APP_BASE_URL = None + + +@pytest.fixture +def django_client(): + """Return a Django test client with a User-Agent header for handling redirects""" + return Client(HTTP_USER_AGENT="Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:15.0) Gecko/20100101 Firefox/15.0.1") + + +@pytest.fixture +def reset_user(db): + """Create a user that owns a valid password reset link""" + user = User.objects.create(email="reset-user@plane.so", is_password_autoset=True) + user.set_password("user@123") + user.save() + return user + + +def _encode(value): + return urlsafe_base64_encode(smart_bytes(value)) + + +# The reset-password patterns share their url names with the forgot-password +# ones, so reverse() resolves to the wrong view - build the paths by hand. +def _space_url(uidb64, token): + return f"/auth/spaces/reset-password/{uidb64}/{token}/" + + +def _app_url(uidb64, token): + return f"/auth/reset-password/{uidb64}/{token}/" + + +@pytest.mark.contract +class TestResetPasswordSpaceEndpoint: + """The space reset-password endpoint must redirect - never 500 - on a bad uidb64""" + + @pytest.mark.django_db + def test_unknown_user_id_redirects(self, django_client, reset_user): + """A well formed uidb64 for a user that does not exist redirects with INVALID_PASSWORD_TOKEN""" + uidb64 = _encode(uuid.uuid4()) + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_space_url(uidb64, token), {"password": STRONG_PASSWORD}) + + assert response.status_code == 302 + assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + + @pytest.mark.django_db + def test_non_uuid_id_redirects(self, django_client, reset_user): + """A uidb64 that decodes to something that is not a UUID redirects instead of raising ValidationError""" + uidb64 = _encode("not-a-uuid") + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_space_url(uidb64, token), {"password": STRONG_PASSWORD}) + + assert response.status_code == 302 + assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + + @pytest.mark.django_db + def test_malformed_base64_redirects(self, django_client, reset_user): + """A uidb64 that is not decodable base64 redirects instead of raising ValueError""" + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_space_url("a", token), {"password": STRONG_PASSWORD}) + + assert response.status_code == 302 + assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + + @pytest.mark.django_db + def test_undecodable_uidb64_redirects(self, django_client, reset_user): + """A uidb64 that decodes to invalid utf-8 keeps the EXPIRED_PASSWORD_TOKEN response""" + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_space_url("not", token), {"password": STRONG_PASSWORD}) + + assert response.status_code == 302 + assert f"error_code={AUTHENTICATION_ERROR_CODES['EXPIRED_PASSWORD_TOKEN']}" in response["Location"] + + @pytest.mark.django_db + def test_missing_password_redirects(self, django_client, reset_user): + """A valid link without a password is still rejected with INVALID_PASSWORD""" + uidb64 = _encode(reset_user.id) + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_space_url(uidb64, token), {}) + + assert response.status_code == 302 + assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD']}" in response["Location"] + + @pytest.mark.django_db + def test_invalid_token_redirects(self, django_client, reset_user): + """An existing user with a token that does not belong to them is still rejected""" + uidb64 = _encode(reset_user.id) + + response = django_client.post(_space_url(uidb64, "invalid-token"), {"password": STRONG_PASSWORD}) + + assert response.status_code == 302 + assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + reset_user.refresh_from_db() + assert not reset_user.check_password(STRONG_PASSWORD) + + @pytest.mark.django_db + def test_weak_password_redirects(self, django_client, reset_user): + """A valid link with a weak password does not change the password""" + uidb64 = _encode(reset_user.id) + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_space_url(uidb64, token), {"password": "password"}) + + assert response.status_code == 302 + assert f"error_code={AUTHENTICATION_ERROR_CODES['PASSWORD_TOO_WEAK']}" in response["Location"] + reset_user.refresh_from_db() + assert not reset_user.check_password("password") + + @pytest.mark.django_db + def test_valid_link_resets_password(self, django_client, reset_user): + """A valid link with a strong password still resets the password""" + uidb64 = _encode(reset_user.id) + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_space_url(uidb64, token), {"password": STRONG_PASSWORD}) + + assert response.status_code == 302 + assert "error_code" not in response["Location"] + reset_user.refresh_from_db() + assert reset_user.check_password(STRONG_PASSWORD) + assert reset_user.is_password_autoset is False + + +@pytest.mark.contract +class TestResetPasswordAppEndpoint: + """The app reset-password endpoint must redirect - never 500 - on a bad uidb64""" + + @pytest.mark.django_db + def test_non_uuid_id_redirects(self, django_client, reset_user): + """A uidb64 that decodes to something that is not a UUID redirects instead of raising ValidationError""" + uidb64 = _encode("not-a-uuid") + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_app_url(uidb64, token), {"password": STRONG_PASSWORD}) + + assert response.status_code == 302 + assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + + @pytest.mark.django_db + def test_unknown_user_id_redirects(self, django_client, reset_user): + """A well formed uidb64 for a user that does not exist redirects with INVALID_PASSWORD_TOKEN""" + uidb64 = _encode(uuid.uuid4()) + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_app_url(uidb64, token), {"password": STRONG_PASSWORD}) + + assert response.status_code == 302 + assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + + @pytest.mark.django_db + def test_malformed_base64_redirects(self, django_client, reset_user): + """A uidb64 that is not decodable base64 redirects instead of raising ValueError""" + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_app_url("a", token), {"password": STRONG_PASSWORD}) + + assert response.status_code == 302 + assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + + @pytest.mark.django_db + def test_undecodable_uidb64_redirects(self, django_client, reset_user): + """A uidb64 that decodes to invalid utf-8 keeps the EXPIRED_PASSWORD_TOKEN response""" + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_app_url("not", token), {"password": STRONG_PASSWORD}) + + assert response.status_code == 302 + assert f"error_code={AUTHENTICATION_ERROR_CODES['EXPIRED_PASSWORD_TOKEN']}" in response["Location"] + + @pytest.mark.django_db + def test_valid_link_resets_password(self, django_client, reset_user): + """A valid link with a strong password still resets the password""" + uidb64 = _encode(reset_user.id) + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_app_url(uidb64, token), {"password": STRONG_PASSWORD}) + + assert response.status_code == 302 + assert "sign-in?success=True" in response["Location"] + reset_user.refresh_from_db() + assert reset_user.check_password(STRONG_PASSWORD) + assert reset_user.is_password_autoset is False From d0c09b66c4cb5f499feb4b9af6f8eacb6d737a07 Mon Sep 17 00:00:00 2001 From: TemoSulava Date: Sat, 22 Aug 2026 19:14:57 +0400 Subject: [PATCH 2/5] test(api): cover the app endpoint's token and password branches Review follow-up: TestResetPasswordAppEndpoint only exercised uid handling and the success path, so the invalid-token, missing-password and weak-password branches of ResetPasswordEndpoint were untested. Assert the redirect destination and that a rejected reset leaves is_password_autoset alone, and document the touched functions. --- .../views/app/password_management.py | 5 ++ .../views/space/password_management.py | 5 ++ .../tests/contract/app/test_password_reset.py | 52 ++++++++++++++++++- 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/apps/api/plane/authentication/views/app/password_management.py b/apps/api/plane/authentication/views/app/password_management.py index b61158dcc6e..dda3588631b 100644 --- a/apps/api/plane/authentication/views/app/password_management.py +++ b/apps/api/plane/authentication/views/app/password_management.py @@ -98,6 +98,11 @@ def post(self, request): class ResetPasswordEndpoint(View): def post(self, request, uidb64, token): + """Set a new password for the user encoded in uidb64. + + Always redirects: to sign-in on success, back to the reset-password page with an error code when the + link or the submitted password is rejected. + """ try: # Decode the id from the uidb64 id = smart_str(urlsafe_base64_decode(uidb64)) diff --git a/apps/api/plane/authentication/views/space/password_management.py b/apps/api/plane/authentication/views/space/password_management.py index 47d111323aa..0ce943d79e6 100644 --- a/apps/api/plane/authentication/views/space/password_management.py +++ b/apps/api/plane/authentication/views/space/password_management.py @@ -110,6 +110,11 @@ def post(self, request): class ResetPasswordSpaceEndpoint(View): def post(self, request, uidb64, token): + """Set a new password for the user encoded in uidb64. + + Always redirects: to the space host on success, back to the reset-password page with an error code when + the link or the submitted password is rejected. + """ try: # Decode the id from the uidb64 id = smart_str(urlsafe_base64_decode(uidb64)) diff --git a/apps/api/plane/tests/contract/app/test_password_reset.py b/apps/api/plane/tests/contract/app/test_password_reset.py index cd7db399026..99292558112 100644 --- a/apps/api/plane/tests/contract/app/test_password_reset.py +++ b/apps/api/plane/tests/contract/app/test_password_reset.py @@ -40,16 +40,22 @@ def reset_user(db): def _encode(value): + """Encode a user id the way generate_password_token() does""" return urlsafe_base64_encode(smart_bytes(value)) -# The reset-password patterns share their url names with the forgot-password -# ones, so reverse() resolves to the wrong view - build the paths by hand. def _space_url(uidb64, token): + """Build the space reset-password path. + + Three patterns share name="forgot-password" and two share + name="space-forgot-password", so reverse() picks between them by argument + count alone. Hardcoding the paths pins the URL contract these tests assert. + """ return f"/auth/spaces/reset-password/{uidb64}/{token}/" def _app_url(uidb64, token): + """Build the app reset-password path - see _space_url for why reverse() is not used""" return f"/auth/reset-password/{uidb64}/{token}/" @@ -118,9 +124,11 @@ def test_invalid_token_redirects(self, django_client, reset_user): response = django_client.post(_space_url(uidb64, "invalid-token"), {"password": STRONG_PASSWORD}) assert response.status_code == 302 + assert "accounts/reset-password" in response["Location"] assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] reset_user.refresh_from_db() assert not reset_user.check_password(STRONG_PASSWORD) + assert reset_user.is_password_autoset is True @pytest.mark.django_db def test_weak_password_redirects(self, django_client, reset_user): @@ -134,6 +142,7 @@ def test_weak_password_redirects(self, django_client, reset_user): assert f"error_code={AUTHENTICATION_ERROR_CODES['PASSWORD_TOO_WEAK']}" in response["Location"] reset_user.refresh_from_db() assert not reset_user.check_password("password") + assert reset_user.is_password_autoset is True @pytest.mark.django_db def test_valid_link_resets_password(self, django_client, reset_user): @@ -209,3 +218,42 @@ def test_valid_link_resets_password(self, django_client, reset_user): reset_user.refresh_from_db() assert reset_user.check_password(STRONG_PASSWORD) assert reset_user.is_password_autoset is False + + @pytest.mark.django_db + def test_invalid_token_redirects(self, django_client, reset_user): + """An existing user with a token that does not belong to them is still rejected""" + uidb64 = _encode(reset_user.id) + + response = django_client.post(_app_url(uidb64, "invalid-token"), {"password": STRONG_PASSWORD}) + + assert response.status_code == 302 + assert "accounts/reset-password" in response["Location"] + assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + reset_user.refresh_from_db() + assert not reset_user.check_password(STRONG_PASSWORD) + assert reset_user.is_password_autoset is True + + @pytest.mark.django_db + def test_missing_password_redirects(self, django_client, reset_user): + """A valid link without a password is still rejected with INVALID_PASSWORD""" + uidb64 = _encode(reset_user.id) + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_app_url(uidb64, token), {}) + + assert response.status_code == 302 + assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD']}" in response["Location"] + + @pytest.mark.django_db + def test_weak_password_redirects(self, django_client, reset_user): + """A valid link with a weak password does not change the password""" + uidb64 = _encode(reset_user.id) + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_app_url(uidb64, token), {"password": "password"}) + + assert response.status_code == 302 + assert f"error_code={AUTHENTICATION_ERROR_CODES['PASSWORD_TOO_WEAK']}" in response["Location"] + reset_user.refresh_from_db() + assert not reset_user.check_password("password") + assert reset_user.is_password_autoset is True From 1a2ecbeeef85f3cfc26ebe804c19a2ca3c873fc2 Mon Sep 17 00:00:00 2001 From: TemoSulava Date: Sat, 22 Aug 2026 19:29:48 +0400 Subject: [PATCH 3/5] test(api): assert the exact reset redirect and that rejects preserve the hash Review follow-up: - Substring checks on Location passed even if a branch redirected to the wrong page. Parse the redirect and compare origin, path and query exactly. Only the doubled slash after the space base path is normalized - base_host() already ends in a slash, so that quirk predates this branch; the same defect anywhere else now fails. - `not check_password(candidate)` only proved the submitted password was not stored, not that the existing one survived. Snapshot the hash before the request and assert it is unchanged. - Pin SPACE_BASE_PATH alongside the host so the expected paths do not depend on the ambient environment. - Cover token replay: a spent link must be rejected, since the token is hashed over the stored password. --- .../tests/contract/app/test_password_reset.py | 185 ++++++++++++------ 1 file changed, 126 insertions(+), 59 deletions(-) diff --git a/apps/api/plane/tests/contract/app/test_password_reset.py b/apps/api/plane/tests/contract/app/test_password_reset.py index 99292558112..3af3043ce8d 100644 --- a/apps/api/plane/tests/contract/app/test_password_reset.py +++ b/apps/api/plane/tests/contract/app/test_password_reset.py @@ -3,6 +3,7 @@ # See the LICENSE file for details. import uuid +from urllib.parse import parse_qs, urlparse import pytest from django.contrib.auth.tokens import PasswordResetTokenGenerator @@ -13,7 +14,16 @@ from plane.authentication.adapter.error import AUTHENTICATION_ERROR_CODES from plane.db.models import User +EXPECTED_ORIGIN = ("http", "testserver") + STRONG_PASSWORD = "correct-horse-battery-staple-9x" +WEAK_PASSWORD = "password" + +# Where each endpoint sends a rejected reset, and where it sends a successful one +SPACE_ERROR_PATH = "/spaces/accounts/reset-password" +SPACE_SUCCESS_PATH = "/spaces" +APP_ERROR_PATH = "/accounts/reset-password" +APP_SUCCESS_PATH = "/sign-in" @pytest.fixture(autouse=True) @@ -22,6 +32,7 @@ def _pin_web_url(settings): settings.WEB_URL = "http://testserver" settings.SPACE_BASE_URL = None settings.APP_BASE_URL = None + settings.SPACE_BASE_PATH = "/spaces/" @pytest.fixture @@ -59,6 +70,34 @@ def _app_url(uidb64, token): return f"/auth/reset-password/{uidb64}/{token}/" +def _error_query(error_code_key): + """Return the query string both endpoints attach to a rejected reset""" + return {"error_code": [str(AUTHENTICATION_ERROR_CODES[error_code_key])], "error_message": [error_code_key]} + + +def _assert_redirect(response, expected_path, expected_query): + """Assert the response redirects to expected_path on the pinned origin, carrying exactly expected_query. + + Only the doubled slash after the space base path is normalized: base_host() + already ends in a slash, so the space endpoint emits + "/spaces//accounts/reset-password/". That quirk predates these tests and is + not what they pin - but every other path is compared as emitted, so the same + defect appearing anywhere else does fail. + """ + assert response.status_code == 302 + location = urlparse(response["Location"]) + assert (location.scheme, location.netloc) == EXPECTED_ORIGIN + assert location.path.replace("/spaces//", "/spaces/", 1).rstrip("/") == expected_path + assert parse_qs(location.query, keep_blank_values=True) == expected_query + + +def _assert_credentials_untouched(user, password_hash): + """Assert a rejected reset left the stored hash and the autoset flag alone""" + user.refresh_from_db() + assert user.password == password_hash + assert user.is_password_autoset is True + + @pytest.mark.contract class TestResetPasswordSpaceEndpoint: """The space reset-password endpoint must redirect - never 500 - on a bad uidb64""" @@ -66,83 +105,83 @@ class TestResetPasswordSpaceEndpoint: @pytest.mark.django_db def test_unknown_user_id_redirects(self, django_client, reset_user): """A well formed uidb64 for a user that does not exist redirects with INVALID_PASSWORD_TOKEN""" + password_hash = reset_user.password uidb64 = _encode(uuid.uuid4()) token = PasswordResetTokenGenerator().make_token(reset_user) response = django_client.post(_space_url(uidb64, token), {"password": STRONG_PASSWORD}) - assert response.status_code == 302 - assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + _assert_redirect(response, SPACE_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) + _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_non_uuid_id_redirects(self, django_client, reset_user): """A uidb64 that decodes to something that is not a UUID redirects instead of raising ValidationError""" + password_hash = reset_user.password uidb64 = _encode("not-a-uuid") token = PasswordResetTokenGenerator().make_token(reset_user) response = django_client.post(_space_url(uidb64, token), {"password": STRONG_PASSWORD}) - assert response.status_code == 302 - assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + _assert_redirect(response, SPACE_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) + _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_malformed_base64_redirects(self, django_client, reset_user): """A uidb64 that is not decodable base64 redirects instead of raising ValueError""" + password_hash = reset_user.password token = PasswordResetTokenGenerator().make_token(reset_user) response = django_client.post(_space_url("a", token), {"password": STRONG_PASSWORD}) - assert response.status_code == 302 - assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + _assert_redirect(response, SPACE_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) + _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_undecodable_uidb64_redirects(self, django_client, reset_user): """A uidb64 that decodes to invalid utf-8 keeps the EXPIRED_PASSWORD_TOKEN response""" + password_hash = reset_user.password token = PasswordResetTokenGenerator().make_token(reset_user) response = django_client.post(_space_url("not", token), {"password": STRONG_PASSWORD}) - assert response.status_code == 302 - assert f"error_code={AUTHENTICATION_ERROR_CODES['EXPIRED_PASSWORD_TOKEN']}" in response["Location"] + _assert_redirect(response, SPACE_ERROR_PATH, _error_query("EXPIRED_PASSWORD_TOKEN")) + _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_missing_password_redirects(self, django_client, reset_user): """A valid link without a password is still rejected with INVALID_PASSWORD""" + password_hash = reset_user.password uidb64 = _encode(reset_user.id) token = PasswordResetTokenGenerator().make_token(reset_user) response = django_client.post(_space_url(uidb64, token), {}) - assert response.status_code == 302 - assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD']}" in response["Location"] + _assert_redirect(response, SPACE_ERROR_PATH, _error_query("INVALID_PASSWORD")) + _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_invalid_token_redirects(self, django_client, reset_user): """An existing user with a token that does not belong to them is still rejected""" + password_hash = reset_user.password uidb64 = _encode(reset_user.id) response = django_client.post(_space_url(uidb64, "invalid-token"), {"password": STRONG_PASSWORD}) - assert response.status_code == 302 - assert "accounts/reset-password" in response["Location"] - assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] - reset_user.refresh_from_db() - assert not reset_user.check_password(STRONG_PASSWORD) - assert reset_user.is_password_autoset is True + _assert_redirect(response, SPACE_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) + _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_weak_password_redirects(self, django_client, reset_user): """A valid link with a weak password does not change the password""" + password_hash = reset_user.password uidb64 = _encode(reset_user.id) token = PasswordResetTokenGenerator().make_token(reset_user) - response = django_client.post(_space_url(uidb64, token), {"password": "password"}) + response = django_client.post(_space_url(uidb64, token), {"password": WEAK_PASSWORD}) - assert response.status_code == 302 - assert f"error_code={AUTHENTICATION_ERROR_CODES['PASSWORD_TOO_WEAK']}" in response["Location"] - reset_user.refresh_from_db() - assert not reset_user.check_password("password") - assert reset_user.is_password_autoset is True + _assert_redirect(response, SPACE_ERROR_PATH, _error_query("PASSWORD_TOO_WEAK")) + _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_valid_link_resets_password(self, django_client, reset_user): @@ -152,12 +191,26 @@ def test_valid_link_resets_password(self, django_client, reset_user): response = django_client.post(_space_url(uidb64, token), {"password": STRONG_PASSWORD}) - assert response.status_code == 302 - assert "error_code" not in response["Location"] + _assert_redirect(response, SPACE_SUCCESS_PATH, {}) reset_user.refresh_from_db() assert reset_user.check_password(STRONG_PASSWORD) assert reset_user.is_password_autoset is False + @pytest.mark.django_db + def test_token_cannot_be_replayed(self, django_client, reset_user): + """A token stops working once it has been spent - it is hashed over the stored password""" + uidb64 = _encode(reset_user.id) + token = PasswordResetTokenGenerator().make_token(reset_user) + django_client.post(_space_url(uidb64, token), {"password": STRONG_PASSWORD}) + reset_user.refresh_from_db() + password_hash = reset_user.password + + response = django_client.post(_space_url(uidb64, token), {"password": "another-correct-horse-99x"}) + + _assert_redirect(response, SPACE_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) + reset_user.refresh_from_db() + assert reset_user.password == password_hash + @pytest.mark.contract class TestResetPasswordAppEndpoint: @@ -166,94 +219,108 @@ class TestResetPasswordAppEndpoint: @pytest.mark.django_db def test_non_uuid_id_redirects(self, django_client, reset_user): """A uidb64 that decodes to something that is not a UUID redirects instead of raising ValidationError""" + password_hash = reset_user.password uidb64 = _encode("not-a-uuid") token = PasswordResetTokenGenerator().make_token(reset_user) response = django_client.post(_app_url(uidb64, token), {"password": STRONG_PASSWORD}) - assert response.status_code == 302 - assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + _assert_redirect(response, APP_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) + _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_unknown_user_id_redirects(self, django_client, reset_user): """A well formed uidb64 for a user that does not exist redirects with INVALID_PASSWORD_TOKEN""" + password_hash = reset_user.password uidb64 = _encode(uuid.uuid4()) token = PasswordResetTokenGenerator().make_token(reset_user) response = django_client.post(_app_url(uidb64, token), {"password": STRONG_PASSWORD}) - assert response.status_code == 302 - assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + _assert_redirect(response, APP_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) + _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_malformed_base64_redirects(self, django_client, reset_user): """A uidb64 that is not decodable base64 redirects instead of raising ValueError""" + password_hash = reset_user.password token = PasswordResetTokenGenerator().make_token(reset_user) response = django_client.post(_app_url("a", token), {"password": STRONG_PASSWORD}) - assert response.status_code == 302 - assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] + _assert_redirect(response, APP_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) + _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_undecodable_uidb64_redirects(self, django_client, reset_user): """A uidb64 that decodes to invalid utf-8 keeps the EXPIRED_PASSWORD_TOKEN response""" + password_hash = reset_user.password token = PasswordResetTokenGenerator().make_token(reset_user) response = django_client.post(_app_url("not", token), {"password": STRONG_PASSWORD}) - assert response.status_code == 302 - assert f"error_code={AUTHENTICATION_ERROR_CODES['EXPIRED_PASSWORD_TOKEN']}" in response["Location"] - - @pytest.mark.django_db - def test_valid_link_resets_password(self, django_client, reset_user): - """A valid link with a strong password still resets the password""" - uidb64 = _encode(reset_user.id) - token = PasswordResetTokenGenerator().make_token(reset_user) - - response = django_client.post(_app_url(uidb64, token), {"password": STRONG_PASSWORD}) - - assert response.status_code == 302 - assert "sign-in?success=True" in response["Location"] - reset_user.refresh_from_db() - assert reset_user.check_password(STRONG_PASSWORD) - assert reset_user.is_password_autoset is False + _assert_redirect(response, APP_ERROR_PATH, _error_query("EXPIRED_PASSWORD_TOKEN")) + _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_invalid_token_redirects(self, django_client, reset_user): """An existing user with a token that does not belong to them is still rejected""" + password_hash = reset_user.password uidb64 = _encode(reset_user.id) response = django_client.post(_app_url(uidb64, "invalid-token"), {"password": STRONG_PASSWORD}) - assert response.status_code == 302 - assert "accounts/reset-password" in response["Location"] - assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD_TOKEN']}" in response["Location"] - reset_user.refresh_from_db() - assert not reset_user.check_password(STRONG_PASSWORD) - assert reset_user.is_password_autoset is True + _assert_redirect(response, APP_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) + _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_missing_password_redirects(self, django_client, reset_user): """A valid link without a password is still rejected with INVALID_PASSWORD""" + password_hash = reset_user.password uidb64 = _encode(reset_user.id) token = PasswordResetTokenGenerator().make_token(reset_user) response = django_client.post(_app_url(uidb64, token), {}) - assert response.status_code == 302 - assert f"error_code={AUTHENTICATION_ERROR_CODES['INVALID_PASSWORD']}" in response["Location"] + _assert_redirect(response, APP_ERROR_PATH, _error_query("INVALID_PASSWORD")) + _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_weak_password_redirects(self, django_client, reset_user): """A valid link with a weak password does not change the password""" + password_hash = reset_user.password + uidb64 = _encode(reset_user.id) + token = PasswordResetTokenGenerator().make_token(reset_user) + + response = django_client.post(_app_url(uidb64, token), {"password": WEAK_PASSWORD}) + + _assert_redirect(response, APP_ERROR_PATH, _error_query("PASSWORD_TOO_WEAK")) + _assert_credentials_untouched(reset_user, password_hash) + + @pytest.mark.django_db + def test_valid_link_resets_password(self, django_client, reset_user): + """A valid link with a strong password still resets the password""" uidb64 = _encode(reset_user.id) token = PasswordResetTokenGenerator().make_token(reset_user) - response = django_client.post(_app_url(uidb64, token), {"password": "password"}) + response = django_client.post(_app_url(uidb64, token), {"password": STRONG_PASSWORD}) + + _assert_redirect(response, APP_SUCCESS_PATH, {"success": ["True"]}) + reset_user.refresh_from_db() + assert reset_user.check_password(STRONG_PASSWORD) + assert reset_user.is_password_autoset is False + + @pytest.mark.django_db + def test_token_cannot_be_replayed(self, django_client, reset_user): + """A token stops working once it has been spent - it is hashed over the stored password""" + uidb64 = _encode(reset_user.id) + token = PasswordResetTokenGenerator().make_token(reset_user) + django_client.post(_app_url(uidb64, token), {"password": STRONG_PASSWORD}) + reset_user.refresh_from_db() + password_hash = reset_user.password + + response = django_client.post(_app_url(uidb64, token), {"password": "another-correct-horse-99x"}) - assert response.status_code == 302 - assert f"error_code={AUTHENTICATION_ERROR_CODES['PASSWORD_TOO_WEAK']}" in response["Location"] + _assert_redirect(response, APP_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) reset_user.refresh_from_db() - assert not reset_user.check_password("password") - assert reset_user.is_password_autoset is True + assert reset_user.password == password_hash From cd35735829b218cea75940787373a55b07302eb3 Mon Sep 17 00:00:00 2001 From: TemoSulava Date: Sat, 22 Aug 2026 19:48:22 +0400 Subject: [PATCH 4/5] test(api): compare reset redirect paths without stripping trailing slashes rstrip("/") erased the one difference between the two endpoints: the space endpoint builds its redirects with f-strings and emits a trailing slash, the app endpoint uses urljoin() and does not. Stripping let either one drift onto the other's form unnoticed. Compare the path as emitted and carry the trailing slash in the space constants; the targeted "/spaces//" normalization stays. --- .../plane/tests/contract/app/test_password_reset.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/apps/api/plane/tests/contract/app/test_password_reset.py b/apps/api/plane/tests/contract/app/test_password_reset.py index 3af3043ce8d..a1a5ed0eb6c 100644 --- a/apps/api/plane/tests/contract/app/test_password_reset.py +++ b/apps/api/plane/tests/contract/app/test_password_reset.py @@ -19,9 +19,11 @@ STRONG_PASSWORD = "correct-horse-battery-staple-9x" WEAK_PASSWORD = "password" -# Where each endpoint sends a rejected reset, and where it sends a successful one -SPACE_ERROR_PATH = "/spaces/accounts/reset-password" -SPACE_SUCCESS_PATH = "/spaces" +# Where each endpoint sends a rejected reset, and where it sends a successful one. +# The trailing slashes differ because the space endpoint builds its redirects +# with f-strings and the app endpoint with urljoin() - compared as emitted. +SPACE_ERROR_PATH = "/spaces/accounts/reset-password/" +SPACE_SUCCESS_PATH = "/spaces/" APP_ERROR_PATH = "/accounts/reset-password" APP_SUCCESS_PATH = "/sign-in" @@ -87,7 +89,7 @@ def _assert_redirect(response, expected_path, expected_query): assert response.status_code == 302 location = urlparse(response["Location"]) assert (location.scheme, location.netloc) == EXPECTED_ORIGIN - assert location.path.replace("/spaces//", "/spaces/", 1).rstrip("/") == expected_path + assert location.path.replace("/spaces//", "/spaces/", 1) == expected_path assert parse_qs(location.query, keep_blank_values=True) == expected_query From 67d08b9554a37a6d76b00f092eac263808d0e630 Mon Sep 17 00:00:00 2001 From: TemoSulava Date: Mon, 24 Aug 2026 00:05:15 +0400 Subject: [PATCH 5/5] fix(api): answer an undecodable reset uidb64 with INVALID_PASSWORD_TOKEN DjangoUnicodeDecodeError subclasses ValueError, so the tuple clause around the decode and the lookup already covers a uidb64 whose bytes are not utf-8. Drop the dedicated handler from both endpoints rather than ordering it first: such a link was never valid, so 5125 INVALID_PASSWORD_TOKEN describes it more accurately than 5130 EXPIRED_PASSWORD_TOKEN, and it is the code the app endpoint already returned on preview, where the same handler was unreachable. Only the space endpoint changes against preview (5130 -> 5125). 5130 is no longer emitted by the API; the frontend enum entries are left in place. Name the two rejected-uidb64 fixtures after the branch each one reaches and pin those raise sites, since both now produce the same response, and pin the error-code literals that packages/constants/src/auth/index.ts hardcodes. --- .../views/app/password_management.py | 15 ++---- .../views/space/password_management.py | 12 ++--- .../tests/contract/app/test_password_reset.py | 50 +++++++++++++++---- 3 files changed, 46 insertions(+), 31 deletions(-) diff --git a/apps/api/plane/authentication/views/app/password_management.py b/apps/api/plane/authentication/views/app/password_management.py index dda3588631b..10f718079c9 100644 --- a/apps/api/plane/authentication/views/app/password_management.py +++ b/apps/api/plane/authentication/views/app/password_management.py @@ -18,7 +18,7 @@ from django.core.exceptions import ValidationError from django.core.validators import validate_email from django.http import HttpResponseRedirect -from django.utils.encoding import DjangoUnicodeDecodeError, smart_bytes, smart_str +from django.utils.encoding import smart_bytes, smart_str from django.utils.http import urlsafe_base64_decode, urlsafe_base64_encode from django.views import View @@ -107,18 +107,9 @@ def post(self, request, uidb64, token): # Decode the id from the uidb64 id = smart_str(urlsafe_base64_decode(uidb64)) user = User.objects.get(id=id) - except DjangoUnicodeDecodeError: - exc = AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES["EXPIRED_PASSWORD_TOKEN"], - error_message="EXPIRED_PASSWORD_TOKEN", - ) - url = urljoin( - base_host(request=request, is_app=True), - "accounts/reset-password?" + urlencode(exc.get_error_dict()), - ) - return HttpResponseRedirect(url) except (ValueError, ValidationError, User.DoesNotExist): - # Malformed base64, a non-UUID id or an id that matches no user + # Malformed base64, bytes that are not utf-8 (DjangoUnicodeDecodeError subclasses ValueError), + # a non-UUID id or an id that matches no user exc = AuthenticationException( error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD_TOKEN"], error_message="INVALID_PASSWORD_TOKEN", diff --git a/apps/api/plane/authentication/views/space/password_management.py b/apps/api/plane/authentication/views/space/password_management.py index 0ce943d79e6..b1059b811a7 100644 --- a/apps/api/plane/authentication/views/space/password_management.py +++ b/apps/api/plane/authentication/views/space/password_management.py @@ -18,7 +18,7 @@ from django.core.exceptions import ValidationError from django.core.validators import validate_email from django.http import HttpResponseRedirect -from django.utils.encoding import DjangoUnicodeDecodeError, smart_bytes, smart_str +from django.utils.encoding import smart_bytes, smart_str from django.utils.http import urlsafe_base64_decode, urlsafe_base64_encode from django.views import View @@ -119,15 +119,9 @@ def post(self, request, uidb64, token): # Decode the id from the uidb64 id = smart_str(urlsafe_base64_decode(uidb64)) user = User.objects.get(id=id) - except DjangoUnicodeDecodeError: - exc = AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES["EXPIRED_PASSWORD_TOKEN"], - error_message="EXPIRED_PASSWORD_TOKEN", - ) - url = f"{base_host(request=request, is_space=True)}/accounts/reset-password/?{urlencode(exc.get_error_dict())}" # noqa: E501 - return HttpResponseRedirect(url) except (ValueError, ValidationError, User.DoesNotExist): - # Malformed base64, a non-UUID id or an id that matches no user + # Malformed base64, bytes that are not utf-8 (DjangoUnicodeDecodeError subclasses ValueError), + # a non-UUID id or an id that matches no user exc = AuthenticationException( error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD_TOKEN"], error_message="INVALID_PASSWORD_TOKEN", diff --git a/apps/api/plane/tests/contract/app/test_password_reset.py b/apps/api/plane/tests/contract/app/test_password_reset.py index a1a5ed0eb6c..b10450f7b62 100644 --- a/apps/api/plane/tests/contract/app/test_password_reset.py +++ b/apps/api/plane/tests/contract/app/test_password_reset.py @@ -8,8 +8,8 @@ import pytest from django.contrib.auth.tokens import PasswordResetTokenGenerator from django.test import Client -from django.utils.encoding import smart_bytes -from django.utils.http import urlsafe_base64_encode +from django.utils.encoding import DjangoUnicodeDecodeError, smart_bytes, smart_str +from django.utils.http import urlsafe_base64_decode, urlsafe_base64_encode from plane.authentication.adapter.error import AUTHENTICATION_ERROR_CODES from plane.db.models import User @@ -19,6 +19,11 @@ STRONG_PASSWORD = "correct-horse-battery-staple-9x" WEAK_PASSWORD = "password" +# Two ways a uidb64 fails before the user lookup - same except clause, different raise +# sites, pinned by test_uidb64_fixtures_reach_their_branches +MALFORMED_UIDB64 = "a" # not valid base64: urlsafe_base64_decode raises binascii.Error +UNDECODABLE_UIDB64 = "not" # decodes to b"ž‹": smart_str raises DjangoUnicodeDecodeError + # Where each endpoint sends a rejected reset, and where it sends a successful one. # The trailing slashes differ because the space endpoint builds its redirects # with f-strings and the app endpoint with urljoin() - compared as emitted. @@ -100,6 +105,31 @@ def _assert_credentials_untouched(user, password_hash): assert user.is_password_autoset is True +@pytest.mark.contract +def test_error_code_wire_values(): + """Pin the numbers clients hardcode - packages/constants/src/auth/index.ts carries the same literals""" + assert AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD_TOKEN"] == 5125 + assert AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD"] == 5020 + assert AUTHENTICATION_ERROR_CODES["PASSWORD_TOO_WEAK"] == 5021 + + +@pytest.mark.contract +def test_uidb64_fixtures_reach_their_branches(): + """Pin that the two rejected-uidb64 fixtures fail where their names say they do. + + Both endpoints answer either one with INVALID_PASSWORD_TOKEN, so the response + alone no longer tells them apart: without this, a change that stopped + UNDECODABLE_UIDB64 from raising at all would leave the utf-8 branch untested + and every test still green. + """ + with pytest.raises(ValueError) as malformed: + urlsafe_base64_decode(MALFORMED_UIDB64) + assert not isinstance(malformed.value, DjangoUnicodeDecodeError) + + with pytest.raises(DjangoUnicodeDecodeError): + smart_str(urlsafe_base64_decode(UNDECODABLE_UIDB64)) + + @pytest.mark.contract class TestResetPasswordSpaceEndpoint: """The space reset-password endpoint must redirect - never 500 - on a bad uidb64""" @@ -134,20 +164,20 @@ def test_malformed_base64_redirects(self, django_client, reset_user): password_hash = reset_user.password token = PasswordResetTokenGenerator().make_token(reset_user) - response = django_client.post(_space_url("a", token), {"password": STRONG_PASSWORD}) + response = django_client.post(_space_url(MALFORMED_UIDB64, token), {"password": STRONG_PASSWORD}) _assert_redirect(response, SPACE_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_undecodable_uidb64_redirects(self, django_client, reset_user): - """A uidb64 that decodes to invalid utf-8 keeps the EXPIRED_PASSWORD_TOKEN response""" + """A uidb64 that decodes to invalid utf-8 is rejected as an invalid link""" password_hash = reset_user.password token = PasswordResetTokenGenerator().make_token(reset_user) - response = django_client.post(_space_url("not", token), {"password": STRONG_PASSWORD}) + response = django_client.post(_space_url(UNDECODABLE_UIDB64, token), {"password": STRONG_PASSWORD}) - _assert_redirect(response, SPACE_ERROR_PATH, _error_query("EXPIRED_PASSWORD_TOKEN")) + _assert_redirect(response, SPACE_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db @@ -248,20 +278,20 @@ def test_malformed_base64_redirects(self, django_client, reset_user): password_hash = reset_user.password token = PasswordResetTokenGenerator().make_token(reset_user) - response = django_client.post(_app_url("a", token), {"password": STRONG_PASSWORD}) + response = django_client.post(_app_url(MALFORMED_UIDB64, token), {"password": STRONG_PASSWORD}) _assert_redirect(response, APP_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db def test_undecodable_uidb64_redirects(self, django_client, reset_user): - """A uidb64 that decodes to invalid utf-8 keeps the EXPIRED_PASSWORD_TOKEN response""" + """A uidb64 that decodes to invalid utf-8 is rejected as an invalid link""" password_hash = reset_user.password token = PasswordResetTokenGenerator().make_token(reset_user) - response = django_client.post(_app_url("not", token), {"password": STRONG_PASSWORD}) + response = django_client.post(_app_url(UNDECODABLE_UIDB64, token), {"password": STRONG_PASSWORD}) - _assert_redirect(response, APP_ERROR_PATH, _error_query("EXPIRED_PASSWORD_TOKEN")) + _assert_redirect(response, APP_ERROR_PATH, _error_query("INVALID_PASSWORD_TOKEN")) _assert_credentials_untouched(reset_user, password_hash) @pytest.mark.django_db