darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-02-04 10:21:14 -05:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2701 from alphagov/jwt-nightmare

Browse Source 
Catch uncaught JWT exceptions
This commit is contained in:
David McDonald
2020-02-14 14:34:11 +00:00
committed by GitHub
parent c71de8d62a f861da1843
commit ce253a990b
5 changed files with 60 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
app/authentication/auth.py
View File

@@ -1,7 +1,7 @@
from flask import request, _request_ctx_stack, current_app, g
from notifications_python_client.authentication import decode_jwt_token, get_token_issuer
from notifications_python_client.errors import (
TokenDecodeError, TokenExpiredError, TokenIssuerError, TokenAlgorithmError
TokenDecodeError, TokenExpiredError, TokenIssuerError, TokenAlgorithmError, TokenError
)
from notifications_utils import request_helper
from sqlalchemy.exc import DataError
@@ -10,6 +10,9 @@ from sqlalchemy.orm.exc import NoResultFound
from app.dao.services_dao import dao_fetch_service_by_id_with_api_keys
GENERAL_TOKEN_ERROR_MESSAGE = 'Invalid token: make sure your API token matches the example at https://docs.notifications.service.gov.uk/rest-api.html#authorisation-header' # noqa
class AuthError(Exception):
def __init__(self, message, code, service_id=None, api_key_id=None):
self.message = {"token": [message]}
@@ -36,12 +39,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,17 +63,17 @@ 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():
request_helper.check_proxy_header_before_request()
auth_token = get_auth_token(request)
client = __get_token_issuer(auth_token)
issuer = __get_token_issuer(auth_token) # ie the `iss` claim which should be a service ID
try:
service = dao_fetch_service_by_id_with_api_keys(client)
service = dao_fetch_service_by_id_with_api_keys(issuer)
except DataError:
raise AuthError("Invalid token: service id is not the right data type", 403)
except NoResultFound:
@@ -92,7 +95,15 @@ def requires_auth():
err_msg = "Invalid token: algorithm used is not HS256"
raise AuthError(err_msg, 403, service_id=service.id, api_key_id=api_key.id)
except TokenDecodeError:
# we attempted to validate the token but it failed meaning it was not signed using this api key.
# Let's try the next one
# TODO: Change this so it doesn't also catch `TokenIssuerError` or `TokenIssuedAtError` exceptions (which
# are children of `TokenDecodeError`) as these should cause an auth error immediately rather than
# continue on to check the next API key
continue
except TokenError:
# General error when trying to decode and validate the token
raise AuthError(GENERAL_TOKEN_ERROR_MESSAGE, 403, service_id=service.id, api_key_id=api_key.id)
if api_key.expiry_date:
raise AuthError("Invalid token: API key revoked", 403, service_id=service.id, api_key_id=api_key.id)
@@ -100,7 +111,7 @@ def requires_auth():
g.service_id = api_key.service_id
_request_ctx_stack.top.authenticated_service = service
_request_ctx_stack.top.api_user = api_key
current_app.logger.info('API authorised for service {} with api key {}, using client {}'.format(
current_app.logger.info('API authorised for service {} with api key {}, using issuer {}'.format(
service.id,
api_key.id,
request.headers.get('User-Agent')
@@ -108,17 +119,17 @@ 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):
try:
client = get_token_issuer(auth_token)
issuer = 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)
return client
raise AuthError(GENERAL_TOKEN_ERROR_MESSAGE, 403)
return issuer
def handle_admin_key(auth_token, secret):
@@ -127,4 +138,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: could not decode your API token", 403)

4
app/errors.py
View File

@@ -88,12 +88,12 @@ def register_errors(blueprint):
@blueprint.errorhandler(401)
def unauthorized(e):
error_message = "Unauthorized, authentication token must be provided"
error_message = "Unauthorized: authentication token must be provided"
return jsonify(result='error', message=error_message), 401, [('WWW-Authenticate', 'Bearer')]
@blueprint.errorhandler(403)
def forbidden(e):
error_message = "Forbidden, invalid authentication token provided"
error_message = "Forbidden: invalid authentication token provided"
return jsonify(result='error', message=error_message), 403
@blueprint.errorhandler(429)

4
requirements.txt
View File

@@ -36,13 +36,13 @@ alembic==1.4.0
amqp==1.4.9
anyjson==0.3.3
attrs==19.3.0
awscli==1.17.16
awscli==1.17.17
bcrypt==3.1.7
billiard==3.3.0.23
bleach==3.1.0
boto==2.49.0
boto3==1.10.38
botocore==1.14.16
botocore==1.14.17
certifi==2019.11.28
chardet==3.0.4
Click==7.0

41
tests/app/authentication/test_authentication.py
View File

@@ -12,7 +12,7 @@ from notifications_python_client.authentication import create_jwt_token
from app import api_user
from app.dao.api_key_dao import get_unsigned_secrets, save_model_api_key, get_unsigned_secret, expire_api_key
from app.models import ApiKey, KEY_TYPE_NORMAL
from app.authentication.auth import AuthError, requires_admin_auth, requires_auth
from app.authentication.auth import AuthError, requires_admin_auth, requires_auth, GENERAL_TOKEN_ERROR_MESSAGE
from tests.conftest import set_config
@@ -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 == GENERAL_TOKEN_ERROR_MESSAGE
@pytest.mark.parametrize('auth_fn', [requires_auth, requires_admin_auth])
@@ -80,7 +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'
assert exc.value.short_message == 'Invalid token: API key not found'
def test_auth_should_not_allow_request_with_non_hs256_algorithm(client, sample_api_key):
@@ -123,7 +123,30 @@ 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: could not decode your API token"
def test_auth_should_not_allow_request_with_extra_claims(client, sample_api_key):
iss = str(sample_api_key.service_id)
key = get_unsigned_secrets(sample_api_key.service_id)[0]
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=key, headers=headers).decode()
request.headers = {'Authorization': 'Bearer {}'.format(token)}
with pytest.raises(AuthError) as exc:
requires_auth()
assert exc.value.short_message == GENERAL_TOKEN_ERROR_MESSAGE
def test_should_not_allow_invalid_secret(client, sample_api_key):
@@ -136,7 +159,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 +276,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: could not decode your API token"]}
def test_authentication_returns_error_when_admin_client_secret_is_invalid(client):
@@ -268,7 +291,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: could not decode your API token"]}
current_app.config['ADMIN_CLIENT_SECRET'] = api_secret

2
tests/app/v2/notifications/test_post_notifications.py
View File

@@ -256,7 +256,7 @@ def test_post_notification_returns_401_and_well_formed_auth_error(client, sample
error_resp = json.loads(response.get_data(as_text=True))
assert error_resp['status_code'] == 401
assert error_resp['errors'] == [{'error': "AuthError",
'message': 'Unauthorized, authentication token must be provided'}]
'message': 'Unauthorized: authentication token must be provided'}]
@pytest.mark.parametrize("notification_type, key_send_to, send_to",