Merge pull request #2688 from GSA/2032-bug-redirect-after-user-register-is-going-to-an-error-page

redirecting users to 404
This commit is contained in:
Beverly Nguyen
2025-06-23 17:54:35 -07:00
committed by GitHub
4 changed files with 19 additions and 54 deletions

View File

@@ -527,7 +527,7 @@
"filename": "tests/app/main/views/test_register.py", "filename": "tests/app/main/views/test_register.py",
"hashed_secret": "bdbb156d25d02fd7792865824201dda1c60f4473", "hashed_secret": "bdbb156d25d02fd7792865824201dda1c60f4473",
"is_verified": false, "is_verified": false,
"line_number": 122, "line_number": 114,
"is_secret": false "is_secret": false
}, },
{ {
@@ -535,7 +535,7 @@
"filename": "tests/app/main/views/test_register.py", "filename": "tests/app/main/views/test_register.py",
"hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8", "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8",
"is_verified": false, "is_verified": false,
"line_number": 199, "line_number": 184,
"is_secret": false "is_secret": false
}, },
{ {
@@ -543,7 +543,7 @@
"filename": "tests/app/main/views/test_register.py", "filename": "tests/app/main/views/test_register.py",
"hashed_secret": "bb5b7caa27d005d38039e3797c3ddb9bcd22c3c8", "hashed_secret": "bb5b7caa27d005d38039e3797c3ddb9bcd22c3c8",
"is_verified": false, "is_verified": false,
"line_number": 272, "line_number": 255,
"is_secret": false "is_secret": false
} }
], ],
@@ -634,5 +634,5 @@
} }
] ]
}, },
"generated_at": "2025-06-10T18:39:51Z" "generated_at": "2025-06-18T17:05:12Z"
} }

View File

@@ -13,12 +13,10 @@ from flask import (
session, session,
url_for, url_for,
) )
from flask_login import current_user
from app import redis_client, user_api_client from app import redis_client, user_api_client
from app.main import main from app.main import main
from app.main.forms import ( from app.main.forms import (
RegisterUserForm,
RegisterUserFromOrgInviteForm, RegisterUserFromOrgInviteForm,
SetupUserProfileForm, SetupUserProfileForm,
) )
@@ -32,15 +30,7 @@ from app.utils.user import is_gov_user
@main.route("/register", methods=["GET", "POST"]) @main.route("/register", methods=["GET", "POST"])
@hide_from_search_engines @hide_from_search_engines
def register(): def register():
if current_user and current_user.is_authenticated: abort(404)
return redirect(url_for("main.show_accounts_or_dashboard"))
form = RegisterUserForm()
if form.validate_on_submit():
_do_registration(form, send_sms=False)
return redirect(url_for("main.registration_continue"))
return render_template("views/register.html", form=form)
@main.route("/register-from-org-invite", methods=["GET", "POST"]) @main.route("/register-from-org-invite", methods=["GET", "POST"])
@@ -108,10 +98,7 @@ def _do_registration(form, send_sms=True, send_email=True, organization_id=None)
@main.route("/registration-continue") @main.route("/registration-continue")
def registration_continue(): def registration_continue():
if not session.get("user_details"): abort(404)
return redirect(url_for(".show_accounts_or_dashboard"))
else:
raise Exception("Unexpected routing in registration_continue")
def get_invite_data_from_redis(state): def get_invite_data_from_redis(state):

View File

@@ -65,7 +65,6 @@ def test_robots(client_request):
("endpoint", "kwargs"), ("endpoint", "kwargs"),
[ [
("sign_in", {}), ("sign_in", {}),
("register", {}),
pytest.param("index", {}, marks=pytest.mark.xfail(raises=AssertionError)), pytest.param("index", {}, marks=pytest.mark.xfail(raises=AssertionError)),
], ],
) )

View File

