JSCO-68: Analytics SDK - Support for mTLS#19
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds mTLS client-certificate authentication support alongside existing password and JWT credentials.
Changes:
- Adds
CertificateCredentialwith PKCS#12 and PEM credential options. - Updates HTTP client behavior to use TLS client certs and omit
Authorizationfor certificate auth. - Adds validation/tests for mTLS usage, credential rotation, and public exports.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
lib/credential.ts |
Adds certificate credential type and extends ClusterCredential. |
lib/httpclient.ts |
Applies certificate material to HTTPS agents and blocks mTLS over HTTP. |
lib/cluster.ts |
Accepts CertificateCredential in credential validation. |
lib/analytics.ts |
Exports CertificateCredential from the public API barrel. |
test/credential.test.ts |
Adds tests for certificate construction, HTTP rejection, rotation, and headers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| /** | ||
| * Replace the credential used for subsequent HTTP requests, for example to |
There was a problem hiding this comment.
Lets update this doc to mention the client cert rotation as well.
| }) | ||
| } | ||
| const tlsOptions = this._buildTlsOptions(securityOptions) | ||
| const tlsOptions = this._buildTlsOptions(this._securityOptions) |
There was a problem hiding this comment.
Since the http client owns _securityOptions. I wonder if we can just have _buildTlsOptions() do: const securityOptions = this._securityOptions instead of needing to passing it as an argument?
| assert.throws(() => new CertificateCredential({}), InvalidArgumentError) | ||
| }) | ||
|
|
||
| it('rejects cert without key', function () { |
There was a problem hiding this comment.
I think it would be nice to also have a unit test that confirms we reject the key if we don't have a cert.
thejcfactor
left a comment
thejcfactor
left a comment
There was a problem hiding this comment.
Just a couple of small comments/thoughts. Looking good though!
JSCO-68: mTLS client-certificate auth
new CertificateCredential({ pfx: Buffer, passphrase?: string })
new CertificateCredential({ cert: string|Buffer, key: string|Buffer, passphrase?: string })
type ClusterCredential = Credential | JwtCredential | CertificateCredential
cluster.setCredential(newCertCred) // same-type only
Adds a third credential variant. The constructor takes either PKCS#12
(pfx) or PEM (cert + key); both, or neither, throws. mTLS requires an
https:// endpoint; http:// is rejected.
Cert credentials don't send an Authorization header — auth happens
during the TLS handshake. setCredential supports cert-cert rotation;
cross-type rotation throws.
JSCO-68: mTLS client-certificate auth
new CertificateCredential({ pfx: Buffer, passphrase?: string })
new CertificateCredential({ cert: string|Buffer, key: string|Buffer, passphrase?: string })
type ClusterCredential = Credential | JwtCredential | CertificateCredential
cluster.setCredential(newCertCred) // same-type only
Adds a third credential variant. The constructor takes either PKCS#12 (pfx) or PEM (cert + key); both, or neither, throws. mTLS requires an https:// endpoint; http:// is rejected.
Cert credentials don't send an Authorization header — auth happens during the TLS handshake. setCredential supports cert-cert rotation; cross-type rotation throws.