Skip to content
This repository has been archived by the owner on May 5, 2023. It is now read-only.

transport.py: Don't disable SSL cert validation #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kmurphy4
Copy link
Collaborator

@kmurphy4 kmurphy4 commented Dec 9, 2022

This is basically cherry-picks podio#40.

Original commit message from @gmcguire:

Allow normal SSL certificate checking

This fixes issue podio#27. As far as I can tell api.podio.com has a perfectly valid SSL certificate.

Closes https://podio.com/easyesicom/feature-roadmap/apps/bugs/items/22339

gmcguire and others added 2 commits December 8, 2022 19:43
This is basically cherry-picks podio#40.

Original commit message from @gmcguire:

> Allow normal SSL certificate checking
>
> This fixes issue podio#27. As far as I can tell api.podio.com has a perfectly valid SSL certificate.

Closes https://podio.com/easyesicom/feature-roadmap/apps/bugs/items/22339.
@kmurphy4
Copy link
Collaborator Author

kmurphy4 commented Dec 9, 2022

I haven't tested this yet.

@kmurphy4
Copy link
Collaborator Author

kmurphy4 commented Dec 9, 2022

FWIW, I'm not actually able to reproduce the issue: I made a copy of the import-users-to-podio script:

but it seems to work just fine?

kevin.murphy@us-dev-web:~$ python3.8 -m pip freeze | grep pypodio2
pypodio2 @ https://github.com/Everlaw/podio-py/archive/0.2.1+everlaw1.tar.gz
kevin.murphy@us-dev-web:~$ ./kevin.py
<pypodio2.client.Client object at 0x7fce8ee11a90>

@ryanblais
Copy link
Collaborator

The sooner we stop using this fork the better!

This seems like a strict security improvement, so if this is working we should definitely make this change. Thanks for looking into this (or whatever the original issue was...)

@kmurphy4
Copy link
Collaborator Author

Do you still think we should make the change even tho i can't repro the original bug?

@ryanblais
Copy link
Collaborator

Up to you. I'm fine with opportunistically making this change. But I think getting of podio-py is the real solution, which is pretty close to happening AFAICT.

@ryanblais
Copy link
Collaborator

Feel free to merge if you're happy with testing, even if you can't reproduce.

@kmurphy4
Copy link
Collaborator Author

Idea to migrate off of this fork: https://everlaw.aha.io/ideas/ideas/EP-I-5748.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants