Access control adjustments (#253)

* Stop using Team.plugins_opt_in * Plugins access control (#231) * Use more robust plugin debug descriptions with pluginDigest * Update plugin60 for is_global * Update query for plugins currently in force * Improve pluginDigest format * Keep respecting team opt out of plugins * Add plugins_access_level to sql org helper * Use pluginDigest * Improve specificField type * Fix mockPlugin * Adjust some more tests
PostHog · Mar 17, 2021 · dd6e88d · dd6e88d
1 parent 0fdc4d5
commit dd6e88d
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 29 deletions.
diff --git a/benchmarks/vm/memory.benchmark.ts b/benchmarks/vm/memory.benchmark.ts
@@ -31,6 +31,7 @@ const mockPlugin: Plugin = {
     tag: 'v1.0.0',
     archive: null,
     error: undefined,
+    is_global: false,
     created_at: new Date().toISOString(),
     updated_at: new Date().toISOString(),
 }

diff --git a/src/plugins/loadPlugin.ts b/src/plugins/loadPlugin.ts
@@ -3,7 +3,7 @@ import * as path from 'path'
 
 import { processError } from '../error'
 import { PluginConfig, PluginJsonConfig, PluginsServer } from '../types'
-import { getFileFromArchive } from '../utils'
+import { getFileFromArchive, pluginDigest } from '../utils'
 
 export async function loadPlugin(server: PluginsServer, pluginConfig: PluginConfig): Promise<boolean> {
     const { plugin } = pluginConfig
@@ -28,7 +28,7 @@ export async function loadPlugin(server: PluginsServer, pluginConfig: PluginConf
                     await processError(
                         server,
                         pluginConfig,
-                        `Could not load posthog config at "${configPath}" for plugin "${plugin.name}"`
+                        `Could not load posthog config at "${configPath}" for ${pluginDigest(plugin)}`
                     )
                     return false
                 }
@@ -39,7 +39,7 @@ export async function loadPlugin(server: PluginsServer, pluginConfig: PluginConf
                 await processError(
                     server,
                     pluginConfig,
-                    `No "main" config key or "index.js" file found for plugin "${plugin.name}"`
+                    `No "main" config key or "index.js" file found for ${pluginDigest(plugin)}`
                 )
                 return false
             }
@@ -51,7 +51,7 @@ export async function loadPlugin(server: PluginsServer, pluginConfig: PluginConf
                 server,
                 pluginConfig,
                 indexJs,
-                `local plugin "${plugin.name}" from "${pluginPath}"!`
+                `local ${pluginDigest(plugin)} from "${pluginPath}"!`
             )
             return true
         } else if (plugin.archive) {
@@ -63,29 +63,29 @@ export async function loadPlugin(server: PluginsServer, pluginConfig: PluginConf
                     config = JSON.parse(json)
                 } catch (error) {
                     pluginConfig.vm?.failInitialization!()
-                    await processError(server, pluginConfig, `Can not load plugin.json for plugin "${plugin.name}"`)
+                    await processError(server, pluginConfig, `Can not load plugin.json for ${pluginDigest(plugin)}`)
                     return false
                 }
             }
 
             const indexJs = await getFileFromArchive(archive, config['main'] || 'index.js')
 
             if (indexJs) {
-                void pluginConfig.vm?.initialize!(server, pluginConfig, indexJs, `plugin "${plugin.name}"!`)
+                void pluginConfig.vm?.initialize!(server, pluginConfig, indexJs, pluginDigest(plugin))
                 return true
             } else {
                 pluginConfig.vm?.failInitialization!()
-                await processError(server, pluginConfig, `Could not load index.js for plugin "${plugin.name}"!`)
+                await processError(server, pluginConfig, `Could not load index.js for ${pluginDigest(plugin)}!`)
             }
         } else if (plugin.plugin_type === 'source' && plugin.source) {
-            void pluginConfig.vm?.initialize!(server, pluginConfig, plugin.source, `plugin "${plugin.name}"!`)
+            void pluginConfig.vm?.initialize!(server, pluginConfig, plugin.source, pluginDigest(plugin))
             return true
         } else {
             pluginConfig.vm?.failInitialization!()
             await processError(
                 server,
                 pluginConfig,
-                `Un-downloaded remote plugins not supported! Plugin: "${plugin.name}"`
+                `Tried using undownloaded remote ${pluginDigest(plugin)}, which is not supported!`
             )
         }
     } catch (error) {

diff --git a/src/sql.ts b/src/sql.ts
@@ -1,35 +1,36 @@
 import { Plugin, PluginAttachmentDB, PluginConfig, PluginConfigId, PluginError, PluginsServer } from './types'
 
+function pluginConfigsInForceQuery(specificField?: keyof PluginConfig): string {
+    return `SELECT posthog_pluginconfig.${specificField || '*'}
+       FROM posthog_pluginconfig
+       LEFT JOIN posthog_team ON posthog_team.id = posthog_pluginconfig.team_id
+       LEFT JOIN posthog_organization ON posthog_organization.id = posthog_team.organization_id
+       LEFT JOIN posthog_plugin ON posthog_plugin.id = posthog_pluginconfig.plugin_id
+       WHERE (
+           (posthog_team.id IS NULL OR posthog_team.plugins_opt_in='t')
+           AND posthog_pluginconfig.enabled='t' AND posthog_organization.plugins_access_level > 0
+           AND (posthog_plugin.organization_id = posthog_organization.id OR posthog_plugin.is_global)
+       )`
+}
+
 export async function getPluginRows(server: PluginsServer): Promise<Plugin[]> {
     const { rows: pluginRows }: { rows: Plugin[] } = await server.db.postgresQuery(
-        `SELECT posthog_plugin.* FROM posthog_plugin WHERE id in
-            (SELECT posthog_pluginconfig.plugin_id
-                FROM posthog_pluginconfig
-                LEFT JOIN posthog_team ON posthog_team.id = posthog_pluginconfig.team_id
-                WHERE (posthog_team.id IS NULL OR posthog_team.plugins_opt_in='t') AND posthog_pluginconfig.enabled='t'
-                GROUP BY posthog_pluginconfig.plugin_id)`
+        `SELECT posthog_plugin.* FROM posthog_plugin
+            WHERE id IN (${pluginConfigsInForceQuery('plugin_id')} GROUP BY posthog_pluginconfig.plugin_id)`
     )
     return pluginRows
 }
 
 export async function getPluginAttachmentRows(server: PluginsServer): Promise<PluginAttachmentDB[]> {
     const { rows }: { rows: PluginAttachmentDB[] } = await server.db.postgresQuery(
-        `SELECT posthog_pluginattachment.* FROM posthog_pluginattachment WHERE plugin_config_id in
-            (SELECT posthog_pluginconfig.id
-                FROM posthog_pluginconfig
-                LEFT JOIN posthog_team ON posthog_team.id = posthog_pluginconfig.team_id
-                WHERE (posthog_team.id IS NULL OR posthog_team.plugins_opt_in='t') AND posthog_pluginconfig.enabled='t')`
+        `SELECT posthog_pluginattachment.* FROM posthog_pluginattachment
+            WHERE plugin_config_id IN (${pluginConfigsInForceQuery('id')})`
     )
     return rows
 }
 
 export async function getPluginConfigRows(server: PluginsServer): Promise<PluginConfig[]> {
-    const { rows }: { rows: PluginConfig[] } = await server.db.postgresQuery(
-        `SELECT posthog_pluginconfig.*
-            FROM posthog_pluginconfig
-            LEFT JOIN posthog_team ON posthog_team.id = posthog_pluginconfig.team_id
-            WHERE (posthog_team.id IS NULL OR posthog_team.plugins_opt_in='t') AND posthog_pluginconfig.enabled='t'`
-    )
+    const { rows }: { rows: PluginConfig[] } = await server.db.postgresQuery(pluginConfigsInForceQuery())
     return rows
 }
 

diff --git a/src/types.ts b/src/types.ts
@@ -99,6 +99,7 @@ export interface Plugin {
     name: string
     plugin_type: 'local' | 'respository' | 'custom' | 'source'
     description?: string
+    is_global: boolean
     url?: string
     config_schema: Record<string, PluginConfigSchema> | PluginConfigSchema[]
     tag?: string

diff --git a/src/utils.ts b/src/utils.ts
@@ -9,7 +9,7 @@ import * as tar from 'tar-stream'
 import * as zlib from 'zlib'
 
 import { status } from './status'
-import { LogLevel, PluginsServerConfig, TimestampFormat } from './types'
+import { LogLevel, Plugin, PluginsServerConfig, TimestampFormat } from './types'
 
 /** Time until autoexit (due to error) gives up on graceful exit and kills the process right away. */
 const GRACEFUL_EXIT_PERIOD_SECONDS = 5
@@ -388,6 +388,14 @@ export async function createRedis(serverConfig: PluginsServerConfig): Promise<Re
     return redis
 }
 
+export function pluginDigest(plugin: Plugin): string {
+    const extras = [`organization ${plugin.organization_id}`]
+    if (plugin.is_global) {
+        extras.push('GLOBAL')
+    }
+    return `plugin "${plugin.name}" (${extras.join(' - ')})`
+}
+
 export function createPostgresPool(serverConfig: PluginsServerConfig): Pool {
     const postgres = new Pool({
         connectionString: serverConfig.DATABASE_URL,

diff --git a/tests/helpers/plugins.ts b/tests/helpers/plugins.ts
@@ -43,6 +43,7 @@ export const plugin60: Plugin = {
     error: undefined,
     from_json: false,
     from_web: false,
+    is_global: false,
     created_at: new Date().toISOString(),
     updated_at: new Date().toISOString(),
 }

diff --git a/tests/helpers/sql.ts b/tests/helpers/sql.ts
@@ -79,6 +79,7 @@ export async function createUserTeamAndOrganization(
     await insertRow(db, 'posthog_organization', {
         id: organizationId,
         name: 'TEST ORG',
+        plugins_access_level: 9,
         created_at: new Date().toISOString(),
         updated_at: new Date().toISOString(),
         personalization: '{}',

diff --git a/tests/plugins.test.ts b/tests/plugins.test.ts
@@ -8,6 +8,7 @@ import { loadSchedule, setupPlugins } from '../src/plugins/setup'
 import { createServer } from '../src/server'
 import { LogLevel, PluginsServer } from '../src/types'
 import {
+    commonOrganizationId,
     mockPluginTempFolder,
     mockPluginWithArchive,
     plugin60,
@@ -202,7 +203,7 @@ test('archive plugin with broken plugin.json does not do much', async () => {
     expect(processError).toHaveBeenCalledWith(
         mockServer,
         pluginConfigs.get(39)!,
-        'Can not load plugin.json for plugin "test-maxmind-plugin"'
+        `Can not load plugin.json for plugin "test-maxmind-plugin" (organization ${commonOrganizationId})`
     )
 
     expect(await pluginConfigs.get(39)!.vm!.getTasks()).toEqual({})
@@ -250,7 +251,7 @@ test('plugin with http urls must have an archive', async () => {
     expect(processError).toHaveBeenCalledWith(
         mockServer,
         pluginConfigs.get(39)!,
-        'Un-downloaded remote plugins not supported! Plugin: "test-maxmind-plugin"'
+        `Tried using undownloaded remote plugin "test-maxmind-plugin" (organization ${commonOrganizationId}), which is not supported!`
     )
     expect(await pluginConfigs.get(39)!.vm!.getTasks()).toEqual({})
 })

diff --git a/tests/sql.test.ts b/tests/sql.test.ts
@@ -86,6 +86,7 @@ test('getPluginRows', async () => {
             from_json: false,
             from_web: false,
             id: 60,
+            is_global: false,
             organization_id: commonOrganizationId,
             latest_tag: null,
             latest_tag_checked_at: null,