From e22386e91b01678551f28a462923eab499e80c1f Mon Sep 17 00:00:00 2001 From: Andrew Shumway Date: Thu, 14 Sep 2023 14:59:48 -0600 Subject: [PATCH 1/5] Redirect implemented with tests coverage --- app/__init__.py | 18 ++++++++++++++++++ tests/app/test_event_handlers.py | 13 +++++++++++++ 2 files changed, 31 insertions(+) diff --git a/app/__init__.py b/app/__init__.py index 5f5bb4680..e93776cb7 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -2,6 +2,7 @@ import os import pathlib from functools import partial from time import monotonic +from urllib.parse import urlparse, urlunparse import jinja2 from flask import ( @@ -254,6 +255,7 @@ def create_app(application): def init_app(application): + application.before_request(redirect_notify_to_beta) application.before_request(load_service_before_request) application.before_request(load_organization_before_request) application.before_request(request_helper.check_proxy_header_before_request) @@ -332,6 +334,22 @@ def make_session_permanent(): session.permanent = True +def create_url(environment, url): + url_created = urlparse(url) + if environment == "production": + url_list = list(url_created) + url_list[1] = "beta.notify.gov" + url_for_redirect = urlunparse(url_list) + return url_for_redirect + return url + + +def redirect_notify_to_beta(): + url_to_beta = create_url(current_app.config["NOTIFY_ENVIRONMENT"], request.url) + if current_app.config["NOTIFY_ENVIRONMENT"] == "production": + redirect(url_to_beta, 301) + + def load_service_before_request(): if "/static/" in request.url: request_ctx.service = None diff --git a/tests/app/test_event_handlers.py b/tests/app/test_event_handlers.py index 0f188a95f..5db6645fd 100644 --- a/tests/app/test_event_handlers.py +++ b/tests/app/test_event_handlers.py @@ -1,6 +1,7 @@ import uuid from unittest.mock import ANY +from app import create_url from app.event_handlers import ( create_add_user_to_service_event, create_archive_service_event, @@ -129,3 +130,15 @@ def test_set_user_permissions(client_request, mock_events): create_set_user_permissions_event(**kwargs) mock_events.assert_called_with("set_user_permissions", event_dict(**kwargs)) + + +def test_create_url(): + url_for_redirect = create_url( + "production", "https://notify.gov/using-notify/get-started" + ) + assert url_for_redirect == "https://beta.notify.gov/using-notify/get-started" + + +def test_create_url_non_production(): + url = create_url("development", "https://notify.gov/using-notify/get-started") + assert url == "https://notify.gov/using-notify/get-started" From 118fd131891dcb5f9e7d403284db01f1470dda25 Mon Sep 17 00:00:00 2001 From: Andrew Shumway Date: Mon, 18 Sep 2023 06:24:17 -0600 Subject: [PATCH 2/5] Refactor code. Tests still failing --- app/__init__.py | 19 ++++++++++--------- tests/app/main/test_beta_redirect.py | 18 ++++++++++++++++++ tests/app/test_event_handlers.py | 13 ------------- 3 files changed, 28 insertions(+), 22 deletions(-) create mode 100644 tests/app/main/test_beta_redirect.py diff --git a/app/__init__.py b/app/__init__.py index d57621b6a..432c7f456 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -330,19 +330,20 @@ def make_session_permanent(): session.permanent = True -def create_url(environment, url): +def create_beta_url(url): url_created = urlparse(url) - if environment == "production": - url_list = list(url_created) - url_list[1] = "beta.notify.gov" - url_for_redirect = urlunparse(url_list) - return url_for_redirect - return url + url_list = list(url_created) + url_list[1] = "beta.notify.gov" + url_for_redirect = urlunparse(url_list) + return url_for_redirect def redirect_notify_to_beta(): - url_to_beta = create_url(current_app.config["NOTIFY_ENVIRONMENT"], request.url) - if current_app.config["NOTIFY_ENVIRONMENT"] == "production": + if ( + current_app.config["NOTIFY_ENVIRONMENT"] == "production" + and "beta.notify.gov" not in request.url + ): + url_to_beta = create_beta_url(request.url) redirect(url_to_beta, 301) diff --git a/tests/app/main/test_beta_redirect.py b/tests/app/main/test_beta_redirect.py new file mode 100644 index 000000000..b3cbb28eb --- /dev/null +++ b/tests/app/main/test_beta_redirect.py @@ -0,0 +1,18 @@ +from flask import current_app +from unittest.mock import patch + +from app import create_beta_url + + +def test_create_beta_url(): + url_for_redirect = create_beta_url("https://notify.gov/using-notify/get-started") + assert url_for_redirect == "https://beta.notify.gov/using-notify/get-started" + + +def test_redirect_notify_to_beta(monkeypatch, client_request): + monkeypatch.setitem(current_app.config, "NOTIFY_ENVIRONMENT", "production") + # import pdb + # pdb.set_trace() + # resp = client_request.get_response_from_url("https://notify.gov/using-notify/get-started") + # assert resp.status_code == 301 + assert current_app.config["NOTIFY_ENVIRONMENT"] == "production" diff --git a/tests/app/test_event_handlers.py b/tests/app/test_event_handlers.py index 5db6645fd..0f188a95f 100644 --- a/tests/app/test_event_handlers.py +++ b/tests/app/test_event_handlers.py @@ -1,7 +1,6 @@ import uuid from unittest.mock import ANY -from app import create_url from app.event_handlers import ( create_add_user_to_service_event, create_archive_service_event, @@ -130,15 +129,3 @@ def test_set_user_permissions(client_request, mock_events): create_set_user_permissions_event(**kwargs) mock_events.assert_called_with("set_user_permissions", event_dict(**kwargs)) - - -def test_create_url(): - url_for_redirect = create_url( - "production", "https://notify.gov/using-notify/get-started" - ) - assert url_for_redirect == "https://beta.notify.gov/using-notify/get-started" - - -def test_create_url_non_production(): - url = create_url("development", "https://notify.gov/using-notify/get-started") - assert url == "https://notify.gov/using-notify/get-started" From 15123f06f992e4762c980beffe9454453e4dd9bb Mon Sep 17 00:00:00 2001 From: Andrew Shumway Date: Mon, 18 Sep 2023 06:33:26 -0600 Subject: [PATCH 3/5] Removed import --- tests/app/main/test_beta_redirect.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/app/main/test_beta_redirect.py b/tests/app/main/test_beta_redirect.py index b3cbb28eb..eaced1b8e 100644 --- a/tests/app/main/test_beta_redirect.py +++ b/tests/app/main/test_beta_redirect.py @@ -1,5 +1,4 @@ from flask import current_app -from unittest.mock import patch from app import create_beta_url From d02d2de9dc70a62974181798e3b5e11fe2490d97 Mon Sep 17 00:00:00 2001 From: Carlo Costino Date: Mon, 18 Sep 2023 15:59:28 -0400 Subject: [PATCH 4/5] Made a few adjustments to test the beta redirect: - Explicitly return the redirect - Change the redirect to be a 302 instead of 301 - Adjusted the test client to allow sub domains - Added the remaining tests A big thank you to @A-Shumway42 for getting this work underway! Signed-off-by: Carlo Costino --- app/__init__.py | 2 +- tests/app/main/test_beta_redirect.py | 29 ++++++++++++++++++++++++---- tests/conftest.py | 1 + 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 432c7f456..7c5879a30 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -344,7 +344,7 @@ def redirect_notify_to_beta(): and "beta.notify.gov" not in request.url ): url_to_beta = create_beta_url(request.url) - redirect(url_to_beta, 301) + return redirect(url_to_beta, 302) def load_service_before_request(): diff --git a/tests/app/main/test_beta_redirect.py b/tests/app/main/test_beta_redirect.py index eaced1b8e..832656fe4 100644 --- a/tests/app/main/test_beta_redirect.py +++ b/tests/app/main/test_beta_redirect.py @@ -8,10 +8,31 @@ def test_create_beta_url(): assert url_for_redirect == "https://beta.notify.gov/using-notify/get-started" +def test_no_redirect_notify_to_beta_non_production(monkeypatch, client_request): + monkeypatch.setitem(current_app.config, "NOTIFY_ENVIRONMENT", "development") + assert current_app.config["NOTIFY_ENVIRONMENT"] == "development" + + + client_request.get_response_from_url( + "https://notify.gov/using-notify/get-started", + _expected_status=200 + ) + + def test_redirect_notify_to_beta(monkeypatch, client_request): monkeypatch.setitem(current_app.config, "NOTIFY_ENVIRONMENT", "production") - # import pdb - # pdb.set_trace() - # resp = client_request.get_response_from_url("https://notify.gov/using-notify/get-started") - # assert resp.status_code == 301 assert current_app.config["NOTIFY_ENVIRONMENT"] == "production" + + client_request.get_response_from_url( + "https://notify.gov/using-notify/get-started", + _expected_status=302 + ) + +def test_no_redirect_beta_notify_to_beta(monkeypatch, client_request): + monkeypatch.setitem(current_app.config, "NOTIFY_ENVIRONMENT", "production") + assert current_app.config["NOTIFY_ENVIRONMENT"] == "production" + + client_request.get_response_from_url( + "https://beta.notify.gov/using-notify/get-started", + _expected_status=200 + ) diff --git a/tests/conftest.py b/tests/conftest.py index 9b6cc3efc..942e00526 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2392,6 +2392,7 @@ def _client(notify_admin): Do not use this fixture directly – use `client_request` instead """ with notify_admin.test_request_context(), notify_admin.test_client() as client: + client.allow_subdomain_redirects = True yield client From 311018940216e660bd52e6858b56d3d4ac0fca8c Mon Sep 17 00:00:00 2001 From: Carlo Costino Date: Mon, 18 Sep 2023 16:18:48 -0400 Subject: [PATCH 5/5] Fix test style issues Signed-off-by: Carlo Costino --- tests/app/main/test_beta_redirect.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/main/test_beta_redirect.py b/tests/app/main/test_beta_redirect.py index 832656fe4..e192941af 100644 --- a/tests/app/main/test_beta_redirect.py +++ b/tests/app/main/test_beta_redirect.py @@ -12,7 +12,6 @@ def test_no_redirect_notify_to_beta_non_production(monkeypatch, client_request): monkeypatch.setitem(current_app.config, "NOTIFY_ENVIRONMENT", "development") assert current_app.config["NOTIFY_ENVIRONMENT"] == "development" - client_request.get_response_from_url( "https://notify.gov/using-notify/get-started", _expected_status=200 @@ -28,6 +27,7 @@ def test_redirect_notify_to_beta(monkeypatch, client_request): _expected_status=302 ) + def test_no_redirect_beta_notify_to_beta(monkeypatch, client_request): monkeypatch.setitem(current_app.config, "NOTIFY_ENVIRONMENT", "production") assert current_app.config["NOTIFY_ENVIRONMENT"] == "production"