diff --git a/lerna.json b/lerna.json index a57875986a..5d16aa7bf1 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.10.9-alpha.3", + "version": "2.10.12-alpha.1", "npmClient": "yarn", "packages": [ "packages/*" diff --git a/packages/backend-core/src/cache/tests/user.spec.ts b/packages/backend-core/src/cache/tests/user.spec.ts new file mode 100644 index 0000000000..80e5bc3063 --- /dev/null +++ b/packages/backend-core/src/cache/tests/user.spec.ts @@ -0,0 +1,145 @@ +import { User } from "@budibase/types" +import { generator, structures } from "../../../tests" +import { DBTestConfiguration } from "../../../tests/extra" +import { getUsers } from "../user" +import { getGlobalDB } from "../../context" +import _ from "lodash" + +import * as redis from "../../redis/init" +import { UserDB } from "../../users" + +const config = new DBTestConfiguration() + +describe("user cache", () => { + describe("getUsers", () => { + const users: User[] = [] + beforeAll(async () => { + const userCount = 10 + const userIds = generator.arrayOf(() => generator.guid(), { + min: userCount, + max: userCount, + }) + + await config.doInTenant(async () => { + const db = getGlobalDB() + for (const userId of userIds) { + const user = structures.users.user({ _id: userId }) + await db.put(user) + users.push(user) + } + }) + }) + + beforeEach(async () => { + jest.clearAllMocks() + + const redisClient = await redis.getUserClient() + await redisClient.clear() + }) + + it("when no user is in cache, all of them are retrieved from db", async () => { + const usersToRequest = _.sampleSize(users, 5) + + const userIdsToRequest = usersToRequest.map(x => x._id!) + + jest.spyOn(UserDB, "bulkGet") + + const results = await config.doInTenant(() => getUsers(userIdsToRequest)) + + expect(results.users).toHaveLength(5) + expect(results).toEqual({ + users: usersToRequest.map(u => ({ + ...u, + budibaseAccess: true, + _rev: expect.any(String), + })), + }) + + expect(UserDB.bulkGet).toBeCalledTimes(1) + expect(UserDB.bulkGet).toBeCalledWith(userIdsToRequest) + }) + + it("on a second all, all of them are retrieved from cache", async () => { + const usersToRequest = _.sampleSize(users, 5) + + const userIdsToRequest = usersToRequest.map(x => x._id!) + + jest.spyOn(UserDB, "bulkGet") + + await config.doInTenant(() => getUsers(userIdsToRequest)) + const resultsFromCache = await config.doInTenant(() => + getUsers(userIdsToRequest) + ) + + expect(resultsFromCache.users).toHaveLength(5) + expect(resultsFromCache).toEqual({ + users: expect.arrayContaining( + usersToRequest.map(u => ({ + ...u, + budibaseAccess: true, + _rev: expect.any(String), + })) + ), + }) + + expect(UserDB.bulkGet).toBeCalledTimes(1) + }) + + it("when some users are cached, only the missing ones are retrieved from db", async () => { + const usersToRequest = _.sampleSize(users, 5) + + const userIdsToRequest = usersToRequest.map(x => x._id!) + + jest.spyOn(UserDB, "bulkGet") + + await config.doInTenant(() => + getUsers([userIdsToRequest[0], userIdsToRequest[3]]) + ) + ;(UserDB.bulkGet as jest.Mock).mockClear() + + const results = await config.doInTenant(() => getUsers(userIdsToRequest)) + + expect(results.users).toHaveLength(5) + expect(results).toEqual({ + users: expect.arrayContaining( + usersToRequest.map(u => ({ + ...u, + budibaseAccess: true, + _rev: expect.any(String), + })) + ), + }) + + expect(UserDB.bulkGet).toBeCalledTimes(1) + expect(UserDB.bulkGet).toBeCalledWith([ + userIdsToRequest[1], + userIdsToRequest[2], + userIdsToRequest[4], + ]) + }) + + it("requesting existing and unexisting ids will return found ones", async () => { + const usersToRequest = _.sampleSize(users, 3) + const missingIds = [generator.guid(), generator.guid()] + + const userIdsToRequest = _.shuffle([ + ...missingIds, + ...usersToRequest.map(x => x._id!), + ]) + + const results = await config.doInTenant(() => getUsers(userIdsToRequest)) + + expect(results.users).toHaveLength(3) + expect(results).toEqual({ + users: expect.arrayContaining( + usersToRequest.map(u => ({ + ...u, + budibaseAccess: true, + _rev: expect.any(String), + })) + ), + notFoundIds: expect.arrayContaining(missingIds), + }) + }) + }) +}) diff --git a/packages/backend-core/src/cache/user.ts b/packages/backend-core/src/cache/user.ts index e2af78adfd..b3fd7c08cd 100644 --- a/packages/backend-core/src/cache/user.ts +++ b/packages/backend-core/src/cache/user.ts @@ -6,6 +6,7 @@ import env from "../environment" import * as accounts from "../accounts" import { UserDB } from "../users" import { sdk } from "@budibase/shared-core" +import { User } from "@budibase/types" const EXPIRY_SECONDS = 3600 @@ -27,6 +28,35 @@ async function populateFromDB(userId: string, tenantId: string) { return user } +async function populateUsersFromDB( + userIds: string[] +): Promise<{ users: User[]; notFoundIds?: string[] }> { + const getUsersResponse = await UserDB.bulkGet(userIds) + + // Handle missed user ids + const notFoundIds = userIds.filter((uid, i) => !getUsersResponse[i]) + + const users = getUsersResponse.filter(x => x) + + await Promise.all( + users.map(async (user: any) => { + user.budibaseAccess = true + if (!env.SELF_HOSTED && !env.DISABLE_ACCOUNT_PORTAL) { + const account = await accounts.getAccount(user.email) + if (account) { + user.account = account + user.accountPortalAccess = true + } + } + }) + ) + + if (notFoundIds.length) { + return { users, notFoundIds } + } + return { users } +} + /** * Get the requested user by id. * Use redis cache to first read the user. @@ -77,6 +107,36 @@ export async function getUser( return user } +/** + * Get the requested users by id. + * Use redis cache to first read the users. + * If not present fallback to loading the users directly and re-caching. + * @param {*} userIds the ids of the user to get + * @param {*} tenantId the tenant of the users to get + * @returns + */ +export async function getUsers( + userIds: string[] +): Promise<{ users: User[]; notFoundIds?: string[] }> { + const client = await redis.getUserClient() + // try cache + let usersFromCache = await client.bulkGet(userIds) + const missingUsersFromCache = userIds.filter(uid => !usersFromCache[uid]) + const users = Object.values(usersFromCache) + let notFoundIds + + if (missingUsersFromCache.length) { + const usersFromDb = await populateUsersFromDB(missingUsersFromCache) + + notFoundIds = usersFromDb.notFoundIds + for (const userToCache of usersFromDb.users) { + await client.store(userToCache._id!, userToCache, EXPIRY_SECONDS) + } + users.push(...usersFromDb.users) + } + return { users, notFoundIds: notFoundIds } +} + export async function invalidateUser(userId: string) { const client = await redis.getUserClient() await client.delete(userId) diff --git a/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts b/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts index 484a118cbd..c3ddf220e6 100644 --- a/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts +++ b/packages/backend-core/src/middleware/passport/sso/tests/sso.spec.ts @@ -102,6 +102,7 @@ describe("sso", () => { // modified external id to match user format ssoUser._id = "us_" + details.userId + delete ssoUser.userId // new sso user won't have a password delete ssoUser.password diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index 5056a5d549..78817d0aa0 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -250,7 +250,7 @@ class RedisWrapper { const prefixedKeys = keys.map(key => addDbPrefix(db, key)) let response = await this.getClient().mget(prefixedKeys) if (Array.isArray(response)) { - let final: any = {} + let final: Record = {} let count = 0 for (let result of response) { if (result) { diff --git a/packages/backend-core/tests/core/utilities/structures/shared.ts b/packages/backend-core/tests/core/utilities/structures/shared.ts deleted file mode 100644 index de0e19486c..0000000000 --- a/packages/backend-core/tests/core/utilities/structures/shared.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { User } from "@budibase/types" -import { generator } from "./generator" -import { uuid } from "./common" - -export const newEmail = () => { - return `${uuid()}@test.com` -} - -export const user = (userProps?: any): User => { - return { - email: newEmail(), - password: "test", - roles: { app_test: "admin" }, - firstName: generator.first(), - lastName: generator.last(), - pictureUrl: "http://test.com", - ...userProps, - } -} diff --git a/packages/backend-core/tests/core/utilities/structures/sso.ts b/packages/backend-core/tests/core/utilities/structures/sso.ts index 4d13635f09..2e3af712a9 100644 --- a/packages/backend-core/tests/core/utilities/structures/sso.ts +++ b/packages/backend-core/tests/core/utilities/structures/sso.ts @@ -13,8 +13,7 @@ import { } from "@budibase/types" import { generator } from "./generator" import { email, uuid } from "./common" -import * as shared from "./shared" -import { user } from "./shared" +import * as users from "./users" import sample from "lodash/sample" export function OAuth(): OAuth2 { @@ -26,7 +25,7 @@ export function OAuth(): OAuth2 { export function authDetails(userDoc?: User): SSOAuthDetails { if (!userDoc) { - userDoc = user() + userDoc = users.user() } const userId = userDoc._id || uuid() @@ -52,7 +51,7 @@ export function providerType(): SSOProviderType { export function ssoProfile(user?: User): SSOProfile { if (!user) { - user = shared.user() + user = users.user() } return { id: user._id!, diff --git a/packages/backend-core/tests/core/utilities/structures/users.ts b/packages/backend-core/tests/core/utilities/structures/users.ts index 0a4f2e8b54..420a9fde0e 100644 --- a/packages/backend-core/tests/core/utilities/structures/users.ts +++ b/packages/backend-core/tests/core/utilities/structures/users.ts @@ -4,11 +4,33 @@ import { BuilderUser, SSOAuthDetails, SSOUser, + User, } from "@budibase/types" -import { user } from "./shared" import { authDetails } from "./sso" +import { uuid } from "./common" +import { generator } from "./generator" +import { tenant } from "." +import { generateGlobalUserID } from "../../../../src/docIds" -export { user, newEmail } from "./shared" +export const newEmail = () => { + return `${uuid()}@test.com` +} + +export const user = (userProps?: Partial>): User => { + const userId = userProps?._id || generateGlobalUserID() + return { + _id: userId, + userId, + email: newEmail(), + password: "test", + roles: { app_test: "admin" }, + firstName: generator.first(), + lastName: generator.last(), + pictureUrl: "http://test.com", + tenantId: tenant.id(), + ...userProps, + } +} export const adminUser = (userProps?: any): AdminUser => { return { diff --git a/packages/bbui/src/Tooltip/AbsTooltip.svelte b/packages/bbui/src/Tooltip/AbsTooltip.svelte index 9be7251445..92d5af26bb 100644 --- a/packages/bbui/src/Tooltip/AbsTooltip.svelte +++ b/packages/bbui/src/Tooltip/AbsTooltip.svelte @@ -126,8 +126,9 @@ transition: top 130ms ease-out, left 130ms ease-out; } .spectrum-Tooltip-label { - text-overflow: ellipsis; - white-space: nowrap; + display: -webkit-box; + -webkit-line-clamp: 3; + -webkit-box-orient: vertical; overflow: hidden; font-size: 12px; font-weight: 600; diff --git a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte index 44c37813d6..75964af513 100644 --- a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte +++ b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte @@ -502,7 +502,7 @@ {#if datasource?.source !== "ORACLE" && datasource?.source !== "SQL_SERVER"}
-
+
diff --git a/packages/builder/src/components/common/CodeEditor/CodeEditor.svelte b/packages/builder/src/components/common/CodeEditor/CodeEditor.svelte index 7ab7c5dddf..76d7a58ef1 100644 --- a/packages/builder/src/components/common/CodeEditor/CodeEditor.svelte +++ b/packages/builder/src/components/common/CodeEditor/CodeEditor.svelte @@ -235,7 +235,7 @@ const baseExtensions = buildBaseExtensions() editor = new EditorView({ - doc: value, + doc: value?.toString(), extensions: buildExtensions(baseExtensions), parent: textarea, }) diff --git a/packages/client/src/components/app/forms/S3Upload.svelte b/packages/client/src/components/app/forms/S3Upload.svelte index dfc5032de9..9985c83bb8 100644 --- a/packages/client/src/components/app/forms/S3Upload.svelte +++ b/packages/client/src/components/app/forms/S3Upload.svelte @@ -17,6 +17,13 @@ let fieldApi let localFiles = [] + $: { + // If the field state is reset, clear the local files + if (!fieldState?.value?.length) { + localFiles = [] + } + } + const { API, notificationStore, uploadStore } = getContext("sdk") const component = getContext("component") diff --git a/packages/server/src/api/controllers/row/external.ts b/packages/server/src/api/controllers/row/external.ts index fb0e1c0bc0..719774f014 100644 --- a/packages/server/src/api/controllers/row/external.ts +++ b/packages/server/src/api/controllers/row/external.ts @@ -28,7 +28,7 @@ export async function handleRequest( ) { // make sure the filters are cleaned up, no empty strings for equals, fuzzy or string if (opts && opts.filters) { - opts.filters = utils.removeEmptyFilters(opts.filters) + opts.filters = sdk.rows.removeEmptyFilters(opts.filters) } if ( !dataFilters.hasFilters(opts?.filters) && diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index f0f2462019..6e0a6d979e 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -72,6 +72,11 @@ export const save = async (ctx: UserCtx) => { const tableId = utils.getTableId(ctx) const body = ctx.request.body + // user metadata doesn't exist yet - don't allow creation + if (utils.isUserMetadataTable(tableId) && !body._rev) { + ctx.throw(400, "Cannot create new user entry.") + } + // if it has an ID already then its a patch if (body && body._id) { return patch(ctx as UserCtx) diff --git a/packages/server/src/api/controllers/row/utils.ts b/packages/server/src/api/controllers/row/utils.ts index 192ba2109c..5f10fd9ad4 100644 --- a/packages/server/src/api/controllers/row/utils.ts +++ b/packages/server/src/api/controllers/row/utils.ts @@ -175,3 +175,7 @@ export function removeEmptyFilters(filters: SearchFilters) { } return filters } + +export function isUserMetadataTable(tableId: string) { + return tableId === InternalTables.USER_METADATA +} diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index a74a9f7960..6a021460ac 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -3,7 +3,7 @@ import { databaseTestProviders } from "../../../integrations/tests/utils" import tk from "timekeeper" import { outputProcessing } from "../../../utilities/rowProcessor" import * as setup from "./utilities" -import { context, roles, tenancy } from "@budibase/backend-core" +import { context, InternalTable, roles, tenancy } from "@budibase/backend-core" import { quotas } from "@budibase/pro" import { FieldType, @@ -1415,6 +1415,23 @@ describe.each([ }) }) + isInternal && + it("doesn't allow creating in user table", async () => { + const userTableId = InternalTable.USER_METADATA + const response = await config.api.row.save( + userTableId, + { + tableId: userTableId, + firstName: "Joe", + lastName: "Joe", + email: "joe@joe.com", + roles: {}, + }, + { expectStatus: 400 } + ) + expect(response.message).toBe("Cannot create new user entry.") + }) + describe("permissions", () => { let viewId: string let tableId: string diff --git a/packages/server/src/integrations/base/sql.ts b/packages/server/src/integrations/base/sql.ts index bf19ec9afe..add7596165 100644 --- a/packages/server/src/integrations/base/sql.ts +++ b/packages/server/src/integrations/base/sql.ts @@ -1,4 +1,8 @@ import { Knex, knex } from "knex" +import { db as dbCore } from "@budibase/backend-core" +import { QueryOptions } from "../../definitions/datasource" +import { isIsoDateString, SqlClient } from "../utils" +import SqlTableQueryBuilder from "./sqlTable" import { Operation, QueryJson, @@ -6,11 +10,8 @@ import { SearchFilters, SortDirection, } from "@budibase/types" -import { db as dbCore } from "@budibase/backend-core" -import { QueryOptions } from "../../definitions/datasource" -import { isIsoDateString, SqlClient } from "../utils" -import SqlTableQueryBuilder from "./sqlTable" import environment from "../../environment" +import { isValidFilter } from "../utils" const envLimit = environment.SQL_MAX_ROWS ? parseInt(environment.SQL_MAX_ROWS) @@ -261,15 +262,17 @@ class InternalBuilder { if (isEmptyObject(value.high)) { value.high = "" } - if (value.low && value.high) { + const lowValid = isValidFilter(value.low), + highValid = isValidFilter(value.high) + if (lowValid && highValid) { // Use a between operator if we have 2 valid range values const fnc = allOr ? "orWhereBetween" : "whereBetween" query = query[fnc](key, [value.low, value.high]) - } else if (value.low) { + } else if (lowValid) { // Use just a single greater than operator if we only have a low const fnc = allOr ? "orWhere" : "where" query = query[fnc](key, ">", value.low) - } else if (value.high) { + } else if (highValid) { // Use just a single less than operator if we only have a high const fnc = allOr ? "orWhere" : "where" query = query[fnc](key, "<", value.high) diff --git a/packages/server/src/integrations/utils.ts b/packages/server/src/integrations/utils.ts index 2883e4471c..75f4bcbfa1 100644 --- a/packages/server/src/integrations/utils.ts +++ b/packages/server/src/integrations/utils.ts @@ -1,6 +1,11 @@ -import { SourceName, SqlQuery, Datasource, Table } from "@budibase/types" +import { SqlQuery, Table, SearchFilters } from "@budibase/types" import { DocumentType, SEPARATOR } from "../db/utils" -import { FieldTypes, BuildSchemaErrors, InvalidColumns } from "../constants" +import { + FieldTypes, + BuildSchemaErrors, + InvalidColumns, + NoEmptyFilterStrings, +} from "../constants" import { helpers } from "@budibase/shared-core" const DOUBLE_SEPARATOR = `${SEPARATOR}${SEPARATOR}` @@ -343,3 +348,36 @@ export function getPrimaryDisplay(testValue: unknown): string | undefined { } return testValue as string } + +export function isValidFilter(value: any) { + return value != null && value !== "" +} + +// don't do a pure falsy check, as 0 is included +// https://github.com/Budibase/budibase/issues/10118 +export function removeEmptyFilters(filters: SearchFilters) { + for (let filterField of NoEmptyFilterStrings) { + if (!filters[filterField]) { + continue + } + + for (let filterType of Object.keys(filters)) { + if (filterType !== filterField) { + continue + } + // don't know which one we're checking, type could be anything + const value = filters[filterType] as unknown + if (typeof value === "object") { + for (let [key, value] of Object.entries( + filters[filterType] as object + )) { + if (value == null || value === "") { + // @ts-ignore + delete filters[filterField][key] + } + } + } + } + } + return filters +} diff --git a/packages/server/src/sdk/app/applications/applications.ts b/packages/server/src/sdk/app/applications/applications.ts index 865b277504..2604b59fa7 100644 --- a/packages/server/src/sdk/app/applications/applications.ts +++ b/packages/server/src/sdk/app/applications/applications.ts @@ -1,13 +1,14 @@ import { AppStatus } from "../../../db/utils" -import { App, ContextUser } from "@budibase/types" +import { App, ContextUser, User } from "@budibase/types" import { getLocksById } from "../../../utilities/redis" import { enrichApps } from "../../users/sessions" import { checkAppMetadata } from "../../../automations/logging" import { db as dbCore, users } from "@budibase/backend-core" +import { groups } from "@budibase/pro" -export function filterAppList(user: ContextUser, apps: App[]) { +export function filterAppList(user: User, apps: App[]) { let appList: string[] = [] - const roleApps = Object.keys(user.roles || {}) + const roleApps = Object.keys(user.roles) if (users.hasAppBuilderPermissions(user)) { appList = user.builder?.apps || [] appList = appList.concat(roleApps) @@ -23,7 +24,12 @@ export async function fetch(status: AppStatus, user: ContextUser) { const dev = status === AppStatus.DEV const all = status === AppStatus.ALL let apps = (await dbCore.getAllApps({ dev, all })) as App[] - apps = filterAppList(user, apps) + + const enrichedUser = await groups.enrichUserRolesFromGroups({ + ...user, + roles: user.roles || {}, + }) + apps = filterAppList(enrichedUser, apps) const appIds = apps .filter((app: any) => app.status === "development") diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 380521a05a..f75bd07437 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -3,6 +3,7 @@ import { isExternalTable } from "../../../integrations/utils" import * as internal from "./search/internal" import * as external from "./search/external" import { Format } from "../../../api/controllers/view/exporters" +export { isValidFilter, removeEmptyFilters } from "../../../integrations/utils" export interface ViewParams { calculation: string diff --git a/packages/server/src/api/controllers/row/tests/utils.spec.ts b/packages/server/src/sdk/tests/rows/search.spec.ts similarity index 71% rename from packages/server/src/api/controllers/row/tests/utils.spec.ts rename to packages/server/src/sdk/tests/rows/search.spec.ts index e0ad637e9d..feae5e7ee8 100644 --- a/packages/server/src/api/controllers/row/tests/utils.spec.ts +++ b/packages/server/src/sdk/tests/rows/search.spec.ts @@ -1,8 +1,8 @@ -import * as utils from "../utils" +import * as search from "../../app/rows/search" describe("removeEmptyFilters", () => { it("0 should not be removed", () => { - const filters = utils.removeEmptyFilters({ + const filters = search.removeEmptyFilters({ equal: { column: 0, }, @@ -11,7 +11,7 @@ describe("removeEmptyFilters", () => { }) it("empty string should be removed", () => { - const filters = utils.removeEmptyFilters({ + const filters = search.removeEmptyFilters({ equal: { column: "", },