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

[Feature] Add SSL Certificate Context And Setting For Async/Sync HTTP Requests #6976

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

deeleeramone
Copy link
Contributor

@deeleeramone deeleeramone commented Nov 29, 2024

  1. Why?:

The Requests library accepts an environment variable, REQUESTS_CA_BUNDLE, and AIOHTTP does not. This creates inconsistencies between synchronous and async HTTP requests. Furthermore, defining this variable limits the Requests library to that particular file. If you want to use a self-signed certificate for only some things, it is not straightforward.

This solution combines the specified certificate with the certifi defaults.

  1. What?:

    • Allows a CA certificate file to be specified for verifying HTTPS requests.
      • Self-signed certificates do not need to be part of the system's trust store.
    • Allows "python_settings" in system_settings.json to accept extras.
      • Uses keys http as a nested dictionary.
    • Applies the configuration to the internal helpers:
      • make_request
      • amake_request
      • amake_requests
    • Handles the scenarios where libraries are using the Requests library directly. This is done by manipulating environment variables before and after making the request.
      • yFinance
      • Finviz
      • Posthog
      • OpenBB Hub
    • Allows keyword arguments to be passed to uvicorn.run by storing them as a dictionary to: system_settings.python_settings.uvicorn
  2. Impact:

    • Should have absolutely no impact unless these items are specified.
    • The behavior of Requests environment variable, REQUESTS_CA_BUNDLE, ports directly to the async requests.
    • Allows it to be defined in system_settings.json instead of environment variables.
    • cafile is an equivalent to REQUESTS_CA_BUNDLE, when pointing to a file.
    • When "verify_ssl": false, SSL certificate verification is disabled within all OpenBB functions.
    • Keyword arguments supplied to the "python_settings["uvicorn"]" dictionary are passed directly to uvicorn.run when launching the API as:
      • python -m openbb_core.api.rest_api
      • openbb-api

These items, in system_settings.json, will take precedence over environment variables:

{
    "python_settings": {
        "http": {
            "cafile": "/full/path/to/certificate/localhost.crt",
            "verify_ssl": null,
            "proxy": null,
            "certfile": null,
            "keyfile": null,
        }
    }
}

certfile and keyfile are intended more for future use, the most important key is cafile.

Note: Keyword arguments added to the command line from openbb-api take precedence over the system_settings.json file.

  1. Testing Done:

    • Create a self-signed certificate and start an OpenBB API over HTTPS
    • Add the full path to the .crt file as shown above.
    • Import both requests and openbb_core.provider.utils.helpers.make_request
      • requests.get should fail while make_request succeeds.
      • Set as an environment variable instead, "cafile" is null and add REQUESTS_CA_BUNDLE='/full/path/to/certificate/localhost.crt' to the .env file.
        • Both requests.get and make_request should succeed.
        • Making an outside call using requests.get("https://google.com") will fail, make_request should succeed.

Screenshot 2024-11-29 at 3 01 25 PM

With the environment variable defined, and not system_settings.json, the same A/B can be applied to yfinance.download() vs. `obb.equity.price.historical(provider="yfinance")

This fails because yFinance is only verifying against the self-signed certificate for localhost. openbb_yfinance.utils.helpers.yf_download applies the environment configuration for the duration of the request.

Screenshot 2024-11-29 at 3 07 01 PM

@deeleeramone deeleeramone added enhancement Enhancement platform OpenBB Platform v4 PRs for v4 labels Nov 29, 2024
@deeleeramone deeleeramone added the do not merge Label to prevent pull request merge label Nov 29, 2024
@deeleeramone deeleeramone removed the do not merge Label to prevent pull request merge label Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement platform OpenBB Platform v4 PRs for v4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG][FR] Impossible to specify SSL certification
1 participant