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

Fix/security vulnerability #5692

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
14 changes: 8 additions & 6 deletions datahub/dnb_api/test/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -604,18 +604,18 @@ def test_post_invalid(
(
[],
400,
'Cannot find a company with duns_number: 123456789',
'The request could not be completed due to an issue with the provided data.',
),
(
['foo', 'bar'],
502,
'Multiple companies found with duns_number: 123456789',
'An error occurred while processing your request. Please try again later.',
),
(
[{'duns_number': '012345678'}],
502,
'DUNS number of the company: 012345678 '
'did not match searched DUNS number: 123456789',
'An error occurred while processing your request. '
'Please try again later.',
),
),
)
Expand Down Expand Up @@ -1042,7 +1042,8 @@ def test_already_linked(self):
)
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json() == {
'detail': f'Company {str(company.id)} is already linked ' 'with duns number 123456789',
'detail':
'The request could not be completed due to a problem with the provided data.',
}

def test_duplicate_duns_number(self):
Expand Down Expand Up @@ -1087,7 +1088,8 @@ def test_company_not_found(
)
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json() == {
'detail': 'Cannot find a company with duns_number: 123456789',
'detail':
'The request could not be completed due to a problem with the provided data.',
}

def test_valid(
Expand Down
7 changes: 5 additions & 2 deletions datahub/dnb_api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1011,8 +1011,11 @@
DNBServiceConnectionError,
DNBServiceTimeoutError,
DNBServiceError,
) as exc:
raise APIUpstreamException(str(exc))
):
logger.error(

Check warning on line 1015 in datahub/dnb_api/utils.py

View check run for this annotation

Codecov / codecov/patch

datahub/dnb_api/utils.py#L1015

Added line #L1015 was not covered by tests
'An error occurred while retrieving company hierarchy data.')
raise APIUpstreamException(

Check warning on line 1017 in datahub/dnb_api/utils.py

View check run for this annotation

Codecov / codecov/patch

datahub/dnb_api/utils.py#L1017

Added line #L1017 was not covered by tests
'An error occurred while retrieving company data. Please try again later.')
family_tree_members = response.data
if not family_tree_members:
return json_response
Expand Down
71 changes: 48 additions & 23 deletions datahub/dnb_api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,10 @@
except (
DNBServiceConnectionError,
DNBServiceTimeoutError,
) as exc:
raise APIUpstreamException(str(exc))
):
logger.error('Error communicating with DNB service during company search.')
raise APIUpstreamException(

Check warning on line 95 in datahub/dnb_api/views.py

View check run for this annotation

Codecov / codecov/patch

datahub/dnb_api/views.py#L94-L95

Added lines #L94 - L95 were not covered by tests
'An error occurred while processing your request. Please try again later.')
except DNBServiceError as exc:
return HttpResponse(
exc.message,
Expand Down Expand Up @@ -198,11 +200,16 @@
DNBServiceConnectionError,
DNBServiceError,
DNBServiceInvalidResponseError,
) as exc:
raise APIUpstreamException(str(exc))

except DNBServiceInvalidRequestError as exc:
raise APIBadRequestException(str(exc))
):
logger.error(
'An error occurred while retrieving data for DUNS number')
raise APIUpstreamException(
'An error occurred while processing your request. Please try again later.')
except DNBServiceInvalidRequestError:
logger.warning(
'Invalid request for DUNS number')
raise APIBadRequestException(
'The request could not be completed due to an issue with the provided data.')

company_serializer = DNBCompanySerializer(
data=dnb_company,
Expand Down Expand Up @@ -265,14 +272,21 @@
DNBServiceConnectionError,
DNBServiceInvalidResponseError,
DNBServiceError,
) as exc:
raise APIUpstreamException(str(exc))
):
logger.error(
'An error occurred while linking company ID with DUNS number',
)
raise APIUpstreamException(
'An error occurred while processing your request. Please try again later.')

except (
DNBServiceInvalidRequestError,
CompanyAlreadyDNBLinkedError,
) as exc:
raise APIBadRequestException(str(exc))
):
logger.warning(
'Invalid request or company ID is already linked to DUNS number')
raise APIBadRequestException(
'The request could not be completed due to a problem with the provided data.')

return Response(
CompanySerializer().to_representation(company),
Expand Down Expand Up @@ -306,8 +320,10 @@
DNBServiceConnectionError,
DNBServiceTimeoutError,
DNBServiceError,
) as exc:
raise APIUpstreamException(str(exc))
):
logger.error('An error occurred while processing DNB companies POST request.')
raise APIUpstreamException(
'An error occurred while processing your request. Please try again later.')

