This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 33e1909ae4 Improve security and error handling for the internal API
(#40999)
33e1909ae4 is described below
commit 33e1909ae4f9e75dac20c6ea0ceadaddc488183f
Author: Jarek Potiuk <[email protected]>
AuthorDate: Wed Jul 24 17:53:21 2024 +0200
Improve security and error handling for the internal API (#40999)
There are a few fixes to the internal API error handling that
caused errors when handling permission errors. The internal API
now handles only application/json content type and only requests
that explicitly accept only application/json responses - which is
an extra layer of security that makes CSRF protection not necessary
(though our token validation should already prevent CSRF issues.
The Permission Denied exceptions did not like the exc_info parameter,
so it has been removed. Also in case of auth_manager not initialized
we should return a very generic error message as this is only in
case of standalone internal_api component.
Co-authored-by: Vincent <[email protected]>
---
airflow/api_internal/endpoints/rpc_api_endpoint.py | 12 +++++++---
airflow/api_internal/internal_api_call.py | 1 +
airflow/www/extensions/init_auth_manager.py | 5 ++++
airflow/www/views.py | 5 +++-
.../endpoints/test_rpc_api_endpoint.py | 28 +++++++++++++++++++++-
5 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/airflow/api_internal/endpoints/rpc_api_endpoint.py
b/airflow/api_internal/endpoints/rpc_api_endpoint.py
index 7e655e5b4e..c3d322f01f 100644
--- a/airflow/api_internal/endpoints/rpc_api_endpoint.py
+++ b/airflow/api_internal/endpoints/rpc_api_endpoint.py
@@ -163,6 +163,12 @@ def log_and_build_error_response(message, status):
def internal_airflow_api(body: dict[str, Any]) -> APIResponse:
"""Handle Internal API /internal_api/v1/rpcapi endpoint."""
+ content_type = request.headers.get("Content-Type")
+ if content_type != "application/json":
+ raise PermissionDenied("Expected Content-Type: application/json")
+ accept = request.headers.get("Accept")
+ if accept != "application/json":
+ raise PermissionDenied("Expected Accept: application/json")
auth = request.headers.get("Authorization", "")
signer = JWTSigner(
secret_key=conf.get("core", "internal_api_secret_key"),
@@ -177,11 +183,11 @@ def internal_airflow_api(body: dict[str, Any]) ->
APIResponse:
except BadSignature:
raise PermissionDenied("Bad Signature. Please use only the tokens
provided by the API.")
except InvalidAudienceError:
- raise PermissionDenied("Invalid audience for the request",
exc_info=True)
+ raise PermissionDenied("Invalid audience for the request")
except InvalidSignatureError:
- raise PermissionDenied("The signature of the request was wrong",
exc_info=True)
+ raise PermissionDenied("The signature of the request was wrong")
except ImmatureSignatureError:
- raise PermissionDenied("The signature of the request was sent from the
future", exc_info=True)
+ raise PermissionDenied("The signature of the request was sent from the
future")
except ExpiredSignatureError:
raise PermissionDenied(
"The signature of the request has expired. Make sure that all
components "
diff --git a/airflow/api_internal/internal_api_call.py
b/airflow/api_internal/internal_api_call.py
index 7962b5b590..c01ee141fe 100644
--- a/airflow/api_internal/internal_api_call.py
+++ b/airflow/api_internal/internal_api_call.py
@@ -119,6 +119,7 @@ def internal_api_call(func: Callable[PS, RT]) ->
Callable[PS, RT]:
)
headers = {
"Content-Type": "application/json",
+ "Accept": "application/json",
"Authorization": signer.generate_signed_token({"method":
method_name}),
}
data = {"jsonrpc": "2.0", "method": method_name, "params": params_json}
diff --git a/airflow/www/extensions/init_auth_manager.py
b/airflow/www/extensions/init_auth_manager.py
index 1c3d399647..f69734ce8a 100644
--- a/airflow/www/extensions/init_auth_manager.py
+++ b/airflow/www/extensions/init_auth_manager.py
@@ -65,3 +65,8 @@ def get_auth_manager() -> BaseAuthManager:
"The `init_auth_manager` method needs to be called first."
)
return auth_manager
+
+
+def is_auth_manager_initialized() -> bool:
+ """Return whether the auth manager has been initialized."""
+ return auth_manager is not None
diff --git a/airflow/www/views.py b/airflow/www/views.py
index 0e0923aa13..568b4bfe39 100644
--- a/airflow/www/views.py
+++ b/airflow/www/views.py
@@ -137,7 +137,7 @@ from airflow.utils.types import NOTSET
from airflow.version import version
from airflow.www import auth, utils as wwwutils
from airflow.www.decorators import action_logging, gzipped
-from airflow.www.extensions.init_auth_manager import get_auth_manager
+from airflow.www.extensions.init_auth_manager import get_auth_manager,
is_auth_manager_initialized
from airflow.www.forms import (
DagRunEditForm,
DateTimeForm,
@@ -688,6 +688,9 @@ def method_not_allowed(error):
def show_traceback(error):
"""Show Traceback for a given error."""
+ if not is_auth_manager_initialized():
+ # this is the case where internal API component is used and auth
manager is not used/initialized
+ return ("Error calling the API", 500)
is_logged_in = get_auth_manager().is_logged_in()
return (
render_template(
diff --git a/tests/api_internal/endpoints/test_rpc_api_endpoint.py
b/tests/api_internal/endpoints/test_rpc_api_endpoint.py
index a453c8f367..8a519db814 100644
--- a/tests/api_internal/endpoints/test_rpc_api_endpoint.py
+++ b/tests/api_internal/endpoints/test_rpc_api_endpoint.py
@@ -123,6 +123,7 @@ class TestRpcApiEndpoint:
mock_test_method.return_value = method_result
headers = {
"Content-Type": "application/json",
+ "Accept": "application/json",
"Authorization": signer.generate_signed_token({"method":
TEST_METHOD_NAME}),
}
input_data = {
@@ -148,6 +149,7 @@ class TestRpcApiEndpoint:
def test_method_with_exception(self, signer: JWTSigner):
headers = {
"Content-Type": "application/json",
+ "Accept": "application/json",
"Authorization": signer.generate_signed_token({"method":
TEST_METHOD_NAME}),
}
mock_test_method.side_effect = ValueError("Error!!!")
@@ -162,6 +164,7 @@ class TestRpcApiEndpoint:
UNKNOWN_METHOD = "i-bet-it-does-not-exist"
headers = {
"Content-Type": "application/json",
+ "Accept": "application/json",
"Authorization": signer.generate_signed_token({"method":
UNKNOWN_METHOD}),
}
data = {"jsonrpc": "2.0", "method": UNKNOWN_METHOD, "params": {}}
@@ -174,6 +177,7 @@ class TestRpcApiEndpoint:
def test_invalid_jsonrpc(self, signer: JWTSigner):
headers = {
"Content-Type": "application/json",
+ "Accept": "application/json",
"Authorization": signer.generate_signed_token({"method":
TEST_METHOD_NAME}),
}
data = {"jsonrpc": "1.0", "method": TEST_METHOD_NAME, "params": {}}
@@ -194,13 +198,14 @@ class TestRpcApiEndpoint:
with pytest.raises(PermissionDenied, match="Unable to authenticate API
via token."):
self.client.post(
"/internal_api/v1/rpcapi",
- headers={"Content-Type": "application/json"},
+ headers={"Content-Type": "application/json", "Accept":
"application/json"},
data=json.dumps(input_data),
)
def test_invalid_token(self, signer: JWTSigner):
headers = {
"Content-Type": "application/json",
+ "Accept": "application/json",
"Authorization": signer.generate_signed_token({"method":
"WRONG_METHOD_NAME"}),
}
data = {"jsonrpc": "1.0", "method": TEST_METHOD_NAME, "params": {}}
@@ -209,3 +214,24 @@ class TestRpcApiEndpoint:
PermissionDenied, match="Bad Signature. Please use only the tokens
provided by the API."
):
self.client.post("/internal_api/v1/rpcapi", headers=headers,
data=json.dumps(data))
+
+ def test_missing_accept(self, signer: JWTSigner):
+ headers = {
+ "Content-Type": "application/json",
+ "Authorization": signer.generate_signed_token({"method":
"WRONG_METHOD_NAME"}),
+ }
+ data = {"jsonrpc": "1.0", "method": TEST_METHOD_NAME, "params": {}}
+
+ with pytest.raises(PermissionDenied, match="Expected Accept:
application/json"):
+ self.client.post("/internal_api/v1/rpcapi", headers=headers,
data=json.dumps(data))
+
+ def test_wrong_accept(self, signer: JWTSigner):
+ headers = {
+ "Content-Type": "application/json",
+ "Accept": "application/html",
+ "Authorization": signer.generate_signed_token({"method":
"WRONG_METHOD_NAME"}),
+ }
+ data = {"jsonrpc": "1.0", "method": TEST_METHOD_NAME, "params": {}}
+
+ with pytest.raises(PermissionDenied, match="Expected Accept:
application/json"):
+ self.client.post("/internal_api/v1/rpcapi", headers=headers,
data=json.dumps(data))
