From 5944eb3b15bcb4358b45d24d69c6c77100b671f4 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Thu, 9 Jan 2020 16:47:53 +0000 Subject: [PATCH] Bump python client to 5.5.0 to bring in better error messages --- app/authentication/auth.py | 12 +-- requirements-app.txt | 2 +- requirements.txt | 8 +- .../app/authentication/test_authentication.py | 80 ++++++++++++------- 4 files changed, 62 insertions(+), 40 deletions(-) diff --git a/app/authentication/auth.py b/app/authentication/auth.py index 9f564df80..729f2172d 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -36,12 +36,12 @@ class AuthError(Exception): def get_auth_token(req): auth_header = req.headers.get('Authorization', None) if not auth_header: - raise AuthError('Unauthorized, authentication token must be provided', 401) + raise AuthError('Unauthorized: authentication token must be provided', 401) auth_scheme = auth_header[:7].title() if auth_scheme != 'Bearer ': - raise AuthError('Unauthorized, authentication bearer scheme must be used', 401) + raise AuthError('Unauthorized: authentication bearer scheme must be used', 401) return auth_header[7:] @@ -60,7 +60,7 @@ def requires_admin_auth(): g.service_id = current_app.config.get('ADMIN_CLIENT_USER_NAME') return handle_admin_key(auth_token, current_app.config.get('ADMIN_CLIENT_SECRET')) else: - raise AuthError('Unauthorized, admin authentication token required', 401) + raise AuthError('Unauthorized: admin authentication token required', 401) def requires_auth(): @@ -108,7 +108,7 @@ def requires_auth(): return else: # service has API keys, but none matching the one the user provided - raise AuthError("Invalid token: signature, api token not found", 403, service_id=service.id) + raise AuthError("Invalid token: API key not found", 403, service_id=service.id) def __get_token_issuer(auth_token): @@ -117,7 +117,7 @@ def __get_token_issuer(auth_token): except TokenIssuerError: raise AuthError("Invalid token: iss field not provided", 403) except TokenDecodeError: - raise AuthError("Invalid token: signature, api token is not valid", 403) + raise AuthError("Invalid token: API token is not valid", 403) return client @@ -127,4 +127,4 @@ def handle_admin_key(auth_token, secret): except TokenExpiredError: raise AuthError("Invalid token: expired, check that your system clock is accurate", 403) except TokenDecodeError: - raise AuthError("Invalid token: signature, api token is not valid", 403) + raise AuthError("Invalid token: API token is not valid", 403) diff --git a/requirements-app.txt b/requirements-app.txt index d1368dce5..d2b867305 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -20,7 +20,7 @@ psycopg2-binary==2.8.4 PyJWT==1.7.1 SQLAlchemy==1.3.10 -notifications-python-client==5.4.1 +notifications-python-client==5.5.0 # PaaS awscli-cwlogs>=1.4,<1.5 diff --git a/requirements.txt b/requirements.txt index f6d903e30..6a9cda5cb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,7 +22,7 @@ psycopg2-binary==2.8.4 PyJWT==1.7.1 SQLAlchemy==1.3.10 -notifications-python-client==5.4.1 +notifications-python-client==5.5.0 # PaaS awscli-cwlogs>=1.4,<1.5 @@ -40,12 +40,12 @@ alembic==1.3.2 amqp==1.4.9 anyjson==0.3.3 attrs==19.3.0 -awscli==1.16.310 +awscli==1.16.313 bcrypt==3.1.7 billiard==3.3.0.23 bleach==3.1.0 boto3==1.10.38 -botocore==1.13.46 +botocore==1.13.49 certifi==2019.11.28 chardet==3.0.4 Click==7.0 @@ -70,7 +70,7 @@ phonenumbers==8.11.1 pyasn1==0.4.8 pycparser==2.19 PyPDF2==1.26.0 -pyrsistent==0.15.6 +pyrsistent==0.15.7 python-dateutil==2.8.1 python-editor==1.0.4 python-json-logger==0.1.11 diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index 8fae6bdab..29464d01c 100644 --- a/tests/app/authentication/test_authentication.py +++ b/tests/app/authentication/test_authentication.py @@ -22,7 +22,7 @@ def test_should_not_allow_request_with_no_token(client, auth_fn): request.headers = {} with pytest.raises(AuthError) as exc: auth_fn() - assert exc.value.short_message == 'Unauthorized, authentication token must be provided' + assert exc.value.short_message == 'Unauthorized: authentication token must be provided' @pytest.mark.parametrize('auth_fn', [requires_auth, requires_admin_auth]) @@ -30,7 +30,7 @@ def test_should_not_allow_request_with_incorrect_header(client, auth_fn): request.headers = {'Authorization': 'Basic 1234'} with pytest.raises(AuthError) as exc: auth_fn() - assert exc.value.short_message == 'Unauthorized, authentication bearer scheme must be used' + assert exc.value.short_message == 'Unauthorized: authentication bearer scheme must be used' @pytest.mark.parametrize('auth_fn', [requires_auth, requires_admin_auth]) @@ -38,7 +38,7 @@ def test_should_not_allow_request_with_incorrect_token(client, auth_fn): request.headers = {'Authorization': 'Bearer 1234'} with pytest.raises(AuthError) as exc: auth_fn() - assert exc.value.short_message == 'Invalid token: signature, api token is not valid' + assert exc.value.short_message == 'Invalid token: API token is not valid' @pytest.mark.parametrize('auth_fn', [requires_auth, requires_admin_auth]) @@ -80,28 +80,7 @@ def test_auth_should_not_allow_request_with_no_iat(client, sample_api_key): request.headers = {'Authorization': 'Bearer {}'.format(token)} with pytest.raises(AuthError) as exc: requires_auth() - assert exc.value.short_message == 'Invalid token: signature, api token not found' - - -def test_auth_should_not_allow_request_with_non_hs256_algorithm(client, sample_api_key): - iss = str(sample_api_key.service_id) - # code copied from notifications_python_client.authentication.py::create_jwt_token - headers = { - "typ": 'JWT', - "alg": 'HS512' - } - - claims = { - 'iss': iss, - 'iat': int(time.time()) - } - - token = jwt.encode(payload=claims, key=str(uuid.uuid4()), headers=headers).decode() - - request.headers = {'Authorization': 'Bearer {}'.format(token)} - with pytest.raises(AuthError) as exc: - requires_auth() - assert exc.value.short_message == 'Invalid token: algorithm used is not HS256' + assert exc.value.short_message == 'Invalid token: API key not found' def test_admin_auth_should_not_allow_request_with_no_iat(client, sample_api_key): @@ -123,7 +102,50 @@ def test_admin_auth_should_not_allow_request_with_no_iat(client, sample_api_key) request.headers = {'Authorization': 'Bearer {}'.format(token)} with pytest.raises(AuthError) as exc: requires_admin_auth() - assert exc.value.short_message == 'Invalid token: signature, api token is not valid' + assert exc.value.short_message == 'Invalid token: API token is not valid' + + +def test_auth_should_not_allow_request_with_non_hs256_algorithm(client, sample_api_key): + iss = str(sample_api_key.service_id) + # code copied from notifications_python_client.authentication.py::create_jwt_token + headers = { + "typ": 'JWT', + "alg": 'HS512' + } + + claims = { + 'iss': iss, + 'iat': int(time.time()) + } + + token = jwt.encode(payload=claims, key=str(uuid.uuid4()), headers=headers).decode() + + request.headers = {'Authorization': 'Bearer {}'.format(token)} + with pytest.raises(AuthError) as exc: + requires_auth() + assert 'Invalid token: algorithm used is not HS256' in exc.value.short_message + + +def test_auth_should_not_allow_request_with_extra_claims(client, sample_api_key): + iss = str(sample_api_key.service_id) + # code copied from notifications_python_client.authentication.py::create_jwt_token + headers = { + "typ": 'JWT', + "alg": 'HS256' + } + + claims = { + 'iss': iss, + 'iat': int(time.time()), + 'aud': 'notifications.service.gov.uk' # extra claim that we don't support + } + + token = jwt.encode(payload=claims, key=str(uuid.uuid4()), headers=headers).decode() + + request.headers = {'Authorization': 'Bearer {}'.format(token)} + with pytest.raises(AuthError) as exc: + requires_auth() + assert exc.value.short_message == 'Invalid token: API key not found' def test_should_not_allow_invalid_secret(client, sample_api_key): @@ -136,7 +158,7 @@ def test_should_not_allow_invalid_secret(client, sample_api_key): ) assert response.status_code == 403 data = json.loads(response.get_data()) - assert data['message'] == {"token": ['Invalid token: signature, api token not found']} + assert data['message'] == {"token": ['Invalid token: API key not found']} @pytest.mark.parametrize('scheme', ['bearer', 'Bearer']) @@ -253,7 +275,7 @@ def test_authentication_returns_error_when_admin_client_has_no_secrets(client): headers={'Authorization': 'Bearer {}'.format(token)}) assert response.status_code == 403 error_message = json.loads(response.get_data()) - assert error_message['message'] == {"token": ["Invalid token: signature, api token is not valid"]} + assert error_message['message'] == {"token": ["Invalid token: API token is not valid"]} def test_authentication_returns_error_when_admin_client_secret_is_invalid(client): @@ -268,7 +290,7 @@ def test_authentication_returns_error_when_admin_client_secret_is_invalid(client headers={'Authorization': 'Bearer {}'.format(token)}) assert response.status_code == 403 error_message = json.loads(response.get_data()) - assert error_message['message'] == {"token": ["Invalid token: signature, api token is not valid"]} + assert error_message['message'] == {"token": ["Invalid token: API token is not valid"]} current_app.config['ADMIN_CLIENT_SECRET'] = api_secret