Merge pull request #14744 from Budibase/logged-out-search-fix

Still fetch flags when the user is not logged in.

packages/backend-core/src/context/mainContext.ts

@@ -253,6 +253,11 @@ export function getAppId(): string | undefined {
   }
 }
 
+export function getIP(): string | undefined {
+  const context = Context.get()
+  return context?.ip
+}
+
 export const getProdAppId = () => {
   const appId = getAppId()
   if (!appId) {
@@ -281,6 +286,10 @@ export function doInScimContext(task: any) {
   return newContext(updates, task)
 }
 
+export function doInIPContext(ip: string, task: any) {
+  return newContext({ ip }, task)
+}
+
 export async function ensureSnippetContext(enabled = !env.isTest()) {
   const ctx = getCurrentContext()
 

packages/backend-core/src/context/types.ts

@@ -9,6 +9,7 @@ export type ContextMap = {
   identity?: IdentityContext
   environmentVariables?: Record<string, string>
   isScim?: boolean
+  ip?: string
   automationId?: string
   isMigrating?: boolean
   vm?: VM

packages/backend-core/src/features/features.ts

@@ -1,7 +1,8 @@
 import env from "../environment"
+import * as crypto from "crypto"
 import * as context from "../context"
 import { PostHog, PostHogOptions } from "posthog-node"
-import { FeatureFlag, IdentityType, UserCtx } from "@budibase/types"
+import { FeatureFlag } from "@budibase/types"
 import tracer from "dd-trace"
 import { Duration } from "../utils"
 
@@ -141,23 +142,17 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
     return this.flagSchema[name as keyof T] !== undefined
   }
 
-  async get<K extends keyof T>(
+  async get<K extends keyof T>(key: K): Promise<FlagValues<T>[K]> {
-    key: K,
+    const flags = await this.fetch()
-    ctx?: UserCtx
-  ): Promise<FlagValues<T>[K]> {
-    const flags = await this.fetch(ctx)
     return flags[key]
   }
 
-  async isEnabled<K extends KeysOfType<T, boolean>>(
+  async isEnabled<K extends KeysOfType<T, boolean>>(key: K): Promise<boolean> {
-    key: K,
+    const flags = await this.fetch()
-    ctx?: UserCtx
-  ): Promise<boolean> {
-    const flags = await this.fetch(ctx)
     return flags[key]
   }
 
-  async fetch(ctx?: UserCtx): Promise<FlagValues<T>> {
+  async fetch(): Promise<FlagValues<T>> {
     return await tracer.trace("features.fetch", async span => {
       const cachedFlags = context.getFeatureFlags<FlagValues<T>>(this.setId)
       if (cachedFlags) {
@@ -198,50 +193,33 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
         tags[`flags.${key}.source`] = "environment"
       }
 
-      const license = ctx?.user?.license
-      if (license) {
-        tags[`readFromLicense`] = true
-
-        for (const feature of license.features) {
-          if (!this.isFlagName(feature)) {
-            continue
-          }
-
-          if (
-            flagValues[feature] === true ||
-            specificallySetFalse.has(feature)
-          ) {
-            // If the flag is already set to through environment variables, we
-            // don't want to override it back to false here.
-            continue
-          }
-
-          // @ts-expect-error - TS does not like you writing into a generic type,
-          // but we know that it's okay in this case because it's just an object.
-          flagValues[feature] = true
-          tags[`flags.${feature}.source`] = "license"
-        }
-      }
-
       const identity = context.getIdentity()
-      tags[`identity.type`] = identity?.type
-      tags[`identity.tenantId`] = identity?.tenantId
-      tags[`identity._id`] = identity?._id
 
-      if (posthog && identity?.type === IdentityType.USER) {
+      let userId = identity?._id
+      if (!userId) {
+        const ip = context.getIP()
+        if (ip) {
+          userId = crypto.createHash("sha512").update(ip).digest("hex")
+        }
+      }
+
+      let tenantId = identity?.tenantId
+      if (!tenantId) {
+        tenantId = currentTenantId
+      }
+
+      tags[`identity.type`] = identity?.type
+      tags[`identity._id`] = identity?._id
+      tags[`tenantId`] = tenantId
+      tags[`userId`] = userId
+
+      if (posthog && userId) {
         tags[`readFromPostHog`] = true
 
-        const personProperties: Record<string, string> = {}
+        const personProperties: Record<string, string> = { tenantId }
-        if (identity.tenantId) {
+        const posthogFlags = await posthog.getAllFlagsAndPayloads(userId, {
-          personProperties.tenantId = identity.tenantId
-        }
-
-        const posthogFlags = await posthog.getAllFlagsAndPayloads(
-          identity._id,
-          {
           personProperties,
-          }
+        })
-        )
 
         for (const [name, value] of Object.entries(posthogFlags.featureFlags)) {
           if (!this.isFlagName(name)) {

packages/backend-core/src/features/tests/features.spec.ts

@@ -1,9 +1,10 @@
-import { IdentityContext, IdentityType, UserCtx } from "@budibase/types"
+import { IdentityContext, IdentityType } from "@budibase/types"
 import { Flag, FlagSet, FlagValues, init, shutdown } from "../"
 import * as context from "../../context"
 import environment, { withEnv } from "../../environment"
 import nodeFetch from "node-fetch"
 import nock from "nock"
+import * as crypto from "crypto"
 
 const schema = {
   TEST_BOOLEAN: Flag.boolean(false),
@@ -17,7 +18,6 @@ interface TestCase {
   identity?: Partial<IdentityContext>
   environmentFlags?: string
   posthogFlags?: PostHogFlags
-  licenseFlags?: Array<string>
   expected?: Partial<FlagValues<typeof schema>>
   errorMessage?: string | RegExp
 }
@@ -27,10 +27,14 @@ interface PostHogFlags {
   featureFlagPayloads?: Record<string, string>
 }
 
-function mockPosthogFlags(flags: PostHogFlags) {
+function mockPosthogFlags(
+  flags: PostHogFlags,
+  opts?: { token?: string; distinct_id?: string }
+) {
+  const { token = "test", distinct_id = "us_1234" } = opts || {}
   nock("https://us.i.posthog.com")
     .post("/decide/?v=3", body => {
-      return body.token === "test" && body.distinct_id === "us_1234"
+      return body.token === token && body.distinct_id === distinct_id
     })
     .reply(200, flags)
     .persist()
@@ -112,17 +116,6 @@ describe("feature flags", () => {
       },
       expected: { TEST_BOOLEAN: true },
     },
-    {
-      it: "should be able to set boolean flags through the license",
-      licenseFlags: ["TEST_BOOLEAN"],
-      expected: { TEST_BOOLEAN: true },
-    },
-    {
-      it: "should not be able to override a negative environment flag from license",
-      environmentFlags: "default:!TEST_BOOLEAN",
-      licenseFlags: ["TEST_BOOLEAN"],
-      expected: { TEST_BOOLEAN: false },
-    },
     {
       it: "should not error on unrecognised PostHog flag",
       posthogFlags: {
@@ -130,18 +123,12 @@ describe("feature flags", () => {
       },
       expected: flags.defaults(),
     },
-    {
-      it: "should not error on unrecognised license flag",
-      licenseFlags: ["UNDEFINED"],
-      expected: flags.defaults(),
-    },
   ])(
     "$it",
     async ({
       identity,
       environmentFlags,
       posthogFlags,
-      licenseFlags,
       expected,
       errorMessage,
     }) => {
@@ -157,8 +144,6 @@ describe("feature flags", () => {
         env.POSTHOG_API_HOST = "https://us.i.posthog.com"
       }
 
-      const ctx = { user: { license: { features: licenseFlags || [] } } }
-
       await withEnv(env, async () => {
         // We need to pass in node-fetch here otherwise nock won't get used
         // because posthog-node uses axios under the hood.
@@ -180,18 +165,13 @@ describe("feature flags", () => {
 
         await context.doInIdentityContext(fullIdentity, async () => {
           if (errorMessage) {
-            await expect(flags.fetch(ctx as UserCtx)).rejects.toThrow(
+            await expect(flags.fetch()).rejects.toThrow(errorMessage)
-              errorMessage
-            )
           } else if (expected) {
-            const values = await flags.fetch(ctx as UserCtx)
+            const values = await flags.fetch()
             expect(values).toMatchObject(expected)
 
             for (const [key, expectedValue] of Object.entries(expected)) {
-              const value = await flags.get(
+              const value = await flags.get(key as keyof typeof schema)
-                key as keyof typeof schema,
-                ctx as UserCtx
-              )
               expect(value).toBe(expectedValue)
             }
           } else {
@@ -214,6 +194,14 @@ describe("feature flags", () => {
       lastName: "User",
     }
 
+    // We need to pass in node-fetch here otherwise nock won't get used
+    // because posthog-node uses axios under the hood.
+    init({
+      fetch: (url, opts) => {
+        return nodeFetch(url, opts)
+      },
+    })
+
     nock("https://us.i.posthog.com")
       .post("/decide/?v=3", body => {
         return body.token === "test" && body.distinct_id === "us_1234"
@@ -230,4 +218,44 @@ describe("feature flags", () => {
       }
     )
   })
+
+  it("should still get flags when user is logged out", async () => {
+    const env: Partial<typeof environment> = {
+      SELF_HOSTED: false,
+      POSTHOG_FEATURE_FLAGS_ENABLED: "true",
+      POSTHOG_API_HOST: "https://us.i.posthog.com",
+      POSTHOG_TOKEN: "test",
+    }
+
+    const ip = "127.0.0.1"
+    const hashedIp = crypto.createHash("sha512").update(ip).digest("hex")
+
+    await withEnv(env, async () => {
+      mockPosthogFlags(
+        {
+          featureFlags: { TEST_BOOLEAN: true },
+        },
+        {
+          distinct_id: hashedIp,
+        }
+      )
+
+      // We need to pass in node-fetch here otherwise nock won't get used
+      // because posthog-node uses axios under the hood.
+      init({
+        fetch: (url, opts) => {
+          return nodeFetch(url, opts)
+        },
+      })
+
+      await context.doInIPContext(ip, async () => {
+        await context.doInTenant("default", async () => {
+          const result = await flags.fetch()
+          expect(result.TEST_BOOLEAN).toBe(true)
+        })
+      })
+
+      shutdown()
+    })
+  })
 })

packages/backend-core/src/middleware/index.ts

@@ -20,3 +20,4 @@ export { default as correlation } from "../logging/correlation/middleware"
 export { default as errorHandling } from "./errorHandling"
 export { default as querystringToBody } from "./querystringToBody"
 export * as joiValidator from "./joi-validator"
+export { default as ip } from "./ip"

packages/backend-core/src/middleware/ip.ts

@@ -0,0 +1,12 @@
+import { Ctx } from "@budibase/types"
+import { doInIPContext } from "../context"
+
+export default async (ctx: Ctx, next: any) => {
+  if (ctx.ip) {
+    return await doInIPContext(ctx.ip, () => {
+      return next()
+    })
+  } else {
+    return next()
+  }
+}

packages/server/src/koa.ts

@@ -12,6 +12,7 @@ import { userAgent } from "koa-useragent"
 
 export default function createKoaApp() {
   const app = new Koa()
+  app.proxy = true
 
   let mbNumber = parseInt(env.HTTP_MB_LIMIT || "10")
   if (!mbNumber || isNaN(mbNumber)) {
@@ -35,6 +36,7 @@ export default function createKoaApp() {
 
   app.use(middleware.correlation)
   app.use(middleware.pino)
+  app.use(middleware.ip)
   app.use(userAgent)
 
   const server = http.createServer(app.callback())

packages/worker/src/index.ts

@@ -46,6 +46,7 @@ bootstrap()
 const app: Application = new Application()
 
 app.keys = ["secret", "key"]
+app.proxy = true
 
 // set up top level koa middleware
 app.use(handleScimBody)
@@ -54,6 +55,7 @@ app.use(koaBody({ multipart: true }))
 app.use(koaSession(app))
 app.use(middleware.correlation)
 app.use(middleware.pino)
+app.use(middleware.ip)
 app.use(userAgent)
 
 // authentication