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

FIX: HTTPS Token Auth verify_ssl propagation #128

Merged
merged 3 commits into from Dec 20, 2022

Conversation

domrim
Copy link
Contributor

@domrim domrim commented Dec 18, 2022

I discovered a bug in the https backend code for token auth. The verify_ssl arg was not passed correctly to the auth backend (and so was the timeout arg). I fixed this and implemented some checks to protect against this error for the future.

FYI this bug produced the folowing output at "normal" requests with correct SSL-Certificates on a proxmox host:

from proxmoxer import ProxmoxAPI
proxmox = ProxmoxAPI('proxmox-1.example.com user='demo@pve', token_name='demo', token_value='redacted', verify_ssl=True)
proxmox.nodes.get()
/Users/dominik/code/virtualenvs/proxmox-utils-vBjC0H-5-py3.10/lib/python3.10/site-packages/urllib3/connectionpool.py:1045: InsecureRequestWarning: Unverified HTTPS request is being made to host 'proxmox-1.example.com'. Adding certificate verification is strongly advised. See: https://urllib3.readthedocs.io/en/1.26.x/advanced-usage.html#ssl-warnings
  warnings.warn(
[{'ssl_fingerprint': 'redacted', 'id': 'node/proxmox-1', 'node': 'proxmox-1', 'level': '', 'status': 'online', 'type': 'node'}]

@domrim
Copy link
Contributor Author

domrim commented Dec 18, 2022

And it would be nice if this bugfix could find its way into a release ASAP as it is complicated to explain to users of provided scripts that they can ignore the warning.

@jhollowe jhollowe self-assigned this Dec 19, 2022
@jhollowe jhollowe added type:bug 🐞 The software does not function as intended type:testing 🧪 New or updated tests backend:https labels Dec 19, 2022
@jhollowe
Copy link
Contributor

I'm going to wait to merge until I have time to do a release so I don't forget about getting this out.

I'll hopefully have time after work today to do a patch release. In the mean time, I think this bug was introduced in v2.0.0 so you can pin to an older version until the fix is released.

@nvollmar
Copy link

Had the same warn log issue through pve-exporter. Upgrading to 2.0.1 didn't solve the issue, but downgrading to 1.3.1 did.

@domrim domrim deleted the fix/https-token-auth branch December 29, 2022 15:10
@domrim
Copy link
Contributor Author

domrim commented Dec 29, 2022

I believe @nvollmar has a point here, if verify_ssl is set to False the warning messages don't get disabled. Is this behaviour intended @jhollowe ?

@jhollowe
Copy link
Contributor

This warning message seems to be new in more recent versions of urllib3. If you look at the URL from the warning there is an easy command to disable these warnings.

>>> import urllib3
>>> urllib3.disable_warnings()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:https type:bug 🐞 The software does not function as intended type:testing 🧪 New or updated tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants