Skip to content

Raise error when sending credentials over HTTP#593

Open
hashhar wants to merge 1 commit intotrinodb:masterfrom
hashhar:hashhar/improve-http-scheme
Open

Raise error when sending credentials over HTTP#593
hashhar wants to merge 1 commit intotrinodb:masterfrom
hashhar:hashhar/improve-http-scheme

Conversation

@hashhar
Copy link
Copy Markdown
Member

@hashhar hashhar commented Mar 6, 2026

Description

Raises a TrinoAuthError when authentication is configured while using an insecure HTTP connection. This aligns with the Java client behavior where TLS/SSL is strictly required for authentication — the Java client raises a RuntimeException with the message "TLS/SSL is required for authentication" in the same scenario.

The error message follows the same phrasing as the Java client and includes Python-specific guidance on how to switch to HTTPS.

Non-technical explanation

Connecting with authentication over plain HTTP is now an error. Use https:// in the host URL or pass http_scheme='https'.

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
(x) Release notes are required, with the following suggested text:

* Raise `TrinoAuthError` when authentication credentials are used over an insecure HTTP connection. ({issue}`593`)

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a runtime warning when authentication is configured while using an insecure HTTP connection, to help prevent accidental credential exposure.

Changes:

  • Emit a UserWarning in Connection.__init__ when auth is set and the resolved scheme is HTTP.
  • Add unit tests asserting the warning is produced for HTTP and not produced for HTTPS.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
trino/dbapi.py Adds warning emission when auth is used over HTTP.
tests/unit/test_dbapi.py Adds/updates tests to validate the new warning behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread trino/dbapi.py Outdated
Comment thread trino/dbapi.py
Comment thread tests/unit/test_dbapi.py Outdated
@hashhar hashhar force-pushed the hashhar/improve-http-scheme branch from 828a7c7 to ae59fe2 Compare March 6, 2026 19:15
@findepi
Copy link
Copy Markdown
Member

findepi commented Mar 9, 2026

Adds warning when sending credentials over HTTP. JDBC actually disallows this entirely but it can be a breaking change for Python client now.

Can we imagine a future where Python client does unsafe thing by default in 10 years from now?
I suppose this is not what we want. This means we need to embrace the breakage.

Do you plan to convert warning into failure in a 1-2 release from now?
If not, let's make it failure today.

@hashhar
Copy link
Copy Markdown
Member Author

hashhar commented Mar 10, 2026

I plan to convert to failure. Just want to see if there are known applications or setups which explode and provide a version they can pin to while they sort out insecure configs.

Aligns with the Java client behavior where TLS/SSL is required for
authentication. The error message matches the Java client phrasing:
"TLS/SSL is required for authentication."

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@hashhar hashhar force-pushed the hashhar/improve-http-scheme branch from ae59fe2 to 1b981b4 Compare April 15, 2026 18:42
@hashhar hashhar changed the title Add warning when sending credentials over HTTP Raise error when sending credentials over HTTP Apr 15, 2026
@hashhar
Copy link
Copy Markdown
Member Author

hashhar commented Apr 15, 2026

@findepi made into an error, was near useless in current shape - i tested with some tools locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants