PYCO-93: Add client certificate (mTLS) authenticator#14
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds client-certificate (mTLS) support to the Couchbase Analytics Python client by introducing certificate-backed Credential construction (PEM keypair or PKCS#12 bundle) and wiring certificate material into the TLS handshake (no Authorization header).
Changes:
- Introduces
Credential.from_certificate(...)and internal SSL-context application logic (PEM viaSSLContext.load_cert_chain, PKCS#12 viacryptographydecode + temporary PEM). - Updates sync/async client adapters to rebuild the
httpxclient on cert rotation and roll back on failures. - Adds
cryptography>=41.0runtime dependency, docs updates, and unit tests for cert creation/rotation and error paths.
Reviewed changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Locks new runtime dependency set including cryptography and transitive deps (cffi, pycparser). |
| requirements.in | Adds cryptography>=41.0 to runtime inputs. |
| requirements.txt | Updates compiled runtime pins/markers to include cryptography (+ transitive deps). |
| pyproject.toml | Declares cryptography>=41.0 as a project dependency. |
| docs/couchbase_analytics_api/credential.rst | Exposes Credential.from_certificate in sync API docs. |
| docs/acouchbase_analytics_api/credential.rst | Exposes Credential.from_certificate in async API docs. |
| couchbase_analytics/common/credential.py | Adds cert credential kind, PKCS#12 handling, request/SSL-context application logic, and repr redaction. |
| couchbase_analytics/protocol/connection.py | Applies cert material to SSL context during security validation. |
| couchbase_analytics/protocol/_core/client_adapter.py | Refactors client creation and rebuilds the httpx.Client on cert rotation with rollback. |
| couchbase_analytics/cluster.py | Updates set_credential docstring to include cert rotation. |
| couchbase_analytics/tests/credential_t.py | Adds mTLS/PKCS#12 construction, repr, https-only, rotation, and rollback tests (sync). |
| acouchbase_analytics/protocol/_core/client_adapter.py | Refactors client creation and rebuilds the httpx.AsyncClient on cert rotation with rollback. |
| acouchbase_analytics/cluster.py | Updates set_credential docstring to include cert rotation. |
| acouchbase_analytics/tests/credential_t.py | Adds mTLS/PKCS#12 construction, repr, https-only, rotation, and rollback tests (async). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
thejcfactor
reviewed
May 8, 2026
thejcfactor
reviewed
May 8, 2026
thejcfactor
reviewed
May 8, 2026
Comment on lines
+146
to
+149
| self._kind = 'cert' | ||
| self._cert_path = validate_path(cert_path) | ||
| self._key_path = validate_path(key_path) if key_path is not None else None | ||
| self._cert_password = cert_password |
Changes ------- * Adds Credential.from_certificate(cert_path, key_path=None, password=None); authenticates during the TLS handshake. No Authorization header is sent. * One factory, two file layouts: PEM cert + PEM key (pass both paths), or a PKCS#12 bundle (pass only cert_path, with optional password). * Cert credentials require an https:// endpoint; http URLs are rejected at construction. * Adds cryptography>=41.0 as a runtime dependency. Stdlib ssl has no PKCS#12 reader, so the bundle is decoded via cryptography and written to a PEM tempfile that load_cert_chain consumes. The unencrypted key sits on disk only between mkstemp/write and unlink. * Cluster.set_credential() (sync + async) rebuilds the httpx Client when the new credential is a cert, since the cert chain is pinned to the SSL context at Client construction. The new Client is built before the old one is closed, so a concurrent send_request never sees self._client missing. * Cert rotation rolls back on failure: if validate_security_options or _build_client raises (malformed PEM, wrong PKCS#12 password), the previous SSL context is restored and the credential holder is left unchanged. * Cross-format rotation between PEM and PKCS#12 is allowed; both share _kind == 'cert'. * Adds unit tests for construction, repr redaction, https-only enforcement, wrong-password failure, cross-format rotation, and the failure rollback path.
Changes ------- * Moves Credential._check_replaceable_with to the top of update_credential (sync + async) so the type check runs before any state changes, on every rotation. * Drops the duplicate _check_replaceable_with from _CredentialHolder.replace; both callers validate first. * Refactors update_credential to put the holder swap and old-client close in the shared path. The cert branch now only handles the SSL-context refresh and Client rebuild. * Logs a warning when the PKCS#12 temporary PEM cleanup fails, instead of swallowing the OSError. The cluster request still succeeds since the SSL context is already loaded. * Tightens the cryptography pin from >=41.0 to ~=47.0. Regenerates requirements.txt and uv.lock. * Tidies up two comments (Credential.__slots__ rationale, cert chain hook in connection.py).
Changes ------- * Replaces two `assert self._cert_path is not None` lines in Credential with explicit if-raise guards, since `python -O` strips asserts and bandit B101 flags them. * Wraps the dispatched-header lookup in the test helper with str() so mypy stops returning Any from the str-typed function. * Adds `[mypy-cryptography.*] ignore_missing_imports = True` to mypy.ini. The cryptography package doesn't ship stubs for the pkcs12 / serialization namespace. * Adds `# type: ignore[import-not-found]` to `import tomli` in couchbase_analytics_version.py, matching the existing ignore on `import tomli_w` two lines below. The pre-commit mypy hook runs in an isolated env that doesn't include tomli.
thejcfactor
approved these changes
May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes