Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Production release #2266

Merged
merged 30 commits into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
5e83c73
add url_has_allowed_host_and_scheme checking
markj0hnst0n Nov 29, 2024
826644e
fix urls
markj0hnst0n Dec 2, 2024
1c68372
add tests
markj0hnst0n Dec 2, 2024
9ea63d8
test update
markj0hnst0n Dec 2, 2024
f2d3894
remove comments
markj0hnst0n Dec 2, 2024
5673f6c
coverage check
markj0hnst0n Dec 2, 2024
94f14c4
revert
markj0hnst0n Dec 2, 2024
1376ad1
change approach
markj0hnst0n Dec 2, 2024
21d5f57
reverse netloc and scheme results
markj0hnst0n Dec 2, 2024
9e55697
adjust variable and add tests
markj0hnst0n Dec 2, 2024
a9255bd
test amendment
markj0hnst0n Dec 2, 2024
7be1fa4
move tests
markj0hnst0n Dec 2, 2024
5fc5ffc
add tests
markj0hnst0n Dec 2, 2024
7b1a3bc
test refactor
markj0hnst0n Dec 2, 2024
26f72d4
add test
markj0hnst0n Dec 2, 2024
bb96e17
test update
markj0hnst0n Dec 3, 2024
e554877
test remove submission
markj0hnst0n Dec 3, 2024
315c7eb
update
markj0hnst0n Dec 3, 2024
eb096b6
remove unused
markj0hnst0n Dec 3, 2024
2ad69f2
revert and remove commented code
markj0hnst0n Dec 3, 2024
b60fed1
amend flow to use sanitised_path var
markj0hnst0n Dec 3, 2024
9f79e0a
remove bespoke exception and just use SuspiciousOperation
markj0hnst0n Dec 4, 2024
4f3bb5d
change approach
markj0hnst0n Dec 4, 2024
25b759c
tidy and add test data
markj0hnst0n Dec 4, 2024
6c645e2
add bespoke error for easier tracking
markj0hnst0n Dec 4, 2024
aac8874
fix validation
depsiatwal Dec 4, 2024
c665057
Merge pull request #2258 from uktrade/LTD-5692-fix-crn-validation
depsiatwal Dec 4, 2024
f1a3344
Merge branch 'dev' into LTD-5695-fix-URL-redirection-from-remote-sour…
markj0hnst0n Dec 5, 2024
df66c55
Merge pull request #2250 from uktrade/LTD-5695-fix-URL-redirection-fr…
markj0hnst0n Dec 5, 2024
b98dd31
Merge pull request #2262 from uktrade/dev
currycoder Dec 6, 2024
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
12 changes: 7 additions & 5 deletions core/helpers.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
from urllib.parse import urlencode
from django.http import (
Http404,
StreamingHttpResponse,
)
from django.http import StreamingHttpResponse
from django.core.exceptions import SuspiciousOperation
from django.utils.http import url_has_allowed_host_and_scheme


class UnsafeURLDestination(SuspiciousOperation):
pass


def dummy_quote(string, safe="", encoding=None, errors=None):
return string

Expand Down Expand Up @@ -59,7 +61,7 @@ def check_url(request, url):
if url_is_safe:
return url
else:
raise Http404
raise UnsafeURLDestination(f"URL destination '{url}' was deemed to be unsafe")


def stream_document_response(api_response):
Expand Down
2 changes: 1 addition & 1 deletion exporter/core/organisation/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class Validation:
UK_VAT_VALIDATION_REGEX = r"^(GB|XI)?([0-9]{9}([0-9]{3})?|(GD|HA)[0-9]{3})$"
UK_EORI_STARTING_LETTERS_REGEX = r"^(GB|XI)"
SIC_NUMBERs_ONLY_REGEX = r"^\d+$"
REGISTRATION_NUM_VALIDATION_REGEX = r"^(RC\d+|\d+)$"
REGISTRATION_NUM_VALIDATION_REGEX = r"^((RC|NI)\d{6}|\d{8})$"

ADDRESS_LINE_MAX_LENGTH = 35
UK_EORI_MAX_LENGTH = 17
Expand Down
12 changes: 7 additions & 5 deletions lite_forms/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
validate_data_unknown,
)
from lite_forms.submitters import submit_paged_form
from core.helpers import check_url


ACTION = "_action"
VALIDATE_ONLY = "validate_only"
Expand Down Expand Up @@ -188,9 +190,6 @@
self.init(request, **kwargs)
submission = self.on_submission(request, **kwargs) # noqa

if submission:
return redirect(submission)

