Fix Logto callback SSL error: extend LOGTO_SKIP_SSL_VERIFY patch to PyJWKClient (JWKS/urllib)
The 'Fail to fetch data from the url' callback error came from PyJWT's PyJWKClient.fetch_data() using urllib to retrieve the JWKS, which is not covered by the existing aiohttp.ClientSession SSL monkey-patch. Extend _apply_logto_ssl_patch() to also replace PyJWKClient inside logto.OidcCore with a subclass that injects the non-verifying ssl.SSLContext via the ssl_context constructor parameter, ensuring both the OIDC discovery/ token requests (aiohttp) and ID-token JWKS verification (urllib) honour LOGTO_SKIP_SSL_VERIFY=True. Agent-Logs-Url: https://github.com/christianlouis/dmarq/sessions/14676b1a-3421-4839-9ba3-8229a3e5adf1 Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
@@ -45,17 +45,31 @@ _SESSION_MAX_AGE = 86_400 # 24 hours
|
|||||||
|
|
||||||
def _apply_logto_ssl_patch() -> None:
|
def _apply_logto_ssl_patch() -> None:
|
||||||
"""
|
"""
|
||||||
If ``LOGTO_SKIP_SSL_VERIFY`` is ``True``, monkey-patch ``aiohttp.ClientSession``
|
If ``LOGTO_SKIP_SSL_VERIFY`` is ``True``, monkey-patch both
|
||||||
so that every session created by the Logto SDK uses a non-verifying SSL connector.
|
``aiohttp.ClientSession`` and the ``PyJWKClient`` used by the Logto SDK so
|
||||||
|
that every connection to the Logto OIDC endpoint skips SSL certificate
|
||||||
|
verification.
|
||||||
|
|
||||||
The Logto SDK creates its own ``aiohttp.ClientSession`` objects internally and
|
Two patches are applied:
|
||||||
provides no mechanism to inject an SSL context. Replacing the class at module
|
|
||||||
level is the only way to propagate the setting without forking the SDK.
|
|
||||||
|
|
||||||
**Scope note:** ``aiohttp`` is not used anywhere else in this application – only
|
1. **aiohttp.ClientSession** – The Logto SDK creates its own
|
||||||
the Logto SDK pulls it in. If additional code in this repository starts using
|
``aiohttp.ClientSession`` objects internally (for the OIDC discovery
|
||||||
``aiohttp`` directly, review whether those connections should also skip
|
document and token-endpoint requests) and provides no mechanism to
|
||||||
verification before enabling this setting.
|
inject an SSL context. Replacing the class at module level is the
|
||||||
|
only way to propagate the setting without forking the SDK.
|
||||||
|
|
||||||
|
2. **PyJWKClient** inside ``logto.OidcCore`` – The Logto SDK uses
|
||||||
|
``PyJWKClient`` (from PyJWT) to fetch and verify the JWKS for
|
||||||
|
ID-token signature validation. ``PyJWKClient`` uses ``urllib``
|
||||||
|
internally, *not* ``aiohttp``, so the first patch does not cover it.
|
||||||
|
We replace the ``PyJWKClient`` reference in the ``logto.OidcCore``
|
||||||
|
module so that every ``OidcCore`` instance gets a client that passes
|
||||||
|
the non-verifying SSL context to ``urllib``.
|
||||||
|
|
||||||
|
**Scope note:** ``aiohttp`` is not used anywhere else in this application
|
||||||
|
– only the Logto SDK pulls it in. If additional code in this repository
|
||||||
|
starts using ``aiohttp`` directly, review whether those connections should
|
||||||
|
also skip verification before enabling this setting.
|
||||||
|
|
||||||
.. warning::
|
.. warning::
|
||||||
Disabling SSL verification removes protection against man-in-the-middle
|
Disabling SSL verification removes protection against man-in-the-middle
|
||||||
@@ -76,6 +90,9 @@ def _apply_logto_ssl_patch() -> None:
|
|||||||
ssl_ctx.check_hostname = False
|
ssl_ctx.check_hostname = False
|
||||||
ssl_ctx.verify_mode = ssl.CERT_NONE
|
ssl_ctx.verify_mode = ssl.CERT_NONE
|
||||||
|
|
||||||
|
# ── Patch 1: aiohttp.ClientSession ───────────────────────────────────────
|
||||||
|
# Covers OIDC discovery-document and token-endpoint requests.
|
||||||
|
|
||||||
_OriginalClientSession = aiohttp.ClientSession
|
_OriginalClientSession = aiohttp.ClientSession
|
||||||
|
|
||||||
class _NoVerifyClientSession(_OriginalClientSession): # type: ignore[misc]
|
class _NoVerifyClientSession(_OriginalClientSession): # type: ignore[misc]
|
||||||
@@ -89,6 +106,29 @@ def _apply_logto_ssl_patch() -> None:
|
|||||||
|
|
||||||
aiohttp.ClientSession = _NoVerifyClientSession # type: ignore[assignment]
|
aiohttp.ClientSession = _NoVerifyClientSession # type: ignore[assignment]
|
||||||
|
|
||||||
|
# ── Patch 2: PyJWKClient inside logto.OidcCore ────────────────────────────
|
||||||
|
# Covers JWKS fetching for ID-token signature verification.
|
||||||
|
# PyJWKClient uses urllib internally, so Patch 1 does not cover it.
|
||||||
|
try:
|
||||||
|
import logto.OidcCore as _oidc_module # noqa: PLC0415
|
||||||
|
from jwt import PyJWKClient as _OrigPyJWKClient # noqa: PLC0415
|
||||||
|
|
||||||
|
class _NoVerifyPyJWKClient(_OrigPyJWKClient): # type: ignore[misc]
|
||||||
|
"""``PyJWKClient`` subclass that injects a non-verifying SSL context."""
|
||||||
|
|
||||||
|
def __init__(self, *args, **kwargs) -> None: # type: ignore[override]
|
||||||
|
kwargs.setdefault("ssl_context", ssl_ctx)
|
||||||
|
super().__init__(*args, **kwargs)
|
||||||
|
|
||||||
|
_oidc_module.PyJWKClient = _NoVerifyPyJWKClient # type: ignore[attr-defined]
|
||||||
|
except Exception as _exc: # pylint: disable=broad-exception-caught
|
||||||
|
logger.warning(
|
||||||
|
"Failed to patch PyJWKClient for LOGTO_SKIP_SSL_VERIFY: %s. "
|
||||||
|
"JWKS fetching will still verify SSL certificates, which may cause "
|
||||||
|
"ID-token verification to fail when using a self-signed certificate.",
|
||||||
|
_exc,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
_apply_logto_ssl_patch()
|
_apply_logto_ssl_patch()
|
||||||
|
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ These tests exercise:
|
|||||||
- /api/v1/auth/me – authenticated and unauthenticated
|
- /api/v1/auth/me – authenticated and unauthenticated
|
||||||
- /api/v1/auth/sign-in – Logto not configured → 503
|
- /api/v1/auth/sign-in – Logto not configured → 503
|
||||||
- /api/v1/auth/sign-out – always clears the session cookie
|
- /api/v1/auth/sign-out – always clears the session cookie
|
||||||
|
- SSL bypass patching (_apply_logto_ssl_patch)
|
||||||
|
|
||||||
All tests use the in-memory SQLite fixture from conftest.py.
|
All tests use the in-memory SQLite fixture from conftest.py.
|
||||||
Logto SDK calls are mocked so no live Logto instance is needed.
|
Logto SDK calls are mocked so no live Logto instance is needed.
|
||||||
@@ -329,3 +330,93 @@ class TestStaticAssetBypass:
|
|||||||
res = client.get("/dashboard", follow_redirects=False)
|
res = client.get("/dashboard", follow_redirects=False)
|
||||||
assert res.status_code == 302
|
assert res.status_code == 302
|
||||||
assert res.headers["location"].startswith("/login")
|
assert res.headers["location"].startswith("/login")
|
||||||
|
|
||||||
|
|
||||||
|
# ── SSL bypass patch ──────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
class TestApplyLogtoSslPatch:
|
||||||
|
"""_apply_logto_ssl_patch should extend both the aiohttp and PyJWKClient patches."""
|
||||||
|
|
||||||
|
def test_no_patch_when_ssl_verify_enabled(self):
|
||||||
|
"""When LOGTO_SKIP_SSL_VERIFY is False the function must not modify aiohttp."""
|
||||||
|
import aiohttp
|
||||||
|
|
||||||
|
original = aiohttp.ClientSession
|
||||||
|
|
||||||
|
mock_settings = MagicMock()
|
||||||
|
mock_settings.LOGTO_SKIP_SSL_VERIFY = False
|
||||||
|
|
||||||
|
with patch("app.core.logto.settings", mock_settings):
|
||||||
|
from app.core.logto import _apply_logto_ssl_patch
|
||||||
|
|
||||||
|
_apply_logto_ssl_patch()
|
||||||
|
|
||||||
|
assert aiohttp.ClientSession is original
|
||||||
|
|
||||||
|
def test_aiohttp_patched_when_ssl_skip_enabled(self):
|
||||||
|
"""When LOGTO_SKIP_SSL_VERIFY is True the aiohttp.ClientSession must be replaced."""
|
||||||
|
import aiohttp
|
||||||
|
|
||||||
|
original = aiohttp.ClientSession
|
||||||
|
|
||||||
|
mock_settings = MagicMock()
|
||||||
|
mock_settings.LOGTO_SKIP_SSL_VERIFY = True
|
||||||
|
|
||||||
|
with patch("app.core.logto.settings", mock_settings):
|
||||||
|
from app.core.logto import _apply_logto_ssl_patch
|
||||||
|
|
||||||
|
_apply_logto_ssl_patch()
|
||||||
|
|
||||||
|
try:
|
||||||
|
assert aiohttp.ClientSession is not original
|
||||||
|
finally:
|
||||||
|
# Restore so later tests are not affected.
|
||||||
|
aiohttp.ClientSession = original
|
||||||
|
|
||||||
|
def test_pyjwkclient_patched_when_ssl_skip_enabled(self):
|
||||||
|
"""When LOGTO_SKIP_SSL_VERIFY is True, PyJWKClient in logto.OidcCore must be
|
||||||
|
replaced with a subclass that injects a non-verifying ssl_context."""
|
||||||
|
import logto.OidcCore as _oidc_module
|
||||||
|
from jwt import PyJWKClient
|
||||||
|
|
||||||
|
original_pyjwkclient = _oidc_module.PyJWKClient
|
||||||
|
|
||||||
|
mock_settings = MagicMock()
|
||||||
|
mock_settings.LOGTO_SKIP_SSL_VERIFY = True
|
||||||
|
|
||||||
|
with patch("app.core.logto.settings", mock_settings):
|
||||||
|
from app.core.logto import _apply_logto_ssl_patch
|
||||||
|
|
||||||
|
_apply_logto_ssl_patch()
|
||||||
|
|
||||||
|
try:
|
||||||
|
patched = _oidc_module.PyJWKClient
|
||||||
|
assert patched is not PyJWKClient, "PyJWKClient should be replaced"
|
||||||
|
assert issubclass(patched, PyJWKClient), "Replacement must subclass PyJWKClient"
|
||||||
|
finally:
|
||||||
|
_oidc_module.PyJWKClient = original_pyjwkclient
|
||||||
|
|
||||||
|
def test_pyjwkclient_patch_injects_ssl_context(self):
|
||||||
|
"""The patched PyJWKClient must pass ssl_context to its parent when constructed."""
|
||||||
|
import ssl
|
||||||
|
|
||||||
|
import logto.OidcCore as _oidc_module
|
||||||
|
|
||||||
|
original_pyjwkclient = _oidc_module.PyJWKClient
|
||||||
|
|
||||||
|
mock_settings = MagicMock()
|
||||||
|
mock_settings.LOGTO_SKIP_SSL_VERIFY = True
|
||||||
|
|
||||||
|
with patch("app.core.logto.settings", mock_settings):
|
||||||
|
from app.core.logto import _apply_logto_ssl_patch
|
||||||
|
|
||||||
|
_apply_logto_ssl_patch()
|
||||||
|
|
||||||
|
try:
|
||||||
|
instance = _oidc_module.PyJWKClient("https://example.com/.well-known/jwks.json")
|
||||||
|
assert instance.ssl_context is not None
|
||||||
|
assert isinstance(instance.ssl_context, ssl.SSLContext)
|
||||||
|
assert instance.ssl_context.verify_mode == ssl.CERT_NONE
|
||||||
|
finally:
|
||||||
|
_oidc_module.PyJWKClient = original_pyjwkclient
|
||||||
|
|||||||
Reference in New Issue
Block a user