From af14871bc9855f1b90e996fad53a600687112288 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20K=C3=B6ssler?= Date: Mon, 5 Aug 2024 16:08:51 +0200 Subject: [PATCH] Improve test coverage, add linting to CI --- .github/ISSUE_TEMPLATE/bug_report.md | 2 +- .github/workflows/quality.yml | 19 ++++++++ .vscode/extensions.json | 2 +- .vscode/settings.json | 10 ++-- biome.json | 68 ++++++++++++++-------------- package.json | 7 +-- src/index.ts | 33 ++++++++------ src/tls.ts | 2 +- test/domains.test.ts | 16 +++++-- test/errors.test.ts | 18 +++++++- test/le-revoked.test.ts | 9 +++- 11 files changed, 119 insertions(+), 67 deletions(-) create mode 100644 .github/workflows/quality.yml diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index ebcd5f7..0790c4b 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -1,7 +1,7 @@ --- name: Bug report about: Create a bug report -title: '[Bug]' +title: 'Bug: ' labels: bug assignees: timokoessler --- diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml new file mode 100644 index 0000000..1e65799 --- /dev/null +++ b/.github/workflows/quality.yml @@ -0,0 +1,19 @@ +name: Code quality + +on: + push: + pull_request: + +jobs: + quality: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Checkout repository ⬇️ + uses: actions/checkout@v4 + - name: Setup Biome ⚙️ + uses: biomejs/setup-biome@v2 + with: + version: latest + - name: Run Biome 🚀 + run: biome ci . \ No newline at end of file diff --git a/.vscode/extensions.json b/.vscode/extensions.json index 699ed73..5f44864 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -1,3 +1,3 @@ { - "recommendations": ["biomejs.biome"] + "recommendations": ["biomejs.biome"] } diff --git a/.vscode/settings.json b/.vscode/settings.json index 4512dae..a4b1da5 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,7 +1,7 @@ { - "editor.defaultFormatter": "biomejs.biome", - "editor.formatOnSave": true, - "[typescript]": { - "editor.defaultFormatter": "biomejs.biome" - } + "editor.defaultFormatter": "biomejs.biome", + "editor.formatOnSave": true, + "[typescript]": { + "editor.defaultFormatter": "biomejs.biome" + } } diff --git a/biome.json b/biome.json index 30e13dc..fcd2a37 100644 --- a/biome.json +++ b/biome.json @@ -1,38 +1,38 @@ { - "$schema": "./node_modules/@biomejs/biome/configuration_schema.json", - "organizeImports": { - "enabled": false - }, - "linter": { - "enabled": true, - "rules": { - "recommended": true, - "correctness": { - "noUndeclaredVariables": "error", - "noUnusedImports": "error" - }, - "suspicious": { - "noConsoleLog": "error", - "noEmptyBlockStatements": "error", - "useAwait": "error" - } - } - }, - "formatter": { - "enabled": true, - "indentWidth": 4, - "indentStyle": "space", - "lineWidth": 140 - }, - "javascript": { + "$schema": "./node_modules/@biomejs/biome/configuration_schema.json", + "organizeImports": { + "enabled": false + }, + "linter": { + "enabled": true, + "rules": { + "recommended": true, + "correctness": { + "noUndeclaredVariables": "error", + "noUnusedImports": "error" + }, + "suspicious": { + "noConsoleLog": "error", + "noEmptyBlockStatements": "error", + "useAwait": "error" + } + } + }, "formatter": { - "quoteStyle": "single", - "semicolons": "always", - "indentStyle": "space", - "indentWidth": 4 + "enabled": true, + "indentWidth": 4, + "indentStyle": "space", + "lineWidth": 140 + }, + "javascript": { + "formatter": { + "quoteStyle": "single", + "semicolons": "always", + "indentStyle": "space", + "indentWidth": 4 + } + }, + "files": { + "ignore": ["dist", "node_modules", "coverage", "docs", "examples"] } - }, - "files": { - "ignore": ["dist", "node_modules", "coverage", "docs"] - } } diff --git a/package.json b/package.json index acb22a8..e631748 100644 --- a/package.json +++ b/package.json @@ -22,12 +22,7 @@ "type": "git", "url": "git+https://github.com/timokoessler/easy-ocsp.git" }, - "keywords": [ - "ocsp", - "ocsp client", - "X.509", - "certificate" - ], + "keywords": ["ocsp", "ocsp client", "X.509", "certificate"], "engines": { "node": ">=18" }, diff --git a/src/index.ts b/src/index.ts index 09c139b..c39a179 100644 --- a/src/index.ts +++ b/src/index.ts @@ -94,20 +94,24 @@ export type OCSPStatusResponse = { * This function is used internally to download the issuer certificate if it is not provided in the config * Its exported for convenience if you want to download the issuer certificate manually for some reason * @param cert The certificate to download the issuer certificate for - * @param config Additional configuration + * @param timeout Optional timeout in milliseconds for the request. Default is 6000ms * @returns A pkijs.Certificate object of the issuer certificate */ export async function downloadIssuerCert( cert: string | Buffer | X509Certificate | pkijs.Certificate, - config: OCSPStatusConfig, + timeout?: number, ): Promise { + let _timeoutMs = 6000; + if (typeof timeout === 'number') { + _timeoutMs = timeout; + } const { issuerUrl } = getCAInfoUrls(convertToPkijsCert(cert)); const ac = new AbortController(); - const timeout = setTimeout(() => ac.abort(), config.timeout); + const _timeout = setTimeout(() => ac.abort(), _timeoutMs); const res = await fetch(issuerUrl, { signal: ac.signal, }); - clearTimeout(timeout); + clearTimeout(_timeout); if (!res.ok) { throw new Error(`Issuer certificate download failed with status ${res.status} ${res.statusText} ${issuerUrl}`); } @@ -157,7 +161,7 @@ async function sendOCSPRequest(cert: string | Buffer | X509Certificate | pkijs.C let issuerCertificate: pkijs.Certificate; if (!config.ca) { - issuerCertificate = await downloadIssuerCert(certificate, config); + issuerCertificate = await downloadIssuerCert(certificate, config.timeout); } else { issuerCertificate = convertToPkijsCert(config.ca); } @@ -192,10 +196,10 @@ async function sendOCSPRequest(cert: string | Buffer | X509Certificate | pkijs.C * @throws AbortError if the request timed out */ export async function getCertStatus(cert: string | Buffer | X509Certificate | pkijs.Certificate, config?: OCSPStatusConfig) { - config = { ...defaultConfig, ...config }; + const _config = { ...defaultConfig, ...config }; - const { response, certificate, issuerCertificate, nonce } = await sendOCSPRequest(cert, config); - return parseOCSPResponse(response, certificate, issuerCertificate, config, nonce); + const { response, certificate, issuerCertificate, nonce } = await sendOCSPRequest(cert, _config); + return parseOCSPResponse(response, certificate, issuerCertificate, _config, nonce); } /** @@ -207,19 +211,20 @@ export async function getCertStatus(cert: string | Buffer | X509Certificate | pk * @throws AbortError if the request timed out */ export async function getCertStatusByDomain(domain: string, config?: OCSPStatusConfig) { + let _domain = domain; let timeout = 6000; if (config && typeof config.timeout === 'number') { timeout = config.timeout; } - if (domain.includes('/')) { + if (_domain.includes('/')) { try { - const url = new URL(domain); - domain = url.hostname; + const url = new URL(_domain); + _domain = url.hostname; } catch (e) { throw new Error('Invalid URL'); } } - return getCertStatus(await downloadCert(domain, timeout), config); + return getCertStatus(await downloadCert(_domain, timeout), config); } /** @@ -230,9 +235,9 @@ export async function getCertStatusByDomain(domain: string, config?: OCSPStatusC * @returns The raw OCSP response as a buffer, the nonce and the pem encoded issuer certificate */ export async function getRawOCSPResponse(cert: string | Buffer | X509Certificate | pkijs.Certificate, config?: OCSPStatusConfig) { - config = { ...defaultConfig, ...config }; + const _config = { ...defaultConfig, ...config }; - const { response, issuerCertificate, nonce } = await sendOCSPRequest(cert, config); + const { response, issuerCertificate, nonce } = await sendOCSPRequest(cert, _config); return { rawResponse: response, diff --git a/src/tls.ts b/src/tls.ts index a991cbb..fdc06f9 100644 --- a/src/tls.ts +++ b/src/tls.ts @@ -2,7 +2,7 @@ import { connect as tlsConnect, type ConnectionOptions } from 'node:tls'; /** * Get a TLS certificate by hostname. This function will always connect to port 443. - * @param hostname Hostname to connect to (e.g. 'github.com') + * @param hostname Hostname to connect to (e.g. 'github.com') - not an URL * @param timeout Timeout in milliseconds (default: 6000) * @returns Buffer containing the raw certificate (DER) * @throws AbortError if the request timed out diff --git a/test/domains.test.ts b/test/domains.test.ts index f4be801..db7018f 100644 --- a/test/domains.test.ts +++ b/test/domains.test.ts @@ -1,5 +1,5 @@ -import {expect, describe, test} from '@jest/globals'; -import { getCertStatusByDomain } from '../src/index'; +import { expect, describe, test } from '@jest/globals'; +import { downloadCert, getCertStatus, getCertStatusByDomain } from '../src/index'; const domains = [ 'timokoessler.de', @@ -18,15 +18,25 @@ const domains = [ 'www.godaddy.com', 'www.rapidssl.com', 'www.entrust.com', + 'https://tkoessler.de', ]; describe('Get certificate status by domain', () => { for (const domain of domains) { test(domain, async () => { - const response = await getCertStatusByDomain(domain); + const response = await getCertStatusByDomain(domain, { + timeout: 10000, + }); expect(response.status).toBe('good'); expect(response.revocationTime).toBe(undefined); expect(response.producedAt).toBeInstanceOf(Date); }); } + + test('Download cert should lead to same result', async () => { + const response = await getCertStatus(await downloadCert('timokoessler.de')); + expect(response.status).toBe('good'); + expect(response.revocationTime).toBe(undefined); + expect(response.producedAt).toBeInstanceOf(Date); + }); }); diff --git a/test/errors.test.ts b/test/errors.test.ts index b775e46..90a9e52 100644 --- a/test/errors.test.ts +++ b/test/errors.test.ts @@ -1,5 +1,5 @@ import { expect, beforeAll, test } from '@jest/globals'; -import { getCertStatus, getCertURLs } from '../src'; +import { downloadIssuerCert, getCertStatus, getCertStatusByDomain, getCertURLs } from '../src'; import { readCertFile } from './test-helper'; let leCert: string; @@ -60,3 +60,19 @@ test('Wrong ocsp server', async () => { test('Expired certificate', async () => { await expect(getCertStatus(leStagingExpired)).rejects.toThrow('The certificate is already expired'); }); + +test('Invalid url', async () => { + await expect(getCertStatusByDomain('test:// invalid %')).rejects.toThrow('Invalid URL'); +}); + +test('Invalid domain', async () => { + await expect(getCertStatusByDomain('enotfound.example.com')).rejects.toThrow('getaddrinfo ENOTFOUND enotfound.example.com'); +}); + +test('Abort getCertStatus', async () => { + await expect(getCertStatus(leCert, { timeout: 0 })).rejects.toThrow('This operation was aborted'); +}); + +test('Aboirt download issuer cert', async () => { + await expect(downloadIssuerCert(leCert, 0)).rejects.toThrow('This operation was aborted'); +}); diff --git a/test/le-revoked.test.ts b/test/le-revoked.test.ts index 1b05559..dd2850d 100644 --- a/test/le-revoked.test.ts +++ b/test/le-revoked.test.ts @@ -1,7 +1,8 @@ import { expect, test, beforeAll } from '@jest/globals'; import { X509Certificate } from 'node:crypto'; -import { getCertStatus, getCertURLs, getRawOCSPResponse, OCSPRevocationReason } from '../src/index'; +import { downloadIssuerCert, getCertStatus, getCertURLs, getRawOCSPResponse, OCSPRevocationReason } from '../src/index'; import { readCertFile } from './test-helper'; +import { convertToPkijsCert } from '../src/convert'; let cert: string; let intermediateCA: string; @@ -72,3 +73,9 @@ test('Get raw response', async () => { expect(result.nonce).toBeInstanceOf(Buffer); expect(result.issuerCert.replace(/[\n\r]/g, '')).toEqual(intermediateCA.replace(/[\n\r]/g, '')); }); + +test('Download issuer cert', async () => { + const issuerCert = await downloadIssuerCert(cert); + const expectedIssuerCert = convertToPkijsCert(intermediateCA); + expect(issuerCert.toJSON()).toEqual(expectedIssuerCert.toJSON()); +});