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

@@ -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)

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)

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<Expectations, "headers" | "headersNotPresent">
+
+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
+    )
+  }
+}

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()

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,
+          }
+        )
+      }
+    )
+  })
+})

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<string, string | { path: string }>,
-    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") {

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) &&

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
+}

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"

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<User, "roles"> {
   globalId?: string
@@ -31,6 +37,7 @@ export interface BBRequest<RequestBody> extends Request {
 export interface Ctx<RequestBody = any, ResponseBody = any> extends Context {
   request: BBRequest<RequestBody>
   body: ResponseBody
+  userAgent: UserAgentContext["userAgent"]
 }
 
 /**
@@ -40,6 +47,7 @@ export interface UserCtx<RequestBody = any, ResponseBody = any>
   extends Ctx<RequestBody, ResponseBody> {
   user: ContextUser
   roleId?: string
+  loginMethod?: LoginMethod
 }
 
 /**