From ff48aaec6e45939f797ac21f9f6558c2d2dcf0f3 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Tue, 24 May 2022 09:54:36 +0100 Subject: [PATCH] User context updates and misc fixes --- packages/backend-core/context.js | 2 + packages/backend-core/src/context/index.js | 79 +++++++------------ .../backend-core/src/events/identification.ts | 8 +- .../src/middleware/authenticated.js | 2 +- .../backend-core/src/middleware/tenancy.js | 51 +++++++++--- packages/backend-core/tsconfig.build.json | 2 - packages/backend-core/tsconfig.json | 1 - packages/server/src/api/controllers/dev.js | 2 +- .../src/api/controllers/table/external.js | 2 +- .../migrations/functions/backfill/account.ts | 16 ---- .../functions/backfill/global/configs.ts | 2 +- packages/types/src/events/auth.ts | 2 +- .../worker/src/api/controllers/global/auth.ts | 17 ++-- 13 files changed, 91 insertions(+), 95 deletions(-) delete mode 100644 packages/server/src/migrations/functions/backfill/account.ts diff --git a/packages/backend-core/context.js b/packages/backend-core/context.js index 4bc100687d..4ba3e6bffe 100644 --- a/packages/backend-core/context.js +++ b/packages/backend-core/context.js @@ -5,6 +5,7 @@ const { getAppId, updateAppId, doInAppContext, + doInUserContext, } = require("./src/context") module.exports = { @@ -14,4 +15,5 @@ module.exports = { getAppId, updateAppId, doInAppContext, + doInUserContext, } diff --git a/packages/backend-core/src/context/index.js b/packages/backend-core/src/context/index.js index 2af8b9b0f8..ae9c7b92ea 100644 --- a/packages/backend-core/src/context/index.js +++ b/packages/backend-core/src/context/index.js @@ -1,5 +1,4 @@ const env = require("../environment") -const { Headers } = require("../../constants") const { SEPARATOR, DocumentTypes } = require("../db/constants") const { DEFAULT_TENANT_ID } = require("../constants") const cls = require("./FunctionContext") @@ -77,7 +76,11 @@ exports.isMultiTenant = () => { exports.doInTenant = (tenantId, task) => { // the internal function is so that we can re-use an existing // context - don't want to close DB on a parent context - async function internal(opts = { existing: false }) { + async function internal(opts = { existing: false, user: undefined }) { + // preserve the user + if (user) { + exports.setUser(user) + } // set the tenant id if (!opts.existing) { cls.setOnContext(ContextKeys.TENANT_ID, tenantId) @@ -98,14 +101,16 @@ exports.doInTenant = (tenantId, task) => { } } } + + const user = cls.getFromContext(ContextKeys.USER) const using = cls.getFromContext(ContextKeys.IN_USE) if (using && cls.getFromContext(ContextKeys.TENANT_ID) === tenantId) { cls.setOnContext(ContextKeys.IN_USE, using + 1) - return internal({ existing: true }) + return internal({ existing: true, user }) } else { return cls.run(async () => { cls.setOnContext(ContextKeys.IN_USE, 1) - return internal() + return internal({ existing: false, user }) }) } } @@ -143,7 +148,11 @@ exports.doInAppContext = (appId, task) => { // the internal function is so that we can re-use an existing // context - don't want to close DB on a parent context - async function internal(opts = { existing: false }) { + async function internal(opts = { existing: false, user: undefined }) { + // preserve the user + if (user) { + exports.setUser(user) + } // set the app tenant id if (!opts.existing) { setAppTenantId(appId) @@ -162,28 +171,39 @@ exports.doInAppContext = (appId, task) => { } } } + const user = cls.getFromContext(ContextKeys.USER) const using = cls.getFromContext(ContextKeys.IN_USE) if (using && cls.getFromContext(ContextKeys.APP_ID) === appId) { cls.setOnContext(ContextKeys.IN_USE, using + 1) - return internal({ existing: true }) + return internal({ existing: true, user }) } else { return cls.run(async () => { cls.setOnContext(ContextKeys.IN_USE, 1) - return internal() + return internal({ existing: false, user }) }) } } exports.doInUserContext = (user, task) => { - return cls.run(async () => { - cls.setOnContext(ContextKeys.USER, user) + return cls.run(() => { + let tenantId = user.tenantId + if (!tenantId) { + tenantId = exports.getTenantId() + } + cls.setOnContext(ContextKeys.TENANT_ID, tenantId) + exports.setUser(user) return task() }) } +exports.setUser = user => { + cls.setOnContext(ContextKeys.USER, user) +} + exports.getUser = () => { try { - return cls.getFromContext(ContextKeys.USER) + const user = cls.getFromContext(ContextKeys.USER) + return user } catch (e) { // do nothing - user is not in context } @@ -208,45 +228,6 @@ exports.updateAppId = async appId => { } } -exports.setTenantId = ( - ctx, - opts = { allowQs: false, allowNoTenant: false } -) => { - let tenantId - // exit early if not multi-tenant - if (!exports.isMultiTenant()) { - cls.setOnContext(ContextKeys.TENANT_ID, exports.DEFAULT_TENANT_ID) - return exports.DEFAULT_TENANT_ID - } - - const allowQs = opts && opts.allowQs - const allowNoTenant = opts && opts.allowNoTenant - const header = ctx.request.headers[Headers.TENANT_ID] - const user = ctx.user || {} - if (allowQs) { - const query = ctx.request.query || {} - tenantId = query.tenantId - } - // override query string (if allowed) by user, or header - // URL params cannot be used in a middleware, as they are - // processed later in the chain - tenantId = user.tenantId || header || tenantId - - // Set the tenantId from the subdomain - if (!tenantId) { - tenantId = ctx.subdomains && ctx.subdomains[0] - } - - if (!tenantId && !allowNoTenant) { - ctx.throw(403, "Tenant id not set") - } - // check tenant ID just incase no tenant was allowed - if (tenantId) { - cls.setOnContext(ContextKeys.TENANT_ID, tenantId) - } - return tenantId -} - exports.setGlobalDB = tenantId => { const dbName = baseGlobalDBName(tenantId) const db = dangerousGetDB(dbName) diff --git a/packages/backend-core/src/events/identification.ts b/packages/backend-core/src/events/identification.ts index f86d6abe46..0b7e1f12f9 100644 --- a/packages/backend-core/src/events/identification.ts +++ b/packages/backend-core/src/events/identification.ts @@ -1,7 +1,3 @@ -import { - isCloudAccount, - isSSOAccount, -} from "./../../../types/src/documents/account/account" import * as context from "../context" import env from "../environment" import { @@ -13,6 +9,8 @@ import { Account, AccountIdentity, BudibaseIdentity, + isCloudAccount, + isSSOAccount, } from "@budibase/types" import { analyticsProcessor } from "./processors" @@ -26,7 +24,7 @@ export const getCurrentIdentity = (): Identity => { } else if (env.SELF_HOSTED) { id = "installationId" // TODO } else { - id = context.getTenantId() + id = tenantId } return { diff --git a/packages/backend-core/src/middleware/authenticated.js b/packages/backend-core/src/middleware/authenticated.js index a1bb0a8ef1..3fef6eadac 100644 --- a/packages/backend-core/src/middleware/authenticated.js +++ b/packages/backend-core/src/middleware/authenticated.js @@ -137,7 +137,7 @@ module.exports = ( if (user && user.email) { return context.doInUserContext(user, next) } else { - return next + return next() } } catch (err) { // invalid token, clear the cookie diff --git a/packages/backend-core/src/middleware/tenancy.js b/packages/backend-core/src/middleware/tenancy.js index 9a0cb8a0c6..b3e61a2fe4 100644 --- a/packages/backend-core/src/middleware/tenancy.js +++ b/packages/backend-core/src/middleware/tenancy.js @@ -1,6 +1,38 @@ -const { setTenantId, setGlobalDB, closeTenancy } = require("../tenancy") -const cls = require("../context/FunctionContext") +const { doInTenant, isMultiTenant, DEFAULT_TENANT_ID } = require("../tenancy") const { buildMatcherRegex, matches } = require("./matchers") +const { Headers } = require("../../constants") + +const getTenantID = (ctx, opts = { allowQs: false, allowNoTenant: false }) => { + // exit early if not multi-tenant + if (!isMultiTenant()) { + return DEFAULT_TENANT_ID + } + + let tenantId + const allowQs = opts && opts.allowQs + const allowNoTenant = opts && opts.allowNoTenant + const header = ctx.request.headers[Headers.TENANT_ID] + const user = ctx.user || {} + if (allowQs) { + const query = ctx.request.query || {} + tenantId = query.tenantId + } + // override query string (if allowed) by user, or header + // URL params cannot be used in a middleware, as they are + // processed later in the chain + tenantId = user.tenantId || header || tenantId + + // Set the tenantId from the subdomain + if (!tenantId) { + tenantId = ctx.subdomains && ctx.subdomains[0] + } + + if (!tenantId && !allowNoTenant) { + ctx.throw(403, "Tenant id not set") + } + + return tenantId +} module.exports = ( allowQueryStringPatterns, @@ -11,15 +43,10 @@ module.exports = ( const noTenancyOptions = buildMatcherRegex(noTenancyPatterns) return async function (ctx, next) { - return cls.run(async () => { - const allowNoTenant = - opts.noTenancyRequired || !!matches(ctx, noTenancyOptions) - const allowQs = !!matches(ctx, allowQsOptions) - const tenantId = setTenantId(ctx, { allowQs, allowNoTenant }) - setGlobalDB(tenantId) - const res = await next() - await closeTenancy() - return res - }) + const allowNoTenant = + opts.noTenancyRequired || !!matches(ctx, noTenancyOptions) + const allowQs = !!matches(ctx, allowQsOptions) + const tenantId = getTenantID(ctx, { allowQs, allowNoTenant }) + return doInTenant(tenantId, next) } } diff --git a/packages/backend-core/tsconfig.build.json b/packages/backend-core/tsconfig.build.json index 57783aab7a..b5f8607023 100644 --- a/packages/backend-core/tsconfig.build.json +++ b/packages/backend-core/tsconfig.build.json @@ -3,8 +3,6 @@ "extends": "./tsconfig.json", "exclude": [ "node_modules", - "dist/**/*", - "**/*.json", "**/*.spec.js", "**/*.spec.ts" ] diff --git a/packages/backend-core/tsconfig.json b/packages/backend-core/tsconfig.json index 373b8440f7..5f9000c18f 100644 --- a/packages/backend-core/tsconfig.json +++ b/packages/backend-core/tsconfig.json @@ -29,7 +29,6 @@ ], "exclude": [ "node_modules", - "dist/**/*", "**/*.spec.js", // "**/*.spec.ts" // don't exclude spec.ts files for editor support ] diff --git a/packages/server/src/api/controllers/dev.js b/packages/server/src/api/controllers/dev.js index 1146013c3b..e78fe7ddce 100644 --- a/packages/server/src/api/controllers/dev.js +++ b/packages/server/src/api/controllers/dev.js @@ -131,5 +131,5 @@ exports.getBudibaseVersion = async ctx => { ctx.body = { version, } - events.org.versionChecked(version) + await events.org.versionChecked(version) } diff --git a/packages/server/src/api/controllers/table/external.js b/packages/server/src/api/controllers/table/external.js index 5a7239e90c..34319c5bff 100644 --- a/packages/server/src/api/controllers/table/external.js +++ b/packages/server/src/api/controllers/table/external.js @@ -315,6 +315,6 @@ exports.bulkImport = async function (ctx) { await handleRequest(DataSourceOperation.BULK_CREATE, table._id, { rows, }) - events.row.import(table, "csv", rows.length) + await events.rows.imported(table, "csv", rows.length) return table } diff --git a/packages/server/src/migrations/functions/backfill/account.ts b/packages/server/src/migrations/functions/backfill/account.ts deleted file mode 100644 index a0f10d218f..0000000000 --- a/packages/server/src/migrations/functions/backfill/account.ts +++ /dev/null @@ -1,16 +0,0 @@ -// TODO: Add migrations to account portal - -import { events, db } from "@budibase/backend-core" -import { Account } from "@budibase/types" - -export const backfill = async (appDb: any) => { - const accounts: Account[] = [] - - for (const account of accounts) { - events.account.created(account) - - if (account.verified) { - events.account.verified(account) - } - } -} diff --git a/packages/server/src/migrations/functions/backfill/global/configs.ts b/packages/server/src/migrations/functions/backfill/global/configs.ts index 5c3644fca5..4705e49a38 100644 --- a/packages/server/src/migrations/functions/backfill/global/configs.ts +++ b/packages/server/src/migrations/functions/backfill/global/configs.ts @@ -36,7 +36,7 @@ export const backfill = async (globalDb: any) => { if (isOIDCConfig(config)) { await events.auth.SSOCreated("oidc") if (config.config.configs[0].activated) { - events.auth.SSOActivated("oidc") + await events.auth.SSOActivated("oidc") } } if (isSettingsConfig(config)) { diff --git a/packages/types/src/events/auth.ts b/packages/types/src/events/auth.ts index aaa2cdd834..e5dfb224bc 100644 --- a/packages/types/src/events/auth.ts +++ b/packages/types/src/events/auth.ts @@ -1,4 +1,4 @@ -export type LoginSource = "local" | "sso" +export type LoginSource = "local" | "google" | "oidc" export type SSOType = "oidc" | "google" export interface LoginEvent { diff --git a/packages/worker/src/api/controllers/global/auth.ts b/packages/worker/src/api/controllers/global/auth.ts index 4555ef05d0..71d0afbb52 100644 --- a/packages/worker/src/api/controllers/global/auth.ts +++ b/packages/worker/src/api/controllers/global/auth.ts @@ -14,8 +14,9 @@ const { isMultiTenant, } = require("@budibase/backend-core/tenancy") const env = require("../../../environment") -const { events, users: usersCore } = require("@budibase/backend-core") +import { events, users as usersCore, context } from "@budibase/backend-core" import { users } from "../../../sdk" +import { User } from "@budibase/types" const ssoCallbackUrl = async (config: any, type: any) => { // incase there is a callback URL from before @@ -71,7 +72,9 @@ export const authenticate = async (ctx: any, next: any) => { "local", async (err: any, user: any, info: any) => { await authInternal(ctx, user, err, info) - await events.auth.login("local") + await context.doInUserContext(user, async () => { + await events.auth.login("local") + }) ctx.status = 200 } )(ctx, next) @@ -105,7 +108,7 @@ export const reset = async (ctx: any) => { ) } try { - const user = await usersCore.getGlobalUserByEmail(email) + const user = (await usersCore.getGlobalUserByEmail(email)) as User // only if user exists, don't error though if they don't if (user) { await sendEmail(email, EmailTemplatePurpose.PASSWORD_RECOVERY, { @@ -212,7 +215,9 @@ export const googleAuth = async (ctx: any, next: any) => { { successRedirect: "/", failureRedirect: "/error" }, async (err: any, user: any, info: any) => { await authInternal(ctx, user, err, info) - await events.auth.login("google") + await context.doInUserContext(user, async () => { + await events.auth.login("google") + }) ctx.redirect("/") } )(ctx, next) @@ -256,7 +261,9 @@ export const oidcAuth = async (ctx: any, next: any) => { { successRedirect: "/", failureRedirect: "/error" }, async (err: any, user: any, info: any) => { await authInternal(ctx, user, err, info) - await events.auth.login("oidc") + await context.doInUserContext(user, async () => { + await events.auth.login("oidc") + }) ctx.redirect("/") } )(ctx, next)