From 34d727748bfac7ca8ac40eecb77c349721d9d88a Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Fri, 26 Jul 2024 09:15:09 -0700 Subject: [PATCH 1/3] cleanup notify-compliance-46 --- app/notify_client/__init__.py | 20 +++++++++++-------- .../test_notify_admin_api_client.py | 19 ++++++++++++++++++ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/app/notify_client/__init__.py b/app/notify_client/__init__.py index 5f65e85c67..ea3bd8e616 100644 --- a/app/notify_client/__init__.py +++ b/app/notify_client/__init__.py @@ -56,6 +56,9 @@ def check_inactive_service(self): ): abort(403) + def is_calling_signin_url(self, arg): + return arg[0].startswith("/user") + def check_inactive_user(self, *args): still_signing_in = False @@ -64,14 +67,15 @@ def check_inactive_user(self, *args): # and we only want to check the first arg for arg in args: arg = str(arg) - if ( - "get-login-gov-user" in arg - or "user/email" in arg - or "/activate" in arg - or "/email-code" in arg - or "/verify/code" in arg - or "/user" in arg - ): + if self.is_calling_signin_url(arg): + # if ( + # "get-login-gov-user" in arg + # or "user/email" in arg + # or "/activate" in arg + # or "/email-code" in arg + # or "/verify/code" in arg + # or "/user" in arg + # ): still_signing_in = True # This seems to be a weird edge case that happens intermittently with invites diff --git a/tests/app/notify_client/test_notify_admin_api_client.py b/tests/app/notify_client/test_notify_admin_api_client.py index e68d4669f8..ec5de4522f 100644 --- a/tests/app/notify_client/test_notify_admin_api_client.py +++ b/tests/app/notify_client/test_notify_admin_api_client.py @@ -41,6 +41,25 @@ def test_active_service_can_be_modified(notify_admin, method, user, service): assert ret == request.return_value +@pytest.mark.parametrize( + ("arg", "expected_result"), + [ + ( + ("/user/c5f8a5c9-56d5-4fa9-8c30-3449ae10c072/verify/code",), + True, + ), + ( + ("/service/blahblahblah",), + False, + ), + ], +) +def test_is_calling_signin_url(arg, expected_result): + api_client = NotifyAdminAPIClient() + result = api_client.is_calling_signin_url(arg) + assert result == expected_result + + @pytest.mark.parametrize("method", ["put", "post", "delete"]) def test_inactive_service_cannot_be_modified_by_normal_user( notify_admin, api_user_active, method From e884f31fa08e86f4d2485672646b51f280a67004 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Fri, 26 Jul 2024 09:49:12 -0700 Subject: [PATCH 2/3] write test --- app/notify_client/__init__.py | 2 +- tests/app/notify_client/test_notify_admin_api_client.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/notify_client/__init__.py b/app/notify_client/__init__.py index ea3bd8e616..3d7fd62a83 100644 --- a/app/notify_client/__init__.py +++ b/app/notify_client/__init__.py @@ -57,7 +57,7 @@ def check_inactive_service(self): abort(403) def is_calling_signin_url(self, arg): - return arg[0].startswith("/user") + return arg.startswith("('/user") def check_inactive_user(self, *args): still_signing_in = False diff --git a/tests/app/notify_client/test_notify_admin_api_client.py b/tests/app/notify_client/test_notify_admin_api_client.py index ec5de4522f..d15eebfc30 100644 --- a/tests/app/notify_client/test_notify_admin_api_client.py +++ b/tests/app/notify_client/test_notify_admin_api_client.py @@ -45,11 +45,12 @@ def test_active_service_can_be_modified(notify_admin, method, user, service): ("arg", "expected_result"), [ ( - ("/user/c5f8a5c9-56d5-4fa9-8c30-3449ae10c072/verify/code",), + "('/user/c5f8a5c9-56d5-4fa9-8c30-3449ae10c072/verify/code',)", True, ), + ("('/user/get-login-gov-user',)", True), ( - ("/service/blahblahblah",), + "('/service/blahblahblah',)", False, ), ], From af59b13836ff069405d0cfff70e09b15c61765a4 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Fri, 2 Aug 2024 14:30:35 -0700 Subject: [PATCH 3/3] code review feedback --- app/notify_client/__init__.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/notify_client/__init__.py b/app/notify_client/__init__.py index 3d7fd62a83..8db3425f2a 100644 --- a/app/notify_client/__init__.py +++ b/app/notify_client/__init__.py @@ -68,14 +68,6 @@ def check_inactive_user(self, *args): for arg in args: arg = str(arg) if self.is_calling_signin_url(arg): - # if ( - # "get-login-gov-user" in arg - # or "user/email" in arg - # or "/activate" in arg - # or "/email-code" in arg - # or "/verify/code" in arg - # or "/user" in arg - # ): still_signing_in = True # This seems to be a weird edge case that happens intermittently with invites