Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 43 additions & 11 deletions app/config.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import os
from urllib.parse import urlparse


class Config(object):
# if we're not on cloudfoundry, we can get to this app from localhost. but on cloudfoundry its different
class Config:
ADMIN_CLIENT_SECRET = os.environ.get("ADMIN_CLIENT_SECRET")
ADMIN_CLIENT_USER_NAME = "notify-admin"
SECRET_KEY = os.environ.get("SECRET_KEY")
Expand All @@ -16,7 +16,8 @@ class Config(object):
DEBUG = False

DOCUMENT_DOWNLOAD_API_HOST_NAME = os.environ.get("DOCUMENT_DOWNLOAD_API_HOST_NAME")
DOCUMENT_DOWNLOAD_API_HOST_NAME_INTERNAL = os.environ.get("DOCUMENT_DOWNLOAD_API_HOST_NAME_INTERNAL")
DOCUMENT_DOWNLOAD_API_HOST_NAME_INTERNAL = os.environ.get("DOCUMENT_DOWNLOAD_API_HOST_NAME_INTERNAL", "")
DOCUMENT_DOWNLOAD_FRONTEND_HOST_NAME = os.environ.get("DOCUMENT_DOWNLOAD_FRONTEND_HOST_NAME", "")

HEADER_COLOUR = "#FFBF47" # $yellow
HTTP_PROTOCOL = "http"
Expand All @@ -26,14 +27,44 @@ class Config(object):
# needs to refer to notify for utils
NOTIFY_LOG_PATH = os.getenv("NOTIFY_LOG_PATH")

@property
def COOKIE_DOMAIN(self):
# the cookie is set by the frontend, and read by the api. setting the domain attribute allows us to specify
# a domain (settable to the current domain or any superdomain) - and importantly all pages on subdomains of
# the cookie's domain can access that cookie.
# docs: https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#domain_attribute
api_domain = urlparse(self.DOCUMENT_DOWNLOAD_API_HOST_NAME).netloc
fe_domain = urlparse(self.DOCUMENT_DOWNLOAD_FRONTEND_HOST_NAME).netloc

shared_domain = []
for api_path, fe_path in zip(reversed(api_domain.split(".")), reversed(fe_domain.split("."))):
if api_path != fe_path:
break
shared_domain.append(api_path)

# shared_domain must be at least a website with a TLD `documents.service.gov.uk` or `notify.localhost`
if len(shared_domain) < 2:
raise ValueError(
f"{self.DOCUMENT_DOWNLOAD_API_HOST_NAME=} and {self.DOCUMENT_DOWNLOAD_FRONTEND_HOST_NAME=} "
"must have a valid shared domain to set cookie on"
)

return ".".join(reversed(shared_domain))


class Development(Config):
SERVER_NAME = os.getenv("SERVER_NAME")
SERVER_NAME = os.getenv("DOCUMENT_DOWNLOAD_FRONTEND_HOST_NAME")

API_HOST_NAME = os.environ.get("API_HOST_NAME", "http://localhost:6011")

# we dont need to distinguish between internal and external when running locally/in docker
DOCUMENT_DOWNLOAD_API_HOST_NAME = os.environ.get("DOCUMENT_DOWNLOAD_API_HOST_NAME", "http://localhost:7000")
DOCUMENT_DOWNLOAD_API_HOST_NAME_INTERNAL = os.environ.get(
"DOCUMENT_DOWNLOAD_API_HOST_NAME", "http://localhost:7000"
)
DOCUMENT_DOWNLOAD_FRONTEND_HOST_NAME = os.environ.get(
"DOCUMENT_DOWNLOAD_FRONTEND_HOST_NAME", "https://localhost:7001"
)

ADMIN_CLIENT_SECRET = "dev-notify-secret-key"
SECRET_KEY = "dev-notify-secret-key"
Expand All @@ -50,8 +81,9 @@ class Test(Development):
SERVER_NAME = "document-download-frontend.gov"

