From 5326284075a3c75d4ae314951f6ab4ef93e0297f Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 17 Dec 2021 14:08:48 +0000 Subject: [PATCH 1/5] Some fixes after testing dynamic variables in rest a bit more. --- packages/auth/src/constants.js | 1 + packages/auth/src/middleware/authenticated.js | 7 ++++--- packages/auth/src/utils.js | 13 ++++++++++++- packages/builder/src/stores/backend/queries.js | 1 + packages/server/src/api/controllers/query/index.js | 8 ++++++-- .../server/src/api/controllers/query/validation.js | 3 ++- packages/server/src/threads/query.js | 5 +++++ packages/worker/src/api/controllers/global/auth.js | 5 ++++- 8 files changed, 35 insertions(+), 8 deletions(-) diff --git a/packages/auth/src/constants.js b/packages/auth/src/constants.js index 363274eda5..28b9ced49b 100644 --- a/packages/auth/src/constants.js +++ b/packages/auth/src/constants.js @@ -16,6 +16,7 @@ exports.Headers = { APP_ID: "x-budibase-app-id", TYPE: "x-budibase-type", TENANT_ID: "x-budibase-tenant-id", + TOKEN: "x-budibase-token", } exports.GlobalRoles = { diff --git a/packages/auth/src/middleware/authenticated.js b/packages/auth/src/middleware/authenticated.js index f0fb6e21c5..87bd4d35ce 100644 --- a/packages/auth/src/middleware/authenticated.js +++ b/packages/auth/src/middleware/authenticated.js @@ -1,5 +1,5 @@ const { Cookies, Headers } = require("../constants") -const { getCookie, clearCookie } = require("../utils") +const { getCookie, clearCookie, openJwt } = require("../utils") const { getUser } = require("../cache/user") const { getSession, updateSessionTTL } = require("../security/sessions") const { buildMatcherRegex, matches } = require("./matchers") @@ -35,8 +35,9 @@ module.exports = ( publicEndpoint = true } try { - // check the actual user is authenticated first - const authCookie = getCookie(ctx, Cookies.Auth) + // check the actual user is authenticated first, try header or cookie + const headerToken = ctx.request.headers[Headers.TOKEN] + const authCookie = getCookie(ctx, Cookies.Auth) || openJwt(headerToken) let authenticated = false, user = null, internal = false diff --git a/packages/auth/src/utils.js b/packages/auth/src/utils.js index b8fa7b9588..8c00f2a8b8 100644 --- a/packages/auth/src/utils.js +++ b/packages/auth/src/utils.js @@ -63,6 +63,17 @@ exports.getAppId = ctx => { return appId } +/** + * opens the contents of the specified encrypted JWT. + * @return {object} the contents of the token. + */ +exports.openJwt = token => { + if (!token) { + return token + } + return jwt.verify(token, options.secretOrKey) +} + /** * Get a cookie from context, and decrypt if necessary. * @param {object} ctx The request which is to be manipulated. @@ -75,7 +86,7 @@ exports.getCookie = (ctx, name) => { return cookie } - return jwt.verify(cookie, options.secretOrKey) + return exports.openJwt(cookie) } /** diff --git a/packages/builder/src/stores/backend/queries.js b/packages/builder/src/stores/backend/queries.js index e4e6732871..0e767fa5c0 100644 --- a/packages/builder/src/stores/backend/queries.js +++ b/packages/builder/src/stores/backend/queries.js @@ -91,6 +91,7 @@ export function createQueriesStore() { {} ), datasourceId: query.datasourceId, + queryId: query._id || undefined, }) if (response.status !== 200) { diff --git a/packages/server/src/api/controllers/query/index.js b/packages/server/src/api/controllers/query/index.js index d0992d037a..4c7c7398be 100644 --- a/packages/server/src/api/controllers/query/index.js +++ b/packages/server/src/api/controllers/query/index.js @@ -104,8 +104,10 @@ exports.preview = async function (ctx) { const db = new CouchDB(ctx.appId) const datasource = await db.get(ctx.request.body.datasourceId) - - const { fields, parameters, queryVerb, transformer } = ctx.request.body + // preview may not have a queryId as it hasn't been saved, but if it does + // this stops dynamic variables from calling the same query + const { fields, parameters, queryVerb, transformer, queryId } = + ctx.request.body try { const { rows, keys, info, extra } = await Runner.run({ @@ -115,6 +117,7 @@ exports.preview = async function (ctx) { fields, parameters, transformer, + queryId, }) ctx.body = { @@ -143,6 +146,7 @@ async function execute(ctx, opts = { rowsOnly: false }) { fields: query.fields, parameters: ctx.request.body.parameter, transformer: query.transformer, + queryId: ctx.params.queryId, }) if (opts && opts.rowsOnly) { ctx.body = rows diff --git a/packages/server/src/api/controllers/query/validation.js b/packages/server/src/api/controllers/query/validation.js index a17a752ca5..4958433849 100644 --- a/packages/server/src/api/controllers/query/validation.js +++ b/packages/server/src/api/controllers/query/validation.js @@ -36,6 +36,7 @@ exports.generateQueryPreviewValidation = () => { extra: Joi.object().optional(), datasourceId: Joi.string().required(), transformer: Joi.string().optional(), - parameters: Joi.object({}).required().unknown(true) + parameters: Joi.object({}).required().unknown(true), + queryId: Joi.string().optional(), })) } diff --git a/packages/server/src/threads/query.js b/packages/server/src/threads/query.js index b86c1a49fd..0a46cc0271 100644 --- a/packages/server/src/threads/query.js +++ b/packages/server/src/threads/query.js @@ -13,6 +13,7 @@ class QueryRunner { this.fields = input.fields this.parameters = input.parameters this.transformer = input.transformer + this.queryId = input.queryId this.noRecursiveQuery = flags.noRecursiveQuery } @@ -108,6 +109,10 @@ class QueryRunner { // need to see if this uses any variables const stringFields = JSON.stringify(fields) const foundVars = dynamicVars.filter(variable => { + // don't allow a query to use its own dynamic variable (loop) + if (variable.queryId === this.queryId) { + return false + } // look for {{ variable }} but allow spaces between handlebars const regex = new RegExp(`{{[ ]*${variable.name}[ ]*}}`) return regex.test(stringFields) diff --git a/packages/worker/src/api/controllers/global/auth.js b/packages/worker/src/api/controllers/global/auth.js index cd7d8abcee..93a15a6514 100644 --- a/packages/worker/src/api/controllers/global/auth.js +++ b/packages/worker/src/api/controllers/global/auth.js @@ -12,7 +12,7 @@ const { hash, platformLogout, } = authPkg.utils -const { Cookies } = authPkg.constants +const { Cookies, Headers } = authPkg.constants const { passport } = authPkg.auth const { checkResetPasswordCode } = require("../../../utilities/redis") const { @@ -60,7 +60,10 @@ async function authInternal(ctx, user, err = null, info = null) { return ctx.throw(403, info ? info : "Unauthorized") } + // set a cookie for browser access setCookie(ctx, user.token, Cookies.Auth, { sign: false }) + // set the token in a header as well for APIs + ctx.set(Headers.TOKEN, user.token) // get rid of any app cookies on login // have to check test because this breaks cypress if (!env.isTest()) { From 58734cd1f2e6283cfd267bcd6a364aef193284a9 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 17 Dec 2021 17:16:06 +0000 Subject: [PATCH 2/5] Fixing issues with query dynamic variables being able to overwrite/appearing in other queries. --- .../rest/RestExtraConfigForm.svelte | 8 ++++---- packages/builder/src/helpers/data/utils.js | 17 ++++++++++------- .../_components/DynamicVariableModal.svelte | 11 ++++++++--- .../rest/[query]/index.svelte | 10 ++++++---- 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/rest/RestExtraConfigForm.svelte b/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/rest/RestExtraConfigForm.svelte index 4a104a1987..2c8b699849 100644 --- a/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/rest/RestExtraConfigForm.svelte +++ b/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/rest/RestExtraConfigForm.svelte @@ -60,9 +60,9 @@ Variables enabled you to store and reuse values in queries. Static variables - use constant values while dynamic values can be bound to the response headers - or body of a queryVariables enable you to store and re-use values in queries, with the choice + of a static value such as a token using static variables, or a value from a + query response using dynamic variables. Static @@ -78,7 +78,7 @@ Dynamic Dynamic variables are evaluated when a dependant query is executed. The value - is cached for 24 hours and will re-evaluate if the dependendent query fails. + is cached for a period of time and will be refreshed if a query fails. diff --git a/packages/builder/src/helpers/data/utils.js b/packages/builder/src/helpers/data/utils.js index e163043470..c8eb7a4d3b 100644 --- a/packages/builder/src/helpers/data/utils.js +++ b/packages/builder/src/helpers/data/utils.js @@ -121,10 +121,13 @@ export function flipHeaderState(headersActivity) { } // convert dynamic variables list to simple key/val object -export function variablesToObject(datasource) { +export function getDynamicVariables(datasource, queryId) { const variablesList = datasource?.config?.dynamicVariables if (variablesList && variablesList.length > 0) { - return variablesList.reduce( + const filtered = queryId + ? variablesList.filter(variable => variable.queryId === queryId) + : variablesList + return filtered.reduce( (acc, next) => ({ ...acc, [next.name]: next.value }), {} ) @@ -133,10 +136,10 @@ export function variablesToObject(datasource) { } // convert dynamic variables object back to a list, enrich with query id -export function rebuildVariables(queryId, variables) { - let vars = [] +export function rebuildVariables(datasource, queryId, variables) { + let finalVars = [] if (variables) { - vars = Object.entries(variables).map(entry => { + finalVars = Object.entries(variables).map(entry => { return { name: entry[0], value: entry[1], @@ -144,7 +147,7 @@ export function rebuildVariables(queryId, variables) { } }) } - return vars + return [...(datasource?.config?.dynamicVariables || []), ...finalVars] } export function shouldShowVariables(dynamicVariables, variablesReadOnly) { @@ -173,7 +176,7 @@ export default { keyValueToQueryParameters, queryParametersToKeyValue, schemaToFields, - variablesToObject, + getDynamicVariables, rebuildVariables, shouldShowVariables, buildAuthConfigs, diff --git a/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/_components/DynamicVariableModal.svelte b/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/_components/DynamicVariableModal.svelte index 1fdbc98483..2941826427 100644 --- a/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/_components/DynamicVariableModal.svelte +++ b/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/_components/DynamicVariableModal.svelte @@ -2,9 +2,10 @@ import { Input, ModalContent, Modal } from "@budibase/bbui" export let dynamicVariables + export let datasource export let binding - let name, modal, valid + let name, modal, valid, allVariableNames export const show = () => { modal.show() @@ -17,11 +18,15 @@ if (!name) { return false } - const varKeys = Object.keys(vars || {}) - return varKeys.find(key => key.toLowerCase() === name.toLowerCase()) == null + return !allVariableNames.find( + varName => varName.toLowerCase() === name.toLowerCase() + ) } $: valid = checkValid(dynamicVariables, name) + $: allVariableNames = (datasource?.config?.dynamicVariables || []).map( + variable => variable.name + ) $: error = name && !valid ? "Variable name is already in use." : null async function saveVariable() { diff --git a/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/rest/[query]/index.svelte b/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/rest/[query]/index.svelte index 62a16e9316..a54a4f205a 100644 --- a/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/rest/[query]/index.svelte +++ b/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/rest/[query]/index.svelte @@ -116,6 +116,7 @@ if (dynamicVariables) { datasource.config.dynamicVariables = restUtils.rebuildVariables( + datasource, saveId, dynamicVariables ) @@ -213,7 +214,7 @@ if (query && !query.fields.bodyType) { query.fields.bodyType = "none" } - dynamicVariables = restUtils.variablesToObject(datasource) + dynamicVariables = restUtils.getDynamicVariables(datasource, query._id) }) @@ -396,9 +397,10 @@ {#if showVariablesTab} - {"Create dynamic variables to use body and headers results in other queries"} + + Create dynamic variables based on response body or headers + from other queries. + Date: Fri, 17 Dec 2021 17:56:28 +0000 Subject: [PATCH 3/5] Adding query invalidation, when a query fails that has dynamic variables it will invalidate the cache value for all dynamic variable values. --- packages/auth/src/middleware/authenticated.js | 3 +++ packages/server/src/threads/query.js | 19 +++++++++++++++++++ packages/server/src/threads/utils.js | 10 ++++++++++ 3 files changed, 32 insertions(+) diff --git a/packages/auth/src/middleware/authenticated.js b/packages/auth/src/middleware/authenticated.js index 87bd4d35ce..50d6705748 100644 --- a/packages/auth/src/middleware/authenticated.js +++ b/packages/auth/src/middleware/authenticated.js @@ -37,6 +37,9 @@ module.exports = ( try { // check the actual user is authenticated first, try header or cookie const headerToken = ctx.request.headers[Headers.TOKEN] + if (headerToken) { + throw { name: "JsonWebTokenError" } + } const authCookie = getCookie(ctx, Cookies.Auth) || openJwt(headerToken) let authenticated = false, user = null, diff --git a/packages/server/src/threads/query.js b/packages/server/src/threads/query.js index 0a46cc0271..cabffd7beb 100644 --- a/packages/server/src/threads/query.js +++ b/packages/server/src/threads/query.js @@ -15,6 +15,8 @@ class QueryRunner { this.transformer = input.transformer this.queryId = input.queryId this.noRecursiveQuery = flags.noRecursiveQuery + this.cachedVariables = [] + this.hasRerun = false } async execute() { @@ -44,6 +46,19 @@ class QueryRunner { rows = runner.execute() } + // if the request fails we retry once, invalidating the cached value + if ( + info && + info.code >= 400 && + this.cachedVariables.length > 0 && + !this.hasRerun + ) { + this.hasRerun = true + // invalidate the cache value + await threadUtils.invalidateDynamicVariables(this.cachedVariables) + return this.execute() + } + // needs to an array for next step if (!Array.isArray(rows)) { rows = [rows] @@ -89,6 +104,8 @@ class QueryRunner { if (!value) { value = await this.runAnotherQuery(queryId, parameters) await threadUtils.storeDynamicVariable(queryId, name, value) + } else { + this.cachedVariables.push({ queryId, name }) } return value } @@ -125,6 +142,8 @@ class QueryRunner { data: responses[i].rows, info: responses[i].extra, }) + // make sure its known that this uses dynamic variables in case it fails + this.hasDynamicVariables = true } } return parameters diff --git a/packages/server/src/threads/utils.js b/packages/server/src/threads/utils.js index 34ae4f0477..fee1e19b67 100644 --- a/packages/server/src/threads/utils.js +++ b/packages/server/src/threads/utils.js @@ -38,6 +38,16 @@ exports.checkCacheForDynamicVariable = async (queryId, variable) => { return cache.get(makeVariableKey(queryId, variable)) } +exports.invalidateDynamicVariables = async cachedVars => { + let promises = [] + for (let variable of cachedVars) { + promises.push( + client.delete(makeVariableKey(variable.queryId, variable.name)) + ) + } + await Promise.all(promises) +} + exports.storeDynamicVariable = async (queryId, variable, value) => { const cache = await getClient() await cache.store( From 9dd987ffc2dc088ca6934b0468b54681435172a8 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 17 Dec 2021 17:57:40 +0000 Subject: [PATCH 4/5] Removing faked error. --- packages/auth/src/middleware/authenticated.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/auth/src/middleware/authenticated.js b/packages/auth/src/middleware/authenticated.js index 50d6705748..87bd4d35ce 100644 --- a/packages/auth/src/middleware/authenticated.js +++ b/packages/auth/src/middleware/authenticated.js @@ -37,9 +37,6 @@ module.exports = ( try { // check the actual user is authenticated first, try header or cookie const headerToken = ctx.request.headers[Headers.TOKEN] - if (headerToken) { - throw { name: "JsonWebTokenError" } - } const authCookie = getCookie(ctx, Cookies.Auth) || openJwt(headerToken) let authenticated = false, user = null, From ae510760c3f26906134b3451db7cf8e3a5d103ac Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 17 Dec 2021 18:21:36 +0000 Subject: [PATCH 5/5] Some minor UI tweaks. --- packages/builder/src/helpers/data/utils.js | 14 +++++++++++--- .../_components/DynamicVariableModal.svelte | 6 +++++- packages/server/src/threads/query.js | 10 +++++++++- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/builder/src/helpers/data/utils.js b/packages/builder/src/helpers/data/utils.js index c8eb7a4d3b..6d7843e0e5 100644 --- a/packages/builder/src/helpers/data/utils.js +++ b/packages/builder/src/helpers/data/utils.js @@ -137,9 +137,9 @@ export function getDynamicVariables(datasource, queryId) { // convert dynamic variables object back to a list, enrich with query id export function rebuildVariables(datasource, queryId, variables) { - let finalVars = [] + let newVariables = [] if (variables) { - finalVars = Object.entries(variables).map(entry => { + newVariables = Object.entries(variables).map(entry => { return { name: entry[0], value: entry[1], @@ -147,7 +147,15 @@ export function rebuildVariables(datasource, queryId, variables) { } }) } - return [...(datasource?.config?.dynamicVariables || []), ...finalVars] + let existing = datasource?.config?.dynamicVariables || [] + // filter out any by same name + existing = existing.filter( + variable => + !newVariables.find( + newVar => newVar.name.toLowerCase() === variable.name.toLowerCase() + ) + ) + return [...existing, ...newVariables] } export function shouldShowVariables(dynamicVariables, variablesReadOnly) { diff --git a/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/_components/DynamicVariableModal.svelte b/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/_components/DynamicVariableModal.svelte index 2941826427..61d0a1993c 100644 --- a/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/_components/DynamicVariableModal.svelte +++ b/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/_components/DynamicVariableModal.svelte @@ -1,5 +1,5 @@