response, data = submit_paged_form(
request,
self.get_forms(),
Expand Down Expand Up @@ -340,7 +339,8 @@
if self.validate_only_until_final_submission:
return self.generate_summary_list()

return redirect(request.path)
sanitised_url = check_url(request, request.path)
return redirect(sanitised_url)
Dismissed Show dismissed Hide dismissed

def post(self, request, **kwargs):
return self._post(request, **kwargs)
Expand Down Expand Up @@ -403,7 +403,9 @@
if self.validate_only_until_final_submission:
return self.generate_summary_list()

return redirect(request.path)
sanitised_url = check_url(request, request.path)

return redirect(sanitised_url)
Dismissed Show dismissed Hide dismissed

def dispatch(self, request, *args, **kwargs):
if request.method.lower() in self.http_method_names:
Expand Down
9 changes: 4 additions & 5 deletions unit_tests/core/test_helpers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import pytest
from django.http import Http404
from core.helpers import convert_parameters_to_query_params, check_url
from core.helpers import convert_parameters_to_query_params, check_url, UnsafeURLDestination


@pytest.fixture
Expand All @@ -15,13 +14,13 @@ def test_convert_parameters_to_query_params():
assert convert_parameters_to_query_params(params) == "?org_type=individual&org_type=commercial&page=1"


@pytest.mark.parametrize("url", ["/next-url/", "http://testserver/next-url/"])
@pytest.mark.parametrize("url", ["/next-url/", "http://testserver/next-url/", "\\valid\path"])
def test_check_url(mock_request, url):
assert check_url(mock_request, url) == url


# Check not valid host and non secure url
@pytest.mark.parametrize("url", ["http://not-the-same.com/next/", "https://test-server/next/"])
@pytest.mark.parametrize("url", ["http://not-the-same.com/next/", "https://test-server/next/", "https:/malicious.com"])
def test_check_url_not_allowed(mock_request, url):
with pytest.raises(Http404):
with pytest.raises(UnsafeURLDestination):
check_url(mock_request, url)
25 changes: 25 additions & 0 deletions unit_tests/exporter/applications/views/test_end_use_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,28 @@ def test_application_end_use_summary(
response.context["instruction_text"]
== "Review your answers below and make any amends you need to. Click 'Save and continue' to save your progress."
)


def test_application_end_use_summary_post_url_has_allowed_host_and_scheme(
authorized_client, mock_application_get, application_end_use_summary_url, application_task_list_url
):
response = authorized_client.post(
application_end_use_summary_url,
data={
"_action": "submit",
},
)
assert response.status_code == 302


def test_application_end_use_summary_get_next_form_url_has_allowed_host_and_scheme(
authorized_client, mock_application_get, application_end_use_summary_url, application_task_list_url
):
response = authorized_client.post(
application_end_use_summary_url,
data={
"_action": "finish",
"form_pk": "1",
},
)
assert response.status_code == 302
63 changes: 53 additions & 10 deletions unit_tests/exporter/core/organisation/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,10 @@ def test_register_details_form_required_fields(
"Enter a SIC code that is 5 numbers long, like 12345",
"SIC code can only include numbers",
],
"registration_number": ["The CRN or RC number is too long"],
"registration_number": [
"The CRN or RC number is too long",
"Enter a CRN or RC number in the correct format",
],
},
forms.RegisterDetailsCommercialOverseasForm,
),
Expand All @@ -177,7 +180,10 @@ def test_register_details_form_required_fields(
{
"sic_number": ["Enter a SIC code that is 5 numbers long, like 12345"],
"vat_number": ["UK VAT number is too long", "Enter a UK VAT number in the correct format"],
"registration_number": ["The CRN or RC number is too long"],
"registration_number": [
"The CRN or RC number is too long",
"Enter a CRN or RC number in the correct format",
],
},
forms.RegisterDetailsCommercialUKForm,
),
Expand All @@ -200,7 +206,10 @@ def test_register_details_form_required_fields(
"UK VAT number can only include numbers and letters",
"Enter a UK VAT number in the correct format",
],
"registration_number": ["The CRN or RC number is too long"],
"registration_number": [
"The CRN or RC number is too long",
"Enter a CRN or RC number in the correct format",
],
},
forms.RegisterDetailsCommercialUKForm,
),
Expand All @@ -223,7 +232,10 @@ def test_register_details_form_required_fields(
"UK VAT number can only include numbers and letters",
"Enter a UK VAT number in the correct format",
],
"registration_number": ["The CRN or RC number is too long"],
"registration_number": [
"The CRN or RC number is too long",
"Enter a CRN or RC number in the correct format",
],
},
forms.RegisterDetailsCommercialUKForm,
),
Expand Down Expand Up @@ -311,7 +323,10 @@ def test_register_details_form_field_validation(
},
False,
{
"registration_number": ["The CRN or RC number is too long"],
"registration_number": [
"The CRN or RC number is too long",
"Enter a CRN or RC number in the correct format",
],
},
),
(
Expand All @@ -320,7 +335,10 @@ def test_register_details_form_field_validation(
},
False,
{
"registration_number": ["The CRN or RC number is too short"],
"registration_number": [
"The CRN or RC number is too short",
"Enter a CRN or RC number in the correct format",
],
},
),
(
Expand Down Expand Up @@ -350,7 +368,7 @@ def test_register_details_form_field_validation(
),
(
{
"registration_number": "FF123456",
"registration_number": "123456RC",
},
False,
{
Expand All @@ -359,7 +377,7 @@ def test_register_details_form_field_validation(
),
(
{
"registration_number": "123456RC",
"registration_number": "ABC12345",
},
False,
{
Expand All @@ -368,14 +386,39 @@ def test_register_details_form_field_validation(
),
(
{
"registration_number": "RC123456",
"registration_number": "A1234567",
},
False,
{
"registration_number": ["Enter a CRN or RC number in the correct format"],
},
),
(
{
"registration_number": "SO123456",
},
False,
{
"registration_number": ["Enter a CRN or RC number in the correct format"],
},
),
(
{
"registration_number": "12345678",
},
True,
None,
),
(
{
"registration_number": "12345678",
"registration_number": "NI123456",
},
True,
None,
),
(
{
"registration_number": "RC123456",
},
True,
None,
Expand Down
Loading