return Response(response)

Expand All @@ -331,9 +347,11 @@
DNBServiceConnectionError,
DNBServiceTimeoutError,
DNBServiceError,
) as exc:
raise APIUpstreamException(str(exc))

):
logger.error(
'An error occurred while processing GET request for DUNS number')
raise APIUpstreamException(
'An error occurred while processing your request. Please try again later.')
return Response(response)


Expand Down Expand Up @@ -367,9 +385,10 @@
DNBServiceConnectionError,
DNBServiceTimeoutError,
DNBServiceError,
) as exc:
raise APIUpstreamException(str(exc))

):
logger.error('An error occurred while creating an investigation for the company.')
raise APIUpstreamException(
'An error occurred while processing your request. Please try again later.')
company.dnb_investigation_id = response['id']
company.pending_dnb_investigation = True
company.save()
Expand Down Expand Up @@ -400,8 +419,11 @@
DNBServiceError,
DNBServiceInvalidRequestError,
DNBServiceInvalidResponseError,
) as exc:
raise APIUpstreamException(str(exc))
):
logger.error(
'Failed to retrieve company hierarchy count for DUNS number from dnb-service')
raise APIUpstreamException(
'An error occurred while processing your request. Please try again later.')

manually_verified = self.get_manually_verified_subsidiaries(
company_id,
Expand Down Expand Up @@ -524,8 +546,11 @@
DNBServiceConnectionError,
DNBServiceTimeoutError,
DNBServiceError,
) as exc:
raise APIUpstreamException(str(exc))
):
logger.error(

Check warning on line 550 in datahub/dnb_api/views.py

View check run for this annotation

Codecov / codecov/patch

datahub/dnb_api/views.py#L550

Added line #L550 was not covered by tests
'Failed to retrieve company hierarchy count for DUNS number')
raise APIUpstreamException(

Check warning on line 552 in datahub/dnb_api/views.py

View check run for this annotation

Codecov / codecov/patch

datahub/dnb_api/views.py#L552

Added line #L552 was not covered by tests
'An error occurred while processing your request. Please try again later.')

json_response = {
'related_companies_count': companies_count,
Expand Down
8 changes: 6 additions & 2 deletions datahub/omis/payment/govukpay.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,12 @@ def _raise_for_status(self, response):
"""
try:
response.raise_for_status()
except requests.HTTPError as exc:
raise GOVUKPayAPIException(exc) from exc
except requests.HTTPError:

logger.error(f'HTTPError occurred with status code: {response.status_code}')
# Raise a generic exception for the user with less information
raise GOVUKPayAPIException(
'An error occurred while processing your request. Please try again later.')

def _request(self, method, path, **kwargs):
"""
Expand Down
14 changes: 10 additions & 4 deletions datahub/omis/payment/test/test_govukpay.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ def test_http_error(self, status_code, error_msg, requests_mock):
description='payment description',
return_url='http://return.example.com',
)
assert exc.value.detail == f'{error_msg}: {error_msg} for url: {url}'
# Check that the exception contains the generic message
assert exc.value.detail == (
'An error occurred while processing your request. Please try again later.')


class TestPayClientGetPaymentById:
Expand Down Expand Up @@ -126,7 +128,9 @@ def test_http_error(self, status_code, error_msg, requests_mock):
with pytest.raises(GOVUKPayAPIException) as exc:
pay = PayClient()
pay.get_payment_by_id(payment_id)
assert exc.value.detail == f'{error_msg}: {error_msg} for url: {url}'
# Check that the exception contains the generic message
assert exc.value.detail == (
'An error occurred while processing your request. Please try again later.')


class TestPayClientCancelPayment:
Expand Down Expand Up @@ -158,12 +162,14 @@ def test_ok(self, requests_mock):
),
)
def test_http_error(self, status_code, error_msg, requests_mock):
"""Test that if GOV.UK Pay returns an HTTP error, an exception is raised."""
"""Test that if GOV.UK Pay returns an HTTP error, a generic exception is raised."""
payment_id = '123abc123abc123abc123abc12'
url = govuk_url(f'payments/{payment_id}/cancel')
requests_mock.post(url, status_code=status_code, reason=error_msg)

with pytest.raises(GOVUKPayAPIException) as exc:
pay = PayClient()
pay.cancel_payment(payment_id)
assert exc.value.detail == f'{error_msg}: {error_msg} for url: {url}'
# Check that the exception contains the generic message
assert exc.value.detail == (
'An error occurred while processing your request. Please try again later.')