Support granular API auth for internal apps

Previously we just had a single array of API keys / secrets, any of
which could be used to get past the "requires_admin_auth" check.
While multiple keys are necessary to allow for rotation, we should
avoid giving other apps access this way (too much privilege).

This converts the existing config vars into a new dictionary, keyed
by client_id. We can then use the dictionary to scope auth for new
API consumers like gov.uk/alerts to just the endpoints they need to
access, while maintaining existing access for the Admin app.

Once the new dictionary is available as a JSON environment variable,
we'll be able to remove the old credentials / config. In the next
commits, we'll look at more tests for the new functionality.
This commit is contained in:
Ben Thorner
2021-07-26 16:45:10 +01:00
parent 312a895822
commit 49455d9890
4 changed files with 78 additions and 62 deletions

View File

@@ -67,30 +67,37 @@ def requires_no_auth():
def requires_admin_auth(): def requires_admin_auth():
requires_internal_auth(current_app.config.get('ADMIN_CLIENT_USER_NAME'))
def requires_internal_auth(expected_client_id):
if expected_client_id not in current_app.config.get('INTERNAL_CLIENT_API_KEYS'):
raise TypeError("Unknown client_id for internal auth")
request_helper.check_proxy_header_before_request() request_helper.check_proxy_header_before_request()
auth_token = get_auth_token(request) auth_token = get_auth_token(request)
client = __get_token_issuer(auth_token) client_id = __get_token_issuer(auth_token)
if client == current_app.config.get('ADMIN_CLIENT_USER_NAME'): if client_id != expected_client_id:
g.service_id = current_app.config.get('ADMIN_CLIENT_USER_NAME') raise AuthError("Unauthorized: not allowed to perform this action", 401)
for secret in current_app.config.get('API_INTERNAL_SECRETS'): g.service_id = client_id
try: secrets = current_app.config.get('INTERNAL_CLIENT_API_KEYS')[client_id]
decode_jwt_token(auth_token, secret)
return
except TokenExpiredError:
raise AuthError("Invalid token: expired, check that your system clock is accurate", 403)
except TokenDecodeError:
# 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 admin client secret
continue
# Either there are no admin client secrets or their token didn't match one of them so error for secret in secrets:
raise AuthError("Unauthorized: admin authentication token not found", 401) try:
else: decode_jwt_token(auth_token, secret)
raise AuthError('Unauthorized: admin authentication token required', 401) return
except TokenExpiredError:
raise AuthError("Invalid token: expired, check that your system clock is accurate", 403)
except TokenDecodeError:
# 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 admin client secret
continue
# Either there are no admin client secrets or their token didn't match one of them so error
raise AuthError("Unauthorized: API authentication token not found", 401)
def requires_auth(): def requires_auth():

View File

@@ -84,9 +84,16 @@ class Config(object):
# URL of api app (on AWS this is the internal api endpoint) # URL of api app (on AWS this is the internal api endpoint)
API_HOST_NAME = os.getenv('API_HOST_NAME') API_HOST_NAME = os.getenv('API_HOST_NAME')
# secrets that internal apps, such as the admin app or document download, must use to authenticate with the API # LEGACY: replacing with INTERNAL_CLIENT_API_KEYS
API_INTERNAL_SECRETS = json.loads(os.environ.get('API_INTERNAL_SECRETS', '[]')) API_INTERNAL_SECRETS = json.loads(os.environ.get('API_INTERNAL_SECRETS', '[]'))
# secrets that internal apps, such as the admin app or document download, must use to authenticate with the API
ADMIN_CLIENT_USER_NAME = 'notify-admin'
INTERNAL_CLIENT_API_KEYS = {
ADMIN_CLIENT_USER_NAME: API_INTERNAL_SECRETS
}
# encyption secret/salt # encyption secret/salt
SECRET_KEY = os.getenv('SECRET_KEY') SECRET_KEY = os.getenv('SECRET_KEY')
DANGEROUS_SALT = os.getenv('DANGEROUS_SALT') DANGEROUS_SALT = os.getenv('DANGEROUS_SALT')
@@ -129,7 +136,6 @@ class Config(object):
########################### ###########################
NOTIFY_ENVIRONMENT = 'development' NOTIFY_ENVIRONMENT = 'development'
ADMIN_CLIENT_USER_NAME = 'notify-admin'
AWS_REGION = 'eu-west-1' AWS_REGION = 'eu-west-1'
INVITATION_EXPIRATION_DAYS = 2 INVITATION_EXPIRATION_DAYS = 2
NOTIFY_APP_NAME = 'api' NOTIFY_APP_NAME = 'api'
@@ -399,7 +405,10 @@ class Development(Config):
TRANSIENT_UPLOADED_LETTERS = 'development-transient-uploaded-letters' TRANSIENT_UPLOADED_LETTERS = 'development-transient-uploaded-letters'
LETTER_SANITISE_BUCKET_NAME = 'development-letters-sanitise' LETTER_SANITISE_BUCKET_NAME = 'development-letters-sanitise'
API_INTERNAL_SECRETS = ['dev-notify-secret-key'] INTERNAL_CLIENT_API_KEYS = {
Config.ADMIN_CLIENT_USER_NAME: ['dev-notify-secret-key']
}
SECRET_KEY = 'dev-notify-secret-key' SECRET_KEY = 'dev-notify-secret-key'
DANGEROUS_SALT = 'dev-notify-salt' DANGEROUS_SALT = 'dev-notify-salt'

