From 9ecbfb7e05ce21f26c89056ff7138c6e16c2a7f5 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 6 May 2024 10:43:01 -0700 Subject: [PATCH 01/12] notify-admin-1459 --- app/main/views/sign_in.py | 117 ++++++++++------------ poetry.lock | 2 + tests/app/main/views/test_sign_in.py | 144 +-------------------------- 3 files changed, 59 insertions(+), 204 deletions(-) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index 532bd9500..7af9ce8ca 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -4,27 +4,15 @@ import uuid import jwt import requests -from flask import ( - Response, - abort, - current_app, - flash, - redirect, - render_template, - request, - session, - url_for, -) +from flask import Response, current_app, redirect, render_template, request, url_for from flask_login import current_user -from markupsafe import Markup from notifications_utils.url_safe_token import generate_token from app import login_manager, user_api_client from app.main import main -from app.main.forms import LoginForm from app.main.views.index import error from app.main.views.verify import activate_user -from app.models.user import InvitedUser, User +from app.models.user import User from app.utils import hide_from_search_engines from app.utils.login import is_safe_redirect_url from app.utils.time import is_less_than_days_ago @@ -129,6 +117,16 @@ def verify_email(user, redirect_url): ) +def _handle_e2e_tests(redirect_url): + current_app.logger.warning("E2E TESTS ARE ENABLED.") + current_app.logger.warning( + "If you are getting a 404 on signin, comment out E2E vars in .env file!" + ) + user = user_api_client.get_user_by_email(os.getenv("NOTIFY_E2E_TEST_EMAIL")) + activate_user(user["id"]) + return redirect(url_for("main.show_accounts_or_dashboard", next=redirect_url)) + + @main.route("/sign-in", methods=(["GET", "POST"])) @hide_from_search_engines def sign_in(): @@ -146,67 +144,60 @@ def sign_in(): redirect_url = request.args.get("next") if os.getenv("NOTIFY_E2E_TEST_EMAIL"): - current_app.logger.warning("E2E TESTS ARE ENABLED.") - current_app.logger.warning( - "If you are getting a 404 on signin, comment out E2E vars in .env file!" - ) - user = user_api_client.get_user_by_email(os.getenv("NOTIFY_E2E_TEST_EMAIL")) - activate_user(user["id"]) - return redirect(url_for("main.show_accounts_or_dashboard", next=redirect_url)) + return _handle_e2e_tests(redirect_url) - current_app.logger.info(f"current user is {current_user}") if current_user and current_user.is_authenticated: if redirect_url and is_safe_redirect_url(redirect_url): return redirect(redirect_url) return redirect(url_for("main.show_accounts_or_dashboard")) - form = LoginForm() - current_app.logger.info("Got the login form") - password_reset_url = url_for(".forgot_password", next=request.args.get("next")) + # form = LoginForm() + # current_app.logger.info("Got the login form") + # password_reset_url = url_for(".forgot_password", next=request.args.get("next")) - if form.validate_on_submit(): - user = User.from_email_address_and_password_or_none( - form.email_address.data, form.password.data - ) + # if form.validate_on_submit(): + # user = User.from_email_address_and_password_or_none( + # form.email_address.data, form.password.data + # ) - if user: - # add user to session to mark us as in the process of signing the user in - session["user_details"] = {"email": user.email_address, "id": user.id} + # if user: + # # add user to session to mark us as in the process of signing the user in + # session["user_details"] = {"email": user.email_address, "id": user.id} - if user.state == "pending": - return redirect( - url_for("main.resend_email_verification", next=redirect_url) - ) + # if user.state == "pending": + # return redirect( + # url_for("main.resend_email_verification", next=redirect_url) + # ) - if user.is_active: - if session.get("invited_user_id"): - invited_user = InvitedUser.from_session() - if user.email_address.lower() != invited_user.email_address.lower(): - flash("You cannot accept an invite for another person.") - session.pop("invited_user_id", None) - abort(403) - else: - invited_user.accept_invite() + # if user.is_active: + # if session.get("invited_user_id"): + # invited_user = InvitedUser.from_session() + # if user.email_address.lower() != invited_user.email_address.lower(): + # flash("You cannot accept an invite for another person.") + # session.pop("invited_user_id", None) + # abort(403) + # else: + # invited_user.accept_invite() - user.send_login_code() + # user.send_login_code() - if user.sms_auth: - return redirect(url_for(".two_factor_sms", next=redirect_url)) + # if user.sms_auth: + # return redirect(url_for(".two_factor_sms", next=redirect_url)) - if user.email_auth: - return redirect( - url_for(".two_factor_email_sent", next=redirect_url) - ) + # if user.email_auth: + # return redirect( + # url_for(".two_factor_email_sent", next=redirect_url) + # ) - # Vague error message for login in case of user not known, locked, inactive or password not verified - flash( - Markup( - ( - f"The email address or password you entered is incorrect." - f" Forgot your password?" - ) - ) - ) + # # Vague error message for login in case of user not known, locked, inactive or password not verified + # flash( + # Markup( + # ( + # f"The email address or password you entered is incorrect." + # f" Forgot your password?" + # ) + # ) + # ) other_device = current_user.logged_in_elsewhere() @@ -222,10 +213,10 @@ def sign_in(): url = url.replace("STATE", token) return render_template( "views/signin.html", - form=form, + # form=form, again=bool(redirect_url), other_device=other_device, - password_reset_url=password_reset_url, + # password_reset_url=password_reset_url, initial_signin_url=url, ) diff --git a/poetry.lock b/poetry.lock index 69007e3b5..d9cef6ee5 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1278,6 +1278,7 @@ files = [ {file = "lxml-5.2.1-cp36-cp36m-manylinux_2_12_i686.manylinux2010_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:c38d7b9a690b090de999835f0443d8aa93ce5f2064035dfc48f27f02b4afc3d0"}, {file = "lxml-5.2.1-cp36-cp36m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:5670fb70a828663cc37552a2a85bf2ac38475572b0e9b91283dc09efb52c41d1"}, {file = "lxml-5.2.1-cp36-cp36m-manylinux_2_28_x86_64.whl", hash = "sha256:958244ad566c3ffc385f47dddde4145088a0ab893504b54b52c041987a8c1863"}, + {file = "lxml-5.2.1-cp36-cp36m-manylinux_2_5_x86_64.manylinux1_x86_64.whl", hash = "sha256:b6241d4eee5f89453307c2f2bfa03b50362052ca0af1efecf9fef9a41a22bb4f"}, {file = "lxml-5.2.1-cp36-cp36m-musllinux_1_1_aarch64.whl", hash = "sha256:2a66bf12fbd4666dd023b6f51223aed3d9f3b40fef06ce404cb75bafd3d89536"}, {file = "lxml-5.2.1-cp36-cp36m-musllinux_1_1_ppc64le.whl", hash = "sha256:9123716666e25b7b71c4e1789ec829ed18663152008b58544d95b008ed9e21e9"}, {file = "lxml-5.2.1-cp36-cp36m-musllinux_1_1_s390x.whl", hash = "sha256:0c3f67e2aeda739d1cc0b1102c9a9129f7dc83901226cc24dd72ba275ced4218"}, @@ -1594,6 +1595,7 @@ files = [ {file = "msgpack-1.0.8-cp39-cp39-musllinux_1_1_x86_64.whl", hash = "sha256:5fbb160554e319f7b22ecf530a80a3ff496d38e8e07ae763b9e82fadfe96f273"}, {file = "msgpack-1.0.8-cp39-cp39-win32.whl", hash = "sha256:f9af38a89b6a5c04b7d18c492c8ccf2aee7048aff1ce8437c4683bb5a1df893d"}, {file = "msgpack-1.0.8-cp39-cp39-win_amd64.whl", hash = "sha256:ed59dd52075f8fc91da6053b12e8c89e37aa043f8986efd89e61fae69dc1b011"}, + {file = "msgpack-1.0.8-py3-none-any.whl", hash = "sha256:24f727df1e20b9876fa6e95f840a2a2651e34c0ad147676356f4bf5fbb0206ca"}, {file = "msgpack-1.0.8.tar.gz", hash = "sha256:95c02b0e27e706e48d0e5426d1710ca78e0f0628d6e89d5b5a5b91a5f12274f3"}, ] diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index e89cb6e7b..0b0638382 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -134,116 +134,7 @@ def test_logged_in_user_doesnt_do_evil_redirect(client_request): ) -@pytest.mark.parametrize( - "redirect_url", - [ - None, - f"/services/{SERVICE_ONE_ID}/templates", - ], -) -@pytest.mark.parametrize( - ("email_address", "password"), - [ - ("valid@example.gsa.gov", "val1dPassw0rd!"), - (" valid@example.gsa.gov ", " val1dPassw0rd! "), - ], -) -def test_process_sms_auth_sign_in_return_2fa_template( - client_request, - api_user_active, - mock_send_verify_code, - mock_get_user, - mock_get_user_by_email, - mock_verify_password, - email_address, - password, - redirect_url, -): - client_request.logout() - client_request.post( - "main.sign_in", - next=redirect_url, - _data={ - "email_address": email_address, - "password": password, - }, - _expected_redirect=url_for(".two_factor_sms", next=redirect_url), - ) - mock_verify_password.assert_called_with(api_user_active["id"], password) - mock_get_user_by_email.assert_called_with("valid@example.gsa.gov") - - -@pytest.mark.parametrize( - "redirect_url", - [ - None, - f"/services/{SERVICE_ONE_ID}/templates", - ], -) -def test_process_email_auth_sign_in_return_2fa_template( - client_request, - api_user_active_email_auth, - mock_send_verify_code, - mock_verify_password, - mocker, - redirect_url, -): - client_request.logout() - mocker.patch( - "app.user_api_client.get_user", return_value=api_user_active_email_auth - ) - mocker.patch( - "app.user_api_client.get_user_by_email", return_value=api_user_active_email_auth - ) - - client_request.post( - "main.sign_in", - next=redirect_url, - _data={ - "email_address": "valid@example.gsa.gov", - "password": "val1dPassw0rd!", - }, - _expected_redirect=url_for(".two_factor_email_sent", next=redirect_url), - ) - - mock_send_verify_code.assert_called_with( - api_user_active_email_auth["id"], "email", None, redirect_url - ) - mock_verify_password.assert_called_with( - api_user_active_email_auth["id"], "val1dPassw0rd!" - ) - - -def test_should_return_locked_out_true_when_user_is_locked( - client_request, - mock_get_user_by_email_locked, -): - client_request.logout() - page = client_request.post( - "main.sign_in", - _data={ - "email_address": "valid@example.gsa.gov", - "password": "whatIsMyPassword!", - }, - _expected_status=200, - ) - assert "The email address or password you entered is incorrect" in page.text - - -def test_should_return_200_when_user_does_not_exist( - client_request, - mock_get_user_by_email_not_found, -): - client_request.logout() - page = client_request.post( - "main.sign_in", - _data={"email_address": "notfound@gsa.gov", "password": "doesNotExist!"}, - _expected_status=200, - ) - - assert "The email address or password you entered is incorrect" in page.text - - +@pytest.mark.skip("TODO is this still relevant post login.gov switch?") def test_should_return_redirect_when_user_is_pending( client_request, mock_get_user_by_email_pending, @@ -273,6 +164,7 @@ def test_should_return_redirect_when_user_is_pending( f"/services/{SERVICE_ONE_ID}/templates", ], ) +@pytest.mark.skip("TODO is this still relevant post login.gov switch?") def test_should_attempt_redirect_when_user_is_pending( client_request, mock_get_user_by_email_pending, mock_verify_password, redirect_url ): @@ -288,37 +180,7 @@ def test_should_attempt_redirect_when_user_is_pending( ) -def test_email_address_is_treated_case_insensitively_when_signing_in_as_invited_user( - client_request, - mocker, - mock_verify_password, - api_user_active, - sample_invite, - mock_accept_invite, - mock_send_verify_code, - mock_get_invited_user_by_id, -): - client_request.logout() - sample_invite["email_address"] = "TEST@user.gsa.gov" - - mocker.patch( - "app.models.user.User.from_email_address_and_password_or_none", - return_value=User(api_user_active), - ) - - with client_request.session_transaction() as session: - session["invited_user_id"] = sample_invite["id"] - - client_request.post( - "main.sign_in", - _data={"email_address": "test@user.gsa.gov", "password": "val1dPassw0rd!"}, - ) - - assert mock_accept_invite.called - assert mock_send_verify_code.called - mock_get_invited_user_by_id.assert_called_once_with(sample_invite["id"]) - - +@pytest.mark.skip("TODO move this to register and update with login.gov") def test_when_signing_in_as_invited_user_you_cannot_accept_an_invite_for_another_email_address( client_request, mocker, From 78e8dc95fe7c17c7936c9167e84462a8b8070161 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 6 May 2024 13:12:27 -0700 Subject: [PATCH 02/12] fix tests --- app/main/views/register.py | 70 +++++++++++---------------- tests/app/main/views/test_register.py | 37 ++++++++------ 2 files changed, 49 insertions(+), 58 deletions(-) diff --git a/app/main/views/register.py b/app/main/views/register.py index 90b340b1c..5bd4d08c7 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -25,7 +25,6 @@ from app.main.forms import ( ) from app.main.views import sign_in from app.main.views.verify import activate_user -from app.models.service import Service from app.models.user import InvitedOrgUser, InvitedUser, User from app.utils import hide_from_search_engines, hilite @@ -156,41 +155,12 @@ def set_up_your_profile(): state = request.args.get("state") login_gov_error = request.args.get("error") if code and state: - _handle_login_dot_gov_invite(code, state) + _handle_login_dot_gov_invite(code, state, form) elif login_gov_error: current_app.logger.error(f"login.gov error: {login_gov_error}") raise Exception(f"Could not login with login.gov {login_gov_error}") # end login.gov - # create the user - # TODO we have to provide something for password until that column goes away - # TODO ideally we would set the user's preferred timezone here as well - - user = user_api_client.get_user_by_uuid_or_email(user_uuid, user_email) - if user is None: - user = User.register( - name=form.name.data, - email_address=user_email, - mobile_number=form.mobile_number.data, - password=str(uuid.uuid4()), - auth_type="sms_auth", - ) - - # activate the user - user = user_api_client.get_user_by_uuid_or_email(user_uuid, user_email) - activate_user(user["id"]) - usr = User.from_id(user["id"]) - usr.add_to_service( - invited_service.id, - invite_data["permissions"], - invite_data["folder_permissions"], - invite_data["from_user_id"], - ) - current_app.logger.debug( - hilite(f"Added user {usr.email_address} to service {invited_service.name}") - ) - return redirect(url_for("main.show_accounts_or_dashboard")) - return render_template("views/set-up-your-profile.html", form=form) @@ -208,26 +178,16 @@ def invited_user_accept_invite(invited_user_id): invited_user.accept_invite() -def _handle_login_dot_gov_invite(code, state): +def _handle_login_dot_gov_invite(code, state, form): access_token = sign_in._get_access_token(code, state) - print(f"access token {access_token}") user_email, user_uuid = sign_in._get_user_email_and_uuid(access_token) - print(f"user_email {user_email} user_uuid {user_uuid}") - print(f"state = {state}") invite_data = state.encode("utf8") - print(f"encoded invite data {invite_data}") invite_data = base64.b64decode(invite_data) - print(f"decoded invite data = {invite_data}") invite_data = json.loads(invite_data) - print(f"final invite data {invite_data}") - invited_service = Service.from_id(invite_data["service_id"]) - print(f"invited service {invited_service}") invited_user_id = invite_data["invited_user_id"] invited_user_email_address = get_invited_user_email_address(invited_user_id) - print(f"invited_user_email_address = {invited_user_email_address}") if user_email.lower() != invited_user_email_address.lower(): - print(f"HITTING THE FLASH") flash("You cannot accept an invite for another person.") session.pop("invited_user_id", None) abort(403) @@ -239,3 +199,29 @@ def _handle_login_dot_gov_invite(code, state): ) ) current_app.logger.debug(hilite("ACCEPTED INVITE")) + user = user_api_client.get_user_by_uuid_or_email(user_uuid, user_email) + if user is None: + user = User.register( + name=form.name.data, + email_address=user_email, + mobile_number=form.mobile_number.data, + password=str(uuid.uuid4()), + auth_type="sms_auth", + ) + + # activate the user + user = user_api_client.get_user_by_uuid_or_email(user_uuid, user_email) + activate_user(user["id"]) + usr = User.from_id(user["id"]) + usr.add_to_service( + invite_data["service_id"], + invite_data["permissions"], + invite_data["folder_permissions"], + invite_data["from_user_id"], + ) + current_app.logger.debug( + hilite( + f"Added user {usr.email_address} to service {invite_data['service_id']}" + ) + ) + return redirect(url_for("main.show_accounts_or_dashboard")) diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index b82f7dde1..2a13ebb9d 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -5,8 +5,9 @@ from unittest.mock import ANY import pytest from flask import url_for +from app.main.forms import RegisterUserForm from app.main.views.register import _handle_login_dot_gov_invite -from app.models.user import InvitedUser, User +from app.models.user import User from tests.conftest import normalize_spaces @@ -549,11 +550,6 @@ def test_handle_login_dot_gov_invite_bad_email(client_request, mocker): return_value=["fake@fake.gov", "12345"], ) - mocker.patch( - "app.main.views.register.Service.from_id", - return_value={"service_from_id"}, - ) - mocker.patch( "app.main.views.register.get_invited_user_email_address", return_value="boo@fake.gov", @@ -570,8 +566,7 @@ def test_handle_login_dot_gov_invite_bad_email(client_request, mocker): invite_data = invite_data.encode("utf8") invite_data = base64.b64encode(invite_data) invite_data = invite_data.decode("utf8") - print(f"sending this as state {invite_data}") - _handle_login_dot_gov_invite("code", invite_data) + _handle_login_dot_gov_invite("code", invite_data, RegisterUserForm()) mock_flash.assert_called_once_with( "You cannot accept an invite for another person." ) @@ -590,23 +585,33 @@ def test_handle_login_dot_gov_invite_good_email(client_request, mocker): return_value=["fake@fake.gov", "12345"], ) - mocker.patch( - "app.main.views.register.Service.from_id", - return_value={"service_from_id"}, - ) - mocker.patch( "app.main.views.register.get_invited_user_email_address", return_value="fake@fake.gov", ) + mocker.patch( + "app.main.views.register.user_api_client.get_user_by_uuid_or_email", + return_value={"id": "abc"}, + ) + + mock_user = mocker.patch( + "app.main.views.register.User.add_to_service", + ) + mock_accept = mocker.patch("app.main.views.register.invited_user_accept_invite") - invite_data = {"service_id": "service", "invited_user_id": "invited_user"} + invite_data = { + "service_id": "service", + "invited_user_id": "invited_user", + "permissions": ["manage_everything"], + "folder_permissions": [], + "from_user_id": "xyz", + } invite_data = json.dumps(invite_data) invite_data = invite_data.encode("utf8") invite_data = base64.b64encode(invite_data) invite_data = invite_data.decode("utf8") - print(f"sending this as state {invite_data}") - _handle_login_dot_gov_invite("code", invite_data) + _handle_login_dot_gov_invite("code", invite_data, RegisterUserForm()) mock_accept.assert_called_once() + mock_user.assert_called_once_with("service", ["manage_everything"], [], "xyz") From 125ad7a2f75fd31275afd6fc8743e0e7dd3182e4 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 7 May 2024 11:11:12 -0700 Subject: [PATCH 03/12] remove logged in elsewhere check --- app/models/user.py | 4 ++- tests/app/main/views/test_sign_in.py | 45 ---------------------------- 2 files changed, 3 insertions(+), 46 deletions(-) diff --git a/app/models/user.py b/app/models/user.py index 7e9f10632..3261dec01 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -141,7 +141,9 @@ class User(JSONModel, UserMixin): ) def logged_in_elsewhere(self): - return session.get("current_session_id") != self.current_session_id + # This check is deprecated due to the transition to using login.gov. + return False + # return session.get("current_session_id") != self.current_session_id def activate(self): if self.is_pending: diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index 0b0638382..135f4a5ba 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -47,21 +47,6 @@ def test_sign_in_explains_session_timeout(client_request): ) -def test_sign_in_explains_other_browser(client_request, api_user_active, mocker): - api_user_active["current_session_id"] = str(uuid.UUID(int=1)) - mocker.patch("app.user_api_client.get_user", return_value=api_user_active) - - with client_request.session_transaction() as session: - session["current_session_id"] = str(uuid.UUID(int=2)) - - page = client_request.get("main.sign_in", next="/foo") - - assert ( - "We signed you out because you logged in to Notify on another device" - in page.text - ) - - def test_doesnt_redirect_to_sign_in_if_no_session_info( client_request, api_user_active, @@ -78,36 +63,6 @@ def test_doesnt_redirect_to_sign_in_if_no_session_info( client_request.get("main.add_service") -@pytest.mark.parametrize( - ("db_sess_id", "cookie_sess_id"), - [ - (None, None), - (None, uuid.UUID(int=1)), # BAD - cookie doesn't match db - ( - uuid.UUID(int=1), - None, - ), # BAD - has used other browsers before but this is a brand new browser with no cookie - ( - uuid.UUID(int=1), - uuid.UUID(int=2), - ), # BAD - this person has just signed in on a different browser - ], -) -def test_redirect_to_sign_in_if_logged_in_from_other_browser( - client_request, api_user_active, mocker, db_sess_id, cookie_sess_id -): - api_user_active["current_session_id"] = db_sess_id - mocker.patch("app.user_api_client.get_user", return_value=api_user_active) - with client_request.session_transaction() as session: - session["current_session_id"] = str(cookie_sess_id) - - client_request.get( - "main.choose_account", - _expected_status=302, - _expected_redirect=url_for("main.sign_in", next="/accounts"), - ) - - def test_logged_in_user_redirects_to_account(client_request): client_request.get( "main.sign_in", From 64aa45dcaa4d9ec189ff99794bf2c36f0dfa08b0 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 7 May 2024 12:37:04 -0700 Subject: [PATCH 04/12] remove commented out code --- app/main/views/sign_in.py | 50 --------------------------------------- 1 file changed, 50 deletions(-) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index 7af9ce8ca..f9873c656 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -151,54 +151,6 @@ def sign_in(): return redirect(redirect_url) return redirect(url_for("main.show_accounts_or_dashboard")) - # form = LoginForm() - # current_app.logger.info("Got the login form") - # password_reset_url = url_for(".forgot_password", next=request.args.get("next")) - - # if form.validate_on_submit(): - # user = User.from_email_address_and_password_or_none( - # form.email_address.data, form.password.data - # ) - - # if user: - # # add user to session to mark us as in the process of signing the user in - # session["user_details"] = {"email": user.email_address, "id": user.id} - - # if user.state == "pending": - # return redirect( - # url_for("main.resend_email_verification", next=redirect_url) - # ) - - # if user.is_active: - # if session.get("invited_user_id"): - # invited_user = InvitedUser.from_session() - # if user.email_address.lower() != invited_user.email_address.lower(): - # flash("You cannot accept an invite for another person.") - # session.pop("invited_user_id", None) - # abort(403) - # else: - # invited_user.accept_invite() - - # user.send_login_code() - - # if user.sms_auth: - # return redirect(url_for(".two_factor_sms", next=redirect_url)) - - # if user.email_auth: - # return redirect( - # url_for(".two_factor_email_sent", next=redirect_url) - # ) - - # # Vague error message for login in case of user not known, locked, inactive or password not verified - # flash( - # Markup( - # ( - # f"The email address or password you entered is incorrect." - # f" Forgot your password?" - # ) - # ) - # ) - other_device = current_user.logged_in_elsewhere() token = generate_token( @@ -213,10 +165,8 @@ def sign_in(): url = url.replace("STATE", token) return render_template( "views/signin.html", - # form=form, again=bool(redirect_url), other_device=other_device, - # password_reset_url=password_reset_url, initial_signin_url=url, ) From 9c40a109003b43b3171f9f527859698127d73568 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 7 May 2024 13:58:59 -0700 Subject: [PATCH 05/12] cleanup --- app/main/views/register.py | 62 +++----- tests/app/main/views/test_accept_invite.py | 124 ---------------- tests/app/main/views/test_register.py | 156 --------------------- tests/app/test_navigation.py | 1 - 4 files changed, 20 insertions(+), 323 deletions(-) diff --git a/app/main/views/register.py b/app/main/views/register.py index 5bd4d08c7..1df991733 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -17,9 +17,8 @@ from flask_login import current_user from app import user_api_client from app.main import main -from app.main.forms import ( +from app.main.forms import ( # RegisterUserFromInviteForm, RegisterUserForm, - RegisterUserFromInviteForm, RegisterUserFromOrgInviteForm, SetupUserProfileForm, ) @@ -43,39 +42,10 @@ def register(): return render_template("views/register.html", form=form) -@main.route("/register-from-invite", methods=["GET", "POST"]) -# TODO This is deprecated, we are now handling invites in the -# login.gov workflow -def register_from_invite(): - invited_user = InvitedUser.from_session() - if not invited_user: - abort(404) - - form = RegisterUserFromInviteForm(invited_user) - - if form.validate_on_submit(): - if ( - form.service.data != invited_user.service - or form.email_address.data != invited_user.email_address - ): - abort(400) - _do_registration(form, send_email=False, send_sms=invited_user.sms_auth) - invited_user.accept_invite() - if invited_user.sms_auth: - return redirect(url_for("main.verify")) - else: - # we've already proven this user has email because they clicked the invite link, - # so just activate them straight away - return activate_user(session["user_details"]["id"]) - - return render_template( - "views/register-from-invite.html", invited_user=invited_user, form=form - ) - - @main.route("/register-from-org-invite", methods=["GET", "POST"]) # TODO This is deprecated, we are now handling invites in the -# login.gov workflow +# login.gov workflow. Leaving it here until we write the new +# org registration. def register_from_org_invite(): invited_org_user = InvitedOrgUser.from_session() if not invited_org_user: @@ -178,27 +148,35 @@ def invited_user_accept_invite(invited_user_id): invited_user.accept_invite() +def debug_msg(msg): + current_app.logger.debug(hilite(msg)) + + def _handle_login_dot_gov_invite(code, state, form): access_token = sign_in._get_access_token(code, state) + debug_msg("Got the access token for login.gov") user_email, user_uuid = sign_in._get_user_email_and_uuid(access_token) + debug_msg( + f"Got the user_email {user_email} and user_uuid {user_uuid} from login.gov" + ) invite_data = state.encode("utf8") invite_data = base64.b64decode(invite_data) invite_data = json.loads(invite_data) invited_user_id = invite_data["invited_user_id"] invited_user_email_address = get_invited_user_email_address(invited_user_id) + debug_msg(f"email address from the invite_date is {invited_user_email_address}") if user_email.lower() != invited_user_email_address.lower(): + debug_msg("invited user email did not match expected email, abort(403)") flash("You cannot accept an invite for another person.") session.pop("invited_user_id", None) abort(403) else: invited_user_accept_invite() - current_app.logger.debug( - hilite( - f"INVITED USER {invited_user_email_address} to service {invite_data['service_id']}" - ) + debug_msg( + f"invited user {invited_user_email_address} to service {invite_data['service_id']}" ) - current_app.logger.debug(hilite("ACCEPTED INVITE")) + debug_msg("accepted invite") user = user_api_client.get_user_by_uuid_or_email(user_uuid, user_email) if user is None: user = User.register( @@ -208,10 +186,12 @@ def _handle_login_dot_gov_invite(code, state, form): password=str(uuid.uuid4()), auth_type="sms_auth", ) + debug_msg(f"registered user {form.name.data} with email {user_email}") # activate the user user = user_api_client.get_user_by_uuid_or_email(user_uuid, user_email) activate_user(user["id"]) + debug_msg("activated user") usr = User.from_id(user["id"]) usr.add_to_service( invite_data["service_id"], @@ -219,9 +199,7 @@ def _handle_login_dot_gov_invite(code, state, form): invite_data["folder_permissions"], invite_data["from_user_id"], ) - current_app.logger.debug( - hilite( - f"Added user {usr.email_address} to service {invite_data['service_id']}" - ) + debug_msg( + f"Added user {usr.email_address} to service {invite_data['service_id']}" ) return redirect(url_for("main.show_accounts_or_dashboard")) diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 38202dfb2..a6d622c12 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -431,71 +431,6 @@ def test_existing_signed_out_user_accept_invite_redirects_to_sign_in( ) -@pytest.mark.usefixtures("_mock_no_users_for_service") -def test_new_user_accept_invite_calls_api_and_redirects_to_registration( - client_request, - service_one, - mock_check_invite_token, - mock_dont_get_user_by_email, - mock_add_user_to_service, - mock_get_service, - mocker, -): - client_request.logout() - client_request.get( - "main.accept_invite", - token="thisisnotarealtoken", - _expected_redirect="/register-from-invite", - ) - - mock_check_invite_token.assert_called_with("thisisnotarealtoken") - mock_dont_get_user_by_email.assert_called_with("invited_user@test.gsa.gov") - - -@pytest.mark.usefixtures("_mock_no_users_for_service") -def test_new_user_accept_invite_calls_api_and_views_registration_page( - client_request, - service_one, - sample_invite, - mock_check_invite_token, - mock_dont_get_user_by_email, - mock_get_invited_user_by_id, - mock_add_user_to_service, - mock_get_service, - mocker, -): - client_request.logout() - page = client_request.get( - "main.accept_invite", - token="thisisnotarealtoken", - _follow_redirects=True, - ) - - mock_check_invite_token.assert_called_with("thisisnotarealtoken") - mock_dont_get_user_by_email.assert_called_with("invited_user@test.gsa.gov") - mock_get_invited_user_by_id.assert_called_once_with(sample_invite["id"]) - - assert page.h1.string.strip() == "Create an account" - - assert normalize_spaces(page.select_one("main p").text) == ( - "Your account will be created with this email address: " - "invited_user@test.gsa.gov" - ) - - form = page.find("form") - name = form.find("input", id="name") - password = form.find("input", id="password") - service = form.find("input", type="hidden", id="service") - email = form.find("input", type="hidden", id="email_address") - - assert email - assert email.attrs["value"] == "invited_user@test.gsa.gov" - assert name - assert password - assert service - assert service.attrs["value"] == service_one["id"] - - def test_cancelled_invited_user_accepts_invited_redirect_to_cancelled_invitation( client_request, mock_get_user, @@ -562,65 +497,6 @@ def test_new_user_accept_invite_with_malformed_token( ) -@pytest.mark.usefixtures("_mock_no_users_for_service") -def test_new_user_accept_invite_completes_new_registration_redirects_to_verify( - client_request, - service_one, - sample_invite, - api_user_active, - mock_check_invite_token, - mock_dont_get_user_by_email, - mock_email_is_not_already_in_use, - mock_register_user, - mock_send_verify_code, - mock_get_invited_user_by_id, - mock_accept_invite, - mock_add_user_to_service, - mock_get_service, - mocker, -): - client_request.logout() - expected_redirect_location = "/register-from-invite" - - client_request.get( - "main.accept_invite", - token="thisisnotarealtoken", - _expected_redirect=expected_redirect_location, - ) - with client_request.session_transaction() as session: - assert session.get("invited_user_id") == sample_invite["id"] - - data = { - "service": sample_invite["service"], - "email_address": sample_invite["email_address"], - "from_user": sample_invite["from_user"], - "password": "longpassword", - "mobile_number": "+12027890123", - "name": "Invited User", - "auth_type": "email_auth", - } - - expected_redirect_location = "/verify" - client_request.post( - "main.register_from_invite", - _data=data, - _expected_redirect=expected_redirect_location, - ) - - mock_send_verify_code.assert_called_once_with(ANY, "sms", data["mobile_number"]) - mock_get_invited_user_by_id.assert_called_once_with(sample_invite["id"]) - - mock_register_user.assert_called_with( - data["name"], - data["email_address"], - data["mobile_number"], - data["password"], - data["auth_type"], - ) - - assert mock_accept_invite.call_count == 1 - - def test_signed_in_existing_user_cannot_use_anothers_invite( client_request, mocker, diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index 2a13ebb9d..b491a1b53 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -219,144 +219,6 @@ def test_register_with_existing_email_sends_emails( ) -@pytest.mark.parametrize( - ("email_address", "expected_value"), - [ - ("first.last@example.com", "First Last"), - ("first.middle.last@example.com", "First Middle Last"), - ("first.m.last@example.com", "First Last"), - ("first.last-last@example.com", "First Last-Last"), - ("first.o'last@example.com", "First O’Last"), - ("first.last+testing@example.com", "First Last"), - ("first.last+testing+testing@example.com", "First Last"), - ("first.last6@example.com", "First Last"), - ("first.last.212@example.com", "First Last"), - ("first.2.last@example.com", "First Last"), - ("first.2b.last@example.com", "First Last"), - ("first.1.2.3.last@example.com", "First Last"), - ("first.last.1.2.3@example.com", "First Last"), - # Instances where we can’t make a good-enough guess: - ("example123@example.com", None), - ("f.last@example.com", None), - ("f.m.last@example.com", None), - ], -) -def test_shows_name_on_registration_page_from_invite( - client_request, - fake_uuid, - email_address, - expected_value, - sample_invite, - mock_get_invited_user_by_id, -): - sample_invite["email_address"] = email_address - with client_request.session_transaction() as session: - session["invited_user_id"] = sample_invite - - page = client_request.get("main.register_from_invite") - assert page.select_one("input[name=name]").get("value") == expected_value - - -def test_shows_hidden_email_address_on_registration_page_from_invite( - client_request, - fake_uuid, - sample_invite, - mock_get_invited_user_by_id, -): - with client_request.session_transaction() as session: - session["invited_user_id"] = sample_invite - - page = client_request.get("main.register_from_invite") - assert normalize_spaces(page.select_one("main p").text) == ( - "Your account will be created with this email address: invited_user@test.gsa.gov" - ) - hidden_input = page.select_one("form .usa-sr-only input") - for attr, value in ( - ("type", "email"), - ("name", "username"), - ("id", "username"), - ("value", "invited_user@test.gsa.gov"), - ("disabled", "disabled"), - ("tabindex", "-1"), - ("aria-hidden", "true"), - ("autocomplete", "username"), - ): - assert hidden_input[attr] == value - - -@pytest.mark.parametrize( - "extra_data", - [ - {}, - # The username field is present in the page but the POST request - # should ignore it - {"username": "invited@user.com"}, - {"username": "anythingelse@example.com"}, - ], -) -def test_register_from_invite( - client_request, - fake_uuid, - mock_email_is_not_already_in_use, - mock_register_user, - mock_send_verify_code, - mock_accept_invite, - mock_get_invited_user_by_id, - sample_invite, - extra_data, -): - client_request.logout() - with client_request.session_transaction() as session: - session["invited_user_id"] = sample_invite["id"] - client_request.post( - "main.register_from_invite", - _data=dict( - name="Registered in another Browser", - email_address=sample_invite["email_address"], - mobile_number="+12024900460", - service=sample_invite["service"], - password="somreallyhardthingtoguess", - auth_type="sms_auth", - **extra_data, - ), - _expected_redirect=url_for("main.verify"), - ) - mock_register_user.assert_called_once_with( - "Registered in another Browser", - sample_invite["email_address"], - "+12024900460", - "somreallyhardthingtoguess", - "sms_auth", - ) - mock_get_invited_user_by_id.assert_called_once_with(sample_invite["id"]) - - -def test_register_from_invite_when_user_registers_in_another_browser( - client_request, - api_user_active, - mock_get_user_by_email, - mock_accept_invite, - mock_get_invited_user_by_id, - sample_invite, -): - client_request.logout() - sample_invite["email_address"] = api_user_active["email_address"] - with client_request.session_transaction() as session: - session["invited_user_id"] = sample_invite["id"] - client_request.post( - "main.register_from_invite", - _data={ - "name": "Registered in another Browser", - "email_address": api_user_active["email_address"], - "mobile_number": api_user_active["mobile_number"], - "service": sample_invite["service"], - "password": "somreallyhardthingtoguess", - "auth_type": "sms_auth", - }, - _expected_redirect=url_for("main.verify"), - ) - - @pytest.mark.parametrize( "invite_email_address", ["gov-user@gsa.gov", "non-gov-user@example.com"] ) @@ -520,24 +382,6 @@ def test_cannot_register_with_sms_auth_and_missing_mobile_number( assert err.attrs["data-error-label"] == "mobile_number" -def test_register_from_invite_form_doesnt_show_mobile_number_field_if_email_auth( - client_request, - sample_invite, - mock_get_invited_user_by_id, -): - client_request.logout() - sample_invite["auth_type"] = "email_auth" - with client_request.session_transaction() as session: - session["invited_user_id"] = sample_invite["id"] - - page = client_request.get("main.register_from_invite") - - assert ( - page.find("input", attrs={"name": "auth_type"}).attrs["value"] == "email_auth" - ) - assert page.find("input", attrs={"name": "mobile_number"}) is None - - def test_handle_login_dot_gov_invite_bad_email(client_request, mocker): mocker.patch( diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index 4634a65cf..6f1cf58eb 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -146,7 +146,6 @@ EXCLUDED_ENDPOINTS = tuple( "received_text_messages_callback", "redact_template", "register", - "register_from_invite", "register_from_org_invite", "registration_continue", "remove_user_from_organization", From 1d9457f9a2e978727f0c3ca61b3ee0f6fb1b7ac1 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 7 May 2024 14:36:21 -0700 Subject: [PATCH 06/12] add debug statements --- app/main/views/register.py | 10 +++++++--- tests/app/main/views/test_register.py | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/app/main/views/register.py b/app/main/views/register.py index 1df991733..fbcf42a73 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -125,7 +125,7 @@ def set_up_your_profile(): state = request.args.get("state") login_gov_error = request.args.get("error") if code and state: - _handle_login_dot_gov_invite(code, state, form) + return _handle_login_dot_gov_invite(code, state, form) elif login_gov_error: current_app.logger.error(f"login.gov error: {login_gov_error}") raise Exception(f"Could not login with login.gov {login_gov_error}") @@ -153,16 +153,20 @@ def debug_msg(msg): def _handle_login_dot_gov_invite(code, state, form): - + debug_msg(f"enter _handle_login_dot_gov_invite with code {code} state {state}") access_token = sign_in._get_access_token(code, state) debug_msg("Got the access token for login.gov") user_email, user_uuid = sign_in._get_user_email_and_uuid(access_token) debug_msg( f"Got the user_email {user_email} and user_uuid {user_uuid} from login.gov" ) + debug_msg(f"raw state {state}") invite_data = state.encode("utf8") + debug_msg(f"utf8 encoded state {invite_data}") invite_data = base64.b64decode(invite_data) + debug_msg(f"b64 decoded state {invite_data}") invite_data = json.loads(invite_data) + debug_msg(f"final state {invite_data}") invited_user_id = invite_data["invited_user_id"] invited_user_email_address = get_invited_user_email_address(invited_user_id) debug_msg(f"email address from the invite_date is {invited_user_email_address}") @@ -172,7 +176,7 @@ def _handle_login_dot_gov_invite(code, state, form): session.pop("invited_user_id", None) abort(403) else: - invited_user_accept_invite() + invited_user_accept_invite(invited_user_id) debug_msg( f"invited user {invited_user_email_address} to service {invite_data['service_id']}" ) diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index b491a1b53..f6a96d6b0 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -459,3 +459,30 @@ def test_handle_login_dot_gov_invite_good_email(client_request, mocker): _handle_login_dot_gov_invite("code", invite_data, RegisterUserForm()) mock_accept.assert_called_once() mock_user.assert_called_once_with("service", ["manage_everything"], [], "xyz") + + +# Taken from the API project in service_invite/rest.py +def get_user_data_url_safe(data): + data = json.dumps(data) + data = base64.b64encode(data.encode("utf8")) + return data.decode("utf8") + + +def get_decoded(state): + state = state.encode("utf8") + state = base64.b64decode(state) + state = json.loads(state) + return state + + +# Test that we can successfully decode the invited user +# data that is sent in the state param +def test_decode_state(): + invite_data = { + "from_user_id": "abc", + "service_id": "bcd", + "permissions": ["manage_everything"], + "folder_permissions": [], + } + state = get_user_data_url_safe(invite_data) + assert invite_data == get_decoded(state) From 87a0204fe2f74e6c7ba67af8d7a17799f90e3ae8 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 8 May 2024 08:36:39 -0700 Subject: [PATCH 07/12] explain how to create a first user in the db --- README.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/README.md b/README.md index f907dca88..ebbb5af88 100644 --- a/README.md +++ b/README.md @@ -351,6 +351,28 @@ This will run the local development web server and make the admin site available at http://localhost:6012; remember to make sure that the Notify.gov API is running as well! +## Creating a 'First User' in the database + +After you have completed all setup steps, you will be unable to log in, because there +will not be a user in the database to link to the login.gov account you are using. So +you will need to create that user in your database using the 'create-test-user' command. + +Open two terminals pointing to the api project and then run these commands in the +respective terminals. Make sure the email address is the same one you are using in +login.gov and make sure your phone number is in the format 5555555555. + +(Server 1) +env ALLOW_EXPIRED_API_TOKEN=1 make run-flask + +(Server 2) +poetry run flask command create-admin-jwt | tail -n 1 | pbcopy +poetry run flask command create-test-user --name="" --email="" --mobile_number="" --password="" --admin=True; + +If for any reason in the course of development it is necessary for your to delete your db +via the `dropdb` command, you will need to repeat these steps when you recreate your db. + + + ## Git Hooks We're using [`pre-commit`](https://pre-commit.com/) to manage hooks in order to From b79835dfd2cd2556a51c8b1e3f58e9abe151cd00 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 8 May 2024 09:10:41 -0700 Subject: [PATCH 08/12] code review feedback --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index ebbb5af88..0d7ffc5bc 100644 --- a/README.md +++ b/README.md @@ -358,15 +358,17 @@ will not be a user in the database to link to the login.gov account you are usin you will need to create that user in your database using the 'create-test-user' command. Open two terminals pointing to the api project and then run these commands in the -respective terminals. Make sure the email address is the same one you are using in -login.gov and make sure your phone number is in the format 5555555555. +respective terminals. (Server 1) env ALLOW_EXPIRED_API_TOKEN=1 make run-flask (Server 2) poetry run flask command create-admin-jwt | tail -n 1 | pbcopy -poetry run flask command create-test-user --name="" --email="" --mobile_number="" --password="" --admin=True; +poetry run flask command create-test-user --admin=True; + +Supply your name, email address, mobile number, and password when prompted. Make sure the email address +is the same one you are using in login.gov and make sure your phone number is in the format 5555555555. If for any reason in the course of development it is necessary for your to delete your db via the `dropdb` command, you will need to repeat these steps when you recreate your db. From e51517347ffa8b1656533d88d10895b28ae64150 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 8 May 2024 11:06:10 -0700 Subject: [PATCH 09/12] convert to test fixture --- tests/app/main/views/test_register.py | 22 +++++++--------------- tests/conftest.py | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index f6a96d6b0..cf4fba5a2 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -461,14 +461,7 @@ def test_handle_login_dot_gov_invite_good_email(client_request, mocker): mock_user.assert_called_once_with("service", ["manage_everything"], [], "xyz") -# Taken from the API project in service_invite/rest.py -def get_user_data_url_safe(data): - data = json.dumps(data) - data = base64.b64encode(data.encode("utf8")) - return data.decode("utf8") - - -def get_decoded(state): +def decode_invite_data(state): state = state.encode("utf8") state = base64.b64decode(state) state = json.loads(state) @@ -477,12 +470,11 @@ def get_decoded(state): # Test that we can successfully decode the invited user # data that is sent in the state param -def test_decode_state(): - invite_data = { - "from_user_id": "abc", - "service_id": "bcd", - "permissions": ["manage_everything"], +def test_decode_state(encoded_invite_data): + assert decode_invite_data(encoded_invite_data) == { "folder_permissions": [], + "from_user_id": "xyz", + "invited_user_id": "invited_user", + "permissions": ["manage_everything"], + "service_id": "service", } - state = get_user_data_url_safe(invite_data) - assert invite_data == get_decoded(state) diff --git a/tests/conftest.py b/tests/conftest.py index ab86e3052..fe1ee0c02 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,4 @@ +import base64 import copy import json import os @@ -1901,6 +1902,25 @@ def sample_invite(mocker, service_one): ) +@pytest.fixture() +def encoded_invite_data(): + """ + This mimics what API does when it encodes invite data in + service_invite/rest.py + """ + invite_data = { + "service_id": "service", + "invited_user_id": "invited_user", + "permissions": ["manage_everything"], + "folder_permissions": [], + "from_user_id": "xyz", + } + invite_data = json.dumps(invite_data) + invite_data = invite_data.encode("utf8") + invite_data = base64.b64encode(invite_data) + return invite_data.decode("utf8") + + @pytest.fixture() def expired_invite(service_one): id_ = USER_ONE_ID From 852c0e57c8b0f2e2b6b327ee5dd7965a73f76356 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 8 May 2024 11:30:51 -0700 Subject: [PATCH 10/12] convert to test fixture --- app/main/views/register.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/main/views/register.py b/app/main/views/register.py index fbcf42a73..e9e03a76a 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -17,7 +17,7 @@ from flask_login import current_user from app import user_api_client from app.main import main -from app.main.forms import ( # RegisterUserFromInviteForm, +from app.main.forms import ( RegisterUserForm, RegisterUserFromOrgInviteForm, SetupUserProfileForm, From 26988d255fad93a81e8279df26020479b6bc2dc8 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 8 May 2024 12:45:01 -0700 Subject: [PATCH 11/12] remove logged_in_elsewhere as per code review feedback --- app/main/views/sign_in.py | 3 --- app/models/user.py | 11 +---------- tests/app/models/test_user.py | 1 - 3 files changed, 1 insertion(+), 14 deletions(-) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index f9873c656..618a35654 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -151,8 +151,6 @@ def sign_in(): return redirect(redirect_url) return redirect(url_for("main.show_accounts_or_dashboard")) - other_device = current_user.logged_in_elsewhere() - token = generate_token( str(request.remote_addr), current_app.config["SECRET_KEY"], @@ -166,7 +164,6 @@ def sign_in(): return render_template( "views/signin.html", again=bool(redirect_url), - other_device=other_device, initial_signin_url=url, ) diff --git a/app/models/user.py b/app/models/user.py index 3261dec01..932e754c8 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -140,11 +140,6 @@ class User(JSONModel, UserMixin): set_by_id=set_by_id, ) - def logged_in_elsewhere(self): - # This check is deprecated due to the transition to using login.gov. - return False - # return session.get("current_session_id") != self.current_session_id - def activate(self): if self.is_pending: user_data = user_api_client.activate_user(self.id) @@ -198,7 +193,7 @@ class User(JSONModel, UserMixin): @property def is_authenticated(self): - return not self.logged_in_elsewhere() and super(User, self).is_authenticated + return super(User, self).is_authenticated @property def platform_admin(self): @@ -676,10 +671,6 @@ class InvitedOrgUser(JSONModel): class AnonymousUser(AnonymousUserMixin): - # set the anonymous user so that if a new browser hits us we don't error http://stackoverflow.com/a/19275188 - - def logged_in_elsewhere(self): - return False @property def default_organization(self): diff --git a/tests/app/models/test_user.py b/tests/app/models/test_user.py index 8c4c26907..1caf85f4a 100644 --- a/tests/app/models/test_user.py +++ b/tests/app/models/test_user.py @@ -6,7 +6,6 @@ from tests.conftest import SERVICE_ONE_ID, USER_ONE_ID def test_anonymous_user(notify_admin): assert AnonymousUser().is_authenticated is False - assert AnonymousUser().logged_in_elsewhere() is False assert AnonymousUser().default_organization.name is None assert AnonymousUser().default_organization.domains == [] assert AnonymousUser().default_organization.organization_type is None From 85ddd494b35d728cbb4dbaaff6e130f7c2804c25 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Thu, 9 May 2024 07:33:26 -0700 Subject: [PATCH 12/12] remove debug statement --- app/utils/csv.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/utils/csv.py b/app/utils/csv.py index 04d5d19ed..e6119b073 100644 --- a/app/utils/csv.py +++ b/app/utils/csv.py @@ -1,7 +1,6 @@ import datetime import pytz -from flask import current_app from flask_login import current_user from notifications_utils.recipients import RecipientCSV @@ -67,7 +66,6 @@ def generate_notifications_csv(**kwargs): from app import notification_api_client from app.s3_client.s3_csv_client import s3download - current_app.logger.info("\n\n\n\nENTER generate_notifications_csv") if "page" not in kwargs: kwargs["page"] = 1