From cdd046b8c1c5a7cfe29c5d7702918a406745c658 Mon Sep 17 00:00:00 2001 From: Chang-Hung Liang Date: Fri, 8 Dec 2017 21:45:21 +0800 Subject: [PATCH 1/7] zapier convert: Add support for basic auth mapping --- scaffold/convert/header.template.js | 16 +++++++++++++++- src/utils/convert.js | 14 +++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/scaffold/convert/header.template.js b/scaffold/convert/header.template.js index 150a83e..59c839e 100644 --- a/scaffold/convert/header.template.js +++ b/scaffold/convert/header.template.js @@ -1,4 +1,6 @@ -<% if (before && !session && !oauth) { %>const maybeIncludeAuth = (request, z, bundle) => { +const { replaceVars } = require('./utils'); + +<% if (before && !session && !oauth && !customBasic) { %>const maybeIncludeAuth = (request, z, bundle) => { <% Object.keys(mapping).forEach((mapperKey) => { fields.forEach((field) => { @@ -14,6 +16,18 @@ %> return request; }; +<% } else if (customBasic) { %> +const maybeIncludeAuth = (request, z, bundle) => { + const mapping = { + username: '<%= mapping.username %>', + password: '<%= mapping.password %>' + }; + const username = replaceVars(mapping.username, bundle); + const password = replaceVars(mapping.password, bundle); + const encoded = `${username}:${password}`.toString('base64'); + request.headers.Authorization = `Baisc ${encoded}`; + return request; +}; <% } if (before && session) { %>const maybeIncludeAuth = (request, z, bundle) => { diff --git a/src/utils/convert.js b/src/utils/convert.js index 9443e3d..aec8736 100644 --- a/src/utils/convert.js +++ b/src/utils/convert.js @@ -199,6 +199,10 @@ const renderAuthTemplate = (authType, definition) => { const testTriggerKey = getTestTriggerKey(definition); const { hasGetConnectionLabelScripting } = getAuthMetaData(definition); + if (authType === 'basic' && !_.isEmpty(definition.general.auth_mapping)) { + authType = 'custom'; + } + const templateContext = { TEST_TRIGGER_MODULE: `./triggers/${snakeCase(testTriggerKey)}`, TYPE: authType, @@ -446,11 +450,16 @@ const getMetaData = (definition) => { }); }); - const hasBefore = (type === 'api-header' || type === 'api-query' || type === 'session' || type === 'oauth2' || type === 'oauth2-refresh'); + const isCustomBasic = (type === 'basic' && !_.isEmpty(definition.general.auth_mapping)); + const hasBefore = ( + type === 'api-header' || type === 'api-query' || type === 'session' || + type === 'oauth2' || type === 'oauth2-refresh' || isCustomBasic + ); const hasAfter = (type === 'session'); const fieldsOnQuery = (authPlacement === 'params' || type === 'api-query'); const isSession = (type === 'session'); const isOAuth = (type === 'oauth2' || type === 'oauth2-refresh'); + const needsLegacyScriptingRunner = isSession || hasAnyScriptingMethods; return { @@ -460,6 +469,7 @@ const getMetaData = (definition) => { fieldsOnQuery, isSession, isOAuth, + isCustomBasic, needsLegacyScriptingRunner, }; }; @@ -472,6 +482,7 @@ const getHeader = (definition) => { hasAfter, isSession, isOAuth, + isCustomBasic, fieldsOnQuery, } = getMetaData(definition); @@ -481,6 +492,7 @@ const getHeader = (definition) => { after: hasAfter, session: isSession, oauth: isOAuth, + customBasic: isCustomBasic, fields: Object.keys(definition.auth_fields), mapping: _.get(definition, ['general', 'auth_mapping'], {}), query: fieldsOnQuery, From e48ae431810553c1514e3e1acd1c71e6f808242a Mon Sep 17 00:00:00 2001 From: Chang-Hung Liang Date: Mon, 11 Dec 2017 15:18:29 +0800 Subject: [PATCH 2/7] zapier convert: Fix auth tests Most of these tests failed because I added heading and trailing spaces to the trigger template. This commit fixes them by removing the heading and trailing spaces with .trim() method before making assertion in the test code. This is temporary because I think we can use a tool like Prettier to format the generated code automitcally, so we don't have to manually format it in the templates, which is such a pain. --- scaffold/convert/header.template.js | 3 +- src/tests/utils/convert.js | 34 +++++++------------ .../utils/definitions/basic-scripting.json | 4 +-- src/tests/utils/definitions/basic.json | 4 +-- 4 files changed, 18 insertions(+), 27 deletions(-) diff --git a/scaffold/convert/header.template.js b/scaffold/convert/header.template.js index 59c839e..7773df4 100644 --- a/scaffold/convert/header.template.js +++ b/scaffold/convert/header.template.js @@ -1,5 +1,6 @@ +<% if (customBasic) { %> const { replaceVars } = require('./utils'); - +<% } %> <% if (before && !session && !oauth && !customBasic) { %>const maybeIncludeAuth = (request, z, bundle) => { <% Object.keys(mapping).forEach((mapperKey) => { diff --git a/src/tests/utils/convert.js b/src/tests/utils/convert.js index 7538091..8695e42 100644 --- a/src/tests/utils/convert.js +++ b/src/tests/utils/convert.js @@ -131,7 +131,7 @@ describe('convert render functions', () => { return convert.renderAuth(wbDef) .then(string => { const auth = loadAuthModuleFromString(string); - auth.type.should.eql('basic'); + auth.type.should.eql('custom'); auth.fields.should.eql([ { key: 'username', @@ -157,7 +157,7 @@ describe('convert render functions', () => { return convert.renderAuth(wbDef) .then(string => { const auth = loadAuthModuleFromString(string); - auth.type.should.eql('basic'); + auth.type.should.eql('custom'); auth.fields.should.eql([ { key: 'username', @@ -202,14 +202,12 @@ describe('convert render functions', () => { return convert.getHeader(wbDef) .then(string => { - string.should.eql(`const maybeIncludeAuth = (request, z, bundle) => { + string.trim().should.eql(`const maybeIncludeAuth = (request, z, bundle) => { request.headers['Authorization'] = bundle.authData['api_key']; return request; -}; - -`); +};`); }); }); @@ -238,14 +236,12 @@ describe('convert render functions', () => { return convert.getHeader(wbDef) .then(string => { - string.should.eql(`const maybeIncludeAuth = (request, z, bundle) => { + string.trim().should.eql(`const maybeIncludeAuth = (request, z, bundle) => { request.params['api_key'] = bundle.authData['api_key']; return request; -}; - -`); +};`); }); }); @@ -281,7 +277,7 @@ describe('convert render functions', () => { return convert.getHeader(wbDef) .then(string => { - string.should.eql(`const maybeIncludeAuth = (request, z, bundle) => { + string.trim().should.eql(`const maybeIncludeAuth = (request, z, bundle) => { request.headers['X-Token'] = bundle.authData.sessionKey; @@ -320,9 +316,7 @@ const getSessionKey = (z, bundle) => { sessionKey: firstKeyValue }; }); -}; - -`); +};`); }); }); @@ -371,14 +365,12 @@ const getSessionKey = (z, bundle) => { return convert.getHeader(wbDef) .then(string => { - string.should.eql(`const maybeIncludeAuth = (request, z, bundle) => { + string.trim().should.eql(`const maybeIncludeAuth = (request, z, bundle) => { request.headers.Authorization = \`Bearer \${bundle.authData.access_token}\`; return request; -}; - -`); +};`); }); }); @@ -493,14 +485,12 @@ const getSessionKey = (z, bundle) => { return convert.getHeader(wbDef) .then(string => { - string.should.eql(`const maybeIncludeAuth = (request, z, bundle) => { + string.trim().should.eql(`const maybeIncludeAuth = (request, z, bundle) => { request.headers.Authorization = \`Bearer \${bundle.authData.access_token}\`; return request; -}; - -`); +};`); }); }); diff --git a/src/tests/utils/definitions/basic-scripting.json b/src/tests/utils/definitions/basic-scripting.json index 38192f1..1226419 100644 --- a/src/tests/utils/definitions/basic-scripting.json +++ b/src/tests/utils/definitions/basic-scripting.json @@ -72,8 +72,8 @@ "auth_data": {}, "auth_label": "{{username}}", "auth_mapping": { - "Password": "{{password}}", - "Username": "{{username}}" + "password": "{{password}}", + "username": "{{username}}" }, "auth_type": "Basic Auth", "auth_urls": {}, diff --git a/src/tests/utils/definitions/basic.json b/src/tests/utils/definitions/basic.json index cac8115..1069337 100644 --- a/src/tests/utils/definitions/basic.json +++ b/src/tests/utils/definitions/basic.json @@ -72,8 +72,8 @@ "auth_data": {}, "auth_label": "{{username}}", "auth_mapping": { - "Password": "{{password}}", - "Username": "{{username}}" + "password": "{{password}}", + "username": "{{username}}" }, "auth_type": "Basic Auth", "auth_urls": {}, From e6bd39a833c4d2f73d385d8e7cf5091c385fd443 Mon Sep 17 00:00:00 2001 From: Chang-Hung Liang Date: Mon, 11 Dec 2017 16:46:29 +0800 Subject: [PATCH 3/7] zapier convert: Handle no auth correctly If no auth, there should be no `authetication` section in the app definition nor `authencation.js`. --- scaffold/convert/index.template.js | 4 +--- src/utils/convert.js | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/scaffold/convert/index.template.js b/scaffold/convert/index.template.js index 61750a9..4a318f4 100644 --- a/scaffold/convert/index.template.js +++ b/scaffold/convert/index.template.js @@ -7,10 +7,8 @@ const App = { version: require('./package.json').version, platformVersion: require('zapier-platform-core').version, -<% if (hasAuth) { %> +<% if (needsAuth) { %> authentication, -<% } else { %> - authentication: {}, <% } %> beforeRequest: [ <%= BEFORE_REQUESTS %> diff --git a/src/utils/convert.js b/src/utils/convert.js index aec8736..68a094d 100644 --- a/src/utils/convert.js +++ b/src/utils/convert.js @@ -95,7 +95,7 @@ const getAuthType = (definition) => { }; const hasAuth = (definition) => { - return getAuthType(definition) !== 'custom' || !_.isEmpty(definition.auth_fields); + return getAuthType(definition) !== 'custom' && !_.isEmpty(definition.auth_fields); }; const renderField = (definition, key, indent = 0) => { @@ -450,15 +450,16 @@ const getMetaData = (definition) => { }); }); - const isCustomBasic = (type === 'basic' && !_.isEmpty(definition.general.auth_mapping)); - const hasBefore = ( + const needsAuth = hasAuth(definition); + const isCustomBasic = (needsAuth && type === 'basic' && !_.isEmpty(definition.general.auth_mapping)); + const hasBefore = needsAuth && ( type === 'api-header' || type === 'api-query' || type === 'session' || type === 'oauth2' || type === 'oauth2-refresh' || isCustomBasic ); - const hasAfter = (type === 'session'); + const hasAfter = (needsAuth && type === 'session'); const fieldsOnQuery = (authPlacement === 'params' || type === 'api-query'); - const isSession = (type === 'session'); - const isOAuth = (type === 'oauth2' || type === 'oauth2-refresh'); + const isSession = (needsAuth && type === 'session'); + const isOAuth = needsAuth && (type === 'oauth2' || type === 'oauth2-refresh'); const needsLegacyScriptingRunner = isSession || hasAnyScriptingMethods; @@ -673,7 +674,7 @@ const writeUtils = (newAppDir) => { }; const renderIndex = (legacyApp) => { - const _hasAuth = hasAuth(legacyApp); + const needsAuth = hasAuth(legacyApp); const templateContext = { HEADER: '', TRIGGERS: '', @@ -681,7 +682,7 @@ const renderIndex = (legacyApp) => { CREATES: '', BEFORE_REQUESTS: getBeforeRequests(legacyApp), AFTER_RESPONSES: getAfterResponses(legacyApp), - hasAuth: _hasAuth + needsAuth, }; return getHeader(legacyApp) @@ -690,7 +691,7 @@ const renderIndex = (legacyApp) => { const importLines = []; - if (_hasAuth) { + if (needsAuth) { importLines.push("const authentication = require('./authentication');"); } From fba836962ddb48cdbb76a0629780173448f1c0e5 Mon Sep 17 00:00:00 2001 From: Chang-Hung Liang Date: Mon, 11 Dec 2017 16:49:30 +0800 Subject: [PATCH 4/7] zapier convert: Generated test code should use auth_fields We were assuming basic auth is always username and password when generating test code. The right thing to do is to use `auth_fields`. This commit only addresses the issue for basic auth. We should really do this for all the auth types. --- src/utils/convert.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/utils/convert.js b/src/utils/convert.js index 68a094d..be2ab89 100644 --- a/src/utils/convert.js +++ b/src/utils/convert.js @@ -598,15 +598,20 @@ const writeStep = (type, definition, key, legacyApp, newAppDir) => { }; // render the authData used in the trigger/search/create test code -const renderAuthData = (authType) => { +const renderAuthData = (definition) => { + const authType = getAuthType(definition); let result; switch (authType) { - case 'basic': + case 'basic': { + let lines = _.map(definition.auth_fields, (field, key) => { + const upperKey = key.toUpperCase(); + return ` ${key}: process.env.${upperKey}`; + }); result = `{ - username: process.env.USERNAME, - password: process.env.PASSWORD +${lines.join(',\n')} }`; break; + } case 'oauth2': result = `{ access_token: process.env.ACCESS_TOKEN @@ -639,9 +644,8 @@ const renderAuthData = (authType) => { }; const renderStepTest = (type, definition, key, legacyApp) => { - const authType = getAuthType(legacyApp); const label = definition.label || _.capitalize(key); - const authData = renderAuthData(authType); + const authData = renderAuthData(legacyApp); const templateContext = { KEY: key, LABEL: label, From e1fdd111e17eab11d2c6d4e3cc91761cd69bd7ef Mon Sep 17 00:00:00 2001 From: Chang-Hung Liang Date: Mon, 11 Dec 2017 16:56:41 +0800 Subject: [PATCH 5/7] zapier convert: Add test app to test basic auth with auth mapping --- scripts/test-convert.js | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/test-convert.js b/scripts/test-convert.js index 9f57a3c..0ad83b9 100755 --- a/scripts/test-convert.js +++ b/scripts/test-convert.js @@ -15,6 +15,7 @@ const appsToConvert = [ {id: 82250, name: 'search-oauth'}, {id: 82251, name: 'basic-api-header'}, {id: 82460, name: 'custom-fields'}, + {id: 80444, name: 'custom-baisc'}, // TODO: Add more apps that use different scripting methods, as we start to support them ]; From 0a2b0f955353d01b5efbe06be48d8da01e6f6ede Mon Sep 17 00:00:00 2001 From: Chang-Hung Liang Date: Mon, 11 Dec 2017 17:00:09 +0800 Subject: [PATCH 6/7] zapier convert: Fix no auth test --- src/tests/utils/convert.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/utils/convert.js b/src/tests/utils/convert.js index 8695e42..6d5c80c 100644 --- a/src/tests/utils/convert.js +++ b/src/tests/utils/convert.js @@ -120,7 +120,7 @@ describe('convert render functions', () => { const wbDef = definitions.noAuth; return convert.renderIndex(wbDef) .then(string => { - string.should.containEql('authentication: {}'); + string.should.not.containEql('authentication:'); convert.hasAuth(wbDef).should.be.false(); }); }); From 7ac3941153d53d3fb0671669561b6babd299ed1d Mon Sep 17 00:00:00 2001 From: Chang-Hung Liang Date: Thu, 14 Dec 2017 15:45:22 +0800 Subject: [PATCH 7/7] zapier convert: Fix base64 encoding for basic auth --- scaffold/convert/header.template.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scaffold/convert/header.template.js b/scaffold/convert/header.template.js index 7773df4..a057f36 100644 --- a/scaffold/convert/header.template.js +++ b/scaffold/convert/header.template.js @@ -25,7 +25,7 @@ const maybeIncludeAuth = (request, z, bundle) => { }; const username = replaceVars(mapping.username, bundle); const password = replaceVars(mapping.password, bundle); - const encoded = `${username}:${password}`.toString('base64'); + const encoded = new Buffer(`${username}:${password}`).toString('base64'); request.headers.Authorization = `Baisc ${encoded}`; return request; };