Revert "feat(auth): Add Kerberos support for Trino/Presto (growthbook#4850)" (growthbook#4911)

This reverts commit c2aa785.

Author: tzjames

Dockerfile

@@ -61,7 +61,7 @@ FROM python:${PYTHON_MAJOR}-slim
 ARG NODE_MAJOR
 WORKDIR /usr/local/src/app
 RUN apt-get update && \
-  apt-get install -y wget gnupg2 build-essential ca-certificates libkrb5-dev && \
+  apt-get install -y wget gnupg2 build-essential ca-certificates && \
   mkdir -p /etc/apt/keyrings && \
   wget -qO- https://deb.nodesource.com/gpgkey/nodesource.gpg.key | gpg --dearmor -o /etc/apt/keyrings/nodesource.gpg && \
   echo "deb [signed-by=/etc/apt/keyrings/nodesource.gpg] https://deb.nodesource.com/node_$NODE_MAJOR.x buster main" > /etc/apt/sources.list.d/nodesource.list && \

packages/back-end/package.json

@@ -52,7 +52,6 @@
     "@stdlib/random": "^0.1.1",
     "@stdlib/stats": "^0.1.1",
     "@types/handlebars": "^4.1.0",
-    "@types/presto-client": "^1.0.2",
     "agenda": "^5.0.0",
     "asn1.js": "^5.4.1",
     "async": "^3.1.0",
@@ -81,7 +80,6 @@
     "json5": "^2.2.3",
     "jsonwebtoken": "^8.5.1",
     "jwks-rsa": "^1.8.0",
-    "kerberos": "^2.1.1",
     "lodash": "^4.17.21",
     "md5": "^2.3.0",
     "module-alias": "^2.2.3",
@@ -131,7 +129,6 @@
     "@types/jest": "^27.0.1",
     "@types/js-yaml": "^4.0.2",
     "@types/jsonwebtoken": "^8.5.1",
-    "@types/kerberos": "^1.1.5",
     "@types/lodash": "^4.14.157",
     "@types/md5": "^2.2.0",
     "@types/module-alias": "^2.0.4",

packages/back-end/src/controllers/datasources.ts

@@ -259,7 +259,6 @@ export async function postDataSources(
       datasource: getDataSourceWithParams(integration),
     });
   } catch (e) {
-    req.log.error(e, "Failed to create data source");
     res.status(400).json({
       status: 400,
       message: e.message || "An error occurred",

packages/back-end/src/integrations/Presto.ts

@@ -1,4 +1,5 @@
-import { Client, ClientOptions, QueryOptions } from "presto-client";
+/// <reference types="../../typings/presto-client" />
+import { Client, IPrestoClientOptions } from "presto-client";
 import { format } from "shared/sql";
 import { FormatDialect } from "shared/src/types";
 import { prestoCreateTablePartitions } from "shared/enterprise";
@@ -10,7 +11,6 @@ import {
   MaxTimestampIncrementalUnitsQueryParams,
   MaxTimestampMetricSourceQueryParams,
 } from "back-end/src/types/Integration";
-import { getKerberosHeader } from "../util/kerberos.util";
 import SqlIntegration from "./SqlIntegration";
 
 // eslint-disable-next-line
@@ -36,8 +36,7 @@ export default class Presto extends SqlIntegration {
     return true;
   }
   runQuery(sql: string): Promise<QueryResponse> {
-    const configOptions: ClientOptions = {
-      engine: this.params.engine,
+    const configOptions: IPrestoClientOptions = {
       host: this.params.host,
       port: this.params.port,
       user: "growthbook",
@@ -56,18 +55,6 @@ export default class Presto extends SqlIntegration {
     if (this.params?.authType === "customAuth") {
       configOptions.custom_auth = this.params.customAuth || "";
     }
-    if (this.params?.authType === "kerberos") {
-      const servicePrincipal = this.params.kerberosServicePrincipal;
-      const clientPrincipal = this.params.kerberosClientPrincipal;
-      if (!servicePrincipal) {
-        throw new Error(
-          "Kerberos service principal is required for Kerberos authentication",
-        );
-      }
-      // Use a function to generate fresh Kerberos tokens for each request
-      configOptions.custom_auth = () =>
-        getKerberosHeader(servicePrincipal, clientPrincipal);
-    }
     if (this.params?.ssl) {
       configOptions.ssl = {
         ca: this.params?.caCert,
@@ -83,7 +70,7 @@ export default class Presto extends SqlIntegration {
       const rows: Row[] = [];
       const statistics: QueryStatistics = {};
 
-      const executeOptions: QueryOptions = {
+      client.execute({
         query: sql,
         catalog: this.params.catalog,
         schema: this.params.schema,
@@ -110,7 +97,6 @@ export default class Presto extends SqlIntegration {
             statistics.bytesProcessed = Number(stats.processedBytes);
             statistics.rowsProcessed = Number(stats.processedRows);
             statistics.physicalWrittenBytes = Number(
-              // @ts-expect-error - From our testing this does exist but types are not happy
               stats.physicalWrittenBytes,
             );
           }
@@ -124,15 +110,7 @@ export default class Presto extends SqlIntegration {
             statistics,
           });
         },
-      };
-
-      // For Kerberos auth we need to explicitly set user
-      // which sets X-Trino-User header and is required by Trino
-      if (this.params?.authType === "kerberos") {
-        executeOptions.user = this.params.kerberosUser || "growthbook";
-      }
-
-      client.execute(executeOptions);
+      });
     });
   }
   addTime(

packages/back-end/src/util/kerberos.util.ts

@@ -1,14 +1,11 @@
 export interface PrestoConnectionParams {
-  authType?: "basicAuth" | "none" | "customAuth" | "kerberos";
+  authType?: "basicAuth" | "none" | "customAuth";
   engine: "presto" | "trino";
   host: string;
   port: number;
   username?: string;
   password?: string;
   customAuth?: string;
-  kerberosServicePrincipal?: string;
-  kerberosClientPrincipal?: string;
-  kerberosUser?: string;
   source?: string;
   catalog: string;
   schema: string;

packages/back-end/types/integrations/presto.d.ts

@@ -0,0 +1,119 @@
+/* eslint-disable */
+declare module "presto-client" {
+  interface IPrestoClientOptions {
+    host: string;
+    ssl?: {
+      ca?: string;
+      cert: string;
+      ciphers?: string;
+      key?: string;
+      passphrase?: string;
+      secureProtocol: string;
+      pfx?: string;
+      rejectUnauthorized?: boolean;
+      servername?: string;
+    };
+    port: number;
+    source: string;
+    user: string;
+    catalog: string;
+    checkInterval: number;
+    basic_auth?: {
+      user: string;
+      password: string;
+    };
+    custom_auth?: string;
+    schema?: string;
+    enableVerboseStateCallback?: boolean;
+    jsonParser?: {
+      parse: (data: any) => any;
+    };
+    engine?: string;
+    timeout?: number;
+  }
+
+  enum PrestoClientQueryStates {
+    WAITING_FOR_PREREQUISITES = "WAITING_FOR_PREREQUISITES",
+    QUEUED = "QUEUED",
+    WAITING_FOR_RESOURCES = "WAITING_FOR_RESOURCES",
+    DISPATCHING = "DISPATCHING",
+    PLANNING = "PLANNING",
+    STARTING = "STARTING",
+    RUNNING = "RUNNING",
+    FINISHING = "FINISHING",
+    FINISHED = "FINISHED",
+    CANCELED = "CANCELED",
+    FAILED = "FAILED",
+  }
+
+  interface IPrestoClientStats {
+    processedBytes: number;
+    processedRows: number;
+    wallTimeMillis: number;
+    cpuTimeMillis: number;
+    userTimeMillis: number;
+    physicalWrittenBytes: number;
+    state: PrestoClientQueryStates;
+    scheduled: boolean;
+    nodes: number;
+    totalSplits: number;
+    queuedSplits: number;
+    runningSplits: number;
+    completedSplits: number;
+  }
+
+  // NOTE: Needs to be extended with missing items
+  enum PrestoClientPrestoTypes {
+    varchar = "varchar",
+    bigint = "bigint",
+    boolean = "boolean",
+    char = "char",
+    date = "date",
+    decimal = "decimal",
+    double = "double",
+  }
+
+  interface IPrestoClientColumnMetaData {
+    name: string;
+    type: PrestoClientPrestoTypes;
+  }
+
+  type PrestoClientColumnDatum = [string, any];
+
+  interface IPrestoClientExecuteOptions {
+    query: string;
+    catalog: string;
+    schema: string;
+    timezone?: string;
+    user?: string;
+    prepares?: string[];
+    timeout?: null | number;
+    info?: boolean;
+    cancel?: () => boolean;
+    state?: (error: any, query_id: string, stats: IPrestoClientStats) => void;
+    columns?: (error: any, data: IPrestoClientColumnMetaData[]) => void;
+    data?: (
+      error: any,
+      data: PrestoClientColumnDatum[],
+      columns: IPrestoClientColumnMetaData[],
+      stats: IPrestoClientStats,
+    ) => void;
+    success?: (error: any, stats: IPrestoClientStats) => void;
+    error?: (error: any) => void;
+    callback?: (error: any, stats: IPrestoClientStats) => void;
+  }
+
+  class Client {
+    constructor(options: IPrestoClientOptions);
+    public execute(options: IPrestoClientExecuteOptions): void;
+    public query(
+      query_id: string,
+      callback: (error: any, data?: any) => void,
+    ): void;
+    public kill(query_id: string, callback: (error: any) => void): void;
+    public nodes(
+      opts: null | undefined | {},
+      callback: (error: any, data: {}[]) => void,
+    ): void;
+  }
+}