From 52fe4730630b20f71cc65bed07b92e93330fbfca Mon Sep 17 00:00:00 2001 From: Gregory Schier Date: Tue, 31 Oct 2017 19:05:35 +0100 Subject: [PATCH] New ClientCertificate model and ability to have private certificates (#555) * Pull out client certificates * Add test for the migration --- app/models/__tests__/workspace.test.js | 56 ++++ app/models/client-certificate.js | 68 ++++ app/models/environment.js | 2 + app/models/index.js | 5 +- app/models/workspace.js | 41 ++- app/network/__tests__/network.test.js | 9 - app/network/network.js | 10 +- app/sync/index.js | 3 +- .../modals/workspace-settings-modal.js | 317 ++++++++++-------- app/ui/components/wrapper.js | 4 + app/ui/containers/app.js | 4 +- app/ui/css/constants/dimensions.less | 2 +- app/ui/index.js | 2 + app/ui/redux/modules/index.js | 3 +- app/ui/redux/selectors.js | 8 + 15 files changed, 373 insertions(+), 161 deletions(-) create mode 100644 app/models/__tests__/workspace.test.js create mode 100644 app/models/client-certificate.js diff --git a/app/models/__tests__/workspace.test.js b/app/models/__tests__/workspace.test.js new file mode 100644 index 0000000000..cbdcf7606d --- /dev/null +++ b/app/models/__tests__/workspace.test.js @@ -0,0 +1,56 @@ +import * as models from '../index'; +import {globalBeforeEach} from '../../__jest__/before-each'; + +describe('migrate()', () => { + beforeEach(globalBeforeEach); + it('migrates client certificates properly', async () => { + const workspace = await models.workspace.create({ + name: 'My Workspace', + certificates: [ + {key: 'key', passphrase: 'mypass'}, + {disabled: true, cert: 'cert'} + ] + }); + + const migratedWorkspace = await models.workspace.migrate(workspace); + const certs = await models.clientCertificate.findByParentId(workspace._id); + + // Delete modified and created so we can assert them + for (const cert of certs) { + expect(typeof cert.modified).toBe('number'); + expect(typeof cert.created).toBe('number'); + delete cert.modified; + delete cert.created; + } + + expect(certs.length).toBe(2); + expect(certs.sort((c1, c2) => c1._id > c2._id ? -1 : 1)).toEqual([{ + _id: 'crt_a262d22b5fa8491c9bd958fba03e301e', + cert: null, + disabled: false, + isPrivate: false, + key: 'key', + parentId: 'wrk_cc1dd2ca4275747aa88199e8efd42403', + passphrase: 'mypass', + pfx: null, + type: 'ClientCertificate' + }, { + _id: 'crt_2e7c268809ee44b8900d5cbbaa7d3a19', + cert: 'cert', + disabled: false, + isPrivate: false, + key: null, + parentId: 'wrk_cc1dd2ca4275747aa88199e8efd42403', + passphrase: null, + pfx: null, + type: 'ClientCertificate' + }]); + + expect(migratedWorkspace.certificates).toBeUndefined(); + + // Make sure we don't create new certs if we migrate again + await models.workspace.migrate(migratedWorkspace); + const certsAgain = await models.clientCertificate.findByParentId(workspace._id); + expect(certsAgain.length).toBe(2); + }); +}); diff --git a/app/models/client-certificate.js b/app/models/client-certificate.js new file mode 100644 index 0000000000..ecd3dd45b4 --- /dev/null +++ b/app/models/client-certificate.js @@ -0,0 +1,68 @@ +// @flow +import * as db from '../common/database'; +import type {BaseModel} from './index'; + +export const name = 'Client Certificate'; +export const type = 'ClientCertificate'; +export const prefix = 'crt'; +export const canDuplicate = true; + +type BaseClientCertificate = { + parentId: string, + host: string, + passphrase: string | null, + cert: string | null, + key: string | null, + pfx: string | null, + disabled: boolean, + + // For sync control + isPrivate: boolean +}; + +export type ClientCertificate = BaseModel & BaseClientCertificate; + +export function init (): BaseClientCertificate { + return { + parentId: '', + host: '', + passphrase: null, + disabled: false, + cert: null, + key: null, + pfx: null, + isPrivate: false + }; +} + +export async function migrate (doc: ClientCertificate) { + return doc; +} + +export function create (patch: Object = {}): Promise { + if (!patch.parentId) { + throw new Error('New ClientCertificate missing `parentId`: ' + JSON.stringify(patch)); + } + + return db.docCreate(type, patch); +} + +export function update (cert: ClientCertificate, patch: Object = {}): Promise { + return db.docUpdate(cert, patch); +} + +export function getById (id: string): Promise { + return db.get(type, id); +} + +export function findByParentId (parentId: string): Promise> { + return db.find(type, {parentId}); +} + +export function remove (cert: ClientCertificate): Promise { + return db.remove(cert); +} + +export function all (): Promise> { + return db.all(type); +} diff --git a/app/models/environment.js b/app/models/environment.js index 415e32ccc7..44a8fedaa3 100644 --- a/app/models/environment.js +++ b/app/models/environment.js @@ -12,6 +12,8 @@ type BaseEnvironment = { name: string, data: Object, color: string | null, + + // For sync control isPrivate: boolean }; diff --git a/app/models/index.js b/app/models/index.js index 713fb9dfab..8f3964db8f 100644 --- a/app/models/index.js +++ b/app/models/index.js @@ -12,6 +12,7 @@ import * as _requestVersion from './request-version'; import * as _requestMeta from './request-meta'; import * as _response from './response'; import * as _oAuth2Token from './o-auth-2-token'; +import * as _clientCertificate from './client-certificate'; import {generateId} from '../common/misc'; export type BaseModel = { @@ -36,6 +37,7 @@ export const requestVersion = _requestVersion; export const requestMeta = _requestMeta; export const response = _response; export const oAuth2Token = _oAuth2Token; +export const clientCertificate = _clientCertificate; export function all () { return [ @@ -51,7 +53,8 @@ export function all () { requestVersion, requestMeta, response, - oAuth2Token + oAuth2Token, + clientCertificate ]; } diff --git a/app/models/workspace.js b/app/models/workspace.js index 58bb8df44c..737f7286cb 100644 --- a/app/models/workspace.js +++ b/app/models/workspace.js @@ -10,14 +10,7 @@ export const canDuplicate = true; type BaseWorkspace = { name: string, - description: string, - certificates: Array<{ - host: string, - passphrase: string, - cert: string, - key: string, - pfx: string - }> + description: string }; export type Workspace = BaseModel & BaseWorkspace; @@ -25,10 +18,7 @@ export type Workspace = BaseModel & BaseWorkspace; export function init () { return { name: 'New Workspace', - description: '', - certificates: [ - // {host, port, cert, key, pfx, passphrase} - ] + description: '' }; } @@ -41,6 +31,7 @@ export async function migrate (doc: Workspace): Promise { } await _ensureDependencies(doc); + doc = await _migrateExtractClientCertificates(doc); return doc; } @@ -82,3 +73,29 @@ async function _ensureDependencies (workspace: Workspace) { await models.cookieJar.getOrCreateForParentId(workspace._id); await models.environment.getOrCreateForWorkspaceId(workspace._id); } + +async function _migrateExtractClientCertificates (workspace: Workspace) { + if (!Array.isArray(workspace.certificates)) { + // Already migrated + return workspace; + } + + for (const cert of workspace.certificates) { + await models.clientCertificate.create({ + parentId: workspace._id, + host: cert.host, + passphrase: cert.passphrase || null, + cert: cert.cert || null, + key: cert.key || null, + pfx: cert.pfx || null, + isPrivate: false + }); + } + + delete workspace.certificates; + + // This will remove the now-missing `certificates` property + await update(workspace, {}); + + return workspace; +} diff --git a/app/network/__tests__/network.test.js b/app/network/__tests__/network.test.js index 2de0a35e6a..961d8552d2 100644 --- a/app/network/__tests__/network.test.js +++ b/app/network/__tests__/network.test.js @@ -75,7 +75,6 @@ describe('actuallySend()', () => { ACCEPT_ENCODING: '', COOKIEFILE: '', FOLLOWLOCATION: true, - MAXREDIRS: -1, HTTPHEADER: [ 'Content-Type: application/json', 'Expect: ', @@ -129,7 +128,6 @@ describe('actuallySend()', () => { ACCEPT_ENCODING: '', COOKIEFILE: '', FOLLOWLOCATION: true, - MAXREDIRS: -1, HTTPHEADER: [ 'Content-Type: application/x-www-form-urlencoded', 'Expect: ', @@ -208,7 +206,6 @@ describe('actuallySend()', () => { CUSTOMREQUEST: 'GET', ACCEPT_ENCODING: '', FOLLOWLOCATION: true, - MAXREDIRS: -1, HTTPHEADER: [ 'Content-Type: application/json', 'Expect: ', @@ -259,7 +256,6 @@ describe('actuallySend()', () => { CUSTOMREQUEST: 'POST', COOKIEFILE: '', FOLLOWLOCATION: true, - MAXREDIRS: -1, HTTPHEADER: [ 'Content-Type: application/octet-stream', 'Expect: ', @@ -316,7 +312,6 @@ describe('actuallySend()', () => { ACCEPT_ENCODING: '', COOKIEFILE: '', FOLLOWLOCATION: true, - MAXREDIRS: -1, CUSTOMREQUEST: 'POST', HTTPHEADER: [ 'Content-Type: multipart/form-data; boundary=X-INSOMNIA-BOUNDARY', @@ -369,7 +364,6 @@ describe('actuallySend()', () => { ACCEPT_ENCODING: '', COOKIEFILE: '', FOLLOWLOCATION: true, - MAXREDIRS: -1, HTTPHEADER: ['content-type: '], NOPROGRESS: false, PROXY: '', @@ -404,7 +398,6 @@ describe('actuallySend()', () => { ACCEPT_ENCODING: '', COOKIEFILE: '', FOLLOWLOCATION: true, - MAXREDIRS: -1, HTTPHEADER: ['content-type: '], NOPROGRESS: false, PROXY: '', @@ -438,7 +431,6 @@ describe('actuallySend()', () => { ACCEPT_ENCODING: '', COOKIEFILE: '', FOLLOWLOCATION: true, - MAXREDIRS: -1, HTTPHEADER: ['content-type: '], NOPROGRESS: false, PROXY: '', @@ -473,7 +465,6 @@ describe('actuallySend()', () => { ACCEPT_ENCODING: '', COOKIEFILE: '', FOLLOWLOCATION: true, - MAXREDIRS: -1, HTTPHEADER: ['content-type: '], NOPROGRESS: false, PROXY: '', diff --git a/app/network/network.js b/app/network/network.js index a15c36ff20..27f694ee4a 100644 --- a/app/network/network.js +++ b/app/network/network.js @@ -132,12 +132,17 @@ export function _actuallySend ( // Set all the basic options setOpt(Curl.option.FOLLOWLOCATION, settings.followRedirects); - setOpt(Curl.option.MAXREDIRS, settings.maxRedirects); setOpt(Curl.option.TIMEOUT_MS, settings.timeout); // 0 for no timeout setOpt(Curl.option.VERBOSE, true); // True so debug function works setOpt(Curl.option.NOPROGRESS, false); // False so progress function works setOpt(Curl.option.ACCEPT_ENCODING, ''); // Auto decode everything + // Set maximum amount of redirects allowed + // NOTE: Setting this to -1 breaks some versions of libcurl + if (settings.maxRedirects > 0) { + setOpt(Curl.option.MAXREDIRS, settings.maxRedirects); + } + // Only set CURLOPT_CUSTOMREQUEST if not HEAD or GET. This is because Curl // See https://curl.haxx.se/libcurl/c/CURLOPT_CUSTOMREQUEST.html switch (renderedRequest.method.toUpperCase()) { @@ -327,7 +332,8 @@ export function _actuallySend ( } // Set client certs if needed - for (const certificate of workspace.certificates) { + const clientCertificates = await models.clientCertificate.findByParentId(workspace._id); + for (const certificate of clientCertificates) { if (certificate.disabled) { continue; } diff --git a/app/sync/index.js b/app/sync/index.js index bedbe053db..8ce2edcec3 100644 --- a/app/sync/index.js +++ b/app/sync/index.js @@ -17,7 +17,8 @@ const WHITE_LIST = { [models.request.type]: true, [models.requestGroup.type]: true, [models.environment.type]: true, - [models.cookieJar.type]: true + [models.cookieJar.type]: true, + [models.clientCertificate.type]: true }; export const logger = new Logger(); diff --git a/app/ui/components/modals/workspace-settings-modal.js b/app/ui/components/modals/workspace-settings-modal.js index 7c8adcfab3..47667320a7 100644 --- a/app/ui/components/modals/workspace-settings-modal.js +++ b/app/ui/components/modals/workspace-settings-modal.js @@ -1,5 +1,5 @@ -import React, {PureComponent} from 'react'; -import PropTypes from 'prop-types'; +// @flow +import * as React from 'react'; import autobind from 'autobind-decorator'; import {Tab, Tabs, TabList, TabPanel} from 'react-tabs'; import DebouncedInput from '../base/debounced-input'; @@ -12,25 +12,56 @@ import PromptButton from '../base/prompt-button'; import * as models from '../../../models/index'; import {trackEvent} from '../../../analytics/index'; import MarkdownEditor from '../markdown-editor'; +import type {Workspace} from '../../../models/workspace'; +import type {ClientCertificate} from '../../../models/client-certificate'; + +type Props = { + clientCertificates: Array, + workspace: Workspace, + editorFontSize: number, + editorIndentSize: number, + editorKeyMap: string, + editorLineWrapping: boolean, + nunjucksPowerUserMode: boolean, + handleRender: Function, + handleGetRenderContext: Function, + handleRemoveWorkspace: Function, + handleDuplicateWorkspace: Function +}; + +type State = { + showAddCertificateForm: boolean, + host: string, + crtPath: string, + keyPath: string, + pfxPath: string, + isPrivate: boolean, + passphrase: string, + showDescription: boolean, + defaultPreviewMode: boolean +}; @autobind -class WorkspaceSettingsModal extends PureComponent { - constructor (props) { +class WorkspaceSettingsModal extends React.PureComponent { + modal: Modal | null; + + constructor (props: Props) { super(props); this.state = { showAddCertificateForm: false, + host: '', crtPath: '', keyPath: '', pfxPath: '', - host: '', passphrase: '', + isPrivate: false, showDescription: false, defaultPreviewMode: false }; } - _workspaceUpdate (patch) { + _workspaceUpdate (patch: Object) { models.workspace.update(this.props.workspace, patch); } @@ -39,7 +70,7 @@ class WorkspaceSettingsModal extends PureComponent { trackEvent('Workspace', 'Add Description'); } - _handleSetModalRef (n) { + _handleSetModalRef (n: ?Modal) { this.modal = n; } @@ -55,14 +86,22 @@ class WorkspaceSettingsModal extends PureComponent { } _handleToggleCertificateForm () { - this.setState({showAddCertificateForm: !this.state.showAddCertificateForm}); + this.setState(state => ({ + showAddCertificateForm: !state.showAddCertificateForm, + crtPath: '', + keyPath: '', + pfxPath: '', + host: '', + passphrase: '', + isPrivate: false + })); } - _handleRename (name) { + _handleRename (name: string) { this._workspaceUpdate({name}); } - _handleDescriptionChange (description) { + _handleDescriptionChange (description: string) { this._workspaceUpdate({description}); if (this.state.defaultPreviewMode !== false) { @@ -70,84 +109,75 @@ class WorkspaceSettingsModal extends PureComponent { } } - _handleCreateHostChange (e) { - this.setState({host: e.target.value}); + _handleCreateHostChange (e: SyntheticEvent) { + this.setState({host: e.currentTarget.value}); } - _handleCreatePfxChange (pfxPath) { + _handleCreatePfxChange (pfxPath: string) { this.setState({pfxPath}); } - _handleCreateCrtChange (crtPath) { + _handleCreateCrtChange (crtPath: string) { this.setState({crtPath}); } - _handleCreateKeyChange (keyPath) { + _handleCreateKeyChange (keyPath: string) { this.setState({keyPath}); } - _handleCreatePassphraseChange (e) { - this.setState({passphrase: e.target.value}); + _handleCreatePassphraseChange (e: SyntheticEvent) { + this.setState({passphrase: e.currentTarget.value}); } - async _handleSubmitCertificate (e) { + _handleCreateIsPrivateChange (e: SyntheticEvent) { + this.setState({isPrivate: e.currentTarget.checked}); + } + + async _handleCreateCertificate (e: SyntheticEvent) { e.preventDefault(); const {workspace} = this.props; - const {pfxPath, crtPath, keyPath, host, passphrase} = this.state; + const {pfxPath, crtPath, keyPath, host, passphrase, isPrivate} = this.state; const certificate = { host, - passphrase, - cert: crtPath, - key: keyPath, - pfx: pfxPath, - disabled: false + isPrivate, + parentId: workspace._id, + passphrase: passphrase || null, + disabled: false, + cert: crtPath || null, + key: keyPath || null, + pfx: pfxPath || null }; - const certificates = [ - ...workspace.certificates.filter(c => c.host !== certificate.host), - certificate - ]; - - await models.workspace.update(workspace, {certificates}); + await models.clientCertificate.create(certificate); this._handleToggleCertificateForm(); trackEvent('Certificates', 'Create'); } - _handleDeleteCertificate (certificate) { - const {workspace} = this.props; - const certificates = workspace.certificates.filter(c => c.host !== certificate.host); - models.workspace.update(workspace, {certificates}); + async _handleDeleteCertificate (certificate: ClientCertificate) { + await models.clientCertificate.remove(certificate); trackEvent('Certificates', 'Delete'); } - _handleToggleCertificate (certificate) { - const {workspace} = this.props; - const certificates = workspace.certificates.map( - c => c === certificate ? Object.assign({}, c, {disabled: !c.disabled}) : c - ); - models.workspace.update(workspace, {certificates}); + async _handleToggleCertificate (certificate: ClientCertificate) { + await models.clientCertificate.update(certificate, {disabled: !certificate.disabled}); trackEvent('Certificates', 'Toggle'); } show () { const hasDescription = !!this.props.workspace.description; this.setState({ - showAddCertificateForm: false, - crtPath: '', - keyPath: '', - pfxPath: '', - host: '', - passphrase: '', showDescription: hasDescription, - defaultPreviewMode: hasDescription + defaultPreviewMode: hasDescription, + showAddCertificateForm: false }); - this.modal.show(); + + this.modal && this.modal.show(); } hide () { - this.modal.hide(); + this.modal && this.modal.hide(); } renderModalHeader () { @@ -163,8 +193,73 @@ class WorkspaceSettingsModal extends PureComponent { ); } + renderCertificate (certificate: ClientCertificate) { + return ( +
+
+
+ + PFX: + {' '} + {certificate.pfx + ? + : + } + + + CRT: + {' '} + {certificate.cert + ? + : + } + + + Key: + {' '} + {certificate.key + ? + : + } + + + Passphrase: + {' '} + {certificate.passphrase + ? + : + } + + + Host: + {' '} + {certificate.host} + +
+
+ + this._handleDeleteCertificate(certificate)}> + + +
+
+
+ ); + } + renderModalBody () { const { + clientCertificates, workspace, editorLineWrapping, editorFontSize, @@ -175,10 +270,14 @@ class WorkspaceSettingsModal extends PureComponent { nunjucksPowerUserMode } = this.props; + const publicCertificates = clientCertificates.filter(c => !c.isPrivate); + const privateCertificates = clientCertificates.filter(c => c.isPrivate); + const { pfxPath, crtPath, keyPath, + isPrivate, showAddCertificateForm, showDescription, defaultPreviewMode @@ -246,84 +345,38 @@ class WorkspaceSettingsModal extends PureComponent { {!showAddCertificateForm ? (
- {workspace.certificates.length === 0 ? ( -

+ {clientCertificates.length === 0 ? ( +

You have not yet added any certificates

- ) : workspace.certificates.map(certificate => ( -
-

- Client certificates are an experimental feature -

-
-
- - PFX: - {' '} - {certificate.pfx - ? - : - } - - - CRT: - {' '} - {certificate.cert - ? - : - } - - - Key: - {' '} - {certificate.key - ? - : - } - - - Passphrase: - {' '} - {certificate.passphrase - ? - : - } - - - Host: - {' '} - {certificate.host} - -
-
- - this._handleDeleteCertificate(certificate)}> - - -
-
+ ) : null} + + {publicCertificates.length > 0 + ? publicCertificates.map(this.renderCertificate) + : null + } + + {privateCertificates.length > 0 ? ( +
+

+ Private Certificates + + Private certificates will not by synced. + +

+ {privateCertificates.map(this.renderCertificate)}
- ))} + ) : null}
) : ( -
+
+
+ +

- {' '} -
@@ -418,17 +482,4 @@ class WorkspaceSettingsModal extends PureComponent { } } -WorkspaceSettingsModal.propTypes = { - workspace: PropTypes.object.isRequired, - editorFontSize: PropTypes.number.isRequired, - editorIndentSize: PropTypes.number.isRequired, - editorKeyMap: PropTypes.string.isRequired, - editorLineWrapping: PropTypes.bool.isRequired, - nunjucksPowerUserMode: PropTypes.bool.isRequired, - handleRender: PropTypes.func.isRequired, - handleGetRenderContext: PropTypes.func.isRequired, - handleRemoveWorkspace: PropTypes.func.isRequired, - handleDuplicateWorkspace: PropTypes.func.isRequired -}; - export default WorkspaceSettingsModal; diff --git a/app/ui/components/wrapper.js b/app/ui/components/wrapper.js index 944441f36f..9e7006d8d4 100644 --- a/app/ui/components/wrapper.js +++ b/app/ui/components/wrapper.js @@ -42,6 +42,7 @@ import * as importers from 'insomnia-importers'; import type {CookieJar} from '../../models/cookie-jar'; import type {Environment} from '../../models/environment'; import ErrorBoundary from './error-boundary'; +import type {ClientCertificate} from '../../models/client-certificate'; type Props = { // Helper Functions @@ -102,6 +103,7 @@ type Props = { activeWorkspace: Workspace, activeCookieJar: CookieJar, activeEnvironment: Environment | null, + activeWorkspaceClientCertificates: Array, // Optional oAuth2Token: ?OAuth2Token, @@ -339,6 +341,7 @@ class Wrapper extends React.PureComponent { activeCookieJar, activeRequestResponses, activeResponse, + activeWorkspaceClientCertificates, environments, handleActivateRequest, handleCreateRequest, @@ -455,6 +458,7 @@ class Wrapper extends React.PureComponent { { + return entities.clientCertificates.filter(c => c.parentId === activeWorkspace._id); + } +); + export const selectActiveWorkspaceMeta = createSelector( selectActiveWorkspace, selectEntitiesLists,