@@ -7,20 +7,13 @@ from flask import url_for
from app.main.views.register import check_invited_user_email_address_matches_expected from app.main.views.register import check_invited_user_email_address_matches_expected
from app.models.user import User from app.models.user import User
from tests.conftest import normalize_spaces
def test_render_register_returns_template_with_form(client_request, mocker): def test_render_register_returns_template_with_form(client_request, mocker):
mocker.patch("app.notify_client.user_api_client.UserApiClient.deactivate_user") mocker.patch("app.notify_client.user_api_client.UserApiClient.deactivate_user")
client_request.logout() client_request.logout()
page = client_request.get_url("/register") client_request.get_url("/register", _expected_status=404)
assert page.find("input", attrs={"name": "auth_type"}).attrs["value"] == "sms_auth"
assert page.select_one("#email_address")["spellcheck"] == "false"
assert page.select_one("#email_address")["autocomplete"] == "email"
assert page.select_one("#password")["autocomplete"] == "new-password"
assert "Create an account" in page.text
def test_logged_in_user_redirects_to_account( def test_logged_in_user_redirects_to_account(
@@ -28,8 +21,7 @@ def test_logged_in_user_redirects_to_account(
): ):
client_request.get( client_request.get(
"main.register", "main.register",
_expected_status=302, _expected_status=404,
_expected_redirect=url_for("main.show_accounts_or_dashboard"),
) )
@@ -99,7 +91,7 @@ def test_register_continue_handles_missing_session_sensibly(client_request, mock
# session is not set # session is not set
client_request.get( client_request.get(
"main.registration_continue", "main.registration_continue",
_expected_redirect=url_for("main.show_accounts_or_dashboard"), _expected_status=404,
) )
@@ -113,7 +105,7 @@ def test_process_register_returns_200_when_mobile_number_is_invalid(
mocker.patch("app.notify_client.user_api_client.UserApiClient.deactivate_user") mocker.patch("app.notify_client.user_api_client.UserApiClient.deactivate_user")
client_request.logout() client_request.logout()
page = client_request.post( client_request.post(
"main.register", "main.register",
_data={ _data={
"name": "Bad Mobile", "name": "Bad Mobile",
@@ -121,11 +113,9 @@ def test_process_register_returns_200_when_mobile_number_is_invalid(
"mobile_number": "not good", "mobile_number": "not good",
"password": "validPassword!", # noqa "password": "validPassword!", # noqa
}, },
_expected_status=200, _expected_status=404,
) )
assert "The string supplied did not seem to be a phone number" in page.text
def test_should_return_200_when_email_is_not_gov_uk( def test_should_return_200_when_email_is_not_gov_uk(
client_request, mock_get_organizations, mocker client_request, mock_get_organizations, mocker
@@ -133,7 +123,7 @@ def test_should_return_200_when_email_is_not_gov_uk(
mocker.patch("app.notify_client.user_api_client.UserApiClient.deactivate_user") mocker.patch("app.notify_client.user_api_client.UserApiClient.deactivate_user")
client_request.logout() client_request.logout()
page = client_request.post( client_request.post(
"main.register", "main.register",
_data={ _data={
"name": "Firstname Lastname", "name": "Firstname Lastname",
@@ -141,11 +131,7 @@ def test_should_return_200_when_email_is_not_gov_uk(
"mobile_number": "2020900123", "mobile_number": "2020900123",
"password": "validPassword!", "password": "validPassword!",
}, },
_expected_status=200, _expected_status=404,
)
assert "Enter a public sector email address." in normalize_spaces(
page.select_one(".usa-error-message").text
) )
@@ -179,9 +165,8 @@ def test_should_add_user_details_to_session(
"mobile_number": "+12023123123", "mobile_number": "+12023123123",
"password": "validPassword!", "password": "validPassword!",
}, },
_expected_status=404,
) )
with client_request.session_transaction() as session:
assert session["user_details"]["email"] == email_address
def test_should_return_200_if_password_is_on_list_of_commonly_used_passwords( def test_should_return_200_if_password_is_on_list_of_commonly_used_passwords(
@@ -190,7 +175,7 @@ def test_should_return_200_if_password_is_on_list_of_commonly_used_passwords(
mocker.patch("app.notify_client.user_api_client.UserApiClient.deactivate_user") mocker.patch("app.notify_client.user_api_client.UserApiClient.deactivate_user")
client_request.logout() client_request.logout()
page = client_request.post( client_request.post(
"main.register", "main.register",
_data={ _data={
"name": "Bad Mobile", "name": "Bad Mobile",
@@ -198,11 +183,9 @@ def test_should_return_200_if_password_is_on_list_of_commonly_used_passwords(
"mobile_number": "+12021234123", "mobile_number": "+12021234123",
"password": "password", # noqa "password": "password", # noqa
}, },
_expected_status=200, _expected_status=404,
) )
assert "Choose a password thats harder to guess" in page.text
def test_register_with_existing_email_sends_emails( def test_register_with_existing_email_sends_emails(
client_request, client_request,
@@ -224,7 +207,7 @@ def test_register_with_existing_email_sends_emails(
client_request.post( client_request.post(
"main.register", "main.register",
_data=user_data, _data=user_data,
_expected_redirect=url_for("main.registration_continue"), _expected_status=404,
) )
@@ -383,20 +366,16 @@ def test_cannot_register_with_sms_auth_and_missing_mobile_number(
mocker.patch("app.notify_client.user_api_client.UserApiClient.deactivate_user") mocker.patch("app.notify_client.user_api_client.UserApiClient.deactivate_user")
client_request.logout() client_request.logout()
page = client_request.post( client_request.post(
"main.register", "main.register",
_data={ _data={
"name": "Missing Mobile", "name": "Missing Mobile",
"email_address": "missing_mobile@example.gsa.gov", "email_address": "missing_mobile@example.gsa.gov",
"password": "validPassword!", "password": "validPassword!",
}, },
_expected_status=200, _expected_status=404,
) )
err = page.select_one(".usa-error-message")
assert err.text.strip() == "Error: Cannot be empty"
assert err.attrs["data-error-label"] == "mobile_number"
def test_check_invited_user_email_address_matches_expected(mocker): def test_check_invited_user_email_address_matches_expected(mocker):
mock_flash = mocker.patch("app.main.views.register.flash") mock_flash = mocker.patch("app.main.views.register.flash")