Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: pre-request script variable hostname certificate resolution [INS-4733] #8249

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions packages/insomnia-sdk/src/objects/insomnia.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { expect } from 'chai';
import type { ClientCertificate } from 'insomnia/src/models/client-certificate';
import type { RequestHeader } from 'insomnia/src/models/request';
import type { Settings } from 'insomnia/src/models/settings';
import { filterClientCertificates } from 'insomnia/src/network/certificate';

import { toPreRequestAuth } from './auth';
import { CookieObject } from './cookies';
Expand Down Expand Up @@ -167,16 +168,26 @@ export async function initInsomniaObject(
localVars: localVariables,
});

const existClientCert = rawObj.clientCertificates != null && rawObj.clientCertificates.length > 0;
const certificate = existClientCert && rawObj.clientCertificates[0] ?
// replace all variables in the raw URL before parsing it
const sanitizedRawUrl = `${rawObj.request.url}`.replace(/{{\s*\_\./g, '{{');
const renderedRawUrl = variables.replaceIn(sanitizedRawUrl);

// parse the URL to get the host + protocol
const renderedUrl = toUrlObject(renderedRawUrl);

// filter client certificates by the rendered base URL
const renderedBaseUrl = toUrlObject(`${renderedUrl.protocol || 'http:'}//${renderedUrl.getHost()}`);
const filteredCerts = filterClientCertificates(rawObj.clientCertificates || [], renderedBaseUrl.toString());
const existingClientCert = filteredCerts != null && filteredCerts.length > 0 && filteredCerts[0];
const certificate = existingClientCert ?
{
disabled: rawObj.clientCertificates[0].disabled,
disabled: existingClientCert.disabled,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might notice that there seems no perfect way to transform it between 2 sides, I'm thinking if we could just leave the certificate undefined at the beginning, if user specified the cert in the script, we prepend to the cert list, or return the original certs, pls let me know what you think.

Copy link
Contributor Author

@ryan-willis ryan-willis Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd much rather return original certs, but I'm thinking we revamp this later

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we can revamp it later and the prepend operation is for those cases who would like to update the cert through script.

name: 'The first certificate from Settings',
matches: [rawObj.clientCertificates[0].host],
key: { src: rawObj.clientCertificates[0].key || '' },
cert: { src: rawObj.clientCertificates[0].cert || '' },
passphrase: rawObj.clientCertificates[0].passphrase || undefined,
pfx: { src: rawObj.clientCertificates[0].pfx || '' }, // PFX or PKCS12 Certificate
matches: [existingClientCert.host],
key: { src: existingClientCert.key || '' },
cert: { src: existingClientCert.cert || '' },
passphrase: existingClientCert.passphrase || undefined,
pfx: { src: existingClientCert.pfx || '' }, // PFX or PKCS12 Certificate
} :
{ disabled: true };

Expand Down
8 changes: 4 additions & 4 deletions packages/insomnia-sdk/src/objects/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,8 @@ export function mergeClientCertificates(
...baseCertificate,
key: null,
cert: null,
name: updatedReq.certificate.name || '',
disabled: updatedReq.certificate.disabled || false,
passphrase: updatedReq.certificate.passphrase || null,
pfx: updatedReq.certificate.pfx?.src,
}];
Expand All @@ -534,10 +536,8 @@ export function mergeClientCertificates(
modified: 0,
created: 0,
isPrivate: false,
name: updatedReq.name || '',
host: updatedReq.url.getHost() || '',
disabled: updatedReq.disabled || false,

name: updatedReq.certificate.name || '',
disabled: updatedReq.certificate.disabled || false,
key: updatedReq.certificate.key?.src,
cert: updatedReq.certificate.cert?.src,
passphrase: updatedReq.certificate.passphrase || null,
Expand Down
91 changes: 91 additions & 0 deletions packages/insomnia-smoke-test/fixtures/client-certs.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
_type: export
__export_format: 4
__export_date: 2024-12-11T21:49:50.826Z
__export_source: insomnia.desktop.app:v10.2.1-beta.1
resources:
- _id: req_23f466953bff448faaf482e59624b17d
parentId: wrk_78f64a950d1248599ddbfcb2dbe4a4e0
modified: 1733953779068
created: 1733930967254
url: https://localhost:4011/protected/pets/2
name: pet 2
description: ""
method: GET
body: {}
parameters: []
headers:
- name: User-Agent
value: insomnia/10.2.1-beta.1
authentication: {}
preRequestScript: console.log('yippee')
metaSortKey: -1733930967254
isPrivate: false
pathParameters: []
settingStoreCookies: true
settingSendCookies: true
settingDisableRenderRequestBody: false
settingEncodeUrl: true
settingRebuildPath: true
settingFollowRedirects: global
_type: request
- _id: wrk_78f64a950d1248599ddbfcb2dbe4a4e0
parentId: null
modified: 1733930502550
created: 1733930502550
name: client-certs
description: ""
scope: collection
_type: workspace
- _id: req_01d6681fd6434cc7a75c8c4e3deee713
parentId: wrk_78f64a950d1248599ddbfcb2dbe4a4e0
modified: 1733953775398
created: 1733953597632
url: "{{_.srvr}}/protected/pets/2"
name: pet 2 with url var
description: ""
method: GET
body: {}
parameters: []
headers:
- name: User-Agent
value: insomnia/10.2.1-beta.1
authentication: {}
preRequestScript: console.log("yeehaw")
metaSortKey: -1732678181446
isPrivate: false
pathParameters: []
settingStoreCookies: true
settingSendCookies: true
settingDisableRenderRequestBody: false
settingEncodeUrl: true
settingRebuildPath: true
settingFollowRedirects: global
_type: request
- _id: env_1140e4f10f8a7e3ae858474a594d0bc440e35c99
parentId: wrk_78f64a950d1248599ddbfcb2dbe4a4e0
modified: 1733953690814
created: 1733930502551
name: Base Environment
data:
srvr: https://localhost:4011
dataPropertyOrder:
"&":
- srvr
color: null
isPrivate: false
metaSortKey: 1733930502551
environmentType: kv
kvPairData:
- id: envPair_6117101ea3704c85bc3deab101603717
name: srvr
value: https://localhost:4011
type: str
enabled: true
_type: environment
- _id: jar_1140e4f10f8a7e3ae858474a594d0bc440e35c99
parentId: wrk_78f64a950d1248599ddbfcb2dbe4a4e0
modified: 1733953690813
created: 1733930502552
name: Default Jar
cookies: []
_type: cookie_jar
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ resources:
cert: {src: 'invalid.cert'},
passphrase: '',
pfx: {src: ''},
});
});
_type: request
- _id: req_89dade2ee9ee42fbb22d588783a9df3e
parentId: fld_01de564274824ecaad272330339ea6b2
Expand Down
9 changes: 7 additions & 2 deletions packages/insomnia-smoke-test/server/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import crypto from 'node:crypto';

