From 463f1eefaf5aa6c9a1febb9235499a9b628e334b Mon Sep 17 00:00:00 2001 From: Athanasios Voutsadakis Date: Tue, 27 Mar 2018 17:37:09 +0100 Subject: [PATCH] Move proxy header check to auth-requiring endpoints The main drive behind this is to allow us to enable http healthchecks on the `/_status` endpoint. The healthcheck requests are happening directly on the instances without going to the proxy to get the header properly set. In any case, endpoints like `/_status` should be generally accessible by anything without requiring any form of authorization. --- app/__init__.py | 2 -- app/authentication/auth.py | 5 ++++ .../app/authentication/test_authentication.py | 30 +++++++++++++++++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index c56ec476c..81eeec6e0 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -219,8 +219,6 @@ def init_app(app): def record_user_agent(): statsd_client.incr("user-agent.{}".format(process_user_agent(request.headers.get('User-Agent', None)))) - app.before_request(request_helper.check_proxy_header_before_request) - @app.before_request def record_request_details(): g.start = monotonic() diff --git a/app/authentication/auth.py b/app/authentication/auth.py index ea42791ee..f878ccda7 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -1,6 +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 +from notifications_utils import request_helper from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound @@ -48,6 +49,8 @@ def requires_no_auth(): def requires_admin_auth(): + request_helper.check_proxy_header_before_request() + auth_token = get_auth_token(request) client = __get_token_issuer(auth_token) @@ -59,6 +62,8 @@ def requires_admin_auth(): def requires_auth(): + request_helper.check_proxy_header_before_request() + auth_token = get_auth_token(request) client = __get_token_issuer(auth_token) diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index 4677ecce3..e6d78d317 100644 --- a/tests/app/authentication/test_authentication.py +++ b/tests/app/authentication/test_authentication.py @@ -325,11 +325,11 @@ def __create_token(service_id): @pytest.mark.parametrize('check_proxy_header,header_value,expected_status', [ (True, 'key_1', 200), - (True, 'wrong_key', 403), + (True, 'wrong_key', 200), (False, 'key_1', 200), (False, 'wrong_key', 200), ]) -def test_route_correct_secret_key(notify_api, check_proxy_header, header_value, expected_status): +def test_proxy_key_non_auth_endpoint(notify_api, check_proxy_header, header_value, expected_status): with set_config_values(notify_api, { 'ROUTE_SECRET_KEY_1': 'key_1', 'ROUTE_SECRET_KEY_2': '', @@ -344,3 +344,29 @@ def test_route_correct_secret_key(notify_api, check_proxy_header, header_value, ] ) assert response.status_code == expected_status + + +@pytest.mark.parametrize('check_proxy_header,header_value,expected_status', [ + (True, 'key_1', 200), + (True, 'wrong_key', 403), + (False, 'key_1', 200), + (False, 'wrong_key', 200), +]) +def test_proxy_key_on_admin_auth_endpoint(notify_api, check_proxy_header, header_value, expected_status): + token = create_jwt_token(current_app.config['ADMIN_CLIENT_SECRET'], current_app.config['ADMIN_CLIENT_USER_NAME']) + + with set_config_values(notify_api, { + 'ROUTE_SECRET_KEY_1': 'key_1', + 'ROUTE_SECRET_KEY_2': '', + 'CHECK_PROXY_HEADER': check_proxy_header, + }): + + with notify_api.test_client() as client: + response = client.get( + path='/service', + headers=[ + ('X-Custom-Forwarder', header_value), + ('Authorization', 'Bearer {}'.format(token)) + ] + ) + assert response.status_code == expected_status