Skip to content
Snippets Groups Projects
Unverified Commit 13eb9ae2 authored by Dennis Schridde's avatar Dennis Schridde Committed by GitHub
Browse files

Omit database details from H2 dump when is_attached_dwh (#50036)

== Goal ==

Hide attached DWH database details from anyone incl. admins:
* Do not show them in the UI
* Do not permit to change them
* Do not serialize them
  - This is what we're adding for H2 snapshots (dumps) with this PR.

The aim is that customers cannot gain access to (parts of) credentials,
and they cannot break a feature they are paying for by changing
connection details.

== Implementation ==

In 592360c9 I wrongly understood that
database details would be omitted when dumping to H2, but this is only
true for dumping H2 databases.

Fix this by omitting database details also when dumping databases with
`is_attached_dwh` set.

== How to test ==

To prepare, download a H2 JAR matching the H2 version used by Metabase:
```
wget https://repo1.maven.org/maven2/com/h2database/h2/2.1.214/h2-2.1.214.jar
```

If the H2 version does not match, you will get an error when trying to
open the H2 shell:
```
Exception in thread "main" org.h2.jdbc.JdbcSQLNonTransientConnectionException: Unsupported database file version or invalid file header in file "[REDACTED]" [90048-232]
```

=== New behaviour ===

Setting the `is_attached_dwh` field hides the database details from H2 dumps:

1. Configure a database as described in https://www.metabase.com/docs/latest/configuring-metabase/config-file#databases.
   - In addition to the fields you would normally set, also set
     `is_attached_dwh: true`.
   - This also works when adding this flag to a database that previously
     did not have this flag set.
2. Start your Metabase instance.
3. Run `${metabase} dump-to-h2 ./dump-file-h2` to create the H2 snapshot file.
4. Run `java -cp h2-2.1.214.jar org.h2.tools.Shell -url "jdbc:h2:file:./dump-file-h2;ifexists=true"` to open a H2 shell.
5. Verify that `SELECT name,details FROM metabase_database;` shows `{}` for the database you added in step 1

=== Original behaviour ===

Behaviour without setting the `is_attached_dwh` field is unchanged:

1. Configure a database as described in https://www.metabase.com/docs/latest/configuring-metabase/config-file#databases.
   - Only set the fields you would normally set.  Do not set
     `is_attached_dwh` (or set it to `false`).
2. Start your Metabase instance.
3. Run `${metabase} dump-to-h2 ./dump-file-h2` to create the H2 snapshot file.
4. Run `java -cp h2-2.1.214.jar org.h2.tools.Shell -url "jdbc:h2:file:./dump-file-h2;ifexists=true"` to open a H2 shell.
5. Verify that `SELECT name,details FROM metabase_database;` shows a non-empty object (i.e. not `{}`) for the database you added in step 1

Fixes: 592360c9
Closes: https://github.com/metabase/harbormaster/issues/5526
parent 6c0d44d1
No related branches found
No related tags found
No related merge requests found
......@@ -164,11 +164,11 @@
;; Sample Database, the correct details are reset automatically on every
;; launch (see [[metabase.sample-data/update-sample-database-if-needed!]]), and we don't support connecting other H2
;; Databases in prod anyway, so this ultimately shouldn't cause anyone any problems.
(if *copy-h2-database-details*
identity
(map (fn [database]
(cond-> database
(= (:engine database) "h2") (assoc :details "{}")))))
(map (fn [database]
(cond-> database
(or (:is_attached_dwh database)
(and (not *copy-h2-database-details*)
(= (:engine database) "h2"))) (assoc :details "{}"))))
:model/Setting
;; Never create dumps with read-only-mode turned on.
;; It will be confusing to restore from and prevent key rotation.
......
......@@ -107,3 +107,38 @@
(is (not (= "{\"db\":\"/tmp/test.db\"}"
(:details (first (jdbc/query {:connection target-conn}
"select details from metabase_database where id=1;")))))))))))))))
(deftest dump-to-h2-dump-is-attached-dwh-test
(testing "dump-to-h2 --dump-plaintext with is_attached_dwh"
(let [h2-fixture-db-file @cmd.test-util/fixture-db-file-path
db-name (str "test_" (mt/random-name))]
(mt/with-temp-file [h2-file (format "out-%s.db" (mt/random-name))]
(mt/test-drivers #{:h2 :postgres :mysql}
(with-redefs [i18n.impl/site-locale-from-setting (constantly nil)]
(binding [config/*disable-setting-cache* true
mdb.connection/*application-db* (mdb.connection/application-db
driver/*driver*
(persistent-data-source driver/*driver* db-name))]
(when-not (= driver/*driver* :h2)
(tx/create-db! driver/*driver* {:database-name db-name}))
(binding [copy/*copy-h2-database-details* true]
(load-from-h2/load-from-h2! h2-fixture-db-file)
(encryption-test/with-secret-key "89ulvIGoiYw6mNELuOoEZphQafnF/zYe+3vT+v70D1A="
(t2/insert! Database {:engine "h2"
:name "normal-db"
:details {:db "/tmp/test.db"}
:is_attached_dwh false})
(t2/insert! Database {:engine "h2"
:name "attached-dwh"
:details {:db "/tmp/test.db"}
:is_attached_dwh true})
(dump-to-h2/dump-to-h2! h2-file {:dump-plaintext? true})))
(with-open [target-conn (.getConnection (copy.h2/h2-data-source h2-file))]
(testing "preserves details when is_attached_dwh is not set"
(is (= "{\"db\":\"/tmp/test.db\"}"
(:details (first (jdbc/query {:connection target-conn}
"select details from metabase_database where name='normal-db';"))))))
(testing "preserves details when is_attached_dwh is not set"
(is (= "{}"
(:details (first (jdbc/query {:connection target-conn}
"select details from metabase_database where name='attached-dwh';"))))))))))))))
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment