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.

Sign up for GitHub

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

Add support for system truststore #9805

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add support for system truststore #9805

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion poetry.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ dependencies = [
"tomlkit (>=0.11.4,<1.0.0)",
# trove-classifiers uses calver, so version is unclamped
"trove-classifiers (>=2022.5.19)",
"truststore (>=0.10.0,<1.0.0) ; python_version >= '3.10'",
"virtualenv (>=20.26.6,<21.0.0)",
"xattr (>=1.0.0,<2.0.0) ; sys_platform == 'darwin'",
]
Expand Down Expand Up @@ -187,6 +188,7 @@ module = [
'shellingham.*',
'virtualenv.*',
'xattr.*',
'truststore.*' # not available on Python < 3.10
]
ignore_missing_imports = true

Expand Down
3 changes: 3 additions & 0 deletions src/poetry/config/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ class Config:
"keyring": {
"enabled": True,
},
# TODO: Flip to default True on the next release after dropping Python 3.9
"system-truststore": False,
}

def __init__(self, use_environment: bool = True) -> None:
Expand Down Expand Up @@ -303,6 +305,7 @@ def _get_normalizer(name: str) -> Callable[[str], Any]:
"solver.lazy-wheel",
"system-git-client",
"keyring.enabled",
"system-truststore",
}:
return boolean_normalizer

Expand Down
10 changes: 10 additions & 0 deletions src/poetry/console/application.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ def _run(self, io: IO) -> int:
self._configure_custom_application_options(io)

self._load_plugins(io)
self._load_system_truststore()

with directory(self._working_directory):
exit_code: int = super()._run(io)
Expand Down Expand Up @@ -441,6 +442,15 @@ def _load_plugins(self, io: IO) -> None:

self._plugins_loaded = True

@staticmethod
def _load_system_truststore() -> None:
from poetry.utils.ssl_truststore import is_truststore_enabled

if is_truststore_enabled():
import truststore

truststore.inject_into_ssl()
Comment on lines +450 to +452
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be wrapped in try/catch for ImportError? We should not need the ssl_truststore module.

Copy link
Member Author

Choose a reason for hiding this comment

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

As you can see, there are several checks done in ssl_truststore module. I wanted to isolate it and avoid cluttering the Application with it.

Copy link
Member

Choose a reason for hiding this comment

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

As I understood it those checks are redundant. If truststore cannot be used an import error will be raised.

Copy link
Member Author

Choose a reason for hiding this comment

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

Simple ImportError is too little in my taste. The _is_truststore_available() function has various checks and logs a proper message.

Copy link
Member

Choose a reason for hiding this comment

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

It might be, but it does not add anything in this context.

https://truststore.readthedocs.io/en/latest/

If Truststore can’t work for a given platform due to APIs not being available then at import time the exception ImportError will be raised with an informative message

The only case where it makes sense is if we need to programmatically check if truststore is available. If that is the case, we might not want inject_into_ssl anyway.

Also note;

The call to truststore.inject_into_ssl() should be called as early as possible in your program as modules that have already imported ssl.SSLContext won’t be affected.

This means this might be too late as imports to repos and authenticator would have already happened.



def main() -> int:
exit_code: int = Application().run()
Expand Down
32 changes: 32 additions & 0 deletions src/poetry/utils/ssl_truststore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
from __future__ import annotations

import logging
import sys

from poetry.config.config import Config


logger = logging.getLogger(__name__)


def _is_truststore_available() -> bool:
if sys.version_info < (3, 10):
logger.debug("Disabling truststore because Python version isn't 3.10+")
return False

try:
import ssl # noqa: F401
except ImportError:
logger.warning("Disabling truststore since ssl support is missing")
return False

try:
import truststore # noqa: F401
except ImportError:
logger.warning("Disabling truststore because `truststore` package is missing`")
return False
return True


def is_truststore_enabled() -> bool:
return Config.create().get("system-truststore") and _is_truststore_available()
6 changes: 5 additions & 1 deletion tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,11 @@ def git_mock(mocker: MockerFixture, request: FixtureRequest) -> None:


@pytest.fixture
def http() -> Iterator[type[httpretty.httpretty]]:
def http(mocker: MockerFixture) -> Iterator[type[httpretty.httpretty]]:
import sys

if sys.version_info >= (3, 10):
mocker.patch("truststore.inject_into_ssl")
httpretty.reset()
with httpretty.enabled(allow_net_connect=False, verbose=True):
yield httpretty
Expand Down
6 changes: 6 additions & 0 deletions tests/console/commands/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def test_list_displays_default_value_if_not_set(
requests.max-retries = 0
solver.lazy-wheel = true
system-git-client = false
system-truststore = false
virtualenvs.create = true
virtualenvs.in-project = null
virtualenvs.options.always-copy = false
Expand Down Expand Up @@ -96,6 +97,7 @@ def test_list_displays_set_get_setting(
requests.max-retries = 0
solver.lazy-wheel = true
system-git-client = false
system-truststore = false
virtualenvs.create = false
virtualenvs.in-project = null
virtualenvs.options.always-copy = false
Expand Down Expand Up @@ -149,6 +151,7 @@ def test_unset_setting(
requests.max-retries = 0
solver.lazy-wheel = true
system-git-client = false
system-truststore = false
virtualenvs.create = true
virtualenvs.in-project = null
virtualenvs.options.always-copy = false
Expand Down Expand Up @@ -180,6 +183,7 @@ def test_unset_repo_setting(
requests.max-retries = 0
solver.lazy-wheel = true
system-git-client = false
system-truststore = false
virtualenvs.create = true
virtualenvs.in-project = null
virtualenvs.options.always-copy = false
Expand Down Expand Up @@ -309,6 +313,7 @@ def test_list_displays_set_get_local_setting(
requests.max-retries = 0
solver.lazy-wheel = true
system-git-client = false
system-truststore = false
virtualenvs.create = false
virtualenvs.in-project = null
virtualenvs.options.always-copy = false
Expand Down Expand Up @@ -349,6 +354,7 @@ def test_list_must_not_display_sources_from_pyproject_toml(
requests.max-retries = 0
solver.lazy-wheel = true
system-git-client = false
system-truststore = false
virtualenvs.create = true
virtualenvs.in-project = null
virtualenvs.options.always-copy = false
Expand Down
Loading