PYCO-93: Add client certificate (mTLS) authenticator#14
Open
anirudhlakhotia wants to merge 1 commit intocouchbase:mainfrom
Open
PYCO-93: Add client certificate (mTLS) authenticator#14anirudhlakhotia wants to merge 1 commit intocouchbase:mainfrom
anirudhlakhotia wants to merge 1 commit intocouchbase:mainfrom
Conversation
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.
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.
Comment on lines
+320
to
+324
| finally: | ||
| try: | ||
| os.unlink(tmp_path) | ||
| except OSError: | ||
| pass |
Comment on lines
+37
to
+42
| @pytest.fixture(scope='session') | ||
| def pkcs12_path(cert_paths: Tuple[str, str], tmp_path_factory: pytest.TempPathFactory) -> str: | ||
| """Bundle the session cert + key into a password-encrypted PKCS#12 file.""" | ||
| if shutil.which('openssl') is None: | ||
| pytest.skip('openssl CLI not available; skipping PKCS#12 tests') | ||
| cert, key = cert_paths |
Comment on lines
+37
to
+42
| @pytest.fixture(scope='session') | ||
| def pkcs12_path(cert_paths: Tuple[str, str], tmp_path_factory: pytest.TempPathFactory) -> str: | ||
| """Bundle the session cert + key into a password-encrypted PKCS#12 file.""" | ||
| if shutil.which('openssl') is None: | ||
| pytest.skip('openssl CLI not available; skipping PKCS#12 tests') | ||
| cert, key = cert_paths |
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