Skip to content

added httplib as parameter#12

Open
databill86 wants to merge 1 commit intosvintit:masterfrom
databill86:master
Open

added httplib as parameter#12
databill86 wants to merge 1 commit intosvintit:masterfrom
databill86:master

Conversation

@databill86
Copy link

@databill86 databill86 commented Feb 24, 2021

I've added http as a parameter to prevent the creation of a new Http() for every request.
This way, we can also disable ssl certificate verification:
oidc = OpenIDConnect(app, http=httplib2.Http(disable_ssl_certificate_validation=True))

@OneWithTheCore
Copy link

I think adding the ability to add an Http object is worthwhile, but disabling certificate verification any point is definitely a very bad idea, especially in an authentication flow. Self-signed certificates would be the only even remote exception, but you shouldn't be using a self-signed certificate on an authentication server, anyway.

warn("HTTP argument is deprecated and unused", DeprecationWarning)
self.http = http
else:
self.http = httplib2.Http()
Copy link

@aaronjolson aaronjolson Mar 15, 2022

Choose a reason for hiding this comment

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

Why use self.http? Why does it need to be part of the class instance? Why not leave the scope local?

@svintit
Copy link
Owner

svintit commented Mar 16, 2022

I agree with @OneWithTheCore, disabling certificate verification is not an option we want to enable.

@databill86 Regarding the HTTP parameter being set on the class instance, what use case do you have that this would be helpful with?

@jangaraj
Copy link

jangaraj commented Jan 3, 2023

Disabling certificate verification is definitely a bad idea, but here it is only a option, which is is disabled by default (so default setup is a secure setup).
I would like to have it, because I'm in the env where I have deep packet inspection, so infosec replaces certs with own certs and it is a problem to sync certs in WSL2 with host OS, where Infosec certs are available.

@aaronjolson
Copy link

The Requests library has a verify=False optional parameter to skip SSL verification. I can imagine there being situations where skipping verification of SSL certs is necessary/useful.

@fredericoschardong
Copy link

This PR solves my issue. I have a self-signed certificate for my OIDC OP, which I could easily make it trustable with:

oidc = OpenIDConnect(app, http=httplib2.Http(ca_certs='...'))

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants