diff --git a/packages/backend-core/src/middleware/passport/sso/sso.ts b/packages/backend-core/src/middleware/passport/sso/sso.ts index ee84f03dae..8901fcc56f 100644 --- a/packages/backend-core/src/middleware/passport/sso/sso.ts +++ b/packages/backend-core/src/middleware/passport/sso/sso.ts @@ -2,7 +2,6 @@ import { generateGlobalUserID } from "../../../db" import { authError } from "../utils" import * as users from "../../../users" import * as context from "../../../context" -import fetch from "node-fetch" import { SaveSSOUserFunction, SSOAuthDetails, @@ -97,28 +96,13 @@ export async function authenticate( return done(null, ssoUser) } -async function getProfilePictureUrl(user: User, details: SSOAuthDetails) { - const pictureUrl = details.profile?._json.picture - if (pictureUrl) { - const response = await fetch(pictureUrl) - if (response.status === 200) { - const type = response.headers.get("content-type") as string - if (type.startsWith("image/")) { - return pictureUrl - } - } - } -} - /** * @returns a user that has been sync'd with third party information */ async function syncUser(user: User, details: SSOAuthDetails): Promise { let firstName let lastName - let pictureUrl let oauth2 - let thirdPartyProfile if (details.profile) { const profile = details.profile @@ -134,12 +118,6 @@ async function syncUser(user: User, details: SSOAuthDetails): Promise { lastName = name.familyName } } - - pictureUrl = await getProfilePictureUrl(user, details) - - thirdPartyProfile = { - ...profile._json, - } } // oauth tokens for future use @@ -155,8 +133,6 @@ async function syncUser(user: User, details: SSOAuthDetails): Promise { providerType: details.providerType, firstName, lastName, - thirdPartyProfile, - pictureUrl, oauth2, } } diff --git a/packages/backend-core/tests/core/utilities/structures/accounts.ts b/packages/backend-core/tests/core/utilities/structures/accounts.ts index daf4965c81..7910f3c423 100644 --- a/packages/backend-core/tests/core/utilities/structures/accounts.ts +++ b/packages/backend-core/tests/core/utilities/structures/accounts.ts @@ -59,10 +59,8 @@ export function ssoAccount(account: Account = cloudAccount()): SSOAccount { accessToken: generator.string(), refreshToken: generator.string(), }, - pictureUrl: generator.url(), provider: provider(), providerType: providerType(), - thirdPartyProfile: {}, } } @@ -76,9 +74,7 @@ export function verifiableSsoAccount( accessToken: generator.string(), refreshToken: generator.string(), }, - pictureUrl: generator.url(), provider: AccountSSOProvider.MICROSOFT, providerType: AccountSSOProviderType.MICROSOFT, - thirdPartyProfile: { id: "abc123" }, } } diff --git a/packages/backend-core/tests/core/utilities/structures/users.ts b/packages/backend-core/tests/core/utilities/structures/users.ts index 0171353e23..ffddae663b 100644 --- a/packages/backend-core/tests/core/utilities/structures/users.ts +++ b/packages/backend-core/tests/core/utilities/structures/users.ts @@ -25,7 +25,6 @@ export const user = (userProps?: Partial>): User => { roles: { app_test: "admin" }, firstName: generator.first(), lastName: generator.last(), - pictureUrl: "http://example.com", tenantId: tenant.id(), ...userProps, } @@ -86,9 +85,5 @@ export function ssoUser( oauth2: opts.details?.oauth2, provider: opts.details?.provider!, providerType: opts.details?.providerType!, - thirdPartyProfile: { - email: base.email, - picture: base.pictureUrl, - }, } } diff --git a/packages/server/src/api/controllers/view/viewsV2.ts b/packages/server/src/api/controllers/view/viewsV2.ts index c98940ed88..f864df9e9e 100644 --- a/packages/server/src/api/controllers/view/viewsV2.ts +++ b/packages/server/src/api/controllers/view/viewsV2.ts @@ -127,13 +127,13 @@ export async function create(ctx: Ctx) { const parsedView: Omit, "id" | "version"> = { name: view.name, + type: view.type, tableId: view.tableId, query: view.query, queryUI: view.queryUI, sort: view.sort, schema, primaryDisplay: view.primaryDisplay, - uiMetadata: view.uiMetadata, } const result = await sdk.views.create(tableId, parsedView) ctx.status = 201 @@ -163,6 +163,7 @@ export async function update(ctx: Ctx) { const parsedView: RequiredKeys = { id: view.id, name: view.name, + type: view.type, version: view.version, tableId: view.tableId, query: view.query, @@ -170,7 +171,6 @@ export async function update(ctx: Ctx) { sort: view.sort, schema, primaryDisplay: view.primaryDisplay, - uiMetadata: view.uiMetadata, } const result = await sdk.views.update(tableId, parsedView) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index dce5a660b5..75b409dced 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -25,6 +25,7 @@ import { BBReferenceFieldSubType, NumericCalculationFieldMetadata, ViewV2Schema, + ViewV2Type, } from "@budibase/types" import { generator, mocks } from "@budibase/backend-core/tests" import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" @@ -143,7 +144,7 @@ describe.each([ }) it("can persist views with all fields", async () => { - const newView: Required> = { + const newView: Required> = { name: generator.name(), tableId: table._id!, primaryDisplay: "id", @@ -165,9 +166,6 @@ describe.each([ visible: true, }, }, - uiMetadata: { - foo: "bar", - }, } const res = await config.api.viewV2.create(newView) @@ -537,6 +535,7 @@ describe.each([ let view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { sum: { visible: true, @@ -560,11 +559,35 @@ describe.each([ expect(sum.field).toEqual("Price") }) + it("cannot create a view with calculation fields unless it has the right type", async () => { + await config.api.viewV2.create( + { + tableId: table._id!, + name: generator.guid(), + schema: { + sum: { + visible: true, + calculationType: CalculationType.SUM, + field: "Price", + }, + }, + }, + { + status: 400, + body: { + message: + "Calculation fields are not allowed in non-calculation views", + }, + } + ) + }) + it("cannot create a calculation view with more than 5 aggregations", async () => { await config.api.viewV2.create( { tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { sum: { visible: true, @@ -606,6 +629,113 @@ describe.each([ } ) }) + + it("cannot create a calculation view with duplicate calculations", async () => { + await config.api.viewV2.create( + { + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + sum: { + visible: true, + calculationType: CalculationType.SUM, + field: "Price", + }, + sum2: { + visible: true, + calculationType: CalculationType.SUM, + field: "Price", + }, + }, + }, + { + status: 400, + body: { + message: + 'Duplicate calculation on field "Price", calculation type "sum"', + }, + } + ) + }) + + it("finds duplicate counts", async () => { + await config.api.viewV2.create( + { + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + count: { + visible: true, + calculationType: CalculationType.COUNT, + }, + count2: { + visible: true, + calculationType: CalculationType.COUNT, + }, + }, + }, + { + status: 400, + body: { + message: + 'Duplicate calculation on field "*", calculation type "count"', + }, + } + ) + }) + + it("finds duplicate count distincts", async () => { + await config.api.viewV2.create( + { + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + count: { + visible: true, + calculationType: CalculationType.COUNT, + distinct: true, + field: "Price", + }, + count2: { + visible: true, + calculationType: CalculationType.COUNT, + distinct: true, + field: "Price", + }, + }, + }, + { + status: 400, + body: { + message: + 'Duplicate calculation on field "Price", calculation type "count"', + }, + } + ) + }) + + it("does not confuse counts and count distincts in the duplicate check", async () => { + await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + type: ViewV2Type.CALCULATION, + schema: { + count: { + visible: true, + calculationType: CalculationType.COUNT, + }, + count2: { + visible: true, + calculationType: CalculationType.COUNT, + distinct: true, + field: "Price", + }, + }, + }) + }) }) describe("update", () => { @@ -650,7 +780,9 @@ describe.each([ it("can update all fields", async () => { const tableId = table._id! - const updatedData: Required> = { + const updatedData: Required< + Omit + > = { version: view.version, id: view.id, tableId, @@ -678,9 +810,6 @@ describe.each([ readonly: true, }, }, - uiMetadata: { - foo: "bar", - }, } await config.api.viewV2.update(updatedData) @@ -865,6 +994,32 @@ describe.each([ ) }) + it("cannot update view type after creation", async () => { + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + schema: { + id: { visible: true }, + Price: { + visible: true, + }, + }, + }) + + await config.api.viewV2.update( + { + ...view, + type: ViewV2Type.CALCULATION, + }, + { + status: 400, + body: { + message: "Cannot update view type after creation", + }, + } + ) + }) + isInternal && it("updating schema will only validate modified field", async () => { let view = await config.api.viewV2.create({ @@ -937,6 +1092,7 @@ describe.each([ view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { country: { visible: true, @@ -2690,6 +2846,7 @@ describe.each([ it("should be able to search by calculations", async () => { const view = await config.api.viewV2.create({ tableId: table._id!, + type: ViewV2Type.CALCULATION, name: generator.guid(), schema: { "Quantity Sum": { @@ -2724,6 +2881,7 @@ describe.each([ const view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { quantity: { visible: true, @@ -2762,6 +2920,7 @@ describe.each([ const view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { aggregate: { visible: true, @@ -2822,6 +2981,7 @@ describe.each([ const view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { count: { visible: true, @@ -2856,6 +3016,7 @@ describe.each([ { tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { count: { visible: true, @@ -2904,6 +3065,7 @@ describe.each([ const view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), + type: ViewV2Type.CALCULATION, schema: { sum: { visible: true, diff --git a/packages/server/src/sdk/app/views/external.ts b/packages/server/src/sdk/app/views/external.ts index 3afd7e9bf9..b69ac0b9eb 100644 --- a/packages/server/src/sdk/app/views/external.ts +++ b/packages/server/src/sdk/app/views/external.ts @@ -70,6 +70,9 @@ export async function update(tableId: string, view: ViewV2): Promise { if (!existingView || !existingView.name) { throw new HTTPError(`View ${view.id} not found in table ${tableId}`, 404) } + if (isV2(existingView) && existingView.type !== view.type) { + throw new HTTPError(`Cannot update view type after creation`, 400) + } delete views[existingView.name] views[view.name] = view diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index e1568eac6e..07f24d8f02 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -72,11 +72,23 @@ async function guardCalculationViewSchema( ) } - for (const calculationFieldName of Object.keys(calculationFields)) { - const schema = calculationFields[calculationFieldName] + const seen: Record> = {} + + for (const name of Object.keys(calculationFields)) { + const schema = calculationFields[name] const isCount = schema.calculationType === CalculationType.COUNT const isDistinct = isCount && "distinct" in schema && schema.distinct + const field = isCount && !isDistinct ? "*" : schema.field + if (seen[field]?.[schema.calculationType]) { + throw new HTTPError( + `Duplicate calculation on field "${field}", calculation type "${schema.calculationType}"`, + 400 + ) + } + seen[field] ??= {} as Record + seen[field][schema.calculationType] = true + // Count fields that aren't distinct don't need to reference another field, // so we don't validate it. if (isCount && !isDistinct) { @@ -86,14 +98,14 @@ async function guardCalculationViewSchema( const targetSchema = table.schema[schema.field] if (!targetSchema) { throw new HTTPError( - `Calculation field "${calculationFieldName}" references field "${schema.field}" which does not exist in the table schema`, + `Calculation field "${name}" references field "${schema.field}" which does not exist in the table schema`, 400 ) } if (!isCount && !helpers.schema.isNumeric(targetSchema)) { throw new HTTPError( - `Calculation field "${calculationFieldName}" references field "${schema.field}" which is not a numeric field`, + `Calculation field "${name}" references field "${schema.field}" which is not a numeric field`, 400 ) } @@ -119,6 +131,13 @@ async function guardViewSchema( if (helpers.views.isCalculationView(view)) { await guardCalculationViewSchema(table, view) + } else { + if (helpers.views.hasCalculationFields(view)) { + throw new HTTPError( + "Calculation fields are not allowed in non-calculation views", + 400 + ) + } } await checkReadonlyFields(table, view) diff --git a/packages/server/src/sdk/app/views/internal.ts b/packages/server/src/sdk/app/views/internal.ts index 19a9f6ab14..96b41bffe8 100644 --- a/packages/server/src/sdk/app/views/internal.ts +++ b/packages/server/src/sdk/app/views/internal.ts @@ -59,6 +59,10 @@ export async function update(tableId: string, view: ViewV2): Promise { throw new HTTPError(`View ${view.id} not found in table ${tableId}`, 404) } + if (isV2(existingView) && existingView.type !== view.type) { + throw new HTTPError(`Cannot update view type after creation`, 400) + } + delete table.views[existingView.name] table.views[view.name] = view await db.put(table) diff --git a/packages/shared-core/src/helpers/views.ts b/packages/shared-core/src/helpers/views.ts index f41c66adc8..0113375adf 100644 --- a/packages/shared-core/src/helpers/views.ts +++ b/packages/shared-core/src/helpers/views.ts @@ -3,6 +3,7 @@ import { ViewCalculationFieldMetadata, ViewFieldMetadata, ViewV2, + ViewV2Type, } from "@budibase/types" import { pickBy } from "lodash" @@ -21,6 +22,10 @@ export function isBasicViewField( type UnsavedViewV2 = Omit export function isCalculationView(view: UnsavedViewV2) { + return view.type === ViewV2Type.CALCULATION +} + +export function hasCalculationFields(view: UnsavedViewV2) { return Object.values(view.schema || {}).some(isCalculationField) } diff --git a/packages/types/src/api/account/accounts.ts b/packages/types/src/api/account/accounts.ts index 1be506e14e..e05c1d0bf3 100644 --- a/packages/types/src/api/account/accounts.ts +++ b/packages/types/src/api/account/accounts.ts @@ -12,7 +12,6 @@ export interface CreateAccountRequest { name?: string password: string provider?: AccountSSOProvider - thirdPartyProfile: object } export interface SearchAccountsRequest { diff --git a/packages/types/src/documents/account/account.ts b/packages/types/src/documents/account/account.ts index c219229889..aac5bf2d20 100644 --- a/packages/types/src/documents/account/account.ts +++ b/packages/types/src/documents/account/account.ts @@ -98,8 +98,6 @@ export interface AccountSSO { provider: AccountSSOProvider providerType: AccountSSOProviderType oauth2?: OAuthTokens - pictureUrl?: string - thirdPartyProfile: any // TODO: define what the google profile looks like } export type SSOAccount = (Account | CloudAccount) & AccountSSO diff --git a/packages/types/src/documents/app/view.ts b/packages/types/src/documents/app/view.ts index c58852ecea..1b2372da85 100644 --- a/packages/types/src/documents/app/view.ts +++ b/packages/types/src/documents/app/view.ts @@ -79,10 +79,15 @@ export enum CalculationType { MAX = "max", } +export enum ViewV2Type { + CALCULATION = "calculation", +} + export interface ViewV2 { version: 2 id: string name: string + type?: ViewV2Type primaryDisplay?: string tableId: string query?: LegacyFilter[] | SearchFilters @@ -94,7 +99,6 @@ export interface ViewV2 { type?: SortType } schema?: ViewV2Schema - uiMetadata?: Record } export type ViewV2Schema = Record diff --git a/packages/types/src/documents/global/user.ts b/packages/types/src/documents/global/user.ts index 7605f013d1..a1c5b2506f 100644 --- a/packages/types/src/documents/global/user.ts +++ b/packages/types/src/documents/global/user.ts @@ -21,7 +21,6 @@ export interface UserSSO { provider: string // the individual provider e.g. Okta, Auth0, Google providerType: SSOProviderType oauth2?: OAuth2 - thirdPartyProfile?: SSOProfileJson profile?: { displayName?: string name?: { @@ -45,7 +44,6 @@ export interface User extends Document { userId?: string firstName?: string lastName?: string - pictureUrl?: string forceResetPassword?: boolean roles: UserRoles builder?: {