Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve security and error handling for the internal API #40999

Merged
merged 1 commit into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions airflow/api_internal/endpoints/rpc_api_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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 "
Expand Down
1 change: 1 addition & 0 deletions airflow/api_internal/internal_api_call.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ def make_jsonrpc_request(method_name: str, params_json: str) -> bytes:
)
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}
Expand Down
5 changes: 5 additions & 0 deletions airflow/www/extensions/init_auth_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 4 additions & 1 deletion airflow/www/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@
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,
Expand Down Expand Up @@ -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(
Expand Down
28 changes: 27 additions & 1 deletion tests/api_internal/endpoints/test_rpc_api_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ def test_method(self, input_params, method_result, result_cmp_func, method_param
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 = {
Expand All @@ -148,6 +149,7 @@ def test_method(self, input_params, method_result, result_cmp_func, method_param
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!!!")
Expand All @@ -162,6 +164,7 @@ def test_unknown_method(self, signer: JWTSigner):
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": {}}
Expand All @@ -174,6 +177,7 @@ def test_unknown_method(self, signer: JWTSigner):
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": {}}
Expand All @@ -194,13 +198,14 @@ def test_missing_token(self):
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": {}}
Expand All @@ -209,3 +214,24 @@ def test_invalid_token(self, signer: JWTSigner):
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))