import * as bodyParser from 'body-parser';
import * as cookieParser from 'cookie-parser';
import cookieParser from 'cookie-parser';
import express from 'express';
import { readFileSync } from 'fs';
import { createHandler } from 'graphql-http/lib/use/http';
Expand All @@ -14,11 +14,12 @@ import gitlabApi from './gitlab-api';
import { schema } from './graphql';
import { startGRPCServer } from './grpc';
import insomniaApi from './insomnia-api';
import { mtlsRouter } from './mtls';
import { oauthRoutes } from './oauth';
import { startWebSocketServer } from './websocket';

const app = express();
app.use(cookieParser.default());
app.use(cookieParser());
const port = 4010;
const httpsPort = 4011;
const grpcPort = 50051;
Expand Down Expand Up @@ -64,6 +65,7 @@ app.get('/cookies', (_req, res) => {

app.use('/file', express.static('fixtures/files'));
app.use('/auth/basic', basicAuthRouter);
app.use('/protected', mtlsRouter);

githubApi(app);
gitlabApi(app);
Expand Down Expand Up @@ -129,6 +131,9 @@ startWebSocketServer(app.listen(port, () => {
startWebSocketServer(createServer({
cert: readFileSync(join(__dirname, '../fixtures/certificates/localhost.pem')),
key: readFileSync(join(__dirname, '../fixtures/certificates/localhost-key.pem')),
ca: readFileSync(join(__dirname, '../fixtures/certificates/rootCA.pem')),
requestCert: true,
rejectUnauthorized: false,
}, app).listen(httpsPort, () => {
console.log(`Listening at https://localhost:${httpsPort}`);
console.log(`Listening at wss://localhost:${httpsPort}`);
Expand Down
17 changes: 17 additions & 0 deletions packages/insomnia-smoke-test/server/mtls.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import express, { NextFunction, Response } from 'express';

export const mtlsRouter = express.Router();

mtlsRouter.use(clientCertificateAuth);

async function clientCertificateAuth(req: any, res: Response, next: NextFunction) {
if (!req.client.authorized) {
return res.status(401).send({ error: 'Client certificate required' });
}

next();
}

mtlsRouter.get('/pets/:id', (req, res) => {
res.status(200).send({ id: req.params.id });
});
73 changes: 73 additions & 0 deletions packages/insomnia-smoke-test/tests/smoke/mtls.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@

import path from 'node:path';

import { expect } from '@playwright/test';

import { getFixturePath, loadFixture } from '../../playwright/paths';
import { test } from '../../playwright/test';

test('can use client certificate for mTLS', async ({ app, page }) => {
const statusTag = page.locator('[data-testid="response-status-tag"]:visible');
const responseBody = page.locator('[data-testid="response-pane"] >> [data-testid="CodeEditor"]:visible', {
has: page.locator('.CodeMirror-activeline'),
});

const clientCertsCollectionText = await loadFixture('client-certs.yaml');
await app.evaluate(async ({ clipboard }, text) => clipboard.writeText(text), clientCertsCollectionText);

await page.getByLabel('Import').click();
await page.locator('[data-test-id="import-from-clipboard"]').click();
await page.getByRole('button', { name: 'Scan' }).click();
await page.getByRole('dialog').getByRole('button', { name: 'Import' }).click();
await page.getByLabel('client-certs').click();

await page.getByLabel('Request Collection').getByTestId('pet 2 with url var').press('Enter');

await page.getByRole('button', { name: 'Send', exact: true }).click();
await page.getByText('Error: SSL peer certificate or SSH remote key was not OK').click();

const fixturePath = getFixturePath('certificates');

await page.getByRole('button', { name: 'Add Certificates' }).click();

let fileChooser = page.waitForEvent('filechooser');
await page.getByRole('button', { name: 'Add CA Certificate' }).click();
await (await fileChooser).setFiles(path.join(fixturePath, 'rootCA.pem'));

await page.getByRole('button', { name: 'Done' }).click();
await page.getByRole('button', { name: 'Send', exact: true }).click();

await expect(statusTag).toContainText('401 Unauthorized');
await expect(responseBody).toContainText('Client certificate required');

await page.getByRole('button', { name: 'Add Certificates' }).click();
await page.getByRole('button', { name: 'Add client certificate' }).click();
await page.locator('[name="host"]').fill('localhost');

fileChooser = page.waitForEvent('filechooser');
await page.locator('[data-test-id="add-client-certificate-file-chooser"]').click();
await (await fileChooser).setFiles(path.join(fixturePath, 'client.crt'));

fileChooser = page.waitForEvent('filechooser');
await page.locator('[data-test-id="add-client-certificate-key-file-chooser"]').click();
await (await fileChooser).setFiles(path.join(fixturePath, 'client.key'));

await page.getByRole('button', { name: 'Add certificate' }).click();
await page.getByRole('button', { name: 'Done' }).click();

await page.getByRole('button', { name: 'Send', exact: true }).click();

await expect(statusTag).toContainText('200 OK');
await expect(responseBody).toContainText('"id": "2"');

// ensure disabling the cert actually disables it
await page.getByRole('button', { name: 'Add Certificates' }).click();
await page.locator('[data-test-id="client-certificate-toggle"]').click();
await page.getByRole('button', { name: 'Done' }).click();
await page.getByLabel('Request Collection').getByTestId('pet 2').press('Enter');

await page.getByRole('button', { name: 'Send', exact: true }).click();
await expect(statusTag).toContainText('401 Unauthorized');
await expect(responseBody).toContainText('Client certificate required');

ryan-willis marked this conversation as resolved.
Show resolved Hide resolved
});
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ test.describe('pre-request features tests', async () => {
// update proxy configuration
await page.locator('text=Add Certificates').click();
await page.locator('text=Add client certificate').click();
await page.locator('[name="host"]').fill('a.com');
await page.locator('[name="host"]').fill('127.0.0.1');
await page.locator('[data-key="pfx"]').click();

const fileChooserPromise = page.waitForEvent('filechooser');
Expand Down
2 changes: 1 addition & 1 deletion packages/insomnia/src/network/certificate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { ClientCertificate } from '../models/client-certificate';
import { setDefaultProtocol } from '../utils/url/protocol';
import { urlMatchesCertHost } from './url-matches-cert-host';

export function filterClientCertificates(clientCertificates: ClientCertificate[], requestUrl: string, protocol: string) {
export function filterClientCertificates(clientCertificates: ClientCertificate[], requestUrl: string, protocol?: string) {
const res = clientCertificates.filter(c => !c.disabled && urlMatchesCertHost(setDefaultProtocol(c.host, protocol), requestUrl, true));
// If didn't get a matching certificate at the first time, ignore the port check and try again
if (!res.length) {
Expand Down
5 changes: 3 additions & 2 deletions packages/insomnia/src/network/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ export const tryToExecutePreRequestScript = async (
globals: activeGlobalEnvironment,
userUploadEnvironment,
requestTestResults: new Array<RequestTestResult>(),
transientVariables,
};
}
const joinedScript = [...folderScripts].join('\n');
Expand Down Expand Up @@ -550,8 +551,8 @@ export const tryToInterpolateRequest = async ({
purpose?: RenderPurpose;
extraInfo?: ExtraRenderInfo;
baseEnvironment?: Environment;
userUploadEnvironment?: UserUploadEnvironment;
transientVariables?: Environment;
userUploadEnvironment?: UserUploadEnvironment;
transientVariables?: Environment;
ignoreUndefinedEnvVariable?: boolean;
}
) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ const ClientCertificateGridListItem = ({ certificate }: {
)}
<div className='flex items-center gap-2 h-6'>
<ToggleButton
data-test-id="client-certificate-toggle"
onChange={isSelected => {
updateClientCertificateFetcher.submit({ ...certificate, disabled: !isSelected }, {
action: `/organization/${organizationId}/project/${projectId}/workspace/${workspaceId}/clientcert/update`,
Expand Down
10 changes: 5 additions & 5 deletions packages/insomnia/src/ui/routes/request.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -483,14 +483,14 @@ export const sendActionImplementation = async ({
iterationCount,
transientVariables,
}: {
requestId: string;
requestId: string;
shouldPromptForPathAfterResponse: boolean | undefined;
ignoreUndefinedEnvVariable: boolean | undefined;
testResultCollector?: RunnerContextForRequest;
iteration?: number;
iterationCount?: number;
userUploadEnvironment?: UserUploadEnvironment;
transientVariables?: Environment;
iteration?: number;
iterationCount?: number;
userUploadEnvironment?: UserUploadEnvironment;
transientVariables?: Environment;
}) => {
window.main.startExecution({ requestId });
const requestData = await fetchRequestData(requestId);
Expand Down
Loading