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 e4c3ecf8ce Disable default allowing the testing of connections in UI,
API and CLI (#32052)
e4c3ecf8ce is described below
commit e4c3ecf8ceaefa17525b495e4bcb5b2f41309603
Author: Pankaj Koti <[email protected]>
AuthorDate: Tue Jul 4 19:29:29 2023 +0530
Disable default allowing the testing of connections in UI, API and CLI
(#32052)
* Disable allowing by default testing of connnections in UI
Users can enable test connection functionaility in UI with caution
by setting the `enable_test_connection` key to `True` in the
`[webserver]` section of airflow.cfg or by setting the environment
variable `AIRFLOW__WEBSERVER__ENABLE_TEST_CONNECTION` to `True`.
---
.../api_connexion/endpoints/connection_endpoint.py | 10 +++++++++-
airflow/cli/commands/connection_command.py | 7 +++++++
airflow/config_templates/config.yml | 19 +++++++++++++++----
airflow/config_templates/default_airflow.cfg | 11 +++++++++++
airflow/www/extensions/init_jinja_globals.py | 1 +
airflow/www/static/js/connection_form.js | 21 +++++++++++++++++++++
airflow/www/templates/airflow/conn_create.html | 1 +
airflow/www/templates/airflow/conn_edit.html | 1 +
newsfragments/32052.significant.rst | 17 +++++++++++++++++
.../endpoints/test_connection_endpoint.py | 16 ++++++++++++++++
tests/cli/commands/test_connection_command.py | 14 +++++++++++++-
tests/www/views/test_views.py | 1 +
12 files changed, 113 insertions(+), 6 deletions(-)
diff --git a/airflow/api_connexion/endpoints/connection_endpoint.py
b/airflow/api_connexion/endpoints/connection_endpoint.py
index 737ee54d6c..cfb185d44b 100644
--- a/airflow/api_connexion/endpoints/connection_endpoint.py
+++ b/airflow/api_connexion/endpoints/connection_endpoint.py
@@ -20,7 +20,7 @@ import os
from http import HTTPStatus
from connexion import NoContent
-from flask import request
+from flask import Response, request
from marshmallow import ValidationError
from sqlalchemy import func, select
from sqlalchemy.orm import Session
@@ -36,6 +36,7 @@ from airflow.api_connexion.schemas.connection_schema import (
connection_test_schema,
)
from airflow.api_connexion.types import APIResponse, UpdateMask
+from airflow.configuration import conf
from airflow.models import Connection
from airflow.secrets.environment_variables import CONN_ENV_PREFIX
from airflow.security import permissions
@@ -180,6 +181,13 @@ def test_connection() -> APIResponse:
env var, as some hook classes tries to find out the conn from their
__init__ method & errors out
if not found. It also deletes the conn id env variable after the test.
"""
+ if conf.get("core", "test_connection",
fallback="Disabled").lower().strip() != "enabled":
+ return Response(
+ "Testing connections is disabled in Airflow configuration. Contact
your deployment admin to "
+ "enable it.",
+ 403,
+ )
+
body = request.json
transient_conn_id = get_random_string()
conn_env_var = f"{CONN_ENV_PREFIX}{transient_conn_id.upper()}"
diff --git a/airflow/cli/commands/connection_command.py
b/airflow/cli/commands/connection_command.py
index 6d137bc896..63888c0ec9 100644
--- a/airflow/cli/commands/connection_command.py
+++ b/airflow/cli/commands/connection_command.py
@@ -30,6 +30,7 @@ from sqlalchemy.orm import exc
from airflow.cli.simple_table import AirflowConsole
from airflow.compat.functools import cache
+from airflow.configuration import conf
from airflow.exceptions import AirflowNotFoundException
from airflow.hooks.base import BaseHook
from airflow.models import Connection
@@ -343,6 +344,12 @@ def _import_helper(file_path: str, overwrite: bool) ->
None:
def connections_test(args) -> None:
"""Test an Airflow connection."""
console = AirflowConsole()
+ if conf.get("core", "test_connection",
fallback="Disabled").lower().strip() != "enabled":
+ console.print(
+ "[bold yellow]\nTesting connections is disabled in Airflow
configuration. "
+ "Contact your deployment admin to enable it.\n"
+ )
+ raise SystemExit(1)
print(f"Retrieving connection: {args.conn_id!r}")
try:
diff --git a/airflow/config_templates/config.yml
b/airflow/config_templates/config.yml
index 88aa055a39..1ad8705a70 100644
--- a/airflow/config_templates/config.yml
+++ b/airflow/config_templates/config.yml
@@ -450,6 +450,21 @@ core:
type: string
default: ~
example: 'http://localhost:8080'
+ test_connection:
+ description: |
+ The ability to allow testing connections across Airflow UI, API and
CLI.
+ Supported options: Disabled, Enabled, Hidden. Default: Disabled
+ Disabled - Disables the test connection functionality and disables the
Test Connection button in UI.
+ Enabled - Enables the test connection functionality and shows the Test
Connection button in UI.
+ Hidden - Disables the test connection functionality and hides the Test
Connection button in UI.
+ Before setting this to Enabled, make sure that you review the users
who are able to add/edit
+ connections and ensure they are trusted. Connection testing can be
done maliciously leading to
+ undesired and insecure outcomes. For more information on capabilities
of users, see the documentation:
+
https://airflow.apache.org/docs/apache-airflow/stable/security/index.html#capabilities-of-authenticated-ui-users
+ version_added: 2.7.0
+ type: string
+ example: ~
+ default: "Disabled"
database:
description: ~
options:
@@ -891,8 +906,6 @@ logging:
type: boolean
example: ~
default: "False"
-
-
metrics:
description: |
StatsD (https://github.com/etsy/statsd) integration settings.
@@ -1801,7 +1814,6 @@ webserver:
type: string
example: "sha256"
default: "md5"
-
email:
description: |
Configuration email backend and whether to
@@ -1859,7 +1871,6 @@ email:
type: string
example: "Airflow <[email protected]>"
default: ~
-
smtp:
description: |
If you want airflow to send emails on retries, failure, and you want to use
diff --git a/airflow/config_templates/default_airflow.cfg
b/airflow/config_templates/default_airflow.cfg
index c28155ac2c..0cc99a3f4e 100644
--- a/airflow/config_templates/default_airflow.cfg
+++ b/airflow/config_templates/default_airflow.cfg
@@ -257,6 +257,17 @@ database_access_isolation = False
# Example: internal_api_url = http://localhost:8080
# internal_api_url =
+# The ability to allow testing connections across Airflow UI, API and CLI.
+# Supported options: Disabled, Enabled, Hidden. Default: Disabled
+# Disabled - Disables the test connection functionality and disables the Test
Connection button in UI.
+# Enabled - Enables the test connection functionality and shows the Test
Connection button in UI.
+# Hidden - Disables the test connection functionality and hides the Test
Connection button in UI.
+# Before setting this to Enabled, make sure that you review the users who are
able to add/edit
+# connections and ensure they are trusted. Connection testing can be done
maliciously leading to
+# undesired and insecure outcomes. For more information on capabilities of
users, see the documentation:
+#
https://airflow.apache.org/docs/apache-airflow/stable/security/index.html#capabilities-of-authenticated-ui-users
+test_connection = Disabled
+
[database]
# Path to the ``alembic.ini`` file. You can either provide the file path
relative
# to the Airflow home directory or the absolute path if it is located
elsewhere.
diff --git a/airflow/www/extensions/init_jinja_globals.py
b/airflow/www/extensions/init_jinja_globals.py
index 2d5778af4b..0674a8e4a3 100644
--- a/airflow/www/extensions/init_jinja_globals.py
+++ b/airflow/www/extensions/init_jinja_globals.py
@@ -68,6 +68,7 @@ def init_jinja_globals(app):
"git_version": git_version,
"k8s_or_k8scelery_executor": IS_K8S_OR_K8SCELERY_EXECUTOR,
"rest_api_enabled": False,
+ "config_test_connection": conf.get("core", "test_connection",
fallback="Disabled"),
}
backends = conf.get("api", "auth_backends")
diff --git a/airflow/www/static/js/connection_form.js
b/airflow/www/static/js/connection_form.js
index 41df55dc07..453be58411 100644
--- a/airflow/www/static/js/connection_form.js
+++ b/airflow/www/static/js/connection_form.js
@@ -23,6 +23,9 @@
/* global document, DOMParser, $, CodeMirror */
import { getMetaValue } from "./utils";
+const configTestConnection = getMetaValue("config_test_connection")
+ .toLowerCase()
+ .trim();
const restApiEnabled = getMetaValue("rest_api_enabled") === "True";
const connectionTestUrl = getMetaValue("test_url");
@@ -126,6 +129,24 @@ function applyFieldBehaviours(connection) {
*/
function handleTestConnection(connectionType, testableConnections) {
const testButton = document.getElementById("test-connection");
+
+ if (configTestConnection === "hidden") {
+ // If test connection is hidden in config, hide button and return.
+ $(testButton).hide();
+ return;
+ }
+ if (configTestConnection === "disabled") {
+ // If test connection is not enabled in config, disable button and display
toolip
+ // alerting the user.
+ $(testButton)
+ .prop("disabled", true)
+ .attr(
+ "title",
+ "Testing connections is disabled in Airflow configuration. Contact
your deployment admin to enable it."
+ );
+ return;
+ }
+
const testConnEnabled = testableConnections.includes(connectionType);
if (testConnEnabled) {
diff --git a/airflow/www/templates/airflow/conn_create.html
b/airflow/www/templates/airflow/conn_create.html
index 34c2f32ded..ac92b967f7 100644
--- a/airflow/www/templates/airflow/conn_create.html
+++ b/airflow/www/templates/airflow/conn_create.html
@@ -21,6 +21,7 @@
{% block head_css %}
{{ super() }}
+<meta name="config_test_connection" content="{{ config_test_connection }}">
<meta name="rest_api_enabled" content="{{ rest_api_enabled }}">
<meta name="test_url"
content="{{
url_for('/api/v1.airflow_api_connexion_endpoints_connection_endpoint_test_connection')
}}">
diff --git a/airflow/www/templates/airflow/conn_edit.html
b/airflow/www/templates/airflow/conn_edit.html
index e963b8ac0c..653b0dd3ce 100644
--- a/airflow/www/templates/airflow/conn_edit.html
+++ b/airflow/www/templates/airflow/conn_edit.html
@@ -21,6 +21,7 @@
{% block head_css %}
{{ super() }}
+<meta name="config_test_connection" content="{{ config_test_connection }}">
<meta name="rest_api_enabled" content="{{ rest_api_enabled }}">
<meta name="test_url"
content="{{
url_for('/api/v1.airflow_api_connexion_endpoints_connection_endpoint_test_connection')
}}">
diff --git a/newsfragments/32052.significant.rst
b/newsfragments/32052.significant.rst
new file mode 100644
index 0000000000..d4ec9a934f
--- /dev/null
+++ b/newsfragments/32052.significant.rst
@@ -0,0 +1,17 @@
+Disable default allowing the testing of connections in UI, API and CLI
+
+The test connection functionality is disabled by default across Airflow UI,
+API and CLI. The availability of the functionality can be controlled by the
+``test_connection`` flag in the ``core`` section of the Airflow
+configuration (``airflow.cfg``). It can also be controlled by the
+environment variable ``AIRFLOW__CORE__TEST_CONNECTION``.
+The following values are accepted for this config param:
+1. ``Disabled``: Disables the test connection functionality and
+disables(greys out) the Test Connection button in the UI.
+This is also the default value set in the Airflow configuration.
+2. ``Enabled``: Enables the test connection functionality and
+activates the Test Connection button in the UI.
+3. ``Hidden``: Disables the test connection functionality and
+hides the Test Connection button in UI.
+For more information on capabilities of users, see the documentation:
+https://airflow.apache.org/docs/apache-airflow/stable/security/index.html#capabilities-of-authenticated-ui-users
diff --git a/tests/api_connexion/endpoints/test_connection_endpoint.py
b/tests/api_connexion/endpoints/test_connection_endpoint.py
index bce7150f8b..de0b7aad97 100644
--- a/tests/api_connexion/endpoints/test_connection_endpoint.py
+++ b/tests/api_connexion/endpoints/test_connection_endpoint.py
@@ -16,6 +16,9 @@
# under the License.
from __future__ import annotations
+import os
+from unittest import mock
+
import pytest
from airflow.api_connexion.exceptions import EXCEPTIONS_LINK_MAP
@@ -603,6 +606,7 @@ class TestPostConnection(TestConnectionEndpoint):
class TestConnection(TestConnectionEndpoint):
+ @mock.patch.dict(os.environ, {"AIRFLOW__CORE__TEST_CONNECTION": "Enabled"})
def test_should_respond_200(self):
payload = {"connection_id": "test-connection-id", "conn_type":
"sqlite"}
response = self.client.post(
@@ -614,6 +618,7 @@ class TestConnection(TestConnectionEndpoint):
"message": "Connection successfully tested",
}
+ @mock.patch.dict(os.environ, {"AIRFLOW__CORE__TEST_CONNECTION": "Enabled"})
def test_post_should_respond_400_for_invalid_payload(self):
payload = {
"connection_id": "test-connection-id",
@@ -635,3 +640,14 @@ class TestConnection(TestConnectionEndpoint):
)
assert_401(response)
+
+ def test_should_respond_403_by_default(self):
+ payload = {"connection_id": "test-connection-id", "conn_type":
"sqlite"}
+ response = self.client.post(
+ "/api/v1/connections/test", json=payload,
environ_overrides={"REMOTE_USER": "test"}
+ )
+ assert response.status_code == 403
+ assert response.text == (
+ "Testing connections is disabled in Airflow configuration. "
+ "Contact your deployment admin to enable it."
+ )
diff --git a/tests/cli/commands/test_connection_command.py
b/tests/cli/commands/test_connection_command.py
index 42f6fc8cc2..12b3de6170 100644
--- a/tests/cli/commands/test_connection_command.py
+++ b/tests/cli/commands/test_connection_command.py
@@ -18,6 +18,7 @@ from __future__ import annotations
import io
import json
+import os
import re
import shlex
import warnings
@@ -938,6 +939,7 @@ class TestCliTestConnections:
def setup_class(self):
clear_db_connections()
+ @mock.patch.dict(os.environ, {"AIRFLOW__CORE__TEST_CONNECTION": "Enabled"})
@mock.patch("airflow.providers.http.hooks.http.HttpHook.test_connection")
def test_cli_connections_test_success(self, mock_test_conn):
"""Check that successful connection test result is displayed
properly."""
@@ -948,6 +950,7 @@ class TestCliTestConnections:
assert "Connection success!" in stdout.getvalue()
+ @mock.patch.dict(os.environ, {"AIRFLOW__CORE__TEST_CONNECTION": "Enabled"})
@mock.patch("airflow.providers.http.hooks.http.HttpHook.test_connection")
def test_cli_connections_test_fail(self, mock_test_conn):
"""Check that failed connection test result is displayed properly."""
@@ -958,9 +961,18 @@ class TestCliTestConnections:
assert "Connection failed!\nFailed.\n\n" in stdout.getvalue()
+ @mock.patch.dict(os.environ, {"AIRFLOW__CORE__TEST_CONNECTION": "Enabled"})
def test_cli_connections_test_missing_conn(self):
"""Check a connection test on a non-existent connection raises a
"Connection not found" message."""
with redirect_stdout(io.StringIO()) as stdout,
pytest.raises(SystemExit):
connection_command.connections_test(self.parser.parse_args(["connections",
"test", "missing"]))
+ assert "Connection not found.\n\n" in stdout.getvalue()
- assert "Connection not found.\n\n" in stdout.getvalue()
+ def test_cli_connections_test_disabled_by_default(self):
+ """Check that test connection functionality is disabled by default."""
+ with redirect_stdout(io.StringIO()) as stdout,
pytest.raises(SystemExit):
+
connection_command.connections_test(self.parser.parse_args(["connections",
"test", "missing"]))
+ assert (
+ "Testing connections is disabled in Airflow configuration. Contact
your deployment admin to "
+ "enable it.\n\n"
+ ) in stdout.getvalue()
diff --git a/tests/www/views/test_views.py b/tests/www/views/test_views.py
index e266b9b8c1..755df3b7aa 100644
--- a/tests/www/views/test_views.py
+++ b/tests/www/views/test_views.py
@@ -84,6 +84,7 @@ def test_redoc_should_render_template(capture_templates,
admin_client):
assert len(templates) == 1
assert templates[0].name == "airflow/redoc.html"
assert templates[0].local_context == {
+ "config_test_connection": "Disabled",
"openapi_spec_url": "/api/v1/openapi.yaml",
"rest_api_enabled": True,
"get_docs_url": get_docs_url,
