Change variable name to make more descriptive

Also remove unnecessary if statement Also add manifest change to make sure relevant environment variables makes it into the app
2026-01-31 23:26:23 -05:00 · 2020-02-20 15:16:37 +00:00
parent 2967fdce08
commit 2dc5550159
5 changed files with 28 additions and 29 deletions
--- a/app/authentication/auth.py
+++ b/app/authentication/auth.py
@@ -62,18 +62,17 @@ def requires_admin_auth():
    if client == current_app.config.get('ADMIN_CLIENT_USER_NAME'):
        g.service_id = current_app.config.get('ADMIN_CLIENT_USER_NAME')

-        if len(current_app.config.get('ADMIN_CLIENT_SECRETS')):
-            for secret in current_app.config.get('ADMIN_CLIENT_SECRETS'):
-                try:
-                    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 API key
-                    continue
+        for secret in current_app.config.get('API_INTERNAL_SECRETS'):
+            try:
+                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
        raise AuthError("Unauthorized: admin authentication token not found", 401)
--- a/app/config.py
+++ b/app/config.py
@@ -64,8 +64,8 @@ class Config(object):
    # URL of api app (on AWS this is the internal api endpoint)
    API_HOST_NAME = os.getenv('API_HOST_NAME')

-    # admin app api keys
-    ADMIN_CLIENT_SECRETS = json.loads(os.environ.get('ADMIN_CLIENT_SECRETS', '[]'))
+    # secrets that internal apps, such as the admin app or document download, must use to authenticate with the API
+    API_INTERNAL_SECRETS = json.loads(os.environ.get('API_INTERNAL_SECRETS', '[]'))

    # encyption secret/salt
    SECRET_KEY = os.getenv('SECRET_KEY')
@@ -369,7 +369,7 @@ class Development(Config):
    TRANSIENT_UPLOADED_LETTERS = 'development-transient-uploaded-letters'
    LETTER_SANITISE_BUCKET_NAME = 'development-letters-sanitise'

-    ADMIN_CLIENT_SECRETS = ['dev-notify-secret-key']
+    API_INTERNAL_SECRETS = ['dev-notify-secret-key']
    SECRET_KEY = 'dev-notify-secret-key'
    DANGEROUS_SALT = 'dev-notify-salt'

--- a/manifest.yml.j2
+++ b/manifest.yml.j2
@@ -72,7 +72,7 @@ applications:

      # Credentials variables
      ADMIN_BASE_URL: '{{ ADMIN_BASE_URL }}'
-      ADMIN_CLIENT_SECRET: '{{ ADMIN_CLIENT_SECRET }}'
+      API_INTERNAL_SECRETS: '{{ API_INTERNAL_SECRETS }}'
      API_HOST_NAME: '{{ API_HOST_NAME }}'
      DANGEROUS_SALT: '{{ DANGEROUS_SALT }}'
      SECRET_KEY: '{{ SECRET_KEY }}'
--- a/tests/init.py
+++ b/tests/init.py
@@ -28,7 +28,7 @@ def create_authorization_header(service_id=None, key_type=KEY_TYPE_NORMAL):

    else:
        client_id = current_app.config['ADMIN_CLIENT_USER_NAME']
-        secret = current_app.config['ADMIN_CLIENT_SECRETS'][0]
+        secret = current_app.config['API_INTERNAL_SECRETS'][0]

    token = create_jwt_token(secret=secret, client_id=client_id)
    return 'Authorization', 'Bearer {}'.format(token)
--- a/tests/app/authentication/test_authentication.py
+++ b/tests/app/authentication/test_authentication.py
@@ -106,7 +106,7 @@ 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):
    iss = current_app.config['ADMIN_CLIENT_USER_NAME']
