[PR] Add opt-in allow_insecure_auth extra to Trino connection [airflow]

["ccoliu (via GitHub)" <gi...@apache.org>]

ccoliu opened a new pull request, #74085:
URL: https://github.com/apache/airflow/pull/74085

   Starting with `trino` 0.339.0, the Python client refuses to send credentials over plain HTTP (`TrinoAuthError: TLS/SSL is required for authentication`). `trino` 0.340.0 added an `allow_insecure_auth` parameter for setups where the connection is encrypted below the application layer (e.g. an mTLS service-mesh sidecar such as Istio), but `TrinoHook` had no way to pass it, so password auth over `http` (including the UI "Test connection" button) could not work.
   
   This adds an opt-in `allow_insecure_auth` connection extra (default `false`). It is only passed to `trino.dbapi.connect()` when enabled, so behavior is unchanged for everyone else and older `trino` versions without the parameter keep working. It is deliberately not enabled by default, since that would allow credentials to be sent in cleartext.
   
   Verified against the real client (not mocked), basic auth over `http`:
   
   | trino | extra not set | `allow_insecure_auth: true` |
   |---|---|---|
   | 0.339.0 | `TrinoAuthError` | `TypeError` (param not supported; docs note `trino>=0.340.0` is required) |
   | 0.340.0 | `TrinoAuthError` (secure default kept) | connection created |
   
   Tests: added `test_get_conn_allow_insecure_auth` covering unset / false / true (bool and string). The `true` cases fail without the fix. Docs: documented the new extra in `connections.rst`.
   
   closes: #74080
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes (please specify the tool below)
   
   Generated-by: Claude Code (Claude Opus 5.5) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Add opt-in allow_insecure_auth extra to Trino connection [airflow]

["ccoliu (via GitHub)" <gi...@apache.org>]

ccoliu commented on PR #74085:
URL: https://github.com/apache/airflow/pull/74085#issuecomment-5989999358

   Thanks for the review, agreed. To answer your question: no, the hook can't verify it. Encryption below the application layer (e.g. an Istio mTLS sidecar) is transparent to the client, which only sees a plain `http` connection. That's why it's opt-in and the operator has to enable it explicitly.
   
   I've pushed an update that:
   
   - logs a warning each time a connection with `allow_insecure_auth` enabled is created, stating that the HTTPS requirement is disabled and credentials may be sent in cleartext if the transport isn't protected, and
   - adds a `.. warning::` block to the connection docs saying the same, including that Airflow cannot verify the lower-layer encryption.
   
   The tests also assert the warning is logged only when the option is enabled.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Add opt-in allow_insecure_auth extra to Trino connection [airflow]

["ccoliu (via GitHub)" <gi...@apache.org>]

ccoliu commented on PR #74085:
URL: https://github.com/apache/airflow/pull/74085#issuecomment-6490147975

   Thanks @jason810496! Tested end to end with a real setup:
   
   - **Trino 484** (`trinodb/trino`) with file-based password authentication.
   - An **nginx** sidecar in front of it standing in for the mesh: the client talks plain HTTP to it, and it forwards to Trino with `X-Forwarded-Proto: https` (Trino runs with `http-server.process-forwarded=true`).
   - **Airflow 3.3.2** with the trino provider built from this branch and `trino==0.340.0`.
   - Two connections to `http://trino-mesh:8080` with the same login/password, one without and one with `allow_insecure_auth`.
   
   **1. Without `allow_insecure_auth`: Test Connection fails with the error from #74080**
   
   <img width="1500" height="900" alt="test_connection_1_trino_http_password" src="https://github.com/user-attachments/assets/5feb59fb-0528-40d3-8102-969052489279" />
   
   **2. With `allow_insecure_auth: true`: Test Connection succeeds**
   
   <img width="1500" height="900" alt="test_connection_2_trino_mtls_mesh" src="https://github.com/user-attachments/assets/f18a1675-82ce-4ff8-861e-308a971180b5" />
   
   **3. `SQLExecuteQueryOperator` against the same connection: the query runs as the authenticated user, and the new warning is logged**
   
   <img width="1500" height="900" alt="task_log" src="https://github.com/user-attachments/assets/0f5dc3e0-f484-46dc-a0f7-53857b298444" />
   
   
   With a wrong password the same request gets `401 Invalid credentials` from Trino, so the password is really checked by the coordinator.
   
   While testing I also found that the coordinator hint in the docs was wrong (I had taken it from the trino client's error message): with `http-server.authentication.allow-insecure-over-http=true` alone, Trino still rejects passwords over HTTP (`Password not allowed for insecure authentication`). What works is `http-server.process-forwarded=true` behind a sidecar or proxy that sends `X-Forwarded-Proto: https`. I pushed a fix to `connections.rst`.
   
   Here is the testing source code, if you need further information:
   ```python
   from __future__ import annotations
   
   import pendulum
   
   from airflow.providers.common.sql.operators.sql import SQLExecuteQueryOperator
   from airflow.sdk import DAG
   
   with DAG(
       "trino_insecure_auth_demo",
       schedule=None,
       start_date=pendulum.datetime(2026, 1, 1, tz="UTC"),
       catchup=False,
   ):
       SQLExecuteQueryOperator(
           task_id="query_trino",
           conn_id="trino_mtls_mesh", # The connection is set to trino_mtls_mesh
           sql="SELECT current_user AS user_name, node_version FROM system.runtime.nodes",
           show_return_value_in_logs=True,
       )
   
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org