Merge pull request #377 from communitiesuk/e2e/govuk-notify-reference

Set GOV.UK Notify references in emails for magic links

.vscode/settings.json

@@ -5,4 +5,5 @@
     ],
     "python.testing.unittestEnabled": false,
     "python.testing.pytestEnabled": true,
+    "ruff.enable": false,
 }

config/envs/default.py

@@ -135,8 +135,7 @@ class DefaultConfig(object):
     """
     Magic Links
     """
-    MAGIC_LINK_EXPIRY_DAYS = 1
-    MAGIC_LINK_EXPIRY_SECONDS = 86400 * MAGIC_LINK_EXPIRY_DAYS
+    MAGIC_LINK_EXPIRY_SECONDS = 60 * 60  # last for an hour
     MAGIC_LINK_RECORD_PREFIX = "link"
     MAGIC_LINK_USER_PREFIX = "account"
     MAGIC_LINK_LANDING_PAGE = "/service/magic-links/landing/"

frontend/magic_links/routes.py

@@ -1,3 +1,5 @@
+import uuid
+
 from config import Config
 from flask import abort
 from flask import Blueprint
@@ -120,6 +122,13 @@ def new():
     # Grabbing fund and round info from query params and validating
     fund_short_name = request.args.get("fund")
     round_short_name = request.args.get("round")
+    govuk_notify_reference = request.args.get("govuk_notify_reference", None)
+    if govuk_notify_reference:
+        try:
+            uuid.UUID(govuk_notify_reference)  # Let's only accept UUID references
+        except ValueError:
+            govuk_notify_reference = None
+
     fund = FundMethods.get_fund(fund_short_name)
     round = get_round_data(fund_short_name, round_short_name)
 
@@ -138,6 +147,7 @@ def new():
                 email=form.data.get("email"),
                 fund_short_name=fund_short_name,
                 round_short_name=round_short_name,
+                govuk_notify_reference=govuk_notify_reference,
             )
 
             if Config.AUTO_REDIRECT_LOGIN:

models/account.py

@@ -183,6 +183,7 @@ def get_magic_link(
         email: str,
         fund_short_name: str = None,
         round_short_name: str = None,
+        govuk_notify_reference: str = None,
     ) -> str:
         """
         Create a new magic link for a user
@@ -227,6 +228,7 @@ def get_magic_link(
                 NotifyConstants.TEMPLATE_TYPE_MAGIC_LINK,
                 account.email,
                 notification_content,
+                govuk_notify_reference=govuk_notify_reference,
             )
 
             return new_link_json.get("link")

models/magic_link.py

@@ -11,9 +11,7 @@
 from urllib.parse import urljoin
 
 from config import Config
-from flask import abort
 from flask import current_app
-from flask import Response
 from flask_redis import FlaskRedis
 from security.utils import create_token
 
@@ -45,16 +43,6 @@ def __init__(self, message="Sorry, there was a problem, please try later"):
 
 
 class MagicLinkMethods(object):
-    def search(self):
-        """
-        GET /magic-links endpoint
-        :return: Json Response
-        """
-        if not Config.FLASK_ENV == "production":
-            return Response(json.dumps(self.links), mimetype="application/json")
-        else:
-            return abort(403)
-
     @property
     def redis_mlinks(self) -> FlaskRedis:
         """
@@ -103,8 +91,7 @@ def _make_link_json(account: Account, redirect_url: str):
                 (
                     datetime.now()
                     + timedelta(
-                        days=Config.MAGIC_LINK_EXPIRY_DAYS,
-                        minutes=1,
+                        seconds=Config.MAGIC_LINK_EXPIRY_SECONDS + 60,
                     )
                 ).timestamp()
             ),
@@ -131,6 +118,7 @@ def _set_unique_keyed_record(self, value: str, prefix=None, max_tries: int = 6)
             )
             if created:
                 return prefixed_key, unique_key
+        return None
 
     @staticmethod
     def get_user_record_key(account_id: str):
