From f0e26ecf6ac0f666857e32bd7ecae1194b4a631f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 20 Mar 2025 16:44:05 +0100 Subject: [PATCH 01/11] Update crud usages --- packages/backend-core/src/docIds/params.ts | 10 ++ packages/server/src/sdk/app/oauth2/crud.ts | 102 ++++++++------------- packages/types/src/documents/app/oauth2.ts | 7 +- packages/types/src/documents/document.ts | 1 - 4 files changed, 50 insertions(+), 70 deletions(-) diff --git a/packages/backend-core/src/docIds/params.ts b/packages/backend-core/src/docIds/params.ts index 61708bb71b..748f681049 100644 --- a/packages/backend-core/src/docIds/params.ts +++ b/packages/backend-core/src/docIds/params.ts @@ -200,3 +200,13 @@ export function getStartEndKeyURL(baseKey: any, tenantId?: string) { export const getPluginParams = (pluginId?: string | null, otherProps = {}) => { return getDocParams(DocumentType.PLUGIN, pluginId, otherProps) } + +/** + * Gets parameters for retrieving OAuth2 configs, this is a utility function for the getDocParams function. + */ +export const getOAuth2ConfigParams = ( + configId?: string | null, + otherProps = {} +) => { + return getDocParams(DocumentType.OAUTH2_CONFIG, configId, otherProps) +} diff --git a/packages/server/src/sdk/app/oauth2/crud.ts b/packages/server/src/sdk/app/oauth2/crud.ts index dd1a0fa1c7..353b09d693 100644 --- a/packages/server/src/sdk/app/oauth2/crud.ts +++ b/packages/server/src/sdk/app/oauth2/crud.ts @@ -1,101 +1,77 @@ -import { context, HTTPError, utils } from "@budibase/backend-core" +import { context, docIds, HTTPError, utils } from "@budibase/backend-core" import { - Database, DocumentType, OAuth2Config, - OAuth2Configs, PASSWORD_REPLACEMENT, SEPARATOR, - VirtualDocumentType, + WithRequired, } from "@budibase/types" -async function getDocument(db: Database = context.getAppDB()) { - const result = await db.tryGet(DocumentType.OAUTH2_CONFIG) - return result +async function guardName(name: string, id?: string) { + const existingConfigs = await fetch() + + if (existingConfigs.find(c => c.name === name && c._id !== id)) { + throw new HTTPError( + `OAuth2 config with name '${name}' is already taken.`, + 400 + ) + } } export async function fetch(): Promise { - const result = await getDocument() - if (!result) { - return [] - } - return Object.values(result.configs) + const db = context.getAppDB() + const docs = await db.allDocs(docIds.getOAuth2ConfigParams()) + const result = docs.rows.map(r => r.doc!) + return result } export async function create( config: Omit ): Promise { const db = context.getAppDB() - const doc: OAuth2Configs = (await getDocument(db)) ?? { - _id: DocumentType.OAUTH2_CONFIG, - configs: {}, - } - if (Object.values(doc.configs).find(c => c.name === config.name)) { - throw new HTTPError("Name already used", 400) - } + await guardName(config.name) - const id = `${VirtualDocumentType.OAUTH2_CONFIG}${SEPARATOR}${utils.newid()}` - doc.configs[id] = { - id, + const response = await db.put({ + id: `${DocumentType.OAUTH2_CONFIG}${SEPARATOR}${utils.newid()}`, + ...config, + }) + return { + _id: response.id, + _rev: response.rev, ...config, } - - await db.put(doc) - return doc.configs[id] } export async function get(id: string): Promise { - const doc = await getDocument() - return doc?.configs?.[id] + const db = context.getAppDB() + return await db.tryGet(id) } -export async function update(config: OAuth2Config): Promise { +export async function update( + config: WithRequired +): Promise { const db = context.getAppDB() - const doc: OAuth2Configs = (await getDocument(db)) ?? { - _id: DocumentType.OAUTH2_CONFIG, - configs: {}, + await guardName(config.name, config._id) + + const existing = await get(config._id) + if (!existing) { + throw new HTTPError(`OAuth2 config with id '${config._id}' not found.`, 404) } - if (!doc.configs[config.id]) { - throw new HTTPError(`OAuth2 config with id '${config.id}' not found.`, 404) - } - - if ( - Object.values(doc.configs).find( - c => c.name === config.name && c.id !== config.id - ) - ) { - throw new HTTPError( - `OAuth2 config with name '${config.name}' is already taken.`, - 400 - ) - } - - doc.configs[config.id] = { + const toUpdate = { ...config, clientSecret: config.clientSecret === PASSWORD_REPLACEMENT - ? doc.configs[config.id].clientSecret + ? existing.clientSecret : config.clientSecret, } - await db.put(doc) - return doc.configs[config.id] + const result = await db.put(toUpdate) + return { ...toUpdate, _rev: result.rev } } -export async function remove(configId: string): Promise { +export async function remove(configId: string, _rev: string): Promise { const db = context.getAppDB() - const doc: OAuth2Configs = (await getDocument(db)) ?? { - _id: DocumentType.OAUTH2_CONFIG, - configs: {}, - } - - if (!doc.configs[configId]) { - throw new HTTPError(`OAuth2 config with id '${configId}' not found.`, 404) - } - - delete doc.configs[configId] - - await db.put(doc) + await db.remove(configId, _rev) } diff --git a/packages/types/src/documents/app/oauth2.ts b/packages/types/src/documents/app/oauth2.ts index 74ed11ab70..d2ad895529 100644 --- a/packages/types/src/documents/app/oauth2.ts +++ b/packages/types/src/documents/app/oauth2.ts @@ -5,15 +5,10 @@ export enum OAuth2CredentialsMethod { BODY = "BODY", } -export interface OAuth2Config { - id: string +export interface OAuth2Config extends Document { name: string url: string clientId: string clientSecret: string method: OAuth2CredentialsMethod } - -export interface OAuth2Configs extends Document { - configs: Record -} diff --git a/packages/types/src/documents/document.ts b/packages/types/src/documents/document.ts index a4d8ad11ac..eadf2e6b71 100644 --- a/packages/types/src/documents/document.ts +++ b/packages/types/src/documents/document.ts @@ -82,7 +82,6 @@ export enum InternalTable { export enum VirtualDocumentType { VIEW = "view", ROW_ACTION = "row_action", - OAUTH2_CONFIG = "oauth2", } // Because VirtualDocumentTypes can overlap, we need to make sure that we search From 7e48bd2e4bec19c15ea3f129a749416c6d77bf19 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 20 Mar 2025 16:50:07 +0100 Subject: [PATCH 02/11] Fix crud --- packages/server/src/sdk/app/oauth2/crud.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/server/src/sdk/app/oauth2/crud.ts b/packages/server/src/sdk/app/oauth2/crud.ts index 353b09d693..cd368814b7 100644 --- a/packages/server/src/sdk/app/oauth2/crud.ts +++ b/packages/server/src/sdk/app/oauth2/crud.ts @@ -20,7 +20,9 @@ async function guardName(name: string, id?: string) { export async function fetch(): Promise { const db = context.getAppDB() - const docs = await db.allDocs(docIds.getOAuth2ConfigParams()) + const docs = await db.allDocs( + docIds.getOAuth2ConfigParams(null, { include_docs: true }) + ) const result = docs.rows.map(r => r.doc!) return result } @@ -33,7 +35,7 @@ export async function create( await guardName(config.name) const response = await db.put({ - id: `${DocumentType.OAUTH2_CONFIG}${SEPARATOR}${utils.newid()}`, + _id: `${DocumentType.OAUTH2_CONFIG}${SEPARATOR}${utils.newid()}`, ...config, }) return { From d3e4604d96e1ca2c0009f61beeb153ee6096f35e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 20 Mar 2025 16:50:12 +0100 Subject: [PATCH 03/11] Type --- packages/backend-core/src/docIds/params.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/docIds/params.ts b/packages/backend-core/src/docIds/params.ts index 748f681049..76d4efbde7 100644 --- a/packages/backend-core/src/docIds/params.ts +++ b/packages/backend-core/src/docIds/params.ts @@ -206,7 +206,7 @@ export const getPluginParams = (pluginId?: string | null, otherProps = {}) => { */ export const getOAuth2ConfigParams = ( configId?: string | null, - otherProps = {} + otherProps: Partial = {} ) => { return getDocParams(DocumentType.OAUTH2_CONFIG, configId, otherProps) } From 47f8ce025f5ea19a1ab2f21c6a1153d9038309f7 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 20 Mar 2025 16:59:31 +0100 Subject: [PATCH 04/11] Fix create tests --- packages/server/src/api/controllers/oauth2.ts | 9 ++-- .../src/api/routes/tests/oauth2.spec.ts | 44 ++++++++++--------- packages/server/src/sdk/app/oauth2/crud.ts | 2 +- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/packages/server/src/api/controllers/oauth2.ts b/packages/server/src/api/controllers/oauth2.ts index 75c86ba888..b006d1053d 100644 --- a/packages/server/src/api/controllers/oauth2.ts +++ b/packages/server/src/api/controllers/oauth2.ts @@ -4,7 +4,6 @@ import { Ctx, FetchOAuth2ConfigsResponse, OAuth2Config, - RequiredKeys, OAuth2ConfigResponse, PASSWORD_REPLACEMENT, ValidateConfigResponse, @@ -16,7 +15,7 @@ function toFetchOAuth2ConfigsResponse( config: OAuth2Config ): OAuth2ConfigResponse { return { - id: config.id, + id: config._id!, name: config.name, url: config.url, clientId: config.clientId, @@ -38,7 +37,7 @@ export async function create( ctx: Ctx ) { const { body } = ctx.request - const newConfig: RequiredKeys> = { + const newConfig = { name: body.name, url: body.url, clientId: body.clientId, @@ -57,8 +56,8 @@ export async function edit( ctx: Ctx ) { const { body } = ctx.request - const toUpdate: RequiredKeys = { - id: ctx.params.id, + const toUpdate = { + _id: body._id, name: body.name, url: body.url, clientId: body.clientId, diff --git a/packages/server/src/api/routes/tests/oauth2.spec.ts b/packages/server/src/api/routes/tests/oauth2.spec.ts index ea63abd997..edfe1ad09e 100644 --- a/packages/server/src/api/routes/tests/oauth2.spec.ts +++ b/packages/server/src/api/routes/tests/oauth2.spec.ts @@ -1,9 +1,9 @@ import { + DocumentType, OAuth2Config, OAuth2CredentialsMethod, PASSWORD_REPLACEMENT, UpsertOAuth2ConfigRequest, - VirtualDocumentType, } from "@budibase/types" import * as setup from "./utilities" import { generator } from "@budibase/backend-core/tests" @@ -27,7 +27,7 @@ describe("/oauth2", () => { beforeEach(async () => await config.newTenant()) const expectOAuth2ConfigId = expect.stringMatching( - `^${VirtualDocumentType.OAUTH2_CONFIG}_.+$` + `^${DocumentType.OAUTH2_CONFIG}_.+$` ) describe("fetch", () => { @@ -90,24 +90,26 @@ describe("/oauth2", () => { await config.api.oauth2.create(oauth2Config2, { status: 201 }) const response = await config.api.oauth2.fetch() - expect(response.configs).toEqual([ - { - id: expectOAuth2ConfigId, - name: oauth2Config.name, - url: oauth2Config.url, - clientId: oauth2Config.clientId, - clientSecret: PASSWORD_REPLACEMENT, - method: oauth2Config.method, - }, - { - id: expectOAuth2ConfigId, - name: oauth2Config2.name, - url: oauth2Config2.url, - clientId: oauth2Config2.clientId, - clientSecret: PASSWORD_REPLACEMENT, - method: oauth2Config2.method, - }, - ]) + expect(response.configs).toEqual( + expect.arrayContaining([ + { + id: expectOAuth2ConfigId, + name: oauth2Config.name, + url: oauth2Config.url, + clientId: oauth2Config.clientId, + clientSecret: PASSWORD_REPLACEMENT, + method: oauth2Config.method, + }, + { + id: expectOAuth2ConfigId, + name: oauth2Config2.name, + url: oauth2Config2.url, + clientId: oauth2Config2.clientId, + clientSecret: PASSWORD_REPLACEMENT, + method: oauth2Config2.method, + }, + ]) + ) expect(response.configs[0].id).not.toEqual(response.configs[1].id) }) @@ -118,7 +120,7 @@ describe("/oauth2", () => { await config.api.oauth2.create(oauth2Config2, { status: 400, body: { - message: "Name already used", + message: `OAuth2 config with name '${oauth2Config.name}' is already taken.`, status: 400, }, }) diff --git a/packages/server/src/sdk/app/oauth2/crud.ts b/packages/server/src/sdk/app/oauth2/crud.ts index cd368814b7..42eba5dd0c 100644 --- a/packages/server/src/sdk/app/oauth2/crud.ts +++ b/packages/server/src/sdk/app/oauth2/crud.ts @@ -28,7 +28,7 @@ export async function fetch(): Promise { } export async function create( - config: Omit + config: Omit ): Promise { const db = context.getAppDB() From 6f34f0c2beeaeb0fec80e1b880486025a0ca8612 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 20 Mar 2025 17:31:11 +0100 Subject: [PATCH 05/11] Fix crud --- packages/server/src/api/controllers/oauth2.ts | 28 +++++---- packages/server/src/api/routes/oauth2.ts | 49 ++++++++------- .../src/api/routes/tests/oauth2.spec.ts | 63 +++++++++++-------- packages/server/src/sdk/app/oauth2/crud.ts | 9 ++- .../server/src/tests/utilities/api/oauth2.ts | 19 +++--- packages/types/src/api/web/app/oauth2.ts | 23 +++++-- 6 files changed, 116 insertions(+), 75 deletions(-) diff --git a/packages/server/src/api/controllers/oauth2.ts b/packages/server/src/api/controllers/oauth2.ts index b006d1053d..0d18a33b18 100644 --- a/packages/server/src/api/controllers/oauth2.ts +++ b/packages/server/src/api/controllers/oauth2.ts @@ -1,6 +1,4 @@ import { - UpsertOAuth2ConfigRequest, - UpsertOAuth2ConfigResponse, Ctx, FetchOAuth2ConfigsResponse, OAuth2Config, @@ -8,6 +6,10 @@ import { PASSWORD_REPLACEMENT, ValidateConfigResponse, ValidateConfigRequest, + InsertOAuth2ConfigRequest, + InsertOAuth2ConfigResponse, + UpdateOAuth2ConfigRequest, + UpdateOAuth2ConfigResponse, } from "@budibase/types" import sdk from "../../sdk" @@ -15,7 +17,8 @@ function toFetchOAuth2ConfigsResponse( config: OAuth2Config ): OAuth2ConfigResponse { return { - id: config._id!, + _id: config._id!, + _rev: config._rev!, name: config.name, url: config.url, clientId: config.clientId, @@ -34,7 +37,7 @@ export async function fetch(ctx: Ctx) { } export async function create( - ctx: Ctx + ctx: Ctx ) { const { body } = ctx.request const newConfig = { @@ -53,11 +56,12 @@ export async function create( } export async function edit( - ctx: Ctx + ctx: Ctx ) { const { body } = ctx.request const toUpdate = { _id: body._id, + _rev: body._rev, name: body.name, url: body.url, clientId: body.clientId, @@ -71,12 +75,10 @@ export async function edit( } } -export async function remove( - ctx: Ctx -) { - const configToRemove = ctx.params.id +export async function remove(ctx: Ctx) { + const { id, rev } = ctx.params - await sdk.oauth2.remove(configToRemove) + await sdk.oauth2.remove(id, rev) ctx.status = 204 } @@ -91,10 +93,10 @@ export async function validate( method: body.method, } - if (config.clientSecret === PASSWORD_REPLACEMENT && body.id) { - const existingConfig = await sdk.oauth2.get(body.id) + if (config.clientSecret === PASSWORD_REPLACEMENT && body._id) { + const existingConfig = await sdk.oauth2.get(body._id) if (!existingConfig) { - ctx.throw(`OAuth2 config with id '${body.id}' not found.`, 404) + ctx.throw(`OAuth2 config with id '${body._id}' not found.`, 404) } config.clientSecret = existingConfig.clientSecret diff --git a/packages/server/src/api/routes/oauth2.ts b/packages/server/src/api/routes/oauth2.ts index b3a16974d1..7dae4be57d 100644 --- a/packages/server/src/api/routes/oauth2.ts +++ b/packages/server/src/api/routes/oauth2.ts @@ -6,7 +6,7 @@ import authorized from "../../middleware/authorized" import * as controller from "../controllers/oauth2" import Joi from "joi" -const baseValidation = { +const baseSchema = { url: Joi.string().required(), clientId: Joi.string().required(), clientSecret: Joi.string().required(), @@ -15,24 +15,27 @@ const baseValidation = { .valid(...Object.values(OAuth2CredentialsMethod)), } -function oAuth2ConfigValidator() { - return middleware.joiValidator.body( - Joi.object({ - name: Joi.string().required(), - ...baseValidation, - }), - { allowUnknown: false } - ) -} +const insertSchema = Joi.object({ + name: Joi.string().required(), + ...baseSchema, +}) -function oAuth2ConfigValidationValidator() { - return middleware.joiValidator.body( - Joi.object({ - id: Joi.string().required(), - ...baseValidation, - }), - { allowUnknown: false } - ) +const updateSchema = Joi.object({ + _id: Joi.string().required(), + _rev: Joi.string().required(), + name: Joi.string().required(), + ...baseSchema, +}) + +const validationSchema = Joi.object({ + name: Joi.string().required(), + ...baseSchema, +}) + +function oAuth2ConfigValidator( + schema: typeof validationSchema | typeof insertSchema | typeof updateSchema +) { + return middleware.joiValidator.body(schema, { allowUnknown: false }) } const router: Router = new Router() @@ -41,24 +44,24 @@ router.get("/api/oauth2", authorized(PermissionType.BUILDER), controller.fetch) router.post( "/api/oauth2", authorized(PermissionType.BUILDER), - oAuth2ConfigValidator(), + oAuth2ConfigValidator(insertSchema), controller.create ) router.put( - "/api/oauth2/:id", + "/api/oauth2", authorized(PermissionType.BUILDER), - oAuth2ConfigValidator(), + oAuth2ConfigValidator(updateSchema), controller.edit ) router.delete( - "/api/oauth2/:id", + "/api/oauth2/:id/:rev", authorized(PermissionType.BUILDER), controller.remove ) router.post( "/api/oauth2/validate", authorized(PermissionType.BUILDER), - oAuth2ConfigValidationValidator(), + oAuth2ConfigValidator(validationSchema), controller.validate ) diff --git a/packages/server/src/api/routes/tests/oauth2.spec.ts b/packages/server/src/api/routes/tests/oauth2.spec.ts index edfe1ad09e..7acd0cbdb2 100644 --- a/packages/server/src/api/routes/tests/oauth2.spec.ts +++ b/packages/server/src/api/routes/tests/oauth2.spec.ts @@ -1,9 +1,9 @@ import { DocumentType, - OAuth2Config, + InsertOAuth2ConfigRequest, + OAuth2ConfigResponse, OAuth2CredentialsMethod, PASSWORD_REPLACEMENT, - UpsertOAuth2ConfigRequest, } from "@budibase/types" import * as setup from "./utilities" import { generator } from "@budibase/backend-core/tests" @@ -12,7 +12,7 @@ import _ from "lodash/fp" describe("/oauth2", () => { let config = setup.getConfig() - function makeOAuth2Config(): UpsertOAuth2ConfigRequest { + function makeOAuth2Config(): InsertOAuth2ConfigRequest { return { name: generator.guid(), url: generator.url(), @@ -43,7 +43,7 @@ describe("/oauth2", () => { for (let i = 0; i < 10; i++) { const oauth2Config = makeOAuth2Config() const result = await config.api.oauth2.create(oauth2Config) - existingConfigs.push({ ...oauth2Config, id: result.config.id }) + existingConfigs.push(result.config) } const response = await config.api.oauth2.fetch() @@ -51,7 +51,8 @@ describe("/oauth2", () => { expect(response).toEqual({ configs: expect.arrayContaining( existingConfigs.map(c => ({ - id: c.id, + _id: c._id, + _rev: c._rev, name: c.name, url: c.url, clientId: c.clientId, @@ -72,7 +73,8 @@ describe("/oauth2", () => { expect(response).toEqual({ configs: [ { - id: expectOAuth2ConfigId, + _id: expectOAuth2ConfigId, + _rev: expect.stringMatching(/^1-\w+/), name: oauth2Config.name, url: oauth2Config.url, clientId: oauth2Config.clientId, @@ -93,7 +95,8 @@ describe("/oauth2", () => { expect(response.configs).toEqual( expect.arrayContaining([ { - id: expectOAuth2ConfigId, + _id: expectOAuth2ConfigId, + _rev: expect.stringMatching(/^1-\w+/), name: oauth2Config.name, url: oauth2Config.url, clientId: oauth2Config.clientId, @@ -101,7 +104,8 @@ describe("/oauth2", () => { method: oauth2Config.method, }, { - id: expectOAuth2ConfigId, + _id: expectOAuth2ConfigId, + _rev: expect.stringMatching(/^1-\w+/), name: oauth2Config2.name, url: oauth2Config2.url, clientId: oauth2Config2.clientId, @@ -110,7 +114,7 @@ describe("/oauth2", () => { }, ]) ) - expect(response.configs[0].id).not.toEqual(response.configs[1].id) + expect(response.configs[0]._id).not.toEqual(response.configs[1]._id) }) it("cannot create configurations with already existing names", async () => { @@ -128,7 +132,8 @@ describe("/oauth2", () => { const response = await config.api.oauth2.fetch() expect(response.configs).toEqual([ { - id: expectOAuth2ConfigId, + _id: expectOAuth2ConfigId, + _rev: expect.stringMatching(/^1-\w+/), name: oauth2Config.name, url: oauth2Config.url, clientId: oauth2Config.clientId, @@ -140,7 +145,7 @@ describe("/oauth2", () => { }) describe("update", () => { - let existingConfigs: OAuth2Config[] = [] + let existingConfigs: OAuth2ConfigResponse[] = [] beforeEach(async () => { existingConfigs = [] @@ -148,14 +153,14 @@ describe("/oauth2", () => { const oauth2Config = makeOAuth2Config() const result = await config.api.oauth2.create(oauth2Config) - existingConfigs.push({ ...oauth2Config, id: result.config.id }) + existingConfigs.push(result.config) } }) it("can update an existing configuration", async () => { - const { id: configId, ...configData } = _.sample(existingConfigs)! + const configData = _.sample(existingConfigs)! - await config.api.oauth2.update(configId, { + await config.api.oauth2.update({ ...configData, name: "updated name", }) @@ -165,7 +170,8 @@ describe("/oauth2", () => { expect(response.configs).toEqual( expect.arrayContaining([ { - id: configId, + _id: configData._id, + _rev: expect.not.stringMatching(configData._rev), name: "updated name", url: configData.url, clientId: configData.clientId, @@ -177,7 +183,12 @@ describe("/oauth2", () => { }) it("throw if config not found", async () => { - await config.api.oauth2.update("unexisting", makeOAuth2Config(), { + const toUpdate = { + ...makeOAuth2Config(), + _id: "unexisting", + _rev: "unexisting", + } + await config.api.oauth2.update(toUpdate, { status: 404, body: { message: "OAuth2 config with id 'unexisting' not found." }, }) @@ -185,12 +196,10 @@ describe("/oauth2", () => { it("throws if trying to use an existing name", async () => { const [config1, config2] = _.sampleSize(2, existingConfigs) - const { id: configId, ...configData } = config1 await config.api.oauth2.update( - configId, { - ...configData, + ...config1, name: config2.name, }, { @@ -204,7 +213,7 @@ describe("/oauth2", () => { }) describe("delete", () => { - let existingConfigs: OAuth2Config[] = [] + let existingConfigs: OAuth2ConfigResponse[] = [] beforeEach(async () => { existingConfigs = [] @@ -212,22 +221,26 @@ describe("/oauth2", () => { const oauth2Config = makeOAuth2Config() const result = await config.api.oauth2.create(oauth2Config) - existingConfigs.push({ ...oauth2Config, id: result.config.id }) + existingConfigs.push(result.config) } }) it("can delete an existing configuration", async () => { - const { id: configId } = _.sample(existingConfigs)! + const configToDelete = _.sample(existingConfigs)! - await config.api.oauth2.delete(configId, { status: 204 }) + await config.api.oauth2.delete(configToDelete._id, configToDelete._rev, { + status: 204, + }) const response = await config.api.oauth2.fetch() expect(response.configs).toHaveLength(existingConfigs.length - 1) - expect(response.configs.find(c => c.id === configId)).toBeUndefined() + expect( + response.configs.find(c => c._id === configToDelete._id) + ).toBeUndefined() }) it("throw if config not found", async () => { - await config.api.oauth2.delete("unexisting", { + await config.api.oauth2.delete("unexisting", "rev", { status: 404, body: { message: "OAuth2 config with id 'unexisting' not found." }, }) diff --git a/packages/server/src/sdk/app/oauth2/crud.ts b/packages/server/src/sdk/app/oauth2/crud.ts index 42eba5dd0c..3cf0bb96c4 100644 --- a/packages/server/src/sdk/app/oauth2/crud.ts +++ b/packages/server/src/sdk/app/oauth2/crud.ts @@ -75,5 +75,12 @@ export async function update( export async function remove(configId: string, _rev: string): Promise { const db = context.getAppDB() - await db.remove(configId, _rev) + try { + await db.remove(configId, _rev) + } catch (e: any) { + if (e.status === 404) { + throw new HTTPError(`OAuth2 config with id '${configId}' not found.`, 404) + } + throw e + } } diff --git a/packages/server/src/tests/utilities/api/oauth2.ts b/packages/server/src/tests/utilities/api/oauth2.ts index d4c99c9598..77ac5da590 100644 --- a/packages/server/src/tests/utilities/api/oauth2.ts +++ b/packages/server/src/tests/utilities/api/oauth2.ts @@ -1,7 +1,9 @@ import { - UpsertOAuth2ConfigRequest, - UpsertOAuth2ConfigResponse, + InsertOAuth2ConfigRequest, + InsertOAuth2ConfigResponse, FetchOAuth2ConfigsResponse, + UpdateOAuth2ConfigRequest, + UpdateOAuth2ConfigResponse, } from "@budibase/types" import { Expectations, TestAPI } from "./base" @@ -13,10 +15,10 @@ export class OAuth2API extends TestAPI { } create = async ( - body: UpsertOAuth2ConfigRequest, + body: InsertOAuth2ConfigRequest, expectations?: Expectations ) => { - return await this._post("/api/oauth2", { + return await this._post("/api/oauth2", { body, expectations: { status: expectations?.status ?? 201, @@ -26,18 +28,17 @@ export class OAuth2API extends TestAPI { } update = async ( - id: string, - body: UpsertOAuth2ConfigRequest, + body: UpdateOAuth2ConfigRequest, expectations?: Expectations ) => { - return await this._put(`/api/oauth2/${id}`, { + return await this._put(`/api/oauth2`, { body, expectations, }) } - delete = async (id: string, expectations?: Expectations) => { - return await this._delete(`/api/oauth2/${id}`, { + delete = async (id: string, rev: string, expectations?: Expectations) => { + return await this._delete(`/api/oauth2/${id}/${rev}`, { expectations, }) } diff --git a/packages/types/src/api/web/app/oauth2.ts b/packages/types/src/api/web/app/oauth2.ts index 3a0bcdf72d..b6d4d89ea6 100644 --- a/packages/types/src/api/web/app/oauth2.ts +++ b/packages/types/src/api/web/app/oauth2.ts @@ -1,7 +1,8 @@ import { OAuth2CredentialsMethod } from "@budibase/types" export interface OAuth2ConfigResponse { - id: string + _id: string + _rev: string name: string url: string clientId: string @@ -13,7 +14,7 @@ export interface FetchOAuth2ConfigsResponse { configs: OAuth2ConfigResponse[] } -export interface UpsertOAuth2ConfigRequest { +export interface InsertOAuth2ConfigRequest { name: string url: string clientId: string @@ -21,12 +22,26 @@ export interface UpsertOAuth2ConfigRequest { method: OAuth2CredentialsMethod } -export interface UpsertOAuth2ConfigResponse { +export interface InsertOAuth2ConfigResponse { + config: OAuth2ConfigResponse +} + +export interface UpdateOAuth2ConfigRequest { + _id: string + _rev: string + name: string + url: string + clientId: string + clientSecret: string + method: OAuth2CredentialsMethod +} + +export interface UpdateOAuth2ConfigResponse { config: OAuth2ConfigResponse } export interface ValidateConfigRequest { - id?: string + _id?: string url: string clientId: string clientSecret: string From 52bf5dc8af22a9e53e15e01a8226cda94e81c259 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 20 Mar 2025 17:34:56 +0100 Subject: [PATCH 06/11] Fix generate token --- packages/server/src/sdk/app/oauth2/crud.ts | 20 ++++++++++++------- .../src/sdk/app/oauth2/tests/utils.spec.ts | 8 ++++---- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/server/src/sdk/app/oauth2/crud.ts b/packages/server/src/sdk/app/oauth2/crud.ts index 3cf0bb96c4..1a92070aee 100644 --- a/packages/server/src/sdk/app/oauth2/crud.ts +++ b/packages/server/src/sdk/app/oauth2/crud.ts @@ -7,6 +7,8 @@ import { WithRequired, } from "@budibase/types" +type CreatedOAuthConfig = WithRequired + async function guardName(name: string, id?: string) { const existingConfigs = await fetch() @@ -18,18 +20,22 @@ async function guardName(name: string, id?: string) { } } -export async function fetch(): Promise { +export async function fetch(): Promise { const db = context.getAppDB() const docs = await db.allDocs( docIds.getOAuth2ConfigParams(null, { include_docs: true }) ) - const result = docs.rows.map(r => r.doc!) + const result = docs.rows.map(r => ({ + ...r.doc!, + _id: r.doc!._id!, + _rev: r.doc!._rev!, + })) return result } export async function create( config: Omit -): Promise { +): Promise { const db = context.getAppDB() await guardName(config.name) @@ -39,8 +45,8 @@ export async function create( ...config, }) return { - _id: response.id, - _rev: response.rev, + _id: response.id!, + _rev: response.rev!, ...config, } } @@ -51,8 +57,8 @@ export async function get(id: string): Promise { } export async function update( - config: WithRequired -): Promise { + config: CreatedOAuthConfig +): Promise { const db = context.getAppDB() await guardName(config.name, config._id) diff --git a/packages/server/src/sdk/app/oauth2/tests/utils.spec.ts b/packages/server/src/sdk/app/oauth2/tests/utils.spec.ts index 2f8b151908..6cb33b9836 100644 --- a/packages/server/src/sdk/app/oauth2/tests/utils.spec.ts +++ b/packages/server/src/sdk/app/oauth2/tests/utils.spec.ts @@ -55,7 +55,7 @@ describe("oauth2 utils", () => { method, }) - const response = await generateToken(oauthConfig.id) + const response = await generateToken(oauthConfig._id) return response }) @@ -73,7 +73,7 @@ describe("oauth2 utils", () => { method, }) - await generateToken(oauthConfig.id) + await generateToken(oauthConfig._id) }) ).rejects.toThrow("Error fetching oauth2 token: Not Found") }) @@ -89,7 +89,7 @@ describe("oauth2 utils", () => { method, }) - await generateToken(oauthConfig.id) + await generateToken(oauthConfig._id) }) ).rejects.toThrow( "Error fetching oauth2 token: Invalid client or Invalid client credentials" @@ -107,7 +107,7 @@ describe("oauth2 utils", () => { method, }) - await generateToken(oauthConfig.id) + await generateToken(oauthConfig._id) }) ).rejects.toThrow( "Error fetching oauth2 token: Invalid client or Invalid client credentials" From 9b0660ee27f87f91416d7368e72fb4132ce27bcd Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 20 Mar 2025 17:35:53 +0100 Subject: [PATCH 07/11] Fix rest test --- packages/server/src/integrations/tests/rest.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/integrations/tests/rest.spec.ts b/packages/server/src/integrations/tests/rest.spec.ts index a266ce8420..95f4179bc1 100644 --- a/packages/server/src/integrations/tests/rest.spec.ts +++ b/packages/server/src/integrations/tests/rest.spec.ts @@ -307,7 +307,7 @@ describe("REST Integration", () => { config.appId, async () => await integration.read({ - authConfigId: oauthConfig.id, + authConfigId: oauthConfig._id, authConfigType: RestAuthType.OAUTH2, }) ) @@ -349,7 +349,7 @@ describe("REST Integration", () => { config.appId, async () => await integration.read({ - authConfigId: oauthConfig.id, + authConfigId: oauthConfig._id, authConfigType: RestAuthType.OAUTH2, }) ) From a40c51eededd65c4ce981fb4f3a076bf3eb0adef Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 20 Mar 2025 17:47:27 +0100 Subject: [PATCH 08/11] Fix frontend usage --- .../oauth2/OAuth2ConfigModalContent.svelte | 15 +++++++---- packages/builder/src/stores/builder/oauth2.ts | 17 ++++++++----- packages/builder/src/types/oauth2.ts | 7 +----- packages/frontend-core/src/api/oauth2.ts | 25 ++++++++++--------- packages/server/src/api/routes/oauth2.ts | 2 +- 5 files changed, 36 insertions(+), 30 deletions(-) diff --git a/packages/builder/src/pages/builder/app/[application]/settings/oauth2/OAuth2ConfigModalContent.svelte b/packages/builder/src/pages/builder/app/[application]/settings/oauth2/OAuth2ConfigModalContent.svelte index d1ca44c018..21df65be12 100644 --- a/packages/builder/src/pages/builder/app/[application]/settings/oauth2/OAuth2ConfigModalContent.svelte +++ b/packages/builder/src/pages/builder/app/[application]/settings/oauth2/OAuth2ConfigModalContent.svelte @@ -1,6 +1,6 @@