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

Request to add custom SSL certificate verification #23

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

malakhalifa0
Copy link

@malakhalifa0 malakhalifa0 commented Sep 10, 2024

Issue #24

Currently, watttime/api.py does not specify custom SSL certificate verification when making requests to the WattTime API, instead, it uses the system defaults. For enhanced security and compatibility with specific certificate requirements, we need to update the function to use a custom certificate location for SSL verification.

This would be really useful in the cases where we would need to trust a specific certificate and would improve the security and compatibility of the API login process.

@malakhalifa0 malakhalifa0 changed the title adding verify parameter for ssl verification Request to add custom SSL certificate verification Sep 10, 2024
@sam-watttime
Copy link
Contributor

Hi @malakhalifa0, thanks for the PR! It looks like tests are failing because self.certificate_location is never set. Could you add an optional argument to the init of WattTimeBase for this?

@xginn8 any thoughts on what the default value for that argument should be?

@malakhalifa0
Copy link
Author

Hi @malakhalifa0, thanks for the PR! It looks like tests are failing because self.certificate_location is never set. Could you add an optional argument to the init of WattTimeBase for this?

@xginn8 any thoughts on what the default value for that argument should be?

Thanks for your feedback! For some reason self.certificate_location wasn't initialised in this branch, merging it with main updated the code properly. Let me know if the tests pass now. Thanks!

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.

2 participants