From 2bd0b3c73263accdb568937dcf9e381230f4e5ea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?N=C3=ADcolas=20Sims=20Botelho?= <nbotelho.dev@gmail.com>
Date: Wed, 17 May 2023 13:17:27 -0300
Subject: [PATCH] Add Basic Authentication to Druid (#20412)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* feat: allow authorization token for druid database
* fix: adjust in basic auth header and new field username
* fix: add scenario test
* fix: remove debug code
* fix: use metabase-plugin for new fields and improve tests
* fix: change type of auth-token to password
* fix: adjustment tested
* fix: adjustment tested
---------
Co-authored-by: NÃcolas Botelho <nicolas.botelho@piposaude.com.br>
Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>
---
.../druid/resources/metabase-plugin.yaml | 18 ++++++++++
.../druid/src/metabase/driver/druid.clj | 5 ++-
.../src/metabase/driver/druid/client.clj | 23 ++++++++++---
.../druid/src/metabase/driver/druid/sync.clj | 8 ++++-
.../metabase/driver/druid/client_test.clj | 33 +++++++++++++++++++
5 files changed, 81 insertions(+), 6 deletions(-)
diff --git a/modules/drivers/druid/resources/metabase-plugin.yaml b/modules/drivers/druid/resources/metabase-plugin.yaml
index 1ceb96174b2..9fb46d2bff5 100644
--- a/modules/drivers/druid/resources/metabase-plugin.yaml
+++ b/modules/drivers/druid/resources/metabase-plugin.yaml
@@ -17,7 +17,25 @@ driver:
- cloud-ip-address-info
- ssh-tunnel
- advanced-options-start
+ - name: auth-enabled
+ display-name: Authentication header
+ default: false
+ type: boolean
+ visible-if:
+ advanced-options: true
+ - name: auth-username
+ display-name: Username
+ type: string
+ visible-if:
+ auth-enabled: true
+ - name: auth-token
+ display-name: Token
+ type: secret
+ secret-kind: password
+ visible-if:
+ auth-enabled: true
- default-advanced-options
+
init:
- step: load-namespace
namespace: metabase.driver.druid
diff --git a/modules/drivers/druid/src/metabase/driver/druid.clj b/modules/drivers/druid/src/metabase/driver/druid.clj
index fbc2890e4fd..1ab80f52752 100644
--- a/modules/drivers/druid/src/metabase/driver/druid.clj
+++ b/modules/drivers/druid/src/metabase/driver/druid.clj
@@ -20,7 +20,10 @@
[_ details]
{:pre [(map? details)]}
(ssh/with-ssh-tunnel [details-with-tunnel details]
- (= 200 (:status (http/get (druid.client/details->url details-with-tunnel "/status"))))))
+ (let [{:keys [auth-enabled auth-username auth-token-value]} details]
+ (= 200 (:status (http/get (druid.client/details->url details-with-tunnel "/status")
+ (cond-> {}
+ auth-enabled (assoc :basic-auth (str auth-username ":" auth-token-value)))))))))
(defmethod driver/describe-table :druid
[_ database table]
diff --git a/modules/drivers/druid/src/metabase/driver/druid/client.clj b/modules/drivers/druid/src/metabase/driver/druid/client.clj
index 1fb721d810e..67e09a91557 100644
--- a/modules/drivers/druid/src/metabase/driver/druid/client.clj
+++ b/modules/drivers/druid/src/metabase/driver/druid/client.clj
@@ -2,6 +2,7 @@
(:require [cheshire.core :as json]
[clj-http.client :as http]
[clojure.core.async :as a]
+ [metabase.models.secret :as secret]
[metabase.query-processor.error-type :as qp.error-type]
[metabase.util :as u]
[metabase.util.i18n :refer [trs tru]]
@@ -25,8 +26,11 @@
[request-fn url & {:as options}]
{:pre [(fn? request-fn) (string? url)]}
;; this is the way the `Content-Type` header is formatted in requests made by the Druid web interface
- (let [options (cond-> (merge {:content-type "application/json;charset=UTF-8"} options)
- (:body options) (update :body json/generate-string))]
+ (let [{:keys [auth-enabled auth-username auth-token-value]} options
+ options (cond-> (merge {:content-type "application/json;charset=UTF-8"} options)
+ (:body options) (update :body json/generate-string)
+ auth-enabled (assoc :basic-auth (str auth-username ":" auth-token-value)))]
+
(try
(let [{:keys [status body]} (request-fn url options)]
(when (not= status 200)
@@ -62,7 +66,13 @@
{:pre [(map? details) (map? query)]}
(ssh/with-ssh-tunnel [details-with-tunnel details]
(try
- (POST (details->url details-with-tunnel "/druid/v2"), :body query)
+ (POST (details->url details-with-tunnel "/druid/v2"),
+ :body query
+ :auth-enabled (:auth-enabled details)
+ :auth-username (:auth-username details)
+ :auth-token-value (-> details
+ (secret/db-details-prop->secret-map "auth-token")
+ secret/value->string))
;; don't need to do anything fancy if the query was killed
(catch InterruptedException e
(throw e))
@@ -82,7 +92,12 @@
(log/warn (trs "Client closed connection, canceling Druid queryId {0}" query-id))
(try
(log/debug (trs "Canceling Druid query with ID {0}" query-id))
- (DELETE (details->url details-with-tunnel (format "/druid/v2/%s" query-id)))
+ (DELETE (details->url details-with-tunnel (format "/druid/v2/%s" query-id))
+ :auth-enabled (:auth-enabled details)
+ :auth-username (:auth-username details)
+ :auth-token-value (-> details
+ (secret/db-details-prop->secret-map "auth-token")
+ secret/value->string))
(catch Exception cancel-e
(log/warn cancel-e (trs "Failed to cancel Druid query with queryId {0}" query-id)))))))
diff --git a/modules/drivers/druid/src/metabase/driver/druid/sync.clj b/modules/drivers/druid/src/metabase/driver/druid/sync.clj
index dcbc7c1f0db..3a9877ea5c4 100644
--- a/modules/drivers/druid/src/metabase/driver/druid/sync.clj
+++ b/modules/drivers/druid/src/metabase/driver/druid/sync.clj
@@ -1,6 +1,7 @@
(ns metabase.driver.druid.sync
(:require [medley.core :as m]
[metabase.driver.druid.client :as druid.client]
+ [metabase.models.secret :as secret]
[metabase.util.ssh :as ssh]))
(defn- do-segment-metadata-query [details datasource]
@@ -50,7 +51,12 @@
[database]
{:pre [(map? (:details database))]}
(ssh/with-ssh-tunnel [details-with-tunnel (:details database)]
- (let [druid-datasources (druid.client/GET (druid.client/details->url details-with-tunnel "/druid/v2/datasources"))]
+ (let [druid-datasources (druid.client/GET (druid.client/details->url details-with-tunnel "/druid/v2/datasources")
+ :auth-enabled (-> database :details :auth-enabled)
+ :auth-username (-> database :details :auth-username)
+ :auth-token-value (-> (:details database)
+ (secret/db-details-prop->secret-map "auth-token")
+ secret/value->string))]
{:tables (set (for [table-name druid-datasources]
{:schema nil, :name table-name}))})))
diff --git a/modules/drivers/druid/test/metabase/driver/druid/client_test.clj b/modules/drivers/druid/test/metabase/driver/druid/client_test.clj
index 46a83850ca0..3ae40e72cad 100644
--- a/modules/drivers/druid/test/metabase/driver/druid/client_test.clj
+++ b/modules/drivers/druid/test/metabase/driver/druid/client_test.clj
@@ -70,3 +70,36 @@
(or (when (instance? java.net.ConnectException e)
(throw e))
(some-> (.getCause e) recur)))))))))
+
+(defn- test-request
+ ([request-fn]
+ (test-request request-fn true))
+ ([request-fn basic-auth?]
+ (try
+ (request-fn "http://localhost:8082/druid/v2"
+ :auth-enabled basic-auth?
+ :auth-username "nbotelho"
+ :auth-token-value "12345678910")
+ (catch Exception e
+ (ex-data e)))))
+
+(defn- get-auth-header [m]
+ (select-keys (:request-options m) [:basic-auth]))
+
+(deftest basic-auth-test
+ (let [get-request (test-request druid.client/GET)
+ post-request (test-request druid.client/POST)
+ delete-request (test-request druid.client/DELETE)]
+ (is (= {:basic-auth "nbotelho:12345678910"}
+ (get-auth-header get-request)
+ (get-auth-header post-request)
+ (get-auth-header delete-request)) "basic auth header included with successfully"))
+
+ (let [no-auth-basic false
+ get-request (test-request druid.client/GET no-auth-basic)
+ post-request (test-request druid.client/POST no-auth-basic)
+ delete-request (test-request druid.client/DELETE no-auth-basic)]
+ (is (= no-auth-basic
+ (contains? (get-auth-header get-request) :basic-auth)
+ (contains? (get-auth-header post-request) :basic-auth)
+ (contains? (get-auth-header delete-request) :basic-auth)) "basic auth header not included")))
--
GitLab