View File

@@ -28,7 +28,7 @@ def create_authorization_header(service_id=None, key_type=KEY_TYPE_NORMAL):
else: else:
client_id = current_app.config['ADMIN_CLIENT_USER_NAME'] client_id = current_app.config['ADMIN_CLIENT_USER_NAME']
secret = current_app.config['API_INTERNAL_SECRETS'][0] secret = current_app.config['INTERNAL_CLIENT_API_KEYS'][client_id][0]
token = create_jwt_token(secret=secret, client_id=client_id) token = create_jwt_token(secret=secret, client_id=client_id)
return 'Authorization', 'Bearer {}'.format(token) return 'Authorization', 'Bearer {}'.format(token)

View File

@@ -116,8 +116,8 @@ def test_auth_should_not_allow_request_with_non_hs256_algorithm(client, sample_a
def test_admin_auth_should_not_allow_request_with_no_iat(client): def test_admin_auth_should_not_allow_request_with_no_iat(client):
iss = current_app.config['ADMIN_CLIENT_USER_NAME'] client_id = current_app.config['ADMIN_CLIENT_USER_NAME']
secret = current_app.config['API_INTERNAL_SECRETS'][0] secret = current_app.config['INTERNAL_CLIENT_API_KEYS'][client_id][0]
# code copied from notifications_python_client.authentication.py::create_jwt_token # code copied from notifications_python_client.authentication.py::create_jwt_token
headers = { headers = {
@@ -126,7 +126,7 @@ def test_admin_auth_should_not_allow_request_with_no_iat(client):
} }
claims = { claims = {
'iss': iss 'iss': client_id,
# 'iat': not provided # 'iat': not provided
} }
@@ -135,12 +135,12 @@ def test_admin_auth_should_not_allow_request_with_no_iat(client):
request.headers = {'Authorization': 'Bearer {}'.format(token)} request.headers = {'Authorization': 'Bearer {}'.format(token)}
with pytest.raises(AuthError) as exc: with pytest.raises(AuthError) as exc:
requires_admin_auth() requires_admin_auth()
assert exc.value.short_message == "Unauthorized: admin authentication token not found" assert exc.value.short_message == "Unauthorized: API authentication token not found"
def test_admin_auth_should_not_allow_request_with_old_iat(client): def test_admin_auth_should_not_allow_request_with_old_iat(client):
iss = current_app.config['ADMIN_CLIENT_USER_NAME'] client_id = current_app.config['ADMIN_CLIENT_USER_NAME']
secret = current_app.config['API_INTERNAL_SECRETS'][0] secret = current_app.config['INTERNAL_CLIENT_API_KEYS'][client_id][0]
# code copied from notifications_python_client.authentication.py::create_jwt_token # code copied from notifications_python_client.authentication.py::create_jwt_token
headers = { headers = {
@@ -149,7 +149,7 @@ def test_admin_auth_should_not_allow_request_with_old_iat(client):
} }
claims = { claims = {
'iss': iss, 'iss': client_id,
'iat': int(time.time()) - 60 'iat': int(time.time()) - 60
} }
@@ -224,24 +224,24 @@ def test_should_allow_valid_token_for_request_with_path_params_for_public_url(cl
def test_should_allow_valid_token_for_request_with_path_params_for_admin_url(client): def test_should_allow_valid_token_for_request_with_path_params_for_admin_url(client):
token = create_jwt_token( client_id = current_app.config['ADMIN_CLIENT_USER_NAME']
current_app.config['API_INTERNAL_SECRETS'][0], current_app.config['ADMIN_CLIENT_USER_NAME'] secret = current_app.config['INTERNAL_CLIENT_API_KEYS'][client_id][0]
)
token = create_jwt_token(secret, client_id)
response = client.get('/service', headers={'Authorization': 'Bearer {}'.format(token)}) response = client.get('/service', headers={'Authorization': 'Bearer {}'.format(token)})
assert response.status_code == 200 assert response.status_code == 200
def test_should_allow_valid_token_for_request_with_path_params_for_admin_url_with_second_secret(client): def test_should_allow_valid_token_for_request_with_path_params_for_admin_url_with_second_secret(client):
with set_config(client.application, 'API_INTERNAL_SECRETS', ["secret1", "secret2"]): client_id = current_app.config['ADMIN_CLIENT_USER_NAME']
token = create_jwt_token( new_secrets = {client_id: ["secret1", "secret2"]}
current_app.config['API_INTERNAL_SECRETS'][0], current_app.config['ADMIN_CLIENT_USER_NAME']
) with set_config(client.application, 'INTERNAL_CLIENT_API_KEYS', new_secrets):
token = create_jwt_token("secret1", client_id)
response = client.get('/service', headers={'Authorization': 'Bearer {}'.format(token)}) response = client.get('/service', headers={'Authorization': 'Bearer {}'.format(token)})
assert response.status_code == 200 assert response.status_code == 200
token = create_jwt_token( token = create_jwt_token("secret2", client_id)
current_app.config['API_INTERNAL_SECRETS'][1], current_app.config['ADMIN_CLIENT_USER_NAME']
)
response = client.get('/service', headers={'Authorization': 'Bearer {}'.format(token)}) response = client.get('/service', headers={'Authorization': 'Bearer {}'.format(token)})
assert response.status_code == 200 assert response.status_code == 200
@@ -317,35 +317,35 @@ def test_authentication_returns_token_expired_when_service_uses_expired_key_and_
def test_authentication_returns_error_when_admin_client_has_no_secrets(client): def test_authentication_returns_error_when_admin_client_has_no_secrets(client):
api_secret = current_app.config.get('API_INTERNAL_SECRETS')[0] client_id = current_app.config.get('ADMIN_CLIENT_USER_NAME')
api_service_id = current_app.config.get('ADMIN_CLIENT_USER_NAME') secret = current_app.config.get('INTERNAL_CLIENT_API_KEYS')[client_id][0]
token = create_jwt_token( token = create_jwt_token(secret, client_id)
secret=api_secret, new_secrets = {client_id: []}
client_id=api_service_id
) with set_config(client.application, 'INTERNAL_CLIENT_API_KEYS', new_secrets):
with set_config(client.application, 'API_INTERNAL_SECRETS', []):
response = client.get( response = client.get(
'/service', '/service',
headers={'Authorization': 'Bearer {}'.format(token)}) headers={'Authorization': 'Bearer {}'.format(token)})
assert response.status_code == 401 assert response.status_code == 401
error_message = json.loads(response.get_data()) error_message = json.loads(response.get_data())
assert error_message['message'] == {"token": ["Unauthorized: admin authentication token not found"]} assert error_message['message'] == {"token": ["Unauthorized: API authentication token not found"]}
def test_authentication_returns_error_when_admin_client_secret_is_invalid(client): def test_authentication_returns_error_when_admin_client_secret_is_invalid(client):
api_secret = current_app.config.get('API_INTERNAL_SECRETS')[0] client_id = current_app.config.get('ADMIN_CLIENT_USER_NAME')
token = create_jwt_token( secret = current_app.config.get('INTERNAL_CLIENT_API_KEYS')[client_id][0]
secret=api_secret, token = create_jwt_token(secret, client_id)
client_id=current_app.config.get('ADMIN_CLIENT_USER_NAME') new_secrets = {client_id: ['something-wrong']}
)
current_app.config['API_INTERNAL_SECRETS'][0] = 'something-wrong' with set_config(client.application, 'INTERNAL_CLIENT_API_KEYS', new_secrets):
response = client.get( response = client.get(
'/service', '/service',
headers={'Authorization': 'Bearer {}'.format(token)}) headers={'Authorization': 'Bearer {}'.format(token)})
assert response.status_code == 401 assert response.status_code == 401
error_message = json.loads(response.get_data()) error_message = json.loads(response.get_data())
assert error_message['message'] == {"token": ["Unauthorized: admin authentication token not found"]} assert error_message['message'] == {"token": ["Unauthorized: API authentication token not found"]}
current_app.config['API_INTERNAL_SECRETS'][0] = api_secret
def test_authentication_returns_error_when_service_doesnt_exit( def test_authentication_returns_error_when_service_doesnt_exit(
@@ -450,9 +450,9 @@ def test_proxy_key_non_auth_endpoint(notify_api, check_proxy_header, header_valu
(False, 'wrong_key', 200), (False, 'wrong_key', 200),
]) ])
def test_proxy_key_on_admin_auth_endpoint(notify_api, check_proxy_header, header_value, expected_status): def test_proxy_key_on_admin_auth_endpoint(notify_api, check_proxy_header, header_value, expected_status):
token = create_jwt_token( client_id = current_app.config.get('ADMIN_CLIENT_USER_NAME')
current_app.config['API_INTERNAL_SECRETS'][0], current_app.config['ADMIN_CLIENT_USER_NAME'] secret = current_app.config.get('INTERNAL_CLIENT_API_KEYS')[client_id][0]
) token = create_jwt_token(secret, client_id)
with set_config_values(notify_api, { with set_config_values(notify_api, {
'ROUTE_SECRET_KEY_1': 'key_1', 'ROUTE_SECRET_KEY_1': 'key_1',