From c1b08e4cbc24fc0348c3019d1231bbb07036abb6 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 29 Apr 2021 18:48:59 +0100 Subject: [PATCH] make sure all non-uuid service ids 403 in api keys previously 'invalid-strings' would be handled, but integers would just return 500. --- app/authentication/auth.py | 12 ++++++++---- tests/app/authentication/test_authentication.py | 7 ++++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/app/authentication/auth.py b/app/authentication/auth.py index c9efab976..b215d6b90 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -1,3 +1,5 @@ +import uuid + from flask import _request_ctx_stack, current_app, g, request from gds_metrics import Histogram from notifications_python_client.authentication import ( @@ -12,7 +14,6 @@ from notifications_python_client.errors import ( TokenIssuerError, ) from notifications_utils import request_helper -from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound from app.serialised_models import SerialisedService @@ -99,10 +100,13 @@ def requires_auth(): issuer = __get_token_issuer(auth_token) # ie the `iss` claim which should be a service ID try: - with AUTH_DB_CONNECTION_DURATION_SECONDS.time(): - service = SerialisedService.from_id(issuer) - except DataError: + service_id = uuid.UUID(issuer) + except Exception: raise AuthError("Invalid token: service id is not the right data type", 403) + + try: + with AUTH_DB_CONNECTION_DURATION_SECONDS.time(): + service = SerialisedService.from_id(service_id) except NoResultFound: raise AuthError("Invalid token: service not found", 403) diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index c4b9de346..e675fccf3 100644 --- a/tests/app/authentication/test_authentication.py +++ b/tests/app/authentication/test_authentication.py @@ -204,9 +204,10 @@ def test_should_allow_valid_token(client, sample_api_key, scheme): assert response.status_code == 200 -def test_should_not_allow_service_id_that_is_not_the_wrong_data_type(client, sample_api_key): +@pytest.mark.parametrize('service_id', ['not-a-valid-id', 1234]) +def test_should_not_allow_service_id_that_is_not_the_wrong_data_type(client, sample_api_key, service_id): token = create_jwt_token(secret=get_unsigned_secrets(sample_api_key.service_id)[0], - client_id=str('not-a-valid-id')) + client_id=service_id) response = client.get( '/notifications', headers={'Authorization': "Bearer {}".format(token)} @@ -491,5 +492,5 @@ def test_should_cache_service_and_api_key_lookups(mocker, client, sample_api_key call(str(sample_api_key.service_id)) ] assert mock_get_service.call_args_list == [ - call(str(sample_api_key.service_id)) + call(sample_api_key.service_id) ]