From 5a46e16b8d3f75b9d9de74e43a9865b1cbf86375 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 24 Oct 2024 16:54:08 +0100 Subject: [PATCH 1/5] Adding some tests around the openAPI public APIs to make sure the security works the way we expect, do not redirect API requests. --- packages/backend-core/src/context/identity.ts | 2 +- .../src/middleware/authenticated.ts | 50 +++++--- .../src/api/routes/public/tests/Request.ts | 110 ++++++++++++++++++ .../{metrics.spec.js => metrics.spec.ts} | 2 +- .../api/routes/public/tests/security.spec.ts | 74 ++++++++++++ .../src/api/routes/public/tests/utils.ts | 27 +++-- packages/server/src/middleware/currentapp.ts | 4 +- packages/server/src/middleware/utils.ts | 13 ++- packages/types/package.json | 3 +- packages/types/src/sdk/koa.ts | 8 ++ 10 files changed, 258 insertions(+), 35 deletions(-) create mode 100644 packages/server/src/api/routes/public/tests/Request.ts rename packages/server/src/api/routes/public/tests/{metrics.spec.js => metrics.spec.ts} (94%) create mode 100644 packages/server/src/api/routes/public/tests/security.spec.ts diff --git a/packages/backend-core/src/context/identity.ts b/packages/backend-core/src/context/identity.ts index 84de3b68c9..41f4b1eb66 100644 --- a/packages/backend-core/src/context/identity.ts +++ b/packages/backend-core/src/context/identity.ts @@ -27,7 +27,7 @@ export function doInUserContext(user: User, ctx: Ctx, task: any) { hostInfo: { ipAddress: ctx.request.ip, // filled in by koa-useragent package - userAgent: ctx.userAgent._agent.source, + userAgent: ctx.userAgent.source, }, } return doInIdentityContext(userContext, task) diff --git a/packages/backend-core/src/middleware/authenticated.ts b/packages/backend-core/src/middleware/authenticated.ts index 85aa2293ef..b713f509e0 100644 --- a/packages/backend-core/src/middleware/authenticated.ts +++ b/packages/backend-core/src/middleware/authenticated.ts @@ -1,20 +1,26 @@ import { Cookie, Header } from "../constants" import { - getCookie, clearCookie, - openJwt, + getCookie, isValidInternalAPIKey, + openJwt, } from "../utils" import { getUser } from "../cache/user" import { getSession, updateSessionTTL } from "../security/sessions" import { buildMatcherRegex, matches } from "./matchers" -import { SEPARATOR, queryGlobalView, ViewName } from "../db" -import { getGlobalDB, doInTenant } from "../context" +import { queryGlobalView, SEPARATOR, ViewName } from "../db" +import { doInTenant, getGlobalDB } from "../context" import { decrypt } from "../security/encryption" import * as identity from "../context/identity" import env from "../environment" -import { Ctx, EndpointMatcher, SessionCookie, User } from "@budibase/types" -import { InvalidAPIKeyError, ErrorCode } from "../errors" +import { + Ctx, + EndpointMatcher, + LoginMethod, + SessionCookie, + User, +} from "@budibase/types" +import { ErrorCode, InvalidAPIKeyError } from "../errors" import tracer from "dd-trace" const ONE_MINUTE = env.SESSION_UPDATE_PERIOD @@ -26,16 +32,18 @@ interface FinaliseOpts { internal?: boolean publicEndpoint?: boolean version?: string - user?: any + user?: User | { tenantId: string } + loginMethod?: LoginMethod } function timeMinusOneMinute() { return new Date(Date.now() - ONE_MINUTE).toISOString() } -function finalise(ctx: any, opts: FinaliseOpts = {}) { +function finalise(ctx: Ctx, opts: FinaliseOpts = {}) { ctx.publicEndpoint = opts.publicEndpoint || false ctx.isAuthenticated = opts.authenticated || false + ctx.loginMethod = opts.loginMethod ctx.user = opts.user ctx.internal = opts.internal || false ctx.version = opts.version @@ -120,9 +128,10 @@ export default function ( } const tenantId = ctx.request.headers[Header.TENANT_ID] - let authenticated = false, - user = null, - internal = false + let authenticated: boolean = false, + user: User | { tenantId: string } | undefined = undefined, + internal: boolean = false, + loginMethod: LoginMethod | undefined = undefined if (authCookie && !apiKey) { const sessionId = authCookie.sessionId const userId = authCookie.userId @@ -146,6 +155,7 @@ export default function ( } // @ts-ignore user.csrfToken = session.csrfToken + loginMethod = LoginMethod.COOKIE if (session?.lastAccessedAt < timeMinusOneMinute()) { // make sure we denote that the session is still in use @@ -170,17 +180,16 @@ export default function ( apiKey, populateUser ) - if (valid && foundUser) { + if (valid) { authenticated = true + loginMethod = LoginMethod.API_KEY user = foundUser - } else if (valid) { - authenticated = true - internal = true + internal = !foundUser } } if (!user && tenantId) { user = { tenantId } - } else if (user) { + } else if (user && "password" in user) { delete user.password } // be explicit @@ -204,7 +213,14 @@ export default function ( } // isAuthenticated is a function, so use a variable to be able to check authed state - finalise(ctx, { authenticated, user, internal, version, publicEndpoint }) + finalise(ctx, { + authenticated, + user, + internal, + version, + publicEndpoint, + loginMethod, + }) if (isUser(user)) { return identity.doInUserContext(user, ctx, next) diff --git a/packages/server/src/api/routes/public/tests/Request.ts b/packages/server/src/api/routes/public/tests/Request.ts new file mode 100644 index 0000000000..70e83d711d --- /dev/null +++ b/packages/server/src/api/routes/public/tests/Request.ts @@ -0,0 +1,110 @@ +import { User, Table, SearchFilters, Row } from "@budibase/types" +import { HttpMethod, MakeRequestResponse, generateMakeRequest } from "./utils" +import TestConfiguration from "../../../../tests/utilities/TestConfiguration" +import { Expectations } from "../../../../tests/utilities/api/base" + +type RequestOpts = { internal?: boolean; appId?: string } + +type PublicAPIExpectations = Omit + +export class PublicAPIRequest { + private makeRequest: MakeRequestResponse | undefined + private appId: string | undefined + private _tables: PublicTableAPI | undefined + private _rows: PublicRowAPI | undefined + private _apiKey: string | undefined + + async init(config: TestConfiguration, user: User, opts?: RequestOpts) { + this._apiKey = await config.generateApiKey(user._id) + this.makeRequest = generateMakeRequest(this.apiKey, opts) + this.appId = opts?.appId + this._tables = new PublicTableAPI(this) + this._rows = new PublicRowAPI(this) + return this + } + + opts(opts: RequestOpts) { + if (opts.appId) { + this.appId = opts.appId + } + this.makeRequest = generateMakeRequest(this.apiKey, opts) + } + + async send( + method: HttpMethod, + endpoint: string, + body?: any, + expectations?: PublicAPIExpectations + ) { + if (!this.makeRequest) { + throw new Error("Init has not been called") + } + const res = await this.makeRequest(method, endpoint, body, this.appId) + if (expectations?.status) { + expect(res.status).toEqual(expectations.status) + } + if (expectations?.body) { + expect(res.body).toEqual(expectations?.body) + } + return res.body + } + + get apiKey(): string { + if (!this._apiKey) { + throw new Error("Init has not been called") + } + return this._apiKey + } + + get tables(): PublicTableAPI { + if (!this._tables) { + throw new Error("Init has not been called") + } + return this._tables + } + + get rows(): PublicRowAPI { + if (!this._rows) { + throw new Error("Init has not been called") + } + return this._rows + } +} + +export class PublicTableAPI { + request: PublicAPIRequest + + constructor(request: PublicAPIRequest) { + this.request = request + } + + async create( + table: Table, + expectations?: PublicAPIExpectations + ): Promise<{ data: Table }> { + return this.request.send("post", "/tables", table, expectations) + } +} + +export class PublicRowAPI { + request: PublicAPIRequest + + constructor(request: PublicAPIRequest) { + this.request = request + } + + async search( + tableId: string, + query: SearchFilters, + expectations?: PublicAPIExpectations + ): Promise<{ data: Row[] }> { + return this.request.send( + "post", + `/tables/${tableId}/rows/search`, + { + query, + }, + expectations + ) + } +} diff --git a/packages/server/src/api/routes/public/tests/metrics.spec.js b/packages/server/src/api/routes/public/tests/metrics.spec.ts similarity index 94% rename from packages/server/src/api/routes/public/tests/metrics.spec.js rename to packages/server/src/api/routes/public/tests/metrics.spec.ts index 2fb5e91000..84d47dfa1d 100644 --- a/packages/server/src/api/routes/public/tests/metrics.spec.js +++ b/packages/server/src/api/routes/public/tests/metrics.spec.ts @@ -1,4 +1,4 @@ -const setup = require("../../tests/utilities") +import * as setup from "../../tests/utilities" describe("/metrics", () => { let request = setup.getRequest() diff --git a/packages/server/src/api/routes/public/tests/security.spec.ts b/packages/server/src/api/routes/public/tests/security.spec.ts new file mode 100644 index 0000000000..971cb086eb --- /dev/null +++ b/packages/server/src/api/routes/public/tests/security.spec.ts @@ -0,0 +1,74 @@ +import * as setup from "../../tests/utilities" +import { roles } from "@budibase/backend-core" +import { basicTable } from "../../../../tests/utilities/structures" +import { Table, User } from "@budibase/types" +import { PublicAPIRequest } from "./Request" + +const BROWSER_USER_AGENT = + "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/128.0.0.0 Safari/537.36" + +describe("check public API security", () => { + const config = setup.getConfig() + let builderRequest: PublicAPIRequest, + appUserRequest: PublicAPIRequest, + table: Table, + appUser: User + + beforeAll(async () => { + await config.init() + const builderUser = await config.globalUser() + appUser = await config.globalUser({ + builder: { global: false }, + roles: { + [config.getProdAppId()]: roles.BUILTIN_ROLE_IDS.BASIC, + }, + }) + builderRequest = await new PublicAPIRequest().init(config, builderUser) + appUserRequest = await new PublicAPIRequest().init(config, appUser) + table = (await builderRequest.tables.create(basicTable())).data + }) + + it("should allow with builder API key", async () => { + const res = await builderRequest.rows.search( + table._id!, + {}, + { + status: 200, + } + ) + expect(res.data.length).toEqual(0) + }) + + it("should 403 when from browser, but API key", async () => { + await appUserRequest.rows.search( + table._id!, + {}, + { + status: 403, + } + ) + }) + + it("should re-direct when using cookie", async () => { + const headers = await config.login({ + userId: appUser._id!, + builder: false, + prodApp: false, + }) + await config.withHeaders( + { + ...headers, + "User-Agent": BROWSER_USER_AGENT, + }, + async () => { + await config.api.row.search( + table._id!, + { query: {} }, + { + status: 302, + } + ) + } + ) + }) +}) diff --git a/packages/server/src/api/routes/public/tests/utils.ts b/packages/server/src/api/routes/public/tests/utils.ts index 1b57682af9..4fb048a540 100644 --- a/packages/server/src/api/routes/public/tests/utils.ts +++ b/packages/server/src/api/routes/public/tests/utils.ts @@ -21,17 +21,19 @@ export type MakeRequestWithFormDataResponse = ( function base( apiKey: string, endpoint: string, - intAppId: string | null, - isInternal: boolean + opts?: { + intAppId?: string + internal?: boolean + } ) { const extraHeaders: any = { "x-budibase-api-key": apiKey, } - if (intAppId) { - extraHeaders["x-budibase-app-id"] = intAppId + if (opts?.intAppId) { + extraHeaders["x-budibase-app-id"] = opts.intAppId } - const url = isInternal + const url = opts?.internal ? endpoint : checkSlashesInUrl(`/api/public/v1/${endpoint}`) return { headers: extraHeaders, url } @@ -39,7 +41,7 @@ function base( export function generateMakeRequest( apiKey: string, - isInternal = false + opts?: { internal?: boolean } ): MakeRequestResponse { const request = setup.getRequest()! const config = setup.getConfig()! @@ -47,9 +49,12 @@ export function generateMakeRequest( method: HttpMethod, endpoint: string, body?: any, - intAppId: string | null = config.getAppId() + intAppId: string | undefined = config.getAppId() ) => { - const { headers, url } = base(apiKey, endpoint, intAppId, isInternal) + const { headers, url } = base(apiKey, endpoint, { ...opts, intAppId }) + if (body && typeof body !== "string") { + headers["Content-Type"] = "application/json" + } const req = request[method](url).set(config.defaultHeaders(headers)) if (body) { req.send(body) @@ -62,7 +67,7 @@ export function generateMakeRequest( export function generateMakeRequestWithFormData( apiKey: string, - isInternal = false + opts?: { internal?: boolean; browser?: boolean } ): MakeRequestWithFormDataResponse { const request = setup.getRequest()! const config = setup.getConfig()! @@ -70,9 +75,9 @@ export function generateMakeRequestWithFormData( method: HttpMethod, endpoint: string, fields: Record, - intAppId: string | null = config.getAppId() + intAppId: string | undefined = config.getAppId() ) => { - const { headers, url } = base(apiKey, endpoint, intAppId, isInternal) + const { headers, url } = base(apiKey, endpoint, { ...opts, intAppId }) const req = request[method](url).set(config.defaultHeaders(headers)) for (let [field, value] of Object.entries(fields)) { if (typeof value === "string") { diff --git a/packages/server/src/middleware/currentapp.ts b/packages/server/src/middleware/currentapp.ts index d616377298..d5840712a0 100644 --- a/packages/server/src/middleware/currentapp.ts +++ b/packages/server/src/middleware/currentapp.ts @@ -10,7 +10,7 @@ import { import { generateUserMetadataID, isDevAppID } from "../db/utils" import { getCachedSelf } from "../utilities/global" import env from "../environment" -import { isWebhookEndpoint } from "./utils" +import { isWebhookEndpoint, isBrowser, isApiKey } from "./utils" import { UserCtx, ContextUser } from "@budibase/types" import tracer from "dd-trace" @@ -27,7 +27,7 @@ export default async (ctx: UserCtx, next: any) => { } // deny access to application preview - if (!env.isTest()) { + if (isBrowser(ctx) && !isApiKey(ctx)) { if ( isDevAppID(requestAppId) && !isWebhookEndpoint(ctx) && diff --git a/packages/server/src/middleware/utils.ts b/packages/server/src/middleware/utils.ts index 714df12f38..650260b136 100644 --- a/packages/server/src/middleware/utils.ts +++ b/packages/server/src/middleware/utils.ts @@ -1,9 +1,18 @@ -import { BBContext } from "@budibase/types" +import { LoginMethod, UserCtx } from "@budibase/types" const WEBHOOK_ENDPOINTS = new RegExp( ["webhooks/trigger", "webhooks/schema"].join("|") ) -export function isWebhookEndpoint(ctx: BBContext) { +export function isWebhookEndpoint(ctx: UserCtx) { return WEBHOOK_ENDPOINTS.test(ctx.request.url) } + +export function isBrowser(ctx: UserCtx) { + const browser = ctx.userAgent.browser + return browser !== "unknown" +} + +export function isApiKey(ctx: UserCtx) { + return ctx.loginMethod === LoginMethod.API_KEY +} diff --git a/packages/types/package.json b/packages/types/package.json index bce25c7086..d132383081 100644 --- a/packages/types/package.json +++ b/packages/types/package.json @@ -19,7 +19,8 @@ "@types/koa": "2.13.4", "@types/redlock": "4.0.7", "rimraf": "3.0.2", - "typescript": "5.5.2" + "typescript": "5.5.2", + "koa-useragent": "^4.1.0" }, "dependencies": { "scim-patch": "^0.8.1" diff --git a/packages/types/src/sdk/koa.ts b/packages/types/src/sdk/koa.ts index a7df701171..38622b974a 100644 --- a/packages/types/src/sdk/koa.ts +++ b/packages/types/src/sdk/koa.ts @@ -2,6 +2,12 @@ import { Context, Request } from "koa" import { User, Role, UserRoles, Account, ConfigType } from "../documents" import { FeatureFlag, License } from "../sdk" import { Files } from "formidable" +import { UserAgentContext } from "koa-useragent" + +export enum LoginMethod { + API_KEY = "api_key", + COOKIE = "cookie", +} export interface ContextUser extends Omit { globalId?: string @@ -31,6 +37,7 @@ export interface BBRequest extends Request { export interface Ctx extends Context { request: BBRequest body: ResponseBody + userAgent: UserAgentContext["userAgent"] } /** @@ -40,6 +47,7 @@ export interface UserCtx extends Ctx { user: ContextUser roleId?: string + loginMethod?: LoginMethod } /** From 0863a1167cba60bfd962840e0f0da22eaa1d3173 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 24 Oct 2024 17:41:34 +0100 Subject: [PATCH 2/5] Updating OpenAPI definition to contain all required variables. --- packages/server/specs/generate.ts | 18 +++++-------- packages/server/specs/openapi.json | 30 ++++++++++++---------- packages/server/specs/openapi.yaml | 22 +++++++++------- packages/server/src/definitions/openapi.ts | 21 ++++++++++----- 4 files changed, 51 insertions(+), 40 deletions(-) diff --git a/packages/server/specs/generate.ts b/packages/server/specs/generate.ts index eea00f83aa..8f6376195f 100644 --- a/packages/server/specs/generate.ts +++ b/packages/server/specs/generate.ts @@ -20,19 +20,15 @@ const options = { { url: "https://budibase.app/api/public/v1", description: "Budibase Cloud API", - }, - { - url: "{protocol}://{hostname}/api/public/v1", - description: "Budibase self hosted API", variables: { - protocol: { - default: "http", - description: - "Whether HTTP or HTTPS should be used to communicate with your Budibase instance.", + apiKey: { + default: "", + description: "The API key of the user to assume for API call.", }, - hostname: { - default: "localhost:10000", - description: "The URL of your Budibase instance.", + appId: { + default: "", + description: + "The ID of the app the calls will be executed within the context of, this should start with app_ (production) or app_dev (development).", }, }, }, diff --git a/packages/server/specs/openapi.json b/packages/server/specs/openapi.json index b21554505b..c96296a8bb 100644 --- a/packages/server/specs/openapi.json +++ b/packages/server/specs/openapi.json @@ -8,19 +8,15 @@ "servers": [ { "url": "https://budibase.app/api/public/v1", - "description": "Budibase Cloud API" - }, - { - "url": "{protocol}://{hostname}/api/public/v1", - "description": "Budibase self hosted API", + "description": "Budibase Cloud API", "variables": { - "protocol": { - "default": "http", - "description": "Whether HTTP or HTTPS should be used to communicate with your Budibase instance." + "apiKey": { + "default": "", + "description": "The API key of the user to assume for API call." }, - "hostname": { - "default": "localhost:10000", - "description": "The URL of your Budibase instance." + "appId": { + "default": "", + "description": "The ID of the app the calls will be executed within the context of, this should start with app_ (production) or app_dev (development)." } } } @@ -833,7 +829,8 @@ "type": "string", "enum": [ "static", - "dynamic" + "dynamic", + "ai" ], "description": "Defines whether this is a static or dynamic formula." } @@ -857,6 +854,7 @@ "link", "formula", "auto", + "ai", "json", "internal", "barcodeqr", @@ -1042,7 +1040,8 @@ "type": "string", "enum": [ "static", - "dynamic" + "dynamic", + "ai" ], "description": "Defines whether this is a static or dynamic formula." } @@ -1066,6 +1065,7 @@ "link", "formula", "auto", + "ai", "json", "internal", "barcodeqr", @@ -1262,7 +1262,8 @@ "type": "string", "enum": [ "static", - "dynamic" + "dynamic", + "ai" ], "description": "Defines whether this is a static or dynamic formula." } @@ -1286,6 +1287,7 @@ "link", "formula", "auto", + "ai", "json", "internal", "barcodeqr", diff --git a/packages/server/specs/openapi.yaml b/packages/server/specs/openapi.yaml index 6a2ae89c61..9ac7b7533a 100644 --- a/packages/server/specs/openapi.yaml +++ b/packages/server/specs/openapi.yaml @@ -6,16 +6,14 @@ info: servers: - url: https://budibase.app/api/public/v1 description: Budibase Cloud API - - url: "{protocol}://{hostname}/api/public/v1" - description: Budibase self hosted API variables: - protocol: - default: http - description: Whether HTTP or HTTPS should be used to communicate with your - Budibase instance. - hostname: - default: localhost:10000 - description: The URL of your Budibase instance. + apiKey: + default: + description: The API key of the user to assume for API call. + appId: + default: + description: The ID of the app the calls will be executed within the context of, + this should start with app_ (production) or app_dev (development). components: parameters: tableId: @@ -761,6 +759,7 @@ components: enum: - static - dynamic + - ai description: Defines whether this is a static or dynamic formula. - type: object properties: @@ -779,6 +778,7 @@ components: - link - formula - auto + - ai - json - internal - barcodeqr @@ -929,6 +929,7 @@ components: enum: - static - dynamic + - ai description: Defines whether this is a static or dynamic formula. - type: object properties: @@ -947,6 +948,7 @@ components: - link - formula - auto + - ai - json - internal - barcodeqr @@ -1104,6 +1106,7 @@ components: enum: - static - dynamic + - ai description: Defines whether this is a static or dynamic formula. - type: object properties: @@ -1122,6 +1125,7 @@ components: - link - formula - auto + - ai - json - internal - barcodeqr diff --git a/packages/server/src/definitions/openapi.ts b/packages/server/src/definitions/openapi.ts index cc3c8a7d4d..3abf7e6fa7 100644 --- a/packages/server/src/definitions/openapi.ts +++ b/packages/server/src/definitions/openapi.ts @@ -257,7 +257,7 @@ export interface components { * @description Defines whether this is a static or dynamic formula. * @enum {string} */ - formulaType?: "static" | "dynamic"; + formulaType?: "static" | "dynamic" | "ai"; } | { /** @@ -277,11 +277,14 @@ export interface components { | "link" | "formula" | "auto" + | "ai" | "json" | "internal" | "barcodeqr" + | "signature_single" | "bigint" - | "bb_reference"; + | "bb_reference" + | "bb_reference_single"; /** @description A constraint can be applied to the column which will be validated against when a row is saved. */ constraints?: { /** @enum {string} */ @@ -366,7 +369,7 @@ export interface components { * @description Defines whether this is a static or dynamic formula. * @enum {string} */ - formulaType?: "static" | "dynamic"; + formulaType?: "static" | "dynamic" | "ai"; } | { /** @@ -386,11 +389,14 @@ export interface components { | "link" | "formula" | "auto" + | "ai" | "json" | "internal" | "barcodeqr" + | "signature_single" | "bigint" - | "bb_reference"; + | "bb_reference" + | "bb_reference_single"; /** @description A constraint can be applied to the column which will be validated against when a row is saved. */ constraints?: { /** @enum {string} */ @@ -477,7 +483,7 @@ export interface components { * @description Defines whether this is a static or dynamic formula. * @enum {string} */ - formulaType?: "static" | "dynamic"; + formulaType?: "static" | "dynamic" | "ai"; } | { /** @@ -497,11 +503,14 @@ export interface components { | "link" | "formula" | "auto" + | "ai" | "json" | "internal" | "barcodeqr" + | "signature_single" | "bigint" - | "bb_reference"; + | "bb_reference" + | "bb_reference_single"; /** @description A constraint can be applied to the column which will be validated against when a row is saved. */ constraints?: { /** @enum {string} */ From 68354cc50f80576b6ae94a8dc8fb72336d791816 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 24 Oct 2024 17:48:25 +0100 Subject: [PATCH 3/5] Defaulting app ID to variable. --- packages/server/specs/openapi.json | 2 ++ packages/server/specs/openapi.yaml | 2 ++ packages/server/specs/parameters.ts | 2 ++ 3 files changed, 6 insertions(+) diff --git a/packages/server/specs/openapi.json b/packages/server/specs/openapi.json index c96296a8bb..f3091a1fc7 100644 --- a/packages/server/specs/openapi.json +++ b/packages/server/specs/openapi.json @@ -47,6 +47,7 @@ "required": true, "description": "The ID of the app which this request is targeting.", "schema": { + "default": "{{ appId }}", "type": "string" } }, @@ -56,6 +57,7 @@ "required": true, "description": "The ID of the app which this request is targeting.", "schema": { + "default": "{{ appId }}", "type": "string" } }, diff --git a/packages/server/specs/openapi.yaml b/packages/server/specs/openapi.yaml index 9ac7b7533a..1e9b9921cf 100644 --- a/packages/server/specs/openapi.yaml +++ b/packages/server/specs/openapi.yaml @@ -36,6 +36,7 @@ components: required: true description: The ID of the app which this request is targeting. schema: + default: "{{ appId }}" type: string appIdUrl: in: path @@ -43,6 +44,7 @@ components: required: true description: The ID of the app which this request is targeting. schema: + default: "{{ appId }}" type: string queryId: in: path diff --git a/packages/server/specs/parameters.ts b/packages/server/specs/parameters.ts index a928026bb1..a4f2b27ae4 100644 --- a/packages/server/specs/parameters.ts +++ b/packages/server/specs/parameters.ts @@ -24,6 +24,7 @@ export const appId = { required: true, description: "The ID of the app which this request is targeting.", schema: { + default: "{{ appId }}", type: "string", }, } @@ -34,6 +35,7 @@ export const appIdUrl = { required: true, description: "The ID of the app which this request is targeting.", schema: { + default: "{{ appId }}", type: "string", }, } From c33f3319041a6662364c51bae297ba9d907f474c Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 24 Oct 2024 18:08:49 +0100 Subject: [PATCH 4/5] Test fix. --- packages/server/src/middleware/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/middleware/utils.ts b/packages/server/src/middleware/utils.ts index 650260b136..8e7c623e2a 100644 --- a/packages/server/src/middleware/utils.ts +++ b/packages/server/src/middleware/utils.ts @@ -9,8 +9,8 @@ export function isWebhookEndpoint(ctx: UserCtx) { } export function isBrowser(ctx: UserCtx) { - const browser = ctx.userAgent.browser - return browser !== "unknown" + const browser = ctx.userAgent?.browser + return browser && browser !== "unknown" } export function isApiKey(ctx: UserCtx) { From f1fa0a3a6f2963accc49c28442895c8a004711e1 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 25 Oct 2024 10:41:20 +0100 Subject: [PATCH 5/5] Fixing tests, updating to typescript. --- .../api/routes/public/tests/security.spec.ts | 5 +- .../{routing.spec.js => routing.spec.ts} | 61 ++++++++++++------- ...{currentapp.spec.js => currentapp.spec.ts} | 16 +++-- .../src/tests/utilities/TestConfiguration.ts | 5 ++ 4 files changed, 55 insertions(+), 32 deletions(-) rename packages/server/src/api/routes/tests/{routing.spec.js => routing.spec.ts} (72%) rename packages/server/src/middleware/tests/{currentapp.spec.js => currentapp.spec.ts} (94%) diff --git a/packages/server/src/api/routes/public/tests/security.spec.ts b/packages/server/src/api/routes/public/tests/security.spec.ts index 971cb086eb..a285bc7736 100644 --- a/packages/server/src/api/routes/public/tests/security.spec.ts +++ b/packages/server/src/api/routes/public/tests/security.spec.ts @@ -4,9 +4,6 @@ import { basicTable } from "../../../../tests/utilities/structures" import { Table, User } from "@budibase/types" import { PublicAPIRequest } from "./Request" -const BROWSER_USER_AGENT = - "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/128.0.0.0 Safari/537.36" - describe("check public API security", () => { const config = setup.getConfig() let builderRequest: PublicAPIRequest, @@ -58,7 +55,7 @@ describe("check public API security", () => { await config.withHeaders( { ...headers, - "User-Agent": BROWSER_USER_AGENT, + "User-Agent": config.browserUserAgent(), }, async () => { await config.api.row.search( diff --git a/packages/server/src/api/routes/tests/routing.spec.js b/packages/server/src/api/routes/tests/routing.spec.ts similarity index 72% rename from packages/server/src/api/routes/tests/routing.spec.js rename to packages/server/src/api/routes/tests/routing.spec.ts index 5bdd4ee852..0d4ffc7c3e 100644 --- a/packages/server/src/api/routes/tests/routing.spec.js +++ b/packages/server/src/api/routes/tests/routing.spec.ts @@ -1,9 +1,10 @@ -const setup = require("./utilities") -const { basicScreen, powerScreen } = setup.structures -const { checkBuilderEndpoint, runInProd } = require("./utilities/TestFunctions") -const { roles } = require("@budibase/backend-core") -const { BUILTIN_ROLE_IDS } = roles +import * as setup from "./utilities" +import { checkBuilderEndpoint, runInProd } from "./utilities/TestFunctions" +import { roles } from "@budibase/backend-core" +import { Screen } from "@budibase/types" +const { BUILTIN_ROLE_IDS } = roles +const { basicScreen, powerScreen } = setup.structures const route = "/test" // there are checks which are disabled in test env, @@ -12,7 +13,7 @@ const route = "/test" describe("/routing", () => { let request = setup.getRequest() let config = setup.getConfig() - let basic, power + let basic: Screen, power: Screen afterAll(setup.afterAll) @@ -25,26 +26,40 @@ describe("/routing", () => { describe("fetch", () => { it("prevents a public user from accessing development app", async () => { - await runInProd(() => { - return request - .get(`/api/routing/client`) - .set(config.publicHeaders({ prodApp: false })) - .expect(302) - }) + await config.withHeaders( + { + "User-Agent": config.browserUserAgent(), + }, + async () => { + await runInProd(() => { + return request + .get(`/api/routing/client`) + .set(config.publicHeaders({ prodApp: false })) + .expect(302) + }) + } + ) }) it("prevents a non builder from accessing development app", async () => { - await runInProd(async () => { - return request - .get(`/api/routing/client`) - .set( - await config.roleHeaders({ - roleId: BUILTIN_ROLE_IDS.BASIC, - prodApp: false, - }) - ) - .expect(302) - }) + await config.withHeaders( + { + "User-Agent": config.browserUserAgent(), + }, + async () => { + await runInProd(async () => { + return request + .get(`/api/routing/client`) + .set( + await config.roleHeaders({ + roleId: BUILTIN_ROLE_IDS.BASIC, + prodApp: false, + }) + ) + .expect(302) + }) + } + ) }) it("returns the correct routing for basic user", async () => { const res = await request diff --git a/packages/server/src/middleware/tests/currentapp.spec.js b/packages/server/src/middleware/tests/currentapp.spec.ts similarity index 94% rename from packages/server/src/middleware/tests/currentapp.spec.js rename to packages/server/src/middleware/tests/currentapp.spec.ts index 22e47b0a6e..202d9f96d5 100644 --- a/packages/server/src/middleware/tests/currentapp.spec.js +++ b/packages/server/src/middleware/tests/currentapp.spec.ts @@ -1,4 +1,6 @@ -require("../../db").init() +import * as db from "../../db" + +db.init() mockAuthWithNoCookie() mockWorker() mockUserGroups() @@ -45,7 +47,7 @@ function mockAuthWithNoCookie() { }, cache: { user: { - getUser: async id => { + getUser: async () => { return { _id: "us_uuid1", } @@ -82,7 +84,7 @@ function mockAuthWithCookie() { }, cache: { user: { - getUser: async id => { + getUser: async () => { return { _id: "us_uuid1", } @@ -94,6 +96,10 @@ function mockAuthWithCookie() { } class TestConfiguration { + next: jest.MockedFunction + throw: jest.MockedFunction + ctx: any + constructor() { this.next = jest.fn() this.throw = jest.fn() @@ -130,7 +136,7 @@ class TestConfiguration { } describe("Current app middleware", () => { - let config + let config: TestConfiguration beforeEach(() => { config = new TestConfiguration() @@ -192,7 +198,7 @@ describe("Current app middleware", () => { }, cache: { user: { - getUser: async id => { + getUser: async () => { return { _id: "us_uuid1", } diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 713f8b31de..5ed60a59b6 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -423,6 +423,7 @@ export default class TestConfiguration { Accept: "application/json", Cookie: [`${constants.Cookie.Auth}=${authToken}`], [constants.Header.APP_ID]: appId, + ...this.temporaryHeaders, } }) } @@ -527,6 +528,10 @@ export default class TestConfiguration { return this.login({ userId: email, roleId, builder, prodApp }) } + browserUserAgent() { + return "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/128.0.0.0 Safari/537.36" + } + // TENANCY tenantHost() {