API_HOST_NAME = "http://test-notify-api"
DOCUMENT_DOWNLOAD_API_HOST_NAME = "https://download.test-doc-download-api.gov.uk"
DOCUMENT_DOWNLOAD_API_HOST_NAME_INTERNAL = "https://download.test-doc-download-api-internal.gov.uk"
DOCUMENT_DOWNLOAD_API_HOST_NAME = "https://api.test-doc-download-api.gov.uk"
DOCUMENT_DOWNLOAD_API_HOST_NAME_INTERNAL = "https://api-internal.test-doc-download-api-internal.gov.uk"
DOCUMENT_DOWNLOAD_FRONTEND_HOST_NAME = "https://frontend.test-doc-download-api.gov.uk"


class Preview(Config):
Expand All @@ -70,9 +102,9 @@ class Production(Config):


configs = {
"development": Development,
"test": Test,
"preview": Preview,
"staging": Staging,
"production": Production,
"development": Development(),
"test": Test(),
"preview": Preview(),
"staging": Staging(),
"production": Production(),
}
8 changes: 1 addition & 7 deletions app/main/views/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,9 @@ def confirm_email_address(service_id, document_id):
"path": authentication_data["cookie_path"],
"secure": current_app.config["HTTP_PROTOCOL"] == "https",
"httponly": True,
"domain": current_app.config["COOKIE_DOMAIN"],
}

# the cookie is set by the frontend, and read by the api
# this works fine when they're both under the same domain
# but when we're in a subdomain, we need to allow the cookie to be sent to subdomains too
# docs: https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#domain_attribute
cookie_domain = current_app.config["DOCUMENT_DOWNLOAD_API_HOST_NAME"].replace("https://download.", "")
set_cookie_values["domain"] = cookie_domain

response = redirect(url_for(".download_document", service_id=service_id, document_id=document_id, key=key))
response.set_cookie(**set_cookie_values)
return response
Expand Down
1 change: 1 addition & 0 deletions manifest.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ applications:
FLASK_APP: application.py
DOCUMENT_DOWNLOAD_API_HOST_NAME: https://download.{{ hostname }}
DOCUMENT_DOWNLOAD_API_HOST_NAME_INTERNAL: https://download.{{ hostname }}
DOCUMENT_DOWNLOAD_FRONTEND_HOST_NAME: {{ hostname }}

NOTIFY_ENVIRONMENT: {{ environment }}
AWS_ACCESS_KEY_ID: {{ DOCUMENT_DOWNLOAD_AWS_ACCESS_KEY_ID }}
Expand Down
2 changes: 1 addition & 1 deletion tests/app/main/views/test_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def test_services_view_redirects_to_api(client, url):
response = client.get(url)

assert response.status_code == 301
assert response.location == f"https://download.test-doc-download-api.gov.uk{url}"
assert response.location == f"https://api.test-doc-download-api.gov.uk{url}"


@pytest.mark.parametrize(
Expand Down
32 changes: 32 additions & 0 deletions tests/app/test_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import pytest
from flask import Flask

from app.config import Config


@pytest.mark.parametrize(
["api_host_name", "frontend_host_name", "expected_cookie_domain"],
[
# prod
("https://documents.download.service.gov.uk", "https://download.service.gov.uk", "download.service.gov.uk"),
# notifications-local docker-compose
(
"http://api.document-download.localhost",
"http://frontend.document-download.localhost",
"document-download.localhost",
),
# running locally outside of docker is no longer supported? :thinking_face:
pytest.param(
"http://localhost:7001", "http://localhost:7002", None, marks=pytest.mark.xfail(raises=ValueError)
Copy link
Contributor Author

@leohemsted leohemsted Jan 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i suspect that the cookies never worked previously if you ran natively on your laptop (rather than under document-download.localhost a la docker compose), but it only happened at cookie-setting-time - which is only if you have confirm emails set to true.

),
],
)
def test_cookie_domain_set_correctly(api_host_name, frontend_host_name, expected_cookie_domain):
conf = Config()
conf.DOCUMENT_DOWNLOAD_API_HOST_NAME = api_host_name
conf.DOCUMENT_DOWNLOAD_FRONTEND_HOST_NAME = frontend_host_name

app = Flask("app")
app.config.from_object(conf)

assert app.config["COOKIE_DOMAIN"] == expected_cookie_domain