-    secret = current_app.config['ADMIN_CLIENT_SECRETS'][0]
+    secret = current_app.config['API_INTERNAL_SECRETS'][0]

    # code copied from notifications_python_client.authentication.py::create_jwt_token
    headers = {
@@ -129,7 +129,7 @@ def test_admin_auth_should_not_allow_request_with_no_iat(client):

 def test_admin_auth_should_not_allow_request_with_old_iat(client):
    iss = current_app.config['ADMIN_CLIENT_USER_NAME']
-    secret = current_app.config['ADMIN_CLIENT_SECRETS'][0]
+    secret = current_app.config['API_INTERNAL_SECRETS'][0]

    # code copied from notifications_python_client.authentication.py::create_jwt_token
    headers = {
@@ -213,22 +213,22 @@ 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):
    token = create_jwt_token(
-        current_app.config['ADMIN_CLIENT_SECRETS'][0], current_app.config['ADMIN_CLIENT_USER_NAME']
+        current_app.config['API_INTERNAL_SECRETS'][0], current_app.config['ADMIN_CLIENT_USER_NAME']
    )
    response = client.get('/service', headers={'Authorization': 'Bearer {}'.format(token)})
    assert response.status_code == 200


 def test_should_allow_valid_token_for_request_with_path_params_for_admin_url_with_second_secret(client):
-    with set_config(client.application, 'ADMIN_CLIENT_SECRETS', ["secret1", "secret2"]):
+    with set_config(client.application, 'API_INTERNAL_SECRETS', ["secret1", "secret2"]):
        token = create_jwt_token(
-            current_app.config['ADMIN_CLIENT_SECRETS'][0], current_app.config['ADMIN_CLIENT_USER_NAME']
+            current_app.config['API_INTERNAL_SECRETS'][0], current_app.config['ADMIN_CLIENT_USER_NAME']
        )
        response = client.get('/service', headers={'Authorization': 'Bearer {}'.format(token)})
        assert response.status_code == 200

        token = create_jwt_token(
-            current_app.config['ADMIN_CLIENT_SECRETS'][1], current_app.config['ADMIN_CLIENT_USER_NAME']
+            current_app.config['API_INTERNAL_SECRETS'][1], current_app.config['ADMIN_CLIENT_USER_NAME']
        )
        response = client.get('/service', headers={'Authorization': 'Bearer {}'.format(token)})
        assert response.status_code == 200
@@ -305,13 +305,13 @@ def test_authentication_returns_token_expired_when_service_uses_expired_key_and_


 def test_authentication_returns_error_when_admin_client_has_no_secrets(client):
-    api_secret = current_app.config.get('ADMIN_CLIENT_SECRETS')[0]
+    api_secret = current_app.config.get('API_INTERNAL_SECRETS')[0]
    api_service_id = current_app.config.get('ADMIN_CLIENT_USER_NAME')
    token = create_jwt_token(
        secret=api_secret,
        client_id=api_service_id
    )
-    with set_config(client.application, 'ADMIN_CLIENT_SECRETS', []):
+    with set_config(client.application, 'API_INTERNAL_SECRETS', []):
        response = client.get(
            '/service',
            headers={'Authorization': 'Bearer {}'.format(token)})
@@ -321,19 +321,19 @@ def test_authentication_returns_error_when_admin_client_has_no_secrets(client):


 def test_authentication_returns_error_when_admin_client_secret_is_invalid(client):
-    api_secret = current_app.config.get('ADMIN_CLIENT_SECRETS')[0]
+    api_secret = current_app.config.get('API_INTERNAL_SECRETS')[0]
    token = create_jwt_token(
        secret=api_secret,
        client_id=current_app.config.get('ADMIN_CLIENT_USER_NAME')
    )
-    current_app.config['ADMIN_CLIENT_SECRETS'][0] = 'something-wrong'
+    current_app.config['API_INTERNAL_SECRETS'][0] = 'something-wrong'
    response = client.get(
        '/service',
        headers={'Authorization': 'Bearer {}'.format(token)})
    assert response.status_code == 401
    error_message = json.loads(response.get_data())
    assert error_message['message'] == {"token": ["Unauthorized: admin authentication token not found"]}
-    current_app.config['ADMIN_CLIENT_SECRETS'][0] = api_secret
+    current_app.config['API_INTERNAL_SECRETS'][0] = api_secret


 def test_authentication_returns_error_when_service_doesnt_exit(
@@ -439,7 +439,7 @@ def test_proxy_key_non_auth_endpoint(notify_api, check_proxy_header, header_valu
 ])
 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_SECRETS'][0], current_app.config['ADMIN_CLIENT_USER_NAME']
+        current_app.config['API_INTERNAL_SECRETS'][0], current_app.config['ADMIN_CLIENT_USER_NAME']
    )

    with set_config_values(notify_api, {