add a test to make sure H2 DBs don't allow SQL queries with no USER

src/metabase/driver/generic_sql/native.clj

@@ -23,7 +23,7 @@
   {:pre [(string? sql)
          (integer? database-id)]}
   (log/debug "QUERY: \n"
-             (with-out-str (clojure.pprint/pprint query)))
+             (with-out-str (clojure.pprint/pprint (update query :driver class))))
   (try (let [database (sel :one [Database :engine :details] :id database-id)
              db (-> database
                     db->korma-db

test/metabase/driver/generic_sql/native_test.clj

@@ -2,7 +2,9 @@
   (:require [clojure.tools.logging :as log]
             [colorize.core :as color]
             [expectations :refer :all]
+            [metabase.db :refer [ins cascade-delete]]
             [metabase.driver :as driver]
+            [metabase.models.database :refer [Database]]
             [metabase.test.data :refer :all]))
 
 ;; Just check that a basic query works
@@ -40,3 +42,13 @@
               :stacktrace
               :query
               :expanded-query)))
+
+;; Check that we're not allowed to run SQL against an H2 database with a non-admin account
+(expect "Running SQL queries against H2 databases using the default (admin) database user is forbidden."
+  ;; Insert a fake Database. It doesn't matter that it doesn't actually exist since query processing should
+  ;; fail immediately when it realizes this DB doesn't have a USER
+  (let [db (ins Database :name "Fake-H2-DB", :engine "h2", :details {:db "mem:fake-h2-db"})]
+    (try (:error (driver/process-query {:database (:id db)
+                                        :type     :native
+                                        :native   {:query "SELECT 1;"}}))
+         (finally (cascade-delete Database :name "Fake-H2-DB")))))

test/metabase/test/data/h2.clj

@@ -28,10 +28,7 @@
 (defn- connection-details
   "Return a Metabase `Database.details` for H2 database defined by DATABASE-DEFINITION."
   [^DatabaseDefinition {:keys [short-lived?], :as database-definition}]
-  {:db (str (format "mem:%s" (escaped-name database-definition))
-            ;; For non "short-lived" (temp) databases keep the connection open for the duration of unit tests
-            (when-not short-lived?
-              ";DB_CLOSE_DELAY=-1"))
+  {:db           (format "mem:%s" (escaped-name database-definition))
    :short-lived? short-lived?})
 
 (defn- korma-connection-pool
@@ -93,11 +90,17 @@
   (create-physical-db! [this database-definition]
     ;; Create the "physical" database which in this case actually just means creating the schema
     (generic/create-physical-db! this (format-for-h2 database-definition))
+
     ;; Now create a non-admin account 'GUEST' which will be used from here on out
     (generic/execute-sql! this database-definition "CREATE USER IF NOT EXISTS GUEST PASSWORD 'guest';")
     ;; Grant the GUEST account SELECT permissions for all the Tables in this DB
     (doseq [{:keys [table-name]} (:table-definitions database-definition)]
-      (generic/execute-sql! this database-definition (format "GRANT SELECT ON %s TO GUEST" table-name))))
+      (generic/execute-sql! this database-definition (format "GRANT SELECT ON %s TO GUEST;" table-name)))
+
+    ;; If this isn't a "short-lived" database we need to set DB_CLOSE_DELAY to -1 here because only admins are allowed to do it
+    ;; so we can't set it via the connection string :/
+    (when-not (:short-lived? database-definition)
+      (generic/execute-sql! this database-definition "SET DB_CLOSE_DELAY -1;")))
 
   (load-table-data! [this database-definition table-definition]
     (generic/load-table-data! this database-definition table-definition))