@@ -189,7 +177,6 @@ def create_magic_link(
         :return:
         """
         current_app.logger.info(f"Creating magic link for {account}")
-        self.clear_existing_user_record(account.id)
 
         if not redirect_url:
             redirect_url = urljoin(

models/notification.py

@@ -16,7 +16,7 @@ class Notification(object):
     """
 
     @staticmethod
-    def send(template_type: str, to_email: str, content: dict):
+    def send(template_type: str, to_email: str, content: dict, govuk_notify_reference: str | None = None):
         """
         Sends a notification using the Gov.UK Notify Service
 
@@ -46,6 +46,9 @@ def send(template_type: str, to_email: str, content: dict):
             NotifyConstants.FIELD_TO: to_email,
             NotifyConstants.FIELD_CONTENT: content,
         }
+        if govuk_notify_reference:
+            params["govuk_notify_reference"] = govuk_notify_reference
+
         try:
             sqs_extended_client = Notification._get_sqs_client()
             message_id = sqs_extended_client.submit_single_message(

openapi/api.yml

@@ -11,23 +11,6 @@ tags:
   - name: sessions
     description: Session operations
 paths:
-  /magic-links:
-    get:
-      tags:
-        - magic links
-      summary: Search magic link
-      description: List all magic links
-      operationId: api.MagicLinksView.search
-      responses:
-        200:
-          description: SUCCESS - A list of magic link keys
-          content:
-            application/json:
-              schema:
-                type: array
-                items:
-                  type: string
-
   '/magic-links/{link_id}':
     get:
       tags:

tests/test_account.py

@@ -108,7 +108,20 @@ def test_create_magic_link(self, mocker, mock_get_account, flask_test_client, mo
             ),
         )
         mocker.patch("models.account.get_round_data", return_value=Round(contact_email="asdf@asdf.com"))
-        mocker.patch("models.account.Notification.send", return_value=True)
+        mock_send_notification = mocker.patch("models.account.Notification.send", return_value=True)
 
-        result = AccountMethods.get_magic_link(email=test_user_email, fund_short_name="COF", round_short_name="R1W1")
+        result = AccountMethods.get_magic_link(
+            email=test_user_email,
+            fund_short_name="COF",
+            round_short_name="R1W1",
+            govuk_notify_reference="1f829816-b7e5-4cf7-bbbb-1b062e5ee399",
+        )
         assert result.endswith("?fund=COF&round=R1W1")
+        assert mock_send_notification.call_args_list == [
+            mocker.call(
+                "MAGIC_LINK",
+                "john@example.com",
+                mocker.ANY,
+                govuk_notify_reference="1f829816-b7e5-4cf7-bbbb-1b062e5ee399",
+            )
+        ]

tests/test_magic_links.py

@@ -9,7 +9,6 @@
 from api.session.auth_session import AuthSessionView
 from app import app
 from bs4 import BeautifulSoup
-from config import Config
 from frontend.magic_links.forms import EmailForm
 from models.account import AccountMethods
 from security.utils import validate_token
@@ -219,30 +218,6 @@ def test_invalid_magic_link_returns_link_expired(self, flask_test_client):
         assert b"Link expired" in response.data
         assert b"Request a new link" in response.data
 
-    @mock.patch.object(Config, "FLASK_ENV", "production")
-    def test_search_magic_link_forbidden_on_production(self, flask_test_client):
-        use_endpoint = "/magic-links"
-        response = flask_test_client.get(use_endpoint, follow_redirects=True)
-        assert response.status_code == 403
-
-    def test_search_magic_link_returns_magic_links(self, flask_test_client):
-        with mock.patch("models.fund.FundMethods.get_fund"), mock.patch("models.account.get_round_data"), mock.patch(
-            "frontend.magic_links.routes.get_round_data"
-        ):
-            payload = {
-                "email": "new_user@example.com",
-                "redirectUrl": "https://example.com/redirect-url",
-            }
-            endpoint = "/service/magic-links/new?fund=cof&round=r2w3"
-
-            flask_test_client.post(endpoint, data=payload)
-
-        endpoint = "/magic-links"
-        get_response = flask_test_client.get(endpoint)
-        assert get_response.status_code == 200
-        assert "account:usernew" in get_response.get_json()
-        assert next(x for x in get_response.get_json() if x.startswith("link:")) is not None
-
     def test_assessor_roles_is_empty_via_magic_link_auth(self):
         """
         GIVEN we are on the production environment
@@ -306,5 +281,41 @@ def test_magic_link_route_new(self, flask_test_client):
                         email="example@email.com",
                         fund_short_name="COF",
                         round_short_name="R2W3",
+                        govuk_notify_reference=None,
+                    )
+                    assert response.status_code == 200
+
+    def test_magic_link_route_new_with_notify_reference(self, flask_test_client):
+        # create a MagicMock object for the form used in new():
+        mock_form = mock.MagicMock(spec=EmailForm)
+        mock_form.validate_on_submit.return_value = True
+        mock_form.data = {"email": "example@email.com"}
+
+        # mock get_magic_link() used in new():
+        mock_account = mock.MagicMock(spec=AccountMethods)
+        mock_account.get_magic_link.return_value = True
+
+        # Test post request with fund and round short names:
+        with mock.patch("frontend.magic_links.routes.EmailForm", return_value=mock_form):
+            with mock.patch(
+                "frontend.magic_links.routes.AccountMethods",
+                return_value=mock_account,
+            ):
+                with mock.patch(
+                    "frontend.magic_links.routes.get_round_data",
+                    return_value=mock_account,
+                ):
+                    response = flask_test_client.post(
+                        "service/magic-links/new"
+                        "?fund=COF&round=R2W3&govuk_notify_reference=1f829816-b7e5-4cf7-bbbb-1b062e5ee399",
+                        follow_redirects=True,
+                    )
+
+                    # Assert get_magic_link() was called with short_names:
+                    frontend.magic_links.routes.AccountMethods.get_magic_link.assert_called_once_with(  # noqa
+                        email="example@email.com",
+                        fund_short_name="COF",
+                        round_short_name="R2W3",
+                        govuk_notify_reference="1f829816-b7e5-4cf7-bbbb-1b062e5ee399",
                     )
                     assert response.status_code == 200

tests/test_notification.py

@@ -59,7 +59,9 @@ def test_notification_send_success(app_context, monkeypatch, enable_notification
         sqs_extended_client.s3_client = s3_connection
         Config.AWS_SQS_NOTIF_APP_PRIMARY_QUEUE_URL = queue_response["QueueUrl"]
         mock_get_sqs_client.return_value = sqs_extended_client
-        result = Notification.send(template_type, to_email, content)
+        result = Notification.send(
+            template_type, to_email, content, govuk_notify_reference="1f829816-b7e5-4cf7-bbbb-1b062e5ee399"
+        )
         assert result is True